Fix missing sidebar ports for agent-run dev servers#2562
Merged
austinywang merged 3 commits intomainfrom Apr 4, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughCapture and propagate Codex agent PIDs across hook lifecycle, add workspace-scoped agent port scanning and tracking, surface agent-owned listening ports into workspace state, and add end-to-end tests exercising PID registration and sidebar port lifecycle. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent Process
participant CLI as CLI/cmux
participant TC as TerminalController
participant PS as PortScanner
participant WS as Workspace
Agent->>CLI: Codex hook session-start / prompt-submit / stop
CLI->>CLI: inferCodexAgentPID() / write PID to session store
CLI->>TC: send "set_agent_pid <key> <pid> --tab=<workspaceId>"
TC->>WS: update workspace.agentPIDs
TC->>PS: refreshAgentPorts(workspaceId, agentPIDs)
PS->>PS: schedule/reschedule agent rescan timer
PS->>PS: scan agent process trees (ps -ax) and run lsof
PS->>TC: onAgentPortsUpdated(workspaceId, ports)
TC->>WS: set workspace.agentListeningPorts and recompute listeningPorts
sequenceDiagram
participant TM as TabManager
participant PS as PortScanner
participant TC as TerminalController
participant WS as Workspace
TM->>TM: sweepStaleAgentPIDs()
TM->>TM: remove dead PID entries, compute remaining valid PIDs
TM->>PS: refreshAgentPorts(workspaceId, remainingPIDs)
PS->>PS: update tracking state & revision, immediate scan
PS->>TC: onAgentPortsUpdated (if changed)
TC->>WS: update agentListeningPorts and recompute listeningPorts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:12925">
P2: Sanitize `sessionId` before building `agentPIDKey`; raw IDs can break `set_agent_pid` command tokenization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| !sessionId.isEmpty else { | ||
| return "codex" | ||
| } | ||
| return "codex.\(sessionId)" |
There was a problem hiding this comment.
P2: Sanitize sessionId before building agentPIDKey; raw IDs can break set_agent_pid command tokenization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLI/cmux.swift, line 12925:
<comment>Sanitize `sessionId` before building `agentPIDKey`; raw IDs can break `set_agent_pid` command tokenization.</comment>
<file context>
@@ -12875,6 +12917,67 @@ struct CMUXCLI {
+ !sessionId.isEmpty else {
+ return "codex"
+ }
+ return "codex.\(sessionId)"
+ }
+
</file context>
Contributor
Greptile SummaryThis PR adds workspace-scoped agent port tracking so dev servers launched by Codex/agent process trees (outside the shell TTY path) appear in the sidebar. It does this by registering root agent PIDs via The implementation is well-structured and the two new E2E tests (direct Confidence Score: 5/5Safe to merge — all findings are P2 style suggestions with no correctness impact. The revision-gating logic is correct and prevents stale scan results from being applied. Dead process trees are handled properly by the BFS expansion returning no live children, which causes empty port delivery. The No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent as Agent CLI
participant TC as TerminalController
participant WS as Workspace
participant PS as PortScanner
Agent->>TC: set_agent_pid key pid --tab=id
TC-->>Agent: OK
TC->>WS: agentPIDs[key] = pid
WS->>PS: refreshAgentPorts(workspaceId, pids)
PS->>PS: increment revision, start 2s timer
PS->>WS: agentPIDsProvider callback
WS-->>PS: active PIDs
PS->>PS: expandProcessTree then lsof
PS-->>WS: onAgentPortsUpdated(ports)
WS->>WS: agentListeningPorts = ports, recomputeListeningPorts
loop every 2s periodic rescan
PS->>WS: agentPIDsProvider callback
WS-->>PS: active PIDs
PS->>PS: expandProcessTree then lsof
PS-->>WS: onAgentPortsUpdated(ports)
end
Agent->>TC: clear_agent_pid key --tab=id
TC-->>Agent: OK
TC->>WS: agentPIDs.removeValue(key)
WS->>PS: refreshAgentPorts(workspaceId, empty)
PS->>PS: remove workspace, cancel timer
PS-->>WS: onAgentPortsUpdated(empty)
WS->>WS: agentListeningPorts = empty, recomputeListeningPorts
Reviews (1): Last reviewed commit: "fix: track agent dev-server ports in sid..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if not line or line.startswith(" ") or "=" not in line: | ||
| continue | ||
| key, value = line.split("=", 1) | ||
| data[key.strip()] = v.strip() if (v := value.strip()) else "" |
Contributor
There was a problem hiding this comment.
value.strip()
v.strip() is the same as value.strip() (it was already stripped to produce v), so the truthy branch and the falsy "" branch both produce value.strip(). The expression reduces to a plain assignment.
Suggested change
| data[key.strip()] = v.strip() if (v := value.strip()) else "" | |
| data[key.strip()] = value.strip() |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
Sources/TerminalController.swift (1)
14495-14512: Only refresh tracked agent ports when the PID map actually changes.Lines 14532-14534 already gate the refresh on real removal, but the other new paths still call
refreshTrackedAgentPortswhen the stored PID is unchanged orclear_agent_pidremoves nothing. Repeated status/agent heartbeats will keep rescanning the same process set.♻️ Suggested change
@@ if let pidValue { - tab.agentPIDs[key] = pidValue - self.refreshTrackedAgentPorts(for: tab) + if tab.agentPIDs[key] != pidValue { + tab.agentPIDs[key] = pidValue + self.refreshTrackedAgentPorts(for: tab) + } } @@ - tab.agentPIDs[key] = pid - self.refreshTrackedAgentPorts(for: tab) + if tab.agentPIDs[key] != pid { + tab.agentPIDs[key] = pid + self.refreshTrackedAgentPorts(for: tab) + } @@ - tab.agentPIDs.removeValue(forKey: key) - self.refreshTrackedAgentPorts(for: tab) + if tab.agentPIDs.removeValue(forKey: key) != nil { + self.refreshTrackedAgentPorts(for: tab) + }As per coding guidelines: "Do not use
DispatchQueue.main.syncfor 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 withDispatchQueue.main.async."Also applies to: 14532-14534, 14555-14583
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 14495 - 14512, Only call refreshTrackedAgentPorts(for:) when tab.agentPIDs actually changes: before assigning tab.agentPIDs[key] = pidValue, compare pidValue to the existing tab.agentPIDs[key] and only set + call refreshTrackedAgentPorts(for: tab) when they differ; likewise for the clear/remove path (only remove and call refresh if tab.agentPIDs contained the key beforehand). Also ensure these telemetry/status updates are coalesced off-main and that any current DispatchQueue.main.sync usage in the related status/agent handling is replaced with DispatchQueue.main.async for the minimal UI/model mutation.tests/test_codex_hook_agent_ports.py (1)
34-170: Extract the shared port/process helpers into a test utility.Most of this helper block is now duplicated with
tests/test_sidebar_ports.py. Pulling_parse_sidebar_state,_wait_for*, and_terminate_process_groupinto a shared module will keep the two regressions from drifting the next time timing or cleanup behavior changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_codex_hook_agent_ports.py` around lines 34 - 170, Extract the duplicated helpers (_parse_sidebar_state, _wait_for, _wait_for_lsof_listen_pid, _wait_for_lsof_listen_gone, _wait_for_port, _wait_for_port_absent, _terminate_process_group, _find_free_port) into a new shared test utility module (e.g., test_port_utils), replace the duplicated definitions in both test files with imports from that module, update any references to those functions to use the imported names, remove the original duplicated blocks, and run the test suite to ensure imports and behavior remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 12802-12809: The current prompt-submit path only calls
sessionStore.upsert when mappedSession exists so recovered PID/state isn't
persisted; modify the logic in the prompt-submit handler to always call
sessionStore.upsert (use sessionId from parsedInput and codexPid) even when
mappedSession is nil: supply surfaceId as mappedSession?.surfaceId or a sensible
default, cwd as parsedInput.cwd ?? mappedSession?.cwd, and workspaceId as
available; ensure the upsert invocation (sessionStore.upsert) runs for both
branches so recovered sessions are saved for later stop handling.
- Around line 12932-12939: The parent-chain traversal in the loop using
codexHookWrapperProcessNames is too narrow and breaks when encountering shells
not listed; update detection in the loop that uses processName(for:) and
parentPID(of:) to broaden or normalize matches: either expand
codexHookWrapperProcessNames to include additional common shells (ksh, tcsh,
fish, etc.) and normalize processName(for:) results by stripping
version/suffixes and resolving symlinks, or change the logic to prefer an
explicit PID from the hook payload/environment when present (add a check before
the while using the hook-provided PID), and allow a configurable wrapper
whitelist rather than hard-coded names so remainingWrapperSkips and candidate
traversal correctly climb past transient wrappers.
In `@Sources/PortScanner.swift`:
- Around line 291-299: When clearing trackedAgentWorkspaces (the guard-let
branch where agentPIDsProvider is nil and the similar block later that calls
trackedAgentWorkspaces.removeAll(), updateAgentScanTimerLocked(), and
deliverAgentResults), also advance/bump the stored workspace revision(s) in
agentRevisions for each workspaceId so any in-flight validatedAgentResults with
older revisions will be ignored; modify the code that prepares agentRevisions in
these paths to increment or set a new revision token for each workspaceId before
calling deliverAgentResults, and ensure this revision scheme matches how
validatedAgentResults compares revisions.
In `@Sources/TerminalController.swift`:
- Around line 1030-1049: The closures assigned to PortScanner.shared
(onAgentPortsUpdated and agentPIDsProvider) currently locate workspaces via
self.tabManager.tabs (active-window only) causing cross-window workspaces to be
ignored; change both lookups to resolve workspaces app-wide using
AppDelegate.shared?.workspaceFor(tabId:) (same resolver used by
sendPickedElementToTerminal(workspaceId:summary:)), i.e. replace uses of
tabManager.tabs.first(where: { $0.id == workspaceId }) with an
AppDelegate.shared? workspaceFor(tabId:) call and keep the rest of the logic
(updating workspace.agentListeningPorts, recomputeListeningPorts, building
pidsByWorkspace) intact while preserving the weak self capture.
In `@tests/cmux.py`:
- Around line 755-771: The new methods set_agent_pid and clear_agent_pid accept
tab=None but annotate it as str, causing RUF013; update their signatures to use
Optional[str] for the tab parameter and add an import for Optional from typing
(or use typing.Optional) so the type matches the runtime default None and
resolves the lint error; keep the existing default None and the rest of the
method logic unchanged.
---
Nitpick comments:
In `@Sources/TerminalController.swift`:
- Around line 14495-14512: Only call refreshTrackedAgentPorts(for:) when
tab.agentPIDs actually changes: before assigning tab.agentPIDs[key] = pidValue,
compare pidValue to the existing tab.agentPIDs[key] and only set + call
refreshTrackedAgentPorts(for: tab) when they differ; likewise for the
clear/remove path (only remove and call refresh if tab.agentPIDs contained the
key beforehand). Also ensure these telemetry/status updates are coalesced
off-main and that any current DispatchQueue.main.sync usage in the related
status/agent handling is replaced with DispatchQueue.main.async for the minimal
UI/model mutation.
In `@tests/test_codex_hook_agent_ports.py`:
- Around line 34-170: Extract the duplicated helpers (_parse_sidebar_state,
_wait_for, _wait_for_lsof_listen_pid, _wait_for_lsof_listen_gone,
_wait_for_port, _wait_for_port_absent, _terminate_process_group,
_find_free_port) into a new shared test utility module (e.g., test_port_utils),
replace the duplicated definitions in both test files with imports from that
module, update any references to those functions to use the imported names,
remove the original duplicated blocks, and run the test suite to ensure imports
and behavior remain correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aaefcf2f-41df-4d39-8734-75b2322d3d17
📒 Files selected for processing (8)
CLI/cmux.swiftSources/PortScanner.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/Workspace.swifttests/cmux.pytests/test_codex_hook_agent_ports.pytests/test_sidebar_ports.py
Comment on lines
+12802
to
+12809
| if let sessionId = parsedInput.sessionId, let mappedSession { | ||
| try? sessionStore.upsert( | ||
| sessionId: sessionId, | ||
| workspaceId: workspaceId, | ||
| surfaceId: mappedSession.surfaceId, | ||
| cwd: parsedInput.cwd ?? mappedSession.cwd, | ||
| pid: codexPid | ||
| ) |
There was a problem hiding this comment.
Persist the recovered session on prompt-submit.
Line 12802 only upserts when a prior session record already exists. If SessionStart was skipped or the store entry was lost, this path still recovers/registers a PID, but none of that recovered state is saved, so stop has to rediscover everything from ambient state again.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 12802 - 12809, The current prompt-submit path
only calls sessionStore.upsert when mappedSession exists so recovered PID/state
isn't persisted; modify the logic in the prompt-submit handler to always call
sessionStore.upsert (use sessionId from parsedInput and codexPid) even when
mappedSession is nil: supply surfaceId as mappedSession?.surfaceId or a sensible
default, cwd as parsedInput.cwd ?? mappedSession?.cwd, and workspaceId as
available; ensure the upsert invocation (sessionStore.upsert) runs for both
branches so recovered sessions are saved for later stop handling.
Comment on lines
+12932
to
+12939
| while candidate > 1, remainingWrapperSkips > 0 { | ||
| guard let processName = processName(for: candidate) else { break } | ||
| if !codexHookWrapperProcessNames.contains(processName) { | ||
| break | ||
| } | ||
| let next = parentPID(of: candidate) | ||
| guard next > 1, next != candidate else { break } | ||
| candidate = next |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check if the file exists and read the specific lines
wc -l CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Read lines 12932-12939 and surrounding context (12920-12950) for better understanding
sed -n '12920,12950p' CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 1214
🏁 Script executed:
#!/bin/bash
# Search for the definition of codexHookWrapperProcessNames
rg "codexHookWrapperProcessNames" CLI/cmux.swift -B 2 -A 2Repository: manaflow-ai/cmux
Length of output: 386
🏁 Script executed:
#!/bin/bash
# Search for how the wrapper detection logic is used and the rescan mechanism
rg "parentPID\|remainingWrapperSkips" CLI/cmux.swift -B 3 -A 3Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Get the full definition of codexHookWrapperProcessNames
rg "private let codexHookWrapperProcessNames" CLI/cmux.swift -A 10Repository: manaflow-ai/cmux
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Search for hook payload or environment variable usage for PID
rg "CODEX_HOOK|hook.*PID|payload.*PID" CLI/cmux.swift -iRepository: manaflow-ai/cmux
Length of output: 781
🏁 Script executed:
#!/bin/bash
# Search for how inferredCodexAgentPID is used and whether there are other PID detection mechanisms
rg "inferredCodexAgentPID\|codexAgentPID" CLI/cmux.swift -B 2 -A 2 | head -60Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check for any normalization or variant handling of process names
rg "processName|sh|bash|zsh|env" CLI/cmux.swift | grep -E "(normalize|variant|strip|replace)" | head -20Repository: manaflow-ai/cmux
Length of output: 1491
Wrapper detection is too narrow and needs broadening or alternative strategy.
The hard-coded codexHookWrapperProcessNames set contains only ["sh", "bash", "zsh", "env"]. Line 12934 exits the parent-chain traversal as soon as any process name falls outside this set. If Codex runs through wrapper shells not in this list (e.g., ksh, tcsh, fish, or non-standard login-shell variants), the inferred PID will stop prematurely at the wrapper rather than the Codex root. This causes the periodic rescan to drop tracking as soon as that short-lived wrapper exits.
Either:
- Normalize process names (strip version suffixes, resolve symlinks) and broaden the detection set, or
- Extract and prefer an explicit PID from hook payload/environment when available (no such mechanism currently exists).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 12932 - 12939, The parent-chain traversal in the
loop using codexHookWrapperProcessNames is too narrow and breaks when
encountering shells not listed; update detection in the loop that uses
processName(for:) and parentPID(of:) to broaden or normalize matches: either
expand codexHookWrapperProcessNames to include additional common shells (ksh,
tcsh, fish, etc.) and normalize processName(for:) results by stripping
version/suffixes and resolving symlinks, or change the logic to prefer an
explicit PID from the hook payload/environment when present (add a check before
the while using the hook-provided PID), and allow a configurable wrapper
whitelist rather than hard-coded names so remainingWrapperSkips and candidate
traversal correctly climb past transient wrappers.
Comment on lines
+291
to
+299
| guard let agentPIDsProvider else { | ||
| trackedAgentWorkspaces.removeAll() | ||
| updateAgentScanTimerLocked() | ||
| deliverAgentResults( | ||
| workspaceIds: workspaceIds, | ||
| agentPortsByWorkspace: [:], | ||
| agentRevisions: agentRevisions | ||
| ) | ||
| return |
There was a problem hiding this comment.
Bump the workspace revision when provider reconciliation drops tracking.
When these paths clear trackedAgentWorkspaces without advancing the revision, any older in-flight scan for the same workspace still passes validatedAgentResults(...) and can republish stale agent ports after we've already decided the workspace is inactive. That shows up exactly in the exit/crash path this timer is meant to clean up.
🛠️ Proposed fix
guard let agentPIDsProvider else {
+ var clearedRevisions = agentRevisions
+ for workspaceId in workspaceIds {
+ clearedRevisions[workspaceId] = nextAgentRevision(for: workspaceId)
+ }
trackedAgentWorkspaces.removeAll()
updateAgentScanTimerLocked()
deliverAgentResults(
workspaceIds: workspaceIds,
agentPortsByWorkspace: [:],
- agentRevisions: agentRevisions
+ agentRevisions: clearedRevisions
)
return
}
@@
- if !inactiveWorkspaceIds.isEmpty {
+ var deliveredRevisions = agentRevisions
+ if !inactiveWorkspaceIds.isEmpty {
+ for workspaceId in inactiveWorkspaceIds {
+ deliveredRevisions[workspaceId] = nextAgentRevision(for: workspaceId)
+ }
trackedAgentWorkspaces.subtract(inactiveWorkspaceIds)
updateAgentScanTimerLocked()
}
scanAgentPorts(
workspaceIds: workspaceIds,
agentPIDsByWorkspace: normalizedPIDsByWorkspace,
- agentRevisions: agentRevisions
+ agentRevisions: deliveredRevisions
)Also applies to: 327-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/PortScanner.swift` around lines 291 - 299, When clearing
trackedAgentWorkspaces (the guard-let branch where agentPIDsProvider is nil and
the similar block later that calls trackedAgentWorkspaces.removeAll(),
updateAgentScanTimerLocked(), and deliverAgentResults), also advance/bump the
stored workspace revision(s) in agentRevisions for each workspaceId so any
in-flight validatedAgentResults with older revisions will be ignored; modify the
code that prepares agentRevisions in these paths to increment or set a new
revision token for each workspaceId before calling deliverAgentResults, and
ensure this revision scheme matches how validatedAgentResults compares
revisions.
Comment on lines
+1030
to
+1049
| PortScanner.shared.onAgentPortsUpdated = { [weak self] workspaceId, ports in | ||
| guard let self, let tabManager = self.tabManager else { return } | ||
| guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return } | ||
| if workspace.agentListeningPorts != ports { | ||
| workspace.agentListeningPorts = ports | ||
| workspace.recomputeListeningPorts() | ||
| } | ||
| } | ||
| PortScanner.shared.agentPIDsProvider = { [weak self] workspaceIds in | ||
| guard let self, let tabManager = self.tabManager else { return [:] } | ||
| var pidsByWorkspace: [UUID: Set<Int>] = [:] | ||
| for workspaceId in workspaceIds { | ||
| guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { continue } | ||
| let pids = Set(workspace.agentPIDs.values.compactMap { $0 > 0 ? Int($0) : nil }) | ||
| if !pids.isEmpty { | ||
| pidsByWorkspace[workspaceId] = pids | ||
| } | ||
| } | ||
| return pidsByWorkspace | ||
| } |
There was a problem hiding this comment.
Resolve agent workspaces through the app-wide lookup.
These closures only search self.tabManager, which is the active window’s manager. Agent PID tracking already accepts --tab= targets from other windows, so the later port update/clear callback will be dropped for those workspaces until that window becomes active.
♻️ Suggested change
- PortScanner.shared.onAgentPortsUpdated = { [weak self] workspaceId, ports in
- guard let self, let tabManager = self.tabManager else { return }
- guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return }
+ PortScanner.shared.onAgentPortsUpdated = { workspaceId, ports in
+ guard let tabManager = AppDelegate.shared?.tabManagerFor(tabId: workspaceId),
+ let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return }
if workspace.agentListeningPorts != ports {
workspace.agentListeningPorts = ports
workspace.recomputeListeningPorts()
}
}
- PortScanner.shared.agentPIDsProvider = { [weak self] workspaceIds in
- guard let self, let tabManager = self.tabManager else { return [:] }
+ PortScanner.shared.agentPIDsProvider = { workspaceIds in
var pidsByWorkspace: [UUID: Set<Int>] = [:]
for workspaceId in workspaceIds {
- guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { continue }
+ guard let tabManager = AppDelegate.shared?.tabManagerFor(tabId: workspaceId),
+ let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { continue }
let pids = Set(workspace.agentPIDs.values.compactMap { $0 > 0 ? Int($0) : nil })
if !pids.isEmpty {
pidsByWorkspace[workspaceId] = pids
}
}
return pidsByWorkspace
}Based on learnings: sendPickedElementToTerminal(workspaceId:summary:) resolves the target workspace via AppDelegate.shared?.workspaceFor(tabId: workspaceId), which searches across all mainWindowContexts (not self.tabManager which is active-window only).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 1030 - 1049, The closures
assigned to PortScanner.shared (onAgentPortsUpdated and agentPIDsProvider)
currently locate workspaces via self.tabManager.tabs (active-window only)
causing cross-window workspaces to be ignored; change both lookups to resolve
workspaces app-wide using AppDelegate.shared?.workspaceFor(tabId:) (same
resolver used by sendPickedElementToTerminal(workspaceId:summary:)), i.e.
replace uses of tabManager.tabs.first(where: { $0.id == workspaceId }) with an
AppDelegate.shared? workspaceFor(tabId:) call and keep the rest of the logic
(updating workspace.agentListeningPorts, recomputeListeningPorts, building
pidsByWorkspace) intact while preserving the weak self capture.
Comment on lines
+755
to
+771
| def set_agent_pid(self, key: str, pid: int, tab: str = None) -> None: | ||
| """Register an agent PID for workspace-scoped ownership tracking.""" | ||
| cmd = f"set_agent_pid {key} {pid}" | ||
| if tab: | ||
| cmd += f" --tab={tab}" | ||
| response = self._send_command(cmd) | ||
| if not response.startswith("OK"): | ||
| raise cmuxError(response) | ||
|
|
||
| def clear_agent_pid(self, key: str, tab: str = None) -> None: | ||
| """Clear a previously registered agent PID.""" | ||
| cmd = f"clear_agent_pid {key}" | ||
| if tab: | ||
| cmd += f" --tab={tab}" | ||
| response = self._send_command(cmd) | ||
| if not response.startswith("OK"): | ||
| raise cmuxError(response) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd tests && head -n 50 cmux.py | grep -E "^from|^import"Repository: manaflow-ai/cmux
Length of output: 218
🏁 Script executed:
sed -n '755,771p' tests/cmux.pyRepository: manaflow-ai/cmux
Length of output: 762
🏁 Script executed:
grep -n "Optional" tests/cmux.py | head -20Repository: manaflow-ai/cmux
Length of output: 342
Annotate tab as optional in the new client methods.
These two additions default tab to None but annotate it as str, creating RUF013 violations. Use Optional[str] to match the runtime contract.
♻️ Suggested fix
- def set_agent_pid(self, key: str, pid: int, tab: str = None) -> None:
+ def set_agent_pid(self, key: str, pid: int, tab: Optional[str] = None) -> None:
"""Register an agent PID for workspace-scoped ownership tracking."""
cmd = f"set_agent_pid {key} {pid}"
if tab:
cmd += f" --tab={tab}"
response = self._send_command(cmd)
if not response.startswith("OK"):
raise cmuxError(response)
- def clear_agent_pid(self, key: str, tab: str = None) -> None:
+ def clear_agent_pid(self, key: str, tab: Optional[str] = None) -> None:
"""Clear a previously registered agent PID."""
cmd = f"clear_agent_pid {key}"
if tab:
cmd += f" --tab={tab}"
response = self._send_command(cmd)
if not response.startswith("OK"):
raise cmuxError(response)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def set_agent_pid(self, key: str, pid: int, tab: str = None) -> None: | |
| """Register an agent PID for workspace-scoped ownership tracking.""" | |
| cmd = f"set_agent_pid {key} {pid}" | |
| if tab: | |
| cmd += f" --tab={tab}" | |
| response = self._send_command(cmd) | |
| if not response.startswith("OK"): | |
| raise cmuxError(response) | |
| def clear_agent_pid(self, key: str, tab: str = None) -> None: | |
| """Clear a previously registered agent PID.""" | |
| cmd = f"clear_agent_pid {key}" | |
| if tab: | |
| cmd += f" --tab={tab}" | |
| response = self._send_command(cmd) | |
| if not response.startswith("OK"): | |
| raise cmuxError(response) | |
| def set_agent_pid(self, key: str, pid: int, tab: Optional[str] = None) -> None: | |
| """Register an agent PID for workspace-scoped ownership tracking.""" | |
| cmd = f"set_agent_pid {key} {pid}" | |
| if tab: | |
| cmd += f" --tab={tab}" | |
| response = self._send_command(cmd) | |
| if not response.startswith("OK"): | |
| raise cmuxError(response) | |
| def clear_agent_pid(self, key: str, tab: Optional[str] = None) -> None: | |
| """Clear a previously registered agent PID.""" | |
| cmd = f"clear_agent_pid {key}" | |
| if tab: | |
| cmd += f" --tab={tab}" | |
| response = self._send_command(cmd) | |
| if not response.startswith("OK"): | |
| raise cmuxError(response) |
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 755-755: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
[warning] 764-764: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cmux.py` around lines 755 - 771, The new methods set_agent_pid and
clear_agent_pid accept tab=None but annotate it as str, causing RUF013; update
their signatures to use Optional[str] for the tab parameter and add an import
for Optional from typing (or use typing.Optional) so the type matches the
runtime default None and resolves the lint error; keep the existing default None
and the rest of the method logic unchanged.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/TerminalController.swift (1)
14662-14666: Unconditional refresh inconsistent with status key clearing path.In the status key clearing handler (lines 14622-14624), refresh is guarded with
if tab.agentPIDs.removeValue(forKey: key) != nil. Here, refresh is called unconditionally afterremoveValue, triggering unnecessary port scanner work when the key doesn't exist.♻️ Align with conditional refresh pattern
DispatchQueue.main.async { [weak self] in guard let self, let tab = self.tabForSidebarMutation(id: targetTabId) else { return } - tab.agentPIDs.removeValue(forKey: key) - self.refreshTrackedAgentPorts(for: tab) + if tab.agentPIDs.removeValue(forKey: key) != nil { + self.refreshTrackedAgentPorts(for: tab) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 14662 - 14666, The DispatchQueue.main.async block unconditionally calls refreshTrackedAgentPorts(for:) after attempting to remove the key from tab.agentPIDs, causing unnecessary work; change the logic to mirror the other handler by only calling self.refreshTrackedAgentPorts(for: tab) when tab.agentPIDs.removeValue(forKey: key) returns a non-nil value—locate the async closure that uses tabForSidebarMutation(id:), the agentPIDs dictionary, and refreshTrackedAgentPorts(for:) and make the refresh conditional on the removeValue result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/TerminalController.swift`:
- Around line 14662-14666: The DispatchQueue.main.async block unconditionally
calls refreshTrackedAgentPorts(for:) after attempting to remove the key from
tab.agentPIDs, causing unnecessary work; change the logic to mirror the other
handler by only calling self.refreshTrackedAgentPorts(for: tab) when
tab.agentPIDs.removeValue(forKey: key) returns a non-nil value—locate the async
closure that uses tabForSidebarMutation(id:), the agentPIDs dictionary, and
refreshTrackedAgentPorts(for:) and make the refresh conditional on the
removeValue result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d731ef4a-d1ba-4683-981f-ae15ff65cc64
📒 Files selected for processing (4)
CLI/cmux.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/Workspace.swift
✅ Files skipped from review due to trivial changes (1)
- CLI/cmux.swift
Jesssullivan
added a commit
to Jesssullivan/cmux
that referenced
this pull request
Apr 5, 2026
* fix: fall back to focused surface when new-split target is stale When `new-split` receives a surface UUID that no longer exists (e.g. a closed teammate pane), fall back to the focused surface instead of returning a "Surface not found" error. This matches `new-pane` behavior and fixes agent spawning after team shutdown in Claude Code. * Add editable workspace descriptions (manaflow-ai#2475) * Add editable workspace descriptions * Add workspace description focus UI tests * Include workspace description UI tests in project * Fix workspace description palette focus * Stabilize workspace description UI tests * Force socket mode in description UI tests * Use tagged socket path in description UI tests * Fix workspace description UI test socket setup * Start control socket for UI test launches * Use socket env overrides in description UI tests * Rewrite description UI tests without socket access * Verify saved description by reopening editor * Add workspace description focus debug logs * Prevent terminal focus restore during command palette * Trace workspace description shift-enter handling * Add failing Shift-Enter description UI test * Fix Shift-Enter in workspace description editor * Use live multiline editor state for description submit * Log submitted and normalized workspace descriptions * Trace lower-level Shift-Enter editor routing * Add failing sidebar markdown line-break regression test * Preserve workspace description line breaks in sidebar * Add sidebar multiline description UI smoke test * Expose multiline sidebar descriptions to accessibility * Fix sidebar description refresh lag * Add failing workspace description whitespace test * Preserve workspace description whitespace * fix: keep multiline palette navigation in editor * fix: cap workspace description editor growth --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com> * Detect listening ports for remote SSH workspaces (manaflow-ai#2398) * Add failing SSH remote port detection regression * Detect listening ports for remote SSH workspaces * Retry remote SSH TTY reporting until the target surface exists * Address relay RPC review comments * Avoid host-wide remote port leaks for cmux ssh * Make remote port scans prompt-aware * Fall back when remote ss omits pid data * Tighten remote port scan handoff * Keep raw RPC output intact * Clean up relay TTY handoff review follow-up * Fix remote ssh port surfacing * Fix cmux ssh bootstrap and orphan cleanup * Address SSH remote review feedback * Stabilize SSH remote metadata regression * Fix shell timing and suppressed focus recovery * Harden suppressed focus recovery in tests * Stabilize focus recovery regression timing * Fix SSH relay auth and bootstrap handoff * Fix SSH metadata cleanup assertion --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com> * Add SSH foreground-auth regression tests * Defer remote bootstrap until SSH auth succeeds * Fix session restore terminal cursor focus race (manaflow-ai#2471) * Fix session restore terminal cursor focus race * Fix terminal ready focus observer key * wip * Revert accidental submodule pointer changes from wip commit The wip commit bumped ghostty and bonsplit pointers alongside the actual code fix. Revert them to match main so the PR only contains the session cursor race fix. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * Support chorded keyboard shortcuts (manaflow-ai#2528) * Support chorded keyboard shortcuts * Fix escape handling in shortcut recorder * Add settings.json shortcut overrides * Add regression test for chord reset on deactivate * Fix shortcut chord cleanup edge cases * Add regression tests for chord prefix edge cases * Fix configurable chord prefix routing * Add CLI reload-config command * Simplify reload-config CLI command * Add regression tests for shortcut reload edges * Fix shortcut reload and chord edge cases * Add managed settings.json defaults and schema docs * fix: harden shortcut routing edge cases * fix: preserve palette and browser shortcut routing * chore: retrigger missing PR workflows * feat: add settings.json entry points * feat: make workspace color palette a named dictionary * feat: add textedit button for settings file * refactor: reorder ghostty menu items * refactor: restore settings menu ordering * refactor: restyle settings file entry * refactor: move settings file actions into header * refactor: simplify settings file header action * docs: explain shortcut chords from settings * refactor: reorder shortcut chord actions * fix: notify after swapping settings file store --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com> * Find Homebrew go for dev remote bootstrap * Fix missing sidebar git branch metadata for workspaces (manaflow-ai#2563) * Fix sidebar workspace branch backfill * Add stale branch sidebar regression * Patch GhosttyKit header compatibility * Match Ghostty clipboard callback signature * Tighten sidebar git metadata polling * Fix Ghostty clipboard callback bridge --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com> * Reuse SSH control master for remote relay * fix: harden deferred ssh reconnect handling * Fix missing sidebar ports for agent-run dev servers (manaflow-ai#2562) * test: cover agent-owned sidebar ports * fix: track agent dev-server ports in sidebar * fix: harden ssh localcommand escaping * Fix sidebar layout loop and CLI socket deadlocks (manaflow-ai#2601) * Fix sidebar layout loop and CLI socket deadlocks * Fix sidebar socket target validation * Move sidebar tab resolution onto main queue * Fix sidebar mutation closure parameters * fix: add missing CryptoKit import from upstream merge --------- Co-authored-by: Anusheel Bhushan <anusheel@gmail.com> Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com> Co-authored-by: Austin Wang <austinwang115@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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
Validation
./scripts/reload.sh --tag agent-dev-server-portsCloses #2508
Summary by cubic
Fixes missing sidebar ports for dev servers started by the agent/Codex by tracking agent root PIDs and scanning their descendant processes. Ports now show up quickly and clear when the agent tree stops. Closes #2508.
set_agent_pid/clear_agent_pid; Codex hook infers the real agent PID and records it across start/progress/stop usingcodex.<sessionId>keys.PortScanneragent PID provider and agent-port callback; wire throughTerminalControllerandTabManagerto refresh on PID changes;Workspacenow unionssurfaceListeningPorts,agentListeningPorts, and remote forwards.Written for commit 8900fc2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests