fix(runtime): a project removal stops its workspaces' terminals together under one deadline; every teardown is classified by kind (#30752)

* fix(runtime): a project removal stops its workspaces' terminals together under one deadline; every teardown is classified by kind

* fix(runtime): a workspace the inventory finds mid-sweep is still stopped; the walk test sees const and top-level teardowns
This commit is contained in:
Neil
2026-10-19 09:12:26 -07:00
committed by GitHub
parent 59a9da22a0
commit 5cc0f8575e
2 changed files with 105 additions and 4 deletions
@@ -1,8 +1,14 @@
import { readdirSync, readFileSync } from 'node:fs'
import { join } from 'node:path'
import { describe, expect, it, vi } from 'vitest'
import type { RuntimeClientEvent } from '../../shared/runtime-client-events'
import type { RuntimeNotifier } from './runtime-notifier-contract'
import type { AgentBrowserBridge } from '../browser/agent-browser-bridge'
import { OWNED_SESSION_KINDS, OWNED_SESSION_TEARDOWN } from './owned-session-teardown-policy'
import {
OWNED_SESSION_KINDS,
OWNED_SESSION_TEARDOWN,
type OwnedSessionKind
} from './owned-session-teardown-policy'
import {
getRuntimeBrowserPageRegistry,
type RuntimeBrowserClientPage
@@ -146,4 +152,77 @@ describe('owned-session teardown policy', () => {
expect(release.mock.calls.map(([worktreeId]) => worktreeId)).toEqual(released)
}
)
it("stops every project workspace's terminals under one deadline, not one after another", async () => {
const { runtime, runtimeStore } = makeRuntime()
const [repo] = runtimeStore.getRepos()
const slow = `${repo.id}::/slow`
const fast = `${repo.id}::/fast`
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the sweep reads only id and repoId.
const worktrees = new Map([slow, fast].map((id) => [id, { id, repoId: repo.id }])) as never
vi.spyOn(runtime, 'getResolvedWorktreeMap').mockResolvedValue(worktrees)
const stop = vi.spyOn(runtime, 'stopTerminalsForWorktree').mockImplementation(
async (selector) =>
await new Promise<{ stopped: number }>((resolve) => {
// The slow workspace's provider never answers; the fast one takes a moment.
if (selector === `id:${fast}`) {
setTimeout(() => resolve({ stopped: 1 }), 20)
}
})
)
const result = await runtime.stopTerminalsForProject(repo, { deadline: Date.now() + 500 })
expect(stop.mock.calls.map(([selector]) => selector).sort()).toEqual([
`id:${fast}`,
`id:${slow}`
])
expect(result.stopped).toBe(1)
})
})
// Every exported or method-level stop/close/teardown of a workspace's sessions in src/main/runtime,
// and the owned-session kinds it ends. A new one fails here until someone classifies it.
const TEARDOWN_FUNCTIONS: Readonly<Record<string, readonly OwnedSessionKind[]>> = {
closeClientHostedBrowserPagesForWorktree: ['host-browser'],
closeStructuredSessionsForProject: ['structured-chat'],
closeStructuredSessionsForWorktree: ['structured-chat'],
retireStructuredSessionTabsForWorktree: ['structured-chat'],
closeTerminalsForWorktree: ['pty'],
killAllProcessesForWorktree: ['pty', 'structured-chat'],
sleepTerminalsForWorktree: ['pty'],
stopExactTerminalsForWorktree: ['pty'],
stopTerminalsForWorktree: ['pty'],
stopTerminalsForProject: ['pty', 'structured-chat', 'host-browser', 'launch']
}
// Functions, `const` arrows and class members; a call or import matching it is harmless.
const TEARDOWN_NAME =
/(?:^(?:export )?(?:async )?function |^(?:export )?(?:const|let) |^\s+(?:(?:async|private|protected|public|override|static|abstract|readonly) )*)((?:stop|close|teardown|kill|sleep|retire|forget)\w*For(?:Worktree|Project)s?)\b/gm
function declaredTeardownFunctions(dir: string): string[] {
return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
const path = join(dir, entry.name)
if (entry.isDirectory()) {
return declaredTeardownFunctions(path)
}
if (!entry.name.endsWith('.ts') || /\.(test|spec)\.ts$|\.test-fixture\.ts$/.test(entry.name)) {
return []
}
return [...readFileSync(path, 'utf8').matchAll(TEARDOWN_NAME)].map((match) => match[1])
})
}
describe('owned-session teardown walk', () => {
const declared = new Set(declaredTeardownFunctions(__dirname))
it('classifies every stop, close and teardown of a workspace in src/main/runtime', () => {
expect([...declared].filter((name) => !(name in TEARDOWN_FUNCTIONS)).sort()).toEqual([])
expect(Object.keys(TEARDOWN_FUNCTIONS).filter((name) => !declared.has(name))).toEqual([])
})
it('reaches every owned-session kind', () => {
const reached = new Set(Object.values(TEARDOWN_FUNCTIONS).flat())
expect(OWNED_SESSION_KINDS.filter((kind) => !reached.has(kind))).toEqual([])
})
})
@@ -170,11 +170,12 @@ export abstract class OrcaRuntimeWithStopTerminalsForProject extends OrcaRuntime
if (this.projectSiblingSharesFence(repo)) {
return { stopped: 0, worktreeIds: [] }
}
let stopped = 0
for (const id of listProjectTerminalWorktreeIds(repo.id, this.leaves, this.ptysById)) {
projectWorktreeIds.add(id)
}
for (const id of projectWorktreeIds) {
// Why all at once: one deadline covers the project, so a slow worktree must not spend the
// budget its siblings' stops need; each still holds only its own worktree's lock.
const stopWorktree = async (id: string): Promise<number> => {
const opts = { resolvedWorktreeId: id, ...fence, ...options, requestStopsPastDeadline: true }
// Why exclusive: it waits out a spawn that holds the shared lease, so its PTY is registered
// and stopped. Bounded by the deadline so repo.rm still answers; past it PTYs are only asked.
@@ -190,11 +191,32 @@ export abstract class OrcaRuntimeWithStopTerminalsForProject extends OrcaRuntime
this.stopTerminalsForWorktree(`id:${id}`, opts),
options.deadline
)
stopped += result?.stopped ?? 0
return result?.stopped ?? 0
} finally {
release?.()
}
}
// Why rounds: the inventory refresh may still add workspaces after its own budget gave up.
const started = new Set<string>()
const stops: PromiseSettledResult<number>[] = []
for (;;) {
const added = [...projectWorktreeIds].filter((id) => !started.has(id))
if (added.length === 0) {
break
}
for (const id of added) {
started.add(id)
}
stops.push(...(await Promise.allSettled(added.map(stopWorktree))))
}
const failure = stops.find((stop) => stop.status === 'rejected')
if (failure) {
throw failure.reason
}
const stopped = stops.reduce(
(total, stop) => total + (stop.status === 'fulfilled' ? stop.value : 0),
0
)
return { stopped, worktreeIds: [...projectWorktreeIds] }
}