diff --git a/.claude/rules/file-map.md b/.claude/rules/file-map.md index 8a221bc0..320d613f 100644 --- a/.claude/rules/file-map.md +++ b/.claude/rules/file-map.md @@ -121,7 +121,7 @@ Full module index for LocalChat. **Keep this current** — update in the same co | `tests/unit/test_frontend_gkb_contribution.py` | GKB-2 — the Documents page shows the contribute and withdraw controls only when `/api/users/me` says so (hidden by default, and without an active workspace), selectors only for a contributor, and the "shared globally" tag on exactly the shared document | | `tests/unit/test_gkb_contribution_routes.py` | GKB-2 — contributing needs a global admin even from a workspace owner, a workspace API key is refused, a malformed selection never reaches the database; withdrawing needs only ws:owner | | `tests/unit/test_db_contributions.py` | GKB-2 — `ContributionsMixin` against a scripted cursor: a refused contribution writes and commits nothing, each document is reported from its own statements, and every write names its workspace | -| `tests/integration/test_gkb_contribution.py` | GKB-2 against Postgres — the copy carries chunks and embeddings, re-contributing updates in place, two projects share a filename, a foreign document refuses the whole contribution, archiving removes it from what another workspace reads under RLS, and a workspace's own document list tags what it shares, read under its own scope | +| `tests/integration/test_gkb_contribution.py` | GKB-2 against Postgres — the copy carries chunks and embeddings, re-contributing updates in place, two projects share a filename, a foreign document refuses the whole contribution, archiving removes it from what another workspace reads under RLS, and a workspace's own document list tags what it shares, read under its own scope; a contribution is neither matched as a document without a workspace nor picked for re-ingest | | `tests/unit/test_scoped_connection.py` | P2-1b-iii in the fast suite — `get_connection(scope=)` issues the role switch, binds the workspace as a parameter, and sets the iterative HNSW scan; `ALL_WORKSPACES` sets `app.all_workspaces` and switches no role (QP-5), `None` issues nothing; an autocommit connection and an empty workspace are refused | | `tests/integration/test_row_level_security.py` | The IVP for 0017 — an unscoped transaction sees zero rows from all ten protected tables, a foreign scope sees zero, and the owning scope sees its own. Creates and migrates its **own** database: the shared CI one gets its schema from `_ensure_extensions_and_tables()` and never the Alembic chain, so this would have skipped in CI and a vacuous pass would have looked identical to a real one. Also P2-1b-iii's end-to-end proof: the application's own `get_connection(scope=)` runs as `localchat_scoped` and hides a foreign workspace from a query with no WHERE clause, a table added later is granted to the role at the next boot, and a scoped vector search over 10k chunks still returns its full `top_k`. And 0019's: a scoped transaction reads only the live contribution (not an orphan or an archived one), cannot update, delete or insert global rows, sees nothing unscoped, and `retrieve_context` returns each tier through the application. And 0022's: without the flag a role neither 0017 nor 0021 admits sees nothing, with it every workspace; the flag cannot widen a `localchat_scoped` transaction, and ends with its transaction. And 0023's: the transition policy is gone and RLS is forced on all ten | | `CONTRIBUTING.md` | QP-8's release policy (semver, a security fix released within 7 days, a hand-written changelog) and QP-9 — which paths are security-critical, and the review a separate agent session gives a change to them before it merges: the checklist, what the review is and is not, where findings go | diff --git a/CHANGELOG.md b/CHANGELOG.md index 1840adf3..f1eceab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,14 @@ OpenAI, `https://api.openai.com/v1`) and drop any `provider/` prefix from `CLOUD not OpenAI-shaped (Anthropic's native API, Vertex) is no longer reachable, as ADR-4 accepted. +### Fixed + +- **A global contribution is never mistaken for a document without a workspace.** Both have + no `workspace_id`, so an upload naming no workspace, an admin's workspace-less connector, + or the re-ingest worker could update a contribution in place, and every workspace reads + the tier. The worker also picked contributions as stale and tried to re-read a source + file they do not have. Found by the separate review of #528. + ## [3.5.0] — 2026-10-31 The release that closes the quality plan's maturity target (Initiative 12). All six diff --git a/src/db/documents.py b/src/db/documents.py index 189b4678..5077ff48 100644 --- a/src/db/documents.py +++ b/src/db/documents.py @@ -642,7 +642,9 @@ def exists( Scoped to ``workspace_id`` — filename uniqueness is per-workspace, not global. ``IS NOT DISTINCT FROM`` matches NULL-workspace_id documents - against each other without matching a real workspace's documents. + against each other without matching a real workspace's documents. A global + contribution has no workspace either, and is never matched: an upload naming + no workspace must not update what every workspace reads. """ if not self._db.is_connected: raise DatabaseUnavailableError("Cannot check document existence: Database is not connected") @@ -656,6 +658,7 @@ def exists( LEFT JOIN document_chunks dc ON d.id = dc.document_id AND dc.deleted_at IS NULL WHERE d.filename = %s AND d.deleted_at IS NULL AND d.workspace_id IS NOT DISTINCT FROM %s + AND d.contributed_at IS NULL GROUP BY d.id, d.created_at, d.content_hash """, (filename, workspace_id)) row = cursor.fetchone() @@ -839,6 +842,7 @@ def stale(self, max_age_hours: int, *, scope: Scope) -> list[dict[str, Any]]: """Return documents whose last_ingested_at is older than max_age_hours. Documents without last_ingested_at fall back to created_at for comparison. + A global contribution is a copy with no source file to re-read, and is skipped. """ if not self._db.is_connected: return [] @@ -853,7 +857,7 @@ def stale(self, max_age_hours: int, *, scope: Scope) -> list[dict[str, Any]]: FROM documents WHERE COALESCE(last_ingested_at, created_at) < NOW() - (INTERVAL '1 hour' * %s) - AND deleted_at IS NULL{scope_sql} + AND deleted_at IS NULL AND contributed_at IS NULL{scope_sql} """ # nosec B608 # scope_predicate fragment: literal column, value is %s params: list[Any] = [max_age_hours, *scope_params] cursor.execute(sql, params) diff --git a/tests/integration/test_gkb_contribution.py b/tests/integration/test_gkb_contribution.py index 2766c058..9118561c 100644 --- a/tests/integration/test_gkb_contribution.py +++ b/tests/integration/test_gkb_contribution.py @@ -252,3 +252,12 @@ def tags(ws: str) -> dict[str, bool]: assert tags(workspaces["b"]) == {"retro.md": False} database.contributions.archive(workspaces["a"], admin) assert tags(workspaces["a"]) == {"retro.md": False, "draft.md": False} + + +def test_a_contribution_is_not_mistaken_for_a_document_without_a_workspace(database, workspaces, admin): + """An upload naming no workspace, or the re-ingest worker, must not update the global tier.""" + filename = f"gkb-nows-{uuid.uuid4().hex[:8]}.txt" + doc = _document(database, workspaces["a"], filename) + global_id = database.contributions.contribute(workspaces["a"], [doc], admin)[0]["global_document_id"] + stale_ids = {d["id"] for d in database.documents.stale(0, scope=ALL_WORKSPACES)} + assert (database.documents.exists(filename, None)[0], global_id in stale_ids) == (False, False)