From 2327ba228f3dd564958203d561f8e663dc3bbdb0 Mon Sep 17 00:00:00 2001 From: iaj6 Date: Sun, 12 Jul 2026 11:31:30 -0400 Subject: [PATCH] fix(cli): atomic hook state writes, SDK fetch timeout, correct git cwd 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 --- packages/cli/src/__tests__/git.test.ts | 104 ++++-------------- .../cli/src/__tests__/hook-handlers.test.ts | 40 +++++++ .../cli/src/__tests__/hook-ops-sdk.test.ts | 102 ++++++++++++++++- packages/cli/src/__tests__/hook.test.ts | 68 +++++++++++- packages/cli/src/commands/hook.ts | 55 ++++++--- packages/cli/src/git.ts | 35 +----- packages/cli/src/hook-ops.ts | 29 ++++- 7 files changed, 301 insertions(+), 132 deletions(-) diff --git a/packages/cli/src/__tests__/git.test.ts b/packages/cli/src/__tests__/git.test.ts index de92d6d..9e9893a 100644 --- a/packages/cli/src/__tests__/git.test.ts +++ b/packages/cli/src/__tests__/git.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { getCurrentRepo, getCurrentBranch, getDiff, getChangedFiles, getCommitLog, snapshotRef, getWorkingTreeDiff } from "../git.js"; +import { getCurrentRepo, getCurrentBranch, getChangedFiles, getWorkingTreeDiff } from "../git.js"; // Mock child_process.execSync vi.mock("node:child_process", () => ({ @@ -92,43 +92,6 @@ describe("getCurrentBranch", () => { }); }); -describe("getDiff", () => { - it("gets diff between two refs", () => { - mockExecSync.mockReturnValue("+added line\n"); - const result = getDiff("abc123", "def456"); - expect(mockExecSync).toHaveBeenCalledWith( - "git diff abc123 def456", - expect.objectContaining({ encoding: "utf-8" }), - ); - expect(result).toBe("+added line"); - }); - - it("gets diff from a single ref", () => { - mockExecSync.mockReturnValue("-removed\n"); - getDiff("abc123"); - expect(mockExecSync).toHaveBeenCalledWith( - "git diff abc123", - expect.objectContaining({ encoding: "utf-8" }), - ); - }); - - it("defaults to diff HEAD when no refs given", () => { - mockExecSync.mockReturnValue("some diff\n"); - getDiff(); - expect(mockExecSync).toHaveBeenCalledWith( - "git diff HEAD", - expect.objectContaining({ encoding: "utf-8" }), - ); - }); - - it("returns empty string when git fails", () => { - mockExecSync.mockImplementation(() => { - throw new Error("not a git repo"); - }); - expect(getDiff()).toBe(""); - }); -}); - describe("getChangedFiles", () => { it("parses porcelain status output", () => { // Note: git() trims the full output, so leading space on first line is lost. @@ -167,52 +130,22 @@ describe("getChangedFiles", () => { }); expect(getChangedFiles()).toEqual([]); }); -}); - -describe("getCommitLog", () => { - it("returns commit log output", () => { - mockExecSync.mockReturnValue("abc1234 Initial commit\ndef5678 Second commit\n"); - const log = getCommitLog(); - expect(log).toContain("abc1234 Initial commit"); - }); - - it("passes since argument when provided", () => { - mockExecSync.mockReturnValue("abc1234 Recent commit\n"); - getCommitLog("2025-01-01"); - expect(mockExecSync).toHaveBeenCalledWith( - expect.stringContaining('--since="2025-01-01"'), - expect.anything(), - ); - }); - - it("defaults to -10 when no since provided", () => { - mockExecSync.mockReturnValue("some log\n"); - getCommitLog(); - expect(mockExecSync).toHaveBeenCalledWith( - expect.stringContaining("-10"), - expect.anything(), - ); - }); -}); - -describe("snapshotRef", () => { - it("returns stash create result when available", () => { - mockExecSync.mockReturnValueOnce("abc123def\n"); - expect(snapshotRef()).toBe("abc123def"); - }); - it("falls back to rev-parse HEAD when stash create returns empty", () => { - mockExecSync - .mockReturnValueOnce("\n") // stash create returns empty - .mockReturnValueOnce("deadbeef\n"); // rev-parse HEAD - expect(snapshotRef()).toBe("deadbeef"); + it("passes cwd through to git when provided", () => { + // finalizeSession runs in the hook subprocess, whose cwd is wherever + // Claude Code was launched from — the status must run in the tracked + // repo (state.cwd) or the run records another repo's changes. + mockExecSync.mockReturnValue("M src/index.ts\n"); + getChangedFiles("/some/working/dir"); + const [, opts] = mockExecSync.mock.calls[0]!; + expect((opts as { cwd?: string }).cwd).toBe("/some/working/dir"); }); - it("returns empty string when both fail", () => { - mockExecSync.mockImplementation(() => { - throw new Error("not a git repo"); - }); - expect(snapshotRef()).toBe(""); + it("does not set cwd when none is provided", () => { + mockExecSync.mockReturnValue("\n"); + getChangedFiles(); + const [, opts] = mockExecSync.mock.calls[0]!; + expect((opts as { cwd?: string }).cwd).toBeUndefined(); }); }); @@ -227,4 +160,13 @@ describe("getWorkingTreeDiff", () => { mockExecSync.mockReturnValue("\n"); expect(getWorkingTreeDiff()).toBe(""); }); + + it("passes cwd through to git when provided", () => { + mockExecSync.mockReturnValue("+new line\n"); + getWorkingTreeDiff("/some/working/dir"); + expect(mockExecSync).toHaveBeenCalledWith( + "git diff HEAD", + expect.objectContaining({ cwd: "/some/working/dir" }), + ); + }); }); diff --git a/packages/cli/src/__tests__/hook-handlers.test.ts b/packages/cli/src/__tests__/hook-handlers.test.ts index e76af23..85f5d75 100644 --- a/packages/cli/src/__tests__/hook-handlers.test.ts +++ b/packages/cli/src/__tests__/hook-handlers.test.ts @@ -9,6 +9,7 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { execSync } from "node:child_process"; import { _readState, _cleanupState, @@ -794,6 +795,45 @@ describe("handleSessionEnd", () => { } }); + it("computes the git diff from the session's cwd, not the hook process cwd", async () => { + // Build a real throwaway git repo under tmpHome with one unstaged + // modification. The vitest process itself runs from the agentops repo, + // so if finalizeSession ran git without a cwd it would diff THIS repo + // (and never see the marker below). + const repoDir = join(tmpHome, "session-repo"); + mkdirSync(repoDir, { recursive: true }); + const g = (cmd: string) => + execSync(`git ${cmd}`, { cwd: repoDir, stdio: ["pipe", "pipe", "pipe"] }); + g("init"); + writeFileSync(join(repoDir, "tracked.txt"), "original contents\n", "utf-8"); + g("add tracked.txt"); + g('-c user.email=t@t.test -c user.name=t commit -m init'); + writeFileSync( + join(repoDir, "tracked.txt"), + "MARKER-changed-in-session-repo\n", + "utf-8", + ); + + const sid = freshSessionId(); + await _handleSessionStart({ session_id: sid, cwd: repoDir }, testDbPath); + const state = _readState(sid)!; + + await runHook(() => _handleSessionEnd({ session_id: sid, cwd: repoDir }, testDbPath)); + + const db = getDb(testDbPath); + const run = getRun(db, createRunId(state.runId))!; + const diffs = run.artifacts.flatMap((a) => a.diffs).join("\n"); + expect(diffs).toContain("MARKER-changed-in-session-repo"); + + // changedFilesCount flows into the run.completed event payload. + const events = listEvents(db, { limit: 20 }); + const completed = events.find( + (e) => e.type === "run.completed" && e.sourceId === state.runId, + )!; + expect(completed).toBeDefined(); + expect((completed.payload as { filesChanged?: number }).filesChanged).toBe(1); + }); + it("emits run.completed event", async () => { const sid = freshSessionId(); await _handleSessionStart({ session_id: sid, cwd: "/tmp/test-cwd" }, testDbPath); diff --git a/packages/cli/src/__tests__/hook-ops-sdk.test.ts b/packages/cli/src/__tests__/hook-ops-sdk.test.ts index abaa7a5..476f0ee 100644 --- a/packages/cli/src/__tests__/hook-ops-sdk.test.ts +++ b/packages/cli/src/__tests__/hook-ops-sdk.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { existsSync, mkdirSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { resolve } from "node:path"; -import { createOps, resolveOpsConfig, isSdkMode, SdkError } from "../hook-ops.js"; +import { createOps, resolveOpsConfig, isSdkMode, SdkError, sdkTimeoutMs } from "../hook-ops.js"; import { outboxPath } from "../outbox.js"; import type { Action, Metrics } from "@agentops/core"; import { createActionId } from "@agentops/core"; @@ -136,6 +136,106 @@ describe("resolveOpsConfig + createOps", () => { }); }); +// ─── Fetch timeout ───────────────────────────────────────────────────────── +// +// A dashboard that HANGS (accepts the connection but never responds) must +// not stall the hook until Claude Code's 60s hook timeout. Every SDK fetch +// carries AbortSignal.timeout(sdkTimeoutMs()); the resulting rejection is +// caught like any network error (status 0), so it follows the existing +// transient semantics: outboxed for reports, thrown (→ fail-open in the +// handlers) for synchronous policy decisions. + +describe("SDK fetch timeout", () => { + let savedTimeout: string | undefined; + + beforeEach(() => { + savedTimeout = process.env["AGENTOPS_SDK_TIMEOUT_MS"]; + }); + + afterEach(() => { + if (savedTimeout === undefined) delete process.env["AGENTOPS_SDK_TIMEOUT_MS"]; + else process.env["AGENTOPS_SDK_TIMEOUT_MS"] = savedTimeout; + }); + + /** Fetch stub that never resolves — only rejects when its signal aborts. */ + function mockHangingFetch(): ReturnType { + const fn = vi.fn( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init.signal; + if (!signal) return; // no signal → hang forever (test would time out) + if (signal.aborted) reject(signal.reason); + else signal.addEventListener("abort", () => reject(signal.reason)); + }), + ); + vi.stubGlobal("fetch", fn); + return fn; + } + + it("defaults to 5000ms, honors AGENTOPS_SDK_TIMEOUT_MS, ignores garbage", () => { + delete process.env["AGENTOPS_SDK_TIMEOUT_MS"]; + expect(sdkTimeoutMs()).toBe(5000); + process.env["AGENTOPS_SDK_TIMEOUT_MS"] = "250"; + expect(sdkTimeoutMs()).toBe(250); + process.env["AGENTOPS_SDK_TIMEOUT_MS"] = "not-a-number"; + expect(sdkTimeoutMs()).toBe(5000); + process.env["AGENTOPS_SDK_TIMEOUT_MS"] = "-1"; + expect(sdkTimeoutMs()).toBe(5000); + }); + + it("attaches an AbortSignal to every SDK fetch", async () => { + const fetchMock = mockFetch([{ status: 200, body: { ok: true } }]); + const ops = createOps(resolveOpsConfig(), "timeout-signal-session"); + + await ops.reportAction("run_abc", makeAction()); + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(init.signal).toBeInstanceOf(AbortSignal); + }); + + it("checkPolicy against a hanging server rejects with SdkError status 0 (transient)", async () => { + process.env["AGENTOPS_SDK_TIMEOUT_MS"] = "25"; + mockHangingFetch(); + const ops = createOps(resolveOpsConfig(), "timeout-hang-session"); + + let caught: unknown; + try { + await ops.checkPolicy({ + runId: "run_abc", + toolName: "Bash", + toolInput: { command: "ls" }, + cumulativeCostUsd: 0, + }); + expect.unreachable("checkPolicy should have thrown"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(SdkError); + // Status 0 = network-shaped failure → handlers fail-open (or block + // under AGENTOPS_FAIL_CLOSED), exactly like a refused connection. + expect((caught as SdkError).status).toBe(0); + }); + + it("reportAction against a hanging server is queued in the outbox, not thrown", async () => { + process.env["AGENTOPS_SDK_TIMEOUT_MS"] = "25"; + mockHangingFetch(); + const ops = createOps(resolveOpsConfig(), "timeout-outbox-session"); + + const errSpy = vi + .spyOn(process.stderr, "write") + .mockImplementation(() => true); + try { + await expect(ops.reportAction("run_abc", makeAction())).resolves.toBeUndefined(); + } finally { + errSpy.mockRestore(); + } + + const outbox = readOutbox("timeout-outbox-session"); + expect(outbox).toHaveLength(1); + expect(outbox[0]!.op).toBe("reportAction"); + }); +}); + // ─── Happy path ──────────────────────────────────────────────────────────── describe("SdkOps happy path", () => { diff --git a/packages/cli/src/__tests__/hook.test.ts b/packages/cli/src/__tests__/hook.test.ts index 4b372f9..950ed7e 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { Command } from "commander"; -import { existsSync, readFileSync, writeFileSync, unlinkSync, mkdirSync } from "node:fs"; +import { existsSync, readFileSync, writeFileSync, unlinkSync, mkdirSync, readdirSync, rmSync } from "node:fs"; import { tmpdir, homedir } from "node:os"; import { join, resolve } from "node:path"; import { registerHookCommand } from "../commands/hook.js"; @@ -139,6 +139,72 @@ describe("State file management", () => { }); }); +// ─── Atomic state write tests ──────────────────────────────────────────────── +// +// writeState must be temp-file + rename (atomic on POSIX): the state file is +// shared between SubagentStop's read-modify-write and parallel subagents' +// PreToolUse/PostToolUse readers. A torn read would make readState return +// null and silently skip policy enforcement for that tool call. + +describe("Atomic state writes", () => { + const testSessionId = "test-session-atomic-" + Date.now(); + + const state: HookState = { + runId: "run_atomic", + sessionId: "session_atomic", + dbPath: "/tmp/test.db", + startTime: "2025-01-01T00:00:00.000Z", + agentsSpawned: 0, + agentsCompleted: 0, + finalized: false, + }; + + afterEach(() => { + _cleanupState(testSessionId); + }); + + it("leaves no temp file behind after a successful write", () => { + _writeState(testSessionId, state); + const dir = join(homedir(), ".agentops", "state"); + const leftovers = readdirSync(dir).filter( + (f) => f.startsWith(`${testSessionId}.json.`) && f.endsWith(".tmp"), + ); + expect(leftovers).toEqual([]); + }); + + it("produces complete, valid JSON on disk", () => { + _writeState(testSessionId, state); + const raw = readFileSync(stateFilePath(testSessionId), "utf-8"); + expect(JSON.parse(raw)).toEqual(state); + }); + + it("overwrite replaces the whole file, never mixes contents", () => { + // A longer first payload followed by a shorter one would leave trailing + // garbage under a naive truncate-less in-place write scheme. + const long: HookState = { ...state, cwd: "/very/long/path/".repeat(50) }; + _writeState(testSessionId, long); + _writeState(testSessionId, state); + const raw = readFileSync(stateFilePath(testSessionId), "utf-8"); + expect(JSON.parse(raw)).toEqual(state); + }); + + it("cleans up the temp file when the rename fails", () => { + // Occupy the final path with a directory so renameSync fails. + const finalPath = stateFilePath(testSessionId); + mkdirSync(finalPath, { recursive: true }); + try { + expect(() => _writeState(testSessionId, state)).toThrow(); + const dir = join(homedir(), ".agentops", "state"); + const leftovers = readdirSync(dir).filter( + (f) => f.startsWith(`${testSessionId}.json.`) && f.endsWith(".tmp"), + ); + expect(leftovers).toEqual([]); + } finally { + rmSync(finalPath, { recursive: true, force: true }); + } + }); +}); + // ─── Action mapping tests ─────────────────────────────────────────────────── describe("Tool-to-action mapping", () => { diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index dcd4d23..8fa7329 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -1,5 +1,5 @@ import { Command } from "commander"; -import { readFileSync, writeFileSync, unlinkSync, existsSync, mkdirSync, chmodSync, statSync, realpathSync } from "node:fs"; +import { readFileSync, writeFileSync, renameSync, unlinkSync, existsSync, mkdirSync, chmodSync, statSync, realpathSync } from "node:fs"; import { homedir } from "node:os"; import { join, resolve, sep } from "node:path"; import { @@ -76,12 +76,34 @@ function writeState(claudeSessionId: string, state: HookState): void { mkdirSync(dir, { recursive: true, mode: 0o700 }); } const path = stateFilePath(claudeSessionId); - writeFileSync(path, JSON.stringify(state), { encoding: "utf-8", mode: 0o600 }); - // chmod in case the file already existed with looser perms. + // Atomic write: temp file in the same directory, then rename (atomic on + // POSIX) — same pattern as Outbox.drain in outbox.ts. The state file is + // shared: handleSubagentStop does a read-modify-write while parallel + // subagents' PreToolUse/PostToolUse hooks read it concurrently. A bare + // writeFileSync can expose a torn file; readState swallows the parse + // error and returns null, which silently skips policy enforcement for + // that tool call. The pid suffix keeps two concurrent hook processes + // from clobbering each other's temp file. + const tmp = `${path}.${process.pid}.tmp`; try { - chmodSync(path, 0o600); - } catch { - // Best-effort; not all filesystems support chmod. + writeFileSync(tmp, JSON.stringify(state), { encoding: "utf-8", mode: 0o600 }); + // chmod in case the temp file already existed with looser perms + // (writeFileSync's mode only applies at creation). The rename carries + // the temp file's mode to the final path. + try { + chmodSync(tmp, 0o600); + } catch { + // Best-effort; not all filesystems support chmod. + } + renameSync(tmp, path); + } catch (err) { + // Don't leave a stray temp file behind on failure. + try { + unlinkSync(tmp); + } catch { + // Ignore — the temp file may never have been created. + } + throw err; } } @@ -148,15 +170,6 @@ function readHookUsage( return { usage, backend }; } -// ─── Stale state detection ─────────────────────────────────────────────────── - -function handleStaleState(sessionId: string, state: HookState): void { - process.stderr.write( - `[agentops] Stale session detected (database was reset). Restart Claude Code to begin tracking.\n`, - ); - cleanupState(sessionId); -} - // ─── Fail-closed enforcement ───────────────────────────────────────────────── // // Default behavior is fail-open: when the SDK can't reach the dashboard or @@ -445,9 +458,15 @@ function opsConfigFromState(state: HookState, dbPath?: string) { async function finalizeSession(input: HookInput, state: HookState, dbPath?: string): Promise { const ops = createOps(opsConfigFromState(state, dbPath), input.session_id); - // Compute wall time + git diff locally — both are on this machine. - const changedFiles = getChangedFiles(); - const diff = getWorkingTreeDiff(); + // Compute wall time + git diff locally — both are on this machine. Run + // git in the session's tracked directory (state.cwd, captured at + // session-start) rather than the hook subprocess's inherited cwd — the + // subprocess inherits wherever Claude Code was launched from, which may + // be a different repo entirely. Same reasoning as the cwd passed to the + // git lookups in handleSessionStart. + const gitCwd = state.cwd ?? input.cwd; + const changedFiles = getChangedFiles(gitCwd); + const diff = getWorkingTreeDiff(gitCwd); const wallTimeMs = Date.now() - new Date(state.startTime).getTime(); // Read final cost/token usage from the local transcript. diff --git a/packages/cli/src/git.ts b/packages/cli/src/git.ts index 9104a90..5f2205d 100644 --- a/packages/cli/src/git.ts +++ b/packages/cli/src/git.ts @@ -51,24 +51,13 @@ export function getCurrentBranch(cwd?: string): string { return git("rev-parse --abbrev-ref HEAD", cwd) || "unknown"; } -export function getDiff(fromRef?: string, toRef?: string): string { - if (fromRef && toRef) { - return git(`diff ${fromRef} ${toRef}`); - } - if (fromRef) { - return git(`diff ${fromRef}`); - } - // Default: working tree diff (staged + unstaged) - return git("diff HEAD"); -} - export interface ChangedFile { status: "added" | "modified" | "deleted" | "renamed" | "unknown"; path: string; } -export function getChangedFiles(): ChangedFile[] { - const output = git("status --porcelain"); +export function getChangedFiles(cwd?: string): ChangedFile[] { + const output = git("status --porcelain", cwd); if (!output) return []; return output.split("\n").filter(Boolean).map((line) => { @@ -98,24 +87,10 @@ export function getChangedFiles(): ChangedFile[] { }); } -export function getCommitLog(since?: string): string { - const sinceArg = since ? ` --since="${since}"` : " -10"; - return git(`log --oneline${sinceArg}`); -} - -/** - * Take a snapshot of the current working tree state for diff comparison. - * Returns the current HEAD commit hash (or empty string if no commits). - */ -export function snapshotRef(): string { - return git("stash create") || git("rev-parse HEAD") || ""; -} - /** - * Get the diff of all changes in the working tree (staged + unstaged + untracked shown as new). + * Get the diff of all changes in the working tree (staged + unstaged). */ -export function getWorkingTreeDiff(): string { +export function getWorkingTreeDiff(cwd?: string): string { // Include both staged and unstaged - const tracked = git("diff HEAD"); - return tracked; + return git("diff HEAD", cwd); } diff --git a/packages/cli/src/hook-ops.ts b/packages/cli/src/hook-ops.ts index 00865da..df382ca 100644 --- a/packages/cli/src/hook-ops.ts +++ b/packages/cli/src/hook-ops.ts @@ -544,6 +544,29 @@ class DirectOps implements HookOps { // stays in "running" status until an admin intervenes, which is surfaced // to the operator via a loud stderr warning. +// Every SDK HTTP call is bounded by this timeout. Without one, a dashboard +// that HANGS (rather than refuses the connection) stalls checkPolicy on +// every PreToolUse until Claude Code's own hook timeout (60s default), +// freezing the user's session once per tool call — the fail-open path only +// helps when the request errors. Default 5s; override with the +// AGENTOPS_SDK_TIMEOUT_MS env var (positive integer, milliseconds). +// +// A timeout rejects the fetch, which post() catches like any other network +// error (status 0) — so it flows through the existing transient semantics: +// SdkError(status 0) → outboxed for reportAction/reportArtifact/ +// reportMetrics, fail-open (or AGENTOPS_FAIL_CLOSED block) for +// checkPolicy/checkBudget. +const DEFAULT_SDK_TIMEOUT_MS = 5000; + +export function sdkTimeoutMs(): number { + const raw = process.env["AGENTOPS_SDK_TIMEOUT_MS"]?.trim(); + if (raw) { + const n = Number(raw); + if (Number.isFinite(n) && n > 0) return n; + } + return DEFAULT_SDK_TIMEOUT_MS; +} + class SdkOps implements HookOps { private readonly base: string; private readonly token: string; @@ -589,11 +612,15 @@ class SdkOps implements HookOps { Authorization: `Bearer ${this.token}`, }, body: JSON.stringify(body), + // Bound every call so a hanging dashboard can't stall the hook + // (and with it, the user's Claude Code session). See sdkTimeoutMs. + signal: AbortSignal.timeout(sdkTimeoutMs()), }); const data = (await res.json().catch(() => ({}))) as T; return { status: res.status, data }; } catch (err) { - // Network failure surfaces as status 0 so callers treat as transient. + // Network failure (including timeout aborts) surfaces as status 0 + // so callers treat it as transient. const message = err instanceof Error ? err.message : String(err); return { status: 0, data: { error: message } as unknown as T }; }