Repository navigation
Conversation
understudy hooks install adds UserPromptSubmit and PostToolUse (Bash) command hooks to Claude Code's settings. The hidden hooks git-state command they run reports a session's repository, branch and HEAD to Understudy only when they change, so Understudy can show what each pull request cost. It sends no code, prompts, paths or remote credentials, prints nothing, and always exits 0. understudy hooks uninstall removes only these hooks.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 614b8503ed
ℹ️ 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".
|
|
||
| /** How this process was started: the compiled binary, or node with the bundled script. */ | ||
| export function cliEntry(execPath = process.execPath, script = process.argv[1]): string[] { | ||
| return script && /\.(?:c|m)?js$/.test(script) ? [execPath, path.resolve(script)] : [execPath]; |
There was a problem hiding this comment.
When this package is installed through its declared bin, package managers expose an extensionless symlink such as /usr/local/bin/understudy, and Node retains that symlink in process.argv[1]. This suffix check therefore returns only process.execPath, installing a command like node hooks git-state ...; Node then tries to load a script named hooks, so every installed hook exits before reporting. I reproduced this with an extensionless bin symlink under Node 24.15; retain or resolve the script path when the runtime is Node rather than inferring its presence from the filename extension.
AGENTS.md reference: AGENTS.md:L71-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3373343: the script is kept whenever it exists on disk, so an extensionless bin link stays in the command. Bun's compiled binary reports a script path that exists only inside it, so it gets the binary alone. New test.
| z.object({ recorded: z.literal(true) }), | ||
| ); | ||
| // Only a recorded change is remembered, so a failed one is sent again. | ||
| await dependencies.state.write(pruned(stored, input.sessionId, current, observedAt)); |
There was a problem hiding this comment.
When two sessions fire hooks concurrently, both processes can read the same snapshot before either request completes and then this unconditional write lets the last finisher erase the other session—or even regress one session to an older state. The installed handlers are explicitly asynchronous, and Claude's hook reference confirms that each async execution creates a separate background process, so overlapping read/request/write cycles are expected; subsequent hooks then resend unchanged states with missing or incorrect prev_seen_at_ms. Serialize the transaction or merge against the latest state before replacing the shared file.
AGENTS.md reference: AGENTS.md:L71-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3373343: each session has its own owner-only state file under ~/.understudy/work-hooks/, so concurrent sessions can't erase each other. A write never replaces a newer look. New test.
Installed through a package manager's bin link, Node reports the script without an extension, which dropped it from the hook command. Keep the script whenever it exists on disk; Bun's compiled binary reports a path that exists only inside it. Async hooks of different sessions shared one state file and could erase each other's entries. Each session now has its own owner-only file, an older look never replaces a newer one, and files untouched for 14 days are removed when a new session starts.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3373343bd3
ℹ️ 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".
| const latest = await store.read(); | ||
| if (!latest || latest.lastSeenMs <= observedAt) await store.write(current); |
There was a problem hiding this comment.
When two async hooks for the same session reach remember concurrently, both can read the same latest value and pass the check, after which the older observation can finish its write last and regress the file. Claude's hook documentation confirms that every async execution uses a separate background process, and the private store's atomic rename does not make this read/check/write sequence atomic; later hooks can consequently resend stale state with an incorrect prev_seen_at_ms. Fresh evidence after the stated fix is this remaining TOCTOU window, so serialize the transaction per session or use a real compare-and-swap.
AGENTS.md reference: AGENTS.md:L71-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9176123: each session's read-check-write runs under a per-session lock (an atomic mkdir, with stale-lock recovery after 10s). A hook that can't get the lock skips remembering and the next hook sends again; a duplicate report is harmless on the service side.
| hooks: { | ||
| ...hooks, | ||
| UserPromptSubmit: [...(hooks?.UserPromptSubmit ?? []), { hooks: [handler] }], | ||
| PostToolUse: [...(hooks?.PostToolUse ?? []), { matcher: "Bash", hooks: [handler] }], |
There was a problem hiding this comment.
When a Bash invocation changes HEAD but exits nonzero, such as git commit ...; false, Claude fires PostToolUseFailure rather than PostToolUse; the official hook lifecycle distinguishes the success and failure events. Because only the success handler is installed here, the state is not reported before Claude's next model request and can be lost entirely if the session then ends, so install the same handler for failed Bash calls.
AGENTS.md reference: AGENTS.md:L71-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9176123: the same handler is installed for PostToolUseFailure.
| hooks: { | ||
| ...hooks, | ||
| UserPromptSubmit: [...(hooks?.UserPromptSubmit ?? []), { hooks: [handler] }], | ||
| PostToolUse: [...(hooks?.PostToolUse ?? []), { matcher: "Bash", hooks: [handler] }], |
There was a problem hiding this comment.
On native Windows sessions where Claude executes the PowerShell tool, this exact Bash matcher never runs, so commits and branch changes made by those shell calls are omitted despite the hook input parser explicitly accepting Windows paths. Claude's hook documentation treats Bash and PowerShell as separate tool names and uses Bash|PowerShell to cover both; include both tools to uphold the documented after-each-shell-command behavior.
AGENTS.md reference: AGENTS.md:L71-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9176123: both shell hooks match Bash|PowerShell.
…commands Two async hooks of one session could both pass the freshness check and the older write land last. Each session's read-check-write now runs under an mkdir lock; a hook that can't get it skips remembering, and the next hook sends again. The shell hook also runs on PostToolUseFailure, since a command can commit and then fail, and matches PowerShell for Windows sessions.
Summary
Adds
understudy hooks, so Understudy can show what each pull request cost. Claude Code sends its session id on every request but not its git state. These hooks report that state when it changes.understudy hooks install [--settings <file>] [--api-url <origin>]merges two command hooks into Claude Code's settings ($CLAUDE_CONFIG_DIRor~/.claude):UserPromptSubmit;PostToolUseonBash, since shell commands are where git state changes.understudy hooks uninstallremoves only these hooks.understudy hooks git-stateruns as the hook:session_idandcwdfrom the hook input.gitfor the branch, HEAD, its committer time, and the normalizedorigin.POST /admin/v1/orgs/{org}/team/work-events.host/owner/name, branch, HEAD, commit time, and when the hook ran.UNDERSTUDY_API_KEY, else Claude Code'sANTHROPIC_API_KEYwhen it is an Understudy key (never a provider key), else the stored credential.--api-urlorUNDERSTUDY_API_URL, with http allowed only on localhost; production otherwise.docs/hooks-commands.mddocuments it.AGENTS.mdputs work hooks in scope with that boundary.Evidence
npm run check: 697 tests pass, and the public boundary check passes.tests/hooks.test.mjscovers:prev_seen_at_ms, detached HEAD, failures kept for retry, provider keys refused, origin rules;Not run: a release build (
npm run build:binary).Merge Danger
Door: two-way. Nothing runs until someone installs the hooks.
Blast Radius: Low. The hook never blocks or fails Claude Code. The service side is off unless the organization has work attribution enabled; without it the hook gets a 404 and records nothing.
🤖 Generated with Claude Code