Skip to content
Draft
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
2 changes: 2 additions & 0 deletions apps/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"@anthropic-ai/claude-agent-sdk": "^0.2.77",
"@effect/platform-node": "catalog:",
"@effect/sql-sqlite-bun": "catalog:",
"@github/copilot": "1.0.10",
"@github/copilot-sdk": "0.2.0",
"@pierre/diffs": "^1.1.0-beta.16",
"effect": "catalog:",
"node-pty": "^1.1.0",
Expand Down
116 changes: 81 additions & 35 deletions apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

🟢 Low

const runtime = ManagedRuntime.make(layer);
const engine = await runtime.runPromise(Effect.service(OrchestrationEngineService));
const reactor = await runtime.runPromise(Effect.service(ProviderCommandReactor));
scope = await Effect.runPromise(Scope.make("sequential"));
await Effect.runPromise(reactor.start.pipe(Scope.provide(scope)));
const drain = () => Effect.runPromise(reactor.drain);

The local const runtime declared at line 222 shadows the outer let runtime at line 64, so the outer variable remains null. Consequently, afterEach never calls runtime.dispose(), leaking the ManagedRuntime and its resources (SQLite connections, layer finalizers) on every test. Rename the local variable so the outer runtime is assigned.

-    const runtime = ManagedRuntime.make(layer);
+    const testRuntime = ManagedRuntime.make(layer);
 
-    const engine = await runtime.runPromise(Effect.service(OrchestrationEngineService));
-    const reactor = await runtime.runPromise(Effect.service(ProviderCommandReactor));
+    const engine = await testRuntime.runPromise(Effect.service(OrchestrationEngineService));
+    const reactor = await testRuntime.runPromise(Effect.service(ProviderCommandReactor));
     scope = await Effect.runPromise(Scope.make("sequential"));
     await Effect.runPromise(reactor.start.pipe(Scope.provide(scope)));
     const drain = () => Effect.runPromise(reactor.drain);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts around lines 222-228:

The local `const runtime` declared at line 222 shadows the outer `let runtime` at line 64, so the outer variable remains `null`. Consequently, `afterEach` never calls `runtime.dispose()`, leaking the `ManagedRuntime` and its resources (SQLite connections, layer finalizers) on every test. Rename the local variable so the outer `runtime` is assigned.

Evidence trail:
apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts lines 64-67 (outer `let runtime = null`), lines 71-79 (`afterEach` that checks `if (runtime)` and calls `runtime.dispose()`), lines 86-269 (`createHarness` function), line 222 (`const runtime = ManagedRuntime.make(layer);` - local shadowing), lines 258-269 (return object does not include runtime)

Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ describe("ProviderCommandReactor", () => {
typeof input === "object" &&
input !== null &&
"provider" in input &&
(input.provider === "codex" || input.provider === "claudeAgent")
(input.provider === "codex" ||
input.provider === "claudeAgent" ||
input.provider === "copilot")
? input.provider
: "codex";
const resumeCursor =
Expand Down Expand Up @@ -217,10 +219,10 @@ describe("ProviderCommandReactor", () => {
Layer.provideMerge(ServerConfig.layerTest(process.cwd(), stateDir)),
Layer.provideMerge(NodeServices.layer),
);
const runtime = ManagedRuntime.make(layer);
const testRuntime = (runtime = ManagedRuntime.make(layer));

const engine = await runtime.runPromise(Effect.service(OrchestrationEngineService));
const reactor = await runtime.runPromise(Effect.service(ProviderCommandReactor));
const engine = await testRuntime.runPromise(Effect.service(OrchestrationEngineService));
const reactor = await testRuntime.runPromise(Effect.service(ProviderCommandReactor));
scope = await Effect.runPromise(Scope.make("sequential"));
await Effect.runPromise(reactor.start.pipe(Scope.provide(scope)));
const drain = () => Effect.runPromise(reactor.drain);
Expand Down Expand Up @@ -288,8 +290,8 @@ describe("ProviderCommandReactor", () => {
}),
);

await waitFor(() => harness.startSession.mock.calls.length === 1);
await waitFor(() => harness.sendTurn.mock.calls.length === 1);
await waitFor(() => harness.startSession.mock.calls.length > 0);
await waitFor(() => harness.sendTurn.mock.calls.length > 0);
expect(harness.startSession.mock.calls[0]?.[0]).toEqual(ThreadId.makeUnsafe("thread-1"));
expect(harness.startSession.mock.calls[0]?.[1]).toMatchObject({
cwd: "/tmp/provider-project",
Expand Down Expand Up @@ -332,8 +334,8 @@ describe("ProviderCommandReactor", () => {
}),
);

await waitFor(() => harness.startSession.mock.calls.length === 1);
await waitFor(() => harness.sendTurn.mock.calls.length === 1);
await waitFor(() => harness.startSession.mock.calls.length > 0);
await waitFor(() => harness.sendTurn.mock.calls.length > 0);
expect(harness.startSession.mock.calls[0]?.[1]).toMatchObject({
model: "gpt-5.3-codex",
modelOptions: {
Expand Down Expand Up @@ -493,7 +495,7 @@ describe("ProviderCommandReactor", () => {
});
});

it("rejects a first turn when requested provider conflicts with the thread model", async () => {
it("binds a pristine thread to the explicitly requested provider on first turn", async () => {
const harness = await createHarness();
const now = new Date().toISOString();

Expand All @@ -505,44 +507,34 @@ describe("ProviderCommandReactor", () => {
message: {
messageId: asMessageId("user-message-provider-first"),
role: "user",
text: "hello claude",
text: "hello copilot",
attachments: [],
},
provider: "claudeAgent",
provider: "copilot",
model: "claude-sonnet-4.6",
interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE,
runtimeMode: "approval-required",
createdAt: now,
}),
);

await waitFor(async () => {
const readModel = await Effect.runPromise(harness.engine.getReadModel());
const thread = readModel.threads.find(
(entry) => entry.id === ThreadId.makeUnsafe("thread-1"),
);
return (
thread?.activities.some((activity) => activity.kind === "provider.turn.start.failed") ??
false
);
});
await waitFor(() => harness.startSession.mock.calls.length === 1);
await waitFor(() => harness.sendTurn.mock.calls.length === 1);

expect(harness.startSession).not.toHaveBeenCalled();
expect(harness.sendTurn).not.toHaveBeenCalled();
expect(harness.startSession).toHaveBeenCalledWith(
ThreadId.makeUnsafe("thread-1"),
expect.objectContaining({
provider: "copilot",
model: "claude-sonnet-4.6",
}),
);

const readModel = await Effect.runPromise(harness.engine.getReadModel());
const thread = readModel.threads.find((entry) => entry.id === ThreadId.makeUnsafe("thread-1"));
expect(thread?.session).toBeNull();
expect(
thread?.activities.find((activity) => activity.kind === "provider.turn.start.failed"),
).toMatchObject({
summary: "Provider turn start failed",
payload: {
detail: expect.stringContaining("cannot switch to 'claudeAgent'"),
},
});
expect(thread?.session?.providerName).toBe("copilot");
});

it("rejects a turn when the requested model belongs to a different provider", async () => {
it("infers the provider from the selected model on a pristine thread", async () => {
const harness = await createHarness();
const now = new Date().toISOString();

Expand All @@ -564,6 +556,60 @@ describe("ProviderCommandReactor", () => {
}),
);

await waitFor(() => harness.startSession.mock.calls.length === 1);
await waitFor(() => harness.sendTurn.mock.calls.length === 1);

expect(harness.startSession).toHaveBeenCalledWith(
ThreadId.makeUnsafe("thread-1"),
expect.objectContaining({
provider: "claudeAgent",
model: "claude-sonnet-4-6",
}),
);
});

it("rejects a provider-scoped model change after the thread is already bound", async () => {
const harness = await createHarness();
const now = new Date().toISOString();

await Effect.runPromise(
harness.engine.dispatch({
type: "thread.turn.start",
commandId: CommandId.makeUnsafe("cmd-turn-start-established-model-provider-1"),
threadId: ThreadId.makeUnsafe("thread-1"),
message: {
messageId: asMessageId("user-message-established-model-provider-1"),
role: "user",
text: "first",
attachments: [],
},
interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE,
runtimeMode: "approval-required",
createdAt: now,
}),
);

await waitFor(() => harness.startSession.mock.calls.length === 1);
await waitFor(() => harness.sendTurn.mock.calls.length === 1);

await Effect.runPromise(
harness.engine.dispatch({
type: "thread.turn.start",
commandId: CommandId.makeUnsafe("cmd-turn-start-established-model-provider-2"),
threadId: ThreadId.makeUnsafe("thread-1"),
message: {
messageId: asMessageId("user-message-established-model-provider-2"),
role: "user",
text: "second",
attachments: [],
},
model: "claude-sonnet-4-6",
interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE,
runtimeMode: "approval-required",
createdAt: now,
}),
);

await waitFor(async () => {
const readModel = await Effect.runPromise(harness.engine.getReadModel());
const thread = readModel.threads.find(
Expand All @@ -575,8 +621,8 @@ describe("ProviderCommandReactor", () => {
);
});

expect(harness.startSession).not.toHaveBeenCalled();
expect(harness.sendTurn).not.toHaveBeenCalled();
expect(harness.startSession.mock.calls.length).toBe(1);
expect(harness.sendTurn.mock.calls.length).toBe(1);

const readModel = await Effect.runPromise(harness.engine.getReadModel());
const thread = readModel.threads.find((entry) => entry.id === ThreadId.makeUnsafe("thread-1"));
Expand Down
34 changes: 29 additions & 5 deletions apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
ProviderCommandReactor,
type ProviderCommandReactorShape,
} from "../Services/ProviderCommandReactor.ts";
import { inferProviderForModel } from "@t3tools/shared/model";
import { getModelOptions, inferProviderForModel, normalizeModelSlug } from "@t3tools/shared/model";

type ProviderIntentEvent = Extract<
OrchestrationEvent,
Expand Down Expand Up @@ -139,6 +139,13 @@ function buildGeneratedWorktreeBranchName(raw: string): string {
return `${WORKTREE_BRANCH_PREFIX}/${safeFragment}`;
}

function isBuiltInModelForProvider(provider: ProviderKind, model: string | undefined): boolean {
const normalized = normalizeModelSlug(model, provider);
return (
normalized !== null && getModelOptions(provider).some((option) => option.slug === normalized)
);
}

const make = Effect.gen(function* () {
const orchestrationEngine = yield* OrchestrationEngineService;
const providerService = yield* ProviderService;
Expand Down Expand Up @@ -233,8 +240,23 @@ const make = Effect.gen(function* () {
)
? thread.session.providerName
: undefined;
const threadProvider: ProviderKind = currentProvider ?? inferProviderForModel(thread.model);
if (options?.provider !== undefined && options.provider !== threadProvider) {
const defaultThreadProvider = inferProviderForModel(thread.model);
const isThreadProviderLocked = currentProvider !== undefined || thread.latestTurn !== null;
const requestedProvider =
options?.provider ??
(options?.model !== undefined
? inferProviderForModel(options.model, defaultThreadProvider)
: undefined);
const threadProvider: ProviderKind =
currentProvider ??
(isThreadProviderLocked
? defaultThreadProvider
: (requestedProvider ?? defaultThreadProvider));
if (
isThreadProviderLocked &&
options?.provider !== undefined &&
options.provider !== threadProvider
) {
return yield* new ProviderAdapterRequestError({
provider: threadProvider,
method: "thread.turn.start",
Expand All @@ -243,15 +265,17 @@ const make = Effect.gen(function* () {
}
if (
options?.model !== undefined &&
inferProviderForModel(options.model, threadProvider) !== threadProvider
inferProviderForModel(options.model, threadProvider) !== threadProvider &&
!isBuiltInModelForProvider(threadProvider, options.model)
) {
return yield* new ProviderAdapterRequestError({
provider: threadProvider,
method: "thread.turn.start",
detail: `Model '${options.model}' does not belong to provider '${threadProvider}' for thread '${threadId}'.`,
});
}
const preferredProvider: ProviderKind = currentProvider ?? threadProvider;
const preferredProvider: ProviderKind =
currentProvider ?? requestedProvider ?? defaultThreadProvider;
const desiredModel = options?.model ?? thread.model;
const effectiveCwd = resolveThreadWorkspaceCwd({
thread,
Expand Down
Loading