test(codex): give the shutdown regressions a timeout longer than their own bounds

[ndycode]

Summary

• Fixes a latent flake in the two shutdown regression tests added by fix(codex): route resume/fork through the canonical home and bound helper shutdown (#647) #648: both ran under Vitest's default 5s per-test timeout, which is shorter than the bounds they set for themselves.
• Found by running the suite on Linux as a non-root user, which is how CI runs. This repo's Actions have never executed, so nothing had ever run these tests that way.

What Changed

test/codex-bin-wrapper.test.ts gives both shutdown tests an explicit 30s per-test timeout.

Two distinct consequences of the missing timeout, both real:

1. The SIGTERM test could exceed 5s and fail. Its budget is the wrapper's 2s graceful window plus up to 4s of polling for the helper to be reaped, plus spawn overhead — over 5s in the worst case. It passed locally only because the helper usually died fast. As a non-root user on Linux it failed reliably with Test timed out in 5000ms.

2. The 12s spawnSync bound could never report anything. fix(codex): route resume/fork through the canonical home and bound helper shutdown (#647) #648 added that bound specifically so a stuck wrapper would fail with a useful message instead of hanging the run. But Vitest killed the test at 5s first, so the message never printed. The bound was inert for its stated purpose.

Validation

Linux, non-root, default timeout (the configuration that exposed this):

• Before: FAIL ... force-stops an app helper that ignores SIGTERM (#647) → Test timed out in 5000ms
• After: passes in ~2.1s

And against the unfixed wrapper, the diagnostic now surfaces as intended:

AssertionError: wrapper never returned within 12000ms: the helper ignored SIGTERM

rather than a bare Vitest timeout.

• npm run lint
• npm run typecheck
• npm test — Windows: 5315 passed, 4 skipped, 0 failed
• Linux container, root and non-root

Risk and Rollback

• Risk: very low. Test-only; no production code touched. A longer per-test timeout cannot mask a failure — the tests still assert their own 12s bound internally, so a stuck wrapper fails at 12s with a message rather than at 30s.
• Rollback: revert the commit.

Additional Notes

Worth flagging separately: vitest.config.ts sets no testTimeout, so every test in this repo runs under the 5s default. Any other test that spawns real subprocesses is exposed to the same class of flake. This PR only fixes the two tests it introduced.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr gives two shutdown regression tests enough time to reach their own bounded failure diagnostics instead of vitest’s 5s default timeout.

• adds a shared 30s per-test timeout.
• leaves production behavior and token handling unchanged.
• preserves the windows skip for the posix-only signal test; no windows filesystem risk or concurrency issue is introduced.
• no vitest coverage is missing for this timeout-only correction.

Confidence Score: 5/5

this test-only change appears safe to merge.

the 30s outer timeout exceeds all bounded work in both shutdown tests, while their existing 12s process timeout still produces the intended diagnostic first.

Important Files Changed

Filename	Overview
test/codex-bin-wrapper.test.ts	the explicit timeout safely exceeds the tests’ 12s process bound and approximately 4s polling allowance without changing their assertions.

_{Reviews (1): Last reviewed commit: "test(codex): give the shutdown regressio..." | Re-trigger Greptile}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

📝 Walkthrough

this is a minor, test-only change. it adds explicit 30-second timeouts to the two shutdown regression tests in test/codex-bin-wrapper.test.ts, which prevents vitest’s default 5-second timeout from masking the tests’ internal spawnSync and exit-polling diagnostics. no security, data-loss, or architectural risks are introduced.

• updates coverage for leaked helper stdio and helpers that ignore SIGTERM in test/codex-bin-wrapper.test.ts.
• preserves windows coverage and supports reliable execution across root and non-root linux users.
• no missing regression tests are apparent. the tests cover the reported shutdown failures.
• no new concurrency risk is introduced; the longer timeout only allows existing shutdown polling to complete.

Walkthrough

the shutdown tests now use a 30-second timeout. this allows their internal spawnSync timeout and exit polling to complete.

Changes

shutdown test timing

Layer / File(s)	Summary
configure shutdown test timeout test/codex-bin-wrapper.test.ts:672, test/codex-bin-wrapper.test.ts:3213, test/codex-bin-wrapper.test.ts:3279	defines SHUTDOWN_TEST_TIMEOUT_MS as 30 seconds and applies it to the leaked-stdio and SIGTERM-escalation shutdown tests. no new regression cases were added. windows-specific behavior and concurrency risks are not changed by this diff.

Estimated code review effort: 1 (trivial) | ~5 minutes

Suggested reviewers: fnmendez

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	the title uses the required conventional commit format, but its 81-character length exceeds the 72-character limit.	shorten the title to 72 characters or fewer while preserving the lowercase imperative summary.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	the description includes the required sections and clearly documents the test-only change, validation results, risks, and rollback plan.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/shutdown-test-timeouts

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/shutdown-test-timeouts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}