Skip to content

Inventory the DESTRUCTIVE_CHANGE 409's wire faces, and pin the sole carrier that forbids the trim - #11016

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10886-destructive-409-face-inventory
Aug 22, 2026
Merged

Inventory the DESTRUCTIVE_CHANGE 409's wire faces, and pin the sole carrier that forbids the trim#11016
os-elon merged 1 commit into
mainfrom
claude/issue-10886-destructive-409-face-inventory

Conversation

@os-elon

@os-elonos-elon commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10886

The card's own first step is the face inventory, and it binds: which faces quote this 409's message, and which of them carry issues structurally. The inventory is done, and its answer is DO NOT TRIM — the same verdict, reached the same way, as the sibling INVALID_METADATA message that was trial-trimmed during #10524 and reverted.

No behaviour changes. One new pin file, plus a comment recording the verdict at the throw site so the next author is told which measurement they are standing on.

The duplication is real

saveMetaItem Phase 3a-destructive (packages/metadata-protocol/src/protocol.ts, the join now at :13169 — the card's :13131-13141 is stale, #10895 moved it) renders its findings into the message and attaches the same array as err.issues. Measured through the real producer:

message "[destructive_change] object/crm_task would drop or transform existing data: Field 'b' removed — existing data in this column will become inaccessible.; … — re-submit with ?force=true to proceed."
issues [{ code: 'field_removed', field: 'b', message: "Field 'b' removed — existing data in this column will become inaccessible." }, …]

A console rendering both channels shows every finding twice.

The inventory, and how it was enumerated

The 409 is raised in exactly one place, so a face is any place a caller's catch puts the thrown value's .message onto a response. The enumeration is therefore mechanical: every caller of saveMetaItem in the repo (grep -rn "saveMetaItem(" --include=*.ts --include=*.tsx --include=*.mts, minus prose, tests and dist — seven call sites; a .js/.mjs/.cjs sweep adds only check-durability-degradation-log-level.mjs's own self-test fixtures), then for each: can it reach the gate, and if so what does its catch emit.

The gate fires only when ALL of !request.force, folded type object or field, an item already exists under the target name, and the diff is non-empty. That predicate eliminates four of the seven outright.

#callertypeforcereaches gatefaceissues structurally
1rest/src/rest-server.ts:5472PUT /meta/:type/:nameanyfrom ?forceyeshandleRouteError 409 bodyyes — top-level issues
2rest/src/rest-server.ts:6590PUT /meta/:type/:a/:banyneveryesthe same handleRouteError bodyyes (same face as 1)
3runtime/src/domains/meta.ts:417 — dispatcher PUT /metaanyneveryeserrorFromThrown to details.issuesyes
4runtime/src/domains/packages.ts:517 — ADR-0045 visibility flipliteral 'app'nono — type(unhideError)n/a
5protocol.ts:14145migrateStoredMetadataanyforce: trueno — force(rows[].reason)n/a
6protocol.ts:16807duplicatePackagerow.type incl. objectnoyesfailed[].error on a 200NO — sole carrier
7plugin-security/src/permission-set-projection.ts (x4)literal 'permission'nono — typen/an/a

Rows 4, 5 and 7 are eliminated by a constant in the call itself — a literal type, or force: true — so there is no runtime state that could make them reach the gate. Rows 1-3 and 6 are pinned in the new test file.

The plugin REST API catalog (packages/spec/src/api/plugin-rest-api.zod.ts:816) declares saveMetaItem as a handler, but it is a declaration rather than a dispatcher: the routes it describes are rows 1-3, already counted.

Rows 1-3 do not block a trim

All three resolve through the one shared rule, resolveThrownHttpError (@objectstack/types). Measured:

  • REST — status 409, and a flat body carrying error (the message text), code: 'DESTRUCTIVE_CHANGE', and a top-level issues array. resolveErrorResponse's 4xx arm reads error.issues straight onto the body.
  • dispatcher — details.issues on the ApiErrorSchema envelope.

A message trim loses nothing on any of them.

Row 6 is why the trim is refused

duplicatePackage reports per-item failures as response DATA on a 200 (POST /packages/:id/duplicate), so no HTTP boundary is involved and details.issues never exists. Three facts, each independently sufficient:

  • the array is typed inline as Array<{ type: string; name: string; error: string }> (protocol.ts:16691) — no issues slot;
  • the push (protocol.ts:16838) adds no issues key;
  • unlike publishPackageDrafts, whose failed[]Publish refusals: declare failed[].issues + seedApplied.issues, then trim error to a headline #10895 could extend because PublishPackageDraftsResponseSchema exists, duplicatePackage has no response schema in packages/spec at all (grep -rn "DuplicatePackage" packages/spec/src returns nothing).

So the declare half of declare-then-trim is not done for this refusal, and trimming would delete the prescription from that wire.

Reaching row 6 was measured, not argued, and the obvious first attempt says the wrong thing: a duplicate re-namespaces every object (com.acme.crm to com.acme.crm2 maps crm_task to crm2_task), so the target name usually does not exist, prev is null, and the gate is skipped — the copy fails the author-time gate instead. The gate fires on the ordinary duplicate-again workflow, where the target namespace already holds the renamed object:

failed[0].error = "[destructive_change] object/crm2_task would drop or transform existing data:
Field 'b' removed — existing data in this column will become inaccessible.; …
— re-submit with ?force=true to proceed."
failed[0].issues = undefined ('issues' in failed[0] === false)

Expected NON-effects, stated before each run and confirmed

Reverse verification — direction predicted before running

Predicted with the message trimmed to a headline (count plus field locators, remedy kept — the trim this card declines): the two prose assertions go RED, everything else GREEN, because the structured channel is untouched by a message trim. Predicted 2 red / 5 green. Measured exactly that, and the two red cases were the two predicted:

x the message restates the SAME prose the `issues` array carries
x [GUARD] … carries the per-field prose with NO structured channel beside it
✓ declares the ADR-0112 envelope and attaches the structured findings
✓ [GUARD] the message ends with the actionable remedy
✓ the shared boundary resolver threads `issues` into `details`
✓ reaches the gate at all — the copy is refused with the destructive change
✓ [GUARD] carries the `?force=true` remedy, on a response with no other channel for it
Tests 2 failed | 5 passed (7)

src/dist in both directions: the tests import ./protocol.js — a relative source specifier — and packages/metadata-protocol ships no vitest config and no alias table, so vitest resolves the subject to src/protocol.ts and no dist/ is on the path. The mutation therefore needed no rebuild, and its RED result is what rules out the stale-artifact false green (a mutation that never reached the subject stays green). Restoration proved on disk: git hash-objectd1a54eca01c78e96360ab002172475bee5770956 equals git rev-parse HEAD:packages/metadata-protocol/src/protocol.ts, git diff --exit-code 0, porcelain empty, ablation marker absent (grep -c 0). The restore leg then re-ran green 7/7.

Tests and gates — all at 25c318d944, the final commit

pnpm --filter @objectstack/metadata-protocol test
Test Files 133 passed | 2 skipped (135)
Tests 1823 passed | 10 skipped (1833)

metadata-protocol declares no typecheck script (it carries a DEBT ledger entry instead), so its type coverage is the ratchet's, run below.

Gate union from node scripts/pm/dispatch-gates.mjs with no path arguments, after the final commit — 10 path-matched plus the 5 it names as convention-triggered by adding a test file. Exits captured before any pipe. All green:

cross-package-test-inputs EXIT=0 query-options-erasure EXIT=0
durability-log-level EXIT=0 type-check-coverage EXIT=0
filter-alias-parity EXIT=0 engine-double-contract EXIT=0
slot-lookup EXIT=0 where-matcher EXIT=0
test-source-alias EXIT=0 nul-bytes EXIT=0
type-source-resolution EXIT=0
check-ci-filter-parity.mjs EXIT=0
check-cross-package-test-inputs.mjs EXIT=0
check-plugin-teardown-shape.mjs EXIT=0
docs-audit/check-affected-docs.mjs EXIT=0

Quoting the gates' own verdict lines rather than a bare exit code:

check-engine-double-contract: OK — 377 pinned, 133 in the DEBT ledger, 2 exempt.
check-type-check-coverage: OK — 65/78 workspace packages type-checked (plus the root), 13 in the DEBT ledger …
check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 327.1s, 1908 raw tsc error(s) total, none above its recorded number.
check-nul-bytes: OK (scanned 6363 text file(s) … no raw ASCII control bytes).
✓ where-matcher conformance holds: 276 matcher(s) discovered …
OK: 13 package(s) read outside themselves, all declared …

check:type-check-debt is the ratchet half and refuses to run on an unbuilt worktree, so the workspace closure was built first exactly as lint.yml does (turbo run build --filter=./packages/* --filter=./packages/*/*, 70/70 successful) before the --re-measure above. No baseline or debt ledger moved in either direction — git status after the run named only the two files in this diff.

One gate result is worth recording because the first run was RED. check:engine-double-contract asked for the new test's fake engine to be added to the pinned ledger. Rather than growing the ledger, the double's update/delete were removed: every case in this file drives a REFUSAL, so nothing is ever persisted and those verbs were never called — verified by removing them and re-running 7/7 green. A comment in their place states that adding a persisting case means adding them back through assertEngineUpdateDispatch / assertEngineDeleteDispatch. The gate then passed with no ledger change.

Changeset

None, deliberately. The diff is one new test file plus a comment block; nothing user-visible ships and no package's behaviour changes. Requesting skip-changeset.

Out-of-scope findings, filed unassigned rather than fixed here


Generated by Claude Code

… and pin the sole carrier
#10886. `saveMetaItem`'s Phase 3a-destructive 409 renders its findings into
the message AND attaches the same array as `err.issues`, so a console that
renders both channels shows every finding twice — the render-then-attach shape
#10524 trimmed on the publish refusals.
The card's own first step is the face inventory, and the inventory says DO NOT
TRIM. Enumerated from every caller of `saveMetaItem` in the repo, then filtered
by the gate's own predicate (`!force`, folded type `object`/`field`, an existing
item under the target name, a non-empty diff), four of the seven callers cannot
reach the gate at all — three pass a literal non-object `type`, one passes
`force: true`. Of the three that can:
- the two `@objectstack/rest` `PUT /meta` doors and the `@objectstack/runtime`
dispatcher door all resolve through `resolveThrownHttpError`, so `issues`
reaches the wire structurally (a top-level `issues` on the REST body,
`details.issues` on the dispatcher envelope) — a message trim would lose
nothing there;
- `duplicatePackage`'s `failed[].error` is a SOLE CARRIER. It reports per-item
failures as DATA on a 200 (`POST /packages/:id/duplicate`), so no HTTP
boundary is involved and `details.issues` never exists; the array is typed
inline as `{ type, name, error }`; and unlike `publishPackageDrafts` — whose
`failed[]` #10895 could extend because it has a response schema —
`duplicatePackage` has none in `packages/spec` at all.
So the declare half of declare-then-trim is not done for this refusal, and the
trim is refused. Declaring a channel on that face is a `packages/spec` change
and is deliberately out of this card's scope; reported to the PM instead.
Reaching that face was measured, not argued, and the obvious attempt misleads: a
duplicate re-namespaces objects, so the target name usually does not exist, the
gate is skipped, and the copy fails the author-time gate instead. The gate fires
on the duplicate-again workflow, where the target namespace already holds the
renamed object.
No behaviour change: one new pin file plus a comment recording the verdict at
the throw site, so the next author is told which measurement they are standing
on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 3 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
  • 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 — 7 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 5359a9b4c67c737b8319e8b08e44561aaed4dd1apackageMentionDocs.

Which tree this was computed on

This run read content/docs from fcf4a2ddca22948ae22fa616837db5498d32d485 — the merge of head 25c318d94498df6c976f78745aa97e09b9a93a1e into base 5359a9b4c67c737b8319e8b08e44561aaed4dd1a, 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 fcf4a2ddca22948ae22fa616837db5498d32d485 && git checkout fcf4a2ddca22948ae22fa616837db5498d32d485
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5359a9b4c67c737b8319e8b08e44561aaed4dd1a 25c318d94498df6c976f78745aa97e09b9a93a1e && git checkout -B drift-repro 5359a9b4c67c737b8319e8b08e44561aaed4dd1a && git merge --no-ff 25c318d94498df6c976f78745aa97e09b9a93a1e
node scripts/docs-audit/affected-docs.mjs --json 5359a9b4c67c737b8319e8b08e44561aaed4dd1a

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

@os-elonos-elon added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 22, 2026 — with Claude
@os-elon
os-elon marked this pull request as ready for review August 22, 2026 06:04
@os-elon
os-elon enabled auto-merge August 22, 2026 06:04
@os-elon
os-elon added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit 809e612Aug 22, 2026
38 checks passed
@os-elon
os-elon deleted the claude/issue-10886-destructive-409-face-inventory branch August 22, 2026 06:22
@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 5.91s
    

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

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

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

历史信号:

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

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-elon@claude