Repository navigation
fix(service,codex): recognize Desktop supervision on Windows and discover the Codex app CLI on macOS - #7011
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (31)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3441130ea8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| in the final report. | ||
|
|
||
|
|
||
| ## Outcome (wp3 restack) |
There was a problem hiding this comment.
Move the completed lane record to
_fin
This section records the terminal outcome, and 000_plan.md already identifies a final head and passing reviews, so this unit is no longer open. Leaving the directory under devlog/_plan/ misrepresents completed work as active; move the entire 261013_rt284_l4_platform unit to devlog/_fin/ in this commit.
AGENTS.md reference: AGENTS.md:L108-L111
Useful? React with 👍 / 👎.
d9ce24c to
80f886c
Compare
5787743 to
310af60
Compare
80f886c to
dce81dc
Compare
310af60 to
d3a5818
Compare
dce81dc to
ed2e435
Compare
inspectDesktopSupervision returned "unsupported" on Windows, so status, doctor, service install/repair/start/restart, stop and update all behaved as if no Desktop ran beside a package-installed ocx. Windows now proves supervision with the issue's standard: ocx.pid and runtime-port.json both name the runtime; its parent is a live opencodex-desktop.exe; both images are drive-qualified and the runtime is the Desktop's sibling ocx.exe; parent and child share a session; the parent was created first; and two snapshots agree. Each snapshot is one bounded PowerShell Win32_Process read (Node-safe, no FFI) inside a 6 s budget per inspection. Anything inconclusive stays unknown, and a Desktop image observed before a later field fails validation keeps its veto. The startup diagnostic gains a Windows path: it names the live supervisor (and, for a durable claim, checks the roaming install-id) but never credits Start at Login, so Windows does not read as Desktop-protected and drops ocx service advice. For an older Windows runtime without a Desktop diagnostic, status and doctor accept an owned local reading bound to the same PID pair; macOS and Linux keep requiring an unowned one. Closes #6853
A Mac whose only Codex is the Codex desktop app (ChatGPT.app, bundle id com.openai.codex) left OpenCodex with no runtime: the macOS installed candidates never looked inside the app, and the Desktop sidecar's GUI PATH never contains it (#6963). macOS now adds the app's CLI as the lowest-precedence candidate: after the environment override, the persisted selection, the shim, PATH, established install locations and the bare fallback; system /Applications before ~/Applications; the current entrypoint Contents/Resources/codex-cli/bin/codex (measured on 26.1007) before the older Contents/Resources/codex. A candidate counts only when the bundle's top-level XML Info.plist reports CFBundleIdentifier com.openai.codex, read in-process with a 64 KiB bound and no subprocess. The identity is re-checked for any path inside either bundle, including an explicit override or a persisted app path after the app is replaced. App candidates never take over a runnable automatically persisted selection: they are excluded from the deferred fast-path handover and from supersededDiscovered, and only appear in newerAvailable. Deferred resolution skips the unproven bare fallback only when a verified app candidate exists. The bundled-catalog and prompt consumers already map unsupported commands and spawn failures to unavailable; structurally malformed JSON (null model rows, null message or content items) no longer throws. Closes #6973
The #6959 passive-read effect watcher counted every numeric open flag other than exactly O_RDONLY as a write. With #6973 a passive GET /api/settings on a Mac that has the Codex app installed resolves the runtime and reads the app's Info.plist with O_RDONLY | O_NONBLOCK (the non-blocking flag keeps a FIFO at that path from hanging discovery), which the watcher reported as a write. Numeric flags are now judged by their write bits (O_WRONLY, O_RDWR, O_CREAT, O_TRUNC, O_APPEND) and string flags by Node's write-capable modes, with a deterministic assertion so the check does not depend on the host having the app installed. Subprocess and write watchers are unchanged.
… control-character regex ca94624 (#6973) rejected control characters in each plist node with a control-character regex literal and an eslint-disable-next-line no-control-regex directive, which the PR hygiene gate refuses as a new suppression. The node check now uses the reader's existing XML 1.0 Char predicate (allXmlChars), a code-point comparison: it rejects everything the regex did (C0 controls other than tab/LF/CR, U+FFFE, U+FFFF) and is the same rule already applied to the whole decoded document, so accepted and rejected plists are unchanged. No suppression remains.
d3a5818 to
d91ecf8
Compare
Ten more commits landed on dev (lidge-jun#7010, lidge-jun#7011). The merge is automatic and none of them touch gui/, src/oauth/, src/providers/ or src/server/management/, the areas this branch changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Two platform changes stacked on the proxy fixes.
52ca9103f7: on Windows,ocx status,doctorandresolvenow recognize OpenCodex Desktop as the proxy's supervisor, and service install/repair/start/restart refuse to compete with it. Desktop is confirmed only when the PID files agree, the parent isopencodex-desktop.exewith the proxy asocx.exein the same install folder, both share a session, the parent started first, and two readings agree; anything uncertain isunknown, never "no Desktop".ca946248f6: on macOS, the CLI bundled in the Codex app (ChatGPT.app, bundle idcom.openai.codex, currentContents/Resources/codex-cli/bin/codexand the olderContents/Resources/codex) is an installed candidate after every existing one. A persisted CLI selection is never silently moved to the app binary.91864ee9e7: union fix. With the app installed, a read-only Remote Link settings request (fix(link): read-only Home management, Home Codex settings and one CLI refusal on Remote Link Children #6959) reads the app'sInfo.plist; the relay test now judges opens by write capability.Closes #6853
Closes #6973
Verification
3441130ea8(Bun 1.4.3 test runner, external checkout):bun run typecheck,structure:check,privacy:scan,skill:surface:checkexit 0; guards (test-layout, layout tooling with the pinned runner, file-size ratchet, repo hygiene, skill-ocx, structure-ssot) 110 pass / 0 fail; GUI locale parity 27 pass;lint:guiandbuild:guiexit 0;scripts/test-layout/layout.json1999 lines.Checklist