diff --git a/src/main/ipc/worktrees/removal/remove-folder-workspace.ts b/src/main/ipc/worktrees/removal/remove-folder-workspace.ts index c03b1449136..916f4574a92 100644 --- a/src/main/ipc/worktrees/removal/remove-folder-workspace.ts +++ b/src/main/ipc/worktrees/removal/remove-folder-workspace.ts @@ -43,6 +43,9 @@ export async function removeFolderWorkspace( : {}), localProvider: sshPtyProvider ?? getLocalPtyProvider(), onPtyStopped: clearProviderPtyState, + // A structured session is on no PTY surface, so the sweeps above leave it attached to a + // workspace this removal is about to forget. Closed best-effort, exactly like those PTYs. + closeStructuredSessions: true, ...(externalHost ? { includeProviderInventory: ownerHost?.kind === 'ssh' && Boolean(sshPtyProvider), diff --git a/src/main/runtime/folder-workspace-pty-teardown.ts b/src/main/runtime/folder-workspace-pty-teardown.ts index f5a5b7df4b3..e54c9165c46 100644 --- a/src/main/runtime/folder-workspace-pty-teardown.ts +++ b/src/main/runtime/folder-workspace-pty-teardown.ts @@ -29,6 +29,9 @@ export async function teardownFolderWorkspacePtys( ...(connectionId ? { resolvedConnectionId: connectionId } : {}), localProvider: ptyProvider, onPtyStopped: deps.onPtyStopped ?? undefined, + // A structured session is on no PTY surface, so the sweeps above leave it attached to a + // workspace this removal is about to forget. Closed best-effort, exactly like those PTYs. + closeStructuredSessions: true, ...(connectionId ? { includeProviderInventory: Boolean(sshPtyProvider), includeLocalRegistry: false } : {}) diff --git a/src/main/runtime/orca-runtime-remove-orphan-or-folder-worktree.ts b/src/main/runtime/orca-runtime-remove-orphan-or-folder-worktree.ts index 01f4f305803..6c011014b6c 100644 --- a/src/main/runtime/orca-runtime-remove-orphan-or-folder-worktree.ts +++ b/src/main/runtime/orca-runtime-remove-orphan-or-folder-worktree.ts @@ -44,6 +44,9 @@ export async function removeOrphanOrFolderWorktree({ : {}), localProvider: ptyProvider, onPtyStopped: runtime.onPtyStopped ?? undefined, + // A structured session is on no PTY surface, so the sweeps above leave it attached to a + // workspace this removal is about to forget. Closed best-effort, exactly like those PTYs. + closeStructuredSessions: true, ...(externalOrphanHost ? { includeProviderInventory: orphanHost?.kind === 'ssh' && Boolean(sshPtyProvider), diff --git a/src/main/runtime/orchestration/structured-worker-group-addressing.test.ts b/src/main/runtime/orchestration/structured-worker-group-addressing.test.ts index 7eff64bc61f..604c3d1f13d 100644 --- a/src/main/runtime/orchestration/structured-worker-group-addressing.test.ts +++ b/src/main/runtime/orchestration/structured-worker-group-addressing.test.ts @@ -11,6 +11,7 @@ vi.mock('../../native-chat/agent-session-wire/structured-agent-session-registry' const { listAddressableStructuredWorkers, structuredWorkerAgentStatus } = await import('./structured-worker-group-addressing') const { resolveGroupAddress } = await import('./groups') +const { sendGroupMessage } = await import('../rpc/methods/orchestration-send-group') const { mintStructuredWorkerHandle, mintStructuredWorkerPaneKey, @@ -164,3 +165,56 @@ describe('group addressing and structured workers', () => { expect(structuredWorkerAgentStatus(SESSION_ID)).toBeNull() }) }) + +describe('sendGroupMessage actually composes structured workers in', () => { + beforeEach(() => { + structuredWorkerIdentities.clear() + hostRef.current = null + }) + + /** + * Drives the real `sendGroupMessage`, not `resolveGroupAddress`. + * + * The suite above hand-composed `[PTY_TERMINAL, ...listAddressableStructuredWorkers()]` itself, + * so deleting the composition at the call site left it green — the exact regression the fix + * describes could come straight back. This test owns that seam. + */ + it('addresses a structured worker that only the call site can enumerate', async () => { + const handle = registerWorker() + installHost({}) + const inserted: { to: string }[] = [] + const db = { + getLegacyAdoptedRunMailboxOwner: () => null, + getCurrentRunForPane: () => undefined, + getActiveDispatchMailboxOwners: () => [], + getRunMailboxOwnerIdsForHandle: () => [], + insertMessages: (rows: { to: string }[]) => { + inserted.push(...rows) + return rows.map((row, index) => ({ id: `m${index}`, to_handle: row.to, type: 'status' })) + } + } + const runtime = { + // No PTY terminals at all: if the call site does not compose structured workers in, the + // group resolves empty and this throws instead of delivering. + listTerminals: async () => ({ terminals: [] }), + getAgentStatusForHandle: () => 'idle', + getLiveTerminalPaneKey: () => structuredWorkerIdentities.get(handle)!.paneKey, + notifyMessageArrived: () => {} + } + await sendGroupMessage({ + params: { subject: 's', body: 'b', type: 'status', priority: 'normal' }, + runtime: runtime as never, + db: db as never, + from: 'term_sender', + groupAddress: '@all', + senderPaneKey: undefined, + senderRunId: undefined, + explicitRunId: undefined, + legacyCoordinatorRunId: undefined, + revalidateLegacyCoordinator: undefined, + recordMutationReceipt: undefined, + withSendWarnings: (receipt) => receipt + } as never) + expect(inserted.map((row) => row.to)).toEqual([handle]) + }) +}) diff --git a/src/main/runtime/structured-session-worktree-teardown.test.ts b/src/main/runtime/structured-session-worktree-teardown.test.ts index d3af1d0300c..53c9f126387 100644 --- a/src/main/runtime/structured-session-worktree-teardown.test.ts +++ b/src/main/runtime/structured-session-worktree-teardown.test.ts @@ -8,6 +8,7 @@ vi.mock('../native-chat/agent-session-wire/structured-agent-session-registry', ( })) const { killAllProcessesForWorktree } = await import('./worktree-teardown') +const { classifyWorktreeForceDeleteReason } = await import('../../shared/worktree/removal') const { listLiveStructuredSessionsForWorktree } = await import('./structured-session-worktree-teardown') @@ -99,6 +100,49 @@ describe('worktree teardown and structured agent sessions', () => { await expect(killAllProcessesForWorktree(WORKTREE, destructiveDeps())).rejects.toThrow(/force/i) }) + it('classifies for the desktop Force Delete button, not just the CLI', async () => { + // The #11960 dead end, and the shape this file's own comments warn about: the desktop + // affordance comes ONLY from the classifier, and an ordinary delete already passes force:true + // for the dirty-file skip — so a refusal with no matcher shows raw CLI wording with no button. + installHost({ records: [record('s1', WORKTREE)] }) + const error = await killAllProcessesForWorktree(WORKTREE, destructiveDeps()).catch( + (thrown: Error) => thrown.message + ) + expect(classifyWorktreeForceDeleteReason(error as string, true)).toBe('running-agent-session') + // Nulled once the waiver is spent, exactly as `unstopped-pty` is, so the button does not + // reappear on a delete the user already forced. + expect(classifyWorktreeForceDeleteReason(error as string, true, true)).toBeNull() + }) + + it('keeps session ids out of a message users and agents read', async () => { + // A session id is one tab-id hop from the random pane key that gates a worker's mailbox, and + // this string reaches CLI output and a desktop toast. A count and the providers are what a + // user deciding whether to force actually needs. + installHost({ records: [record('s1', WORKTREE)] }) + const error = await killAllProcessesForWorktree(WORKTREE, destructiveDeps()).catch( + (thrown: Error) => thrown.message + ) + expect(error).not.toContain('s1') + expect(error).toContain('1 running agent session') + }) + + it('closes best-effort for a folder-workspace removal, which requires no stop proof', async () => { + // Those paths sweep and kill PTYs without `requirePhysicalStop`, so the structured sweep used + // to no-op there and left a live session bound to a workspace Orca was about to forget. They + // do not refuse: the root is shared so no checkout vanishes, and one of them is a never-throw + // forget that a refusal would wedge. + const host = installHost({ records: [record('s1', WORKTREE)] }) + await expect( + killAllProcessesForWorktree(WORKTREE, { + localProvider, + includeProviderInventory: false, + includeLocalRegistry: false, + closeStructuredSessions: true + }) + ).resolves.toMatchObject({ structuredStopped: 1 }) + expect(host.closed).toEqual(['s1']) + }) + it('closes them under force instead of orphaning the child', async () => { const host = installHost({ records: [record('s1', WORKTREE), record('s2', WORKTREE)] }) const result = await killAllProcessesForWorktree( diff --git a/src/main/runtime/structured-session-worktree-teardown.ts b/src/main/runtime/structured-session-worktree-teardown.ts index 15801835f89..226f785f039 100644 --- a/src/main/runtime/structured-session-worktree-teardown.ts +++ b/src/main/runtime/structured-session-worktree-teardown.ts @@ -59,14 +59,19 @@ export function listLiveStructuredSessionsForWorktree( .map((record) => ({ sessionId: record.sessionId, agent: record.provider })) } +/** + * Counts and providers, never session ids. + * + * A session id is one tab-id hop from the random pane key that gates a worker's mailbox, and this + * string reaches agent-readable CLI output and a desktop toast. The count and the providers are + * what a user deciding whether to force actually needs; the ids identify nothing they can act on. + */ export function describeLiveStructuredSessions( - worktreeId: string, sessions: readonly LiveStructuredSessionInWorkspace[] ): string { const noun = sessions.length === 1 ? 'agent session' : 'agent sessions' - return `${sessions.length} running ${noun} in ${worktreeId} (${sessions - .map((session) => `${session.agent}:${session.sessionId}`) - .join(', ')})` + const providers = [...new Set(sessions.map((session) => session.agent))].sort().join(', ') + return `${sessions.length} running ${noun} (${providers})` } /** @@ -79,6 +84,11 @@ export async function closeStructuredSessionsForWorktree( worktreeId: string, runtime?: StructuredWorktreeSweepRuntime ): Promise<{ closed: number; unstopped: LiveStructuredSessionInWorkspace[] }> { + // No `afterClose` for a dispatched worker: `host.close` drops the holds, so nothing keeps a + // provider child un-evictable, but the dispatch's redrive subscription and registry entry do + // survive until it settles by another verb. That is a bounded leak, not a hazard — and passing + // one here would mean resolving a dispatch id per session on a teardown path that must stay + // inside the sweep deadline. const sessions = listLiveStructuredSessionsForWorktree(worktreeId) const unstopped: LiveStructuredSessionInWorkspace[] = [] let closed = 0 diff --git a/src/main/runtime/structured-worker-agent-presence.test.ts b/src/main/runtime/structured-worker-agent-presence.test.ts new file mode 100644 index 00000000000..ca31e891cc9 --- /dev/null +++ b/src/main/runtime/structured-worker-agent-presence.test.ts @@ -0,0 +1,46 @@ +/** + * `isTerminalRunningAgent` for a worker that IS a structured agent session. + * + * This seam had no test at all: nothing in the repo referenced `isLiveStructuredAgent`, so the + * early return could be deleted and every suite stayed green. `dispatch --to --inject` + * depends on it — without it `getLiveLeaf` throws, the catch returns false, and a coordinator is + * told its worker is a bare shell (`no_agent_detected`). + */ + +import { describe, expect, it, vi } from 'vitest' +import { RuntimeTerminalAgentPresence } from './runtime-terminal-agent-presence' + +function presence(isLiveStructuredAgent: (handle: string) => boolean) { + const getLiveLeaf = vi.fn(() => { + // Exactly what the runtime does for a handle with no pane, and the reason the catch below + // used to swallow the question into `false`. + throw new Error('terminal_handle_stale') + }) + return { + getLiveLeaf, + presence: new RuntimeTerminalAgentPresence({ + isLiveStructuredAgent, + getLivePty: () => null, + getLiveLeaf: getLiveLeaf as never, + getPrimaryLeaf: () => null, + getTrackedPty: () => null, + getTabTitle: () => null, + getForegroundProcess: () => null + }) + } +} + +describe('agent presence for a structured worker', () => { + it('reports the session as running an agent without probing a pane', async () => { + const { presence: subject, getLiveLeaf } = presence(() => true) + await expect(subject.isRunning('structworker_1')).resolves.toBe(true) + // A structured session IS the agent; there is no foreground process to recognise, and the + // leaf probe would only throw. + expect(getLiveLeaf).not.toHaveBeenCalled() + }) + + it('still answers false for a handle that is not a live structured worker', async () => { + const { presence: subject } = presence(() => false) + await expect(subject.isRunning('term_gone')).resolves.toBe(false) + }) +}) diff --git a/src/main/runtime/structured-worker-terminal-read.test.ts b/src/main/runtime/structured-worker-terminal-read.test.ts index 714357a6c0b..9bad5d46d1e 100644 --- a/src/main/runtime/structured-worker-terminal-read.test.ts +++ b/src/main/runtime/structured-worker-terminal-read.test.ts @@ -87,15 +87,29 @@ describe('reading a structured worker through the terminal-read path', () => { expect(read?.truncated).toBe(false) }) - it('pages by cursor and limit exactly as a PTY read does', () => { + it('honours limit, and claims no cursor space it cannot honour', () => { const handle = registerWorker() installHost({ items: [message('i1', 'a'), message('i2', 'b'), message('i3', 'c')] }) - const first = readStructuredWorkerTerminal({ handle, db: null, cursor: 0, limit: 2 }) - expect(first?.tail).toEqual(['[assistant] a', '[assistant] b']) - expect(first?.nextCursor).toBe('2') - const second = readStructuredWorkerTerminal({ handle, db: null, cursor: 2 }) - expect(second?.tail).toEqual(['[assistant] c']) - expect(second?.latestCursor).toBe('3') + const read = readStructuredWorkerTerminal({ handle, db: null, limit: 2 }) + expect(read?.tail).toEqual(['[assistant] b', '[assistant] c']) + // No index is advertised: the next read re-projects a sliding window, so 0/length would name + // positions that address different lines by then. + expect(read?.nextCursor).toBeNull() + expect(read?.oldestCursor).toBeUndefined() + expect(read?.latestCursor).toBeUndefined() + }) + + it('refuses a cursor read rather than silently misdelivering lines', () => { + // The PTY cursor indexes an append-only completed-line buffer with a monotone count. This + // window is a bounded tail re-projected every read, so a saved index addresses different lines + // as the journal grows — and `truncated` could never fire to say so, because it tests + // `cursor < oldestCursor` and `oldestCursor` was always 0. A poller would get wrong or + // duplicated lines with `truncated:false`. + const handle = registerWorker() + installHost({ items: [message('i1', 'a')] }) + expect(() => readStructuredWorkerTerminal({ handle, db: null, cursor: 0 })).toThrow( + /worker-read --source transcript/ + ) }) it('reports dropped history as truncated rather than pretending the page is whole', () => { diff --git a/src/main/runtime/structured-worker-terminal-read.ts b/src/main/runtime/structured-worker-terminal-read.ts index 691f125175b..a98d846d572 100644 --- a/src/main/runtime/structured-worker-terminal-read.ts +++ b/src/main/runtime/structured-worker-terminal-read.ts @@ -6,9 +6,15 @@ * coordinator standing a peer does not have, so the only agent-to-agent read verb refused to * resolve the handle. This serves the same verb from the session's journal. * - * The result is a plain `RuntimeTerminalRead` — the journal is projected to LINES and paged by the - * very reader the PTY tail uses — so nothing an agent reads reveals which kind of worker answered, - * and cursor, limit, `truncated` and `nextCursor` keep their existing meanings. + * The result is a plain `RuntimeTerminalRead` — the journal is projected to LINES and bounded by + * the very reader the PTY tail uses — so nothing an agent reads reveals which kind of worker + * answered. `limit` and `truncated` keep their existing meanings. + * + * `cursor` does NOT, and is refused rather than approximated. The PTY contract is an index into an + * append-only completed-line buffer with a monotone count; a session journal is a bounded tail + * re-projected on every read, so the same index addresses different lines over time and + * `oldestCursor` can never grow to admit it. Pagination with a real anchor lives on + * `worker-read --source transcript`. * * READ ONLY, deliberately. `terminal.show` still refuses a structured handle: synthesising a * `ptyId`/`leafId`/`paneRuntimeId` would hand every public terminal verb something that looks @@ -45,6 +51,20 @@ export function readStructuredWorkerTerminal(args: { if (!identity) { return null } + if (args.cursor !== undefined) { + // A numeric cursor cannot be re-anchored here. `terminal.read`'s cursor is an index into an + // append-only completed-line buffer with a monotone count; this window is a BOUNDED tail that + // is re-projected every read, so the same index means different lines as the journal grows, + // and `truncated` (`cursor < oldestCursor`) could never fire to say so because `oldestCursor` + // is always 0. Serving it would silently return wrong or duplicated lines to a poller. The + // journal does have stable item identity, but the terminal cursor is a number on the wire and + // cannot carry it — `worker-read --source transcript` already has that contract, including + // `source_changed` detection. + throw new Error( + `${args.handle} serves recent output without a cursor; its history is not line-addressable. ` + + 'Read it without --cursor, or page it with `orca orchestration worker-read --source transcript`.' + ) + } const page = readStructuredJournalPage(identity.sessionId) if (!page) { // Honest refusal, and the same one the send lane reports: an empty tail would read as "this @@ -56,18 +76,24 @@ export function readStructuredWorkerTerminal(args: { const lines = bounded.messages.flatMap((message) => formatWorkerTranscriptMessage(message).split('\n') ) - return readTerminalTail({ + const read = readTerminalTail({ handle: args.handle, status: structuredWorkerTerminalState(observeStructuredWorker(identity).status), previewLines: lines, - completedLines: lines, - // Every projected line is complete; a session journal has no half-written trailing line. + // Unreachable without a cursor, and deliberately empty rather than a copy of `lines`: a + // running turn's text is still growing, so calling it "completed" is the `"hel"`/`"hello"` + // hazard the PTY reader guards against. + completedLines: [], partialLine: '', - completedLineCount: lines.length, + completedLineCount: 0, // Older items really were dropped, by the page limit or the byte bound; `truncated` is how the // PTY read already says exactly that. bufferTruncated: page.hasOlder || bounded.limited, - ...(args.cursor === undefined ? {} : { cursor: args.cursor }), ...(args.limit === undefined ? {} : { limit: args.limit }) }) + // No cursor space is claimed, because none exists here. `nextCursor: null` is the contract's own + // "nothing to continue from"; emitting 0/length would advertise an index the next read cannot + // honour. + const { oldestCursor: _oldest, latestCursor: _latest, ...withoutCursorSpace } = read + return { ...withoutCursorSpace, nextCursor: null } } diff --git a/src/main/runtime/worktree-teardown.ts b/src/main/runtime/worktree-teardown.ts index 1ce60d3ee65..82fa055cc3a 100644 --- a/src/main/runtime/worktree-teardown.ts +++ b/src/main/runtime/worktree-teardown.ts @@ -2,6 +2,8 @@ import type { IPtyProvider } from '../providers/types' import type { OrcaRuntimeService } from './orca-runtime' import { isUnstoppedPtyRemovalError, + RUNNING_AGENT_SESSION_REMOVAL_PREFIX, + UNSTOPPED_PTY_DETAIL_SEPARATOR, WORKTREE_TEARDOWN_FORCE_HINT, WORKTREE_TEARDOWN_TIMEOUT_PREFIX } from '../../shared/worktree/removal' @@ -40,6 +42,15 @@ export type WorktreeTeardownDeps = { allowUnverifiedStop?: boolean includeProviderInventory?: boolean includeLocalRegistry?: boolean + /** + * Close structured agent sessions best-effort, for a destructive removal that does NOT require + * PTY-stop proof — the folder-workspace paths, which sweep and kill PTYs the same way. + * + * Separate from `requirePhysicalStop` because the two questions are different: that one asks + * whether a stop must be PROVEN before files are touched, and it is what licenses a refusal. + * Reconciliation sweeps set neither; they repair state and must never close anything. + */ + closeStructuredSessions?: boolean } export type WorktreeTeardownResult = { @@ -92,7 +103,7 @@ export async function killAllProcessesForWorktree( // of the three surfaces below, so all three answered zero and removal deleted the checkout out // from under a running provider child. Refusing costs nothing when there are none, and the check // is synchronous, so a destructive removal fails fast instead of after the whole sweep budget. - const structuredStopped = await sweepStructuredSessions(worktreeId, deps) + const structuredStopped = await sweepStructuredSessions(worktreeId, deps, deadline, deadlineError) const sweeps = createWorktreeSweepTracker() const stopAttempts = new Map>() const stopPty = ( @@ -243,7 +254,10 @@ export async function killAllProcessesForWorktree( } } else { const summary = describeUnstoppedPtys(worktreeId, failedPtyIds, verdict) - if (!deps.allowUnverifiedStop) { + // Only a proof-requiring removal may refuse. A folder-workspace removal shares its root, so no + // checkout disappears under the child — the harm is a session left pointing at a workspace Orca + // has forgotten — and one of those paths is a never-throw forget, which a refusal would wedge. + if (deps.requirePhysicalStop && !deps.allowUnverifiedStop) { throw new Error(`${summary}. ${WORKTREE_TEARDOWN_FORCE_HINT}`) } // Why: force is the documented escape hatch, so removal continues — but the @@ -271,31 +285,50 @@ export async function killAllProcessesForWorktree( * the same `--force` escape hatch. Force closes them properly instead of orphaning a child against * a `cwd` that is about to disappear. * - * Only the destructive path (`requirePhysicalStop`) participates: the best-effort callers are - * reconciliation sweeps that must never fail a repair, and they delete nothing. + * Two callers participate, for different reasons. A proof-requiring removal (`requirePhysicalStop`) + * refuses, then closes under force. A folder-workspace removal (`closeStructuredSessions`) closes + * best-effort without refusing: it shares its root so no checkout vanishes under the child, and one + * of those paths is a never-throw forget that a refusal would wedge. Reconciliation sweeps set + * neither — they repair state, delete nothing, and must never close a session. */ async function sweepStructuredSessions( worktreeId: string, - deps: WorktreeTeardownDeps + deps: WorktreeTeardownDeps, + deadline: number, + deadlineError: Error ): Promise { - if (!deps.requirePhysicalStop) { + if (!deps.requirePhysicalStop && !deps.closeStructuredSessions) { return 0 } const live = listLiveStructuredSessionsForWorktree(worktreeId) if (live.length === 0) { return 0 } - if (!deps.allowUnverifiedStop) { + // Only a proof-requiring removal may refuse. A folder-workspace removal shares its root, so no + // checkout disappears under the child — the harm is a session left pointing at a workspace Orca + // has forgotten — and one of those paths is a never-throw forget, which a refusal would wedge. + if (deps.requirePhysicalStop && !deps.allowUnverifiedStop) { + // The prefix is what the desktop classifier matches on; without it the toast shows raw CLI + // wording and hides the Force Delete button — the #11960 dead end this file already documents. throw new Error( - `Refusing to remove ${worktreeId}: ${describeLiveStructuredSessions(worktreeId, live)}. ${WORKTREE_TEARDOWN_FORCE_HINT}` + `${RUNNING_AGENT_SESSION_REMOVAL_PREFIX} ${worktreeId}${UNSTOPPED_PTY_DETAIL_SEPARATOR}${describeLiveStructuredSessions(live)}. ${WORKTREE_TEARDOWN_FORCE_HINT}` ) } - const { closed, unstopped } = await closeStructuredSessionsForWorktree(worktreeId, deps.runtime) + // Raced against the same sweep budget every PTY surface is bounded by: `host.close` awaits a + // provider round trip, and a wedged one would otherwise hang `worktree rm --force` forever with + // no timeout error at all. On expiry the force path reports the timeout exactly as the PTY + // sweeps do rather than proceeding as if the sessions had closed. + const { closed, unstopped } = await settleBeforeDeadline( + () => closeStructuredSessionsForWorktree(worktreeId, deps.runtime), + { closed: 0, unstopped: live }, + deadline, + deadlineError + ) if (unstopped.length > 0) { // Force is the documented escape hatch, so removal continues — but say so, because the child // outliving its `cwd` is the failure this sweep exists to make visible. console.warn( - `[worktree-teardown] forcing removal with ${describeLiveStructuredSessions(worktreeId, unstopped)} still attached` + `[worktree-teardown] forcing removal of ${worktreeId} with ${describeLiveStructuredSessions(unstopped)} still attached` ) } return closed diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.ts index 1c013da0da6..946e99eadee 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.ts @@ -74,6 +74,23 @@ export function getDeleteWorktreeToastCopy( isDestructive: false } } + if (forceDeleteReason === 'running-agent-session') { + return { + title: translate( + 'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5', + 'Failed to delete workspace {{value0}}', + { value0: worktreeName } + ), + // Why this is not the "could not confirm" wording: Orca watched these sessions stay + // attached, so there is no doubt to waive — Force Delete ends a conversation that is + // running right now, and any work it holds goes with it. + description: translate( + 'auto.components.sidebar.delete.worktree.toast.runningAgentSession', + 'This workspace still has running agent sessions, so Orca stopped before deleting any files. Force Delete will close them and discard any work they hold.' + ), + isDestructive: false + } + } if (forceDeleteReason === 'missing-registration') { return { title: translate( diff --git a/src/shared/worktree/removal.ts b/src/shared/worktree/removal.ts index 8f5a6c4a4d2..59e5803eddb 100644 --- a/src/shared/worktree/removal.ts +++ b/src/shared/worktree/removal.ts @@ -15,6 +15,7 @@ export type WorktreeForceDeleteReason = | 'orphan-directory' | 'missing-registration' | 'unstopped-pty' + | 'running-agent-session' // Why: everything before this separator is the worktree id — a user-chosen filesystem path. // Only the detail after it is Orca's own wording, so verdict matchers anchor on the boundary @@ -32,6 +33,17 @@ export const UNSTOPPED_PTY_LIVE_DETAIL_PREFIX = 'still live:' // its own matcher the force affordance stayed hidden for the very case it was added for. export const WORKTREE_TEARDOWN_TIMEOUT_PREFIX = 'Timed out waiting for physical PTY teardown:' +// Why (#11960 again): a running agent SESSION blocks removal for the same reason an unstopped PTY +// does, and it needs its own prefix for the same reason the timeout above needed one — the desktop +// force affordance comes only from the classifier below, so a refusal with no matcher shows raw +// CLI wording and hides the Force Delete button. Matcher and hint stay in this file together. +export const RUNNING_AGENT_SESSION_REMOVAL_PREFIX = + 'Refusing to remove worktree with running agent sessions:' + +export function isRunningAgentSessionRemovalError(error: string): boolean { + return error.includes(RUNNING_AGENT_SESSION_REMOVAL_PREFIX) +} + export function isUnstoppedPtyRemovalError(error: string): boolean { return ( error.includes(UNSTOPPED_PTY_REMOVAL_PREFIX) || error.includes(WORKTREE_TEARDOWN_TIMEOUT_PREFIX) @@ -103,6 +115,12 @@ export function classifyWorktreeForceDeleteReason( if (isUnstoppedPtyRemovalError(error)) { return allowUnverifiedPtyStop ? null : 'unstopped-pty' } + // Same placement and the same reason: decided BEFORE the `force` guard, because an ordinary + // desktop delete already passes force:true to skip the dirty-file prompt and that says nothing + // about whether the user has waived closing a live agent session. Only the waiver itself does. + if (isRunningAgentSessionRemovalError(error)) { + return allowUnverifiedPtyStop ? null : 'running-agent-session' + } if (force) { return null }