refactor: resolve host process state through Effect#2959
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: WoA heuristics ignore build platform
- Added an optional buildPlatform parameter to resolveHostProcessArch and passed the build platform from getDefaultBuildArch, so WoA env var heuristics are skipped when the target platform is not Windows.
Or push these changes by commenting:
@cursor push c3131c09e1
Preview (c3131c09e1)
diff --git a/scripts/lib/build-target-arch.test.ts b/scripts/lib/build-target-arch.test.ts
--- a/scripts/lib/build-target-arch.test.ts
+++ b/scripts/lib/build-target-arch.test.ts
@@ -83,7 +83,7 @@
it.effect("does not apply Windows host env heuristics for non-Windows targets", () =>
Effect.gen(function* () {
const arch = yield* getDefaultBuildArch("linux", { archChoices: ["x64", "arm64"] }).pipe(
- withHostRuntime("linux", "x64", {
+ withHostRuntime("win32", "x64", {
PROCESSOR_ARCHITECTURE: "AMD64",
PROCESSOR_ARCHITEW6432: "ARM64",
}),
diff --git a/scripts/lib/build-target-arch.ts b/scripts/lib/build-target-arch.ts
--- a/scripts/lib/build-target-arch.ts
+++ b/scripts/lib/build-target-arch.ts
@@ -40,13 +40,19 @@
const optionToUndefined = <A>(value: Option.Option<A>): A | undefined =>
Option.getOrUndefined(value);
-export const resolveHostProcessArch = Effect.fn("resolveHostProcessArch")(function* () {
+export const resolveHostProcessArch = Effect.fn("resolveHostProcessArch")(function* (
+ buildPlatform?: BuildPlatform,
+) {
const platform = yield* HostProcessPlatform;
const processArch = yield* HostProcessArchitecture;
if (processArch === "arm64") return "arm64";
if (processArch === "x64") {
if (platform !== "win32") return "x64";
+ // Only apply WoA heuristics when building for Windows (or when no build
+ // platform is specified, e.g. bare host-arch queries).
+ if (buildPlatform !== undefined && buildPlatform !== "win") return "x64";
+
// On Windows-on-Arm, x64 Node/Bun can run under emulation while the host
// still reports ARM64 via the processor environment variables.
const env = yield* WindowsProcessorArchitectureConfig;
@@ -63,7 +69,7 @@
platform: BuildPlatform,
platformConfig: PlatformConfig,
) {
- const hostArch = yield* resolveHostProcessArch();
+ const hostArch = yield* resolveHostProcessArch(platform);
if (hostArch && platformConfig.archChoices.includes(hostArch)) {
return hostArch;
}You can send follow-ups to the cloud agent here.
ApprovabilityVerdict: Needs human review Major infrastructure refactor introducing injectable Effect references for host process state across ~60 files. The shell command resolution changes add new Windows argument escaping behavior that affects runtime spawning. Warrants human review due to scope and behavioral changes in core command execution paths. You can customize Macroscope's approvability policy. Learn more. |
|
🚀 Expo continuous deployment is ready!
|
0e3f530 to
761cc7f
Compare
761cc7f to
d3e884f
Compare
d3e884f to
5744aa5
Compare
Dismissing prior approval to re-evaluate 5744aa5
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Editor check ignores host env
- Replaced isCommandAvailable() (which uses process.platform/process.env) with isCommandAvailableForPlatform() using the yielded HostProcessPlatform and HostProcessEnv values, making the availability check consistent with the spawn behavior.
Or push these changes by commenting:
@cursor push 26a4a08a64
Preview (26a4a08a64)
diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts
--- a/apps/server/src/process/externalLauncher.ts
+++ b/apps/server/src/process/externalLauncher.ts
@@ -14,7 +14,6 @@
} from "@t3tools/contracts";
import { HostProcessEnv, HostProcessPlatform } from "@t3tools/shared/hostProcess";
import {
- isCommandAvailable,
isCommandAvailableForPlatform,
type PlatformCommandAvailabilityOptions,
} from "@t3tools/shared/shell";
@@ -347,13 +346,14 @@
export const launchEditorProcess = Effect.fn("externalLauncher.launchEditorProcess")(function* (
launch: EditorLaunch,
): Effect.fn.Return<void, ExternalLauncherError, ChildProcessSpawner.ChildProcessSpawner> {
- if (!isCommandAvailable(launch.command)) {
+ const platform = yield* HostProcessPlatform;
+ const env = yield* HostProcessEnv;
+
+ if (!isCommandAvailableForPlatform(launch.command, { platform, env })) {
return yield* new ExternalLauncherError({
message: `Editor command not found: ${launch.command}`,
});
}
-
- const platform = yield* HostProcessPlatform;
const isWin32 = platform === "win32";
yield* launchAndUnref(
{You can send follow-ups to the cloud agent here.
5744aa5 to
8857505
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Terminal drops non-whitelisted env
- Restored HostProcessEnv Context.Reference in hostProcess.ts and used it as the base for createTerminalSpawnEnv so spawned terminals inherit the full host process.env (minus blocklisted keys), while keeping readTerminalHostEnv for shell resolution.
Or push these changes by commenting:
@cursor push d2667f4b40
Preview (d2667f4b40)
diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts
--- a/apps/server/src/terminal/Layers/Manager.ts
+++ b/apps/server/src/terminal/Layers/Manager.ts
@@ -10,7 +10,7 @@
type TerminalSummary,
} from "@t3tools/contracts";
import { makeKeyedCoalescingWorker } from "@t3tools/shared/KeyedCoalescingWorker";
-import { HostProcessPlatform } from "@t3tools/shared/hostProcess";
+import { HostProcessEnv, HostProcessPlatform } from "@t3tools/shared/hostProcess";
import { getTerminalLabel } from "@t3tools/shared/terminalLabels";
import * as Config from "effect/Config";
import * as DateTime from "effect/DateTime";
@@ -990,6 +990,7 @@
const logsDir = options.logsDir;
const historyLineLimit = options.historyLineLimit ?? DEFAULT_HISTORY_LINE_LIMIT;
const platform = yield* HostProcessPlatform;
+ const hostEnv = yield* HostProcessEnv;
const baseEnv = yield* readTerminalHostEnv;
const shellResolver = options.shellResolver ?? (() => defaultShellResolver(platform, baseEnv));
const processRunner = yield* ProcessRunner.ProcessRunner;
@@ -1668,7 +1669,7 @@
Effect.andThen(
Effect.gen(function* () {
const shellCandidates = resolveShellCandidates(shellResolver, platform, baseEnv);
- const terminalEnv = createTerminalSpawnEnv(baseEnv, session.runtimeEnv);
+ const terminalEnv = createTerminalSpawnEnv(hostEnv, session.runtimeEnv);
const spawnResult = yield* trySpawn(shellCandidates, terminalEnv, session);
ptyProcess = spawnResult.process;
startedShell = spawnResult.shellLabel;
diff --git a/packages/shared/src/hostProcess.ts b/packages/shared/src/hostProcess.ts
--- a/packages/shared/src/hostProcess.ts
+++ b/packages/shared/src/hostProcess.ts
@@ -15,4 +15,11 @@
},
);
+export const HostProcessEnv = Context.Reference<NodeJS.ProcessEnv>(
+ "@t3tools/shared/hostProcess/HostProcessEnv",
+ {
+ defaultValue: () => process.env,
+ },
+);
+
export const isHostWindows = Effect.map(HostProcessPlatform, (platform) => platform === "win32");You can send follow-ups to the cloud agent here.
0d4ef3a to
cf151de
Compare
cf151de to
c395992
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Empty env blocks PATH fallback
- Added Effect.map after orElseSucceed in both readCommandLookupEnv definitions to detect empty env objects and fall back to process.env, ensuring the nullish coalescing in resolveCommandPathForPlatform correctly uses the host PATH.
Or push these changes by commenting:
@cursor push c445860c82
Preview (c445860c82)
diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts
--- a/apps/server/src/process/externalLauncher.ts
+++ b/apps/server/src/process/externalLauncher.ts
@@ -96,7 +96,10 @@
}).pipe(Config.map(compactEnv));
const readBrowserLaunchEnv = BrowserLaunchEnvConfig.pipe(Effect.orElseSucceed(() => ({})));
-const readCommandLookupEnv = CommandLookupEnvConfig.pipe(Effect.orElseSucceed(() => ({})));
+const readCommandLookupEnv = CommandLookupEnvConfig.pipe(
+ Effect.orElseSucceed(() => ({})),
+ Effect.map((env) => (Object.keys(env).length > 0 ? env : process.env)),
+);
function parseTargetPathAndPosition(target: string): Option.Option<TargetPathAndPosition> {
const match = TARGET_WITH_POSITION_PATTERN.exec(target);
diff --git a/apps/server/src/provider/providerMaintenance.ts b/apps/server/src/provider/providerMaintenance.ts
--- a/apps/server/src/provider/providerMaintenance.ts
+++ b/apps/server/src/provider/providerMaintenance.ts
@@ -35,7 +35,10 @@
PATHEXT: Config.string("PATHEXT").pipe(Config.option),
}).pipe(Config.map(compactEnv));
-const readCommandLookupEnv = CommandLookupEnvConfig.pipe(Effect.orElseSucceed(() => ({})));
+const readCommandLookupEnv = CommandLookupEnvConfig.pipe(
+ Effect.orElseSucceed(() => ({})),
+ Effect.map((env) => (Object.keys(env).length > 0 ? env : process.env)),
+);
export interface ProviderMaintenanceCapabilities {
readonly provider: ProviderDriverKind;You can send follow-ups to the cloud agent here.
c395992 to
1f88949
Compare
646023a to
8e368a7
Compare
b6da521 to
9899660
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
… args Review follow-ups for the host-process refactor: - terminals inherit the full host env again (blocklist semantics) instead of a Config-backed allowlist; `options.env` is the test seam - drop cmd.exe shell mode for real executables (git, ssh, tailscale, powershell.exe, node test peers) — shell-mode arg joining re-tokenizes pipes and paths with spaces; resolve `.exe` names per platform instead - add escapeWindowsShellArg/sanitizeShellModeArgs (cross-spawn style) and apply them at every spawn that keeps shell mode for npm `.cmd` shims (provider binaries, ACP agents, codex app-server, editor launches) - restore the electron-builder env copy + empty-string scrub in build-desktop-artifact (set-but-empty CSC_* vars are treated as enabled) - downgrade fixPath defects to warnings on the win32 branch so PATH hydration can never fail server startup - collapse mergeProviderInstanceEnvironmentEffect back to the sync helper and drop the duplicate getAvailableEditors alias - migrate GrokProvider (new on main) to HostProcessPlatform + sanitizer Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: codex <codex@users.noreply.github.com>
…tandalone effect-acp and effect-codex-app-server should not depend on @t3tools/shared. Remove layerCommand (the only consumer of the platform/sanitizer helpers) and have callers spawn the child themselves and hand a handle to layerChildProcess — the same shape effect-acp already uses. - CodexProvider's probe now spawns `codex app-server` itself (host platform + sanitized shell-mode args + spawn-error mapping) and builds the client via layerChildProcess - probe/example scripts read the platform from node:os like other standalone scripts - both package manifests no longer reference @t3tools/shared; lockfile again only records the deliberate @t3tools/tailscale -> shared link Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: codex <codex@users.noreply.github.com>
- inject the host platform into preview port discovery\n- avoid global platform reads in preview tests and desktop launcher\n- retain formatted Codex app-server argument handling\n\nCo-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
9899660 to
5303484
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 86eab5b
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate df7c07d
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Codex session PATH merge missing
- Added HostProcessEnvironment merge into the env passed to resolveSpawnCommand in CodexSessionRuntime, matching the existing pattern in CodexProvider's probe path so PATH/PATHEXT are available for Windows binary resolution.
Or push these changes by commenting:
@cursor push 8d74ee9a07
Preview (8d74ee9a07)
diff --git a/apps/server/src/provider/Layers/CodexSessionRuntime.ts b/apps/server/src/provider/Layers/CodexSessionRuntime.ts
--- a/apps/server/src/provider/Layers/CodexSessionRuntime.ts
+++ b/apps/server/src/provider/Layers/CodexSessionRuntime.ts
@@ -17,6 +17,7 @@
TurnId,
} from "@t3tools/contracts";
import { resolveSpawnCommand } from "@t3tools/shared/shell";
+import { HostProcessEnvironment } from "@t3tools/shared/hostProcess";
import { normalizeModelSlug } from "@t3tools/shared/model";
import * as Crypto from "effect/Crypto";
import * as DateTime from "effect/DateTime";
@@ -724,10 +725,11 @@
...options.environment,
...(resolvedHomePath ? { CODEX_HOME: resolvedHomePath } : {}),
};
+ const hostEnvironment = yield* HostProcessEnvironment;
const spawnCommand = yield* resolveSpawnCommand(
options.binaryPath,
["app-server", ...(options.appServerArgs ?? [])],
- { env },
+ { env: { ...hostEnvironment, ...env } },
);
const child = yield* spawner
.spawn(You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit df7c07d. Configure here.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>


Summary
@t3tools/shared/hostProcessreferences for host platform, architecture, and process envprocess.*accesspnpm-lock.yamlonly records the new@t3tools/tailscale -> @t3tools/sharedworkspace dependencyRelated issues
taskkill/process-finalizer flashing described there remains out of scope.az.cmd#2576 (and addresses its duplicate [Bug]: Settings : Source control Providers : Azure DevOps KO #2661):VcsProcessnow resolves Windows.cmd/.batshims through the centralized escaped spawn path, including Azure CLI (az.cmd).bun run dev:desktopfails on Windows — ChildProcess.spawn cannot resolve.cmdshims #474 and [Bug]: Nightly - OpenCode provider false "not installed / not on PATH" on Windows #2163; both are already closed by their original fixes.Validation
vp installvp install --frozen-lockfilevp checkvp run typecheckvp test scripts/lib/build-target-arch.test.ts scripts/build-desktop-artifact.test.ts packages/ssh/src/auth.test.ts packages/ssh/src/command.test.ts packages/ssh/src/tunnel.test.ts packages/tailscale/src/tailscale.test.tsNotes
vp test, but stopped it after it started collecting vendored.repostests and hit existing unrelated web test failures in saved-environment/thread-subscription/message timeline suites.process.*uses are mostly pure helper seams, terminal internals, config/bootstrap paths, test fixtures/examples, package dist output, and the shared reference defaults.Note
Medium Risk
Broad changes to process spawning and Windows quoting/shell behavior across providers and the process runner; incorrect shim resolution could break CLI launches on Windows while Effect Context wiring mistakes could mis-detect platform in edge deployments.
Overview
Adds
@t3tools/shared/hostProcess(HostProcessPlatform,HostProcessArchitecture,HostProcessHostname,HostProcessEnvironment) and an oxlint rulet3code/no-global-process-runtimeso runtime code reads the host OS through Effect Context instead ofprocess.platform/process.arch/ ambient env.resolveSpawnCommandin shared shell now picks Windows.cmd/.batshims,shellmode, and escaping;ProcessRunner,ExternalLauncher, provider probes (Claude, Codex, Cursor, Grok, OpenCode, ACP), CLI publish, and related call sites route spawns through it rather than per-fileprocess.platform === "win32"checks. Real executables likegitandpowershell.exestay off cmd shell re-tokenization where called out.Desktop Effect layers (
ElectronMenu,ElectronWindow,DesktopWindow,main) and standalone scripts usenode:osor injected platform; server paths (bootstrap, diagnostics, port scan, PATH hydration viafixPath, telemetry, environment labels**) yield platform from Context.ExternalLauncheris consolidated behind a service with Config-driven env; large pure-function test matrices shrink. Codex probing spawns vialayerChildProcessinstead oflayerCommand. Tests swapprocess.platformpatching forLayer.succeed(HostProcessPlatform, …);ProviderVersionCachereplaces a global npm version cache for tests.Reviewed by Cursor Bugbot for commit 361b3d4. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Route host process state through Effect context instead of ambient globals
HostProcessPlatform,HostProcessArchitecture,HostProcessHostname, andHostProcessEnvironmentservices inpackages/shared/src/hostProcess.tsas injectableContext.Referencevalues with defaults fromprocess.*.process.platform,process.arch,process.env, andos.hostname()across the server, desktop, SSH, terminal, and script layers with these injected services.resolveSpawnCommandinpackages/shared/src/shell.tsto centralize Windows.cmd/.batshim detection, argument escaping, and shell-mode selection; adopted by providers, runners, and the external launcher.t3code/no-global-process-runtimethat errors on directprocess.platform/process.archreads outside the host process reference file.CodexClient.layerCommandin favor of callers constructing a child process and passing it toCodexClient.layerChildProcess..cmd/.batshims are now quoted and run undercmd.exeviashell: true; previously some call sites used manual quoting or hardcoded shell flags inconsistently.Macroscope summarized 361b3d4.