diff --git a/packages/core/src/agent/agent-loop.test.ts b/packages/core/src/agent/agent-loop.test.ts index 4126320..29d9bfd 100644 --- a/packages/core/src/agent/agent-loop.test.ts +++ b/packages/core/src/agent/agent-loop.test.ts @@ -131,6 +131,38 @@ describe("AgentLoop", () => { }); }); + it("fails without requesting the model when compaction leaves no output budget", async () => { + const memory = new ConversationMemory(makeMemoryConfig()); + const createChatCompletion = vi.fn(); + const loop = new AgentLoop({ + model: "gpt-4o", + client: { + createChatCompletion, + countPromptTokens: vi.fn().mockResolvedValue(128_000), + } as never, + memory, + tools: { + getDefinitions: vi.fn().mockReturnValue([]), + executeTool: vi.fn(), + inspectTool: vi.fn(), + getCatalog: vi.fn().mockReturnValue([]), + searchTools: vi.fn().mockReturnValue([]), + getCodeModeToolBindings: vi.fn().mockReturnValue([]), + } as never, + systemPrompt: "sys", + workspaceRoot: "/tmp", + config: makeConfig(), + }); + + const result = await loop.run("test"); + + expect(result.output).toContain("Context window remains exhausted"); + expect(result.stateTimeline.some((entry) => entry.state === "failed")).toBe( + true, + ); + expect(createChatCompletion).not.toHaveBeenCalled(); + }); + describe("AgentLoopOptions types", () => { it("options accept hooks", () => { const hooks: NonNullable = { diff --git a/packages/core/src/agent/agent-loop.ts b/packages/core/src/agent/agent-loop.ts index b9206fc..923f5c2 100644 --- a/packages/core/src/agent/agent-loop.ts +++ b/packages/core/src/agent/agent-loop.ts @@ -291,6 +291,28 @@ export class AgentLoop { requestTemplate, ); const stepMaxTokens = computeStepMaxTokens(this.config, promptTokens); + if (stepMaxTokens === 0) { + const failure = + "Context window remains exhausted after compaction; cannot request a model completion."; + this.emitState( + stateMachine.transition({ + state: "failed", + step, + toolCalls: totalToolCalls, + note: failure, + }), + ); + this.memory.addAssistant(failure); + this.memory.recordDecision(failure); + return this.finishRun({ + output: failure, + step, + toolCalls: totalToolCalls, + success: false, + stateMachine, + actions, + }); + } const request = { ...requestTemplate, max_tokens: stepMaxTokens, @@ -497,12 +519,18 @@ export class AgentLoop { if (hookRejection) { result = hookRejection; } else { - this.hooks?.onToolStart?.({ - toolName, - rawArgs: toolCall.function.arguments, - workspaceRoot: this.workspaceRoot, - inspection, - }); + try { + this.hooks?.onToolStart?.({ + toolName, + rawArgs: toolCall.function.arguments, + workspaceRoot: this.workspaceRoot, + inspection, + }); + } catch (error) { + this.memory.recordDecision( + `onToolStart hook failed: ${shorten(toErrorMessage(error), 160)}`, + ); + } result = await this.executeToolWithRetries( toolName, toolCall.function.arguments, @@ -511,11 +539,17 @@ export class AgentLoop { } } - this.hooks?.onToolResult?.({ - toolName, - toolCallId: toolCall.id, - result, - }); + try { + this.hooks?.onToolResult?.({ + toolName, + toolCallId: toolCall.id, + result, + }); + } catch (error) { + this.memory.recordDecision( + `onToolResult hook failed: ${shorten(toErrorMessage(error), 160)}`, + ); + } this.memory.addTool( toolCall.id, @@ -679,7 +713,11 @@ export class AgentLoop { for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { this.throwIfInterrupted(); + const attemptAbort = createDerivedAbortSignal(this.signal); + let retryDelayMs: number | undefined; try { + // Derive a fresh signal per attempt so listeners don't accumulate + // on a shared signal object across retries. const currentRequest: CompletionRequest = { ...request, trace: request.trace @@ -688,7 +726,9 @@ export class AgentLoop { requestAttempt: attempt, } : undefined, - ...(this.signal ? { signal: this.signal } : undefined), + ...(attemptAbort.signal + ? { signal: attemptAbort.signal } + : undefined), }; if (this.client.streamChatCompletion) { @@ -723,11 +763,16 @@ export class AgentLoop { break; } - const delayMs = computeRetryDelayMs(attempt); + retryDelayMs = computeRetryDelayMs(attempt, this.config); this.memory.recordDecision( - `Model request failed (attempt ${attempt}/${maxAttempts}): ${shorten(toErrorMessage(error), 180)}; retrying in ${delayMs}ms`, + `Model request failed (attempt ${attempt}/${maxAttempts}): ${shorten(toErrorMessage(error), 180)}; retrying in ${retryDelayMs}ms`, ); - await sleep(delayMs, this.signal); + } finally { + attemptAbort.cleanup(); + } + + if (retryDelayMs !== undefined) { + await sleep(retryDelayMs, this.signal); } } @@ -962,8 +1007,10 @@ export class AgentLoop { try { await this.hooks.onCheckpoint({ step, toolCalls, snapshot }); } catch (error) { + const msg = `Checkpoint hook failed: ${shorten(toErrorMessage(error), 160)}`; + this.memory.recordDecision(msg); this.memory.recordDecision( - `Checkpoint hook failed: ${shorten(toErrorMessage(error), 160)}`, + "WARNING: checkpoint not persisted — state may be lost on recovery", ); } } @@ -1041,13 +1088,41 @@ function shouldRetryToolResult(result: ToolExecutionResult): boolean { return code === "TOOL_EXECUTION_FAILED"; } -function computeRetryDelayMs(attempt: number): number { - const base = 300; - const exponential = Math.min(2_500, base * 2 ** (attempt - 1)); - const jitter = Math.floor(Math.random() * 120); +function computeRetryDelayMs( + attempt: number, + config?: Pick< + AgentRunConfig, + "retryDelayBase" | "retryDelayMax" | "retryDelayJitter" + >, +): number { + const base = config?.retryDelayBase ?? 300; + const maxDelay = config?.retryDelayMax ?? 2_500; + const jitter = Math.floor(Math.random() * (config?.retryDelayJitter ?? 120)); + const exponential = Math.min(maxDelay, base * 2 ** (attempt - 1)); return exponential + jitter; } +function createDerivedAbortSignal(parent?: AbortSignal): { + signal?: AbortSignal; + cleanup: () => void; +} { + if (!parent) { + return { cleanup: () => undefined }; + } + + const controller = new AbortController(); + const abort = () => controller.abort(parent.reason); + if (parent.aborted) { + abort(); + } else { + parent.addEventListener("abort", abort, { once: true }); + } + return { + signal: controller.signal, + cleanup: () => parent.removeEventListener("abort", abort), + }; +} + async function sleep(delayMs: number, signal?: AbortSignal): Promise { await new Promise((resolve, reject) => { const timer = setTimeout(() => { @@ -1180,13 +1255,14 @@ function computeStepMaxTokens( config: AgentRunConfig, promptTokens: number, ): number { + const remainingBudget = + config.maxContextTokens - promptTokens - config.outputTokenSafetyMargin; + if (remainingBudget <= 0) return 0; const minOutputTokens = Math.min( config.minOutputTokens, config.maxOutputTokens, ); const maxOutputTokens = Math.max(minOutputTokens, config.maxOutputTokens); - const remainingBudget = - config.maxContextTokens - promptTokens - config.outputTokenSafetyMargin; return clamp(remainingBudget, minOutputTokens, maxOutputTokens); } @@ -1254,9 +1330,9 @@ function toRunMetadata( goalId: context?.goalId, attemptId: context?.attemptId, runStartedAt: context?.runStartedAt, - workspaceMode: context?.executionProfile.workspaceMode, - memoryMode: context?.executionProfile.memoryMode, - priority: context?.executionProfile.priority, + workspaceMode: context?.executionProfile?.workspaceMode, + memoryMode: context?.executionProfile?.memoryMode, + priority: context?.executionProfile?.priority, }; } diff --git a/packages/core/src/agent/state-machine.test.ts b/packages/core/src/agent/state-machine.test.ts index 9b8378a..e2a00a1 100644 --- a/packages/core/src/agent/state-machine.test.ts +++ b/packages/core/src/agent/state-machine.test.ts @@ -50,6 +50,56 @@ describe("AgentStateMachine", () => { expect(sm.getTimeline()).toHaveLength(1); }); + it("rejects invalid transitions", () => { + const sm = new AgentStateMachine(); + + expect(() => + sm.transition({ state: "final_response", step: 1, toolCalls: 0 }), + ).toThrow("Invalid state transition: prepare_context -> final_response"); + }); + + it("accepts the AgentLoop state transition sequence", () => { + const sm = new AgentStateMachine(); + const states = [ + "goal_start", + "prepare_context", + "before_model_request_hooks", + "context_compaction", + "model_request", + "tool_execution", + "apply_tool_results", + "prepare_context", + "context_compaction", + "model_request", + "final_response", + "goal_complete", + ] as const; + + expect(() => { + for (const [step, state] of states.entries()) { + sm.transition({ state, step, toolCalls: 0 }); + } + }).not.toThrow(); + }); + + it("accepts the AgentLoop failure completion sequence", () => { + const sm = new AgentStateMachine(); + const states = [ + "goal_start", + "prepare_context", + "context_compaction", + "model_request", + "failed", + "goal_complete", + ] as const; + + expect(() => { + for (const [step, state] of states.entries()) { + sm.transition({ state, step, toolCalls: 0 }); + } + }).not.toThrow(); + }); + it("accumulates multiple transitions in the timeline", () => { const sm = new AgentStateMachine(); diff --git a/packages/core/src/agent/state-machine.ts b/packages/core/src/agent/state-machine.ts index e6f5341..2e5c3cb 100644 --- a/packages/core/src/agent/state-machine.ts +++ b/packages/core/src/agent/state-machine.ts @@ -35,9 +35,33 @@ export interface AgentStateSnapshot { priority?: AgentPriority; } +const ALLOWED_TRANSITIONS: Record = { + goal_start: ["prepare_context", "failed"], + prepare_context: [ + "goal_start", + "before_model_request_hooks", + "context_compaction", + "model_request", + "failed", + ], + before_model_request_hooks: ["context_compaction", "failed"], + context_compaction: ["model_request", "failed"], + model_request: ["tool_execution", "final_response", "failed"], + tool_execution: ["apply_tool_results", "failed"], + apply_tool_results: ["prepare_context", "failed"], + final_response: ["goal_complete", "failed"], + goal_complete: [], + failed: ["goal_complete"], +}; + export class AgentStateMachine { private currentState: AgentState = "prepare_context"; private readonly timeline: AgentStateSnapshot[] = []; + private readonly maxTimelineSize: number; + + constructor(maxTimelineSize = 200) { + this.maxTimelineSize = maxTimelineSize; + } transition(input: { state: AgentState; @@ -45,6 +69,14 @@ export class AgentStateMachine { toolCalls: number; note?: string; }): AgentStateSnapshot { + if (input.state !== this.currentState) { + const allowed = ALLOWED_TRANSITIONS[this.currentState]; + if (!allowed.includes(input.state)) { + throw new Error( + `Invalid state transition: ${this.currentState} -> ${input.state}`, + ); + } + } this.currentState = input.state; const context = getHarnessContext(); const snapshot: AgentStateSnapshot = { @@ -59,13 +91,13 @@ export class AgentStateMachine { sessionId: context?.sessionId, goalId: context?.goalId, attemptId: context?.attemptId, - workspaceMode: context?.executionProfile.workspaceMode, - memoryMode: context?.executionProfile.memoryMode, - priority: context?.executionProfile.priority, + workspaceMode: context?.executionProfile?.workspaceMode, + memoryMode: context?.executionProfile?.memoryMode, + priority: context?.executionProfile?.priority, }; this.timeline.push(snapshot); - if (this.timeline.length > 200) { - this.timeline.splice(0, this.timeline.length - 200); + if (this.timeline.length > this.maxTimelineSize) { + this.timeline.splice(0, this.timeline.length - this.maxTimelineSize); } return snapshot; } diff --git a/packages/core/src/tools/runtime.ts b/packages/core/src/tools/runtime.ts index 117b189..bf76902 100644 --- a/packages/core/src/tools/runtime.ts +++ b/packages/core/src/tools/runtime.ts @@ -14,6 +14,7 @@ import type { ToolSpec, } from "@step-cli/protocol"; import { cloneJsonSchema } from "@step-cli/utils/json-schema.js"; +import { normalizeToolArguments } from "@step-cli/utils/json.js"; import { scoreFuzzyMatch } from "@step-cli/utils/search.js"; import { buildCodeModeToolBindings, @@ -456,11 +457,11 @@ export class ToolRuntime implements ToolRuntimeApi { } if (!cachedApproval && decision === "allow-always") { - this.approvedFingerprints.add(fingerprint); - if (this.approvedFingerprints.size > MAX_APPROVAL_FINGERPRINTS) { - this.approvedFingerprints.clear(); - this.approvedFingerprints.add(fingerprint); + if (this.approvedFingerprints.size >= MAX_APPROVAL_FINGERPRINTS) { + const oldest = this.approvedFingerprints.values().next().value; + if (oldest) this.approvedFingerprints.delete(oldest); } + this.approvedFingerprints.add(fingerprint); } } @@ -812,42 +813,10 @@ function createApprovalFingerprint( return `${toolName}:${explicitFingerprint}`; } - const normalizedArgs = normalizeArgsForFingerprint(rawArgs); + const normalizedArgs = normalizeToolArguments(rawArgs); return `${toolName}:${normalizedArgs}`; } -function normalizeArgsForFingerprint(rawArgs: string): string { - try { - const parsed = JSON.parse(rawArgs) as unknown; - return stableStringify(parsed); - } catch { - return rawArgs.replace(/\s+/g, " ").trim(); - } -} - -function stableStringify(value: unknown): string { - return JSON.stringify(sortRecursively(value)); -} - -function sortRecursively(value: unknown): unknown { - if (Array.isArray(value)) { - return value.map((entry) => sortRecursively(entry)); - } - - if (value && typeof value === "object") { - const entries = Object.entries(value as Record).sort( - ([left], [right]) => left.localeCompare(right), - ); - const sorted: Record = {}; - for (const [key, child] of entries) { - sorted[key] = sortRecursively(child); - } - return sorted; - } - - return value; -} - function parseApprovedFingerprints(state: unknown): string[] | null { if (!state || typeof state !== "object") { return null; diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 888fa1c..c3b8fa4 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -500,6 +500,12 @@ export interface AgentRunConfig { maxToolResultCharsInContext: number; modelRequestRetries: number; toolExecutionRetries: number; + /** Base delay in ms for model request retry backoff (default 300). */ + retryDelayBase?: number; + /** Maximum delay in ms for retry backoff (default 2_500). */ + retryDelayMax?: number; + /** Jitter in ms applied to retry delay (default 120). */ + retryDelayJitter?: number; } export interface MemoryStats { diff --git a/packages/utils/src/json.ts b/packages/utils/src/json.ts index a272763..d4a064e 100644 --- a/packages/utils/src/json.ts +++ b/packages/utils/src/json.ts @@ -20,3 +20,33 @@ export function safeParseJson( return fallback; } } + +function sortRecursively(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map((entry) => sortRecursively(entry)); + } + if (value && typeof value === "object") { + const entries = Object.entries(value as Record).sort( + ([left], [right]) => left.localeCompare(right), + ); + const sorted: Record = {}; + for (const [key, child] of entries) { + sorted[key] = sortRecursively(child); + } + return sorted; + } + return value; +} + +export function stableStringify(value: unknown): string { + return JSON.stringify(sortRecursively(value)); +} + +export function normalizeToolArguments(rawArgs: string): string { + try { + const parsed = JSON.parse(rawArgs) as unknown; + return stableStringify(parsed); + } catch { + return rawArgs.replace(/\s+/g, " ").trim(); + } +}