merge PR #5683 via eval/pr-5683: style(session): satisfy xdev rewind checks
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<AgentMessage, { role: "toolResult" }> } {
|
||||
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<TextContent | ImageContent>;
|
||||
};
|
||||
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 = [
|
||||
"<system-reminder>",
|
||||
"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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<typeof xdevWriteSchema, unknown> = {
|
||||
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<typeof checkpointSchema, { startedAt: string }> = {
|
||||
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<Harness & { mock: MockModel }> {
|
||||
async function createHarness(
|
||||
responses: MockResponse[],
|
||||
tools: AgentTool[] = [checkpointTool as AgentTool, rewindTool as AgentTool],
|
||||
): Promise<Harness & { mock: MockModel }> {
|
||||
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<Harness & { moc
|
||||
"todo.reminders": false,
|
||||
});
|
||||
settings.setModelRole("default", `${mock.provider}/${mock.id}`);
|
||||
|
||||
const tools = [checkpointTool as AgentTool, rewindTool as AgentTool];
|
||||
const agent = new Agent({
|
||||
getApiKey: () => "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 } });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user