Skip to content
50 changes: 32 additions & 18 deletions apps/server/src/git/GitWorkflowService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,36 +25,50 @@ function layer(input: {
);
}

const jjHandle = (rootPath: string) => ({
kind: "jj" as const,
repository: {
kind: "jj" as const,
rootPath,
metadataPath: `${rootPath}/.jj`,
freshness: {
source: "live-local" as const,
observedAt: DateTime.makeUnsafe("2026-01-01T00:00:00.000Z"),
expiresAt: Option.none(),
},
},
driver: {} as VcsDriverRegistry.VcsDriverHandle["driver"],
});

describe("GitWorkflowService", () => {
it.effect("reports a non-Git VCS repository as not a Git repository", () =>
it.effect("distinguishes no VCS from a non-Git repository", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
const isRepository = yield* workflow.isRepository("/jj-repo");
// A plain folder is the Scratch case callers degrade a worktree request for.
assert.isFalse(yield* workflow.isGitRepository("/folder"));

assert.equal(isRepository, false);
// A jj repository is not a folder, so this must fail rather than answer
// false and let a caller share the project checkout.
const failure = yield* Effect.flip(workflow.isGitRepository("/jj-repo"));
assert.include(failure.detail, "detected jj");
}).pipe(
Effect.provide(
layer({
detect: () =>
Effect.succeed({
kind: "jj",
repository: {
kind: "jj",
rootPath: "/jj-repo",
metadataPath: "/jj-repo/.jj",
freshness: {
source: "live-local",
observedAt: DateTime.makeUnsafe("2026-01-01T00:00:00.000Z"),
expiresAt: Option.none(),
},
},
driver: {} as VcsDriverRegistry.VcsDriverHandle["driver"],
}),
detect: (input) => Effect.succeed(input.cwd === "/folder" ? null : jjHandle("/jj-repo")),
}),
),
),
);

it.effect("reports a non-Git VCS repository as not a Git repository", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
const isRepository = yield* workflow.isRepository("/jj-repo");

assert.equal(isRepository, false);
}).pipe(Effect.provide(layer({ detect: () => Effect.succeed(jjHandle("/jj-repo")) }))),
);

it.effect("returns an empty local status when no VCS repository is detected", () =>
Effect.gen(function* () {
const workflow = yield* GitWorkflowService.GitWorkflowService;
Expand Down
7 changes: 7 additions & 0 deletions apps/server/src/git/GitWorkflowService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ export class GitWorkflowService extends Context.Service<
GitWorkflowService,
{
readonly isRepository: (cwd: string) => Effect.Effect<boolean, GitManagerServiceError>;
/**
* False only when no VCS repository backs this cwd at all; a non-Git repository
* (jj) fails, so a caller never mistakes another VCS for a plain folder.
*/
readonly isGitRepository: (cwd: string) => Effect.Effect<boolean, GitCommandError>;
readonly hasCommit: (input: {
readonly cwd: string;
readonly refName: string;
Expand Down Expand Up @@ -288,6 +293,8 @@ export const make = Effect.gen(function* () {
}),
),
),
isGitRepository: (cwd) =>
detectGitRepositoryForCommand("GitWorkflowService.isGitRepository", cwd),
hasCommit: (input) =>
ensureGitCommand("GitWorkflowService.hasCommit", input.cwd).pipe(
Effect.andThen(
Expand Down
152 changes: 152 additions & 0 deletions apps/server/src/orchestration-v2/ThreadLaunchService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ interface HarnessOptions {
readonly createWorktree?: GitWorkflow.GitWorkflowService["Service"]["createWorktree"];
readonly fetchRemote?: GitWorkflow.GitWorkflowService["Service"]["fetchRemote"];
readonly hasCommit?: GitWorkflow.GitWorkflowService["Service"]["hasCommit"];
readonly isGitRepository?: GitWorkflow.GitWorkflowService["Service"]["isGitRepository"];
readonly renameBranch?: GitWorkflow.GitWorkflowService["Service"]["renameBranch"];
readonly runSetup?: ProjectSetupScriptRunner.ProjectSetupScriptRunner["Service"]["runForThread"];
readonly generateTitle?: TextGeneration.TextGeneration["Service"]["generateThreadTitle"];
Expand Down Expand Up @@ -170,6 +171,7 @@ function makeHarness(options: HarnessOptions = {}) {
renameBranch,
fetchRemote: options.fetchRemote ?? (() => Effect.void),
hasCommit: options.hasCommit ?? (() => Effect.succeed(false)),
isGitRepository: options.isGitRepository ?? (() => Effect.succeed(true)),
remoteExists: () => Effect.succeed(true),
remoteBranchExists: () => Effect.succeed(true),
removeWorktree,
Expand Down Expand Up @@ -245,6 +247,19 @@ function makeHarness(options: HarnessOptions = {}) {
};
}

/** A Scratch-style project whose threads each get `folderPath`, recorded in `claims`. */
function scratchFolders(folderPath: string, claims: Array<string>) {
return Layer.mock(ManagedProjectFolders.ManagedProjectFolders)({
namedProjectsRoot: "/projects",
folderForThread: (input) =>
Effect.sync(() => {
if (input.projectId !== projectId) return Option.none();
claims.push(input.threadId);
return Option.some(folderPath);
}),
});
}

function launchInput(input: {
readonly command: string;
readonly thread: string;
Expand Down Expand Up @@ -1104,6 +1119,143 @@ it.effect("runs a Scratch thread launched at the root in its own folder", () =>
}),
);

it.effect("runs a worktree request against a non-repository project as a root launch", () =>
Effect.gen(function* () {
// The first detection answers "no VCS here", and every later one fails. A
// replay whose run already recorded the folder must not detect again.
let detections = 0;
const claims: Array<string> = [];
const harness = makeHarness({
isGitRepository: () =>
Effect.sync(() => ++detections).pipe(
Effect.flatMap((attempt) =>
attempt === 1
? Effect.succeed(false)
: Effect.fail(
new GitCommandError({
operation: "isGitRepository",
command: "git-rev-parse",
cwd: "/scratch",
detail: "Git is unreachable right now.",
}),
),
),
),
// Only `projectId` stands in for the Scratch project here.
managedFolders: scratchFolders("/scratch/folder-worktree", claims),
});
yield* Effect.gen(function* () {
const launches = yield* ThreadLaunch.ThreadLaunchService;
const input = launchInput({
command: "command:launch:scratch-worktree",
thread: "thread:launch:scratch-worktree",
message: "Review this",
workspace: { type: "worktree", baseRef: "main", startFromOrigin: true },
});
const launched = yield* launches.launch(input);
assert.equal(launched.projection.thread.worktreePath, "/scratch/folder-worktree");
assert.isNull(launched.projection.thread.branch);
yield* waitUntil(() => Effect.sync(() => harness.runSetup.mock.calls.length === 1));
assert.equal(harness.runSetup.mock.calls[0]?.[0]?.worktreePath, "/scratch/folder-worktree");
assert.equal(harness.createWorktree.mock.calls.length, 0);

// A replay reuses the folder the accepted run recorded, without detecting.
const replayed = yield* launches.launch(input);
assert.isTrue(replayed.resumed);
assert.equal(detections, 1);
assert.deepEqual(claims, [launched.threadId]);
assert.equal(replayed.projection.thread.worktreePath, "/scratch/folder-worktree");
}).pipe(Effect.provide(harness.layer));
}),
);

it.effect("keeps the claimed folder when a retried launch never dispatched its message", () =>
Effect.gen(function* () {
// A create that was accepted with no message behind it, which is what a failed
// message dispatch leaves behind. The thread owns the folder that create
// claimed, so the retry prepares it rather than the shared project root.
const claims: Array<string> = [];
const harness = makeHarness({
isGitRepository: () => Effect.succeed(false),
managedFolders: scratchFolders("/scratch/folder-partial", claims),
});
yield* Effect.gen(function* () {
const launches = yield* ThreadLaunch.ThreadLaunchService;
const threads = yield* ThreadManagement.ThreadManagementService;
const input = launchInput({
command: "command:launch:scratch-partial",
thread: "thread:launch:scratch-partial",
message: "Review this",
workspace: { type: "worktree", baseRef: "main" },
});
yield* threads.dispatch({
type: "thread.create",
commandId: input.commandId,
threadId: input.threadId,
projectId,
title: input.title,
modelSelection,
runtimeMode: input.runtimeMode,
interactionMode: input.interactionMode,
branch: null,
worktreePath: "/scratch/folder-partial",
createdBy: "user",
creationSource: "web",
});
assert.isEmpty(claims);

const retried = yield* launches.launch(input);
assert.isTrue(retried.resumed);
// No second folder is claimed; the bound one is prepared.
assert.isEmpty(claims);
assert.equal(retried.projection.thread.worktreePath, "/scratch/folder-partial");
yield* waitUntil(() => Effect.sync(() => harness.runSetup.mock.calls.length === 1));
assert.equal(harness.runSetup.mock.calls[0]?.[0]?.worktreePath, "/scratch/folder-partial");
assert.equal(harness.createWorktree.mock.calls.length, 0);
}).pipe(Effect.provide(harness.layer));
}),
);

it.effect(
"fails a worktree request against a non-Git repository instead of running it at the root",
() =>
Effect.gen(function* () {
// A jj project is not a folder. Degrading it to root would quietly share the
// project checkout, so the driver's unsupported-VCS error has to survive.
const harness = makeHarness({
isGitRepository: () =>
Effect.fail(
new GitCommandError({
operation: "isGitRepository",
command: "vcs-route",
cwd: "/jj-repo",
detail:
"The GitWorkflowService.isGitRepository command currently supports Git repositories only; detected jj.",
}),
),
});
yield* Effect.gen(function* () {
const launches = yield* ThreadLaunch.ThreadLaunchService;
const failure = yield* Effect.flip(
launches.launch(
launchInput({
command: "command:launch:jj-worktree",
thread: "thread:launch:jj-worktree",
message: "Build the feature",
workspace: { type: "worktree", baseRef: "main" },
}),
),
);
assert.equal(failure.operation, "provision-worktree");
assert.include(
failure.cause instanceof Error ? failure.cause.message : String(failure.cause),
"currently supports Git repositories only; detected jj",
);
assert.equal(harness.createWorktree.mock.calls.length, 0);
}).pipe(Effect.provide(harness.layer));
}),
);

it.effect("names the worktree itself when the client provides no branch", () =>
Effect.gen(function* () {
const harness = makeHarness();
Expand Down
45 changes: 36 additions & 9 deletions apps/server/src/orchestration-v2/ThreadLaunchService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -743,10 +743,33 @@ const make = Effect.gen(function* () {
yield* validateReusableThread(input, candidateThreadId);
}

// Derived from the command id, so a replay is known before anything else runs.
const messageCommandId = CommandId.make(`${input.commandId}:initial-message`);
const messageWasAlreadyAccepted =
input.initialMessage === undefined
? false
: Option.isSome(yield* readReceipt(input, messageCommandId));

// A worktree needs a Git repository. The Scratch project is a folder, so a
// worktree request there becomes a root launch, which the block below
// answers with a folder of its own. Resolved before the strategy reaches
// the run, so a retry replays what happened. Only "no VCS at all" degrades:
// a non-Git repository keeps its unsupported-VCS failure rather than
// silently sharing the project checkout. A replay detects nothing, since its
// run already recorded the answer.
const requestedStrategy: ThreadLaunchWorkspaceStrategy =
input.workspaceStrategy.type === "worktree" &&
!messageWasAlreadyAccepted &&
!(yield* git
.isGitRepository(project.workspaceRoot)
.pipe(Effect.mapError(mapError(input, "provision-worktree", candidateThreadId))))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
? { type: "root" }
: input.workspaceStrategy;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// A Scratch thread launched at the project root runs in a folder of its
// own. Only the first attempt claims one; a retry replays its create.
const workspaceStrategy: ThreadLaunchWorkspaceStrategy =
input.workspaceStrategy.type === "root" && Option.isNone(launchReceipt)
requestedStrategy.type === "root" && Option.isNone(launchReceipt)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
? Option.match(
yield* managedFolders
.folderForThread({
Expand All @@ -756,11 +779,11 @@ const make = Effect.gen(function* () {
})
.pipe(Effect.mapError(mapError(input, "provision-worktree", candidateThreadId))),
{
onNone: () => input.workspaceStrategy,
onNone: () => requestedStrategy,
onSome: (worktreePath) => ({ type: "existing_worktree", worktreePath }),
},
)
: input.workspaceStrategy;
: requestedStrategy;
const initialBranch = workspaceStrategy.branch ?? null;
const initialWorktreePath =
workspaceStrategy.type === "existing_worktree" ? workspaceStrategy.worktreePath : null;
Expand Down Expand Up @@ -806,11 +829,7 @@ const make = Effect.gen(function* () {
}

let runId: RunId | null = null;
let messageWasAlreadyAccepted = false;
if (input.initialMessage !== undefined) {
const messageCommandId = CommandId.make(`${input.commandId}:initial-message`);
const messageReceipt = yield* readReceipt(input, messageCommandId);
messageWasAlreadyAccepted = Option.isSome(messageReceipt);
const messageId =
input.initialMessage.messageId ??
(yield* ids.allocate
Expand Down Expand Up @@ -861,16 +880,24 @@ const make = Effect.gen(function* () {
// A retried root launch prepares the folder its first attempt bound, so
// a Scratch thread keeps its own. Other root launches bind no folder.
const boundWorktreePath = projection.thread.worktreePath;
// A replay prepares what its accepted run recorded. A run this attempt just
// created only echoes the strategy above, so reading it back would shadow
// the bound-folder recovery a partial launch still needs.
const recordedStrategy =
messageWasAlreadyAccepted && runId !== null
? projection.runs.find((run) => run.id === runId)?.workspacePreparation
: undefined;
const preparationStrategy: ThreadLaunchWorkspaceStrategy =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High orchestration-v2/ThreadLaunchService.ts:890

A replayed Scratch launch can run setup and the agent in the shared project root, losing its per-thread workspace isolation. When the accepted run records root, recordedStrategy takes precedence over the bound-folder recovery below; use the bound folder when the recorded strategy is root.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadLaunchService.ts around line 890:

A replayed Scratch launch can run setup and the agent in the shared project root, losing its per-thread workspace isolation. When the accepted run records `root`, `recordedStrategy` takes precedence over the bound-folder recovery below; use the bound folder when the recorded strategy is `root`.

Option.isSome(launchReceipt) &&
recordedStrategy ??
(Option.isSome(launchReceipt) &&
workspaceStrategy.type === "root" &&
boundWorktreePath !== null
? {
type: "existing_worktree",
worktreePath: boundWorktreePath,
branch: workspaceStrategy.branch,
}
: workspaceStrategy;
: workspaceStrategy);
if (shouldSchedule) {
const ownsPreparation = yield* reservePreparation(input.commandId);
if (ownsPreparation) {
Expand Down
Loading