Repository navigation
fix(webapi): correct native binding contracts [Codex] - #399
jderochervlk wants to merge 24 commits into
Conversation
…o agent/alpha-binding-contracts
…o agent/alpha-binding-contracts
…o agent/alpha-binding-contracts
…o agent/alpha-binding-contracts
…o agent/alpha-binding-contracts
|
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:
Additional acceptance gaps beyond the draft's acknowledged stream-chunk, shared/service-worker runtime, IIR call, and permission-value gaps:
Checks executed successfully: |
…o agent/alpha-binding-contracts
|
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 The prior Crypto's fixed HMAC/ECDSA generators and disjoint export formats have sound successful result shapes; generic results remain After |
|
Cross-stack #383 assessment at binding head GitHub CI |
|
Independent review of d938c71, against 6fa3016 ( Actionable findings
Prior findings and native evidence
Isolation, checks, and remaining acceptanceAll requested commands passed serially: A live unrelated server occupying port 4192 made Playwright exit 1 rather than reuse it. Invalid ports Documentation acceptance is also deferred, not met by this snapshot: no migration guidance for the #391 broad All mutation experiments used scratch package copies or browser interception/overrides only. The repository HEAD and tracked worktree/index remained unchanged and clean. |
|
Supplemental independent review of exact head f5f7d33, against exact base 283591a ( Remaining actionable findings
Resolved prior findings and requested confirmations
Validation boundaryRan serially against a |
|
Supplemental independent review of dd9f1c4, against exact base 283591a ( Confirmed corrections, with counterfactual evidence
Retained prior corrections
Checks and scopeRan serially in a scratch archive of exact dd9f1c4: 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. |
…o agent/alpha-binding-contracts
|
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.
Checks for this supplement: exact Git object/ancestry comparisons, parsed package-metadata comparison, The attached repository remained clean and unchanged at its original d938 HEAD. No repository edits, approval, merge, publication, or settings changes. |
|
Author acceptance evidence for head
Serialized checks on this snapshot passed: 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. |
|
Final author acceptance mapping for head
Serialized final checks passed: clean 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. |
|
Additional focused #388 finding from final coordination review of The HTTP stream inputs remain broader than the newly promised byte-stream output. BodyInit.fromReadableStream and Response.fromReadableStream still accept 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 Uint8ArrayA 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 |
|
Supplemental author acceptance update for head No issues are closed by this update; final PR CI is running on this head. |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Final author acceptance mapping for exact head
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. |
|
Cross-stack #383 assessment at binding snapshot
GitHub CI |
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.algorithmStringandalgorithmObjectprovide 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 withWEBAPI_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-releaseheadebe4fcac0e9ca68eb4b62f7209be8cf2e173fa95, 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.