diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b3b6d857c..14c02c8ab 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -23,6 +23,9 @@ - Fixed `startup.quiet` still rendering the `xdev: xd://: mounted …` status line when MCP tools connect; quiet startup now suppresses only the user-visible mount notice while retaining the hidden model-facing device update ([#5670](https://github.com/can1357/oh-my-pi/issues/5670)). - Fixed command error in `hub` tool with a non-POSIX shell ([#5682](https://github.com/can1357/oh-my-pi/pull/5682)) +### Fixed + +- Fixed xdev-routed checkpoint and rewind writes not tracking checkpoint state and leaving rewinding results in rebuilt provider and session context. ## [17.0.1] - 2026-07-16 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 29a318181..8348a3858 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -532,6 +532,43 @@ function reportFromRewindReportContent(content: string): string { return report.trim(); } +type SemanticCheckpointToolName = "checkpoint" | "rewind"; + +type SemanticToolResult = { + toolName: SemanticCheckpointToolName; + details?: unknown; +}; + +/** + * Normalize checkpoint/rewind results across native calls and `write xd://` + * dispatches. Xdev keeps the wrapped tool's result details under `xdev.inner`, + * while direct calls put them on the result itself. + */ +function semanticToolResult(toolName: string | undefined, result: unknown): SemanticToolResult | undefined { + if (toolName === "checkpoint" || toolName === "rewind") { + const details = result && typeof result === "object" && "details" in result ? result.details : undefined; + return { toolName, details }; + } + const dispatch = writeDeviceDispatch(toolName ?? "", result); + if (dispatch?.mode !== "execute" || (dispatch.tool !== "checkpoint" && dispatch.tool !== "rewind")) { + return undefined; + } + return { toolName: dispatch.tool, details: dispatch.inner }; +} + +function isTodoPhase(value: unknown): value is TodoPhase { + if (!isRecord(value) || typeof value.name !== "string" || !Array.isArray(value.tasks)) return false; + return value.tasks.every( + task => + isRecord(task) && + typeof task.content === "string" && + (task.status === "pending" || + task.status === "in_progress" || + task.status === "completed" || + task.status === "abandoned"), + ); +} + function completedRewindFromEntry(entry: SessionEntry): CompletedRewindState | undefined { if (entry.type !== "custom_message" || entry.customType !== "rewind-report") return undefined; const details = entry.details; @@ -544,21 +581,18 @@ function completedRewindFromEntry(entry: SessionEntry): CompletedRewindState | u reportFromRewindReportContent(customMessageContentText(entry.content)); return report.length > 0 ? { report, startedAt, rewoundAt } : undefined; } - -function isSuccessfulCheckpointEntry(entry: SessionEntry): entry is SessionMessageEntry & { - message: { role: "toolResult"; toolName: "checkpoint"; isError?: false }; -} { - return ( - entry.type === "message" && - entry.message.role === "toolResult" && - entry.message.toolName === "checkpoint" && - entry.message.isError !== true - ); +function isSuccessfulCheckpointEntry( + entry: SessionEntry, +): entry is SessionEntry & { type: "message"; message: Extract } { + if (entry.type !== "message" || entry.message.role !== "toolResult" || entry.message.isError === true) { + return false; + } + return semanticToolResult(entry.message.toolName, entry.message)?.toolName === "checkpoint"; } function checkpointStartedAtFromEntry(entry: SessionEntry): string | undefined { if (!isSuccessfulCheckpointEntry(entry)) return undefined; - const details = entry.message.details; + const details = semanticToolResult(entry.message.toolName, entry.message)?.details; if (details && typeof details === "object") { const startedAt = stringProperty(details, "startedAt"); if (startedAt) return startedAt; @@ -4012,7 +4046,7 @@ export class AgentSession { } const skipPersistedRewindResult = message.role === "toolResult" && - message.toolName === "rewind" && + semanticToolResult(message.toolName, message)?.toolName === "rewind" && this.#rewoundToolResultIds.delete(message.toolCallId); if (!skipPersistedRewindResult) { this.#appendSessionMessage(message); @@ -4383,29 +4417,28 @@ export class AgentSession { } } if (event.message.role === "toolResult") { - const { toolName, toolCallId, details, isError, content } = event.message as { - toolCallId?: string; - toolName?: string; - details?: { op?: string; path?: string; phases?: TodoPhase[]; report?: string; startedAt?: string }; - isError?: boolean; - content?: Array; - }; + const { toolName, toolCallId, isError, content } = event.message; + const details = isRecord(event.message.details) ? event.message.details : undefined; + const semanticResult = semanticToolResult(toolName, event.message); + const semanticDetails = isRecord(semanticResult?.details) ? semanticResult.details : undefined; // A tool actually ran. Clear the post-reminder suppression: the agent did // productive work in response to the prior nudge, so the next text-only stop // is allowed to escalate to the next reminder if todos remain incomplete. this.#todoReminderAwaitingProgress = false; // Invalidate streaming edit cache when edit tool completes to prevent stale data - if (toolName === "edit" && details?.path) { - this.#invalidateFileCacheForPath(details.path); + const editedPath = details ? getStringProperty(details, "path") : undefined; + if (toolName === "edit" && editedPath) { + this.#invalidateFileCacheForPath(editedPath); } - if (toolName === "todo" && !isError && Array.isArray(details?.phases)) { - this.setTodoPhases(details.phases); + const phases = details?.phases; + if (toolName === "todo" && !isError && details && Array.isArray(phases) && phases.every(isTodoPhase)) { + this.setTodoPhases(phases); if (this.#isTodoInitResult(details, toolCallId)) { this.#scheduleReplanTitleRefresh(); } } if (toolName === "todo" && isError) { - const errorText = content?.find(part => part.type === "text")?.text; + const errorText = content.find(part => part.type === "text")?.text; const reminderText = [ "", "todo failed, so todo progress is not visible to the user.", @@ -4423,18 +4456,19 @@ export class AgentSession { { deliverAs: "nextTurn" }, ); } - if (toolName === "checkpoint" && !isError) { + if (semanticResult?.toolName === "checkpoint" && !isError) { const checkpointEntryId = this.sessionManager.getEntries().at(-1)?.id ?? null; this.#checkpointState = { checkpointMessageCount: this.agent.state.messages.length, checkpointEntryId, - startedAt: details?.startedAt ?? new Date().toISOString(), + startedAt: + (semanticDetails && stringProperty(semanticDetails, "startedAt")) ?? new Date().toISOString(), }; this.#pendingRewindReport = undefined; this.#lastCompletedRewind = undefined; } - if (toolName === "rewind" && !isError && this.#checkpointState) { - const detailReport = typeof details?.report === "string" ? details.report.trim() : ""; + if (semanticResult?.toolName === "rewind" && !isError && this.#checkpointState) { + const detailReport = semanticDetails ? (stringProperty(semanticDetails, "report")?.trim() ?? "") : ""; const textReport = content?.find(part => part.type === "text")?.text?.trim() ?? ""; const report = detailReport || textReport; if (report.length > 0) { @@ -11628,8 +11662,10 @@ export class AgentSession { if (this.#pendingRewindReport) return this.#pendingRewindReport; for (let i = messages.length - 1; i >= 0; i--) { const message = messages[i]; - if (message?.role !== "toolResult" || message.toolName !== "rewind" || message.isError) continue; - const details = message.details; + if (message?.role !== "toolResult" || message.isError) continue; + const semanticResult = semanticToolResult(message.toolName, message); + if (semanticResult?.toolName !== "rewind") continue; + const details = semanticResult.details; const detailReport = details && typeof details === "object" && "report" in details && typeof details.report === "string" ? details.report.trim() @@ -11670,7 +11706,7 @@ export class AgentSession { if (activeMessages) { for (const message of activeMessages) { - if (message.role === "toolResult" && message.toolName === "rewind") { + if (message.role === "toolResult" && semanticToolResult(message.toolName, message)?.toolName === "rewind") { this.#rewoundToolResultIds.add(message.toolCallId); } } diff --git a/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts b/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts index da59ae2fe..7e3098614 100644 --- a/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts +++ b/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts @@ -16,6 +16,40 @@ import { TempDir } from "@oh-my-pi/pi-utils"; const checkpointSchema = z.object({ goal: z.string() }); const rewindSchema = z.object({ report: z.string() }); +const xdevWriteSchema = z.object({ path: z.string(), content: z.string() }); + +const xdevWriteTool: AgentTool = { + name: "write", + label: "Write", + description: "Dispatch a write to an xd:// device", + parameters: xdevWriteSchema, + async execute(_toolCallId, params) { + const tool = params.path === "xd://checkpoint" ? "checkpoint" : "rewind"; + if (/^\s*(?:\?|help)?\s*$/i.test(params.content)) { + return { + content: [{ type: "text" as const, text: `${tool} docs via xdev` }], + details: { xdev: { tool, mode: "help" } }, + }; + } + const parsed: unknown = JSON.parse(params.content); + const args = parsed && typeof parsed === "object" && !Array.isArray(parsed) ? parsed : {}; + const goal = "goal" in args && typeof args.goal === "string" ? args.goal : undefined; + const report = "report" in args && typeof args.report === "string" ? args.report : undefined; + const inner = tool === "checkpoint" ? { goal, startedAt: "2026-01-01T00:00:00.000Z" } : { report, rewound: true }; + return { + content: [{ type: "text" as const, text: `${tool} via xdev` }], + details: { + xdev: { + tool, + mode: "execute", + args, + inner, + }, + }, + }; + }, +}; + const checkpointTool: AgentTool = { name: "checkpoint", label: "Checkpoint", @@ -67,7 +101,10 @@ function signedThinking(thinking: string, thinkingSignature: string): MockConten return { type: "thinking", thinking, thinkingSignature } as unknown as MockContent; } -async function createHarness(responses: MockResponse[]): Promise { +async function createHarness( + responses: MockResponse[], + tools: AgentTool[] = [checkpointTool as AgentTool, rewindTool as AgentTool], +): Promise { const tempDir = TempDir.createSync("@pi-checkpoint-rewind-branch-"); const authStorage = await AuthStorage.create(path.join(tempDir.path(), "auth.db")); authStorage.setRuntimeApiKey("mock", "test-key"); @@ -82,8 +119,6 @@ async function createHarness(responses: MockResponse[]): Promise "test-key", initialState: { @@ -204,6 +239,88 @@ describe("AgentSession checkpoint rewind branch context", () => { expect(finalThinking?.thinkingSignature).toBe("sig_after_rewind"); }); + it("does not start checkpoint tracking for xdev help envelopes", async () => { + const { session } = await createHarness( + [ + { + content: [ + { + type: "toolCall", + id: "call_checkpoint_help", + name: "write", + arguments: { path: "xd://checkpoint", content: "help" }, + }, + ], + stopReason: "toolUse", + }, + { content: ["DONE"], stopReason: "stop" }, + ], + [xdevWriteTool], + ); + + await session.prompt("show checkpoint help"); + + expect(session.getCheckpointState()).toBeUndefined(); + }); + + it("tracks checkpoint and rewind through execute xdev write results", async () => { + const report = "findings: xdev wrapper"; + const { session, mock } = await createHarness( + [ + { + content: [ + { + type: "toolCall", + id: "call_checkpoint_xdev", + name: "write", + arguments: { + path: "xd://checkpoint", + content: JSON.stringify({ goal: "inspect" }), + }, + }, + ], + stopReason: "toolUse", + }, + { + content: [ + { + type: "toolCall", + id: "call_rewind_xdev", + name: "write", + arguments: { + path: "xd://rewind", + content: JSON.stringify({ report }), + }, + }, + ], + stopReason: "toolUse", + }, + { content: ["DONE"], stopReason: "stop" }, + ], + [xdevWriteTool], + ); + + await session.prompt("investigate with an xdev checkpoint"); + + const finalCall = mock.calls.at(-1); + if (!finalCall) throw new Error("Expected final post-rewind provider call"); + expect( + finalCall.context.messages.some( + message => message.role === "toolResult" && message.toolCallId === "call_rewind_xdev", + ), + ).toBe(false); + expect( + session.messages.some(message => message.role === "toolResult" && message.toolCallId === "call_rewind_xdev"), + ).toBe(false); + expect(session.messages).toEqual(session.sessionManager.buildSessionContext().messages); + + expect(session.getLastCompletedRewind()).toEqual({ + report, + startedAt: "2026-01-01T00:00:00.000Z", + rewoundAt: expect.any(String), + }); + }); + it("rehydrates completed rewind state from the retained report on resume", async () => { const report = "findings: retained after resume"; const harness = await createHarness([ @@ -419,4 +536,95 @@ describe("AgentSession checkpoint rewind branch context", () => { true, ); }); + it("rehydrates an active checkpoint from an xdev write after branching and resume", async () => { + const harness = await createHarness( + [ + { + content: [ + { + type: "toolCall", + id: "call_checkpoint_xdev", + name: "write", + arguments: { + path: "xd://checkpoint", + content: JSON.stringify({ goal: "inspect" }), + }, + }, + ], + stopReason: "toolUse", + }, + { + content: [ + { + type: "toolCall", + id: "call_rewind_xdev", + name: "write", + arguments: { + path: "xd://rewind", + content: JSON.stringify({ report: "findings" }), + }, + }, + ], + stopReason: "toolUse", + }, + { content: ["DONE"], stopReason: "stop" }, + ], + [xdevWriteTool], + ); + await harness.session.prompt("investigate with an xdev checkpoint"); + + const checkpointEntry = harness.session.sessionManager.getBranch().find(entry => { + if (entry.type !== "message" || entry.message.role !== "toolResult" || entry.message.toolName !== "write") { + return false; + } + const details = entry.message.details as { xdev?: { tool?: string } } | undefined; + return details?.xdev?.tool === "checkpoint"; + }); + if (!checkpointEntry) throw new Error("Expected xdev checkpoint tool result entry"); + harness.session.sessionManager.branch(checkpointEntry.id); + + const reloadedMock = createMockModel({ responses: [] }); + const reloadedSettings = Settings.isolated({ + "compaction.enabled": false, + "retry.enabled": false, + "todo.enabled": false, + "todo.eager": "default", + "todo.reminders": false, + }); + reloadedSettings.setModelRole("default", `${reloadedMock.provider}/${reloadedMock.id}`); + const reloadedTools = [xdevWriteTool as AgentTool]; + const reloadedAgent = new Agent({ + getApiKey: () => "test-key", + initialState: { + model: reloadedMock, + systemPrompt: ["Test"], + tools: reloadedTools, + messages: harness.session.sessionManager.buildSessionContext().messages, + }, + convertToLlm, + streamFn: reloadedMock.stream, + }); + const reloadedSession = new AgentSession({ + agent: reloadedAgent, + sessionManager: harness.session.sessionManager, + settings: reloadedSettings, + modelRegistry: new ModelRegistry( + harness.authStorage, + path.join(harness.tempDir.path(), "models-xdev-reloaded.yml"), + ), + toolRegistry: new Map(reloadedTools.map(tool => [tool.name, tool])), + }); + harness.extraSessions.push(reloadedSession); + + expect(reloadedSession.getCheckpointState()).toMatchObject({ + checkpointEntryId: checkpointEntry.id, + startedAt: "2026-01-01T00:00:00.000Z", + }); + expect(reloadedSession.getLastCompletedRewind()).toBeUndefined(); + await expect( + rewindToolForSession(reloadedSession).execute("call_rewind_after_xdev_resume", { + report: "post-resume findings", + }), + ).resolves.toMatchObject({ details: { report: "post-resume findings", rewound: true } }); + }); });