Skip to content

Commit 094c88c

Browse files
committed
Address thread start MCP review feedback
1 parent 31d431e commit 094c88c

3 files changed

Lines changed: 107 additions & 24 deletions

File tree

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

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { GitWorkflowService } from "../../../git/GitWorkflowService.ts";
2020
import * as BootstrapTurnStartDispatcher from "../../../orchestration/Services/BootstrapTurnStartDispatcher.ts";
2121
import { OrchestrationEngineService } from "../../../orchestration/Services/OrchestrationEngine.ts";
2222
import { ProjectionSnapshotQuery } from "../../../orchestration/Services/ProjectionSnapshotQuery.ts";
23+
import * as ServerRuntimeStartup from "../../../serverRuntimeStartup.ts";
2324
import * as McpInvocationContext from "../../McpInvocationContext.ts";
2425
import { ThreadToolkitRegistrationLive } from "../../McpHttpServer.ts";
2526
import { ThreadStartRuntimeLive } from "./handlers.ts";
@@ -90,14 +91,26 @@ const TestCryptoLive = Layer.sync(Crypto.Crypto, () => {
9091
});
9192
});
9293

93-
const makeTestLayer = (commands: OrchestrationCommand[]) => {
94+
const makeTestLayer = (
95+
commands: OrchestrationCommand[],
96+
options: { readonly enqueueCalls?: number[] } = {},
97+
) => {
9498
const bootstrapTurnStartDispatcherLayer = Layer.mock(
9599
BootstrapTurnStartDispatcher.BootstrapTurnStartDispatcher,
96100
)({
97101
dispatch: (command) =>
98102
Effect.sync(() => {
99103
commands.push(command);
100-
return { sequence: 1 };
104+
return {
105+
sequence: 1,
106+
branch:
107+
command.bootstrap?.prepareWorktree?.branch ??
108+
command.bootstrap?.createThread?.branch ??
109+
null,
110+
worktreePath: command.bootstrap?.prepareWorktree
111+
? "/repo/.worktrees/generated"
112+
: (command.bootstrap?.createThread?.worktreePath ?? null),
113+
};
101114
}),
102115
});
103116

@@ -161,10 +174,24 @@ const makeTestLayer = (commands: OrchestrationCommand[]) => {
161174
streamDomainEvents: Stream.empty,
162175
}),
163176
),
177+
Layer.provide(
178+
Layer.mock(ServerRuntimeStartup.ServerRuntimeStartup)({
179+
awaitCommandReady: Effect.void,
180+
markHttpListening: Effect.void,
181+
enqueueCommand: (effect) =>
182+
Effect.sync(() => {
183+
options.enqueueCalls?.push(1);
184+
}).pipe(Effect.flatMap(() => effect)),
185+
}),
186+
),
164187
);
165188
};
166189

167-
const callStartTool = (arguments_: Record<string, unknown>, commands: OrchestrationCommand[]) =>
190+
const callStartTool = (
191+
arguments_: Record<string, unknown>,
192+
commands: OrchestrationCommand[],
193+
options: { readonly enqueueCalls?: number[] } = {},
194+
) =>
168195
Effect.gen(function* () {
169196
const server = yield* McpServer.McpServer;
170197
return yield* server
@@ -173,7 +200,7 @@ const callStartTool = (arguments_: Record<string, unknown>, commands: Orchestrat
173200
Effect.provideService(McpInvocationContext.McpInvocationContext, invocation),
174201
Effect.provideService(McpSchema.McpServerClient, client),
175202
);
176-
}).pipe(Effect.provide(makeTestLayer(commands)));
203+
}).pipe(Effect.provide(makeTestLayer(commands, options)));
177204

178205
it.effect("starts a new worktree thread by default and inherits source settings", () =>
179206
Effect.gen(function* () {
@@ -184,7 +211,7 @@ it.effect("starts a new worktree thread by default and inherits source settings"
184211
expect(result.structuredContent).toMatchObject({
185212
projectId,
186213
mode: "new_worktree",
187-
worktreePath: null,
214+
worktreePath: "/repo/.worktrees/generated",
188215
});
189216
const command = commands[0];
190217
expect(command?.type).toBe("thread.turn.start");
@@ -202,6 +229,46 @@ it.effect("starts a new worktree thread by default and inherits source settings"
202229
}),
203230
);
204231

232+
it.effect("queues thread starts through server runtime startup", () =>
233+
Effect.gen(function* () {
234+
const commands: OrchestrationCommand[] = [];
235+
const enqueueCalls: number[] = [];
236+
const result = yield* callStartTool({ prompt: "Wait for server readiness" }, commands, {
237+
enqueueCalls,
238+
});
239+
240+
expect(result.isError).toBe(false);
241+
expect(enqueueCalls).toHaveLength(1);
242+
expect(commands).toHaveLength(1);
243+
}),
244+
);
245+
246+
it.effect("passes explicit setup script requests for existing worktrees", () =>
247+
Effect.gen(function* () {
248+
const commands: OrchestrationCommand[] = [];
249+
const result = yield* callStartTool(
250+
{
251+
prompt: "Use existing checkout",
252+
mode: "existing_worktree",
253+
worktreePath: "/repo/existing",
254+
runSetupScript: true,
255+
},
256+
commands,
257+
);
258+
259+
expect(result.isError).toBe(false);
260+
expect(result.structuredContent).toMatchObject({
261+
mode: "existing_worktree",
262+
worktreePath: "/repo/existing",
263+
});
264+
const command = commands[0];
265+
expect(command?.type).toBe("thread.turn.start");
266+
if (command?.type !== "thread.turn.start") return;
267+
expect(command.bootstrap?.prepareWorktree).toBeUndefined();
268+
expect(command.bootstrap?.runSetupScript).toBe(true);
269+
}),
270+
);
271+
205272
it.effect("starts current-checkout threads with warning metadata", () =>
206273
Effect.gen(function* () {
207274
const commands: OrchestrationCommand[] = [];

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
MessageId,
44
ThreadId,
55
type ModelSelection,
6+
type OrchestrationCommand,
67
type OrchestrationProjectShell,
78
type OrchestrationThreadShell,
89
} from "@t3tools/contracts";
@@ -18,6 +19,7 @@ import * as McpInvocationContext from "../../McpInvocationContext.ts";
1819
import * as BootstrapTurnStartDispatcher from "../../../orchestration/Services/BootstrapTurnStartDispatcher.ts";
1920
import { ProjectionSnapshotQuery } from "../../../orchestration/Services/ProjectionSnapshotQuery.ts";
2021
import { GitWorkflowService } from "../../../git/GitWorkflowService.ts";
22+
import * as ServerRuntimeStartup from "../../../serverRuntimeStartup.ts";
2123
import {
2224
ThreadStartToolError,
2325
type ThreadStartMode,
@@ -60,6 +62,7 @@ const makeActiveThreadStartRuntime = Effect.fn("ThreadToolkit.makeActiveRuntime"
6062
const crypto = yield* Crypto.Crypto;
6163
const projectionSnapshotQuery = yield* ProjectionSnapshotQuery;
6264
const gitWorkflow = yield* GitWorkflowService;
65+
const startup = yield* ServerRuntimeStartup.ServerRuntimeStartup;
6366
const uuid = () => crypto.randomUUIDv4.pipe(Effect.orDie);
6467

6568
const makeIds = Effect.fn("ThreadToolkit.makeIds")(function* () {
@@ -186,12 +189,13 @@ const makeActiveThreadStartRuntime = Effect.fn("ThreadToolkit.makeActiveRuntime"
186189
branch: branch ?? undefined,
187190
}
188191
: undefined;
192+
const runSetupScript = input.runSetupScript ?? (mode === "new_worktree" ? true : undefined);
189193

190194
if (mode === "existing_worktree" && !worktreePath) {
191195
return yield* fail("existing_worktree mode requires worktreePath.");
192196
}
193197

194-
yield* BootstrapTurnStartDispatcher.dispatchActive({
198+
const command: Extract<OrchestrationCommand, { type: "thread.turn.start" }> = {
195199
type: "thread.turn.start",
196200
commandId: ids.commandId,
197201
threadId: ids.threadId,
@@ -216,26 +220,26 @@ const makeActiveThreadStartRuntime = Effect.fn("ThreadToolkit.makeActiveRuntime"
216220
worktreePath,
217221
createdAt,
218222
},
219-
...(prepareWorktree
220-
? {
221-
prepareWorktree,
222-
runSetupScript: input.runSetupScript ?? true,
223-
}
224-
: {}),
223+
...(prepareWorktree ? { prepareWorktree } : {}),
224+
...(runSetupScript === undefined ? {} : { runSetupScript }),
225225
},
226226
createdAt,
227-
}).pipe(
228-
Effect.mapError((error) =>
229-
fail(error instanceof Error ? error.message : "Failed to start child thread."),
230-
),
231-
);
227+
};
228+
229+
const result = yield* startup
230+
.enqueueCommand(BootstrapTurnStartDispatcher.dispatchActive(command))
231+
.pipe(
232+
Effect.mapError((error) =>
233+
fail(error instanceof Error ? error.message : "Failed to start child thread."),
234+
),
235+
);
232236

233237
return {
234238
threadId: ids.threadId,
235239
projectId: project.id,
236240
mode,
237-
branch,
238-
worktreePath,
241+
branch: result.branch ?? branch,
242+
worktreePath: result.worktreePath,
239243
...(mode === "current_checkout"
240244
? {
241245
warning:

apps/server/src/orchestration/Services/BootstrapTurnStartDispatcher.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,14 @@ function projectSetupScriptCompatibilityDetail(
5454
}
5555

5656
export interface BootstrapTurnStartDispatcherShape {
57-
readonly dispatch: (
58-
command: ThreadTurnStartCommand,
59-
) => Effect.Effect<{ readonly sequence: number }, OrchestrationDispatchCommandError>;
57+
readonly dispatch: (command: ThreadTurnStartCommand) => Effect.Effect<
58+
{
59+
readonly sequence: number;
60+
readonly branch: string | null;
61+
readonly worktreePath: string | null;
62+
},
63+
OrchestrationDispatchCommandError
64+
>;
6065
}
6166

6267
export class BootstrapTurnStartDispatcher extends Context.Service<
@@ -68,7 +73,7 @@ let activeDispatcher: BootstrapTurnStartDispatcherShape | null = null;
6873

6974
export const dispatchActive = (
7075
command: ThreadTurnStartCommand,
71-
): Effect.Effect<{ readonly sequence: number }, OrchestrationDispatchCommandError> => {
76+
): ReturnType<BootstrapTurnStartDispatcherShape["dispatch"]> => {
7277
const dispatcher = activeDispatcher;
7378
if (!dispatcher) {
7479
return Effect.fail(
@@ -176,6 +181,7 @@ export const layer = Layer.effect(
176181
let createdThread = false;
177182
let targetProjectId = bootstrap?.createThread?.projectId;
178183
let targetProjectCwd = bootstrap?.prepareWorktree?.projectCwd;
184+
let targetBranch = bootstrap?.createThread?.branch ?? null;
179185
let targetWorktreePath = bootstrap?.createThread?.worktreePath ?? null;
180186

181187
const cleanupCreatedThread = () =>
@@ -346,6 +352,7 @@ export const layer = Layer.effect(
346352
baseRefName: bootstrap.prepareWorktree.baseBranch,
347353
path: null,
348354
});
355+
targetBranch = worktree.worktree.refName;
349356
targetWorktreePath = worktree.worktree.path;
350357
yield* orchestrationEngine.dispatch({
351358
type: "thread.meta.update",
@@ -359,7 +366,12 @@ export const layer = Layer.effect(
359366

360367
yield* runSetupProgram();
361368

362-
return yield* orchestrationEngine.dispatch(finalTurnStartCommand);
369+
const result = yield* orchestrationEngine.dispatch(finalTurnStartCommand);
370+
return {
371+
...result,
372+
branch: targetBranch,
373+
worktreePath: targetWorktreePath,
374+
};
363375
});
364376

365377
return yield* bootstrapProgram.pipe(

0 commit comments

Comments
 (0)