Skip to content

Commit de6daa5

Browse files
os-zhuangclaude
andauthored
fix(runtime)!: the /share-links dispatcher domain stops emitting a duplicate link/links beside data (#4038) (#4049)
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 — `{ success: true, data: link, link }` and `{ success: true, data: links, 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 cloud sweep is what #4038 was waiting on, and it came back clean. `data` is unchanged on both routes — only the duplicate key is gone, so anything reading `body.data` or going through `unwrapResponse` sees no difference. 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. (My own issue text said to route BOTH through the helper; that was wrong about the status.) `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. Not touching objectui's `body.links ?? body.data`: with both producers converged that is version skew, not drift, and dropping it would turn "old framework" from working into a silently empty share list. Deleting it needs a version gate, which is a larger call than this issue. Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z Co-authored-by: Claude <noreply@anthropic.com>
1 parent b3a2318 commit de6daa5

3 files changed

Lines changed: 126 additions & 2 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime)!: the /share-links dispatcher domain stops emitting a duplicate `link`/`links` beside `data` (#4038)
6+
7+
The producer-side other half of #3983. That PR moved the sharing plugin's routes
8+
onto the declared envelope; this removes the compatibility shim the dispatcher
9+
twin had been carrying *because* that surface answered bare.
10+
11+
Create and list answered with the payload under **two** keys:
12+
13+
```ts
14+
{ success: true, data: link, link } // POST /share-links
15+
{ success: true, data: links, links } // GET /share-links
16+
```
17+
18+
The duplicate existed so readers predating the envelope kept working — which is
19+
why objectui's `ShareDialog` reads `body.links ?? body.data`. Once #3983 made both
20+
surfaces answer `data`, that first branch had no producer left, and the duplicate
21+
had no reader in **any** repo:
22+
23+
- **framework** — no consumer of these routes at all
24+
- **objectui**`ShareDialog` already falls through to `body.data`
25+
- **cloud** — swept: it only *registers* `SharingServicePlugin` into per-environment
26+
kernels with `registerShareLinkRoutes: false` so this dispatcher serves the paths.
27+
It never calls them and never reads a body. That sweep is what #4038 was waiting
28+
on, and it came back clean.
29+
30+
## Shape
31+
32+
| route | was | now |
33+
|---|---|---|
34+
| `POST /share-links` | `{ success, data: link, link }` | `{ success, data: link }` |
35+
| `GET /share-links` | `{ success, data: links, links }` | `{ success, data: links }` |
36+
37+
`data` is unchanged in both — only the duplicate key is gone. Anything reading
38+
`body.data`, or going through `ObjectStackClient.unwrapResponse`, sees no
39+
difference. A raw reader of the top-level `body.link` / `body.links` must move to
40+
`body.data`.
41+
42+
The list route now routes through `deps.success(...)` like the domain's other
43+
three. Create stays hand-built, because `deps.success` hardcodes status 200 and
44+
this route is a **201** — the same reason `/keys` hand-builds its own 201, and the
45+
same shape it uses.
46+
47+
## Guard
48+
49+
`scripts/check-route-envelope.mjs` does not and cannot cover this file: it scans
50+
route modules that write via `res.json(...)`, while dispatcher domains return
51+
`{ status, body }` for a central sender. So the drift was invisible to it by
52+
construction. Three tests in `domain-handler-registry.test.ts` cover it instead —
53+
two per-route, plus a general one asserting no success body carries a top-level
54+
key outside `success` / `data` / `meta`. Restoring the duplicates fails all three.

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,62 @@ describe('HttpDispatcher extracted domains (PR-4: share-links)', () => {
357357
expect(record?.secret).toBeUndefined();
358358
});
359359

360+
// ── #4038: one shape, no duplicate keys ──────────────────────────────
361+
//
362+
// Create and list used to answer `{ success: true, data: link, link }` /
363+
// `{ …, data: links, links }` — the payload under BOTH the envelope's `data`
364+
// and a legacy top-level key. That shim existed because the sharing plugin's
365+
// routes (the other surface for these same paths) answered bare, so every
366+
// consumer had to read `body.links ?? body.data`. #3983 moved that surface
367+
// onto this shape and left the duplicate with no reader anywhere — framework,
368+
// objectui, or cloud — so it is gone. These pin that: a body that grows a
369+
// second spelling of its payload fails here.
370+
const authed: any = { executionContext: { userId: 'u1' } };
371+
const LINK = { id: 'l1', token: 't1', object_name: 'account', record_id: 'r1' };
372+
373+
it('POST /share-links answers 201 { success, data } — data IS the link, no top-level `link`', async () => {
374+
const shareLinks = { createLink: vi.fn().mockResolvedValue(LINK) };
375+
const result = await makeDispatcher({ shareLinks })
376+
.handleShareLinks('', 'POST', { object: 'account', recordId: 'r1' }, {}, authed);
377+
expect(result.response?.status).toBe(201);
378+
expect(result.response?.body?.success).toBe(true);
379+
expect(result.response?.body?.data).toMatchObject({ id: 'l1', token: 't1' });
380+
expect(result.response?.body?.link).toBeUndefined();
381+
});
382+
383+
it('GET /share-links answers { success, data } — data IS the array, no top-level `links`', async () => {
384+
const shareLinks = { listLinks: vi.fn().mockResolvedValue([LINK]) };
385+
const result = await makeDispatcher({ shareLinks }).handleShareLinks('', 'GET', undefined, {}, authed);
386+
expect(result.response?.status).toBe(200);
387+
expect(result.response?.body?.success).toBe(true);
388+
expect(Array.isArray(result.response?.body?.data)).toBe(true);
389+
expect(result.response?.body?.data?.[0]).toMatchObject({ id: 'l1' });
390+
expect(result.response?.body?.links).toBeUndefined();
391+
});
392+
393+
it('every success body carries its payload under `data` and nowhere else', async () => {
394+
// The general form of the two assertions above, over all four success
395+
// routes: no key beside the envelope's own may hold the payload.
396+
const ENVELOPE_KEYS = new Set(['success', 'data', 'meta']);
397+
const shareLinks = {
398+
createLink: vi.fn().mockResolvedValue(LINK),
399+
listLinks: vi.fn().mockResolvedValue([LINK]),
400+
revokeLink: vi.fn().mockResolvedValue(undefined),
401+
};
402+
const d = makeDispatcher({ shareLinks });
403+
const bodies = [
404+
(await d.handleShareLinks('', 'POST', { object: 'account', recordId: 'r1' }, {}, authed)).response?.body,
405+
(await d.handleShareLinks('', 'GET', undefined, {}, authed)).response?.body,
406+
(await d.handleShareLinks('/l1', 'DELETE', undefined, {}, authed)).response?.body,
407+
];
408+
for (const body of bodies) {
409+
expect(body?.success).toBe(true);
410+
for (const key of Object.keys(body ?? {})) {
411+
expect(ENVELOPE_KEYS.has(key), `body carries a non-envelope top-level key: ${key}`).toBe(true);
412+
}
413+
}
414+
});
415+
360416
it('unmatched sub-path returns the standard ROUTE_NOT_FOUND envelope', async () => {
361417
const shareLinks = { resolveToken: vi.fn(), createLink: vi.fn(), listLinks: vi.fn(), revokeLink: vi.fn() };
362418
const context: any = { executionContext: { userId: 'u1' } };

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@
2323
* the authorisation. The underlying record is fetched with a SYSTEM
2424
* context (per-env RLS is bypassed because the token gates access), and
2525
* `redactFields` are stripped before the record leaves the server.
26+
*
27+
* Every route answers the declared `{ success: true, data }` envelope with `data`
28+
* carrying the payload directly. Create and list used to emit a duplicate
29+
* top-level `link` / `links` beside `data` — a producer-side shim for readers
30+
* predating the envelope, kept alive because the sharing plugin's routes (the
31+
* OTHER surface for these same paths) still answered bare. #3983 moved that
32+
* surface onto this shape, which left the duplicate with no reader in any repo —
33+
* framework, objectui, or cloud — so #4038 removed it. Both surfaces now emit one
34+
* shape, which is what lets `ObjectStackClient.unwrapResponse` return the same
35+
* value whichever one served the request.
2636
*/
2737

2838
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
@@ -191,7 +201,11 @@ export async function handleShareLinksRequest(
191201
},
192202
callerCtx,
193203
);
194-
return { handled: true, response: { status: 201, body: { success: true, data: link, link } } };
204+
// Hand-built rather than `deps.success(...)` for the 201 alone — that
205+
// helper hardcodes 200. Same shape the `/keys` domain builds for its
206+
// own 201, and nothing more: the duplicate top-level `link` this used
207+
// to carry beside `data` is gone (#4038).
208+
return { handled: true, response: { status: 201, body: { success: true, data: link } } };
195209
}
196210

197211
// GET /share-links?object&recordId → list the caller's own links
@@ -207,7 +221,7 @@ export async function handleShareLinksRequest(
207221
},
208222
callerCtx,
209223
);
210-
return { handled: true, response: { status: 200, body: { success: true, data: links, links } } };
224+
return { handled: true, response: deps.success(links) };
211225
}
212226

213227
// DELETE /share-links/:idOrToken → revoke

0 commit comments

Comments
 (0)