From 960013e312970494e6eb7a3209b33491627f4316 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Sun, 16 Aug 2026 07:46:25 +0700 Subject: [PATCH 1/2] fix(service): keep an absolute POSIX sqlite home literal in POSIX service files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A generated systemd unit or launchd plist written from a Windows host carried `CODEX_SQLITE_HOME=D:\tmp\codex-sqlite-home` for an input of `/tmp/codex-sqlite-home`, while `CODEX_HOME` in the same file kept its POSIX value. One file, two variables holding the same kind of value, disagreeing with each other. `currentCodexSqliteHomeAbsolute` ends in `resolve()`, which is host-relative in both directions: on a Windows host it anchors `/tmp/x` to the current drive, and on a POSIX host it would turn `C:\data` into `/C:\data`. Neither is a path the target can use. The Windows branch above it already made this argument and preserved an already-absolute drive/UNC path; the POSIX side had no counterpart. Absolute-for-the-target paths are now preserved on both sides. A relative value still resolves, which is why the `resolve()` is there at all — a service unit has no meaningful working directory. Fixes the two long-standing Windows failures in `tests/service.test.ts` (`systemd service unit > preserves custom Codex and OpenCodex homes` and the launchd equivalent), which the reporter had been carrying as tolerated-by-name for several releases: 119 pass / 2 fail -> 123 pass / 0 fail. The added invariant test was driven red against the unfixed resolver. Note it can only fail on a Windows host: on POSIX, `resolve()` of an absolute POSIX path is identity, so this class of bug is invisible there — which is why it survived CI for so long. Closes #1786 --- src/service.ts | 21 ++++++++++++++------- tests/service.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/service.ts b/src/service.ts index 84008dfe4f..253ff00e71 100644 --- a/src/service.ts +++ b/src/service.ts @@ -9,7 +9,7 @@ import { execFileSync, execSync, spawnSync } from "node:child_process"; import { findLiveProxy, proxyIdentityAt, SERVICE_STOP_LIVENESS } from "./server/proxy-liveness"; import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmdirSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; -import { dirname, join, resolve, win32 } from "node:path"; +import { dirname, join, posix, resolve, win32 } from "node:path"; import { expandUserPath, getConfigDir, readPid, removePid, removeRuntimePort, verifyPidIdentity } from "./config"; import { loadConfig } from "./config"; import { restoreNativeCodex, restoreNativeCodexAsync } from "./codex/inject"; @@ -113,12 +113,19 @@ function currentCodexSqliteHomeAbsolute(target: "native" | "windows" = "native") const raw = process.env.CODEX_SQLITE_HOME?.trim(); if (!raw) return undefined; const expanded = expandUserPath(raw); - // Windows service artifacts can be rendered by cross-platform tests and - // repair tooling. Preserve an already-absolute drive/UNC path instead of - // anchoring it beneath the current POSIX worktree. - return target === "windows" && win32.isAbsolute(expanded) - ? win32.normalize(expanded) - : resolve(expanded); + // Service artifacts can be rendered by cross-platform tests and repair tooling, so an + // already-absolute path for the TARGET platform is preserved rather than re-anchored + // against the writing host. `resolve()` is host-relative in both directions: on a POSIX + // host it turns `C:\data` into `/C:\data`, and on a Windows host it turns `/tmp/x` + // into `D:\tmp\x` — neither is a path the target can use. A relative value still resolves, + // because a service unit has no meaningful working directory. + // + // CODEX_HOME and OPENCODEX_HOME are carried through literally, so without this the same + // generated file disagreed with itself about two variables holding the same kind of value. + if (target === "windows") { + return win32.isAbsolute(expanded) ? win32.normalize(expanded) : resolve(expanded); + } + return posix.isAbsolute(expanded) ? posix.normalize(expanded) : resolve(expanded); } function currentOpenCodexHome(): string { diff --git a/tests/service.test.ts b/tests/service.test.ts index 41c9cc77da..2bd7e9677d 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -663,6 +663,45 @@ describe("launchd service plist", () => { else process.env.OPENCODEX_API_AUTH_TOKEN = oldApiAuthToken; } }); + + // A POSIX unit must carry the literal POSIX path no matter which host writes it. The two + // cases above are where this actually bites: on a Windows host `resolve("/tmp/x")` anchors + // to the current drive and the generated file said `D:\tmp\codex-sqlite-home`, while + // CODEX_HOME beside it kept `/tmp/codex-home`. The same file disagreed with itself about two + // variables holding the same kind of value. This states the rule directly so the intent + // survives; on a POSIX host `resolve()` is identity here, so only Windows can catch it. + test("carries an absolute POSIX sqlite home into POSIX units without host anchoring", () => { + const inherited = process.env.CODEX_SQLITE_HOME; + try { + process.env.CODEX_SQLITE_HOME = "/var/lib/opencodex/codex-sqlite"; + + expect(buildPlist()).toContain( + "CODEX_SQLITE_HOME/var/lib/opencodex/codex-sqlite", + ); + expect(buildUnit()).toContain( + 'Environment="CODEX_SQLITE_HOME=/var/lib/opencodex/codex-sqlite"', + ); + } finally { + if (inherited === undefined) delete process.env.CODEX_SQLITE_HOME; + else process.env.CODEX_SQLITE_HOME = inherited; + } + }); + + // The relative case is why the resolve() is there at all: a service unit has no meaningful + // working directory, so a relative home must still be made absolute. + test("still absolutizes a relative sqlite home", () => { + const inherited = process.env.CODEX_SQLITE_HOME; + try { + process.env.CODEX_SQLITE_HOME = "relative-sqlite-home"; + const plist = buildPlist(); + + expect(plist).toContain("CODEX_SQLITE_HOME"); + expect(plist).not.toContain("relative-sqlite-home"); + } finally { + if (inherited === undefined) delete process.env.CODEX_SQLITE_HOME; + else process.env.CODEX_SQLITE_HOME = inherited; + } + }); }); describe("service lifecycle cleanup ordering", () => { From 8c6fa8132ef0add4be3248539ab842dbc47db2dc Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 16 Aug 2026 21:34:07 +0900 Subject: [PATCH 2/2] test(service): assert the relative sqlite home is actually absolutized The relative-path case only rejected the exact raw string, so it stayed green for any other non-absolute transform. Extract the emitted value per artifact format (launchd is XML, systemd is a quoted Environment= line) and assert absoluteness plus the expected terminal component. --- tests/service.test.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/service.test.ts b/tests/service.test.ts index 2bd7e9677d..174953a9bb 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, spyOn, test } from "bun:test"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { isAbsolute, join, posix, win32 } from "node:path"; import * as serviceModule from "../src/service"; import { saveConfig } from "../src/config"; import { windowsEnvIndirectBatchValue } from "../src/lib/win-paths"; @@ -695,8 +695,24 @@ describe("launchd service plist", () => { process.env.CODEX_SQLITE_HOME = "relative-sqlite-home"; const plist = buildPlist(); - expect(plist).toContain("CODEX_SQLITE_HOME"); - expect(plist).not.toContain("relative-sqlite-home"); + // Assert the emitted value is actually absolute. Rejecting only the raw string would + // stay green for any other non-absolute transform, which is the whole thing this test + // exists to catch. The two artifact formats differ, so each is extracted on its own + // terms: launchd is XML, systemd is a quoted Environment= line. + const plistValue = /CODEX_SQLITE_HOME<\/key>\s*([^<]*)<\/string>/.exec(plist)?.[1]; + expect(plistValue).toBeDefined(); + expect( + isAbsolute(plistValue!) || posix.isAbsolute(plistValue!) || win32.isAbsolute(plistValue!), + ).toBe(true); + expect(plistValue!.endsWith("relative-sqlite-home")).toBe(true); + + const unit = buildUnit(); + const unitValue = /Environment="CODEX_SQLITE_HOME=([^"]*)"/.exec(unit)?.[1]; + expect(unitValue).toBeDefined(); + expect( + isAbsolute(unitValue!) || posix.isAbsolute(unitValue!) || win32.isAbsolute(unitValue!), + ).toBe(true); + expect(unitValue!.endsWith("relative-sqlite-home")).toBe(true); } finally { if (inherited === undefined) delete process.env.CODEX_SQLITE_HOME; else process.env.CODEX_SQLITE_HOME = inherited;