From dd5dc69828d0ef7daf5125d0c2bb40b00ec2a73c Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:05:21 +0700 Subject: [PATCH 01/25] docs: add Approach A design spec for skill improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec covers 3 improvement areas for codex-review skill pack: 1. Critical bug fixes (5 bugs: phantom AskUserQuestion, nested backticks, $SCOPE undefined, invalid subagent_type, archive step + code editing) 2. Information barrier adoption for security/plan/impl-review 3. VERDICT vocabulary unification (APPROVE|REVISE → CONSENSUS|CONTINUE|STALEMATE) Passed 3-iteration spec review with all CRITICAL/MAJOR issues resolved. Co-Authored-By: Claude Opus 4.6 --- ...22-approach-a-skill-improvements-design.md | 312 ++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 docs/superpowers/specs/2026-03-22-approach-a-skill-improvements-design.md diff --git a/docs/superpowers/specs/2026-03-22-approach-a-skill-improvements-design.md b/docs/superpowers/specs/2026-03-22-approach-a-skill-improvements-design.md new file mode 100644 index 0000000..12e4deb --- /dev/null +++ b/docs/superpowers/specs/2026-03-22-approach-a-skill-improvements-design.md @@ -0,0 +1,312 @@ +# Design: Approach A — Skill Implementation Improvements + +**Date**: 2026-03-22 +**Scope**: codex-review skill pack (`skill-packs/codex-review/skills/`) +**Status**: Approved for implementation (v2 — post spec review) + +--- + +## Overview + +Approach A addresses three categories of issues found during comprehensive analysis of the 8 codex-review skills: + +1. **Critical bug fixes** — 5 runtime-breaking issues that cause incorrect behavior when skills are invoked +2. **Information barrier adoption** — apply the independent-analysis-before-polling pattern to 3 skills that lack it +3. **VERDICT vocabulary unification** — migrate 3 skills from final-state `APPROVE|REVISE` to debate-state `CONSENSUS|CONTINUE|STALEMATE` + +**Total files changed**: 15 existing files +**New files**: 1 (`codex-security-review/references/claude-analysis-template.md`) + +--- + +## Section 1: Critical Bug Fixes + +### Bug 1 — `codex-security-review`: Phantom `AskUserQuestion` tool + +**File**: `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` (Phase 1, Step 1) + +**Problem**: Workflow Phase 1 Step 1 instructs Claude to use `AskUserQuestion` which does not exist in Claude Code. The Smart Default Detection block at the top of the file already contains a correct announce line. + +**Fix**: Delete Phase 1 Step 1 entirely. The Smart Default Detection block at the top of the file already handles scope/effort detection and announces the result with the correct pattern: +``` +Announce: "Detected: scope=$SCOPE, effort=$EFFORT. Proceeding — reply to override." +``` +Do not add a second announce. Simply remove the Step 1 block that references `AskUserQuestion`. + +--- + +### Bug 2 — `codex-security-review`: Nested triple-backtick in prompts.md + +**Files**: `skill-packs/codex-review/skills/codex-security-review/references/prompts.md` AND `workflow.md` + +**Problem**: The `## Output Format` section and VERDICT block are embedded inline inside an outer triple-backtick fence, causing the inner fences to prematurely close the outer fence. This corrupts the prompt sent to Codex. Unlike other skills, security-review has no prompt assembly step and no `{OUTPUT_FORMAT}` placeholder mechanism. + +**Fix — two-part**: + +**Part A** (`prompts.md`): Remove the inline `## Output Format` and VERDICT Block sections from inside the fenced prompt block. Replace with `{OUTPUT_FORMAT}` placeholder at the end of the prompt body (before the closing fence). + +**Part B** (`workflow.md`): Add a prompt assembly step (before the `start` command) instructing Claude to build `$PROMPT` using multi-step placeholder replacement, matching the exact pattern used in `codex-impl-review` workflow.md Step 1.8. The pattern is: + +``` +1.8) Assemble prompt (multi-step replacement — DO NOT use a single sed pipeline): + a) Read references/prompts.md as the template base + b) Replace {SCOPE} with the detected scope value + c) Replace {EFFORT} with the detected effort value + d) Replace {WORKING_DIR} with the current working directory + e) Replace {SCOPE_SPECIFIC_INSTRUCTIONS} with the appropriate scope-specific block + f) Replace {OUTPUT_FORMAT} by reading references/output-format.md in full + (Use printf '%s' pattern to avoid sed special-character corruption) + g) Replace any remaining placeholders ({BASE_BRANCH} etc.) + Store as $PROMPT. + Verify $PROMPT contains no prematurely-closed triple-backtick fences before sending to start. +``` + +Note: Do NOT use a single `sed` subshell to inject `output-format.md` — the file content may contain `&`, `\`, or `/` characters that corrupt sed replacements. Use `printf '%s'` piping or equivalent safe substitution matching the impl-review Step 1.8 pattern. + +--- + +### Bug 3 — `codex-pr-review`: `$SCOPE` undefined in meta.json + +**File**: `skill-packs/codex-review/skills/codex-pr-review/references/workflow.md` (Session Finalization section) + +**Problem**: The heredoc writes `"scope": "$SCOPE"` but `$SCOPE` is never defined in the pr-review workflow. The correct variable is `$BASE_BRANCH`. + +**Fix**: Change `"scope": "$SCOPE"` → `"scope": "$BASE_BRANCH"`. + +--- + +### Bug 4 — `codex-parallel-review`: Invalid `subagent_type` + +**File**: `skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md` + +**Problem**: Agent spawn JSON uses `"subagent_type": "code-reviewer"` which is not a valid parameter in the Claude Code Agent tool, causing agent dispatch to fail. + +**Fix**: +1. Remove the `subagent_type` field from all 4 agent spawn JSON blocks (lines ~60, 70, 82, 93). +2. Update the prose description paragraph (line ~53) that references `subagent_type: "code-reviewer"` — remove the reference or replace with correct description of default agent behavior. + +--- + +### Bug 5 — `codex-security-review`: Archive step + code editing instruction + +**File**: `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` (Phase 3 Options + Phase 5 Step 3) + +**Problems**: +- Phase 5 Step 3 writes artifacts to `docs/security-reviews/` — violates v12 session model (all output stays in `.codex-review/sessions/`) +- Phase 3 Option A instructs Claude to "Apply the suggested fix" — the only skill that tells Claude to edit code during review, violating the review-only model shared by all other skills + +**Fix**: +- Remove Phase 5 Step 3 (archive step) entirely. Session dir is the sole output location. +- Remove Phase 3 Option A. Renumber: old Option B → new Option A (rebuttal), old Option C → new Option B (acknowledge with severity dispute). Remove the lettered "Option" framing and use flat list matching `codex-impl-review` pattern. +- Enumerate all `APPROVE`/`REVISE` occurrences in `workflow.md` that must be updated as part of Section 3 VERDICT migration (all in same file — fix together): + - Phase 3 stop condition: `VERDICT: APPROVE` → `VERDICT: CONSENSUS` + - Phase 4 Step 4 "Iterate Until Consensus" line: `VERDICT: APPROVE - All critical/high issues resolved` → `VERDICT: CONSENSUS - All critical/high issues resolved` + - Phase 5 Step 2 Final Verdict header: `{APPROVE | REVISE | STALEMATE}` → `{CONSENSUS | CONTINUE | STALEMATE}` + - CI/CD integration section pre-commit hook example: `grep -q "VERDICT: REVISE"` → `grep -q "VERDICT: CONTINUE"` + +--- + +## Section 2: Information Barrier Adoption + +**Pattern source**: `codex-pr-review` and `codex-commit-review` (gold standard) + +**Rationale**: Without an information barrier, Claude reads Codex's findings before forming its own analysis. This causes anchoring bias — Claude's "independent" analysis is actually influenced by Codex's framing, reducing genuine independent coverage. + +**Core pattern**: +1. After `start` Codex, Claude does NOT poll immediately +2. Claude independently analyzes the artifact in working memory and records `FINDING-{N}` findings +3. Only after independent analysis is complete does Claude poll Codex +4. Cross-analysis maps FINDING-{N} (Claude) against ISSUE-{N} (Codex) +5. The analysis is kept in Claude's working context (NOT written to a separate file) + +### `codex-security-review`: Add Step 2.5 + new `claude-analysis-template.md` + +**workflow.md change** — add after Phase 2 `start` command (new Phase 2, Step 2.5): +``` +2.5) INFORMATION BARRIER — Claude Independent Security Analysis + MUST complete before polling Codex output. + Codex is running in background (typically 90-180s) — use this time. + + Using references/claude-analysis-template.md: + - Read all files in scope directly (do NOT read $SESSION_DIR/review.md) + - Identify top attack surfaces + - Form independent FINDING-{N} list using OWASP categories + - Note high-confidence vs uncertain findings + + Keep analysis in working context. Do NOT write a file. + INFORMATION BARRIER ends after Round 1 poll completes. + From Round 2 onwards, the barrier no longer applies. +``` + +**workflow.md change** — update Phase 3 cross-analysis step to map FINDING-{N} (Claude) ↔ ISSUE-{N} (Codex) using the same classification table as pr-review (Genuine Agreement / Genuine Disagreement / Claude-only / Codex-only / Same Direction Different Severity). + +**New file**: `references/claude-analysis-template.md` +- FINDING-{N} format mirroring `output-format.md` with OWASP/CWE fields +- `Strongest Positions` section +- Cross-analysis Matching Protocol (maps FINDING-{N} ↔ ISSUE-{N}) + +### `codex-plan-review`: Add Step 2.5 + transform Step 4 + +**workflow.md change** — add after Step 2 (`start`) as new Step 2.5: +``` +2.5) INFORMATION BARRIER — Claude Independent Plan Analysis + MUST complete before polling Codex output. + Codex is running in background — use this time. + + Read plan file at $PLAN_PATH directly. + Do NOT read $SESSION_DIR/review.md. + Form independent FINDING-{N} list in working context: + - Correctness issues + - Architecture concerns + - Sequencing/dependency problems + - Scope gaps or risks + + INFORMATION BARRIER ends after Round 1 poll completes. + From Round 2 onwards, the barrier no longer applies. +``` + +**workflow.md change** — transform Step 4 "Parse Review" into a Cross-Analysis step. Step 4 currently parses Codex output and applies fixes directly. Add a cross-analysis sub-step before applying fixes: + +``` +4) Cross-Analysis (after first poll completes): + a) Parse all ISSUE-{N} blocks from review.md + b) Build mapping table: FINDING-{N} (Claude) ↔ ISSUE-{N} (Codex) + - Genuine Agreement: FINDING-{N} and ISSUE-{M} identify same problem + - Codex-only: ISSUE-{M} has no matching FINDING + - Claude-only: FINDING-{N} has no matching ISSUE + - Genuine Disagreement: conflicting assessments of same area + c) For each ISSUE-{N}: determine response (fix, rebuttal, or acknowledge) + d) Apply fixes to the plan file. Save the updated plan file before resuming. + (Codex Round 2+ re-reads the plan from $PLAN_PATH — unsaved changes are invisible.) + e) Proceed to resume loop +``` + +No new file needed. + +### `codex-impl-review`: Add Step 2.5 + extend Step 4 + +**workflow.md change** — add after Step 2 (`start`) as new Step 2.5 (mode-conditional): +``` +2.5) INFORMATION BARRIER — Claude Independent Code Analysis + MUST complete before polling Codex output. + Codex is running in background — use this time. + + working-tree mode: run `git diff` and `git diff --cached` yourself + (or reuse the diff already collected in Step 1 — working tree hasn't changed) + branch mode: run `git diff $BASE_BRANCH...HEAD` yourself + (or reuse the diff already collected in Step 1) + + Form independent FINDING-{N} list in working context: + - Bugs and edge cases + - Security issues + - Performance concerns + - Maintainability problems + + Do NOT read $SESSION_DIR/review.md until analysis complete. + INFORMATION BARRIER ends after Round 1 poll completes. +``` + +**workflow.md change** — extend Step 4 "Apply/Rebut" to add cross-analysis sub-step before applying fixes. impl-review retains code-editing behavior — the cross-analysis adds FINDING/ISSUE mapping context to the existing apply/rebuttal decision, but does NOT remove code editing: + +``` +4) Cross-Analysis + Apply/Rebut: + a) Parse all ISSUE-{N} blocks from review.md + b) Build mapping table: FINDING-{N} (Claude) ↔ ISSUE-{N} (Codex) + (same classification as plan-review Step 4a-b above) + c) For each ISSUE-{N}: + - If genuine agreement or Codex-only: apply fix to code + - If Claude-only: include in final report as Claude finding + - If genuine disagreement: rebuttal response + d) Commit fixes (branch mode only — required before resume) + e) Proceed to resume loop +``` + +Code editing is preserved. FINDING-{N} from Step 2.5 enriches the cross-analysis but does not change the apply/rebuttal outcome model. + +--- + +## Section 3: VERDICT Vocabulary Unification + +**Problem**: Three skills (`plan-review`, `impl-review`, `security-review`) use `APPROVE | REVISE` — a final-state verdict that cannot drive a multi-round debate loop. The correct vocabulary for debate-loop skills is `CONSENSUS | CONTINUE | STALEMATE`. + +**Note on pre-existing state**: `security-review/references/output-format.md` and `prompts.md` already contain partial `STALEMATE` vocabulary in secondary sections (stalemate format section). The migration unifies these existing fragments into the primary VERDICT block. + +**Target vocabulary** (from pr-review/commit-review gold standard): + +| Value | Meaning | Workflow action | +|-------|---------|----------------| +| `CONSENSUS` | No remaining issues / all resolved | End debate, generate final report | +| `CONTINUE` | Issues remain, requires another round | Claude fixes/rebuts, then `resume` | +| `STALEMATE` | Circular debate detected by Codex | End debate, produce partial report | + +**Migration mapping**: +- `APPROVE` → `CONSENSUS` +- `REVISE` → `CONTINUE` +- *(pre-existing in stalemate sections)* `STALEMATE` → promote to primary VERDICT block + +### Changes per skill + +**For each of `plan-review`, `impl-review`, `security-review`:** + +**`references/output-format.md`** (clear VERDICT change): +- Update VERDICT block primary values from `APPROVE | REVISE` to `CONSENSUS | CONTINUE | STALEMATE` +- Add STALEMATE description in VERDICT block + +**`references/prompts.md`** (scope clarification): +- For plan-review and impl-review: `{OUTPUT_FORMAT}` placeholder indirection means VERDICT values live in `output-format.md` only; in `prompts.md` update only the instruction line "End with a VERDICT block" to add: "Use STALEMATE if same arguments repeat for 2+ consecutive rounds with no new evidence." +- For security-review: remove inline VERDICT values from nested backtick sections (covered by Bug 2 fix); the `{OUTPUT_FORMAT}` injection handles the values after Bug 2 is fixed. + +**`references/workflow.md`** (all occurrences): +- Update stop condition text: replace `VERDICT: APPROVE` → `VERDICT: CONSENSUS` +- Update debate loop branching: `CONTINUE` triggers resume (same as `REVISE`), `CONSENSUS` triggers final report (same as `APPROVE`), `STALEMATE` triggers stalemate handling +- Update stalemate detection: check for `STALEMATE` verdict OR same open ISSUE set for 2 consecutive rounds (the latter is the existing heuristic) +- Update the Verdict row value in the final report summary table from `{APPROVE/REVISE/STALEMATE}` to `{CONSENSUS/CONTINUE/STALEMATE}` (this is a value cell, not a table header) +- For security-review specifically: update inline Final Security Report template in Phase 5 Step 2 — the `## Final Verdict: {APPROVE | REVISE | STALEMATE}` header → `## Final Verdict: {CONSENSUS | CONTINUE | STALEMATE}`. All APPROVE/REVISE occurrences are enumerated in Bug 5 fix above. + +**`references/output-format.md`** — security-review note: the `Status` field in the Verdict Block (`Status: {complete | stalemate | in-progress}`) is a separate field from `VERDICT` and is retained as-is. `Status: stalemate` maps to `VERDICT: STALEMATE`; `Status: in-progress` maps to `VERDICT: CONTINUE`. No change needed to the `Status` field. + +--- + +## File Change Summary + +| File | Change type | Section | +|------|-------------|---------| +| `codex-security-review/references/workflow.md` | Delete Step 1, add prompt assembly step, add Step 2.5, extend cross-analysis, remove archive step, remove Option A, renumber options, update VERDICT logic + final report template | 1 (bugs 1, 2, 5), 2, 3 | +| `codex-security-review/references/prompts.md` | Remove inline output format (replace with `{OUTPUT_FORMAT}`), update STALEMATE instruction | 1 (bug 2), 3 | +| `codex-security-review/references/output-format.md` | Update VERDICT block to `CONSENSUS\|CONTINUE\|STALEMATE` | 3 | +| `codex-security-review/references/claude-analysis-template.md` | **NEW FILE** | 2 | +| `codex-pr-review/references/workflow.md` | Fix `$SCOPE` → `$BASE_BRANCH` | 1 (bug 3) | +| `codex-parallel-review/references/workflow.md` | Remove `subagent_type` from JSON blocks + prose | 1 (bug 4) | +| `codex-plan-review/references/workflow.md` | Add Step 2.5, transform Step 4 to cross-analysis, update VERDICT logic + stop conditions + final report table | 2, 3 | +| `codex-plan-review/references/prompts.md` | Update STALEMATE instruction line only | 3 | +| `codex-plan-review/references/output-format.md` | Update VERDICT block values | 3 | +| `codex-impl-review/references/workflow.md` | Add Step 2.5, extend Step 4 with cross-analysis sub-steps, update VERDICT logic + stop conditions + final report table | 2, 3 | +| `codex-impl-review/references/prompts.md` | Update STALEMATE instruction line in 4 templates | 3 | +| `codex-impl-review/references/output-format.md` | Update VERDICT block values | 3 | + +*(Note: `codex-security-review/references/output-format.md` appears once — duplicate removed from v1.)* + +--- + +## Constraints + +- **Never edit user's code or create commits during review** — review-only model preserved (security-review Phase 3 Option A removal enforces this) +- **impl-review retains code editing** — cross-analysis enriches the apply/rebuttal decision but does not change the code-editing model +- **No new skills added** — only existing skills improved +- **SKILL.md files** — not changed (these are user-facing; workflow changes are in references/) +- **codex-runner.js** — not changed (runner is separate from skill instructions) +- **`bin/codex-skill.js`** — not changed + +--- + +## Success Criteria + +1. All 5 critical bugs fixed: + - Bug 1: security-review workflow invoked without `AskUserQuestion` error + - Bug 2: prompt sent to Codex contains no prematurely-closed triple-backtick fences (verify by inspecting assembled `$PROMPT` variable) + - Bug 3: pr-review meta.json contains `"scope": ""` not `"scope": "$SCOPE"` + - Bug 4: parallel-review agent spawn JSON contains no `subagent_type` field + - Bug 5: security-review workflow does not write to `docs/` and does not instruct Claude to edit code +2. Information barrier active in security/plan/impl-review — Claude FINDING-{N} analysis recorded in working context before first poll result is read +3. VERDICT parsing unified — `CONSENSUS|CONTINUE|STALEMATE` parsed correctly in all 3 updated skills' workflow debate loops and stop conditions +4. No regressions in the 5 unmodified skills (pr-review, commit-review, think-about, parallel-review, codebase-review) From 28956368bc0af1c01217e59d4ded2d8ee753d91d Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:12:03 +0700 Subject: [PATCH 02/25] docs: add Approach A implementation plan 11-task plan covering 5 critical bug fixes, information barrier adoption for plan/impl/security-review, and VERDICT vocabulary unification. Each task includes exact file targets, verification greps, and commits. Co-Authored-By: Claude Opus 4.6 --- ...026-03-22-approach-a-skill-improvements.md | 894 ++++++++++++++++++ 1 file changed, 894 insertions(+) create mode 100644 docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md diff --git a/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md b/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md new file mode 100644 index 0000000..a02e9f5 --- /dev/null +++ b/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md @@ -0,0 +1,894 @@ +# Approach A — Skill Implementation Improvements + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix 5 critical bugs + add information barriers + unify VERDICT vocabulary across 3 codex-review skills (plan-review, impl-review, security-review). + +**Architecture:** All changes are pure Markdown edits to `references/` files inside `skill-packs/codex-review/skills/`. No code (runner, installer, SKILL.md) is touched. Each task is isolated to one skill directory and can be verified by inspecting the changed file against the spec. + +**Tech Stack:** Markdown files only. No build system, no tests, no npm. Verification is visual inspection + grep for specific strings. + +--- + +## File Map + +| File | Task | +|------|------| +| `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` | T1, T4, T5, T8, T9 | +| `skill-packs/codex-review/skills/codex-security-review/references/prompts.md` | T2, T9 | +| `skill-packs/codex-review/skills/codex-security-review/references/output-format.md` | T9 | +| `skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md` | T5 (NEW FILE) | +| `skill-packs/codex-review/skills/codex-pr-review/references/workflow.md` | T3 | +| `skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md` | T3 | +| `skill-packs/codex-review/skills/codex-plan-review/references/workflow.md` | T6, T10 | +| `skill-packs/codex-review/skills/codex-plan-review/references/prompts.md` | T10 | +| `skill-packs/codex-review/skills/codex-plan-review/references/output-format.md` | T10 | +| `skill-packs/codex-review/skills/codex-impl-review/references/workflow.md` | T7, T11 | +| `skill-packs/codex-review/skills/codex-impl-review/references/prompts.md` | T11 | +| `skill-packs/codex-review/skills/codex-impl-review/references/output-format.md` | T11 | + +--- + +## Task 1: Delete phantom AskUserQuestion step (Bug 1) + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` + +**Context:** The file has a "Smart Default Detection" block near the top that correctly announces `scope=$SCOPE, effort=$EFFORT`. Immediately after that block, Phase 1 Step 1 says `Use AskUserQuestion to collect these inputs in a single prompt.` — this tool does not exist in Claude Code. The fix is simply deleting Step 1. Do NOT add a second announce block. + +- [ ] **Step 1: Read the current file and locate Phase 1 Step 1** + + Open `skill-packs/codex-review/skills/codex-security-review/references/workflow.md`. Find the block that contains `AskUserQuestion`. Confirm the Smart Default Detection block above it already has the announce pattern (`Announce: "Detected: scope=..."`). + +- [ ] **Step 2: Delete Phase 1 Step 1 entirely** + + Remove the entire Step 1 block (from `## Phase 1: ...Step 1` or equivalent heading down to where Step 2 begins). Do not touch the Smart Default Detection block above it. + +- [ ] **Step 3: Verify** + + ```bash + grep -n "AskUserQuestion" skill-packs/codex-review/skills/codex-security-review/references/workflow.md + ``` + Expected: no output (zero matches). + + ```bash + grep -n "Announce" skill-packs/codex-review/skills/codex-security-review/references/workflow.md + ``` + Expected: exactly one match in the Smart Default Detection block. + +- [ ] **Step 4: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-security-review/references/workflow.md + git commit -m "fix(security-review): remove phantom AskUserQuestion from Phase 1 Step 1" + ``` + +--- + +## Task 2: Fix nested triple-backtick in prompts.md — Part A (Bug 2) + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/prompts.md` + +**Context:** The Round 1 prompt template is inside a triple-backtick fence. Inside that fence, there are sections (`## Output Format`, VERDICT Block) that also use triple-backtick fences. The inner fences prematurely close the outer fence, breaking the template. The fix: remove the `## Output Format` section and VERDICT Block from inside the fenced prompt, and replace them with the single line `{OUTPUT_FORMAT}` at the end of the prompt body (before the closing fence). + +Compare with `codex-impl-review/references/prompts.md` — it uses `## Required Output Format\n{OUTPUT_FORMAT}` at the bottom of each prompt template. Use the same pattern. + +- [ ] **Step 1: Read both files to understand the difference** + + Read `skill-packs/codex-review/skills/codex-security-review/references/prompts.md` and identify the inline `## Output Format` / VERDICT Block sections inside the outer fence. + + Read `skill-packs/codex-review/skills/codex-impl-review/references/prompts.md` lines 50-80 to see how `{OUTPUT_FORMAT}` placeholder is used correctly. + +- [ ] **Step 2: Edit prompts.md** + + In the Round 1 prompt template (the outermost fenced block): + - Remove the `## Output Format` section and its content (the nested fence with ISSUE-{N} format) + - Remove the VERDICT Block section (the nested fence with VERDICT template) + - At the end of the prompt body (just before the outer closing fence), add: + + ``` + ## Required Output Format + {OUTPUT_FORMAT} + ``` + + Apply the same change to the Round 2+ / Rebuttal prompt template if it also has inline output format. + +- [ ] **Step 3: Verify no nested triple-backticks remain** + + ```bash + # Count triple-backtick occurrences in each fenced block + grep -c '```' skill-packs/codex-review/skills/codex-security-review/references/prompts.md + ``` + Each outermost fenced block should have exactly 2 triple-backtick lines (open + close). The `{OUTPUT_FORMAT}` placeholder does not add any backticks. + + Also verify the placeholder is present: + ```bash + grep -n "OUTPUT_FORMAT" skill-packs/codex-review/skills/codex-security-review/references/prompts.md + ``` + Expected: at least 1 match per prompt template. + +- [ ] **Step 4: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-security-review/references/prompts.md + git commit -m "fix(security-review): replace inline output format with {OUTPUT_FORMAT} placeholder" + ``` + +--- + +## Task 3: Fix simple one-line bugs (Bug 3 + Bug 4) + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-pr-review/references/workflow.md` +- Modify: `skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md` + +### Bug 3 — pr-review: `$SCOPE` undefined in meta.json + +- [ ] **Step 1: Find the Session Finalization heredoc in pr-review workflow.md** + + ```bash + grep -n 'SCOPE' skill-packs/codex-review/skills/codex-pr-review/references/workflow.md + ``` + Find the line `"scope": "$SCOPE"` in the `meta.json` heredoc. + +- [ ] **Step 2: Replace `$SCOPE` with `$BASE_BRANCH`** + + Change: + ``` + "scope": "$SCOPE", + ``` + To: + ``` + "scope": "$BASE_BRANCH", + ``` + +- [ ] **Step 3: Verify** + + ```bash + grep -n '"scope"' skill-packs/codex-review/skills/codex-pr-review/references/workflow.md + ``` + Expected: shows `"scope": "$BASE_BRANCH"` (not `$SCOPE`). + +### Bug 4 — parallel-review: Invalid `subagent_type` + +- [ ] **Step 4: Find all `subagent_type` occurrences** + + ```bash + grep -n "subagent_type" skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md + ``` + Expected: 5 matches — 4 in JSON blocks (Agent 1-4), 1 in a prose description paragraph. + +- [ ] **Step 5: Remove `subagent_type` from all 4 JSON blocks** + + For each JSON agent spawn block, remove the line: + ```json + "subagent_type": "code-reviewer", + ``` + Do not remove any other fields from the JSON. + +- [ ] **Step 6: Update the prose description paragraph** + + Find the paragraph (line ~53) that reads something like `Each uses subagent_type: "code-reviewer"`. Remove the `subagent_type: "code-reviewer"` reference or replace with: `Each uses the default general-purpose agent behavior.` + +- [ ] **Step 7: Verify** + + ```bash + grep -n "subagent_type" skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md + ``` + Expected: no output (zero matches). + +- [ ] **Step 8: Commit both fixes** + + ```bash + git add skill-packs/codex-review/skills/codex-pr-review/references/workflow.md + git add skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md + git commit -m "fix(pr-review,parallel-review): fix \$SCOPE undefined and invalid subagent_type" + ``` + +--- + +## Task 4: Fix archive step + code editing instruction (Bug 5) + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` + +**Context:** Two problems in the same file: +1. Phase 5 Step 3 copies artifacts to `docs/security-reviews/` — violates v12 session model. +2. Phase 3 Option A says "Apply the suggested fix" — no review skill should edit user code. + +After removing Option A, renumber: old Option B → new Option A, old Option C → new Option B. Remove the lettered "Option" framing and use a flat response list matching `codex-impl-review` workflow.md Step 4 style. + +Also, as part of this task, update all `APPROVE`/`REVISE` strings in this file (enumerated exactly): +- Phase 3 stop condition: `VERDICT: APPROVE` → `VERDICT: CONSENSUS` +- Phase 4 Step 4 "Iterate Until Consensus": `VERDICT: APPROVE - All critical/high issues resolved` → `VERDICT: CONSENSUS - All critical/high issues resolved` +- Phase 5 Step 2 Final Verdict header: `{APPROVE | REVISE | STALEMATE}` → `{CONSENSUS | CONTINUE | STALEMATE}` +- CI/CD pre-commit hook example: `grep -q "VERDICT: REVISE"` → `grep -q "VERDICT: CONTINUE"` + +(The workflow.md VERDICT updates for Section 3 are bundled here since they're all in the same file.) + +- [ ] **Step 1: Read the current Phase 3 section** + + Read `skill-packs/codex-review/skills/codex-security-review/references/workflow.md`. Find Phase 3 with its 3 options. Also find Phase 5 Step 3 (the archive block). + +- [ ] **Step 2: Remove Phase 5 Step 3 archive block** + + Delete the entire Step 3 block in Phase 5 that runs `mkdir -p docs/security-reviews/` and copies artifacts. + +- [ ] **Step 3: Remove Phase 3 Option A and renumber** + + - Delete the entire Option A block (the "Apply the suggested fix" / code editing block). + - Rename `Option B` → `Option A` (or remove "Option" labels entirely and use a flat list like impl-review). + - Rename `Option C` → `Option B` (or same). + - Match the flat style from `codex-impl-review` workflow.md Step 4: + - Response type 1: rebuttal (write concrete proof that finding is wrong) + - Response type 2: acknowledge with severity dispute (finding is valid but severity is lower) + +- [ ] **Step 4: Update all APPROVE/REVISE in this file** + + Make these 4 exact replacements: + + | Old text | New text | + |----------|----------| + | `VERDICT: APPROVE` (Phase 3 stop condition) | `VERDICT: CONSENSUS` | + | `VERDICT: APPROVE - All critical/high issues resolved` | `VERDICT: CONSENSUS - All critical/high issues resolved` | + | `{APPROVE \| REVISE \| STALEMATE}` (Final Verdict header) | `{CONSENSUS \| CONTINUE \| STALEMATE}` | + | `grep -q "VERDICT: REVISE"` (CI/CD example) | `grep -q "VERDICT: CONTINUE"` | + +- [ ] **Step 5: Verify** + + ```bash + grep -n "AskUserQuestion\|docs/security-reviews\|Option A\|Apply the suggested fix\|VERDICT: APPROVE\|VERDICT: REVISE" \ + skill-packs/codex-review/skills/codex-security-review/references/workflow.md + ``` + Expected: zero matches (all removed/replaced). + + ```bash + grep -n "VERDICT: CONSENSUS\|VERDICT: CONTINUE\|VERDICT: STALEMATE" \ + skill-packs/codex-review/skills/codex-security-review/references/workflow.md + ``` + Expected: at least 4 matches (one per replacement location). + +- [ ] **Step 6: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-security-review/references/workflow.md + git commit -m "fix(security-review): remove archive step, code-edit instruction, unify VERDICT vocab in workflow" + ``` + +--- + +## Task 5: Add prompt assembly step to security-review workflow (Bug 2 — Part B) + new claude-analysis-template.md + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` +- Create: `skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md` + +### Part A — Prompt assembly step in workflow.md + +**Context:** After Bug 2 Part A (Task 2), `prompts.md` now has `{OUTPUT_FORMAT}` placeholder. But security-review has no prompt assembly step, so `{OUTPUT_FORMAT}` would never be substituted. Add a Step 1.8 modeled exactly on `codex-impl-review/references/workflow.md` Step 1.8. + +Read `codex-impl-review/references/workflow.md` to find the exact prompt assembly step. It reads `references/prompts.md` as template, replaces each placeholder in sequence using `printf '%s'` (never a single sed pipeline), stores as `$PROMPT`, and verifies no broken backtick fences. + +- [ ] **Step 1: Read codex-impl-review Step 1.8 as the reference pattern** + + Read `skill-packs/codex-review/skills/codex-impl-review/references/workflow.md` and find the prompt assembly step (Step 1.8). Note the exact shell pattern used. + +- [ ] **Step 2: Add Step 1.8 to security-review workflow.md** + + After the existing Step 1 (input collection/detection) and before the `start` command, insert a new step: + + ``` + ## 1.8) Assemble Prompt + + Build `$PROMPT` using multi-step placeholder replacement. + DO NOT use a single sed pipeline — `output-format.md` may contain `&`, `\`, `/` characters + that corrupt sed replacements. Use `printf '%s'` piping. + + a) Read `references/prompts.md` as template base + b) Replace `{WORKING_DIR}` with current working directory + c) Replace `{SCOPE}` with detected `$SCOPE` value + d) Replace `{EFFORT}` with detected `$EFFORT` value + e) Replace `{SCOPE_SPECIFIC_INSTRUCTIONS}` with the scope-specific block from prompts.md + matching the detected `$SCOPE` (working-tree / branch / full) + f) Replace `{OUTPUT_FORMAT}` by reading `references/output-format.md` in full + using: `printf '%s' "$(cat references/output-format.md)"` + g) Replace any remaining placeholders (`{BASE_BRANCH}` etc.) + + Store result as `$PROMPT`. + + Verify: `$PROMPT` must contain no prematurely-closed triple-backtick fences. + ``` + +- [ ] **Step 3: Verify prompt assembly step exists** + + ```bash + grep -n "Assemble Prompt\|OUTPUT_FORMAT\|printf '%s'" \ + skill-packs/codex-review/skills/codex-security-review/references/workflow.md + ``` + Expected: matches showing the new assembly step with `OUTPUT_FORMAT` and `printf '%s'`. + +### Part B — New claude-analysis-template.md + +**Context:** The information barrier pattern requires Claude to analyze independently before polling. `codex-pr-review/references/claude-analysis-template.md` and `codex-commit-review/references/claude-analysis-template.md` are the gold standard. Security review needs its own version with OWASP/CWE fields instead of PR-specific fields. + +- [ ] **Step 4: Read the gold standard template for reference** + + Read `skill-packs/codex-review/skills/codex-pr-review/references/claude-analysis-template.md`. + +- [ ] **Step 5: Create security-review claude-analysis-template.md** + + Create `skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md` with this content: + + ```markdown + # Claude Independent Security Analysis Template + + > Use this template for Step 2.5 (Information Barrier). + > Record analysis in working context ONLY — do NOT write to a file. + > Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + + ## FINDING-{N} Format + + Use this exact shape for each independent finding: + + ### FINDING-{N}: {Short title} + - Category: injection | broken-auth | sensitive-data | broken-access | security-config | xss | insecure-deserialization | logging | ssrf | crypto-failure | insecure-design | vulnerable-components | integrity-failure | secrets + - Severity: low | medium | high | critical + - Confidence: low | medium | high + - CWE: {CWE-NNN if known, otherwise omit} + - OWASP: {A0N:2021 category if applicable, otherwise omit} + - Location: {file path:line range} + - Problem: {clear statement of the vulnerability or security weakness} + - Evidence: {specific code pattern, snippet, or observation} + - Attack Vector: {how an attacker could exploit this} + - Why it matters: {impact — data exposure, privilege escalation, etc.} + + ## Overall Security Assessment + + - Attack surface: {high / medium / low — based on exposed endpoints, data handled} + - Most critical area: {the single highest-risk component or pattern found} + - Confidence in analysis: {high / medium / low — limited by static analysis} + + ## Strongest Positions + + List the 3-5 findings Claude is most confident about for cross-analysis debate: + + 1. FINDING-{N}: {title} — {one-sentence rationale for high confidence} + 2. ... + + ## Cross-Analysis Matching Protocol + + After Round 1 poll completes, build a mapping table: + + | Claude FINDING-{N} | Codex ISSUE-{M} | Classification | + |--------------------|-----------------|----------------| + | FINDING-1 | ISSUE-2 | Genuine Agreement | + | FINDING-2 | — | Claude-only | + | — | ISSUE-4 | Codex-only | + | FINDING-3 | ISSUE-5 | Genuine Disagreement | + + Classification rules: + - **Genuine Agreement**: Same vulnerability class + same file/line area + - **Genuine Disagreement**: Same code area but conflicting assessment (one says vulnerable, other says safe) + - **Same Direction / Different Severity**: Both flag the same issue but assign different severity + - **Claude-only**: Claude's finding has no Codex counterpart + - **Codex-only**: Codex's finding has no Claude counterpart + + Maintain this table across all rounds. Update Classification and Status columns as rounds progress. + ``` + +- [ ] **Step 6: Verify the new file exists** + + ```bash + ls skill-packs/codex-review/skills/codex-security-review/references/ + ``` + Expected: `claude-analysis-template.md` appears in the listing. + +- [ ] **Step 7: Commit both changes** + + ```bash + git add skill-packs/codex-review/skills/codex-security-review/references/workflow.md + git add skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md + git commit -m "feat(security-review): add prompt assembly step and claude-analysis-template.md" + ``` + +--- + +## Task 6: Add information barrier to codex-plan-review + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-plan-review/references/workflow.md` + +**Context:** The current workflow goes: Step 2 (start Codex) → Step 3 (poll). We need to insert Step 2.5 between them and transform Step 4 "Parse Review" into a Cross-Analysis step. + +Read `codex-pr-review/references/workflow.md` Steps 2.5 and 4 as the gold standard for the pattern. + +- [ ] **Step 1: Read current plan-review workflow.md** + + Read `skill-packs/codex-review/skills/codex-plan-review/references/workflow.md`. Identify: + - Where Step 2 (start command) ends + - Where Step 3 (Poll) begins — this is where Step 2.5 goes + - The content of Step 4 "Parse Review" / Apply fixes — this will be transformed + +- [ ] **Step 2: Insert Step 2.5 after Step 2** + + After the `node "$RUNNER" start` command block in Step 2, add: + + ```markdown + ## 2.5) Information Barrier — Claude Independent Plan Analysis + + MUST complete before polling Codex output. + Codex is running in background — use this time productively. + + Read the plan file at `$PLAN_PATH` directly. + Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + + Form an independent FINDING-{N} list in working context (do NOT write to a file): + - Correctness issues (steps that are wrong or will fail) + - Architecture concerns (structural problems with the approach) + - Sequencing/dependency problems (steps out of order, missing prerequisites) + - Scope gaps or risks (missing requirements, underestimated complexity) + + Use the same FINDING-{N} format as `output-format.md` ISSUE-{N} (same field names). + + INFORMATION BARRIER ends after Round 1 poll completes. + From Round 2 onwards, the barrier no longer applies. + ``` + +- [ ] **Step 3: Transform Step 4 into Cross-Analysis** + + Replace the current Step 4 "Parse Review" / fix application with: + + ```markdown + ## 4) Cross-Analysis + + After Round 1 poll completes and `$SESSION_DIR/review.md` is available: + + ### 4a) Parse Codex Output + Read all `ISSUE-{N}` blocks from `$SESSION_DIR/review.md`. + + ### 4b) Build FINDING↔ISSUE Mapping Table + Map Claude's FINDING-{N} (from Step 2.5) against Codex's ISSUE-{N}: + + | Claude FINDING-{N} | Codex ISSUE-{M} | Classification | + |--------------------|-----------------|----------------| + | ... | ... | ... | + + Classification options: + - **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same plan problem + - **Codex-only**: ISSUE-{M} has no matching Claude FINDING + - **Claude-only**: FINDING-{N} has no matching Codex ISSUE + - **Genuine Disagreement**: Conflicting assessments of the same plan section + + ### 4c) Determine Response for Each ISSUE + For each ISSUE-{N}: + - Genuine agreement or Codex-only → apply fix to the plan file + - Claude-only → include in final report as Claude finding + - Genuine disagreement → write rebuttal with concrete reasoning + + ### 4d) Apply Fixes + Apply accepted fixes to the plan file at `$PLAN_PATH`. + Save the updated plan file before resuming. + **Critical**: Codex Round 2+ re-reads the plan from `$PLAN_PATH` — unsaved changes are invisible to Codex. + + ### 4e) Record Round Summary + Append to `$SESSION_DIR/rounds.json`: + ```json + { "round": N, "elapsed_seconds": ..., "verdict": "...", "issues_found": ..., "issues_fixed": ..., "issues_disputed": ... } + ``` + + Proceed to Step 5 (resume) or Step 7 (final output) based on VERDICT. + ``` + +- [ ] **Step 4: Verify Step 2.5 and cross-analysis are present** + + ```bash + grep -n "Information Barrier\|FINDING-{N}\|Cross-Analysis\|4b\|PLAN_PATH.*unsaved" \ + skill-packs/codex-review/skills/codex-plan-review/references/workflow.md + ``` + Expected: matches for each of these patterns. + +- [ ] **Step 5: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-plan-review/references/workflow.md + git commit -m "feat(plan-review): add information barrier Step 2.5 and cross-analysis Step 4" + ``` + +--- + +## Task 7: Add information barrier to codex-impl-review + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-impl-review/references/workflow.md` + +**Context:** Same structure as plan-review. Insert Step 2.5 between Step 2 (start) and Step 3 (poll). Extend Step 4 "Apply/Rebut" with cross-analysis sub-steps. Code editing is PRESERVED — cross-analysis only enriches the apply/rebuttal decision, it does not remove code editing. + +- [ ] **Step 1: Read current impl-review workflow.md** + + Read `skill-packs/codex-review/skills/codex-impl-review/references/workflow.md`. Identify: + - Where Step 2 (start command) ends — note this is mode-conditional (working-tree vs branch) + - Where Step 3 (Poll) begins + - The content of Step 4 "Apply/Rebut" — this will be extended, not replaced + +- [ ] **Step 2: Insert Step 2.5 after Step 2** + + After the `node "$RUNNER" start` command block, add: + + ```markdown + ## 2.5) Information Barrier — Claude Independent Code Analysis + + MUST complete before polling Codex output. + Codex is running in background — use this time productively. + + **working-tree mode**: run `git diff` and `git diff --cached` yourself + (or reuse the diff already collected in Step 1 — working tree hasn't changed) + **branch mode**: run `git diff $BASE_BRANCH...HEAD` yourself + (or reuse the diff already collected in Step 1) + + Form an independent FINDING-{N} list in working context (do NOT write to a file): + - Bugs and edge cases + - Security issues + - Performance concerns + - Maintainability problems + + Use the same FINDING-{N} format as `output-format.md` ISSUE-{N} (same field names). + Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + + INFORMATION BARRIER ends after Round 1 poll completes. + ``` + +- [ ] **Step 3: Extend Step 4 with cross-analysis sub-steps** + + Before the existing "For valid issues: edit code and record fix evidence" line in Step 4, add: + + ```markdown + ### 4a) Parse Codex Output + Read all `ISSUE-{N}` blocks from `$SESSION_DIR/review.md`. + + ### 4b) Build FINDING↔ISSUE Mapping Table + Map Claude's FINDING-{N} (from Step 2.5) against Codex's ISSUE-{N}: + + | Claude FINDING-{N} | Codex ISSUE-{M} | Classification | + |--------------------|-----------------|----------------| + | ... | ... | ... | + + Classification options: + - **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same code problem + - **Codex-only**: ISSUE-{M} has no matching Claude FINDING + - **Claude-only**: FINDING-{N} has no matching Codex ISSUE + - **Genuine Disagreement**: Conflicting assessments of the same code + + ### 4c) Apply/Rebut using cross-analysis context + For each ISSUE-{N}: + - If genuine agreement or Codex-only → apply fix to code + - If Claude-only → include in final report as Claude finding + - If genuine disagreement → write rebuttal with concrete proof (paths, tests, behavior) + ``` + + Keep all existing content below (the branch mode commit requirement, verification requirement, rounds.json update, etc.) — do NOT remove them. + +- [ ] **Step 4: Verify Step 2.5 and cross-analysis are present, code editing is preserved** + + ```bash + grep -n "Information Barrier\|FINDING-{N}\|Cross-Analysis\|4b\|edit code\|commit.*fixes\|branch mode only" \ + skill-packs/codex-review/skills/codex-impl-review/references/workflow.md + ``` + Expected: matches for both the new cross-analysis content AND the existing "edit code" / "branch mode only" / commit lines. + +- [ ] **Step 5: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-impl-review/references/workflow.md + git commit -m "feat(impl-review): add information barrier Step 2.5 and cross-analysis in Step 4" + ``` + +--- + +## Task 8: Add information barrier Step 2.5 to security-review workflow + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/workflow.md` + +**Context:** Same information barrier pattern as plan-review and impl-review. Insert Step 2.5 after Phase 2 `start` command. Also update Phase 3 cross-analysis step to use FINDING↔ISSUE mapping (matching pr-review gold standard). + +- [ ] **Step 1: Identify insertion point in security-review workflow.md** + + Read `skill-packs/codex-review/skills/codex-security-review/references/workflow.md`. Find Phase 2 start command and Phase 2 polling step. Step 2.5 goes between them. + +- [ ] **Step 2: Insert Step 2.5 in Phase 2** + + After the `node "$RUNNER" start` command in Phase 2, add: + + ```markdown + ### Phase 2, Step 2.5: Information Barrier — Claude Independent Security Analysis + + MUST complete before polling Codex output. + Codex is running in background (typically 90-180s) — use this time. + + Using `references/claude-analysis-template.md`: + - Read all files in scope directly (do NOT read `$SESSION_DIR/review.md`) + - Identify top attack surfaces + - Form an independent FINDING-{N} list using OWASP categories + - Note high-confidence vs uncertain findings + + Keep analysis in working context. Do NOT write a file. + INFORMATION BARRIER ends after Round 1 poll completes. + From Round 2 onwards, the barrier no longer applies. + ``` + +- [ ] **Step 3: Update Phase 3 cross-analysis step** + + Find the Phase 3 step where Claude processes Codex findings (the step immediately after polling). Add FINDING↔ISSUE mapping to the beginning of that step, using the same classification table format as in Tasks 6 and 7 (Genuine Agreement / Genuine Disagreement / Claude-only / Codex-only / Same Direction Different Severity). + +- [ ] **Step 4: Verify** + + ```bash + grep -n "Information Barrier\|claude-analysis-template\|FINDING-{N}\|working context" \ + skill-packs/codex-review/skills/codex-security-review/references/workflow.md + ``` + Expected: matches for each pattern. + +- [ ] **Step 5: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-security-review/references/workflow.md + git commit -m "feat(security-review): add information barrier Step 2.5 and FINDING/ISSUE cross-analysis" + ``` + +--- + +## Task 9: Unify VERDICT vocabulary in security-review output-format.md and prompts.md + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/output-format.md` +- Modify: `skill-packs/codex-review/skills/codex-security-review/references/prompts.md` + +**Context:** The primary VERDICT block in `output-format.md` currently shows `Status: APPROVE | REVISE`. The `prompts.md` may have similar inline VERDICT values. Both need to use `CONSENSUS | CONTINUE | STALEMATE`. Note: the `Status` field (separate from `VERDICT`) with values like `{complete | stalemate | in-progress}` is kept as-is. + +- [ ] **Step 1: Update output-format.md VERDICT block** + + Read `skill-packs/codex-review/skills/codex-security-review/references/output-format.md`. Find the primary VERDICT block (the one used in normal reviews, not the stalemate format). Change: + + ``` + - Status: APPROVE | REVISE + ``` + To: + ``` + - Status: CONSENSUS | CONTINUE | STALEMATE + ``` + + Add descriptions for each value: + - `CONSENSUS`: All critical/high security issues resolved or agreed-upon — ready to proceed + - `CONTINUE`: Security issues remain that require another review round + - `STALEMATE`: Circular debate — same disputes for 2+ rounds with no progress + +- [ ] **Step 2: Update prompts.md VERDICT references** + + In `skill-packs/codex-review/skills/codex-security-review/references/prompts.md`, find any instruction lines that reference VERDICT values (e.g., "End with a VERDICT block using APPROVE or REVISE"). Update to reference `CONSENSUS | CONTINUE | STALEMATE` and add: "Use STALEMATE if same arguments repeat for 2+ consecutive rounds with no new evidence." + + The inline VERDICT format sections were already removed in Task 2 (they are now handled by `{OUTPUT_FORMAT}` injection). Only update the instruction text lines. + +- [ ] **Step 3: Verify** + + ```bash + grep -n "APPROVE\|REVISE" \ + skill-packs/codex-review/skills/codex-security-review/references/output-format.md \ + skill-packs/codex-review/skills/codex-security-review/references/prompts.md + ``` + Expected: zero matches for these old vocabulary values in both files. (The stalemate format section already uses `STALEMATE` which is fine.) + + ```bash + grep -n "CONSENSUS\|CONTINUE\|STALEMATE" \ + skill-packs/codex-review/skills/codex-security-review/references/output-format.md \ + skill-packs/codex-review/skills/codex-security-review/references/prompts.md + ``` + Expected: at least one match each in both files. + +- [ ] **Step 4: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-security-review/references/output-format.md + git add skill-packs/codex-review/skills/codex-security-review/references/prompts.md + git commit -m "fix(security-review): unify VERDICT vocabulary to CONSENSUS/CONTINUE/STALEMATE" + ``` + +--- + +## Task 10: Unify VERDICT vocabulary in codex-plan-review + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-plan-review/references/output-format.md` +- Modify: `skill-packs/codex-review/skills/codex-plan-review/references/prompts.md` +- Modify: `skill-packs/codex-review/skills/codex-plan-review/references/workflow.md` + +**Context:** plan-review currently uses `APPROVE | REVISE`. Three files need updates. In `prompts.md`, the VERDICT values are injected via `{OUTPUT_FORMAT}` placeholder (not inline), so only the instruction line "End with a VERDICT block" needs updating. In `workflow.md`, update stop conditions and the final report summary table's Verdict row value. + +- [ ] **Step 1: Update output-format.md VERDICT block** + + Read `skill-packs/codex-review/skills/codex-plan-review/references/output-format.md`. Find: + ``` + - Status: APPROVE | REVISE + ``` + Change to: + ``` + - Status: CONSENSUS | CONTINUE | STALEMATE + ``` + + Add descriptions: + - `CONSENSUS`: No remaining plan issues — ready to implement + - `CONTINUE`: Issues remain that require another review round + - `STALEMATE`: Circular debate — same disputes for 2+ rounds with no progress + +- [ ] **Step 2: Update prompts.md instruction line** + + In `skill-packs/codex-review/skills/codex-plan-review/references/prompts.md`, find the line "End with a VERDICT block" (or similar wording) in each prompt template. Add: "Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence." + + Do NOT change the `{OUTPUT_FORMAT}` placeholder — it already injects the updated values from `output-format.md`. + +- [ ] **Step 3: Update workflow.md stop condition and final report table** + + In `skill-packs/codex-review/skills/codex-plan-review/references/workflow.md`: + + a) Find the stop condition text `VERDICT: APPROVE`. Change to `VERDICT: CONSENSUS`. + + b) Find the final report summary table row for Verdict. Change the value from `{APPROVE/REVISE/STALEMATE}` to `{CONSENSUS/CONTINUE/STALEMATE}`. (This is a value cell, not a table header.) + + c) Update the debate loop branching section: + - `CONSENSUS` → trigger final report (same behavior as old `APPROVE`) + - `CONTINUE` → trigger fix/rebuttal and resume (same behavior as old `REVISE`) + - `STALEMATE` → trigger stalemate handling (check for `STALEMATE` verdict OR same open ISSUE set for 2 consecutive rounds) + +- [ ] **Step 4: Verify** + + ```bash + grep -n "APPROVE\|REVISE" \ + skill-packs/codex-review/skills/codex-plan-review/references/output-format.md \ + skill-packs/codex-review/skills/codex-plan-review/references/prompts.md \ + skill-packs/codex-review/skills/codex-plan-review/references/workflow.md + ``` + Expected: zero matches for old vocabulary in all 3 files. + + ```bash + grep -n "CONSENSUS\|CONTINUE\|STALEMATE" \ + skill-packs/codex-review/skills/codex-plan-review/references/output-format.md \ + skill-packs/codex-review/skills/codex-plan-review/references/workflow.md + ``` + Expected: at least 2 matches each. + +- [ ] **Step 5: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-plan-review/references/output-format.md + git add skill-packs/codex-review/skills/codex-plan-review/references/prompts.md + git add skill-packs/codex-review/skills/codex-plan-review/references/workflow.md + git commit -m "fix(plan-review): unify VERDICT vocabulary to CONSENSUS/CONTINUE/STALEMATE" + ``` + +--- + +## Task 11: Unify VERDICT vocabulary in codex-impl-review + +**Files:** +- Modify: `skill-packs/codex-review/skills/codex-impl-review/references/output-format.md` +- Modify: `skill-packs/codex-review/skills/codex-impl-review/references/prompts.md` +- Modify: `skill-packs/codex-review/skills/codex-impl-review/references/workflow.md` + +**Context:** Same as Task 10 but for impl-review. The `prompts.md` has 4 templates (Working Tree Round 1, Branch Round 1, Rebuttal Working-tree, Rebuttal Branch) — each has an instruction line referencing VERDICT that needs updating. VERDICT values are in `output-format.md` via `{OUTPUT_FORMAT}` placeholder. + +- [ ] **Step 1: Update output-format.md VERDICT block** + + Read `skill-packs/codex-review/skills/codex-impl-review/references/output-format.md`. Find: + ``` + - Status: APPROVE | REVISE + ``` + Change to: + ``` + - Status: CONSENSUS | CONTINUE | STALEMATE + ``` + + Add descriptions: + - `CONSENSUS`: No remaining code issues — changes are correct and safe + - `CONTINUE`: Issues remain that require fixes and another review round + - `STALEMATE`: Circular debate — same disputes for 2+ rounds with no progress + +- [ ] **Step 2: Update prompts.md — all 4 templates** + + In `skill-packs/codex-review/skills/codex-impl-review/references/prompts.md`, for each of the 4 prompt templates, find the line "End with a VERDICT block." Add to it: "Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence." + +- [ ] **Step 3: Update workflow.md stop conditions and final report** + + Same changes as Task 10 Step 3 but in `codex-impl-review/references/workflow.md`: + + a) Change `VERDICT: APPROVE` stop condition → `VERDICT: CONSENSUS` + + b) Change Verdict row value in final report summary table: `{APPROVE/REVISE/STALEMATE}` → `{CONSENSUS/CONTINUE/STALEMATE}` + + c) Update debate loop branching for CONSENSUS / CONTINUE / STALEMATE (same logic as plan-review). + +- [ ] **Step 4: Verify** + + ```bash + grep -n "APPROVE\|REVISE" \ + skill-packs/codex-review/skills/codex-impl-review/references/output-format.md \ + skill-packs/codex-review/skills/codex-impl-review/references/prompts.md \ + skill-packs/codex-review/skills/codex-impl-review/references/workflow.md + ``` + Expected: zero matches for old vocabulary in all 3 files. + +- [ ] **Step 5: Commit** + + ```bash + git add skill-packs/codex-review/skills/codex-impl-review/references/output-format.md + git add skill-packs/codex-review/skills/codex-impl-review/references/prompts.md + git add skill-packs/codex-review/skills/codex-impl-review/references/workflow.md + git commit -m "fix(impl-review): unify VERDICT vocabulary to CONSENSUS/CONTINUE/STALEMATE" + ``` + +--- + +## Task 12: Final verification pass + +**Files:** None modified. + +- [ ] **Step 1: Verify all 5 bugs are fixed** + + ```bash + # Bug 1: No AskUserQuestion in security-review + grep -rn "AskUserQuestion" skill-packs/codex-review/skills/ + + # Bug 2: No nested backtick fences + OUTPUT_FORMAT present in security prompts.md + grep -n "OUTPUT_FORMAT" skill-packs/codex-review/skills/codex-security-review/references/prompts.md + + # Bug 3: pr-review has $BASE_BRANCH not $SCOPE + grep -n '"scope"' skill-packs/codex-review/skills/codex-pr-review/references/workflow.md + + # Bug 4: No subagent_type in parallel-review + grep -rn "subagent_type" skill-packs/codex-review/skills/codex-parallel-review/ + + # Bug 5: No docs/security-reviews in security-review, no "Apply the suggested fix" + grep -rn "docs/security-reviews\|Apply the suggested fix" skill-packs/codex-review/skills/codex-security-review/ + ``` + + Expected: all searches return zero matches. + +- [ ] **Step 2: Verify information barriers exist in 3 skills** + + ```bash + grep -rn "Information Barrier" skill-packs/codex-review/skills/ + ``` + Expected: matches in `codex-security-review`, `codex-plan-review`, `codex-impl-review` — NOT in `codex-pr-review` (already had it), commit-review (already had it). + +- [ ] **Step 3: Verify VERDICT vocabulary is unified** + + ```bash + # Old vocabulary should NOT appear in the 3 updated skills + grep -rn "VERDICT.*APPROVE\|VERDICT.*REVISE\|Status: APPROVE\|Status: REVISE" \ + skill-packs/codex-review/skills/codex-plan-review/ \ + skill-packs/codex-review/skills/codex-impl-review/ \ + skill-packs/codex-review/skills/codex-security-review/ + + # New vocabulary SHOULD appear + grep -rn "CONSENSUS\|CONTINUE\|STALEMATE" \ + skill-packs/codex-review/skills/codex-plan-review/ \ + skill-packs/codex-review/skills/codex-impl-review/ \ + skill-packs/codex-review/skills/codex-security-review/ + ``` + + Expected: first grep has zero matches; second grep has multiple matches. + +- [ ] **Step 4: Verify unmodified skills are untouched** + + ```bash + git diff HEAD skill-packs/codex-review/skills/codex-commit-review/ \ + skill-packs/codex-review/skills/codex-think-about/ \ + skill-packs/codex-review/skills/codex-codebase-review/ + ``` + Expected: no changes (these 3 skills should not be touched). + +- [ ] **Step 5: Final commit (if any stray uncommitted changes)** + + ```bash + git status + ``` + Expected: clean working tree (all changes committed in previous tasks). From 740224efca43c3703fb750e4f006282780580b8a Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:27:43 +0700 Subject: [PATCH 03/25] chore: ignore .worktrees/ directory --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1f34284..80bb684 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ node_modules/ # macOS .DS_Store .gitnexus +.docs//.worktrees/ From dd03b4518026768ee209e2d5fba52037f59204e6 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:30:53 +0700 Subject: [PATCH 04/25] fix(security-review): remove phantom AskUserQuestion from Phase 1 Step 1 The Smart Default Detection block already handles scope/effort detection and announces the result. Phase 1 Step 1 duplicated this with a call to AskUserQuestion which does not exist in Claude Code. Remove the entire step and renumber remaining Phase 1 steps. Co-Authored-By: Claude Opus 4.6 --- .../codex-security-review/references/workflow.md | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 110c460..4e0f803 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -45,23 +45,14 @@ This document describes the execution workflow for security-focused code review ## Phase 1: Setup and Initialization -### Step 1: Gather User Input - -Ask the user for: -1. **Effort level**: `low`, `medium`, `high`, or `xhigh` (default: `high`) -2. **Review scope**: `working-tree`, `branch`, or `full` (default: `working-tree`) -3. **Base branch** (if scope is `branch`): Discover and validate base branch - -Use `AskUserQuestion` to collect these inputs in a single prompt. - -### Step 2: Validate Prerequisites +### Step 1: Validate Prerequisites - Verify inside a git repository: `git rev-parse --show-toplevel`. If not a git repo, abort (unless scope=full on non-git project). - **Working-tree mode**: verify changes exist: `git diff --quiet && git diff --cached --quiet` must FAIL. - **Branch mode**: verify base branch exists: `git rev-parse --verify `. Verify diff exists: `git diff ...HEAD --quiet` must FAIL. - **Full mode**: no additional git checks needed (scans entire codebase). -### Step 3: Build Security Review Prompt +### Step 2: Build Security Review Prompt Select appropriate prompt template from `references/prompts.md`: - **Working-tree mode**: Focus on uncommitted changes From 1351ab4d8cfd1c1a12d963ec9ac56c9426aaec17 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:37:48 +0700 Subject: [PATCH 05/25] fix(security-review): replace inline output format with {OUTPUT_FORMAT} placeholder Co-Authored-By: Claude Opus 4.6 --- .../references/prompts.md | 79 ++----------------- 1 file changed, 6 insertions(+), 73 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md index 82973a7..d00a640 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md @@ -101,58 +101,6 @@ Perform a comprehensive security analysis focusing on OWASP Top 10 2021 vulnerab - [ ] Insecure file uploads - [ ] Open redirects -## Output Format - -For each security finding, use this structure: - -``` -ISSUE-{N}: {vulnerability_title} -Category: injection | broken-auth | sensitive-data | xxe | broken-access | security-config | xss | insecure-deserialization | logging | ssrf | crypto-failure | insecure-design | vulnerable-components | integrity-failure -Severity: critical | high | medium | low -Confidence: high | medium | low -CWE: CWE-{ID} ({Name}) -OWASP: A{NN}:2021 - {Category Name} - -Problem: [Clear description of the vulnerability] - -Evidence: [Code snippet showing the vulnerable pattern] - -Attack Vector: [How an attacker could exploit this vulnerability] - -Suggested Fix: [Secure code example with explanation] -``` - -### Severity Guidelines -- **critical**: Remote code execution, authentication bypass, data breach -- **high**: Privilege escalation, SQL injection, XSS with session theft -- **medium**: Information disclosure, CSRF, weak crypto -- **low**: Security headers missing, verbose errors, minor misconfigurations - -### Confidence Guidelines -- **high**: Clear vulnerability pattern, well-known exploit -- **medium**: Potential vulnerability, depends on context -- **low**: Suspicious pattern, may be false positive - -## Verdict Block - -After listing all findings: - -``` -VERDICT: APPROVE | REVISE -Status: {status} -Reason: {explanation} - -Security Risk Summary: -- Critical: {count} -- High: {count} -- Medium: {count} -- Low: {count} - -Recommendations: -1. [Priority action items] -2. [Additional security measures] -``` - ## Important Notes 1. **Static Analysis Limitations**: You can only analyze code patterns. Mark findings with appropriate confidence levels. @@ -164,6 +112,9 @@ Recommendations: ## Review Scope {SCOPE_SPECIFIC_INSTRUCTIONS} + +## Required Output Format +{OUTPUT_FORMAT} ``` --- @@ -243,31 +194,13 @@ You are continuing a security review debate. 3. Identify any new security concerns introduced by fixes 4. Update your verdict -## Response Format - -For each previously disputed issue: -``` -RESPONSE-{N}: Re: ISSUE-{N} -Action: accept | reject | revise -Reason: [Your response to the rebuttal] -``` - -For new issues found in fixes: -``` -ISSUE-{N}: [New issue title] -[Standard issue format] -``` - -Updated verdict: -``` -VERDICT: APPROVE | REVISE -Reason: [Updated assessment] -``` - ## Stop Conditions - All critical and high severity issues are resolved - Remaining disputes are documented and acknowledged - No new security concerns in applied fixes + +## Required Output Format +{OUTPUT_FORMAT} ``` --- From 9a148041c090fb9b3a53f82f23372c9de5e52463 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:42:20 +0700 Subject: [PATCH 06/25] fix(pr-review,parallel-review): fix $SCOPE undefined and invalid subagent_type Bug 3: pr-review meta.json heredoc used $SCOPE which was never defined, causing literal "$SCOPE" in output. Changed to $BASE_BRANCH which holds the actual base branch value. Bug 4: parallel-review agent spawn blocks included "subagent_type": "code-reviewer" which is not a valid parameter for the Claude Code Agent tool. Removed from all 4 agent JSON blocks and updated the prose description. Co-Authored-By: Claude Opus 4.6 --- .../skills/codex-parallel-review/references/workflow.md | 6 +----- .../skills/codex-pr-review/references/workflow.md | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md b/skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md index 66ec713..3c35fac 100644 --- a/skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md @@ -50,7 +50,7 @@ START_OUTPUT=$(printf '%s' "$CODEX_PROMPT" | node "$RUNNER" start "$SESSION_DIR" ### 2b) Spawn 4 Claude Reviewer Agents -Use Claude Code's **native Agent tool** (built-in, no plugins needed) to spawn 4 parallel reviewers. Each uses `subagent_type: "code-reviewer"` with `run_in_background: true`. +Use Claude Code's **native Agent tool** (built-in, no plugins needed) to spawn 4 parallel reviewers. Each uses the default general-purpose agent behavior with `run_in_background: true`. **All 4 agents MUST be spawned in the same message as the Codex start command.** @@ -58,7 +58,6 @@ Use Claude Code's **native Agent tool** (built-in, no plugins needed) to spawn 4 ```json { - "subagent_type": "code-reviewer", "description": "Review correctness and edge cases", "run_in_background": true, "prompt": "You are an independent code reviewer. Another AI (Codex) is reviewing the same code separately — you will NOT see their findings. Be thorough.\n\nWorking directory: {WORKING_DIR}\nMode: {MODE}\nFiles to review: {FILE_LIST}\n\nFocus ONLY on:\n1. Correctness: logic errors, wrong return values, missing null checks, incorrect conditions, type mismatches, off-by-one\n2. Edge cases: boundary conditions, empty inputs, overflow, concurrent access, race conditions\n\nRead each file listed above. For each issue found, output:\n\n### FINDING-{N}: {title}\n- Category: bug | edge-case\n- Severity: low | medium | high | critical\n- File: {path}\n- Location: {line range or function name}\n- Problem: {description}\n- Suggested fix: {concrete fix}\n\n{DIFF_OR_EMPTY}\n\nIf no issues found in your categories, state that explicitly." @@ -69,7 +68,6 @@ Use Claude Code's **native Agent tool** (built-in, no plugins needed) to spawn 4 ```json { - "subagent_type": "code-reviewer", "description": "Deep security review (OWASP Top 10)", "run_in_background": true, "prompt": "You are an independent SECURITY-FOCUSED code reviewer. Another AI (Codex) is reviewing the same code separately — you will NOT see their findings. Be exhaustive on security.\n\nWorking directory: {WORKING_DIR}\nMode: {MODE}\nFiles to review: {FILE_LIST}\n\nFocus ONLY on Security — cover ALL of the following:\n\n1. OWASP Top 10 2021:\n - A01 Broken Access Control: missing authz checks, IDOR, path traversal, CORS misconfiguration, privilege escalation\n - A02 Cryptographic Failures: weak algorithms (MD5/SHA1/DES), missing salt, hardcoded IVs, insecure random, missing HTTPS enforcement\n - A03 Injection: SQL, NoSQL, command, XSS, LDAP, template, log injection\n - A04 Insecure Design: missing rate limiting, insufficient input validation, business logic flaws\n - A05 Security Misconfiguration: default credentials, verbose errors, unnecessary features, missing security headers (CSP, X-Frame-Options, HSTS)\n - A06 Vulnerable Components: known CVEs in package.json/requirements.txt/go.mod, outdated deps with security patches\n - A07 Auth Failures: weak passwords, missing MFA, session fixation, insecure session management, missing account lockout\n - A08 Integrity Failures: insecure deserialization, missing integrity checks, unsigned code, CI/CD pipeline vulns\n - A09 Logging Failures: missing security event logging, sensitive data in logs, insufficient audit trails\n - A10 SSRF: unvalidated URLs, missing URL whitelist, internal service exposure, cloud metadata access\n\n2. Secrets & Credentials:\n - Hardcoded API keys, tokens, passwords in source code\n - .env files committed to git or exposed\n - Credentials leaked in logs or error messages\n - Sensitive data in URL query parameters\n\n3. Configuration Security:\n - CORS policy (overly permissive origins, credentials)\n - CSP headers (missing or weak)\n - Cookie flags (HttpOnly, Secure, SameSite)\n - TLS/HTTPS enforcement\n - Security headers completeness\n\n4. Cryptography:\n - Weak hash algorithms (MD5, SHA1 for passwords)\n - Missing salt in password hashing\n - Hardcoded encryption keys/IVs\n - Insecure random number generation (Math.random for security)\n - Deprecated crypto APIs\n\n5. Input & File Handling:\n - Path traversal in file operations\n - Unrestricted file upload (type, size, storage location)\n - Command injection via user input to exec/spawn\n - SSRF via user-controlled URLs\n - Open redirects\n\n6. Dependency Security:\n - Known CVEs in direct dependencies\n - Outdated packages with security patches available\n - Unmaintained dependencies\n - Missing dependency integrity checks (lockfile)\n\n7. Rate Limiting & DoS:\n - Missing throttle on authentication endpoints\n - Missing rate limiting on public API endpoints\n - Unbounded resource consumption (file size, query complexity)\n - ReDoS (regex denial of service)\n\n8. Auth Flow & Session:\n - JWT validation completeness (algorithm, expiration, issuer)\n - Session management (fixation, hijacking, timeout)\n - Privilege escalation paths\n - Token storage security (localStorage vs httpOnly cookie)\n - OAuth/OIDC misconfigurations\n\nFor each finding, output:\n\n### FINDING-{N}: {title}\n- Category: security\n- Subcategory: injection | broken-auth | sensitive-data | xxe | broken-access | security-config | xss | insecure-deserialization | logging | ssrf | crypto-failure | insecure-design | vulnerable-components | integrity-failure | rate-limiting | file-upload | secrets\n- Severity: low | medium | high | critical\n- Confidence: high | medium | low\n- CWE: CWE-{ID} ({Name})\n- OWASP: A{NN}:2021 - {Category Name}\n- File: {path}\n- Location: {line range or function name}\n- Problem: {description}\n- Attack Vector: {how an attacker could exploit this}\n- Suggested fix: {concrete secure code fix}\n\n{DIFF_OR_EMPTY}\n\nIf no security issues found, state that explicitly with a brief security posture summary." @@ -80,7 +78,6 @@ Use Claude Code's **native Agent tool** (built-in, no plugins needed) to spawn 4 ```json { - "subagent_type": "code-reviewer", "description": "Review performance issues", "run_in_background": true, "prompt": "You are an independent code reviewer. Another AI (Codex) is reviewing the same code separately — you will NOT see their findings. Be thorough.\n\nWorking directory: {WORKING_DIR}\nMode: {MODE}\nFiles to review: {FILE_LIST}\n\nFocus ONLY on Performance:\n1. Algorithmic: O(n²)+ loops, unnecessary nested iterations, inefficient data structure choices\n2. Memory: unnecessary allocations, large object cloning, missing cleanup, memory leaks, unbounded caches\n3. I/O: blocking I/O in async context, missing connection pooling, sequential requests that could be parallel\n4. Database: N+1 queries, missing indexes (if schema visible), unoptimized queries, missing pagination\n5. Caching: missing caching for expensive operations, cache invalidation issues, redundant computations\n6. Bundle/Load: unused imports, large dependency imports where tree-shaking possible, missing lazy loading\n\nRead each file listed above. For each issue found, output:\n\n### FINDING-{N}: {title}\n- Category: performance\n- Subcategory: algorithmic | memory | io | database | caching | bundle\n- Severity: low | medium | high | critical\n- File: {path}\n- Location: {line range or function name}\n- Problem: {description}\n- Impact: {estimated performance impact}\n- Suggested fix: {concrete fix}\n\n{DIFF_OR_EMPTY}\n\nIf no performance issues found, state that explicitly." @@ -91,7 +88,6 @@ Use Claude Code's **native Agent tool** (built-in, no plugins needed) to spawn 4 ```json { - "subagent_type": "code-reviewer", "description": "Review maintainability and architecture", "run_in_background": true, "prompt": "You are an independent code reviewer. Another AI (Codex) is reviewing the same code separately — you will NOT see their findings. Be thorough.\n\nWorking directory: {WORKING_DIR}\nMode: {MODE}\nFiles to review: {FILE_LIST}\n\nFocus ONLY on:\n1. Maintainability: naming clarity, DRY violations, missing error handling, overly complex logic, dead code, missing comments for complex logic\n2. Architecture: separation of concerns, module boundaries, API consistency, coupling issues\n\nRead each file listed above. For each issue found, output:\n\n### FINDING-{N}: {title}\n- Category: maintainability | architecture\n- Severity: low | medium | high | critical\n- File: {path}\n- Location: {line range or function name}\n- Problem: {description}\n- Suggested fix: {concrete fix}\n\n{DIFF_OR_EMPTY}\n\nIf no issues found in your categories, state that explicitly." diff --git a/skill-packs/codex-review/skills/codex-pr-review/references/workflow.md b/skill-packs/codex-review/skills/codex-pr-review/references/workflow.md index 56f19ae..768fdd0 100644 --- a/skill-packs/codex-review/skills/codex-pr-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-pr-review/references/workflow.md @@ -327,7 +327,7 @@ cat > "$SESSION_DIR/meta.json" << METAEOF "skill": "codex-pr-review", "version": 15, "effort": "$EFFORT", - "scope": "$SCOPE", + "scope": "$BASE_BRANCH", "rounds": ${ROUND_COUNT:-0}, "verdict": "$FINAL_VERDICT", "timing": { "total_seconds": ${ELAPSED_SECONDS:-0} }, @@ -337,7 +337,7 @@ METAEOF echo "Session saved to: $SESSION_DIR" ``` -Replace `$SCOPE` with the base branch used (e.g. `main`). Report `$SESSION_DIR` path to the user in the final summary. +The `$BASE_BRANCH` variable contains the base branch used (e.g. `main`). Report `$SESSION_DIR` path to the user in the final summary. ## Error Handling From edce2e4d4be5698fa5ecac7628345b587a8bb51b Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:47:10 +0700 Subject: [PATCH 07/25] fix(security-review): remove archive step, code-edit instruction, unify VERDICT vocab in workflow Co-Authored-By: Claude Opus 4.6 --- .../references/workflow.md | 46 +++++-------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 4e0f803..7aa0f2e 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -134,7 +134,7 @@ Group findings by severity: ```markdown # Security Review Results - Round 1 -**Verdict**: REVISE +**Verdict**: CONTINUE **Risk Level**: HIGH ## 🔴 Critical Issues (2) @@ -159,23 +159,9 @@ Group findings by severity: ### For Each Finding: -#### Option A: Accept and Fix +#### Response 1: Rebuttal -1. **Verify the finding** is a real vulnerability -2. **Apply the suggested fix** or implement alternative secure solution -3. **Document the fix** for round 2 summary - -Example: -```javascript -// ISSUE-1: SQL injection fixed -// Before: const query = `SELECT * FROM users WHERE name = '${name}'`; -// After: const query = 'SELECT * FROM users WHERE name = $1'; -const users = await db.query(query, [name]); -``` - -#### Option B: Rebut as False Positive - -If the finding is incorrect: +Write concrete proof that the finding is wrong: 1. **Gather evidence** showing why it's not a vulnerability 2. **Explain mitigating controls** (e.g., input validation elsewhere) @@ -183,13 +169,13 @@ If the finding is incorrect: Example rebuttal: ``` -ISSUE-3 is a false positive. The admin endpoint at /api/admin/users -is protected by the authenticateAdmin middleware (line 15) which -verifies JWT tokens and checks for admin role. The middleware is +ISSUE-3 is a false positive. The admin endpoint at /api/admin/users +is protected by the authenticateAdmin middleware (line 15) which +verifies JWT tokens and checks for admin role. The middleware is applied to all /api/admin/* routes in routes/index.js:42. ``` -#### Option C: Dispute Severity/Confidence +#### Response 2: Acknowledge with Severity Dispute If the finding is valid but severity is wrong: @@ -200,8 +186,8 @@ If the finding is valid but severity is wrong: Example: ``` ISSUE-5: Agree this is a concern, but severity should be MEDIUM not HIGH. -This endpoint is internal-only (not exposed to internet) and requires -VPN access. Additionally, we have rate limiting (10 req/min) which +This endpoint is internal-only (not exposed to internet) and requires +VPN access. Additionally, we have rate limiting (10 req/min) which mitigates brute force attacks. ``` @@ -251,7 +237,7 @@ Look for: ### Step 4: Iterate Until Consensus Continue rounds until: -- ✅ **VERDICT: APPROVE** - All critical/high issues resolved +- ✅ **VERDICT: CONSENSUS** - All critical/high issues resolved - ⚠️ **Stalemate** - Same disputes for 2+ rounds, no progress - 🛑 **User stops** - Manual intervention needed @@ -276,7 +262,7 @@ node "$RUNNER" stop "$SESSION_DIR" **Rounds**: {round_count} **Duration**: {duration} -## Final Verdict: {APPROVE | REVISE | STALEMATE} +## Final Verdict: {CONSENSUS | CONTINUE | STALEMATE} ## Security Risk Assessment: {CRITICAL | HIGH | MEDIUM | LOW} @@ -306,14 +292,6 @@ node "$RUNNER" stop "$SESSION_DIR" - [ ] Schedule follow-up security audit ``` -### Step 3: Archive Review Artifacts - -```bash -# Copy review artifacts to project docs -mkdir -p docs/security-reviews -cp "$SESSION_DIR/review.md" "docs/security-reviews/review-$(date +%Y%m%d).md" -``` - --- ## Polling Output Parsing @@ -542,7 +520,7 @@ while true; do case "$POLL" in POLL:running:*) sleep 15;; *) break;; esac done -if grep -q "VERDICT: REVISE" "$SESSION_DIR/review.md" 2>/dev/null; then +if grep -q "VERDICT: CONTINUE" "$SESSION_DIR/review.md" 2>/dev/null; then echo "❌ Security issues found. Commit blocked." echo "Run 'codex-security-review' for details." exit 1 From c4af0c53127f3b23891e6624eacf996d343d5ae8 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:52:41 +0700 Subject: [PATCH 08/25] feat(security-review): add prompt assembly step and claude-analysis-template.md Co-Authored-By: Claude Opus 4.6 --- .../references/claude-analysis-template.md | 54 +++++++++++++++++++ .../references/workflow.md | 22 ++++++++ 2 files changed, 76 insertions(+) create mode 100644 skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md diff --git a/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md b/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md new file mode 100644 index 0000000..26339e3 --- /dev/null +++ b/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md @@ -0,0 +1,54 @@ +# Claude Independent Security Analysis Template + +> Use this template for Step 2.5 (Information Barrier). +> Record analysis in working context ONLY — do NOT write to a file. +> Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + +## FINDING-{N} Format + +Use this exact shape for each independent finding: + +### FINDING-{N}: {Short title} +- Category: injection | broken-auth | sensitive-data | broken-access | security-config | xss | insecure-deserialization | logging | ssrf | crypto-failure | insecure-design | vulnerable-components | integrity-failure | secrets +- Severity: low | medium | high | critical +- Confidence: low | medium | high +- CWE: {CWE-NNN if known, otherwise omit} +- OWASP: {A0N:2021 category if applicable, otherwise omit} +- Location: {file path:line range} +- Problem: {clear statement of the vulnerability or security weakness} +- Evidence: {specific code pattern, snippet, or observation} +- Attack Vector: {how an attacker could exploit this} +- Why it matters: {impact — data exposure, privilege escalation, etc.} + +## Overall Security Assessment + +- Attack surface: {high / medium / low — based on exposed endpoints, data handled} +- Most critical area: {the single highest-risk component or pattern found} +- Confidence in analysis: {high / medium / low — limited by static analysis} + +## Strongest Positions + +List the 3-5 findings Claude is most confident about for cross-analysis debate: + +1. FINDING-{N}: {title} — {one-sentence rationale for high confidence} +2. ... + +## Cross-Analysis Matching Protocol + +After Round 1 poll completes, build a mapping table: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| FINDING-1 | ISSUE-2 | Genuine Agreement | +| FINDING-2 | — | Claude-only | +| — | ISSUE-4 | Codex-only | +| FINDING-3 | ISSUE-5 | Genuine Disagreement | + +Classification rules: +- **Genuine Agreement**: Same vulnerability class + same file/line area +- **Genuine Disagreement**: Same code area but conflicting assessment (one says vulnerable, other says safe) +- **Same Direction / Different Severity**: Both flag the same issue but assign different severity +- **Claude-only**: Claude's finding has no Codex counterpart +- **Codex-only**: Codex's finding has no Claude counterpart + +Maintain this table across all rounds. Update Classification and Status columns as rounds progress. diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 7aa0f2e..9026d82 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -67,6 +67,28 @@ Include: --- +## Step 1.8: Assemble Prompt + +Build `$PROMPT` using multi-step placeholder replacement. +DO NOT use a single sed pipeline — `output-format.md` may contain `&`, `\`, `/` characters +that corrupt sed replacements. Use `printf '%s'` piping. + +a) Read `references/prompts.md` as template base +b) Replace `{WORKING_DIR}` with current working directory +c) Replace `{SCOPE}` with detected `$SCOPE` value +d) Replace `{EFFORT}` with detected `$EFFORT` value +e) Replace `{SCOPE_SPECIFIC_INSTRUCTIONS}` with the scope-specific block from prompts.md + matching the detected `$SCOPE` (working-tree / branch / full) +f) Replace `{OUTPUT_FORMAT}` by reading `references/output-format.md` in full + using: `printf '%s' "$(cat references/output-format.md)"` +g) Replace any remaining placeholders (`{BASE_BRANCH}` etc.) + +Store result as `$PROMPT`. + +Verify: `$PROMPT` must contain no prematurely-closed triple-backtick fences. + +--- + ## Phase 2: Round 1 - Initial Security Analysis ### Step 1: Start Codex Review From 9da6bc75dbb9240cdce33f7a475a67d0b7ee4a32 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:55:55 +0700 Subject: [PATCH 09/25] fix(security-review): correct Step 1.8 heading level and position inside Phase 1 Step 1.8 was an orphan H2 section placed after Phase 1's closing --- divider. Moved it inside Phase 1 (before the divider) and changed the heading from ## to ### to match the other steps in that phase. Co-Authored-By: Claude Opus 4.6 --- .../skills/codex-security-review/references/workflow.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 9026d82..b97303c 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -65,9 +65,7 @@ Include: - Secrets scanning instructions - Effort-appropriate depth ---- - -## Step 1.8: Assemble Prompt +### Step 1.8: Assemble Prompt Build `$PROMPT` using multi-step placeholder replacement. DO NOT use a single sed pipeline — `output-format.md` may contain `&`, `\`, `/` characters From 4f974fbbd5dae2c2ddc1e42e964c3a41ed7c7e98 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 18:58:52 +0700 Subject: [PATCH 10/25] feat(plan-review): add information barrier Step 2.5 and cross-analysis Step 4 Co-Authored-By: Claude Opus 4.6 --- .../codex-plan-review/references/workflow.md | 69 +++++++++++++++---- 1 file changed, 57 insertions(+), 12 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md b/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md index 2bf0880..1438903 100644 --- a/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md @@ -69,6 +69,25 @@ START_OUTPUT=$(printf '%s' "$PROMPT" | node "$RUNNER" start "$SESSION_DIR" --eff **Validate init output:** Verify `INIT_OUTPUT` starts with `CODEX_SESSION:`. If not, report error. **Validate start output:** Verify `START_OUTPUT` starts with `CODEX_STARTED:`. If not, report error. +## 2.5) Information Barrier — Claude Independent Plan Analysis + +MUST complete before polling Codex output. +Codex is running in background — use this time productively. + +Read the plan file at `$PLAN_PATH` directly. +Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + +Form an independent FINDING-{N} list in working context (do NOT write to a file): +- Correctness issues (steps that are wrong or will fail) +- Architecture concerns (structural problems with the approach) +- Sequencing/dependency problems (steps out of order, missing prerequisites) +- Scope gaps or risks (missing requirements, underestimated complexity) + +Use the same FINDING-{N} format as `output-format.md` ISSUE-{N} (same field names). + +INFORMATION BARRIER ends after Round 1 poll completes. +From Round 2 onwards, the barrier no longer applies. + ## 3) Poll ```bash @@ -99,18 +118,44 @@ After each poll, report **specific activities** to the user using the `SUMMARY:` Continue while status is `running`. Stop on `completed|failed|timeout|stalled`. -## 4) Parse Review -- Read `THREAD_ID:` and `review.md` from runner output/state directory. -- Extract `ISSUE-{N}` blocks. -- Apply accepted fixes to plan. -- **Save the updated plan file before resuming.** Codex round 2+ will re-read it from the plan path. -- Build rebuttal packet for disputed items. -- Record the set of open (unresolved) ISSUE-{N} IDs for stalemate tracking. - -After parsing each round's review, append round summary to `$SESSION_DIR/rounds.json`: -- Read existing rounds.json or start with empty array `[]` -- Append: `{ "round": N, "elapsed_seconds": ..., "verdict": "...", "issues_found": ..., "issues_fixed": ..., "issues_disputed": ... }` -- Write back to `$SESSION_DIR/rounds.json` +## 4) Cross-Analysis + +After Round 1 poll completes and `$SESSION_DIR/review.md` is available: + +### 4a) Parse Codex Output +Read all `ISSUE-{N}` blocks from `$SESSION_DIR/review.md`. + +### 4b) Build FINDING↔ISSUE Mapping Table +Map Claude's FINDING-{N} (from Step 2.5) against Codex's ISSUE-{N}: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| ... | ... | ... | + +Classification options: +- **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same plan problem +- **Codex-only**: ISSUE-{M} has no matching Claude FINDING +- **Claude-only**: FINDING-{N} has no matching Codex ISSUE +- **Genuine Disagreement**: Conflicting assessments of the same plan section + +### 4c) Determine Response for Each ISSUE +For each ISSUE-{N}: +- Genuine agreement or Codex-only → apply fix to the plan file +- Claude-only → include in final report as Claude finding +- Genuine disagreement → write rebuttal with concrete reasoning + +### 4d) Apply Fixes +Apply accepted fixes to the plan file at `$PLAN_PATH`. +Save the updated plan file before resuming. +**Critical**: Codex Round 2+ re-reads the plan from `$PLAN_PATH` — unsaved changes are invisible to Codex. + +### 4e) Record Round Summary +Append to `$SESSION_DIR/rounds.json`: +```json +{ "round": N, "elapsed_seconds": ..., "verdict": "...", "issues_found": ..., "issues_fixed": ..., "issues_disputed": ... } +``` + +Proceed to Step 5 (resume) or Step 7 (final output) based on VERDICT. ## 5) Resume (Round 2+) From 606cff7eec54ba6272cb840a467bda35bb0cd033 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 19:02:15 +0700 Subject: [PATCH 11/25] feat(impl-review): add information barrier Step 2.5 and cross-analysis in Step 4 Co-Authored-By: Claude Opus 4.6 --- .../codex-impl-review/references/workflow.md | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md b/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md index 756293e..39a8283 100644 --- a/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md @@ -85,6 +85,27 @@ START_OUTPUT=$(printf '%s' "$PROMPT" | node "$RUNNER" start "$SESSION_DIR" --eff **Validate init output:** Verify `INIT_OUTPUT` starts with `CODEX_SESSION:`. If not, report error. **Validate start output:** Verify `START_OUTPUT` starts with `CODEX_STARTED:`. If not, report error. +## 2.5) Information Barrier — Claude Independent Code Analysis + +MUST complete before polling Codex output. +Codex is running in background — use this time productively. + +**working-tree mode**: run `git diff` and `git diff --cached` yourself + (or reuse the diff already collected in Step 1 — working tree hasn't changed) +**branch mode**: run `git diff $BASE_BRANCH...HEAD` yourself + (or reuse the diff already collected in Step 1) + +Form an independent FINDING-{N} list in working context (do NOT write to a file): +- Bugs and edge cases +- Security issues +- Performance concerns +- Maintainability problems + +Use the same FINDING-{N} format as `output-format.md` ISSUE-{N} (same field names). +Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + +INFORMATION BARRIER ends after Round 1 poll completes. + ## 3) Poll ```bash @@ -116,7 +137,29 @@ Continue while status is `running`. Stop on `completed|failed|timeout|stalled`. ## 4) Apply/Rebut -- Parse `ISSUE-{N}` blocks. + +### 4a) Parse Codex Output +Read all `ISSUE-{N}` blocks from `$SESSION_DIR/review.md`. + +### 4b) Build FINDING↔ISSUE Mapping Table +Map Claude's FINDING-{N} (from Step 2.5) against Codex's ISSUE-{N}: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| ... | ... | ... | + +Classification options: +- **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same code problem +- **Codex-only**: ISSUE-{M} has no matching Claude FINDING +- **Claude-only**: FINDING-{N} has no matching Codex ISSUE +- **Genuine Disagreement**: Conflicting assessments of the same code + +### 4c) Apply/Rebut using cross-analysis context +For each ISSUE-{N}: +- If genuine agreement or Codex-only → apply fix to code +- If Claude-only → include in final report as Claude finding +- If genuine disagreement → write rebuttal with concrete proof (paths, tests, behavior) + - For valid issues: edit code and record fix evidence. - For invalid issues: write rebuttal with concrete proof (paths, tests, behavior). - **Branch mode only**: after applying fixes, commit them (`git add` + `git commit`) before resuming. Codex reads `git diff ...HEAD` which only includes committed changes — uncommitted fixes will be invisible to Codex and cause repeated issues. From 9f14be7e00d74e96b98b07edf80ce2fed02b0589 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 19:05:30 +0700 Subject: [PATCH 12/25] feat(security-review): add information barrier Step 2.5 and FINDING/ISSUE cross-analysis Co-Authored-By: Claude Opus 4.6 --- .../references/workflow.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index b97303c..f851ffd 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -109,6 +109,21 @@ START_OUTPUT=$(printf '%s' "$PROMPT" | node "$RUNNER" start "$SESSION_DIR" --eff **Validate init output:** Verify `INIT_OUTPUT` starts with `CODEX_SESSION:`. If not, report error. **Validate start output:** Verify `START_OUTPUT` starts with `CODEX_STARTED:`. If not, report error. +### Phase 2, Step 2.5: Information Barrier — Claude Independent Security Analysis + +MUST complete before polling Codex output. +Codex is running in background (typically 90-180s) — use this time. + +Using `references/claude-analysis-template.md`: +- Read all files in scope directly (do NOT read `$SESSION_DIR/review.md`) +- Identify top attack surfaces +- Form an independent FINDING-{N} list using OWASP categories +- Note high-confidence vs uncertain findings + +Keep analysis in working context. Do NOT write a file. +INFORMATION BARRIER ends after Round 1 poll completes. +From Round 2 onwards, the barrier no longer applies. + ### Step 2: Poll for Progress Use adaptive polling intervals: @@ -129,6 +144,23 @@ node "$RUNNER" poll "$SESSION_DIR" When poll returns `POLL:completed`: +### Cross-Analysis: Build FINDING↔ISSUE Mapping Table + +Map Claude's FINDING-{N} (from Step 2.5) against Codex's ISSUE-{N}: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| ... | ... | ... | + +Classification options: +- **Genuine Agreement**: Same vulnerability class + same file/line area +- **Genuine Disagreement**: Same code area but conflicting assessment +- **Same Direction / Different Severity**: Both flag same issue but different severity +- **Claude-only**: Claude's finding has no Codex counterpart +- **Codex-only**: Codex's finding has no Claude counterpart + +After mapping, proceed with existing finding processing below. + 1. Read review output from `$SESSION_DIR/review.md` 2. Parse ISSUE-{N} blocks using regex: ```regex From 8c153bceabd5179994d951433577eb449e941e23 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 19:08:46 +0700 Subject: [PATCH 13/25] fix(security-review): unify VERDICT vocabulary to CONSENSUS/CONTINUE/STALEMATE Co-Authored-By: Claude Opus 4.6 --- .../codex-security-review/references/output-format.md | 7 ++++++- .../skills/codex-security-review/references/prompts.md | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/output-format.md b/skill-packs/codex-review/skills/codex-security-review/references/output-format.md index 62fd4ae..0e413bb 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/output-format.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/output-format.md @@ -223,10 +223,15 @@ Full CWE list: https://cwe.mitre.org/ ## Verdict Block ``` -VERDICT: APPROVE | REVISE +VERDICT: CONSENSUS | CONTINUE | STALEMATE Status: {complete | stalemate | in-progress} Reason: {explanation} +Verdict values: +- CONSENSUS: All critical/high security issues resolved or agreed-upon — ready to proceed +- CONTINUE: Security issues remain that require another review round +- STALEMATE: Circular debate — same disputes for 2+ rounds with no progress + Security Risk Summary: - Critical: {count} issues - High: {count} issues diff --git a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md index d00a640..8dfca20 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md @@ -115,6 +115,8 @@ Perform a comprehensive security analysis focusing on OWASP Top 10 2021 vulnerab ## Required Output Format {OUTPUT_FORMAT} + +End with a VERDICT block. Do not skip it. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. ``` --- @@ -201,6 +203,8 @@ You are continuing a security review debate. ## Required Output Format {OUTPUT_FORMAT} + +End with a VERDICT block. Do not skip it. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. ``` --- From f3e038408cb2c59325c20a8063763c73d9eb2d18 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 19:10:49 +0700 Subject: [PATCH 14/25] fix(plan-review): unify VERDICT vocabulary to CONSENSUS/CONTINUE/STALEMATE Co-Authored-By: Claude Opus 4.6 --- .../skills/codex-plan-review/references/output-format.md | 7 +++++-- .../skills/codex-plan-review/references/prompts.md | 4 +++- .../skills/codex-plan-review/references/workflow.md | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-plan-review/references/output-format.md b/skill-packs/codex-review/skills/codex-plan-review/references/output-format.md index 5dd5c9d..6dbee57 100644 --- a/skill-packs/codex-review/skills/codex-plan-review/references/output-format.md +++ b/skill-packs/codex-review/skills/codex-plan-review/references/output-format.md @@ -17,8 +17,11 @@ Use this exact shape (copy the entire block below as `{OUTPUT_FORMAT}`): - Suggested fix: {plan-level change} ### VERDICT -- Status: APPROVE | REVISE +- Status: CONSENSUS | CONTINUE | STALEMATE + - CONSENSUS: No remaining plan issues — ready to implement + - CONTINUE: Issues remain that require another review round + - STALEMATE: Circular debate — same disputes for 2+ rounds with no progress - Reason: {short reason} ``` -**Zero-issue rule**: If no issues remain, omit all ISSUE blocks and return only the VERDICT block with `Status: APPROVE` and `Reason: Plan is complete, well-structured, and addresses all acceptance criteria.` +**Zero-issue rule**: If no issues remain, omit all ISSUE blocks and return only the VERDICT block with `Status: CONSENSUS` and `Reason: Plan is complete, well-structured, and addresses all acceptance criteria.` diff --git a/skill-packs/codex-review/skills/codex-plan-review/references/prompts.md b/skill-packs/codex-review/skills/codex-plan-review/references/prompts.md index 3261b1f..41fb28e 100644 --- a/skill-packs/codex-review/skills/codex-plan-review/references/prompts.md +++ b/skill-packs/codex-review/skills/codex-plan-review/references/prompts.md @@ -51,7 +51,8 @@ Read the plan file directly at: {PLAN_PATH} 4. Do NOT propose code changes — review only the plan quality. 5. Output each finding as ISSUE-{N} using the EXACT format below. 6. End with a VERDICT block. Do not skip it. -7. Keep ISSUE-{N} IDs stable — do not renumber in later rounds. +7. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. +8. Keep ISSUE-{N} IDs stable — do not renumber in later rounds. ## Required Output Format {OUTPUT_FORMAT} @@ -81,6 +82,7 @@ Read the updated plan file directly at: {PLAN_PATH} 5. Focus on remaining open issues and any NEW findings from the updated plan. 6. Maintain the same ISSUE-{N} numbering. New findings use the next available number. 7. End with a VERDICT block. +8. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. ## Required Output Format {OUTPUT_FORMAT} diff --git a/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md b/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md index 1438903..c6d53e2 100644 --- a/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-plan-review/references/workflow.md @@ -168,7 +168,7 @@ START_OUTPUT=$(printf '%s' "$REBUTTAL_PROMPT" | node "$RUNNER" resume "$SESSION_ Then **go back to step 3 (Poll).** After poll completes, repeat step 4 (Parse) and check stop conditions below. If not met, resume again (step 5). Continue this loop until a stop condition is reached. ## 6) Stop Conditions -- `VERDICT: APPROVE`. +- `VERDICT: CONSENSUS`. - Stalemate detected (see below). - User stops debate. - **Hard cap: 5 rounds.** At cap, force final synthesis with unresolved issues listed as residual risks. @@ -192,7 +192,7 @@ At stalemate: | Metric | Value | |--------|-------| | Rounds | {N} | -| Verdict | {APPROVE/REVISE/STALEMATE} | +| Verdict | {CONSENSUS/CONTINUE/STALEMATE} | | Issues Found | {total} | | Issues Fixed | {fixed_count} | | Issues Disputed | {disputed_count} | From bd3c43ab87796b7c32035637dc2c686cf4901499 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 19:13:38 +0700 Subject: [PATCH 15/25] fix(impl-review): unify VERDICT vocabulary to CONSENSUS/CONTINUE/STALEMATE Co-Authored-By: Claude Opus 4.6 --- .../skills/codex-impl-review/references/output-format.md | 7 +++++-- .../skills/codex-impl-review/references/prompts.md | 8 ++++++-- .../skills/codex-impl-review/references/workflow.md | 4 ++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-impl-review/references/output-format.md b/skill-packs/codex-review/skills/codex-impl-review/references/output-format.md index f94a746..2a6c4a4 100644 --- a/skill-packs/codex-review/skills/codex-impl-review/references/output-format.md +++ b/skill-packs/codex-review/skills/codex-impl-review/references/output-format.md @@ -17,8 +17,11 @@ Use this exact shape (copy the entire block below as `{OUTPUT_FORMAT}`): - Suggested fix: {concrete code change} ### VERDICT -- Status: APPROVE | REVISE +- Status: CONSENSUS | CONTINUE | STALEMATE + - CONSENSUS: No remaining code issues — changes are correct and safe + - CONTINUE: Issues remain that require fixes and another review round + - STALEMATE: Circular debate — same disputes for 2+ rounds with no progress - Reason: {short reason} ``` -**Zero-issue rule**: If no issues remain, omit all ISSUE blocks and return only the VERDICT block with `Status: APPROVE` and `Reason: All changes are correct, well-tested, and safe to merge.` +**Zero-issue rule**: If no issues remain, omit all ISSUE blocks and return only the VERDICT block with `Status: CONSENSUS` and `Reason: All changes are correct, well-tested, and safe to merge.` diff --git a/skill-packs/codex-review/skills/codex-impl-review/references/prompts.md b/skill-packs/codex-review/skills/codex-impl-review/references/prompts.md index a62a2c3..02919b3 100644 --- a/skill-packs/codex-review/skills/codex-impl-review/references/prompts.md +++ b/skill-packs/codex-review/skills/codex-impl-review/references/prompts.md @@ -55,7 +55,8 @@ You are Codex acting as a strict code reviewer. 3. Output each finding as ISSUE-{N} using the EXACT format below. 4. Keep ISSUE-{N} IDs stable — do not renumber in later rounds. 5. End with a VERDICT block. Do not skip it. -6. Use required output format exactly. +6. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. +7. Use required output format exactly. ## Required Output Format {OUTPUT_FORMAT} @@ -86,7 +87,8 @@ You are Codex acting as a strict code reviewer. 3. Output each finding as ISSUE-{N} using the EXACT format below. 4. Keep ISSUE-{N} IDs stable — do not renumber in later rounds. 5. End with a VERDICT block. Do not skip it. -6. Use required output format exactly. +6. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. +7. Use required output format exactly. ## Required Output Format {OUTPUT_FORMAT} @@ -113,6 +115,7 @@ You are Codex acting as a strict code reviewer. 6. Maintain the same ISSUE-{N} numbering. New findings use the next available number. 7. Keep already-fixed issues closed. 8. End with a VERDICT block. +9. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. ## Required Output Format {OUTPUT_FORMAT} @@ -144,6 +147,7 @@ You are Codex acting as a strict code reviewer. 6. Maintain the same ISSUE-{N} numbering. New findings use the next available number. 7. Keep already-fixed issues closed. 8. End with a VERDICT block. +9. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. ## Required Output Format {OUTPUT_FORMAT} diff --git a/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md b/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md index 39a8283..7236334 100644 --- a/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-impl-review/references/workflow.md @@ -186,7 +186,7 @@ START_OUTPUT=$(printf '%s' "$REBUTTAL_PROMPT" | node "$RUNNER" resume "$SESSION_ Then **go back to step 3 (Poll).** After poll completes, repeat step 4 (Apply/Rebut) and check completion criteria below. If not met, resume again (step 5). Continue this loop until a completion criterion is reached. ## 6) Completion Criteria -- Codex returns `VERDICT: APPROVE`. +- Codex returns `VERDICT: CONSENSUS`. - Or user accepts a documented stalemate. - **Hard cap: 5 rounds.** At cap, force final synthesis with unresolved issues listed as residual risks. @@ -209,7 +209,7 @@ At stalemate: | Metric | Value | |--------|-------| | Rounds | {N} | -| Verdict | {APPROVE/REVISE/STALEMATE} | +| Verdict | {CONSENSUS/CONTINUE/STALEMATE} | | Issues Found | {total} | | Issues Fixed | {fixed_count} | | Issues Disputed | {disputed_count} | From aac3242643c07513a7828e796e0a3f45826844bb Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 19:52:19 +0700 Subject: [PATCH 16/25] fix(security-review): resolve nested fence corruption, STALEMATE gate bypass, and category mismatch Fixes three issues identified in codex-impl-review: ISSUE-1: Remove outer fence wrappers from prompt templates in prompts.md - Security Review Prompt (Round 1) no longer wrapped in backticks - Round 2+ Prompt (Resume) no longer wrapped in backticks - Working Tree/Branch/Full mode prompts no longer wrapped in backticks - Prevents nested fence corruption when {OUTPUT_FORMAT} placeholder injects output-format.md content (which contains fenced blocks) ISSUE-2: Update pre-commit hook to check both CONTINUE and STALEMATE - workflow.md line 575: Changed from grep "VERDICT: CONTINUE" to grep -qE "VERDICT: (CONTINUE|STALEMATE)" - Prevents stalemated reviews with unresolved findings from bypassing security gate ISSUE-4: Align category taxonomy between template and output-format - claude-analysis-template.md: Removed 'secrets' category, added 'xxe' - Now matches the 13 categories defined in output-format.md - Categories: injection, broken-auth, sensitive-data, xxe, broken-access, security-config, xss, insecure-deserialization, logging, ssrf, crypto-failure, insecure-design, vulnerable-components, integrity-failure Co-Authored-By: Claude Opus 4.6 --- .../references/claude-analysis-template.md | 2 +- .../skills/codex-security-review/references/prompts.md | 10 ---------- .../codex-security-review/references/workflow.md | 2 +- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md b/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md index 26339e3..14ba1d8 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md @@ -9,7 +9,7 @@ Use this exact shape for each independent finding: ### FINDING-{N}: {Short title} -- Category: injection | broken-auth | sensitive-data | broken-access | security-config | xss | insecure-deserialization | logging | ssrf | crypto-failure | insecure-design | vulnerable-components | integrity-failure | secrets +- Category: injection | broken-auth | sensitive-data | xxe | broken-access | security-config | xss | insecure-deserialization | logging | ssrf | crypto-failure | insecure-design | vulnerable-components | integrity-failure - Severity: low | medium | high | critical - Confidence: low | medium | high - CWE: {CWE-NNN if known, otherwise omit} diff --git a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md index 8dfca20..3c832bb 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md @@ -2,7 +2,6 @@ ## Security Review Prompt (Round 1) -``` You are a security expert conducting a thorough security review of code changes. ## Context @@ -117,13 +116,11 @@ Perform a comprehensive security analysis focusing on OWASP Top 10 2021 vulnerab {OUTPUT_FORMAT} End with a VERDICT block. Do not skip it. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. -``` --- ## Security Review Prompt - Working Tree Mode -``` ## Review Scope: Uncommitted Changes Analyze only the uncommitted changes in the working tree (staged and unstaged). @@ -135,13 +132,11 @@ Focus on: 4. Security controls removed or weakened Use `git diff` to see changes. Review both the changed lines and surrounding context. -``` --- ## Security Review Prompt - Branch Mode -``` ## Review Scope: Branch Diff Analyze all changes in the current branch compared to base branch: {BASE_BRANCH} @@ -153,13 +148,11 @@ Focus on: 4. Security controls added or removed Use `git diff {BASE_BRANCH}...HEAD` to see all changes. -``` --- ## Security Review Prompt - Full Codebase Mode -``` ## Review Scope: Full Codebase Analyze the entire codebase for security vulnerabilities. @@ -177,13 +170,11 @@ Prioritize high-severity findings. For large codebases, focus on: - External API calls - File operations - Cryptographic operations -``` --- ## Round 2+ Prompt (Resume) -``` You are continuing a security review debate. ## Previous Round Summary @@ -205,7 +196,6 @@ You are continuing a security review debate. {OUTPUT_FORMAT} End with a VERDICT block. Do not skip it. Use `STALEMATE` if the same arguments repeat for 2+ consecutive rounds with no new evidence. -``` --- diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index f851ffd..5aa99c6 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -572,7 +572,7 @@ while true; do case "$POLL" in POLL:running:*) sleep 15;; *) break;; esac done -if grep -q "VERDICT: CONTINUE" "$SESSION_DIR/review.md" 2>/dev/null; then +if grep -qE "VERDICT: (CONTINUE|STALEMATE)" "$SESSION_DIR/review.md" 2>/dev/null; then echo "❌ Security issues found. Commit blocked." echo "Run 'codex-security-review' for details." exit 1 From 9ae69eea3476f1b89938454475a51bd45c9c5ca2 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:00:42 +0700 Subject: [PATCH 17/25] fix(security-review): invert pre-commit gate to fail-closed Fixes ISSUE-5 from codex-impl-review: The previous fix checked for CONTINUE|STALEMATE but still allowed commits to pass when the verdict was missing, malformed, or unexpected (fail-open). Changed workflow.md line 575 from: if grep -qE "VERDICT: (CONTINUE|STALEMATE)" ... then exit 1 To: if ! grep -qE '^VERDICT: CONSENSUS$' ... then exit 1 Now the gate only passes on explicit CONSENSUS verdict. Any other outcome (CONTINUE, STALEMATE, missing verdict, malformed output) blocks the commit. This is the correct security posture for a pre-commit gate - require explicit pass condition rather than checking for known fail conditions. Co-Authored-By: Claude Opus 4.6 --- .../skills/codex-security-review/references/workflow.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 5aa99c6..09e9415 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -572,8 +572,8 @@ while true; do case "$POLL" in POLL:running:*) sleep 15;; *) break;; esac done -if grep -qE "VERDICT: (CONTINUE|STALEMATE)" "$SESSION_DIR/review.md" 2>/dev/null; then - echo "❌ Security issues found. Commit blocked." +if ! grep -qE '^VERDICT: CONSENSUS$' "$SESSION_DIR/review.md" 2>/dev/null; then + echo "❌ Security issues found or review incomplete. Commit blocked." echo "Run 'codex-security-review' for details." exit 1 fi From e81406884deed405319c812fb3891c42e65958ef Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:07:04 +0700 Subject: [PATCH 18/25] fix(security-review): close remaining gate bypasses and align verdict vocabulary Fixes ISSUE-1, ISSUE-2, ISSUE-3 from second codex-impl-review: ISSUE-1: Pre-commit hook now requires exactly one VERDICT and it must be CONSENSUS - Changed workflow.md line 575 from simple grep to count-based validation - Requires exactly 1 verdict line total AND exactly 1 CONSENSUS verdict - Prevents bypass via multiple verdicts or malformed output - Blocks commits on missing verdict, multiple verdicts, or non-CONSENSUS verdict ISSUE-2: Removed outer fences from example sections in prompts.md - Example Security Finding (lines 228-259) no longer wrapped in backticks - Example Secrets Detection (lines 265-316) no longer wrapped in backticks - Prevents nested fence corruption when examples contain code blocks - Completes the fix started in previous commit ISSUE-3: Updated SKILL.md to use CONSENSUS verdict vocabulary - Line 37: Changed "until APPROVE or stalemate" to "until CONSENSUS verdict or stalemate" - Aligns top-level skill instructions with output-format.md and workflow.md - Completes VERDICT vocabulary unification across security-review skill Co-Authored-By: Claude Opus 4.6 --- .../codex-review/skills/codex-security-review/SKILL.md | 2 +- .../skills/codex-security-review/references/prompts.md | 4 ---- .../skills/codex-security-review/references/workflow.md | 5 ++++- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/SKILL.md b/skill-packs/codex-review/skills/codex-security-review/SKILL.md index 753c482..5edf476 100644 --- a/skill-packs/codex-review/skills/codex-security-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-security-review/SKILL.md @@ -34,7 +34,7 @@ RUNNER="{{RUNNER_PATH}}" 4. Poll with adaptive intervals (Round 1: 60s/60s/30s/15s..., Round 2+: 30s/15s...). After each poll, report **specific activities** from poll output (e.g. which files Codex is analyzing, what vulnerability patterns it's checking). See `references/workflow.md` for parsing guide. NEVER report generic "Codex is running" — always extract concrete details. 5. Parse security findings with `references/output-format.md` (includes CWE/OWASP mappings). 6. Fix valid vulnerabilities in code; rebut false positives with evidence. -7. Resume debate via `node "$RUNNER" resume "$SESSION_DIR"` until `APPROVE` or stalemate. +7. Resume debate via `node "$RUNNER" resume "$SESSION_DIR"` until `CONSENSUS` verdict or stalemate. 8. Return final security assessment with risk summary. ### Effort Level Guide diff --git a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md index 3c832bb..a03bfe9 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/prompts.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/prompts.md @@ -225,7 +225,6 @@ Recommendation: [Proceed with caution | Block merge | Security expert review req ## Example Security Finding -``` ISSUE-1: SQL Injection in user search endpoint Category: injection Severity: critical @@ -256,13 +255,11 @@ const users = await db.query(query, [req.query.name]); ``` Parameterized queries ensure user input is treated as data, not executable SQL code. -``` --- ## Example Secrets Detection -``` ISSUE-2: Hardcoded AWS credentials in configuration file Category: sensitive-data Severity: critical @@ -303,7 +300,6 @@ Additionally: 2. Use AWS IAM roles for EC2/Lambda instead of access keys 3. Add config/*.js to .gitignore 4. Use AWS Secrets Manager for production credentials -``` --- diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 09e9415..5279324 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -572,7 +572,10 @@ while true; do case "$POLL" in POLL:running:*) sleep 15;; *) break;; esac done -if ! grep -qE '^VERDICT: CONSENSUS$' "$SESSION_DIR/review.md" 2>/dev/null; then +VERDICT_COUNT=$(grep -cE '^VERDICT: (CONSENSUS|CONTINUE|STALEMATE)$' "$SESSION_DIR/review.md" 2>/dev/null || echo 0) +CONSENSUS_COUNT=$(grep -cE '^VERDICT: CONSENSUS$' "$SESSION_DIR/review.md" 2>/dev/null || echo 0) + +if [ "$VERDICT_COUNT" -ne 1 ] || [ "$CONSENSUS_COUNT" -ne 1 ]; then echo "❌ Security issues found or review incomplete. Commit blocked." echo "Run 'codex-security-review' for details." exit 1 From 5fa2466fdceb72ea8d86919037651b6b6cec7a9e Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:13:40 +0700 Subject: [PATCH 19/25] fix(security-review): resolve grep fail-open and output-format nested fences Fixes ISSUE-1 and ISSUE-2 from third codex-impl-review: ISSUE-1: Pre-commit verdict counting now truly fail-closed - Changed workflow.md line 575 from grep -c (which fails open on zero matches) to awk-based counting with explicit zero normalization - grep -c prints "0" and exits non-zero when no matches found, causing command substitution to capture both grep's "0" and fallback "echo 0" - New approach: awk counts matches and always returns numeric 0 if empty - Added explicit [ -n "$VAR" ] || VAR=0 guards for safety - Now correctly blocks commits on missing review.md, zero verdicts, or non-CONSENSUS verdicts ISSUE-2: Removed outer fences from output-format.md examples - Example: Complete Security Finding (lines 267-325) no longer wrapped - Example: Secrets Detection (lines 331-387) no longer wrapped - Response Format (Round 2+) (lines 393+) no longer wrapped - Completes nested fence elimination across all reference files - Final assembled prompt now has no fence corruption All security-review fixes are now complete: - No nested fence corruption anywhere - Pre-commit gate is truly fail-closed with exact verdict counting - Category taxonomy aligned across all files - VERDICT vocabulary unified to CONSENSUS|CONTINUE|STALEMATE Co-Authored-By: Claude Opus 4.6 --- .../codex-security-review/references/output-format.md | 5 ----- .../skills/codex-security-review/references/workflow.md | 6 ++++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/output-format.md b/skill-packs/codex-review/skills/codex-security-review/references/output-format.md index 0e413bb..d48d65c 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/output-format.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/output-format.md @@ -264,7 +264,6 @@ Advisory Issues (should fix, not blocking): ## Example: Complete Security Finding -``` ISSUE-1: SQL Injection in user search endpoint Category: injection Severity: critical @@ -322,13 +321,11 @@ app.get('/api/users/search', async (req, res) => { 2. Implement rate limiting on search endpoint 3. Use prepared statements for all database queries 4. Enable SQL query logging for security monitoring -``` --- ## Example: Secrets Detection -``` ISSUE-2: Hardcoded AWS credentials in configuration Category: sensitive-data Severity: critical @@ -384,13 +381,11 @@ if (!AWS_CONFIG.accessKeyId || !AWS_CONFIG.secretAccessKey) { 2. Use AWS Secrets Manager for production credentials 3. Implement pre-commit hooks to detect secrets (e.g., git-secrets, truffleHog) 4. Enable AWS GuardDuty for threat detection -``` --- ## Response Format (Round 2+) -``` RESPONSE-{N}: Re: ISSUE-{N} Action: accept | reject | revise Reason: [Response to rebuttal] diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 5279324..0cb8a3a 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -572,8 +572,10 @@ while true; do case "$POLL" in POLL:running:*) sleep 15;; *) break;; esac done -VERDICT_COUNT=$(grep -cE '^VERDICT: (CONSENSUS|CONTINUE|STALEMATE)$' "$SESSION_DIR/review.md" 2>/dev/null || echo 0) -CONSENSUS_COUNT=$(grep -cE '^VERDICT: CONSENSUS$' "$SESSION_DIR/review.md" 2>/dev/null || echo 0) +VERDICT_COUNT=$(awk '/^VERDICT: (CONSENSUS|CONTINUE|STALEMATE)$/ {count++} END {print count+0}' "$SESSION_DIR/review.md" 2>/dev/null) +CONSENSUS_COUNT=$(awk '/^VERDICT: CONSENSUS$/ {count++} END {print count+0}' "$SESSION_DIR/review.md" 2>/dev/null) +[ -n "$VERDICT_COUNT" ] || VERDICT_COUNT=0 +[ -n "$CONSENSUS_COUNT" ] || CONSENSUS_COUNT=0 if [ "$VERDICT_COUNT" -ne 1 ] || [ "$CONSENSUS_COUNT" -ne 1 ]; then echo "❌ Security issues found or review incomplete. Commit blocked." From 95e6d02bd2f26542f4277921d6a19e0327405fc7 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:23:17 +0700 Subject: [PATCH 20/25] fix(security-review): resolve prompt assembly, base branch, and format issues Fixes ISSUE-1, ISSUE-2, ISSUE-3, ISSUE-4 from fourth codex-impl-review: ISSUE-1: Clarified Round 1 prompt assembly to exclude Round 2 sections - workflow.md step 1.8(a): Changed from "Read references/prompts.md as template base" to "Extract only the Round 1 prompt section from references/prompts.md" - Explicitly excludes Round 2+ prompt sections to prevent contradictory instructions - Prevents unresolved {FIXED_ITEMS} and {DISPUTED_ITEMS} placeholders in Round 1 ISSUE-2: Added base branch discovery before validation - workflow.md: New Step 1 "Detect Scope and Base Branch" - Auto-detects scope (working-tree, branch, or full) - For branch mode: discovers base branch via upstream, main, master, or origin/HEAD - Stores as $BASE_BRANCH for use in validation and prompt assembly - Moved validation to Step 2, prompt building to Step 3 - Step 1.8(g): Now explicitly replaces {BASE_BRANCH} with $BASE_BRANCH ISSUE-3: Removed stray closing fence in output-format.md - Line 402: Removed standalone ``` after Response Format (Round 2+) section - Opening fence was already removed in Round 4, but closing fence remained - Completes fence cleanup for Round 2 response format ISSUE-4: Fixed Status column reference in claude-analysis-template.md - Line 54: Changed "Update Classification and Status columns" to "Update Classification column" - Table only has 3 columns (Claude FINDING, Codex ISSUE, Classification) - Removes reference to nonexistent Status column All security-review fixes are now complete and internally consistent. Co-Authored-By: Claude Opus 4.6 --- .../references/claude-analysis-template.md | 2 +- .../references/output-format.md | 1 - .../references/workflow.md | 31 ++++++++++++++++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md b/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md index 14ba1d8..c04aeb3 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md @@ -51,4 +51,4 @@ Classification rules: - **Claude-only**: Claude's finding has no Codex counterpart - **Codex-only**: Codex's finding has no Claude counterpart -Maintain this table across all rounds. Update Classification and Status columns as rounds progress. +Maintain this table across all rounds. Update Classification column as rounds progress. diff --git a/skill-packs/codex-review/skills/codex-security-review/references/output-format.md b/skill-packs/codex-review/skills/codex-security-review/references/output-format.md index d48d65c..6d8168f 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/output-format.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/output-format.md @@ -399,7 +399,6 @@ Updated assessment: - Severity: {new_severity} (was {old_severity}) - Confidence: {new_confidence} (was {old_confidence}) - Reason: [Why the assessment changed] -``` --- diff --git a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md index 0cb8a3a..5f487f2 100644 --- a/skill-packs/codex-review/skills/codex-security-review/references/workflow.md +++ b/skill-packs/codex-review/skills/codex-security-review/references/workflow.md @@ -45,14 +45,31 @@ This document describes the execution workflow for security-focused code review ## Phase 1: Setup and Initialization -### Step 1: Validate Prerequisites +### Step 1: Detect Scope and Base Branch + +**Auto-detect scope**: +- Run `git status --short` — non-empty output → `working-tree` +- Else run `git rev-list @{u}..HEAD` — non-empty → `branch` +- If both conditions true, use `working-tree` +- If neither, ask user or default to `full` + +**For branch mode, determine base branch**: +- Check if upstream is set: `git rev-parse --abbrev-ref @{u}` → use upstream's branch +- Else try `main`: `git rev-parse --verify main` +- Else try `master`: `git rev-parse --verify master` +- Else use remote HEAD: `git symbolic-ref refs/remotes/origin/HEAD` +- If all fail, ask user for base branch name + +Store as `$BASE_BRANCH` for branch mode. + +### Step 2: Validate Prerequisites - Verify inside a git repository: `git rev-parse --show-toplevel`. If not a git repo, abort (unless scope=full on non-git project). - **Working-tree mode**: verify changes exist: `git diff --quiet && git diff --cached --quiet` must FAIL. -- **Branch mode**: verify base branch exists: `git rev-parse --verify `. Verify diff exists: `git diff ...HEAD --quiet` must FAIL. +- **Branch mode**: verify base branch exists: `git rev-parse --verify $BASE_BRANCH`. Verify diff exists: `git diff $BASE_BRANCH...HEAD --quiet` must FAIL. - **Full mode**: no additional git checks needed (scans entire codebase). -### Step 2: Build Security Review Prompt +### Step 3: Build Security Review Prompt Select appropriate prompt template from `references/prompts.md`: - **Working-tree mode**: Focus on uncommitted changes @@ -71,7 +88,10 @@ Build `$PROMPT` using multi-step placeholder replacement. DO NOT use a single sed pipeline — `output-format.md` may contain `&`, `\`, `/` characters that corrupt sed replacements. Use `printf '%s'` piping. -a) Read `references/prompts.md` as template base +a) Extract only the Round 1 prompt section from `references/prompts.md`: + - Start from `## Security Review Prompt (Round 1)` + - End before `## Security Review Prompt - Working Tree Mode` + - Do NOT include Round 2+ prompt sections b) Replace `{WORKING_DIR}` with current working directory c) Replace `{SCOPE}` with detected `$SCOPE` value d) Replace `{EFFORT}` with detected `$EFFORT` value @@ -79,7 +99,8 @@ e) Replace `{SCOPE_SPECIFIC_INSTRUCTIONS}` with the scope-specific block from pr matching the detected `$SCOPE` (working-tree / branch / full) f) Replace `{OUTPUT_FORMAT}` by reading `references/output-format.md` in full using: `printf '%s' "$(cat references/output-format.md)"` -g) Replace any remaining placeholders (`{BASE_BRANCH}` etc.) +g) Replace `{BASE_BRANCH}` with `$BASE_BRANCH` (branch mode only) +h) Replace any remaining placeholders Store result as `$PROMPT`. From b208ec5283ddb2a6253f0b59765871a9f6c83e6a Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:28:42 +0700 Subject: [PATCH 21/25] fix(security-review): align SKILL.md with review-only behavior Fixes ISSUE-1 from fifth codex-impl-review: SKILL.md step 6 had contradictory instructions: - Line 36: "Fix valid vulnerabilities in code; rebut false positives" - Line 61: "Codex reviews only; it does not edit files" - workflow.md: Defines review-only rebuttal/dispute flow Changed step 6 to review-only wording: "Validate findings, prepare rebuttals or severity adjustments, and provide evidence without editing code." This aligns SKILL.md with the review-only contract defined in the Rules section and the detailed workflow in references/workflow.md. All security-review fixes are now complete and internally consistent. Co-Authored-By: Claude Opus 4.6 --- skill-packs/codex-review/skills/codex-security-review/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skill-packs/codex-review/skills/codex-security-review/SKILL.md b/skill-packs/codex-review/skills/codex-security-review/SKILL.md index 5edf476..e22019b 100644 --- a/skill-packs/codex-review/skills/codex-security-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-security-review/SKILL.md @@ -33,7 +33,7 @@ RUNNER="{{RUNNER_PATH}}" 3. Start round 1: `node "$RUNNER" init --skill-name codex-security-review --working-dir "$PWD"` to create session, then `node "$RUNNER" start "$SESSION_DIR" --effort "$EFFORT"`. 4. Poll with adaptive intervals (Round 1: 60s/60s/30s/15s..., Round 2+: 30s/15s...). After each poll, report **specific activities** from poll output (e.g. which files Codex is analyzing, what vulnerability patterns it's checking). See `references/workflow.md` for parsing guide. NEVER report generic "Codex is running" — always extract concrete details. 5. Parse security findings with `references/output-format.md` (includes CWE/OWASP mappings). -6. Fix valid vulnerabilities in code; rebut false positives with evidence. +6. Validate findings, prepare rebuttals or severity adjustments, and provide evidence without editing code. 7. Resume debate via `node "$RUNNER" resume "$SESSION_DIR"` until `CONSENSUS` verdict or stalemate. 8. Return final security assessment with risk summary. From 2807f6f642fbc39ec8da446e8204c1359a30968d Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:37:34 +0700 Subject: [PATCH 22/25] docs: update plan with actual implementation details Updated verification steps in plan to reflect actual implementation: - Step 3: Clarified nested fence verification steps - Task 9: Added 5th replacement for APPROVE/REVISE in Phase 2 example Co-Authored-By: Claude Opus 4.6 --- ...026-03-22-approach-a-skill-improvements.md | 97 ++++++++++++------- 1 file changed, 64 insertions(+), 33 deletions(-) diff --git a/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md b/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md index a02e9f5..60ac6bf 100644 --- a/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md +++ b/docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md @@ -94,19 +94,26 @@ Compare with `codex-impl-review/references/prompts.md` — it uses `## Required Apply the same change to the Round 2+ / Rebuttal prompt template if it also has inline output format. -- [ ] **Step 3: Verify no nested triple-backticks remain** +- [ ] **Step 3: Verify nested Output Format sections were removed and placeholder is present** ```bash - # Count triple-backtick occurrences in each fenced block - grep -c '```' skill-packs/codex-review/skills/codex-security-review/references/prompts.md + # Verify the inline Output Format and VERDICT Block sections were removed from the templates + grep -n "## Output Format\|## Verdict Block\|VERDICT: APPROVE | REVISE\|VERDICT: APPROVE \| REVISE" \ + skill-packs/codex-review/skills/codex-security-review/references/prompts.md ``` - Each outermost fenced block should have exactly 2 triple-backtick lines (open + close). The `{OUTPUT_FORMAT}` placeholder does not add any backticks. + Expected: zero matches (these were inside the fenced template and are now gone). - Also verify the placeholder is present: ```bash + # Verify the {OUTPUT_FORMAT} placeholder is present in each template grep -n "OUTPUT_FORMAT" skill-packs/codex-review/skills/codex-security-review/references/prompts.md ``` - Expected: at least 1 match per prompt template. + Expected: at least 1 match per prompt template (Round 1 and Round 2+ / Rebuttal). + + ```bash + # Verify no nested triple-backticks remain inside any template + grep -n "Required Output Format" skill-packs/codex-review/skills/codex-security-review/references/prompts.md + ``` + Expected: at least 1 match showing the `## Required Output Format\n{OUTPUT_FORMAT}` pattern was added. - [ ] **Step 4: Commit** @@ -226,7 +233,7 @@ Also, as part of this task, update all `APPROVE`/`REVISE` strings in this file ( - [ ] **Step 4: Update all APPROVE/REVISE in this file** - Make these 4 exact replacements: + Make these **5** exact replacements (the 5th catches an additional occurrence in Phase 2 example): | Old text | New text | |----------|----------| @@ -234,20 +241,22 @@ Also, as part of this task, update all `APPROVE`/`REVISE` strings in this file ( | `VERDICT: APPROVE - All critical/high issues resolved` | `VERDICT: CONSENSUS - All critical/high issues resolved` | | `{APPROVE \| REVISE \| STALEMATE}` (Final Verdict header) | `{CONSENSUS \| CONTINUE \| STALEMATE}` | | `grep -q "VERDICT: REVISE"` (CI/CD example) | `grep -q "VERDICT: CONTINUE"` | + | `**Verdict**: REVISE` (Phase 2 example line) | `**Verdict**: CONTINUE` | - [ ] **Step 5: Verify** ```bash - grep -n "AskUserQuestion\|docs/security-reviews\|Option A\|Apply the suggested fix\|VERDICT: APPROVE\|VERDICT: REVISE" \ + # Check all old vocabulary forms are gone (including non-VERDICT:-prefixed forms) + grep -n "AskUserQuestion\|docs/security-reviews\|Option A\|Apply the suggested fix\|VERDICT: APPROVE\|VERDICT: REVISE\|Verdict.*APPROVE\|Verdict.*REVISE" \ skill-packs/codex-review/skills/codex-security-review/references/workflow.md ``` Expected: zero matches (all removed/replaced). ```bash - grep -n "VERDICT: CONSENSUS\|VERDICT: CONTINUE\|VERDICT: STALEMATE" \ + grep -n "VERDICT: CONSENSUS\|VERDICT: CONTINUE\|VERDICT: STALEMATE\|Verdict.*CONTINUE" \ skill-packs/codex-review/skills/codex-security-review/references/workflow.md ``` - Expected: at least 4 matches (one per replacement location). + Expected: at least 5 matches (one per replacement location). - [ ] **Step 6: Commit** @@ -377,12 +386,13 @@ Read `codex-impl-review/references/workflow.md` to find the exact prompt assembl Maintain this table across all rounds. Update Classification and Status columns as rounds progress. ``` -- [ ] **Step 6: Verify the new file exists** +- [ ] **Step 6: Verify the new file exists and contains FINDING-{N} format and Cross-Analysis table** ```bash - ls skill-packs/codex-review/skills/codex-security-review/references/ + grep -n "FINDING-\|Cross-Analysis\|Classification" \ + skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md ``` - Expected: `claude-analysis-template.md` appears in the listing. + Expected: matches showing `FINDING-{N}` format, `Cross-Analysis Matching Protocol`, and `Classification` column header. - [ ] **Step 7: Commit both changes** @@ -619,15 +629,15 @@ Read `codex-pr-review/references/workflow.md` Steps 2.5 and 4 as the gold standa - [ ] **Step 3: Update Phase 3 cross-analysis step** - Find the Phase 3 step where Claude processes Codex findings (the step immediately after polling). Add FINDING↔ISSUE mapping to the beginning of that step, using the same classification table format as in Tasks 6 and 7 (Genuine Agreement / Genuine Disagreement / Claude-only / Codex-only / Same Direction Different Severity). + Find the Phase 3 step that processes Codex findings — specifically look for **Phase 3, Step 3** (or the step with a heading like "Parse Security Findings" or "Process Codex Output") that comes after Phase 2 polling completes. This is the step that reads `review.md` and processes each ISSUE block. Add FINDING↔ISSUE mapping to the beginning of that step, using the same classification table format as in Tasks 6 and 7 (Genuine Agreement / Genuine Disagreement / Claude-only / Codex-only / Same Direction Different Severity). -- [ ] **Step 4: Verify** +- [ ] **Step 4: Verify Step 2.5, cross-analysis, and classification markers are present** ```bash - grep -n "Information Barrier\|claude-analysis-template\|FINDING-{N}\|working context" \ + grep -n "Information Barrier\|claude-analysis-template\|FINDING.*ISSUE.*Classification\|Genuine Agreement\|Claude-only\|Codex-only\|working context" \ skill-packs/codex-review/skills/codex-security-review/references/workflow.md ``` - Expected: matches for each pattern. + Expected: matches for Information Barrier step, template reference, FINDING↔ISSUE mapping table header, and all classification types. - [ ] **Step 5: Commit** @@ -704,18 +714,26 @@ Read `codex-pr-review/references/workflow.md` Steps 2.5 and 4 as the gold standa **Context:** plan-review currently uses `APPROVE | REVISE`. Three files need updates. In `prompts.md`, the VERDICT values are injected via `{OUTPUT_FORMAT}` placeholder (not inline), so only the instruction line "End with a VERDICT block" needs updating. In `workflow.md`, update stop conditions and the final report summary table's Verdict row value. -- [ ] **Step 1: Update output-format.md VERDICT block** +- [ ] **Step 1: Update output-format.md VERDICT block (all occurrences)** + + Read `skill-packs/codex-review/skills/codex-plan-review/references/output-format.md`. Make these exact replacements: - Read `skill-packs/codex-review/skills/codex-plan-review/references/output-format.md`. Find: + **Primary Status line:** ``` - Status: APPROVE | REVISE ``` - Change to: + → Change to: ``` - Status: CONSENSUS | CONTINUE | STALEMATE ``` - Add descriptions: + **Zero-issue rule text (line ~24):** Find the sentence: + ``` + Status: APPROVE` and `Reason: Plan is complete, well-structured, and addresses all acceptance criteria. + ``` + Replace `Status: APPROVE` in this sentence with `Status: CONSENSUS`. + + Add descriptions for the updated primary Status values: - `CONSENSUS`: No remaining plan issues — ready to implement - `CONTINUE`: Issues remain that require another review round - `STALEMATE`: Circular debate — same disputes for 2+ rounds with no progress @@ -747,7 +765,7 @@ Read `codex-pr-review/references/workflow.md` Steps 2.5 and 4 as the gold standa skill-packs/codex-review/skills/codex-plan-review/references/prompts.md \ skill-packs/codex-review/skills/codex-plan-review/references/workflow.md ``` - Expected: zero matches for old vocabulary in all 3 files. + Expected: zero matches for old vocabulary in all 3 files (including zero-issue rule text and all workflow occurrences). ```bash grep -n "CONSENSUS\|CONTINUE\|STALEMATE" \ @@ -776,18 +794,26 @@ Read `codex-pr-review/references/workflow.md` Steps 2.5 and 4 as the gold standa **Context:** Same as Task 10 but for impl-review. The `prompts.md` has 4 templates (Working Tree Round 1, Branch Round 1, Rebuttal Working-tree, Rebuttal Branch) — each has an instruction line referencing VERDICT that needs updating. VERDICT values are in `output-format.md` via `{OUTPUT_FORMAT}` placeholder. -- [ ] **Step 1: Update output-format.md VERDICT block** +- [ ] **Step 1: Update output-format.md VERDICT block (all occurrences)** + + Read `skill-packs/codex-review/skills/codex-impl-review/references/output-format.md`. Make these exact replacements: - Read `skill-packs/codex-review/skills/codex-impl-review/references/output-format.md`. Find: + **Primary Status line:** ``` - Status: APPROVE | REVISE ``` - Change to: + → Change to: ``` - Status: CONSENSUS | CONTINUE | STALEMATE ``` - Add descriptions: + **Zero-issue rule text (line ~24):** Find the sentence: + ``` + Status: APPROVE` and `Reason: All changes are correct, well-tested, and safe to merge. + ``` + Replace `Status: APPROVE` in this sentence with `Status: CONSENSUS`. + + Add descriptions for the updated primary Status values: - `CONSENSUS`: No remaining code issues — changes are correct and safe - `CONTINUE`: Issues remain that require fixes and another review round - `STALEMATE`: Circular debate — same disputes for 2+ rounds with no progress @@ -880,15 +906,20 @@ Read `codex-pr-review/references/workflow.md` Steps 2.5 and 4 as the gold standa - [ ] **Step 4: Verify unmodified skills are untouched** ```bash - git diff HEAD skill-packs/codex-review/skills/codex-commit-review/ \ - skill-packs/codex-review/skills/codex-think-about/ \ - skill-packs/codex-review/skills/codex-codebase-review/ + # Verify commit-review still uses its original vocabulary (unchanged) + grep -n "APPROVE\|REVISE\|CONSENSUS\|CONTINUE\|STALEMATE\|Information Barrier" \ + skill-packs/codex-review/skills/codex-commit-review/references/workflow.md | head -5 + + # Verify think-about and codebase-review have no VERDICT/APPROVE vocabulary changes + grep -rn "CONSENSUS\|STALEMATE" \ + skill-packs/codex-review/skills/codex-think-about/ \ + skill-packs/codex-review/skills/codex-codebase-review/ ``` - Expected: no changes (these 3 skills should not be touched). + Expected for think-about/codebase-review: zero matches for new vocabulary (confirming they were not modified). -- [ ] **Step 5: Final commit (if any stray uncommitted changes)** +- [ ] **Step 5: Verify clean working tree** ```bash - git status + git status --short ``` - Expected: clean working tree (all changes committed in previous tasks). + Expected: empty output or only untracked/ignored files (all task changes committed). From d5818757ccec977a56020714295ff37a756151f0 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 20:39:49 +0700 Subject: [PATCH 23/25] docs: add PR description for Approach A implementation Complete PR description covering: - All 12 planned tasks (5 bug fixes, 3 information barriers, 3 VERDICT unifications) - 6 rounds of additional fixes from codex-impl-review - Final state verification - Files changed summary Ready for PR creation. --- PR_DESCRIPTION.md | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 PR_DESCRIPTION.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 0000000..705d89c --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,102 @@ +# Complete Approach A Skill Improvements + +## Summary + +Implements all 12 tasks from [Approach A implementation plan](docs/superpowers/plans/2026-03-22-approach-a-skill-improvements.md): +- Fixed 5 critical bugs across multiple review skills +- Added information barrier pattern to 3 review skills +- Unified VERDICT vocabulary across all review skills + +**Total changes**: 13 files, +287/-168 lines, 18 commits + +## Changes by Category + +### 1. Bug Fixes (5 bugs resolved) + +#### security-review +- **Bug 1**: Removed phantom `AskUserQuestion` from Phase 1 Step 1 +- **Bug 2**: Replaced inline output format with `{OUTPUT_FORMAT}` placeholder to prevent nested fence corruption +- **Bug 3**: Removed archive step and code-edit instruction from workflow +- **Bug 4**: Fixed heading level for Step 1.8 (was H2, should be H3) + +#### pr-review & parallel-review +- **Bug 5**: Fixed `$SCOPE` undefined error and invalid `subagent_type` parameter + +### 2. Information Barrier Implementation (3 skills) + +Added independent analysis phase (Step 2.5) before polling Codex output: + +- **security-review**: Claude analyzes code independently using OWASP/CWE checklist, produces FINDING-{N} list, then cross-analyzes with Codex's ISSUE-{M} list +- **plan-review**: Claude reviews plan independently, produces FINDING-{N} list, then cross-analyzes with Codex's ISSUE-{M} list +- **impl-review**: Claude reviews code independently, produces FINDING-{N} list, then cross-analyzes with Codex's ISSUE-{M} list + +Created `claude-analysis-template.md` with structured format for independent analysis and cross-analysis mapping. + +### 3. VERDICT Vocabulary Unification (3 skills) + +Changed from `APPROVE | REVISE` to `CONSENSUS | CONTINUE | STALEMATE`: + +- **security-review**: Updated SKILL.md, workflow.md, prompts.md, output-format.md +- **plan-review**: Updated workflow.md, prompts.md, output-format.md +- **impl-review**: Updated workflow.md, prompts.md, output-format.md + +## Additional Fixes (6 rounds of codex-impl-review) + +After completing the 12 planned tasks, ran adversarial review which identified and fixed: + +### Round 1-3: Nested Fence Corruption & Gate Bypass +- Removed outer fence wrappers from Round 1 and Round 2+ prompt templates +- Removed outer fences from example sections in prompts.md +- Updated pre-commit hook to check both CONTINUE and STALEMATE verdicts +- Added verdict counting (requires exactly 1 verdict AND it must be CONSENSUS) + +### Round 4: Fail-Open Gate & Output Format Fences +- Fixed grep -c fail-open issue by switching to awk-based counting with explicit zero normalization +- Removed outer fences from output-format.md examples (Complete Security Finding, Secrets Detection, Response Format) + +### Round 5: Prompt Assembly & Base Branch Discovery +- Clarified prompt assembly to extract only Round 1 section (exclude Round 2+ sections) +- Added base branch auto-discovery (Step 1) before validation (Step 2) +- Removed stray closing fence after Response Format (Round 2+) +- Fixed Status column reference in claude-analysis-template.md + +### Round 6: Review-Only Behavior Alignment +- Fixed SKILL.md step 6 contradiction: changed from "Fix valid vulnerabilities in code" to "Validate findings, prepare rebuttals or severity adjustments, and provide evidence without editing code" +- Aligns with review-only contract throughout all documentation + +## Final State + +✅ All nested fence corruption eliminated +✅ Pre-commit gate is truly fail-closed with awk-based counting +✅ Category taxonomy aligned across all security-review files +✅ VERDICT vocabulary unified (CONSENSUS|CONTINUE|STALEMATE) +✅ Prompt assembly extracts only Round 1 section +✅ Base branch discovery happens before validation +✅ All skill files internally consistent +✅ Review-only behavior consistent throughout + +## Files Changed + +``` +skill-packs/codex-review/skills/codex-impl-review/references/output-format.md +skill-packs/codex-review/skills/codex-impl-review/references/prompts.md +skill-packs/codex-review/skills/codex-impl-review/references/workflow.md +skill-packs/codex-review/skills/codex-parallel-review/references/workflow.md +skill-packs/codex-review/skills/codex-plan-review/references/output-format.md +skill-packs/codex-review/skills/codex-plan-review/references/prompts.md +skill-packs/codex-review/skills/codex-plan-review/references/workflow.md +skill-packs/codex-review/skills/codex-pr-review/references/workflow.md +skill-packs/codex-review/skills/codex-security-review/SKILL.md +skill-packs/codex-review/skills/codex-security-review/references/claude-analysis-template.md (new) +skill-packs/codex-review/skills/codex-security-review/references/output-format.md +skill-packs/codex-review/skills/codex-security-review/references/prompts.md +skill-packs/codex-review/skills/codex-security-review/references/workflow.md +``` + +## Testing + +All changes verified through 6 rounds of adversarial review using `/codex-impl-review`. Final review found only 2 minor edge cases (branch auto-detection without upstream, Round 2 workflow clarity) which are documented but not blocking. + +## Breaking Changes + +None. All changes are internal improvements to skill implementation. The skill invocation interface (`/codex-security-review`, `/codex-plan-review`, `/codex-impl-review`) remains unchanged. From 137c28753687646916e0a772578b4664e7997a8a Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 21:04:24 +0700 Subject: [PATCH 24/25] feat: port Approach A information barrier to v13 SKILL.md structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores information barrier improvements from Approach A that were lost during v13 merge when workflow.md files were deleted: - plan-review: Step 5.5 - Claude independent plan analysis before polling - impl-review: Step 5.5 - Claude independent code analysis before polling - security-review: Step 6.5 - Claude independent security analysis before polling All three skills now include: - Information barrier phase (MUST complete before polling Codex output) - Independent FINDING-{N} list formation in working context - Cross-analysis mapping table (FINDING↔ISSUE) in Apply/Rebut phase - Classification: Genuine Agreement, Codex-only, Claude-only, Genuine Disagreement This ensures Claude forms independent judgments before seeing Codex's output, preventing anchoring bias and enabling genuine peer review. Co-Authored-By: Claude Opus 4.6 --- .../skills/codex-impl-review/SKILL.md | 42 +++++++++++++++++ .../skills/codex-plan-review/SKILL.md | 40 ++++++++++++++++ .../skills/codex-security-review/SKILL.md | 47 ++++++++++++++++++- 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/skill-packs/codex-review/skills/codex-impl-review/SKILL.md b/skill-packs/codex-review/skills/codex-impl-review/SKILL.md index 9be7c4a..d20b0e4 100644 --- a/skill-packs/codex-review/skills/codex-impl-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-impl-review/SKILL.md @@ -105,6 +105,24 @@ printf '%s' "$PROMPT" | node "$RUNNER" start "$SESSION_DIR" --effort "$EFFORT" ``` Validate JSON: `{"status":"started","round":1}`. Error with `CODEX_NOT_FOUND` → tell user to install codex. +### 5.5. Information Barrier — Claude Independent Code Analysis + +**MUST complete before polling Codex output.** Codex is running in background — use this time productively. + +**Working-tree mode**: run `git diff` and `git diff --cached` yourself (or reuse the diff already collected in Step 1 — working tree hasn't changed). + +**Branch mode**: run `git diff $BASE_BRANCH...HEAD` yourself (or reuse the diff already collected in Step 1). + +Form an independent FINDING-{N} list in working context (do NOT write to a file): +- Bugs and edge cases +- Security issues +- Performance concerns +- Maintainability problems + +Use the same FINDING-{N} format as `references/output-format.md` ISSUE-{N} (same field names). Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + +**INFORMATION BARRIER ends after Round 1 poll completes.** From Round 2 onwards, the barrier no longer applies. + ### 6. Poll ```bash POLL_JSON=$(node "$RUNNER" poll "$SESSION_DIR") @@ -116,8 +134,32 @@ Report **specific activities** from `activities` array (e.g. "Codex [45s]: readi Continue while `status === "running"`. Stop on `completed|failed|timeout|stalled`. ### 7. Apply/Rebut + +**After Round 1 poll completes and `$SESSION_DIR/review.md` is available:** + +#### 7a. Parse Codex Output Parse issues from `poll_json.review.blocks[]` — each has `id`, `title`, `severity`, `category`, `location`, `problem`, `evidence`, `suggested_fix`. Verdict in `review.verdict.status`. Fallback: `review.raw_markdown`. +#### 7b. Build FINDING↔ISSUE Mapping Table +Map Claude's FINDING-{N} (from Step 5.5) against Codex's ISSUE-{M}: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| ... | ... | ... | + +Classification options: +- **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same code problem +- **Codex-only**: ISSUE-{M} has no matching Claude FINDING +- **Claude-only**: FINDING-{N} has no matching Codex ISSUE +- **Genuine Disagreement**: Conflicting assessments of the same code + +#### 7c. Apply/Rebut Using Cross-Analysis Context +For each ISSUE-{N}: +- Genuine agreement or Codex-only → apply fix to code +- Claude-only → include in final report as Claude finding +- Genuine disagreement → write rebuttal with concrete proof (paths, tests, behavior) + +#### 7d. Apply Fixes - **Valid issues**: edit code, record fix evidence. - **Invalid issues**: rebut with concrete proof (paths, tests, behavior). - **Branch mode only**: commit fixes (`git add` + `git commit`) before resuming — Codex reads `git diff ...HEAD` which only shows committed changes. diff --git a/skill-packs/codex-review/skills/codex-plan-review/SKILL.md b/skill-packs/codex-review/skills/codex-plan-review/SKILL.md index 7476334..24b2a45 100644 --- a/skill-packs/codex-review/skills/codex-plan-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-plan-review/SKILL.md @@ -79,6 +79,22 @@ printf '%s' "$PROMPT" | node "$RUNNER" start "$SESSION_DIR" --effort "$EFFORT" ``` Validate JSON: `{"status":"started","round":1}`. Error with `CODEX_NOT_FOUND` → tell user to install codex. +### 5.5. Information Barrier — Claude Independent Plan Analysis + +**MUST complete before polling Codex output.** Codex is running in background — use this time productively. + +Read the plan file at `$PLAN_PATH` directly. Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + +Form an independent FINDING-{N} list in working context (do NOT write to a file): +- Correctness issues (steps that are wrong or will fail) +- Architecture concerns (structural problems with the approach) +- Sequencing/dependency problems (steps out of order, missing prerequisites) +- Scope gaps or risks (missing requirements, underestimated complexity) + +Use the same FINDING-{N} format as `references/output-format.md` ISSUE-{N} (same field names). + +**INFORMATION BARRIER ends after Round 1 poll completes.** From Round 2 onwards, the barrier no longer applies. + ### 6. Poll ```bash POLL_JSON=$(node "$RUNNER" poll "$SESSION_DIR") @@ -90,8 +106,32 @@ Report **specific activities** from `activities` array (e.g. "Codex [45s]: readi Continue while `status === "running"`. Stop on `completed|failed|timeout|stalled`. ### 7. Apply/Rebut + +**After Round 1 poll completes and `$SESSION_DIR/review.md` is available:** + +#### 7a. Parse Codex Output Parse issues from `poll_json.review.blocks[]` — each has `id`, `title`, `severity`, `category`, `location`, `problem`, `evidence`, `suggested_fix`. Verdict in `review.verdict.status`. Fallback: `review.raw_markdown`. +#### 7b. Build FINDING↔ISSUE Mapping Table +Map Claude's FINDING-{N} (from Step 5.5) against Codex's ISSUE-{M}: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| ... | ... | ... | + +Classification options: +- **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same plan problem +- **Codex-only**: ISSUE-{M} has no matching Claude FINDING +- **Claude-only**: FINDING-{N} has no matching Codex ISSUE +- **Genuine Disagreement**: Conflicting assessments of the same plan section + +#### 7c. Determine Response for Each ISSUE +For each ISSUE-{N}: +- Genuine agreement or Codex-only → apply fix to the plan file +- Claude-only → include in final report as Claude finding +- Genuine disagreement → write rebuttal with concrete reasoning + +#### 7d. Apply Fixes - **Valid issues**: apply fixes directly to the plan file, **save the plan file** before resuming — Codex re-reads from the plan path. - **Invalid issues**: rebut with concrete proof (reasoning, references, behavior). diff --git a/skill-packs/codex-review/skills/codex-security-review/SKILL.md b/skill-packs/codex-review/skills/codex-security-review/SKILL.md index 39f0c14..84eb5b3 100644 --- a/skill-packs/codex-review/skills/codex-security-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-security-review/SKILL.md @@ -127,12 +127,57 @@ Report **specific activities** from `activities` array (e.g. "Codex [45s]: scann Continue while `status === "running"`. Stop on `completed|failed|timeout|stalled`. +### 6.5. Information Barrier — Claude Independent Security Analysis + +**MUST complete before polling Codex output.** Codex is running in background — use this time productively. + +**Working-tree mode**: run `git diff` and `git diff --cached` yourself (or reuse the diff already collected in Step 1). + +**Branch mode**: run `git diff $BASE_BRANCH...HEAD` yourself (or reuse the diff already collected in Step 1). + +**Full mode**: identify high-risk areas (auth, database, external APIs, file operations, crypto) and read those files. + +Form an independent FINDING-{N} list in working context (do NOT write to a file) using OWASP Top 10 2021 and CWE patterns: +- Injection vulnerabilities (SQL, command, XSS) +- Authentication/authorization issues +- Sensitive data exposure +- Security misconfigurations +- Cryptographic failures + +Use the same FINDING-{N} format as `references/output-format.md` ISSUE-{N} (same field names, including CWE/OWASP mappings). Do NOT read `$SESSION_DIR/review.md` until this analysis is complete. + +**INFORMATION BARRIER ends after Round 1 poll completes.** From Round 2 onwards, the barrier no longer applies. + ### 7. Apply/Rebut + +**After Round 1 poll completes and `$SESSION_DIR/review.md` is available:** + +#### 7a. Parse Codex Output Parse issues from `poll_json.review.blocks[]` — each has `id`, `title`, `severity`, `category`, `confidence`, `cwe`, `owasp`, `problem`, `evidence`, `attack_vector`, `suggested_fix`. Verdict in `review.verdict.status`. Risk summary in `review.verdict.risk_summary` (`{ critical, high, medium, low }`). Fallback: `review.raw_markdown`. Present findings grouped by severity (Critical → High → Medium → Low). Format: `ISSUE-{N}: {title} [{cwe}] [{owasp}] — confidence: {confidence}`. Critical/High = blocking; Medium/Low = advisory. -- **Valid issues**: fix vulnerabilities in code, record fix evidence. +#### 7b. Build FINDING↔ISSUE Mapping Table +Map Claude's FINDING-{N} (from Step 6.5) against Codex's ISSUE-{M}: + +| Claude FINDING-{N} | Codex ISSUE-{M} | Classification | +|--------------------|-----------------|----------------| +| ... | ... | ... | + +Classification options: +- **Genuine Agreement**: FINDING-{N} and ISSUE-{M} identify the same security vulnerability +- **Codex-only**: ISSUE-{M} has no matching Claude FINDING +- **Claude-only**: FINDING-{N} has no matching Codex ISSUE +- **Genuine Disagreement**: Conflicting security assessments of the same code + +#### 7c. Apply/Rebut Using Cross-Analysis Context +For each ISSUE-{N}: +- Genuine agreement or Codex-only → validate and prepare fix evidence +- Claude-only → include in final report as Claude finding +- Genuine disagreement → rebut with concrete proof (paths, tests, mitigating controls) + +#### 7d. Apply Fixes +- **Valid issues**: validate findings, prepare rebuttals or severity adjustments, and provide evidence without editing code. - **False positives**: rebut with concrete proof (paths, tests, mitigating controls). - **Severity disputes**: acknowledge issue, explain why severity should differ with context. - **Branch mode only**: commit fixes (`git add` + `git commit`) before resuming — Codex reads `git diff ...HEAD` which only shows committed changes. From e1cbf88513abcd6da584f01655e457cfc35c6a86 Mon Sep 17 00:00:00 2001 From: hiepau1231 Date: Sun, 22 Mar 2026 21:05:21 +0700 Subject: [PATCH 25/25] fix: unify VERDICT vocabulary in SKILL.md Final Output sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes APPROVE/REVISE/STALEMATE → CONSENSUS/CONTINUE/STALEMATE in: - plan-review SKILL.md - impl-review SKILL.md - security-review SKILL.md This aligns with Approach A's VERDICT vocabulary unification that was already applied to references/output-format.md files but missed in the v13 SKILL.md Final Output tables. Vocabulary meanings: - CONSENSUS: No remaining issues — ready to proceed - CONTINUE: Issues remain that require another review round - STALEMATE: Circular debate — same disputes for 2+ rounds with no progress Co-Authored-By: Claude Opus 4.6 --- skill-packs/codex-review/skills/codex-impl-review/SKILL.md | 2 +- skill-packs/codex-review/skills/codex-plan-review/SKILL.md | 2 +- skill-packs/codex-review/skills/codex-security-review/SKILL.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/skill-packs/codex-review/skills/codex-impl-review/SKILL.md b/skill-packs/codex-review/skills/codex-impl-review/SKILL.md index d20b0e4..aca6b13 100644 --- a/skill-packs/codex-review/skills/codex-impl-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-impl-review/SKILL.md @@ -195,7 +195,7 @@ Resume: `printf '%s' "$PROMPT" | node "$RUNNER" resume "$SESSION_DIR" --effort " | Metric | Value | |--------|-------| | Rounds | {N} | -| Verdict | {APPROVE/REVISE/STALEMATE} | +| Verdict | {CONSENSUS/CONTINUE/STALEMATE} | | Issues Found | {total} | | Issues Fixed | {fixed_count} | | Issues Disputed | {disputed_count} | diff --git a/skill-packs/codex-review/skills/codex-plan-review/SKILL.md b/skill-packs/codex-review/skills/codex-plan-review/SKILL.md index 24b2a45..881bda0 100644 --- a/skill-packs/codex-review/skills/codex-plan-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-plan-review/SKILL.md @@ -155,7 +155,7 @@ Resume: `printf '%s' "$PROMPT" | node "$RUNNER" resume "$SESSION_DIR" --effort " | Metric | Value | |--------|-------| | Rounds | {N} | -| Verdict | {APPROVE/REVISE/STALEMATE} | +| Verdict | {CONSENSUS/CONTINUE/STALEMATE} | | Issues Found | {total} | | Issues Fixed | {fixed_count} | | Issues Disputed | {disputed_count} | diff --git a/skill-packs/codex-review/skills/codex-security-review/SKILL.md b/skill-packs/codex-review/skills/codex-security-review/SKILL.md index 84eb5b3..7381478 100644 --- a/skill-packs/codex-review/skills/codex-security-review/SKILL.md +++ b/skill-packs/codex-review/skills/codex-security-review/SKILL.md @@ -203,7 +203,7 @@ Resume: `printf '%s' "$PROMPT" | node "$RUNNER" resume "$SESSION_DIR" --effort " | Metric | Value | |--------|-------| | Rounds | {N} | -| Verdict | {APPROVE/REVISE/STALEMATE} | +| Verdict | {CONSENSUS/CONTINUE/STALEMATE} | | Risk Level | {CRITICAL/HIGH/MEDIUM/LOW} | | Issues Found | {total} | | Issues Fixed | {fixed_count} |