Skip to content

fix(metadata-protocol): package-less draft saves inherit the overlaid active row's package binding (#11087) - #11139

Merged
os-zhuang merged 4 commits into
mainfrom
fix/listdrafts-scoping-11087
Aug 22, 2026
Merged

fix(metadata-protocol): package-less draft saves inherit the overlaid active row's package binding (#11087)#11139
os-zhuang merged 4 commits into
mainfrom
fix/listdrafts-scoping-11087

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

What

SysMetadataRepository.put() resolves a state='draft' save's package binding as: explicit opts.packageId (never overridden, ADR-0048) → the overlaid active row's package_id → NULL (brand-new item drafted first, unchanged semantics). Plus orphan adoption: an inherited-binding save whose scoped existing-row lookup misses re-reads the package-less draft row for the same (org, type, name) and updates it in place — adopting the pre-fix orphan instead of forking a second draft row.

Why

Live tenant measurement (cloud#1593): a console PUT /meta/:type/:name?mode=draft over a row owned by app.k9qk landed package_id NULLGET /meta/_drafts listed 1 draft, GET /meta/_drafts?packageId=app.k9qk listed 0. Every package-scoped consumer (the AI build surface's standing pending-changes bar objectui#5696, inline publish counts, publishPackageDrafts) is blind to such drafts, and per-package publish can never promote them.

Note: the issue's original first half (org-scope list blindness) was retracted after live re-verification — see the correction comment on #11087; this PR is the confirmed second half.

Tests

4 pins in sys-metadata-repository.draft-package-inherit.test.ts (inheritance + scoped listDrafts counts it; explicit package untouched; new-item NULL; orphan adopted not forked). Full metadata-protocol suite: 1857 passed.

Closes#11087. Cloud consumer: cloud#1593 (pin bump to follow).

🤖 Generated with Claude Code

…id active row's package binding (#11087)
A state='draft' save is a pending change OVER the published row, and
every package-scoped consumer — listDrafts({packageId}), the console's
pending-changes surfaces, publishPackageDrafts — keys drafts by
package_id. A caller that names no base (the console's plain
PUT …?mode=draft) stamped NULL even when the overlaid active row is
package-bound, producing an orphan draft no package view counts and no
per-package publish can promote. Measured live on a cloud tenant
(cloud#1593): GET /meta/_drafts listed the draft, ?packageId= listed
nothing, and the AI build surface's pending-changes bar stayed dark
over a publishable change.
put() now resolves the draft's package binding in this order: the
caller's explicit opts.packageId (ADR-0048 — callers state their
scope, never overridden), else the overlaid ACTIVE row's package_id,
else NULL (a brand-new item drafted first keeps package-less
semantics). Plus orphan adoption: when the binding was inherited and
the scoped existing-row lookup misses, a pre-fix NULL-package draft
for the same (org,type,name) is updated in place and adopted into the
package — never forked into a second draft row.
Four pins in sys-metadata-repository.draft-package-inherit.test.ts;
full metadata-protocol suite 1857 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actionsBot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/error-catalog.mdx(via sys_metadata (literal))
  • content/docs/api/index.mdx(via sys_metadata (literal))
  • content/docs/concepts/metadata-lifecycle.mdx(via sys_metadata (literal))
  • content/docs/data-modeling/drivers.mdx(via sys_metadata (literal))
  • content/docs/data-modeling/objects.mdx(via sys_metadata (literal))
  • content/docs/deployment/cli.mdx(via sys_metadata (literal))
  • content/docs/deployment/validating-metadata.mdx(via sys_metadata (literal))
  • content/docs/kernel/services-checklist.mdx(via sys_metadata (literal))
  • content/docs/permissions/authorization.mdx(via sys_metadata (literal))
  • content/docs/permissions/permission-sets.mdx(via sys_metadata (literal))
  • content/docs/plugins/packages.mdx(via sys_metadata (literal))

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

  • content/docs/releases/implementation-status.mdx(via sys_metadata (literal))
  • content/docs/releases/v16.mdx(via sys_metadata (literal))
  • content/docs/releases/v17.mdx(via sys_metadata (literal))

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 name(s) were too generic to anchor anything (single lowercase words)

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

Which tree this was computed on

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

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…edger
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Lint & Repo Gates is red — diagnosed, and the remedy is a few lines. From the domain:engine seat, not the PR's owner.

Everything else on this head is green (28 of 29 checks, including all six Test Core shards, Temporal Conformance, both type-check gates and all three Dogfood shards). The single failure is check:engine-double-contract, exit 1, 2 problems — and both are this PR's new test file:

x PINNED [delete]: packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts
declares 1 engine double(s) whose delete() does not route through assertEngineDeleteDispatch (line 56)
x PINNED [update]: …same file… whose update() does not route through assertEngineUpdateDispatch (line 56)

That is makeFakeEngine — its update(table, data, opts) and its async delete() { /* not exercised here */ }. The gate's stated reason: "A fake looser than ObjectQL.update is how #4434 shipped a dead REST route with its suite green." A double that accepts dispatch shapes the real engine refuses lets a test pass over a call the product would reject.

The remedy the gate itself names

Open the fake's two verbs with the shared predicates:

import{assertEngineUpdateDispatch,assertEngineDeleteDispatch}from'@objectstack/metadata-core';// …asyncupdate(table,data,opts){assertEngineUpdateDispatch(data,opts);/* … */},asyncdelete(table,opts){assertEngineDeleteDispatch(opts);/* … */},

Prefer @objectstack/metadata-core over @objectstack/objectql (which re-exports them) — the gate spells out why: @objectstack/objectqldepends onmetadata-protocol, so importing from objectql here would be the reverse edge of a cycle turbo refuses. Add the devDependency if the package lacks it.

The baseline entry is not the other half of a choice. The gate says so in as many words — adding a row to scripts/engine-double-contract.baseline.json weakens a shrink-only ratchet and is MAINTAINER-ONLY, explicitly "do not take this path to get CI green." Nor is restating the double as Object.assign(base, …) / { ...base, … } a third option: that inherits the base's accounting, which is a hole rather than a pass if the override restates the verb.

⚠️ Heads-up you cannot see from inside this PR

Issue #11009 is in flight right now and its entire subject is assertEngineUpdateDispatch and its delete twin — it makes a by-id dispatch carrying predicate keys beyond id a loud error, in packages/metadata-core/src/engine-update-dispatch.ts and packages/objectql/src/engine.ts. So the predicate you are about to wire this fake to is being tightened while your PR is open.

Stating the limit of what I checked rather than overstating it: this repository's whereFor(ref, state, packageId) builds a multi-key predicate (type, name, organization_id, state, package_id) with no id, so your put() writes look like predicate dispatch rather than by-id, and #11009's refusal is aimed at the by-id arm. On that reading the two changes do not collide. I have not traced every engine.update call your fake serves, so treat that as the likely case and not a guarantee — wire the assert, and if it starts refusing something after #11009 lands, that interaction is the first place to look rather than a mystery.

No action requested from me and I am not pushing to your branch. Flagged because a single red gate on an otherwise-green head is cheap to fix now and expensive to rediscover later.


Generated by Claude Code

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@os-zhuang
os-zhuang added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit 17bad12Aug 22, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the fix/listdrafts-scoping-11087 branch August 22, 2026 21:44
@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 5.89s
    

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

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

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

历史信号:

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

分诊清单:

  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

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

1 participant

@os-zhuang