diff --git a/src/workspaces/officerdev/src/apps/Chat/running-tasks.test.ts b/src/workspaces/officerdev/src/apps/Chat/running-tasks.test.ts new file mode 100644 index 00000000..1cc85dc1 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/running-tasks.test.ts @@ -0,0 +1,97 @@ +import { describe, test, expect } from 'bun:test'; +import { spliceRunningTasks } from './running-tasks'; +import type { ChatMessage } from './types'; + +// The two phrasings the harness actually produces, copied from a live transcript. Both carry the task id, +// which is the only part this relies on. +const bashOutput = (id: string) => `Command running in background with ID: ${id}. Output is being written to: /tmp/x`; +const monitorOutput = (id: string) => `Monitor started (task ${id}, timeout 900000ms). You will be notified.`; + +const tool = (toolName: string, output: string): ChatMessage => ({ + role: 'tool', + toolName, + toolInput: {}, + toolCallId: `call-${output.slice(-8)}`, + output, +}); + +const user = (text: string): ChatMessage => ({ role: 'user', text }); +const assistant = (text: string): ChatMessage => ({ role: 'assistant', text }); + +const roles = (messages: ChatMessage[]) => messages.map((m) => (m.role === 'task' ? `task:${m.taskId}` : m.role)); + +describe('spliceRunningTasks', () => { + test('a recovered row lands right after the tool call that started it', () => { + const messages = [ + user('start a job'), + tool('Bash', bashOutput('bg1')), + assistant('started it'), + user('anything else?'), + ]; + const out = spliceRunningTasks(messages, [{ taskId: 'bg1', description: 'long job' }]); + expect(roles(out)).toEqual(['user', 'tool', 'task:bg1', 'assistant', 'user']); + }); + + test('several tasks each land at their own anchor', () => { + const messages = [ + tool('Bash', bashOutput('bg1')), + assistant('one'), + tool('Monitor', monitorOutput('bg2')), + assistant('two'), + ]; + const out = spliceRunningTasks(messages, [ + { taskId: 'bg2', description: 'watcher' }, + { taskId: 'bg1', description: 'ticker' }, + ]); + expect(roles(out)).toEqual(['tool', 'task:bg1', 'assistant', 'tool', 'task:bg2', 'assistant']); + }); + + test('a task whose tool call is not in the loaded window still appends', () => { + // Reverse pagination means the anchor can genuinely be off-screen. Appending is worse than in place + // and much better than dropping the row, which would hide running work. + const messages = [assistant('scrolled past the start')]; + const out = spliceRunningTasks(messages, [{ taskId: 'gone', description: 'orphan' }]); + expect(roles(out)).toEqual(['assistant', 'task:gone']); + }); + + test('a task already on screen is never added twice', () => { + const messages: ChatMessage[] = [ + tool('Bash', bashOutput('bg1')), + { role: 'task', taskId: 'bg1', description: 'long job' }, + ]; + expect(spliceRunningTasks(messages, [{ taskId: 'bg1', description: 'long job' }])).toBe(messages); + }); + + test('nothing to recover returns the same array, not a copy', () => { + const messages = [user('hi')]; + expect(spliceRunningTasks(messages, [])).toBe(messages); + }); + + test('the id is matched in tool output only, never in prose that mentions it', () => { + // The id is quoted in the conversation — this very session does it — and a message about a task is + // not the tool call that started one. + const messages = [user('what happened to bg1?'), assistant('bg1 is still running')]; + const out = spliceRunningTasks(messages, [{ taskId: 'bg1', description: 'ticker' }]); + expect(roles(out)).toEqual(['user', 'assistant', 'task:bg1']); + }); + + test('the spawning call wins over a later command that merely prints the id', () => { + // Caught on real data: a diagnostic script that grepped the transcript printed both live task ids, + // and matching the most recent mention parked the rows next to that script instead of next to the + // calls that started them. The id cannot exist before the call that mints it, so first wins. + const messages = [ + tool('Bash', bashOutput('bg1')), + assistant('started'), + tool('Bash', `checking… found beqzu: bg1 still alive`), + assistant('yes, still going'), + ]; + const out = spliceRunningTasks(messages, [{ taskId: 'bg1', description: 'ticker' }]); + expect(roles(out)).toEqual(['tool', 'task:bg1', 'assistant', 'tool', 'assistant']); + }); + + test('the description and type survive the round trip', () => { + const messages = [tool('Bash', bashOutput('bg1'))]; + const out = spliceRunningTasks(messages, [{ taskId: 'bg1', description: 'ticker', taskType: 'local_bash' }]); + expect(out[1]).toEqual({ role: 'task', taskId: 'bg1', description: 'ticker', taskType: 'local_bash' }); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/running-tasks.ts b/src/workspaces/officerdev/src/apps/Chat/running-tasks.ts new file mode 100644 index 00000000..f516d080 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/running-tasks.ts @@ -0,0 +1,45 @@ +import type { ChatMessage, RunningTask } from './types'; + +/** + * Put recovered background-task rows back where they belong, rather than at the end. + * + * A task row is officer's own invention — nothing corresponding to it is written to Claude's transcript — + * so a refresh rebuilds the conversation without it, and the server hands the still-running ones back on + * `sync:live`. The question is where they go. + * + * The row carries no timestamp, but it does not need one: the harness stamps the task's id into the output + * of the very tool call that started it ("running in background with ID: …"). Live, `task:started` arrives + * immediately after that tool result — anchoring there reproduces the position the row would have had if + * you had never refreshed. + * + * The *first* mention wins, and that is the whole correctness argument: the id is minted by the call that + * spawns the task, so nothing earlier in the conversation can contain it. Later mentions are commonplace — + * the agent quoting a task id while diagnosing something, or a command that prints one — and matching the + * most recent instead put rows next to a diagnostic that merely named them. + * + * A task whose tool call has scrolled out of the loaded window still appends. That is the old behaviour + * kept as the fallback: at the end is worse than in place, and better than silently dropped. + */ +export function spliceRunningTasks(messages: ChatMessage[], tasks: RunningTask[]): ChatMessage[] { + const known = new Set(messages.filter((m) => m.role === 'task').map((m) => m.taskId)); + const fresh = tasks.filter((t) => !known.has(t.taskId)); + if (!fresh.length) return messages; + + const anchored = new Map(); + const unanchored: ChatMessage[] = []; + for (const task of fresh) { + const row: ChatMessage = { role: 'task', ...task }; + const at = messages.findIndex((m) => m.role === 'tool' && m.output?.includes(task.taskId)); + if (at === -1) unanchored.push(row); + else anchored.set(at, [...(anchored.get(at) ?? []), row]); + } + if (!anchored.size) return [...messages, ...unanchored]; + + const next: ChatMessage[] = []; + messages.forEach((message, i) => { + next.push(message); + const rows = anchored.get(i); + if (rows) next.push(...rows); + }); + return [...next, ...unanchored]; +} diff --git a/src/workspaces/officerdev/src/hooks/useChat.ts b/src/workspaces/officerdev/src/hooks/useChat.ts index 0497e7b6..40592ac7 100644 --- a/src/workspaces/officerdev/src/hooks/useChat.ts +++ b/src/workspaces/officerdev/src/hooks/useChat.ts @@ -3,6 +3,7 @@ import { useChatWebSocket } from 'hooks/useChatWebSocket'; import { useClient } from 'hooks/useClient'; import { useSettings } from 'state/useSettings'; import type { ChatMessage, ServerMessage, TaskInfo, Message } from '../apps/Chat/types'; +import { spliceRunningTasks } from '../apps/Chat/running-tasks'; const SAVE_DEBOUNCE_MS = 1000; const OLDER_PAGE_SIZE = 20; // messages fetched per scroll-up (matches the initial tail window) @@ -355,19 +356,8 @@ export function useChat(initialSessionId?: string, initialModel?: string | null, streamingRef.current = msg.streamingText; flushStreaming(); } - // The one thing the transcript genuinely cannot supply. Appended rather than spliced into place: - // a task row carries no timestamp to sort by, so a recovered one lands at the end of the - // conversation instead of where it was started. The tray reads by taskId and renders correctly - // either way, and the alternative is inventing an ordering the record does not contain. - if (msg.runningTasks?.length) { - setMessages((prev) => { - const known = new Set(prev.filter((m) => m.role === 'task').map((m) => m.taskId)); - const recovered = msg.runningTasks - .filter((t) => !known.has(t.taskId)) - .map((t): ChatMessage => ({ role: 'task', ...t })); - return recovered.length ? [...prev, ...recovered] : prev; - }); - } + // The one thing the transcript genuinely cannot supply — see `sync:live` on the server. + if (msg.runningTasks?.length) setMessages((prev) => spliceRunningTasks(prev, msg.runningTasks)); break; }