fix(codex): stop recursive dynamic-launcher shims - #1441
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Unix Codex shim detects same-process recursion and exits with status 126. Installation and refresh paths run bounded probes, clean unsafe process groups, and roll back wrappers, backups, and state when validation fails. ChangesUnix shim safety
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant GeneratedShim
participant SavedLauncher
participant ProcessGroup
participant ShimState
Installer->>GeneratedShim: run --version with bypass
GeneratedShim->>SavedLauncher: resolve saved launcher
SavedLauncher->>GeneratedShim: recurse or spawn descendants
GeneratedShim-->>Installer: return classified probe status
Installer->>ProcessGroup: terminate unsafe process group
Installer->>SavedLauncher: restore original launcher
Installer->>ShimState: commit state after successful validation
Installer-->>GeneratedShim: remove wrapper and artifacts after failure
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Requesting changes on the current head (90ad67a37061cd9a5451f2cd2887f011d259a80d). I validated these against the current implementation and #1439 acceptance criteria.
- Medium — fresh install still commits a recursive launcher; the PID guard only covers same-process
execrecursion.
The new OCX_SHIM_ACTIVE_PID=$$ guard correctly stops the reported mise exec -- codex loop because that chain preserves the PID. However, installCodexShimInternal() still accepts any non-shim launcher from PATH, renames it to .opencodex-real, writes the OpenCodex shim, writes state, and performs no bounded behavioural validation of the saved launcher before committing the installation.
That means #1439's install-safety invariant is still unmet: installation can leave a codex -> saved launcher -> codex cycle behind. It also leaves equivalent recursion forms possible when the dynamic launcher starts a new child process rather than exec-replacing itself, because each child gets a new PID and passes the guard. A launcher that sanitises OCX_SHIM_ACTIVE_PID before redispatch likewise bypasses this guard.
Please keep the runtime fail-fast guard, but also make the install transaction validate the saved launcher before committing. A bounded probe should detect re-entry/timeout and roll back to the original launcher without overwriting an existing backup or persisting shim state. Add install-level regression coverage for same-PID redispatch, child-process redispatch, failed validation rollback, and existing-backup preservation.
- Medium — the fail-fast recovery instruction points to the wrong command.
The new diagnostic tells users to run ocx restore, but ocx restore restores Codex routing/config integration; it does not remove the autostart shim or rename .opencodex-real back into place. The actual shim recovery path is ocx codex-shim uninstall / remove, which calls uninstallCodexShim() and is already excluded from shim auto-restore.
When this guard fires, plain codex is already unusable, so the recovery message needs to direct users to the actual shim removal path. Please update the diagnostic and add a regression assertion for the supported recovery command.
The exact reported mise exec reproduction is fixed, and I found no credential, auth, shell-injection, Windows, or secret-disclosure regression in the patch. The remaining blockers are the incomplete install-safety contract and the incorrect recovery guidance.
|
|
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/shim.ts`:
- Around line 442-456: The probe in probeUnixShimInstall must isolate the
launcher in its own process group, terminate the entire group on timeout, and
wait until the group has exited before rollbackFreshShimInstall runs. Apply the
probe change at src/codex/shim.ts lines 442-456 and ensure the rollback flow at
src/codex/shim.ts lines 1114-1126 occurs only after termination. Update
tests/codex-shim.test.ts lines 299-330 to fork a child process and assert that
no descendant remains, replacing the direct /bin/sleep-only fixture.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1534a890-481b-4c9e-a353-200ba87e0a78
📒 Files selected for processing (2)
src/codex/shim.tstests/codex-shim.test.ts
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/shim.ts (1)
1195-1209: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftProbe both refresh paths before committing a new shim.
refreshShimFileinsrc/codex/shim.ts:805-826moves a non-shim launcher tobackupPathand writes a shim without callingprobeUnixShimInstall.src/update/index.ts:289-295invokes this refresh automatically after an update. A dynamic launcher such asmise ... codexcan therefore makecodexhit the same-PID guard and exit 126.The
expectedReplacementspath used byautoRestoreCodexShimhas the same gap. It checks stable metadata, then commits the replacement without executing--version. Its rollback handles transaction errors, not an unsafe launcher. Reuse the probe-and-rollback logic in both paths, or defer the refresh and restore the launcher when the probe reports an unsafe result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/shim.ts` around lines 1195 - 1209, Reuse the existing Unix safety probe and rollback behavior in both refresh paths: update refreshShimFile and the expectedReplacements flow used by autoRestoreCodexShim to probe the saved/original launcher with --version before committing a generated shim. If the probe reports recursive resolution, timeout, or lingering descendants, restore the original launcher and return the same failed-install result/message instead of committing the shim; preserve normal replacement behavior for safe launchers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/shim.ts`:
- Around line 498-515: Update probeUnixShimInstall() to catch failures from
terminateUnixProcessGroup(groupId) and return "descendants" or another handled
probe result instead of propagating the exception. Preserve the existing timeout
and marker classification while ensuring rollbackFreshShimInstall() and
writeState() remain reachable after any filesystem mutation.
- Around line 42-73: Update CODEX_SHIM_INSTALL_PROBE_SCRIPT to resolve sleep
through PATH instead of assuming /bin/sleep, and make the watchdog exit without
writing the timeout marker or killing the launcher when sleep cannot start.
Preserve the outer spawnSync timeout as the fallback, and update the related
/bin/sleep and /bin/ps assumptions in the codex shim tests.
In `@tests/codex-shim.test.ts`:
- Line 252: Update the stderr assertion in the relevant codex shim test to
compare against the exported CODEX_SHIM_REENTRY_DIAGNOSTIC constant rather than
a shortened literal. Export or test-only re-export that constant from
src/codex/shim.ts, preserving the existing production classifier and ensuring
the test validates the complete diagnostic generated by buildUnixCodexShim.
- Around line 330-339: Strengthen the installCodexShim regression test to record
the grandchild /bin/sleep PID and the launcher’s process-group ID from the
fixture. Wait for the launcher’s descendant-check synchronization, then assert
both the grandchild is no longer running and process.kill with the negative
group ID fails. Preserve the existing child, shim-file, and install-result
assertions, following the coverage pattern used by the timeout test.
- Line 385: Update the process-group probe assertion near the existing
process.kill call to catch the thrown error and assert its
NodeJS.ErrnoException.code is "ESRCH", rather than using a broad toThrow()
check. Preserve the probe’s intended behavior of confirming the group no longer
exists, consistent with the process-group handling in the referenced shim logic.
---
Outside diff comments:
In `@src/codex/shim.ts`:
- Around line 1195-1209: Reuse the existing Unix safety probe and rollback
behavior in both refresh paths: update refreshShimFile and the
expectedReplacements flow used by autoRestoreCodexShim to probe the
saved/original launcher with --version before committing a generated shim. If
the probe reports recursive resolution, timeout, or lingering descendants,
restore the original launcher and return the same failed-install result/message
instead of committing the shim; preserve normal replacement behavior for safe
launchers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d3fb9e9-bef9-4bcd-8fc1-af077bbdb5fe
📒 Files selected for processing (2)
src/codex/shim.tstests/codex-shim.test.ts
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
@Wibias I addressed both requested changes on the current head
Local |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/shim.ts`:
- Line 476: Extend the UnixShimProbeResult type and the --version probe logic
around the recursive classification to return a distinct failure result whenever
result.status is non-zero. Update the rollback handling in the saved-launcher
installation flow to restore the original launcher, remove shim state, and
report that the saved launcher failed its --version probe. Add a regression test
covering a non-zero-exit launcher and asserting both restoration and state
removal.
In `@tests/codex-shim.test.ts`:
- Line 40: Update the cross-platform PATH assignments in withInstalledShim,
including both occurrences, to use the platform delimiter imported from
node:path instead of a hardcoded colon, ensuring installCodexShim can find the
temporary codex wrapper on Windows.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca0cdd58-9ba7-4fab-8f35-70a158f9d970
📒 Files selected for processing (2)
src/codex/shim.tstests/codex-shim.test.ts
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
|
|
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8087d3505
ℹ️ 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".
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Requesting changes on current head a8087d35050aabc413201a764ff03e51c50d9bd8.
The previous exit-127 coverage gap is fixed, but three production blockers remain:
-
P1 — the Unix probe isolation relies on
set -mjob control in non-interactive/bin/sh. The probe assumes background jobs receive process groups keyed by their PIDs and then signals-$launcher_pid/-$watchdog_pid. That is not a portable/bin/shcontract and is problematic on Debian/Ubuntu where/bin/shis commonlydash; valid launchers can be rejected or wait for the outer timeout. Replace shell job-control-dependent grouping with explicit, reliable session/process-group isolation and add a Linux regression using the system/bin/sh. -
P1 — the guarded refresh transaction does not revalidate wrapper ownership after the potentially five-second behavioural probe. An external Codex updater can replace
wrapperPathwhile the probe runs. A successful probe can then commit state for a shim OpenCodex no longer owns; an unsafe probe enters rollback, whose unconditional unlink can delete the updater's newer launcher before restoring the older candidate. Fingerprint the generated shim after writing it and revalidate that exact file before commit/rollback. If ownership changed, preserve the external replacement and defer rather than unlinking it. Apply the same post-probe ownership principle to fresh install. -
Medium — unexpected probe-infrastructure exceptions can still bypass rollback after filesystem mutation.
probeUnixShimInstall()still has unguarded metadata reads such asreadFileSync(markerPath)/readFileSync(groupPath), and transaction callers invokeprobeUnixShimFiles()after moving launchers and writing shims. If the probe throws instead of returning a classified result, fresh install and guarded refresh can escape withoutrollbackFreshShimInstall()/rollbackGuardedRefresh(). The transaction invariant must be: safe => commit; known unsafe => rollback; unexpected probe failure => rollback and surface the probe error.
Please add focused regressions for the non-interactive Linux shell path, concurrent wrapper replacement during the probe, and an injected probe-infrastructure failure after mutation.
Exact-head GitHub Actions are still action_required with no Cross-platform jobs executed, so obtain real exact-head Linux/macOS CI after these fixes.
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
@Wibias Your remaining blocker was valid and in scope. In I also addressed the two current Codex findings in the same focused pass: the probe no longer depends on shell job control ( Verification: focused shim suite 52 passed / 0 failed; full |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git diff --stat 'origin/dev...HEAD' || git diff --stat
git diff -- 'src/codex/shim.ts' 'tests/codex-shim.test.ts'
ast-grep outline src/codex/shim.ts --items all --view expanded
ast-grep outline tests/codex-shim.test.ts --items all --view expandedLength of output: 10365 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 1441 --repo lidge-jun/opencodex --json baseRefOid,headRefOid,baseRefName,headRefName
base_sha="$(gh pr view 1441 --repo lidge-jun/opencodex --json baseRefOid --jq .baseRefOid)"
git diff --find-renames --unified=80 "${base_sha}...HEAD" -- src/codex/shim.ts tests/codex-shim.test.ts || true
printf '\n--- probe and cleanup implementation ---\n'
sed -n '471,665p' src/codex/shim.ts
printf '\n--- refresh transaction implementation ---\n'
sed -n '891,1308p' src/codex/shim.ts
printf '\n--- installation and auto-restore implementation ---\n'
sed -n '1294,1510p' src/codex/shim.ts
printf '\n--- probe-related tests ---\n'
rg -n -C 12 'probe|recurs|descendant|timeout|refresh|backup|child' tests/codex-shim.test.tsLength of output: 184381 🏁 Script executed: #!/bin/bash
set -euo pipefail
nl -ba src/codex/shim.ts | sed -n '360,420p;40,125p;540,625p'
rg -n -C 5 'ELF|native executable|direct executable|spawn\(.*wrapper|launcherShellPath|successfulLauncher' src testsLength of output: 555 🏁 Script executed: #!/bin/bash
set -euo pipefail
awk 'NR >= 40 && NR <= 125 { printf "%6d %s\n", NR, $0 }
NR >= 360 && NR <= 420 { printf "%6d %s\n", NR, $0 }
NR >= 540 && NR <= 625 { printf "%6d %s\n", NR, $0 }' src/codex/shim.ts
rg -n -C 5 'ELF|native executable|direct executable|launcherShellPath|successfulLauncher' src testsLength of output: 23740
P1 — Probe the launcher as an executable, not as a shell script
spawn(launcherShellPath, [wrapperPath, "--version"], ...)
However, Launch the saved launcher directly instead: spawn(wrapperPath, ["--version"], {
detached: true,
env: process.env,
stdio: ["ignore", "ignore", "pipe"],
});Direct execution supports both native executables and shebang-based script launchers. Add a regression test that installs a shim over a direct executable, for example a temporary ✅ Action performedReview finished.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- current HEAD ---'
git rev-parse HEAD
git status --short
printf '%s\n' '--- shim outline ---'
ast-grep outline src/codex/shim.ts --items all --view expanded
printf '%s\n' '--- probe implementation and launcher discovery ---'
rg -n -C 8 'spawn\(|launcherShellPath|findCodexOnPath|probe.*launcher|--version' src/codex/shim.ts
printf '%s\n' '--- direct executable test coverage ---'
rg -n -C 8 'direct executable|native executable|symlink.*executable|--version|findCodexOnPath|probe' tests/codex-shim.test.ts
printf '%s\n' '--- changes after reviewed head, if any ---'
git diff --find-renames --stat aa8525a6cc13469f71364099d1cb73a39da7c234...HEAD
git diff --find-renames -- aa8525a6cc13469f71364099d1cb73a39da7c234...HEAD -- src/codex/shim.ts tests/codex-shim.test.tsLength of output: 32838
P1 — The probe still rejects valid native launchers
spawn(launcherShellPath, [wrapperPath, "--version"], ...)This makes Execute the saved launcher directly: spawn(wrapperPath, ["--version"], {
detached: true,
env: process.env,
stdio: ["ignore", "ignore", "pipe"],
});Add a Unix regression test that points The current You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
A blocking P1 finding remains unresolved in
Execute the saved launcher directly with The PR is not ready until this compatibility case is fixed and verified. You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/shim.ts (1)
933-948: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winGate refresh success on complete installation
readState()accepts multi-filewrappersstate on Unix, andinstallCodexShim()does not reject it. If onerefreshShimFile()call succeeds while a later file removes its generated wrapper after an unsafe probe atsrc/codex/shim.ts:943,refreshedremainstrue. Therefreshed || allInstalledcheck at line 1334 then writes state and reports success althoughallInstalledisfalse. Gate state writes and success onallInstalled, or defer and roll back the entire refresh.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/shim.ts` around lines 933 - 948, Update installCodexShim() so refresh success and state persistence require all wrapper files to be installed successfully, not merely any refreshShimFile() call returning true. Ensure the refreshed || allInstalled path cannot write state or report success when allInstalled is false, including when refreshShimFile() removes a wrapper after an unsafe probe.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/shim.ts`:
- Around line 558-562: Update the probe setup around the metadata paths and the
cleanup finally block: create a private 0o700 temporary directory with
mkdtempSync, place the marker, group, and stderr files inside it, and remove
that directory with rmSync in finally. Keep the existing probe behavior and
result classification unchanged while ensuring all three paths use the private
directory.
- Around line 587-598: Update the cleanup classification around
unixProcessGroupAlive, terminateUnixProcessGroup, and the result checks so a
surviving group is reported as "descendants" when termination succeeds,
including when no marker was recorded. Return "cleanup" only when
terminateUnixProcessGroup throws or when an existing cleanup error condition
independently applies.
- Around line 1160-1186: Update the rollback ownership logic around
sourceOccupied and ownsWrapper to treat a started wrapper write with an
undefined writtenWrapperFingerprint as owned when
stableShimPathProbe(entry.operation.file.wrapperPath) confirms the file contains
SHIM_MARKER; remove that partial wrapper so the moved replacement is restored to
sourcePath. Add a regression in tests/codex-shim.test.ts that fails writeShim
after creation and verifies the replacement launcher returns to the codex path.
- Around line 90-119: Update the launcher exit handling around spawn and finish
so the exit status is captured first, then finish() runs only after
launcher.stderr emits end or a bounded fallback deadline expires; do not wait
indefinitely for close because inherited descendant pipes may remain open.
Preserve existing error, timeout, and exit-code behavior, and add a regression
test covering immediate recursive diagnostic output being fully written before
the probe result is finalized.
In `@tests/codex-shim.test.ts`:
- Around line 322-341: Update the Unix launcher tests around installCodexShim to
use test.skipIf for the missing /bin/dash capability so the runner reports the
skip instead of returning silently. Add a focused companion test that sets
setCodexShimProbeShellForTests to a nonexistent shell path and asserts
installCodexShim returns installed === false, proving probeUnixShimInstall
honors the injected shell seam.
- Around line 902-916: The concurrent-replacement fixtures duplicate the output
format of successfulLauncher, allowing them to drift. In
tests/codex-shim.test.ts lines 902-916, derive the printf arguments from
successfulLauncher("concurrent updater replacement") instead of hard-coding the
script lines; likewise update lines 993-999 using successfulLauncher("concurrent
unsafe updater replacement") while preserving the exit 127 status for the
failed-probe path.
---
Outside diff comments:
In `@src/codex/shim.ts`:
- Around line 933-948: Update installCodexShim() so refresh success and state
persistence require all wrapper files to be installed successfully, not merely
any refreshShimFile() call returning true. Ensure the refreshed || allInstalled
path cannot write state or report success when allInstalled is false, including
when refreshShimFile() removes a wrapper after an unsafe probe.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3535a2b2-a6df-47e3-a7e9-92f327e30d75
📒 Files selected for processing (2)
src/codex/shim.tstests/codex-shim.test.ts
The resolve-pr job mapped a trusted CodeRabbit status SHA back to a PR via the commit-to-PR association index. That index can lag a fresh head push (seen on PR lidge-jun#1441), returning zero matches for a genuine current head, so revalidation was silently skipped and new unresolved findings never re-drafted the PR. When the index yields no unique match, fall back to reconciling directly against the live open-PR list by head.sha — the same authoritative identity the write gate uses. Resolution still requires exactly one open match, so ambiguity remains fail-closed. The fallback is a read and stays within the resolver's pull-requests:read permission. Add behavioural harness tests for the resolver: empty-index fallback, no-match skip, fresh-index direct resolve, and fail-closed on error.
The resolve-pr job mapped a trusted CodeRabbit status SHA back to a PR via the commit-to-PR association index. That index can lag a fresh head push (seen on PR lidge-jun#1441), returning zero matches for a genuine current head, so revalidation was silently skipped and new unresolved findings never re-drafted the PR. When the index yields no unique match, fall back to reconciling directly against the live open-PR list by head.sha — the same authoritative identity the write gate uses. Resolution still requires exactly one open match, so ambiguity remains fail-closed. The fallback is a read and stays within the resolver's pull-requests:read permission. Add behavioural harness tests for the resolver: empty-index fallback, no-match skip, fresh-index direct resolve, and fail-closed on error.
102cb51 to
35d3431
Compare
2 participants
Summary
codexback to the wrapper, as observed withmise exec -- codex.--versionprobe before committing state. Unsafe results and unexpected probe exceptions roll back the new launcher, prior owned backup, wrapper, and state transaction.ESRCHhandling.ocx codex-shim uninstallrecovery command, while a legitimate child Codex invocation remains allowed.upstream/devat8ab41d35b3b8508f6d6f81d8feb05bf24ff2367d.Closes #1439
Verification
bun run prepushon the latestdevbase after the final review fixes: 10,829 passed, 7 skipped, 0 failed; typecheck and privacy scan passed.bun test tests/codex-shim.test.ts(52 passed, 0 failed).bun run typecheck.bun run privacy:scan.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Tests