Skip to content

Commit a5c4e86

Browse files
authored
feat(vnext): add private parser worker protocol (#180)
## Summary - add a strict private parser worker wire protocol and browser worker endpoint - load only literal PostgreSQL and BigQuery parser modules inside the isolated worker - fail closed on malformed messages, unsafe realms, overlap, and poisoned parser descriptors - add Node and Chromium coverage plus package artifact assertions ## Validation - 1,159 Node tests passed with one intentional expected failure - 4 Chromium worker tests passed - typecheck, oxlint, test integrity, demo, package smoke, worker placement, and changed coverage passed - changed production coverage: 99.46% statements/lines and 100% branches/functions - exact-head adversarial review approved by architecture/security, lifecycle/session, and packaging/browser reviewers ## Scope Private infrastructure only: no public exports, session wiring, semantic extraction, cache, or API compatibility commitment. ## Review policy Copilot review is requested once for this PR. It will not be re-requested after later pushes. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Adds a private, versioned wire protocol and a dedicated module-worker endpoint for `node-sql-parser` to run parsing in an isolated browser worker. The endpoint now guards the entire backend operation and uses stricter, bounded wire decoding; artifacts are production-shaped but still private. - **New Features** - Closed protocol v1 for parse requests/responses; no AST, source text, or raw errors on the wire; retryability derived from failure code. - Dedicated worker endpoint that lazily loads `bigquery`/`postgresql`, reuses the backend normalizer, and restores exact `NodeSQLParser`/`global` descriptors around module load, decode, parser construction, parse, and normalization. - Single-flight only: one in-flight request; malformed or overlapping requests emit a protocol error and then close. - Outcomes include normalized statement kind, bounded unsupported reasons, or bounded failure codes. - **Bug Fixes** - Guarded the full worker backend parse and permanently poison on restoration failure; close the generation on `module-load` failures. - Bounded and hardened wire decoding: accept only closed plain records, enforce key limits, and avoid accessor traps. <sup>Written for commit e06a42f. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/180?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 6e0e234 commit a5c4e86

12 files changed

Lines changed: 2803 additions & 5 deletions

docs/adr/0004-isolated-parser-execution.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,9 @@ node-sql-parser/build/bigquery.js
116116

117117
The worker verifies that `self === globalThis` and that no DOM window exists.
118118
It snapshots and restores the exact `NodeSQLParser` and `global` descriptors
119-
around dialect loading. Cleanup failure poisons that worker generation.
119+
around the complete backend operation: dialect loading, module decoding,
120+
parser construction, parsing, and output normalization. Cleanup failure
121+
poisons that worker generation.
120122

121123
### Private wire protocol
122124

@@ -139,7 +141,7 @@ The initial response contains only one closed outcome:
139141
- Parsed normalized statement kind
140142
- Syntax rejection
141143
- Bounded unsupported reason
142-
- Bounded failure code plus retryability
144+
- Bounded failure code; retryability is derived from that code
143145

144146
Messages do not contain:
145147

@@ -150,6 +152,11 @@ Messages do not contain:
150152
- Absolute document ranges
151153
- Raw ASTs or generic payload bags
152154

155+
The wire does not transport an independently supplied retryability boolean.
156+
The host treats only `module-load` as retryable; `backend` and
157+
`malformed-output` are terminal. A module-load failure closes the current
158+
worker generation so a retry cannot reuse a rejected dynamic-import realm.
159+
153160
The host requires the current protocol version and correlation ID, validates
154161
all keys and closed values, and copies accepted data into new frozen objects.
155162
It then constructs an authentic `SqlParserAnalysis` with the exact pending

docs/vnext/node-sql-parser-adapter.md

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,45 @@ exists, or `global` does not resolve exactly to `globalThis`, before loading a
5959
bundle. This blocks browser windows and Node DOM shims from exposing an
6060
unguarded secondary target. Pure Node loads restore the exact prior descriptors
6161
synchronously after module evaluation, including removing names that were
62-
previously absent. Cleanup failure permanently poisons loading. A
63-
dedicated-worker loader remains future work.
62+
previously absent. Cleanup failure permanently poisons loading. The dedicated
63+
worker applies the same exact restoration rule around the complete backend
64+
operation, including module evaluation, module decoding, parser construction,
65+
parsing, and output normalization.
66+
67+
### Private browser worker endpoint
68+
69+
The package contains a production-shaped but private module-worker endpoint.
70+
It is not exported from the package and is not reachable through `/vnext`.
71+
There is no public worker constructor, executor, queue, language-service
72+
module, or session integration yet.
73+
74+
The endpoint:
75+
76+
- uses only the extension-qualified PostgreSQL and BigQuery builds above;
77+
- loads each grammar lazily after a valid request;
78+
- reuses the realm-neutral backend engine for module, AST, and parser-error
79+
normalization;
80+
- accepts and emits only a closed, versioned plain-data protocol;
81+
- returns normalized statement kind, bounded unsupported or failure evidence,
82+
and never returns source text, raw errors, or backend ASTs;
83+
- derives retryability from the closed failure code instead of trusting a
84+
separate wire flag;
85+
- restores the exact prior `NodeSQLParser` and `global` descriptors around
86+
each complete backend operation; and
87+
- permanently poisons and closes its worker realm if cleanup cannot be proven
88+
exact.
89+
90+
The endpoint accepts only one request at a time. Overlap and malformed messages
91+
fail closed instead of creating an implicit worker-side queue. The future
92+
service-owned executor is responsible for serialization, correlation,
93+
deadlines, cancellation, generation replacement, and disposal.
94+
95+
Direct Chromium tests construct this source module worker and exercise both
96+
real grammar builds. The separate worker-placement fixture remains
97+
diagnostic packaging evidence: it records resource timing, emitted chunk
98+
reachability, and bundle sizes with a fixture-owned protocol. It is not the
99+
public integration boundary and must not be read as evidence that an executor
100+
or session API already exists.
64101

65102
Approximate local Node 24 arm64 measurements for the installed package were:
66103

scripts/changed-coverage.mjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ const changedProductionFiles = execFileSync(
3434
!path.endsWith(".test.ts") &&
3535
!path.includes("/__tests__/") &&
3636
!path.includes("/browser_tests/") &&
37-
path !== "src/debug.ts",
37+
path !== "src/debug.ts" &&
38+
path !== "src/vnext/node-sql-parser-browser-worker.ts",
3839
);
3940
const changedRuntimeFiles = (
4041
await Promise.all(

scripts/package-smoke.mjs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,21 @@ try {
169169
if (typeof packedPackage.dependencies?.["node-sql-parser"] !== "string") {
170170
throw new Error("Packed manifest does not declare node-sql-parser");
171171
}
172+
const privateWorkerArtifacts = [
173+
"dist/vnext/node-sql-parser-browser-worker.d.ts",
174+
"dist/vnext/node-sql-parser-browser-worker.js",
175+
"dist/vnext/node-sql-parser-browser-worker-endpoint.d.ts",
176+
"dist/vnext/node-sql-parser-browser-worker-endpoint.js",
177+
"dist/vnext/node-sql-parser-wire.d.ts",
178+
"dist/vnext/node-sql-parser-wire.js",
179+
];
180+
for (const artifact of privateWorkerArtifacts) {
181+
if (!existsSync(join(packageDirectory, artifact))) {
182+
throw new Error(
183+
`Packed archive omitted private worker artifact ${artifact}`,
184+
);
185+
}
186+
}
172187

173188
writeFileSync(
174189
join(temporaryDirectory, "vnext-consumer.mjs"),

0 commit comments

Comments
 (0)