fix(platform): harden knowledge ingest, corpus scope and retrieval - #3249
Merged
Conversation
reconcileDocumentScopeStamps compared a fixed `ORDER BY id LIMIT 1000` subset with no cursor. Document ids are uuids, so an organization with more than a page of live file-backed documents had the same arbitrary thousand re-checked every nightly run and the rest never visited: a stamp that drifted while the corpus was unreachable stayed wrong indefinitely, and the "corrected" warning under-reported. The reconcile now walks the whole live corpus in keyset pages (`id > last`, one document read, one folder-tree read and one corpus UPDATE per page) and stops on the first short page. The page size stays the `limit` argument. The tests' `sql` double answers the read the way the page reads it, and two new cases pin the walk (every document compared once, the cursor advancing, the boundary read). Finding: knowledge-domain-3.
…eues requeueEmbeddingBlockedDocuments flipped the `failed — no embedding model` rows to `queued` and enqueued their jobs in one transaction, but never emitted the realtime hint the document lists refetch on. Every other viewer kept seeing the failure and its Settings deep link until the worker's first `running` write per file — minutes with a backlog behind one worker — for a problem the admin had just fixed. The transition now emits the same org-wide document hint the sibling re-queue (`requeueRefusedFiles`) emits, inside the same transaction, and only when a row actually moved. Finding: knowledge-domain-10.
The #3220 decision (an emailed attachment is retrievable inside the scope of the conversation it arrived on) could not fire for any person. The reused search/fetch modules hand the caller's identity to the retrievable filter as a top-level `userId`, but the shim's `filterRetrievableRagFileIds` handler cast that field away and passed no `caller`, so the conversation branch always denied. And no 0.5 scope producer set `includeConversationScoped`, so the corpus pre-filter never admitted such rows and `fetchDocumentByFileId` short-circuited them. - The shim handler now resolves `userId` to a decided caller through the member row (`isAdminRole` for the admin arm; no live member is no caller) before calling the filter. - The chat turn's knowledge scope (`resolveKnowledgeAccess`, also the user-keyed sandbox session's) admits conversation-scoped rows for a live member; a disabled or missing member still gets nothing. - The itest emailed-attachment lane now dispatches the wire shape (`userId`) instead of a hand-built caller, and adds the anonymous denial. The app's Knowledge search page keeps its explicit `includeConversationScoped: false`. `resolveKnowledgeAccessForUser` was already gone from `core/documents/access.ts`. Finding: knowledge-domain-1.
…ized attachEntryDocument enqueued `rag.index_file` for a file row whose `rag_status` was NULL — the first version's INSERT set none, and the rotate branch reset it to NULL explicitly. NULL reads as "Not indexed" in the document list until the worker's first `running` write, and a job lost after its retries left the row there forever: the indexing watchdog only reconciles `queued`/`running` rows. Both branches now call `markRagQueued` (the producer idiom every other enqueue uses) before enqueuing, so the row reads "Queued" from the enqueue on and a lost job is a stale queued row the watchdog sees. The rotate branch clears the previous version's failure text and code with the same write, since the rotated blob is new content. Finding: knowledge-domain-8.
storeEntryBlob PUT the entry's markdown to the presigned object-store URL with a bare `fetch` — no signal, and Node's fetch has no timeout of its own. A stalled S3/MinIO connection held the create/update handler (app and REST) and the caller's spinner indefinitely, with the rate-limit token already spent. It is the only server-side fetch of a presigned PUT in the backend; the other presign sites hand the URL to the client. The upload now carries `AbortSignal.timeout(30s)`, the repo's outbound idiom, and the abort maps to a coded `KNOWLEDGE_ENTRY_STORE_TIMEOUT` 503 (the status union widened) that the app route answers as such; any other store failure passes through unchanged. Finding: knowledge-domain-9.
A knowledge entry's backing document is an ordinary hub row, so it can be trashed or purged through any document door — a WebDAV DELETE, the WebDAV folder cascade, the Documents tab's purge, the retention sweep. Only the retention purge touched `app.knowledge_entries`; every other door left the chain active: the entry stayed listed, counted and served to the agent leg while its corpus rows were dark, and an edit re-materialized a new version onto the trashed document (indexed, never retrievable). 0.4's `deleteDocument → markEntryChainDeleted` hook was never ported. `markEntryChainDeletedForDocument` is now the ONE org-scoped soft-delete of the chain a document backs (every version sharing the topic key), called from the WebDAV single-document trash, the WebDAV folder cascade, and the purge the Documents tab and retention share. `updateKnowledgeEntry` joins the document's lifecycle and answers KNOWLEDGE_ENTRY_NOT_FOUND for a backing document that is no longer active. Two integration proofs drive the purge door and the WebDAV door end to end. No change to `domains/documents/service.ts` was needed: the tab's delete already funnels through the shared purge. Finding: knowledge-domain-2.
`core/knowledge_entries/helpers.ts` still carried findActiveEntryByTopicKey, upsertEntryRow, markEntryChainDeleted and UpsertEntryResult, all written against a Convex MutationCtx no 0.5 backend has; only their own tests called them, and the header cited a `documents/mutations.ts` delete hook that does not exist in 0.5 (the hook itself is `markEntryChainDeletedForDocument` in the service now). Only validateTopicAndContent is reused. The helpers, their fake-ctx tests and the ctx/row type imports are removed, and KNOWLEDGE_SOURCE_PROVIDER — defined and never read — now names the `source_provider` the service used to hard-code. Finding: knowledge-domain-4.
A file refused while a BM25 index was being rebuilt parks under `index_rebuilding` with a note that indexing "resumes automatically — no action is needed". That was true on two of the rebuild job's arms (`repaired`, `healthy`) and false on every other path: `missing` lifted the refusal and returned; `invalid` and `unverifiable` returned with the refusal in place and nothing scheduled; `repair_failed`/`not_retried` hardened the refusal while the parked rows kept promising a resumption; and the write guard's own self-lift (a write re-verified the index healthy after another process's rebuild or an operator's REINDEX) resumed writes and left the parked files behind. The watchdog never re-queues, so those rows stayed parked for good. - `missing` re-queues what it parked, like `healthy`. - `invalid` is a failed repair: refusal hardened, announced with the operator move (DROP INDEX CONCURRENTLY …), parked files re-stamped. - `repair_failed`/`not_retried` re-stamp the parked files with the repair-failed code and prose (new `failRefused` effect). - `unverifiable` re-schedules the rebuild after five minutes instead of dropping it (`scheduleRebuild` takes a delay). - The core write guard gains `setCorpusWritesResumedHook`; the app installs it next to the bootstrap hook and re-queues everything parked on that database (new `resumeRefused` effect, orgs resolved by URL). - The requeue selects both index codes, since a healthy index resumes what a failed repair parked too; the repair-failed prose says so. Finding: knowledge-domain-6.
applyPin compared the chunks column's declared width with the configured model's, warned when they differed, and then ALTERed the column to the new width anyway. The only refusal was the in-memory pin map, empty after every process start. On the shared bundled database, tenant A pins vector(1536); after a restart, tenant B's config (768) searches or indexes first: the ALTER takes an ACCESS EXCLUSIVE lock on every tenant's chunks table and begins a rewrite pgvector then rejects — every tenant blocked meanwhile, B handed a raw database error on every request — and on an EMPTY table it succeeds, silently re-pinning the whole corpus (semantic cache truncated), so every 1536 tenant's next write fails. The module doc promised the opposite. Only an untyped `vector` column is ALTERed now. A column declared at another width is refused with EmbeddingDimensionMismatch and its width is recorded as the database's pin, so later callers are answered from memory and a matching caller proceeds; a declaration this module cannot read as a width is refused outright. Finding: knowledge-core-3.
The embedder's OpenAI client kept the SDK's defaults: a ten-minute per-request timeout and two internal retries, stacked under this module's own three-attempt loop. A black-holed or slow-dripping provider is not a connection error, so one batch took at least thirty minutes to fail — twice the 900 s budget of `rag.index_file`. pg-boss's expiry does not cancel the handler: it failed the job and re-ran it (up to five times, backoff) while the first handler kept embedding the same file, so two indexers wrote `running` and chunks for one document; on the search path a chat or REST request hung just as long. The client now carries a 60 s request budget and `maxRetries: 0`, so this module's loop (which already treats a connection timeout as retryable) is the one retry policy and the worst case for a batch stays well inside the job budget. Finding: knowledge-core-4.
applyCorpusSchema read a schema's ledger and applied its pending files with no lock and no transaction spanning the two. A deployment runs at least an api and a worker, each lazily bootstrapping a fresh bring-your-own database on first use; concurrent `CREATE … IF NOT EXISTS` on the same objects is not atomic in PostgreSQL, so the loser died on a catalog unique violation that the first search, index or crawl request on that process then surfaced (the single-flight retried on the next call, so it was transient, not corrupting). The apply now runs on one reserved connection holding a session-level advisory lock on the knowledge database (`CORPUS_BOOTSTRAP_LOCK_KEY`, distinct from the index-repair key), and reads the ledger only once the lock is held, so a process that waited on another's bootstrap sees its ledger and applies nothing. Session-level rather than transactional because the baseline files carry their own BEGIN/COMMIT. Finding: knowledge-core-5.
Discovery swallowed every sitemap and link-walk fetch failure with a
bare `catch { continue; }` — a timeout, the size cap, an SSRF refusal
of the host, a non-2xx answer — so a large site quietly degraded to a
small link walk and the scan reported a handful of URLs with nothing in
the logs saying which of those it was. The sibling robots.txt and page
fetches in the same file already warn with the URL and the message;
these now do the same, control flow unchanged.
Finding: knowledge-core-7.
…wl shim `internal.knowledge.crawl_ops.*` named six handlers no ctx-shim table in the codebase mapped, and the five wrappers in `core/websites/internal_actions.ts` dispatching them (plus `deregisterAndDeleteWebsiteRow`, and with it the orphaned `websites.internal_mutations.deleteWebsite`) had no caller: their 0.5 equivalents register, re-scan and deregister domains directly in `domains/websites/service.ts`. A handler name with no table entry throws at the call, not at type-check, and the websites surface was the one ctx dispatch with no reachability gate to say so. The block and the module are gone; `scanIntervalToSeconds` moves next to its only consumer. `domains/websites/shim.test.ts` now walks the crawl engine's import graph on the shared reachability walk and requires a handler (or one of the two scheduled refs the job mapper answers) for every `internal.*` it reaches. It excludes `session_exec.ts` — the render lane imports only `runStepsInSession`, which dispatches nothing — and asserts both halves of that reason so the exclusion cannot silently widen. Finding: knowledge-core-1.
`lib/knowledge/search-node.ts` defined a `knowledge.search` automation
node whose two wiring calls — `registerKnowledgeSearchNode` and
`setKnowledgeSearchBackend` — had no caller anywhere, so the node type
was never registered and its module-level backend stayed null; the only
production reference to the module was `knowledgeSearchBackendFor` in
`core/knowledge/search.ts`, itself never called. Retrieval has exactly
the callers the header now names (the chat tools, the app's Knowledge
page, the REST search endpoint), all through `searchKnowledgeForOrg`.
The module, its test and the backend factory are gone. With them go the
`lib/knowledge/index.ts` barrel — no importer, and a header describing
`convex/knowledge/` seams that do not exist — and `lib/engine/index.ts`,
whose last importer was that deleted test. `encodeMessageRef` and
`parseMessageRef` (unused) are dropped; `isMessageRef` replaces the two
literal `startsWith('msg:')` checks in the retrievable filter and the
ref release. The `lib/knowledge/**` knip parking (PR #2857) is lifted,
since every remaining module is imported directly by `core/knowledge/*`
and the domains; the constants and helpers only their own tests read
are no longer exported.
Findings: knowledge-core-2, knowledge-domain-5.
Since the corpus bootstrap serializes on one reserved connection (`applyCorpusSchema` calls `sql.reserve()` and holds the advisory lock on that session), every pool test that routes an organization to its own database bootstrapped it through a double with no `reserve`, and four tenant-routing and health-hook cases failed with "sql.reserve is not a function". The double now answers `reserve()` with the same recording surface and a no-op release, as the ddl double already does. Follow-up to the corpus bootstrap lock (knowledge-core-5).
…reads `closeKnowledgePools` existed but nothing in the process called it: the shutdown sequence closed the HTTP server, stopped pg-boss gracefully and ended the app pool, then exited with every knowledge database connection — the shared one and each bring-your-own pool — left to `process.exit`. The shutdown now drains them between `boss.stop` and `sql.end`, and the pool test proves the close ends every open pool and forgets it. `pinnedDimensions` and `corpusWriteRefusal` were exported for their own tests only; the tests now observe the same facts through the production doors — a pinned width answers the same width from memory and refuses another, an unpinned database is read again, and a refusal is what `assertCorpusWritable` throws (or does not). The re-check window is pinned at its edge (the next write after the window verifies again) instead of by reading the internal deadline. `forgetPinnedDimensions` is documented as the tests-only seam it is. Finding: knowledge-core-6.
…s topic `markEntryChainDeletedForDocument` (bf0df76) soft-deleted every live row whose topic_key matched ANY row pointing at the document — the subquery had no deleted_at_ms filter. A deleted chain frees its topic key (`findActiveByTopicKey` ignores deleted rows), so a member who deletes entry Foo (chain soft-deleted, doc A trashed) and re-creates Foo gets a NEW chain on doc B. When the retention sweep or the Documents-tab Trash later purged doc A, the hook found 'foo' through A's deleted rows and retired the live chain on B. main's purge predicate was document-only; this restores it. Every version of a chain re-materializes onto the same document (`attachEntryDocument` rotates the existing one), so the document IS the chain and the topic-key hop was never needed. The unit test now pins the parameter tuple (org, document) and a single statement; the knowledge entries integration lane adds the real sequence — delete, re-create the same topic, purge the old document — and checks the new chain stays live, listed and editable while the old rows stay deleted. Refs knowledge-domain-2 (review follow-up on #3249).
The rebuild job's `not_retried` and `invalid` arms announced `repair_failed` with `path: 'background', reindexMs: 0` although no rebuild ran on that pass, so the audit row read as a background rebuild that took 0 ms. Both now announce `path: null` (the union already allows it, and the scan-path `not_retried` arm already did so). The invalid-arm test pins the shape. Refs knowledge-domain-6 (review follow-up on #3249).
The retention sweep's Pass A (documentsRetentionDays set, deletionGraceDays > 0) flips ACTIVE app.documents rows to lifecycle_status 'expired' with no source filter, so a knowledge entry's backing document (lifecycle NULL, created by the entry author) is a candidate like any other. That flip is a door that hides a document, and it did not call markEntryChainDeletedForDocument — only the WebDAV trash lanes and the purge did. For the whole grace window the chain stayed live: the entry was listed, counted and served to the agent leg while the retrievability filter kept its corpus rows dark, and an update onto it answered 404 for a row the list still showed (knowledge-domain-2, re-review round 2). The flip now runs in one transaction with the chain retire, through a new batch form of the hook, markEntryChainsDeletedForDocuments (one statement, document_id = ANY(ids), the same org + deleted_at_ms IS NULL predicate); the single-document hook delegates to it. The sweep unit test pins the retire right after the flip with the flipped ids, and its absence when the flip hides nothing; the knowledge entries itest lane ages an entry's document past a 100-year policy, runs the sweep with grace, and asserts the document is 'expired', the chain deleted, the entry unlisted and its update a 404. The only other lifecycle flip on app.documents is the project cascade, which a knowledge document never reaches (no INSERT or UPDATE gives one a project_id).
…hy index The repair_failed note promised that indexing "resumes on its own once the index verifies healthy again", but the promise was kept only in-process: the write guard's self-lift fires for a refusal in THIS process's memory, and the job's healthy/repaired arms run only while the singleton job is still pending. After an operator's REINDEX and a restart, the boot scan's healthy arm re-queued nothing, so the parked files stayed parked forever under a note that said they would resume — with the actionable "retry indexing" instruction gone (knowledge-domain-6 class, re-review round 2). applyIndexHealthReport now re-queues the parked files once per report when every index verifies healthy (found so, or repaired inline) — decided per report, not per index, so a database with one healthy and one still rebuilding index does not re-queue files the guard would only park again. The scan-path repair_failed and not_retried arms also re-stamp the files parked as "rebuilding" with the operator prose (failRefused), as their job twins do. The prose now names when the resume happens: at the next indexing attempt or service start. Tests pin the healthy-report requeue (one call, scope + url), the no-requeue cases (unverifiable/invalid, healthy next to deferred, empty report), the repaired-inline requeue and the scan-path failRefused calls.
A corpus baseline file carries its own BEGIN … COMMIT and is applied with one multi-statement unsafe() call. A statement failing midway left the reserved session in an aborted transaction: the pg_advisory_unlock in the inner finally then failed too (masking the migration's own error), and session.release() returned the connection to the pool with the session-level bootstrap lock still held — every later bootstrap of that database waited on it forever. The apply now rolls back on a failed file before rethrowing, so the unlock runs and the failure that surfaces is the migration's. The ddl test drives a file dying midway and pins ROLLBACK → unlock → release, no ledger row for the failed file, and the original error.
The new retention-expiry lane aged the entry's document 100 years, which is a negative millisecond epoch: the sweep floors a document's age at coalesce(status_changed_at_ms, 0), so the row could never age out and the lane read the untouched active row as "missing". A ~55-year window keeps both the cutoff and the aged stamp positive while still older than any real row of the org; the lane now prints the raw lifecycle status.
…ession #3235 gave the purge its pool through getKnowledgePoolForOrg with a double that answers unsafe/begin; this branch's corpus bootstrap now takes its advisory lock on sql.reserve(), so the double answers that too — the same shape pool.test.ts already uses.
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
Knowledge theme of the backend fix campaign (knowledge-domain + knowledge-core slices): ingest, corpus scope and retrieval hardening. Sixteen commits, one per finding (or one tight group), each behaviour change with a regression test — plus the review follow-up commits of rounds 2 and 3 (see the sections at the end).
userId, and the chat turn's knowledge scope admits conversation-scoped rows for a live member.reconcileDocumentScopeStampswalks the whole live corpus in keyset pages instead of re-checking a fixed arbitrary thousand. There is deliberately no per-run page cap: the nightly cost is three statements per 1000 live documents (a 100k-document org spends ~300 statements per night), which is the known quantity that buys a walk that always completes without a cursor table.index_rebuildingresumes (or is re-stamped with the repair-failed code) on every arm of the rebuild job, on the write guard's own self-lift, and — new in round 3 — when a boot scan finds every index of the database verified healthy (the cross-process case after an operator'sREINDEX+ restart); an embedding-config re-queue emits the document hint.knowledge.searchautomation node, thelib/knowledgeandlib/enginebarrels, the Convex-era entry helpers, thecrawl_opsdispatch residue (with a reachability gate on the crawl shim) and three test-only exports are gone; thelib/knowledge/**knip parking is lifted.boss.stopandsql.end.Findings fixed
filterRetrievableRagFileIdsresolvesuserId→ caller (member role); chat scope admits conversation-scoped rows for a live member; itest lane dispatches the wire shape + anonymous denial.markEntryChainDeletedForDocument(org + document keyed) called from WebDAV trash, WebDAV folder cascade, the shared purge and — batch formmarkEntryChainsDeletedForDocuments— the retention sweep's expiry flip, in the flip's transaction;updateKnowledgeEntryanswersKNOWLEDGE_ENTRY_NOT_FOUNDfor a non-active backing document; three itest proofs (purge, WebDAV DELETE, and the delete → re-create → purge-old-document regression).id > last), stops on the first short page.core/knowledge_entries/helpers.tsrow helpers + fake-ctx tests removed;KNOWLEDGE_SOURCE_PROVIDERnow read.lib/knowledge/index.tsbarrel,encodeMessageRef/parseMessageRefremoved;isMessageRefused at bothmsg:sites;lib/knowledge/**un-parked in knip; test-only constants un-exported.unverifiablere-schedules;setCorpusWritesResumedHookre-queues on the write guard's self-lift; a boot/bootstrap scan whose report leaves every index verified healthy re-queues once per report (scan-pathrepair_failed/not_retriedre-stamp like their job twins); thenot_retried/invalidarms announcepath: null(no rebuild ran) so the audit row never claims a 0 ms background rebuild.attachEntryDocumentcallsmarkRagQueuedon both branches before enqueueing.storeEntryBlobPUT carriesAbortSignal.timeout(30s); abort → codedKNOWLEDGE_ENTRY_STORE_TIMEOUT503.requeueEmbeddingBlockedDocumentsemits the org-wide document hint inside the transaction when a row moved.crawl_opshandler names +core/websites/internal_actions.tsremoved;scanIntervalToSecondsmoved to its consumer;domains/websites/shim.test.tsgates everyinternal.*the crawl engine reaches.lib/knowledge/search-node.ts(+ test) andknowledgeSearchBackendForremoved;search.tsheader names the real callers;lib/engine/index.ts(last importer was that test) removed.applyPinALTERs only an untypedvectorcolumn; another declared width →EmbeddingDimensionMismatch, recorded as the database's pin; unreadable type refused outright.maxRetries: 0(this module's loop is the one retry policy).applyCorpusSchemaruns on one reserved connection underCORPUS_BOOTSTRAP_LOCK_KEY(session-level advisory lock), ledger read after the lock.closeKnowledgePoolscalled in the shutdown sequence (pool test proves close + forget);pinnedDimensionsandcorpusWriteRefusalremoved, tests observe throughpinDimensionsstatements /assertCorpusWritable;forgetPinnedDimensionsdocumented tests-only.catch { continue; }.Skipped
mainby fix(platform): close document review, cascade and blob-leak gaps #3227 (4a24de2): the lane now picks a live document that HAS a corpus row (file_ref = ANY(corpus file_ids),ORDER BY created_at_ms, id). Verified green in this branch's integration run (line below) and in every sibling run based on ≥ fix(platform): close document review, cascade and blob-leak gaps #3227; thedrifted=nullfailures in the brief were all on pre-fix(platform): close document review, cascade and blob-leak gaps #3227 bases.Tests & gates observed
All gates re-run after the round-3 follow-up commits (
d840f67aa,ca5dda627,748f9cbfa,17fd1f515,59fa33b5d; HEAD59fa33b5d):bun run check(worktree root):Tasks: 40 successful, 40 total·@tale/platform:test: Test Files 532 passed (532) / Tests 73364 passed (73364)·@tale/platform:test:ui: Test Files 456 passed (456) / Tests 3512 passed (3512)·@tale/cli:test: 334 pass / 0 fail·@tale/shared,@tale/docs,@tale/ui,@tale/webpassed ·CHECK_EXIT=0(tsc, oxlint, format:check included;Time: 2m0.082s).bunx tsc --noEmitexit 0;bunx oxlint --type-awareexit 0;bunx vitest --run --project server:Test Files 524 passed (524) / Tests 6125 passed (6125).bun run knip:check: exit 0 — one pre-existing configuration hint (cron-parser services/platform knip.config.ts Remove from ignoreDependencies, unchanged from main).run-itest.sh, real Postgres + pg-boss), third run of round 3:[itest] 479/480 checks passed across 134/134 lanes, noRUN TRUNCATED. The one FAIL iswebdav re-home (protocol + tree + locks + visibility on pg)— pre-existing on main and owned by the webdav theme. The two earlier round-3 runs truncated (RUN TRUNCATED at checkWatchdogs/at checkReviewArc) on the harness's knownlockChainHeadserialization-retry exhaustion (could not serialize access due to concurrent update, 5/5 attempts → 500 → the lane's JSON parse throws) — the same stack appears in 11 campaign logs across 8 other themes including a main-based run; my diff adds no audit writes. The first of those runs also had my new lane red for a lane bug of its own (a 100-year policy is a negative epoch the sweep floors to 0), fixed in59fa33b5d.PASS knowledge entries: the retention sweep's expiry flip retires the chain with the document — swept=1 (want ≥1), doc=expired (want expired), chain=true (want true), listed=false (want false), update=404 (want 404)(the round-3 regression lane)PASS knowledge entries: purging a deleted chain's old document spares the chain that reused its topic — delete=200 (want 200), recreate=true newDoc≠old=true, purgeOld=200 (want 200), newLive=true (want true), oldDeleted=true (want true), listed=true (want true), update=200 (want 200)(the round-2 regression lane)PASS knowledge entries: purging the backing document retires the chain — purge=200 (want 200), chain=true,true (want 2× true), listed=false (want false), update=404 KNOWLEDGE_ENTRY_NOT_FOUNDPASS knowledge entries: a WebDAV DELETE of the backing document retires the chain — created=true, delete=204 (want 204), doc=trashed (want trashed), chainDeleted=true (want true), update=404 (want 404)PASS knowledge entries: chain + stable corpus key + agent leg + delete,PASS knowledge entries: member role is read-onlyPASS knowledge: corpus scope drift is corrected and reported — drifted=["itest-drifted-team"], first=corrected 1/scanned 12, repaired={"teamIds":null,"projectId":null} …PASS emailed attachment: the bind un-skips, queues and dispatches indexing inside the conversation scope — rows=1 (want 1) … skip=false (want false) …rag.index_filejobs against a corpus pinned at another width fail withEmbeddingDimensionMismatch(the core-3 refusal) instead of anALTER TABLE— visible in the worker log of the run.Notes for the reviewer
domains/documents/service.tswas NOT changed for domain-2: the Documents tab's delete already funnels through the shared purge, which is one of the three hook sites (the README anticipated an additive change there; none was needed).core/legacy/knowledge_delete.tsand thelegacyhandler-name block (reserved for fix(platform): route legacy knowledge purges through the live pool and prune dead script paths #3235) are untouched; core-1 removed only thecrawl_opsblock andwebsites.internal_mutations.deleteWebsite.test(platform): teach the knowledge pool double the reserved sessionrepairs fourpool.test.tscases that the core-5 commit (bootstrap now callssql.reserve()) had left red — the double had noreserve.knip:checkprints one pre-existing configuration hint (cron-parserinignoreDependencies, unchanged from main); exit 0.webdav re-home (protocol + tree + locks + visibility on pg)(webdav theme).KNOWLEDGE_ENTRY_STORE_TIMEOUTcode followsKNOWLEDGE_ENTRY_NOT_FOUND(the app maps onlyKNOWLEDGE_ENTRY_DUPLICATEspecially); the repair-failed prose is an English backend literal like its siblings; the operate docs already describe automatic re-queue after a rebuild and carry no "retry indexing afterwards" instruction in any locale.governance/trash.tsrestoreSoftDeletedRow, resource typedocument, writeslifecycle_status = NULLwhich every reader treats as active) does NOT resurrect its entry chain — the chain stays soft-deleted while the document comes back as an ordinary hub row. That matches 0.4, where the delete hook was final; flagged here rather than widened. (WebDAV has no restore door; an earlier revision of this body named the wrong door.)main.tshas no unit harness; the shutdown ordering change is covered by thecloseKnowledgePoolscontract test and observed through the integration run's process lifecycle only.Review follow-up (round 2)
c49d6ba85):markEntryChainDeletedForDocumenthoppedtopic_key IN (SELECT topic_key … WHERE document_id = $doc)with nodeleted_at_msfilter in the subquery. SincecreateKnowledgeEntryonly checks the ACTIVE chain, a deleted topic's key is reused by a new entry on a new document; a later purge of the OLD trashed document (retention sweep or Documents-tab Trash) then retired the NEW live chain. Restored main's document-only predicate (org_id = $org AND document_id = $doc AND deleted_at_ms IS NULL) — every version of a chain shares its document, so the hop was never needed. Unit test now pins the[now, org, doc]tuple and a single statement; new itest laneknowledge entries: purging a deleted chain's old document spares the chain that reused its topicdrives the real sequence over the wire.invalidarmpath: 'background'(fixed,4d140fbe6):path: nullon both theinvalidand the job-pathnot_retriedarm (same pattern, same file; the scan-pathnot_retriedarm already did this).restoreSoftDeletedRowDOES coverapp.documents(TRASH_SOURCES.document,restoreValue: null), so a document restore door exists; the caveat stands with the door name corrected from WebDAV to the admin Trash surface.Review follow-up (round 3)
d840f67aa+59fa33b5d): confirmed by reading —sweepDocumentsPass A (documentsRetentionDaysset,deletionGraceDays > 0) flips ACTIVEapp.documentsrows to'expired'with no source filter, and nothing retired the chain until the later purge, so for the whole grace window the entry stayed listed/counted/served whileisActiveLifecyclekept its corpus rows dark. The flip now runs in one transaction with the chain retire through a new batch form of the hook,markEntryChainsDeletedForDocuments(one statement,document_id = ANY(ids), sameorg_id+deleted_at_ms IS NULLpredicate; the single-document hook delegates to it). Unit tests: the sweep double answers the Pass A flip and pins the retire as the statement right after the flip with the flipped ids (and no retire when nothing flipped); the hook test pins the batch tuple and the empty-batch no-op. Itest: new laneknowledge entries: the retention sweep's expiry flip retires the chain with the document(entry created over the wire, document aged past a ~55-year policy, realsweepOrgPhase2with grace → document'expired', chain deleted, entry unlisted, update 404). Swept the otherapp.documentslifecycle flips: the only other one is the project cascade (projects/service.ts), which a knowledge document never reaches — no INSERT or UPDATE gives one aproject_id— so it was left alone. Header comment and the Summary now name the expiry flip explicitly.ca5dda627): confirmed — after an operatorREINDEX+ restart the boot scan'shealthyarm re-queued nothing.applyIndexHealthReportnow re-queues the parked files once per report when EVERY index of the database verifies healthy (found so, or repaired inline). Decided per report rather than per index on purpose: a database with one healthy and one still-deferred index would otherwise re-queue files the guard immediately parks again (the reviewer's per-index one-liner has that churn). Tests pin: a healthy report → exactly onerequeueRefused(scope, url)and no announce; healthy + unverifiable/invalid → nothing; healthy next to deferred → schedule only; empty report (locked/no_indexes) → nothing; repaired-inline → one requeue. The prose now also names when the resume happens (… at the next indexing attempt or service start), because an idle deployment whose operator repairs the index by hand is re-verified only by the guard's next write or a boot — the sentence is now literally true in every case.repair_failed/not_retrieddo notfailRefused(done, same commit): both scan arms now re-stamp the files a previous process parked as "rebuilding", as their job twins do; tests assert the(scope, url, index)call.crawlHandlers/SCHEDULED_CRAWL_REFStest-only exports (done,17fd1f515): both doc comments say "Exported for tests only" and name the production path (crawlCtx).ddl.tsaborted-transaction release (done,748f9cbfa): verified worse than stated — in an aborted transaction thepg_advisory_unlockin the innerfinallyfails too, so the session-level lock returned to the pool still held and every later bootstrap of that database waited forever. A failed migration file nowROLLBACKs before rethrowing; ddl test drives a file dying midway and pinsROLLBACK → unlock → release, no ledger row, the original error.ca5dda627, 77 chars) has the same defect, noted here rather than force-pushed.