Repository navigation
fix(review): prevent Fable built-in tools before required review #167
Description
Goal
Make the required Fable High review lane tool-free by construction so a valid frozen candidate is not repeatedly blocked after Claude/Fable invokes built-in Bash or advisor tools despite the existing denial flags.
Severity
P0 release blocker. This now blocks #109 and therefore the 1.0.0 release train. No review exception is available.
Evidence
The fail-closed detector from #158 is working, but prevention remains unreliable:
- During fix(review): stop tool-free cross-model reviews when Claude invokes advisor #158, Claude Code 2.1.221 was launched with an empty tool set, advisor explicitly disallowed, safe mode, no slash commands, no MCP servers, and strict MCP configuration. Fable still emitted server-side advisor tool use.
- fix(review): stop tool-free cross-model reviews when Claude invokes advisor #158 merged with its one-time bootstrap exception consumed.
- The immediately preceding Dual-review should distinguish infrastructure failure from reviewer verdict #157 final gate succeeded normally: Fable High completed and certified in 175930 ms; Sol High certified in 285605 ms.
- On the post-Dual-review should distinguish infrastructure failure from reviewer verdict #157 Complete the bounded incremental-review pilot and final candidate gate #109 gate, Fable High emitted forbidden Bash tool use after 122125 ms. The runner correctly classified this as non-transient invalid execution, wrote an explicitly non-certifying partial receipt, cancelled the incomplete Sol peer, and recommended no retry.
- Complete the bounded incremental-review pilot and final candidate gate #109 candidate tree
6f7c46cda02685feaf27eabe45e83c7fd1d604b2remains unchanged with canonical 11/11 proof; no commit, push, or PR occurred.
This is not auth, quota, timeout, or a candidate verdict. It is a recurring provider-control failure. Detection protects correctness but does not provide a usable required review lane.
Required invariant
A reviewer required to be tool-free must either be launched through a mechanism that actually makes forbidden tools unavailable, or fail before candidate review with an honest non-certifying classification. Prompt instructions and observed CLI flags are not sufficient proof of enforcement.
Acceptance criteria
- Add captured RED fixtures for nested Fable/Claude Bash tool use as well as server-side advisor tool use.
- Audit the complete Fable execution surface and explicitly deny every built-in/client/server tool class except the one structured-response mechanism the gate requires.
- Ensure tool identity normalization cannot bypass denial through case, namespace, alias, or nested stream shape.
- Add a bounded live preflight or equivalent capability check that fails before the expensive joint candidate review when the provider cannot honor the tool-free contract.
- Never classify forbidden tool use as auth, quota, timeout, model unavailability, or a retryable infrastructure failure.
- Preserve any reviewer result that completed before its peer failed, while keeping every partial receipt explicitly non-certifying.
- Do not fall back, retry, waive, or fabricate certification for forbidden tool use.
- Focused proof, one canonical broad proof, Sol High, and Fable High all pass before delivery.
- Resume Complete the bounded incremental-review pilot and final candidate gate #109 only after this prevention contract is integrated and verified on the actual configured Fable lane.
Related
- fix(review): stop tool-free cross-model reviews when Claude invokes advisor #158 owns fail-fast detection and is complete.
- Dual-review should distinguish infrastructure failure from reviewer verdict #157 owns truthful outcome classification and partial receipts and is complete.
- Complete the bounded incremental-review pilot and final candidate gate #109 is the blocked release candidate.
Activity
BaseInfinity commentedon Aug 23, 2026
RED checkpoint (2026-08-23)
Clean worktree:
/Users/stefanayala/codex-sdlc-issue167Branch/base:
fix/issue-167-prevent-fable-toolsata07fc3d3fb77767de4c6e677e24bce628256668aDriver: Sol High (explicit owner selection)
Writable RED added:
tools: []and return the exact plain-JSON acknowledgement.Focused command:
node tests/test-review-supervision.cjsObserved RED:
Expected 'forbidden reviewer tool use: StructuredOutput'; actual ''at
testClaudeToolUseDetectorRejectsEveryToolIncludingStructuredOutput.No implementation edit, commit, push, or PR yet. Dirty root and frozen #109 worktree remain untouched.
BaseInfinity commentedon Aug 23, 2026
GREEN / pre-canonical checkpoint (2026-08-23)
Clean worktree:
/Users/stefanayala/codex-sdlc-issue167Branch/base:
fix/issue-167-prevent-fable-toolsata07fc3d3fb77767de4c6e677e24bce628256668aDriver: Sol High.
Implemented:
tools: [], exact acknowledgement JSON, and expected model identity.tools: []; nested/case/namespaced tool-use events remain fail-fast invalid execution.ROADMAP.mdto the live milestone: 9 open / 13 completed, ordered fix(review): prevent Fable built-in tools before required review #167 -> Enforce Sol High implementation and High-only planning/review #86 -> Complete the bounded incremental-review pilot and final candidate gate #109.Focused proof (all green):
node tests/test-review-supervision.cjsbash tests/test-roadmap.sh(15/15)git diff --checkAuthor exact-diff self-review completed. Dirty root remains unchanged; frozen #109 remains untouched. No commit, push, PR, or review exception. Next: the single canonical broad proof, then enforced Sol High followed by Fable High.
BaseInfinity commentedon Aug 23, 2026
Canonical proof checkpoint (2026-08-23 11:24 PDT)
Exact staged candidate tree:
b56b9f62f5c5c9d59a997f59e48a4e4bdba0f1ddHEAD/base:
a07fc3d3fb77767de4c6e677e24bce628256668aSingle canonical command:
node .codex/hooks/git-guard.cjs prove --reviewed --check "node scripts/run-proof-suite.cjs"Result: 11/11 PASS; fresh proof verified and bound to the staged candidate. Key durations: adapter 354.0s, setup 292.1s, update 696.5s. No unstaged tracked changes.
Next action: run the current-main exact-candidate dual gate with Sol High and required Fable High. No review exception is available.
BaseInfinity commentedon Aug 23, 2026
First live dual-review checkpoint (2026-08-23)
Candidate tree
b56b9f62f5c5c9d59a997f59e48a4e4bdba0f1ddwas not certified. No commit/push/PR.Prevention result:
Both reviewers independently found the same valid P1: the top-level preflight exits on genuine Fable quota/model unavailability before the existing independently-confirmed Opus fallback can run. Sol also found that selected/fallback preflights reuse one log path.
Next RED/GREEN:
No exception or waiver is available or requested.
BaseInfinity commentedon Aug 23, 2026
Certified candidate checkpoint (2026-08-23 11:48 PDT)
Corrected exact tree:
f3ce7e877270adb77b57a93f9e6b5c4fb1995bbePatch:
sha256:d589651895e5b96375ce6fb87ba92430455cdc3581d81d1b6554c53194bc35ecEvidence:
CERTIFIED; no reconciliation needed;Non-blocking review notes remain P2/P3 under normal verdict rules. The main substantive follow-ups are standalone
fable-review.cjsprocess-tree bounding and certifying-run model-identity validation; they do not weaken the certified dual gate or block #167 delivery and will be tracked separately rather than reopening this release-blocking candidate.Next: fixed-argv GitHub delivery, required CI verification, and post-merge main verification.
BaseInfinity commentedon Aug 23, 2026
Final post-merge checkpoint
#167 is complete with no review exception.
The nonblocking standalone-adapter P2 findings are durably tracked in #171 under the 1.1.0 milestone. They do not weaken or reopen the certified dual-review path delivered here.