test(metadata-protocol,metadata-core): run SysMetadataRepository through the shared repository contract suite - #10851
Conversation
📓 Docs Drift Check9 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅ What this run could not see
Coarse fallback — 3 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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 |
|
ℹ️ Filed as #10857 with the full evidence. Summary: Ruled out as this PR's by content, not by re-running: the failure is a bundle-time ⛔ Deliberately not done: no re-run (a build-time missing export cannot flake), no change to this diff to route around it, and no 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 |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32490891332 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes #10420
runRepositoryContractTestsexists so that one table of invariants holds for everyMetadataRepositoryimplementation. It had two call sites:InMemoryRepositoryandFileSystemRepository— the two implementations that carry the test traffic.SysMetadataRepository, which carries the production traffic (561 of 1,729put()invocations in the #8006 census, including all four productionput()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/maindid 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:state: 'draft' | 'active'activedefault; no draft row is ever createdpackageIdpackage_id IS NULL)SysMetadataRepositoryignoresref.organd stamps its ownFileSystemRepositorytakesOne 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 defaultoverride-artifactintent because its registry entry isallowOrgOverride: false. SoContractSuiteOptionsgainedprimaryType/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
FileSystemRepositoryface.put()hands backhashSpec({ __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:Predicted non-effects, all observed green:
canonical hash invariant: item.hash === hashSpec(item.body)(it asserts onlyget()'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, bothlistclauses, and thepackage_idpin.Ablation B — the
InMemoryRepositoryface. The storedsys_metadatabody becomes{ ...body, __ABLATION_B__: 1 }while the checksum stayshashSpec(body). The history row is left alone on purpose, to bound the blast radius.9 failed | 25 passed (34), exactly as predicted:Predicted non-effects, all observed green: all six
re-putting the same spec is a no-oprows (the no-op short-circuit compares checksums, which this ablation does not touch), andgetByHash() resolves the current HEAD hash to the HEAD body— its staying green is what proves the mutation is confined to the rowget()reads.Restore, proved on disk in both directions — never an editor's exit code. For each leg: marker present during the ablation,
0occurrences after;git hash-objectequal togit rev-parse HEAD:PATH(58aeef4cd7cd74bd07d32e80496a2723de93402f); anchored grep for the real line back at its pre-ablation count;git diff --exit-codeexit0;git status --porcelainempty. Re-run at the restored tree:34 passed (34).src/dist — this file is a MIXED case, and it was worked out, not assumed
./sys-metadata-repository.js, which vitest transforms from source. Proof rather than inference:packages/metadata-protocol/distdoes not exist in this worktree at all, and both ablations reddened with no rebuild between mutation and run. Noablation-dist-preflightis owed for either leg.@objectstack/metadata-core/testingresolves topackages/metadata-core/dist/testing.cjs(measured withcreateRequire().resolve;packages/metadata-protocolhas novitest.config.*, consistent with its entry inKNOWN_UNALIASED_TEST_IMPORTS). Sometadata-corewas rebuilt after every suite edit, and a staledisthere would have been a false green.That second half was reverse-verified rather than trusted:
primaryType: 'not_a_metadata_type'was pasted intometadata-fs's call site (whosetsconfig.test.jsontype-checks itstest/dir through the same built.d.ts) andtscrefused 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 exit0.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:
put().versionidentifies the bytes actually stored (#7856) #7992 pins — both faces, all 12 rows.put().version === get().hashandget().hash === hashSpec(get().body)are asserted together on each of the sixSERIALISATION_SHAPESrows (control,Dateat a key,Dateunder an array index, atoJSONcollapsing to a string, an object literal carrying its owntoJSON, atoJSONnested in an array element), plus the six matchingre-putting the same spec is a no-oprows. 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 wrongparentVersion.seq— 1 of 2 ran.seq strictly increases within an orgruns and passes.different orgs have independent sequencesis SKIPPED, not passed: the clause's owntry/catch … returnescape for single-org backends fires, becauseSysMetadataRepositoryignoresref.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.getreturns null after delete whilehistorystill shows['create','delete']withparentHashchained andhash: null; and recreate-after-delete with a null parent.getByHashclauses,historymonotonic ordering, and bothlistclauses.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 withseq > sincebefore delivering live events — is not implemented bySysMetadataRepository. Itswatch()registers an in-memory listener and readssinceonly as a drop filter on live events; it never readssys_metadata_historyon subscribe. Two clauses caught it, in two different faces, which is #7992's own shape one more time: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()andMetadataCache.start()) subscribe with nosince, so implementing the no-sinceface the wayInMemoryRepositorydoes would replay the entire org history through Studio HMR and cache invalidation at everysetRepository(). And whatwatch()with nosinceowes is not written inrepository.tsat all — it exists only asInMemoryRepository'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.declaredDivergencesrequires the call site to name the tracking issue, and in exchange the suite swaps in a clause that pins the divergent behaviour — onenext()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 carryDECLARED DIVERGENCE #10842in 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.tsis re-exported bysrc/testing.ts, which is a tsup entry and a publishedexportssubpath (@objectstack/metadata-core/testing), so the two new optional options and the new exportedDeclaredDivergencesinterface are additive public API of a published package.packages/metadata-protocolis not listed: it gains a test file only, and itsfiles/tsup entry ship nothing from it.Gates — each gate's own verdict line, at
5eaaf14fddUnion 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 ascripts/path, which widenedcross-package-test-inputs' matched set and named no new gate.check:type-check-debt --re-measurerequired the built workspace closure (it refuses on an unbuilt tree rather than measuring a different world), so the farm was built first aslint.ymldoes: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-contractreportedRETAINED [delete]/RETAINED [update]for the new file: the double is pinned toassertEngineDeleteDispatch/assertEngineUpdateDispatch, but the pinned ledger had to learn about it.node scripts/check-engine-double-contract.mjs --writeadded two rows toscripts/engine-double-contract.pinned.jsonand leftengine-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
metadata-coreandmetadata-fsare 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 ofmetadata-corewere 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
SysMetadataRepository.watch()never replays fromsys_metadata_history— contract invariant 6 (resumability) is unimplemented in the repository backing every production metadata write #10842 is where that decision lives — reasoning above. That issue is deliberately NOT addressed by this PR and must stay open after it merges.watch filters by type and nameclause. It leans on unspecified no-sincereplay behaviour, which is arguably a defect in the clause. Rewriting it would change what the two currently-green implementations are held to — collateral that should not ride along on this card.SysMetadataRepository.watch()never replays fromsys_metadata_history— contract invariant 6 (resumability) is unimplemented in the repository backing every production metadata write #10842 carries it.@objectstack/plugin-auth's TEST_DEBT surplus (records 109, tsc now reports 97). Pre-existing, unrelated to this diff, and [RESCUED — evidence complete] feat(platform-objects): declare sys_session ttl sparing revoked tombstones (#7826) #10633 is in the merge queue touching that package.Generated by Claude Code