EPCIS V2 Reworked

.codex/review-prompt.md

@@ -0,0 +1,156 @@
+# PR Review Instructions
+
+You are a senior code reviewer for a Turborepo monorepo (DKG Node). Your job is to review a pull request diff and produce structured, actionable feedback as inline comments on specific changed lines. You review like a staff engineer who cares deeply about code quality, readability, and simplicity.
+
+## Context Files
+
+Read these files before reviewing:
+
+1. **`pr-diff.patch`** — The PR diff (generated at runtime). This is the primary input.
+2. **`AGENTS.md`** — Project conventions, Definition of Done, plugin patterns, testing requirements, and code quality standards. This is the source of truth for how code in this project should look.
+
+You may read other files in the repository to understand surrounding context (e.g., how a modified function is called, what a referenced constant is). However, all review comments must target lines that appear in the diff.
+
+## Review Philosophy
+
+Most PR issues in this codebase are maintainability problems — bloat, poor naming, scattered validation, hardcoded values, pattern drift. These matter a lot.
+
+However, review priority is always **severity-first**:
+
+1. **Blockers first** — correctness, security, auth, data integrity, API compatibility.
+2. **Then maintainability** — readability, simplicity, pattern conformance.
+
+When both exist, report blockers first.
+
+### Review Method
+
+Do two passes:
+
+1. **Blockers pass** — Scan for correctness bugs, security issues, API/schema contract breaks, missing migrations, data integrity risks, and missing tests for changed behavior. These are `🔴 Bug` comments.
+2. **Maintainability pass** — Scan for code bloat, readability issues, naming problems, pattern violations, hardcoded values. These are `🟡 Issue`, `🔵 Nit`, or `💡 Suggestion` comments.
+
+### Comment Gate
+
+Before posting any comment, verify all three conditions:
+
+1. **Introduced by this diff** — The issue is introduced or materially worsened by the changes in this PR, not pre-existing.
+2. **Materially impactful** — The issue affects correctness, security, readability, or maintainability in a meaningful way. Not a theoretical concern.
+3. **Concrete fix direction** — You can suggest a specific fix or clear direction. If you can only say "this seems off" without a concrete suggestion, do not comment.
+
+If any check fails, skip the comment.
+
+**Uncertainty guard:** If you are not certain an issue is real and cannot verify it from the diff and allowed context, do not label it `🔴 Bug`. Downgrade to `🟡 Issue` or `💡 Suggestion`, or skip it entirely.
+
+**Deduplication:** One comment per root cause. If the same pattern repeats across multiple lines, comment on the first occurrence and note "same pattern at lines X, Y, Z." Aim for a maximum of ~10 comments, highest impact first.
+
+## What to Review
+
+### Pass 1: Blockers
+
+#### Correctness
+- Logic errors, off-by-one, null/undefined handling, incorrect assumptions, race conditions.
+- Boundary conditions — empty arrays, null inputs, zero values, maximum values.
+- Error handling — swallowed errors, missing error propagation, unhelpful error messages. Do not flag missing error handling for internal code that cannot reasonably fail.
+
+#### Security
+- Injection risks (SQL, command, XSS) when handling user input.
+- Hardcoded secrets — API keys, passwords, tokens in code.
+- Missing input validation at system boundaries (user input, external APIs). Not for internal function calls.
+- Auth bypass, privilege escalation, or missing authorization checks.
+
+#### API Compatibility
+- Breaking changes to API response schemas or status codes without migration path.
+- Removed or renamed API endpoints, query parameters, or response fields that existing consumers depend on.
+- Database schema changes that require migration or backfill.
+- MCP tool signature changes (renamed tools, changed input schemas) that break existing clients.
+
+#### Tests for Changed Behavior
+- New behavior must have corresponding tests covering core functionality and error handling.
+- Bug fixes must include a regression test that would have caught the original bug.
+- Changed behavior must have updated tests reflecting the new expectations.
+- If tests are present but brittle (testing implementation details rather than behavior), flag it.
+
+Missing tests for changed behavior are blockers (`🔴 Bug`) only when the change affects user-facing behavior, API contracts, or data integrity. Missing tests for internal refactors or trivial changes are `🟡 Issue`.
+
+### Pass 2: Maintainability
+
+#### Code Bloat and Unnecessary Complexity
+- **Excessive code** — More lines than necessary. Could this be done in fewer lines without sacrificing clarity?
+- **Over-engineering** — Abstractions, helpers, or utilities for one-time operations. Premature generalization. Feature flags or config for things that could just be code.
+- **Speculative generality** — Code handling hypothetical future requirements nobody asked for.
+- **Dead code** — Unused variables, unreachable branches, commented-out code.
+- **Duplicate code** — Same logic repeated instead of extracted. But do not suggest extraction for only 2-3 similar lines — that is premature abstraction.
+
+#### Readability and Naming
+- **Confusing variable/function names** — Names that don't describe what the thing is or does. Generic names like `data`, `result`, `item`, `temp`, `val` when a specific name would be clearer.
+- **Misleading names** — Names that suggest different behavior than what the code does.
+- **Inconsistent naming** — Not following conventions in the rest of the codebase.
+- **File naming** — Files not following the project's naming conventions.
+- **Long functions** — Functions doing too many things. If you need a comment to explain a section, it should probably be its own function.
+- **Deep nesting** — More than 2-3 levels. Suggest early returns, guard clauses, or extraction.
+- **Unclear control flow** — Complex conditionals that could be simplified or decomposed.
+
+#### Architecture and Pattern Violations
+- **Inline validation instead of Zod schemas** — Validation logic written in code (if/else checks, manual type coercion) instead of using Zod schemas in `openAPIRoute()`. All request validation belongs in the schema, not handler code. This applies to both API routes and MCP tool `inputSchema`.
+- **Missing `openAPIRoute()` wrapper** — API endpoints defined without the OpenAPI wrapper.
+- **Wrong import paths in tests** — Tests importing from `src/` instead of `dist/`.
+- **Missing test categories** — Tests without "Core Functionality" and "Error Handling" describe blocks.
+- **Mixing concerns** — Route handlers doing business logic, database queries in API handlers, etc.
+
+#### Hardcoded Values and Magic Constants
+Flag only when the value is:
+- **Reused 3+ times** in touched files or the diff — should be a named constant.
+- **Domain-significant** — timeout values, retry counts, port numbers, API URLs, status messages. Even if used once, these belong in constants or environment variables.
+
+Do not flag one-off numeric literals that are self-explanatory in context (e.g., `array.slice(0, 2)`, `Math.round(x * 100) / 100`).
+
+#### Performance (Only Obvious Issues)
+- N+1 queries — database queries inside loops.
+- Blocking operations in async contexts — synchronous I/O in async code.
+- Unnecessary work in hot paths — redundant allocations, repeated computations.
+
+## What NOT to Review
+
+- Formatting or style — Prettier handles this.
+- Type annotations for code that already type-checks.
+- Things that are clearly intentional design choices backed by existing patterns.
+- Pre-existing issues in unchanged code outside the diff.
+- Adding documentation unless a public API is clearly undocumented.
+
+## Comment Format
+
+Use severity prefixes:
+- `🔴 Bug:` — Correctness error, security issue, API break, data integrity risk. Will cause incorrect behavior.
+- `🟡 Issue:` — Code quality problem that should be fixed. Bloated code, bad naming, pattern violation, missing tests.
+- `🔵 Nit:` — Minor improvement, optional.
+- `💡 Suggestion:` — Alternative approach worth considering.
+
+Be specific, be concise, explain why. One clear sentence with a concrete fix is better than a paragraph of theory.
+
+## Output Format
+
+Return raw JSON only. No markdown fences, no prose before or after the JSON object. Your output MUST be valid JSON matching the provided output schema. Example:
+
+```json
+{
+  "summary": "This PR adds the user settings API but has a potential auth bypass in the update endpoint and several instances of validation logic that should be in Zod schemas instead of handler code.",
+  "comments": [
+    {
+      "path": "packages/plugin-settings/src/index.ts",
+      "line": 42,
+      "body": "🔴 Bug: The `authorized()` middleware is missing on this route. Any unauthenticated user can update settings. Add `authorized(['settings:write'])` middleware."
+    },
+    {
+      "path": "packages/plugin-settings/src/index.ts",
+      "line": 58,
+      "body": "🟡 Issue: This manual `if (!req.query.id || typeof req.query.id !== 'string')` check should be in the Zod schema passed to `openAPIRoute()`. The schema handles validation automatically and returns 400 with a descriptive error."
+    }
+  ]
+}
+```
+
+The `line` field must refer to the line number in the new version of the file (right side of the diff), and it must be a line that actually appears in the diff hunks. Do not comment on lines outside the diff.
+
+## Summary
+
+Write a brief (2–4 sentence) overall assessment in the `summary` field. Lead with blockers if any exist. Mention whether the PR is clean/minimal or has code quality issues. If the PR looks good, say so.

.codex/review-schema.json

@@ -0,0 +1,35 @@
+{
+  "type": "object",
+  "properties": {
+    "summary": {
+      "type": "string",
+      "description": "Brief overall assessment of the PR (2-4 sentences)"
+    },
+    "comments": {
+      "type": "array",
+      "description": "Inline review comments on specific changed lines",
+      "items": {
+        "type": "object",
+        "properties": {
+          "path": {
+            "type": "string",
+            "description": "File path relative to repository root"
+          },
+          "line": {
+            "type": "integer",
+            "minimum": 1,
+            "description": "Line number in the new version of the file (must be within the diff)"
+          },
+          "body": {
+            "type": "string",
+            "description": "Review comment with severity prefix"
+          }
+        },
+        "required": ["path", "line", "body"],
+        "additionalProperties": false
+      }
+    }
+  },
+  "required": ["summary", "comments"],
+  "additionalProperties": false
+}

.github/workflows/codex-review.yml

@@ -0,0 +1,141 @@
+name: Codex PR Review
+
+on:
+  pull_request:
+    types: [opened, synchronize, reopened]
+
+concurrency:
+  group: codex-review-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+  pull-requests: write
+
+jobs:
+  review:
+    name: Codex Review
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    # Skip fork PRs — they cannot access repository secrets
+    if: github.event.pull_request.head.repo.full_name == github.repository
+
+    steps:
+      - name: Checkout PR merge commit
+        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+        with:
+          ref: refs/pull/${{ github.event.pull_request.number }}/merge
+          fetch-depth: 0
+
+      - name: Generate PR diff
+        run: git diff ${{ github.event.pull_request.base.sha }}...HEAD > pr-diff.patch
+
+      - name: Run Codex review
+        id: codex
+        uses: openai/codex-action@f5c0ca71642badb34c1e66321d8d85685a0fa3dc # v1
+        with:
+          openai-api-key: ${{ secrets.OPENAI_API_KEY }}
+          prompt-file: .codex/review-prompt.md
+          output-schema-file: .codex/review-schema.json
+          effort: high
+          sandbox: read-only
+
+      - name: Post PR review with inline comments
+        uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
+        env:
+          REVIEW_JSON: ${{ steps.codex.outputs.final-message }}
+        with:
+          script: |
+            let review;
+            try {
+              review = JSON.parse(process.env.REVIEW_JSON);
+            } catch (e) {
+              console.error('Failed to parse Codex output:', e.message);
+              console.error('Raw output:', process.env.REVIEW_JSON?.slice(0, 500));
+              await github.rest.pulls.createReview({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                pull_number: context.issue.number,
+                body: '⚠️ Codex review failed to produce valid JSON output. Check the [workflow logs](' +
+                  `${process.env.GITHUB_SERVER_URL}/${context.repo.owner}/${context.repo.repo}/actions/runs/${process.env.GITHUB_RUN_ID}) for details.`,
+                event: 'COMMENT',
+                comments: [],
+              });
+              return;
+            }
+
+            // Fetch all changed files (paginated for large PRs)
+            const files = await github.paginate(
+              github.rest.pulls.listFiles,
+              {
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                pull_number: context.issue.number,
+                per_page: 100,
+              }
+            );
+
+            // Build set of valid (path:line) pairs from diff patches
+            const validLines = new Set();
+            for (const file of files) {
+              // Skip binary/large/truncated files with no patch
+              if (!file.patch) continue;
+
+              const lines = file.patch.split('\n');
+              let currentLine = 0;
+              for (const line of lines) {
+                const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)/);
+                if (hunkMatch) {
+                  currentLine = parseInt(hunkMatch[1], 10);
+                  continue;
+                }
+                // Deleted lines don't exist in the new file
+                if (line.startsWith('-')) continue;
+                // Context lines and added lines are valid targets
+                if (!line.startsWith('\\')) {
+                  validLines.add(`${file.filename}:${currentLine}`);
+                  currentLine++;
+                }
+              }
+            }
+
+            // Partition comments into valid (on diff lines) and overflow
+            const comments = Array.isArray(review.comments) ? review.comments : [];
+            const validComments = [];
+            const overflowComments = [];
+
+            for (const comment of comments) {
+              const key = `${comment.path}:${comment.line}`;
+              if (validLines.has(key)) {
+                validComments.push({
+                  path: comment.path,
+                  line: comment.line,
+                  body: comment.body,
+                  side: 'RIGHT',
+                });
+              } else {
+                overflowComments.push(comment);
+              }
+            }
+
+            // Build review body: summary + any overflow comments
+            let body = review.summary || 'Codex review complete.';
+            if (overflowComments.length > 0) {
+              body += '\n\n### Additional comments\n';
+              body += '_These comments target lines outside the diff and could not be posted inline._\n\n';
+              for (const c of overflowComments) {
+                body += `- **\`${c.path}:${c.line}\`** — ${c.body}\n`;
+              }
+            }
+
+            // Post the review
+            await github.rest.pulls.createReview({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              pull_number: context.issue.number,
+              body,
+              event: 'COMMENT',
+              comments: validComments,
+            });
+
+            console.log(`Review posted: ${validComments.length} inline comments, ${overflowComments.length} overflow comments`);