Uh oh!
There was an error while loading. Please reload this page.
fix(service-datasource): ledger the mounted datasource-admin routes (#7744) - #7957
Conversation
…7744) The admin CRUD family mounted at `/api/v1/datasources` had no route-ledger entry anywhere. The three ledgers that came before it each stop at their own package boundary and this surface is outside all of them: `objectstack serve` builds a small plugin that resolves `http.server` and calls `registerDatasourceAdminRoutes` straight on `IHttpServer`, so neither `RouteManager` nor `RestServer.getRoutes()` has ever seen these ten routes — the same third surface `service-storage` and `service-i18n` grew their own ledgers for. The REST ledger's five `datasources` rows are the FEDERATION family, spelled `/:name/external/…`. That is a different mounted route family in a different package, not the same one misspelled: `GET /:name/remote-tables` and `GET /:name/external/tables` are both live, and their overlap was deliberately reconciled rather than removed (one failure contract on both paths). So the ledger is written at the spelling the mount uses, and no live route is renamed. Adds `datasource-route-ledger.ts` (ten reviewed `server-only` rows — no client method and no CLI command reaches any of them) plus a conformance guard that DERIVES its expectations from the registrar: the mount is captured against a recording `IHttpServer` and compared to the ledger in both directions, with a non-vacuity assertion so the guard cannot pass by enumerating nothing. An eleventh route now fails the test instead of silently re-creating this gap. The client half joins the existing tranche-3 guard in `service-route-ledger-coverage.test.ts`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NzFcriQJsJaaiieWSsrMu7
…t surfaced (#7744) #7954 — the family has no SDK expression; the ten rows are recorded `server-only` rather than `gap` because promoting them is a product decision, not a bookkeeping one. #7955 — the two `listRemoteTables` twins diverge on `?schema=`: only the federation spelling forwards it. Out of scope here (a live route's request handling), so it is filed rather than changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NzFcriQJsJaaiieWSsrMu7
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckNo hand-written docs reference the 1 changed package(s). ✅ |
hotlong
commented
Aug 12, 2026
PM review — The landing-site deviation is correct, and the dispatch was one of the things that was wrongThis PR does not put the entry in
The binding requirement — a ledger entry at the live spelling, pinned by a mount-derived test — is met exactly; only the file differs, and it differs toward the shape the repo already grew for this class ( Two anti-vacuity moves worth calling out, because neither was asked for
(a) is falsified, and that is the success outcomeAll three conditions fail, each cited, with a control on both greps as required. Condition 1 is dispositive and settles it on its own: late binding is not merely possible, it is the written contract — Condition 3 is nearly as strong: Condition 2 is honestly reported as the weaker one (both pinning tests drive a known driver, so an unknown-driver-only refusal would not by itself turn them red; what they pin is the accept-and-return-summary shape). Saying that plainly rather than rounding it up to a third confirmation is what makes the other two believable. And ⛔ no consolation change was made — the error message was not "improved". That instruction was followed to the letter. So the card's second half is not a bug. Also endorsedThe 25/25 checks green, Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7744
The card had two halves, dispatched on different terms. (b) landed. (a) is falsified and nothing was changed for it — the evidence is below.
(b) Route-ledger parity — landed
What was actually wrong
The ten admin CRUD routes under
/api/v1/datasourceshad no route-ledger entry in any of the platform's ledgers, and no guard was comparing them to anything. They are mounted the "third way" thatservice-storageandservice-i18ngrew their own ledgers for (#3636):objectstack servebuilds a smallcom.objectstack.cli.datasource-admin-routesplugin whoseinit()resolveshttp.serverand callsregisterDatasourceAdminRoutesstraight onIHttpServer(packages/cli/src/commands/serve.ts). NeitherRouteManagernorRestServer.getRoutes()has ever seen them.Why the entry is NOT in
packages/rest/src/rest-route-ledger.tsThe card and the dispatch both name
packages/restas the site. That file structurally cannot host these rows, and the guard next to it says so in both directions:rest-route-ledger.conformance.test.tsenumerates one thing — aRestServerbooted the way production boots it, its ownregisterRoutes()plusmountAndRecordDirectRoutes(rest 的 9 条 direct-mount 路由对RestServer不可枚举 —— 因此进不了/openapi.json,也进不了任何运行时自省 #5822).registerDatasourceAdminRoutesis not one of the registrars that function composes (direct-mount-composition.tscallsregisterPackageRoutesandregisterExternalDatasourceRoutes, and nothing else).direct-mountrow added there would fail two assertions immediately: "every direct-mount ledger entry is really registered by its registrar", and thedirectMounted.length === ledgerKeys('direct-mount').sizeidentity.The binding requirement from the dispatch — a ledger entry at the live spelling, pinned by a mount-derived test — is met; only the file it lives in differs, and it differs to the shape the repo already uses for exactly this class of route. New:
packages/services/service-datasource/src/datasource-route-ledger.ts+ its conformance guard, mirroringstorage-route-ledger.ts/i18n-route-ledger.ts.The spelling, and what was NOT done
⛔ No live route is renamed. The five
datasourcesrows the REST ledger carries are the federation family (/:name/external/…) — a different, separately mounted family in a different package, not the admin family misspelled. Both are live. Two pairs do overlap (remote-tables↔external/tablesoverlistRemoteTables;object-draft↔external/tables/:remote/draftovergenerateObjectDraft), and that overlap is known and was deliberately reconciled rather than removed — #4249 gave the two paths one failure contract ("One operation, one failure contract now, on both paths",external-datasource-routes.ts). Every row here carries the spelling the mount uses.The pin is derived from the mount, not transcribed
datasource-route-ledger.conformance.test.tsruns the registrar against a capturingIHttpServerand treats its registration calls as the route set. No path literal in the test is compared against the mount, so a future route addition fails the test instead of quietly re-creating this card. It also asserts non-vacuity (live.size > 0andlive.size === ledger.size), because both set-difference checks would pass on an empty enumeration — "absence must be loud".Reverse-verified. Deleting the new
GET /api/v1/datasources/:name/remote-tablesrow (fix committed first, restored withgit checkout BRANCH -- PATH— nevergit stash) fails 3 assertions for the right reasons:Restored → 7/7 green.
Dispositions
All ten rows are
server-only, each with a note naming its real consumer. Audited: noObjectStackClientmethod and no CLI command reaches any of them (the client'sdatasourcesnamespace holds onlyexternal, which targets the federation family; the three CLIdatasourcecommands all call/external/*). Writing tengaprows instead would have been a product decision made inside a bookkeeping fix — filed as #7954 for review instead. The client half joins the existing tranche-3 guard inservice-route-ledger-coverage.test.ts.(a) Unknown-driver draft → 201 with
status:'error'— NOT implemented; premise falsifiedAll three conditions were required. All three fail. Per the dispatch this is the success outcome for (a): it converts the card's second half from a bug into a documented design decision.
Condition 1 — "no late binding" — FAILS. This one is dispositive.
Late binding is not merely possible here; it is the written contract of the seam, and the create path's behaviour under it is specified.
packages/services/service-datasource/src/contracts/datasource-driver-factory.ts:7-18:That last clause is the accept-then-report design, stated at the contract. Three more reads carry it:
datasource-connection-service.ts:137— the factory field is a thunk, documented "Resolve the host driver factory (lazy — may be registered after init)."datasource-admin-plugin.ts:230-239— the factory is resolved per call, with the reason given inline:init()may run before other plugins register their services, and admin requests arrive long after boot.datasource-admin-plugin.ts:381-391, 428-455—start()runsrestoreRuntimeDatasources()thenrehydratePools(), so a persisted record's driver is re-resolved on every boot. A driver contributed by a plugin (or an optional peer package such as@objectstack/driver-turso) installed after the draft exists binds then. Refusing at create time would make that authoring state unreachable.And the create path already states the boundary deliberately —
datasource-admin-service.ts:360-361: "A driver the platform ships no contract for passes untouched, matching the spec gate's boundary rather than inventing a stricter one for the UI."Condition 2 — "nothing pins the 201" — FAILS.
service-datasource/src/__tests__/admin-routes.test.ts:139-151—it('POST /api/v1/datasources creates a runtime datasource (201)'),expect(res.status).toBe(201).service-datasource/src/__tests__/envelope.conformance.test.ts:113-116—POST /datasources (201)as a declared envelope-conformance case.Stated precisely: both drive
driver: 'postgres', a known id, so an unknown-driver-only refusal would not by itself turn them red. What they pin is the create route's accept-and-return-the-summary shape, which is the shape the refusal would fork.Control for this grep: the same query shape returns hits where a status code really is pinned —
packages/types/src/response-envelope.test.ts:60,packages/rest/src/public-form-routes.test.ts:124,rest-dropped-fields.test.ts:107and others — so the hits above are a real reading, not a lucky pattern.Condition 3 — "
status:'error'is not a consumed design state" — FAILS, and it reads the unknown-driver case by name.The unknown-driver path is a named state with its own vocabulary entry, mapped through a declared class into a declared contract field, carrying a machine-readable reason:
factory.supports(driver) === false→ConnectStatus 'skipped-unsupported'(datasource-connection-service.ts:154, 532-537, whose reason string isno driver factory supports driver 'NAME') →availabilityOf→'failed'(:202-205) →summaryStatus→status: 'error'+statusReason(datasource-admin-service.ts:109-125, 401-419).The vocabulary doc spells out that this is intended —
datasource-connection-service.ts:174-176:Consumed and tested:
contracts/datasource-admin-service.ts:92declaresstatus: 'ok' | 'error' | 'blocked' | 'unvalidated'onDatasourceSummary;__tests__/datasource-admin-service.test.ts:412-425pins each availability class onto a distinguishable status (expect(byName.dead!.status).toBe('error'));packages/runtime/src/datasource-autoconnect.test.ts:463pins the sibling'unvalidated'. The whole field exists because of #3827 — before it, "a datasource that died at boot looked exactly like one nobody had tested."Control for this grep: querying the same vocabulary for the sibling value
unvalidatedreturns hits acrosspackages/runtime,packages/objectqlandpackages/spec, so a zero-hit result for'error'would have meant absence rather than a broken search — and it was not zero.Consequence
No code changed for (a), and per the dispatch no consolation change was made — the error message was not "improved". A
status:'error'carryingstatusReason: no driver factory supports driver 'NAME'is health-reporting working as designed, on a create path that is documented to accept a draft whose driver is not buildable in this process.Verification
pnpm --filter @objectstack/service-datasource typecheck— clean.pnpm --filter @objectstack/service-datasource test— 13 files / 333 tests pass (7 of them new).service-route-ledger-coverage,rest-route-ledger-coverage,route-ledger-response-schema— 11 pass.check:type-check-debtdoes not rise.packages/client'scheck:test-typecheckreports 3 problems locally, allTS2307: Cannot find module '@objectstack/runtime'in files this PR never touches; reverting only this PR's client edit reproduces the identical 3, so it is the unbuilt-local-dependency artifact of AGENTS.md §9, not a ratchet movement. Neither touched package carries a DEBT/TEST_DEBT entry, and the new test file is insidetsc --noEmit's program (no*.test.tsexclusion) with zero errors. Confirmed on CI: the requiredTypeScript Type Checkjob is green..changeset/eighty-hoops-repeat.md(patch). Not breaking, so no ADR-0087 marker is owed.Out-of-scope findings filed
server-onlyvsgap#7954 — the family has no SDK expression at all;server-onlyvsgapis a product call.listRemoteTablesroute twins diverge on?schema=— the admin spelling drops it #7955 — the twolistRemoteTablestwins diverge on?schema=: only the federation spelling forwards it, so the admin spelling silently drops the filter. Changing a live route's request handling was out of scope here.Generated by Claude Code