Skip to content

fix(service-datasource): stop rendering the unauthorable fields.*.primaryKey in object drafts - #11073

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-11000-draft-drops-unauthorable-primarykey
Aug 22, 2026
Merged

fix(service-datasource): stop rendering the unauthorable fields.*.primaryKey in object drafts#11073
os-warren merged 1 commit into
mainfrom
claude/issue-11000-draft-drops-unauthorable-primarykey

Conversation

@os-warren

@os-warrenos-warren commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes#11000

Note on spelling: the real key is fields. + fieldname + .primaryKey, spelled in the source with angle brackets around the field placeholder. GitHub's body sanitizer strips short angle-bracket fragments — even inside backticks — so this description writes it fields.*.primaryKey throughout, including in the quoted generated source below. The committed files use the angle-bracket spelling.

generateObjectDraft emitted fields.*.primaryKey: true and renderObjectSource rendered , primaryKey: true onto the field line. primaryKey is not a key of the spec field schema, so the *.object.ts the review-before-commit flow handed the user was refused by both instruments the file is annotated for — tsc --noEmit against ServiceObject (TS2353) and ObjectSchema.safeParse (unrecognized_keys).

Per the maintainer ruling of 2026-08-22 (live session, 「同意所有」 item 8 = D): the key stops being emitted, and the introspected value survives as a comment in the generated source — information preserved for the reader, zero contract face. Option C (external.primaryKey: string[] on ObjectExternalBindingSchema) is deferred, not rejected; it returns as its own packages/spec card when federated upsert has a live runtime consumer. No packages/spec file is touched here.

Acceptance — the os build pipeline, both paths, before and after

Three stages driven for real: defineStack()authoringRulesFor('build') (41 rules) → tsc --noEmit over draft.source. Measured at eb16902cc.

pathstageBEFORE (3e26359a7)AFTER (eb16902cc)
opts.primaryKeyunset1 defineStack()PASSPASS
2 authoringRulesFor('build')PASS (41 rules, 0 gating)PASS (41 rules, 0 gating)
3 tsc --noEmitPASS (0 diagnostics)PASS (0 diagnostics)
opts.primaryKeySET1 defineStack()FAILobjects.0.fields.id: Unrecognized key(s) on this field: `primaryKey` PASS
2 authoringRulesFor('build')FAIL (protocol schema; the rules were NEVER REACHED)PASS (41 rules, 0 gating)
3 tsc --noEmitFAILdraft.ts(10,25): error TS2353: … 'primaryKey' does not exist in typePASS (0 diagnostics)

The BEFORE column is a live re-measurement on this branch (pre-fix generator checked out, artifact rebuilt), not an inherited claim. It reproduces exactly what #11059's seat reported.

#10712's acceptance — "both paths build" — is only now fully discharged. #11059 landed the namespace/sharingModel half and left the primaryKey-set row red; this PR clears it. No closing keyword is added for that card: it is already closed by #11059.

⭐ The error-ordering hazard #11059 flagged is confirmed here rather than assumed: on the BEFORE/SET path stage 1 aborted on unrecognized_keys, so nothing downstream of it was measured on that path at all. One path's verdict never covers the other's.

What the replacement comment renders

Single key (opts.primaryKey: ['id'], or introspection reporting one column):

external: {remoteSchema: 'mart',remoteName: 'customers'},// Remote primary key: id// Preserved as a COMMENT because 'ServiceObject' has no authorable key for a// federated object's remote primary key (#11000): 'fields.*.primaryKey' is// not part of the field schema, so emitting it produced a draft that neither// 'tsc' nor 'ObjectSchema' accepted. Nothing below reads this line.// It names the column(s) THIS DRAFT WAS GIVEN as the key. For a COMPOSITE key// some drivers report only the first column (#10997), so treat the list as a// lower bound and check it against the remote table before relying on it.fields: {id: {type: 'text'},
  • composite key → every member, in order: // Remote primary key: order_id, line_no
  • no key reportedno comment at all. There is nothing to preserve, and a "none reported" banner would be noise in every draft of every keyless table.
  • renamed field → the comment names the field name, not the remote column. It sits directly above the fields block, so it has to speak that block's vocabulary or it points at a line that is not there.

On #10997 (SQLite introspection returns only the first column of a composite key — unfixed, engine lane): the wording deliberately does not claim to name "the primary key". It says it names the column(s) this draft was given, and states outright that the list is a lower bound. That caveat is pinned, not just written.

The pin flip

The existing suite pinned the invalid emission. It was flipped, not deleted — inverted, it is now the guard that the unauthorable key does not come back — and the explanatory text beside it was rewritten to cite this ruling (the #11046 stale-comment shape).

What is pinned — four directions

New file external-object-draft-primary-key.test.ts (16 cases):

  1. absent from the definition — every field record's keys are exactly ['type'], on the option path and the introspection path, and nothing reaches importObject's metadata store either.
  2. the draft parses — full ObjectSchema.safeParse success (a value verdict, not an absence-of-unknown-keys check), single and composite. Positive control: the same definition with the key put back is refused, and refused for unrecognized_keys — without that, the green above could be a schema that stopped refusing anything.
  3. information preserved (load-bearing) — the comment names the key, in order, under rename; carries the driver-sql: SQLite introspection reports only the FIRST column of a composite primary key (pk === 1 vs SQLite's 1,2,3… numbering) #10997 caveat; says why; and renders nothing when there is no key. An implementation that just dropped opts.primaryKey passes every other direction and fails these.
  4. the unset path is unchanged from fix(service-datasource): generate an object draft that os build accepts #11059 — namespace prefix, sharingModel, and fix(service-datasource): generate an object draft that os build accepts #11059's own still-generates cases all re-run green.

Proof

Ablation A — comment renderer removed. Predicted in writing first: reddening (not diagnostics-increase, not inversion), toContain on draft.source, and — enumerated before running — exactly 8 named cases, with the absence-asserting cases staying green. Observed: exactly those 8, AssertionError: expected '// Generated by `os datasource intros…' to contain '// Remote primary key: order_id'; 54 passed. (The coarser count written before the suite existed said 3; recorded, and wrong.)

Ablation B — the key put back on the definition side only, renderer untouched. Predicted 8 named cases red, with a discriminator: the field-line and comment cases must stay GREEN, because the renderer no longer reads f.primaryKey. Observed: exactly those 8; discriminator held. The two halves of ruling D are independently pinned.

Both restores proved byte-identical (git hash-object942f2c6ea223e42a173b9b9b8e4375eb50f25ded, matching the pre-ablation hash) and re-run to a real verdict — 62/62 green each time, not trusted to the hash.

Zero-hit counter-check, positive control run FIRST. The same grep -nE "primaryKey: true|, primaryKey: true|primaryKey\?: boolean" matched the pre-fix file at 4 lines — read individually, they are the 4 sites this PR removes (two type annotations, one definition write, one render) — and matches nothing in the fixed file (exit 1). The zero is measured, not assumed.

src vs dist, re-verified from the files rather than inherited — and it is two answers in one PR:

  • unit pins — the subject is imported as '../external-datasource-service.js', a relative specifier that cannot route through exports, so it measures src/. This package's vitest.config.ts carries exactly one alias, anchored /^@objectstack\/core$/, which does not touch it. But the instrument (ObjectSchema from @objectstack/spec/data) is a bare specifier → the workspace link → exports['./data']packages/spec/dist/data/index.mjs, so the schema half needs the closure built. Built and verified present (ObjectSchema in that artifact).
  • os build harness — loads ExternalDatasourceService from packages/services/service-datasource/dist/index.js, so it measures the artifact and is rebuilt before every measurement. Each run fingerprints it: AFTER/RESTORE 126976 bytes with Remote primary key present and , primaryKey: true absent; BEFORE 126139 bytes with the inverse. Two instrument defects in the harness itself were caught and fixed before any verdict was believed — an incomplete manifest that failed stage 1 on both paths, and a startsWith('draft.ts') diagnostic classifier that reported 0 for both files (reading as a clean draft and a silent control at once). The tsc positive control was silent until the second was fixed; it now fires on every run.

Gates — derived on the final commit, clean tree

node scripts/pm/dispatch-gates.mjs with no path arguments, on eb16902cc (6 paths vs merge base 3e26359a7, working tree 0, untracked 0). It named the gates below; the dispatch carried no gate list, so this derivation is the list. Exit codes captured before any pipe; each verdict is the gate's own printed line.

gateverdict
check:changeset-gate-self-tests✓ 212 + 116 assertions over real temp git repos
check:objectui-changeset✓ all checks passed
check:slot-lookup✓ ratchet holds — 107 unswept sites in 25 files, none new
check:test-source-aliasOK — 72 packages with tests scanned
check:type-source-resolutionOK — 77 packages with a tsconfig scanned
check-adr-0087-registration✓ no declared-breaking changeset (2 non-breaking seen)
check-changeset-no-major✓ no major bump
check-ci-filter-parityOK — all 83 declared cross-package globs covered
check-empty-changeset✓ no empty-frontmatter changeset (2 declaring added)
check-plugin-teardown-shape✓ 63 Plugin implementations, baseline fully burned down
check-affected-docs✓ 339 cases pass
check:query-options-erasure✓ ratchet holds — 67 sites, none new
check:type-check-coverageOK — 65/78 packages type-checked
check:type-check-debt --re-measureOK — 33 ledger entries re-measured in 248.1s, none above its recorded number
check:engine-double-contractOK — self-test + 6 consumer seams, all refusing
check:where-matcher✓ 277 matchers, all conforming
check:nul-bytes✓ self-test, 75 assertions

No gate returned a refusal (PREREQUISITE NOT MET / cannot run / Nothing was checked) — the full workspace closure (70 build tasks) was built first, exactly as lint.yml does, so the ratchet re-measured for real. No baseline was touched.check:type-check-debt reports a pre-existing 12-error surplus on @objectstack/plugin-auth that it explicitly calls "not an error"; unrelated to this diff and deliberately left alone.

Package tests: pnpm --filter @objectstack/service-datasource test25 files, 559 tests, all passing.

Declared narrowings

  • Local scope narrowed to the derived union, not the whole check:* farm — CI runs the farm once regardless.
  • tsc --noEmit over draft.source runs in the PR harness, not the committed suite — following fix(service-datasource): generate an object draft that os build accepts #11059's precedent, which committed no tsc case either. The committed suite pins parse + shape + comment; the compile verdict is the measurement above.
  • A caller who passes primaryKey: ['x']andexcludeColumns: ['x'] gets no mention of x at all — the column is not in the draft to name. A contradictory caller, left as-is rather than widened into. Noted, not filed.

Not in scope

#10997 (SQLite composite-PK truncation) and #10998 (introspectSchema omits dialect/introspectedAt) are untouched — engine lane, #10676 ruling. This PR only makes sure the generated comment does not overclaim in the presence of #10997. #10676 itself remains open: generateObjectDraft still reads col.primaryKey directly rather than through this file's own primaryKeyReader union, which is that card's seam and not widened here.


Generated by Claude Code

…primaryKey`
`generateObjectDraft` emitted a field-level `primaryKey: true` and
`renderObjectSource` rendered it onto the field line. `primaryKey` is not a key
of the spec field schema, so the `*.object.ts` the review-before-commit flow
produced was refused by both instruments the file is annotated for: `tsc
--noEmit` against `ServiceObject` (`TS2353`) and `ObjectSchema.safeParse`
(`unrecognized_keys`).
Per the maintainer ruling of 2026-08-22, the key stops being emitted and the
introspected value survives as a comment above the `fields` block — information
preserved for the reader, zero contract face. The comment names the column(s)
the draft was given, says why it is a comment, and does not claim to be a
complete key (some drivers report only the first column of a composite key).
A table with no reported key renders no comment.
With #10712's namespace/`sharingModel` repairs already landed, both paths --
`opts.primaryKey` set and unset -- now clear all three `os build` stages.
An authorable spelling on the binding schema is deferred, not rejected; it
returns as its own spec change once federated upsert has a runtime consumer.
Fixes#11000
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

6 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

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 — 0 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 1c3a46f876822d488d3af43189dbc33273b28015packageMentionDocs.

Which tree this was computed on

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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 22, 2026
@os-warrenos-warren changed the title fix(service-datasource): stop rendering the unauthorable fields.<f>.primaryKey in object draftsfix(service-datasource): stop rendering the unauthorable fields.*.primaryKey in object draftsAug 22, 2026
@os-warren
os-warren marked this pull request as ready for review August 22, 2026 15:25
@os-warren
os-warren enabled auto-merge August 22, 2026 15:26
@os-warren
os-warren added this pull request to the merge queueAug 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

队列构建 32581971774 红了。队列跑的是全量套件(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 队列共有 84 个失败构建(不含本次)。

分诊清单:

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

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

Merged via the queue into main with commit 4257e4eAug 22, 2026
35 checks passed
@os-warren
os-warren deleted the claude/issue-11000-draft-drops-unauthorable-primarykey branch August 22, 2026 15:47
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

Development

Successfully merging this pull request may close these issues.

External object-draft renders a field key that is not authorable: fields.&lt;f&gt;.primaryKey fails tsc against ServiceObject and ObjectSchema.safeParse

2 participants

@os-warren@claude