fix(worktrees): protect user changes across ambiguous move failures

Post-merge hardening of the session-to-worktree move (#2998), driven by
review findings on the follow-up pass:

- an ambiguous transport failure (relay abort, timeout) on the
  change-carrying move no longer force-deletes the fresh worktree that
  may hold the user's only copy of their changes; both intent kinds
  surface honest guidance and refresh both directories
- assertSdkSuccess re-tags ambiguous transport errors when wrapping SDK
  failures, so ambiguity classification survives the wrapper on every
  path, matching the prompt-send precedent
- session liveness checks scan all child stores plus the global status
  index, and report unknown (not idle) when no store covers the session
  — an evicted background directory can no longer make a busy session
  look movable
- incomplete-rollback errors carry the changes-may-be-in-destination
  guidance instead of swallowing it
- move-message assembly shared across the three call sites; tests now
  exercise the real ambiguity classifier (extracted to
  send-failure-classification.ts) instead of a hand-mirrored mock
- i18n fallout from the merge train: Turkish gains the 21 worktree-move
  keys, all 12 locales get the hedged ambiguous-failure toast; owning
  DOCUMENTATION.md files record the new contracts
This commit is contained in:
Bohdan Triapitsyn
2026-08-28 12:03:55 +03:00
parent 9d279137ce
commit 4f53db17e6
21 changed files with 493 additions and 107 deletions
@@ -7,6 +7,7 @@ import path from 'node:path';
import type { State } from '@/sync/types';
import type { WorktreeMetadata } from '@/types/worktree';
import type { ProjectRef } from '@/lib/worktrees/worktreeManager';
import { markAmbiguousTransportFailure } from '@/lib/relay/transport-error';
import type { SessionTreeMoveIntent, SessionTreeMoveMessages } from './sessionWorktreeMove';
const moveCalls: Array<{
@@ -175,11 +176,28 @@ mock.module('@/stores/useGlobalSessionsStore', () => ({
},
}));
// Mirrors session-actions: every child store is scanned, because a session's
// live status can be reported by a directory other than its own, and "no store
// covers this session" is 'unknown', never 'idle' — a populated store map says
// nothing about a session none of its stores holds.
const getSessionLiveActivity = (sessionId: string): 'unknown' | 'idle' | 'active' => {
for (const state of directoryStates.values()) {
const status = state.session_status[sessionId];
if (status && status.type !== 'idle') return 'active';
}
for (const state of directoryStates.values()) {
if (Object.hasOwn(state.session_status, sessionId)) return 'idle';
}
return 'unknown';
};
mock.module('@/sync/session-actions', () => ({
moveSessionToDirectory: (session: Session, sourceDirectory: string, destinationDirectory: string, moveChanges = true) => {
moveCalls.push({ sessionId: session.id, sourceDirectory, destinationDirectory, moveChanges });
return moveSessionImplementation(session, sourceDirectory, destinationDirectory, moveChanges);
},
getSessionLiveActivity,
isSessionBusyNow: (sessionId: string) => getSessionLiveActivity(sessionId) === 'active',
}));
mock.module('@/sync/session-ui-store', () => ({
@@ -239,6 +257,7 @@ const makeMoveMessages = (): SessionTreeMoveMessages => ({
failure: 'move failed',
sourceVerificationFailed: 'source verification failed',
applyChangesFailed: 'apply changes failed',
changesMayBeInDestination: 'changes may be in destination',
});
const makeQuickIntent = (): SessionTreeMoveIntent => ({
@@ -1044,6 +1063,206 @@ describe('moveSessionTreeToExistingWorktree', () => {
expect(toastErrors).toEqual([{ title: 'move failed', description: 'Destination directory belongs to another project' }]);
});
const requestDirtyQuickMove = (): void => {
getGitStatusImplementation = async () => ({
current: 'feature',
isClean: false,
files: [{ path: 'working.ts', index: ' ', working_dir: 'M' }],
});
requestSessionTreeMove(makeQuickIntent());
};
test('keeps a newly created worktree when an ambiguous failure may have transferred the changes', async () => {
setStatuses('/source', { root: 'idle' });
moveSessionImplementation = async () => {
throw new Error('Request timed out');
};
requestDirtyQuickMove();
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(true);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'move failed', description: 'changes may be in destination' }]);
expect(removeWorktreeCalls).toEqual([]);
});
test('removes a newly created worktree when the change transfer is definitely rejected', async () => {
setStatuses('/source', { root: 'idle' });
moveSessionImplementation = async () => {
throw Object.assign(new Error('Unable to apply your changes in the destination directory: conflict'), { status: 400 });
};
requestDirtyQuickMove();
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(true);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'move failed', description: 'apply changes failed' }]);
expect(removeWorktreeCalls).toEqual([{
projectDirectory: '/repo',
directory: '/created-worktree',
deleteLocalBranch: true,
}]);
});
test('removes a newly created worktree when an ambiguous failure carried no changes', async () => {
setStatuses('/source', { root: 'idle' });
moveSessionImplementation = async () => {
throw new Error('Request timed out');
};
requestDirtyQuickMove();
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(false);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'move failed', description: 'Request timed out' }]);
expect(removeWorktreeCalls).toEqual([{
projectDirectory: '/repo',
directory: '/created-worktree',
deleteLocalBranch: true,
}]);
});
test('removes a newly created worktree when a descendant fails ambiguously before the root moved', async () => {
setStatuses('/source', { root: 'idle', child: 'idle' });
moveSessionImplementation = async (session) => {
if (session.id === 'child') throw new Error('Request timed out');
};
getGitStatusImplementation = async () => ({
current: 'feature',
isClean: false,
files: [{ path: 'working.ts', index: ' ', working_dir: 'M' }],
});
requestSessionTreeMove({
kind: 'quick',
root: makeSession('root'),
descendants: [makeSession('child')],
sourceDirectory: '/source',
messages: makeMoveMessages(),
});
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(true);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'move failed', description: 'Request timed out' }]);
expect(removeWorktreeCalls).toEqual([{
projectDirectory: '/repo',
directory: '/created-worktree',
deleteLocalBranch: true,
}]);
});
test('refuses to move a session whose live status is reported by another directory', async () => {
setStatuses('/source', { root: 'idle' });
setStatuses('/other-directory', { root: 'busy' });
await expect(moveSessionTreeToExistingWorktree({
root: makeSession('root'),
descendants: [],
sourceDirectory: '/source',
destination: makeWorktreeMetadata(),
moveChanges: false,
})).rejects.toThrow('Session is not idle');
expect(moveCalls).toEqual([]);
});
test('refuses to move when no child store can report session status', async () => {
directoryStates.clear();
await expect(moveSessionTreeToExistingWorktree({
root: makeSession('root'),
descendants: [],
sourceDirectory: '/source',
destination: makeWorktreeMetadata(),
moveChanges: false,
})).rejects.toThrow('Session status is unavailable');
expect(moveCalls).toEqual([]);
});
test('keeps a newly created worktree when the relay tags the failure as dispatched', async () => {
setStatuses('/source', { root: 'idle' });
moveSessionImplementation = async () => {
// The relay tunnel's own tag, matched by no message heuristic.
throw markAmbiguousTransportFailure(new Error('stream aborted by host'));
};
requestDirtyQuickMove();
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(true);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'move failed', description: 'changes may be in destination' }]);
expect(removeWorktreeCalls).toEqual([]);
expect(refreshCalls).toEqual([['/source', '/created-worktree']]);
});
test('reports the destination guidance for an ambiguous existing-worktree move', async () => {
setStatuses('/source', { root: 'idle' });
getGitStatusImplementation = async () => ({
current: 'feature',
isClean: false,
files: [{ path: 'working.ts', index: ' ', working_dir: 'M' }],
});
moveSessionImplementation = async () => {
throw markAmbiguousTransportFailure(new Error('stream aborted by host'));
};
requestSessionTreeMove({
kind: 'existing',
root: makeSession('root'),
descendants: [],
sourceDirectory: '/source',
destination: makeWorktreeMetadata(),
messages: makeMoveMessages(),
});
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(true);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors).toEqual([{ title: 'move failed', description: 'changes may be in destination' }]);
expect(removeWorktreeCalls).toEqual([]);
expect(refreshCalls).toEqual([['/source', '/destination']]);
});
test('keeps the destination guidance when rollback is also incomplete', async () => {
setStatuses('/source', { root: 'idle', child: 'idle' });
setStatuses('/destination', { child: 'idle' });
getGitStatusImplementation = async () => ({
current: 'feature',
isClean: false,
files: [{ path: 'working.ts', index: ' ', working_dir: 'M' }],
});
moveSessionImplementation = async (session, sourceDirectory) => {
if (session.id === 'root' && sourceDirectory === '/source') {
throw markAmbiguousTransportFailure(new Error('stream aborted by host'));
}
if (session.id === 'child' && sourceDirectory === '/destination') {
throw new Error('rollback failed');
}
};
requestSessionTreeMove({
kind: 'existing',
root: makeSession('root'),
descendants: [makeSession('child')],
sourceDirectory: '/source',
destination: makeWorktreeMetadata(),
messages: makeMoveMessages(),
});
await waitFor(() => getSessionTreeMoveConfirmation() !== null);
confirmSessionTreeMove(true);
await waitFor(() => toastErrors.length === 1);
expect(toastErrors[0]?.title).toBe('move failed');
expect(toastErrors[0]?.description).toContain('could not be fully rolled back');
expect(toastErrors[0]?.description).toContain('changes may be in destination');
});
test('surfaces a pre-destination preparation failure without attempting removal', async () => {
setStatuses('/source', { root: 'idle' });
resolveProjectRefImplementation = () => null;
@@ -1,13 +1,14 @@
import type { Session } from '@opencode-ai/sdk/v2';
import type { I18nKey } from '@/lib/i18n';
import { toast } from '@/components/ui';
import { checkIsGitRepository, getGitStatus } from '@/lib/gitApi';
import { normalizePath } from '@/lib/pathNormalization';
import { createQuickWorktree, resolveProjectRef } from '@/lib/worktreeSessionCreator';
import { getLatestWorktreeMetadata, removeProjectWorktree, type ProjectRef } from '@/lib/worktrees/worktreeManager';
import { refreshGlobalSessionsForDirectories } from '@/stores/useGlobalSessionsStore';
import { moveSessionToDirectory } from '@/sync/session-actions';
import { isAmbiguousSendFailure } from '@/sync/send-failure-classification';
import { getSessionLiveActivity, isSessionBusyNow, moveSessionToDirectory } from '@/sync/session-actions';
import { useSessionUIStore } from '@/sync/session-ui-store';
import { getDirectoryState } from '@/sync/sync-refs';
import type { WorktreeMetadata } from '@/types/worktree';
import { waitForWorktreeGitReady } from '@/lib/worktrees/worktreeBootstrap';
import { create } from 'zustand';
@@ -17,8 +18,22 @@ export type SessionTreeMoveMessages = {
failure: string;
sourceVerificationFailed: string;
applyChangesFailed: string;
changesMayBeInDestination: string;
};
/** Every move surface differs only in the success/failure pair, so the shared
* failure copy is resolved once here instead of at each call site. */
export const buildSessionTreeMoveMessages = (
t: (key: I18nKey) => string,
keys: { success: I18nKey; failure: I18nKey },
): SessionTreeMoveMessages => ({
success: t(keys.success),
failure: t(keys.failure),
sourceVerificationFailed: t('sessions.sidebar.session.moveToWorktree.sourceVerificationFailed'),
applyChangesFailed: t('sessions.sidebar.session.moveToWorktree.applyChangesFailed'),
changesMayBeInDestination: t('sessions.sidebar.session.moveToWorktree.changesMayBeInDestination'),
});
export type SessionTreeMoveIntent =
| {
kind: 'existing';
@@ -87,6 +102,13 @@ const setSessionMoveRequesting = (sessionId: string, requesting: boolean): void
});
};
// The control plane flattens every move failure into a single
// `MoveSessionError` carrying only `data.message`, so there is no status or
// error code to match on. This prefix is the exact text OpenCode's
// `message(MoveSession.ApplyChangesError)` returns in
// `packages/opencode/src/server/routes/instance/httpapi/handlers/control-plane.ts`.
// If upstream reworks that wording the friendlier toast silently degrades to
// the raw message, which is why the fallback stays readable.
const APPLY_CHANGES_MESSAGE = 'Unable to apply your changes in the destination directory';
const isApplyChangesError = (error: Error): boolean => {
@@ -116,16 +138,16 @@ const resolveSourceBranch = async (directory: string, projectDirectory: string):
throw new Error('Unable to determine the current branch');
};
const assertSessionsIdle = (sessions: Session[], sourceDirectory: string): void => {
const directoryState = getDirectoryState(sourceDirectory);
if (!directoryState) throw new Error('Session status is unavailable');
const statuses = directoryState.session_status;
const hasActiveSession = sessions.some((session) => {
const status = statuses[session.id]?.type;
return status === 'busy' || status === 'retry';
});
if (hasActiveSession) throw new Error('Session is not idle');
// Scans every child store instead of the source directory's: a session's live
// status can be reported by a directory other than the one that wins the
// directory dedup, and a directory-scoped read would then see no status at all
// and move a running session.
const assertSessionsIdle = (sessions: Session[]): void => {
for (const session of sessions) {
const activity = getSessionLiveActivity(session.id);
if (activity === 'unknown') throw new Error('Session status is unavailable');
if (activity === 'active') throw new Error('Session is not idle');
}
};
type RollbackFailure = {
@@ -133,21 +155,34 @@ type RollbackFailure = {
error: Error;
};
const createIncompleteRollbackError = (moveError: Error, rollbackFailures: RollbackFailure[]): Error => {
/** Rollback left sessions in the destination. `changesMayBeInDestination` says
* the same failure also carried the working tree changes with an unknown
* outcome, so the toast must keep that guidance instead of dropping it. */
class IncompleteRollbackError extends Error {
readonly changesMayBeInDestination: boolean;
constructor(message: string, cause: unknown, changesMayBeInDestination: boolean) {
super(message, { cause });
this.name = 'IncompleteRollbackError';
this.changesMayBeInDestination = changesMayBeInDestination;
}
}
const createIncompleteRollbackError = (
moveError: Error,
rollbackFailures: RollbackFailure[],
changesMayBeInDestination: boolean,
): Error => {
const rollbackSummary = rollbackFailures
.map(({ sessionId, error }) => `${sessionId}: ${error.message}`)
.join(', ');
return new Error(
return new IncompleteRollbackError(
`Session move partially failed and could not be fully rolled back: ${moveError.message}. Rollback failures: ${rollbackSummary}`,
{ cause: { moveError, rollbackFailures } },
{ moveError, rollbackFailures },
changesMayBeInDestination,
);
};
const isSessionBusyOrRetrying = (session: Session, directory: string): boolean => {
const status = getDirectoryState(directory)?.session_status[session.id]?.type;
return status === 'busy' || status === 'retry';
};
const rollbackMovedSessions = async (
sessions: Session[],
sourceDirectory: string,
@@ -156,7 +191,7 @@ const rollbackMovedSessions = async (
): Promise<RollbackFailure[]> => {
const failures: RollbackFailure[] = [];
for (const session of [...sessions].reverse()) {
if (isSessionBusyOrRetrying(session, worktreeDirectory)) {
if (isSessionBusyNow(session.id)) {
failures.push({ sessionId: session.id, error: new Error('Session is not idle') });
continue;
}
@@ -178,6 +213,16 @@ const rollbackMovedSessions = async (
return failures;
};
/** The move failed after the change-carrying request was already dispatched, so
* the user's changes may already be in the destination. A freshly created
* worktree is kept rather than deleted, because it may hold the only copy. */
class ChangesMayBeInDestinationError extends Error {
constructor(moveError: Error) {
super(moveError.message, { cause: moveError });
this.name = 'ChangesMayBeInDestinationError';
}
}
const removeFailedWorktree = async (
project: ProjectRef,
worktree: WorktreeMetadata,
@@ -191,6 +236,17 @@ const removeFailedWorktree = async (
throw moveError;
};
const refreshMovedDirectories = async (sourceDirectory: string, destinationDirectory: string | undefined): Promise<void> => {
const directories = destinationDirectory ? [sourceDirectory, destinationDirectory] : [sourceDirectory];
try {
await refreshGlobalSessionsForDirectories(directories);
} catch (error) {
// Direct action updates already reconciled both stores. Keep the outcome
// unchanged if this best-effort authoritative refresh is unavailable.
console.warn('[session-worktree-move] Failed to refresh moved sessions', error);
}
};
const moveSessionTreeTransaction = async (
input: {
root: Session;
@@ -217,10 +273,11 @@ const moveSessionTreeTransaction = async (
useSessionUIStore.getState().getWorktreeMetadata(session.id),
]),
);
assertSessionsIdle(sessions, input.sourceDirectory);
assertSessionsIdle(sessions);
let destination: Awaited<ReturnType<typeof prepareDestination>> | null = null;
const moved: Session[] = [];
let changesMoveOutcomeUnknown = false;
try {
destination = await prepareDestination();
for (const [index, session] of sessions.entries()) {
@@ -228,13 +285,18 @@ const moveSessionTreeTransaction = async (
// session to start running, so re-check the remaining source tree
// immediately before each move. The root moves last so no later
// descendant failure can require replaying a transferred patch.
assertSessionsIdle(sessions.slice(index), input.sourceDirectory);
await moveSessionToDirectory(
session,
input.sourceDirectory,
destination.directory,
session.id === input.root.id && input.moveChanges,
);
assertSessionsIdle(sessions.slice(index));
const movesChanges = session.id === input.root.id && input.moveChanges;
try {
await moveSessionToDirectory(session, input.sourceDirectory, destination.directory, movesChanges);
} catch (error) {
// A transport failure on the change-carrying request leaves the
// destination unknown: the server may have applied the patch before
// the response was lost. Definite rejections (the destination refused
// the patch) keep this false.
if (movesChanges && isAmbiguousSendFailure(error)) changesMoveOutcomeUnknown = true;
throw error;
}
moved.push(session);
if (session.id === input.root.id) continue;
useSessionUIStore.getState().setWorktreeMetadata(session.id, getLatestWorktreeMetadata(destination.metadata));
@@ -247,9 +309,19 @@ const moveSessionTreeTransaction = async (
destination?.directory ?? input.sourceDirectory,
previousMetadata,
);
if (rollbackFailures.length > 0) {
throw createIncompleteRollbackError(moveError, rollbackFailures);
if (changesMoveOutcomeUnknown) {
// The move request may have completed server-side, so the session's
// directory is unknown too. Reconcile both directories now instead of
// letting the sidebar contradict the toast until the next poll.
await refreshMovedDirectories(input.sourceDirectory, destination?.directory);
}
if (rollbackFailures.length > 0) {
throw createIncompleteRollbackError(moveError, rollbackFailures, changesMoveOutcomeUnknown);
}
// Checked before `onMoveFailure` so the quick path's worktree removal
// never runs while the user's changes may be sitting in it. Both intent
// kinds share the messaging.
if (changesMoveOutcomeUnknown) throw new ChangesMayBeInDestinationError(moveError);
if (destination?.onMoveFailure) {
return destination.onMoveFailure(moveError);
}
@@ -257,13 +329,7 @@ const moveSessionTreeTransaction = async (
}
useSessionUIStore.getState().setWorktreeMetadata(input.root.id, getLatestWorktreeMetadata(destination.metadata));
try {
await refreshGlobalSessionsForDirectories([input.sourceDirectory, destination.directory]);
} catch (error) {
// Direct action updates already reconciled both stores. Keep the move
// successful if this best-effort authoritative refresh is unavailable.
console.warn('[session-worktree-move] Failed to refresh moved sessions', error);
}
await refreshMovedDirectories(input.sourceDirectory, destination.directory);
return destination.directory;
} finally {
setSessionMovePending(input.root.id, false);
@@ -315,11 +381,27 @@ const moveSessionTreeToQuickWorktree = async (input: {
return {
directory: worktree.path,
metadata: worktree,
onMoveFailure: (error) => removeFailedWorktree(project, worktree, error),
// removeFailedWorktree force-deletes the worktree and its branch. The
// transaction skips this callback when the change transfer's outcome is
// unknown, so the worktree survives whenever it may hold the only copy.
onMoveFailure: async (error) => removeFailedWorktree(project, worktree, error),
};
});
};
const describeMoveFailure = (
messages: SessionTreeMoveMessages,
failure: Error,
moveChanges: boolean,
): string => {
if (failure instanceof ChangesMayBeInDestinationError) return messages.changesMayBeInDestination;
if (failure instanceof IncompleteRollbackError && failure.changesMayBeInDestination) {
return `${failure.message} ${messages.changesMayBeInDestination}`;
}
if (moveChanges && isApplyChangesError(failure)) return messages.applyChangesFailed;
return failure.message;
};
const executeSessionTreeMove = (intent: SessionTreeMoveIntent, moveChanges: boolean): void => {
const movePromise = intent.kind === 'existing'
? moveSessionTreeToExistingWorktree({
@@ -341,9 +423,7 @@ const executeSessionTreeMove = (intent: SessionTreeMoveIntent, moveChanges: bool
.catch((error) => {
const failure = error instanceof Error ? error : new Error(String(error));
toast.error(intent.messages.failure, {
description: moveChanges && isApplyChangesError(failure)
? intent.messages.applyChangesFailed
: failure.message,
description: describeMoveFailure(intent.messages, failure, moveChanges),
});
});
};