You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Route B as adjudicated: one exported MetadataLockState, both declarations derived from it, and the ResourceEditPage lock banner's three && branches replaced by an exhaustive keyed lookup that cannot render an empty title.
The confidence gap, measured
Triage recorded that whether the wire can already carry values outside the four (older/newer servers) is unmeasured. It can, and nothing in this repo stops it.
MetadataClient.layered() casts the wire value straight through, over a raw res.json() body — no parse, no allowlist, no default:
The unions type what this repo may write; they constrain nothing about what a server may send. So this was never only "the day a fifth state lands" — a back end that grows a fifth state reaches the banner with zero code change here.
What a user saw: feeding no-publish through the real render path opened the amber box, drew the padlock and the border, and left the title <div> completely empty — a locked-looking banner that never says what is locked or why. Measured, not inferred:
× an out-of-vocabulary state from the wire still gets a title
→ expected '' not to be '' // Object.is equality
This is why a compile-time-only exhaustiveness check would not have been a fix: satisfies is satisfied by the current four either way, and would have type-checked green over that exact blank render.
What a user sees now, captured from the same render path:
This item is locked, but this console does not recognise the lock state ‘no-publish’ — it may come from a newer server. Some operations will be blocked.
The raw token is deliberate: the operator who meets this is the only person who can report which state their server actually sent.
The card's premise was half wrong — and it makes B strictly better
The card states @objectstack/spec 也没有对应的 z.enum 可派生, and the audit panel's comment says the same. @objectstack/spec 17.1.0 already declares it, in GetMetaItemLayeredResponseSchema:
So the two hand unions were restating a schema that existed rather than filling a gap. MetadataLockState therefore derives from the spec enum rather than being a fresh local alias — the same treatment MetadataOverlayScope already gets twenty lines above it in the same file.
This stays route B and does not stray into A. Route A was that the spec should grow a z.enum — contract-surface work needing cross-repo coordination and maintainer sign-off. The enum is already there; consuming an existing declared type adds no contract surface and moves no accept set. The accept set is byte-identical to both unions it replaces (none / no-overlay / no-delete / full), so clause ② holds on the ruling's own stated ground. The upshot is that B as implemented also closes the cross-repo half of the drift that A was wanted for, at no contract cost.
Reverse-verification
The keyed lookup is only worth having if it can fail. Deleting one key from LOCK_BANNER_TITLE_KEY (mutation confirmed on disk, restored under a trap):
TC_ABLATION_EXIT=2
error TS2741: Property '"no-delete"' is missing in type '{ 'no-overlay': string; full: string; }'
but required in type 'Record<"no-overlay" | "no-delete" | "full", string>'
The required type in that message is the derivation and the exhaustiveness in one line: the key set resolves through MetadataLockState to the spec enum. Baseline type-check was green before the mutation and the restore was byte-identical, so the red is the pin and not a pre-existing failure.
Two comments corrected alongside
Both stated the now-falsified premise, in the same defect class as this card, and would have taught the next reader exactly the belief that produced it:
i18n.ts (LOCK_STATE_ZH) — "not a @objectstack/spec enum, because this repo owns that union today"
AuditPanel.lockState.test.tsx — "there is no @objectstack/spec enum to read at runtime"
Evidence is the spec dist/api/index.d.ts excerpt above. Neither record needed a code change: both key off MetadataAuditEntry['lockState'], so they now bind to the spec-derived union automatically.
Deliberately not done
Hardening layered() to actually parse the protection envelope against the spec schema would move an accept set — a different card by this dispatch's own terms, so it is reported rather than taken here.
Verification
All at b7f0de1d8, the final commit, on a clean tree.
Tests — 375 files / 4190 tests, all green, run from the repo root in four stages (the metadata-admin stage alone takes 578s, so a single run would hit the 10-minute cap):
Superset argument — ResourceEditPage is imported nowhere outside metadata-admin (measured repo-wide). i18n.ts does reach further (studio-design, three views/ modules, one packages/i18n test that reads it as text), and all of those are in the stages above. The i18n change is purely additive — one new key in both tables — so no existing lookup can change verdict; check-i18n-en-drift confirms 0 en value(s) changed, and no app-shell test asserts wholesale over the string tables.
Type-check — @object-ui/app-shell + @object-ui/data-objectstack, both Done, after building the dependency closure.
Lint — both affected packages linted whole (each package's lint is eslint .): 0 errors in both. Warnings on ResourceEditPage.tsx are pre-existing and at lines far from the change; the new test file draws none.
Gates — check-changeset-presence (✅ 5 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)), check-changeset-fixed, check-changeset-no-major, check-control-bytes, check-i18n-call-site-keys (validates the new {state} hole), check-i18n-dead-keys, check-i18n-en-drift, check-spec-symbol-derivation, check-lint-coverage, check-type-check-coverage, check-phantom-dependencies, check-package-self-import — all exit 0.
Reachability of the new export was measured rather than grepped: the exports map has a single . entry pointing at dist/index.d.ts, and that built entry both declares type MetadataLockState and names it in the export list.
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it.
Session session_012u2pRjcqAYtoEjgr3wwhnK. Diff read in full, key claims re-derived locally. No open questions; landing on green.
The premise refutation is the most valuable thing in this report
The card — and two comments in the repo — assert that @objectstack/spec has no z.enum to derive from. You measured that this is false, and the fix got better for it: MetadataLockState now derives rather than being a fresh local alias.
One detail I can add from the cross-card view, which makes the refutation sharper than you stated it. On origin/main, packages/data-objectstack/src/metadata-client.ts already carried, at line 266:
Thirty-two lines above the hand-spelled union at line 298:
lock?: 'none'|'no-overlay'|'no-delete'|'full';
So a sibling field of the same response type, derived from the same spec import, was already sitting immediately above the copy that claimed no such derivation was possible. The counter-example was not buried in a dependency — it was adjacent, in the same file, in the same interface's neighbourhood. That is worth recording because it says something about how the premise survived review: nobody looked up.
Route B, and why this is still B
Your reasoning holds and I endorse it. The A/B distinction was about whether spec grows a surface, not about where the alias's right-hand side comes from. Spec does not change here; you consume a declaration that already exists. The accept set is byte-identical to both unions replaced, so clause ② is satisfied on the merits and not on a technicality. B as implemented also closes the cross-repo drift A was wanted for, at zero contract cost — that is a strictly better outcome than the route as written, reached without exceeding it.
Wire reachability is the finding, and it reframes the card
This is the part that turns a tidy-up into a defect fix:
The unions type what this repo WRITES and constrain nothing about what a server SENDS.
MetadataClient.layered() casts body.lock straight through over a raw res.json(), and isLocked = layered?.lock && layered.lock !== 'none' is true for any non-none value. So the blank banner needed no fifth state added to this repo at all — only a server speaking a slightly newer dialect. The amber box opened, the padlock and border drew, and the title <div> rendered empty: a locked-looking banner that never says what is locked or why.
And you rendered it. The before state is quoted from the real render path, and so is the after:
This item is locked, but this console does not recognise the lock state 'no-publish' — it may come from a newer server. Some operations will be blocked.
Carrying the raw token is the right call for the reason you gave — the operator meeting this is the only person who can report which state their server sent, and a generic "this is locked" would take that away from them.
I also checked the two defensive choices in lockBannerTitle and both are correct rather than incidental: Object.prototype.hasOwnProperty.call (so an inherited key cannot satisfy the lookup) and String(lock) rather than a cast (the same unchecked path can hand you a number or an object, and the fallback must not throw on its way to explaining itself).
Why the two legs were both necessary
You separated them and stated why, which is the part that usually goes missing:
The runtime leg was written before the fix and measured red through the real render path — Tests 3 failed | 3 passed (6). The three passing states are the non-vacuity control: they prove the harness actually mounts the page, so the red is the defect and not broken setup. The failure expected '' not to be '' is precisely the diagnosis — the title element exists and is empty.
The compile-time leg is separately ablated, because an exhaustiveness pin satisfied by the current four proves nothing on its own. Deleting one key gave TS2741, and the message's required type — Record<"no-overlay" | "no-delete" | "full", string> — shows the key set resolving through MetadataLockState to the spec enum. Derivation and exhaustiveness proven in one line. That is a genuinely elegant use of a compiler error as evidence.
And the sentence that ties them: "a satisfies assertion is satisfied by the current four either way and would have type-checked green over that exact blank render." That is exactly why the compile-time half could not have been the whole fix, stated in the form that makes it checkable.
The bounded in-place fix
Correcting the two comments that asserted the false no-spec-enum premise (i18n.ts's LOCK_STATE_ZH preamble, AuditPanel.lockState.test.tsx's preamble) is in scope and I endorse it: same defect class, mechanical, no code change, and both key off MetadataAuditEntry['lockState'] so they are pinned by the same derivation. Leaving a refuted claim in place next to the fix that refutes it would have shipped the contradiction.
Worth naming, because it happened twice this round, independently: #5673 records that #4580's justification docstring is a vacuous measurement written into the repo as evidence, which then propagated into #4972's dispatch obligation. Your two comments are the same failure mode — a false claim written into a source file as evidence, inherited by the next reader as precedent. Two different cards, two different authors, one shape. That is a pattern, not a coincidence, and both were caught by measuring the claim instead of reading it.
Your out-of-scope finding, filed
You declined to file with the quota exhausted rather than skip the dedupe search — correct, and it cost nothing. Filed as #5676: layered() casts the entire ADR-0010 protection envelope through unchecked while spec declares Zod for all of it. I kept your framing that the fix is not "add a .parse()" — parsing turns a silently-wrong render into a thrown response, a behaviour change for every consumer — and that the real question is what a console should do when the server speaks a dialect it does not know. Your deliberate refusal to fix it here (it would move an accept set) is recorded there as the reason it is a separate card.
Landing
Currently 14 success + 3 always-skipped + 5 in_progress (Type Check and all four shards). Flipping ready and enqueuing at 19 + 3. No governed path in the diff; no content/** or apps/site/** path, so Build Docs skips and #5668 does not apply.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5024
Route B as adjudicated: one exported
MetadataLockState, both declarations derived from it, and theResourceEditPagelock banner's three&&branches replaced by an exhaustive keyed lookup that cannot render an empty title.The confidence gap, measured
Triage recorded that whether the wire can already carry values outside the four (older/newer servers) is unmeasured. It can, and nothing in this repo stops it.
MetadataClient.layered()casts the wire value straight through, over a rawres.json()body — no parse, no allowlist, no default:The unions type what this repo may write; they constrain nothing about what a server may send. So this was never only "the day a fifth state lands" — a back end that grows a fifth state reaches the banner with zero code change here.
What a user saw: feeding
no-publishthrough the real render path opened the amber box, drew the padlock and the border, and left the title<div>completely empty — a locked-looking banner that never says what is locked or why. Measured, not inferred:This is why a compile-time-only exhaustiveness check would not have been a fix:
satisfiesis satisfied by the current four either way, and would have type-checked green over that exact blank render.What a user sees now, captured from the same render path:
The raw token is deliberate: the operator who meets this is the only person who can report which state their server actually sent.
The card's premise was half wrong — and it makes B strictly better
The card states
@objectstack/spec也没有对应的z.enum可派生, and the audit panel's comment says the same.@objectstack/spec17.1.0 already declares it, inGetMetaItemLayeredResponseSchema:So the two hand unions were restating a schema that existed rather than filling a gap.
MetadataLockStatetherefore derives from the spec enum rather than being a fresh local alias — the same treatmentMetadataOverlayScopealready gets twenty lines above it in the same file.This stays route B and does not stray into A. Route A was that the spec should grow a
z.enum— contract-surface work needing cross-repo coordination and maintainer sign-off. The enum is already there; consuming an existing declared type adds no contract surface and moves no accept set. The accept set is byte-identical to both unions it replaces (none/no-overlay/no-delete/full), so clause ② holds on the ruling's own stated ground. The upshot is that B as implemented also closes the cross-repo half of the drift that A was wanted for, at no contract cost.Reverse-verification
The keyed lookup is only worth having if it can fail. Deleting one key from
LOCK_BANNER_TITLE_KEY(mutation confirmed on disk, restored under atrap):The required type in that message is the derivation and the exhaustiveness in one line: the key set resolves through
MetadataLockStateto the spec enum. Baselinetype-checkwas green before the mutation and the restore was byte-identical, so the red is the pin and not a pre-existing failure.Two comments corrected alongside
Both stated the now-falsified premise, in the same defect class as this card, and would have taught the next reader exactly the belief that produced it:
i18n.ts(LOCK_STATE_ZH) — "not a@objectstack/specenum, because this repo owns that union today"AuditPanel.lockState.test.tsx— "there is no@objectstack/specenum to read at runtime"Evidence is the spec
dist/api/index.d.tsexcerpt above. Neither record needed a code change: both key offMetadataAuditEntry['lockState'], so they now bind to the spec-derived union automatically.Deliberately not done
Hardening
layered()to actually parse the protection envelope against the spec schema would move an accept set — a different card by this dispatch's own terms, so it is reported rather than taken here.Verification
All at
b7f0de1d8, the final commit, on a clean tree.metadata-adminstage alone takes 578s, so a single run would hit the 10-minute cap):packages/app-shell/src/views/metadata-admin— 192 files, 1969 passed, 1 skippedpackages/data-objectstack+packages/i18n— 91 files, 1436 passedpackages/app-shell/src/views/studio-design— 34 files, 195 passedpackages/app-shell/src/views/*.test.*— 58 files, 590 passedResourceEditPageis imported nowhere outsidemetadata-admin(measured repo-wide).i18n.tsdoes reach further (studio-design, threeviews/modules, onepackages/i18ntest that reads it as text), and all of those are in the stages above. The i18n change is purely additive — one new key in both tables — so no existing lookup can change verdict;check-i18n-en-driftconfirms0 en value(s) changed, and no app-shell test asserts wholesale over the string tables.@object-ui/app-shell+@object-ui/data-objectstack, bothDone, after building the dependency closure.lintiseslint .):0 errorsin both. Warnings onResourceEditPage.tsxare pre-existing and at lines far from the change; the new test file draws none.check-changeset-presence(✅ 5 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)),check-changeset-fixed,check-changeset-no-major,check-control-bytes,check-i18n-call-site-keys(validates the new{state}hole),check-i18n-dead-keys,check-i18n-en-drift,check-spec-symbol-derivation,check-lint-coverage,check-type-check-coverage,check-phantom-dependencies,check-package-self-import— all exit 0.Reachability of the new export was measured rather than grepped: the
exportsmap has a single.entry pointing atdist/index.d.ts, and that built entry both declarestype MetadataLockStateand names it in the export list.Generated by Claude Code
Generated by Claude Code