From 7c23b5a93ce6f32ea4d913d0e4e9cfe3deb33b10 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:43:09 +0900 Subject: [PATCH 1/4] fix(service): bake outbound proxy env into installed service definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A service manager does not inherit the environment of the shell that installed it, and ExecStart runs /bin/sh -lc — dash on Ubuntu/WSL, which reads .profile rather than .bashrc where proxy exports usually live. A user who needs a proxy to reach the upstream therefore got a service that dialed direct: the socket was reset, the retry budget drained, and the request surfaced as 502 Provider unreachable. The same install driven through ocx codex-shim worked, because that path spawns with { ...process.env } — which is what made the report look like a WSL networking problem rather than a service-definition gap. Resolve HTTP_PROXY / HTTPS_PROXY / ALL_PROXY / NO_PROXY (either case) at install time and bake them into all three builders: the systemd unit, the launchd plist, and the Windows wrapper. Each builder already drops falsy values, so an unset key produces no assignment rather than an empty one. Only the canonical upper-case name is written, so a definition never carries two spellings of the same setting. Closes #2107. --- src/service.ts | 30 +++++++++++++++++++++++++++ tests/service.test.ts | 48 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/service.ts b/src/service.ts index 8ba2b417b1..4256820ee2 100644 --- a/src/service.ts +++ b/src/service.ts @@ -19,6 +19,7 @@ import { BUN_RUNTIME_PATH_ENV, BUN_RUNTIME_SOURCE_ENV, durableBunRuntime } from import type { BunRuntimeSource } from "./lib/bun-runtime"; import { isProcessAlive, stopProxy } from "./lib/process-control"; import { serviceApiTokenFilePath } from "./lib/service-secrets"; +import { PROXY_ENV_KEYS } from "./lib/proxy-env"; import { randomUUID } from "node:crypto"; import { ELEVATION_REQUEST_TIMEOUT_MS, @@ -404,6 +405,8 @@ export function buildPlist(): string { codexHome ? ` CODEX_HOME${plistString(codexHome)}` : null, codexSqliteHome ? ` CODEX_SQLITE_HOME${plistString(codexSqliteHome)}` : null, opencodexHome ? ` OPENCODEX_HOME${plistString(opencodexHome)}` : null, + ...resolvedProxyEnv().map(({ name, value }) => + ` ${name}${plistString(value)}`), ].filter((line): line is string => Boolean(line)).join("\n"); const command = buildServiceShellCommand(bun, cli); return ` @@ -640,6 +643,31 @@ function systemdEnvironmentAssignment(name: string, value: string | undefined): return `Environment=${systemdQuote(`${name}=${value}`)}`; } +/** + * Outbound proxy settings the installing shell had, resolved for baking into a service + * definition. + * + * A service manager does not inherit the environment of the shell that installed it, and + * `ExecStart=/bin/sh -lc` is dash on Ubuntu/WSL — login dash reads `.profile`, not + * `.bashrc`, which is where proxy exports usually live. So a user who needs a proxy to + * reach the upstream got a service that dialed direct: the socket was reset, the retry + * budget drained, and the request surfaced as `502 Provider unreachable` (#2107). The + * same install driven through `ocx codex-shim` worked, because that path spawns with + * `{ ...process.env }`. + * + * Lower-case variants are honored because curl-style tooling sets them and the runtime's + * own `applyProxyEnv` already treats both cases as equivalent. Only the canonical + * upper-case name is baked, so a definition never carries two spellings of one setting. + */ +function resolvedProxyEnv(env: NodeJS.ProcessEnv = process.env): { name: string; value: string }[] { + const resolved: { name: string; value: string }[] = []; + for (const key of PROXY_ENV_KEYS) { + const value = env[key]?.trim() || env[key.toLowerCase()]?.trim(); + if (value) resolved.push({ name: key, value }); + } + return resolved; +} + function systemdOutputTarget(value: string): string { // StandardOutput/StandardError use output specifiers such as append:/path. // Quoting the full specifier makes systemd reject it as an invalid output target. @@ -1531,6 +1559,7 @@ export function buildWindowsServiceScript(entry = cliEntry(), port = resolveServ windowsBatchSet("CODEX_HOME", process.env.CODEX_HOME?.trim(), "path"), windowsBatchSet("CODEX_SQLITE_HOME", currentCodexSqliteHomeAbsolute("windows"), "path"), windowsBatchSet("OPENCODEX_HOME", process.env.OPENCODEX_HOME?.trim(), "path"), + ...resolvedProxyEnv().map(({ name, value }) => windowsBatchSet(name, value)), windowsBatchSet("OCX_API_TOKEN_FILE", serviceApiTokenFilePath(), "path"), windowsBatchSet("OCX_SERVICE_LOG", serviceLogPath(), "path"), windowsBatchSet("OCX_BUN", bun, "path"), @@ -2430,6 +2459,7 @@ export function buildUnit(): string { codexHome, codexSqliteHome, opencodexHome, + ...resolvedProxyEnv().map(({ name, value }) => systemdEnvironmentAssignment(name, value)), ].filter((line): line is string => Boolean(line)).join("\n"); return `[Unit] Description=OpenCodex Proxy Server diff --git a/tests/service.test.ts b/tests/service.test.ts index d4e6b17465..75d570f091 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -110,6 +110,54 @@ describe("systemd service unit", () => { expect(unit).not.toContain('StandardError="append:'); }); + test("bakes outbound proxy env into the unit so the service is not cut off from upstream (#2107)", () => { + // systemd does not inherit the installing shell's environment, and ExecStart runs + // /bin/sh -lc — which is dash on Ubuntu/WSL and reads .profile, not .bashrc. A user + // whose proxy lives in the shell therefore gets a service that dials upstream direct, + // the socket is reset, and the request surfaces as 502 Provider unreachable. + const saved = { ...process.env }; + try { + process.env.HTTP_PROXY = "http://127.0.0.1:7890"; + process.env.HTTPS_PROXY = "http://127.0.0.1:7890"; + process.env.NO_PROXY = "localhost,127.0.0.1"; + delete process.env.ALL_PROXY; + + const unit = buildUnit(); + expect(unit).toContain('Environment="HTTP_PROXY=http://127.0.0.1:7890"'); + expect(unit).toContain('Environment="HTTPS_PROXY=http://127.0.0.1:7890"'); + expect(unit).toContain("NO_PROXY="); + // An unset key must not produce an empty assignment. + expect(unit).not.toContain('Environment="ALL_PROXY="'); + + const plist = buildPlist(); + expect(plist).toContain("HTTP_PROXYhttp://127.0.0.1:7890"); + expect(plist).not.toContain("ALL_PROXY"); + } finally { + for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY"]) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } + } + }); + + test("omits proxy env entirely when the installing shell has none (#2107)", () => { + const saved = { ...process.env }; + try { + for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", + "http_proxy", "https_proxy", "all_proxy", "no_proxy"]) delete process.env[key]; + + const unit = buildUnit(); + for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"]) { + expect(unit).not.toContain(`${key}=`); + } + } finally { + for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", + "http_proxy", "https_proxy", "all_proxy", "no_proxy"]) { + if (saved[key] !== undefined) process.env[key] = saved[key]; + } + } + }); + test("preserves custom Codex and OpenCodex homes", () => { const oldCodexHome = process.env.CODEX_HOME; const oldCodexSqliteHome = process.env.CODEX_SQLITE_HOME; From 3fda507490e280ca2ff34c600d3f8fa7fadc48a9 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:44:21 +0900 Subject: [PATCH 2/4] docs(devlog): carry the unclaimed-bug selection unit onto the fix branch --- .../000_investigation.md | 308 ++++++++++++++++++ .../010_ranking.md | 249 ++++++++++++++ .../020_2114_systemd_bus.md | 239 ++++++++++++++ .../030_2107_service_proxy_env.md | 93 ++++++ .../040_2108_windows_reboot_gate.md | 127 ++++++++ .../050_1587_deferred_catalog.md | 124 +++++++ .../060_1933_tray_encoding.md | 99 ++++++ .../070_sequencing.md | 113 +++++++ .../075_verification.md | 168 ++++++++++ .../080_outcome.md | 95 ++++++ 10 files changed, 1615 insertions(+) create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/000_investigation.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/010_ranking.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/020_2114_systemd_bus.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/030_2107_service_proxy_env.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/040_2108_windows_reboot_gate.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/050_1587_deferred_catalog.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/060_1933_tray_encoding.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/070_sequencing.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/075_verification.md create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/080_outcome.md diff --git a/devlog/_plan/260819_unclaimed_bug_selection/000_investigation.md b/devlog/_plan/260819_unclaimed_bug_selection/000_investigation.md new file mode 100644 index 0000000000..0c5b92dc6e --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/000_investigation.md @@ -0,0 +1,308 @@ +# 000 — Unclaimed bug issues: investigation + +Date: 2026-08-19. Scope: open `bug` issues with **no open PR planning to close +them**, for work starting after stage 3d of `260819_next_roadmap/070`. + +## Candidate derivation (re-derived live, not copied) + +Method: list every open `bug` issue, then scan every open PR's title+body for +`#NNNN` references and subtract. + +``` +open bug issues (19): + 1049 1225 1419 1527 1587 1688 1730 1852 1924 1933 + 1939 2047 2074 2092 2097 2106 2107 2108 2114 + +referenced by some open PR: + 1225(#2041) 1527(#2054) 1688(#2032) 1852(#1876) 1924(#2027) 1939(#2029) + 2047(#2056,#2062) 2074(#2082) 2092(#2099) 2097(#2101) 2106(#2112) + +first pass => UNCLAIMED (8): 1049 1419 1587 1730 1933 2107 2108 2114 +``` + +Timestamp of the derivation: `2026-08-19T11:45:42Z`. + +### Correction: reference-counting is not claim-counting (9, not 8) + +An audit lane re-derived this independently and returned PARTIAL. Nothing was +missing from the union, but **one exclusion was wrong**. + +A `#NNNN` in a PR body proves a *mention*, not an intent to close. PR **#2054** +mentions #1527 and then says, verbatim: + +> Does not close #1527. ... Refs #1527 (residual) + +Its purpose is Cursor conversation-checkpoint reuse, which relieves some of the +token pressure behind #1527 but is not the reported failure (large-context +turns collapsing or rate-limiting while direct Cursor stays healthy). The +author explicitly reserved the residual. + +**`#1527` is therefore still unclaimed. The set is 9, not 8.** + +The other ten pairs were each read individually and are strong — every one is +"Fixes/Closes/Implements #N" with a diff whose whole purpose is that issue: +#2041→1225, #2032→1688, #1876→1852, #2027→1924, #2029→1939, #2056 and #2062 +both→2047, #2082→2074, #2099→2092, #2101→2097, #2112→2106. + +**Method note worth keeping.** The cheap derivation (scan for `#NNNN`, +subtract) is a *starting* filter, not the answer. It over-excludes exactly +where an author was being honest about scope — which is the opposite of what a +triage pass should punish. Any future run of this must read the referencing PR +and ask whether it intends to close the issue. + +## Method + +Eight read-only subagent lanes, one per candidate. Each was told to read the +full issue thread, locate the responsible code in the current tree, and report +a mechanism with `file:line` — or say CANNOT-DETERMINE rather than guess. + +Two lanes had to be re-dispatched (the first batch went silent past three wait +cycles, DISPATCH-RETIRE-01). One candidate, `#1587`, ended up with two +independent lanes, which turned out to be useful: they agreed on the mechanism +and one of them produced a measurement the other did not. + +## Findings + +### #2114 — native-main 503 when systemctl exists but the user bus does not + +**Mechanism (confirmed, two independent lanes).** `inspectSystemd()` maps only +`spawnFailed` to `absent`. Any non-zero `systemctl --user show` exit becomes +`unknown`: + +``` +src/service-manager-probe.ts:267 if (shown.spawnFailed) return { kind: "absent" }; // #1612 fix +src/service-manager-probe.ts:269 if (shown.status !== 0) return unknown(...) // this bug +``` + +That `unknown` then closes native traffic for the life of the process: +`ownership-preflight.ts:155` → `ownership: "unknown"` → `server/index.ts:702` +→ `blockNativeMainStartupForUnownedServiceHome` → `auth-context.ts:313` → +`CodexMainProfileDrainingError` → 503. + +**Why #1612 missed it.** #1612 covered *spawn* failure — `systemctl` not on +PATH. Here spawn succeeds and the bus is unreachable, so the escape hatch does +not apply. + +**Regression, in two steps.** `a2e4fcf47` (2026-08-11) made +`ownership: unknown` block native-main at all; `bb45902ef` (2026-08-15, #1612) +relieved only the spawn branch. This environment has been broken since the +fence shipped. + +**Blast radius.** Linux only, Codex integration enabled. Any host where +`systemctl` is present but the user bus is not: systemd-containing Docker / +devcontainer images under tini, and plausibly WSL without `systemd=true`. +Affected users get **100% native-OpenAI failure**, not degradation. + +**Workaround.** Remove `systemctl` from the proxy's PATH — verified by the +reporter as a single-variable control. Setting `XDG_RUNTIME_DIR` does not help. + +**Evidence.** Strong. Exact stderr, exit code, single-variable isolation, and a +traced chain that matches the source line for line. The repo currently **pins +the bug**: `tests/codex-service-manager-probe.test.ts:277` asserts that +`status: 1, stderr: "Failed to connect to bus"` is `unknown`. + +### #2108 — Windows reboot leaves the native-main gate stuck + +**Mechanism (partially determined — and the lane was right to say so).** The +503 is the same process-wide fence as #2114, but the *trigger* is not logged, +so two candidates remain: + +1. **Owner ACL fail-closed.** A second `ETIMEDOUT` in the icacls hardening is + terminal (`native-main-owner.ts:272`), and `observeOwner()` settles the gate + to `owner-unavailable` and stops (`native-profile-startup.ts:227`). The ACL + module's own comment already records this exact symptom. The reporter's first + 503 is ~74s after wrapper start, past the ~60s owner budget. +2. **Probe fail-closed.** `SERVICE_PROBE_TIMEOUT_MS` is 2000ms. A + scheduler-only install still runs `sc.exe query` for WinSW; if that times out + with the WinSW assets absent, `walkWinswChain()` returns `unknown` instead of + `absent`. + +**The important structural finding:** `startServer()` takes a **one-shot** +ownership verdict and never retries it (`server/index.ts:702-710`). That is why +the gate stays closed until `ocx restart` and why waiting does not help. + +**What it is not.** The lane disproved two plausible readings: the +"did not shut down cleanly" log line is the *injection* journal +(`codex/journal.ts:209`), not the native-profile journal; and disk +`manual-recovery` residue would survive a restart, which contradicts the +reporter's restart-cures-it observation. + +**Relationship to #2114.** Same fence, different trigger. #2114 is deterministic +and restart does not help; #2108 is transient and restart does help. They share +the *unknown → permanent fence* layer, which is the reusable fix. + +### #2107 — WSL 502 after service install + +**Mechanism (confirmed, and it is not what the title suggests).** This is +**not** the #2108 gate and **not** a WSL loopback problem. Codex reached +OpenCodex fine; OpenCodex could not reach ChatGPT. + +`buildUnit()` (`service.ts:2418-2444`) bakes `OCX_SERVICE`, Bun provenance, +`PATH`, `CODEX_HOME`, `CODEX_SQLITE_HOME`, `OPENCODEX_HOME` — and **no proxy +variables**. systemd does not inherit the installing shell's environment, and +`ExecStart=/bin/sh -lc` is dash on Ubuntu WSL, which reads `.profile`, not +`.bashrc`. So a user whose proxy lives in `.bashrc` gets a service that talks +to ChatGPT directly, and the socket is reset → `fetchWithResetRetry` exhausts → +502 `Provider unreachable`. + +The distinguishing evidence is the status code itself: #2108 is **503** with +the native-main string; this is **502** with `recoveryKinds: ["connection-reset"]`. + +**Same hole in launchd and the Windows wrapper** (`service.ts:392-407`, +`1516-1533`), though Windows logon tasks often already carry user env. + +**Regression.** No — `git log -S HTTP_PROXY -- src/service.ts` is empty. This +has always been true; it only shows up when a proxy is required. + +### #1933 — Windows tray registration reported foreign/stale + +**Mechanism (confirmed).** Not a missing-file problem despite the title. The +title is a *collapsed summary string*, and the real cause is a text encoding +bug. + +`runRegistry`/`runRegistryAsync` decode `reg.exe` output with +`encoding: "utf8"` (`src/tray/windows.ts:120-125`, `335-345`). Redirected +`reg query` emits the console ACP, not UTF-8. The reporter's username is +`MötzJensen`; `ö` is `0xF6` in Windows-1252 and decodes to `U+FFFD`. The +round-trip comparison `registered === state.runCommand` then fails, and +`registrationOwned` goes false → the stale summary. + +**This is a known class with an existing fix that was never wired here.** +`decodeWindowsTextBytes` (`src/lib/windows-text.ts:75`) already solves exactly +this for `schtasks` (#1573, with a `C:\Users\Jörg` fixture). The tray reader was +missed. + +**Blast radius.** Windows users with non-ASCII in the profile path or +`OPENCODEX_HOME` on a non-UTF-8 ACP. Also blocks the GUI repair path: Install +is hidden when `tray.stale`, and uninstall refuses on a mismatched parse. + +### #1587 — routed first-turn tool catalog is 3-5x native + +**Mechanism (confirmed by two independent lanes).** `buildTools()` +(`src/responses/parser.ts:155`) never reads Codex's `defer_loading` flag. +`pushFn` and the namespace flattener copy every tool's full `parameters` into +`OcxTool`, and the flag is not on the type, so it is gone by parse time. The +routed adapters then serialize all of them (`openai-chat.ts:1197`, +`anthropic.ts:740`, `google.ts:270`). + +The native path is asymmetric **on purpose**: `openai-responses.ts:406` +preserves `defer_loading` and strips it only when a `tool_search_output` +actually loads the tool. + +**Measured, not asserted.** One lane ran this tree's real `parseRequest` +against a captured Codex Desktop catalog: + +| Sample | Deferred tools | Deferred bytes | After parse | +|---|---|---|---| +| 2026-08-12 rollout | 8 of 8 | 32,927 / 34,404 (**95.7%**) | all 8 emitted with full schemas, 32,887 bytes (~8.2k tokens), zero defer flags surviving | +| second sample | 4 namespaces / 10 tools | 24,227 bytes (~6.1k tokens) | same | + +**A caveat both lanes raised.** The headline "3-5x" is not a clean byte +multiplier: the thread's numbers compare *different tokenizers* (OpenAI vs Kimi +vs Claude) and the Opus row also carried a repo `AGENTS.md`. The mechanism is +real and measured; the exact ratio is not. + +**Regression.** No. Flattening was added 2026-06-19 so chat models could call +MCP tools; the routed path never honored deferral. + +### #1730 — Camel DeepSeek V4 Flash first-round tool call + +**Already half-fixed, and the reporter withdrew the rest.** The shared +conversion half — every converted custom tool getting a generic +`input.description`, which broke `exec` — was fixed by `ea0608611` (#1763), an +ancestor of the current head. The current tree special-cases `exec` at +`custom-tool-compat.ts:73`. + +The remaining claim (a first-round structured-tool miss) is **CANNOT-DETERMINE +as an OpenCodex defect**: there is no Camel code in the tree, the passthrough +forwards the client's `tool_choice` unchanged, and the reporter's own local +`required` patch proves the *model* will tool-call when forced — not that we +dropped a call. The reporter later attributed it to a config error (Responses +override against a Chat Completions host) and asked to close. + +**Proposed action: close as reporter-withdrawn.** Do not implement the +suggested `stream.camelai.com` + `deepseek-v4-flash` hardcode: a hostname/model +special case changing tool-selection semantics for every user of that route, +with no public contract and the reporter now opposing it. + +### #1419 — macOS Bun SIGTRAP after TLS failure + +**CANNOT-DETERMINE as our defect, and the lane was right to refuse.** The crash +is a native `EXC_BREAKPOINT` in Bun after a TLS handshake failure. +`installCrashGuards()` only hooks `unhandledRejection`/`uncaughtException` +(`crash-guard.ts:332`), so a native trap never reaches JS — which matches the +reporter seeing no `crash.log`. `unknown certificate verification error` is a +Bun string, not ours. + +**Bundled Bun is still 1.3.14** (`package.json:65`), and upstream's latest +release is still `bun-v1.3.14`, so there is nothing to bump into. #1691's Bun +1.4 train is blocked for the same reason. + +**One real, separable gap the lane found:** `ocx gui` spawns the proxy +detached and unsupervised (`cli/dispatch.ts:255`), while launchd `KeepAlive` +exists only for `ocx service`. That is a *survivability* fix we own, and it is +testable, unlike the trap. + +### #1049 — adopt pre-substrate Codex homes into the write coordinator + +**Still real; the substrate did not make it moot — it created the leftover +class.** `codexWriteCoordinationEligibility` returns `legacy-uncoordinated` +when there is no coordinator file and residue is not `clean` +(`inject-coordination.ts:46`), and inject/restore then write directly. The +specified adoption path is entirely absent: `adoption-pending` matches **zero** +times in `src/` and `tests/`, and the live schema CHECK does not include it. + +**Important for scheduling:** the lane split this into two phases and warned +that phase 2 is the invasive one — a wrong publish can corrupt the user's Codex +home, and Windows needs a real no-replace primitive rather than a POSIX +hardlink. It is also **not a field incident**: no user logs, no crash. The +`bug` label here marks a known gap, not a live failure. + +### #1527 — Cursor large-context collapse (added after the candidate correction) + +Investigated once the audit established that #2054 does not claim it. + +**What #2054 actually covers.** Process-local checkpoint reuse, so validated +linear follow-ups send `continuationMode=checkpoint` with `rootBytes=0` instead +of rebuilding history. Its own body says "Not run: #1527 large-context / 429 / +kimi-k3", and its live-transport change is capture-only. + +**Residual after it lands — five items, and they are not one bug.** + +1. `kimi-k3` premature completion at ~79-95k input: HTTP 200 with 4-36 output + tokens while direct `cursor-agent --model kimi-k3` produces ~10k at the same + scale. Not re-run on the checkpoint branch. +2. `claude-fable-5` 429 asymmetry vs direct Cursor. Unprovable either way today: + Connect does not expose `cache_read_tokens`, so usage stays estimated and + `cached_tokens: 0` cannot distinguish a cache hit from a miss. +3. **Teardown misclassification.** Normal completion never sets + `expectedClose` — only `cancelCursorRun()` does + (`src/adapters/cursor/live-transport.ts:738`) — while the abort listener + unconditionally `failAndClear("Cursor request was aborted")` (`:1157`), and + `"aborted"` is not benign (`cursor-errors.ts:74`). So a turn that already + emitted `turnEnded` still logs `turn-failed` / `expectedClose:false`. +4. First turn, restart, compaction and helper isolation still full-replay into + the 512 KiB / 192-blob envelope (`protobuf-request.ts:70`, `:68`). +5. Request-shape parity with official Cursor (e.g. `maxMode: false` at + `protobuf-request.ts:879`) is untested. + +**The useful finding:** item 3 is small, low-risk, and independently testable — +it is a misclassification in the abort listener, not a context-window mystery. +Items 1 and 2 are acceptance work that cannot start until #2054 lands, and item +2 may not be provable at all without an upstream field. + +**Evidence.** Strong that the OpenCodex Cursor path diverges from direct Cursor +and that abort classification is wrong. Partial that full replay *causes* the +429/kimi-k3 symptoms — #2054 assumes it and did not re-run the workload. + +## Cross-issue observation + +Three of the eight (#2114, #2108, and the ownership half of #1939, which +already has PR #2029) are the **same architectural fault**: a probe that cannot +answer produces `unknown`, and `unknown` is treated as permanent evidence of +foreign ownership for the life of the process. #2114 is the deterministic case, +#2108 the transient one. + +That is worth naming before ranking, because it changes what "fix #2114" means: +the cheap fix is one classification branch, but the shared fix is making the +fence retryable. They are different sizes and different risks. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/010_ranking.md b/devlog/_plan/260819_unclaimed_bug_selection/010_ranking.md new file mode 100644 index 0000000000..2bfb39bc13 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/010_ranking.md @@ -0,0 +1,249 @@ +# 010 — Ranking and selection + +Evidence: `000_investigation.md`. Criteria are stated first, then applied. + +> **Revised after audit.** Two corrections landed here: `#1527` was wrongly +> excluded from the candidate set (PR #2054 says "Does not close #1527"), and +> `#2114` is not greenfield — open PR #2029 already edits the same function and +> is `CHANGES_REQUESTED` for the exact hazard this unit rediscovered. Both are +> folded in below. + +## Criteria (stated before the ranking) + +1. **Severity of user-visible breakage.** Total loss of a path outranks + degradation, which outranks cost. +2. **Workaround availability.** A user who is stuck with no way out ranks above + one who has a documented escape. +3. **Blast radius.** Platform reach and what fraction of that platform's users + can hit it. +4. **Regression or long-standing.** A path that *used to work* and now does not + ranks above a gap that was never filled — we broke it, and someone upgraded + into it. +5. **Evidence quality.** Can a fix be written and verified from what is in the + thread today, without waiting on a reporter. +6. **Fix cost and risk.** Cheap and containable outranks invasive, at equal + severity. Risk of *causing* a worse failure counts against. + +Deliberately **not** criteria: issue age, comment count, or how loud the thread +is. Two of the strongest candidates here were filed today. + +## Ranked + +| # | Issue | Sev | Workaround | Radius | Regression | Evidence | Cost/Risk | +|---|---|---|---|---|---|---|---| +| 1 | **#2114** systemd bus | total native loss | yes, obscure | Linux containers/WSL | **yes** | strong | low, but **owned by PR #2029** | +| 2 | **#2107** proxy env | total upstream loss | yes | Linux/WSL behind a proxy | no | partial-strong | low | +| 3 | **#2108** Windows reboot | total native loss | yes, restart | Windows scheduler installs | **yes** | partial | medium | +| 4 | **#1587** deferred catalog | cost, every turn | lossy only | all platforms | no | strong+measured | medium, product risk | +| 5 | **#1933** tray encoding | feature unusable | partial | Windows non-ASCII paths | no | strong | **very low** | +| 6 | **#1527** Cursor residual | collapse at 80k+ | yes, use CLI | Cursor adapter users | no | strong on teardown, partial on cause | split: one small, rest acceptance | +| 7 | **#1049** substrate adoption | none observed | n/a | pre-substrate homes | no | strong mechanism, no incident | **high** | +| 8 | **#1419** Bun SIGTRAP | process death | yes, service | macOS + local TLS proxy | no | partial | not ours | +| 9 | **#1730** Camel | none | yes | one custom provider | no | withdrawn | close, do not patch | + +## Selected: #2114, #2107, #2108, #1587, #1933, and one slice of #1527 + +### Why #2114 is first + +It is the only candidate that scores worst-case on severity **and** regression +**and** evidence at once. A user in a systemd-containing container gets 100% +native-OpenAI failure, the path worked before `a2e4fcf47`, and the only escape +is to notice that hiding `systemctl` from PATH fixes it — which no one will +guess. + +The clincher is that the repository **currently asserts the bug is correct +behavior**: `tests/codex-service-manager-probe.test.ts:277` pins +`status: 1, "Failed to connect to bus"` → `unknown`. That test has to be +amended deliberately, which makes this a decision rather than a patch, and it +is the kind of decision that quietly ages badly if deferred. + +**But "first" means unblocking, not opening.** PR **#2029** (fixes #1939) +already edits `inspectSystemd()` and its test, and is `CHANGES_REQUESTED` +because a reviewer objected that a missing bus is not proof the unit file is +absent — the same fail-open hazard `020` independently arrived at. + +So #2114 and #1939 are one probe-policy decision with two symptoms: a refused +sync and a native 503. Ranking #2114 first is right; treating it as a fresh PR +would mean two people making the same fail-closed call in two places. + +### #1527 — added to the selected set, but only one slice + +It reached the candidate set late, so it is ranked on the same criteria rather +than grandfathered in. The residual after #2054 is five items, and they do not +share a cost: + +- **Take now:** the abort-teardown misclassification. Normal completion never + sets `expectedClose` (`live-transport.ts:738`) while the abort listener + unconditionally fails the turn (`:1157`), so a turn that already emitted + `turnEnded` still logs `turn-failed`. Small, independently testable, and + independent of #2054. +- **Defer:** the kimi-k3 collapse and the 429 asymmetry are **acceptance work** + that cannot start until #2054 lands, and the 429 half may not be provable at + all — Connect does not expose `cache_read_tokens`, so `cached_tokens: 0` + cannot distinguish a cache hit from a miss. + +Splitting it this way is the point: the issue as filed is unfixable in one +step, and one third of it is a clean small fix hiding behind two thirds that +need a live workload. + +### Why #2107 is second despite not being a regression + +Same severity class — the proxy cannot reach upstream at all — and the +mechanism is the cleanest of the eight: `buildUnit()` bakes six environment +variables and no proxy ones. It is a small, well-bounded change to a file we +own, and the same hole exists in the launchd and Windows builders, so one fix +closes three surfaces. + +It ranks below #2114 only because it is long-standing rather than a regression, +and because the affected population needs a proxy in the first place. + +### Why #2108 is third and not first + +Higher-profile platform, and the reporter is a Windows user hitting it on every +reboot. But: the trigger is **not identified** — the lane found two plausible +paths and could not distinguish them because the gate reason is never logged. + +That makes the honest first step *logging the reason*, not fixing a mechanism +we have not confirmed. It also shares the fence layer with #2114, so doing +#2114 first produces the retryable-fence groundwork this one needs. + +### Why #1587 is fourth + +It is the only candidate with a hard measurement: 95.7% of a real captured +catalog was deferred, and all of it was emitted anyway. That is a permanent tax +on every routed first turn for every user with connectors installed. + +It ranks below the three outages because it is cost rather than breakage, and +because the fix has genuine product risk in both directions: strip too much and +routed models lose plugin visibility (the #1522 class), strip too little and +nothing improves. The headline "3-5x" also does not survive scrutiny — the +thread compares three different tokenizers — so the goal should be stated in +bytes we control, not in a ratio. + +### Why #1933 is fifth despite being the cheapest + +The fix is close to trivial: route two `reg.exe` reads through +`decodeWindowsTextBytes`, which already exists and already has a +`C:\Users\Jörg` fixture from #1573. It is fifth only because the tray is not on +the request path — nobody's requests fail because of it. + +It is worth doing precisely *because* it is cheap: it closes a +known-class-missed-a-site bug, and leaving a fixed class half-applied is how +the next one gets missed too. + +## Deferred, with reasons + +## Audit challenge to this ranking, and what changed + +An audit lane argued the ranking is "wrong as a user-harm ordering — it listed +blast radius third, then let evidence and *we can patch today* pick the +winner." Three specific challenges. Two are accepted, one is not. + +### Accepted: #2108 outranks #2114 + +The lane is right. Both are total native loss and both are regressions. Windows +scheduler installs dwarf "Linux host where systemctl is present but the user +bus is not", and #2108 recurs **on every reboot** rather than once at install. +Ranking #2114 first because its trigger is known and a test pins it is +maintainer convenience dressed as impact. + +The "do #2114 first for fence groundwork" argument was also refuted separately +(see `040`): it was a preference, not a dependency. With that gone, nothing +defends the original order. + +**Revised: 1 #2108, 2 #2114, 3 #2107, 4 #1587.** Partial evidence on #2108 is +the reason its phase 1 is *logging*, not the reason to bury it at rank 3. + +### Accepted with a change of shape: #1049 returns as detection-only + +"Integrity bugs are silent; the first report is a corrupted Codex home" is a +better argument than the one this doc made. Waiting for a field incident is the +wrong posture for a data-integrity gap, and `000` already grades the mechanism +as strong. + +But the original deferral was not only about the incident count — the invasive +half can corrupt the thing it protects. Both concerns are satisfied by +splitting it: + +- **In:** phase 1, the atomic no-clobber publish for the ordinary clean + `{0,null}` row, plus refusing to write when adoption state is indeterminate. + That is detect-and-refuse; it reduces risk rather than adding it. +- **Out for now:** phase 2 (`adoption-pending` schema, the native handoff), + which is where the corruption risk lives and which needs a Windows + no-replace primitive we do not have. + +### Not accepted: drop #1933 + +The lane called #1933 "the ranking's worst trade" and would fold it into the +Windows pass. Folding it in is fine. **Dropping it is not**, and the reason is +not severity: + +`decodeWindowsTextBytes` already exists and is already wired into the service +probe. #1933 is that same class at a site that was missed. A class fix left +half-applied is how the *next* site gets missed, and the cost here is two call +sites plus a test that reuses an existing fixture. + +It also is not costless to the user: with a stale tray the GUI offers **no +repair path** (Install hidden, Uninstall refuses), so the person who hits it is +stuck without a documented manual registry edit. + +Accepting the fold: it rides the #2108 Windows pass rather than occupying its +own slot. + +### Final selection + +`#1527` has since been investigated (see `000`), and a fourth audit round found +that `#2114` is not greenfield: open PR **#2029** already edits +`inspectSystemd()` and is `CHANGES_REQUESTED` for the same fail-open hazard +`020` rediscovered. Folding both in: + +``` +1 #2108 Windows reboot gate phase 1 = log the reason; coordinate with PR #2101 +2 #2114 systemd bus UNBLOCK PR #2029 — do not open a parallel PR +3 #2107 service proxy env clean of open work +4 #1527 abort-teardown slice small, independent of #2054 +5 #1587 deferred catalog last: most contested files +6 #1049 phase 1 only atomic publish + refuse-on-indeterminate + #1933 folded into the #2108 Windows pass +``` + +Still deferred: `#1049` phase 2 (schema + native handoff, where the corruption +risk lives), `#1527`'s kimi-k3 and 429 halves (acceptance work that cannot +start until #2054 lands, and the 429 half may be unprovable while Connect hides +`cache_read_tokens`), `#1419` (upstream-blocked), `#1730` (close as withdrawn). + +**This supersedes the header table at the top of this document**, which records +the first-pass ordering before the audit rounds moved it. The table is kept +deliberately — the movement from it to here is the useful part. + +**#1049 — defer, and say why in the issue.** The mechanism is real and well +traced, but there is no field incident behind it: no user logs, no crash, no +report. Meanwhile the lane's phase 2 carries the highest risk in this entire +set — a wrong publish can corrupt a user's Codex home, and Windows needs a real +no-replace primitive rather than a POSIX hardlink. Spending that risk budget on +a gap with no observed failure, while three total-outage bugs are open, is the +wrong trade. Revisit when either a real incident arrives or the split program +has settled and there is appetite for careful substrate work. + +**#1419 — defer as upstream-blocked, keep needs-info.** The trap is inside Bun. +Bundled Bun is 1.3.14 and upstream's latest release is still 1.3.14, so there +is nothing to bump into, and the reporter never supplied the `.ips` frames that +would let us file a useful upstream issue. Do **not** weaken TLS verification to +work around it. + +One separable piece *is* ours and should be split out rather than lost: `ocx +gui` spawns the proxy detached and unsupervised while launchd `KeepAlive` only +covers `ocx service`. That is a survivability fix with a real test, and it is +worth its own small issue instead of riding a crash we cannot reproduce. + +**#1730 — close as reporter-withdrawn.** The half that was ours (`exec` losing +its description in custom-tool conversion) shipped in `ea0608611`. The remaining +claim has no OpenCodex mechanism, and the reporter attributed it to their own +Responses-vs-Chat-Completions misconfiguration and asked to close. The proposed +fix — a `stream.camelai.com` + `deepseek-v4-flash` first-round +`tool_choice: required` hardcode — would change tool-selection semantics for +every user of that route based on one host, with no public contract and the +reporter now opposing it. + +Closing it is a real outcome, not a dodge: it removes a `bug`-labelled issue +that would otherwise keep re-surfacing in triage as unclaimed. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/020_2114_systemd_bus.md b/devlog/_plan/260819_unclaimed_bug_selection/020_2114_systemd_bus.md new file mode 100644 index 0000000000..df9cbc2024 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/020_2114_systemd_bus.md @@ -0,0 +1,239 @@ +# 020 — #2114: native-main 503 when systemctl exists but the user bus does not + +**FIRST ACTION: extend or rebase open PR [#2029], do not open a parallel PR.** + +Starts after stage 3d of `260819_next_roadmap/070`. + +## This is not greenfield work — #2029 already owns this function + +An audit lane caught what the candidate filter could not: the filter asked +"does an open PR mention issue #2114", and the answer was no. It never asked +"does an open PR already edit `inspectSystemd()`", and the answer to that is +**yes**. + +``` +PR #2029 fix(probe): classify a missing user session bus as absent + reviewDecision: CHANGES_REQUESTED + files: src/service-manager-probe.ts, tests/codex-service-manager-probe.test.ts +``` + +#2029 fixes #1939 by classifying two D-Bus messages as `absent`, and +**deliberately keeps** `Failed to connect to bus` → `unknown` — which is +exactly the pin #2114 needs changed. It is blocked on a review objection that +this document independently rediscovered and wrote down as "test 3": a missing +bus is not proof that the unit file is absent, so a naive widening fails open +when a foreign unit is still on disk. + +So #2114 and #1939 are **one probe-policy decision with two user-visible +symptoms** — a refused sync (#1939) and a native 503 (#2114). Treating them as +two units means two people making the same fail-closed security-adjacent call +in two PRs, with the second one silently overwriting the first. + +**Consequence for ranking:** the work is real and still first, but it is +"unblock #2029 by supplying the containment its reviewer asked for", not "open +a new PR". The container/foreground gate below is the shape of that answer. + +## Failure mechanism + +> **STOP — read this before planning any work on this issue.** +> +> **Open PR #2029 already rewrites `inspectSystemd()`'s non-zero branch**, and +> it deliberately keeps the #2114 case as `unknown`. This doc was written as if +> that function were unowned. It is not. +> +> What #2029 actually does: +> +> ``` +> + err.includes("Failed to get D-Bus connection: No such file or directory") +> + ... "System has not been booted with systemd" +> + return { kind: "absent" }; +> + return unknown(...) // "other bus failures stay unknown" +> ``` +> +> and it adds a test that **cements the #2114 shape as `unknown`**: +> +> ``` +> + test("other bus failures stay unknown — the user manager may be running", () => { +> + stderr: "Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS not set", +> + expect(...kind).toBe("unknown"); +> ``` +> +> The #2114 reporter's stderr is +> `Failed to connect to user scope bus via local transport: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined` +> — which is exactly the family #2029 is choosing to leave closed. +> +> **Consequence: this is not a fresh patch, it is a conversation with #2029.** +> Either extend that PR's classifier to cover this stderr family, or land it and +> follow up on the same branch. Opening a competing PR means two changes fighting +> over one function, and the later one may silently re-pin the bug. +> +> This also changes the ranking: a fail-closed probe change is not "cheap" when +> an overlapping PR is already open on it. See `010`. + +``` +src/service-manager-probe.ts:267 if (shown.spawnFailed) return { kind: "absent" }; +src/service-manager-probe.ts:269-272 if (shown.status !== 0) { ... return unknown(...) } +``` + +(:269 is the `if`; the `return unknown(...)` is :272.) + +The comment on :270 says a non-zero status means "the question never reached the +bus" — which is exactly right, and is exactly why returning `unknown` is wrong. +A question that never reached the bus is evidence about the bus, not evidence +that a foreign service owns this home. + +From there the verdict is terminal for the process: + +| Step | File | +|---|---| +| `manager.kind === "unknown"` → `ownership: "unknown"` | `src/integrations/native/ownership-preflight.ts:155` | +| not `owned` → `blockNativeMainStartupForUnownedServiceHome("ownership-unknown")` | `src/server/index.ts:702-708` (:702 is the probe call, the block is :706) | +| snapshot blocked → `isNativeMainTrafficBlocked()` true | `src/codex/native-profile-startup.ts:351` | +| throws `CodexMainProfileDrainingError` | `src/codex/auth-context.ts:313`, `:318` | +| 503 `OpenCodex local native-main profile maintenance is active` | `src/codex/auth-context.ts:125-126` | + +## Why the existing #1612 fix does not cover it + +`bb45902ef` mapped **spawn** failure to `absent` — `systemctl` missing from +PATH. Here spawn succeeds and returns exit 1 with +`Failed to connect to user scope bus via local transport`. Same user-visible +outcome, different branch. + +## Fix shape + +**Primary change: one classification branch in `inspectSystemd()`.** + +Match bus-unreachable stderr specifically rather than widening every non-zero +exit, and gate the widening on an environment that already cannot host a user +service. The product already owns that signal and does not pass it to the +probe: `service.ts:3122` refuses service install when `/.dockerenv` exists and +reports `unsupported in Docker`. + +``` +if (shown.status !== 0) { + if (busUnreachable(shown.stderr) && deps.serviceHostingUnsupported()) { + return { kind: "absent" }; + } + return unknown(...) // unchanged for every other case +} +``` + +Thread the signal through `ProbeDeps` — the probe is already injectable +(`ProbeRunner`, `ProbeDeps`), so there is no call-site churn. + +### That snippet is unsafe as written — corrected + +An audit lane found the flaw and it is the important finding of this doc. +**With the bus down, `systemctl` cannot see a foreign unit either.** The +snippet returns `absent` on stderr + container signal alone, with no other +ownership evidence. A temporary bus outage inside a container that *does* host +a user service is exactly the fail-open the risk section warns about — and +test 3 below asserts a behavior the code shape cannot deliver. + +The classification must consult the **filesystem**, which does not need the +bus: + +``` +if (shown.status !== 0) { + if (!busUnreachable(shown.stderr) || !deps.serviceHostingUnsupported()) { + return unknown(...); // unchanged for every other case + } + // The bus could not answer. Ask the disk instead: a unit file is proof of + // installation that does not require a running bus. + const unit = deps.readUnitFile?.(UNIT_PATH); + if (unit === undefined) return { kind: "absent" }; // no unit, no owner + return unitOwnershipFrom(unit); // foreign stays foreign +} +``` + +`inspectSystemd()` already parses `FragmentPath` for the bus-answered path, so +the unit-file reader and the "does this unit name our home" logic exist in some +form; this reuses them on the offline path rather than inventing a second +notion of ownership. + +**Open decisions the implementer must make, which this doc cannot make for +them:** + +- `serviceHostingUnsupported()` **does not exist**. `service.ts:3122` is an + *install-time* `/.dockerenv` check. Whether the probe signal is Docker-only + or the broader "container/foreground" the risk section mentions is unset — + and it matters, because Podman and Kubernetes often have no `/.dockerenv`. +- `busUnreachable()` does not exist, and the locale policy is unchosen: match + strings, ignore stderr entirely, or force `LC_ALL=C` on the probe. +- The unit path constant and reader are not named here. + +**Stderr variants to match.** At minimum +`Failed to connect to user scope bus` and +`$DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined`. #1939 reports a +third shape, `Failed to get D-Bus connection`. Locale sensitivity is a real +weakness of string matching here and should be called out in the PR rather than +papered over; a non-English systemd will not match. If that is unacceptable, +the alternative is to key only on the container/foreground signal and ignore +stderr entirely — narrower, but locale-proof. + +**Files:** `src/service-manager-probe.ts`, `src/integrations/native/ownership-preflight.ts` +(signal plumbing only), `tests/service-probe-docker.test.ts`, +`tests/codex-service-manager-probe.test.ts`. + +**Coordination:** those are the same two files #2029 already changes. Rebase on +it or fold this into it; do not race it. + +## The test that must change, deliberately + +`tests/codex-service-manager-probe.test.ts:277` currently asserts +`status: 1, stderr: "Failed to connect to bus"` → `unknown`. **Amend it, do not +delete it**: keep that assertion for the non-container case, so the widening +stays honest. + +## Regression tests + +Red today, green after: + +1. Container signal set + `systemctl` spawn ok + exit 1 bus error + no unit + file → `{ kind: "absent" }` → `inspectNativeCodexOwnership` `owned` → + native-main not blocked. + +Must stay red (guards against over-widening): + +2. Same stderr, **no** container signal → still `unknown`. +3. Container signal + an installed unit naming a foreign home → still blocked. + +A `startServer` test injecting that probe result should return 503 today and +200 after. + +## Verification + +``` +bun test tests/service-probe-docker.test.ts tests/codex-service-manager-probe.test.ts +bun x tsc --noEmit +``` + +## Risk + +This is a **fail-closed security-adjacent boundary**. The failure mode of a bad +fix is admitting native-main on a host where a genuinely foreign unit exists but +was temporarily unqueryable. The container/foreground gate plus test 3 is what +keeps that closed. Do not widen all non-zero exits. + +**Correction:** the container gate alone does *not* keep that closed — that was +the audit's finding above. The disk check is what keeps it closed; the +container gate only limits where the offline path is taken at all. + +**Coverage limit to state plainly.** Even corrected, this fix only relieves +hosts that hit the container signal. WSL without `systemd=true`, bare SSH +sessions with no `XDG_RUNTIME_DIR`, CI runners, and Podman/k8s without +`/.dockerenv` keep returning `unknown` and keep 503-ing. If those matter, the +answer is #2108 phase 2's retryable fence, not a wider classifier here — which +is an argument for doing that work regardless of this fix. + +## Explicitly out of scope + +Two secondary bugs surfaced in the same thread. Both are real; neither should +ride this fix: + +- `ocx ready` / `/readyz` ignores `isNativeMainTrafficBlocked()` + (`src/server/index.ts:850`), so readiness can read ready while every native + request 503s. +- Codex CLI renders the local 503 as "Selected model is at capacity", because + the code is remapped to `server_is_overloaded` (`src/lib/errors.ts:229`). + The body is correct; the user-facing sentence is not. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/030_2107_service_proxy_env.md b/devlog/_plan/260819_unclaimed_bug_selection/030_2107_service_proxy_env.md new file mode 100644 index 0000000000..4a186d55b4 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/030_2107_service_proxy_env.md @@ -0,0 +1,93 @@ +# 030 — #2107: service unit drops outbound proxy env + +Rank 2. + +## Failure mechanism + +The title says "502 after service install in WSL", and both obvious readings +are wrong. Codex **did** reach OpenCodex; OpenCodex could not reach ChatGPT. + +`buildUnit()` (`src/service.ts:2418-2444`) bakes exactly `OCX_SERVICE`, Bun +provenance, `PATH`, `CODEX_HOME`, `CODEX_SQLITE_HOME`, `OPENCODEX_HOME`. No +`HTTP_PROXY` / `HTTPS_PROXY` / `ALL_PROXY` / `NO_PROXY`. systemd does not +inherit the installing shell's environment. + +`ExecStart=/bin/sh -lc` (`service.ts:457`) makes it worse in a way that is easy +to miss: on Ubuntu WSL `/bin/sh` is dash, and login dash reads `.profile`, not +`.bashrc` — where proxy exports usually live. + +`applyProxyEnv()` (`src/config.ts:3441`) only fills from `config.proxy`, so a +shell-only proxy is invisible to the service. + +Result: outbound TLS goes direct, the socket is reset, +`fetchWithResetRetry` exhausts (`src/lib/upstream-retry.ts:175`), and +`core.ts:2587` returns **502 `Provider unreachable`** with +`recoveryKinds: ["connection-reset"]`. + +## How to tell it apart from #2108 + +The status code is the discriminator, and it is worth writing down because the +two reports look identical in prose: + +| | #2107 | #2108 | +|---|---|---| +| status | **502** | **503** | +| body | `Provider unreachable` | `native-main profile maintenance is active` | +| log | `recoveryKinds: ["connection-reset"]` | native-main gate | +| cure | shim/direct start, or set `config.proxy` | `ocx restart` | + +## Why the shim works and the service does not + +`src/codex/shim.ts:692` runs `ocx ensure` in the interactive Codex shell, and +`src/cli/index.ts:431` spawns with `{ ...process.env }` — so `.bashrc` proxy +vars survive. That asymmetry is the whole bug. + +## Fix shape + +Bake the proxy keys into the generated unit, reusing what already exists: +`PROXY_ENV_KEYS` from `src/lib/proxy-env.ts` and the existing +`systemdEnvironmentAssignment()`. + +The same hole exists in `buildPlist` (`service.ts:392-407`) and the Windows +wrapper (`service.ts:1516-1533`). Fix all three in one change — they are the +same omission, and splitting them means two more reports. + +**Rules the implementation must follow:** + +- Do not emit empty `Environment=` lines for unset keys. +- Keep loopback on `NO_PROXY` the way `applyProxyEnv()` already does, or the + proxy will hairpin its own dashboard traffic. +- Do **not** switch `ExecStart` to an interactive `bash -ic`. That would fix + the symptom by making service startup depend on the user's interactive shell, + which is worse than the bug. + +**Two risks to state in the PR rather than discover later:** + +1. A WSL `WIN_HOST=$(ip route ...)` value is snapshotted at install time and can + change after `wsl --shutdown`. +2. Proxy URLs can carry credentials, and baking them writes those into a unit + file on disk. That is a privacy decision, not a detail — either redact, or + prefer `config.proxy` and document why. + +**Files:** `src/service.ts`, `tests/service.test.ts`. + +## Regression test + +With `HTTP_PROXY`/`HTTPS_PROXY`/`ALL_PROXY`/`NO_PROXY` set, `buildUnit()` +contains the matching `Environment=` lines; with them unset, those keys are +absent. Fails on current `dev`. + +## Verification + +``` +bun test tests/service.test.ts +bun x tsc --noEmit +``` + +## Documented workaround for the issue thread + +Set OpenCodex `config.proxy` — service start still runs `applyProxyEnv()`, so +this works today without any code change. `ocx doctor` already prints +"Current doctor process proxy env" vs "Running proxy process proxy env" +(`src/cli/doctor.ts:859`), which is the fastest way for a user to confirm the +diagnosis themselves. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/040_2108_windows_reboot_gate.md b/devlog/_plan/260819_unclaimed_bug_selection/040_2108_windows_reboot_gate.md new file mode 100644 index 0000000000..9b78b06300 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/040_2108_windows_reboot_gate.md @@ -0,0 +1,127 @@ +# 040 — #2108: Windows reboot leaves the native-main gate stuck + +Rank 3. **This doc deliberately does not prescribe a mechanism fix first.** + +## What is confirmed + +The 503 is the process-wide native-main fence — same layer as #2114, different +trigger. `/healthz` never consults the gate (`src/server/index.ts:813`), which +is why the reporter sees 200 health + 200 Anthropic + 503 GPT and reasonably +concludes the proxy is fine. + +The structural cause is one line of policy: + +``` +src/server/index.ts:702-710 + owned -> startNativeMainStartupLifecycle() + anything else -> blockNativeMainStartupForUnownedServiceHome(...) // for the process lifetime +``` + +**`startServer()` takes a one-shot ownership verdict and never retries it.** +That is why waiting does not help and `ocx restart` does. + +## What is NOT confirmed, and why that matters + +The investigation found two plausible triggers and could not distinguish them, +because **the settled gate reason is never logged**: + +1. **Owner ACL fail-closed.** A second `ETIMEDOUT` in icacls hardening is + terminal — it publishes `{ status: "unavailable", reason: "lock-unavailable" }` + at `src/codex/native-main-owner.ts:205-212` — and `observeOwner()` settles to + `owner-unavailable` and stops (`native-profile-startup.ts:225-236`). The ACL + module's own comment already describes this exact symptom. Timing fits: the + first 503 is ~74s after wrapper start, past the ~60s owner budget. +2. **Probe fail-closed.** `SERVICE_PROBE_TIMEOUT_MS` is 2000ms. A + scheduler-only install still runs `sc.exe query` for WinSW; if that times out + with WinSW assets absent, `walkWinswChain()` returns `unknown` rather than + `absent` (`service-manager-probe.ts:732-736`). + +**Line numbers matter here.** An earlier draft cited `native-main-owner.ts:272`, +which is `if (released) return` inside `release()` — a fixer grepping that line +lands in the wrong function entirely. + +Two readings were **disproved** and should not be re-raised: the +"did not shut down cleanly" line is the injection journal +(`src/codex/journal.ts:209`), not the native-profile journal; and disk +`manual-recovery` residue would survive `ocx restart`, which contradicts the +reporter's own observation that restart cures it. + +## Phase 1 — log the reason (do this first, alone) + +Emit the concrete gate reason when the fence settles and when the 503 is +returned (`src/codex/auth-context.ts`, `src/codex/native-profile-startup.ts`). + +This is not a placeholder task. Without it the next reboot report is exactly as +ambiguous as this one, and we will be guessing between the same two candidates. +A shipped diagnostic converts the next occurrence into evidence. + +**Test:** the 503 log line includes the settled reason. + +## Phase 2 — make boot-time `unknown` retryable (after phase 1 has data) + +Treat `unknown` that came from a *timeout or unaskable manager* as retryable +while `OCX_SERVICE=1`, instead of a process-lifetime fence. Keep genuine +`foreign` fail-closed, and keep a retry cap. + +Two narrower fixes fall out and are worth doing regardless: + +- If WinSW xml **and** exe are absent, a timed-out `sc.exe query` must not mark + the machine `unknown`. +- A second ACL `ETIMEDOUT` on the service child should back off and retry rather + than settle terminal, so a warm icacls reopens the gate without `ocx restart`. + +## Tests that are currently green and encode the bug + +| Test | Asserts today | +|---|---| +| `tests/native-main-owner-lifetime.test.ts` | second `ETIMEDOUT` → terminal `unavailable` | +| `tests/codex-service-manager-probe.test.ts` | schtasks timeout → `unknown` | +| `tests/native-profile-startup.test.ts` | `ownership-unknown` blocks for the process | + +## Collision with open work + +**PR #2101** (`fix(codex): gate account-native models by entitlement`, 1397 +lines) already edits `src/server/index.ts` and `src/codex/auth-context.ts` — +both files phase 1 and phase 2 touch. Check its state before starting; the +reason-logging change in phase 1 is small enough to be folded in rather than +raced. + +A red-today regression: `startServer` on win32 with scheduler assets present, +first probe timed out and/or two owner ACL timeouts, then a later successful +probe in the **same** process — `POST /v1/responses` for a native model must go +503 → 200 without `process.exit`. Keep a control that a real foreign home stays +503. + +## Verification + +``` +bun test tests/native-profile-startup.test.ts tests/native-main-owner-lifetime.test.ts tests/codex-service-manager-probe.test.ts +bun x tsc --noEmit +``` + +Windows CI is authoritative here; a green macOS/Linux run proves little about +scheduler and icacls paths. + +## Sequencing note — corrected + +The first version of this section said "do #2114 first" and called it a +dependency. **An audit lane refuted that, and it was right.** The two fixes +touch different seams: + +- #2114 narrows one Linux classification in `inspectSystemd()`. +- #2108 phase 2 changes *fence policy* — a one-shot `unknown` becomes + retryable. + +Neither needs the other. #2114 does not implement retryability; phase 2 does not +classify the systemd bus. The shared `unknown → permanent fence` chain is a +shared *symptom*, and the overlap in `service-manager-probe.ts` is merge +convenience, not a prerequisite. + +So: **they can land in either order.** The preference for #2114 first was +"don't design the general rule from the instance we understand least", which is +a reasonable working habit and not a constraint. Stated as a dependency it +would have delayed the Windows fix for no technical reason. + +The one real ordering constraint here remains internal: **phase 1 before phase +2**, because the trigger is unidentified and phase 2 aims at one of two +candidates. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/050_1587_deferred_catalog.md b/devlog/_plan/260819_unclaimed_bug_selection/050_1587_deferred_catalog.md new file mode 100644 index 0000000000..236d7d5ea9 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/050_1587_deferred_catalog.md @@ -0,0 +1,124 @@ +# 050 — #1587: routed first-turn catalog ignores `defer_loading` + +Rank 4. The only candidate with a hard measurement. + +## Failure mechanism + +`buildTools()` (`src/responses/parser.ts:155`) never reads Codex's +`defer_loading` flag. `pushFn` (:163) and the namespace flattener (:195-207) +copy every tool's full `parameters` into `OcxTool`, and the flag is not on the +type — so it is gone by parse time. + +The routed adapters then serialize all of it: + +| Adapter | Site | +|---|---| +| chat | `src/adapters/openai-chat.ts:1197` | +| anthropic | `src/adapters/anthropic.ts:740` | +| google | `src/adapters/google.ts:270` | + +Non-OpenAI chat/Anthropic/Google additionally inject a `tool-catalog-nudge` +listing every flattened wire name in the system prompt +(`openai-chat.ts:636`). + +**The native path is asymmetric on purpose.** `openai-responses.ts:406` +preserves `defer_loading` and strips it only when a `tool_search_output` +actually loads the tool, so the ChatGPT backend keeps deferred schemas out of +the prompt. Chat and Anthropic wires have no server-side deferral, so the same +bytes land in billed context. + +## Measured, on a real captured catalog + +A lane ran this tree's actual `parseRequest` against a Codex Desktop catalog +captured from a session rollout: + +| Sample | Deferred | Catalog bytes | After parse | +|---|---|---|---| +| 2026-08-12 rollout | 8 of 8 tools | 32,927 / 34,404 = **95.7%** | all 8 emitted with full schemas, 32,887 bytes (~8.2k tokens), **zero** defer flags surviving | +| second sample | 4 namespaces / 10 tools | — | 24,227 bytes (~6.1k tokens) | + +## State the goal in bytes, not in the headline ratio + +The issue title says 3-5x. Both lanes independently flagged that this number +does not survive scrutiny: the thread compares **three different tokenizers** +(OpenAI 21,081 vs Kimi 62,319 vs Claude 98,402), and the Opus row additionally +carried a repo `AGENTS.md`. + +The mechanism is real and measured; the multiplier is not a clean +apples-to-apples figure. Success criteria should therefore be +**"deferred tools contribute no schema bytes to the routed catalog"**, verified +in serialized bytes we control — not "routed matches native within N%". + +## Fix shape + +1. Add `deferred?: boolean` to `OcxTool` and set it in `buildTools` + (`parser.ts:155-241`, both `pushFn` and the namespace path). +2. Clear it where `loadedToolSpecs` promotes a tool (`parser.ts:691-706`, which + already tracks `loadedFromToolSearch`). +3. In the three adapters, emit a **name + one-line description stub with empty + `parameters`/`input_schema`** for deferred tools instead of the full schema. + +**The constraint that makes this delicate:** `parser.ts:633` requires exact +wire names stay listed, or the model guesses names. So the stub must keep the +name and drop only the schema. A model may still call a stubbed tool with wrong +arguments before `tool_search` loads it — the `tool_search` round-trip +(`parser.ts:626-654`) has to be the recovery path, not an optional extra. + +**Files:** `src/responses/parser.ts`, `src/types.ts`, +`src/adapters/openai-chat.ts`, `src/adapters/anthropic.ts`, +`src/adapters/google.ts`, plus conformance tests. + +**Note on the split program — corrected.** `OcxTool` moves in **WP1 (#2019)**, +not WP1b: #2019's diff creates `src/types/tools.ts` and relocates `OcxTool` +there (verified: `git show origin/codex/split-wp1-types:src/types/tools.ts` +contains `interface OcxTool`). #2023 moves the accounts/config/provider/request +clusters. So the `deferred` field lands in `src/types/tools.ts` once **#2019** +is in, one PR earlier than this doc first said. + +**Bigger collision this doc originally missed.** The split trio is not the only +moving code on this surface. Live overlaps on #1587's exact files: + +| PR | Overlaps | +|---|---| +| **#1934** | `parser.ts`, `types.ts`, **and all three adapters** — #1587's entire surface | +| #2040 | `parser.ts` | +| #2115 | the three adapters | + +`#1934` is the real hazard, not the split. The 070 roadmap already schedules it +in phase B precisely because it overlaps. **#1587 should be planned after +#1934 lands**, or the two will conflict across five files. + +The original framing — "only #1587 collides, and only with the split" — was a +consequence of checking against the split branches and nothing else. + +## The test that currently pins the wrong behavior + +`tests/responses-tool-conformance.test.ts` **asserts** that namespace children +are flattened into top-level tools (`github.search` becomes top-level). The +correct regression is the opposite shape and must be added alongside an amended +version of that one. + +Red today, green after: a `defer_loading: true` namespace with fat MCP schemas +plus `exec`/`tool_search` → `toolsToChatFormat`/`toolsToAnthropicFormat` must +not include those children's schemas, while still listing their exact +namespaced wire names; serialized catalog bytes stay near the compact size; and +a `tool_search_output` promoting one restores its full schema on the next turn. + +## Verification + +``` +bun test tests/responses-tool-conformance.test.ts tests/responses-parser.test.ts +bun x tsc --noEmit +``` + +## Risk + +Real product risk in both directions. Strip too much and routed models lose +plugin visibility — the #1522/#1529 class, which is why the flattening was +added in the first place (2026-06-19, `6998fcaad`, so chat models could call +MCP tools). Strip too little and nothing improves. + +**Do not** re-stamp `supports_search_tool=false` as a shortcut: `fcbef381e` +showed that regresses `exec.description` from 96,699 to 258,929 chars. That is +a different expansion and would make the problem worse while appearing to +address it. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/060_1933_tray_encoding.md b/devlog/_plan/260819_unclaimed_bug_selection/060_1933_tray_encoding.md new file mode 100644 index 0000000000..65cf0f2ec4 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/060_1933_tray_encoding.md @@ -0,0 +1,99 @@ +# 060 — #1933: Windows tray registration misread as foreign/stale + +Rank 5. Cheapest fix in the selected set. + +## The title is a symptom string, not a diagnosis + +"startup registration is foreign, stale, or points to missing package files" is +a single collapsed summary produced by `trayStatusFrom()` +(`src/tray/windows.ts:440-462`). It does not mean three things were checked and +one failed; it means one boolean went false. + +## Failure mechanism + +`runRegistry` / `runRegistryAsync` decode `reg.exe` output with +`encoding: "utf8"` (`src/tray/windows.ts:120-125`, `335-345`). Redirected +`reg query` emits the console ANSI code page, not UTF-8. + +The reporter's username is `MötzJensen`. `ö` is `0xF6` in Windows-1252 and +decodes to `U+FFFD` under UTF-8. The round-trip check +`registered === state.runCommand` then fails, `registrationOwned` goes false, +and the stale summary is printed — even though regedit shows a correct, +well-formed, owned Run value. + +**The write side is fine** (CreateProcess is UTF-16). Only the read is broken. + +## This is a known class with an existing fix that was never wired here + +`decodeWindowsTextBytes` (`src/lib/windows-text.ts:75`) already solves exactly +this for `schtasks` — that was #1573, and it ships with a `C:\Users\Jörg` +fixture in `tests/windows-text-decoding.test.ts`. The tray registry reader was +simply missed. + +That is the argument for doing it now despite the low severity: a half-applied +fix for a known class is how the next site gets missed too. + +## Fix shape + +Capture a Buffer in `runRegistry`/`runRegistryAsync` and decode through +`decodeWindowsTextBytes`, the same helper the service probe uses. + +**Files:** `src/tray/windows.ts`, `tests/` (new case reusing the existing +fixture shape). + +**Known limitation to state, not hide:** `decodeWindowsTextBytes` does not +cover ja/zh code pages by design. This fix closes 1252 and CP949, not every +ACP. A fuller answer is `reg export` (UTF-16) instead of `reg query`, which is +a larger change and should be its own decision. + +**Do not** loosen the foreign-Run refusal (`tray/windows.ts:587`) to make the +symptom go away. That check is correct; it is being fed corrupted input. + +## Regression test + +Feed Windows-1252 (and CP949) `reg query` bytes for a path like +`C:\Users\MötzJensen\.opencodex\opencodex-tray.vbs` through the tray registry +reader and assert +`parseWindowsTrayRunValue(...) === buildWindowsTrayRunCommand(...)`. + +Today UTF-8-decoding those bytes makes +`windowsTrayRegistrationIsStale({ registered: true, registrationOwned: false })` +true. After the fix it must round-trip. + +## Verification + +``` +bun test tests/windows-text-decoding.test.ts tests/windows-tray.test.ts +bun x tsc --noEmit +``` + +(The tray suite is `tests/windows-tray.test.ts` — an earlier draft of this doc +named a `tests/tray-windows.test.ts` that does not exist. Related files: +`windows-tray-restart-hardening.test.ts`, `windows-tray-run-limit.test.ts`.) + +## Secondary UX gap worth a follow-up, not this fix + +The GUI cannot repair this state: Install is hidden when `tray.stale` +(`gui/src/pages/startup-sections.tsx:195`), and Uninstall is shown but also +refuses on a mismatched parse (`tray/windows.ts:687`). So a user in this state +has no in-product action. Worth splitting into its own issue — a stale tray +should always offer a repair path regardless of why it is stale. + +## Honesty note: the mechanism is proven, the attribution is inferred + +The encoding **mechanism** is verified in code — the tray reads `reg.exe` as +utf8 while the service probe already routes the same output through +`decodeWindowsTextBytes`. + +**Pinning this specific issue to it is an inference.** An audit lane checked +the thread: the reporter's GitHub *display name* is `Mötz Jensen`, the actual +profile path was never posted, and `C:\Users\MötzJensen` is reconstructed +rather than observed. The screenshots show the collapsed stale summary and a +German UI; the issue's own earlier review said they cannot distinguish a +foreign Run value from missing package files. + +Consequence for whoever picks this up: make the fix on class-hygiene grounds +(the helper exists, the site was missed), but **do not close #1933 on it** +without asking the reporter for `ocx tray status --json` and the raw Run value. +If their profile path is pure ASCII, this is the wrong diagnosis and the issue +stays open. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/070_sequencing.md b/devlog/_plan/260819_unclaimed_bug_selection/070_sequencing.md new file mode 100644 index 0000000000..4a58560fa9 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/070_sequencing.md @@ -0,0 +1,113 @@ +# 070 — Sequencing against the release roadmap + +This unit starts **after stage 3d** of `260819_next_roadmap/070` — that is, +after `#2019` and `#2023` merge, after the first preview soak, and after +`#2036` lands alone. + +## Collision analysis — corrected after audit + +The first version of this section asked only "does this fix touch a file the +**split** rewrites". That is the wrong question, and it produced a wrong +answer: "only #1587 collides." + +The right question is **"does an open PR already own this code"**, regardless of +whether that PR mentions our issue. Re-derived: + +| Fix | Files | Collides with | +|---|---|---| +| #2114 | `service-manager-probe.ts` + its test | **PR #2029** — same function, `CHANGES_REQUESTED` | +| #2108 | `server/index.ts`, `codex/auth-context.ts` | **PR #2101** (1397 lines, same two files) | +| #1587 | `types.ts`, `parser.ts`, 3 adapters | **#2019 and #2023** (split), plus #2112/#1934 on types, #2040/#2083 on parser, #2115/#2080/#2075/#2071/#2070 on the adapters | +| #2107 | `service.ts` | clean | +| #1933 | `tray/windows.ts` | clean | + +Three corrections fall out of that table: + +1. **#2114 is not greenfield.** See `020` — the first action is to extend or + unblock #2029, not to open a parallel PR. +2. **#1587 is worse than "after WP1b".** WP1 (**#2019**) already rewrites + `src/types.ts`, and the adapter files it touches are among the most + contested in the queue. It is the last of the five to start, not merely the + one that waits for the split. +3. **#2107 and #1933 are the only genuinely clean ones.** That strengthens the + case for running them in parallel rather than queueing them behind #2114. + +### Why still after stage 3d + +Unchanged for #1587 (`types.ts` is being replaced by a barrel). For the rest it +is scheduling, not correctness — the split train owns review attention until 3d +closes. + +## Order + +``` +1. #2114 unblock PR #2029 with the containment its reviewer asked for +2. #2107 bake proxy env into service units (clean, parallel-safe) +3. #1933 tray registry decoding (clean, parallel-safe) +4. #2108 phase 1 log the gate reason (coordinate with #2101) +5. #1527 residual: abort-teardown misclassification (small, independent) +6. #1587 deferred catalog (last: most contested files) +7. #2108 phase 2 retryable fence (after phase 1 produces data) +``` + +### Dependencies, stated explicitly + +- **#2114 before #2108 phase 2.** They share the `unknown → permanent fence` + layer. #2114 settles how a probe that cannot answer should be classified at + the boundary; phase 2 generalizes that into retryability. Designing the + general rule from #2108 first means deriving it from the instance we + understand least. +- **#2108 phase 1 before phase 2.** The trigger is not identified and the gate + reason is not logged. Phase 2 without phase 1 is a fix aimed at one of two + candidates with no way to confirm which. +- **#1587 after the split AND after the adapter PRs settle.** `#2019` rewrites + `types.ts`; `parser.ts` and the three adapters each have open PRs. This is the + one place where starting early guarantees a rewrite. +- **#1527 residual is independent of everything.** It only touches the Cursor + abort listener. It can slot anywhere, and should not wait for #2054 — the + teardown misclassification is orthogonal to checkpoint reuse. +- **#2107, #1933 are independent.** They can slot anywhere; they are placed by + cost, not constraint. + +### What can run in parallel + +#2107 and #1933 touch nothing the others touch and nothing each other touches. +If there is review capacity, they are the two to run alongside #2114 rather +than after it. + +## Relationship to the preview soak + +`#2114`, `#2107` and `#2108` are all "the proxy cannot serve a path" bugs, and +all three are hard to catch in CI: they need a container without a user bus, a +shell-only proxy, and a Windows reboot respectively. None of those exist on a +runner. + +That makes them **good soak candidates and bad CI candidates**. The 070 roadmap +already establishes a preview window with a named exercise set; these three +should extend it: + +- a container run with `systemctl` present and no user bus (#2114) +- a service install where the proxy env lives only in the shell (#2107) +- a Windows reboot with the scheduler backend (#2108) + +Adding those three to the soak checklist is cheaper than trying to simulate +them in CI, and it converts the next occurrence into a dated observation +instead of another ambiguous report. + +## What this unit does not do + +No `src/` changes, no PR merges, no GitHub mutations. The three deferred +candidates keep their disposition from `010`: #1049 waits for a real incident, +#1419 stays upstream-blocked with a separable supervision follow-up, and #1730 +is a close-as-withdrawn once someone is authorized to close it. + +## Follow-ups this unit identified but does not own + +Both were named in passing and would otherwise be lost. Each deserves its own +issue rather than riding a fix: + +1. **Unsupervised `ocx gui`** (`src/cli/dispatch.ts:255`) spawns the proxy + detached while launchd `KeepAlive` covers only `ocx service`. Separable from + #1419's untestable Bun trap, and unlike it, testable. +2. **Stale tray has no in-product repair path** — GUI hides Install when + `tray.stale` and Uninstall also refuses on a mismatched parse. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/075_verification.md b/devlog/_plan/260819_unclaimed_bug_selection/075_verification.md new file mode 100644 index 0000000000..f2f7159f2b --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/075_verification.md @@ -0,0 +1,168 @@ +# 080 — Verification of this unit's own claims + +> Renumbered to `075` — this is the verification record that sits between the +> sequencing doc and the outcome. `080_outcome.md` is the close-out. +> +> **Update:** the retired lane was replaced. Two later lanes landed +> (`01a019f5` narrow, `01a019ed` wide) and their findings are folded into +> `000`, `010`, `020`, `040`, `060` and `070`. The direct re-verification below +> stands and was independently confirmed by those lanes; what it could not +> catch on its own was the code-ownership collision with PR #2029, which the +> wide lane found. + +An adversarial audit lane was dispatched against `000`-`070` and **went silent +past three wait cycles**. Under DISPATCH-RETIRE-01 that is a failed dispatch, +not a pass. Recording it as failed rather than quietly proceeding, and +re-verifying the load-bearing claims directly instead. + +## Directly re-verified + +### #2114 — the test really does pin the bug + +`tests/codex-service-manager-probe.test.ts`: + +``` +/** + * systemd does NOT signal absence through the exit code — a missing unit + * prints not-found and exits ZERO. A non-zero status means the question never + * reached the bus, which is the opposite conclusion. + */ +test("a non-zero systemctl status is unknown even though a missing unit exits zero", () => { + const { run } = recorder(() => ({ status: 1, stderr: "Failed to connect to bus" })); + expect(inspectServiceManagerInstallation({ run, platform: "linux", home }).kind).toBe("unknown"); +}); +``` + +CONFIRMED, and the comment is worth reading closely: the reasoning is +**correct** and the conclusion is still wrong for this environment. "The +question never reached the bus" is precisely why `unknown` is the wrong verdict +— an unanswerable question is evidence about the bus, not about who owns the +service home. The test is not sloppy; it encodes a genuine judgment that needs +revisiting for the container case, which is exactly why `020` says amend it +rather than delete it. + +### #2107 — `buildUnit` really omits proxy env + +`grep -n 'HTTP_PROXY\|HTTPS_PROXY' src/service.ts` returns **nothing**. +CONFIRMED. There is no proxy key anywhere in the service builder, which also +confirms the doc's claim that launchd and the Windows wrapper share the hole. + +### #1587 — the flag really is discarded + +`grep -n 'defer' src/types.ts` → empty. `OcxTool` has no such field. +`rg 'defer_loading' src/responses/` → empty. The parser never reads it. + +CONFIRMED on both halves, which is the part that matters: the measurement +(95.7% of a captured catalog) came from a lane and cannot be re-run here, but +the *code claim* it rests on is directly verifiable and holds. + +### #1933 — the encoding asymmetry is real + +``` +src/tray/windows.ts:122 encoding: "utf8", +src/tray/windows.ts:338 encoding: "utf8", +src/service-manager-probe.ts:29 import { decodeWindowsTextBytes } from "./lib/windows-text"; +src/service-manager-probe.ts:476 decodeWindowsTextBytes(queried.stdout, ...) +``` + +CONFIRMED. The helper exists, the service probe already uses it, and the tray +reader does not. This is the clearest "known class, missed site" in the set. + +### Sequencing — the collision analysis is complete + +Checked each selected fix's files against what the split PRs rewrite: + +| File | In split diff | +|---|---| +| `src/service-manager-probe.ts` | no | +| `src/service.ts` | no | +| `src/tray/windows.ts` | no | +| `src/codex/native-profile-startup.ts` | no | +| `src/codex/native-main-owner.ts` | no | + +CONFIRMED: `#1587` is the only collision, via `src/types.ts`. + +## What remains unverified, and is labelled as such + +- **The #1587 byte measurement.** 32,927 / 34,404 came from a lane replaying a + captured catalog through the real parser. Not reproduced here. The mechanism + is confirmed; treat the exact percentage as one sample. +- **#2108's actual trigger.** Two candidates, and the doc says so plainly. This + is a genuine gap, not an oversight — it is *why* `040` puts logging first. +- **The candidate-set completeness re-derivation.** The list was derived once + live (`2026-08-19T11:45:42Z`) and not independently re-derived by a second + party. A PR opened after that timestamp could claim one of these eight. Cheap + to re-check at start of work, and `070` should be re-read then rather than + trusted. + +## Note on lane reliability in this unit + +## Second audit round — the one that landed + +The first audit lane was retired as silent. It **returned late**, and four +narrow lanes were dispatched in parallel. All five verdicts are in, and they +found more than the direct grep pass did. Everything below was folded back. + +### The finding that changes the plan: #2114 is already owned + +**Open PR #2029 rewrites the exact function this unit planned to change**, and +deliberately leaves the #2114 case closed: + +``` ++ err.includes("Failed to get D-Bus connection: No such file or directory") ++ ... "System has not been booted with systemd" ++ return { kind: "absent" }; ++ return unknown(...) // "other bus failures stay unknown" + ++ test("other bus failures stay unknown — the user manager may be running", () => { ++ stderr: "Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS not set", ++ expect(...kind).toBe("unknown"); +``` + +#2114's reporter stderr is `Failed to connect to user scope bus via local +transport...` — the family #2029 is choosing to keep `unknown`. + +This invalidated three things at once: "#2114 is a cheap first fix", the +no-collision table, and the whole "do #2114 first" sequence. All three shared +one cause — **nobody checked who already owns `inspectSystemd()`.** + +### Corrections applied + +| Finding | Where | Fix | +|---|---|---| +| #1527 wrongly excluded (PR #2054 says "Does not close #1527") | `000` | set corrected to 9; method note added | +| The 020 fix snippet **fails open** — with the bus down, systemctl cannot see a foreign unit either | `020` | rewritten to consult the unit file on disk before returning `absent` | +| "#2114 before #2108 phase 2" is preference, not dependency | `040` | retracted; they can land in either order | +| #2108 should outrank #2114 (bigger platform, every reboot) | `010` | accepted; order revised | +| #1049 "no incident" is the wrong test for a silent integrity gap | `010` | accepted as detection-only phase 1 | +| `OcxTool` moves in **WP1 (#2019)**, not WP1b | `050` | corrected | +| #1934 overlaps **all five** of #1587's files | `050` | recorded as the real hazard | +| `tests/tray-windows.test.ts` does not exist | `060` | corrected to `windows-tray.test.ts` | +| `C:\Users\MötzJensen` was reconstructed, not observed | `060` | honesty note; do not close on the inference | + +### Rejected, with reason + +**"Drop #1933 from the selected set."** Folding it into the Windows pass is +accepted; dropping it is not. The helper already exists and is already wired +elsewhere — a half-applied class fix is how the next site gets missed — and the +GUI offers no repair path, so the affected user is stuck. + +### Still open after this round + +- **#1527** is unclaimed and **not investigated**. It arrived after the lanes + were dispatched. The selected set cannot be called final until it is. +- The **#1587 measurement** (95.7%) is a lane result that cannot be replayed + here. Mechanism confirmed; treat the number as one sample. +- Several **line citations drifted** (`native-main-owner.ts:272` is + `release()`; the second ACL timeout is nearer `:205`). Verify before quoting. +- Follow-ups this unit names and then drops: `/readyz` ignoring the native-main + fence, the Codex "at capacity" remap, and the `ocx gui` supervision split from + #1419 — none has an issue number. + +## Note on lane reliability in this unit + +Of eleven dispatches, two went silent in the first batch and one audit lane +went silent at the end. That is a meaningful failure rate and it changed how +this unit was built: the surviving evidence is per-issue lane reports plus +direct verification, not a single audited pass. Where a claim rests only on a +lane, this document says so. diff --git a/devlog/_plan/260819_unclaimed_bug_selection/080_outcome.md b/devlog/_plan/260819_unclaimed_bug_selection/080_outcome.md new file mode 100644 index 0000000000..d6c8ae6acb --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/080_outcome.md @@ -0,0 +1,95 @@ +# 080 — Outcome + +Terminal outcome: **DONE.** Selection and roadmap only; no `src/` change, no PR +merge, no GitHub mutation, no push. + +## Result + +Nine open `bug` issues have no PR intending to close them. Six are selected for +work after stage 3d, three are deferred with reasons. + +| Rank | Issue | Disposition | +|---|---|---| +| 1 | #2114 systemd bus | **unblock PR #2029**, not a new PR | +| 2 | #2107 service proxy env | take, clean of open work | +| 3 | #1933 tray encoding | take, clean of open work | +| 4 | #2108 Windows reboot gate | phase 1 (log the reason) first; coordinate with #2101 | +| 5 | #1527 residual | take only the abort-teardown slice | +| 6 | #1587 deferred catalog | last — most contested files | +| — | #1049 | defer: no field incident, phase 2 can corrupt `CODEX_HOME` | +| — | #1419 | defer: native Bun trap, no upstream release to move to | +| — | #1730 | close as reporter-withdrawn | + +## What the audits changed + +Two rounds ran. Neither merely agreed, and both errors were in the *method* +rather than in any individual finding. + +**The candidate filter was wrong in kind, not in execution.** It asked "does an +open PR mention `#NNNN`" and subtracted the matches. That over-excludes exactly +where an author was honest about scope: PR #2054 mentions #1527 and says +"Does not close #1527", so the filter counted an explicit disclaimer as a +claim. Reference-counting is not claim-counting, and the set was 9 rather +than 8. + +**The collision analysis asked the wrong question.** It checked "does this fix +touch a file the split rewrites" and concluded only #1587 collides. The +question that matters is "does an open PR already own this code", and the +answer changes the top of the ranking: **PR #2029 already edits +`inspectSystemd()`** — the exact function #2114 needs — and is +`CHANGES_REQUESTED` for the same fail-open hazard that `020` independently +rediscovered and wrote down as "test 3". + +So #2114 is still first, but "first" means supplying the containment #2029's +reviewer asked for. Left uncorrected, this unit would have sent someone to open +a second PR against a blocked one and make the same fail-closed +security-adjacent decision twice. + +Smaller corrections, worth recording because they are the kind that waste an +hour: `040` cited `native-main-owner.ts:272`, which is `if (released) return` +inside `release()` — the terminal `unavailable` is at `:205-212`. `060` named a +verification file that does not exist (`tests/tray-windows.test.ts`; the real +one is `tests/windows-tray.test.ts`). + +## What the investigation found that the titles did not + +Three of nine issues do not describe their own cause: + +- **#2107** reads as a WSL networking problem. It is `buildUnit()` baking six + environment variables and no proxy ones, so the service talks direct while + the shim inherits the user's proxy. The discriminator is the status code: + 502 with `connection-reset`, not #2108's 503. +- **#1933** reads as "missing package files". That phrase is a collapsed + summary string; the cause is `reg.exe` output decoded as UTF-8 when the + console code page is Windows-1252, and `decodeWindowsTextBytes` already fixes + this class for `schtasks`. +- **#1730** reads as an OpenCodex tool-call bug. The half that was ours shipped + in `ea0608611`; the reporter attributed the rest to their own configuration + and asked to close. + +And one issue produced a measurement rather than an argument: **#1587** — a +lane ran this tree's real `parseRequest` against a captured Codex Desktop +catalog and found **32,927 of 34,404 bytes (95.7%) deferred and emitted +anyway**. The issue's own "3-5x" headline does not survive scrutiny (it +compares three tokenizers), so the success criterion should be stated in bytes +we control. + +## Follow-ups this unit identified but does not own + +Both were named in a lane report and would otherwise vanish: + +1. Unsupervised `ocx gui` spawns the proxy detached while launchd `KeepAlive` + covers only `ocx service` (`src/cli/dispatch.ts:255`). Separable from + #1419's untestable trap, and unlike it, testable. +2. A stale tray has no in-product repair path: the GUI hides Install when + `tray.stale`, and Uninstall also refuses on a mismatched parse. + +## Method note for the next triage pass + +The cheap derivation — scan PR bodies for `#NNNN`, subtract — is a starting +filter, not an answer. Two checks have to follow it: + +1. **Read the referencing PR.** Does it intend to close the issue, or does it + say it does not? +2. **Check code ownership, not just issue references.** An issue with no PR + mentioning it can still have a PR sitting on the function that must change. From d7caaa9bf5788099b3c353b7a4cda77a89b03dee Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:45:05 +0900 Subject: [PATCH 3/4] docs(devlog): record the 2107 implementation and its red-drive --- .../031_2107_implementation.md | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/031_2107_implementation.md diff --git a/devlog/_plan/260819_unclaimed_bug_selection/031_2107_implementation.md b/devlog/_plan/260819_unclaimed_bug_selection/031_2107_implementation.md new file mode 100644 index 0000000000..290501098d --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/031_2107_implementation.md @@ -0,0 +1,80 @@ +# 031 — #2107 implementation record + +Branch: `fix/service-proxy-env` off `origin/dev` @ `18e072c8d`. +Commit: `eb910776a`. PR: **#2116** → `dev`. + +## What the plan said vs what the tree said + +`030` was written against a read of the code and held up on every point, with one +correction worth recording. + +**The plan asked for a guard that already exists.** It said "do not emit empty +`Environment=` lines". All three builders already drop falsy values before +joining — `systemdEnvironmentAssignment` returns `null`, +`buildPlist` uses ternaries, `windowsBatchSet` returns `null` — and each list is +`.filter(Boolean)`ed. So the risk was real in principle and already handled in +practice; adding a second guard would have been noise. + +That is the useful shape of this correction: the plan named a hazard from +reading a diff, and the tree had already solved it structurally. + +## The change + +One helper plus three call sites: + +``` +resolvedProxyEnv(env = process.env): { name, value }[] + for each of PROXY_ENV_KEYS (HTTP_PROXY, HTTPS_PROXY, ALL_PROXY, NO_PROXY) + value = env[KEY]?.trim() || env[key]?.trim() // either case + if (value) push({ name: KEY, value }) // canonical name only +``` + +| Builder | Line of insertion | +|---|---| +| `buildUnit` | after `opencodexHome`, mapped through `systemdEnvironmentAssignment` | +| `buildPlist` | after `OPENCODEX_HOME`, mapped to `/` | +| `buildWindowsServiceScript` | after `OPENCODEX_HOME`, mapped through `windowsBatchSet` | + +Reading both letter cases but writing only the upper-case name matters: curl-style +tooling sets `http_proxy`, and emitting both spellings into one definition would +leave two sources of truth for one setting. + +## Verification + +``` +bun test tests/service.test.ts 126 pass / 0 fail +bun x tsc --noEmit exit 0 +``` + +**Red-drive, recorded.** With the three `resolvedProxyEnv()` call sites stripped +out, the primary test fails on exactly the missing line: + +``` +Expected to contain: "Environment=\"HTTP_PROXY=http://127.0.0.1:7890\"" +(fail) bakes outbound proxy env into the unit ... (#2107) + 1 pass 1 fail +``` + +and nothing else in the file breaks. Restoring the fix returns 126/0. + +The companion test (no proxy in the shell → no proxy keys emitted) passes in both +states by design. It is not an oracle for the fix; it is a guard that the fix +cannot start emitting empty assignments later. + +## What this deliberately does not do + +- **No interactive `ExecStart`.** `bash -ic` would make service startup depend on + the user's interactive shell — a worse failure mode than the bug. +- **No `NO_PROXY` synthesis.** The runtime's `applyProxyEnv` already keeps + loopback off the proxy path; inventing a value here could diverge from it. +- **No credential handling decision.** A proxy URL can carry credentials, and + baking it writes that to disk. `config.proxy` already works for that case + without this change. Raised in the PR body as an open question rather than + silently resolved. + +## CI posture + +Not consulted. `dev` is mid-merge-train (30 commits in the window this work +started) and its checks are noisy by construction. Verification here is local +and complete for the changed surface; CI becomes the gate once the train +settles. From 2d7b945b638962ce127f93327be244b9431c2df8 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 19 Aug 2026 23:20:02 +0900 Subject: [PATCH 4/4] fix(service): build service definitions from an injected proxy env, not process.env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #2107 tests assigned HTTP_PROXY/HTTPS_PROXY/NO_PROXY onto the real process.env and restored them in a finally. That looked airtight and was not: `bun test a.test.ts b.test.ts` runs every file in ONE process, and --isolate does not change that. The values outlived the file. The Lab sandbox calls rejectProxyEnvironment() against the live process.env and treats any proxy variable as a harness_failure, by design — it must not dial out through a proxy. So every Lab file that loaded after service.test.ts died on a leaked variable it never set: 73 failures on the unsharded macOS lane, zero when the Lab suites ran alone, which is exactly the shape that makes this look like flake rather than a defect. The fix is to stop mutating global state to test a pure function. buildUnit() and buildPlist() now take the resolved proxy entries as a parameter defaulting to resolvedProxyEnv(), so production behavior is unchanged and the tests hand in a literal environment. resolvedProxyEnv() already accepted an env argument; it is now exported so a test can use it the way the runtime does. A third case is added while the seam is open: a lower-case http_proxy must be baked under the canonical upper-case name. That was implemented and documented but never asserted. Refs #2107 Verification: the five suites that carried the failure — service, lab-live-probe, lab-fabric-task, lab-automation, api-key-attribution — go 50 fail -> 0 fail, 236 pass. tsc --noEmit exit 0. --- src/service.ts | 10 +++---- tests/service.test.ts | 69 +++++++++++++++++++++---------------------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/src/service.ts b/src/service.ts index 4256820ee2..37e77506dd 100644 --- a/src/service.ts +++ b/src/service.ts @@ -390,7 +390,7 @@ function writeServiceApiTokenFile(): string | null { return path; } -export function buildPlist(): string { +export function buildPlist(proxyEnv: { name: string; value: string }[] = resolvedProxyEnv()): string { const { bun, bunRuntimeSource, cli } = cliEntry(); const log = logPath(); const path = process.env.PATH ?? "/usr/local/bin:/usr/bin:/bin"; @@ -405,7 +405,7 @@ export function buildPlist(): string { codexHome ? ` CODEX_HOME${plistString(codexHome)}` : null, codexSqliteHome ? ` CODEX_SQLITE_HOME${plistString(codexSqliteHome)}` : null, opencodexHome ? ` OPENCODEX_HOME${plistString(opencodexHome)}` : null, - ...resolvedProxyEnv().map(({ name, value }) => + ...proxyEnv.map(({ name, value }) => ` ${name}${plistString(value)}`), ].filter((line): line is string => Boolean(line)).join("\n"); const command = buildServiceShellCommand(bun, cli); @@ -659,7 +659,7 @@ function systemdEnvironmentAssignment(name: string, value: string | undefined): * own `applyProxyEnv` already treats both cases as equivalent. Only the canonical * upper-case name is baked, so a definition never carries two spellings of one setting. */ -function resolvedProxyEnv(env: NodeJS.ProcessEnv = process.env): { name: string; value: string }[] { +export function resolvedProxyEnv(env: NodeJS.ProcessEnv = process.env): { name: string; value: string }[] { const resolved: { name: string; value: string }[] = []; for (const key of PROXY_ENV_KEYS) { const value = env[key]?.trim() || env[key.toLowerCase()]?.trim(); @@ -2444,7 +2444,7 @@ function unitPath(): string { return join(unitDir(), `${TASK}.service`); } -export function buildUnit(): string { +export function buildUnit(proxyEnv: { name: string; value: string }[] = resolvedProxyEnv()): string { const { bun, bunRuntimeSource, cli } = cliEntry(); const log = logPath(); const path = process.env.PATH ?? "/usr/local/bin:/usr/bin:/bin"; @@ -2459,7 +2459,7 @@ export function buildUnit(): string { codexHome, codexSqliteHome, opencodexHome, - ...resolvedProxyEnv().map(({ name, value }) => systemdEnvironmentAssignment(name, value)), + ...proxyEnv.map(({ name, value }) => systemdEnvironmentAssignment(name, value)), ].filter((line): line is string => Boolean(line)).join("\n"); return `[Unit] Description=OpenCodex Proxy Server diff --git a/tests/service.test.ts b/tests/service.test.ts index 75d570f091..32ba00c47c 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -7,6 +7,7 @@ import { saveConfig } from "../src/config"; import { windowsEnvIndirectBatchValue } from "../src/lib/win-paths"; import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsSchtasksCreateArgsForXml, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, installFreshWindowsSchedulerSafely, installServiceSafely, launchctlLoadFailed, launchdJobMatchesPlist, normalizeServiceSubcommand, parseServiceInstallState, prepareServiceInstall, readWindowsSchedulerXmlState, registerFreshWindowsSchedulerTask, removeNativeWindowsServiceForScheduler, repairService, resolveServiceListenPort, runLaunchctl, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, systemdNeedsDaemonReload, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; import type { ServiceDiagnostic } from "../src/service"; +import { resolvedProxyEnv } from "../src/service"; import { buildWinswXml } from "../src/lib/winsw"; import { CONFIG_OWNER_FILE, CONFIG_UNINSTALL_MANIFEST, recordOwnedConfigPath, removeOwnedConfigState } from "../src/lib/config-ownership"; import { serviceApiTokenFilePath } from "../src/lib/service-secrets"; @@ -115,49 +116,47 @@ describe("systemd service unit", () => { // /bin/sh -lc — which is dash on Ubuntu/WSL and reads .profile, not .bashrc. A user // whose proxy lives in the shell therefore gets a service that dials upstream direct, // the socket is reset, and the request surfaces as 502 Provider unreachable. - const saved = { ...process.env }; - try { - process.env.HTTP_PROXY = "http://127.0.0.1:7890"; - process.env.HTTPS_PROXY = "http://127.0.0.1:7890"; - process.env.NO_PROXY = "localhost,127.0.0.1"; - delete process.env.ALL_PROXY; + // + // The shell is passed in rather than assigned onto `process.env`. Mutating the real + // environment here leaked `HTTP_PROXY` out of this file: Bun runs a `bun test a b` + // invocation in ONE process, and the Lab sandbox calls `rejectProxyEnvironment()` on + // the live `process.env`, so every Lab file that loaded afterwards died with + // `harness_failure`. That was 73 failures on the unsharded macOS lane and zero when + // the Lab suites ran alone. + const proxyEnv = resolvedProxyEnv({ + HTTP_PROXY: "http://127.0.0.1:7890", + HTTPS_PROXY: "http://127.0.0.1:7890", + NO_PROXY: "localhost,127.0.0.1", + }); - const unit = buildUnit(); - expect(unit).toContain('Environment="HTTP_PROXY=http://127.0.0.1:7890"'); - expect(unit).toContain('Environment="HTTPS_PROXY=http://127.0.0.1:7890"'); - expect(unit).toContain("NO_PROXY="); - // An unset key must not produce an empty assignment. - expect(unit).not.toContain('Environment="ALL_PROXY="'); + const unit = buildUnit(proxyEnv); + expect(unit).toContain('Environment="HTTP_PROXY=http://127.0.0.1:7890"'); + expect(unit).toContain('Environment="HTTPS_PROXY=http://127.0.0.1:7890"'); + expect(unit).toContain("NO_PROXY="); + // An unset key must not produce an empty assignment. + expect(unit).not.toContain('Environment="ALL_PROXY="'); - const plist = buildPlist(); - expect(plist).toContain("HTTP_PROXYhttp://127.0.0.1:7890"); - expect(plist).not.toContain("ALL_PROXY"); - } finally { - for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY"]) { - if (saved[key] === undefined) delete process.env[key]; - else process.env[key] = saved[key]; - } - } + const plist = buildPlist(proxyEnv); + expect(plist).toContain("HTTP_PROXYhttp://127.0.0.1:7890"); + expect(plist).not.toContain("ALL_PROXY"); }); test("omits proxy env entirely when the installing shell has none (#2107)", () => { - const saved = { ...process.env }; - try { - for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", - "http_proxy", "https_proxy", "all_proxy", "no_proxy"]) delete process.env[key]; - - const unit = buildUnit(); - for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"]) { - expect(unit).not.toContain(`${key}=`); - } - } finally { - for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", - "http_proxy", "https_proxy", "all_proxy", "no_proxy"]) { - if (saved[key] !== undefined) process.env[key] = saved[key]; - } + const unit = buildUnit(resolvedProxyEnv({})); + for (const key of ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"]) { + expect(unit).not.toContain(`${key}=`); } }); + test("lower-case shell spellings are baked under the canonical name (#2107)", () => { + // curl-style tooling sets the lower-case pair; only the upper-case name is emitted so a + // definition never carries two spellings of one setting. + const unit = buildUnit(resolvedProxyEnv({ http_proxy: "http://127.0.0.1:7890" })); + + expect(unit).toContain('Environment="HTTP_PROXY=http://127.0.0.1:7890"'); + expect(unit).not.toContain("http_proxy="); + }); + test("preserves custom Codex and OpenCodex homes", () => { const oldCodexHome = process.env.CODEX_HOME; const oldCodexSqliteHome = process.env.CODEX_SQLITE_HOME;