fix(sessions): scope Claude sessions per chat + thread (DM/group/topic) - #6
Merged
Merged
Conversation
…atch 1 Confirms design decision: SessionManager identity stays on session_id (the globally unique string assigned by Claude). Scope resolution lives at the handler boundary, which selects the right session_id and hands it to the facade. ClaudeSession / SessionManager / ClaudeIntegration do not need chat_id or thread_id fields in batch 1. See sdd/session-scoping/design for the full rationale.
Add ensure_dm_workdir helper that idempotently creates the per-user DM working directory and raises DmWorkdirError on failure. Wire it into every agentic Claude invocation path in MessageOrchestrator (agentic_text, agentic_document, _handle_agentic_media_message) via a _ensure_dm_workdir_or_abort helper that replies with an explicit error and aborts the turn on mkdir failure. No silent fallback to a shared workdir — per-user isolation is load-bearing.
…on v8 Add smoke tests proving the scope-key helper fixes the bug: * DM and forum-topic scopes yield distinct user_data session keys for the same user * /new in one scope does not clear another scope's session * Restart simulation (fresh user_data) recovers the right session per scope via an in-memory stand-in for load_session_by_scope * Legacy pre-v8 rows (chat_id IS NULL) are excluded from scope lookups * DM workdir helper creates the directory, is idempotent, and raises DmWorkdirError when mkdir fails Add migration v8 tests (tests/unit/test_storage/test_migration_v8.py): fresh DB gets chat_id/thread_id columns + idx_sessions_scope index, legacy rows flipped inactive by the v8 UPDATE, and re-initializing an already-v8 DB is a no-op (migrations gated by schema_version).
Add docs/session-scoping.md explaining: * (user_id, chat_id, thread_id) scope triple derivation * DM convention /workspace/_dm_<user_id> with fail-loud mkdir policy * Migration v8 forward-only + v9 rollback DDL placeholder * src/bot/session_scope.py as the single entry point Link from docs/README.md index.
test_agentic_text_calls_claude and test_agentic_voice_calls_claude run in DM scope (chat_id == user_id == 123), which now triggers the lazy /workspace/_dm_<user_id> provisioning added in this branch. The test host has no writable /workspace, so the real mkdir would abort the turn and Claude would never be invoked. Monkeypatch ensure_dm_workdir in both tests to a tmp no-op — the fail-loud behaviour itself is already covered by the tests/bot/test_session_scope.py suite.
When the scope is a DM, resolve current_dir to dm_workdir_for(user_id) instead of the shared approved_directory. Without this, Claude kept running at /workspace root for DMs — ensure_dm_workdir created the per-user path but nothing routed it to run_command, defeating R2. Applied in agentic_text, agentic_document, and _handle_agentic_media_message right after _ensure_dm_workdir_or_abort. Non-DM scopes (groups and forum topics) keep the existing resolution (explicit current_directory or settings.approved_directory).
Two new tests cover the R2 binding: - test_agentic_text_dm_uses_dm_workdir_as_working_directory asserts that a DM update causes run_command to receive working_directory=/workspace/_dm_<user_id> — not the shared approved_directory. - test_agentic_text_forum_topic_does_not_use_dm_workdir guards the non-DM branch: a forum-topic update keeps approved_directory and never calls ensure_dm_workdir, so we do not accidentally route every scope through the DM path.
Spell out the three chat-type cases (DM, plain group, forum topic) in a table so readers do not have to cross-reference the helper module and the orchestrator to understand where Claude runs. Also add an explicit warning that classic-mode handlers do NOT provision DM workdirs — a revisit item if classic mode is ever re-enabled.
CI black --check was flagging 5 files. No logic changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a regex-based redactor (src/security/secret_scrubber.py) that replaces common secret formats — GitHub PATs (classic + fine-grained), Anthropic / OpenAI keys, AWS access keys, Slack tokens, Google API keys — with [REDACTED-<KIND>] tags before the text reaches the DB. Wired into MessageRepository.save_message() so prompt, response, and error are all scrubbed on write. Motivated by a 2026-04-13 incident where a typo of `/git set` leaked a GitHub PAT into messages.prompt as plaintext. Scrubbing is applied only at persistence/logging boundaries — never before sending text to Claude, since Claude may legitimately need the literal value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the bug where a forum-topic Claude session was leaking into DM conversations (and vice versa) because
context.user_data["claude_session_id"]is only scoped per-user — it ignores which chat/thread the user is writing from.Now every Claude session is keyed by a namespaced scope
{chat_id}:{thread_id}:dm:<user_id>with an isolated workdir/workspace/_dm_<user_id><chat_id>:main<chat_id>:<thread_id>Classic DM usage keeps working — no features lost, no migration pain for existing users (legacy rows get
is_active=FALSEand users just start a fresh session).What changed
src/bot/session_scope.py—scope_key,user_data_session_key,is_dm,dm_workdir_for,ensure_dm_workdirchat_id+thread_idtosessions+idx_sessions_scope; deactivates pre-v8 rowsSessionStorage+ repos now read/write scope columns; newget_active_session_by_scopesession.py+facade.pyplumb scope through session resolutioncommand.py,callback.py,message.py,sdd_handler.py+ 12 inorchestrator.pyreplaced to use scoped keys/workspace/_dm_<user_id>and assigns it tocurrent_dirwhenis_dm(update)at 3 sitestests/bot/test_session_scope.py,tests/unit/test_storage/test_migration_v8.py, new orchestrator testsdocs/session-scoping.mdwith chat-type matrixTest plan
bot.db.pre-v8taken on EC2)🤖 Generated with Claude Code