Skip to content

Refuse a second session on another session's checkpoint thread - #34

Merged
Shashankss1205 merged 1 commit into
mainfrom
fix/issue-18
Jul 31, 2026
Merged

Refuse a second session on another session's checkpoint thread#34
Shashankss1205 merged 1 commit into
mainfrom
fix/issue-18

Conversation

@Shashankss1205

Copy link
Copy Markdown
Collaborator

What

SessionStore.create() guarded session-id reuse and let thread-id reuse straight through, and sharing a checkpoint thread was an approval-gate bypass entirely inside the session API: the second session got a fresh record with no holds, resumed the first one's checkpointed boundary, and — because interrupt_before does not re-fire for a boundary it has already stopped at — ran the held gated node with no approval ever given, while the first session's record still said the hold was open.

How

  • grapharc/session/store.py — inside the existing BEGIN IMMEDIATE transaction, create() now does SELECT id FROM sessions WHERE thread_id = ? before the insert and refuses a collision. Check-then-insert happens under one write lock, so the refusal is atomic with the insert: no session row and no transition row left behind, including under two processes racing on one store. The create() docstring and the IntegrityError comment now state the guarantee the comment already believed: a checkpoint thread belongs to exactly one session.
  • grapharc/session/errors.py — a new sibling of SessionExistsError, ThreadInUseError, carrying thread_id and session_id and naming both in its message (thread 'shared' already belongs to session 'a'). Exported from grapharc.session.
  • Default path unchanged — thread id equals session id, and ids are unique, so a fresh id can never collide; a reused id still lands as SessionExistsError exactly as before (the thread check lets a holder with the same id fall through to the insert).
  • Deliberately no schema change — no UNIQUE index: an existing store on disk may already carry duplicates, and creating the index at open would brick it. The transactional check needs no migration. The HTTP layer's CreateSessionRequest.thread_id is untouched (belongs to server: the HTTP API does not use the durable session layer #7).

Tests

Two new tests in tests/test_session.py, both red without the source change:

  • test_reusing_another_sessions_thread_is_refused — the collision raises ThreadInUseError naming both sides, and leaves no session row and no transition row behind.
  • test_a_second_session_on_one_thread_cannot_run_a_held_gated_node — the issue's repro as a regression test: session A holds apply, session B on the same thread is refused at create(), the gated node never runs, and A's hold is still the truth.

README/cookbook only describe the other bypass (driving session.graph.invoke(...) directly), not this one, so no doc text needed updating.

Verification

  • pytest -q: 1617 passed, 0 failed
  • ruff check .: clean

Fixes #18

🤖 Generated with Claude Code

SessionStore.create() guarded id reuse and let thread_id reuse straight
through, and sharing a thread was an approval-gate bypass entirely
inside the session API: the second session got a fresh record with no
holds, resumed the first one's checkpointed boundary, and — because
interrupt_before does not re-fire for a boundary it has already stopped
at — ran the held gated node with no approval ever given, while the
first session's record still said the hold was open.

The guard is a SELECT inside the existing BEGIN IMMEDIATE transaction,
so the refusal (ThreadInUseError, naming both the thread and the
session holding it) is atomic with the insert: no session row and no
transition row left behind, including under two processes racing on one
store. No schema change — a UNIQUE index taken at open would brick an
existing store already carrying duplicates. The default path, thread id
equals session id, is unchanged: ids are unique, and a reused id still
lands as SessionExistsError.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Shashankss1205
Shashankss1205 merged commit 2e5e521 into main Jul 31, 2026
6 checks passed
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.

session: two sessions can share one checkpoint thread, and the second runs a held gated node unapproved

1 participant