Fix hooks feed hangs for unsupported agents - #8968
Conversation
📝 WalkthroughWalkthroughChangesThe PR removes Antigravity tool-gating hooks, makes unknown feed sources telemetry-only, adds Gemini-specific approval semantics, and propagates absolute deadlines through socket, relay, authentication, and feed-hook operations. Tests cover updated hook installation, classification, and non-blocking behavior. Feed Hook Safety
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FeedHook
participant SocketClient
participant cmuxDaemon
FeedHook->>SocketClient: connect with client deadline
SocketClient->>cmuxDaemon: authenticate and send feed command
cmuxDaemon-->>SocketClient: bounded feed response
SocketClient-->>FeedHook: response or timeout
Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors)
✅ Passed checks (23 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 |
Greptile SummaryThis PR prevents unsupported agent hooks from blocking and bounds supported feed decisions within a shared deadline.
Confidence Score: 5/5The PR appears safe to merge. The previously reported setup and partial-write deadline escapes are closed by propagating the same absolute deadline through connection, authentication, writes, and response reads; no blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent
participant Hook as cmux hooks feed
participant Socket as SocketClient
participant App as cmux app
Agent->>Hook: Invoke generated hook (120s limit)
Hook->>Hook: Create absolute client deadline (118s)
Hook->>Socket: Connect using remaining deadline
Socket->>App: Authenticate using remaining deadline
Hook->>App: Send feed event using remaining deadline
alt Decision arrives before deadline
App-->>Hook: Approval response
Hook-->>Agent: Agent-specific decision output
else Any phase expires or fails
Hook-->>Agent: "Neutral {}"
end
Reviews (12): Last reviewed commit: "fix: address hook deadline review feedba..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 2423-2478: Update remainingSocketTimeout to bound the effective
timeout by both responseTimeout and the deadline’s remaining duration, matching
the min(timeout, remaining) behavior used by boundedTimeout in send(). Preserve
the existing default response timeout when no phase-specific timeout is
provided, and continue throwing when the resulting deadline budget is exhausted.
- Around line 1933-1957: Consolidate the duplicated retry/backoff logic by
making the existing connect() delegate to connect(deadline:) using an unbounded
deadline, while ensuring connect(deadline:) does not reject attempts solely
because that sentinel deadline has no expiration. Preserve the shared
shouldRetryConnect gate, retry interval, and per-attempt timeout behavior, with
finite deadlines still enforcing expiration.
In `@cmuxTests/CLIHookNoResponseTests.swift`:
- Line 223: Increase largeToolInput in the no-response socket test to a payload
size known to exceed platform socket buffers, or update
startAcceptedSocketThatDoesNotRead to configure a deliberately small receive
buffer. Ensure the peer still does not read and the client-side write deadline
is deterministically exercised.
🪄 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 Plus
Run ID: a37a7ce2-faf5-4782-81dd-7864c707095f
📒 Files selected for processing (7)
CLI/CMUXCLI+AgentHookCatalog.swiftCLI/CMUXCLI+AgentHookDefinitions.swiftCLI/FeedEventClassifier.swiftCLI/cmux.swiftcmuxTests/CLIGenericHookPersistenceTests.swiftcmuxTests/CLIHookNoResponseTests.swiftcmuxTests/FeedEventClassificationTests.swift
|
Follow-up triage for the latest CodeRabbit custom-check summary on
The canonical structured review found no actionable defects, Greptile re-reviewed this exact head at 5/5 and confirmed all blocking deadline escapes are closed, and all required PR checks are passing. |
Summary
Testing
Localization
Closes #8921
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes hooks feed hangs by making blocking approvals opt in per agent and enforcing a single absolute client deadline across connect/auth/read/write (socket and relay) and retries, so unsupported or stalled hooks fail neutral. Closes #8921.
geminiopts in;antigravity/cursortool-starts are non-blocking.PreToolUse/PostToolUsefeed hooks forantigravity.antigravity/cursortelemetry paths.Written for commit 87c247d. Summary will update on new commits.
Summary by CodeRabbit