Skip to content

Commit 72d652a

Browse files
mason: fix pi todowrite stale args rendering
Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent 03b8846 commit 72d652a

5 files changed

Lines changed: 250 additions & 13 deletions

File tree

packages/pi-plugin/src/index.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,10 @@ import {
144144
import { withTimeout } from "./timeout";
145145
import { registerMagicContextTools } from "./tools";
146146
import {
147+
parseTodos,
147148
registerTodoOverlay,
148149
registerTodoStateLifecycle,
150+
rememberTodowriteToolCallTodos,
149151
setTodoSnapshot,
150152
} from "./tools/todo-view-pi";
151153

@@ -199,7 +201,7 @@ type TodoOverlayUpdater = { update: (sessionId?: string) => void };
199201

200202
type CompatiblePiTodoCapture = {
201203
normalized: string;
202-
todos: unknown[];
204+
todos: Exclude<ReturnType<typeof parseTodos>, null>;
203205
};
204206

205207
function getCompatiblePiTodoCapture(
@@ -208,7 +210,9 @@ function getCompatiblePiTodoCapture(
208210
if (!Array.isArray(todos)) return null;
209211
const normalized = normalizeTodoStateJson(todos);
210212
if (normalized === null) return null;
211-
return { normalized, todos };
213+
const parsed = parseTodos(todos);
214+
if (parsed === null) return null;
215+
return { normalized, todos: parsed };
212216
}
213217

214218
function applyCompatiblePiTodoCapture(args: {
@@ -217,8 +221,10 @@ function applyCompatiblePiTodoCapture(args: {
217221
todowriteEnabled: boolean;
218222
todoOverlay?: TodoOverlayUpdater;
219223
persist: boolean;
224+
toolCallId?: string;
220225
capture: CompatiblePiTodoCapture;
221226
}): void {
227+
rememberTodowriteToolCallTodos(args.toolCallId, args.capture.todos);
222228
if (args.todowriteEnabled) {
223229
setTodoSnapshot(args.sessionId, args.capture.todos);
224230
args.todoOverlay?.update(args.sessionId);
@@ -233,7 +239,8 @@ function applyCompatiblePiTodoCapture(args: {
233239
/**
234240
* Capture a `todowrite` args.todos payload only when it matches Magic Context's
235241
* exact todo enum contract. Third-party Pi extensions can reuse the same tool
236-
* name, so incompatible shapes must leave `last_todo_state` untouched.
242+
* name, so incompatible shapes must not update `last_todo_state` or the
243+
* transcript render cache.
237244
*/
238245
export function capturePiTodowriteArgsIfCompatible(args: {
239246
db: ContextDatabase;
@@ -242,6 +249,7 @@ export function capturePiTodowriteArgsIfCompatible(args: {
242249
todowriteEnabled: boolean;
243250
todoOverlay?: TodoOverlayUpdater;
244251
persist: boolean;
252+
toolCallId?: string;
245253
}): boolean {
246254
const capture = getCompatiblePiTodoCapture(args.todos);
247255
if (capture === null) return false;
@@ -1597,6 +1605,10 @@ export default async function (pi: ExtensionAPI): Promise<void> {
15971605
const todoArgs = event.args as
15981606
| { todos?: Array<{ status?: string }> }
15991607
| undefined;
1608+
const toolCallId =
1609+
typeof (event as { toolCallId?: unknown }).toolCallId === "string"
1610+
? (event as { toolCallId: string }).toolCallId
1611+
: undefined;
16001612
const todos = todoArgs?.todos;
16011613
const sessionMeta = Array.isArray(todos)
16021614
? getOrCreateSessionMeta(db, sessionId)
@@ -1607,8 +1619,9 @@ export default async function (pi: ExtensionAPI): Promise<void> {
16071619
// state on EVERY todowrite call so the transform-time
16081620
// injection path in pi-pipeline.ts always has a current
16091621
// snapshot to replay on the next cache-busting pass.
1610-
// Cache-safe: this is a pure DB write with no message
1611-
// mutation. Subagents skip — they do not get synthetic
1622+
// Render-safe: this only stores validated todos in shared
1623+
// session state and the local tool-call cache; it does not
1624+
// mutate Pi messages. Subagents skip — they do not get synthetic
16121625
// todowrite injection. Foreign Pi extensions can share the
16131626
// `todowrite` name, so only the exact Magic Context todo
16141627
// shape updates the stored snapshot.
@@ -1619,6 +1632,7 @@ export default async function (pi: ExtensionAPI): Promise<void> {
16191632
todowriteEnabled,
16201633
todoOverlay,
16211634
persist: Boolean(sessionMeta && !sessionMeta.isSubagent),
1635+
toolCallId,
16221636
});
16231637

16241638
if (

packages/pi-plugin/src/todo-capture-pi.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,42 @@ import { assistantToolCall, createTestDb } from "./test-utils.test";
1313
import {
1414
__resetTodoSnapshotsForTests,
1515
getTodoSnapshot,
16+
renderTodowriteCall,
1617
} from "./tools/todo-view-pi";
1718

19+
const identityTheme = {
20+
fg: (_color: string, text: string) => text,
21+
bold: (text: string) => text,
22+
} as never;
23+
1824
beforeEach(() => {
1925
__resetTodoSnapshotsForTests();
2026
});
2127

2228
describe("Pi todowrite capture compatibility", () => {
29+
it("tool_execution_start capture seeds the render cache for compatible payloads", () => {
30+
const db = createTestDb();
31+
try {
32+
const captured = capturePiTodowriteArgsIfCompatible({
33+
db,
34+
sessionId: "ses-args-cache",
35+
todos: [{ content: "Build", status: "in_progress" }],
36+
todowriteEnabled: true,
37+
persist: true,
38+
toolCallId: "call-cache",
39+
});
40+
41+
expect(captured).toBe(true);
42+
expect(
43+
renderTodowriteCall({}, identityTheme, {
44+
toolCallId: "call-cache",
45+
} as never).render(80)[0],
46+
).toContain("Todos — 1 active");
47+
} finally {
48+
closeQuietly(db);
49+
}
50+
});
51+
2352
it("tool_execution_start capture ignores foreign status values", () => {
2453
const db = createTestDb();
2554
const previousState =
@@ -35,6 +64,7 @@ describe("Pi todowrite capture compatibility", () => {
3564
todowriteEnabled: true,
3665
todoOverlay: { update: () => overlayUpdates++ },
3766
persist: true,
67+
toolCallId: "call-foreign",
3868
});
3969

4070
expect(captured).toBe(false);
@@ -43,6 +73,11 @@ describe("Pi todowrite capture compatibility", () => {
4373
);
4474
expect(getTodoSnapshot("ses-args").todos).toEqual([]);
4575
expect(overlayUpdates).toBe(0);
76+
expect(
77+
renderTodowriteCall({}, identityTheme, {
78+
toolCallId: "call-foreign",
79+
} as never).render(80)[0],
80+
).toContain("Todos — 0 active");
4681
} finally {
4782
closeQuietly(db);
4883
}

packages/pi-plugin/src/tools/todo-view-pi.test.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
getTodoSnapshot,
99
registerTodoOverlay,
1010
registerTodosCommand,
11+
rememberTodowriteToolCallTodos,
1112
setTodoSnapshot,
1213
TodoOverlay,
1314
} from "./todo-view-pi";
@@ -89,6 +90,129 @@ describe("todowrite tool rendering", () => {
8990
renderCall({ todos } as never, identityTheme, {} as never).render(80)[0],
9091
).toContain("Todos — 3 active");
9192
});
93+
it("uses cached todos when the transcript component was created with empty args", () => {
94+
const tool = createTodowriteTool();
95+
const renderCall = tool.renderCall;
96+
if (!renderCall) throw new Error("todowrite renderCall missing");
97+
const todos = [
98+
{ id: "1", content: "Work", status: "in_progress" as const },
99+
...Array.from({ length: 9 }, (_, index) => ({
100+
id: String(index + 2),
101+
content: `Done ${index + 2}`,
102+
status: "completed" as const,
103+
})),
104+
];
105+
rememberTodowriteToolCallTodos("call-stale", todos);
106+
expect(
107+
renderCall({}, identityTheme, {
108+
toolCallId: "call-stale",
109+
} as never).render(80)[0],
110+
).toContain("Todos — 1 active");
111+
});
112+
it("falls back to args when no cached tool-call todos exist", () => {
113+
const tool = createTodowriteTool();
114+
const renderCall = tool.renderCall;
115+
if (!renderCall) throw new Error("todowrite renderCall missing");
116+
const todos = [
117+
{ id: "1", content: "Plan", status: "pending" as const },
118+
{ id: "2", content: "Build", status: "in_progress" as const },
119+
{ id: "3", content: "Done", status: "completed" as const },
120+
];
121+
expect(
122+
renderCall({ todos } as never, identityTheme, {
123+
toolCallId: "call-miss",
124+
} as never).render(80)[0],
125+
).toContain("Todos — 2 active");
126+
});
127+
it("renders identically when cache and args already agree", () => {
128+
const tool = createTodowriteTool();
129+
const renderCall = tool.renderCall;
130+
if (!renderCall) throw new Error("todowrite renderCall missing");
131+
const todos = [
132+
{ id: "1", content: "Plan", status: "pending" as const },
133+
{ id: "2", content: "Done", status: "completed" as const },
134+
];
135+
rememberTodowriteToolCallTodos("call-equal", todos);
136+
expect(
137+
renderCall({ todos } as never, identityTheme, {
138+
toolCallId: "call-equal",
139+
} as never).render(80),
140+
).toEqual(
141+
renderCall({ todos } as never, identityTheme, {
142+
toolCallId: "call-equal-miss",
143+
} as never).render(80),
144+
);
145+
});
146+
it("uses cached todos for stale-empty results", () => {
147+
const tool = createTodowriteTool();
148+
const renderResult = tool.renderResult;
149+
if (!renderResult) throw new Error("todowrite renderResult missing");
150+
const todos = [
151+
{ id: "1", content: "Work", status: "in_progress" as const },
152+
{ id: "2", content: "Done", status: "completed" as const },
153+
];
154+
rememberTodowriteToolCallTodos("call-stale-result", todos);
155+
const rendered = renderResult(
156+
{
157+
details: { todos: [] },
158+
content: [{ type: "text", text: "[]" }],
159+
},
160+
{ expanded: false, isPartial: false },
161+
identityTheme,
162+
{ toolCallId: "call-stale-result" } as never,
163+
)
164+
.render(120)
165+
.join("\n");
166+
expect(rendered).toContain("◐ #1 Work");
167+
expect(rendered).toContain("✓ #2 Done");
168+
});
169+
it("prefers parsed tool results over cached todos when both are present", () => {
170+
const tool = createTodowriteTool();
171+
const renderResult = tool.renderResult;
172+
if (!renderResult) throw new Error("todowrite renderResult missing");
173+
rememberTodowriteToolCallTodos("call-real-result", [
174+
{ id: "stale", content: "Stale", status: "pending" },
175+
]);
176+
const rendered = renderResult(
177+
{
178+
details: {
179+
todos: [{ id: "fresh", content: "Fresh", status: "pending" }],
180+
},
181+
content: [{ type: "text", text: "[]" }],
182+
},
183+
{ expanded: false, isPartial: false },
184+
identityTheme,
185+
{ toolCallId: "call-real-result" } as never,
186+
)
187+
.render(120)
188+
.join("\n");
189+
expect(rendered).toContain("○ #fresh Fresh");
190+
expect(rendered).not.toContain("Stale");
191+
});
192+
it("evicts the oldest cached tool-call todos after the cap", () => {
193+
const tool = createTodowriteTool();
194+
const renderCall = tool.renderCall;
195+
if (!renderCall) throw new Error("todowrite renderCall missing");
196+
for (let index = 1; index <= 51; index++) {
197+
rememberTodowriteToolCallTodos(`call-${index}`, [
198+
{
199+
id: String(index),
200+
content: `Task ${index}`,
201+
status: "pending",
202+
},
203+
]);
204+
}
205+
expect(
206+
renderCall({}, identityTheme, { toolCallId: "call-1" } as never).render(
207+
80,
208+
)[0],
209+
).toContain("Todos — 0 active");
210+
expect(
211+
renderCall({}, identityTheme, { toolCallId: "call-51" } as never).render(
212+
80,
213+
)[0],
214+
).toContain("Todos — 1 active");
215+
});
92216
it("caps result rows at 12 with a +N more tail", async () => {
93217
const tool = createTodowriteTool();
94218
const todos = Array.from({ length: 14 }, (_, index) => ({
@@ -258,10 +382,16 @@ describe("TodoOverlay lifecycle", () => {
258382
expect(widget.render(120)).toEqual([]);
259383
});
260384
it("seeds from session_meta.last_todo_state and clears on session switch", async () => {
385+
const tool = createTodowriteTool();
386+
const renderCall = tool.renderCall;
387+
if (!renderCall) throw new Error("todowrite renderCall missing");
261388
const handlers = new Map<
262389
string,
263390
(event: unknown, ctx: unknown) => Promise<void> | void
264391
>();
392+
rememberTodowriteToolCallTodos("call-session-switch", [
393+
{ id: "cached", content: "Cached", status: "pending" },
394+
]);
265395
registerTodoOverlay(
266396
{
267397
on: (event, handler) => handlers.set(event, handler as never),
@@ -293,6 +423,11 @@ describe("TodoOverlay lifecycle", () => {
293423
"magic-context-todos",
294424
undefined,
295425
]);
426+
expect(
427+
renderCall({}, identityTheme, {
428+
toolCallId: "call-session-switch",
429+
} as never).render(80)[0],
430+
).toContain("Todos — 0 active");
296431
});
297432
it("caps content rows with a +N more tail", () => {
298433
setTodoSnapshot(

0 commit comments

Comments
 (0)