diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index 3fcde09b..1762f551 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -353,7 +353,10 @@ chatRouter.get('/tasks/:id', async (ctx) => { // 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')); + // `?force=1` escalates to SIGKILL. A query parameter rather than a second route: it is the same verb on + // the same thing, differing only in how hard it asks. + const force = ctx.req.query('force') === '1'; + const result = await stopBackgroundTask(who, ctx.req.param('id'), force); return ctx.json(result, result.stopped ? 200 : 400); }); diff --git a/src/servers/api/chat/claude-sessions.ts b/src/servers/api/chat/claude-sessions.ts index 222e5f67..bc0c395e 100644 --- a/src/servers/api/chat/claude-sessions.ts +++ b/src/servers/api/chat/claude-sessions.ts @@ -767,8 +767,14 @@ export async function pidsHoldingFile(who: ChatIdentity, path: string): Promise< * 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. + * SIGTERM by default, and SIGKILL only when asked. 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. + * + * `force` exists because that reasoning stops applying the moment a process ignores SIGTERM: there is + * then no graceful shutdown to protect, only a job that cannot be stopped from the UI at all. The comment + * in task-restart.ts already said "that is what the second click is for" — this is the second click + * actually doing something different from the first. * * 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. @@ -776,7 +782,8 @@ export async function pidsHoldingFile(who: ChatIdentity, path: string): Promise< export async function stopBackgroundTask( who: ChatIdentity, taskId: string, -): Promise<{ stopped: boolean; reason?: string }> { + force = false, +): Promise<{ stopped: boolean; reason?: string; signal?: 'TERM' | 'KILL' }> { 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' }; diff --git a/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx b/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx index 1f1c471e..cd2c9212 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/BackgroundTaskTray.tsx @@ -1,8 +1,9 @@ import type { ReactNode } from 'react'; import { useEffect, useRef, useState } from 'react'; import { toast } from 'sonner'; +import { shouldOfferKill } from '../kill-escalation'; import { useClient } from 'hooks/useClient'; -import { Check, ChevronDown, CircleSlash, Loader2, Pause, Pin, Play, RotateCw, Square, X } from 'lucide-react'; +import { Check, ChevronDown, CircleSlash, Loader2, Pause, Pin, Play, RotateCw, Skull, Square, X } from 'lucide-react'; import type { Tone } from '@/components/Data'; import { toneText } from '@/components/Data'; import type { ChatMessage } from '../types'; @@ -56,7 +57,20 @@ export const BackgroundTaskTray = ({ messages, sessionId }: BackgroundTaskTrayPr * 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) => { + /** + * When Stop was pressed on a job that is still running. + * + * SIGTERM is a request, and a process is free to ignore it. Until now the second click sent the same + * signal again — `task-restart.ts` says "that is what the second click is for", and it was not. This is + * what turns the button into Kill once the polite version has visibly failed. + * + * Local state rather than `consumeStopRequested`, which clears itself on the next notification because + * its job is relabelling a finished task. This one has to survive for as long as the job stubbornly + * does not. + */ + const [stopAttempted, setStopAttempted] = useState>({}); + + const stopTask = async (taskId: string, force = false) => { // Pressing stop is deliberate and aimed at one job, so its death is worth reacting to even if an // agent started it — the preview may well be pointed at exactly this. markUserBackgroundJob(taskId); @@ -64,13 +78,31 @@ export const BackgroundTaskTray = ({ messages, sessionId }: BackgroundTaskTrayPr // (observed live at exit code 144, read as "failed") — so the client relabels it itself. See // `background-job-origin.ts`. markStopRequested(taskId); + setStopAttempted((prev) => ({ ...prev, [taskId]: Date.now() })); try { - await client.delete(`/chat/tasks/${taskId}`); + await client.delete(`/chat/tasks/${taskId}${force ? '?force=1' : ''}`); } catch (err) { toast.error(err instanceof Error ? err.message : 'Could not stop that job'); } }; + /** + * Has a stop been asked for, and ignored? + * + * The delay is the point. A job that exits cleanly does so in well under a second, so offering Kill + * immediately would train people to reach for it first — and SIGKILL gives a process no chance to take + * its children down or release a port, which is worse than waiting a moment. + */ + const [now, setNow] = useState(Date.now()); + useEffect(() => { + // Only ticks while something is pending, so an idle tray does no work. + if (!Object.keys(stopAttempted).length) return; + const id = setInterval(() => setNow(Date.now()), 1000); + return () => clearInterval(id); + }, [stopAttempted]); + + const killOffered = (taskId: string) => shouldOfferKill(stopAttempted[taskId], now); + /** * Run this job's command again, without asking the agent. * @@ -126,7 +158,7 @@ export const BackgroundTaskTray = ({ messages, sessionId }: BackgroundTaskTrayPr // 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), + onStop: task.status ? undefined : () => void stopTask(task.taskId, killOffered(task.taskId)), }); return ( @@ -135,7 +167,8 @@ export const BackgroundTaskTray = ({ messages, sessionId }: BackgroundTaskTrayPr setOpenId(null)} - onStop={open.status ? undefined : () => void stopTask(open.taskId)} + onStop={open.status ? undefined : (force?: boolean) => void stopTask(open.taskId, force)} + killOffered={killOffered(open.taskId)} onPause={(next) => pauseTask(open.taskId, next)} paused={pausedIds.includes(open.taskId)} onRestart={() => void restartTask(open.taskId)} @@ -292,7 +325,10 @@ const TaskChip = ({ task, active, pinned, onClick, onTogglePin, onDismiss, onSto type TaskPanelProps = { /** Present only while the task is running — the panel shows a real button rather than a bare glyph. */ - onStop?: () => void; + /** Takes `force`: the same control, asking harder once a polite stop has been ignored. */ + onStop?: (force?: boolean) => void; + /** Stop was pressed and the job is still running, so the button becomes Kill. */ + killOffered?: boolean; /** Freeze and thaw the process. Present while it is running; the label follows the state. */ onPause?: (paused: boolean) => Promise; /** Frozen right now — owned by the tray, so closing this panel does not forget it. */ @@ -303,7 +339,15 @@ type TaskPanelProps = { onClose: () => void; }; -const TaskPanel = ({ task, onClose, onStop, onPause, onRestart, paused = false }: TaskPanelProps) => { +const TaskPanel = ({ + killOffered = false, + task, + onClose, + onStop, + onPause, + onRestart, + paused = false, +}: TaskPanelProps) => { const running = !task.status; const { data, isLoading } = useTaskDetail(task.taskId, running); const bodyRef = useRef(null); @@ -364,12 +408,20 @@ const TaskPanel = ({ task, onClose, onStop, onPause, onRestart, paused = false } {running && onStop && ( )}