Skip to content

fix: Stop hook blocks only the run's owning session, not other sessions in the project#48

Closed
cannt wants to merge 1 commit into
bmad-code-org:mainfrom
cannt:fix/stop-hook-owner-gating
Closed

fix: Stop hook blocks only the run's owning session, not other sessions in the project#48
cannt wants to merge 1 commit into
bmad-code-org:mainfrom
cannt:fix/stop-hook-owner-gating

Conversation

@cannt

@cannt cannt commented Jun 25, 2026

Copy link
Copy Markdown

Problem

The story-automator Stop hook is registered project-scoped, so it fires for every Claude Code session opened in that project. While a run is active (marker present, stories remaining), the hook returns {"decision":"block"} to keep the orchestrator from stopping early — but it has no way to tell which session owns the run. So any other session in the same project gets blocked too: a second session opened to watch the run, or unrelated work in another worktree, cannot stop. The existing STORY_AUTOMATOR_CHILD exemption only covers spawned child sessions.

Fix

Record the orchestrator's own session id as ownerSession in the active marker at create-time (Claude Code exports CLAUDE_CODE_SESSION_ID into hook/CLI subprocesses). In the Stop hook, read the caller's session_id (Claude passes the hook payload on stdin) and only keep blocking when it matches ownerSession; any other session returns 0 (not blocked).

Backward compatibility

Markers created before this change have no ownerSession, so the hook falls back to the prior block behavior — no regression for in-flight runs.

Changes

  • commands/orchestrator.py — record ownerSession in the marker create payload
  • commands/basic.pycmd_stop_hook reads the caller session_id and gates on owner

Tests

Added:

  • the owning session is still blocked,
  • a different session is not blocked while a run is active,
  • marker create records ownerSession from CLAUDE_CODE_SESSION_ID.

(The legacy / no-ownerSession path is already covered by the existing test_stop_hook_uses_project_root_env... test, which uses an owner-less marker and asserts a block.)

tests/test_stop_hooks.py → 30/30. npm run pack:dry-run, npm run test:cli, and npm run test:smoke all pass. Full suite → 413/414; the one failing test (test_build_cmd_uses_legacy_ai_command_consistently_for_claude) also fails on a clean checkout of main and is unrelated to this change.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@cannt, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 58 minutes and 26 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1367ed2f-228c-4691-be32-a3aff6c08e1a

📥 Commits

Reviewing files that changed from the base of the PR and between f332173 and c461f97.

📒 Files selected for processing (3)
  • skills/bmad-story-automator/src/story_automator/commands/basic.py
  • skills/bmad-story-automator/src/story_automator/commands/orchestrator.py
  • tests/test_stop_hooks.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@cannt cannt marked this pull request as ready for review June 25, 2026 10:45
@cannt cannt requested a review from bma-d as a code owner June 25, 2026 10:45
@cannt

cannt commented Jun 25, 2026

Copy link
Copy Markdown
Author

Closing in favor of coordinating on #45, which already reworks cmd_stop_hook (reads the stdin payload, restructures the early-returns) to fix the runaway-polling problem. The owner-session gating proposed here is a small, complementary addition that belongs on top of that work rather than as a competing change to the same function. I've raised it on #45.

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.

1 participant