From 577f0a687fca6d538fac160f1f965202a73de258 Mon Sep 17 00:00:00 2001 From: N3thunt3r69 Date: Tue, 4 Aug 2026 04:40:21 -0400 Subject: [PATCH 1/5] feat(llm): resume the Claude Code session across a kill-chain phase Every ReAct iteration flattens the growing transcript into a brand-new prompt and spawns a fresh `claude -p` process. Within one task that is a quadratic resend; across a mission it also means each spawn re-pays Claude Code's own CLAUDE.md/skills/MCP bootstrap tax from scratch, since T3MP3ST never told the CLI these calls belong to one session. LocalAgentAdapter now tracks the Claude Code session id from the JSON envelope and passes it back in with --resume on every later call, so the CLI carries the accumulated transcript itself and the repeated part is billed at cache-read pricing instead of resent. The session is scoped to one kill-chain phase: TempestCommand drops it for every operator right after advancePhase(), since local-agent operators are spawned once for the whole mission and would otherwise carry one session across all 7 phases. A stale or expired session id fails fast (confirmed against the real CLI: nonzero exit, no JSON on stdout) and localAgentChat retries once without --resume rather than failing the task over it. Verified against the real CLI: a resumed call recalled prior context correctly and dropped from $0.151 to $0.008 (cache_read_input_tokens absorbing the prior turn instead of cache_creation), and an invalid --resume id triggered the fallback and still succeeded. Ran live against an authorized target: the same session id held across all 4 recon tasks in one phase, then a newly spawned operator's first call in the next phase carried no --resume, confirming both the resume and the reset. Full suite: 764/764, no regressions. --- .../local-agent-path-resolution.test.ts | 56 ++++++++++++++++ .../local-agent-tool-calling.test.ts | 64 +++++++++++++++++++ src/agent/local-agents.ts | 23 +++++-- src/index.ts | 6 ++ src/llm/index.ts | 34 ++++++++-- src/operators/index.ts | 10 +++ 6 files changed, 184 insertions(+), 9 deletions(-) diff --git a/src/__tests__/local-agent-path-resolution.test.ts b/src/__tests__/local-agent-path-resolution.test.ts index 968e5ddb..3ef29893 100644 --- a/src/__tests__/local-agent-path-resolution.test.ts +++ b/src/__tests__/local-agent-path-resolution.test.ts @@ -283,3 +283,59 @@ describe('spawn call-sites use the resolved path (issue #78 — detected-but-uns })).resolves.toBe('--no-tools --model openai-codex/gpt-5|long planning prompt'); }); }); + +/** + * REGRESSION guard (Tier 2 — #139 follow-up, session resume): confirmed empirically against the + * real Claude Code CLI that an unknown/expired --resume id is a hard, fast failure (nonzero exit, + * "No conversation found with session ID: ..." on stderr, no JSON on stdout) — the CLI never falls + * back to a fresh session on its own. localAgentChat does that fallback itself, once. These tests + * drive the REAL function (not mocked) through a fake CLI script so the retry wiring is pinned, + * not just the higher-level LocalAgentAdapter call-shape covered in local-agent-tool-calling.test.ts. + */ +const FAKE_CLI_RESUME_STALE = `#!/bin/sh +cat >/dev/null 2>&1 +case "$*" in + *--resume*) echo "No conversation found with session ID: fake" >&2; exit 1 ;; + *) echo '{"result":"fresh ok","session_id":"new-session-123"}' ;; +esac +`; + +describe('localAgentChat — stale Claude session fallback (Tier 2)', () => { + it('retries WITHOUT --resume when the resumed session is stale, and succeeds', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_RESUME_STALE); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + const out = await localAgentChat('claude', 'ping', { sessionId: 'stale-uuid', timeoutMs: 4000 }); + expect(JSON.parse(out).result).toBe('fresh ok'); + }); + + it('never adds --resume when no sessionId is supplied (no unnecessary retry path)', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_RESUME_STALE); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + const out = await localAgentChat('claude', 'ping', { timeoutMs: 4000 }); + expect(JSON.parse(out).result).toBe('fresh ok'); + }); + + it('a genuine failure with no session in play still rejects (no unnecessary-retry regression)', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', '#!/bin/sh\ncat >/dev/null 2>&1\necho "boom" >&2\nexit 1\n'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + await expect(localAgentChat('claude', 'ping', { timeoutMs: 4000 })).rejects.toThrow(/boom/); + }); + + it('propagates the fallback attempt\'s own error when both the resumed and fresh calls fail', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', '#!/bin/sh\ncat >/dev/null 2>&1\necho "both broken" >&2\nexit 1\n'); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + await expect(localAgentChat('claude', 'ping', { sessionId: 'whatever', timeoutMs: 4000 })).rejects.toThrow(/both broken/); + }); +}); diff --git a/src/__tests__/local-agent-tool-calling.test.ts b/src/__tests__/local-agent-tool-calling.test.ts index fcb26c85..00954228 100644 --- a/src/__tests__/local-agent-tool-calling.test.ts +++ b/src/__tests__/local-agent-tool-calling.test.ts @@ -195,6 +195,70 @@ describe('Claude local-agent usage accounting (#139)', () => { }); }); +describe('Claude local-agent session resume (Tier 2)', () => { + it('the first call on a fresh adapter passes no sessionId', async () => { + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await claudeBackbone().chat([{ role: 'user', content: 'hello' }]); + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + }); + + it('captures session_id from the response and passes it as --resume on the next call', async () => { + const be = claudeBackbone(); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await be.chat([{ role: 'user', content: 'hello' }]); + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-1' })); + await be.chat([{ role: 'user', content: 'again' }]); + + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: 'sess-1' }); + }); + + it('resetLocalAgentSession() drops the tracked session so the next call starts fresh', async () => { + const be = claudeBackbone(); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await be.chat([{ role: 'user', content: 'hello' }]); + + be.resetLocalAgentSession(); + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-2' })); + await be.chat([{ role: 'user', content: 'again' }]); + + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + }); + + it('a separate LLMBackbone instance never inherits another instance\'s session (per-operator isolation)', async () => { + const opA = claudeBackbone(); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'a1', session_id: 'sess-a' })); + await opA.chat([{ role: 'user', content: 'hello' }]); + + const opB = claudeBackbone(); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'b1', session_id: 'sess-b' })); + await opB.chat([{ role: 'user', content: 'hello' }]); + + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + }); + + it('a resumed session with no usable id in the envelope keeps the PRIOR session for the next call', async () => { + const be = claudeBackbone(); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await be.chat([{ role: 'user', content: 'hello' }]); + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'second' })); // no session_id this time + await be.chat([{ role: 'user', content: 'again' }]); + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'third', session_id: 'sess-1' })); + await be.chat([{ role: 'user', content: 'once more' }]); + + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: 'sess-1' }); + }); + + it('non-claude local agents (codex CLI via local-agent provider) never receive a sessionId', async () => { + cli.mockResolvedValueOnce('done'); + await localBackbone().chat([{ role: 'user', content: 'hello' }]); + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + }); +}); + describe('codex backbone surfaces toolCalls (guards the CodexAdapter half of the fix)', () => { it('returns toolCalls when codex emits the contract', async () => { fileRead.mockResolvedValueOnce('```json\n{"tool_calls":[{"name":"nmap_scan","arguments":{"target":"x"}}]}\n```'); diff --git a/src/agent/local-agents.ts b/src/agent/local-agents.ts index b2699b65..3ae774f8 100644 --- a/src/agent/local-agents.ts +++ b/src/agent/local-agents.ts @@ -489,7 +489,7 @@ export function pingLocalAgent(id: string, prompt?: string, timeoutMs?: number): * clean reply, while Hermes takes the prompt as an arg. Provider keys are stripped so each CLI uses its own * login (no API key needed). Throws on non-zero exit / timeout so the LLMBackbone retry/fallback fires. */ -export function localAgentChat(id: string, prompt: string, opts: { model?: string; timeoutMs?: number } = {}): Promise { +export function localAgentChat(id: string, prompt: string, opts: { model?: string; timeoutMs?: number; sessionId?: string } = {}): Promise { const spec = getSpec(id); if (!spec) return Promise.reject(new Error(`unknown local agent: ${id}`)); // child env: provider keys stripped + HOME pinned to the real agent home (see childEnv). @@ -498,6 +498,7 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin const timeoutMs = opts.timeoutMs ?? envTimeoutMs('T3MP3ST_LOCAL_AGENT_TIMEOUT_MS', 600000); let args: string[]; + let claudeArgsNoResume: string[] | null = null; let viaStdin = true; let outFile: string | null = null; let workDir: string | null = null; @@ -505,7 +506,12 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin // json (not text): the envelope carries REAL per-call token usage (input/output tokens // plus prompt-cache creation/read) that LocalAgentAdapter.chat() parses to drive // AgentLoop's token budget check. text mode reports no usage at all. - args = ['-p', '--output-format', 'json', ...(model ? ['--model', model] : [])]; + claudeArgsNoResume = ['-p', '--output-format', 'json', ...(model ? ['--model', model] : [])]; + // --resume continues a prior Claude Code session (LocalAgentAdapter tracks the id) so the CLI + // carries the accumulated transcript itself instead of the caller resending it every turn. + args = opts.sessionId + ? ['-p', '--output-format', 'json', '--resume', opts.sessionId, ...(model ? ['--model', model] : [])] + : claudeArgsNoResume; } else if (id === 'codex') { workDir = mkdtempSync(join(tmpdir(), 't3mp3st-codexllm-')); outFile = join(workDir, 'reply.txt'); @@ -523,8 +529,8 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin const cleanup = () => { if (workDir) { try { rmSync(workDir, { recursive: true, force: true }); } catch { /* noop */ } } }; const resolvedBin = resolveBin(spec.bin) || spec.bin; - return new Promise((resolve, reject) => { - const child = spawnAgent(resolvedBin, args, { env, stdio: [viaStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); + const runOnce = (argv: string[]): Promise => new Promise((resolve, reject) => { + const child = spawnAgent(resolvedBin, argv, { env, stdio: [viaStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); let out = ''; let errOut = ''; let done = false; @@ -545,4 +551,13 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin }); if (viaStdin && child.stdin) { child.stdin.write(prompt); child.stdin.end(); } }); + + const first = runOnce(args); + if (!claudeArgsNoResume || args === claudeArgsNoResume) return first; + // A resumed Claude session can go stale (CLI storage pruned, different session dir, expired). + // Confirmed empirically: an unknown/invalid --resume id is a hard, fast failure (nonzero exit, + // "No conversation found with session ID: ..." on stderr, no JSON on stdout) — the CLI never + // falls back to a fresh session on its own. Do that fallback here, once, rather than failing + // the whole task over a stale id. + return first.catch(() => runOnce(claudeArgsNoResume as string[])); } diff --git a/src/index.ts b/src/index.ts index b9d162dd..5da13f70 100755 --- a/src/index.ts +++ b/src/index.ts @@ -999,6 +999,12 @@ export class TempestCommand extends EventEmitter { this.mission.generateNextPhaseTasks(target.address); } + // Local-agent operators are pre-spawned once and reused for the whole mission (see + // autoSpawnForPhase below), so a resumed CLI session (Claude Code --resume) would + // otherwise span every phase. Drop it here so each phase starts a fresh session instead + // of one unbounded session for the entire kill chain. + for (const op of this.cell.getAllOperators()) op.resetLLMSession(); + // Auto-spawn operators for the new phase const nextPhase = mission.currentPhase; this.autoSpawnForPhase(nextPhase); diff --git a/src/llm/index.ts b/src/llm/index.ts index 001601ce..430f3aa8 100755 --- a/src/llm/index.ts +++ b/src/llm/index.ts @@ -48,6 +48,8 @@ export interface LLMProviderAdapter { chat(messages: LLMMessage[], options?: ChatOptions): Promise; stream?(messages: LLMMessage[], options?: ChatOptions): AsyncGenerator; validateConfig(): { valid: boolean; error?: string }; + /** Drop any resumed backend session so the next chat() starts clean. No-op where not applicable. */ + resetSession?(): void; } export interface ChatOptions { @@ -1315,6 +1317,7 @@ class CodexAdapter implements LLMProviderAdapter { // see and which measured 4-25k tokens on a single trivial call in testing. interface ClaudeJsonEnvelope { result?: string; + session_id?: string; usage?: { input_tokens?: number; output_tokens?: number; @@ -1324,7 +1327,7 @@ interface ClaudeJsonEnvelope { } /** Parse the envelope; invalid usage is omitted so the caller can retain content and estimate. */ -function parseClaudeJsonEnvelope(raw: string): { content: string; usage?: LLMResponse['usage'] } | null { +function parseClaudeJsonEnvelope(raw: string): { content: string; sessionId?: string; usage?: LLMResponse['usage'] } | null { let json: ClaudeJsonEnvelope; try { json = JSON.parse(raw); @@ -1332,16 +1335,17 @@ function parseClaudeJsonEnvelope(raw: string): { content: string; usage?: LLMRes return null; } if (typeof json.result !== 'string') return null; + const sessionId = typeof json.session_id === 'string' && json.session_id ? json.session_id : undefined; const u = json.usage || {}; const tokenValues = [u.input_tokens, u.output_tokens, u.cache_creation_input_tokens, u.cache_read_input_tokens]; if (!tokenValues.some((v) => v !== undefined) || tokenValues.some((v) => v !== undefined && (!Number.isFinite(v) || v < 0))) { - return { content: json.result }; + return { content: json.result, sessionId }; } const promptTokens = (u.input_tokens ?? 0) + (u.cache_creation_input_tokens ?? 0) + (u.cache_read_input_tokens ?? 0); const completionTokens = u.output_tokens ?? 0; - if (promptTokens + completionTokens === 0) return { content: json.result }; - return { content: json.result, usage: { promptTokens, completionTokens, totalTokens: promptTokens + completionTokens } }; + if (promptTokens + completionTokens === 0) return { content: json.result, sessionId }; + return { content: json.result, sessionId, usage: { promptTokens, completionTokens, totalTokens: promptTokens + completionTokens } }; } // Character-based fallback ONLY — used when the real envelope can't be parsed (older CLI, @@ -1360,7 +1364,14 @@ function estimateUsage(promptText: string, completionText: string): LLMResponse[ class LocalAgentAdapter implements LLMProviderAdapter { name = 'local-agent'; private config: LLMConfig; + // Claude Code session to --resume (see local-agents.ts). Lives as long as this adapter does — + // one LocalAgentAdapter is held for an operator's whole life, so this naturally spans every task + // until the caller (TempestCommand, on phase advance) calls resetSession() to start a new one. + private claudeSessionId?: string; constructor(config: LLMConfig) { this.config = config; } + resetSession(): void { + this.claudeSessionId = undefined; + } // Split "agentId[::model]" → { agentId, agentModel }. No separator = just the agent id (CLI default model). private parseAgentSpec(): { agentId: string; agentModel?: string } { const raw = this.config.model || 'codex'; @@ -1392,7 +1403,11 @@ class LocalAgentAdapter implements LLMProviderAdapter { const { agentId, agentModel } = this.parseAgentSpec(); const prompt = this.formatPrompt(messages, options); const timeoutMs = typeof this.config.timeout === 'number' && this.config.timeout > 0 ? this.config.timeout : undefined; - const raw = (await localAgentChat(agentId, prompt, { model: agentModel, timeoutMs })).trim(); + const raw = (await localAgentChat(agentId, prompt, { + model: agentModel, + timeoutMs, + sessionId: agentId === 'claude' ? this.claudeSessionId : undefined, + })).trim(); // Claude requests --output-format json (see local-agents.ts) so this parses to REAL usage. // Anything else (parse failure, or a non-claude agent) falls back to the raw text — claude // additionally gets a character-based usage ESTIMATE so its budget check is never blind again. @@ -1401,6 +1416,10 @@ class LocalAgentAdapter implements LLMProviderAdapter { const usage = agentId === 'claude' ? (parsed?.usage ?? estimateUsage(prompt, parsed?.content ?? raw)) : undefined; + // Carry the session forward so the NEXT call (--resume) picks up where this one left off. + // Empirically stable across --resume (confirmed against the real CLI), but re-capture anyway + // in case a future CLI version rotates it. + if (agentId === 'claude' && parsed?.sessionId) this.claudeSessionId = parsed.sessionId; // Tool-calling over text: if the Arsenal was offered, parse the agent's tool requests so the // ReAct loop EXECUTES them instead of treating this planning turn as the (abstaining) final answer. const toolCalls = options?.tools?.length ? parseTextToolCalls(content) : undefined; @@ -1574,6 +1593,11 @@ export class LLMBackbone extends EventEmitter { return this.config.model; } + /** Drop any resumed local-agent session (e.g. Claude Code's --resume id) so the next chat() starts fresh. */ + resetLocalAgentSession(): void { + this.adapter.resetSession?.(); + } + /** * Validate the configuration */ diff --git a/src/operators/index.ts b/src/operators/index.ts index 32abc634..a66640af 100755 --- a/src/operators/index.ts +++ b/src/operators/index.ts @@ -413,6 +413,16 @@ export class OperatorAgent extends EventEmitter { this.agentLoop = agentLoop; } + /** + * Drop any resumed local-agent session (e.g. Claude Code's --resume id). Called at kill-chain + * phase boundaries: local-agent operators are pre-spawned once and reused for the whole + * mission (auto-spawn-per-phase is disabled for local-agent), so nothing else would naturally + * end a resumed session between phases. + */ + resetLLMSession(): void { + this.llm?.resetLocalAgentSession(); + } + /** Attach the shared pack board so this operator sees the swarm's live lead-board (Phase-2). */ attachBoard(board: PackBoard): void { this.board = board; From 66373b1256586b40b25705f63642e81367378123 Mon Sep 17 00:00:00 2001 From: N3thunt3r69 Date: Thu, 13 Aug 2026 10:23:28 -0400 Subject: [PATCH 2/5] fix(llm): send only the delta on a resumed Claude Code call jmagly's PR #149 review caught it: LocalAgentAdapter.chat() built its prompt from the full messages array unconditionally, so a resumed call sent the whole growing transcript again on top of a session that already had it. The resend the feature exists to remove was still happening, and the resumed session's own history now grew on top of it too. Existing tests only asserted that sessionId reached the CLI, never what was actually in the prompt, so this passed green. LocalAgentAdapter now tracks the exact messages array object and how many of its entries were already sent. A later call against the SAME array (AgentLoop growing one task's transcript via push) sends only the new tail. A DIFFERENT array (a new task) still sends everything, since the resumed session has never seen that task's content even though the CLI session itself carries over. The stale-session fallback in localAgentChat now takes a separate fallbackPrompt: a fresh session has no history, so its retry gets the full transcript, never the delta sized for the failed resumed attempt. Verified against the real CLI with a synthetic multi-round exchange (~750 tokens of padding added between each call): promptTokens went 19127 -> 19625 -> 20135 -> 20645, a flat ~510 tokens per round regardless of the accumulating transcript, instead of resending or compounding. Added the regression coverage the review asked for: a same-array resumed call excludes the prior turn's content while retaining the tool contract, a new-array call still gets the full prompt, and the fallback retry receives fallbackPrompt rather than the delta, checked both at the LocalAgentAdapter call-shape level and against a real spawned CLI script. Full suite: 769/769, no regressions. --- .../local-agent-path-resolution.test.ts | 38 ++++++++++ .../local-agent-tool-calling.test.ts | 69 +++++++++++++++++++ src/agent/local-agents.ts | 13 ++-- src/llm/index.ts | 27 +++++++- 4 files changed, 138 insertions(+), 9 deletions(-) diff --git a/src/__tests__/local-agent-path-resolution.test.ts b/src/__tests__/local-agent-path-resolution.test.ts index 3ef29893..9692c6c1 100644 --- a/src/__tests__/local-agent-path-resolution.test.ts +++ b/src/__tests__/local-agent-path-resolution.test.ts @@ -300,6 +300,16 @@ case "$*" in esac `; +// Echoes exactly what it received on stdin back to stdout on the fresh (non-resume) path — lets a +// test prove WHICH prompt string a fresh-session retry actually received, not just that it succeeded. +const FAKE_CLI_ECHO_ON_FRESH = `#!/bin/sh +prompt=$(cat) +case "$*" in + *--resume*) echo "No conversation found with session ID: fake" >&2; exit 1 ;; + *) printf '%s' "$prompt" ;; +esac +`; + describe('localAgentChat — stale Claude session fallback (Tier 2)', () => { it('retries WITHOUT --resume when the resumed session is stale, and succeeds', async () => { const home = scratch(); @@ -338,4 +348,32 @@ describe('localAgentChat — stale Claude session fallback (Tier 2)', () => { await expect(localAgentChat('claude', 'ping', { sessionId: 'whatever', timeoutMs: 4000 })).rejects.toThrow(/both broken/); }); + + // PR #149 review (jmagly): the fresh-session retry has no history at all, so it must receive the + // FULL transcript (fallbackPrompt) — never the delta sized for the (failed) resumed attempt. + it('the fresh-session retry receives fallbackPrompt, not the delta prompt of the failed resumed attempt', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_ECHO_ON_FRESH); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + const out = await localAgentChat('claude', 'DELTA-ONLY-CONTENT', { + sessionId: 'stale-uuid', + fallbackPrompt: 'FULL-TRANSCRIPT-CONTENT', + timeoutMs: 4000, + }); + + expect(out).toBe('FULL-TRANSCRIPT-CONTENT'); + expect(out).not.toContain('DELTA-ONLY-CONTENT'); + }); + + it('falls back to the (delta) prompt itself when no fallbackPrompt is supplied', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_ECHO_ON_FRESH); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + const out = await localAgentChat('claude', 'ONLY-PROMPT-CONTENT', { sessionId: 'stale-uuid', timeoutMs: 4000 }); + expect(out).toBe('ONLY-PROMPT-CONTENT'); + }); }); diff --git a/src/__tests__/local-agent-tool-calling.test.ts b/src/__tests__/local-agent-tool-calling.test.ts index 00954228..c1c7d20f 100644 --- a/src/__tests__/local-agent-tool-calling.test.ts +++ b/src/__tests__/local-agent-tool-calling.test.ts @@ -257,6 +257,75 @@ describe('Claude local-agent session resume (Tier 2)', () => { await localBackbone().chat([{ role: 'user', content: 'hello' }]); expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); }); + + // PR #149 review (jmagly): formatPrompt() always serialized the WHOLE messages array, so a + // resumed call duplicated the transcript into a session that already had it — the exact resend + // this feature exists to eliminate. These pin the fix: same-array (same task) reuse sends only + // the NEW messages once a session exists; a different array (a new task) still gets everything. + it('a resumed call on the SAME growing messages array sends only the delta, not the full transcript', async () => { + const be = claudeBackbone(); + const messages: LLMMessage[] = [ + { role: 'system', content: 'SYSTEM-PROMPT-MARKER' }, + { role: 'user', content: 'USER-TASK-MARKER' }, + ]; + + cli.mockResolvedValueOnce(JSON.stringify({ + result: '{"tool_calls":[{"name":"nmap_scan","arguments":{"target":"x"}}]}', + session_id: 'sess-1', + })); + await be.chatWithTools(messages, TOOLS as never); + + // AgentLoop's real growth pattern: push the assistant turn + tool result onto the SAME array. + messages.push({ role: 'assistant', content: 'reasoning', toolCalls: [{ id: 'c1', name: 'nmap_scan', arguments: { target: 'x' } }] }); + messages.push({ role: 'tool', content: 'TOOL-RESULT-MARKER', toolCallId: 'c1', name: 'nmap_scan' }); + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'Final debrief.', session_id: 'sess-1' })); + await be.chatWithTools(messages, TOOLS as never); + + const secondPrompt = String(cli.mock.calls.at(-1)?.[1] || ''); + expect(secondPrompt).not.toContain('SYSTEM-PROMPT-MARKER'); + expect(secondPrompt).not.toContain('USER-TASK-MARKER'); + expect(secondPrompt).toContain('TOOL-RESULT-MARKER'); + expect(secondPrompt).toContain('ACTION CONTRACT'); // tool contract retained on the delta call + }); + + it('a NEW task (different messages array) still gets the full prompt even though the session resumes', async () => { + const be = claudeBackbone(); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await be.chat([{ role: 'system', content: 'TASK-ONE-SYSTEM' }, { role: 'user', content: 'TASK-ONE-USER' }]); + + // A brand-new task builds a brand-new array (AgentLoop.run() does this per task) — the + // resumed session has never seen this content, so it must go out in full despite --resume. + cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-1' })); + await be.chat([{ role: 'system', content: 'TASK-TWO-SYSTEM' }, { role: 'user', content: 'TASK-TWO-USER' }]); + + const secondCall = cli.mock.calls.at(-1); + const secondPrompt = String(secondCall?.[1] || ''); + expect(secondPrompt).toContain('TASK-TWO-SYSTEM'); + expect(secondPrompt).toContain('TASK-TWO-USER'); + expect(secondCall?.[2]).toMatchObject({ sessionId: 'sess-1', fallbackPrompt: undefined }); + }); + + it('passes fallbackPrompt (the FULL transcript) alongside a delta send, so a stale-session retry never gets the delta', async () => { + const be = claudeBackbone(); + const messages: LLMMessage[] = [{ role: 'system', content: 'S' }, { role: 'user', content: 'U' }]; + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await be.chat(messages); + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ fallbackPrompt: undefined }); // nothing to fall back from yet + + messages.push({ role: 'assistant', content: 'ok' }); + cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-1' })); + await be.chat(messages); + + const call = cli.mock.calls.at(-1); + const deltaPrompt = String(call?.[1] || ''); + const fallbackPrompt = String((call?.[2] as { fallbackPrompt?: string } | undefined)?.fallbackPrompt || ''); + expect(deltaPrompt).not.toContain('### SYSTEM\nS'); // delta omits the first task's system/user content + expect(deltaPrompt).not.toContain('### USER\nU'); + expect(fallbackPrompt).toContain('### SYSTEM\nS'); // fallback carries the FULL transcript a fresh session would need + expect(fallbackPrompt).toContain('### USER\nU'); + }); }); describe('codex backbone surfaces toolCalls (guards the CodexAdapter half of the fix)', () => { diff --git a/src/agent/local-agents.ts b/src/agent/local-agents.ts index 3ae774f8..4e5762db 100644 --- a/src/agent/local-agents.ts +++ b/src/agent/local-agents.ts @@ -489,7 +489,7 @@ export function pingLocalAgent(id: string, prompt?: string, timeoutMs?: number): * clean reply, while Hermes takes the prompt as an arg. Provider keys are stripped so each CLI uses its own * login (no API key needed). Throws on non-zero exit / timeout so the LLMBackbone retry/fallback fires. */ -export function localAgentChat(id: string, prompt: string, opts: { model?: string; timeoutMs?: number; sessionId?: string } = {}): Promise { +export function localAgentChat(id: string, prompt: string, opts: { model?: string; timeoutMs?: number; sessionId?: string; fallbackPrompt?: string } = {}): Promise { const spec = getSpec(id); if (!spec) return Promise.reject(new Error(`unknown local agent: ${id}`)); // child env: provider keys stripped + HOME pinned to the real agent home (see childEnv). @@ -529,7 +529,7 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin const cleanup = () => { if (workDir) { try { rmSync(workDir, { recursive: true, force: true }); } catch { /* noop */ } } }; const resolvedBin = resolveBin(spec.bin) || spec.bin; - const runOnce = (argv: string[]): Promise => new Promise((resolve, reject) => { + const runOnce = (argv: string[], promptToSend: string): Promise => new Promise((resolve, reject) => { const child = spawnAgent(resolvedBin, argv, { env, stdio: [viaStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); let out = ''; let errOut = ''; @@ -549,15 +549,16 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin else reject(new Error((errOut.trim() || content || `exited with code ${code}`).slice(0, 800))); }); }); - if (viaStdin && child.stdin) { child.stdin.write(prompt); child.stdin.end(); } + if (viaStdin && child.stdin) { child.stdin.write(promptToSend); child.stdin.end(); } }); - const first = runOnce(args); + const first = runOnce(args, prompt); if (!claudeArgsNoResume || args === claudeArgsNoResume) return first; // A resumed Claude session can go stale (CLI storage pruned, different session dir, expired). // Confirmed empirically: an unknown/invalid --resume id is a hard, fast failure (nonzero exit, // "No conversation found with session ID: ..." on stderr, no JSON on stdout) — the CLI never // falls back to a fresh session on its own. Do that fallback here, once, rather than failing - // the whole task over a stale id. - return first.catch(() => runOnce(claudeArgsNoResume as string[])); + // the whole task over a stale id. The fresh session has no history, so it MUST get the full + // transcript (fallbackPrompt), never the delta `prompt` that was sized for the resumed attempt. + return first.catch(() => runOnce(claudeArgsNoResume as string[], opts.fallbackPrompt ?? prompt)); } diff --git a/src/llm/index.ts b/src/llm/index.ts index 430f3aa8..54c85623 100755 --- a/src/llm/index.ts +++ b/src/llm/index.ts @@ -1368,9 +1368,20 @@ class LocalAgentAdapter implements LLMProviderAdapter { // one LocalAgentAdapter is held for an operator's whole life, so this naturally spans every task // until the caller (TempestCommand, on phase advance) calls resetSession() to start a new one. private claudeSessionId?: string; + // The exact `messages` array object sent last call, and how many of its entries were sent. + // A resumed session already HAS everything up to that point server-side — resending it would + // duplicate the transcript into the session instead of avoiding the resend (PR #149 review). + // Reference equality is deliberate: AgentLoop grows ONE array via push() across a task's + // iterations, so `messages === lastSentMessages` means "same task, continuing" and the tail + // past `lastSentCount` is the delta. A DIFFERENT array (a new task) means the resumed session + // has never seen any of it, so it goes out in full even though the CLI session itself resumes. + private lastSentMessages?: LLMMessage[]; + private lastSentCount = 0; constructor(config: LLMConfig) { this.config = config; } resetSession(): void { this.claudeSessionId = undefined; + this.lastSentMessages = undefined; + this.lastSentCount = 0; } // Split "agentId[::model]" → { agentId, agentModel }. No separator = just the agent id (CLI default model). private parseAgentSpec(): { agentId: string; agentModel?: string } { @@ -1401,20 +1412,30 @@ class LocalAgentAdapter implements LLMProviderAdapter { } async chat(messages: LLMMessage[], options?: ChatOptions): Promise { const { agentId, agentModel } = this.parseAgentSpec(); - const prompt = this.formatPrompt(messages, options); + const fullPrompt = this.formatPrompt(messages, options); + // Resuming AND still the same task's (same array object) growing transcript → send only the + // NEW messages since last call. A resumed session already has everything up to lastSentCount; + // resending it would duplicate the transcript into the session rather than avoiding the resend. + // A different array (a new task) or no session yet falls through to the full prompt. + const isDelta = agentId === 'claude' && this.claudeSessionId !== undefined && messages === this.lastSentMessages; + const sendPrompt = isDelta ? this.formatPrompt(messages.slice(this.lastSentCount), options) : fullPrompt; const timeoutMs = typeof this.config.timeout === 'number' && this.config.timeout > 0 ? this.config.timeout : undefined; - const raw = (await localAgentChat(agentId, prompt, { + const raw = (await localAgentChat(agentId, sendPrompt, { model: agentModel, timeoutMs, sessionId: agentId === 'claude' ? this.claudeSessionId : undefined, + // The stale-session fallback starts a genuinely fresh session with no history — it needs + // the FULL transcript, never the delta computed for the (failed) resumed attempt. + fallbackPrompt: isDelta ? fullPrompt : undefined, })).trim(); + if (agentId === 'claude') { this.lastSentMessages = messages; this.lastSentCount = messages.length; } // Claude requests --output-format json (see local-agents.ts) so this parses to REAL usage. // Anything else (parse failure, or a non-claude agent) falls back to the raw text — claude // additionally gets a character-based usage ESTIMATE so its budget check is never blind again. const parsed = agentId === 'claude' ? parseClaudeJsonEnvelope(raw) : null; const content = parsed ? parsed.content : raw; const usage = agentId === 'claude' - ? (parsed?.usage ?? estimateUsage(prompt, parsed?.content ?? raw)) + ? (parsed?.usage ?? estimateUsage(sendPrompt, parsed?.content ?? raw)) : undefined; // Carry the session forward so the NEXT call (--resume) picks up where this one left off. // Empirically stable across --resume (confirmed against the real CLI), but re-capture anyway From a0f0d4554837e9b16fa348907aacc90a5854b432 Mon Sep 17 00:00:00 2001 From: N3thunt3r69 Date: Thu, 13 Aug 2026 10:31:45 -0400 Subject: [PATCH 3/5] fix(test): import LLMMessage in local-agent-tool-calling.test.ts Hosted CI caught it, not my local run: the previous commit used LLMMessage[] type annotations in two new tests without importing the type, so tsc failed with TS2304 while vitest itself stayed green (esbuild strips type annotations without checking them). I ran typecheck once before writing those tests and never reran it after, so this went out uncaught. Full suite: 769/769. npx tsc --noEmit: clean. --- src/__tests__/local-agent-tool-calling.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/local-agent-tool-calling.test.ts b/src/__tests__/local-agent-tool-calling.test.ts index c1c7d20f..31aa46b2 100644 --- a/src/__tests__/local-agent-tool-calling.test.ts +++ b/src/__tests__/local-agent-tool-calling.test.ts @@ -23,7 +23,7 @@ vi.mock('fs/promises', async (orig) => { return { ...actual, readFile: vi.fn() }; }); -import { parseTextToolCalls, LLMBackbone } from '../llm/index.js'; +import { parseTextToolCalls, LLMBackbone, type LLMMessage } from '../llm/index.js'; import { localAgentChat } from '../agent/local-agents.js'; import { readFile } from 'fs/promises'; const cli = vi.mocked(localAgentChat); From 07108d511472f72e826340fbbd76b9eab09a5689 Mon Sep 17 00:00:00 2001 From: N3thunt3r69 Date: Thu, 13 Aug 2026 11:10:19 -0400 Subject: [PATCH 4/5] fix(llm): isolate per-operator Claude sessions and narrow the fallback A self-review after the last round turned up a more serious version of what jmagly caught: TempestCommand built exactly one LLMBackbone and handed the SAME instance to every operator's AgentLoop (and to each OperatorAgent's own decompose-on-failure path). LocalAgentAdapter's new session/delta state lived on that shared instance, so one operator's Claude Code session could get resumed by a completely different operator, with the second operator's unrelated task content sent into the first operator's conversation. Every test for the resume feature so far constructed independent LLMBackbone instances per test, which is not how spawnOperator() actually wires things, so nothing caught it. spawnOperator() now builds a fresh LLMBackbone per operator from the same config, used for both that operator's AgentLoop and its own decompose fallback, so state stays consistent within one operator and isolated from every other one. OperatorCell.spawnOperator() takes an optional llm override to carry that through; existing callers that don't pass one keep the cell's shared default. Two narrower issues from the same review: localAgentChat's stale-session fallback retried on ANY failure of a resumed call, not just a confirmed stale session. A real network error or auth failure would silently double latency on a retry that could not fix it, and compounded with LLMBackbone's own retry ladder into up to 6 CLI spawns for one failing turn. The fallback now only fires for the exact empirically-confirmed stale-session error text ("No conversation found with session ID"). The character-estimate usage fallback (used only when the JSON envelope fails to parse) estimated from the delta prompt instead of the full transcript. A resumed call's real promptTokens reflects the whole cached conversation, confirmed empirically in the last round (19127 -> 19625 -> 20135 -> 20645), so estimating from the small delta text instead undercounted the budget check by an order of magnitude in that one edge case. One finding from the review was deliberately left alone: resending the tool contract on every delta call is real overhead, but jmagly's review explicitly required the contract stay on every call ("retaining ... the tool contract") and the existing test already pins that. Cutting it would contradict that requirement for a small, bounded savings. New coverage: an isolation test that spawns two operators through a real TempestCommand and confirms neither's LocalAgentAdapter nor its session leaks into the other, a test proving a non-stale failure propagates without a fallback retry, and a test proving the usage estimate reflects the full transcript on a mid-session parse failure. Full suite: 772/772, no regressions. --- src/__tests__/index.test.ts | 35 ++++++++++++++++++- .../local-agent-path-resolution.test.ts | 30 ++++++++++++++-- .../local-agent-tool-calling.test.ts | 24 +++++++++++++ src/agent/local-agents.ts | 10 +++++- src/index.ts | 17 +++++++-- src/llm/index.ts | 7 +++- src/operators/index.ts | 5 +-- 7 files changed, 117 insertions(+), 11 deletions(-) diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 1fc8835d..738ce2c0 100755 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -4,7 +4,7 @@ * Basic test suite for core functionality. */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { Arsenal, BUILTIN_TOOLS, successResult, failResult, createToolContext } from '../arsenal/index.js'; import { FRONTIER_ARSENAL_MILESTONE, SAFE_COMMANDS, TOOL_ADAPTERS, adaptersForFamily, summarizeToolCatalog } from '../arsenal/catalog.js'; import { createKnowledgeBase, createEvasionEngine, CVE_DATABASE, MITRE_TECHNIQUES } from '../stubs/index.js'; @@ -12,6 +12,12 @@ import { AGENT_PROMPT_PACKS, FOREFRONT_PRESSURE_LANES, OPERATOR_RUNBOOKS, forefr import { OpGeneral } from '../general/index.js'; import { LLMBackbone } from '../llm/index.js'; +// Scoped to this file's new per-operator-isolation test (PR #149 review follow-up) — every other +// test here uses provider: 'mock', which never touches this module. +vi.mock('../agent/local-agents.js', () => ({ localAgentChat: vi.fn() })); +import { localAgentChat } from '../agent/local-agents.js'; +const localAgentCli = vi.mocked(localAgentChat); + describe('Arsenal', () => { let arsenal: Arsenal; @@ -465,6 +471,33 @@ describe('Wedged-dispatch timeout backstop', () => { else process.env.T3MP3ST_TASK_TIMEOUT_MS = prev; } }, 15000); + + // PR #149 review follow-up: spawnOperator() used to hand every operator the SAME shared + // this.llm, so LocalAgentAdapter's Claude Code session-resume state (added for --resume) leaked + // across operators — one operator's session could get resumed with a different operator's + // unrelated task content. Each operator now gets its own LLMBackbone from the same config. + it('spawnOperator() gives each operator its OWN LLMBackbone, not a shared one (no cross-operator Claude session leakage)', async () => { + const mod = await import('../index.js'); + const command = new mod.TempestCommand({ + name: 'Isolation Op', + llm: { provider: 'local-agent', model: 'claude' }, + }); + + const opA = command.spawnOperator('Recon-Iso', 'recon'); + const opB = command.spawnOperator('Scanner-Iso', 'scanner'); + + expect((opA as any).llm).not.toBe((opB as any).llm); + expect((opA as any).llm).not.toBe(command.llm); + + localAgentCli.mockResolvedValueOnce(JSON.stringify({ result: 'a1', session_id: 'sess-A' })); + await (opA as any).llm.chat([{ role: 'user', content: 'hello A' }]); + + // Operator B has never talked to Claude before — its first call must NOT resume A's session. + localAgentCli.mockResolvedValueOnce(JSON.stringify({ result: 'b1', session_id: 'sess-B' })); + await (opB as any).llm.chat([{ role: 'user', content: 'hello B' }]); + + expect(localAgentCli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + }); }); describe('Codex account provider', () => { diff --git a/src/__tests__/local-agent-path-resolution.test.ts b/src/__tests__/local-agent-path-resolution.test.ts index 9692c6c1..3fe5f002 100644 --- a/src/__tests__/local-agent-path-resolution.test.ts +++ b/src/__tests__/local-agent-path-resolution.test.ts @@ -310,6 +310,17 @@ case "$*" in esac `; +// A --resume failure that is NOT the stale-session error. If a fallback retry incorrectly fired, +// the fresh (non-resume) branch below would resolve successfully — so a test asserting this +// REJECTS proves no retry happened, not just that the final error text matches. +const FAKE_CLI_NONSTALE_RESUME_ERROR = `#!/bin/sh +cat >/dev/null 2>&1 +case "$*" in + *--resume*) echo "connection refused" >&2; exit 1 ;; + *) echo '{"result":"should not be reached — a retry fired for a non-stale error","session_id":"x"}' ;; +esac +`; + describe('localAgentChat — stale Claude session fallback (Tier 2)', () => { it('retries WITHOUT --resume when the resumed session is stale, and succeeds', async () => { const home = scratch(); @@ -340,13 +351,26 @@ describe('localAgentChat — stale Claude session fallback (Tier 2)', () => { await expect(localAgentChat('claude', 'ping', { timeoutMs: 4000 })).rejects.toThrow(/boom/); }); - it('propagates the fallback attempt\'s own error when both the resumed and fresh calls fail', async () => { + it('a non-stale failure with a session in play propagates without a fallback retry (narrowed trigger, PR #149 review)', async () => { + const home = scratch(); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_NONSTALE_RESUME_ERROR); + process.env.T3MP3ST_AGENT_HOME = home; + process.env.PATH = '/usr/bin:/bin'; + + // If the (old, unnarrowed) fallback fired here, this would RESOLVE with the fresh branch's + // success text instead of rejecting — a rejection proves no retry happened. + await expect(localAgentChat('claude', 'ping', { sessionId: 'whatever', timeoutMs: 4000 })) + .rejects.toThrow(/connection refused/); + }); + + it('the specific stale-session error still triggers exactly one fallback retry', async () => { const home = scratch(); - putExe(join(home, '.local', 'bin'), 'claude', '#!/bin/sh\ncat >/dev/null 2>&1\necho "both broken" >&2\nexit 1\n'); + putExe(join(home, '.local', 'bin'), 'claude', FAKE_CLI_RESUME_STALE); process.env.T3MP3ST_AGENT_HOME = home; process.env.PATH = '/usr/bin:/bin'; - await expect(localAgentChat('claude', 'ping', { sessionId: 'whatever', timeoutMs: 4000 })).rejects.toThrow(/both broken/); + const out = await localAgentChat('claude', 'ping', { sessionId: 'stale-uuid', timeoutMs: 4000 }); + expect(JSON.parse(out).result).toBe('fresh ok'); }); // PR #149 review (jmagly): the fresh-session retry has no history at all, so it must receive the diff --git a/src/__tests__/local-agent-tool-calling.test.ts b/src/__tests__/local-agent-tool-calling.test.ts index 31aa46b2..6ed69817 100644 --- a/src/__tests__/local-agent-tool-calling.test.ts +++ b/src/__tests__/local-agent-tool-calling.test.ts @@ -326,6 +326,30 @@ describe('Claude local-agent session resume (Tier 2)', () => { expect(fallbackPrompt).toContain('### SYSTEM\nS'); // fallback carries the FULL transcript a fresh session would need expect(fallbackPrompt).toContain('### USER\nU'); }); + + // PR #149 review follow-up: a real envelope's promptTokens on a resumed call reflects the WHOLE + // cached conversation (input + cache_creation + cache_read), not the wire size of the delta — + // confirmed empirically. The character-estimate fallback (used only when the envelope fails to + // parse) must approximate that same full-conversation size, or it silently undercounts the + // budget check by an order of magnitude on a delta call. + it('the usage estimate fallback reflects the FULL transcript, not the delta, on a mid-session parse failure', async () => { + const be = claudeBackbone(); + const messages: LLMMessage[] = [ + { role: 'system', content: 'S'.repeat(2000) }, + { role: 'user', content: 'U'.repeat(2000) }, + ]; + + cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); + await be.chat(messages); + + messages.push({ role: 'assistant', content: 'ok' }); + cli.mockResolvedValueOnce('not json'); // envelope parse failure -> falls to the estimate + const res = await be.chat(messages); + + // A delta-only estimate would be tiny (just "ok" plus the preamble/tool contract). The full + // ~4000 chars of original system/user content must show up in the estimate too. + expect(res.usage?.promptTokens).toBeGreaterThan(900); + }); }); describe('codex backbone surfaces toolCalls (guards the CodexAdapter half of the fix)', () => { diff --git a/src/agent/local-agents.ts b/src/agent/local-agents.ts index 4e5762db..3b4f7d69 100644 --- a/src/agent/local-agents.ts +++ b/src/agent/local-agents.ts @@ -560,5 +560,13 @@ export function localAgentChat(id: string, prompt: string, opts: { model?: strin // falls back to a fresh session on its own. Do that fallback here, once, rather than failing // the whole task over a stale id. The fresh session has no history, so it MUST get the full // transcript (fallbackPrompt), never the delta `prompt` that was sized for the resumed attempt. - return first.catch(() => runOnce(claudeArgsNoResume as string[], opts.fallbackPrompt ?? prompt)); + // + // Only retry for a CONFIRMED stale session (the exact empirical error text above). Any other + // failure — a network blip, a real auth error, a timeout — propagates as-is instead of silently + // doubling latency and cost on a retry that would not fix the actual problem (PR #149 review). + return first.catch((err) => { + const message = err instanceof Error ? err.message : String(err); + if (!/no conversation found/i.test(message)) throw err; + return runOnce(claudeArgsNoResume as string[], opts.fallbackPrompt ?? prompt); + }); } diff --git a/src/index.ts b/src/index.ts index 5da13f70..1999bbdf 100755 --- a/src/index.ts +++ b/src/index.ts @@ -293,6 +293,11 @@ export class TempestCommand extends EventEmitter { public readonly comms: CommsChannel; public readonly analysis: AnalysisEngine; public readonly llm: LLMBackbone; + // The config `this.llm` was built from — kept so spawnOperator() can build each operator its + // OWN LLMBackbone instead of sharing this one. LocalAgentAdapter carries per-conversation Claude + // Code session state (session id, sent-message tracking); sharing one instance across operators + // let one operator's session state leak into another's calls (PR #149 review follow-up). + private readonly llmConfig: TempestConfig['llm']; /** * Stub modules (interface-only). @@ -369,6 +374,7 @@ export class TempestCommand extends EventEmitter { this.taskTimeoutMs = TempestCommand.resolveTaskTimeoutMs(config.llm.provider); // Initialize LLM backbone + this.llmConfig = config.llm; this.llm = new LLMBackbone(config.llm); // Initialize core subsystems @@ -1242,16 +1248,21 @@ export class TempestCommand extends EventEmitter { callsign: string, archetype: OperatorArchetype ): OperatorAgent { - const operator = this.cell.spawnOperator(callsign, archetype); + // Each operator gets its OWN LLMBackbone (same config as the mission's) rather than sharing + // this.llm — see the llmConfig field comment. Used for BOTH this operator's AgentLoop and its + // own decompose-on-failure fallback, so the two stay consistent with each other while staying + // isolated from every other operator in the cell. + const operatorLLM = new LLMBackbone(this.llmConfig); + const operator = this.cell.spawnOperator(callsign, archetype, undefined, operatorLLM); this.setupOperatorEvents(operator); // Attach the agent loop scoped to this archetype's SPECIALIZED role toolkit (defaultTools = // the curated per-operator tool allowlist). toolCategories stays as a coarse fallback. const profile = ARCHETYPE_PROFILES[archetype]; - const maxIterations = this.llm.getProvider() === 'local-agent' + const maxIterations = operatorLLM.getProvider() === 'local-agent' ? LOCAL_AGENT_MAX_ITERATIONS : DEFAULT_AGENT_MAX_ITERATIONS; - const agentLoop = new AgentLoop(this.llm, this.arsenal, { + const agentLoop = new AgentLoop(operatorLLM, this.arsenal, { maxIterations, maxTokens: 50000, toolCategories: profile.toolCategories, diff --git a/src/llm/index.ts b/src/llm/index.ts index 54c85623..f1a6ce3e 100755 --- a/src/llm/index.ts +++ b/src/llm/index.ts @@ -1434,8 +1434,13 @@ class LocalAgentAdapter implements LLMProviderAdapter { // additionally gets a character-based usage ESTIMATE so its budget check is never blind again. const parsed = agentId === 'claude' ? parseClaudeJsonEnvelope(raw) : null; const content = parsed ? parsed.content : raw; + // Estimate from the FULL prompt, not sendPrompt: a real envelope's promptTokens on a delta + // call is large (input + cache_creation + cache_read for the WHOLE resumed context, confirmed + // empirically — a resumed call's real promptTokens tracks the full conversation, not the wire + // size of the delta). Estimating from the tiny delta text instead would undercount the budget + // check by an order of magnitude in the one case this estimate exists to cover (PR #149 review). const usage = agentId === 'claude' - ? (parsed?.usage ?? estimateUsage(sendPrompt, parsed?.content ?? raw)) + ? (parsed?.usage ?? estimateUsage(fullPrompt, parsed?.content ?? raw)) : undefined; // Carry the session forward so the NEXT call (--resume) picks up where this one left off. // Empirically stable across --resume (confirmed against the real CLI), but re-capture anyway diff --git a/src/operators/index.ts b/src/operators/index.ts index a66640af..4ae2a284 100755 --- a/src/operators/index.ts +++ b/src/operators/index.ts @@ -849,7 +849,8 @@ export class OperatorCell extends EventEmitter { spawnOperator( callsign: string, archetype: OperatorArchetype, - config?: Partial + config?: Partial, + llm?: LLMBackbone ): OperatorAgent { if (this.operators.size >= this.maxOperators) { this.emit('cell:capacity_warning', { @@ -866,7 +867,7 @@ export class OperatorCell extends EventEmitter { } } - const operator = new OperatorAgent(callsign, archetype, config, this.llm); + const operator = new OperatorAgent(callsign, archetype, config, llm ?? this.llm); // Forward operator events operator.on('status:changed', ({ oldStatus }) => { From 2ab943898df4793d9d9db3bfe8b0f68ec6892e44 Mon Sep 17 00:00:00 2001 From: N3thunt3r69 Date: Thu, 13 Aug 2026 12:07:50 -0400 Subject: [PATCH 5/5] fix(llm): reset the Claude session at every task boundary jmagly's third review: switching from delta to full-prompt sending on a new messages array wasn't enough, since LocalAgentAdapter.chat() still passed the OLD claudeSessionId either way. A second task on the same operator inherited the first task's persisted Claude session, carrying its system prompt, target data, and tool output (including anything attacker-controlled) across a boundary T3MP3ST already has a single sanctioned channel for: PackBoard. chat() now clears claudeSessionId whenever the incoming messages array is not the one it last saw, before building the request. A different array means AgentLoop.run() started a new task; the call after that reset carries no --resume, so it opens a genuinely fresh session instead of resuming the old one under a bigger prompt. Within one task the array is the same object across iterations (AgentLoop grows it via push), so resumption and the delta-only send from the last round are untouched. Rewrote the tests that asserted continuity across separate array literals, since that shape doesn't happen in AgentLoop and was masking this exact gap: they now grow one array via push to represent a real task, matching the pattern already used for the delta tests. Added the test jmagly asked for directly: one operator, two sequential tasks, task 2 has no session id, both at the LocalAgentAdapter level and through a real TempestCommand.spawnOperator(). Full suite: 773/773, no regressions. --- src/__tests__/index.test.ts | 23 +++++++++ .../local-agent-tool-calling.test.ts | 48 +++++++++++++------ src/llm/index.ts | 9 ++++ 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 738ce2c0..423a6d8a 100755 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -498,6 +498,29 @@ describe('Wedged-dispatch timeout backstop', () => { expect(localAgentCli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); }); + + // PR #149 review, round 3 (jmagly): cross-OPERATOR isolation isn't enough — the SAME operator + // running a second task must not resume the first task's session either. The old session belongs + // to a task (and potentially a target) that never earned a place in the new task's context; + // PackBoard is the one sanctioned channel for carrying anything across a task boundary. + it('one operator running TWO SEQUENTIAL tasks does not resume task 1\'s session for task 2 (production-shaped)', async () => { + const mod = await import('../index.js'); + const command = new mod.TempestCommand({ + name: 'Task Boundary Op', + llm: { provider: 'local-agent', model: 'claude' }, + }); + const op = command.spawnOperator('Recon-Task-Boundary', 'recon'); + + localAgentCli.mockResolvedValueOnce(JSON.stringify({ result: 'task1', session_id: 'sess-task-1' })); + await (op as any).llm.chat([{ role: 'system', content: 'TASK-1' }, { role: 'user', content: 'do task 1' }]); + expect(localAgentCli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + + // AgentLoop.run() builds a brand-new messages array per task — simulate that exactly. + localAgentCli.mockResolvedValueOnce(JSON.stringify({ result: 'task2', session_id: 'sess-task-2' })); + await (op as any).llm.chat([{ role: 'system', content: 'TASK-2' }, { role: 'user', content: 'do task 2' }]); + + expect(localAgentCli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); + }); }); describe('Codex account provider', () => { diff --git a/src/__tests__/local-agent-tool-calling.test.ts b/src/__tests__/local-agent-tool-calling.test.ts index 6ed69817..034e6ced 100644 --- a/src/__tests__/local-agent-tool-calling.test.ts +++ b/src/__tests__/local-agent-tool-calling.test.ts @@ -202,26 +202,33 @@ describe('Claude local-agent session resume (Tier 2)', () => { expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); }); - it('captures session_id from the response and passes it as --resume on the next call', async () => { + it('captures session_id from the response and passes it as --resume on a LATER call within the SAME task', async () => { const be = claudeBackbone(); + // AgentLoop's real shape: one array, grown via push() across a task's iterations. + const messages: LLMMessage[] = [{ role: 'user', content: 'hello' }]; cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); - await be.chat([{ role: 'user', content: 'hello' }]); + await be.chat(messages); + messages.push({ role: 'assistant', content: 'first' }, { role: 'user', content: 'again' }); cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-1' })); - await be.chat([{ role: 'user', content: 'again' }]); + await be.chat(messages); expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: 'sess-1' }); }); - it('resetLocalAgentSession() drops the tracked session so the next call starts fresh', async () => { + it('resetLocalAgentSession() drops the tracked session so the next call starts fresh, even mid-task', async () => { const be = claudeBackbone(); + // SAME array both calls, so this isolates the EXPLICIT reset from the automatic task-boundary + // reset (a different array would reset on its own regardless of this call). + const messages: LLMMessage[] = [{ role: 'user', content: 'hello' }]; cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); - await be.chat([{ role: 'user', content: 'hello' }]); + await be.chat(messages); be.resetLocalAgentSession(); + messages.push({ role: 'assistant', content: 'first' }, { role: 'user', content: 'again' }); cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-2' })); - await be.chat([{ role: 'user', content: 'again' }]); + await be.chat(messages); expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); }); @@ -238,16 +245,19 @@ describe('Claude local-agent session resume (Tier 2)', () => { expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); }); - it('a resumed session with no usable id in the envelope keeps the PRIOR session for the next call', async () => { + it('a resumed session with no usable id in the envelope keeps the PRIOR session for the next call in the SAME task', async () => { const be = claudeBackbone(); + const messages: LLMMessage[] = [{ role: 'user', content: 'hello' }]; cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); - await be.chat([{ role: 'user', content: 'hello' }]); + await be.chat(messages); + messages.push({ role: 'assistant', content: 'first' }, { role: 'user', content: 'again' }); cli.mockResolvedValueOnce(JSON.stringify({ result: 'second' })); // no session_id this time - await be.chat([{ role: 'user', content: 'again' }]); + await be.chat(messages); + messages.push({ role: 'assistant', content: 'second' }, { role: 'user', content: 'once more' }); cli.mockResolvedValueOnce(JSON.stringify({ result: 'third', session_id: 'sess-1' })); - await be.chat([{ role: 'user', content: 'once more' }]); + await be.chat(messages); expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: 'sess-1' }); }); @@ -289,21 +299,29 @@ describe('Claude local-agent session resume (Tier 2)', () => { expect(secondPrompt).toContain('ACTION CONTRACT'); // tool contract retained on the delta call }); - it('a NEW task (different messages array) still gets the full prompt even though the session resumes', async () => { + // PR #149 review (jmagly, round 3): a new task must not just switch from delta to full-prompt + // sending while still resuming the OLD session — it must start a genuinely FRESH session. The + // old session belongs to a different task (and potentially a different target); resuming it + // would carry that task's system prompt, target data, and tool output — including anything + // attacker-controlled — into a task that never earned it, bypassing PackBoard as the one + // sanctioned cross-task channel. One operator, two sequential tasks: task 2 gets no --resume. + it('a NEW task (different messages array) starts a genuinely FRESH session, not a resume of the prior task', async () => { const be = claudeBackbone(); cli.mockResolvedValueOnce(JSON.stringify({ result: 'first', session_id: 'sess-1' })); await be.chat([{ role: 'system', content: 'TASK-ONE-SYSTEM' }, { role: 'user', content: 'TASK-ONE-USER' }]); + expect(cli.mock.calls.at(-1)?.[2]).toMatchObject({ sessionId: undefined }); - // A brand-new task builds a brand-new array (AgentLoop.run() does this per task) — the - // resumed session has never seen this content, so it must go out in full despite --resume. - cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-1' })); + // A brand-new task builds a brand-new array (AgentLoop.run() does this per task). + cli.mockResolvedValueOnce(JSON.stringify({ result: 'second', session_id: 'sess-2' })); await be.chat([{ role: 'system', content: 'TASK-TWO-SYSTEM' }, { role: 'user', content: 'TASK-TWO-USER' }]); const secondCall = cli.mock.calls.at(-1); const secondPrompt = String(secondCall?.[1] || ''); expect(secondPrompt).toContain('TASK-TWO-SYSTEM'); expect(secondPrompt).toContain('TASK-TWO-USER'); - expect(secondCall?.[2]).toMatchObject({ sessionId: 'sess-1', fallbackPrompt: undefined }); + expect(secondPrompt).not.toContain('TASK-ONE-SYSTEM'); // task 1's content never enters the wire prompt either + // The core assertion: task 2's session id is NOT task 1's — no --resume across the task boundary. + expect(secondCall?.[2]).toMatchObject({ sessionId: undefined, fallbackPrompt: undefined }); }); it('passes fallbackPrompt (the FULL transcript) alongside a delta send, so a stale-session retry never gets the delta', async () => { diff --git a/src/llm/index.ts b/src/llm/index.ts index f1a6ce3e..14d06ab1 100755 --- a/src/llm/index.ts +++ b/src/llm/index.ts @@ -1412,6 +1412,15 @@ class LocalAgentAdapter implements LLMProviderAdapter { } async chat(messages: LLMMessage[], options?: ChatOptions): Promise { const { agentId, agentModel } = this.parseAgentSpec(); + // Task boundary: a DIFFERENT messages array means AgentLoop.run() started a new task (it builds + // one fresh array per task). Resuming across that boundary would carry the prior task's system + // prompt, target data, and tool output — including anything attacker-controlled — into a task + // that never earned it, outside T3MP3ST's one sanctioned cross-task channel (PackBoard). Only + // resume WITHIN one task's own growing transcript; a new task always starts a fresh session + // (PR #149 review). + if (agentId === 'claude' && this.lastSentMessages !== undefined && messages !== this.lastSentMessages) { + this.claudeSessionId = undefined; + } const fullPrompt = this.formatPrompt(messages, options); // Resuming AND still the same task's (same array object) growing transcript → send only the // NEW messages since last call. A resumed session already has everything up to lastSentCount;