fix(rest): /meta/:type/:name/history and /diff state the org partition they read (#13406) - #13756
Conversation
…register its pins (#13406)
📓 Docs Drift Check
What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
… the adjacency drift and the diff door's absent guard (#13406)
…ta-read-doors-forward-org
Merge ordering — this PR goes SECOND, behind #13763Both PRs touch ⭐ The part that matters for whoever merges this one: that file is NOT text-merged
⇒ After #13763 lands, do not hand-resolve this file and do not reason about line ranges. The sequence is:
That third step is why "regenerate with The two PRs are independent in both dimensions anyway
Neither PR's source edit shifts the file the other cites, so neither invalidates the other's anchor values. The collision is nominal — same artifact, disjoint content. (Note the cited files: not State at
|
…nd, and a pin proves ?limit= travels the door (#13406)
Fixes #13406
GET /api/v1/meta/:type/:name/historyanswered{ events: [] }, andGET /api/v1/meta/:type/:name/diffanswered an all-empty diff, for metadata whose overlay was authored org-scoped — whilesys_metadata_historyheld the full log. Both doors named no organization.sys_metadata_historyis a per-org table.SysMetadataRepository.history()anddiffMetaItemboth filterorganization_idby strict equality (no$or), andevent_seqis declared on the object as a "Per-organization monotonic event log cursor". So a door that states no organization does not read "everything" — it reads the env partition (request.organizationId ?? null). The write door has stated the org since #8805; only these two read doors had not.Direction is fail-closed: the caller's OWN org data was under-served. There is no cross-org read, and the controls below keep it that way.
Call-side only.
packages/specandprotocol.tsare untouched —organizationIdwas already declared on the request contract, andrequest.organizationId ?? nullis the legitimate expression of env scope that every correct caller depends on.Generated by Claude Code · session
session_01UngCYXF98BVpYA9hfz6NYk⭐ The escalation gate, answered FIRST — p2 stands, no regrade
Triage made this the executor's first mandatory item: if any of these doors is a compliance / audit evidence surface, "an org cannot see its own history" stops being a functional gap. Measured answer: no. Three independent in-repo classifications agree, and all name the audit table, never the history table:
sys_metadata_overlayandsys_metadata_history(ADR-0008) — the latter store state, the former stores intent and provenance. Compliance reports read from the audit table." §8 repeats it: the compliance-grade trail is the audit one.audit/ compliance-ledger lifecycle class assys_audit_logandsys_metadata_audit.sys_metadata_historyappears nowhere in that ADR.sys-metadata-audit.object.tscarrieslifecycle: { class: 'audit' };sys-metadata-history.object.tscarries nolifecycleblock at all and describes itself as a "Durable event log of metadata overlay changes (per-org, append-only)".And the compliance-evidence door in this family —
/audit— already forwards the org; it was scoped in #8747. So the surface the gate protects is not one of the affected doors.⇒ Grade proposal: p2 unchanged,
bug, nosecuritylabel. Stated rather than chosen silently.⭐ Premise re-measurement — symptom 3 was already delivered (a good run, not a scope cut)
The card names three doors. On
origin/maintoday, two were open. The third — "single-itemGET /meta/dashboard/:nameignores an org-scoped overlay" — is already correct, threaded by #9454 / #9727 before this branch. The uncached arm thatdashboardtakes (isDashboardTypebypasses the cache) carriesreadOrganizationId, hoisted above the fork precisely so the two arms cannot disagree about scope, anddashboardisallowOrgOverride: trueso the registry predicate returns the tenant.Rather than assert that in a report, this branch pins the disputed fact next to the two doors that were genuinely open, so the falsification is auditable:
the card's third symptom, RE-MEASURED on today's main→single-item dashboard read ALREADY serves the org overlay. It authors a real org-scoped overlay, asserts the persisted row carriesorganization_id, then asserts the door serves it.The card's line numbers were all dead too (
#13521de-cast the history door after triage wrote its note). Everything here was located by symbol.⭐ The design decision:
organizationIdForMetaRead, not/audit's expressionThe card calls
/audit"zero-tradeoff prior art". It is prior art for the intent; copying its expression is wrong twice, and both halves are measured rather than argued.1. The predicate.
auditMetaItemreads with$or: [{organization_id: org}, {organization_id: null}]— a union, so a rawctx?.tenantIdthere can only add rows. These two doors read with strict equality. Under strict equality a raw tenant id asks the org partition for the history of everyallowOrgOverride: falsetype that is still runtime-writable —object,hook,page,app,dataset— becauseorganizationIdForMetaWritedeliberately lands those env-wide under the #6190 ruling. That answers{ events: [] }for them: this card's own defect, newly minted one type family over. Gating the read on the same registry predicate the write uses is what keeps the two sides incapable of drifting.2. The spelling.
HistoryMetaItemRequestSchemadeclaresorganizationId: z.string().optional()— optional plainstring, not nullable, mirroring the implementation'sorganizationId?: string. The key is therefore spread, neverorganizationId: x ?? null.Both were confirmed by ablation, with the direction predicted before running:
ctx?.tenantIdon the history doorserves a NON-overridable type's env-wide history to an org session; every org-overridable case stays green?? nullon the history door instead of the omit-spreadtsc --noEmitreds namingorganizationIdandnull; TS2322 (null not assignable tostring | undefined), not TS2353 — the latter is the code for an undeclared member, and this member is declaredsrc/rest-server.ts(6238,25): error TS2322: Type 'string | null' is not assignable to type 'string | undefined'.— 1 error, exactly as predicted, code includedBoth legs were run from a committed implementation and restored with
git checkout HEAD --against the target's ABSOLUTE path under anEXIT INT TERMtrap; each mutation was proven on disk by injected/deleted occurrence counts plus agit hash-objectcomparison against the HEAD blob, and each restore proven the same way plus an emptygit diff HEAD. Neither leg needed a rebuild: the pins import./rest-server.jsinside their own package, so the mutation reaches source directly — and ablation A changing exactly one verdict is itself the proof that it did.diffMetaItemthrough(p as any), so the compiler checks nothing about that literal. There,?? nulltype-checks and is a silent runtime no-op (null ?? nullisnull) — a fix-shaped non-fix. The two doors are not identical and were measured separately.⭐ The mandatory answer, BY ENUMERATION
Every meta read door in
rest-server.ts, against/audit's parameter passing. Located by symbol; line numbers omitted deliberately because this file changed three times in a day./meta/types/meta/diagnosticsgetMetaDiagnosticsdeclaresorganizationIdand passes it intogetMetaItems; the door supplies none. Out of scope → #13753/meta/_draftsorganizationId: ctx?.tenantId ?? undefined/meta/:type(list)organizationIdForMetaRead(canonicalMetaUrlType(type), ctx?.tenantId)(#9454)/meta/:type/:name/referencesfindReferencesToMetadeclaresorganizationIdand spreads it intogetMetaItems; the door supplies none. Out of scope → #13753/meta/:type/:name/layers/meta/book/:name/treebookanddocareallowOrgOverride: false; naming an org would resurrect #6190's phantom rows/meta/:type/:name(single item)/meta/:type/:name/historyorganizationIdForMetaRead, omit-spread/meta/:type/:name/auditorganizationId: ctx?.tenantId ?? null(#8747)/meta/:type/:name/difforganizationIdForMetaRead, omit-spread/meta/object/:name/state/:fieldobjectisallowOrgOverride: false/meta/:type/:name/published...(ctx?.tenantId ? { organizationId: ctx.tenantId } : {})So the enumeration found two doors beyond this card's scope with the identical omission. They are filed unassigned as #13753, not fixed here. The
referenceshalf is the sharper one: it backs the admin "Used by" panel whose empty case reads "Nothing in the metadata graph points at this item. Safe to delete.", shown to an operator about to delete — the exact false-negative class that door's own 501 refusal was added to prevent.Tests — positive controls, not just green
packages/rest/src/rest-server-meta-history-diff-org-scope.test.tsdrives real REST routes against a realObjectStackProtocolImplementationover a stub engine, one boot, write-then-read round trips.The load-bearing difference from the sibling
rest-server-meta-read-org-scope.test.ts: its stub returns everysys_metadata_historyrow unfiltered, so over that engine both doors pass with or without this change — there is no partition to miss. This harness honours thewhere,organization_idincluded.Every read assertion is gated on a fixture proof first (
historyRowsFor), because "the read is org-scoped" is worthless if the fixture never created an org-scoped row — the card's own repro bar was "confirm the pg rows exist before hitting the read door":allowOrgOverride: truetypes, aPUTunder an active org appends exactly 1 history row in the org partition and 0 env-wide. Both halves asserted: the second is why an org-blind door missed them./history— two authored revisions come back as two events,version[1, 2]./diff?from=1&to=2— resolves both org revisions and reports the real change ({ path: 'label', from, to }), not the card's echoed-bounds/empty-buckets shape.objectwrite under an active org lands env-wide, and the org session still reads it.Commands, all at
0df058f2(the implementation commit; see the patch round below ford73a967d)pnpm --filter @objectstack/rest testTest Files 164 passed (164)·Tests 2760 passed (2760)pnpm --filter @objectstack/rest typecheckcheck:test-typecheck: OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.jsonpnpm --filter @objectstack/dogfood exec vitest run --shard=1/3Test Files 43 passed (43)·Tests 313 passed (313)pnpm --filter @objectstack/dogfood exec vitest run --shard=2/3Test Files 43 passed (43)·Tests 288 passed | 1 skipped (289)pnpm --filter @objectstack/dogfood exec vitest run --shard=3/3Test Files 42 passed | 1 skipped (43)·Tests 379 passed | 2 skipped (381)pnpm lint(repo-wide,eslint . --no-inline-config)node scripts/check-system-context-census.mjsOK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-readpnpm check:engine-double-contractOK — 727 pinned, 134 in the DEBT ledger, 3 exemptpnpm check:where-matcher320 matcher(s) discovered, 320 answer the combinator battery correctly or refuse it loudly (201 refuse)pnpm check:type-check-debtOK — 29 ledger entr(ies) re-measured in 255.0s, 1542 raw tsc error(s) total, none above its recorded numberpnpm check:authz-resolver·check:route-envelope·check:nul-bytes·check:cross-package-test-inputs·check:query-options-erasure·check:type-check-coverage·check:dispatcher-error-vocabulary·check:test-source-aliasThe typecheck really does cover the new test file, rather than excluding it:
tsc -p tsconfig.test.json --listFilescounts it 1, and the package's main program counts it 0.Not measured, reported separately from the passes —
pnpm --filter '@objectstack/rest^...' buildfails on@objectstack/verify, and it is a filter artefact, not a defect in this diff:@objectstack/verifydepends on@objectstack/rest, and the^excludesrestitself from its own dependency closure, soverify's dts build can never resolve it under that spelling. Building as CI does —turbo run build --filter='./packages/*' --filter='./packages/*/*'— is 70/70 tasks successful. The first dogfood attempt on the unbuilt tree wasPREREQUISITE NOT MET(Failed to resolve entry for package "@objectstack/runtime"), not a red; the shards above are the runs on the built tree.Ratchets moved, and why
execctx-consumer-census.test.ts— two newresolveExecCtxsites, both locally caught on the continuation line:73 → 75sites,92 → 95mentions (+2calls and+1prose mention — the two numbers move by different amounts on purpose),20 → 22caught,16same-line and53bare unchanged. Counts re-derived from the tree, not hand-edited.content/docs/permissions/system-context.mdx— re-anchored by regeneration (check-system-context-census.mjs --fix), never by hand, twice: once for the implementation (:6382→:6432,:6575→:6625) and again for the comment-only patch round below (:6432→:6450,:6625→:6643). Each run rewrote exactly 2 anchors, matching the 2 it flagged — so unlike the sibling that found 13 silently stale extras, there was no hidden staleness in either pass.scripts/engine-double-contract.pinned.json— 3 new pinned rows for the new file's double (delete,findOne,update), all routed through theassertEngine*Dispatchhelpers; registered with--writeas the gate's own message prescribes, and the DEBT baseline did not grow.$orrather than reading it as a field name —check:where-matcherexecutes an$andbattery against every discovered matcher, and refusing loudly is the convention 201 of the 320 already follow.Patch round —
d73a967d, comment-onlyPM review caught a real defect in the written record: two comments I landed named TS2353 where the measured code is TS2322.
organizationIdis declared onHistoryMetaItemRequestSchema, so?? nullthere is an assignability failure (Type 'string | null' is not assignable to type 'string | undefined'), not an unknown-property one. TS2353 is the undeclared-member code. My own ablation leg B predicted TS2322, reasoned about why TS2353 would be wrong, and measured TS2322 — and then the comments said TS2353 anyway.Mechanism, named in the fix so the next reader does not repeat it: the pre-existing paragraph nine lines above the history door's org comment correctly says "an undeclared member here is now TS2353" — right in its context, which is about undeclared members. Mine sits directly beneath it and describes a declared member. Comment drift by adjacency.
Both sites now say TS2322, and the test-file header keeps the two doors explicitly apart, because this is the half that matters most:
/historyreddens with TS2322;/diffreddens with NOTHING. The diff door reachesdiffMetaItemthrough(p as any), so?? nulltype-checks there and is a silent runtime no-op — the guard is weakest exactly where "the compiler catches this" is easiest to assume. A reader who trusted "TS2353" and went looking for unknown-property behaviour would have concluded the opposite.Re-run on
d73a967d— scoped to what a comment edit can actually disturb, deliberately not the whole union:pnpm lint(repo-wide)node scripts/check-system-context-census.mjsrest-server.ts), re-anchored by--fix, thenOK — 145 anchors resolvepnpm check:doc-authoringpnpm check:doc-anchorspnpm check:nul-bytesexecctx-consumer-censuscounts, re-derived from the patched file75 / 95 / 22 / 16 / 53— all five unchanged, because the new prose contains noresolveExecCtxoccurrence. Measured, not assumed: a mention count is exactly the ratchet prose can moveDeliberately NOT re-run, and why: the
@objectstack/restsuite, the three dogfood shards,check:type-check-debt, and the rest of the path-derived union. The diff is comment-only — verified mechanically, every changed line in both TS files begins with//— so no test behaviour, no type, and no gate population changes. The one gate a comment edit did disturb (the census, via line-number shift) was caught and repaired.Recorded because the standing note says a PATCH merely downgrades the session-URL footer to the bare form, and that the bare form then survives. Measured here, two trials on this body:
---rule and the_Generated by …_line. Read back:_Generated byoccurrences 0,claude.ai/codeoccurrences 0. Not a downgrade; a deletion.Mitigation, and it held: the session id was deliberately also written into prose before that PATCH, and it survived at 1 occurrence. The footer above has been moved out of the trailing position into the header for the same reason. ⇒ Never rely on a trailing footer for attribution across an edited body; put the session id in the body text.
Reading requested by review — the sibling harness, measured
Does
rest-server-meta-read-org-scope.test.tsassert anything about org scoping of/historyor/difftoday? No. Itsdrivehelper exposes exactly three routes and its tests reach no others:PUT /meta/:type/:nameGET /meta/:type/:nameGET /meta/:typeEvery occurrence of "history" in that file is stub-engine plumbing for
sys_metadata_history(the tableSysMetadataRepository.put()appends to during the PUT — without it the write throws). No/historyor/diffroute is registered, driven, or asserted on. ⇒ No vacuous claim exists there today. Its name is broader than its content, but every assertion it makes is about the single-item and list read doors, and its stub genuinely keeps all of them.Is the stub nonetheless a trap for anyone who later adds one? Yes — and on both seams:
Both discard
opts.wherefor that table.historyMetaItem(viarepo.history()) anddiffMetaItemboth filterorganization_idby strict equality, so over this stub that filter is a no-op: every history row comes back whichever partition was asked for. An org-scoping assertion for/historyor/diffadded to that file would therefore pass with or without the org being forwarded — vacuously green. That is precisely why the new file carries its own partitioned stub rather than extending this one.What this does not do
⛔ No
packages/specedit is owed and none is made. The spec anticipated this card by name:HistoryMetaItemRequestSchema's describe text already records that the door "currently sends no organization at all (whether it should is a tenant-scoping question measured separately for that door — declaring the member records the implementation contract, it does not answer that question)", andprotocol.test.tscalls it "the #8747-family measurement the card fences to a future issue". This is that issue. Clause-② is no.⛔
protocol.tsis unmodified. ⛔ #13753 is not addressed here and remains open.