fix(metadata-protocol): render a composite externalId in seed diagnostics instead of its NUL-joined key

[os-musk]

Fixes #16488

A composite externalId's NUL joiner no longer reaches a human-readable seed diagnostic. The map key is untouched.

Source reading: objectstack-ai/ats#20 — measured there, filed here.

1. Call-site classification (the first deliverable)

Counts re-derived on this branch's base (origin/main d4401f7), and the two circulating counts measure different things — so I say which I counted:

what is counted	count
occurrences of the identifier externalIdKey in seed-loader.ts	13
occurrences of externalIdKey(	11
actual call sites	10

The 11 is the 10 calls plus the private externalIdKey( definition on line 2642; the 13 adds two doc-comment mentions ({@link SeedLoaderService.externalIdKey} and a prose sentence on DeferredUpdate.recordExternalId). Triage's "11 call sites" and the dispatching seat's "13 occurrences" are therefore both right about their own subject, and neither is the call-site count. I counted the 10 calls.

line	site	what consumes it	verdict
795	const extIdOf = (rec) => this.externalIdKey(rec, externalId)	existing.has(k) / existing.get(key) at 827, 852, 868	TRUE MAP KEY — unchanged
1235	recordExternalId: on a DeferredUpdate	both: insertedRecords.get(...) fallback lookup at 1760, and five messages that name the record by it	DUAL — the stored field stays the key; the five messages now render it
1368	const externalIdValue (self-ref write, success)	insertedRecords.set	TRUE MAP KEY
1379	const externalIdValue (self-ref write, catch)	existingRecords.get + insertedRecords.set	TRUE MAP KEY
1400	const externalIdValue (batched path)	insertedRecords.set x2, pendingInserts.push({ externalIdValue }) -> insertedRecords.set at 886	TRUE MAP KEY
1449	const externalIdValue (dry run)	insertedRecords.set	TRUE MAP KEY
2142	writeRecord	existingRecords.get(...)	TRUE MAP KEY
2213	decideWriteAction	existingRecords.get(...)	TRUE MAP KEY
2308	buildWriteError keyValue	both: attemptedValue (structured) and the (label=value) parenthetical	DUAL — attemptedValue keeps the key; the message renders
2563	loadExistingRecords	map.set(key, record)	TRUE MAP KEY

Eight of the ten are pure map keys and not one character of them moved. The four sites triage flagged as externalIdValue (1368 / 1379 / 1400 / 1449) are in that eight: the variable name does say "value" while it holds a key, but every one of them is consumed only as a Map key, so renaming — not re-rendering — is what they would want. That is recorded in the acceptance notes, not done here.

Two sites are dual, and the split runs between structured datum and prose:

• structured stays the key — errors[].attemptedValue (the loader's own header calls it "the record's EXTERNAL key ('which row')"), and the two logger.error context objects that carry recordExternalId. A machine reads these, and both serialisers a logger uses (JSON.stringify, util.inspect) escape a control character rather than emitting the byte.
• prose renders — six interpolation points across five messages: buildWriteError's parenthetical (the line the card quotes), pass 2's back-fill-FAILED line, pass 2's DROPPED logger line (two interpolations), pass 2's DROPPED payload message, and pass 2's UNRESOLVED-after-pass-2 line.

2. The joiner is a fossil — read first, kept, quoted

The standing instruction applies squarely here, so the guard's own comment was read before anything moved. It is a ruling and it gives its reason:

joins the per-field values with a separator (U+0000) that cannot occur
in a natural-key value, so ('a', 'b') and ('a\0b', '') never collide.

(The source spells that separator as a JavaScript unicode escape; it is written here as U+0000 on purpose, so that no channel between the source file and this page can turn the quotation itself into the raw byte this card is about.)

The fossil says the current KEY behaviour is deliberate, so the key is exactly as it was. What the fossil does not say — and never claimed — is that the same string belongs in a message. That is the only thing this PR changes. The quoted sentence is reproduced verbatim in the new renderer's own doc comment and in the regression file's header, so the next reader meets it before the same temptation.

3. Rendering chosen: a JSON array of the parts

New private externalIdDisplay(key):

• a key containing no U+0000 — every single-field key — is returned byte-identical;
• a composite key renders as JSON.stringify(key.split(U+0000)), e.g. (employer+user=["ats_employer-1788753956811-1","usr_ats_quillstone_admin"]).

Why JSON and not a + b. Triage's steer was to match externalIdLabel's + unless a value can itself contain +, in which case take JSON. A key part is String(record[field]) for an arbitrary declared field — a composite externalId is not restricted to resolved foreign keys, and a natural-key text value such as Sales + Marketing is ordinary. So the joined form is ambiguous exactly where a composite key is interesting, and the condition triage named for JSON holds. JSON buys a second property the + form does not have: JSON.stringify escapes control characters rather than passing them through, so a value that carries a control byte of its own cannot reintroduce the defect through this path — the rendering is NUL-free by construction, not by removal.

Guarding on the bytes (key.includes(U+0000)) rather than on Array.isArray(externalId) is deliberate for the same reason: it is the byte that hurts, so the byte is the discriminator.

4. The assertion reads bytes, and it was measured red then green

packages/metadata-protocol/src/seed-loader-composite-key-diagnostic.test.ts counts occurrences of U+0000 and demands zero, over both halves of every diagnostic a load produced (result.errors[].message and every logger.error line). A toContain('employer+user') assertion is the trap this card names: the label side always had its +, so that assertion passes with the NUL still in place.

Measured, at the two commits, one command:

tree	result
pins on, seed-loader.ts at base (dcfb364)	3 failed / 5 passed
pins on, fix applied (cb1639c)	8 passed / 0 failed

The three that were red are exactly the three composite-diagnostic assertions; the base run's received line shows the defect literally: (employer+user=ats_employer-1788753956811-1 usr_ats_quillstone_admin) — that gap is the raw NUL. The other five were green at base and after, which is what makes them controls rather than decoration.

Anti-vacuity, in the file: each scenario first asserts that the diagnostic under test actually fired, and one test proves the counter can answer non-zero on a control string, so a silently-broken counter cannot green the suite.

5. Both negative controls

A — a single-key diagnostic is byte-identical. Pinned as a whole-string toBe, not a toContain:

Failed to write demo_solo record #0 (name=acme): the data engine rejected the write; the reason is in the server log

It was green at the base commit and green after — the same string on both trees, which is the measurement "byte-identical" is a claim about. The three pre-existing single-key pins agree: seed-loader-driver-text.test.ts ((name=acme):), @objectstack/objectql's seed-loader-authoring-feedback.test.ts ((name=bad_row):), and the two runtime pins in packages/runtime/src/seed-loader.test.ts. All were run — see verification below.

B — the map key still separates. Two tests, and both were green at base and after:

1. the ruling's own pair, ('a','b') and ('a\0b',''), still land as two distinct rows;
2. the non-degenerate form of the same argument: ('x','y+z') and ('x+y','z') both join to x+y+z under a + joiner and would collide, and under the real joiner they insert as two rows on the first boot and skip as two rows on replay. That test goes red on exactly the "swap the NUL for + so the log looks nicer" edit this card forbids.

6. Verification

Built the dependency closure before the first measurement (pnpm --filter '@objectstack/metadata-protocol^...' build, exit 0). Every heavy run went through scripts/pm/os-verify-lock.sh; verdicts read from its VERDICT command-exit line.

run	verdict
pnpm --filter @objectstack/metadata-protocol test	0 — 170 files passed, 2 skipped; 2441 tests passed, 10 skipped
pnpm --filter @objectstack/metadata-protocol typecheck	0 — and tsc --listFiles confirms the new test file is in the program, so the green covers it
@objectstack/objectql seed-loader-authoring-feedback.test.ts (after building its closure)	0 — 1/1
@objectstack/runtime src/seed-loader.test.ts (after building its closure)	0 — 41/41
pnpm lint (eslint . --no-inline-config, repo-wide, at 9996cb7)	0 — no narrowing claimed, the whole tree was linted

pnpm check:nul-bytes — squarely on-topic, so quoted whole:

✓ check-nul-bytes --self-test: 75 assertions over a temp git repo (real scan() path)
check-nul-bytes: OK (scanned 8297 text file(s) -- 8297 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes).

Every escape in the new and edited source was written as an escape spelling and never as a byte; the diff was additionally hand-scanned with grep -naP over the ASCII control range — no hits.

Gate reconciliation, verbatim (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran ...):

Run reconciliation — 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 63 derived famil(ies) accounted for — 63 run, 0 NOT-MEASURED.

That line accounts for coverage, not verdicts. The verdicts, stated separately:

• 61 of 63 exit 0.
• pnpm check:engine-double-contract exited 1 first — the new test file pinned two engine doubles the ledger did not record ("New pinned coverage is GOOD ... the ledger just has to learn about it"). Regenerated with --write (3 rows added, 0 lost) and committed; the re-run exits 0.
• 2 exit 3 = PREREQUISITE NOT MET, which is NOT MEASURED and is not a pass: check:dual-build-cjs-loads ("Run pnpm build first. This is NOT a pass: nothing was measured") and check:type-check-debt (needs the full closure built). Both read built output for the whole repo; CI builds it and runs them there.
• The derived set grew from 55 to 63 once the ledger file entered the change set — the extra 8 were run too, all exit 0.

7. Changeset

Route 1 of .github/workflows/pr-automation.yml's WHICH LEVEL block (around lines 660-690): "It releases something -> run 'pnpm changeset' and name the packages it releases."

The text rejected is route 2 — "It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -> apply the 'skip-changeset' label." It does not apply, and the decision is a measurement of what this package actually ships, not a feeling about size:

shipped artifact	externalIdDisplay occurrences
packages/metadata-protocol/dist/chunk-3DGZOO2L.js (ESM)	4
packages/metadata-protocol/dist/chunk-JBBC54YJ.cjs (CJS)	4
packages/metadata-protocol/dist/index.d.ts / .d.cts	2 each (one private externalIdDisplay;, one inside a doc comment)

The sibling round's reading holds here too: this bundle carries comment text into dist, so even the doc-comment half of the diff ships. seed-loader.ts is behind index through code splitting, which is why index.js itself shows zero — the code ships in the chunk it re-exports.

Level patch. The WHICH LEVEL rule raises to minor for "a purely additive widening of a published package's public surface (a new exported symbol on an index, a new accepted key or value)". externalIdDisplay is a private class member — emitted into the .d.ts as private externalIdDisplay; beside its siblings private externalIdKey; / private externalIdLabel;, unusable by a consumer — so this is "a fix( that changes no public surface", which the same paragraph keeps at patch. AGENTS.md:1028 is read as the floor against none ("A bug fix in a released package takes a patch changeset — never none, and never skip-changeset"), not as a ceiling; nothing here asks for a level above it.

Clause-②: no

The diagnostic's rendering moves. No exported symbol changes, no key on a published payload changes, and the map-key semantics do not move — my measurement agrees with the seat's declaration on all three. (errors[].attemptedValue keeps its exact previous value; the message string changes only for a composite key, which is the defect.)

验收备注

The triage 验收口径 (all five items), copied in as required — quoted in the original, because rewriting a ruling is rewriting the ruling:

1. 先分类那 11 处(见上):真 map key 的⛔ 一个字都不能动 —— \0 作为分隔符是对的,你已确认它的注释说明了理由(('a','b') 与 ('a\0b','') 不碰撞)。展示用的改为可见渲染。
2. 渲染取一种,⛔ 不要两种:你给了两个候选(a + b 或 parts 的 JSON)。建议与 externalIdLabel 的 + 保持一致(employer+user=A + B),⛔ 但若值本身可能含 +,就取 JSON —— PR 里说明选了哪个及为什么。
3. 回归断言必须读字节,⛔ 不读字符串:断言诊断行里 NUL 出现 0 次。
4. 阴性对照必测:单键 externalId 的诊断行逐字不变;map key 的行为不变。
5. 来源读数请回链 The app cannot boot with data: single tenancy posture vs one organization per employer refuses all 472 seed writes, and no sign-in account exists ats#20,⛔ 不要重述为"某 app 报告"。

All five are addressed above: 1 in section 1 (ten calls classified, eight untouched), 2 in section 3 (JSON, with the +-in-a-value condition triage itself named), 3 in section 4, 4 in section 5, 5 at the top.

Out of scope, noted and not filed:

• The four locals named externalIdValue (lines 1368 / 1379 / 1400 / 1449) hold a key, not a value. Every use is a Map key, so this is a naming defect, not a behavioural one — and a rename is outside this card. Recorded here rather than filed.
• errors[].attemptedValue deliberately keeps the NUL-joined key for a composite. No consumer in this tree renders it into a log line (checked: nothing in packages/cli reads it), and both serialisation paths escape the byte — but a future consumer that string-concatenates it would reproduce the defect at its own boundary. An observation, not a reproducible defect here.
• A gate for control bytes in emitted diagnostics stays fenced by triage as a separate domain:devx card, which must first answer how to inspect emitted bytes without driving a real failure — 「⛔ 不平凡」. Not filed by me.

priority:p3 is the consequence grade, not licence to do it partially — 「它的修法成本近乎零,所以 ⛔ 不要因为 p3 就把它排到很后面」.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

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

• content/docs/data-modeling/validation.mdx (via SeedLoaderService (symbol, a top-level class))
• content/docs/protocol/objectql/state-machine.mdx (via SeedLoaderService (symbol, a top-level class))

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

• content/docs/releases/v17.mdx (via SeedLoaderService (symbol, a top-level class))

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 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 10 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 7f96e1417e01d011884272b28278b6b400521415 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 7f96e1417e01d011884272b28278b6b400521415

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

[os-musk]

落地前检：② 已满足，③ 尚未 —— 保持 draft

domain:engine execution PM seat。本席这一轮把四个 Clause-②: no 的 PR 走完了同一套前检并落了地（#16808 · #16809 · #16817 · #16819）。本 PR 是唯一没落的，理由是它还没绿。

① 不适用 —— Clause-②: no，⛔ 从未挂过载体（标签逐字读过：PR documentation, size/l, tests, tooling；卡 #16488 bug, pm:dispatched, domain:engine, priority:p3 —— 两侧都没有 needs:contract-review）。

② ✅ exit 0：✓ check-clause2-carriers: PR #16830 / card #16488 — the clause-② declaration is readable in the fixed spelling and both carriers agree.

⭐ 两个阴性对照都发火，各自 exit 4：控制 A 只删掉 claim 评论里的 Clause-②: 那一行 ⇒ the DECLARATION LINE is what is missing；控制 B 删掉整条 claim 评论 ⇒ the CLAIM COMMENT is what is missing。⇒ ⛔ 不是空转的绿。文档的 files 由 git diff --name-status 从 merge base d4401f75bb 生成（4 个文件，与 PR 的 changed_files 一致），head 断言等于当刻 tip 9996cb7638，⛔ 正文与评论一律截断而非重打（截断只能让文档说得更少）。

③ ⛔ 未满足：10 个 workflow run 里，Lint & Type Check 仍是 in_progress（run_id 34208478490，无 conclusion）。其余 9 个已完成：8 success（CI · PR Automation · Governed Surface Guard · Docs Drift Check · Check Links · Duplicate Fix Guard · Single-Claim Path Guard · Part-of Closing-Keyword Guard）+ 1 skipped（Pack Smoke，opt-in），⛔ 0 failure。

⇒ 「入队资格要求全部 check 全绿」，而一个 in_progress 不是绿。⛔ 本席不翻 ready、不挂 auto-merge。⚠️ 这不是对本 PR 的判断，是它还在跑 —— 复核清单自己写着：dev 的契约是草稿 PR 时点交报，那时 gate in_progress 是诚实读数；收敛是复核侧的事。

⇒ 本席下一轮巡检重读这一条；Lint & Type Check 一绿即按同一套落地。

---

Generated by Claude Code