Skip to content

test(metadata-protocol,metadata-core): run SysMetadataRepository through the shared repository contract suite - #10851

Merged
os-elon merged 4 commits into
mainfrom
claude/issue-10420-sysmetadata-contract-suite
Aug 21, 2026
Merged

test(metadata-protocol,metadata-core): run SysMetadataRepository through the shared repository contract suite#10851
os-elon merged 4 commits into
mainfrom
claude/issue-10420-sysmetadata-contract-suite

Conversation

@os-elon

@os-elon os-elon commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Fixes #10420

runRepositoryContractTests exists so that one table of invariants holds for every MetadataRepository implementation. It had two call sites: InMemoryRepository and FileSystemRepository — the two implementations that carry the test traffic. SysMetadataRepository, which carries the production traffic (561 of 1,729 put() invocations in the #8006 census, including all four production put() call sites), was never handed to it. This PR hands it over.

Gates and clause counts below were produced at 5eaaf14fdd, the final commit (origin/main did not move during the card, so the merged head equals it).

The shape, and why this one

Adapted into the existing suite via an engine-double-backed factory — not a narrower "serialized-form identity" slice.

The narrower slice was the option to beat, and it loses on the same argument the suite's own header makes: a second table turns "which invariants apply to which implementation" from something the code enforces into something a reader has to remember. The #7856 / #7992 pins were deliberately placed in the shared suite rather than beside either bug for exactly that reason, and a third table would have been the first step in undoing it.

The cost the slice was supposed to buy off — the dimensions the suite's factory() shape does not model — turned out not to need buying off:

dimension what the suite actually drives how it is held
state: 'draft' | 'active' always the active default; no draft row is ever created pinned by an implementation-specific clause, not asserted in prose
packageId always the unbound row (package_id IS NULL) pinned the same way
org scoping SysMetadataRepository ignores ref.org and stamps its own pinned, and it is why the suite's "different orgs have independent sequences" clause self-skips — the same escape FileSystemRepository takes

One thing genuinely had to move, and it is a fixture knob rather than an invariant knob. The suite hard-coded its two metadata types ('view' everywhere, 'object' for the single clause that must prove a type filter discriminates). SysMetadataRepository — unlike the other two — sits behind a write-authorization door keyed on the type: assertAllowed() refuses 'object' under the default override-artifact intent because its registry entry is allowOrgOverride: false. So ContractSuiteOptions gained primaryType / secondaryType, defaulting to today's values; this call site names 'view' / 'dashboard', both of which the door admits.

⛔ Deliberately not done: wrapping the repository in an adapter that injects intent: 'runtime-only'. That would have put a test-only shim between the suite and the subject — the exact "reads as coverage" failure the card is about. factory() returns the real repository and every call goes through its own door.

Green was the expected outcome, so here is the control for the control

Two ablations, each aimed at one of the two faces #7992 fixed, with the non-effects predicted before each run. Every prediction hit, clause name for clause name.

Ablation A — the FileSystemRepository face. put() hands back hashSpec({ __ABLATION_A__: true }) — a well-formed sha256 that simply does not identify the stored bytes.

24 failed | 10 passed (34), exactly as predicted. The named #7992 pin reds by name:

× serialized-form identity (#7856) > version identifies the stored bytes — control — plain JSON, no toJSON anywhere in the graph
  → expected 'sha256:f26287fbd25e42e8b586e5da242ebf…' to be 'sha256:820dade9817fa4dae415b760f4e0b8…'
× serialized-form identity (#7856) > re-putting the same spec is a no-op — control — plain JSON…
  → Conflict on view/sample_view: expected parent sha256:820dade9…, current head is sha256:f26287fb…

Predicted non-effects, all observed green: canonical hash invariant: item.hash === hashSpec(item.body) (it asserts only get()'s self-consistency — its survival is what makes this ablation aimed rather than blunt), returns null for missing item, getByHash() resolves the current HEAD hash to the HEAD body, getByHash() returns null for an unknown hash, throws ConflictError when creating over an existing item with null parent, different orgs have independent sequences (self-skips), the live-stream watch clause, both list clauses, and the package_id pin.

Ablation B — the InMemoryRepository face. The stored sys_metadata body becomes { ...body, __ABLATION_B__: 1 } while the checksum stays hashSpec(body). The history row is left alone on purpose, to bound the blast radius.

9 failed | 25 passed (34), exactly as predicted:

× canonical hash invariant: item.hash === hashSpec(item.body)
  Expected: "sha256:3a3e680a1b50de4571b6dfebab64f69e6572153d72e64d0910cd7ec110e6e671"
  Received: "sha256:f6b8f51cc91d7b4339a36c784179404f96d25c2cf816f4bea72f7eaf0e7f529d"

Predicted non-effects, all observed green: all six re-putting the same spec is a no-op rows (the no-op short-circuit compares checksums, which this ablation does not touch), and getByHash() resolves the current HEAD hash to the HEAD body — its staying green is what proves the mutation is confined to the row get() reads.

Restore, proved on disk in both directions — never an editor's exit code. For each leg: marker present during the ablation, 0 occurrences after; git hash-object equal to git rev-parse HEAD:PATH (58aeef4cd7cd74bd07d32e80496a2723de93402f); anchored grep for the real line back at its pre-ablation count; git diff --exit-code exit 0; git status --porcelain empty. Re-run at the restored tree: 34 passed (34).

src/dist — this file is a MIXED case, and it was worked out, not assumed

  • Subject side — source. The test imports the repository by the relative in-package specifier ./sys-metadata-repository.js, which vitest transforms from source. Proof rather than inference: packages/metadata-protocol/dist does not exist in this worktree at all, and both ablations reddened with no rebuild between mutation and run. No ablation-dist-preflight is owed for either leg.
  • Suite side — artifact. @objectstack/metadata-core/testing resolves to packages/metadata-core/dist/testing.cjs (measured with createRequire().resolve; packages/metadata-protocol has no vitest.config.*, consistent with its entry in KNOWN_UNALIASED_TEST_IMPORTS). So metadata-core was rebuilt after every suite edit, and a stale dist here would have been a false green.

That second half was reverse-verified rather than trusted: primaryType: 'not_a_metadata_type' was pasted into metadata-fs's call site (whose tsconfig.test.json type-checks its test/ dir through the same built .d.ts) and tsc refused it — test/contract.test.ts(36,5): error TS2322: Type '"not_a_metadata_type"' is not assignable to type '"object" | "field" | … | undefined' — then restored byte-identically, typecheck back to exit 0.

Clause census — which clauses RAN, not "it passes"

34 clauses total: 31 from the shared suite plus 3 implementation-specific pins.

Ran and passed (31 of the 34), including every clause the card asked about by name:

  • fix(metadata-core,metadata-fs): hash the serialized form, so put().version identifies the bytes actually stored (#7856) #7992 pins — both faces, all 12 rows. put().version === get().hash and get().hash === hashSpec(get().body) are asserted together on each of the six SERIALISATION_SHAPES rows (control, Date at a key, Date under an array index, a toJSON collapsing to a string, an object literal carrying its own toJSON, a toJSON nested in an array element), plus the six matching re-putting the same spec is a no-op rows. All 12 ran; all 12 pass.
  • ConflictError — all 3 ran, all pass: parent mismatch (plus the "correct parent succeeds" sanity leg), create-over-existing with a null parent, and delete with a wrong parentVersion.
  • Monotonic seq — 1 of 2 ran. seq strictly increases within an org runs and passes. different orgs have independent sequences is SKIPPED, not passed: the clause's own try/catch … return escape for single-org backends fires, because SysMetadataRepository ignores ref.org. This is the one clause the suite cannot ask of this implementation, and the skip is now a pinned, asserted fact rather than a silent early return.
  • Tombstones — both ran, both pass: get returns null after delete while history still shows ['create','delete'] with parentHash chained and hash: null; and recreate-after-delete with a null parent.
  • Plus: atomic put from a null parent, parent chaining, the standalone canonical-hash invariant, missing-item null, no-op write, both getByHash clauses, history monotonic ordering, and both list clauses.

Ran in an adapted form (2), because of the divergence below — adapted, not skipped, and named as such in the test output.

The divergence the suite found — filed, not fixed here

Contract invariant 6 (resumability) — watch(_, since) MUST replay all events with seq > since before delivering live events — is not implemented by SysMetadataRepository. Its watch() registers an in-memory listener and reads since only as a drop filter on live events; it never reads sys_metadata_history on subscribe. Two clauses caught it, in two different faces, which is #7992's own shape one more time:

× watch(sinceSeq) replays subsequent events then goes live 5008ms → Test timed out in 5000ms.
× watch filters by type and name 207ms → expected +0 to be 1

Filed as #10842, with the fork and a recommendation. It is not addressed here, and that is a judgement this PR should be read as making explicitly: both production watch() consumers (MetadataManager.startRepositoryWatch() and MetadataCache.start()) subscribe with no since, so implementing the no-since face the way InMemoryRepository does would replay the entire org history through Studio HMR and cache invalidation at every setRepository(). And what watch() with no since owes is not written in repository.ts at all — it exists only as InMemoryRepository's implementation, which the shared suite silently leans on. That is a contract decision, not a small local fix.

⛔ It is not hidden behind a skip either. ContractSuiteOptions.declaredDivergences requires the call site to name the tracking issue, and in exchange the suite swaps in a clause that pins the divergent behaviour — one next() proved not to settle from replay, then settled by a live event — so it reds the day replay lands, and the author of that repair is told by name to delete the declaration (#10842). The second replacement re-asks the filter question the original clause is named for, sourced from the live stream, so filter coverage is not traded away for the exception. Shrink-only, audited in the fixing direction, like the repo's other ledgers. Both replacements carry DECLARED DIVERGENCE #10842 in their test names, so a reader of the output sees the exception rather than an absence.

Changeset — yes, and why

.changeset/sysmetadata-repository-contract-suite.md, @objectstack/metadata-core: minor.

This is not test-only. contract-suite.ts is re-exported by src/testing.ts, which is a tsup entry and a published exports subpath (@objectstack/metadata-core/testing), so the two new optional options and the new exported DeclaredDivergences interface are additive public API of a published package. packages/metadata-protocol is not listed: it gains a test file only, and its files/tsup entry ship nothing from it.

Gates — each gate's own verdict line, at 5eaaf14fdd

Union re-derived with node scripts/pm/dispatch-gates.mjs (no path arguments) after the final commit; it was re-derived a second time after the ledger commit added a scripts/ path, which widened cross-package-test-inputs' matched set and named no new gate.

check-empty-changeset --self-test: 118 assertions over real temp git repos (real scan() path)
check-adr-0087-registration --self-test: 212 assertions over real temp git repos
check-changeset-no-major --self-test: 116 assertions
OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
durability-degradation log levels: 30 durability-critical catch seam(s), all loud, rethrowing or propagating
read-seam invention: 67 read seam(s), none invents an unreported answer
objectui-ratchet: every listed entry carries the level objectui declared
slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new … baseline key set verified against 5f2e54c: no files added.
check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through `dist/`; 44 published subpath(s) resolved through every alias table.
check-type-source-resolution OK — 76 packages with a tsconfig.json scanned; 51 registered as still resolving a workspace dep's types through `dist/`.
check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
This diff introduces no `major` bump.
No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
check:plugin-teardown-shape: 54 Plugin implementation(s) across 4384 source(s) under packages/**; every teardown-shaped method sits beside a real destroy()
affected-docs self-test: 308 cases pass.
query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new … baseline key set verified against 5f2e54c: no files added.
check-type-check-coverage: OK — 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledger (436 frozen raw errors), 1 exempt.
check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 227.2s, 1912 raw tsc error(s) total, none above its recorded number.
check-engine-double-contract: OK — 373 pinned, 133 in the DEBT ledger, 2 exempt.
where-matcher conformance holds: 273 matcher(s) discovered, 273 answer the combinator battery correctly or refuse it loudly (164 refuse).
check-nul-bytes: OK (scanned 6259 text file(s) -- 6259 tracked, 0 untracked-not-ignored; skipped 5 binary; no raw ASCII control bytes).

check:type-check-debt --re-measure required the built workspace closure (it refuses on an unbuilt tree rather than measuring a different world), so the farm was built first as lint.yml does: 70 successful, 70 total, 4m55s.

One gate reddened on this change and was repaired at the author's end, not by raising a ledger. check:engine-double-contract reported RETAINED [delete] / RETAINED [update] for the new file: the double is pinned to assertEngineDeleteDispatch / assertEngineUpdateDispatch, but the pinned ledger had to learn about it. node scripts/check-engine-double-contract.mjs --write added two rows to scripts/engine-double-contract.pinned.json and left engine-double-contract.baseline.json — the shrink-only DEBT ledger — untouched. Its DEBT count is unchanged at 133; pinned coverage grew from 371 to 373.

Suites

packages/metadata-protocol  1772 passed | 10 skipped (1782), 128 files passed | 2 skipped
packages/metadata-core       165 passed (165), 11 files
packages/metadata-fs          56 passed (56), 7 files
packages/metadata-core typecheck (tsc --noEmit && tsc --noEmit -p tsconfig.test.json)  exit 0

metadata-core and metadata-fs are the two pre-existing call sites; both are unchanged by the new options (defaults preserve today's fixture types, and no declaration means the original clauses run verbatim). The 66 downstream consumers of metadata-core were not swept: the change is purely additive — two optional fields and a new exported interface — so no consumer's existing code can stop compiling.

Deliberately not done


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

9 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 3 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 23502649659115a98a80544f4b80f7bf21568601packageMentionDocs.

Which tree this was computed on

This run read content/docs from a17a2f004db85dd333a428df94c99371dc609357 — the merge of head 5eaaf14fdd4f07d4035da3ae6089813f8c591853 into base 23502649659115a98a80544f4b80f7bf21568601, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a17a2f004db85dd333a428df94c99371dc609357 && git checkout a17a2f004db85dd333a428df94c99371dc609357
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 23502649659115a98a80544f4b80f7bf21568601 5eaaf14fdd4f07d4035da3ae6089813f8c591853 && git checkout -B drift-repro 23502649659115a98a80544f4b80f7bf21568601 && git merge --no-ff 5eaaf14fdd4f07d4035da3ae6089813f8c591853

node scripts/docs-audit/affected-docs.mjs --json 23502649659115a98a80544f4b80f7bf21568601

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

os-elon commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

ℹ️ Console Pin Gate is red on main, not on this PR — no change is owed here, and this note exists so nobody re-investigates it.

Filed as #10857 with the full evidence. Summary: 35ad101bc3 (#10695, card #10485, ADR-0049) retired ColorPaletteSchema / TypographySchema / BorderRadiusSchema / ShadowSchema / ThemeModeSchema from @objectstack/spec/ui, while objectui still imports all five at packages/types/src/zod/theme.zod.ts:19-21both at the pinned 9a3daf8d37ad and on objectui's own tip ac73c24b01, so it is a one-sided retirement rather than a stale pin, and bumping .objectui-sha would reproduce it exactly.

Ruled out as this PR's by content, not by re-running: the failure is a bundle-time MISSING_EXPORT in packages/spec/src/ui/, and this PR's diff is packages/metadata-core + packages/metadata-protocol — it touches packages/spec not at all. It reproduces on the base branch, which is the one case where a red is legitimately not the PR's.

⛔ Deliberately not done: no re-run (a build-time missing export cannot flake), no change to this diff to route around it, and no .objectui-sha bump riding along on this card — the fix has a forced ordering that starts in objectui and belongs to #10857.

This PR stays in the queue. If the gate turns out to be required after all and blocks the merge, #10857 is the blocker to watch.


Generated by Claude Code

Merged via the queue into main with commit 05bc692 Aug 21, 2026
32 checks passed
@os-elon
os-elon deleted the claude/issue-10420-sysmetadata-contract-suite branch August 21, 2026 14:26
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32490891332 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 4.60s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️ 本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 12 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SysMetadataRepository — the implementation backing every production metadata write — is not run through the shared runRepositoryContractTests suite

2 participants