fix: make AdvancedSQLiteSession.pop_item branch-aware

[winklemad]

What

AdvancedSQLiteSession tracks branches (_current_branch_id, a message_structure table) and its get_items is branch-scoped, but it does not override pop_item. The inherited SQLiteSession.pop_item deletes the newest row for the whole session:

DELETE FROM {messages} WHERE id = (
    SELECT id FROM {messages} WHERE session_id = ? ORDER BY id DESC LIMIT 1
)

There is no branch filter, so on a non-main branch it pops an item that belongs to another branch (usually main) and silently corrupts it — pop_item returns the wrong item and the other branch loses its tail. This is reachable via the runner's session-rewind path, which verifies the tail with branch-aware get_items and then removes it with pop_item.

Reproduction

# main = [u1, a1, u2, a2]; branch from turn 2 -> current branch = [u1, a1]
popped = await session.pop_item()          # returns "a2" (main's tail), not "a1"
await session.get_items(branch_id="main")  # main is now [u1, a1, u2] -- corrupted

Fix

Override pop_item in AdvancedSQLiteSession to scope the pop to the current branch (join message_structure, filter by branch_id, order by the branch's sequence_number). Because branches share message rows and foreign keys are not enforced (so ON DELETE CASCADE does not run), it drops the branch's newest message_structure row and deletes the underlying message only when no other branch still references it. Flat (non-branched) sessions behave exactly as before.

Tests

Adds test_pop_item_is_branch_scoped: pops on a branch and asserts it returns the branch tail, leaves main untouched, still pops normally on main, and returns None on an empty branch. It fails on the current code (returns main's tail and corrupts main) and passes with the fix. ruff and mypy are clean; the full AdvancedSQLiteSession test file passes (46).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ddbb945a02

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve branches when popping their last item

When pop_item() removes the final message_structure row for a branch, that branch effectively disappears because list_branches() groups only rows in message_structure and switch_to_branch() validates existence with COUNT(*) from the same table. In a branch with a single item (or after popping down to one item), popping the tail leaves _current_branch_id set but the branch is no longer listable or switchable once the user switches away, so branch-aware rewinds can silently orphan an otherwise valid empty branch.

Useful? React with 👍 / 👎.

[seratch]

Thanks for the contribution. The reported bug is real: AdvancedSQLiteSession.get_items() is branch-scoped, while the inherited pop_item() deletes the session-wide tail.

However, #3755 already addresses the same pop_item bug and also covers the metadata and ownership invariants needed for a safe fix, including turn_usage cleanup, a call-time branch snapshot, shared-message ownership, and controlled interleaving tests. This PR still leaves stale turn_usage when a pop empties a turn, and a concurrent branch switch can redirect an in-flight pop because the branch is read inside the worker.

To avoid maintaining competing implementations, I am going to close this PR in favor of #3755. Thank you for the clear reproduction and regression test.