feat(knowledge): DocumentKB S1 — index the team's own documents so agents can cite them - #731
feat(knowledge): DocumentKB S1 — index the team's own documents so agents can cite them#731alokgp wants to merge 4 commits into
Conversation
|
@apackeer ready for a review |
|
Alok, this PR adds DocumentKB S1: a new The direction is sound. I am requesting changes because the current implementation still has several
Additional P2s:
Test-maintenance notes: credit The UX direction itself is coherent: no destructive Verification: STAMP: tests/logs/2026-08-10T04-10-31Z That slice passed 2,459 assertions; package parity, coverage-registry drift, |
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
I reviewed the current head (c30fc765) for gaps beyond the findings already documented in the existing review comment. The earlier blockers still apply because no commits have landed since that review. I found the following additional issues:
1. P1: sync publishes derived content before the authoritative commit
Location: core/tools/aidlc-knowledge.ts:1764-1772,1828-1833
The changed/retried branches write content.md and emit audit events before metadata and index.json are committed.
I reproduced a late metadata-write failure after changing a source document. sync threw, and index.json retained the old digest, but content.md already contained the new text. Because the old row still considers its derivative current, show can serve new content as though it belonged to the old revision.
Requested change: Stage content, metadata, index, and audit effects transactionally, then publish only after every write has succeeded. Add write-failure injection covering failures after content generation but before the index commit.
2. P1: a stale sync plan can overwrite a concurrent rebind
Location: core/tools/aidlc-knowledge.ts:1662-1744,1747-1807
sync plans changes and performs extraction outside the lock. After acquiring the lock, it reads the fresh index but only looks rows up by ID before applying the old plan.
It does not verify that the row still has the source path, digest, extraction state, or tombstone state used during planning. A concurrent rebind can therefore complete while sync is extracting, after which sync can apply its stale move/removal/change decision to the rebound row.
Requested change: Treat the planned row state as a compare-and-swap precondition. If the fresh row differs from the snapshot used to build the plan, skip or replan it instead of applying the stale mutation. Add mixed sync/rebind and sync/onboard concurrency tests.
3. P1: identical digests can transfer document identity arbitrarily
Location: core/tools/aidlc-knowledge.ts:1723-1735
Move detection checks whether each missing row has exactly one unclaimed file with the same digest. It does not detect competition between multiple old rows for the same candidate.
I reproduced this sequence:
- Index
a.mdandb.mdwith identical bytes. - Delete both.
- Add
c.mdwith those same bytes. - Run
sync.
One old row is reported as moved to c.md; the other is tombstoned. Which identity survives depends on index iteration order, even though there is no evidence tying c.md to either original. That can silently attach the wrong citation history to the replacement file.
Requested change: Resolve digest matches globally. If multiple live rows compete for the same candidate set, fail closed and require rebind.
4. P1: rebind leaves extraction permanently invalidated
Location: core/tools/aidlc-knowledge.ts:1711-1719,1850-1863,2244-2247
rebindDocument() explicitly sets extraction to invalidated and says the next sync will re-extract it. However, shouldRetryExtraction() only handles extractor_unavailable and selected extraction_failed states.
I reproduced a moved-and-edited text document followed by rebind and sync. sync reported unchanged; the row remained invalidated, and content.md was never regenerated.
This may be the concrete defect behind the existing review's broader extractor-retry note, but it needs explicit coverage because it breaks the documented rebind recovery path.
Requested change: Make a digest-unchanged invalidated row eligible for extraction and add a rebind -> sync -> extracted content regression test.
5. P2: onboard --intent silently ignores the requested scope for an existing document
Location: core/tools/aidlc-knowledge.ts:1124-1138
The unchanged-document shortcut returns already before intentUuid is applied.
I reproduced:
- Onboard a document without
--intent. - Onboard the same unchanged document with
--intent. - Observe status
already. - Read the row and find no
related_intent_ids.
The command succeeds but does not perform the scope operation the user requested.
Requested change: Apply the requested association idempotently before returning already, or refuse with guidance to use associate. Add an unchanged-onboard-with-intent test.
6. P2: advertised Word support cannot select a Word extractor
Location: docs/guide/12-cli-commands.md:234-236, core/tools/aidlc-knowledge.ts:414-423,555-562
The user guide advertises Word files, and configured extractors are selected by MIME type. However, MIME detection recognizes only PDF magic, text/Markdown, and generic binary.
A normal DOCX ZIP container is classified as application/octet-stream, so a configured extractor for the DOCX MIME type can never be selected.
Requested change: Detect supported Word container types, or remove the Word support claim until that detection and extractor path exist. Add DOCX MIME routing coverage.
7. P2: malformed removed_at values pass schema validation as live rows
Location: core/tools/aidlc-documentkb-schema.ts:354-403,502-505
validateRow() does not validate removed_at. A row with removed_at: {} passes validateDocumentIndex(), while isTombstoned() treats it as active because it only recognizes non-empty strings.
I reproduced this with a schema-valid row containing an object-valued removed_at.
Requested change: When present, require removed_at to be a non-empty ISO timestamp string. Add malformed tombstone cases to t276.
8. P2: extractor configuration does not require an input placeholder
Location: core/tools/aidlc-lib.ts:283-323, core/tools/aidlc-knowledge.ts:493-500
The extractor configuration validator accepts an argv array containing no $IN. The configured process then runs without receiving the document path and can record constant or unrelated stdout as the extraction for every document.
Requested change: Require exactly one $IN occurrence in each configured extractor invocation, unless an explicit and tested stdin-input mode is introduced.
Validation
I ran six deterministic probes against the shipped implementation. They confirmed:
rebind remains invalidated after sync
unchanged onboard ignores the requested intent
late sync failure publishes new content beside the old index
DOCX is classified as application/octet-stream
malformed removed_at passes schema validation
one replacement arbitrarily inherits one of two identical identities
The temporary probes were removed after validation. Existing CI remains green because the current suite does not cover these scenarios.
Answers two independent reviews of PR awslabs#731. Grouped by the invariant each class violated rather than patched per finding, because a per-case fix in this file has repeatedly closed the named site and missed its sibling. CONTAINMENT. A committed symlink at `documentkb/.journal` let a plain `sync` recursively delete an external directory while printing "Up to date." and exiting 0; a symlinked `documentkb/<id>` let sync write outside the project; and the `aidlc/active-space` cursor was read unvalidated, so `..` escaped the `spaces/` jail. Containment is now re-checked per path component through one funnel, enforced by two independent layers: a biome `noRestrictedImports` override that makes a raw mutating `node:fs` import in this tool impossible, and a TypeScript-AST completeness property that treats an unrecognised fs binding as a mutator until a human classifies it. Mutations route through `ensureDirSync`/`renameIntoPlace`/`removeTreeSync`. Four earlier attempts enumerated known-good and failed open one level up (parameter names, then primitive names, then regex-parsed imports); t277's header records the source-code-level routes that remain out of reach of any in-repo check. ROW IDENTITY. Re-onboarding an edited file created two live rows for one path; now the live row at that path IS the identity, refreshed in place and reported as `edited`. Two identical-byte rows competing for one replacement had the winner decided by array order; digests are now resolved globally and the ambiguous case fails closed, requiring `rebind`. `content` and `summary.path` are bound canonically to the row's own id, so a spliced catalog can no longer cite one document while serving another's text. `removed_at` must be a non-empty ISO string, so a malformed tombstone cannot read as a live row. `onboard --intent` on an unchanged document applied the association it previously dropped. PUBLISHED CLAIMS. `rm -rf documentkb/` then `sync` was documented as a recovery; measured, a full wipe loses ids, tombstones and intent links. The claim is narrowed to index-only recovery in four places and pinned by a pair of tests -- one proving what survives an index loss, one proving what does not survive a tree wipe. `resolveIntentFlag` now accepts a record-dir name and a canonical UUID, so the remedy its own error message suggests works. The usage line lists all seven verbs. The audit-shard scoping gap is documented rather than fixed: document events land in the space shard, but an unscoped `readAllAuditShards` resolves through the active intent, so `--doctor --export` omits them once an intent exists. TEST HYGIENE. `t275` becomes `t286` (PR awslabs#730 ships a different t275). `t279` no longer mutates the shared checkout -- it works on a scratch copy, so a killed run orphans a temp dir instead of dirtying `dist/`. The t28/t81 audit-count prose now derives 85; both pins were already correct.
Closes the remaining transactionality findings from two independent reviews of PR awslabs#731. PUBLISH ONLY AFTER VALIDATION. `onboard` renamed a staged directory into place and only then discovered the row was invalid, so an unregistered intent resolving to an empty UUID published a metadata.json the schema itself refuses while index.json was never written -- and every later `sync` failed the same validation forever, with no remedy. Both batch verbs now commit through one `assertPublishable` gate that validates the whole candidate index before anything lands, so a batch that would fail publishes nothing and a plain `sync` always recovers. COMPARE-AND-SWAP ON THE PLANNED ROW. `sync` plans and extracts outside the lock, then looked its rows up by id alone -- so a `rebind` completing in between let a decision made about the row's old identity land on its new one. The commit now requires the fresh row to still match the planned snapshot on path, digest, extraction state and tombstone state; any mismatch skips the row and leaves it for the next sync to replan from truth. A digest-only recheck would not have closed this: `rebind` changes the path, not the digest. CONTENT NO LONGER LEADS THE INDEX. A late metadata failure used to leave index.json on the old digest while content.md already held new text, so `show` served the new content as the old revision. Index and metadata publish first, so a later content failure makes `derivativeIsCurrent` read false and `show` withholds the text instead of misattributing it. JOURNAL COLLECTION UNDER THE LOCK. Stale-journal collection ran before the lock was held, where it could remove a live onboard's staging directory. It now runs inside, alongside every other mutation. Audit emission for the DocumentKB events consequently moves after the writes it describes, inverting the framework's audit-first rule. That is deliberate and now documented as a scoped exception: the catalog is derived and `sync` rebuilds it, so a missing ledger row understates history and is re-derivable, whereas a phantom row asserts a revision that never happened and survives any rebuild. The behavioural test for the compare-and-swap is titled and commented for what it actually proves. It asserts the outcome a losing race must produce, but does not exercise the precondition -- measured by stubbing the check out, which left it passing, because planning reads the index at call time and a single process never holds a stale plan. Reaching that window needs a test-only seam in the commit path or a sub-millisecond timing race; the four-field completeness is pinned structurally instead, and the limit is written down rather than implied.
… config Closes the last four review findings on PR awslabs#731. REBIND'S DOCUMENTED RECOVERY NOW COMPLETES. `rebind` sets extraction to `invalidated` and promises the next `sync` re-extracts, but `shouldRetryExtraction` only handled `extractor_unavailable` and selected `extraction_failed` states -- so a moved-and-edited document reported `unchanged`, stayed `invalidated`, and never regenerated `content.md`. An `invalidated` row is now retry-eligible, and the state always transitions away from it on that next sync, so the retry cannot repeat forever. AN EXTRACTOR THAT CANNOT RECEIVE THE DOCUMENT IS REFUSED. A configured `argv` with no `$IN` placeholder never got the document path, so whatever the process printed was recorded as the extraction of EVERY document routed to it. Exactly one `$IN` is now required: zero and two both fail closed, because a config that cannot receive its input is a config error either way. WORD FILES CAN NOW REACH A CONFIGURED EXTRACTOR. A `.docx` is an ordinary ZIP, so it classified as `application/octet-stream` and no Word extractor could ever be selected, while the guide advertised Word support. Detection now walks the real local-file-header chain and requires `[Content_Types].xml` and `word/document.xml` as declared entry NAMES. A `.docx` with no configured extractor still catalogues as `unsupported_type` and stays citable -- it was never an error before and is not now. That parser reads customer-chosen bytes, so it is bounded rather than trusted: entry count capped, every offset verified before it is read, streamed entries refused, and a self-contradictory size pair (compressed 0, uncompressed non-zero) refused. The last of those closes a resync a reviewer found -- an entry that under-declared its size made the walk step into its own payload, where forged headers naming the two markers were read as real entry names, reintroducing the NAME-versus-CONTENT confusion the parser exists to prevent one level down. Malformed input can now only end the walk early. THE BATCH CAPS ARE ENFORCED, AND SIZE IS CHECKED BEFORE THE READ. The 20-document and 256 MiB caps had no uses at all, and the 32 MiB per-document check ran only after the file was fully buffered -- 121 MB resident for a 40 MiB input that was then refused. Size is read from the existing `lstat` before the file is opened, so an oversized document is refused without being read (34 MB resident for the same input), and both batch caps now apply to pathless `onboard` and to `sync`, naming the cap and the remedy.
|
@apackeer @leandrodamascena — all 22 findings are closed at Three of your descriptions differed from what I measured, and in each case the 1. [P1] Descendant symlinks bypass the trust anchor
Measured before: a committed Containment is now re-checked per path component through one funnel. Four Pinned: Disclosed limit: this defends against a hostile filesystem, not hostile source 2. [P1] Re-onboarding an edited file creates two live rows for one path
Measured: two rows, both Pinned: 3. [P1] Orphan intent handling publishes invalid state before validation
Not just invalid state: reachable on a fresh space with zero hand-editing, and it Both batch verbs now commit through one Pinned: 4. [P1] The compiled dispatcher cannot run any knowledge verb
5. [P1]
|
There was a problem hiding this comment.
Re-review at b1962660. Several prior findings are fixed, but the current head still has merge-blocking issues.
[P1] The public /aidlc knowledge command remains unreachable
knowledge is registered as top-passthrough under group knowledge. resolveTop() only considers group top, while resolveNoun() accepts only noun-passthrough and noun-map. I reproduced:
aidlc: unknown verb 'list' for noun 'knowledge'
Change the route to noun-passthrough and add an executed dispatcher test.
Location: core/tools/aidlc.ts:367-377,741-786.
[P1] sync can publish extraction associated with stale source bytes
Source hashing and extraction happen outside the lock. The commit-time CAS validates only the catalog row, not the current source path and digest. If the source changes during extraction, sync can commit an earlier digest with text read from later bytes.
Revalidate every planned source inside the lock before publication, as onboard already does.
Location: core/tools/aidlc-knowledge.ts:2171-2224,2309-2367.
[P1] A present but unreadable or oversized source is tombstoned as removed
When readCandidate() fails, the path is omitted from byPath. Reconciliation then treats the live row as missing and tombstones it. I reproduced this by growing an indexed document above 32 MiB: sync returned change: "removed" while the source still existed.
Track refused paths separately and exclude them from removal reconciliation, or fail the complete sync.
Location: core/tools/aidlc-knowledge.ts:2171-2182,2207-2269.
[P1] A symlinked documents/ root is trusted and read
The trust-anchor check reaches documentkb/, but not documents/. sync resolves a symlinked documents/ directory and treats the external destination as its trusted walk root. External files can be read and passed to extractors before later path validation refuses publication.
Include documents/ itself in the no-symlink trust chain.
Location: core/tools/aidlc-knowledge.ts:1119-1136,2130-2182.
[P2] Edited onboard rows retain unsafe publication ordering
The edited-row path writes metadata and content.md before index.json. If the final index write fails, the old index still considers its old derivative current while content.md contains new text. Apply the index-before-content ordering already used by sync.
Location: core/tools/aidlc-knowledge.ts:1605-1626.
[P2] Catalogs above 20 documents cannot be synchronized
Twenty-one documents can be onboarded individually, but every subsequent sync refuses because it always walks the whole tree. The error recommends syncing smaller subdirectories, although sync accepts no path. I reproduced a 21-row catalog permanently refusing reconciliation.
Support scoped sync or apply the cap only to documents requiring processing.
Location: core/tools/aidlc-knowledge.ts:2145-2168; docs/guide/12-cli-commands.md:263-266.
[P2] DOCX extractor installation does not make unavailable rows retryable
extractor_unavailable does not preserve the detected MIME. detectMimeFromRow() later recognizes only PDF and otherwise returns text/plain, so an unavailable DOCX extractor is never rediscovered after installation.
Location: core/tools/aidlc-knowledge.ts:608-612,2492-2515.
[P2] $IN is accepted as the executable but never substituted
Validation accepts argv: ["$IN"], but runtime substitution only processes argv.slice(1). The tool therefore probes and spawns the literal executable $IN.
Require a non-placeholder executable at index 0 and exactly one $IN in the remaining arguments.
Location: core/tools/aidlc-lib.ts:318-331; core/tools/aidlc-knowledge.ts:616-619.
[P2] removed_at still accepts non-ISO strings
Validation requires only a non-empty string despite promising an ISO timestamp. I confirmed that removed_at: "not-a-date" passes validateDocumentIndex().
Location: core/tools/aidlc-documentkb-schema.ts:441-451.
[P2] Document audit events remain invisible to standard audit readers
Document events are written under spaces/<space>/intents/audit/, while standard readers inspect only the active intent's shard. Doctor bundles therefore omit DocumentKB history. The reference also documents the incorrect path spaces/<space>/audit/.
Location: core/tools/aidlc-knowledge.ts:118-142; core/tools/aidlc-lib.ts:3336-3378; docs/reference/12-state-machine.md:339-343.
Verification
- Targeted DocumentKB and dispatcher suites: 335 tests passed
- Package parity and coverage registry: passed
- Typecheck and lint: passed
git diff --check: passed- Deterministic probes reproduced the broken dispatcher, live-file tombstone, permanent 21-document sync refusal, and malformed
removed_atacceptance.
|
@leandrodamascena — thank you for the re-review. All 10 findings are fixed at 1. I made a false claim in my last replyI wrote that finding #4 was "closed… Now the noun-passthrough route." That was 2. So I re-audited all 22 earlier claims by executionNot by reading the diff — that is exactly what produced the false claim. 16 held. Two of the 8 were only catchable by racing the tool and by injecting a write The 10 findings[P1] The public [P1] [P1] A present but unreadable or oversized source is tombstoned as removed — [P1] A symlinked [P2] Edited [P2] Catalogs above 20 documents cannot be synchronized — fixed. My cap bounded [P2] DOCX extractor installation does not make unavailable rows retryable — [P2] [P2] [P2] Document audit events remain invisible to standard audit readers — the The verification gap, and what closed itTen user-reachable defects survived 460 passing tests, four green CI jobs and six
RED-verified against the real defects: reverting the dispatcher route in State464 tests / 1,760 assertions across 13 unit files, counted per-file and The 22-claim re-auditHeld (16): Did not hold (8), all fixed here: the dispatcher · source revalidation · Ready for another look whenever you have time. |
157205d to
9de8ebd
Compare
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Approved at 9de8ebd7. I rebuilt the branch as a single commit on current v2, moved the release to 2.5.75, preserved the upstream release history, regenerated all seven distributions, and resolved the test-number collisions.
The remaining review findings are now closed: extracted content has its own verified digest and self-heals after partial publication, fresh sync rows receive commit-time source revalidation, unchanged catalog scans no longer retain the whole corpus in memory, present non-regular sources are refused rather than tombstoned, and standard audit readers include the space-level DocumentKB shard. Extractor retry metadata and timestamp validation were hardened as well.
Verification completed:
- DocumentKB suites: 468/468 passed
- Additional renamed regression suites: 137/137 passed
- Typecheck, lint, package parity, coverage registry, and version sync passed
- Deterministic integration tests passed; the local live preflight was unavailable because the AWS credentials on this machine are expired
- All GitHub checks are green
The PR is mergeable and ready for maintainer merge.
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Withdrawing my approval after an independent post-push review found additional transaction and audit regressions that the green suite did not cover. The main blockers are permanent audit/catalog divergence after audit-last failures, a stale onboard plan that can overwrite a concurrent rebind, and side effects from merging space-level audit shards into workflow readers. Maintainer fixes are in progress; this review should remain blocking until the updated branch is pushed and independently re-verified.
0fea473 to
1d0264f
Compare
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks for putting this together. I went ahead and fixed a few issues I found during review to help accelerate the merge, including transaction races, audit recovery and security edge cases, and the upstream rebase/version conflict.
The previously identified issues are now resolved:
- commit-time CAS and topology races are covered;
- audit-last and metadata recovery are idempotent;
- audit append, fork, and merge paths are hardened;
- malformed, forged, truncated, and cross-shard audit cases fail closed;
- the branch is rebased onto v2 and correctly bumped to 2.5.76.
I re-reviewed the final branch after those changes. Local validation passed:
- 774/774 affected unit assertions;
- 17/17 audit fork/merge E2E assertions;
- typecheck, lint, package parity, coverage registry, and changelog/version checks.
The PR looks good to merge. Approving.
3 participants
DocumentKB S1 — index the team's own documents so agents can cite them
Refs #714
Adds
/aidlc knowledge <verb>and a/aidlc-knowledgeskill. A team drops PDFs,Markdown, Word files or plain text under
aidlc/spaces/<space>/knowledge/documents/; the tool derives a catalog next doorin
knowledge/documentkb/that agents can cite. Slice 1 of a multi-slicedesign — hence
Refs, notCloses.knowledge/documents/knowledge/documentkb/index.jsonrebuilds from the per-document records; the whole tree does not (see Honest limits)There is deliberately no
removeverb: deletion is "delete your own file,then
sync", so the tool never holds a destructive verb over user-owned files.Verbs:
onboard [path],sync,list,show <id>,associate/dissociate <id> --intent [slug],rebind <id> --to <path>.Version 2.5.70, merged with
upstream/v2@cbf3f30a. Seven harnesses.Review response — two rounds, 32 findings
@apackeer filed 7 P1 + 7 P2. @leandrodamascena filed 4 P1 + 4 P2, then
re-reviewed and filed 10 more (4 P1, 6 P2) — because several of the first
round's fixes did not hold.
I have to correct something I wrote here
My previous reply said finding #4 (the compiled dispatcher) was "closed… Now the
noun-passthrough route." That was false.
git logshows no commit of oursever touched that route.
aidlc knowledge <verb>returnedunknown verbfor allseven verbs the entire time, while the tool worked perfectly when invoked
directly. I reasoned from the finding text instead of running the command.
So I re-audited all 22 previously-"closed" claims by execution rather than by
reading the diff. 16 held. 8 did not. The full table is at the end of this
description so each line can be checked independently.
Why 8 claims were wrong — three distinct failure modes
aidlc-knowledge.tsdirectly, so the only entry point a user has was never exercisedsync;onboard'seditedbranch — new code in the same effort — kept the unsafe order. The trust anchor covereddocumentkb/but not its siblingdocuments/removed_atchecked non-empty while its message promised ISO. The$INvalidator counted occurrences while substitution skipped index 0Two findings were regressions from my own earlier fixes, and both were worse
than the problem they replaced: stat-before-read turned a refused read into
apparent data loss, and the batch cap bounded tree size instead of batch work,
making a 21-document catalog permanently unsyncable.
Evidence — every fix run against the shipped tool
Commands were executed against
dist/claude/.claude/tools/, through the publicaidlc.ts knowledge <verb>wherever a user would.unknown verbfor all 7 → 7/7 verbs reach the toolremoved→present_but_refused; a real deletion stilltombstonedsyncpublished against unread bytes84325551c170b698on both, under 400 concurrent overwritesdocuments/root walkedremoved_ataccepted junk"not-a-date"accepted → refused; a real ISO stamp still accepted$INaccepted asargv[0]$IN→ refused: "argv[0] must be a real executable name"synccontent.mdstaysv1 originalwhen the index write failsspaces/<space>/audit/→ events measured inspaces/<space>/intents/audit, and the doc now says soFindings 6 and 7 were checked in both directions, so the fix refuses bad input
without becoming over-strict.
The verification gap, and what closed it
Ten user-reachable defects survived 460 passing tests, four green CI jobs and six
adversarial reviews. The reason is structural: our tests spawn the tool
per-behaviour with a purpose-built fixture, so nothing ever asked "does the
documented workflow work end to end, from an empty project, using the command a
person types?"
tests/unit/t290-knowledge-journey.test.tsnow does. It drives onlyaidlc.ts knowledge <verb>as real processes — never importing the library,never calling the knowledge tool directly, because that indirection is exactly
what the dispatcher defect hid behind. It walks onboard (single + folder) → list
→ show → edit-and-reonboard → move → delete → associate → dissociate → rebind →
grow past 32 MiB → past 20 documents, and after every step asserts the catalog
is truthful, not merely non-erroring:
source.pathdocumentkb/<id>/listandshowagree; the index still validatesVerified against the real defects rather than assumed: reverting the dispatcher
route in
dist/fails the journey on its first command; reverting therefused-path guard fails the oversized-file step. An independent review reproduced
both and confirmed the truthfulness checks fire on a hand-corrupted index rather
than being unreachable.
That single test would have caught findings 1, 2, 5 and 9.
Tests: 464 passing across 13 unit files, 1,760 assertions
t278-knowledge-transactiont285-knowledge-skillt279-document-extractors-seamt286-read-write-boundaryt280-knowledge-extractiont287-knowledge-sync-cast281-knowledge-list-showt288-documentkb-schemat282-knowledge-linked-sourcest289-knowledge-onboard-boundaryt283-knowledge-intentst290-knowledge-journeyt284-knowledge-sync-rebindPer-file counts were taken individually and cross-checked against one batch
run; both give 464 / 1,760. Unit tier deliberately: CI gates
--smoke --unitonly (
.github/workflows/ci.yml:66). Every new test was RED-verified by revertingits fix in
dist/— acore/-only revert proves nothing, since the tests spawndist/.Local gate status
All four GitHub jobs verified locally:
biome --error-on-warnings)t248, belowzensical --strict)t248-codekb-scope-difffails with 2 assertions. Not ours: our branch touchesno
t248or codekb file, and the same two assertions fail in a cleanupstream/v2worktree. CI's Linux runner passes it.Honest limits
is later. S2 is blocked on [Feature]: auditable supplemental-knowledge selection and delivery across stage topologies #694.
index.jsonrebuilds from survivingper-document records, tombstones included. Deleting the whole
documentkb/tree deletes those records too, so ids, tombstones and intent links do not
survive it. Both sides are pinned by a pair of tests.
spaces/<space>/intents/audit/, but an unscopedreadAllAuditShardsresolvesthrough the active intent, so
--doctor --exportomits them once work hasstarted.
list/showare unaffected — they read the catalog, not the ledger.The write side is correct; the gap is on a framework-shared read path, and the
reference chapter now states the real path and this consequence. This is the
third time it has been raised; it is recorded as a real limitation, not as
"fixed".
t287's behavioural CAS test does not exercise its precondition. Measured bystubbing the check out and watching it still pass: planning reads the index at
call time, so one process never holds a stale plan. Four-field completeness is
pinned structurally, and the limit is stated in the test's own header.
Bun.write,spawnSync("rm")and a re-exportedrmSyncall evade it; eachneeds code added to the module. Recorded in
t289's header.writer.pidof1makes a journal staging dir permanentlyuncollectible — a leak, not an escape; the cost of fail-safe over fail-clean.
uncovered. No independent verb-discovery signal exists in that table.
copilotandcursorare packaged and pinned, not exercised against a livehost install.
The full 22-claim re-audit
Verified by execution, not by reading the diff. 16 held, and the 8 that did not
are the findings fixed in this round.
Held:
.journalsymlink refusal · one live row on re-onboard · orphan-intentrefusal with no poisoning · cross-row
contentsplice refused · index-onlyrecovery preserving ids and tombstones ·
rebindsurviving a latersync·identical-digest competition failing closed ·
rebind→syncre-extracting ·cursor
../../../evilrefused · extractor-retry docs namingsync·same-slug intents resolvable by record-dir name and by UUID · help listing all
seven verbs · 85 audit event types with no assertion changed ·
t279leaving thecheckout clean ·
onboard --intentapplying on an already-indexed row · DOCXdetection with the decoy zip correctly rejected.
Did not hold (all fixed here): the dispatcher ·
syncrevalidating sourcebytes · edited-onboard publish order · the audit-shard claim · the batch caps ·
removed_at·$IN· extractor-unavailable retryability.Two of those — source revalidation and the edited-row ordering — were only
catchable by racing the tool and by injecting a write failure. Neither was
reachable by reading the code, which is why the journey test above exists.