Skip to content
Open
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
36 changes: 29 additions & 7 deletions packages/provider-acp/src/server/clientTerminals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,48 @@ describe("AcpClientTerminals", () => {
Effect.gen(function* () {
const terminals = yield* makeAcpClientTerminals({
defaultCwd: process.cwd(),
environmentForSession: (sessionId) => ({
T3_TEST_ACP_SESSION_TOKEN: sessionId === "session-a" ? "token-a" : "token-b",
}),
environmentForSession: (sessionId) =>
sessionId === "unknown"
? undefined
: { T3_ACP_MCP_AUTHORIZATION: sessionId === "session-a" ? "token-a" : "token-b" },
});
yield* Effect.addFinalizer(() => terminals.disposeAll);

for (const [sessionId, expected] of [
["session-a", "token-a"],
["session-b", "token-b"],
["session-a", "token-a||kept|T3_TEST_OVERRIDE=request"],
["session-b", "token-b||kept|T3_TEST_OVERRIDE=request"],
["unknown", "||kept|T3_TEST_OVERRIDE=request"],
] as const) {
const created = yield* terminals.create({
sessionId,
command: process.execPath,
args: ["-e", "process.stdout.write(process.env.T3_TEST_ACP_SESSION_TOKEN ?? '')"],
args: [
"-e",
"const env = process.env; process.stdout.write([env.T3_ACP_MCP_AUTHORIZATION ?? '', env.t3_Acp_Mcp_Endpoint ?? '', env.T3_TEST_INHERITED, ...Object.keys(env).filter((name) => name.toUpperCase() === 'T3_TEST_OVERRIDE').map((name) => name + '=' + env[name])].join('|'))",
],
env: [
{ name: "T3_TEST_OVERRIDE", value: "first" },
{ name: "t3_Test_Override", value: "second" },
{ name: "T3_TEST_OVERRIDE", value: "request" },
],
});
yield* terminals.waitForExit({ sessionId, terminalId: created.terminalId });
const output = yield* terminals.output({ sessionId, terminalId: created.terminalId });
expect(output.output).toBe(expected);
}
}).pipe(Effect.provide(NodeServices.layer), Effect.scoped),
}).pipe(
Effect.provideService(HostProcess.Environment, {
...process.env,
T3_ACP_MCP_AUTHORIZATION: "launching-thread-token",
t3_Acp_Mcp_Endpoint: "launching-thread-endpoint",
T3_TEST_INHERITED: "kept",
T3_TEST_OVERRIDE: "inherited",
}),
// Exercise Windows' case-insensitive variable names on every OS.
Effect.provideService(HostProcess.Platform, "win32"),
Effect.provide(NodeServices.layer),
Effect.scoped,
),
);

it.live("runs Devin shell source with arguments, pipes, and session fallback variables", () =>
Expand Down
43 changes: 32 additions & 11 deletions packages/provider-acp/src/server/clientTerminals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner";
import * as EffectAcpErrors from "effect-acp/errors";
import type * as EffectAcpSchema from "effect-acp/compat";
import * as NodeBuffer from "node:buffer";
import * as HostProcess from "@t3tools/shared/HostProcess";

const MAX_LIVE_TERMINALS = 16;
const MAX_UNRELEASED_TERMINALS = 64;
Expand Down Expand Up @@ -135,22 +136,46 @@ function acpTerminalCommand(input: {
readonly request: EffectAcpSchema.CreateTerminalRequest;
readonly shellCommands?: boolean | undefined;
readonly defaultCwd: string;
readonly environment?: NodeJS.ProcessEnv | undefined;
readonly environment: NodeJS.ProcessEnv;
readonly forceKillAfter: Duration.Input;
}) {
return ChildProcess.make(input.request.command, input.request.args ?? [], {
cwd: input.request.cwd ?? input.defaultCwd,
...(input.environment === undefined ? {} : { env: input.environment }),
env: input.environment,
shell: input.shellCommands === true && (input.request.args?.length ?? 0) === 0,
forceKillAfter: input.forceKillAfter,
});
}

// Windows variable names are case-insensitive and Node passes only one spelling to the child,
// so a later layer's `Path` must replace an earlier `PATH` rather than sit beside it.
function mergeEnvironments(
isWindows: boolean,
layers: ReadonlyArray<Iterable<readonly [string, string | undefined]>>,
) {
const variables = new Map<string, readonly [string, string | undefined]>();
for (const layer of layers) {
for (const [name, value] of layer) {
variables.set(isWindows ? name.toUpperCase() : name, [name, value]);
}
}
return Object.fromEntries(variables.values());
}

export const makeAcpClientTerminals = Effect.fn("makeAcpClientTerminals")(function* (
options: AcpClientTerminalsOptions,
): Effect.fn.Return<AcpClientTerminals, never, ChildProcessSpawner.ChildProcessSpawner> {
const spawner = yield* ChildProcessSpawner.ChildProcessSpawner;
const forceKillAfter = options.forceKillAfter ?? "5 seconds";
const isWindows = yield* HostProcess.isWindows;
// T3 MCP credentials come from environmentForSession; inherited ones belong to whichever
// thread launched this server, so a terminal must never fall back to them. Windows treats
// variable names case-insensitively, so match the prefix regardless of case.
const inheritedEnvironment = Object.fromEntries(
Object.entries(options.environment ?? (yield* HostProcess.Environment)).filter(
([name]) => !name.toUpperCase().startsWith("T3_ACP_MCP_"),
),
);
const creationPermit = yield* Semaphore.make(1);
const terminals = new Map<string, ManagedAcpTerminal>();
let nextTerminalNumber = 1;
Expand Down Expand Up @@ -213,16 +238,12 @@ export const makeAcpClientTerminals = Effect.fn("makeAcpClientTerminals")(functi
`ACP terminal/create exceeded the limit of ${MAX_LIVE_TERMINALS} concurrent terminals.`,
);
}
const requestEnvironment = Object.fromEntries(
(request.env ?? []).map((variable) => [variable.name, variable.value] as const),
);
const sessionEnvironment = options.environmentForSession?.(request.sessionId);
const environment =
options.environment === undefined &&
sessionEnvironment === undefined &&
(request.env?.length ?? 0) === 0
? undefined
: { ...options.environment, ...sessionEnvironment, ...requestEnvironment };
const environment = mergeEnvironments(isWindows, [
Object.entries(inheritedEnvironment),
Object.entries(sessionEnvironment ?? {}),
(request.env ?? []).map((variable) => [variable.name, variable.value] as const),
]);
const terminalScope = yield* Scope.make();
const child = yield* restore(
spawner
Expand Down
Loading