refactor(db): every read path takes a Scope — "every workspace" is passed, never defaulted (P2-1b-ii)

[jwvanderstam]

Draft: depends on #404 and #405. The branch carries their commits underneath this one (b4f5c0b is #404, 888f081 is #405's commit, cherry-picked). Review only the top commit, c7e6124. Once both have merged, I'll rebase onto main so that only this commit remains, then mark the PR ready.

Second of three PRs for the application half of P2-1b (ADR-5).

What changes

Fifteen database methods took workspace_id: str | None = None and read None as every workspace. That's the pattern P0-1 removed from the by-id paths, and it was still present in the listings, the counts and retrieval itself. Each of the 15 now:

• takes a mandatory keyword-only scope: Scope;
• builds its SQL through scope_predicate, which refuses None and "" at runtime;
• passes the scope to get_connection(scope=).

Area	Methods
Retrieval	search_similar_chunks, search_lexical_chunks, search_memories
Listings / counts	get_all_documents, get_document_count, get_chunk_count, list_conversations, count_conversations, list_connectors, get_all_memories
Other reads / bulk	get_stale_documents, get_low_confidence_queries, get_workspace_ontology, is_duplicate_memory, delete_all_conversations

The retrieval chain above them takes a Scope too: retrieve_context, _run_retrieval_pipeline, MemoryRetriever.retrieve, suggest_documents, and chat's retrieve_contexts / get_rag_context / retrieve_plan_and_memory. The translation happens once, at the route, through get_scope(request). It is not repeated as workspace_id or ALL_WORKSPACES further down.

Behaviour: unchanged by construction, with one deliberate exception

Every place that passed None now passes an explicit ALL_WORKSPACES, where a reviewer can see it:

• the SyncWorker stale sweep;
• connector loading;
• the boot-time count;
• the admin-only settings stats;
• the two token-authenticated MCP list_sources calls;
• the eval script when run without --workspace-id;
• memory dedup for a memory that has no workspace.

On routes, get_scope returns what get_workspace_id did for members. For an admin who named no workspace, it returns ALL_WORKSPACES where the old code got None.

The exception is a fourth disclosure, which the conversion surfaced. GET /api/status needs only a session, and it counted documents for whatever X-Workspace-ID named: any workspace, or the whole installation when no header was sent. get_scope() refused on that route because no guard had run there, which is how it showed up. Status now runs the workspace check without requiring it to pass: an authorised caller gets their scope's count, and anyone else gets 0, so the status bar still renders. It's a regression test in TestApiStatus that fails without the fix.

Not converted, deliberately

• document_exists and the five inserts take the workspace a row is written to. There None means "no workspace", matched with IS NOT DISTINCT FROM. P2-1b-iii has to handle them because of RLS insert checks.
• The 13 methods in workspaces.py / workspace_keys.py take the workspace itself as the object being addressed.

How it is held

SCOPED_METHODS now lists 33 methods. The AST checks fail on any of these:

• a call in src/ or tests/ that omits scope=;
• a scoped method with an optional scope;
• a scoped method that opens a connection without passing its own scope;
• a scoped method in src/db that is missing from the list.

The one blind spot is code held in strings. test_list_params_survive_vector_adapters.py runs Python through a subprocess. The static check couldn't see those calls, and the integration run caught them, so both are needed.

Verification (local, Python 3.12 container + throwaway pgvector pg16, isolated from the live stack)

• ruff, mypy, bandit: clean.
• Fast suite: 3196 passed, 0 failed.
• Integration suite (-m "not ollama", as CI runs it): 330 passed after fixing the two string-embedded calls above; 3 skipped, as in CI.
• Test updates follow one rule. A call with no workspace meant every workspace, so it now passes ALL_WORKSPACES; workspace_id=X becomes scope=X. Two memory assertions change from %s::uuid to %s, because scope_predicate emits no cast. The by-id paths already rely on that against real Postgres. There's also one new test: _allowed_workspace_ids(None) now raises instead of reading as unscoped.

Noticed, not changed

RetrievalMixin._get_app_cache() always returns None, so the query-result cache in retrieve_context is dead code. Its key is (query, top_k, min_similarity, hybrid), with no scope, filters or source ids. If anyone switches it on as written, it becomes a cross-workspace leak. It should either be deleted or get the scope in its key before it's revived.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud