diff --git a/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts b/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts index 867993b0..dcbfa2c2 100644 --- a/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts +++ b/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test'; -import { orderTasks, type BackgroundTask } from './useBackgroundTasks'; +import { orderTasks, visibleTasks, type BackgroundTask } from './useBackgroundTasks'; // The scenario this exists for: a long-lived job is asked for, then short-lived ones run while you talk. // Newest-first alone pushed the running job off the right-hand end of a strip that scrolls. @@ -47,3 +47,50 @@ describe('orderTasks', () => { expect(ids(input)).toEqual(['d', 'r']); }); }); + +// ── Which chips the tray shows ── +// +// The harness auto-backgrounds a long foreground command, so the agent's own greps and typechecks arrive +// as `task_started` events indistinguishable from a job somebody asked for. Measured, not assumed: a +// 24-second foreground Bash call emits `task_started` with `task_type: 'local_bash'`. + +describe('visibleTasks', () => { + const mine = + (...ids: string[]) => + (id: string) => + ids.includes(id); + const base = { dismissed: [], pinnedIds: [], isMine: () => false }; + + test("the agent's finished jobs leave the tray; yours stay", () => { + const tasks = [done('agent-grep'), done('your-dev-server'), done('agent-typecheck')]; + const shown = visibleTasks({ ...base, tasks, isMine: mine('your-dev-server') }); + expect(ids(shown)).toEqual(['your-dev-server']); + }); + + test('a RUNNING job is shown whoever started it — it is live and it can be stopped', () => { + const tasks = [running('agent-build'), done('agent-grep')]; + expect(ids(visibleTasks({ ...base, tasks }))).toEqual(['agent-build']); + }); + + test('pinning keeps one of the agent\'s, because pinning is an explicit "keep this"', () => { + const tasks = [done('agent-grep'), done('agent-typecheck')]; + expect(ids(visibleTasks({ ...base, tasks, pinnedIds: ['agent-grep'] }))).toEqual(['agent-grep']); + }); + + test('dismissed beats everything', () => { + const tasks = [done('yours')]; + expect(visibleTasks({ ...base, tasks, dismissed: ['yours'], isMine: mine('yours') })).toEqual([]); + }); + + test('the finished cap applies to yours, and running jobs never count against it', () => { + const tasks = [running('r1'), ...Array.from({ length: 8 }, (_, i) => done(`d${i}`))]; + const shown = visibleTasks({ ...base, tasks, isMine: () => true }); + expect(ids(shown).filter((id) => id.startsWith('r'))).toEqual(['r1']); + expect(ids(shown).filter((id) => id.startsWith('d'))).toHaveLength(5); + }); + + test('running still sorts ahead of finished', () => { + const tasks = [done('yours-old'), running('agent-live')]; + expect(ids(visibleTasks({ ...base, tasks, isMine: mine('yours-old') }))).toEqual(['agent-live', 'yours-old']); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts b/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts index 046ef5d1..dc92753d 100644 --- a/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts +++ b/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts @@ -2,6 +2,7 @@ import { useMemo, useState } from 'react'; import { useQuery } from '@tanstack/react-query'; import { useClient } from 'hooks/useClient'; import type { ChatMessage } from './types'; +import { isUserBackgroundJob } from '../../background-job-origin'; export type BackgroundTask = Extract; @@ -46,6 +47,57 @@ export function orderTasks(tasks: BackgroundTask[]): BackgroundTask[] { return [...tasks.filter((t) => !t.status), ...tasks.filter((t) => t.status)]; } +type VisibleParams = { + /** Newest-first, as `tasks` already is. */ + tasks: BackgroundTask[]; + dismissed: string[]; + pinnedIds: string[]; + /** Did this job come from the background-job dialog, or from the stop button? See `background-job-origin`. */ + isMine: (taskId: string) => boolean; +}; + +/** + * Which chips the tray shows. + * + * ── Why "finished" is not enough to decide ── + * + * The harness AUTO-BACKGROUNDS a foreground command that runs long. Measured against the installed SDK: a + * 24-second foreground `Bash` call emits `task_started` with `task_type: 'local_bash'`, byte-identical in + * shape to one started with `run_in_background: true`. There is no field that tells them apart, and there + * should not be — from the harness's point of view they became the same thing. + * + * The consequence on screen was a tray filling with green ticks for the agent's own greps and typechecks — + * work nobody asked to watch, sitting beside the one job that was actually started on purpose. + * + * So the rule is about ORIGIN, not about state: + * + * running always shown, whoever started it — it is live, and it can be stopped + * finished, yours kept (up to FINISHED_KEPT), because you asked for it and the outcome is the point + * finished, the agent's dropped the moment it ends, because it was never yours to read + * pinned kept regardless, on both counts + * + * A job stopped by hand counts as yours from that moment — `markUserBackgroundJob` — since pressing stop + * is an act on a specific job and its outcome is now something you are waiting for. + * + * `[open]` Origin is process state, so a page reload forgets it and a previously-yours finished chip is + * treated as the agent's and dropped. Acceptable: after a reload you are not watching that completion any + * more. Making it survive means recording origin in the transcript, which is a wire change. + */ +export function visibleTasks({ tasks, dismissed, pinnedIds, isMine }: VisibleParams): BackgroundTask[] { + let finished = 0; + const kept = tasks.filter((task) => { + if (dismissed.includes(task.taskId)) return false; + // A pin is an explicit "keep this", so it outranks every rule below — otherwise pinning the one task + // you care about would not stop five newer ones from pushing it out. + if (pinnedIds.includes(task.taskId)) return true; + if (!task.status) return true; + if (!isMine(task.taskId)) return false; + finished += 1; + return finished <= FINISHED_KEPT; + }); + return orderTasks(kept); +} + export function useBackgroundTasks(messages: ChatMessage[]) { const [dismissed, setDismissed] = useState([]); const [pinnedIds, setPinnedIds] = useState([]); @@ -56,22 +108,10 @@ export function useBackgroundTasks(messages: ChatMessage[]) { return [...byId.values()].reverse(); }, [messages]); - const visible = useMemo(() => { - let finished = 0; - // `tasks` is already newest-first, so counting forwards drops the oldest finished chips and keeps the - // ones whose notifications are still recent enough to be worth reading. - const kept = tasks.filter((t) => { - if (dismissed.includes(t.taskId)) return false; - // A pin is an explicit "keep this", so it outranks the cap — otherwise pinning the one task you care - // about would not stop five newer ones from pushing it out. - if (!t.status || pinnedIds.includes(t.taskId)) return true; - finished += 1; - return finished <= FINISHED_KEPT; - }); - // Ordered after the cap, not before: the cap is about which finished chips are worth keeping, and that - // question is answered by age. Where they then sit is a different one. - return orderTasks(kept); - }, [tasks, dismissed, pinnedIds]); + const visible = useMemo( + () => visibleTasks({ tasks, dismissed, pinnedIds, isMine: isUserBackgroundJob }), + [tasks, dismissed, pinnedIds], + ); const running = visible.filter((t) => !t.status); const pinned = visible.filter((t) => pinnedIds.includes(t.taskId)); @@ -79,7 +119,9 @@ export function useBackgroundTasks(messages: ChatMessage[]) { // Uncapped on purpose: clearing the finished chips must also clear the ones the cap is hiding behind // them, or the bulk clear just deals the next five off the same deck. - const clearable = tasks.filter((t) => t.status && !pinnedIds.includes(t.taskId)); + // Uncapped, but only over chips that are actually shown: the agent's own finished jobs are already gone + // from the tray, and "clear all finished" must not silently count them. + const clearable = tasks.filter((t) => t.status && !pinnedIds.includes(t.taskId) && isUserBackgroundJob(t.taskId)); return { tasks: visible,