fix(cli): atomic hook state writes, SDK fetch timeout, correct git cwd - #25
Merged
Merged
Conversation
Hardens the Claude Code hook pipeline per the July 2026 audit: - writeState now writes to a pid-suffixed temp file and renameSync's it into place (atomic on POSIX, same pattern as Outbox.drain). Prevents parallel-subagent hooks from reading a torn state file, which made readState return null and silently skipped policy enforcement for that tool call. 0600 mode preserved; temp file cleaned up on failure. - Every SdkOps fetch now carries AbortSignal.timeout(sdkTimeoutMs()) so a hanging (rather than refusing) dashboard can't stall checkPolicy on each PreToolUse until Claude Code's 60s hook timeout. Default 5s, overridable via AGENTOPS_SDK_TIMEOUT_MS. Timeouts surface through the existing network-error path (status 0 -> transient SdkError): reports are outboxed, policy checks fail-open (or block under AGENTOPS_FAIL_CLOSED). - finalizeSession now passes state.cwd ?? input.cwd into getChangedFiles/getWorkingTreeDiff (both grew an optional cwd param), matching the cwd threading session-start already did. Previously the hook subprocess diffed whatever directory Claude Code was launched from, recording another repo's changes on the run. - Deleted dead, shell-interpolating git helpers getDiff, getCommitLog, snapshotRef (zero callers outside their own tests) and their tests. - Deleted dead handleStaleState: its call sites (getRun-returns-null after a DB reset) were removed when the HookOps abstraction landed in Phase 3.4, and the current handler layer never sees run existence, so there is no sensible call site without widening the HookOps contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hook-pipeline hardening from the July audit's medium tier:
writeStatenow writes a pid-suffixed temp file andrenameSyncs it into place (mirroring the outbox pattern), so parallel-subagent hooks can never observe a torn state file (a torn read silently skipped policy enforcement for that tool call). 0600/0700 modes preserved; temp cleaned up on rename failure.AbortSignal.timeout(...), default 5s, overridable viaAGENTOPS_SDK_TIMEOUT_MS. A hanging (not refusing) dashboard used to stall every PreToolUse for Claude Code's full 60s hook timeout. Timeouts flow through the existing status-0SdkErrorpath, so fail-open/fail-closed and outbox semantics are unchanged.getChangedFiles/getWorkingTreeDiffaccept acwdandfinalizeSessionpasses the session's; previously a Claude Code session launched from outside the tracked repo recorded another repo's uncommitted changes as the run's diff.getDiff/getCommitLog/snapshotRef(zero callers, shell-interpolatedexecSyncargs — latent injection) andhandleStaleState(call sites removed in the Phase 3.4 HookOps refactor; DirectOps/SdkOps no longer observe run existence, so there is no sensible call site).Tests
New suites: atomic-write behavior (no temp leftovers, no torn content, cleanup on rename failure), timeout (env parsing, every fetch has a signal, hanging-server stub → fail-open + outbox), git cwd threading, and an end-to-end hook test against a real throwaway git repo asserting the run diff comes from the session's cwd (fails pre-fix). Full workspace suite green (1,294 tests).
🤖 Generated with Claude Code