[lidge-jun]

Summary

On dev f924820, full-matrix run 37407882844 failed windows 9/9 on all three attempts in tests/update/update-restart-lease.test.ts. In each attempt, a spawnHolder child missed its 8 s acknowledgment window ("holder did not acknowledge acquisition"); the affected case differed between attempts.

The cause is the holder fixture. Before it can acquire the lease, it re-resolves the service-state paths in a fresh Bun child by cold-importing src/service/state.ts, which pulls in the whole config graph (about 460 modules and about 1 s of CPU). On hosted Windows runners that cold import has a multi-second I/O-latency tail. Instrumented holders spent up to 8.0 s of wall time on 0.6 s of CPU inside the config.ts import, and a cross-run scan of 24 Windows runs shows the same tail (holder cases at 4–19 s against a normal 0.8 s). The failures are that tail crossing the 8 s budget.

The fix: the holder now receives the paths the parent already resolved and proved contained in sandbox(), and imports only src/service/ownership-mutation-lease.mjs (Node builtins only). That is the same shape contenderAcquire and the restart-veto inline holder already use. The holder still checks containment against the sandbox root and the authority match before it acquires, and the parent still verifies the receipt and the lock owner PID. Child-side authority resolution from the stored service-manager environment stays covered by serviceManagerChildAuthority() (the case 1/2 controls and two restart-veto cases).

This is test-only. The patch changes no product source, and the stall sits in fixture import plumbing ahead of any lease logic. Diagnostics on the throwaway branch codex/b8-lease-windows-diag (runs 37414104873, 37415821537, 37417148609, 37419782477, 37419800317) ruled out the suspected product causes:

• The holder's import graph changed by one pure module between 7e7468a and f924820, and a sandboxed trace of the import spawned no processes.
• Process snapshots taken during a stall showed no orphaned PowerShell or icacls processes and no leftover Bun children.
• An in-run A/B test of ocx start with the Claude intercept on and off (fix(claude): retain approved Desktop picker trust across restarts #6639's Windows CA work) stalled in both arms.
• Switching the parent from a synchronous to an async acknowledgment wait made no difference.

These diagnostics do not show that product startup is free of similar cold-import latency on such runners.

Verification

No local tests were run, per owner instruction.

• Static checks: bun run typecheck, bun run privacy:scan, bun run structure:check, bun scripts/file-size-ratchet.ts all exit 0. No new files; this file has no ratchet entry.
• Independent review (sol reviewer, static): PASS, no blockers.
• Exact-head PR CI at c9bdcd7c26f629373b4fcebf7b61f5971344503d: run 37422534227 green (full-matrix jobs skip on pull_request by design).
• Full-matrix lane=all dispatches at the same head: windows 9/9 green in 37422532742 (job 112134885543) and 37422563221 (job 112137937070). Holder-driven cases now acknowledge in 53–373 ms (EOF release 252/140 ms, drain-cleanup 76/81 ms, missing-ack 53/373 ms), down from 0.8–1.1 s normally and 4–19 s on the tail. The 8 s cases sit at their designed graceful-timeout floor (8.05–8.22 s).
• In 37422532742, windows 6/9 and windows 7/9 failed in files this PR does not touch: the tray probe timing assertion (Expected: < 250, Windows tray packaging and command safety > terminates and later reaps hung, overflowing, or failed tray probes without stacking) and GET /api/settings reports external Codex ownership without an apply attempt (10.5 s). Every Windows shard passed in 37422563221; the failed jobs are being rerun.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (test-only; none needed).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (no product or credential paths touched).

[github-actions]

✅ Deterministic PR hygiene checks passed.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-06T06:15:40.949555Z	c9bdcd7	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

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: 305999bc-ec38-43ce-8316-e0855758295b

📥 Commits

Reviewing files that changed from the base of the PR and between f924820 and c9bdcd7.

📒 Files selected for processing (1)

• tests/update/update-restart-lease.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

The update lease test now resolves service-state paths in the parent process and passes them to the spawned holder. The holder retains its child-side containment and authority checks.

Changes

Update lease holder

Layer / File(s)	Summary
Parent-resolved lease holder paths tests/update/update-restart-lease.test.ts	spawnHolder resolves serviceStatePaths() in the parent and embeds the paths in the child. Comments describe the separate service-manager authority probe and the measured cold-start timing for the import.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to c9bdc

The fixture change preserves its path, authority, and lease checks while avoiding the holder’s cold import. No concrete merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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
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	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: the lease-holder fixture in tests/update/update-restart-lease.test.ts no longer cold-imports the service graph.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

• Autopilot · 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[lidge-jun]

Maintainer integration (lidge-jun, dev only) — 2026-10-05 bug triage round 2, lane B8.

• Head: c9bdcd7
• CI at this head: PR CI 37422534227; full-matrix dispatch 37422563221 success (windows 9/9 green); every executed job passed.
• Review: test-only: lease holder fixture receives parent-resolved paths and imports only ownership-mutation-lease.mjs instead of cold-importing the ~460-module service/config graph, removing the Windows I/O-latency tail that crossed the 8 s ack budget; containment and authority checks preserved.
• No local tests were run, per owner instruction; hosted CI is the evidence.
• scripts/ci/assert-mergeable-review.sh --maintainer-integration: OK.