Skip to content

fix(rest): POST /datasources/:name/external/validate does URL-scoped work - #10960

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10537-per-datasource-validate-scope
Aug 22, 2026
Merged

fix(rest): POST /datasources/:name/external/validate does URL-scoped work#10960
os-elon merged 1 commit into
mainfrom
claude/issue-10537-per-datasource-validate-scope

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10537

POST /api/v1/datasources/:name/external/validate asked the external-datasource
service for validateAll() — every federated object on every federated datasource,
each validation driving a live introspect(datasource) remote-schema read — and then
kept only the rows whose datasource matched the URL. The rows were correct; the
work was not scoped.

The fan-out, re-derived at the branch point

Measured through the real Hono adapter and the real ExternalDatasourceService over a
recording introspector, on a fixture with three federated datasources
(external-datasource-validate-scope.test.ts, run before the production change):

AssertionError: expected [ 'wh_a', 'wh_b', 'wh_c' ] to deeply equal [ 'wh_a' ]
❯ src/external-datasource-validate-scope.test.ts:181 POST /datasources/wh_a/external/validate
AssertionError: expected [ 'wh_a', 'wh_b', 'wh_c' ] to deeply equal []
❯ src/external-datasource-validate-scope.test.ts:226 POST /datasources/no_such_ds/external/validate

Three remotes dialled for a one-datasource request; three remotes dialled to answer the
empty report an unknown name already answered. Same run, Tests 3 failed | 3 passed
the three that passed are the body-shape cases, which is the point below.

What changed

ExternalDatasourceService gains validateDatasource(datasource): the scoped twin of
the sweep, composed from the same primitives (listObjects → filter → validateObject),
the same federation predicate — now spelled once and shared with validateAll() — and
the same per-object catch. The REST registrar calls it. The same request now dials
['wh_a']; the unknown name dials nothing.

Response unchanged: same rows, same diffs, same data.ok verdict, same 200, the
same 400 EXTERNAL_DATASOURCE_ERROR on a service refusal, the same 503 SERVICE_UNAVAILABLE when federation is not wired in, and an unknown :name still
answers an empty, vacuously ok report rather than a 404. The selection is keyed on
o.datasource ?? 'default', which is exactly the value validateObject reports back as
result.datasource — so "the rows the post-filter kept" and "the objects this selects"
are the same set. Every case pins that equivalence by computing the sweep-then-filter
answer live from a second service instance and deep-comparing, rather than against a
remembered body.

validateDatasource is deliberately not on IExternalDatasourceService: triage
authorized the composition, not a contract-surface expansion. The service is the only
registrant of the external-datasource slot, and the registrar probes for the method;
a wired service without it takes the same 503 arm the family already takes when the
service cannot serve a route — not a silent fallback to validateAll(), which would
leave the fan-out reachable on a path no test drives.

How the work — not the output — was proved to narrow

The output was already right, so a test comparing response bodies would pass on both
sides of this change. The load-bearing pins are the call record:

  • introspected — which datasources were dialled — asserted as an exact list, over a
    fixture with three federated datasources so the assertion can fail;
  • validateAll() asserted never called on the scoped path (vi.spyOn on the real
    service instance);
  • a dead sibling remote (wh_b unreachable) asserted never dialled for a wh_a
    request.

Service-side, validateDatasource is pinned the same way plus the equivalence: for each
of the three datasources, the scoped rows deep-equal the sweep-then-filter rows while the
sweep's own introspection count is asserted greater than one — otherwise the equivalence
line would be vacuous. The default-bound-external-object edge is pinned too: it is the
case where "bound to :name" and "federated" come apart, and it is why the scoped filter
is not simply datasource === name.

Reverse verification

Predicted before running: reverting only the route production change turns the three
call-record cases RED while the body-shape cases stay GREEN, and additionally reddens the
two fixtures whose fakes now carry only the scoped spelling — i.e. more diagnostics,
across three files, not one.

Leg 1 — git restore --source=origin/main -- packages/rest/src/external-datasource-routes.ts
(the fix was committed first, so the restore point is a commit, and the service helper and
fixtures stayed in place):

Test Files 3 failed (3)
Tests 8 failed | 25 passed (33)
AssertionError: expected [ 'wh_a', 'wh_b', 'wh_c' ] to deeply equal [ 'wh_a' ]
AssertionError: expected [ 'wh_a', 'wh_b', 'wh_c' ] to deeply equal []
AssertionError: validateDatasource: expected "vi.fn()" to be called at least once
AssertionError: expected "vi.fn()" to be called with arguments: [ 'pg_main' ]
AssertionError: expected 503 to be 200 (envelope conformance: the fake has only the scoped spelling)

Leg 2 — git checkout HEAD -- packages/rest/src/external-datasource-routes.ts
(git status --porcelain empty afterwards, so the restored bytes are the committed ones):

Test Files 3 passed (3)
Tests 33 passed (33)

No rebuild mediates either leg: the mutated file is the test's own package source,
imported relatively, and the one cross-package import
(@objectstack/service-datasource) is aliased to that package's src/ by
packages/rest/vitest.config.ts — an alias that predates this PR, for the reason its own
comment gives. @objectstack/service-datasource was rebuilt anyway before typechecking,
since packages/rest's tsc reads its dist/*.d.ts.

Verification

Dependency closure built first, then, at this PR's final commit 5163a56be:

@objectstack/service-datasource testsTest Files 22 passed (22) · Tests 519 passed (519)
@objectstack/rest testsTest Files 134 passed (134) · Tests 2179 passed (2179)
@objectstack/service-datasource typecheckexit 0
@objectstack/rest typecheckexit 0

Gate union derived at that commit with node scripts/pm/dispatch-gates.mjs (no paths
passed — the script derives the change set itself from the merge base). All 20 green,
each exit code captured before any pipe:

check:nul-bytes · check:changeset-gate-self-tests · check:cross-package-test-inputs
check:dispatcher-error-vocabulary · check:objectui-changeset · check:route-envelope
check:slot-lookup · check:test-source-alias · check:type-source-resolution
check-adr-0087-registration · check-changeset-no-major · check-ci-filter-parity
check-cross-package-test-inputs.mjs · check-empty-changeset · check-plugin-teardown-shape
check-affected-docs · check:query-options-erasure · check:type-check-coverage
check:engine-double-contract · check:where-matcher

The ratchet, in its own words — it first refused on an unbuilt closure
(--re-measure cannot run: 31 workspace dependencies ... have no built type entry point on disk), which is a NOT-MEASURED verdict rather than a pass, so the closure was built
exactly as lint.yml does and it was re-run:

✓ check:type-check-coverage --self-test — 47 semantic case(s) + 59 observation case(s)
+ 29 re-measure case(s) + 28 built-closure case(s) + 19 auto-lowering case(s) hold.
check-type-check-coverage: OK — 64/77 workspace packages type-checked (plus the root),
13 in the DEBT ledger (436 frozen raw errors), 1 exempt.
check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 193.1s,
1908 raw tsc error(s) total, none above its recorded number.

That one matters here rather than being ceremony: @objectstack/rest is a TEST_DEBT
ledger entry, so the new test file is exactly what it measures. No gate was weakened, no
threshold moved, no ledger or baseline edited.

Not in this PR

ExternalValidationPlugin.runDriftCheck(ctx, datasource) in packages/runtime carries
the same shape — validateAll() then a filter on r.datasource — and
validateObject re-introspects once per object rather than once per datasource. Both are
different packages / different consumers with their own verification surface, so they are
filed separately rather than ridden in here; neither is addressed by this PR. Issue
numbers are in the dev report on #10537.


Generated by Claude Code

)
The route called validateAll() — every federated object on every federated
datasource, each validation driving a live introspect(datasource) — and then
kept only the rows matching :name. The rows were right; the work was not
scoped. Measured at the branch point: a request for one of three federated
datasources introspected all three.
ExternalDatasourceService gains validateDatasource(datasource), the scoped twin
of the sweep composed from the same primitives (listObjects -> filter ->
validateObject) with the same per-object catch; the REST registrar probes for
it and answers 503 when the wired service has no scoped spelling, rather than
falling back to the fan-out. Response is unchanged, pinned against the
sweep-then-filter answer in both packages.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

9 anchor(s) derived from 2 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 — 13 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 926778bce01620ae56a9b0eecbf0400c43a64aa8packageMentionDocs.

Which tree this was computed on

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 6.21s
    

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

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

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

历史信号:

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

分诊清单:

  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/lteststooling

Projects

None yet

2 participants

@os-elon@claude