fix(approvals+storage): decision attachments — name, open, correct download filename (#3504) (#3505)

* fix(approvals+storage): surface decision attachments with name, open, 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>

* chore(spec): regenerate API surface snapshot for the two added interfaces

Additive only (0 breaking): ApprovalActionAttachment + PresignedDownloadOptions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(approvals): correct the attachment read-shape rationale and share 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

* docs(storage): record the presigned-download options in the hand-written 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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: baozhoutao

.changeset/approval-attachment-descriptors.md

@@ -0,0 +1,29 @@
+---
+"@objectstack/spec": patch
+"@objectstack/plugin-approvals": patch
+---
+
+fix(approvals): return decision attachments as file values, not "[object Object]" (#3504)
+
+`sys_approval_action.attachments` is a `Field.file`, so the column **stores an
+opaque `sys_file` id** (ADR-0104 D3 — the stored form of every media field). The
+ObjectQL read path resolves that id into its expanded
+`{ id, name, size, mimeType, url }` form on the way out. But `rowFromAction`
+mapped the column with `.map(String)`, collapsing each expanded value to the
+literal string `"[object Object]"`. Every `listActions` consumer (the approval
+inbox timeline) then received garbage: the attachment chip had no filename and
+its id was `"[object Object]"`, so opening it 404'd.
+
+- `ApprovalActionRow.attachments` is now `ApprovalActionAttachment[]` — the
+  expanded file value plus its id, so a consumer can label and open an
+  attachment without needing read access to the system `sys_file` object (which
+  regular approvers do not have).
+- Three read forms are accepted: the expanded value (the normal case), a bare id
+  (nothing to expand it into — storage service absent, file not committed), and
+  a legacy inline blob written before file-as-reference (`file_id` /
+  `mime_type`), until the backfill converts it. The id test reuses the
+  platform's `isFileIdToken`, so this and the engine's read resolver cannot
+  disagree about what counts as an id.
+- The decision *input* (`ApprovalDecisionInput.attachments`) is unchanged — it
+  still takes fileId strings, which is also exactly what the column stores. Only
+  the read shape changed.

.changeset/storage-download-filename.md

@@ -0,0 +1,22 @@
+---
+"@objectstack/spec": patch
+"@objectstack/service-storage": patch
+---
+
+fix(storage): downloads carry the real filename + content-type, not the URL token (#3504)
+
+A presigned download served the bytes as `application/octet-stream` with no
+`Content-Disposition`, so a browser saved the file under the opaque URL token
+(e.g. `eyJrIjoiYXR0YWNo…`) instead of its real name — an approval's
+`signed-contract.pdf` downloaded as a nameless blob.
+
+- `IStorageService.getSignedUrl` / `getPresignedDownload` take an optional
+  `PresignedDownloadOptions` (`filename`, `contentType`, `disposition`).
+- The REST download routes (`GET /storage/files/:id/url` and `/:id`) pass the
+  `sys_file` record's `name` + `mime_type`.
+- The local adapter carries them in the signed token; the `_local/raw` route
+  emits `Content-Type` + an RFC 5987 `Content-Disposition` (ASCII fallback +
+  `filename*=UTF-8''…` for non-ASCII names). The S3 adapter bakes the same into
+  the signed URL via `ResponseContentType` / `ResponseContentDisposition`.
+- Default disposition is `inline`, so previewable types (PDF, images) still open
+  in the browser — now with the correct name when saved.

.claude/launch.json

@@ -18,6 +18,24 @@
         "file:/tmp/showcase-dogfood-3777/data.db"
       ],
       "port": 3777
+    },
+    {
+      "name": "pr3505-attach-3477",
+      "runtimeExecutable": "pnpm",
+      "runtimeArgs": [
+        "-C",
+        "/home/user/objectstack-pr3505/examples/app-showcase",
+        "exec",
+        "objectstack",
+        "dev",
+        "--ui",
+        "--seed-admin",
+        "-p",
+        "3477",
+        "-d",
+        "file:/tmp/pr3505-attach/data.db"
+      ],
+      "port": 3477
     }
   ]
 }

content/docs/kernel/contracts/storage-service.mdx

@@ -29,15 +29,19 @@ export interface IStorageService {
   list?(prefix: string): Promise<StorageFileInfo[]>;
 
   // Signed URL (optional)
-  getSignedUrl?(key: string, expiresIn: number): Promise<string>;
+  getSignedUrl?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<string>;
 
   // Presigned browser-direct upload / download (optional)
   getPresignedUpload?(
     key: string,
     expiresIn: number,
     options?: StorageUploadOptions,
   ): Promise<PresignedUploadDescriptor>;
-  getPresignedDownload?(key: string, expiresIn: number): Promise<PresignedDownloadDescriptor>;
+  getPresignedDownload?(
+    key: string,
+    expiresIn: number,
+    options?: PresignedDownloadOptions,
+  ): Promise<PresignedDownloadDescriptor>;
 
   // Chunked / multipart upload (optional)
   initiateChunkedUpload?(key: string, options?: StorageUploadOptions): Promise<string>;

content/docs/kernel/runtime-services/storage-service.mdx

@@ -17,14 +17,14 @@ services.storage.delete(key: string): Promise<void>
 services.storage.exists(key: string): Promise<boolean>
 services.storage.getInfo(key: string): Promise<StorageFileInfo>
 services.storage.list?(prefix: string): Promise<StorageFileInfo[]>
-services.storage.getSignedUrl?(key: string, expiresIn: number): Promise<string>
+services.storage.getSignedUrl?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<string>
 ```
 
 ## Presigned / Chunked (optional)
 
 ```ts
 services.storage.getPresignedUpload?(key: string, expiresIn: number, options?: StorageUploadOptions): Promise<PresignedUploadDescriptor>
-services.storage.getPresignedDownload?(key: string, expiresIn: number): Promise<PresignedDownloadDescriptor>
+services.storage.getPresignedDownload?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<PresignedDownloadDescriptor>
 services.storage.initiateChunkedUpload?(key: string, options?: StorageUploadOptions): Promise<string>
 services.storage.uploadChunk?(uploadId: string, partNumber: number, data: Buffer): Promise<string>
 services.storage.completeChunkedUpload?(uploadId: string, parts: Array<{ partNumber: number; eTag: string }>): Promise<string>

packages/plugins/plugin-approvals/src/approval-service.test.ts

@@ -1969,15 +1969,58 @@ describe('ApprovalService — decision_progress & deep links (#2678 P1.5)', () =
 // listActions must surface decision attachments through the contract mapping
 // (#3266 — the column existed but rowFromAction dropped it; caught in browser).
 describe('ApprovalService — listActions attachments mapping (#3266)', () => {
-  it('returns the attachments recorded on a decision', async () => {
+  it('normalizes a bare fileId string into an attachment descriptor', async () => {
     const engine = makeFakeEngine();
     let n = 0;
     const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } });
     const req = await svc.openNodeRequest(openInput(['u9']), CTX);
     await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9', attachments: ['file_a'] }, SYS);
     const acts = await svc.listActions(req.id, SYS);
     const approve = acts.find(a => a.action === 'approve');
-    expect(approve?.attachments).toEqual(['file_a']);
+    expect(approve?.attachments).toEqual([{ id: 'file_a' }]);
+  });
+
+  // The normal case. The column STORES an opaque sys_file id (ADR-0104 D3);
+  // the ObjectQL read path resolves it into the expanded
+  // `{ id, name, size, mimeType, url }` form on the way out. The old
+  // `.map(String)` turned that object into "[object Object]", so the inbox chip
+  // had no name and 404'd on open. The mapping must pass it through unmangled.
+  it('passes the engine-expanded file value through with its name and url', async () => {
+    const engine = makeFakeEngine();
+    let n = 0;
+    const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } });
+    const req = await svc.openNodeRequest(openInput(['u9']), CTX);
+    await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
+    // Simulate what the read path hands back after resolving the stored id.
+    const row = engine._tables['sys_approval_action'].find((a: any) => a.action === 'approve');
+    row.attachments = [
+      { id: 'file_a', name: 'signed-contract.pdf', mimeType: 'application/pdf', size: 24, url: '/api/v1/storage/files/file_a' },
+    ];
+    const acts = await svc.listActions(req.id, SYS);
+    const approve = acts.find(a => a.action === 'approve');
+    expect(approve?.attachments).toEqual([
+      { id: 'file_a', name: 'signed-contract.pdf', mimeType: 'application/pdf', size: 24, url: '/api/v1/storage/files/file_a' },
+    ]);
+  });
+
+  // Rows written before file-as-reference hold an inline blob whose keys are
+  // snake_case (`file_id`, `mime_type`). They stay readable until the backfill
+  // converts them, so both casings must map — the same drift that made objectui
+  // stop recognising images when the expanded form arrived.
+  it('maps a legacy inline blob (file_id / mime_type) written before the cutover', async () => {
+    const engine = makeFakeEngine();
+    let n = 0;
+    const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } });
+    const req = await svc.openNodeRequest(openInput(['u9']), CTX);
+    await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);
+    const row = engine._tables['sys_approval_action'].find((a: any) => a.action === 'approve');
+    row.attachments = [
+      { file_id: 'file_b', name: 'old.pdf', mime_type: 'application/pdf', size: 12, url: 'https://cdn/old.pdf' },
+    ];
+    const acts = await svc.listActions(req.id, SYS);
+    expect(acts.find(a => a.action === 'approve')?.attachments).toEqual([
+      { id: 'file_b', name: 'old.pdf', mimeType: 'application/pdf', size: 12, url: 'https://cdn/old.pdf' },
+    ]);
   });
 });
 

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -19,6 +19,7 @@ import type {
   IApprovalService,
   ApprovalRequestRow,
   ApprovalActionRow,
+  ApprovalActionAttachment,
   ApprovalDecisionInput,
   ApprovalDecisionResult,
   ApprovalRecallInput,
@@ -30,6 +31,7 @@ import type {
   ApprovalStatus,
   SharingExecutionContext,
 } from '@objectstack/spec/contracts';
+import { isFileIdToken } from '@objectstack/spec/data';
 import { isGrantActive } from '@objectstack/core';
 
 /**
@@ -255,7 +257,58 @@ function slaDueAt(createdAt: unknown, cfg: any): string | undefined {
   return new Date(t + hours * 3600_000).toISOString();
 }
 
+/**
+ * Normalize one raw `attachments` entry into an {@link ApprovalActionAttachment}.
+ *
+ * `sys_approval_action.attachments` is a `Field.file` (multiple), so the column
+ * **stores opaque `sys_file` ids** — that is the stored form of every media
+ * field (ADR-0104 D3). What arrives here is whichever of three forms the read
+ * path produced:
+ *
+ *  1. the **expanded** `{ id, name, size, mimeType, url }` the ObjectQL read
+ *     path resolves a stored id into — the normal case;
+ *  2. a **bare id**, when there was nothing to expand it into (storage service
+ *     absent, file not committed);
+ *  3. a **legacy inline blob** (`{ file_id, name, mime_type, url, … }`) written
+ *     before file-as-reference, until the backfill converts it.
+ *
+ * The original mapping did `String(entry)`, which turned form 1 into the
+ * literal `"[object Object]"` — so the inbox timeline showed a nameless,
+ * un-openable attachment chip (#3266 follow-up; caught by browser verification).
+ *
+ * Note the casing: the expanded form carries `mimeType`, the legacy blob
+ * `mime_type`. Both are accepted for the duration of the migration window.
+ */
+function normalizeActionAttachment(entry: any): ApprovalActionAttachment | undefined {
+  if (entry == null) return undefined;
+  // Form 2 — a bare reference. `isFileIdToken` is the platform's single arbiter
+  // of "is this string an opaque file id, or a URL?", shared with the engine's
+  // read resolver, so the two cannot disagree about what counts as an id.
+  if (typeof entry === 'string') {
+    const id = entry.trim();
+    if (!id) return undefined;
+    return isFileIdToken(id) ? { id } : { id, url: id };
+  }
+  if (typeof entry === 'object') {
+    // Forms 1 and 3 — `file_id` is the legacy blob's key for the same thing.
+    const id = entry.id ?? entry.file_id;
+    if (id == null || String(id) === '') return undefined;
+    const mimeType = entry.mimeType ?? entry.mime_type;
+    return {
+      id: String(id),
+      name: typeof entry.name === 'string' ? entry.name : undefined,
+      url: typeof entry.url === 'string' ? entry.url : undefined,
+      mimeType: typeof mimeType === 'string' ? mimeType : undefined,
+      size: typeof entry.size === 'number' ? entry.size : undefined,
+    };
+  }
+  return undefined;
+}
+
 function rowFromAction(row: any): ApprovalActionRow {
+  const attachments = Array.isArray(row.attachments)
+    ? row.attachments.map(normalizeActionAttachment).filter((a: ApprovalActionAttachment | undefined): a is ApprovalActionAttachment => !!a)
+    : [];
   return {
     id: String(row.id),
     request_id: String(row.request_id),
@@ -264,10 +317,9 @@ function rowFromAction(row: any): ApprovalActionRow {
     action: row.action,
     actor_id: row.actor_id ?? undefined,
     comment: row.comment ?? undefined,
-    // Decision attachments (#3266). The column shipped in #3268 but this
-    // contract mapping didn't — the raw engine row carried the fileIds while
-    // every consumer of listActions saw none (caught by browser verification).
-    attachments: Array.isArray(row.attachments) && row.attachments.length ? row.attachments.map(String) : undefined,
+    // Decision attachments (#3266): rich descriptors carrying the display name +
+    // download URL, so consumers label/open them without reading `sys_file`.
+    attachments: attachments.length ? attachments : undefined,
     created_at: row.created_at ?? undefined,
   };
 }

packages/services/service-storage/src/content-disposition.test.ts

@@ -0,0 +1,30 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import { contentDispositionValue } from './content-disposition.js';
+
+describe('contentDispositionValue', () => {
+  it('defaults to inline and emits both filename and filename*', () => {
+    expect(contentDispositionValue('signed-contract.pdf')).toBe(
+      "inline; filename=\"signed-contract.pdf\"; filename*=UTF-8''signed-contract.pdf",
+    );
+  });
+
+  it('honors an attachment disposition', () => {
+    expect(contentDispositionValue('report.csv', 'attachment')).toBe(
+      "attachment; filename=\"report.csv\"; filename*=UTF-8''report.csv",
+    );
+  });
+
+  it('keeps a non-ASCII name in filename* and sanitizes the ASCII fallback', () => {
+    const v = contentDispositionValue('季度报告.pdf');
+    // ASCII fallback replaces each of the 4 non-ascii chars; filename* preserves them.
+    expect(v).toContain('filename="____.pdf"');
+    expect(v).toContain("filename*=UTF-8''%E5%AD%A3%E5%BA%A6%E6%8A%A5%E5%91%8A.pdf");
+  });
+
+  it('neutralizes quotes/backslashes in the ASCII fallback (no header injection)', () => {
+    const v = contentDispositionValue('a"b\\c.txt');
+    expect(v).toContain('filename="a_b_c.txt"');
+  });
+});

packages/services/service-storage/src/content-disposition.ts

@@ -0,0 +1,22 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Build a `Content-Disposition` header value that survives non-ASCII filenames.
+ *
+ * Emits both a sanitized ASCII `filename=` (legacy fallback) and the RFC 5987
+ * `filename*=UTF-8''…` form that modern browsers prefer — so a download saves
+ * under its real name instead of the opaque URL token. Used by the local
+ * adapter's `_local/raw` route (header) and the S3 adapter's
+ * `ResponseContentDisposition` (baked into the signed URL).
+ */
+export function contentDispositionValue(
+  filename: string,
+  type: 'inline' | 'attachment' = 'inline',
+): string {
+  const asciiFallback = filename.replace(/[^\x20-\x7e]/g, '_').replace(/["\\]/g, '_');
+  const encoded = encodeURIComponent(filename).replace(
+    /['()*]/g,
+    (c) => '%' + c.charCodeAt(0).toString(16).toUpperCase(),
+  );
+  return `${type}; filename="${asciiFallback}"; filename*=UTF-8''${encoded}`;
+}

packages/services/service-storage/src/local-storage-adapter.test.ts

@@ -151,6 +151,21 @@ describe('LocalStorageAdapter', () => {
       expect(desc.downloadUrl).toContain('/_local/raw/');
       expect(desc.expiresIn).toBe(60);
     });
+
+    it('carries filename + content-type into the download token (so the browser saves the real name)', async () => {
+      await createTempDir();
+      await adapter.upload('attachments/abc.pdf', Buffer.from('%PDF-'));
+      const desc = await adapter.getPresignedDownload!('attachments/abc.pdf', 60, {
+        filename: 'signed-contract.pdf',
+        contentType: 'application/pdf',
+        disposition: 'inline',
+      });
+      const token = desc.downloadUrl.split('/_local/raw/')[1];
+      const payload = adapter.verifyToken(token, 'get');
+      expect(payload.n).toBe('signed-contract.pdf');
+      expect(payload.ct).toBe('application/pdf');
+      expect(payload.d).toBe('inline');
+    });
   });
 
   // =========================================================================