fix(runtime)!: the /share-links dispatcher domain stops emitting a duplicate link/links beside data (#4038)

.changeset/share-links-dispatcher-dual-key.md

@@ -0,0 +1,54 @@
+---
+"@objectstack/runtime": patch
+---
+
+fix(runtime)!: the /share-links dispatcher domain stops emitting a duplicate `link`/`links` beside `data` (#4038)
+
+The producer-side other half of #3983. That PR moved the sharing plugin's routes
+onto the declared envelope; this removes the compatibility shim the dispatcher
+twin had been carrying *because* that surface answered bare.
+
+Create and list answered with the payload under **two** keys:
+
+```ts
+{ success: true, data: link,  link  }   // POST /share-links
+{ success: true, data: links, links }   // GET  /share-links
+```
+
+The duplicate existed so readers predating the envelope kept working — which is
+why objectui's `ShareDialog` reads `body.links ?? body.data`. Once #3983 made both
+surfaces answer `data`, that first branch had no producer left, and the duplicate
+had no reader in **any** repo:
+
+- **framework** — no consumer of these routes at all
+- **objectui** — `ShareDialog` already falls through to `body.data`
+- **cloud** — swept: it only *registers* `SharingServicePlugin` into per-environment
+  kernels with `registerShareLinkRoutes: false` so this dispatcher serves the paths.
+  It never calls them and never reads a body. That sweep is what #4038 was waiting
+  on, and it came back clean.
+
+## Shape
+
+| route | was | now |
+|---|---|---|
+| `POST /share-links` | `{ success, data: link, link }` | `{ success, data: link }` |
+| `GET /share-links` | `{ success, data: links, links }` | `{ success, data: links }` |
+
+`data` is unchanged in both — only the duplicate key is gone. Anything reading
+`body.data`, or going through `ObjectStackClient.unwrapResponse`, sees no
+difference. A raw reader of the top-level `body.link` / `body.links` must move to
+`body.data`.
+
+The list route now routes through `deps.success(...)` like the domain's other
+three. Create stays hand-built, because `deps.success` hardcodes status 200 and
+this route is a **201** — the same reason `/keys` hand-builds its own 201, and the
+same shape it uses.
+
+## Guard
+
+`scripts/check-route-envelope.mjs` does not and cannot cover this file: it scans
+route modules that write via `res.json(...)`, while dispatcher domains return
+`{ status, body }` for a central sender. So the drift was invisible to it by
+construction. Three tests in `domain-handler-registry.test.ts` cover it instead —
+two per-route, plus a general one asserting no success body carries a top-level
+key outside `success` / `data` / `meta`. Restoring the duplicates fails all three.

packages/runtime/src/domain-handler-registry.test.ts

@@ -357,6 +357,62 @@ describe('HttpDispatcher extracted domains (PR-4: share-links)', () => {
         expect(record?.secret).toBeUndefined();
     });
 
+    // ── #4038: one shape, no duplicate keys ──────────────────────────────
+    //
+    // Create and list used to answer `{ success: true, data: link, link }` /
+    // `{ …, data: links, links }` — the payload under BOTH the envelope's `data`
+    // and a legacy top-level key. That shim existed because the sharing plugin's
+    // routes (the other surface for these same paths) answered bare, so every
+    // consumer had to read `body.links ?? body.data`. #3983 moved that surface
+    // onto this shape and left the duplicate with no reader anywhere — framework,
+    // objectui, or cloud — so it is gone. These pin that: a body that grows a
+    // second spelling of its payload fails here.
+    const authed: any = { executionContext: { userId: 'u1' } };
+    const LINK = { id: 'l1', token: 't1', object_name: 'account', record_id: 'r1' };
+
+    it('POST /share-links answers 201 { success, data } — data IS the link, no top-level `link`', async () => {
+        const shareLinks = { createLink: vi.fn().mockResolvedValue(LINK) };
+        const result = await makeDispatcher({ shareLinks })
+            .handleShareLinks('', 'POST', { object: 'account', recordId: 'r1' }, {}, authed);
+        expect(result.response?.status).toBe(201);
+        expect(result.response?.body?.success).toBe(true);
+        expect(result.response?.body?.data).toMatchObject({ id: 'l1', token: 't1' });
+        expect(result.response?.body?.link).toBeUndefined();
+    });
+
+    it('GET /share-links answers { success, data } — data IS the array, no top-level `links`', async () => {
+        const shareLinks = { listLinks: vi.fn().mockResolvedValue([LINK]) };
+        const result = await makeDispatcher({ shareLinks }).handleShareLinks('', 'GET', undefined, {}, authed);
+        expect(result.response?.status).toBe(200);
+        expect(result.response?.body?.success).toBe(true);
+        expect(Array.isArray(result.response?.body?.data)).toBe(true);
+        expect(result.response?.body?.data?.[0]).toMatchObject({ id: 'l1' });
+        expect(result.response?.body?.links).toBeUndefined();
+    });
+
+    it('every success body carries its payload under `data` and nowhere else', async () => {
+        // The general form of the two assertions above, over all four success
+        // routes: no key beside the envelope's own may hold the payload.
+        const ENVELOPE_KEYS = new Set(['success', 'data', 'meta']);
+        const shareLinks = {
+            createLink: vi.fn().mockResolvedValue(LINK),
+            listLinks: vi.fn().mockResolvedValue([LINK]),
+            revokeLink: vi.fn().mockResolvedValue(undefined),
+        };
+        const d = makeDispatcher({ shareLinks });
+        const bodies = [
+            (await d.handleShareLinks('', 'POST', { object: 'account', recordId: 'r1' }, {}, authed)).response?.body,
+            (await d.handleShareLinks('', 'GET', undefined, {}, authed)).response?.body,
+            (await d.handleShareLinks('/l1', 'DELETE', undefined, {}, authed)).response?.body,
+        ];
+        for (const body of bodies) {
+            expect(body?.success).toBe(true);
+            for (const key of Object.keys(body ?? {})) {
+                expect(ENVELOPE_KEYS.has(key), `body carries a non-envelope top-level key: ${key}`).toBe(true);
+            }
+        }
+    });
+
     it('unmatched sub-path returns the standard ROUTE_NOT_FOUND envelope', async () => {
         const shareLinks = { resolveToken: vi.fn(), createLink: vi.fn(), listLinks: vi.fn(), revokeLink: vi.fn() };
         const context: any = { executionContext: { userId: 'u1' } };

packages/runtime/src/domains/share-links.ts

@@ -23,6 +23,16 @@
  * the authorisation. The underlying record is fetched with a SYSTEM
  * context (per-env RLS is bypassed because the token gates access), and
  * `redactFields` are stripped before the record leaves the server.
+ *
+ * Every route answers the declared `{ success: true, data }` envelope with `data`
+ * carrying the payload directly. Create and list used to emit a duplicate
+ * top-level `link` / `links` beside `data` — a producer-side shim for readers
+ * predating the envelope, kept alive because the sharing plugin's routes (the
+ * OTHER surface for these same paths) still answered bare. #3983 moved that
+ * surface onto this shape, which left the duplicate with no reader in any repo —
+ * framework, objectui, or cloud — so #4038 removed it. Both surfaces now emit one
+ * shape, which is what lets `ObjectStackClient.unwrapResponse` return the same
+ * value whichever one served the request.
  */
 
 import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
@@ -191,7 +201,11 @@ export async function handleShareLinksRequest(
                 },
                 callerCtx,
             );
-            return { handled: true, response: { status: 201, body: { success: true, data: link, link } } };
+            // Hand-built rather than `deps.success(...)` for the 201 alone — that
+            // helper hardcodes 200. Same shape the `/keys` domain builds for its
+            // own 201, and nothing more: the duplicate top-level `link` this used
+            // to carry beside `data` is gone (#4038).
+            return { handled: true, response: { status: 201, body: { success: true, data: link } } };
         }
 
         // GET /share-links?object&recordId → list the caller's own links
@@ -207,7 +221,7 @@ export async function handleShareLinksRequest(
                 },
                 callerCtx,
             );
-            return { handled: true, response: { status: 200, body: { success: true, data: links, links } } };
+            return { handled: true, response: deps.success(links) };
         }
 
         // DELETE /share-links/:idOrToken → revoke