Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion apps/server/src/orchestration-v2/ThreadLaunchService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ function makeHarness(options: HarnessOptions = {}) {
(_input: Parameters<GitWorkflow.GitWorkflowService["Service"]["removeWorktree"]>[0]) =>
Effect.void,
);
const deleteLocalBranch = vi.fn(
(_input: Parameters<GitWorkflow.GitWorkflowService["Service"]["deleteLocalBranch"]>[0]) =>
Effect.void,
);
const runSetup = vi.fn(
options.runSetup ?? (() => Effect.succeed({ status: "no-script" as const })),
);
Expand Down Expand Up @@ -208,6 +212,7 @@ function makeHarness(options: HarnessOptions = {}) {
remoteExists: options.remoteExists ?? (() => Effect.succeed(true)),
remoteBranchExists: () => Effect.succeed(true),
removeWorktree,
deleteLocalBranch,
...options.git,
}),
Layer.succeed(ProjectSetupScriptRunner.ProjectSetupScriptRunner, {
Expand Down Expand Up @@ -276,6 +281,7 @@ function makeHarness(options: HarnessOptions = {}) {
),
createWorktree,
removeWorktree,
deleteLocalBranch,
renameBranch,
generateBranchName,
generateThreadTitle,
Expand Down Expand Up @@ -2188,8 +2194,15 @@ it.effect("shared intake preserves durable attachment bytes after a lost launch
it.effect("cancels tracked setup before provider work is released", () =>
Effect.gen(function* () {
const entered = yield* Deferred.make<void>();
const renamed = yield* Deferred.make<void>();
const harness = makeHarness({
runSetup: () => Deferred.succeed(entered, undefined).pipe(Effect.andThen(Effect.never)),
renameBranch: (input) =>
Deferred.succeed(renamed, undefined).pipe(Effect.as({ branch: input.newBranch })),
runSetup: () =>
Deferred.await(renamed).pipe(
Effect.andThen(Deferred.succeed(entered, undefined)),
Effect.andThen(Effect.never),
),
});
yield* Effect.gen(function* () {
const launches = yield* ThreadLaunch.ThreadLaunchService;
Expand All @@ -2210,11 +2223,50 @@ it.effect("cancels tracked setup before provider work is released", () =>
const projection = yield* threads.getThreadProjection(launched.threadId);
assert.equal(projection.runs[0]?.status, "failed");
assert.isNull(projection.thread.worktreePath);
assert.deepEqual(harness.deleteLocalBranch.mock.calls[0]?.[0], {
cwd: "/repo",
refName: "generated-branch",
force: true,
});
assert.isEmpty(yield* outbox.listByCommandId(CommandId.make(`${input.commandId}:release`)));
}).pipe(Effect.provide(harness.layer));
}),
);

it.effect(
"deletes a claimed worktree's branch when setup is cancelled before checkout returns",
() =>
Effect.gen(function* () {
const claimed = yield* Deferred.make<void>();
const harness = makeHarness({
createWorktree: (_input, options) =>
(options?.progress?.onWorktreeClaimed?.("/repo-worktrees/feature") ?? Effect.void).pipe(
Effect.andThen(Deferred.succeed(claimed, undefined)),
Effect.andThen(Effect.never),
),
});
yield* Effect.gen(function* () {
const launches = yield* ThreadLaunch.ThreadLaunchService;
const tracker = yield* WorktreeSetupTracker.WorktreeSetupTracker;
const launched = yield* launches.launch(
launchInput({
command: "launch:cancel-claimed",
thread: "thread:cancel-claimed",
message: "Start",
workspace: { type: "worktree", baseRef: "main" },
}),
);
yield* Deferred.await(claimed);
assert.isTrue(yield* tracker.cancel(launched.threadId));
assert.equal(harness.removeWorktree.mock.calls[0]?.[0].path, "/repo-worktrees/feature");
assert.equal(
harness.deleteLocalBranch.mock.calls[0]?.[0].refName,
harness.createWorktree.mock.calls[0]?.[0].newRefName,
);
}).pipe(Effect.provide(harness.layer));
}),
);

it.effect("stops an in-flight branch rename when tracked setup is cancelled", () =>
Effect.gen(function* () {
const setupEntered = yield* Deferred.make<void>();
Expand Down Expand Up @@ -2259,6 +2311,10 @@ it.effect("stops an in-flight branch rename when tracked setup is cancelled", ()
);
assert.equal(harness.renameBranch.mock.calls.length, 0);
assert.isNull((yield* threads.getThreadProjection(launched.threadId)).thread.worktreePath);
assert.equal(
harness.deleteLocalBranch.mock.calls[0]?.[0].refName,
harness.createWorktree.mock.calls[0]?.[0].newRefName,
);
}).pipe(Effect.provide(harness.layer));
}),
);
Expand Down
37 changes: 31 additions & 6 deletions apps/server/src/orchestration-v2/ThreadLaunchService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ const make = Effect.gen(function* () {
const reused = input.reusedWorktree;
const tracked = input.workspaceStrategy.type === "worktree" || reused !== undefined;
let createdWorktreePath: string | null = null;
let createdBranch: string | null = null;
let setupTerminalId: string | null = null;
let workspaceRecorded = false;
let runReleased = false;
Expand Down Expand Up @@ -448,6 +449,7 @@ const make = Effect.gen(function* () {
onWorktreeClaimed: (path) =>
Effect.sync(() => {
createdWorktreePath = path;
createdBranch = branch;
Comment thread
saphid marked this conversation as resolved.
Resolved
}),
onCheckoutProgress: (progress) =>
setupTracker.stage(threadId, "checkout", { percent: progress.percent }),
Expand All @@ -458,6 +460,7 @@ const make = Effect.gen(function* () {
worktreePath = worktree.worktree.path;
branch = worktree.worktree.refName;
createdWorktreePath = worktreePath;
createdBranch = branch;
yield* setupTracker.update(threadId, (snapshot) => ({ ...snapshot, worktreePath, branch }));
yield* setupTracker.stageStatus(threadId, "checkout", "done");
}
Expand Down Expand Up @@ -487,12 +490,22 @@ const make = Effect.gen(function* () {
const worktreeCwd = worktreePath;
branchRename = yield* generateBranchNameFor(worktreeCwd, initialMessage).pipe(
Effect.flatMap(({ branch: newBranch, exactName }) =>
git.renameBranch({
cwd: worktreeCwd,
oldBranch,
newBranch,
...(exactName ? { exactName: true } : {}),
}),
git
.renameBranch({
cwd: worktreeCwd,
oldBranch,
newBranch,
...(exactName ? { exactName: true } : {}),
})
.pipe(
// A cancel must see the renamed branch, or its cleanup deletes the old name.
Effect.tap((renamed) =>
Effect.sync(() => {
createdBranch = renamed.branch;
}),
),
Effect.uninterruptible,
),
),
Effect.flatMap((renamed) =>
Effect.gen(function* () {
Expand Down Expand Up @@ -680,10 +693,22 @@ const make = Effect.gen(function* () {
) {
if (branchRename !== null) yield* Fiber.interrupt(branchRename);
const removedPath = createdWorktreePath;
const removedBranch = createdBranch;
// Forget worktrees only after removal succeeds; failed removals must not reuse partially written checkouts.
yield* git
.removeWorktree({ cwd: project.workspaceRoot, path: removedPath, force: true })
.pipe(
Effect.andThen(
removedBranch === null
? Effect.void
: git
.deleteLocalBranch({
cwd: project.workspaceRoot,
refName: removedBranch,
force: true,
})
.pipe(Effect.ignoreCause({ log: true })),
),
Effect.andThen(
threads
.dispatch({
Expand Down
Loading