fix(runtime,hono,plugin-dev): 退役 dispatcher 的 /storage bridge —— 它从来没按契约调用过 storage (#4087)

.changeset/retire-dispatcher-storage-bridge.md

@@ -0,0 +1,63 @@
+---
+"@objectstack/runtime": minor
+"@objectstack/hono": minor
+"@objectstack/plugin-dev": patch
+---
+
+fix(runtime,hono,plugin-dev): retire the dispatcher's `/storage` bridge — it never spoke the storage contract (#4087)
+
+`POST /api/v1/storage/upload` and `GET /api/v1/storage/file/:id` were a
+dispatcher-side bridge to the `file-storage` service slot, written against a
+service shape that does not exist:
+
+- **Upload** called the contract's `upload(key, data, options?)` as
+  `upload(file, { request })` — the parsed file object landed in the `key`
+  slot and `{ request }` in `data`. That is a `TypeError` against every
+  implementation in the repo (`S3StorageAdapter`, `LocalStorageAdapter`,
+  `SwappableStorageService`, plugin-dev's in-memory one), not a
+  near-miss: `Buffer.from({}) → ERR_INVALID_ARG_TYPE`, or an object used as
+  an S3 object key / `path.join` segment.
+- **Download** branched on `result.url` / `result.redirect` / `result.stream`
+  / `result.mimeType` while the contract's `download(key)` resolves a
+  `Buffer`, so every branch fell through and the route answered a
+  JSON-serialized Buffer.
+
+Both routes are removed, along with `HttpDispatcher.handleStorage()`, the
+`/storage` domain registration, the dispatcher-plugin mounts and the two route
+ledger rows.
+
+**Migration.** There is nothing to migrate off in practice — neither route
+could complete a request. (They were reachable: `service-storage` mounts
+`/storage/upload/presigned`, not `/storage/upload`, so nothing shadowed them.
+They simply had no caller — no SDK method builds those URLs.)
+`/api/v1/storage` is `@objectstack/service-storage`'s surface and always was
+the working one:
+
+- Upload — FROM `POST /api/v1/storage/upload` TO the presigned protocol
+  (`POST /storage/upload/presigned` → direct `PUT` to the returned URL →
+  `POST /storage/upload/complete`), or `client.storage.upload(file)`, which
+  runs all three steps.
+- Download — FROM `GET /api/v1/storage/file/:id` TO
+  `GET /storage/files/:fileId/url` (`client.storage.getDownloadUrl(fileId)`)
+  for a signed URL, or `GET /storage/files/:fileId` for a stable browser URL
+  that 302s to it.
+
+Install `@objectstack/service-storage` to get those routes; without it
+`/api/v1/storage` now has no handler, which is the same answer every other
+uninstalled capability gives.
+
+Two follow-on corrections keep `declared === enforced`:
+
+- `@objectstack/hono` no longer mounts `app.all('<prefix>/storage/*')`. That
+  wildcard claimed the whole `/storage` subtree for the two dead routes, so
+  every other path under it — service-storage's protocol above all — got the
+  bridge's own 404 rather than falling through. Storage is ordinary catch-all
+  traffic now.
+- Discovery keeps gating `routes.storage` on `isServiceServeable` — the shared
+  `handlerReady` predicate #4058 step 2 introduced — and plugin-dev's in-memory
+  implementation now self-declares `handlerReady: false`. #4058 deliberately
+  left that one serving because the `/storage` bridge was still there to serve
+  it; with the bridge retired nothing routes HTTP to that slot, so `false` is
+  the honest value — the position `realtime` has held since ADR-0076 D12. The
+  implementation keeps working for in-process callers; it is simply no longer
+  advertised as a reachable HTTP capability.

content/docs/api/plugin-endpoints.mdx

@@ -144,10 +144,43 @@ protocol `ai.chatStream` parses and owns message state for you.
 
 ### File Storage (`/storage`) — Plugin Required
 
-| Method | Endpoint | Description |
-|:-------|:---------|:------------|
-| POST | `/storage/upload` | Upload a file |
-| GET | `/storage/file/:id` | Download a file |
+Provided by `@objectstack/service-storage`, which registers these routes on the
+host HTTP server. Uploads are a three-step protocol — ask for a presigned
+target, send the bytes straight to it, then commit — so the bytes never proxy
+through the API server, and the same shape works for S3, local disk, or any
+other adapter.
+
+| Method | Endpoint | SDK | Description |
+|:-------|:---------|:----|:------------|
+| POST | `/storage/upload/presigned` | `storage.getPresignedUrl` | Step 1 — mint an upload target for a new `sys_file` row |
+| POST | `/storage/upload/complete` | `storage.upload` | Step 3 — commit the uploaded bytes |
+| POST | `/storage/upload/chunked` | `storage.initChunkedUpload` | Open a chunked / resumable session |
+| PUT | `/storage/upload/chunked/:uploadId/chunk/:chunkIndex` | `storage.uploadPart` | Send one chunk |
+| POST | `/storage/upload/chunked/:uploadId/complete` | `storage.completeChunkedUpload` | Assemble the parts |
+| GET | `/storage/upload/chunked/:uploadId/progress` | `storage.resumeUpload` | Read session progress (first step of a resume) |
+| GET | `/storage/files/:fileId/url` | `storage.getDownloadUrl` | Resolve a short-lived signed download URL |
+| GET | `/storage/files/:fileId` | — | Stable browser URL; 302s to the same signed URL (what file fields carry) |
+
+`storage.upload(file)` runs steps 1–3 for you, including the direct-to-storage
+`PUT` in step 2.
+
+<Callout type="warn">
+**This table used to list two routes that never worked (#4087).** `POST
+/storage/upload` and `GET /storage/file/:id` were a dispatcher-side bridge to
+the `file-storage` service, written against a service shape that does not
+exist: it called `upload(key, data, options?)` as `upload(file, { request })` —
+a `TypeError` against every implementation in the repo — and read the `Buffer`
+that `download(key)` resolves as if it were a `{ url | stream | mimeType }`
+descriptor. They stayed mounted and reachable — nothing shadowed them — so
+anyone who followed this table got a 500; everyone who followed the SDK used
+the protocol above and never touched them.
+
+Both routes are retired. `/storage` is service-storage's surface — install that
+package to get it. Discovery advertises the route only when the occupant of the
+`file-storage` slot actually mounts HTTP handlers, so an in-memory dev
+implementation now reports `handlerReady: false` and no `routes.storage`
+instead of pointing at a path with nothing behind it.
+</Callout>
 
 ---
 

docs/audits/2026-07-dispatcher-client-route-coverage.md

@@ -111,6 +111,37 @@ had zero call sites in `objectstack` and `objectui`.
   dispatcher's two plain routes remain a low-level redirect/stream compat
   surface, deliberately outside the SDK.
 
+### Amendment (#4087) — the storage half rested on its own bad premise
+
+The #3584 pass corrected one premise in this table and kept another it never
+checked: that the dispatcher's two storage routes were *a working low-level
+compat surface*. End-to-end verification found they were not, and never had
+been. `POST /storage/upload` called `upload(key, data, options?)` as
+`upload(file, { request })` — the file object landing in the `key` slot and
+`{ request }` in `data` — which is a `TypeError` against every implementation
+in the repo (`s3-storage-adapter`, `local-storage-adapter`,
+`swappable-storage-service`, plugin-dev's in-memory one). `GET
+/storage/file/:id` had the mirror-image defect one layer quieter: it branched
+on `result.url` / `result.redirect` / `result.stream` / `result.mimeType`
+while the contract's `download(key)` resolves a `Buffer`, so every branch fell
+through to "serialize the Buffer as JSON".
+
+Neither could serve a request. What kept that hidden was not shadowing —
+service-storage mounts `/storage/upload/presigned`, not `/storage/upload`, so
+both bridge routes stayed mounted and reachable in a full stack — but the
+absence of any caller: no SDK method, no console call — only the
+`content/docs/api/plugin-endpoints` table advertising them to readers. The
+tests that covered them mocked the shape the handler wanted rather than the
+shape `IStorageService` declares, so the suite stayed green on a pair of routes
+that answered 500.
+
+**Both routes are retired** (#4087) — the `/storage` domain, its
+dispatcher-plugin mounts, the hono adapter's `/storage/*` wildcard (which had
+been claiming the whole subtree for them) and the two ledger rows are gone.
+Read the two storage rows in §4 as retired rather than as the `server-only`
+disposition #3584 gave them: the surface belongs entirely to `service-storage`,
+enumerated in `packages/services/service-storage/src/storage-route-ledger.ts`.
+
 ## 5. Client-internal findings
 
 - **`trigger` vs `execute`** hit different URLs for the same intent

packages/adapters/hono/src/__mocks__/runtime.ts

@@ -7,7 +7,6 @@ export class HttpDispatcher {
   handleAuth = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: { ok: true } } });
   handleMetadata = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: { objects: [] } } });
   handleData = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: { records: [] } } });
-  handleStorage = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: {} } });
 
   constructor(_kernel: any) {}
 }

packages/adapters/hono/src/hono.test.ts

@@ -8,7 +8,6 @@ const mockDispatcher = {
   getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', endpoints: [] }),
   handleAuth: vi.fn().mockResolvedValue({ handled: true, response: { body: { ok: true }, status: 200 } }),
   handleGraphQL: vi.fn().mockResolvedValue({ data: {} }),
-  handleStorage: vi.fn().mockResolvedValue({ handled: true, response: { body: {}, status: 200 } }),
   dispatch: vi.fn().mockResolvedValue({ handled: true, response: { body: { success: true }, status: 200 } }),
 };
 
@@ -486,6 +485,29 @@ describe('createHonoApp', () => {
         '/api',
       );
     });
+
+    // [#4087] The retired `/storage/*` mount was a WILDCARD: it claimed the
+    // whole subtree for two dispatcher routes, so service-storage's protocol
+    // (`/storage/upload/presigned`, `/storage/files/:id/url`, …) — registered
+    // under the same prefix on the same server — could never be reached
+    // through this app. Storage is ordinary catch-all traffic now; nothing
+    // here parses formData or shadows the subtree.
+    it('POST /api/storage/upload/presigned reaches the catch-all, not a storage mount', async () => {
+      const res = await app.request('/api/storage/upload/presigned', {
+        method: 'POST',
+        body: JSON.stringify({ filename: 'a.txt' }),
+        headers: { 'Content-Type': 'application/json' },
+      });
+      expect(res.status).toBe(200);
+      expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
+        'POST',
+        '/storage/upload/presigned',
+        { filename: 'a.txt' },
+        expect.any(Object),
+        expect.objectContaining({ request: expect.anything() }),
+        '/api',
+      );
+    });
   });
 
   describe('Error Handling', () => {

packages/adapters/hono/src/index.ts

@@ -96,8 +96,8 @@ export function objectStackMiddleware(kernel: ObjectKernel) {
 /**
  * Creates a full-featured Hono app with all ObjectStack route dispatchers.
  *
- * Only routes that need framework-specific handling (auth service, storage
- * formData, GraphQL raw result, discovery wrapper) are registered explicitly.
+ * Only routes that need framework-specific handling (auth service, GraphQL
+ * raw result, discovery wrapper) are registered explicitly.
  * All other routes (meta, data, packages, analytics, automation, i18n, ui,
  * openapi, custom endpoints, and any future routes) are handled by a
  * catch-all that delegates to `HttpDispatcher.dispatch()`.
@@ -347,24 +347,18 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
     }
   });
 
-  // --- Storage (needs formData parsing) ---
-  app.all(`${prefix}/storage/*`, async (c) => {
-    try {
-      const subPath = c.req.path.substring(`${prefix}/storage`.length);
-      const method = c.req.method;
-
-      let file: any = undefined;
-      if (method === 'POST' && subPath === '/upload') {
-        const formData = await c.req.formData();
-        file = formData.get('file');
-      }
-
-      const result = await dispatcher.handleStorage(subPath, method, file, { request: c.req.raw });
-      return toResponse(c, result);
-    } catch (err: any) {
-      return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500);
-    }
-  });
+  // --- Storage: deliberately NOT mounted (#4087) ---
+  // This used to be `app.all(prefix + '/storage/*')` feeding
+  // `dispatcher.handleStorage`. Two things were wrong with it. The handler it
+  // called spoke a storage contract that does not exist (it passed the parsed
+  // file as the `key` argument of `upload(key, data, options?)`, and read the
+  // Buffer that `download(key)` resolves as a `{ url | stream }` descriptor) —
+  // retired with this change. And the wildcard was wider than the two routes
+  // it served: it claimed the WHOLE `/storage` subtree, so every other path
+  // under it — the presigned / chunked / signed-URL protocol `service-storage`
+  // registers, above all — was answered by the bridge's own 404 instead of
+  // falling through to anything else this app might mount. Storage is ordinary
+  // catch-all traffic now, like every other domain.
 
   // ─── Catch-all: delegate to dispatcher.dispatch() ─────────────────────────
   // Handles meta, data, packages, analytics, automation, i18n, ui, openapi,

packages/metadata-protocol/src/protocol.ts

@@ -1480,10 +1480,13 @@ export class ObjectStackProtocolImplementation implements
         // registers the service (service-storage's own `/api/v1/storage`
         // routes, plugin-search, plugin-graphql, …) rather than to a dispatcher
         // domain, so `handlerReady` there says nothing about whether THAT route
-        // is mounted, and suppressing it would be a guess. `file-storage` is
-        // listed because the dispatcher does own a `/storage` bridge for the
-        // no-plugin case; when service-storage is installed it registers a real
-        // (unmarked) service, so the entry never fires.
+        // is mounted, and suppressing it would be a guess. `file-storage` stays
+        // listed as the one entry with no dispatcher domain behind it — #4087
+        // retired the `/storage` bridge — because for that slot `handlerReady`
+        // is not a proxy for anything, it is the fact itself: an occupant that
+        // mounts no HTTP surface must not have `/api/v1/storage` advertised on
+        // its behalf. When service-storage is installed it registers a real
+        // (unmarked) service and mounts the routes, so the entry never fires.
         const DISPATCHER_GATED_SERVICES = new Set([
             'analytics', 'automation', 'notification', 'ai', 'i18n', 'file-storage',
         ]);

packages/plugins/plugin-dev/src/dev-plugin.test.ts

@@ -275,7 +275,11 @@ describe('DevPlugin', () => {
 
     // No HTTP/WS surface is mounted for these, so no handler can be ready —
     // independent of the implementation being real (ADR-0076 D12, realtime).
-    for (const name of ['cache', 'queue', 'job', 'realtime']) {
+    // `file-storage` joined them in #4087: the dispatcher's `/storage` bridge
+    // was the only thing that ever routed HTTP to this slot, and it was
+    // retired — `@objectstack/service-storage` owns that surface and mounts
+    // its own routes against its own adapters, never this implementation.
+    for (const name of ['cache', 'queue', 'job', 'realtime', 'file-storage']) {
       expect(readServiceSelfInfo(registeredServices.get(name))?.handlerReady, `${name} handlerReady`).toBe(false);
     }
 

packages/plugins/plugin-dev/src/dev-plugin.ts

@@ -295,14 +295,18 @@ const DEV_STUB_FACTORIES: Record<string, () => Record<string, any>> = {
  *
  * `handlerReady` answers a different question: does an HTTP handler genuinely
  * serve this? It stays `false` for every `stub`, and also for `degraded`
- * services with no HTTP surface at all (`realtime` — the D12 precedent).
+ * services with no HTTP surface at all (`realtime` — the D12 precedent, joined
+ * by `file-storage` in #4087: the dispatcher's `/storage` bridge — the one
+ * thing that ever routed HTTP to this slot — was retired, and the surface it
+ * claimed belongs to `@objectstack/service-storage`, which mounts its own
+ * routes and does not use this implementation).
  *
  * Entries are only needed for plugin-dev's OWN fakes. The wrapped kernel
  * fallbacks (`cache` / `queue` / `job` / `i18n`) already carry their own
  * `__serviceInfo`, and {@link applySelfInfo} never overwrites one.
  */
 const DEV_STUB_SELF_INFO: Record<string, { status: 'stub' | 'degraded'; handlerReady?: boolean; message: string }> = {
-  'file-storage': { status: 'degraded', message: 'Dev in-memory file storage — really stores and returns bytes, but nothing survives a restart. Register a storage plugin for durable files.' },
+  'file-storage': { status: 'degraded', handlerReady: false, message: 'Dev in-memory file storage — really stores and returns bytes to in-process callers, but nothing survives a restart and no HTTP surface is mounted. Register @objectstack/service-storage for durable files and the upload/download protocol.' },
   'search':       { status: 'degraded', message: 'Dev in-memory index — real substring matching over indexed documents, no ranking, analyzers, or persistence. Register a search plugin for the real engine.' },
   'metadata':     { status: 'degraded', message: 'Dev in-memory metadata registry — real reads and writes, no persistence. Register MetadataPlugin for a persisted registry.' },
   'workflow':     { status: 'degraded', message: 'Dev in-memory workflow state — transitions are recorded and read back, but NOTHING is validated: every transition is accepted and availableTransitions is always empty.' },

packages/runtime/src/dispatcher-plugin.error-envelope.test.ts

@@ -4,7 +4,7 @@
  * #3867 — the dispatcher-plugin error exit.
  *
  * `errorResponseBase` is the single error exit for EVERY route this plugin
- * mounts (`/analytics`, `/packages`, `/i18n`, `/storage`, `/automation`,
+ * mounts (`/analytics`, `/packages`, `/i18n`, `/automation`,
  * `/auth`, `/notifications`, `/mcp`, …): each handler catches and calls it
  * rather than re-throwing. Two defects lived there, and neither is visible
  * until a real error actually reaches it: