Repository navigation
fix(providers): the ObjectStack upload adapter never sends an upload path as the storage scope (objectui#12055) - #12058
Merged
Conversation
…path as the storage scope (objectui#12055) createObjectStackUploadAdapter sent `scope: opts.scope ?? options.path`, so an adapter built without a scope turned an upload's generic `path` (a key prefix for the S3/Azure presign callbacks) into the storage scope, which the server refuses unless it names a value of its sys_file scope select. Both sites now read only the adapter's own scope; with none, the request carries no scope and the server applies its default. The `scope` docblock drops the key-prefix examples and names the server's vocabulary as the authority; the option's type is unchanged. Claude-Session: https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU 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
|
Contributor
Author
Contract-tier review of record — PR objectui#12058 (objectui#12055, the shared half) · PASS on head
|
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.
Part of #12055
Clause-②: no (narrowing)
Summary
createObjectStackUploadAdaptersentscope: opts.scope ?? options.pathat two sites: the presigned upload request body andUploadResult.meta. An adapter built without ascopetherefore turned an upload's genericpathinto the storage scope. Thatpathis a key prefix for the S3 and Azure presign callbacks, and the server refuses any scope that is not a value of itssys_filescope select.This PR lands the half of objectui#12055 that every reading of the card shares:
scope. With none, the request carries noscopeand the server applies its default.scopedocblock drops the key-prefix examples ("avatars", "logos", "attachments/case") and names the server'ssys_filescope select as the vocabulary. TheUploadOptions.pathdocblock says the ObjectStack adapter never sends it.packages/providers/src/__tests__/objectstack-upload-scope-12055.test.tscover the path case, the forwarding case and the two upload shapes this repo uses..changeset/12055-upload-path-not-scope.md:@object-ui/providersminor, with the breaking meaning and the remedy stated.The option's type is unchanged (
string). The card's type half is not in this PR: typingscopefrom@objectstack/spec'sStorageScope, and the type-level pin that refusesscope: 'avatars'. Measurement falsified its premise (next section), so the card stays open for that decision. The first line isPart of, not a closing keyword, because the card goes to the decision box.Why the type half stops:
StorageScopeis not the upload vocabularyMeasured at the
@objectstack/specversion objectui installs (17.7.0, the lockfile's resolution):StorageScopeis exported from@objectstack/spec/systemas thez.inputofStorageScopeSchema. It has ten members: global, tenant, user, session, temp, cache, data, logs, config, public.publicis still in it.scopeselect on thesys_fileobject in@objectstack/service-storage: user, tenant, public, private, temp, attachments. The two upload-starting handlers pass the caller's value straight to that insert. storage: thepublicstorage scope is described as "publicly accessible static assets", but after PR #22439 a default-acl file with that scope needs a signed-in caller — trim the value or enforce it objectstack#22443's dev report (6079733604) measured the same split:StorageScopeSchemais read only byObjectStorageConfigSchema.scope.GetPresignedUrlRequestSchema, declaresscopeas an openz.string()with defaultuser.StorageScoperefuses'private'and'attachments'(TS2322) and accepts'global'.So typing the option as
StorageScopewould stopRecordAttachmentsPanel(scope: 'attachments') from compiling. It would also refuse a value the server accepts (private) and admit six that the server refuses (global, session, cache, data, logs, config). The claim keepsRecordAttachmentsPaneland@objectstack/specoff the file surface, so I stopped there. The options are in the dev report on the card.Hypotheses
StorageScopeandStorageScopeSchemafrom@objectstack/spec/system, ten members,publicincluded. It is not the server's upload vocabulary (above).StorageScope. Callers ofcreateObjectStackUploadAdapter: the console's app-wide adapter (apps/console/src/App.tsx, noscope) andRecordAttachmentsPanel(scope: 'attachments'). Two suites mock the factory:App.uploadAltitude-10131andRecordAttachmentsPanel.affordanceGrant-12047. Upload calls:ProfilePageupload(file), the organizationSettingsPageupload(file),AvatarFieldupload(file),FileFieldupload(file, { onProgress }),ImageFieldupload(blob)andupload(file), andRecordAttachmentsPaneladapter.upload(file). None passes apath. Nothing readsUploadResult.meta.scope.'attachments'is in the server's vocabulary and not inStorageScope.mainand on this head, measured by the reverse leg below. With no scope and no path, the body isfilename,mimeTypeandsize. Withscope: 'attachments'and no path, it also carriesscope: 'attachments'.@object-ui/providersis eager in the console (App.tsximports it at module scope). I ranvite buildofapps/consoleunder the verify lock twice, at this head and withmain's blob of the adapter. The chunk that carries the adapter (ui-components-*) measured 1098012 bytes raw and 282728 gzip onmain, and 1097996 raw and 282729 gzip on head: 16 bytes smaller raw, 1 byte larger gzip. Both builds emitted 2474 JS chunks. The budget bot's CI reading is still to come.Tests and gates (head
d28934975, every run after the last commit)pnpm exec vitest run --maxWorkers=2 packages/providers/ apps/console/src/__tests__/App.uploadAltitude-10131.test.tsx packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.affordanceGrant-12047.test.tsx packages/core/src/utils/__tests__/column-identity.ratchet.test.ts scripts/__tests__/one-authority-per-exported-name-6273.test.ts, run from the repo root under the lock:Test Files 7 passed (7),Tests 39 passed (39),VERDICT command-exit 0. That covers everypackages/providerssuite, both suites that mount the adapter factory, and both repo-wide ratchets.pnpm --filter '@object-ui/providers^...' build(the closure,@object-ui/types), thenpnpm --filter @object-ui/providers type-check(tsc --noEmit && tsc -p tsconfig.test.json):VERDICT command-exit 0.tsc -p tsconfig.test.json --listFileslists the new test file, so the test program compiles it.@object-ui/app-shell,@object-ui/console): not run locally, and owed to CI. The published declaration is unchanged. I emitted declarations with--removeCommentsfor this head and formain's blob, andUploadProvider.d.tsis byte-identical (1995 bytes). No caller's compile can move.node scripts/check-changeset-presence.mjs,check-changeset-no-major.mjs,check-changeset-fixed.mjs,check-changeset-overwrite.mjs,check-changeset-claims.mjsandcheck-pending-changeset-literals.mjs: all exit 0.pnpm check:new-line-citations:VERDICT new-cross-file-line-citations: 0 new citation(s), exit 0.check:control-bytes,check-test-path-rootsandcheck-vi-mock-specifiersalso exit 0.check:phantom-depsanddocs:check-linksare not owed: nopackage.jsonand no docs page changed.pnpm exec eslint --no-inline-config --format jsonon the two.ts/.tsxfiles returned 2 results, 0 errors and 7 warnings, all inUploadProvider.tsx(react-refresh/only-export-componentsandreact-hooks/refs, on code this diff does not touch).main's blob of that file gives the same 0 errors and 7 warnings. The population iseslint.config.js's own**/*.{ts,tsx}glob. That config enables no type-aware linting (noparserOptionsorprojectService), so this diff cannot move the verdict on any untouched file. The repo-widepnpm lintis CI's.node scripts/check-governed-queue-guard.mjs --teston the three paths answeredNOT GOVERNED.Reverse leg
The fix was committed first (
d28934975).main's blob ofpackages/providers/src/UploadProvider.tsxwas written to disk, and its hash was checked againstmain's blob. The anchoropts.scope ?? options.pathcounted 2 on disk. On that blob the new file ranTests 1 failed | 3 passed (4). The failure was the path pin (expected true to be false: the body carriedscope). The forwarding pin and both in-repo shape pins stayed green, which is H3's evidence. A trap restored the file withgit checkout HEAD --. The restore was proven by the on-disk hash matchingHEAD's blob, by an emptygit diff HEAD, and by an anchor count of 0. On head the file runs4 passed. No type-level pin is in this PR (see above).Acceptance notes
content/docs/fields/file.mdxnamescreateObjectStackUploadAdapterbut documents neitherscopenorpath.packages/providers/README.mddoes not cover the upload provider. The option docblocks are the documentation, and this PR rewrites them.RecordAttachmentsPanel,apps/console/src/App.tsx,packages/providers/package.json, the lockfile,@objectstack/specand everypackages/i18nkey are unchanged. Nothing is exported from any package.StorageScopeis named like the upload scope but holds an unrelated vocabulary, and that is how this card's premise formed. Whether the spec declares the upload vocabulary (an enum forGetPresignedUrlRequestSchema.scope) is the open triage question on finding(service-storage): an upload start naming a scope outside the sys_file vocabulary answers 500 INTERNAL (an engine invalid_option relayed as an internal fault) instead of a 400 naming the allowed scopes objectstack#22470, which owns it.Review
This landing owes the
domain:ui#2seat's contract-tier review of record on this PR, per the claim on the card.Generated by Claude Code