From 74f2e59069691a9b982d84181874be5287715ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sun, 16 Aug 2026 12:02:32 +0000 Subject: [PATCH] the tray stops filling with the agent's own commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "So the pills don't get polluted with those calls that you're making." First, a correction to what I assumed. The platform is not mis-detecting anything: the SDK emits `task_started` only for real background work, and I checked — a foreground `echo` produces no task event at all. What produces these is the HARNESS auto-backgrounding 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'`, identical in shape to one started with `run_in_background: true`. There is no field that separates them, and there should not be — by then they are the same thing. So the chips were real. They were just not anybody's business: a tray of green ticks for greps and typechecks, sitting beside the one job that was started on purpose. The rule is now about ORIGIN rather than state, using the arm-and-claim that already exists for the preview: running shown, whoever started it — it is live and it can be stopped finished, yours kept (up to five), because you asked for it and the outcome is the point finished, the agent's dropped the moment it ends pinned kept regardless Running jobs stay visible deliberately, including mine. A background command of the agent's that has gone wrong is exactly the thing worth seeing and stopping, and the stop button already adopts anything you press it on. Extracted as `visibleTasks`, pure and tested — six cases including the cap, the pin override, and running-before-finished surviving the new filter. `[open]` Origin is process state, so a reload forgets it and a previously-yours finished chip is then treated as the agent's and dropped. Acceptable — after a reload you are not watching that completion — and making it survive means putting origin in the transcript, which is a wire change. tsgo clean, frontend builds, 884 pass / 7 fail unchanged. Co-Authored-By: Claude Opus 5 --- .../src/apps/Chat/order-tasks.test.ts | 49 +++++++++++- .../src/apps/Chat/useBackgroundTasks.ts | 76 ++++++++++++++----- 2 files changed, 107 insertions(+), 18 deletions(-) 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,