Skip to content
This repository was archived by the owner on Oct 26, 2026. It is now read-only.

Commit 6d8bf0c

Browse files
fix(app-shell,data-objectstack): concurrent identical Studio reads share one in-flight request (objectui#11797) (#11856)
Fixes #11797 Clause-②: no ## What changes Opening a Studio package mounts several readers that ask the same question at once, and each one sent its own request. A caller that arrives while an identical request is still pending now waits for that request instead of sending another: - **`fetchPackages`** (`@object-ui/app-shell`, `studio-design/packages-io.ts`): the package switcher, the writability courtesy gate and the `DataPillar` namespace lookup share one `GET /api/v1/packages`. - **`fetchPendingDrafts`** (`@object-ui/app-shell`, `preview/usePendingDrafts.ts`): the hook's mount read and the surface's own mount `refresh()` (Studio top bar, chat bar) share one `GET /api/v1/meta/_drafts` per scope. - **`MetadataClient`** (`@object-ui/data-objectstack`, `metadata-client.ts`): `listTypes`, `list`, `listDrafts` and `get` / `getDraft` share one GET per method, URL and headers. The sharing works across every client built on the same `fetch`, because the console creates one client per component, all on one authenticated fetch. Every caller, the first one included, gets an answer object no other caller holds: the last caller to be handed the answer takes the parsed original and every earlier one a `structuredClone` copy, so nothing is copied when nobody joined (patch round 1, `bab3004`). No call site changed. The `fetchPackages` call sites in `StudioDesignSurface.tsx` and `BuilderLanding.tsx` are untouched: the read is shared below them. ## Measured: the duplicates are concurrent mounts, and in-flight sharing alone removes them (H1) Requests are counted on the wire, through transports that hold every request open until the test answers it. **Before** is the ablation run: the three production files checked out at the base `455c646`, with the tests unchanged. | What | Before | After | |---|---|---| | The real `StudioDesignSurface` on the Data tab: `GET /api/v1/packages` | 3 | 1 | | The same render: `_drafts?packageId=…` | not reached (the packages assertion fails first) | 1 | | Two count surfaces (the hook plus their own mount refresh) on one package: `_drafts` | 4 | 1 | | Two `MetadataClient`s on one fetch, each calling `listDrafts`, `list` and `get` | 6 | 3 | | Three concurrent `fetchPackages()` calls | 3 | 1 | The before counts match the card's "packages ×3" and "_drafts ×4" readings. ## Design: in-flight sharing only, with no reuse window (H2) I cannot name every write that must clear a cached answer. An agent creates packages and stages drafts on the server, and the client never sees those writes. So, per H2, this keeps **only** the in-flight sharing, with no reuse window: - An entry is dropped the moment its request settles, so a call made after an answer always goes to the server. - A read already on the wire when a write lands was asked before that write, so a read asked after the write must not join it. The writes each reader can see drop the pending entry: - **`MetadataClient`:** every `save`, `publishDraft`, `publishPackageDrafts`, `reset`, `publish` and `rollback` through a client on the same `fetch` drops every pending read on that `fetch`. The drop happens when the write's response arrives, whether the write succeeded or failed. - **packages:** `duplicatePackage` (the same module) and the existing `objectui:packages-changed` announcement (create and edit, from `PackageFormDialog`). - **drafts:** the publish pulse (`emitMetadataRefresh`), which the module doc already names as the contract every publish path keeps. The drop runs before the hooks it wakes read again, so those hooks share **one** new request. - **Residual:** a write the reader cannot see (a package delete or disable through `PackagesPage`, a draft save while a raw `_drafts` read is pending, any server-side agent write) can still be answered by a read that was already on the wire. The window is one round trip, and the same residual applies to the adapter's existing `find` sharing. A read asked after that pending read settles always goes to the server. Failures are shared, not remembered (H3). A failed request rejects every caller that was waiting on it, and the next call retries. Tests cover an HTTP failure and a transport failure (`fetch` rejecting) for the metadata client. No exported signature changes (H4). There is no new export, method, option or type. `shareRead`, `sendWrite`, `readPackagesPayload` and `readDraftsPayload` are all module-private or `private`. ## `sys_user_preference`: no duplicate to remove (the premise does not hold for this family) The card reported three `GET /api/v1/data/sys_user_preference` reads "with the identical filter" on Home → Studio. I measured this through the real `ObjectStackAdapter` and client (`userState.inflightShare-11797.test.ts`). `UserStateBridge` attaches three adapters, one each for `ui.favorites`, `ui.recent` and `ui.flow.palette.recents`, and each one loads its own row. The result is three requests whose filters differ only in the `key` predicate, which sits at the tail of a long encoded query string. They are three different questions, so one read per key is already met. The real duplicate case is two loads of one `(user, key)` at once, and the adapter's existing `find` sharing already sends that as one request (pinned). **`userState.ts` is not changed.** ## Files - `packages/app-shell/src/views/studio-design/packages-io.ts` - `packages/app-shell/src/preview/usePendingDrafts.ts` - `packages/data-objectstack/src/metadata-client.ts` - `packages/data-objectstack/README.md`: the **Shared In-Flight Reads** bullet now covers `MetadataClient` (patch round 1) - Tests: `packages-io.inflightShare-11797.test.tsx`, `StudioDesignSurface.entryReadsOnce-11797.test.tsx` (a new test file beside `packages-io.ts`; `StudioDesignSurface.tsx` itself is not edited), `usePendingDrafts.inflightShare-11797.test.tsx`, `metadata-client.inflightShare-11797.test.ts`, `userState.inflightShare-11797.test.ts` - `.changeset/11797-shared-inflight-reads.md`: patch for `@object-ui/app-shell` and `@object-ui/data-objectstack` ## Verification Round 0 ran at `c3759d6`. Patch round 1 re-ran at `bab3004` (the final commit): new pins, 5 files / 32 tests passed; regression, the same 247-file set, 247 files / 2233 tests passed; app-shell and data-objectstack type-check exit 0. The figures below are round 0's. - **New pins** (5 files, run with `pnpm exec vitest run` from the repo root): `Test Files 5 passed (5)`, `Tests 27 passed (27)`. - **Ablation:** - Each production file was checked out at `455c646`, and on-disk markers confirmed the swap: the hash equals the BASE blob for all 3 files. - Result: `Test Files 4 failed | 1 passed (5)`, `Tests 12 failed | 15 passed (27)`. The `sys_user_preference` file stays green because it pins existing behaviour. - Restored with `git checkout HEAD --`: the hash equals the HEAD blob for all 3 files, and `git diff HEAD` is empty. After the restore: `Test Files 5 passed (5)`. - **Regression:** - Scope: every test file that imports a touched module or builds a `MetadataClient`, plus all of `packages/data-objectstack/` and `StudioRoute`. That is 247 files across app-shell, data-objectstack, plugin-designer, plugin-form, plugin-kanban, plugin-calendar, react and console. - Result: `Test Files 247 passed (247)`, `Tests 2228 passed (2228)`. - This is a declared narrowing. The full 8-shard `pnpm test` is left to CI. - **Builds and type-checks:** - `turbo run build --filter=@object-ui/app-shell^...`: exit 0. - `pnpm --filter @object-ui/data-objectstack type-check`: exit 0. - `pnpm --filter @object-ui/app-shell type-check`, which runs both `tsc --noEmit` and `tsc -p tsconfig.test.json` (the second includes `src/**/*.test.ts(x)`): exit 0. - **Lint:** `pnpm exec eslint` on the 8 touched source and test files: exit 0, with 0 errors and 9 warnings. All 9 warnings are on lines that were already there (`any` casts in `listTypes`, `setState` in the effect). - **Repo gates:** - `node scripts/check-changeset-presence.mjs`: `✅ 8 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)`. - `pnpm check:new-line-citations`: `VERDICT new-cross-file-line-citations: 0 new citation(s)`. - `pnpm check:control-bytes`: `✅ check-control-bytes: OK`. - Also exit 0: `check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:test-path-roots`, `check:metadata-write-doors`, `check:changeset-claims`, `check:pending-changeset-literals`, `check:side-effects-array`, `check:unreferenced-sources`, `check:esm-specifiers`, `check:handler-key-reads`, `check:phantom-deps`, `check:self-import`, `scripts/check-type-check-coverage.mjs`, and `check-governed-queue-guard --test` (NOT GOVERNED). - **NOT MEASURED:** - `check:published-dist`: it builds every package including the console app, which is a repo-wide run that CI owns. A local attempt was stopped by a 300 s timeout before it printed a verdict. - `check:dist-completeness --all` for app-shell: app-shell itself was not built locally (only its dependency closure was). The gate reported only that unbuilt package, and no other. ## Acceptance notes (not fixed here, not filed) - **README gap, resolved in patch round 1** (`bab3004`, seat verdict `6050853625`). The data-objectstack README bullet "Shared In-Flight Reads" describes only the adapter's `find` and `findOne`. A sentence about `MetadataClient` sharing is owed under AGENTS.md #2, but the README is outside this card's claimed file surface, so I left it for the seat. - **`find` is not dropped by writes.** `ObjectStackAdapter.emitMutation` drops the pending `findOne` reads of the resource it writes, but not the pending `find` reads. A list refetch asked after a write can therefore join a `find` that was sent before the write. This was found by reading the code, with no public-door reproduction. - **A second packages reader.** `ContextSelectors` (the app top bar) reads its options endpoint with its own `fetch`, separately from `fetchPackages`. It is not part of the Studio entry path. - **Not measured:** whether Home → Studio remounts `ConnectedShell` and so re-attaches the user-state bridge. That would explain why the three preference reads repeat on that navigation, but they are still one read per key. - **Hung requests.** A request that never settles keeps its joiners waiting. This is the same property as the adapter's existing `find` / `findOne` sharing. - **Overlap with objectui#11799.** That card is queued behind this one and touches the `?state=draft` probe. This PR changes only the transport under `readItemResponse` (now wrapped in `shareRead`); the probe logic is unchanged. The implementing session is `https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU` (seat `domain:ui#2`). --- _Generated by [Claude Code](https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cef0eee commit 6d8bf0c

10 files changed

Lines changed: 1302 additions & 58 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/data-objectstack': patch
4+
---
5+
6+
Opening a Studio package no longer sends the same read several times at once (objectui#11797).
7+
8+
Several readers mount together on Studio entry and each one used to send its own request for the same answer. A caller that arrives while an identical request is still pending now waits for that request instead of sending another:
9+
10+
- **`@object-ui/app-shell`:** the package list (`GET /api/v1/packages`, read by the Studio package switcher, the read-only gate and the object-name namespace lookup) and the pending-drafts count (`GET /api/v1/meta/_drafts`, read by the Studio top bar and the chat bar).
11+
- **`@object-ui/data-objectstack`:** `MetadataClient.listTypes`, `list`, `listDrafts`, `get` and `getDraft`. Requests are shared between clients built on the same `fetch`, so the per-component clients the console creates share them too. Each caller, the first one included, still gets an answer object that no other caller holds.
12+
13+
This is not a response cache. Once a request settles, the next call goes to the server again. A failed request is shared only by the callers already waiting, and the next call retries. A write the reader can see drops any read still pending, so a read made after the write never gets the answer of a request sent before it. For the metadata client that is any save, publish or reset through a client on the same `fetch`. For the app-shell reads it is a package duplicate, the `objectui:packages-changed` announcement and the publish refresh pulse.
14+
15+
No exported function, method signature, option or type changes.
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*
8+
* objectui#11797 — the pending-drafts ledger is read once per question, not
9+
* once per reader.
10+
*
11+
* Each surface that shows the count mounts `usePendingDrafts` (which reads on
12+
* mount) AND calls the hook's `refresh()` from a mount effect of its own: the
13+
* Studio topbar from its publish/draft-nonce effect, the chat bar from its
14+
* idle-edge effect. `Surface` below is that shape. With two such surfaces
15+
* open on one package, every one of those reads sent its own identical
16+
* `GET /api/v1/meta/_drafts` request.
17+
*
18+
* Counted on the wire: the stubbed `fetch` holds every request open until the
19+
* test answers it, so "at once" means the first request is genuinely pending
20+
* when the next read is asked.
21+
*
22+
* The rules pinned besides the one-request count:
23+
* - a different scope is a different question;
24+
* - the entry goes when the request settles, so a later refresh asks again
25+
* (no reuse window);
26+
* - a failure reaches every reader that shared it and is not remembered;
27+
* - a publish pulse drops the pending request, so the reads it triggers are
28+
* never answered by a request sent before the publish — and they share
29+
* ONE new request between them.
30+
*/
31+
import React from 'react';
32+
import { afterEach, describe, expect, it, vi } from 'vitest';
33+
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
34+
import { fetchPendingDrafts, usePendingDrafts } from './usePendingDrafts.js';
35+
import { emitMetadataRefresh } from '../assistant/assistantBus.js';
36+
37+
interface HeldRequest {
38+
url: string;
39+
respond: (status: number, body: unknown) => void;
40+
}
41+
42+
function heldFetch() {
43+
const held: HeldRequest[] = [];
44+
vi.stubGlobal(
45+
'fetch',
46+
vi.fn(
47+
(input: RequestInfo | URL) =>
48+
new Promise((resolve) => {
49+
held.push({
50+
url: String(input),
51+
respond: (status, body) =>
52+
resolve({ ok: status >= 200 && status < 300, status, json: async () => body }),
53+
});
54+
}),
55+
),
56+
);
57+
const reads = (url: string) => held.filter((r) => r.url === url);
58+
return { held, reads };
59+
}
60+
61+
const CRM = '/api/v1/meta/_drafts?packageId=app.crm';
62+
const draft = (name: string) => ({ type: 'object', name, packageId: 'app.crm' });
63+
64+
/**
65+
* A surface showing the count: the hook, plus its own mount refresh. The
66+
* button is the surface-local trigger (a draft saved, a turn gone idle).
67+
*/
68+
function Surface({ id, packageId }: { id: string; packageId: string | null }) {
69+
const { count, refresh } = usePendingDrafts({ packageId });
70+
React.useEffect(() => {
71+
void refresh();
72+
}, [refresh]);
73+
return (
74+
<>
75+
<span data-testid={id}>{count === null ? 'unknown' : String(count)}</span>
76+
<button type="button" data-testid={`${id}-refresh`} onClick={() => void refresh()} />
77+
</>
78+
);
79+
}
80+
81+
async function drain() {
82+
for (let i = 0; i < 5; i++) await new Promise((r) => setTimeout(r, 0));
83+
}
84+
85+
afterEach(() => {
86+
vi.unstubAllGlobals();
87+
});
88+
89+
describe('usePendingDrafts — one request per question, not per reader (objectui#11797)', () => {
90+
it('two surfaces opening one package put one `_drafts` request on the wire, and both show its count', async () => {
91+
const { held, reads } = heldFetch();
92+
render(
93+
<>
94+
<Surface id="topbar" packageId="app.crm" />
95+
<Surface id="chatbar" packageId="app.crm" />
96+
</>,
97+
);
98+
await act(drain);
99+
expect(reads(CRM)).toHaveLength(1);
100+
expect(held).toHaveLength(1);
101+
102+
await act(async () => reads(CRM)[0].respond(200, { drafts: [draft('crm_account'), draft('crm_contact')] }));
103+
await waitFor(() => expect(screen.getByTestId('topbar').textContent).toBe('2'));
104+
expect(screen.getByTestId('chatbar').textContent).toBe('2');
105+
});
106+
107+
it('a caller editing its entries in its own continuation leaves another caller untouched', async () => {
108+
// Every caller maps the shared payload itself into new entries that hold
109+
// only primitives, so no caller is handed anything that points into the
110+
// payload or into another caller's list.
111+
const { reads } = heldFetch();
112+
const first = fetchPendingDrafts('app.crm').then((mine) => {
113+
mine[0].name = 'edited-by-first';
114+
mine.push({ type: 'view', name: 'extra', packageId: null });
115+
return mine;
116+
});
117+
const second = fetchPendingDrafts('app.crm');
118+
await drain();
119+
expect(reads(CRM)).toHaveLength(1);
120+
reads(CRM)[0].respond(200, { drafts: [draft('crm_account')] });
121+
122+
expect((await first).map((d) => d.name)).toEqual(['edited-by-first', 'extra']);
123+
expect(await second).toEqual([{ type: 'object', name: 'crm_account', packageId: 'app.crm' }]);
124+
});
125+
126+
it('keeps different scopes apart', async () => {
127+
const { held, reads } = heldFetch();
128+
render(
129+
<>
130+
<Surface id="pkg" packageId="app.crm" />
131+
<Surface id="org" packageId={null} />
132+
</>,
133+
);
134+
await act(drain);
135+
expect(reads(CRM)).toHaveLength(1);
136+
expect(reads('/api/v1/meta/_drafts')).toHaveLength(1);
137+
expect(held).toHaveLength(2);
138+
});
139+
140+
it('is not a response cache: a refresh after the answer asks the server again', async () => {
141+
const { reads } = heldFetch();
142+
render(<Surface id="topbar" packageId="app.crm" />);
143+
await act(drain);
144+
await act(async () => reads(CRM)[0].respond(200, [draft('crm_account')]));
145+
await waitFor(() => expect(screen.getByTestId('topbar').textContent).toBe('1'));
146+
147+
await act(async () => {
148+
fireEvent.click(screen.getByTestId('topbar-refresh'));
149+
await drain();
150+
});
151+
expect(reads(CRM)).toHaveLength(2);
152+
await act(async () => reads(CRM)[1].respond(200, []));
153+
await waitFor(() => expect(screen.getByTestId('topbar').textContent).toBe('0'));
154+
});
155+
156+
it('a failed request leaves every reader that shared it at "unknown", and is not remembered', async () => {
157+
const { reads } = heldFetch();
158+
render(
159+
<>
160+
<Surface id="topbar" packageId="app.crm" />
161+
<Surface id="chatbar" packageId="app.crm" />
162+
</>,
163+
);
164+
await act(drain);
165+
expect(reads(CRM)).toHaveLength(1);
166+
await act(async () => reads(CRM)[0].respond(503, { success: false }));
167+
await act(drain);
168+
expect(screen.getByTestId('topbar').textContent).toBe('unknown');
169+
expect(screen.getByTestId('chatbar').textContent).toBe('unknown');
170+
171+
await act(async () => {
172+
fireEvent.click(screen.getByTestId('topbar-refresh'));
173+
fireEvent.click(screen.getByTestId('chatbar-refresh'));
174+
await drain();
175+
});
176+
expect(reads(CRM)).toHaveLength(2);
177+
await act(async () => reads(CRM)[1].respond(200, [draft('crm_account')]));
178+
await waitFor(() => expect(screen.getByTestId('topbar').textContent).toBe('1'));
179+
expect(screen.getByTestId('chatbar').textContent).toBe('1');
180+
});
181+
182+
it('a publish pulse while a read is pending sends ONE new request, and its answer wins', async () => {
183+
const { reads } = heldFetch();
184+
render(
185+
<>
186+
<Surface id="topbar" packageId="app.crm" />
187+
<Surface id="chatbar" packageId="app.crm" />
188+
</>,
189+
);
190+
await act(drain);
191+
expect(reads(CRM)).toHaveLength(1);
192+
193+
// Published while the mount read is still on the wire: the reads the pulse
194+
// triggers must not join it, and must not each send their own either.
195+
await act(async () => {
196+
emitMetadataRefresh();
197+
await drain();
198+
});
199+
expect(reads(CRM)).toHaveLength(2);
200+
201+
await act(async () => {
202+
reads(CRM)[0].respond(200, [draft('crm_account'), draft('crm_contact')]);
203+
reads(CRM)[1].respond(200, []);
204+
await drain();
205+
});
206+
await waitFor(() => expect(screen.getByTestId('topbar').textContent).toBe('0'));
207+
expect(screen.getByTestId('chatbar').textContent).toBe('0');
208+
});
209+
});

‎packages/app-shell/src/preview/usePendingDrafts.ts‎

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,74 @@ export interface PendingDraftEntry {
4141
packageId: string | null;
4242
}
4343

44+
/**
45+
* The `_drafts` requests currently on the wire, per `fetch` and URL
46+
* (objectui#11797).
47+
*
48+
* Opening a package asked the same ledger several times at once: this hook
49+
* reads on mount, and the surfaces that mount it (the Studio topbar, the chat
50+
* bar) also call `refresh()` from a mount effect of their own. A caller that
51+
* arrives while a request for the same URL is pending now waits for that
52+
* request instead of sending another.
53+
*
54+
* ⛔ Not a response cache, deliberately:
55+
* - the entry goes the moment the request settles, so a call made after an
56+
* answer always reaches the network. There is no reuse window because the
57+
* ledger changes on writes this module never sees (a draft save, an
58+
* agent's server-side staging), and a window would answer the read after
59+
* such a write with the count from before it;
60+
* - a failure reaches the callers already waiting and is then forgotten;
61+
* - the publish pulse (`emitMetadataRefresh`, the contract every publish path
62+
* keeps — see the module doc) drops the pending requests before the hooks
63+
* it wakes read again, so their read is never answered by a request sent
64+
* before the publish.
65+
*
66+
* Keyed by the `fetch` the request went through: a request only answers
67+
* callers of the same transport. Each caller parses the shared payload itself,
68+
* so every caller still gets its own entry list.
69+
*/
70+
const pendingDraftReads = new WeakMap<typeof fetch, Map<string, Promise<unknown>>>();
71+
let publishPulseObserved = false;
72+
73+
function readDraftsPayload(url: string): Promise<unknown> {
74+
// Registered on first use rather than at import, and so before the hook's
75+
// own pulse subscription (the hook reads first, then subscribes): the drop
76+
// runs before the refresh that pulse triggers.
77+
if (!publishPulseObserved) {
78+
publishPulseObserved = true;
79+
subscribeMetadataRefresh(() => pendingDraftReads.get(globalThis.fetch)?.clear());
80+
}
81+
const transport = globalThis.fetch;
82+
let reads = pendingDraftReads.get(transport);
83+
if (!reads) {
84+
reads = new Map();
85+
pendingDraftReads.set(transport, reads);
86+
}
87+
const pending = reads.get(url);
88+
if (pending) return pending;
89+
const read = (async () => {
90+
const res = await fetch(url, {
91+
credentials: 'include',
92+
headers: { Accept: 'application/json' },
93+
cache: 'no-store',
94+
});
95+
if (!res.ok) throw new Error(`_drafts HTTP ${res.status}`);
96+
return res.json() as Promise<unknown>;
97+
})();
98+
reads.set(url, read);
99+
const owner = reads;
100+
const settle = () => {
101+
if (owner.get(url) === read) owner.delete(url);
102+
};
103+
read.then(settle, settle);
104+
return read;
105+
}
106+
44107
export async function fetchPendingDrafts(
45108
packageId?: string | null,
46109
): Promise<PendingDraftEntry[]> {
47110
const qs = packageId ? `?packageId=${encodeURIComponent(packageId)}` : '';
48-
const res = await fetch(`/api/v1/meta/_drafts${qs}`, {
49-
credentials: 'include',
50-
headers: { Accept: 'application/json' },
51-
cache: 'no-store',
52-
});
53-
if (!res.ok) throw new Error(`_drafts HTTP ${res.status}`);
54-
const data = (await res.json()) as
111+
const data = (await readDraftsPayload(`/api/v1/meta/_drafts${qs}`)) as
55112
| Array<Record<string, unknown>>
56113
| { drafts?: Array<Record<string, unknown>>; data?: { drafts?: Array<Record<string, unknown>> } };
57114
const list = Array.isArray(data) ? data : (data?.drafts ?? data?.data?.drafts ?? []);

0 commit comments

Comments
 (0)