Repository navigation
fix(console): mint a sys_file id for files picked in the global action dialogs - #10165
Conversation
The console's UploadProvider was mounted inside AppContent — the element of a single route (`/apps/:appName/*`). ConnectedShell's GlobalActionRuntimeProvider renders the action-param dialog and the ModalForm a modal action opens as SIBLINGS of the route element, so those dialogs rendered outside the provider. useUpload() fails open: with no provider above it, it silently returns createObjectUrlAdapter(), which mints a `blob:` URL locally and reports success. A file picked in one of those dialogs produced zero upload requests, fileValueForSubmit stored the legacy inline blob instead of a sys_file reference, and the engine rejected the record with "expected string, received object" — an error pointing at serialisation while the upload is what is missing. Mount it above ConsoleShell instead, so every console surface shares one upload destination, and build it with createAuthenticatedFetch so the storage routes see the session rather than a bare cookie-only fetch. Detail on objectui#10131. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
✅ 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
|
ACCEPT — PR #10165, card objectui#10131Reviewed by the ⭐ Outcome 1 — and the missing piece was a provider's ALTITUDE, not a missing clientThe dispatch offered three outcomes and insisted the side be established before anything was written. It was, and the answer is better than either half the card guessed at. Everything the card said was missing already exists: The defect is where one provider was mounted. ⇒ ⭐ and ⭐ The card's own framing — 「the two halves are both broken and they mask each other」 — turns out to be exactly right, and they mask each other because they are one cause. Re-derived by this seat on
|
2 participants
Fixes #10131
Clause-②: no— no exported symbol is added, removed or renamed, no accept set moves, and no key on a published payload changes. This package's published barrel isplugin.ts(ConsolePlugin,staticPath), which this diff does not touch; theAppandAppContentexports of the SPA source keep their names and types. What moves is WHERE a provider is mounted inside the console's own React tree. (The claim comment deliberately carried noClause-②:line — it is declared here, from the diff.)Step 1, answered: the mint path exists, and the widget already calls it
FileFieldin@object-ui/fields(with its grid-cell siblingFileCell), reached fromfield-type-alias'sfileentry. It is not thefile-uploadregistry component in@object-ui/components, which is a designer demo widget.sys_file:createObjectStackUploadAdapterin@object-ui/providersruns the server's three-step protocol (presign, PUT, complete) and returns the minted id asmeta.fileId.@objectstack/service-storagemountsPOST /api/v1/storage/upload/presignedandPOST /api/v1/storage/upload/complete, and its route ledger names them as the SDK upload path. Outcome 2 is ruled out.FileFieldalready calls that path throughuseUpload(), andfileValueForSubmitalready stores the bare id when the adapter surfaced one. Measured on a pre-fix tree, driving the real widget: with a provider whose adapter mints an id,onChangereceives the stringf0e1d2c3b4a5968778695a4b3c2d1e0f; with no provider, zero fetches andonChangereceives{name, original_name, size, mime_type, url: "blob:…"}.So neither half of the card was a missing client. This is outcome 1, and the thing that was not wired up is the provider's ALTITUDE.
The defect
ConnectedShell'sGlobalActionRuntimeProviderrenders{children}and then{runtime.dialogs}+{modalElement}as its siblings. The action-param dialog and theModalForma modal action opens — a lookup's inline "create the referenced record", a related list's New — are therefore mounted beside the route element, not under it.UploadProviderwas mounted insideAppContent, which is the element of exactly one route,/apps/:appName/*. Every one of those dialogs rendered outside it.And
useUpload()fails open: with no provider above it, it silently returnscreateObjectUrlAdapter(), which mints ablob:URL locally and reports success. That is both halves of the card in one mount point:filechooserproduced zero requests on the wire;meta.fileId,fileValueForSubmitfalls back to the legacy inline blob, and the engine answersexpected string, received object.The 400 names the payload while the upload is what is missing, exactly as the card says.
The lit control
A lookup in that same dialog round-trips its opaque server-minted id, and the difference is not the widget layer — it is one provider's altitude. The
dataSourcea lookup needs was deliberately hoisted to that level;GlobalActionRuntimeProvider's own docstring says it sits there "so it reaches EVERY field widget, including a relation field inside a create/edit form that renders in a Radix Dialog portal (which sits above the lower per-view ActionProviders)".SchemaRendererProviderand the action runtime got that treatment.UploadProviderdid not.The second half of the contrast is the failure mode. When a lookup has no data source it renders an explicit empty state —
OptionsEmptyStatehas a no-provider case of its own.useUpload()has no such state: it fabricates an adapter that reports success. That is why this stayed invisible until a record was saved.The fix
One
UploadProvider, mounted inAppaboveConsoleShell, so every console surface shares one upload destination — the app routes, the global dialogs, the two form routes and the organization settings page alike.AppContent's copy is removed, so there is exactly one in the tree.It is built with
createAuthenticatedFetch. That is not decoration:RecordAttachmentsPanel— the one console surface that was already uploading correctly — states the requirement, that "the storage routes require a session and there is no cookie forcredentials: 'include'to carry", andAppContent's own/me/permissionscomment records the same session shape ("with the cookie-only default fetch, a token-only session resolved as anonymous"). The same wrapper also carriesX-Tenant-ID, so an upload lands in the tenant the rest of the session is reading. Shipping the hoist with the adapter's bare defaultfetchwould have moved the failure from "no request" to "401", which reads like progress and is not.In-flight and failure behaviour — decided, and pinned as the decision
The card asks for this to be decided rather than inherited, so: the control holds the field value unchanged until the upload settles, and on failure never writes to it at all.
useFileUploadscallsonChangeonly after the adapter resolves, anduseUploadingSignalreports the transition to the host. So the value never holds a half-uploaded thing, and a host that must block Submit can —ActionParamDialogalready does.onChangeis not called, an inline error is rendered, and the field keeps what it had. For a new record with a requiredfilethat is empty, so the form's own required validation refuses the submit.This is the choice because it is the only one that avoids both hazards the card names: it never submits a stale id (the value is not touched) and it never submits nothing silently (there is an on-screen error, and required validation still refuses).
Reverse verification
The fix was committed first, then both source files were restored to the branch base and the pin re-run. The mutation is proved on disk rather than by an exit code:
UploadProvideroccurrences inApp.tsxwent 3 to 0 and inAppContent.tsx0 to 3, and both blob hashes differed from their HEAD blobs.The two that stayed green are the counter-probes, and that is the expected direction: the defect-signature probe (no provider anywhere) and the no-file-field control must not flip, or the red would be about the harness rather than the altitude. Restore was verified by blob-hash equality against HEAD and an empty
git diff HEAD, not by the restore command's exit code.Checks
All from the repo root, on the merged tree.
vitest run --project @object-ui/console(under the shared verify lock)VERDICT command-exit 0turbo run type-check --filter=@object-ui/console(under the lock)--listFilesconfirms the new pin file is inside the program, so the typecheck really covers itpnpm --filter @object-ui/console lintcheck:eager-closure(after a real@object-ui/consolebuild)AppContentcheck:control-bytes,check:new-line-citations,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:test-path-roots,check:changeset-claims,check:pending-changeset-literals,check-changeset-presence,check-changeset-no-majorcheck-governed-queue-guard --teston the four changed pathsNOT GOVERNED— ordinary review and merge-queue routeAcceptance notes
Found while establishing the above. None is in this diff.
To file (reproducible defects):
onUploadingChangehas exactly one consumer,ActionParamDialog. The record-form chain in@object-ui/plugin-formdoes not read it, so a user who picks a file and hits Save immediately saves the record without the attachment. Dedupe words: onUploadingChange, ObjectForm, submit, in-flight, upload. Not touched here because those files were declared hands-off for this round.filefield on a FormView renders as a plain text box. The/f/:slugand/forms/:nameroutes go throughFormPage, which is this repo's second form renderer and has its own hand-rolled input switch with nofilearm, so a file field falls through to the default text input. Same shape as the lookup sibling that landed as PR objectui#10158, on the other renderer. Dedupe words: FormPage, FormView, file, text input, second renderer.AppContent's docstring points hosts that do not want the console's system routes atDefaultAppContent, and neither it norConnectedShellmounts anUploadProvider— so such a host gets the same silent object-URL fallback in the same dialogs. Whether app-shell should carry a default upload destination is a contract question, not a repair. Dedupe words: DefaultAppContent, UploadProvider, third-party host, ConnectedShell, upload destination.Noted, not filed:
useUpload's fail-open fallback is what made all of the above silent — a deployment with no upload destination is reported to the widget as a successful upload. Making it observable is a contract change to a published hook with existing callers (ProfilePage, the organizationSettingsPage), so it belongs to whoever owns that contract rather than to this card. Successor: the third-party-host card above is the one that has to answer it.Generated by Claude Code