From 9efcf1123f23274925d710d38dd87b82ddebb529 Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Sat, 26 Sep 2026 15:51:57 -0400 Subject: [PATCH 1/8] feat(coding-agent): persist plan step identity - Keeps step IDs and success criteria across updates, resume, and compaction - Appends current plan state to every model request after tool execution --- .../coding-agent/src/core/agent-session.ts | 75 ++------ .../src/core/current-plan-state.ts | 15 +- .../src/core/tools/update-plan.ts | 161 +++++++++++++++--- .../test/current-plan-state.test.ts | 42 +++-- ...t-session-current-plan-restoration.test.ts | 71 ++++---- .../test/suite/agent-session-prompt.test.ts | 4 +- .../suite/agent-session-update-plan.test.ts | 126 +++++++++++--- .../coding-agent/test/system-prompt.test.ts | 2 +- .../tool-system-prompt-contributions.test.ts | 4 +- .../test/update-plan-tool.test.ts | 147 +++++++++++++--- 10 files changed, 460 insertions(+), 187 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 667307dd1..9996e8775 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -159,6 +159,7 @@ import { type BashOperations, createLocalBashOperations } from "./tools/bash.ts" import { createAllToolDefinitions } from "./tools/index.ts"; import { createRequestUserInputToolDefinition, REQUEST_USER_INPUT_TOOL_NAME } from "./tools/request-user-input.ts"; import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.ts"; +import type { UpdatePlanItem } from "./tools/update-plan.ts"; import { addUsageToTotals, createUsageTotals } from "./usage-totals.ts"; // ============================================================================ @@ -459,9 +460,7 @@ function estimateMessagesTokens(messages: AgentMessage[]): number { /** Standard thinking levels */ const THINKING_LEVELS: ThinkingLevel[] = ["off", "minimal", "low", "medium", "high"]; -const CURRENT_PLAN_RESTORATION_CUSTOM_TYPE = "diffex.current-plan-restoration"; -const CURRENT_PLAN_RESTORATION_REMINDER = - "Reminder: restored context is not proof of completion. Verify current state before claiming work is done."; +const CURRENT_TASK_CONTEXT_CUSTOM_TYPE = "diffex.current-task-context"; const MAX_PENDING_SUBAGENT_ESCALATION_CHARACTERS = 8_000; const INCOMPATIBLE_SUBAGENT_PARENT_HOOKS = [ "resources_discover", @@ -501,7 +500,6 @@ export class AgentSession { private _followUpMessages: string[] = []; /** Messages queued to be included with the next user prompt as context ("asides"). */ private _pendingNextTurnMessages: CustomMessage[] = []; - private _pendingCurrentPlanRestoration = false; private readonly _skillReferenceSnapshots = new Map(); private readonly _skillEnablementOverrides = new Map(); private _activeSkillTrace: ActiveSkillTrace | undefined; @@ -613,13 +611,11 @@ export class AgentSession { this._telemetryContext = config.telemetryContext ?? NOOP_TELEMETRY_CONTEXT; this._lifecycleObserver = config.lifecycleObserver; - this._armInitialCurrentPlanRestoration(); - // Always subscribe to agent events for internal handling // (session persistence, extensions, auto-compaction, retry logic) this._unsubscribeAgent = this.agent.subscribe(this._handleAgentEvent); this._installAgentToolHooks(); - this._installAgentContextRestoration(); + this._installCurrentTaskContext(); this._installAgentNextTurnRefresh(); this._buildRuntime({ @@ -843,60 +839,27 @@ export class AgentSession { }; } - private _installAgentContextRestoration(): void { + private _installCurrentTaskContext(): void { const previousTransformContext = this.agent.transformContext; this.agent.transformContext = async (messages, signal) => { const transformedMessages = previousTransformContext ? await previousTransformContext(messages, signal) : messages; - return this._consumePendingCurrentPlanRestoration(transformedMessages); - }; - } - - private _armInitialCurrentPlanRestoration(): void { - if (this._sessionStartEvent.reason !== "startup" && this._sessionStartEvent.reason !== "resume") { - return; - } - this._armCurrentPlanRestorationIfActive(); - } - - private _armCurrentPlanRestorationIfActive(): void { - if (resolveCurrentPlanState(this.sessionManager).status === "active") { - this._pendingCurrentPlanRestoration = true; - } - } - - private _consumePendingCurrentPlanRestoration(messages: AgentMessage[]): AgentMessage[] { - if (!this._pendingCurrentPlanRestoration) { - return messages; - } - - const state = resolveCurrentPlanState(this.sessionManager); - this._pendingCurrentPlanRestoration = false; - if (state.status !== "active") { - return messages; - } - - const lines = [ - "Restored unfinished update_plan checklist from the current session. Use this hidden context to continue the work.", - ]; - if (state.operation.explanation !== undefined) { - lines.push("", "Explanation:", state.operation.explanation); - } - lines.push("", "Checklist:"); - for (const [index, item] of state.operation.plan.entries()) { - lines.push(`${index + 1}. [${item.status}] ${item.step}`); - } - lines.push("", CURRENT_PLAN_RESTORATION_REMINDER); - - const restoredContext: CustomMessage = { - role: "custom", - customType: CURRENT_PLAN_RESTORATION_CUSTOM_TYPE, - content: [{ type: "text", text: lines.join("\n") }], - display: false, - timestamp: Date.now(), + const state = resolveCurrentPlanState(this.sessionManager); + let plan: UpdatePlanItem[] = []; + if (state.status === "active") plan = state.operation.plan; + else if (state.status === "inactive" && "plan" in state.operation) plan = state.operation.plan; + const currentTaskContext: CustomMessage = { + role: "custom", + customType: CURRENT_TASK_CONTEXT_CUSTOM_TYPE, + content: [ + { type: "text", text: `Current task context (latest plan):\n${JSON.stringify({ plan }, null, 2)}` }, + ], + display: false, + timestamp: Date.now(), + }; + return [...transformedMessages, currentTaskContext]; }; - return [...messages, restoredContext]; } // ========================================================================= @@ -2378,7 +2341,6 @@ export class AgentSession { if (!pendingCompaction) return; pendingCompaction.commit(); pendingCompaction = undefined; - this._armCurrentPlanRestorationIfActive(); }; const rollbackPendingCompaction = (): void => { if (!pendingCompaction) return; @@ -3548,7 +3510,6 @@ export class AgentSession { usage: options.usage, details: options.details, }; - if (!options.deferCommit) this._armCurrentPlanRestorationIfActive(); return compactionResult; } diff --git a/packages/coding-agent/src/core/current-plan-state.ts b/packages/coding-agent/src/core/current-plan-state.ts index ccae4c4ca..7021d7fed 100644 --- a/packages/coding-agent/src/core/current-plan-state.ts +++ b/packages/coding-agent/src/core/current-plan-state.ts @@ -1,7 +1,7 @@ import type { SessionManager } from "./session-manager.ts"; import { createUpdatePlanToolDefinition, - parseUpdatePlanOperation, + resolveUpdatePlanOperation, type UpdatePlanToolDetails, } from "./tools/update-plan.ts"; @@ -17,7 +17,7 @@ export function appendCurrentPlanRecord( sessionManager: Pick, operation: UpdatePlanToolDetails, ): string { - return sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, parseUpdatePlanOperation(operation)); + return sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, resolveUpdatePlanOperation(operation)); } export function resolveCurrentPlanState(sessionManager: Pick): CurrentPlanState { @@ -30,7 +30,7 @@ export function resolveCurrentPlanState(sessionManager: Pick, + sessionManager: Pick, ): ReturnType { - const definition = createUpdatePlanToolDefinition(); + const definition = createUpdatePlanToolDefinition(undefined, () => { + const state = resolveCurrentPlanState(sessionManager); + if (state.status === "active") return state.operation.plan; + if (state.status === "inactive" && "plan" in state.operation) return state.operation.plan; + return undefined; + }); const execute = definition.execute; return { ...definition, diff --git a/packages/coding-agent/src/core/tools/update-plan.ts b/packages/coding-agent/src/core/tools/update-plan.ts index a14f55fd5..f7ef1eebe 100644 --- a/packages/coding-agent/src/core/tools/update-plan.ts +++ b/packages/coding-agent/src/core/tools/update-plan.ts @@ -11,8 +11,10 @@ const UPDATE_PLAN_STATUSES = ["pending", "in_progress", "completed"] as const; export type UpdatePlanStatus = (typeof UPDATE_PLAN_STATUSES)[number]; export interface UpdatePlanItem { + id: string; step: string; status: UpdatePlanStatus; + success_criteria?: string[]; } export type UpdatePlanToolInput = @@ -20,12 +22,16 @@ export type UpdatePlanToolInput = plan: UpdatePlanItem[]; explanation?: string; } + | { + update: { id: string; step?: string; status?: UpdatePlanStatus; success_criteria?: string[] }; + explanation?: string; + } | { clear: true; explanation?: string; }; -export type UpdatePlanToolDetails = UpdatePlanToolInput; +export type UpdatePlanToolDetails = Exclude; const updatePlanStatusSchema = Type.Unsafe({ type: "string", @@ -35,8 +41,10 @@ const updatePlanStatusSchema = Type.Unsafe({ const updatePlanItemSchema = Type.Object( { + id: Type.String({ description: "Stable step ID; retain it across edits and reordering." }), step: Type.String({ description: "Task step text." }), status: updatePlanStatusSchema, + success_criteria: Type.Optional(Type.Array(Type.String(), { description: "Observable completion criteria." })), }, { additionalProperties: false }, ); @@ -46,12 +54,23 @@ const updatePlanSchema = Type.Object( plan: Type.Optional( Type.Array(updatePlanItemSchema, { minItems: 1, - description: "Full ordered plan snapshot. Provide exactly one operation: plan or clear.", + description: "Full ordered plan snapshot. Keep existing IDs for retained steps.", }), ), + update: Type.Optional( + Type.Object( + { + id: Type.String({ description: "ID of an existing step." }), + step: Type.Optional(Type.String()), + status: Type.Optional(updatePlanStatusSchema), + success_criteria: Type.Optional(Type.Array(Type.String())), + }, + { additionalProperties: false }, + ), + ), clear: Type.Optional( Type.Literal(true, { - description: "Explicitly abandon and clear the current plan. Provide exactly one operation: plan or clear.", + description: "Explicitly abandon and clear the current plan.", }), ), explanation: Type.Optional(Type.String({ description: "Reason for changing or clearing the plan." })), @@ -60,12 +79,12 @@ const updatePlanSchema = Type.Object( ); export const updatePlanToolSystemPromptContribution = { - snippet: "track progress with an ordered task checklist", + snippet: "persist an ordered plan with step IDs and optional success criteria", guidelines: [ "Use update_plan for complex, ambiguous, dependent, long-running, or multi-part work, when the user requests a plan, or when checkpoints would improve collaboration; skip it for simple or immediately executable work.", - "Write short, ordered, outcome-focused steps with observable evidence where useful; do not narrate commands or pad the checklist with obvious actions.", + "Write short, ordered, outcome-focused steps. Give each step a stable ID and add success_criteria for subtasks needing validation.", "Include final verification when correctness requires it.", - "Update the checklist at meaningful milestones and mark completed work before moving on.", + "Update by ID at meaningful milestones; retain IDs when rewording, reordering, or changing status.", "Explain material replanning, omit explanations for routine progress, and use clear: true only when abandoning the checklist.", "Publishing the checklist communicates progress but does not execute, schedule, or prove completion of its steps.", ], @@ -79,23 +98,43 @@ function isUpdatePlanStatus(value: unknown): value is UpdatePlanStatus { return typeof value === "string" && (UPDATE_PLAN_STATUSES as readonly string[]).includes(value); } +function validateCriteria(value: unknown, label: string): string[] { + if ( + !Array.isArray(value) || + value.some((criterion) => typeof criterion !== "string" || criterion.trim().length === 0) + ) { + throw new Error(`${label} must be an array of non-empty strings.`); + } + return [...value]; +} + function validatePlanSnapshot(rawPlan: unknown): UpdatePlanItem[] { if (!Array.isArray(rawPlan) || rawPlan.length === 0) { throw new Error("Plan must contain at least one item."); } const plan: UpdatePlanItem[] = []; + const ids = new Set(); for (let index = 0; index < rawPlan.length; index++) { const rawItem = rawPlan[index]; if (!isRecord(rawItem)) { throw new Error(`Plan item ${index + 1} must be an object.`); } - const unexpectedField = Object.keys(rawItem).find((field) => field !== "step" && field !== "status"); + const unexpectedField = Object.keys(rawItem).find( + (field) => field !== "id" && field !== "step" && field !== "status" && field !== "success_criteria", + ); if (unexpectedField !== undefined) { throw new Error(`Plan item ${index + 1} contains unexpected field: ${unexpectedField}.`); } - const { step, status } = rawItem; + const { id, step, status } = rawItem; + if (typeof id !== "string" || id.trim().length === 0) { + throw new Error(`Plan item ${index + 1} id must be a non-empty string.`); + } + if (ids.has(id)) { + throw new Error(`Duplicate plan item id: ${id}.`); + } + ids.add(id); if (typeof step !== "string" || step.trim().length === 0) { throw new Error(`Plan item ${index + 1} step must be a non-empty string.`); } @@ -103,7 +142,19 @@ function validatePlanSnapshot(rawPlan: unknown): UpdatePlanItem[] { throw new Error(`Plan item ${index + 1} status must be pending, in_progress, or completed.`); } - plan.push({ step, status }); + plan.push({ + id, + step, + status, + ...(Object.hasOwn(rawItem, "success_criteria") + ? { + success_criteria: validateCriteria( + rawItem.success_criteria, + `Plan item ${index + 1} success_criteria`, + ), + } + : {}), + }); } const counts = countPlanStatuses(plan); @@ -114,22 +165,23 @@ function validatePlanSnapshot(rawPlan: unknown): UpdatePlanItem[] { return plan; } -export function parseUpdatePlanOperation(input: unknown): UpdatePlanToolDetails { +export function parseUpdatePlanOperation(input: unknown): UpdatePlanToolInput { if (!isRecord(input)) { throw new Error("update_plan input must be an object."); } const unexpectedField = Object.keys(input).find( - (field) => field !== "plan" && field !== "clear" && field !== "explanation", + (field) => field !== "plan" && field !== "update" && field !== "clear" && field !== "explanation", ); if (unexpectedField !== undefined) { throw new Error(`Unexpected update_plan field: ${unexpectedField}.`); } const hasPlan = Object.hasOwn(input, "plan"); + const hasUpdate = Object.hasOwn(input, "update"); const hasClear = Object.hasOwn(input, "clear"); - if (hasPlan === hasClear) { - throw new Error("Provide exactly one update_plan operation: plan or clear."); + if (Number(hasPlan) + Number(hasUpdate) + Number(hasClear) !== 1) { + throw new Error("Provide exactly one update_plan operation: plan, update, or clear."); } const hasExplanation = Object.hasOwn(input, "explanation"); @@ -144,11 +196,63 @@ export function parseUpdatePlanOperation(input: unknown): UpdatePlanToolDetails } return hasExplanation ? { clear: true, explanation } : { clear: true }; } + if (hasUpdate) { + if (!isRecord(input.update)) throw new Error("Update must be an object."); + const unexpectedUpdateField = Object.keys(input.update).find( + (field) => field !== "id" && field !== "step" && field !== "status" && field !== "success_criteria", + ); + if (unexpectedUpdateField !== undefined) throw new Error(`Unexpected update field: ${unexpectedUpdateField}.`); + const { id, step, status } = input.update; + if (typeof id !== "string" || id.trim().length === 0) throw new Error("Update id must be a non-empty string."); + if (step !== undefined && (typeof step !== "string" || step.trim().length === 0)) { + throw new Error("Update step must be a non-empty string."); + } + if (status !== undefined && !isUpdatePlanStatus(status)) { + throw new Error("Update status must be pending, in_progress, or completed."); + } + const hasCriteria = Object.hasOwn(input.update, "success_criteria"); + if (step === undefined && status === undefined && !hasCriteria) + throw new Error("Update must change a step field."); + const update = { + id, + ...(step === undefined ? {} : { step }), + ...(status === undefined ? {} : { status }), + ...(hasCriteria + ? { success_criteria: validateCriteria(input.update.success_criteria, "Update success_criteria") } + : {}), + }; + return hasExplanation ? { update, explanation } : { update }; + } const plan = validatePlanSnapshot(input.plan); return hasExplanation ? { plan, explanation } : { plan }; } +export function resolveUpdatePlanOperation( + input: unknown, + currentPlan?: readonly UpdatePlanItem[], +): UpdatePlanToolDetails { + const operation = parseUpdatePlanOperation(input); + if (!("update" in operation)) return operation; + if (!currentPlan) throw new Error("Cannot update a step without a current plan."); + const index = currentPlan.findIndex((item) => item.id === operation.update.id); + if (index < 0) throw new Error(`Unknown plan step id: ${operation.update.id}.`); + const plan = currentPlan.map((item, itemIndex) => { + if (itemIndex !== index) return item; + const { success_criteria, ...changes } = operation.update; + const updated = { ...item, ...changes }; + if (success_criteria !== undefined) { + if (success_criteria.length === 0) delete updated.success_criteria; + else updated.success_criteria = success_criteria; + } + return updated; + }); + return { + plan: validatePlanSnapshot(plan), + ...(operation.explanation === undefined ? {} : { explanation: operation.explanation }), + }; +} + function countPlanStatuses(plan: readonly UpdatePlanItem[]): { completed: number; inProgress: number; @@ -200,7 +304,7 @@ function formatPlanStep(step: unknown, theme: Theme): string { function formatPlanCall(args: unknown, theme: Theme, showInvalidArgs: boolean): string { let text = theme.fg("toolTitle", theme.bold("update_plan")); - let details: UpdatePlanToolDetails; + let details: UpdatePlanToolInput; try { details = parseUpdatePlanOperation(args); } catch { @@ -213,25 +317,38 @@ function formatPlanCall(args: unknown, theme: Theme, showInvalidArgs: boolean): if ("clear" in details) { return `${text}\n${theme.fg("toolOutput", "Plan cleared.")}`; } + if ("update" in details) { + text += `\n${theme.fg("toolOutput", `Update ${details.update.id}`)}`; + if (details.update.status) text += ` ${formatPlanMarker(details.update.status, theme)}`; + if (details.update.step) text += ` ${formatPlanStep(details.update.step, theme)}`; + if (details.update.success_criteria) { + for (const criterion of details.update.success_criteria) + text += `\n ${theme.fg("toolOutput", `- ${criterion}`)}`; + } + return text; + } for (const item of details.plan) { - text += `\n${formatPlanMarker(item.status, theme)} ${formatPlanStep(item.step, theme)}`; + text += `\n${formatPlanMarker(item.status, theme)} ${theme.fg("toolOutput", item.id)} ${formatPlanStep(item.step, theme)}`; + for (const criterion of item.success_criteria ?? []) text += `\n ${theme.fg("toolOutput", `- ${criterion}`)}`; } return text; } -export function createUpdatePlanToolDefinition(): ToolDefinition { +export function createUpdatePlanToolDefinition( + _cwd?: string, + getCurrentPlan?: () => readonly UpdatePlanItem[] | undefined, +): ToolDefinition { return { name: "update_plan", label: "update_plan", - description: - "Replace or explicitly clear the full ordered task checklist. Unfinished plans require exactly one in_progress item.", + description: "Write or clear a plan, or update one step by ID. Use optional success_criteria for validation.", promptSnippet: updatePlanToolSystemPromptContribution.snippet, promptGuidelines: [...updatePlanToolSystemPromptContribution.guidelines], parameters: updatePlanSchema, executionMode: "sequential", async execute(_toolCallId, input) { - const details = parseUpdatePlanOperation(input); + const details = resolveUpdatePlanOperation(input, getCurrentPlan?.()); return { content: [ { @@ -258,6 +375,8 @@ export function createUpdatePlanToolDefinition(): ToolDefinition { - return wrapToolDefinition(createUpdatePlanToolDefinition()); +export function createUpdatePlanTool( + getCurrentPlan?: () => readonly UpdatePlanItem[] | undefined, +): AgentTool { + return wrapToolDefinition(createUpdatePlanToolDefinition(undefined, getCurrentPlan)); } diff --git a/packages/coding-agent/test/current-plan-state.test.ts b/packages/coding-agent/test/current-plan-state.test.ts index a6d533196..c9a376fb7 100644 --- a/packages/coding-agent/test/current-plan-state.test.ts +++ b/packages/coding-agent/test/current-plan-state.test.ts @@ -25,8 +25,8 @@ describe("current plan state", () => { const sessionManager = SessionManager.inMemory(); const operation = { plan: [ - { step: " Preserve exact whitespace ", status: "in_progress" as const }, - { step: "Finish", status: "pending" as const }, + { id: "preserve-exact-whitespace", step: " Preserve exact whitespace ", status: "in_progress" as const }, + { id: "finish", step: "Finish", status: "pending" as const }, ], explanation: " exact explanation ", }; @@ -41,8 +41,8 @@ describe("current plan state", () => { customType: CURRENT_PLAN_CUSTOM_ENTRY_TYPE, data: { plan: [ - { step: " Preserve exact whitespace ", status: "in_progress" }, - { step: "Finish", status: "pending" }, + { id: "preserve-exact-whitespace", step: " Preserve exact whitespace ", status: "in_progress" }, + { id: "finish", step: "Finish", status: "pending" }, ], explanation: " exact explanation ", }, @@ -53,8 +53,8 @@ describe("current plan state", () => { status: "active", operation: { plan: [ - { step: " Preserve exact whitespace ", status: "in_progress" }, - { step: "Finish", status: "pending" }, + { id: "preserve-exact-whitespace", step: " Preserve exact whitespace ", status: "in_progress" }, + { id: "finish", step: "Finish", status: "pending" }, ], explanation: " exact explanation ", }, @@ -71,11 +71,11 @@ describe("current plan state", () => { it("uses only the newest record on the selected branch", () => { const sessionManager = SessionManager.inMemory(); const rootId = appendCurrentPlanRecord(sessionManager, { - plan: [{ step: "Root plan", status: "in_progress" }], + plan: [{ id: "root-plan", step: "Root plan", status: "in_progress" }], explanation: "root", }); const firstBranchId = appendCurrentPlanRecord(sessionManager, { - plan: [{ step: "First branch", status: "in_progress" }], + plan: [{ id: "first-branch", step: "First branch", status: "in_progress" }], }); sessionManager.branch(rootId); @@ -88,7 +88,7 @@ describe("current plan state", () => { sessionManager.branch(firstBranchId); expect(resolveCurrentPlanState(sessionManager)).toEqual({ status: "active", - operation: { plan: [{ step: "First branch", status: "in_progress" }] }, + operation: { plan: [{ id: "first-branch", step: "First branch", status: "in_progress" }] }, }); }); @@ -101,8 +101,8 @@ describe("current plan state", () => { sessionManager.appendMessage(assistantMsg("working")); appendCurrentPlanRecord(sessionManager, { plan: [ - { step: "Done", status: "completed" }, - { step: "Current", status: "in_progress" }, + { id: "done", step: "Done", status: "completed" }, + { id: "current", step: "Current", status: "in_progress", success_criteria: ["Survives reopen"] }, ], explanation: " reopened exactly ", }); @@ -111,6 +111,16 @@ describe("current plan state", () => { expect(sessionFile).toBeDefined(); const reopened = SessionManager.open(sessionFile!, tempDir); expect(resolveCurrentPlanState(reopened)).toEqual(resolveCurrentPlanState(sessionManager)); + const reopenedState = resolveCurrentPlanState(reopened); + expect(reopenedState.status).toBe("active"); + if (reopenedState.status === "active") { + expect(reopenedState.operation.plan[1]).toEqual({ + id: "current", + step: "Current", + status: "in_progress", + success_criteria: ["Survives reopen"], + }); + } }); it("classifies completed, cleared, absent, and corrupt newest states", () => { @@ -118,13 +128,13 @@ describe("current plan state", () => { expect(resolveCurrentPlanState(sessionManager)).toEqual({ status: "absent" }); appendCurrentPlanRecord(sessionManager, { - plan: [{ step: "Finished", status: "completed" }], + plan: [{ id: "finished", step: "Finished", status: "completed" }], explanation: "complete", }); expect(resolveCurrentPlanState(sessionManager)).toEqual({ status: "inactive", operation: { - plan: [{ step: "Finished", status: "completed" }], + plan: [{ id: "finished", step: "Finished", status: "completed" }], explanation: "complete", }, }); @@ -136,10 +146,10 @@ describe("current plan state", () => { }); appendCurrentPlanRecord(sessionManager, { - plan: [{ step: "Older active plan", status: "in_progress" }], + plan: [{ id: "older-active-plan", step: "Older active plan", status: "in_progress" }], }); sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, { - plan: [{ step: "Malformed newest", status: "pending" }], + plan: [{ id: "malformed-newest", step: "Malformed newest", status: "pending" }], }); expect(resolveCurrentPlanState(sessionManager)).toEqual({ status: "corrupt" }); }); @@ -148,7 +158,7 @@ describe("current plan state", () => { const sessionManager = SessionManager.inMemory(); const tool = createUpdatePlanTool(); await tool.execute("standalone", { - plan: [{ step: "Standalone", status: "in_progress" }], + plan: [{ id: "standalone", step: "Standalone", status: "in_progress" }], }); expect(sessionManager.getBranch()).toEqual([]); }); diff --git a/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts b/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts index 668e398f0..d06f46f03 100644 --- a/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts +++ b/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts @@ -16,16 +16,19 @@ type SessionWithCompactionInternals = { _runAutoCompaction: (reason: "overflow" | "threshold", willRetry: boolean) => Promise; }; -const RESTORED_CONTEXT_HEADER = "Restored unfinished update_plan checklist from the current session."; -const RESTORED_CONTEXT_REMINDER = - "Reminder: restored context is not proof of completion. Verify current state before claiming work is done."; +const CURRENT_TASK_CONTEXT_HEADER = "Current task context (latest plan):"; function activeOperation(): PlanUpdateDetails { return { plan: [ - { step: "Read existing state", status: "completed" }, - { step: " Implement exact restore ", status: "in_progress" }, - { step: "Verify once", status: "pending" }, + { id: "read-existing-state", step: "Read existing state", status: "completed" }, + { + id: "implement-exact-restore", + step: " Implement exact restore ", + status: "in_progress", + success_criteria: ["Plan survives resume", "Criteria survive compaction"], + }, + { id: "verify-once", step: "Verify once", status: "pending" }, ], explanation: " keep this exact explanation ", }; @@ -33,23 +36,12 @@ function activeOperation(): PlanUpdateDetails { function operationWithoutExplanation(): PlanUpdateDetails { return { - plan: [{ step: "Continue unfinished work", status: "in_progress" }], + plan: [{ id: "continue-unfinished-work", step: "Continue unfinished work", status: "in_progress" }], }; } function expectedRestoredText(operation: PlanUpdateDetails): string { - const lines = [ - "Restored unfinished update_plan checklist from the current session. Use this hidden context to continue the work.", - ]; - if (operation.explanation !== undefined) { - lines.push("", "Explanation:", operation.explanation); - } - lines.push("", "Checklist:"); - for (const [index, item] of operation.plan.entries()) { - lines.push(`${index + 1}. [${item.status}] ${item.step}`); - } - lines.push("", RESTORED_CONTEXT_REMINDER); - return lines.join("\n"); + return `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: operation.plan }, null, 2)}`; } function createPlanSessionManager(operation: PlanUpdateDetails = activeOperation()): SessionManager { @@ -75,11 +67,11 @@ function messageText(message: Message): string { } function findRestoredText(messages: Message[]): string | undefined { - return messages.map(messageText).find((text) => text.includes(RESTORED_CONTEXT_HEADER)); + return messages.map(messageText).find((text) => text.includes(CURRENT_TASK_CONTEXT_HEADER)); } function findRestoredContextText(texts: string[]): string | undefined { - return texts.find((text) => text.includes(RESTORED_CONTEXT_HEADER)); + return texts.find((text) => text.includes(CURRENT_TASK_CONTEXT_HEADER)); } async function submitAndCaptureContextTexts(harness: Harness, prompt = "continue"): Promise { @@ -132,7 +124,7 @@ describe("AgentSession current-plan restoration", () => { } }); - it("injects exact active plan context once after opening or resuming an existing session", async () => { + it("appends exact current-task context on every request after opening or resuming", async () => { const operation = activeOperation(); const extensionContextMessage: AgentMessage = { role: "custom", @@ -154,11 +146,12 @@ describe("AgentSession current-plan restoration", () => { const firstContextTexts = await submitAndCaptureContextTexts(opened, "first submission"); expect(findRestoredContextText(firstContextTexts)).toBe(expectedRestoredText(operation)); expect(firstContextTexts.indexOf("extension transformed context")).toBeLessThan( - firstContextTexts.findIndex((text) => text.includes(RESTORED_CONTEXT_HEADER)), + firstContextTexts.findIndex((text) => text.includes(CURRENT_TASK_CONTEXT_HEADER)), ); + expect(firstContextTexts.at(-1)).toBe(expectedRestoredText(operation)); const secondContextTexts = await submitAndCaptureContextTexts(opened, "second submission"); - expect(findRestoredContextText(secondContextTexts)).toBeUndefined(); + expect(secondContextTexts.at(-1)).toBe(expectedRestoredText(operation)); expect(opened.session.messages.some((message) => message.role === "custom")).toBe(false); expect(opened.sessionManager.getEntries().filter((entry) => entry.type === "custom_message")).toHaveLength(0); @@ -179,7 +172,7 @@ describe("AgentSession current-plan restoration", () => { expect(findRestoredContextText(resumedContextTexts)).toBe(expectedRestoredText(operation)); }); - it("does not arm restoration for new sessions or forks", async () => { + it("includes current-task context for new sessions and forks", async () => { for (const reason of ["new", "fork"] as const) { const startEvent: SessionStartEvent = { type: "session_start", @@ -193,11 +186,11 @@ describe("AgentSession current-plan restoration", () => { harnesses.push(harness); const contextTexts = await submitAndCaptureContextTexts(harness, `${reason} submission`); - expect(findRestoredContextText(contextTexts)).toBeUndefined(); + expect(contextTexts.at(-1)).toBe(expectedRestoredText(activeOperation())); } }); - it("arms through successful manual and auto compaction completion but not cancelled compaction", async () => { + it("keeps current-task context after manual, auto, and cancelled compaction", async () => { const manualOperation = operationWithoutExplanation(); const manual = await createHarness({ extensionFactories: [ @@ -222,7 +215,7 @@ describe("AgentSession current-plan restoration", () => { const manualContextTexts = await submitAndCaptureContextTexts(manual, "after manual compaction"); const restoredManual = findRestoredContextText(manualContextTexts); expect(restoredManual).toBe(expectedRestoredText(manualOperation)); - expect(restoredManual).not.toContain("Explanation:"); + expect(restoredManual).not.toContain("explanation"); const autoOperation = operationWithoutExplanation(); const auto = await createHarness({ @@ -262,10 +255,10 @@ describe("AgentSession current-plan restoration", () => { await expect(cancelled.session.compact()).rejects.toThrow("Compaction cancelled"); const cancelledContextTexts = await submitAndCaptureContextTexts(cancelled, "after cancelled compaction"); - expect(findRestoredContextText(cancelledContextTexts)).toBeUndefined(); + expect(findRestoredContextText(cancelledContextTexts)).toBe(expectedRestoredText(operationWithoutExplanation())); }); - it("preserves pending restoration through preflight paths and consumes it on model failure", async () => { + it("includes current-task context after preflight paths and model failure", async () => { let handleInput = true; const handled = await createHarness({ sessionManager: createPlanSessionManager(), @@ -337,16 +330,18 @@ describe("AgentSession current-plan restoration", () => { } const afterFailureTexts = await submitAndCaptureContextTexts(modelFailure, "after model failure"); - expect(findRestoredContextText(afterFailureTexts)).toBeUndefined(); + expect(findRestoredContextText(afterFailureTexts)).toBe(expectedRestoredText(activeOperation())); }); - it("consumes without stale injection when newest state becomes inactive, absent, or corrupt", async () => { + it("uses the newest state when a plan is cleared, absent, or corrupt", async () => { const inactive = await createHarness({ sessionManager: createPlanSessionManager() }); harnesses.push(inactive); appendCurrentPlanRecord(inactive.sessionManager, { clear: true }); const inactiveTexts = await submitAndCaptureContextTexts(inactive, "after clear"); - expect(findRestoredContextText(inactiveTexts)).toBeUndefined(); + expect(findRestoredContextText(inactiveTexts)).toBe( + `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: [] }, null, 2)}`, + ); expect(resolveCurrentPlanState(inactive.sessionManager).status).toBe("inactive"); const absent = await createHarness({ sessionManager: createPlanSessionManager() }); @@ -354,16 +349,20 @@ describe("AgentSession current-plan restoration", () => { absent.sessionManager.resetLeaf(); const absentTexts = await submitAndCaptureContextTexts(absent, "after branch reset"); - expect(findRestoredContextText(absentTexts)).toBeUndefined(); + expect(findRestoredContextText(absentTexts)).toBe( + `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: [] }, null, 2)}`, + ); const corrupt = await createHarness({ sessionManager: createPlanSessionManager() }); harnesses.push(corrupt); corrupt.sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, { - plan: [{ step: "Do not revive older valid state", status: "pending" }], + plan: [{ id: "do-not-revive-older-valid-state", step: "Do not revive older valid state", status: "pending" }], }); const corruptTexts = await submitAndCaptureContextTexts(corrupt, "after corrupt newest"); - expect(findRestoredContextText(corruptTexts)).toBeUndefined(); + expect(findRestoredContextText(corruptTexts)).toBe( + `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: [] }, null, 2)}`, + ); expect(resolveCurrentPlanState(corrupt.sessionManager).status).toBe("corrupt"); }); diff --git a/packages/coding-agent/test/suite/agent-session-prompt.test.ts b/packages/coding-agent/test/suite/agent-session-prompt.test.ts index 8c5ed1c40..10d6bf95e 100644 --- a/packages/coding-agent/test/suite/agent-session-prompt.test.ts +++ b/packages/coding-agent/test/suite/agent-session-prompt.test.ts @@ -1022,7 +1022,7 @@ describe("AgentSession prompt characterization", () => { const model = harness.getModel(); harness.sessionManager.appendMessage({ role: "user", content: "previous", timestamp: now - 1000 }); harness.sessionManager.appendCustomEntry("diffex.current-plan", { - plan: [{ step: "continue", status: "in_progress" }], + plan: [{ id: "continue", step: "continue", status: "in_progress" }], }); harness.sessionManager.appendMessage({ ...fauxAssistantMessage("previous response", { stopReason: "length", timestamp: now - 500 }), @@ -1050,7 +1050,7 @@ describe("AgentSession prompt characterization", () => { harness.setResponses([ (context) => { restoredPlan = context.messages.some((message) => - getMessageText(message).includes("Restored unfinished update_plan checklist"), + getMessageText(message).includes("Current task context (latest plan):"), ); return fauxAssistantMessage("done"); }, diff --git a/packages/coding-agent/test/suite/agent-session-update-plan.test.ts b/packages/coding-agent/test/suite/agent-session-update-plan.test.ts index 05d2567d8..656b5afe1 100644 --- a/packages/coding-agent/test/suite/agent-session-update-plan.test.ts +++ b/packages/coding-agent/test/suite/agent-session-update-plan.test.ts @@ -32,7 +32,9 @@ describe("AgentSession update_plan tool", () => { harnesses.push(harness); expect(harness.session.getActiveToolNames()).toContain("update_plan"); - expect(harness.session.systemPrompt).toContain("- update_plan: track progress with an ordered task checklist"); + expect(harness.session.systemPrompt).toContain( + "- update_plan: persist an ordered plan with step IDs and optional success criteria", + ); for (const guideline of updatePlanToolSystemPromptContribution.guidelines) { expect(harness.session.systemPrompt).toContain(`- ${guideline}`); } @@ -51,10 +53,10 @@ describe("AgentSession update_plan tool", () => { const harness = await createHarness(); harnesses.push(harness); const plan = [ - { step: "Read the code", status: "completed" }, - { step: " Implement update_plan ", status: "in_progress" }, - { step: "Run checks", status: "pending" }, - ] as Array<{ step: string; status: "pending" | "in_progress" | "completed" }>; + { id: "read-the-code", step: "Read the code", status: "completed" }, + { id: "implement-update-plan", step: " Implement update_plan ", status: "in_progress" }, + { id: "run-checks", step: "Run checks", status: "pending" }, + ] as Array<{ id: string; step: string; status: "pending" | "in_progress" | "completed" }>; harness.setResponses([ fauxAssistantMessage(fauxToolCall("update_plan", { plan, explanation: " exact explanation " }), { @@ -92,11 +94,94 @@ describe("AgentSession update_plan tool", () => { } } expect(entries[0].data).toMatchObject({ - plan: [{ step: "Read the code" }, { step: " Implement update_plan " }, { step: "Run checks" }], + plan: [ + { id: "read-the-code", step: "Read the code" }, + { id: "implement-update-plan", step: " Implement update_plan " }, + { id: "run-checks", step: "Run checks" }, + ], explanation: " exact explanation ", }); }); + it("puts the latest persisted plan at the end of each request after edits and reordering", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const firstPlan = [ + { id: "settings", step: "Fix settings", status: "in_progress", success_criteria: ["Survives restart"] }, + { id: "checks", step: "Run checks", status: "pending" }, + ]; + const reorderedPlan = [ + { id: "checks", step: "Run verification", status: "in_progress", success_criteria: ["Checks pass"] }, + { + id: "settings", + step: "Fix settings persistence", + status: "completed", + success_criteria: ["Settings remain readable"], + }, + ]; + const tailContexts: string[] = []; + const capture = (context: { messages: unknown[] }) => { + tailContexts.push(getMessageText(context.messages.at(-1))); + }; + harness.setResponses([ + (context) => { + capture(context); + return fauxAssistantMessage(fauxToolCall("update_plan", { plan: firstPlan }), { stopReason: "toolUse" }); + }, + (context) => { + capture(context); + return fauxAssistantMessage( + fauxToolCall("update_plan", { + update: { + id: "settings", + step: "Fix settings persistence", + success_criteria: ["Settings remain readable"], + }, + }), + { stopReason: "toolUse" }, + ); + }, + (context) => { + capture(context); + return fauxAssistantMessage(fauxToolCall("update_plan", { plan: reorderedPlan }), { + stopReason: "toolUse", + }); + }, + (context) => { + capture(context); + return fauxAssistantMessage("done"); + }, + ]); + + await harness.session.prompt("fix settings"); + + const prefix = "Current task context (latest plan):\n"; + expect(tailContexts).toEqual([ + `${prefix}${JSON.stringify({ plan: [] }, null, 2)}`, + `${prefix}${JSON.stringify({ plan: firstPlan }, null, 2)}`, + `${prefix}${JSON.stringify( + { + plan: [ + { + id: "settings", + step: "Fix settings persistence", + status: "in_progress", + success_criteria: ["Settings remain readable"], + }, + firstPlan[1], + ], + }, + null, + 2, + )}`, + `${prefix}${JSON.stringify({ plan: reorderedPlan }, null, 2)}`, + ]); + expect(resolveCurrentPlanState(harness.sessionManager)).toEqual({ + status: "active", + operation: { plan: reorderedPlan }, + }); + }); + it("executes batched update_plan operations sequentially in source order", async () => { const harness = await createHarness(); harnesses.push(harness); @@ -107,14 +192,14 @@ describe("AgentSession update_plan tool", () => { fauxToolCall( "update_plan", { - plan: [{ step: "Implement the change", status: "in_progress" }], + plan: [{ id: "implement-the-change", step: "Implement the change", status: "in_progress" }], explanation: "initial explanation", }, { id: "plan-update" }, ), fauxToolCall( "update_plan", - { plan: [{ step: "Replacement", status: "in_progress" }] }, + { plan: [{ id: "replacement", step: "Replacement", status: "in_progress" }] }, { id: "plan-replacement" }, ), fauxToolCall("update_plan", { clear: true }, { id: "plan-clear" }), @@ -152,18 +237,18 @@ describe("AgentSession update_plan tool", () => { ]); expect(toolResults.map((message) => message.details)).toEqual([ { - plan: [{ step: "Implement the change", status: "in_progress" }], + plan: [{ id: "implement-the-change", step: "Implement the change", status: "in_progress" }], explanation: "initial explanation", }, - { plan: [{ step: "Replacement", status: "in_progress" }] }, + { plan: [{ id: "replacement", step: "Replacement", status: "in_progress" }] }, { clear: true }, ]); expect(getCurrentPlanEntries(harness).map((entry) => entry.data)).toEqual([ { - plan: [{ step: "Implement the change", status: "in_progress" }], + plan: [{ id: "implement-the-change", step: "Implement the change", status: "in_progress" }], explanation: "initial explanation", }, - { plan: [{ step: "Replacement", status: "in_progress" }] }, + { plan: [{ id: "replacement", step: "Replacement", status: "in_progress" }] }, { clear: true }, ]); expect(resolveCurrentPlanState(harness.sessionManager)).toEqual({ @@ -178,7 +263,7 @@ describe("AgentSession update_plan tool", () => { harness.setResponses([ fauxAssistantMessage( fauxToolCall("update_plan", { - plan: [{ step: "Keep this", status: "in_progress" }], + plan: [{ id: "keep-this", step: "Keep this", status: "in_progress" }], explanation: "prior", }), { stopReason: "toolUse" }, @@ -188,9 +273,12 @@ describe("AgentSession update_plan tool", () => { await harness.session.prompt("set a plan"); harness.setResponses([ - fauxAssistantMessage(fauxToolCall("update_plan", { plan: [{ step: "Invalid", status: "pending" }] }), { - stopReason: "toolUse", - }), + fauxAssistantMessage( + fauxToolCall("update_plan", { plan: [{ id: "invalid", step: "Invalid", status: "pending" }] }), + { + stopReason: "toolUse", + }, + ), fauxAssistantMessage("continue after error"), ]); await harness.session.prompt("attempt an invalid replacement"); @@ -199,7 +287,7 @@ describe("AgentSession update_plan tool", () => { expect(resolveCurrentPlanState(harness.sessionManager)).toEqual({ status: "active", operation: { - plan: [{ step: "Keep this", status: "in_progress" }], + plan: [{ id: "keep-this", step: "Keep this", status: "in_progress" }], explanation: "prior", }, }); @@ -220,7 +308,7 @@ describe("AgentSession update_plan tool", () => { harness.setResponses([ fauxAssistantMessage( fauxToolCall("update_plan", { - plan: [{ step: "Cannot persist", status: "in_progress" }], + plan: [{ id: "cannot-persist", step: "Cannot persist", status: "in_progress" }], }), { stopReason: "toolUse" }, ), @@ -247,7 +335,7 @@ describe("AgentSession update_plan tool", () => { harness.setResponses([ fauxAssistantMessage( fauxToolCall("update_plan", { - plan: [{ step: "Shadowed", status: "in_progress" }], + plan: [{ id: "shadowed", step: "Shadowed", status: "in_progress" }], }), { stopReason: "toolUse" }, ), diff --git a/packages/coding-agent/test/system-prompt.test.ts b/packages/coding-agent/test/system-prompt.test.ts index 7d29b5aec..baa4349a0 100644 --- a/packages/coding-agent/test/system-prompt.test.ts +++ b/packages/coding-agent/test/system-prompt.test.ts @@ -69,7 +69,7 @@ describe("buildSystemPrompt", () => { bash: "Execute bash commands", edit: "Make surgical edits", write: "Create or overwrite files", - update_plan: "track progress with an ordered task checklist", + update_plan: "persist an ordered plan with step IDs and optional success criteria", }, contextFiles: [], skills: [], diff --git a/packages/coding-agent/test/tool-system-prompt-contributions.test.ts b/packages/coding-agent/test/tool-system-prompt-contributions.test.ts index 5ba0c21d5..93aeb3f83 100644 --- a/packages/coding-agent/test/tool-system-prompt-contributions.test.ts +++ b/packages/coding-agent/test/tool-system-prompt-contributions.test.ts @@ -42,9 +42,9 @@ describe("built-in tool system prompt contributions", () => { test("provides outcome-focused checklist lifecycle guidance", () => { expect(updatePlanToolSystemPromptContribution.guidelines).toEqual([ "Use update_plan for complex, ambiguous, dependent, long-running, or multi-part work, when the user requests a plan, or when checkpoints would improve collaboration; skip it for simple or immediately executable work.", - "Write short, ordered, outcome-focused steps with observable evidence where useful; do not narrate commands or pad the checklist with obvious actions.", + "Write short, ordered, outcome-focused steps. Give each step a stable ID and add success_criteria for subtasks needing validation.", "Include final verification when correctness requires it.", - "Update the checklist at meaningful milestones and mark completed work before moving on.", + "Update by ID at meaningful milestones; retain IDs when rewording, reordering, or changing status.", "Explain material replanning, omit explanations for routine progress, and use clear: true only when abandoning the checklist.", "Publishing the checklist communicates progress but does not execute, schedule, or prove completion of its steps.", ]); diff --git a/packages/coding-agent/test/update-plan-tool.test.ts b/packages/coding-agent/test/update-plan-tool.test.ts index 918f66bb4..2cc640f55 100644 --- a/packages/coding-agent/test/update-plan-tool.test.ts +++ b/packages/coding-agent/test/update-plan-tool.test.ts @@ -6,6 +6,7 @@ import type { ToolDefinition } from "../src/core/extensions/types.ts"; import { createUpdatePlanTool, createUpdatePlanToolDefinition, + resolveUpdatePlanOperation, type UpdatePlanToolDetails, type UpdatePlanToolInput, } from "../src/core/tools/update-plan.ts"; @@ -40,9 +41,14 @@ describe("update_plan tool", () => { const input = { explanation: " Repository evidence changed.\n", plan: [ - { step: "Read the code", status: "completed" }, - { step: " Implement the tool ", status: "in_progress" }, - { step: "Run checks", status: "pending" }, + { id: "read-the-code", step: "Read the code", status: "completed" }, + { + id: "implement-the-tool", + step: " Implement the tool ", + status: "in_progress", + success_criteria: ["Survives restart"], + }, + { id: "run-checks", step: "Run checks", status: "pending" }, ], } satisfies UpdatePlanToolInput; @@ -57,43 +63,115 @@ describe("update_plan tool", () => { expect(result.details.plan[0]).not.toBe(input.plan[0]); input.plan[0]!.step = "mutated"; + input.plan[1]!.success_criteria![0] = "mutated"; input.explanation = "mutated"; expect(result.details.plan[0]?.step).toBe("Read the code"); expect(result.details.plan[1]?.step).toBe(" Implement the tool "); + expect(result.details.plan[1]?.success_criteria).toEqual(["Survives restart"]); expect(result.details.explanation).toBe(" Repository evidence changed.\n"); }); test("accepts all-completed plans without an in-progress item", async () => { const result = await executeUpdatePlan({ plan: [ - { step: "Read the code", status: "completed" }, - { step: "Run checks", status: "completed" }, + { id: "read-the-code", step: "Read the code", status: "completed" }, + { id: "run-checks", step: "Run checks", status: "completed" }, ], }); expect(getTextOutput(result)).toBe("Plan updated: 2 completed, 0 in progress, 0 pending."); }); + test("updates one step by ID while preserving its identity and the other steps", async () => { + const plan = [ + { + id: "settings", + step: "Fix settings", + status: "in_progress" as const, + success_criteria: ["Survives restart"], + }, + { id: "checks", step: "Run checks", status: "pending" as const }, + ]; + const tool = createUpdatePlanTool(() => plan); + const result = await tool.execute("tool-1", { + update: { + id: "settings", + step: "Fix settings persistence", + success_criteria: ["Changed settings survive restart", "Existing settings remain readable"], + }, + }); + + expect(getTextOutput(result)).toBe("Plan updated: 0 completed, 1 in progress, 1 pending."); + expect(result.details).toEqual({ + plan: [ + { + id: "settings", + step: "Fix settings persistence", + status: "in_progress", + success_criteria: ["Changed settings survive restart", "Existing settings remain readable"], + }, + plan[1], + ], + }); + expect(plan[0]?.step).toBe("Fix settings"); + expect(plan[0]?.success_criteria).toEqual(["Survives restart"]); + + const clearedCriteria = resolveUpdatePlanOperation({ update: { id: "settings", success_criteria: [] } }, plan); + expect(clearedCriteria).toEqual({ + plan: [{ id: "settings", step: "Fix settings", status: "in_progress" }, plan[1]], + }); + }); + + test("validates step identity, criteria, and single-step updates", async () => { + await expect( + executeUpdatePlan({ plan: [{ step: "Missing ID", status: "in_progress" }] } as unknown as UpdatePlanToolInput), + ).rejects.toThrow("id must be a non-empty string"); + await expect( + executeUpdatePlan({ + plan: [ + { id: "same", step: "First", status: "in_progress" }, + { id: "same", step: "Second", status: "pending" }, + ], + }), + ).rejects.toThrow("Duplicate plan item id: same."); + await expect( + executeUpdatePlan({ plan: [{ id: "one", step: "One", status: "in_progress", success_criteria: [" "] }] }), + ).rejects.toThrow("success_criteria must be an array of non-empty strings"); + expect(() => resolveUpdatePlanOperation({ update: { id: "unknown", status: "completed" } })).toThrow( + "without a current plan", + ); + expect(() => + resolveUpdatePlanOperation({ update: { id: "unknown", status: "completed" } }, [ + { id: "one", step: "One", status: "in_progress" }, + ]), + ).toThrow("Unknown plan step id: unknown."); + expect(() => + resolveUpdatePlanOperation({ update: { id: "one", status: "pending" } }, [ + { id: "one", step: "One", status: "in_progress" }, + ]), + ).toThrow("exactly one in_progress"); + }); + test("treats each call as a replacement snapshot", async () => { const first = await executeUpdatePlan({ explanation: "Initial approach", plan: [ - { step: "One", status: "in_progress" }, - { step: "Two", status: "pending" }, + { id: "one", step: "One", status: "in_progress" }, + { id: "two", step: "Two", status: "pending" }, ], }); const second = await executeUpdatePlan({ - plan: [{ step: "Different", status: "completed" }], + plan: [{ id: "different", step: "Different", status: "completed" }], }); expect(first.details).toEqual({ explanation: "Initial approach", plan: [ - { step: "One", status: "in_progress" }, - { step: "Two", status: "pending" }, + { id: "one", step: "One", status: "in_progress" }, + { id: "two", step: "Two", status: "pending" }, ], }); - expect(second.details).toEqual({ plan: [{ step: "Different", status: "completed" }] }); + expect(second.details).toEqual({ plan: [{ id: "different", step: "Different", status: "completed" }] }); expect("explanation" in second.details).toBe(false); expect(getTextOutput(second)).toBe("Plan updated: 1 completed, 0 in progress, 0 pending."); }); @@ -110,14 +188,14 @@ describe("update_plan tool", () => { test("rejects ambiguous operations and unknown fields", async () => { await expect(executeUpdatePlan({} as UpdatePlanToolInput)).rejects.toThrow( - "Provide exactly one update_plan operation: plan or clear.", + "Provide exactly one update_plan operation: plan, update, or clear.", ); await expect( executeUpdatePlan({ - plan: [{ step: "One", status: "completed" }], + plan: [{ id: "one", step: "One", status: "completed" }], clear: true, } as unknown as UpdatePlanToolInput), - ).rejects.toThrow("Provide exactly one update_plan operation: plan or clear."); + ).rejects.toThrow("Provide exactly one update_plan operation: plan, update, or clear."); await expect(executeUpdatePlan({ clear: false } as unknown as UpdatePlanToolInput)).rejects.toThrow( "Clear must be true.", ); @@ -134,33 +212,33 @@ describe("update_plan tool", () => { await expect(executeUpdatePlan({ plan: ["step"] } as unknown as UpdatePlanToolInput)).rejects.toThrow( "Plan item 1 must be an object.", ); - await expect(executeUpdatePlan({ plan: [{ step: " ", status: "in_progress" }] })).rejects.toThrow( + await expect(executeUpdatePlan({ plan: [{ id: "blank", step: " ", status: "in_progress" }] })).rejects.toThrow( "Plan item 1 step must be a non-empty string.", ); await expect( executeUpdatePlan({ - plan: [{ step: "One", status: "in_progress", owner: "agent" }], + plan: [{ id: "one", step: "One", status: "in_progress", owner: "agent" }], } as unknown as UpdatePlanToolInput), ).rejects.toThrow("Plan item 1 contains unexpected field: owner."); await expect( executeUpdatePlan({ plan: [ - { step: "One", status: "pending" }, - { step: "Two", status: "pending" }, + { id: "one", step: "One", status: "pending" }, + { id: "two", step: "Two", status: "pending" }, ], }), ).rejects.toThrow("Unfinished plans must contain exactly one in_progress item."); await expect( executeUpdatePlan({ plan: [ - { step: "One", status: "in_progress" }, - { step: "Two", status: "in_progress" }, + { id: "one", step: "One", status: "in_progress" }, + { id: "two", step: "Two", status: "in_progress" }, ], }), ).rejects.toThrow("Unfinished plans must contain exactly one in_progress item."); await expect( executeUpdatePlan({ - plan: [{ step: "One", status: "blocked" }], + plan: [{ id: "one", step: "One", status: "blocked" }], } as unknown as UpdatePlanToolInput), ).rejects.toThrow("Plan item 1 status must be pending, in_progress, or completed."); }); @@ -173,12 +251,15 @@ describe("update_plan tool", () => { properties?: { clear?: { const?: boolean }; explanation?: { type?: string }; + update?: { additionalProperties?: boolean; properties?: { id?: { type?: string } } }; plan?: { minItems?: number; items?: { additionalProperties?: boolean; properties?: { + id?: { type?: string }; status?: { enum?: string[] }; + success_criteria?: { type?: string }; }; }; }; @@ -194,6 +275,10 @@ describe("update_plan tool", () => { } expect(parameters.properties?.plan?.minItems).toBe(1); expect(parameters.properties?.plan?.items?.additionalProperties).toBe(false); + expect(parameters.properties?.plan?.items?.properties?.id?.type).toBe("string"); + expect(parameters.properties?.plan?.items?.properties?.success_criteria?.type).toBe("array"); + expect(parameters.properties?.update?.additionalProperties).toBe(false); + expect(parameters.properties?.update?.properties?.id?.type).toBe("string"); expect(parameters.properties?.plan?.items?.properties?.status?.enum).toEqual([ "pending", "in_progress", @@ -308,9 +393,14 @@ describe("update_plan tool", () => { const input = { explanation: "Repository evidence changed.", plan: [ - { step: "Read the code", status: "completed" }, - { step: "Implement the tool", status: "in_progress" }, - { step: "Run checks", status: "pending" }, + { id: "read-the-code", step: "Read the code", status: "completed" }, + { + id: "implement-the-tool", + step: "Implement the tool", + status: "in_progress", + success_criteria: ["Works after restart"], + }, + { id: "run-checks", step: "Run checks", status: "pending" }, ], } satisfies UpdatePlanToolInput; const definition = createUpdatePlanToolDefinition() as ToolDefinition; @@ -336,9 +426,10 @@ describe("update_plan tool", () => { const rendered = stripAnsi(component.render(120).join("\n")); expect(rendered).toContain("update_plan"); expect(rendered).toContain("Explanation: Repository evidence changed."); - expect(rendered).toContain("[x] Read the code"); - expect(rendered).toContain("[>] Implement the tool"); - expect(rendered).toContain("[ ] Run checks"); + expect(rendered).toContain("[x] read-the-code Read the code"); + expect(rendered).toContain("[>] implement-the-tool Implement the tool"); + expect(rendered).toContain("- Works after restart"); + expect(rendered).toContain("[ ] run-checks Run checks"); expect(rendered).not.toContain("Plan updated: 1 completed, 1 in progress, 1 pending."); }); @@ -374,7 +465,7 @@ describe("update_plan tool", () => { const component = new ToolExecutionComponent( "update_plan", "tool-1", - { plan: [{ step: "Read", status: "pending" }] }, + { plan: [{ id: "read", step: "Read", status: "pending" }] }, {}, definition, createFakeTui(), From 6dfa4847ea804f1c7df038e47553a27da9b23d25 Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Sat, 26 Sep 2026 15:52:13 -0400 Subject: [PATCH 2/8] docs(coding-agent): document plan context - Describes step IDs, criteria, single-step updates, and per-request plan context --- packages/coding-agent/README.md | 5 ++++- packages/coding-agent/docs/sdk.md | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/README.md b/packages/coding-agent/README.md index 5aa2c68fc..7d6096236 100644 --- a/packages/coding-agent/README.md +++ b/packages/coding-agent/README.md @@ -243,7 +243,10 @@ Configure delivery in [settings](docs/settings.md): `steeringMode` and `followUp Sessions are stored as JSONL files with a tree structure. Each entry has an `id` and `parentId`, enabling in-place branching without creating new files. See [docs/session-format.md](docs/session-format.md) for file format. -The built-in `update_plan` checklist records the latest checklist state in the session. When an unfinished checklist exists after opening or resuming a session, or after successful compaction, Diffex restores it once as hidden context for the next model request. This is not a workflow engine: it does not schedule, execute, gate, verify, or prove work. +The built-in `update_plan` tool stores an ordered plan with stable step IDs and optional `success_criteria` in the session. +It can replace the plan, update one step by ID, or clear the plan. +Every model request ends with hidden current-task context containing the latest plan, including after resume and compaction. +The plan records progress; it does not execute, gate, verify, or prove work. The selected collaboration mode is stored separately as an interactive branch preference. Resume, fork, and `/tree` navigation restore it from the selected branch; non-interactive modes always use Default and do not overwrite that preference. diff --git a/packages/coding-agent/docs/sdk.md b/packages/coding-agent/docs/sdk.md index 177a85710..641faa2f5 100644 --- a/packages/coding-agent/docs/sdk.md +++ b/packages/coding-agent/docs/sdk.md @@ -614,7 +614,9 @@ Specify which tools to enable: - `excludeTools` disables specific built-in, extension, custom, or coordination tool names after any `tools` allowlist is applied The `edit` tool returns `details.diff` for Diffex's TUI display and `details.patch` as a standard unified patch for SDK consumers. -The `update_plan` tool returns `details.plan` with the full ordered checklist snapshot. +The `update_plan` tool accepts a full `plan`, a single-step `update` by stable ID, or `clear: true`. +Each plan step has an `id`, `step`, and `status`, with optional `success_criteria` strings. +It returns a short acknowledgement; plan writes and step updates also return the resulting snapshot in `details.plan`. ```typescript import { createAgentSession } from "@diffexai/diffex"; From ddc9d652972c2313942368a7eb5bd379d3b2e2bd Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Sat, 26 Sep 2026 16:07:21 -0400 Subject: [PATCH 3/8] feat(coding-agent): show footer cache hit rate - Makes session cache reuse visible alongside context usage - Fits the rate within narrow footer widths --- .../modes/interactive/components/footer.ts | 22 +++++++-- .../coding-agent/test/footer-width.test.ts | 48 +++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/src/modes/interactive/components/footer.ts b/packages/coding-agent/src/modes/interactive/components/footer.ts index ec12b1c90..8bd79bd8e 100644 --- a/packages/coding-agent/src/modes/interactive/components/footer.ts +++ b/packages/coding-agent/src/modes/interactive/components/footer.ts @@ -159,6 +159,12 @@ export class FooterComponent implements Component { theme.fg("footerContext", `context ${contextPercent}%`) + theme.fg("borderMuted", " / ") + theme.fg("footerWindow", windowDisplay); + const { input, cacheRead, cacheWrite } = this.session.getSessionStats().tokens; + const promptTokens = input + cacheRead + cacheWrite; + const cacheHitPercent = + promptTokens > 0 && (cacheRead > 0 || cacheWrite > 0) + ? `${((cacheRead / promptTokens) * 100).toFixed(1)}%` + : undefined; const planMode = getCollaborationMode(this.session) === "plan"; const planFull = theme.fg("warning", "Plan mode"); @@ -215,6 +221,9 @@ export class FooterComponent implements Component { const tryAddField = (field: FooterField): boolean => trySetFields([...fields, field]); const tryUpgradeField = (key: string, text: string): boolean => trySetFields(fields.map((field) => (field.key === key ? { ...field, text } : field))); + if (cacheHitPercent !== undefined) { + tryAddField({ key: "cache", order: 3, text: theme.fg("dim", `cache ${cacheHitPercent}`) }); + } // Extension statuses are public UI state, so admit them before optional // detail upgrades and shortcut hints. Truncate only their own field. @@ -229,7 +238,7 @@ export class FooterComponent implements Component { if (statusText.length > 0 && availableWidth > 0) { tryAddField({ key: "extensions", - order: 4, + order: 5, text: truncateToWidth(theme.fg("dim", statusText), availableWidth, glyphs.ellipsis), }); } @@ -238,15 +247,18 @@ export class FooterComponent implements Component { tryAddField({ key: "thinking", order: 1, text: theme.fg("thinkingText", thinkingLevel) }); tryAddField({ key: "harness", - order: 3, + order: 4, text: theme.fg("dim", `h:${harnessVersion}`) + evolutionSpinner, }); tryUpgradeField("context", contextMedium); tryUpgradeField("thinking", theme.fg("thinkingText", `thinking ${thinkingLevel}`)); + if (cacheHitPercent !== undefined) { + tryUpgradeField("cache", theme.fg("dim", `input cache hit ${cacheHitPercent}`)); + } tryUpgradeField("harness", theme.fg("dim", `harness ${harnessVersion}`) + evolutionSpinner); tryUpgradeField("context", contextFull); - tryAddField({ key: "hotkeys", order: 7, text: theme.fg("dim", "/hotkeys") }); + tryAddField({ key: "hotkeys", order: 8, text: theme.fg("dim", "/hotkeys") }); const getBindingHint = (action: "app.thinking.cycle" | "app.model.cycleForward", label: string) => { try { @@ -258,11 +270,11 @@ export class FooterComponent implements Component { }; const thinkingHint = getBindingHint("app.thinking.cycle", "thinking"); if (thinkingHint) { - tryAddField({ key: "thinkingHint", order: 5, text: thinkingHint }); + tryAddField({ key: "thinkingHint", order: 6, text: thinkingHint }); } const modelHint = getBindingHint("app.model.cycleForward", "model"); if (modelHint) { - tryAddField({ key: "modelHint", order: 6, text: modelHint }); + tryAddField({ key: "modelHint", order: 7, text: modelHint }); } return [renderFields(fields)]; diff --git a/packages/coding-agent/test/footer-width.test.ts b/packages/coding-agent/test/footer-width.test.ts index ccc4af061..d63eddcb7 100644 --- a/packages/coding-agent/test/footer-width.test.ts +++ b/packages/coding-agent/test/footer-width.test.ts @@ -104,6 +104,13 @@ function createSession(options: { getCwd: () => "/tmp/project", }, getContextUsage: () => ({ contextWindow: 200_000, percent: 12.3 }), + getSessionStats: () => ({ + tokens: { + input: usage?.input ?? 0, + cacheRead: usage?.cacheRead ?? 0, + cacheWrite: usage?.cacheWrite ?? 0, + }, + }), modelRuntime: { isUsingSubscription: () => options.usingSubscription ?? false, }, @@ -238,6 +245,47 @@ describe("FooterComponent width handling", () => { expect(lines[0]).toContain("/hotkeys"); }); + it("shows the session input cache hit rate alongside context usage", () => { + const session = createSession({ + sessionName: "", + modelId: "gpt-5.5", + thinkingLevel: "medium", + usage: { + input: 1_000, + output: 200, + cacheRead: 8_000, + cacheWrite: 1_000, + cost: { total: 0.1 }, + }, + }); + const footer = new FooterComponent(session, createFooterData(1)); + + const wideLine = stripAnsi(footer.render(160)[0] ?? ""); + expect(wideLine).toContain("input cache hit 80.0%"); + expect(wideLine).toContain("context 12.3% / 200k"); + expect(wideLine).toContain("thinking medium"); + + const standardLine = stripAnsi(footer.render(60)[0] ?? ""); + expect(standardLine).toContain("cache 80.0%"); + expect(visibleWidth(standardLine)).toBeLessThanOrEqual(60); + }); + + it("shows a zero cache hit rate when the provider reports only cache writes", () => { + const session = createSession({ + sessionName: "", + usage: { + input: 9_000, + output: 200, + cacheRead: 0, + cacheWrite: 1_000, + cost: { total: 0.1 }, + }, + }); + const footer = new FooterComponent(session, createFooterData(1)); + + expect(stripAnsi(footer.render(160)[0] ?? "")).toContain("input cache hit 0.0%"); + }); + it("uses dedicated colors for footer model, context usage, and context window", () => { const session = createSession({ sessionName: "", From dd966f051770aececd5cddfaea8a3b19252b277c Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Sat, 26 Sep 2026 16:58:13 -0400 Subject: [PATCH 4/8] fix(coding-agent): cache settled tool output - Prevents saved tool output from being remeasured on every keystroke - Keeps typing responsive as session history grows --- .../interactive/components/tool-group.ts | 30 ++++++++++++++----- .../test/tool-group-component.test.ts | 23 ++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/src/modes/interactive/components/tool-group.ts b/packages/coding-agent/src/modes/interactive/components/tool-group.ts index abee5ca34..225369950 100644 --- a/packages/coding-agent/src/modes/interactive/components/tool-group.ts +++ b/packages/coding-agent/src/modes/interactive/components/tool-group.ts @@ -19,6 +19,7 @@ export class ToolGroupComponent implements Component { private pathOrigin: DiffexPathOrigin; private reusesLeadingGap: boolean; private settled = false; + private cachedRender?: { width: number; lines: string[] }; constructor( outputPad = 1, @@ -36,6 +37,7 @@ export class ToolGroupComponent implements Component { return existing; } this.tools.set(toolCallId, component); + this.cachedRender = undefined; return component; } @@ -44,7 +46,9 @@ export class ToolGroupComponent implements Component { } removeTool(toolCallId: string): boolean { - return this.tools.delete(toolCallId); + const removed = this.tools.delete(toolCallId); + if (removed) this.cachedRender = undefined; + return removed; } replaceTools(tools: ReadonlyArray): void { @@ -53,6 +57,7 @@ export class ToolGroupComponent implements Component { this.tools.delete(toolCallId); this.tools.set(toolCallId, component); } + this.cachedRender = undefined; } hasTools(): boolean { @@ -66,41 +71,49 @@ export class ToolGroupComponent implements Component { setPathOrigin(pathOrigin: DiffexPathOrigin, options: { reuseLeadingGap?: boolean } = {}): void { this.pathOrigin = pathOrigin; this.reusesLeadingGap = pathOrigin === "standalone" && options.reuseLeadingGap === true; + this.cachedRender = undefined; } markSettled(): void { this.settled = true; + this.cachedRender = undefined; } setOutputPad(padding: number): void { this.outputPad = padding; + this.cachedRender = undefined; } setExpanded(expanded: boolean): void { for (const tool of this.tools.values()) { tool.setExpanded(expanded); } + this.cachedRender = undefined; } setShowImages(show: boolean): void { for (const tool of this.tools.values()) { tool.setShowImages(show); } + this.cachedRender = undefined; } setImageWidthCells(width: number): void { for (const tool of this.tools.values()) { tool.setImageWidthCells(width); } + this.cachedRender = undefined; } invalidate(): void { for (const tool of this.tools.values()) { tool.invalidate(); } + this.cachedRender = undefined; } render(width: number): string[] { + if (this.settled && this.cachedRender?.width === width) return this.cachedRender.lines; const safeWidth = Math.max(1, width); const outputPad = Math.min(Math.max(0, this.outputPad), Math.max(0, safeWidth - 1)); const outerRailWidth = safeWidth - outputPad > 1 ? 1 : 0; @@ -120,12 +133,14 @@ export class ToolGroupComponent implements Component { } if (renderedTools.length === 0) { - if (this.pathOrigin !== "continued") { - return []; - } - return this.settled - ? new DiffexPathTerminus(outputPad).render(safeWidth) - : new DiffexPathRail(outputPad).render(safeWidth); + const lines = + this.pathOrigin !== "continued" + ? [] + : this.settled + ? new DiffexPathTerminus(outputPad).render(safeWidth) + : new DiffexPathRail(outputPad).render(safeWidth); + if (this.settled) this.cachedRender = { width, lines }; + return lines; } const label = this.settled ? "Explored" : "Exploring"; @@ -158,6 +173,7 @@ export class ToolGroupComponent implements Component { } } + if (this.settled) this.cachedRender = { width, lines: result }; return result; } } diff --git a/packages/coding-agent/test/tool-group-component.test.ts b/packages/coding-agent/test/tool-group-component.test.ts index 0552b8aa6..b4d372279 100644 --- a/packages/coding-agent/test/tool-group-component.test.ts +++ b/packages/coding-agent/test/tool-group-component.test.ts @@ -55,6 +55,29 @@ describe("ToolGroupComponent", () => { expect(settled).not.toContain("Exploring"); }); + test("reuses settled output until width or display settings change", () => { + const group = new ToolGroupComponent(1); + const tool = createTool("cached", "Cached"); + const render = vi.spyOn(tool, "render"); + group.addTool("cached", tool); + group.markSettled(); + + const first = group.render(80); + expect(group.render(80)).toBe(first); + expect(render).toHaveBeenCalledTimes(1); + + group.render(90); + expect(render).toHaveBeenCalledTimes(2); + + group.setExpanded(true); + group.render(90); + expect(render).toHaveBeenCalledTimes(3); + + group.invalidate(); + group.render(90); + expect(render).toHaveBeenCalledTimes(4); + }); + test("renders a standalone tool group as one open Diffex path", () => { const glyphs = getGlyphs(); const group = new ToolGroupComponent(1); From de0b6039b43bd6fb8652ee26f2d90b4ad0bcdb5c Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Sat, 26 Sep 2026 17:23:07 -0400 Subject: [PATCH 5/8] fix(coding-agent): preserve plan cache prefix - Keep plan snapshots in persisted tool results and compaction summaries - Let ordinary requests reuse prior model message prefixes --- README.md | 7 + packages/coding-agent/docs/sdk.md | 5 +- .../coding-agent/src/core/agent-session.ts | 38 +- .../src/core/tools/update-plan.ts | 5 +- .../suite/agent-session-compaction.test.ts | 4 +- ...t-session-current-plan-restoration.test.ts | 502 ++++++------------ .../test/suite/agent-session-prompt.test.ts | 13 +- .../suite/agent-session-update-plan.test.ts | 83 +-- .../regressions/249-plan-cache-prefix.test.ts | 103 ++++ .../test/update-plan-tool.test.ts | 16 +- 10 files changed, 306 insertions(+), 470 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/249-plan-cache-prefix.test.ts diff --git a/README.md b/README.md index 2b82f4968..ecf7521a3 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,13 @@ npm run check `npm run check` runs formatting, linting, dependency policy checks, and TypeScript validation. `./test.sh` runs the non-E2E test suites without activating tests that require provider credentials. +## Plan History + +`update_plan` saves each changed plan in the session history and returns the complete resolved plan in its tool result, including step IDs, statuses, and success criteria. +Clearing the plan returns `Plan cleared.` +Successful compaction summaries include the latest plan snapshot, or an empty plan when it was cleared or absent, so resumed and forked sessions retain the task context. +Ordinary requests reuse the existing conversation prefix without appending a plan message to every model request. + ## Documentation - [Coding-agent documentation](packages/coding-agent/docs/index.md) diff --git a/packages/coding-agent/docs/sdk.md b/packages/coding-agent/docs/sdk.md index 641faa2f5..7c75619d6 100644 --- a/packages/coding-agent/docs/sdk.md +++ b/packages/coding-agent/docs/sdk.md @@ -616,7 +616,10 @@ Specify which tools to enable: The `edit` tool returns `details.diff` for Diffex's TUI display and `details.patch` as a standard unified patch for SDK consumers. The `update_plan` tool accepts a full `plan`, a single-step `update` by stable ID, or `clear: true`. Each plan step has an `id`, `step`, and `status`, with optional `success_criteria` strings. -It returns a short acknowledgement; plan writes and step updates also return the resulting snapshot in `details.plan`. +Plan writes and step updates return the complete resolved plan in the model-visible tool result, including IDs, statuses, and success criteria, and in `details.plan`. +The session persists each successful plan change in conversation history, while `clear: true` returns `Plan cleared.` +Successful compaction summaries include the latest plan snapshot, or an empty plan if the plan was cleared or absent, so resume and fork retain it after older tool results are compacted away. +Ordinary requests do not append a separate plan message, allowing subsequent requests to reuse the same prompt prefix. ```typescript import { createAgentSession } from "@diffexai/diffex"; diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 9996e8775..d7016b1c8 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -159,7 +159,6 @@ import { type BashOperations, createLocalBashOperations } from "./tools/bash.ts" import { createAllToolDefinitions } from "./tools/index.ts"; import { createRequestUserInputToolDefinition, REQUEST_USER_INPUT_TOOL_NAME } from "./tools/request-user-input.ts"; import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.ts"; -import type { UpdatePlanItem } from "./tools/update-plan.ts"; import { addUsageToTotals, createUsageTotals } from "./usage-totals.ts"; // ============================================================================ @@ -460,7 +459,6 @@ function estimateMessagesTokens(messages: AgentMessage[]): number { /** Standard thinking levels */ const THINKING_LEVELS: ThinkingLevel[] = ["off", "minimal", "low", "medium", "high"]; -const CURRENT_TASK_CONTEXT_CUSTOM_TYPE = "diffex.current-task-context"; const MAX_PENDING_SUBAGENT_ESCALATION_CHARACTERS = 8_000; const INCOMPATIBLE_SUBAGENT_PARENT_HOOKS = [ "resources_discover", @@ -615,7 +613,6 @@ export class AgentSession { // (session persistence, extensions, auto-compaction, retry logic) this._unsubscribeAgent = this.agent.subscribe(this._handleAgentEvent); this._installAgentToolHooks(); - this._installCurrentTaskContext(); this._installAgentNextTurnRefresh(); this._buildRuntime({ @@ -839,29 +836,6 @@ export class AgentSession { }; } - private _installCurrentTaskContext(): void { - const previousTransformContext = this.agent.transformContext; - this.agent.transformContext = async (messages, signal) => { - const transformedMessages = previousTransformContext - ? await previousTransformContext(messages, signal) - : messages; - const state = resolveCurrentPlanState(this.sessionManager); - let plan: UpdatePlanItem[] = []; - if (state.status === "active") plan = state.operation.plan; - else if (state.status === "inactive" && "plan" in state.operation) plan = state.operation.plan; - const currentTaskContext: CustomMessage = { - role: "custom", - customType: CURRENT_TASK_CONTEXT_CUSTOM_TYPE, - content: [ - { type: "text", text: `Current task context (latest plan):\n${JSON.stringify({ plan }, null, 2)}` }, - ], - display: false, - timestamp: Date.now(), - }; - return [...transformedMessages, currentTaskContext]; - }; - } - // ========================================================================= // Event Subscription // ========================================================================= @@ -3462,9 +3436,17 @@ export class AgentSession { signal?: AbortSignal; deferCommit?: (transaction: CompactionAppendTransaction) => void; }): Promise { + const planState = resolveCurrentPlanState(this.sessionManager); + const plan = + planState.status === "active" + ? planState.operation.plan + : planState.status === "inactive" && "plan" in planState.operation + ? planState.operation.plan + : []; + const summary = `${options.summary}\n\nCurrent task context (latest plan):\n${JSON.stringify({ plan }, null, 2)}`; const transaction = appendCompactionTransaction( this.sessionManager, - options.summary, + summary, options.firstKeptEntryId, options.tokensBefore, options.details, @@ -3503,7 +3485,7 @@ export class AgentSession { else transaction.commit(); const compactionResult: CompactionResult = { - summary: options.summary, + summary, firstKeptEntryId: options.firstKeptEntryId, tokensBefore: options.tokensBefore, estimatedTokensAfter, diff --git a/packages/coding-agent/src/core/tools/update-plan.ts b/packages/coding-agent/src/core/tools/update-plan.ts index f7ef1eebe..d9b9a13d3 100644 --- a/packages/coding-agent/src/core/tools/update-plan.ts +++ b/packages/coding-agent/src/core/tools/update-plan.ts @@ -353,7 +353,10 @@ export function createUpdatePlanToolDefinition( content: [ { type: "text", - text: "clear" in details ? "Plan cleared." : formatPlanSummary(details.plan), + text: + "clear" in details + ? "Plan cleared." + : `${formatPlanSummary(details.plan)}\n${JSON.stringify({ plan: details.plan }, null, 2)}`, }, ], details, diff --git a/packages/coding-agent/test/suite/agent-session-compaction.test.ts b/packages/coding-agent/test/suite/agent-session-compaction.test.ts index 14cb992c8..f197caa57 100644 --- a/packages/coding-agent/test/suite/agent-session-compaction.test.ts +++ b/packages/coding-agent/test/suite/agent-session-compaction.test.ts @@ -131,7 +131,9 @@ describe("AgentSession compaction characterization", () => { const compactionEntries = harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction"); const estimatedTokensAfter = harness.session.messages.reduce((sum, message) => sum + estimateTokens(message), 0); - expect(result.summary).toBe("summary from extension"); + expect(result.summary).toBe( + `summary from extension\n\nCurrent task context (latest plan):\n${JSON.stringify({ plan: [] }, null, 2)}`, + ); expect(result.usage).toEqual(summaryUsage); expect(result.estimatedTokensAfter).toBe(estimatedTokensAfter); expect(compactionEntries).toHaveLength(1); diff --git a/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts b/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts index d06f46f03..c14a50930 100644 --- a/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts +++ b/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts @@ -1,103 +1,16 @@ -import type { AgentMessage } from "@diffexai/diffex-agent-core"; -import { fauxAssistantMessage, type Message, type TextContent } from "@diffexai/diffex-ai"; +import { fauxAssistantMessage, fauxToolCall, type Message } from "@diffexai/diffex-ai"; import { afterEach, describe, expect, it } from "vitest"; -import { - appendCurrentPlanRecord, - CURRENT_PLAN_CUSTOM_ENTRY_TYPE, - resolveCurrentPlanState, -} from "../../src/core/current-plan-state.ts"; -import type { SessionStartEvent } from "../../src/core/extensions/index.ts"; -import { SessionManager } from "../../src/core/session-manager.ts"; -import { createUpdatePlanToolDefinition, type UpdatePlanToolDetails } from "../../src/core/tools/update-plan.ts"; -import { createHarness, type Harness } from "./harness.ts"; +import { appendCurrentPlanRecord, CURRENT_PLAN_CUSTOM_ENTRY_TYPE } from "../../src/core/current-plan-state.ts"; +import type { UpdatePlanItem } from "../../src/core/tools/update-plan.ts"; +import { createHarness, getMessageText, type Harness } from "./harness.ts"; -type PlanUpdateDetails = Extract; -type SessionWithCompactionInternals = { - _runAutoCompaction: (reason: "overflow" | "threshold", willRetry: boolean) => Promise; -}; +const plan: UpdatePlanItem[] = [ + { id: "inspect", step: "Inspect state", status: "completed" }, + { id: "implement", step: "Implement change", status: "in_progress", success_criteria: ["Resume retains the plan"] }, +]; -const CURRENT_TASK_CONTEXT_HEADER = "Current task context (latest plan):"; - -function activeOperation(): PlanUpdateDetails { - return { - plan: [ - { id: "read-existing-state", step: "Read existing state", status: "completed" }, - { - id: "implement-exact-restore", - step: " Implement exact restore ", - status: "in_progress", - success_criteria: ["Plan survives resume", "Criteria survive compaction"], - }, - { id: "verify-once", step: "Verify once", status: "pending" }, - ], - explanation: " keep this exact explanation ", - }; -} - -function operationWithoutExplanation(): PlanUpdateDetails { - return { - plan: [{ id: "continue-unfinished-work", step: "Continue unfinished work", status: "in_progress" }], - }; -} - -function expectedRestoredText(operation: PlanUpdateDetails): string { - return `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: operation.plan }, null, 2)}`; -} - -function createPlanSessionManager(operation: PlanUpdateDetails = activeOperation()): SessionManager { - const sessionManager = SessionManager.inMemory(); - sessionManager.appendMessage({ - role: "user", - content: [{ type: "text", text: "existing session prompt" }], - timestamp: Date.now() - 10_000, - }); - appendCurrentPlanRecord(sessionManager, operation); - return sessionManager; -} - -function messageText(message: Message): string { - const content = message.content; - if (typeof content === "string") { - return content; - } - return content - .filter((part): part is TextContent => part.type === "text") - .map((part) => part.text) - .join("\n"); -} - -function findRestoredText(messages: Message[]): string | undefined { - return messages.map(messageText).find((text) => text.includes(CURRENT_TASK_CONTEXT_HEADER)); -} - -function findRestoredContextText(texts: string[]): string | undefined { - return texts.find((text) => text.includes(CURRENT_TASK_CONTEXT_HEADER)); -} - -async function submitAndCaptureContextTexts(harness: Harness, prompt = "continue"): Promise { - const capturedContexts: Message[][] = []; - harness.setResponses([ - (context) => { - capturedContexts.push(context.messages); - return fauxAssistantMessage("done"); - }, - ]); - - await harness.session.prompt(prompt); - - expect(capturedContexts).toHaveLength(1); - return (capturedContexts[0] ?? []).map(messageText); -} - -function createUsage(totalTokens: number) { - return { - input: totalTokens, - output: 0, - cacheRead: 0, - cacheWrite: 0, - totalTokens, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, - }; +function planSnapshot(items: readonly UpdatePlanItem[]): string { + return `Current task context (latest plan):\n${JSON.stringify({ plan: items }, null, 2)}`; } function seedCompactableSession(harness: Harness): void { @@ -109,281 +22,166 @@ function seedCompactableSession(harness: Harness): void { timestamp: now - 1000, }); harness.sessionManager.appendMessage({ - ...fauxAssistantMessage("assistant response to compact", { timestamp: now - 500 }), - usage: createUsage(100), + ...fauxAssistantMessage("response to compact", { timestamp: now - 500 }), + usage: { + input: 100, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 100, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, }); harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; } -describe("AgentSession current-plan restoration", () => { +describe("AgentSession durable plan restoration", () => { const harnesses: Harness[] = []; - afterEach(() => { - while (harnesses.length > 0) { - harnesses.pop()?.cleanup(); - } + afterEach(async () => { + while (harnesses.length > 0) await harnesses.pop()?.cleanup(); }); - it("appends exact current-task context on every request after opening or resuming", async () => { - const operation = activeOperation(); - const extensionContextMessage: AgentMessage = { - role: "custom", - customType: "test.context", - content: "extension transformed context", - display: false, - timestamp: Date.now(), - }; - const opened = await createHarness({ - sessionManager: createPlanSessionManager(operation), - extensionFactories: [ - (diffex) => { - diffex.on("context", (event) => ({ messages: [...event.messages, extensionContextMessage] })); + it("retains the complete tool result on resume and fork while preserving extension context transforms", async () => { + const original = await createHarness(); + harnesses.push(original); + original.setResponses([ + fauxAssistantMessage(fauxToolCall("update_plan", { plan }), { stopReason: "toolUse" }), + fauxAssistantMessage("created"), + ]); + await original.session.prompt("create plan"); + + const leaf = original.sessionManager.getLeafEntry(); + if (!leaf) throw new Error("Missing session leaf"); + for (const sessionManager of [original.sessionManager, original.sessionManager.forkBranch(leaf.id)]) { + const restored = await createHarness({ + sessionManager, + extensionFactories: [ + (diffex) => + diffex.on("context", (event) => ({ + messages: [ + ...event.messages, + { + role: "custom", + customType: "test.extension-context", + content: "extension context", + display: false, + timestamp: Date.now(), + }, + ], + })), + ], + }); + harnesses.push(restored); + restored.session.agent.state.messages = sessionManager.buildSessionContext().messages; + const requests: Message[][] = []; + restored.setResponses([ + (context) => { + requests.push(context.messages); + return fauxAssistantMessage("resumed"); }, - ], - }); - harnesses.push(opened); - - const firstContextTexts = await submitAndCaptureContextTexts(opened, "first submission"); - expect(findRestoredContextText(firstContextTexts)).toBe(expectedRestoredText(operation)); - expect(firstContextTexts.indexOf("extension transformed context")).toBeLessThan( - firstContextTexts.findIndex((text) => text.includes(CURRENT_TASK_CONTEXT_HEADER)), - ); - expect(firstContextTexts.at(-1)).toBe(expectedRestoredText(operation)); - - const secondContextTexts = await submitAndCaptureContextTexts(opened, "second submission"); - expect(secondContextTexts.at(-1)).toBe(expectedRestoredText(operation)); - - expect(opened.session.messages.some((message) => message.role === "custom")).toBe(false); - expect(opened.sessionManager.getEntries().filter((entry) => entry.type === "custom_message")).toHaveLength(0); - expect( - opened.events.some( - (event) => - (event.type === "message_start" || event.type === "message_end") && event.message.role === "custom", - ), - ).toBe(false); - - const resumed = await createHarness({ - sessionManager: createPlanSessionManager(operation), - sessionStartEvent: { type: "session_start", reason: "resume", previousSessionFile: "previous.jsonl" }, - }); - harnesses.push(resumed); - - const resumedContextTexts = await submitAndCaptureContextTexts(resumed, "resumed submission"); - expect(findRestoredContextText(resumedContextTexts)).toBe(expectedRestoredText(operation)); + ]); + await restored.session.prompt("continue"); + expect(requests[0]?.map(getMessageText)).toContain("extension context"); + expect(requests[0]?.map(getMessageText)).toContain( + `Plan updated: 1 completed, 1 in progress, 0 pending.\n${JSON.stringify({ plan }, null, 2)}`, + ); + expect(requests[0]?.map(getMessageText).some((text) => text.startsWith("Current task context"))).toBe(false); + } }); - it("includes current-task context for new sessions and forks", async () => { - for (const reason of ["new", "fork"] as const) { - const startEvent: SessionStartEvent = { - type: "session_start", - reason, - previousSessionFile: "previous.jsonl", - }; + it("persists the latest plan in manual and automatic compaction summaries", async () => { + for (const mode of ["manual", "auto"] as const) { const harness = await createHarness({ - sessionManager: createPlanSessionManager(), - sessionStartEvent: startEvent, + extensionFactories: [ + (diffex) => + diffex.on("session_before_compact", (event) => ({ + compaction: { + summary: `${mode} summary`, + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })), + ], }); harnesses.push(harness); - - const contextTexts = await submitAndCaptureContextTexts(harness, `${reason} submission`); - expect(contextTexts.at(-1)).toBe(expectedRestoredText(activeOperation())); + seedCompactableSession(harness); + appendCurrentPlanRecord(harness.sessionManager, { plan }); + + if (mode === "manual") await harness.session.compact(); + else + await ( + harness.session as unknown as { + _runAutoCompaction: (reason: "threshold", willRetry: boolean) => Promise; + } + )._runAutoCompaction("threshold", false); + + const entry = harness.sessionManager.getLeafEntry(); + expect(entry?.type).toBe("compaction"); + if (entry?.type !== "compaction") continue; + expect(entry.summary).toBe(`${mode} summary\n\n${planSnapshot(plan)}`); + const summaryMessage = harness.sessionManager.buildSessionContext().messages[0]; + expect(summaryMessage?.role).toBe("compactionSummary"); + if (summaryMessage?.role === "compactionSummary") expect(summaryMessage.summary).toContain(planSnapshot(plan)); + const fork = harness.sessionManager.forkBranch(entry.id); + const forkSummary = fork.buildSessionContext().messages[0]; + expect(forkSummary?.role).toBe("compactionSummary"); + if (forkSummary?.role === "compactionSummary") expect(forkSummary.summary).toContain(planSnapshot(plan)); + for (const sessionManager of [harness.sessionManager, fork]) { + const resumed = await createHarness({ sessionManager }); + harnesses.push(resumed); + resumed.session.agent.state.messages = sessionManager.buildSessionContext().messages; + resumed.setResponses([ + (context) => { + expect(context.messages.map(getMessageText).join("\n")).toContain(planSnapshot(plan)); + return fauxAssistantMessage("continued"); + }, + ]); + await resumed.session.prompt("continue after compaction"); + } } }); - it("keeps current-task context after manual, auto, and cancelled compaction", async () => { - const manualOperation = operationWithoutExplanation(); - const manual = await createHarness({ - extensionFactories: [ - (diffex) => { - diffex.on("session_before_compact", (event) => ({ - compaction: { - summary: "manual summary", - firstKeptEntryId: event.preparation.firstKeptEntryId, - tokensBefore: event.preparation.tokensBefore, - details: {}, - }, - })); - }, - ], - }); - harnesses.push(manual); - seedCompactableSession(manual); - appendCurrentPlanRecord(manual.sessionManager, manualOperation); - - await manual.session.compact(); - - const manualContextTexts = await submitAndCaptureContextTexts(manual, "after manual compaction"); - const restoredManual = findRestoredContextText(manualContextTexts); - expect(restoredManual).toBe(expectedRestoredText(manualOperation)); - expect(restoredManual).not.toContain("explanation"); - - const autoOperation = operationWithoutExplanation(); - const auto = await createHarness({ - extensionFactories: [ - (diffex) => { - diffex.on("session_before_compact", (event) => ({ - compaction: { - summary: "auto summary", - firstKeptEntryId: event.preparation.firstKeptEntryId, - tokensBefore: event.preparation.tokensBefore, - details: {}, - }, - })); - }, - ], - }); - harnesses.push(auto); - seedCompactableSession(auto); - appendCurrentPlanRecord(auto.sessionManager, autoOperation); - - await (auto.session as unknown as SessionWithCompactionInternals)._runAutoCompaction("threshold", false); - - const autoContextTexts = await submitAndCaptureContextTexts(auto, "after auto compaction"); - expect(findRestoredContextText(autoContextTexts)).toBe(expectedRestoredText(autoOperation)); - - const cancelled = await createHarness({ - extensionFactories: [ - (diffex) => { - diffex.on("session_before_compact", () => ({ cancel: true })); - }, - ], - }); - harnesses.push(cancelled); - seedCompactableSession(cancelled); - appendCurrentPlanRecord(cancelled.sessionManager, operationWithoutExplanation()); - - await expect(cancelled.session.compact()).rejects.toThrow("Compaction cancelled"); - - const cancelledContextTexts = await submitAndCaptureContextTexts(cancelled, "after cancelled compaction"); - expect(findRestoredContextText(cancelledContextTexts)).toBe(expectedRestoredText(operationWithoutExplanation())); - }); - - it("includes current-task context after preflight paths and model failure", async () => { - let handleInput = true; - const handled = await createHarness({ - sessionManager: createPlanSessionManager(), - extensionFactories: [ - (diffex) => { - diffex.on("input", () => (handleInput ? { action: "handled" } : { action: "continue" })); - }, - ], - }); - harnesses.push(handled); - - await handled.session.prompt("handled before model"); - expect(handled.faux.state.callCount).toBe(0); - handleInput = false; - - const handledContextTexts = await submitAndCaptureContextTexts(handled, "actual model submission"); - expect(findRestoredContextText(handledContextTexts)).toBe(expectedRestoredText(activeOperation())); - - const preflight = await createHarness({ - sessionManager: createPlanSessionManager(), - withConfiguredAuth: false, - }); - harnesses.push(preflight); - - await expect(preflight.session.prompt("missing auth")).rejects.toThrow( - `No API key found for ${preflight.getModel().provider}.`, - ); - const model = preflight.getModel(); - preflight.session.modelRuntime.registerProvider(model.provider, { - baseUrl: model.baseUrl, - apiKey: "faux-key", - api: preflight.faux.api, - models: preflight.models.map((registeredModel) => ({ - id: registeredModel.id, - name: registeredModel.name, - api: registeredModel.api, - reasoning: registeredModel.reasoning, - input: registeredModel.input, - cost: registeredModel.cost, - contextWindow: registeredModel.contextWindow, - maxTokens: registeredModel.maxTokens, - baseUrl: registeredModel.baseUrl, - })), - }); - - const preflightContextTexts = await submitAndCaptureContextTexts(preflight, "after preflight failure"); - expect(findRestoredContextText(preflightContextTexts)).toBe(expectedRestoredText(activeOperation())); - - const modelFailure = await createHarness({ - sessionManager: createPlanSessionManager(), - settings: { retry: { enabled: false } }, - }); - harnesses.push(modelFailure); - const failureContexts: Message[][] = []; - modelFailure.setResponses([ - (context) => { - failureContexts.push(context.messages); - throw new Error("model failed"); - }, - ]); - - await modelFailure.session.prompt("fails after submission"); - expect(findRestoredText(failureContexts[0] ?? [])).toBe(expectedRestoredText(activeOperation())); - const failedMessage = modelFailure.session.messages.at(-1); - expect(failedMessage?.role).toBe("assistant"); - if (failedMessage?.role === "assistant") { - expect(failedMessage.stopReason).toBe("error"); - expect(failedMessage.errorMessage).toContain("model failed"); + it("records an empty plan after clearing or when no valid plan exists", async () => { + for (const state of ["cleared", "absent", "corrupt"] as const) { + const harness = await createHarness({ + extensionFactories: [ + (diffex) => + diffex.on("session_before_compact", (event) => ({ + compaction: { + summary: "summary", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })), + ], + }); + harnesses.push(harness); + seedCompactableSession(harness); + if (state === "cleared") { + appendCurrentPlanRecord(harness.sessionManager, { plan }); + appendCurrentPlanRecord(harness.sessionManager, { clear: true }); + } else if (state === "corrupt") { + appendCurrentPlanRecord(harness.sessionManager, { plan }); + harness.sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, { plan: [] }); + } + const result = await harness.session.compact(); + expect(result.summary).toBe(`summary\n\n${planSnapshot([])}`); } - - const afterFailureTexts = await submitAndCaptureContextTexts(modelFailure, "after model failure"); - expect(findRestoredContextText(afterFailureTexts)).toBe(expectedRestoredText(activeOperation())); - }); - - it("uses the newest state when a plan is cleared, absent, or corrupt", async () => { - const inactive = await createHarness({ sessionManager: createPlanSessionManager() }); - harnesses.push(inactive); - appendCurrentPlanRecord(inactive.sessionManager, { clear: true }); - - const inactiveTexts = await submitAndCaptureContextTexts(inactive, "after clear"); - expect(findRestoredContextText(inactiveTexts)).toBe( - `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: [] }, null, 2)}`, - ); - expect(resolveCurrentPlanState(inactive.sessionManager).status).toBe("inactive"); - - const absent = await createHarness({ sessionManager: createPlanSessionManager() }); - harnesses.push(absent); - absent.sessionManager.resetLeaf(); - - const absentTexts = await submitAndCaptureContextTexts(absent, "after branch reset"); - expect(findRestoredContextText(absentTexts)).toBe( - `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: [] }, null, 2)}`, - ); - - const corrupt = await createHarness({ sessionManager: createPlanSessionManager() }); - harnesses.push(corrupt); - corrupt.sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, { - plan: [{ id: "do-not-revive-older-valid-state", step: "Do not revive older valid state", status: "pending" }], - }); - - const corruptTexts = await submitAndCaptureContextTexts(corrupt, "after corrupt newest"); - expect(findRestoredContextText(corruptTexts)).toBe( - `${CURRENT_TASK_CONTEXT_HEADER}\n${JSON.stringify({ plan: [] }, null, 2)}`, - ); - expect(resolveCurrentPlanState(corrupt.sessionManager).status).toBe("corrupt"); }); - it("restores independently of active tool selection and same-name custom tools", async () => { - const excluded = await createHarness({ - sessionManager: createPlanSessionManager(), - excludedToolNames: ["update_plan"], + it("persists nothing when compaction is cancelled", async () => { + const harness = await createHarness({ + extensionFactories: [(diffex) => diffex.on("session_before_compact", () => ({ cancel: true }))], }); - harnesses.push(excluded); - expect(excluded.session.getActiveToolNames()).not.toContain("update_plan"); - - const excludedTexts = await submitAndCaptureContextTexts(excluded, "without update_plan selected"); - expect(findRestoredContextText(excludedTexts)).toBe(expectedRestoredText(activeOperation())); - - const shadowed = await createHarness({ - sessionManager: createPlanSessionManager(), - extensionFactories: [(diffex) => diffex.registerTool(createUpdatePlanToolDefinition())], - }); - harnesses.push(shadowed); - - const shadowedTexts = await submitAndCaptureContextTexts(shadowed, "with shadowing update_plan"); - expect(findRestoredContextText(shadowedTexts)).toBe(expectedRestoredText(activeOperation())); + harnesses.push(harness); + seedCompactableSession(harness); + appendCurrentPlanRecord(harness.sessionManager, { plan }); + const entriesBefore = harness.sessionManager.getEntries(); + await expect(harness.session.compact()).rejects.toThrow("Compaction cancelled"); + expect(harness.sessionManager.getEntries()).toEqual(entriesBefore); + expect(harness.sessionManager.getEntries().some((entry) => entry.type === "compaction")).toBe(false); }); }); diff --git a/packages/coding-agent/test/suite/agent-session-prompt.test.ts b/packages/coding-agent/test/suite/agent-session-prompt.test.ts index 10d6bf95e..d38db5039 100644 --- a/packages/coding-agent/test/suite/agent-session-prompt.test.ts +++ b/packages/coding-agent/test/suite/agent-session-prompt.test.ts @@ -886,7 +886,10 @@ describe("AgentSession prompt characterization", () => { const reason = new Error("cancel saved compaction"); const prompt = harness.session.prompt("new prompt", { signal: controller.signal }); await compactHookStarted.promise; - expect(sessionManager.getLeafEntry()).toMatchObject({ type: "compaction", summary: "temporarily recorded" }); + expect(sessionManager.getLeafEntry()).toMatchObject({ + type: "compaction", + summary: `temporarily recorded\n\nCurrent task context (latest plan):\n${JSON.stringify({ plan: [] }, null, 2)}`, + }); controller.abort(reason); releaseCompactHook.resolve(); @@ -988,7 +991,9 @@ describe("AgentSession prompt characterization", () => { expect(harness.eventsOfType("compaction_end")).toEqual([ expect.objectContaining({ aborted: false, - result: expect.objectContaining({ summary: "completed before acceptance" }), + result: expect.objectContaining({ + summary: `completed before acceptance\n\nCurrent task context (latest plan):\n${JSON.stringify({ plan: [] }, null, 2)}`, + }), }), ]); expect(harness.faux.state.callCount).toBe(0); @@ -1042,7 +1047,9 @@ describe("AgentSession prompt characterization", () => { }), ).rejects.toBe(callbackError); expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction")).toEqual([ - expect.objectContaining({ summary: "committed before notification failure" }), + expect.objectContaining({ + summary: `committed before notification failure\n\nCurrent task context (latest plan):\n${JSON.stringify({ plan: [{ id: "continue", step: "continue", status: "in_progress" }] }, null, 2)}`, + }), ]); let restoredPlan = false; diff --git a/packages/coding-agent/test/suite/agent-session-update-plan.test.ts b/packages/coding-agent/test/suite/agent-session-update-plan.test.ts index 656b5afe1..38105e98b 100644 --- a/packages/coding-agent/test/suite/agent-session-update-plan.test.ts +++ b/packages/coding-agent/test/suite/agent-session-update-plan.test.ts @@ -78,7 +78,9 @@ describe("AgentSession update_plan tool", () => { expect(toolResult?.role).toBe("toolResult"); if (toolResult?.role !== "toolResult") return; expect(toolResult.toolName).toBe("update_plan"); - expect(getMessageText(toolResult)).toBe("Plan updated: 1 completed, 1 in progress, 1 pending."); + expect(getMessageText(toolResult)).toBe( + `Plan updated: 1 completed, 1 in progress, 1 pending.\n${JSON.stringify({ plan }, null, 2)}`, + ); expect(toolResult.details).toEqual({ plan, explanation: " exact explanation " }); expect(getMessageText(harness.session.messages[3])).toBe("continuing after plan"); @@ -103,85 +105,6 @@ describe("AgentSession update_plan tool", () => { }); }); - it("puts the latest persisted plan at the end of each request after edits and reordering", async () => { - const harness = await createHarness(); - harnesses.push(harness); - const firstPlan = [ - { id: "settings", step: "Fix settings", status: "in_progress", success_criteria: ["Survives restart"] }, - { id: "checks", step: "Run checks", status: "pending" }, - ]; - const reorderedPlan = [ - { id: "checks", step: "Run verification", status: "in_progress", success_criteria: ["Checks pass"] }, - { - id: "settings", - step: "Fix settings persistence", - status: "completed", - success_criteria: ["Settings remain readable"], - }, - ]; - const tailContexts: string[] = []; - const capture = (context: { messages: unknown[] }) => { - tailContexts.push(getMessageText(context.messages.at(-1))); - }; - harness.setResponses([ - (context) => { - capture(context); - return fauxAssistantMessage(fauxToolCall("update_plan", { plan: firstPlan }), { stopReason: "toolUse" }); - }, - (context) => { - capture(context); - return fauxAssistantMessage( - fauxToolCall("update_plan", { - update: { - id: "settings", - step: "Fix settings persistence", - success_criteria: ["Settings remain readable"], - }, - }), - { stopReason: "toolUse" }, - ); - }, - (context) => { - capture(context); - return fauxAssistantMessage(fauxToolCall("update_plan", { plan: reorderedPlan }), { - stopReason: "toolUse", - }); - }, - (context) => { - capture(context); - return fauxAssistantMessage("done"); - }, - ]); - - await harness.session.prompt("fix settings"); - - const prefix = "Current task context (latest plan):\n"; - expect(tailContexts).toEqual([ - `${prefix}${JSON.stringify({ plan: [] }, null, 2)}`, - `${prefix}${JSON.stringify({ plan: firstPlan }, null, 2)}`, - `${prefix}${JSON.stringify( - { - plan: [ - { - id: "settings", - step: "Fix settings persistence", - status: "in_progress", - success_criteria: ["Settings remain readable"], - }, - firstPlan[1], - ], - }, - null, - 2, - )}`, - `${prefix}${JSON.stringify({ plan: reorderedPlan }, null, 2)}`, - ]); - expect(resolveCurrentPlanState(harness.sessionManager)).toEqual({ - status: "active", - operation: { plan: reorderedPlan }, - }); - }); - it("executes batched update_plan operations sequentially in source order", async () => { const harness = await createHarness(); harnesses.push(harness); diff --git a/packages/coding-agent/test/suite/regressions/249-plan-cache-prefix.test.ts b/packages/coding-agent/test/suite/regressions/249-plan-cache-prefix.test.ts new file mode 100644 index 000000000..bbf2861c0 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/249-plan-cache-prefix.test.ts @@ -0,0 +1,103 @@ +import { fauxAssistantMessage, fauxToolCall, type Message } from "@diffexai/diffex-ai"; +import { afterEach, describe, expect, it } from "vitest"; +import { createHarness, getMessageText, type Harness } from "../harness.ts"; + +describe("PR 249 plan prompt cache prefix", () => { + let harness: Harness | undefined; + + afterEach(async () => { + await harness?.cleanup(); + }); + + it("extends the prior request after plan creation", async () => { + harness = await createHarness(); + const requests: Message[][] = []; + const capture = (context: { messages: Message[] }) => { + requests.push(context.messages); + return fauxAssistantMessage("done"); + }; + harness.setResponses([ + fauxAssistantMessage( + fauxToolCall("update_plan", { + plan: [{ id: "work", step: "Do the work", status: "in_progress" }], + }), + { stopReason: "toolUse" }, + ), + fauxAssistantMessage("plan created"), + capture, + capture, + capture, + ]); + + await harness.session.prompt("create a plan"); + await harness.session.prompt("ordinary request one"); + await harness.session.prompt("ordinary request two"); + await harness.session.prompt("ordinary request three"); + + expect(requests).toHaveLength(3); + for (let index = 1; index < requests.length; index++) { + const previous = requests[index - 1].map(getMessageText); + const current = requests[index].map(getMessageText); + expect(current.slice(0, previous.length)).toEqual(previous); + } + }); + + it("keeps reusable prefixes after step updates, reordering, and clearing", async () => { + harness = await createHarness(); + const firstPlan = [ + { id: "work", step: "Do the work", status: "in_progress" as const, success_criteria: ["Works"] }, + { id: "verify", step: "Verify the work", status: "pending" as const }, + ]; + const updatedPlan = [ + { ...firstPlan[0], step: "Do the work carefully", success_criteria: ["Works", "Persists"] }, + firstPlan[1], + ]; + const reorderedPlan = [ + { id: "verify", step: "Verify the work", status: "in_progress" as const, success_criteria: ["Tests pass"] }, + { ...updatedPlan[0], status: "completed" as const }, + ]; + const submitTool = async (input: object, prompt: string) => { + harness?.setResponses([ + fauxAssistantMessage(fauxToolCall("update_plan", input), { stopReason: "toolUse" }), + fauxAssistantMessage("continued"), + ]); + await harness?.session.prompt(prompt); + }; + const checkOrdinaryPrefix = async (label: string, expectedResult: string) => { + const requests: Message[][] = []; + harness?.setResponses( + [1, 2].map(() => (context: { messages: Message[] }) => { + requests.push(context.messages); + return fauxAssistantMessage("done"); + }), + ); + await harness?.session.prompt(`${label} one`); + await harness?.session.prompt(`${label} two`); + expect(requests[0]?.map(getMessageText)).toContain(expectedResult); + expect(requests[1]?.slice(0, requests[0]?.length).map(getMessageText)).toEqual( + requests[0]?.map(getMessageText), + ); + }; + + await submitTool({ plan: firstPlan }, "create plan"); + await checkOrdinaryPrefix( + "created", + `Plan updated: 0 completed, 1 in progress, 1 pending.\n${JSON.stringify({ plan: firstPlan }, null, 2)}`, + ); + await submitTool( + { update: { id: "work", step: "Do the work carefully", success_criteria: ["Works", "Persists"] } }, + "update step", + ); + await checkOrdinaryPrefix( + "updated", + `Plan updated: 0 completed, 1 in progress, 1 pending.\n${JSON.stringify({ plan: updatedPlan }, null, 2)}`, + ); + await submitTool({ plan: reorderedPlan }, "reorder plan"); + await checkOrdinaryPrefix( + "reordered", + `Plan updated: 1 completed, 1 in progress, 0 pending.\n${JSON.stringify({ plan: reorderedPlan }, null, 2)}`, + ); + await submitTool({ clear: true }, "clear plan"); + await checkOrdinaryPrefix("cleared", "Plan cleared."); + }); +}); diff --git a/packages/coding-agent/test/update-plan-tool.test.ts b/packages/coding-agent/test/update-plan-tool.test.ts index 2cc640f55..8462c121f 100644 --- a/packages/coding-agent/test/update-plan-tool.test.ts +++ b/packages/coding-agent/test/update-plan-tool.test.ts @@ -54,7 +54,9 @@ describe("update_plan tool", () => { const result = await executeUpdatePlan(input); - expect(getTextOutput(result)).toBe("Plan updated: 1 completed, 1 in progress, 1 pending."); + expect(getTextOutput(result)).toBe( + `Plan updated: 1 completed, 1 in progress, 1 pending.\n${JSON.stringify({ plan: input.plan }, null, 2)}`, + ); expect(result.details).toEqual(input); expect(result.details).not.toBe(input); expect("plan" in result.details).toBe(true); @@ -79,7 +81,9 @@ describe("update_plan tool", () => { ], }); - expect(getTextOutput(result)).toBe("Plan updated: 2 completed, 0 in progress, 0 pending."); + expect(getTextOutput(result)).toBe( + `Plan updated: 2 completed, 0 in progress, 0 pending.\n${JSON.stringify({ plan: result.details && "plan" in result.details ? result.details.plan : [] }, null, 2)}`, + ); }); test("updates one step by ID while preserving its identity and the other steps", async () => { @@ -101,7 +105,9 @@ describe("update_plan tool", () => { }, }); - expect(getTextOutput(result)).toBe("Plan updated: 0 completed, 1 in progress, 1 pending."); + expect(getTextOutput(result)).toBe( + `Plan updated: 0 completed, 1 in progress, 1 pending.\n${JSON.stringify(result.details && "plan" in result.details ? { plan: result.details.plan } : {}, null, 2)}`, + ); expect(result.details).toEqual({ plan: [ { @@ -173,7 +179,9 @@ describe("update_plan tool", () => { }); expect(second.details).toEqual({ plan: [{ id: "different", step: "Different", status: "completed" }] }); expect("explanation" in second.details).toBe(false); - expect(getTextOutput(second)).toBe("Plan updated: 1 completed, 0 in progress, 0 pending."); + expect(getTextOutput(second)).toBe( + `Plan updated: 1 completed, 0 in progress, 0 pending.\n${JSON.stringify(second.details && "plan" in second.details ? { plan: second.details.plan } : {}, null, 2)}`, + ); }); test("clears idempotently with or without an exact explanation", async () => { From c138e1c66a77eb872a65043d68de3057bfebccca Mon Sep 17 00:00:00 2001 From: Jay Yu <117954436+Pepps233@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:21:39 -0400 Subject: [PATCH 6/8] Modify guidelines to clarify success criteria usage Updated guidelines to include optional success criteria for subtasks. --- packages/coding-agent/src/core/tools/update-plan.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coding-agent/src/core/tools/update-plan.ts b/packages/coding-agent/src/core/tools/update-plan.ts index d9b9a13d3..5486e1a13 100644 --- a/packages/coding-agent/src/core/tools/update-plan.ts +++ b/packages/coding-agent/src/core/tools/update-plan.ts @@ -82,7 +82,7 @@ export const updatePlanToolSystemPromptContribution = { snippet: "persist an ordered plan with step IDs and optional success criteria", guidelines: [ "Use update_plan for complex, ambiguous, dependent, long-running, or multi-part work, when the user requests a plan, or when checkpoints would improve collaboration; skip it for simple or immediately executable work.", - "Write short, ordered, outcome-focused steps. Give each step a stable ID and add success_criteria for subtasks needing validation.", + "Write short, ordered, outcome-focused steps. Give each step a stable ID and add optional success_criteria for subtasks needing validation.", "Include final verification when correctness requires it.", "Update by ID at meaningful milestones; retain IDs when rewording, reordering, or changing status.", "Explain material replanning, omit explanations for routine progress, and use clear: true only when abandoning the checklist.", From e770a5025aaef28124f43b62fbd380185987e973 Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Mon, 28 Sep 2026 16:27:08 -0400 Subject: [PATCH 7/8] fix(coding-agent): restore plan on tree navigation - Make the destination plan explicit when abandoned branches are summarized - Persist the snapshot so restored sessions retain unambiguous plan context --- README.md | 1 + packages/coding-agent/docs/sdk.md | 2 + .../coding-agent/src/core/agent-session.ts | 14 +- .../src/core/current-plan-state.ts | 21 +- ...t-session-current-plan-restoration.test.ts | 5 +- .../regressions/249-tree-plan-context.test.ts | 194 ++++++++++++++++++ 6 files changed, 224 insertions(+), 13 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/249-tree-plan-context.test.ts diff --git a/README.md b/README.md index ecf7521a3..a98e8d155 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ npm run check `update_plan` saves each changed plan in the session history and returns the complete resolved plan in its tool result, including step IDs, statuses, and success criteria. Clearing the plan returns `Plan cleared.` Successful compaction summaries include the latest plan snapshot, or an empty plan when it was cleared or absent, so resumed and forked sessions retain the task context. +When `/tree` summarizes an abandoned branch, its saved summary identifies that branch's plan references as historical and includes the selected branch's authoritative plan snapshot. Ordinary requests reuse the existing conversation prefix without appending a plan message to every model request. ## Documentation diff --git a/packages/coding-agent/docs/sdk.md b/packages/coding-agent/docs/sdk.md index 7c75619d6..993d676a7 100644 --- a/packages/coding-agent/docs/sdk.md +++ b/packages/coding-agent/docs/sdk.md @@ -619,6 +619,8 @@ Each plan step has an `id`, `step`, and `status`, with optional `success_criteri Plan writes and step updates return the complete resolved plan in the model-visible tool result, including IDs, statuses, and success criteria, and in `details.plan`. The session persists each successful plan change in conversation history, while `clear: true` returns `Plan cleared.` Successful compaction summaries include the latest plan snapshot, or an empty plan if the plan was cleared or absent, so resume and fork retain it after older tool results are compacted away. +When `navigateTree()` summarizes the abandoned branch, the saved branch summary includes the destination branch's authoritative plan snapshot and marks plan references in the exploration summary as historical. +Selecting a user message uses the plan at its parent, and navigating before any plan or after clearing it records an empty plan. Ordinary requests do not append a separate plan message, allowing subsequent requests to reuse the same prompt prefix. ```typescript diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index d7016b1c8..9cb758052 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -87,7 +87,7 @@ import { prepareCompaction, shouldCompact, } from "./compaction/index.ts"; -import { createSessionUpdatePlanToolDefinition, resolveCurrentPlanState } from "./current-plan-state.ts"; +import { createSessionUpdatePlanToolDefinition, formatCurrentPlanSnapshot } from "./current-plan-state.ts"; import { DEFAULT_THINKING_LEVEL } from "./defaults.ts"; import { exportSessionToHtml, type ToolHtmlRenderer } from "./export-html/index.ts"; import { createToolHtmlRenderer } from "./export-html/tool-renderer.ts"; @@ -3436,14 +3436,7 @@ export class AgentSession { signal?: AbortSignal; deferCommit?: (transaction: CompactionAppendTransaction) => void; }): Promise { - const planState = resolveCurrentPlanState(this.sessionManager); - const plan = - planState.status === "active" - ? planState.operation.plan - : planState.status === "inactive" && "plan" in planState.operation - ? planState.operation.plan - : []; - const summary = `${options.summary}\n\nCurrent task context (latest plan):\n${JSON.stringify({ plan }, null, 2)}`; + const summary = `${options.summary}\n\n${formatCurrentPlanSnapshot(this.sessionManager)}`; const transaction = appendCompactionTransaction( this.sessionManager, summary, @@ -4704,9 +4697,10 @@ export class AgentSession { let summaryEntry: BranchSummaryEntry | undefined; const mutateLeaf = (): void => { if (summaryText) { + const summary = `${summaryText}\n\nThe plan below is authoritative for the selected conversation branch. Plan references in the exploration summary above are historical.\n${formatCurrentPlanSnapshot(this.sessionManager, newLeafId)}`; const summaryId = this.sessionManager.branchWithSummary( newLeafId, - summaryText, + summary, summaryDetails, fromExtension, summaryUsage, diff --git a/packages/coding-agent/src/core/current-plan-state.ts b/packages/coding-agent/src/core/current-plan-state.ts index 7021d7fed..76cab2300 100644 --- a/packages/coding-agent/src/core/current-plan-state.ts +++ b/packages/coding-agent/src/core/current-plan-state.ts @@ -20,8 +20,11 @@ export function appendCurrentPlanRecord( return sessionManager.appendCustomEntry(CURRENT_PLAN_CUSTOM_ENTRY_TYPE, resolveUpdatePlanOperation(operation)); } -export function resolveCurrentPlanState(sessionManager: Pick): CurrentPlanState { - const branch = sessionManager.getBranch(); +export function resolveCurrentPlanState( + sessionManager: Pick, + fromId?: string | null, +): CurrentPlanState { + const branch = fromId === null ? [] : sessionManager.getBranch(fromId); for (let index = branch.length - 1; index >= 0; index--) { const entry = branch[index]; if (entry.type !== "custom" || entry.customType !== CURRENT_PLAN_CUSTOM_ENTRY_TYPE) { @@ -43,6 +46,20 @@ export function resolveCurrentPlanState(sessionManager: Pick, + fromId?: string | null, +): string { + const state = resolveCurrentPlanState(sessionManager, fromId); + const plan = + state.status === "active" + ? state.operation.plan + : state.status === "inactive" && "plan" in state.operation + ? state.operation.plan + : []; + return `Current task context (latest plan):\n${JSON.stringify({ plan }, null, 2)}`; +} + export function createSessionUpdatePlanToolDefinition( sessionManager: Pick, ): ReturnType { diff --git a/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts b/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts index c14a50930..209fc56e6 100644 --- a/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts +++ b/packages/coding-agent/test/suite/agent-session-current-plan-restoration.test.ts @@ -132,13 +132,16 @@ describe("AgentSession durable plan restoration", () => { const resumed = await createHarness({ sessionManager }); harnesses.push(resumed); resumed.session.agent.state.messages = sessionManager.buildSessionContext().messages; + const requests: Message[][] = []; resumed.setResponses([ (context) => { - expect(context.messages.map(getMessageText).join("\n")).toContain(planSnapshot(plan)); + requests.push(context.messages); return fauxAssistantMessage("continued"); }, ]); await resumed.session.prompt("continue after compaction"); + expect(requests).toHaveLength(1); + expect(requests[0].map(getMessageText).join("\n")).toContain(planSnapshot(plan)); } } }); diff --git a/packages/coding-agent/test/suite/regressions/249-tree-plan-context.test.ts b/packages/coding-agent/test/suite/regressions/249-tree-plan-context.test.ts new file mode 100644 index 000000000..e635f83bc --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/249-tree-plan-context.test.ts @@ -0,0 +1,194 @@ +import { fauxAssistantMessage, fauxToolCall, type Message } from "@diffexai/diffex-ai"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveCurrentPlanState } from "../../../src/core/current-plan-state.ts"; +import type { UpdatePlanItem } from "../../../src/core/tools/update-plan.ts"; +import { createHarness, getMessageText, type Harness } from "../harness.ts"; + +const originalPlan: UpdatePlanItem[] = [ + { + id: "implement", + step: "Implement original approach", + status: "in_progress", + success_criteria: ["Original checks pass"], + }, + { id: "verify", step: "Verify original approach", status: "pending" }, +]; +const abandonedPlan: UpdatePlanItem[] = [ + { + id: "replacement", + step: "Implement abandoned approach", + status: "in_progress", + success_criteria: ["Abandoned checks pass"], + }, +]; +const explorationSummary = `Explored a later branch with this plan:\n${JSON.stringify({ plan: abandonedPlan }, null, 2)}`; + +function expectedSnapshot(plan: readonly UpdatePlanItem[]): string { + return `Current task context (latest plan):\n${JSON.stringify({ plan }, null, 2)}`; +} + +async function writePlan(harness: Harness, plan: UpdatePlanItem[]): Promise { + harness.setResponses([ + fauxAssistantMessage(fauxToolCall("update_plan", { plan }), { stopReason: "toolUse" }), + fauxAssistantMessage("plan saved"), + ]); + await harness.session.prompt("set the plan"); + const leafId = harness.sessionManager.getLeafId(); + if (!leafId) throw new Error("Missing plan session leaf"); + return leafId; +} + +async function captureRequest(harness: Harness, prompt: string): Promise { + const requests: Message[][] = []; + harness.setResponses([ + (context) => { + requests.push(context.messages); + return fauxAssistantMessage("continued"); + }, + ]); + await harness.session.prompt(prompt); + expect(requests).toHaveLength(1); + return requests[0]; +} + +describe("PR 249 tree plan context", () => { + const harnesses: Harness[] = []; + + afterEach(async () => { + while (harnesses.length > 0) await harnesses.pop()?.cleanup(); + }); + + it.each([ + ["extension", "assistant"], + ["model", "assistant"], + ["extension", "user"], + ["model", "user"], + ] as const)("makes the selected plan authoritative with a %s summary at a %s target", async (source, targetKind) => { + const harness = await createHarness({ + extensionFactories: + source === "extension" + ? [ + (diffex) => + diffex.on("session_before_tree", () => ({ + summary: { summary: explorationSummary }, + })), + ] + : undefined, + }); + harnesses.push(harness); + const originalLeafId = await writePlan(harness, originalPlan); + await writePlan(harness, abandonedPlan); + const userTarget = harness.sessionManager + .getBranch() + .filter((entry) => entry.type === "message" && entry.message.role === "user") + .at(-1); + const targetId = targetKind === "user" ? userTarget?.id : originalLeafId; + if (!targetId) throw new Error("Missing navigation target"); + if (source === "model") harness.setResponses([fauxAssistantMessage(explorationSummary)]); + + const navigation = await harness.session.navigateTree(targetId, { summarize: true }); + expect(navigation.cancelled).toBe(false); + expect(resolveCurrentPlanState(harness.sessionManager)).toEqual({ + status: "active", + operation: { plan: originalPlan }, + }); + const requests: Message[][] = []; + harness.setResponses( + [1, 2].map(() => (context: { messages: Message[] }) => { + requests.push(context.messages); + return fauxAssistantMessage("continued"); + }), + ); + await harness.session.prompt("continue selected branch"); + await harness.session.prompt("continue again"); + + const summaryText = requests[0].map(getMessageText).find((text) => text.includes(explorationSummary)); + expect(summaryText).toContain("The plan below is authoritative for the selected conversation branch."); + expect(summaryText).toContain(expectedSnapshot(originalPlan)); + expect(summaryText?.lastIndexOf(expectedSnapshot(originalPlan))).toBeGreaterThan( + summaryText?.indexOf(explorationSummary) ?? -1, + ); + expect(navigation.summaryEntry?.summary.endsWith(expectedSnapshot(originalPlan))).toBe(true); + expect(requests[1].slice(0, requests[0].length)).toEqual(requests[0]); + + const summaryId = navigation.summaryEntry?.id; + if (!summaryId) throw new Error("Missing persisted branch summary"); + for (const sessionManager of [harness.sessionManager, harness.sessionManager.forkBranch(summaryId)]) { + const restored = await createHarness({ sessionManager }); + harnesses.push(restored); + restored.session.agent.state.messages = sessionManager.buildSessionContext().messages; + const messages = await captureRequest(restored, "continue restored branch"); + const summary = messages.map(getMessageText).find((text) => text.includes(explorationSummary)); + expect(summary).toContain(expectedSnapshot(originalPlan)); + } + }); + + it.each(["absent", "cleared", "completed"] as const)("restores a %s destination plan", async (state) => { + const harness = await createHarness({ + extensionFactories: [ + (diffex) => diffex.on("session_before_tree", () => ({ summary: { summary: explorationSummary } })), + ], + }); + harnesses.push(harness); + const selectedPlan = + state === "completed" ? originalPlan.map((item) => ({ ...item, status: "completed" as const })) : originalPlan; + let targetId = await writePlan(harness, selectedPlan); + if (state === "absent") { + const firstUser = harness.sessionManager + .getBranch() + .find((entry) => entry.type === "message" && entry.message.role === "user"); + if (!firstUser) throw new Error("Missing root user message"); + targetId = firstUser.id; + } else if (state === "cleared") { + harness.setResponses([ + fauxAssistantMessage(fauxToolCall("update_plan", { clear: true }), { stopReason: "toolUse" }), + fauxAssistantMessage("plan cleared"), + ]); + await harness.session.prompt("clear the plan"); + const leafId = harness.sessionManager.getLeafId(); + if (!leafId) throw new Error("Missing cleared plan leaf"); + targetId = leafId; + } + await writePlan(harness, abandonedPlan); + const result = await harness.session.navigateTree(targetId, { summarize: true }); + const expectedPlan = state === "completed" ? selectedPlan : []; + expect(result.summaryEntry?.summary.endsWith(expectedSnapshot(expectedPlan))).toBe(true); + const messages = await captureRequest(harness, "continue destination"); + const summary = messages.map(getMessageText).find((text) => text.includes(explorationSummary)); + expect(summary).toContain(expectedSnapshot(expectedPlan)); + }); + + it("restores the selected history without adding a summary when summarization is disabled", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const targetId = await writePlan(harness, originalPlan); + await writePlan(harness, abandonedPlan); + const result = await harness.session.navigateTree(targetId, { summarize: false }); + expect(result.summaryEntry).toBeUndefined(); + expect(resolveCurrentPlanState(harness.sessionManager)).toEqual({ + status: "active", + operation: { plan: originalPlan }, + }); + const messages = await captureRequest(harness, "continue without summary"); + const history = messages.map(getMessageText).join("\n"); + expect(history).toContain(JSON.stringify({ plan: originalPlan }, null, 2)); + expect(history).not.toContain("Implement abandoned approach"); + }); + + it("persists nothing when summarized tree navigation is cancelled", async () => { + const harness = await createHarness({ + extensionFactories: [(diffex) => diffex.on("session_before_tree", () => ({ cancel: true }))], + }); + harnesses.push(harness); + const targetId = await writePlan(harness, originalPlan); + await writePlan(harness, abandonedPlan); + const entriesBefore = structuredClone(harness.sessionManager.getEntries()); + const leafBefore = harness.sessionManager.getLeafId(); + const messagesBefore = structuredClone(harness.session.messages); + const result = await harness.session.navigateTree(targetId, { summarize: true }); + expect(result).toEqual({ cancelled: true }); + expect(harness.sessionManager.getLeafId()).toBe(leafBefore); + expect(harness.sessionManager.getEntries()).toEqual(entriesBefore); + expect(harness.session.messages).toEqual(messagesBefore); + }); +}); From 4673137cf94d1a43acbc20e320a8b3e11757c2e9 Mon Sep 17 00:00:00 2001 From: Jay Yu Date: Mon, 28 Sep 2026 16:33:46 -0400 Subject: [PATCH 8/8] fix(coding-agent): align plan guidance test - Keep the prompt assertion consistent with optional success criteria --- .../coding-agent/test/tool-system-prompt-contributions.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coding-agent/test/tool-system-prompt-contributions.test.ts b/packages/coding-agent/test/tool-system-prompt-contributions.test.ts index 93aeb3f83..7f61be299 100644 --- a/packages/coding-agent/test/tool-system-prompt-contributions.test.ts +++ b/packages/coding-agent/test/tool-system-prompt-contributions.test.ts @@ -42,7 +42,7 @@ describe("built-in tool system prompt contributions", () => { test("provides outcome-focused checklist lifecycle guidance", () => { expect(updatePlanToolSystemPromptContribution.guidelines).toEqual([ "Use update_plan for complex, ambiguous, dependent, long-running, or multi-part work, when the user requests a plan, or when checkpoints would improve collaboration; skip it for simple or immediately executable work.", - "Write short, ordered, outcome-focused steps. Give each step a stable ID and add success_criteria for subtasks needing validation.", + "Write short, ordered, outcome-focused steps. Give each step a stable ID and add optional success_criteria for subtasks needing validation.", "Include final verification when correctness requires it.", "Update by ID at meaningful milestones; retain IDs when rewording, reordering, or changing status.", "Explain material replanning, omit explanations for routine progress, and use clear: true only when abandoning the checklist.",