Merge pull request #38 from pavelhov/codex/fix-windows-launcher-verification

fix: qualify Windows coordination for v0.1.5

Author: pavelhov

.github/workflows/cross-platform.yml

@@ -61,8 +61,12 @@ jobs:
         working-directory: gui
         run: bun run test
 
+      - name: Verify Windows account coordination and CLI lifecycle
+        if: matrix.platform == 'windows'
+        run: bun run test:parallel tests/codex-user-identity.test.ts tests/codex-inject-write-lock.test.ts tests/codex-composed-acceptance.test.ts
+
       - name: Test suite
-        run: bun run test
+        run: bun run test:parallel
 
       - name: CLI help smoke
         run: bun run src/cli/index.ts help

app/Sources/MenuBarUITests/main.swift

@@ -1688,9 +1688,16 @@ func runCompanionHeartbeatTests(_ runner: TestRunner) {
         heartbeat.start()
         heartbeat.start()
         heartbeat.start()
+        // Hosted runners may delay the first timer delivery beyond 120ms.
+        // Wait for actual delivery, then measure a fixed window for duplicates.
+        let firstFireDeadline = Date().addingTimeInterval(3)
+        while recorder.count == 0 && Date() < firstFireDeadline {
+            spinMainRunLoop(seconds: 0.01)
+        }
+        runner.expect(recorder.count >= 1, "the repeating timer fires before the deadline")
+        let initialCount = recorder.count
         spinMainRunLoop(seconds: 0.12)
-        let during = recorder.count
-        runner.expect(during >= 1, "the repeating timer fires at least once (got \(during))")
+        let during = recorder.count - initialCount
         // One 20ms timer over 120ms fires ~6 times; stacked duplicates would fire ~18.
         runner.expect(during <= 9, "repeated start() must not duplicate the timer (got \(during))")
         runner.equal(recorder.statuses.first, .enabled, "the sampled status is what gets reported")

docs-site/src/content/docs/guides/codex-integration.md

@@ -147,6 +147,10 @@ $CODEX_HOME/codexcommander-catalog.json
 $CODEX_HOME/models_cache.json
 ```
 
+On Windows, processes using the same Codex home coordinate writes through the effective Windows
+account's LocalAppData directory. Overriding `HOME`, `USERPROFILE`, or `LOCALAPPDATA` in a launcher
+does not move that coordination state; `CODEX_HOME` still selects the Codex files to manage.
+
 On WSL, if `CODEX_HOME` is unset and the Linux `~/.codex/config.toml` is absent, CodexCommander also
 checks for a single Windows Codex Desktop home at `/mnt/c/Users/*/.codex/config.toml`. When exactly
 one candidate exists, it uses that directory so WSL app-server mode and Windows Codex Desktop share

package.json

@@ -1,6 +1,6 @@
 {
   "name": "codexcommander",
-  "version": "0.1.4",
+  "version": "0.1.5",
   "private": true,
   "description": "CodexCommander — universal provider proxy for OpenAI Codex & Claude Code — use any LLM with Codex CLI/App/SDK and Claude Code",
   "type": "module",

src/codex/auth-api.ts

@@ -613,6 +613,12 @@ interface PoolQuotaRefreshFlight {
 
 const poolQuotaRefreshInFlight = new Map<string, Set<PoolQuotaRefreshFlight>>();
 const MAX_POOL_QUOTA_FLIGHTS = 16;
+let poolQuotaFlightJoinedForTests: (() => void) | null = null;
+
+/** Observe a real compatible-flight join without changing quota admission. */
+export function setPoolQuotaFlightJoinedForTests(observer: (() => void) | null): void {
+  poolQuotaFlightJoinedForTests = observer;
+}
 
 export class PoolQuotaProbeBusyError extends ResourceAdmissionError {
   constructor() {
@@ -803,7 +809,10 @@ async function fetchPoolAccountQuota(accountId: string, forceRefresh = false, co
       ?? flight.state.startCredentialGeneration;
     return generation !== undefined && isCodexAccountGenerationLive(accountId, generation);
   });
-  if (current) return current.promise;
+  if (current) {
+    poolQuotaFlightJoinedForTests?.();
+    return current.promise;
+  }
   if (poolQuotaFlightCount() >= MAX_POOL_QUOTA_FLIGHTS) throw new PoolQuotaProbeBusyError();
 
   const state: PoolQuotaRefreshFlight["state"] = {

src/codex/user-identity.ts

@@ -48,17 +48,30 @@ function powershellValue(expression: string): string {
       "-NoLogo",
       "-NoProfile",
       "-NonInteractive",
-      "-Command",
-      expression,
+      "-EncodedCommand",
+      Buffer.from(
+        "$ErrorActionPreference = 'Stop'; [Console]::OutputEncoding = [System.Text.Encoding]::UTF8;\n" + expression,
+        "utf16le",
+      ).toString("base64"),
     ], {
       stdin: "ignore",
       stdout: "pipe",
       stderr: "pipe",
+      timeout: 10_000,
     });
   } catch (cause) {
     refuse("Windows effective-account lookup could not start.", cause);
   }
-  if (result.exitCode !== 0) refuse("Windows effective-account lookup failed.");
+  if (result.exitCode !== 0) {
+    // Only this fixed diagnostic grammar may cross the subprocess boundary.
+    // Compiler messages and native errors can contain paths or account details.
+    const diagnostic = new TextDecoder().decode(result.stderr).match(
+      /^CCX_IDENTITY_FAILURE:(compile|token-environment|registered-folder):([A-Za-z0-9_.]{1,80}):([0-9A-F]{8})\r?$/m,
+    );
+    refuse(diagnostic
+      ? `Windows effective-account lookup failed (${diagnostic[1]}, ${diagnostic[2]}, HRESULT 0x${diagnostic[3]}).`
+      : `Windows effective-account lookup failed (exit ${Number.isSafeInteger(result.exitCode) ? result.exitCode : "unknown"}).`);
+  }
   const value = new TextDecoder().decode(result.stdout).trim();
   if (!value) refuse("Windows effective-account lookup returned an empty value.");
   return value;
@@ -143,12 +156,94 @@ function resolvePosixRuntimeRoot(uid: number): string {
 
 function resolveWindowsRuntimeRoot(identity: Extract<UserIdentity, { platform: "win32" }>): string {
   if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID.");
-  const localAppData = powershellValue(
-    "[Environment]::GetFolderPath([Environment+SpecialFolder]::LocalApplicationData)",
-  );
+  // Shell folder expansion can retain the launcher's ambient USERPROFILE.
+  // Expand the raw registered location against the OS token's environment,
+  // without consulting or mutating this process's environment.
+  const localAppData = powershellValue(String.raw`
+$stage = 'compile'
+try {
+Add-Type -TypeDefinition @'
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Runtime.InteropServices;
+using System.Security.Principal;
+using System.Text.RegularExpressions;
+using Microsoft.Win32;
+public static class CodexCommanderKnownFolders {
+  public static string Stage = "token-environment";
+  [DllImport("userenv.dll", SetLastError = true, ExactSpelling = true)]
+  [return: MarshalAs(UnmanagedType.Bool)]
+  private static extern bool CreateEnvironmentBlock(out IntPtr block, IntPtr token, [MarshalAs(UnmanagedType.Bool)] bool inherit);
+  [DllImport("userenv.dll", ExactSpelling = true)]
+  [return: MarshalAs(UnmanagedType.Bool)]
+  private static extern bool DestroyEnvironmentBlock(IntPtr block);
+  public static string LocalAppData(string expectedSid) {
+    using (WindowsIdentity identity = WindowsIdentity.GetCurrent()) {
+      if (!String.Equals(identity.User.Value, expectedSid, StringComparison.OrdinalIgnoreCase))
+        throw new InvalidOperationException("Effective account changed during environment resolution.");
+      IntPtr block = IntPtr.Zero;
+      try {
+        if (!CreateEnvironmentBlock(out block, identity.Token, false))
+          throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error());
+        var values = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+        IntPtr cursor = block;
+        while (Marshal.ReadInt16(cursor) != 0) {
+          string entry = Marshal.PtrToStringUni(cursor);
+          int separator = entry.IndexOf('=');
+          if (separator > 0) values[entry.Substring(0, separator)] = entry.Substring(separator + 1);
+          cursor = IntPtr.Add(cursor, checked((entry.Length + 1) * 2));
+        }
+        Stage = "registered-folder";
+        string path;
+        bool expand;
+        using (RegistryKey users = RegistryKey.OpenBaseKey(RegistryHive.Users, RegistryView.Registry64))
+        using (RegistryKey key = users.OpenSubKey(identity.User.Value + @"\Software\Microsoft\Windows\CurrentVersion\Explorer\User Shell Folders")) {
+          if (key == null) throw new InvalidOperationException("Registered folder key is unavailable.");
+          var kind = key.GetValueKind("Local AppData");
+          if (kind != RegistryValueKind.String && kind != RegistryValueKind.ExpandString)
+            throw new InvalidOperationException("Registered folder value is not a string.");
+          expand = kind == RegistryValueKind.ExpandString;
+          path = key.GetValue("Local AppData", null, RegistryValueOptions.DoNotExpandEnvironmentNames) as string;
+        }
+        if (String.IsNullOrWhiteSpace(path)) throw new InvalidOperationException("Registered folder is empty.");
+        var seen = new HashSet<string>(StringComparer.Ordinal);
+        for (int depth = 0; expand && path.IndexOf('%') >= 0; depth++) {
+          if (depth >= 16 || path.Length > 32767 || !seen.Add(path))
+            throw new InvalidOperationException("Registered folder expansion did not converge.");
+          bool replaced = false;
+          path = Regex.Replace(path, @"%([^%]+)%", match => {
+            string value;
+            if (!values.TryGetValue(match.Groups[1].Value, out value))
+              throw new InvalidOperationException("Registered folder references an unavailable variable.");
+            replaced = true;
+            return value;
+          });
+          if (!replaced) throw new InvalidOperationException("Registered folder has an unresolved variable.");
+        }
+        string root = Path.GetPathRoot(path);
+        if (path.Length > 32767 || path.IndexOf('\0') >= 0 || String.IsNullOrEmpty(root) || root.Length < 3 || root.EndsWith(":"))
+          throw new InvalidOperationException("Registered folder is not an absolute path.");
+        return Path.GetFullPath(path);
+      } finally {
+        if (block != IntPtr.Zero) DestroyEnvironmentBlock(block);
+      }
+    }
+  }
+}
+'@
+$stage = 'token-environment'
+[CodexCommanderKnownFolders]::LocalAppData('${identity.sid}')
+} catch {
+  if ($stage -ne 'compile') { $stage = [CodexCommanderKnownFolders]::Stage }
+  $failure = $_.Exception.GetBaseException()
+  [Console]::Error.WriteLine('CCX_IDENTITY_FAILURE:' + $stage + ':' + $failure.GetType().FullName + ':' + $failure.HResult.ToString('X8'))
+  exit 1
+}
+`);
   if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path.");
 
-  // The SID and known-folder values come from the effective token/.NET OS APIs,
+  // The SID and registered folder values come from the effective token/registry,
   // never USERPROFILE or LOCALAPPDATA. WP11 adds descriptor/reparse/ACL checks at
   // the stable-database open boundary where those checks can cover SQLite too.
   const root = resolve(localAppData, "CodexCommander", "Runtime", "v1", identity.sid.toUpperCase());

src/lib/service-files.ts

@@ -83,7 +83,8 @@ export function writePrivateServiceFile(path: string, content: string | Uint8Arr
     writeFileSync(temp, content, { encoding, mode, flag: "wx" });
     // Best effort on filesystems that do not support fsync; failure is surfaced
     // rather than silently declaring an on-disk credential durable.
-    const fd = openSync(temp, "r");
+    // Windows requires a writable handle for FlushFileBuffers (fsync).
+    const fd = openSync(temp, "r+");
     try { fsyncSync(fd); } finally { closeSync(fd); }
     if (existing) {
       const now = assertPrivateServiceFile(path);

tests/ccx-launcher-runtime.test.ts

@@ -5,14 +5,35 @@ import { createServer } from "node:net";
 import { tmpdir } from "node:os";
 import { join, resolve } from "node:path";
 import { bundledBunPath } from "../src/lib/bun-runtime";
-import { killProxy } from "../src/lib/process-control";
+import { redactSecretString } from "../src/lib/redact";
 
 const BIN_CCX = join(import.meta.dir, "..", "bin", "ccx.mjs");
 const nodeAvailable = spawnSync("node", ["--version"], {
   stdio: "ignore",
   windowsHide: true,
 }).status === 0;
 const runnable = process.platform === "win32" && nodeAvailable;
+const STARTUP_STDERR_LIMIT = 16_384;
+
+function startupStderrDiagnostic(stderr: string, env: NodeJS.ProcessEnv): string {
+  let sanitized = stderr;
+  for (const [name, value] of Object.entries(env)) {
+    if (value && /token|secret|password|credential|api_?key/i.test(name)) {
+      sanitized = sanitized.replaceAll(value, "[REDACTED]");
+    }
+  }
+  return redactSecretString(sanitized).replace(/\x1b\[[0-9;]*m/g, "").trim() || "(empty)";
+}
+
+test("launcher startup diagnostics redact labelled and inherited credentials", () => {
+  const result = startupStderrDiagnostic(
+    "startup failed: fixture-private-value\nAuthorization: Bearer fixture-bearer-value\n",
+    { CUSTOM_SECRET: "fixture-private-value" },
+  );
+  expect(result).toContain("startup failed: [REDACTED]");
+  expect(result).not.toContain("fixture-private-value");
+  expect(result).not.toContain("fixture-bearer-value");
+});
 
 type Health = {
   status: string;
@@ -155,6 +176,22 @@ function inspectWindowsProcessIdentity(pid: number): WindowsProcessIdentity | nu
   throw lastError;
 }
 
+function stopOwnedWindowsProcessTree(identity: WindowsProcessIdentity): void {
+  const current = inspectWindowsProcessIdentity(identity.pid);
+  if (!current) return;
+  if (!sameProcess(current, identity)) throw new Error("owned test process identity changed before cleanup");
+  // This is a test-owned Node launcher (or its proven Bun child), not the
+  // current home's proxy. Production killProxy correctly refuses that target.
+  const result = spawnSync("taskkill.exe", ["/PID", String(identity.pid), "/T", "/F"], {
+    stdio: "ignore",
+    timeout: 10_000,
+    windowsHide: true,
+  });
+  if (result.status !== 0 && sameProcess(inspectWindowsProcessIdentity(identity.pid), identity)) {
+    throw new Error(`owned test process tree did not stop (taskkill exit=${result.status})`);
+  }
+}
+
 function removeTree(path: string): void {
   // Windows can retain the copied executable's image handle briefly after
   // taskkill returns. Retry only transient fixture-cleanup errors, with a cap.
@@ -187,17 +224,52 @@ async function effectiveRuntime(override: string): Promise<string> {
   let primaryError: unknown;
   try {
     port = await freePort();
+    const launcherEnv = isolatedLauncherEnv(root, override);
+    // Explicit start persists routing intent, so its isolated home needs an
+    // installed config. Use static models to keep discovery out of this test.
+    writeFileSync(join(launcherEnv.CODEXCOMMANDER_HOME!, "config.json"), JSON.stringify({
+      port,
+      hostname: "127.0.0.1",
+      multiAgentGuidanceEnabled: true,
+      defaultProvider: "mock",
+      providers: {
+        mock: {
+          adapter: "openai-chat",
+          baseUrl: "http://127.0.0.1:9/v1",
+          allowPrivateNetwork: true,
+          liveModels: false,
+          models: ["test-model"],
+          defaultModel: "test-model",
+        },
+      },
+    }), { mode: 0o600 });
+    let startupStderr = "";
     launcher = spawn("node", [BIN_CCX, "start", "--port", String(port)], {
-      stdio: "ignore",
+      stdio: ["ignore", "ignore", "pipe"],
       windowsHide: true,
-      env: isolatedLauncherEnv(root, override),
+      env: launcherEnv,
+    });
+    launcher.stderr?.setEncoding("utf8");
+    launcher.stderr?.on("data", (chunk: string) => {
+      // Drain the pipe continuously, retaining only a bounded startup prefix.
+      startupStderr += chunk.slice(0, Math.max(0, STARTUP_STDERR_LIMIT - startupStderr.length));
     });
     if (!launcher.pid) throw new Error("Node launcher has no process id");
     launcherPid = launcher.pid;
     ownedLauncher = captureWindowsProcessIdentity(launcherPid);
 
     const health = await waitForHealth(port, 25_000, launcher);
-    if (!health) throw new Error("proxy did not become healthy");
+    if (!health) {
+      // Drop the last line if the capture filled, so truncation cannot expose a
+      // partial credential whose label/value boundary fell beyond the limit.
+      const stderr = startupStderr.length === STARTUP_STDERR_LIMIT
+        ? startupStderr.slice(0, Math.max(0, startupStderr.lastIndexOf("\n")))
+        : startupStderr;
+      throw new Error(
+        `proxy did not become healthy (exit=${launcher.exitCode}, signal=${launcher.signalCode}); `
+        + `startup stderr: ${startupStderrDiagnostic(stderr, launcherEnv)}`,
+      );
+    }
     const identity = windowsProcessIdentity(health.pid);
     if (!identity || identity.parentPid !== launcher.pid) {
       throw new Error("health PID is not the spawned Node launcher's direct Bun child");
@@ -218,16 +290,20 @@ async function effectiveRuntime(override: string): Promise<string> {
   if (ownedLauncher) {
     try {
       if (sameProcess(inspectWindowsProcessIdentity(ownedLauncher.pid), ownedLauncher)) {
-        killProxy(ownedLauncher.pid);
+        stopOwnedWindowsProcessTree(ownedLauncher);
         launcherTreeStopped = true;
       }
     } catch (error) {
       cleanupErrors.push(`launcher cleanup failed: ${String(error)}`);
     }
   }
-  if (!launcherTreeStopped && launcher && launcherPid && launcher.exitCode === null && launcher.signalCode === null) {
+  if (!ownedLauncher && !launcherTreeStopped && launcher && launcherPid && launcher.exitCode === null && launcher.signalCode === null) {
     try {
-      killProxy(launcherPid);
+      const identity = captureWindowsProcessIdentity(launcherPid);
+      if (launcher.exitCode !== null || launcher.signalCode !== null) {
+        throw new Error("test launcher exited before fallback cleanup identity was captured");
+      }
+      stopOwnedWindowsProcessTree(identity);
       launcherTreeStopped = true;
     } catch (error) {
       cleanupErrors.push(`launcher tree fallback failed: ${String(error)}`);
@@ -239,7 +315,7 @@ async function effectiveRuntime(override: string): Promise<string> {
   if (ownedProxy) {
     try {
       if (sameProcess(inspectWindowsProcessIdentity(ownedProxy.pid), ownedProxy)) {
-        killProxy(ownedProxy.pid);
+        stopOwnedWindowsProcessTree(ownedProxy);
       }
     } catch (error) {
       cleanupErrors.push(`proxy cleanup failed: ${String(error)}`);

tests/cli-account.test.ts

@@ -1,5 +1,6 @@
 import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, spyOn, test } from "bun:test";
 import { PassThrough, Readable } from "node:stream";
+import { fileURLToPath } from "node:url";
 import { cmdAccount, classifyAccount, formatAccountTable, type AccountDeps } from "../src/cli/account";
 import type { AccountStdin } from "../src/cli/account-api";
 import { printSubcommandUsage } from "../src/cli/help";
@@ -404,7 +405,7 @@ function stdinFrom(value: string, isTTY = false): AccountStdin {
 
 test("the login URL reaches piped stdout before the polling window (#1007)", async () => {
   const child = Bun.spawn({
-    cmd: [process.execPath, "run", new URL("./helpers/account-login-pipe-child.ts", import.meta.url).pathname],
+    cmd: [process.execPath, "run", fileURLToPath(new URL("./helpers/account-login-pipe-child.ts", import.meta.url))],
     stdout: "pipe",
     stderr: "pipe",
   });