From 2ab06561c1e6de902fef7993a3f34d988d90c5d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= <baozhoutao@hotoa.com>
Date: Sun, 26 Jul 2026 03:54:36 -0700
Subject: [PATCH 1/4] fix(approvals+storage): surface decision attachments with
 name, open, and correct download filename (#3504)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .changeset/approval-attachment-descriptors.md | 22 ++++++++++
 .changeset/storage-download-filename.md       | 22 ++++++++++
 .../src/approval-service.test.ts              | 26 ++++++++++-
 .../plugin-approvals/src/approval-service.ts  | 43 +++++++++++++++++--
 .../src/content-disposition.test.ts           | 30 +++++++++++++
 .../src/content-disposition.ts                | 22 ++++++++++
 .../src/local-storage-adapter.test.ts         | 15 +++++++
 .../src/local-storage-adapter.ts              | 25 +++++++++--
 .../service-storage/src/s3-storage-adapter.ts | 23 ++++++++--
 .../service-storage/src/storage-routes.ts     | 16 +++++--
 .../src/swappable-storage-service.ts          |  9 ++--
 .../spec/src/contracts/approval-service.ts    | 22 +++++++++-
 .../spec/src/contracts/storage-service.ts     | 20 ++++++++-
 13 files changed, 269 insertions(+), 26 deletions(-)
 create mode 100644 .changeset/approval-attachment-descriptors.md
 create mode 100644 .changeset/storage-download-filename.md
 create mode 100644 packages/services/service-storage/src/content-disposition.test.ts
 create mode 100644 packages/services/service-storage/src/content-disposition.ts

diff --git a/.changeset/approval-attachment-descriptors.md b/.changeset/approval-attachment-descriptors.md
new file mode 100644
index 0000000000..8a5de8f879
--- /dev/null
+++ b/.changeset/approval-attachment-descriptors.md
@@ -0,0 +1,22 @@
+---
+"@objectstack/spec": patch
+"@objectstack/plugin-approvals": patch
+---
+
+fix(approvals): return decision attachments as file descriptors, not "[object Object]" (#3504)
+
+The `sys_approval_action.attachments` file field stores rich descriptors
+(`{ id, name, url, mimeType, size }`), not bare fileId strings — a fileId passed
+to the decision is resolved to a full descriptor on write. But `rowFromAction`
+mapped the column with `.map(String)`, collapsing each descriptor object 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
+  descriptor carries `id` + display `name` + a download `url`, 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). A bare-string fileId
+  still normalizes to `{ id }` for safety.
+- The decision *input* (`ApprovalDecisionInput.attachments`) is unchanged — it
+  still takes fileId strings. Only the read shape changed.
diff --git a/.changeset/storage-download-filename.md b/.changeset/storage-download-filename.md
new file mode 100644
index 0000000000..08466bf709
--- /dev/null
+++ b/.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.
diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts
index 2ef565cfa7..f2d01d43f8 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.test.ts
+++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts
@@ -1948,7 +1948,7 @@ 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) } });
@@ -1956,6 +1956,28 @@ describe('ApprovalService — listActions attachments mapping (#3266)', () => {
     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 real column value: `attachments` is a `Field.file` (multiple), which
+  // stores rich descriptors — NOT fileId strings. The old `.map(String)` turned
+  // each into "[object Object]", so the inbox chip had no name and 404'd on open.
+  // The mapping must pass the descriptor (id + name + url) through unmangled.
+  it('passes a stored file descriptor object 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 the Field.file column's real shape (what the engine returns).
+    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' },
+    ]);
   });
 });
diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts
index 671f8a8331..b9b9c27252 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.ts
+++ b/packages/plugins/plugin-approvals/src/approval-service.ts
@@ -19,6 +19,7 @@ import type {
   IApprovalService,
   ApprovalRequestRow,
   ApprovalActionRow,
+  ApprovalActionAttachment,
   ApprovalDecisionInput,
   ApprovalDecisionResult,
   ApprovalRecallInput,
@@ -255,7 +256,42 @@ 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}.
+ *
+ * The `sys_approval_action.attachments` file field stores **rich descriptors**
+ * (`{ id, name, url, mimeType, size }`), not bare fileId strings — even when the
+ * decision was recorded with a fileId, the field resolves it on write. The
+ * original mapping did `String(entry)`, which turned each descriptor object into
+ * the literal `"[object Object]"` — so the inbox timeline showed a nameless,
+ * un-openable attachment chip (#3266 follow-up; caught by browser verification).
+ * We pass the descriptor through, tolerating a bare-string fileId for safety.
+ */
+function normalizeActionAttachment(entry: any): ApprovalActionAttachment | undefined {
+  if (entry == null) return undefined;
+  if (typeof entry === 'string') {
+    const id = entry.trim();
+    return id ? { id } : undefined;
+  }
+  if (typeof entry === 'object') {
+    const id = entry.id ?? entry.fileId ?? 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 +300,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,
   };
 }
diff --git a/packages/services/service-storage/src/content-disposition.test.ts b/packages/services/service-storage/src/content-disposition.test.ts
new file mode 100644
index 0000000000..a60cc7eeb9
--- /dev/null
+++ b/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"');
+  });
+});
diff --git a/packages/services/service-storage/src/content-disposition.ts b/packages/services/service-storage/src/content-disposition.ts
new file mode 100644
index 0000000000..c330511435
--- /dev/null
+++ b/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}`;
+}
diff --git a/packages/services/service-storage/src/local-storage-adapter.test.ts b/packages/services/service-storage/src/local-storage-adapter.test.ts
index e93ee1472c..184aece397 100644
--- a/packages/services/service-storage/src/local-storage-adapter.test.ts
+++ b/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');
+    });
   });
 
   // =========================================================================
diff --git a/packages/services/service-storage/src/local-storage-adapter.ts b/packages/services/service-storage/src/local-storage-adapter.ts
index 19271cccc0..8c09abc32d 100644
--- a/packages/services/service-storage/src/local-storage-adapter.ts
+++ b/packages/services/service-storage/src/local-storage-adapter.ts
@@ -14,6 +14,7 @@ import type {
   StorageFileInfo,
   PresignedUploadDescriptor,
   PresignedDownloadDescriptor,
+  PresignedDownloadOptions,
 } from '@objectstack/spec/contracts';
 
 /**
@@ -48,6 +49,8 @@ export interface LocalStorageAdapterOptions {
 interface PresignTokenPayload {
   k: string;       // storage key
   ct?: string;     // content-type
+  n?: string;      // download filename (Content-Disposition)
+  d?: 'inline' | 'attachment'; // disposition type (default 'inline')
   exp: number;     // expiry epoch seconds
   op: 'put' | 'get';
 }
@@ -268,17 +271,31 @@ export class LocalStorageAdapter implements IStorageService {
     };
   }
 
-  async getPresignedDownload(key: string, expiresIn: number): Promise<PresignedDownloadDescriptor> {
+  async getPresignedDownload(
+    key: string,
+    expiresIn: number,
+    options?: PresignedDownloadOptions,
+  ): Promise<PresignedDownloadDescriptor> {
     const exp = Math.floor(Date.now() / 1000) + Math.max(1, expiresIn);
-    const token = this.signToken({ k: key, exp, op: 'get' });
+    // Carry filename + content-type in the token so the `_local/raw` route can
+    // emit a real Content-Disposition / Content-Type (else the browser saves
+    // the file under the opaque token, as `application/octet-stream`).
+    const token = this.signToken({
+      k: key,
+      exp,
+      op: 'get',
+      ct: options?.contentType,
+      n: options?.filename,
+      d: options?.disposition,
+    });
     return {
       downloadUrl: `${this.baseUrl}${this.basePath}/_local/raw/${token}`,
       expiresIn,
     };
   }
 
-  async getSignedUrl(key: string, expiresIn: number): Promise<string> {
-    const desc = await this.getPresignedDownload(key, expiresIn);
+  async getSignedUrl(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<string> {
+    const desc = await this.getPresignedDownload(key, expiresIn, options);
     return desc.downloadUrl;
   }
 
diff --git a/packages/services/service-storage/src/s3-storage-adapter.ts b/packages/services/service-storage/src/s3-storage-adapter.ts
index 840c32f8fd..29d5fcd221 100644
--- a/packages/services/service-storage/src/s3-storage-adapter.ts
+++ b/packages/services/service-storage/src/s3-storage-adapter.ts
@@ -11,7 +11,9 @@ import type {
   StorageFileInfo,
   PresignedUploadDescriptor,
   PresignedDownloadDescriptor,
+  PresignedDownloadOptions,
 } from '@objectstack/spec/contracts';
+import { contentDispositionValue } from './content-disposition.js';
 
 /**
  * Configuration for the S3 storage adapter.
@@ -227,8 +229,8 @@ export class S3StorageAdapter implements IStorageService {
   // Presigned URLs
   // ---------------------------------------------------------------------------
 
-  async getSignedUrl(key: string, expiresIn: number): Promise<string> {
-    const desc = await this.getPresignedDownload(key, expiresIn);
+  async getSignedUrl(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<string> {
+    const desc = await this.getPresignedDownload(key, expiresIn, options);
     return desc.downloadUrl;
   }
 
@@ -256,11 +258,24 @@ export class S3StorageAdapter implements IStorageService {
     };
   }
 
-  async getPresignedDownload(key: string, expiresIn: number): Promise<PresignedDownloadDescriptor> {
+  async getPresignedDownload(
+    key: string,
+    expiresIn: number,
+    options?: PresignedDownloadOptions,
+  ): Promise<PresignedDownloadDescriptor> {
     const client = await this.getClient();
     const s3 = await this.s3Mod();
     const { getSignedUrl } = await this.presignerMod();
-    const cmd = new s3.GetObjectCommand({ Bucket: this.bucket, Key: key });
+    // S3 bakes these response overrides into the signed URL, so the download
+    // carries the real filename + type instead of the object key + octet-stream.
+    const cmd = new s3.GetObjectCommand({
+      Bucket: this.bucket,
+      Key: key,
+      ...(options?.contentType ? { ResponseContentType: options.contentType } : {}),
+      ...(options?.filename
+        ? { ResponseContentDisposition: contentDispositionValue(options.filename, options.disposition ?? 'inline') }
+        : {}),
+    });
     const url = await getSignedUrl(client, cmd, { expiresIn });
     return { downloadUrl: url, expiresIn };
   }
diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts
index f9a66db99f..a57db99528 100644
--- a/packages/services/service-storage/src/storage-routes.ts
+++ b/packages/services/service-storage/src/storage-routes.ts
@@ -4,6 +4,7 @@ import { randomUUID } from 'node:crypto';
 import type { IHttpServer, IHttpRequest, IHttpResponse, IStorageService } from '@objectstack/spec/contracts';
 import type { StorageMetadataStore, FileRecord } from './metadata-store.js';
 import type { LocalStorageAdapter } from './local-storage-adapter.js';
+import { contentDispositionValue } from './content-disposition.js';
 
 /** Authorization verdict for an attachments-scope download (#2970 item 2). */
 export type FileReadVerdict = 'allow' | 'deny' | 'unauthenticated';
@@ -518,12 +519,13 @@ export function registerStorageRoutes(
       const ttl = await authorizeDownload(file, req, res);
       if (ttl === false) return;
 
+      const downloadOpts = { filename: file.name, contentType: file.mime_type };
       let url: string;
       if (storage.getPresignedDownload) {
-        const desc = await storage.getPresignedDownload(file.key, ttl);
+        const desc = await storage.getPresignedDownload(file.key, ttl, downloadOpts);
         url = desc.downloadUrl;
       } else if (storage.getSignedUrl) {
-        url = await storage.getSignedUrl(file.key, ttl);
+        url = await storage.getSignedUrl(file.key, ttl, downloadOpts);
       } else {
         url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`;
       }
@@ -557,12 +559,13 @@ export function registerStorageRoutes(
       const ttl = await authorizeDownload(file, req, res);
       if (ttl === false) return;
 
+      const downloadOpts = { filename: file.name, contentType: file.mime_type };
       let url: string;
       if (storage.getPresignedDownload) {
-        const desc = await storage.getPresignedDownload(file.key, ttl);
+        const desc = await storage.getPresignedDownload(file.key, ttl, downloadOpts);
         url = desc.downloadUrl;
       } else if (storage.getSignedUrl) {
-        url = await storage.getSignedUrl(file.key, ttl);
+        url = await storage.getSignedUrl(file.key, ttl, downloadOpts);
       } else {
         url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`;
       }
@@ -621,6 +624,11 @@ export function registerStorageRoutes(
 
       res.header('content-type', payload.ct ?? 'application/octet-stream');
       res.header('content-length', String(data.byteLength));
+      // When the token carries the original filename, advertise it so the
+      // browser saves the file under its real name (not the opaque URL token).
+      if (payload.n) {
+        res.header('content-disposition', contentDispositionValue(payload.n, payload.d ?? 'inline'));
+      }
       res.send(data);
     } catch (err: any) {
       const statusCode = err.message?.includes('expired') || err.message?.includes('signature') ? 403 : 500;
diff --git a/packages/services/service-storage/src/swappable-storage-service.ts b/packages/services/service-storage/src/swappable-storage-service.ts
index 670e290b92..f4f1af7d0a 100644
--- a/packages/services/service-storage/src/swappable-storage-service.ts
+++ b/packages/services/service-storage/src/swappable-storage-service.ts
@@ -6,6 +6,7 @@ import type {
   StorageUploadOptions,
   PresignedUploadDescriptor,
   PresignedDownloadDescriptor,
+  PresignedDownloadOptions,
 } from '@objectstack/spec/contracts';
 
 /**
@@ -77,11 +78,11 @@ export class SwappableStorageService implements IStorageService {
     return this.inner.list(prefix);
   }
 
-  getSignedUrl(key: string, expiresIn: number): Promise<string> {
+  getSignedUrl(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<string> {
     if (typeof this.inner.getSignedUrl !== 'function') {
       return Promise.reject(new Error('Active storage adapter does not support getSignedUrl()'));
     }
-    return this.inner.getSignedUrl(key, expiresIn);
+    return this.inner.getSignedUrl(key, expiresIn, options);
   }
 
   getPresignedUpload(
@@ -95,11 +96,11 @@ export class SwappableStorageService implements IStorageService {
     return this.inner.getPresignedUpload(key, expiresIn, options);
   }
 
-  getPresignedDownload(key: string, expiresIn: number): Promise<PresignedDownloadDescriptor> {
+  getPresignedDownload(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<PresignedDownloadDescriptor> {
     if (typeof this.inner.getPresignedDownload !== 'function') {
       return Promise.reject(new Error('Active storage adapter does not support getPresignedDownload()'));
     }
-    return this.inner.getPresignedDownload(key, expiresIn);
+    return this.inner.getPresignedDownload(key, expiresIn, options);
   }
 
   initiateChunkedUpload(key: string, options?: StorageUploadOptions): Promise<string> {
diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts
index 698de94bda..4f9759c1d6 100644
--- a/packages/spec/src/contracts/approval-service.ts
+++ b/packages/spec/src/contracts/approval-service.ts
@@ -199,6 +199,24 @@ export type ApprovalActionKind =
   /** #1322 M1: an out-of-office approver's slot was auto-rerouted to their delegate at resolution time. */
   | 'ooo_substitute';
 
+/**
+ * A file attached to a decision action (#3266). Resolved from the
+ * `sys_approval_action.attachments` file field, which stores rich descriptors
+ * (not bare fileIds) — so the row carries the display name and a download URL
+ * directly, and consumers never need read access to the system `sys_file`
+ * object to label or open an attachment.
+ */
+export interface ApprovalActionAttachment {
+  /** The `sys_file` id — pass to `GET /storage/files/:id/url` for a signed URL. */
+  id: string;
+  /** Original filename, for the chip label. */
+  name?: string;
+  /** Stable download URL (`/api/v1/storage/files/:id`); may be relative. */
+  url?: string;
+  mimeType?: string;
+  size?: number;
+}
+
 /** Audit row. */
 export interface ApprovalActionRow {
   id: string;
@@ -208,8 +226,8 @@ export interface ApprovalActionRow {
   action: ApprovalActionKind;
   actor_id?: string;
   comment?: string;
-  /** File references attached to this action (decision attachments, #3266). */
-  attachments?: string[];
+  /** Files attached to this action (decision attachments, #3266). */
+  attachments?: ApprovalActionAttachment[];
   created_at?: string;
   /** Display name of the actor (`sys_user.name`), when resolvable. */
   actor_name?: string;
diff --git a/packages/spec/src/contracts/storage-service.ts b/packages/spec/src/contracts/storage-service.ts
index d06e73d950..db493c9e04 100644
--- a/packages/spec/src/contracts/storage-service.ts
+++ b/packages/spec/src/contracts/storage-service.ts
@@ -71,6 +71,22 @@ export interface PresignedDownloadDescriptor {
     expiresIn: number;
 }
 
+/**
+ * Presentation hints for a presigned download. Without these the served bytes
+ * default to `application/octet-stream` with no filename, so a browser saves
+ * the file under the opaque URL token instead of its real name. Callers that
+ * know the file's metadata (e.g. the REST download routes, which have the
+ * `sys_file` record) should pass it so the download carries a real name + type.
+ */
+export interface PresignedDownloadOptions {
+    /** Original filename → `Content-Disposition` (the browser's save-as name). */
+    filename?: string;
+    /** Content type → `Content-Type` (defaults to `application/octet-stream`). */
+    contentType?: string;
+    /** `inline` previews in the browser (default); `attachment` forces a download. */
+    disposition?: 'inline' | 'attachment';
+}
+
 export interface IStorageService {
     /**
      * Upload a file to storage
@@ -120,7 +136,7 @@ export interface IStorageService {
      * @param expiresIn - URL expiration time in seconds
      * @returns Pre-signed URL string
      */
-    getSignedUrl?(key: string, expiresIn: number): Promise<string>;
+    getSignedUrl?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<string>;
 
     // ==========================================
     // Presigned Upload / Download (browser-direct)
@@ -155,7 +171,7 @@ export interface IStorageService {
      * @param key - Storage key/path
      * @param expiresIn - URL expiration time in seconds
      */
-    getPresignedDownload?(key: string, expiresIn: number): Promise<PresignedDownloadDescriptor>;
+    getPresignedDownload?(key: string, expiresIn: number, options?: PresignedDownloadOptions): Promise<PresignedDownloadDescriptor>;
 
     // ==========================================
     // Chunked / Multipart Upload Methods

From 3b14f45de2545bb8278835295c8fdf8a1a51c932 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= <baozhoutao@hotoa.com>
Date: Sun, 26 Jul 2026 04:11:40 -0700
Subject: [PATCH 2/4] 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>
---
 packages/spec/api-surface.json | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json
index 108820987c..52a309416d 100644
--- a/packages/spec/api-surface.json
+++ b/packages/spec/api-surface.json
@@ -3434,6 +3434,7 @@
     "AnalyticsQuery (interface)",
     "AnalyticsResult (interface)",
     "AnalyticsStrategy (interface)",
+    "ApprovalActionAttachment (interface)",
     "ApprovalActionKind (type)",
     "ApprovalActionRow (interface)",
     "ApprovalDecisionInput (interface)",
@@ -3590,6 +3591,7 @@
     "Plugin (interface)",
     "PluginStartupResult (interface)",
     "PresignedDownloadDescriptor (interface)",
+    "PresignedDownloadOptions (interface)",
     "PresignedUploadDescriptor (interface)",
     "ProposePendingActionInput (interface)",
     "PubSubHandler (type)",

From 874144ed57c608c4c620a69a31959563e3366593 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 27 Jul 2026 07:03:57 +0000
Subject: [PATCH 3/4] fix(approvals): correct the attachment read-shape
 rationale and share the id rule
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .changeset/approval-attachment-descriptors.md | 27 +++++++++------
 .../src/approval-service.test.ts              | 33 +++++++++++++++----
 .../plugin-approvals/src/approval-service.ts  | 33 ++++++++++++++-----
 .../spec/src/contracts/approval-service.ts    | 17 +++++++---
 4 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/.changeset/approval-attachment-descriptors.md b/.changeset/approval-attachment-descriptors.md
index 8a5de8f879..544b79ce8a 100644
--- a/.changeset/approval-attachment-descriptors.md
+++ b/.changeset/approval-attachment-descriptors.md
@@ -3,20 +3,27 @@
 "@objectstack/plugin-approvals": patch
 ---
 
-fix(approvals): return decision attachments as file descriptors, not "[object Object]" (#3504)
+fix(approvals): return decision attachments as file values, not "[object Object]" (#3504)
 
-The `sys_approval_action.attachments` file field stores rich descriptors
-(`{ id, name, url, mimeType, size }`), not bare fileId strings — a fileId passed
-to the decision is resolved to a full descriptor on write. But `rowFromAction`
-mapped the column with `.map(String)`, collapsing each descriptor object to the
+`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
-  descriptor carries `id` + display `name` + a download `url`, 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). A bare-string fileId
-  still normalizes to `{ id }` for safety.
+  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. Only the read shape changed.
+  still takes fileId strings, which is also exactly what the column stores. Only
+  the read shape changed.
diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts
index f2d01d43f8..4c6accbcaa 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.test.ts
+++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts
@@ -1959,17 +1959,18 @@ describe('ApprovalService — listActions attachments mapping (#3266)', () => {
     expect(approve?.attachments).toEqual([{ id: 'file_a' }]);
   });
 
-  // The real column value: `attachments` is a `Field.file` (multiple), which
-  // stores rich descriptors — NOT fileId strings. The old `.map(String)` turned
-  // each into "[object Object]", so the inbox chip had no name and 404'd on open.
-  // The mapping must pass the descriptor (id + name + url) through unmangled.
-  it('passes a stored file descriptor object through with its name and url', async () => {
+  // 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 the Field.file column's real shape (what the engine returns).
+    // 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' },
@@ -1980,4 +1981,24 @@ describe('ApprovalService — listActions attachments mapping (#3266)', () => {
       { 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' },
+    ]);
+  });
 });
diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts
index b9b9c27252..c75354c28c 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.ts
+++ b/packages/plugins/plugin-approvals/src/approval-service.ts
@@ -31,6 +31,7 @@ import type {
   ApprovalStatus,
   SharingExecutionContext,
 } from '@objectstack/spec/contracts';
+import { isFileIdToken } from '@objectstack/spec/data';
 import { isGrantActive } from '@objectstack/core';
 
 /**
@@ -259,22 +260,38 @@ function slaDueAt(createdAt: unknown, cfg: any): string | undefined {
 /**
  * Normalize one raw `attachments` entry into an {@link ApprovalActionAttachment}.
  *
- * The `sys_approval_action.attachments` file field stores **rich descriptors**
- * (`{ id, name, url, mimeType, size }`), not bare fileId strings — even when the
- * decision was recorded with a fileId, the field resolves it on write. The
- * original mapping did `String(entry)`, which turned each descriptor object into
- * the literal `"[object Object]"` — so the inbox timeline showed a nameless,
+ * `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).
- * We pass the descriptor through, tolerating a bare-string fileId for safety.
+ *
+ * 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();
-    return id ? { id } : undefined;
+    if (!id) return undefined;
+    return isFileIdToken(id) ? { id } : { id, url: id };
   }
   if (typeof entry === 'object') {
-    const id = entry.id ?? entry.fileId ?? entry.file_id;
+    // 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 {
diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts
index 4f9759c1d6..26ac8bf78b 100644
--- a/packages/spec/src/contracts/approval-service.ts
+++ b/packages/spec/src/contracts/approval-service.ts
@@ -200,11 +200,18 @@ export type ApprovalActionKind =
   | 'ooo_substitute';
 
 /**
- * A file attached to a decision action (#3266). Resolved from the
- * `sys_approval_action.attachments` file field, which stores rich descriptors
- * (not bare fileIds) — so the row carries the display name and a download URL
- * directly, and consumers never need read access to the system `sys_file`
- * object to label or open an attachment.
+ * A file attached to a decision action (#3266) — the READ shape of one
+ * `sys_approval_action.attachments` entry.
+ *
+ * The column **stores an opaque `sys_file` id** (ADR-0104 D3: that is the
+ * stored form of every media field). The name, size, MIME type and URL are not
+ * stored alongside it — the ObjectQL read path resolves the id into its
+ * expanded `FileValueSchema` form on the way out, and this interface is that
+ * form plus the id. So a consumer gets everything it needs to label and open an
+ * attachment without read access to the system `sys_file` object, while the
+ * write side stays a plain id (see `ApprovalDecisionInput.attachments`).
+ *
+ * Field names follow the expanded form — `mimeType`, not `mime_type`.
  */
 export interface ApprovalActionAttachment {
   /** The `sys_file` id — pass to `GET /storage/files/:id/url` for a signed URL. */

From 2fe9ba075b2b4eb4775bc3d1c6d928f12e9b69a3 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 27 Jul 2026 07:56:58 +0000
Subject: [PATCH 4/4] docs(storage): record the presigned-download options in
 the hand-written contract docs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .claude/launch.json                            | 18 ++++++++++++++++++
 .../docs/kernel/contracts/storage-service.mdx  |  8 ++++++--
 .../runtime-services/storage-service.mdx       |  4 ++--
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/.claude/launch.json b/.claude/launch.json
index f2ec1bcf39..6341418fd0 100644
--- a/.claude/launch.json
+++ b/.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
     }
   ]
 }
diff --git a/content/docs/kernel/contracts/storage-service.mdx b/content/docs/kernel/contracts/storage-service.mdx
index be8f95bda8..e7e0a4cc65 100644
--- a/content/docs/kernel/contracts/storage-service.mdx
+++ b/content/docs/kernel/contracts/storage-service.mdx
@@ -29,7 +29,7 @@ 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?(
@@ -37,7 +37,11 @@ export interface IStorageService {
     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>;
diff --git a/content/docs/kernel/runtime-services/storage-service.mdx b/content/docs/kernel/runtime-services/storage-service.mdx
index bd29137bdd..0a78a2ce25 100644
--- a/content/docs/kernel/runtime-services/storage-service.mdx
+++ b/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>