Repository navigation
agent-session: reliable tracking system + Codex picker GUI + debug trace - #6798
Conversation
…(Slice A) Foundation for the reliable agent-session tracking redesign (see docs/agent-session-tracking-spec.md). Makes the host the single source of truth and gives the client an authoritative pull path so a missed or out-of-order best-effort push self-heals. - ChatSessionDescriptor + AgentChatSessionRecord gain a monotonic `version`, stamped by AgentChatSessionRegistry on every write (one chokepoint, counter not hash, so strict monotonicity holds even when a change reverts a field). - New `mobile.chat.session` RPC: authoritative single-session snapshot pull for reconnect / foreground / version-gap / manual-refresh. - ChatSessionListReducer version-gates descriptor upserts: a lower-version push never clobbers newer state from a later push or a snapshot pull. Equal version passes through (counter guarantees equal == identical content; keeps unversioned payloads upserting as before). +1 unit test. - iOS MobileChatEventSource.session(sessionID:) pull primitive + response type. Verified: CmuxAgentChat builds + 128 tests pass; CmuxMobileShell builds; full macOS app builds (tag agentsot). No heuristics removed yet; no behavior removed. iOS pull-trigger wiring and the process-exit backstop are next. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes Slice A's client side. The list seed via `source.sessions(...)` is already an authoritative pull that re-runs on reconnect (the connection epoch in `chatRefreshKey`). Add a foreground epoch so returning from `.background` re-subscribes and re-pulls: pushes are best-effort and can be dropped while the app is suspended, so on foreground we re-read the host's authoritative list rather than trust that every push arrived. Transient `.inactive` (control center, a banner) does not churn the subscription; only real background does. Pairs with the version-gated reducer so a pull that races a late push converges. The single-session `mobile.chat.session` pull primitive remains available for finer-grained version-gap healing in the conversation view. Verified: CmuxMobileShellUI builds for iOS Simulator (BUILD SUCCEEDED). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the per-`sessions()` `kill(pid,0)` polling sweep with an event-driven `DispatchSourceProcess` (`.exit`) watcher per agent pid. cmux does not own a `Process` handle for a terminal agent (it is a child in the pty), so the deterministic exit signal is a process source on the pid cmux already knows from hook events / the store. On exit, the session flips to `.ended` on the main actor, but only if the exited pid is still the record's current pid, so a `claude --resume` under a new pid is never ended by its predecessor's exit. - `syncProcessExitWatch(for:)` reconciles the watcher with the record's pid at every store path (idempotent; cancels on pid change / clear / end). A pid already dead at registration ends the session on a fresh main-actor turn rather than waiting for an `.exit` that never comes. - `ended` stays retained: the GUI keeps showing the session and the input bar disables; only the watcher is torn down. - `sessions()` no longer sweeps on every read; the per-bound-session `kill(pid,0)` guard in `liveSession` stays as a cheap correctness backstop. A watcher unit test needs a real child process (timing-dependent, app test target only), so this is verified by build + dogfood rather than a flaky unit test. macOS app builds (tag agentsot). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… already exists Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the unreliable agent-session detection layer (terminal-TITLE matching and the newest-.jsonl-by-MTIME scan, plus their claim/forced-retry/provisional machinery) while preserving the reliable path: hook events, the hook-store as cmux-written persistence/seed, and transcript resolution keyed by the exact recorded path or session id. Dropping detection of agents that never fire a hook is intended. Deleted: - Sources/Mobile/AgentChat/AgentChatTranscriptService+TitleDetection.swift - cmuxTests/AgentChatTranscriptResolverTests.swift (only covered newestClaudeTranscript) - Resolver: newestClaudeTranscript, cwdCandidates, claudeTranscriptTitle(at:)/(in:), normalizedClaudeTitle + title-read constants - Service: adoptDetectedClaudeSession, private newestClaudeTranscript, observeAgentTitleChanges, ghosttyTitleSubscription, titleAdoptionHandler, all title-detection state vars + constants, provisional/title-key helpers, the PendingTitleChange/ClaudeTranscriptResolutionKey typealiases, the provisional branch in history(), and the clearTitleDetectionState call. start(adoptDetectedAgentSession:) -> start() (just seedFromHookStores). - Registry: claimedSessionIDs(), adoptDetectedSession() - TerminalController+MobileChat: adoptDetectedAgentSession(s) variants; v2MobileChatSessions now just lists registry sessions filtered by mobileChatBindingIsCurrentAgent. - TerminalController+MobileWorkspaceList: the adoptDetectedAgentSessions calls - AppDelegate: start() no-arg call site Kept (reliable): hook path, hook-store seed/refresh/adoptBindings, transcript resolution by recorded path + claudeFallbackPath/codexFallbackPath, encodeClaudeProjectDir, the GhosttyTitleChange(+Subscription) types (used for tab titles), and Slice A/B work. RestorableAgentSession.swift's newestClaudeTranscript is KEPT: it is the session-restore mechanism keyed by the recorded session id (workflow-container resolution), not the unreliable mobile-chat detection heuristic. Verified: CmuxAgentChat builds + 128 tests pass; macOS app Build complete (tag agentsotd); iOS CmuxMobileShellUI BUILD SUCCEEDED. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… rationale Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce C) Per the owner's directive "no jsonl parsing or heavy work on the main thread." After Slice D the only heavy main-actor parse left in this subsystem was the hook-store whole-file JSON read. Move all three read sites off-main: - seedFromHookStores is now async; the Data(contentsOf:)+JSONSerialization runs in a utility Task.detached, only the (cheap) record application touches main state. start() kicks it off and returns. - noteHookEvent no longer reads the store inline. When a binding is still missing (throttled to once per 30s/session) it returns immediately and defers an off-main backfill (backfillBindingsFromStore) that applies only still-nil fields via update() — so the live event stays authoritative and the hot tool- storm path never parses JSON on main. applyStoreBackfill no-ops when it learns nothing new, avoiding a spurious version bump / descriptor push. - refreshBindingsFromHookStore is async (off-main read); the send/interrupt/ answer + history RPC chain is threaded async to match. The transcript tailer already parses off its own actor; descriptor wire-encoding on main is small, not a whole-file parse. macOS app builds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y holds for terminal agents) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bal install Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… injection (Slice F)
Make Codex sessions track in the iOS GUI as reliably as Claude, without
installing anything into the user's ~/.codex and without clobbering their
existing notify/config.
cmux-codex-wrapper mirrors cmux-claude-wrapper: when inside a cmux terminal
(CMUX_SURFACE_ID + live socket) and a session entrypoint (bare codex, a
prompt, or codex exec/e), it execs the real codex with per-invocation hooks:
--enable hooks --dangerously-bypass-hook-trust -c 'hooks.SessionStart=[{hooks=[{type="command",command='''<gated>''',timeout=...}]}]' (and UserPromptSubmit/Stop/PreToolUse/PostToolUse/PermissionRequest)
The injected command is the exact gated shape cmux installs for persisted
codex hooks (resolve cmux CLI, require surface+socket+not-disabled, run
'cmux hooks codex <event>', else echo '{}'), carried as a TOML multi-line
literal string so its single quotes need no escaping. Verified empirically
against codex-cli 0.141.0: all hooks fire and codex passes session_id +
transcript_path on stdin, binding the transcript by real session id.
Belt-and-suspenders: the wrapper also fires a one-way 'cmux hooks codex
session-start' (surface/pid/cwd, empty stdin) BEFORE exec, so detection
happens at launch even if codex's own SessionStart is delayed; the registry
dedups by session id so the two reconcile.
Passthrough safety mirrors the claude wrapper exactly: every gate (opt-out
via CMUX_CODEX_HOOKS_DISABLED, outside cmux, dead socket, non-session
subcommand like resume/doctor/--help) and find_real_codex failure exec the
real codex unchanged, so installing the wrapper can never break codex.
A per-surface 'codex' PATH shim is written into the same cmux-cli-shims dir
as the claude shim (already on PATH), resolving+exec'ing the wrapper, else
stripping the shim dirs and exec'ing real codex. Bundled into the app
Resources/bin via the Copy CLI phase alongside cmux-claude-wrapper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed.push event (fix) A live codex/claude session record from chat.sessions.dump showed surface_id=None / transcript_path=None even though the hook store had both. The feed.push event carried workspace_id/cwd but no surface_id or transcript_path, and the .sessionStart store-backfill was suppressed and 30s-throttled, so a fresh (or short-lived `codex exec`) session stayed unbound until the next consult. Option A (timing-independent): carry the hook-resolved surface/transcript in the event itself. - WorkstreamEvent: add surfaceId (surface_id) and transcriptPath (transcript_path), mirroring workspaceId exactly (default nil, decodeIfPresent, encodeIfPresent, and via CodingKeys.allCases they stay in the knownKeys set). - AgentChatSessionRegistry.noteHookEvent: apply event.surfaceId and event.transcriptPath onto the record alongside workspaceId/cwd, so a live event binds immediately without waiting on the throttled store consult. - CLI sendFeedTelemetry: add surfaceId param and write surface_id + transcript_path (from parsedInput.transcriptPath) into the feed.push event. Thread the hook-RESOLVED target.surfaceId through sendAgentFeedTelemetry / sendAgentFeedTelemetryUnlessSuppressed at every agent-hook call site that has a resolved target in scope (session-start, prompt-submit, stop/notification, session-end via mapped.surfaceId). Verified live: a real `codex exec` session 019ef2cc-... appeared as a single non-fallback codex record with non-null surface_id and transcript_path in state idle, then transitioned to ended after the process exited. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… on cmux
The codex wrapper injected per-invocation [hooks] whose command called
`cmux hooks codex <sub>` SYNCHRONOUSLY. Codex runs hooks synchronously and
blocks until they return, so every launch hung ~35s on "Running SessionStart
hook" and every prompt lagged on UserPromptSubmit while the cmux call did
socket round-trips.
Reuse cmux's proven fire-and-forget shape (CMUXCLI.codexFireAndForget-
AgentHookShellCommand): capture codex's stdin payload to a temp file, nohup-
background the cmux call with a 30s watchdog, and `echo '{}'` back to codex
instantly. Detection still binds the real session_id/transcript_path because
the backgrounded call gets codex's real stdin.
Implementation: a hidden, socket-free `cmux hooks codex inject-args` emits the
exact codex arg list (NUL-terminated) to enable + inject the fire-and-forget
hooks for all six events (SessionStart, UserPromptSubmit, Stop, PreToolUse,
PostToolUse, PermissionRequest), each fire-and-forget command carried in a
TOML multi-line literal. The wrapper reads that stream into a bash array and
execs codex with it, replacing the hand-rolled TOML/quoting in bash. All
passthrough-safety gates (not in cmux / dead socket / hooks-disabled /
non-session subcommand / emit fails) still fall back to plain `exec codex`.
Two bugs found and fixed during live verification: the CLI emitted args
NUL-SEPARATED (dropped the final PermissionRequest arg at EOF) -> now
NUL-terminated; and the wrapper read via `raw="$(...)"` command substitution,
which bash strips NUL bytes from, collapsing the stream and silently dropping
the whole injection -> now reads the command directly via process
substitution. Verified live: hook returns {} in ~0.01s, the codex session
binds (surface_id + transcript_path) and goes ended after exit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ace, not stale stored workspace_id cmux workspace ids regenerate on every Mac relaunch while surface ids are stable and rehydrate verbatim, so a chat session created before the last relaunch carries a stale stored workspace_id and was dropped from its terminal's current workspace (no iOS chat toggle). Scope the workspace- filtered mobile.chat.sessions listing by the surface's CURRENT workspace: resolve the requested workspace, return every session whose surface is a live terminal panel there and that matches its agent against that workspace+panel, and re-stamp each returned record to the requested workspace so the seed and live descriptorChanged pushes both scope to it. Also exposes mobile.chat.sessions over the local control/debug socket for dogfood verification of this path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…to reopened live session Ended sessions were dropped from the workspace list because the is-current-agent check requires the terminal to be running the agent; that contradicts the retained-ended GUI and made the toggle go stale + vanish on tap after the agent exited. Now ended sessions are kept whenever their surface is a live terminal in the workspace (live sessions still require the agent match). iOS re-pins from an ended pinned session to a newer live session on the same terminal so reopening the agent makes the GUI editable again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…umed sessions keep hooks (editable in GUI) Mirror claude's wrapper-shim resume mechanism for codex. On Mac relaunch the restore launcher replayed a bare `codex resume <id>`, which resolved to the real codex binary inside the `$SHELL -lic` shell, bypassing cmux-codex-wrapper. No hooks fired, no SessionStart, the registry never marked the resumed session live, and the iOS GUI stayed read-only. - AgentResumeArgv: add codexWrapperShellExecutableToken (resolves CMUX_CODEX_WRAPPER_SHIM, degrades to bare codex) plus portable/render helpers, mirroring the claude token + /bin/sh -c wrapping for fish/csh. - TerminalSurfaceClaudeCommandShim: carry the sibling codex shim so the install result plumbs it forward. - TerminalSurface+RuntimeSurfaceCreation: export CMUX_CODEX_WRAPPER_SHIM (+_ROOT) into the managed env alongside the claude shim, so the restore launcher inherits it (previously only set in a sourced snippet). - SessionIndexModels + RestorableAgentSession (AgentResumeCommandBuilder): render the first bare codex token as the wrapper token and wrap in /bin/sh -c, exactly like claude. Full-path codex executables are unaffected. - SurfaceResumeCommandCanonicalizer: route a stale codex executable through the codex wrapper token too (generalized the claude path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
codex does NOT fire its own SessionStart hook when resuming a session, so a resumed codex never re-binds: the registry keeps the stale pre-relaunch record whose pid is already dead, the exit watcher flips it to .ended, and the iOS chat shows it read-only with no input bar (and the GUI can't recover, since you can't submit a prompt from a composer that isn't shown). The wrapper, unlike codex, knows the resumed session id (it is in argv) and the new live pid ($$), so it fires the session-start itself, fire-and-forget. The handler binds surface/workspace/cwd from the cmux env and pid from CMUX_CODEX_PID, re-binding the resumed session to its live pid and flipping it back to idle/editable. Also inject hooks on resume so subsequent turn events keep state accurate (codex does fire those on resume). Verified: resuming a session through the wrapper flips its store/registry pid from the dead original to the live process, with no phantom fallback-* record. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Detection of a resumed agent session was hook-driven: the GUI learned a session was live on a surface only when the agent fired a SessionStart hook. codex fires NO SessionStart on resume, and a subrouter/sr or absolute-path launch bypasses the cmux wrapper, so a resumed session kept its stale pre-relaunch record (dead pid -> exit watcher -> .ended) and showed read-only with no composer. Resume is ALWAYS cmux-initiated, so cmux already holds the (session, surface) pair at restore time. Record it directly instead of waiting for a hook the agent may never send: AgentChatSessionRegistry.noteResumeInitiated binds the surface, flips to .idle, and CLEARS the stale pid (re-arming a watcher on the dead pid would immediately re-end the session); the live pid backfills from the agent's own hooks when it has them. Wired from the session-restore path (Workspace.createPanel) for both the restorable-agent and agent-hook-binding restores. Buffered through a static entry point + flush in start(), because restore can run before the service is wired (a direct call would be a silent no-op). Verified on device: all 9 restored codex sessions fire the re-bind and become .idle/editable on relaunch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… match, resume re-key) Four correctness fixes from an adversarial review of the iOS coding-agent GUI across Claude + Codex, so the Telegram<->GUI flow (toggle appears, message sends, response live) holds in more cases per the spec. - List reducer ignores the unversioned `stateChanged`: every transition also emits a versioned `descriptorChanged` carrying the same state, so the list is driven solely by the version-gated descriptor path; a reordered/duplicated bare `stateChanged` can no longer regress newer state. The focused conversation's store still consumes `stateChanged` directly. +2 reducer tests. - mobileChatRecordMatchesAgent is now deterministic (spec principle 2): the live send/list gate uses process liveness (kill(pid,0)) instead of terminal-title / screen-scraped agent detection, which could hide a correctly-bound live session. When the pid is unknown (a session re-bound on resume from cmux's own authority, e.g. `sr codex resume` that bypasses the hook shim), trust the durable surface binding rather than invent a negative. - Resume re-bind is keyed on the real `terminalPanel.id` and recorded after the surface is created, fixing the surface-id-collision case (restore-into-live / duplicate-workspace) where a fresh id was minted and the old key bound nothing. - Resume re-bind no longer gated on cmux generating the resume launch, so an auto-resume-off user who resumes manually (`sr codex resume`) gets an editable GUI (.idle) instead of a stuck read-only (.ended) record. Recording .idle is the safe direction per spec (never invent ended). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old "isn't readable on the Mac yet. Send the agent a prompt, then retry." was misleading when the agent runs under a git-rooted home directory: Claude Code does not persist a project transcript when the session's git root is $HOME, so retrying never produces a transcript. New copy covers both the just-started timing case (send a prompt + Retry) and the structural case (home directory keeps no transcript -> use the Terminal tab). en + ja updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lback The transcript resolver's derived-path fallback hardcoded ~/.claude and ~/.codex, so a user who relocates their agent config dir (CLAUDE_CONFIG_DIR for Claude, CODEX_HOME for Codex, e.g. via a launcher/subrouter) would have fallback-resolved transcripts (notably codex resumed sessions, resolved by scanning the sessions dir) come up empty even though the files exist. Resolve the config-dir root from the env override (expanding a leading ~), defaulting to ~/.claude / ~/.codex. The PRIMARY source is unchanged: the hook-recorded absolute transcriptPath already encodes any custom dir; this only hardens the fallback used when no path was recorded. environment is injectable for tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A session's liveness was judged from a single recorded pid. With any launcher indirection (a subrouter like `sr`, a `node` shim), that pid is the launcher, not the agent (the real codex/claude binary is deeper in the process tree). So when the launcher or an intermediate exited, cmux wrongly marked a live session `.ended` (GUI shows no input bar). Now, before ending, verify against the surface's process tree off-main: if a real agent process matching the session's kind still exists anywhere under the surface, re-bind the record's pid to it (re-arming the exit watcher on the real agent) instead of ending. Only end when no agent remains in the tree. The synchronous dead-pid check in liveSession() defers to the same tree-aware path and keeps showing the session meanwhile (never hides a live agent). Reuses the existing CmuxTopProcessSnapshot + CmuxTaskManagerCodingAgentDefinition classifier; the tree walk runs off-main only at the rare exit-decision moment, never on the typing path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex hook injection was always-on (gated only by the CMUX_CODEX_HOOKS_DISABLED env opt-out, no UI). Add a first-class "Codex Integration" toggle in Automation settings, mirroring "Claude Code Integration": - New catalog key integrations.codex.hooksEnabled (default true), threaded through AgentIntegrationSettingsReading/Store and TerminalSurfaceSpawnPolicy. - When off, the spawn path exports CMUX_CODEX_HOOKS_DISABLED=1; the codex wrapper already no-ops on that env (shim stays on PATH, harmless), so resumed codex still routes through the shim but injects no hooks. - Settings UI codexCard + en/ja strings. The note states cmux still tracks live Codex sessions it can observe even when the toggle is off (the observe floor), so disabling it never blinds the GUI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The wrapper injected each codex hook as an inline shell-snippet `command`
string. Normal codex runs that through a shell, but some codex-compatible
runtimes (subrouters/proxies) exec the `command` string directly as a program,
so the snippet failed with "No such file or directory (os error 2)" and the
session was shown inline as a failed hook (and could lose state tracking).
emitCodexWrapperInjectArgs now writes each event's body to a #!/bin/sh script in
a cmux-owned dir (~/.cmux/hooks, NOT the user's ~/.codex), idempotently +
executable, and emits the bare script PATH as the hook command. A file path
execs correctly whether the runtime runs it directly or via a shell, so normal
codex is unaffected and subrouter runtimes stop erroring. Any write failure
falls back to the inline snippet, so the working path can never regress.
Verified: emitted SessionStart command is now the script path, and direct-exec
of the script (the os-error-2 path) returns `{}` exit 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ree) Slice 2 of the reliable-tracking system: discover live codex/claude sessions by observing the process table, with no dependency on hooks firing, so a session launched through any indirection (a subrouter, a wrapper) that fired no hook is still found and bound. On the iOS list pull, a throttled off-main scan walks every cmux-scoped process, matches the real agent binary via the existing coding-agent classifier (deep in an sr -> node -> codex tree the codex binary still matches by basename), and resolves identity without hooks: codex via the rollout .jsonl it holds open (new libproc PROC_PIDLISTFDS/PROC_PIDFDVNODEPATHINFO reader, which also yields the transcript path), claude via --session-id/--resume in argv. Untracked sessions get an .idle presence record that pushes itself to subscribers via onRecordChanged; existing records only get missing bindings backfilled, never a state downgrade. Fire-and-forget so it never blocks the list pull. No config touched, no consent needed (pure observation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slice 4: the visible, consented, never-silent global-install option. The Codex Integration card now states that to also track Codex launched through a custom launcher that bypasses the wrapper (e.g. a subrouter), the user runs `cmux hooks setup --agent codex`, which installs hooks into ~/.codex/hooks.json (stating exactly what is written, where). This matches cmux's established consent pattern for amp/cursor/gemini global hooks, and pairs with the observe floor (slice 2): a user who installs nothing still gets presence/liveness/ transcript tracking; the global install only adds richer hook state on wrapper-bypassing launchers. en/ja updated. A one-click installer button over the existing `cmux hooks setup` CLI is a follow-up refinement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the agent-session subsystem debuggable end to end. DEBUG-gated cmuxDebugLog
lines with a consistent `agentChat.*` prefix at every decision point, so one
`grep 'agentChat\.' /tmp/cmux-debug-<tag>.log` shows the whole flow when a bug
like a missing question/transcript happens:
- agentChat.hook — every hook event ingested (event name, tool name incl.
AskUserQuestion, has-toolInput, surface, has-transcript).
- agentChat.detect — observe-floor process-tree detections (session, kind,
surface, pid, id resolved via fd vs argv, new/bind).
- agentChat.state — every state transition at the single update() chokepoint
(idle/working/needsInput/ended, version).
- agentChat.transcript.resolve — transcript path resolution (file or UNRESOLVED
with kind+cwd, so home-dir / config-dir misses are obvious).
- agentChat.transcript.batch — each tail batch (appended/updated/reset/title
counts), so "did transcript content actually stream" is
visible.
All DEBUG-only and off the typing path. Covers detection, tool use, and
transcript stuff in one greppable trace.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uestions Interactive pickers in the GUI were Claude-only: ClaudeTranscriptParser turns an AskUserQuestion tool into a tappable .question node, but CodexTranscriptParser produced none, so a Codex picker streamed into the GUI as plain text with no way to select. Codex writes its picker as a `request_user_input` function_call whose arguments carry `questions[]` in the exact same shape as Claude's AskUserQuestion (question + options[].label/description). Parse it into the same ChatQuestion node, one tappable question per entry. And make mobile.chat.answer agent-aware: Claude submits on the digit alone, Codex's picker needs Enter, so append a carriage return for codex. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p tapping)
Codex pickers rendered as tappable questions but never resolved, so they stayed
interactive forever (even past, answered ones) and never showed the chosen
option. Codex pairs the answer to its request_user_input call via a
function_call_output whose JSON is {"answers":{"<id>":{"answers":["<label>"]}}}.
Register the parsed question under its call id (pendingKey) so the existing
resolve path pairs the output, and teach the shared answer extractor codex's
JSON format (single-question picker -> first selected label). The question then
becomes an answered ChatQuestion with selectedOptionLabel, which the GUI renders
as the chosen selection, non-interactive.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| } | ||
| } | ||
| }, | ||
| "settings.automation.codex": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Codex Integration" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Codex連携" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.automation.codex.subtitleOn": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Sidebar shows Codex session status and notifications." | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "サイドバーにCodexセッションの状態と通知が表示されます。" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.automation.codex.subtitleOff": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Codex runs without cmux integration." | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Codexはcmux連携なしで実行されます。" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.automation.codex.note": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "When enabled, cmux wraps the codex command to inject session tracking and notification hooks. Disable if you prefer to manage Codex hooks yourself. cmux still tracks live Codex sessions it can observe even when this is off. To also track Codex launched through a custom launcher that bypasses the wrapper (e.g. a subrouter), run `cmux hooks setup --agent codex`, which installs hooks into ~/.codex/hooks.json." | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "有効にすると、cmuxはcodexコマンドをラップしてセッション追跡と通知フックを注入します。Codexのフックを自分で管理したい場合は無効にしてください。無効でも、cmuxは観測できるライブCodexセッションを追跡します。ラッパーをバイパスするカスタムランチャー(例: サブルーター)経由で起動したCodexも追跡するには、`cmux hooks setup --agent codex` を実行してください。これは ~/.codex/hooks.json にフックをインストールします。" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.automation.claudeCode": { | ||
| "extractionState": "manual", | ||
| "localizations": { |
There was a problem hiding this comment.
New Codex settings strings missing translations for 17+ locales
The four new settings.automation.codex* strings (settings.automation.codex, settings.automation.codex.subtitleOn, settings.automation.codex.subtitleOff, settings.automation.codex.note) only ship with en and ja entries. The sibling settings.automation.claudeCode strings they mirror carry translations for en, ja, zh-Hans, zh-Hant, ko, de, es, fr, it, da, pl, ru, bs, ar, nb, pt-BR, th, tr, and uk. Every user running cmux in one of those 17 locales will see raw English in the new Codex Integration settings card.
Rule Used: Flag production user-facing text that is not fully... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| /// Watches terminal title changes so a coding agent launched without a | ||
| /// hook (e.g. via a shell wrapper that bypasses cmux's hook injection) is | ||
| /// adopted the instant its terminal title becomes the agent's (e.g. | ||
| /// "✳ Claude Code"), not only when the workspace is next opened. Adoption | ||
| /// emits a descriptor change, which pushes the toggle to listening phones. | ||
| private func observeAgentTitleChanges() { | ||
| ghosttyTitleSubscription = GhosttyTitleChangeSubscription { [weak self] change in | ||
| self?.scheduleTitleDetectedAdoption(change) | ||
| /// Resume re-binds recorded before ``start()`` wired the live instance. | ||
| private static var pendingResumeIntents: [PendingResumeIntent] = [] | ||
| /// The started service, used to apply resume re-binds immediately once live. | ||
| private static weak var liveInstance: AgentChatTranscriptService? | ||
|
|
||
| /// Records, from cmux's own authority, that it is resuming `sessionID` onto | ||
| /// `surfaceID` (see | ||
| /// ``AgentChatSessionRegistry/noteResumeInitiated(sessionID:source:surfaceID:workspaceID:workingDirectory:)``). | ||
| /// Static so the restore path need not hold a service reference: before the | ||
| /// service starts (restore can run first) the intent is buffered and flushed | ||
| /// in ``start()``; after, it applies immediately. | ||
| static func recordResumeIntent( | ||
| sessionID: String, | ||
| source: String, | ||
| surfaceID: String?, | ||
| workspaceID: String?, | ||
| workingDirectory: String? | ||
| ) { | ||
| if let live = liveInstance { | ||
| live.noteResumeInitiated( | ||
| sessionID: sessionID, | ||
| source: source, | ||
| surfaceID: surfaceID, | ||
| workspaceID: workspaceID, | ||
| workingDirectory: workingDirectory | ||
| ) | ||
| } else { | ||
| pendingResumeIntents.append(PendingResumeIntent( | ||
| sessionID: sessionID, | ||
| source: source, | ||
| surfaceID: surfaceID, | ||
| workspaceID: workspaceID, | ||
| workingDirectory: workingDirectory | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| /// Seeds the session registry from the on-disk hook stores. Call once | ||
| /// at app startup. Sessions are tracked only via the reliable hook-event | ||
| /// path thereafter; cmux does not detect agents that never fire a hook. | ||
| func start() { | ||
| Self.liveInstance = self | ||
| // Apply resume re-binds buffered before the service was wired. The seed | ||
| // only creates records that don't already exist, so an intent applied | ||
| // here is preserved (the seed skips it) and one applied after flips the |
There was a problem hiding this comment.
Static mutable state missing actor isolation annotation
pendingResumeIntents and liveInstance are static stored properties on a non-@MainActor class, yet every caller (recordResumeIntent from Workspace.swift, start() from the app delegate) runs on the main actor. Without an explicit @MainActor annotation on these statics, Swift 6 strict concurrency cannot verify the isolation is safe — a future caller from a background context would be a silent data race. Marking the statics and the static method @MainActor fixes this and makes the invariant compiler-enforced rather than convention-enforced.
Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
| } | ||
| } | ||
|
|
||
| /// Seeds the session registry from the on-disk hook stores. Call once | ||
| /// at app startup. | ||
| /// | ||
| /// - Parameter adoptDetectedAgentSession: Composition-root callback that | ||
| /// adopts a title-detected agent for the surface whose title changed, | ||
| /// returning whether the surface was resolved and adoption was queued. | ||
| func start(adoptDetectedAgentSession: @escaping @MainActor (GhosttyTitleChange) -> Bool) { | ||
| guard ghosttyTitleSubscription == nil else { return } | ||
| titleAdoptionHandler = adoptDetectedAgentSession | ||
| registry.seedFromHookStores() | ||
| observeAgentTitleChanges() | ||
| /// A `(session, surface)` resume re-bind cmux authored during session | ||
| /// restore, buffered until the service is live (restore can run before app | ||
| /// setup assigns this service, so a direct call would be a silent no-op). | ||
| private struct PendingResumeIntent { | ||
| let sessionID: String | ||
| let source: String | ||
| let surfaceID: String? | ||
| let workspaceID: String? | ||
| let workingDirectory: String? | ||
| } | ||
|
|
||
| /// Watches terminal title changes so a coding agent launched without a | ||
| /// hook (e.g. via a shell wrapper that bypasses cmux's hook injection) is | ||
| /// adopted the instant its terminal title becomes the agent's (e.g. | ||
| /// "✳ Claude Code"), not only when the workspace is next opened. Adoption | ||
| /// emits a descriptor change, which pushes the toggle to listening phones. | ||
| private func observeAgentTitleChanges() { | ||
| ghosttyTitleSubscription = GhosttyTitleChangeSubscription { [weak self] change in | ||
| self?.scheduleTitleDetectedAdoption(change) | ||
| /// Resume re-binds recorded before ``start()`` wired the live instance. |
There was a problem hiding this comment.
Implicit singleton side-channel for initialization ordering
The liveInstance / pendingResumeIntents pattern is a static side-channel that routes session restore events through a global buffer instead of a direct call graph. The ordering invariant ("restore fires before start(); start() drains the buffer") is correct today but nowhere enforced structurally. Consider making the restore path pass its intents into start(intents:) as parameters, eliminating both static vars entirely.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/TerminalController+MobileChat.swift (1)
365-369: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate refreshed bindings with the refreshed workspace ID.
After
refreshSessionBindings, this still resolves with the staleworkspaceIDcaptured from the old record. If the refresh repaired the workspace binding, the retry is discarded and mobile send/interrupt/answer still fail.Proposed fix
if let refreshed = await service.refreshSessionBindings(sessionID: sessionID), - let surfaceID = refreshed.surfaceID, - mobileChatBindingResolves(workspaceID: workspaceID, surfaceID: surfaceID), - mobileChatBindingIsCurrentAgent(refreshed) { - return ["workspace_id": workspaceID, "surface_id": surfaceID] + let surfaceID = refreshed.surfaceID { + let refreshedWorkspaceID = refreshed.workspaceID ?? workspaceID + if mobileChatBindingResolves(workspaceID: refreshedWorkspaceID, surfaceID: surfaceID), + mobileChatBindingIsCurrentAgent(refreshed) { + return ["workspace_id": refreshedWorkspaceID, "surface_id": surfaceID] + } }🤖 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/TerminalController`+MobileChat.swift around lines 365 - 369, The refreshed binding check in the mobile chat flow is still using the old workspace value, so the retry can be rejected even after a successful repair. Update the logic in TerminalController+MobileChat’s session refresh path to use the workspace ID from the refreshed binding returned by refreshSessionBindings, and pass that refreshed workspace value into mobileChatBindingResolves alongside refreshed.surfaceID and mobileChatBindingIsCurrentAgent.Sources/SessionIndexModels.swift (1)
340-364: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winQuote
sessionIdbefore passing the command to/bin/sh -c.Line 351 embeds
sessionIddirectly into the inner shell command. If the Codex session id ever contains shell metacharacters, the wrapped/bin/sh -cwill interpret them; quote it like the other dynamic Codex arguments.Proposed fix
- var parts = ["\(AgentResumeArgv.codexWrapperShellExecutableToken) resume \(sessionId)"] + var parts = ["\(AgentResumeArgv.codexWrapperShellExecutableToken) resume \(Self.shellQuote(sessionId))"]🤖 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/SessionIndexModels.swift` around lines 340 - 364, The Codex resume command built in SessionIndexModels.swift embeds sessionId directly into the inner /bin/sh -c command, which can be interpreted as shell syntax if it contains metacharacters. Update the resume command assembly in the codex wrapper path so sessionId is quoted the same way as the other dynamic Codex arguments (for example alongside model and effort) before it is concatenated into parts and passed through AgentResumeArgv.portableCodexResumeShellCommand.
🤖 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 `@CLI/CMUXCLI`+CodexFireAndForgetHooks.swift:
- Around line 10-17: The wrapper currently routes all entries in
codexWrapperInjectionEvents through codexFireAndForgetAgentHookShellCommand,
which breaks the blocking behavior for PreToolUse and PermissionRequest. Update
CMUXCLI+CodexFireAndForgetHooks so those two events do not use the
fire-and-forget wrapper, either by removing them from
codexWrapperInjectionEvents or by adding a blocking path in the wrapper that
matches codexHookCanRunFireAndForget’s behavior. Keep the existing native
injection behavior aligned with the persistent path for those event types.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 228-231: The chat refresh flow still restarts when the app
backgrounds because `chatRefreshKey` changes and `refreshChatSessions()` has no
early exit. Add a guard at the start of `refreshChatSessions()` in
`WorkspaceDetailView` to return immediately when `scenePhase` is `.background`,
before calling `store.makeChatEventSource()` or starting the stream. Keep the
existing fallback logic for the non-background path so the task only runs while
the app is active.
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatSessionDescriptor.swift`:
- Around line 41-46: The synthesized Equatable for ChatSessionDescriptor now
includes version, so descriptor diffs can fire on activity-only registry writes.
Update AgentChatTranscriptService.descriptorChangedMeaningfully to normalize or
ignore version alongside lastActivityAt, and compare a sanitized descriptor so
pure hook/pre/post-tool activity bumps do not trigger descriptorChanged; use
ChatSessionDescriptor and descriptorChangedMeaningfully as the main touchpoints.
In
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/CodexTranscriptParser.swift`:
- Around line 215-232: All questions in a multi-question card are not being
registered under the same pending key, so only the first one gets resolved.
Update CodexTranscriptParser’s question appending loop so every ChatMessage
created in the enumerated questions block uses pendingKey: callID, while keeping
the existing id scheme and seq/timestamp/role/kind behavior. This ensures
TranscriptBatchAssembler.resolve can match the single function_call_output
against all related questions and clear them together.
In `@Resources/bin/cmux-codex-wrapper`:
- Line 307: The resume payload built in cmux_codex_resume_payload is
hand-assembling JSON and interpolates $PWD raw, so quotes or backslashes can
corrupt the payload. Update the session resume path in cmux-codex-wrapper to
serialize the payload through a JSON-safe encoder or the cmux CLI instead of
string concatenation, keeping cmux_codex_resume_sid and cwd as properly escaped
fields. Ensure the generated payload remains valid JSON for paths containing "
or \ so the session-start hook can parse it reliably.
In `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Around line 276-277: The rollout path detection in AgentChatSessionRegistry
should not hard-code the “.codex” root, because it misses sessions created under
a custom CODEX_HOME. Update openCodexRolloutPath to पहचान the rollout file by
its sessions-directory shape and .jsonl suffix, or resolve the root from process
environment/config instead of checking for "/.codex/sessions/". Keep the
existing path matching logic in that method but make it root-agnostic so custom
session roots are included.
- Around line 85-92: The early return in syncProcessExitWatch(for:) is leaving
an existing exit watcher active when the same session transitions to .ended, so
adjust the watcher synchronization logic to always cancel and clear any existing
watcher before checking the new record state. Keep the existing pid/sessionID
reuse guard for still-active sessions, but ensure that when record.state becomes
.ended the exitWatchers entry is removed and its source is cancelled rather than
returning early.
- Around line 197-203: The observed-session path in
AgentChatSessionRegistry.update(sessionID:session:) is still calling update and
bumping the version even when rec.surfaceID, rec.workspaceID,
rec.transcriptPath, and rec.pid are already set. Add a guard in that branch so
the closure only runs when the incoming session can fill at least one missing
binding, and skip the update entirely otherwise to avoid unnecessary descriptor
pushes.
In `@Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swift`:
- Around line 30-42: The config-root fallback in AgentChatTranscriptResolver
currently uses the app process environment, so it misses
agent-launcher/subrouter overrides for CLAUDE_CONFIG_DIR and CODEX_HOME. Update
the transcript resolution flow to use the per-session agent environment or
observed config root when resolving missing transcriptPath values, by threading
that information through AgentChatTranscriptResolver’s init/configRoot path or
by recording the resolved transcript path during hook/process detection.
In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift`:
- Around line 272-276: The DEBUG trace in AgentChatTranscriptService’s
transcript resolution path is logging the raw working directory via the
cmuxDebugLog call, which can expose sensitive path details. Update the log in
this unresolved-session branch to avoid printing record.workingDirectory
directly and instead emit only a presence indicator or a redacted placeholder.
Keep the existing sessionID and agentKind context, but ensure the cwd portion is
sanitized while preserving usefulness for debugging.
In `@Sources/TerminalController`+MobileChat.swift:
- Around line 153-160: The error response in TerminalController+MobileChat’s
session-not-found path is leaking the session identifier by including it in the
err payload’s data. Remove the session_id entry from the .err construction in
this MobileChat terminal handler and keep only the user-facing message/code so
the caller does not receive session IDs in API error bodies.
- Around line 219-224: Resolve the terminal binding a single time in the send
flow instead of calling both mobileChatTerminalParams(sessionID:) and
mobileChatTerminalPanel(sessionID:) separately. In
TerminalController+MobileChat.swift, update the send path to reuse one resolved
terminal target (or a shared lookup helper) so attachments, separators, and text
all go to the same panel/params instance even if the registry changes between
awaits. Keep the existing not_found error handling, but base it on that one
resolution.
---
Outside diff comments:
In `@Sources/SessionIndexModels.swift`:
- Around line 340-364: The Codex resume command built in
SessionIndexModels.swift embeds sessionId directly into the inner /bin/sh -c
command, which can be interpreted as shell syntax if it contains metacharacters.
Update the resume command assembly in the codex wrapper path so sessionId is
quoted the same way as the other dynamic Codex arguments (for example alongside
model and effort) before it is concatenated into parts and passed through
AgentResumeArgv.portableCodexResumeShellCommand.
In `@Sources/TerminalController`+MobileChat.swift:
- Around line 365-369: The refreshed binding check in the mobile chat flow is
still using the old workspace value, so the retry can be rejected even after a
successful repair. Update the logic in TerminalController+MobileChat’s session
refresh path to use the workspace ID from the refreshed binding returned by
refreshSessionBindings, and pass that refreshed workspace value into
mobileChatBindingResolves alongside refreshed.surfaceID and
mobileChatBindingIsCurrentAgent.
🪄 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: 826345d6-03e3-4ff9-8c75-d7e6f76c0c9a
📒 Files selected for processing (49)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/CMUXCLI+CodexFireAndForgetHooks.swiftCLI/cmux.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatQuestion.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatSessionDescriptor.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/CodexTranscriptParser.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/TranscriptToolCompletion.swiftPackages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatSessionListReducer.swiftPackages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatSessionListReducerTests.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatSessionResponse.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentResumeArgv.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamEvent.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentResumeArgvTests.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/MobileHost/ControlCommandCoordinator+MobileHost.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/MobileHost/ControlMobileHostContext.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/IntegrationsCatalogSection.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/AgentIntegrationSettingsReading.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Stores/AgentIntegrationSettingsStore.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AutomationSection.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+StartupEnvironment.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurfaceSpawnPolicy.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeSurfaceCreation.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/SurfaceValues/TerminalSurfaceClaudeCommandShim.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/SurfaceValues/TerminalSurfaceCodexCommandShim.swiftResources/Localizable.xcstringsResources/bin/cmux-codex-wrapperSources/AppDelegate.swiftSources/Mobile/AgentChat/AgentChatSessionRecord.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/AgentChat/AgentChatTranscriptResolver.swiftSources/Mobile/AgentChat/AgentChatTranscriptService+TitleDetection.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swiftSources/RestorableAgentSession.swiftSources/SessionIndexModels.swiftSources/SurfaceResumeCommandCanonicalizer+PortableAgentExecutable.swiftSources/TerminalController+ControlMobileHostContext.swiftSources/TerminalController+MobileChat.swiftSources/TerminalController+MobileWorkspaceList.swiftSources/TerminalSurfaceRuntimeWiring.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AgentChatTranscriptResolverTests.swiftcmuxTests/RestorableAgentSessionIndexTests.swiftcmuxTests/SessionIndexViewTests.swiftdocs/agent-session-tracking-spec.mddocs/codex-agent-detection-plan.md
💤 Files with no reviewable changes (3)
- Sources/Mobile/AgentChat/AgentChatTranscriptService+TitleDetection.swift
- Sources/TerminalController+MobileWorkspaceList.swift
- cmuxTests/AgentChatTranscriptResolverTests.swift
| private var chatRefreshKey: String { | ||
| "\(workspace.id.rawValue)#\(store.connectionState == .connected ? 1 : 0)" | ||
| let connected = store.connectionState == .connected ? 1 : 0 | ||
| let foreground = scenePhase == .background ? 0 : 1 | ||
| return "\(workspace.id.rawValue)#\(connected)#\(foreground)" |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the task keyed by chatRefreshKey calls refreshChatSessions without a scenePhase guard.
rg -n -C4 'task\s*\(id:\s*chatRefreshKey|refreshChatSessions\(\)|chatRefreshKey|scenePhase' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftRepository: manaflow-ai/cmux
Length of output: 3048
🏁 Script executed:
sed -n '244,265p' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftRepository: manaflow-ai/cmux
Length of output: 1207
Do not restart the chat stream while backgrounded
The chatRefreshKey sets foreground = 0 when scenePhase == .background (lines 228‑231). Because .task(id:) cancels the old task and immediately spawns a new one when the key changes, the transition to background currently launches refreshChatSessions() again. The function lacks a guard, so it will attempt to establish the event source and stream while suspended, contradicting the comment that the stream should "tear down" in the background.
Add an explicit guard at the entry of refreshChatSessions() to exit early if the app is in the background:
private func refreshChatSessions() async {
guard scenePhase != .background else { return }
guard let source = store.makeChatEventSource() else {
chatSessions = []
applyChatModeFallback()
return
}
// ... rest of the function🤖 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/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`
around lines 228 - 231, The chat refresh flow still restarts when the app
backgrounds because `chatRefreshKey` changes and `refreshChatSessions()` has no
early exit. Add a guard at the start of `refreshChatSessions()` in
`WorkspaceDetailView` to return immediately when `scenePhase` is `.background`,
before calling `store.makeChatEventSource()` or starting the stream. Keep the
existing fallback logic for the non-background path so the task only runs while
the app is active.
| /// Monotonic per-session revision, bumped by the host on every change to | ||
| /// this session. The client reconciles best-effort pushes against | ||
| /// authoritative pulls by this number: apply a push only when its version | ||
| /// is strictly greater than the last applied, and replace wholesale from a | ||
| /// snapshot pull. A missed or duplicated push self-heals on the next pull. | ||
| public var version: Int = 0 |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Don't let version force activity-only descriptor pushes.
version now participates in synthesized Equatable, but AgentChatTranscriptService.descriptorChangedMeaningfully only normalizes lastActivityAt. Since registry writes stamp new versions for hook activity, pre/post-tool activity can emit descriptorChanged despite the “pure activity bumps” guard.
Proposed fix
private static func descriptorChangedMeaningfully(
previous: AgentChatSessionRecord?,
current: AgentChatSessionRecord
) -> Bool {
guard var normalizedPrevious = previous else { return true }
normalizedPrevious.lastActivityAt = current.lastActivityAt
+ normalizedPrevious.version = current.version
return normalizedPrevious.descriptor != current.descriptor
}📝 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.
| /// Monotonic per-session revision, bumped by the host on every change to | |
| /// this session. The client reconciles best-effort pushes against | |
| /// authoritative pulls by this number: apply a push only when its version | |
| /// is strictly greater than the last applied, and replace wholesale from a | |
| /// snapshot pull. A missed or duplicated push self-heals on the next pull. | |
| public var version: Int = 0 | |
| private static func descriptorChangedMeaningfully( | |
| previous: AgentChatSessionRecord?, | |
| current: AgentChatSessionRecord | |
| ) -> Bool { | |
| guard var normalizedPrevious = previous else { return true } | |
| normalizedPrevious.lastActivityAt = current.lastActivityAt | |
| normalizedPrevious.version = current.version | |
| return normalizedPrevious.descriptor != current.descriptor | |
| } |
🤖 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/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Model/ChatSessionDescriptor.swift`
around lines 41 - 46, The synthesized Equatable for ChatSessionDescriptor now
includes version, so descriptor diffs can fire on activity-only registry writes.
Update AgentChatTranscriptService.descriptorChangedMeaningfully to normalize or
ignore version alongside lastActivityAt, and compare a sanitized descriptor so
pure hook/pre/post-tool activity bumps do not trigger descriptorChanged; use
ChatSessionDescriptor and descriptorChangedMeaningfully as the main touchpoints.
| for (index, question) in questions.enumerated() { | ||
| let baseID = callID ?? "line-\(seq)" | ||
| assembler.append( | ||
| ChatMessage( | ||
| id: index == 0 ? baseID : "\(baseID)-q\(index)", | ||
| seq: seq, | ||
| role: .agent, | ||
| timestamp: timestamp, | ||
| kind: .question(question) | ||
| ), | ||
| // Pair with the request_user_input function_call_output by | ||
| // call id so the answer marks the question resolved (the | ||
| // GUI then shows the selection and stops being tappable). | ||
| pendingKey: index == 0 ? callID : nil | ||
| ) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how the assembler registers pendingKey messages and resolves them,
# to confirm a single call_id output resolves at most one message.
fd -t f 'TranscriptBatchAssembler.swift' --exec cat -n {}Repository: manaflow-ai/cmux
Length of output: 5202
All multi-question cards must register the same pendingKey.
The TranscriptBatchAssembler.resolve method correctly iterates over all messages grouped under a pendingKey, but the parser only registers the first question (index == 0) with pendingKey: callID. Subsequent questions (index > 0) are appended with pendingKey: nil, so they are never added to the pending dictionary and remain unresolved even after the user answers.
Register every question under the same pendingKey: callID so the single function_call_output resolves all cards:
for (index, question) in questions.enumerated() {
let baseID = callID ?? "line-\(seq)"
assembler.append(
ChatMessage(
id: index == 0 ? baseID : "\(baseID)-q\(index)",
seq: seq,
role: .agent,
timestamp: timestamp,
kind: .question(question)
),
// Pair ALL questions with the same call id so the output resolves every card.
pendingKey: callID
)
}🤖 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/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Parsing/CodexTranscriptParser.swift`
around lines 215 - 232, All questions in a multi-question card are not being
registered under the same pending key, so only the first one gets resolved.
Update CodexTranscriptParser’s question appending loop so every ChatMessage
created in the enumerated questions block uses pendingKey: callID, while keeping
the existing id scheme and seq/timestamp/role/kind behavior. This ensures
TranscriptBatchAssembler.resolve can match the single function_call_output
against all related questions and clear them together.
| cmux_codex_resume_sid="$(cmux_codex_resume_session_id "$@")" | ||
| if [[ -n "$cmux_codex_resume_sid" \ | ||
| && -n "$CMUX_CODEX_HOOK_CMUX_BIN" && -x "$CMUX_CODEX_HOOK_CMUX_BIN" ]]; then | ||
| cmux_codex_resume_payload="{\"session_id\":\"$cmux_codex_resume_sid\",\"cwd\":\"$PWD\"}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Resume payload JSON is not escaped — a "/\ in $PWD yields malformed JSON.
cmux_codex_resume_sid is UUID-validated, but $PWD is interpolated raw. On macOS, directory names may legally contain " and \, which break the hand-built JSON. The session-start hook then fails to parse and the resumed session never re-binds to its live pid — i.e. it stays .ended/read-only, the exact failure this change targets. Consider serializing via the cmux CLI or a JSON-safe encoder rather than string interpolation.
🤖 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 `@Resources/bin/cmux-codex-wrapper` at line 307, The resume payload built in
cmux_codex_resume_payload is hand-assembling JSON and interpolates $PWD raw, so
quotes or backslashes can corrupt the payload. Update the session resume path in
cmux-codex-wrapper to serialize the payload through a JSON-safe encoder or the
cmux CLI instead of string concatenation, keeping cmux_codex_resume_sid and cwd
as properly escaped fields. Ensure the generated payload remains valid JSON for
paths containing " or \ so the session-start hook can parse it reliably.
| if path.hasSuffix(".jsonl"), path.contains("/.codex/sessions/") { | ||
| return path |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't hard-code .codex for rollout detection.
openCodexRolloutPath misses Codex sessions using CODEX_HOME because custom roots won’t contain /.codex/sessions/. Match the rollout file shape under a sessions directory, or use the process environment/config root.
Proposed fix
- if path.hasSuffix(".jsonl"), path.contains("/.codex/sessions/") {
+ let fileName = (path as NSString).lastPathComponent
+ if fileName.hasPrefix("rollout-"),
+ fileName.hasSuffix(".jsonl"),
+ path.contains("/sessions/") {
return path
}📝 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.
| if path.hasSuffix(".jsonl"), path.contains("/.codex/sessions/") { | |
| return path | |
| let fileName = (path as NSString).lastPathComponent | |
| if fileName.hasPrefix("rollout-"), | |
| fileName.hasSuffix(".jsonl"), | |
| path.contains("/sessions/") { | |
| return path | |
| } |
🤖 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 276 -
277, The rollout path detection in AgentChatSessionRegistry should not hard-code
the “.codex” root, because it misses sessions created under a custom CODEX_HOME.
Update openCodexRolloutPath to पहचान the rollout file by its sessions-directory
shape and .jsonl suffix, or resolve the root from process environment/config
instead of checking for "/.codex/sessions/". Keep the existing path matching
logic in that method but make it root-agnostic so custom session roots are
included.
| init( | ||
| homeDirectory: URL = FileManager.default.homeDirectoryForCurrentUser, | ||
| environment: [String: String] = ProcessInfo.processInfo.environment | ||
| ) { | ||
| self.homeDirectory = homeDirectory | ||
| self.claudeConfigRoot = Self.configRoot( | ||
| override: environment["CLAUDE_CONFIG_DIR"], | ||
| default: homeDirectory.appendingPathComponent(".claude", isDirectory: true) | ||
| ) | ||
| self.codexConfigRoot = Self.configRoot( | ||
| override: environment["CODEX_HOME"], | ||
| default: homeDirectory.appendingPathComponent(".codex", isDirectory: true) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use the agent environment for config-root fallbacks.
This captures only the cmux app process environment. If CLAUDE_CONFIG_DIR or CODEX_HOME is set by the agent launcher/subrouter, fallback resolution still searches the app’s default roots for observed/resumed sessions without a recorded transcriptPath. Plumb the per-session config root from hook/process observation, or record the resolved transcript path during 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/AgentChatTranscriptResolver.swift` around lines 30 -
42, The config-root fallback in AgentChatTranscriptResolver currently uses the
app process environment, so it misses agent-launcher/subrouter overrides for
CLAUDE_CONFIG_DIR and CODEX_HOME. Update the transcript resolution flow to use
the per-session agent environment or observed config root when resolving missing
transcriptPath values, by threading that information through
AgentChatTranscriptResolver’s init/configRoot path or by recording the resolved
transcript path during hook/process detection.
| #if DEBUG | ||
| cmuxDebugLog( | ||
| "agentChat.transcript.resolve session=\(record.sessionID.prefix(8)) " | ||
| + "kind=\(record.agentKind.sourceName) cwd=\(record.workingDirectory ?? "nil") UNRESOLVED" | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't log raw working directories in the DEBUG trace.
cwd can include usernames, repo names, or customer/project paths. Log presence or a redacted value instead.
Proposed fix
cmuxDebugLog(
"agentChat.transcript.resolve session=\(record.sessionID.prefix(8)) "
- + "kind=\(record.agentKind.sourceName) cwd=\(record.workingDirectory ?? "nil") UNRESOLVED"
+ + "kind=\(record.agentKind.sourceName) cwd=\(record.workingDirectory == nil ? "nil" : "set") UNRESOLVED"
)📝 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.
| #if DEBUG | |
| cmuxDebugLog( | |
| "agentChat.transcript.resolve session=\(record.sessionID.prefix(8)) " | |
| + "kind=\(record.agentKind.sourceName) cwd=\(record.workingDirectory ?? "nil") UNRESOLVED" | |
| ) | |
| `#if` DEBUG | |
| cmuxDebugLog( | |
| "agentChat.transcript.resolve session=\(record.sessionID.prefix(8)) " | |
| "kind=\(record.agentKind.sourceName) cwd=\(record.workingDirectory == nil ? "nil" : "set") UNRESOLVED" | |
| ) |
🤖 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/AgentChatTranscriptService.swift` around lines 272 -
276, The DEBUG trace in AgentChatTranscriptService’s transcript resolution path
is logging the raw working directory via the cmuxDebugLog call, which can expose
sensitive path details. Update the log in this unresolved-session branch to
avoid printing record.workingDirectory directly and instead emit only a presence
indicator or a redacted placeholder. Keep the existing sessionID and agentKind
context, but ensure the cwd portion is sanitized while preserving usefulness for
debugging.
Source: Coding guidelines
| return .err( | ||
| code: "not_found", | ||
| message: String( | ||
| localized: "mobile.chat.error.sessionNotFound", | ||
| defaultValue: "That agent session is no longer available." | ||
| ), | ||
| data: ["session_id": sessionID] | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not echo session IDs in API error bodies.
The caller already has sessionID; returning it in data exposes a session identifier in an error payload.
As per coding guidelines, “user-facing errors, alerts, command output, API error bodies … must not expose … session IDs.”
Proposed fix
- data: ["session_id": sessionID]
+ data: nil📝 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.
| return .err( | |
| code: "not_found", | |
| message: String( | |
| localized: "mobile.chat.error.sessionNotFound", | |
| defaultValue: "That agent session is no longer available." | |
| ), | |
| data: ["session_id": sessionID] | |
| ) | |
| return .err( | |
| code: "not_found", | |
| message: String( | |
| localized: "mobile.chat.error.sessionNotFound", | |
| defaultValue: "That agent session is no longer available." | |
| ), | |
| data: nil | |
| ) |
🤖 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/TerminalController`+MobileChat.swift around lines 153 - 160, The
error response in TerminalController+MobileChat’s session-not-found path is
leaking the session identifier by including it in the err payload’s data. Remove
the session_id entry from the .err construction in this MobileChat terminal
handler and keep only the user-facing message/code so the caller does not
receive session IDs in API error bodies.
Source: Coding guidelines
| guard let terminalParams = await mobileChatTerminalParams(sessionID: sessionID) else { | ||
| return .err(code: "not_found", message: Self.chatTerminalBindingErrorMessage, data: [ | ||
| "session_id": sessionID | ||
| ]) | ||
| } | ||
| guard let terminalPanel = mobileChatTerminalPanel(sessionID: sessionID) else { | ||
| guard let terminalPanel = await mobileChatTerminalPanel(sessionID: sessionID) else { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the target terminal only once per send.
mobileChatTerminalParams(...) and mobileChatTerminalPanel(...) each re-resolve/refresh the binding. If the registry changes between awaits, attachments can paste using one terminalParams while separators/text are sent to another panel.
Proposed fix
guard let terminalParams = await mobileChatTerminalParams(sessionID: sessionID) else {
return .err(code: "not_found", message: Self.chatTerminalBindingErrorMessage, data: [
"session_id": sessionID
])
}
- guard let terminalPanel = await mobileChatTerminalPanel(sessionID: sessionID) else {
+ guard let resolved = mobileResolveWorkspaceAndSurface(params: terminalParams, requireTerminal: true),
+ let surfaceId = resolved.surfaceId,
+ let terminalPanel = resolved.workspace.terminalPanel(for: surfaceId) else {
return .err(code: "not_found", message: Self.chatTerminalBindingErrorMessage, data: [
"session_id": sessionID
])
}📝 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.
| guard let terminalParams = await mobileChatTerminalParams(sessionID: sessionID) else { | |
| return .err(code: "not_found", message: Self.chatTerminalBindingErrorMessage, data: [ | |
| "session_id": sessionID | |
| ]) | |
| } | |
| guard let terminalPanel = mobileChatTerminalPanel(sessionID: sessionID) else { | |
| guard let terminalPanel = await mobileChatTerminalPanel(sessionID: sessionID) else { | |
| guard let terminalParams = await mobileChatTerminalParams(sessionID: sessionID) else { | |
| return .err(code: "not_found", message: Self.chatTerminalBindingErrorMessage, data: [ | |
| "session_id": sessionID | |
| ]) | |
| } | |
| guard let resolved = mobileResolveWorkspaceAndSurface(params: terminalParams, requireTerminal: true), | |
| let surfaceId = resolved.surfaceId, | |
| let terminalPanel = resolved.workspace.terminalPanel(for: surfaceId) else { | |
| return .err(code: "not_found", message: Self.chatTerminalBindingErrorMessage, data: [ | |
| "session_id": sessionID | |
| ]) | |
| } |
🤖 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/TerminalController`+MobileChat.swift around lines 219 - 224, Resolve
the terminal binding a single time in the send flow instead of calling both
mobileChatTerminalParams(sessionID:) and mobileChatTerminalPanel(sessionID:)
separately. In TerminalController+MobileChat.swift, update the send path to
reuse one resolved terminal target (or a shared lookup helper) so attachments,
separators, and text all go to the same panel/params instance even if the
registry changes between awaits. Keep the existing not_found error handling, but
base it on that one resolution.
# Conflicts: # CLI/CMUXCLI+AgentHookDefinitions.swift # cmuxTests/AgentChatTranscriptResolverTests.swift
| let sessionID = Self.normalizedSessionID(rawSessionID, source: source) | ||
| let now = Date() | ||
| cmuxDebugLog( | ||
| "agentChat.resumeInitiated session=\(sessionID.prefix(8)) source=\(source) " | ||
| + "surface=\((surfaceID ?? "nil").prefix(8)) existed=\(records[sessionID] != nil)" | ||
| ) |
There was a problem hiding this comment.
The
cmuxDebugLog call in noteResumeInitiated is not wrapped in #if DEBUG, but cmuxDebugLog is only defined inside #if DEBUG in Sources/App/DebugLogging.swift. This unconditional call will produce a compile error in release builds. All three other cmuxDebugLog calls added in this file are correctly guarded — this one was missed.
| let sessionID = Self.normalizedSessionID(rawSessionID, source: source) | |
| let now = Date() | |
| cmuxDebugLog( | |
| "agentChat.resumeInitiated session=\(sessionID.prefix(8)) source=\(source) " | |
| + "surface=\((surfaceID ?? "nil").prefix(8)) existed=\(records[sessionID] != nil)" | |
| ) | |
| let sessionID = Self.normalizedSessionID(rawSessionID, source: source) | |
| let now = Date() | |
| #if DEBUG | |
| cmuxDebugLog( | |
| "agentChat.resumeInitiated session=\(sessionID.prefix(8)) source=\(source) " | |
| + "surface=\((surfaceID ?? "nil").prefix(8)) existed=\(records[sessionID] != nil)" | |
| ) | |
| #endif |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift (1)
260-278: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFail closed instead of repinning by terminal recency.
repinToReopenedSession()repins an ended pinned chat to the most recently active newer non-ended session for the same terminal. Terminal identity + recency is not an authoritative session lineage signal, so this can show the wrong conversation when a terminal has multiple newer sessions. Only repin when the descriptor carries a structured resume/reopen relationship to the pinned session; otherwise leavechosenChatSessionnil and let the existing fallback exit chat mode. As per path instructions, correctness-critical agent/session identity must use one authoritative structured source and fail closed instead of using “better than nothing” fallbacks.🤖 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/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift` around lines 260 - 278, The repin logic in repinToReopenedSession currently uses terminalID plus lastActivityAt to choose a newer live session, which can attach the chat to the wrong conversation. Update WorkspaceDetailView so the repin only happens when there is an explicit structured resume/reopen relationship from the descriptor to the pinned session, and do not infer it from recency or terminal matching alone. If no authoritative link exists, leave chosenChatSession nil and let the existing fallback exit chat mode.Source: Path instructions
CLI/CMUXCLI+AgentHookDefinitions.swift (1)
386-406: 🩺 Stability & Availability | 🔵 TrivialSeparate the script-path computation from the file write.
codexPersistentHookScriptCommandinvokeswriteCodexHookScriptwhenever a hook command is generated, including during the equality check insideisCmuxOwnedHookCommand. AlthoughwriteCodexHookScriptis idempotent (it skips writing if the file contents match), invoking a filesystem-write operation within a predicate violates separation of concerns and introduces unnecessary I/O on hot paths.Refactor into two distinct functions:
codexPersistentHookScriptPath(subcommand:_:)— computes the deterministic file path without touching the disk.installCodexPersistentHookScript(...)— performs the atomic write and is called only from the explicit install or refresh flow.Update
isCmuxOwnedHookCommandto compare the command against the computed path (or inline fallback) without triggering any write operations.🤖 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 `@CLI/CMUXCLI`+AgentHookDefinitions.swift around lines 386 - 406, `codexPersistentHookScriptCommand` is doing filesystem writes while merely generating/comparing hook commands, which affects hot-path predicates like `isCmuxOwnedHookCommand`. Split the logic so `codexPersistentHookScriptPath(subcommand:_:)` only computes the deterministic script path, and `installCodexPersistentHookScript(...)` performs the atomic write only in the explicit install/refresh flow. Then update `isCmuxOwnedHookCommand` to use the computed path or inline fallback without calling `writeCodexHookScript`, keeping the existing `codexPersistentHookScriptCommand` behavior as a thin wrapper if needed.
🤖 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 `@CLI/cmux.swift`:
- Around line 33353-33371: The PostToolUse scalar conversion in
boundedPostToolUseScalarValue is treating NSNumber values 0/1 as Bool, which
corrupts exitCode metadata. Update the type checks in
boundedPostToolUseScalarValue to distinguish real CFBoolean/Bool values from
numeric NSNumber instances, using the same CFGetTypeID approach already used in
safeV2DetailValue, and keep numeric exitcode values flowing through as numbers.
---
Outside diff comments:
In `@CLI/CMUXCLI`+AgentHookDefinitions.swift:
- Around line 386-406: `codexPersistentHookScriptCommand` is doing filesystem
writes while merely generating/comparing hook commands, which affects hot-path
predicates like `isCmuxOwnedHookCommand`. Split the logic so
`codexPersistentHookScriptPath(subcommand:_:)` only computes the deterministic
script path, and `installCodexPersistentHookScript(...)` performs the atomic
write only in the explicit install/refresh flow. Then update
`isCmuxOwnedHookCommand` to use the computed path or inline fallback without
calling `writeCodexHookScript`, keeping the existing
`codexPersistentHookScriptCommand` behavior as a thin wrapper if needed.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 260-278: The repin logic in repinToReopenedSession currently uses
terminalID plus lastActivityAt to choose a newer live session, which can attach
the chat to the wrong conversation. Update WorkspaceDetailView so the repin only
happens when there is an explicit structured resume/reopen relationship from the
descriptor to the pinned session, and do not infer it from recency or terminal
matching alone. If no authoritative link exists, leave chosenChatSession nil and
let the existing fallback exit chat mode.
🪄 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: ccb79efd-36c5-4be8-b926-7fd39058a4cf
📒 Files selected for processing (5)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamEvent.swiftResources/Localizable.xcstrings
💤 Files with no reviewable changes (2)
- Resources/Localizable.xcstrings
- Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamEvent.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift (1)
260-278: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFail closed instead of repinning by terminal recency.
repinToReopenedSession()repins an ended pinned chat to the most recently active newer non-ended session for the same terminal. Terminal identity + recency is not an authoritative session lineage signal, so this can show the wrong conversation when a terminal has multiple newer sessions. Only repin when the descriptor carries a structured resume/reopen relationship to the pinned session; otherwise leavechosenChatSessionnil and let the existing fallback exit chat mode. As per path instructions, correctness-critical agent/session identity must use one authoritative structured source and fail closed instead of using “better than nothing” fallbacks.🤖 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/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift` around lines 260 - 278, The repin logic in repinToReopenedSession currently uses terminalID plus lastActivityAt to choose a newer live session, which can attach the chat to the wrong conversation. Update WorkspaceDetailView so the repin only happens when there is an explicit structured resume/reopen relationship from the descriptor to the pinned session, and do not infer it from recency or terminal matching alone. If no authoritative link exists, leave chosenChatSession nil and let the existing fallback exit chat mode.Source: Path instructions
CLI/CMUXCLI+AgentHookDefinitions.swift (1)
386-406: 🩺 Stability & Availability | 🔵 TrivialSeparate the script-path computation from the file write.
codexPersistentHookScriptCommandinvokeswriteCodexHookScriptwhenever a hook command is generated, including during the equality check insideisCmuxOwnedHookCommand. AlthoughwriteCodexHookScriptis idempotent (it skips writing if the file contents match), invoking a filesystem-write operation within a predicate violates separation of concerns and introduces unnecessary I/O on hot paths.Refactor into two distinct functions:
codexPersistentHookScriptPath(subcommand:_:)— computes the deterministic file path without touching the disk.installCodexPersistentHookScript(...)— performs the atomic write and is called only from the explicit install or refresh flow.Update
isCmuxOwnedHookCommandto compare the command against the computed path (or inline fallback) without triggering any write operations.🤖 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 `@CLI/CMUXCLI`+AgentHookDefinitions.swift around lines 386 - 406, `codexPersistentHookScriptCommand` is doing filesystem writes while merely generating/comparing hook commands, which affects hot-path predicates like `isCmuxOwnedHookCommand`. Split the logic so `codexPersistentHookScriptPath(subcommand:_:)` only computes the deterministic script path, and `installCodexPersistentHookScript(...)` performs the atomic write only in the explicit install/refresh flow. Then update `isCmuxOwnedHookCommand` to use the computed path or inline fallback without calling `writeCodexHookScript`, keeping the existing `codexPersistentHookScriptCommand` behavior as a thin wrapper if needed.
🤖 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 `@CLI/cmux.swift`:
- Around line 33353-33371: The PostToolUse scalar conversion in
boundedPostToolUseScalarValue is treating NSNumber values 0/1 as Bool, which
corrupts exitCode metadata. Update the type checks in
boundedPostToolUseScalarValue to distinguish real CFBoolean/Bool values from
numeric NSNumber instances, using the same CFGetTypeID approach already used in
safeV2DetailValue, and keep numeric exitcode values flowing through as numbers.
---
Outside diff comments:
In `@CLI/CMUXCLI`+AgentHookDefinitions.swift:
- Around line 386-406: `codexPersistentHookScriptCommand` is doing filesystem
writes while merely generating/comparing hook commands, which affects hot-path
predicates like `isCmuxOwnedHookCommand`. Split the logic so
`codexPersistentHookScriptPath(subcommand:_:)` only computes the deterministic
script path, and `installCodexPersistentHookScript(...)` performs the atomic
write only in the explicit install/refresh flow. Then update
`isCmuxOwnedHookCommand` to use the computed path or inline fallback without
calling `writeCodexHookScript`, keeping the existing
`codexPersistentHookScriptCommand` behavior as a thin wrapper if needed.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 260-278: The repin logic in repinToReopenedSession currently uses
terminalID plus lastActivityAt to choose a newer live session, which can attach
the chat to the wrong conversation. Update WorkspaceDetailView so the repin only
happens when there is an explicit structured resume/reopen relationship from the
descriptor to the pinned session, and do not infer it from recency or terminal
matching alone. If no authoritative link exists, leave chosenChatSession nil and
let the existing fallback exit chat mode.
🪄 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: ccb79efd-36c5-4be8-b926-7fd39058a4cf
📒 Files selected for processing (5)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamEvent.swiftResources/Localizable.xcstrings
💤 Files with no reviewable changes (2)
- Resources/Localizable.xcstrings
- Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamEvent.swift
🛑 Comments failed to post (1)
CLI/cmux.swift (1)
33353-33371: 🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
In Swift, does (NSNumber(value: 1) as? Bool) and (NSNumber(value: 0) as? Bool) succeed and return true/false?💡 Result:
In Swift, casting
NSNumber(value: 1) as? Boolsucceeds and returnstrue, and castingNSNumber(value: 0) as? Boolsucceeds and returnsfalse[1]. This behavior is governed by the conditional bridging mechanism betweenNSNumberandBool[1]. The standard library implementation specifically checks for these values: 1.NSNumber(value: 1)orkCFBooleanTrueis bridged totrue[1]. 2.NSNumber(value: 0)orkCFBooleanFalseis bridged tofalse[1]. If anNSNumbercontains a value other than 0 or 1 (such as0.25or5), the conditional castas? Boolwill fail and returnnil[1][2]. This behavior ensures that the bridge is restrictive and does not interpret arbitrary non-zero numbers astrue, which differs from the behavior of theboolValueproperty (which treats any non-zero value astrue) [1][3].Citations:
- 1: https://github.com/apple/swift/blob/a28c9d62138e5386ba9341332af218cd232c455e/stdlib/public/SDK/Foundation/NSNumber.swift
- 2: swiftlang/swift-corelibs-foundation#3632
- 3: https://developer.apple.com/documentation/foundation/nsnumber/boolvalue
exitCode0/1 rendered incorrectly asfalse/truein PostToolUse feed summaryThe
boundedPostToolUseScalarValuefunction checksrawValue as? Boolbeforeas? NSNumber. In Swift,NSNumberinstances wrapping 0 or 1 successfully cast toBoolvia conditional bridging, returningfalseandtruerespectively. This causes numericexitcodevalues infeedPostToolUseMetadataKeysto be coerced into booleans, corrupting telemetry data.Adopt the
CFGetTypeIDcheck used insafeV2DetailValueto distinguishCFBooleanreferences from numericNSNumbertypes:🐛 Proposed fix
private static func boundedPostToolUseScalarValue(_ rawValue: Any) -> (value: Any, truncated: Bool)? { if rawValue is NSNull { return (NSNull(), false) } - if let bool = rawValue as? Bool { - return (bool, false) - } if let number = rawValue as? NSNumber { + if CFGetTypeID(number) == CFBooleanGetTypeID() { + return (number.boolValue, false) + } return (number, false) } + if let bool = rawValue as? Bool { + return (bool, false) + } if let string = rawValue as? String {📝 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.private static func boundedPostToolUseScalarValue(_ rawValue: Any) -> (value: Any, truncated: Bool)? { if rawValue is NSNull { return (NSNull(), false) } if let number = rawValue as? NSNumber { if CFGetTypeID(number) == CFBooleanGetTypeID() { return (number.boolValue, false) } return (number, false) } if let bool = rawValue as? Bool { return (bool, false) } if let string = rawValue as? String { let preview = feedUTF8Prefix( string, maxBytes: feedPostToolUseScalarStringLimitBytes ) return (preview.value, preview.truncated) } return nil }🤖 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 `@CLI/cmux.swift` around lines 33353 - 33371, The PostToolUse scalar conversion in boundedPostToolUseScalarValue is treating NSNumber values 0/1 as Bool, which corrupts exitCode metadata. Update the type checks in boundedPostToolUseScalarValue to distinguish real CFBoolean/Bool values from numeric NSNumber instances, using the same CFGetTypeID approach already used in safeV2DetailValue, and keep numeric exitcode values flowing through as numbers.
AgentChatSessionRegistry grew past the 500-line untracked threshold (the observe-floor process-tree scan, libproc rollout-fd reader, argv id parsing, and the agentChat.* debug trace), and a few tracked files grew on the merge with main. Refresh the budget (--write-budget) to accept the legitimate growth so the workflow-guard-tests file-length gate passes. Splitting the observe-floor detection out of the registry into its own file is a reasonable follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…store data-plane-only boundary) My earlier commit bd85a66 added mobile.chat.sessions to the mobile-host coordinator (handleMobileHost) as a dogfood-verification convenience. That overloads the real data-plane verb name onto a dispatcher that is deliberately forbidden from owning mobile.chat.* verbs: the header doc and the v2SurfaceMobileHostHandlerIgnoresDataPlaneOnlyVerbs test both encode that those verbs reach the Mac only through the mobile data-plane RPC (mobileHostHandleRPC). The added protocol requirement also broke every CmuxControlSocketTests fake (the shared ControlMobileHostContext extension had no default), failing swift-package-tests. Remove the three pieces of the debug seam: the handleMobileHost dispatch case, the controlMobileChatSessions protocol requirement, and the TerminalController conformance. The real fix from bd85a66 (v2MobileChatSessions scoping chat sessions by the surface's CURRENT workspace) is untouched and is still reached by the data-plane RPC at TerminalController+MobileChat.swift:35. A workspace-scoped debug verb can be re-added later under a distinct debug-only name instead of overloading the data-plane verb. CmuxControlSocket: 178 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Exit
The off-main re-bind in handleProcessExit nests a MainActor.run closure
inside a Task.detached { [weak self] } closure. The inner closure referenced
the outer closure's captured weak `self` var across the concurrency boundary,
which Swift flags as "reference to captured var 'self' in concurrently-
executing code" (a hard error in the Swift 6 language mode). This tripped the
tests-build-and-lag swift_warning_budget gate as a new actual=1 budget=0
bucket.
Give the inner MainActor.run closure its own [weak self] capture so it binds
self from the enclosing scope instead of referencing the outer closure's var.
Behavior is unchanged; the guard still no-ops on a deallocated registry.
Verified: tagged app build succeeds with the warning gone.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // live so the GUI shows the resumed session immediately; the transcript | ||
| // path resolves on demand from the session id. | ||
| var record = AgentChatSessionRecord( | ||
| sessionID: sessionID, | ||
| agentKind: ChatAgentKind(source: source), | ||
| workspaceID: normalizedWorkspace, |
There was a problem hiding this comment.
Unconditional
cmuxDebugLog call in noteResumeInitiated will fail to compile in release builds
cmuxDebugLog is defined only inside #if DEBUG (in Sources/App/DebugLogging.swift), but the call inside noteResumeInitiated is not wrapped in a matching #if DEBUG guard. Every other cmuxDebugLog call added in this file (in applyObservedSessions, update, and noteHookEvent) is correctly guarded — this one was missed. The release build will produce a "use of unresolved identifier 'cmuxDebugLog'" error.
…tracing Adds a host-side bisection tool for the agent-chat pipeline so a missing iOS GUI can be localized to the Mac, the phone, or update delivery instead of guessed at. - scripts/cmux-chat-debug.py: reads the live registry over the tagged debug socket (`cmux rpc chat.sessions.dump`) and cross-references it against the app's current surfaces (`debug-terminals`) to bucket every session as reaches-phone / dropped-by-filter / stale (surface not in any current workspace). Surfaces the registry-hygiene reality directly: most records are seeded from the append-only Claude/Codex hook stores on launch and reference surfaces that no longer exist after a relaunch. - v2MobileChatSessions: DEBUG-only cmuxDebugLog tracing of the requested workspace, whether it resolved, and the per-session keep/drop reason (not-in-workspace vs dead-pid), plus a summary line. This is the trace that pinpoints why a workspace-scoped pull returns empty. No release-build behavior change (tracing is #if DEBUG; the script is tooling). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/TerminalController+MobileChat.swift (2)
374-392: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse refreshed workspace bindings after hook-store refresh.
After
refreshSessionBindings, the code still resolvessurfaceIDagainst the oldworkspaceID. If the refresh fixed a stale workspace binding, this still reports “terminal moved” and prevents sending.Proposed fix
- guard let record = service.sessionRecord(sessionID: sessionID), - let workspaceID = record.workspaceID else { + guard let record = service.sessionRecord(sessionID: sessionID) else { return nil } - if let surfaceID = record.surfaceID, + if let workspaceID = record.workspaceID, + let surfaceID = record.surfaceID, mobileChatBindingResolves(workspaceID: workspaceID, surfaceID: surfaceID), mobileChatBindingIsCurrentAgent(record) { return ["workspace_id": workspaceID, "surface_id": surfaceID] } @@ if let refreshed = await service.refreshSessionBindings(sessionID: sessionID), + let workspaceID = refreshed.workspaceID, let surfaceID = refreshed.surfaceID, mobileChatBindingResolves(workspaceID: workspaceID, surfaceID: surfaceID), mobileChatBindingIsCurrentAgent(refreshed) { return ["workspace_id": workspaceID, "surface_id": surfaceID]🤖 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/TerminalController`+MobileChat.swift around lines 374 - 392, The mobile chat terminal parameter lookup is still validating the refreshed surface against the stale workspace value, which can leave “terminal moved” handling incorrect. In mobileChatTerminalParams(sessionID:), after refreshSessionBindings returns a refreshed record, use the refreshed workspaceID (falling back to the original only if needed) when calling mobileChatBindingResolves and when building the returned workspace_id/surface_id pair. Keep the existing checks around agent ownership and the refresh path in TerminalController+MobileChat.swift.
34-86: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAwait the observe-floor scan before reading sessions.
mobile.chat.sessionsreturns the current registry whileobserveAgentProcesses()runs later in an unstructuredTask, so the first pull can still return stale/empty results for an unhooked live Codex/Claude session. Since the dispatch path is alreadyasync, makev2MobileChatSessionsasync and await the throttled off-main refresh before enumerating records.As per coding guidelines and path instructions, avoid fire-and-forget
Taskwork with meaningful lifecycle, and do not introduce visible staleness windows for correctness-critical session/liveness tracking.Proposed fix
switch method { case "mobile.chat.sessions": - return v2MobileChatSessions(params: params) + return await v2MobileChatSessions(params: params) @@ - func v2MobileChatSessions(params: [String: Any]) -> V2CallResult { + func v2MobileChatSessions(params: [String: Any]) async -> V2CallResult { @@ - Task { await service.observeAgentProcesses() } + await service.observeAgentProcesses()🤖 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/TerminalController`+MobileChat.swift around lines 34 - 86, `v2MobileChatSessions` is launching `agentChatTranscriptService.observeAgentProcesses()` in a fire-and-forget `Task`, so the session list can be read before the refresh completes. Make `v2MobileChatSessions(params:)` async, await the observe-floor scan directly before collecting sessions, and keep the existing `mobile.chat.sessions` dispatch path aligned with the async call chain. Use `agentChatTranscriptService` and `observeAgentProcesses()` as the key symbols to update so the registry is refreshed before returning results.Sources: Coding guidelines, 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.
Inline comments:
In `@scripts/cmux-chat-debug.py`:
- Around line 32-35: The cli helper currently hides failures by returning only
stdout from subprocess.run, which can turn missing CMUX_TAG or a failing
cmux-debug-cli.sh into an empty result. Update cli in cmux-chat-debug.py to
check the subprocess result and raise or exit with a clear error when the
command fails or required env like CMUX_TAG is missing, so callers don’t render
“0 records” on failure.
- Around line 124-130: The live refresh loop in cmux-chat-debug.py shells out
with os.system("clear"), which should be removed to avoid shell-execution
checks. Update the --live branch inside the while True loop to clear the
terminal using ANSI escape sequences instead, while keeping the existing
render(show_all) and refresh behavior intact.
---
Outside diff comments:
In `@Sources/TerminalController`+MobileChat.swift:
- Around line 374-392: The mobile chat terminal parameter lookup is still
validating the refreshed surface against the stale workspace value, which can
leave “terminal moved” handling incorrect. In
mobileChatTerminalParams(sessionID:), after refreshSessionBindings returns a
refreshed record, use the refreshed workspaceID (falling back to the original
only if needed) when calling mobileChatBindingResolves and when building the
returned workspace_id/surface_id pair. Keep the existing checks around agent
ownership and the refresh path in TerminalController+MobileChat.swift.
- Around line 34-86: `v2MobileChatSessions` is launching
`agentChatTranscriptService.observeAgentProcesses()` in a fire-and-forget
`Task`, so the session list can be read before the refresh completes. Make
`v2MobileChatSessions(params:)` async, await the observe-floor scan directly
before collecting sessions, and keep the existing `mobile.chat.sessions`
dispatch path aligned with the async call chain. Use
`agentChatTranscriptService` and `observeAgentProcesses()` as the key symbols to
update so the registry is refreshed before returning results.
🪄 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: 09fc7097-444e-4a91-9ec2-5edd0c635544
📒 Files selected for processing (2)
Sources/TerminalController+MobileChat.swiftscripts/cmux-chat-debug.py
# Conflicts: # .github/swift-file-length-budget.tsv
…g script
Review fixes:
- P0 (Greptile): the cmuxDebugLog call in noteResumeInitiated was unconditional.
cmuxDebugLog only exists under #if DEBUG (no release stub), so the bare call
would fail the release/beta build. Wrap it in #if DEBUG like every other
agent-chat trace. (The release-build CI job was stuck queued, so this latent
break was never surfaced.) Swept all PR-changed Swift files: no other
unconditional calls remain.
- cmux-chat-debug.py: fail loudly when CMUX_TAG is unset or the debug CLI
returns nonzero (was silently returning empty); replace os.system("clear")
with an ANSI clear instead of shelling out each refresh.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Review triage:
|
…+2 lines) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reliable agent-session tracking for the iOS coding-agent GUI, Codex interactive-picker support, and an end-to-end debug trace. Built and dogfooded iteratively this session across Claude + Codex.
Reliability system (two-pillar: observe floor + hooks)
nodeshim) re-binds to the real agent instead of falsely ending a live session.--session-id/--resumeargv).stateChanged(versioneddescriptorChangedis authoritative); the live send/list gate is deterministic pid-liveness, not the terminal-title heuristic; resume re-bind keyed on the realterminalPanel.id, fired whenever a restored surface carries a resumable binding.CLAUDE_CONFIG_DIR/CODEX_HOME; accurate "can't find transcript" copy for the$HOME-cwd case.#!/bin/shin~/.cmux/hooks) for both wrapper-injected and persistent~/.codex/hooks.jsoncommands so non-shell runtimes (subrouters) stop erroring withos error 2.Codex interactive pickers in the GUI
request_user_inputparses into the same tappableChatQuestionthe GUI renders for Claude's AskUserQuestion.mobile.chat.answeris agent-aware (Codex needs Enter after the digit).function_call_output; multi-question codex calls resolve each card by question id.Debuggability
agentChat.*trace across the whole pipeline (hook ingest, process-tree detection, state transitions, transcript resolve, transcript batch). Onegrep 'agentChat\.' /tmp/cmux-debug-<tag>.logshows the full flow.Verification
codexwrap).{}0.Known follow-ups (not regressions)
SessionStart/UserPromptSubmitleaves a stale inline duplicate (fresh installs are clean); the fire-and-forget format dodges the ownership matcher.$HOME-cwd Claude session that Claude itself doesn't persist a transcript for; cmux reports it honestly. An FSEvents transcript watcher (capture every file the agent writes) is the planned robust fix.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Rebuilt agent-session tracking for iOS with a versioned single source of truth, process‑tree detection, and full Codex support (wrapper, resume, GUI pickers) so sessions stay reliably live and editable. Adds a DEBUG trace and a host-side
scripts/cmux-chat-debug.pyinspector, restores the data‑plane boundary formobile.chat.sessions, and fixes a Swift‑6 warning.New Features
version, version‑gateddescriptorChangedupserts, andmobile.chat.sessionsnapshot pull; iOS re‑pulls on foreground.--session-id/--resume).cmux-codex-wrapperwith fire‑and‑forget hooks; sibling PATH shim exportsCMUX_CODEX_WRAPPER_SHIM; wrapper firessession-starton resume; persistent hooks emit#!/bin/shscripts; Settings toggle; honorsCODEX_HOME.request_user_inputrenders as tappableChatQuestion;mobile.chat.answerappends Enter for Codex; multi‑question resolution byquestionID.mobile.chat.sessionsscopes by the surface’s current workspace, retains ended sessions, and re‑pins to a reopened live session; data‑plane only.agentChat.*DEBUG logs across hook ingest, detection, state, and transcript; DEBUG‑only tracing insidemobile.chat.sessions; host‑sidescripts/cmux-chat-debug.py(now fails loudly on unsetCMUX_TAG/CLI errors and uses ANSI clear); the resume‑initiated log is gated under#if DEBUG.selfwarning in the process‑exit handler.Migration
cmux hooks setup --agent codexto install persistent hooks.CMUX_CODEX_HOOKS_DISABLED=1).Written for commit fe9df5e. Summary will update on new commits.
Summary by CodeRabbit
questionIDto support multi-question Codex and id-based answer completion.