Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6d67c0a
agent-session SoT: add session version + authoritative pull snapshot …
azooz2003-bit Jun 23, 2026
000cbec
agent-session SoT: iOS re-pulls the session list on foreground (Slice A)
azooz2003-bit Jun 23, 2026
edc04ec
agent-session SoT: deterministic process-exit backstop (Slice B)
azooz2003-bit Jun 23, 2026
775ce2f
agent-session SoT spec: mark Slices A+B done; note ended-input-bar UI…
azooz2003-bit Jun 23, 2026
1392e7e
agent-session SoT: delete title/mtime detection heuristics (Slice D)
azooz2003-bit Jun 23, 2026
324f085
agent-session SoT spec: Slice D done; C/E/F scoped as follow-ups with…
azooz2003-bit Jun 23, 2026
6f2e0d8
agent-session SoT: move hook-store JSON reads off the main actor (Sli…
azooz2003-bit Jun 23, 2026
ef03f5c
agent-session SoT spec: Slice C done; E/F deferred with rationale
azooz2003-bit Jun 23, 2026
e6f4804
agent-session SoT spec: resolve Slice E (not needed; invariant alread…
azooz2003-bit Jun 23, 2026
5d0d79e
codex detection: plan (Slice F) — wrapper-emits-session-start, no glo…
azooz2003-bit Jun 23, 2026
4bcca3e
codex detection: cmux-codex-wrapper + PATH shim + per-invocation hook…
azooz2003-bit Jun 23, 2026
fbfc7da
codex detection: bind surface/transcript on live session-start via fe…
azooz2003-bit Jun 23, 2026
4a1a8c8
codex detection: plan status -> implemented + live-verified
azooz2003-bit Jun 23, 2026
bef654d
codex detection: fire-and-forget injected hooks so codex never blocks…
azooz2003-bit Jun 23, 2026
bd85a66
agent-session: scope mobile chat sessions by surface's current worksp…
azooz2003-bit Jun 23, 2026
d8e980f
Merge pull request #6655 from manaflow-ai/feat-codex-detection
azooz2003-bit Jun 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions CLI/CMUXCLI+CodexFireAndForgetHooks.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,62 @@
import Foundation

extension CMUXCLI {
/// The per-invocation Codex hook events the wrapper injects, paired with the
/// cmux subcommand they call and the codex hook timeout (ms). Lifecycle
/// events are short; feed events (`PreToolUse`/`PermissionRequest`) are long
/// because the user may take time to approve. This is the single source of
/// truth for `cmux-codex-wrapper`'s injection, mirrored from the historic
/// hand-rolled `cmux_codex_add_hook` calls in the wrapper.
static let codexWrapperInjectionEvents: [(agentEvent: String, cmuxSubcommand: String, timeoutMs: Int)] = [
("SessionStart", "session-start", 10000),
("UserPromptSubmit", "prompt-submit", 10000),
("Stop", "stop", 10000),
("PreToolUse", "pre-tool-use", 120000),
("PostToolUse", "post-tool-use", 10000),
("PermissionRequest", "notification", 120000),
]

/// Emit, NUL-separated to stdout, the exact codex arg list the wrapper must
/// splice ahead of the user's args to enable + inject cmux's fire-and-forget
/// hooks for one codex invocation. Returns the arg list:
/// --enable\0hooks\0--dangerously-bypass-hook-trust\0
/// -c\0hooks.SessionStart=[{hooks=[{type="command",command='''<ff>''',timeout=10000}]}]\0
/// -c\0hooks.UserPromptSubmit=...\0 ... (one `-c` pair per event)
/// where `<ff>` is `codexFireAndForgetAgentHookShellCommand(...)` so each
/// hook returns `{}` to codex instantly and backgrounds the real cmux call.
/// Requires no live socket: pure string construction from the agent def.
func emitCodexWrapperInjectArgs() throws {
guard let codexDef = Self.agentDef(named: "codex") else {
throw CLIError(message: "Codex hook integration is unavailable.")
}
var args: [String] = ["--enable", "hooks", "--dangerously-bypass-hook-trust"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

❓ Verification inconclusive

🌐 Web query:

For the OpenAI Codex CLI version range cmux supports, when was --dangerously-bypass-hook-trust introduced, and does it bypass review for all enabled hook sources in an invocation or only hooks injected via command-line -c arguments?

💡 Result:

The flag was introduced in OpenAI Codex via PR #21768 (May 8, 2026) and first stable in Codex 0.131.0 (after being merged May 13, 2026). [1][2] Effect: it is an invocation-scoped runtime-only override that bypasses the “persisted hook trust” requirement for enabled hooks for that specific Codex invocation, not just hooks injected via command-line -c arguments. Evidence: - PR #21768 states the flag “sets a runtime-only bypass_hook_trust override” and is “one invocation-scoped escape hatch,” plus explicitly notes it affects “enabled hooks” being allowed to run without requiring persisted hook trust for that invocation. [1] - The OpenAI Developers “Hooks” docs describe that for “one-off automation… pass --dangerously-bypass-hook-trust to run enabled hooks without requiring persisted hook trust for that invocation.” This implies the behavior is based on which hooks are enabled, not whether they were provided via -c arguments. [3] - Additional Codex issue discussion confirms there are separate CLI/non-interactive vs TUI behaviors, but in the described working behavior the bypass suppresses the startup trust prompt and “Enabled hooks run without requiring per-session trust (still respecting explicit disabled hooks).” [2] So, for a given Codex invocation where the flag is actually honored (e.g., not lost across a TUI/app-server path), it bypasses review/trust gating for all hooks that are enabled in that invocation (subject to explicit disabled hooks), regardless of whether they were supplied through -c arguments or otherwise discovered/loaded. [1][3][2] Note: there were reported cases where TUI mode startup initially didn’t respect the flag (blocking on the interactive “Hooks need review” prompt), but that was later fixed to skip the prompt appropriately; also, persistent/resumed threads may keep review prompt when the bypass override can’t be safely applied. [2][4]

Citations:


🏁 Script executed:

# Find and inspect the file with the flag usage
fd -t f "CMUXCLI+CodexFireAndForgetHooks.swift" -x wc -l {}

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Read the file with context around line 32
fd -t f "CMUXCLI+CodexFireAndForgetHooks.swift" -x cat -n {} | head -50

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Search for Codex version constraints in cmux
rg "version|Codex" -t swift --max-count=5 -B 2 -A 2 | head -80

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Search for hook configuration and bypass flag usage elsewhere
rg "dangerously-bypass-hook-trust|hook" -t swift -B 3 -A 3 | head -100

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Find the file in the entire repo
find . -name "*CodexFireAndForget*" -type f 2>/dev/null

Repository: manaflow-ai/cmux

Length of output: 198


🏁 Script executed:

# Search for the bypass flag usage in any file
rg "dangerously-bypass" -t swift -B 5 -A 5

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Search for Codex version or requirements
rg "Codex|codex" -t swift --max-count=10 | head -50

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look at file structure to understand where CLI files are
find . -path "*/CLI/*.swift" -type f 2>/dev/null | head -20

Repository: manaflow-ai/cmux

Length of output: 857


🏁 Script executed:

# Read the CodexFireAndForgetHooks file to understand context
cat -n ./CLI/CMUXCLI+CodexFireAndForgetHooks.swift

Repository: manaflow-ai/cmux

Length of output: 5363


🏁 Script executed:

# Search for any version constraints or requirements for Codex
rg -i "codex.*version|requirement|support" -t swift --max-count=20

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look for how hooks are loaded/configured in cmux
rg -i "hook.*config|load.*hook|enabled.*hook" -t swift -B 2 -A 2 | head -80

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Search for Codex minimum version requirements or constraints
rg -i "0\.[0-9]+\.[0-9]+" -t swift -B 2 -A 2 | grep -i codex

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look for any configuration or user hook loading
rg "user.*hook|project.*hook|~/.config|\.cmux|hook.*source" -t swift -B 2 -A 2 | head -60

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Check if there are any conditions/gates around the bypass flag usage
rg "bypass.*hook|hook.*bypass" -t swift -B 10 -A 10

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Search for Codex agent definition to understand supported versions
rg "agentDef|codex" -t swift | grep -i "version\|require\|support" | head -20

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look for agent definitions or Codex version info
find . -name "*.swift" -type f -exec grep -l "agentDef\|Agent.*Definition" {} \; | head -10

Repository: manaflow-ai/cmux

Length of output: 500


🏁 Script executed:

# Check for any Codex-specific configuration or minimum version checks
rg "Codex\|codex" -t swift --context 3 | head -100

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Search for wrapper-related code that might show context
rg "wrapper|cmux-codex-wrapper|emitCodex" -t swift -B 2 -A 5 | head -80

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look at git history/blame for this file to understand intent
cd . && git log --oneline -20 -- CLI/CMUXCLI+CodexFireAndForgetHooks.swift 2>/dev/null || echo "No git history available"

Repository: manaflow-ai/cmux

Length of output: 226


🏁 Script executed:

# Look at the agent hook definitions to understand version requirements
cat -n ./CLI/CMUXCLI+AgentHookDefinitions.swift | head -80

Repository: manaflow-ai/cmux

Length of output: 5085


🏁 Script executed:

# Check the merge commit that added this feature
git show --stat d8e980f2 2>/dev/null | head -50

Repository: manaflow-ai/cmux

Length of output: 3107


🏁 Script executed:

# Look for any documentation about Codex support or version requirements
rg -i "codex.*version|minimum.*codex|require.*codex" -t swift -t md -t txt | head -40

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Check if there are any discussions or TODOs about the bypass flag
rg "dangerously|bypass|hook.*trust|trust.*hook" -t swift -B 3 -A 3

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Get more of the agent definitions to find Codex config
cat -n ./CLI/CMUXCLI+AgentHookDefinitions.swift | grep -A 20 "\"codex\""

Repository: manaflow-ai/cmux

Length of output: 4311


🏁 Script executed:

# Search for how this function is called and whether there are any guards
rg "emitCodexWrapperInjectArgs" -t swift -B 5 -A 5

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look for any tests or documentation about Codex hook integration
find . -name "*.swift" -o -name "*.md" | xargs grep -l "CodexFireAndForget\|codex.*hook\|bypass.*hook" 2>/dev/null

Repository: manaflow-ai/cmux

Length of output: 1492


🏁 Script executed:

# Check if there's a README or docs about supported agent versions
find . -name "README*" -o -name "*SETUP*" -o -name "*AGENT*" | head -20

Repository: manaflow-ai/cmux

Length of output: 962


🏁 Script executed:

# Look at the Codex agent detection plan
cat -n ./docs/codex-agent-detection-plan.md | head -100

Repository: manaflow-ai/cmux

Length of output: 6068


🏁 Script executed:

# Look at agent hooks documentation
cat -n ./docs/agent-hooks.md | head -150

Repository: manaflow-ai/cmux

Length of output: 11116


🏁 Script executed:

# Check the CLI contract for version constraints
rg "version|support|require" ./docs/cli-contract.md -B 2 -A 2

Repository: manaflow-ai/cmux

Length of output: 5294


🏁 Script executed:

# Check if there's any version checking or gating for the bypass flag
rg "0\.131|codex.*version|version.*codex" -t swift -B 3 -A 3

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Look for any discussions or decision records about this trade-off
rg "bypass.*security|security.*bypass|hook.*trust|trust.*hook" -t md -B 2 -A 2

Repository: manaflow-ai/cmux

Length of output: 154


🏁 Script executed:

# Check if there are any guards or conditions around enabling the bypass
ast-grep outline ./CLI/CMUXCLI+CodexFireAndForgetHooks.swift

Repository: manaflow-ai/cmux

Length of output: 355


🏁 Script executed:

# Look at how the wrapper is configured and whether user hooks are involved
rg "codex.*hook|hook.*codex" -t swift -B 3 -A 3 | head -100

Repository: manaflow-ai/cmux

Length of output: 154


Gate Codex hook-trust bypass by version and scope.

Line 32 unconditionally emits --dangerously-bypass-hook-trust on every Codex invocation. The flag (introduced in Codex v0.131.0, May 2026) bypasses hook review for all enabled hooks in that invocation—not just the cmux-injected ones via -c arguments. Since Codex persists hooks in ~/.codex/hooks.json and config in ~/.codex/config.toml, any user or project hooks already installed there would bypass review too, even though cmux did not inject them. The design correctly states cmux should not clobber user Codex config, so a user with existing project/personal Codex hooks would have those hooks automatically authorized without their knowledge. Confirm that cmux-owned Codex wrapper injection scopes the bypass only to cmux-injected hooks, or gate the flag by minimum Codex version and fail closed if user hook sources are detected. Document the security boundary in the code.

🤖 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`+CodexFireAndForgetHooks.swift at line 32, The unconditional
emission of `--dangerously-bypass-hook-trust` on line 32 bypasses hook review
for all hooks in Codex (including any pre-existing user/project hooks in
~/.codex/hooks.json and ~/.codex/config.toml), not just cmux-injected ones,
which violates cmux's principle of not clobbering user Codex config. To fix
this, either verify that the cmux-owned Codex wrapper injection scopes the
bypass only to cmux-injected hooks via the `-c` arguments, or gate the
`--dangerously-bypass-hook-trust` flag by checking the Codex version (introduced
in v0.131.0) and fail closed if user hook sources are detected in the Codex
config. Additionally, document the security boundary in code comments explaining
which hooks this flag applies to and what assumptions are being made about the
scope of the bypass.

for event in Self.codexWrapperInjectionEvents {
let ff = Self.codexFireAndForgetAgentHookShellCommand(
"cmux hooks codex \(event.cmuxSubcommand)", for: codexDef
)
// TOML multi-line literal string ('''...''') preserves bytes verbatim
// and may contain single quotes, so the embedded `echo '{}'` / `sh -c
// '...'` survive with no escaping. TOML forbids only a literal triple
// single quote inside; guard against it (the command never has one).
guard !ff.contains("'''") else {
throw CLIError(message: "Codex fire-and-forget hook command contains a triple single quote and cannot be TOML-encoded.")
}
let toml = "hooks.\(event.agentEvent)=[{hooks=[{type=\"command\",command='''\(ff)''',timeout=\(event.timeoutMs)}]}]"
args.append("-c")
args.append(toml)
}
// NUL-TERMINATE each arg (trailing NUL after the last too) so a bash
// `while IFS= read -r -d '' arg` loop captures every element including
// the final one — a separator-only stream drops the unterminated last
// arg at EOF.
var out = Data()
for arg in args {
out.append(Data(arg.utf8))
out.append(0)
}
FileHandle.standardOutput.write(out)
}

static func codexFireAndForgetAgentHookShellCommand(_ command: String, for def: AgentHookDef) -> String {
let routedArguments = command.hasPrefix("cmux ") ? String(command.dropFirst("cmux ".count)) : command
let runner = "payload=\"$1\"; shift; \"$@\" <\"$payload\" >/dev/null 2>&1 & child=\"$!\"; ( sleep 30; kill \"$child\" 2>/dev/null || true ) & watchdog=\"$!\"; wait \"$child\" 2>/dev/null || true; kill \"$watchdog\" 2>/dev/null || true; rm -f \"$payload\""
Expand Down
36 changes: 25 additions & 11 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29587,14 +29587,15 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
client: client
)
}
func sendAgentFeedTelemetry(workspaceId: String? = nil) {
func sendAgentFeedTelemetry(workspaceId: String? = nil, surfaceId: String? = nil) {
didSendFeedTelemetry = true
sendFeedTelemetry(
client: client,
source: def.name,
subcommand: subcommand,
parsedInput: input,
workspaceId: workspaceId ?? workspaceArg(),
surfaceId: surfaceId,
socketPassword: socketPassword
)
}
Expand All @@ -29610,11 +29611,11 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
}
return def.feedHookEvents.contains(event)
}
func sendAgentFeedTelemetryUnlessSuppressed(workspaceId: String? = nil) {
func sendAgentFeedTelemetryUnlessSuppressed(workspaceId: String? = nil, surfaceId: String? = nil) {
if shouldSuppressGenericFeedTelemetry() {
didSendFeedTelemetry = true
} else {
sendAgentFeedTelemetry(workspaceId: workspaceId)
sendAgentFeedTelemetry(workspaceId: workspaceId, surfaceId: surfaceId)
}
}
func notificationDedupeFingerprint(status: AgentHookNotificationStatus?) -> String? {
Expand Down Expand Up @@ -29820,7 +29821,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
print("{}")
return
}
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId)
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId, surfaceId: surfaceId)
if !suppressVisibleMutations {
if codexSessionStartWentStaleAfterAccept() {
telemetry.breadcrumb("\(def.name)-hook.session-start.stale-after-turn")
Expand Down Expand Up @@ -30083,7 +30084,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
stopStaleCodexPromptSubmit()
return
}
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId)
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId, surfaceId: surfaceId)
if !sessionId.isEmpty, !suppressVisibleMutations {
let acceptedRunningUpdate: Bool
if def.name == "codex" {
Expand Down Expand Up @@ -30231,7 +30232,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
}
let workspaceId = target.workspaceId
let surfaceId = target.surfaceId
sendAgentFeedTelemetry(workspaceId: workspaceId)
sendAgentFeedTelemetry(workspaceId: workspaceId, surfaceId: surfaceId)
let pid = mapped?.pid ?? inferredPID
let codexFailure: CodexHookFailureSummary?
let codexSubagentSignals: CodexTranscriptSubagentSignals
Expand Down Expand Up @@ -30551,7 +30552,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
}
let workspaceId = target.workspaceId
let surfaceId = target.surfaceId
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId)
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId, surfaceId: surfaceId)
let pid = mapped?.pid ?? inferredPID
let launchCommand = agentLaunchCommandFromEnvironment(
env,
Expand Down Expand Up @@ -30692,7 +30693,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
env: env
)
#endif
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId)
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId, surfaceId: surfaceId)
print("{}")
return
}
Expand All @@ -30707,7 +30708,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
env: env
)
#endif
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId)
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId, surfaceId: surfaceId)
print("{}")
return
}
Expand Down Expand Up @@ -30854,15 +30855,15 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
case nil:
break
}
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId)
sendAgentFeedTelemetryUnlessSuppressed(workspaceId: workspaceId, surfaceId: surfaceId)

case .sessionEnd:
if def.name == "codex", !sessionId.isEmpty {
retireCodexMonitorLeases(sessionId: sessionId, turnId: nil, env: env)
}
if def.sessionEndIsTurnBoundary {
if let mapped = sessionId.isEmpty ? nil : (try? store.lookup(sessionId: sessionId)) {
sendAgentFeedTelemetry(workspaceId: mapped.workspaceId)
sendAgentFeedTelemetry(workspaceId: mapped.workspaceId, surfaceId: mapped.surfaceId)
_ = try? store.recordPromptStop(
sessionId: sessionId,
workspaceId: mapped.workspaceId,
Expand Down Expand Up @@ -30932,6 +30933,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
subcommand: String,
parsedInput: ClaudeHookParsedInput,
workspaceId: String? = nil,
surfaceId: String? = nil,
socketPassword: String? = nil
) {
let hookEventName = Self.feedEventName(forClaudeSubcommand: subcommand)
Expand All @@ -30955,6 +30957,12 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
if let workspaceId = feedWorkspaceId(rawObject: parsedInput.object, fallback: workspaceId) {
event["workspace_id"] = workspaceId
}
if let surfaceId, !surfaceId.isEmpty {
event["surface_id"] = surfaceId
}
if let transcriptPath = parsedInput.transcriptPath, !transcriptPath.isEmpty {
event["transcript_path"] = transcriptPath
}
if let cwd = parsedInput.cwd { event["cwd"] = cwd }
let toolName = parsedInput.object?["tool_name"] as? String
if let toolName, !toolName.isEmpty {
Expand Down Expand Up @@ -33538,6 +33546,12 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
}
let actionArgs = Array(rest.dropFirst())
switch action {
case "inject-args" where def.name == "codex":
// Hidden: emit the NUL-separated codex arg list the wrapper
// (Resources/bin/cmux-codex-wrapper) splices to inject cmux's
// fire-and-forget hooks for one invocation. No socket required.
try emitCodexWrapperInjectArgs()
return true
case "install":
try installHooksForAgent(def, arguments: actionArgs)
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
/// Timestamp of the most recent transcript or hook activity.
public let lastActivityAt: Date?

/// 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

/// Creates a session descriptor.
///
/// - Parameters:
Expand Down Expand Up @@ -86,7 +93,8 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
terminalID: String? = nil,
workingDirectory: String? = nil,
state: ChatAgentState = .idle,
lastActivityAt: Date? = nil
lastActivityAt: Date? = nil,
version: Int = 0
) {
self.id = id
self.agentKind = agentKind
Expand All @@ -97,6 +105,7 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
self.workingDirectory = workingDirectory
self.state = state
self.lastActivityAt = lastActivityAt
self.version = version
}

/// A copy with a new live state, leaving identity and bindings intact.
Expand All @@ -115,7 +124,8 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
terminalID: terminalID,
workingDirectory: workingDirectory,
state: newState,
lastActivityAt: lastActivityAt
lastActivityAt: lastActivityAt,
version: version
)
}

Expand All @@ -129,6 +139,7 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
case workingDirectory = "cwd"
case state
case lastActivityAt = "last_activity_at"
case version
}

// Custom Codable so `kind` decodes with a `.agent` default when absent
Expand All @@ -145,6 +156,7 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
workingDirectory = try container.decodeIfPresent(String.self, forKey: .workingDirectory)
state = try container.decode(ChatAgentState.self, forKey: .state)
lastActivityAt = try container.decodeIfPresent(Date.self, forKey: .lastActivityAt)
version = try container.decodeIfPresent(Int.self, forKey: .version) ?? 0
}

public func encode(to encoder: any Encoder) throws {
Expand All @@ -158,5 +170,6 @@ public struct ChatSessionDescriptor: Identifiable, Sendable, Equatable, Codable
try container.encodeIfPresent(workingDirectory, forKey: .workingDirectory)
try container.encode(state, forKey: .state)
try container.encodeIfPresent(lastActivityAt, forKey: .lastActivityAt)
try container.encode(version, forKey: .version)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ public struct ChatSessionListReducer: Sendable {
}
var updated = sessions
if let index = updated.firstIndex(where: { $0.id == descriptor.id }) {
// Version-gated upsert: best-effort pushes can arrive out of
// order, be duplicated, or race an authoritative pull. The host
// stamps a strictly increasing `version` on every change, so a
// descriptor whose version is LOWER than the one already
// applied is stale (or out of order) and must not clobber newer
// state the client got from a later push or a snapshot pull.
// Equal version is allowed through (a no-op in practice: the
// monotonic counter guarantees equal version == identical
// content), which also keeps unversioned (version 0) payloads
// upserting as before.
guard descriptor.version >= updated[index].version else {
return sessions
}
Comment on lines +41 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Equal-version upserts can re-apply stale descriptor payloads after a newer state transition.

Line 51 accepts descriptor.version == updated[index].version. But Line 66 applies .stateChanged without advancing version, so a delayed old .descriptorChanged at the same version can overwrite the newer state. For versioned payloads, this should be strict >; keep equality only for legacy 0/0.

💡 Suggested fix
             if let index = updated.firstIndex(where: { $0.id == descriptor.id }) {
-                guard descriptor.version >= updated[index].version else {
+                let currentVersion = updated[index].version
+                let incomingVersion = descriptor.version
+                let allowLegacyUnversionedEqual = (currentVersion == 0 && incomingVersion == 0)
+                guard incomingVersion > currentVersion || allowLegacyUnversionedEqual else {
                     return sessions
                 }
                 updated[index] = descriptor
             } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Version-gated upsert: best-effort pushes can arrive out of
// order, be duplicated, or race an authoritative pull. The host
// stamps a strictly increasing `version` on every change, so a
// descriptor whose version is LOWER than the one already
// applied is stale (or out of order) and must not clobber newer
// state the client got from a later push or a snapshot pull.
// Equal version is allowed through (a no-op in practice: the
// monotonic counter guarantees equal version == identical
// content), which also keeps unversioned (version 0) payloads
// upserting as before.
guard descriptor.version >= updated[index].version else {
return sessions
}
// Version-gated upsert: best-effort pushes can arrive out of
// order, be duplicated, or race an authoritative pull. The host
// stamps a strictly increasing `version` on every change, so a
// descriptor whose version is LOWER than the one already
// applied is stale (or out of order) and must not clobber newer
// state the client got from a later push or a snapshot pull.
// Equal version is allowed through (a no-op in practice: the
// monotonic counter guarantees equal version == identical
// content), which also keeps unversioned (version 0) payloads
// upserting as before.
if let index = updated.firstIndex(where: { $0.id == descriptor.id }) {
let currentVersion = updated[index].version
let incomingVersion = descriptor.version
let allowLegacyUnversionedEqual = (currentVersion == 0 && incomingVersion == 0)
guard incomingVersion > currentVersion || allowLegacyUnversionedEqual else {
return sessions
}
updated[index] = descriptor
} else {
🤖 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/Store/ChatSessionListReducer.swift`
around lines 41 - 53, The guard condition checking descriptor.version >=
updated[index].version allows equal versions to pass through, which causes a
race condition where a delayed old .descriptorChanged with the same version can
overwrite newer state applied by .stateChanged (which doesn't advance the
version). Change the guard statement to use strict greater-than comparison (>)
for versioned payloads, but preserve the equality check only for legacy
unversioned payloads where both versions are 0. This prevents equal-version
stale descriptors from clobbering newer state while maintaining backward
compatibility.

updated[index] = descriptor
} else {
updated.append(descriptor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,28 @@ struct ChatSessionListReducerTests {
)
#expect(reducer.applying(frame, to: seed).count == 1)
}

@Test("a lower-version descriptorChanged is dropped; a higher one applies")
func versionGatedUpsert() {
let reducer = ChatSessionListReducer(workspaceID: "ws-1")
func desc(_ state: ChatAgentState, _ version: Int) -> ChatSessionDescriptor {
ChatSessionDescriptor(
id: "s1", agentKind: .claude, workspaceID: "ws-1",
terminalID: "s1", state: state, version: version
)
}
// Seed at version 5 (working).
let seed = [desc(Self.working, 5)]
// A stale push (version 3, idle) arrives out of order and is dropped:
// the newer working state the client already holds must survive.
let stale = ChatSessionEventFrame(sessionID: "s1", event: .descriptorChanged(desc(.idle, 3)))
let afterStale = reducer.applying(stale, to: seed)
#expect(afterStale.first?.state == Self.working)
#expect(afterStale.first?.version == 5)
// A newer push (version 6, ended) applies.
let newer = ChatSessionEventFrame(sessionID: "s1", event: .descriptorChanged(desc(.ended, 6)))
let afterNewer = reducer.applying(newer, to: afterStale)
#expect(afterNewer.first?.state == .ended)
#expect(afterNewer.first?.version == 6)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,29 @@ public actor MobileChatEventSource: ChatEventSource {
return try coding.decode(MobileChatSessionsResponse.self, from: result).sessions
}

/// Pulls the authoritative snapshot of one session by id.
///
/// The client's reconcile path: on (re)connect, foreground, a detected
/// version gap, or manual refresh, the host fetches the current descriptor
/// and folds it through the same version-gated upsert as a push, so a pull
/// that races a newer push converges. Pull is authoritative; push is a
/// best-effort hint, so a missed or out-of-order push self-heals here.
///
/// Not part of ``CmuxAgentChat/ChatEventSource`` (which is scoped to one
/// conversation). Throws when the host no longer knows the session (treat
/// as gone) or the request fails.
///
/// - Parameter sessionID: The session to snapshot.
/// - Returns: The session's current descriptor, with its `version`.
public func session(sessionID: String) async throws -> ChatSessionDescriptor {
let request = try MobileCoreRPCClient.requestData(
method: "mobile.chat.session",
params: ["session_id": sessionID]
)
let result = try await client.sendRequest(request)
return try coding.decode(MobileChatSessionResponse.self, from: result).session
}

/// Opens the live stream of session-list events for every session the
/// Mac knows about (not scoped to one conversation).
///
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
public import CmuxAgentChat

/// Result payload of `mobile.chat.session` (single-session snapshot pull).
public struct MobileChatSessionResponse: Sendable, Equatable, Codable {
/// The authoritative current descriptor for the requested session,
/// carrying the host's monotonic `version` for reconciliation.
public let session: ChatSessionDescriptor

/// Creates a response.
///
/// - Parameter session: The session descriptor.
public init(session: ChatSessionDescriptor) {
self.session = session
}

private enum CodingKeys: String, CodingKey {
case session
}
}
Loading