Repository navigation
fix(hooks): name the launch route when quest is missing from a hook's PATH - #544
Conversation
Quest-Claim: b5ddfaa3ac50870b
… PATH The documented worktree hook entries ran a bare `quest worktree hook`, so a Claude Code session without quest on its hook PATH failed with the shell's `quest: not found` and no guidance. Each entry now checks for quest first and otherwise prints how to fix it, naming `nix develop --command claude`, and exits 1. Exit 1 rather than 2: Claude Code keeps a subagent running on exit 2 from SubagentStop, so a missing quest would loop it, while WorktreeCreate fails on any non-zero exit. The test runs the README's entry with and without quest on PATH. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 7e42174 against base 1c3cf6a.
No actionable findings. The four documented hook commands guard lookup before invoking Quest, keep the diagnostic on stderr, and leave stdin and the actual hook invocation untouched when Quest is available. The test reads the README JSON itself, verifies the complete HOOK_EVENTS set shares the command, and exercises both missing and present executable paths. SETUP.md still links to these entries, and the completed quest and index link are removed consistently.
Direction: this is ready for the normal validation/merge process from this review's scope. Existing installations need to adopt the updated entries; this does not automatically rewrite user settings.
Verification limits: GitHub-only static inspection of the diff, surrounding README/SETUP guidance and hook implementation. I did not run tests or launch Claude Code, and did not independently reproduce how each Claude Code version displays hook failures. Windows execution is not covered by the added test.
(Written by review (OpenAI))
|
quest land watch: the merge queue removed this pull request at 7e42174 because it conflicts with The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs. (written by quest land watch) |
|
quest land watch: the merge queue removed this pull request at 12ebe7e because its The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
…ms�[22m�[39m
�[33m�[2m✓�[22m�[39m hands back a fixer's resolution of a protected conflict as a ready commit, never pushing it�[33m 2609�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m hands back a protected fix that merges no base as a commit on the PR head�[33m 3185�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m refuses a PR whose head moved since the handback�[33m 4773�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m trusts only an App's resolution�[33m 3685�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m refuses a resolution whose tree the rebuild does not reach�[33m 926�[2mms�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Tests 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m test/worktree.test.ts�[2m > �[22mClaude Code hooks�[2m > �[22mthe documented entries say so when quest is not on PATH
�[31m�[1mAssertionError�[22m: expected 127 to be +0 // Object.is equality�[39m
�[32m- Expected�[39m
�[31m+ Received�[39m
�[32m- 0�[39m
�[31m+ 127�[39m
�[36m �[2m❯�[22m test/worktree.test.ts:�[2m1120:26�[22m�[39m
�[90m1118|�[39m const input = JSON.stringify({ hook_event_name: "WorktreeCreate", cw…
�[90m1119|�[39m const present = run(`${bin}:/bin:/usr/bin`, input);
�[90m1120|�[39m expect(present.status).toBe(0);
�[90m |�[39m �[31m^�[39m
�[90m1121|�[39m expect(present.stdout).toBe(`quest worktree hook\n${input}`);
�[90m1122|�[39m });
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m29 passed�[39m�[22m�[90m (30)�[39m
�[2m Tests �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m1104 passed�[39m�[22m�[2m | �[22m�[33m6 skipped�[39m�[90m (1111)�[39m
�[2m Start at �[22m 13:01:26
�[2m Duration �[22m 306.16s�[2m (tests 80%, transform 17%, import 2%)�[22m
##[error]AssertionError: expected 127 to be +0 // Object.is equality
- Expected
+ Received
- 0
+ 127
❯ test/worktree.test.ts:1120:26
error: script "test" exited with code 1
error: recipe `_test` failed on line 33 with exit code 1
error: recipe `test` failed on line 30 with exit code 1
##[error]Process completed with exit code 1.
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/1bce2c87-30ed-4246-b33a-15dc332b24d9' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/quest/quest
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/95af9e5a-dde2-4d3d-b3d1-5de4244ecf3a' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/quest/quest
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 0 objects and 0 action results (0 B logical); 49.8 GiB logical remain
Cleaning up orphan processes
Current runner version: '2.338.2'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20261028.624
Commit: 54898b74c23fde5556c9b3c3a2290865feef0dfe
Build Date: 2026-10-28T21:51:42Z
Worker ID: {d5d04f20-d87a-4d6b-8016-0e14eaac0117}
Region: centralus
Cloud: Azure
##[endgroup]
##[group]Operating System
Ubuntu
24.04.5
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20261101.360.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20261101.360/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20261101.360
##[endgroup]
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
PullRequests: read
##[endgroup]
Secret source: Actions
Cache mode: write
Prepare workflow directory
Prepare all required actions
Complete job name: Queue
##[group]Run [[ "$RESULT" == success ]] || { echo "::error::the queue build ended $RESULT" >&2; exit 1; }
�[36;1m[[ "$RESULT" == success ]] || { echo "::error::the queue build ended $RESULT" >&2; exit 1; }�[0m
shell: /usr/bin/bash -e {0}
env:
RESULT: failure
##[endgroup]
##[error]the queue build ended failure
##[error]Process completed with exit code 1.
Cleaning up orphan processes
(written by quest land watch) |
|
quest land watch: armed auto-merge on this pull request at 2d7e544, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix ( (written by quest land watch) |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Incremental review of 2d7e544 against base a80b397, following review 5814465179 at 7e42174 and separating base changes.
No actionable findings. The branch-specific update changes the successful-hook test from a hard-coded /bin:/usr/bin PATH to the test process's PATH, still prefixed with the temporary fake quest executable. That lets the fake script find cat in environments such as Nix while preserving which quest is invoked. The missing-executable case still uses an isolated empty directory, so this does not weaken that assertion. Documentation and hook command changes otherwise match the previous review.
Direction: retain this portability fix and run the hook test in the normal development/CI environment. Existing installations still need to adopt the documented hook entries.
Verification limits: GitHub-only static comparison; no tests or Claude Code execution performed, and no host settings changed. COMMENT only, not approval.
(Written by review (OpenAI))
Summary
The README's worktree hook entries ran a bare
quest worktree hook. Whenquestis not on the PATH Claude Code runs hooks with, the session showed the shell'squest: not found(exit 127) and nothing about how to fix it.Each documented entry now checks for
questfirst. When it is missing, the entry prints one line on stderr that names the launch route (nix develop --command claudein this repository) and the install route, then exits 1. When it is present, the entry runsquest worktree hookas before.Behavior change: sessions whose hooks were set up from the old README keep the old failure until they copy the new entries.
SETUP.mdstill points to the README section, so it is unchanged, which keeps the merge with #517 clean.Choices made without asking
SubagentStopblocks the subagent from stopping, so a missingquestwould keep a subagent running in a loop.WorktreeCreatefails on any non-zero exit, so exit 1 still aborts the worktree with the message. Exit 1 also matchesquest worktree hook's own "exits 1, never 2" rule.Public API and wire impact
None. No CLI flag, output, or on-disk format changes. The only change is the command string in the README's documented hook JSON.
Validation
/bin/sh: 1: quest: not found.quest gate just check: passes.quest gate just test: passes (exit 0). vitest: 30 files, 1101 passed, 6 skipped; worker tests 21 files, 539 passed.the documented entries say so when quest is not on PATHintest/worktree.test.ts(skipped on Windows). It checks exit 1, empty stdout, and the exact stderr withquestmissing, and that with a fakequeston PATH the hook runs with its arguments and passes stdin through.Quest
Completes
quest/m2/hook-path-error.md: the file is deleted and its link is removed fromquest/m2/README.md.Caused by
None. The bare
quest worktree hookentry has been the documented form since the hooks were documented, so the missing-PATH failure is as old as the entry.Suggested follow-ups
quest setuphooks. If it writes these entries, it should use this guarded command, so rebase it onto this change before it merges.Models
Size: XS
Implementer: Claude Code, claude-haiku-5-5, high effort
(Written by Claude Haiku 5.5 with high effort)
Review
mechanical fix by codex, gpt-6.1-sol, medium effort:
Merged base and preserved the runner PATH in the hook test fixture so cat re·solves on Nix; ran just install, just check, and just test successfully.(head 2d7e544)mechanical fix by quest land watch:
merged main at 6b6ce221323fda9350f3dbc84232bd5f5867a3d8 with the quest merge driver(head 12ebe7e)Rewritten by
quest reviewafter each round.01a1b762-f27d-7d01-9533-4dbe52af737d, round 1The documented guards provide actionable guidance when Quest is missing, preserve invocation and stdin forwarding when present, and include focused coverage; tests were not run in the read-only environment.