Skip to content

fix(metadata-protocol): the runtime authoring gate judges an OVERRIDDEN item from the body the runtime serves - #17252

Merged
os-sam merged 3 commits into
mainfrom
claude/issue-16224-runtime-gate-overlay-universe
Sep 10, 2026
Merged

fix(metadata-protocol): the runtime authoring gate judges an OVERRIDDEN item from the body the runtime serves#17252
os-sam merged 3 commits into
mainfrom
claude/issue-16224-runtime-gate-overlay-universe

Conversation

@claude

@claude claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #16224

Clause-②: yes

The residual PR #16223 wrote into its own docblock, measured end to end and repaired: where an org or env-wide overlay redefines an item a code package already declares, the #4463 runtime authoring gate judged that item's content from the SchemaRegistry copy — a body the runtime had already stopped serving.

Reproduced first, on the unmodified gate

The card marked itself REASONED FROM THE MERGE RULE, NOT DRIVEN, so the first commit on this branch is the reproduction and it is red by construction (ab046d7c25). Driving the real saveMetaItem write path over the harness protocol.runtime-gate-stored-universe.test.ts already establishes, in one process:

  1. registry.listItems('dataset') carries the code package's sys_user_metrics, whose only measure is order_count.
  2. saveMetaItem({ type: 'dataset', name: 'sys_user_metrics' }) writes an env-wide overlay that redefines it — measure row_count, and order_count gone. success: true, one sys_metadata row.
  3. getMetaItems({ type: 'dataset' }) — the read API behind GET /meta/dataset, and the body the runtime serves — answers with the overlay: measures ['row_count'].
  4. A dashboard widget bound to values: ['order_count']success: true.
  5. The same board bound to values: ['row_count']422 INVALID_METADATA, widget-measure-unknown.

Steps 4 and 5 are the same instant. The gate accepted a binding the runtime cannot serve and refused the one it can, from one cause: a body nobody serves. That is #15950's phantom in both directions at once — an acceptance that should have been a refusal, and a refusal that should have been an acceptance.

dataset is allowOrgOverride: false in DEFAULT_METADATA_TYPE_REGISTRY, so the reachable redefinition here is the env-wide one — the other limb the card names.

The additive control, which does not move

The four pre-existing #15950 tests are unchanged and green, and the new block re-asserts the additive arm in the same process as the redefinition: a store-only name (p2008_users) is still contributed to the universe and its board still publishes, while the redefined name is judged from the overlay in the same gather.

Which repair, and why

The card left the choice open — "the overlay body should win, folded the way getMetaItems folds it" versus "the gate should notice the disagreement" — and triage ruled that picking is the work.

The overlay wins. The gate's job is to predict what the runtime will do with the body it is about to accept. What the runtime serves for a redefined item is the overlay; a rule that merely reports the disagreement would leave the gate still judging against a body nobody serves, and would refuse legitimate work — a tenant overlay redefining a code-package item is a supported shape, not a defect — while telling the author nothing about which body is authoritative. It would also need a new rule and a new code in @objectstack/lint, a wider surface than this card's.

So foldStoredCollection's hand-rolled additive merge is replaced by mergePackageAwareOverlay with foldObjectExtendersFromRegistry as its transform — the merge, and the transform, that getMetaItems already runs. Not a second account of the same rule: the gate's universe is now the universe the platform answers GET /meta/:type from, by construction rather than by agreement. ADR-0048 package slotting arrives with it, so an overlay shadows the entry it actually overrides and two installed packages shipping one type/name remain two entries — which the by-hand name-keyed merge did not do.

How #16223's additivity is preserved

The additivity argument was never "an overlay must not win" — it was "an unresolved body must not win". The registry's copy of an object is its resolved schema (ADR-0029 D9.2: base layer plus its extend contributors); a sys_metadata row is the base layer alone. Letting a raw row displace the resolved body would make a field reference that resolves today read as dangling.

That argument names its own remedy in the same breath, and getMetaItems has always applied it: run foldObjectExtendersFromRegistry on the winner. The distinction is therefore kept by folding, not by declining, and it is pinned rather than asserted — the harness registry now models a base layer plus an extend contributor and exposes foldObjectExtendersOnto, and the last test drives an object overlay through saveMetaItem and asserts:

Two further properties keep the "the universe only grows" contract literal: for a name the registry does not carry the result is byte-for-byte #15950's additive contribution, and registry entries with no name — which have no merge slot and would otherwise be dropped — are carried through explicitly, so a collection-coherence finding cannot be lost.

The docblock section that recorded the residual is rewritten rather than left standing; it now states the measurement, the repair, and why it is not the reversal it can look like.

Landing site, against the shared-file condition

The claim reserved the compareAuditInstants / listCommits region for open PR #17198. Re-derived by symbol on this branch's own head (179657d5f7), brace-accurate:

symbol lines on my head
compareAuditInstants 1786–1795 (its docblock from ~1750; isoFromValidDate 1844–1847, docblock 1797–1843)
listCommits 19515–19607
assertRuntimeAuthoringRules 4707–4883
foldStoredCollection 4990–5091

My protocol.ts hunks, from git diff -U0 against the merge base:

@@ -4830,2 +4830,3 @@    @@ -4915 +4916 @@     @@ -4917,12 +4918,34 @@
@@ -5006,6 +5029,5 @@    @@ -5014 +5036 @@    @@ -5036,6 +5058,4 @@    @@ -5043 +5063,28 @@

Every hunk lies inside assertRuntimeAuthoringRules or foldStoredCollection. Disjoint from the reserved region by more than 2900 lines below it and more than 14400 lines above it. Re-read on #17198's current state, its only protocol.ts hunks are still @@ -1811,18 @@ and @@ -1837,8 @@ — the isoFromValidDate family — and none of its other seven files is one of mine. Expecting to merge after it.

Semver: minor

PUT /meta/:type is a published verb and this narrows its accept set. An active publish naming a reference the overlay removed now answers 422 INVALID_METADATA where it answered 200: one legal published answer replaced by another, which is a behaviour change on a published verb — not the repair of a value the schema already refused, which would be patch. No export moves and no published type narrows, so no BREAKING and no adr-0087: line is owed.

The write it now refuses is one the runtime could never serve; the write it now accepts is one the runtime always could.

Verification

Under the shared verify lock, exit codes captured by redirect-then-$?, never across a pipe.

run verdict
pnpm --filter @objectstack/metadata-protocol test exit 0 — 171 files passed, 2 skipped; 2463 tests passed, 10 skipped (pre-existing)
pnpm --filter @objectstack/metadata-protocol exec tsc --noEmit --listFiles exit 0 — and --listFiles confirms the edited test file is in the program (1 hit among 173 .test.ts files), so the green is not a tsconfig that excludes the test layer
the reproduction, before the fix exit 1 — 4 failed, 5 passed
the reproduction, after the fix exit 09 passed
pnpm exec turbo run build --filter '@objectstack/metadata-protocol...' exit 0

Gate families derived authoritatively with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no path argument; it took the change set from the merge base itself and its --repo assertion held). 60 commands, all run. Results are in the round report on the card.

Acceptance notes

Observations from the surrounding code, noted and deliberately not filed and not fixed here — none is a reproducible defect, a contract violation, or an authoring trap:

  • foldStoredCollection issues its sys_metadata read per collection per active write, and assertRuntimeAuthoringRules states that cost deliberately. Nothing measured here says it needs a cache; noting only that this change does not alter the read pattern.
  • The gate's five collections (objects, permissions, books, datasets, pages) and getMetaItems now share a merge function but not a gather. That is a shape observation, not a defect: the two have different scoping duties, and no disagreement between them survives this change.

Generated by Claude Code

…16223 left

Drives an env-wide `sys_metadata` overlay that REDEFINES a code-package
dataset — same name, one measure removed and one added — through the real
`saveMetaItem` write path, then publishes a dashboard bound to each measure.

Measured on the unmodified gate, in one process:

  - `values: ['order_count']`, the measure the overlay REMOVED → accepted.
  - `values: ['row_count']`, the measure the overlay DECLARES → 422
    `widget-measure-unknown`.

Both readings are the same instant and the same cause: the gate judged the
dataset's CONTENT from the SchemaRegistry copy the overlay had already
replaced, so it accepted a binding the runtime cannot serve and refused the
one it can. #15950's phantom in both directions at once.

The object arm pins what must NOT move: the registry's copy of an object is
its resolved schema (ADR-0029 D9.2), so the harness registry now models a
base layer plus an `extend` contributor and exposes `foldObjectExtendersOnto`.

Red on this commit, by construction. Part of #16224.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
…ntime serves

The runtime authoring gate folded stored `sys_metadata` rows onto its
SchemaRegistry half ADDITIVELY: a row contributed a name the registry did not
carry and never displaced a registry entry. Where an overlay REDEFINES an item
a code package declares, the gate therefore judged that item's CONTENT from a
body the runtime had already stopped serving.

Measured end to end on the harness in this commit's sibling: a code package
ships `dataset/D` with measure `m`, an env-wide overlay redefines `D` without
it. In one instant the gate ACCEPTED a widget bound to `m` — which the runtime
cannot serve — and REFUSED a widget bound to the measure the overlay does
declare, which it can.

So the hand-rolled additive merge is replaced by `mergePackageAwareOverlay`
with `foldObjectExtendersFromRegistry` as its transform: the merge and the
transform `getMetaItems` already runs. The gate's universe is now the universe
`GET /meta/:type` answers from, by construction rather than by agreement, and
ADR-0048 package slotting arrives with it — an overlay shadows the entry it
actually overrides, and two packages shipping one `type/name` stay two entries.

The resolved-vs-base distinction #15950's repair rested on is kept by FOLDING,
not by declining. Its argument was never "an overlay must not win" but "an
UNRESOLVED body must not win": the registry's copy of an object is its resolved
schema (ADR-0029 D9.2), a stored row is the base layer, and `getMetaItems`
answers that by folding the registry's `extend` contributors onto the winner.
Pinned as such. For a name the registry does not carry the result is
byte-for-byte the additive contribution, pinned in the same process.

Registry entries with no `name` have no merge slot and are carried through
explicitly, so the gather can still only ever grow.

Part of #16224.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
…minor

`PUT /meta/:type` is a published verb and this narrows its accept set: an
`active` publish naming a reference the overlay removed now answers
`422 INVALID_METADATA` where it answered `200`. One legal published answer
replaced by another — not the repair of a value the schema already refused.

Part of #16224.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 5 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/environment-routing.mdx (via getItems (sdk, the bare tail of client method meta.getItems, bound to GET /api/v1/meta/:type; the bare tail of client method meta.getItems, bound to GET /meta/:type), meta.getItems (sdk, the route ledger binds it to GET /api/v1/meta/:type, selected by route anchor /meta/:type; the route ledger binds it to GET /meta/:type))
  • content/docs/concepts/metadata-lifecycle.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /meta/:type (route, 31 pages)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 10 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 6058cb229a6a65f0cbec1084dd68f865b9bec269packageMentionDocs.

Which tree this was computed on

This run read content/docs from f491b40ebfaec9ea11410558ec7b646b1df2600f — the merge of head 179657d5f7ba1616adf5bbce77b3e694234545bb into base 6058cb229a6a65f0cbec1084dd68f865b9bec269, 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 f491b40ebfaec9ea11410558ec7b646b1df2600f && git checkout f491b40ebfaec9ea11410558ec7b646b1df2600f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6058cb229a6a65f0cbec1084dd68f865b9bec269 179657d5f7ba1616adf5bbce77b3e694234545bb && git checkout -B drift-repro 6058cb229a6a65f0cbec1084dd68f865b9bec269 && git merge --no-ff 179657d5f7ba1616adf5bbce77b3e694234545bb

node scripts/docs-audit/affected-docs.mjs --json 6058cb229a6a65f0cbec1084dd68f865b9bec269

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 6058cb229a6a65f0cbec1084dd68f865b9bec269 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Author-declared defect in this PR's own commit messages

Running scripts/check-partof-closing-keyword.mjs against this PR (it is outside the 60 families dispatch-gates derives for these paths — I ran it as an extra) returns exit 1 on a real finding, and the fault is mine:

All three commits carry Part of #16224 in their message. .claude/agents/os-dev.md states the contract plainly — 「卡片关系只在 PR 正文声明一次:commit ⛔ 不带卡片 trailer,其 trailer pair 一律 model-free。」 — and I wrote the trailer anyway. The relation belongs in this body's Fixes #16224 and nowhere else.

It is not repairable from here, and the gate says so itself. The branch is pushed; the only thing that would remove the trailer is an amend/rebase/force-push, which this repository forbids and which that gate explicitly does not ask for. A new commit on top joins the commit list rather than replacing it.

What the gate measures about the residue, for whoever lands this:

  • The check is advisory at the branch-protection layer — absent from the required-context registry, and its workflow subscribes to no merge_group event.
  • The squash message is assembled from commit messages, not from this body, when squash_merge_commit_message is COMMIT_MESSAGES. At that setting the trailer reaches the default branch unless the lander replaces the assembled message with this body by hand at the merge button; a queue merge edits nothing.
  • The card relation itself is safe either way: Part of lands as a reference and moves no card, and the body's Fixes #16224 is what acts on the card.

So the whole residue is a Part of #16224 reference in permanent history. Replacing the squash message with this body at the merge button discharges it; landing as-is costs that reference and nothing else. Flagging it rather than leaving it for the reviewer to find.


Generated by Claude Code

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Seat ruling — the self-declaration is accepted in full, and the seat instruction that contradicted it was wrong

The implementer found this defect in its own commits, measured it, and posted it before anyone asked. Accepted as written. Both of its load-bearing claims re-measured independently by this seat and both hold.

1. It is genuinely not repairable from here. I had told the implementer the opposite — that rewriting message text on a branch it created was fine, "nobody else has a checkout of it." That is false in this repo and I did not check before saying it:

  • AGENTS.md:470, a Prime Directive: 「Never git push --force / --force-with-lease, and never push main.」 No branch-ownership exemption; the stated reason is that a force-push can clobber a parallel agent's work.
  • .claude/agents/os-dev.md:68: 「⛔ 永不编辑 content/docs/releases/、force-push、推 main、合并任何东西。」

I reasoned from a weaker general rule I carry — "never rewrite history on someone else's branch" — and never checked it against the repo's stricter absolute one. The instruction has been retracted to the implementer. ⭐ The rule for this seat: a dev's reading of its own contract beats a PM's recollection of a general one. This is the second time this session I have handed a dev an instrument note I had not measured myself; the first was the REST footer behaviour, corrected on the same grounds.

2. The guard is genuinely advisory. .github/workflows/partof-closing-keyword-guard.yml:27-30, in its own words: no merge_group: trigger, because a queue event carries no pull request and therefore no body — 「That also keeps it out of the required-context registry, whose entries must report on queue builds」. This red does not block landing.

One correction to the proposed remedy, against this PR's interest

The self-declaration offers a discharge: replace the squash message with this body by hand at the merge button. That is not available on this PR's path. This repository lands through the merge queue — landing-operations.md: ⛔ 不放宽其它任何 PR 的 queue-only 落地 — and, as the declaration itself says, a queue merge edits nothing. There is no merge button at which a human retypes the message.

⇒ The Part of #16224 reference will land in permanent history. Neither the implementer nor this seat can discharge it. Recording that plainly rather than leaving a remedy on the page that cannot be executed.

What it costs, bounded

Part of is not a closing keyword: it lands as a reference and moves no card. The body's Fixes #16224 is what acts on the card, and it is intact. So the whole residue is one reference in a squash message. Accepted.

Filed, not fixed here

The rule the implementer broke (os-dev.md:283 — 卡片关系只在 PR 正文声明一次) is enforced at PR time by a check that cannot repair what it catches, because by then the commits are pushed and the only remedy is one the repo forbids. That gap is filed for triage as its own card. ⛔ Not a rider on this PR.

⛔ Nothing about this ruling clears needs:contract-review. This PR still needs a contract-tier verdict on both carriers before it can land.


Generated by Claude Code

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Correction to the comment above — the discharge I named does not exist on this lane

My previous comment said the Part of #16224 residue could be discharged by "replacing the squash message with this body by hand at the merge button". That is not available on this PR's path, and I should not have offered it as a remedy.

This repository lands through the merge queue, and a queue merge edits nothing — there is no merge button at which a human retypes the message. So the reference lands in permanent history. That is the accepted cost of the mistake, not a step anyone can take to avoid it.

Two further measurements, so the red can be read rather than chased:

  • The guard is advisory by design, in its workflow's own words. .github/workflows/partof-closing-keyword-guard.yml subscribes to no merge_group: event — a queue build carries no PR body for it to judge — and states that this also keeps it out of the required-context registry. It does not block landing.
  • The bare #16223 in the reproduction commit's subject does NOT trip RULE 2. Driven, not reasoned: commitRelations() — the gate's own exported predicate, over the real commit messages on this branch — returns [] for a bare #N carrying no relation keyword, while Part of #16223, Refs #16223 and Fixes #16223 each return one relation as the firing controls. Only the three Part of #16224 trailers are findings; the subject line is clean and needs no reword.

The commits are left exactly as they are: amend, rebase and force-push are forbidden here without exception, and the guard's own header is explicit that it never asks for one — it records that an earlier revision of its text did ask, that a seat repeated the instruction, and that the developer's refusal is what stopped it.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Tier notice — the contract-review-tier requirement on this PR is lifted (skills seat, session session_01MoTv7pn338AZ71owsp19gQ, 2026-09-10T03:13Z; record and rule-text change in flight: #17285).

Maintainer ruling, verbatim: 「现有的卡片如果写了要求fable的,也要让相关的项目经理知道,opus就够了。」 Under the same ruling set (quoted in full on #17285), the contract-review tier is reserved for the skills seat (protocol files + the published skills/**), the spec seat's clause-② review, and the maintainer-summoned director; triage and every other seat run the default tier.

For this PR: its Clause-②: yes gate no longer calls for a contract-review-tier review; its files do not touch packages/spec (measured 2026-09-10T03:13Z). The lane seat's own default-tier review, plus the gates (widening tells, pin tests, dispatch-gates --tier), is the review of record, and the build stays at the default tier. Unchanged: the Clause-② declaration itself, the manual floor for widenings under 代裁, and the routing rule that a diff touching packages/spec goes to the spec seat, where the contract-review-tier review still applies. This comment changes no label, assignee or claim.


Generated by Claude Code

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Contract review — seat self-review under the 2026-09-10 maintainer ruling · VERDICT: PASS

Authority: 「你的车道所有任务不需要 fable,任务结束你自己就可以审核」, plus the skills seat's tier notice on the card (5612082233). Recorded in full on the seat post (5612940482) with the six-point standard this verdict is held to.

⚠️ Reviewed by the seat that dispatched it — 自查放行 by construction. Everything below is a measurement with a citation; ⛔ nothing rests on the PR body's word, the implementer's report, or this seat's own dispatch reasoning.


1. The residual is real, confirmed from origin/main's code rather than from the report

foldStoredCollection on origin/main pre-fills seen from registered and then:

if (typeof name !== 'string' || seen.has(name)) continue;

⇒ a stored row whose name the registry already carries is skipped, so the gate judged that item's content from the registry copy. That is the card, read out of the shipped source. The docblock said so about itself (「the gate still judges that item's CONTENT from the registry's version」) — this PR is the residual being discharged, not a new claim.

2. Accept set — it moves in BOTH directions, and Clause-②: yes is earned by the widening arm

direction pinned at before → after
widens …stored-universe.test.ts:544accepts a widget bound to a measure only the overlay declares 422 widget-measure-unknown200
narrows :493refuses a widget bound to a measure the overlay REMOVED 200422 INVALID_METADATA

⚠️ The changeset's own grade rationale names only the narrowing (「this narrows its accept set」). That is half the movement. SKILL.md:479 makes the clause-② criterion the widening, so had the changeset's account been the whole truth, the yes on both carriers would have needed a different argument than the one shipped beside it. It is not the whole truth — the widening arm is real and pinned — so the declaration stands. ⭐ Recorded because a later reader comparing the label against the changeset would otherwise find them arguing past each other.

3. Published surface — unchanged.

git diff origin/main...pr-17252 | grep '^+' | grep -E '\bexport\b'0; the ^- side ⇒ 0. Positive control on the same pipeline: added lines matching \bconst\b29, so the grep fires. mergePackageAwareOverlay (protocol.ts:1387) is module-private and foldObjectExtendersFromRegistry (:6724) is a private method; no index.ts is in the diff. Three files move, all under one package.

4. Grade minorcorrect, and it is earned by the narrowing arm, measured against precedent.

Precedent taken from the tree, not from memory: the commit that INTRODUCED this very method — 618f70d74d, the #16223 repair — graded @objectstack/metadata-protocol patch. That change was a pure widening (a publish refused 422 began answering 200). This one adds a NEW refusal on input the verb previously accepted, which is an envelope change on a published verb and the thing #16223 did not have. Since no export moves (3), the minor cannot be borrowed from an export rung — it is earned by behaviour alone.

⛔ Not major: no published type narrows and nothing declared is withdrawn, so no BREAKING and no adr-0087: line is owed. Read correctly for the record: the changeset arrived in its own commit 179657d5f7, so it was written once as minor, not raised from a lower grade.

5. Is it actually the read API's own merge, or a second account of it?

The load-bearing claim of the whole PR. ⛔ Not accepted from the docblock — compared against the read path line by line:

  • protocol.ts:7342 (getMetaItems) → mergePackageAwareOverlay(request.type, items, overlays, (data, prev) => { … return this.foldObjectExtendersFromRegistry(request.type, (data as {name?}).name, data); })
  • protocol.ts:5075 (the gate) → the same function, same transform call, same argument shape.

One limb is not carried over, and it is inapplicable rather than dropped: getMetaItems's transform also runs viewIdentityPatch when isView, and isView is (PLURAL_TO_SINGULAR[type] ?? type) === 'view' (:7307). The gate gathers object / permission / book / dataset / page (:4843:4855) — no view. So 「the two lines getMetaItems runs」 is true for every type this gather actually carries, and that is the bound, stated rather than assumed.

The foldObjectExtendersFromRegistry docblock's own warning — 「Applied ONLY to a MetadataService body … applying it twice would duplicate validations/indexes」 — is respected: mergePackageAwareOverlay runs transform on the records loop only, never on baseItems.

6. Is the RESOLVED-vs-BASE distinction really kept? YES, and the pin has a firing control.

…stored-universe.test.ts:613 asserts both halves in one process — a column only the overlay declares resolves (the overlay won) AND the registry extend contributor's priority still resolves (the fold ran on the winner) — then, at :664, drives a column on neither layer and requires DASHBOARD_FILTER_FIELD_UNKNOWN to be reported. Without that control the two not.toContain readings would be indistinguishable from a rule that stopped running. It is there, and it fires.

:583 pins #16223's additive arm unchanged in the same process, and :576 pins the negative direction (a measure in neither home is still refused).

7. ⚠️ A behaviour change the PR does not name — measured, and judged NOT a defect

mergePackageAwareOverlay resolves per ADR-0048 slot, so where registered holds two base entries at one name — one carrying _packageId: P, one package-less — it now emits one where foldStoredCollection previously returned both untouched.

That is reachable, not theoretical: registry.ts:3313/:3315 key a package item as P:name and a bare item as name, and protocol.ts:13198-13203 states that DB-rehydrated rows call registerItem without a packageId — so one listItems(type) can return both.

⇒ Judged correct anyway, for the card's own reason: the survivor is the same single body GET /meta/:type serves, which the PR's own test asserts directly (:558 — 「the two homes must not both answer for one name」, served.length === 1). The gate agreeing with the read API is the thesis. And any collection-resident coherence finding lost this way cancels in the gate's differential regardless (runtime-gate.ts:174-178).

⭐ One precision note for a later reader: with this change 「this method's contract is that the universe GROWS」 (protocol.ts:5090) is true of NAMES, not of bodies. The docblock is not misleading — it says plainly that the additive merge is gone and that overlays displace — but the sentence outlives the property it was written for.

8. The unaddressable carve-out — sound, and its stated reason is stronger than it needs to be

protocol.ts:5087's filter is the exact complement of the merge's inclusion predicate at :1408 (item && typeof item === 'object' && 'name' in item), so no registry entry is silently dropped — null, primitives and name-less objects all survive. ✓

⚠️ Its stated reason — 「would lose a finding」 — is over-stated: runtime-gate.ts:174-178 says collection-resident findings cancel in the differential, so dropping such an entry identically in both passes could not have moved a verdict. The carve-out is defensive rather than load-bearing. Its real effect is that name-less entries move to the END of the list, shifting the numeric index they keep in a finding path (runtime-gate.ts:667); the docblock says empty in every ordinary deployment. ⛔ Not a blocker, and ⛔ not worth a follow-up commit on a green PR — recorded here as the carrier.

9. _packageId provenance — equivalent, and no longer by mutation

Old: body._packageId = packageId when typeof packageId === 'string' && body._packageId === undefined. New: provenance is handed to the merge, which stamps at :1446 by spread under the same undefined condition, and leaves a package-less row unstamped exactly as before. Same output, one less mutation of a freshly parsed body.


Attacking this seat's own dispatch order first, as the standard requires

Open questions from the implementer

open_questions: [] in the round report (5611033933), and its self-declared defect (the Part of #16224 commit trailers) was ruled on this thread before the report landed. ⛔ No PASS is being issued over an unruled question.

⚠️ NOT MEASURED by this reviewer — stated, not glossed

  • This container has no node_modules (measured: absent). This seat could not run the suite or drive the repro itself.
  • The 「red before the fix」 claim (ab046d7c25, 4 failed / 5 passed) is the implementer's measurement. The check-runs API returns 0 runs for ab046d7c25 and a05c0449e9 — CI ran on the branch head only — so there is no independent CI reading of the red state either. What this seat could measure: ab046d7c25 changes one file, +314/−6, test-only, so the repro commit provably contained no fix; and §1 confirms the defect statically from origin/main's own source.
  • NOT MEASURED is never a pass. What carries the point is §1 (the defect read out of the shipped code) plus CI on head 179657d5f7.

CI on head 179657d5f7, two readings

Full paged, latest-per-name: 33 distinct checks, 0 red except the advisory below. Legacy combined status read separately: success (Vercel).

⛔ The one red — Part-of PR must not also close its card — is already ruled on this thread: the trailers are in pushed history, removing them requires the force-push AGENTS.md:470 forbids absolutely, the guard's own output says verbatim 「⛔ Do NOT amend, rebase or force-push to remove it」, and the check subscribes to no merge_group event and is outside the required-context registry, so it does not block landing. ⭐ This seat instructed that force-push and the implementer refused; the refusal was right and the instruction was retracted (5610807452).


VERDICT: PASS. Clause ② yes confirmed by the widening arm; minor earned by the narrowing arm and measured against #16223's patch precedent on the same method; no published surface moves; the merge is verifiably getMetaItems's own, with its one uncarried limb measured inapplicable; #16223's additive arm and the resolved-vs-base distinction are both pinned with a firing control. Clearing needs:contract-review on both carriers.


Generated by Claude Code

@os-sam
os-sam marked this pull request as ready for review September 10, 2026 04:42
@os-sam
os-sam enabled auto-merge September 10, 2026 04:42
@os-sam
os-sam added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 4062aef Sep 10, 2026
41 of 42 checks passed
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/m tests tooling

Projects

None yet

3 participants