Repository navigation
Conversation
|
@EtanHey is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughRefactors TerminalController socket command handling to resolve tab/panel targets off-main and schedule UI state mutations asynchronously on the main thread, preserving immediate responses and returning errors early when target resolution fails. Changes
Sequence Diagram(s)sequenceDiagram
participant Socket as Socket Thread
participant Resolver as Off-Main Resolver
participant Main as Main Thread (UI)
participant Store as Panel/Tab Store
Socket->>Resolver: request resolve target (explicit --tab or selected)
Resolver-->>Socket: return tabId or error
alt resolved
Socket->>Socket: send immediate OK/response to sender
Socket->>Main: schedule mutation block (DispatchQueue.main.async)
Main->>Store: apply mutation (update metadata/status)
Main-->>Socket: mutation complete (async)
else error
Resolver-->>Socket: return error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/TerminalController.swift (1)
13614-13665: Extract shared async sidebar-mutation flow to one helper.
reportPullRequest,clearPullRequest, andreportPortsnow duplicate the same parse/resolve/surface-select/prune/async-mutate pattern. A shared helper would reduce drift and make future behavior fixes (like fully async tab resolution) safer.Also applies to: 13670-13703, 13719-13757
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 13614 - 13665, Multiple commands (reportPullRequest, clearPullRequest, reportPorts) duplicate the same parse/resolve/select/prune/async-mutate flow; extract that flow into a single helper to remove duplication and centralize logic. Create a helper (e.g., performSidebarMutation or withSidebarSurface) that accepts parsed options, reportArgs, and a closure to run inside DispatchQueue.main.async receiving (self, tab, surfaceId) after calling resolveTabIdForSidebarMutation, validating parsed panel id via UUID(uuidString:), pruning via tab.pruneSurfaceMetadata(validSurfaceIds:), and selecting the surfaceId (parsedPanelId or tab.focusedPanelId); then move the mutation bodies (calls to shouldReplacePullRequest, updatePanelPullRequest, or clear/update port state) into the closure and call DispatchQueue.main.async from the helper so reportPullRequest, clearPullRequest, and reportPorts become thin wrappers that invoke the helper with their specific mutation closure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TerminalController.swift`:
- Around line 13120-13123: The calls to resolveTabIdForSidebarMutation(...) are
synchronously dispatching to the main queue (via DispatchQueue.main.sync), which
blocks the socket thread for high-frequency telemetry paths; change the flow so
argument parsing and validation for report_*/clear_*/ports_kick/status updates
happens off-main, eliminate use of DispatchQueue.main.sync inside
resolveTabIdForSidebarMutation and any callers, and instead have
resolveTabIdForSidebarMutation return a synchronous, off-main-resolved result
(or an indication that a main-queue action is needed) and schedule only the
minimal UI/model mutation with DispatchQueue.main.async; update the usages at
the shown sites (the call in TerminalController.swift and the other occurrences
you noted) to perform dedupe/coalesce off-main and call main.async only for the
final apply-to-model step.
---
Nitpick comments:
In `@Sources/TerminalController.swift`:
- Around line 13614-13665: Multiple commands (reportPullRequest,
clearPullRequest, reportPorts) duplicate the same
parse/resolve/select/prune/async-mutate flow; extract that flow into a single
helper to remove duplication and centralize logic. Create a helper (e.g.,
performSidebarMutation or withSidebarSurface) that accepts parsed options,
reportArgs, and a closure to run inside DispatchQueue.main.async receiving
(self, tab, surfaceId) after calling resolveTabIdForSidebarMutation, validating
parsed panel id via UUID(uuidString:), pruning via
tab.pruneSurfaceMetadata(validSurfaceIds:), and selecting the surfaceId
(parsedPanelId or tab.focusedPanelId); then move the mutation bodies (calls to
shouldReplacePullRequest, updatePanelPullRequest, or clear/update port state)
into the closure and call DispatchQueue.main.async from the helper so
reportPullRequest, clearPullRequest, and reportPorts become thin wrappers that
invoke the helper with their specific mutation closure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9a94c95-c9b5-40e0-9149-462004a909d9
📒 Files selected for processing (1)
Sources/TerminalController.swift
Comment on lines
+13120
to
+13123
| let tabResolution = resolveTabIdForSidebarMutation(reportArgs: args, options: parsed.options) | ||
| guard let targetTabId = tabResolution.tabId else { | ||
| return tabResolution.error ?? "ERROR: No tab selected" | ||
| } |
There was a problem hiding this comment.
Telemetry handlers still synchronously hop to the main queue.
These high-frequency clear_*/report_* paths still call resolveTabIdForSidebarMutation(...), which uses DispatchQueue.main.sync. That keeps socket-thread blocking risk under render pressure and prevents true fire-and-forget behavior.
As per coding guidelines "Do not use DispatchQueue.main.sync for high-frequency socket telemetry commands (report_*, ports_kick, status/progress/log metadata updates). Parse and validate arguments off-main, dedupe/coalesce off-main first, then schedule minimal UI/model mutation with DispatchQueue.main.async only when needed."
Also applies to: 13624-13631, 13680-13684, 13730-13733
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 13120 - 13123, The calls to
resolveTabIdForSidebarMutation(...) are synchronously dispatching to the main
queue (via DispatchQueue.main.sync), which blocks the socket thread for
high-frequency telemetry paths; change the flow so argument parsing and
validation for report_*/clear_*/ports_kick/status updates happens off-main,
eliminate use of DispatchQueue.main.sync inside resolveTabIdForSidebarMutation
and any callers, and instead have resolveTabIdForSidebarMutation return a
synchronous, off-main-resolved result (or an indication that a main-queue action
is needed) and schedule only the minimal UI/model mutation with
DispatchQueue.main.async; update the usages at the shown sites (the call in
TerminalController.swift and the other occurrences you noted) to perform
dedupe/coalesce off-main and call main.async only for the final apply-to-model
step.
Author
There was a problem hiding this comment.
Acknowledged — this is a real concern. resolveTabIdForSidebarMutation still uses DispatchQueue.main.sync internally for the tab ID lookup, so socket threads briefly block there.
However, making tab lookup fully async requires thread-safe TabManager reads, which is a larger refactor. The current fix matches the shipped reportMeta pattern (which also uses this same helper) and eliminates the long-running mutation from the sync path.
Filed #1567 to track both the shared helper extraction and the fully async tab resolution as follow-up work.
There was a problem hiding this comment.
@EtanHey Thanks for the context and for filing #1567. While the full async tab lookup is indeed a broader refactor, there’s a small, low‑risk improvement we can land in this PR to cut most main.sync hops without touching call sites:
- Add a fast-path in resolveTabIdForSidebarMutation: if --tab is present and a valid UUID, return it immediately off‑main. Only fall back to main.sync when --tab is absent. Also surface an immediate “invalid tab id” error if the flag is present but malformed. This preserves all current behaviors while eliminating main.sync on the common explicit-ID paths.
Proposed patch:
diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift
@@
private func resolveTabIdForSidebarMutation(
reportArgs: String,
options: [String: String]
) -> (tabId: UUID?, error: String?) {
- var tabId: UUID?
- DispatchQueue.main.sync {
- if let tab = resolveTabForReport(reportArgs) {
- tabId = tab.id
- }
- }
- if let tabId {
- return (tabId, nil)
- }
- let error = options["tab"] != nil ? "ERROR: Tab not found" : "ERROR: No tab selected"
- return (nil, error)
+ // Off-main fast-path: honor explicit --tab without touching the main thread.
+ if let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines),
+ !rawTab.isEmpty {
+ guard let explicitId = UUID(uuidString: rawTab) else {
+ return (nil, "ERROR: Invalid tab id '\(rawTab)'")
+ }
+ return (explicitId, nil)
+ }
+
+ // Fallback: resolve the currently-selected tab on main as a short, bounded hop.
+ var selectedId: UUID?
+ DispatchQueue.main.sync {
+ if let tab = resolveTabForReport(reportArgs) {
+ selectedId = tab.id
+ }
+ }
+ if let selectedId {
+ return (selectedId, nil)
+ }
+ return (nil, "ERROR: No tab selected")
}Why this helps:
- High-frequency agent/shell flows typically provide explicit IDs; those no longer incur any main.sync.
- Callers and user-visible behavior are unchanged unless --tab is malformed (now reported precisely).
- The actual mutations are already scheduled via DispatchQueue.main.async, so this keeps us aligned with the “parse/validate off‑main; apply on main” guideline from CLAUDE.md (I’m referencing an existing learning).
If you’re okay with this micro-optimization, I can prepare a follow-up to land it here, and keep #1567 focused on the larger “async lookup + optimistic OK everywhere” refactor.
🧠 Learnings used
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-12T21:54:38.291Z
Learning: Applies to Sources/**/*.swift : Do not use `DispatchQueue.main.sync` for high-frequency socket telemetry commands (`report_*`, `ports_kick`, status/progress/log metadata updates). Parse and validate arguments off-main, dedupe/coalesce off-main first, then schedule minimal UI/model mutation with `DispatchQueue.main.async` only when needed.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-12T21:54:38.291Z
Learning: Applies to Sources/**/*TerminalWindowPortal.swift : In `WindowTerminalHostView.hitTest()` in `TerminalWindowPortal.swift`: all divider/sidebar/drag routing must be gated to pointer events only. Do not add work outside the `isPointerEvent` guard to avoid typing latency.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-05T03:20:48.079Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus; only explicit focus-intent commands may mutate in-app focus/selection
Learnt from: debgotwired
Repo: manaflow-ai/cmux PR: 1149
File: Sources/ContentView.swift:3977-3978
Timestamp: 2026-03-10T09:33:37.952Z
Learning: In manaflow-ai/cmux (Sources/ContentView.swift), scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh:) is shared command‑palette infrastructure across all submenus. Do not change its sync‑seeding behavior within feature‑scoped PRs; treat brief initial flashes as consistent with existing submenus. Any UX improvement (e.g., synchronous seeding on forced corpus refresh) should be implemented and tested globally in a dedicated follow‑up PR.
Learnt from: 0xble
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-09T21:53:05.451Z
Learning: Repo: manaflow-ai/cmux — For browser portals, visibility resync is centralized in BrowserWindowPortalRegistry.updateEntryVisibleInUI(for: WKWebView, visibleInUI: Bool), which immediately synchronizes the WKWebView on visibility changes. Call sites (e.g., Workspace.reconcilePanelPortalVisibilityForCurrentLayout()) should use this helper instead of ad‑hoc resync logic.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-12T21:54:38.291Z
Learning: Socket/CLI commands must not steal macOS app focus. Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents). All non-focus commands should preserve current user focus context.
Learnt from: homanp
Repo: manaflow-ai/cmux PR: 883
File: CLI/cmux.swift:1569-1586
Timestamp: 2026-03-04T22:05:07.913Z
Learning: In manaflow-ai/cmux CLI (CLI/cmux.swift), for parity with existing browser commands, markdown.open intentionally normalizes --surface before resolving --workspace/--window, relying on server-side resolution to disambiguate final routing. Avoid one-off reordering in markdown; consider any change only as a coordinated, cross-command refactor.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:4012-4023
Timestamp: 2026-03-16T08:05:16.048Z
Learning: In Sources/TerminalController.swift, implement v2SurfaceSplitSized(params:) to validate the ratio parameter as follows: if ratio is present, it must be numeric (NSNumber/Double-coercible) and strictly 0 < ratio < 1; if not, return invalid_params. If ratio is absent, use the default 0.6. This enforces the rule that a present-but-invalid parameter should yield invalid_params rather than silently falling back to defaults.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…ast-path - Convert reportPorts from DispatchQueue.main.sync to schedulePanelMetadataMutation helper, matching reportPullRequest and clearPullRequest (manaflow-ai#1550) - Wire up existing but unused shouldReplacePorts() for dedup/coalescing - Add off-main fast-path in resolveTabIdForSidebarMutation: when explicit --tab UUID is provided, skip main.sync entirely (agent/shell flows always provide IDs) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
EtanHey
force-pushed
the
fix/main-thread-starvation-logging
branch
from
March 17, 2026 10:53
25e9602 to
d9e5ac2
Compare
Author
|
Addressed both CodeRabbit findings: 1. 2. Duplicate parse/resolve/prune/async-mutate pattern (nitpick) Net: -41 lines, +26 lines. All three sidebar telemetry commands now use the shared async helper. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TerminalController.swift`:
- Around line 13692-13710: The fast-path currently rejects any non-UUID --tab
and skips index-based routing; change the logic around options["tab"] so that if
rawTab is not a UUID you attempt to parse it as an integer index and resolve the
corresponding tab (preserving the old numeric-index behavior) before falling
back to the main-thread resolveTabForReport(reportArgs). Specifically: when
options["tab"] exists and trimming yields rawTab, try UUID(uuidString:), else
try Int(rawTab) and lookup the tab by index (returning a "Tab not found" error
if the index is out of range), and only if neither UUID nor numeric index yields
a tab continue to the existing DispatchQueue.main.sync fallback that calls
resolveTabForReport(reportArgs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be95f9c0-45dd-410b-9fa5-fabb9a4f8a73
📒 Files selected for processing (1)
Sources/TerminalController.swift
Comment on lines
+13692
to
+13710
| if let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !rawTab.isEmpty { | ||
| guard let explicitId = UUID(uuidString: rawTab) else { | ||
| return (nil, "ERROR: Invalid tab id '\(rawTab)'") | ||
| } | ||
| return (explicitId, nil) | ||
| } | ||
|
|
||
| // Fallback: resolve the currently-selected tab on main as a short, bounded hop. | ||
| var selectedId: UUID? | ||
| DispatchQueue.main.sync { | ||
| if let tab = resolveTabForReport(reportArgs) { | ||
| tabId = tab.id | ||
| selectedId = tab.id | ||
| } | ||
| } | ||
| if let tabId { | ||
| return (tabId, nil) | ||
| if let selectedId { | ||
| return (selectedId, nil) | ||
| } | ||
| let error = options["tab"] != nil ? "ERROR: Tab not found" : "ERROR: No tab selected" | ||
| return (nil, error) | ||
| return (nil, "ERROR: No tab selected") |
There was a problem hiding this comment.
Preserve --tab index routing here.
This fast-path now rejects every non-UUID --tab, but the existing resolver still accepts numeric tab indices. Calls like report_meta ... --tab=0 / set_status ... --tab=1 used to work and will now fail here, and index misses now fall through to "ERROR: No tab selected" instead of "ERROR: Tab not found".
💡 Suggested fix
private func resolveTabIdForSidebarMutation(
reportArgs: String,
options: [String: String]
) -> (tabId: UUID?, error: String?) {
- // Off-main fast-path: honor explicit --tab UUID without touching the main thread.
- // High-frequency agent/shell flows typically provide explicit IDs.
- if let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines),
- !rawTab.isEmpty {
- guard let explicitId = UUID(uuidString: rawTab) else {
- return (nil, "ERROR: Invalid tab id '\(rawTab)'")
- }
- return (explicitId, nil)
+ let rawTab = options["tab"]?.trimmingCharacters(in: .whitespacesAndNewlines)
+ let hasExplicitTab = rawTab?.isEmpty == false
+
+ // Off-main fast-path: honor explicit UUIDs without touching the main thread.
+ if let rawTab, !rawTab.isEmpty {
+ if let explicitId = UUID(uuidString: rawTab) {
+ return (explicitId, nil)
+ }
+ // Keep numeric-index routing on the existing resolveTabForReport path.
+ if Int(rawTab) == nil {
+ return (nil, "ERROR: Invalid tab id '\(rawTab)'")
+ }
}
// Fallback: resolve the currently-selected tab on main as a short, bounded hop.
var selectedId: UUID?
DispatchQueue.main.sync {
if let tab = resolveTabForReport(reportArgs) {
selectedId = tab.id
}
}
if let selectedId {
return (selectedId, nil)
}
- return (nil, "ERROR: No tab selected")
+ return (nil, hasExplicitTab ? "ERROR: Tab not found" : "ERROR: No tab selected")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 13692 - 13710, The fast-path
currently rejects any non-UUID --tab and skips index-based routing; change the
logic around options["tab"] so that if rawTab is not a UUID you attempt to parse
it as an integer index and resolve the corresponding tab (preserving the old
numeric-index behavior) before falling back to the main-thread
resolveTabForReport(reportArgs). Specifically: when options["tab"] exists and
trimming yields rawTab, try UUID(uuidString:), else try Int(rawTab) and lookup
the tab by index (returning a "Tab not found" error if the index is out of
range), and only if neither UUID nor numeric index yields a tab continue to the
existing DispatchQueue.main.sync fallback that calls
resolveTabForReport(reportArgs).
Author
|
Closing — issue #1550 was resolved by PR #1564 (merged Mar 17 by @austinywang). My fix paralleled the same approach (schedulePanelMetadataMutation / async dispatch) but #1564 landed first. Thanks for the quick turnaround! 🙏 |
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reportPullRequest,clearPullRequest,reportPorts, andclearSidebarMetadatafromDispatchQueue.main.synctomain.async, matching the pattern already used byupsertSidebarMetadata(reportMeta)resolveTabIdForSidebarMutation(short sync), then schedule model mutation withmain.asyncand return"OK"immediatelymain.syncblock indefinitely while the main thread is stuck in a SwiftUI render pass (especiallyBrowserPanelView.body)Root Cause
reportPullRequestandclearPullRequest(andreportPorts,clearSidebarMetadata) usedDispatchQueue.main.syncfor their entire body — tab resolution + validation + mutation — all in one synchronous block. When external tools like Claude Code fire these commands rapidly on multiple socket threads, and the main thread is occupied by a SwiftUI render, all socket threads convoy-block, starving the event loop and causing "Not Responding" hangs.reportMeta(viaupsertSidebarMetadata) was already fixed to usemain.async. This PR applies the same pattern to the remaining telemetry commands.Behavioral Change
These commands now return
"OK"optimistically instead of reporting tab/panel resolution errors synchronously. This is acceptable because they are fire-and-forget telemetry — callers do not retry on error, and the panel/tab validation still happens in the async block (silently dropping invalid mutations).Fixes #1550
Test plan
report_prcommandsclear_prstill clears badgesreport_portsstill updates port indicatorsclear_status/clear_metastill work🤖 Generated with Claude Code
Summary by cubic
Make socket telemetry updates async and add a fast-path for tab resolution to prevent main-thread starvation and “Not Responding” hangs during heavy SwiftUI renders. Fixes #1550.
reportPullRequest,clearPullRequest,reportPorts, andclearSidebarMetadatatomain.asyncviaschedulePanelMetadataMutation; commands return "OK" immediately instead of blocking.resolveTabIdForSidebarMutationfor explicit--tabUUIDs; fallback is a short main-thread hop when no tab is provided.reportPortsnow dedupes and sorts port lists and usesshouldReplacePortsto coalesce updates and reduce redundant model work.Written for commit d9e5ac2. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Refactor