Skip to content

Commit 3c0dbe4

Browse files
committed
fix(reclaim): never clamp live tool executions; exclude open arcs from selectors (#250)
The arg clamp mutated the LIVE part objects OpenCode still executes from (a pending task part's input.prompt became 'Fix...[truncated]' in the child spawn), and OpenCode {type:'tool'} parts counted as having a result by TYPE even while pending/running, so open arcs were drop-eligible. Clamps now rewrite a deep clone swapped into the wire array (live object byte-identical) and hasResult requires a completed result (output present or errored). Fixes #250.
2 parents 25a1080 + eea417b commit 3c0dbe4

4 files changed

Lines changed: 389 additions & 8 deletions

File tree

packages/plugin/src/hooks/magic-context/apply-operations.tool-drop.test.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,102 @@ describe("apply operations for tool drops", () => {
150150
expect(getTagById(db, "ses-1", toolTagId!)?.dropMode).toBe("truncated");
151151
});
152152

153+
it("defers a pending/running task part and keeps its long prompt byte-identical (#250 open arc)", () => {
154+
useTempDataHome("context-tool-drop-pending-task-");
155+
const db = openDatabase();
156+
// Pre-seed a tool tag for the task call so the still-running part binds
157+
// into a drop target (mirrors a tag adopted from an earlier pass). The
158+
// part itself has NO result output yet — the child is still spawning.
159+
insertTag(db, "ses-1", "call-task", "tool", 123, 7);
160+
const tagger = createTagger();
161+
tagger.initFromDb("ses-1", db);
162+
163+
const longPrompt = `Fix the auth bug and add coverage. ${"x".repeat(600)}`;
164+
const taskPart = {
165+
type: "tool",
166+
tool: "task",
167+
callID: "call-task",
168+
state: {
169+
status: "running",
170+
input: { prompt: longPrompt, subagent_type: "mason" },
171+
},
172+
};
173+
const pristine = JSON.stringify(taskPart);
174+
const messages: MessageLike[] = [
175+
{
176+
info: { id: "m-task", role: "assistant", sessionID: "ses-1" },
177+
parts: [taskPart],
178+
},
179+
];
180+
181+
const { targets } = tagMessages("ses-1", messages, tagger, db);
182+
queuePendingOp(db, "ses-1", 7, "drop");
183+
const didMutate = applyPendingOperations("ses-1", db, targets);
184+
185+
// Open arc: the drop is deferred (not applied), the pending op stays
186+
// queued, the tag stays active, and the LIVE task part is byte-identical
187+
// — the child agent's prompt is never clamped mid-spawn.
188+
expect(didMutate).toBe(false);
189+
expect(JSON.stringify(taskPart)).toBe(pristine);
190+
expect(taskPart.state.input.prompt).toBe(longPrompt);
191+
expect(getPendingOps(db, "ses-1")).toHaveLength(1);
192+
expect(getTagById(db, "ses-1", 7)?.status).toBe("active");
193+
});
194+
195+
it("still clamps a completed task arc on the wire while preserving the live object (#250 mutation-safety)", () => {
196+
useTempDataHome("context-tool-drop-completed-task-");
197+
const db = openDatabase();
198+
const tagger = createTagger();
199+
const longPrompt = `Investigate and fix the regression. ${"y".repeat(600)}`;
200+
const taskPart = {
201+
type: "tool",
202+
tool: "task",
203+
callID: "call-bg",
204+
state: {
205+
status: "completed",
206+
input: { prompt: longPrompt, subagent_type: "mason" },
207+
output: '<task state="running">started</task>',
208+
},
209+
};
210+
const messages: MessageLike[] = [
211+
{
212+
info: { id: "m-bg", role: "assistant", sessionID: "ses-1" },
213+
parts: [taskPart],
214+
},
215+
];
216+
217+
const { targets, batch } = tagMessages("ses-1", messages, tagger, db);
218+
const toolTagId = tagger.getToolTag("ses-1", "call-bg", "m-bg");
219+
expect(toolTagId).toBeDefined();
220+
221+
// Snapshot AFTER tagging (tagMessages legitimately prefixes the output
222+
// with §N§). The reclaim pass below must not change the live object any
223+
// further — this is the byte-identity we assert.
224+
const pristine = JSON.stringify(taskPart);
225+
226+
// Within the skeleton window → truncate (skeleton) path, as before.
227+
queuePendingOp(db, "ses-1", toolTagId!, "drop");
228+
const didMutate = applyPendingOperations("ses-1", db, targets);
229+
batch.finalize();
230+
231+
// No reclaim regression: the completed arc still clamps.
232+
expect(didMutate).toBe(true);
233+
expect(getTagById(db, "ses-1", toolTagId!)?.dropMode).toBe("truncated");
234+
235+
// The wire copy now in the array is clamped + sentinelled...
236+
const wire = messages[0]?.parts[0] as {
237+
state: { input: Record<string, unknown>; output: string };
238+
};
239+
expect(wire).not.toBe(taskPart);
240+
expect(wire.state.output).toBe(`[dropped \u00a7${toolTagId}\u00a7]`);
241+
expect(wire.state.input.prompt).toBe("Inves...[truncated]");
242+
243+
// ...but the LIVE object OpenCode still holds is byte-identical (the long
244+
// prompt is intact), so a background child spawning from it is unharmed.
245+
expect(JSON.stringify(taskPart)).toBe(pristine);
246+
expect(taskPart.state.input.prompt).toBe(longPrompt);
247+
});
248+
153249
it("edit_marker: preserves filePath + region hint, freezes mode, replays byte-identically", () => {
154250
useTempDataHome("context-edit-marker-");
155251
const db = openDatabase();

packages/plugin/src/hooks/magic-context/tag-messages.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import {
2727
createToolDropTarget,
2828
extractToolCallObservation,
29+
partHasCompletedResult,
2930
type ToolCallIndex,
3031
type ToolDropResult,
3132
ToolMutationBatch,
@@ -488,7 +489,13 @@ export function tagMessages(
488489
hasResult: false,
489490
};
490491
entry.occurrences.push({ message, part, kind: toolObservation.kind });
491-
if (toolObservation.kind === "result") entry.hasResult = true;
492+
// An OpenCode `{ type: "tool" }` part is observed as a "result" by
493+
// its TYPE even while the call is still pending/running, so gate
494+
// hasResult on an ACTUAL completed result (state.output present).
495+
// This keeps open arcs out of every drop/clamp selector — a live
496+
// task part's input must never become a reclaim target.
497+
if (toolObservation.kind === "result" && partHasCompletedResult(part))
498+
entry.hasResult = true;
492499
toolCallIndex.set(compositeKey, entry);
493500

494501
const _tGetTool = performance.now();

packages/plugin/src/hooks/magic-context/tool-drop-target.test.ts

Lines changed: 206 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
createToolDropTarget,
77
extractToolCallObservation,
88
hasMeaningfulPart,
9+
partHasCompletedResult,
910
type ToolCallIndex,
1011
ToolMutationBatch,
1112
} from "./tool-drop-target";
@@ -41,7 +42,11 @@ function buildIndex(messages: MessageLike[]): ToolCallIndex {
4142
hasResult: false,
4243
};
4344
entry.occurrences.push({ message: msg, part, kind: observation.kind });
44-
if (observation.kind === "result") entry.hasResult = true;
45+
// Mirror production (tag-messages.ts): an OpenCode `{ type: "tool" }`
46+
// part is a "result" observation by type even while pending, so gate
47+
// hasResult on an actual completed result.
48+
if (observation.kind === "result" && partHasCompletedResult(part))
49+
entry.hasResult = true;
4550
index.set(observation.callId, entry);
4651
}
4752
}
@@ -89,6 +94,51 @@ describe("tool-drop-target", () => {
8994
});
9095
});
9196

97+
describe("partHasCompletedResult", () => {
98+
it("counts a completed OpenCode tool part (output string) as closed", () => {
99+
expect(
100+
partHasCompletedResult({ type: "tool", callID: "c", state: { output: "done" } }),
101+
).toBe(true);
102+
});
103+
104+
it("counts an errored OpenCode tool part (status error, no output) as closed", () => {
105+
expect(
106+
partHasCompletedResult({
107+
type: "tool",
108+
callID: "c",
109+
state: { status: "error", error: "boom", input: { content: "x".repeat(600) } },
110+
}),
111+
).toBe(true);
112+
});
113+
114+
it("keeps a running OpenCode tool part (no output, no error) open", () => {
115+
expect(
116+
partHasCompletedResult({
117+
type: "tool",
118+
callID: "c",
119+
state: { status: "running", input: { prompt: "p" } },
120+
}),
121+
).toBe(false);
122+
});
123+
124+
it("keeps a pending OpenCode tool part open", () => {
125+
expect(
126+
partHasCompletedResult({ type: "tool", callID: "c", state: { status: "pending" } }),
127+
).toBe(false);
128+
});
129+
130+
it("counts an Anthropic tool_result part as closed", () => {
131+
expect(partHasCompletedResult({ type: "tool_result", tool_use_id: "c" })).toBe(true);
132+
});
133+
134+
it("excludes invocation-shaped parts and non-records", () => {
135+
expect(partHasCompletedResult({ type: "tool-invocation", callID: "c" })).toBe(false);
136+
expect(partHasCompletedResult({ type: "tool_use", id: "c" })).toBe(false);
137+
expect(partHasCompletedResult(null)).toBe(false);
138+
expect(partHasCompletedResult({ type: "tool" })).toBe(false);
139+
});
140+
});
141+
92142
describe("createToolDropTarget", () => {
93143
describe("#given a complete invocation/result tool pair", () => {
94144
describe("#when dropping the call", () => {
@@ -303,7 +353,10 @@ describe("tool-drop-target", () => {
303353

304354
expect(hasCall(messages, "call-3")).toBe(true);
305355
expect(messages).toHaveLength(3);
306-
expect(toolPart.state as Record<string, unknown>).toEqual({
356+
357+
// The wire copies now in the message array carry the sentinel...
358+
const wireToolPart = messages[1]?.parts[0] as { state: Record<string, unknown> };
359+
expect(wireToolPart.state).toEqual({
307360
input: {
308361
query: "abcdef",
309362
short: "abc",
@@ -314,7 +367,26 @@ describe("tool-drop-target", () => {
314367
},
315368
output: "[dropped \u00a77\u00a7]",
316369
});
317-
expect(toolResultPart.content).toBe("[dropped \u00a77\u00a7]");
370+
const wireResultPart = messages[2]?.parts[0] as { content: string };
371+
expect(wireResultPart.content).toBe("[dropped \u00a77\u00a7]");
372+
373+
// ...while the LIVE part objects OpenCode handed us stay
374+
// byte-identical: the clamp swaps in a clone and never touches
375+
// the originals (mutation-safety guarantee).
376+
expect(toolPart.state as Record<string, unknown>).toEqual({
377+
input: {
378+
query: "abcdef",
379+
short: "abc",
380+
files: ["a", "b"],
381+
metadata: { nested: true },
382+
exact: true,
383+
limit: 2,
384+
},
385+
output: "old-tool",
386+
});
387+
expect(toolResultPart.content).toBe("old-result");
388+
expect(wireToolPart).not.toBe(toolPart);
389+
expect(wireResultPart).not.toBe(toolResultPart);
318390
expect(thinkingParts[0]?.thinking).toBe("[cleared]");
319391
expect(thinkingParts[1]?.text).toBe("[cleared]");
320392
});
@@ -349,14 +421,144 @@ describe("tool-drop-target", () => {
349421

350422
expect(target.truncate()).toBe("truncated");
351423

352-
expect(toolPart.state as Record<string, unknown>).toEqual({
424+
// The wire copy now in the message array carries the clamp...
425+
const wireToolPart = messages[1]?.parts[0] as { state: Record<string, unknown> };
426+
expect(wireToolPart.state).toEqual({
353427
input: {
354428
query: "xxxxx...[truncated]",
355429
files: "[2 items]",
356430
metadata: "[object]",
357431
},
358432
output: "[dropped \u00a79\u00a7]",
359433
});
434+
435+
// ...while the LIVE part object stays byte-identical (long
436+
// prompt intact) so a still-executing tool is never corrupted.
437+
expect(toolPart.state as Record<string, unknown>).toEqual({
438+
input: {
439+
query: largeQuery,
440+
files: ["a", "b"],
441+
metadata: { nested: true },
442+
},
443+
output: "old-tool",
444+
});
445+
expect(wireToolPart).not.toBe(toolPart);
446+
});
447+
});
448+
});
449+
450+
describe("#given a pending/running OpenCode tool part (no result output yet)", () => {
451+
describe("#when any drop/clamp selector runs", () => {
452+
it("#then it is treated as an open arc: never targeted and left byte-identical", () => {
453+
const longPrompt = `Fix the auth bug and add coverage. ${"x".repeat(600)}`;
454+
const taskPart = {
455+
type: "tool",
456+
tool: "task",
457+
callID: "call-task",
458+
state: {
459+
status: "running",
460+
input: { prompt: longPrompt, subagent_type: "mason" },
461+
},
462+
};
463+
const pristine = JSON.stringify(taskPart);
464+
const messages: MessageLike[] = [message("m-task", "assistant", [taskPart])];
465+
const index = buildIndex(messages);
466+
const batch = new ToolMutationBatch(messages);
467+
const target = createToolDropTarget("call-task", [], index, batch, 11);
468+
469+
// Open arc (invocation with no completed result) is excluded
470+
// from EVERY selector: canDrop, drop, truncate, editMarker.
471+
expect(target.canDrop()).toBe(false);
472+
expect(target.truncate()).toBe("incomplete");
473+
expect(target.drop()).toBe("incomplete");
474+
expect(target.editMarker()).toBe("incomplete");
475+
476+
// The live part object is byte-identical and still the same
477+
// reference in the wire array.
478+
expect(JSON.stringify(taskPart)).toBe(pristine);
479+
expect(messages[0]?.parts[0]).toBe(taskPart);
480+
});
481+
});
482+
});
483+
484+
describe("#given a completed tool part with a long argument (background task shape)", () => {
485+
describe("#when truncate runs", () => {
486+
it("#then the wire copy carries the clamp while the live object stays intact", () => {
487+
const longPrompt = `Investigate and fix the regression. ${"y".repeat(600)}`;
488+
const taskPart = {
489+
type: "tool",
490+
tool: "task",
491+
callID: "call-bg",
492+
state: {
493+
status: "completed",
494+
input: { prompt: longPrompt, subagent_type: "mason" },
495+
output: '<task state="running">started</task>',
496+
},
497+
};
498+
const pristine = JSON.stringify(taskPart);
499+
const messages: MessageLike[] = [message("m-bg", "assistant", [taskPart])];
500+
const index = buildIndex(messages);
501+
const batch = new ToolMutationBatch(messages);
502+
const target = createToolDropTarget("call-bg", [], index, batch, 12);
503+
504+
expect(target.canDrop()).toBe(true);
505+
expect(target.truncate()).toBe("truncated");
506+
507+
// The wire copy now in the array is clamped + sentinelled...
508+
const wire = messages[0]?.parts[0] as {
509+
state: { input: Record<string, unknown>; output: string };
510+
};
511+
expect(wire).not.toBe(taskPart);
512+
expect(wire.state.output).toBe("[dropped \u00a712\u00a7]");
513+
expect(wire.state.input.prompt).toBe("Inves...[truncated]");
514+
515+
// ...but the LIVE object OpenCode still holds is byte-identical
516+
// to before the reclaim pass — the long prompt is intact, so a
517+
// child agent spawning from it is never corrupted.
518+
expect(JSON.stringify(taskPart)).toBe(pristine);
519+
expect(taskPart.state.input.prompt).toBe(longPrompt);
520+
});
521+
});
522+
});
523+
524+
describe("#given an errored OpenCode tool part (status error, no output, large input)", () => {
525+
describe("#when the selectors run", () => {
526+
it("#then it IS clamp-eligible (closed arm) and the clamp stays on the wire only", () => {
527+
const bigContent = "z".repeat(600);
528+
const failedWrite = {
529+
type: "tool",
530+
tool: "write",
531+
callID: "call-err",
532+
state: {
533+
status: "error",
534+
error: "permission denied",
535+
input: { filePath: "/spec.md", content: bigContent },
536+
},
537+
};
538+
const pristine = JSON.stringify(failedWrite);
539+
const messages: MessageLike[] = [message("m-err", "assistant", [failedWrite])];
540+
const index = buildIndex(messages);
541+
const batch = new ToolMutationBatch(messages);
542+
const target = createToolDropTarget("call-err", [], index, batch, 13);
543+
544+
// Errored arm is closed → eligible for every selector (this is
545+
// the reclaim the old type-based gate also allowed; the new
546+
// gate must NOT leak it).
547+
expect(target.canDrop()).toBe(true);
548+
expect(target.truncate()).toBe("truncated");
549+
550+
// Wire copy carries the clamp + sentinel...
551+
const wire = messages[0]?.parts[0] as {
552+
state: { input: Record<string, unknown>; output: string };
553+
};
554+
expect(wire).not.toBe(failedWrite);
555+
expect(wire.state.output).toBe("[dropped \u00a713\u00a7]");
556+
expect(wire.state.input.content).toBe("zzzzz...[truncated]");
557+
558+
// ...live object stays byte-identical: reclaimed via a clone,
559+
// never by mutating the original part.
560+
expect(JSON.stringify(failedWrite)).toBe(pristine);
561+
expect(failedWrite.state.input.content).toBe(bigContent);
360562
});
361563
});
362564
});

0 commit comments

Comments
 (0)