Skip to content
Open
Changes from all commits
Commits
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
45 changes: 39 additions & 6 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ private struct ClaudeHookSessionRecord: Codable {
var lastBody: String?
var startedAt: TimeInterval
var updatedAt: TimeInterval
/// Timestamp of the most recent pre-tool-use event. Used to suppress
/// premature stop-hook notifications when Claude auto-continues mid-task.
var lastToolUseAt: TimeInterval?
}

private struct ClaudeHookSessionStoreFile: Codable {
Expand Down Expand Up @@ -373,7 +376,8 @@ private final class ClaudeHookSessionStore {
cwd: String?,
pid: Int? = nil,
lastSubtitle: String? = nil,
lastBody: String? = nil
lastBody: String? = nil,
lastToolUseAt: TimeInterval? = nil
) throws {
let normalized = normalizeSessionId(sessionId)
guard !normalized.isEmpty else { return }
Expand Down Expand Up @@ -406,6 +410,9 @@ private final class ClaudeHookSessionStore {
if let body = normalizeOptional(lastBody) {
record.lastBody = body
}
if let lastToolUseAt {
record.lastToolUseAt = lastToolUseAt
}
record.updatedAt = now
state.sessions[normalized] = record
}
Expand Down Expand Up @@ -10328,11 +10335,24 @@ struct CMUXCLI {
}

if let completion {
let title = "Claude Code"
let subtitle = sanitizeNotificationField(completion.subtitle)
let body = sanitizeNotificationField(completion.body)
let payload = "\(title)|\(subtitle)|\(body)"
_ = try? sendV1Command("notify_target \(workspaceId) \(surfaceId) \(payload)", client: client)
// Suppress notification if a tool was used very recently — this
// indicates an auto-continue turn (Claude stopped briefly between
// tool calls) rather than true task completion. The threshold of
// 2 seconds is generous: auto-continue stop→pre-tool-use gaps are
// typically < 500 ms, while genuine completions require user think-time.
let shouldNotify: Bool
if let lastToolUse = mappedSession?.lastToolUseAt {
shouldNotify = Date().timeIntervalSince1970 - lastToolUse > 2.0
} else {
shouldNotify = true
}
if shouldNotify {
Comment on lines +10338 to +10349
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This is a session-wide cooldown, not a turn-local signal.

Line 10491 only sets lastToolUseAt, and Line 10344 then treats it as a 2-second global cooldown. That means a later stop in the same Claude session can suppress a real completion even when the current turn never used a tool, e.g. a quick no-tool follow-up right after prompt-submit.

Also applies to: 10488-10499

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 10338 - 10349, The cooldown logic wrongly treats
mappedSession?.lastToolUseAt as a session-wide timestamp, so a prior tool use in
any earlier turn can suppress a later legitimate completion; change the logic to
be turn-local by either resetting lastToolUseAt at turn start or, better,
introduce and use a per-turn flag (e.g., toolUsedThisTurn or a turn-scoped
lastToolUseAt on the current turn object) when computing shouldNotify instead of
the session-wide mappedSession?.lastToolUseAt; update the code paths that set
lastToolUseAt (the stop/tool handlers around lines that set lastToolUseAt) to
set the per-turn marker and modify the shouldNotify computation (the block that
computes shouldNotify using Date().timeIntervalSince1970 and lastToolUseAt) to
consult that turn-local marker so only tool use in the same turn suppresses
notification.

let title = "Claude Code"
let subtitle = sanitizeNotificationField(completion.subtitle)
let body = sanitizeNotificationField(completion.body)
let payload = "\(title)|\(subtitle)|\(body)"
_ = try? sendV1Command("notify_target \(workspaceId) \(surfaceId) \(payload)", client: client)
Comment on lines +10350 to +10354
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize the new completion notification title.

This "Claude Code" literal is sent through notify_target and shown in the app notification UI, so it shouldn't introduce another hard-coded English string here.

As per coding guidelines, **/*.swift: All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") for every string shown in the UI.`

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 10350 - 10354, Replace the hard-coded title
"Claude Code" with a localized string using String(localized: ..., defaultValue:
...); update the code where title is defined (the title variable near
sanitizeNotificationField and sendV1Command) to call String(localized:
"notification.claudeCode.title", defaultValue: "Claude Code") (or an appropriate
key) so the payload passed to sendV1Command("notify_target \(workspaceId)
\(surfaceId) \(payload)", client: client) contains the localized title while
leaving sanitizeNotificationField, workspaceId, surfaceId and sendV1Command
calls unchanged.

}
Comment on lines 10337 to +10355
Copy link

Choose a reason for hiding this comment

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

P2 2-second window can suppress genuine completion notifications

The heuristic correctly handles the common case, but the 2-second threshold measures the gap between pre-tool-use and stop — which includes both tool execution time AND LLM inference time for the same turn.

For fast tool calls (e.g. read_file completing in ~50 ms) followed by a short Claude response (LLM inference ~0.5–1 s for a brief reply), stop can fire well within 2 seconds of the last pre-tool-use. In that scenario the final-turn notification is silently suppressed even though the task is genuinely complete.

Concretely: pre-tool-use at T → read_file runs in 0.05 s → Claude generates a 30-word reply in 0.7 s → stop at T + 0.75 s → 0.75 < 2.0 → notification dropped.

The PR description notes "genuine completions require user think-time", which holds when Claude produces a long response, but not for short, fast final replies. Consider documenting this known limitation and/or providing a mechanism to tune the threshold via a user default (similar to claudeCodeVerboseStatus), so users with consistently fast pipelines can lower it.

}

try? setClaudeStatus(
Expand Down Expand Up @@ -10465,6 +10485,19 @@ struct CMUXCLI {
)
let claudePid = mappedSession?.pid

// Record the timestamp of this tool use so the stop handler can detect
// auto-continue turns (stop fired immediately after a tool use) and
// suppress premature notifications.
if let sessionId = parsedInput.sessionId, let existingSession = mappedSession {
try? sessionStore.upsert(
sessionId: sessionId,
workspaceId: workspaceId,
surfaceId: existingSession.surfaceId,
cwd: parsedInput.cwd,
lastToolUseAt: Date().timeIntervalSince1970
)
}
Comment on lines +10491 to +10499
Copy link

Choose a reason for hiding this comment

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

P2 lastToolUseAt not reset on prompt-submit

lastToolUseAt persists across consecutive turns within the same Claude Code session (sessions share a sessionId). If Turn N's last pre-tool-use fired at time T, and the user immediately sends a new prompt (Turn N+1), and Claude responds without any tool calls, stop for Turn N+1 fires with the stale lastToolUseAt = T. If the elapsed time is still < 2 s, the notification is suppressed.

Practical impact: A user who quickly follows up after a tool-heavy turn and gets a fast tool-free reply could silently miss the completion notification.

Adding a lastToolUseAt = nil reset inside the prompt-submit case would close this gap:

// prompt-submit case — after clearing notifications
if let sessionId = parsedInput.sessionId, let existingSession = mappedSession {
    try? sessionStore.upsert(
        sessionId: sessionId,
        workspaceId: workspaceId,
        surfaceId: existingSession.surfaceId,
        cwd: parsedInput.cwd
        // lastToolUseAt not passed → remains nil after explicit reset
    )
}

This would require adding an explicit "clear" path (e.g. a resetLastToolUseAt: Bool = false flag to upsert) rather than the current nil-means-no-change pattern.


// AskUserQuestion means Claude is about to ask the user something.
// Save question text in session so the Notification handler can use it
// instead of the generic "Claude Code needs your attention".
Expand Down