Repository navigation
Fix #5286: surface needs-input attention for feed-routed blocking decisions - #5313
Merged
Merged
Conversation
…ention Adds a regression test asserting that FeedCoordinator.ingestBlocking, for a blocking PermissionRequest decision routed through `cmux hooks feed`, requests in-app attention surfacing (needs-input status + bell + elevation). This fails today because the feed decision bridge only ingests the card and posts an inactive-app banner — it never drives the central attention path that the `cmux hooks claude notification` hook uses (#5286). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When Claude Code (or any agent) blocks mid-turn on a PermissionRequest, ExitPlanMode, or AskUserQuestion, the hook fires `cmux hooks feed` which sends a blocking `feed.push`. The app ingested the Feed card and (when inactive) posted a banner, but never drove the in-app attention path that the `cmux hooks <agent> notification` hook uses — so there was no sidebar "Needs input" status, no bell, and no tab/workspace elevation while the app was active. The PreToolUse→PermissionRequest migration left this convergence behind. Fix it at the single choke point every feed-routed blocking decision flows through: FeedCoordinator.ingestBlocking now calls surfaceBlockingDecisionAttention for any blocking-decision event, which - flips the owning workspace's agent lifecycle to .needsInput and sets the localized "Needs input" sidebar status (source→key mapped: claude→claude_code so the existing per-agent resume hooks clear it), - elevates the workspace when Reorder on Notification is enabled, - rings the bell. This fires regardless of app focus; the existing inactive-app banner (with its inline allow/deny actions) remains the desktop surface. Driving it once, for every blocking-decision event, eliminates the class where a new feed event type silently swallows attention. Fixes #5286. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSurfaces in-app “needs input” attention for blocking feed events: tracks refcounts per target, sets workspace lifecycle to .needsInput, writes a sidebar status entry, optionally reorders the tab, requests user attention, adds a localized string, and includes tests and a DEBUG hook. ChangesBlocking decision attention surfacing
Sequence DiagramsequenceDiagram
participant FeedCoordinator
participant WorkstreamEvent
participant Workspace
participant Sidebar
participant TabManager
participant System
FeedCoordinator->>WorkstreamEvent: ingestBlocking(event)
FeedCoordinator->>FeedCoordinator: surfaceBlockingDecisionAttention(event)
FeedCoordinator->>Workspace: set lifecycle to .needsInput (store previous)
FeedCoordinator->>Sidebar: write status entry "Needs input"
FeedCoordinator->>TabManager: optionally move workspace tab to top
FeedCoordinator->>System: request user attention (NSApp.requestUserAttention)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryFixes #5286 by converging all feed-routed blocking decisions (
Confidence Score: 5/5Safe to merge; attention surfacing is well-scoped to FeedCoordinator with correct refcounting, race-free target recording, and guarded cleanup. The convergence logic in ingestBlocking is carefully sequenced: disk I/O is moved off the main.sync critical section, the AttentionTarget is stored on the waiter inside main.sync before any reply can fire, and concludeBlockingDecisionAttention guards every mutation with a value check so agent hooks always win. Double-conclude is prevented by the pendingAttentionStates guard. Localization covers all 20 catalog locales. Sources/Feed/FeedCoordinator.swift — grew significantly; the attention-surfacing extension could be split into its own file if the team wants to keep file sizes in check. Important Files Changed
Sequence DiagramsequenceDiagram
participant Hook as CLI Hook (socket thread)
participant FC as FeedCoordinator
participant Main as Main Thread
participant UI as Workspace / Sidebar
Hook->>FC: ingestBlocking(event, timeout)
FC->>FC: resolveAttentionTarget() [disk I/O, off-main]
FC->>Main: DispatchQueue.main.sync
Main->>UI: store.ingest(event)
Main->>UI: surfaceBlockingDecisionAttention()
UI-->>UI: setAgentLifecycle(.needsInput)
UI-->>UI: statusEntries[Needs input]
UI-->>UI: moveTabToTopForNotification (if enabled)
UI-->>UI: NSApp.requestUserAttention
Main->>FC: "waiters[requestId].attentionTarget = target"
Main-->>Hook: main.sync returns
Hook->>Hook: semaphore.wait(timeout)
alt User decides (deliverReply)
Main->>FC: deliverReply(requestId, decision)
FC->>FC: signal semaphore + read attentionTarget
FC->>Main: concludeAttentionOnMain(target) async
Main->>UI: concludeBlockingDecisionAttention()
UI-->>UI: reset lifecycle if still .needsInput
UI-->>UI: remove status entry if still Needs input
Hook-->>Hook: semaphore.wait success resolved
else Timeout
Hook-->>Hook: semaphore.wait timedOut
FC->>Main: concludeAttentionOnMain(target) async
Main->>UI: concludeBlockingDecisionAttention()
end
Reviews (10): Last reviewed commit: "Convert merged attention tests to Swift ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…ce via session store - Make the needs-input overlay fully transient: capture the prior agent lifecycle + sidebar status when surfacing a blocking decision and restore it when the decision concludes (resolved via deliverReply, or timed out in ingestBlocking). Previously the lifecycle/status were never cleared on timeout, leaving a stale "Needs input" badge (cursor[bot]). The overlay context rides on the existing PendingWaiter (no new lock). - Resolve the owning workspace from the agent's hook-session store as a fallback when the event omits workspace_id, instead of exiting early (cursor[bot]). - Map the session-store surface id to its owning panel id. - Add unit tests for the blocking-decision predicate and the source→status-key mapping (documents the claude→claude_code special case, greptile). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The snapshot/restore approach (capture prior lifecycle on surface, restore on conclusion) had two concurrency hazards the bots correctly flagged: - The waiter context was stored after main.sync returned, racing deliverReply (cursor/greptile P1): a fast reply could read a nil context, skip restore, and the .resolved path returned assuming restore already ran → stuck badge. - Restoring a captured snapshot unconditionally clobbered a newer overlapping decision's needs-input on the same panel (cubic P2). Remove the snapshot/restore machinery entirely (BlockingAttentionContext, the waiter field, restore helpers, the context slot). surfaceBlockingDecisionAttention now only *sets* the needs-input overlay, mirroring the existing `cmux hooks <agent> notification` path. Clearing back to running/idle is owned by the agent's own lifecycle hooks under the same statusKey (Claude's pre-tool-use on resume, notification re-asserting after a timed-out TUI fallback, stop on turn end). Single owner for lifecycle transitions = no race, no clobber. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 299-302: When resolveAttentionTarget(event:) returns nil the
function silently returns; update FeedCoordinator to capture that failure by
adding a debug log or telemetry breadcrumb before returning. Specifically, in
the guard that uses Self.resolveAttentionTarget(event: event), detect the nil
case and emit a telemetry/log entry containing the event.sessionId and
event.hookEventName (and any other useful context like event.id or timestamp),
then return as before; reference the existing guard and the
resolveAttentionTarget(event:) symbol so the change is localized and
non-invasive.
🪄 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: 0b54cf61-c7d2-4884-930b-2bc59f96eb6b
📒 Files selected for processing (1)
Sources/Feed/FeedCoordinator.swift
…kspace Add a DEBUG breadcrumb when surfaceBlockingDecisionAttention can't resolve an owning workspace and returns early. This PR is about silent swallowing, so making the one remaining silent return observable (session id + hook event + workspace) helps diagnose field cases where a blocking decision fails to surface. Wrapped in #if DEBUG per the debug-log policy (coderabbitai). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two follow-up findings (cursor): (1) needs-input could stay stuck after an approval for agents without a resume hook (e.g. Codex), since the feed relied on agent hooks to clear it; (2) a stale hook-session map could win over the live workspace_id and target the wrong workspace. Reintroduce a feed-side clear, but correct this time: - surfaceBlockingDecisionAttention returns an AttentionTarget and the target is stored on the waiter INSIDE the ingest main.sync (before the card renders / a reply can fire), closing the race the prior attempt had. - Clearing is refcounted per (workspace, panel, statusKey): overlapping decisions keep the badge lit until the last concludes — no clobber. - concludeBlockingDecisionAttention is check-and-clear: it only sets lifecycle back to .running if it's still .needsInput, and removes the status entry only if it still holds our "Needs input" value, so a real agent-hook update always wins. - deliverReply concludes on resolve; the ingestBlocking timeout branches conclude on timeout — exactly once per surfaced decision. - resolveAttentionTarget now prefers the live event workspace_id over the session store, and only trusts the session surface when its workspace matches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ends The sidebar status entry is workspace-level (keyed only by statusKey), so two panels running the same agent share it. The per-(workspace, panel, statusKey) refcount protected the per-panel lifecycle but not the shared status entry: concluding one panel's decision could wipe another panel's active "Needs input" badge (cubic). Guard the status removal on no other panel in the same workspace having a pending decision under the same key. Lifecycle clearing stays per-panel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7e9a0eb. Configure here.
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Main migrated FeedCoordinatorTests to Swift Testing while this branch added XCTest-style tests; the merge left XCTest symbols out of scope. Match the suite's semaphore + #expect house pattern and accept the FeedCoordinator growth in the file-length budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 9, 2026
austinywang
added a commit
that referenced
this pull request
Jun 10, 2026
After the PreToolUse->PermissionRequest migration (#5313), Claude Code's blocking decisions surface "Needs input" through the feed path (FeedCoordinator.surfaceBlockingDecisionAttention), which sets the sidebar status and per-panel needsInput lifecycle WITHOUT creating a store notification. The notification-store acknowledgement added for #2576 can never reach that state, so clicking into the workspace left the badge stuck on "Needs input". Add FeedCoordinator.acknowledgeBlockingDecisionAttention(workspaceId:panelId:) — the feed-path analogue of the store mark-read acknowledgement — and call it from TabManager.dismissNotification, the single shared focus/interaction seam every entrypoint (sidebar resume, panel click, terminal interaction) funnels through. Targets are force-concluded regardless of refcount and scoped to the focused panel so sibling panels keep their badge; the decision's later concludeBlockingDecisionAttention becomes a safe no-op. Overlay teardown is factored into a shared clearResolvedBlockingDecisionOverlay helper reused by conclude and acknowledge. Fixes #2576 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 10, 2026
This was referenced Jun 20, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When Claude Code blocks mid-turn on a
PermissionRequest(tool permission,ExitPlanMode,AskUserQuestion), the bundled wrapper firescmux hooks feed --source claude, which sends a blockingfeed.pushand waits. cmux surfaced nothing: no sidebar "Needs input" status, no bell, no tab/workspace elevation (and no banner while the app is active). The command blocked until the 125s timeout and Claude continued without the user ever knowing it asked. Thecmux hooks claude notification|stoppaths work in the same setup — the break is specific to thefeeddecision bridge. (#5286)Minimal local repro (no Claude Code needed)
Blocks waiting for a decision, but nothing surfaces in the UI.
Root cause
The reporter's diagnosis is correct. The
feed.pushblocking bridge and the in-app attention surface diverged during the PreToolUse→PermissionRequest migration.FeedCoordinator.ingestBlockingingested the Feed card and (only when the app is inactive) posted a desktop banner, but never drove the central attention path — agent lifecycleneedsInput+ "Needs input" sidebar status + workspace reorder + bell — that the workingcmux hooks <agent> notificationhook uses.This is a class of bug: every event type routed through
feed(PermissionRequest, ExitPlanMode, AskUserQuestion, and future kinds) silently swallowed in-app attention.Fix
Converge at the single choke point every feed-routed blocking decision flows through.
FeedCoordinator.ingestBlockingnow callssurfaceBlockingDecisionAttention(event:)for any blocking-decision event, which:.needsInputand sets the localized "Needs input" sidebar status, keyed by a source→status-key map (claude→claude_code) so the existing per-agent resume hooks (e.g. Claude'spre-tool-use) clear it once the agent continues;requestUserAttention).This fires regardless of app focus. The existing inactive-app feed banner — with its inline allow/deny actions — remains the desktop surface and the decision channel. Doing the surfacing once, for every blocking-decision event, eliminates the whole class rather than patching
PermissionRequestalone.Tests
Two-commit red/green:
FeedCoordinatorTests.testBlockingIngestSurfacesNeedsInputAttentionForPermissionRequestasserts that a blockingPermissionRequestrouted throughingestBlockingrequests in-app attention surfacing. Red before the fix.Localization
Added
feed.status.needsInput(en: "Needs input", ja: "入力待ち") toResources/Localizable.xcstrings. No other new user-facing strings.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches main-thread UI state, workspace targeting, and blocking-hook timing; wrong targeting or overlay cleanup could hide or stick badges, but scope is localized to FeedCoordinator with refcounting and conservative teardown.
Overview
Blocking
feed.pushdecisions (permissions, plan approval, questions) now drive the same in-app attention path as agent notification hooks: Needs input sidebar status,.needsInputlifecycle, optional workspace reorder, and dock attention—even when the app is focused (inactive-only banners were insufficient).FeedCoordinatorresolves workspace/panel targets (preferring liveworkspace_id, refcounting overlapping decisions), records targets on waiters during ingest, and clears overlays on reply or timeout without clobbering agent-updated status. Addsfeed.status.needsInputlocalization and tests for the blocking ingest path, decision predicate, andclaude→claude_codestatus key mapping.Reviewed by Cursor Bugbot for commit 5988f89. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Surfaces in-app “Needs input” for all feed-routed blocking decisions and clears it on resolve or timeout. Fixes #5286 by aligning
feed.pushwith notification hooks and tightening workspace/panel targeting.FeedCoordinator.ingestBlockingforPermissionRequest,ExitPlanMode, andAskUserQuestion: set.needsInput, show localized “Needs input”, ring the bell, and optionally reorder; inactive-app banner remains.AttentionTargeton the waiter inside ingestmain.sync, conclude exactly once in reply/timeout, and don’t remove the shared workspace badge if another panel under the same status key still pends.main.sync, prefer liveworkspace_id, fall back to the session store only when it matches the resolved workspace, map surface → owning panel, DEBUG-log when a workspace can’t be resolved, and mapclaude→claude_code; addfeed.status.needsInputand tests for the blocking path, decision predicate, and lifecycle key mapping.Written for commit 0443082. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization
Tests