From e7f7eea71fd553d45c7a654407c99f4e50472acf Mon Sep 17 00:00:00 2001 From: Austin Kurpuis Date: Mon, 27 Jul 2026 20:04:15 -0700 Subject: [PATCH 1/2] Bound a remote-control turn by silence, not by a 30-minute stopwatch An agent turn on issue #149 ran correctly for half an hour and was then killed and reported to the user as "Timed out after 1800000ms waiting for the remote-control session to finish". The run had never once been idle. `runClaudeTurnRemoteControlled` was the only bound in the system that measured DURATION rather than SILENCE, and it was the shortest one by two orders of magnitude: - gateway poll budget 8h5m (values-production, fixed for this exact bug class: "a bound shorter than the work it's bounding turns a success into a visible failure") - Job activeDeadlineSeconds 8h ("a real coding task ... can legitimately take hours") - orchestrator awaitReply 10min of silence, reset by any up-message - remote-control runner 30min WALL CLOCK <-- the outlier The one-shot `claude -p` path has no wall-clock cap at all; it waits for the child. A feature-sized coding task sits comfortably past 30 minutes, so the cap turned finished work into a reported failure. Replace it with an idle bound over the session's own transcript, which gains an entry for every assistant message, tool call, and tool result: growth is proof of life, and a static transcript is the only honest definition of stuck. Deliberately not measured from the pty output, which a TUI redraws continuously whether or not anything is happening. There is no absolute cap by default any more -- the Job's activeDeadlineSeconds is the ceiling, as it already claims to be. `maxWaitMs` survives as an opt-in escape hatch, and both bounds are env/Helm tunable so a wrong default costs a values change rather than a rebuild. Three defects fall out of the same rewrite: - The heartbeat was a content-free ticker that fired every 20s whether or not anything was happening, so a genuinely wedged session looked alive for the full 30 minutes. It now narrates each new transcript entry as it appears (the tool-call trail the `-p` path already emits), and only says "still running" during real silence, naming how long the silence has lasted. - Giving up now says WHICH way it failed. "Timed out after 1800000ms" could not distinguish a session that worked and then wedged from one whose prompt never landed -- and not being able to tell those apart is most of why this issue survived four attempts at it. Startup gets its own, much shorter bound, because before a session registers there is no transcript to measure silence against and a CLI wedged on an unexpected first-run prompt never exits. - `agents --json --all` lists ended sessions too, so on a second turn in the same pod the previous turn's session is still listed at the same cwd -- and its transcript already carries a `turn_duration`. Matching it would hand the previous turn's answer back as this turn's, instantly and silently. The pre-existing sessions are now snapshotted before we spawn ours, and skipped. The fakes in claude-runner-remote-control.test.ts grew a sentinel so a session only appears in `claude agents` once its process has started, which is what a real one does and what the snapshot above relies on. New coverage, all against real subprocesses and a real transcript on disk: a turn that runs many times longer than the idle window while working is not killed (and no `maxWaitMs` is passed, because there is no absolute cap); a worked-then-wedged session is reported as a stall naming the silence; a registered-but-never-started session is reported distinctly; a never-registering session gives up on the startup bound rather than the idle one; an explicit `maxWaitMs` still caps; and a prior turn's session is not mistaken for this one's. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NyKcQdvju68R3edo4GXD3T --- .../src/agents/nats-agent-channel.ts | 10 +- .../src/claude-runner-remote-control.test.ts | 202 ++++++++++++- .../src/claude-runner.ts | 277 +++++++++++++++--- apps/claude-code-swe-agent/src/config.ts | 28 ++ apps/claude-code-swe-agent/src/index.ts | 7 +- .../templates/agent-claude-code-swe.yaml | 8 + .../community-components/values-ci-all.yaml | 4 + charts/community-components/values.yaml | 12 + 8 files changed, 499 insertions(+), 49 deletions(-) diff --git a/apps/agent-orchestrator/src/agents/nats-agent-channel.ts b/apps/agent-orchestrator/src/agents/nats-agent-channel.ts index 8f6e053..d00ec30 100644 --- a/apps/agent-orchestrator/src/agents/nats-agent-channel.ts +++ b/apps/agent-orchestrator/src/agents/nats-agent-channel.ts @@ -149,8 +149,14 @@ export interface AgentOrchestratorChannel { * - during ordinary work the up subject carries every `opencode_event` * (`opencode-swe-agent`'s `subscribeEvents`), i.e. near-continuous; * - the remote-control wait — the one place a turn blocks on a human — - * heartbeats "still running…" every 20s (`claude-runner.ts`) and caps - * itself at 30 min anyway. + * narrates each transcript entry as it appears and, while the session is + * quiet, heartbeats every 20s (`claude-runner.ts`). + * + * That runner owns the decision about whether a remote-control turn is stuck, + * because it is the only component that can see the session's actual activity; + * this bound never fires for one. It deliberately does NOT cap total duration + * (it used to cap at 30 min — see issue #149, where that killed a healthy + * turn), so do not restore a wall-clock cap here either. * * Note that an agent asking a question does NOT sit inside `awaitReply`: * `ask()` publishes `reply{final:false}` (`agent-runtime`'s `runtime.ts`), diff --git a/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts b/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts index 53c3bb8..0da1e23 100644 --- a/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts +++ b/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts @@ -1,4 +1,4 @@ -import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -32,6 +32,20 @@ async function installFake(name: string, body: string): Promise { // Fake `script -q -c /dev/null`: simulate the interactive session by // writing its transcript, then stay alive until the runner kills us. +// +// Modes: +// complete - bridge line + assistant text + turn_duration, written at once +// running - bridge line + assistant text, never completes (works, then wedges) +// stalled - bridge line only (registers, but the turn never begins) +// streaming - bridge line, then a tool_use entry every FAKE_STEP_MS for +// FAKE_STEPS steps, then assistant text + turn_duration. Models a +// long but healthy turn -- the case issue #149 killed. +// nofile - no transcript at all +// +// The sentinel file matters: a real session does not appear in `claude agents` +// until its process is running, and the runner now snapshots the pre-existing +// sessions before it spawns. A fake that listed the session unconditionally +// would claim our own session already existed before we started it. const FAKE_SCRIPT = ` const fs = require("fs"), path = require("path"); const HOME = process.env.HOME, SID = process.env.FAKE_SID; @@ -40,27 +54,59 @@ const HOME = process.env.HOME, SID = process.env.FAKE_SID; // path hash would then differ from what the runner reads (a test-only quirk; // Linux/production has no such symlink). const dir = path.join(HOME, ".claude", "projects", process.env.FAKE_SESSION_CWD.replace(/[/.]/g, "-")); +const file = path.join(dir, SID + ".jsonl"); const mode = process.env.FAKE_SCRIPT_MODE || "complete"; +const assistant = (text) => JSON.stringify({ type: "assistant", message: { role: "assistant", content: [{ type: "text", text }] } }); +const toolUse = (name, i) => JSON.stringify({ type: "assistant", message: { role: "assistant", content: [{ type: "tool_use", id: "t" + i, name, input: {} }] } }); +const done = () => JSON.stringify({ type: "system", subtype: "turn_duration", durationMs: 5 }); +const bridge = () => JSON.stringify({ type: "summary", bridgeSessionId: "cse_TESTBRIDGE", lastSequenceNum: 0 }); + +// Announce that this session now exists (see comment above). +fs.writeFileSync(path.join(HOME, "session-started"), "1"); if (process.env.FAKE_SCRIPT_OUTPUT) process.stdout.write(process.env.FAKE_SCRIPT_OUTPUT); + if (mode !== "nofile") { fs.mkdirSync(dir, { recursive: true }); - const lines = [JSON.stringify({ type: "summary", bridgeSessionId: "cse_TESTBRIDGE", lastSequenceNum: 0 })]; - if (mode !== "running") lines.push(JSON.stringify({ type: "assistant", message: { role: "assistant", content: [{ type: "text", text: process.env.FAKE_ASSISTANT_TEXT || "DONE" }] } })); - if (mode === "complete") lines.push(JSON.stringify({ type: "system", subtype: "turn_duration", durationMs: 5 })); - fs.writeFileSync(path.join(dir, SID + ".jsonl"), lines.join("\\n") + "\\n"); + if (mode === "streaming") { + fs.writeFileSync(file, bridge() + "\\n"); + const stepMs = Number(process.env.FAKE_STEP_MS || 40); + const steps = Number(process.env.FAKE_STEPS || 15); + let i = 0; + const timer = setInterval(() => { + if (++i <= steps) { fs.appendFileSync(file, toolUse("Bash", i) + "\\n"); return; } + clearInterval(timer); + fs.appendFileSync(file, assistant(process.env.FAKE_ASSISTANT_TEXT || "DONE") + "\\n" + done() + "\\n"); + }, stepMs); + } else { + const lines = [bridge()]; + if (mode !== "stalled") lines.push(assistant(process.env.FAKE_ASSISTANT_TEXT || "DONE")); + if (mode === "complete") lines.push(done()); + fs.writeFileSync(file, lines.join("\\n") + "\\n"); + } } if (process.env.FAKE_SCRIPT_EXIT === "1") process.exit(0); setInterval(() => {}, 1e9); `; -// Fake `claude`: only `agents --json --all` is used by this path. +// Fake `claude`: only `agents --json --all` is used by this path. Our session +// is listed only once the fake `script` has started it; FAKE_PRIOR_SID (if set) +// is a session from an earlier turn, listed from the very beginning and ahead +// of ours -- exactly the ordering that would trap a first-match lookup. const FAKE_CLAUDE = ` +const fs = require("fs"), path = require("path"); const args = process.argv.slice(2); if (args[0] === "agents") { - if (process.env.FAKE_NO_SESSION === "1") { console.log("[]"); process.exit(0); } - const s = { kind: "interactive", cwd: process.env.FAKE_SESSION_CWD, sessionId: process.env.FAKE_SID, id: "short1", status: "idle" }; - if (process.env.FAKE_SESSION_STATE) s.state = process.env.FAKE_SESSION_STATE; - console.log(JSON.stringify([s])); + const list = []; + if (process.env.FAKE_PRIOR_SID) { + list.push({ kind: "interactive", cwd: process.env.FAKE_SESSION_CWD, sessionId: process.env.FAKE_PRIOR_SID, id: "short0", status: "idle" }); + } + const started = fs.existsSync(path.join(process.env.HOME, "session-started")); + if (started && process.env.FAKE_NO_SESSION !== "1") { + const s = { kind: "interactive", cwd: process.env.FAKE_SESSION_CWD, sessionId: process.env.FAKE_SID, id: "short1", status: "idle" }; + if (process.env.FAKE_SESSION_STATE) s.state = process.env.FAKE_SESSION_STATE; + list.push(s); + } + console.log(JSON.stringify(list)); process.exit(0); } process.exit(0); @@ -115,6 +161,33 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { expect(progress).toContainEqual({ message: "https://claude.ai/code/session_TESTBRIDGE", stage: "remote-control-url" }); }); + // Regression for issue #149. A feature-sized coding turn ran correctly for + // half an hour and was then killed by an ABSOLUTE 30-minute cap and reported + // to the user as "Timed out after 1800000ms waiting for the remote-control + // session to finish" -- the run had never once been idle. Every other bound + // in the system measures silence rather than duration for exactly this + // reason. Here the turn runs many times longer than the idle window while + // narrating throughout, and must survive: no `maxWaitMs` is passed, because + // there is no absolute cap any more. + it("does not kill a turn that runs far longer than the idle window while it is still working", async () => { + const progress: Array<{ message: string; stage: string }> = []; + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_SCRIPT_MODE: "streaming", FAKE_STEP_MS: "30", FAKE_STEPS: "20" }), + settings: {}, + runId: "run-long", + pollIntervalMs: 15, + idleTimeoutMs: 250, // far shorter than the ~600ms the turn takes + onProgress: (message, stage) => progress.push({ message, stage }), + }); + + expect(result.failed).toBe(false); + expect(result.failureDetail).toBeNull(); + expect(result.finalMessage).toBe("DONE"); + // ...and the caller saw the real tool-call trail, not a content-free ticker. + expect(progress).toContainEqual({ message: "running Bash", stage: "agent" }); + }); + it("emits the URL even when the turn never completes (URL known before completion)", async () => { const progress: Array<{ message: string; stage: string }> = []; const result = await runClaudeTurnRemoteControlled("do the thing", { @@ -123,13 +196,118 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { settings: {}, runId: "run-2", pollIntervalMs: 20, - maxWaitMs: 300, + idleTimeoutMs: 300, onProgress: (message, stage) => progress.push({ message, stage }), }); expect(progress).toContainEqual({ message: "https://claude.ai/code/session_TESTBRIDGE", stage: "remote-control-url" }); expect(result.failed).toBe(true); - expect(result.failureDetail).toMatch(/Timed out/); + }); + + // The two ways a wait ends now say which one happened. A single "Timed out + // after 1800000ms" could not distinguish a session that worked and then + // wedged from one whose prompt never landed, and that ambiguity is most of + // why #149 took so long to pin down. + it("reports a session that worked and then went silent as a stall, naming the silence", async () => { + const progress: Array<{ message: string; stage: string }> = []; + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_SCRIPT_MODE: "running", FAKE_ASSISTANT_TEXT: "picked up the issue" }), + settings: {}, + runId: "run-silent", + pollIntervalMs: 20, + idleTimeoutMs: 300, + heartbeatIntervalMs: 100, + onProgress: (message, stage) => progress.push({ message, stage }), + }); + + expect(result.failed).toBe(true); + expect(result.authError).toBe(false); + expect(result.failureDetail).toMatch(/stopped producing output/); + expect(result.failureDetail).toMatch(/no transcript activity for \d+s/); + expect(result.failureDetail).toMatch(/last activity: picked up the issue/); + // The heartbeat reports the actual silence rather than asserting "still + // running" about a session that had already stopped. + expect(progress.some((p) => /quiet for \d+s/.test(p.message))).toBe(true); + }); + + it("reports a session that registered but never began its turn distinctly", async () => { + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_SCRIPT_MODE: "stalled" }), // bridge line only: no assistant text, no tools + settings: {}, + runId: "run-stalled", + pollIntervalMs: 20, + idleTimeoutMs: 300, + }); + + expect(result.failed).toBe(true); + expect(result.failureDetail).toMatch(/never started its turn/); + expect(result.failureDetail).toMatch(/prompt may not have been submitted/); + }); + + it("gives up on a session that never registers, without waiting for the idle window", async () => { + const started = Date.now(); + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + // Child stays resident (no FAKE_SCRIPT_EXIT) but never registers -- a CLI + // wedged on an unexpected first-run prompt, which never exits on its own. + env: env({ FAKE_SCRIPT_MODE: "nofile", FAKE_NO_SESSION: "1", FAKE_SCRIPT_OUTPUT: "Is this a project you trust?" }), + settings: {}, + runId: "run-noreg", + pollIntervalMs: 20, + startupTimeoutMs: 200, + idleTimeoutMs: 60_000, // must not be what ends this wait + }); + + expect(result.failed).toBe(true); + expect(result.failureDetail).toMatch(/never registered with the CLI/); + expect(Date.now() - started).toBeLessThan(5_000); + }); + + it("still honours an explicit absolute cap when a caller sets one", async () => { + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_SCRIPT_MODE: "streaming", FAKE_STEP_MS: "20", FAKE_STEPS: "1000" }), + settings: {}, + runId: "run-capped", + pollIntervalMs: 15, + idleTimeoutMs: 60_000, // never idle; only the absolute cap can end this + maxWaitMs: 300, + }); + + expect(result.failed).toBe(true); + expect(result.failureDetail).toMatch(/Timed out after 300ms/); + }); + + // `agents --json --all` lists ended sessions too, so on a second turn in the + // same pod the previous turn's session is still there at the same cwd -- and + // its transcript already carries a `turn_duration`. Matching it would hand + // the previous turn's answer back as this turn's, instantly and silently. + it("ignores a session that already existed before this run started", async () => { + const priorSid = "99999999-8888-7777-6666-555555555555"; + const projectDir = join(homeDir, ".claude", "projects", cwd.replace(/[/.]/g, "-")); + await mkdir(projectDir, { recursive: true }); + await writeFile( + join(projectDir, `${priorSid}.jsonl`), + [ + JSON.stringify({ type: "summary", bridgeSessionId: "cse_STALEBRIDGE", lastSequenceNum: 0 }), + JSON.stringify({ type: "assistant", message: { role: "assistant", content: [{ type: "text", text: "STALE" }] } }), + JSON.stringify({ type: "system", subtype: "turn_duration", durationMs: 5 }), + ].join("\n") + "\n", + ); + + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_PRIOR_SID: priorSid, FAKE_ASSISTANT_TEXT: "FRESH" }), + settings: {}, + runId: "run-second-turn", + pollIntervalMs: 20, + idleTimeoutMs: 5_000, + }); + + expect(result.finalMessage).toBe("FRESH"); + expect(result.sessionId).toBe(FAKE_SID); }); it("reports failure (and classifies auth errors) when agents --json marks the session failed", async () => { diff --git a/apps/claude-code-swe-agent/src/claude-runner.ts b/apps/claude-code-swe-agent/src/claude-runner.ts index 230f5e6..29c9b80 100644 --- a/apps/claude-code-swe-agent/src/claude-runner.ts +++ b/apps/claude-code-swe-agent/src/claude-runner.ts @@ -40,7 +40,25 @@ export interface RemoteControlRunOptions extends ClaudeRunOptions { runId: string; /** Cadence for polling `claude agents --json`. Defaults to {@link REMOTE_CONTROL_POLL_INTERVAL_MS}; injectable for tests. */ pollIntervalMs?: number; - /** Max time to wait for the background session to conclude before reporting a timeout failure. Defaults to {@link REMOTE_CONTROL_MAX_WAIT_MS}; injectable for tests. */ + /** + * How long the session may go with NO new transcript activity before the + * turn is given up on. Bounds SILENCE, not total duration. Defaults to + * {@link REMOTE_CONTROL_IDLE_TIMEOUT_MS}; injectable for tests. + */ + idleTimeoutMs?: number; + /** + * How long to wait for the interactive session to register with the CLI at + * all before giving up on startup. Defaults to + * {@link REMOTE_CONTROL_STARTUP_TIMEOUT_MS}; injectable for tests. + */ + startupTimeoutMs?: number; + /** + * Optional ABSOLUTE wall-clock backstop. Defaults to no bound at all: a real + * coding turn can legitimately run for hours, the idle bound above is what + * ends a stuck one, and the AgentRun Job's `activeDeadlineSeconds` is the + * ceiling (see agent-orchestrator's `config.ts`). Set only when a caller + * genuinely wants to cap total duration. + */ maxWaitMs?: number; } @@ -325,8 +343,41 @@ export function runClaudeTurn(prompt: string, opts: ClaudeRunOptions): Promise` we - * pass), so matching is by kind + cwd, not name. Returns `null` (never - * throws) until the session appears / on any unexpected shape. + * pass), so matching is by kind + cwd, not name. + * + * `exclude` holds the interactive session ids that already existed at this cwd + * BEFORE this run spawned its own, and they are skipped. `--all` lists ended + * sessions too, so on a second turn in the same pod the previous turn's + * session is still listed at the same cwd and would otherwise be matched + * first -- and its transcript already carries a `turn_duration`, which would + * hand the previous turn's answer back as this turn's, instantly and + * silently. + * + * Returns `null` (never throws) until a session appears / on any unexpected + * shape. */ -function findInteractiveSession(raw: string, cwd: string): InteractiveSession | null { +function findInteractiveSession( + raw: string, + cwd: string, + exclude: ReadonlySet = new Set(), +): InteractiveSession | null { let parsed: unknown; try { parsed = JSON.parse(raw); @@ -478,7 +543,7 @@ function findInteractiveSession(raw: string, cwd: string): InteractiveSession | const entryCwd = typeof rec.cwd === "string" ? rec.cwd : ""; if (entryCwd !== cwd && !entryCwd.startsWith(`${cwd}/`) && !entryCwd.startsWith(cwd)) continue; const sessionId = typeof rec.sessionId === "string" ? rec.sessionId : ""; - if (!sessionId) continue; + if (!sessionId || exclude.has(sessionId)) continue; const state = String(rec.state ?? "").toLowerCase(); const status = String(rec.status ?? "").toLowerCase(); const TERMINAL_FAILED = ["failed", "error", "errored"]; @@ -488,6 +553,30 @@ function findInteractiveSession(raw: string, cwd: string): InteractiveSession | return null; } +/** + * Every interactive session id `agents --json --all` reports at `cwd`, used to + * snapshot what already existed before this run spawned its own (see + * {@link findInteractiveSession}'s `exclude`). + */ +function listInteractiveSessionIds(raw: string, cwd: string): Set { + const ids = new Set(); + const seen = new Set(); + // Reuse the one matcher so "which sessions count as at this cwd" cannot + // drift between the snapshot and the later lookup. + for (;;) { + const found = findInteractiveSession(raw, cwd, seen); + if (!found) return ids; + ids.add(found.sessionId); + seen.add(found.sessionId); + } +} + +/** A narratable thing the session did, derived from one transcript entry. */ +interface TranscriptEvent { + kind: "agent-text" | "agent"; + message: string; +} + interface TranscriptState { /** The `bridgeSessionId` (e.g. "cse_01YB…") -> feeds `remoteControlUrlFromBridge`. Null until it appears. */ bridgeSessionId: string | null; @@ -495,6 +584,19 @@ interface TranscriptState { finalText: string | null; /** True once a `{type:"system",subtype:"turn_duration"}` entry appears, i.e. the turn finished. */ turnComplete: boolean; + /** + * Count of parsed transcript entries. This is the run's liveness signal: the + * transcript gains an entry for every assistant message, tool call, and tool + * result, so a growing count is proof the session is doing work and a static + * one is the only honest definition of "stuck". + * + * Deliberately NOT measured from the pty output, which a TUI redraws + * continuously whether or not anything is happening -- treating that as + * activity would make the idle bound unable to ever fire. + */ + entryCount: number; + /** Narratable events in transcript order, so newly-appeared ones can be forwarded as progress. */ + events: TranscriptEvent[]; } /** @@ -509,6 +611,8 @@ function parseTranscript(raw: string): TranscriptState { let bridgeSessionId: string | null = null; let finalText: string | null = null; let turnComplete = false; + let entryCount = 0; + const events: TranscriptEvent[] = []; for (const line of raw.split("\n")) { if (!line.trim()) continue; let entry: unknown; @@ -518,27 +622,30 @@ function parseTranscript(raw: string): TranscriptState { continue; } if (typeof entry !== "object" || entry === null) continue; + entryCount += 1; const rec = entry as Record; if (typeof rec.bridgeSessionId === "string" && rec.bridgeSessionId) bridgeSessionId = rec.bridgeSessionId; if (rec.type === "system" && rec.subtype === "turn_duration") turnComplete = true; if (rec.type === "assistant" && typeof rec.message === "object" && rec.message !== null) { const content = (rec.message as Record).content; if (Array.isArray(content)) { - const text = content - .filter( - (b): b is { type: string; text: string } => - typeof b === "object" && - b !== null && - (b as { type?: unknown }).type === "text" && - typeof (b as { text?: unknown }).text === "string", - ) - .map((b) => b.text) - .join(""); + const texts: string[] = []; + for (const block of content) { + if (typeof block !== "object" || block === null) continue; + const b = block as Record; + if (b.type === "text" && typeof b.text === "string" && b.text) { + texts.push(b.text); + events.push({ kind: "agent-text", message: b.text }); + } else if (b.type === "tool_use" && typeof b.name === "string" && b.name) { + events.push({ kind: "agent", message: `running ${b.name}` }); + } + } + const text = texts.join(""); if (text) finalText = text; } } } - return { bridgeSessionId, finalText, turnComplete }; + return { bridgeSessionId, finalText, turnComplete, entryCount, events }; } /** Reads the session's transcript, or null if it doesn't exist yet / can't be read. */ @@ -579,10 +686,15 @@ function claudeProjectDirName(cwd: string): string { * id, then read its JSONL transcript for the URL (`bridgeSessionId`), the * final reply (last assistant text), and completion (`turn_duration`). * 4. Emit the URL via `onProgress` as soon as it's known (near the start), - * and resolve with the same `ClaudeRunResult` shape as `runClaudeTurn` - * once the turn completes / fails / the child exits / `maxWaitMs` elapses. + * narrate each new transcript entry as it appears, and resolve with the + * same `ClaudeRunResult` shape as `runClaudeTurn` once the turn completes + * / fails / the child exits / the session goes silent for `idleTimeoutMs`. * The interactive session stays resident after its turn, so it's killed * on the way out. + * + * The wait is bounded by SILENCE, not by total duration -- see + * {@link REMOTE_CONTROL_IDLE_TIMEOUT_MS} and issue #149, where a healthy + * long-running turn was killed by a 30-minute absolute cap. */ export async function runClaudeTurnRemoteControlled( prompt: string, @@ -591,6 +703,19 @@ export async function runClaudeTurnRemoteControlled( const homeDir = opts.env.HOME ?? ""; seedRemoteControlConfig(homeDir, opts.cwd); + // Snapshot the interactive sessions that already exist at this cwd BEFORE + // spawning ours, so the poll below cannot mistake a previous turn's session + // (still listed by `--all`, transcript already carrying a `turn_duration`) + // for this turn's. A failed probe just yields an empty set -- the same + // behaviour as before this snapshot existed. + const preExisting = await spawnAndCapture(["agents", "--json", "--all"], { + cwd: opts.cwd, + env: opts.env, + signal: opts.signal, + mirrorStderr: false, + }); + const priorSessionIds = preExisting.error ? new Set() : listInteractiveSessionIds(preExisting.stdout, opts.cwd); + const sessionName = `swe-${opts.runId}`; // Prompt/settings/name travel via env vars referenced (quoted) inside a // fixed wrapper string, so the large newline/quote/backtick-heavy prompt is @@ -651,13 +776,46 @@ export async function runClaudeTurnRemoteControlled( }; const pollIntervalMs = opts.pollIntervalMs ?? REMOTE_CONTROL_POLL_INTERVAL_MS; - const maxWaitMs = opts.maxWaitMs ?? REMOTE_CONTROL_MAX_WAIT_MS; + const idleTimeoutMs = opts.idleTimeoutMs ?? REMOTE_CONTROL_IDLE_TIMEOUT_MS; + const startupTimeoutMs = opts.startupTimeoutMs ?? REMOTE_CONTROL_STARTUP_TIMEOUT_MS; const heartbeatMs = opts.heartbeatIntervalMs ?? HEARTBEAT_INTERVAL_MS; - const deadline = Date.now() + maxWaitMs; + // No absolute cap by default -- see `maxWaitMs`'s doc comment and issue #149. + const absoluteDeadline = opts.maxWaitMs === undefined ? Infinity : Date.now() + opts.maxWaitMs; + const startedAt = Date.now(); let lastHeartbeatAt = Date.now(); + // Liveness state. `lastProgressAt` is the clock the idle bound measures, and + // it is reset by any growth in the transcript -- never by the mere passage of + // polls, and never by pty output. + let lastProgressAt = Date.now(); + let sessionFoundAt: number | null = null; + /** Our session's id once discovered -- carried into a give-up result so a stuck run is traceable to a transcript. */ + let discoveredSessionId: string | null = null; + let entryCount = 0; + let narratedEvents = 0; + let sawTurnActivity = false; + let lastActivity: string | null = null; + + /** + * Distinguishes the two shapes of "we gave up waiting", because the previous + * single message ("Timed out after 1800000ms…") could not tell them apart and + * that ambiguity is most of why issue #149 stayed open: a session that + * registered but never began its turn (the prompt never landed) and a session + * that worked and then wedged look identical from the outside. + */ + const idleFailure = (silentForMs: number): ClaudeRunResult => { + const silentSecs = Math.round(silentForMs / 1000); + const ranForSecs = Math.round((Date.now() - startedAt) / 1000); + const detail = sawTurnActivity + ? `The remote-control session stopped producing output: no transcript activity for ${silentSecs}s (turn ran ${ranForSecs}s, ${entryCount} transcript entries` + + `${lastActivity ? `, last activity: ${clip(lastActivity, 200)}` : ""}).` + : `The remote-control session registered but never started its turn: no transcript activity for ${silentSecs}s. ` + + `The prompt may not have been submitted to the session.`; + return { finalMessage: null, failed: true, failureDetail: detail, authError: false, sessionId: discoveredSessionId }; + }; + try { - while (Date.now() < deadline) { + for (;;) { if (opts.signal?.aborted) { return { finalMessage: null, failed: true, failureDetail: "Aborted while waiting for the remote-control session", authError: false, sessionId: null }; } @@ -668,12 +826,35 @@ export async function runClaudeTurnRemoteControlled( signal: opts.signal, mirrorStderr: false, }); - const session = poll.error ? null : findInteractiveSession(poll.stdout, opts.cwd); + const session = poll.error ? null : findInteractiveSession(poll.stdout, opts.cwd, priorSessionIds); if (session) { + discoveredSessionId = session.sessionId; + if (sessionFoundAt === null) { + sessionFoundAt = Date.now(); + // Registration is itself progress; restart the idle clock from here + // so slow startup isn't charged against the session's first tool. + lastProgressAt = sessionFoundAt; + } const raw = await readTranscript(homeDir, opts.cwd, session.sessionId); if (raw) { const st = parseTranscript(raw); + // Any growth in the transcript is proof of life -- this is what makes + // a long, healthy turn immune to the bound that killed issue #149's. + if (st.entryCount > entryCount) { + entryCount = st.entryCount; + lastProgressAt = Date.now(); + } + // Forward only the events that appeared since the last poll, so the + // caller sees the real tool-call trail (as the one-shot `-p` path + // narrates it) rather than a content-free ticker. + for (const ev of st.events.slice(narratedEvents)) { + sawTurnActivity = true; + lastActivity = ev.message; + logProgress(ev.kind, ev.message); + opts.onProgress?.(ev.message, ev.kind); + } + narratedEvents = st.events.length; // Emit the URL the moment it's known (well before completion) so the // caller posts the "watch it here" comment near the start of the run. if (st.bridgeSessionId) reportUrl(remoteControlUrlFromBridge(st.bridgeSessionId)); @@ -719,23 +900,51 @@ export async function runClaudeTurnRemoteControlled( }; } - if (Date.now() - lastHeartbeatAt >= heartbeatMs) { + // Startup bound: no session yet means there is no transcript to measure + // silence against, and a CLI wedged on an unexpected prompt never exits. + if (sessionFoundAt === null && Date.now() - startedAt >= startupTimeoutMs) { + return { + finalMessage: null, + failed: true, + failureDetail: + `The remote-control session never registered with the CLI within ${Math.round(startupTimeoutMs / 1000)}s. ` + + (clip(ptyOutput.trim(), 600) || "The session produced no output."), + authError: looksLikeAuthError(ptyOutput), + sessionId: null, + }; + } + + const silentForMs = Date.now() - lastProgressAt; + if (sessionFoundAt !== null && silentForMs >= idleTimeoutMs) return idleFailure(silentForMs); + + if (Date.now() >= absoluteDeadline) { + return { + finalMessage: null, + failed: true, + failureDetail: `Timed out after ${opts.maxWaitMs}ms waiting for the remote-control session to finish`, + authError: false, + sessionId: null, + }; + } + + // Heartbeat only during genuine silence -- real activity narrates itself + // above, and a heartbeat that fires regardless would say "still running" + // about a session that had already stopped doing anything. + if (silentForMs >= heartbeatMs && Date.now() - lastHeartbeatAt >= heartbeatMs) { lastHeartbeatAt = Date.now(); - const heartbeatMessage = "still running the remote-control session…"; + const secs = Math.round(silentForMs / 1000); + const heartbeatMessage = + sessionFoundAt === null + ? `still starting the remote-control session… (${secs}s)` + : lastActivity + ? `still running the remote-control session — quiet for ${secs}s since: ${clip(lastActivity, 120)}` + : `still running the remote-control session — no activity yet (${secs}s)`; logProgress("agent", heartbeatMessage); opts.onProgress?.(heartbeatMessage, "agent"); } await sleep(pollIntervalMs, opts.signal); } - - return { - finalMessage: null, - failed: true, - failureDetail: `Timed out after ${maxWaitMs}ms waiting for the remote-control session to finish`, - authError: false, - sessionId: null, - }; } finally { kill(); } diff --git a/apps/claude-code-swe-agent/src/config.ts b/apps/claude-code-swe-agent/src/config.ts index a91af3e..88b4275 100644 --- a/apps/claude-code-swe-agent/src/config.ts +++ b/apps/claude-code-swe-agent/src/config.ts @@ -95,6 +95,22 @@ export interface AgentToolConfig { * selects which invocation shape to use. */ remoteControlEnabled: boolean; + /** + * How long a Remote Control session may produce NO transcript activity + * before its turn is given up on (`0`/unset uses claude-runner.ts's + * default). An operational setting rather than an internal detail for the + * same reason `AGENT_REPLY_ACK_TIMEOUT_MS` is (ADR 0033): getting it wrong + * either strands a wedged pod or kills healthy work, and neither should + * need a rebuild to correct. + */ + remoteControlIdleTimeoutMs: number; + /** + * Optional ABSOLUTE cap on a Remote Control turn. Unset means no cap: the + * idle bound above ends a stuck turn and the Job's `activeDeadlineSeconds` + * is the wall-clock ceiling. Setting this reintroduces the behaviour behind + * issue #149, so it exists only as an escape hatch. + */ + remoteControlMaxWaitMs: number; /** * The `~/.claude/.credentials.json` blob this run was launched with, as * injected by agent-orchestrator (`CLAUDE_LOGIN_CREDENTIALS_JSON`, the same @@ -124,6 +140,16 @@ function normalizePem(value: string | undefined): string { return value.includes("\\n") ? value.replace(/\\n/g, "\n") : value; } +/** + * Parses an optional positive-integer env var, yielding `0` ("not set") for + * anything absent, non-numeric, or non-positive -- so a typo falls back to the + * documented default rather than silently disabling a bound. + */ +function positiveInt(value: string | undefined): number { + const parsed = Number(value); + return Number.isFinite(parsed) && parsed > 0 ? Math.floor(parsed) : 0; +} + export function loadToolConfig(env: NodeJS.ProcessEnv = process.env): AgentToolConfig { return { githubToken: env.GITHUB_TOKEN ?? "", @@ -140,6 +166,8 @@ export function loadToolConfig(env: NodeJS.ProcessEnv = process.env): AgentToolC workdir: env.SWE_WORKDIR ?? `/tmp/swe-${randomUUID()}`, homeDir: env.SWE_HOME ?? "/tmp/home", remoteControlEnabled: env.CLAUDE_REMOTE_CONTROL === "true", + remoteControlIdleTimeoutMs: positiveInt(env.CLAUDE_REMOTE_CONTROL_IDLE_TIMEOUT_MS), + remoteControlMaxWaitMs: positiveInt(env.CLAUDE_REMOTE_CONTROL_MAX_WAIT_MS), loginCredentialsJson: env.CLAUDE_LOGIN_CREDENTIALS_JSON ?? "", credentialsWritebackUrl: env.CLAUDE_CREDENTIALS_WRITEBACK_URL ?? "", credentialsWritebackToken: env.CLAUDE_CREDENTIALS_WRITEBACK_TOKEN ?? "", diff --git a/apps/claude-code-swe-agent/src/index.ts b/apps/claude-code-swe-agent/src/index.ts index 6ae73ae..dec94b5 100644 --- a/apps/claude-code-swe-agent/src/index.ts +++ b/apps/claude-code-swe-agent/src/index.ts @@ -195,7 +195,12 @@ async function handler(session: AgentSession): Promise { onProgress: (message: string, stage: string) => void session.progress(clip(message, 500), { stage }), }; const outcome = toolConfig.remoteControlEnabled - ? await runClaudeTurnRemoteControlled(prompt, { ...runOpts, runId: session.runId }) + ? await runClaudeTurnRemoteControlled(prompt, { + ...runOpts, + runId: session.runId, + ...(toolConfig.remoteControlIdleTimeoutMs ? { idleTimeoutMs: toolConfig.remoteControlIdleTimeoutMs } : {}), + ...(toolConfig.remoteControlMaxWaitMs ? { maxWaitMs: toolConfig.remoteControlMaxWaitMs } : {}), + }) : await runClaudeTurn(prompt, runOpts); // Runs on every outcome, including a failed one: the CLI may well have diff --git a/charts/community-components/templates/agent-claude-code-swe.yaml b/charts/community-components/templates/agent-claude-code-swe.yaml index 057dbd0..1e25a4b 100644 --- a/charts/community-components/templates/agent-claude-code-swe.yaml +++ b/charts/community-components/templates/agent-claude-code-swe.yaml @@ -185,6 +185,14 @@ spec: # `runClaudeTurnRemoteControlled` (claude-runner.ts). - name: CLAUDE_REMOTE_CONTROL value: "true" + {{- if .Values.claudeCodeSweAgent.remoteControl.idleTimeoutMs }} + - name: CLAUDE_REMOTE_CONTROL_IDLE_TIMEOUT_MS + value: {{ .Values.claudeCodeSweAgent.remoteControl.idleTimeoutMs | quote }} + {{- end }} + {{- if .Values.claudeCodeSweAgent.remoteControl.maxWaitMs }} + - name: CLAUDE_REMOTE_CONTROL_MAX_WAIT_MS + value: {{ .Values.claudeCodeSweAgent.remoteControl.maxWaitMs | quote }} + {{- end }} {{- end }} {{- end }} {{- end }} diff --git a/charts/community-components/values-ci-all.yaml b/charts/community-components/values-ci-all.yaml index d715cb0..69bd24d 100644 --- a/charts/community-components/values-ci-all.yaml +++ b/charts/community-components/values-ci-all.yaml @@ -65,6 +65,10 @@ claudeCodeSweAgent: - claude-remote remoteControl: enabled: true + # Set here purely so validate-crds renders these two template branches; + # the values are the production defaults spelled out (see values.yaml). + idleTimeoutMs: "1200000" + maxWaitMs: "" # E2E-only test double. Validated here anyway: it is a real chart template that # renders a real Agent CR, and the acknowledgement flag below is exactly the diff --git a/charts/community-components/values.yaml b/charts/community-components/values.yaml index 517717d..cf52486 100644 --- a/charts/community-components/values.yaml +++ b/charts/community-components/values.yaml @@ -270,6 +270,18 @@ claudeCodeSweAgent: # baked into this catalog entry. credentialsSecretName: "" credentialsSecretKey: "" + # How long a Remote Control session may produce NO transcript activity + # (no assistant message, no tool call, no tool result) before its turn is + # declared stuck. This bounds SILENCE, not total duration -- a turn that + # keeps working is never cut off, however long it takes. Empty uses + # claude-runner.ts's default (20 minutes). + idleTimeoutMs: "" + # Escape hatch only: an ABSOLUTE cap on a turn's total duration. Empty + # (the default) means no cap -- the idle bound above ends a stuck turn and + # the AgentRun Job's activeDeadlineSeconds is the wall-clock ceiling. + # Setting this reintroduces the behaviour behind issue #149, where a + # healthy half-hour coding turn was killed and reported as a timeout. + maxWaitMs: "" # github: runs a single allowlisted gh CLI command (issue/pr/repo/release/ # search/workflow/run), preferably authenticated as the calling user's own From 236c31d01b084bbf7826d041ff13a4bf295d2521 Mon Sep 17 00:00:00 2001 From: Austin Kurpuis Date: Mon, 27 Jul 2026 20:42:53 -0700 Subject: [PATCH 2/2] Let the session say whether it is working, instead of inferring it from silence The idle bound in the previous commit could not tell a wedged session from a slow one: a single long tool call writes `tool_use` when it STARTS and `tool_result` when it FINISHES and nothing in between, so a 40-minute test suite is 40 minutes of a completely static transcript belonging to a session that is plainly alive. 20 minutes was a guess sitting between those two cases. `claude agents --json` already answers the question directly. Confirmed against a real CLI (v2.1.220), live and by reading the bundle, rather than assumed -- the listing builds each interactive entry with ...d.status && {status: EMm(d.status)}, ...d.status === "waiting" && d.waitingFor && {waitingFor: d.waitingFor} where EMm is total: EMm(e) = e === "idle" ? "idle" : e === "waiting" ? "waiting" : "busy" so the field is exactly those three values or ABSENT, and an unrecognized internal status degrades to "busy" -- the safe direction. The statuses are computed as: `waiting` when a prompt or dialog is up (with `waitingFor` naming it: "input needed", "dialog open", "sandbox request", "worker request", or the dialog's own label), `busy` when `isLoading || delegatedActive`, `idle` otherwise. Sampled live every 3s across a 36s Bash call, the session reported `busy` for all twelve samples. So each state now gets the bound its evidence deserves: busy -> none. It is working; duration is not our business. The Job's activeDeadlineSeconds is the ceiling, as it already claims. waiting -> 5 min. Nothing in a headless Job will ever answer the prompt, so this could be zero; it is not, because the run's own first comment says "take over the session here" and a human who does needs a window. Bounded anyway -- an unattended run at 3am must not hold a pod until the Job deadline waiting for someone who is asleep. idle -> 90s. Not silence to wait out: the session is saying it is not working. The grace exists only because a session is briefly idle between registering and picking up its prompt, and any `busy` sample resets it, so a blip costs nothing. absent -> the previous commit's 20-minute transcript-silence bound, now demoted to the fallback it should always have been. Giving up now names which signal ended the wait ("reported itself idle for Ns" vs "produced no transcript activity for Ns (status not reported)"), so a future report says whether the session was asked or inferred about. Splitting the clocks matters more than it looks. `busy` resets the stall clock on every poll, so a heartbeat sharing that clock would go silent for exactly the case that most needs narrating -- and the orchestrator ends a turn after 10 minutes without an up-message, which would have killed the long turn from the other side while this code sat happily waiting. The heartbeat now runs off "when did we last say anything", and reports the status it is reporting about. New coverage: a `busy` session with a permanently static transcript survives both silence bounds set to 50ms (only an explicit absolute cap ends the test) and keeps heartbeating `[busy]` throughout; a `waiting` session fails fast naming what it is blocked on; a listing with no status falls back to silence and says so. The long-turn regression test from the previous commit now pins both silence bounds, since leaving the status grace at its default let it pass without testing anything. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NyKcQdvju68R3edo4GXD3T --- .../src/claude-runner-remote-control.test.ts | 97 +++++++- .../src/claude-runner.ts | 216 +++++++++++++++--- apps/claude-code-swe-agent/src/config.ts | 16 ++ apps/claude-code-swe-agent/src/index.ts | 2 + .../templates/agent-claude-code-swe.yaml | 8 + .../community-components/values-ci-all.yaml | 2 + charts/community-components/values.yaml | 21 +- 7 files changed, 319 insertions(+), 43 deletions(-) diff --git a/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts b/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts index 0da1e23..0e02af7 100644 --- a/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts +++ b/apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts @@ -102,7 +102,11 @@ if (args[0] === "agents") { } const started = fs.existsSync(path.join(process.env.HOME, "session-started")); if (started && process.env.FAKE_NO_SESSION !== "1") { - const s = { kind: "interactive", cwd: process.env.FAKE_SESSION_CWD, sessionId: process.env.FAKE_SID, id: "short1", status: "idle" }; + const s = { kind: "interactive", cwd: process.env.FAKE_SESSION_CWD, sessionId: process.env.FAKE_SID, id: "short1" }; + // Mirrors the real listing: status is emitted conditionally (so it can be + // absent), and waitingFor only ever accompanies status "waiting". + if (process.env.FAKE_NO_STATUS !== "1") s.status = process.env.FAKE_SESSION_STATUS || "idle"; + if (s.status === "waiting" && process.env.FAKE_WAITING_FOR) s.waitingFor = process.env.FAKE_WAITING_FOR; if (process.env.FAKE_SESSION_STATE) s.state = process.env.FAKE_SESSION_STATE; list.push(s); } @@ -177,7 +181,12 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { settings: {}, runId: "run-long", pollIntervalMs: 15, - idleTimeoutMs: 250, // far shorter than the ~600ms the turn takes + // BOTH silence bounds far shorter than the ~600ms the turn takes, and + // the fake reports "idle" throughout -- so the only thing keeping this + // turn alive is its transcript growing. Leaving the status bound at its + // 90s default would let this pass without testing anything. + idleTimeoutMs: 250, + idleStatusGraceMs: 250, onProgress: (message, stage) => progress.push({ message, stage }), }); @@ -196,7 +205,7 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { settings: {}, runId: "run-2", pollIntervalMs: 20, - idleTimeoutMs: 300, + idleStatusGraceMs: 300, onProgress: (message, stage) => progress.push({ message, stage }), }); @@ -216,15 +225,16 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { settings: {}, runId: "run-silent", pollIntervalMs: 20, - idleTimeoutMs: 300, + idleStatusGraceMs: 300, heartbeatIntervalMs: 100, onProgress: (message, stage) => progress.push({ message, stage }), }); expect(result.failed).toBe(true); expect(result.authError).toBe(false); - expect(result.failureDetail).toMatch(/stopped producing output/); - expect(result.failureDetail).toMatch(/no transcript activity for \d+s/); + expect(result.failureDetail).toMatch(/stopped working/); + // The session's own word for it, not our inference from silence. + expect(result.failureDetail).toMatch(/reported itself idle for \d+s/); expect(result.failureDetail).toMatch(/last activity: picked up the issue/); // The heartbeat reports the actual silence rather than asserting "still // running" about a session that had already stopped. @@ -238,7 +248,7 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { settings: {}, runId: "run-stalled", pollIntervalMs: 20, - idleTimeoutMs: 300, + idleStatusGraceMs: 300, }); expect(result.failed).toBe(true); @@ -246,6 +256,79 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => { expect(result.failureDetail).toMatch(/prompt may not have been submitted/); }); + // The status signal, which is strictly better evidence than silence: a + // single long tool call writes `tool_use` when it STARTS and `tool_result` + // when it FINISHES and nothing in between, so a long test suite is a + // completely static transcript belonging to a session that is plainly alive. + // Confirmed against a real CLI (v2.1.220): this session reported "busy" for + // every 3s sample across a 36s Bash call. + it("never gives up on a session reporting busy, however long its transcript stays static", async () => { + const progress: Array<{ message: string; stage: string }> = []; + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + // Bridge line only: after registering, the transcript never grows again. + env: env({ FAKE_SCRIPT_MODE: "stalled", FAKE_SESSION_STATUS: "busy" }), + settings: {}, + runId: "run-busy", + pollIntervalMs: 15, + // Both silence-based bounds are set absurdly short. Neither may fire. + idleTimeoutMs: 50, + idleStatusGraceMs: 50, + heartbeatIntervalMs: 80, + maxWaitMs: 600, // the only thing allowed to end this test + onProgress: (message, stage) => progress.push({ message, stage }), + }); + + expect(result.failureDetail).toMatch(/Timed out after 600ms/); + expect(result.failureDetail).not.toMatch(/stopped working|never started/); + + // And it kept narrating throughout. This is load-bearing, not cosmetic: + // `busy` resets the stall clock on every poll, so if the heartbeat shared + // that clock it would go silent for exactly the case that most needs + // narrating -- and the orchestrator ends a turn after 10 minutes without + // an up-message, killing the long turn from the other side. + const beats = progress.filter((p) => /still running/.test(p.message)); + expect(beats.length).toBeGreaterThanOrEqual(2); + expect(beats[0].message).toMatch(/\[busy\]/); + }); + + it("gives up quickly on a session blocked waiting for input, naming what it is blocked on", async () => { + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_SCRIPT_MODE: "running", FAKE_SESSION_STATUS: "waiting", FAKE_WAITING_FOR: "input needed" }), + settings: {}, + runId: "run-waiting", + pollIntervalMs: 20, + waitingTimeoutMs: 200, + idleTimeoutMs: 60_000, + idleStatusGraceMs: 60_000, + }); + + expect(result.failed).toBe(true); + expect(result.failureDetail).toMatch(/blocked waiting for input \(input needed\)/); + expect(result.failureDetail).toMatch(/Take over the session/); + }); + + // `status` is emitted conditionally by the real listing, so it can simply be + // absent -- an older CLI, or a session that has not reported one yet. Then + // transcript silence is the only signal there is, and the message must not + // claim the session said anything about itself. + it("falls back to transcript silence when the listing reports no status", async () => { + const result = await runClaudeTurnRemoteControlled("do the thing", { + cwd, + env: env({ FAKE_SCRIPT_MODE: "running", FAKE_NO_STATUS: "1" }), + settings: {}, + runId: "run-nostatus", + pollIntervalMs: 20, + idleTimeoutMs: 300, + idleStatusGraceMs: 60_000, // must not be what ends this wait + }); + + expect(result.failed).toBe(true); + expect(result.failureDetail).toMatch(/status not reported/); + expect(result.failureDetail).not.toMatch(/reported itself idle/); + }); + it("gives up on a session that never registers, without waiting for the idle window", async () => { const started = Date.now(); const result = await runClaudeTurnRemoteControlled("do the thing", { diff --git a/apps/claude-code-swe-agent/src/claude-runner.ts b/apps/claude-code-swe-agent/src/claude-runner.ts index 29c9b80..7f27492 100644 --- a/apps/claude-code-swe-agent/src/claude-runner.ts +++ b/apps/claude-code-swe-agent/src/claude-runner.ts @@ -41,11 +41,16 @@ export interface RemoteControlRunOptions extends ClaudeRunOptions { /** Cadence for polling `claude agents --json`. Defaults to {@link REMOTE_CONTROL_POLL_INTERVAL_MS}; injectable for tests. */ pollIntervalMs?: number; /** - * How long the session may go with NO new transcript activity before the - * turn is given up on. Bounds SILENCE, not total duration. Defaults to + * Fallback bound, used only while the session reports no `status`: how long + * it may go with NO new transcript activity before the turn is given up on. + * Bounds SILENCE, not total duration. Defaults to * {@link REMOTE_CONTROL_IDLE_TIMEOUT_MS}; injectable for tests. */ idleTimeoutMs?: number; + /** Bound on a session reporting `status: "idle"`. Defaults to {@link REMOTE_CONTROL_IDLE_STATUS_GRACE_MS}. */ + idleStatusGraceMs?: number; + /** Bound on a session reporting `status: "waiting"`. Defaults to {@link REMOTE_CONTROL_WAITING_TIMEOUT_MS}. */ + waitingTimeoutMs?: number; /** * How long to wait for the interactive session to register with the CLI at * all before giving up on startup. Defaults to @@ -344,8 +349,11 @@ export function runClaudeTurn(prompt: string, opts: ClaudeRunOptions): Promise { }); } +/** + * What `agents --json` reports a session to be doing. CONFIRMED against a real + * CLI (v2.1.220), both live and by reading the bundle -- the listing builds + * each entry with `...d.status && {status: EMm(d.status)}` and + * `...d.status === "waiting" && d.waitingFor && {waitingFor: d.waitingFor}`, + * where `EMm` is total: + * + * EMm(e) = e === "idle" ? "idle" : e === "waiting" ? "waiting" : "busy" + * + * so the field is exactly these three values or absent (an unrecognized + * internal status becomes `"busy"`, which is the safe direction for us). + * The status itself is computed as: + * + * waiting -> a prompt/dialog is up (see {@link InteractiveSession.waitingFor}) + * busy -> `isLoading || delegatedActive`, i.e. genuinely working + * idle -> neither + */ +type SessionStatus = "idle" | "waiting" | "busy"; + +const SESSION_STATUSES: readonly string[] = ["idle", "waiting", "busy"]; + interface InteractiveSession { /** The long UUID-shaped session id -- names the transcript file `.jsonl`. */ sessionId: string; @@ -498,6 +557,19 @@ interface InteractiveSession { shortId: string | null; /** True when `agents --json` reports this session in a terminal-failed lifecycle state. */ failed: boolean; + /** + * The session's own account of what it is doing, or `null` when the listing + * omits it (it is emitted conditionally, so a session that has not reported + * one yet simply has no field). `null` means "no opinion" and must never be + * read as "not working" -- the caller falls back to the transcript. + */ + status: SessionStatus | null; + /** + * Why the session is blocked, when `status === "waiting"`. Free text from the + * CLI: `"input needed"`, `"dialog open"`, `"sandbox request"`, `"worker + * request"`, or the specific dialog's own label. + */ + waitingFor: string | null; } /** @@ -545,10 +617,21 @@ function findInteractiveSession( const sessionId = typeof rec.sessionId === "string" ? rec.sessionId : ""; if (!sessionId || exclude.has(sessionId)) continue; const state = String(rec.state ?? "").toLowerCase(); - const status = String(rec.status ?? "").toLowerCase(); + const rawStatus = String(rec.status ?? "").toLowerCase(); + // Kept as a defensive path even though the v2.1.220 listing cannot produce + // it (`EMm` maps every internal status into idle/waiting/busy, and no + // `state` key is emitted for an interactive entry) -- the shape is not + // contractual across CLI versions, and mistaking a failed session for a + // running one is the more expensive error. const TERMINAL_FAILED = ["failed", "error", "errored"]; - const failed = TERMINAL_FAILED.includes(state) || TERMINAL_FAILED.includes(status); - return { sessionId, shortId: typeof rec.id === "string" ? rec.id : null, failed }; + const failed = TERMINAL_FAILED.includes(state) || TERMINAL_FAILED.includes(rawStatus); + return { + sessionId, + shortId: typeof rec.id === "string" ? rec.id : null, + failed, + status: SESSION_STATUSES.includes(rawStatus) ? (rawStatus as SessionStatus) : null, + waitingFor: typeof rec.waitingFor === "string" && rec.waitingFor ? rec.waitingFor : null, + }; } return null; } @@ -692,9 +775,11 @@ function claudeProjectDirName(cwd: string): string { * The interactive session stays resident after its turn, so it's killed * on the way out. * - * The wait is bounded by SILENCE, not by total duration -- see - * {@link REMOTE_CONTROL_IDLE_TIMEOUT_MS} and issue #149, where a healthy - * long-running turn was killed by a 30-minute absolute cap. + * The wait is never bounded by total duration -- see issue #149, where a + * healthy long-running turn was killed by a 30-minute absolute cap. What ends + * it is the session's own reported {@link SessionStatus} (`busy` runs as long + * as it likes; `idle` and `waiting` each have their own short bound), falling + * back to transcript silence only while no status is reported. */ export async function runClaudeTurnRemoteControlled( prompt: string, @@ -767,27 +852,46 @@ export async function runClaudeTurnRemoteControlled( } }; + /** + * When we last told the caller ANYTHING. Distinct from the stall clock + * below, and load-bearing: the orchestrator ends a turn after + * `agentIdleTimeoutSeconds` without an up-message (10 min), so this + * guarantees we speak often enough to stay inside that whatever the session + * is doing. Collapsing the two clocks would silence us for exactly the case + * that most needs narrating -- a long `busy` tool call, which resets the + * stall clock on every poll while emitting no transcript entries at all. + */ + let lastEmitAt = Date.now(); + const emit = (message: string, stage: "agent-text" | "agent" | "remote-control-url"): void => { + lastEmitAt = Date.now(); + logProgress(stage, message); + opts.onProgress?.(message, stage); + }; + let urlReported = false; const reportUrl = (url: string): void => { if (urlReported) return; urlReported = true; - logProgress("remote-control-url", url); - opts.onProgress?.(url, "remote-control-url"); + emit(url, "remote-control-url"); }; const pollIntervalMs = opts.pollIntervalMs ?? REMOTE_CONTROL_POLL_INTERVAL_MS; const idleTimeoutMs = opts.idleTimeoutMs ?? REMOTE_CONTROL_IDLE_TIMEOUT_MS; + const idleStatusGraceMs = opts.idleStatusGraceMs ?? REMOTE_CONTROL_IDLE_STATUS_GRACE_MS; + const waitingTimeoutMs = opts.waitingTimeoutMs ?? REMOTE_CONTROL_WAITING_TIMEOUT_MS; const startupTimeoutMs = opts.startupTimeoutMs ?? REMOTE_CONTROL_STARTUP_TIMEOUT_MS; const heartbeatMs = opts.heartbeatIntervalMs ?? HEARTBEAT_INTERVAL_MS; // No absolute cap by default -- see `maxWaitMs`'s doc comment and issue #149. const absoluteDeadline = opts.maxWaitMs === undefined ? Infinity : Date.now() + opts.maxWaitMs; const startedAt = Date.now(); - let lastHeartbeatAt = Date.now(); - // Liveness state. `lastProgressAt` is the clock the idle bound measures, and - // it is reset by any growth in the transcript -- never by the mere passage of - // polls, and never by pty output. + // Liveness state. `lastProgressAt` is the STALL clock: reset by growth in the + // transcript and by a `busy` status -- never by the mere passage of polls, + // and never by pty output (a TUI redraws continuously whether or not + // anything is happening, so counting it would stop the bound ever firing). let lastProgressAt = Date.now(); + /** Last growth in the transcript specifically -- for wording, not for deciding. */ + let lastTranscriptAt = Date.now(); let sessionFoundAt: number | null = null; /** Our session's id once discovered -- carried into a give-up result so a stuck run is traceable to a transcript. */ let discoveredSessionId: string | null = null; @@ -795,6 +899,12 @@ export async function runClaudeTurnRemoteControlled( let narratedEvents = 0; let sawTurnActivity = false; let lastActivity: string | null = null; + /** The session's last reported status, so a give-up message can say which signal ended the wait. */ + let lastStatus: SessionStatus | null = null; + /** When the session first reported `waiting` without having moved since. */ + let waitingSinceAt: number | null = null; + /** The reason accompanying the most recent `waiting` status, for the give-up message. */ + let lastWaitingFor: string | null = null; /** * Distinguishes the two shapes of "we gave up waiting", because the previous @@ -806,10 +916,13 @@ export async function runClaudeTurnRemoteControlled( const idleFailure = (silentForMs: number): ClaudeRunResult => { const silentSecs = Math.round(silentForMs / 1000); const ranForSecs = Math.round((Date.now() - startedAt) / 1000); + // `idle` is the session's own word for it; no status at all means we are + // inferring from silence and should say so rather than overclaim. + const basis = lastStatus === "idle" ? `reported itself idle for ${silentSecs}s` : `produced no transcript activity for ${silentSecs}s (status not reported)`; const detail = sawTurnActivity - ? `The remote-control session stopped producing output: no transcript activity for ${silentSecs}s (turn ran ${ranForSecs}s, ${entryCount} transcript entries` + + ? `The remote-control session stopped working: ${basis} (turn ran ${ranForSecs}s, ${entryCount} transcript entries` + `${lastActivity ? `, last activity: ${clip(lastActivity, 200)}` : ""}).` - : `The remote-control session registered but never started its turn: no transcript activity for ${silentSecs}s. ` + + : `The remote-control session registered but never started its turn: ${basis}. ` + `The prompt may not have been submitted to the session.`; return { finalMessage: null, failed: true, failureDetail: detail, authError: false, sessionId: discoveredSessionId }; }; @@ -830,6 +943,19 @@ export async function runClaudeTurnRemoteControlled( if (session) { discoveredSessionId = session.sessionId; + lastStatus = session.status; + // `busy` is the session itself saying it is working, which is stronger + // evidence than anything the transcript can offer: a single long tool + // call writes `tool_use` when it STARTS and `tool_result` when it + // FINISHES and nothing in between, so a 40-minute test suite is 40 + // minutes of silence from a session that is plainly alive. Counting it + // as progress is what lets real work run as long as it needs. + if (session.status === "busy") lastProgressAt = Date.now(); + // `waiting` means a prompt or dialog is up. Its clock is separate: it + // is not silence to be waited out, it is a state only a human can + // leave, so it must not be reset by transcript growth. + waitingSinceAt = session.status === "waiting" ? (waitingSinceAt ?? Date.now()) : null; + if (session.status === "waiting") lastWaitingFor = session.waitingFor ?? lastWaitingFor; if (sessionFoundAt === null) { sessionFoundAt = Date.now(); // Registration is itself progress; restart the idle clock from here @@ -844,6 +970,7 @@ export async function runClaudeTurnRemoteControlled( if (st.entryCount > entryCount) { entryCount = st.entryCount; lastProgressAt = Date.now(); + lastTranscriptAt = lastProgressAt; } // Forward only the events that appeared since the last poll, so the // caller sees the real tool-call trail (as the one-shot `-p` path @@ -851,8 +978,7 @@ export async function runClaudeTurnRemoteControlled( for (const ev of st.events.slice(narratedEvents)) { sawTurnActivity = true; lastActivity = ev.message; - logProgress(ev.kind, ev.message); - opts.onProgress?.(ev.message, ev.kind); + emit(ev.message, ev.kind); } narratedEvents = st.events.length; // Emit the URL the moment it's known (well before completion) so the @@ -915,7 +1041,32 @@ export async function runClaudeTurnRemoteControlled( } const silentForMs = Date.now() - lastProgressAt; - if (sessionFoundAt !== null && silentForMs >= idleTimeoutMs) return idleFailure(silentForMs); + if (sessionFoundAt !== null) { + if (waitingSinceAt !== null && Date.now() - waitingSinceAt >= waitingTimeoutMs) { + const blockedSecs = Math.round((Date.now() - waitingSinceAt) / 1000); + return { + finalMessage: null, + failed: true, + failureDetail: + `The remote-control session is blocked waiting for input (${lastWaitingFor ?? "reason not reported"}) ` + + `and nothing answered it for ${blockedSecs}s. Take over the session at its claude.ai URL to answer, or re-trigger with more detail in the request.`, + authError: false, + sessionId: discoveredSessionId, + }; + } + // Which bound applies is the session's own status, not a guess: + // busy -> none; it is working, and duration is not our business + // waiting -> none here; handled above, on its own clock + // idle -> a short grace, because "not working" is a real answer + // absent -> the transcript-silence fallback, our only signal + const bound = + lastStatus === "busy" || lastStatus === "waiting" + ? Infinity + : lastStatus === "idle" + ? idleStatusGraceMs + : idleTimeoutMs; + if (silentForMs >= bound) return idleFailure(silentForMs); + } if (Date.now() >= absoluteDeadline) { return { @@ -927,20 +1078,21 @@ export async function runClaudeTurnRemoteControlled( }; } - // Heartbeat only during genuine silence -- real activity narrates itself - // above, and a heartbeat that fires regardless would say "still running" - // about a session that had already stopped doing anything. - if (silentForMs >= heartbeatMs && Date.now() - lastHeartbeatAt >= heartbeatMs) { - lastHeartbeatAt = Date.now(); - const secs = Math.round(silentForMs / 1000); + // Fires only when nothing else has been said for a full interval -- real + // activity narrates itself above and pushes this out. What it reports is + // the session's actual state, not a blanket "still running": the old + // ticker said that about sessions that had already stopped. + if (Date.now() - lastEmitAt >= heartbeatMs) { + const secs = Math.round((Date.now() - lastTranscriptAt) / 1000); const heartbeatMessage = sessionFoundAt === null ? `still starting the remote-control session… (${secs}s)` - : lastActivity - ? `still running the remote-control session — quiet for ${secs}s since: ${clip(lastActivity, 120)}` - : `still running the remote-control session — no activity yet (${secs}s)`; - logProgress("agent", heartbeatMessage); - opts.onProgress?.(heartbeatMessage, "agent"); + : lastStatus === "waiting" + ? `remote-control session is waiting for input (${lastWaitingFor ?? "reason not reported"})` + : lastActivity + ? `still running the remote-control session [${lastStatus ?? "status unknown"}] — quiet for ${secs}s since: ${clip(lastActivity, 120)}` + : `still running the remote-control session [${lastStatus ?? "status unknown"}] — no activity yet (${secs}s)`; + emit(heartbeatMessage, "agent"); } await sleep(pollIntervalMs, opts.signal); diff --git a/apps/claude-code-swe-agent/src/config.ts b/apps/claude-code-swe-agent/src/config.ts index 88b4275..0d3537c 100644 --- a/apps/claude-code-swe-agent/src/config.ts +++ b/apps/claude-code-swe-agent/src/config.ts @@ -104,6 +104,20 @@ export interface AgentToolConfig { * need a rebuild to correct. */ remoteControlIdleTimeoutMs: number; + /** + * How long a Remote Control session reporting `status: "idle"` may stay that + * way before its turn is given up on (`0`/unset uses the default). Much + * shorter than the silence bound above, because the session is not being + * quiet -- it is reporting that it is not working. + */ + remoteControlIdleStatusGraceMs: number; + /** + * How long a Remote Control session reporting `status: "waiting"` may stay + * blocked on a prompt before its turn is given up on (`0`/unset uses the + * default). Sized by human reaction time -- it is the window someone has to + * take over the session at its claude.ai URL and answer. + */ + remoteControlWaitingTimeoutMs: number; /** * Optional ABSOLUTE cap on a Remote Control turn. Unset means no cap: the * idle bound above ends a stuck turn and the Job's `activeDeadlineSeconds` @@ -167,6 +181,8 @@ export function loadToolConfig(env: NodeJS.ProcessEnv = process.env): AgentToolC homeDir: env.SWE_HOME ?? "/tmp/home", remoteControlEnabled: env.CLAUDE_REMOTE_CONTROL === "true", remoteControlIdleTimeoutMs: positiveInt(env.CLAUDE_REMOTE_CONTROL_IDLE_TIMEOUT_MS), + remoteControlIdleStatusGraceMs: positiveInt(env.CLAUDE_REMOTE_CONTROL_IDLE_STATUS_GRACE_MS), + remoteControlWaitingTimeoutMs: positiveInt(env.CLAUDE_REMOTE_CONTROL_WAITING_TIMEOUT_MS), remoteControlMaxWaitMs: positiveInt(env.CLAUDE_REMOTE_CONTROL_MAX_WAIT_MS), loginCredentialsJson: env.CLAUDE_LOGIN_CREDENTIALS_JSON ?? "", credentialsWritebackUrl: env.CLAUDE_CREDENTIALS_WRITEBACK_URL ?? "", diff --git a/apps/claude-code-swe-agent/src/index.ts b/apps/claude-code-swe-agent/src/index.ts index dec94b5..cac1cd6 100644 --- a/apps/claude-code-swe-agent/src/index.ts +++ b/apps/claude-code-swe-agent/src/index.ts @@ -199,6 +199,8 @@ async function handler(session: AgentSession): Promise { ...runOpts, runId: session.runId, ...(toolConfig.remoteControlIdleTimeoutMs ? { idleTimeoutMs: toolConfig.remoteControlIdleTimeoutMs } : {}), + ...(toolConfig.remoteControlIdleStatusGraceMs ? { idleStatusGraceMs: toolConfig.remoteControlIdleStatusGraceMs } : {}), + ...(toolConfig.remoteControlWaitingTimeoutMs ? { waitingTimeoutMs: toolConfig.remoteControlWaitingTimeoutMs } : {}), ...(toolConfig.remoteControlMaxWaitMs ? { maxWaitMs: toolConfig.remoteControlMaxWaitMs } : {}), }) : await runClaudeTurn(prompt, runOpts); diff --git a/charts/community-components/templates/agent-claude-code-swe.yaml b/charts/community-components/templates/agent-claude-code-swe.yaml index 1e25a4b..b51affa 100644 --- a/charts/community-components/templates/agent-claude-code-swe.yaml +++ b/charts/community-components/templates/agent-claude-code-swe.yaml @@ -189,6 +189,14 @@ spec: - name: CLAUDE_REMOTE_CONTROL_IDLE_TIMEOUT_MS value: {{ .Values.claudeCodeSweAgent.remoteControl.idleTimeoutMs | quote }} {{- end }} + {{- if .Values.claudeCodeSweAgent.remoteControl.idleStatusGraceMs }} + - name: CLAUDE_REMOTE_CONTROL_IDLE_STATUS_GRACE_MS + value: {{ .Values.claudeCodeSweAgent.remoteControl.idleStatusGraceMs | quote }} + {{- end }} + {{- if .Values.claudeCodeSweAgent.remoteControl.waitingTimeoutMs }} + - name: CLAUDE_REMOTE_CONTROL_WAITING_TIMEOUT_MS + value: {{ .Values.claudeCodeSweAgent.remoteControl.waitingTimeoutMs | quote }} + {{- end }} {{- if .Values.claudeCodeSweAgent.remoteControl.maxWaitMs }} - name: CLAUDE_REMOTE_CONTROL_MAX_WAIT_MS value: {{ .Values.claudeCodeSweAgent.remoteControl.maxWaitMs | quote }} diff --git a/charts/community-components/values-ci-all.yaml b/charts/community-components/values-ci-all.yaml index 69bd24d..e14f3fe 100644 --- a/charts/community-components/values-ci-all.yaml +++ b/charts/community-components/values-ci-all.yaml @@ -68,6 +68,8 @@ claudeCodeSweAgent: # Set here purely so validate-crds renders these two template branches; # the values are the production defaults spelled out (see values.yaml). idleTimeoutMs: "1200000" + idleStatusGraceMs: "90000" + waitingTimeoutMs: "300000" maxWaitMs: "" # E2E-only test double. Validated here anyway: it is a real chart template that diff --git a/charts/community-components/values.yaml b/charts/community-components/values.yaml index cf52486..c6dfd42 100644 --- a/charts/community-components/values.yaml +++ b/charts/community-components/values.yaml @@ -270,12 +270,25 @@ claudeCodeSweAgent: # baked into this catalog entry. credentialsSecretName: "" credentialsSecretKey: "" - # How long a Remote Control session may produce NO transcript activity - # (no assistant message, no tool call, no tool result) before its turn is - # declared stuck. This bounds SILENCE, not total duration -- a turn that - # keeps working is never cut off, however long it takes. Empty uses + # FALLBACK bound, used only while `claude agents --json` reports no status + # for the session: how long it may produce NO transcript activity (no + # assistant message, no tool call, no tool result) before its turn is + # declared stuck. Bounds SILENCE, not total duration. Empty uses # claude-runner.ts's default (20 minutes). idleTimeoutMs: "" + # How long a session REPORTING "idle" may stay that way before its turn is + # declared stuck. Much shorter than idleTimeoutMs above, because it is much + # better evidence: the session is not being quiet, it is saying it is not + # working. (A session that is working reports "busy" and is never cut off, + # however long a single tool call keeps its transcript static.) Empty uses + # claude-runner.ts's default (90 seconds). + idleStatusGraceMs: "" + # How long a session REPORTING "waiting" -- blocked on a prompt or dialog + # only a human can clear -- may stay blocked. Nothing in a headless Job + # will ever answer it, so this is purely the window someone has to take + # over the session at its claude.ai URL (which the run's own first comment + # links). Empty uses claude-runner.ts's default (5 minutes). + waitingTimeoutMs: "" # Escape hatch only: an ABSOLUTE cap on a turn's total duration. Empty # (the default) means no cap -- the idle bound above ends a stuck turn and # the AgentRun Job's activeDeadlineSeconds is the wall-clock ceiling.