fix(approvals+storage): decision attachments — name, open, correct download filename (#3504) - #3505
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
… and correct download filename (#3504) The approval inbox timeline showed a nameless "附件" chip that did nothing on click, and even the downloaded file was named after the opaque URL token. Two root causes, both in the framework: approvals — `sys_approval_action.attachments` (a `Field.file`) stores rich descriptors `{ id, name, url, mimeType, size }`, but `rowFromAction` mapped them with `.map(String)`, collapsing each to "[object Object]". `ApprovalActionRow. attachments` is now `ApprovalActionAttachment[]`; the descriptor carries name + url, so consumers label/open attachments without reading the system `sys_file`. storage — presigned downloads served `application/octet-stream` with no `Content-Disposition`. `getSignedUrl`/`getPresignedDownload` now take `PresignedDownloadOptions { filename, contentType, disposition }`; the REST download routes pass the `sys_file` name+mime; the local adapter carries them in the token and `_local/raw` emits an RFC 5987 Content-Disposition; S3 bakes the same into the signed URL. Default `inline` preserves in-browser preview. Verified end-to-end in app-showcase. Refs objectui #2820. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aces Additive only (0 breaking): ApprovalActionAttachment + PresignedDownloadOptions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… the id rule
Follow-up on this PR's own change, after ADR-0104 D3 wave 2 landed on main.
The fix in this PR is right; its stated cause was inverted. It says the
`sys_approval_action.attachments` column "stores rich descriptors — a fileId is
resolved to a full descriptor on write". The code says otherwise:
approval-service.ts writes `input.attachments` verbatim (a fileId string[]),
and there is no write-side descriptor resolution anywhere in the repo. What
actually happens is that the column stores an opaque sys_file id — the stored
form of every media field — and the ObjectQL read path expands it into
`{ id, name, size, mimeType, url }` on the way out. That resolver was already
in this PR's base commit, so the descriptors observed in listActions came from
the read path, not from storage.
Left as written, that inverted account would have been a false statement about
storage sitting in the protocol contract, in the changeset, and in a regression
test comment — and PR-5a has since made the stored form explicitly an id, so it
would also have been contradicted by the schema. Corrected in all three places.
Also names the three read forms the normalizer actually handles, rather than
one: the expanded value (normal), a bare id (nothing to expand it into), and a
legacy inline blob written before the cutover, whose keys are snake_case
(`file_id`, `mime_type`). The last of those had no test; it has one now. The
id-token test reuses `isFileIdToken` from @objectstack/spec/data — the
platform's single arbiter of "opaque id or URL?" — so this normalizer and the
engine's read resolver cannot drift apart on that question, and a non-id string
is now surfaced as a url rather than mislabelled an id.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
os-zhuang
force-pushed
the
claude/approval-attachment-authz
branch
from
July 27, 2026 07:04
b6f4740 to
874144e
Compare
Contributor
|
我 rebase 了这个分支到当前 main,并推了一个修正 commit( 背景:ADR-0104 D3 wave 2 在这个 PR 开出之后合并了(#3527 / #3534 / #3535 / #3555 + objectui#2828),涉及同一片区域。 好消息:两半都不用重做存储那一半原样保留。 审批那一半的修法也对, 我改了什么:因果讲反了PR 描述/changeset/spec 注释/测试注释里说了四遍:
代码不支持这个说法:
真实情况是反的:列里存的就是 留着不改的话,这会变成协议契约里一句关于存储的假话,而且 #3555 之后 没有改任何行为,只改了对行为的描述,加上下面两点。 顺带补的两点
测试:plugin-approvals 195 ✅ · service-storage 173 ✅ · spec 6717 ✅ · build ✅ · lint ✅ · nul-bytes ✅ · check:api-surface ✅ · check:docs ✅
|
…ten contract docs The drift check flagged these: both storage-service pages transcribe IStorageService by hand, and this PR adds an optional PresignedDownloadOptions argument to getSignedUrl / getPresignedDownload. Left alone they would describe a signature the code no longer has — the same declared-vs-actual gap the rest of this PR is about, one level up in the docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
This was referenced Jul 27, 2026
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
3 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 #3504. Frontend companion: objectstack-ai/objectui#2820.
Problem
A decision attachment in the approval inbox timeline (审批动态) showed a nameless "附件" chip that did nothing when clicked — and even when the file did download, it was named after the opaque signed-URL token (
eyJrIjoiYXR0YWNo…) asapplication/octet-stream.Root cause 1 — approvals read path stringified descriptors
sys_approval_action.attachmentsis aField.file, which stores rich descriptors{ id, name, url, mimeType, size }(a fileId is resolved to a full descriptor on write) — not fileId strings.rowFromActionmapped the column with.map(String), collapsing each descriptor to the literal"[object Object]". EverylistActionsconsumer received garbage.Root cause 2 — storage download had no filename/type
Presigned downloads served
application/octet-streamwith noContent-Disposition, and the signed token carried no filename, so browsers saved files under the URL token.Changes
approvals (
@objectstack/spec,@objectstack/plugin-approvals)ApprovalActionAttachment;ApprovalActionRow.attachmentsis nowApprovalActionAttachment[].rowFromActionpasses descriptors through (tolerating a bare-string fileId). Decision input staysstring[]. Consumers no longer need read access to the systemsys_fileobject.storage (
@objectstack/spec,@objectstack/service-storage)getSignedUrl/getPresignedDownloadtake optionalPresignedDownloadOptions { filename, contentType, disposition }.GET /storage/files/:id/url,/:id) pass thesys_filename+mime_type._local/rawemitsContent-Type+ RFC 5987Content-Disposition(ASCII fallback +filename*=UTF-8''…). S3 adapter usesResponseContentType/ResponseContentDisposition. Defaultinlinepreserves in-browser preview.Testing
@objectstack/plugin-approvals: 165/165 (incl. new descriptor-passthrough cases).@objectstack/service-storage: 104/104 (incl. new token-metadata + Content-Disposition helper cases).app-showcase(approvals + storage): chip showssigned-contract.pdf, click opens the real PDF, and the download respondscontent-type: application/pdf+content-disposition: inline; filename="signed-contract.pdf".🤖 Generated with Claude Code