diff --git a/src/coding-agent.ts b/src/coding-agent.ts index 1a88531..5838596 100644 --- a/src/coding-agent.ts +++ b/src/coding-agent.ts @@ -55,6 +55,7 @@ import { type WatchdogConfig, } from "./watchdog"; import { shouldCompact, pickCutoff } from "./compaction-policy"; +import { shouldRunFinalSummary, stripHarnessNotices } from "./final-summary-policy"; import { detectSameToolRepetition } from "./loop-detection"; import { pruneOversizedToolResults } from "./own-loop-prune"; import { nextRetry } from "./overflow-retry"; @@ -855,6 +856,11 @@ export class CodingAgent extends Think { let compactionTriggered = false; let consecutiveNoTextSteps = 0; // Track iterations where the model produces tool calls but no text let exitReason: "natural" | "step-limit" | "budget-limit" | "doom-loop" | "no-text-loop" | "text-loop" | "abort" = "natural"; + // Running total of plain text emitted across iterations. Used after the + // loop to decide whether a forced final-summary turn is needed (only + // when the loop ended on a stuck signal and the model never wrote + // a real conclusion). + let turnText = ""; // ─── Budget thresholds (% of tokenBudget) ─── const WARN_THRESHOLD = 0.70; @@ -1359,6 +1365,7 @@ export class CodingAgent extends Think { const c = chunk as { type?: string; delta?: string; errorText?: string; error?: string }; if (c.type === "text-delta" && c.delta) { iterationText += c.delta; + turnText += c.delta; } else if (c.type === "error") { hasErrorChunk = true; errorText = c.errorText ?? c.error ?? "unknown"; @@ -1540,6 +1547,124 @@ export class CodingAgent extends Think { exitReason = "step-limit"; } + // ─── Final-summary turn (after a stuck-loop exit) ─── + // + // When the loop exits on a stuck signal (doom-loop, no-text-loop, + // text-loop, or budget hard-stop without auto-continuation), + // the model often left only the harness's own + // "[Stopped: ...]" delta in the user-visible response — no real + // conclusion. The auto-continuation block below SKIPS those exits + // by design (they mean the model is stuck and shouldn't be + // restarted). But it leaves users with a stop notice and no + // answer. + // + // Run one more single-turn streamText with NO tools and a + // strict 'write your conclusion now' system message. The model + // has no choice but to emit text. Bounded by a short timeout + // so this can't itself loop. + // + // Guards: + // - skipped on abort (the user asked to stop) + // - skipped on `natural` exit (the model already wrapped up) + // - skipped when the turn already produced substantive text + // (>=200 chars of non-stop-notice content) + // - skipped on cost-runaway exits where there's no point + // spending more tokens + const FINAL_SUMMARY_MIN_EXISTING_TEXT = 200; + const FINAL_SUMMARY_TIMEOUT_MS = 30_000; + const FINAL_SUMMARY_MAX_OUTPUT_TOKENS = 800; + + const turnTextWithoutHarnessNotices = stripHarnessNotices(turnText); + const runFinalSummary = shouldRunFinalSummary({ + exitReason, + signalAborted: !!signal?.aborted, + turnText, + minExistingTextChars: FINAL_SUMMARY_MIN_EXISTING_TEXT, + }); + + if (runFinalSummary) { + log("info", "own-loop: final-summary turn starting", { + sessionId, + exitReason, + existingTextChars: turnTextWithoutHarnessNotices.length, + step, + }); + + // Pure-text system injection appended to the existing messages. + // No tools handed to the model — it cannot make another tool + // call, only write text. This is the whole point. + const summaryInjection: ModelMessage = { + role: "system" as const, + content: [ + "[FINAL TURN — NO TOOLS AVAILABLE]", + "The session has been stopped by the harness because " + + (exitReason === "doom-loop" + ? "you called the same tool too many times in a row" + : exitReason === "no-text-loop" + ? "you made many tool calls without writing any text" + : "your responses started repeating") + + ".", + "Write your final answer to the user now. Summarise:", + " 1. What you were trying to do.", + " 2. What you actually found out (the useful information from your tool calls).", + " 3. What you would have done next if you'd had more turns.", + "Do NOT apologise, do NOT explain that you stopped — the user already knows. Just give the conclusion.", + ].join("\n"), + }; + + const summaryMessages = [...messages, summaryInjection]; + + // Bound this turn with a fresh AbortController chained to the + // outer signal, so a timeout here doesn't leak the outer + // controller. We can't directly time-bound streamText, but + // we can race its iterator against a timer. + const summaryController = new AbortController(); + const onOuterAbort = () => summaryController.abort(); + signal?.addEventListener("abort", onOuterAbort, { once: true }); + const timeoutHandle = setTimeout(() => { + summaryController.abort(); + log("warn", "own-loop: final-summary turn timed out", { + sessionId, + timeoutMs: FINAL_SUMMARY_TIMEOUT_MS, + }); + }, FINAL_SUMMARY_TIMEOUT_MS); + + try { + const summaryResult = streamText({ + model, + system, + messages: summaryMessages, + tools: {}, // No tools — the model must write text. + maxOutputTokens: FINAL_SUMMARY_MAX_OUTPUT_TOKENS, + abortSignal: summaryController.signal, + }); + // Separator so the conclusion is visibly distinct from + // any harness stop notice that came before it. + yield { + type: "text-delta", + id: crypto.randomUUID(), + delta: "\n\n---\n\n", + }; + for await (const chunk of summaryResult.toUIMessageStream()) { + yield chunk; + } + log("info", "own-loop: final-summary turn complete", { + sessionId, + exitReason, + }); + } catch (err) { + log("warn", "own-loop: final-summary turn failed", { + sessionId, + error: err instanceof Error ? err.message : String(err), + }); + // Non-fatal — the stop notice already in `turnText` is the + // user-visible result; we tried for more and failed. + } finally { + clearTimeout(timeoutHandle); + signal?.removeEventListener("abort", onOuterAbort); + } + } + // ─── Multi-phase auto-continuation ─── // When the loop ends due to resource limits (step or budget), truncate // context in-memory and start a new phase. Repeats up to MAX_PHASES diff --git a/src/final-summary-policy.ts b/src/final-summary-policy.ts new file mode 100644 index 0000000..5a7588a --- /dev/null +++ b/src/final-summary-policy.ts @@ -0,0 +1,70 @@ +/** + * Pure decision logic for the own-loop's "final-summary turn" feature. + * + * After the own-loop exits, the harness sometimes wants to run one more + * no-tools turn to elicit a real conclusion from the model — but only + * when: + * - the loop exited because the model was stuck (not because it + * finished naturally, hit a cost backstop, or the user aborted), + * - AND the model didn't already write a substantive text answer. + * + * Extracted from `coding-agent.ts:onChatMessage()` so the boundary + * conditions are testable without booting a Worker. + */ + +export type OwnLoopExitReason = + | "natural" + | "step-limit" + | "budget-limit" + | "doom-loop" + | "no-text-loop" + | "text-loop" + | "abort"; + +/** Exit reasons that mean "the model got stuck — try one more nudge". */ +export const STUCK_EXIT_REASONS: ReadonlySet = new Set([ + "doom-loop", + "no-text-loop", + "text-loop", +]); + +/** + * Strip the harness's own bracketed notices ("[Stopped: ...]", + * "[Compacting context ...]", "[Loop detected ...]") from a string. + * Used when judging whether the model itself produced a real text + * answer — without this scrub, a 50-char stop notice would count as + * "model wrote text" and we'd skip the final-summary turn. + */ +export function stripHarnessNotices(text: string): string { + return text + .replace(/\[Stopped:[^\]]*\]/g, "") + .replace(/\[Compacting context[^\]]*\]/g, "") + .replace(/\[Loop detected[^\]]*\]/g, "") + .trim(); +} + +export interface FinalSummaryDecisionInputs { + /** How the own-loop exited. */ + exitReason: OwnLoopExitReason; + /** Whether the outer abort signal was tripped. */ + signalAborted: boolean; + /** All assistant text emitted across the turn so far. */ + turnText: string; + /** + * If the model already wrote at least this many chars of + * non-harness-notice text, the summary turn is skipped. Defaults + * to 200. + */ + minExistingTextChars?: number; +} + +/** Should the own-loop run a no-tools final-summary turn? */ +export function shouldRunFinalSummary( + inputs: FinalSummaryDecisionInputs, +): boolean { + if (inputs.signalAborted) return false; + if (!STUCK_EXIT_REASONS.has(inputs.exitReason)) return false; + const min = inputs.minExistingTextChars ?? 200; + const stripped = stripHarnessNotices(inputs.turnText); + return stripped.length < min; +} diff --git a/test/final-summary-policy-unit.test.ts b/test/final-summary-policy-unit.test.ts new file mode 100644 index 0000000..82e2612 --- /dev/null +++ b/test/final-summary-policy-unit.test.ts @@ -0,0 +1,187 @@ +/** + * Unit tests for the final-summary turn decision logic. + * + * The behaviour grid: + * + * - abort signal tripped → always false (user asked to stop) + * - exit reason isn't a stuck signal → false (natural finish or + * cost-runaway — no point spending more tokens) + * - exit reason is stuck AND model already wrote enough text → false + * - exit reason is stuck AND model wrote nothing meaningful → true + * - harness's own [Stopped: ...] / [Compacting ...] notices don't + * count as "model wrote text" — they get scrubbed first + */ +import { describe, expect, it } from "vitest"; +import { + shouldRunFinalSummary, + stripHarnessNotices, + STUCK_EXIT_REASONS, +} from "../src/final-summary-policy"; + +describe("stripHarnessNotices", () => { + it("removes [Stopped: ...] notices", () => { + const input = "Hello world.\n\n[Stopped: codemode called 10 times in a row]\n\nDone."; + expect(stripHarnessNotices(input)).toBe("Hello world.\n\n\n\nDone."); + }); + + it("removes [Compacting context ...] markers", () => { + const input = "Step 1.\n[Compacting context and continuing... (phase 2)]\nStep 2."; + expect(stripHarnessNotices(input)).toBe("Step 1.\n\nStep 2."); + }); + + it("removes [Loop detected ...] markers", () => { + const input = "[Loop detected — summarizing progress so far]\nFinal output."; + expect(stripHarnessNotices(input)).toBe("Final output."); + }); + + it("trims whitespace from the result", () => { + expect(stripHarnessNotices("\n\n [Stopped: x]\n\n ")).toBe(""); + }); + + it("returns plain text unchanged", () => { + const input = "I analyzed the codebase and found three bugs."; + expect(stripHarnessNotices(input)).toBe(input); + }); +}); + +describe("STUCK_EXIT_REASONS", () => { + it("contains exactly the loop-detection exit reasons", () => { + expect(STUCK_EXIT_REASONS.has("doom-loop")).toBe(true); + expect(STUCK_EXIT_REASONS.has("no-text-loop")).toBe(true); + expect(STUCK_EXIT_REASONS.has("text-loop")).toBe(true); + // Not stuck — model decided to stop or hit a resource limit. + expect(STUCK_EXIT_REASONS.has("natural")).toBe(false); + expect(STUCK_EXIT_REASONS.has("step-limit")).toBe(false); + expect(STUCK_EXIT_REASONS.has("budget-limit")).toBe(false); + expect(STUCK_EXIT_REASONS.has("abort")).toBe(false); + }); +}); + +describe("shouldRunFinalSummary", () => { + const base = { + exitReason: "doom-loop" as const, + signalAborted: false, + turnText: "", + }; + + it("returns true on a stuck exit when the model wrote nothing", () => { + expect(shouldRunFinalSummary(base)).toBe(true); + }); + + it("returns true when the model only emitted a harness stop notice", () => { + expect( + shouldRunFinalSummary({ + ...base, + turnText: "\n\n[Stopped: codemode called 10 times in a row]\n\n", + }), + ).toBe(true); + }); + + it("returns false when the model wrote a substantive conclusion", () => { + const realAnswer = + "I analyzed src/coding-agent.ts and found the auto-nudge feature " + + "is implemented via the watchdog system. The runWatchdogCheck method " + + "fires on a cron schedule and dispatches a nudge prompt when a session " + + "has been stalled past the configured threshold. I verified the unit " + + "tests cover the decision logic comprehensively."; + expect( + shouldRunFinalSummary({ + ...base, + turnText: realAnswer, + }), + ).toBe(false); + }); + + it("returns false when the abort signal is tripped (user asked to stop)", () => { + expect( + shouldRunFinalSummary({ + ...base, + signalAborted: true, + turnText: "", + }), + ).toBe(false); + }); + + it("returns false on a natural exit (model already wrapped up)", () => { + expect( + shouldRunFinalSummary({ + ...base, + exitReason: "natural", + }), + ).toBe(false); + }); + + it("returns false on a step-limit exit (auto-continuation handles it)", () => { + expect( + shouldRunFinalSummary({ + ...base, + exitReason: "step-limit", + }), + ).toBe(false); + }); + + it("returns false on a budget-limit exit (cost-runaway, no point spending more)", () => { + expect( + shouldRunFinalSummary({ + ...base, + exitReason: "budget-limit", + }), + ).toBe(false); + }); + + it("returns false on an abort exit", () => { + expect( + shouldRunFinalSummary({ + ...base, + exitReason: "abort", + }), + ).toBe(false); + }); + + it("respects a custom minExistingTextChars threshold", () => { + // 50 chars — below the default 200 threshold, above a custom 20. + const shortAnswer = "Done. Found the bug in line 42. Easy fix."; + expect( + shouldRunFinalSummary({ + ...base, + turnText: shortAnswer, + minExistingTextChars: 200, + }), + ).toBe(true); + expect( + shouldRunFinalSummary({ + ...base, + turnText: shortAnswer, + minExistingTextChars: 20, + }), + ).toBe(false); + }); + + it("scrubs harness notices BEFORE measuring length", () => { + // The harness stop notice is ~60 chars; the real text is ~30. + // With default 200 minimum, both should fail to clear. We're + // confirming the scrubber runs (i.e. the notice doesn't get + // counted). + const mixed = + "Found it.\n\n[Stopped: codemode called 10 times in a row without producing a text answer — write your conclusion from what you have so far.]"; + const stripped = stripHarnessNotices(mixed); + expect(stripped.length).toBeLessThan(mixed.length); + // With min=20, stripped="Found it." (9 chars) is still below + // threshold → true. + expect( + shouldRunFinalSummary({ + ...base, + turnText: mixed, + minExistingTextChars: 20, + }), + ).toBe(true); + // With min=5, stripped clears → false. + expect( + shouldRunFinalSummary({ + ...base, + turnText: mixed, + minExistingTextChars: 5, + }), + ).toBe(false); + }); +});