Skip to content

fix(service-datasource): generate an object draft that os build accepts - #11059

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-10712-external-draft-os-build
Aug 22, 2026
Merged

fix(service-datasource): generate an object draft that os build accepts#11059
os-warren merged 1 commit into
mainfrom
claude/issue-10712-external-draft-os-build

Conversation

@os-warren

@os-warrenos-warren commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10712. The generated object draft now passes os build on the default
path. What the opts.primaryKey path still does — and why that is a different card —
is stated under "The acceptance boundary" below, measured rather than assumed.

The two defects, reproduced first

Both reproduce on 2866d5f97 (the card measured at 79ebb37; the tree has moved
several times since, so neither failure is inherited). The generator was driven off a
real SqlDriver.introspectSchema() against a live in-memory SQLite database, then
the rendered draft was held to the same three stages os build holds it to.

stageinstrumentbefore
1defineStack() — namespace-prefix validation (ADR-0028)Object 'customers' is missing the package namespace prefix. Rename it to 'wh_customers' (namespace = 'wh').
2authoringRulesFor('build') — the 41 rules os compile runssecurity-owd-unset at objects[0].sharingModel
3tsc --noEmit over draft.source✓ (clean on the default path)

Stage 3 is the instrument the #10676 seat left in PR #11001renderObjectSource
annotates its output ServiceObject, so tsc over the rendered file is a complete
check of the draft's shape. It is complete for shape and blind to the two defects
above, which is why stages 1 and 2 are here: neither an unprefixed name nor a missing
sharingModel is a type error.

The namespace half — where the prefix comes from

Derived from the datasource's own owning package. A federated object is bound to
exactly one datasource (definition.datasource), so the package that declared that
datasource is the package the object belongs in. Both links are read, not assumed:

  • _packageId is stamped onto every registered metadata item carrying package coords
    by applyProtection (@objectstack/spec/shared), on both load paths — the artifact
    loader and registry.registerItem. The 'sys_metadata' rehydration sentinel is
    excluded, exactly as the registry's own isCodeArtifactBody excludes it.
  • the package record is what installPackage stored under manifest.id — the same
    { manifest } shape the runtime publish gate reads for this identical check.

Resolution is injected (ExternalDatasourceServiceConfig.getNamespace), keeping the
service kernel-free like every other read it makes; plugin.ts supplies the wiring.

The prefix itself is applied through validateObjectNamespacePrefix — the single
source of the rule, shared verbatim with defineStack() and
MetadataManager.publishPackage. Imported rather than re-spelled as startsWith: a
hand-rolled check here could pass while the real gate refuses, which is the drift that
produced this defect. It also gets the already-prefixed case right for free — a remote
table wh_accounts under namespace wh stays wh_accounts, not wh_wh_accounts.

When the namespace is absent or empty

The draft keeps the bare remote-table name — today's output, unchanged — and the
rendered source carries a loud TODO(namespace) naming ADR-0028 and the rename to
make. It does not invent a prefix. That mirrors defineStack, which skips the
check entirely for a stack with no manifest.namespace and, in its own words, "does
not invent a prefix on the author's behalf because doing so would silently introduce a
second writing style". A bare name is legal exactly where that stack is legal.

A blank or whitespace-only namespace is normalised to absent. That case is asserted,
not assumed: validateObjectNamespacePrefix skips a falsy namespace, but ' ' is
truthy and would have rendered _customers — one invalid draft traded for another,
which is the failure this card exists to close.

The sharingModel half — #9666's shape, not a new judgement

#9666 hit this same rule family for the os init template, and its resolution
(PR #9736, live in packages/cli/src/commands/init.ts) is: declare the value
explicitly, and pick 'private'
— the rule's own recommended default — with the
reason attached as a comment. This draft follows that verbatim, comment included.

It transfers for three reasons stated rather than assumed: 'private' is what
ADR-0090 D1 already resolves an unset OWD to at runtime, so the draft describes the
posture the platform would apply anyway; it is the most restrictive of the four
canonical values, so this default can never publish a federated object wider than its
author intended; and the change is that the baseline becomes an authored decision
instead of an accident, which is the whole point of security-owd-unset. No new
security posture is being chosen here.

The acceptance boundary — what the opts.primaryKey path still does

Measured on the final commit, both paths, same harness:

opts.primaryKeystage 1 defineStackstage 2 rulesstage 3 tsc
unset (the default path)✓ PASS✓ PASS✓ PASS
set (the #11000 path)Unrecognized key(s) on this field: primaryKeynot reached — schema parse failedTS2353 … 'primaryKey' does not exist

The default draft now builds; the opts.primaryKey path still does not, pending
#11000.
Both of this card's own defects are visibly discharged on that failing path
too — it renders name: 'wh_customers' and sharingModel: 'private' — so what remains
is #11000's alone: fields.FIELDNAME.primaryKey is not an authorable spec field key.
Its likely fix is packages/spec surface and its routing is an open contract question,
so it is deliberately untouched here. #11000 remains open and is not addressed by this PR.

Before the fix, the primaryKey-set path failed on unrecognized_keysbefore the
namespace check ran — so on that path this card's first defect was masked, not absent.

What is pinned — src/__tests__/external-object-draft-os-build.test.ts, 15 cases

The pre-existing 525 stay green unchanged, which is the finding in its own right:
that suite is blind to both defects, and stays blind because it wires no namespace.

  • the two defects pinned separately. A single "it builds now" assertion cannot say
    which one it measures, nor fail informatively when one regresses alone. The prefix
    case asserts through validateObjectNamespacePrefix — the function defineStack()
    itself calls. The OWD case runs a full ObjectSchema.safeParse, because what is
    guarded is a value's verdict, not a key's presence.
  • still-generates (load-bearing). Both defects are satisfiable by emitting less: a
    minimal valid stub would score green on a validator-only suite while destroying what
    the generator is for. So the introspected columns and their mapped types, the remote
    table name, remoteSchema, and the external binding are all asserted to survive —
    including that the object was renamed to wh_customers while external.remoteName
    stayed customers.
  • the absent/empty-namespace case, four ways (no resolver wired, a resolver that
    finds nothing, '', ' '), each asserting the bare name and never a leading
    underscore, plus the TODO's presence and — the other direction — its absence once a
    namespace does resolve.
  • importObject inherits both repairs through the draft pipeline.

Verification — 14550cb58, clean tree

pnpm --filter @objectstack/service-datasource testTest Files 24 passed (24) ·
Tests 540 passed (540). typecheck → exit 0, no error TS.

Ablations — signatures predicted in writing BEFORE mutating, one per defect

Two ablations, because the card requires the two defects to be pinned separately and
separate ablations are what prove the separation.

A — revert the prefix only (applyNamespacePrefix(shortName, ns)shortName).
Predicted: exactly 2 failures, both AssertionError on the name, in
"prefixes the derived name…" and "persists the prefixed name and the explicit OWD";
totals 1 failed | 23 passed (24), 2 failed | 538 passed (540). Predicted not to
redden, with reasons: the double-prefix case (its table is already wh_accounts, so
dropping the step leaves the same correct name — it guards double-prefixing, not the
prefix being dropped), the label case, the whole absent-namespace block, the TODO-absence
case, and all of still-generates.
Observed: 2 failed | 538 passed (540), 1 failed | 23 passed (24), both
AssertionError: expected 'customers' to be 'wh_customers', exactly those two cases.
Prediction matched exactly.

B — revert the sharingModel emission only (definition key + the six rendered lines).
Predicted: exactly 4 failures — the three defect 2 cases and the importObject case;
totals 4 failed | 536 passed (540). Predicted not to redden: every
ObjectSchema.safeParse(...).success assertion, because sharingModel is optional in
ObjectSchema and security-owd-unset is a lint rule, not a schema refusal — which is
precisely why reproducing this half needed a lint-level instrument at all.
Observed: 4 failed | 536 passed (540), 1 failed | 23 passed (24), three
expected undefined to be 'private' plus the source toContain failure, exactly those
four cases, every safeParse assertion green. Prediction matched exactly.

Restores proved byte-identical by git hash-object:
60cc5dfde720df630e9db485be1fd16aa7aa8134 before → mutated
40893e6d1334d601888629ccf261fb576778400f (A) / a6e8e0327c6a83dad420146a8a6879645a4b741a (B)
60cc5dfde720df630e9db485be1fd16aa7aa8134 after, both times. Both restore legs were
re-run to a real verdict
rather than trusted on the hash: 540 passed (540) each.

src/ vs dist/, argued from the files — and re-verified, not inherited

Two conflicting claims existed: #11001's seat measured service-datasource as having
no dist/, while the card said the defect was "confirmed in the shipped dist".
Re-verified here, and both readings need retiring as stated:

The argument that survives: the pin imports its subject by the relative specifier
../external-datasource-service.js, which cannot route through package exports at
all — vitest resolves it to src/external-datasource-service.ts. That is proved
empirically, not just read: both ablations reddened with no rebuild between edit and
run
, which is only possible if the suite is executing src/.

The os build harness is the opposite case and is treated as such: it drives the
generator through the package's built dist/ (it must — it also needs @objectstack/lint,
which is not a dependency of this package). Every measurement through it is preceded by
a rebuild, and the fix was proved to have reached the artifact with
node scripts/ablation-dist-preflight.mjs @objectstack/service-datasource GENERATED_SHARING_MODEL
✓ marker present in 2 built files. The @objectstack/spec and @objectstack/lint
halves of that harness resolve through their own built dist/; neither is mutated.

Zero-hit counter-check — positive control run FIRST

The dispatch's stop-condition: the object NAME changes (customers
wh_customers), so a consumer depending on the unprefixed name would make this reading
wrong.

Positive control first, so the silence is readable: a sweep for generateObjectDraft
across both repos, excluding dist/ and node_modules, returned 13 files — and
surfaced a route surface not named in the card (packages/rest/src/external-datasource-routes.ts),
so the instrument is demonstrably reaching further than the assumption behind it.

Every one of those consumers, read individually, treats draft.name as opaque server
output
: both route handlers (service-datasource/admin-routes.ts,
rest/external-datasource-routes.ts) pass the whole draft through untouched; the CLI
(os datasource introspect) writes draft.source and never derives a path from the
name; objectui uses it verbatim as a storage key
(metaClient.save('object', draft.name, draft.definition) — both halves move together,
so #7378's register contract still holds) and as display text. The two tests that assert
a bare name (admin-routes.test.ts, objectui's external/api.test.ts) are stubs
vi.fn().mockResolvedValue(...) and a stubbed fetch — asserting transport, not
generator output; neither reaches the real generator. No consumer depends on the name
being unprefixed.

Gates

Union derived on the final commit with a clean tree via
node scripts/pm/dispatch-gates.mjs, no path arguments (exit 0; 11 path-matched
families + 5 convention-triggered). Every exit code was captured before any pipe
(cmd > file 2>&1; EXIT=$?), and each row below quotes the gate's own printed
verdict line, never a bare $?.

gateverdict line
check:changeset-gate-self-tests✓ check-empty-changeset --self-test: 118 assertions over real temp git repos
check:objectui-changeset✓ objectui-range --self-test: all checks passed
check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scanned
check:type-source-resolutioncheck-type-source-resolution OK — 77 packages with a tsconfig.json scanned
check-adr-0087-registration.mjs✓ … this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
check-changeset-no-major.mjs✓ This diff introduces no major bump.
check-ci-filter-parity.mjsOK: all 83 declared cross-package glob(s) (72 unique) are covered
check-empty-changeset.mjs✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
check-plugin-teardown-shape.mjs✓ check:plugin-teardown-shape: 61 Plugin implementation(s) across 4444 source(s)
check-affected-docs.mjs✓ affected-docs self-test: 339 cases pass.
check:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) … none new
check:type-check-coveragecheck-type-check-coverage: OK — 65/78 workspace packages type-checked
check:engine-double-contractcheck-engine-double-contract: OK — 377 pinned, 133 in the DEBT ledger, 2 exempt.
check:where-matcher✓ where-matcher conformance holds: 276 matcher(s) discovered
check:nul-bytescheck-nul-bytes: OK (scanned 6391 text file(s) … no raw ASCII control bytes)
check:type-check-debt --re-measureOK — 33 ledger entr(ies) re-measured in 250.0s, 1908 raw tsc error(s) total, none above its recorded number

Two notes on that table, both about not reading a green as more than it is:

  • check:type-check-debt --re-measure first REFUSED--re-measure cannot run: 37 workspace dependenc(ies) … have no built type entry point on disk. A refusal is NOT
    MEASURED, never a pass, so the closure was built exactly as lint.yml does
    (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful)
    and the gate re-run to the real verdict quoted above. It reports a pre-existing
    -12 surplus on @objectstack/plugin-auth; that package is untouched here.
  • check:nul-bytes is not in the derived union. Class [finding] Every PM dispatch list is short by the same ~5 changeset-triggered gate families — they are path-derivable, but the changeset does not exist yet when the list is derived #10309 — the derivation has
    been short on every services PR measured today — so it was added on judgement (any
    edit can carry a control byte) and run explicitly. It is the only family added beyond
    the derived set; nothing else the derivation named was skipped.

Declared narrowings, on the record rather than implied:

  1. Only the SQLite arm of SqlDriver was executed (better-sqlite3, in-memory) in
    the reproduction. Postgres and MySQL were not run — no server reachable. This is a
    weaker limitation here than it was for External object-draft drops the introspected primary key (isPrimary vs primaryKey seam) #10676: neither defect reads dialect-specific
    introspection output at all. The object name derives from the remote table name and
    the OWD is a constant, so no dialect can change either verdict.
  2. packages/create-objectstack/bin/create-objectstack.js appears in the derived change
    set: pnpm install mode-flipped it (100644 → 100755) in this worktree. Not my edit,
    deliberately left unstaged and out of the commit; git diff --stat confirms
    0 insertions, 0 deletions. It only widened the derived gate set (packages/**
    families), never narrowed it.
  3. The os build acceptance was driven through the harness described above rather than
    by spawning the real os build binary against a scaffolded project. The harness runs
    the same three stages the command runs — defineStack(), authoringRulesFor('build')
    (41 rules, the set os compile runs, reached the same way
    packages/cli/src/utils/scaffold-validate.ts reaches it), and tsc over the rendered
    source — but it is a reconstruction of that pipeline, not the binary.

Not addressed here

#11000 remains open, as covered above. Also untouched and out of scope: #10997
(SQLite composite-PK truncation in the driver) and #10998 (introspectSchema omits
the contract's dialect / introspectedAt) — two further findings from the same
introspection seam, neither of which this change bears on.

Generated by Claude Code

…epts
`generateObjectDraft` rendered a `*.object.ts` the platform's own validator
refuses, on two independent counts: the object name carried no `${namespace}_`
prefix (`defineStack()` refuses it, ADR-0028) and no `sharingModel` was emitted
(`security-owd-unset` refuses it, ADR-0090 D1).
The namespace is derived from the datasource's own owning package and applied
through `validateObjectNamespacePrefix`, the single source of that rule. The
OWD follows the shape #9666 settled for generated scaffolds: an explicit
`'private'`, rendered with the reason attached. An unresolvable namespace keeps
the bare name plus a loud TODO rather than inventing a prefix.
Fixes#10712
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

This PR changes 1 package(s): @objectstack/service-datasource, touching 12 documentable anchor(s).

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

  • content/docs/api/declarative-endpoints.mdx(via manifest.namespace (literal))
  • 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 manifest.namespace (literal), sys_metadata (literal))
  • content/docs/deployment/validating-metadata.mdx(via sys_metadata (literal))
  • content/docs/getting-started/quick-reference.mdx(via manifest.namespace (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))
  • content/docs/protocol/kernel/http-protocol.mdx(via manifest.namespace (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
  • 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 072d072d2a74e358e923f6f6a76acbf7621305c3packageMentionDocs.

Which tree this was computed on

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

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

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

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

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

    ✗ Build failed in 5.65s
    

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

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

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

历史信号:

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

分诊清单:

  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

Development

Successfully merging this pull request may close these issues.

[finding] External object-draft output fails os build as generated — object name lacks the ${namespace}_ prefix and no sharingModel is emitted

2 participants

@os-warren@claude