Skip to content

Scope Stop hook + start-loop to current session_id (fix multi-session pollution) - #4

Open
rgindrat-gif wants to merge 1 commit into
promptadvisers:mainfrom
rgindrat-gif:fix/session-scoping-multi-session
Open

Scope Stop hook + start-loop to current session_id (fix multi-session pollution)#4
rgindrat-gif wants to merge 1 commit into
promptadvisers:mainfrom
rgindrat-gif:fix/session-scoping-multi-session

Conversation

@rgindrat-gif

Copy link
Copy Markdown

Summary

When you have two Claude Code sessions open in the same project, claudex currently breaks in two ways:

  1. Cross-session pollution. The Stop hook resolves the active loop with claudex_find_active_loop, which returns the most-recent .state file regardless of who created it. So if session A is mid-loop and session B ends a turn, session B gets BLOCKed with session A's round-N instructions, prompted to run A's runner script. From the user's perspective the loop "jumps" between terminals and it becomes impossible to tell which session is doing what.
  2. False concurrency lockout. start-loop.sh refuses to start if any non-terminal .state exists in the project, even when it belongs to a different session. Two unrelated topics in two terminals are impossible.

The state file already records session_id (set in start-loop.sh line 179) but neither the hook nor the start gate reads it. This PR wires that field through.

Changes

  • scripts/state-helpers.sh — new claudex_find_active_loop_for_session <sid> helper. Iterates state files in mtime order and returns the first one whose session_id matches. Empty/unknown sid → non-zero. The legacy claudex_find_active_loop is kept untouched so existing tests and any external callers still work.
  • hooks/stop-hook.sh — extracts session_id from the JSON hook input via python3 (with $CLAUDE_SESSION_ID as fallback), then calls the scoped lookup. If session_id can't be determined, fail-open approve so older Claude Code versions that don't pass session_id keep working without surprise.
  • scripts/start-loop.sh — the existing anti-double-launch loop now compares each active state's session_id against $CLAUDE_SESSION_ID. Only refuses if this session already has an active loop; loops in other sessions are allowed to coexist.
  • tests/platform-validation.sh — new section 11b adds 5 checks covering the scoped helper (empty arg, no loops, two sessions routed to their own state, unknown sid).

Test plan

  • bash plugins/claudex/tests/platform-validation.sh → 55 passed (was 50).
  • Synthetic E2E with two fake state files + two simulated hook fires:
    • Hook input {"session_id":"SID_B"} against state owned by SID_A{"decision":"approve"}
    • Hook input {"session_id":"SID_A"} against state owned by SID_A (phase=reviewing) → {"decision":"block",...} with the correct round-N instructions ✓
    • Hook input {} (no session_id) → {"decision":"approve"} (fail-open) ✓
  • start-loop.sh simulation:
    • Existing active loop owned by SESSION_OTHER + new launch from SESSION_NEW → allowed ✓
    • Existing active loop owned by SESSION_OTHER + new launch from SESSION_OTHER → refused with the existing error message ✓
  • Single-session sanity: behavior unchanged — drafting → reviewing → done.
  • bash -n clean on all three patched shell scripts.

Backwards compatibility

  • claudex_find_active_loop is kept untouched. Anything in the wild that imports state-helpers.sh still works.
  • If session_id is missing from the hook JSON (old Claude Code, weird env), the hook fails open (approve) rather than trapping the user. Matches the project's existing fail-open philosophy in hooks/stop-hook.sh.
  • No state file format change — session_id was already being written.

Known remaining limitation (not in this PR)

Two concurrent loops in the same working directory still both target PLAN.md at the project root and would clobber each other. A --plan-file <path> flag on /claudex:plan (and matching read in stop-hook.sh) would solve it cleanly. Happy to do that in a follow-up PR if you'd like — wanted to keep this one tight and focused on the scoping bug since that's what surfaces first.

Notes for review

  • Commit is signed by Raphael Gindrat <raphael.gindrat@nuavo.com> (my real email, not the GitHub noreply). Let me know if you'd prefer a Signed-off-by trailer or anything else.
  • Repro for the original pollution: open two Claude Code sessions in the same project, run /claudex:plan add foo in one, then end a turn (any turn) in the other. Pre-patch the second session prints the round-N instructions of the first.

🤖 Generated with Claude Code

Two related bugs hit users running multiple Claude Code sessions in the
same project:

1. Cross-session pollution. The Stop hook used `claudex_find_active_loop`,
   which returns the most-recent .state file in the project regardless of
   which session created it. So if session A is in the middle of a loop
   and session B ends a turn, session B gets BLOCKed with session A's
   round-N instructions. Two terminals quickly become unintelligible.

2. False concurrency lockout. start-loop.sh refused to start if ANY
   non-terminal .state file existed in the project, even one owned by a
   different session. Two unrelated topics in two terminals were
   impossible.

The state file already records `session_id` (line 179 of start-loop.sh)
but neither the hook nor the start gate ever reads it. This patch wires
session_id through:

- state-helpers.sh: new `claudex_find_active_loop_for_session <sid>`
  helper that iterates state files in mtime order and returns the first
  one whose `session_id` field matches. Empty/unknown sid -> non-zero.
  The legacy `claudex_find_active_loop` is kept untouched for backwards
  compatibility (used in tests, possibly by external callers).

- stop-hook.sh: extracts `session_id` from the JSON hook input via
  python3 (with $CLAUDE_SESSION_ID as fallback), then uses the scoped
  lookup. If session_id can't be determined, fail-open (approve) so
  older Claude Code versions that don't pass session_id keep working.

- start-loop.sh: the existing anti-double-launch loop now compares each
  active state's session_id against $CLAUDE_SESSION_ID. Only refuses if
  THIS session already has an active loop; loops in other sessions are
  allowed to coexist.

- tests/platform-validation.sh: new section 11b adds 5 checks for the
  session-scoped lookup helper (empty arg, no loops, two sessions
  routed to their own state, unknown sid).

All 55 tests pass (was 50). No behavior change for single-session use.

Known remaining limitation (not addressed here): two concurrent loops
in the *same* working directory still both target `PLAN.md` and would
clobber each other. A `--plan-file <path>` flag would solve it cleanly
but is a larger UX change; happy to do it in a follow-up if maintainers
agree on the shape.
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