Skip to content

feat: add Knowledge project resource links - #88

Merged
andrei-hasna merged 4 commits into
mainfrom
feat/4080541a-knowledge-project-links
Aug 10, 2026
Merged

feat: add Knowledge project resource links#88
andrei-hasna merged 4 commits into
mainfrom
feat/4080541a-knowledge-project-links

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a package-owned Knowledge project/collection aggregate with deterministic stable IDs
  • add explicit bind-existing item membership, immutable forward/inverse receipts, and exact readback
  • add complete project/collection/item/taxonomy enumeration with population-digest keyset cursors
  • expose SQLite/Postgres, REST/OpenAPI, SDK, CLI, and generated package parity

Contract

Implements DUB-00030 against docs/knowledge/projects-resource-link-contract-v1.md. Ordinary or later-created Knowledge items remain outside the collection until explicitly bound. Hosted production acceptance remains a separate deployment/version-skew gate and is not claimed by this source PR.

Verification

  • bun test: 479 pass, 2 skip, 0 fail
  • focused project-link regression: 5 pass, 0 fail, 70 assertions
  • bunx tsc -p tsconfig.build.json --noEmit
  • bun run build
  • bun run verify:generated: 6 generated bundles byte-identical
  • bun run contracts:conformance
  • contracts no-cloud-scan .
  • bun run release:pack:check
  • bun test tests/package-release.test.ts: 6 pass, 0 fail
  • staged and branch gitleaks: no leaks found

Review boundary

Exact candidate: d034864bc836d7b87e627a6353a879a77fd3125a. Acceptance is zero in-scope P0/P1 defects affecting aggregate identity, explicit membership, receipt/inverse safety, population completeness, backend/API/SDK/CLI/OpenAPI parity, or required package gates. P2/P3 and unrelated hosted deployment skew are non-blocking follow-ups.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #88 @ d034864 — lens: correctness+isolation+wiring, reviewer codewith-sol-reviewer (1 of 1)

P1 — src/project-links.ts / tests/project-links.test.ts: createLocalKnowledgeProjectLinksAuthority opens a bun:sqlite Database but exposes no close/dispose path. The exact-head Windows check fails during the new test's afterAll cleanup at line 28 with EBUSY: resource busy or locked, consistent with those live handles. This is a current required-platform gate failure, not an optional cleanup concern.

P1 — src/project-links.ts: compensation is not isolated from later accepted adopters. A second registerCollection or bindItem can return an accepted adopted_existing_* receipt, but the original creator's compensation checks only current membership/owner receipt and can delete that shared collection or membership. The later operation is left with an accepted immutable receipt pointing to an absent resource. The added test exercises this sequence but never checks the adopter after creator compensation.

P1 — src/project-links.ts: inverse idempotency does not bind accepted_receipt_id. On a duplicate inverse attempt, assertIdempotent(duplicate, request) compares only idempotency_key, because inverse requests carry no request/precondition digest. Reusing the same operation/step/key with a different accepted receipt therefore returns the first accepted inverse receipt without compensating the requested target, creating a false-success rollback result.

P1 — src/serve.ts: OpenAPI schemas ProjectCollectionRecord and ProjectResource declare required fields plus additionalProperties: false but define no properties. No real response can validate against either schema, so generated/validating clients reject every successful exact-read/resource response.

Could not verify: local execution, clean zero-to-head migration replay, or realistic-volume query plans because no checkout at the supplied head was available. Current exact-head CI was inspected: Windows Bun and macOS Node checks are failing; other completed test checks pass. No repo or external state was modified.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — focused remediation cycle 1 — hasna/knowledge#88 @ 4d48f7e6f8a6bd0e7a3c2b4076fc9c1b3d006bb8

Exact candidate: base 5cba5d57741d843d9b355076871c15ea251cc6ae, head 4d48f7e6f8a6bd0e7a3c2b4076fc9c1b3d006bb8, tree 76843aaf114bef3fff0dcbb19b3e4f66c3faf0fc. This pass is limited to the four P1 blockers in review comment 5242109118 and their direct regressions.

Three blockers are fixed, and the fourth is code-complete but still lacks its required platform gate:

  1. SQLite authority close/dispose and Windows cleanup — NO_GO pending Windows evidence. The candidate now gives every project-links authority a close() contract; SQLite drains its queued work and closes the owned database handle idempotently; KnowledgeService.close() and the CLI finally path dispose it. The direct cleanup test passes locally and in both macOS attempts. However, the Windows matrix was cancelled by fail-fast before tests/project-links.test.ts ran in both attempts. The terminal line is ##[error]The operation was canceled. A targeted rerun was rejected with job 93526922403 cannot be rerun. Because the original defect is a Windows-reachable open-handle cleanup failure, a cancelled lane is not a passing compatibility gate.
  2. Later-adopter-safe owner compensation — fixed. Compensation now detects a later accepted forward receipt for the same collection or membership and returns terminal non-acceptance without deleting the adopted target. The regression exercises both collection and membership adoption and exact readback.
  3. Inverse accepted_receipt_id idempotency binding — fixed. Duplicate inverse requests now compare accepted_receipt_id as part of the idempotency contract and reject a changed receipt with KNOWLEDGE_PROJECT_LINKS_IDEMPOTENCY_MISMATCH; both collection and membership regressions preserve the second target.
  4. Exact OpenAPI response properties and real-response validation — fixed. ProjectCollectionRecord and ProjectResource now define their required properties, enums, nested locator, and additionalProperties: false; the HTTP/SDK regression validates real returned records against those schemas.

Affected direct gates:

  • tests/project-links.test.ts: 7 pass, 0 fail, 167 expect() calls.
  • tests/cli.test.ts: 88 pass, 0 fail, 1098 expect() calls.
  • tests/package-release.test.ts: 6 pass, 0 fail, 100 expect() calls.
  • Typecheck, generated-artifact identity (6 generated bundles rebuild byte-identically), contracts conformance, no-cloud scan, public-package validation, and base-to-head gitleaks: exit 0; gitleaks reports no leaks found.
  • CI rerun: Ubuntu Bun, Ubuntu Node, and Ubuntu matrix succeeded. macOS project-links ran 7/7 successfully. The macOS jobs failed only on unchanged FCAME-1 guarded-writer timeouts; those are out of this focused scope and are not review blockers themselves. Their fail-fast cancellation of Windows is blocking only because it prevents the required Windows cleanup evidence.

Focused remediation needed: obtain one terminal Windows run at this exact candidate that executes the project-links close/cleanup regression successfully. Re-review should remain limited to that evidence and direct regressions; do not reopen the other three fixed blockers or unrelated FCAME-1 timing failures.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #88 @ 4d48f7e — lens: correctness+isolation+wiring, reviewer codewith-sol-reviewer (1 of 1)

P0 — None.

P1 — dist/serve.js / package.json: the candidate adds a static import { Database } from "bun:sqlite" to the exported @hasna/knowledge/serve module while the package still declares Node >=18. This makes the previously Node-importable server entry fail before any API is usable. Controls: CONTROL: current dist/serve.js imports under Node; candidate import shape returns CANDIDATE-IMPORT: ERR_UNSUPPORTED_ESM_URL_SCHEME ... Received protocol 'bun:'.

P1 — src/project-links.ts / src/generated/storage-kit/query.ts: PostgreSQL compensation can race an adopter and leave an immutable accepted receipt pointing to an absent collection or membership. The transaction wrapper uses plain BEGIN (READ COMMITTED); registration/binding reads do not lock the target, and compensation checks for other receipts before deleting. An adopter can read the target, compensation can observe no adopter receipt and commit deletion, then the adopter can insert its accepted receipt. There is no receipt-to-target foreign key or final target readback preventing this schedule.

P1 — src/project-links.ts / src/serve.ts: project-resource pagination is not bounded. buildResources() reads every membership, launches one repo.get(id, tenantId) per member via Promise.all, constructs every item/taxonomy resource, and only then applies slice(0, limit). A limit=1 read on a large collection still performs O(all members) storage work and concurrent queries, violating the advertised keyset/bounded path and exposing an authenticated read-driven resource-exhaustion path.

P2/P3 — None material to merge acceptance.

Could not verify exact-head execution, zero-to-head migration replay, 10k–100k EXPLAIN plans, or Windows cleanup behavior: no checkout at the supplied SHA was available, and the supplied exact-head Windows check was cancelled. No repository or external state was modified.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Final remediation evidence at exact candidate b0de40d (tree 24b41234879786164be88c6118f679e513da1ef9): Actions run 31414838047 completed success across all seven jobs. Windows job 93541278596 reports (pass) Knowledge Projects resource-link producer > local authority close releases the owned SQLite handle before directory cleanup [92.62ms], all seven project-links cases passed, (pass) generated artifact verification > the cross-platform matrix does not cancel Windows when another OS fails [0.19ms], 479 pass, 0 fail, Ran 484 tests across 51 files. Staged and origin/main..HEAD gitleaks scans report no leaks. The same fixed reviewer is performing final focused remediation cycle 2 against only the four original P1 defects, their fixes, and direct workflow regressions.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] GO — final focused remediation cycle 2 — hasna/knowledge#88 @ b0de40d63f8d4e28fd3c863d2361270db094f982

Exact candidate: base 5cba5d57741d843d9b355076871c15ea251cc6ae, head b0de40d63f8d4e28fd3c863d2361270db094f982, tree 24b41234879786164be88c6118f679e513da1ef9. Worktree readback is clean and the remote PR head matches.

This final pass remained limited to the four original P1 blockers, their fixes, and direct regressions from the workflow fail-fast/CRLF-normalization remediation. All four blockers are closed:

  1. Owned SQLite close/dispose: the authority/service/CLI disposal path remains intact, and the previously missing platform gate now passes on Windows: (pass) Knowledge Projects resource-link producer > local authority close releases the owned SQLite handle before directory cleanup [92.62ms].
  2. Later-adopter compensation safety: Windows and the focused local run pass (pass) Knowledge Projects resource-link producer > later accepted adopters preserve creator-owned collections and memberships.
  3. Inverse accepted-receipt idempotency: Windows and the focused local run pass (pass) Knowledge Projects resource-link producer > inverse removes solely owned effects and binds idempotency to the accepted receipt.
  4. OpenAPI property schemas and real responses: Windows and the focused local run pass (pass) Knowledge Projects resource-link producer > HTTP routes, SDK group, and OpenAPI expose the same producer contract.

Direct regression evidence:

  • The matrix now has fail-fast: false; its CRLF-normalizing regression passes locally and on Windows. Raw Windows line: (pass) generated artifact verification > the cross-platform matrix does not cancel Windows when another OS fails [0.19ms].
  • Fresh exact-head Actions run 31414838047 completed success across all seven jobs. Windows job 93541278596 ran all seven project-link cases successfully and ended with the raw lines 479 pass, 0 fail, and Ran 484 tests across 51 files.
  • Focused local command over tests/project-links.test.ts plus tests/generated-artifacts.test.ts: 16 pass, 0 fail, 210 expect() calls.
  • Base-to-head gitleaks: 4 commits scanned., scanned ~618183 bytes (618.18 KB), no leaks found, exit 0.

No concrete, evidence-backed, reachable in-scope P0/P1 remains. Pre-existing, unrelated, and P2/P3 matters were not reopened. Verdict: GO.

@andrei-hasna
andrei-hasna marked this pull request as ready for review August 10, 2026 17:48
@andrei-hasna
andrei-hasna merged commit 46dce61 into main Aug 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant