Skip to content

Commit d39d908

Browse files
authored
feat(vnext): coordinate bounded catalog search work (#195)
## Summary - add the package-private bounded catalog search-work coordinator - compose provider authentication, epoch authority, latest-wins ownership, cancellation, deadlines, and response publication - harden hostile and reentrant lifecycle boundaries, including exact disposal hooks and captured Promise intrinsics - document the scheduler contract and deferred cache/session/UI responsibilities in ADR 0005 - add strict API fixtures, invariant-heavy unit coverage, and capacity/performance benchmarks ## Invariants - at most 8 provider searches are active and 64 are queued - owners join only exact structural request keys and supersede independently - scope epoch changes retire same-scope work without crossing scope boundaries - failed captures, invalid input, cancellation, disposal, overload, and deadlines settle promptly and exactly once - provider calls, timers, clocks, thenables, callbacks, and response decoding may be hostile or reentrant without reviving retired work - the authenticated dialect runtime is the sole dialect identity authority ## Validation - `vitest`: 1,856 passed, 1 expected failure - changed coverage: 97.29% statements, 96.23% branches, 99.46% functions, 97.73% lines - search coordinator: 97.03% statements, 95.09% branches, 100% functions, 97.84% lines - epoch coordinator: 98.31% statements, 97.42% branches, 98.21% functions, 98.89% lines - strict source, tests, vNext API fixture, loose-optional fixture, and demo typechecks - zero-warning `oxlint` - test-integrity gate - 4 browser files / 7 browser tests - exact-tarball package smoke test - worker placement and bundle budgets - production dependency audit: no known vulnerabilities - catalog search-work benchmark suite - two independent adversarial exact-head approvals Part of #169. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Adds a package-private, bounded catalog search-work coordinator that composes provider auth with epoch authority to run searches safely, dedupe by exact keys, and enforce deadlines. Also centralizes catalog scope validation to a shared boundary utility. Progress toward #169. - **New Features** - Added `createSqlCatalogSearchWorkCoordinator` with 8 active and 64 queued limits, exact-key in-flight sharing, and latest-wins per owner. - Owners capture scope and dialect; the authenticated dialect runtime defines the provider ID and epoch authority. - Independent cancellation, queue and execution deadlines, and a small synchronous budget; outcomes include usable, superseded, cancelled, and unavailable. - Safe response decoding and epoch publication; first baseline re-keys unobserved work in the same scope. - Benchmarks and strict unit/type tests for lifecycle, concurrency, deadlines, and adversarial scenarios. - ADR 0005 updated to document scheduler/deadlines and package-owned disposal semantics. - **Refactors** - Epoch coordinator now accepts a package-owned disposal target, invokes it exactly once, and drains non-undefined returns with captured Promise intrinsics (resilient to a replaced global Promise). - Centralized scope validation into `isValidSqlCatalogScope` in the boundary; adopted by epoch and search-work paths with new tests. - Hardened lifecycle boundaries and cleanup; exact disposal and hostile thenable handling. - Dialect runtime exposes a stable `id`; tests assert coherence and deep-freeze properties. <sup>Written for commit 061a3c6. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/195?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
1 parent 5ad22fd commit d39d908

11 files changed

Lines changed: 5859 additions & 43 deletions

docs/adr/0005-parser-independent-relation-completion.md

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,12 @@ state preparation and suppresses later revision listeners; an accidental
484484
Promise result receives best-effort detached rejection draining. Coordinator
485485
disposal revokes the preparation closure before external cleanup. The hook
486486
remains package-private and is not a provider or session extension point.
487+
The combined search coordinator also installs one package-owned disposal
488+
target. Epoch self-quarantine makes the outer coordinator inert before
489+
subscription cleanup continues, so search work cannot outlive its epoch
490+
authority. The target is receiver-free, invoked at most once, and its failure
491+
cannot reopen disposal. It is synchronously exact-return and must return
492+
`undefined`; any other runtime result is detached and rejection-drained.
487493

488494
A search that discovers a higher epoch supersedes itself instead of publishing
489495
against its older captured revision. Pages and cache entries from different
@@ -555,7 +561,9 @@ callback can always create a poisoned or independent unhandled rejection that
555561
no JavaScript library can retroactively contain. Legitimate asynchronous
556562
teardown must consume or report its own failure before the cleanup closure
557563
returns `undefined`. The valid `undefined` path returns without Promise
558-
allocation or a microtask. A closure avoids a structural TypeScript
564+
allocation or a microtask. Detached assimilation uses module-captured Promise
565+
intrinsics, so replacing the global Promise constructor cannot disable the
566+
drain. A closure avoids a structural TypeScript
559567
contract that would accept class instances or inherited methods which the
560568
hostile runtime boundary could not safely validate. Dropping the last owner
561569
removes the complete scope incarnation before cleanup. A later join creates a
@@ -654,6 +662,8 @@ outcomes settle through a discriminated request result.
654662
Completion is latest-wins per session:
655663

656664
- a new completion request supersedes the previous request;
665+
- a new request that cannot capture an active epoch still supersedes and
666+
detaches the previous request before reporting its unavailable outcome;
657667
- same-key supersession atomically attaches the new request consumer, or
658668
retags the existing observer as that consumer, before removing old request
659669
ownership; different-key supersession revokes the old observer first;
@@ -744,6 +754,25 @@ refresh notification and leaves the already-returned incomplete result valid.
744754
No optional catalog promise can keep `complete()` pending indefinitely or
745755
block the local baseline past its product response budget.
746756

757+
The first implementation increment of this section is intentionally
758+
package-private. It combines an authenticated provider with the epoch
759+
coordinator and owns the fixed 8-active/64-queued scheduler, exact-key
760+
in-flight sharing, one latest-wins consumer per owner, independent
761+
cancellation, absolute queue and execution deadlines, response decoding, and
762+
epoch publication. An owner captures its scope and dialect when prepared, so
763+
individual requests cannot substitute provider, scope, dialect, or epoch
764+
authority. The authenticated dialect runtime owns its canonical provider ID;
765+
callers cannot pair an unrelated ID and runtime. Establishing the first
766+
baseline re-keys other joinable unobserved work in that scope, allowing
767+
newly-observed consumers to join it without duplicating a provider call.
768+
769+
Cache entries, loading/retry policy, refresh observers and their leases, the
770+
40 ms completion-response budget, pagination composition, ranking, session
771+
composition, and CodeMirror integration do not belong to that increment. They
772+
remain explicit follow-up increments; the coordinator must not expose a
773+
premature public surface that makes those deferred semantics difficult to add
774+
or test.
775+
747776
The exact structural cache and shared-work key contains:
748777

749778
- service-owned provider configuration identity and unique provider ID;

src/vnext/__tests__/relation-catalog-boundary.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
createSqlCatalogSearchRequest,
66
decodeSqlCatalogInvalidation,
77
decodeSqlCatalogSearchResponse,
8+
isValidSqlCatalogScope,
89
MAX_CATALOG_CONTINUATION_TOKEN_LENGTH,
910
MAX_CATALOG_DETAIL_LENGTH,
1011
MAX_CATALOG_ENTITY_ID_LENGTH,
@@ -345,6 +346,29 @@ describe("relation catalog provider capture", () => {
345346
});
346347
});
347348

349+
describe("catalog scope validation", () => {
350+
it("accepts only bounded, non-NUL, well-formed text", () => {
351+
expect(isValidSqlCatalogScope("connection:primary")).toBe(true);
352+
expect(
353+
isValidSqlCatalogScope(
354+
`\ud83d\ude80${"x".repeat(MAX_CATALOG_SCOPE_LENGTH - 2)}`,
355+
),
356+
).toBe(true);
357+
for (const candidate of [
358+
null,
359+
1,
360+
"",
361+
"bad\0scope",
362+
"\ud800",
363+
"\ud800x",
364+
"\udc00",
365+
"x".repeat(MAX_CATALOG_SCOPE_LENGTH + 1),
366+
]) {
367+
expect(isValidSqlCatalogScope(candidate)).toBe(false);
368+
}
369+
});
370+
});
371+
348372
describe("catalog search request snapshots", () => {
349373
it("copies and recursively freezes the exact provider request", () => {
350374
const raw = request();

src/vnext/__tests__/relation-catalog-epoch-coordinator.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,108 @@ describe("catalog epoch coordinator construction and membership", () => {
236236
expect(reads).toBe(0);
237237
});
238238

239+
it("validates, drains, and invokes the package disposal target exactly once", async () => {
240+
expect(
241+
Reflect.apply(createSqlCatalogEpochCoordinator, undefined, [
242+
capturedProvider(),
243+
undefined,
244+
1,
245+
]),
246+
).toEqual({
247+
reason: "invalid-disposal-target",
248+
status: "unavailable",
249+
});
250+
251+
let disposalCalls = 0;
252+
const created = createSqlCatalogEpochCoordinator(
253+
capturedProvider(),
254+
undefined,
255+
() => {
256+
disposalCalls += 1;
257+
throw new Error("package disposal target failed");
258+
},
259+
);
260+
expect(created.status).toBe("created");
261+
if (created.status !== "created") {
262+
throw new Error("Expected a coordinator fixture");
263+
}
264+
expect(() => {
265+
created.coordinator.dispose();
266+
created.coordinator.dispose();
267+
}).not.toThrow();
268+
expect(disposalCalls).toBe(1);
269+
270+
const invalidReturn = Promise.reject(
271+
new Error("invalid async package disposal"),
272+
);
273+
const withInvalidReturn = Reflect.apply(
274+
createSqlCatalogEpochCoordinator,
275+
undefined,
276+
[
277+
capturedProvider(),
278+
undefined,
279+
() => invalidReturn,
280+
],
281+
);
282+
expect(withInvalidReturn.status).toBe("created");
283+
if (withInvalidReturn.status !== "created") {
284+
throw new Error("Expected a coordinator fixture");
285+
}
286+
withInvalidReturn.coordinator.dispose();
287+
await Promise.resolve();
288+
await Promise.resolve();
289+
});
290+
291+
it("drains a plain thenable with captured Promise intrinsics after the global constructor is replaced", async () => {
292+
const intrinsicPromise = Promise;
293+
let thenCalls = 0;
294+
const thenable = new Proxy(
295+
{},
296+
{
297+
get(target, property, receiver): unknown {
298+
if (property === "then") {
299+
return (
300+
_resolve: (value: unknown) => void,
301+
reject: (reason?: unknown) => void,
302+
): void => {
303+
thenCalls += 1;
304+
reject(new Error("detached plain thenable"));
305+
};
306+
}
307+
return Reflect.get(target, property, receiver);
308+
},
309+
},
310+
);
311+
function HostilePromise(): never {
312+
throw new Error("mutable global Promise was used");
313+
}
314+
expect(
315+
Reflect.set(globalThis, "Promise", HostilePromise),
316+
).toBe(true);
317+
try {
318+
const created = Reflect.apply(
319+
createSqlCatalogEpochCoordinator,
320+
undefined,
321+
[
322+
capturedProvider(),
323+
undefined,
324+
() => thenable,
325+
],
326+
);
327+
expect(created.status).toBe("created");
328+
if (created.status !== "created") {
329+
throw new Error("Expected a coordinator fixture");
330+
}
331+
const createdCoordinator = created.coordinator;
332+
expect(() => createdCoordinator.dispose()).not.toThrow();
333+
} finally {
334+
Reflect.set(globalThis, "Promise", intrinsicPromise);
335+
}
336+
await intrinsicPromise.resolve();
337+
await intrinsicPromise.resolve();
338+
expect(thenCalls).toBe(1);
339+
});
340+
239341
it("validates exact bounded well-formed scopes without raw errors", () => {
240342
const owner = coordinator();
241343
expect(

0 commit comments

Comments
 (0)