Skip to content

fix(webapi): correct native binding contracts [Codex] - #399

Open
jderochervlk wants to merge 24 commits into
agent/alpha-tooling-releasefrom
agent/alpha-binding-contracts
Open

jderochervlk wants to merge 24 commits into
agent/alpha-tooling-releasefrom
agent/alpha-binding-contracts

Conversation

@jderochervlk

@jderochervlk jderochervlk commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

This stacked batch corrects native API representations, receiver dispatch, callback arity, and nullable/empty result contracts for #386–#392. The changes preserve native throws and rejections and avoid implicit copies: byte readers, Blob.stream chunks, and Request/Response body-stream adapters use Uint8Array; Web Audio and matrix APIs use native typed arrays, and native output methods write into buffers supplied by callers.

The batch also corrects ResizeObserver argument shape; WebCrypto result types and public typed workflows; CustomElementRegistry constructor, broad closest, and HTML/SVG className contracts; and the empty, nullable, static, and operation-specific result cases.

For WebCrypto, generic algorithm-dependent generation/export returns unknown; precise HMAC-key and ECDSA-key-pair helpers fix their native result shapes. algorithmString and algorithmObject provide raw/native AlgorithmIdentifier input without copying, and the public algorithm dictionary represents operation-specific parameters (including AES-GCM IV/AAD/tag length, AES-CBC/CTR, PBKDF2/HKDF fields, ECDH public key, and RSA-PSS saltLength). Binary and JWK export use disjoint format types. Packed and Chromium workflows cover HMAC/ECDSA and AES-GCM generate/import/sign/verify/encrypt/decrypt/digest-to-byte-view/export without unsafe casts. Negative consumers isolate the former JSON result assumptions, wrong algorithms, formats, and stream chunk types.

Native regressions use local fixtures: WebSocket text/ArrayBuffer/Blob sends reach a real loopback echo server; a real worker uses both explicit-init POST and omitted-init GET against a loopback HTTP server that echoes method/header/body; ResizeObserver observes an actual resize and disconnects; Request/Response/Blob bytes and stream chunks, SafeFetch bytes/identity, OfflineAudioContext buffers/filters, non-identity DOMMatrix conversions, custom elements/closest/SVG, and #392 native edge cases are asserted.

The browser runner defaults to loopback port 4173, never reuses an existing server, rejects invalid/occupied ports, and supports parallel worktrees with WEBAPI_TEST_PORT=4192 npm run test:browser. SharedWorkerScope and ServiceWorkerScope inherited Request-fetch bindings are compile-checked; the native Request-fetch run uses the base Worker scope.

This PR is stacked on foundation PR #397 at agent/alpha-tooling-release head ebe4fcac0e9ca68eb4b62f7209be8cf2e173fa95, whose tree matches the independently reviewed foundation tree. It does not duplicate the harness, packaging, or workflow foundation. The port runbook note is included here because isolated-port behavior is part of this binding PR; documentation PR #398 is downstream.

Fixes #386
Fixes #389
Refs #383
Refs #387
Refs #388
Refs #390
Refs #391
Refs #392

Downstream PR #398 completes the remaining documentation/example acceptance for the referenced issues and owns their closing references, including #383 after the test and contributor-documentation slices are complete. No issues are manually closed by this PR.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Independent review of 5ce03b4, against 1a6aceb (#397 foundation). Not acceptance-complete; no approval/merge performed. All seven requested checks pass, but the following P2 regression/evidence defects need attention:

  1. The closest negative tests nullability, not narrowing. ClosestCannotNarrow.res:1–2 assigns Null.t<…> directly to a nonnullable input. In an isolated package copy, restoring the old Null.t<'e> signature still produces the expected diagnostic. Change the target to Null.t<WebAPI.HTMLInputElement.t> (or unwrap first) and match the element-type diagnostic. The corrected nullable fixture compiles with the old signature and rejects at this head.
  2. Worker initializer forwarding is not asserted. worker-fetch.mjs:5–7 reads a static file from a method-insensitive server. Browser interception that drops {method: "POST"} leaves the entire binding fixture passing. Assert the method/headers/body received by a local endpoint and cover omitted init separately.
  3. Biquad assertions do not establish any output write. BindingContracts__test.res:144–153 starts with zero-filled outputs and only checks types/lengths/!isNaN(0). Removing getFrequencyResponse through browser interception still passes the complete fixture. Use sentinels and assert meaningful magnitude/phase values in both caller-owned buffers.
  4. The WebSocket test does not exercise a native local connection. websocket-bindings.spec.mjs:6–8 uses routeWebSocket without connecting to a server; Playwright replaces WebSocket with its mock. Add the local native WebSocket endpoint required by Fix receiver dispatch for WebSocket sends and worker Request fetches #386.

Additional acceptance gaps beyond the draft's acknowledged stream-chunk, shared/service-worker runtime, IIR call, and permission-value gaps:

  • ResizeObserver only observes the initial element; no resize-triggered callback is tested.
  • Matrix evidence uses identity input and does not exercise DOMMatrixReadOnly.fromFloat32Array; strengthen nonidentity round trips. Float analyser runtime evidence is absent.
  • Correct WebCrypto result types and verify key workflows #390 lacks registered negative tests for former JSON/key-result assumptions and cross-format exports, and no positive Crypto-only packed workflow is registered. I independently compiled/ran public-only packed HMAC + ECDSA generate/sign/verify, digest-to-byte-view, and raw/JWK export successfully; wrong-format exports both reject. The generators hard-code HMAC/ECDSA, so their successful result shapes are sound.
  • Correct DOM constructor, narrowing, and SVG className contracts #391 lacks HTML className/browser coverage, a broad-selector string-access negative, and migration documentation.
  • Represent native empty, nullable, and operation-specific results accurately #392's generic raw predicates do not enforce callback nullability or delete-result types; add typed/negative fixtures for these, empty enums, and the removed Notification instance field.

Checks executed successfully: npm run build, npm test, npm run check:features, npm run format:check, npm run test:negative, npm run test:browser (3 tests), npm run check:consumer. Mutation experiments used only scratch package copies/browser interception; repository sources were not edited.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Independent re-review of exact head d938c71, against pinned foundation 6fa3016 (not the subsequently moved base-branch tip). Reviewed the full source diff and #386–#392. No confirmed source-runtime defect found, but one false-positive negative fixture remains; not fully acceptance-complete. No approval, merge, or source edits performed.

P2 — IndexedDB negative does not protect the corrected result type. IDBDeleteResultIsNotDatabase.res:4–5 assigns request.result to string. In isolated packed copies, both the current unit result and the old idbDatabase result reject with the registered This has type: diagnostic. Replace this with actual database use, e.g. request.result->WebAPI.IDBDatabase.close, and match the operation-specific diagnostic. I verified that replacement rejects this head and compiles with only the old delete signature restored. Native deletion's undefined assertion itself passes.

The prior closest false positive is fixed: the current nullable expected type rejects this head and compiles with only the old polymorphic signature restored. The real worker/local HTTP test checks POST/header/body; WebSocket text/ArrayBuffer/Blob reach the real ws loopback server with awaited child teardown. Biquad/IIR sentinel checks now detect writes: independently replacing either native method with a no-op fails the browser fixture. ResizeObserver's induced second observation/disconnect, Uint8Array stream chunk, exhaustive Notification permission, HTML/SVG behavior, nonidentity matrix conversions, audio buffers, and nullable/default cases all execute successfully.

Crypto's fixed HMAC/ECDSA generators and disjoint export formats have sound successful result shapes; generic results remain unknown. Beyond the registered packed compilation and Chromium workflows, I compiled and ran the public-only Crypto packed consumer in isolation: HMAC and ECDSA verification true, digest/raw export 32 bytes, JWK kty: "oct". A typed algorithm-record extension with safe coercion also compiled and ran AES-GCM; the smaller shared record is not, by itself, a blocker. Remaining literal acceptance gap for #390: the registered negatives cover algorithms/formats, not the former JSON result assumptions. Migration-documentation acceptance remains dependent on downstream #398; not reviewed here.

After npm ci, independently passed: npm run build, npm run format:check, npm run test:negative, npm test, npm run check:features, npm run test:browser (4 tests), WEBAPI_TEST_PORT=4192 npm run test:browser (4 tests), npm run check:consumer, and npm run build && npm run build:docs. I also reran the occupied-port experiment: with an unrelated HTTP 200 endpoint on 4192, the exact browser command exits 1 before tests instead of reusing it. GitHub CI build and Release validate are now SUCCESS. Shared/service-worker paths are compile-checked only; runtime worker evidence is the base Worker path.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Cross-stack #383 assessment at binding head d938c719b016e439aeeec3da2d185dcd0b4e714f (not an approval): paired with foundation #397 head 1aadc66d4d30d1b1bf44394ca35509682060c03c, the explicit test categories/discovery checks, Node assertions, compile-only/documentation examples, browser failure reporting, and negative diagnostic harness are in place. This PR adds native Chromium regressions, 15 binding-specific diagnostic-matched negative fixtures (plus the harness sentinel and the packed Fetch-only SafeFetch rejection), packed Crypto consumer coverage, local WebSocket and HTTP-worker flows, typed byte/Float32Array operations, ResizeObserver resize/cleanup, non-identity matrices, and native API contracts. reuseExistingServer: false plus validated WEBAPI_TEST_PORT and configurable baseURL close the local cross-worktree stale-server risk; the author reports default and port-4192 passes plus occupied/invalid-port rejection.

GitHub CI build and Release validate passed on d938. One remaining test-case question from the prior independent review: the Worker.fetchWithRequest browser spec verifies the explicit POST initializer path; I did not find a separate omitted-init runtime case in the current diff. I asked the binding author whether that behavior is covered elsewhere or should be added. The final independent review of d938 is still pending, so this is a provisional acceptance assessment, not final approval or issue closure.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Independent review of d938c71, against 6fa3016 (agent/alpha-tooling-release). I read #383 and #386–#392, their acceptance criteria, and the previous review. Recommend changes; this snapshot is not acceptance-complete. No approval, merge, source edits, publication, or settings changes performed.

Actionable findings

  1. P2 — The new closed Crypto algorithm record excludes valid native operations. SubtleCrypto.res:20–26, WebCryptoTypes.res:74–81. The replacement algorithm record only supports name, hash, length, and namedCurve. It cannot express required AES-GCM/CBC iv, AES-CTR counter, RSA-PSS saltLength, PBKDF2/HKDF parameters, or ECDH public. This affects encrypt/decrypt/sign/verify/deriveKey/unwrapKey parameters changed from algorithmIdentifier = unknown, beyond the result-type correction. In an isolated packed package, an AES-GCM parameter record passed as Unknown(params) fails at this head; restoring only encrypt's previous unknown parameter makes the identical public consumer compile. The current closed record cannot replace that consumer because it has no iv. Preserve an honest raw/native parameter path or expose operation-specific public parameter types; keep the fixed HMAC/ECDSA helpers separate. Add a packed consumer/native regression for a parameterized operation, not just HMAC/ECDSA.

  2. P2 — The IndexedDB negative fixture does not enforce the corrected delete result. IDBDeleteResultIsNotDatabase.res:4–5. It assigns request.result to string, not to a database. Both the corrected unit and the old idbDatabase result reject, and both satisfy the registered generic This has type: matcher. I restored the old return type in a scratch package and the registered negative still rejected. Instead, try a database-only public operation such as request.result->WebAPI.IDBDatabase.close, and match the intended unit/database diagnostic. That version rejects at this head and compiles with the old return type in the same scratch package. A positive unit result fixture would complement it.

  3. P2 — Static Notification permission is still not runtime-tested. BindingContracts__test.res:76–78. Every arm of the exhaustive switch returns (). The compiler eliminates the switch and the external property read entirely: the emitted browser module has no Notification/permission reference. I replaced Notification.permission with a counting getter that throws on access; the entire fixture still passed with zero reads. Use an observable assertion on the returned permission value, checking membership in default/denied/granted or comparison with an independent native read. This is a remaining Represent native empty, nullable, and operation-specific results accurately #392 acceptance gap, not a defect in the static accessor itself.

Prior findings and native evidence

  • Closest: corrected. The target is now Null.t<HTMLInputElement.t>, isolating element widening from nullability. Scratch mutation to the former Null.t<'e> signature makes the fixture compile; the pinned head rejects it.
  • Worker Request fetch: corrected receiver dispatch and meaningful forwarding evidence. The real base Worker receives POST|forwarded|binding-request-body from a loopback HTTP server. Browser-only mutations dropping the entire initializer, method, header, or body respectively produced GET||, GET|forwarded|, POST||binding-request-body, and POST|forwarded|, all distinguishable from the expected response. SharedWorkerScope and ServiceWorkerScope inherited members are compile-covered; they are not runtime-tested. A successful omitted-init case is not separately registered.
  • WebSocket: corrected. Text, ArrayBuffer, and Blob go through a real ws loopback echo server; there is no routeWebSocket substitution.
  • Audio: corrected output-write evidence. Individually replacing Biquad/IIR getFrequencyResponse, analyser byte/float time-domain methods, and AudioBuffer copy-to/copy-from with no-ops caused assertion failures at the corresponding output checks. The filters now use sentinels. Source bindings dispatch directly into caller buffers, without hidden output copies.
  • ResizeObserver: now waits for the initial delivery, changes size from 10 to 20, waits for a second delivery, uses the supplied observer, disconnects, and removes the target.
  • Matrices: nonidentity Float32Array inputs exercise both DOMMatrix and DOMMatrixReadOnly factories, with meaningful round-trip component assertions. Both assertions currently use the read-only output binding; the changed mutable DOMMatrix.toFloat32Array is not separately executed.
  • DOM contracts: real constructor registration/construction, nullable broad closest, HTML string className, SVGAnimatedString/baseVal, and HTML/SVG broad selector values are covered; broad Element string-className access is negatively compiled. Generic classList/attribute bindings remain intact.
  • Fetch/Blob/SafeFetch: native Request/Response/Blob bytes and Blob stream Uint8Array chunks are asserted. SafeFetch asserts response identity and byte representation/content; its implementation still returns the native body and preserves errors as values, without implicit array conversion.
  • Represent native empty, nullable, and operation-specific results accurately #392 other cases: Canvas and Locks nullable callbacks have targeted negatives and native null-path tests; delete runtime result is checked as undefined; empty variants are added in both referrer-policy definitions, Request destination, and canPlayType; Notification instance permission is negatively rejected. Request default-string checks are raw runtime checks rather than typed enum fixtures.
  • Crypto workflows: positive Crypto-only packed consumer compiles; Chromium executes HMAC and ECDSA generate/sign/verify, key-pair use, digest-to-byte-view, and raw/JWK export. Fixed generators hard-code algorithms and disjoint export formats reject cross-format calls. However, Correct WebCrypto result types and verify key workflows #390's explicitly requested negatives for former JSON/key-result assumptions are still absent; current Crypto negatives cover extra algorithm arguments and export formats, not those former result assumptions.

Isolation, checks, and remaining acceptance

All requested commands passed serially: npm ci; npm run build; npm run format:check; npm run test:negative; npm test; npm run check:features; npm run test:browser (4 tests); WEBAPI_TEST_PORT=4192 npm run test:browser (4 tests); npm run check:consumer; npm run build; npm run build:docs. Local environment: Node 24.16.0, npm 11.15.0. Docs build had nonfatal i18n/404 warnings. GitHub CI/build and Release/validate are green at the pinned head; I did not independently rerun check:release or publication dry-runs.

A live unrelated server occupying port 4192 made Playwright exit 1 rather than reuse it. Invalid ports 0, 65536, and invalid also exited 1 with the intended validation diagnostic. All test endpoints bind loopback; worker/WS servers select ephemeral ports, tests kill and await child exit in finally, and no fixture-server processes remained after validation. URL-only example.invalid constructions do not issue public-network requests.

Documentation acceptance is also deferred, not met by this snapshot: no migration guidance for the #391 broad closest/HTML-vs-SVG className boundary is included, and #390's new public workflows lack usage/migration documentation beyond tests and signatures. Downstream docs PR #398 may address this, but it is not evidence for the pinned tree.

All mutation experiments used scratch package copies or browser interception/overrides only. The repository HEAD and tracked worktree/index remained unchanged and clean.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Supplemental independent review of exact head f5f7d33, against exact base 283591a (agent/alpha-tooling-release). Two of my three prior P2s are resolved; Notification runtime coverage is not. Two additional test defects remain. This is not an approval or a full re-review of unrelated scope.

Remaining actionable findings

  1. P2 — Four new Crypto JSON-result negatives reject on input mismatch, not the output contract. The ~algorithm annotations use WebAPI.WebCryptoTypes.algorithm, a record, while the functions now accept algorithmIdentifier = unknown: sign:5–9, digest:1–5, verify:1–7, generate:4–9. In a packed scratch package I restored only each operation's old promise<JSON.t> output. All four fixtures still rejected, with algorithmIdentifier (unknown) vs algorithm as the incompatible part; the generic registered diagnostic accepts this. Changing those annotations to the pre-existing WebCryptoTypes.algorithmIdentifier made all four compile with old JSON outputs restored. Use the actual input type and match the return-type diagnostic. The fifth fixture, generic export, does isolate its output: it rejects at head and compiles when only its old JSON output is restored.

  2. P2 — The prior Notification runtime-test finding remains unresolved. BindingContracts__test.res:76–78 still has the exhaustive switch whose arms all return (). The compiler removes the entire property read. At this exact head the emitted module contains no Notification.permission; installing a counting getter that throws still lets the entire fixture pass with zero reads. Make the permission value participate in an observable assertion, not a no-op exhaustive switch.

  3. P2 — The new omitted-init worker coverage has a response-order race. worker-fetch.spec.mjs:35–40,52–58 launches explicit-init POST and omitted-init GET concurrently, collects completion order, then asserts POST first. Async worker listeners do not serialize fetch completion. Keeping the real local HTTP endpoint and delaying only POST request delivery by 150ms produces ["GET||", "POST|forwarded|binding-request-body"]: both bindings work, but the assertion fails. Await the first result before sending the second, or correlate results by case rather than arrival order. Both cases do reach the same real worker and server; the semantic coverage is present once ordering is fixed.

Resolved prior findings and requested confirmations

  • AES-GCM closed-record finding: resolved. WebCryptoTypes.res:74–97 preserves the raw/native unknown input path, while SubtleCrypto.res:17–18,24–40 provides identity helpers and accepts that path. My original raw Unknown(params) AES-GCM consumer now compiles. The expanded dictionary covers the requested IV/AAD/tag, CTR, PBKDF2/HKDF, ECDH public mapping, and RSA-PSS fields. Packed AES-GCM import/encrypt/decrypt:62–78 compiles, and Chromium executes the corresponding native round trip and checks decrypted ABC bytes. The wider input path does not introduce output unsoundness: fixed results remain ArrayBuffer/bool/CryptoKey; generic generate/export remain unknown; fixed HMAC/ECDSA helpers retain their algorithm-fixed result shapes. Identity helpers do not insert copies.
  • IndexedDB negative: resolved. IDBDeleteResultIsNotDatabase.res:4–5 now passes the delete result to the database-only close operation. It rejects at the pinned head specifically because unit is not a database. Restoring only the old database result type makes the exact fixture compile.
  • Port documentation placement: confirmed. testing.mdx:55–64 is an addition in the diff against exact base 283591a, including the 4192 command and fail-closed explanation.

Validation boundary

Ran serially against a git archive scratch snapshot of f5f7d33: npm ci, npm run build, npm run test:negative, npm run test:browser (4 passed), and npm run check:consumer. All passed. These green checks do not detect the mutation-proven issues above. This supplemental pass did not rerun format, Node tests, feature builds, alternate-port tests, docs build, or release validation. Package mutations and browser overrides were isolated to scratch/browser contexts. The attached repository remained clean at its original d938 HEAD; no repository sources, GitHub settings, approvals, merges, or publications were changed.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Supplemental independent review of dd9f1c4, against exact base 283591a (agent/alpha-tooling-release). All three findings from my f5f7d33 supplemental review are resolved. No additional actionable defect found within this narrow re-review scope. This is not a GitHub approval or a new full acceptance review of every issue.

Confirmed corrections, with counterfactual evidence

  1. Crypto JSON-result negatives now isolate their result contracts. The annotations now use the actual, pre-existing WebCryptoTypes.algorithmIdentifier input for sign:4–9, digest:1–5, verify:1–7, and generic generate:4–9. Generic export:1–5 retains its correct pre-existing keyFormat input. I independently compiled all five in an installed scratch package: each rejects at this head specifically for ArrayBuffer/bool/unknown versus JSON, and each exact fixture compiles when only that operation's old JSON result is restored. No input annotations or new helper types were needed to make the mutations compile.

  2. Notification permission is now observably read and validated. BindingContracts__test.res:77–79 retains the static read in emitted JavaScript and checks membership in the allowed native permission strings using the predicate at lines 36–37. A counting getter returning an invalid string now causes an assertion failure at line 79 with exactly one read. A throwing getter also fails after exactly one read. The ordinary native fixture passes.

  3. Worker results no longer depend on completion order. worker-fetch.spec.mjs:35–66 correlates explicit-init POST and omitted-init GET by id; worker failures reject the test. I executed the exact spec's page-evaluation body against its real worker and local HTTP server, delaying only POST delivery by 150ms. Arrival order was default, then initialized; the resulting keyed object still matched both expected responses. This resolves the reproduced race without weakening method/header/body or omitted-init assertions.

Retained prior corrections

  • IndexedDB negative remains correct: IDBDeleteResultIsNotDatabase.res:4–5 passes the delete result to the database-only close operation. Independently rechecked: unit-versus-database rejection at head; restoring only the old database result makes the exact fixture compile.
  • AES-GCM finding remains resolved without output-shape unsoundness: WebCryptoTypes.res:74–97 preserves an open raw/native input path through unknown, alongside the expanded convenience dictionary. The dictionary itself is still a closed record; the raw path is what keeps additional native dictionaries usable. The original Unknown(params) AES-GCM consumer compiles. The packed typed AES-GCM import/encrypt/decrypt consumer compiles and Chromium's native AES round trip passes. Identity helpers and fixed-result operations do not insert copies or turn algorithm-dependent outputs into unjustified precise results: generic generate/export remain unknown, fixed outputs remain precise, and HMAC/ECDSA generators still hard-code algorithms to justify their result shapes.
  • The testing port paragraph remains an addition in the binding diff versus exact base 283591a, at testing.mdx:55–64.

Checks and scope

Ran serially in a scratch archive of exact dd9f1c4: npm ci, npm run build, npm run format:check, npm run test:negative, npm run test:browser (4 passed), and npm run check:consumer. All passed. Also checked diff whitespace against the exact base. This supplemental pass did not rerun Node tests, feature builds, alternate-port/isolation checks, docs build, or release validation, nor revisit downstream migration-documentation acceptance.

All counterfactual package changes and browser overrides were scratch-only. The attached repository remained clean and unchanged at its original d938 HEAD. No approval, merge, publication, settings changes, or repository source edits were performed.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Narrow final-snapshot supplement for fb9d51c, against foundation ebe4fca. No new actionable finding in the requested content/ancestry scope. The dd9 fixes remain intact by exact content identity. No approval/merge performed.

  • Foundation ebe4fca and the originally reviewed foundation 6fa3016 have the same complete Git tree: dfd9e21de46e3c757793692e05abb00ccb12585c. The final head descends from ebe4fca (verified merge-base).
  • src/, tests/, scripts/, docs/, playwright.config.mjs, and rescript.json are Git-object identical between dd9f1c4 and this final head. Thus the mutation-tested fixes from my exact-dd9 review are unchanged: the five output-isolating Crypto negatives; observable Notification permission read/assertion; order-independent worker responses; corrected IDB close negative; and raw/typed AES-GCM inputs with sound result contracts.
  • Relative to dd9, the only file changes are package.json and package-lock.json. Structural comparison confirms their only change is removing entities: "4.5.0" from root direct dev-dependency metadata. No resolved dependency entries or versions changed. Both final package files are also byte-identical to the original d938 package snapshot on which I ran the full requested validation sequence.
  • Against the corrected final base, the package delta is the intended ws development dependency and lockfile entries, not an entities addition/removal. A fresh exact-head npm ci passed. npm ls entities ws confirms ws 8.22.1 directly and entities 4.5.0 transitively through Astro's dependency tree, without a direct entities dependency.
  • The only final-base-to-head docs change is the already-reviewed testing.mdx port paragraph, lines 55–64. It is correctly owned by the binding diff. Its content is unchanged from dd9; it is not a new text change relative to dd9.

Checks for this supplement: exact Git object/ancestry comparisons, parsed package-metadata comparison, git diff --check, fresh npm ci, and npm ls entities ws. All passed. I did not repeat source review, compilation, browser tests, negative consumers, docs build, or release validation at fb9; the source/test conclusions carry forward by exact identity, not a claimed rerun. CI/build and Release/validate were pending when inspected. This supplement does not extend the prior narrow review into unreviewed acceptance/documentation scope.

The attached repository remained clean and unchanged at its original d938 HEAD. No repository edits, approval, merge, publication, or settings changes.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Author acceptance evidence for head fb9d51cbfcd5c1964bcc1c65c534d626e485c06d (base ebe4fcac0e9ca68eb4b62f7209be8cf2e173fa95):

  • Fix receiver dispatch for WebSocket sends and worker Request fetches #386 — receiver dispatch: WebSocket text/ArrayBuffer/Blob sends are exercised through a real loopback ws echo server. The base Worker Request-fetch test uses a real worker and local HTTP server to assert both explicit POST/header/body forwarding and omitted-init GET behavior. Worker.Impl inheritance for SharedWorkerScope and ServiceWorkerScope is compile-covered; those two scope-specific runtime paths are not separately run.
  • Correct ResizeObserver callback arity #387 — ResizeObserver: the Chromium fixture checks both callback arguments, waits for initial delivery and a subsequent induced resize, then disconnects the observer and removes the target. A packed negative rejects the old nested callback.
  • Preserve Uint8Array types in Fetch, Blob, and SafeFetch byte APIs #388 — byte representations: Request, Response, Blob and Blob.stream results are checked as Uint8Array with contents; SafeFetch.bytes checks representation, content, response identity, and its existing error-as-value behavior. Packed negative consumer rejects assignment to an ordinary array.
  • Use native typed arrays in Web Audio and DOMMatrix bindings #389 — typed buffers: OfflineAudioContext exercises analyser float/byte data, AudioBuffer get/copy-to/copy-from caller buffers, and Biquad/IIR frequency response using sentinels to verify the supplied arrays were written. DOMMatrix and DOMMatrixReadOnly perform non-identity Float32Array round-trips. Packed negatives reject ordinary arrays for analyser and matrix APIs.
  • Correct WebCrypto result types and verify key workflows #390 — WebCrypto: positive packed Crypto-only consumer plus Chromium tests cover digest-to-byte-view, HMAC generate/sign/verify, ECDSA key-pair sign/verify, AES-GCM import/encrypt/decrypt, and raw/JWK export without casts. Generic algorithm-dependent outputs remain unknown; fixed HMAC/ECDSA helpers and disjoint export formats have negative algorithm/format cases. Legacy JSON result assumptions have separate return-type negatives.
  • Correct DOM constructor, narrowing, and SVG className contracts #391 — DOM contracts: Chromium registers and constructs a custom-element class, observes broad closest, and checks HTML string versus SVG animated className through direct and broad selector access. Packed negatives reject element instances as constructors, unrelated closest narrowing, and className on broad Element values.
  • Represent native empty, nullable, and operation-specific results accurately #392 — empty/null/operation-specific values: Chromium asserts empty Request destination/referrer policy, unsupported MIME type, null zero-size Canvas toBlob, contended ifAvailable Lock callback, undefined IndexedDB delete result, and a valid static Notification.permission value. Packed negatives cover non-nullable Canvas/Locks callbacks, database-only use of delete result, and the removed Notification instance property.
  • Establish reliable runtime regression tests for the alpha #383 — harness/regression coverage: fixtures are categorized and packed negatives match specific diagnostics. The browser runner has a validated configurable loopback port, refuses to reuse occupied servers, and the explicit occupied-port probe failed closed instead of passing against an unrelated HTTP 200 server. WEBAPI_TEST_PORT=4192 npm run test:browser passed.

Serialized checks on this snapshot passed: npm ci (0 audit vulnerabilities), npm run build, npm run format:check, npm run test:negative, npm test, npm run check:features, default and port-overridden npm run test:browser (4 tests each), npm run check:consumer (including packed Crypto), and npm run build && npm run build:docs. GitHub CI build and Release validation are successful. Independent source review plus supplemental exact-head review found and verified corrections to the reported findings; reviewer summaries are linked in the PR timeline.

The API migration guide is the downstream docs-only PR #398; it must be integrated before considering its documentation acceptance complete. No issues are closed by this comment.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Final author acceptance mapping for head fb9d51cbfcd5c1964bcc1c65c534d626e485c06d (base ebe4fcac0e9ca68eb4b62f7209be8cf2e173fa95):

  • Fix receiver dispatch for WebSocket sends and worker Request fetches #386: WebSocket send methods use receiver dispatch; Chromium sends text, ArrayBuffer, and Blob to a real loopback WebSocket echo server. A real Worker uses Request-based fetch with explicit POST/header/body and omitted-init GET against a loopback HTTP server. SharedWorkerScope and ServiceWorkerScope inherited fetch bindings are compile-checked; base Worker runtime is exercised.
  • Correct ResizeObserver callback arity #387: ResizeObserver callback is one invocation with two arguments; Chromium awaits initial and induced-resize callbacks, checks entries/observer, disconnects, and removes the target. A packed negative rejects the old nested callback.
  • Preserve Uint8Array types in Fetch, Blob, and SafeFetch byte APIs #388: Request, Response, Blob, Blob.stream, and SafeFetch bytes are checked for Uint8Array representation/content; SafeFetch also checks original response identity and preserves the error-as-value path. A packed negative rejects ordinary-array assignment; no conversion copies were added.
  • Use native typed arrays in Web Audio and DOMMatrix bindings #389: OfflineAudioContext exercises Float32Array/Uint8Array analyser buffers, AudioBuffer channel and caller-owned copy buffers, and Biquad/IIR response buffers initialized with sentinels. Non-identity DOMMatrix and DOMMatrixReadOnly Float32Array conversions round-trip. Packed negatives reject ordinary arrays for matrix and analyser APIs.
  • Correct WebCrypto result types and verify key workflows #390: Public packed Crypto consumer and Chromium tests cover HMAC generate/sign/verify, digest-to-byte-view, ECDSA key-pair sign/verify, raw/JWK export, and AES-GCM raw import/encrypt/decrypt, without unsafe casts. Generic algorithm-dependent results remain unknown; fixed HMAC/ECDSA helpers constrain result shapes. Negative packed consumers isolate former JSON-result assumptions, wrong key algorithms, and cross-format exports. algorithmString/algorithmObject preserve the native string/dictionary parameter path, including AES-GCM IV fields.
  • Correct DOM constructor, narrowing, and SVG className contracts #391: Chromium defines and constructs a custom element class, checks broad closest, HTML string className, SVG animated className directly and through broad selectors. Packed negatives reject element-instance constructors, unrelated closest narrowing, and className on broad Element.
  • Represent native empty, nullable, and operation-specific results accurately #392: Chromium checks empty Request destination/referrer defaults, unsupported MIME, nullable zero-size Canvas callback, contended nullable Web Lock, undefined IDB delete result, and a valid static Notification.permission value. Packed negatives cover nullable Canvas/Locks callbacks, database-only use of delete result, and the removed Notification instance property.
  • Establish reliable runtime regression tests for the alpha #383 harness contribution: all negative consumers match expected diagnostics; the browser runner validates WEBAPI_TEST_PORT, never reuses an existing server, and supports parallel worktrees. An unrelated HTTP 200 server on the selected port was rejected rather than reused.

Serialized final checks passed: clean npm ci (0 audit vulnerabilities), npm run build, npm run format:check, npm run test:negative, npm test, npm run check:features, npm run test:browser (4 Chromium tests), WEBAPI_TEST_PORT=4192 npm run test:browser (4 tests), npm run check:consumer (including packed Crypto), and npm run build && npm run build:docs. GitHub CI build and Release validate also passed. Independent review and supplemental review summaries are linked in the PR timeline; no remaining code/test finding is outstanding.

The API migration notes are in downstream docs PR #398, which is still draft and currently needs to be retargeted from its older binding snapshot to this final head before its documentation evidence is complete. No issues are closed by this comment.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Additional focused #388 finding from final coordination review of fb9d51cbfcd5c1964bcc1c65c534d626e485c06d:

The HTTP stream inputs remain broader than the newly promised byte-stream output. BodyInit.fromReadableStream and Response.fromReadableStream still accept ReadableStream.t<'t>, including a correctly typed string stream, while the Request/Response body fields now promise Uint8Array chunks.

A native Node probe confirms that Response does not normalize an arbitrary stream's chunks:

const stream = new ReadableStream({
  start(controller) {
    controller.enqueue("not bytes");
    controller.close();
  },
});
const response = new Response(stream);
(await response.body.getReader().read()).value; // "not bytes", not Uint8Array

A Uint8Array-producing stream preserves Uint8Array chunks as expected. This was a native JavaScript probe plus inspection of the exact public signatures, not a claim that I compiled a new ReScript consumer.

Requested bounded follow-up: constrain these HTTP-body adapters to ReadableStream.t<Uint8Array.t> (or supply another demonstrably sound input/output contract), without changing generic streams elsewhere or introducing hidden copies/encoding. Add negative consumers rejecting string streams through both adapters and a positive native byte-stream construction/read regression, then include this delta in independent review and the author acceptance mapping. The earlier reviewed fixes remain valid; this is a related input-boundary gap before #388 is called complete.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Supplemental author acceptance update for head 0b4642539c9a41d9e2aa53c3278a9e6850d7a744: #388 stream inputs are now constrained to byte chunks in both BodyInit.fromReadableStream and Response.fromReadableStream. Packed negatives reject ReadableStream<string> at each adapter; the compile-positive fixture accepts a byte stream as BodyInit, and the Chromium regression calls the compiled Response.fromReadableStream binding, reads the body, and asserts Uint8Array chunk representation and all bytes [65, 66, 67]. A deliberately wrong middle byte is rejected by the shared predicate. The independent reviewer mutation probe confirmed restoring the old predicate or corrupting the native stream chunk makes the browser regression fail. npm run test:negative and the browser suite passed after the adapter change; the final independent supplemental review confirms the direct stream-byte assertion gap is resolved.

No issues are closed by this update; final PR CI is running on this head.

@jderochervlk
jderochervlk marked this pull request as ready for review November 7, 2026 16:54
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-07T16:55:56.064572Z 0b46425 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Final author acceptance mapping for exact head 0b4642539c9a41d9e2aa53c3278a9e6850d7a744, base ebe4fcac0e9ca68eb4b62f7209be8cf2e173fa95:

  • Fix receiver dispatch for WebSocket sends and worker Request fetches #386 — receiver dispatch: WebSocket text/ArrayBuffer/Blob sends use the native receiver and are echoed by a real loopback ws server. A real Worker runs Request-based fetch with explicit POST/header/body and omitted-init GET against a loopback HTTP server. SharedWorkerScope and ServiceWorkerScope inherited paths are compile-covered; the base Worker path is runtime-covered.
  • Correct ResizeObserver callback arity #387 — ResizeObserver: callback takes (entries, observer) in one invocation. Chromium awaits initial and resize-triggered callbacks, verifies entries and observer, disconnects, and removes the target; a packed negative rejects the old nested callback.
  • Preserve Uint8Array types in Fetch, Blob, and SafeFetch byte APIs #388 — byte streams: Request, Response, Blob, Blob.stream, BodyInit.fromReadableStream, Response.fromReadableStream, and SafeFetch use Uint8Array chunk/result types. Native bytes and stream contents [65,66,67] are checked through compiled bindings; the byte predicate rejects [65,0,67]. Packed negatives reject string streams at both HTTP body adapters and ordinary-array assignments. SafeFetch also checks response identity and preserves error-as-value behavior; no copying/encoding adapter was introduced.
  • Use native typed arrays in Web Audio and DOMMatrix bindings #389 — typed buffers: OfflineAudioContext tests Float32Array/Uint8Array analyser writes, AudioBuffer channel/copy-to/copy-from caller buffers, and Biquad/IIR output writes with sentinel buffers. DOMMatrix and DOMMatrixReadOnly exercise non-identity Float32Array round-trips; packed negatives reject ordinary arrays for matrix and analyser APIs.
  • Correct WebCrypto result types and verify key workflows #390 — WebCrypto: public packed Crypto consumer and Chromium cover HMAC/ECDSA generate/sign/verify, digest-to-byte-view, key-pair use, raw/JWK export, and AES-GCM import/encrypt/decrypt without casts. algorithmString/algorithmObject expose the native algorithm-string/dictionary boundary; fixed HMAC/ECDSA helpers constrain result shapes; generic algorithm-dependent outputs remain unknown. Packed negatives isolate legacy JSON assumptions, wrong algorithms, and cross-format exports.
  • Correct DOM constructor, narrowing, and SVG className contracts #391 — DOM types: Chromium defines and constructs a custom-element class, checks broad closest, HTML string className and SVG animated className directly/through broad selectors. Packed negatives reject element instances as constructors, unrelated closest narrowing, and className on broad Element.
  • Represent native empty, nullable, and operation-specific results accurately #392 — null/empty/static/operation-specific results: Chromium checks empty Request destination/referrer defaults, unsupported MIME, null zero-size Canvas callback, contended nullable Web Lock, undefined IDB delete result, and an allowed static Notification.permission value. Packed negatives reject nonnullable callbacks, database-only use of delete result, and the removed Notification instance property.
  • Establish reliable runtime regression tests for the alpha #383 regression infrastructure: categorized fixtures and diagnostic-matched packed negatives cover the native binding contracts. The browser harness validates WEBAPI_TEST_PORT, never reuses an existing server, and supports separate local worktrees; an unrelated HTTP 200 endpoint is rejected. WEBAPI_TEST_PORT=4192 npm run test:browser succeeds.

The independent source review and supplemental reviews found and verified corrections to each reported defect; the final Uint8Array byte assertion was separately mutation-checked. GitHub CI build and Release validate passed at this exact head. Serialized local validation included clean install/audit, build, format, negative consumers, Node tests, feature tests, default and alternate-port browser tests, packed consumer (including Crypto), and docs build; the final assertion-only change was followed by a browser rerun and clean format check. No issue was manually closed.

The migration/API documentation is in downstream docs PR #398, now ready and based on this exact binding head.

@jderochervlk

Copy link
Copy Markdown
Collaborator Author

Cross-stack #383 assessment at binding snapshot 0b4642539c9a41d9e2aa53c3278a9e6850d7a744, based on foundation #397 tree 779568e28c1caef5554b6ebbe9292fc0e9daa0dd:

  • Foundation supplies explicit compile-only/documentation-example/Node/browser categories, an all-fixture registration guard, meaningful Node assertions, removal of the ineffective generated-JS drift guard, compile-only isolation, and diagnostic-matched negative consumer infrastructure.
  • This binding snapshot registers native Chromium regressions: WebSocket payloads use a real loopback server; Worker fetch verifies explicit POST/header/body and omitted-init GET against a local HTTP server; ResizeObserver awaits initial and resize-triggered callbacks and disconnects; byte streams and typed buffers assert native values; matrix conversions are non-identity; WebCrypto verifies public packed HMAC/ECDSA flows; and DOM/custom-element/null/empty/IDB/lock/notification cases are covered.
  • The packed negative manifest contains 23 cases (the generic harness sentinel plus 22 binding contract cases); checks require both compiler failure and the stated diagnostic. check:consumer also retains the packed Fetch-only SafeFetch rejection.
  • The Playwright runner uses WEBAPI_TEST_PORT, a shared base URL, and reuseExistingServer: false; the author reports default/4192 passes and invalid/occupied-port rejection. Contributor documentation for this local-isolation contract is in downstream docs PR docs(webapi): document alpha and validate docs distribution [Codex] #398.

GitHub CI build and Release validate passed on 0b4642539c9a41d9e2aa53c3278a9e6850d7a744; docs PR #398 is based on this snapshot and its build/validate checks also passed. The binding PR's final independent review was reported active when I made this assessment, so this is not a formal approval or issue closure. Subject to that final review, I assess #383's acceptance criteria as covered across the #397/#399/#398 stack.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant