Skip to content

fix(metadata-protocol): render the spec-validation 422 findings clause per write face (#10888) - #11120

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10888-face-aware-422-trim
Aug 22, 2026
Merged

fix(metadata-protocol): render the spec-validation 422 findings clause per write face (#10888)#11120
os-zhuang merged 1 commit into
mainfrom
claude/issue-10888-face-aware-422-trim

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #10888

⚠️ Notation. GitHub's body sanitizer deletes short angle-bracketed fragments from a PR body on save — even inside code spans, and even backslash-escaped (both were tried on earlier versions of this text, and the second one silently turned the measurement below into its own opposite). So where the code emits the literal placeholder root wrapped in angle brackets, this body writes ⟨root⟩. The substitution is this body's, not the code's.

saveMetaItem's spec-validation 422 INVALID_METADATA rendered its own findings into the message — the first three issues, each as its path, a colon, and its message, plus a (+N more) tail — and attached the same array as err.issues. On the HTTP 422 both channels ride one response, so every console rendering both showed each finding twice: the #10524 shape, on the save door.

Implements the maintainer ruling on #11017 (2026-08-22, option D). Quoted verbatim, untranslated:

本卡的 declare-then-trim 前置改为:不再等待 duplicatePackage 响应契约(A 不做)。trim 改按 face 执行——复用 #11015/PR #11099(commit 82cb6e84)建立的按 face 渲染机制:有结构化 issues[] 通道的 face 删除 422 散文复述,duplicate face 保留全文(其 failed[].error 是唯一载体,#10886 判定 + GUARD pin 原样)。

Zero packages/spec change. Option A (declaring a response contract for duplicatePackage) is not taken here.

The ruling's premise, measured before any edit

The ruling marked its own load-bearing assumption unmeasured: that the 422 refusal flows through the same face-declaration channel #11099 built. It does. Re-derived on origin/main @ 147f9cd29 (the card body's ~:13306 is stale):

claim verdict
the 422 site is inside saveMetaItem's body zodIssuesToMetadataIssues(parsed.error.issues) at :13443; saveMetaItem spans :12908:13996 (next method migrateStoredMetadata at :13997)
request.writeFace is in scope at the 422 site ✅ same parameter list, :12908
the duplicate face actually reaches the 422 ✅ and it was already measured in-tree — duplicatePackage's catch at :16925 records that the spec rejection is 422 INVALID_METADATA and that the whole "Unrecognized key(s) on this view container … defineView({ … })" prescription comes through unchanged

The inventory had to be re-derived — the 409's table is wrong here in three rows

This is the part that shaped the design. The 409 fires only when !request.force and the folded type is object/field and a row exists and the diff is non-empty; that predicate eliminates four of its seven callers. This gate has none of those conditions — it fires whenever getMetadataTypeSchema resolves a schema for the folded singular type and the body fails safeParse.

# caller type reaches THIS gate face structured issues[]
1 rest PUT /meta/:type/:name any yes sendError 422 body yeserror-response.ts:1454 threads a top-level issues
2 rest PUT /meta/:type/:a/:b any yes same body yes
3 runtime dispatcher PUT /meta any yes errorFromThrowndetails.issues yeshttp-dispatcher.ts:732, pinned at http-dispatcher.test.ts:223
4 runtime ADR-0045 visibility flip app yesapp: AppSchema is registered unhideError + log no
5 migrateStoredMetadata any yesforce: true does not gate this check rows[].reason no
6 duplicatePackage row.type yes failed[].error on a 200 no — sole carrier
7 plugin-security projection ×4 permission yespermission: PermissionSetSchema logger.error text no

Rows 4, 5 and 7 are the ones the 409's inventory eliminates by a constant and this one cannot. Row 5's own catch comment states the dependency outright ("each is still quoted verbatim and still names the fix"); row 7's log prescribes "make the record body spec-valid (the error names the offending key)" — a remedy written on the assumption that this clause names it.

The ruling's principle holds unchanged, but the "keep full text" side has four members rather than one.

Consequence for the design: silence keeps the prose

Rows 4 and 7 live in other packages, reached through (protocol as any). Neither can state a face without making writeFace a field an arbitrary caller sets. Under the opposite polarity — trim by default, message-only faces opt out — both lose their prescription silently, and so does every write door added later by an author who never read the comment.

So the polarity is declare-to-trim: an absent or unrecognised face renders the full prose byte-identically, and only rows 1–3 declare writeFace: 'meta-envelope'. The destructive direction requires an explicit, reviewable declaration at a call site and can never happen by omission.

Not client-settable, structurally: each door builds the request object field by field from named req values and never spreads the body.

GUARD pins: all three green, unmodified

The card named three pins that went red when a blanket trim was tried. Each was checked for which face it protects:

pin protects verdict
protocol.batch-verb-driver-text.test.ts P10 the duplicate face (row 6), end-to-end through duplicatePackage must stay green — it does, untouched
protocol.save-union-issues.test.ts:140 the message on a saveMetaItem call with no face (rows 4–7) must stay green — it does, untouched
protocol.save-union-issues.test.ts:262 same — the container prescription on the default face must stay green — it does, untouched

No pin was moved, and none was made green by deleting what it protects. That all three sit on the keep side is the strongest evidence the polarity is right.

Reverse verification — direction predicted before running

Predicted: deleting specValidationFindings' 'meta-envelope' case (so every face falls to the prose branch) turns the 'meta-envelope' pins RED and moves nothing else, because the three GUARD pins assert the default face.

Mutation proven on disk before measuring — anchored greps on both the injected and the deleted text, not an editor exit code:

MUTATION ON DISK: injected=1 original=0
 FAIL  protocol.invalid-metadata-422-face-inventory.test.ts > no issue message is restated in the sentence
 FAIL  protocol.invalid-metadata-422-face-inventory.test.ts > the headline still names HOW MANY and WHERE
 FAIL  protocol.invalid-metadata-422-face-inventory.test.ts > a root-level union failure still locates itself, never blank
 Tests  3 failed | 1850 passed | 10 skipped (1863)
RESTORE PROOF: injected=0 original=1

Exactly the predicted direction and scope. The subject is imported as ./protocol.js — a relative source specifier — so vitest resolves it to src/protocol.ts, no dist/ on the path, and the ablation needs no rebuild; the restore leg is proven on disk too, and the mutation script carried a trap … EXIT INT TERM.

The headline's worst input, measured (the card's open question)

The card asked whether a trimmed headline degrades unacceptably on root-level union failures, where every locator's path is empty and the key names live only in issues[]. Its worry was that the locators would collapse to nothing but a bare [invalid_union]; [unrecognized_keys] with empty space where the paths should be.

Measured: they do not render blank. An empty path falls back to the ⟨root⟩ placeholder, so the locator reads ⟨root⟩ [invalid_union]; ⟨root⟩ [unrecognized_keys]. That fallback predates this card and survives it. Degraded but not lossy: by the definition of the face, issues[] rides along carrying the key names, and the pin asserts both halves — including the absence of the blank-locator shape.

Hoisting key names into the locator would change metadataIssueHeadline, which the seed refusal and the author-time gate also compose — three doors' grammar. Deliberately not done here; per the dispatch that is an improvement, not a requirement.

Changed files

  • packages/metadata-protocol/src/protocol.ts — new specValidationFindings face switch; MetadataWriteFace extracted so the 409 and 422 switches share one vocabulary; the 422 site; the stale "deliberately NOT trimmed" comment replaced.
  • packages/rest/src/rest-server.ts, packages/runtime/src/domains/meta.ts — rows 1–3 declare the face.
  • packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.tsnew, the inventory and its pins (8 tests).
  • packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts — a guard that a face declared for the 422 does not move the 409 remedy. Without it, folding the two switches together would change a 409 remedy on the busiest write door while every 422 test stayed green.
  • packages/runtime/src/http-dispatcher.test.ts — two exact-match call-shape pins now record the server-stated face.
  • packages/rest/src/rest-4xx-message-truncation.test.tsfixture triage, no assertion changed: its docblock claimed its constructed long message is what this route receives. That is no longer true for the /meta door, so the docblock now says the long form belongs to the message-only faces and that the subject under test is sendError's bound, not the metadata door's wording.
  • scripts/engine-double-contract.pinned.json — the new test file's engine double, recorded as pinned coverage (--write); the shrink-only baseline is unchanged at 66/100.

Deviation from the dispatched file surface, declared

The dispatch expected protocol.ts + tests. Rows 1–3 are in rest and runtime, and touching them is forced by the measurement: inside protocol.ts every caller of saveMetaItem is a message-only face, so a server-stated-only mechanism could mark only the keep side — which forces the unsafe polarity. No contract surface widens: spec's SaveMetaItemRequestSchema declares {type, name, item} only, and force/mode/packageId/source/parentVersion/writeFace are all already undeclared internal fields.

Verification — all on 9b2dc276d

  • pnpm --filter @objectstack/metadata-protocol test1853 passed | 2 skipped (1844 before, +9 new)
  • pnpm --filter @objectstack/rest test2179 passed; pnpm --filter @objectstack/runtime test2696 passed
  • pnpm --filter @objectstack/rest --filter @objectstack/runtime typecheck — both Done. metadata-protocol declares no typecheck script (ledger package); its closure is covered by the build and the debt ratchet below.
  • Gate union re-derived with node scripts/pm/dispatch-gates.mjs (no paths — it reads the change set itself) and re-run on this commit: check:authz-resolver, check:changeset-gate-self-tests, check:cross-package-test-inputs, check:dispatcher-error-vocabulary, check:durability-log-level, check:filter-alias-parity, check:objectui-changeset, check:route-envelope, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:type-check-coverage, check:nul-bytes, plus check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-ci-filter-parity, check-cross-package-test-inputs, check-plugin-teardown-shape, docs-audit/check-affected-docs — all exit 0.
  • pnpm check:type-check-debt — refused first with the unbuilt-closure error, so the closure was built (turbo run build, 70/70) and it was re-run: OK — 33 ledger entr(ies) re-measured in 355.3s, 1908 raw tsc error(s) total, none above its recorded number.
  • ESLint: repo-wide pnpm lint is CI's run; delivered here as a measured narrowingeslint --no-inline-config --format json over the 7 changed files, 7 files linted, 0 errors, 0 warnings. The narrowing excludes nothing: eslint.config.mjs never enables type-aware linting for any file (:328, "no parserOptions.project, no typed @typescript-eslint rules", verified there with a positive control), so this diff cannot move the verdict on any untouched file.

Generated by Claude Code

…e per write face (#10888)

`saveMetaItem`'s `422 INVALID_METADATA` restated its own findings in the
message (`<path>: <message>` for the first three, plus a `(+N more)` tail)
while attaching the same array as `err.issues`. On the HTTP 422 both channels
ride one response, so every console rendering both showed each finding twice.

The clause is now rendered per face, reusing the `writeFace` channel #11015
established for the sibling 409. The `/meta` HTTP write doors — REST's
`PUT /meta/:type/:name` and `PUT /meta/:type/:a/:b`, and the runtime
dispatcher's `PUT /meta` — declare `writeFace: 'meta-envelope'` and get the
#10524 headline: issue count plus up to three `path [zod code]` locators, the
same grammar `seedRequestValidationError` and the author-time gate compose.
`err.issues` is attached unchanged on every face.

Silence keeps the full prose. The inventory re-derived for THIS gate finds
four message-only faces, not one: `duplicatePackage`'s `failed[].error`,
`migrateStoredMetadata`'s `rows[].reason`, and two out-of-package log faces
that the 409's table eliminates by a constant and this gate cannot — `force`
does not exempt a caller here, and `app` and `permission` both have registered
schemas. Two of them live in other packages and could not declare a face
without making the field caller-settable, so the destructive direction
requires an explicit declaration and can never happen by omission.

#10886's verdict is untouched: the duplicate face keeps the prescription in
full, and its three #8333 GUARD pins stay green unmodified.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/concepts/metadata-lifecycle.mdx (via saveMetaItem (symbol))
  • content/docs/deployment/validating-metadata.mdx (via saveMetaItem (symbol))
  • content/docs/kernel/services-checklist.mdx (via saveMetaItem (symbol))
  • content/docs/permissions/authorization.mdx (via saveMetaItem (symbol))

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

  • content/docs/releases/v17.mdx (via saveMetaItem (symbol))

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 changed file(s) yielded no anchor (packages/rest/src/rest-server.ts) — pages documenting those are invisible to this run
  • 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 — 32 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 5337ef162dbbb5c24125045ee6ef7375fb984966packageMentionDocs.

Which tree this was computed on

This run read content/docs from 03fbee70fd0dca830fc8d6dd4aaec648f6ba5c00 — the merge of head 9b2dc276d3f378b1aadf86b078cc1abd049934d6 into base 5337ef162dbbb5c24125045ee6ef7375fb984966, 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 03fbee70fd0dca830fc8d6dd4aaec648f6ba5c00 && git checkout 03fbee70fd0dca830fc8d6dd4aaec648f6ba5c00
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5337ef162dbbb5c24125045ee6ef7375fb984966 9b2dc276d3f378b1aadf86b078cc1abd049934d6 && git checkout -B drift-repro 5337ef162dbbb5c24125045ee6ef7375fb984966 && git merge --no-ff 9b2dc276d3f378b1aadf86b078cc1abd049934d6

node scripts/docs-audit/affected-docs.mjs --json 5337ef162dbbb5c24125045ee6ef7375fb984966

⚠️ 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 5337ef162dbbb5c24125045ee6ef7375fb984966 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 5.93s
    

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

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

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

历史信号:

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

分诊清单:

  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

2 participants