feat(codex): add safe cross-platform setup - #140
Merged
Merged
Conversation
Codex ignores hooks declared by a plugin manifest, so the hooks must be written into the user's hooks.json. Doing that in Go rather than in shell gives one implementation for macOS, Linux, and Windows and drops the jq dependency that no platform ships by default. setup-codex installs a self-contained copy of the bundle at <codex-home>/claude-notifications-go and registers the hooks against that stable path: Codex hashes the command string for its trust review, so a versioned plugin-cache path would force a re-approval after every update. The hooks.json merge is conservative: foreign handlers, foreign events and unknown keys survive verbatim, the previous file is backed up, the write is atomic, an unparseable file is refused rather than overwritten, and re-runs produce byte-identical output so the trust hash stays valid. Also drops async from the hook contract. Measured against Codex v0.152.0: an async handler never runs under codex exec, which exits as soon as the turn ends, so the notification was silently lost. Synchronous handlers fire reliably; the hook is fast and fail-open and the timeout caps the worst case. Proven end-to-end: setup command, then a live codex turn, then a real desktop notification.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Three defects found by review of the setup command: - Foreign handlers were round-tripped through this package's typed struct, which injected an empty "command" field into handlers of other kinds (mcp_tool, prompt, agent) and dropped explicit zero/false values. Codex rejects a handler carrying fields its variant does not define, so a user with an mcp_tool hook would have lost every hook in the file. Handlers parsed from an existing file are now re-emitted byte-for-byte, and group-level unknown keys are preserved too. - Command quoting used %q, which doubles backslashes; cmd.exe does not unescape them and the shape diverged from the frozen contract. Paths are now quoted literally: single quotes on POSIX (so a path containing $ or a backslash stays literal, with embedded quotes escaped) and plain double quotes for cmd.exe. - The install-dir assertion compared a raw path against JSON that spells it differently per platform, so it would have failed on the Windows runner. It now compares against the rendered commands, and the quoting helpers are tested directly with an explicit Windows-style path.
Two more defects from review:
- The backup used a fixed .backup name, so a second setup run overwrote the
user's original file with our own generated output and left nothing to
restore from. Backups are now timestamped, with a collision guard.
- copyBundle merged over the existing install instead of replacing it, so
files removed by a newer release survived. Inside ClaudeNotifier.app that
is not cosmetic: a leftover file invalidates the code signature ("a sealed
resource is missing or invalid") and macOS notifications stop working.
Each copied top-level entry is now replaced; runtime files the install dir
accumulates at its own root, such as the debug log, are untouched.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/codex-rich-notifications #140 +/- ##
=================================================================
+ Coverage 59.30% 60.43% +1.13%
=================================================================
Files 51 60 +9
Lines 5516 6581 +1065
=================================================================
+ Hits 3271 3977 +706
- Misses 2018 2290 +272
- Partials 227 314 +87 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Refs #140; unreviewed recovery checkpoint after hosted worker interruption
Refs #140; unreviewed recovery checkpoint after hosted worker interruption
Handle installer lock close failures and fixture I/O errors. Use platform-native absolute path expectations. Refs #140
Cover behavior-bearing Markdown and checksum paths in the release selector and check the Codex manifest version. Refs #140
Mirror the rust-v0.153.4 command runner quoting in the disposable test. Synchronize manifest commands before the first Codex release and clarify explicit versus native hook registration. Refs #140
Keep Codex version stamps and installer side effects separate from Claude, reject destination aliases to foreign bundles, and avoid session content locks for independently deduplicated Codex prompts and subagents. Add regression fixtures for each reviewed failure. Refs #140
Check delivery after lock release and report fixture cleanup failures. Refs #140
Preserve Claude event dedup in the positive control and clarify experimental question-hook and draft canary evidence. Refs #140
777genius
changed the title
777genius
marked this pull request as ready for review
September 8, 2026 20:56
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🤖 ReviewRouter Progress
Review needs attentionWhat failed: Codex OAuth is stale or expired. How to fix
Technical detailsError code: All LLM providers failed during review; failing because FAIL_ON_NO_HEALTHY_PROVIDERS=true. codex/gpt-5.5: Codex CLI failed with exit code 1: Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again. Your refresh token has already been used to generate a new access token. Please try signing in again. refresh_token_reused Codex authentication failed. If using ChatGPT subscription OAuth, reseed auth.json by running |
ReviewRouter🔴 Review failed before comments could be completed. PR: #140 What failedCodex OAuth is stale or expired. Why it mattersCodex could not create a review because the ChatGPT subscription refresh token no longer works in CI. How to fix
Run this from a trusted machine after curl -fsSL https://reviewrouter.site/install/codex | REVIEW_ROUTER_CONFIRM_WRITE=1 REVIEW_ROUTER_SECRET_SCOPE=repo REVIEW_ROUTER_REPO=777genius/claude-notifications-go bashTechnical details |
This was referenced Sep 8, 2026
777genius
changed the base branch from
feat/codex-rich-notifications
to
main
September 8, 2026 23:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds explicit Codex hook registration and safe shared installation/update behavior while preserving Claude installations and unrelated hooks. Stacked on #139; SDK #173, planning #137 and runtime #138 are merged.
User behavior
From a Codex-capable v1.42.0+ bundle, run CN_PRODUCT=codex bash bin/install.sh, then ./bin/claude-notifications setup-codex --plugin-root . (the downloaded .exe on Windows). Review and trust the entries in Codex /hooks. Setup stages a stable runtime copy, preserves foreign hook JSON and unknown fields, and saves unique backups. Repeated setup retains hook identity and existing settings.
The shared installer stages downloads before replacing live files, keeps the existing runtime during failed/offline force updates, prepares required desktop components before main binary promotion, repairs broken notifier fallbacks and retries incomplete optional utilities. Codex uses separate update stamps and does not require Claude Code installation. Both products retain the existing shared settings path.
Verification
Final product head: fef9744.
All three pass on Go 1.22 and 1.26, including lint, Swift tests, native Windows command execution, setup rollback/foreign-hook preservation, and installer failure/interruption cases. Follow-up audit found and fixed the literal README dot-path failure and unsafe installer replacement ordering. Independent installer review passed at 5b2e3d8; subsequent changes only correct test portability and integrate main metadata/docs.
Real Codex CLI 0.153.4 E2E in a disposable loopback-only namespace delivered three local webhook notifications across five deterministic provider turns. Untrusted and modified definitions were skipped; trust persisted across source relocation and repeated setup. No credentials or real user projects were used. A later source-build release preflight used actual install.sh and an unmodified v1.42.0 binary, with no version shim; real v1.41.0 Claude delivery was proven before upgrade and after upgrade with settings preserved. A separate fresh-profile test captured the actual desktop D-Bus Notify call with default desktop configuration. These are source-build checks, not downloaded draft qualification.
Interactive /hooks trust UX was not automated: trust was seeded from Codex-returned hashes. Native marketplace installation, visible desktop banners/audio/click-to-focus and live TUI question-tool firing are not claimed as qualified. Question hooks remain experimental. ReviewRouter and Codecov are excluded by owner decision; no unresolved inline review threads remain.
Release
Draft artifact qualification is tracked in #141. No product release is published. The reviewed SDK pseudo-version remains pinned; its SDK subtree matches merged SDK main. Publishing sdk/v1.2.0 and product v1.42.0 each needs separate owner approval. Code integration does not depend on publishing the SDK module tag.