mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-13 12:14:00 +00:00
fix: small wiki fixes
This commit is contained in:
1 parent
8b099ef1aa
commit
ef44459984
6 files changed
+73
-19
No files matched your search
@@ -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
|
||||
|
||||
@@ -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"):
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
):
|
||||
|
||||
Reference in new issue
Block a user