mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(worktree): block removal when the archive hook fails (#20153)
* fix(worktree): block removal when the archive hook fails A repo's orca.yaml archive hook is the user's last chance to save work off a checkout Orca is about to delete. A failed hook was logged as advisory and stepped over, so the removal went ahead with nothing archived — and the caller could still be told it succeeded. The hook is now a blocking precondition, evaluated while the checkout, its Git registration, its agents and Orca's ownership evidence are all still intact: it sits ahead of the registration re-read, the lock/dirty preflights, stopPtys() and removeWorktree in every orchestrator that runs it. Failure is typed (worktree_archive_hook_failed) and carries the worktree path, outcome, exit code where one was observed, and the hook's output. unverifiable stays distinct from exited, so loss of contact is never read as a pass. The waiver rides its own field at every layer and is never implied by --force, which already carries the PTY-stop waiver; when used, the waived failure comes back on result.archiveHookOverride rather than being swallowed. worktree.archive-failure-blocking.v1 is advertised so an integration can tell "accepts --run-hooks" from "safely propagates a failing hook" without risking the data loss to find out. The runtime's SSH path cannot run a hook at all, so rather than delete with the archive step silently skipped it refuses — waivable like every other refusal here. #18563 retires that gate by making the path run the hook for real. Stacked on #20559, which makes a timed-out hook report honestly; without it a hook that traps SIGTERM and exits 0 would defeat this gate. Fixes #19334 * fix(worktree): close the skip-confirm dead end and the client/hook timeout gap Four review findings on the gate. A retry from the failure toast could fail for a DIFFERENT reason than the one the user had just answered, and that second failure got a bare toast with no buttons. With skipDeleteWorktreeConfirm set, the delete helpers pass no force, so waiving a failed archive hook on a dirty checkout landed on the dirty preflight and stopped there. Retry failures now re-enter the same failure toast, so every retry stays as actionable as the first attempt. Third instance of this class. The renderer gave worktree.rm a 60s budget while an archive hook may run for 120s. A hook that took 90s and succeeded timed the client out and reported failure while the host went on to delete — telling the user their delete failed and their checkout was gone. The budget is now derived from the hook's, and only when a hook can run. The SSH fail-open is logged rather than silent, and the capability's doc comment scopes what it claims: a hook that RUNS and fails cannot delete the checkout; it is not a promise the hook was found. The SSH owner-resolution test now reads a real remote orca.yaml through a stubbed provider and asserts the returned script is the remote one. It previously stopped at the lookup key, which is the coverage that let this path break twice. It fails against the row-only resolution. * fix(worktree): name a signalled hook exit, and state why prunable cleanup skips the gate Two things the rebase onto #20617 and #20576 surfaced, both found by rerunning the real-repo harness rather than by reading the diff. - #20617 added a registration-cleanup branch that returns before the archive gate. That ordering is correct — both of its arms describe a row with no checkout behind it, so there is nothing to archive and running the hook would fail on the missing cwd — but the gate's ordering invariant is documented, so the exception should be too. - A signalled hook reported `Command failed with exit code null.`, which reads as a reporting glitch rather than the `unverifiable` verdict it is about to produce. It now says the command was terminated without reporting an exit code. Introduced by #20576; the withheld `exitCode` itself was always right. Fixes #19334
This commit is contained in:
Neil
@@ -0,0 +1,526 @@
|
||||
/**
|
||||
* Real-repo verification for #19334 — run with:
|
||||
* node config/scripts/archive-hook-removal-repro.mjs
|
||||
*
|
||||
* Requires a prior `build:cli` and `build:electron-vite`; it drives the BUILT CLI against the
|
||||
* BUILT headless runtime, so it proves the shipped artifacts rather than the test harness.
|
||||
*: a failed archive hook must BLOCK a destructive
|
||||
* worktree removal, and the checkout, its git registration and its files must all survive.
|
||||
*
|
||||
* Boots the BUILT headless runtime (`out/main/index.js --serve`), pairs the BUILT CLI to it,
|
||||
* and drives `orca worktree rm` end to end against real git worktrees on disk.
|
||||
*/
|
||||
import { spawn, spawnSync } from 'node:child_process'
|
||||
import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, dirname, resolve } from 'node:path'
|
||||
import { randomBytes } from 'node:crypto'
|
||||
|
||||
const projectDir = resolve(import.meta.dirname, '../..')
|
||||
const serveEntry = join(projectDir, 'out', 'main', 'index.js')
|
||||
const cliEntry = join(projectDir, 'out', 'cli', 'index.js')
|
||||
const PORT = 6900 + Math.floor(Math.random() * 400)
|
||||
const READY_TIMEOUT_MS = 180_000
|
||||
|
||||
const control = mkdtempSync(join(tmpdir(), 'agh-control-'))
|
||||
const modeFile = join(control, 'mode')
|
||||
const ranFile = join(control, 'ran')
|
||||
const setMode = (m) => writeFileSync(modeFile, m)
|
||||
const hookRuns = () => (existsSync(ranFile) ? readFileSync(ranFile, 'utf8').trim().split('\n') : [])
|
||||
|
||||
let failures = 0
|
||||
const out = (s) => process.stdout.write(`${s}\n`)
|
||||
const banner = (s) => out(`\n${'='.repeat(78)}\n${s}\n${'='.repeat(78)}`)
|
||||
function check(label, ok, detail = '') {
|
||||
out(` ${ok ? 'PASS' : 'FAIL'} ${label}${detail ? ` -- ${detail}` : ''}`)
|
||||
if (!ok) {
|
||||
failures++
|
||||
}
|
||||
}
|
||||
|
||||
let pairingCode = null
|
||||
|
||||
/** Run the real CLI against the booted server. Returns the raw process result. */
|
||||
function cli(args, { json = true } = {}) {
|
||||
return spawnSync(
|
||||
process.execPath,
|
||||
[cliEntry, ...args, '--pairing-code', pairingCode, ...(json ? ['--json'] : [])],
|
||||
{ encoding: 'utf8', shell: false }
|
||||
)
|
||||
}
|
||||
|
||||
/** Run the CLI and require success, returning result payload. */
|
||||
function ok(args) {
|
||||
const r = cli(args)
|
||||
const parsed = parseJsonLine(r)
|
||||
if (!parsed) {
|
||||
throw new Error(`orca ${args.join(' ')} produced no JSON:\n${r.stdout}\n${r.stderr}`)
|
||||
}
|
||||
if (parsed.ok === false) {
|
||||
throw new Error(`orca ${args.join(' ')} failed: ${parsed.error?.code} ${parsed.error?.message}`)
|
||||
}
|
||||
return parsed.result
|
||||
}
|
||||
|
||||
/** The CLI pretty-prints one JSON document to stdout. */
|
||||
function parseJsonLine(r) {
|
||||
const text = (r.stdout ?? '').trim()
|
||||
const start = text.indexOf('{')
|
||||
if (start === -1) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
return JSON.parse(text.slice(start))
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function git(cwd, ...args) {
|
||||
const r = spawnSync('git', args, { cwd, encoding: 'utf8' })
|
||||
if (r.status !== 0) {
|
||||
throw new Error(`git ${args.join(' ')}: ${r.stderr || r.stdout}`)
|
||||
}
|
||||
return r.stdout
|
||||
}
|
||||
|
||||
const ARCHIVE_HOOK = `echo "[archive-hook] running in $PWD"
|
||||
echo "$PWD" >> ${JSON.stringify(ranFile).slice(1, -1)}
|
||||
mode=$(cat ${JSON.stringify(modeFile).slice(1, -1)})
|
||||
case "$mode" in
|
||||
ok) echo "[archive-hook] archived OK"; exit 0 ;;
|
||||
fail) echo "[archive-hook] backup target unreachable" >&2; exit 23 ;;
|
||||
signal) echo "[archive-hook] losing the execution host now"; kill -KILL $$ ;;
|
||||
esac
|
||||
echo "unknown mode $mode" >&2; exit 99
|
||||
`
|
||||
|
||||
/** A throwaway git repo with one commit; optionally an orca.yaml archive hook. */
|
||||
function seedGitRepo(label, withHook, githubSlug) {
|
||||
const dir = mkdtempSync(join(tmpdir(), `agh-repo-${label}-`))
|
||||
writeFileSync(join(dir, 'README.md'), `# ${label}\n`)
|
||||
if (withHook) {
|
||||
writeFileSync(
|
||||
join(dir, 'orca.yaml'),
|
||||
`scripts:\n archive: |\n${ARCHIVE_HOOK.split('\n')
|
||||
.map((l) => ` ${l}`)
|
||||
.join('\n')}\n`
|
||||
)
|
||||
}
|
||||
git(dir, 'init', '-b', 'main')
|
||||
git(dir, 'config', 'user.email', 'verify@orca.test')
|
||||
git(dir, 'config', 'user.name', 'Archive Gate Verify')
|
||||
if (githubSlug) {
|
||||
git(dir, 'remote', 'add', 'origin', `https://github.com/agh-owner/${githubSlug}.git`)
|
||||
}
|
||||
git(dir, 'add', '-A')
|
||||
git(dir, 'commit', '-m', 'seed')
|
||||
return dir
|
||||
}
|
||||
|
||||
function waitForReady(child) {
|
||||
return new Promise((res, rej) => {
|
||||
let buffered = ''
|
||||
let serverErr = ''
|
||||
const timer = setTimeout(
|
||||
() => rej(new Error(`no ready payload in ${READY_TIMEOUT_MS}ms\n${serverErr}`)),
|
||||
READY_TIMEOUT_MS
|
||||
)
|
||||
child.stderr.setEncoding('utf8')
|
||||
child.stderr.on('data', (c) => {
|
||||
serverErr += c
|
||||
})
|
||||
child.stdout.setEncoding('utf8')
|
||||
child.stdout.on('data', (chunk) => {
|
||||
buffered += chunk
|
||||
for (const line of buffered.split('\n')) {
|
||||
if (!line.startsWith('{')) {
|
||||
continue
|
||||
}
|
||||
try {
|
||||
const p = JSON.parse(line)
|
||||
if (p.type === 'orca_server_ready') {
|
||||
clearTimeout(timer)
|
||||
res(p)
|
||||
return
|
||||
}
|
||||
} catch {
|
||||
/* partial */
|
||||
}
|
||||
}
|
||||
})
|
||||
child.on('exit', (code) => {
|
||||
clearTimeout(timer)
|
||||
rej(new Error(`server exited ${code} before ready:\n${serverErr}`))
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
/** Filesystem + git truth about a worktree, read directly rather than through Orca. */
|
||||
function evidence(repoPath, wtPath) {
|
||||
const ls = spawnSync('ls', ['-la', wtPath], { encoding: 'utf8' })
|
||||
const list = spawnSync('git', ['worktree', 'list'], { cwd: repoPath, encoding: 'utf8' })
|
||||
return {
|
||||
dirExists: existsSync(wtPath),
|
||||
fileExists: existsSync(join(wtPath, 'PRECIOUS.txt')),
|
||||
fileBody: existsSync(join(wtPath, 'PRECIOUS.txt'))
|
||||
? readFileSync(join(wtPath, 'PRECIOUS.txt'), 'utf8').trim()
|
||||
: null,
|
||||
registered: (list.stdout ?? '').includes(wtPath),
|
||||
ls: (ls.stdout ?? '').trim(),
|
||||
worktreeList: (list.stdout ?? '').trim()
|
||||
}
|
||||
}
|
||||
|
||||
function showEvidence(e) {
|
||||
out(' --- ls -la <worktree> ---')
|
||||
out(
|
||||
e.ls
|
||||
.split('\n')
|
||||
.map((l) => ` ${l}`)
|
||||
.join('\n')
|
||||
)
|
||||
out(' --- git worktree list (in the repo) ---')
|
||||
out(
|
||||
e.worktreeList
|
||||
.split('\n')
|
||||
.map((l) => ` ${l}`)
|
||||
.join('\n')
|
||||
)
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const userDataDir = mkdtempSync(join(tmpdir(), 'agh-userdata-'))
|
||||
out(`booting headless runtime on port ${PORT}, userData ${userDataDir}`)
|
||||
const child = spawn(
|
||||
'npx',
|
||||
[
|
||||
'electron',
|
||||
serveEntry,
|
||||
'--serve',
|
||||
'--serve-port',
|
||||
String(PORT),
|
||||
'--serve-json',
|
||||
`--user-data-dir=${userDataDir}`
|
||||
],
|
||||
{
|
||||
cwd: projectDir,
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
env: { ...process.env, ORCA_BACKGROUND_LAUNCH: '1' }
|
||||
}
|
||||
)
|
||||
const created = []
|
||||
|
||||
try {
|
||||
const ready = await waitForReady(child)
|
||||
pairingCode = new URL(ready.pairing.url).searchParams.get('code')
|
||||
out(`ready: ${ready.advertisedEndpoint}`)
|
||||
|
||||
// ---------------------------------------------------------------- setup
|
||||
const hookRepoPath = seedGitRepo('hooked', true)
|
||||
const folderProjectSlug = `agh-folder-proof-${randomBytes(3).toString('hex')}`
|
||||
const bareRepoPath = seedGitRepo('nohook', false, folderProjectSlug)
|
||||
const hookRepo = ok(['repo', 'add', '--path', hookRepoPath]).repo
|
||||
const bareRepo = ok(['repo', 'add', '--path', bareRepoPath]).repo
|
||||
out(`repo with archive hook: ${hookRepoPath} (${hookRepo.id})`)
|
||||
out(`repo without archive hook: ${bareRepoPath} (${bareRepo.id})`)
|
||||
|
||||
const makeWorktree = (repo, repoPath, name) => {
|
||||
const wt = ok([
|
||||
'worktree',
|
||||
'create',
|
||||
'--repo',
|
||||
`id:${repo.id}`,
|
||||
'--name',
|
||||
name,
|
||||
'--setup',
|
||||
'skip'
|
||||
]).worktree
|
||||
created.push(wt)
|
||||
const unarchivedBody = `unarchived work for ${name}`
|
||||
writeFileSync(join(wt.path, 'PRECIOUS.txt'), `${unarchivedBody}\n`)
|
||||
return { ...wt, repoPath, unarchivedBody }
|
||||
}
|
||||
|
||||
// ============================================================ SCENARIO 1
|
||||
banner('SCENARIO 1 — archive hook exits 23: removal MUST be refused, nothing deleted')
|
||||
setMode('fail')
|
||||
const wt1 = makeWorktree(hookRepo, hookRepoPath, `gate-fail-${randomBytes(3).toString('hex')}`)
|
||||
out(`worktree: ${wt1.path}`)
|
||||
const before = evidence(wt1.repoPath, wt1.path)
|
||||
|
||||
out('\n$ orca worktree rm --worktree <id> --run-hooks (human output)')
|
||||
const human = cli(['worktree', 'rm', '--worktree', wt1.id, '--run-hooks'], { json: false })
|
||||
out(` exit code: ${human.status}`)
|
||||
out(' --- stdout ---')
|
||||
out(
|
||||
(human.stdout ?? '')
|
||||
.trimEnd()
|
||||
.split('\n')
|
||||
.map((l) => ` ${l}`)
|
||||
.join('\n')
|
||||
)
|
||||
out(' --- stderr ---')
|
||||
out(
|
||||
(human.stderr ?? '')
|
||||
.trimEnd()
|
||||
.split('\n')
|
||||
.map((l) => ` ${l}`)
|
||||
.join('\n')
|
||||
)
|
||||
|
||||
out(
|
||||
'\n$ orca worktree rm --worktree <id> --force --run-hooks --json (--force must NOT waive)'
|
||||
)
|
||||
const forced = cli(['worktree', 'rm', '--worktree', wt1.id, '--force', '--run-hooks'])
|
||||
const forcedJson = parseJsonLine(forced)
|
||||
out(` exit code: ${forced.status}`)
|
||||
out(` ${JSON.stringify(forcedJson)}`)
|
||||
|
||||
const after1 = evidence(wt1.repoPath, wt1.path)
|
||||
showEvidence(after1)
|
||||
|
||||
check('CLI exits non-zero', human.status !== 0, `got ${human.status}`)
|
||||
check(
|
||||
'human stderr names the archive hook',
|
||||
/Archive hook failed for worktree/.test(human.stderr ?? '')
|
||||
)
|
||||
check('--force also refused (non-zero)', forced.status !== 0, `got ${forced.status}`)
|
||||
check(
|
||||
'typed error code',
|
||||
forcedJson?.error?.code === 'worktree_archive_hook_failed',
|
||||
JSON.stringify(forcedJson?.error?.code)
|
||||
)
|
||||
check(
|
||||
"error data outcome is 'exited'",
|
||||
forcedJson?.error?.data?.outcome === 'exited',
|
||||
JSON.stringify(forcedJson?.error?.data)
|
||||
)
|
||||
check('error data carries exitCode 23', forcedJson?.error?.data?.exitCode === 23)
|
||||
check('checkout directory still exists', after1.dirExists)
|
||||
// Assert the CONTENTS, not just the path: a file that survived as an empty stub would prove
|
||||
// nothing about the work the archive hook was supposed to rescue.
|
||||
check(
|
||||
'unarchived file PRECIOUS.txt survives with its contents',
|
||||
after1.fileExists && after1.fileBody === wt1.unarchivedBody,
|
||||
`exists=${after1.fileExists} body=${JSON.stringify(after1.fileBody)}`
|
||||
)
|
||||
check('git worktree registration survives', after1.registered)
|
||||
check(
|
||||
'nothing changed vs. before the attempt',
|
||||
before.dirExists === after1.dirExists && before.registered === after1.registered
|
||||
)
|
||||
const shown = ok(['worktree', 'show', '--worktree', wt1.id]).worktree
|
||||
check('Orca still resolves the worktree', shown?.id === wt1.id)
|
||||
check(
|
||||
'the hook really ran (twice: plain + --force)',
|
||||
hookRuns().length >= 2,
|
||||
`runs=${hookRuns().length}`
|
||||
)
|
||||
// The checkout is dirty (untracked PRECIOUS.txt). The plain run reported the ARCHIVE failure,
|
||||
// not the dirty-preflight failure, so the gate is evaluated before that preflight.
|
||||
check(
|
||||
'archive gate precedes the dirty preflight (dirty checkout, archive error reported)',
|
||||
/Archive hook failed/.test(human.stderr ?? '') &&
|
||||
!/\?\? PRECIOUS\.txt/.test(human.stderr ?? '')
|
||||
)
|
||||
|
||||
// ============================================================ SCENARIO 2
|
||||
banner('SCENARIO 2 — --allow-failed-archive-hook: removal proceeds, waiver recorded')
|
||||
// --force here waives only the DIRTY preflight (PRECIOUS.txt is untracked on purpose);
|
||||
// scenario 1 already proved it does not waive the archive gate.
|
||||
const waived = cli([
|
||||
'worktree',
|
||||
'rm',
|
||||
'--worktree',
|
||||
wt1.id,
|
||||
'--force',
|
||||
'--run-hooks',
|
||||
'--allow-failed-archive-hook'
|
||||
])
|
||||
const waivedJson = parseJsonLine(waived)
|
||||
out(` exit code: ${waived.status}`)
|
||||
out(` ${JSON.stringify(waivedJson)}`)
|
||||
const after2 = evidence(wt1.repoPath, wt1.path)
|
||||
out(` checkout still on disk: ${after2.dirExists}`)
|
||||
out(
|
||||
` --- git worktree list ---\n${after2.worktreeList
|
||||
.split('\n')
|
||||
.map((l) => ` ${l}`)
|
||||
.join('\n')}`
|
||||
)
|
||||
check('override exits zero', waived.status === 0, `got ${waived.status}`)
|
||||
check('removal reported', waivedJson?.result?.removed === true)
|
||||
check('checkout is GONE', !after2.dirExists)
|
||||
check('git registration is gone', !after2.registered)
|
||||
check(
|
||||
'archiveHookOverride recorded',
|
||||
waivedJson?.result?.archiveHookOverride?.overridden === true,
|
||||
JSON.stringify(waivedJson?.result?.archiveHookOverride)
|
||||
)
|
||||
check(
|
||||
'override records exit 23 / exited',
|
||||
waivedJson?.result?.archiveHookOverride?.exitCode === 23 &&
|
||||
waivedJson?.result?.archiveHookOverride?.outcome === 'exited'
|
||||
)
|
||||
|
||||
// ============================================================ SCENARIO 3
|
||||
banner('SCENARIO 3 — archive hook exits 0: removal proceeds')
|
||||
setMode('ok')
|
||||
const wt3 = makeWorktree(hookRepo, hookRepoPath, `gate-ok-${randomBytes(3).toString('hex')}`)
|
||||
out(`worktree: ${wt3.path}`)
|
||||
const okRm = cli(['worktree', 'rm', '--worktree', wt3.id, '--force', '--run-hooks'])
|
||||
const okJson = parseJsonLine(okRm)
|
||||
out(` exit code: ${okRm.status}`)
|
||||
out(` ${JSON.stringify(okJson)}`)
|
||||
const after3 = evidence(wt3.repoPath, wt3.path)
|
||||
check('exits zero', okRm.status === 0)
|
||||
check('checkout deleted', !after3.dirExists)
|
||||
check('git registration gone', !after3.registered)
|
||||
check(
|
||||
'no archiveHookOverride on a clean run',
|
||||
okJson?.result?.archiveHookOverride === undefined
|
||||
)
|
||||
|
||||
// ============================================================ SCENARIO 4
|
||||
banner('SCENARIO 4 — no archive hook configured: removal proceeds unchanged')
|
||||
const wt4 = makeWorktree(
|
||||
bareRepo,
|
||||
bareRepoPath,
|
||||
`gate-nohook-${randomBytes(3).toString('hex')}`
|
||||
)
|
||||
out(`worktree: ${wt4.path}`)
|
||||
const runsBefore = hookRuns().length
|
||||
const noHook = cli(['worktree', 'rm', '--worktree', wt4.id, '--force', '--run-hooks'])
|
||||
const noHookJson = parseJsonLine(noHook)
|
||||
out(` exit code: ${noHook.status}`)
|
||||
out(` ${JSON.stringify(noHookJson)}`)
|
||||
const after4 = evidence(wt4.repoPath, wt4.path)
|
||||
check('exits zero', noHook.status === 0)
|
||||
check('checkout deleted', !after4.dirExists)
|
||||
check('no hook was run', hookRuns().length === runsBefore)
|
||||
|
||||
// ============================================================ SCENARIO 5
|
||||
banner('SCENARIO 5 — hook never reports an exit (killed): must BLOCK as `unverifiable`')
|
||||
setMode('signal')
|
||||
const wt5 = makeWorktree(hookRepo, hookRepoPath, `gate-unver-${randomBytes(3).toString('hex')}`)
|
||||
out(`worktree: ${wt5.path}`)
|
||||
const unver = cli(['worktree', 'rm', '--worktree', wt5.id, '--run-hooks'])
|
||||
const unverJson = parseJsonLine(unver)
|
||||
out(` exit code: ${unver.status}`)
|
||||
out(` ${JSON.stringify(unverJson)}`)
|
||||
const after5 = evidence(wt5.repoPath, wt5.path)
|
||||
showEvidence(after5)
|
||||
check('blocked (non-zero)', unver.status !== 0, `got ${unver.status}`)
|
||||
check('typed error code', unverJson?.error?.code === 'worktree_archive_hook_failed')
|
||||
check(
|
||||
"outcome is 'unverifiable', NOT 'exited'",
|
||||
unverJson?.error?.data?.outcome === 'unverifiable',
|
||||
JSON.stringify(unverJson?.error?.data)
|
||||
)
|
||||
check(
|
||||
'exit code is WITHHELD (never read as a pass)',
|
||||
unverJson?.error?.data?.exitCode === undefined
|
||||
)
|
||||
check('checkout survives', after5.dirExists && after5.fileExists)
|
||||
check('git registration survives', after5.registered)
|
||||
|
||||
// clean up scenario 5 with the waiver so the temp dirs go away
|
||||
setMode('ok')
|
||||
cli(['worktree', 'rm', '--worktree', wt5.id, '--force'])
|
||||
|
||||
// ============================================================ SCENARIO 6
|
||||
banner('SCENARIO 6 — folder workspace removal (the boundary that runs no hook) is unchanged')
|
||||
const folderDir = mkdtempSync(join(tmpdir(), 'agh-folder-'))
|
||||
mkdirSync(join(folderDir, 'src'))
|
||||
writeFileSync(join(folderDir, 'src', 'app.txt'), 'folder workspace content\n')
|
||||
// A folder workspace is imported against an existing project identity, so anchor it on the
|
||||
// hookless repo's GitHub-derived project.
|
||||
const folderProjectId = `github:agh-owner/${folderProjectSlug}`
|
||||
ok([
|
||||
'project',
|
||||
'setup-existing-folder',
|
||||
'--project',
|
||||
folderProjectId,
|
||||
'--host',
|
||||
'local',
|
||||
'--path',
|
||||
folderDir,
|
||||
'--kind',
|
||||
'folder'
|
||||
])
|
||||
const folderRepo = (ok(['repo', 'list']).repos ?? []).find((r) => r.path === folderDir) ?? null
|
||||
out(`folder repo: ${folderDir} (${folderRepo?.id}) kind=${folderRepo?.kind}`)
|
||||
check('registered repo kind is folder', folderRepo?.kind === 'folder', String(folderRepo?.kind))
|
||||
// The project ROOT of a folder project is not deletable (pre-existing rule, unrelated to the
|
||||
// gate); the deletable folder workspace is a child created under it.
|
||||
const folderRoot = ok(['worktree', 'show', '--worktree', `path:${folderDir}`]).worktree
|
||||
const rootRm = cli(['worktree', 'rm', '--worktree', folderRoot.id, '--force', '--run-hooks'])
|
||||
out(
|
||||
` root refusal (unchanged): exit ${rootRm.status} ${parseJsonLine(rootRm)?.error?.code} -- ${parseJsonLine(rootRm)?.error?.message}`
|
||||
)
|
||||
check(
|
||||
'folder project root still refuses for its own reason, not the archive gate',
|
||||
rootRm.status !== 0 && parseJsonLine(rootRm)?.error?.code !== 'worktree_archive_hook_failed'
|
||||
)
|
||||
|
||||
const folderChild = ok([
|
||||
'worktree',
|
||||
'create',
|
||||
'--repo',
|
||||
`id:${folderRepo.id}`,
|
||||
'--name',
|
||||
'agh-folder-child',
|
||||
'--setup',
|
||||
'skip'
|
||||
]).worktree
|
||||
out(`folder workspace: ${folderChild.id}`)
|
||||
const runsBeforeFolder = hookRuns().length
|
||||
out(`\n$ orca worktree rm --worktree <folder workspace> --force --run-hooks`)
|
||||
const folderRm = cli(['worktree', 'rm', '--worktree', folderChild.id, '--force', '--run-hooks'])
|
||||
const folderJson = parseJsonLine(folderRm)
|
||||
out(` exit code: ${folderRm.status}`)
|
||||
out(` ${JSON.stringify(folderJson)}`)
|
||||
const stillThere = cli(['worktree', 'show', '--worktree', folderChild.id])
|
||||
check(
|
||||
'folder workspace removal exits zero',
|
||||
folderRm.status === 0,
|
||||
`${folderRm.status} ${folderRm.stderr}`
|
||||
)
|
||||
check('folder removal ran no archive hook', hookRuns().length === runsBeforeFolder)
|
||||
check(
|
||||
'folder contents left on disk (forget, not delete)',
|
||||
existsSync(join(folderDir, 'src', 'app.txt'))
|
||||
)
|
||||
check(
|
||||
'folder workspace is deregistered',
|
||||
stillThere.status !== 0 && parseJsonLine(stillThere)?.error?.code === 'selector_not_found'
|
||||
)
|
||||
rmSync(folderDir, { recursive: true, force: true })
|
||||
|
||||
banner(failures === 0 ? 'ALL CHECKS PASSED' : `${failures} CHECK(S) FAILED`)
|
||||
} catch (error) {
|
||||
out(`\nHARNESS ERROR: ${error instanceof Error ? error.stack : String(error)}`)
|
||||
failures++
|
||||
} finally {
|
||||
for (const wt of created) {
|
||||
if (existsSync(wt.path)) {
|
||||
cli(['worktree', 'rm', '--worktree', wt.id, '--force'])
|
||||
rmSync(dirname(wt.path), { recursive: true, force: true })
|
||||
}
|
||||
}
|
||||
if (child.exitCode === null && child.signalCode === null) {
|
||||
child.kill('SIGTERM')
|
||||
await Promise.race([
|
||||
new Promise((r) => child.on('exit', r)),
|
||||
new Promise((r) => setTimeout(r, 15_000))
|
||||
])
|
||||
child.kill('SIGKILL')
|
||||
}
|
||||
rmSync(userDataDir, { recursive: true, force: true })
|
||||
}
|
||||
process.exitCode = failures === 0 ? 0 : 1
|
||||
}
|
||||
|
||||
setMode('ok')
|
||||
main()
|
||||
@@ -0,0 +1,37 @@
|
||||
import {
|
||||
formatArchiveHookOverride,
|
||||
type ArchiveHookOverride
|
||||
} from '../../shared/worktree/archive-hook-removal-gate'
|
||||
|
||||
type HookWarningResult = {
|
||||
warning?: string
|
||||
archiveHookOverride?: ArchiveHookOverride
|
||||
}
|
||||
|
||||
type PreservedBranchResult = {
|
||||
preservedBranch?: {
|
||||
branchName: string
|
||||
}
|
||||
}
|
||||
|
||||
export function printHookWarning(result: HookWarningResult, json: boolean): void {
|
||||
if (json) {
|
||||
return
|
||||
}
|
||||
if (result.warning) {
|
||||
console.error(`warning: ${result.warning}`)
|
||||
}
|
||||
// Why (#19334): a waived archive-hook failure is the one case where Orca deleted a checkout
|
||||
// whose archive step did not succeed. It has to stay visible in human output.
|
||||
if (result.archiveHookOverride) {
|
||||
console.error(`warning: ${formatArchiveHookOverride(result.archiveHookOverride)}`)
|
||||
}
|
||||
}
|
||||
|
||||
export function printPreservedBranchWarning(result: PreservedBranchResult, json: boolean): void {
|
||||
if (!json && result.preservedBranch) {
|
||||
console.error(
|
||||
`warning: local branch "${result.preservedBranch.branchName}" was kept because Git could not safely delete it`
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import type {
|
||||
RuntimeWorktreeRemoveResult
|
||||
} from '../../shared/runtime-types'
|
||||
import type { CommandHandler } from '../dispatch'
|
||||
import { printHookWarning, printPreservedBranchWarning } from './worktree-removal-warnings'
|
||||
import { formatWorktreeList, formatWorktreePs, formatWorktreeShow, printResult } from '../format'
|
||||
import {
|
||||
annotateOmittedHostScope,
|
||||
@@ -38,30 +39,6 @@ import {
|
||||
} from './worktree-create-parent-selector'
|
||||
import { getOptionalLinearIssueLinkFlag } from './worktree-linear-issue-link'
|
||||
|
||||
type HookWarningResult = {
|
||||
warning?: string
|
||||
}
|
||||
|
||||
type PreservedBranchResult = {
|
||||
preservedBranch?: {
|
||||
branchName: string
|
||||
}
|
||||
}
|
||||
|
||||
function printHookWarning(result: HookWarningResult, json: boolean): void {
|
||||
if (!json && result.warning) {
|
||||
console.error(`warning: ${result.warning}`)
|
||||
}
|
||||
}
|
||||
|
||||
function printPreservedBranchWarning(result: PreservedBranchResult, json: boolean): void {
|
||||
if (!json && result.preservedBranch) {
|
||||
console.error(
|
||||
`warning: local branch "${result.preservedBranch.branchName}" was kept because Git could not safely delete it`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
function assertParentWorktreeFlagsCompatible(flags: Map<string, string | boolean>): void {
|
||||
if (flags.has('parent-worktree') && flags.get('no-parent') === true) {
|
||||
throw new RuntimeClientError(
|
||||
@@ -305,13 +282,24 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
|
||||
'Orca cannot tell which host owns this workspace. Refresh projects and try again.'
|
||||
)
|
||||
}
|
||||
// Why (#19334): the waiver only ever applies to a hook that ran, so without --run-hooks it
|
||||
// silently does nothing. Rejecting it beats letting someone believe they waived something.
|
||||
if (flags.get('allow-failed-archive-hook') === true && flags.get('run-hooks') !== true) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
'--allow-failed-archive-hook waives a FAILED archive hook, but without --run-hooks no hook runs at all. Pass --run-hooks too, or drop the waiver.'
|
||||
)
|
||||
}
|
||||
const result = await client.call<RuntimeWorktreeRemoveResult>('worktree.rm', {
|
||||
worktree,
|
||||
hostId,
|
||||
force: flags.get('force') === true,
|
||||
// Why (#11960): --force is explicit here, so it may also waive PTY-stop proof.
|
||||
allowUnverifiedPtyStop: flags.get('force') === true,
|
||||
runHooks: flags.get('run-hooks') === true
|
||||
runHooks: flags.get('run-hooks') === true,
|
||||
// Why (#19334): deliberately NOT coupled to --force, which above already waives PTY-stop
|
||||
// proof. Waiving a failed archive hook is a separate decision about the user's data.
|
||||
allowFailedArchiveHook: flags.get('allow-failed-archive-hook') === true
|
||||
})
|
||||
printHookWarning(result.result, json)
|
||||
printPreservedBranchWarning(result.result, json)
|
||||
|
||||
@@ -135,6 +135,83 @@ describe('command aliases dispatch to the canonical handler', () => {
|
||||
}
|
||||
})
|
||||
|
||||
// #19334: a failed archive hook blocks removal, so the CLI must exit non-zero rather than
|
||||
// report a delete that did not happen — and the waiver must ride its own flag, never --force.
|
||||
it('exits non-zero when worktree removal is refused by a failed archive hook', async () => {
|
||||
queueFixtures(callMock, okFixture('req_show', { worktree: { hostId: 'local' } }))
|
||||
callMock.mockRejectedValueOnce(
|
||||
Object.assign(new Error('Archive hook failed for worktree: /tmp/wt — exited 23.'), {
|
||||
code: 'worktree_archive_hook_failed'
|
||||
})
|
||||
)
|
||||
const priorExitCode = process.exitCode
|
||||
|
||||
try {
|
||||
await main(
|
||||
['worktree', 'rm', '--worktree', 'id:wt-1', '--force', '--run-hooks', '--json'],
|
||||
'/tmp/repo'
|
||||
)
|
||||
|
||||
expect(process.exitCode).toBe(1)
|
||||
expect(callMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'worktree.rm',
|
||||
expect.objectContaining({
|
||||
runHooks: true,
|
||||
allowFailedArchiveHook: false
|
||||
})
|
||||
)
|
||||
} finally {
|
||||
process.exitCode = priorExitCode
|
||||
}
|
||||
})
|
||||
|
||||
// #19334 S4: the waiver only applies to a hook that ran, so alone it silently does nothing.
|
||||
it('rejects the archive-hook waiver without --run-hooks instead of ignoring it', async () => {
|
||||
queueFixtures(callMock, okFixture('req_show', { worktree: { hostId: 'local' } }))
|
||||
const priorExitCode = process.exitCode
|
||||
|
||||
try {
|
||||
await main(
|
||||
['worktree', 'rm', '--worktree', 'id:wt-1', '--allow-failed-archive-hook', '--json'],
|
||||
'/tmp/repo'
|
||||
)
|
||||
|
||||
expect(process.exitCode).toBe(1)
|
||||
// The removal must never have been attempted.
|
||||
expect(callMock).not.toHaveBeenCalledWith('worktree.rm', expect.anything())
|
||||
} finally {
|
||||
process.exitCode = priorExitCode
|
||||
}
|
||||
})
|
||||
|
||||
it('forwards the explicit archive-hook waiver on worktree rm', async () => {
|
||||
queueFixtures(
|
||||
callMock,
|
||||
okFixture('req_show', { worktree: { hostId: 'local' } }),
|
||||
okFixture('req', { removed: true })
|
||||
)
|
||||
|
||||
await main(
|
||||
[
|
||||
'worktree',
|
||||
'rm',
|
||||
'--worktree',
|
||||
'id:wt-1',
|
||||
'--run-hooks',
|
||||
'--allow-failed-archive-hook',
|
||||
'--json'
|
||||
],
|
||||
'/tmp/repo'
|
||||
)
|
||||
|
||||
expect(callMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'worktree.rm',
|
||||
expect.objectContaining({ runHooks: true, allowFailedArchiveHook: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('still runs `terminal focus` after the handler de-duplication', async () => {
|
||||
queueFixtures(callMock, okFixture('req', { focus: { ok: true } }))
|
||||
|
||||
|
||||
@@ -52,7 +52,7 @@ export const ROOT_HELP_TEXT_SECONDARY = [
|
||||
' orca worktree show --worktree <selector> [--json]',
|
||||
' orca worktree current [--json]',
|
||||
' orca worktree set --worktree <selector> [--display-name <name>] [--issue <number|null>] [--linear-issue <identifier-or-url|null>] [--comment <text>] [--workspace-status <id>] [--parent-worktree <selector>|--no-parent] [--json]',
|
||||
' orca worktree rm --worktree <selector> [--force] [--run-hooks] [--json]',
|
||||
' orca worktree rm --worktree <selector> [--force] [--run-hooks] [--allow-failed-archive-hook] [--json]',
|
||||
' orca worktree ps [--limit <n>] [--json]',
|
||||
' orca file open <path> [--worktree <selector>] [--json]',
|
||||
' orca file diff <path> [--staged] [--worktree <selector>] [--json]',
|
||||
|
||||
@@ -172,10 +172,13 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [
|
||||
],
|
||||
destructive: true,
|
||||
summary: 'Remove a worktree from Orca and git',
|
||||
usage: 'orca worktree rm --worktree <selector> [--force] [--run-hooks] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'force', 'run-hooks'],
|
||||
usage:
|
||||
'orca worktree rm --worktree <selector> [--force] [--run-hooks] [--allow-failed-archive-hook] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'force', 'run-hooks', 'allow-failed-archive-hook'],
|
||||
notes: [
|
||||
'Repo-defined orca.yaml archive hooks are skipped unless --run-hooks is passed.',
|
||||
'With --run-hooks, a failed archive hook blocks the removal: nothing is stopped, deleted or deregistered, and the command exits non-zero with error code worktree_archive_hook_failed. --force does not waive this.',
|
||||
'Pass --allow-failed-archive-hook to delete anyway after the hook has run and failed; the waived failure is reported back on result.archiveHookOverride. It requires --run-hooks and is rejected without it, because with no hook running there is no failure to waive.',
|
||||
'For Git worktrees, removal also attempts to delete the checked-out local branch, with or without --force. Orca retains branches it knows predated the worktree and any branch whose changes it cannot prove are already merged.'
|
||||
]
|
||||
},
|
||||
|
||||
@@ -118,6 +118,11 @@ describe('archive hook exit observation', () => {
|
||||
).resolves.toMatchObject({ success: true })
|
||||
})
|
||||
|
||||
it('names a signalled exit as one rather than reporting "exit code null"', async () => {
|
||||
const result = await runArchiveWith({ code: null, signal: 'SIGKILL' })
|
||||
expect(result.output).toContain('terminated without reporting an exit code')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['was killed by a signal', { code: null, signal: 'SIGKILL' as const }],
|
||||
// A real spawn failure carries a STRING code; the guard under test is `typeof code ===
|
||||
|
||||
+6
-1
@@ -44,7 +44,12 @@ function classifyHookProcessResult(
|
||||
return { success: false, output: `${streams}\n${message}`.trim() }
|
||||
}
|
||||
if (result.code !== 0) {
|
||||
const message = `Command failed with exit code ${result.code}.`
|
||||
// `null` means signalled: there is no exit code, and saying "exit code null" reads as a
|
||||
// reporting glitch rather than the `unverifiable` verdict the gate is about to give it.
|
||||
const message =
|
||||
result.code === null
|
||||
? 'Command was terminated without reporting an exit code.'
|
||||
: `Command failed with exit code ${result.code}.`
|
||||
console.error(`[hooks] ${context.hookName} hook failed in ${context.cwd}:`, message)
|
||||
return {
|
||||
success: false,
|
||||
|
||||
@@ -14,6 +14,13 @@ import {
|
||||
} from './worktrees-test-module-mocks'
|
||||
import { handlers, setupWorktreeHandlers, store } from './worktrees-test-harness'
|
||||
import { mockKnownFeatureWorktree } from './worktrees-test-fixtures'
|
||||
import {
|
||||
ARCHIVE_HOOK_FAILED_REMOVAL_CODE,
|
||||
asArchiveHookRefusal,
|
||||
type WorktreeArchiveHookFailedError
|
||||
} from '../../shared/worktree/archive-hook-removal-gate'
|
||||
import type { RemoveWorktreeResult } from '../../shared/worktree/create-types'
|
||||
import type { RemoveWorktreeArgs } from './worktrees/ipc-context-schemas'
|
||||
import type { WorktreeRuntimeStub } from './worktrees-test-runtime-stub'
|
||||
|
||||
vi.mock('electron', async () =>
|
||||
@@ -98,6 +105,25 @@ vi.mock('../runtime/worktree-teardown', async () =>
|
||||
)
|
||||
vi.mock('./pty', async () => (await import('./worktrees-test-module-mocks')).ptyModuleMock())
|
||||
|
||||
// The shared IPC surface types every handler as returning `unknown`; removal's contract is
|
||||
// narrower, and #19334's whole point is that a caller can name and branch on it.
|
||||
async function removeWorktreeViaIpc(args: RemoveWorktreeArgs): Promise<RemoveWorktreeResult> {
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the registry types every handler as `(...) => unknown`, so this is the only place the real `worktrees:remove` return shape can be named; the production caller in worktree-ipc.ts declares the same type.
|
||||
return (await handlers['worktrees:remove'](null, args)) as RemoveWorktreeResult
|
||||
}
|
||||
|
||||
/** Narrows through the exported error class — the same branch a real caller would write. */
|
||||
async function expectArchiveHookRefusal(
|
||||
args: RemoveWorktreeArgs
|
||||
): Promise<WorktreeArchiveHookFailedError> {
|
||||
try {
|
||||
await removeWorktreeViaIpc(args)
|
||||
} catch (error) {
|
||||
return asArchiveHookRefusal(error)
|
||||
}
|
||||
throw new Error(`expected removal of ${args.worktreeId} to be refused by the archive hook`)
|
||||
}
|
||||
|
||||
describe('registerWorktreeHandlers', () => {
|
||||
let runtimeStub: WorktreeRuntimeStub
|
||||
|
||||
@@ -443,7 +469,8 @@ describe('registerWorktreeHandlers', () => {
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', true)
|
||||
})
|
||||
|
||||
it('continues SSH worktree removal when the archive hook fails', async () => {
|
||||
// Was "continues SSH worktree removal when the archive hook fails" (#19334): it now refuses.
|
||||
it('refuses SSH worktree removal when the remote archive hook exits non-zero', async () => {
|
||||
const repo = {
|
||||
id: 'repo-ssh',
|
||||
path: '/remote/repo',
|
||||
@@ -453,7 +480,6 @@ describe('registerWorktreeHandlers', () => {
|
||||
connectionId: 'conn-1',
|
||||
worktreeBaseRef: null
|
||||
}
|
||||
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const provider = {
|
||||
listWorktrees: vi.fn().mockResolvedValue([
|
||||
{
|
||||
@@ -492,21 +518,18 @@ describe('registerWorktreeHandlers', () => {
|
||||
getSshFilesystemProviderMock.mockReturnValue(fsProvider)
|
||||
getEffectiveHooksFromConfigMock.mockReturnValue({ scripts: { archive: 'exit 7' } })
|
||||
|
||||
try {
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-ssh::/remote/feature-wt'
|
||||
})
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined)
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
'[hooks] archive hook failed for /remote/feature-wt:',
|
||||
expect.stringContaining('archive hook exited 7')
|
||||
)
|
||||
} finally {
|
||||
consoleErrorSpy.mockRestore()
|
||||
}
|
||||
const refusal = await expectArchiveHookRefusal({
|
||||
worktreeId: 'repo-ssh::/remote/feature-wt'
|
||||
})
|
||||
|
||||
expect(refusal.code).toBe(ARCHIVE_HOOK_FAILED_REMOVAL_CODE)
|
||||
expect(refusal.data).toMatchObject({ outcome: 'exited', exitCode: 7 })
|
||||
expect(provider.worktreeIsClean).not.toHaveBeenCalled()
|
||||
expect(provider.removeWorktree).not.toHaveBeenCalled()
|
||||
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('continues SSH worktree removal when archive hook execution rejects', async () => {
|
||||
it('does not read a lost SSH connection as an archive hook that passed', async () => {
|
||||
const repo = {
|
||||
id: 'repo-ssh',
|
||||
path: '/remote/repo',
|
||||
@@ -516,7 +539,6 @@ describe('registerWorktreeHandlers', () => {
|
||||
connectionId: 'conn-1',
|
||||
worktreeBaseRef: null
|
||||
}
|
||||
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const provider = {
|
||||
listWorktrees: vi.fn().mockResolvedValue([
|
||||
{
|
||||
@@ -550,18 +572,15 @@ describe('registerWorktreeHandlers', () => {
|
||||
getSshFilesystemProviderMock.mockReturnValue(fsProvider)
|
||||
getEffectiveHooksFromConfigMock.mockReturnValue({ scripts: { archive: 'echo archived' } })
|
||||
|
||||
try {
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-ssh::/remote/feature-wt'
|
||||
})
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined)
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
'[hooks] archive hook failed for /remote/feature-wt:',
|
||||
'relay disconnected'
|
||||
)
|
||||
} finally {
|
||||
consoleErrorSpy.mockRestore()
|
||||
}
|
||||
const refusal = await expectArchiveHookRefusal({
|
||||
worktreeId: 'repo-ssh::/remote/feature-wt'
|
||||
})
|
||||
|
||||
// Loss of contact is `unverifiable`, never evidence the hook succeeded.
|
||||
expect(refusal.data).toMatchObject({ outcome: 'unverifiable' })
|
||||
expect(refusal.data.exitCode).toBeUndefined()
|
||||
expect(provider.removeWorktree).not.toHaveBeenCalled()
|
||||
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('uses cmd.exe for archive hooks on Windows-like SSH worktree paths', async () => {
|
||||
@@ -681,4 +700,116 @@ describe('registerWorktreeHandlers', () => {
|
||||
expect(provider.execNonInteractive).not.toHaveBeenCalled()
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined)
|
||||
})
|
||||
|
||||
// Regression cover for #19334: a failed archive hook is a blocking precondition, not an advisory.
|
||||
it('refuses removal and mutates nothing when the local archive hook exits 23', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
removeWorktreeMock.mockResolvedValue(undefined)
|
||||
getEffectiveHooksMock.mockReturnValue({
|
||||
scripts: { archive: 'echo archived' }
|
||||
})
|
||||
runHookMock.mockResolvedValue({
|
||||
success: false,
|
||||
output: 'backup target unreachable',
|
||||
exitCode: 23
|
||||
})
|
||||
|
||||
const refusal = await expectArchiveHookRefusal({
|
||||
worktreeId: 'repo-1::/workspace/feature-wt'
|
||||
})
|
||||
|
||||
expect(refusal.code).toBe(ARCHIVE_HOOK_FAILED_REMOVAL_CODE)
|
||||
expect(refusal.data).toEqual({
|
||||
worktreePath: '/workspace/feature-wt',
|
||||
outcome: 'exited',
|
||||
exitCode: 23,
|
||||
output: 'backup target unreachable'
|
||||
})
|
||||
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(assertWorktreeCleanForRemovalMock).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
|
||||
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('classifies a local archive hook that never reported an exit as unverifiable', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
getEffectiveHooksMock.mockReturnValue({
|
||||
scripts: { archive: 'echo archived' }
|
||||
})
|
||||
runHookMock.mockResolvedValue({
|
||||
success: false,
|
||||
output: 'Hook timed out after 120000ms.'
|
||||
})
|
||||
|
||||
const refusal = await expectArchiveHookRefusal({
|
||||
worktreeId: 'repo-1::/workspace/feature-wt'
|
||||
})
|
||||
|
||||
expect(refusal.data).toEqual({
|
||||
worktreePath: '/workspace/feature-wt',
|
||||
outcome: 'unverifiable',
|
||||
output: 'Hook timed out after 120000ms.'
|
||||
})
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('removes and records the waiver when a failed archive hook is explicitly overridden', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
removeWorktreeMock.mockResolvedValue({})
|
||||
getEffectiveHooksMock.mockReturnValue({
|
||||
scripts: { archive: 'echo archived' }
|
||||
})
|
||||
runHookMock.mockResolvedValue({
|
||||
success: false,
|
||||
output: 'boom',
|
||||
exitCode: 23
|
||||
})
|
||||
|
||||
const result = await removeWorktreeViaIpc({
|
||||
worktreeId: 'repo-1::/workspace/feature-wt',
|
||||
allowFailedArchiveHook: true
|
||||
})
|
||||
|
||||
expect(result.archiveHookOverride).toEqual({
|
||||
worktreePath: '/workspace/feature-wt',
|
||||
outcome: 'exited',
|
||||
exitCode: 23,
|
||||
output: 'boom',
|
||||
overridden: true
|
||||
})
|
||||
expect(removeWorktreeMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// The folder-workspace path runs no archive hook at all (no Git removal step), so the gate has
|
||||
// nothing to evaluate there. Pinned so a future hook added to that path is a deliberate change.
|
||||
it('removes a folder workspace without consulting the archive hook', async () => {
|
||||
const repo = {
|
||||
id: 'repo-folder',
|
||||
path: '/workspace/folder-project',
|
||||
displayName: 'folder',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
kind: 'folder' as const,
|
||||
worktreeBaseRef: null
|
||||
}
|
||||
store.getRepos.mockReturnValue([repo])
|
||||
store.getRepo.mockReturnValue(repo)
|
||||
getEffectiveHooksMock.mockReturnValue({ scripts: { archive: 'exit 23' } })
|
||||
runHookMock.mockResolvedValue({
|
||||
success: false,
|
||||
output: 'boom',
|
||||
exitCode: 23
|
||||
})
|
||||
|
||||
const result = await removeWorktreeViaIpc({
|
||||
worktreeId: 'repo-folder::/workspace/folder-project/nested'
|
||||
})
|
||||
|
||||
expect(result).toEqual({})
|
||||
expect(runHookMock).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(store.removeWorktreeMeta).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -21,6 +21,9 @@ export type RemoveWorktreeArgs = {
|
||||
/** Explicit Force Delete only — `force` alone is set by the ordinary confirmation (#11960). */
|
||||
allowUnverifiedPtyStop?: boolean
|
||||
skipArchive?: boolean
|
||||
/** Explicit waiver for a FAILED archive hook (#19334). Distinct from `skipArchive`, which
|
||||
* never runs the hook at all, and never implied by `force`. */
|
||||
allowFailedArchiveHook?: boolean
|
||||
snapshotPruneBatchId?: string
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,8 @@ import { isPrunableGitFileWorktree } from '../../../worktree-prunable-git-file'
|
||||
import { findRegisteredDeletableWorktree } from '../../../worktree-removal-safety'
|
||||
import { removeStaleLocalWorktreeRegistration } from '../../../local-worktree-removal-recovery'
|
||||
import { runHook } from '../../../hooks'
|
||||
import type { ArchiveHookOverride } from '../../../../shared/worktree/archive-hook-removal-gate'
|
||||
import { gateWorktreeRemovalOnArchiveHook } from '../../../worktree-archive-hook-gate'
|
||||
import { withWorktreeRemoveStageSpan } from '../../../observability/instrumentation'
|
||||
import {
|
||||
cleanupUnusedWorktreePushTargetRemote,
|
||||
@@ -84,6 +86,10 @@ export async function executeWorktreeRemoval(
|
||||
throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false))
|
||||
}
|
||||
|
||||
// Ahead of the archive-hook gate below, and that ordering is right: both arms describe a
|
||||
// registration with no checkout behind it — a row whose path IS a `.git` file, or a tree already
|
||||
// gone from disk. There is nothing to archive, and running the hook would fail on the missing
|
||||
// cwd and block a cleanup that has no user data to lose.
|
||||
if (
|
||||
!repo.connectionId &&
|
||||
((await isPrunableGitFileWorktree(registeredWorktree, localWorktreeGitOptions)) ||
|
||||
@@ -126,10 +132,18 @@ export async function executeWorktreeRemoval(
|
||||
return removalResult ?? {}
|
||||
}
|
||||
|
||||
// No connectionId override here, deliberately: this path derives its host from the repo row
|
||||
// (`getRepoExecutionHostId` in register-worktree-removal-handlers) and resolves its provider, git
|
||||
// options, listing and dispatch from `repo.connectionId` alone. Passing a different owner to the
|
||||
// hook reader would read one host's orca.yaml while running the other host's git. The runtime's
|
||||
// SSH path is the one that carries a route owner separate from the row, and it passes it.
|
||||
const hooks = await getArchiveHooksForRemoval(repo)
|
||||
|
||||
const archiveScript = hooks?.scripts.archive
|
||||
|
||||
// Precondition, not an advisory (#19334): both branches below stop PTYs and delete the
|
||||
// checkout, so a hook failure has to throw here — before either is reached.
|
||||
let archiveHookOverride: ArchiveHookOverride | undefined
|
||||
if (archiveScript && !args.skipArchive) {
|
||||
// Why the branch on connectionId: this block is shared by both flows, so a hardcoded
|
||||
// 'remote' would file every local archive hook under the SSH breakdown.
|
||||
@@ -146,38 +160,40 @@ export async function executeWorktreeRemoval(
|
||||
undefined,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
if (!result.success) {
|
||||
console.error(`[hooks] archive hook failed for ${canonicalWorktreePath}:`, result.output)
|
||||
}
|
||||
archiveHookOverride = gateWorktreeRemovalOnArchiveHook({
|
||||
worktreePath: canonicalWorktreePath,
|
||||
result,
|
||||
allowFailure: args.allowFailedArchiveHook === true
|
||||
})
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
const remoteConnectionId = repo.connectionId ?? undefined
|
||||
if (remoteConnectionId) {
|
||||
return removeRegisteredRemoteWorktree(
|
||||
context,
|
||||
args,
|
||||
repo,
|
||||
repoId,
|
||||
canonicalWorktreePath,
|
||||
removalHostId,
|
||||
registeredWorktree,
|
||||
removedPushTarget,
|
||||
provider!,
|
||||
deleteBranch
|
||||
)
|
||||
}
|
||||
return removeRegisteredLocalWorktree(
|
||||
context,
|
||||
args,
|
||||
repo,
|
||||
repoId,
|
||||
canonicalWorktreePath,
|
||||
removalHostId,
|
||||
removedPushTarget,
|
||||
localWorktreeGitOptions,
|
||||
hasLocalWorktreeGitOptions,
|
||||
deleteBranch
|
||||
)
|
||||
const result = remoteConnectionId
|
||||
? await removeRegisteredRemoteWorktree(
|
||||
context,
|
||||
args,
|
||||
repo,
|
||||
repoId,
|
||||
canonicalWorktreePath,
|
||||
removalHostId,
|
||||
registeredWorktree,
|
||||
removedPushTarget,
|
||||
provider!,
|
||||
deleteBranch
|
||||
)
|
||||
: await removeRegisteredLocalWorktree(
|
||||
context,
|
||||
args,
|
||||
repo,
|
||||
repoId,
|
||||
canonicalWorktreePath,
|
||||
removalHostId,
|
||||
removedPushTarget,
|
||||
localWorktreeGitOptions,
|
||||
hasLocalWorktreeGitOptions,
|
||||
deleteBranch
|
||||
)
|
||||
return archiveHookOverride ? { ...result, archiveHookOverride } : result
|
||||
}
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import type * as HooksModule from '../../../hooks'
|
||||
|
||||
const { getSshFilesystemProviderMock, getEffectiveHooksMock } = vi.hoisted(() => ({
|
||||
getSshFilesystemProviderMock: vi.fn(),
|
||||
getEffectiveHooksMock: vi.fn()
|
||||
}))
|
||||
vi.mock('../../../providers/ssh-filesystem-dispatch', () => ({
|
||||
getSshFilesystemProvider: getSshFilesystemProviderMock
|
||||
}))
|
||||
// Only `getEffectiveHooks` is stubbed: the module under test also imports `parseOrcaYaml` from
|
||||
// here, and replacing it wholesale made the parse throw into the fail-open catch — which answers
|
||||
// "no hook", so the test saw an empty result rather than an error.
|
||||
vi.mock('../../../hooks', async () => ({
|
||||
...(await vi.importActual<typeof HooksModule>('../../../hooks')),
|
||||
getEffectiveHooks: getEffectiveHooksMock
|
||||
}))
|
||||
|
||||
import { getArchiveHooksForRemoval } from './worktree-archive-hook'
|
||||
|
||||
const REMOTE_REPO: Repo = {
|
||||
id: 'r',
|
||||
path: '/home/orca/repo',
|
||||
displayName: 'r',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}
|
||||
|
||||
// Why (#19334): a worktree row can name its owner only as `executionHostId: 'ssh:<target>'`, leaving
|
||||
// `repo.connectionId` null. Resolving hooks off the row alone then reads THIS machine's disk for a
|
||||
// repo that lives on an SSH host — the committed archive hook goes unseen and the removal proceeds
|
||||
// as though none were configured, which is the bug the gate exists to stop.
|
||||
describe('getArchiveHooksForRemoval owner resolution', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
getSshFilesystemProviderMock.mockReturnValue(undefined)
|
||||
getEffectiveHooksMock.mockReturnValue(null)
|
||||
})
|
||||
|
||||
// Why this reads a file rather than just checking the lookup key: SSH owner resolution has been
|
||||
// wrong twice on this path, and both times the fix looked right. Asserting only that
|
||||
// `'ssh-target'` was passed stops short of the thing that broke — whether the hook actually comes
|
||||
// from the REMOTE orca.yaml. This drives a stubbed provider holding real content and asserts the
|
||||
// returned script is the remote one.
|
||||
it('returns the hook from the execution host\u2019s orca.yaml, not the local disk', async () => {
|
||||
const readFile = vi.fn().mockResolvedValue({
|
||||
isBinary: false,
|
||||
content: 'scripts:\n archive: remote-archive.sh\n'
|
||||
})
|
||||
getSshFilesystemProviderMock.mockReturnValue({ readFile })
|
||||
// If the local reader were consulted it would answer with a DIFFERENT script, so a wrong
|
||||
// resolution shows up as the wrong value rather than as a silent absence.
|
||||
getEffectiveHooksMock.mockReturnValue({ scripts: { archive: 'local-archive.sh' } })
|
||||
|
||||
const hooks = await getArchiveHooksForRemoval(REMOTE_REPO, 'ssh-target')
|
||||
|
||||
expect(getSshFilesystemProviderMock).toHaveBeenCalledWith('ssh-target')
|
||||
expect(readFile).toHaveBeenCalledWith('/home/orca/repo/orca.yaml')
|
||||
expect(hooks?.scripts.archive).toBe('remote-archive.sh')
|
||||
expect(getEffectiveHooksMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('falls back to the repo row when the caller names no owner', async () => {
|
||||
await getArchiveHooksForRemoval({ ...REMOTE_REPO, connectionId: 'row-connection' })
|
||||
|
||||
expect(getSshFilesystemProviderMock).toHaveBeenCalledWith('row-connection')
|
||||
expect(getEffectiveHooksMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reads locally only when neither names a connection', async () => {
|
||||
await getArchiveHooksForRemoval({ ...REMOTE_REPO, path: '/local/repo' })
|
||||
|
||||
expect(getSshFilesystemProviderMock).not.toHaveBeenCalled()
|
||||
expect(getEffectiveHooksMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Known limitation, pinned so it is a decision rather than a surprise: the relay rewrites a
|
||||
// non-numeric error code to -32000, so a missing orca.yaml and an unreachable host arrive
|
||||
// identically. Both answer "no hook", which lets the removal proceed. Reporting them apart needs
|
||||
// a provider contract that returns absence as a successful outcome — tracked in #20196.
|
||||
it('answers "no hook" when the host cannot be read, missing or unreachable alike', async () => {
|
||||
getSshFilesystemProviderMock.mockReturnValue({
|
||||
readFile: vi.fn().mockRejectedValue(
|
||||
Object.assign(new Error('transport closed'), {
|
||||
code: -32000
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
await expect(getArchiveHooksForRemoval(REMOTE_REPO, 'ssh-target')).resolves.toEqual(null)
|
||||
})
|
||||
})
|
||||
@@ -7,16 +7,42 @@ import { getSshFilesystemProvider } from '../../../providers/ssh-filesystem-disp
|
||||
import { requireSshGitProvider } from '../../../providers/ssh-git-dispatch'
|
||||
import { joinWorktreeRelativePath } from '../../../runtime/runtime-relative-paths'
|
||||
import { getSetupRunnerEnvVars } from '../../../setup-hook-env-vars'
|
||||
import {
|
||||
ARCHIVE_HOOK_TIMEOUT_MS,
|
||||
type ArchiveHookRunResult
|
||||
} from '../../../../shared/worktree/archive-hook-removal-gate'
|
||||
|
||||
const WORKTREE_ARCHIVE_HOOK_TIMEOUT_MS = 120_000
|
||||
|
||||
export async function getArchiveHooksForRemoval(repo: Repo): Promise<OrcaHooks | null> {
|
||||
if (!repo.connectionId) {
|
||||
/**
|
||||
* Resolve the archive hook against the host that owns the worktree.
|
||||
*
|
||||
* A failed read is answered as "no hook", which is a known limitation rather than a judgement: a
|
||||
* missing `orca.yaml` is indistinguishable from an unreachable one here, because the relay rewrites
|
||||
* a non-numeric error code to `-32000` (`src/relay/dispatcher-rpc-routing.ts`), so nothing survives
|
||||
* to tell ENOENT from a transport failure. Reporting it as unreadable fired on every SSH repo that
|
||||
* simply has no orca.yaml; blocking on it would refuse those deletes outright. Distinguishing the
|
||||
* two needs a provider contract that reports absence as a successful outcome — tracked in #20196.
|
||||
*
|
||||
* @param connectionId Overrides `repo.connectionId`, which answers null for a row that names its
|
||||
* owner only as `executionHostId: 'ssh:<target>'`. Callers holding a resolved removal route must
|
||||
* pass it, or an SSH-hosted repo is read on the local disk and its archive hook goes unseen.
|
||||
*/
|
||||
export async function getArchiveHooksForRemoval(
|
||||
repo: Repo,
|
||||
connectionId?: string
|
||||
): Promise<OrcaHooks | null> {
|
||||
const owner = connectionId ?? repo.connectionId
|
||||
if (!owner) {
|
||||
return getEffectiveHooks(repo)
|
||||
}
|
||||
|
||||
const fsProvider = getSshFilesystemProvider(repo.connectionId)
|
||||
const fsProvider = getSshFilesystemProvider(owner)
|
||||
if (!fsProvider) {
|
||||
// Fail-open, and the one case here we can name confidently: no provider means the host's
|
||||
// orca.yaml was never even looked at, so "no archive hook" is an assumption. Logged rather
|
||||
// than surfaced, because the removal that follows fails on its own missing provider anyway.
|
||||
console.warn(
|
||||
`[hooks] no SSH filesystem provider for ${owner}; treating ${repo.path} as having no archive hook`
|
||||
)
|
||||
return getEffectiveHooksFromConfig(repo, null)
|
||||
}
|
||||
|
||||
@@ -24,7 +50,16 @@ export async function getArchiveHooksForRemoval(repo: Repo): Promise<OrcaHooks |
|
||||
const result = await fsProvider.readFile(joinWorktreeRelativePath(repo.path, 'orca.yaml'))
|
||||
const yamlHooks = result.isBinary ? null : parseOrcaYaml(result.content)
|
||||
return getEffectiveHooksFromConfig(repo, yamlHooks)
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// Indistinguishable from "there is no orca.yaml": the relay rewrites a non-numeric error code
|
||||
// to -32000 (src/relay/dispatcher-rpc-routing.ts), so nothing survives to tell ENOENT from a
|
||||
// transport failure. Logged so an operator can see it; not surfaced, because reporting it as
|
||||
// unreadable fired on every SSH repo that simply has none. Distinguishing them needs a provider
|
||||
// contract that returns absence as a successful outcome — #20196.
|
||||
console.warn(
|
||||
`[hooks] could not read orca.yaml for ${repo.path} on ${owner}; treating it as having no archive hook:`,
|
||||
error instanceof Error ? error.message : String(error)
|
||||
)
|
||||
return getEffectiveHooksFromConfig(repo, null)
|
||||
}
|
||||
}
|
||||
@@ -33,7 +68,7 @@ export async function runRemoteArchiveHook(
|
||||
repo: Repo,
|
||||
worktreePath: string,
|
||||
script: string
|
||||
): Promise<{ success: boolean; output: string }> {
|
||||
): Promise<ArchiveHookRunResult> {
|
||||
if (!repo.connectionId) {
|
||||
return { success: true, output: '' }
|
||||
}
|
||||
@@ -46,7 +81,7 @@ export async function runRemoteArchiveHook(
|
||||
isWindowsRemote ? 'cmd.exe' : '/bin/bash',
|
||||
isWindowsRemote ? ['/d', '/s', '/c', script] : ['-lc', script],
|
||||
worktreePath,
|
||||
WORKTREE_ARCHIVE_HOOK_TIMEOUT_MS,
|
||||
ARCHIVE_HOOK_TIMEOUT_MS,
|
||||
undefined,
|
||||
env
|
||||
)
|
||||
@@ -70,8 +105,15 @@ export async function runRemoteArchiveHook(
|
||||
.join('\n')
|
||||
.trim()
|
||||
|
||||
// Why (#19334): a spawn error or timeout means the host never reported an exit for this run, so
|
||||
// the code is withheld and the gate classifies the failure `unverifiable` rather than `exited`.
|
||||
const observedExit =
|
||||
!result.spawnError && !result.timedOut && typeof result.exitCode === 'number'
|
||||
? result.exitCode
|
||||
: undefined
|
||||
return {
|
||||
success: !result.spawnError && !result.timedOut && result.exitCode === 0,
|
||||
output
|
||||
success: observedExit === 0,
|
||||
output,
|
||||
...(observedExit !== undefined ? { exitCode: observedExit } : {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,14 +8,21 @@ export type WorktreeRemovalInFlight = {
|
||||
}
|
||||
|
||||
export function getWorktreeRemovalOptionsKey(
|
||||
args: Pick<RemoveWorktreeArgs, 'force' | 'allowUnverifiedPtyStop' | 'skipArchive'>
|
||||
args: Pick<
|
||||
RemoveWorktreeArgs,
|
||||
'force' | 'allowUnverifiedPtyStop' | 'skipArchive' | 'allowFailedArchiveHook'
|
||||
>
|
||||
): string {
|
||||
const forceKey = args.force === true ? 'force' : 'normal'
|
||||
const archiveKey = args.skipArchive === true ? 'skip-archive' : 'run-archive'
|
||||
// Why: a Force Delete retry must not coalesce onto the in-flight attempt that
|
||||
// just failed the PTY gate — it would inherit that failure instead of retrying.
|
||||
const ptyKey = args.allowUnverifiedPtyStop === true ? 'allow-unverified-pty' : 'require-pty-stop'
|
||||
return `${forceKey}:${archiveKey}:${ptyKey}`
|
||||
// Same reason for the archive waiver: a retry that waives the failed hook must not coalesce
|
||||
// onto the in-flight attempt that is about to refuse on it.
|
||||
const archiveFailureKey =
|
||||
args.allowFailedArchiveHook === true ? 'allow-failed-archive' : 'require-archive'
|
||||
return `${forceKey}:${archiveKey}:${ptyKey}:${archiveFailureKey}`
|
||||
}
|
||||
|
||||
export function getWorktreeRemovalInFlightKey(
|
||||
|
||||
@@ -7,7 +7,10 @@ import {
|
||||
import type { RemoveWorktreeResult } from '../../shared/worktree/create-types'
|
||||
import { getRepoExecutionHostId, parseExecutionHostId } from '../../shared/execution-host'
|
||||
import { preservedBranchCleanupScopeKey } from '../../shared/preserved-branch-cleanup'
|
||||
import { getRuntimeWorktreeRemovalOptionsKey } from './runtime-worktree-selection'
|
||||
import {
|
||||
getRuntimeWorktreeRemovalOptionsKey,
|
||||
type RemoveManagedWorktreeOptions
|
||||
} from './runtime-worktree-selection'
|
||||
import { withWorktreeSpan } from '../observability/instrumentation'
|
||||
import { invalidateAuthorizedRootsCache } from '../ipc/filesystem-auth'
|
||||
import { resolveWorktreeRemovalRoute } from '../worktree-removal-execution-host-route'
|
||||
@@ -30,11 +33,15 @@ import { deleteRemoteWorktreeHistory } from '../remote-worktree-history-cleanup'
|
||||
export class OrcaRuntimeWithRemoveManagedWorktree extends OrcaRuntimeWithCreateManagedRemoteWorktree {
|
||||
async removeManagedWorktree(
|
||||
worktreeSelector: string,
|
||||
force = false,
|
||||
runHooks = false,
|
||||
allowUnverifiedPtyStop = false,
|
||||
hostId?: string
|
||||
options: RemoveManagedWorktreeOptions = {}
|
||||
): Promise<RemoveWorktreeResult & { warning?: string }> {
|
||||
const {
|
||||
force = false,
|
||||
runHooks = false,
|
||||
allowUnverifiedPtyStop = false,
|
||||
allowFailedArchiveHook = false,
|
||||
hostId
|
||||
} = options
|
||||
if (!this.store) {
|
||||
throw new Error('runtime_unavailable')
|
||||
}
|
||||
@@ -45,7 +52,12 @@ export class OrcaRuntimeWithRemoveManagedWorktree extends OrcaRuntimeWithCreateM
|
||||
worktreeId: removalTarget.id,
|
||||
hostId: cleanupHostId
|
||||
})
|
||||
const optionsKey = getRuntimeWorktreeRemovalOptionsKey(force, runHooks, allowUnverifiedPtyStop)
|
||||
const optionsKey = getRuntimeWorktreeRemovalOptionsKey({
|
||||
force,
|
||||
runHooks,
|
||||
allowUnverifiedPtyStop,
|
||||
allowFailedArchiveHook
|
||||
})
|
||||
const inFlightRemoval = this.removeManagedWorktreeInFlight.get(
|
||||
cleanupScopeKey,
|
||||
removalTarget.id,
|
||||
@@ -190,6 +202,8 @@ export class OrcaRuntimeWithRemoveManagedWorktree extends OrcaRuntimeWithCreateM
|
||||
}
|
||||
if (route.kind === 'ssh') {
|
||||
return removeRuntimeRegisteredRemoteWorktree({
|
||||
runHooks,
|
||||
allowFailedArchiveHook,
|
||||
repo,
|
||||
target: removalTarget,
|
||||
registeredWorktree,
|
||||
@@ -240,6 +254,7 @@ export class OrcaRuntimeWithRemoveManagedWorktree extends OrcaRuntimeWithCreateM
|
||||
hasLocalOptions: hasLocalWorktreeGitOptions,
|
||||
force,
|
||||
runHooks,
|
||||
allowFailedArchiveHook,
|
||||
allowUnverifiedPtyStop,
|
||||
deleteBranch,
|
||||
acquireWatcherRemoval: this.acquireFileWatcherRemoval,
|
||||
|
||||
@@ -396,7 +396,7 @@ describe('OrcaRuntimeService', () => {
|
||||
})
|
||||
|
||||
try {
|
||||
await runtime.removeManagedWorktree('path:/remote/feature', true, false)
|
||||
await runtime.removeManagedWorktree('path:/remote/feature', { force: true, runHooks: false })
|
||||
} finally {
|
||||
unregisterSshGitProvider('ssh-1')
|
||||
}
|
||||
@@ -472,7 +472,7 @@ describe('OrcaRuntimeService', () => {
|
||||
runtime.registerPty('pty-local-same-id', `${TEST_REPO_ID}::/remote/feature`, null)
|
||||
|
||||
try {
|
||||
await runtime.removeManagedWorktree('path:/remote/feature', true, false)
|
||||
await runtime.removeManagedWorktree('path:/remote/feature', { force: true, runHooks: false })
|
||||
} finally {
|
||||
unregisterSshGitProvider('ssh-1')
|
||||
}
|
||||
@@ -519,9 +519,9 @@ describe('OrcaRuntimeService', () => {
|
||||
const runtime = new OrcaRuntimeService(remoteStore as never)
|
||||
|
||||
try {
|
||||
await expect(runtime.removeManagedWorktree('path:/remote/repo', true)).rejects.toThrow(
|
||||
'Refusing to delete protected worktree path: /remote/repo'
|
||||
)
|
||||
await expect(
|
||||
runtime.removeManagedWorktree('path:/remote/repo', { force: true })
|
||||
).rejects.toThrow('Refusing to delete protected worktree path: /remote/repo')
|
||||
} finally {
|
||||
unregisterSshGitProvider('ssh-1')
|
||||
}
|
||||
|
||||
+8
-6
@@ -298,7 +298,7 @@ describe('OrcaRuntimeService', () => {
|
||||
.mockResolvedValue([])
|
||||
|
||||
try {
|
||||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)
|
||||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true })
|
||||
|
||||
expect(result).toEqual({
|
||||
preservedBranch: { branchName: 'feature/foo', head: 'abc' },
|
||||
@@ -340,7 +340,9 @@ describe('OrcaRuntimeService', () => {
|
||||
)
|
||||
|
||||
try {
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)).rejects.toThrow(
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true })
|
||||
).rejects.toThrow(
|
||||
`Failed to force delete worktree at ${TEST_WORKTREE_PATH}. error: failed to delete deep/file.txt: Filename too long`
|
||||
)
|
||||
expect(removePathSpy).not.toHaveBeenCalled()
|
||||
@@ -387,7 +389,7 @@ describe('OrcaRuntimeService', () => {
|
||||
})
|
||||
|
||||
try {
|
||||
const result = await runtime.removeManagedWorktree(worktreeId, true)
|
||||
const result = await runtime.removeManagedWorktree(worktreeId, { force: true })
|
||||
|
||||
expect(result).toEqual({
|
||||
preservedBranch: { branchName: 'feature/foo', head: 'abc' }
|
||||
@@ -425,9 +427,9 @@ describe('OrcaRuntimeService', () => {
|
||||
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'
|
||||
)
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(worktreeId, { force: true, runHooks: false })
|
||||
).rejects.toThrow('Worktree is locked by Git. Lock reason: active agent session')
|
||||
|
||||
expect(removeWorktree).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
|
||||
+28
-17
@@ -94,7 +94,12 @@ describe('OrcaRuntimeService', () => {
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
|
||||
try {
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, false, false, false, 'ssh:ssh-1')
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'ssh:ssh-1'
|
||||
})
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith(TEST_WORKTREE_PATH, false)
|
||||
expect(metaById[TEST_WORKTREE_ID]?.hostId).toBe('local')
|
||||
const result = await runtime.forceDeletePreservedBranch(
|
||||
@@ -181,8 +186,8 @@ describe('OrcaRuntimeService', () => {
|
||||
return {}
|
||||
})
|
||||
|
||||
const first = runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)
|
||||
const second = runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)
|
||||
const first = runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true })
|
||||
const second = runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true })
|
||||
|
||||
await removeStarted.promise
|
||||
await Promise.resolve()
|
||||
@@ -238,14 +243,18 @@ describe('OrcaRuntimeService', () => {
|
||||
registerSshGitProvider('host-b', provider as never)
|
||||
|
||||
try {
|
||||
const local = runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'local')
|
||||
const remote = runtime.removeManagedWorktree(
|
||||
TEST_WORKTREE_ID,
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'ssh:host-b'
|
||||
)
|
||||
const local = runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'local'
|
||||
})
|
||||
const remote = runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'ssh:host-b'
|
||||
})
|
||||
|
||||
await bothStarted.promise
|
||||
expect(removeWorktree).toHaveBeenCalledTimes(1)
|
||||
@@ -271,7 +280,7 @@ describe('OrcaRuntimeService', () => {
|
||||
const first = runtime.removeManagedWorktree(TEST_WORKTREE_ID)
|
||||
|
||||
await removeStarted.promise
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true)).rejects.toThrow(
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true })).rejects.toThrow(
|
||||
'Worktree deletion already in progress'
|
||||
)
|
||||
|
||||
@@ -292,7 +301,7 @@ describe('OrcaRuntimeService', () => {
|
||||
try {
|
||||
vi.mocked(listWorktrees).mockResolvedValue([])
|
||||
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, true)).resolves.toEqual({})
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, { force: true })).resolves.toEqual({})
|
||||
|
||||
expect(removeWorktree).not.toHaveBeenCalled()
|
||||
// The repo resolved to the local host, so the metadata purge names it —
|
||||
@@ -458,7 +467,9 @@ describe('OrcaRuntimeService', () => {
|
||||
})
|
||||
|
||||
try {
|
||||
await expect(runtime.removeManagedWorktree(`id:${worktreeId}`, true)).resolves.toEqual({})
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(`id:${worktreeId}`, { force: true })
|
||||
).resolves.toEqual({})
|
||||
} finally {
|
||||
unregisterSshGitProvider(repo.connectionId)
|
||||
unregisterSshFilesystemProvider(repo.connectionId)
|
||||
@@ -513,7 +524,7 @@ describe('OrcaRuntimeService', () => {
|
||||
try {
|
||||
vi.mocked(listWorktrees).mockResolvedValue([])
|
||||
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, true)).resolves.toEqual({})
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, { force: true })).resolves.toEqual({})
|
||||
|
||||
await expect(lstat(orphanPath)).rejects.toMatchObject({ code: 'ENOENT' })
|
||||
expect(closeLocalWatcherForWorktreePathMock).toHaveBeenCalledWith(
|
||||
@@ -586,7 +597,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect(removeWorktree).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, true)).resolves.toEqual({})
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, { force: true })).resolves.toEqual({})
|
||||
|
||||
await expect(lstat(leftoverPath)).rejects.toMatchObject({ code: 'ENOENT' })
|
||||
expect(assertWorktreeCleanForRemoval).not.toHaveBeenCalled()
|
||||
@@ -642,7 +653,7 @@ describe('OrcaRuntimeService', () => {
|
||||
try {
|
||||
vi.mocked(listWorktrees).mockResolvedValue([])
|
||||
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, true)).rejects.toThrow(
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, { force: true })).rejects.toThrow(
|
||||
`Refusing to delete unregistered worktree path: ${standalonePath}`
|
||||
)
|
||||
|
||||
|
||||
+14
-10
@@ -90,9 +90,9 @@ describe('OrcaRuntimeService', () => {
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
|
||||
try {
|
||||
await expect(runtime.removeManagedWorktree(`id:${worktreeId}`, true)).rejects.toThrow(
|
||||
'SSH filesystem provider unavailable'
|
||||
)
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(`id:${worktreeId}`, { force: true })
|
||||
).rejects.toThrow('SSH filesystem provider unavailable')
|
||||
|
||||
await expect(lstat(localPath)).resolves.toBeTruthy()
|
||||
expect(removeWorktree).not.toHaveBeenCalled()
|
||||
@@ -112,7 +112,7 @@ describe('OrcaRuntimeService', () => {
|
||||
try {
|
||||
vi.mocked(listWorktrees).mockResolvedValue([])
|
||||
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, true)).rejects.toThrow(
|
||||
await expect(runtime.removeManagedWorktree(worktreeId, { force: true })).rejects.toThrow(
|
||||
'Refusing to delete unregistered worktree path'
|
||||
)
|
||||
|
||||
@@ -177,7 +177,9 @@ describe('OrcaRuntimeService', () => {
|
||||
}
|
||||
})
|
||||
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, true)).rejects.toThrow(
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true, runHooks: true })
|
||||
).rejects.toThrow(
|
||||
`Refusing to delete worktree because it contains another registered worktree: ${TEST_WORKTREE_PATH}/child`
|
||||
)
|
||||
|
||||
@@ -238,7 +240,9 @@ describe('OrcaRuntimeService', () => {
|
||||
}
|
||||
])
|
||||
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, true)).rejects.toThrow(
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true, runHooks: true })
|
||||
).rejects.toThrow(
|
||||
`Failed to force delete worktree at ${TEST_WORKTREE_PATH}. Worktree is locked by Git.`
|
||||
)
|
||||
|
||||
@@ -278,9 +282,9 @@ describe('OrcaRuntimeService', () => {
|
||||
}
|
||||
])
|
||||
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, true)).rejects.toThrow(
|
||||
'Worktree is locked by Git'
|
||||
)
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: true, runHooks: true })
|
||||
).rejects.toThrow('Worktree is locked by Git')
|
||||
|
||||
expect(runHook).toHaveBeenCalled()
|
||||
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
|
||||
@@ -377,7 +381,7 @@ describe('OrcaRuntimeService', () => {
|
||||
vi.mocked(runHook).mockResolvedValue({ success: true, output: '' })
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, false, true)
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, { force: false, runHooks: true })
|
||||
|
||||
expect(runHook).toHaveBeenCalledWith(
|
||||
'archive',
|
||||
|
||||
@@ -612,7 +612,12 @@ describe('OrcaRuntimeService', () => {
|
||||
})
|
||||
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, false, false, false, 'runtime:env-b')
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'runtime:env-b'
|
||||
})
|
||||
).rejects.toThrow('no longer belongs to runtime:env-b')
|
||||
|
||||
expect(localProvider.listProcesses).not.toHaveBeenCalled()
|
||||
|
||||
@@ -0,0 +1,242 @@
|
||||
// Regression cover for #19334: a failed archive hook used to be logged and stepped over, so the
|
||||
// checkout was deleted with nothing archived. The hook is a blocking precondition now.
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
assertWorktreeCleanForRemoval,
|
||||
deleteWorktreeHistoryDirMock,
|
||||
getEffectiveHooks,
|
||||
invalidateAuthorizedRootsCacheMock,
|
||||
listWorktreesStrict,
|
||||
removeWorktree,
|
||||
removeWorktreeLinkedPathsMock,
|
||||
runHook
|
||||
} from '../orca-runtime-test-mocks.spec'
|
||||
import {
|
||||
TEST_REPO_PATH,
|
||||
TEST_WORKTREE_ID,
|
||||
TEST_WORKTREE_PATH,
|
||||
createStaleRuntimeWorktreeStore,
|
||||
deferred
|
||||
} from '../orca-runtime-test-fixtures.spec'
|
||||
import { createWorktreeRemovalRuntime } from '../orca-runtime-test-scenario-builders.spec'
|
||||
import {
|
||||
ARCHIVE_HOOK_FAILED_REMOVAL_CODE,
|
||||
asArchiveHookRefusal
|
||||
} from '../../../shared/worktree/archive-hook-removal-gate'
|
||||
|
||||
function withArchiveHook(): void {
|
||||
vi.mocked(getEffectiveHooks).mockReturnValue({
|
||||
scripts: { archive: 'pnpm worktree:archive' }
|
||||
})
|
||||
}
|
||||
|
||||
function expectNothingMutated(removeWorktreeMeta: ReturnType<typeof vi.fn>): void {
|
||||
// The checkout, its Git registration, its agents and Orca's ownership evidence all survive.
|
||||
expect(removeWorktree).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
|
||||
expect(deleteWorktreeHistoryDirMock).not.toHaveBeenCalled()
|
||||
expect(invalidateAuthorizedRootsCacheMock).not.toHaveBeenCalled()
|
||||
// The gate runs before the registration re-read, so even the preflights never start. The one
|
||||
// listing is the orchestrator's own lookup ahead of the hook; the post-hook refresh never runs.
|
||||
expect(listWorktreesStrict).toHaveBeenCalledTimes(1)
|
||||
expect(assertWorktreeCleanForRemoval).not.toHaveBeenCalled()
|
||||
}
|
||||
|
||||
describe('archive hook removal gate', () => {
|
||||
// These specs are imported into one aggregate test file, so the module-level mocks arrive with
|
||||
// calls from earlier specs. Clear counts here and restore the shared defaults afterwards.
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.mocked(getEffectiveHooks).mockReturnValue(null)
|
||||
vi.mocked(runHook).mockResolvedValue({ success: true, output: '' })
|
||||
})
|
||||
|
||||
it('refuses removal and mutates nothing when the archive hook exits 23', async () => {
|
||||
const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID)
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
withArchiveHook()
|
||||
vi.mocked(runHook).mockResolvedValue({
|
||||
success: false,
|
||||
output: 'backup target unreachable',
|
||||
exitCode: 23
|
||||
})
|
||||
|
||||
const failure = await runtime
|
||||
.removeManagedWorktree(TEST_WORKTREE_ID, { force: false, runHooks: true })
|
||||
.catch((error: unknown) => error)
|
||||
|
||||
const refusal = asArchiveHookRefusal(failure)
|
||||
expect(refusal.code).toBe(ARCHIVE_HOOK_FAILED_REMOVAL_CODE)
|
||||
expect(refusal.data).toEqual({
|
||||
worktreePath: TEST_WORKTREE_PATH,
|
||||
outcome: 'exited',
|
||||
exitCode: 23,
|
||||
output: 'backup target unreachable'
|
||||
})
|
||||
expectNothingMutated(removeWorktreeMeta)
|
||||
})
|
||||
|
||||
it('refuses removal when the hook never reported an exit, without claiming it passed', async () => {
|
||||
const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID)
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
withArchiveHook()
|
||||
// A timeout or a lost execution host yields no exit code: `unverifiable`, never a pass.
|
||||
vi.mocked(runHook).mockResolvedValue({
|
||||
success: false,
|
||||
output: 'Hook timed out after 120000ms.'
|
||||
})
|
||||
|
||||
const failure = await runtime
|
||||
.removeManagedWorktree(TEST_WORKTREE_ID, { force: false, runHooks: true })
|
||||
.catch((error: unknown) => error)
|
||||
|
||||
const refusal = asArchiveHookRefusal(failure)
|
||||
expect(refusal.data).toEqual({
|
||||
worktreePath: TEST_WORKTREE_PATH,
|
||||
outcome: 'unverifiable',
|
||||
output: 'Hook timed out after 120000ms.'
|
||||
})
|
||||
expectNothingMutated(removeWorktreeMeta)
|
||||
})
|
||||
|
||||
it('does not let --force waive a failed archive hook', async () => {
|
||||
const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID)
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
withArchiveHook()
|
||||
vi.mocked(runHook).mockResolvedValue({
|
||||
success: false,
|
||||
output: 'boom',
|
||||
exitCode: 23
|
||||
})
|
||||
|
||||
await expect(
|
||||
// force + the PTY-stop waiver, i.e. everything the desktop Force Delete sets.
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: true,
|
||||
allowUnverifiedPtyStop: true
|
||||
})
|
||||
).rejects.toMatchObject({ code: ARCHIVE_HOOK_FAILED_REMOVAL_CODE })
|
||||
expectNothingMutated(removeWorktreeMeta)
|
||||
})
|
||||
|
||||
it('removes and records the waiver when the failure is explicitly overridden', async () => {
|
||||
const runtime = createWorktreeRemovalRuntime()
|
||||
withArchiveHook()
|
||||
vi.mocked(runHook).mockResolvedValue({
|
||||
success: false,
|
||||
output: 'boom',
|
||||
exitCode: 23
|
||||
})
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: true,
|
||||
allowUnverifiedPtyStop: false,
|
||||
allowFailedArchiveHook: true
|
||||
})
|
||||
|
||||
expect(result.archiveHookOverride).toEqual({
|
||||
worktreePath: TEST_WORKTREE_PATH,
|
||||
outcome: 'exited',
|
||||
exitCode: 23,
|
||||
output: 'boom',
|
||||
overridden: true
|
||||
})
|
||||
expect(removeWorktree).toHaveBeenCalledWith(
|
||||
TEST_REPO_PATH,
|
||||
TEST_WORKTREE_PATH,
|
||||
false,
|
||||
expect.objectContaining({
|
||||
knownRemovedWorktree: expect.objectContaining({
|
||||
path: TEST_WORKTREE_PATH
|
||||
})
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('removes without an override record when the hook succeeds', async () => {
|
||||
const runtime = createWorktreeRemovalRuntime()
|
||||
withArchiveHook()
|
||||
vi.mocked(runHook).mockResolvedValue({
|
||||
success: true,
|
||||
output: '',
|
||||
exitCode: 0
|
||||
})
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: true
|
||||
})
|
||||
|
||||
expect(result.archiveHookOverride).toBeUndefined()
|
||||
expect(removeWorktree).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('removes when the hook is configured but not requested', async () => {
|
||||
const runtime = createWorktreeRemovalRuntime()
|
||||
withArchiveHook()
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID)
|
||||
|
||||
expect(runHook).not.toHaveBeenCalled()
|
||||
expect(result.warning).toContain('archive hook skipped')
|
||||
expect(removeWorktree).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('removes when no archive hook is configured', async () => {
|
||||
const runtime = createWorktreeRemovalRuntime()
|
||||
vi.mocked(getEffectiveHooks).mockReturnValue(null)
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: true
|
||||
})
|
||||
|
||||
expect(runHook).not.toHaveBeenCalled()
|
||||
expect(result.warning).toBeUndefined()
|
||||
expect(removeWorktree).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not coalesce an override retry onto the refusal already in flight', async () => {
|
||||
const runtime = createWorktreeRemovalRuntime()
|
||||
withArchiveHook()
|
||||
const hookRun = deferred<{
|
||||
success: boolean
|
||||
output: string
|
||||
exitCode?: number
|
||||
}>()
|
||||
vi.mocked(runHook).mockReturnValue(hookRun.promise)
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
const refused = runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: true
|
||||
})
|
||||
await vi.waitFor(() => expect(runHook).toHaveBeenCalled())
|
||||
|
||||
// The waiver is part of the in-flight options key, so a concurrent waived retry is refused
|
||||
// outright rather than handed the in-flight attempt that is about to reject on the hook.
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: false,
|
||||
runHooks: true,
|
||||
allowUnverifiedPtyStop: false,
|
||||
allowFailedArchiveHook: true
|
||||
})
|
||||
).rejects.toThrow('Worktree deletion already in progress')
|
||||
|
||||
hookRun.resolve({ success: false, output: 'boom', exitCode: 23 })
|
||||
await expect(refused).rejects.toMatchObject({
|
||||
code: ARCHIVE_HOOK_FAILED_REMOVAL_CODE
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -102,7 +102,12 @@ describe('OrcaRuntimeService worktree removal execution host', () => {
|
||||
vi.spyOn(runtime, 'acquireFileWatcherRemoval').mockResolvedValue({ finish: vi.fn() })
|
||||
|
||||
try {
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'ssh:target-a')
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'ssh:target-a'
|
||||
})
|
||||
|
||||
expect(provider.listWorktrees).toHaveBeenCalledWith(REMOTE_REPO_PATH)
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith(TEST_WORKTREE_PATH, true)
|
||||
@@ -127,7 +132,12 @@ describe('OrcaRuntimeService worktree removal execution host', () => {
|
||||
|
||||
try {
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'ssh:target-a')
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'ssh:target-a'
|
||||
})
|
||||
).resolves.toEqual({})
|
||||
|
||||
expect(provider.listWorktrees).toHaveBeenCalledWith(REMOTE_REPO_PATH)
|
||||
@@ -152,7 +162,12 @@ describe('OrcaRuntimeService worktree removal execution host', () => {
|
||||
vi.spyOn(runtime, 'acquireFileWatcherRemoval').mockResolvedValue({ finish: vi.fn() })
|
||||
|
||||
try {
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'ssh:target-b')
|
||||
await runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'ssh:target-b'
|
||||
})
|
||||
|
||||
expect(providerB.removeWorktree).toHaveBeenCalledWith(TEST_WORKTREE_PATH, true)
|
||||
expect(providerA.listWorktrees).not.toHaveBeenCalled()
|
||||
@@ -168,7 +183,12 @@ describe('OrcaRuntimeService worktree removal execution host', () => {
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'ssh:target-a')
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'ssh:target-a'
|
||||
})
|
||||
).rejects.toThrow('Remote connection dropped')
|
||||
|
||||
expect(listWorktreesStrict).not.toHaveBeenCalled()
|
||||
@@ -181,7 +201,12 @@ describe('OrcaRuntimeService worktree removal execution host', () => {
|
||||
const runtime = createWorktreeRemovalRuntime(runtimeStore)
|
||||
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'runtime:env-1')
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'runtime:env-1'
|
||||
})
|
||||
).rejects.toThrow('not dispatched by this process')
|
||||
|
||||
expect(listWorktreesStrict).not.toHaveBeenCalled()
|
||||
@@ -201,7 +226,12 @@ describe('OrcaRuntimeService worktree removal execution host', () => {
|
||||
|
||||
try {
|
||||
await expect(
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, true, false, false, 'runtime:env-1')
|
||||
runtime.removeManagedWorktree(TEST_WORKTREE_ID, {
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
hostId: 'runtime:env-1'
|
||||
})
|
||||
).rejects.toThrow('not dispatched by this process')
|
||||
|
||||
// Selector resolution still lists through the raw field before removal begins — a read on
|
||||
|
||||
@@ -110,6 +110,7 @@ await import('./orca-runtime-tests/worktree-removal-and-reconciliation.spec')
|
||||
await import('./orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec')
|
||||
await import('./orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec')
|
||||
await import('./orca-runtime-tests/worktree-removal-and-reconciliation-part-04.spec')
|
||||
await import('./orca-runtime-tests/worktree-removal-archive-hook-gate.spec')
|
||||
await import('./orca-runtime-tests/worktree-removal-execution-host.spec')
|
||||
await import('./orca-runtime-tests/targeting-and-resilience.spec')
|
||||
await import('./orca-runtime-tests/worktree-scan-cache-ttl.spec')
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
} from '../../../shared/skill-install-failure'
|
||||
import { GIT_DIFF_TOO_LARGE_CODE } from '../../../shared/git-diff-transport-budget'
|
||||
import { AUTOMATION_OWNER_CONFLICT_CODES } from '../../../shared/automation-owner-conflict'
|
||||
import { ARCHIVE_HOOK_FAILED_REMOVAL_CODE } from '../../../shared/worktree/archive-hook-removal-gate'
|
||||
import { NESTED_WORKER_DEPTH_EXCEEDED_CODE } from '../../../shared/nested-worker-depth'
|
||||
|
||||
export function successResponse(id: string, meta: RpcEnvelopeMeta, result: unknown): RpcSuccess {
|
||||
@@ -126,6 +127,9 @@ const STRUCTURED_RUNTIME_PASSTHROUGH_CODES: ReadonlySet<string> = new Set([
|
||||
'stale_delivery',
|
||||
'waiter_exists',
|
||||
'invalid_argument',
|
||||
// Why (#19334): "your archive hook failed, nothing was deleted" is a distinct decision — retry,
|
||||
// waive, or skip the hook. Flattened to runtime_error a caller can only pattern-match the text.
|
||||
ARCHIVE_HOOK_FAILED_REMOVAL_CODE,
|
||||
NESTED_WORKER_DEPTH_EXCEEDED_CODE,
|
||||
GIT_DIFF_TOO_LARGE_CODE,
|
||||
ARTIFACT_SHARING_DISABLED_CODE,
|
||||
|
||||
@@ -20,6 +20,15 @@ function makeRequest(params: unknown): RpcRequest {
|
||||
return { id: 'req-1', authToken: 'tok', method: 'worktree.rm', params }
|
||||
}
|
||||
|
||||
/** The removal options every case forwards; only the resolved host differs. */
|
||||
const forwarded = (hostId?: string): Record<string, unknown> => ({
|
||||
force: true,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
allowFailedArchiveHook: false,
|
||||
...(hostId ? { hostId } : {})
|
||||
})
|
||||
|
||||
describe('worktree.rm host qualification', () => {
|
||||
it('routes an explicitly qualified removal to that host', async () => {
|
||||
const runtime = makeRuntime()
|
||||
@@ -29,13 +38,7 @@ describe('worktree.rm host qualification', () => {
|
||||
makeRequest({ worktree: 'id:wt-1', hostId: 'local', force: true, runHooks: false })
|
||||
)
|
||||
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'local'
|
||||
)
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-1', forwarded('local'))
|
||||
expect(response).toMatchObject({ ok: true, result: { removed: true } })
|
||||
})
|
||||
|
||||
@@ -54,10 +57,7 @@ describe('worktree.rm host qualification', () => {
|
||||
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
`id:${WORKTREE_ID}`,
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'local'
|
||||
forwarded('local')
|
||||
)
|
||||
expect(response).toMatchObject({ ok: true, result: { removed: true } })
|
||||
})
|
||||
@@ -77,10 +77,7 @@ describe('worktree.rm host qualification', () => {
|
||||
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
`id:${WORKTREE_ID}`,
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'runtime:env-1'
|
||||
forwarded('runtime:env-1')
|
||||
)
|
||||
})
|
||||
|
||||
@@ -119,10 +116,7 @@ describe('worktree.rm host qualification', () => {
|
||||
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
`id:${WORKTREE_ID}`,
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'ssh:target-a'
|
||||
forwarded('ssh:target-a')
|
||||
)
|
||||
})
|
||||
|
||||
@@ -151,13 +145,7 @@ describe('worktree.rm host qualification', () => {
|
||||
)
|
||||
|
||||
expect(runtime.showManagedWorktree).toHaveBeenCalledWith('id:wt-1')
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'local'
|
||||
)
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-1', forwarded('local'))
|
||||
expect(response).toMatchObject({ ok: true, result: { removed: true } })
|
||||
})
|
||||
|
||||
@@ -205,13 +193,7 @@ describe('worktree.rm host qualification', () => {
|
||||
expect(response).toMatchObject({ ok: true, result: { removed: true } })
|
||||
// Unqualified on purpose: removeManagedWorktree owns the stale-row path and
|
||||
// still refuses on its own if the id turns out to have two owners.
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
'id:wt-gone',
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
undefined
|
||||
)
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-gone', forwarded())
|
||||
})
|
||||
|
||||
it('propagates a non-missing lookup failure instead of deleting unqualified', async () => {
|
||||
|
||||
@@ -14,74 +14,75 @@ function makeRuntime(): OrcaRuntimeService {
|
||||
} as unknown as OrcaRuntimeService
|
||||
}
|
||||
|
||||
// Why (#11960): waiving the proof that every PTY stopped must ride its own field.
|
||||
// The desktop sets `force` for an ordinary confirmed delete, so keying the waiver
|
||||
// off `force` would silently disable the gate on the primary delete path.
|
||||
describe('worktree.rm PTY-stop waiver', () => {
|
||||
it('forwards an explicit waiver to the runtime', async () => {
|
||||
/** The dispatcher validates against the Zod schema, so the test spells the wire shape. */
|
||||
type RmParams = {
|
||||
hostId?: string
|
||||
force?: boolean
|
||||
runHooks?: boolean
|
||||
allowUnverifiedPtyStop?: boolean
|
||||
allowFailedArchiveHook?: boolean
|
||||
}
|
||||
|
||||
async function dispatchRm(runtime: OrcaRuntimeService, params: RmParams): Promise<void> {
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS })
|
||||
const request: RpcRequest = {
|
||||
id: 'req-1',
|
||||
authToken: 'tok',
|
||||
method: 'worktree.rm',
|
||||
params: { worktree: 'id:wt-1', ...params }
|
||||
}
|
||||
await dispatcher.dispatch(request)
|
||||
}
|
||||
|
||||
/** Every waiver off unless a case turns it on — the defaults are the assertion. */
|
||||
const forwarded = (overrides: Partial<Record<string, unknown>> = {}): Record<string, unknown> => ({
|
||||
force: false,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
allowFailedArchiveHook: false,
|
||||
hostId: 'local',
|
||||
...overrides
|
||||
})
|
||||
|
||||
// Why (#11960 and #19334): each waiver rides its own field. The desktop sets `force` for an
|
||||
// ordinary confirmed delete, so keying either waiver off `force` would silently disable that gate
|
||||
// on the primary delete path. These cases exist to keep `force` from acquiring a second meaning.
|
||||
describe('worktree.rm waivers travel on their own fields', () => {
|
||||
it.each([
|
||||
[
|
||||
'an explicit PTY-stop waiver reaches the runtime',
|
||||
{ hostId: 'local', force: true, allowUnverifiedPtyStop: true, runHooks: false },
|
||||
forwarded({ force: true, allowUnverifiedPtyStop: true })
|
||||
],
|
||||
[
|
||||
'force alone does NOT waive the PTY-stop proof',
|
||||
{ hostId: 'local', force: true, runHooks: false },
|
||||
forwarded({ force: true })
|
||||
],
|
||||
[
|
||||
'an explicit archive-hook waiver reaches the runtime',
|
||||
{ hostId: 'local', runHooks: true, allowFailedArchiveHook: true },
|
||||
forwarded({ runHooks: true, allowFailedArchiveHook: true })
|
||||
],
|
||||
[
|
||||
'force plus a PTY waiver does NOT waive a failed archive hook',
|
||||
{ hostId: 'local', force: true, allowUnverifiedPtyStop: true, runHooks: true },
|
||||
forwarded({ force: true, runHooks: true, allowUnverifiedPtyStop: true })
|
||||
]
|
||||
])('%s', async (_name, params, expected) => {
|
||||
const runtime = makeRuntime()
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS })
|
||||
|
||||
await dispatcher.dispatch({
|
||||
id: 'req-1',
|
||||
authToken: 'tok',
|
||||
method: 'worktree.rm',
|
||||
params: {
|
||||
worktree: 'id:wt-1',
|
||||
hostId: 'local',
|
||||
force: true,
|
||||
allowUnverifiedPtyStop: true,
|
||||
runHooks: false
|
||||
}
|
||||
} satisfies RpcRequest)
|
||||
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
true,
|
||||
false,
|
||||
true,
|
||||
'local'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not infer a waiver from force alone', async () => {
|
||||
const runtime = makeRuntime()
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS })
|
||||
|
||||
await dispatcher.dispatch({
|
||||
id: 'req-1',
|
||||
authToken: 'tok',
|
||||
method: 'worktree.rm',
|
||||
params: { worktree: 'id:wt-1', hostId: 'local', force: true, runHooks: false }
|
||||
} satisfies RpcRequest)
|
||||
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'local'
|
||||
)
|
||||
await dispatchRm(runtime, params)
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-1', expected)
|
||||
})
|
||||
|
||||
it('resolves the host before forwarding an unqualified removal', async () => {
|
||||
const runtime = makeRuntime()
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS })
|
||||
|
||||
await dispatcher.dispatch({
|
||||
id: 'req-1',
|
||||
authToken: 'tok',
|
||||
method: 'worktree.rm',
|
||||
params: { worktree: 'id:wt-1', force: true, runHooks: false }
|
||||
} satisfies RpcRequest)
|
||||
await dispatchRm(runtime, { force: true, runHooks: false })
|
||||
|
||||
expect(runtime.showManagedWorktree).toHaveBeenCalledWith('id:wt-1')
|
||||
expect(runtime.removeManagedWorktree).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
'ssh:builder'
|
||||
forwarded({ force: true, hostId: 'ssh:builder' })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -235,13 +235,13 @@ export const WORKTREE_METHODS = [
|
||||
}
|
||||
}
|
||||
}
|
||||
const removalArgs = [
|
||||
params.worktree,
|
||||
params.force === true,
|
||||
params.runHooks === true,
|
||||
params.allowUnverifiedPtyStop === true
|
||||
] as const
|
||||
const result = await runtime.removeManagedWorktree(...removalArgs, resolvedHostId)
|
||||
const result = await runtime.removeManagedWorktree(params.worktree, {
|
||||
force: params.force === true,
|
||||
runHooks: params.runHooks === true,
|
||||
allowUnverifiedPtyStop: params.allowUnverifiedPtyStop === true,
|
||||
allowFailedArchiveHook: params.allowFailedArchiveHook === true,
|
||||
...(resolvedHostId ? { hostId: resolvedHostId } : {})
|
||||
})
|
||||
return { removed: true, ...result }
|
||||
}
|
||||
}),
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import type { GitPushTarget, GitWorktreeInfo } from '../../shared/worktree/types'
|
||||
import type { RemoveWorktreeResult } from '../../shared/worktree/create-types'
|
||||
import type { ArchiveHookOverride } from '../../shared/worktree/archive-hook-removal-gate'
|
||||
import { gateWorktreeRemovalOnArchiveHook } from '../worktree-archive-hook-gate'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree/removal'
|
||||
import type { LocalProjectWorktreeGitOptions } from '../project-runtime-git-options'
|
||||
@@ -40,6 +42,8 @@ export async function removeRuntimeRegisteredLocalWorktree(args: {
|
||||
hasLocalOptions: boolean
|
||||
force: boolean
|
||||
runHooks: boolean
|
||||
/** Explicit waiver for a FAILED archive hook. Never implied by `force` — see #19334. */
|
||||
allowFailedArchiveHook: boolean
|
||||
allowUnverifiedPtyStop: boolean
|
||||
deleteBranch: boolean
|
||||
acquireWatcherRemoval: (path: string) => Promise<{ finish: (removed: boolean) => Promise<void> }>
|
||||
@@ -60,6 +64,9 @@ export async function removeRuntimeRegisteredLocalWorktree(args: {
|
||||
const canonicalPath = registeredWorktree.path
|
||||
const hooks = getEffectiveHooks(repo)
|
||||
let warning: string | undefined
|
||||
// Precondition, not an advisory: this runs before the registration refresh, the preflights, the
|
||||
// PTY stop and `removeWorktree`, so a throw here leaves every one of them untouched (#19334).
|
||||
let archiveHookOverride: ArchiveHookOverride | undefined
|
||||
if (hooks?.scripts.archive && args.runHooks) {
|
||||
const result = await runHook(
|
||||
'archive',
|
||||
@@ -68,9 +75,11 @@ export async function removeRuntimeRegisteredLocalWorktree(args: {
|
||||
undefined,
|
||||
args.hasLocalOptions ? localOptions : undefined
|
||||
)
|
||||
if (!result.success) {
|
||||
console.error(`[hooks] archive hook failed for ${canonicalPath}:`, result.output)
|
||||
}
|
||||
archiveHookOverride = gateWorktreeRemovalOnArchiveHook({
|
||||
worktreePath: canonicalPath,
|
||||
result,
|
||||
allowFailure: args.allowFailedArchiveHook
|
||||
})
|
||||
} else if (hooks?.scripts.archive) {
|
||||
warning = `orca.yaml archive hook skipped for ${canonicalPath}; pass --run-hooks to run it.`
|
||||
console.warn(`[hooks] ${warning}`)
|
||||
@@ -151,7 +160,10 @@ export async function removeRuntimeRegisteredLocalWorktree(args: {
|
||||
await cleanupPushTarget(args)
|
||||
args.finishRemoval(undefined, false, refreshed.head)
|
||||
completed = true
|
||||
return warning ? { warning } : {}
|
||||
return {
|
||||
...(archiveHookOverride ? { archiveHookOverride } : {}),
|
||||
...(warning ? { warning } : {})
|
||||
}
|
||||
} else {
|
||||
throw new Error(formatWorktreeRemovalError(error, canonicalPath, args.force))
|
||||
}
|
||||
@@ -162,7 +174,11 @@ export async function removeRuntimeRegisteredLocalWorktree(args: {
|
||||
}
|
||||
await cleanupPushTarget(args)
|
||||
args.finishRemoval(removalResult, true, refreshed.head)
|
||||
return { ...removalResult, ...(warning ? { warning } : {}) }
|
||||
return {
|
||||
...removalResult,
|
||||
...(archiveHookOverride ? { archiveHookOverride } : {}),
|
||||
...(warning ? { warning } : {})
|
||||
}
|
||||
}
|
||||
|
||||
async function cleanupOrphanedDirectory(
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { SshGitProvider } from '../providers/ssh-git-provider'
|
||||
import { cleanupUnusedWorktreePushTargetRemoteSsh } from '../ipc/worktree-remote'
|
||||
import type { RuntimeStore } from './runtime-store-contract'
|
||||
import type { RuntimeWorktreeRemovalTarget } from './runtime-worktree-selection'
|
||||
import { gateRemovalWhereArchiveHookCannotRun } from '../worktree-archive-hook-gate'
|
||||
|
||||
export async function removeRuntimeRegisteredRemoteWorktree(args: {
|
||||
repo: Repo
|
||||
@@ -15,6 +16,10 @@ export async function removeRuntimeRegisteredRemoteWorktree(args: {
|
||||
provider: SshGitProvider
|
||||
/** From the resolved removal route; `repo.connectionId!` answered null for an `ssh:`-only row. */
|
||||
connectionId: string
|
||||
/** #19334: this path runs no archive hook, so the gate below decides what that means. */
|
||||
runHooks: boolean
|
||||
/** Explicit waiver for that refusal; without it the block has no exit on this path. */
|
||||
allowFailedArchiveHook: boolean
|
||||
force: boolean
|
||||
allowUnverifiedPtyStop: boolean
|
||||
deleteBranch: boolean
|
||||
@@ -29,8 +34,17 @@ export async function removeRuntimeRegisteredRemoteWorktree(args: {
|
||||
fallbackHead: string | undefined
|
||||
) => RemoveWorktreeResult
|
||||
finishRemoval: (result: RemoveWorktreeResult) => void
|
||||
}): Promise<RemoveWorktreeResult> {
|
||||
}): Promise<RemoveWorktreeResult & { warning?: string }> {
|
||||
const { repo, target, registeredWorktree, provider, connectionId } = args
|
||||
// Precondition, before anything is stopped or deleted: no archive hook runs here, so a removal
|
||||
// that asked for one refuses rather than deleting with the archive step silently skipped.
|
||||
const hookGate = await gateRemovalWhereArchiveHookCannotRun({
|
||||
repo,
|
||||
connectionId,
|
||||
worktreePath: registeredWorktree.path,
|
||||
runHooks: args.runHooks,
|
||||
allowFailedArchiveHook: args.allowFailedArchiveHook
|
||||
})
|
||||
const removeOptions = !args.deleteBranch ? { deleteBranch: args.deleteBranch } : {}
|
||||
const gate = await args.acquireWatcherRemoval(registeredWorktree.path, connectionId)
|
||||
let rawResult: RemoveWorktreeResult | undefined
|
||||
@@ -54,5 +68,9 @@ export async function removeRuntimeRegisteredRemoteWorktree(args: {
|
||||
)
|
||||
await args.deleteHistory()
|
||||
args.finishRemoval(result)
|
||||
return result
|
||||
return {
|
||||
...result,
|
||||
...(hookGate.override ? { archiveHookOverride: hookGate.override } : {}),
|
||||
...(hookGate.warning ? { warning: hookGate.warning } : {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,39 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { runtimeRepoMatchesExecutionHost } from './runtime-worktree-selection'
|
||||
import {
|
||||
getRuntimeWorktreeRemovalOptionsKey,
|
||||
runtimeRepoMatchesExecutionHost
|
||||
} from './runtime-worktree-selection'
|
||||
|
||||
describe('getRuntimeWorktreeRemovalOptionsKey', () => {
|
||||
it('separates a waived archive-hook retry from the attempt about to refuse on it (#19334)', () => {
|
||||
const strict = getRuntimeWorktreeRemovalOptionsKey({ runHooks: true })
|
||||
expect(
|
||||
getRuntimeWorktreeRemovalOptionsKey({ runHooks: true, allowFailedArchiveHook: true })
|
||||
).not.toBe(strict)
|
||||
})
|
||||
|
||||
it('keeps every waiver on its own axis, so none of them coalesce', () => {
|
||||
const keys = [
|
||||
{},
|
||||
{ force: true },
|
||||
{ runHooks: true },
|
||||
{ allowUnverifiedPtyStop: true },
|
||||
{ allowFailedArchiveHook: true }
|
||||
].map(getRuntimeWorktreeRemovalOptionsKey)
|
||||
expect(new Set(keys).size).toBe(keys.length)
|
||||
})
|
||||
|
||||
it('treats an omitted option as its off value', () => {
|
||||
expect(getRuntimeWorktreeRemovalOptionsKey({})).toBe(
|
||||
getRuntimeWorktreeRemovalOptionsKey({
|
||||
force: false,
|
||||
runHooks: false,
|
||||
allowUnverifiedPtyStop: false,
|
||||
allowFailedArchiveHook: false
|
||||
})
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('runtimeRepoMatchesExecutionHost', () => {
|
||||
it('matches an unstamped SSH repo against its own host (#11163)', () => {
|
||||
|
||||
@@ -26,15 +26,35 @@ export function gitStatusErrorMeansNotRepository(error: unknown): boolean {
|
||||
return /not a git repository/i.test(`${message}\n${stderr}`)
|
||||
}
|
||||
|
||||
/**
|
||||
* Options for `removeManagedWorktree`. Named rather than positional on purpose: three of the
|
||||
* four are interchangeable booleans that each waive a different safety check on a destructive
|
||||
* delete, so a transposition would silently delete a checkout the caller meant to protect.
|
||||
*/
|
||||
export type RemoveManagedWorktreeOptions = {
|
||||
force?: boolean
|
||||
runHooks?: boolean
|
||||
/** Waives proof that every PTY stopped (#11960). Set by explicit Force Delete only. */
|
||||
allowUnverifiedPtyStop?: boolean
|
||||
/** Waives a FAILED archive hook (#19334). Never implied by `force`, never by `runHooks`. */
|
||||
allowFailedArchiveHook?: boolean
|
||||
hostId?: string
|
||||
}
|
||||
|
||||
export function getRuntimeWorktreeRemovalOptionsKey(
|
||||
force: boolean,
|
||||
runHooks: boolean,
|
||||
allowUnverifiedPtyStop: boolean
|
||||
options: Pick<
|
||||
RemoveManagedWorktreeOptions,
|
||||
'force' | 'runHooks' | 'allowUnverifiedPtyStop' | 'allowFailedArchiveHook'
|
||||
>
|
||||
): string {
|
||||
// Why: a forced retry must not coalesce onto the in-flight attempt that just
|
||||
// failed the PTY gate — it would inherit that failure instead of retrying.
|
||||
const ptyKey = allowUnverifiedPtyStop ? 'allow-unverified-pty' : 'require-pty-stop'
|
||||
return `${force ? 'force' : 'normal'}:${runHooks ? 'run-hooks' : 'skip-hooks'}:${ptyKey}`
|
||||
const ptyKey = options.allowUnverifiedPtyStop ? 'allow-unverified-pty' : 'require-pty-stop'
|
||||
// Same reason for the archive waiver: a retry that waives the failed hook must not coalesce
|
||||
// onto the in-flight attempt that is about to refuse on it.
|
||||
const archiveKey = options.allowFailedArchiveHook ? 'allow-failed-archive' : 'require-archive'
|
||||
const hooksKey = options.runHooks ? 'run-hooks' : 'skip-hooks'
|
||||
return `${options.force ? 'force' : 'normal'}:${hooksKey}:${ptyKey}:${archiveKey}`
|
||||
}
|
||||
|
||||
// Null executionHostId means host-unaware: path-only callers match any repo, and the first runtime
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { Repo } from '../shared/repo-types'
|
||||
import { gateRemovalWhereArchiveHookCannotRun } from './worktree-archive-hook-gate'
|
||||
import {
|
||||
ARCHIVE_HOOK_FAILED_REMOVAL_CODE,
|
||||
asArchiveHookRefusal
|
||||
} from '../shared/worktree/archive-hook-removal-gate'
|
||||
|
||||
// Mocked at the SSH-aware reader, because that is the whole point: on an SSH worktree the hook
|
||||
// lives on the execution host, not on the runtime's local disk.
|
||||
const { getArchiveHooksForRemovalMock } = vi.hoisted(() => ({
|
||||
getArchiveHooksForRemovalMock: vi.fn()
|
||||
}))
|
||||
vi.mock('./ipc/worktrees/removal/worktree-archive-hook', () => ({
|
||||
getArchiveHooksForRemoval: getArchiveHooksForRemovalMock
|
||||
}))
|
||||
|
||||
const REPO: Repo = { id: 'r', path: '/repo', displayName: 'r', badgeColor: '#000', addedAt: 0 }
|
||||
|
||||
const withArchiveHook = (present: boolean): void => {
|
||||
getArchiveHooksForRemovalMock.mockResolvedValue(
|
||||
present ? { scripts: { archive: 'archive.sh' } } : null
|
||||
)
|
||||
}
|
||||
|
||||
const gate = (over: Partial<Parameters<typeof gateRemovalWhereArchiveHookCannotRun>[0]> = {}) =>
|
||||
gateRemovalWhereArchiveHookCannotRun({
|
||||
repo: REPO,
|
||||
connectionId: undefined,
|
||||
worktreePath: '/w/f',
|
||||
runHooks: true,
|
||||
allowFailedArchiveHook: false,
|
||||
...over
|
||||
})
|
||||
|
||||
// Why (#19334 / S1): the runtime's SSH path runs no archive hook. Silently deleting there would
|
||||
// reproduce the reported bug in the one place `worktree.archive-failure-blocking.v1` promises it
|
||||
// cannot happen, so the capability would be advertising a guarantee it does not keep.
|
||||
describe('gateRemovalWhereArchiveHookCannotRun', () => {
|
||||
it('lets a repo with no archive hook through untouched', async () => {
|
||||
withArchiveHook(false)
|
||||
await expect(gate()).resolves.toEqual({})
|
||||
})
|
||||
|
||||
it('warns rather than refuses when hooks were not requested', async () => {
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
withArchiveHook(true)
|
||||
await expect(gate({ runHooks: false })).resolves.toMatchObject({
|
||||
warning: expect.stringContaining('pass --run-hooks to run it')
|
||||
})
|
||||
})
|
||||
|
||||
// Why (#19334): reading locally would miss the committed hook on an SSH host entirely.
|
||||
it('asks the execution host whether a hook exists, not the local disk', async () => {
|
||||
withArchiveHook(false)
|
||||
await gate({ connectionId: 'ssh-target' })
|
||||
expect(getArchiveHooksForRemovalMock).toHaveBeenCalledWith(REPO, 'ssh-target')
|
||||
})
|
||||
|
||||
it('refuses a hooks-requested removal it cannot honour, as unverifiable', async () => {
|
||||
withArchiveHook(true)
|
||||
const refusal = asArchiveHookRefusal(await gate().catch((error: unknown) => error))
|
||||
|
||||
expect(refusal.code).toBe(ARCHIVE_HOOK_FAILED_REMOVAL_CODE)
|
||||
// Never `exited`: nothing ran, so nothing reported an exit to read.
|
||||
expect(refusal.data).toMatchObject({ worktreePath: '/w/f', outcome: 'unverifiable' })
|
||||
expect(refusal.data.exitCode).toBeUndefined()
|
||||
})
|
||||
|
||||
// Why this matters: without it the refusal is a dead loop. The desktop's "Delete Anyway" and the
|
||||
// CLI's --allow-failed-archive-hook both land here, and a block with no reachable exit on the
|
||||
// surface where it happens is the failure mode this PR fixed on the desktop path.
|
||||
it('deletes anyway when the refusal is explicitly waived, and records it', async () => {
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
withArchiveHook(true)
|
||||
|
||||
const result = await gate({ allowFailedArchiveHook: true })
|
||||
|
||||
expect(result.warning).toBeUndefined()
|
||||
expect(result.override).toMatchObject({
|
||||
worktreePath: '/w/f',
|
||||
outcome: 'unverifiable',
|
||||
overridden: true
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,86 @@
|
||||
import type { Repo } from '../shared/repo-types'
|
||||
import { getArchiveHooksForRemoval } from './ipc/worktrees/removal/worktree-archive-hook'
|
||||
import {
|
||||
WorktreeArchiveHookFailedError,
|
||||
formatArchiveHookOverride,
|
||||
type ArchiveHookFailure,
|
||||
classifyArchiveHookFailure,
|
||||
formatArchiveHookFailure,
|
||||
type ArchiveHookOverride,
|
||||
type ArchiveHookRunResult
|
||||
} from '../shared/worktree/archive-hook-removal-gate'
|
||||
|
||||
/**
|
||||
* The archive-hook precondition for a destructive worktree removal (#19334). Call it while the
|
||||
* checkout, its registration, its agents and its ownership evidence are all still intact: on a
|
||||
* failure it throws, and no caller may stop a PTY, deregister, or delete before it has returned.
|
||||
*
|
||||
* Returns the override record when the failure was explicitly waived, `undefined` on success.
|
||||
*/
|
||||
export function gateWorktreeRemovalOnArchiveHook(args: {
|
||||
worktreePath: string
|
||||
result: ArchiveHookRunResult
|
||||
allowFailure: boolean
|
||||
}): ArchiveHookOverride | undefined {
|
||||
if (args.result.success) {
|
||||
return undefined
|
||||
}
|
||||
const failure = classifyArchiveHookFailure(args.worktreePath, args.result)
|
||||
if (!args.allowFailure) {
|
||||
console.error(`[hooks] ${formatArchiveHookFailure(failure)}`)
|
||||
throw new WorktreeArchiveHookFailedError(failure)
|
||||
}
|
||||
console.warn(
|
||||
`[hooks] archive hook failure overridden for ${args.worktreePath}; deleting anyway:`,
|
||||
args.result.output
|
||||
)
|
||||
return { ...failure, overridden: true }
|
||||
}
|
||||
|
||||
/**
|
||||
* The runtime's SSH removal path cannot run an archive hook at all (see #18563, which adds it).
|
||||
* Until it can, a removal that asked for hooks has to refuse rather than delete: deleting would
|
||||
* repeat exactly the bug this gate exists to stop, and reporting success would make
|
||||
* `worktree.archive-failure-blocking.v1` a lie in the one case the reporter asked it to cover.
|
||||
*
|
||||
* Modelled as `unverifiable` because that is what it is — the hook's outcome was never observed —
|
||||
* so it reuses the same typed error, the same `--allow-failed-archive-hook` waiver, and the same
|
||||
* desktop "Delete Anyway" affordance as any other unobserved hook. Waiving it records the same
|
||||
* `archiveHookOverride` the other paths return, so a caller is told what it accepted.
|
||||
*
|
||||
* Returns the skipped-hook warning when hooks were not requested, matching the local path.
|
||||
*
|
||||
* Hooks are read through `getArchiveHooksForRemoval` rather than `getEffectiveHooks`: on an
|
||||
* SSH-hosted worktree `repo.path` names a path on the EXECUTION host, so a local read would miss
|
||||
* the committed `orca.yaml` this gate exists for, and could refuse on a coincidental local one.
|
||||
*/
|
||||
export async function gateRemovalWhereArchiveHookCannotRun(args: {
|
||||
repo: Repo
|
||||
/** The removal route's owner; `repo.connectionId` is null for an `ssh:`-only row. */
|
||||
connectionId: string | undefined
|
||||
worktreePath: string
|
||||
runHooks: boolean
|
||||
/** Explicit waiver. Without it the refusal below has no exit on this path. */
|
||||
allowFailedArchiveHook: boolean
|
||||
}): Promise<{ warning?: string; override?: ArchiveHookOverride }> {
|
||||
const hooks = await getArchiveHooksForRemoval(args.repo, args.connectionId)
|
||||
if (!hooks?.scripts.archive) {
|
||||
return {}
|
||||
}
|
||||
if (!args.runHooks) {
|
||||
const warning = `orca.yaml archive hook skipped for ${args.worktreePath}; pass --run-hooks to run it.`
|
||||
console.warn(`[hooks] ${warning}`)
|
||||
return { warning }
|
||||
}
|
||||
const failure: ArchiveHookFailure = {
|
||||
worktreePath: args.worktreePath,
|
||||
outcome: 'unverifiable',
|
||||
output:
|
||||
'This host cannot run an archive hook for an SSH-hosted worktree, so the hook never ran. Remove it from the desktop app, which does run it, or delete anyway to accept that nothing was archived.'
|
||||
}
|
||||
if (!args.allowFailedArchiveHook) {
|
||||
throw new WorktreeArchiveHookFailedError(failure)
|
||||
}
|
||||
console.warn(`[hooks] ${formatArchiveHookOverride({ ...failure, overridden: true })}`)
|
||||
return { override: { ...failure, overridden: true } }
|
||||
}
|
||||
@@ -98,6 +98,9 @@ export type WorktreeApi = {
|
||||
// may waive the proof that every PTY stopped.
|
||||
allowUnverifiedPtyStop?: boolean
|
||||
skipArchive?: boolean
|
||||
// Why (#19334): distinct from `skipArchive` (never runs the hook) and never implied by
|
||||
// `force` — this waives a hook that ran and FAILED.
|
||||
allowFailedArchiveHook?: boolean
|
||||
snapshotPruneBatchId?: string
|
||||
}) => Promise<RemoveWorktreeResult>
|
||||
// Forget a workspace from Orca only (no remote Git/FS work) — for workspaces pinned to a removed/disconnected SSH host.
|
||||
|
||||
@@ -102,6 +102,13 @@ function showDeleteFailureToast(): void {
|
||||
),
|
||||
canForceDelete: true,
|
||||
forceDeleteReason: 'dirty',
|
||||
onDeleteAnyway: () =>
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.settings.DevToolsPane.deleteAnywayClicked',
|
||||
'Delete Anyway clicked'
|
||||
)
|
||||
),
|
||||
onViewChanges: () =>
|
||||
toast.message(
|
||||
translate(
|
||||
|
||||
@@ -51,6 +51,7 @@ describe('showDeleteWorktreeFailureToast', () => {
|
||||
it('uses a persistent in-body action footer when force delete is available', () => {
|
||||
const onViewChanges = vi.fn()
|
||||
const onForceDelete = vi.fn()
|
||||
const onDeleteAnyway = vi.fn()
|
||||
|
||||
showDeleteWorktreeFailureToast({
|
||||
error: 'branch has changes',
|
||||
@@ -58,6 +59,7 @@ describe('showDeleteWorktreeFailureToast', () => {
|
||||
forceDeleteReason: 'dirty',
|
||||
onViewChanges,
|
||||
onForceDelete,
|
||||
onDeleteAnyway,
|
||||
worktreeId: 'wt-1',
|
||||
worktreeName: 'feature/foo'
|
||||
})
|
||||
@@ -97,6 +99,7 @@ describe('showDeleteWorktreeFailureToast', () => {
|
||||
forceDeleteReason: null,
|
||||
onViewChanges,
|
||||
onForceDelete: vi.fn(),
|
||||
onDeleteAnyway: vi.fn(),
|
||||
worktreeId: 'wt-2',
|
||||
worktreeName: 'feature/bar'
|
||||
})
|
||||
@@ -119,6 +122,53 @@ describe('showDeleteWorktreeFailureToast', () => {
|
||||
expect(onViewChanges).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// #19334: the archive-hook refusal is the one failure a user clears by waiving rather than by
|
||||
// fixing state, and the desktop is where most people meet it.
|
||||
it('offers Delete Anyway when the archive hook refused the removal', () => {
|
||||
const onDeleteAnyway = vi.fn()
|
||||
|
||||
showDeleteWorktreeFailureToast({
|
||||
error: 'Archive hook failed for worktree: /w/feature — exited 23.',
|
||||
canForceDelete: false,
|
||||
forceDeleteReason: null,
|
||||
canWaiveArchiveHook: true,
|
||||
onViewChanges: vi.fn(),
|
||||
onForceDelete: vi.fn(),
|
||||
onDeleteAnyway,
|
||||
worktreeId: 'wt-archive',
|
||||
worktreeName: 'feature/archive'
|
||||
})
|
||||
|
||||
expect(toast.error).toHaveBeenCalledWith(
|
||||
'Failed to delete workspace feature/archive',
|
||||
// The user has to read the reason before choosing, so this toast must not expire.
|
||||
expect.objectContaining({ duration: Infinity })
|
||||
)
|
||||
|
||||
const body = renderToastBody('error')
|
||||
expect(body.textContent).toContain('Delete Anyway')
|
||||
expect(body.textContent).not.toContain('Force Delete')
|
||||
|
||||
clickButton(body, 'Delete Anyway')
|
||||
expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-archive')
|
||||
expect(onDeleteAnyway).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not offer Delete Anyway for an ordinary failure', () => {
|
||||
showDeleteWorktreeFailureToast({
|
||||
error: 'permission denied',
|
||||
canForceDelete: false,
|
||||
forceDeleteReason: null,
|
||||
onViewChanges: vi.fn(),
|
||||
onForceDelete: vi.fn(),
|
||||
onDeleteAnyway: vi.fn(),
|
||||
worktreeId: 'wt-plain',
|
||||
worktreeName: 'feature/plain'
|
||||
})
|
||||
|
||||
expect(renderToastBody('error').textContent).not.toContain('Delete Anyway')
|
||||
})
|
||||
|
||||
it('offers neither force delete nor View for a locked workspace', () => {
|
||||
const onViewChanges = vi.fn()
|
||||
|
||||
@@ -128,6 +178,7 @@ describe('showDeleteWorktreeFailureToast', () => {
|
||||
forceDeleteReason: null,
|
||||
onViewChanges,
|
||||
onForceDelete: vi.fn(),
|
||||
onDeleteAnyway: vi.fn(),
|
||||
worktreeId: 'wt-locked',
|
||||
worktreeName: 'feature/locked'
|
||||
})
|
||||
@@ -151,6 +202,7 @@ describe('showDeleteWorktreeFailureToast', () => {
|
||||
hasKnownChanges: true,
|
||||
onViewChanges: vi.fn(),
|
||||
onForceDelete: vi.fn(),
|
||||
onDeleteAnyway: vi.fn(),
|
||||
worktreeId: 'wt-locked-dirty',
|
||||
worktreeName: 'feature/locked-dirty'
|
||||
})
|
||||
|
||||
@@ -13,8 +13,11 @@ type DeleteWorktreeFailureToastOptions = {
|
||||
forceDeleteReason: WorktreeForceDeleteReason | null
|
||||
lockReason?: string | null
|
||||
hasKnownChanges?: boolean
|
||||
/** The archive hook refused this removal, so the user may waive it (#19334). */
|
||||
canWaiveArchiveHook?: boolean
|
||||
onViewChanges: () => void
|
||||
onForceDelete: () => void
|
||||
onDeleteAnyway: () => void
|
||||
worktreeId: string
|
||||
worktreeName: string
|
||||
}
|
||||
@@ -26,16 +29,20 @@ function deleteWorktreeFailureToastId(worktreeId: string): string {
|
||||
function DeleteWorktreeFailureToastBody({
|
||||
description,
|
||||
canForceDelete,
|
||||
canWaiveArchiveHook,
|
||||
showViewChanges,
|
||||
onViewChanges,
|
||||
onForceDelete,
|
||||
onDeleteAnyway,
|
||||
toastId
|
||||
}: {
|
||||
description?: string
|
||||
canForceDelete: boolean
|
||||
canWaiveArchiveHook: boolean
|
||||
showViewChanges: boolean
|
||||
onViewChanges: () => void
|
||||
onForceDelete: () => void
|
||||
onDeleteAnyway: () => void
|
||||
toastId: string
|
||||
}): React.JSX.Element {
|
||||
const viewChanges = (): void => {
|
||||
@@ -46,6 +53,10 @@ function DeleteWorktreeFailureToastBody({
|
||||
toast.dismiss(toastId)
|
||||
onForceDelete()
|
||||
}
|
||||
const deleteAnyway = (): void => {
|
||||
toast.dismiss(toastId)
|
||||
onDeleteAnyway()
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="flex w-full flex-col gap-3">
|
||||
@@ -63,6 +74,14 @@ function DeleteWorktreeFailureToastBody({
|
||||
{translate('auto.components.sidebar.delete.worktree.flow.2b20ce87b3', 'Force Delete')}
|
||||
</Button>
|
||||
) : null}
|
||||
{canWaiveArchiveHook ? (
|
||||
<Button type="button" variant="destructive" size="sm" onClick={deleteAnyway}>
|
||||
{translate(
|
||||
'auto.components.sidebar.delete.worktree.failure.archive.waiver',
|
||||
'Delete Anyway'
|
||||
)}
|
||||
</Button>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
@@ -74,8 +93,10 @@ export function showDeleteWorktreeFailureToast({
|
||||
forceDeleteReason,
|
||||
lockReason,
|
||||
hasKnownChanges,
|
||||
canWaiveArchiveHook,
|
||||
onViewChanges,
|
||||
onForceDelete,
|
||||
onDeleteAnyway,
|
||||
worktreeId,
|
||||
worktreeName
|
||||
}: DeleteWorktreeFailureToastOptions): void {
|
||||
@@ -96,13 +117,16 @@ export function showDeleteWorktreeFailureToast({
|
||||
<DeleteWorktreeFailureToastBody
|
||||
description={toastCopy.description}
|
||||
canForceDelete={canForceDelete}
|
||||
canWaiveArchiveHook={canWaiveArchiveHook === true}
|
||||
showViewChanges={!isLockedWorktreeRemovalError(error) || hasKnownChanges === true}
|
||||
onViewChanges={onViewChanges}
|
||||
onForceDelete={onForceDelete}
|
||||
onDeleteAnyway={onDeleteAnyway}
|
||||
toastId={id}
|
||||
/>
|
||||
),
|
||||
duration: canForceDelete ? Infinity : 10000,
|
||||
// A toast offering a destructive choice must not expire before the user reads the reason.
|
||||
duration: canForceDelete || canWaiveArchiveHook === true ? Infinity : 10000,
|
||||
dismissible: true
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1,7 +1,20 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { ExecutionHostId } from '../../../../shared/execution-host'
|
||||
|
||||
type MockWorktreeDeleteState = {
|
||||
isDeleting?: boolean
|
||||
error?: string | null
|
||||
canForceDelete?: boolean
|
||||
forceDeleteReason?: 'dirty' | null
|
||||
lockReason?: string | null
|
||||
canWaiveArchiveHook?: boolean
|
||||
executionHostId?: ExecutionHostId | null
|
||||
}
|
||||
|
||||
const mocks = vi.hoisted(() => {
|
||||
// Declared up here so the empty initialisers can be typed rather than asserted.
|
||||
const gitStatusByWorktree: Record<string, unknown[]> = {}
|
||||
const deleteStateByWorktreeId: Record<string, MockWorktreeDeleteState> = {}
|
||||
const state = {
|
||||
settings: { skipDeleteWorktreeConfirm: false },
|
||||
worktreeMap: new Map<
|
||||
@@ -35,18 +48,8 @@ const mocks = vi.hoisted(() => {
|
||||
setRightSidebarTab: vi.fn(),
|
||||
setRightSidebarOpen: vi.fn(),
|
||||
removeWorktree: vi.fn().mockResolvedValue({ ok: true }),
|
||||
gitStatusByWorktree: {} as Record<string, unknown[]>,
|
||||
deleteStateByWorktreeId: {} as Record<
|
||||
string,
|
||||
{
|
||||
isDeleting?: boolean
|
||||
error?: string | null
|
||||
canForceDelete?: boolean
|
||||
forceDeleteReason?: 'dirty' | null
|
||||
lockReason?: string | null
|
||||
executionHostId?: ExecutionHostId | null
|
||||
}
|
||||
>
|
||||
gitStatusByWorktree,
|
||||
deleteStateByWorktreeId
|
||||
}
|
||||
return { state }
|
||||
})
|
||||
@@ -631,4 +634,42 @@ describe('delete worktree flow', () => {
|
||||
description: 'Refresh Space and try again if the workspace list looks stale.'
|
||||
})
|
||||
})
|
||||
|
||||
// #19334: a waived delete is still a delete — the caller's bookkeeping has to hear about it, or a
|
||||
// batch/Space-panel list keeps showing the workspace it just removed.
|
||||
it('reports a Delete Anyway success to the caller like a force retry', async () => {
|
||||
mocks.state.settings = { skipDeleteWorktreeConfirm: true }
|
||||
mocks.state.removeWorktree
|
||||
.mockImplementationOnce(async () => {
|
||||
mocks.state.deleteStateByWorktreeId['wt-1'] = {
|
||||
isDeleting: false,
|
||||
error: 'Archive hook failed for worktree: /w/one — exited 23.',
|
||||
canForceDelete: false,
|
||||
forceDeleteReason: null,
|
||||
canWaiveArchiveHook: true
|
||||
}
|
||||
return { ok: false, error: 'Archive hook failed for worktree: /w/one — exited 23.' }
|
||||
})
|
||||
.mockResolvedValueOnce({ ok: true })
|
||||
setWorktrees([{ id: 'wt-1', displayName: 'one' }])
|
||||
const onDeleted = vi.fn()
|
||||
|
||||
expect(runWorktreeBatchDelete(['wt-1'], { onDeleted })).toBe(true)
|
||||
|
||||
await vi.waitFor(() => expect(showDeleteWorktreeFailureToast).toHaveBeenCalled())
|
||||
const toastOptions = vi.mocked(showDeleteWorktreeFailureToast).mock.calls[0]?.[0]
|
||||
expect(toastOptions?.canWaiveArchiveHook).toBe(true)
|
||||
toastOptions?.onDeleteAnyway()
|
||||
|
||||
await vi.waitFor(() => {
|
||||
// The waiver rides its own option; force stays whatever the original attempt used.
|
||||
expect(mocks.state.removeWorktree).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
{ id: 'wt-1', executionHostId: null },
|
||||
false,
|
||||
{ allowFailedArchiveHook: true }
|
||||
)
|
||||
expect(onDeleted).toHaveBeenCalledWith([{ id: 'wt-1', executionHostId: null }])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -38,6 +38,101 @@ export function runWorktreeDeleteWithToast(
|
||||
...(options.suppressPreservedBranchToast ? { suppressPreservedBranchToast: true } : {}),
|
||||
...(options.snapshotPruneBatchId ? { snapshotPruneBatchId: options.snapshotPruneBatchId } : {})
|
||||
}
|
||||
const showFailureToast = (
|
||||
error: string,
|
||||
state: ReturnType<typeof getDeleteStateForWorktreeHost>
|
||||
): void => {
|
||||
const hasKnownChanges =
|
||||
(useAppStore.getState().gitStatusByWorktree[worktreeId]?.length ?? 0) > 0
|
||||
showDeleteWorktreeFailureToast({
|
||||
error,
|
||||
canForceDelete: state?.canForceDelete ?? false,
|
||||
canWaiveArchiveHook: state?.canWaiveArchiveHook === true,
|
||||
forceDeleteReason: state?.forceDeleteReason ?? null,
|
||||
lockReason: state?.lockReason ?? null,
|
||||
hasKnownChanges,
|
||||
onViewChanges: () => viewWorktreeDiff(worktreeId, target.executionHostId),
|
||||
// Why (#19334): re-runs the archive hook and waives the failure this time, so the waiver
|
||||
// is an informed choice made after reading the refusal -- not something `force` implied.
|
||||
onDeleteAnyway: () =>
|
||||
retryFromToast({ force: options.force === true, allowFailedArchiveHook: true }),
|
||||
// The explicit Force Delete retry may waive an unverified PTY-stop proof.
|
||||
onForceDelete: () =>
|
||||
retryFromToast({
|
||||
force: true,
|
||||
allowUnverifiedPtyStop: true,
|
||||
failedTitle: translate(
|
||||
'auto.components.sidebar.delete.worktree.flow.4f3876c0f5',
|
||||
'Force delete failed'
|
||||
),
|
||||
withViewAction: true
|
||||
}),
|
||||
worktreeId,
|
||||
worktreeName
|
||||
})
|
||||
}
|
||||
|
||||
// Both toast buttons do the same thing: recapture focus (the user may have navigated while the
|
||||
// toast was open), retry with one waiver added, and report a success through `onForceDeleted` so
|
||||
// the caller's bookkeeping runs. Only the waiver and the failure copy differ.
|
||||
const retryFromToast = (retry: {
|
||||
force: boolean
|
||||
allowUnverifiedPtyStop?: boolean
|
||||
allowFailedArchiveHook?: boolean
|
||||
failedTitle?: string
|
||||
withViewAction?: boolean
|
||||
}): void => {
|
||||
const commitRetryFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId)
|
||||
const viewAction = retry.withViewAction
|
||||
? {
|
||||
action: {
|
||||
label: translate('auto.components.sidebar.delete.worktree.flow.7488ed8711', 'View'),
|
||||
onClick: () => viewWorktreeDiff(worktreeId, target.executionHostId)
|
||||
}
|
||||
}
|
||||
: {}
|
||||
// Why re-show the full failure toast rather than a bare `toast.error` (#19334): a retry can
|
||||
// fail for a DIFFERENT reason than the one the user just answered. Waiving a failed archive
|
||||
// hook on a dirty checkout lands on the dirty preflight next, and a bare error offers no
|
||||
// buttons — leaving the user stuck one step further in, which is the dead end this gate has
|
||||
// now produced three times. Routing back through the same toast keeps every retry actionable.
|
||||
const failed = (description: string): void => {
|
||||
const retryState = getDeleteStateForWorktreeHost(
|
||||
{ id: worktreeId, hostId: target.executionHostId ?? undefined },
|
||||
useAppStore.getState().deleteStateByWorktreeId
|
||||
)
|
||||
if (retryState?.canForceDelete === true || retryState?.canWaiveArchiveHook === true) {
|
||||
showFailureToast(description, retryState)
|
||||
return
|
||||
}
|
||||
toast.error(
|
||||
retry.failedTitle ??
|
||||
translate(
|
||||
'auto.components.sidebar.delete.worktree.flow.ae57cbf6e4',
|
||||
'Failed to delete workspace'
|
||||
),
|
||||
{ description, ...viewAction }
|
||||
)
|
||||
}
|
||||
useAppStore
|
||||
.getState()
|
||||
.removeWorktree(target, retry.force, {
|
||||
...(retry.allowUnverifiedPtyStop ? { allowUnverifiedPtyStop: true } : {}),
|
||||
...(retry.allowFailedArchiveHook ? { allowFailedArchiveHook: true } : {})
|
||||
})
|
||||
.then((result) => {
|
||||
if (!result.ok) {
|
||||
failed(result.error)
|
||||
return
|
||||
}
|
||||
commitRetryFocus()
|
||||
// "A retry started from this toast completed the delete" — callers hang their bookkeeping
|
||||
// off it, so without this a batch or Space-panel delete keeps listing what it removed.
|
||||
options.onForceDeleted?.(target)
|
||||
})
|
||||
.catch((err: unknown) => failed(err instanceof Error ? err.message : String(err)))
|
||||
}
|
||||
|
||||
const removal =
|
||||
Object.keys(removeOptions).length > 0
|
||||
? removeWorktree(target, options.force === true, removeOptions)
|
||||
@@ -61,73 +156,13 @@ export function runWorktreeDeleteWithToast(
|
||||
}
|
||||
return true
|
||||
}
|
||||
const state = getDeleteStateForWorktreeHost(
|
||||
{ id: worktreeId, hostId: target.executionHostId ?? undefined },
|
||||
useAppStore.getState().deleteStateByWorktreeId
|
||||
showFailureToast(
|
||||
result.error,
|
||||
getDeleteStateForWorktreeHost(
|
||||
{ id: worktreeId, hostId: target.executionHostId ?? undefined },
|
||||
useAppStore.getState().deleteStateByWorktreeId
|
||||
)
|
||||
)
|
||||
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, target.executionHostId),
|
||||
onForceDelete: () => {
|
||||
// Recapture focus because the user may have navigated while the toast was open.
|
||||
const commitForceFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId)
|
||||
// The explicit Force Delete retry may waive an unverified PTY-stop proof.
|
||||
const forceRemoval = useAppStore
|
||||
.getState()
|
||||
.removeWorktree(target, true, { allowUnverifiedPtyStop: true })
|
||||
forceRemoval
|
||||
.then((forceResult) => {
|
||||
if (!forceResult.ok) {
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.sidebar.delete.worktree.flow.4f3876c0f5',
|
||||
'Force delete failed'
|
||||
),
|
||||
{
|
||||
description: forceResult.error,
|
||||
action: {
|
||||
label: translate(
|
||||
'auto.components.sidebar.delete.worktree.flow.7488ed8711',
|
||||
'View'
|
||||
),
|
||||
onClick: () => viewWorktreeDiff(worktreeId, target.executionHostId)
|
||||
}
|
||||
}
|
||||
)
|
||||
return
|
||||
}
|
||||
commitForceFocus()
|
||||
options.onForceDeleted?.(target)
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.sidebar.delete.worktree.flow.ae57cbf6e4',
|
||||
'Failed to delete workspace'
|
||||
),
|
||||
{
|
||||
description: err instanceof Error ? err.message : String(err),
|
||||
action: {
|
||||
label: translate(
|
||||
'auto.components.sidebar.delete.worktree.flow.7488ed8711',
|
||||
'View'
|
||||
),
|
||||
onClick: () => viewWorktreeDiff(worktreeId, target.executionHostId)
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
},
|
||||
worktreeId,
|
||||
worktreeName
|
||||
})
|
||||
return false
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
|
||||
@@ -5802,6 +5802,11 @@
|
||||
"unstoppedPtyLive": "This workspace still has running terminals, so Orca stopped before deleting any files. Force Delete will kill them and discard any uncommitted work they hold.",
|
||||
"runningAgentSession": "Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway.",
|
||||
"runningAgentSessionLive": "This workspace still has running agent sessions that Orca could not close, so it stopped before deleting any files. Force Delete will discard any work they hold."
|
||||
},
|
||||
"failure": {
|
||||
"archive": {
|
||||
"waiver": "Delete Anyway"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
@@ -10953,7 +10958,8 @@
|
||||
"orcaCloudSignOut": "Sign out",
|
||||
"orcaCloudConnect": "Connect profile",
|
||||
"orcaCloudRefresh": "Refresh status",
|
||||
"orcaCloudNotConfigured": "Set ORCA_CLOUD_API_URL and ORCA_CLOUD_CLIENT_ID to preview Orca Cloud sign-in in this dev build."
|
||||
"orcaCloudNotConfigured": "Set ORCA_CLOUD_API_URL and ORCA_CLOUD_CLIENT_ID to preview Orca Cloud sign-in in this dev build.",
|
||||
"deleteAnywayClicked": "Delete Anyway clicked"
|
||||
},
|
||||
"EphemeralVmRecipeRow": {
|
||||
"useInWorkspace": "Use in workspace"
|
||||
|
||||
@@ -131,3 +131,38 @@ describe('readIpcErrorDetail', () => {
|
||||
).toBe('SSH connection failed: Relay package not found.')
|
||||
})
|
||||
})
|
||||
|
||||
// Why (#19334): a typed main-process error keeps its class name after the wrapper comes off, and
|
||||
// the worktree-removal refusal is rendered to a user who does not care what the class was called.
|
||||
describe('typed main-process errors', () => {
|
||||
const wrapped = new Error(
|
||||
"Error invoking remote method 'worktrees:remove': WorktreeArchiveHookFailedError: " +
|
||||
'Archive hook failed for worktree: /w/feature — exited 23.\nbackup target unreachable'
|
||||
)
|
||||
|
||||
it('drops the error class name along with the wrapper', () => {
|
||||
expect(readIpcErrorDetail(wrapped)).toBe(
|
||||
'Archive hook failed for worktree: /w/feature — exited 23.\nbackup target unreachable'
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps the detail lines a refusal needs, unlike the clamped read', () => {
|
||||
// The hook's own output is the actionable part of an archive refusal, so the unclamped read
|
||||
// has to survive the newline that `readIpcErrorMessage` deliberately cuts at.
|
||||
expect(readIpcErrorMessage(wrapped)).toBe(
|
||||
'Archive hook failed for worktree: /w/feature — exited 23.'
|
||||
)
|
||||
})
|
||||
|
||||
it('leaves a renderer-local error its class name, having unwrapped nothing', () => {
|
||||
expect(readIpcErrorDetail(new Error('TypeError: x is not a function'))).toBe(
|
||||
'TypeError: x is not a function'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not mistake an errno prefix for a class name', () => {
|
||||
expect(
|
||||
readIpcErrorDetail(new Error("Error occurred in handler for 'fs:read': EACCES: denied"))
|
||||
).toBe('EACCES: denied')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,10 +1,17 @@
|
||||
// Unanchored so caller-owned context around an Electron wrapper survives.
|
||||
const IPC_INVOKE_PREFIX = /Error invoking remote method '[^']*':\s*(?:Error:\s*)?/
|
||||
const IPC_HANDLER_PREFIX = /Error occurred in handler for '[^']*':\s*(?:Error:\s*)?/
|
||||
// Why (#19334): once the wrapper is gone, a typed main-process error still leads with its class
|
||||
// name — "WorktreeArchiveHookFailedError: Archive hook failed for worktree: …". That is noise to
|
||||
// someone reading a toast, and it pushes the sentence that matters off the first line.
|
||||
const ERROR_CLASS_PREFIX = /^(?:[A-Za-z_$][\w$]*)?Error:\s*/
|
||||
|
||||
function unwrapIpcErrorMessage(message: string): string | undefined {
|
||||
const wrapped = IPC_INVOKE_PREFIX.test(message) || IPC_HANDLER_PREFIX.test(message)
|
||||
const detail = message.replace(IPC_INVOKE_PREFIX, '').replace(IPC_HANDLER_PREFIX, '').trim()
|
||||
return detail || undefined
|
||||
// Only strip the class name off something we actually unwrapped, so a renderer-local
|
||||
// `TypeError: …` keeps the prefix that tells you what it was.
|
||||
return (wrapped ? detail.replace(ERROR_CLASS_PREFIX, '').trim() : detail) || undefined
|
||||
}
|
||||
|
||||
export function compactIpcErrorMessage(message: string): string | undefined {
|
||||
|
||||
@@ -377,18 +377,34 @@ describe('removeWorktree cascade', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('offers force delete for Electron-wrapped local dirty preflight errors', async () => {
|
||||
// Why a table (#19334): these three differ only in the wrapped message and the classification it
|
||||
// earns. The shared body is what matters — the IPC wrapper is stripped for display while
|
||||
// classification still reads the wrapped input.
|
||||
it.each([
|
||||
[
|
||||
'offers force delete for Electron-wrapped local dirty preflight errors',
|
||||
"Error invoking remote method 'worktrees:remove': Error: Failed to delete worktree at /workspace/feature-wt. ?? scratch.txt",
|
||||
'Failed to delete worktree at /workspace/feature-wt. ?? scratch.txt',
|
||||
{ canForceDelete: true, forceDeleteReason: 'dirty' }
|
||||
],
|
||||
[
|
||||
'offers force delete when Git already removed an unregistered worktree',
|
||||
"Error invoking remote method 'worktrees:remove': Error: Worktree is no longer registered with Git and its directory is already gone.",
|
||||
'Worktree is no longer registered with Git and its directory is already gone.',
|
||||
{ canForceDelete: true, forceDeleteReason: 'missing-registration' }
|
||||
],
|
||||
[
|
||||
'does not offer force delete when Electron wraps SSH filesystem provider failures',
|
||||
"Error invoking remote method 'worktrees:remove': Error: SSH filesystem provider unavailable",
|
||||
'SSH filesystem provider unavailable',
|
||||
{ canForceDelete: false, forceDeleteReason: null }
|
||||
]
|
||||
])('%s', async (_title, wrapped, displayed, classification) => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo1::/workspace/feature-wt'
|
||||
const error =
|
||||
"Error invoking remote method 'worktrees:remove': Error: Failed to delete worktree at /workspace/feature-wt. ?? scratch.txt"
|
||||
|
||||
mockApi.worktrees.remove.mockRejectedValueOnce(new Error(error))
|
||||
|
||||
mockApi.worktrees.remove.mockRejectedValueOnce(new Error(wrapped))
|
||||
seedStore(store, {
|
||||
worktreesByRepo: {
|
||||
repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1' })]
|
||||
},
|
||||
worktreesByRepo: { repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1' })] },
|
||||
tabsByWorktree: {},
|
||||
ptyIdsByTabId: {},
|
||||
terminalLayoutsByTabId: {}
|
||||
@@ -396,12 +412,11 @@ describe('removeWorktree cascade', () => {
|
||||
|
||||
const result = await store.getState().removeWorktree({ id: worktreeId, executionHostId: null })
|
||||
|
||||
expect(result).toEqual({ ok: false, error })
|
||||
expect(result).toEqual({ ok: false, error: displayed })
|
||||
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
|
||||
isDeleting: false,
|
||||
error,
|
||||
canForceDelete: true,
|
||||
forceDeleteReason: 'dirty'
|
||||
error: displayed,
|
||||
...classification
|
||||
})
|
||||
})
|
||||
|
||||
@@ -462,34 +477,6 @@ describe('removeWorktree cascade', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('offers force delete when Git already removed an unregistered worktree', async () => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo1::/workspace/deleted-wt'
|
||||
const error =
|
||||
"Error invoking remote method 'worktrees:remove': Error: Worktree is no longer registered with Git and its directory is already gone."
|
||||
|
||||
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({ id: worktreeId, executionHostId: null })
|
||||
|
||||
expect(result).toEqual({ ok: false, error })
|
||||
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
|
||||
isDeleting: false,
|
||||
error,
|
||||
canForceDelete: true,
|
||||
forceDeleteReason: 'missing-registration'
|
||||
})
|
||||
})
|
||||
|
||||
it('sets canForceDelete=false when force=true removal fails', async () => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo1::/path/wt1'
|
||||
@@ -579,34 +566,6 @@ describe('removeWorktree cascade', () => {
|
||||
expect(store.getState().deleteStateByWorktreeId[worktreeId]?.canForceDelete).toBe(false)
|
||||
})
|
||||
|
||||
it('does not offer force delete when Electron wraps SSH filesystem provider failures', async () => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo1::/path/wt1'
|
||||
const error =
|
||||
"Error invoking remote method 'worktrees:remove': Error: SSH filesystem provider unavailable"
|
||||
|
||||
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({ id: worktreeId, executionHostId: null })
|
||||
|
||||
expect(result).toEqual({ ok: false, error })
|
||||
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
|
||||
isDeleting: false,
|
||||
error,
|
||||
canForceDelete: false,
|
||||
forceDeleteReason: null
|
||||
})
|
||||
})
|
||||
|
||||
it.each([
|
||||
'Could not connect to the remote Orca runtime.',
|
||||
'Remote Orca runtime closed the connection.',
|
||||
@@ -617,6 +576,8 @@ describe('removeWorktree cascade', () => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo1::/path/wt1'
|
||||
const error = `Error invoking remote method 'runtime-environments:call': Error: ${runtimeFailure}`
|
||||
// The wrapper is stripped for display; the runtime failure text is what the user sees.
|
||||
const displayed = runtimeFailure
|
||||
|
||||
mockApi.runtimeEnvironments.call.mockImplementation((args: { method: string }) => {
|
||||
const compatibility = createCompatibleRuntimeStatusResponseIfNeeded(args)
|
||||
@@ -648,10 +609,10 @@ describe('removeWorktree cascade', () => {
|
||||
.getState()
|
||||
.removeWorktree({ id: worktreeId, executionHostId: null })
|
||||
|
||||
expect(result).toEqual({ ok: false, error })
|
||||
expect(result).toEqual({ ok: false, error: displayed })
|
||||
expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({
|
||||
isDeleting: false,
|
||||
error,
|
||||
error: displayed,
|
||||
canForceDelete: false,
|
||||
forceDeleteReason: null
|
||||
})
|
||||
|
||||
@@ -10,6 +10,8 @@ export type WorktreeDeleteState = {
|
||||
canForceDelete: boolean
|
||||
forceDeleteReason: WorktreeForceDeleteReason | null
|
||||
lockReason?: string | null
|
||||
/** The removal was refused by a failed archive hook, so "Delete anyway" is offered (#19334). */
|
||||
canWaiveArchiveHook?: boolean
|
||||
}
|
||||
|
||||
export type WorktreeDeleteStateTarget = Pick<Worktree, 'id' | 'hostId'>
|
||||
|
||||
@@ -9,6 +9,9 @@ export type RemoveWorktreeOptions = {
|
||||
// Why (#11960): only an explicit Force Delete waives the proof that every
|
||||
// PTY stopped; `force` alone is set by the ordinary delete confirmation.
|
||||
allowUnverifiedPtyStop?: boolean
|
||||
// Why (#19334): waives a FAILED archive hook. Set only by the explicit "Delete anyway" retry
|
||||
// after the user has seen the refusal -- never by the ordinary confirmation, never by `force`.
|
||||
allowFailedArchiveHook?: boolean
|
||||
snapshotPruneBatchId?: string
|
||||
/** Fresh cleanup-scan evidence for a same-id owner not represented in the catalog. */
|
||||
sameIdSurvivingHostId?: ExecutionHostId
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { ARCHIVE_HOOK_TIMEOUT_MS } from '../../../../shared/worktree/archive-hook-removal-gate'
|
||||
import type { AppState } from '../types'
|
||||
import type { RuntimeEnvironmentCallRequest } from '../../runtime/runtime-compatibility-test-fixture'
|
||||
import { makeWorktree } from './worktrees-slice-test-fixtures'
|
||||
@@ -64,7 +65,8 @@ describe('worktree remote runtime mutations', () => {
|
||||
allowUnverifiedPtyStop: false,
|
||||
runHooks: true
|
||||
},
|
||||
timeoutMs: 60_000,
|
||||
// Hooks run here, so the client must outlast the host's archive-hook budget (#19334).
|
||||
timeoutMs: ARCHIVE_HOOK_TIMEOUT_MS + 60_000,
|
||||
expectedEnvironmentPairingRevision: undefined,
|
||||
expectedRuntimeId: undefined
|
||||
})
|
||||
@@ -127,7 +129,8 @@ describe('worktree remote runtime mutations', () => {
|
||||
allowUnverifiedPtyStop: false,
|
||||
runHooks: true
|
||||
},
|
||||
timeoutMs: 60_000,
|
||||
// Hooks run here, so the client must outlast the host's archive-hook budget (#19334).
|
||||
timeoutMs: ARCHIVE_HOOK_TIMEOUT_MS + 60_000,
|
||||
expectedEnvironmentPairingRevision: undefined,
|
||||
expectedRuntimeId: undefined
|
||||
})
|
||||
@@ -225,7 +228,8 @@ describe('worktree remote runtime mutations', () => {
|
||||
allowUnverifiedPtyStop: false,
|
||||
runHooks: true
|
||||
},
|
||||
timeoutMs: 60_000
|
||||
// Hooks run here, so the client must outlast the host's archive-hook budget (#19334).
|
||||
timeoutMs: ARCHIVE_HOOK_TIMEOUT_MS + 60_000
|
||||
})
|
||||
expect(mockApi.worktrees.remove).not.toHaveBeenCalled()
|
||||
expect(store.getState().worktreesByRepo['repo-ssh']).toEqual([])
|
||||
@@ -601,6 +605,8 @@ describe('worktree remote runtime mutations', () => {
|
||||
force: undefined,
|
||||
// Why (#11960): an ordinary remove never waives the PTY-stop proof.
|
||||
allowUnverifiedPtyStop: false,
|
||||
// Why (#19334): nor a failed archive hook — only the explicit "Delete anyway" retry does.
|
||||
allowFailedArchiveHook: false,
|
||||
skipArchive: false
|
||||
})
|
||||
expect(runtimeEnvironmentCall).not.toHaveBeenCalled()
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { RemoveWorktreeResult } from '../../../../../../shared/worktree/cre
|
||||
import { callRuntimeRpc, type getActiveRuntimeTarget } from '../../../../runtime/runtime-rpc-client'
|
||||
import { toRuntimeWorktreeSelector } from '../../../../runtime/runtime-worktree-selector'
|
||||
import type { RemoveWorktreeOptions } from '../../worktree-removal-options'
|
||||
import { ARCHIVE_HOOK_TIMEOUT_MS } from '../../../../../../shared/worktree/archive-hook-removal-gate'
|
||||
|
||||
/**
|
||||
* Sends the destructive removal over whichever transport owns this workspace.
|
||||
@@ -36,6 +37,7 @@ export async function dispatchWorktreeRemoval(args: {
|
||||
hostId,
|
||||
force,
|
||||
allowUnverifiedPtyStop: options?.allowUnverifiedPtyStop === true,
|
||||
allowFailedArchiveHook: options?.allowFailedArchiveHook === true,
|
||||
skipArchive,
|
||||
...snapshotPruneBatch
|
||||
})
|
||||
@@ -50,9 +52,19 @@ export async function dispatchWorktreeRemoval(args: {
|
||||
...(effectiveHostId ? { hostId: effectiveHostId } : {}),
|
||||
force,
|
||||
allowUnverifiedPtyStop: options?.allowUnverifiedPtyStop === true,
|
||||
// Why only when set, unlike the IPC branch: this crosses a version boundary, and a host
|
||||
// that predates the gate drops unknown params silently. Send it when it means something.
|
||||
...(options?.allowFailedArchiveHook === true ? { allowFailedArchiveHook: true } : {}),
|
||||
runHooks: !skipArchive
|
||||
},
|
||||
{ timeoutMs: 60_000 }
|
||||
{
|
||||
// Why not a flat 60s (#19334): the host may run an archive hook for up to
|
||||
// ARCHIVE_HOOK_TIMEOUT_MS before it decides anything. A client that gives up first reports a
|
||||
// failure for a removal that is still in progress — and if the hook then succeeds, the host
|
||||
// deletes the checkout while the user has been told the delete failed. Outlast the hook when
|
||||
// one can run; keep the short budget when none will.
|
||||
timeoutMs: skipArchive ? 60_000 : ARCHIVE_HOOK_TIMEOUT_MS + 60_000
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,8 @@ import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed'
|
||||
import { getActiveRuntimeTarget } from '../../../../runtime/runtime-rpc-client'
|
||||
import { forgetHugeRepoWarningDismissalsForWorktrees } from '@/lib/source-control-huge-repo-warning-dismissals'
|
||||
import { forgetWorktreeSleepIntent } from '@/lib/worktree-sleep-intent'
|
||||
import { readIpcErrorDetail } from '@/lib/ipc-error'
|
||||
import { isArchiveHookRemovalError } from '../../../../../../shared/worktree/archive-hook-removal-gate'
|
||||
import { showPreservedBranchToast } from '@/components/sidebar/preserved-branch-toast'
|
||||
import {
|
||||
resolveWorktreeOperationRouteResult,
|
||||
@@ -297,13 +299,18 @@ export function createRemoveWorktree(
|
||||
} catch (err) {
|
||||
// Why: git refusing a non-force delete for dirty/untracked files is a handled user decision, not an app error.
|
||||
console.warn('Failed to remove worktree:', err)
|
||||
const error = err instanceof Error ? err.message : String(err)
|
||||
// The raw message arrives wrapped in Electron's IPC channel and class names; this string is
|
||||
// read by a user in a toast, and the refusal sentence has to lead it.
|
||||
const error = readIpcErrorDetail(err) ?? (err instanceof Error ? err.message : String(err))
|
||||
const forceDeleteReason = classifyWorktreeForceDeleteReason(
|
||||
error,
|
||||
force,
|
||||
options?.allowUnverifiedPtyStop === true
|
||||
)
|
||||
const locked = isLockedWorktreeRemovalError(error)
|
||||
// Why (#19334): the refusal is the only failure a retry can clear by waiving rather than by
|
||||
// fixing state, so the toast needs to know it may offer that choice.
|
||||
const canWaiveArchiveHook = isArchiveHookRemovalError(error)
|
||||
set((s) => ({
|
||||
deleteStateByWorktreeId: {
|
||||
...s.deleteStateByWorktreeId,
|
||||
@@ -313,6 +320,7 @@ export function createRemoveWorktree(
|
||||
error,
|
||||
canForceDelete: forceDeleteReason !== null,
|
||||
forceDeleteReason,
|
||||
...(canWaiveArchiveHook ? { canWaiveArchiveHook: true } : {}),
|
||||
...(locked ? { lockReason: getLockedWorktreeRemovalReason(error) } : {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ export const CLI_GLOBAL_FLAGS: readonly string[] = ['help', 'json', ...CLI_GLOBA
|
||||
|
||||
export const CLI_BOOLEAN_FLAGS = new Set([
|
||||
'all',
|
||||
'allow-failed-archive-hook',
|
||||
'attachments',
|
||||
'children',
|
||||
'comments',
|
||||
|
||||
@@ -114,6 +114,17 @@ export const TERMINAL_QUICK_COMMANDS_RUNTIME_CAPABILITY = 'terminal.quick-comman
|
||||
// status.worktreeCreateIdempotency carries the optional host retention policy.
|
||||
export const WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY =
|
||||
'worktree.create-idempotency.v1' as const
|
||||
// Scope of the claim: a hook that RUNS and fails cannot delete the checkout. It does not promise
|
||||
// the hook was found — an SSH host whose orca.yaml cannot be read answers "no hook" and the removal
|
||||
// proceeds, because a failed read is indistinguishable from an absent file across the relay
|
||||
// (#20196 tracks the provider contract that would separate them).
|
||||
// Why (#19334): "accepts --run-hooks" and "refuses to delete when the archive hook fails" were
|
||||
// indistinguishable from the outside — both take the flag and behave identically on success, so
|
||||
// the only way to tell an unfixed host apart was to fail a hook and see whether the checkout
|
||||
// survived. Lifecycle integrations keep teardown evidence inside the checkout and cannot risk
|
||||
// that. Advertised unconditionally: every build carrying this constant has the gate.
|
||||
export const WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY =
|
||||
'worktree.archive-failure-blocking.v1' as const
|
||||
export const CODEX_RESET_CREDIT_RUNTIME_CAPABILITY = 'accounts.codex-reset-credit.v1' as const
|
||||
export const ACCOUNT_IMPORT_RUNTIME_CAPABILITY = 'accounts.import-host-credentials.v1' as const
|
||||
// Why: older hosts cannot reconcile terminal.create's mutation after losing the reply, so clients may only retry unknown outcomes when advertised.
|
||||
@@ -283,6 +294,7 @@ export const RUNTIME_CAPABILITIES = [
|
||||
TERMINAL_PAIRED_PARKING_RUNTIME_CAPABILITY,
|
||||
TERMINAL_QUICK_COMMANDS_RUNTIME_CAPABILITY,
|
||||
WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY,
|
||||
WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY,
|
||||
TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY,
|
||||
SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY,
|
||||
SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY,
|
||||
|
||||
@@ -171,7 +171,10 @@ export const WorktreeRemove = WorktreeSelector.extend({
|
||||
// desktop sets `force` for an ordinary confirmed delete too, so the PTY-stop
|
||||
// waiver travels on its own field.
|
||||
allowUnverifiedPtyStop: OptionalBoolean,
|
||||
runHooks: OptionalBoolean
|
||||
runHooks: OptionalBoolean,
|
||||
// Why (#19334): a failed archive hook blocks removal. This waives that refusal and is recorded
|
||||
// in the result; it is NOT `force`, and it does not decide whether the hook runs.
|
||||
allowFailedArchiveHook: OptionalBoolean
|
||||
})
|
||||
|
||||
export const WorktreeForceDeleteBranch = WorktreeSelector.extend({
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
RUNTIME_CAPABILITIES,
|
||||
WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY
|
||||
} from '../protocol-version'
|
||||
|
||||
// Why (#19334): the reporter's integration (Harbour) keeps its teardown ownership evidence inside
|
||||
// the checkout, so it must know *before* removing anything whether this host refuses to delete on a
|
||||
// failed archive hook. It cannot probe for that — probing means risking the data loss.
|
||||
describe('worktree.archive-failure-blocking.v1', () => {
|
||||
it('uses the id the reporting integration already codes against', () => {
|
||||
expect(WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY).toBe(
|
||||
'worktree.archive-failure-blocking.v1'
|
||||
)
|
||||
})
|
||||
|
||||
it('is advertised by every build that carries the gate', () => {
|
||||
expect(RUNTIME_CAPABILITIES).toContain(WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,117 @@
|
||||
// Why (#19334): the archive hook is a user's last chance to save work off a checkout Orca is
|
||||
// about to delete. A failed hook used to be logged and stepped over, so the delete went ahead
|
||||
// with nothing archived. It is a precondition, evaluated before any stop/delete mutation.
|
||||
|
||||
/**
|
||||
* How long an archive hook gets before it is cut off. Shared because a client waiting on a removal
|
||||
* has to outlast it: a client that gives up first reports a failure for a hook that is still
|
||||
* running, and the host then completes the delete anyway — telling the user the opposite of what
|
||||
* happened to their checkout (#19334).
|
||||
*/
|
||||
export const ARCHIVE_HOOK_TIMEOUT_MS = 120_000
|
||||
|
||||
/** RPC/CLI error code for a removal refused because the repo's archive hook did not succeed. */
|
||||
export const ARCHIVE_HOOK_FAILED_REMOVAL_CODE = 'worktree_archive_hook_failed'
|
||||
|
||||
export const ARCHIVE_HOOK_FAILED_REMOVAL_PREFIX = 'Archive hook failed for worktree:'
|
||||
|
||||
// One string, three surfaces: the CLI, RPC callers, and the desktop toast that now carries its own
|
||||
// Delete Anyway button. Naming only the CLI flag sent desktop users to a terminal for a button that
|
||||
// was six inches away, so both affordances are named and neither is presented as the only one.
|
||||
export const ARCHIVE_HOOK_OVERRIDE_HINT =
|
||||
'Nothing was stopped, deleted or deregistered. Fix the hook and retry, or delete anyway with an explicit waiver — "Delete Anyway" in the app, or --allow-failed-archive-hook on the CLI.'
|
||||
|
||||
/**
|
||||
* `exited` means the host reported a non-zero exit for this hook run. `unverifiable` covers every
|
||||
* case where the hook's outcome was never observed — spawn failure, timeout, lost contact with the
|
||||
* execution host. Loss of contact is never evidence that the hook passed, so both block removal.
|
||||
* Vocabulary is deliberately the `UnstoppedPtyVerdict` spelling; see docs/reference/ssh-execution-boundary.md.
|
||||
*/
|
||||
export type ArchiveHookOutcome = 'exited' | 'unverifiable'
|
||||
|
||||
export type ArchiveHookFailure = {
|
||||
worktreePath: string
|
||||
outcome: ArchiveHookOutcome
|
||||
/** Only ever set for `exited` — an absent code is not a zero code. */
|
||||
exitCode?: number
|
||||
output: string
|
||||
}
|
||||
|
||||
/** What a caller sees when the failure was explicitly overridden instead of blocking. */
|
||||
export type ArchiveHookOverride = ArchiveHookFailure & { overridden: true }
|
||||
|
||||
export class WorktreeArchiveHookFailedError extends Error {
|
||||
readonly code = ARCHIVE_HOOK_FAILED_REMOVAL_CODE
|
||||
readonly data: ArchiveHookFailure
|
||||
|
||||
constructor(failure: ArchiveHookFailure) {
|
||||
super(formatArchiveHookFailure(failure))
|
||||
this.name = 'WorktreeArchiveHookFailedError'
|
||||
this.data = failure
|
||||
}
|
||||
}
|
||||
|
||||
function describeArchiveHookVerdict(failure: ArchiveHookFailure): string {
|
||||
return failure.outcome === 'exited'
|
||||
? `exited ${failure.exitCode}`
|
||||
: 'outcome unverifiable (the hook never reported an exit)'
|
||||
}
|
||||
|
||||
export function formatArchiveHookFailure(failure: ArchiveHookFailure): string {
|
||||
const output = failure.output.trim()
|
||||
return [
|
||||
`${ARCHIVE_HOOK_FAILED_REMOVAL_PREFIX} ${failure.worktreePath} — ${describeArchiveHookVerdict(failure)}.`,
|
||||
ARCHIVE_HOOK_OVERRIDE_HINT,
|
||||
...(output ? [output] : [])
|
||||
].join(' ')
|
||||
}
|
||||
|
||||
/**
|
||||
* The waived case says the opposite of the refusal: the removal DID go ahead. Reusing
|
||||
* `formatArchiveHookFailure` here printed "Nothing was stopped, deleted or deregistered" directly
|
||||
* after deleting the checkout.
|
||||
*/
|
||||
export function formatArchiveHookOverride(override: ArchiveHookOverride): string {
|
||||
const output = override.output.trim()
|
||||
return [
|
||||
`Archive hook failed for worktree: ${override.worktreePath} — ${describeArchiveHookVerdict(override)}.`,
|
||||
'Deleted anyway because the failure was explicitly waived; nothing was archived.',
|
||||
...(output ? [output] : [])
|
||||
].join(' ')
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrow an unknown rejection to the typed refusal, or rethrow it. This is the branch a real
|
||||
* caller writes, so tests asserting on a refusal should go through it rather than re-deriving it.
|
||||
*/
|
||||
export function asArchiveHookRefusal(error: unknown): WorktreeArchiveHookFailedError {
|
||||
if (error instanceof WorktreeArchiveHookFailedError) {
|
||||
return error
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
||||
/** Recognise the refusal on a surface that only has the message, e.g. a renderer toast. */
|
||||
export function isArchiveHookRemovalError(error: string): boolean {
|
||||
return error.includes(ARCHIVE_HOOK_FAILED_REMOVAL_PREFIX)
|
||||
}
|
||||
|
||||
/** Shape both the local and the SSH archive runners answer with. */
|
||||
export type ArchiveHookRunResult = {
|
||||
success: boolean
|
||||
output: string
|
||||
/** Omitted whenever no exit was observed, which classifies the failure as `unverifiable`. */
|
||||
exitCode?: number
|
||||
}
|
||||
|
||||
export function classifyArchiveHookFailure(
|
||||
worktreePath: string,
|
||||
result: ArchiveHookRunResult
|
||||
): ArchiveHookFailure {
|
||||
return {
|
||||
worktreePath,
|
||||
outcome: typeof result.exitCode === 'number' ? 'exited' : 'unverifiable',
|
||||
...(typeof result.exitCode === 'number' ? { exitCode: result.exitCode } : {}),
|
||||
output: result.output
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { ExecutionHostId } from '../execution-host'
|
||||
import type { ArchiveHookOverride } from './archive-hook-removal-gate'
|
||||
import type { WorkspaceSource } from '../workspace-source'
|
||||
import type { TaskSourceContext } from '../task-source-context'
|
||||
import type { WorkspaceKey } from '../folder-workspace-types'
|
||||
@@ -207,6 +208,8 @@ export type PreservedWorktreeBranch = {
|
||||
|
||||
export type RemoveWorktreeResult = {
|
||||
preservedBranch?: PreservedWorktreeBranch
|
||||
/** Present only when a FAILED archive hook was explicitly waived for this removal (#19334). */
|
||||
archiveHookOverride?: ArchiveHookOverride
|
||||
}
|
||||
|
||||
export type ForceDeleteWorktreeBranchResult = {
|
||||
|
||||
Reference in New Issue
Block a user