diff --git a/changelog.d/pi-branch-return-outcome.md b/changelog.d/pi-branch-return-outcome.md new file mode 100644 index 0000000000..d3e0e0b698 --- /dev/null +++ b/changelog.d/pi-branch-return-outcome.md @@ -0,0 +1,3 @@ +### Fixed +- A command sent from Chat to a pi session now shows its result when the session moved to another + branch and `/tree` came back before the result was written, instead of ending as unknown. diff --git a/docs/agent-commands.md b/docs/agent-commands.md index fdc0ecdb0f..56d35afc87 100644 --- a/docs/agent-commands.md +++ b/docs/agent-commands.md @@ -188,7 +188,13 @@ Recognized Hermes informational result cards remain available. the exact text; a `model_change`, `thinking_level_change` or `compaction` record in pi, omp, gjc and OmO sessions (omp's `/handoff` writes a compaction with method `handoff`); a `compacted` record for Codex `/compact`. Records are claimed in send order, so two equal sends never share - one, and a rewritten transcript drops what it no longer holds. + one, and a rewritten transcript drops what it no longer holds. Known limit: a pi-family send the + agent dropped without a record takes the record of a later equal send on the same branch, and + that later send ends as unknown. In pi, only the session's active + branch counts: a send stays with the branch it was sent on, also after the session goes to + another branch and `/tree` comes back to it, unless meanwhile something was written under that + branch's last entry or a record a result could be was written elsewhere (`resumedBranch` in + [`server/conversation.ts`](../server/conversation.ts)); then the send ends as unknown. - `dialog`: a card answered from Chat, counted only for the send that raised that card. - `recognized-result`: a reader of kind `result` matched a complete, recorded layout that appeared after this send's own echo and was not on screen when it was sent. Hermes `/help`, `/status`, @@ -214,7 +220,7 @@ program name), it stays with the next session herdr reports in the same terminal agent, or under the label herdr gave the pane at the send (herdr names an OmO pane `pi`). A send to a restarted agent while herdr still shows the session of the process before waits up to a minute (`STALE_SESSION_LAG_MS`) for the agent's own session. Known limits, all of which hide a row rather -than show a wrong result, except the last: +than show a wrong result, except the last two: - That minute is counted when Chat next asks. A session that appeared within it while no Chat was open is taken for a later one, and the row is hidden. diff --git a/server/conversation.ts b/server/conversation.ts index 6562d68106..171b1e1379 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -73,7 +73,7 @@ import { CLAUDE_IMAGE_REF, claudeImageType, claudeResultText, createClaudeTransc import { forgetSubagents, lineNotifications, subagentDetails, type SubagentDetail } from "./claude-subagents.ts"; import { launchedAgents } from "./launched-agents.ts"; import { readsStartScreen, startScreenMetadata } from "./start-screen.ts"; -import { commandTranscriptRecords, type CommandTranscriptRecord } from "./outcome.ts"; +import { commandTranscriptRecords, PI_EVIDENCE_TYPES, type CommandTranscriptRecord } from "./outcome.ts"; import { type ClaudeBranch, claudeForks, followClaudeBranch, type PromptChain, turnStarts } from "./transcript-turns.ts"; import { devinSessionForPane, isDevinProcess } from "./devin-session.ts"; @@ -1589,7 +1589,11 @@ const commandTranscripts = new Map; }>(); +const REMEMBERED_BRANCHES = 8; /** `look`: the snapshot and herdr's `pane.process_info` the caller's poll already read, so neither is asked again. */ export async function readCommandTranscript(paneId: string, codexHome?: string, look: { snapshot?: SessionSnapshot; processes?: KnownProcesses } = {}): Promise { @@ -1633,6 +1637,7 @@ export async function readCommandTranscript(paneId: string, codexHome?: string, const identity = append ? cached.identity : `${source}\0${path}\0${file}:${stat.size}:${stat.mtimeMs}`; let branch = ""; let leaf: string | null = null; + let branches = new Map(); if (source === "pi-transcript") { const nodes = current.filter(record => typeof record.entry.id === "string"); const byId = new Map(nodes.map((record, index) => [String(record.entry.id), { record, index }])); @@ -1657,29 +1662,61 @@ export async function readCommandTranscript(paneId: string, codexHome?: string, // An unexplained missing ancestor or ambiguous index cannot prove branch ownership. current = rooted ? active.reverse() : []; leaf = rooted && current.length > 0 ? String(current.at(-1)!.entry.id) : null; - // An append that runs on from the branch read before is that branch: the forks in the window change as it - // slides (its first node always looks like one), and the sends bound to the branch must keep seeing it (#1487). - // Any other append, a branch switch or a fork made since among them, names its branch afresh. - const continued = append && cached.leaf !== null && cached.branch !== "unresolved" && rooted - && runsOnFrom(cached.leaf, current, forks, nodes[0]); - branch = !rooted ? "unresolved" : continued ? cached.branch - : createHash("sha256").update(forks.map(({ entry }) => String(entry.id)).join("\0")).digest("hex"); + // An append that runs on from the node a branch was last read at is that branch: the forks in the window change + // as it slides (its first node always looks like one), and the session can go to another branch and /tree back, + // yet the sends bound to the branch must keep seeing it (#1487, #1879). Any other path names its branch afresh. + if (append) branches = new Map(cached.branches); + const resumed = rooted ? resumedBranch(branches, nodes, current) : null; + const continued = resumed === null && append && cached.leaf !== null && cached.branch !== "unresolved" && rooted + && runsOnFromCut(cached.leaf, current, forks, nodes[0]); + branch = !rooted ? "unresolved" : resumed ?? (continued ? cached.branch + : createHash("sha256").update(forks.map(({ entry }) => String(entry.id)).join("\0")).digest("hex")); + if (branch !== "unresolved" && leaf !== null) { + branches.delete(branch); + branches.set(branch, { id: leaf, offset: current.at(-1)!.offset }); + if (branches.size > REMEMBERED_BRANCHES) branches.delete(branches.keys().next().value!); + } } const cleared = current.reduce((last, { entry }, index) => isContextClear(entry, source) ? index : last, 0); const answer = branch === "unresolved" ? null : { identity: `${identity}\0${branch}`, records: cleared > 0 ? current.slice(cleared) : current }; - remember(commandTranscripts, path, { signature, file, start, end: stat.size, digests, partialStart, records, identity, branch, leaf, answer }, 4); + remember(commandTranscripts, path, { signature, file, start, end: stat.size, digests, partialStart, records, identity, branch, leaf, answer, branches }, 4); return answer; } /** - * Whether a pi path read after an append runs on from `leaf`, the last node of the path read before, with no fork - * after it. `leaf` is on the path, or was cut away with the window's start and is the parent of its first node (the - * first node read after it, as appends follow it), whose own fork mark only says its predecessor was cut away. + * The remembered pi branch (`branches`: name to the node it was last read at) that `path` runs on from. Its node is on + * the path, the same entry at the same offset (a later entry may reuse its id), and of what was written since (that node was then the file's last), nothing off the path is under it, and + * nothing off the path is a record a send's result could be (`PI_EVIDENCE_TYPES`): a result written on the branch the + * session went to must not leave a send waiting on its old branch for a later identical send's. `nodes` are the + * window's nodes in file order. + */ +function resumedBranch(branches: ReadonlyMap, nodes: readonly CommandTranscriptRecord[], path: readonly CommandTranscriptRecord[]): string | null { + const onPath = new Map(path.map((record) => [String(record.entry.id), record])); + for (const [name, leaf] of branches) { + const at = onPath.get(leaf.id); + if (!at || at.offset !== leaf.offset) continue; + const under = new Set([leaf.id]); + const alone = nodes.every(({ entry, offset }) => { + if (offset <= at.offset) return true; + const id = String(entry.id); + if (typeof entry.parentId === "string" && under.has(entry.parentId)) { + under.add(id); + return onPath.has(id); + } + return onPath.has(id) || !PI_EVIDENCE_TYPES.has(String(entry.type)); + }); + if (alone) return name; + } + return null; +} + +/** + * Whether a pi path read after an append runs on from `leaf`, the last node of the path read before, which was cut + * away with the window's start: it is the parent of the path's first node (the first node read after it, as appends + * follow it), whose own fork mark only says its predecessor was cut away, and no other fork follows. */ -function runsOnFrom(leaf: string, path: readonly CommandTranscriptRecord[], forks: readonly CommandTranscriptRecord[], first: CommandTranscriptRecord | undefined): boolean { - const kept = path.find(({ entry }) => entry.id === leaf); - if (kept) return forks.every((fork) => fork.offset <= kept.offset); +function runsOnFromCut(leaf: string, path: readonly CommandTranscriptRecord[], forks: readonly CommandTranscriptRecord[], first: CommandTranscriptRecord | undefined): boolean { const head = path[0]; return head !== undefined && head === first && head.entry.parentId === leaf && forks.every((fork) => fork === head); } diff --git a/server/outcome-transcript-budget.test.ts b/server/outcome-transcript-budget.test.ts index 325ef97bba..910c92c9ba 100644 --- a/server/outcome-transcript-budget.test.ts +++ b/server/outcome-transcript-budget.test.ts @@ -178,6 +178,189 @@ it("names a fork made after the pi branch's last entry afresh, though that entry } }); +it("completes a pi send bound to a branch after the session leaves it and /tree comes back (#1879)", async () => { + const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-branch-return-")); + const path = join(root, "session.jsonl"); + const at = Date.parse("2026-10-20T00:00:00Z"); + const line = (entry: object) => JSON.stringify(entry) + "\n"; + fs.writeFileSync(path, line({ type: "message", id: "root", parentId: null }) + line({ type: "message", id: "a0", parentId: "root" }) + + line({ type: "message", id: "a1", parentId: "a0" })); + const pane = { pane_id: root, terminal_id: "term", workspace_id: "w", tab_id: "t", focused: false, revision: 0, + agent: "pi", agent_status: "working", cwd: root }; + const clock = spyOn(Date, "now").mockReturnValue(at + 30); + const binding = spyOn(conversation, "paneTranscript").mockResolvedValue({ source: "pi-transcript", path }); + const snapshot = spyOn(client, "sessionSnapshot").mockResolvedValue({ + panes: [pane], layouts: [], agents: [], workspaces: [], tabs: [], protocol: 22, version: "fixture", + }); + const screen = spyOn(client, "paneRead").mockResolvedValue({ ...pane, text: "", source: "detection", format: "text", truncated: false }); + try { + const sent = { id: "branch-return-model", text: "/model chosen-model", at, agent: "pi", + identity: JSON.stringify([pane.terminal_id, undefined]), screen: "", store: null, form: null, + invocations: commandInvocations("pi", "/model chosen-model", []) }; + noteSubmitted(pane.pane_id, sent.text, sent); + const url = new URL(`http://localhost/api/pane/prompt?pane_id=${encodeURIComponent(pane.pane_id)}`); + const latest = async () => ((await (await handlePromptRequest(new Request(url), url))?.json()) as PanePromptResponse).outcomes?.at(-1); + expect(await latest()).toMatchObject({ state: "accepted", evidence: [] }); + // a poll sees the session on a new branch from root + fs.appendFileSync(path, line({ type: "message", id: "b0", parentId: "root" })); + expect(await latest()).toMatchObject({ state: "accepted", evidence: [] }); + // /tree back to a1, where the model the send chose is recorded + fs.appendFileSync(path, line({ type: "model_change", id: "chosen", parentId: "a1", + timestamp: new Date(at + 10).toISOString(), modelId: "chosen-model" })); + expect(await latest()).toMatchObject({ state: "result-observed", evidence: [{ kind: "transcript" }] }); + } finally { + screen.mockRestore(); + snapshot.mockRestore(); + binding.mockRestore(); + clock.mockRestore(); + conversation.forgetTranscriptState(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +it("does not credit a pi send with a later identical send's result when its own went to the branch the session visited", async () => { + const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-branch-visit-")); + const path = join(root, "session.jsonl"); + const at = Date.parse("2026-10-20T00:00:00Z"); + const line = (entry: object) => JSON.stringify(entry) + "\n"; + fs.writeFileSync(path, line({ type: "message", id: "root", parentId: null }) + line({ type: "message", id: "a0", parentId: "root" }) + + line({ type: "message", id: "a1", parentId: "a0" })); + const pane = { pane_id: root, terminal_id: "term", workspace_id: "w", tab_id: "t", focused: false, revision: 0, + agent: "pi", agent_status: "working", cwd: root }; + const clock = spyOn(Date, "now").mockReturnValue(at + 30); + const binding = spyOn(conversation, "paneTranscript").mockResolvedValue({ source: "pi-transcript", path }); + const snapshot = spyOn(client, "sessionSnapshot").mockResolvedValue({ + panes: [pane], layouts: [], agents: [], workspaces: [], tabs: [], protocol: 22, version: "fixture", + }); + const screen = spyOn(client, "paneRead").mockResolvedValue({ ...pane, text: "", source: "detection", format: "text", truncated: false }); + try { + const send = (id: string, sentAt: number) => { + const sent = { id, text: "/model chosen-model", at: sentAt, agent: "pi", + identity: JSON.stringify([pane.terminal_id, undefined]), screen: "", store: null, form: null, + invocations: commandInvocations("pi", "/model chosen-model", []) }; + noteSubmitted(pane.pane_id, sent.text, sent); + }; + const url = new URL(`http://localhost/api/pane/prompt?pane_id=${encodeURIComponent(pane.pane_id)}`); + const outcomes = async () => ((await (await handlePromptRequest(new Request(url), url))?.json()) as PanePromptResponse).outcomes ?? []; + send("first", at); + await outcomes(); + fs.appendFileSync(path, line({ type: "message", id: "b0", parentId: "root" })); + await outcomes(); + // the first send's result, written on the branch the session went to + fs.appendFileSync(path, line({ type: "model_change", id: "first-result", parentId: "b0", + timestamp: new Date(at + 5).toISOString(), modelId: "chosen-model" })); + await outcomes(); + // /tree back to a1, and the same command again + send("second", at + 10); + fs.appendFileSync(path, line({ type: "model_change", id: "second-result", parentId: "a1", + timestamp: new Date(at + 15).toISOString(), modelId: "chosen-model" })); + const shown = await outcomes(); + expect(shown.find(({ id }) => id === "first")?.state).not.toBe("result-observed"); + expect(shown.find(({ id }) => id === "second")).toMatchObject({ state: "result-observed", + evidence: [{ kind: "transcript", at: new Date(at + 15).toISOString() }] }); + } finally { + screen.mockRestore(); + snapshot.mockRestore(); + binding.mockRestore(); + clock.mockRestore(); + conversation.forgetTranscriptState(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +it("keeps a pi branch when the session goes to another branch and back to its last entry between two polls", async () => { + const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-branch-back-")); + const path = join(root, "session.jsonl"); + const line = (entry: object) => JSON.stringify(entry) + "\n"; + fs.writeFileSync(path, line({ type: "message", id: "root", parentId: null }) + line({ type: "message", id: "a0", parentId: "root" })); + const binding = spyOn(conversation, "paneTranscript").mockResolvedValue({ source: "pi-transcript", path }); + try { + const before = await conversation.readCommandTranscript("branch-back"); + fs.appendFileSync(path, line({ type: "message", id: "b0", parentId: "root" }) + line({ type: "message", id: "a1", parentId: "a0" })); + const after = await conversation.readCommandTranscript("branch-back"); + expect(after?.records.map(({ entry }) => entry.id)).toEqual(["root", "a0", "a1"]); + expect(after?.identity).toBe(before?.identity); + } finally { + binding.mockRestore(); + conversation.forgetTranscriptState(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +it("names a pi branch afresh on coming back to its last entry when something else was written under that entry", async () => { + const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-branch-back-hidden-")); + const path = join(root, "session.jsonl"); + const line = (entry: object) => JSON.stringify(entry) + "\n"; + fs.writeFileSync(path, line({ type: "message", id: "root", parentId: null }) + line({ type: "message", id: "a0", parentId: "root" })); + const binding = spyOn(conversation, "paneTranscript").mockResolvedValue({ source: "pi-transcript", path }); + try { + const before = await conversation.readCommandTranscript("branch-back-hidden"); + // a poll sees another branch, then an answer under a0 no poll sees, and /tree back to a0 again + fs.appendFileSync(path, line({ type: "message", id: "b0", parentId: "root" })); + expect((await conversation.readCommandTranscript("branch-back-hidden"))?.identity).not.toBe(before?.identity); + fs.appendFileSync(path, line({ type: "message", id: "a1", parentId: "a0" }) + line({ type: "model_change", id: "chosen", parentId: "a0", modelId: "chosen-model" })); + const after = await conversation.readCommandTranscript("branch-back-hidden"); + expect(after?.records.map(({ entry }) => entry.id)).toEqual(["root", "a0", "chosen"]); + expect(after?.identity).not.toBe(before?.identity); + } finally { + binding.mockRestore(); + conversation.forgetTranscriptState(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +it("does not take a later entry that reuses a remembered pi branch's last id for that branch", async () => { + const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-reused-id-")); + const path = join(root, "session.jsonl"); + const line = (entry: object) => JSON.stringify(entry) + "\n"; + const big = (id: string, parentId: string, mebibytes: number) => line({ type: "message", id, parentId, + message: { role: "assistant", content: "x".repeat(mebibytes * 1024 * 1024) } }); + fs.writeFileSync(path, line({ type: "message", id: "root", parentId: null }) + line({ type: "message", id: "a0", parentId: "root" }) + + line({ type: "message", id: "a1", parentId: "a0" })); + const binding = spyOn(conversation, "paneTranscript").mockResolvedValue({ source: "pi-transcript", path }); + try { + const before = await conversation.readCommandTranscript("reused-id"); + fs.appendFileSync(path, big("b0", "root", 15)); + await conversation.readCommandTranscript("reused-id"); + // another entry with a1's id, then enough that the window no longer holds the first a1 + fs.appendFileSync(path, line({ type: "message", id: "a1", parentId: "b0" })); + expect(await conversation.readCommandTranscript("reused-id")).toBeNull(); + fs.appendFileSync(path, big("b1", "a1", 2)); + const after = await conversation.readCommandTranscript("reused-id"); + expect(after?.records.map(({ entry }) => entry.id)).toEqual(["a1", "b1"]); // control: the first a1 is cut away + expect(after?.identity).not.toBe(before?.identity); + } finally { + binding.mockRestore(); + conversation.forgetTranscriptState(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +it("names a pi branch afresh when an append cuts its last entry away and forks after the window's first entry", async () => { + const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-leaf-cut-fork-")); + const path = join(root, "session.jsonl"); + const entry = (id: string, parentId: string | null) => JSON.stringify({ type: "message", id, parentId, + message: { role: "assistant", content: "x".repeat(1024 * 1024) } }) + "\n"; + fs.writeFileSync(path, JSON.stringify({ type: "session", id: "session", version: 3 }) + "\n"); + for (let index = 0; index < 20; index++) fs.appendFileSync(path, entry(`a${index}`, index ? `a${index - 1}` : null)); + const binding = spyOn(conversation, "paneTranscript").mockResolvedValue({ source: "pi-transcript", path }); + try { + const before = await conversation.readCommandTranscript("leaf-cut-fork"); + expect(before?.records.at(-1)?.entry.id).toBe("a19"); + // h0 runs on from a19 and h13 after it; then /tree back to h0 for another answer + for (let index = 0; index < 14; index++) fs.appendFileSync(path, entry(`h${index}`, index ? `h${index - 1}` : "a19")); + fs.appendFileSync(path, entry("k", "h0") + JSON.stringify({ type: "model_change", id: "chosen", parentId: "k", modelId: "chosen-model" }) + "\n"); + const after = await conversation.readCommandTranscript("leaf-cut-fork"); + // control: a19 is cut away with the window's start and h0 heads it + expect(after?.records.map(({ entry }) => entry.id)).toEqual(["h0", "k", "chosen"]); + expect(after?.identity).not.toBe(before?.identity); + } finally { + binding.mockRestore(); + conversation.forgetTranscriptState(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + it("keeps a pi branch when an append cuts its last entry out of the window", async () => { const root = fs.mkdtempSync(join(tmpdir(), "outcome-pi-leaf-cut-")); const path = join(root, "session.jsonl"); diff --git a/server/outcome.ts b/server/outcome.ts index f633d6d7d3..8b9c63a062 100644 --- a/server/outcome.ts +++ b/server/outcome.ts @@ -189,6 +189,12 @@ function claudeOutputEchoes(records: readonly CommandTranscriptRecord[]): Readon return outputs; } +/** + * The only record types transcriptEvidence takes a pi, omp, gjc or OmO send's result from; a pi branch is resumed + * only when none was written off it meanwhile (resumedBranch in conversation.ts). + */ +export const PI_EVIDENCE_TYPES: ReadonlySet = new Set(["model_change", "thinking_level_change", "compaction"]); + /** Native envelopes, not conversational prose or the model/effort shown in a footer. */ function transcriptEvidence(sent: CommandSend, records: readonly CommandTranscriptRecord[], until: number, used: ReadonlySet = new Set(), echoOffset?: number | null, outputs?: ReadonlyMap): { offset: number; at: number } | null { @@ -221,7 +227,7 @@ function transcriptEvidence(sent: CommandSend, records: readonly CommandTranscri const name = invocation.name.toLocaleLowerCase(); const argument = invocation.argument.split(/\s+/u)[0] ?? ""; if (used.has(offset)) continue; - if (["pi", "omp", "gjc", "omo"].includes(sent.agent)) { + if (["pi", "omp", "gjc", "omo"].includes(sent.agent) && PI_EVIDENCE_TYPES.has(String(entry.type))) { if (entry.type === "model_change" && ["model", "models", "switch"].includes(name) && argument) { const model = typeof entry.model === "string" ? entry.model : entry.modelId; const qualified = typeof entry.provider === "string" ? `${entry.provider}/${model}` : model;