fix(worktrees): fence, bound and word the structured-session sweep

Review follow-ups on the close-first structured sweep. The close-first
direction is unchanged; four things it got wrong are not.

Host fence. `listLiveStructuredSessionsForWorktree` matched on
`location.workspaceId` alone, and a `repoId::path` id names a DIFFERENT
workspace on every host (STA-4343). Once the sweep started closing rather
than refusing, deleting a local workspace could close a live chat on an SSH
or paired-runtime copy of the same id. It now takes the same two host fields
the PTY sweeps already fence on, compared against the session's own
`location.executionHostId`; neither field set means this machine.

Shared budget. The close ran to completion before the first PTY sweep was
constructed, and it is serial with a provider round trip per session — so a
slow one spent the whole budget and the sweeps then rejected with a timeout
for a stop they never attempted. It is now issued first but joined before the
verdict, so the agent plane is still asked ahead of the terminal plane while
the two share the clock.

Timeout wording. The close raced the deadline fail-closed, and that sentinel
carries the PTY timeout prefix, which the classifier reads first — so a
wedged session close refused in terminal wording and refused identically
again under the Force Delete meant to clear it (#11960). A close that ran out
of time is now a session the removal could not confirm closed, which is what
the refusal already words. Tracked, so a forced removal still waits out the
abandoned-sweep grace before deleting files.

Verdict fidelity. `closeStructuredAgentSessionChild` re-observes after the
close, and that verdict was being discarded — so a session Orca watched stay
attached and one it merely could not reach produced the same message, while
the toast asserted "could not confirm" for both. `removal.ts` documents
flattening those two as the thing not to do. The unclosed sessions now carry
their post-close status, the detail uses the shared `still live:` marker, and
the toast branches on it like the PTY pair above it.

Also: the close takes the enumerated list instead of re-deriving it, so it no
longer runs every liveness observation twice or names a session it never
touched; and both teardown log lines count structured closes, since closing a
chat is now an ordinary outcome of this verb.
This commit is contained in:
Merge Sim
2026-09-09 12:16:16 -07:00
parent 5dc2df91cb
commit 2d8e6b3478
10 changed files with 350 additions and 79 deletions
@@ -40,11 +40,17 @@ export async function stopPtysForDestructiveWorktreeRemoval(
...(allowUnverifiedStop ? { allowUnverifiedStop: true } : {}),
...(connectionId ? { includeLocalRegistry: false } : {})
})
// Structured sessions are counted here too: closing a user's chat is now an ordinary outcome
// of this verb, and a removal that closed one but no PTY would otherwise log nothing at all.
const structuredStopped = teardownResult.structuredStopped ?? 0
const total =
teardownResult.runtimeStopped + teardownResult.providerStopped + teardownResult.registryStopped
teardownResult.runtimeStopped +
teardownResult.providerStopped +
teardownResult.registryStopped +
structuredStopped
if (total > 0) {
console.info(
`[worktree-teardown] ${worktreeId} killed runtime=${teardownResult.runtimeStopped} provider=${teardownResult.providerStopped} registry=${teardownResult.registryStopped}`
`[worktree-teardown] ${worktreeId} killed runtime=${teardownResult.runtimeStopped} provider=${teardownResult.providerStopped} registry=${teardownResult.registryStopped} structured=${structuredStopped}`
)
}
}
@@ -149,13 +149,17 @@ export class OrcaRuntimeWithPtyForegroundProcessReads extends OrcaRuntimeWithSta
...(allowUnverifiedStop ? { allowUnverifiedStop: true } : {}),
...(connectionId ? { includeLocalRegistry: false } : {})
})
// Structured sessions are counted here too, mirroring the IPC path: closing a user's chat is
// now an ordinary outcome of this verb, and a removal that closed one but no PTY logged nothing.
const structuredStopped = teardownResult.structuredStopped ?? 0
const total =
teardownResult.runtimeStopped +
teardownResult.providerStopped +
teardownResult.registryStopped
teardownResult.registryStopped +
structuredStopped
if (total > 0) {
console.info(
`[worktree-teardown] ${worktreeId} killed runtime=${teardownResult.runtimeStopped} provider=${teardownResult.providerStopped} registry=${teardownResult.registryStopped}`
`[worktree-teardown] ${worktreeId} killed runtime=${teardownResult.runtimeStopped} provider=${teardownResult.providerStopped} registry=${teardownResult.registryStopped} structured=${structuredStopped}`
)
}
}
@@ -8,18 +8,31 @@ 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 {
classifyWorktreeForceDeleteReason,
isProvenLiveStructuredSessionRemovalError,
isUnstoppedPtyRemovalError
} = await import('../../shared/worktree/removal')
const { listLiveStructuredSessionsForWorktree } =
await import('./structured-session-worktree-teardown')
const WORKTREE = 'repo_1::/tmp/wt-a'
const OTHER_WORKTREE = 'repo_1::/tmp/wt-b'
function record(sessionId: string, workspaceId: string): AgentSessionRecord {
function record(
sessionId: string,
workspaceId: string,
options: { provider?: 'claude' | 'codex'; executionHostId?: string } = {}
): AgentSessionRecord {
return {
sessionId,
provider: 'claude',
location: { executionHostId: 'local', wslDistro: null, workspaceId, workspaceKind: 'folder' },
provider: options.provider ?? 'claude',
location: {
executionHostId: options.executionHostId ?? 'local',
wslDistro: null,
workspaceId,
workspaceKind: 'folder'
},
lease: {
sessionId,
runtimeKind: 'native',
@@ -33,8 +46,12 @@ function record(sessionId: string, workspaceId: string): AgentSessionRecord {
function installHost(options: {
records: AgentSessionRecord[]
/** Sessions the host still holds; a close removes one unless it is listed as stuck. */
/** Sessions the host keeps holding through a close, so the post-close observation is `live`. */
stuck?: Set<string>
/** Sessions the host drops without death evidence, so the observation is `unverifiable`. */
unverifiable?: Set<string>
/** Blocks every close, to exercise the shared sweep budget without fake timers. */
closeGate?: Promise<void>
}): { closed: string[] } {
const held = new Set(options.records.map((entry) => entry.sessionId))
const closed: string[] = []
@@ -44,13 +61,18 @@ function installHost(options: {
setSessionTabVisibility: async () => {},
close: async (sessionId: string) => {
closed.push(sessionId)
if (!options.stuck?.has(sessionId)) {
held.delete(sessionId)
const record = options.records.find((entry) => entry.sessionId === sessionId)
if (record) {
record.lease.claimStatus = 'released'
record.lease.deathEvidence = { kind: 'exit-observed', detail: 'closed', observedAt: 1 }
}
await options.closeGate
if (options.stuck?.has(sessionId)) {
return
}
held.delete(sessionId)
if (options.unverifiable?.has(sessionId)) {
return
}
const record = options.records.find((entry) => entry.sessionId === sessionId)
if (record) {
record.lease.claimStatus = 'released'
record.lease.deathEvidence = { kind: 'exit-observed', detail: 'closed', observedAt: 1 }
}
}
}
@@ -67,7 +89,7 @@ const localProvider = {
shutdown: async () => {}
} as never
function destructiveDeps(extra: { allowUnverifiedStop?: boolean } = {}) {
function destructiveDeps(extra: { allowUnverifiedStop?: boolean; timeoutMs?: number } = {}) {
return {
localProvider,
requirePhysicalStop: true,
@@ -84,7 +106,7 @@ describe('worktree teardown and structured agent sessions', () => {
it('finds sessions by workspace, and ignores a sibling worktree', () => {
installHost({ records: [record('s1', WORKTREE), record('s2', OTHER_WORKTREE)] })
expect(listLiveStructuredSessionsForWorktree(WORKTREE)).toEqual([
expect(listLiveStructuredSessionsForWorktree(WORKTREE, {})).toEqual([
{ sessionId: 's1', agent: 'claude' }
])
})
@@ -105,7 +127,7 @@ describe('worktree teardown and structured agent sessions', () => {
it('refuses only when the close does not settle', async () => {
installHost({ records: [record('s1', WORKTREE)], stuck: new Set(['s1']) })
await expect(killAllProcessesForWorktree(WORKTREE, destructiveDeps())).rejects.toThrow(
/1 running agent session/
/still live: 1 agent session \(claude\)/
)
})
@@ -137,7 +159,7 @@ describe('worktree teardown and structured agent sessions', () => {
(thrown: Error) => thrown.message
)
expect(error).not.toContain('s1')
expect(error).toContain('1 running agent session')
expect(error).toContain('1 agent session (claude)')
})
it('closes best-effort for a folder-workspace removal, which requires no stop proof', async () => {
@@ -191,6 +213,116 @@ describe('worktree teardown and structured agent sessions', () => {
).resolves.toMatchObject({ runtimeStopped: 0 })
})
it('leaves a same-id workspace on another execution host alone', async () => {
// A workspace id is `repoId::path` with no host component, so the local, SSH and paired-runtime
// copies of one id are DIFFERENT workspaces. Unfenced, deleting the local one closed a chat
// running on somebody else's machine — a destructive cross-host act, not a spurious refusal.
const host = installHost({
records: [record('s1', WORKTREE, { executionHostId: 'ssh:host-a' })]
})
await expect(killAllProcessesForWorktree(WORKTREE, destructiveDeps())).resolves.toMatchObject({
runtimeStopped: 0
})
expect(host.closed).toEqual([])
})
it('closes only the session on the host the removal resolved to', async () => {
const host = installHost({
records: [record('s1', WORKTREE, { executionHostId: 'ssh:host-a' }), record('s2', WORKTREE)]
})
await expect(
killAllProcessesForWorktree(WORKTREE, {
...destructiveDeps(),
resolvedConnectionId: 'host-a'
})
).resolves.toMatchObject({ structuredStopped: 1 })
expect(host.closed).toEqual(['s1'])
})
it('names only the sessions that stayed, and every provider still there', async () => {
installHost({
records: [
record('s1', WORKTREE),
record('s2', WORKTREE, { provider: 'codex' }),
record('s3', WORKTREE)
],
stuck: new Set(['s2', 's3'])
})
const error = await killAllProcessesForWorktree(WORKTREE, destructiveDeps()).catch(
(thrown: Error) => thrown.message
)
expect(error).toContain('still live: 2 agent sessions (claude, codex)')
})
it('separates a close it could not confirm from one it watched stay attached', async () => {
// `src/shared/worktree/removal.ts` keeps these two apart on purpose: a user waiving "we could
// not confirm" is making a different decision than one discarding a conversation Orca just saw
// running. The toast branches on this marker, so flattening them makes one of the two a lie.
installHost({ records: [record('s1', WORKTREE)], unverifiable: new Set(['s1']) })
const unconfirmed = await killAllProcessesForWorktree(WORKTREE, destructiveDeps()).catch(
(thrown: Error) => thrown.message
)
expect(unconfirmed).toContain('could not confirm these closed: 1 agent session (claude)')
expect(isProvenLiveStructuredSessionRemovalError(unconfirmed as string)).toBe(false)
installHost({ records: [record('s1', WORKTREE)], stuck: new Set(['s1']) })
const live = await killAllProcessesForWorktree(WORKTREE, destructiveDeps()).catch(
(thrown: Error) => thrown.message
)
expect(isProvenLiveStructuredSessionRemovalError(live as string)).toBe(true)
})
it('refuses in agent-session wording when the close outlives the sweep budget', async () => {
// A structured close that runs out of time used to reject with the PTY timeout sentinel, which
// the classifier reads FIRST — so the toast blamed terminals, and the Force Delete meant to
// clear the wedge hit the same rejection again (#11960).
installHost({ records: [record('s1', WORKTREE)], closeGate: new Promise<void>(() => {}) })
const error = await killAllProcessesForWorktree(
WORKTREE,
destructiveDeps({ timeoutMs: 5 })
).catch((thrown: Error) => thrown.message)
expect(error).toContain('could not confirm these closed: 1 agent session (claude)')
expect(isUnstoppedPtyRemovalError(error as string)).toBe(false)
expect(classifyWorktreeForceDeleteReason(error as string, true)).toBe('running-agent-session')
})
it('never wedges Force Delete on a close that will not settle', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
installHost({ records: [record('s1', WORKTREE)], closeGate: new Promise<void>(() => {}) })
await expect(
killAllProcessesForWorktree(
WORKTREE,
destructiveDeps({ allowUnverifiedStop: true, timeoutMs: 5 })
)
).resolves.toMatchObject({ runtimeStopped: 0 })
expect(warn).toHaveBeenCalledWith(expect.stringContaining('still attached'))
warn.mockRestore()
})
it('starts the terminal sweeps while the structured close is still in flight', async () => {
// The close is serial and each one waits on a provider round trip. Awaiting it before the
// sweeps exist spends the shared budget head-first, and the sweeps then report a timeout for
// a stop they never attempted.
let releaseClose: () => void = () => {}
const closeGate = new Promise<void>((resolve) => {
releaseClose = resolve
})
installHost({ records: [record('s1', WORKTREE)], closeGate })
let terminalSweepStarted = false
const runtime = {
stopTerminalsForWorktree: async () => {
terminalSweepStarted = true
return { stopped: 0 }
}
} as never
const removal = killAllProcessesForWorktree(WORKTREE, { ...destructiveDeps(), runtime })
await vi.waitFor(() => {
expect(terminalSweepStarted).toBe(true)
})
releaseClose()
await expect(removal).resolves.toMatchObject({ structuredStopped: 1 })
})
it('does not block removal when no structured host is installed', async () => {
// Not being able to look is not evidence a child is there, and reading the persisted store
// directly would force-install the host as a side effect of a teardown.
@@ -8,10 +8,10 @@
* kept running with its `cwd` gone, the durable record and chat tab survived to republish at the
* next launch pointing at a deleted worktree, and `worker-show` still reported the worker live.
*
* Membership is `location.workspaceId`, which every structured session carries — so this covers a
* plain chat session in the worktree as well as a dispatched worker. Liveness is
* `observeStructuredWorker`, the same `live` / `unverifiable` / `exited` vocabulary the rest of the
* structured surface uses.
* Membership is `location.workspaceId` PLUS the host fence below, and every structured session
* carries both — so this covers a plain chat session in the worktree as well as a dispatched
* worker. Liveness is `observeStructuredWorker`, the same `live` / `unverifiable` / `exited`
* vocabulary the rest of the structured surface uses.
*
* `live` here is lease state — a provider child is attached — not work in flight, so it says
* nothing about whether the user would lose anything. It selects what to CLOSE, never what to
@@ -19,6 +19,13 @@
* refuses only on a stop it could not verify.
*/
import {
LOCAL_EXECUTION_HOST_ID,
toRuntimeExecutionHostId,
toSshExecutionHostId,
type ExecutionHostId
} from '../../shared/execution-host'
import { STILL_LIVE_DETAIL_PREFIX } from '../../shared/worktree/removal'
import { getStructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-registry'
import { observeStructuredWorker } from './structured-worker-authority'
import { closeStructuredAgentSessionChild } from './structured-agent-session-close'
@@ -29,13 +36,45 @@ export type LiveStructuredSessionInWorkspace = {
agent: 'claude' | 'codex'
}
export type UnclosedStructuredSession = LiveStructuredSessionInWorkspace & {
/** Read AFTER the close: `live` is a child watched stay attached, not merely one left unproven. */
status: 'live' | 'unverifiable'
}
export type StructuredWorktreeSweepRuntime = Pick<
OrcaRuntimeService,
'forgetStructuredSessionMail' | 'retireStructuredAgentSessionTabFromSnapshot'
>
/** The two fields every teardown caller already resolves to fence its PTY sweeps to one host. */
export type StructuredSessionHostFence = {
resolvedConnectionId?: string
resolvedRuntimeEnvironmentId?: string
}
/**
* Structured sessions with a proven-live child in this worktree.
* The one execution host this teardown may touch.
*
* A workspace id is `repoId::path` with no host component, so the local machine, an SSH host and a
* paired runtime can all publish the SAME id and each names a DIFFERENT workspace (STA-4343). The
* PTY sweeps fence on exactly these two fields; a structured session records its host directly, so
* the comparison is on `location.executionHostId` instead of on a pty-id shape — but the
* precedence is the same. Neither field set means the removal targets this machine, which is also
* the safe default: a caller that resolved no host closes nothing on anyone else's.
*/
export function structuredSessionTeardownHostId(
fence: StructuredSessionHostFence
): ExecutionHostId {
if (fence.resolvedRuntimeEnvironmentId !== undefined) {
return toRuntimeExecutionHostId(fence.resolvedRuntimeEnvironmentId)
}
return fence.resolvedConnectionId === undefined
? LOCAL_EXECUTION_HOST_ID
: toSshExecutionHostId(fence.resolvedConnectionId)
}
/**
* Structured sessions with a proven-live child in this worktree, on the fenced host only.
*
* An uninstalled host answers empty rather than throwing: no host in this generation means no
* provider child was started by this process, and the three PTY sweeps fall through the same way
@@ -43,7 +82,8 @@ export type StructuredWorktreeSweepRuntime = Pick<
* directly — that would force-install the host, which is itself a side effect on a teardown path.
*/
export function listLiveStructuredSessionsForWorktree(
worktreeId: string
worktreeId: string,
fence: StructuredSessionHostFence
): LiveStructuredSessionInWorkspace[] {
const host = getStructuredAgentSessionHost()
if (!host) {
@@ -55,48 +95,63 @@ export function listLiveStructuredSessionsForWorktree(
} catch {
return []
}
const hostId = structuredSessionTeardownHostId(fence)
return records
.filter(
(record) =>
record.location.workspaceId === worktreeId &&
record.location.executionHostId === hostId &&
observeStructuredWorker({ sessionId: record.sessionId }).status === 'live'
)
.map((record) => ({ sessionId: record.sessionId, agent: record.provider }))
}
/**
* Counts and providers, never session ids.
* Counts, providers and the post-close verdict — 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.
*
* The verdict is here for the reason `describeUnstoppedPtys` carries one: "we watched it stay
* attached" and "we could not confirm it went" are different decisions to waive, and the delete
* toast branches on this marker. Any proven-live session makes the whole refusal a live one, as it
* does for PTYs — that is the stronger warning, and the one whose work is about to be discarded.
*/
export function describeLiveStructuredSessions(
sessions: readonly LiveStructuredSessionInWorkspace[]
export function describeUnclosedStructuredSessions(
sessions: readonly UnclosedStructuredSession[]
): string {
const noun = sessions.length === 1 ? 'agent session' : 'agent sessions'
const providers = [...new Set(sessions.map((session) => session.agent))].sort().join(', ')
return `${sessions.length} running ${noun} (${providers})`
const stillLive = sessions.filter((session) => session.status === 'live')
const named = stillLive.length > 0 ? stillLive : sessions
const noun = named.length === 1 ? 'agent session' : 'agent sessions'
const providers = [...new Set(named.map((session) => session.agent))].sort().join(', ')
const summary = `${named.length} ${noun} (${providers})`
return stillLive.length > 0
? `${STILL_LIVE_DETAIL_PREFIX} ${summary}`
: `could not confirm these closed: ${summary}`
}
/**
* Closes every live structured session in the worktree, and reports what stayed.
* Closes the given structured sessions, and reports what stayed.
*
* Runs on the ordinary removal too, not just force: a child left running against a deleted `cwd` is
* the outcome this whole sweep exists to prevent, and closing is how you prevent it. What stayed is
* the only thing worth refusing over.
*
* Takes the list rather than re-deriving it, so the sessions reported as unclosed are exactly the
* ones a close was attempted on — re-enumerating would run every liveness observation twice and
* let the refusal name a session this call never touched.
*/
export async function closeStructuredSessionsForWorktree(
worktreeId: string,
sessions: readonly LiveStructuredSessionInWorkspace[],
runtime?: StructuredWorktreeSweepRuntime
): Promise<{ closed: number; unstopped: LiveStructuredSessionInWorkspace[] }> {
): Promise<{ closed: number; unstopped: UnclosedStructuredSession[] }> {
// 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[] = []
const unstopped: UnclosedStructuredSession[] = []
let closed = 0
for (const session of sessions) {
const outcome = await closeStructuredAgentSessionChild(
@@ -105,9 +160,12 @@ export async function closeStructuredSessionsForWorktree(
)
if (outcome.stopped) {
closed += 1
} else {
unstopped.push(session)
continue
}
// Re-observed rather than reusing the close's own reason string: what the user is asked to
// waive is the state AFTER the attempt, and a close that threw never reached an observation.
const status = observeStructuredWorker({ sessionId: session.sessionId }).status
unstopped.push({ ...session, status: status === 'live' ? 'live' : 'unverifiable' })
}
return { closed, unstopped }
}
@@ -2,7 +2,7 @@ import type { IPtyProvider } from '../providers/types'
import type { OrcaRuntimeService } from './orca-runtime'
import {
UNSTOPPED_PTY_DETAIL_SEPARATOR,
UNSTOPPED_PTY_LIVE_DETAIL_PREFIX,
STILL_LIVE_DETAIL_PREFIX,
UNSTOPPED_PTY_REMOVAL_PREFIX
} from '../../shared/worktree/removal'
import {
@@ -105,7 +105,7 @@ export function describeUnstoppedPtys(
): string {
const detail =
verdict.status === 'live'
? `${UNSTOPPED_PTY_LIVE_DETAIL_PREFIX} ${verdict.ptyIds.join(', ')}`
? `${STILL_LIVE_DETAIL_PREFIX} ${verdict.ptyIds.join(', ')}`
: `could not verify these exited: ${failedPtyIds.join(', ')} (${verdict.reason})`
return `${UNSTOPPED_PTY_REMOVAL_PREFIX} ${worktreeId}${UNSTOPPED_PTY_DETAIL_SEPARATOR}${detail}`
}
+46 -25
View File
@@ -15,10 +15,14 @@ import {
} from './worktree-pty-surface-sweeps'
import {
closeStructuredSessionsForWorktree,
describeLiveStructuredSessions,
describeUnclosedStructuredSessions,
listLiveStructuredSessionsForWorktree
} from './structured-session-worktree-teardown'
import { createWorktreeSweepTracker, settleSweepsForForcedRemoval } from './forced-sweep-settlement'
import {
createWorktreeSweepTracker,
settleSweepsForForcedRemoval,
type WorktreeSweepTracker
} from './forced-sweep-settlement'
import {
describeError,
describeFailedPtySweep,
@@ -57,7 +61,7 @@ export type WorktreeTeardownResult = {
runtimeStopped: number
providerStopped: number
registryStopped: number
/** Structured agent sessions closed by the force path; absent when none were found. */
/** Structured agent sessions this teardown closed; absent when it closed none. */
structuredStopped?: number
}
@@ -99,12 +103,18 @@ export async function killAllProcessesForWorktree(
const deadlineError = new Error(
`${WORKTREE_TEARDOWN_TIMEOUT_PREFIX} ${worktreeId}. ${WORKTREE_TEARDOWN_FORCE_HINT}`
)
// FIRST, and before a single PTY sweep starts: a structured agent session is registered on none
// of the three surfaces below, so all three answered zero and removal deleted the checkout out
// from under a running provider child. It returns immediately when there are none, and its own
// close is bounded by the same deadline the PTY sweeps share.
const structuredStopped = await sweepStructuredSessions(worktreeId, deps, deadline, deadlineError)
const sweeps = createWorktreeSweepTracker()
// ISSUED first, before a single PTY is touched: a structured agent session is registered on none
// of the three surfaces below, so all three answered zero and removal deleted the checkout out
// from under a running provider child. Asking the agent plane ahead of the terminal plane also
// keeps an intentional stop from reading as a failed process exit.
//
// Not AWAITED first, though. Its close is serial and each one waits on a provider round trip, so
// awaiting here would spend the shared budget before a single PTY was asked — and the sweeps
// would then report a timeout for a stop they never attempted. It is joined below, ahead of the
// PTY verdict, so a structured refusal still outranks one.
const structuredSweep = sweepStructuredSessions(worktreeId, deps, deadline, sweeps)
void structuredSweep.catch(() => undefined)
const stopAttempts = new Map<string, Promise<boolean>>()
const stopPty = (
ptyId: string,
@@ -196,6 +206,7 @@ export async function killAllProcessesForWorktree(
for (const sweep of [runtimeSweep, providerSweep, registrySweep]) {
void sweep.catch(() => undefined)
}
const structuredStopped = await structuredSweep
let runtimeResult: { stopped: number }
let providerStopped: number
let registryStopped: number
@@ -277,7 +288,7 @@ export async function killAllProcessesForWorktree(
}
/**
* The fourth sweep: structured agent sessions bound to this worktree.
* The fourth sweep: structured agent sessions bound to this worktree, on this host.
*
* Stops first and refuses only on unproven stops, which is the bargain the unstopped-PTY gate
* actually strikes: that gate kills every PTY — a terminal running an agent included — and refuses
@@ -288,50 +299,60 @@ export async function killAllProcessesForWorktree(
* Two callers participate. A proof-requiring removal (`requirePhysicalStop`) refuses when a close
* does not settle, so nothing deletes a checkout out from under a child that is still there. A
* folder-workspace removal (`closeStructuredSessions`) never refuses: 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.
* checkout vanishes under the child, and every one of those call sites discards a rejection, so a
* refusal there would be words nobody reads. Reconciliation sweeps set neither — they repair state,
* delete nothing, and must never close a session.
*/
async function sweepStructuredSessions(
worktreeId: string,
deps: WorktreeTeardownDeps,
deadline: number,
deadlineError: Error
sweeps: WorktreeSweepTracker
): Promise<number> {
if (!deps.requirePhysicalStop && !deps.closeStructuredSessions) {
return 0
}
const live = listLiveStructuredSessionsForWorktree(worktreeId)
// `deps` carries the same two host fields the PTY sweeps fence on, and a `repoId::path` id names
// a different workspace on every host — so an unfenced list would close a live chat belonging to
// an SSH or paired-runtime copy of the id being removed here.
const live = listLiveStructuredSessionsForWorktree(worktreeId, deps)
if (live.length === 0) {
return 0
}
// 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` forever with no
// timeout error at all. On expiry the sweep reports the timeout exactly as the PTY sweeps do
// rather than proceeding as if the sessions had closed.
// Raced against the same sweep budget every PTY surface is bounded by, because `host.close`
// awaits a provider round trip whose own eviction steps are each bounded well past this budget.
//
// Deliberately NOT fail-closed, unlike the PTY sweeps: their timeout sentinel carries the PTY
// timeout prefix, which the desktop classifier reads as a TERMINAL failure — so a wedged session
// close would refuse in terminal wording, and refuse identically again under the Force Delete
// that is meant to clear it (#11960). A close that ran out of time is a session this removal
// could not confirm closed, which is exactly what the branch below already words. Tracked so a
// forced removal still waits out the abandoned-sweep grace before it deletes files.
const { closed, unstopped } = await settleBeforeDeadline(
() => closeStructuredSessionsForWorktree(worktreeId, deps.runtime),
{ closed: 0, unstopped: live },
deadline,
deadlineError
sweeps.track(() => closeStructuredSessionsForWorktree(live, deps.runtime)),
{
closed: 0,
unstopped: live.map((session) => ({ ...session, status: 'unverifiable' as const }))
},
deadline
)
if (unstopped.length === 0) {
return closed
}
// 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.
// has forgotten — and every one of those callers discards a rejection anyway.
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(
`${RUNNING_AGENT_SESSION_REMOVAL_PREFIX} ${worktreeId}${UNSTOPPED_PTY_DETAIL_SEPARATOR}${describeLiveStructuredSessions(unstopped)}. ${WORKTREE_TEARDOWN_FORCE_HINT}`
`${RUNNING_AGENT_SESSION_REMOVAL_PREFIX} ${worktreeId}${UNSTOPPED_PTY_DETAIL_SEPARATOR}${describeUnclosedStructuredSessions(unstopped)}. ${WORKTREE_TEARDOWN_FORCE_HINT}`
)
}
// 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 of ${worktreeId} with ${describeLiveStructuredSessions(unstopped)} still attached`
`[worktree-teardown] forcing removal of ${worktreeId} with ${describeUnclosedStructuredSessions(unstopped)} still attached`
)
return closed
}
@@ -50,6 +50,39 @@ describe('getDeleteWorktreeToastCopy', () => {
})
})
// Why: the structured sweep now CLOSES an attached session on the ordinary delete, so reaching
// this toast means the close was attempted and did not settle — not that Orca declined to try.
it('offers force delete when an agent session could not be confirmed closed', () => {
expect(
toastCopyForRemovalError(
'feature/foo',
'Refusing to remove worktree with running agent sessions: repo-1::/w — could not confirm these closed: 1 agent session (claude). Retry with force delete (--force) to remove it anyway.'
)
).toEqual({
title: 'Failed to delete workspace feature/foo',
description:
'Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway.',
isDestructive: false
})
})
// Why: the same split the PTY pair above draws. Force Delete proceeds either way, and telling a
// user "could not confirm" about a conversation Orca watched stay attached asks them to waive a
// doubt that does not exist — the work in that conversation goes with the delete.
it('names the running agent sessions when the close left them attached', () => {
expect(
toastCopyForRemovalError(
'feature/foo',
'Refusing to remove worktree with running agent sessions: repo-1::/w — still live: 2 agent sessions (claude, codex). Retry with force delete (--force) to remove it anyway.'
)
).toEqual({
title: 'Failed to delete workspace feature/foo',
description:
'This workspace still has running agent sessions that Orca could not close, so it stopped before deleting any files. Force Delete will discard any work they hold.',
isDestructive: false
})
})
// Why: a sweep that never answered wedges removal the same way, and the waiver clears
// both — so it must reach the same force affordance instead of a dead end.
it('offers force delete when the teardown sweep itself timed out', () => {
@@ -2,6 +2,7 @@ import { translate } from '@/i18n/i18n'
import {
isLockedWorktreeRemovalError,
isProvenLivePtyRemovalError,
isProvenLiveStructuredSessionRemovalError,
type WorktreeForceDeleteReason
} from '../../../../shared/worktree/removal'
export type DeleteWorktreeToastCopy = {
@@ -81,13 +82,19 @@ export function getDeleteWorktreeToastCopy(
'Failed to delete workspace {{value0}}',
{ value0: worktreeName }
),
// Why this is the "could not confirm" wording: an ordinary delete already closed these
// sessions, so reaching here means the close did not settle — the same doubt the
// unverified-PTY case asks the user to waive, not a session Orca declined to close.
description: translate(
'auto.components.sidebar.delete.worktree.toast.runningAgentSession',
'Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway.'
),
// Why two branches, like the PTY pair above: an ordinary delete already tried to close
// these sessions, and only the observation AFTER that attempt separates one Orca watched
// stay attached from one it simply could not reach. Telling the first user "could not
// confirm" asks them to waive a doubt that does not exist, and a conversation dies with it.
description: isProvenLiveStructuredSessionRemovalError(error)
? translate(
'auto.components.sidebar.delete.worktree.toast.runningAgentSessionLive',
'This workspace still has running agent sessions that Orca could not close, so it stopped before deleting any files. Force Delete will discard any work they hold.'
)
: translate(
'auto.components.sidebar.delete.worktree.toast.runningAgentSession',
'Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway.'
),
isDestructive: false
}
}
+2 -1
View File
@@ -5786,7 +5786,8 @@
"lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion.",
"unstoppedPty": "Orca could not confirm every terminal in this workspace has exited, so it stopped before deleting any files. Use Force Delete to remove it anyway.",
"unstoppedPtyLive": "This workspace still has running terminals, so Orca stopped before deleting any files. Force Delete will kill them and discard any uncommitted work they hold.",
"runningAgentSession": "Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway."
"runningAgentSession": "Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway.",
"runningAgentSessionLive": "This workspace still has running agent sessions that Orca could not close, so it stopped before deleting any files. Force Delete will discard any work they hold."
}
}
},
+14 -5
View File
@@ -22,11 +22,12 @@ export type WorktreeForceDeleteReason =
// rather than scanning the whole message and letting a path spell out a verdict.
export const UNSTOPPED_PTY_DETAIL_SEPARATOR = ' — '
// Why: verification distinguishes a PTY it watched stay alive from one it could not reach,
// Why: verification distinguishes a process it watched stay alive from one it could not reach,
// and the delete toast must not flatten the two — a user waiving "we could not confirm" is
// making a different decision than one killing a terminal Orca just saw running. The marker
// and its matcher stay together for the same reason the force hint does.
export const UNSTOPPED_PTY_LIVE_DETAIL_PREFIX = 'still live:'
// making a different decision than one killing something Orca just saw running. The marker
// and its matcher stay together for the same reason the force hint does. Shared by the PTY
// sweep and the structured-session sweep, which both re-observe after their stop.
export const STILL_LIVE_DETAIL_PREFIX = 'still live:'
// Why (#11960): a sweep that never answers wedges removal exactly like a stop that could not
// be proven, and the waiver clears both — but this error carries different words, so without
@@ -54,7 +55,15 @@ export function isUnstoppedPtyRemovalError(error: string): boolean {
export function isProvenLivePtyRemovalError(error: string): boolean {
return (
isUnstoppedPtyRemovalError(error) &&
error.includes(`${UNSTOPPED_PTY_DETAIL_SEPARATOR}${UNSTOPPED_PTY_LIVE_DETAIL_PREFIX}`)
error.includes(`${UNSTOPPED_PTY_DETAIL_SEPARATOR}${STILL_LIVE_DETAIL_PREFIX}`)
)
}
/** True only when the observation AFTER the close found the session still attached. */
export function isProvenLiveStructuredSessionRemovalError(error: string): boolean {
return (
isRunningAgentSessionRemovalError(error) &&
error.includes(`${UNSTOPPED_PTY_DETAIL_SEPARATOR}${STILL_LIVE_DETAIL_PREFIX}`)
)
}