fix(cli): suppress premature stop-hook notifications during auto-continue#2146
fix(cli): suppress premature stop-hook notifications during auto-continue#2146anthhub wants to merge 1 commit intomanaflow-ai:mainfrom
Conversation
…inue turns Track the timestamp of each pre-tool-use event in ClaudeHookSessionRecord. In the stop handler, skip notify_target if the last tool use occurred within the past 2 seconds — indicating Claude is mid-task (auto-continue) rather than truly finished. Genuine completions always have > 2 s of user think-time before the next turn, so the threshold is safe for normal workflows. Fixes manaflow-ai#2077
|
@anthhub is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughExtended session tracking to record the timestamp of the last tool use, then implemented a heuristic in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes spurious "Completed" desktop notifications that fired on every Claude Code turn (including mid-task auto-continue pauses) by introducing a timestamp-based heuristic: the Key changes in
Two edge cases to be aware of:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CC as Claude Code
participant H as cmux Hook CLI
participant S as SessionStore
participant N as notify_target
rect rgb(200, 230, 200)
note over CC,N: Auto-continue turn (notification suppressed)
CC->>H: pre-tool-use (turn N)
H->>S: upsert(lastToolUseAt = T)
CC->>H: stop (turn N, at T+0.4s)
H->>S: lookup → lastToolUseAt = T
note over H: now - T = 0.4s < 2.0s → skip
CC->>H: pre-tool-use (turn N+1, at T+0.9s)
H->>S: upsert(lastToolUseAt = T+0.9)
end
rect rgb(200, 210, 255)
note over CC,N: Genuine completion (notification fires)
CC->>H: stop (final, at T+0.9+3s)
H->>S: lookup → lastToolUseAt = T+0.9
note over H: now - (T+0.9) = 3s > 2.0s → notify
H->>N: notify_target workspaceId surfaceId payload
end
rect rgb(255, 230, 200)
note over CC,N: Edge case — fast final tool + short reply
CC->>H: pre-tool-use (last tool, at T2)
H->>S: upsert(lastToolUseAt = T2)
CC->>H: stop (at T2+0.8s, short reply)
H->>S: lookup → lastToolUseAt = T2
note over H: now - T2 = 0.8s < 2.0s → skip ⚠️ missed!
end
Reviews (1): Last reviewed commit: "fix(cli): suppress premature stop-hook n..." | Re-trigger Greptile |
| 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 { | ||
| 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) | ||
| } |
There was a problem hiding this comment.
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.
| if let sessionId = parsedInput.sessionId, let existingSession = mappedSession { | ||
| try? sessionStore.upsert( | ||
| sessionId: sessionId, | ||
| workspaceId: workspaceId, | ||
| surfaceId: existingSession.surfaceId, | ||
| cwd: parsedInput.cwd, | ||
| lastToolUseAt: Date().timeIntervalSince1970 | ||
| ) | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 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.
- Around line 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.
| // 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 { |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
Problem
Closes #2077
The
claude-hook stophandler fires on every Claude Code turn, not just when the task is fully complete. This caused spurious desktop notifications mid-task: each time Claude finished a response and paused (even briefly between tool calls), the user received a "Completed" notification.Root cause
CLI/cmux.swiftstop handler callsnotify_targetunconditionally wheneverstopfires. Claude Code'sstophook is a per-turn lifecycle event; there is no built-in way to distinguish "Claude paused between tool calls (auto-continue)" from "Claude finished the task and is waiting for the human."Fix: timestamp-based deduplication
Track the last
pre-tool-useevent time in the session record. In the stop handler, skip the notification if a tool was used within the past 2 seconds:stopfires immediately after Claude finishes a response, thenpre-tool-usefires within ~500 ms as Claude resumes. When the nextstoparrives,lastToolUseAtis fresh — notification is suppressed.pre-tool-useupdates the timestamp, so the notification fires normally.lastToolUseAtisnil→ notification fires as before (no regression).Changes
CLI/cmux.swiftlastToolUseAt: TimeInterval?field toClaudeHookSessionRecordCLI/cmux.swiftlastToolUseAtparameter toClaudeHookSessionStore.upsert()CLI/cmux.swiftpre-tool-usehandler: recordDate().timeIntervalSince1970on every tool callCLI/cmux.swiftstophandler: guardnotify_targetbehind 2-second staleness checkThe
notificationhook path, OSC 777, and Codex notification paths are not affected.Test plan
lastToolUseAtis nil). Verify notification fires normally.notificationhook (e.g. Claude usesAskUserQuestion). Verify notification is unaffected by this change.🤖 Generated with Claude Code
Summary by cubic
Suppresses premature "Completed" notifications during auto-continue by gating the stop-hook. Users now only get a completion alert when the task actually finishes.
lastToolUseAton eachpre-tool-useevent and store it in the session.stophandler, sendnotify_targetonly if the last tool use was > 2 seconds ago or absent.notificationhook, OSC 777, or Codex notification paths.Written for commit fc64059. Summary will update on new commits.
Summary by CodeRabbit