Skip to content

Commit 36bb26c

Browse files
sambhavthakkarsambhavthakkar
authored
Recover Codex commands for bare Shell rows (#581)
* Recover Codex commands for bare Shell rows Codex labels a path-less listing (`rg --files -g AGENTS.md`) as a `listFiles` action with no path. That derived the bare title "List", which the transcript then collapsed to "Shell" because no path was left to show — the row read "Shell" between two commands that named themselves. Fall through to the command when a listing has no path, and to the intent inferred from it. Rows already saved that way are repaired from Codex's own rollout file, which records every command under its exec item id. Both sides read one set of key spellings and pick a launcher before deciding which argv element is the script, so `git -c core.editor=vim status` is not mistaken for a shell wrapper and a quoted `pwsh.exe` path still resolves. A recovered row is then labelled exactly as it would have rendered live. * Prefer the saved command over the rollout file Reading the command back off disk could write a secret into the transcript store that Codex had kept out of the row it showed. Take the command from the row itself - it is there, because Codex sends it with the item and the shell preview already stores it - so whatever Codex chose to show stays what we show. The rollout file stays as a last resort for a row that never got a preview, where the command is genuinely gone from the session. The saved pass runs first, so a session Codex labelled itself reads no disk at all. * Keep a session openable when its repair cannot be persisted The relabel pass wrote through upsertSession without a guard, so a rejected write failed the whole getSession and the reader lost the session over a cosmetic fix. Every other restoration write in this function is already inside a try; this one was not. Keep the repair in memory when the write fails and let the next load retry it. * Recover Codex Shell rows without the rollout file Codex stores older sessions as .jsonl.zst, which find_codex_rollout_file never matched, so the reader failed silently on the very sessions it was written to repair. It also copied raw commands off disk into the transcript store, which can put back a secret that Codex had kept off the row it showed us. The command is already on the row — Codex sends it with the item and shellCommandPreview keeps it as the preview title — so the saved preview is the whole recovery. A row saved without a usable one has no command left to find and keeps its placeholder. Removes codex_shell_commands and its frontend wiring. The live-label fix and the parity test that pins a repaired row to the live label both stay; the parsed-command key order is now read in one place, as only the live path is left.
1 parent 4bb4a00 commit 36bb26c

5 files changed

Lines changed: 493 additions & 24 deletions

File tree

‎src/features/sessions/data/sessionStore.test.ts‎

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
import { appendUser } from "../../../integrations/harness/core/apply";
1+
import { appendUser, applyHarnessEvents } from "../../../integrations/harness/core/apply";
22
import { describe, expect, it } from "vitest";
3+
import { mapCodexNotification } from "../../../integrations/harness/providers/codex/codexProtocol";
4+
import { toolCallLabel } from "../model/transcriptActivity";
35
import {
46
newSession,
57
type Block,
@@ -8,6 +10,7 @@ import {
810
} from "../model/session";
911
import {
1012
backfillClaudeShellCommands,
13+
backfillCodexShellCommands,
1114
isPersistableId,
1215
persistFingerprint,
1316
sanitizeSessionForPersist,
@@ -56,6 +59,144 @@ describe("Claude Shell row recovery", () => {
5659
});
5760
});
5861

62+
describe("Codex Shell row recovery", () => {
63+
it("relabels from the command saved on the row, keeping redactions", () => {
64+
// The command Codex sent with the item is already on the row as its preview
65+
// title. Reading it back means whatever Codex redacted stays redacted.
66+
const redacted = "/usr/bin/zsh -lc 'curl -H \"token=[redacted]\" example'";
67+
const blocks: Block[] = [
68+
{
69+
id: "shell",
70+
role: "tool",
71+
text: "Shell",
72+
tool: {
73+
callId: "exec-1",
74+
title: "Shell",
75+
kind: "execute",
76+
preview: { kind: "shell", title: redacted },
77+
},
78+
},
79+
];
80+
const repaired = backfillCodexShellCommands(blocks);
81+
expect(repaired[0].text).not.toBe("Shell");
82+
expect(repaired[0].tool?.preview?.title).toContain("[redacted]");
83+
});
84+
85+
it("leaves a row with no usable saved command as it is", () => {
86+
// A weak preview title names no command, so there is nothing to relabel
87+
// from and the row keeps its placeholder.
88+
const blocks: Block[] = [
89+
{
90+
id: "shell",
91+
role: "tool",
92+
text: "Shell",
93+
tool: {
94+
callId: "exec-1",
95+
title: "Shell",
96+
kind: "execute",
97+
preview: { kind: "shell", title: "Shell" },
98+
},
99+
},
100+
];
101+
expect(backfillCodexShellCommands(blocks)).toBe(blocks);
102+
});
103+
104+
it("labels placeholder rows with the saved command and rebuilds the preview", () => {
105+
const blocks: Block[] = [
106+
{
107+
id: "shell",
108+
role: "tool",
109+
text: "Shell",
110+
tool: {
111+
callId: "exec-1",
112+
title: "Shell",
113+
kind: "execute",
114+
status: "failed",
115+
detail: "exit 1",
116+
preview: {
117+
kind: "shell",
118+
title: "rg --files -g AGENTS.md -g '!node_modules'",
119+
},
120+
},
121+
},
122+
{
123+
id: "read",
124+
role: "tool",
125+
text: "Read file.ts",
126+
tool: { callId: "exec-2", kind: "read" },
127+
},
128+
];
129+
const repaired = backfillCodexShellCommands(blocks);
130+
expect(repaired[0]).toMatchObject({
131+
text: "Find files",
132+
tool: {
133+
title: "Find files",
134+
status: "failed",
135+
detail: "exit 1",
136+
preview: { kind: "shell", title: "rg --files -g AGENTS.md -g '!node_modules'" },
137+
},
138+
});
139+
expect(repaired[1]).toBe(blocks[1]);
140+
expect(backfillCodexShellCommands(repaired)).toBe(repaired);
141+
});
142+
143+
it("keeps the raw command when no readable intent is inferred", () => {
144+
const blocks: Block[] = [
145+
{
146+
id: "shell",
147+
role: "tool",
148+
text: "Shell",
149+
tool: {
150+
callId: "exec-3",
151+
title: "Shell",
152+
kind: "execute",
153+
preview: { kind: "shell", title: "git commit -m 'Fix shell labels'" },
154+
},
155+
},
156+
];
157+
const repaired = backfillCodexShellCommands(blocks);
158+
expect(repaired[0].text).toBe("git commit -m 'Fix shell labels'");
159+
});
160+
161+
// A row repaired from the saved preview has to read the same as one rendered
162+
// live, or reopening a session would relabel work the user already saw.
163+
it("labels a recovered row exactly as the live item does", () => {
164+
// Captured from `codex app-server`: the reported session's middle row was
165+
// `rg --files -g AGENTS.md`, which Codex labels a path-less `listFiles`.
166+
const item = {
167+
type: "commandExecution",
168+
id: "exec-88885872",
169+
status: "inProgress",
170+
command: `/usr/bin/zsh -lc "rg --files -g AGENTS.md -g '"'"'!node_modules'"'"'"`,
171+
commandActions: [
172+
{ type: "listFiles", command: "rg --files -g AGENTS.md -g '!node_modules'", path: null },
173+
],
174+
};
175+
let live = newSession("codex", "/home/me/proj");
176+
live = applyHarnessEvents(live, mapCodexNotification("item/started", { item }).events);
177+
const liveRow = live.blocks[0];
178+
179+
// The same row as the buggy build saved it. No recovered map: the command
180+
// is already on the row, which is how it reads in production.
181+
const saved: Block[] = [
182+
{
183+
id: "e84ab067",
184+
role: "tool",
185+
text: "Shell",
186+
tool: { ...liveRow.tool, title: "Shell" },
187+
},
188+
];
189+
const [recovered] = backfillCodexShellCommands(saved);
190+
191+
expect(recovered.text).not.toBe("Shell");
192+
expect(recovered.text).toBe(liveRow.text);
193+
expect(recovered.tool?.title).toBe(liveRow.tool?.title);
194+
expect(toolCallLabel(recovered, "/home/me/proj")).toBe(
195+
toolCallLabel(liveRow, "/home/me/proj"),
196+
);
197+
});
198+
});
199+
59200
describe("isPersistableId", () => {
60201
it("accepts alphanumeric ids with hyphens and underscores", () => {
61202
expect(isPersistableId("acp-session-1")).toBe(true);

‎src/features/sessions/data/sessionStore.ts‎

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { invoke } from "@tauri-apps/api/core";
2-
import { titleFromToolInput } from "../../../integrations/harness/core/preview";
2+
import {
3+
isWeakToolTitle,
4+
titleFromToolInput,
5+
} from "../../../integrations/harness/core/preview";
6+
import { codexCommandPresentation } from "../../../integrations/harness/providers/codex/codexProtocol";
37
import { recoverCursorSubagents } from "../../../integrations/harness/providers/cursor/cursorSubagents";
48
import { persistableAttachment } from "../model/attachments";
59
import type { ContextUsage } from "../model/contextUsage";
@@ -360,14 +364,7 @@ export async function getSession(sessionId: string): Promise<Session | null> {
360364
if (!record) return null;
361365
const session = recordToSession(record);
362366
if (session.harness === "claude" && session.providerSessionId) {
363-
const toolIds = session.blocks.flatMap((block) =>
364-
block.role === "tool" &&
365-
block.tool?.kind === "execute" &&
366-
block.text.trim() === "Shell" &&
367-
block.tool.callId
368-
? [block.tool.callId]
369-
: [],
370-
);
367+
const toolIds = shellPlaceholderIds(session.blocks);
371368
if (toolIds.length) {
372369
try {
373370
const commands = await claudeShellCommands(
@@ -385,6 +382,18 @@ export async function getSession(sessionId: string): Promise<Session | null> {
385382
}
386383
}
387384
}
385+
if (session.harness === "codex") {
386+
// Relabel from the command already saved on the row. Codex sends it with
387+
// the item and `shellCommandPreview` stores it as the preview title, so
388+
// this needs no disk read at all.
389+
const blocks = backfillCodexShellCommands(session.blocks);
390+
if (blocks !== session.blocks) {
391+
session.blocks = blocks;
392+
// A failed write must not cost the reader the session. The repair stays
393+
// in memory and the next load retries it.
394+
await upsertSession(session).catch(() => undefined);
395+
}
396+
}
388397
if (session.harness !== "omp" || !session.providerSessionId) {
389398
return recoverCursorSubagents(session);
390399
}
@@ -437,6 +446,55 @@ export function backfillClaudeShellCommands(
437446
return changed ? repaired : blocks;
438447
}
439448

449+
/** Exec rows that were saved without their command, keyed by their tool call. */
450+
function shellPlaceholderIds(blocks: Block[]): string[] {
451+
return blocks.flatMap((block) =>
452+
block.role === "tool" &&
453+
block.tool?.kind === "execute" &&
454+
block.text.trim() === "Shell" &&
455+
block.tool.callId
456+
? [block.tool.callId]
457+
: [],
458+
);
459+
}
460+
461+
/**
462+
* Relabel exec rows that were saved without their command.
463+
*
464+
* The command is already on the row: Codex sends it with the item, and
465+
* `shellCommandPreview` stores it as the preview title. Reading it back from
466+
* there keeps whatever Codex chose to show the user — including anything it
467+
* redacted — and never re-reads a secret off disk into the transcript store. A
468+
* row saved without a usable preview has no command left to recover, so it keeps
469+
* its placeholder label.
470+
*/
471+
export function backfillCodexShellCommands(blocks: Block[]): Block[] {
472+
let changed = false;
473+
const repaired = blocks.map((block) => {
474+
if (
475+
block.role !== "tool" ||
476+
block.tool?.kind !== "execute" ||
477+
block.text.trim() !== "Shell"
478+
) {
479+
return block;
480+
}
481+
const saved = block.tool.preview?.title?.trim();
482+
if (!saved || isWeakToolTitle(saved)) return block;
483+
changed = true;
484+
const { title, preview } = codexCommandPresentation({}, saved);
485+
return {
486+
...block,
487+
text: title,
488+
tool: {
489+
...block.tool,
490+
title,
491+
...(preview ? { preview } : {}),
492+
},
493+
};
494+
});
495+
return changed ? repaired : blocks;
496+
}
497+
440498
export async function deleteSession(
441499
sessionId: string,
442500
imagePaths: string[] = [],

‎src/features/sessions/data/sessionStoreRestore.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import type { SessionRecord } from "./sessionStore";
3+
4+
const invoke = vi.fn();
5+
vi.mock("@tauri-apps/api/core", () => ({ invoke: (...args: unknown[]) => invoke(...args) }));
6+
7+
const { getSession } = await import("./sessionStore");
8+
9+
/** A saved Codex session holding the one row this PR repairs. */
10+
function codexRecord(): SessionRecord {
11+
return {
12+
id: "s1",
13+
harness: "codex",
14+
model: "gpt-5",
15+
cwd: "/repo",
16+
providerSessionId: "01a0e6f4-13e3-7692-9250-4befceed807b",
17+
blocks: [
18+
{ id: "b0", role: "user", text: "list the agents" },
19+
{
20+
id: "b1",
21+
role: "tool",
22+
text: "Shell",
23+
tool: {
24+
callId: "exec-1",
25+
title: "Shell",
26+
kind: "execute",
27+
status: "completed",
28+
preview: {
29+
kind: "shell",
30+
title: "rg --files -g AGENTS.md -g '!node_modules'",
31+
},
32+
},
33+
},
34+
],
35+
} as unknown as SessionRecord;
36+
}
37+
38+
describe("restoring a session whose repair cannot be persisted", () => {
39+
beforeEach(() => invoke.mockReset());
40+
41+
it("still returns the repaired session when the write fails", async () => {
42+
invoke.mockImplementation((cmd: string) => {
43+
if (cmd === "session_get") return Promise.resolve(codexRecord());
44+
if (cmd === "session_upsert") return Promise.reject(new Error("db locked"));
45+
return Promise.resolve(null);
46+
});
47+
48+
// A failed persistence must not cost the reader the session: the repair
49+
// stays in memory and the next load retries the write.
50+
const session = await getSession("s1");
51+
expect(session).not.toBeNull();
52+
expect(session?.blocks[1].text).toBe("Find files");
53+
});
54+
55+
it("persists the repair when the write succeeds", async () => {
56+
invoke.mockImplementation((cmd: string) => {
57+
if (cmd === "session_get") return Promise.resolve(codexRecord());
58+
return Promise.resolve(null);
59+
});
60+
61+
const session = await getSession("s1");
62+
expect(session?.blocks[1].text).toBe("Find files");
63+
expect(
64+
invoke.mock.calls.some(([cmd]) => cmd === "session_upsert"),
65+
).toBe(true);
66+
});
67+
});

0 commit comments

Comments
 (0)