Repository navigation
fix: preserve PortScanner burst lifecycle - #11180
lawrencecchen wants to merge 4 commits into
Conversation
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
Changeslsof PID batching
Burst lifecycle scheduling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to The PR preserves active bursts when panels unregister, but its batched process scan treats any failed batch as making the entire scan incomplete and can lengthen scans as the process list grows, potentially leaving valid port results missing across panels or workspaces. Merge should wait for this behavior to be fixed or explicitly accepted. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Panel
participant PortScanner
participant DispatchWorkItem
participant CommandRunner
Panel->>PortScanner: register and kick scan
PortScanner->>DispatchWorkItem: schedule coalesced burst
Panel->>PortScanner: unregister panel
PortScanner->>DispatchWorkItem: cancel pending work
DispatchWorkItem->>PortScanner: attempt deferred callback
PortScanner->>PortScanner: reject stale generation
PortScanner->>CommandRunner: continue commands only for remaining panels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
Full details: Cmux Swift Actor IsolationExplanation PASS: The production diff adds no implicit MainActor model or service protocol, and it does not access a UI-bound store from a background context. Full details: Cmux Swift Blocking RuntimeExplanation No prohibited blocking primitive is introduced in production Swift. The review-base Full details: Cmux Browser Automation Off-MainExplanation PASS: The complete pull-request range changes only Full details: Cmux Expensive Synchronous LoadExplanation The PR does not add or move an agent-history load. The production diff only adds PID chunking for asynchronous Full details: Cmux Cache Substitution CorrectnessExplanation PASS: The production diff does not replace a fresh authoritative read with a cache. Full details: Cmux No Hacky SleepsExplanation PASS: The pull request changes only Swift files ( Full details: Cmux Algorithmic ComplexityExplanation No algorithmic-complexity failure is introduced. The new Full details: Cmux Swift ConcurrencyExplanation PASS. The PR does not introduce a custom background queue, Combine state, or a new completion-handler API. Full details: Cmux Swift `@Concurrent`Explanation The diff introduces Resolution Add the repository's compiler-conditional Full details: Cmux Swift Package BoundariesExplanation The PR adds independent Resolution Create a small macOS SwiftPM target named Full details: Cmux Swiftpm LockfilesExplanation PASS. The complete PR range from 699f1a7 to 89ce4a4 changes only Sources/PortScanner+Process.swift, Sources/PortScanner.swift, and cmuxTests/PortScannerTests.swift. No Package.swift, Package.resolved, .gitignore, workflow, or Xcode project path changed. Therefore the PR introduces no SwiftPM dependency or Xcode package-reference change that requires a lockfile diff, and it does not introduce a cmux-owned .gitignore rule for Package.resolved. Full details: Cmux Swift LoggingExplanation PASS. The changed production Swift code adds lsof chunking and burst lifecycle scheduling only. The added lines contain no Full details: Cmux User-Facing Error PrivacyExplanation PASS. The production diff only changes internal Full details: Cmux Full InternationalizationExplanation PASS: The complete PR diff changes only Full details: Cmux Swiftui State LayoutExplanation PASS: The PR changes only PortScanner process/scheduling code and XCTest-style lifecycle support. The changed files contain no SwiftUI views, ObservableObject or property-wrapper state, GeometryReader, lazy/list row subtree, or render-time state mutation. The check is therefore not applicable. Full details: Cmux Architecture RethinkExplanation PASS. The production diff is a local scheduler correctness fix in Full details: Cmux Swift Auxiliary Window Close ShortcutsExplanation PASS: The pull request changes only PortScanner process/scheduling code and PortScanner test infrastructure. The aggregate diff introduces no NSWindow, NSPanel, NSWindowController, SwiftUI Window/WindowGroup, close-shortcut routing, or auxiliary-window identifier code. The added test actor is a test-only fixture, which the rule allows. The auxiliary-window rule is therefore not applicable. Full details: Cmux Source ArtifactsExplanation PASS: The aggregate pull-request diff changes only Full details: Cmux No Test Or Debug Seam In Production SourceExplanation PASS. The PR changes only Full details: Cmux No Ambient Global StateExplanation PASS. The production diff adds only
✨ Finishing Touches 💡 1📝 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 |
89ce4a4 to
cd8293d
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. |
|
The scheduler and bounded lsof handling from this follow-up are already covered on main by #11109 and subsequent PortScanner updates, so this PR is superseded. Closing it. |
Follow-up to #11109.
This branch first adds a regression test proving that unregistering one panel does not cancel another panel's active burst, then fixes the scheduler. It keeps one future burst callback instead of retaining completed work items, cancels the global scheduler only after the final panel is removed, and avoids restarting a scan after the final panel is gone. The lifecycle test uses queue and invocation signals instead of a fixed synchronization sleep.
Commits:
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes the PortScanner burst lifecycle so unregistering one panel no longer cancels another panel's active burst scan.
lsofPID scans into batches of 256.Written for commit cd8293d. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests