fix: deep-explore findings — test glob, review-gate hardening, notebook support - #7
Merged
Merged
Conversation
- Switch reconciliation to git status -uall in 4 hooks (stop-guard, post-skill-auto-loop, post-compact-auto-loop, user-prompt-review-guard) so new untracked files no longer downgrade has_code_change - stop-guard: fail closed on transcript-missing with pending state or .blocked sidecar, sidecar-only with readable transcript, jq-missing sidecar/dual-mode, and present-but-unreadable state file - Bound -uall reconciliation walk with timeout, fix SIGPIPE downgrade via here-string, fail-closed skip when no timeout helper is available - post-edit-format: classify sh/bash/zsh as code so hook edits engage the review gate - Add mutation-pinned regression tests incl. new post-skill-auto-loop suite
Option C two-plane architecture (agent-driven control plane, hook-owned safety plane); v1 scoped to report-only /orchestrate. Records Spike 1/1b verification results and residual fail-open gap closures (-uall, transcript-bypass, sidecar corners) as mutation-orchestration preconditions.
A1+B1+C2 architecture from Codex equilibrium debate: skill-driven /plan-review trigger, namespaced plan_review state field with its own iteration history, deep tier delegated to /codex-brainstorm. Includes feasibility study, requirements, and tech spec.
Skill-driven /plan-review (A1+B1+C2): Codex attacks the plan draft in plan mode and the loop iterates to convergence before ExitPlanMode. State lives in the namespaced plan_review.* subtree of .claude_review_state.json (schema v2->v3 additive migration) with its own iteration budget, fully isolated from the code/doc/aggregate planes. Gate transitions flow through scripts/emit-plan-gate.sh; stop-guard strips plan sentinels first so they never satisfy code/doc gate patterns, and treats pending plan review as warn-only. Adds "## Plan Review" / "## Plan Review Max Rounds" opt-ins to auto-loop-project.md plus regression tests for the new state paths.
… request Tech spec revised to as-built behavior: init_state_file emits schema v3 directly (v2 files upgraded via migration), MCP sentinel routing gains the Priority 1.5 plan branch with token-before-verdict precedence, emit-plan-gate.sh usage/argument validation, and the single-owner history write path. Request ticket records the W0-W4 work breakdown with Done status.
…ly v1) Declarative intent -> planner agent (Explore, admission-constrained) derives a repo-state-aware plan -> human preview -> read-only fanout -> pre/post no-mutation verification (fail-closed) -> report lands through the existing doc review loop. Control-plane run-state lives in gitignored .claude_workflows/<run-id>.json, fully separated from the hook-owned safety plane (.claude_review_state.json is read-only to the orchestrator); v1 ships zero orchestrate hook changes. Includes plan schema, validator, admission allowlist, run-verify/plan-context scripts, and unit + schema tests.
Option C two-plane tech spec for report-only /orchestrate: W1-W5 work breakdown, fail-closed pre/post verification policy, planner admission allowlist, and the .gitignore precondition for .claude_workflows/. Request ticket tracks v1 scope with Candidate Complete status.
… skill dirs generate-readme-catalog now counts untracked-but-not-ignored skill dirs (git ls-files --cached --others --exclude-standard) so newly added skills are no longer undercounted in the window between adding a skill and committing it. Registers both new skills in skill-catalog.yml, bumps README hero/coverage counts to 98 bundled/public, and adds the commands to both CLAUDE quick-reference tables.
npm executes scripts via /bin/sh where test/**/*.test.js does not recurse (globstar off), silently dropping 538 tests in nested directories from every npm test / test:ci run. Switch to $(find test -name '*.test.js') and add a regression test that reproduces the /bin/sh expansion and pins union == disk.
…rites, notebook support - post-tool-review-state: scan all JSON gate markers (BLOCKED wins) instead of first-fence-only parsing; Skill 'Launching skill:' placeholders no longer write state (double-count fix); lock contention now skips with stderr note instead of falling back to unlocked read-modify-write; changed_files reset and review_phase idle transition moved under the lock; REVIEW_STATE_LOCK_TIMEOUT override; jq-missing degradation warned on stderr - post-edit-format/pre-edit-guard: honor NotebookEdit's notebook_path (edits no longer bypass gates and guards); prettier requires an installed binary plus project config — never npx-fetches from the network per edit - stop-guard: strict-mode stderr now carries actionable next-step guidance; .ipynb counted as code in porcelain and transcript reconciliation; NotebookEdit included in transcript tool filter; jq-missing no-state message notes gates are UNENFORCED - post-compact/post-skill/user-prompt hooks: .ipynb in code-extension regex - session-init: remove orphan .blocked sidecar on session change/create Each behavior change lands with regression tests (contention, placeholder, multi-fence, notebook, prettier opt-in, sidecar cleanup).
…rds, catalog counts - commit-msg-guard: bound AI/GPT with \b so ordinary words (maintainer, domain, GPTools) stop false-positiving while true positives still block; verified on BSD and GNU grep; new 14-case test suite - precommit-runner: missing scripts recorded as explicit skip entries; a run where every step is skipped now passes (parity with verify-runner) instead of wedging the strict stop gate on script-less repos - generate-readme-catalog/README: hook and script counts corrected (8/17) with inventory-pinning tests - session-scope-resolver: session_id check documented as internal consistency, not a live-session probe - orchestrate/plan-review tests: skip .claude/ assertions when the gitignored local install state is absent (worktrees/CI)
…mmand guidance - mirror the word-bounded forbidden patterns in create-pr / smart-commit SKILL docs, execute-mode reference, and both tech-specs (canonical source stays scripts/commit-msg-guard.sh); drop stale line-number references - dep-audit: allowed-tools switched to valid colon-form Bash rules (Bash(npm audit:*) etc.) with a frontmatter-pinning test - CLAUDE.md: test command updated to npm test with a note on the /bin/sh globstar pitfall
…nd false-verdict cases - hooks.json: add NotebookEdit to the edit matchers so notebook edits gate reviews - post-tool-review-state: parse the machine gate only from json fences so prose gate strings no longer over-block; treat precommit "NO CHECKS RUN" as a non-verdict so stop-guard cannot wedge on a check-less repo - session-init: preserve the .blocked sidecar when git status fails (tri-state fail-closed), deleting only on a proven-clean or non-git tree - add code-extension consistency test; expand hook tests for the above
… disk-derived catalog counts - commit-msg-guard: keep GPT/OpenAI unbounded to catch ChatGPT/GPT-4 while word-bounding only AI; add ChatGPT/OpenAI regression coverage - precommit-runner: an all-skip run now reports "NO CHECKS RUN" instead of a false-green PASS so the merge gate fails closed - generate-readme-catalog: derive hook/script/agent/rule counts from disk inventory instead of hard-coded literals
…h-specs, and skills - README (5 locales): correct hook count to 8 and script count to 17 in both the What's-included table and prose to match disk - tech-specs and skill docs: state only AI is word-bounded and GPT/OpenAI are intentionally unbounded; clarify the boundary is a GNU/PCRE extension, not POSIX - precommit / precommit-fast skills: handle the runner's "NO CHECKS RUN" third state by falling through to ecosystem detection
…ckstop run-verify's snapshot/compare hashed nine git-scoped fields, but none covered .git/hooks/* or .git/info/exclude. A read-only fanout worker could plant a .git/hooks/pre-commit persistence payload, or append to .git/info/exclude to hide a matching untracked write from both `status --porcelain` and `ls-files --exclude-standard`, defeating the "no git-scoped change" proof. Add git_internals_sha256: hash .git/hooks/* (content + exec bit, symlinked hooks included) and .git/info/exclude, resolved via `git rev-parse --git-path` (worktree-safe; follows effective core.hooksPath since Git 2.10). hash-object is batched in chunks of 500 to stay under E2BIG; any git failure fails closed. Regression tests plant a pre-commit hook, an info/exclude entry plus a matching write, and a chmod +x — each drifts git_internals while porcelain and ls-files stay blind. tech-spec and admission-allowlist document the new coverage.
…ate-plan FORBIDDEN_SENTINELS listed only five gate markers, so a serialized plan could smuggle plan-review Ready/Blocked, doc-review Needs-revision, or precommit Overall headers past the hook parsers. Expand to twelve entries aligned with the full hook sentinel set. Replace recursive DFS cycle detection with an iterative Kahn topological sort over a first-occurrence id map: depends_on must form a DAG, and deep chains no longer risk stack growth. plan-schema.md documents both rules — S1 names the sentinels by bare form to satisfy the self-compliance guard, and SCHEMA adds the DAG requirement.
…iew; default single Codex and updated docs/prompts/tests - Introduces tier-based auto-loop gating: fast (P0), standard (P0/P1), thorough (P0/P1/P2) with per-tier round caps (3, 5, 10). 80 is a passing grade; sub-threshold findings are logged as NIT_DEFERRED and do not re-open the loop. - Keeps Codex as the default single reviewer; dual-review is now opt-in via /codex-review-branch --dual (and mirrored in plan-review variant). This de-emphasizes always-on dual dispatch and reserves it for releases or security-critical reviews. - Updates terminology and structure: replaces dual-review emphasis with tiered/review-by-tiers architecture (The Four Anchors) across docs, prompts, and references; aligns gate logic with tier semantics. - Updates review prompts and calibration: Codex invocation prompts now mandate independent research and include a Severity Calibration section (bidirectional checks). Doc prompts also updated to reflect evaluation and sub-threshold handling. - Broad doc and UI wording refreshed across CLAUDE.md, READMEs (including translations) to reflect single-review default, tier-based blocking, and opt-in dual-review paths. - Tests and test coverage updated: new tests for review dispatch behavior; expectations adjusted from hard-coded P0/P1 to “no blocking findings” semantics where appropriate; dual-review tests re-scoped to the opt-in branch behavior; ensures dual-mode behavior is gated behind the branch flag. - Minor refactors to ensure consistency of gate semantics, deferral handling, and anchor naming (e.g., The Four Anchors, tier-based gating) across rules and skill references.
- Bump version to 4.0.0 across package.json, install-state.json, and related metadata. - Update marketplace and plugin descriptions to drop "dual" review wording and reflect gate-by-tier approach.
…s/prompts/tests
Shifts gating logic to a tier-based auto-loop (single Codex by default) with an opt-in dual-review path, and updates the docs, prompts, and tests to reflect the new model.
- Clarifies blocking behavior in the Codex gate: late findings above BLOCKING influence re-review; branch review is always thorough.
- Removes ambiguous dual-review emphasis in references and renders variables (e.g., ${BLOCKING}) without escape characters.
- Strengthens Codex/secondary handling guidance in review-common.md to state Codex is the gate and secondary results are advisory when blocked.
- Updates tests to accommodate the new gating semantics and adds notes for flaky Linux CI behavior, with targeted skips explained.
Files touched include: SKILL.md, codex-prompt-fast.md, codex-prompt-full.md, review-common.md, and related test files.
sd0xdev
marked this pull request as ready for review
July 26, 2026 07:51
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c8c25210f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const minted = crypto.randomBytes(TOKEN_BYTES).toString('hex'); | ||
| // `wx` so an existing marker is never silently re-minted: re-claiming a live directory would | ||
| // invalidate the token its real owner is holding and orphan the directory permanently. | ||
| fs.writeFileSync(claim, `necessity-audit scratch\ntoken=${minted}\npid=${process.pid}\n`, { |
There was a problem hiding this comment.
When two audits overlap and <AUDIT_TMP_DIR> is mistakenly substituted with the other run’s still-empty tmp.* directory, the shape and emptiness checks pass and this line stamps that foreign directory. The returned token then authorizes its later deletion, so the claim does not prevent the wrong-directory substitution it was introduced to guard against; it only prevents switching directories after claiming. Have this script create the temporary directory and marker atomically and return both the path and token.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all verified findings from the deep-explore audit (4 P1 + 8 P2 + nits) in four grouped commits.
P1
test/**/*.test.jsdoes not recurse — 538 nested tests were silently skipped in every CI run. Now$(find test -name '*.test.js'), with a regression test reproducing the sh expansion.P2 / hardening
Verification