Repository navigation
test: repair stale CI contracts and replace copied IPC inventories - #29499
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
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 9… |
| 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. |
| Title check | |
The title clearly identifies the two main changes: repairing stale CI contracts and replacing copied IPC handler inventories. |
| Description check | |
The description is detailed and aligned with the changes. It covers the required explanation, rationale, linked issue context, visual proof, testing, review notes, compatibility considerations, and ch… |
Full details: Docstring Coverage
-
Explanation
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 9 files.
Full details: Description check
-
Explanation
The description is detailed and aligned with the changes. It covers the required explanation, rationale, linked issue context, visual proof, testing, review notes, compatibility considerations, and checklist. The linked issue section lists related issues rather than using an explicit "Fixes #" entry, but the description is otherwise complete.
✨ Finishing Touches
-
📝 Generate docstrings
-
- 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.
OrcaWin
changed the title
ELI5
Restore full PR validation after recent additions left stale draft/job and IPC test expectations, a lint-rejected mirror annotation, a file over its line cap, and two browser-bridge RPCs missing ownership classifications. These failures currently block otherwise valid PRs.
What Changed
Use the actual job conditions to check ready docs-only and code PR admission, plus draft/ready cache caller admission. Keep README/root guard command checks and Windows cache OS coverage. Remove the two repeated unconditional-job assertions; move the complete ready-guard declaration into the existing draft-admission suite, which owns that policy and has room within the line limit. Extend the existing cache event table with a draft row rather than adding another suite. Also replace the pairing suite’s two copied handler inventories with one strict re-registration test that rejects duplicate handlers and verifies one retained binary listener and a usable current list callback. Use the existing named response tab type in the mirror test instead of lint-rejected
object. Combine the duplicate execution-host type/value imports in runtime-host-services.ts to repair main’s 301-line lint failure without changing behavior or raising the cap. Classify desktopBrowserBridge results as host-owned and its long-lived attach method as a stream; retain the complete RPC inventory check and extend the existing semantic test. No workflow, transport or RPC-schema changes.Why
#29484 intentionally gates draft jobs. Treating that change as a failure or deleting executable guard tests would be wrong. Evaluating both the cache caller and its reusable jobs proves the real admission chain, while existing shared draft tests retain transition and cancellation coverage.
Linked Issue
Internal CI audit; follows #29484, #29371, #29472 and #29384. Consolidates the independently reviewed repairs from #29493 and #29498 so the full gate can exercise them together.
Visual Proof
N/A — CI contract, import and ownership metadata repairs; no visible UI changes.
Testing
Reproduced the seven original failures. Config candidate: 231 passed across the five changed suites and aggregate/ready/reuse siblings. IPC candidate: all 18 on Bun and Node; 46 owner/related cases passed. Mirror candidate: three target tests before and after, 11 target/related tests passed. Combined commit: all 288 cases in 15 suites passed. Changed quality, normal/anti-slop lint, formatting and whitespace checks pass. Node and renderer typechecks pass for their individual repairs; both also pass on the combined commit. Production edits consolidate a type-only import and repair browser-bridge ownership metadata. All existing declarations retained, one moved; one cache table row added. Linux Bun 1.4.3; reporter's Node compatibility version is 26.3.0, subprocess PATH pins Node 24.21. No desktop launch.
Controls: requiring should_run on the detector fails the ready docs-only assertion in its new owner; making the platform cache caller unconditional fails the draft row. Both workflow files restored byte-for-byte. IPC controls catch omitted restart cleanup as a duplicate handler and omitted binary cleanup as three listeners instead of one; all production files restored. Independent final preservation review approved the move and retained guard/event/Windows contracts.
Single-worker local official summed module observation across the same five suites: original 1,602 ms with seven expected baseline failures; candidate 1,570 ms with all cases passing. Process wall 3,493 → 3,352 ms. Failed original and changed table workload prevent a valid speed comparison; no savings claimed. IPC three alternating pairs against a documented list-repaired baseline: module median 2,234 → 2,275 ms, wall 3,750 → 3,888 ms, all19vs18cases pass. Mirror unpaired module observation 818 → 1,607 ms, allthreepass. No timing result establishes a speedup. Hosted run 38863013737 stopped at an inherited main max-lines failure before units; no hosted timings exist for that run. After merging main at 224a139, full Oxlint, Node typecheck, changed quality and 282 cases across eight affected suites pass. The isolated lint command fails before import consolidation (301 lines), passes after (300 lines); single local wall observations 125 ms → 124 ms, with no speed claim. Fresh run 38863500334 passed four unit shards; shard 2 caught the missing browser-bridge ownership classifications inherited from main. The owner regression with new semantic expectations fails two of four cases before the metadata fix and passes all four after. Five owner/host-route/bridge suites pass 20 cases; changed quality and Node/renderer typechecks pass. Removing only the attach stream override fails its semantic assertion even though the complete inventory passes; restored owner passes. Single local module observations 1,363 ms (failed baseline) → 1,443 ms (passing candidate), wall 2,896 → 2,964 ms: no speed comparison or savings claim. Exact-head full PR run 38864838937 passed, including aggregate PR checks passed. All five unit shards are clean and source-qualified (12,305 files). The eight target suites passed 234 cases with no skips on Bun 1.4.3 / Node 24.19.0; official summed worker module cost was 3,650 ms. This is a candidate observation, not a speed comparison; no clean original hosted baseline exists.
Review
Independent reviews confirm preservation of executable guards, event coverage, caller-versus-reusable admission, Windows x64/ARM coverage, IPC lifecycle controls and mirror response typing. No lint suppression or production test seam. Browser attachment stream/result host classification also receives independent source and dispatch review.
Agent skill upstream boundary
Notes
Browser-bridge ownership now matches the existing host registry and authenticated streaming attachment. No bridge permissions, routing implementation, SSH transport, platform, folder or wire changes. This also aligns CI proof with the existing draft policy.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)