test(dsh): drain server fixtures before removing homes by luvs01 · Pull Request #7305 · lidge-jun/opencodex

luvs01 · 2026-10-18T10:11:26Z

Summary

The DSH server fixtures can leave Windows ACL work and a listener's spend-ledger owner alive when a case times out. Drain startup/background work and reap ACL children before removing fixture homes, and track servers outside each case so teardown joins the same memoized stop operation as its local finally.

Use the existing current-install fixture projections and exclude unrelated native Codex synchronization. Handle Request URLs in the upstream mock, assert the imported synthetic token at dispatch, and refuse unexpected external requests. Only the three cases that start a real listener use the existing 30-second SERVER_BUDGET_MS; Bun's global 5-second default stays unchanged. The two load-rejection cases measured 5.58s and 6.05s on Windows. This changes one existing test file and no production code.

Verification

Validated on dev base 8640c2f30edbd2fbb6947ed5c25e2793f5ae72a6 with Bun 1.4.3, the official preload and shared test-run lock:

bun test tests/providers/dsh-account.test.ts: 24 passed, 0 failed, 80 assertions. No cleanup-hook timeout, EBUSY, spend-owner conflict or background-work leak warning; the test sandbox was removed.
Production ablation, without changing test expectations or mocks: temporarily remove DSH credential retirement and invalid-row load rejection, then run the three server cases once. All three failed their intended assertions in 1.70–1.76s, not by timeout. Production files were restored byte-for-byte and their Git diff is empty.
bun node_modules/typescript/bin/tsc --noEmit: passed.
bun scripts/structure-ssot.ts: passed after materializing the unchanged tracked AGENTS.md omitted by the sparse checkout.
Official scanText privacy check and file-size-ratchet evaluate/isOffender for the one changed file: passed; 608 lines, below the existing 2,000-line threshold.
git diff --check: passed.

Forced-timeout cleanup remains UNVERIFIED: the successful run and assertion-failure ablation exercised ordinary/error teardown, without an artificial runner timeout. The lifecycle ablation stops at its retirement assertion; its later second-request assertions passed in the normal 24-case run. The broader repository suite was not rerun locally for this isolated test-maintenance change. Current-head CI run 39118271695 completed successfully on d75d8c59e2d6b9bd06de0c5fac8d871a377066b4: Ubuntu test shards 1/4 through 4/4 and the enabled gates passed. At readiness verification, all head-bound results were complete: 18 success including CodeRabbit, 11 skipped, and no pending or failed results. The policy-skipped checks, including Windows/macOS lanes, do not establish cross-platform coverage. No reviews or inline review threads were present. This PR is ready for review; maintainer approval and merge remain separate.

Checklist

Scope stays focused and avoids unrelated cleanup.
Docs or release notes were updated when needed (no product behavior change).
Changed paths were checked for secrets, auth changes and unsafe defaults; the scoped privacy check passed.

Summary by CodeRabbit

Tests
- Improved automated checks for account lifecycle behavior, including server cleanup, startup handling, request validation, and bounded execution time. These checks help catch regressions in account setup and teardown.

github-actions · 2026-10-18T10:11:37Z

✅ Deterministic PR hygiene checks passed.

coderabbitai · 2026-10-18T10:11:39Z

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: 31d4e5fe-ebbe-418e-911b-4114e7ce3323

📥 Commits

Reviewing files that changed from the base of the PR and between 8640c2f and d75d8c5.

📒 Files selected for processing: 1

tests/providers/dsh-account.test.ts

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

📝 Walkthrough

The DSH account tests now use tracked server cleanup and asynchronous fixture teardown. They also use shared server configuration and budgets, and validate exact provider request URLs and authentication headers.

Changes: DSH account tests

Layer / File(s)	Summary
Tracked server and fixture cleanup `tests/providers/dsh-account.test.ts`	Tests track server instances, stop listeners during teardown, flush background startup and config-hardening work, and remove fixture homes afterward. Lifecycle and wire-pin tests use shared fixture configuration and server budgets.
Provider request checks `tests/providers/dsh-account.test.ts`	Mocks match exact Anthropic and DeepSeek endpoints, assert the DSH auth-token header, pass through only localhost HTTP requests, and reject other destinations.

Priority: ⬇️ Low

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

Change: Other

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to d75d8

The fixture cleanup changes appear ready to merge after normal checks. Forced-timeout behavior has not been exercised, but no failure is established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the main change: draining server fixtures before removing test homes.
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.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

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

Ingwannu

Reviewed the complete one-file diff at d75d8c5 against dev 8640c2f. The fixture now joins the existing memoized listener stop, startup-release/config-hardening drains and background/ACL teardown before home removal; it preserves environment restoration on cleanup failures and confines the larger timeout to real-listener cases. The upstream mock checks the imported synthetic token and refuses unexpected external destinations. No production behavior changes. I verified hosted run 39118271695 succeeded at this exact head, including all four Ubuntu test shards and gates; native Windows/macOS execution was skipped and forced runner-timeout cleanup remains unverified as documented. No local suite or live credential/service execution was performed in this review. Approval covers this test-maintenance patch; merge/release are separate actions.

…ate paths and shim token handling (B5) (#7397) * fix(oauth): allow dismissing a settled failed login flow (carry #7379) Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com> * fix(providers): preserve explicit API-key mode when keys are unavailable (carry #7304) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(dsh): bound account profile response reads (carry #7333) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * test(dsh): drain server fixtures before removing homes (carry #7305) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(claude-cli): keep local errors out of subscription cooldown (carry #7343) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(claude): bound Desktop library reads and preflight writes (carry #7301) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(service): stop scheduler wrappers before Windows reinstall (carry #7313) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(startup): require RunAtLoad before Desktop relaunch (carry #7277) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(codex): strip OpenSSL startup variables from updater child env (carry #7342) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(codex): keep the service token out of bypassed shim launches All three Codex shims loaded the service API token into OPENCODEX_API_AUTH_TOKEN before checking OCX_SHIM_BYPASS, so a bypassed native passthrough launch handed the proxy data-plane token to the real Codex and its descendants. The POSIX, CMD and PowerShell shims now skip the token load under bypass; the Unix revision marker moves to 4 so installed wrappers are refreshed. loadServiceTokenFromFile reads the inherited OCX_API_TOKEN_FILE through a no-follow, nonblocking descriptor, accepts only a regular file of at most 4 KiB and returns only a single-line printable token. * test(claude): wait for a real ctime transition before requiring rejection Re-applying the same utimes moves ctime only after the kernel clock ticks, so the real ctime case from #7301 often observed no change and the reader correctly returned {}. Repeat the call until ctime has advanced (bounded), assert that precondition, then require desktop_config_not_regular. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(google-antigravity): admit a probed project_id fallback in account import (carry #7385) An Antigravity account whose Cloud Code Assist project is not auto-discovered was rejected with missing_project even when the operator knows its project. A Cockpit record may now carry a bounded project_id (Google Cloud project grammar, 6-30 chars, exact match). Discovery keeps precedence; the fallback is admitted only after the account quota probe succeeds with that account own access token against that project. Probe refusal, probe errors and cancellation write nothing. No shared default project. Co-authored-by: yobushka <51312852+yobushka@users.noreply.github.com> * fix(quota): name a closed reason when an OAuth account quota probe fails Anthropic per-account quota rows reported quotaUnavailable with no reason, and Antigravity reported account_unavailable for any credential-side throw. The Anthropic usage read now reports a closed code for an HTTP status class, an unusable body or a thrown dispatch, and a probe that cannot obtain a credential reports account_unavailable without dispatching. Antigravity distinguishes a credential refresh deadline (timeout). Codes reuse the existing closed QUOTA_FAILURE_CODES set, carry no upstream text and never change account health. * test(claude): match the symlink-resolved rollback target in the picker restore case The atomic writer renames onto the symlink-resolved target, so on macOS (/var -> /private/var) the rollback rename never matched the fixture path and the restore-failure injection did not fire. Compare both spellings. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(google-antigravity): cancel the import project probe with the import request probeAntigravityUsageQuota accepts an optional caller signal that cancels every request of the probe and stops before the models fallback; the import verifier forwards the import signal and never admits after cancellation. * fix(codex): upgrade installed Windows shims that load the token before bypass CMD and PowerShell wrappers now carry a Windows revision marker. A marked wrapper without it (installed before the bypass-token fix) is obsolete: installCodexShim and autoRestoreCodexShim rewrite it in place over the same backing launcher instead of reporting it healthy or already installed, and diagnostics report it unhealthy. On Windows the service token reader also binds the opened descriptor to the path by identity, so a link swapped in after lstat is refused. * test(codex): mark the in-place Windows diagnostics fixture with the current revision * fix(codex): refresh obsolete Windows shims without clobbering a concurrent replacement The obsolete-wrapper refresh truncated whatever occupied the wrapper path. It now moves the observed wrapper aside and proves it is the probed file, stages the current wrapper beside it, and publishes with a hard link that refuses an occupied path. A concurrent replacement is restored or kept untouched and the refresh reports deferred. * docs(structure): record the Windows shim refresh transaction * fix(codex): create the Windows shim refresh stage exclusively and resume partial upgrades The refresh stage is now created with O_CREAT|O_EXCL (and O_NOFOLLOW on POSIX) under an independent random name, and publication, withdrawal and cleanup are bound to the identity of the file this refresh created, so a planted link is never written through. Auto-restore resumes a Windows revision-only sibling mix (current plus obsolete wrappers) instead of deferring forever after one transient publish failure. * fix(codex): bind Windows shim refresh rollback and recovery to observed identities Publication keeps a linked entry only if it is the created stage, and withdraws a link to a swapped stage (file or symlink) while leaving independent replacements. Restoring the moved wrapper requires the aside to still be the moved file. Automatic revision recovery now refreshes only the admitted obsolete wrappers and first re-verifies every current sibling, so a replacement during the stability wait is never adopted. * fix(codex): never displace a Windows shim where its replacement cannot be published The refresh proves hard-link support in the wrapper directory with a throwaway pair before moving the wrapper aside, and if a restore link still fails it falls back to an identity-checked rename onto the empty path, so the launcher never stays missing. --------- Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com> Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: yobushka <51312852+yobushka@users.noreply.github.com>

lidge-jun · 2026-10-19T21:58:25Z

Carried into #7397 (squash dbb908f) on dev with a Co-authored-by trailer, as part of the B5 OAuth/accounts/service hardening bundle. It ships in the next release (2.90.0). Thanks!

test(dsh): drain server fixtures before removing homes

d75d8c5

github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Oct 18, 2026

luvs01 marked this pull request as ready for review October 18, 2026 10:26

luvs01 requested review from Ingwannu and lidge-jun as code owners October 18, 2026 10:26

Ingwannu approved these changes Oct 18, 2026

View reviewed changes

luvs01 mentioned this pull request Oct 18, 2026

fix(dsh): bound account profile response reads #7333

Closed

3 of 4 tasks

lidge-jun mentioned this pull request Oct 19, 2026

fix(oauth): harden OAuth login/import, quota diagnostics, service/update paths and shim token handling (B5) #7397

Merged

3 tasks done

lidge-jun closed this Oct 19, 2026

Navigation Menu

Repository navigation

test(dsh): drain server fixtures before removing homes - #7305

test(dsh): drain server fixtures before removing homes#7305
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:codex/stabilize-dsh-server-fixtures-20261018

luvs01 commented Oct 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

Uh oh!

Ingwannu left a comment

Uh oh!

lidge-jun commented Oct 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

luvs01 commented Oct 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Checklist

Summary by CodeRabbit

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

coderabbitai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ingwannu left a comment

Choose a reason for hiding this comment

Uh oh!

lidge-jun commented Oct 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

luvs01 commented Oct 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading