Skip to content

Commit f657115

Browse files
kbwoclaude
kbwo
and
claude
authored
fix: propagate AmbiguousBranchError as a typed Effect error (#306)
* fix: propagate AmbiguousBranchError as a typed Effect error Calling resolveBranchReference() directly inside Effect.gen caused AmbiguousBranchError to escape as an unhandled Die (FiberFailure) instead of a typed Effect error, crashing the TUI when a branch existed in multiple remotes (e.g. origin and a fork). - Add _tag discriminant to AmbiguousBranchError for pattern matching - Add resolveBranchReferenceEffect() wrapping the sync helper in Effect.try - Replace bare sync calls in createWorktreeEffect with yield* calls - Update IWorktreeService and createWorktreeEffect signature to include AmbiguousBranchError in the error union - Handle AmbiguousBranchError via _tag check in App.tsx before falling through to formatPreCreationHookError; remove string-parsing fallback - Replace inline dynamic imports in IWorktreeService with top-level imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: add tests for resolveBranchReferenceEffect and AmbiguousBranchError propagation - resolveBranchReferenceEffect: succeeds with remote ref when single remote has the branch - resolveBranchReferenceEffect: fails with AmbiguousBranchError as Left (not Die) when multiple remotes have the branch - createWorktreeEffect: returns Left with AmbiguousBranchError when branch exists in multiple remotes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: skip branch re-resolution when baseBranch is already a remote ref After the user selects a remote from the disambiguation UI, handleRemoteBranchSelected passes the selected ref (e.g. "origin/feature/feed-mention") as baseBranch. The previous code still called resolveBranchReferenceEffect(branch) first, which triggered AmbiguousBranchError again before baseBranch was ever consulted — making it impossible to successfully create the worktree. When baseBranch ends with "/{branch}", it is already the resolved remote-tracking ref the user chose. Use it directly as startPoint and skip re-resolution to break the loop. Also tighten the catch handler in resolveBranchReferenceEffect to re-throw unexpected non-AmbiguousBranchError errors as defects instead of silently mislabeling them. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c8c0a71 commit f657115

4 files changed

Lines changed: 231 additions & 73 deletions

File tree

‎src/components/App.tsx‎

Lines changed: 27 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import {
2929
GitProject,
3030
MenuAction,
3131
AmbiguousBranchError,
32-
RemoteBranchMatch,
3332
} from '../types/index.js';
3433
import {type AppError, type ProcessError} from '../types/errors.js';
3534
import {configReader} from '../services/config/configReader.js';
@@ -380,37 +379,6 @@ const App: React.FC<AppProps> = ({
380379
};
381380
}, [view, pendingMenuSessionLaunch, startSessionForWorktree]);
382381

383-
// Helper function to parse ambiguous branch error and create AmbiguousBranchError
384-
const parseAmbiguousBranchError = (
385-
errorMessage: string,
386-
): AmbiguousBranchError | null => {
387-
const pattern =
388-
/Ambiguous branch '(.+?)' found in multiple remotes: (.+?)\. Please specify which remote to use\./;
389-
const match = errorMessage.match(pattern);
390-
391-
if (!match) {
392-
return null;
393-
}
394-
395-
const branchName = match[1]!;
396-
const remoteRefsText = match[2]!;
397-
const remoteRefs = remoteRefsText.split(', ');
398-
399-
// Parse remote refs into RemoteBranchMatch objects
400-
const matches: RemoteBranchMatch[] = remoteRefs.map(fullRef => {
401-
const parts = fullRef.split('/');
402-
const remote = parts[0]!;
403-
const branch = parts.slice(1).join('/');
404-
return {
405-
remote,
406-
branch,
407-
fullRef,
408-
};
409-
});
410-
411-
return new AmbiguousBranchError(branchName, matches);
412-
};
413-
414382
// Helper function to handle worktree creation results
415383
const handleWorktreeCreationResult = (
416384
result: {success: boolean; error?: string; warning?: string},
@@ -451,21 +419,8 @@ const App: React.FC<AppProps> = ({
451419
return;
452420
}
453421

454-
const errorMessage = result.error || 'Failed to create worktree';
455-
const ambiguousError = parseAmbiguousBranchError(errorMessage);
456-
457-
if (ambiguousError) {
458-
// Handle ambiguous branch error
459-
setPendingWorktreeCreation({
460-
...creationData,
461-
ambiguousError,
462-
});
463-
navigateWithClear('remote-branch-selector');
464-
} else {
465-
// Handle regular error
466-
setError(errorMessage);
467-
setView('new-worktree');
468-
}
422+
setError(result.error || 'Failed to create worktree');
423+
setView('new-worktree');
469424
};
470425

471426
const handleMenuAction = async (action: MenuAction) => {
@@ -643,9 +598,26 @@ const App: React.FC<AppProps> = ({
643598
),
644599
);
645600

646-
// Transform Effect result to legacy format for handleWorktreeCreationResult
647601
if (result._tag === 'Left') {
648-
// Handle error using pattern matching on _tag
602+
if (result.left._tag === 'AmbiguousBranchError') {
603+
setPendingWorktreeCreation({
604+
path: targetPath,
605+
branch,
606+
baseBranch: request.baseBranch,
607+
copySessionData: request.copySessionData,
608+
copyClaudeDirectory: request.copyClaudeDirectory,
609+
presetId:
610+
request.creationMode === 'prompt' ? request.presetId : undefined,
611+
initialPrompt:
612+
request.creationMode === 'prompt'
613+
? request.initialPrompt
614+
: undefined,
615+
ambiguousError: result.left,
616+
});
617+
navigateWithClear('remote-branch-selector');
618+
return;
619+
}
620+
649621
const errorMessage = formatPreCreationHookError(result.left);
650622
if (result.left._tag === 'ProcessError') {
651623
setError(null);
@@ -733,7 +705,12 @@ const App: React.FC<AppProps> = ({
733705
);
734706

735707
if (result._tag === 'Left') {
736-
// Handle error using pattern matching on _tag
708+
if (result.left._tag === 'AmbiguousBranchError') {
709+
setError(result.left.message);
710+
setView('new-worktree');
711+
return;
712+
}
713+
737714
const errorMessage = formatPreCreationHookError(result.left);
738715
if (result.left._tag === 'ProcessError') {
739716
setError(null);

‎src/services/worktreeService.test.ts‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {existsSync, statSync, Stats} from 'fs';
55
import {configReader} from './config/configReader.js';
66
import {Effect} from 'effect';
77
import {GitError, ProcessError} from '../types/errors.js';
8+
import {AmbiguousBranchError} from '../types/index.js';
89

910
// Mock child_process module
1011
vi.mock('child_process');
@@ -477,6 +478,93 @@ origin/feature/test
477478
});
478479
});
479480

481+
describe('resolveBranchReferenceEffect', () => {
482+
it('should succeed with remote ref when single remote has the branch', async () => {
483+
mockedExecSync.mockImplementation((cmd, _options) => {
484+
if (typeof cmd === 'string') {
485+
if (
486+
cmd.includes('show-ref --verify --quiet refs/heads/foo/bar-xyz')
487+
) {
488+
throw new Error('Local branch not found');
489+
}
490+
if (cmd === 'git remote') {
491+
return 'origin\nupstream\n';
492+
}
493+
if (
494+
cmd.includes(
495+
'show-ref --verify --quiet refs/remotes/origin/foo/bar-xyz',
496+
)
497+
) {
498+
return '';
499+
}
500+
if (
501+
cmd.includes(
502+
'show-ref --verify --quiet refs/remotes/upstream/foo/bar-xyz',
503+
)
504+
) {
505+
throw new Error('Remote branch not found in upstream');
506+
}
507+
}
508+
throw new Error('Command not mocked: ' + cmd);
509+
});
510+
511+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
512+
const effect = (service as any).resolveBranchReferenceEffect(
513+
'foo/bar-xyz',
514+
);
515+
const result = await Effect.runPromise(Effect.either(effect));
516+
517+
expect(result._tag).toBe('Right');
518+
if (result._tag === 'Right') {
519+
expect(result.right).toBe('origin/foo/bar-xyz');
520+
}
521+
});
522+
523+
it('should fail with AmbiguousBranchError (not Die) when multiple remotes have the branch', async () => {
524+
mockedExecSync.mockImplementation((cmd, _options) => {
525+
if (typeof cmd === 'string') {
526+
if (
527+
cmd.includes('show-ref --verify --quiet refs/heads/foo/bar-xyz')
528+
) {
529+
throw new Error('Local branch not found');
530+
}
531+
if (cmd === 'git remote') {
532+
return 'origin\nupstream\n';
533+
}
534+
if (
535+
cmd.includes(
536+
'show-ref --verify --quiet refs/remotes/origin/foo/bar-xyz',
537+
) ||
538+
cmd.includes(
539+
'show-ref --verify --quiet refs/remotes/upstream/foo/bar-xyz',
540+
)
541+
) {
542+
return '';
543+
}
544+
}
545+
throw new Error('Command not mocked: ' + cmd);
546+
});
547+
548+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
549+
const effect = (service as any).resolveBranchReferenceEffect(
550+
'foo/bar-xyz',
551+
);
552+
const result = await Effect.runPromise(Effect.either(effect));
553+
554+
expect(result._tag).toBe('Left');
555+
if (result._tag === 'Left') {
556+
expect(result.left).toBeInstanceOf(AmbiguousBranchError);
557+
expect((result.left as AmbiguousBranchError)._tag).toBe(
558+
'AmbiguousBranchError',
559+
);
560+
expect((result.left as AmbiguousBranchError).branchName).toBe(
561+
'foo/bar-xyz',
562+
);
563+
expect((result.left as AmbiguousBranchError).matches).toHaveLength(2);
564+
}
565+
});
566+
});
567+
480568
describe('hasClaudeDirectoryInBranchEffect', () => {
481569
it('should return Effect with true when .claude directory exists in branch worktree', async () => {
482570
mockedExecSync.mockImplementation((cmd, _options) => {
@@ -876,6 +964,92 @@ branch refs/heads/feature
876964
expect(worktreeAddCmd).toContain('"origin/feature/remote-only"');
877965
});
878966

967+
it('should return Effect Left with AmbiguousBranchError when branch exists in multiple remotes', async () => {
968+
mockedExecSync.mockImplementation((cmd, _options) => {
969+
if (typeof cmd === 'string') {
970+
if (cmd === 'git rev-parse --git-common-dir') {
971+
return '/fake/path/.git\n';
972+
}
973+
if (cmd.includes('show-ref --verify --quiet refs/heads/')) {
974+
throw new Error('Branch not found');
975+
}
976+
if (cmd === 'git remote') {
977+
return 'origin\nkbwo-fork\n';
978+
}
979+
if (cmd.includes('show-ref --verify --quiet refs/remotes/')) {
980+
return ''; // Both remotes have the branch
981+
}
982+
}
983+
throw new Error('Command not mocked: ' + cmd);
984+
});
985+
986+
const effect = service.createWorktreeEffect(
987+
'/path/to/worktree',
988+
'feature/feed-mention',
989+
'main',
990+
);
991+
const result = await Effect.runPromise(Effect.either(effect));
992+
993+
expect(result._tag).toBe('Left');
994+
if (result._tag === 'Left') {
995+
expect(result.left).toBeInstanceOf(AmbiguousBranchError);
996+
expect((result.left as AmbiguousBranchError)._tag).toBe(
997+
'AmbiguousBranchError',
998+
);
999+
expect((result.left as AmbiguousBranchError).branchName).toBe(
1000+
'feature/feed-mention',
1001+
);
1002+
expect((result.left as AmbiguousBranchError).matches).toHaveLength(2);
1003+
expect(
1004+
(result.left as AmbiguousBranchError).matches.map(m => m.remote),
1005+
).toEqual(['origin', 'kbwo-fork']);
1006+
}
1007+
});
1008+
1009+
it('should succeed when baseBranch is already a resolved remote ref for branch (retry after disambiguation)', async () => {
1010+
const executedCommands: string[] = [];
1011+
mockedExecSync.mockImplementation((cmd, _options) => {
1012+
if (typeof cmd === 'string') {
1013+
executedCommands.push(cmd);
1014+
if (cmd === 'git rev-parse --git-common-dir') {
1015+
return '/fake/path/.git\n';
1016+
}
1017+
// No local branch
1018+
if (cmd.includes('show-ref --verify --quiet refs/heads/')) {
1019+
throw new Error('Branch not found');
1020+
}
1021+
if (cmd.includes('git worktree add')) {
1022+
return '';
1023+
}
1024+
}
1025+
return '';
1026+
});
1027+
1028+
// Simulate the retry call: user selected "origin/feature/feed-mention",
1029+
// which is passed as baseBranch.
1030+
const effect = service.createWorktreeEffect(
1031+
'/path/to/worktree',
1032+
'feature/feed-mention',
1033+
'origin/feature/feed-mention',
1034+
);
1035+
const result = await Effect.runPromise(Effect.either(effect));
1036+
1037+
expect(result._tag).toBe('Right');
1038+
1039+
// Should use baseBranch directly as startPoint without calling show-ref
1040+
const worktreeAddCmd = executedCommands.find(c =>
1041+
c.includes('git worktree add'),
1042+
);
1043+
expect(worktreeAddCmd).toContain('-b "feature/feed-mention"');
1044+
expect(worktreeAddCmd).toContain('"origin/feature/feed-mention"');
1045+
// show-ref for remotes must NOT be called (no re-resolution)
1046+
expect(
1047+
executedCommands.some(
1048+
c => c.includes('show-ref') && c.includes('refs/remotes/'),
1049+
),
1050+
).toBe(false);
1051+
});
1052+
8791053
it('should return Effect that fails with GitError on git command failure', async () => {
8801054
mockedExecSync.mockImplementation((cmd, _options) => {
8811055
if (typeof cmd === 'string') {

‎src/services/worktreeService.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,20 @@ export class WorktreeService {
185185
}
186186
}
187187

188+
private resolveBranchReferenceEffect(
189+
branchName: string,
190+
): Effect.Effect<string, AmbiguousBranchError> {
191+
return Effect.try({
192+
try: () => this.resolveBranchReference(branchName),
193+
catch: error => {
194+
if (error instanceof AmbiguousBranchError) return error;
195+
// resolveBranchReference only re-throws AmbiguousBranchError; all
196+
// other errors are swallowed internally. This path is unreachable.
197+
throw error;
198+
},
199+
});
200+
}
201+
188202
/**
189203
* SYNCHRONOUS HELPER: Gets all git remotes for this repository.
190204
*
@@ -895,7 +909,7 @@ export class WorktreeService {
895909
copyClaudeDirectory = false,
896910
): Effect.Effect<
897911
CreateWorktreeResult,
898-
GitError | FileSystemError | ProcessError,
912+
GitError | FileSystemError | ProcessError | AmbiguousBranchError,
899913
never
900914
> {
901915
// eslint-disable-next-line @typescript-eslint/no-this-alias
@@ -980,12 +994,17 @@ export class WorktreeService {
980994
let command: string;
981995
if (localBranchExists) {
982996
command = `git worktree add "${resolvedPath}" "${branch}"`;
997+
} else if (baseBranch.endsWith(`/${branch}`)) {
998+
// baseBranch is already a remote-tracking ref for branch
999+
// (e.g. "origin/feature/x" after the user resolved an ambiguity).
1000+
// Use it directly to avoid re-triggering AmbiguousBranchError.
1001+
command = `git worktree add -b "${branch}" "${resolvedPath}" "${baseBranch}"`;
9831002
} else {
984-
const resolvedRef = self.resolveBranchReference(branch);
1003+
const resolvedRef = yield* self.resolveBranchReferenceEffect(branch);
9851004
const isRemoteBranch = resolvedRef !== branch;
9861005
const startPoint = isRemoteBranch
9871006
? resolvedRef
988-
: self.resolveBranchReference(baseBranch);
1007+
: yield* self.resolveBranchReferenceEffect(baseBranch);
9891008
command = `git worktree add -b "${branch}" "${resolvedPath}" "${startPoint}"`;
9901009
}
9911010

0 commit comments

Comments
 (0)