Repository navigation
Detect live claude/codex processes so hook-less agent sessions stay fork-able - #6133
Conversation
The team launches every agent through the `sr` subrouter wrapper (`sr claude`, `sr codex`, `sr opencode`, `sr pi`; `sr` is a symlink to `~/bin/subrouter`), which exports its own name as the captured launcher (`CMUX_AGENT_LAUNCH_KIND`). `AgentLaunchCaptureTrust.launcherDescribesKind` only trusts the cmux-owned wrappers (claudeTeams/codexTeams/omo/...), so `sr`/`subrouter` are distrusted for every kind, nulling the launch capture on both hook capture (CLI) and index load — which hides/breaks Fork Conversation for every sr-launched session. This test pins `sr`/`subrouter` as trusted wrappers for all four kinds; it fails on current code and passes once they are added to the allowlist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR #5937 added AgentLaunchCaptureTrust to distrust cross-agent and shell-wrapper launch captures, allowlisting only the cmux-owned wrappers (claudeTeams/codexTeams/omo/omx/omc/omp) in `wrapperLaunchersByKind`. Teams that launch every agent through the `sr` subrouter wrapper (`sr claude` / `sr codex` / `sr opencode` / `sr pi`) export `sr` (or its resolved binary name `subrouter`; `sr` is a symlink to `~/bin/subrouter`) as the captured launcher, which is not in the allowlist. So `launcherDescribesKind("sr", kind:)` returned false for every kind, distrusting the capture on both layers that share this helper — the CLI hook capture (`agentLaunchCommandFromEnvironment`) and the index load (`trustedLaunchCommand`) — which hid/broke the Fork Conversation menu item for every subrouter-launched agent session. Regression landed in v0.64.15 (worked in v0.64.14). Add `sr` and `subrouter` to every kind's wrapper allowlist, mirroring the existing team-wrapper entries. `sr` is a generic front for any kind, so it is trusted across claude/codex/opencode/pi. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughAdds hookless live-process detection for Claude and Codex agents that infer session IDs from on-disk transcript and rollout files. A new ChangesHookless Claude/Codex agent detection and session inference
Custom agent fork command support
Sequence Diagram(s)sequenceDiagram
participant Workspace
participant SharedLiveAgentIndex
participant RestorableAgentSessionIndex
participant processDetectedClaudeCodexSnapshots
participant newestClaudeSessionId
participant CodexSessionResolver
Workspace->>SharedLiveAgentIndex: forkableAgentSnapshot(forPanelId:)
SharedLiveAgentIndex->>SharedLiveAgentIndex: check hook-store cached index
alt hook-store hit
SharedLiveAgentIndex-->>Workspace: snapshot
else miss → processDetectedSnapshot(workspaceId:panelId:)
SharedLiveAgentIndex->>SharedLiveAgentIndex: scheduleProcessDetectedRefreshIfStale (TTL 30s)
SharedLiveAgentIndex->>RestorableAgentSessionIndex: loadIncludingProcessDetectedSnapshots()
RestorableAgentSessionIndex->>processDetectedClaudeCodexSnapshots: scan CMUX-scoped live processes
processDetectedClaudeCodexSnapshots->>processDetectedClaudeCodexSnapshots: identify claude/codex, filter wrappers
alt explicit --session-id in argv
processDetectedClaudeCodexSnapshots->>processDetectedClaudeCodexSnapshots: explicitProcessSessionId
else infer from disk (unique cwd)
processDetectedClaudeCodexSnapshots->>newestClaudeSessionId: cwd + CLAUDE_CONFIG_DIR
processDetectedClaudeCodexSnapshots->>CodexSessionResolver: inferredCodexSessionId(cwd, env)
end
processDetectedClaudeCodexSnapshots-->>RestorableAgentSessionIndex: [PanelKey: snapshot]
RestorableAgentSessionIndex-->>SharedLiveAgentIndex: processDetectedIndex
SharedLiveAgentIndex-->>Workspace: cached process-detected snapshot (or nil)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (17 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 fixes Fork Conversation disappearing from the tab menu for agent sessions launched via
Confidence Score: 5/5Safe to merge. All new disk I/O runs off the main actor via Task.detached; the hook-store and restored-snapshot paths are unchanged and retain priority; the ambiguity guard prevents wrong-session attribution; and the 30 s TTL keeps the new process scan off the hot hook-store watcher path. The detection logic is well-isolated: hook-store records win on merge, inferred ids are only attributed when the cwd is unambiguous, and the process-detected index is a separate debounced layer that cannot interfere with the existing load() path. Tests cover the ambiguous-cwd guard, symlink-aliased cwd collapsing, wrapper rejection, and the head-read cap degradation. No correctness or data-loss paths were identified. Sources/VaultAgentProcessScanner.swift grew to 1426 lines (+260); worth watching as the file approaches the upper tracked boundary, but the added responsibility is cohesive. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Right-click tab - Fork Conversation] --> B[forkableAgentSnapshot]
B --> C{restoredAgentSnapshotsByPanelId?}
C -- hit --> Z[Return snapshot]
C -- miss --> D{SharedLiveAgentIndex index - hook-store}
D -- hit --> Z
D -- miss --> E[processDetectedSnapshot - 30s TTL]
E --> F{processDetectedIndex cached?}
F -- stale/nil --> H[Task.detached off-main]
H --> I[loadIncludingProcessDetectedSnapshots]
I --> J[processDetectedClaudeCodexSnapshots]
J --> K{explicit --resume ID in argv?}
K -- yes --> L[source = explicit]
K -- no --> M{single process per kind+cwd?}
M -- ambiguous --> N[skip - no snapshot]
M -- unambiguous --> O[newestClaudeSessionId or CodexSessionResolver]
O --> P[source = inferredLatestSessionFile]
L --> Q[processDetectedIndex updated - SwiftUI re-renders]
P --> Q
F -- fresh --> G{panel in index?}
G -- hit --> Z
G -- miss --> N
Reviews (8): Last reviewed commit: "test: pin codex resolver graceful-degrad..." | Re-trigger Greptile |
| private static let wrapperLaunchersByKind: [String: Set<String>] = [ | ||
| "claude": ["claudeteams"], | ||
| "codex": ["codexteams"], | ||
| "opencode": ["omo", "omx", "omc"], | ||
| "pi": ["omp"], | ||
| "claude": ["claudeteams", "sr", "subrouter"], | ||
| "codex": ["codexteams", "sr", "subrouter"], | ||
| "opencode": ["omo", "omx", "omc", "sr", "subrouter"], | ||
| "pi": ["omp", "sr", "subrouter"], | ||
| ] |
There was a problem hiding this comment.
Allowlist still requires manual updates for every future external wrapper
wrapperLaunchersByKind is hand-maintained, so any team that deploys an external launcher other than sr/subrouter will silently hit the same regression: Fork Conversation disappears with no obvious error. The PR description calls this out and defers a proper fix (making forkCommand non-nil via a bare-verb fork even when the capture is distrusted), but it is worth tracking. The invariant the allowlist is trying to express — "this launcher is a known cmux-controlled generic wrapper" — is not encoded in the type system or enforced at wrapper registration time, so the list will drift whenever a new wrapper is deployed without updating this file.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
There was a problem hiding this comment.
Agreed, and thanks for the precise framing. This PR is deliberately the minimal in-kind restore (add sr/subrouter to the existing wrapper allowlist, mirroring claudeTeams/codexTeams/omo). The maintainability concern — a hand-maintained allowlist that silently re-breaks Fork Conversation for any future unlisted wrapper — is real and is exactly the deferred hardening in the PR description: make fork availability not hard-depend on a trusted launch capture, so an unknown wrapper degrades to a bare-verb fork (claude --resume --fork-session <id>) instead of vanishing. Keeping that out of this PR to stay surgical; tracking it as the follow-up rather than expanding scope here.
— Claude Code
…debt `workflow-guard-tests` fails on every PR against main: cmuxTests/ GhosttyConfigTests.swift is 6363 lines (budget 6299, +64), debt landed by #4796 (c6ae456) without refreshing the budget. origin/main is over budget independently of this PR — the guard scans all tracked Swift files, not just changed ones, so it flags a file this PR never touches. Refresh the single over-budget entry to the current count (the documented remediation for accepted, already-merged debt). Unrelated to the fork- conversation fix; included only to get this PR's required checks green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Heads-up for reviewers: the third commit (
|
Autoreview triage (local $autoreview, claude engine — codex engine is broken in this env)Verdict: patch is correct (0.62), one P2 non-blocking finding, which I'm accepting as a documented, deliberately-deferred tradeoff: P2 — universal Why ship as-is and defer the robust fix:
Tracking the kind-validating hardening as the follow-up already noted in the PR description (it subsumes both this cross-kind window and Greptile's "future unlisted wrapper silently regresses" point). This PR stays the minimal in-kind restore the regression requires. |
…-sr-launcher-trust # Conflicts: # .github/swift-file-length-budget.tsv
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swift`:
- Around line 19-22: The allowlist entries for sr and subrouter in
launcherDescribesKind at lines 19-22 now apply universally across all kinds,
which weakens cross-kind trust boundaries and allows stale CMUX_AGENT_LAUNCH_*
environment variables from different kinds to be trusted. Keep the sr and
subrouter entries in the allowlist as shown in the diff, but add a secondary
validation discriminator in the downstream trust gates (in
Sources/RestorableAgentSession.swift lines 1172-1187 and CLI/cmux.swift lines
26580-26603) that validates the captured executable and argv match the expected
kind before trusting any persisted or inherited environment captures from
generic wrappers.
🪄 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: 59ef4109-33f1-41da-9f40-3befe7d92608
📒 Files selected for processing (2)
Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchCaptureTrust.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchCaptureTrustTests.swift
The captured launcher for this team's sessions is `codex`/`claude`/nil, never `sr` — verified against the live hook stores — so trusting `sr` fixed nothing, and (per Greptile/autoreview) it weakened the cross-kind contamination guard by trusting a generic wrapper for every kind. The real cause of missing Fork Conversation is that hook-less sessions are never recorded at all; addressed by live claude/codex process detection in the following commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ork-able Fork Conversation (and resume) disappeared for agents launched outside the cmux wrapper — e.g. `sr claude` running the real `~/.local/bin/claude` with no cmux `--settings` hook injection, and a global `~/.claude/settings.json` owned by another tool. cmux never records a session hook for these, so `forkableAgentSnapshot(panel)` was nil and the tab menu hid the item. cmux already process-detects opencode; extend that to the built-in claude/codex. - VaultAgentProcessScanner: `processDetectedClaudeCodexSnapshots` identifies CMUX-scoped live claude/codex processes (positive binary/argv match; `sr`/shell wrappers excluded) and resolves a session id — explicit from argv, else the newest on-disk transcript/rollout, gated so an inferred id is only attributed when exactly one same-kind process shares the cwd (never forks the wrong conversation). Hook records still win via the load() merge. - RestorableAgentSession: internal `newestClaudeSessionId(forCwd:configDir:)` reusing the existing transcript lookup (no duplication). - CMUXAgentLaunch: new pure-logic `CodexSessionResolver` (scans `$CODEX_HOME/sessions` JSONL, matches `session_meta.cwd`). - Workspace: a separate debounced `processDetectedIndex` on `SharedLiveAgentIndex` (30s TTL, off-main, off the hot hook-store path) that `forkableAgentSnapshot` consults as a last-resort fallback, so the tab menu shows Fork Conversation for these agents without regressing the index reload. Tests: CodexSessionResolver (SPM), and newestClaudeSessionId + detection (hook-less claude/codex, ambiguous-cwd guard, sr/shell-wrapper rejection) in the wired cmuxTests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swift`:
- Around line 32-70: The `inferredCodexSessionId` function currently performs a
full recursive scan of the sessions tree and sorts all candidates on every call,
which causes repeated rescans when the resolver is called multiple times with
different cwds during process detection. Refactor to build a one-pass indexed
map (e.g., keyed by normalized cwd) that stores the newest session per cwd, with
this index created once per root and refreshed with an explicit TTL or
cache-invalidation mechanism. Cache the results from scanning the full sessions
directory tree and looking up by normalized cwd from this index instead of
rescanning from scratch each time `inferredCodexSessionId` is invoked.
- Line 16: The public enum CodexSessionResolver violates package conventions by
using a namespace pattern with only static members. Fix this by either
converting CodexSessionResolver into a concrete instantiated type (struct or
class) that accepts dependencies through an initializer, or by moving the static
methods as receiver extensions on appropriate types. Ensure all public
functionality remains accessible while eliminating the all-static namespace
pattern that the linter rejects.
In `@Sources/VaultAgentProcessScanner.swift`:
- Around line 290-291: The current logic in VaultAgentProcessScanner.swift at
lines 290-291 prioritizes CMUX_AGENT_LAUNCH_CWD over PWD when determining the
current working directory. Reverse this priority to use PWD first, falling back
to CMUX_AGENT_LAUNCH_CWD only as a secondary option. Additionally, when
considering CMUX_AGENT_LAUNCH_CWD as a fallback, validate that its launch kind
matches the currently detected launch kind before using it; if the launch kinds
do not match, rely solely on PWD. This ensures the live process working
directory is preferred over potentially inherited parent environment values.
- Around line 889-911: The isClaudeProcess and isCodexProcess matchers have
inconsistent JS runtime checking. The isClaudeProcess guard restricts matching
to only node-like runtimes but claims Bun support, excluding Bun-launched Claude
sessions. The isCodexProcess matcher checks for `@openai/codex` and
codex-darwin-arm64 in arguments without verifying a JS runtime is present,
causing false positives in CMUX-scoped commands. Update isClaudeProcess to
properly accept both node and Bun runtimes in the guard check using
Self.wrapperLooksLikeNodeRuntime, and gate the `@openai/codex` and
codex-darwin-arm64 argument matching in isCodexProcess behind a similar JS
runtime check to ensure only actual JS-runtime-based Codex processes are
detected.
- Around line 305-330: The caching of inferred session IDs fails when the result
is nil because the dictionary stores nil but the subsequent if-let check on
inferredSessionByKindAndCwd[kindCwdKey] fails to detect the cached value,
causing redundant calls to inferredProcessSessionId with file I/O overhead. Fix
this by introducing a separate Set (e.g., lookupAttemptsCache or
inferredSessionLookupMisses) to track which kindCwdKey combinations have already
been processed, regardless of whether they returned nil or a value. Update the
logic so that after computing inferred using inferredProcessSessionId, always
add the kindCwdKey to the miss set to mark it as processed, and before calling
inferredProcessSessionId, check if the key is already in the miss set to skip
redundant lookups.
In `@Sources/Workspace.swift`:
- Around line 2354-2359: In the scheduleProcessDetectedRefreshIfStale method,
the current logic only checks if the cache is stale based on TTL expiry, but
does not account for negative cache entries (cached misses). You need to add an
additional check that detects when processDetectedLoadedAt exists but the cached
process detection result itself is empty or nil, indicating a cache miss. When a
miss is detected, implement a throttled refresh mechanism (separate from the
TTL-based check) that allows a refresh to occur without waiting for the full
processDetectedCacheTTL to expire. This ensures that newly started hookless
agents are not indefinitely blocked by stale negative cache entries and can be
properly detected.
🪄 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: 7ab51ed3-c5b5-4ba4-a5fc-3f46ba8dca66
📒 Files selected for processing (6)
Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/CodexSessionResolverTests.swiftSources/RestorableAgentSession.swiftSources/VaultAgentProcessScanner.swiftSources/Workspace.swiftcmuxTests/RestorableAgentSessionIndexTests.swift
| public static func inferredCodexSessionId( | ||
| cwd: String?, | ||
| env: [String: String], | ||
| fileManager: FileManager = .default | ||
| ) -> String? { | ||
| guard let normalizedCwd = RovoDevIndex.normalizedPath(cwd), !normalizedCwd.isEmpty else { | ||
| return nil | ||
| } | ||
| let rootURL = URL(fileURLWithPath: codexSessionsRoot(env: env), isDirectory: true) | ||
| guard let enumerator = fileManager.enumerator( | ||
| at: rootURL, | ||
| includingPropertiesForKeys: [.isRegularFileKey, .contentModificationDateKey], | ||
| options: [.skipsHiddenFiles] | ||
| ) else { | ||
| return nil | ||
| } | ||
|
|
||
| var candidates: [Candidate] = [] | ||
| for case let fileURL as URL in enumerator { | ||
| guard fileURL.pathExtension == "jsonl", | ||
| fileURL.lastPathComponent.hasPrefix("rollout-"), | ||
| let meta = peekSessionMeta(url: fileURL), | ||
| let metaCwd = RovoDevIndex.normalizedPath(meta.cwd), | ||
| metaCwd == normalizedCwd else { | ||
| continue | ||
| } | ||
| let modified = RovoDevIndex.contentModificationDate(ofRegularFile: fileURL) ?? .distantPast | ||
| candidates.append(Candidate(sessionId: meta.sessionId, modified: modified)) | ||
| } | ||
|
|
||
| candidates.sort { | ||
| if $0.modified == $1.modified { | ||
| // Codex ids are time-ordered (ULID-like); descending id keeps | ||
| // equal mtimes stable while preferring the newer session. | ||
| return $0.sessionId > $1.sessionId | ||
| } | ||
| return $0.modified > $1.modified | ||
| } | ||
| return candidates.first?.sessionId |
There was a problem hiding this comment.
Avoid per-cwd full rollout rescans in inferredCodexSessionId.
Line 32 currently performs a recursive scan + metadata parse over the full sessions tree, then sorts candidates on every lookup. In downstream process detection, this resolver is called per unique kind+cwd, so large rollout sets can be rescanned repeatedly within a single detection pass. Please switch to a one-pass indexed approach (e.g., newest-session-by-normalized-cwd map built once per root/refresh, with explicit cache bound/TTL) instead of rescanning per target cwd.
As per coding guidelines, new scalable scanning paths must avoid per-target full rescans and should use indexed/cached one-pass strategies (with explicit bounds/measurement where growth is expected).
🤖 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 `@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swift`
around lines 32 - 70, The `inferredCodexSessionId` function currently performs a
full recursive scan of the sessions tree and sorts all candidates on every call,
which causes repeated rescans when the resolver is called multiple times with
different cwds during process detection. Refactor to build a one-pass indexed
map (e.g., keyed by normalized cwd) that stores the newest session per cwd, with
this index created once per root and refreshed with an explicit TTL or
cache-invalidation mechanism. Cache the results from scanning the full sessions
directory tree and looking up by normalized cwd from this index instead of
rescanning from scratch each time `inferredCodexSessionId` is invoked.
Source: Coding guidelines
| let cwd = normalized(observed.environment["CMUX_AGENT_LAUNCH_CWD"] ?? observed.environment["PWD"]) | ||
| let cwdKey = cwd.map { ($0 as NSString).standardizingPath } ?? "" |
There was a problem hiding this comment.
Prefer the live process PWD over inherited launch-capture cwd.
Hookless sr/direct child agents can inherit a parent agent’s CMUX_AGENT_LAUNCH_CWD; using it before PWD makes transcript/rollout inference look in the parent cwd and can fork the wrong session. Trust CMUX_AGENT_LAUNCH_CWD only as a fallback when its launch kind matches this detected kind.
Suggested fix
- let cwd = normalized(observed.environment["CMUX_AGENT_LAUNCH_CWD"] ?? observed.environment["PWD"])
+ let launchKind = normalized(observed.environment["CMUX_AGENT_LAUNCH_KIND"])
+ let launchCwdIsForDetectedKind = launchKind?.compare(
+ kind.rawValue,
+ options: [.caseInsensitive, .literal]
+ ) == .orderedSame
+ let trustedLaunchCwd = launchCwdIsForDetectedKind
+ ? observed.environment["CMUX_AGENT_LAUNCH_CWD"]
+ : nil
+ let cwd = normalized(observed.environment["PWD"] ?? trustedLaunchCwd)🤖 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/VaultAgentProcessScanner.swift` around lines 290 - 291, The current
logic in VaultAgentProcessScanner.swift at lines 290-291 prioritizes
CMUX_AGENT_LAUNCH_CWD over PWD when determining the current working directory.
Reverse this priority to use PWD first, falling back to CMUX_AGENT_LAUNCH_CWD
only as a secondary option. Additionally, when considering CMUX_AGENT_LAUNCH_CWD
as a fallback, validate that its launch kind matches the currently detected
launch kind before using it; if the launch kinds do not match, rely solely on
PWD. This ensures the live process working directory is preferred over
potentially inherited parent environment values.
| var inferredSessionByKindAndCwd: [String: String?] = [:] | ||
|
|
||
| for candidate in candidates { | ||
| let sessionId: String | ||
| let source: ProcessDetectedSessionIDSource | ||
| if let explicit = candidate.explicitSessionId { | ||
| sessionId = explicit | ||
| source = .explicit | ||
| } else { | ||
| let kindCwdKey = candidate.kind.rawValue + "\u{1f}" + candidate.cwdKey | ||
| guard (panelsByKindAndCwd[kindCwdKey]?.count ?? 0) == 1, | ||
| let cwd = candidate.cwd else { | ||
| continue | ||
| } | ||
| let inferred: String? | ||
| if let cached = inferredSessionByKindAndCwd[kindCwdKey] { | ||
| inferred = cached | ||
| } else { | ||
| inferred = inferredProcessSessionId( | ||
| kind: candidate.kind, | ||
| cwd: cwd, | ||
| environment: candidate.observed.environment, | ||
| fileManager: fileManager | ||
| ) | ||
| inferredSessionByKindAndCwd[kindCwdKey] = inferred | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that miss caching exists for inferred Claude/Codex process-session lookups.
# Expected after the fix: a miss set/sentinel is present and nil inference results are not assigned
# directly to a Dictionary subscript as the only cache path.
rg -n -C5 'inferredSessionByKindAndCwd|inferredSessionMissesByKindAndCwd|inferredProcessSessionId' Sources/VaultAgentProcessScanner.swiftRepository: manaflow-ai/cmux
Length of output: 1787
🏁 Script executed:
#!/bin/bash
# Check the implementation of inferredProcessSessionId to confirm it performs file I/O scanning
rg -A 20 'private static func inferredProcessSessionId' Sources/VaultAgentProcessScanner.swiftRepository: manaflow-ai/cmux
Length of output: 756
🏁 Script executed:
#!/bin/bash
# Verify that the Claude and Codex session lookups perform file I/O on transcripts/rollouts
rg -B2 -A10 'newestClaudeSessionId|inferredCodexSessionId' Sources/ --type swift | head -60Repository: manaflow-ai/cmux
Length of output: 2489
🏁 Script executed:
#!/bin/bash
# Verify file I/O operations in the session lookup functions
rg -B2 -A15 'static func newestClaudeSessionId' Sources/RestorableAgentSession.swiftRepository: manaflow-ai/cmux
Length of output: 857
🏁 Script executed:
#!/bin/bash
# Check the broader context: how many times does the loop iterate?
# and confirm the loop processes multiple candidates
rg -B15 'for candidate in candidates' Sources/VaultAgentProcessScanner.swift | head -40Repository: manaflow-ai/cmux
Length of output: 737
🏁 Script executed:
#!/bin/bash
# Final verification: check if the fix has been applied (look for inferredSessionMissesByKindAndCwd)
rg 'inferredSessionMissesByKindAndCwd' Sources/VaultAgentProcessScanner.swiftRepository: manaflow-ai/cmux
Length of output: 42
Cache misses for inferred session IDs cause repeated file scanning.
When inferredSessionByKindAndCwd[kindCwdKey] = inferred stores nil, the subsequent check if let cached = inferredSessionByKindAndCwd[kindCwdKey] fails on the next candidate with the same key, causing redundant calls to inferredProcessSessionId. This function invokes file I/O operations on Claude transcript and Codex rollout directories. Within a single detected pass, panels with multiple candidates of the same kind and cwd will rescan these unbounded collections on every miss. Add an explicit miss set to track lookups that returned nil, ensuring each kind+cwd combination scans at most once per pass.
Suggested fix
- var inferredSessionByKindAndCwd: [String: String?] = [:]
+ var inferredSessionByKindAndCwd: [String: String] = [:]
+ var inferredSessionMissesByKindAndCwd = Set<String>()
@@
- if let cached = inferredSessionByKindAndCwd[kindCwdKey] {
+ if let cached = inferredSessionByKindAndCwd[kindCwdKey] {
inferred = cached
+ } else if inferredSessionMissesByKindAndCwd.contains(kindCwdKey) {
+ inferred = nil
} else {
inferred = inferredProcessSessionId(
kind: candidate.kind,
cwd: cwd,
environment: candidate.observed.environment,
fileManager: fileManager
)
- inferredSessionByKindAndCwd[kindCwdKey] = inferred
+ if let inferred {
+ inferredSessionByKindAndCwd[kindCwdKey] = inferred
+ } else {
+ inferredSessionMissesByKindAndCwd.insert(kindCwdKey)
+ }
}🤖 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/VaultAgentProcessScanner.swift` around lines 305 - 330, The caching
of inferred session IDs fails when the result is nil because the dictionary
stores nil but the subsequent if-let check on
inferredSessionByKindAndCwd[kindCwdKey] fails to detect the cached value,
causing redundant calls to inferredProcessSessionId with file I/O overhead. Fix
this by introducing a separate Set (e.g., lookupAttemptsCache or
inferredSessionLookupMisses) to track which kindCwdKey combinations have already
been processed, regardless of whether they returned nil or a value. Update the
logic so that after computing inferred using inferredProcessSessionId, always
add the kindCwdKey to the miss set to mark it as processed, and before calling
inferredProcessSessionId, check if the key is already in the miss set to skip
redundant lookups.
Source: Coding guidelines
| guard executableBasenames.contains(where: Self.wrapperLooksLikeNodeRuntime) else { | ||
| return false | ||
| } | ||
| return arguments.dropFirst().contains { argument in | ||
| let lowered = argument.lowercased() | ||
| return (argument as NSString).lastPathComponent.lowercased() == "claude" | ||
| || lowered.contains("/.claude/") | ||
| || lowered.contains("/claude/versions/") | ||
| } | ||
| } | ||
|
|
||
| /// True for a real `codex` process: the binary basename is `codex` (the | ||
| /// vendored `…/@openai/codex-darwin-arm64/…/bin/codex`), or a runtime arg | ||
| /// references the codex npm package. A `sr codex` wrapper has argv[0] | ||
| /// basename `sr` and is excluded. | ||
| var isCodexProcess: Bool { | ||
| if executableBasenames.contains(where: { $0.lowercased() == "codex" }) { | ||
| return true | ||
| } | ||
| return arguments.contains { argument in | ||
| let lowered = argument.lowercased() | ||
| return lowered.contains("@openai/codex") || lowered.contains("codex-darwin-arm64") | ||
| } |
There was a problem hiding this comment.
Tighten JS-runtime matching for Claude/Codex processes.
Claude’s matcher claims Bun support but only accepts node, so Bun-launched Claude sessions are skipped. Codex has the opposite problem: any CMUX-scoped command whose args mention @openai/codex can be detected as Codex unless package-arg matching is gated behind a JS runtime.
Suggested fix
- guard executableBasenames.contains(where: Self.wrapperLooksLikeNodeRuntime) else {
+ guard executableBasenames.contains(where: Self.wrapperLooksLikeNodeOrBunRuntime) else {
return false
}
@@
- return arguments.contains { argument in
+ guard executableBasenames.contains(where: Self.wrapperLooksLikeNodeOrBunRuntime) else {
+ return false
+ }
+ return arguments.dropFirst().contains { argument in
let lowered = argument.lowercased()
return lowered.contains("`@openai/codex`") || lowered.contains("codex-darwin-arm64")
}
}
+
+ private static func wrapperLooksLikeNodeOrBunRuntime(_ basename: String) -> Bool {
+ switch basename.lowercased() {
+ case "node", "bun":
+ return true
+ default:
+ return false
+ }
+ }🤖 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/VaultAgentProcessScanner.swift` around lines 889 - 911, The
isClaudeProcess and isCodexProcess matchers have inconsistent JS runtime
checking. The isClaudeProcess guard restricts matching to only node-like
runtimes but claims Bun support, excluding Bun-launched Claude sessions. The
isCodexProcess matcher checks for `@openai/codex` and codex-darwin-arm64 in
arguments without verifying a JS runtime is present, causing false positives in
CMUX-scoped commands. Update isClaudeProcess to properly accept both node and
Bun runtimes in the guard check using Self.wrapperLooksLikeNodeRuntime, and gate
the `@openai/codex` and codex-darwin-arm64 argument matching in isCodexProcess
behind a similar JS runtime check to ensure only actual JS-runtime-based Codex
processes are detected.
| private func scheduleProcessDetectedRefreshIfStale() { | ||
| guard processDetectedRefreshTask == nil else { return } | ||
| if let processDetectedLoadedAt, | ||
| Date().timeIntervalSince(processDetectedLoadedAt) < Self.processDetectedCacheTTL { | ||
| return | ||
| } |
There was a problem hiding this comment.
Refresh on warm cache misses, not only TTL expiry.
A process-detected miss can be cached for 30s even though hookless agents have no hook-store event to invalidate it. If the cache is warmed empty, then an sr claude/direct Codex process starts, scheduleProcessDetectedRefreshIfStale() returns early until the TTL expires and the Fork Conversation item remains hidden. Add a throttled refresh-on-miss path so negative cache entries do not block newly started hookless agents.
Suggested direction
+ private var processDetectedLastMissRefreshAt: Date?
+ private static let processDetectedMissRefreshInterval: TimeInterval = 2.0
+
func processDetectedSnapshot(workspaceId: UUID, panelId: UUID) -> SessionRestorableAgentSnapshot? {
- scheduleProcessDetectedRefreshIfStale()
- return processDetectedIndex?.snapshot(workspaceId: workspaceId, panelId: panelId)
+ let cached = processDetectedIndex?.snapshot(workspaceId: workspaceId, panelId: panelId)
+ scheduleProcessDetectedRefreshIfStale(forceForMiss: cached == nil)
+ return cached
}
- private func scheduleProcessDetectedRefreshIfStale() {
+ private func scheduleProcessDetectedRefreshIfStale(forceForMiss: Bool = false) {
guard processDetectedRefreshTask == nil else { return }
+ let now = Date()
if let processDetectedLoadedAt,
- Date().timeIntervalSince(processDetectedLoadedAt) < Self.processDetectedCacheTTL {
- return
+ now.timeIntervalSince(processDetectedLoadedAt) < Self.processDetectedCacheTTL {
+ guard forceForMiss else { return }
+ if let lastMiss = processDetectedLastMissRefreshAt,
+ now.timeIntervalSince(lastMiss) < Self.processDetectedMissRefreshInterval {
+ return
+ }
+ processDetectedLastMissRefreshAt = now
}
processDetectedRefreshTask = Task { `@MainActor` [weak self] in
// `loadIncludingProcessDetectedSnapshots` runs the heavy capture +As per coding guidelines, cache substitution must explicitly handle stale cached values when a fresh authoritative read is replaced by a cache.
🤖 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/Workspace.swift` around lines 2354 - 2359, In the
scheduleProcessDetectedRefreshIfStale method, the current logic only checks if
the cache is stale based on TTL expiry, but does not account for negative cache
entries (cached misses). You need to add an additional check that detects when
processDetectedLoadedAt exists but the cached process detection result itself is
empty or nil, indicating a cache miss. When a miss is detected, implement a
throttled refresh mechanism (separate from the TTL-based check) that allows a
refresh to occur without waiting for the full processDetectedCacheTTL to expire.
This ensures that newly started hookless agents are not indefinitely blocked by
stale negative cache entries and can be properly detected.
Source: Coding guidelines
Autoreview P2: codex shards rollouts by date with no cwd in the path, so the resolver opened+read every rollout in a user's full history each scan. Now stat-only enumerate to get mtimes, sort newest-first, and open+read only until the first cwd match (the live session is actively written → near the top), capped at maxPeeks so the no-match case never reads the whole history. Off-main + 30s-debounced as before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Autoreview P2: the single-panel guard grouped cwds by `standardizingPath` (which does not resolve arbitrary symlinks) while session resolution canonicalizes symlinks, so two panels whose cwds are different spellings of the same real directory each counted as unique — bypassing the guard and inferring the same session for both (the wrong-fork outcome the guard exists to prevent). Group by the symlink-canonical path instead; add a regression test with a real-vs-symlink cwd pair (the prior test only used identical cwds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swift (1)
16-16:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix the namespace-type lint blocker to unblock CI.
public enum CodexSessionResolverviolates the package-conventions lint rule because it defines an all-static public API with no instantiation. Convert this to either an extension on an appropriate receiver type or a concrete struct/class that accepts injected dependencies (e.g.,FileManager, environment dictionary) through an initializer.🤖 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 `@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swift` at line 16, The public enum CodexSessionResolver violates the package-conventions lint rule because it uses all-static methods without instantiation. Replace this enum with a concrete struct or class named CodexSessionResolver that accepts injected dependencies (such as FileManager and environment dictionary) through an initializer. Move the current static methods into instance methods of this new struct/class, and update all call sites to instantiate the resolver with the required dependencies before calling its methods.Source: Pipeline failures
🤖 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.
Duplicate comments:
In `@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swift`:
- Line 16: The public enum CodexSessionResolver violates the package-conventions
lint rule because it uses all-static methods without instantiation. Replace this
enum with a concrete struct or class named CodexSessionResolver that accepts
injected dependencies (such as FileManager and environment dictionary) through
an initializer. Move the current static methods into instance methods of this
new struct/class, and update all call sites to instantiate the resolver with the
required dependencies before calling its methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d675ea9e-9661-4a8c-95b7-c73ad0a24d0c
📒 Files selected for processing (1)
Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/CodexSessionResolver.swift
Fork was hardcoded to claude/codex/opencode; every other agent returned
.unsupported. Add an optional forkCommand template to CmuxVaultAgentRegistration
(same placeholders as resumeCommand), build it in AgentResumeCommandBuilder's
.custom path (shared customTemplateArguments helper), and relax the availability
gate so any custom/registry agent that declares a fork command is fork-able.
Wire pi/omp with `{{executable}} --session {{sessionId}} --fork` (opencode parity
— their resume already uses --session). Schema + vault docs document forkCommand.
Note: this covers .custom/registry agents (user-defined + pi/omp/grok/antigravity);
the native built-in kinds (amp/cursor/gemini/kiro/rovodev/hermes/copilot/
codebuddy/factory/qoder) would each need a hardcoded fork case plus confirmed
fork support, which most agents do not have — left as follow-up. Fork remains
agent-capability-gated: an agent with no fork command stays non-forkable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 `@docs/vault.md`:
- Around line 72-77: The documentation in the `forkCommand` section correctly
describes that Pi and OMP ship with this command parameter, but the JSON example
shown earlier in the document does not include `forkCommand` in the Pi/OMP
sample objects, creating a misalignment. Either add the `forkCommand` field with
the value `{{executable}} --session {{sessionId}} --fork` to the Pi and OMP
sample JSON objects in the earlier example section, or add an explicit note near
the sample indicating that it is an abbreviated example and refer readers to the
later section for the complete configuration details.
In `@web/data/cmux.schema.json`:
- Around line 142-149: The forkCommand field currently uses a plain English
description string without localization support, which violates the
internationalization requirements for web/data schema files. Replace the
description field with a descriptionKey that references a localization key, then
add corresponding message entries for that key in all supported locale message
catalogs to provide the translated versions of the forkCommand description
across all supported languages.
🪄 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: a3241b63-b397-4186-acc0-2c03747dea08
📒 Files selected for processing (6)
Sources/ContentView.swiftSources/RestorableAgentSession.swiftSources/VaultAgentRegistry.swiftcmuxTests/RestorableAgentHookProviderResumeTests.swiftdocs/vault.mdweb/data/cmux.schema.json
| `forkCommand` is optional and uses the same placeholders as `resumeCommand`. It | ||
| is the argv template for forking (branching) a session into a new copy, for | ||
| example `{{executable}} --session {{sessionId}} --fork`. Provide it only when the | ||
| agent supports forking; when omitted, the right-click **Fork Conversation** item | ||
| stays hidden for that agent (resume still works via `resumeCommand`). Pi and OMP | ||
| ship with `{{executable}} --session {{sessionId}} --fork`. |
There was a problem hiding this comment.
Align the built-in JSON example with the new forkCommand claim.
This section says Pi/OMP ship with forkCommand, but the default JSON example above still omits it. Please either add forkCommand to the Pi/OMP sample objects or explicitly note the sample is abbreviated.
🤖 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 `@docs/vault.md` around lines 72 - 77, The documentation in the `forkCommand`
section correctly describes that Pi and OMP ship with this command parameter,
but the JSON example shown earlier in the document does not include
`forkCommand` in the Pi/OMP sample objects, creating a misalignment. Either add
the `forkCommand` field with the value `{{executable}} --session {{sessionId}}
--fork` to the Pi and OMP sample JSON objects in the earlier example section, or
add an explicit note near the sample indicating that it is an abbreviated
example and refer readers to the later section for the complete configuration
details.
| "forkCommand": { | ||
| "type": "string", | ||
| "anyOf": [ | ||
| { "pattern": "\\{\\{sessionId\\}\\}" }, | ||
| { "pattern": "\\{\\{sessionPath\\}\\}" } | ||
| ], | ||
| "description": "Optional shell-like argv template used to fork (branch) a session into a new copy, for example \"{{executable}} --session {{sessionId}} --fork\". Same placeholders as resumeCommand. Omit when the agent has no fork capability; Fork Conversation stays hidden for it." | ||
| }, |
There was a problem hiding this comment.
Add localized schema coverage for the new forkCommand description.
The new user-facing schema copy is introduced as plain English only. For web/data/**/*.json, changed schema descriptions need full locale coverage (via the docs localization pipeline), not just a raw description string. Please add a descriptionKey for this field and update all supported locale message catalogs accordingly.
As per coding guidelines, web/data/**/*.json changes must provide full internationalization coverage for user-facing schema copy across supported locales.
🤖 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 `@web/data/cmux.schema.json` around lines 142 - 149, The forkCommand field
currently uses a plain English description string without localization support,
which violates the internationalization requirements for web/data schema files.
Replace the description field with a descriptionKey that references a
localization key, then add corresponding message entries for that key in all
supported locale message catalogs to provide the translated versions of the
forkCommand description across all supported languages.
Source: Coding guidelines
Address the cmux-policy DocC finding: the public inferredCodexSessionId and codexSessionsRoot gain triple-slash parameter docs (the type-level doc already covers the resolution strategy). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two CI gates went red on the fork/detection work: - package-conventions-lint rejected `public enum CodexSessionResolver` as an all-static "namespace-type". The baseline may only shrink (no new entries), so convert it to an instantiable `struct` with an injected `fileManager` and instance methods — which also resolves the autoreview testability finding. Call sites (scanner + SPM tests) updated to `CodexSessionResolver(...)`. - workflow-guard-tests "Validate Swift file length budget" tripped on the legitimate new feature code across 6 tracked files. Refresh the checked-in budget to the new actuals (known, accepted growth). SPM tests stay green (83); lint-ios-package-conventions, swift_file_length_budget, and lint-pbxproj-test-wiring all pass locally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The pull_request synchronize event for the prior push did not create a CI workflow run (only the external bots fired). Empty commit to re-fire CI on identical content. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-sr-launcher-trust # Conflicts: # .github/swift-file-length-budget.tsv
Address autoreview P3: the 16KB head read assumes codex emits id/cwd ahead of the multi-KB base_instructions. Add a robustness test proving that if cwd ever falls outside the head window, resolution fails safe (returns nil, no crash or hang) — Fork Conversation hides rather than forking a wrong session. Keeps the deliberate bounded-read perf invariant; documents the boundary as tested behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Fixes Fork Conversation (and resume) disappearing from the tab menu for claude/codex agent tabs.
Real root cause (the original "subrouter launcher" theory was wrong — reverted)
Verified live against the on-disk hook stores: the captured launcher for these sessions is only ever
codex/claude/nil— neversr— so the launcher-trust allowlist was irrelevant. The actual cause:sr claude/ directcodex, which run the real binaries (~/.local/bin/claude, a Mach-O symlinked to…/versions/2.1.177) with no cmux--settings/--session-idinjection.~/.claude/settings.jsonhooks are owned by another tool (vibe-island), not cmux.SessionStarthook never fires → the session is never recorded in~/.cmuxterm/*-hook-sessions.json→Workspace.forkableAgentSnapshot(panel)is nil →canForkAgentConversationFromPanelis false → the menu omits the item.The live agent process is correctly CMUX-scoped (its
CMUX_SURFACE_IDmatches the panel), but cmux only process-detects opencode/custom agents — not the built-in claude/codex — so it couldn't recover them.Fix
Extend cmux's live-process scanner to detect claude/codex too, and surface those snapshots in the tab menu — so fork/resume works regardless of launch method (cmux wrapper,
sr, or direct binary).VaultAgentProcessScanner.processDetectedClaudeCodexSnapshots— identifies CMUX-scoped live claude/codex processes (positive binary/argv match mirroringliveProcessExecutableMatchesRecordedAgent;sr/shell-dispatcher wrappers excluded), and resolves a session id: explicit from argv, else the newest on-disk transcript/rollout — gated so an inferred id is only attributed when exactly one same-kind process shares the cwd, so an ambiguous cwd never forks the wrong conversation. Existing hook records still win via theload()merge.RestorableAgentSessionIndex.newestClaudeSessionId(forCwd:configDir:)— internal entry point reusing the existing transcript lookup (no duplication).CodexSessionResolver(new pure-logic file inCMUXAgentLaunch) — scans$CODEX_HOME/sessionsJSONL shards, matchessession_meta.cwd, returns the newest session id.SharedLiveAgentIndex— a separate, debouncedprocessDetectedIndex(30s TTL, off-main, off the hot hook-store reload path) thatforkableAgentSnapshotconsults as a last-resort fallback, so the tab menu shows Fork Conversation for hook-less agents without regressing the index reload.Tests
CodexSessionResolverTests(SPM, runnable locally — green:swift test --package-path Packages/CMUXAgentLaunch).cmuxTests/RestorableAgentSessionIndexTests:newestClaudeSessionId, hook-less claude/codex detection, the ambiguous-cwd guard, andsr/shell-wrapper rejection.Verification
Builds clean (
reload.sh --tag).swift testfor the codex resolver passes. The end-to-end "right-click → Fork Conversation" is most reliably confirmed by launching an agent in a tagged Debug build and right-clicking its tab.🤖 Generated with Claude Code
Summary by CodeRabbit
forkCommandtemplate.forkCommandfield and how it affects the Fork Conversation UI.