Merge remote-tracking branch 'origin/main' into nwparker/131-crashes

# Conflicts:
#	src/relay/git-handler-worktree-remove.ts
This commit is contained in:
Neil
2026-07-10 18:05:04 -07:00
64 changed files with 3587 additions and 334 deletions
@@ -38,10 +38,19 @@ describe('canonicalizeWslLinuxPath', () => {
expect(execFileMock).toHaveBeenCalledTimes(1)
const [file, args] = execFileMock.mock.calls[0]
expect(file).toBe('wsl.exe')
expect(args).toEqual(['-d', 'Ubuntu', '--', 'readlink', '-f', '--', '/home/alias'])
expect(args).toEqual([
'-d',
'Ubuntu',
'--',
'sh',
'-c',
`if [ ! -d "$1" ]; then printf '%s\\n' '__ORCA_WSL_PATH_MISSING__'; exit 0; fi; readlink -f -- "$1"`,
'sh',
'/home/alias'
])
})
it('caches the resolved canonical path and stops spawning wsl.exe', () => {
it('returns the cached path while revalidating it asynchronously', () => {
setPlatform('win32')
expect(_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')).toBeNull()
@@ -49,7 +58,42 @@ describe('canonicalizeWslLinuxPath', () => {
callback(null, '/home/alice\n')
expect(_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')).toBe('/home/alice')
expect(execFileMock).toHaveBeenCalledTimes(1)
expect(execFileMock).toHaveBeenCalledTimes(2)
})
it('resolves a custom automount root and notifies the first launch', () => {
setPlatform('win32')
const settled = vi.fn()
const windowsPath = 'D:\\orca\\codex-runtime-home\\home'
expect(
_internals.canonicalizeWslLinuxPath(
'Ubuntu',
'/mnt/d/orca/codex-runtime-home/home',
windowsPath,
settled
)
).toBeNull()
const [file, args, options, callback] = execFileMock.mock.calls[0]
expect(file).toBe('wsl.exe')
expect(args).toEqual([
'-d',
'Ubuntu',
'--',
'sh',
'-c',
`resolved=$(wslpath -a -u "$1") || exit; if [ ! -d "$resolved" ]; then printf '%s\\n' '__ORCA_WSL_PATH_MISSING__'; exit 0; fi; readlink -f -- "$resolved"`,
'sh',
windowsPath
])
expect(options).toMatchObject({ timeout: 5000, windowsHide: true })
callback(null, '/windows/d/orca/codex-runtime-home/home\n')
expect(settled).toHaveBeenCalledWith({
status: 'resolved',
canonicalPath: '/windows/d/orca/codex-runtime-home/home'
})
})
it('does not spawn a second subprocess while one is in flight', () => {
@@ -70,6 +114,81 @@ describe('canonicalizeWslLinuxPath', () => {
expect(execFileMock).toHaveBeenCalledTimes(2)
})
it('keeps the last known-good cache when revalidation later fails', () => {
setPlatform('win32')
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')
const firstCallback = execFileMock.mock.calls[0][3] as (
error: Error | null,
stdout: string
) => void
firstCallback(null, '/home/alice\n')
const settled = vi.fn()
expect(
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias', '/home/alias', settled)
).toBe('/home/alice')
const secondCallback = execFileMock.mock.calls[1][3] as (
error: Error | null,
stdout: string
) => void
secondCallback(new Error('path disappeared'), '')
// Why: a cold or wedged distro times out without proving the path changed.
// Dropping the cache would force the next launch onto /mnt/... and rewrite
// trust under a path Codex will not use when automount is customized.
expect(settled).toHaveBeenCalledWith({ status: 'unavailable' })
expect(_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')).toBe('/home/alice')
})
it('drops the cached identity when WSL confirms the path is missing', () => {
setPlatform('win32')
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')
const firstCallback = execFileMock.mock.calls[0][3] as (
error: Error | null,
stdout: string
) => void
firstCallback(null, '/home/alice\n')
const settled = vi.fn()
expect(
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias', '/home/alias', settled)
).toBe('/home/alice')
const secondCallback = execFileMock.mock.calls[1][3] as (
error: Error | null,
stdout: string
) => void
secondCallback(null, '__ORCA_WSL_PATH_MISSING__\n')
expect(settled).toHaveBeenCalledWith({ status: 'missing' })
expect(_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')).toBeNull()
})
it('replaces a cached path when its canonical identity changes', () => {
setPlatform('win32')
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')
const firstCallback = execFileMock.mock.calls[0][3] as (
error: Error | null,
stdout: string
) => void
firstCallback(null, '/home/alice-old\n')
const settled = vi.fn()
expect(
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias', '/home/alias', settled)
).toBe('/home/alice-old')
const secondCallback = execFileMock.mock.calls[1][3] as (
error: Error | null,
stdout: string
) => void
secondCallback(null, '/home/alice-new\n')
expect(settled).toHaveBeenCalledWith({
status: 'resolved',
canonicalPath: '/home/alice-new'
})
expect(_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')).toBe('/home/alice-new')
})
it('ignores non-absolute readlink output', () => {
setPlatform('win32')
_internals.canonicalizeWslLinuxPath('Ubuntu', '/home/alias')
+95 -25
View File
@@ -15,7 +15,19 @@ export type CodexWslRuntimeHookInstallPlan = {
trustConfigPath: string
}
export type CanonicalizeWslLinuxPath = (distro: string, linuxPath: string) => string | null
export type WslCanonicalPathSettlement =
| { status: 'resolved'; canonicalPath: string }
| { status: 'missing' }
| { status: 'unavailable' }
export type WslCanonicalPathSettled = (settlement: WslCanonicalPathSettlement) => void
export type CanonicalizeWslLinuxPath = (
distro: string,
linuxPath: string,
windowsPath?: string,
onSettled?: WslCanonicalPathSettled
) => string | null
function trimTrailingSlash(value: string): string {
return value.length > 1 ? value.replace(/\/+$/, '') : value
@@ -30,61 +42,118 @@ function toDefaultWslLinuxPath(windowsPath: string): string {
}
const WSL_CANONICALIZE_TIMEOUT_MS = 5000
const WSL_PATH_MISSING_OUTPUT = '__ORCA_WSL_PATH_MISSING__'
// Why: `readlink -f` over wsl.exe stalls up to the timeout on a cold or wedged
// distro. Running it synchronously on the Electron main process froze the UI on
// every Codex WSL launch, so resolve it off-thread and cache the stable result.
// every Codex WSL launch, so resolve it off-thread and cache the latest result.
const canonicalWslPathCache = new Map<string, string>()
const inFlightWslCanonicalizations = new Set<string>()
const inFlightWslCanonicalizations = new Map<string, Set<WslCanonicalPathSettled>>()
function wslCanonicalizeCacheKey(distro: string, linuxPath: string): string {
return `${distro}\x00${linuxPath}`
}
function scheduleWslLinuxPathCanonicalization(distro: string, linuxPath: string): void {
function scheduleWslLinuxPathCanonicalization(
distro: string,
linuxPath: string,
windowsPath: string,
onSettled?: WslCanonicalPathSettled
): void {
const key = wslCanonicalizeCacheKey(distro, linuxPath)
// Why: cap the wedged-WSL blast radius to a single background subprocess per
// (distro, path); the cache holds the answer once one resolution succeeds.
if (canonicalWslPathCache.has(key) || inFlightWslCanonicalizations.has(key)) {
const listeners = inFlightWslCanonicalizations.get(key)
if (listeners) {
if (onSettled) {
listeners.add(onSettled)
}
return
}
inFlightWslCanonicalizations.add(key)
const nextListeners = new Set<WslCanonicalPathSettled>()
if (onSettled) {
nextListeners.add(onSettled)
}
inFlightWslCanonicalizations.set(key, nextListeners)
const drivePath = /^[A-Za-z]:[/\\]/.test(windowsPath)
// Why: wslpath reads each distro's automount root, so a custom root such as
// /windows is discovered without synchronously starting WSL on Electron main.
const args = drivePath
? [
'-d',
distro,
'--',
'sh',
'-c',
`resolved=$(wslpath -a -u "$1") || exit; if [ ! -d "$resolved" ]; then printf '%s\\n' '${WSL_PATH_MISSING_OUTPUT}'; exit 0; fi; readlink -f -- "$resolved"`,
'sh',
windowsPath
]
: [
'-d',
distro,
'--',
'sh',
'-c',
`if [ ! -d "$1" ]; then printf '%s\\n' '${WSL_PATH_MISSING_OUTPUT}'; exit 0; fi; readlink -f -- "$1"`,
'sh',
linuxPath
]
execFile(
'wsl.exe',
['-d', distro, '--', 'readlink', '-f', '--', linuxPath],
args,
{ encoding: 'utf-8', timeout: WSL_CANONICALIZE_TIMEOUT_MS, windowsHide: true },
(error, stdout) => {
inFlightWslCanonicalizations.delete(key)
if (error) {
return
}
const canonicalPath = stdout.trim()
if (canonicalPath.startsWith('/')) {
const resolvedPath = !error && canonicalPath.startsWith('/') ? canonicalPath : null
const pathMissing = !error && canonicalPath === WSL_PATH_MISSING_OUTPUT
const settlement: WslCanonicalPathSettlement = resolvedPath
? { status: 'resolved', canonicalPath: resolvedPath }
: pathMissing
? { status: 'missing' }
: { status: 'unavailable' }
if (settlement.status === 'resolved') {
canonicalWslPathCache.set(key, canonicalPath)
} else if (settlement.status === 'missing') {
// Why: a successful directory probe is stronger than a transport error;
// clear the identity so stale trust can be revoked and later rediscovered.
canonicalWslPathCache.delete(key)
}
// Why: keep the last known-good cache on timeout/transient WSL failures.
// Dropping it forces the next launch onto the logical `/mnt/...` guess,
// which is wrong under custom automount roots and rewrites trust keys.
const settledListeners = inFlightWslCanonicalizations.get(key) ?? new Set()
inFlightWslCanonicalizations.delete(key)
for (const listener of settledListeners) {
try {
listener(settlement)
} catch (listenerError) {
console.warn('[codex-wsl-hook-path] failed to reconcile canonical path', listenerError)
}
}
}
)
}
function canonicalizeWslLinuxPath(distro: string, linuxPath: string): string | null {
function canonicalizeWslLinuxPath(
distro: string,
linuxPath: string,
windowsPath = linuxPath,
onSettled?: WslCanonicalPathSettled
): string | null {
if (process.platform !== 'win32') {
return linuxPath
}
const cached = canonicalWslPathCache.get(wslCanonicalizeCacheKey(distro, linuxPath))
if (cached) {
return cached
}
// Why: no canonical path resolved yet — start the off-thread resolution and
// let the caller use the logical path for now (identical unless HOME crosses a
// symlink). The next launch reads the cached canonical path.
scheduleWslLinuxPathCanonicalization(distro, linuxPath)
return null
// Why: every launch revalidates asynchronously. Returning the cache keeps
// launch prep synchronous while settlement repairs or revokes trust in-place.
scheduleWslLinuxPathCanonicalization(distro, linuxPath, windowsPath, onSettled)
return cached ?? null
}
export function createCodexWslRuntimeHookInstallPlan(
runtimeHomePath: string | null | undefined,
target?: CodexWslRuntimeHookTarget,
canonicalize: CanonicalizeWslLinuxPath = canonicalizeWslLinuxPath
canonicalize: CanonicalizeWslLinuxPath = canonicalizeWslLinuxPath,
onCanonicalPathSettled?: WslCanonicalPathSettled
): CodexWslRuntimeHookInstallPlan | null {
if (!runtimeHomePath) {
return null
@@ -106,7 +175,8 @@ export function createCodexWslRuntimeHookInstallPlan(
// Why: Codex canonicalizes hook sources inside WSL; resolving there keeps
// trust keys valid when HOME or the runtime directory crosses a symlink.
const linuxRuntimeHome = trimTrailingSlash(
canonicalize(distro, logicalLinuxRuntimeHome) ?? logicalLinuxRuntimeHome
canonicalize(distro, logicalLinuxRuntimeHome, runtimeHomePath, onCanonicalPathSettled) ??
logicalLinuxRuntimeHome
)
return {
+28
View File
@@ -1515,6 +1515,34 @@ describe('readHookTrustEntries', () => {
}
)
it.skipIf(process.platform !== 'win32')(
'keeps WSL hook trust paths case-sensitive on a Windows host',
() => {
const upperKey = '/windows/d/Repo/hooks.json:session_start:0:0'
const lowerKey = '/windows/d/repo/hooks.json:session_start:0:0'
writeFileSync(
configPath,
[
`[hooks.state."${upperKey}"]`,
'enabled = true',
'trusted_hash = "sha256:UPPER"',
'',
`[hooks.state."${lowerKey}"]`,
'enabled = true',
'trusted_hash = "sha256:LOWER"',
''
].join('\n'),
'utf-8'
)
const result = readHookTrustEntries(configPath)
expect(result.get(upperKey)?.trustedHash).toBe('sha256:UPPER')
expect(result.get(lowerKey)?.trustedHash).toBe('sha256:LOWER')
expect(result.size).toBe(2)
}
)
it('reads entries from a CRLF-terminated config', () => {
const key = '/x/hooks.json:pre_tool_use:0:0'
const original = [
+5 -1
View File
@@ -529,7 +529,11 @@ export function normalizeHookTrustKeyForLookup(key: string): string {
const separated = parsed
? `${normalizeWindowsPathSeparators(parsed.sourcePath)}:${parsed.eventLabel}:${parsed.groupIndex}:${parsed.handlerIndex}`
: normalizeWindowsPathSeparators(key)
return process.platform === 'win32' ? separated.toLowerCase() : separated
// Why: Windows-native paths are case-insensitive, but WSL and SSH trust
// sources remain case-sensitive even when Orca's host process is Windows.
const caseInsensitiveWindowsPath =
process.platform === 'win32' && usesWindowsPathSeparators(parsed?.sourcePath ?? key)
return caseInsensitiveWindowsPath ? separated.toLowerCase() : separated
}
function findTrustBlockRanges(content: string, key: string): TrustBlockRange[] {
@@ -122,6 +122,94 @@ describe('Codex WSL runtime hook install', () => {
expect(plan?.configPath).toBe(pathWin32.join(runtimeHome, 'hooks.json'))
})
it('removes managed trust when the WSL canonical path changes', () => {
const plan = createTestPlan()
writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8')
writeFileSync(plan.tomlPath, '', 'utf-8')
const oldPlan = {
...plan,
commandScriptPath: '/old/home/.orca/agent-hooks/codex-hook.sh',
trustConfigPath: '/old/home/hooks.json'
}
expect(_internals.installManagedHooksIntoWslRuntime(oldPlan).state).toBe('installed')
const oldCommand = `if [ -r '${oldPlan.commandScriptPath}' ]; then /bin/sh '${oldPlan.commandScriptPath}'; fi`
const oldKey = computeTrustKey(getManagedTrustEntry(oldPlan, oldCommand))
const newPlan = {
...plan,
commandScriptPath: '/new/home/.orca/agent-hooks/codex-hook.sh',
trustConfigPath: '/new/home/hooks.json'
}
expect(_internals.installManagedHooksIntoWslRuntime(newPlan).state).toBe('installed')
const newCommand = `if [ -r '${newPlan.commandScriptPath}' ]; then /bin/sh '${newPlan.commandScriptPath}'; fi`
const newKey = computeTrustKey(getManagedTrustEntry(newPlan, newCommand))
const trustEntries = readHookTrustEntries(plan.tomlPath)
expect(trustEntries.has(oldKey)).toBe(false)
expect(trustEntries.has(newKey)).toBe(true)
})
it('sweeps all managed WSL trust for disable or confirmed absence', () => {
// Why: disable and confirmed absence intentionally pass []. Transient
// unavailability must NOT use this path — last known-good trust remains.
const plan = createTestPlan()
writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8')
writeFileSync(plan.tomlPath, '', 'utf-8')
expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed')
_internals.removeStaleWslRuntimeManagedHookTrustEntries(plan.tomlPath, [])
expect(readHookTrustEntries(plan.tomlPath).size).toBe(0)
})
it('reconciles only current, conclusive WSL path settlements', () => {
expect(
_internals.getWslHookReconciliationAction({
settlement: { status: 'unavailable' },
isCurrentGeneration: true,
installedTrustConfigPath: '/mnt/d/home/hooks.json',
resolvedTrustConfigPath: null
})
).toBe('none')
expect(
_internals.getWslHookReconciliationAction({
settlement: { status: 'missing' },
isCurrentGeneration: false,
installedTrustConfigPath: '/mnt/d/home/hooks.json',
resolvedTrustConfigPath: null
})
).toBe('none')
expect(
_internals.getWslHookReconciliationAction({
settlement: { status: 'missing' },
isCurrentGeneration: true,
installedTrustConfigPath: '/mnt/d/home/hooks.json',
resolvedTrustConfigPath: null
})
).toBe('remove')
expect(
_internals.getWslHookReconciliationAction({
settlement: { status: 'resolved', canonicalPath: '/windows/d/home' },
isCurrentGeneration: true,
installedTrustConfigPath: '/windows/d/home/hooks.json',
resolvedTrustConfigPath: '/windows/d/home/hooks.json'
})
).toBe('none')
expect(
_internals.getWslHookReconciliationAction({
settlement: { status: 'resolved', canonicalPath: '/windows/d/home' },
isCurrentGeneration: true,
installedTrustConfigPath: '/mnt/d/home/hooks.json',
resolvedTrustConfigPath: '/windows/d/home/hooks.json'
})
).toBe('reinstall')
})
it('generates a POSIX hook that bridges WSL loopback failures through Windows curl', () => {
const script = _internals.getManagedScript('posix')
expect(script).toContain('load_hook_endpoint()')
+149 -4
View File
@@ -1,6 +1,6 @@
/* eslint-disable max-lines -- Why: getStatus + install + remove all share the managed-command and trust-key derivation. Splitting would hide that the three operations must agree on group index, event label, and command bytes. */
import { existsSync, readFileSync, unlinkSync } from 'node:fs'
import { join } from 'node:path'
import { join, win32 as pathWin32 } from 'node:path'
import type { SFTPWrapper } from 'ssh2'
import type { AgentHookInstallState, AgentHookInstallStatus } from '../../shared/agent-hook-types'
import {
@@ -46,7 +46,8 @@ import { syncSystemConfigIntoManagedCodexHome } from './codex-config-mirror'
import {
createCodexWslRuntimeHookInstallPlan,
type CodexWslRuntimeHookInstallPlan,
type CodexWslRuntimeHookTarget
type CodexWslRuntimeHookTarget,
type WslCanonicalPathSettlement
} from './codex-wsl-hook-install-plan'
import {
CODEX_HOOK_EVENT_LABEL,
@@ -723,6 +724,52 @@ function removeWslRuntimeManagedHookTrustEntries(plan: CodexWslRuntimeHookInstal
}
}
function removeStaleWslRuntimeManagedHookTrustEntries(
tomlPath: string,
desiredEntries: readonly CodexTrustEntry[]
): void {
const desiredKeys = new Set(
desiredEntries.map((entry) => normalizeHookTrustKeyForLookup(computeTrustKey(entry)))
)
const existingEntries = readHookTrustEntries(tomlPath)
const ourKeys: string[] = []
for (const [key, state] of existingEntries) {
if (desiredKeys.has(normalizeHookTrustKeyForLookup(key))) {
continue
}
const parts = parseTrustKey(key)
if (!parts || !CODEX_MANAGED_EVENT_LABELS.has(parts.eventLabel)) {
continue
}
const sourcePath = parts.sourcePath
// Why: this cleanup owns only guest-side WSL trust. A runtime config can
// still contain user Windows/remote hooks, which must remain untouched.
if (!sourcePath.startsWith('/') || !sourcePath.endsWith('/hooks.json')) {
continue
}
const runtimeHome = sourcePath.slice(0, -'/hooks.json'.length)
const command = wrapReadablePosixHookCommand(`${runtimeHome}/.orca/agent-hooks/codex-hook.sh`)
const expectedEntry: CodexTrustEntry = {
sourcePath: parts.sourcePath,
eventLabel: parts.eventLabel,
groupIndex: parts.groupIndex,
handlerIndex: parts.handlerIndex,
command,
timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS
}
const recognizedHashes = new Set([
computeTrustedHash(expectedEntry),
computeTrustedHash({ ...expectedEntry, timeoutSec: undefined })
])
if (state.trustedHash && recognizedHashes.has(state.trustedHash)) {
ourKeys.push(key)
}
}
if (ourKeys.length > 0) {
removeHookTrustEntries(tomlPath, ourKeys)
}
}
function getManagedScript(target: 'local' | 'posix' = 'local'): string {
if (target === 'local' && process.platform === 'win32') {
return [
@@ -876,6 +923,7 @@ function installManagedHooksIntoWslRuntime(
// Why: WSL runtime homes may carry user hook approvals we did not rebuild
// here; only upsert Orca's entries instead of sweeping the whole source.
upsertHookTrustEntries(plan.tomlPath, trustEntries)
removeStaleWslRuntimeManagedHookTrustEntries(plan.tomlPath, trustEntries)
} catch (error) {
return {
agent: 'codex',
@@ -922,6 +970,13 @@ function refreshWslRuntimeUserHooks(plan: CodexWslRuntimeHookInstallPlan): Agent
}
writeCodexHooksJson(plan.configPath, nextHooks)
removeWslRuntimeManagedHookTrustEntries(plan)
try {
// Why: the disabled path may be reached after the WSL mount root changed,
// so cleanup cannot be scoped only to the plan's current source path.
removeStaleWslRuntimeManagedHookTrustEntries(plan.tomlPath, [])
} catch (error) {
console.warn('[codex-hook-service] failed to clean stale WSL trust entries', error)
}
return {
agent: 'codex',
state: 'not_installed',
@@ -931,12 +986,99 @@ function refreshWslRuntimeUserHooks(plan: CodexWslRuntimeHookInstallPlan): Agent
}
}
// Why: transport failures preserve the last known-good identity, while a
// successful absence probe is strong enough to revoke trust immediately.
function getWslHookReconciliationAction(args: {
settlement: WslCanonicalPathSettlement
isCurrentGeneration: boolean
installedTrustConfigPath: string | null
resolvedTrustConfigPath: string | null
}): 'none' | 'remove' | 'reinstall' {
if (!args.isCurrentGeneration) {
return 'none'
}
if (args.settlement.status === 'missing') {
return 'remove'
}
if (
args.settlement.status !== 'resolved' ||
!args.resolvedTrustConfigPath ||
args.resolvedTrustConfigPath === args.installedTrustConfigPath
) {
return 'none'
}
return 'reinstall'
}
export class CodexHookService {
private readonly wslReconciliationGeneration = new Map<string, number>()
private supersedeWslReconciliation(runtimeHomePath: string | null | undefined): number {
if (!runtimeHomePath) {
return 0
}
const key = process.platform === 'win32' ? runtimeHomePath.toLowerCase() : runtimeHomePath
const generation = (this.wslReconciliationGeneration.get(key) ?? 0) + 1
this.wslReconciliationGeneration.set(key, generation)
return generation
}
installForRuntimeHome(
runtimeHomePath: string | null | undefined,
target?: CodexWslRuntimeHookTarget
): AgentHookInstallStatus | null {
const wslPlan = createCodexWslRuntimeHookInstallPlan(runtimeHomePath, target)
const generation = this.supersedeWslReconciliation(runtimeHomePath)
let installedTrustConfigPath: string | null = null
const onCanonicalPathSettled = (settlement: WslCanonicalPathSettlement): void => {
if (!runtimeHomePath) {
return
}
const key = process.platform === 'win32' ? runtimeHomePath.toLowerCase() : runtimeHomePath
const resolvedPlan =
settlement.status === 'resolved'
? createCodexWslRuntimeHookInstallPlan(
runtimeHomePath,
target,
() => settlement.canonicalPath
)
: null
const action = getWslHookReconciliationAction({
settlement,
isCurrentGeneration: this.wslReconciliationGeneration.get(key) === generation,
installedTrustConfigPath,
resolvedTrustConfigPath: resolvedPlan?.trustConfigPath ?? null
})
if (action === 'none') {
return
}
if (action === 'remove') {
try {
removeStaleWslRuntimeManagedHookTrustEntries(
pathWin32.join(runtimeHomePath, 'config.toml'),
[]
)
} catch (error) {
console.warn('[codex-hook-service] failed to revoke stale WSL hook trust', error)
}
return
}
if (!resolvedPlan) {
return
}
const status = installManagedHooksIntoWslRuntime(resolvedPlan)
if (status.state === 'error') {
console.warn('[codex-hook-service] failed to reconcile WSL hook path', status.detail)
return
}
installedTrustConfigPath = resolvedPlan.trustConfigPath
}
const wslPlan = createCodexWslRuntimeHookInstallPlan(
runtimeHomePath,
target,
undefined,
onCanonicalPathSettled
)
installedTrustConfigPath = wslPlan?.trustConfigPath ?? null
return wslPlan ? installManagedHooksIntoWslRuntime(wslPlan) : null
}
@@ -944,6 +1086,7 @@ export class CodexHookService {
runtimeHomePath: string | null | undefined,
target?: CodexWslRuntimeHookTarget
): AgentHookInstallStatus | null {
this.supersedeWslReconciliation(runtimeHomePath)
const wslPlan = createCodexWslRuntimeHookInstallPlan(runtimeHomePath, target)
return wslPlan ? refreshWslRuntimeUserHooks(wslPlan) : null
}
@@ -1413,5 +1556,7 @@ export const codexHookService = new CodexHookService()
export const _internals = {
getManagedScript,
installManagedHooksIntoWslRuntime,
refreshWslRuntimeUserHooks
refreshWslRuntimeUserHooks,
removeStaleWslRuntimeManagedHookTrustEntries,
getWslHookReconciliationAction
}
+43 -1
View File
@@ -253,7 +253,7 @@ branch refs/heads/main
])
})
it('passes --force before the worktree path when forced removal is requested', async () => {
it('passes one --force before the worktree path for dirty-file removal', async () => {
mockGitCommands({
'git worktree list --porcelain': {
stdout: `worktree /repo
@@ -278,6 +278,48 @@ branch refs/heads/main
expect(getGitCalls()).toContain('git worktree remove --force /repo-feature')
})
it('rejects a locked worktree with stable app-owned copy before invoking remove', async () => {
mockGitCommands({
'git worktree list --porcelain': {
stdout: `worktree /repo
HEAD abc123
branch refs/heads/main
worktree /repo-feature
HEAD def456
branch refs/heads/feature/test
locked active agent session
`
}
})
await expect(removeWorktree('/repo', '/repo-feature', true)).rejects.toThrow(
'Worktree is locked by Git. Lock reason: active agent session.'
)
expect(getGitCalls()).not.toContain('git worktree remove /repo-feature')
})
it('does not treat dirty-file force as permission to override a lock', async () => {
mockGitCommands({
'git worktree list --porcelain': {
stdout: `worktree /repo
HEAD abc123
branch refs/heads/main
worktree /repo-feature
HEAD def456
branch refs/heads/feature/test
locked active agent session
`
}
})
await expect(removeWorktree('/repo', '/repo-feature', true)).rejects.toThrow(
'Worktree is locked by Git. Lock reason: active agent session.'
)
expect(getGitCalls()).not.toContain('git worktree remove --force /repo-feature')
})
it('matches Windows worktree paths before deleting the branch', async () => {
mockGitCommands({
'git worktree list --porcelain': {
+46
View File
@@ -33,6 +33,39 @@ async function createRepoWithNewlineWorktree(): Promise<{
}
}
async function createRepoWithLockedDeletedWorktree(): Promise<{
repoPath: string
worktreePath: string
}> {
const root = await mkdtemp(path.join(tmpdir(), 'orca-worktree-locked-delete-'))
tempRoots.push(root)
const repoPath = path.join(root, 'repo')
const requestedWorktreePath = path.join(root, 'locked deleted worktree')
execFileSync('git', ['init', '--quiet', repoPath])
git(repoPath, ['symbolic-ref', 'HEAD', 'refs/heads/main'])
git(repoPath, ['config', 'user.email', 'test@example.com'])
git(repoPath, ['config', 'user.name', 'Test User'])
git(repoPath, ['commit', '--allow-empty', '--quiet', '-m', 'initial'])
git(repoPath, [
'worktree',
'add',
'--quiet',
'-b',
'feature/locked-delete',
requestedWorktreePath
])
git(repoPath, ['worktree', 'lock', '--reason', 'orca locked stale repro', requestedWorktreePath])
const worktreePath = await realpath(requestedWorktreePath)
await rm(worktreePath, { recursive: true, force: true })
return {
repoPath: await realpath(repoPath),
worktreePath
}
}
function branchExists(repoPath: string, branchName: string): boolean {
try {
git(repoPath, ['show-ref', '--verify', '--quiet', `refs/heads/${branchName}`])
@@ -68,4 +101,17 @@ describe('git worktree paths', () => {
expect(branchExists(repoPath, 'feature/newline')).toBe(false)
}
)
it('preserves a locked worktree whose directory was deleted manually', async () => {
const { repoPath, worktreePath } = await createRepoWithLockedDeletedWorktree()
await expect(removeWorktree(repoPath, worktreePath, true)).rejects.toThrow(
'Worktree is locked by Git'
)
expect(git(repoPath, ['worktree', 'list', '--porcelain'])).toContain(
worktreePath.replaceAll('\\', '/')
)
expect(branchExists(repoPath, 'feature/locked-delete')).toBe(true)
})
})
+41
View File
@@ -104,6 +104,47 @@ describe('listWorktrees in-flight sharing', () => {
})
describe('parseWorktreeList', () => {
it('preserves a locked marker and its reason', () => {
expect(
parseWorktreeList(
'worktree /repo\nHEAD abc\nbranch refs/heads/main\n\nworktree /locked\nHEAD def\nbranch refs/heads/feature\nlocked active agent session\n'
)[1]
).toMatchObject({
path: '/locked',
locked: true,
lockReason: 'active agent session'
})
})
it('decodes C-quoted lock reasons from legacy line porcelain output', () => {
const output =
'worktree /repo\nHEAD abc\nbranch refs/heads/main\n\nworktree /locked\nHEAD def\nbranch refs/heads/feature\nlocked "first line\\nsecond line \\303\\251"\n'
expect(parseWorktreeList(output)[1]).toMatchObject({
locked: true,
lockReason: 'first line\nsecond line é'
})
})
it('keeps NUL-delimited lock reasons raw', () => {
const output = [
'worktree /repo',
'HEAD abc',
'branch refs/heads/main',
'',
'worktree /locked',
'HEAD def',
'branch refs/heads/feature',
'locked "literal\\nquote"',
''
].join('\0')
expect(parseWorktreeList(output, { nulDelimited: true })[1]).toMatchObject({
locked: true,
lockReason: '"literal\\nquote"'
})
})
it('parses regular and bare worktree blocks from porcelain output', () => {
const output = `
worktree /repo
+15 -1
View File
@@ -13,6 +13,8 @@ import type {
LocalBaseRefUpdateSuggestion,
RemoveWorktreeResult
} from '../../shared/types'
import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal'
import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path'
import { parseGitRevListAheadBehindCounts } from '../../shared/git-rev-list-output'
import { parseWslUncPath } from '../../shared/wsl-paths'
import {
@@ -52,7 +54,7 @@ export type AddWorktreeOptions = GitWorktreeExecOptions & {
export type RemoveWorktreeOptions = GitWorktreeExecOptions & {
deleteBranch?: boolean
forceBranchDelete?: boolean
knownRemovedWorktree?: Pick<GitWorktreeInfo, 'branch' | 'head'>
knownRemovedWorktree?: Pick<GitWorktreeInfo, 'branch' | 'head' | 'locked' | 'lockReason'>
}
type LocalBaseRefRefreshability =
@@ -487,6 +489,8 @@ export function parseWorktreeList(
let branch = ''
let isBare = false
let isSparse = false
let locked = false
let lockReason = ''
for (const line of lines) {
if (line.startsWith('worktree ')) {
@@ -499,6 +503,10 @@ export function parseWorktreeList(
isBare = true
} else if (line === 'sparse') {
isSparse = true
} else if (line === 'locked' || line.startsWith('locked ')) {
locked = true
const rawReason = line.slice('locked'.length).trim()
lockReason = options.nulDelimited ? rawReason : decodeGitCQuotedPath(rawReason)
}
}
@@ -510,6 +518,8 @@ export function parseWorktreeList(
branch,
isBare,
...(isSparse ? { isSparse } : {}),
...(locked ? { locked: true } : {}),
...(lockReason ? { lockReason } : {}),
isMainWorktree: worktrees.length === 0
})
}
@@ -1079,6 +1089,10 @@ async function performRemoveWorktree(
const branchName = normalizeLocalBranchRef(removedWorktree?.branch ?? '')
const branchHead = removedWorktree?.head ?? ''
// Why: callers outside the IPC/runtime preflight must not bypass Git's lock
// contract or depend on localized stderr to discover it after side effects.
assertWorktreeUnlockedForRemoval(removedWorktree)
const args = ['worktree', 'remove']
if (force) {
args.push('--force')
+165 -23
View File
@@ -5,8 +5,10 @@ import { lstat, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join, resolve } from 'node:path'
import type { CreateWorktreeResult, GitWorktreeInfo, Worktree } from '../../shared/types'
import * as localWorktreeFilesystem from '../local-worktree-filesystem'
const ORIGINAL_PLATFORM = process.platform
const removeWorktreeLinkedPathsMock = vi.hoisted(() => vi.fn())
function setPlatform(platform: NodeJS.Platform): void {
Object.defineProperty(process, 'platform', {
@@ -168,6 +170,11 @@ vi.mock('../providers/ssh-filesystem-dispatch', () => ({
getSshFilesystemProvider: getSshFilesystemProviderMock
}))
vi.mock('./worktree-symlinks', () => ({
createWorktreeLinkedPaths: vi.fn(),
removeWorktreeLinkedPaths: removeWorktreeLinkedPathsMock
}))
vi.mock('./ssh', () => ({
getActiveMultiplexer: getActiveMultiplexerMock
}))
@@ -336,7 +343,8 @@ describe('registerWorktreeHandlers', () => {
clearProviderPtyStateMock,
getLocalPtyProviderMock,
deleteWorktreeHistoryDirMock,
advertisedUrlWatcherForgetWorktreeMock
advertisedUrlWatcherForgetWorktreeMock,
removeWorktreeLinkedPathsMock
]) {
m.mockReset()
}
@@ -661,8 +669,8 @@ describe('registerWorktreeHandlers', () => {
function mockKnownFeatureWorktree(
path = '/workspace/feature-wt',
repoPath = '/workspace/repo'
): void {
listWorktreesMock.mockResolvedValue([
): GitWorktreeInfo[] {
const worktrees: GitWorktreeInfo[] = [
{
path: repoPath,
head: 'main',
@@ -677,7 +685,9 @@ describe('registerWorktreeHandlers', () => {
isBare: false,
isMainWorktree: false
}
])
]
listWorktreesMock.mockResolvedValue(worktrees)
return worktrees
}
function makeWorktreeMeta(overrides: Record<string, unknown> = {}): Record<string, unknown> {
@@ -6261,7 +6271,11 @@ describe('registerWorktreeHandlers', () => {
const worktreePath = join(parentDir, 'feature-wt')
await mkdir(worktreePath, { recursive: true })
await writeFile(join(worktreePath, 'scratch.txt'), 'delete me')
mockKnownFeatureWorktree(worktreePath, repoPath)
const registeredWorktrees = mockKnownFeatureWorktree(worktreePath, repoPath)
listWorktreesMock
.mockResolvedValueOnce(registeredWorktrees)
.mockResolvedValueOnce(registeredWorktrees)
.mockResolvedValue([])
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta())
const longPathError = Object.assign(new Error('git worktree remove failed'), {
stderr: 'error: failed to delete deep/file.txt: Filename too long'
@@ -6295,7 +6309,11 @@ describe('registerWorktreeHandlers', () => {
it('does not create a preserved-branch target when long-path recovery preserves branch by policy', async () => {
setPlatform('win32')
mockKnownFeatureWorktree()
const registeredWorktrees = mockKnownFeatureWorktree()
listWorktreesMock
.mockResolvedValueOnce(registeredWorktrees)
.mockResolvedValueOnce(registeredWorktrees)
.mockResolvedValue([])
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ preserveBranchOnDelete: true }))
removeWorktreeMock.mockRejectedValue(
Object.assign(new Error('git worktree remove failed'), {
@@ -6335,39 +6353,49 @@ describe('registerWorktreeHandlers', () => {
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
})
it('keeps metadata when Windows long-path recovery deletes the directory but prune fails', async () => {
it('refuses Windows recovery while Git still reports the row and keeps metadata', async () => {
setPlatform('win32')
mockKnownFeatureWorktree()
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta())
const removePathSpy = vi
.spyOn(localWorktreeFilesystem, 'removeLocalWorktreePath')
.mockResolvedValue(undefined)
removeWorktreeMock.mockRejectedValue(
Object.assign(new Error('git worktree remove failed'), {
stderr: 'error: failed to delete deep/file.txt: Filename too long'
})
)
gitExecFileAsyncMock.mockRejectedValue(
Object.assign(new Error('git prune failed'), {
stderr: 'fatal: unable to lock worktree admin dir'
})
)
await expect(
handlers['worktrees:remove'](null, {
worktreeId: 'repo-1::/workspace/feature-wt',
force: true
})
).rejects.toThrow('Git still has stale worktree registration')
try {
await expect(
handlers['worktrees:remove'](null, {
worktreeId: 'repo-1::/workspace/feature-wt',
force: true
})
).rejects.toThrow(
'Failed to force delete worktree at /workspace/feature-wt. error: failed to delete deep/file.txt: Filename too long'
)
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
expect(mainWindow.webContents.send).not.toHaveBeenCalledWith('worktrees:changed', {
repoId: 'repo-1'
})
expect(removePathSpy).not.toHaveBeenCalled()
expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(
['worktree', 'prune'],
expect.anything()
)
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
expect(mainWindow.webContents.send).not.toHaveBeenCalledWith('worktrees:changed', {
repoId: 'repo-1'
})
} finally {
removePathSpy.mockRestore()
}
})
it('retries stale Git registration cleanup after prior local filesystem recovery', async () => {
setPlatform('win32')
const missingWorktreePath = 'C:\\workspace\\already-removed'
const worktreeId = `repo-1::${missingWorktreePath}`
mockKnownFeatureWorktree(missingWorktreePath)
const registeredWorktrees = mockKnownFeatureWorktree(missingWorktreePath)
listWorktreesMock.mockResolvedValueOnce(registeredWorktrees).mockResolvedValue([])
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta())
const result = await handlers['worktrees:remove'](null, {
@@ -6387,6 +6415,40 @@ describe('registerWorktreeHandlers', () => {
expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId)
})
it('preserves a locked missing registration even with force', async () => {
setPlatform('win32')
const missingWorktreePath = 'C:\\workspace\\locked-already-removed'
const worktreeId = `repo-1::${missingWorktreePath}`
const registeredWorktrees: GitWorktreeInfo[] = [
{
path: '/workspace/repo',
head: 'main',
branch: 'main',
isBare: false,
isMainWorktree: true
},
{
path: missingWorktreePath,
head: 'feature',
branch: 'feature',
isBare: false,
isMainWorktree: false,
locked: true,
lockReason: 'active agent session'
}
]
listWorktreesMock.mockResolvedValue(registeredWorktrees)
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta())
removeWorktreeMock.mockResolvedValue({})
await expect(handlers['worktrees:remove'](null, { worktreeId, force: true })).rejects.toThrow(
'Worktree is locked by Git. Lock reason: active agent session'
)
expect(removeWorktreeMock).not.toHaveBeenCalled()
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
})
it('refuses to delete the root workspace for folder-mode repos', async () => {
store.getRepo.mockReturnValue({
id: 'repo-folder',
@@ -7905,6 +7967,86 @@ describe('registerWorktreeHandlers', () => {
expect(removeWorktreeMock).not.toHaveBeenCalled()
})
it('fails locked dirty-force deletes before hooks, link cleanup, or PTY teardown', async () => {
listWorktreesMock.mockResolvedValue([
{
path: '/workspace/repo',
head: 'main',
branch: 'main',
isBare: false,
isMainWorktree: true
},
{
path: '/workspace/feature-wt',
head: 'feature',
branch: 'feature',
isBare: false,
isMainWorktree: false,
locked: true,
lockReason: 'active agent session'
}
])
store.getRepo.mockReturnValue({
id: 'repo-1',
path: '/workspace/repo',
displayName: 'repo',
badgeColor: '#000',
addedAt: 0,
symlinkPaths: ['node_modules']
})
getEffectiveHooksMock.mockReturnValue({ scripts: { archive: 'echo archived' } })
await expect(
handlers['worktrees:remove'](null, {
worktreeId: 'repo-1::/workspace/feature-wt',
force: true
})
).rejects.toThrow(
'Failed to force delete worktree at /workspace/feature-wt. Worktree is locked by Git.'
)
expect(assertWorktreeCleanForRemovalMock).not.toHaveBeenCalled()
expect(runHookMock).not.toHaveBeenCalled()
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
expect(removeWorktreeMock).not.toHaveBeenCalled()
})
it('rechecks a local Git lock after the archive hook before teardown', async () => {
const unlockedWorktrees = mockKnownFeatureWorktree()
const lockedWorktrees = unlockedWorktrees.map((worktree) =>
worktree.path === '/workspace/feature-wt'
? { ...worktree, locked: true, lockReason: 'locked during archive' }
: worktree
)
listWorktreesMock
.mockResolvedValueOnce(unlockedWorktrees)
.mockResolvedValueOnce(lockedWorktrees)
store.getRepo.mockReturnValue({
id: 'repo-1',
path: '/workspace/repo',
displayName: 'repo',
badgeColor: '#000',
addedAt: 0,
symlinkPaths: ['node_modules']
})
getEffectiveHooksMock.mockReturnValue({ scripts: { archive: 'echo archived' } })
runHookMock.mockResolvedValue({ success: true, output: '' })
await expect(
handlers['worktrees:remove'](null, {
worktreeId: 'repo-1::/workspace/feature-wt',
force: true
})
).rejects.toThrow('Worktree is locked by Git')
expect(runHookMock).toHaveBeenCalled()
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
expect(assertWorktreeCleanForRemovalMock).not.toHaveBeenCalled()
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
expect(removeWorktreeMock).not.toHaveBeenCalled()
})
it('formats preflight subprocess failures and does not tear down PTYs', async () => {
mockKnownFeatureWorktree()
getEffectiveHooksMock.mockReturnValue(null)
+49 -14
View File
@@ -29,6 +29,7 @@ import type {
Worktree,
WorktreeMeta
} from '../../shared/types'
import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import {
buildKnownOrcaWorkspaceLayouts,
@@ -160,8 +161,8 @@ import {
toLocalWorktreeRuntimePath
} from '../local-worktree-filesystem'
import {
pruneStaleLocalWorktreeRegistrationAfterFilesystemRemoval,
recoverLocalWindowsLongPathWorktreeRemoval
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval,
recoverLocalWindowsWorktreeRemoval
} from '../local-worktree-removal-recovery'
const WORKTREE_ARCHIVE_HOOK_TIMEOUT_MS = 120_000
@@ -1583,8 +1584,18 @@ export function registerWorktreeHandlers(
const canonicalWorktreePath = registeredWorktree.path
const deleteBranch = removedMeta?.preserveBranchOnDelete !== true
// Why: a Git lock must block before archive hooks or linked-path cleanup
// mutate the workspace; dirty-file force is a separate permission.
try {
assertWorktreeUnlockedForRemoval(registeredWorktree)
} catch (error) {
throw new Error(
formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false)
)
}
// Why: a prior forced Windows recovery can delete the directory but leave
// Git's stale registration; retry by pruning instead of removing a missing path.
// Git's stale registration; recover and verify it before clearing metadata.
if (
!repo.connectionId &&
args.force === true &&
@@ -1594,7 +1605,7 @@ export function registerWorktreeHandlers(
removedMeta &&
(await isAlreadyRemovedWorktreePath(repo, canonicalWorktreePath, localWorktreeGitOptions))
) {
const removalResult = await pruneStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
canonicalWorktreePath,
repoPath: repo.path,
localWorktreeGitOptions,
@@ -1655,9 +1666,10 @@ export function registerWorktreeHandlers(
}
}
const rawRemovalResult = await (deleteBranch
? provider!.removeWorktree(canonicalWorktreePath, args.force)
: provider!.removeWorktree(canonicalWorktreePath, args.force, { deleteBranch }))
const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {}
const rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0
? provider!.removeWorktree(canonicalWorktreePath, args.force, remoteRemoveOptions)
: provider!.removeWorktree(canonicalWorktreePath, args.force))
const removalResult = preserveBranchHeadFallback(
rawRemovalResult,
registeredWorktree.head
@@ -1681,6 +1693,29 @@ export function registerWorktreeHandlers(
return removalResult ?? {}
}
const refreshedWorktrees = hasLocalWorktreeGitOptions
? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions)
: await listGitWorktreesStrict(repo.path)
const refreshedRegisteredWorktree = findRegisteredDeletableWorktree(
repo.path,
canonicalWorktreePath,
refreshedWorktrees
)
if (!refreshedRegisteredWorktree) {
throw new Error(
`Worktree registration changed during deletion: ${canonicalWorktreePath}. Retry deletion.`
)
}
try {
// Why: an archive hook can race another Git client that locks the row;
// recheck before linked-path, watcher, or terminal teardown side effects.
assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree)
} catch (error) {
throw new Error(
formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false)
)
}
// Why: `git worktree remove` (non-force) refuses to delete a worktree
// that has untracked files. User-configured symlinks look untracked,
// so unlink them before the clean check; regular APFS clone-copied
@@ -1738,7 +1773,7 @@ export function registerWorktreeHandlers(
// Why: this handler already paid for an authoritative worktree
// list to validate the target; reuse it instead of rescanning
// every sibling worktree during the hot delete path.
knownRemovedWorktree: registeredWorktree,
knownRemovedWorktree: refreshedRegisteredWorktree,
...(hasLocalWorktreeGitOptions ? localWorktreeGitOptions : {})
}
removalResult = preserveBranchHeadFallback(
@@ -1748,18 +1783,18 @@ export function registerWorktreeHandlers(
args.force ?? false,
removeOptions
),
registeredWorktree.head
refreshedRegisteredWorktree.head
)
} catch (error) {
// Why: Git for Windows can fail long-path directory deletion after
// Orca has already validated the target and explicit force delete.
const recoveredRemovalResult = await recoverLocalWindowsLongPathWorktreeRemoval({
// Why: Git for Windows can deregister a clean worktree before its
// recursive filesystem deletion fails transiently.
const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({
error,
force: args.force ?? false,
canonicalWorktreePath,
repoPath: repo.path,
localWorktreeGitOptions,
registeredWorktree,
registeredWorktree: refreshedRegisteredWorktree,
deleteBranch,
closeWatcher: closeLocalWatcherForRemoval
})
@@ -1825,7 +1860,7 @@ export function registerWorktreeHandlers(
rememberPreservedBranchCleanupTarget(
args.worktreeId,
removalResult,
registeredWorktree.head,
refreshedRegisteredWorktree.head,
removedPushTarget
)
runtime.clearOptimisticReconcileToken(args.worktreeId)
@@ -0,0 +1,128 @@
import { spawn, spawnSync, type ChildProcess } from 'node:child_process'
import { existsSync } from 'node:fs'
import { mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import path from 'node:path'
import { setTimeout as delay } from 'node:timers/promises'
import { afterEach, describe, expect, it } from 'vitest'
import { recoverLocalWindowsWorktreeRemoval } from './local-worktree-removal-recovery'
const tempRoots: string[] = []
function git(cwd: string, args: string[]): string {
const result = spawnSync('git', args, { cwd, encoding: 'utf8' })
if (result.status !== 0) {
throw new Error(result.stderr || result.stdout || `git ${args.join(' ')} failed`)
}
return result.stdout.trim()
}
async function waitForPath(targetPath: string): Promise<void> {
const deadline = Date.now() + 5_000
while (!existsSync(targetPath)) {
if (Date.now() >= deadline) {
throw new Error(`Timed out waiting for ${targetPath}`)
}
await delay(20)
}
}
async function stopProcess(child: ChildProcess): Promise<void> {
if (child.exitCode !== null || child.signalCode !== null) {
return
}
const exited = new Promise<void>((resolve) => child.once('exit', () => resolve()))
child.kill()
await Promise.race([exited, delay(500)])
if (child.exitCode === null && child.pid) {
spawnSync('taskkill', ['/pid', String(child.pid), '/t', '/f'], { stdio: 'ignore' })
await Promise.race([exited, delay(2_000)])
}
if (child.exitCode === null && child.signalCode === null) {
throw new Error('Failed to stop the filesystem churn process')
}
}
describe('local Windows worktree removal recovery (live Git)', () => {
afterEach(async () => {
await Promise.all(
tempRoots
.splice(0)
.map((root) => rm(root, { recursive: true, force: true, maxRetries: 8, retryDelay: 100 }))
)
})
it.runIf(process.platform === 'win32')(
'finishes deletion when Git deregisters before a directory-not-empty failure',
async () => {
const root = await mkdtemp(path.join(tmpdir(), 'orca-worktree-enotempty-'))
tempRoots.push(root)
const repoPath = path.join(root, 'repo')
const worktreePath = path.join(root, 'worktree')
git(root, ['init', '--quiet', repoPath])
git(repoPath, ['config', 'user.email', 'test@example.com'])
git(repoPath, ['config', 'user.name', 'Test User'])
await writeFile(path.join(repoPath, '.gitignore'), 'churn/\n')
git(repoPath, ['add', '.gitignore'])
git(repoPath, ['commit', '--quiet', '-m', 'initial'])
git(repoPath, ['worktree', 'add', '--quiet', '-b', 'feature/churn', worktreePath])
const head = git(worktreePath, ['rev-parse', 'HEAD'])
// Why: a producer racing Git's recursive delete deterministically exposes
// the Windows ENOTEMPTY state reported by real workspace deletions.
const churn = spawn(
process.execPath,
[
'-e',
[
"const fs = require('node:fs')",
"const path = require('node:path')",
'const root = process.argv[1]',
'let i = 0',
'while (fs.existsSync(root)) {',
' try {',
" const dir = path.join(root, 'churn')",
' fs.mkdirSync(dir, { recursive: true })',
" fs.writeFileSync(path.join(dir, 'f' + (i++ % 64) + '.txt'), 'x')",
' } catch {}',
'}'
].join('\n'),
worktreePath
],
{ stdio: 'ignore' }
)
try {
await waitForPath(path.join(worktreePath, 'churn'))
const removal = spawnSync('git', ['worktree', 'remove', worktreePath], {
cwd: repoPath,
encoding: 'utf8'
})
expect(removal.status).not.toBe(0)
expect(git(repoPath, ['worktree', 'list', '--porcelain'])).not.toContain(worktreePath)
expect(existsSync(worktreePath)).toBe(true)
await stopProcess(churn)
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: removal.stderr
}),
force: false,
canonicalWorktreePath: worktreePath,
repoPath,
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature/churn', head },
deleteBranch: false,
closeWatcher: async () => {}
})
).resolves.toEqual({})
expect(existsSync(worktreePath)).toBe(false)
} finally {
await stopProcess(churn)
}
}
)
})
@@ -1,9 +1,12 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { gitExecFileAsyncMock, removeLocalWorktreePathMock } = vi.hoisted(() => ({
gitExecFileAsyncMock: vi.fn(),
removeLocalWorktreePathMock: vi.fn()
}))
const { gitExecFileAsyncMock, listWorktreesStrictMock, removeLocalWorktreePathMock } = vi.hoisted(
() => ({
gitExecFileAsyncMock: vi.fn(),
listWorktreesStrictMock: vi.fn(),
removeLocalWorktreePathMock: vi.fn()
})
)
vi.mock('./git/runner', () => ({
gitExecFileAsync: gitExecFileAsyncMock
@@ -13,7 +16,14 @@ vi.mock('./local-worktree-filesystem', () => ({
removeLocalWorktreePath: removeLocalWorktreePathMock
}))
import { recoverLocalWindowsLongPathWorktreeRemoval } from './local-worktree-removal-recovery'
vi.mock('./git/worktree', () => ({
listWorktreesStrict: listWorktreesStrictMock
}))
import {
recoverLocalWindowsWorktreeRemoval,
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval
} from './local-worktree-removal-recovery'
async function withPlatform<T>(platform: NodeJS.Platform, fn: () => Promise<T>): Promise<T> {
const original = process.platform
@@ -25,11 +35,13 @@ async function withPlatform<T>(platform: NodeJS.Platform, fn: () => Promise<T>):
}
}
describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
describe('recoverLocalWindowsWorktreeRemoval', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
listWorktreesStrictMock.mockReset()
removeLocalWorktreePathMock.mockReset()
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
listWorktreesStrictMock.mockResolvedValue([])
removeLocalWorktreePathMock.mockResolvedValue(undefined)
})
@@ -43,7 +55,7 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
stderr: "error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Permission denied"
})
const result = await recoverLocalWindowsLongPathWorktreeRemoval({
const result = await recoverLocalWindowsWorktreeRemoval({
error,
force: true,
canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd',
@@ -70,6 +82,90 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
})
})
it('finishes a clean non-force deletion after Git leaves a Windows directory behind', async () => {
await withPlatform('win32', async () => {
const error = Object.assign(new Error('git worktree remove failed'), {
stderr:
"error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Directory not empty"
})
const result = await recoverLocalWindowsWorktreeRemoval({
error,
force: false,
canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/delete-e2e-held-cwd', head: 'abc123' },
deleteBranch: true,
closeWatcher: vi.fn().mockResolvedValue(undefined)
})
expect(removeLocalWorktreePathMock).toHaveBeenCalledWith(
'C:/repo/worktree/delete-e2e-held-cwd',
{}
)
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], {
cwd: 'C:/repo'
})
expect(result).toEqual({
preservedBranch: {
branchName: 'delete-e2e-held-cwd',
head: 'abc123'
}
})
})
})
it('recovers localized Windows failures after Git already removed the registration', async () => {
await withPlatform('win32', async () => {
const result = await recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: "Fehler: 'C:/repo/worktree/feature' konnte nicht gelöscht werden"
}),
force: false,
canonicalWorktreePath: 'C:/repo/worktree/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: false,
closeWatcher: vi.fn().mockResolvedValue(undefined)
})
expect(removeLocalWorktreePathMock).toHaveBeenCalledWith('C:/repo/worktree/feature', {})
expect(result).toEqual({})
})
})
it('does not recover unrelated localized failures while Git still owns the row', async () => {
await withPlatform('win32', async () => {
listWorktreesStrictMock.mockResolvedValue([
{
path: 'C:/repo/worktree/feature',
head: 'abc123',
branch: 'refs/heads/feature',
isBare: false,
isMainWorktree: false
}
])
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: 'Fehler: unerwarteter Git-Zustand'
}),
force: false,
canonicalWorktreePath: 'C:/repo/worktree/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: false,
closeWatcher: vi.fn().mockResolvedValue(undefined)
})
).resolves.toBeUndefined()
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
})
})
it('does not recover partial filesystem deletion wording off Windows', async () => {
await withPlatform('linux', async () => {
const error = Object.assign(new Error('git worktree remove failed'), {
@@ -77,7 +173,7 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
})
await expect(
recoverLocalWindowsLongPathWorktreeRemoval({
recoverLocalWindowsWorktreeRemoval({
error,
force: true,
canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd',
@@ -91,4 +187,149 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
})
})
it('does not recover a locked registration after a filesystem removal failure', async () => {
await withPlatform('win32', async () => {
const registeredWorktree = {
branch: 'refs/heads/feature',
head: 'abc123',
locked: true,
lockReason: 'active agent'
}
const closeWatcher = vi.fn().mockResolvedValue(undefined)
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: 'error: failed to delete deep/file.txt: Filename too long'
}),
force: true,
canonicalWorktreePath: 'C:/workspaces/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree,
deleteBranch: true,
closeWatcher
})
).rejects.toThrow('Worktree is locked by Git. Lock reason: active agent')
expect(closeWatcher).not.toHaveBeenCalled()
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
expect(listWorktreesStrictMock).not.toHaveBeenCalled()
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
})
})
it.each([
"error: failed to delete 'C:/repo/worktree/feature': Directory not empty",
'error: failed to delete deep/file.txt: Filename too long'
])(
'does not recover a recognized Windows failure while Git still owns the row: %s',
async (stderr) => {
await withPlatform('win32', async () => {
listWorktreesStrictMock.mockResolvedValue([
{
path: 'C:/repo/worktree/feature',
head: 'abc123',
branch: 'refs/heads/feature',
isBare: false,
isMainWorktree: false
}
])
const closeWatcher = vi.fn().mockResolvedValue(undefined)
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), { stderr }),
force: true,
canonicalWorktreePath: 'C:/repo/worktree/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: { wslDistro: 'Ubuntu' },
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: true,
closeWatcher
})
).resolves.toBeUndefined()
expect(listWorktreesStrictMock).toHaveBeenCalledWith('C:/repo', { wslDistro: 'Ubuntu' })
expect(closeWatcher).not.toHaveBeenCalled()
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
})
}
)
it('does not recover when structural registration verification fails', async () => {
await withPlatform('win32', async () => {
listWorktreesStrictMock.mockRejectedValue(new Error('git list failed'))
const closeWatcher = vi.fn().mockResolvedValue(undefined)
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: "error: failed to delete 'C:/repo/worktree/feature': Directory not empty"
}),
force: true,
canonicalWorktreePath: 'C:/repo/worktree/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: true,
closeWatcher
})
).resolves.toBeUndefined()
expect(closeWatcher).not.toHaveBeenCalled()
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
})
})
})
describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
listWorktreesStrictMock.mockReset()
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
listWorktreesStrictMock.mockResolvedValue([])
})
it('does not override a locked missing registration', async () => {
await expect(
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
canonicalWorktreePath: 'C:/workspaces/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: {
branch: 'refs/heads/feature',
head: 'abc123',
locked: true,
lockReason: 'active agent'
},
deleteBranch: true
})
).rejects.toThrow('Worktree is locked by Git')
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
})
it('does not report success when prune leaves the registration behind', async () => {
listWorktreesStrictMock.mockResolvedValue([
{
path: 'C:/workspaces/feature',
head: 'abc123',
branch: 'refs/heads/feature',
isBare: false,
isMainWorktree: false
}
])
await expect(
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
canonicalWorktreePath: 'C:/workspaces/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: true
})
).rejects.toThrow('Git still has stale worktree registration')
})
})
+96 -51
View File
@@ -1,25 +1,23 @@
import type { GitWorktreeInfo, RemoveWorktreeResult } from '../shared/types'
import {
formatWorktreeRemovalError,
isWindowsLongPathWorktreeRemovalError
} from './ipc/worktree-logic'
import { assertWorktreeUnlockedForRemoval } from '../shared/worktree-removal'
import { areWorktreePathsEqual, formatWorktreeRemovalError } from './ipc/worktree-logic'
import { gitExecFileAsync } from './git/runner'
import type { GitWorktreeExecOptions } from './git/worktree'
import { listWorktreesStrict, type GitWorktreeExecOptions } from './git/worktree'
import { removeLocalWorktreePath } from './local-worktree-filesystem'
type LocalWindowsLongPathRecoveryArgs = {
type LocalWindowsRemovalRecoveryArgs = {
error: unknown
force: boolean
canonicalWorktreePath: string
repoPath: string
localWorktreeGitOptions: GitWorktreeExecOptions
registeredWorktree: Pick<GitWorktreeInfo, 'branch' | 'head'>
registeredWorktree: Pick<GitWorktreeInfo, 'branch' | 'head' | 'locked' | 'lockReason'>
deleteBranch: boolean
closeWatcher: (worktreePath: string) => Promise<void>
}
type StaleLocalWorktreeRegistrationArgs = Omit<
LocalWindowsLongPathRecoveryArgs,
LocalWindowsRemovalRecoveryArgs,
'error' | 'force' | 'closeWatcher'
>
@@ -38,73 +36,120 @@ function preservedBranchResult(
}
}
async function pruneRequiredGitWorktreeRegistration(
function staleRegistrationRecoveryError(
error: unknown,
canonicalWorktreePath: string,
force: boolean
): Error {
return new Error(
`${formatWorktreeRemovalError(
error,
canonicalWorktreePath,
force
)} The worktree directory was removed, but Git still has stale worktree registration. Retry deletion after resolving the Git registration error.`
)
}
async function verifyGitWorktreeRegistrationRemoved(
repoPath: string,
localWorktreeGitOptions: GitWorktreeExecOptions,
canonicalWorktreePath: string
canonicalWorktreePath: string,
force: boolean
): Promise<void> {
try {
await gitExecFileAsync(['worktree', 'prune'], {
cwd: repoPath,
...localWorktreeGitOptions
})
const remainingWorktrees = await listWorktreesStrict(repoPath, localWorktreeGitOptions)
if (
remainingWorktrees.some((worktree) =>
areWorktreePathsEqual(worktree.path, canonicalWorktreePath)
)
) {
throw new Error('Git still reports the worktree registration after cleanup.')
}
} catch (error) {
throw new Error(
`${formatWorktreeRemovalError(
error,
canonicalWorktreePath,
true
)} The worktree directory was removed, but Git still has stale worktree registration. Retry deletion after resolving the Git prune error.`
)
throw staleRegistrationRecoveryError(error, canonicalWorktreePath, force)
}
}
export async function recoverLocalWindowsLongPathWorktreeRemoval(
args: LocalWindowsLongPathRecoveryArgs
async function removeRequiredGitWorktreeRegistration(
args: StaleLocalWorktreeRegistrationArgs,
forceForError = true
): Promise<RemoveWorktreeResult> {
assertWorktreeUnlockedForRemoval(args.registeredWorktree)
let result: RemoveWorktreeResult | undefined
let removalError: unknown
try {
await gitExecFileAsync(['worktree', 'prune'], {
cwd: args.repoPath,
...args.localWorktreeGitOptions
})
result = preservedBranchResult(args.registeredWorktree, args.deleteBranch)
} catch (error) {
removalError = error
}
// Why: Git prune exits successfully while retaining locked registrations;
// a failed remove can also have detached the row before filesystem cleanup.
try {
await verifyGitWorktreeRegistrationRemoved(
args.repoPath,
args.localWorktreeGitOptions,
args.canonicalWorktreePath,
forceForError
)
} catch (verificationError) {
throw removalError
? staleRegistrationRecoveryError(removalError, args.canonicalWorktreePath, forceForError)
: verificationError
}
// Why: if Git detached the row before reporting its filesystem error, keep
// the branch rather than guessing whether the normal branch cleanup ran.
return result ?? preservedBranchResult(args.registeredWorktree, args.deleteBranch)
}
export async function recoverLocalWindowsWorktreeRemoval(
args: LocalWindowsRemovalRecoveryArgs
): Promise<RemoveWorktreeResult | undefined> {
if (!args.force || !isRecoverableWindowsFilesystemRemovalError(args.error)) {
// Why: recovery can recursively delete the remaining directory, so a Git
// lock must reject the attempt before classification or any side effects.
assertWorktreeUnlockedForRemoval(args.registeredWorktree)
if (!(await isRecoverableWindowsFilesystemRemovalFailure(args))) {
return undefined
}
// Why: watcher shutdown is best-effort, but Git registration must be pruned
// before callers clear Orca metadata or the branch remains locked.
// Why: this error means Git accepted removal and started deleting the path;
// finish that partial success with Windows retries, then verify Git metadata.
await args.closeWatcher(args.canonicalWorktreePath).catch(() => {})
try {
await removeLocalWorktreePath(args.canonicalWorktreePath, args.localWorktreeGitOptions)
} catch (error) {
throw new Error(formatWorktreeRemovalError(error, args.canonicalWorktreePath, true))
throw new Error(formatWorktreeRemovalError(error, args.canonicalWorktreePath, args.force))
}
await pruneRequiredGitWorktreeRegistration(
args.repoPath,
args.localWorktreeGitOptions,
args.canonicalWorktreePath
)
return preservedBranchResult(args.registeredWorktree, args.deleteBranch)
return removeRequiredGitWorktreeRegistration(args, args.force)
}
function isRecoverableWindowsFilesystemRemovalError(error: unknown): boolean {
if (isWindowsLongPathWorktreeRemovalError(error)) {
return true
async function isRecoverableWindowsFilesystemRemovalFailure(
args: LocalWindowsRemovalRecoveryArgs
): Promise<boolean> {
if (process.platform !== 'win32' || typeof args.error !== 'object' || args.error === null) {
return false
}
if (process.platform !== 'win32' || typeof error !== 'object' || error === null) {
try {
const worktrees = await listWorktreesStrict(args.repoPath, args.localWorktreeGitOptions)
// Why: error prose can be localized or ambiguous. Only a missing Git row
// proves removal started and makes recursive Windows cleanup safe.
return !worktrees.some((worktree) =>
areWorktreePathsEqual(worktree.path, args.canonicalWorktreePath)
)
} catch {
return false
}
const errorWithDetails = error as { message?: unknown; stderr?: unknown; stdout?: unknown }
const details = [errorWithDetails.stderr, errorWithDetails.stdout, errorWithDetails.message]
.filter((value): value is string => typeof value === 'string' && value.trim().length > 0)
.join('\n')
return /failed to delete .*(?:directory not empty|permission denied|access is denied|being used by another process)|(?:directory not empty|permission denied|access is denied|being used by another process).*failed to delete/i.test(
details
)
}
export async function pruneStaleLocalWorktreeRegistrationAfterFilesystemRemoval(
export async function removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval(
args: StaleLocalWorktreeRegistrationArgs
): Promise<RemoveWorktreeResult> {
await pruneRequiredGitWorktreeRegistration(
args.repoPath,
args.localWorktreeGitOptions,
args.canonicalWorktreePath
)
return preservedBranchResult(args.registeredWorktree, args.deleteBranch)
return removeRequiredGitWorktreeRegistration(args)
}
+131 -9
View File
@@ -74,6 +74,7 @@ import {
unregisterSshFilesystemProvider
} from '../providers/ssh-filesystem-dispatch'
import { registerSshGitProvider, unregisterSshGitProvider } from '../providers/ssh-git-dispatch'
import * as localWorktreeFilesystem from '../local-worktree-filesystem'
import {
DEFAULT_REPO_BADGE_COLOR,
FLOATING_TERMINAL_WORKTREE_ID,
@@ -89,6 +90,12 @@ import { TERMINAL_METHODS } from './rpc/methods/terminal'
const ORIGINAL_PLATFORM = process.platform
const ORIGINAL_PLATFORM_DESCRIPTOR = Object.getOwnPropertyDescriptor(process, 'platform')
const removeWorktreeLinkedPathsMock = vi.hoisted(() => vi.fn())
vi.mock('../ipc/worktree-symlinks', () => ({
createWorktreeLinkedPaths: vi.fn(),
removeWorktreeLinkedPaths: removeWorktreeLinkedPathsMock
}))
async function waitForMobileSessionTabsEvents(
events: RuntimeMobileSessionTabsResult[],
@@ -544,6 +551,7 @@ function resetRuntimeTestMocks(): void {
vi.mocked(assertWorktreeCleanForRemoval).mockReset()
vi.mocked(assertWorktreeCleanForRemoval).mockResolvedValue(undefined)
vi.mocked(removeWorktree).mockReset()
removeWorktreeLinkedPathsMock.mockReset()
vi.mocked(forceDeleteLocalBranchMock).mockReset()
vi.mocked(forceDeleteLocalBranchMock).mockResolvedValue(undefined)
sshGitProviders.clear()
@@ -4443,7 +4451,7 @@ describe('OrcaRuntimeService', () => {
const runtime = new OrcaRuntimeService(remoteStore as never)
try {
await runtime.removeManagedWorktree('path:/remote/feature', true)
await runtime.removeManagedWorktree('path:/remote/feature', true, false)
} finally {
unregisterSshGitProvider('ssh-1')
}
@@ -27101,6 +27109,10 @@ describe('OrcaRuntimeService', () => {
stderr: 'error: failed to delete deep/file.txt: Filename too long'
})
)
vi.mocked(listWorktreesStrict)
.mockResolvedValueOnce(MOCK_GIT_WORKTREES)
.mockResolvedValueOnce(MOCK_GIT_WORKTREES)
.mockResolvedValue([])
try {
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)
@@ -27122,7 +27134,7 @@ describe('OrcaRuntimeService', () => {
}
})
it('keeps runtime metadata when long-path recovery deletes the directory but prune fails', async () => {
it('refuses runtime Windows recovery while Git still reports the row and keeps metadata', async () => {
setPlatform('win32')
const removeWorktreeMeta = vi.fn()
const runtimeStore = {
@@ -27130,11 +27142,13 @@ describe('OrcaRuntimeService', () => {
removeWorktreeMeta
}
const runtime = new OrcaRuntimeService(runtimeStore as never)
const gitSpy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockRejectedValue(
Object.assign(new Error('git prune failed'), {
stderr: 'fatal: unable to lock worktree admin dir'
})
)
const gitSpy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockResolvedValue({
stdout: '',
stderr: ''
})
const removePathSpy = vi
.spyOn(localWorktreeFilesystem, 'removeLocalWorktreePath')
.mockResolvedValue(undefined)
vi.mocked(getEffectiveHooks).mockReturnValue(null)
vi.mocked(removeWorktree).mockRejectedValue(
Object.assign(new Error('git worktree remove failed'), {
@@ -27144,10 +27158,13 @@ describe('OrcaRuntimeService', () => {
try {
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)).rejects.toThrow(
'Git still has stale worktree registration'
`Failed to force delete worktree at ${TEST_WORKTREE_PATH}. error: failed to delete deep/file.txt: Filename too long`
)
expect(removePathSpy).not.toHaveBeenCalled()
expect(gitSpy).not.toHaveBeenCalledWith(['worktree', 'prune'], expect.anything())
expect(removeWorktreeMeta).not.toHaveBeenCalled()
} finally {
removePathSpy.mockRestore()
gitSpy.mockRestore()
}
})
@@ -27179,7 +27196,7 @@ describe('OrcaRuntimeService', () => {
stderr: ''
})
vi.mocked(listWorktrees).mockResolvedValue(registeredWorktrees)
vi.mocked(listWorktreesStrict).mockResolvedValue(registeredWorktrees)
vi.mocked(listWorktreesStrict).mockResolvedValueOnce(registeredWorktrees).mockResolvedValue([])
vi.mocked(getEffectiveHooks).mockReturnValue({
scripts: {
archive: 'pnpm worktree:archive'
@@ -27203,6 +27220,34 @@ describe('OrcaRuntimeService', () => {
}
})
it('preserves a locked missing runtime registration even with force', async () => {
setPlatform('win32')
const missingWorktreePath = 'C:\\workspace\\locked-already-removed'
const worktreeId = `${TEST_REPO_ID}::${missingWorktreePath}`
const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(worktreeId)
const runtime = new OrcaRuntimeService(runtimeStore as never)
const registeredWorktrees = [
{
path: missingWorktreePath,
head: 'abc',
branch: 'refs/heads/feature/foo',
isBare: false,
isMainWorktree: false,
locked: true,
lockReason: 'active agent session'
}
]
vi.mocked(listWorktreesStrict).mockResolvedValue(registeredWorktrees)
vi.mocked(removeWorktree).mockResolvedValue({})
await expect(runtime.removeManagedWorktree(worktreeId, true, false)).rejects.toThrow(
'Worktree is locked by Git. Lock reason: active agent session'
)
expect(removeWorktree).not.toHaveBeenCalled()
expect(removeWorktreeMeta).not.toHaveBeenCalled()
})
it('routes runtime worktree removal through the selected WSL project runtime', async () => {
setPlatform('win32')
const runtimeStore = {
@@ -27931,6 +27976,83 @@ describe('OrcaRuntimeService', () => {
expect(removeWorktree).not.toHaveBeenCalled()
})
it('fails locked dirty-force deletes before hooks, link cleanup, or PTY teardown', async () => {
const repo = { ...store.getRepos()[0], symlinkPaths: ['node_modules'] }
const runtimeStore = {
...store,
getRepos: () => [repo],
getRepo: () => repo
}
const runtime = new OrcaRuntimeService(runtimeStore as never)
const killSpy = vi.fn().mockReturnValue(true)
runtime.setPtyController({
write: () => true,
kill: (id) => killSpy(id),
getForegroundProcess: async () => null
})
syncSinglePty(runtime, 'pty-1')
vi.mocked(getEffectiveHooks).mockReturnValue({
scripts: { archive: 'pnpm worktree:archive' }
})
vi.mocked(listWorktreesStrict).mockResolvedValue([
{
...MOCK_GIT_WORKTREES[0],
locked: true,
lockReason: 'active agent session'
}
])
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, true)).rejects.toThrow(
`Failed to force delete worktree at ${TEST_WORKTREE_PATH}. Worktree is locked by Git.`
)
expect(assertWorktreeCleanForRemoval).not.toHaveBeenCalled()
expect(runHook).not.toHaveBeenCalled()
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
expect(killSpy).not.toHaveBeenCalled()
expect(removeWorktree).not.toHaveBeenCalled()
})
it('rechecks a runtime Git lock after the archive hook before teardown', async () => {
const repo = { ...store.getRepos()[0], symlinkPaths: ['node_modules'] }
const runtimeStore = {
...store,
getRepos: () => [repo],
getRepo: () => repo
}
const runtime = new OrcaRuntimeService(runtimeStore as never)
const killSpy = vi.fn().mockReturnValue(true)
runtime.setPtyController({
write: () => true,
kill: (id) => killSpy(id),
getForegroundProcess: async () => null
})
syncSinglePty(runtime, 'pty-1')
vi.mocked(getEffectiveHooks).mockReturnValue({
scripts: { archive: 'pnpm worktree:archive' }
})
vi.mocked(runHook).mockResolvedValue({ success: true, output: '' })
vi.mocked(listWorktreesStrict)
.mockResolvedValueOnce(MOCK_GIT_WORKTREES)
.mockResolvedValueOnce([
{
...MOCK_GIT_WORKTREES[0],
locked: true,
lockReason: 'locked during archive'
}
])
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, true)).rejects.toThrow(
'Worktree is locked by Git'
)
expect(runHook).toHaveBeenCalled()
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
expect(assertWorktreeCleanForRemoval).not.toHaveBeenCalled()
expect(killSpy).not.toHaveBeenCalled()
expect(removeWorktree).not.toHaveBeenCalled()
})
it('formats preflight subprocess failures and skips PTY teardown', async () => {
const runtime = new OrcaRuntimeService(store)
const killSpy = vi.fn().mockReturnValue(true)
+45 -14
View File
@@ -139,6 +139,7 @@ import type {
WorkspaceSessionState,
DirEntry
} from '../../shared/types'
import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal'
import {
getRepoExecutionHostId,
parseExecutionHostId,
@@ -469,8 +470,8 @@ import {
toLocalWorktreeRuntimePath
} from '../local-worktree-filesystem'
import {
pruneStaleLocalWorktreeRegistrationAfterFilesystemRemoval,
recoverLocalWindowsLongPathWorktreeRemoval
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval,
recoverLocalWindowsWorktreeRemoval
} from '../local-worktree-removal-recovery'
import {
connect as connectLinear,
@@ -16554,8 +16555,16 @@ export class OrcaRuntimeService {
const canonicalWorktreePath = registeredWorktree.path
const deleteBranch = removedMeta?.preserveBranchOnDelete !== true
// Why: a Git lock must block before archive hooks or linked-path cleanup
// mutate the workspace; dirty-file force is a separate permission.
try {
assertWorktreeUnlockedForRemoval(registeredWorktree)
} catch (error) {
throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force))
}
// Why: a prior forced Windows recovery can delete the directory but leave
// Git's stale registration; retry by pruning instead of removing a missing path.
// Git's stale registration; recover and verify it before clearing metadata.
if (
!repo.connectionId &&
force === true &&
@@ -16564,7 +16573,7 @@ export class OrcaRuntimeService {
removedMeta &&
(await isRuntimeWorktreePathMissing(repo, canonicalWorktreePath, localWorktreeGitOptions))
) {
const removalResult = await pruneStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
canonicalWorktreePath,
repoPath: repo.path,
localWorktreeGitOptions,
@@ -16592,9 +16601,10 @@ export class OrcaRuntimeService {
return removalResult ?? {}
}
if (repo.connectionId) {
const rawRemovalResult = await (deleteBranch
? provider!.removeWorktree(canonicalWorktreePath, force)
: provider!.removeWorktree(canonicalWorktreePath, force, { deleteBranch }))
const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {}
const rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0
? provider!.removeWorktree(canonicalWorktreePath, force, remoteRemoveOptions)
: provider!.removeWorktree(canonicalWorktreePath, force))
const removalResult = this.preserveBranchHeadFallback(
rawRemovalResult,
registeredWorktree.head
@@ -16639,6 +16649,27 @@ export class OrcaRuntimeService {
console.warn(`[hooks] ${warning}`)
}
const refreshedWorktrees = hasLocalWorktreeGitOptions
? await listWorktreesStrict(repo.path, localWorktreeGitOptions)
: await listWorktreesStrict(repo.path)
const refreshedRegisteredWorktree = findRegisteredDeletableWorktree(
repo.path,
canonicalWorktreePath,
refreshedWorktrees
)
if (!refreshedRegisteredWorktree) {
throw new Error(
`Worktree registration changed during deletion: ${canonicalWorktreePath}. Retry deletion.`
)
}
try {
// Why: an archive hook can race another Git client that locks the row;
// recheck before linked-path, watcher, or terminal teardown side effects.
assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree)
} catch (error) {
throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force))
}
let shouldTearDownPtys = true
if (repo.symlinkPaths && repo.symlinkPaths.length > 0) {
await removeWorktreeLinkedPaths(canonicalWorktreePath, repo.symlinkPaths)
@@ -16693,23 +16724,23 @@ export class OrcaRuntimeService {
...(!deleteBranch ? { deleteBranch } : {}),
// Why: removal already validated the Git row under the selected
// project runtime; keep branch cleanup on that same canonical row.
knownRemovedWorktree: registeredWorktree,
knownRemovedWorktree: refreshedRegisteredWorktree,
...localWorktreeGitOptions
}
removalResult = this.preserveBranchHeadFallback(
await removeWorktree(repo.path, canonicalWorktreePath, force, removeOptions),
registeredWorktree.head
refreshedRegisteredWorktree.head
)
} catch (error) {
// Why: Git for Windows can fail long-path directory deletion after
// Orca has already validated the target and explicit force delete.
const recoveredRemovalResult = await recoverLocalWindowsLongPathWorktreeRemoval({
// Why: Git for Windows can deregister a clean worktree before its
// recursive filesystem deletion fails transiently.
const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({
error,
force,
canonicalWorktreePath,
repoPath: repo.path,
localWorktreeGitOptions,
registeredWorktree,
registeredWorktree: refreshedRegisteredWorktree,
deleteBranch,
closeWatcher: (worktreePath) =>
closeLocalWatcherForWorktreePath(worktreePath).catch((err) => {
@@ -16778,7 +16809,7 @@ export class OrcaRuntimeService {
this.rememberPreservedBranchCleanupTarget(
removalTarget.id,
removalResult,
registeredWorktree.head,
refreshedRegisteredWorktree.head,
removedPushTarget
)
this.clearOptimisticReconcileToken(removalTarget.id)
@@ -67,6 +67,25 @@ describe('worktree RPC methods', () => {
})
})
it('routes dirty-file force to the runtime server', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
removeManagedWorktree: vi.fn().mockResolvedValue({})
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS })
const response = await dispatcher.dispatch(
makeRequest('worktree.rm', {
worktree: 'id:wt-1',
force: true,
runHooks: false
})
)
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-1', true, false)
expect(response).toMatchObject({ ok: true, result: { removed: true } })
})
it('routes create options to the runtime server', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
+44 -1
View File
@@ -1,6 +1,49 @@
import { describe, expect, it } from 'vitest'
import { parseStatusOutput } from './git-status-output-parser'
import { isUnsupportedWorktreeListZError } from './git-handler-utils'
import { isUnsupportedWorktreeListZError, parseWorktreeList } from './git-handler-utils'
describe('parseWorktreeList', () => {
it('preserves SSH worktree lock metadata from porcelain output', () => {
expect(
parseWorktreeList(
'worktree /repo\nHEAD abc\nbranch refs/heads/main\n\nworktree /locked\nHEAD def\nbranch refs/heads/feature\nlocked remote session\n'
)[1]
).toMatchObject({
path: '/locked',
locked: true,
lockReason: 'remote session'
})
})
it('decodes C-quoted lock reasons from legacy line porcelain output', () => {
const output =
'worktree /repo\nHEAD abc\nbranch refs/heads/main\n\nworktree /locked\nHEAD def\nbranch refs/heads/feature\nlocked "first line\\nsecond line \\303\\251"\n'
expect(parseWorktreeList(output)[1]).toMatchObject({
locked: true,
lockReason: 'first line\nsecond line é'
})
})
it('keeps NUL-delimited lock reasons raw', () => {
const output = [
'worktree /repo',
'HEAD abc',
'branch refs/heads/main',
'',
'worktree /locked',
'HEAD def',
'branch refs/heads/feature',
'locked "literal\\nquote"',
''
].join('\0')
expect(parseWorktreeList(output, { nulDelimited: true })[1]).toMatchObject({
locked: true,
lockReason: '"literal\\nquote"'
})
})
})
describe('isUnsupportedWorktreeListZError', () => {
it('detects an unknown-switch usage error from stderr when the exit code is absent', () => {
+9
View File
@@ -7,6 +7,7 @@
*/
import { existsSync } from 'node:fs'
import * as path from 'node:path'
import { decodeGitCQuotedPath } from '../shared/git-cquoted-path'
import { isBinaryBuffer } from '../shared/binary-buffer'
import type { GitLineStats } from '../shared/git-uncommitted-line-stats'
export { isUnsupportedWorktreeListZError } from '../shared/git-worktree-command-capabilities'
@@ -149,6 +150,8 @@ export function parseWorktreeList(
let head = ''
let branch = ''
let isBare = false
let locked = false
let lockReason = ''
for (const line of lines) {
if (line.startsWith('worktree ')) {
@@ -159,6 +162,10 @@ export function parseWorktreeList(
branch = line.slice('branch '.length)
} else if (line === 'bare') {
isBare = true
} else if (line === 'locked' || line.startsWith('locked ')) {
locked = true
const rawReason = line.slice('locked'.length).trim()
lockReason = options.nulDelimited ? rawReason : decodeGitCQuotedPath(rawReason)
}
}
@@ -168,6 +175,8 @@ export function parseWorktreeList(
head,
branch,
isBare,
...(locked ? { locked: true } : {}),
...(lockReason ? { lockReason } : {}),
isMainWorktree: worktrees.length === 0
})
}
+5 -1
View File
@@ -6,6 +6,8 @@ export type RelayWorktreeInfo = {
path: string
branch?: string
head?: string
locked?: boolean
lockReason?: string
}
export async function readRelayWorktreeList(
@@ -33,7 +35,9 @@ function normalizeRelayWorktrees(worktrees: Record<string, unknown>[]): RelayWor
.map((worktree) => ({
path: typeof worktree.path === 'string' ? worktree.path : '',
head: typeof worktree.head === 'string' ? worktree.head : undefined,
branch: typeof worktree.branch === 'string' ? worktree.branch : undefined
branch: typeof worktree.branch === 'string' ? worktree.branch : undefined,
locked: worktree.locked === true ? true : undefined,
lockReason: typeof worktree.lockReason === 'string' ? worktree.lockReason : undefined
}))
.filter((worktree) => worktree.path.length > 0)
}
@@ -136,6 +136,32 @@ describe('addWorktreeOp', () => {
})
describe('removeWorktreeOp', () => {
it('rejects a locked SSH worktree before invoking remove', async () => {
const git = vi.fn<GitExec>(async (args) => {
if (args[0] === 'rev-parse') {
return { stdout: '/repo/.git\n', stderr: '' }
}
if (args[0] === 'worktree' && args[1] === 'list') {
return {
stdout: `${worktreeList(
{ path: '/repo', branch: 'main' },
{ path: '/repo-feature', branch: 'feature/test' }
)}\nlocked remote agent`,
stderr: ''
}
}
return { stdout: '', stderr: '' }
})
await expect(
removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' })
).rejects.toThrow('Worktree is locked by Git. Lock reason: remote agent.')
expect(git).not.toHaveBeenCalledWith(
['worktree', 'remove', '/repo-feature'],
expect.any(String)
)
})
it('deletes the now-unused branch after removing an SSH worktree', async () => {
const calls: string[] = []
let listCount = 0
@@ -234,10 +260,41 @@ describe('removeWorktreeOp', () => {
forceBranchDelete: true
})
expect(git).toHaveBeenCalledWith(
['worktree', 'remove', '--force', '/repo-feature'],
expect.any(String)
)
expect(git).toHaveBeenCalledWith(['branch', '-D', '--', 'feature/test'], expect.any(String))
expect(git).not.toHaveBeenCalledWith(['branch', '-d', '--', 'feature/test'], expect.any(String))
})
it('does not let force override a locked SSH worktree', async () => {
const git = vi.fn<GitExec>(async (args) => {
if (args[0] === 'rev-parse') {
return { stdout: '/repo/.git\n', stderr: '' }
}
if (args[0] === 'worktree' && args[1] === 'list') {
return {
stdout: `${worktreeList(
{ path: '/repo', branch: 'main' },
{ path: '/repo-feature', branch: 'feature/test' }
)}\nlocked remote agent`,
stderr: ''
}
}
return { stdout: '', stderr: '' }
})
await expect(
removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature', force: true })
).rejects.toThrow('Worktree is locked by Git. Lock reason: remote agent')
expect(git).not.toHaveBeenCalledWith(
['worktree', 'remove', '--force', '/repo-feature'],
expect.any(String)
)
})
it('skips branch deletion entirely when deleteBranch is false', async () => {
const calls: string[] = []
const git = vi.fn<GitExec>(async (args, cwd) => {
+3
View File
@@ -1,5 +1,6 @@
import * as path from 'node:path'
import type { RemoveWorktreeResult } from '../shared/types'
import { assertWorktreeUnlockedForRemoval } from '../shared/worktree-removal'
import { deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure } from './git-handler-branch-cleanup'
import type { GitExec } from './git-handler-ops'
import type { GitCapabilityCache } from '../shared/git-capability-cache'
@@ -148,6 +149,8 @@ export async function removeWorktreeOp(
const branchName = normalizeLocalBranchRef(removedWorktree?.branch ?? '')
const branchHead = removedWorktree?.head ?? ''
assertWorktreeUnlockedForRemoval(removedWorktree)
const args = ['worktree', 'remove']
if (force) {
args.push('--force')
@@ -100,6 +100,7 @@ function showDeleteFailureToast(): void {
'branch has changes'
),
canForceDelete: true,
forceDeleteReason: 'dirty',
onViewChanges: () =>
toast.message(
translate(
@@ -19,7 +19,16 @@ const mocks = vi.hoisted(() => {
settings: null,
gitStatusByWorktree: {} as Record<string, { path: string; status: 'modified' }[]>,
setGitStatus: vi.fn(),
deleteStateByWorktreeId: {}
deleteStateByWorktreeId: {} as Record<
string,
{
isDeleting: boolean
error: string | null
canForceDelete: boolean
forceDeleteReason: 'dirty' | 'orphan-directory' | 'missing-registration' | null
lockReason?: string | null
}
>
}
return { state, buttonProps: [] as Record<string, unknown>[] }
})
@@ -74,6 +83,10 @@ vi.mock('./delete-worktree-flow', () => ({
runWorktreeDeletesInParallel: vi.fn()
}))
vi.mock('./active-worktree-focus-after-delete', () => ({
prepareActiveWorktreeFocusAfterDelete: () => vi.fn()
}))
import { runWorktreeDeletesInParallel } from './delete-worktree-flow'
function makeWorktree(id: string, path: string): Worktree {
@@ -249,6 +262,32 @@ describe('DeleteWorktreeDialog lineage copy', () => {
expect(markup).not.toContain('Also delete local branch')
})
it('shows locked and known dirty details without offering a lock override', async () => {
const workspace = makeWorktree('Locked workspace', '/workspaces/locked')
mocks.state.modalData = { worktreeId: workspace.id }
mocks.state.allWorktrees.mockReturnValue([workspace])
mocks.state.gitStatusByWorktree = {
[workspace.id]: [{ path: 'src/file.ts', status: 'modified' }]
}
mocks.state.deleteStateByWorktreeId = {
[workspace.id]: {
isDeleting: false,
error: 'Worktree is locked by Git. Lock reason: active agent session.',
canForceDelete: false,
forceDeleteReason: null,
lockReason: 'active agent session'
}
}
const { default: DeleteWorktreeDialog } = await import('./DeleteWorktreeDialog')
const markup = renderToStaticMarkup(<DeleteWorktreeDialog />)
expect(markup).toContain('Worktree is locked by Git. Lock reason: active agent session.')
expect(markup).not.toContain('Force Delete')
expect(markup).toContain('1 uncommitted or untracked change')
expect(mocks.state.removeWorktree).not.toHaveBeenCalled()
})
it('notifies the dialog caller after a toast force delete succeeds', async () => {
const workspace = makeWorktree('Workspace', '/workspaces/workspace')
const onDeleted = vi.fn()
@@ -21,6 +21,7 @@ import { DeleteWorktreeDialogDescription } from './DeleteWorktreeDialogDescripti
import { DeleteWorktreeTargetPreview } from './DeleteWorktreeTargetPreview'
import { DeleteWorktreeWarningPanels } from './DeleteWorktreeWarningPanels'
import { persistDeleteWorktreeConfirmSkipPreference } from './delete-worktree-preference-toast'
import { getDeleteWorktreeDirtyChangeCounts } from './delete-worktree-dirty-change-counts'
import {
countFolderWorkspaceDeletes,
getDeleteWorktreeDialogCopy,
@@ -137,19 +138,12 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
[canDeleteAllLineage, lineageDelete.deleteAllTargets, worktrees]
)
const dirtyChangeCountsByWorktreeId = useMemo(() => {
const result = new Map<string, number>()
for (const item of deleteTargets) {
if (item.isMainWorktree || getIsFolderWorkspaceDelete(repoMap, item)) {
continue
}
const statusEntries = gitStatusByWorktree[item.id]
if ((statusEntries?.length ?? 0) > 0) {
result.set(item.id, statusEntries?.length ?? 0)
} else if (deleteStateByWorktreeId[item.id]?.canForceDelete) {
result.set(item.id, 0)
}
}
return result
return getDeleteWorktreeDirtyChangeCounts({
deleteTargets,
deleteStateByWorktreeId,
gitStatusByWorktree,
repoMap
})
}, [deleteStateByWorktreeId, deleteTargets, gitStatusByWorktree, repoMap])
if (!isOpen && dontAskAgain) {
@@ -0,0 +1,32 @@
import type { Repo, Worktree } from '../../../../shared/types'
import type { WorktreeDeleteState } from '../../store/slices/worktree-helpers'
import { isFolderWorkspaceDelete } from './delete-worktree-dialog-copy'
export function getDeleteWorktreeDirtyChangeCounts({
deleteTargets,
deleteStateByWorktreeId,
gitStatusByWorktree,
repoMap
}: {
deleteTargets: readonly Worktree[]
deleteStateByWorktreeId: Record<string, WorktreeDeleteState | undefined>
gitStatusByWorktree: Record<string, readonly unknown[] | undefined>
repoMap: ReadonlyMap<string, Repo>
}): Map<string, number> {
const result = new Map<string, number>()
for (const item of deleteTargets) {
if (item.isMainWorktree || isFolderWorkspaceDelete(repoMap, item)) {
continue
}
const forceDeleteReason = deleteStateByWorktreeId[item.id]?.forceDeleteReason
const changeCount = gitStatusByWorktree[item.id]?.length
if ((changeCount ?? 0) > 0) {
result.set(item.id, changeCount ?? 0)
} else if (forceDeleteReason === 'dirty') {
// Why: Git proved the worktree dirty even when renderer status has not
// loaded; keep the warning visible without inventing a file count.
result.set(item.id, 0)
}
}
return result
}
@@ -55,6 +55,7 @@ describe('showDeleteWorktreeFailureToast', () => {
showDeleteWorktreeFailureToast({
error: 'branch has changes',
canForceDelete: true,
forceDeleteReason: 'dirty',
onViewChanges,
onForceDelete,
worktreeId: 'wt-1',
@@ -93,6 +94,7 @@ describe('showDeleteWorktreeFailureToast', () => {
showDeleteWorktreeFailureToast({
error: 'permission denied',
canForceDelete: false,
forceDeleteReason: null,
onViewChanges,
onForceDelete: vi.fn(),
worktreeId: 'wt-2',
@@ -116,4 +118,45 @@ describe('showDeleteWorktreeFailureToast', () => {
expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-2')
expect(onViewChanges).toHaveBeenCalled()
})
it('offers neither force delete nor View for a locked workspace', () => {
const onViewChanges = vi.fn()
showDeleteWorktreeFailureToast({
error: 'Worktree is locked by Git.',
canForceDelete: false,
forceDeleteReason: null,
onViewChanges,
onForceDelete: vi.fn(),
worktreeId: 'wt-locked',
worktreeName: 'feature/locked'
})
const body = renderToastBody('info')
expect(body.textContent).toContain('This workspace is locked by Git.')
expect(body.textContent).toContain(
'Run git worktree unlock <worktree-path> from its repository, then retry deletion'
)
expect(body.textContent).not.toContain('Force Delete')
expect(body.textContent).not.toContain('View')
expect(onViewChanges).not.toHaveBeenCalled()
})
it('keeps View for a locked workspace when changed files are known', () => {
showDeleteWorktreeFailureToast({
error: 'Worktree is locked by Git.',
canForceDelete: false,
forceDeleteReason: null,
lockReason: 'active agent session',
hasKnownChanges: true,
onViewChanges: vi.fn(),
onForceDelete: vi.fn(),
worktreeId: 'wt-locked-dirty',
worktreeName: 'feature/locked-dirty'
})
const body = renderToastBody('info')
expect(body.textContent).toContain('Git reported: active agent session')
expect(body.textContent).toContain('View')
})
})
@@ -2,10 +2,17 @@ import { toast } from 'sonner'
import { Button } from '../ui/button'
import { getDeleteWorktreeToastCopy } from './delete-worktree-toast'
import { translate } from '@/i18n/i18n'
import {
isLockedWorktreeRemovalError,
type WorktreeForceDeleteReason
} from '../../../../shared/worktree-removal'
type DeleteWorktreeFailureToastOptions = {
error: string
canForceDelete: boolean
forceDeleteReason: WorktreeForceDeleteReason | null
lockReason?: string | null
hasKnownChanges?: boolean
onViewChanges: () => void
onForceDelete: () => void
worktreeId: string
@@ -19,12 +26,14 @@ function deleteWorktreeFailureToastId(worktreeId: string): string {
function DeleteWorktreeFailureToastBody({
description,
canForceDelete,
showViewChanges,
onViewChanges,
onForceDelete,
toastId
}: {
description?: string
canForceDelete: boolean
showViewChanges: boolean
onViewChanges: () => void
onForceDelete: () => void
toastId: string
@@ -44,9 +53,11 @@ function DeleteWorktreeFailureToastBody({
<p className="text-sm leading-5 text-popover-foreground/80">{description}</p>
) : null}
<div className="flex flex-wrap justify-end gap-2">
<Button type="button" variant="outline" size="sm" onClick={viewChanges}>
{translate('auto.components.sidebar.delete.worktree.flow.7488ed8711', 'View')}
</Button>
{showViewChanges ? (
<Button type="button" variant="outline" size="sm" onClick={viewChanges}>
{translate('auto.components.sidebar.delete.worktree.flow.7488ed8711', 'View')}
</Button>
) : null}
{canForceDelete ? (
<Button type="button" variant="destructive" size="sm" onClick={forceDelete}>
{translate('auto.components.sidebar.delete.worktree.flow.2b20ce87b3', 'Force Delete')}
@@ -60,12 +71,20 @@ function DeleteWorktreeFailureToastBody({
export function showDeleteWorktreeFailureToast({
error,
canForceDelete,
forceDeleteReason,
lockReason,
hasKnownChanges,
onViewChanges,
onForceDelete,
worktreeId,
worktreeName
}: DeleteWorktreeFailureToastOptions): void {
const toastCopy = getDeleteWorktreeToastCopy(worktreeName, canForceDelete, error)
const toastCopy = getDeleteWorktreeToastCopy(
worktreeName,
forceDeleteReason,
error,
lockReason ?? null
)
const showToast = toastCopy.isDestructive ? toast.error : toast.info
const id = deleteWorktreeFailureToastId(worktreeId)
@@ -77,6 +96,7 @@ export function showDeleteWorktreeFailureToast({
<DeleteWorktreeFailureToastBody
description={toastCopy.description}
canForceDelete={canForceDelete}
showViewChanges={!isLockedWorktreeRemovalError(error) || hasKnownChanges === true}
onViewChanges={onViewChanges}
onForceDelete={onForceDelete}
toastId={id}
@@ -31,9 +31,16 @@ const mocks = vi.hoisted(() => {
}),
openModal: vi.fn(),
removeWorktree: vi.fn().mockResolvedValue({ ok: true }),
gitStatusByWorktree: {} as Record<string, unknown[]>,
deleteStateByWorktreeId: {} as Record<
string,
{ isDeleting?: boolean; error?: string | null; canForceDelete?: boolean }
{
isDeleting?: boolean
error?: string | null
canForceDelete?: boolean
forceDeleteReason?: 'dirty' | null
lockReason?: string | null
}
>
}
return { state }
@@ -101,6 +108,7 @@ describe('runWorktreeBatchDelete', () => {
mocks.state.openModal.mockClear()
mocks.state.removeWorktree.mockClear().mockResolvedValue({ ok: true })
mocks.state.deleteStateByWorktreeId = {}
mocks.state.gitStatusByWorktree = {}
mocks.state.worktreeLineageById = {}
mocks.state.repos = []
vi.mocked(toast.error).mockClear()
@@ -174,7 +182,8 @@ describe('runWorktreeBatchDelete', () => {
mocks.state.deleteStateByWorktreeId[worktreeId] = {
isDeleting: false,
error: 'changed files',
canForceDelete: true
canForceDelete: true,
forceDeleteReason: 'dirty'
}
return { ok: false, error: 'changed files' }
})
@@ -194,6 +203,34 @@ describe('runWorktreeBatchDelete', () => {
})
})
it('does not offer force delete for a locked worktree', async () => {
mocks.state.settings = { skipDeleteWorktreeConfirm: true }
mocks.state.removeWorktree
.mockImplementationOnce(async (worktreeId: string) => {
mocks.state.deleteStateByWorktreeId[worktreeId] = {
isDeleting: false,
error: 'Worktree is locked by Git.',
canForceDelete: false,
forceDeleteReason: null,
lockReason: 'active agent session'
}
return { ok: false, error: 'Worktree is locked by Git.' }
})
.mockResolvedValueOnce({ ok: true })
setWorktrees([{ id: 'wt-1', displayName: 'one' }])
expect(runWorktreeBatchDelete(['wt-1'])).toBe(true)
await vi.waitFor(() => expect(showDeleteWorktreeFailureToast).toHaveBeenCalled())
const toastOptions = vi.mocked(showDeleteWorktreeFailureToast).mock.calls[0]?.[0]
expect(toastOptions).toMatchObject({
canForceDelete: false,
forceDeleteReason: null,
lockReason: 'active agent session'
})
expect(mocks.state.removeWorktree).toHaveBeenCalledTimes(1)
})
it('keeps parent worktree deletes behind confirmation even when confirmation is skipped', () => {
mocks.state.settings = { skipDeleteWorktreeConfirm: true }
setWorktrees([
@@ -148,18 +148,22 @@ export function runWorktreeDeleteWithToast(
}
const state = useAppStore.getState().deleteStateByWorktreeId[worktreeId]
const canForceDelete = state?.canForceDelete ?? false
const hasKnownChanges =
(useAppStore.getState().gitStatusByWorktree[worktreeId]?.length ?? 0) > 0
showDeleteWorktreeFailureToast({
error: result.error,
canForceDelete,
forceDeleteReason: state?.forceDeleteReason ?? null,
lockReason: state?.lockReason ?? null,
hasKnownChanges,
onViewChanges: () => viewWorktreeDiff(worktreeId),
onForceDelete: () => {
// Why: recapture at click time — the user may have navigated away
// while the failed-delete toast was open, so focus only hands off
// when this is still the workspace they are viewing.
const commitForceFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId)
useAppStore
.getState()
.removeWorktree(worktreeId, true)
const forceRemoval = useAppStore.getState().removeWorktree(worktreeId, true)
forceRemoval
.then((forceResult) => {
if (!forceResult.ok) {
toast.error(
@@ -3,7 +3,7 @@ import { getDeleteWorktreeToastCopy } from './delete-worktree-toast'
describe('getDeleteWorktreeToastCopy', () => {
it('uses direct guidance when force delete is available', () => {
expect(getDeleteWorktreeToastCopy('feature/foo', true, 'branch has changes')).toEqual({
expect(getDeleteWorktreeToastCopy('feature/foo', 'dirty', 'branch has changes')).toEqual({
title: 'Failed to delete workspace feature/foo',
description: 'It has changed files. Use Force Delete to delete it anyway.',
isDestructive: false
@@ -14,7 +14,7 @@ describe('getDeleteWorktreeToastCopy', () => {
expect(
getDeleteWorktreeToastCopy(
'feature/foo',
true,
'orphan-directory',
'Worktree is no longer registered with Git but its directory remains.'
)
).toEqual({
@@ -29,7 +29,7 @@ describe('getDeleteWorktreeToastCopy', () => {
expect(
getDeleteWorktreeToastCopy(
'feature/foo',
true,
'missing-registration',
'Worktree is no longer registered with Git and its directory is already gone.'
)
).toEqual({
@@ -40,10 +40,35 @@ describe('getDeleteWorktreeToastCopy', () => {
})
it('preserves the raw error when force delete is unavailable', () => {
expect(getDeleteWorktreeToastCopy('feature/foo', false, 'permission denied')).toEqual({
expect(getDeleteWorktreeToastCopy('feature/foo', null, 'permission denied')).toEqual({
title: 'Failed to delete workspace feature/foo',
description: 'permission denied',
isDestructive: true
})
})
it('uses lock-specific guidance', () => {
expect(getDeleteWorktreeToastCopy('feature/foo', null, 'Worktree is locked by Git.')).toEqual({
title: 'Failed to delete workspace feature/foo',
description:
'This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.',
isDestructive: false
})
})
it('includes the structured Git lock reason in localized recovery copy', () => {
expect(
getDeleteWorktreeToastCopy(
'feature/foo',
null,
'Worktree is locked by Git. Lock reason: active agent session.',
'active agent session'
)
).toEqual({
title: 'Failed to delete workspace feature/foo',
description:
'This workspace is locked by Git. Git reported: active agent session. Run git worktree unlock <worktree-path> from its repository, then retry deletion.',
isDestructive: false
})
})
})
@@ -1,4 +1,8 @@
import { translate } from '@/i18n/i18n'
import {
isLockedWorktreeRemovalError,
type WorktreeForceDeleteReason
} from '../../../../shared/worktree-removal'
export type DeleteWorktreeToastCopy = {
title: string
description?: string
@@ -7,11 +11,33 @@ export type DeleteWorktreeToastCopy = {
export function getDeleteWorktreeToastCopy(
worktreeName: string,
canForceDelete: boolean,
error: string
forceDeleteReason: WorktreeForceDeleteReason | null,
error: string,
lockReason: string | null = null
): DeleteWorktreeToastCopy {
if (canForceDelete) {
if (error.includes('Worktree is no longer registered with Git but its directory remains.')) {
if (isLockedWorktreeRemovalError(error)) {
return {
title: translate(
'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5',
'Failed to delete workspace {{value0}}',
{ value0: worktreeName }
),
description: lockReason
? translate(
'auto.components.sidebar.delete.worktree.toast.lockedReason',
'This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion.',
{ value0: lockReason }
)
: translate(
'auto.components.sidebar.delete.worktree.toast.locked',
'This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.'
),
isDestructive: false
}
}
if (forceDeleteReason) {
if (forceDeleteReason === 'orphan-directory') {
return {
title: translate(
'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5',
@@ -25,9 +51,7 @@ export function getDeleteWorktreeToastCopy(
isDestructive: false
}
}
if (
error.includes('Worktree is no longer registered with Git and its directory is already gone.')
) {
if (forceDeleteReason === 'missing-registration') {
return {
title: translate(
'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5',
@@ -83,6 +83,7 @@ import {
type WorkspaceSpaceSortKey
} from './workspace-space-presentation'
import { translate } from '@/i18n/i18n'
import type { WorktreeForceDeleteReason } from '../../../../shared/worktree-removal'
const TREEMAP_FILLS = [
'color-mix(in srgb, var(--chart-2) 34%, var(--card))',
@@ -97,6 +98,7 @@ type WorkspaceSpaceDeleteState = {
isDeleting: boolean
error: string | null
canForceDelete: boolean
forceDeleteReason: WorktreeForceDeleteReason | null
}
type WorkspaceGitRefreshState = {
@@ -1,5 +1,6 @@
import type { ParsedAgentStatusPayload } from '../../../../shared/agent-status-types'
import type { GlobalSettings } from '../../../../shared/types'
import type { RecognizedAgentProcess } from '../../../../shared/agent-process-recognition'
import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection'
export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & {
@@ -9,6 +10,7 @@ export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & {
export type AgentCompletionDispatchMeta = {
source: 'hook' | 'title' | 'process-exit'
quietedHookDone: boolean
terminalIdleConfirmed?: boolean
agentStatus?: AgentCompletionStatusSnapshot
}
@@ -27,6 +29,12 @@ export type AgentCompletionCoordinatorOptions = {
) => Promise<RuntimeTerminalProcessInspection>
dispatchCompletion: (title: string, meta?: AgentCompletionDispatchMeta) => void
dispatchAttention?: (title: string, meta: AgentAttentionDispatchMeta) => void
dispatchHookLifecycle?: (payload: AgentCompletionStatusSnapshot) => void
shouldSuppressProcessReplacementCompletion?: (
exited: RecognizedAgentProcess,
replacement: RecognizedAgentProcess
) => boolean
shouldSuppressConfirmedProcessExitCompletion?: (exited: RecognizedAgentProcess) => boolean
isLive: () => boolean
shouldPollProcessCadence?: () => boolean
// Why: on hosts where one inspection forks a whole-process-table scan (local
@@ -204,7 +204,11 @@ describe('agent completion coordinator', () => {
// Second idle sample confirms the exit ~2 hidden polls (~6s) after it happened.
await vi.advanceTimersByTimeAsync(3_000)
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
expect(dispatchCompletion).toHaveBeenCalledWith('codex')
expect(dispatchCompletion).toHaveBeenCalledWith('codex', {
source: 'process-exit',
quietedHookDone: false,
terminalIdleConfirmed: true
})
})
it('clears process evidence after agent exit so later non-agent spinner titles do not notify', async () => {
@@ -272,7 +276,90 @@ describe('agent completion coordinator', () => {
await flushAsyncTicks()
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
expect(dispatchCompletion).toHaveBeenCalledWith('codex')
expect(dispatchCompletion).toHaveBeenCalledWith('codex', {
source: 'process-exit',
quietedHookDone: false,
terminalIdleConfirmed: true
})
})
it('does not mark an agent-to-agent process replacement as terminal idle', async () => {
let foregroundProcess = 'codex'
const dispatchCompletion = vi.fn()
const coordinator = createAgentCompletionCoordinator({
paneKey: 'tab-1:leaf-1',
getPtyId: () => 'pty-1',
getSettings: () => null,
inspectProcess: vi.fn(async () => processResult(foregroundProcess)),
dispatchCompletion,
isLive: () => true
})
coordinator.startProcessTracking()
coordinator.observeTitle('Codex working')
await vi.advanceTimersByTimeAsync(2_000)
foregroundProcess = 'claude'
await vi.advanceTimersByTimeAsync(750)
expect(dispatchCompletion).toHaveBeenCalledWith('codex', {
source: 'process-exit',
quietedHookDone: false
})
})
it('suppresses replacement completion before coordinator state mutation', async () => {
let foregroundProcess = 'codex'
const dispatchCompletion = vi.fn()
const coordinator = createAgentCompletionCoordinator({
paneKey: 'tab-1:leaf-1',
getPtyId: () => 'pty-1',
getSettings: () => null,
inspectProcess: vi.fn(async () => processResult(foregroundProcess)),
dispatchCompletion,
shouldSuppressProcessReplacementCompletion: () => true,
isLive: () => true
})
coordinator.startProcessTracking()
coordinator.observeTitle('Codex working')
await vi.advanceTimersByTimeAsync(2_000)
foregroundProcess = 'claude'
await vi.advanceTimersByTimeAsync(750)
expect(dispatchCompletion).not.toHaveBeenCalled()
coordinator.observeTitle('Claude done')
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
expect(dispatchCompletion).toHaveBeenCalledWith('Claude done')
})
it('suppresses confirmed process exit when the owner vetoes the exited process', async () => {
let foregroundProcess: string | null = 'codex'
const dispatchCompletion = vi.fn()
const shouldSuppressConfirmedProcessExitCompletion = vi.fn(() => true)
const coordinator = createAgentCompletionCoordinator({
paneKey: 'tab-1:leaf-1',
getPtyId: () => 'pty-1',
getSettings: () => null,
inspectProcess: vi.fn(async () => processResult(foregroundProcess)),
dispatchCompletion,
shouldSuppressConfirmedProcessExitCompletion,
isLive: () => true
})
coordinator.startProcessTracking()
coordinator.observeTitle('Codex working')
await vi.advanceTimersByTimeAsync(2_000)
foregroundProcess = null
await vi.advanceTimersByTimeAsync(1_500)
expect(shouldSuppressConfirmedProcessExitCompletion).toHaveBeenCalledWith({
agent: 'codex',
processName: 'codex'
})
expect(dispatchCompletion).not.toHaveBeenCalled()
})
it('suppresses process-exit backstop after a title completion already notified the turn', async () => {
@@ -277,6 +277,7 @@ export function createAgentCompletionCoordinator(
title: string,
optionsOverride: {
quietedHookDone?: boolean
terminalIdleConfirmed?: boolean
agentStatus?: AgentCompletionStatusSnapshot
completionIdentity?: LastCompletionIdentity | null
} = {}
@@ -306,10 +307,16 @@ export function createAgentCompletionCoordinator(
if (optionsOverride.completionIdentity) {
lastCompletionIdentityByPaneKey.set(options.paneKey, optionsOverride.completionIdentity)
}
if (optionsOverride.quietedHookDone === true) {
if (source === 'hook' && optionsOverride.agentStatus) {
options.dispatchHookLifecycle?.(optionsOverride.agentStatus)
}
if (optionsOverride.quietedHookDone === true || source === 'process-exit') {
// Why: confirmed process death is independent completion evidence; keep
// its provenance so stale hook rows cannot veto the notification later.
options.dispatchCompletion(title, {
source,
quietedHookDone: true,
quietedHookDone: optionsOverride.quietedHookDone === true,
...(optionsOverride.terminalIdleConfirmed === true ? { terminalIdleConfirmed: true } : {}),
...(optionsOverride.agentStatus ? { agentStatus: optionsOverride.agentStatus } : {})
})
} else {
@@ -326,6 +333,7 @@ export function createAgentCompletionCoordinator(
return
}
lastAttentionToken = token
options.dispatchHookLifecycle?.(payload)
options.dispatchAttention(payload.agentType ?? options.paneKey, {
source: 'hook',
agentStatus: payload
@@ -438,13 +446,18 @@ export function createAgentCompletionCoordinator(
pendingProcessExitAgent = null
if (lastForegroundAgent?.agent !== process.agent) {
if (lastForegroundAgent && hasAgentRunEvidence) {
dispatchCompletion('process-exit', lastForegroundAgent.processName, {
completionIdentity: {
source: 'process-exit',
identity: `${lastForegroundAgent.agent}:${lastForegroundAgent.processName}`,
agentIdentity: lastForegroundAgent.agent
}
})
if (
options.shouldSuppressProcessReplacementCompletion?.(lastForegroundAgent, process) !==
true
) {
dispatchCompletion('process-exit', lastForegroundAgent.processName, {
completionIdentity: {
source: 'process-exit',
identity: `${lastForegroundAgent.agent}:${lastForegroundAgent.processName}`,
agentIdentity: lastForegroundAgent.agent
}
})
}
}
processSession += 1
}
@@ -486,13 +499,16 @@ export function createAgentCompletionCoordinator(
}
const exited = lastForegroundAgent
pendingProcessExitAgent = null
dispatchCompletion('process-exit', exited.processName, {
completionIdentity: {
source: 'process-exit',
identity: `${exited.agent}:${exited.processName}`,
agentIdentity: exited.agent
}
})
if (options.shouldSuppressConfirmedProcessExitCompletion?.(exited) !== true) {
dispatchCompletion('process-exit', exited.processName, {
terminalIdleConfirmed: true,
completionIdentity: {
source: 'process-exit',
identity: `${exited.agent}:${exited.processName}`,
agentIdentity: exited.agent
}
})
}
lastForegroundAgent = null
clearAgentRunEvidence()
} else {
@@ -787,6 +803,7 @@ export function createAgentCompletionCoordinator(
lastAttentionToken = null
currentTurn += 1
dropPendingTitle()
options.dispatchHookLifecycle?.(payload)
return
}
if (isAttentionHookState(payload.state)) {
@@ -842,11 +859,10 @@ export function createAgentCompletionCoordinator(
agentIdentity: hookCompletionAgentIdentity(payload)
}
: null
dispatchCompletion(
'hook',
payload.agentType ?? options.paneKey,
lastCompletionIdentity ? { completionIdentity: lastCompletionIdentity } : {}
)
dispatchCompletion('hook', payload.agentType ?? options.paneKey, {
agentStatus: payload,
...(lastCompletionIdentity ? { completionIdentity: lastCompletionIdentity } : {})
})
}
}
@@ -203,6 +203,10 @@ describe('agent completion no-evidence inspection cadence', () => {
await vi.advanceTimersByTimeAsync(3_000)
expect(inspectProcess.mock.calls.length).toBeGreaterThan(callsAtExit)
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
expect(dispatchCompletion).toHaveBeenCalledWith('codex')
expect(dispatchCompletion).toHaveBeenCalledWith('codex', {
source: 'process-exit',
quietedHookDone: false,
terminalIdleConfirmed: true
})
})
})
@@ -0,0 +1,50 @@
import { describe, expect, it, vi } from 'vitest'
import {
dispatchAgentHookTerminalLifecycle,
registerAgentHookTerminalLifecycleHandler
} from './agent-hook-terminal-lifecycle'
describe('agent hook terminal lifecycle', () => {
it('routes pane-scoped lifecycle events until unregister', () => {
const handler = vi.fn()
const unregister = registerAgentHookTerminalLifecycleHandler('tab-1:leaf-1', handler)
dispatchAgentHookTerminalLifecycle('tab-1:leaf-1', {
state: 'done',
prompt: 'finish',
agentType: 'codex'
})
unregister()
dispatchAgentHookTerminalLifecycle('tab-1:leaf-1', {
state: 'working',
prompt: 'next',
agentType: 'codex'
})
expect(handler).toHaveBeenCalledTimes(1)
expect(handler).toHaveBeenCalledWith(
expect.objectContaining({ state: 'done', agentType: 'codex' })
)
})
it('does not let stale unregister remove a replacement pane handler', () => {
const staleHandler = vi.fn()
const replacementHandler = vi.fn()
const unregisterStale = registerAgentHookTerminalLifecycleHandler('tab-2:leaf-2', staleHandler)
const unregisterReplacement = registerAgentHookTerminalLifecycleHandler(
'tab-2:leaf-2',
replacementHandler
)
unregisterStale()
dispatchAgentHookTerminalLifecycle('tab-2:leaf-2', {
state: 'blocked',
prompt: 'approval',
agentType: 'claude'
})
unregisterReplacement()
expect(staleHandler).not.toHaveBeenCalled()
expect(replacementHandler).toHaveBeenCalledTimes(1)
})
})
@@ -0,0 +1,26 @@
import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types'
type AgentHookTerminalLifecycleHandler = (payload: AgentCompletionStatusSnapshot) => void
// Why: hook completion authority may live in the global IPC coordinator while
// cursor/cache effects belong to the mounted pane; route accepted events by pane.
const handlersByPaneKey = new Map<string, AgentHookTerminalLifecycleHandler>()
export function registerAgentHookTerminalLifecycleHandler(
paneKey: string,
handler: AgentHookTerminalLifecycleHandler
): () => void {
handlersByPaneKey.set(paneKey, handler)
return () => {
if (handlersByPaneKey.get(paneKey) === handler) {
handlersByPaneKey.delete(paneKey)
}
}
}
export function dispatchAgentHookTerminalLifecycle(
paneKey: string,
payload: AgentCompletionStatusSnapshot
): void {
handlersByPaneKey.get(paneKey)?.(payload)
}
@@ -15436,6 +15436,628 @@ describe('connectPanePty', () => {
)
})
it('ignores title-only idle while fresh hook status is still working', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport()
transportFactoryQueue.push(transport)
const paneKey = makePaneKey('tab-1', LEAF_1)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'still thinking',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
agentType: 'pi',
paneKey,
stateHistory: []
}
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
if (!idleHandler) {
throw new Error('Expected onAgentBecameIdle to be registered')
}
idleHandler('/var/folders/false-idle-title')
expect(deps.dispatchNotification).not.toHaveBeenCalled()
expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalledWith(paneKey, expect.any(Number))
expect(pane.terminal.write).not.toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it('allows an explicit idle title from a different agent than fresh hook status', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport()
transportFactoryQueue.push(transport)
vi.useFakeTimers()
const paneKey = makePaneKey('tab-1', LEAF_1)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'previous agent state',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
agentType: 'pi',
paneKey,
stateHistory: []
}
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
if (!idleHandler) {
throw new Error('Expected onAgentBecameIdle to be registered')
}
idleHandler('* Claude cross-agent done')
vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
expect(deps.dispatchNotification).toHaveBeenCalledWith({
source: 'agent-task-complete',
terminalTitle: '* Claude cross-agent done',
paneKey
})
expect(pane.terminal.write).toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it('ignores an explicit idle title while fresh hook identity is unknown', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport()
transportFactoryQueue.push(transport)
const paneKey = makePaneKey('tab-1', LEAF_1)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'still working without a known agent identity',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
agentType: 'unknown',
paneKey,
stateHistory: []
}
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
if (!idleHandler) {
throw new Error('Expected onAgentBecameIdle to be registered')
}
idleHandler('Claude Code done')
expect(deps.dispatchNotification).not.toHaveBeenCalled()
expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalled()
expect(pane.terminal.write).not.toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it('ignores a Pi idle title while compatible OMP hook status is active', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport()
transportFactoryQueue.push(transport)
const paneKey = makePaneKey('tab-1', LEAF_1)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'OMP is still working',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
agentType: 'omp',
paneKey,
stateHistory: []
}
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
if (!idleHandler) {
throw new Error('Expected onAgentBecameIdle to be registered')
}
idleHandler('Pi ready')
expect(deps.dispatchNotification).not.toHaveBeenCalled()
expect(pane.terminal.write).not.toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it('preserves permission-title cursor and cache side effects through authoritative hook done', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-hook')
transportFactoryQueue.push(transport)
enableActiveRuntimeEnvironment()
const paneKey = makePaneKey('tab-1', LEAF_1)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'waiting',
prompt: 'approve the tool call',
updatedAt: Date.now(),
stateStartedAt: Date.now(),
agentType: 'claude',
paneKey,
stateHistory: []
}
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
const statusHandler = createdTransportOptions[0]?.onAgentStatus as
| ((payload: {
state: 'done'
prompt: string
agentType: 'claude'
lastAssistantMessage: string
}) => void)
| undefined
if (!idleHandler || !statusHandler) {
throw new Error('Expected idle and hook status handlers to be registered')
}
idleHandler('Claude Code permission')
expect(deps.dispatchNotification).not.toHaveBeenCalled()
expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalled()
expect(pane.terminal.write).toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
statusHandler({
state: 'done',
prompt: 'approve the tool call',
agentType: 'claude',
lastAssistantMessage: 'Done.'
})
expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number))
})
it('preserves a genuine hook completion after suppressing an earlier idle title', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-hook')
transportFactoryQueue.push(transport)
enableActiveRuntimeEnvironment()
vi.useFakeTimers()
const paneKey = makePaneKey('tab-1', LEAF_1)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'still working',
updatedAt: Date.now(),
stateStartedAt: Date.now(),
agentType: 'claude',
paneKey,
stateHistory: []
}
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const titleHandler = createdTransportOptions[0]?.onTitleChange as
| ((title: string, rawTitle: string) => void)
| undefined
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
const statusHandler = createdTransportOptions[0]?.onAgentStatus as
| ((payload: {
state: 'done'
prompt: string
agentType: 'claude'
lastAssistantMessage: string
}) => void)
| undefined
if (!titleHandler || !idleHandler || !statusHandler) {
throw new Error('Expected title, idle, and hook status handlers to be registered')
}
titleHandler('Claude working', 'Claude working')
titleHandler('Claude done', 'Claude done')
idleHandler('Claude done')
vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
expect(deps.dispatchNotification).not.toHaveBeenCalled()
statusHandler({
state: 'done',
prompt: 'finish the implementation',
agentType: 'claude',
lastAssistantMessage: 'Done.'
})
notifyStoreSubscribers()
expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalledWith(paneKey, expect.any(Number))
vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS * 2)
expect(deps.dispatchNotification).toHaveBeenCalledWith(
expect.objectContaining({
source: 'agent-task-complete',
paneKey,
agentStatusSnapshot: expect.objectContaining({
state: 'done',
agentType: 'claude',
lastAssistantMessage: 'Done.'
})
})
)
expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number))
expect(pane.terminal.write).toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
expect(storeSubscribers).toHaveLength(1)
})
it('applies accepted hook side effects when every completion alert consumer is disabled', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-hook')
transportFactoryQueue.push(transport)
enableActiveRuntimeEnvironment()
vi.useFakeTimers()
mockStoreState.settings = {
...mockStoreState.settings,
experimentalTerminalAttention: false,
notifications: {
enabled: true,
agentTaskComplete: false,
terminalBell: true,
suppressWhenFocused: false,
customSoundPath: null
}
}
const paneKey = makePaneKey('tab-1', LEAF_1)
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
const statusHandler = createdTransportOptions[0]?.onAgentStatus as
| ((payload: {
state: 'working' | 'done'
prompt: string
agentType: 'claude'
lastAssistantMessage?: string
}) => void)
| undefined
if (!idleHandler || !statusHandler) {
throw new Error('Expected idle and hook status handlers to be registered')
}
statusHandler({
state: 'working',
prompt: 'finish the implementation',
agentType: 'claude'
})
idleHandler('Claude done')
expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalledWith(paneKey, expect.any(Number))
expect(pane.terminal.write).not.toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
statusHandler({
state: 'done',
prompt: 'finish the implementation',
agentType: 'claude',
lastAssistantMessage: 'Done.'
})
vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
expect(deps.dispatchNotification).not.toHaveBeenCalled()
expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number))
expect(pane.terminal.write).toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it.each([
{
name: 'delivers confirmed process exit despite the same stale working hook row',
hookUpdateBeforeDispatch: 'none'
},
{
name: 'delivers confirmed process exit after a same-turn working hook refresh',
hookUpdateBeforeDispatch: 'same-turn'
},
{
name: 'delivers confirmed process exit after same-turn hook identity becomes known',
hookUpdateBeforeDispatch: 'same-turn-known-agent'
},
{
name: 'cancels confirmed process exit delivery when a newer working hook row arrives',
hookUpdateBeforeDispatch: 'new-turn'
}
] as const)('$name', async ({ hookUpdateBeforeDispatch }) => {
const restoreUserAgent = temporarilySetNavigatorUserAgent(
'Mozilla/5.0 (Windows NT 10.0; Win64; x64)'
)
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-crashed-codex')
transportFactoryQueue.push(transport)
vi.useFakeTimers()
try {
const paneKey = makePaneKey('tab-1', LEAF_1)
const crashedTurnStartedAt = Date.now()
const initialAgentType =
hookUpdateBeforeDispatch === 'same-turn-known-agent' ? 'unknown' : 'codex'
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'crash before done hook',
updatedAt: crashedTurnStartedAt,
stateStartedAt: crashedTurnStartedAt,
agentType: initialAgentType,
paneKey,
stateHistory: []
}
const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess)
getForegroundProcess.mockResolvedValue('codex')
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
await flushAsyncTicks()
const titleHandler = createdTransportOptions[0]?.onTitleChange as
| ((title: string, rawTitle: string) => void)
| undefined
if (!titleHandler) {
throw new Error('Expected onTitleChange to be registered')
}
titleHandler('Codex working', 'Codex working')
await vi.advanceTimersByTimeAsync(2_500)
getForegroundProcess.mockResolvedValue(null)
await vi.advanceTimersByTimeAsync(1_800)
if (hookUpdateBeforeDispatch !== 'none') {
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt:
hookUpdateBeforeDispatch === 'new-turn'
? 'new turn after the prior process exited'
: 'same turn hook detail refresh',
updatedAt: Date.now(),
stateStartedAt:
hookUpdateBeforeDispatch === 'new-turn' ? Date.now() : crashedTurnStartedAt,
agentType: 'codex',
paneKey,
stateHistory: []
}
notifyStoreSubscribers()
}
await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
const expectedNotification = {
source: 'agent-task-complete',
terminalTitle: 'codex',
paneKey,
agentCompletionSource: 'process-exit'
}
if (hookUpdateBeforeDispatch === 'new-turn') {
expect(deps.dispatchNotification).not.toHaveBeenCalledWith(expectedNotification)
} else {
expect(deps.dispatchNotification).toHaveBeenCalledWith(expectedNotification)
}
expect(pane.terminal.write).toHaveBeenCalledWith(
`${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}`,
expect.any(Function)
)
} finally {
restoreUserAgent()
}
})
it('drops an exited agent completion when a replacement agent hook row is active', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-replaced-codex')
transportFactoryQueue.push(transport)
vi.useFakeTimers()
const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess)
getForegroundProcess.mockResolvedValue('codex')
const paneKey = makePaneKey('tab-1', LEAF_1)
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
await flushAsyncTicks()
const titleHandler = createdTransportOptions[0]?.onTitleChange as
| ((title: string, rawTitle: string) => void)
| undefined
if (!titleHandler) {
throw new Error('Expected onTitleChange to be registered')
}
titleHandler('Codex working', 'Codex working')
await vi.advanceTimersByTimeAsync(2_500)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'replacement agent turn',
updatedAt: Date.now(),
stateStartedAt: Date.now(),
agentType: 'claude',
paneKey,
stateHistory: []
}
getForegroundProcess.mockResolvedValue('claude')
await vi.advanceTimersByTimeAsync(1_000)
await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
expect(deps.dispatchNotification).not.toHaveBeenCalledWith(
expect.objectContaining({ source: 'agent-task-complete' })
)
expect(pane.terminal.write).not.toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'done',
prompt: 'replacement agent turn',
updatedAt: Date.now(),
stateStartedAt: Date.now(),
agentType: 'claude',
paneKey,
stateHistory: []
}
titleHandler('Claude done', 'Claude done')
await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
expect(deps.dispatchNotification).toHaveBeenCalledWith(
expect.objectContaining({
source: 'agent-task-complete',
terminalTitle: 'Claude done',
paneKey
})
)
})
it('drops confirmed idle exit when a different hook owner appears between null samples', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-replaced-codex')
transportFactoryQueue.push(transport)
vi.useFakeTimers()
const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess)
getForegroundProcess.mockResolvedValue('codex')
const paneKey = makePaneKey('tab-1', LEAF_1)
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
await flushAsyncTicks()
const titleHandler = createdTransportOptions[0]?.onTitleChange as
| ((title: string, rawTitle: string) => void)
| undefined
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
if (!titleHandler || !idleHandler) {
throw new Error('Expected title and idle handlers to be registered')
}
titleHandler('Codex working', 'Codex working')
await vi.advanceTimersByTimeAsync(2_500)
getForegroundProcess.mockResolvedValue(null)
await vi.advanceTimersByTimeAsync(800)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'replacement agent turn',
updatedAt: Date.now(),
stateStartedAt: Date.now(),
agentType: 'claude',
paneKey,
stateHistory: []
}
idleHandler('Claude done')
await vi.advanceTimersByTimeAsync(800)
await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS)
expect(deps.dispatchNotification).not.toHaveBeenCalledWith(
expect.objectContaining({ source: 'agent-task-complete' })
)
expect(pane.terminal.write).not.toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it('preserves replacement-agent title side effects through the process replacement veto', async () => {
const { dispatchAgentHookTerminalLifecycle } = await import('./agent-hook-terminal-lifecycle')
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('pty-replaced-codex')
transportFactoryQueue.push(transport)
vi.useFakeTimers()
const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess)
getForegroundProcess.mockResolvedValue('codex')
const paneKey = makePaneKey('tab-1', LEAF_1)
const pane = createPane(1)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(pane as never, manager as never, deps as never)
await flushAsyncTicks()
const titleHandler = createdTransportOptions[0]?.onTitleChange as
| ((title: string, rawTitle: string) => void)
| undefined
const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as
| ((title: string) => void)
| undefined
if (!titleHandler || !idleHandler) {
throw new Error('Expected title and idle handlers to be registered')
}
titleHandler('Codex working', 'Codex working')
await vi.advanceTimersByTimeAsync(2_500)
mockStoreState.agentStatusByPaneKey[paneKey] = {
state: 'working',
prompt: 'replacement agent turn',
updatedAt: Date.now(),
stateStartedAt: Date.now(),
agentType: 'claude',
paneKey,
stateHistory: []
}
idleHandler('Claude done')
getForegroundProcess.mockResolvedValue('claude')
await vi.advanceTimersByTimeAsync(1_000)
dispatchAgentHookTerminalLifecycle(paneKey, {
state: 'done',
prompt: 'replacement agent turn',
agentType: 'claude',
lastAssistantMessage: 'Done.'
})
expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number))
expect(pane.terminal.write).toHaveBeenCalledWith(
RESET_TERMINAL_CURSOR_STYLE,
expect.any(Function)
)
})
it('resets renderer cursor style when an agent becomes idle', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport()
@@ -120,7 +120,7 @@ import { resolveSshPaneConnectGate } from './ssh-pane-connect-gate'
import { dispatchTerminalCommandFinishedEvent } from '@/hooks/terminal-command-finished-event'
import { e2eConfig } from '@/lib/e2e-config'
import {
AGENT_STATUS_STALE_AFTER_MS,
isFreshNonDoneAgentStatus,
type AgentStatusEntry,
type AgentType
} from '../../../../shared/agent-status-types'
@@ -135,12 +135,19 @@ import {
type AgentInterruptInputIntent
} from '../../../../shared/agent-interrupt-intent'
import { createAgentCompletionCoordinator } from './agent-completion-coordinator'
import {
dispatchAgentHookTerminalLifecycle,
registerAgentHookTerminalLifecycleHandler
} from './agent-hook-terminal-lifecycle'
import {
createCodexAutoApprovalHookCompletionSuppressor,
shouldSuppressCodexAutoApprovalSyntheticTitle,
shouldSuppressCodexAutoApprovalStatus
} from './codex-auto-approval-notification-suppression'
import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types'
import type {
AgentCompletionDispatchMeta,
AgentCompletionStatusSnapshot
} from './agent-completion-coordinator-types'
import {
markTerminalBracketedPasteInterrupted,
observeTerminalBracketedPasteModeOutput
@@ -951,6 +958,7 @@ export function connectPanePty(
let agentTaskCompleteSettingsUnsubscribe: (() => void) | null = null
let agentTaskCompleteNotificationGeneration = 0
let wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled()
let requiresFreshWorkingForAgentTaskCompleteNotification = !wasAgentTaskCompleteTrackingEnabled
let wasAgentTaskCompleteOsNotificationEnabled = isAgentTaskCompleteNotificationEnabled()
let terminalBellNotificationTimer: ReturnType<typeof setTimeout> | null = null
let pendingTerminalBellNotification = false
@@ -1297,13 +1305,94 @@ export function connectPanePty(
const isFreshActivePaneAgentEntry = (
entry: AgentStatusEntry | undefined
): entry is AgentStatusEntry => {
return (
!!entry &&
typeof entry.updatedAt === 'number' &&
Date.now() - entry.updatedAt <= AGENT_STATUS_STALE_AFTER_MS &&
entry.state !== 'done'
)
return isFreshNonDoneAgentStatus(entry)
}
const shouldSuppressTitleCompletionForFreshHook = (
title: string,
activeHookStatus: AgentStatusEntry | undefined
): boolean => {
if (
detectAgentStatusFromTitle(title) === 'working' ||
!isFreshNonDoneAgentStatus(activeHookStatus)
) {
return false
}
const explicitTitleAgentType = resolveCommittedTitleAgentType(title)
const activeHookAgentForTitle = resolveCompatibleAgentTypeForOwner(
activeHookStatus?.agentType,
explicitTitleAgentType
)
const titleNamesDifferentKnownAgent =
explicitTitleAgentType &&
activeHookStatus?.agentType &&
activeHookStatus.agentType !== 'unknown' &&
activeHookAgentForTitle !== explicitTitleAgentType
return !titleNamesDifferentKnownAgent
}
let pendingSuppressedTitleSideEffects: {
title: string
agentType: AgentType | undefined
} | null = null
const clearSuppressedTitleSideEffects = (): void => {
pendingSuppressedTitleSideEffects = null
}
const applyAgentCompletionSideEffects = (
title: string,
agentType: AgentType | undefined
): void => {
const settings = useAppStore.getState().settings
if (
(agentType === 'claude' || isClaudeAgent(title)) &&
(settings === null || settings.promptCacheTimerEnabled)
) {
deps.setCacheTimerStartedAt(cacheKey, Date.now())
}
queueAgentIdleTerminalModeReset()
}
const preserveSuppressedTitleSideEffects = (
title: string,
activeHookStatus: AgentStatusEntry
): void => {
pendingSuppressedTitleSideEffects = {
title,
agentType: activeHookStatus.agentType
}
if (activeHookStatus.state === 'waiting' || activeHookStatus.state === 'blocked') {
queueAgentIdleTerminalModeReset()
}
}
const handleAgentHookTerminalLifecycle = (payload: AgentCompletionStatusSnapshot): void => {
const pending = pendingSuppressedTitleSideEffects
if (!pending) {
return
}
const payloadAgentForPending = resolveCompatibleAgentTypeForOwner(
payload.agentType,
pending.agentType
)
const belongsToPendingAgent =
!pending.agentType ||
pending.agentType === 'unknown' ||
!payload.agentType ||
payload.agentType === 'unknown' ||
payloadAgentForPending === pending.agentType
if (!belongsToPendingAgent || payload.state === 'working') {
clearSuppressedTitleSideEffects()
return
}
if (payload.state === 'done') {
applyAgentCompletionSideEffects(pending.title, payload.agentType ?? pending.agentType)
clearSuppressedTitleSideEffects()
return
}
if (payload.state === 'waiting' || payload.state === 'blocked') {
queueAgentIdleTerminalModeReset()
}
}
const unregisterAgentHookTerminalLifecycle = registerAgentHookTerminalLifecycleHandler(
cacheKey,
handleAgentHookTerminalLifecycle
)
const hasFreshPaneAgentSurface = (): boolean => {
const entry = useAppStore.getState().agentStatusByPaneKey[cacheKey]
if (isFreshActivePaneAgentEntry(entry)) {
@@ -1934,11 +2023,47 @@ export function connectPanePty(
getPtyId: () => transport.getPtyId(),
getSettings: () => useAppStore.getState().settings,
inspectProcess: inspectRuntimeTerminalProcess,
dispatchCompletion: (title, meta) =>
dispatchHookLifecycle: (payload) => dispatchAgentHookTerminalLifecycle(cacheKey, payload),
shouldSuppressProcessReplacementCompletion: (_exited, replacement) => {
const currentStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey]
const currentAgentForReplacement = resolveCompatibleAgentTypeForOwner(
currentStatus?.agentType,
replacement.agent
)
return (
isFreshNonDoneAgentStatus(currentStatus) && currentAgentForReplacement === replacement.agent
)
},
shouldSuppressConfirmedProcessExitCompletion: (exited) => {
const currentStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey]
const currentAgentForExited = resolveCompatibleAgentTypeForOwner(
currentStatus?.agentType,
exited.agent
)
// Why: a replacement hook can lead process visibility by one cadence;
// only a different known active owner can veto confirmed old-process exit.
return Boolean(
isFreshNonDoneAgentStatus(currentStatus) &&
currentStatus.agentType &&
currentStatus.agentType !== 'unknown' &&
currentAgentForExited !== exited.agent
)
},
dispatchCompletion: (title, meta) => {
if (meta?.source === 'process-exit') {
clearSuppressedTitleSideEffects()
}
if (meta?.terminalIdleConfirmed === true) {
// Why: an agent can crash before its done hook; confirmed process death
// must still restore cursor and native Windows Kitty keyboard modes.
queueAgentIdleTerminalModeReset()
}
scheduleAgentTaskCompleteNotification(title, {
allowDoneDetailAfterGrace: meta?.quietedHookDone,
...(meta?.source === 'process-exit' ? { agentCompletionSource: meta.source } : {}),
...(meta?.agentStatus ? { agentStatusSnapshot: meta.agentStatus } : {})
}),
})
},
dispatchAttention: (title, meta) =>
scheduleAgentTaskCompleteNotification(title, {
agentStatusSnapshot: meta.agentStatus
@@ -2254,7 +2379,12 @@ export function connectPanePty(
// feed completion tracking — observeTitle would classify the cleared
// title as idle and mint a task-complete for a merely-paused agent.
if (!meta?.staleWorkingTitleClear && syncAgentTaskCompleteTrackingEnabled()) {
agentCompletionCoordinator.observeTitle(decision.rawTitle)
const activeHookStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey]
if (!shouldSuppressTitleCompletionForFreshHook(decision.rawTitle, activeHookStatus)) {
// Why: display titles still update while hooks are active, but a stale
// idle frame must not complete the coordinator turn before hook `done`.
agentCompletionCoordinator.observeTitle(decision.rawTitle)
}
}
// Why: only the focused pane should drive the tab title — otherwise two
// agents in split panes cause rapid title flickering as each emits OSC
@@ -2521,17 +2651,17 @@ export function connectPanePty(
}
if (!enabled && wasAgentTaskCompleteTrackingEnabled) {
// Why: disabling every completion consumer is an event-time boundary.
// Drop pending timers and coordinator state so old work cannot replay.
// Drop pending alerts while preserving accepted-hook lifecycle state.
agentTaskCompleteNotificationGeneration += 1
requiresFreshWorkingForAgentTaskCompleteNotification = true
clearPendingAgentTaskCompleteNotification()
agentCompletionCoordinator.resetCompletionState({ requireFreshWorking: true })
if (pendingTerminalBellNotification) {
scheduleTerminalBellNotification()
}
} else if (enabled && !wasAgentTaskCompleteTrackingEnabled) {
// Why: a pane may have observed work while all completion consumers were
// disabled. Re-enabling should not let the next idle event report old work.
agentCompletionCoordinator.resetCompletionState({ requireFreshWorking: true })
requiresFreshWorkingForAgentTaskCompleteNotification = true
}
wasAgentTaskCompleteTrackingEnabled = enabled
wasAgentTaskCompleteOsNotificationEnabled = osNotificationsEnabled
@@ -2543,20 +2673,49 @@ export function connectPanePty(
options: {
allowDoneDetailAfterGrace?: boolean
agentStatusSnapshot?: AgentCompletionStatusSnapshot
agentCompletionSource?: AgentCompletionDispatchMeta['source']
} = {}
): void => {
if (!syncAgentTaskCompleteTrackingEnabled()) {
if (
!syncAgentTaskCompleteTrackingEnabled() ||
requiresFreshWorkingForAgentTaskCompleteNotification
) {
return
}
clearPendingAgentTaskCompleteNotification()
let graceElapsed = false
const generationAtSchedule = agentTaskCompleteNotificationGeneration
const agentStatusAtSchedule = useAppStore.getState().agentStatusByPaneKey[cacheKey]
const hasNewerActiveHookStatus = (): boolean => {
const currentStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey]
const scheduledAgentType = agentStatusAtSchedule?.agentType
const currentAgentForScheduledTurn = resolveCompatibleAgentTypeForOwner(
currentStatus?.agentType,
scheduledAgentType
)
const hasDifferentKnownAgent = Boolean(
currentStatus?.agentType &&
scheduledAgentType &&
currentStatus.agentType !== 'unknown' &&
scheduledAgentType !== 'unknown' &&
currentAgentForScheduledTurn !== scheduledAgentType
)
return (
options.agentCompletionSource === 'process-exit' &&
isFreshNonDoneAgentStatus(currentStatus) &&
(!agentStatusAtSchedule ||
currentStatus.state !== agentStatusAtSchedule.state ||
currentStatus.stateStartedAt !== agentStatusAtSchedule.stateStartedAt ||
hasDifferentKnownAgent)
)
}
const dispatch = (): void => {
clearPendingAgentTaskCompleteNotification()
if (
generationAtSchedule !== agentTaskCompleteNotificationGeneration ||
!syncAgentTaskCompleteTrackingEnabled()
!syncAgentTaskCompleteTrackingEnabled() ||
hasNewerActiveHookStatus()
) {
return
}
@@ -2573,12 +2732,21 @@ export function connectPanePty(
source: 'agent-task-complete',
terminalTitle: title,
paneKey: cacheKey,
...(options.agentCompletionSource
? { agentCompletionSource: options.agentCompletionSource }
: {}),
...(shouldDispatchOsNotification ? {} : { suppressOsNotification: true }),
...(options.agentStatusSnapshot ? { agentStatusSnapshot: options.agentStatusSnapshot } : {})
})
}
const dispatchIfDetailed = (): void => {
if (hasNewerActiveHookStatus()) {
// Why: the confirmed exit belongs to the row captured above; a replaced
// active row means a newer turn started during the notification delay.
clearPendingAgentTaskCompleteNotification()
return
}
if (!graceElapsed) {
return
}
@@ -2629,6 +2797,16 @@ export function connectPanePty(
deps.setCacheTimerStartedAt(cacheKey, null)
return
}
const currentState = useAppStore.getState()
const activeHookStatus = currentState.agentStatusByPaneKey[cacheKey]
if (shouldSuppressTitleCompletionForFreshHook(title, activeHookStatus)) {
// Why: agent CLIs can briefly publish an idle title while hook status
// still says the same agent turn is active (e.g. during tool output).
if (activeHookStatus) {
preserveSuppressedTitleSideEffects(title, activeHookStatus)
}
return
}
// Why: only start the prompt-cache countdown for Claude agents — other
// agents have different (or no) prompt-caching semantics and showing a
// timer for them would be misleading.
@@ -2639,7 +2817,7 @@ export function connectPanePty(
// tab silently drops the timer. Writing a timestamp is cheap and the
// CacheTimer component gates rendering on the enabled flag, so a
// spurious write when the feature turns out to be disabled is harmless.
const settings = useAppStore.getState().settings
const settings = currentState.settings
if (isClaudeAgent(title) && (settings === null || settings.promptCacheTimerEnabled)) {
deps.setCacheTimerStartedAt(cacheKey, Date.now())
}
@@ -2651,7 +2829,9 @@ export function connectPanePty(
queueAgentIdleTerminalModeReset()
}
const onAgentBecameWorking = (): void => {
clearSuppressedTitleSideEffects()
if (syncAgentTaskCompleteTrackingEnabled()) {
requiresFreshWorkingForAgentTaskCompleteNotification = false
agentCompletionCoordinator.observeTitleWorking()
}
// Why: a new API call refreshes the prompt-cache TTL, so clear any running
@@ -2663,6 +2843,7 @@ export function connectPanePty(
}
}
const onAgentExited = (): void => {
clearSuppressedTitleSideEffects()
clearCommandInferredPaneAgent()
// Why: when the terminal title reverts to a plain shell (e.g., "bash", "zsh"),
// the agent has exited. Clear any running cache timer so the sidebar doesn't
@@ -2917,14 +3098,18 @@ export function connectPanePty(
} else {
currentState.setAgentStatus(cacheKey, statusPayload, statusTitle)
}
if (syncAgentTaskCompleteTrackingEnabled()) {
const storedStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey]
const notificationPayload =
typeof storedStatus?.stateStartedAt === 'number'
? { ...statusPayload, stateStartedAt: storedStatus.stateStartedAt }
: statusPayload
agentCompletionCoordinator.observeHookStatus(notificationPayload)
const trackingEnabled = syncAgentTaskCompleteTrackingEnabled()
if (payload.state === 'working' && trackingEnabled) {
requiresFreshWorkingForAgentTaskCompleteNotification = false
}
const storedStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey]
const notificationPayload =
typeof storedStatus?.stateStartedAt === 'number'
? { ...statusPayload, stateStartedAt: storedStatus.stateStartedAt }
: statusPayload
// Why: hook lifecycle owns deferred terminal side effects even when
// every outward completion alert consumer is disabled.
agentCompletionCoordinator.observeHookStatus(notificationPayload)
if (payload.state === 'working' && pendingTerminalBellNotification) {
scheduleTerminalBellNotification()
}
@@ -6985,6 +7170,8 @@ export function connectPanePty(
}
cleanupStartupDraftPasteTimers()
releaseUnattemptedStartupDraftPasteDelivery()
unregisterAgentHookTerminalLifecycle()
clearSuppressedTitleSideEffects()
clearPendingAgentTaskCompleteNotification()
pendingTerminalBellNotification = false
clearTerminalBellNotificationTimer()
@@ -1,6 +1,9 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { dispatchTerminalNotification } from './use-notification-dispatch'
import type { AgentStatusEntry } from '../../../../shared/agent-status-types'
import {
AGENT_STATUS_STALE_AFTER_MS,
type AgentStatusEntry
} from '../../../../shared/agent-status-types'
import type { TerminalLayoutSnapshot } from '../../../../shared/types'
import { buildAgentNotificationId } from '../../../../shared/agent-notification-id'
@@ -449,11 +452,12 @@ describe('dispatchTerminalNotification', () => {
expect(mockState.markTerminalPaneUnread).toHaveBeenCalledWith(paneKey)
})
it('does not reuse a fresh stale agent snapshot when the terminal title names another agent', () => {
it('does not let fresh active status suppress a completion from another named agent', () => {
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
state: 'working',
agentType: 'codex',
terminalTitle: 'Codex',
lastAssistantMessage: 'Codex done.'
lastAssistantMessage: undefined
})
dispatchTerminalNotification('wt-primary', {
@@ -554,6 +558,125 @@ describe('dispatchTerminalNotification', () => {
)
})
it('drops a title-only completion when fresh hook state is still active', () => {
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
state: 'working',
prompt: 'still running',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
lastAssistantMessage: undefined
})
dispatchTerminalNotification('wt-primary', {
source: 'agent-task-complete',
terminalTitle: '✳ Launch UI and thumbnail generator',
paneKey
})
expect(window.api.notifications.dispatch).not.toHaveBeenCalled()
expect(mockState.markWorktreeUnread).not.toHaveBeenCalled()
expect(mockState.markAgentCompletionPaneUnread).not.toHaveBeenCalled()
expect(mockState.markTerminalTabUnread).not.toHaveBeenCalled()
expect(mockState.markTerminalPaneUnread).not.toHaveBeenCalled()
})
it('allows confirmed process-exit completion while fresh hook state is still active', () => {
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
state: 'working',
prompt: 'agent crashed before its done hook',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
lastAssistantMessage: undefined
})
dispatchTerminalNotification('wt-primary', {
source: 'agent-task-complete',
terminalTitle: 'codex',
paneKey,
agentCompletionSource: 'process-exit'
})
expect(window.api.notifications.dispatch).toHaveBeenCalledWith(
expect.objectContaining({
source: 'agent-task-complete',
worktreeId: 'wt-primary',
paneKey,
terminalTitle: 'codex'
})
)
expect(mockState.markWorktreeUnread).toHaveBeenCalledWith('wt-primary')
expect(mockState.markTerminalTabUnread).toHaveBeenCalledWith('tab-1')
expect(mockState.markTerminalPaneUnread).toHaveBeenCalledWith(paneKey)
const dispatchArgs = getLastNotificationDispatchArg()
expect(dispatchArgs?.agentState).toBeUndefined()
expect(dispatchArgs?.agentPrompt).toBeUndefined()
})
it.each([undefined, 'unknown'] as const)(
'drops an explicitly named title completion when fresh hook identity is %s',
(agentType) => {
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
state: 'working',
agentType,
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
lastAssistantMessage: undefined
})
dispatchTerminalNotification('wt-primary', {
source: 'agent-task-complete',
terminalTitle: 'Claude Code done',
paneKey
})
expect(window.api.notifications.dispatch).not.toHaveBeenCalled()
expect(mockState.markWorktreeUnread).not.toHaveBeenCalled()
expect(mockState.markAgentCompletionPaneUnread).not.toHaveBeenCalled()
expect(mockState.markTerminalTabUnread).not.toHaveBeenCalled()
expect(mockState.markTerminalPaneUnread).not.toHaveBeenCalled()
}
)
it('drops a Pi title completion while compatible OMP hook status is active', () => {
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
state: 'working',
agentType: 'omp',
updatedAt: Date.now() - 60_000,
stateStartedAt: Date.now() - 60_000,
lastAssistantMessage: undefined
})
dispatchTerminalNotification('wt-primary', {
source: 'agent-task-complete',
terminalTitle: 'Pi ready',
paneKey
})
expect(window.api.notifications.dispatch).not.toHaveBeenCalled()
expect(mockState.markWorktreeUnread).not.toHaveBeenCalled()
expect(mockState.markAgentCompletionPaneUnread).not.toHaveBeenCalled()
expect(mockState.markTerminalTabUnread).not.toHaveBeenCalled()
expect(mockState.markTerminalPaneUnread).not.toHaveBeenCalled()
})
it('allows title-only completion after active hook status becomes stale', () => {
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
state: 'working',
updatedAt: Date.now() - AGENT_STATUS_STALE_AFTER_MS - 1,
stateStartedAt: Date.now() - AGENT_STATUS_STALE_AFTER_MS - 1,
lastAssistantMessage: undefined
})
dispatchTerminalNotification('wt-primary', {
source: 'agent-task-complete',
terminalTitle: '/workspace/orca',
paneKey
})
expect(window.api.notifications.dispatch).toHaveBeenCalled()
expect(mockState.markWorktreeUnread).toHaveBeenCalledWith('wt-primary')
})
it('drops a delayed completion snapshot when the pane has already started a newer turn', () => {
const previousDoneStartedAt = Date.now() - 10_000
mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, {
@@ -5,8 +5,16 @@ import { getRepoMapFromState, getWorktreeMapFromState } from '@/store/selectors'
import { playDesktopNotificationSound } from '@/lib/desktop-notification-sound'
import { showBlockedNotificationFallbackToast } from '@/lib/blocked-notification-fallback'
import { buildAgentNotificationId } from '../../../../shared/agent-notification-id'
import { resolveCompatibleAgentTypeForOwner } from '../../../../shared/agent-title-owner'
import {
isFreshNonDoneAgentStatus,
type AgentStatusEntry
} from '../../../../shared/agent-status-types'
import { isSupersededAgentCompletionSnapshot } from './agent-completion-snapshot-staleness'
import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types'
import type {
AgentCompletionDispatchMeta,
AgentCompletionStatusSnapshot
} from './agent-completion-coordinator-types'
import {
countReposNeedingNotificationDisambiguation,
getPaneKeyTabId,
@@ -28,11 +36,28 @@ function agentSnapshotMatchesExplicitTitle(
return !snapshot || !explicitTitleAgentType || snapshot.agentType === explicitTitleAgentType
}
function hasFreshActiveHookStatus(
snapshot: Pick<AgentStatusEntry, 'state' | 'updatedAt' | 'agentType'> | undefined,
explicitTitleAgentType: string | null
): boolean {
const activeHookAgentForTitle = resolveCompatibleAgentTypeForOwner(
snapshot?.agentType,
explicitTitleAgentType
)
const titleNamesDifferentKnownAgent =
explicitTitleAgentType &&
snapshot?.agentType &&
snapshot.agentType !== 'unknown' &&
activeHookAgentForTitle !== explicitTitleAgentType
return Boolean(isFreshNonDoneAgentStatus(snapshot) && !titleNamesDifferentKnownAgent)
}
export type TerminalNotificationEvent = {
source: 'terminal-bell' | 'agent-task-complete'
terminalTitle?: string
paneKey?: string
agentStatusSnapshot?: AgentCompletionStatusSnapshot
agentCompletionSource?: AgentCompletionDispatchMeta['source']
suppressOsNotification?: boolean
}
@@ -70,9 +95,24 @@ export function dispatchTerminalNotification(
agentSnapshotMatchesExplicitTitle(storedAgentStatus, explicitTitleAgentType)
? storedAgentStatus
: undefined
if (
event.source === 'agent-task-complete' &&
event.agentCompletionSource !== 'process-exit' &&
!eventAgentStatusSnapshot &&
hasFreshActiveHookStatus(storedAgentStatus, explicitTitleAgentType)
) {
// Why: a title-only idle signal can race behind active hook state; a
// confirmed process exit is independent authority that the turn ended.
return
}
// Why: a process can die before its hook emits done; do not label the
// resulting completion notification with that stale active state or prompt.
const agentStatus =
event.source === 'agent-task-complete'
? (eventAgentStatusSnapshot ?? freshStoredAgentStatus)
? (eventAgentStatusSnapshot ??
(event.agentCompletionSource === 'process-exit' && freshStoredAgentStatus?.state !== 'done'
? undefined
: freshStoredAgentStatus))
: undefined
if (
event.source === 'agent-task-complete' &&
@@ -4,6 +4,7 @@ import { YOLO_TUI_AGENT_ARGS } from '../../../shared/tui-agent-permissions'
import { createHookListenerState, normalizeHookPayload } from '../../../shared/agent-hook-listener'
const dispatchTerminalNotification = vi.fn()
const dispatchAgentHookTerminalLifecycle = vi.fn()
type MockStoreState = {
settings: {
@@ -66,6 +67,10 @@ vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({
dispatchTerminalNotification
}))
vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({
dispatchAgentHookTerminalLifecycle
}))
function hookStatus(state: ParsedAgentStatusPayload['state']): ParsedAgentStatusPayload {
return {
state,
@@ -105,6 +110,7 @@ describe('agent hook completion notifications', () => {
vi.resetModules()
vi.useFakeTimers()
dispatchTerminalNotification.mockClear()
dispatchAgentHookTerminalLifecycle.mockClear()
mockStoreState = {
settings: {
experimentalTerminalAttention: false,
@@ -183,6 +189,39 @@ describe('agent hook completion notifications', () => {
)
}, 15_000)
it('accepts hook lifecycle while every completion alert consumer is disabled', async () => {
mockStoreState.settings.notifications.agentTaskComplete = false
mockStoreState.settings.experimentalTerminalAttention = false
const {
observeAgentHookCompletionForNotification,
syncAgentHookCompletionNotificationSettings
} = await import('./agent-hook-completion-notifications')
observeAgentHookCompletionForNotification({
paneKey,
worktreeId: 'wt-1',
payload: hookStatus('working')
})
observeAgentHookCompletionForNotification({
paneKey,
worktreeId: 'wt-1',
payload: hookStatus('done')
})
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
expect(dispatchAgentHookTerminalLifecycle).toHaveBeenCalledWith(
paneKey,
expect.objectContaining({ state: 'done', agentType: 'codex' })
)
expect(dispatchTerminalNotification).not.toHaveBeenCalled()
mockStoreState.settings.notifications.agentTaskComplete = true
syncAgentHookCompletionNotificationSettings()
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
expect(dispatchTerminalNotification).not.toHaveBeenCalled()
})
it('tracks hook completion for terminal attention when OS completion notifications are disabled', async () => {
mockStoreState.settings.experimentalTerminalAttention = true
mockStoreState.settings.notifications.agentTaskComplete = false
@@ -695,6 +734,11 @@ describe('agent hook completion notifications', () => {
})
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS - 1)
expect(dispatchTerminalNotification).not.toHaveBeenCalled()
expect(
dispatchAgentHookTerminalLifecycle.mock.calls.filter(
([, payload]) => payload.state === 'done'
)
).toHaveLength(0)
observeAgentHookCompletionForNotification({
paneKey,
@@ -703,6 +747,11 @@ describe('agent hook completion notifications', () => {
})
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
expect(dispatchTerminalNotification).not.toHaveBeenCalled()
expect(
dispatchAgentHookTerminalLifecycle.mock.calls.filter(
([, payload]) => payload.state === 'done'
)
).toHaveLength(0)
observeAgentHookCompletionForNotification({
paneKey,
@@ -712,6 +761,10 @@ describe('agent hook completion notifications', () => {
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1)
expect(dispatchAgentHookTerminalLifecycle).toHaveBeenCalledWith(
paneKey,
expect.objectContaining({ state: 'done', agentType: 'codex' })
)
expect(dispatchTerminalNotification).toHaveBeenCalledWith(
'wt-1',
expect.objectContaining({
@@ -771,7 +824,7 @@ describe('agent hook completion notifications', () => {
expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(3)
})
it('reads tabsByWorktree once per prune pass regardless of coordinator count', async () => {
it('skips tab scans until a pane-liveness slice changes', async () => {
seedManyLivePanes()
const {
observeAgentHookCompletionForNotification,
@@ -786,21 +839,27 @@ describe('agent hook completion notifications', () => {
})
}
// Count tabsByWorktree reads during a single prune pass. Pre-fix this was
// O(coordinators) because each pane re-flattened tabsByWorktree; the index
// makes it exactly one read for the whole pass.
// Count full tab-map enumerations rather than cheap reference reads.
const realTabs = mockStoreState.tabsByWorktree
let tabsReadCount = 0
Object.defineProperty(mockStoreState, 'tabsByWorktree', {
configurable: true,
get() {
tabsReadCount += 1
return realTabs
let tabEnumerationCount = 0
mockStoreState.tabsByWorktree = new Proxy(realTabs, {
ownKeys(target) {
tabEnumerationCount += 1
return Reflect.ownKeys(target)
}
})
syncAgentHookCompletionNotificationSettings()
expect(tabsReadCount).toBe(1)
expect(tabEnumerationCount).toBe(1)
syncAgentHookCompletionNotificationSettings()
expect(tabEnumerationCount).toBe(1)
mockStoreState.ptyIdsByTabId = { ...mockStoreState.ptyIdsByTabId }
syncAgentHookCompletionNotificationSettings()
expect(tabEnumerationCount).toBe(2)
})
})
@@ -9,6 +9,7 @@ import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-termina
import { dispatchTerminalNotification } from '@/components/terminal-pane/use-notification-dispatch'
import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization'
import { createCodexAutoApprovalHookCompletionSuppressor } from '@/components/terminal-pane/codex-auto-approval-notification-suppression'
import { dispatchAgentHookTerminalLifecycle } from '@/components/terminal-pane/agent-hook-terminal-lifecycle'
type CoordinatorEntry = {
worktreeId: string
@@ -18,14 +19,18 @@ type CoordinatorEntry = {
type StoreSnapshot = ReturnType<typeof useAppStore.getState>
type WorktreeTab = NonNullable<StoreSnapshot['tabsByWorktree']>[string][number]
// Why: a paneKey resolves to a tab by id. Prebuilding this index once per prune
// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x
// tabs)) — the prune runs on every store notify, including every OSC title frame.
// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x tabs)).
type TabIndex = ReadonlyMap<string, WorktreeTab>
type PaneCoordinatorLivenessSnapshot = Pick<
StoreSnapshot,
'tabsByWorktree' | 'ptyIdsByTabId' | 'terminalLayoutsByTabId' | 'suppressedPtyExitIds'
>
const coordinatorsByPaneKey = new Map<string, CoordinatorEntry>()
const paneKeysRequiringFreshWorking = new Set<string>()
let wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled()
let requireFreshWorkingForNewTrackingCoordinators = !wasAgentTaskCompleteTrackingEnabled
let lastPrunedLivenessSnapshot: PaneCoordinatorLivenessSnapshot | null = null
function disposeCoordinatorForPaneKey(paneKey: string): void {
coordinatorsByPaneKey.get(paneKey)?.coordinator.dispose()
@@ -33,9 +38,9 @@ function disposeCoordinatorForPaneKey(paneKey: string): void {
paneKeysRequiringFreshWorking.delete(paneKey)
}
function buildTabIndex(state: StoreSnapshot): TabIndex {
function buildTabIndex(tabsByWorktree: StoreSnapshot['tabsByWorktree']): TabIndex {
const index = new Map<string, WorktreeTab>()
for (const tabs of Object.values(state.tabsByWorktree ?? {})) {
for (const tabs of Object.values(tabsByWorktree ?? {})) {
for (const tab of tabs) {
// Why: first-wins to match the previous Array.flat().find() semantics
// exactly, even in the degenerate case of a tab id shared across worktrees.
@@ -51,11 +56,28 @@ function pruneClosedPaneCoordinators(): void {
// Why: hook-completion coordinators are module-scoped and may outlive a pane
// unless liveness changes from close/sleep paths evict them here.
if (coordinatorsByPaneKey.size === 0 && paneKeysRequiringFreshWorking.size === 0) {
lastPrunedLivenessSnapshot = null
return
}
const state = useAppStore.getState()
const livenessSnapshot: PaneCoordinatorLivenessSnapshot = {
tabsByWorktree: state.tabsByWorktree,
ptyIdsByTabId: state.ptyIdsByTabId,
terminalLayoutsByTabId: state.terminalLayoutsByTabId,
suppressedPtyExitIds: state.suppressedPtyExitIds
}
if (
lastPrunedLivenessSnapshot?.tabsByWorktree === livenessSnapshot.tabsByWorktree &&
lastPrunedLivenessSnapshot.ptyIdsByTabId === livenessSnapshot.ptyIdsByTabId &&
lastPrunedLivenessSnapshot.terminalLayoutsByTabId === livenessSnapshot.terminalLayoutsByTabId &&
lastPrunedLivenessSnapshot.suppressedPtyExitIds === livenessSnapshot.suppressedPtyExitIds
) {
return
}
lastPrunedLivenessSnapshot = livenessSnapshot
// Why: build the paneKey->tab index once for the whole pass instead of
// re-flattening tabsByWorktree inside paneCanReceiveHookCompletion per entry.
const tabIndex = buildTabIndex(useAppStore.getState())
const tabIndex = buildTabIndex(livenessSnapshot.tabsByWorktree)
for (const paneKey of coordinatorsByPaneKey.keys()) {
if (!paneCanReceiveHookCompletion(paneKey, tabIndex)) {
disposeCoordinatorForPaneKey(paneKey)
@@ -66,6 +88,9 @@ function pruneClosedPaneCoordinators(): void {
paneKeysRequiringFreshWorking.delete(paneKey)
}
}
if (coordinatorsByPaneKey.size === 0 && paneKeysRequiringFreshWorking.size === 0) {
lastPrunedLivenessSnapshot = null
}
}
function isAgentTaskCompleteNotificationEnabled(): boolean {
@@ -84,11 +109,10 @@ function isAgentTaskCompleteTrackingEnabled(): boolean {
export function syncAgentHookCompletionNotificationSettings(): boolean {
pruneClosedPaneCoordinators()
const enabled = isAgentTaskCompleteTrackingEnabled()
if (!enabled || (!wasAgentTaskCompleteTrackingEnabled && enabled)) {
if (enabled !== wasAgentTaskCompleteTrackingEnabled) {
requireFreshWorkingForNewTrackingCoordinators = true
for (const [paneKey, entry] of coordinatorsByPaneKey) {
for (const paneKey of coordinatorsByPaneKey.keys()) {
paneKeysRequiringFreshWorking.add(paneKey)
entry.coordinator.resetCompletionState({ requireFreshWorking: true })
}
}
wasAgentTaskCompleteTrackingEnabled = enabled
@@ -195,7 +219,11 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion
foregroundProcess: null,
hasChildProcesses: false
}),
dispatchHookLifecycle: (payload) => dispatchAgentHookTerminalLifecycle(paneKey, payload),
dispatchCompletion: (title, meta) => {
if (!isAgentTaskCompleteTrackingEnabled() || paneKeysRequiringFreshWorking.has(paneKey)) {
return
}
dispatchTerminalNotification(worktreeId, {
source: 'agent-task-complete',
terminalTitle: title,
@@ -205,6 +233,9 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion
})
},
dispatchAttention: (title, meta) => {
if (!isAgentTaskCompleteTrackingEnabled() || paneKeysRequiringFreshWorking.has(paneKey)) {
return
}
// Why: native notification settings still label this channel as "agent
// task complete"; the snapshot state makes the banner read "needs input".
dispatchTerminalNotification(worktreeId, {
@@ -234,13 +265,7 @@ export function observeAgentHookCompletionForNotification({
return
}
if (!syncAgentHookCompletionNotificationSettings()) {
paneKeysRequiringFreshWorking.add(paneKey)
coordinatorsByPaneKey
.get(paneKey)
?.coordinator.resetCompletionState({ requireFreshWorking: true })
return
}
const trackingEnabled = syncAgentHookCompletionNotificationSettings()
let entry = coordinatorsByPaneKey.get(paneKey)
if (!entry || entry.worktreeId !== worktreeId) {
@@ -254,14 +279,12 @@ export function observeAgentHookCompletionForNotification({
paneKeysRequiringFreshWorking.add(paneKey)
}
}
if (paneKeysRequiringFreshWorking.has(paneKey)) {
entry.coordinator.resetCompletionState({ requireFreshWorking: true })
}
entry.coordinator.observeHookStatus(payload)
if (payload.state === 'working') {
// Why: notification preferences may suppress alerts, but accepted hooks must
// still release pane-owned cursor/cache effects after the quiet window.
if (payload.state === 'working' && trackingEnabled) {
paneKeysRequiringFreshWorking.delete(paneKey)
}
entry.coordinator.observeHookStatus(payload)
}
export function resetAgentHookCompletionNotificationCoordinators(): void {
@@ -270,6 +293,7 @@ export function resetAgentHookCompletionNotificationCoordinators(): void {
}
coordinatorsByPaneKey.clear()
paneKeysRequiringFreshWorking.clear()
lastPrunedLivenessSnapshot = null
wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled()
requireFreshWorkingForNewTrackingCoordinators = !wasAgentTaskCompleteTrackingEnabled
}
@@ -5064,6 +5064,78 @@ describe('useIpcEvents agent status snapshot integration', () => {
expect(observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(1)
})
it('does not send an out-of-order hook event to completion lifecycle tracking', async () => {
const setAgentStatus = vi.fn()
const updateTabTitle = vi.fn()
const observeAgentHookCompletionForNotification = vi.fn()
const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = {
current: null
}
const storeState: StoreLike = buildStoreState({
setAgentStatus,
updateTabTitle,
workspaceSessionReady: true,
settings: { terminalFontSize: 13, notifications: { enabled: true, agentTaskComplete: true } },
tabsByWorktree: {
'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Claude' }]
},
agentStatusByPaneKey: {
[FUTURE_PANE_KEY]: {
state: 'working',
prompt: 'newer turn',
agentType: 'claude',
updatedAt: 1_700_000_000_500,
stateStartedAt: 1_700_000_000_400
}
}
})
stubReactSyncEffect()
vi.doMock('../store', () => ({
useAppStore: {
subscribe: vi.fn(() => () => {}),
getState: () => storeState
}
}))
vi.doMock('./agent-hook-completion-notifications', () => ({
observeAgentHookCompletionForNotification,
resetAgentHookCompletionNotificationCoordinators: vi.fn(),
syncAgentHookCompletionNotificationSettings: vi.fn()
}))
stubAuxiliaryModules()
vi.stubGlobal(
'window',
buildWindowApi({
onSet: (cb) => {
onSetListenerRef.current = cb
return () => {}
}
})
)
const { useIpcEvents } = await import('./useIpcEvents')
useIpcEvents()
await Promise.resolve()
if (typeof onSetListenerRef.current !== 'function') {
throw new Error('Expected agentStatus.onSet listener to be registered')
}
onSetListenerRef.current({
paneKey: FUTURE_PANE_KEY,
tabId: 'tab-future',
worktreeId: 'wt-1',
state: 'done',
prompt: 'older turn',
agentType: 'claude',
receivedAt: 1_700_000_000_300,
stateStartedAt: 1_700_000_000_200
})
expect(setAgentStatus).not.toHaveBeenCalled()
expect(updateTabTitle).not.toHaveBeenCalled()
expect(observeAgentHookCompletionForNotification).not.toHaveBeenCalled()
})
it('keeps auto-approved Codex done statuses on the completion path', async () => {
const setAgentStatus = vi.fn()
const observeAgentHookCompletionForNotification = vi.fn()
+5
View File
@@ -3027,6 +3027,11 @@ export function useIpcEvents(): void {
? { ...resolvedPayload, orchestration: data.orchestration }
: resolvedPayload
const existingStatus = store.agentStatusByPaneKey[data.paneKey]
if (existingStatus && data.receivedAt < existingStatus.updatedAt) {
// Why: the store rejects out-of-order status rows; keep notification and
// terminal lifecycle effects on the same accepted event boundary.
return 'dropped'
}
const identity = resolveAgentStatusIdentity({
existing: existingStatus
? {
+3 -1
View File
@@ -4327,7 +4327,9 @@
"1d0fa5c0a5": "Failed to delete workspace {{value0}}",
"ead7b8ee15": "It has changed files. Use Force Delete to delete it anyway.",
"905fc8efac": "Git already removed this workspace. Use Force Delete to clear it from Orca.",
"0899ebdb28": "Git already forgot this workspace, but its directory is still on disk. Use Force Delete to remove the orphaned directory."
"0899ebdb28": "Git already forgot this workspace, but its directory is still on disk. Use Force Delete to remove the orphaned directory.",
"locked": "This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.",
"lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion."
}
}
},
+3 -1
View File
@@ -4304,7 +4304,9 @@
"1d0fa5c0a5": "No se pudo eliminar el espacio de trabajo {{value0}}",
"ead7b8ee15": "Tiene archivos modificados. Usa Forzar eliminación para eliminarlo de todos modos.",
"905fc8efac": "Git ya eliminó este espacio de trabajo. Usa Forzar eliminación para borrarlo de Orca.",
"0899ebdb28": "Git ya olvidó este espacio de trabajo, pero su directorio todavía está en disco. Usa Forzar eliminación para eliminar el directorio huérfano."
"0899ebdb28": "Git ya olvidó este espacio de trabajo, pero su directorio todavía está en disco. Usa Forzar eliminación para eliminar el directorio huérfano.",
"locked": "This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.",
"lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion."
}
}
},
+3 -1
View File
@@ -4285,7 +4285,9 @@
"1d0fa5c0a5": "ワークスペース {{value0}} の削除に失敗しました",
"ead7b8ee15": "ファイルが変更されました。とにかく削除するには、強制削除を使用してください。",
"905fc8efac": "Git はこのワークスペースをすでに削除しています。 Orca から削除するには、強制削除を使用します。",
"0899ebdb28": "Git はすでにこのワークスペースを忘れていますが、そのディレクトリはまだディスク上にあります。孤立したディレクトリを削除するには、強制削除を使用します。"
"0899ebdb28": "Git はすでにこのワークスペースを忘れていますが、そのディレクトリはまだディスク上にあります。孤立したディレクトリを削除するには、強制削除を使用します。",
"locked": "This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.",
"lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion."
}
}
},
+3 -1
View File
@@ -4285,7 +4285,9 @@
"1d0fa5c0a5": "워크스페이스 {{value0}}을(를) 삭제하지 못했습니다.",
"ead7b8ee15": "파일이 변경되었습니다. 어쨌든 삭제하려면 강제 삭제를 사용하세요.",
"905fc8efac": "Git은 이미 이 워크스페이스를 제거했습니다. 강제 삭제를 사용하여 Orca에서 삭제합니다.",
"0899ebdb28": "Git은 이미 이 워크스페이스를 잊어버렸지만 해당 디렉토리는 여전히 디스크에 있습니다. 강제 삭제를 사용하여 분리된 디렉터리를 제거합니다."
"0899ebdb28": "Git은 이미 이 워크스페이스를 잊어버렸지만 해당 디렉토리는 여전히 디스크에 있습니다. 강제 삭제를 사용하여 분리된 디렉터리를 제거합니다.",
"locked": "This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.",
"lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion."
}
}
},
+3 -1
View File
@@ -4285,7 +4285,9 @@
"1d0fa5c0a5": "无法删除工作区 {{value0}}",
"ead7b8ee15": "它已更改文件。无论如何,请使用强制删除将其删除。",
"905fc8efac": "Git 已经删除了这个工作区。使用强制删除将其从 Orca 中清除。",
"0899ebdb28": "Git 已经忘记了这个工作区,但它的目录仍然在磁盘上。使用强制删除来删除孤立目录。"
"0899ebdb28": "Git 已经忘记了这个工作区,但它的目录仍然在磁盘上。使用强制删除来删除孤立目录。",
"locked": "This workspace is locked by Git. Run git worktree unlock <worktree-path> from its repository, then retry deletion.",
"lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Run git worktree unlock <worktree-path> from its repository, then retry deletion."
}
}
},
@@ -130,7 +130,12 @@ describe('removeWorktree cascade', () => {
tab2: makeLayout()
},
deleteStateByWorktreeId: {
[worktreeId]: { isDeleting: false, error: null, canForceDelete: false }
[worktreeId]: {
isDeleting: false,
error: null,
canForceDelete: false,
forceDeleteReason: null
}
},
fileSearchStateByWorktree: {
[worktreeId]: {
@@ -272,7 +277,8 @@ describe('removeWorktree cascade', () => {
expect(s.deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: true
canForceDelete: true,
forceDeleteReason: 'dirty'
})
// State NOT cleaned up
expect(s.worktreesByRepo['repo1']).toHaveLength(1)
@@ -289,7 +295,12 @@ describe('removeWorktree cascade', () => {
seedStore(store, {
deleteStateByWorktreeId: {
[first]: { isDeleting: false, error: 'old failure', canForceDelete: true }
[first]: {
isDeleting: false,
error: 'old failure',
canForceDelete: true,
forceDeleteReason: 'dirty'
}
}
})
@@ -308,7 +319,12 @@ describe('removeWorktree cascade', () => {
seedStore(store, {
deleteStateByWorktreeId: {
[first]: { isDeleting: false, error: 'old failure', canForceDelete: true }
[first]: {
isDeleting: false,
error: 'old failure',
canForceDelete: true,
forceDeleteReason: 'dirty'
}
}
})
@@ -327,7 +343,13 @@ describe('removeWorktree cascade', () => {
seedStore(store, {
deleteStateByWorktreeId: {
[active]: { isDeleting: true, phase: 'deleting', error: null, canForceDelete: false }
[active]: {
isDeleting: true,
phase: 'deleting',
error: null,
canForceDelete: false,
forceDeleteReason: null
}
}
})
@@ -362,7 +384,8 @@ describe('removeWorktree cascade', () => {
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: true
canForceDelete: true,
forceDeleteReason: 'dirty'
})
})
@@ -389,7 +412,37 @@ describe('removeWorktree cascade', () => {
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: true
canForceDelete: true,
forceDeleteReason: 'dirty'
})
})
it('does not offer force delete for locked worktree removal errors', async () => {
const store = createTestStore()
const worktreeId = 'repo1::/workspace/feature-wt'
const error =
"fatal: cannot remove a locked working tree, lock reason: claude session\nuse 'remove -f -f' to override or unlock first"
mockApi.worktrees.remove.mockRejectedValueOnce(new Error(error))
seedStore(store, {
worktreesByRepo: {
repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1' })]
},
tabsByWorktree: {},
ptyIdsByTabId: {},
terminalLayoutsByTabId: {}
})
const result = await store.getState().removeWorktree(worktreeId)
expect(result).toEqual({ ok: false, error })
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: false,
forceDeleteReason: null,
lockReason: null
})
})
@@ -416,7 +469,8 @@ describe('removeWorktree cascade', () => {
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: true
canForceDelete: true,
forceDeleteReason: 'missing-registration'
})
})
@@ -442,7 +496,8 @@ describe('removeWorktree cascade', () => {
expect(s.deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error: 'fatal error',
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
})
})
@@ -476,7 +531,8 @@ describe('removeWorktree cascade', () => {
isDeleting: false,
error:
'Refusing to delete worktree because it contains another registered worktree: /path/wt1/child',
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
})
})
@@ -528,7 +584,8 @@ describe('removeWorktree cascade', () => {
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
})
})
@@ -575,7 +632,8 @@ describe('removeWorktree cascade', () => {
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
isDeleting: false,
error,
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
})
expect(mockApi.worktrees.remove).not.toHaveBeenCalled()
}
@@ -20,6 +20,7 @@ import type {
WorktreeMeta,
WorkspaceKey
} from '../../../../shared/types'
import type { WorktreeForceDeleteReason } from '../../../../shared/worktree-removal'
import type { TerminalGitHubPRLink } from '../../../../shared/terminal-github-pr-link-detector'
import type {
PendingWorktreeCreation,
@@ -33,6 +34,8 @@ export type WorktreeDeleteState = {
phase?: 'deleting' | 'queued'
error: string | null
canForceDelete: boolean
forceDeleteReason: WorktreeForceDeleteReason | null
lockReason?: string | null
}
export type WorktreeMetaUpdateGuard = (worktree: Worktree | DetectedWorktree | undefined) => boolean
@@ -182,7 +185,10 @@ export type WorktreeSlice = {
// 'forget-local' drops the workspace from Orca only (no remote Git/FS work)
// for workspaces pinned to a removed/disconnected SSH host. Reuses the same
// renderer-side teardown/purge as a normal remove.
options?: { mode?: 'remove' | 'forget-local'; suppressPreservedBranchToast?: boolean }
options?: {
mode?: 'remove' | 'forget-local'
suppressPreservedBranchToast?: boolean
}
) => Promise<({ ok: true } & RemoveWorktreeResult) | { ok: false; error: string }>
markWorktreesDeleting: (worktreeIds: readonly string[]) => void
markWorktreesQueuedForDeletion: (worktreeIds: readonly string[]) => void
+21 -29
View File
@@ -66,6 +66,11 @@ import {
worktreeWorkspaceKey
} from '../../../../shared/workspace-scope'
import { folderWorkspaceToWorktree } from '../../../../shared/folder-workspace-worktree'
import {
classifyWorktreeForceDeleteReason,
getLockedWorktreeRemovalReason,
isLockedWorktreeRemovalError
} from '../../../../shared/worktree-removal'
export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers'
// Why: old runtime servers only have `worktree.list`; preserve the large-list
@@ -292,30 +297,6 @@ function toVisibleTabType(contentType: string): WorkspaceVisibleTabType {
return 'editor'
}
const FORCE_RETRYABLE_WORKTREE_REMOVAL_MESSAGES = [
'Worktree has uncommitted or untracked changes',
'contains modified or untracked files',
'Worktree is no longer registered with Git but its directory remains',
'Worktree is no longer registered with Git and its directory is already gone'
] as const
// Why: local preflight formatting can surface raw git porcelain instead of the
// friendly dirty-worktree message; only those status prefixes are forceable.
const FORMATTED_DIRTY_WORKTREE_REMOVAL_PATTERN =
/Failed to delete worktree at [^\n]*\.\s*(?:(?:[MADRCUT][ MADRCUT]| [MADRCUT]|\?\?)\s+\S)/
function canRetryWorktreeRemovalWithForce(error: string, force: boolean | undefined): boolean {
if (force) {
return false
}
// Why: force only helps backend safety refusals that are explicitly safe to
// retry with user confirmation; transport/provider errors need recovery first.
return (
FORCE_RETRYABLE_WORKTREE_REMOVAL_MESSAGES.some((message) => error.includes(message)) ||
FORMATTED_DIRTY_WORKTREE_REMOVAL_PATTERN.test(error)
)
}
type WorktreeWithLineage = Worktree & {
parentWorktreeId?: string | null
childWorktreeIds?: string[]
@@ -3195,7 +3176,8 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
isDeleting: true,
phase: 'deleting',
error: null,
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
}
}
}))
@@ -3237,7 +3219,11 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
: callRuntimeRpc<RemoveWorktreeResult>(
target,
'worktree.rm',
{ worktree: toRuntimeWorktreeSelector(worktreeId), force, runHooks: !skipArchive },
{
worktree: toRuntimeWorktreeSelector(worktreeId),
force,
runHooks: !skipArchive
},
{ timeoutMs: 60_000 }
))
@@ -3578,13 +3564,17 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
// handled user decision point surfaced by the delete toast, not an app error.
console.warn('Failed to remove worktree:', err)
const error = err instanceof Error ? err.message : String(err)
const forceDeleteReason = classifyWorktreeForceDeleteReason(error, force)
const locked = isLockedWorktreeRemovalError(error)
set((s) => ({
deleteStateByWorktreeId: {
...s.deleteStateByWorktreeId,
[worktreeId]: {
isDeleting: false,
error,
canForceDelete: canRetryWorktreeRemovalWithForce(error, force)
canForceDelete: forceDeleteReason !== null,
forceDeleteReason,
...(locked ? { lockReason: getLockedWorktreeRemovalReason(error) } : {})
}
}
}))
@@ -3608,7 +3598,8 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
isDeleting: true,
phase: 'deleting',
error: null,
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
}
changed = true
}
@@ -3632,7 +3623,8 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
isDeleting: true,
phase: 'queued',
error: null,
canForceDelete: false
canForceDelete: false,
forceDeleteReason: null
}
changed = true
}
@@ -1699,6 +1699,60 @@ describe('web worktree preload API', () => {
vi.doUnmock('./web-runtime-client')
})
it('forwards force and archive-hook intent through worktree removal', async () => {
const runtimeCalls: { method: string; params: unknown }[] = []
vi.doMock('./web-runtime-client', () => ({
WebRuntimeClient: class {
call(method: string, params?: unknown): Promise<RuntimeRpcResponse<unknown>> {
runtimeCalls.push({ method, params })
return Promise.resolve({
id: `call-${runtimeCalls.length}`,
ok: true,
result: { removed: true },
_meta: { runtimeId: 'runtime-1' }
})
}
close(): void {}
}
}))
const globals = installBrowserGlobals('Linux')
writeStoredRuntimeEnvironment(globals.storage)
const { installWebPreloadApi } = await import('./web-preload-api')
installWebPreloadApi()
await globals.window.api.worktrees.remove({
worktreeId: 'repo-1::/workspace/locked',
force: true,
skipArchive: false
})
await globals.window.api.worktrees.remove({
worktreeId: 'repo-1::/workspace/dirty',
force: true,
skipArchive: true
})
expect(runtimeCalls).toEqual([
{
method: 'worktree.rm',
params: {
worktree: 'id:repo-1::/workspace/locked',
force: true,
runHooks: true
}
},
{
method: 'worktree.rm',
params: {
worktree: 'id:repo-1::/workspace/dirty',
force: true,
runHooks: false
}
}
])
})
it('falls back to legacy worktree.list when detectedList is unavailable', async () => {
const runtimeCalls: { method: string; params: unknown }[] = []
const worktree = {
+3 -2
View File
@@ -1405,11 +1405,12 @@ function createWorktreesApi(): NonNullable<Partial<PreloadApi>['worktrees']> {
targetBranch,
isCrossRepository
}),
remove: async ({ worktreeId, force }) => {
remove: async ({ worktreeId, force, skipArchive }) => {
invalidateRuntimeWorktreeCaches()
return callRuntimeResult<RemoveWorktreeResult>('worktree.rm', {
worktree: toRuntimeWorktreeSelector(worktreeId),
force
force,
runHooks: skipArchive !== true
})
},
// Why: forget-locally clears a workspace pinned to a disconnected/removed
+2 -1
View File
@@ -1,5 +1,6 @@
import {
AGENT_STATUS_STALE_AFTER_MS,
isFreshNonDoneAgentStatus,
type AgentStatusState,
type AgentType
} from './agent-status-types'
@@ -36,7 +37,7 @@ function isActiveExistingIdentity(
now: number,
staleAfterMs: number
): boolean {
return existing.state !== 'done' && now - existing.updatedAt <= staleAfterMs
return isFreshNonDoneAgentStatus(existing, now, staleAfterMs)
}
export function resolveAgentStatusIdentity(args: {
+9
View File
@@ -223,6 +223,15 @@ export const AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH = 16000
* dashboard + hover only display hook-reported data as-is.
*/
export const AGENT_STATUS_STALE_AFTER_MS = 30 * 60 * 1000
export function isFreshNonDoneAgentStatus(
entry: Pick<AgentStatusEntry, 'state' | 'updatedAt'> | undefined,
now = Date.now(),
staleAfterMs = AGENT_STATUS_STALE_AFTER_MS
): boolean {
return Boolean(entry && entry.state !== 'done' && now - entry.updatedAt <= staleAfterMs)
}
const SINGLE_LINE_FIELD_SCAN_OVERHEAD = 64
const SINGLE_LINE_FIELD_SCAN_MULTIPLIER = 8
+12
View File
@@ -0,0 +1,12 @@
import { describe, expect, it } from 'vitest'
import { decodeGitCQuotedPath } from './git-cquoted-path'
describe('decodeGitCQuotedPath', () => {
it('preserves an octal UTF-8 BOM instead of treating it as Git framing', () => {
expect(decodeGitCQuotedPath('"\\357\\273\\277name"')).toBe('\uFEFFname')
})
it('decodes adjacent UTF-8 octal bytes after a BOM', () => {
expect(decodeGitCQuotedPath('"\\357\\273\\277\\343\\201\\202"')).toBe('\uFEFFあ')
})
})
+22 -9
View File
@@ -41,16 +41,29 @@ export function decodeGitCQuotedPath(value: string): string {
break
default:
if (/[0-7]/.test(escaped)) {
let octal = escaped
while (
index + 1 < value.length - 1 &&
octal.length < 3 &&
/[0-7]/.test(value[index + 1])
) {
index += 1
octal += value[index]
const bytes: number[] = []
let octalStart = index
while (octalStart < value.length - 1) {
let octal = value[octalStart]
let octalEnd = octalStart
while (
octalEnd + 1 < value.length - 1 &&
octal.length < 3 &&
/[0-7]/.test(value[octalEnd + 1])
) {
octalEnd += 1
octal += value[octalEnd]
}
bytes.push(Number.parseInt(octal, 8))
index = octalEnd
if (value[index + 1] !== '\\' || !/[0-7]/.test(value[index + 2] ?? '')) {
break
}
octalStart = index + 2
}
decoded += String.fromCharCode(Number.parseInt(octal, 8))
// Why: Git C-quotes non-ASCII text as adjacent UTF-8 octal bytes;
// decoding each byte as a character corrupts localized lock reasons.
decoded += new TextDecoder('utf-8', { ignoreBOM: true }).decode(Uint8Array.from(bytes))
} else {
decoded += escaped
}
+2
View File
@@ -427,6 +427,8 @@ export type GitWorktreeInfo = {
branch: string
isBare: boolean
isSparse?: boolean
locked?: boolean
lockReason?: string
/** True for the repo's main working tree (the first entry from `git worktree list`).
* Linked worktrees created via `git worktree add` have this set to false. */
isMainWorktree: boolean
+75
View File
@@ -0,0 +1,75 @@
import type { GitWorktreeInfo } from './types'
export const LOCKED_WORKTREE_REMOVAL_PREFIX = 'Worktree is locked by Git.'
export type WorktreeForceDeleteReason = 'dirty' | 'orphan-directory' | 'missing-registration'
export function createLockedWorktreeRemovalError(lockReason?: string): Error {
const reason = lockReason?.trim()
return new Error(
reason
? `${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: ${reason}. Run git worktree unlock <worktree-path> from its repository, then retry deletion.`
: `${LOCKED_WORKTREE_REMOVAL_PREFIX} Run git worktree unlock <worktree-path> from its repository, then retry deletion.`
)
}
export function assertWorktreeUnlockedForRemoval(
worktree: Pick<GitWorktreeInfo, 'locked' | 'lockReason'> | undefined
): void {
if (worktree?.locked) {
throw createLockedWorktreeRemovalError(worktree.lockReason)
}
}
export function isLockedWorktreeRemovalError(error: string): boolean {
return (
error.includes(LOCKED_WORKTREE_REMOVAL_PREFIX) ||
error.includes('cannot remove a locked working tree')
)
}
export function getLockedWorktreeRemovalReason(error: string): string | null {
const prefixIndex = error.indexOf(`${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: `)
if (prefixIndex === -1) {
return null
}
const reasonStart = prefixIndex + `${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: `.length
const recoverySuffix =
'. Run git worktree unlock <worktree-path> from its repository, then retry deletion.'
const suffixIndex = error.indexOf(recoverySuffix, reasonStart)
const reason = error.slice(reasonStart, suffixIndex === -1 ? undefined : suffixIndex).trim()
return reason || null
}
const FORMATTED_DIRTY_WORKTREE_REMOVAL_PATTERN =
/Failed to delete worktree at [^\n]*\.\s*(?:(?:[MADRCUT][ MADRCUT]| [MADRCUT]|\?\?)\s+\S)/
export function classifyWorktreeForceDeleteReason(
error: string,
force = false
): WorktreeForceDeleteReason | null {
if (isLockedWorktreeRemovalError(error)) {
// Why: a Git lock can represent an external safety contract. It must be
// unlocked explicitly rather than folded into Orca's dirty-file force path.
return null
}
if (force) {
return null
}
if (error.includes('Worktree is no longer registered with Git but its directory remains')) {
return 'orphan-directory'
}
if (
error.includes('Worktree is no longer registered with Git and its directory is already gone')
) {
return 'missing-registration'
}
if (
error.includes('Worktree has uncommitted or untracked changes') ||
error.includes('contains modified or untracked files') ||
FORMATTED_DIRTY_WORKTREE_REMOVAL_PATTERN.test(error)
) {
return 'dirty'
}
return null
}