Repository navigation
feat(hooks): template-derived wish validator + corpus lint gate (hooks-v2#validator) - #2785
Conversation
…s-v2#validator) The blocking write check now derives its rules from the canonical wish template instead of vibecode regexes that predated the template's digit groups and table-row metadata. - validate-wish.ts: the template fixture (skills/wish/templates/ wish-template.md) is embedded in the shipped bundle by esbuild's text loader, and every structural rule is parsed from it — required ## sections, the Scope IN/OUT subsections, the '### Group N:' heading shape (digits AND named groups), the '# Wish:' title, and the checkbox item syntax. Status is read from the template's metadata table row (legacy '**Status:**' line tolerated). The checkbox rule exempts completed docs (SHIPPED/DONE/EXECUTED). Legacy formats are tolerated by date: docs predating the template's current form (2026-07-29, b0e77cd) need only a title+status signature, while missing/invalid dates cannot bypass the strict contract. - PreToolUse validates the PROPOSED write result (Write tool_input. content, simulated Edit), so fixing a broken doc is never blocked — only writes whose result is broken are. - The read is bounded and refuses symlinks (readBoundedWishFile, 256 KiB cap, matching the board). - wishes-lint.ts runs the validator over every WISH.md, so the CI lint gates the corpus in both repos (remotty corpus gate: the sibling corpus-repair PR arms a wish-validator workflow in the remotty repo). - hook-bundle-parity.ts gains the .md text loader so the bundled .cjs tracks the template byte for byte; template drift fails CI until the bundle is regenerated. - New validate-wish.test.ts beside the source (dispatch-runtime.test.ts pattern): template pinning, digit/named groups, table-row status, checkbox exemption, legacy tolerance, symlink/size-cap rejects, hook-event CLI behavior, and the whole current genie corpus.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: ccee440cd7
ℹ️ 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".
| for (const issue of result.issues) { | ||
| console.error(` - line ${issue.line}: ${issue.message}`); | ||
| } | ||
| return 1; |
There was a problem hiding this comment.
In the PreToolUse registration in plugins/genie/hooks/hooks.json, this command is described as a blocking write check, but returning 1 only reports a non-blocking hook error, so Claude can still execute a Write/Edit whose proposed wish fails validation. Claude's hook documentation classifies exit code 2 as a “blocking error” and other nonzero codes as a “non-blocking error”; return 2 or emit an explicit deny response for the PreToolUse path instead. See Claude Code hook exit-code behavior.
Useful? React with 👍 / 👎.
| for (const section of CONTRACT.sections) { | ||
| if (!new RegExp(`^##\\s+${escapeRegExp(section)}\\s*$`, 'm').test(content)) { | ||
| issues.push({ line: 1, message: `Missing required section: ## ${section}` }); |
There was a problem hiding this comment.
For post-contract wishes containing Markdown examples, these raw multiline regexes treat headings inside fenced code blocks as real sections, even though the contract parser itself explicitly skips fences. A document can therefore omit an actual required section while satisfying it with a fenced ## ... example; conversely, findHeading can select a fenced ## Success Criteria before the real section and incorrectly reject an otherwise valid document. Reuse the fence-aware line scan for required headings and semantic section lookup.
Useful? React with 👍 / 👎.
…sals (hooks-v2#validator) Execution review FIX-FIRST (loop 1): - HIGH: hook-driven paths (PreToolUse/PostToolUse stdin JSON) now exit 2 on validation failure and on symlink/size-cap refusals — Claude Code treats command-hook exit 2 as the blocking failure (dispatch-runtime contract). The --file CLI keeps its documented exit 1. Tests moved to exit 2 with honest names, and a hook-path symlink refusal test proves the refusal is blocking too. PostToolUse exit 2 feeds the violations back to the agent on stderr. - LOW: readWishFile no longer fails open on stat errors — ENOENT and ENOTDIR are the only 'missing' cases (new wish); every other errno (EACCES, ELOOP, I/O) is a named refusal. Regression test added.
hooks-v2 — Group validator: validate-wish that matches the template
Goal: The blocking write check validates against the canonical template, not vibecode regexes.
Deliverables
1. Template-derived parser —
plugins/genie/scripts/src/validate-wish.tsrewritten. The canonical template fixture (skills/wish/templates/wish-template.md) is embedded in the shipped.cjsbundle by esbuild's text loader, and every structural rule is parsed from it at load time:##sections + the Scope### IN/### OUTsubsections (the template's own headings),### Group N:heading shape derived from the template's group heading — digit groups (the template's form) and named groups both pass,| **Status** | … |), with the legacy**Status:**line tolerated,- [ ]/- [x]items and exempts completed docs (SHIPPED / DONE / EXECUTED),b0e77cd15) need only a status signature; missing/invalid dates cannot bypass the strict contract.2. CI lint over every current WISH.md —
scripts/wishes-lint.tsnow runs the validator on everyWISH.mdit scans, sobun run wishes:lint(already in CI) gates the genie corpus. The remotty corpus is gated by a newwish-validatorworkflow that ships in the sibling remotty corpus-repair PR (remotty is private, so its own CI fetches the public genie gate script at main HEAD and lints.genie/wisheson every wish-doc change — deliberately unpinned so the corpus tracks the current template).3. Symlink reject + size cap — the read is
readBoundedWishFile(the same lstat+O_NOFOLLOW+fstat primitive the board uses) with a 256 KiB cap. Symlinks, oversized files, and non-regular paths are refused with named errors, while a missing file (new wish) skips validation. Stat failures other than ENOENT/ENOTDIR are named refusals, not silent skips.Blocking semantics (execution-review fix): hook-driven paths (PreToolUse/PostToolUse stdin JSON) exit 2 on validation failure and on symlink/size refusals — Claude Code treats command-hook exit 2 as the blocking failure (dispatch-runtime contract) — while
--fileCLI mode keeps the documented exit 1. PostToolUse exit 2 feeds the violations back to the agent on stderr.4. Corpus repairs — shipped in the sibling remotty PR (automagik-dev/remotty#38): headless-turn-open depends-on/blocks prose → cross-wish note; roadmap-truth design-evidence re-review + re-stamp; agent-parity
MERGED→SHIPPED; plus genie-board's two colon-drifted**Validation**labels surfaced by the new validator.New-wish wedge, closed
The old validator demanded
### Group [A-Z]:while the template ships digit groups — every template-shaped write failed. Now the raw template validates clean (pinned by test), and PreToolUse validates the proposed write result (Write tool_input.content, simulatedEdit), so fixing a broken doc is never blocked — only writes whose result is broken are.Validation
bun run check:fastgreen (typecheck, lint, dead-code, skills/wishes/council/budget lints, hook-bundle parity, hook-content, plugin-executables).bun testshows no new failures vs the clean-dev baseline (the only deltas are the known doctor-latency env failure and network-bound release-asset timeouts, both also present in baseline runs).Template drift now fails CI twice over: bundle parity (the embedded template must be regenerated) and the corpus lint (docs must follow the template).
Orchestrator merges; worker never merges.