fix(app-shell): the External Datasource panel unwraps the { success, data } envelope its routes answer (objectui#11628) - #11640
Merged
objectstack-fleet[bot] merged 3 commits intoOct 5, 2026
Conversation
… data }` envelope its routes answer (objectui#11628)
The `/datasources/:name/external/*` client returned the whole body and read
`tables` / `draft` / `catalog` / `{ ok, results }` off its top level, where
the server never puts them: every route there answers through the shared
`sendOk`. The panel listed no remote tables, showed no catalog timestamp,
crashed Validation on `results.length`, and left the import dialog
unreachable. `readExternalData` now unwraps `data` once for the four routes
and refuses a 2xx body that is not the envelope instead of reading it as
empty.
The same reader's failure arm ran `String(body.error)` over the ADR-0112
nested envelope, so every refusal read `[object Object]` and the 503
`SERVICE_UNAVAILABLE` hint never fired. Refusals now go through app-shell's
`readEnvelopeFailureText`. The `/meta` save door the import uses keeps its
own reader: it answers a nested capability refusal and a flat validation
refusal, and both now reach the user.
`api.test.ts` moves to the measured wire shapes, so it fails on the old
client.
Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL
Co-authored-by: Claude <noreply@anthropic.com>
…e read (objectui#11628) Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11628
Clause-②: no
What changed
packages/app-shell/src/views/metadata-admin/external/api.tsread the External Datasource routes as if they answered bare payloads. Every/datasources/:name/external/*route answers through the sharedsendOk, so the payload sits underdata. The client readtables,draft,catalogand{ ok, results }from the top of the body. As a result the panel listed no remote tables (tables ?? []turned the miss into an empty list), showed no catalog timestamp, crashed Validation onresults.length, and the import dialog could not be reached.readExternalDataunwrapsdataonce, for all four/external/*calls. Each caller then reads its own payload key fromdata. A 2xx body without the{ success: true, data }envelope is now an error that names the route, instead of being read as an empty answer.String(body.error)on the ADR-0112 nested envelope, so every refusal read[object Object]. The 503 arm compared that same string withexternal_service_unavailable, a code the server no longer sends, so the "federation not enabled" hint could never show. Refusals now go through app-shell's existingreadEnvelopeFailureText. The 503 is now recognised by the envelope's owncode,SERVICE_UNAVAILABLE. This is the PM's mechanism assumption 2: measured below, it was the same envelope in the same function, so it is fixed in this PR.importObjectDraftkeeps its own reader, because it calls the/metadoor, not an/external/*route. That door's success body is the save result, with nodata. Its refusals come in two dialects, both measured: nested{ error: { code, message } }from the capability gate, and flat{ error: 'SENTENCE', code }from spec validation. The old reader handled only the flat one, and printed a non-admin's 403 as[object Object]. It now reads both.api.test.tsuses the measured wire shapes. It covers all five calls, the 503 arm on each of the four routes, and both/metarefusal dialects.SchemaBrowser,ExternalDatasourcePanel,ValidationPanel,ImportObjectDialog) render correctly as they are.Wire shapes, measured live (objectstack
mainat0a348031, stock showcase,--fresh)listRemoteTablesGET /external/tables{success:true,data:{tables:[customers,orders]}}{success:false,error:{code,message}}(401UNAUTHENTICATED, 403PERMISSION_DENIED)body.tables, missing, so[]generateObjectDraftPOST /external/tables/:remote/draft{success:true,data:{draft:{...}}}EXTERNAL_DATASOURCE_ERRORnestedbody.draft, soundefinedrefreshCatalogPOST /external/refresh-catalog{success:true,data:{catalog:{snapshotAt,...}}}EXTERNAL_DATASOURCE_ERRORnestedbody.catalog, soundefinedvalidateDatasourcePOST /external/validate{success:true,data:{ok:true,results:[2 rows]}}PERMISSION_DENIEDnested{ok,results}, soresultswasundefinedimportObjectDraftPUT /meta/object/:name{success:true,version,seq,state,message}(nodata, not read){error:{code:'FORBIDDEN',message}}nested; 422INVALID_METADATAand 400VALIDATION_ERRORflat{error:'SENTENCE',code}String(body.error): right for flat,[object Object]for nestedThe 503
SERVICE_UNAVAILABLEarm cannot be reached on the showcase, because it wires theexternal-datasourceservice. Its shape comes from the server'sunavailablewriter inregisterExternalDatasourceRoutes(sendError(res, 503, 'SERVICE_UNAVAILABLE', …)) and is covered by the unit test. Live: NOT MEASURED.Live panel, before and after (same backend, console from this worktree)
The console ran from this worktree under
vite, proxied to the backend, at/apps/setup/metadata/datasource/showcase_external. "Before" isapi.tstemporarily restored to the base1c2e2c4from the committed fix; restoring HEAD was confirmed by blob hash. Both runs blocked the dev-only/api/v1/dev/metadata-eventsstream; see Acceptance notes.1c2e2c4)customers7,orders7data.catalog.snapshotAt; no timestamp renderedsnapshot 10/5/2026, 2:51:36 AMrenderedPOST …/tables/customers/draft200; shows object namecustomersand the generated sourceCannot read properties of undefined (reading 'length')showcase_ext_customerandshowcase_ext_orderThe real
api.tswas also called inside the page against the same backend, as the admin and as the non-adminauditor.demo@example.compersona:generateObjectDraft(showcase_external, no_such_table)Error: [object Object]Remote table 'no_such_table' not found on datasource 'showcase_external'. (EXTERNAL_DATASOURCE_ERROR)refreshCatalog(no_such_ds)Error: [object Object][ObjectQL] Datasource 'no_such_ds' has no registered driver to introspect. (EXTERNAL_DATASOURCE_ERROR)listRemoteTables/validateDatasourceError: [object Object]Introspecting an external datasource requires the manage_platform_settings capability. (PERMISSION_DENIED)(the server wraps the capability name in backticks; dropped here)importObjectDraftError: [object Object]Saving a metadata item requires the manage_metadata capability. (FORBIDDEN)importObjectDraft(name mismatch, flat 400)PM mechanism assumptions
[object Object], and the 503 hint never fired. The fix is in this PR, in the same reader.readEnvelopeFailureText(packages/app-shell/src/utils/apiErrorEnvelope.ts, same package, no new export). For success bodies app-shell has no shared unwrap helper.@object-ui/data-objectstack'sunwrapDispatcherEnvelopeis module-private. It is also deliberately lenient (it returns the body as-is when there is nodata), and that is the tolerance this card's defect hid behind. Reusing it would need a new package export, which the Clause-② fence rules out. So the strict unwrap lives in this module.main0a348031, built withOS_SKIP_DTS=1under the verify lock. The shared checkout was not touched.Tests
All gates below ran on the merged head
4779053: this branch plus one merge oforigin/mainat22ddcd5. Each exit code was written to a file as it ran.pnpm exec vitest run packages/app-shell/src/views/metadata-admin/external/api.test.ts790b3b1), thenapi.tsrestored to base1c2e2c4:Tests 12 failed | 4 passed (16), exit 1. The failures wereexpected [] to deeply equal [customers, orders],Cannot read properties of undefined (reading 'name'),expected undefined to deeply equalthe catalog, the envelope returned as{ok,results},Error: [object Object]whereExternalServiceUnavailableErrorwas expected, and'[object Object]'in the message.git checkout HEAD -- api.tsput the fix back; the blob hash matched HEAD andgit diff HEADwas empty. Re-run:Tests 16 passed (16), exit 0.?schemapassthrough, the HTTP-status fallback, the/meta2xx, and the flat/metarefusal. The old client already handled each of these.pnpm exec vitest run packages/app-shell/ --maxWorkers=2(root form, under the verify lock):Test Files 1000 passed | 1 skipped (1001),Tests 9940 passed | 9 skipped (9949), exit 0.pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json; the second config includessrc/**/*.test.ts), afterturbo run build --filter='@object-ui/app-shell^...': exit 0.pnpm --filter @object-ui/app-shell lint: exit 0,0 errors. The warnings are pre-existing, and none is inexternal/api.tsorapi.test.ts.check:metadata-write-doorsreadsimportObjectDraftas a raw-PUT door and printsOK 17 metadata write door(s) derived (3 raw PUT, 14 SDK) … 3 reach assertObjectMetadataWritable.check:new-line-citationsprints0 new citation(s).check:control-bytes,check:test-path-roots,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:changeset-claims,check:pending-changeset-literals,check:self-import,check:esm-specifiers,check:shell-escape-residue,check:unreferenced-sources,check:phantom-deps,check:unused-deps,check:comment-mask-corpus,scripts/check-changeset-presence.mjsandscripts/check-changeset-no-major.mjs.node scripts/check-governed-queue-guard.mjs --teston the three paths reportsNOT GOVERNED.pnpm lintand the full test farm are left to CI.Acceptance notes
vitedev and the backend underobjectstack dev, "Refresh catalog" writes the catalog snapshot. The backend then emits ametadata-changeevent on/api/v1/dev/metadata-events, andapps/console'sMetadataHmrReloaderreloads the page about 0.6 s later. That wipes the panel state, timestamp included. The reloader is enabled byimport.meta.env.DEV, so by its own default the published console (/_console) does not run it (read from source; not measured on/_console). The live measurement blocked that stream to see the panel as the published console behaves. Owner: none.listRemoteTablesno longer applies?? []. With the envelope checked first, that fallback could only hide adatawithouttables, and the server always sendstables.PUT /meta/object/:name, not the server'sPOST /external/tables/:remote/import. service-datasource: importing an external table under a name that differs from its remoteName creates an object that answers 500 "no such table" — and the import does not survive a restart objectstack#21788 (the server-side twin) is about that POST route and stays open; this PR does not touch it.importObjectDraft: a 2xx no longer parses the body, because nothing reads it. A 2xx with an unparseable body used to reject.Generated by Claude Code