Uh oh!
There was an error while loading. Please reload this page.
fix(service-datasource): forward ?schema= on the admin remote-tables route (#7955) - #7998
Conversation
…s route (#7955) `IExternalDatasourceService.listRemoteTables` is reachable through two live routes. The federation spelling — `GET /api/v1/datasources/:name/external/tables` in `packages/rest` — forwards `?schema=` to the service. The admin spelling, `GET /api/v1/datasources/:name/remote-tables`, never read `req.query` at all, so `?schema=public` came back as the UNFILTERED listing: not the filtered set, and not a refusal either — the "declared ≠ enforced" shape at its quietest, since the twin one path over honoured the same parameter. This finishes on the REQUEST path what #4249 did for the failure path ("one operation, one failure contract now, on both paths"). The two routes resolve the SAME `external-datasource` slot and call the SAME method with the same datasource name; the sibling operation `generateObjectDraft` already forwards its options bag on both spellings, so honouring the filter restores symmetry rather than inventing policy. The coercion is copied from the federation route rather than reinvented, down to its treatment of a non-string: a repeated `?schema=a&schema=b` reaches a handler as an array (the adapter surfaces repeated keys that way) and both spellings drop it to `undefined` — no filter. No refusal, warning or deprecation is added: whether an unusable query parameter should be REFUSED is the global ingress-policy question #7606 owns, and honouring the parameter is correct under either answer that card reaches. The invariant is pinned where both registrars are reachable — `packages/rest/src/remote-tables-twin.equivalence.test.ts` drives the same query at BOTH spellings, against one real `ExternalDatasourceService` on one real `HonoHttpServer`, and compares the answers. A test that exercised only the fixed route could not fail if the twins drift apart again. It covers the filtered set, the ABSENT parameter (still unfiltered, so an always-filtering fix cannot pass), a filter that matches nothing, an empty `?schema=`, and the repeated-key array. `@objectstack/rest` gains two dev-only workspace dependencies for it; its published surface is unchanged. `datasource-route-ledger.ts` recorded the divergence as live ("only the federation twin forwards `?schema=`") — that note now records the reconciliation. Fixes#7955 Co-authored-by: Claude <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also reference the affected code. These are read-only:
|
huangyiirene
commented
Aug 12, 2026
PM 复核:补丁轮(⛔ 不是 REWORK)—— 两个门禁都红,同一个根因先说定性:这不是返工。#6644 L2 的契约就是「dev 在草稿 PR 时点即报,CI 收敛归 PM」,收敛期转红走补丁轮是那笔交换已经付过的价钱。分析本身是本轮最扎实的一份(前提实测、反向验证双向、镜像而非自创、 红点一 —— ESLint job 内的 |
…its fixture cast (#7955) Two repo-wide ratchets went red on the placement decision the PR flagged for review — the equivalence test living in `packages/rest`. Both are fixed where the gates prescribe; neither ledger is widened. `check-test-source-alias`: `remote-tables-twin.equivalence.test.ts` imports `@objectstack/service-datasource` and `@objectstack/plugin-hono-server` as VALUES, and unaliased both resolved through the workspace link to `dist/` — a build artifact. `packages/rest/vitest.config.ts` now aliases the two to their `src` entry points, anchored-regex array form so the prefix match cannot swallow a subpath (`@objectstack/service-datasource/contracts` → `…/src/index.ts/ contracts`, ENOTDIR). The hazard is specific here rather than generic. A cross-package equivalence pin exists to notice when one twin moves; resolved through a stale `service-datasource` dist it would report the PRE-fix admin route as agreeing with the federation route — the #7955 defect itself, passing green, with nothing in the output saying so. Re-ran the reverse-verification through the aliased (source) path to confirm the pin still bites: red 2/5 with `admin-routes.ts` at `origin/main`, green 5/5 with the fix — and now without a rebuild between them, because the verdict is about source. `check:type-check-debt --re-measure`: the test added one raw tsc error to `@objectstack/rest`'s hidden test layer (155 → 156). It was the fixture's `as IntrospectedSchema['tables']` assertion, which existed only because the column literals omitted the required `primaryKey`. Spelling the columns in full through a small `col()` helper removes the need for the cast. Measured back at 155 — the recorded number — so the ledger entry is untouched. New debt is not irreducible debt, and the ledger only shrinks (#5278). Behaviour, acceptance shape and the analysis are unchanged: both routes still driven by one test, the absent-parameter case retained, the federation coercion still mirrored, no refusal added, `external-datasource-routes.ts` untouched. Co-authored-by: Claude <noreply@anthropic.com>
huangyiirene
commented
Aug 12, 2026
补丁轮已推送: 红点一 — |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7955
What was wrong
IExternalDatasourceService.listRemoteTablesis reachable through two live routes:GET /api/v1/datasources/:name/external/tablespackages/restlistRemoteTables(name, { schema })GET /api/v1/datasources/:name/remote-tablespackages/services/service-datasourcelistRemoteTables(name)— query never readSo
?schema=publicon the admin spelling returned the unfiltered listing: not the filtered set, and not a refusal either — "declared ≠ enforced" at its quietest, since the twin one path over honoured the same parameter.The premise, measured
The card's ruling rests on the twins being genuinely the same operation. They are:
ctx.getService('external-datasource')(admin-routes.ts:201,external-datasource-routes.ts:81).listRemoteTables(req.params.name, …).external-datasource(Three datasource-admin routes answer 503 naming the wrong service (carried out of #3973 so it is not buried) #4225/The same three datasource-admin routes answer 400 with the wrong service's error code (#4225 follow-up, one field over) #4249), i.e. the codebase already treats it as that service's route, not a differently-scoped admin listing.datasource-route-ledger.tsstates it in prose and cites this issue.No scope difference, no second service method. Route A applies as ruled.
The change
packages/services/service-datasource/src/admin-routes.ts— the one behavioural edit:Wire-visible:
?schema=<name>now narrows the listing on the admin spelling, exactly as the federation twin already did. No?schema=is unchanged — still the full listing.Validation is mirrored, not invented. The coercion is copied verbatim from
packages/rest/src/external-datasource-routes.ts:122— the reference spelling, which this PR does not touch. It is a coercion, not a refusal: a non-string (a repeated?schema=a&schema=b, which the adapter surfaces as an array) degrades toundefined, i.e. no filter, on both paths. The empty string?schema=stays a string and the service's ownopts?.schema &&guard treats it as no filter — again on both paths, from the one service.No refusal was added. Whether an unusable query parameter should be refused is the global ingress-policy question owned by #7606; honouring the parameter is correct under either answer, so the twins can move together then.
Tests
packages/rest/src/remote-tables-twin.equivalence.test.ts(new) drives the same query at both spellings — one realExternalDatasourceServiceover a fake introspector, both registrars mounted on one realHonoHttpServer— and asserts the two answers are equal:?schema=public→ same filtered set (and it really filtered: 2 of 3 tables).?schema=→ same unfiltered set (an always-filtering fix could not pass).?schema=nonexistent→ same empty set.?schema=a&schema=b→ both degrade to no filter, identically.?schema=(empty) → both unfiltered, identically.It lives in
packages/restbecause that is the side that can reach both halves without widening anyone's public API:registerDatasourceAdminRoutesis exported from@objectstack/service-datasource, whileregisterExternalDatasourceRoutesis deliberately internal topackages/rest. The edge is dev-only (rest → service-datasource) and not a cycle.packages/services/service-datasource/src/__tests__/admin-routes.test.tsgains the same-package forwarding case and its existing assertion now pins the options bag.Reverse-verification. With
admin-routes.tsreverted toorigin/mainand the package rebuilt, the equivalence test is red: 2 failed / 3 passed — cases 1 and 3 fail with the admin route returning all three tables where the federation route returned the filtered set / none. With the fix: 5 passed. The three that pass in both directions are the unfiltered arms, which is what makes case 2 a real guard rather than a restatement.Full suites green:
@objectstack/rest1536 tests / 95 files,@objectstack/service-datasource334 tests / 13 files. Typecheck and eslint clean on both.check:route-envelope,check:cross-package-test-inputs,check:published-files,check:empty-changeset,check:adr-0087-registrationall pass.Deliberately not done
packages/rest/src/external-datasource-routes.tsis untouched — it is the reference spelling being converged onto.?schema=(that is [rest] Unknown query parameters are silently dropped on every REST route except/approvals/requests— decide whether the closed-parameter-set rule becomes ingress policy #7606's global call).packages/specedit, so nogen:schema/gen:docsregeneration is due..changeset/remote-tables-twins-forward-schema.md(@objectstack/service-datasourcepatch).@objectstack/restgets no release entry — only its devDependencies changed, so its published artifact is identical.Generated by Claude Code