From 048e718b21b0a451457ad5cb6cd01e7654e68c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sun, 16 Aug 2026 02:03:30 +0000 Subject: [PATCH] a stop button on running background jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Killing a job meant asking the agent in words — a whole turn to say "please stop that", and only possible while the agent is responsive. Now it is a button on the chip. ── Finding the process ── The platform does not own these processes. Claude Code starts them and nothing records a pid. What it has is the file the job streams into, and the job is still holding that file open — so `fuser` gives the pid, and the file is resolved through the SAME scoped lookup the read path already uses: the id is validated against TASK_ID_RE, then matched only under this caller's own directories. An id that resolves to nothing kills nothing. Verified end to end rather than reasoned about: a stand-in job holding its output file open, `fuser` finding it, `{"stopped":true}`, process gone. SIGTERM, not SIGKILL — a dev server with children should get the chance to take them down with it. SIGKILL orphans them and leaves the port bound, which is worse than a stop that takes a second. ── Two things it deliberately does not do ── It does not dismiss the chip or write a `stopped` status locally. The real end arrives the way every other end does — the harness sees the exit and the transcript gets the row — so faking it would be a guess that usually agrees with reality and sometimes does not. A stop that fails leaves the chip running, which is the truth. It does not offer to stop a SUBAGENT. Those write a `.jsonl` transcript rather than holding a log open, so there is no fd to trace; it says so instead of pretending. `Square`, not `X`. X on the same chip means "close this", and confusing "hide the chip" with "kill the process" is the one mistake worth designing out — which is why stop appears only on running tasks and dismiss only on finished ones. tsgo clean, frontend builds, 851 pass / 7 fail unchanged. Co-Authored-By: Claude Opus 5 --- src/servers/api/chat/chat.ts | 11 +++++ src/servers/api/chat/claude-sessions.ts | 45 +++++++++++++++++++ .../Chat/components/BackgroundTaskTray.tsx | 42 +++++++++++++++-- 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index c80604f0..0e6d00f2 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -11,6 +11,7 @@ import { deleteClaudeSession, renameClaudeSession, loadBackgroundTask, + stopBackgroundTask, claudeSessionContext, liveSessionTitle, } from './claude-sessions'; @@ -270,6 +271,16 @@ chatRouter.get('/tasks/:id', async (ctx) => { return ctx.json(detail ?? { kind: 'pending' }); }); +// DELETE /chat/tasks/:id — stop a running background job. +// +// Exists so a job can be stopped by pressing a button rather than by asking the agent in words: a turn to +// say "please stop that" is a turn, and it only works while the agent is responsive. +chatRouter.delete('/tasks/:id', async (ctx) => { + const who = await chatIdentity(ctx.get('user')); + const result = await stopBackgroundTask(who, ctx.req.param('id')); + return ctx.json(result, result.stopped ? 200 : 400); +}); + // GET /chat/models — Claude tiers only (the runner is the `claude` CLI). chatRouter.get('/models', async (ctx: Context) => { const who = await chatIdentity(ctx.get('user')); diff --git a/src/servers/api/chat/claude-sessions.ts b/src/servers/api/chat/claude-sessions.ts index d01a1e29..e3a32c3f 100644 --- a/src/servers/api/chat/claude-sessions.ts +++ b/src/servers/api/chat/claude-sessions.ts @@ -719,6 +719,51 @@ function tailFile(osUser: AsUser, filePath: string, bytes: number): { text: stri return { text: truncated ? text.slice(text.indexOf('\n') + 1) : text, truncated }; } +/** + * Stop a running background job. + * + * The platform does not own these processes — Claude Code starts them, and nothing records a pid. What it + * does have is the file the job streams into, and the job is still holding that file open. So the pid + * comes from `fuser`, and the file is resolved through the SAME scoped lookup the read path uses: a task + * id is validated, then matched only under this caller's own directories. An id that resolves to nothing + * kills nothing. + * + * SIGTERM, not SIGKILL. A dev server with children should get the chance to take them down with it; + * SIGKILL orphans them and leaves the port bound, which is worse than a job that took a second to stop. + * + * Only agent-run SHELL jobs can be stopped this way. A subagent writes a `.jsonl` transcript rather than + * holding a log open, so there is no fd to trace and this reports that rather than pretending. + */ +export async function stopBackgroundTask( + who: ChatIdentity, + taskId: string, +): Promise<{ stopped: boolean; reason?: string }> { + if (!TASK_ID_RE.test(taskId)) return { stopped: false, reason: 'bad task id' }; + const found = findTaskOutput(who, taskId); + if (!found) return { stopped: false, reason: 'no such task' }; + if (found.endsWith('.jsonl')) return { stopped: false, reason: 'subagents cannot be stopped this way' }; + + const proc = Bun.spawn(['fuser', found], { stdout: 'pipe', stderr: 'ignore' }); + const out = await new Response(proc.stdout).text(); + await proc.exited; + + const pids = out + .split(/\s+/) + .map((t) => Number(t.replace(/\D/g, ''))) + .filter((n) => Number.isInteger(n) && n > 1); + + if (pids.length === 0) return { stopped: false, reason: 'already stopped' }; + + for (const pid of pids) { + try { + process.kill(pid, 'SIGTERM'); + } catch { + // Gone between listing and killing, or not ours. Either way there is nothing to do about it. + } + } + return { stopped: true }; +} + /** * What a background task is doing right now. Returns null when nothing has been written yet — which is * the normal state for the first second or two of a task's life, not an error. diff --git a/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx b/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx index 1aa93bf1..79caa898 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx @@ -1,6 +1,8 @@ import type { ReactNode } from 'react'; import { useEffect, useRef, useState } from 'react'; -import { Check, ChevronDown, CircleSlash, Loader2, Pin, X } from 'lucide-react'; +import { toast } from 'sonner'; +import { useClient } from 'hooks/useClient'; +import { Check, ChevronDown, CircleSlash, Loader2, Pin, Square, X } from 'lucide-react'; import type { Tone } from '@/components/Data'; import { toneText } from '@/components/Data'; import type { ChatMessage } from '../types'; @@ -32,6 +34,23 @@ export const BackgroundTaskTray = ({ messages }: BackgroundTaskTrayProps) => { const { tasks, pinned, unpinned, running, clearable, togglePin, dismiss, dismissFinished } = useBackgroundTasks(messages); const [openId, setOpenId] = useState(null); + const client = useClient(); + + /** + * Kill a running job. + * + * The chip is NOT dismissed here and its status is not faked. The job's real end arrives the way every + * other end does — the harness notices the exit and the transcript gets a `stopped` row — so writing + * that state locally would just be a guess that usually agrees with reality and occasionally does not. + * A stop that fails leaves the chip running, which is the truth. + */ + const stopTask = async (taskId: string) => { + try { + await client.delete(`/chat/tasks/${taskId}`); + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Could not stop that job'); + } + }; // Derived, not stored: dismissing the chip that is open closes the panel without a second piece of state // having to be kept in step with the first. @@ -47,6 +66,7 @@ export const BackgroundTaskTray = ({ messages }: BackgroundTaskTrayProps) => { // Finished only. A running chip is the only handle on work still going on, so closing one would hide // something you cannot get back to — the bulk clear draws the same line. onDismiss: task.status ? () => dismiss(task.taskId) : undefined, + onStop: task.status ? undefined : () => void stopTask(task.taskId), }); return ( @@ -116,13 +136,15 @@ type TaskChipProps = { onTogglePin: () => void; /** Absent for a running task — see the call site. */ onDismiss?: () => void; + /** Only for a RUNNING task: kill it. The inverse of `onDismiss`, which only finished tasks get. */ + onStop?: () => void; }; /** * The pill is a div wrapping several buttons rather than one button with controls inside it: a button * nested in a button is invalid, and the browser resolves it by swallowing one of the two clicks. */ -const TaskChip = ({ task, active, pinned, onClick, onTogglePin, onDismiss }: TaskChipProps) => { +const TaskChip = ({ task, active, pinned, onClick, onTogglePin, onDismiss, onStop }: TaskChipProps) => { const { status } = task; const tone = toneText[statusTone(status)]; const label = task.description || 'Background task'; @@ -162,12 +184,26 @@ const TaskChip = ({ task, active, pinned, onClick, onTogglePin, onDismiss }: Tas aria-pressed={pinned} title={pinned ? 'Unpin' : 'Pin to the top row'} aria-label={`${pinned ? 'Unpin' : 'Pin'} ${label}`} - className={`cursor-pointer py-1 pl-0.5 transition-colors ${onDismiss ? 'pr-0.5' : 'pr-2'} ${ + className={`cursor-pointer py-1 pl-0.5 transition-colors ${onDismiss || onStop ? 'pr-0.5' : 'pr-2'} ${ pinned ? 'text-duck-teal' : 'text-muted-foreground/50 hover:text-foreground' }`} > + {/* Stopping is for RUNNING tasks and dismissing is for finished ones, so the two never share the + slot. Square rather than X: X next to it means "close this chip", and confusing "hide the chip" + with "kill the process" is the one mistake worth designing out. */} + {onStop && ( + + )} {onDismiss && (