Address current checkout thread reviews

Author: D3OXY

apps/server/src/mcp/toolkits/thread/handlers.test.ts

@@ -1,6 +1,7 @@
 import { expect, it } from "@effect/vitest";
 import {
   EnvironmentId,
+  GitCommandError,
   ProjectId,
   ProviderInstanceId,
   ThreadId,
@@ -93,7 +94,11 @@ const TestCryptoLive = Layer.sync(Crypto.Crypto, () => {
 
 const makeTestLayer = (
   commands: OrchestrationCommand[],
-  options: { readonly enqueueCalls?: number[] } = {},
+  options: {
+    readonly enqueueCalls?: number[];
+    readonly gitStatus?: GitWorkflowService["Service"]["status"];
+    readonly sourceThread?: OrchestrationThreadShell;
+  } = {},
 ) => {
   const bootstrapTurnStartDispatcherLayer = Layer.mock(
     BootstrapTurnStartDispatcher.BootstrapTurnStartDispatcher,
@@ -126,7 +131,7 @@ const makeTestLayer = (
     Layer.provide(
       Layer.mock(ProjectionSnapshotQuery)({
         getProjectShellById: () => Effect.succeed(Option.some(project)),
-        getThreadShellById: () => Effect.succeed(Option.some(sourceThread)),
+        getThreadShellById: () => Effect.succeed(Option.some(options.sourceThread ?? sourceThread)),
       }),
     ),
     Layer.provide(
@@ -147,24 +152,26 @@ const makeTestLayer = (
             nextCursor: null,
             totalCount: 1,
           }),
-        status: () =>
-          Effect.succeed({
-            isRepo: true,
-            hasPrimaryRemote: true,
-            isDefaultRef: false,
-            refName: "feature/source",
-            hasWorkingTreeChanges: false,
-            workingTree: {
-              files: [],
-              insertions: 0,
-              deletions: 0,
-            },
-            hasUpstream: true,
-            aheadCount: 0,
-            behindCount: 0,
-            aheadOfDefaultCount: 0,
-            pr: null,
-          }),
+        status:
+          options.gitStatus ??
+          (() =>
+            Effect.succeed({
+              isRepo: true,
+              hasPrimaryRemote: true,
+              isDefaultRef: false,
+              refName: "feature/source",
+              hasWorkingTreeChanges: false,
+              workingTree: {
+                files: [],
+                insertions: 0,
+                deletions: 0,
+              },
+              hasUpstream: true,
+              aheadCount: 0,
+              behindCount: 0,
+              aheadOfDefaultCount: 0,
+              pr: null,
+            })),
       }),
     ),
     Layer.provide(
@@ -190,7 +197,11 @@ const makeTestLayer = (
 const callStartTool = (
   arguments_: Record<string, unknown>,
   commands: OrchestrationCommand[],
-  options: { readonly enqueueCalls?: number[] } = {},
+  options: {
+    readonly enqueueCalls?: number[];
+    readonly gitStatus?: GitWorkflowService["Service"]["status"];
+    readonly sourceThread?: OrchestrationThreadShell;
+  } = {},
 ) =>
   Effect.gen(function* () {
     const server = yield* McpServer.McpServer;
@@ -272,23 +283,75 @@ it.effect("passes explicit setup script requests for existing worktrees", () =>
 it.effect("starts current-checkout threads with warning metadata", () =>
   Effect.gen(function* () {
     const commands: OrchestrationCommand[] = [];
+    const sourceWorktreePath = "/repo/.worktrees/source";
     const result = yield* callStartTool(
-      { prompt: "Read current diff", mode: "current_checkout" },
+      { prompt: "Read current diff", mode: "current_checkout", runSetupScript: true },
       commands,
+      { sourceThread: { ...sourceThread, worktreePath: sourceWorktreePath } },
     );
 
     expect(result.isError).toBe(false);
     expect(result.structuredContent).toMatchObject({
       projectId,
       mode: "current_checkout",
       branch: "feature/source",
-      worktreePath: null,
+      worktreePath: sourceWorktreePath,
     });
     expect(result.structuredContent).toHaveProperty("warning");
     const command = commands[0];
     expect(command?.type).toBe("thread.turn.start");
     if (command?.type !== "thread.turn.start") return;
     expect(command.bootstrap?.prepareWorktree).toBeUndefined();
-    expect(command.bootstrap?.createThread?.worktreePath).toBeNull();
+    expect(command.bootstrap?.createThread?.worktreePath).toBe(sourceWorktreePath);
+    expect(command.bootstrap?.runSetupScript).toBe(true);
+  }),
+);
+
+it.effect("fails existing worktree mode when git status cannot resolve the branch", () =>
+  Effect.gen(function* () {
+    const commands: OrchestrationCommand[] = [];
+    const result = yield* callStartTool(
+      { prompt: "Use bad checkout", mode: "existing_worktree", worktreePath: "/missing" },
+      commands,
+      {
+        gitStatus: () =>
+          Effect.fail(
+            new GitCommandError({
+              operation: "status",
+              command: "git status",
+              cwd: "/missing",
+              detail: "not a git repository",
+            }),
+          ),
+      },
+    );
+
+    expect(result.isError).toBe(true);
+    expect(commands).toHaveLength(0);
+  }),
+);
+
+it.effect("fails current checkout mode when fallback branch lookup fails", () =>
+  Effect.gen(function* () {
+    const commands: OrchestrationCommand[] = [];
+    const result = yield* callStartTool(
+      { prompt: "Use current checkout", mode: "current_checkout" },
+      commands,
+      {
+        sourceThread: { ...sourceThread, branch: null, worktreePath: "/missing" },
+        gitStatus: () =>
+          Effect.fail(
+            new GitCommandError({
+              operation: "status",
+              command: "git status",
+              cwd: "/missing",
+              detail: "not a git repository",
+            }),
+          ),
+      },
+    );
+
+    expect(result.isError).toBe(true);
+    expect(commands).toHaveLength(0);
   }),
 );

apps/server/src/mcp/toolkits/thread/handlers.ts

@@ -89,6 +89,17 @@ const makeActiveThreadStartRuntime = Effect.fn("ThreadToolkit.makeActiveRuntime"
     );
   });
 
+  const resolveRequiredCurrentBranch = Effect.fn("ThreadToolkit.resolveRequiredCurrentBranch")(
+    function* (cwd: string, failureMessage: string) {
+      const branch = yield* gitWorkflow.status({ cwd }).pipe(
+        Effect.map((status) => status.refName),
+        Effect.mapError((error) => fail(error instanceof Error ? error.message : failureMessage)),
+      );
+      if (branch) return branch;
+      return yield* fail(failureMessage);
+    },
+  );
+
   const resolveDefaultBranch = Effect.fn("ThreadToolkit.resolveDefaultBranch")(function* (
     cwd: string,
   ) {
@@ -132,9 +143,18 @@ const makeActiveThreadStartRuntime = Effect.fn("ThreadToolkit.makeActiveRuntime"
       if (!input.worktreePath) {
         return yield* fail("existing_worktree mode requires worktreePath.");
       }
-      return yield* resolveCurrentBranch(input.worktreePath);
+      return yield* resolveRequiredCurrentBranch(
+        input.worktreePath,
+        `Could not resolve current branch for existing worktree ${input.worktreePath}.`,
+      );
     }
-    return sourceThread.branch ?? (yield* resolveCurrentBranch(project.workspaceRoot));
+    return (
+      sourceThread.branch ??
+      (yield* resolveRequiredCurrentBranch(
+        sourceThread.worktreePath ?? project.workspaceRoot,
+        "Could not resolve current branch for the current checkout.",
+      ))
+    );
   });
 
   const loadSourceContext = Effect.fn("ThreadToolkit.loadSourceContext")(function* (
@@ -176,7 +196,11 @@ const makeActiveThreadStartRuntime = Effect.fn("ThreadToolkit.makeActiveRuntime"
     const createdAt = yield* nowIso;
     const branch = (yield* resolveInitialBranch(mode, input, project, sourceThread)) ?? null;
     const worktreePath: string | null =
-      mode === "existing_worktree" ? (input.worktreePath ?? null) : null;
+      mode === "existing_worktree"
+        ? (input.worktreePath ?? null)
+        : mode === "current_checkout"
+          ? (sourceThread.worktreePath ?? project.workspaceRoot)
+          : null;
     const title = input.title ?? truncateTitle(input.prompt);
     const modelSelection = resolveModelSelection(input, sourceThread);
     const runtimeMode = input.runtimeMode ?? sourceThread.runtimeMode;