Repository navigation
feat(hooks): enforce the mechanical rules with PreToolUse hooks - #553
Conversation
Quest-Claim: c04c502918737a36
quest setup installs a PreToolUse hook for Claude Code and Codex that denies tool calls breaking the rules a program can check: em dashes an agent adds, a git push without explicit refspecs, a force-push or rebase, a new GitHub post without its footer, and scratch writes under /tmp. Their prose leaves the hook's context and stays in `quest context` for agents without it. quest setup also writes Codex's SessionStart rules hook to hooks.json, with an additionalContextLimit that keeps the rules whole. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d shells, check only clone destinations Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…separate git dir Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Design change: Narrow the body sources. Drop the in-command file tracking (the (Written by Claude Opus 5.5) |
…rule-checks # Conflicts: # README.md # quest/m0/token-waste/README.md # src/cli/hooks.ts # src/cli/main.ts # src/runner/prompt.ts # test/hooks.test.ts
Drops the in-command file tracking that read a here-document's text as the body file's contents. A file an earlier command on the line writes now reads as unknown and passes; `--body-file -` with a here-document stays checked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 54 minutes. View limit details
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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
[P2] Do not inspect relative body files against a stale working directory (src/cli/rules.ts:203). shell() keeps the hook's original cwd for every simple command and does not handle cd. For example, if body.md in the initial directory lacks a footer but .tmp/body.md contains a valid post with its footer, cd .tmp && gh pr comment 5 --body-file body.md reads the former and is denied even though gh would read the latter. The same issue affects stdin redirection. This contradicts the narrow, fail-open behavior: an unrelated file can block a legitimate post. Track directory changes where they are known, or treat relative body sources as unknown after a command changes the directory. Add a regression covering two same-named files with different contents and a preceding cd.
Direction: fix this false denial before landing; the chosen policy of letting unknown body sources pass does not require broader shell emulation.
Static review of the 12-file diff, tests, and recorded design decision only. No shell commands, tests, builds, or live hook checks were run; runtime behavior and CI success are not certified.
(Written by review (OpenAI))
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Design change: Check inline bodies only. Stop reading body files from disk: check only (Written by Claude Opus 5.5) |
The maintainer's pick at the second round-3 design checkpoint: a body file may hold something else by the time the line posts it, so the hook checks only --body, body= fields, git commit -m, and --body-file - with a here-document. The per-line cd and write tracking goes with it, and the footer and em-dash rules stay in prose for bodies posted from a file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…refspecs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…target options Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop cp, mv, ln, rsync, and touch from the /tmp check, and pass any git command with a help flag anywhere in it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d28f1b34fdf # Conflicts: # src/runner/prompt.ts
|
Design change: the maintainer settled the third design checkpoint (review round 3 of the third session, head c25255e) on 2026-11-09 through /quest-complete. ✅ Narrow both checks (rejected: continue per-command parsing; merge as is; narrow the
Reason: it removes the class of findings (per-command option and path grammars) and some code, per the quest's rule that a check which cannot stay narrow stays prose. (Written by Claude Opus 5.5 with medium) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
The previous stale-working-directory finding is resolved: GitHub body files are no longer read from disk. The current attached short-option handling and --edit-last exemption also match the recorded narrowed design.
[P2] Do not classify a patch deletion as a scratch-file write
Location: src/cli/rules.ts:135-138
The header regex treats Delete File exactly like Add File and Update File, so a Codex apply_patch containing only "*** Delete File: /tmp/generated.txt" is denied by the scratch rule when the checkout is elsewhere. Removing an existing temporary file creates no scratch outside the checkout and can be legitimate cleanup of a file produced by a tool. This contradicts the hook's explicit promise not to block legitimate work. Retain the header kind and apply the scratch-write check only to operations that write a destination; add a regression showing that deleting a temporary file passes while adding one is still denied.
Direction: fix this false denial before landing. This needs only a distinction between patch operations, not broader shell emulation.
Verification limits: GitHub-only static review of the current 11-file diff, hook installation/dispatch, shell parsing and rule tests, and the recorded design decisions. The deletion example is a source-level trace, not an executed reproduction. No shell commands, repository code, tests, builds, or live hooks were run. At inspection, the build was in progress and check/queue jobs were skipped; those do not establish a full check pass.
(Written by review (OpenAI))

Finishes quest/m0/token-waste/rule-checks.md: the rules in
assets/CONTEXT.mdthat a program can check are now enforced by aPreToolUsehook instead of read as prose, for Claude Code and Codex, andquest setupgives Codex the sameSessionStartrules hook Claude Code got in moq-dev/quest#517.Behavior
quest setupinstalls one more entry per agent, each runningquest hook:settings.json):PreToolUseonBash|Write|Edit|MultiEdit, next to the existingSessionStartandSubagentStart.$CODEX_HOME/hooks.json, written only when that directory exists):SessionStart(startup|resume|clear|compact) withadditionalContextLimit: 10000, andPreToolUseonBash|apply_patch.quest setupprints a reminder to trust them through/hooks.In a repository with the
Quests:line,quest hookdenies a tool call, giving the rule as the reason, when it:>quote lines;git pushwith a refspec lacking:(or none), outside--delete,--tags,--all,--branches,--mirror;-fin any cluster,--force,--force-with-lease,--mirror, a+refspec), or runsgit rebaseorgit pull --rebase, unless the command starts withQUEST_USER_ASKED=1;git rebase --abort/--quitpass;gh pr create|comment|review,gh issue create|comment,gh apiwith abodyfield, notPATCH/PUT) with a body lacking a(Written by ...)line;/tmpoutside the checkout: redirections,tee,mkdir,git clone,git worktree add,mktempwithout a directory, andWrite/Edit/apply_patchpaths.A new
src/cli/shell.tssplits a command line into simple commands (quotes,&&/||/;/pipes/subshells, redirections, here-documents and here-strings), and the checks peelenv, assignments,command,timeout,direnv exec,nix develop --command,quest gate, andbash -cto reach the real command. The footer and em-dash checks read a body only from the command line itself:--body, a-f body=field,git commit -m, and--body-file -with a here-document. They never read a body file from disk.The checked prose left
assets/CONTEXT.md(now 9,546 characters).quest contextstill prints the rules, as a last## Checkedsection built from the same strings the hook denies with, so an agent without the hook still reads them; only the hook's context leaves it out.What stayed prose, and why
/tmpis checked./tmp(cp,mv,ln,rsync,touch): where each puts its destination is a per-command grammar (-t <dir>,-t.tmp,rsync -t,touch -r,ln's one-operand form) that kept producing false denials, soassets/CONTEXT.mdkeeps "Never write them to/tmp" in the Workspace prose.--body-file path,--body-file - < path,-F body=@path,git commit -F path) or piped intogh: passes unread, since the file may hold something else by the time the line runs, and every review finding in six rounds came from guessing it.assets/CONTEXT.mdkeeps a line next to "Pass PR bodies toghon stdin" saying such a body goes unchecked and still needs the footer and no em dashes.Choices made without asking
QUEST_USER_ASKED=1when the user asked in their own words. Rejected: keeping them prose (the habitual mistake goes unchecked), and Claude Code'saskdecision (Codex does not support it and lets the call through, and it stalls unattended sessions).>quote lines keep quoted ones.gh pr edit,gh issue edit,gh pr|issue comment --edit-last(even with--create-if-none), andgh api -X PATCHare not checked for em dashes or the footer, since an existing body holds other authors' text, such as a reviewer's record.(Written by ...), case-insensitive, with only whitespace after it. A sectionquest reviewappends later never trips it, since that is an edit.quest hookalready adds the context: repositories with theQuests:line./tmppaths inside the checkout holding the cwd pass, so a checkout that lives under/tmpkeeps working. A$TMPDIRpath passes, sinceTMPDIRmay name the checkout;mktempwithout a directory still counts as/tmpunless aTMPDIR=assignment in front of it says otherwise.gitcommand with-hor--helpanywhere in it, andmktemp --help/-u.SubagentStartentry; the quest's Plan asked only forSessionStartparity.Co-Authored-By:trailer were exempt from the footer; the hook never checks commits for it, so that clause went with the rule.Codex evidence
additionalContextLimitis in tokens: a hook printing 10,000 characters ofword(about 2,000 tokens) spilled tohook_outputsat a limit of 1000, but not at 3000 or the default. The limit is set to the rules' 10,000-character cap, which bounds their tokens.quest reviewkeeps user hooks out: in this checkout,codex execran the user's trustedSessionStarthook (hook: SessionStartin its output, its.direnv/codex-env.shwritten), andcodex exec --ignore-user-configdid not (no output line, no file). With--dangerously-bypass-hook-trust,hooks.jsondoes load under--ignore-user-config, so what keeps hooks out of reviews is that their trust lives inconfig.toml's[hooks.state], which the flag skips. No change toquest reviewwas needed.Public API
quest setupalso writes$CODEX_HOME/hooks.json(or~/.codex/hooks.json) when that directory exists, and adds thePreToolUseentry to Claude Code's settings. Rerunning it replaces the earlier entries.quest hookhandlesPreToolUseinput from either agent and prints apermissionDecision: "deny"on a broken rule.quest contextends with a## Checkedsection.Validation
just checkandjust testpass.test/rules.test.ts(parser, each rule's blocked and passing cases) andtest/hooks.test.ts(Codex target, handler settings,PreToolUsedispatch)..claude/settings.local.json,git push --dry-run origin mainwas denied with the refspec rule. Aquest hookcall takes about 0.1s.Decisions
ghcheck finds a post's body; the open one read a body file rewritten by an expanding here-document from its stale contents on disk), settled by the maintainer on 2026-11-08 in moq-dev/quest#553 (comment): ✅ narrow the body sources: drop the in-command file tracking, so a body file the same command line writes (by redirection,tee, or as the destination ofcp/mv/ln/rsync) reads as unknown and passes, while--body-file -with a here-document stays checked directly (rejected: keep in-command tracking and mark expanded writes as unknown; merge as is; drop the footer and em-dash checks onghcommands).cdon the line, then inside a nested shell after one), settled by the maintainer on 2026-11-08 in moq-dev/quest#553 (comment): ✅ check inline bodies only: never read a body file from disk, check--body,-f body=,git commit -m, and--body-file -with a here-document, and keep the footer and em-dash rules in prose for a body posted from a file (rejected: read a body file only whenghorgit commit -Fis alone on the line; keep theghpost checks as prose entirely; drop the quest). Reason: the quest's Plan says a check that cannot stay narrow stays prose. The per-linecdand write tracking went with it.git push --tags origin featurenow needs its refspec's:(only a tags-only push is exempt);mktemphonors aTMPDIR=assignment in front of it; (moq-dev/quest#553 review). Refuted: keeping the checked prose in the hook's context, since the quest's Goal removes it./tmpandgithelp checks), settled by the maintainer on 2026-11-09 through /quest-complete, recorded in moq-dev/quest#553 (comment): ✅ narrow both checks (rejected: continue per-command parsing; merge as is; narrow the/tmpcheck further to redirections andWrite/Edit/apply_patchonly). The/tmpcheck dropscp/mv/ln/rsync/touch, and anygitcommand with-h/--helpanywhere passes, accepting the rare false pass such asgit push -o --help origin main. Reason: removes the class of findings and some code, per the quest's rule that a check which cannot stay narrow stays prose.ghoptions read attached short values (-bDone,-fbody=Done,-XPATCH), and--edit-lastcounts as an edit.## Checkedsection went with it.quest setup --worktrees: the worktree hooks go only into Claude Code's settings;HookEntrycarries acommandbeside its handlersettings, which hold theSessionEndtimeout.Models
Size: M
Implementer: Claude Code, claude-opus-5-5, medium effort
Implementer: Claude Code, claude-opus-5-5, low effort (iterate, M)
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, M)
Suggested follow-ups
None worth a quest.
(Written by Claude Opus 5.5)
Review
Rewritten by
quest reviewafter each round.01a1c14f-fbf0-7c91-bc45-40d31897e86d, round 2Both reported issues are fixed at the head, and inspection of the incremental changes found no new landing blockers; no builds or tests were run.src/cli/rules.ts:390:Parse attached GitHub CLI short-option values;The GitHub body, body-file, title, field, and method parsers now enable the existing attached-short-option handling, covering inputs such as `-bDone`, `-fbody=Done`, and `-XPATCH`.src/cli/rules.ts:386:Distinguish comment edits from new comments;Commands containing `--edit-last` now bypass new-post checks, including with `--create-if-none`, consistent with the checker's policy of allowing ambiguous cases.