Repository navigation
test(windows): use a never-allocated PID for the pending-child recovery fixture - #7949
Conversation
…ry fixture A reused exited-child PID made the recovery parent look alive, so recovery fenced before the publication stage the failure-fence cases inject into (dev batch 40824593871).
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. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Set and verify an absent PID tests/codex-integration/codex-cli-update-windows-pending.test.ts |
The setup assigns 0x7ffffffc to deadPid and asserts that process.kill(deadPid, 0) throws with code ESRCH. It removes the spawnSync import previously used to obtain an exited child process PID. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Other
Possibly related PRs
- lidge-jun/opencodex#7648: Adds Windows CLI update recovery based on process and Job lifetime evidence. This test change supplies a fixed absent PID for the recovery-parent precondition.
- lidge-jun/opencodex#7909: Adds pending-child recovery reconciliation and includes the Windows test file changed here. This test change makes the dead-parent precondition deterministic.
Suggested labels: chore
Suggested reviewers: ingwannu
Merge Risk: 
e7017
-
This PR makes the Windows recovery test deterministic without changing product code or behavior.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "test(windows): use a never-allocated PID for the pending-child recovery fixture" matches the main change. The test fixture no longer uses a short-lived child's PID for deadPid. On native Wi… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "test(windows): use a never-allocated PID for the pending-child recovery fixture" matches the main change. The test fixture no longer uses a short-lived child's PID for deadPid. On native Windows x64 it now uses 0x7ffffffc, which is asserted to read as ESRCH. The objectives describe the same rationale: Windows PID reuse could make the recovery parent appear alive. The title is specific, concise, and single-sentence, and it uses the conventional type(scope) prefix.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Summary
The dev
lane=allbatch 40824593871 failed on hosted Windows (job 123284678453):reconciliation publication failure fences: file-flush / harden / renameintests/codex-integration/codex-cli-update-windows-pending.test.ts:124(Expected: > 0, Received: 0).readbackandflushpassed in the same file, and the same cases passed in the previous batch. No source in the Codex CLI updater changed between the two batch heads.The cause is in the fixture.
beforeAllused the PID of a short-lived child asdeadPidand assumed it stays dead for the whole file. Windows reuses PIDs quickly, and a busy runner can hand that PID to another process. The records usedeadPidas the recovery parent, so a reused PID makes the parent look alive. Recovery then correctly fences admission before it reaches the publication step these cases inject into,injectedstays 0, and the assertion fails. The product behaved correctly.This uses
0x7ffffffcinstead. It is a multiple of 4 like every Windows PID, inside the record validator's range (<= 0xffffffff), and far above anything Windows allocates, so it stays dead for the whole file.beforeAllasserts it reads asESRCH, the same signalprocessGoneuses. No assertion or product code changes.Verification
Windows x64,
OCX_TEST_NO_QUEUE=1:deadPid = process.pid:bun test tests/codex-integration/codex-cli-update-windows-pending.test.ts -t "reconciliation publication failure fences"gives 0 pass / 5 fail, each withReceived: 0, the hosted symptom.bun run typecheck: exit 0.tests/ci-workflows/file-size-ratchet.test.ts: pass.devcheck after this lands.Checklist
Summary by CodeRabbit