Repository navigation
Conversation
…lidation
Four issues found while stress-testing on Windows 10 + Git Bash:
1. **Bug: `/codex:cancel` fails on Windows + Git Bash with `Invalid argument/option - 'C:/Program Files/Git/PID'`.**
`lib/process.mjs` and `lib/app-server.mjs` set `shell: process.env.SHELL || true`
on Windows. Under Git Bash `process.env.SHELL` resolves to `/usr/bin/bash`,
and MSYS path translation rewrites `taskkill /PID 123 /T /F` to
`taskkill /c/Program Files/Git/PID 123 /T /F`. Pinning the shell to
`cmd.exe` keeps `.cmd` shim resolution (PATHEXT) but skips Bash path
translation.
2. **Improvement: Reconnect storm on permanent OpenAI auth failures.**
When the underlying CLI gets a 401/403 from OpenAI, the app-server retries
up to 5 times before surfacing the failure. Add a guard in
`lib/codex.mjs#applyTurnNotification` so that whenever an `error`
notification matches a permanent-auth pattern (401, 403, "Missing bearer",
"invalid api key"), we short-circuit `captureTurn` instead of waiting for
subsequent reconnects to fail too. Cuts review-without-auth from ~60s to
~20s in local testing.
3. **Bug: Invalid `--base <ref>` is only caught after a Codex thread spins up
and round-trips to the API.**
`lib/git.mjs#resolveReviewTarget` accepted the `baseRef` verbatim. Add a
`git rev-parse --verify --quiet <ref>^{commit}` pre-check so a typo or an
unfetched branch fails immediately with a guidance message instead of
surfacing later as a confusing API error.
4. **Improvement: `DeprecationWarning DEP0190` on every spawnSync invocation.**
Same root cause as bug 1 — moving from `shell: SHELL || true` to
`shell: "cmd.exe"` (Windows only) keeps the warning narrowed to genuine
shell-resolution paths instead of leaking on every `taskkill`/`git`/binary
probe call.
Verified locally with the existing stress-test scenarios: setup, status,
result, cancel, review (with/without auth), adversarial-review, task missing
prompt, invalid scope, invalid base ref, long focus text.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
fireblue
added a commit
to fireblue/codex-plugin-cc
that referenced
this pull request
May 17, 2026
…, openai#190) - Isolate session and plugin-data env vars in tests (openai#127) - Fix Windows + Git Bash compatibility and auth-retry hang (openai#294) - Fall back to tmpdir state when plugin-data state is empty (openai#125) - Sanitize codex child process env (openai#190) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Jafee
added a commit
to Jafee/codex-plugin-cc-stable
that referenced
this pull request
Jun 9, 2026
…mark openai#312 superseded Both Opus and Codex (gpt-5.5 xhigh) independently APPROVE the openai#361 reconcile (8be4ab3): all 9 red lines clear, 0 security findings, 3 converged LOW non-blockers (onSocketGone double-event idempotent-but-untested, CEILING_MS backstop untested, openai#355 background-job interaction noted). openai#312 is superseded by openai#361's correct Promise.race implementation; openai#294/openai#343/openai#355 remain HOLD. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Jafee
added a commit
to Jafee/codex-plugin-cc-stable
that referenced
this pull request
Jul 8, 2026
Port of openai/codex-plugin-cc v1.0.6 (net diff 80c31f9..db52e28, PR openai#447). On win32 runCommand defaulted to shell: process.env.SHELL || true, so git()/gitChecked() ran repository-derived arguments (baseRef, branch names, paths) through cmd.exe or the user's shell — a malicious repo could inject commands via a crafted default-branch name. git commands now force shell: false (after the options spread, so callers cannot override it back); runCommand and the test helper honor an explicit options.shell via ?? while keeping the old platform default otherwise. Regression test: a branch named main&branch-helper&x must reach git literally and never execute the repo-local branch-helper.cmd. Fork notes: tests/helpers.mjs merged with the fork's timeout option; upstream version hunks replaced by the fork's own versioning. This supersedes HOLD item openai#294 (same root cause, fixed at the source). Dual review: Claude (Fable 5) APPROVE + Codex (gpt-5.5 xhigh, fresh read-only, pinned-SHA static audit) APPROVE. Clean-env suite 124/124. Co-Authored-By: bryane-oai <[email protected]> Co-Authored-By: Claude Fable 5 <[email protected]>
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
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.
Four issues found while stress-testing on Windows 10 + Git Bash. All four fixes are surgical — no behavior change on POSIX, no public API change.
Bug 1 (critical) —
/codex:cancelis broken on Windows + Git Bash.lib/process.mjssetsshell: process.env.SHELL || true. Under Git Bash, SHELL resolves to bash, and MSYS path translation rewritestaskkill /PID 123 /T /Ftotaskkill /c/Program Files/Git/PID 123 /T /F. Pin to cmd.exe so .cmd shims resolve but Bash translation doesn't kick in.Bug 2 (high) — Reconnect storm on 401/403 from OpenAI (10 retries before surfacing). Add a permanent-auth-error short-circuit in codex.mjs. ~60s → ~20s.
Bug 3 (medium) — Invalid
--base <ref>only fails after the Codex thread is up. Add a localgit rev-parse --verifycheck.Bug 4 (low) — DEP0190 noise on every spawnSync. Same root cause as Bug 1.
Verified all stress-test scenarios. POSIX behavior unchanged. Happy to split into 4 PRs if easier to review.
🤖 Generated with Claude Code