Skip to content

Commit fb015da

Browse files
juliusmarmingeclaude
juliusmarminge
and
claude
committed
fix(server): run native updaters with the instance's own environment
`codex update` replaces the standalone tree under CODEX_HOME. The runner spawned it with the server's environment, so an instance with a custom CODEX_HOME would have updated `~/.codex` and left its own binary behind. Update actions can now declare an environment, the Codex resolver sets CODEX_HOME per instance, and the runner passes it to the spawned process. The copyable command also quotes every word the shell would split, not only the executable, so an npm prefix with spaces pastes correctly. Co-Authored-By: Claude Code <[email protected]>
1 parent 3e98200 commit fb015da

5 files changed

Lines changed: 157 additions & 28 deletions

File tree

‎apps/server/src/provider/Drivers/CodexDriver.ts‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import { makeManagedServerProvider } from "../makeManagedServerProvider.ts";
5252
import * as ModelManifest from "../ModelManifest.ts";
5353
import type { ProviderDriver, ProviderInstance } from "../ProviderDriver.ts";
5454
import { withInstanceIdentity } from "./instanceIdentity.ts";
55+
import { expandHomePath } from "../../pathExpansion.ts";
5556
import { mergeProviderInstanceEnvironment } from "../ProviderInstanceEnvironment.ts";
5657
import {
5758
enrichProviderSnapshotWithVersionAdvisory,
@@ -79,14 +80,23 @@ function isCodexStandaloneCommandPath(commandPath: string): boolean {
7980
return normalizeCommandPath(commandPath).includes("/packages/standalone/");
8081
}
8182

82-
const UPDATE = makePackageManagedProviderMaintenanceResolver({
83-
provider: DRIVER_KIND,
84-
npmPackageName: "@openai/codex",
85-
nativeUpdate: {
86-
args: ["update"],
87-
isCommandPath: isCodexStandaloneCommandPath,
88-
},
89-
});
83+
/**
84+
* `codex update` replaces the standalone tree under `CODEX_HOME`, so the
85+
* updater must run with this instance's home or it would update `~/.codex`
86+
* and leave the instance's binary behind.
87+
*/
88+
function makeCodexMaintenanceResolver(homePath: string) {
89+
const resolvedHomePath = homePath ? expandHomePath(homePath) : undefined;
90+
return makePackageManagedProviderMaintenanceResolver({
91+
provider: DRIVER_KIND,
92+
npmPackageName: "@openai/codex",
93+
nativeUpdate: {
94+
args: ["update"],
95+
isCommandPath: isCodexStandaloneCommandPath,
96+
...(resolvedHomePath ? { env: { CODEX_HOME: resolvedHomePath } } : {}),
97+
},
98+
});
99+
}
90100

91101
/**
92102
* Services the driver needs to materialize an instance. Surfaced as the
@@ -151,10 +161,13 @@ export const CodexDriver: ProviderDriver<CodexSettings, CodexDriverEnv> = {
151161
homePath: homeLayout.effectiveHomePath ?? "",
152162
} satisfies CodexSettings;
153163
const resolveMaintenance = yield* makeCachedProviderMaintenanceResolution(
154-
resolveProviderMaintenanceCapabilitiesEffect(UPDATE, {
155-
binaryPath: effectiveConfig.binaryPath,
156-
env: processEnv,
157-
}).pipe(
164+
resolveProviderMaintenanceCapabilitiesEffect(
165+
makeCodexMaintenanceResolver(effectiveConfig.homePath),
166+
{
167+
binaryPath: effectiveConfig.binaryPath,
168+
env: processEnv,
169+
},
170+
).pipe(
158171
Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
159172
Effect.provideService(FileSystem.FileSystem, fileSystem),
160173
Effect.provideService(Path.Path, pathService),

‎apps/server/src/provider/providerMaintenance.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
homebrewOwnershipFromCommandPath,
2020
makeCachedProviderMaintenanceResolution,
2121
makePackageManagedProviderMaintenanceResolver,
22+
makeProviderMaintenanceCapabilities,
2223
normalizeCommandPath,
2324
npmGlobalPrefixFromCommandPath,
2425
parseHomebrewLatestVersion,
@@ -507,6 +508,59 @@ it.layer(NodeServices.layer)("providerMaintenance", (it) => {
507508
}),
508509
);
509510

511+
it("quotes copyable command words the shell would split", () => {
512+
const posix = makeProviderMaintenanceCapabilities({
513+
provider: driver("packageTool"),
514+
packageName: "@example/package-tool",
515+
updateExecutable: "npm",
516+
updateArgs: [
517+
"install",
518+
"-g",
519+
"--prefix",
520+
"/Users/Jane Doe/.npm-global",
521+
"@example/package-tool@latest",
522+
],
523+
updateLockKey: "npm-global",
524+
platform: "darwin",
525+
});
526+
expect(posix.update?.command).toBe(
527+
"npm install -g --prefix '/Users/Jane Doe/.npm-global' @example/package-tool@latest",
528+
);
529+
const windows = makeProviderMaintenanceCapabilities({
530+
provider: driver("packageTool"),
531+
packageName: null,
532+
updateExecutable: "C:\\Program Files\\Tool\\tool.exe",
533+
updateArgs: ["update"],
534+
updateLockKey: "tool",
535+
platform: "win32",
536+
});
537+
expect(windows.update?.command).toBe("& 'C:\\Program Files\\Tool\\tool.exe' update");
538+
});
539+
540+
it.effect.skipIf(windowsHost)("carries the native updater's environment into the action", () =>
541+
Effect.gen(function* () {
542+
const tempDir = yield* makeTempDir("t3-native-env");
543+
const nativePath = NodePath.join(tempDir, ".local", "bin", "native-package-tool");
544+
writeExecutable(nativePath);
545+
const resolver = makePackageManagedProviderMaintenanceResolver({
546+
provider: driver("nativePackageTool"),
547+
npmPackageName: "@example/native-package-tool",
548+
nativeUpdate: {
549+
args: ["update"],
550+
isCommandPath: isNativeTestCommandPath("/.local/bin/native-package-tool"),
551+
env: { TOOL_HOME: tempDir },
552+
},
553+
});
554+
555+
const capabilities = yield* resolveProviderMaintenanceCapabilitiesEffect(resolver, {
556+
binaryPath: nativePath,
557+
env: { PATH: "" },
558+
}).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, noSpawn));
559+
560+
expect(capabilities.update?.env).toEqual({ TOOL_HOME: tempDir });
561+
}),
562+
);
563+
510564
it("recognizes Homebrew kegs and casks from the real executable path", () => {
511565
expect(
512566
homebrewOwnershipFromCommandPath("/opt/homebrew/Cellar/claude-code@latest/2.1.0/bin/claude"),

‎apps/server/src/provider/providerMaintenance.ts‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ export interface ProviderMaintenanceCommandAction {
7272
readonly executable: string;
7373
readonly args: ReadonlyArray<string>;
7474
readonly lockKey: string;
75+
/**
76+
* Extra environment for the spawned updater, on top of the server's own.
77+
* A native updater finds its install through the same variables the
78+
* provider runs with (e.g. `CODEX_HOME`), so an instance with a custom home
79+
* must update that home and not the default one.
80+
*/
81+
readonly env?: NodeJS.ProcessEnv;
7582
}
7683

7784
/** Where the provider executable was found; every path is absolute. */
@@ -101,6 +108,8 @@ export interface PackageManagedProviderMaintenanceDefinition {
101108
readonly nativeUpdate: {
102109
readonly args: ReadonlyArray<string>;
103110
readonly isCommandPath: (commandPath: string) => boolean;
111+
/** Environment the native updater needs to target this instance's install. */
112+
readonly env?: NodeJS.ProcessEnv;
104113
} | null;
105114
}
106115

@@ -127,13 +136,18 @@ function nonEmptyString(value: unknown): string | null {
127136
* The copyable command must paste into a shell as-is, so an executable path
128137
* with spaces or quotes is quoted for the host's default shell.
129138
*/
139+
function quoteShellWord(word: string, platform: NodeJS.Platform): string {
140+
const safeWord = platform === "win32" ? /^[\w./:\\@=-]+$/ : /^[\w./:@=-]+$/;
141+
if (safeWord.test(word)) return word;
142+
return platform === "win32"
143+
? `'${word.replace(/['\u2018\u2019]/g, "$&$&")}'`
144+
: `'${word.replaceAll("'", "'\\''")}'`;
145+
}
146+
130147
function quoteUpdateExecutable(executable: string, platform: NodeJS.Platform): string {
131-
const safePath = platform === "win32" ? /^[\w./:\\-]+$/ : /^[\w./:-]+$/;
132-
if (safePath.test(executable)) return executable;
148+
const quoted = quoteShellWord(executable, platform);
133149
// Windows terminals default to PowerShell, where a quoted executable needs &.
134-
return platform === "win32"
135-
? `& '${executable.replace(/['\u2018\u2019]/g, "$&$&")}'`
136-
: `'${executable.replaceAll("'", "'\\''")}'`;
150+
return platform === "win32" && quoted !== executable ? `& ${quoted}` : quoted;
137151
}
138152

139153
export function makeProviderMaintenanceCapabilities(input: {
@@ -145,24 +159,24 @@ export function makeProviderMaintenanceCapabilities(input: {
145159
/** Shown to the user instead of `<executable> <args>`; use for a bare tool name like `brew`. */
146160
readonly updateCommand?: string;
147161
readonly platform?: NodeJS.Platform;
162+
readonly env?: NodeJS.ProcessEnv;
148163
readonly latestVersion?: string | null;
149164
}): ProviderMaintenanceCapabilities {
165+
const platform = input.platform ?? HostProcessPlatform.defaultValue();
150166
const update =
151167
input.updateExecutable === null || input.updateLockKey === null
152168
? null
153169
: {
154170
command:
155171
input.updateCommand ??
156172
[
157-
quoteUpdateExecutable(
158-
input.updateExecutable,
159-
input.platform ?? HostProcessPlatform.defaultValue(),
160-
),
161-
...input.updateArgs,
173+
quoteUpdateExecutable(input.updateExecutable, platform),
174+
...input.updateArgs.map((arg) => quoteShellWord(arg, platform)),
162175
].join(" "),
163176
executable: input.updateExecutable,
164177
args: input.updateArgs,
165178
lockKey: input.updateLockKey,
179+
...(input.env ? { env: input.env } : {}),
166180
};
167181
return {
168182
provider: input.provider,
@@ -353,6 +367,7 @@ export const resolvePackageManagedProviderMaintenance = Effect.fn(
353367
updateArgs: nativeUpdate.args,
354368
updateLockKey: `${definition.provider}-native`,
355369
platform: context.platform,
370+
...(nativeUpdate.env ? { env: nativeUpdate.env } : {}),
356371
});
357372
}
358373
if (commandPaths.some(isVitePlusGlobalCommandPath)) {

‎apps/server/src/provider/providerMaintenanceRunner.test.ts‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ function mockSpawnerLayer(
129129
handler: (
130130
command: string,
131131
args: ReadonlyArray<string>,
132+
options: { readonly env?: NodeJS.ProcessEnv | undefined },
132133
) => {
133134
readonly stdout?: string;
134135
readonly stderr?: string;
@@ -142,8 +143,11 @@ function mockSpawnerLayer(
142143
const childProcess = command as unknown as {
143144
readonly command: string;
144145
readonly args: ReadonlyArray<string>;
146+
readonly options: { readonly env?: NodeJS.ProcessEnv | undefined };
145147
};
146-
return Effect.succeed(mockHandle(handler(childProcess.command, childProcess.args)));
148+
return Effect.succeed(
149+
mockHandle(handler(childProcess.command, childProcess.args, childProcess.options)),
150+
);
147151
}),
148152
);
149153
}
@@ -307,6 +311,41 @@ describe("providerMaintenanceRunner", () => {
307311
},
308312
);
309313

314+
it.effect("spawns the updater with the environment its capabilities declare", () => {
315+
const seen: Array<NodeJS.ProcessEnv | undefined> = [];
316+
return Effect.gen(function* () {
317+
const { registry } = yield* makeRegistry(baseProvider);
318+
const updater = yield* makeTestRunner({
319+
...registry,
320+
getProviderMaintenanceCapabilitiesForInstance: (_instanceId, provider) =>
321+
Effect.succeed({
322+
...lifecycleFor(provider),
323+
update: {
324+
command: "codex update",
325+
executable: "/work/codex-home/packages/standalone/bin/codex",
326+
args: ["update"],
327+
lockKey: "codex-native",
328+
env: { CODEX_HOME: "/work/codex-home" },
329+
},
330+
}),
331+
});
332+
333+
yield* updater.updateProvider(CODEX_DRIVER);
334+
assert.deepStrictEqual(seen, [{ CODEX_HOME: "/work/codex-home" }]);
335+
}).pipe(
336+
Effect.provide(
337+
Layer.mergeAll(
338+
NonWindowsPlatform,
339+
latestVersionHttpClient("0.0.0"),
340+
mockSpawnerLayer((_command, _args, options) => {
341+
seen.push(options.env);
342+
return { stdout: "updated" };
343+
}),
344+
),
345+
),
346+
);
347+
});
348+
310349
it.effect("re-resolves ownership before running and executes the fresh command", () => {
311350
const calls: Array<{ command: string; args: ReadonlyArray<string> }> = [];
312351
const fresh: Array<boolean> = [];

‎apps/server/src/provider/providerMaintenanceRunner.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { ProviderRegistry } from "./Services/ProviderRegistry.ts";
2525
import { makeProviderMaintenanceCommandCoordinator } from "./providerMaintenanceCommandCoordinator.ts";
2626
import {
2727
enrichProviderSnapshotWithVersionAdvisory,
28+
type ProviderMaintenanceCommandAction,
2829
ProviderVersionCache,
2930
} from "./providerMaintenance.ts";
3031
import type { ProviderMaintenanceCapabilities } from "./providerMaintenance.ts";
@@ -76,6 +77,7 @@ const runProviderMaintenanceCommandWithSpawner = Effect.fn("ProviderMaintenanceR
7677
readonly spawner: ChildProcessSpawner.ChildProcessSpawner["Service"];
7778
readonly command: string;
7879
readonly args: ReadonlyArray<string>;
80+
readonly env?: NodeJS.ProcessEnv;
7981
}) {
8082
const collectCommandResult = Effect.fn("ProviderMaintenanceRunner.collectCommandResult")(
8183
function* () {
@@ -86,7 +88,12 @@ const runProviderMaintenanceCommandWithSpawner = Effect.fn("ProviderMaintenanceR
8688
// shell. On Linux/macOS (incl. the WSL backend) this is a no-op.
8789
const resolved = yield* resolveSpawnCommand(input.command, input.args);
8890
const child = yield* input.spawner
89-
.spawn(ChildProcess.make(resolved.command, resolved.args, { shell: resolved.shell }))
91+
.spawn(
92+
ChildProcess.make(resolved.command, resolved.args, {
93+
shell: resolved.shell,
94+
...(input.env ? { env: input.env, extendEnv: true } : {}),
95+
}),
96+
)
9097
.pipe(
9198
Effect.mapError(
9299
(cause) =>
@@ -209,11 +216,12 @@ export const make = Effect.fn("ProviderMaintenanceRunner.make")(function* () {
209216
const spawner = yield* ChildProcessSpawner.ChildProcessSpawner;
210217
const httpClient = yield* HttpClient.HttpClient;
211218
const versionCache = yield* ProviderVersionCache;
212-
const runMaintenanceCommand = (command: string, args: ReadonlyArray<string>) =>
219+
const runMaintenanceCommand = (update: ProviderMaintenanceCommandAction) =>
213220
runProviderMaintenanceCommandWithSpawner({
214221
spawner,
215-
command,
216-
args,
222+
command: update.executable,
223+
args: update.args,
224+
...(update.env ? { env: update.env } : {}),
217225
});
218226
const commandCoordinator = yield* makeProviderMaintenanceCommandCoordinator({
219227
makeAlreadyRunningError: () =>
@@ -367,7 +375,7 @@ export const make = Effect.fn("ProviderMaintenanceRunner.make")(function* () {
367375
);
368376
}
369377

370-
const result = yield* runMaintenanceCommand(fresh.update.executable, fresh.update.args);
378+
const result = yield* runMaintenanceCommand(fresh.update);
371379
const finishedAt = yield* nowIso;
372380
if (result.timedOut || result.exitCode !== 0) {
373381
return yield* finish(

0 commit comments

Comments
 (0)