Repository navigation
Test bounded stale-port retirement after listener exit - #11356
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds regression tests for delayed port retirement across panel and workspace scopes. It registers the new test file in the Xcode test target. ChangesPort retirement regression coverage
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Merge Risk: ⚪ Minimal · up to This change adds deterministic regression coverage and test-target wiring without modifying production runtime behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
Full details: Description checkExplanation The description provides a clear summary, context, verification results, limitations, and scope. It omits the template checklist, review-trigger block, and demo-video section, but these omissions are non-critical for this test-only change. Full details: Linked Issues checkExplanation The changes address issue [ Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.) Full details: Cmux Swift Actor IsolationExplanation PASS: The PR diff adds only two Swift test files and registers one test file in the Xcode project. It introduces no production Swift models, protocols, services, shared mutable Sendable references, or UI-store access. The changed declarations are test suites and test helpers, which the check explicitly allows. Full details: Cmux Swift Blocking RuntimeExplanation PASS — The isolated PR diff changes only two Swift test files and Xcode test-target wiring. The tests use finite Full details: Cmux Browser Automation Off-MainExplanation The pull request adds only port-scan regression tests and Xcode test wiring. The browser automation rule applies to browser socket commands in Full details: Cmux Expensive Synchronous LoadExplanation PASS: The pull-request diff adds only two test files and Xcode test-target wiring. The changed Swift code calls Full details: Cmux Cache Substitution CorrectnessExplanation PASS: The pull-request diff contains only two new Swift test files and four Xcode test-target wiring lines. It does not change production Swift, TypeScript, or JavaScript, and it does not substitute a cached value for an authoritative read in a production persistence, history, undo, or snapshot path. Full details: Cmux No Hacky SleepsExplanation PASS. The pull-request diff adds only two Swift test files and Xcode project wiring. It adds no production TypeScript, JavaScript, shell, or build/runtime script changes, and the added lines contain no sleep, timer, polling, or fixed-delay logic. Deterministic test scaffolding is explicitly allowed by the rule. Full details: Cmux Algorithmic ComplexityExplanation PASS. The PR-only commits change two test files and add their Xcode test-target wiring; they introduce no production Swift, TypeScript, JavaScript, shell, or runtime code. The added loops run exactly three iterations over fixed test data, and the Full details: Cmux Swift ConcurrencyExplanation PASS: The PR adds only two Swift test files and Xcode test wiring. The added tests use synchronous Full details: Cmux Swift `@Concurrent`Explanation PASS: The PR changes only two synchronous Swift test functions plus Xcode project wiring. Neither added test uses Full details: Cmux Swift Package BoundariesExplanation PASS. The isolated PR series changes only two Swift test files and test-target wiring in ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Final review audit — rechecked against HEAD
Codex and Greptile have no top-level review body or inline thread on this PR. Final recheck against HEAD: hosted run #13824 ( |
fe92edc to
dcfda79
Compare
|
All contributors have signed the CLA ✍️ ✅ |
dcfda79 to
d54e736
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
3cce67c Vault: recency-first All Sessions view, session search, and checkpoints with fork (manaflow-ai#10215) 94f51fb Fix aggregate child memory pressure before compressor exhaustion (manaflow-ai#10773) 13006ef cloud: surface whether after() has waitUntil for deferred create work (manaflow-ai#11782) 75eee0e Cloud VMs: bake the TigerVNC desktop (dock, wallpaper, cua-driver, noVNC) into the devbox recipe and open it at the machine's private address (manaflow-ai#11776) 36b5536 Fix terminal text bleed during live window resize (manaflow-ai#11530) 44b42c1 Cloud VMs: machines usage decoder and refresh fixes, edge smoke diagnostics (manaflow-ai#11759) f11be3a ci(tui): scope Valgrind test compilation (manaflow-ai#11750) 9184f4c coderouter: many Claude upstream accounts per team, routed with affinity and cooldown failover (manaflow-ai#11775) d3b9cdd cloud: attach waits for the baked supervisor; edge probe span joins the create trace (manaflow-ai#11777) c69e317 test: make the cmuxTests target compile again (main-actor call, CLI-only type) (manaflow-ai#11770) 8185825 fix(history): stop idle History menu graph rebuild loop (manaflow-ai#10661) 723958e Test bounded stale-port retirement after listener exit (manaflow-ai#11356) 367682e Fix native terminal Copy honoring Ghostty clipboard flavor (manaflow-ai#11515) d59055d docs(tui): refresh SDK inventory counts (manaflow-ai#11766) 89e4701 cmux-tui: use JoinSet shutdown for simple drains (manaflow-ai#11745) # Conflicts: # .github/workflows/cmux-tui.yml
This branch was successfully deployed
1 participant
Summary
CmuxCorecoverage for a listener publishing port 48123, exiting, and retiring from both panel and workspace snapshots within the existing six-scan burstPortScanPublicationBufferContext
The reported stable build,
0.64.22 (102) [ddd4a01bc], predates the runtime fix in #10633 and the scanner follow-up in #11109. Current main already has the fix; this PR locks that behavior in without changing runtime code.Closes #11294
Verification
/usr/bin/arch -arm64 /usr/bin/xcrun swift test --filter PortScan— 11 tests passed./scripts/lint-pbxproj-test-wiring.sh— checked 732 test files./scripts/check-pbxproj.shpython3 scripts/check-package-resolved-policy.pypython3 scripts/check-workspace-package-groups.py --checkNo local
xcodebuildor XCUITest invocation was used.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds regression coverage for bounded stale-port retirement after a listener exits, locking in current-main behavior without changing runtime code. Closes #11294.
CmuxCoreretains a stopped port for two complete misses and removes it from panel and workspace snapshots on the third.Written for commit a6196cd. Summary will update on new commits.
Summary by CodeRabbit
Verification notes
origin/mainat2422b69c7cafter hosted run Control socket: never present resume approval UI from surface.resume.set (#13369) #13704 (33565876916) exposed the branch’s pre-fix(tests): cmuxTests compiles on main again (two missing lines from #11059 and #11345) #11346 test-target compatibility gap. The PR diff remains test-only; the rebase carries current-main compatibility commits.33567613600) reached the runner but stopped inoven-sh/setup-bunon a GitHub App installation API rate limit before any test ran; it is an infrastructure limitation, not a test result.CmuxCorepackage run passed all 11 focused port-scan tests.PONG; the headless AWS session exposed no usableTabManagerwindow surface, so a full CLI listener lifecycle could not be completed there. This is an explicit verification limitation, not being treated as end-to-end proof.The intentional trade-off is to add authoritative behavior coverage without re-implementing retirement: #10633/#11109 already fix runtime behavior on main, while stable 0.64.x predates those fixes. The PR therefore needs a release containing current main; it does not add a second retirement owner or timing workaround.
Hosted scanner verification
warp-macos-15-arm64-6xsucceeded at the exact HEADdcfda79bfab. Its app-hostxcodebuildcompiled the test bundle and ran all 3PortScannerPortRetirementTests:publishedPortIsRetiredAfterProcessStopsListening,lateBurstKickRetiresStoppedListener, andliveFullPathTTYAttributesListener.PortScannerregistration → kick → coalesce/burst → scan → reconcile → publication path with real PID identity checks and a deterministic listener lifecycle fixture; the fixture intentionally stubs command output, so it is not a claim of an OS-level TCP socket/lsof test. The AWS direct-app attempt reached socket health but could not expose aTabManagerwindow in its headless session.scripts/swift_warning_budget.pyagainst the full hosted log reports existing main/Xcode warning debt (385 warnings across 241 buckets versus the checked budget); neither changed regression file emitted a warning, and no warning-budget TSV was modified.