History

fix(server): preserve tool lifecycle identity (#7151)

maria committed 2 months ago

Commit b2e2ccf1 parent 792a1404 files · +96 −18Browse files

Unified · Split

4 files
  1. apps/server/src/orchestration/ActivityPayloadProjection.test.ts +40 −1
  2. apps/server/src/orchestration/ActivityPayloadProjection.ts +27 −9
  3. apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts +23 −8
  4. apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts +6 −0
apps/server/src/orchestration/ActivityPayloadProjection.test.ts+40−1
@@ -20,7 +20,7 @@ function activity(payload: Record<string, unknown>): OrchestrationThreadActivity · 19 unmodified lines
2020 * If slimming ever moves to an allowlist over the whole payload, these
2121 * assertions are the tripwire.
2222 */
23−describe("projectActivityPayload agent-field survival", () => {
23+describe("projectActivityPayload", () => {
2424 it("preserves tool attribution (agentId/parentToolUseId) through data slimming", () => {
2525 const projected = projectActivityPayload(
2626 activity({
@@ -97,6 +97,45 @@ describe("projectActivityPayload agent-field survival", () => { · 70 unmodified lines
9797 expect(JSON.stringify(acp.payload).length).toBeLessThan(500);
9898 });
9999
100+ it("normalizes Claude and OpenCode command inputs before slimming provider data", () => {
101+ const claude = projectActivityPayload(
102+ activity({
103+ itemType: "command_execution",
104+ toolCallId: "claude-call-1",
105+ data: {
106+ toolName: "Bash",
107+ input: { command: "vp test run" },
108+ result: { content: "x".repeat(5_000) },
109+ },
110+ }),
111+ );
112+ const openCode = projectActivityPayload(
113+ activity({
114+ itemType: "command_execution",
115+ toolCallId: "opencode-call-1",
116+ data: {
117+ tool: "bash",
118+ state: {
119+ status: "running",
120+ input: { command: "vp lint" },
121+ output: "x".repeat(5_000),
122+ },
123+ },
124+ }),
125+ );
126+
127+ expect(claude.payload).toMatchObject({
128+ toolCallId: "claude-call-1",
129+ data: { command: "vp test run" },
130+ });
131+ expect(openCode.payload).toMatchObject({
132+ toolCallId: "opencode-call-1",
133+ data: { command: "vp lint" },
134+ });
135+ expect(JSON.stringify(claude.payload).length).toBeLessThan(200);
136+ expect(JSON.stringify(openCode.payload).length).toBeLessThan(200);
137+ });
138+
100139 it("slims Codex-shaped mcp_tool_call items to rendered fields plus a result summary", () => {
101140 const projected = projectActivityPayload(
102141 activity({
apps/server/src/orchestration/ActivityPayloadProjection.ts+27−9
@@ -125,6 +125,24 @@ function projectCommandData(data: Record<string, unknown>): Record<string, unkno · 124 unmodified lines
125125 return Object.keys(projectedItem).length > 0 ? projectedItem : undefined;
126126 }
127127
128+function projectCommandValue(data: Record<string, unknown>): unknown {
129+ if (data.command !== undefined) {
130+ return data.command;
131+ }
132+
133+ const input = asRecord(data.input);
134+ if (input?.command !== undefined) {
135+ return input.command;
136+ }
137+
138+ const stateInput = asRecord(asRecord(data.state)?.input);
139+ if (stateInput?.command !== undefined) {
140+ return stateInput.command;
141+ }
142+
143+ return undefined;
144+}
145+
128146 function summarizeToolTextOutput(value: string): string | null {
129147 const lines: string[] = [];
130148 for (const rawLine of value.split(/\r?\n/u)) {
@@ -339,8 +357,9 @@ export function projectActivityPayload( · 208 unmodified lines
339357 if (item) {
340358 projectedData.item = item;
341359 }
342− if ("command" in data) {
343− projectedData.command = data.command;
360+ const command = projectCommandValue(data);
361+ if (command !== undefined) {
362+ projectedData.command = command;
344363 }
345364
346365 const changedFiles: string[] = [];
@@ -418,12 +437,10 @@ function dropStaleContextWindowActivities( · 71 unmodified lines
418437 }
419438
420439 /**
421− * Identity both clients use to fold a tool lifecycle row into the call it
422− * belongs to (`deriveToolLifecycleCollapseKey` in web's `session-logic` and
423− * mobile's `threadActivity`): an explicit `data.toolCallId` when the adapter
424− * emits one, otherwise the itemType/title/detail triple. Returns null for rows
425− * with no identity at all — those never collapse on the client either, so they
426− * must not be dropped here.
440+ * Identity used to retain only the newest lifecycle row for each call in a
441+ * thread snapshot. Prefer the runtime item id, then the legacy nested id, and
442+ * finally the itemType/title/detail triple. Rows without any identity remain
443+ * untouched.
427444 */
428445 function toolLifecycleIdentity(activity: OrchestrationThreadActivity): string | null {
429446 const payload = asRecord(activity.payload);
@@ -431,7 +448,8 @@ function toolLifecycleIdentity(activity: OrchestrationThreadActivity): string | · 1 unmodified line
431448 return null;
432449 }
433450
434− const toolCallId = asTrimmedString(asRecord(payload.data)?.toolCallId);
451+ const toolCallId =
452+ asTrimmedString(payload.toolCallId) ?? asTrimmedString(asRecord(payload.data)?.toolCallId);
435453 if (toolCallId) {
436454 return `id:${toolCallId}`;
437455 }
apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts+23−8
@@ -2815,11 +2815,16 @@ describe("ProviderRuntimeIngestion", () => { · 2814 unmodified lines
28152815 createdAt: now,
28162816 threadId: asThreadId("thread-1"),
28172817 turnId: asTurnId("turn-9"),
2818+ itemId: asItemId("tool-call-9"),
28182819 payload: {
28192820 itemType: "command_execution",
2820− status: "in_progress",
2821− title: "Read file",
2822− detail: "/tmp/file.ts",
2821+ status: "inProgress",
2822+ title: "Command run",
2823+ detail: "Bash: vp test run",
2824+ data: {
2825+ toolName: "Bash",
2826+ input: { command: "vp test run" },
2827+ },
28232828 },
28242829 });
28252830
@@ -2834,11 +2839,20 @@ describe("ProviderRuntimeIngestion", () => { · 8 unmodified lines
28342839 );
28352840
28362841 expect(thread.session?.status).toBe("ready");
2837− expect(
2838− thread.activities.some(
2839− (activity: ProviderRuntimeTestActivity) => activity.kind === "tool.started",
2840− ),
2841− ).toBe(true);
2842+ const activity = thread.activities.find(
2843+ (entry: ProviderRuntimeTestActivity) => entry.kind === "tool.started",
2844+ );
2845+ const payload = activity?.payload as Record<string, unknown> | undefined;
2846+ expect(payload).toMatchObject({
2847+ itemType: "command_execution",
2848+ toolCallId: "tool-call-9",
2849+ status: "inProgress",
2850+ detail: "Bash: vp test run",
2851+ data: {
2852+ toolName: "Bash",
2853+ input: { command: "vp test run" },
2854+ },
2855+ });
28422856 });
28432857
28442858 it("consumes P1 runtime events into thread metadata, diff checkpoints, and activities", async () => {
@@ -2956,6 +2970,7 @@ describe("ProviderRuntimeIngestion", () => { · 111 unmodified lines
29562970 expect(toolUpdate?.kind).toBe("tool.updated");
29572971 expect(toolUpdatePayload?.itemType).toBe("command_execution");
29582972 expect(toolUpdatePayload?.status).toBe("in_progress");
2973+ expect(toolUpdatePayload?.toolCallId).toBe("item-p1-tool");
29592974
29602975 const warning = thread.activities.find(
29612976 (activity: ProviderRuntimeTestActivity) => activity.id === "evt-runtime-warning",
apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts+6−0
@@ -803,6 +803,7 @@ export function runtimeEventToActivities( · 802 unmodified lines
803803 summary: event.payload.title ?? "Tool updated",
804804 payload: {
805805 itemType: event.payload.itemType,
806+ ...(event.itemId !== undefined ? { toolCallId: event.itemId } : {}),
806807 ...(event.payload.status ? { status: event.payload.status } : {}),
807808 ...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}),
808809 ...(event.payload.data !== undefined ? { data: event.payload.data } : {}),
@@ -830,6 +831,8 @@ export function runtimeEventToActivities( · 21 unmodified lines
830831 summary: event.payload.title ?? "Tool",
831832 payload: {
832833 itemType: event.payload.itemType,
834+ ...(event.itemId !== undefined ? { toolCallId: event.itemId } : {}),
835+ ...(event.payload.status ? { status: event.payload.status } : {}),
833836 ...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}),
834837 ...(event.payload.data !== undefined ? { data: event.payload.data } : {}),
835838 ...(event.payload.agentId ? { agentId: event.payload.agentId } : {}),
@@ -856,7 +859,10 @@ export function runtimeEventToActivities( · 20 unmodified lines
856859 summary: `${event.payload.title ?? "Tool"} started`,
857860 payload: {
858861 itemType: event.payload.itemType,
862+ ...(event.itemId !== undefined ? { toolCallId: event.itemId } : {}),
863+ ...(event.payload.status ? { status: event.payload.status } : {}),
859864 ...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}),
865+ ...(event.payload.data !== undefined ? { data: event.payload.data } : {}),
860866 ...(event.payload.agentId ? { agentId: event.payload.agentId } : {}),
861867 ...(event.payload.parentToolUseId
862868 ? { parentToolUseId: event.payload.parentToolUseId }