authorgravatar for saiansh2525@gmail.comYash Singh <saiansh2525@gmail.com> 2026-10-12 20:04:34-05:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-12 20:04:34-05:00
loge2e95a39d309e1ffec901682cc88ea1ba276c868
tree46a92e2c520f3b269945d26fa85d253317bd47d2
parent8110edfddd3fb4d7fd2e0c9d29256c3a1b9da5c8
signature Signed by PGP key B5690EEEBB952194

fix(server): return null for unresolved optional commands (#18871)

9 files changed, 140 insertions(+), 52 deletions(-)

apps/server/src/process/agentScope.ts+8-9
......@@ -106,7 +106,6 @@ export const make = Effect.gen(function* () {
106106 Effect.provideService(FileSystem.FileSystem, fileSystem),
107107 Effect.provideService(Path.Path, path),
108108 Effect.provideService(HostProcess.Platform, platform),
109 Effect.option,
110109 );
111110
112111 const run = (command: string, args: ReadonlyArray<string>) =>
......@@ -120,9 +119,9 @@ export const make = Effect.gen(function* () {
120119 if (runtimeDir === undefined || runtimeDir.length === 0) return undefined;
121120 const systemdRun = yield* resolve("systemd-run", hostEnvironment);
122121 const systemctl = yield* resolve("systemctl", hostEnvironment);
123 if (systemdRun._tag === "None" || systemctl._tag === "None") return undefined;
122 if (systemdRun === null || systemctl === null) return undefined;
124123 const probe = (options: ReadonlyArray<string>) =>
125 run(systemdRun.value, [
124 run(systemdRun, [
126125 "--user",
127126 "--scope",
128127 "--quiet",
......@@ -146,7 +145,7 @@ export const make = Effect.gen(function* () {
146145 return undefined;
147146 }
148147 const limits = agentSliceMemoryLimits(NodeOS.totalmem());
149 const limited = yield* run(systemctl.value, [
148 const limited = yield* run(systemctl, [
150149 "--user",
151150 "set-property",
152151 "--runtime",
......@@ -165,8 +164,8 @@ export const make = Effect.gen(function* () {
165164 oomPolicy: scopeOptions.length > 0,
166165 });
167166 return {
168 systemdRun: systemdRun.value,
169 systemctl: systemctl.value,
167 systemdRun,
168 systemctl,
170169 runtimeDir,
171170 scopeOptions,
172171 };
......@@ -294,9 +293,9 @@ export const make = Effect.gen(function* () {
294293 wrap: Effect.fn("AgentScope.wrap")(function* ({ command, args, name, threadId, env }) {
295294 const resolved = yield* resolve(command, env ?? hostEnvironment);
296295 // Leave unresolved commands alone so the spawn reports the missing binary.
297 if (resolved._tag === "None") return { command, args };
296 if (resolved === null) return { command, args };
298297 const scope = yield* systemd;
299 if (scope === undefined) return agentScopeCommand({ command: resolved.value, args });
298 if (scope === undefined) return agentScopeCommand({ command: resolved, args });
300299 const unit = `t3code-${name}-${NodeCrypto.randomUUID().slice(0, 8)}.scope`;
301300 if (threadId !== undefined) yield* track(scope.systemctl, threadId, unit);
302301 const systemdScope: AgentSystemdScope = {
......@@ -308,7 +307,7 @@ export const make = Effect.gen(function* () {
308307 // the unit even when it fails.
309308 options: threadId === undefined ? [...scope.scopeOptions, "--collect"] : scope.scopeOptions,
310309 };
311 return agentScopeCommand({ command: resolved.value, args, scope: systemdScope });
310 return agentScopeCommand({ command: resolved, args, scope: systemdScope });
312311 }),
313312 oomKilled,
314313 clear,
apps/server/src/provider/CodexInstallation.ts+1
......@@ -335,6 +335,7 @@ export const makeCodexInstallation = Effect.fn("makeCodexInstallation")(function
335335 Effect.provideService(FileSystem.FileSystem, fs),
336336 Effect.provideService(Path.Path, path),
337337 );
338 if (executablePath === null) return null;
338339 // Keep launcher symlinks intact: version-manager shims dispatch by their invoked name.
339340 const realExecutablePath = yield* fs.realPath(executablePath);
340341 // A PATH entry pointing into T3's download remains a managed installation.
apps/server/src/provider/antigravityAuthSupport.test.ts-1
......@@ -592,7 +592,6 @@ it.layer(NodeServices.layer)("Antigravity profile preparation", (it) => {
592592 detail: expect.stringContaining("Install Node.js"),
593593 cause: {
594594 _tag: "NodeRuntimeUnavailableError",
595 cause: { _tag: "CommandResolutionError" },
596595 },
597596 });
598597 }
apps/server/src/provider/providerInstallation.test.ts+21
......@@ -238,4 +238,25 @@ describe("provider installation routing", () => {
238238 assert.deepEqual(harness.calls, ["remove", "refresh"]);
239239 }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
240240 );
241
242 it.effect("keeps only the configured path when another instance's binary is missing", () =>
243 Effect.gen(function* () {
244 const fs = yield* FileSystem.FileSystem;
245 const path = yield* Path.Path;
246 const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-provider-install-route-" });
247 const binaryPath = path.join(directory, "missing");
248 const harness = yield* makeHarness({
249 settings: {
250 providerInstances: {
251 [ProviderInstanceId.make("antigravity-work")]: {
252 driver,
253 config: { binaryPath },
254 },
255 },
256 },
257 });
258 yield* harness.router.remove({ instanceId });
259 assert.deepEqual(harness.protectedPaths(), [binaryPath]);
260 }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
261 );
241262});
apps/server/src/provider/providerInstallation.ts+3-5
......@@ -141,12 +141,10 @@ export const makeProviderInstallation = Effect.fn("makeProviderInstallation")(fu
141141 return [];
142142 }
143143 const binaryPath = entry.config.binaryPath.trim();
144 return yield* resolveCommandPath(binaryPath, {
144 const resolved = yield* resolveCommandPath(binaryPath, {
145145 env: yield* mergeProviderInstanceEnvironment(entry.environment),
146 }).pipe(
147 Effect.map((resolved) => [binaryPath, resolved]),
148 Effect.orElseSucceed(() => [binaryPath]),
149 );
146 });
147 return resolved === null ? [binaryPath] : [binaryPath, resolved];
150148 }),
151149 );
152150 yield* installation
packages/provider-core/src/server/maintenanceResolver.ts+2-6
......@@ -498,9 +498,7 @@ export const resolvePackageManagedProviderMaintenance = Effect.fn(
498498 const kegBrewPath = path.join(homebrew.prefix, "bin", "brew");
499499 const brewPath = (yield* fileSystem.exists(kegBrewPath).pipe(Effect.orElseSucceed(() => false)))
500500 ? kegBrewPath
501 : yield* resolveCommandPath("brew", { env: context.env }).pipe(
502 Effect.catchTags({ CommandResolutionError: () => Effect.succeed(null) }),
503 );
501 : yield* resolveCommandPath("brew", { env: context.env });
504502 if (!brewPath) {
505503 return fallback;
506504 }
......@@ -648,9 +646,7 @@ export const resolveProviderMaintenanceCapabilitiesEffect = Effect.fn(
648646 // resolveCommandPath checks explicit paths for existence too, so a missing
649647 // binary always lands in the no-context branch and never gets an update
650648 // command it cannot run.
651 const resolvedCommandPath = yield* resolveCommandPath(binaryPath, { env }).pipe(
652 Effect.catchTags({ CommandResolutionError: () => Effect.succeed(null) }),
653 );
649 const resolvedCommandPath = yield* resolveCommandPath(binaryPath, { env });
654650 if (!resolvedCommandPath) {
655651 return yield* resolver.resolve(null);
656652 }
packages/shared/src/nodeRuntime.ts+3-3
......@@ -76,15 +76,15 @@ export const resolveNodeExecutable = Effect.fn("nodeRuntime.resolveNodeExecutabl
7676 const path = yield* Path.Path;
7777 const platform = yield* HostProcess.Platform;
7878 const env = environment ?? (yield* HostProcess.Environment);
79 const nodePath = yield* resolveCommandPath(platform === "win32" ? "node.exe" : "node", {
79 const resolvedNodePath = yield* resolveCommandPath(platform === "win32" ? "node.exe" : "node", {
8080 // Batch wrappers require a shell; helper callers launch the runtime directly.
8181 env: platform === "win32" ? { ...env, PATHEXT: ".EXE" } : env,
8282 }).pipe(
8383 // Refresh immediately after the user installs Node and retries setup.
8484 Effect.provideService(CommandResolutionCache, new Map()),
85 Effect.map((commandPath) => path.resolve(commandPath)),
86 Effect.mapError((cause) => new NodeRuntimeUnavailableError({ feature, cause })),
8785 );
86 if (resolvedNodePath === null) return yield* new NodeRuntimeUnavailableError({ feature });
87 const nodePath = path.resolve(resolvedNodePath);
8888 // A launcher or symlink named node must not point back at the standalone app.
8989 const resolvedPath = yield* fs
9090 .realPath(nodePath)
packages/shared/src/shell.test.ts+97-11
......@@ -2,9 +2,12 @@ import * as NodeServices from "@effect/platform-node/NodeServices";
22import { it as effectIt } from "@effect/vitest";
33import * as HostProcess from "@t3tools/shared/HostProcess";
44import * as Effect from "effect/Effect";
5import * as Exit from "effect/Exit";
56import * as FileSystem from "effect/FileSystem";
67import * as Path from "effect/Path";
8import * as Result from "effect/Result";
79import * as TestClock from "effect/testing/TestClock";
10import * as Tracer from "effect/Tracer";
811import { describe, expect, it, vi } from "vite-plus/test";
912
1013import {
......@@ -41,6 +44,19 @@ const withWindowsEnvironmentMocks = <A, E, R>(
4144 Effect.provideService(CommandAvailability, commandAvailable),
4245 );
4346
47const recordCommandTrace = Effect.fnUntraced(function* <A, E, R>(effect: Effect.Effect<A, E, R>) {
48 const spans: Array<Tracer.NativeSpan> = [];
49 const tracer = Tracer.make({
50 span: (options) => {
51 const span = new Tracer.NativeSpan(options);
52 spans.push(span);
53 return span;
54 },
55 });
56 const result = yield* effect.pipe(Effect.withTracer(tracer), Effect.result);
57 return { result, spans };
58});
59
4460describe("readPathFromLoginShell", () => {
4561 it("uses a shell-agnostic printenv PATH probe", () => {
4662 const execFile = vi.fn<
......@@ -338,6 +354,65 @@ describe("resolveKnownWindowsCliDirs", () => {
338354});
339355
340356effectIt.layer(NodeServices.layer)("isCommandAvailable", (it) => {
357 it.effect.each(["win32", "linux"] as const)(
358 "records missing optional commands as successful lookups on %s",
359 (platform) =>
360 Effect.gen(function* () {
361 const fs = yield* FileSystem.FileSystem;
362 const path = yield* Path.Path;
363 const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-command-trace-" });
364 const env = { PATH: cwd, PATHEXT: ".CMD" };
365 const checks = [
366 isCommandAvailable("missing", { env }),
367 isCommandAvailable("missing", { env }),
368 isCommandAvailable("missing", { env: { ...env, PATH: "" } }),
369 isCommandAvailable(path.join(cwd, "missing.cmd"), { env }),
370 ];
371 for (const check of checks) {
372 const { result, spans } = yield* recordCommandTrace(check);
373 expect(Result.getOrThrow(result)).toBe(false);
374 expect(spans.length).toBeGreaterThan(0);
375 for (const span of spans) {
376 expect(span.status._tag).toBe("Ended");
377 if (span.status._tag === "Ended") {
378 expect(Exit.isSuccess(span.status.exit), span.name).toBe(true);
379 }
380 }
381 }
382 }).pipe(
383 Effect.provideService(HostProcess.Platform, platform),
384 Effect.provideService(CommandResolutionCache, new Map()),
385 ),
386 );
387
388 it.effect.each(["win32", "linux"] as const)(
389 "records available commands as successful lookups on %s",
390 (platform) =>
391 Effect.gen(function* () {
392 const fs = yield* FileSystem.FileSystem;
393 const path = yield* Path.Path;
394 const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-command-trace-" });
395 const executable = path.join(cwd, platform === "win32" ? "available.cmd" : "available");
396 yield* fs.writeFileString(executable, "");
397 yield* fs.chmod(executable, 0o700);
398 const env = { PATH: cwd, PATHEXT: ".CMD" };
399 for (const command of ["available", executable]) {
400 const { result, spans } = yield* recordCommandTrace(isCommandAvailable(command, { env }));
401 expect(Result.getOrThrow(result)).toBe(true);
402 expect(spans.length).toBeGreaterThan(0);
403 for (const span of spans) {
404 expect(span.status._tag).toBe("Ended");
405 if (span.status._tag === "Ended") {
406 expect(Exit.isSuccess(span.status.exit), span.name).toBe(true);
407 }
408 }
409 }
410 }).pipe(
411 Effect.provideService(HostProcess.Platform, platform),
412 Effect.provideService(CommandResolutionCache, new Map()),
413 ),
414 );
415
341416 it.effect("returns false when PATH is empty", () =>
342417 Effect.gen(function* () {
343418 expect(
......@@ -350,13 +425,29 @@ effectIt.layer(NodeServices.layer)("isCommandAvailable", (it) => {
350425});
351426
352427effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => {
353 it.effect("fails when PATH is empty", () =>
428 it.effect("records a successful lookup when a command is missing", () =>
429 Effect.gen(function* () {
430 const { result, spans } = yield* recordCommandTrace(
431 resolveCommandPath("missing", { env: { PATH: "" } }),
432 );
433 expect(Result.getOrThrow(result)).toBeNull();
434 expect(spans.length).toBeGreaterThan(0);
435 for (const span of spans) {
436 expect(span.status._tag).toBe("Ended");
437 if (span.status._tag === "Ended") {
438 expect(Exit.isSuccess(span.status.exit), span.name).toBe(true);
439 }
440 }
441 }),
442 );
443
444 it.effect("returns null when PATH is empty", () =>
354445 Effect.gen(function* () {
355446 const result = yield* resolveCommandPath("definitely-not-installed", {
356447 env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" },
357 }).pipe(Effect.provideService(HostProcess.Platform, "win32"), Effect.result);
448 }).pipe(Effect.provideService(HostProcess.Platform, "win32"));
358449
359 expect(result._tag).toBe("Failure");
450 expect(result).toBeNull();
360451 }),
361452 );
362453
......@@ -379,10 +470,9 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => {
379470 }),
380471 }),
381472 ),
382 Effect.result,
383473 );
384474
385 expect(result._tag).toBe("Failure");
475 expect(result).toBeNull();
386476 return probed;
387477 });
388478
......@@ -448,13 +538,9 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => {
448538 const executable = path.join(cwd, "appeared.CMD");
449539 const options = { env: { PATH: `${cwd};${cwd}`, PATHEXT: ".CMD" } };
450540
451 expect((yield* resolveCommandPath("appeared", options).pipe(Effect.result))._tag).toBe(
452 "Failure",
453 );
541 expect(yield* resolveCommandPath("appeared", options)).toBeNull();
454542 yield* fs.writeFileString(executable, "@echo off\n");
455 expect((yield* resolveCommandPath("appeared", options).pipe(Effect.result))._tag).toBe(
456 "Failure",
457 );
543 expect(yield* resolveCommandPath("appeared", options)).toBeNull();
458544 expect(yield* resolveCommandPath(executable, options)).toBe(executable);
459545 yield* TestClock.adjust("30 seconds");
460546 expect(yield* resolveCommandPath("appeared", options)).toBe(executable);
packages/shared/src/shell.ts+5-17
......@@ -5,7 +5,6 @@ import * as NodeChildProcess from "node:child_process";
55import * as NodeFS from "node:fs";
66import * as Cache from "effect/Cache";
77import * as Clock from "effect/Clock";
8import * as Data from "effect/Data";
98import * as Effect from "effect/Effect";
109import * as FileSystem from "effect/FileSystem";
1110import * as Option from "effect/Option";
......@@ -43,11 +42,6 @@ export type CommandAvailabilityChecker = (
4342 options?: CommandAvailabilityOptions,
4443) => Effect.Effect<boolean, never, FileSystem.FileSystem | Path.Path>;
4544
46export class CommandResolutionError extends Data.TaggedError("CommandResolutionError")<{
47 readonly command: string;
48 readonly reason: "not-found";
49}> {}
50
5145const WINDOWS_SHELL_META_CHARS = /([()\][%!^"`<>&|;, *?])/g;
5246
5347/**
......@@ -612,7 +606,7 @@ const isExecutableFile = Effect.fnUntraced(function* (
612606const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlatform")(function* (
613607 command: string,
614608 options: CommandAvailabilityOptions & { readonly platform: NodeJS.Platform },
615): Effect.fn.Return<string, CommandResolutionError, FileSystem.FileSystem | Path.Path> {
609): Effect.fn.Return<string | null, never, FileSystem.FileSystem | Path.Path> {
616610 const path = yield* Path.Path;
617611 const platform = options.platform;
618612 const env = options.env ?? process.env;
......@@ -630,12 +624,12 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat
630624 return candidate;
631625 }
632626 }
633 return yield* new CommandResolutionError({ command, reason: "not-found" });
627 return null;
634628 }
635629
636630 const pathValue = resolvePathEnvironmentVariable(env);
637631 if (pathValue.length === 0) {
638 return yield* new CommandResolutionError({ command, reason: "not-found" });
632 return null;
639633 }
640634
641635 const cacheKey = [platform, pathValue, windowsPathExtensions.join(";"), command].join(
......@@ -645,9 +639,6 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat
645639 const nowNanos = yield* Clock.monotonicTimeNanos;
646640 const cached = cache.get(cacheKey);
647641 if (cached !== undefined && cached.expiresAtNanos > nowNanos) {
648 if (cached.resolvedPath === null) {
649 return yield* new CommandResolutionError({ command, reason: "not-found" });
650 }
651642 return cached.resolvedPath;
652643 }
653644
......@@ -680,7 +671,7 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat
680671 }
681672 }
682673 cacheCommandResolution(cache, cacheKey, null, nowNanos);
683 return yield* new CommandResolutionError({ command, reason: "not-found" });
674 return null;
684675});
685676
686677export const resolveCommandPath = Effect.fn("shell.resolveCommandPath")(function* (
......@@ -800,10 +791,7 @@ export const isCommandAvailable = Effect.fn("shell.isCommandAvailable")(function
800791 command: string,
801792 options: CommandAvailabilityOptions = {},
802793) {
803 return yield* resolveCommandPath(command, options).pipe(
804 Effect.as(true),
805 Effect.catchTags({ CommandResolutionError: () => Effect.succeed(false) }),
806 );
794 return (yield* resolveCommandPath(command, options)) !== null;
807795});
808796
809797export function resolveKnownWindowsCliDirs(env: NodeJS.ProcessEnv): ReadonlyArray<string> {