From 614b8503ed3c7eda0ddad25f178106123e56182d Mon Sep 17 00:00:00 2001 From: aamir Date: Wed, 4 Nov 2026 07:46:21 -0600 Subject: [PATCH 1/3] Add Claude Code work hooks that report git state changes 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. --- AGENTS.md | 4 + docs/hooks-commands.md | 40 ++++++ src/cli.ts | 2 + src/commands/hooks.ts | 138 ++++++++++++++++++++ src/hooks/git-state.ts | 168 +++++++++++++++++++++++++ src/hooks/repo.ts | 34 +++++ src/hooks/settings.ts | 131 +++++++++++++++++++ tests/hooks.test.mjs | 280 +++++++++++++++++++++++++++++++++++++++++ 8 files changed, 797 insertions(+) create mode 100644 docs/hooks-commands.md create mode 100644 src/commands/hooks.ts create mode 100644 src/hooks/git-state.ts create mode 100644 src/hooks/repo.ts create mode 100644 src/hooks/settings.ts create mode 100644 tests/hooks.test.mjs diff --git a/AGENTS.md b/AGENTS.md index 54e9e0b..6183b71 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,6 +68,10 @@ do not import whole files or unrelated workflows merely to preserve source shape and journals deterministic. Workflow, harness adaptations, clusters, reviews, and presentation belong to the maintained Markdown skill and private agent code. Replay mocks never execute customer tools or change routing. +- Work hooks (`understudy hooks`) are in scope: on a coding tool's hook they + report only the session id, repository identity, branch, and HEAD commit + when those change (`docs/hooks-commands.md`). They never send code, prompts, + paths, or remote credentials, print nothing, and never fail the agent. - Monitoring, Desktop, local model serving, and training job execution remain deferred. - Until automated tracked-and-packed-content checks exist, manual inspection is diff --git a/docs/hooks-commands.md b/docs/hooks-commands.md new file mode 100644 index 0000000..176f1dc --- /dev/null +++ b/docs/hooks-commands.md @@ -0,0 +1,40 @@ +# Work hooks + +`understudy hooks install` adds two command hooks to Claude Code's settings +(`--settings `, else `$CLAUDE_CONFIG_DIR/settings.json`, else +`~/.claude/settings.json`). Claude Code runs them on each prompt and after each +shell command. Understudy uses what they report to show what each pull request +cost: a request belongs to the pull request holding the next commit its +session made. + +`understudy hooks uninstall` removes only these hooks. Reinstalling replaces +them, so it is safe to run after moving or updating the CLI. + +## What leaves the machine + +Only when a session's git state changes, one request to +`POST /admin/v1/orgs/{org}/team/work-events` carrying: + +- the Claude Code session id; +- the repository as `host/owner/name`, from `origin`, without credentials; +- the branch name, or nothing on a detached HEAD; +- the HEAD commit and its committer time; +- when the hook ran, and when this session's previous state was last seen. + +No code, diffs, prompts, file paths, or remote credentials are sent. Outside a +git repository the hook sends nothing. + +## Credentials and failures + +The hook reports with `UNDERSTUDY_API_KEY`, else Claude Code's +`ANTHROPIC_API_KEY` when it is an Understudy key (never a provider key), else +the key stored by `understudy setup`. The organization comes from +`UNDERSTUDY_ORG_ID` or the stored credentials. The API origin is `--api-url` +at install, else `UNDERSTUDY_API_URL`, else production; plain http is accepted +only for localhost. + +The hook prints nothing and always exits 0, because Claude Code adds a hook's +output to the model's context. A failed report leaves the local state +unchanged, so the next hook sends it again. Local state is +`~/.understudy/work-hook-state.json` (owner-only), holding each recent +session's last reported state for 14 days. diff --git a/src/cli.ts b/src/cli.ts index bc2ec33..5b019ad 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -9,6 +9,7 @@ import { addModelsCommand } from "./commands/models.js"; import { addBillingCommands } from "./commands/billing.js"; import { addCapturesCommands } from "./commands/captures.js"; import { addContextCommands } from "./commands/context.js"; +import { addHooksCommands } from "./commands/hooks.js"; import { addKeysCommands } from "./commands/keys.js"; import { addMigrationCommands } from "./commands/migrations.js"; import { addProjectsCommands } from "./commands/projects.js"; @@ -45,6 +46,7 @@ export function createCli( addBillingCommands(program); addCapturesCommands(program); addContextCommands(program); + addHooksCommands(program); addKeysCommands(program); addModelsCommand(program); addMigrationCommands(program); diff --git a/src/commands/hooks.ts b/src/commands/hooks.ts new file mode 100644 index 0000000..8401c1c --- /dev/null +++ b/src/commands/hooks.ts @@ -0,0 +1,138 @@ +import { execFile } from "node:child_process"; +import { homedir } from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { Command } from "commander"; + +import { createCredentialStore } from "../auth/credentials.js"; +import { CliError } from "../errors.js"; +import { + hookStateSchema, + reportGitState, + resolveApiUrl, + resolveHookCredential, + type GitStateDependencies, +} from "../hooks/git-state.js"; +import { + cliEntry, + gitStateCommand, + readSettings, + resolveClaudeSettingsPath, + withGitStateHooks, + withoutGitStateHooks, + writeSettings, +} from "../hooks/settings.js"; +import { createInferenceCredentialStore } from "../inference/credentials.js"; +import { formatOutput } from "../output.js"; +import { createPrivateJsonStore } from "../storage/private-json.js"; + +const execute = promisify(execFile); +const hookInputMaxBytes = 256 * 1024; + +export function addHooksCommands( + program: Command, + gitStateDependencies: () => GitStateDependencies = createGitStateDependencies, +): void { + const hooks = program.command("hooks") + .description("Install Claude Code hooks that tie its spend to the pull requests it produces."); + + hooks.command("install") + .description("Add the hooks to Claude Code settings, replacing an earlier install.") + .option("--settings ", "Claude Code settings file; defaults to $CLAUDE_CONFIG_DIR or ~/.claude settings.json.") + .option("--api-url ", "Understudy API origin, for local development.") + .addHelpText("after", ` +On each prompt and after each shell command, Claude Code runs this CLI, which +reports the session's repository, branch, and commit to Understudy when they +change. It sends no code, prompts, paths, or remote credentials. Understudy +uses it to show what each pull request cost. The key is UNDERSTUDY_API_KEY, +an Understudy ANTHROPIC_API_KEY, or the one stored by understudy setup. +`) + .action(async function (this: Command, options: { settings?: string; apiUrl?: string }) { + if (options.apiUrl !== undefined && resolveApiUrl(options.apiUrl, {}) === null) { + throw new CliError("--api-url must be an https origin, or http on localhost."); + } + const file = resolveClaudeSettingsPath(options.settings); + const command = gitStateCommand(cliEntry(), options.apiUrl); + await writeSettings(file, withGitStateHooks(await readSettings(file), command)); + process.stdout.write(`${formatOutput(this, { installed: true, settings: file, command }, [ + `Installed the work hooks in ${file}.`, + "New Claude Code sessions report their git state; restart running ones.", + ])}\n`); + }); + + hooks.command("uninstall") + .description("Remove only these hooks from Claude Code settings.") + .option("--settings ", "Claude Code settings file; defaults to $CLAUDE_CONFIG_DIR or ~/.claude settings.json.") + .action(async function (this: Command, options: { settings?: string }) { + const file = resolveClaudeSettingsPath(options.settings); + await writeSettings(file, withoutGitStateHooks(await readSettings(file))); + process.stdout.write(`${formatOutput(this, { installed: false, settings: file }, [ + `Removed the work hooks from ${file}.`, + ])}\n`); + }); + + // Claude Code runs this; nothing reaches stdout or stderr and it always + // exits 0, since a hook's output joins the model's context. + hooks.command("git-state", { hidden: true }) + .option("--harness ", "The coding tool running the hook.", "claude_code") + .option("--api-url ") + .action(async (options: { harness: string; apiUrl?: string }) => { + const apiUrl = resolveApiUrl(options.apiUrl, process.env); + if (options.harness !== "claude_code" || apiUrl === null) return; + await reportGitState({ harness: "claude_code", apiUrl }, gitStateDependencies()); + }); +} + +export function createGitStateDependencies(): GitStateDependencies { + const directory = path.join(homedir(), ".understudy"); + const inferenceStore = createInferenceCredentialStore(); + const authStore = createCredentialStore(); + return { + now: () => Date.now(), + readInput: () => readStdin(hookInputMaxBytes, 1_000), + git: async (cwd, args) => { + // Without inherited GIT_* overrides, like the migration commands. + const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_"))); + try { + const { stdout } = await execute("git", ["-C", cwd, ...args], { env, timeout: 1_500, maxBuffer: 64 * 1024 }); + return stdout.trim(); + } catch { + return null; + } + }, + state: createPrivateJsonStore({ + directory, + fileName: "work-hook-state.json", + label: "work hook state", + schema: hookStateSchema, + }), + credential: async () => { + const inference = await inferenceStore.read().catch(() => null); + const auth = await authStore.read().catch(() => null); + return resolveHookCredential(process.env, [ + inference ?? {}, + auth?.method === "organization_key" ? auth : {}, + ]); + }, + }; +} + +function readStdin(maxBytes: number, timeoutMs: number): Promise { + return new Promise((resolve) => { + const chunks: Buffer[] = []; + let size = 0; + const finish = () => { + clearTimeout(timer); + process.stdin.pause(); + resolve(size > maxBytes ? "" : Buffer.concat(chunks).toString("utf8")); + }; + const timer = setTimeout(finish, timeoutMs); + process.stdin.on("data", (chunk: Buffer) => { + size += chunk.length; + if (size <= maxBytes) chunks.push(chunk); + else finish(); + }); + process.stdin.on("end", finish); + process.stdin.on("error", finish); + }); +} diff --git a/src/hooks/git-state.ts b/src/hooks/git-state.ts new file mode 100644 index 0000000..cbc4442 --- /dev/null +++ b/src/hooks/git-state.ts @@ -0,0 +1,168 @@ +import { z } from "zod"; + +import { defaultManagementUrl, requestManagementJson } from "../management/client.js"; +import type { PrivateJsonStore } from "../storage/private-json.js"; +import { normalizeGitRepo } from "./repo.js"; + +const sessionIdPattern = /^[A-Za-z0-9._:-]{1,128}$/; +const shaPattern = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; +const understudyKeyPattern = /^sk_[A-Za-z0-9_-]+$/; +const localHosts = new Set(["localhost", "127.0.0.1", "[::1]"]); +const stateRetentionMs = 14 * 24 * 60 * 60 * 1000; +const stateMaxSessions = 500; + +export const hookStateSchema = z + .object({ + version: z.literal(1), + sessions: z.record( + z.string(), + z.object({ repo: z.string(), branch: z.string(), headSha: z.string(), lastSeenMs: z.number() }).strict(), + ), + }) + .strict(); +export type HookState = z.infer; + +export interface HookCredential { + apiKey: string; + organizationId: string; +} + +export interface GitStateDependencies { + now(): number; + readInput(): Promise; + /** Trimmed stdout, or null when git fails. */ + git(cwd: string, args: readonly string[]): Promise; + state: PrivateJsonStore; + credential(): Promise; + fetchImplementation?: typeof fetch; +} + +export type GitStateOutcome = "skipped" | "unchanged" | "recorded" | "failed"; + +/** + * Reports a coding tool session's git state to Understudy when it changed + * since this session's last report. Sends the session id, repository + * identity, branch, HEAD and its committer time; never paths, diffs, or + * remote credentials. Every failure is swallowed: a hook must never slow or + * break the agent. + */ +export async function reportGitState( + options: { harness: "claude_code"; apiUrl: string }, + dependencies: GitStateDependencies, +): Promise { + try { + const observedAt = dependencies.now(); + const input = parseHookInput(await dependencies.readInput()); + if (!input) return "skipped"; + const head = await dependencies.git(input.cwd, ["log", "-1", "--format=%H%x00%ct"]); + const [sha, committedSeconds] = head?.split("\0") ?? []; + if (!sha || !shaPattern.test(sha) || !/^\d+$/.test(committedSeconds ?? "")) return "skipped"; + const branch = (await dependencies.git(input.cwd, ["symbolic-ref", "-q", "--short", "HEAD"])) ?? ""; + const remote = await dependencies.git(input.cwd, ["config", "--get", "remote.origin.url"]); + const repo = (remote && normalizeGitRepo(remote)) ?? ""; + + const stored = (await dependencies.state.read()) ?? { version: 1 as const, sessions: {} }; + const previous = stored.sessions[input.sessionId]; + const current = { repo, branch, headSha: sha, lastSeenMs: observedAt }; + if (previous && previous.repo === repo && previous.branch === branch && previous.headSha === sha) { + await dependencies.state.write(pruned(stored, input.sessionId, current, observedAt)); + return "unchanged"; + } + + const credential = await dependencies.credential(); + if (!credential) return "skipped"; + await requestManagementJson( + { + accessToken: credential.apiKey, + organizationId: credential.organizationId, + credentialClass: "organization_key", + baseUrl: options.apiUrl, + fetchImplementation: dependencies.fetchImplementation, + }, + { + method: "POST", + path: `/admin/v1/orgs/${encodeURIComponent(credential.organizationId)}/team/work-events`, + body: { + harness: options.harness, + session_id: input.sessionId, + observed_at_ms: observedAt, + prev_seen_at_ms: previous?.lastSeenMs ?? null, + repo, + branch, + head_sha: sha, + head_committed_at_ms: Number(committedSeconds) * 1000, + }, + action: "Work event", + timeoutMs: 3_000, + }, + 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)); + return "recorded"; + } catch { + return "failed"; + } +} + +function parseHookInput(text: string): { sessionId: string; cwd: string } | null { + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return null; + } + if (parsed === null || typeof parsed !== "object") return null; + const { session_id: sessionId, cwd } = parsed as Record; + if (typeof sessionId !== "string" || !sessionIdPattern.test(sessionId)) return null; + if (typeof cwd !== "string" || !cwd.startsWith("/") && !/^[A-Za-z]:\\/.test(cwd)) return null; + return { sessionId, cwd }; +} + +function pruned( + state: HookState, + sessionId: string, + current: HookState["sessions"][string], + now: number, +): HookState { + const sessions = Object.entries({ ...state.sessions, [sessionId]: current }) + .filter(([, session]) => session.lastSeenMs >= now - stateRetentionMs) + .sort(([, a], [, b]) => b.lastSeenMs - a.lastSeenMs) + .slice(0, stateMaxSessions); + return { version: 1, sessions: Object.fromEntries(sessions) }; +} + +/** + * The key and organization to report with: an Understudy key from the + * environment (the harness's own `ANTHROPIC_API_KEY` when it is one, never a + * provider key), else the CLI's stored credentials. + */ +export function resolveHookCredential( + environment: NodeJS.ProcessEnv, + stored: { apiKey?: string; organizationId?: string }[], +): HookCredential | null { + const fromEnvironment = [environment.UNDERSTUDY_API_KEY, environment.ANTHROPIC_API_KEY] + .map((value) => value?.trim()) + .find((value) => value && understudyKeyPattern.test(value)); + const apiKey = fromEnvironment ?? stored.find((entry) => entry.apiKey)?.apiKey; + const organizationId = + environment.UNDERSTUDY_ORG_ID?.trim() || stored.find((entry) => entry.organizationId)?.organizationId; + return apiKey && organizationId ? { apiKey, organizationId } : null; +} + +/** + * `--api-url`, else `UNDERSTUDY_API_URL`, else production. An origin only: + * https, or http to this machine for local development. + */ +export function resolveApiUrl(flag: string | undefined, environment: NodeJS.ProcessEnv): string | null { + let url: URL; + try { + url = new URL(flag ?? environment.UNDERSTUDY_API_URL ?? defaultManagementUrl); + } catch { + return null; + } + const local = localHosts.has(url.hostname); + if (url.username || url.password || url.pathname !== "/" || url.search || url.hash) return null; + if (url.protocol !== "https:" && !(url.protocol === "http:" && local)) return null; + return url.origin; +} diff --git a/src/hooks/repo.ts b/src/hooks/repo.ts new file mode 100644 index 0000000..0ed9001 --- /dev/null +++ b/src/hooks/repo.ts @@ -0,0 +1,34 @@ +const remoteProtocols = new Set(["https:", "http:", "ssh:", "git:", "git+ssh:", "ssh+git:"]); +const repoPattern = /^[a-z0-9.-]+(?:\/[a-z0-9._-]+){2,}$/; +const repoMaxCharacters = 200; + +/** + * A git remote as `host/owner/name`, lowercased, without credentials, port, + * or `.git`, matching the Understudy service's normalization. Null for local + * paths, `file://` remotes, and anything unparsable. + */ +export function normalizeGitRepo(remote: string): string | null { + const value = remote.trim(); + let host: string; + let pathname: string; + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(value)) { + let url: URL; + try { + url = new URL(value); + } catch { + return null; + } + if (!remoteProtocols.has(url.protocol)) return null; + host = url.hostname; + pathname = url.pathname; + } else { + // scp-like `user@host:owner/name`; `host:/abs/path` is a local-style path. + const scp = /^(?:[^@/\s]+@)?([^:/\s]+):(?!\/)(\S+)$/.exec(value); + if (!scp) return null; + host = scp[1]!; + pathname = scp[2]!; + } + const segments = pathname.replace(/\/+$/, "").replace(/\.git$/i, "").split("/").filter(Boolean); + const repo = [host, ...segments].join("/").toLowerCase(); + return repo.length <= repoMaxCharacters && repoPattern.test(repo) ? repo : null; +} diff --git a/src/hooks/settings.ts b/src/hooks/settings.ts new file mode 100644 index 0000000..0cf1fc6 --- /dev/null +++ b/src/hooks/settings.ts @@ -0,0 +1,131 @@ +import { randomUUID } from "node:crypto"; +import { constants } from "node:fs"; +import { lstat, open, readFile, rename, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { CliError } from "../errors.js"; + +/** Recognizes the hooks this CLI installed, so reinstalling replaces them. */ +export const gitStateCommandMarker = "'hooks' 'git-state'"; + +export interface HookSettingsContext { + homeDirectory?: string; + environment?: NodeJS.ProcessEnv; +} + +/** `--settings`, else Claude Code's settings in `$CLAUDE_CONFIG_DIR` or `~/.claude`. */ +export function resolveClaudeSettingsPath(settings: string | undefined, context: HookSettingsContext = {}): string { + const homeDirectory = context.homeDirectory ?? os.homedir(); + const environment = context.environment ?? process.env; + const expand = (value: string) => + value === "~" ? homeDirectory : value.startsWith("~/") ? path.join(homeDirectory, value.slice(2)) : path.resolve(value); + if (settings !== undefined) { + if (!settings.trim() || settings.includes("\0")) throw new CliError("--settings must be a file path."); + return expand(settings); + } + const configDirectory = environment.CLAUDE_CONFIG_DIR?.trim(); + return path.join(configDirectory ? expand(configDirectory) : path.join(homeDirectory, ".claude"), "settings.json"); +} + +/** + * The command Claude Code runs: this CLI by absolute path, since hooks run + * in a shell whose PATH may not find it, and each argument single-quoted. + */ +export function gitStateCommand(entry: readonly string[], apiUrl: string | undefined): string { + const words = [...entry, "hooks", "git-state", "--harness", "claude_code", ...(apiUrl ? ["--api-url", apiUrl] : [])]; + for (const word of words) { + if (word.includes("'")) throw new CliError("The CLI's path contains a quote; install it elsewhere and retry."); + } + return words.map((word) => `'${word}'`).join(" "); +} + +/** 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]; +} + +type Settings = Record; +type HookGroup = { matcher?: string; hooks?: Array<{ type?: string; command?: string }> }; + +/** + * Claude Code runs the hook on each prompt and after each shell command, the + * only places git state changes before its next model request. `async` keeps + * the hook off the agent's path. + */ +export function withGitStateHooks(settings: Settings, command: string): Settings { + const handler = { type: "command", command, timeout: 10, async: true }; + const hooks = withoutGitStateHooks(settings).hooks as Record | undefined; + return { + ...settings, + hooks: { + ...hooks, + UserPromptSubmit: [...(hooks?.UserPromptSubmit ?? []), { hooks: [handler] }], + PostToolUse: [...(hooks?.PostToolUse ?? []), { matcher: "Bash", hooks: [handler] }], + }, + }; +} + +export function withoutGitStateHooks(settings: Settings): Settings { + const hooks = settings.hooks; + if (hooks === undefined) return settings; + if (!isObject(hooks)) throw new CliError("Claude Code settings have a `hooks` value that is not an object."); + const kept: Record = {}; + for (const [event, groups] of Object.entries(hooks)) { + if (!Array.isArray(groups)) { + kept[event] = groups; + continue; + } + const remaining = (groups as HookGroup[]) + .map((group) => + Array.isArray(group?.hooks) + ? { ...group, hooks: group.hooks.filter((hook) => !hook?.command?.includes(gitStateCommandMarker)) } + : group, + ) + .filter((group) => !Array.isArray(group?.hooks) || group.hooks.length > 0); + if (remaining.length > 0) kept[event] = remaining; + } + const { hooks: _hooks, ...rest } = settings; + return Object.keys(kept).length > 0 ? { ...rest, hooks: kept } : rest; +} + +export async function readSettings(file: string): Promise { + let text: string; + try { + text = await readFile(file, "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return {}; + throw error; + } + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + throw new CliError(`${file} is not valid JSON. Fix it, then retry; nothing was changed.`); + } + if (!isObject(parsed)) throw new CliError(`${file} must hold a JSON object; nothing was changed.`); + return parsed; +} + +/** Writes a sibling temporary file, then renames it over the settings, keeping their mode. */ +export async function writeSettings(file: string, settings: Settings): Promise { + const mode = await lstat(file).then((entry) => entry.mode & 0o777, () => 0o600); + const temporary = path.join(path.dirname(file), `.${path.basename(file)}.${randomUUID()}.tmp`); + const handle = await open(temporary, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, mode); + try { + await handle.writeFile(`${JSON.stringify(settings, null, 2)}\n`); + await handle.sync(); + } finally { + await handle.close(); + } + try { + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }); + throw error; + } +} + +function isObject(value: unknown): value is Settings { + return value !== null && typeof value === "object" && !Array.isArray(value); +} diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs new file mode 100644 index 0000000..1639f75 --- /dev/null +++ b/tests/hooks.test.mjs @@ -0,0 +1,280 @@ +import assert from "node:assert/strict"; +import { execFileSync, spawn } from "node:child_process"; +import { mkdtemp, readFile, readdir, realpath, rm, stat, writeFile, chmod } from "node:fs/promises"; +import { createServer } from "node:http"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { CliError } from "../dist/errors.js"; +import { reportGitState, resolveApiUrl, resolveHookCredential } from "../dist/hooks/git-state.js"; +import { normalizeGitRepo } from "../dist/hooks/repo.js"; +import { + cliEntry, + gitStateCommand, + readSettings, + resolveClaudeSettingsPath, + withGitStateHooks, + withoutGitStateHooks, + writeSettings, +} from "../dist/hooks/settings.js"; + +const binPath = fileURLToPath(new URL("../dist/bin.js", import.meta.url)); +const syntheticKey = ["sk", "synthetic", "hook", "key"].join("_"); +const sha = (character) => character.repeat(40); +const sessionId = "00000000-0000-4000-8000-000000000001"; + +async function fixture(t) { + const directory = await mkdtemp(path.join(await realpath(os.tmpdir()), "synthetic-hooks-")); + t.after(() => rm(directory, { recursive: true, force: true })); + return directory; +} + +function memoryStore(initial = null) { + const store = { value: initial, writes: 0 }; + return { + store, + state: { + read: async () => store.value, + write: async (value) => { + store.value = value; + store.writes += 1; + }, + clear: async () => { + store.value = null; + }, + }, + }; +} + +function dependencies({ git = {}, state, fetchImplementation, credential, input, now = 1_000_000 }) { + const answers = { + "log -1 --format=%H%x00%ct": `${sha("a")}\u0000990`, + "symbolic-ref -q --short HEAD": "feature-a", + "config --get remote.origin.url": "https://github.com/Acme/Widgets.git", + ...git, + }; + return { + now: () => now, + readInput: async () => input ?? JSON.stringify({ session_id: sessionId, cwd: "/synthetic/repo", hook_event_name: "PostToolUse" }), + git: async (_cwd, args) => (args.join(" ") in answers ? answers[args.join(" ")] : null), + state, + credential: async () => (credential === undefined ? { apiKey: syntheticKey, organizationId: "org_synthetic" } : credential), + fetchImplementation, + }; +} + +function recordingFetch(status = 200) { + const calls = []; + const fetchImplementation = async (url, init) => { + calls.push({ url, init, body: JSON.parse(init.body) }); + return new Response(JSON.stringify({ recorded: true }), { status, headers: { "content-type": "application/json" } }); + }; + return { calls, fetchImplementation }; +} + +test("normalizes every remote form of one repository, without credentials", () => { + const scp = ["git", "@", "github.com:Acme/Widgets.git"].join(""); + const withPassword = ["https://bot:", "synthetic", "@github.com/acme/widgets"].join(""); + for (const remote of [scp, "https://github.com/acme/widgets/", "ssh://git.example.test:22/acme/widgets", withPassword]) { + assert.match(normalizeGitRepo(remote), /^(github\.com|git\.example\.test)\/acme\/widgets$/); + } + for (const remote of ["/srv/git/widgets", "file:///srv/git/widgets", "github.com/acme"]) { + assert.equal(normalizeGitRepo(remote), null); + } +}); + +test("reports a session's first git state, then only changes", async () => { + const { store, state } = memoryStore(); + const { calls, fetchImplementation } = recordingFetch(); + + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation })), "recorded"); + assert.equal(calls.length, 1); + assert.equal(calls[0].url, "https://api.example.test/admin/v1/orgs/org_synthetic/team/work-events"); + assert.equal(calls[0].init.method, "POST"); + assert.equal(calls[0].init.headers.authorization, `Bearer ${syntheticKey}`); + assert.deepEqual(calls[0].body, { + harness: "claude_code", + session_id: sessionId, + observed_at_ms: 1_000_000, + prev_seen_at_ms: null, + repo: "github.com/acme/widgets", + branch: "feature-a", + head_sha: sha("a"), + head_committed_at_ms: 990_000, + }); + + // Unchanged: no request, but the session's last look moves forward. + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation, now: 1_005_000 })), "unchanged"); + assert.equal(calls.length, 1); + assert.equal(store.value.sessions[sessionId].lastSeenMs, 1_005_000); + + // A new commit is reported with when the previous state was last seen. + const changed = dependencies({ state, fetchImplementation, now: 1_010_000, git: { "log -1 --format=%H%x00%ct": `${sha("b")}\u00001008` } }); + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, changed), "recorded"); + assert.equal(calls[1].body.prev_seen_at_ms, 1_005_000); + assert.equal(calls[1].body.head_sha, sha("b")); +}); + +test("reports a detached HEAD with no branch and a repository with no origin", async () => { + const { state } = memoryStore(); + const { calls, fetchImplementation } = recordingFetch(); + const detached = dependencies({ state, fetchImplementation, git: { "symbolic-ref -q --short HEAD": null, "config --get remote.origin.url": null } }); + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, detached), "recorded"); + assert.equal(calls[0].body.branch, ""); + assert.equal(calls[0].body.repo, ""); +}); + +test("keeps a failed report so the next hook sends it again", async () => { + const { store, state } = memoryStore(); + const { calls, fetchImplementation } = recordingFetch(500); + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation })), "failed"); + assert.equal(calls.length, 1); + assert.equal(store.value, null); + const throwing = async () => { + throw new Error("offline"); + }; + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation: throwing })), "failed"); +}); + +test("skips without a usable hook input, a repository, or a credential", async () => { + const { state } = memoryStore(); + const { calls, fetchImplementation } = recordingFetch(); + const options = { harness: "claude_code", apiUrl: "https://api.example.test" }; + for (const input of ["not json", "{}", JSON.stringify({ session_id: "has space", cwd: "/x" }), JSON.stringify({ session_id: sessionId, cwd: "relative" })]) { + assert.equal(await reportGitState(options, dependencies({ state, fetchImplementation, input })), "skipped"); + } + assert.equal(await reportGitState(options, dependencies({ state, fetchImplementation, git: { "log -1 --format=%H%x00%ct": null } })), "skipped"); + assert.equal(await reportGitState(options, dependencies({ state, fetchImplementation, credential: null })), "skipped"); + assert.equal(calls.length, 0); +}); + +test("forgets sessions after two weeks", async () => { + const old = { repo: "", branch: "", headSha: sha("c"), lastSeenMs: 0 }; + const { store, state } = memoryStore({ version: 1, sessions: { "old-session": old } }); + const { fetchImplementation } = recordingFetch(); + await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation, now: 15 * 86_400_000 })); + assert.deepEqual(Object.keys(store.value.sessions), [sessionId]); +}); + +test("uses an Understudy key from the environment, never a provider key", () => { + const storedKey = ["stored", "synthetic"].join("-"); + const stored = [{ apiKey: storedKey, organizationId: "org_stored" }]; + assert.deepEqual(resolveHookCredential({ ANTHROPIC_API_KEY: syntheticKey }, stored), { apiKey: syntheticKey, organizationId: "org_stored" }); + assert.deepEqual(resolveHookCredential({ ANTHROPIC_API_KEY: ["sk", "ant", "synthetic"].join("-") }, stored), { apiKey: storedKey, organizationId: "org_stored" }); + assert.deepEqual(resolveHookCredential({ UNDERSTUDY_API_KEY: syntheticKey, UNDERSTUDY_ORG_ID: "org_env" }, []), { apiKey: syntheticKey, organizationId: "org_env" }); + assert.equal(resolveHookCredential({ UNDERSTUDY_API_KEY: syntheticKey }, []), null); +}); + +test("accepts an https origin, or http only on this machine", () => { + assert.equal(resolveApiUrl(undefined, {}), "https://api.understudylabs.com"); + assert.equal(resolveApiUrl(undefined, { UNDERSTUDY_API_URL: "http://localhost:8789" }), "http://localhost:8789"); + assert.equal(resolveApiUrl("http://127.0.0.1:8789/", {}), "http://127.0.0.1:8789"); + for (const value of ["http://api.example.test", "https://api.example.test/v1", ["https://user:", "secret@api.example.test"].join(""), "not a url"]) { + assert.equal(resolveApiUrl(value, {}), null, value); + } +}); + +test("installs the hooks once, keeping every other setting and hook", async (t) => { + const directory = await fixture(t); + const file = path.join(directory, "settings.json"); + const other = { type: "command", command: "/usr/bin/true" }; + await writeFile(file, JSON.stringify({ model: "opus", hooks: { PostToolUse: [{ matcher: "Edit", hooks: [other] }], Stop: [{ hooks: [other] }] } }), { mode: 0o640 }); + const command = gitStateCommand(["/opt/understudy/understudy"], undefined); + + await writeSettings(file, withGitStateHooks(await readSettings(file), command)); + await writeSettings(file, withGitStateHooks(await readSettings(file), command)); + const installed = JSON.parse(await readFile(file, "utf8")); + assert.equal(installed.model, "opus"); + assert.deepEqual(installed.hooks.Stop, [{ hooks: [other] }]); + assert.deepEqual(installed.hooks.PostToolUse, [ + { matcher: "Edit", hooks: [other] }, + { matcher: "Bash", hooks: [{ type: "command", command, timeout: 10, async: true }] }, + ]); + assert.deepEqual(installed.hooks.UserPromptSubmit, [{ hooks: [{ type: "command", command, timeout: 10, async: true }] }]); + assert.equal((await stat(file)).mode & 0o777, 0o640); + assert.deepEqual((await readdir(directory)).sort(), ["settings.json"]); + + await writeSettings(file, withoutGitStateHooks(await readSettings(file))); + assert.deepEqual(JSON.parse(await readFile(file, "utf8")), { model: "opus", hooks: { PostToolUse: [{ matcher: "Edit", hooks: [other] }], Stop: [{ hooks: [other] }] } }); +}); + +test("refuses settings that are not a JSON object, changing nothing", async (t) => { + const directory = await fixture(t); + const file = path.join(directory, "settings.json"); + await writeFile(file, "{ broken"); + await assert.rejects(readSettings(file), CliError); + assert.equal(await readFile(file, "utf8"), "{ broken"); + assert.deepEqual(await readSettings(path.join(directory, "missing.json")), {}); +}); + +test("finds Claude Code settings and quotes the hook command", () => { + assert.equal(resolveClaudeSettingsPath(undefined, { homeDirectory: "/srv/synthetic-home", environment: {} }), "/srv/synthetic-home/.claude/settings.json"); + assert.equal(resolveClaudeSettingsPath(undefined, { homeDirectory: "/srv/synthetic-home", environment: { CLAUDE_CONFIG_DIR: "~/alt" } }), "/srv/synthetic-home/alt/settings.json"); + assert.equal(resolveClaudeSettingsPath("~/s.json", { homeDirectory: "/srv/synthetic-home", environment: {} }), "/srv/synthetic-home/s.json"); + assert.equal( + gitStateCommand(["/usr/bin/node", "/opt/cli/bin.js"], "http://localhost:8789"), + "'/usr/bin/node' '/opt/cli/bin.js' 'hooks' 'git-state' '--harness' 'claude_code' '--api-url' 'http://localhost:8789'", + ); + assert.throws(() => gitStateCommand(["/opt/it's/understudy"], undefined), CliError); + assert.deepEqual(cliEntry("/usr/bin/node", "/opt/cli/dist/bin.js"), ["/usr/bin/node", "/opt/cli/dist/bin.js"]); + assert.deepEqual(cliEntry("/opt/understudy/understudy", "/$bunfs/root/understudy"), ["/opt/understudy/understudy"]); +}); + +test("the hook command reports a real repository's state and prints nothing", async (t) => { + const directory = await fixture(t); + const home = path.join(directory, "home"); + const repository = path.join(directory, "repository"); + const git = (...args) => execFileSync("git", ["-C", repository, ...args], { encoding: "utf8" }).trim(); + execFileSync("git", ["init", "-q", "-b", "feature-a", repository]); + git("config", "user.email", "synthetic@example.test"); + git("config", "user.name", "Synthetic"); + git("remote", "add", "origin", "https://github.com/acme/widgets.git"); + git("commit", "-q", "--allow-empty", "-m", "synthetic"); + const head = git("rev-parse", "HEAD"); + + const received = []; + const server = createServer((request, response) => { + let body = ""; + request.on("data", (chunk) => (body += chunk)); + request.on("end", () => { + received.push({ url: request.url, authorization: request.headers.authorization, body: JSON.parse(body) }); + response.writeHead(200, { "content-type": "application/json" }); + response.end(JSON.stringify({ recorded: true })); + }); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + t.after(() => server.close()); + const { port } = server.address(); + + const run = () => + new Promise((resolve) => { + const child = spawn(process.execPath, [binPath, "hooks", "git-state", "--api-url", `http://127.0.0.1:${port}`], { + env: { PATH: process.env.PATH, HOME: home, UNDERSTUDY_API_KEY: syntheticKey, UNDERSTUDY_ORG_ID: "org_synthetic" }, + stdio: ["pipe", "pipe", "pipe"], + }); + let output = ""; + child.stdout.on("data", (chunk) => (output += chunk)); + child.stderr.on("data", (chunk) => (output += chunk)); + child.on("close", (code) => resolve({ code, output })); + child.stdin.end(JSON.stringify({ session_id: sessionId, cwd: repository, hook_event_name: "UserPromptSubmit" })); + }); + + assert.deepEqual(await run(), { code: 0, output: "" }); + assert.deepEqual(await run(), { code: 0, output: "" }); + assert.equal(received.length, 1); + assert.equal(received[0].url, "/admin/v1/orgs/org_synthetic/team/work-events"); + assert.equal(received[0].authorization, `Bearer ${syntheticKey}`); + assert.deepEqual( + { ...received[0].body, observed_at_ms: 0, head_committed_at_ms: 0 }, + { harness: "claude_code", session_id: sessionId, observed_at_ms: 0, prev_seen_at_ms: null, repo: "github.com/acme/widgets", branch: "feature-a", head_sha: head, head_committed_at_ms: 0 }, + ); + assert.equal((await stat(path.join(home, ".understudy", "work-hook-state.json"))).mode & 0o777, 0o600); + + // An unreachable service changes nothing the agent can see. + await chmod(path.join(home, ".understudy"), 0o700); + server.close(); + git("commit", "-q", "--allow-empty", "-m", "synthetic two"); + assert.deepEqual(await run(), { code: 0, output: "" }); +}); From 3373343bd360e5c6773836d6f73bbd58dd2eea81 Mon Sep 17 00:00:00 2001 From: aamir Date: Wed, 4 Nov 2026 08:15:31 -0600 Subject: [PATCH 2/3] Keep extensionless bin paths and give each session its own hook state 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. --- docs/hooks-commands.md | 6 ++-- src/commands/hooks.ts | 27 +++++++++++----- src/hooks/git-state.ts | 46 ++++++++++----------------- src/hooks/settings.ts | 16 +++++++--- tests/hooks.test.mjs | 71 ++++++++++++++++++++++++++---------------- 5 files changed, 95 insertions(+), 71 deletions(-) diff --git a/docs/hooks-commands.md b/docs/hooks-commands.md index 176f1dc..b21bd59 100644 --- a/docs/hooks-commands.md +++ b/docs/hooks-commands.md @@ -35,6 +35,6 @@ only for localhost. The hook prints nothing and always exits 0, because Claude Code adds a hook's output to the model's context. A failed report leaves the local state -unchanged, so the next hook sends it again. Local state is -`~/.understudy/work-hook-state.json` (owner-only), holding each recent -session's last reported state for 14 days. +unchanged, so the next hook sends it again. Local state is one owner-only +file per session in `~/.understudy/work-hooks/`, holding its last reported +state; files untouched for 14 days are removed. diff --git a/src/commands/hooks.ts b/src/commands/hooks.ts index 8401c1c..70cc791 100644 --- a/src/commands/hooks.ts +++ b/src/commands/hooks.ts @@ -1,4 +1,6 @@ import { execFile } from "node:child_process"; +import { createHash } from "node:crypto"; +import { lstat, readdir, unlink } from "node:fs/promises"; import { homedir } from "node:os"; import path from "node:path"; import { promisify } from "node:util"; @@ -7,8 +9,8 @@ import { Command } from "commander"; import { createCredentialStore } from "../auth/credentials.js"; import { CliError } from "../errors.js"; import { - hookStateSchema, reportGitState, + sessionStateSchema, resolveApiUrl, resolveHookCredential, type GitStateDependencies, @@ -84,7 +86,7 @@ an Understudy ANTHROPIC_API_KEY, or the one stored by understudy setup. } export function createGitStateDependencies(): GitStateDependencies { - const directory = path.join(homedir(), ".understudy"); + const directory = path.join(homedir(), ".understudy", "work-hooks"); const inferenceStore = createInferenceCredentialStore(); const authStore = createCredentialStore(); return { @@ -100,12 +102,21 @@ export function createGitStateDependencies(): GitStateDependencies { return null; } }, - state: createPrivateJsonStore({ - directory, - fileName: "work-hook-state.json", - label: "work hook state", - schema: hookStateSchema, - }), + sessionState: (sessionId) => + createPrivateJsonStore({ + directory, + fileName: `${createHash("sha256").update(sessionId).digest("hex").slice(0, 32)}.json`, + label: "work hook state", + schema: sessionStateSchema, + }), + pruneSessionStates: async (olderThanMs) => { + for (const name of await readdir(directory).catch(() => [])) { + if (!name.endsWith(".json")) continue; + const file = path.join(directory, name); + const entry = await lstat(file).catch(() => null); + if (entry?.isFile() && entry.mtimeMs < olderThanMs) await unlink(file).catch(() => undefined); + } + }, credential: async () => { const inference = await inferenceStore.read().catch(() => null); const auth = await authStore.read().catch(() => null); diff --git a/src/hooks/git-state.ts b/src/hooks/git-state.ts index cbc4442..6e3bb89 100644 --- a/src/hooks/git-state.ts +++ b/src/hooks/git-state.ts @@ -9,18 +9,11 @@ const shaPattern = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; const understudyKeyPattern = /^sk_[A-Za-z0-9_-]+$/; const localHosts = new Set(["localhost", "127.0.0.1", "[::1]"]); const stateRetentionMs = 14 * 24 * 60 * 60 * 1000; -const stateMaxSessions = 500; -export const hookStateSchema = z - .object({ - version: z.literal(1), - sessions: z.record( - z.string(), - z.object({ repo: z.string(), branch: z.string(), headSha: z.string(), lastSeenMs: z.number() }).strict(), - ), - }) +export const sessionStateSchema = z + .object({ version: z.literal(1), repo: z.string(), branch: z.string(), headSha: z.string(), lastSeenMs: z.number() }) .strict(); -export type HookState = z.infer; +export type SessionState = z.infer; export interface HookCredential { apiKey: string; @@ -32,7 +25,9 @@ export interface GitStateDependencies { readInput(): Promise; /** Trimmed stdout, or null when git fails. */ git(cwd: string, args: readonly string[]): Promise; - state: PrivateJsonStore; + /** One file per session, so concurrent hooks of different sessions never collide. */ + sessionState(sessionId: string): PrivateJsonStore; + pruneSessionStates(olderThanMs: number): Promise; credential(): Promise; fetchImplementation?: typeof fetch; } @@ -61,11 +56,16 @@ export async function reportGitState( const remote = await dependencies.git(input.cwd, ["config", "--get", "remote.origin.url"]); const repo = (remote && normalizeGitRepo(remote)) ?? ""; - const stored = (await dependencies.state.read()) ?? { version: 1 as const, sessions: {} }; - const previous = stored.sessions[input.sessionId]; - const current = { repo, branch, headSha: sha, lastSeenMs: observedAt }; + const store = dependencies.sessionState(input.sessionId); + const previous = await store.read(); + const current: SessionState = { version: 1, repo, branch, headSha: sha, lastSeenMs: observedAt }; + // Async hooks run concurrently; never replace a newer look at this session. + const remember = async () => { + const latest = await store.read(); + if (!latest || latest.lastSeenMs <= observedAt) await store.write(current); + }; if (previous && previous.repo === repo && previous.branch === branch && previous.headSha === sha) { - await dependencies.state.write(pruned(stored, input.sessionId, current, observedAt)); + await remember(); return "unchanged"; } @@ -98,7 +98,8 @@ export async function reportGitState( 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)); + await remember(); + if (!previous) await dependencies.pruneSessionStates(observedAt - stateRetentionMs); return "recorded"; } catch { return "failed"; @@ -119,19 +120,6 @@ function parseHookInput(text: string): { sessionId: string; cwd: string } | null return { sessionId, cwd }; } -function pruned( - state: HookState, - sessionId: string, - current: HookState["sessions"][string], - now: number, -): HookState { - const sessions = Object.entries({ ...state.sessions, [sessionId]: current }) - .filter(([, session]) => session.lastSeenMs >= now - stateRetentionMs) - .sort(([, a], [, b]) => b.lastSeenMs - a.lastSeenMs) - .slice(0, stateMaxSessions); - return { version: 1, sessions: Object.fromEntries(sessions) }; -} - /** * The key and organization to report with: an Understudy key from the * environment (the harness's own `ANTHROPIC_API_KEY` when it is one, never a diff --git a/src/hooks/settings.ts b/src/hooks/settings.ts index 0cf1fc6..a70c85e 100644 --- a/src/hooks/settings.ts +++ b/src/hooks/settings.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { constants } from "node:fs"; +import { constants, existsSync } from "node:fs"; import { lstat, open, readFile, rename, rm } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -40,9 +40,17 @@ export function gitStateCommand(entry: readonly string[], apiUrl: string | undef return words.map((word) => `'${word}'`).join(" "); } -/** 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]; +/** + * How this process was started: node with its script (which a package + * manager's `bin` link may name without an extension), or the compiled + * binary, whose script path exists only inside it. + */ +export function cliEntry( + execPath = process.execPath, + script = process.argv[1], + exists: (file: string) => boolean = existsSync, +): string[] { + return script && exists(script) ? [execPath, path.resolve(script)] : [execPath]; } type Settings = Record; diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 1639f75..7512f87 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -31,24 +31,26 @@ async function fixture(t) { return directory; } -function memoryStore(initial = null) { - const store = { value: initial, writes: 0 }; +function memorySessions(initial = {}) { + const sessions = new Map(Object.entries(initial)); + const pruned = []; return { - store, - state: { - read: async () => store.value, - write: async (value) => { - store.value = value; - store.writes += 1; - }, - clear: async () => { - store.value = null; - }, + sessions, + pruned, + sessionState: (id) => ({ + read: async () => sessions.get(id) ?? null, + write: async (value) => void sessions.set(id, value), + clear: async () => void sessions.delete(id), + }), + pruneSessionStates: async (olderThanMs) => { + pruned.push(olderThanMs); + for (const [id, session] of sessions) if (session.lastSeenMs < olderThanMs) sessions.delete(id); }, }; } function dependencies({ git = {}, state, fetchImplementation, credential, input, now = 1_000_000 }) { + state ??= memorySessions(); const answers = { "log -1 --format=%H%x00%ct": `${sha("a")}\u0000990`, "symbolic-ref -q --short HEAD": "feature-a", @@ -59,7 +61,8 @@ function dependencies({ git = {}, state, fetchImplementation, credential, input, now: () => now, readInput: async () => input ?? JSON.stringify({ session_id: sessionId, cwd: "/synthetic/repo", hook_event_name: "PostToolUse" }), git: async (_cwd, args) => (args.join(" ") in answers ? answers[args.join(" ")] : null), - state, + sessionState: state.sessionState, + pruneSessionStates: state.pruneSessionStates, credential: async () => (credential === undefined ? { apiKey: syntheticKey, organizationId: "org_synthetic" } : credential), fetchImplementation, }; @@ -86,7 +89,7 @@ test("normalizes every remote form of one repository, without credentials", () = }); test("reports a session's first git state, then only changes", async () => { - const { store, state } = memoryStore(); + const state = memorySessions(); const { calls, fetchImplementation } = recordingFetch(); assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation })), "recorded"); @@ -108,7 +111,7 @@ test("reports a session's first git state, then only changes", async () => { // Unchanged: no request, but the session's last look moves forward. assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation, now: 1_005_000 })), "unchanged"); assert.equal(calls.length, 1); - assert.equal(store.value.sessions[sessionId].lastSeenMs, 1_005_000); + assert.equal(state.sessions.get(sessionId).lastSeenMs, 1_005_000); // A new commit is reported with when the previous state was last seen. const changed = dependencies({ state, fetchImplementation, now: 1_010_000, git: { "log -1 --format=%H%x00%ct": `${sha("b")}\u00001008` } }); @@ -118,7 +121,7 @@ test("reports a session's first git state, then only changes", async () => { }); test("reports a detached HEAD with no branch and a repository with no origin", async () => { - const { state } = memoryStore(); + const state = memorySessions(); const { calls, fetchImplementation } = recordingFetch(); const detached = dependencies({ state, fetchImplementation, git: { "symbolic-ref -q --short HEAD": null, "config --get remote.origin.url": null } }); assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, detached), "recorded"); @@ -127,11 +130,11 @@ test("reports a detached HEAD with no branch and a repository with no origin", a }); test("keeps a failed report so the next hook sends it again", async () => { - const { store, state } = memoryStore(); + const state = memorySessions(); const { calls, fetchImplementation } = recordingFetch(500); assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation })), "failed"); assert.equal(calls.length, 1); - assert.equal(store.value, null); + assert.equal(state.sessions.size, 0); const throwing = async () => { throw new Error("offline"); }; @@ -139,7 +142,7 @@ test("keeps a failed report so the next hook sends it again", async () => { }); test("skips without a usable hook input, a repository, or a credential", async () => { - const { state } = memoryStore(); + const state = memorySessions(); const { calls, fetchImplementation } = recordingFetch(); const options = { harness: "claude_code", apiUrl: "https://api.example.test" }; for (const input of ["not json", "{}", JSON.stringify({ session_id: "has space", cwd: "/x" }), JSON.stringify({ session_id: sessionId, cwd: "relative" })]) { @@ -150,12 +153,21 @@ test("skips without a usable hook input, a repository, or a credential", async ( assert.equal(calls.length, 0); }); -test("forgets sessions after two weeks", async () => { - const old = { repo: "", branch: "", headSha: sha("c"), lastSeenMs: 0 }; - const { store, state } = memoryStore({ version: 1, sessions: { "old-session": old } }); +test("forgets other sessions untouched for two weeks when a new one starts", async () => { + const old = { version: 1, repo: "", branch: "", headSha: sha("c"), lastSeenMs: 0 }; + const state = memorySessions({ "old-session": old }); const { fetchImplementation } = recordingFetch(); await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation, now: 15 * 86_400_000 })); - assert.deepEqual(Object.keys(store.value.sessions), [sessionId]); + assert.deepEqual([...state.sessions.keys()], [sessionId]); + assert.deepEqual(state.pruned, [86_400_000]); +}); + +test("never replaces a newer look at the session with an older one", async () => { + const newer = { version: 1, repo: "github.com/acme/widgets", branch: "feature-a", headSha: sha("a"), lastSeenMs: 2_000_000 }; + const state = memorySessions({ [sessionId]: newer }); + const { fetchImplementation } = recordingFetch(); + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, dependencies({ state, fetchImplementation })), "unchanged"); + assert.equal(state.sessions.get(sessionId).lastSeenMs, 2_000_000); }); test("uses an Understudy key from the environment, never a provider key", () => { @@ -218,8 +230,11 @@ test("finds Claude Code settings and quotes the hook command", () => { "'/usr/bin/node' '/opt/cli/bin.js' 'hooks' 'git-state' '--harness' 'claude_code' '--api-url' 'http://localhost:8789'", ); assert.throws(() => gitStateCommand(["/opt/it's/understudy"], undefined), CliError); - assert.deepEqual(cliEntry("/usr/bin/node", "/opt/cli/dist/bin.js"), ["/usr/bin/node", "/opt/cli/dist/bin.js"]); - assert.deepEqual(cliEntry("/opt/understudy/understudy", "/$bunfs/root/understudy"), ["/opt/understudy/understudy"]); + const onDisk = (file) => file.startsWith("/opt/cli/") || file === "/usr/local/bin/understudy"; + assert.deepEqual(cliEntry("/usr/bin/node", "/opt/cli/dist/bin.js", onDisk), ["/usr/bin/node", "/opt/cli/dist/bin.js"]); + // A package manager's bin link has no extension. + assert.deepEqual(cliEntry("/usr/bin/node", "/usr/local/bin/understudy", onDisk), ["/usr/bin/node", "/usr/local/bin/understudy"]); + assert.deepEqual(cliEntry("/opt/understudy/understudy", "/$bunfs/root/understudy", onDisk), ["/opt/understudy/understudy"]); }); test("the hook command reports a real repository's state and prints nothing", async (t) => { @@ -270,10 +285,12 @@ test("the hook command reports a real repository's state and prints nothing", as { ...received[0].body, observed_at_ms: 0, head_committed_at_ms: 0 }, { harness: "claude_code", session_id: sessionId, observed_at_ms: 0, prev_seen_at_ms: null, repo: "github.com/acme/widgets", branch: "feature-a", head_sha: head, head_committed_at_ms: 0 }, ); - assert.equal((await stat(path.join(home, ".understudy", "work-hook-state.json"))).mode & 0o777, 0o600); + const [stateFile] = await readdir(path.join(home, ".understudy", "work-hooks")); + assert.match(stateFile, /^[0-9a-f]{32}\.json$/); + assert.equal((await stat(path.join(home, ".understudy", "work-hooks", stateFile))).mode & 0o777, 0o600); // An unreachable service changes nothing the agent can see. - await chmod(path.join(home, ".understudy"), 0o700); + await chmod(path.join(home, ".understudy", "work-hooks"), 0o700); server.close(); git("commit", "-q", "--allow-empty", "-m", "synthetic two"); assert.deepEqual(await run(), { code: 0, output: "" }); From 91761234d7a1c8f4bda658313dccab3b8bbba2e0 Mon Sep 17 00:00:00 2001 From: aamir Date: Wed, 4 Nov 2026 08:29:53 -0600 Subject: [PATCH 3/3] Lock each session's hook state and report after failed or 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. --- docs/hooks-commands.md | 4 ++-- src/commands/hooks.ts | 33 ++++++++++++++++++++++++++++++--- src/hooks/git-state.ts | 11 +++++++---- src/hooks/settings.ts | 11 +++++++---- tests/hooks.test.mjs | 17 +++++++++++++++-- 5 files changed, 61 insertions(+), 15 deletions(-) diff --git a/docs/hooks-commands.md b/docs/hooks-commands.md index b21bd59..c2ad197 100644 --- a/docs/hooks-commands.md +++ b/docs/hooks-commands.md @@ -1,9 +1,9 @@ # Work hooks -`understudy hooks install` adds two command hooks to Claude Code's settings +`understudy hooks install` adds command hooks to Claude Code's settings (`--settings `, else `$CLAUDE_CONFIG_DIR/settings.json`, else `~/.claude/settings.json`). Claude Code runs them on each prompt and after each -shell command. Understudy uses what they report to show what each pull request +shell command (`Bash` or `PowerShell`), including failed ones. Understudy uses what they report to show what each pull request cost: a request belongs to the pull request holding the next commit its session made. diff --git a/src/commands/hooks.ts b/src/commands/hooks.ts index 70cc791..73c9a87 100644 --- a/src/commands/hooks.ts +++ b/src/commands/hooks.ts @@ -1,6 +1,7 @@ import { execFile } from "node:child_process"; import { createHash } from "node:crypto"; -import { lstat, readdir, unlink } from "node:fs/promises"; +import { lstat, mkdir, readdir, rmdir, unlink } from "node:fs/promises"; +import { setTimeout as sleep } from "node:timers/promises"; import { homedir } from "node:os"; import path from "node:path"; import { promisify } from "node:util"; @@ -26,7 +27,7 @@ import { } from "../hooks/settings.js"; import { createInferenceCredentialStore } from "../inference/credentials.js"; import { formatOutput } from "../output.js"; -import { createPrivateJsonStore } from "../storage/private-json.js"; +import { createPrivateJsonStore, ensurePrivateDirectory } from "../storage/private-json.js"; const execute = promisify(execFile); const hookInputMaxBytes = 256 * 1024; @@ -105,10 +106,32 @@ export function createGitStateDependencies(): GitStateDependencies { sessionState: (sessionId) => createPrivateJsonStore({ directory, - fileName: `${createHash("sha256").update(sessionId).digest("hex").slice(0, 32)}.json`, + fileName: `${sessionFile(sessionId)}.json`, label: "work hook state", schema: sessionStateSchema, }), + // mkdir is atomic: whoever creates the directory holds the lock. A lock + // older than a hook's timeout belongs to a process that died. + lockSession: async (sessionId, operation) => { + await ensurePrivateDirectory(directory); + const lock = path.join(directory, `${sessionFile(sessionId)}.lock`); + for (let attempt = 0; attempt < 40; attempt += 1) { + try { + await mkdir(lock, { mode: 0o700 }); + } catch { + const entry = await lstat(lock).catch(() => null); + if (entry && Date.now() - entry.mtimeMs > 10_000) await rmdir(lock).catch(() => undefined); + await sleep(25); + continue; + } + try { + return await operation(); + } finally { + await rmdir(lock).catch(() => undefined); + } + } + return undefined; + }, pruneSessionStates: async (olderThanMs) => { for (const name of await readdir(directory).catch(() => [])) { if (!name.endsWith(".json")) continue; @@ -128,6 +151,10 @@ export function createGitStateDependencies(): GitStateDependencies { }; } +function sessionFile(sessionId: string): string { + return createHash("sha256").update(sessionId).digest("hex").slice(0, 32); +} + function readStdin(maxBytes: number, timeoutMs: number): Promise { return new Promise((resolve) => { const chunks: Buffer[] = []; diff --git a/src/hooks/git-state.ts b/src/hooks/git-state.ts index 6e3bb89..31d97b0 100644 --- a/src/hooks/git-state.ts +++ b/src/hooks/git-state.ts @@ -27,6 +27,8 @@ export interface GitStateDependencies { git(cwd: string, args: readonly string[]): Promise; /** One file per session, so concurrent hooks of different sessions never collide. */ sessionState(sessionId: string): PrivateJsonStore; + /** Runs `operation` while holding the session's lock; undefined when it can't be had. */ + lockSession(sessionId: string, operation: () => Promise): Promise; pruneSessionStates(olderThanMs: number): Promise; credential(): Promise; fetchImplementation?: typeof fetch; @@ -60,10 +62,11 @@ export async function reportGitState( const previous = await store.read(); const current: SessionState = { version: 1, repo, branch, headSha: sha, lastSeenMs: observedAt }; // Async hooks run concurrently; never replace a newer look at this session. - const remember = async () => { - const latest = await store.read(); - if (!latest || latest.lastSeenMs <= observedAt) await store.write(current); - }; + const remember = () => + dependencies.lockSession(input.sessionId, async () => { + const latest = await store.read(); + if (!latest || latest.lastSeenMs <= observedAt) await store.write(current); + }); if (previous && previous.repo === repo && previous.branch === branch && previous.headSha === sha) { await remember(); return "unchanged"; diff --git a/src/hooks/settings.ts b/src/hooks/settings.ts index a70c85e..79c478e 100644 --- a/src/hooks/settings.ts +++ b/src/hooks/settings.ts @@ -57,19 +57,22 @@ type Settings = Record; type HookGroup = { matcher?: string; hooks?: Array<{ type?: string; command?: string }> }; /** - * Claude Code runs the hook on each prompt and after each shell command, the - * only places git state changes before its next model request. `async` keeps - * the hook off the agent's path. + * Claude Code runs the hook on each prompt and after each shell command, + * failed ones included (a commit can precede the failure), the only places + * git state changes before its next model request. Windows sessions run + * shell commands as `PowerShell`. `async` keeps the hook off the agent's path. */ export function withGitStateHooks(settings: Settings, command: string): Settings { const handler = { type: "command", command, timeout: 10, async: true }; + const shell = { matcher: "Bash|PowerShell", hooks: [handler] }; const hooks = withoutGitStateHooks(settings).hooks as Record | undefined; return { ...settings, hooks: { ...hooks, UserPromptSubmit: [...(hooks?.UserPromptSubmit ?? []), { hooks: [handler] }], - PostToolUse: [...(hooks?.PostToolUse ?? []), { matcher: "Bash", hooks: [handler] }], + PostToolUse: [...(hooks?.PostToolUse ?? []), shell], + PostToolUseFailure: [...(hooks?.PostToolUseFailure ?? []), shell], }, }; } diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 7512f87..f845038 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -42,6 +42,7 @@ function memorySessions(initial = {}) { write: async (value) => void sessions.set(id, value), clear: async () => void sessions.delete(id), }), + lockSession: async (_id, operation) => operation(), pruneSessionStates: async (olderThanMs) => { pruned.push(olderThanMs); for (const [id, session] of sessions) if (session.lastSeenMs < olderThanMs) sessions.delete(id); @@ -62,6 +63,7 @@ function dependencies({ git = {}, state, fetchImplementation, credential, input, readInput: async () => input ?? JSON.stringify({ session_id: sessionId, cwd: "/synthetic/repo", hook_event_name: "PostToolUse" }), git: async (_cwd, args) => (args.join(" ") in answers ? answers[args.join(" ")] : null), sessionState: state.sessionState, + lockSession: state.lockSession, pruneSessionStates: state.pruneSessionStates, credential: async () => (credential === undefined ? { apiKey: syntheticKey, organizationId: "org_synthetic" } : credential), fetchImplementation, @@ -162,6 +164,15 @@ test("forgets other sessions untouched for two weeks when a new one starts", asy assert.deepEqual(state.pruned, [86_400_000]); }); +test("skips remembering the state when the session's lock can't be had", async () => { + const state = memorySessions(); + const { calls, fetchImplementation } = recordingFetch(); + const deps = { ...dependencies({ state, fetchImplementation }), lockSession: async () => undefined }; + assert.equal(await reportGitState({ harness: "claude_code", apiUrl: "https://api.example.test" }, deps), "recorded"); + assert.equal(calls.length, 1); + assert.equal(state.sessions.size, 0); +}); + test("never replaces a newer look at the session with an older one", async () => { const newer = { version: 1, repo: "github.com/acme/widgets", branch: "feature-a", headSha: sha("a"), lastSeenMs: 2_000_000 }; const state = memorySessions({ [sessionId]: newer }); @@ -200,11 +211,13 @@ test("installs the hooks once, keeping every other setting and hook", async (t) const installed = JSON.parse(await readFile(file, "utf8")); assert.equal(installed.model, "opus"); assert.deepEqual(installed.hooks.Stop, [{ hooks: [other] }]); + const handler = { type: "command", command, timeout: 10, async: true }; assert.deepEqual(installed.hooks.PostToolUse, [ { matcher: "Edit", hooks: [other] }, - { matcher: "Bash", hooks: [{ type: "command", command, timeout: 10, async: true }] }, + { matcher: "Bash|PowerShell", hooks: [handler] }, ]); - assert.deepEqual(installed.hooks.UserPromptSubmit, [{ hooks: [{ type: "command", command, timeout: 10, async: true }] }]); + assert.deepEqual(installed.hooks.PostToolUseFailure, [{ matcher: "Bash|PowerShell", hooks: [handler] }]); + assert.deepEqual(installed.hooks.UserPromptSubmit, [{ hooks: [handler] }]); assert.equal((await stat(file)).mode & 0o777, 0o640); assert.deepEqual((await readdir(directory)).sort(), ["settings.json"]);