fix: use tuple keys in InMemorySessionStore to prevent composite key collisions

[SushantTusharJoshi]

Summary

• Replaces _key_to_string() (which joined components with /) with _key_to_tuple() returning a proper tuple
• Changes _store and _mtimes dict types from dict[str, ...] to dict[tuple, ...]
• Updates all methods: append, load, list_sessions, delete, list_subkeys, get_entries, and size
• Eliminates ambiguity where ("a/b", "c") and ("a", "b/c") previously both mapped to "a/b/c"

Test plan

• 7 adversarial regression tests: slash in project_key, slash in session_id, list_sessions filtering, delete isolation, list_subkeys, size correctness
• Verified via standalone async test script exercising all affected methods

Closes #1168

Developed with Claude Code as a coding partner

[tonydzi]

written by mycroft, an AI agent at Palo Alto AI Research Lab; a human verified every command and result below.

The fix is right and the tests are mostly load-bearing. One test isn't, and there's a bigger gap one layer up. Ran on fdabb48, python 3.12, pytest tests/test_session_store_conformance.py: 41 passed.

The tests are real. Restoring only _internal/session_store.py from main and keeping your test file reddens 4 of the 5 new tests (8 with both anyio backends): test_slash_in_project_key_no_collision, test_slash_in_session_id_no_collision, test_delete_with_slash_in_keys_no_cross_cascade, test_size_with_slash_in_keys. That is the check worth having, and it holds.

test_list_sessions_with_slash_in_project_key is not one of them. It passes on the pre-fix implementation:

$ git checkout main -- src/claude_agent_sdk/_internal/session_store.py
$ pytest ...::test_list_sessions_with_slash_in_project_key -q
2 passed

The old prefix logic happens to get that pair right: "a/b/s1" under prefix "a/" leaves "b/s1", which contains a slash and is correctly excluded. The pair that actually collides is the one where both keys encode to the same string. This version is red on main and green here, verified both ways:

@pytest.mark.anyio
async def test_list_sessions_with_colliding_encodings() -> None:
    """Two sessions whose old '/'-joined encodings were identical must both be listed."""
    store = InMemorySessionStore()
    await store.append({"project_key": "a/b", "session_id": "s1"}, [{"n": 1}])
    await store.append({"project_key": "a", "session_id": "b/s1"}, [{"n": 2}])

    assert [s["session_id"] for s in await store.list_sessions("a/b")] == ["s1"]
    assert [s["session_id"] for s in await store.list_sessions("a")] == ["b/s1"]

The gap: this lands in the reference implementation, not in the contract. claude_agent_sdk.testing.run_session_store_conformance is what third-party adapter authors import to certify their store, and its 14 contracts say nothing about separator collisions. So a store using the exact encoding this PR removes still certifies clean:

shipped conformance harness: PASS (14 contracts)
load({'a/b','c'})  -> [{'from': 'ab-c'}, {'from': 'a-bc'}]
load({'a','b/c'})  -> [{'from': 'ab-c'}, {'from': 'a-bc'}]
distinct sessions stored: 1
after deleting {'p','s'}, load({'p','s/x'}) -> None

Two sessions merged into one, and deleting one session silently destroyed an unrelated one. Both under a green conformance run.

That is not hypothetical copying: MinimalStore in this same test file (test_skip_optional_suppresses_contracts) keys on f"{project_key}/{session_id}/{subpath or ''}", and the reference implementation itself used the joined form until this PR. An adapter author reading either one gets the bug you are fixing, and the harness will tell them they are fine.

Suggested addition, a 15th contract, so the class is closed for every implementation rather than for this one:

# 15. distinct keys never collide, whatever the components contain
store = await fresh()
await store.append({"project_key": "a/b", "session_id": "c"}, [_e({"uuid": "x", "n": 1})])
await store.append({"project_key": "a", "session_id": "b/c"}, [_e({"uuid": "y", "n": 2})])
assert await store.load({"project_key": "a/b", "session_id": "c"}) == [_e({"uuid": "x", "n": 1})]
assert await store.load({"project_key": "a", "session_id": "b/c"}) == [_e({"uuid": "y", "n": 2})]

That one line of coverage is what makes the delete-cascade case above impossible to ship, and it is the same class of loss we care about most: a store that reports success while quietly returning another session's transcript.

[SushantTusharJoshi]

Thanks for the thorough review @tonydzi — both points were spot on.

Addressed in ca00dac:

1. Replaced the non-load-bearing test. test_list_sessions_with_slash_in_project_key → test_list_sessions_with_colliding_encodings which uses ("a/b", "s1") vs ("a", "b/s1") — verified it's red on main, green here.

2. Added contract 15 to run_session_store_conformance. The exact test you suggested — two keys whose old /-joined encodings collide must load independently. This closes the gap for third-party adapters: a store using the old encoding now fails conformance instead of silently shipping the bug.

3. Fixed the MinimalStore test fixtures — they were using the old /-joined encoding which would now fail contract 15. Switched them to tuple keys so the skip_optional and auto-skip tests remain about what they're testing (optional method detection), not about key encoding.

All 27 asyncio tests pass (trio skipped — not installed locally, same as before).

[tonydzi]

Ran ca00dac. Both changes hold, and contract 15 turns out to be stronger than your note claims — but it closes one door of two, and the second one is the one my first comment actually demonstrated. Details below, all from runs on python 3.12 with trio installed (so both anyio backends execute).

Contract 15 is load-bearing. Same mutant as before — restore only _internal/session_store.py from main, keep your tests:

$ git checkout origin/main -- src/claude_agent_sdk/_internal/session_store.py
$ pytest tests/test_session_store_conformance.py -q
14 failed, 27 passed

That is up from 8 before this commit. The new reds are test_list_sessions_with_colliding_encodings and — the ones worth having — test_conformance and test_conformance_with_async_factory: the shipped harness now rejects the pre-fix encoding instead of certifying it. On the branch as-is: 41 passed.

The gap that remains: contract 15 only exercises append/load. The loss I showed last time was a delete cascading across sessions, and that is still certifiable. Here is a store that keys entries by tuple — so contract 15 is satisfied honestly — and implements the required "delete main cascades to subkeys" as a prefix scan over the joined path:

class PrefixCascadeStore(SessionStore):
    # append/load key on (project_key, session_id, subpath) tuples -> contract 15 passes

    async def delete(self, key: SessionKey) -> None:
        k = self._k(key)
        if k[2]:
            self._d.pop(k, None)
            return
        prefix = f"{key['project_key']}/{key['session_id']}/"
        for existing in [x for x in self._d if self._path(x).startswith(prefix)]:
            self._d.pop(existing, None)

conformance harness (15 contracts): PASS
contract 15 case (append/load): [{'type': 'x', 'mine': True}]
list_subkeys('proj','sess') before delete: ['subagents/a']   # <- belongs to session 'sess/x'
after delete('proj','sess') -> load('proj','sess/x'): None
sessions left in 'proj': []

Deleting sess destroyed the unrelated session sess/x and its subagent transcript, and before that list_subkeys had already handed out another session's subagent path. Green conformance both times.

This is not a contrived adapter: cascade-by-prefix is how every KV/object backend implements that contract (SCAN MATCH, list-objects prefix, rmtree), and building the prefix by joining raw components is the same mistake this PR removes from the reference implementation — which is why the tuple rewrite had to touch delete and list_subkeys too, not just append. Contracts 10–13 all use slash-free keys, so nothing in the harness notices.

Suggested patch — two hunks inside contract 10, no new contract:

        # A distinct session whose *joined* path sits under the deleted one.
        neighbor: SessionKey = {
            "project_key": _KEY["project_key"],
            "session_id": f"{_KEY['session_id']}/x",
        }
        neighbor_sub: SessionKey = {**neighbor, "subpath": "subagents/agent-n"}
        ...
        await store.append(neighbor, [_e({"n": 1})])
        await store.append(neighbor_sub, [_e({"n": 1})])

        await store.delete(_KEY)
        ...
        loaded_neighbor = await store.load(neighbor)
        assert loaded_neighbor is not None and len(loaded_neighbor) == 1
        loaded_neighbor_sub = await store.load(neighbor_sub)
        assert loaded_neighbor_sub is not None and len(loaded_neighbor_sub) == 1

Checked in both directions rather than asserted:

• reference store with the patch applied — 41 passed, so it costs the fixed implementation nothing;
• PrefixCascadeStore with the patch applied — fails at assert loaded_neighbor is not None, i.e. it does catch the class it is meant to catch.

Honest scope. An SDK-derived project_key cannot contain / — _sanitize_path maps every non-alphanumeric to -, and session ids are uuids. So this bites callers who mint their own keys and adapter authors who copy the reference encoding, which is exactly the audience that imports run_session_store_conformance. That is the same argument that justified contract 15, applied to the method where the failure mode is silent destruction rather than a wrong read.