Skip to content

fix: settle Claude Stop reentry after hook block - #15603

Merged
teamleaderleo merged 3 commits into
manaflow-ai:mainfrom
teamleaderleo:fix/claude-stop-hook-active
Sep 29, 2026
Merged

teamleaderleo merged 3 commits into
manaflow-ai:mainfrom
teamleaderleo:fix/claude-stop-hook-active

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes #15595.

Claude marks a final Stop callback with stop_hook_active=true when it is re-entering after a Stop hook was blocked. cmux treated that marker as unfinished work and left the session Running even when no background task remained. The callback now settles Idle based only on authoritative background-work signals.

Testing

  • Added a regression to the Claude Stop hook socket harness covering stop_hook_active=true and the resulting Idle status.
  • Swift syntax parse passed.
  • Python test compilation passed.
  • Native runtime execution is left to PR CI because the local checkout lacks a valid GhosttyKit binary.

Changelog

Fixed: Claude sessions now return to Idle after a final Stop callback that re-enters a previously blocked hook.

Demo Video

Not recorded; this is covered by the socket regression harness.

Checklist

  • Behavior changes have added or updated tests
  • Reviewed locally for shared lifecycle and notification paths

Summary by CodeRabbit

  • Bug Fixes
    • Improved work-status detection so an active stop hook alone no longer indicates pending work.
    • Claude’s status is now set to Idle after the Stop hook completes.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a565102f-1ed9-4788-ab02-8e22cb35e124

📥 Commits

Reviewing files that changed from the base of the PR and between efac47a and 664fe49.

📒 Files selected for processing (1)
  • tests/test_claude_hook_stop_last_assistant.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The Stop-hook pending-work check no longer treats stop_hook_active as evidence of pending work. The test sets that field to true and checks for an Idle status command targeting the payload’s workspace and surface.

Changes

Stop-hook status handling

Layer / File(s) Summary
Pending-work check and Stop-hook validation
CLI/cmux.swift, tests/test_claude_hook_stop_last_assistant.py
hasUnsettledWork is true only when stopFailure is nil and hasPendingBackgroundWork is true. The test sets stop_hook_active to true and checks for an Idle status command for the payload’s workspace and surface.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 664fe

The change addresses sessions remaining Running after a blocked Stop hook, with no identified issue requiring resolution before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 664fe

The change fixes a session that remained Running after work had finished. No new external access was identified. A narrow concurrency question remains: an older Stop callback might mark a newer turn Idle.

Retained concerns

  • Low · reliability · inferred: A reentrant Stop callback that passes the freshness check could overlap a newer turn, then write Idle through the separate lifecycle update. Because Idle is hibernatable, this could weaken containment of a live task. The overlap and its production frequency remain unverified.
Security review details

Security Blast Radius

  • observed — The exercised effect is the lifecycle and status of a resolved Claude session on a workspace and surface. The changed test supplies its own socket and workspace identifiers; it does not introduce a production entrypoint.

Trust Boundaries and Controls

  • observed — Before visible Stop mutation, the handler applies event, session-freshness, and nested-agent guards. The freshness check precedes, rather than forms one atomic operation with, the subsequent lifecycle write.

Hardening Proposals

  • proposed — Consider coupling the session-and-turn freshness check to the lifecycle update before emitting Idle, so an overlapping newer turn cannot be settled by an older Stop callback.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the Claude Stop reentry fix, which is the main change in the pull request.
Description check ✅ Passed The description includes Summary, Testing, Changelog, Demo Video, and Checklist sections. It explains the problem, resulting behavior, regression coverage, validation performed, and remaining native-r…
Linked Issues check ✅ Passed The change satisfies issue #15595. In CLI/cmux.swift, hasUnsettledWork now depends on hasPendingBackgroundWork and no longer treats stop_hook_active == true as pending work. The existing `stop…
Out of Scope Changes check ✅ Passed The changed Swift logic directly fixes the lifecycle condition reported in issue #15595. The Python changes add the matching regression input, isolate CMUX_ environment variables, and assert the res…
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request changes Claude Stop lifecycle handling in CLI/cmux.swift and its regression test. It does not change Cloud terminal creation, persistent cmux-tui transport, manual renderer …
Cmux Swift Actor Isolation ✅ Passed PASS. The production diff only changes the local hasUnsettledWork Boolean expression in CMUXCLI.runClaudeHook. It adds no model, protocol, Sendable reference type, actor annotation, or UI-store …
Cmux Swift Blocking Runtime ✅ Passed The production Swift diff only changes hasUnsettledWork to use hasPendingBackgroundWork. It adds no semaphore, blocking wait, sleep, delayed dispatch, polling loop, main-queue sync, or manual lock…
Cmux Browser Automation Off-Main ✅ Passed PASS: The PR changes only Claude Stop lifecycle handling in CLI/cmux.swift and its Python regression test. The diff adds no browser.* command, WebKit/page wait, main-actor routing, worker browser …
Cmux Expensive Synchronous Load ✅ Passed PASS: The Swift diff only changes hasUnsettledWork in the existing runClaudeHook socket handler from stopFailure == nil && (hasPendingBackgroundWork || stop_hook_active) to `stopFailure == nil &…
Cmux Cache Substitution Correctness ✅ Passed PASS: The production diff only removes parsedInput.rawObject?["stop_hook_active"] from hasUnsettledWork. It continues to use the current hook payload through the existing `hasActiveClaudeBackgroun…
Cmux No Hacky Sleeps ✅ Passed PASS: The PR changes only CLI/cmux.swift and a Python regression test. The production change is Swift, which this check explicitly excludes. The Python diff adds deterministic test input and an Idle…
Cmux Algorithmic Complexity ✅ Passed PASS: The production diff in CLI/cmux.swift only removes a boolean check from hasUnsettledWork; it does not add a collection scan, sorting, filtering, join, or batch rescan. `hasActiveClaudeBackgr…
Cmux Swift Concurrency ✅ Passed The Swift diff only removes stop_hook_active from the existing hasUnsettledWork boolean and adds explanatory comments. It introduces no Dispatch, Combine, completion-handler, or fire-and-forget `T…
Cmux Swift @Concurrent ✅ Passed PASS. The Swift diff only changes the synchronous runClaudeHook(...) throws logic that computes hasUnsettledWork; it adds no async, nonisolated, @concurrent, actor isolation, or new call sit…
Cmux Swift Package Boundaries ✅ Passed PASS: The Swift diff changes one existing Claude Stop lifecycle predicate in CLI/cmux.swift; it removes stop_hook_active from the app’s status, session-store, journal, and notification decision. T…
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only CLI/cmux.swift and tests/test_claude_hook_stop_last_assistant.py. The diff contains no Package.swift, Package.resolved, .gitignore, workflow, Xcode project, or dependency…
Cmux Swift Logging ✅ Passed PASS. The Swift diff changes lifecycle logic and adds comments only. It does not add or materially change print, debugPrint, dump, NSLog, file logging, or Logger declarations. The added `print…
Cmux User-Facing Error Privacy ✅ Passed PASS: The production diff changes Claude lifecycle classification only. It removes stop_hook_active from the pending-work condition and continues to use existing generic Running and Idle status …
Cmux Full Internationalization ✅ Passed PASS: The PR changes lifecycle logic and a regression test only. The existing Swift status values remain routed through String(localized:defaultValue:); the diff adds no user-facing text, localizati…
Cmux Swiftui State Layout ✅ Passed PASS: The PR changes only Claude hook lifecycle logic in CLI/cmux.swift and a Python regression test. The Swift diff replaces the hasUnsettledWork expression and updates comments; it adds no `Obse…
Cmux Architecture Rethink ✅ Passed PASS: The Swift change is a small local correctness fix. It removes stop_hook_active from hasUnsettledWork and keeps hasActiveClaudeBackgroundWork(parsedInput) as the single source of truth for …
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The Swift diff only changes Claude lifecycle state handling in CLI/cmux.swift. It does not add or materially change an NSWindow, NSPanel, NSWindowController, SwiftUI Window/`WindowGrou…
Cmux Source Artifacts ✅ Passed The diff changes only CLI/cmux.swift and tests/test_claude_hook_stop_last_assistant.py. These are intentional product source and regression-test files. No logs, screenshots, recordings, temporary …
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The PR changes only CLI/cmux.swift and tests/test_claude_hook_stop_last_assistant.py. The only changed Swift file is not under a production **/Sources/** path, and the test change is outsi…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

CI failure attribution

CI passes on 664fe49ef7 (run 36582647152 attempt 1).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 29, 2026 14:03
@teamleaderleo
teamleaderleo merged commit c48b690 into manaflow-ai:main Sep 29, 2026
67 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 664fe49ef7: every check was green at merge (17 verified; 20 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 29, 2026
c7fea92 Fix Codex monitor recovery during transient owner loss (manaflow-ai#15612)
c48b690 fix: settle Claude Stop reentry after hook block (manaflow-ai#15603)
39d4a47 fix(ci): classify all missing Xcode pin failures (manaflow-ai#15605)
c0538b5 test: isolate mobile lifecycle registry from live host (manaflow-ai#15566)
c9ced10 test: remove flaky shell startup timing assertion (manaflow-ai#15589)
4de2a66 test: isolate mirror topology fixtures from window docks (manaflow-ai#15573)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude session stuck as Running when final Stop has stop_hook_active=true

1 participant