Improve WSL-hosted Windows interop for process spawning, editor/file-manager launch, and browser open#170
Open
juliusmarminge wants to merge 10 commits intomainfrom
Open
Improve WSL-hosted Windows interop for process spawning, editor/file-manager launch, and browser open#170juliusmarminge wants to merge 10 commits intomainfrom
juliusmarminge wants to merge 10 commits intomainfrom
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Timeout silently treated as success in health check
- Added
version.timedOutcheck to the version probe condition and atimedOutguard at the start ofparseAuthStatusFromOutputso timed-out probes are correctly reported as errors/unknown instead of silently treated as success.
- Added
- ✅ Fixed: Exported
spawnProcessis unused outside its module- Removed the
exportkeyword fromspawnProcesssince it is only called internally byspawnPipedProcessandspawnDetachedProcesswithin the same file.
- Removed the
Or push these changes by commenting:
@cursor push 976009abbd
Preview (976009abbd)
diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts
--- a/apps/server/src/processRunner.ts
+++ b/apps/server/src/processRunner.ts
@@ -79,7 +79,7 @@
};
}
-export function spawnProcess(
+function spawnProcess(
command: string,
args: readonly string[],
options: ProcessSpawnOptions = {},
diff --git a/apps/server/src/provider/Layers/ProviderHealth.ts b/apps/server/src/provider/Layers/ProviderHealth.ts
--- a/apps/server/src/provider/Layers/ProviderHealth.ts
+++ b/apps/server/src/provider/Layers/ProviderHealth.ts
@@ -109,6 +109,14 @@
readonly authStatus: ServerProviderAuthStatus;
readonly message?: string;
} {
+ if (result.timedOut) {
+ return {
+ status: "warning",
+ authStatus: "unknown",
+ message: "Timed out while checking Codex authentication status.",
+ };
+ }
+
const lowerOutput = `${result.stdout}\n${result.stderr}`.toLowerCase();
if (
@@ -237,7 +245,7 @@
}
const version = versionProbe.success;
- if (version.code !== 0) {
+ if (version.timedOut || version.code !== 0) {
const detail = detailFromResult(version);
return {
provider: CODEX_PROVIDER,![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
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: Effect Cause mistakenly checked with instanceof Error
- Used Cause.squash to convert the Effect Cause object into a concrete error before checking instanceof Error, so actual error messages are now properly extracted.
Or push these changes by commenting:
@cursor push e3a4c867b5
Preview (e3a4c867b5)
diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts
--- a/apps/server/src/codexAppServerManager.ts
+++ b/apps/server/src/codexAppServerManager.ts
@@ -17,7 +17,7 @@
type ProviderTurnStartResult,
} from "@t3tools/contracts";
import type * as NodeServices from "@effect/platform-node/NodeServices";
-import { Effect, Exit, ServiceMap, Scope, Stream } from "effect";
+import { Cause, Effect, Exit, ServiceMap, Scope, Stream } from "effect";
import type { ChildProcessSpawner } from "effect/unstable/process";
import { makeRuntimeCommand, spawnManagedCommand } from "./processRunner";
@@ -566,8 +566,11 @@
return;
}
+ const squashed = Cause.squash(cause);
const message =
- cause instanceof Error ? cause.message : "codex app-server process errored.";
+ squashed instanceof Error
+ ? squashed.message
+ : "codex app-server process errored.";
this.updateSession(context, {
status: "error",
activeTurnId: undefined,
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Duplicated utility functions across two files
- Extracted stripWrappingQuotes, resolvePathEnvironmentVariable, resolveWindowsPathExtensions, resolveCommandCandidates, and isExecutableFile into a shared commandResolution.ts module, with both processRunner.ts and open.ts now importing from it.
- ✅ Fixed: Constructor fallback unsafely casts away Effect service requirements
- Made the services parameter required in the CodexAppServerManager constructor, removing the unsafe fallback that cast away NodeServices requirements with
as unknown as Effect.Effect<unknown, never>.
- Made the services parameter required in the CodexAppServerManager constructor, removing the unsafe fallback that cast away NodeServices requirements with
Or push these changes by commenting:
@cursor push 6b8805bc3b
Preview (6b8805bc3b)
diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts
--- a/apps/server/src/codexAppServerManager.ts
+++ b/apps/server/src/codexAppServerManager.ts
@@ -169,15 +169,9 @@
options?: Effect.RunOptions | undefined,
) => Promise<A>;
- constructor(services?: ServiceMap.ServiceMap<NodeServices.NodeServices>) {
+ constructor(services: ServiceMap.ServiceMap<NodeServices.NodeServices>) {
super();
- this.runPromise = services
- ? Effect.runPromiseWith(services)
- : ((effect, options) =>
- Effect.runPromise(
- effect as unknown as Effect.Effect<unknown, never>,
- options,
- )) as typeof this.runPromise;
+ this.runPromise = Effect.runPromiseWith(services);
}
async startSession(input: ProviderSessionStartInput): Promise<ProviderSession> {
diff --git a/apps/server/src/commandResolution.ts b/apps/server/src/commandResolution.ts
new file mode 100644
--- /dev/null
+++ b/apps/server/src/commandResolution.ts
@@ -1,0 +1,71 @@
+import { accessSync, constants, statSync } from "node:fs";
+import { extname } from "node:path";
+
+export function stripWrappingQuotes(value: string): string {
+ return value.replace(/^"+|"+$/g, "");
+}
+
+export function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string {
+ return env.PATH ?? env.Path ?? env.path ?? "";
+}
+
+export function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray<string> {
+ const rawValue = env.PATHEXT;
+ const fallback = [".COM", ".EXE", ".BAT", ".CMD"];
+ if (!rawValue) return fallback;
+
+ const parsed = rawValue
+ .split(";")
+ .map((entry) => entry.trim())
+ .filter((entry) => entry.length > 0)
+ .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`));
+ return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback;
+}
+
+export function resolveCommandCandidates(
+ command: string,
+ platform: NodeJS.Platform,
+ windowsPathExtensions: ReadonlyArray<string>,
+): ReadonlyArray<string> {
+ if (platform !== "win32") return [command];
+ const extension = extname(command);
+ const normalizedExtension = extension.toUpperCase();
+
+ if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) {
+ const commandWithoutExtension = command.slice(0, -extension.length);
+ return Array.from(
+ new Set([
+ command,
+ `${commandWithoutExtension}${normalizedExtension}`,
+ `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`,
+ ]),
+ );
+ }
+
+ const candidates: string[] = [];
+ for (const candidateExtension of windowsPathExtensions) {
+ candidates.push(`${command}${candidateExtension}`);
+ candidates.push(`${command}${candidateExtension.toLowerCase()}`);
+ }
+ return Array.from(new Set(candidates));
+}
+
+export function isExecutableFile(
+ filePath: string,
+ platform: NodeJS.Platform,
+ windowsPathExtensions: ReadonlyArray<string>,
+): boolean {
+ try {
+ const stat = statSync(filePath);
+ if (!stat.isFile()) return false;
+ if (platform === "win32") {
+ const extension = extname(filePath);
+ if (extension.length === 0) return false;
+ return windowsPathExtensions.includes(extension.toUpperCase());
+ }
+ accessSync(filePath, constants.X_OK);
+ return true;
+ } catch {
+ return false;
+ }
+}
diff --git a/apps/server/src/open.ts b/apps/server/src/open.ts
--- a/apps/server/src/open.ts
+++ b/apps/server/src/open.ts
@@ -6,11 +6,17 @@
*
* @module Open
*/
-import { accessSync, constants, statSync } from "node:fs";
-import { extname, join } from "node:path";
+import { join } from "node:path";
import { EDITORS, type EditorId, type ServerRuntimeEnvironment } from "@t3tools/contracts";
import { ServiceMap, Schema, Effect, Layer } from "effect";
+import {
+ isExecutableFile,
+ resolveCommandCandidates,
+ resolvePathEnvironmentVariable,
+ resolveWindowsPathExtensions,
+ stripWrappingQuotes,
+} from "./commandResolution";
import { spawnDetachedProcess, spawnProcessSync } from "./processRunner";
import { detectServerRuntimeEnvironment } from "./runtimeEnvironment";
@@ -72,75 +78,6 @@
}
}
-function stripWrappingQuotes(value: string): string {
- return value.replace(/^"+|"+$/g, "");
-}
-
-function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string {
- return env.PATH ?? env.Path ?? env.path ?? "";
-}
-
-function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray<string> {
- const rawValue = env.PATHEXT;
- const fallback = [".COM", ".EXE", ".BAT", ".CMD"];
- if (!rawValue) return fallback;
-
- const parsed = rawValue
- .split(";")
- .map((entry) => entry.trim())
- .filter((entry) => entry.length > 0)
- .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`));
- return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback;
-}
-
-function resolveCommandCandidates(
- command: string,
- platform: NodeJS.Platform,
- windowsPathExtensions: ReadonlyArray<string>,
-): ReadonlyArray<string> {
- if (platform !== "win32") return [command];
- const extension = extname(command);
- const normalizedExtension = extension.toUpperCase();
-
- if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) {
- const commandWithoutExtension = command.slice(0, -extension.length);
- return Array.from(
- new Set([
- command,
- `${commandWithoutExtension}${normalizedExtension}`,
- `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`,
- ]),
- );
- }
-
- const candidates: string[] = [];
- for (const extension of windowsPathExtensions) {
- candidates.push(`${command}${extension}`);
- candidates.push(`${command}${extension.toLowerCase()}`);
- }
- return Array.from(new Set(candidates));
-}
-
-function isExecutableFile(
- filePath: string,
- platform: NodeJS.Platform,
- windowsPathExtensions: ReadonlyArray<string>,
-): boolean {
- try {
- const stat = statSync(filePath);
- if (!stat.isFile()) return false;
- if (platform === "win32") {
- const extension = extname(filePath);
- if (extension.length === 0) return false;
- return windowsPathExtensions.includes(extension.toUpperCase());
- }
- accessSync(filePath, constants.X_OK);
- return true;
- } catch {
- return false;
- }
-}
-
function resolvePathDelimiter(platform: NodeJS.Platform): string {
return platform === "win32" ? ";" : ":";
}
diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts
--- a/apps/server/src/processRunner.ts
+++ b/apps/server/src/processRunner.ts
@@ -5,13 +5,19 @@
spawnSync,
type StdioOptions,
} from "node:child_process";
-import { statSync } from "node:fs";
import { extname, join } from "node:path";
import type { ServerRuntimeEnvironment } from "@t3tools/contracts";
import { Effect, Exit, Scope } from "effect";
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
+import {
+ isExecutableFile,
+ resolveCommandCandidates,
+ resolvePathEnvironmentVariable,
+ resolveWindowsPathExtensions,
+ stripWrappingQuotes,
+} from "./commandResolution";
import { detectServerRuntimeEnvironment } from "./runtimeEnvironment";
interface ProcessSpawnBaseOptions {
@@ -93,71 +99,6 @@
return runtimeEnvironment ?? detectServerRuntimeEnvironment();
}
-function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string {
- return env.PATH ?? env.Path ?? env.path ?? "";
-}
-
-function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray<string> {
- const rawValue = env.PATHEXT;
- const fallback = [".COM", ".EXE", ".BAT", ".CMD"];
- if (!rawValue) return fallback;
-
- const parsed = rawValue
- .split(";")
- .map((entry) => entry.trim())
- .filter((entry) => entry.length > 0)
- .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`));
- return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback;
-}
-
-function resolveCommandCandidates(
- command: string,
- windowsPathExtensions: ReadonlyArray<string>,
-): ReadonlyArray<string> {
- const extension = extname(command);
- const normalizedExtension = extension.toUpperCase();
-
- if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) {
- const commandWithoutExtension = command.slice(0, -extension.length);
- return Array.from(
- new Set([
- command,
- `${commandWithoutExtension}${normalizedExtension}`,
- `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`,
- ]),
- );
- }
-
- const candidates: string[] = [command];
- for (const candidateExtension of windowsPathExtensions) {
- candidates.push(`${command}${candidateExtension}`);
- candidates.push(`${command}${candidateExtension.toLowerCase()}`);
- }
- return Array.from(new Set(candidates));
-}
-
-function stripWrappingQuotes(value: string): string {
- return value.replace(/^"+|"+$/g, "");
-}
-
-function isExecutableFile(
- filePath: string,
- windowsPathExtensions: ReadonlyArray<string>,
-): boolean {
- try {
- const stat = statSync(filePath);
- if (!stat.isFile()) return false;
- const extension = extname(filePath);
- if (extension.length === 0) return false;
- if (windowsPathExtensions.length === 0) {
- return true;
- }
- return windowsPathExtensions.includes(extension.toUpperCase());
- } catch {
- return false;
- }
-}
-
function resolveEffectiveEnvironment(options: ProcessLaunchPlanOptions): NodeJS.ProcessEnv {
const env = (options.env ?? {}) as NodeJS.ProcessEnv;
if (options.inheritParentEnv === false) {
@@ -175,7 +116,7 @@
env: NodeJS.ProcessEnv,
): ResolvedWindowsCommand | null {
const windowsPathExtensions = resolveWindowsPathExtensions(env);
- const candidates = resolveCommandCandidates(command, windowsPathExtensions);
+ const candidates = resolveCommandCandidates(command, "win32", windowsPathExtensions);
const classify = (filePath: string): ResolvedWindowsCommand => {
const extension = extname(filePath).toUpperCase();
@@ -187,7 +128,7 @@
if (command.includes("/") || command.includes("\\")) {
for (const candidate of candidates) {
- if (isExecutableFile(candidate, windowsPathExtensions)) {
+ if (isExecutableFile(candidate, "win32", windowsPathExtensions)) {
return classify(candidate);
}
}
@@ -202,7 +143,7 @@
for (const pathEntry of pathEntries) {
for (const candidate of candidates) {
const candidatePath = join(pathEntry, candidate);
- if (isExecutableFile(candidatePath, windowsPathExtensions)) {
+ if (isExecutableFile(candidatePath, "win32", windowsPathExtensions)) {
return classify(candidatePath);
}
}
diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts
--- a/apps/server/src/provider/Layers/CodexAdapter.test.ts
+++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts
@@ -15,7 +15,7 @@
import { afterAll, assert, it, vi } from "@effect/vitest";
import { assertFailure } from "@effect/vitest/utils";
-import { Effect, Fiber, Layer, Option, Stream } from "effect";
+import { Effect, Fiber, Layer, Option, ServiceMap, Stream } from "effect";
import {
CodexAppServerManager,
@@ -132,7 +132,9 @@
listSessionIds: () => Effect.succeed([]),
});
-const validationManager = new FakeCodexManager();
+const validationManager = new FakeCodexManager(
+ ServiceMap.empty() as ServiceMap.ServiceMap<NodeServices.NodeServices>,
+);
const validationLayer = it.layer(
makeCodexAdapterLive({ manager: validationManager }).pipe(
Layer.provideMerge(ServerConfig.layerTest(process.cwd(), process.cwd())),
@@ -164,7 +166,9 @@
);
});
-const sessionErrorManager = new FakeCodexManager();
+const sessionErrorManager = new FakeCodexManager(
+ ServiceMap.empty() as ServiceMap.ServiceMap<NodeServices.NodeServices>,
+);
sessionErrorManager.sendTurnImpl.mockImplementation(async () => {
throw new Error("Unknown session: sess-missing");
});
@@ -204,7 +208,9 @@
);
});
-const lifecycleManager = new FakeCodexManager();
+const lifecycleManager = new FakeCodexManager(
+ ServiceMap.empty() as ServiceMap.ServiceMap<NodeServices.NodeServices>,
+);
const lifecycleLayer = it.layer(
makeCodexAdapterLive({ manager: lifecycleManager }).pipe(
Layer.provideMerge(ServerConfig.layerTest(process.cwd(), process.cwd())),
Contributor
|
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
Or push these changes by commenting: Preview (04ffe3b247)diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts
--- a/apps/server/src/processRunner.ts
+++ b/apps/server/src/processRunner.ts
@@ -24,16 +24,16 @@
shell?: boolean | string | undefined;
}
-export interface ProcessSpawnOptions extends ProcessSpawnBaseOptions {
+interface ProcessSpawnOptions extends ProcessSpawnBaseOptions {
stdio?: StdioOptions | undefined;
detached?: boolean | undefined;
}
-export interface RuntimeCommandOptions extends ChildProcess.CommandOptions {
+interface RuntimeCommandOptions extends ChildProcess.CommandOptions {
runtimeEnvironment?: ServerRuntimeEnvironment | undefined;
}
-export interface ProcessSpawnSyncOptions extends ProcessSpawnBaseOptions {
+interface ProcessSpawnSyncOptions extends ProcessSpawnBaseOptions {
stdio?: StdioOptions | undefined;
detached?: boolean | undefined;
encoding?: BufferEncoding | undefined;
@@ -90,11 +90,12 @@
};
}
-export function toRuntimeCommandOptions(
+function toRuntimeCommandOptions(
options: RuntimeCommandOptions = {},
): ChildProcess.CommandOptions {
+ const { runtimeEnvironment: _, ...rest } = options;
return {
- ...options,
+ ...rest,
shell: options.shell ?? shouldUseShell(options),
};
}
@@ -107,7 +108,7 @@
return ChildProcess.make(command, [...args], toRuntimeCommandOptions(options));
}
-export interface ManagedChildProcess {
+interface ManagedChildProcess {
readonly scope: Scope.Closeable;
readonly handle: ChildProcessSpawner.ChildProcessHandle;
}
@@ -135,7 +136,7 @@
return spawn(command, args, toSpawnOptions(options));
}
-export function spawnPipedProcess(
+function spawnPipedProcess(
command: string,
args: readonly string[],
options: Omit<ProcessSpawnOptions, "stdio" | "detached"> = {},
@@ -252,7 +253,7 @@
* wrapper, leaving the actual command running. Use `taskkill /T` to kill the
* entire process tree instead.
*/
-export function killProcessTree(
+function killProcessTree(
child: ChildProcessHandle,
options: {
runtimeEnvironment?: ServerRuntimeEnvironment | undefined;
diff --git a/apps/server/src/provider/Layers/ProviderHealth.ts b/apps/server/src/provider/Layers/ProviderHealth.ts
--- a/apps/server/src/provider/Layers/ProviderHealth.ts
+++ b/apps/server/src/provider/Layers/ProviderHealth.ts
@@ -76,6 +76,9 @@
if (stderr) return stderr;
const stdout = nonEmptyTrimmed(result.stdout);
if (stdout) return stdout;
+ if (result.code === null) {
+ return "Command was terminated before reporting an exit code.";
+ }
if (result.code !== 0) {
return `Command exited with code ${result.code}.`;
} |
- Centralize process spawning/termination in `processRunner` for consistent Windows behavior - Add runtime environment detection to handle WSL-hosted command resolution - Update open/editor/browser launch logic to prefer Linux shims and fall back to Windows executables - Expand server tests for WSL launch paths and shared process helpers
- Replace Node child process/readline handling with Effect streams and scoped handles - Wire CodexAppServerManager to adapter services for runtime-managed execution - Harden write/exit/stderr handling and close process scope on shutdown Co-authored-by: codex <codex@users.noreply.github.com>
- pass `NodeServices` service map through `CodexAdapter` and provider layer - make `CodexAppServerManager` run effects with injected services - update manager tests to initialize/dispose a NodeServices runtime per test
- squash Effect causes to surface real Codex process error messages - normalize Git process results to error on signal/null exit and keep details - treat timed-out Codex health probes/auth checks as unavailable or warning with clear messages - add tests for cause extraction, Git signal termination, and provider timeout paths
- add a shared launch-plan resolver to avoid default shell mode on Windows - resolve executables via PATH/PATHEXT and route `.cmd`/`.bat` through `cmd.exe /d /s /c` - update runtime/spawn helpers and add focused tests for Windows + WSL behavior
- Extract shared command/path executable resolution into `commandResolution` - Resolve Windows relative launchers from `cwd` and fix batch argument quoting - Use managed Effect runtime lifecycle in `CodexAppServerManager` - Surface signal-based provider health failures clearly and expand tests - Exclude `*.browser.ts(x)` from web TypeScript project
- Remove tests from the quality job to keep lint/typecheck/build focused - Add a dedicated test job that runs on both Linux and Windows runners
juliusmarminge
force-pushed
the
t3code/improve-windows-wsl-interop
branch
from
1b59130 to
4119c72
Compare
- make fake codex test launcher cross-platform (`.cmd` + Node script, PATH delimiter aware) - stabilize git/integration temp repo behavior (`core.autocrlf=false`, retrying `rmSync` cleanup) - remove OS-fragile assertions in CLI, keybindings, and terminal manager tests
Contributor
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: Detached spawn never removes unused event listener
- Each handler now removes the other listener via
child.removeListener()before settling the promise, preventing unnecessary closure retention on long-lived detached processes.
- Each handler now removes the other listener via
Or push these changes by commenting:
@cursor push 30035f91dd
Preview (30035f91dd)
diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts
--- a/apps/server/src/processRunner.ts
+++ b/apps/server/src/processRunner.ts
@@ -364,14 +364,18 @@
});
const handleSpawn = () => {
+ child.removeListener("error", handleError);
child.unref();
resolve();
};
+ const handleError = (error: Error) => {
+ child.removeListener("spawn", handleSpawn);
+ reject(normalizeSpawnError(command, args, error));
+ };
+
child.once("spawn", handleSpawn);
- child.once("error", (error) => {
- reject(normalizeSpawnError(command, args, error));
- });
+ child.once("error", handleError);
});
}- Launch Windows .bat/.cmd wrappers directly with `shell` set to COMSPEC to preserve argument/path handling (including spaced paths) - Update process runner tests to cover the new Windows launch behavior - Add `removeDirectoryBestEffort` and use it in integration/tests to reduce flaky temp-dir cleanup failures
Contributor
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: Scope never closed on unexpected process exit
- Changed both onFailure and onSuccess exit handlers from Effect.sync to Effect.suspend, returning Scope.close(context.scope, Exit.void).pipe(Effect.ignore) after the sync cleanup work so the scope is properly closed on unexpected process exit.
Or push these changes by commenting:
@cursor push be9ec914b2
Preview (be9ec914b2)
diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts
--- a/apps/server/src/codexAppServerManager.ts
+++ b/apps/server/src/codexAppServerManager.ts
@@ -575,9 +575,9 @@
yield* Effect.forkScoped(
Effect.matchCauseEffect(context.child.exitCode, {
onFailure: (cause) =>
- Effect.sync(() => {
+ Effect.suspend(() => {
if (context.stopping) {
- return;
+ return Effect.void;
}
const message = messageFromCodexProcessCause(cause);
@@ -588,11 +588,12 @@
});
this.emitErrorEvent(context, "process/error", message);
this.sessions.delete(context.session.sessionId);
+ return Scope.close(context.scope, Exit.void).pipe(Effect.ignore);
}),
onSuccess: (code) =>
- Effect.sync(() => {
+ Effect.suspend(() => {
if (context.stopping) {
- return;
+ return Effect.void;
}
const message = `codex app-server exited (code=${code}).`;
@@ -603,6 +604,7 @@
});
this.emitLifecycleEvent(context, "session/exited", message);
this.sessions.delete(context.session.sessionId);
+ return Scope.close(context.scope, Exit.void).pipe(Effect.ignore);
}),
}),
);- Guard writes when app-server stdin is unavailable - Close session scopes reliably on unexpected exits and during stopAll - Stream stderr by line and make managed command spawn uninterruptible - Fix explorer.exe WSL path translation to preserve valid Windows targets
| export const ServerProviderStatuses = Schema.Array(ServerProviderStatus); | ||
| export type ServerProviderStatuses = typeof ServerProviderStatuses.Type; | ||
|
|
||
| export const ServerRuntimePlatform = Schema.Literals(["windows", "linux", "macos"]); |
Contributor
There was a problem hiding this comment.
Schema.Literals called with array instead of spread arguments
High Severity
Schema.Literals is called with a single array argument like Schema.Literals(["windows", "linux", "macos"]) instead of spread arguments like Schema.Literals("windows", "linux", "macos"). The Effect Schema.Literals API expects variadic literal arguments, not an array wrapper. Wrapping them in an array creates a schema that accepts ["windows", "linux", "macos"] as a single tuple literal value, rather than accepting any one of the three string literals individually. This affects ServerRuntimePlatform, ServerPathStyle, and ServerWindowsInteropMode, making runtime validation and type inference incorrect.
Additional Locations (2)
Contributor
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
In effect-smol (the version used by this project), Schema.Literals is defined as function Literals<const L extends ReadonlyArray<AST.LiteralValue>>(literals: L) — it takes a single array argument, not spread/variadic arguments, so the code is correct.
| kind: "exit", | ||
| code, | ||
| }), | ||
| ), |
Contributor
There was a problem hiding this comment.
Exit code type not converted from branded ExitCode
Medium Severity
In attachProcessListeners, the onSuccess handler for context.child.exitCode passes the raw code value to handleUnexpectedProcessExit which expects code: number. The previous implementation explicitly converted via Effect.map((value) => Number(value)). The Effect ChildProcessSpawner exit code is a branded ExitCode type; while it may work as a plain number at runtime, the old code's explicit Number() conversion was removed, which could cause issues if the branded type's runtime representation ever changes.
Contributor
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
ExitCode is defined as Brand.Branded<number, "ExitCode"> which extends number and is directly assignable to the code: number parameter without explicit conversion; the typecheck passes cleanly.
|
@juliusmarminge Thanks for working on this. Codex App's WSL implementation currently doesn't work on my computer, so I thought maybe I would explain my WSL usage in the hopes it can help you build a better app. There are three major ways I use code harnesses on windows:
Critically, the first two options can be the same project because you may need a utility on you windows side like Chrome Connector for Claude, which only works on Windows side, or perhaps on WSL side because you want to symbolically link through pnpm. The optimal design is to create project-based settings that auto-routes to a terminal for each project, such that I can open the same project on both sides while still being in the One major thing to watch out for is links and paths. There are helpful utilities to convert paths like |
github-actions
Bot
added
the
vouch:trusted
|
will this be merged? |
unwarlikepoodle
pushed a commit
to tesseracode/t3code
that referenced
this pull request
Apr 23, 2026
Research phase complete for windows-wsl-support: - Desktop spawning flow analyzed - Upstream issues reviewed (pingdotgg#671, pingdotgg#170, pingdotgg#716, pingdotgg#870) - VS Code remote pattern validated - Component analysis complete - Full plan at .claude/plans/ Phase 0 (BackendTarget abstraction) ready to implement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Apr 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
processRunnerhelpers and use them across Codex app-server, git execution, and detached launches--gototargets)resolveBrowserLaunchfallback logic (wslviewthenexplorer.exe) and wire it into server open behaviorTesting
apps/server/src/open.test.ts: verified WSL-hosted editor/file-manager/browser resolution and path translation scenariosapps/server/src/processRunner.test.ts: verified shared spawn strategy forrunProcess, sync spawn, and detached spawnapps/server/src/runtimeEnvironment.test.ts: verified runtime environment detection behaviorapps/server/src/terminal/Layers/Manager.test.ts: updated/added coverage for terminal launch behaviorapps/server/src/provider/Layers/ProviderHealth.test.ts: updated provider health behavior coverageapps/server/src/wsServer.test.ts: updated websocket path coveragebun lint: Not runbun typecheck: Not runNote
High Risk
High risk because it refactors core process spawning/lifecycle management (Codex app-server, git, provider health, terminal, open/browser) and changes the server config contract to include runtime-environment data, which can impact cross-platform behavior and shutdown semantics.
Overview
Improves Windows/WSL interoperability by introducing
detectServerRuntimeEnvironmentand using it to choose editor/file-manager/browser launch commands (including WSL-hosted fallbacks likewslview/explorer.exe) and to translate WSL paths for Windows executables.Centralizes process execution behind an expanded
processRunner(runtime-aware launch planning, batch-vs-native Windows handling, safer tree-kill, detached/sync helpers) and migrates multiple call sites (CodexAppServerManager,GitService, Codex text generation, provider health) off ad-hocspawn/ChildProcessSpawnerusage.Tightens lifecycle/cleanup and tests: Codex app-server sessions are now scope-managed with better pending-request rejection on exits, temp dir cleanup uses
removeDirectoryBestEffort, CI splits unit tests into a Linux+Windows matrix, and websocketserverGetConfignow returnsruntimeEnvironment(with contracts updated accordingly).Written by Cursor Bugbot for commit aae0832. This will update automatically on new commits. Configure here.
Note
Unify process spawning and WSL-hosted Windows interop across
apps/serverby routing editor/file-manager launches and browser opens throughprocessRunner.runProcessandopen.resolveEditorLaunch, and expose detected runtime environment viawsServer.createServerIntroduce a shared command resolution and spawn plan with
processRunner.resolveProcessLaunchPlanandprocessRunner.runProcess, add WSL-aware runtime detection viaruntimeEnvironment.detectServerRuntimeEnvironment, refactor open flows to useopen.resolveEditorLaunchwith Windows path translation, and includeruntimeEnvironmentinWS_METHODS.serverGetConfigfrom wsServer.ts.📍Where to Start
Start with runtime detection in runtimeEnvironment.ts, then review the shared spawning plan in processRunner.ts, and the WSL-aware open logic in open.ts.
Macroscope summarized aae0832.