Repository navigation
Improve Claude GUI mode detection for iOS - #7067
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesworkingDirectory propagation through agent session scanning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR improves Claude GUI mode detection for iOS by teaching the observe-floor scanner to discover wrapper-hosted Claude sessions (
Confidence Score: 5/5Safe to merge; the core detect-and-route logic is well-tested and the two findings are non-blocking quality nits. The detection pipeline, pending-alias lifecycle, tombstone fix, and wire protocol changes are all covered by the new test suites. No correctness-critical paths have missing guards or wrong state transitions. The two findings are a per-call encoder allocation regression and internal visibility on test-injectable helper overloads — neither affects runtime correctness. AgentChatSessionRegistry+LiveAgentPID.swift and AgentChatSessionRegistry+ObserveScan.swift — injectable-closure overloads that were private in the old file are now internal, consistent with a pattern flagged in a previous review round that remains unresolved. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Process scan Task.detached] -->|ObservedAgentSession list| B[applyObservedSessions MainActor]
B --> C{sessionID resolved?}
C -->|real UUID env/argv/rollout| D[canonicalClaudeSessionID]
C -->|no session id idle claude| E[pendingClaudeSessionID surfaceID]
D --> F{pending alias on surface?}
F -->|yes| G[Route into pending record hookStoreSessionID=realID]
F -->|no| H[New record under real ID]
E --> I[New pending record state=idle]
G --> J[Hook event fires noteHookEvent]
H --> J
I --> J
J --> K{ended and no transcript?}
K -->|yes| L[emit session_removed versioned]
K -->|no| M[emit descriptorChanged or stateChanged]
L --> N[ChatSessionListReducer removes row no tombstone]
M --> O[iOS session list updated]
N --> P{future descriptorChanged version gt removed?}
P -->|yes| Q[tombstone cleared row re-added]
P -->|no| R[row stays hidden]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Process scan Task.detached] -->|ObservedAgentSession list| B[applyObservedSessions MainActor]
B --> C{sessionID resolved?}
C -->|real UUID env/argv/rollout| D[canonicalClaudeSessionID]
C -->|no session id idle claude| E[pendingClaudeSessionID surfaceID]
D --> F{pending alias on surface?}
F -->|yes| G[Route into pending record hookStoreSessionID=realID]
F -->|no| H[New record under real ID]
E --> I[New pending record state=idle]
G --> J[Hook event fires noteHookEvent]
H --> J
I --> J
J --> K{ended and no transcript?}
K -->|yes| L[emit session_removed versioned]
K -->|no| M[emit descriptorChanged or stateChanged]
L --> N[ChatSessionListReducer removes row no tombstone]
M --> O[iOS session list updated]
N --> P{future descriptorChanged version gt removed?}
P -->|yes| Q[tombstone cleared row re-added]
P -->|no| R[row stays hidden]
Reviews (33): Last reviewed commit: "Address agent chat policy review fixes" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Line 262: The workingDirectory assignment in AgentChatSessionRegistry should
not fall back to generic PWD because it can be stale or spoofed and then gets
persisted as authoritative state. Update observedWorkingDirectory (and the
related uses around SessionRegistry persistence) to source the path only from
CMUX_AGENT_LAUNCH_CWD, or introduce a separate trusted process-cwd source and
fail closed when that trusted value is missing. Ensure the same rule is applied
consistently wherever workingDirectory is recorded or preserved.
- Around line 250-253: Restrict the argv-based session ID fallback in
AgentChatSessionRegistry so it only runs for Claude and not for Codex-derived
identities. In the session lookup logic around sessionIDFromArguments(_:), keep
the existing rollout-path source as the authoritative identity source, and if
that is unavailable for Codex, fail closed instead of parsing
details?.arguments. Update the conditional that currently falls back when
sessionID is nil so it checks the agent identity first, preserves argv parsing
for Claude only, and avoids binding arbitrary UUID-like argv values to Codex
sessions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a94ffb57-4218-426e-bc76-2888352b3b4f
📒 Files selected for processing (2)
Sources/Mobile/AgentChat/AgentChatSessionRegistry.swiftcmuxTests/TaskManagerResourcesTests.swift
…etect # Conflicts: # cmux.xcodeproj/project.pbxproj
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift (1)
124-143: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the detail-aware classifier for live Claude PID recovery.
Line 135 still matches descendants with empty argv/env, so a live
node/bun-hosted Claude process that the new scanner can detect will not be found here. That can leave observed Claude sessions without a live PID or mark them stale during exit/liveness recovery.Suggested fix
guard let info = snapshot.process(pid: pid), - let def = CmuxTaskManagerCodingAgentDefinition.matchingDefinition( - processName: info.name, - processPath: info.path, - arguments: [], - environment: [:] + let def = codingAgentDefinition( + for: info, + processArgumentsAndEnvironment: CmuxTopProcessSnapshot.processArgumentsAndEnvironment(for:) ), def.id == wantedID else { continue }As per path instructions, correctness-critical agent/session identity should use a reliable structured source instead of process-name-only detection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift` around lines 124 - 143, The live PID recovery in AgentChatSessionRegistry.liveAgentPID still uses process-name-only matching with empty arguments/environment, so it can miss Claude processes that the detail-aware scanner recognizes. Update this lookup to use the same detail-aware classifier/structured identity source as the new scanner when iterating snapshot.expandedPIDs, and keep matching against ChatAgentKind.sourceName only after obtaining a structured agent match.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Around line 124-143: The live PID recovery in
AgentChatSessionRegistry.liveAgentPID still uses process-name-only matching with
empty arguments/environment, so it can miss Claude processes that the
detail-aware scanner recognizes. Update this lookup to use the same detail-aware
classifier/structured identity source as the new scanner when iterating
snapshot.expandedPIDs, and keep matching against ChatAgentKind.sourceName only
after obtaining a structured agent match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9a0434fc-fb2b-4dc0-a7e3-3bd9bfb97cc2
📒 Files selected for processing (4)
Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AgentChatSessionRegistryObservationTests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swift (1)
14-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDirect Codex detections still drop
workingDirectory.If a
codexprocess is identified from its binary name/path and rollout file,loadDetails()never runs, sodetailsstaysniland Line 50 always writesworkingDirectory: nil. That means the new cwd preservation only works for sessions that already needed argv/env reads, while the direct-Codex path silently loses the launch cwd.Proposed fix
- guard let resolved = sessionID, !seen.contains(resolved) else { continue } + guard let resolved = sessionID, !seen.contains(resolved) else { continue } + let environment = loadDetails()?.environment seen.insert(resolved) result.append(ObservedAgentSession( sessionID: resolved, agentKind: ChatAgentKind(source: def.id), surfaceID: surfaceID.uuidString, workspaceID: process.cmuxWorkspaceID?.uuidString, pid: process.pid, - workingDirectory: observedWorkingDirectory(details?.environment), + workingDirectory: observedWorkingDirectory(environment), transcriptPath: transcriptPath ))Also applies to: 44-50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry`+ObserveScan.swift around lines 14 - 19, The direct Codex detection path is skipping `loadDetails()`, so `details` remains nil and `workingDirectory` is always written as nil in `AgentChatSessionRegistry+ObserveScan`. Update the scan logic around the `details` cache and the code that builds the session record so `processArgumentsAndEnvironment(process.pid)` is also consulted for the Codex binary-name/rollout-file path, then preserve the extracted cwd when constructing the `workingDirectory` field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry`+ObserveScan.swift:
- Around line 14-19: The direct Codex detection path is skipping
`loadDetails()`, so `details` remains nil and `workingDirectory` is always
written as nil in `AgentChatSessionRegistry+ObserveScan`. Update the scan logic
around the `details` cache and the code that builds the session record so
`processArgumentsAndEnvironment(process.pid)` is also consulted for the Codex
binary-name/rollout-file path, then preserve the extracted cwd when constructing
the `workingDirectory` field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8337c23c-c6e3-4ef0-bf48-7ec5832d3a26
📒 Files selected for processing (2)
Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swiftcmuxTests/AgentChatSessionRegistryObservationTests.swift
![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 2 files (changes from recent commits).
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="Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift:200">
P1: The narrowed `allowsUnidentifiedClaudeLivenessFallback` may incorrectly end active canonical Claude sessions that run under wrapper processes without detectable session-ID markers. Previously all Claude sessions benefited from the unidentified liveness fallback; now only pending sessions do. When a watched wrapper PID exits and the underlying real agent lacks `--session-id`/`CLAUDE_CODE_SESSION_ID` markers, `liveAgentPID` returns nil and the session transitions to `.ended` even though the agent is still alive. Consider keeping the fallback active for canonical sessions that were originally discovered via wrapper args, or ensuring the real binary reliably carries the session markers so liveness can be matched via `expectedSessionIDs`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| surfaceID: surfaceID, | ||
| kind: kind, | ||
| matchingSessionIDs: expectedSessionIDs, | ||
| allowUnidentifiedFallback: Self.allowsUnidentifiedClaudeLivenessFallback(for: record) |
There was a problem hiding this comment.
P1: The narrowed allowsUnidentifiedClaudeLivenessFallback may incorrectly end active canonical Claude sessions that run under wrapper processes without detectable session-ID markers. Previously all Claude sessions benefited from the unidentified liveness fallback; now only pending sessions do. When a watched wrapper PID exits and the underlying real agent lacks --session-id/CLAUDE_CODE_SESSION_ID markers, liveAgentPID returns nil and the session transitions to .ended even though the agent is still alive. Consider keeping the fallback active for canonical sessions that were originally discovered via wrapper args, or ensuring the real binary reliably carries the session markers so liveness can be matched via expectedSessionIDs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift, line 200:
<comment>The narrowed `allowsUnidentifiedClaudeLivenessFallback` may incorrectly end active canonical Claude sessions that run under wrapper processes without detectable session-ID markers. Previously all Claude sessions benefited from the unidentified liveness fallback; now only pending sessions do. When a watched wrapper PID exits and the underlying real agent lacks `--session-id`/`CLAUDE_CODE_SESSION_ID` markers, `liveAgentPID` returns nil and the session transitions to `.ended` even though the agent is still alive. Consider keeping the fallback active for canonical sessions that were originally discovered via wrapper args, or ensuring the real binary reliably carries the session markers so liveness can be matched via `expectedSessionIDs`.</comment>
<file context>
@@ -197,7 +197,7 @@ final class AgentChatSessionRegistry {
kind: kind,
matchingSessionIDs: expectedSessionIDs,
- allowUnidentifiedFallback: kind == .claude
+ allowUnidentifiedFallback: Self.allowsUnidentifiedClaudeLivenessFallback(for: record)
)
await MainActor.run { [weak self] in
</file context>
| allowUnidentifiedFallback: Self.allowsUnidentifiedClaudeLivenessFallback(for: record) | |
| allowUnidentifiedFallback: kind == .claude |
There was a problem hiding this comment.
1 issue found across 11 files (changes from recent commits).
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="Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift:200">
P1: The narrowed `allowsUnidentifiedClaudeLivenessFallback` may incorrectly end active canonical Claude sessions that run under wrapper processes without detectable session-ID markers. Previously all Claude sessions benefited from the unidentified liveness fallback; now only pending sessions do. When a watched wrapper PID exits and the underlying real agent lacks `--session-id`/`CLAUDE_CODE_SESSION_ID` markers, `liveAgentPID` returns nil and the session transitions to `.ended` even though the agent is still alive. Consider keeping the fallback active for canonical sessions that were originally discovered via wrapper args, or ensuring the real binary reliably carries the session markers so liveness can be matched via `expectedSessionIDs`.</violation>
</file>
<file name="Sources/Mobile/AgentChat/AgentChatEndedTranscriptListabilityCache.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatEndedTranscriptListabilityCache.swift:19">
P1: Negative cache entries can become permanently stale because `boundedTranscriptPath` checks live filesystem state (`fileExists`), which can change even when record fields haven't. Once an ended session is cached as unreadable, the early return on `readableBySessionID != nil` prevents ever re-checking the filesystem, so the session may never appear in the transcript list even after its file is written. Consider only short-circuiting when the cached value is already `true` (positive cache), or include an invalidation path for negative entries so transcript availability is eventually re-evaluated.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="Sources/Mobile/AgentChat/AgentChatTranscriptService+Wire.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatTranscriptService+Wire.swift:18">
P2: Wire payload construction fails closed with silent `nil`, risking undetected dropped fan-out events on encoding or schema mismatch. The `wirePayload` helper suppresses all encoding and JSON-shape failures with `try?` and returns `nil` without any diagnostics. Because this helper is explicitly used to create event fan-out payloads, a schema mismatch or caller passing an `Encodable` that does not serialize to a top-level object can cause outbound updates to be dropped silently. At the call site in `AgentChatTranscriptService.emit(frame:)`, a `nil` payload causes the frame to be silently discarded. In `TerminalController+MobileChat.v2MobileChatSessions`, descriptors that fail to encode are silently filtered out via `compactMap` with no non-debug logging. Even if failures are rare today, the silent suppression removes any signal when a regression does occur, making mobile transcript/session state desyncs hard to detect and debug. Consider surfacing encoding failures via at least debug+assert logging inside `wirePayload`, or propagating the error to callers so they can log or handle it appropriately.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| /// event fan-out expects. | ||
| func wirePayload<T: Encodable>(_ value: T) -> [String: Any]? { | ||
| let coding = ChatWireCoding() | ||
| guard let data = try? coding.encode(value), |
There was a problem hiding this comment.
P2: Wire payload construction fails closed with silent nil, risking undetected dropped fan-out events on encoding or schema mismatch. The wirePayload helper suppresses all encoding and JSON-shape failures with try? and returns nil without any diagnostics. Because this helper is explicitly used to create event fan-out payloads, a schema mismatch or caller passing an Encodable that does not serialize to a top-level object can cause outbound updates to be dropped silently. At the call site in AgentChatTranscriptService.emit(frame:), a nil payload causes the frame to be silently discarded. In TerminalController+MobileChat.v2MobileChatSessions, descriptors that fail to encode are silently filtered out via compactMap with no non-debug logging. Even if failures are rare today, the silent suppression removes any signal when a regression does occur, making mobile transcript/session state desyncs hard to detect and debug. Consider surfacing encoding failures via at least debug+assert logging inside wirePayload, or propagating the error to callers so they can log or handle it appropriately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Mobile/AgentChat/AgentChatTranscriptService+Wire.swift, line 18:
<comment>Wire payload construction fails closed with silent `nil`, risking undetected dropped fan-out events on encoding or schema mismatch. The `wirePayload` helper suppresses all encoding and JSON-shape failures with `try?` and returns `nil` without any diagnostics. Because this helper is explicitly used to create event fan-out payloads, a schema mismatch or caller passing an `Encodable` that does not serialize to a top-level object can cause outbound updates to be dropped silently. At the call site in `AgentChatTranscriptService.emit(frame:)`, a `nil` payload causes the frame to be silently discarded. In `TerminalController+MobileChat.v2MobileChatSessions`, descriptors that fail to encode are silently filtered out via `compactMap` with no non-debug logging. Even if failures are rare today, the silent suppression removes any signal when a regression does occur, making mobile transcript/session state desyncs hard to detect and debug. Consider surfacing encoding failures via at least debug+assert logging inside `wirePayload`, or propagating the error to callers so they can log or handle it appropriately.</comment>
<file context>
@@ -0,0 +1,24 @@
+ /// event fan-out expects.
+ func wirePayload<T: Encodable>(_ value: T) -> [String: Any]? {
+ let coding = ChatWireCoding()
+ guard let data = try? coding.encode(value),
+ let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else {
+ return nil
</file context>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swift:286">
P2: The inline `explicitSessionOption` check duplicates the option-grammar already recognized by `sessionIDFromArguments`. If the grammar is ever extended, both sites must be updated or pending-session detection will diverge. Extract a shared helper (or reuse the parser) so the grammar lives in one place.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift:200">
P1: The narrowed `allowsUnidentifiedClaudeLivenessFallback` may incorrectly end active canonical Claude sessions that run under wrapper processes without detectable session-ID markers. Previously all Claude sessions benefited from the unidentified liveness fallback; now only pending sessions do. When a watched wrapper PID exits and the underlying real agent lacks `--session-id`/`CLAUDE_CODE_SESSION_ID` markers, `liveAgentPID` returns nil and the session transitions to `.ended` even though the agent is still alive. Consider keeping the fallback active for canonical sessions that were originally discovered via wrapper args, or ensuring the real binary reliably carries the session markers so liveness can be matched via `expectedSessionIDs`.</violation>
</file>
<file name="Sources/Mobile/AgentChat/AgentChatTranscriptService+Wire.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatTranscriptService+Wire.swift:18">
P2: Wire payload construction fails closed with silent `nil`, risking undetected dropped fan-out events on encoding or schema mismatch. The `wirePayload` helper suppresses all encoding and JSON-shape failures with `try?` and returns `nil` without any diagnostics. Because this helper is explicitly used to create event fan-out payloads, a schema mismatch or caller passing an `Encodable` that does not serialize to a top-level object can cause outbound updates to be dropped silently. At the call site in `AgentChatTranscriptService.emit(frame:)`, a `nil` payload causes the frame to be silently discarded. In `TerminalController+MobileChat.v2MobileChatSessions`, descriptors that fail to encode are silently filtered out via `compactMap` with no non-debug logging. Even if failures are rare today, the silent suppression removes any signal when a regression does occur, making mobile transcript/session state desyncs hard to detect and debug. Consider surfacing encoding failures via at least debug+assert logging inside `wirePayload`, or propagating the error to callers so they can log or handle it appropriately.</violation>
</file>
<file name="Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRegistry+ObserveScan.swift:286">
P2: The inline `explicitSessionOption` check duplicates the option-grammar already recognized by `sessionIDFromArguments`. If the grammar is ever extended, both sites must be updated or pending-session detection will diverge. Extract a shared helper (or reuse the parser) so the grammar lives in one place.</violation>
</file>
<file name="Sources/Mobile/AgentChat/AgentChatSessionRecord.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRecord.swift:83">
P2: The `adoptMissingBindings` method duplicates the same field-by-field adoption logic as `adoptBindings` with only a different overwrite policy. If a new binding field is added, both methods must be updated in lockstep or hydration behavior will diverge depending on which adoption path is used. Consider using a shared adoption primitive (for example, a single method with a propagation-policy parameter or a private field-by-field helper) so the field list is declared once.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
|
|
||
| /// Fills gaps from the hook store without replacing live cmux bindings. | ||
| mutating func adoptMissingBindings( |
There was a problem hiding this comment.
P2: The adoptMissingBindings method duplicates the same field-by-field adoption logic as adoptBindings with only a different overwrite policy. If a new binding field is added, both methods must be updated in lockstep or hydration behavior will diverge depending on which adoption path is used. Consider using a shared adoption primitive (for example, a single method with a propagation-policy parameter or a private field-by-field helper) so the field list is declared once.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Mobile/AgentChat/AgentChatSessionRecord.swift, line 83:
<comment>The `adoptMissingBindings` method duplicates the same field-by-field adoption logic as `adoptBindings` with only a different overwrite policy. If a new binding field is added, both methods must be updated in lockstep or hydration behavior will diverge depending on which adoption path is used. Consider using a shared adoption primitive (for example, a single method with a propagation-policy parameter or a private field-by-field helper) so the field list is declared once.</comment>
<file context>
@@ -79,6 +79,19 @@ struct AgentChatSessionRecord: Sendable {
}
+ /// Fills gaps from the hook store without replacing live cmux bindings.
+ mutating func adoptMissingBindings(
+ from entry: AgentChatHookSessionStore.Entry,
+ includingPID: Bool = true
</file context>
There was a problem hiding this comment.
2 issues found across 19 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="Sources/Mobile/AgentChat/AgentChatSessionRegistry+LiveAgentPID.swift">
<violation number="1" location="Sources/Mobile/AgentChat/AgentChatSessionRegistry+LiveAgentPID.swift:85">
P2: `loadDetails()?.arguments` is evaluated eagerly for every non-root foreground PID before `codingAgentDefinition` runs, because Swift evaluates function arguments left-to-right. This means `processArgumentsAndEnvironment` is called even when the classifier's direct name/path match would succeed without reading argv, or when `shouldReadDetails` is `false` and argv is never needed. This contradicts the PR description's stated goal of "inspecting argv/env only after the process classifier confirms the process is Claude" and adds unnecessary per-process syscalls during tree scans. Consider deferring the argv read: either pass a lazy closure to `allowsLaunchKindEnvironment` or compute the wrapper check inside `codingAgentDefinition` after the `shouldReadDetails` gate.</violation>
</file>
<file name="cmuxTests/ClaudeHookFeedTelemetrySwiftTests.swift">
<violation number="1" location="cmuxTests/ClaudeHookFeedTelemetrySwiftTests.swift:337">
P2: The `runProcess` helper has a hang risk when the child process does not respond to `terminate()` (SIGTERM) within the 1-second grace period. After the timeout path fires, `readDataToEndOfFile()` blocks indefinitely because the process still holds the pipe write-ends open. If this edge case triggers, it stalls the entire test suite. Consider escalating to SIGKILL when the process still hasn't exited after the `terminate()` wait, and using nonblocking reads or closing the pipe write-ends to bound the drain. This follows the established pattern from `Ensure exit before draining pipes` guidance where the process must be confirmed dead before draining pipes.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| info.isTerminalForegroundProcessGroup, | ||
| let def = codingAgentDefinition( | ||
| for: info, | ||
| allowLaunchKindEnvironment: allowsLaunchKindEnvironment( |
There was a problem hiding this comment.
P2: loadDetails()?.arguments is evaluated eagerly for every non-root foreground PID before codingAgentDefinition runs, because Swift evaluates function arguments left-to-right. This means processArgumentsAndEnvironment is called even when the classifier's direct name/path match would succeed without reading argv, or when shouldReadDetails is false and argv is never needed. This contradicts the PR description's stated goal of "inspecting argv/env only after the process classifier confirms the process is Claude" and adds unnecessary per-process syscalls during tree scans. Consider deferring the argv read: either pass a lazy closure to allowsLaunchKindEnvironment or compute the wrapper check inside codingAgentDefinition after the shouldReadDetails gate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Mobile/AgentChat/AgentChatSessionRegistry+LiveAgentPID.swift, line 85:
<comment>`loadDetails()?.arguments` is evaluated eagerly for every non-root foreground PID before `codingAgentDefinition` runs, because Swift evaluates function arguments left-to-right. This means `processArgumentsAndEnvironment` is called even when the classifier's direct name/path match would succeed without reading argv, or when `shouldReadDetails` is `false` and argv is never needed. This contradicts the PR description's stated goal of "inspecting argv/env only after the process classifier confirms the process is Claude" and adds unnecessary per-process syscalls during tree scans. Consider deferring the argv read: either pass a lazy closure to `allowsLaunchKindEnvironment` or compute the wrapper check inside `codingAgentDefinition` after the `shouldReadDetails` gate.</comment>
<file context>
@@ -82,7 +82,11 @@ extension AgentChatSessionRegistry {
let def = codingAgentDefinition(
for: info,
- allowLaunchKindEnvironment: rootPIDs.contains(pid),
+ allowLaunchKindEnvironment: allowsLaunchKindEnvironment(
+ for: info,
+ rootPIDs: rootPIDs,
</file context>
| let timedOut = exitSignal.wait(timeout: .now() + timeout) == .timedOut | ||
| if timedOut { | ||
| process.terminate() | ||
| _ = exitSignal.wait(timeout: .now() + 1) |
There was a problem hiding this comment.
P2: The runProcess helper has a hang risk when the child process does not respond to terminate() (SIGTERM) within the 1-second grace period. After the timeout path fires, readDataToEndOfFile() blocks indefinitely because the process still holds the pipe write-ends open. If this edge case triggers, it stalls the entire test suite. Consider escalating to SIGKILL when the process still hasn't exited after the terminate() wait, and using nonblocking reads or closing the pipe write-ends to bound the drain. This follows the established pattern from Ensure exit before draining pipes guidance where the process must be confirmed dead before draining pipes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmuxTests/ClaudeHookFeedTelemetrySwiftTests.swift, line 337:
<comment>The `runProcess` helper has a hang risk when the child process does not respond to `terminate()` (SIGTERM) within the 1-second grace period. After the timeout path fires, `readDataToEndOfFile()` blocks indefinitely because the process still holds the pipe write-ends open. If this edge case triggers, it stalls the entire test suite. Consider escalating to SIGKILL when the process still hasn't exited after the `terminate()` wait, and using nonblocking reads or closing the pipe write-ends to bound the drain. This follows the established pattern from `Ensure exit before draining pipes` guidance where the process must be confirmed dead before draining pipes.</comment>
<file context>
@@ -0,0 +1,369 @@
+ let timedOut = exitSignal.wait(timeout: .now() + timeout) == .timedOut
+ if timedOut {
+ process.terminate()
+ _ = exitSignal.wait(timeout: .now() + 1)
+ }
+
</file context>
3 conflicts, resolved keeping HEAD's refactored structure: - TaskManagerTypes.swift: kept HEAD (CmuxTaskManagerCodingAgentDefinition lives in the CMUXAgentLaunch package; main re-added it inline — dropped the dup). - TerminalController+MobileChat.swift: kept HEAD's MobileChatRPCHandler delegation; main's inline v2MobileChat* methods moved to the handler in the refactor. - project.pbxproj: union-dedup (#7067's new AgentChat scan/test files auto-merged and wired; 0 dangling refs, normalized, test-wiring lint green). #7067's core detection work (observeAgentProcessesForListing, AgentChatSessionRegistry scan files, tests) auto-merged cleanly. One divergent piece recorded for owner reconcile (merge-deferred-gaps): whether MobileChatRPCHandler.sessions() adopts #7067's observe-before-listing under HEAD's PR-6631 hook-event agent-session SoT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/argumentNeedles public #7067's app-target AgentChatSessionRegistry+ObserveScan reads these; the refactor moved the struct into the CMUXAgentLaunch package and left them internal (main had it app-target, same-module). Widen to public for the detection API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
CLAUDE_CODE_SESSION_ID,--session-id, or--resumeyet by creating a cmux-owned pending session keyed to the terminal surface.session_removedframes for retired aliases, and make iOS tombstone only versioned removals so old unversioned removes cannot hide future descriptors permanently.Why
Codex rediscovery was reliable because Codex usually has an agent-named executable and an open rollout JSONL. Claude often appears as
node,bun, or a versioned launcher; a brand-new idleclaudeprompt can also have no real session id in argv/env until the first hook fires. The observer now handles both paths without treating stale hook-store PIDs as live truth.Tests
python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsvpython3 scripts/check-test-determinism.py --strictgit diff --checkswift test --package-path Packages/Shared/CmuxAgentChat --filter 'ChatSessionListReducerTests|TerminalWireCodableTests'xcodebuild test -project cmux.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-iosgui-removal-tests6 -only-testing:cmuxTests/AgentChatSessionRegistryObservationTests -only-testing:cmuxTests/AgentChatSessionRegistryLifecycleTestsReviews and CI
$autoreview: clean on head3e8f3ea5a68563abc3a3febe10c3b8ac6fec5a5a.Reloads
guifix, built via./scripts/reload-cloud.sh --tag guifixwith local fallback because no cloud builder slot was configured.guifix, built, installed, launched, signed in, and auto-paired on iPhone 17 simulator.devicectlreports Abdulaziz’s iPad, Aziz, and MyPhone as unavailable.web/scripts/db-local.shrequiresdocker, anddockeris not installed on this machine.Notes
.claude/settings.jsonallow entries are ignored until the project is trusted, but it is not the root cause of the GUI session detection bug fixed here.