Skip to content

fix(metadata-protocol): make the destructive-change remedy clause face-aware — stop prescribing ?force=true on the duplicate door (#11015) - #11099

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11015-duplicate-remedy-clause
Aug 22, 2026
Merged

fix(metadata-protocol): make the destructive-change remedy clause face-aware — stop prescribing ?force=true on the duplicate door (#11015)#11099
os-zhuang merged 2 commits into
mainfrom
claude/issue-11015-duplicate-remedy-clause

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #11015

saveMetaItem's Phase 3a-destructive refusal ended every message with — re-submit with ?force=true to proceed. The refusal is raised in one place and quoted onto whatever response the caller's catch builds, so that sentence went out on every face that reaches the gate — including POST /packages/:id/duplicate, where there is no force to set. A caller who does what the sentence says gets the identical refusal back.

Premise re-verified on current origin/main (4257e4e)

The card measured on 490879ad0f; re-derived here, with two line drifts:

claim verdict
duplicate route accepts only targetPackageId / targetName / targetNamespace / organizationId / actor, no force holdspackages/runtime/src/domains/packages.ts:832, the line the card names
duplicatePackage's request type has no force field holdsprotocol.ts:16557
the saveMetaItem call is around protocol.ts:16807 moved — it is at :16844
the clause itself is at protocol.ts:13210, not near :16807
both doors share one message producer holds — one new Error(...) in the Phase 3a-destructive gate

Reachability re-confirmed: a plain duplicate re-namespaces every object, so the target name usually does not exist and the gate is skipped. It fires on the duplicate-again workflow, where the target namespace already holds the renamed object.

The repair

The clause is rendered per face by destructiveChangeRemedy(face, name). duplicatePackage states its face on the internal saveMetaItem call, and gets the remedies that exist there:

… — this copy cannot be forced: the duplicate door accepts no `force`.
Duplicate into a target namespace that does not already hold 'crm2_task', or
reconcile that item with the source first.

Faces that state no door keep the previous wording byte for bytePUT /api/v1/meta/:type/:name reads ?force and threads it, so ?force=true is right there.

Three lines this deliberately does not cross

Verification — all at c8b63f34f

Ablation — direction predicted before running. Mutation: revert only the producer (protocol.ts) to origin/main, keeping the new tests. Predicted the four remedy-clause assertions go RED while the behaviour and prose assertions stay GREEN — a discriminating split, since an all-red result would mean the fixture tests something broader than the clause. Measured exactly that: 4 failed | 9 passed.

RED: section 3's replaced assertion · never prescribes force · prescribes the remedies that DO exist · the face is stated by the SERVER.
GREEN: [#10886 non-effect] findings prose still there · the refusal still REFUSES · OTHER faces keep the ?force=true wording · all four #10886 structural pins.

No rebuild leg is owed and this is not the dogfood exemption being assumed: packages/metadata-protocol/dist did not exist for the whole run, so vitest could only have resolved the relative ./protocol.js specifier to src/protocol.ts. The mutation was confirmed on disk by anchored counts, not by an editor's exit code — destructiveChangeRemedy refs 3 → 0, old clause literal 0 → 1 — and the script carried trap … EXIT INT TERM; the restore leg was confirmed the same way (3 / 0, clean tree) and re-run green.

One stale fixture, replaced not re-spelled. protocol.destructive-409-face-inventory.test.ts asserted expect(r.failed[0].error).toContain(REMEDY) on the duplicate face with REMEDY = 're-submit with ?force=true to proceed.'. It passed because of the branch this PR changes — it was pinning the defect. Replaced with a pin of the corrected remedy; what #10886 put it there to protect (some remedy reaches the caller through this string and nothing else) is unchanged and still asserted.

Suites. Full @objectstack/metadata-protocol suite: Test Files 134 passed | 2 skipped (136), Tests 1844 passed | 10 skipped (1854). Package build (tsup, dts on): exit 0 — so protocol.ts typechecks under declaration emit.

Gates — 20 run, all green at c8b63f34f, exit codes captured before any pipe. Re-derived with node scripts/pm/dispatch-gates.mjs (no paths passed) rather than from the dispatch list, which surfaced five convention-triggered families the dispatch list did not namecheck:query-options-erasure, check:type-check-coverage, check:type-check-debt, check:engine-double-contract, check:where-matcher — because this PR edits a test file. All but the debt ratchet were run and are green:

check:cross-package-test-inputs · check:durability-log-level · check:filter-alias-parity · check:slot-lookup · check:test-source-alias · check:type-source-resolution · check:changeset-gate-self-tests · check:objectui-changeset · check:query-options-erasure · check:engine-double-contract · check:where-matcher · check:type-check-coverage · check:nul-bytes · check-adr-0087-registration · check-changeset-no-major · check-ci-filter-parity · check-cross-package-test-inputs · check-empty-changeset · check-plugin-teardown-shape · docs-audit/check-affected-docs

Two declared narrowings

check:type-check-debt needs the whole workspace built and refuses on an unbuilt worktree. Measured its content directly instead, with the gate's own invocation form: tsc --noEmit -p packages/metadata-protocol/tsconfig.json reports 63 errors against a ledger entry recording 63 (scripts/check-type-check-coverage.mjs:530), and none of the 63 is in either file this PR touches — all sit in 14 untouched test files. The ratchet re-measures per ledger entry and this diff touches exactly one package, so no other entry's count can move.

pnpm lint is the repo-wide scan CI owns. Narrowed to the changed files, with the evidence that makes the narrowing a measurement rather than a skip: ① the population came from eslint resolving the paths under its own config, not from a guess about which files count; ② --format json reported 2 files linted, 0 errors, 0 warnings; ③ the config enables no type-aware linting — no parserOptions.project, no typed @typescript-eslint rules, stated as an invariant at eslint.config.mjs:328 — so this diff cannot move the verdict on any file it does not contain.

Filed, not fixed

The same inventory says rows 2 and 3@objectstack/rest's compound-name PUT /meta/:type/:a/:b (rest-server.ts:6590) and @objectstack/runtime's dispatcher PUT /meta (domains/meta.ts:417) — reach the same gate and also never thread force, so they carry the same unactionable sentence. Filed unassigned as #11095 rather than folded in: unlike the duplicate gesture, those doors have no collision-free alternative to prescribe, so the honest repair may be to thread force on them — a contract decision, not a message fix. #11095 remains open. The docblock and the test header both name it.


Generated by Claude Code

claude added 2 commits August 22, 2026 16:34
…e-aware (#11015)

`saveMetaItem`'s Phase 3a-destructive refusal ended every message with
`— re-submit with ?force=true to proceed.` The refusal is raised in one
place and quoted onto whatever response the caller's catch builds, so
that sentence went out on every face — including
`POST /packages/:id/duplicate`, which accepts no `force` in the query
string or the body, and whose `duplicatePackage` request type has no
`force` field for the internal `saveMetaItem` call to carry.

The clause is now rendered per face. `duplicatePackage` states the face
on its internal call — server-stated, exactly as `source` already is —
and gets the remedies that exist there: a free target namespace, or
reconciling the collision first. Faces that state no door keep the
previous wording byte for byte.

Repairs the CLAUSE, not the door: no `force` is added to the duplicate
route. #10886's verdict is untouched — the per-field findings prose
stays, because `failed[].error` is its sole carrier on that face.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
…paired faces (#11015)

The docblock and the face-inventory header both recorded that inventory
rows 2 and 3 carry the same wrong remedy and were knowingly left. Point
them at the issue that now holds that measurement.

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 1 package(s): @objectstack/metadata-protocol, touching 13 documentable anchor(s).

19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json ab47f6974a275586355f06abd83a0975b8637d12.

3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

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 ab47f6974a275586355f06abd83a0975b8637d12packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json ab47f6974a275586355f06abd83a0975b8637d12

⚠️ 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 ab47f6974a275586355f06abd83a0975b8637d12 → 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 构建失败 — 先分诊,再决定要不要重排

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

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

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

    ✗ Build failed in 6.26s
    

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

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

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

历史信号:

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

分诊清单:

  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/m tests tooling

Projects

None yet

2 participants