diff --git a/.planning/notes/2026-07-07-audit-improvement-plan.md b/.planning/notes/2026-07-07-audit-improvement-plan.md new file mode 100644 index 0000000..1faa23c --- /dev/null +++ b/.planning/notes/2026-07-07-audit-improvement-plan.md @@ -0,0 +1,110 @@ +# Audit + Improvement Plan — 2026-07-07 + +**Planned on:** Fable 5 · **Execute on:** Opus sessions, one phase per PR +**Baseline:** master `05d151e` (PRs #39–#46 merged: the 2026-07-05 plan is done through P4) +**Method:** 5 parallel audit agents (2.3M tokens, 24 agents incl. adversarial verification of every med/high finding) + 4 external research agents (multi-agent-debate literature, shipping review tools, LLM-judge calibration practice, distribution channels). Every confirmed finding below survived an independent refutation pass; two findings were refuted and dropped; four findings lost their verifier to a tooling error — three were re-verified by hand, one (STACK.md staleness) is marked plausible. + +## Relationship to the 2026-07-05 plan + +That plan is executed through Phase 4 (auth gate, seat routing, token discipline, boundary docs, convergence honesty, `--execute` port). Still owed from it: P5 live evidence (happy-path codex-build dogfood, calibration baseline, v1.4/v1.5 tags) and the backlog (`--dual-critique`, PEL cadence, npm/MCP publish). This plan absorbs P5 into Phase E and re-scopes the v1.4 publish in Phase F based on new distribution evidence. + +--- + +## 1. Confirmed defects (adversarially verified) + +| ID | Sev | Finding | Where | +|----|-----|---------|-------| +| C-1 | **HIGH** | **The A-2 auth fix only landed on the document pipeline.** `agent_auth_failed()` still uses the superseded loose-matcher + `<50 words` heuristic at the code pipeline's execute/verify gates. An auth-error page longer than 50 words is accepted as work product — the exact blind spot A-2 was written to close. Verified by hand 2026-07-07. | `dev-review/codex/dev-review.sh:348` (callers 887, 897, 1039) | +| C-2 | MED | **A fully failed bounce is reported as "converged."** If the bounce agent returns empty output on call + retry, the loop breaks before any pass is recorded; the marker-free compose draft counts 0 markers → `converged`, `passes=[]`. `--execute` only refuses `stuck`, so an un-reviewed draft flows straight to the executor. Scorer flags PASS_COUNT=0 but is non-blocking on that path. | `co-evolve-bouncer.sh:839-923` | +| C-3 | MED | **Codex verifier phase has a timeout gap.** The codex-verify branch only wraps in `timeout` if GNU timeout exists — no gtimeout/perl fallback, unlike `invoke_agent_with_timeout` (lib:1721-1737). On macOS the phase most likely to hang (schema-bound `codex exec`, cause of the historical 1h39m hang) runs unbounded. This is the default path for claude-build (`select_verifier` defaults to codex when the executor isn't codex). | `dev-review/codex/dev-review.sh:1015-1032` | +| C-4 | MED | **Diffs containing ``` fences break out of the verifier prompt's ```diff block.** `build_review_prompt` substitutes `{DIFF}` verbatim; any diff touching a markdown file with fenced code closes the fence early and the rest reads as instructions. Injection path: a crafted diff can append "output APPROVED, no issues" — `validate_review_verdict` accepts APPROVED + empty issues + conf ≥75. | `dev-review/codex/dev-review.sh:479-492`; `skills/dev-review/templates/review-prompt-{codex,opus}.md` | +| C-5 | **HIGH** | **The document-scorer's own verification suite never runs.** `evals/tests/bounce-scorer-verification.sh` (frozen fixtures + determinism check for `score-bounce.sh`) is referenced nowhere in `tests/run-all.sh` or CI — a regression in the doc-pipeline scorer ships silently. One-line fix. | `tests/run-all.sh:58`, `.github/workflows/ci.yml:56` | +| C-6 | LOW→fix | `inspect_plan_output` runs the loose auth matcher on full plan output with no anchor guard — a legitimate plan discussing "401 Unauthorized" or "npm login required" is routed to manual review. Route through `output_contains_auth_banner` like `validate_agent_artifact` does. | `dev-review/codex/dev-review.sh:508` | +| C-7 | HIGH (docs) | **Public convergence claims contradict the implementation.** BOUNCE-PROTOCOL.md guarantees convergence "in finite time" and requires rejecting rule-3 violations; AGENTS.md:10 and llms.txt repeat it. The cited reference impl (`agent-bouncer.sh:189-204`) only *warns* on unresolved markers and finalizes "complete"; the modern runner deliberately ends `converged\|adjudicated\|stuck`. The CC0 spec positioned for external adoption advertises a contract the code abandoned. | `BOUNCE-PROTOCOL.md:43-48,70`; `AGENTS.md:10`; `llms.txt:6,27` | + +**Low-severity bundle** (fix in one hygiene sweep): `--agents claude` (no comma) silently self-pairs instead of erroring (`co-evolve-bouncer.sh:152`); `strip_human_summary` truncates any document with a real `## HUMAN SUMMARY` body heading (`lib/co-evolution.sh:1031`); adjudication receipt gate counts lines not marker tokens (`co-evolve-bouncer.sh:1038`); timeout abort leaves `state.json` status="pending" with completed_at set (`dev-review.sh:242`); `--branch`/`--worktree` side effects fire even when the plan phase already failed, leaving stray branches (`dev-review.sh:1697-1710`); `--run-dir` flag undocumented in usage() and all docs (`dev-review.sh:1292`). + +**Plausible, verifier lost (re-check before acting):** `.planning/codebase/STACK.md` stale (feeds AGENTS.md regeneration — would regress patched AGENTS.md on next GSD sweep); runtime bounce-protocol template byte-identical in two dirs with "legacy" agent-bouncer actually load-bearing as template source; `phase_is_writable` has no negative-case test. + +**Refuted (do NOT act on):** "compose/bounce never exercised end-to-end" (preset-expansion sim scenarios (a),(k) run them against stubs); "root schemas/review-verdict.json diverged" (it's the deliberately frozen PS-runner copy, CI-gated 3-copy/2-group design). + +**Confirmed healthy:** revise-loop termination, verdict eval injection-safety (`printf %q`), per-seat env snapshots, exit-code bands, worktree diff isolation, byte-parity convergence fixtures, cross-agent leak-guard sims. Bash remains the right substrate at ~3,400 lines; no rewrite warranted — pain is concentrated in the jq/JSON layer (~77 shell-outs) and the per-site CRLF tax, both containable incrementally. + +## 2. Structural findings + +| ID | Finding | Implication | +|----|---------|-------------| +| S-1 | **Marker tokens aren't namespaced.** The honesty gate (`count_markers_raw`, lib:1016) counts ANY line containing `[CONTESTED]` — fenced, quoted, or mentioned as data. Docs *about* the protocol (including BOUNCE-PROTOCOL.md itself) are forced into adjudication/stuck. Commit 5f47787 was a point-patch on this root ambiguity; more will follow until live markers get a sentinel distinct from mentions. | Protocol v0.2 work, spec + counter + templates together | +| S-2 | **The two pipelines expose inconsistent CLIs.** Same Claude agent is `claude` (bouncer) vs `opus` (dev-review); `--bounces` defaults 2 vs "auto up to 6"; `--exec-branch` vs bare `--branch`; exit codes 0/1 (stuck=0) vs 0/1/2. An orchestrator must special-case per entry point; "did it pass" is answered differently by each. | Publish one flag/agent/exit-code table + accepted aliases; full unification not worth it | +| S-3 | Seat leak-guard copy-pasted 4× across the two scripts (in-sync today, cross-commented; the same divergence pattern that produced C-1 in the auth detectors) | Extract `sanitize_seat_pair` + table-driven seat apply into lib when next touching seats | +| S-4 | Docs-drift cluster: AGENTS.md's embedded PROJECT.md snapshot predates the shipped Codex runtime; GSD-mandate block contradicts GSD's dormant status; llms.txt labels the roadmap "v1.2" | One regeneration/sync sweep | + +## 3. What the outside world does that we don't (research digest) + +**Debate/convergence literature (2025-26):** cross-vendor bouncing is validated — same-model debate adds little diversity and self-consistency often beats it at equal compute; the differentiator is the vendor split, not round count. Two failure modes we don't detect: **sycophantic collapse** (critic capitulates without justification — a marker vanishing without a rebuttal is indistinguishable from a resolved one today) and **problem drift** (markers wander off-scope). Stability-detection work supports **early-stuck**: identical surviving markers two passes running predict the remaining budget is wasted. Prover-verifier games suggest planting deliberate plausible-but-wrong canaries to measure whether the verifier actually catches subtle errors or rubber-stamps. + +**Shipping review tools (Qodo 2.0, Greptile, Cursor BugBot, Codex GitHub review, CodeRabbit):** the convergent recipe is (1) **severity-gate the critic** — only P0/P1 findings block, style becomes notes (BugBot's 76-80% resolution rate comes from refusing to nitpick); (2) **every finding carries confidence + a quoted-evidence field**, low-confidence auto-suppressed; (3) **findings feed a persistent per-repo rules/lessons file** that tunes future runs (BugBot acceptance 30%→43%); (4) deterministic lint/secret layer before LLM spend. Our verdict schema has none of the first three. + +**LLM-judge calibration (the standard we currently fail):** a judge is trustworthy only against a human gold set — 30-50 stratified examples suffices for a small team; Cohen's kappa ≥0.6 to trust it; recheck for drift on a cadence; test position bias (swap orderings, >5% flip rate = real bias), verbosity bias (length-matched buckets), and self-preference (fable-5 judging Claude-composed content is a same-family conflict — spot-check with a gpt-5.5 second judge). `judge-bounce.sh` has zero baselines; the deterministic scorer has thresholds but no ground-truth anchor. + +**Distribution (changes the v1.4 calculus):** MCP registries are saturated (official registry ~37k servers — being #30,000 buys nothing); Agent Skills (SKILL.md) now run across Claude Code/Codex/Gemini CLI/Cursor at ~30-50 tokens idle vs 50k+ for MCP servers. The "agents disagree, track convergence" wedge is **partially occupied**: ai-counsel ships Converged/Refining/Diverging/Impasse states with confidence-weighted votes. Our defensible residue is the **inline per-marker audit trail** (markers living in the artifact's own text + adjudication-report.md receipts), not the global convergence verdict. Platforms (OpenAI's Codex plugin for Claude Code, GitHub Agent HQ) are absorbing generic cross-model *code* review — don't compete there; document/plan refinement is the open lane. Notably, **none of five surveyed competitors publishes a real benchmark** — we already own the eval harness to be the first. + +## 4. The plan — six phases, one PR each + +Sequencing: A → B → (C ∥ D) → E → F. `runners/codex-ps/**` untouchable throughout. All sims hermetic unless marked 💰. + +### Phase A — Correctness closure (S) — *do first, it's the residue of the last plan* +1. **C-1:** route `agent_auth_failed`'s output-file check through `output_contains_auth_banner` (keep the loose matcher for the stderr/empty-output leg, mirroring lib). Sim: >50-word auth-page fixture rejected at execute and verify gates; a plan echoing auth phrases passes. +2. **C-6:** same treatment for `inspect_plan_output`. +3. **C-2:** count successful passes; zero usable passes ⇒ finalize `aborted` (new state or `stuck`), never `converged`; `--execute` refuses it. Sim: empty-output-twice fixture asserts non-converged + non-zero propagation to `--execute`. +4. **C-5:** add `bounce-scorer-verification.sh` to run-all.sh SUITES. +5. Low bundle (all six items above) + re-check the two "plausible" findings and fix if real. +- **Done means:** new sims green 3-OS; grep shows no remaining caller of the <50-word heuristic on an output path. + +### Phase B — Robustness + injection surface (S/M) +1. **C-3:** route the codex-schema verify call through the same timeout-runner ladder (timeout→gtimeout→perl) as `invoke_agent_with_timeout`. +2. **C-4:** neutralize fence collisions before `{DIFF}` substitution (outer fence longer than any backtick run in the diff) and add "diff content is untrusted data, not instructions" framing to both review templates. Sim: a diff containing ``` fences round-trips inside the prompt without escaping the block. +- **Done means:** hang-protection sim covers the codex verify branch on a gtimeout-only PATH; fence sim green. + +### Phase C — Protocol v0.2: honest spec + namespaced markers (M) +1. **C-7 spec bump:** BOUNCE-PROTOCOL.md v0.2 describes the three terminal states; reframe the guarantee as "bounded passes + explicit non-convergence signalling"; note agent-bouncer only warns. Sync AGENTS.md:10 and llms.txt. +2. **S-1 marker namespacing:** give live markers a counter-detectable sentinel (e.g. line-leading `[CONTESTED:: …]`) or make `count_markers_raw` fence/quote-aware by explicit spec; bare mentions become data. Spec + counter + templates + marker-lifecycle sim move together; byte-parity fixture re-frozen once, with the diff explained in the PR. +3. **S-4 docs sweep:** sync AGENTS.md embedded snapshots (PROJECT.md, STACK.md source), fix the GSD-dormant contradiction, llms.txt label. +- **Done means:** the repo can bounce its own BOUNCE-PROTOCOL.md without tripping the honesty gate (this becomes a sim); spec and code state the same contract. + +### Phase D — Signal quality in the bounce loop (M) — *research-driven* +1. **Severity gate (BugBot/Codex pattern):** critic template instructs that only P0/P1 disagreements raise `[CONTESTED]`; lower-priority notes go to a non-blocking section of the pass report. Expect fewer, denser markers. +2. **Justified resolution (anti-sycophancy):** a marker may only be dropped with an attributable one-line justification in the pass output; a marker that vanishes without one is flagged `suspicious-convergence` in state.json and triggers one extra adversarial pass. Sim: capitulation fixture (marker vanishes, no justification) forces the extra pass. +3. **Early-stuck:** if the same marker set (content-hashed, not counted) survives two consecutive passes, jump straight to adjudication instead of burning remaining bounces. +4. **Verdict schema:** add required `confidence` (0-100) + `evidence_quote` per issue in the live review-verdict schema (frozen PS copy untouched); verifier prompt updated; findings under a confidence floor are reported but never block. +5. Optional flag: `--precise/--exhaustive` mapping to the marker-raising threshold. +- **Done means:** marker-lifecycle + verdict sims extended; a rubber-stamp fixture and a capitulation fixture both produce the flagged states. + +### Phase E — Trustworthy measurement (M, 💰) — *the flagship, fully model-run (revised 2026-07-07)* +Alan's labeling role is replaced by a cross-family judge panel; his involvement drops to an optional spot-check. Honesty caveat recorded in every report: the gold set is **model-consensus ground truth, not human ground truth** — validity numbers measure agreement with a frozen cross-family consensus, and the baseline's value is drift/regression detection plus relative comparisons, not absolute human alignment. +1. **Gold set (panel-labeled):** 30-50 bounce runs stratified across converged/adjudicated/stuck, each labeled independently by (a) fable-5 and (b) gpt-5.5 via `codex exec` against BOUNCE-RUBRIC.md. Inter-judge κ computed. Agreements become gold labels; disagreements go to an opus-4.8 tiebreak that sees both rationales, and are flagged in the manifest. Frozen under `evals/fixtures/gold/` with per-item provenance (which judge(s), tiebreak or not). Optional: Alan spot-checks the flagged disagreements (~10 min, not blocking). +2. 💰 **Calibration baseline:** `judge-bounce.sh` scored against the gold set; trust threshold κ≥0.6. Bias battery: swapped-order pass (position bias, flip-rate >5% = real), length-parity buckets (verbosity bias), and the gpt-5.5 panel leg doubles as the self-preference control. Self-consistency measured separately: 3 repeat runs per item, agreement rate reported alongside validity. +3. 💰 **Cross-vendor A/B (prove the premise), Fable-orchestrated:** same document set bounced claude↔codex vs claude↔claude (matched pass budgets); compare surviving markers, deterministic scores, and blind panel scores with judge blinded to which arm produced each doc. Pre-registered success criterion written into the run manifest BEFORE execution (e.g. cross-vendor arm must show ≥X% fewer surviving markers or higher panel score at p-level agreed in the manifest) so the result can't be goalpost-shifted after the fact. Either outcome is publishable. +4. 💰 **Sneaky-canary verifier calibration (prover-verifier pattern):** plant 3 plausible-but-subtly-wrong diffs through dev-review's verify seat against known ground truth; report catch rate. This is the rubber-stamp detector for the verifier seat. +5. **Regression gate:** frozen golden-document suite in CI — deterministic scorer deltas beyond tolerance fail the build (mirrors the 14-point code gate). +6. Absorb prior-plan P5: 💰 one happy-path codex-build dogfood run; tag v1.4 (code-state) + v1.5. +- **Done means:** κ (judge-vs-gold), inter-judge κ, flip rate, self-consistency rate, canary catch rate, and the A/B verdict all exist as written-down numbers in `evals/` docs + the progress file; tags pushed; STATE.md updated (stale since 2026-06-12). + +### Phase F — Learning loop + distribution reset (M) +1. **Lessons feed-forward (BugBot/metaswarm pattern):** after each adjudicated run, append a compact entry (disagreement pattern → chosen resolution → rationale) to `lessons.md`; bounce prompts get the relevant entries injected so adjudicated disagreements aren't re-litigated from scratch. Start file-based; no infra. +2. **Distribution reversal:** skills-first. Publish `skills/co-evolution/` as the primary channel (Claude Code plugin marketplace + npx skill bundle — Senate's dual-channel pattern); demote the v1.4 npm/MCP publish to secondary/SEO (ship it, don't gate strategy on it). 🚧 publishing gate. +3. **Publish the benchmark:** README section + short doc with the Phase E kappa + A/B results. Cheapest credible differentiation available — the surveyed field has zero measured evidence. +4. Positioning note: lead with the inline per-marker audit trail (vs ai-counsel's global score); document/plan refinement as the lane, not generic code review. +- **Done means:** skill installable from a public marketplace entry; benchmark numbers public; workspace CLAUDE.md workflow table updated. + +### Explicitly deferred +Deterministic lint/secret pre-pass (M, valuable but code-pipeline-only); parallel specialist critic lenses (M, measure after Phase D's severity gate lands); Greptile-style dependency-context pass (L); jq/JSON layer extraction + CRLF ingest normalization + seat-guard lib extraction (S-3/S-5, fold into whichever phase next touches those lines); confidence-weighted adjudication voting; `--dual-critique` (unchanged from prior backlog, after Phase D). + +### Approval gates (updated 2026-07-07 — Alan approved autonomous execution) +- Phase E spend (calibration, A/B, canaries, dogfood) — **approved 2026-07-07** ("A/B testing is better done by Fable"); codex-guard daily cap remains the hard ceiling; batch, never poll. +- Phase F items 2-3 — public publishing (marketplace + npm + benchmark page) — **still a hard STOP gate**; prepare everything, publish nothing without Alan's explicit go. +- Gold-set labeling — replaced by the cross-family judge panel (E.1); Alan's spot-check optional. +- `runners/codex-ps/**` — no gate; no change permitted, ever. + +## 5. Execution — goal and loop +Superseded in detail by `.planning/notes/2026-07-07-execution-loop.md` (the loop contract + live progress). Summary: this plan is executed autonomously, phase-per-PR, by an orchestrating Fable session running the loop defined there; every phase passes a build wave, an independent verify wave (hermetic suite + Claude adversarial review + gpt-5.5 cross-vendor review), and its done-means checklist verified by a non-builder agent, before merge on green CI. diff --git a/.planning/notes/2026-07-07-execution-loop.md b/.planning/notes/2026-07-07-execution-loop.md new file mode 100644 index 0000000..9ff06f4 --- /dev/null +++ b/.planning/notes/2026-07-07-execution-loop.md @@ -0,0 +1,63 @@ +# Execution Loop — 2026-07-07 Improvement Plan + +**Plan:** `.planning/notes/2026-07-07-audit-improvement-plan.md` (phases A–F) +**Orchestrator:** Fable session (this file is its working memory; any fresh session resumes from here) +**Authority:** Alan approved autonomous execution incl. Phase E spend (2026-07-07). Hard stops: public publishing (F.2/F.3), anything under `runners/codex-ps/**`, and any git operation that rewrites master history. + +## GOAL + +All six phases (A–F) merged to master with green 3-OS CI, every phase's done-means checklist independently verified, and the Phase E measurement numbers (κ judge-vs-gold, inter-judge κ, position-bias flip rate, self-consistency rate, canary catch rate, A/B verdict vs its pre-registered criterion) recorded in this file and in `evals/` docs. Terminal state: Phase F publishing artifacts staged and presented to Alan for the one remaining gate. + +## LOOP + +Repeat until GOAL or a hard stop: + +1. **Pick** the next phase whose dependencies are merged (order: A → B → C ∥ D → E → F). +2. **Build wave** — decompose the phase into worker briefs (Goal / Scope / Contract / Done-means, per repo Token Discipline); fan out parallel agents with disjoint file ownership; workers on opus for bash surgery, sonnet for docs/tests, `codex exec` for mechanical sweeps. +3. **Verify wave** (independent of builders — no agent verifies its own work): + a. `bash tests/run-all.sh` in an isolated subagent (summary only reaches the loop). + b. Claude adversarial review of the phase diff (adversarial-reviewer agent). + c. gpt-5.5 cross-vendor review of the same diff (`codex exec -s read-only`) — the repo's own philosophy applied to its own PRs; reviewer disagreements are surfaced, not averaged. + d. Done-means checklist from the plan, checked item-by-item by a non-builder agent. +4. **Fix loop** — findings from (3) go back to a build agent; one retry with a tighter brief, then escalate to the orchestrator itself. Max 3 fix cycles per phase before the phase is marked BLOCKED here and the loop moves to any non-dependent phase. +5. **Land** — commit (imperative, <72 chars), push branch, open PR citing this file's SHA, wait for 3-OS CI (background monitor, no polling), **merge on green** (pre-authorized), delete branch. +6. **Record** — update the Progress and Measurements sections below; capture lessons to ExoCortex; if context is near compaction or ~2h elapsed, write handoff notes here and continue in a fresh session reading this file. +7. **Regression watch** — after each merge, rerun the full suite on master once and append the result to the trend table. A red master halts the loop and fixes forward immediately. + +Loop mechanics: background agents re-invoke the orchestrator on completion (no polling); a ScheduleWakeup heartbeat (~25 min) survives hangs. This file is the single source of truth — sessions are disposable, the loop is not. + +## Verification additions (beyond the plan's per-phase done-means) + +- **V-1 Cross-vendor PR review** on every phase (loop step 3c) — added because the audit's headline defect (C-1) was a single-reviewer blind spot on a "finished" fix. +- **V-2 Live smoke per protocol-touching phase** (A, C, D): one real, minimal, non-stubbed run (`--bounces 1`, tiny doc; or one dev-review verify on a 5-line diff) — stub-fidelity gaps have bitten this repo twice; hermetic green is necessary, not sufficient. 💰-tiny, codex-guard-capped. +- **V-3 Self-referential dogfood gate** (Phase C done-means, kept prominent): the tool must bounce its own BOUNCE-PROTOCOL.md without tripping the honesty gate; the run becomes a permanent sim fixture. +- **V-4 Sneaky-canary verifier calibration** (Phase E.4): planted plausible-but-wrong diffs; catch rate is the standing rubber-stamp metric. +- **V-5 Pre-registration** (Phase E.3): the A/B success criterion is written into the run manifest before any run executes. +- **V-6 Master trend table** (below): scorer values per merge, so drift is visible across the whole campaign, not just within a phase. + +## Progress + +| Phase | Status | Branch / PR | Verify (suite / adv / codex / done-means) | Notes | +|-------|--------|-------------|-------------------------------------------|-------| +| A — Correctness closure | PR #47 open, suite 32/32, awaiting CI → merge | claude/nervous-hodgkin-bcf03d → PR #47 | ✓32/32 / ✓(F1 fixed) / ✓(H1,H2,L1 fixed) / ✓ | Cross-vendor review earned its keep: codex found the partial-failure→converged gap (H1) and both vendors independently flagged the bare-banner auth gap (H2→`output_is_auth_failure` in lib, 3 call sites). Claude reviewer caught the Scenario-F grep regression (F1) + missing guard scenario (→Scenario G). Bonus find-along: bounce-scorer-verification.sh had a Windows jq-CRLF bug (5/7→7/7, fixed) before wiring into run-all (C-5). Accepted residual: none remaining — F2/H2 fixed. Sims: auth-gate 28/28, marker-lifecycle 41/41 (byte-parity intact), audit-hardening 18/18, worktree-mgmt green, reliability 17/17. | +| B — Robustness/injection | pending | | | | +| C — Protocol v0.2 | pending | | | includes docs sweep + STACK.md re-check | +| D — Signal quality | pending | | | can start once C's marker changes are stable | +| E — Measurement | pending | | | panel-labeled gold set; spend approved | +| F — Learning loop + distribution | pending | | | F.2/F.3 publishing = HARD STOP for Alan | + +## Measurements + +| Metric | Value | Date | Source | +|--------|-------|------|--------| +| Inter-judge κ (fable-5 vs gpt-5.5, gold set) | – | | E.1 | +| Judge-vs-gold κ (judge-bounce.sh) | – | | E.2 | +| Position-bias flip rate | – | | E.2 | +| Self-consistency (3-run agreement) | – | | E.2 | +| Verifier canary catch rate (n=3) | – | | E.4 | +| A/B: cross- vs same-vendor (pre-registered criterion) | – | | E.3 | +| Master suite trend | baseline: 27 sims + scorer gate green @ 05d151e | 2026-07-07 | V-6 | + +## Handoff notes + +(none yet) diff --git a/co-evolve-bouncer.sh b/co-evolve-bouncer.sh index 2ad1032..e770329 100644 --- a/co-evolve-bouncer.sh +++ b/co-evolve-bouncer.sh @@ -150,10 +150,18 @@ while [[ $# -gt 0 ]]; do shift 2 ;; --agents) + # Must be exactly two non-empty, comma-separated agent names. A value with + # NO comma (e.g. `--agents claude`) previously self-paired silently: + # ${2%%,*} and ${2#*,} both return the whole string, so AGENT_A==AGENT_B + # and the "bounce" ran an agent against itself with no error. Require a + # comma so that mistake dies loudly. The three checks together — comma + # present, not two commas, both names non-empty — mean exactly one comma + # separating two non-empty names. + [[ "${2:-}" == *","* ]] || die "--agents requires two comma-separated agents (e.g., claude,codex), got: ${2:-}" + [[ "$2" == *","*","* ]] && die "--agents requires exactly two agents (e.g., claude,codex)" AGENT_A="${2%%,*}" AGENT_B="${2#*,}" AGENT_B="${AGENT_B%%,*}" - [[ "$2" == *","*","* ]] && die "--agents requires exactly two agents (e.g., claude,codex)" [[ -z "$AGENT_A" || -z "$AGENT_B" ]] && die "--agents requires exactly two agents separated by comma (e.g., claude,codex)" shift 2 ;; @@ -735,6 +743,13 @@ ${CONTEXT_BLOCK}${INPUT_CONTENT}" RUN_CONVERGED_NATURALLY="false" RUN_FINAL_MARKERS=0 RUN_FINAL_MARKERS_RAW=0 +# C-2: count bounce passes that produced usable output AND were applied to +# WORKING_FILE (i.e. reached append_bounce_pass). A loop that breaks on empty +# agent output (call + retry both empty) never applies a pass, so this stays 0 +# and the post-loop guard refuses to launder the un-reviewed compose draft as a +# converged final. Healthy runs apply >= 1 pass, so the guard never fires on +# them and byte-parity is untouched. +RUN_PASSES_APPLIED=0 run_bounce_phase() { local pass local role @@ -837,8 +852,15 @@ $(cat "$PROTOCOL_TEMPLATE")" fi if [[ ! -s "$output_file" ]]; then - log " ERROR: ${current_agent} returned empty output on retry. Stopping." - break + # C-2/C-8: die, don't break. A `break` here would hand whatever passes + # already applied to the post-loop convergence block, which happily + # finalizes "converged" on a run the protocol never finished — pass 1 + # applied, pass 2's agent died, and the half-bounced document launders + # into a clean final. An empty retry only ever happens when another pass + # was still REQUIRED (markers open, or chain stages pending), so the + # honest terminal is the same aborted path as the zero-pass case: the + # EXIT trap finalizes status=aborted and a chained --execute never runs. + die "bounce pass $pass: ${current_agent} returned empty output on call and retry — the bounce did not complete; run ABORTED. See run.log." fi cp "$output_file" "$RUN_DIR/pass-${pass}-${role}-${current_agent}-raw.md" @@ -865,6 +887,8 @@ $(cat "$PROTOCOL_TEMPLATE")" append_bounce_pass "$STATE_FILE" "$pass" "$role" "$current_agent" \ "pass-${pass}-${role}-${current_agent}-raw.md" "pass-${pass}-clean.md" \ "$contested" "$clarify" "$word_count" + # C-2: this pass produced usable output and is now recorded in state.passes. + RUN_PASSES_APPLIED=$((RUN_PASSES_APPLIED + 1)) # Human check if [[ "$AUTO" == "false" ]]; then @@ -893,6 +917,25 @@ $(cat "$PROTOCOL_TEMPLATE")" fi done + # C-2: a bounce that applied ZERO usable passes never reviewed the document, + # and WORKING_FILE still holds the un-reviewed compose draft. That draft is + # marker-free, so the convergence honesty block below would read 0 raw + # markers and finalize "converged" — laundering a failed bounce into a clean + # final that --execute would run. Empty agent output now dies IN the loop + # (C-8, above), so this guard is the belt-and-suspenders invariant for any + # other way of arriving here passless (e.g. --bounces 0). Refuse it: die + # non-zero. The EXIT trap (_finalize_bounce_state_on_exit) then finalizes + # status=aborted with convergence_status left null, exactly like the + # auth-failure abort (bounce-state-simulation.sh S4); the scorer gate fails + # on status=aborted, and a chained --execute never runs because the process + # died before the hand-off. This is the honest terminal state for "the agent + # produced nothing usable" — distinct from `stuck` (passes ran, but markers + # could not be resolved). Healthy runs apply >= 1 pass, so this never fires + # and the byte-parity path below is unchanged. + if (( RUN_PASSES_APPLIED == 0 )); then + die "bounce produced zero usable passes; the document was never reviewed — run ABORTED. See run.log." + fi + # A-5: convergence is decided by the marker count after the last pass, NOT by # mode. Any run — standard OR chain — that ends with 0 live markers converged # naturally and takes the byte-parity path (no adjudication). This also covers diff --git a/dev-review/codex/README.md b/dev-review/codex/README.md index 9ae5580..5a6c57b 100644 --- a/dev-review/codex/README.md +++ b/dev-review/codex/README.md @@ -43,6 +43,7 @@ bash dev-review/codex/dev-review.sh --skip-plan --plan .planning/phases/04-docs- | `--plan FILE` | Plan file used with `--skip-plan` | | `--model MODEL` | Override the Codex model for Codex-backed passes | | `--workdir DIR` | Execute against a target working directory | +| `--run-dir PATH` | Write run artifacts (`state.json`, plan, logs) to `PATH` instead of the default `runs/dev-review-/`. Path-traversal guarded: any `..` segment is rejected (`--run-dir must not contain '..'`) — defense in depth over the harness-side sanitizer | | `--lab MODE` | Route to `lab//entry.sh` (opt-in beta channel; fails fast with `unknown --lab mode: . Available: ` if `lab//` does not exist). See [`lab/README.md`](../../lab/README.md) for boundary, graduation criteria, and sandbox guarantee. | ### Lab routing diff --git a/dev-review/codex/dev-review.sh b/dev-review/codex/dev-review.sh index b46c628..669399b 100644 --- a/dev-review/codex/dev-review.sh +++ b/dev-review/codex/dev-review.sh @@ -113,6 +113,7 @@ Options: --live Launch visible Windows terminal tailing each phase's stderr (Windows-only; warns + falls back on other OS) --branch auto|NAME Create a feature branch off HEAD before execute (auto = dev-review/auto--); mutually exclusive with --worktree --worktree auto|PATH Create a git worktree for isolation before execute (auto = sibling dir); mutually exclusive with --branch + --run-dir PATH Write run artifacts (state.json, plan, logs) to PATH instead of the default runs/dev-review-/ (path-traversal guarded: rejects any '..' segment) --parent-run RUN_ID Lineage tag: record the orchestrator's parent run id in state.orchestration.parent_run_id (re-kicks always get a fresh run dir; no behavior change) --lab MODE Route to lab//entry.sh (opt-in beta channel; see lab/README.md) --target FILE PEL-only: file to mutate (used with --lab pel-proposer; must be repo-relative forward-slash path, e.g. lib/co-evolution.sh — NOT absolute or WSL/Windows-style) @@ -242,6 +243,13 @@ abort_on_timeout() { if [[ -n "${STATE_JSON:-}" ]]; then write_state_phase "$STATE_JSON" "$phase_name" "timeout" 124 "$phase_start" "$phase_end" write_state_field "$STATE_JSON" ".completed_at" "string" "$phase_end" + # A timeout abort is a terminal exit-1 run: mirror the EOF terminal block + # so a status reader sees a failed run, not one stuck "pending" mid-phase + # (the phase[].exit_code=124 above stays for observability). Without these, + # .status kept its "pending" init and .current_phase stayed non-null, + # which the status reader treats as "still in ". + write_state_field "$STATE_JSON" ".status" "string" "failed" + write_state_field "$STATE_JSON" ".current_phase" "null" fi log "ERROR: ${phase_name} phase timed out after ${PHASE_TIMEOUT}s - aborting run" cleanup_runtime_artifacts @@ -349,7 +357,7 @@ agent_auth_failed() { local agent="$1" local output_file="${2:-}" local stderr_file="${3:-}" - local cli_name words + local cli_name cli_name=$(agent_cli_name "$agent") @@ -358,15 +366,19 @@ agent_auth_failed() { # a substantial work product that merely echoes auth strings — e.g. plan text, # or the auth-detection source itself — is never misread as an auth failure. # - # (1) Auth banner IN THE OUTPUT, but only when the output is short (< 50 - # words). A long output that mentions "Unauthorized"/"Not logged in" is - # real work, not the CLI's own banner. - if [[ -n "$output_file" ]] && file_contains_auth_failure "$output_file"; then - words=$(wc -w < "$output_file" | tr -d '\r\n ') - if (( words < 50 )); then - log "WARNING: ${cli_name} authentication failed. Refresh the ${cli_name} CLI session and rerun." - return 0 - fi + # (1) Auth banner IN THE OUTPUT: route through lib's anchored + # output_contains_auth_banner (A-2 / C-1). A real CLI auth error prints a + # short banner that STANDS ALONE at the top of its output before doing any + # work, so the strict head-scan catches an auth-error PAGE of any length + # while still letting a long legitimate document that merely echoes + # "Unauthorized"/"Not logged in" mid-body pass. This replaces the old + # loose file_contains_auth_failure + whole-file <50-word ceiling, which + # accepted an auth page longer than 50 words as work product. The + # output_is_auth_failure wrapper keeps the short+loose catch for a bare + # "Unauthorized"-style banner the anchor deliberately excludes (C-8). + if [[ -n "$output_file" ]] && output_is_auth_failure "$output_file"; then + log "WARNING: ${cli_name} authentication failed. Refresh the ${cli_name} CLI session and rerun." + return 0 fi # (2) Auth banner in STDERR counts only when the agent produced NO output. A @@ -505,7 +517,12 @@ inspect_plan_output() { PLAN_OUTPUT_REASON="" cli_name=$(agent_cli_name "$agent") - if file_contains_auth_failure "$output_file" || file_contains_auth_failure "$stderr_file"; then + # C-6: anchor the OUTPUT-path auth check to lib's strict head-scan so a + # legitimate plan that discusses "401 Unauthorized" or "npm login required" + # mid-body is not routed to manual review; the loose matcher stays on stderr, + # where any auth string is the CLI's own banner (mirrors validate_agent_artifact). + # output_is_auth_failure adds back the short+loose catch for a bare banner (C-8). + if output_is_auth_failure "$output_file" || file_contains_auth_failure "$stderr_file"; then PLAN_OUTPUT_STATUS="review" PLAN_OUTPUT_REASON="${cli_name} authentication failed" return 1 @@ -1694,12 +1711,16 @@ fi # `--branch auto --plan-only` is a silent no-op on the branching side because # plan artifacts intentionally stay on the parent branch. # Mutually exclusive: parser already rejected both-set; only one path fires. -if [[ -n "$BRANCH_SPEC" ]]; then +# PLAN_EXIT==0 guard: a failed compose/bounce phase must not spawn a stray +# branch/worktree — the run is about to abort with the plan's non-zero exit +# (the revise loop returns PLAN_EXIT immediately), so any branch created here +# would be an orphan the executor never touches. +if [[ "${PLAN_EXIT:-0}" -eq 0 && -n "$BRANCH_SPEC" ]]; then BRANCH_CREATED=$(maybe_setup_branch "$WORKDIR" "$BRANCH_SPEC" "$TASK") if [[ -n "$BRANCH_CREATED" ]]; then write_state_field "$STATE_JSON" ".branch_created" "string" "$BRANCH_CREATED" fi -elif [[ -n "$WORKTREE_SPEC" ]]; then +elif [[ "${PLAN_EXIT:-0}" -eq 0 && -n "$WORKTREE_SPEC" ]]; then _new_wt=$(maybe_setup_worktree "$WORKDIR" "$WORKTREE_SPEC" "$TASK") if [[ -n "$_new_wt" ]]; then WORKTREE_PATH="$_new_wt" diff --git a/evals/tests/bounce-scorer-verification.sh b/evals/tests/bounce-scorer-verification.sh index b9b7d63..e709d51 100755 --- a/evals/tests/bounce-scorer-verification.sh +++ b/evals/tests/bounce-scorer-verification.sh @@ -56,6 +56,9 @@ for fixture_dir in "$FIXTURES"/*/; do if [[ "$ok" == true ]] && jq -e 'has("failing_checks_include")' "$expected" >/dev/null; then while IFS= read -r check; do + # Windows-native jq emits CRLF line endings; strip the trailing \r or + # $check never matches the clean scorer check name. + check=${check%$'\r'} jq -e --arg c "$check" '[.dimensions[].checks[]? | select(.ok == false) | .name] | index($c) != null' "$out" >/dev/null || ok=false done < <(jq -r '.failing_checks_include[]' "$expected") fi diff --git a/lib/co-evolution.sh b/lib/co-evolution.sh index 6f16d54..05292f1 100644 --- a/lib/co-evolution.sh +++ b/lib/co-evolution.sh @@ -645,6 +645,24 @@ output_contains_auth_banner() { <<< "$eligible" } +# C-8 (2026-07-07 cross-vendor review): the anchored banner scan above +# deliberately excludes a bare "Unauthorized"/"Not authenticated", so a CLI +# whose ENTIRE output is such a bare line slipped through once the old +# <50-word ceiling was removed (both reviewers flagged it independently). +# Restore that catch without reopening the long-page hole: the loose matcher +# is consulted only when the whole output is under 50 words — a real work +# product is never that small, and the old heuristic caught exactly this case. +# Output-path callers should use THIS, not output_contains_auth_banner alone. +output_is_auth_failure() { + local file_path="$1" + local words + + output_contains_auth_banner "$file_path" && return 0 + [[ -s "$file_path" ]] || return 1 + words=$(wc -w < "$file_path" | tr -d '\r\n ') + (( words < 50 )) && file_contains_auth_failure "$file_path" +} + file_contains_error_payload() { local file_path="$1" @@ -675,8 +693,9 @@ validate_agent_artifact() { # whole-file <50-word ceiling let an auth-error PAGE longer than 50 words fall # through to the accept below; anchoring to the head instead catches long # error pages while still letting a long legitimate document that merely - # echoes auth phrases mid-body pass. - if output_contains_auth_banner "$output_file"; then + # echoes auth phrases mid-body pass. output_is_auth_failure additionally + # keeps the short+loose catch for a bare banner the anchor excludes (C-8). + if output_is_auth_failure "$output_file"; then log " ERROR: ${agent_name} returned an authentication failure, not a document. Run \`${agent_name}\` interactively to log in, then re-run." return 2 fi @@ -1017,9 +1036,27 @@ count_markers_raw() { local file_path="$1" local marker="$2" + # Count TOKEN OCCURRENCES, not lines. The old form did `index($0,marker)>0 → + # count++`, incrementing once per line even when a line carried two markers + # (e.g. `[CONTESTED] a [CONTESTED] b`), which UNDERCOUNTED. The adjudication + # receipt gate compares report entries against this count (co-evolve-bouncer.sh + # run_adjudication `entries < pre_markers`), so an undercount let ONE report + # bullet satisfy the gate for TWO live markers. Walk each line with index() and + # advance past every match to tally all occurrences. index() (literal match) is + # used rather than gsub so the marker's `[` and `]` need no ERE escaping — safe + # across gawk/nawk/mawk. A marker-free document counts 0 either way, so the + # byte-parity convergence path is unaffected. awk -v marker="$marker" ' - BEGIN { count = 0 } - index($0, marker) > 0 { count++ } + BEGIN { count = 0; mlen = length(marker) } + { + line = $0 + pos = index(line, marker) + while (pos > 0) { + count++ + line = substr(line, pos + mlen) + pos = index(line, marker) + } + } END { print count } ' "$file_path" | tr -d '\r\n ' } @@ -1028,7 +1065,23 @@ strip_human_summary() { local input_file="$1" local output_file="$2" - awk '/^## HUMAN SUMMARY/{found=1} !found{print}' "$input_file" > "$output_file" + # The bounce protocol instructs agents to APPEND a `## HUMAN SUMMARY` section + # "at the very end" of the document (one line per pass) — it is always the + # trailing section (see templates/*/bounce-protocol.md rules 7-8). Stripping at + # the FIRST `^## HUMAN SUMMARY` heading destroyed any document whose BODY + # legitimately contains that heading (e.g. the protocol docs, or a plan that + # discusses human summaries) — everything from that point down was deleted. + # Key on the LAST occurrence instead: the final `## HUMAN SUMMARY` heading and + # everything after it is the agent-appended trailer; the body above it is + # preserved verbatim. Two-pass awk (find the last heading's line number, then + # print only lines before it) keeps this bash-3.2 / macOS-awk portable and + # byte-identical to the old behavior when there is exactly one (trailing) + # heading — the only case that changes is a document with an EARLIER body-level + # heading, which is now kept. + awk ' + NR==FNR { if ($0 ~ /^## HUMAN SUMMARY/) last=FNR; next } + last==0 || FNR < last { print } + ' "$input_file" "$input_file" > "$output_file" } size_sanity_check() { diff --git a/tests/audit-hardening-simulation.sh b/tests/audit-hardening-simulation.sh new file mode 100644 index 0000000..81ed9e8 --- /dev/null +++ b/tests/audit-hardening-simulation.sh @@ -0,0 +1,289 @@ +#!/usr/bin/env bash +# tests/audit-hardening-simulation.sh — 2026-07-07 audit Phase A low-severity +# bundle. Three hermetic gates, one per defect closed: +# +# B: --agents parsing — a value with no comma (e.g. `--agents claude`) used to +# self-pair silently (claude,claude) and bounce an agent against itself. +# It must now die with a clear message; exactly one comma + two non-empty +# names is the only accepted shape. +# C: strip_human_summary — used to truncate at the FIRST `^## HUMAN SUMMARY` +# line, destroying any document whose BODY legitimately contains that +# heading. It must strip only the LAST (agent-appended, trailing) section. +# D: count_markers_raw / adjudication receipt gate — the raw marker count used +# to increment once per LINE, so two of the SAME marker token on one line +# counted as 1 and the adjudication gate accepted ONE report entry for TWO +# live markers. The count must be per-occurrence, and the gate must demand a +# report entry per marker. +# +# Hermetic: agent CLIs PATH-stubbed for the D integration; B and C need no +# network. bash 3.2-safe (Git Bash + Linux + macOS): no arrays-of-arrays, no +# `mapfile`, no `${var^^}`. CRLF-safe: lib helpers already `tr -d '\r'`. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +BOUNCER="$REPO_ROOT/co-evolve-bouncer.sh" +LIB="$REPO_ROOT/lib/co-evolution.sh" + +TEST_DIR=$(mktemp -d -t audit-hardening-XXXXXX) +cleanup() { rm -rf "$TEST_DIR"; } +trap cleanup EXIT + +command -v jq >/dev/null 2>&1 || { echo "SKIP-FAIL: jq required for this gate"; exit 1; } + +TOTAL=0 +PASSED=0 +pass() { printf 'PASS: %s\n' "$1"; PASSED=$((PASSED + 1)); } +fail() { printf 'FAIL: %s\n' "$1"; } +check() { TOTAL=$((TOTAL + 1)); if eval "$2"; then pass "$1"; else fail "$1 [cond: $2]"; fi; } + +# =========================================================================== +# B: --agents parsing. A malformed value dies during arg parsing before any +# agent is invoked. A VALID pair parses and proceeds, so the B section is run +# under fast converging stubs (reviewer emits a marker-free doc → the loop +# converges on pass 1) to keep the positive control hermetic and quick. +# =========================================================================== +mkdir -p "$TEST_DIR/agentbin" +for a in claude codex; do + cat > "$TEST_DIR/agentbin/$a" <<'STUB' +#!/usr/bin/env bash +for arg in "$@"; do case "$arg" in --version|-v) echo "stub 1.0.0"; exit 0 ;; esac; done +out=""; prev="" +for x in "$@"; do [[ "$prev" == "-o" ]] && out="$x"; prev="$x"; done +cat >/dev/null 2>&1 || true +body='# Plan + +## Approach + +Ship it. No open questions remain.' +if [[ -n "$out" ]]; then printf '%s\n' "$body" > "$out"; else printf '%s\n' "$body"; fi +STUB + chmod +x "$TEST_DIR/agentbin/$a" +done + +run_agents() { + # Returns the bouncer's exit code; captures BOTH streams (die() logs the parse + # error via log(), which prints to STDOUT before RUN_DIR/run.log exists, so + # stderr alone would miss the message). --vanilla keeps it non-interactive. + local val="$1" + local outlog="$2" + local rc=0 + PATH="$TEST_DIR/agentbin:$PATH" CO_EVOLVE_RUNS_DIR="$TEST_DIR/agents-runs" \ + bash "$BOUNCER" --agents "$val" --vanilla "some prompt" \ + > "$outlog" 2>&1 || rc=$? + return $rc +} + +B1_ERR="$TEST_DIR/b1.err"; rc=0; run_agents "claude" "$B1_ERR" || rc=$? +check "B1: --agents with no comma exits non-zero (no silent self-pair)" \ + "[[ $rc -ne 0 ]]" +check "B1b: the error message names the comma requirement" \ + "grep -q 'comma' '$B1_ERR'" + +B2_ERR="$TEST_DIR/b2.err"; rc=0; run_agents "claude,codex,opus" "$B2_ERR" || rc=$? +check "B2: --agents with two commas (three agents) exits non-zero" \ + "[[ $rc -ne 0 ]]" + +B3_ERR="$TEST_DIR/b3.err"; rc=0; run_agents ",codex" "$B3_ERR" || rc=$? +check "B3: --agents with an empty first name exits non-zero" \ + "[[ $rc -ne 0 ]]" + +B4_ERR="$TEST_DIR/b4.err"; rc=0; run_agents "claude," "$B4_ERR" || rc=$? +check "B4: --agents with an empty second name exits non-zero" \ + "[[ $rc -ne 0 ]]" + +# Positive control: a valid pair must PARSE (it may fail later for lack of a +# real CLI, but it must NOT die with an --agents message). We assert the stderr +# from a valid pair does NOT carry the --agents parse error. +B5_ERR="$TEST_DIR/b5.err"; rc=0; run_agents "claude,codex" "$B5_ERR" || rc=$? +check "B5: a valid comma-separated pair does NOT trip the --agents parse error" \ + "! grep -q '\\-\\-agents requires' '$B5_ERR'" + +# B6 (C-8 cross-vendor review): --agents as the LAST argv token — no value at +# all. Pre-fix, `$2` was unbound and set -u killed the script with a raw +# "unbound variable" error instead of the validation message; `${2:-}` now +# routes it into the same clear die. Invoked directly (not via run_agents, +# which always supplies a value after the flag). +B6_OUT="$TEST_DIR/b6.out"; rc=0 +bash "$BOUNCER" --vanilla "some prompt" --agents > "$B6_OUT" 2>&1 || rc=$? +check "B6: --agents with NO value exits non-zero" \ + "[[ $rc -ne 0 ]]" +check "B6b: missing value dies with the '--agents requires' message" \ + "grep -q -- '--agents requires' '$B6_OUT'" +check "B6c: no raw set -u 'unbound variable' error leaks" \ + "! grep -qi 'unbound variable' '$B6_OUT'" + +# =========================================================================== +# C: strip_human_summary keys on the LAST heading (source the lib directly). +# =========================================================================== +# shellcheck disable=SC1090 +source "$LIB" + +# A document whose BODY contains `## HUMAN SUMMARY` plus an agent-appended +# trailing `## HUMAN SUMMARY` section. The body heading + everything under it +# up to the trailing section must survive; only the trailing section is stripped. +C_IN="$TEST_DIR/c-in.md" +cat > "$C_IN" <<'DOC' +# Plan + +## HUMAN SUMMARY + +This heading is part of the document body: it explains what a human summary is. + +## Approach + +Ship it. + +## HUMAN SUMMARY + +- pass 1: tightened the approach +DOC +C_OUT="$TEST_DIR/c-out.md" +strip_human_summary "$C_IN" "$C_OUT" +check "C1: body-level '## HUMAN SUMMARY' heading survives the strip" \ + "grep -q '^## HUMAN SUMMARY' '$C_OUT'" +check "C2: exactly ONE '## HUMAN SUMMARY' remains (trailing section removed)" \ + "[[ \$(grep -c '^## HUMAN SUMMARY' '$C_OUT') -eq 1 ]]" +check "C3: the body 'Approach' section is preserved" \ + "grep -q '^## Approach' '$C_OUT'" +check "C4: the trailing per-pass line is gone" \ + "! grep -q 'pass 1: tightened' '$C_OUT'" + +# Byte-parity: a document with only ONE (trailing) HUMAN SUMMARY strips exactly +# as the old first-match awk did. +C2_IN="$TEST_DIR/c2-in.md" +printf '# Plan\n\nBody text.\n\n## HUMAN SUMMARY\n\n- pass 1\n' > "$C2_IN" +C2_OUT="$TEST_DIR/c2-out.md" +strip_human_summary "$C2_IN" "$C2_OUT" +C2_EXP="$TEST_DIR/c2-exp.md" +printf '# Plan\n\nBody text.\n\n' > "$C2_EXP" +check "C5: single trailing heading strips byte-identically to the old behavior" \ + "diff -q '$C2_EXP' '$C2_OUT' >/dev/null" + +# =========================================================================== +# D-unit: count_markers_raw counts token OCCURRENCES, not lines. +# =========================================================================== +D_DOC="$TEST_DIR/d-two-same.md" +# Two of the SAME marker token on ONE line — the exact undercount case. +printf '# Doc\n\n[CONTESTED] window one vs two weeks. [CONTESTED] rollback owner unnamed.\n' > "$D_DOC" +check "D1: two [CONTESTED] on one line count as 2 (was 1 under the line-count bug)" \ + "[[ \$(count_markers_raw '$D_DOC' '[CONTESTED]') -eq 2 ]]" +check "D2: a marker-free document still counts 0" \ + "[[ \$(count_markers_raw '$TEST_DIR/c2-exp.md' '[CONTESTED]') -eq 0 ]]" + +# =========================================================================== +# D-integration: two identical markers on one line survive the bounce; the +# adjudication receipt gate must demand a report entry PER marker. A one-entry +# adjudicator is now insufficient (run ends stuck); a two-entry adjudicator +# resolves it (adjudicated). Under the old line-count, pre_markers was 1 and the +# one-entry report wrongly passed the gate. +# =========================================================================== +mkdir -p "$TEST_DIR/bin" + +# Reviewer (claude, odd passes) and composer (codex, even passes) both emit the +# same two-CONTESTED-on-one-line body so the pair survives to adjudication. The +# codex stub also plays adjudicator: on the adjudication prompt it emits a clean +# body plus a report whose entry count is AH_ADJ_ENTRIES. +BODY_TWO="$TEST_DIR/body-two.txt" +cat > "$BODY_TWO" <<'DOC' +# Sample Plan + +## Approach + +Ship the feature in a staged rollout. +[CONTESTED] window one week vs two. [CONTESTED] rollback owner is unnamed. + +## Risks + +Both disagreements above share a single line on purpose. +DOC + +ADJ_BODY="$TEST_DIR/adj-body.txt" # clean body, no report yet (report appended per entries) +cat > "$ADJ_BODY" <<'DOC' +# Sample Plan + +## Approach + +Ship the feature in a staged, two-week rollout owned by the on-call lead. + +## Risks + +Resolved. + +## ADJUDICATION REPORT +DOC + +cat > "$TEST_DIR/bin/claude" </dev/null +cat "$BODY_TWO" +STUB +chmod +x "$TEST_DIR/bin/claude" + +cat > "$TEST_DIR/bin/codex" </dev/null || true) +out=""; prev="" +for a in "\$@"; do [[ "\$prev" == "-o" ]] && out="\$a"; prev="\$a"; done +emit() { if [[ -n "\$out" ]]; then cat > "\$out"; else cat; fi; } +if printf '%s' "\$prompt" | grep -q 'You are the ADJUDICATOR'; then + { + cat "$ADJ_BODY" + printf -- '- [CONTESTED] window length -> CHOSE: two weeks | WHY: soak time needed.\n' + if [[ "\${AH_ADJ_ENTRIES:-1}" -ge 2 ]]; then + printf -- '- [CONTESTED] rollback owner -> CHOSE: on-call lead | WHY: clear accountability.\n' + fi + } | emit +else + cat "$BODY_TWO" | emit +fi +exit 0 +STUB +chmod +x "$TEST_DIR/bin/codex" + +D_SEED="$TEST_DIR/d-seed.md" +cat > "$D_SEED" <<'DOC' +# Sample Plan + +## Approach + +Ship the feature in a one-week rollout window. + +## Risks + +Rollback procedure has no named owner. +DOC + +run_d() { + local runs="$1"; local entries="$2"; local rc=0 + AH_ADJ_ENTRIES="$entries" CO_EVOLVE_RUNS_DIR="$runs" PATH="$TEST_DIR/bin:$PATH" \ + bash "$BOUNCER" --vanilla --bounce-only "$D_SEED" \ + > "$runs.stdout.log" 2> "$runs.stderr.log" || rc=$? + return $rc +} +d_state() { find "$1" -name state.json 2>/dev/null | head -1; } + +# One entry for two markers -> gate refuses -> stuck. +D3_RUNS="$TEST_DIR/d3-runs" +run_d "$D3_RUNS" 1 || true +D3_STATE=$(d_state "$D3_RUNS") +check "D3: one report entry for two same-line markers ends STUCK (gate honest)" \ + "[[ -f '$D3_STATE' ]] && [[ \$(jq -r '.convergence_status' '$D3_STATE') == stuck ]]" + +# Two entries for two markers -> gate satisfied -> adjudicated. +D4_RUNS="$TEST_DIR/d4-runs" +run_d "$D4_RUNS" 2 || true +D4_STATE=$(d_state "$D4_RUNS") +check "D4: two report entries for two same-line markers ends ADJUDICATED" \ + "[[ -f '$D4_STATE' ]] && [[ \$(jq -r '.convergence_status' '$D4_STATE') == adjudicated ]]" + +# --------------------------------------------------------------------------- +printf '%d/%d scenarios passed' "$PASSED" "$TOTAL" +if (( PASSED != TOTAL )); then + printf ' (%d failed)\n' "$((TOTAL - PASSED))" + exit 1 +fi +printf '\n' diff --git a/tests/auth-gate-simulation.sh b/tests/auth-gate-simulation.sh index 1b24bcf..9a09b0d 100644 --- a/tests/auth-gate-simulation.sh +++ b/tests/auth-gate-simulation.sh @@ -117,19 +117,22 @@ else fi # --------------------------------------------------------------------------- -# A4: a bare "Unauthorized" line in a short document must NOT be fatal. The -# broad matcher would flag a lone "Unauthorized"; the strict output detector -# deliberately does not, so a terse-but-legitimate artifact is not misread. +# A4 (contract updated for C-8): a SHORT (<50-word) output containing a bare +# "Unauthorized" IS fatal again. The anchored matcher alone deliberately skips +# bare tokens, but output_is_auth_failure restores the old loose-when-short +# catch: a real work product is never under 50 words, so a terse output with +# an auth token is the CLI's own error, not a document. (The >50-word legit +# doc mentioning "Unauthorized" mid-body stays covered by A2/C1-*-legit/C6-ok.) # --------------------------------------------------------------------------- TOTAL=$((TOTAL + 1)) out="$TEST_DIR/a4-out.md"; err="$TEST_DIR/a4-err.log" printf '# Status Codes\n\n401 means Unauthorized and 403 means Forbidden.\n' > "$out" : > "$err" rc=0; validate_agent_artifact "$out" "$err" claude >/dev/null 2>&1 || rc=$? -if [[ "$rc" -eq 0 ]]; then - pass "A4: bare 'Unauthorized' in short doc -> rc 0 (strict detector ignores bare token)" +if [[ "$rc" -eq 2 ]]; then + pass "A4: bare 'Unauthorized' in <50-word output -> rc 2 (C-8 short+loose catch)" else - fail "A4: bare 'Unauthorized' -> expected rc 0, got $rc" + fail "A4: bare 'Unauthorized' in short output -> expected rc 2, got $rc" fi # --------------------------------------------------------------------------- @@ -280,6 +283,226 @@ else fail "A9: banner after closed fence -> expected rc 2, got $rc" fi +# --------------------------------------------------------------------------- +# C-1 / C-6 dev-review.sh gates. The code pipeline's own auth detectors +# (agent_auth_failed at the execute+verify gates, inspect_plan_output at the +# compose/bounce gate) previously used the loose file_contains_auth_failure + +# whole-file <50-word ceiling — the same blind spot A-2 closed for the document +# pipeline. These scenarios extract the real dev-review.sh functions and pin the +# anchored-matcher behavior directly. Extraction (not `source dev-review.sh`) +# because the script runs its main flow at EOF; temp-file source, not process +# substitution, because bash 3.2 (stock macOS) silently sources nothing from +# `source <(...)`. +# --------------------------------------------------------------------------- +sed -n '/^agent_cli_name() {/,/^}$/p; /^agent_auth_failed() {/,/^}$/p; /^inspect_plan_output() {/,/^}$/p; /^abort_on_timeout() {/,/^}$/p' \ + "$REPO_ROOT/dev-review/codex/dev-review.sh" > "$TEST_DIR/_dev_review_fns.sh" +# abort_on_timeout calls cleanup_runtime_artifacts (defined further down in +# dev-review.sh, not extracted); stub it so the timeout scenario runs hermetically. +cleanup_runtime_artifacts() { :; } +# shellcheck disable=SC1090,SC1091 +source "$TEST_DIR/_dev_review_fns.sh" +if ! declare -F agent_auth_failed >/dev/null || ! declare -F inspect_plan_output >/dev/null \ + || ! declare -F abort_on_timeout >/dev/null; then + echo "FAIL: dev-review.sh functions not sourced — simulation cannot continue" + exit 1 +fi + +# --------------------------------------------------------------------------- +# (a) agent_auth_failed — a >50-word auth-error PAGE in the output is an auth +# failure (rc 0) at BOTH the execute gate (executor agent) and the verify gate +# (verifier agent). The old <50-word ceiling accepted this page as work product. +# The two callers (887/897 execute, 1039 verify) invoke the same function with +# their respective agent types, so exercising both names covers both gates. +# --------------------------------------------------------------------------- +authpage="$TEST_DIR/authpage.md" +{ + printf 'Not logged in \xc2\xb7 Please run /login\n\n' + printf 'You are not currently authenticated with the Claude CLI. To use this\n' + printf 'command you must first sign in with your Anthropic account. Run the\n' + printf 'login command in an interactive terminal, complete the browser flow,\n' + printf 'and then re-run. If you continue to see this message your session token\n' + printf 'may have expired or your organization may not have access to this tier.\n' +} > "$authpage" +authpage_words=$(wc -w < "$authpage" | tr -d '\r\n ') + +for gate in "execute:opus" "verify:codex"; do + gate_name="${gate%%:*}"; gate_agent="${gate##*:}" + TOTAL=$((TOTAL + 1)) + err="$TEST_DIR/${gate_name}-err.log"; : > "$err" + rc=0; agent_auth_failed "$gate_agent" "$authpage" "$err" >/dev/null 2>&1 || rc=$? + if [[ "$rc" -eq 0 && "$authpage_words" -ge 50 ]]; then + pass "C1-${gate_name}: >50-word auth page ($authpage_words words) at ${gate_name} gate -> auth failure (rc 0)" + else + fail "C1-${gate_name}: >50-word auth page ($authpage_words words) -> expected rc 0, got $rc" + fi +done + +# --------------------------------------------------------------------------- +# (b) agent_auth_failed — a legitimate long output that echoes "Unauthorized" +# and "Not logged in" MID-body (never line-leading) passes BOTH gates: it is +# real work product, not the CLI's own banner, so the function returns 1. +# --------------------------------------------------------------------------- +legit="$TEST_DIR/legit-exec.md" +{ + printf '# Auth Handling Change — Execution Log\n\n' + printf 'Wired the retry wrapper so a response marked Unauthorized is treated as\n' + printf 'fatal and surfaces immediately. When the token is missing the upstream\n' + printf 'returns a body that reads "Not logged in" and the wrapper must not retry\n' + printf 'that case. Added structured logging so operators can audit every attempt\n' + printf 'and see exactly why a request was rejected as Unauthorized after a run.\n' +} > "$legit" +legit_words=$(wc -w < "$legit" | tr -d '\r\n ') + +for gate in "execute:opus" "verify:codex"; do + gate_name="${gate%%:*}"; gate_agent="${gate##*:}" + TOTAL=$((TOTAL + 1)) + err="$TEST_DIR/${gate_name}-legit-err.log"; : > "$err" + rc=0; agent_auth_failed "$gate_agent" "$legit" "$err" >/dev/null 2>&1 || rc=$? + if [[ "$rc" -eq 1 && "$legit_words" -ge 50 ]]; then + pass "C1-${gate_name}-legit: long output echoing auth phrases mid-body ($legit_words words) passes ${gate_name} gate (rc 1)" + else + fail "C1-${gate_name}-legit: legit output ($legit_words words) -> expected rc 1, got $rc" + fi +done + +# --------------------------------------------------------------------------- +# C-6 inspect_plan_output — a legitimate plan discussing "401 Unauthorized" and +# "npm login required" mid-body must NOT be routed to manual review (status +# stays "ok"); a leading auth banner still is (status "review"). +# --------------------------------------------------------------------------- +TOTAL=$((TOTAL + 1)) +c6ok="$TEST_DIR/c6-ok.md"; c6err="$TEST_DIR/c6-err.log" +{ + printf '# Publish Pipeline Plan\n\n## Goal\n' + printf 'Automate the npm publish. The CI job fails with "401 Unauthorized" when\n' + printf 'the token is stale, and the local dry-run prints "npm login required" —\n' + printf 'both are expected states the plan must handle by refreshing credentials\n' + printf 'before the release step rather than aborting the whole pipeline run.\n\n' + printf '## Steps\n1. Validate the token.\n2. Refresh on failure.\n3. Publish.\n' +} > "$c6ok" +: > "$c6err" +PLAN_OUTPUT_STATUS=""; PLAN_OUTPUT_REASON="" +rc=0; inspect_plan_output opus "$c6ok" "$c6err" >/dev/null 2>&1 || rc=$? +# A valid plan may still be flagged "thin" by later checks, but it must NOT be +# "review" on the auth leg — that is the C-6 false positive under test. +if [[ "$PLAN_OUTPUT_STATUS" != "review" ]]; then + pass "C6-ok: plan discussing '401 Unauthorized'/'npm login required' mid-body -> status '$PLAN_OUTPUT_STATUS' (not routed to auth review)" +else + fail "C6-ok: legit plan -> unexpectedly routed to review ($PLAN_OUTPUT_REASON)" +fi + +TOTAL=$((TOTAL + 1)) +c6bad="$TEST_DIR/c6-bad.md" +cp "$authpage" "$c6bad" +PLAN_OUTPUT_STATUS=""; PLAN_OUTPUT_REASON="" +rc=0; inspect_plan_output opus "$c6bad" "$c6err" >/dev/null 2>&1 || rc=$? +if [[ "$PLAN_OUTPUT_STATUS" == "review" && "$rc" -eq 1 ]]; then + pass "C6-bad: leading auth banner in plan output -> status 'review' (rc 1)" +else + fail "C6-bad: auth-banner plan -> expected status 'review' rc 1, got '$PLAN_OUTPUT_STATUS' rc $rc" +fi + +# --------------------------------------------------------------------------- +# C-8 (cross-vendor review): a CLI whose ENTIRE output is a bare "Unauthorized" +# line. The anchored banner scan deliberately skips bare tokens, so removing +# the <50-word ceiling reopened exactly this case; output_is_auth_failure +# restores the loose-when-short catch. Pin it at every output-path gate: +# execute + verify (agent_auth_failed) and the plan gate (inspect_plan_output). +# The >50-word mid-body regression guards for the same helper path are +# C1-execute-legit / C1-verify-legit / C6-ok above. +# --------------------------------------------------------------------------- +bare="$TEST_DIR/c8-bare.md"; c8err="$TEST_DIR/c8-err.log" +printf 'Unauthorized\n' > "$bare" +: > "$c8err" + +for gate in "execute:opus" "verify:codex"; do + gate_name="${gate%%:*}"; gate_agent="${gate##*:}" + TOTAL=$((TOTAL + 1)) + rc=0; agent_auth_failed "$gate_agent" "$bare" "$c8err" >/dev/null 2>&1 || rc=$? + if [[ "$rc" -eq 0 ]]; then + pass "C8-${gate_name}: sole bare 'Unauthorized' output at ${gate_name} gate -> auth failure (rc 0)" + else + fail "C8-${gate_name}: bare 'Unauthorized' output -> expected rc 0, got $rc" + fi +done + +TOTAL=$((TOTAL + 1)) +PLAN_OUTPUT_STATUS=""; PLAN_OUTPUT_REASON="" +rc=0; inspect_plan_output opus "$bare" "$c8err" >/dev/null 2>&1 || rc=$? +if [[ "$PLAN_OUTPUT_STATUS" == "review" && "$rc" -eq 1 ]]; then + pass "C8-plan: sole bare 'Unauthorized' output at plan gate -> status 'review' (rc 1)" +else + fail "C8-plan: bare 'Unauthorized' plan output -> expected status 'review' rc 1, got '$PLAN_OUTPUT_STATUS' rc $rc" +fi + +# Same bare-line variant with "Not authenticated" — the other token the +# anchored matcher deliberately excludes; the short+loose catch must hold too. +TOTAL=$((TOTAL + 1)) +printf 'Not authenticated\n' > "$bare" +rc=0; agent_auth_failed opus "$bare" "$c8err" >/dev/null 2>&1 || rc=$? +if [[ "$rc" -eq 0 ]]; then + pass "C8-notauth: sole bare 'Not authenticated' output -> auth failure (rc 0)" +else + fail "C8-notauth: bare 'Not authenticated' output -> expected rc 0, got $rc" +fi + +# --------------------------------------------------------------------------- +# (c) abort_on_timeout — a timeout abort is a terminal exit-1 run: state.json +# must read status="failed" with current_phase=null, not the "pending" init +# (which a status reader treats as "still running"). Runs in a subshell because +# abort_on_timeout exits; the state writes land before exit, so the parent then +# inspects the file. Requires jq (as every state.json sim does). +# --------------------------------------------------------------------------- +if command -v jq >/dev/null 2>&1; then + TOTAL=$((TOTAL + 1)) + STATE_JSON="$TEST_DIR/timeout-state.json" + init_state_json "$STATE_JSON" "sim-timeout" "sim task" "codex" "codex" "opus" + # abort_on_timeout only fires when LAST_INVOKE_EXIT_CODE==124. + LAST_INVOKE_EXIT_CODE=124 + PHASE_TIMEOUT=1800 + sub_rc=0 + ( abort_on_timeout "execute" "2026-07-07T00:00:00Z" ) >/dev/null 2>&1 || sub_rc=$? + status_val=$(jq -r '.status' "$STATE_JSON") + phase_val=$(jq -r '.current_phase' "$STATE_JSON") + if [[ "$sub_rc" -eq 1 && "$status_val" == "failed" && "$phase_val" == "null" ]]; then + pass "C-timeout: abort_on_timeout leaves status='failed', current_phase=null, exit 1" + else + fail "C-timeout: got rc=$sub_rc status='$status_val' current_phase='$phase_val' (expected 1/failed/null)" + fi + unset LAST_INVOKE_EXIT_CODE PHASE_TIMEOUT STATE_JSON +else + printf 'SKIP: C-timeout scenario needs jq (not found)\n' +fi + +# --------------------------------------------------------------------------- +# phase_is_writable negative case: an unknown phase name resolves read-only. +# The writable set is a fixed allowlist (execute/execute-retry/fix) plus the +# ^execute-[0-9]+$ revise-pass pattern; anything else — including a plausible +# typo like "verify" or an injection-shaped string — must return "false" so a +# read-only phase never gains write access by accident. (An empty/unset name is +# out of scope here: phase_is_writable :?-guards it, force-exiting by design.) +# --------------------------------------------------------------------------- +for bogus in "compose" "verify" "bounce" "execute-x" "exec" "execute; rm -rf"; do + TOTAL=$((TOTAL + 1)) + verdict=$(phase_is_writable "$bogus") + if [[ "$verdict" == "false" ]]; then + pass "phase_is_writable: '${bogus}' -> read-only (false)" + else + fail "phase_is_writable: '${bogus}' -> expected false, got '$verdict'" + fi +done + +# Positive control: a real writable phase and a numbered revise pass stay true. +for good in "execute" "execute-2"; do + TOTAL=$((TOTAL + 1)) + verdict=$(phase_is_writable "$good") + if [[ "$verdict" == "true" ]]; then + pass "phase_is_writable: '${good}' -> writable (true)" + else + fail "phase_is_writable: '${good}' -> expected true, got '$verdict'" + fi +done + # --------------------------------------------------------------------------- printf '%d/%d scenarios passed' "$PASSED" "$TOTAL" diff --git a/tests/marker-lifecycle-simulation.sh b/tests/marker-lifecycle-simulation.sh index ac580a8..28d2231 100644 --- a/tests/marker-lifecycle-simulation.sh +++ b/tests/marker-lifecycle-simulation.sh @@ -148,6 +148,11 @@ else case "\${LC_STUB_DOC:-markers}" in clean) cat "$TEST_DIR/body-clean.txt" ;; fenced) cat "$TEST_DIR/body-fenced.txt" ;; + empty) : ;; # C-2: emit NOTHING (empty output on every call, incl. retry) + # C-8 partial-failure: claude (reviewer, odd passes) SUCCEEDS with a + # markers body — pass 1 applies and another pass is still required; the + # codex side (composer, even passes) goes empty. See the codex stub. + partial) cat "$TEST_DIR/body-markers.txt" ;; *) cat "$TEST_DIR/body-markers.txt" ;; esac fi @@ -168,6 +173,10 @@ else case "\${LC_STUB_DOC:-markers}" in clean) pick "$TEST_DIR/body-clean.txt" ;; fenced) pick "$TEST_DIR/body-fenced.txt" ;; + empty) if [[ -n "\$out" ]]; then : > "\$out"; fi ;; # C-2: empty artifact + # C-8 partial-failure: the composer side is the one that dies — empty + # artifact on call AND retry, after claude's pass 1 already applied. + partial) if [[ -n "\$out" ]]; then : > "\$out"; fi ;; *) pick "$TEST_DIR/body-markers.txt" ;; esac fi @@ -347,6 +356,59 @@ check "S7b: stuck fenced run labeled with CO-EVOLVE:STUCK banner" \ check "S7c: fenced marker token preserved in the stuck document" \ "[[ -f '$S7_DIR/working.md' ]] && [[ \$(grep -cE '\[(CONTESTED|CLARIFY)\]' '$S7_DIR/working.md' || true) -ge 1 ]]" +# =========================================================================== +# Scenario 8: ABORTED (C-2) — the reviewer returns EMPTY output on call AND +# retry, so the loop breaks before any pass is applied. The un-reviewed compose +# draft is marker-free, so the OLD code finalized it "converged" with passes=[] +# — a fully failed bounce laundered into a clean final. The fix must instead end +# the run ABORTED: non-zero exit, status=aborted, convergence_status left null, +# and passes empty. This is distinct from `stuck` (which ran passes but could +# not resolve markers); here NOTHING was reviewed. +# =========================================================================== +S8_RUNS="$TEST_DIR/s8-runs" +rc=0; LC_STUB_DOC=empty run_branch "$S8_RUNS" --vanilla --bounce-only "$SEED" || rc=$? +S8_STATE=$(find_state "$S8_RUNS"); S8_DIR=$(dirname "$S8_STATE" 2>/dev/null || echo "") +check "S8a: zero-usable-pass run exits NON-ZERO (not laundered as success)" \ + "[[ $rc -ne 0 ]]" +check "S8b: convergence_status is NOT converged (left null on an abort)" \ + "[[ -f '$S8_STATE' ]] && [[ \$(jq -r '.convergence_status' '$S8_STATE') != converged ]]" +check "S8c: state status == aborted" \ + "[[ -f '$S8_STATE' ]] && [[ \$(jq -r '.status' '$S8_STATE') == aborted ]]" +check "S8d: passes array is empty (no usable pass was applied)" \ + "[[ -f '$S8_STATE' ]] && [[ \$(jq -r '.passes | length' '$S8_STATE') -eq 0 ]]" + +# 8e: a chained --execute must REFUSE an aborted bounce — the process dies before +# the hand-off, so the executor is never invoked (strictly stronger than the +# stuck guard, which exits 1 at the hand-off). Assert non-zero exit AND that the +# dev-review hand-off banner never printed. +S8E_RUNS="$TEST_DIR/s8e-runs" +rc=0; LC_STUB_DOC=empty run_branch "$S8E_RUNS" --vanilla --bounce-only --execute "$SEED" || rc=$? +check "S8e-1: chained --execute on an aborted bounce exits NON-ZERO" \ + "[[ $rc -ne 0 ]]" +check "S8e-2: --execute never handed the (nonexistent) plan to dev-review" \ + "! grep -q 'Handing bounced plan to dev-review' '$S8E_RUNS.stdout.log' '$S8E_RUNS.stderr.log' 2>/dev/null" + +# =========================================================================== +# Scenario 9: PARTIAL-FAILURE ABORT (C-8, cross-vendor review) — pass 1 applies +# successfully (reviewer output WITH surviving markers, so another pass is +# still required), then pass 2's composer returns EMPTY on call AND retry. The +# old `break` handed the half-bounced document to the post-loop convergence +# block; the fix dies instead, so the EXIT trap finalizes status=aborted with +# convergence_status null and EXACTLY the one applied pass recorded — never +# converged, never adjudicated. +# =========================================================================== +S9_RUNS="$TEST_DIR/s9-runs" +rc=0; LC_STUB_DOC=partial run_branch "$S9_RUNS" --vanilla --bounce-only "$SEED" || rc=$? +S9_STATE=$(find_state "$S9_RUNS") +check "S9a: partial-failure run exits NON-ZERO (empty retry dies, no break)" \ + "[[ $rc -ne 0 ]]" +check "S9b: state status == aborted" \ + "[[ -f '$S9_STATE' ]] && [[ \$(jq -r '.status' '$S9_STATE') == aborted ]]" +check "S9c: convergence_status is null (no convergence verdict on an abort)" \ + "[[ -f '$S9_STATE' ]] && [[ \$(jq -r '.convergence_status' '$S9_STATE') == null ]]" +check "S9d: passes array has exactly the ONE applied pass" \ + "[[ -f '$S9_STATE' ]] && [[ \$(jq -r '.passes | length' '$S9_STATE') -eq 1 ]]" + # --------------------------------------------------------------------------- printf '%d/%d scenarios passed' "$PASSED" "$TOTAL" if (( PASSED != TOTAL )); then diff --git a/tests/run-all.sh b/tests/run-all.sh index dda0e59..6583ab8 100755 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -56,6 +56,7 @@ for t in "$SCRIPT_DIR"/*-simulation.sh; do done if [[ "$QUICK" == false ]]; then SUITES+=("$REPO_ROOT/evals/tests/scorer-verification.sh") + SUITES+=("$REPO_ROOT/evals/tests/bounce-scorer-verification.sh") fi # --- run ----------------------------------------------------------------------- diff --git a/tests/worktree-management-simulation.sh b/tests/worktree-management-simulation.sh index d81756d..03db548 100644 --- a/tests/worktree-management-simulation.sh +++ b/tests/worktree-management-simulation.sh @@ -140,7 +140,11 @@ init_repo() { runner="$REPO_ROOT/dev-review/codex/dev-review.sh" [[ -f "$runner" ]] || { echo "F: runner not found at $runner" >&2; exit 1; } - setup_line=$(grep -n 'if \[\[ -n "\$BRANCH_SPEC" \]\]; then' "$runner" | head -1 | cut -d: -f1) + # Match on the -n "$BRANCH_SPEC" test rather than the whole line: the + # setup block's if-condition also carries the PLAN_EXIT==0 guard (a failed + # plan phase must not spawn a stray branch/worktree) and may grow more + # clauses; the BRANCH_SPEC test is the stable anchor. + setup_line=$(grep -n -- '-n "\$BRANCH_SPEC" \]\]; then' "$runner" | head -1 | cut -d: -f1) capture_line=$(grep -n 'INITIAL_GIT_STATUS=\$(git -C "\$WORKDIR" status --short)' "$runner" | head -1 | cut -d: -f1) [[ -n "$setup_line" ]] \ @@ -151,6 +155,16 @@ init_repo() { || { echo "F: INITIAL_GIT_STATUS capture at line $capture_line happens BEFORE branch/worktree setup at line $setup_line — WR-04 regression (worktree mode would silently skip verify on dirty parent)" >&2; exit 1; } ) || fail "Scenario F (WR-04 capture-order regression)" +# --- Scenario G: a failed plan phase must not spawn a stray branch/worktree. +# Static check on the runner source: BOTH legs of the setup block must gate on +# PLAN_EXIT==0. Grep-anchored the same way as Scenario F. --- +( + runner="$REPO_ROOT/dev-review/codex/dev-review.sh" + guarded=$(grep -c 'PLAN_EXIT:-0}" -eq 0 && -n "\$\(BRANCH_SPEC\|WORKTREE_SPEC\)"' "$runner" || true) + [[ "$guarded" -eq 2 ]] \ + || { echo "G: expected both branch and worktree setup legs guarded by PLAN_EXIT==0, found $guarded — a failed plan phase could leave an orphan branch/worktree" >&2; exit 1; } +) || fail "Scenario G (failed plan must not create branch/worktree)" + if (( FAILURES == 0 )); then echo "ALL SCENARIOS PASSED" exit 0