mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
OrcaWinandOrcaWin
20a2901677
* fix(worktree): tell the truth about live PTYs, and offer force for a wedged sweep Two gaps in the #11960 force path: The delete toast described every unstopped-PTY failure as "could not confirm every terminal has exited", including the case where verification positively watched them running. Force Delete proceeds either way, so the user was being asked to waive a doubt that did not exist while a running agent's uncommitted work died with it. The live verdict now gets copy that says so. A sweep that rejects before any per-PTY verdict exists (wedged daemon, dropped SSH channel) fails with a teardown-timeout message that the force classifier did not recognise, so no Force Delete button appeared — the exact dead end #11960 set out to remove. That error now carries the shared prefix and classifies. * fix(worktree): close the sweep-rejection wedge and stop racing the delete Review of #12394 found the fix covered only half the wedge it named, and routed users into a force path whose own safety comment was untrue. 1. Only the outer deadline was classifiable. When a provider *rejects* the sweep — dropped SSH channel, erroring daemon — settleBeforeDeadline rejects with the provider's original error, which carries no marker, so classifyWorktreeForceDeleteReason still returned null and no Force Delete button rendered. That is the exact case #11960 named. A rejected sweep on the destructive path is now reworded through the existing unstopped-PTY prefix (provider text preserved, original kept as `cause`), so old and new clients alike classify it as 'unstopped-pty'. 2. Force could delete files while a sweep was still running. The deadline rejects without cancelling run(), so allSettled resolved with shutdown() still in flight — by construction the deadline error can only fire while something is in flight. Force then deleted the directory a live PTY still held open (EBUSY / half-delete on Windows and WSL). Sweeps are now tracked so the forced path waits for the abandoned work, bounded by a 2s grace; force never wedges, and when the grace expires the warning says handles may outlive the delete instead of implying the sweep finished. 3. The toast test named for the classifier passed the reason in as a literal, so it never exercised it. It now derives the reason exactly as the store does, and fails against main. 4. Added the missing unstoppedPtyLive key to the English catalog. 5. isProvenLivePtyRemovalError anchored the 'still live:' marker to the detail separator, so a worktree path can no longer spell out a live verdict and flip the toast to the destructive copy. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
70 lines
3.2 KiB
TypeScript
70 lines
3.2 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
import {
|
|
classifyWorktreeForceDeleteReason,
|
|
isProvenLivePtyRemovalError,
|
|
UNSTOPPED_PTY_REMOVAL_PREFIX,
|
|
WORKTREE_TEARDOWN_FORCE_HINT,
|
|
WORKTREE_TEARDOWN_TIMEOUT_PREFIX
|
|
} from './worktree-removal'
|
|
|
|
const liveError = `${UNSTOPPED_PTY_REMOVAL_PREFIX} repo-1::/w — still live: term_a. ${WORKTREE_TEARDOWN_FORCE_HINT}`
|
|
const unverifiableError = `${UNSTOPPED_PTY_REMOVAL_PREFIX} repo-1::/w — could not verify these exited: term_a (daemon socket closed). ${WORKTREE_TEARDOWN_FORCE_HINT}`
|
|
|
|
// Why (#11960): the desktop Force Delete button renders only when this classifier
|
|
// returns a reason. The PTY-teardown error tells the user to force-delete, so an
|
|
// unclassified message leaves them reading advice they cannot act on.
|
|
describe('classifyWorktreeForceDeleteReason for unstopped PTYs', () => {
|
|
it('offers force for a PTY that is still live', () => {
|
|
expect(classifyWorktreeForceDeleteReason(liveError)).toBe('unstopped-pty')
|
|
})
|
|
|
|
it('offers force when the stop could not be verified', () => {
|
|
expect(classifyWorktreeForceDeleteReason(unverifiableError)).toBe('unstopped-pty')
|
|
})
|
|
|
|
// Why (#11960): the ordinary delete confirmation already passes force:true to skip
|
|
// the dirty-file prompt. If that suppressed the offer, the most common desktop
|
|
// delete would hit the gate with no Force Delete button anywhere — the original
|
|
// dead end, restored.
|
|
it('still offers force when the failed attempt only set force', () => {
|
|
expect(classifyWorktreeForceDeleteReason(liveError, true)).toBe('unstopped-pty')
|
|
})
|
|
|
|
it('does not re-offer force once the waiver itself was already used', () => {
|
|
expect(classifyWorktreeForceDeleteReason(liveError, true, true)).toBeNull()
|
|
expect(classifyWorktreeForceDeleteReason(liveError, false, true)).toBeNull()
|
|
})
|
|
|
|
// Why: a wedged provider rejects the sweep before any per-PTY verdict exists, so the
|
|
// failure arrives worded as a teardown timeout. The waiver clears it exactly like an
|
|
// unproven stop, so leaving it unclassified hid the button for the wedge #11960 targeted.
|
|
it('offers force when the teardown sweep itself timed out', () => {
|
|
expect(
|
|
classifyWorktreeForceDeleteReason(
|
|
`${WORKTREE_TEARDOWN_TIMEOUT_PREFIX} repo-1::/w. ${WORKTREE_TEARDOWN_FORCE_HINT}`
|
|
)
|
|
).toBe('unstopped-pty')
|
|
})
|
|
|
|
it('leaves unrelated failures unclassified', () => {
|
|
expect(classifyWorktreeForceDeleteReason('some other failure')).toBeNull()
|
|
})
|
|
})
|
|
|
|
// The live verdict escalates the delete toast to "Force Delete will kill them", so it must
|
|
// come from Orca's own detail — not from the worktree id, which is a user-chosen path.
|
|
describe('isProvenLivePtyRemovalError', () => {
|
|
it('reads the verdict Orca actually recorded', () => {
|
|
expect(isProvenLivePtyRemovalError(liveError)).toBe(true)
|
|
expect(isProvenLivePtyRemovalError(unverifiableError)).toBe(false)
|
|
})
|
|
|
|
it('does not let a worktree path spell out a live verdict', () => {
|
|
expect(
|
|
isProvenLivePtyRemovalError(
|
|
`${UNSTOPPED_PTY_REMOVAL_PREFIX} repo-1::/Users/dev/still live: notes — could not verify these exited: term_a (daemon socket closed)`
|
|
)
|
|
).toBe(false)
|
|
})
|
|
})
|