Skip to content

test(codex): drain quota fixture owners before home removal - #7001

Closed
Ingwannu wants to merge 1 commit into
devfrom
fix/7000-quota-fixture-cleanup-s119
Closed

Ingwannu wants to merge 1 commit into
devfrom
fix/7000-quota-fixture-cleanup-s119

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Closes [Bug][CI]: codex-quota-auto-refresh test cleanup hits EBUSY on a Windows shard #7000: migrate the Codex quota-auto-refresh case teardown to the existing drainAndRemoveFixtureRoots helper instead of a bare synchronous rmSync.
  • The helper settles producers and root-specific config hardening, closes the request-history connection, drains Windows ACL children, then uses the shared bounded transient-removal retry policy. Environment restoration stays in its finally path. A failed drain preserves the root and still fails the case.
  • Reuse the existing lifecycle owner instead of adding sleeps, assertion retries, wider deadlines, or product changes. Windows handle release needs an active event loop; synchronous removal retry alone cannot observe an asynchronous reap. Update the owning cleanup contract document in the same change.
  • Quota/401 behavior, assertions, credentials, runtime and CI configuration are unchanged. No GUI change.

Verification

  • Base: trusted dev 3e58c2ce3ee5433901ed467fe87c32fb4b5c5e77; candidate 6f50b502e1.
  • git diff --check: passed.
  • Final-head local focused execution on Linux/Bun 1.4.0: bun test tests/codex-integration/codex-quota-auto-refresh.test.ts tests/ci-workflows/fixture-teardown-helper.test.ts tests/ci-workflows/test-sandbox-cleanup.test.ts — 44 pass, 0 fail, 161 assertions. Includes the actual quota fixture hook, delayed synthetic ACL reaps, failure-gated removal and environment restoration.
  • bun scripts/structure-ssot.ts — pass. Every local execution used ubuntu1000, newly created private temporary homes, verified hard cgroups, nice10, SwapMax0 and finite deadlines: focused CPU75%/MemoryHigh1280M/Max1536M/Tasks64; structure CPU50%/384M/512M/Tasks32. No overlapping builds/tests.
  • Local bun node_modules/typescript/bin/tsc --noEmit — incomplete/nonpassing, reached the 240-second cgroup wall limit (exit1/timeout; process killed at cleanup). No diagnostics were emitted. Limits were not raised and no unbounded retry was made.
  • Exact-head hosted Typecheck passed: automatic Cross-platform CI run 38263934278, gates job 114866245839, Typecheck step success at this candidate. This is hosted evidence, not a local typecheck pass. Other current-head jobs remain in progress; not claiming all required CI green.
  • Protected runtime files compare unchanged (existence/UID/GID/mode/size/SHA256); no service/file permission change. No full local suite under the shared-host resource exception.
  • No independent Windows EBUSY reproduction or exact-head Windows CI pass is claimed. Full-suite execution is deferred under the shared-host resource exception, not described as successful.
  • No security/privacy scan was run locally or dispatched manually. The user permits only this newly created PR's automatic privacy CI. No real provider/native client, service restart or runtime-home change.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No authentication/credential/workflow implementation changed; this is test-only teardown reuse, not a waiver for any future boundary change.

Latest hosted receipt (2026-10-12): exact-head Cross-platform CI 38263934278 completed successfully at 6f50b502e13cf25aa46f025e46d88f7ff91f2dc4, including all four selected Linux test shards and the gates job/typecheck. Full Windows and macOS test shards were unselected/skipped, so this is not a native Windows EBUSY acceptance receipt. PR is Ready; @lidge-jun review requested. No approval/merge/release is claimed.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 90876d50-14b7-439d-ac83-22a42847e8ba

📥 Commits

Reviewing files that changed from the base of the PR and between 3e58c2c and 6f50b50.


📒 Files selected for processing (2)
  • structure/ops/test-sandbox-cleanup.md
  • tests/codex-integration/codex-quota-auto-refresh.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough
📝 Walkthrough

Walkthrough

The quota-auto-refresh test now performs asynchronous fixture teardown through drainAndRemoveFixtureRoots. It retains fetch restoration and account-state resets. The cleanup documentation now lists the test’s temporary roots.

Changes

Codex quota test cleanup

Layer / File(s) Summary
Async fixture teardown
structure/ops/test-sandbox-cleanup.md, tests/codex-integration/codex-quota-auto-refresh.test.ts
The cleanup documentation lists the quota-auto-refresh test roots. The test removes its synchronous directory check and removal, then passes its temporary root and an OPENCODEX_HOME restoration callback to drainAndRemoveFixtureRoots.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low


Merge Risk: ⚪ Minimal · up to 6f50b

The updated teardown waits for fixture cleanup and retries transient removal failures, reducing the chance that delayed Windows handles fail the quota-test shard. No actionable merge blocker remains.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check Passed Issue #7000 requires the quota test cleanup to tolerate delayed Windows handle release or release the held handle before the case ends. tests/codex-integration/codex-quota-auto-refresh.test.ts now u…
Out of Scope Changes check Passed The reported pull-request diff changes only the quota-auto-refresh teardown integration and its owning cleanup contract document. The test keeps quota behavior, assertions, credentials, runtime config…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: the Codex quota fixture teardown now drains fixture owners before removing the temporary home. It is concise, specific, and aligned with the changes in tes…

✨ 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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Oct 12, 2026
@Ingwannu
Ingwannu marked this pull request as ready for review October 12, 2026 15:16
@Ingwannu
Ingwannu requested a review from lidge-jun as a code owner October 12, 2026 15:16
lidge-jun added a commit that referenced this pull request Oct 12, 2026
fix: carry five contributor fixes for the 2.84.0 train (#7005 #7001 #7003 #7002 #6968)
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev through #7008 (merge a63267c134), carried as commit 5f3481b with your authorship and a Co-authored-by trailer preserved. Thank you — this ships in 2.84.0.

@lidge-jun lidge-jun closed this Oct 12, 2026
phillipod pushed a commit to phillipod/opencodex that referenced this pull request Oct 12, 2026
…n#7001)

Carried from lidge-jun#7001 into the 2.84.0 release train (lane L2).
Fixes lidge-jun#7000.

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
phillipod pushed a commit to phillipod/opencodex that referenced this pull request Oct 12, 2026
Plan, architect consultation, audit amendments and final review for the
2.84.0 release-train contributor carry (lidge-jun#7005 lidge-jun#7001 lidge-jun#7003 lidge-jun#7002 lidge-jun#6968).
@lidge-jun lidge-jun mentioned this pull request Oct 13, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants