fix: deep-explore findings — test glob, review-gate hardening, notebook support#7
Draft
sd0xdev wants to merge 17 commits into
Draft
fix: deep-explore findings — test glob, review-gate hardening, notebook support#7sd0xdev wants to merge 17 commits into
sd0xdev wants to merge 17 commits into
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.
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