fix(worktree): address PR review — test assertion, dry-run sentinel, git timeouts
- Update allProjects test expectation to match [parent, composite] (matches JSDoc + callers in ContextBuilder/context handlers). - Replace string-matched __DRY_RUN_ROLLBACK__ sentinel with dedicated DryRunRollback class to avoid swallowing unrelated errors. - Add 5000ms timeout to spawnSync git calls in WorktreeAdoption and ProcessManager so worker startup can't hang on a stuck git process. - Drop unreachable break after process.exit(0) in adopt case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+118
-118
File diff suppressed because one or more lines are too long
@@ -685,7 +685,10 @@ type CwdClassification =
|
|||||||
| { kind: 'skip' };
|
| { kind: 'skip' };
|
||||||
|
|
||||||
function gitQuery(cwd: string, args: string[]): string | null {
|
function gitQuery(cwd: string, args: string[]): string | null {
|
||||||
const r = spawnSync('git', ['-C', cwd, ...args], { encoding: 'utf8' });
|
const r = spawnSync('git', ['-C', cwd, ...args], {
|
||||||
|
encoding: 'utf8',
|
||||||
|
timeout: 5000
|
||||||
|
});
|
||||||
if (r.status !== 0) return null;
|
if (r.status !== 0) return null;
|
||||||
return (r.stdout ?? '').trim();
|
return (r.stdout ?? '').trim();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -48,8 +48,20 @@ interface WorktreeEntry {
|
|||||||
branch: string | null;
|
branch: string | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const GIT_TIMEOUT_MS = 5000;
|
||||||
|
|
||||||
|
class DryRunRollback extends Error {
|
||||||
|
constructor() {
|
||||||
|
super('dry-run rollback');
|
||||||
|
this.name = 'DryRunRollback';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function gitCapture(cwd: string, args: string[]): string | null {
|
function gitCapture(cwd: string, args: string[]): string | null {
|
||||||
const r = spawnSync('git', ['-C', cwd, ...args], { encoding: 'utf8' });
|
const r = spawnSync('git', ['-C', cwd, ...args], {
|
||||||
|
encoding: 'utf8',
|
||||||
|
timeout: GIT_TIMEOUT_MS
|
||||||
|
});
|
||||||
if (r.status !== 0) return null;
|
if (r.status !== 0) return null;
|
||||||
return (r.stdout ?? '').trim();
|
return (r.stdout ?? '').trim();
|
||||||
}
|
}
|
||||||
@@ -221,16 +233,15 @@ export async function adoptMergedWorktrees(opts: {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (dryRun) {
|
if (dryRun) {
|
||||||
// Throw to force rollback. Sentinel caught below.
|
// Throw a dedicated error to force rollback. Caught below by instanceof check.
|
||||||
throw new Error('__DRY_RUN_ROLLBACK__');
|
throw new DryRunRollback();
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
try {
|
try {
|
||||||
tx();
|
tx();
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const message = err instanceof Error ? err.message : String(err);
|
if (err instanceof DryRunRollback) {
|
||||||
if (message === '__DRY_RUN_ROLLBACK__') {
|
|
||||||
// Rolled back as intended for dry-run — counts are still useful.
|
// Rolled back as intended for dry-run — counts are still useful.
|
||||||
} else {
|
} else {
|
||||||
throw err;
|
throw err;
|
||||||
|
|||||||
@@ -1235,7 +1235,6 @@ async function main() {
|
|||||||
console.log(` ! ${err.worktree}: ${err.error}`);
|
console.log(` ! ${err.worktree}: ${err.error}`);
|
||||||
}
|
}
|
||||||
process.exit(0);
|
process.exit(0);
|
||||||
break;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
case '--daemon':
|
case '--daemon':
|
||||||
|
|||||||
@@ -130,7 +130,7 @@ describe('getProjectContext', () => {
|
|||||||
expect(ctx.isWorktree).toBe(true);
|
expect(ctx.isWorktree).toBe(true);
|
||||||
expect(ctx.primary).toBe('main-repo/my-worktree');
|
expect(ctx.primary).toBe('main-repo/my-worktree');
|
||||||
expect(ctx.parent).toBe('main-repo');
|
expect(ctx.parent).toBe('main-repo');
|
||||||
expect(ctx.allProjects).toEqual(['main-repo/my-worktree']);
|
expect(ctx.allProjects).toEqual(['main-repo', 'main-repo/my-worktree']);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('write-path call sites resolve to composite name in worktrees', () => {
|
it('write-path call sites resolve to composite name in worktrees', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user