Skip to content

feat(hooks): enforce the mechanical rules with PreToolUse hooks - #553

Merged
kixelated merged 14 commits into
mainfrom
quest/m0/token-waste/rule-checks
Nov 9, 2026
Merged

kixelated merged 14 commits into
mainfrom
quest/m0/token-waste/rule-checks

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m0/token-waste/rule-checks.md: the rules in assets/CONTEXT.md that a program can check are now enforced by a PreToolUse hook instead of read as prose, for Claude Code and Codex, and quest setup gives Codex the same SessionStart rules hook Claude Code got in moq-dev/quest#517.

Behavior

quest setup installs one more entry per agent, each running quest hook:

  • Claude Code (settings.json): PreToolUse on Bash|Write|Edit|MultiEdit, next to the existing SessionStart and SubagentStart.
  • Codex ($CODEX_HOME/hooks.json, written only when that directory exists): SessionStart (startup|resume|clear|compact) with additionalContextLimit: 10000, and PreToolUse on Bash|apply_patch. quest setup prints a reminder to trust them through /hooks.

In a repository with the Quests: line, quest hook denies a tool call, giving the rule as the reason, when it:

  • adds an em dash in an edit, a written file, a commit message, or a new GitHub post, counted against the text it replaces and skipping > quote lines;
  • runs git push with a refspec lacking : (or none), outside --delete, --tags, --all, --branches, --mirror;
  • force-pushes (-f in any cluster, --force, --force-with-lease, --mirror, a + refspec), or runs git rebase or git pull --rebase, unless the command starts with QUEST_USER_ASKED=1; git rebase --abort/--quit pass;
  • posts to GitHub (gh pr create|comment|review, gh issue create|comment, gh api with a body field, not PATCH/PUT) with a body lacking a (Written by ...) line;
  • writes scratch under /tmp outside the checkout: redirections, tee, mkdir, git clone, git worktree add, mktemp without a directory, and Write/Edit/apply_patch paths.

A new src/cli/shell.ts splits a command line into simple commands (quotes, &&/||/;/pipes/subshells, redirections, here-documents and here-strings), and the checks peel env, assignments, command, timeout, direnv exec, nix develop --command, quest gate, and bash -c to 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 context still prints the rules, as a last ## Checked section 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

  • "Never write them elsewhere in or beside the checkout, or in a directory other agents share": a write outside the checkout cannot be told apart from legitimate work, so only /tmp is checked.
  • Scratch copied, moved, linked, synced, or touched into /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, so assets/CONTEXT.md keeps "Never write them to /tmp" in the Workspace prose.
  • When to merge the base (only on a conflict or a landing refusal): a judgment, not visible in one command.
  • Em dashes in the agent's replies to the user: no tool call to check. Removed from the always-loaded prose with the rest, as the quest decided.
  • The footer and em-dash rules for a body posted from a file (--body-file path, --body-file - < path, -F body=@path, git commit -F path) or piped into gh: 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.md keeps a line next to "Pass PR bodies to gh on stdin" saying such a body goes unchecked and still needs the footer and no em dashes.

Choices made without asking

  • Force-push and rebase: a check cannot know whether the user asked, so the denial says to prefix QUEST_USER_ASKED=1 when the user asked in their own words. Rejected: keeping them prose (the habitual mistake goes unchecked), and Claude Code's ask decision (Codex does not support it and lets the call through, and it stalls unattended sessions).
  • Em dashes count against the replaced text, so editing a file that already holds them passes, and > quote lines keep quoted ones. gh pr edit, gh issue edit, gh pr|issue comment --edit-last (even with --create-if-none), and gh api -X PATCH are not checked for em dashes or the footer, since an existing body holds other authors' text, such as a reviewer's record.
  • The footer is a last line matching (Written by ...), case-insensitive, with only whitespace after it. A section quest review appends later never trips it, since that is an edit.
  • The checks act only where quest hook already adds the context: repositories with the Quests: line.
  • /tmp paths inside the checkout holding the cwd pass, so a checkout that lives under /tmp keeps working. A $TMPDIR path passes, since TMPDIR may name the checkout; mktemp without a directory still counts as /tmp unless a TMPDIR= assignment in front of it says otherwise.
  • Help and dry-run modes pass: any git command with -h or --help anywhere in it, and mktemp --help/-u.
  • Codex gets no SubagentStart entry; the quest's Plan asked only for SessionStart parity.
  • Commit messages with a 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

  • additionalContextLimit is in tokens: a hook printing 10,000 characters of word (about 2,000 tokens) spilled to hook_outputs at 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 review keeps user hooks out: in this checkout, codex exec ran the user's trusted SessionStart hook (hook: SessionStart in its output, its .direnv/codex-env.sh written), and codex exec --ignore-user-config did not (no output line, no file). With --dangerously-bypass-hook-trust, hooks.json does load under --ignore-user-config, so what keeps hooks out of reviews is that their trust lives in config.toml's [hooks.state], which the flag skips. No change to quest review was needed.

Public API

  • quest setup also writes $CODEX_HOME/hooks.json (or ~/.codex/hooks.json) when that directory exists, and adds the PreToolUse entry to Claude Code's settings. Rerunning it replaces the earlier entries.
  • quest hook handles PreToolUse input from either agent and prints a permissionDecision: "deny" on a broken rule.
  • quest context ends with a ## Checked section.
  • Wire: none.

Validation

  • just check and just test pass.
  • New tests: test/rules.test.ts (parser, each rule's blocked and passing cases) and test/hooks.test.ts (Codex target, handler settings, PreToolUse dispatch).
  • Live in Claude Code: with the built hook in this checkout's ignored .claude/settings.local.json, git push --dry-run origin main was denied with the refspec rule. A quest hook call takes about 0.1s.

Decisions

  • Design checkpoint after review round 3 (three rounds of findings, all in how the gh check 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 of cp/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 on gh commands).
  • Second design checkpoint after review round 3 of the second session (scores 6, 10, 9; the findings after the first pick all came from reading body files from disk, first after a cd on 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 when gh or git commit -F is alone on the line; keep the gh post checks as prose entirely; drop the quest). Reason: the quest's Plan says a check that cannot stay narrow stays prose. The per-line cd and write tracking went with it.
  • Review fixes after the first pick: git push --tags origin feature now needs its refspec's : (only a tags-only push is exempt); mktemp honors a TMPDIR= 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.
  • Third design checkpoint after review round 3 of the third session (scores 7, 8, 8; head c25255e; every open finding came from per-command option and path grammars in the /tmp and git help 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 /tmp check further to redirections and Write/Edit/apply_patch only). The /tmp check drops cp/mv/ln/rsync/touch, and any git command with -h/--help anywhere passes, accepting the rare false pass such as git 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.
  • Review fixes in the fresh session: gh options read attached short values (-bDone, -fbody=Done, -XPATCH), and --edit-last counts as an edit.
  • Merging main's deletion of the runner (moq-dev/quest#575): the runner prompt's ## Checked section went with it.
  • Merging main's quest setup --worktrees: the worktree hooks go only into Claude Code's settings; HookEntry carries a command beside its handler settings, which hold the SessionEnd timeout.

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 review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1c14f-fbf0-7c91-bc45-40d31897e86d, round 2
  • Head: 44997f1
  • Base: origin/main at eaa9c96
  • Score: 10/10, Both reported issues are fixed at the head, and inspection of the incremental changes found no new landing blockers; no builds or tests were run.
  • Findings: none
  • The last round's findings, at 5e28d47:
    • Fixed: medium, 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`.
    • Fixed: medium, 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.

@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

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>
@kixelated kixelated changed the title Mechanical rules become checks feat(hooks): enforce the mechanical rules with PreToolUse hooks Nov 7, 2026
kixelated and others added 2 commits November 7, 2026 13:59
…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>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: Narrow the body sources. Drop the in-command file tracking (the written map): a body file that the same command line writes reads as unknown and passes, while --body-file - with a here-document stays checked directly. The maintainer picked this at the round-3 design checkpoint through /quest-unblock on 2026-11-08 (rejected: keep in-command tracking and mark expanded writes as unknown; merge as is; drop the footer and em-dash checks on gh commands).

(Written by Claude Opus 5.5)

kixelated and others added 2 commits November 7, 2026 20:47
…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>
@kixelated
kixelated marked this pull request as ready for review November 8, 2026 04:56
@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2e4bb05d-c217-4169-a29d-963433b2acd6
Commits

Reviewing files that changed from the base of the PR and between fcca33e and 44997f1.

Files selected for processing: 11

  • README.md
  • SETUP.md
  • assets/CONTEXT.md
  • quest/m0/token-waste/README.md
  • quest/m0/token-waste/rule-checks.md
  • src/cli/hooks.ts
  • src/cli/main.ts
  • src/cli/rules.ts
  • src/cli/shell.ts
  • test/hooks.test.ts
  • test/rules.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Head: e100158
Base: 738bbde

[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>
@kixelated
kixelated marked this pull request as draft November 8, 2026 05:12
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: Check inline bodies only. Stop reading body files from disk: check only --body, --body-file - with a here-document, and git commit -m, the pattern the rules already ask for. A body file posted from disk goes unchecked, and the footer and em-dash rules stay in prose for that case. The maintainer picked this at the second round-3 design checkpoint through /quest-unblock on 2026-11-08, following the quest's own rule that a check which can't stay narrow stays prose (rejected: read a body file only when gh or git commit -F is alone on the line; keep the gh post checks as prose entirely; drop the quest).

(Written by Claude Opus 5.5)

kixelated and others added 3 commits November 7, 2026 22:06
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>
kixelated and others added 2 commits November 9, 2026 09:14
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
@kixelated

Copy link
Copy Markdown
Collaborator Author

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 /tmp check further to redirections and Write/Edit/apply_patch only).

  • The /tmp check drops cp, mv, ln, rsync, and touch, keeping redirections, tee, mkdir, mktemp, git clone/git worktree add, and Write/Edit/apply_patch paths. assets/CONTEXT.md again says never to write scratch to /tmp, covering what the hook no longer sees.
  • Any git command with -h/--help anywhere in it passes, accepting the rare false pass (such as git push -o --help origin main), since a missed nudge is not a block.

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
kixelated marked this pull request as ready for review November 9, 2026 17:22
@kixelated
kixelated added this pull request to the merge queue Nov 9, 2026

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Head: 44997f1
Base: fcca33e

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))

Merged via the queue into main with commit 37963bf Nov 9, 2026
20 checks passed
@kixelated
kixelated deleted the quest/m0/token-waste/rule-checks branch November 9, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant