From ef444599841ef6c00eb649e1b08c70a758812f01 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 23 Jun 2026 00:19:56 +0100 Subject: [PATCH] fix: small wiki fixes --- application/agents/tools/wiki.py | 2 +- .../api/answer/services/stream_processor.py | 17 +++++--------- .../storage/db/repositories/wiki_pages.py | 21 +++++++++++++---- application/worker.py | 9 +++++--- .../db/repositories/test_wiki_pages.py | 20 ++++++++++++++++ tests/worker/test_convert_source_to_wiki.py | 23 +++++++++++++++++++ 6 files changed, 73 insertions(+), 19 deletions(-) diff --git a/application/agents/tools/wiki.py b/application/agents/tools/wiki.py index 7378cf8b..b780259e 100644 --- a/application/agents/tools/wiki.py +++ b/application/agents/tools/wiki.py @@ -201,7 +201,7 @@ class WikiTool(Tool): def _reembed(self, path: str, content_hash: str) -> None: """Enqueue an async re-embed for ``path``, authored as the source owner. - The Unit-2 worker loads the source via ``get_any(source_id, user)`` + The re-embed worker loads the source via ``get_any(source_id, user)`` (owner-scoped), so the owner — not the caller — must be passed as ``user`` or a team editor's edit would fail to re-embed. A per-page idempotency key guards each edit independently and dedups broker diff --git a/application/api/answer/services/stream_processor.py b/application/api/answer/services/stream_processor.py index 300c4b48..68694a5a 100644 --- a/application/api/answer/services/stream_processor.py +++ b/application/api/answer/services/stream_processor.py @@ -912,13 +912,14 @@ class StreamProcessor: return value or "prefetch" def _build_wiki_config(self) -> Optional[Dict[str, Any]]: - """Resolve the WikiTool config for the first writable wiki source (D18). + """Resolve the WikiTool config for the first writable wiki source. A source qualifies when ``SourceConfig.parse(config).kind == "wiki"`` and the principal can write it (``effective_write_owner`` returns an owner — owner or team editor; viewers get None and no tool). v1 supports one - writable wiki source; extras are logged and skipped. Returns None when no - writable wiki source is present. + writable wiki source; the first match wins and the scan stops there so + this runs at most one owner+source lookup per chat on the hot path. + Returns None when no writable wiki source is present. """ from application.api.user.team_sharing import effective_write_owner @@ -943,19 +944,13 @@ class StreamProcessor: continue if SourceConfig.parse(source_doc.get("config")).kind != "wiki": continue - if wiki_config is not None: - logger.info( - "Multiple writable wiki sources for this agent; " - "using the first and skipping source %s", - str(source_doc["id"]), - ) - continue wiki_config = { "source_id": str(source_doc["id"]), "source_owner_id": owner, "decoded_token": self.decoded_token, "user": caller, } + break except Exception: logger.exception("Failed to resolve wiki tool config") return None @@ -1602,7 +1597,7 @@ class StreamProcessor: "tool_executor": tool_executor, } - # Wiki tool injection + authz (D18): only for agent types that build a + # Wiki tool injection + authz: only for agent types that build a # tools_dict (classic/agentic/research), and only when a writable wiki # source is present for the principal (viewers get nothing). if agent_type in ("classic", "agentic", "research"): diff --git a/application/storage/db/repositories/wiki_pages.py b/application/storage/db/repositories/wiki_pages.py index 8ad378ae..d7ffaeb1 100644 --- a/application/storage/db/repositories/wiki_pages.py +++ b/application/storage/db/repositories/wiki_pages.py @@ -26,6 +26,17 @@ def _content_hash(content: str) -> str: return hashlib.sha256(content.encode("utf-8")).hexdigest() +def _escape_like(value: str) -> str: + """Escape ``LIKE`` wildcards so a path prefix matches literally. + + Without this a path containing ``_`` or ``%`` (e.g. ``/api_v1/``) would be + a wildcard pattern, so a prefix list/delete could over-match sibling paths. + Backslash is escaped first so the wildcard escapes are not double-escaped; + callers must pair the result with ``LIKE ... ESCAPE '\\'``. + """ + return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + + class WikiPagesRepository: def __init__(self, conn: Connection) -> None: self._conn = conn @@ -149,10 +160,11 @@ class WikiPagesRepository: result = self._conn.execute( text( "SELECT * FROM wiki_pages " - "WHERE source_id = CAST(:source_id AS uuid) AND path LIKE :prefix " + "WHERE source_id = CAST(:source_id AS uuid) " + "AND path LIKE :prefix ESCAPE '\\' " "ORDER BY path" ), - {"source_id": source_id, "prefix": prefix + "%"}, + {"source_id": source_id, "prefix": _escape_like(prefix) + "%"}, ) return [row_to_dict(r) for r in result.fetchall()] @@ -180,9 +192,10 @@ class WikiPagesRepository: result = self._conn.execute( text( "DELETE FROM wiki_pages " - "WHERE source_id = CAST(:source_id AS uuid) AND path LIKE :prefix" + "WHERE source_id = CAST(:source_id AS uuid) " + "AND path LIKE :prefix ESCAPE '\\'" ), - {"source_id": source_id, "prefix": prefix + "%"}, + {"source_id": source_id, "prefix": _escape_like(prefix) + "%"}, ) return result.rowcount diff --git a/application/worker.py b/application/worker.py index 6f033951..2e71294c 100755 --- a/application/worker.py +++ b/application/worker.py @@ -2205,6 +2205,12 @@ def convert_source_to_wiki_worker(self, source_id, user): grouped: dict[str, list[tuple[object, str]]] = {} original_doc_ids: list[object] = [] for chunk in store.get_chunks() or []: + # Track every original chunk so a skipped one (empty/no-path/invalid) + # is purged on convert too, not left orphaned in the vector store after + # the source flips to wiki. Only deleted once pages are created below. + doc_id = _chunk_doc_id(chunk) + if doc_id is not None: + original_doc_ids.append(doc_id) metadata = _chunk_metadata(chunk) rel_path = _chunk_page_path(metadata) text = _chunk_text(chunk) @@ -2216,9 +2222,6 @@ def convert_source_to_wiki_worker(self, source_id, user): if _wiki_page_path_from_rel(rel_path) is None: skipped.append({"file": rel_path, "reason": "invalid path"}) continue - doc_id = _chunk_doc_id(chunk) - if doc_id is not None: - original_doc_ids.append(doc_id) grouped.setdefault(rel_path, []).append((_chunk_order_hint(metadata), text)) with db_session() as conn: diff --git a/tests/storage/db/repositories/test_wiki_pages.py b/tests/storage/db/repositories/test_wiki_pages.py index 4dd4d5a7..b521eb83 100644 --- a/tests/storage/db/repositories/test_wiki_pages.py +++ b/tests/storage/db/repositories/test_wiki_pages.py @@ -117,6 +117,16 @@ class TestListByPrefix: results = repo.list_by_prefix(sid, "/docs/") assert {r["path"] for r in results} == {"/docs/a.md", "/docs/b.md"} + def test_prefix_underscore_is_literal_not_wildcard(self, pg_conn): + # ``_`` is a LIKE single-char wildcard; without escaping, listing + # ``/api_v1/`` would also match the sibling ``/apiXv1/``. + repo = _repo(pg_conn) + sid = _make_source(pg_conn) + repo.upsert(sid, "/api_v1/a.md", "a") + repo.upsert(sid, "/apiXv1/b.md", "b") + results = repo.list_by_prefix(sid, "/api_v1/") + assert {r["path"] for r in results} == {"/api_v1/a.md"} + def test_list_for_source_returns_all(self, pg_conn): repo = _repo(pg_conn) sid = _make_source(pg_conn) @@ -184,6 +194,16 @@ class TestDelete: assert repo.delete_by_prefix(sid, "/dir/") == 2 assert repo.get_by_path(sid, "/other/c.md") is not None + def test_delete_by_prefix_underscore_is_literal_not_wildcard(self, pg_conn): + # Deleting ``/api_v1/`` must not also drop the sibling ``/apiXv1/`` + # via the unescaped ``_`` LIKE wildcard. + repo = _repo(pg_conn) + sid = _make_source(pg_conn) + repo.upsert(sid, "/api_v1/a.md", "a") + repo.upsert(sid, "/apiXv1/b.md", "b") + assert repo.delete_by_prefix(sid, "/api_v1/") == 1 + assert repo.get_by_path(sid, "/apiXv1/b.md") is not None + class TestExpectedVersion: def test_matching_version_updates(self, pg_conn): diff --git a/tests/worker/test_convert_source_to_wiki.py b/tests/worker/test_convert_source_to_wiki.py index 12e7de11..fa637799 100644 --- a/tests/worker/test_convert_source_to_wiki.py +++ b/tests/worker/test_convert_source_to_wiki.py @@ -132,6 +132,29 @@ class TestConvertSourceToWikiWorker: assert result["status"] == "converted" store.delete_chunk.assert_not_called() + def test_skipped_chunk_with_doc_id_still_deleted( + self, pg_conn, patch_worker_db, task_self, monkeypatch + ): + # A chunk skipped for an invalid path must still have its original + # vector chunk purged, not left orphaned after the source flips to wiki. + from application import worker + + source_id = _seed_source(pg_conn) + store = _patch_store( + monkeypatch, + [ + _chunk("good", source="ok.md", doc_id="ok"), + _chunk("bad", source="../evil.md", doc_id="evil"), + ], + ) + _patch_reembed(monkeypatch) + + result = worker.convert_source_to_wiki_worker(task_self, source_id, "alice") + + assert result["pages_created"] == 1 + deleted = {c.args[0] for c in store.delete_chunk.call_args_list} + assert deleted == {"ok", "evil"} + def test_no_pages_path_deletes_nothing( self, pg_conn, patch_worker_db, task_self, monkeypatch ):