feat(sandbox): prove playground and iOS corpus coverage with real trap-form emission#2673
Open
slepp wants to merge 15 commits into
Open
feat(sandbox): prove playground and iOS corpus coverage with real trap-form emission#2673slepp wants to merge 15 commits into
slepp wants to merge 15 commits into
Conversation
Execute every runnable playground manifest entry through native Hew and the sandbox VM, pinning stdout to the checked-in expected output while preserving fail-closed assertions for unsupported entries.
Require every Hew source in examples/sandbox-graduation to remain represented in the native-to-sandbox execution parity suite.
Snapshot the iOS default template, bundled examples, and tutorial starters. Run valid supported programs at native-to-sandbox parity and assert typed fail-closed behavior for the stale template, indirect function calls, and closures.
Snapshot all quick-reference snippets, execute the 13 standalone programs using the app's wrapping rule, pin higher-order and native-import rejection, and enumerate six context-dependent fragments.
hew-sandbox-wasm's new playground.rs and ios_subset.rs tests spawn the hew-sandbox-vm Node/npm toolchain directly and unconditionally, so they ran under any generic 'cargo nextest run --workspace' invocation - Linux/macOS/Windows/FreeBSD CI, release-gate.yml, and local 'make test' - all of which fail on an unprovisioned runner with 'hew-sandbox-vm dependencies are not installed'. Exclude both test binaries from the nextest default-filter (.config/nextest.toml, profile.default and profile.ci), matching the existing binary(parity) convention used for parity.rs. Add matching the existing convention on parity.rs and parity_ratchet.rs's VM-dependent tests. Split hew-sandbox-vm's npm-ci-with-hash-stamp logic out of the sandbox-parity make target into a standalone sandbox-vm-deps target, and extend sandbox-parity's cargo test invocation to run all four VM-dependent binaries (parity, parity_ratchet, playground, ios_subset) instead of just the first two, so the dedicated provisioned gate actually exercises every native-vs-VM test that generic nextest runs now skip. Add scripts/check-sandbox-parity-coverage.py (wired into 'make lint' and the CI lint job) as a regression guard: it scans hew-sandbox-wasm/tests/*.rs for tests that call ensure_parity_runner_built(), and asserts each one is both excluded from the generic nextest default-filter and named in sandbox-parity's cargo test invocation, so a future VM-dependent test can't land in only one of those two states.
check-sandbox-parity-coverage.py only matched a literal ensure_parity_runner_built() call inside a test's own body, missing indirect VM invocation: parity_ratchet.rs's live_gate_matches_declared_coverage reaches the npm parity:run spawn only through assert_admitted_runs_clean/assert_admitted_traps/ assert_admitted_but_fails -> run_sandbox_inline, a second, independent spawn path that never calls ensure_parity_runner_built. Today's classification happened to be correct only because that test also calls ensure_parity_runner_built() directly elsewhere in its body - a future test reaching a VM spawn purely transitively would have been missed entirely. Rewrite the checker around a same-file call graph instead of a single literal-string check on each test's own body: - Parse every function (not just #[test] ones) in a test file. - Mark a function as a VM 'spawn marker' if its body contains the literal hew-sandbox-vm path segment - the common substring every real Node-toolchain spawn path names, whether via ensure_parity_runner_built, run_sandbox, or run_sandbox_inline. - Build a call graph from literal name( occurrences and compute, for every #[test], whether it transitively reaches a marker function. - Conservative fallback: if a marker function is not transitively reached by any #[test] in its file via the static graph (e.g. it is only reachable through indirection this script cannot parse, such as a closure or function pointer), treat every test in that binary as VM-dependent and require whole-binary exclusion rather than trusting a narrower per-test exclusion for that file. Coverage assertions are unchanged in spirit: every VM-dependent test must be excluded from nextest's profile.default and profile.ci default-filter, and its binary must be named in sandbox-parity's cargo test invocation. Add scripts/tests/test_check_sandbox_parity_coverage.py as a regression proof, wired into a new test-sandbox-parity-coverage-check make target that sandbox-parity-coverage-check now depends on (mirrors verify-ffi/test-verify-ffi). Covers: a direct marker call, a two-hop indirect call chain with no direct call in the test body (the exact parity_ratchet.rs shape), a non-VM structural test staying unflagged, an orphaned-marker case tripping the binary-level fallback, and an end-to-end assertion against the real parity_ratchet.rs file - including replaying the original regression by stripping the direct ensure_parity_runner_built() call from live_gate_matches_declared_coverage and confirming the transitive path through run_sandbox_inline alone still marks it VM-dependent.
…-test The call-graph attribution in check-sandbox-parity-coverage.py was unsound: it excluded from generic nextest only the #[test] functions it could statically prove reached a VM-spawning helper, and fell back to whole-binary exclusion only when a marker function was unreachable from ANY test in its file. That fallback condition is not the right trigger. Proving test A reaches a spawning function via the call graph says nothing about whether some OTHER test B in the same file also reaches it through a path the graph does not model - a runtime dispatch table, a closure passed through a combinator, a trait object. Since A already 'explains' the marker's reachability, the marker is not orphaned, the fallback never fires, and B keeps running under generic (unprovisioned) nextest despite spawning the VM at runtime through indirection the graph cannot trace. Fix: drop per-test/call-graph attribution entirely. A test file is VM-dependent if it contains a spawn marker anywhere, full stop; the whole binary is then excluded from nextest's default-filter via binary(<name>) - never a narrower test(<name>) - and the whole binary is run under the provisioned make sandbox-parity gate. This cannot be evaded by any indirection because it no longer depends on proving which test reaches what; it only depends on whether the file contains a spawning function at all. - .config/nextest.toml: replace parity_ratchet's test(live_gate_matches_declared_coverage) exclusion with binary(parity_ratchet) in profile.default and profile.ci, so its non-VM structural ratchet tests move to sandbox-parity alongside the VM test - an acceptable, correctness-driven consequence of whole- binary containment, not a coverage loss (sandbox-parity still runs them every time). Also add the same exclusion to profile.lane, which had no exclusion for parity_ratchet at all: verified this was a real, pre-existing failure (cargo nextest run --profile lane failed on live_gate_matches_declared_coverage without node_modules) rather than something introduced by this branch, but it is the identical defect in the identical file for the identical binary, so it is fixed alongside the profile.default/profile.ci change rather than left broken. - scripts/check-sandbox-parity-coverage.py: rewritten around a plain whole-file substring search for the hew-sandbox-vm marker instead of a same-file call graph. Function extraction is kept only for --verbose diagnostics (naming which function(s) triggered the classification), not for the pass/fail decision. - scripts/tests/test_check_sandbox_parity_coverage.py: rewritten to prove the new contract, including a synthetic case with two tests - one reaching a spawn marker via a call the graph could trace, one reaching the very same marker only through a runtime dispatch table - and asserting both land in the VM-dependent binary regardless, plus that excludes_binary() rejects a bare test(<name>) filter for a VM-dependent binary and only accepts binary(<name>). - Makefile / ci.yml: comments updated to describe whole-binary containment; sandbox-parity's own comment now notes explicitly that parity_ratchet's structural tests run there too. Verified: cargo nextest run -p hew-sandbox-wasm under profile.default, profile.ci, and profile.lane all pass (51/51, 0 skipped) with node_modules removed; make sandbox-parity passes all 10 tests across the 4 binaries with node_modules restored; the coverage checker and its self-test pass and were confirmed to fail correctly against the pre-fix nextest.toml (stashed and reverted) with the exact two FAIL lines expected.
…fault/ci REQUIRED_PROFILES in check-sandbox-parity-coverage.py listed only default and ci, but make test-lane, make test-lane-all, and make test-fast all run generic 'cargo nextest run --profile lane' too - another unprovisioned entry point a VM-dependent binary could leak into unnoticed. Add lane to REQUIRED_PROFILES so the checker asserts binary(<name>) whole-binary exclusion in profile.lane exactly as it already does for profile.default and profile.ci. .config/nextest.toml's profile.lane already carries binary(parity_ratchet) (added when the previous fix in this lane's history discovered and closed a real, verified pre-existing gap there), so no nextest.toml change is needed here - only the checker itself was under-scoped. Add a sabotage self-test that removes binary(parity_ratchet) from profile.lane's default-filter alone (profile.default and profile.ci keep their real exclusions), runs the checker's main() in-process against the mutated file, and asserts it fails with exactly one FAIL line naming profile.lane and parity_ratchet - proving lane is actually enforced, not just present in the tuple. Also asserts the checker passes again once the real file is restored, and adds a one-line regression pin asserting 'lane' is in REQUIRED_PROFILES. Verified: cargo nextest run -p hew-sandbox-wasm under profile.default, profile.ci, and profile.lane all pass (51/51, 0 skipped) with node_modules removed; make sandbox-parity passes all 10 tests across the 4 binaries with node_modules restored; the coverage checker now reports 16/16 passing checks (4 binaries x 3 profiles + 4 Makefile checks, up from 12); its self-test (10 cases, including the new sabotage proof) passes 3x; cargo fmt, actionlint, and both preflight-parity scripts are clean.
…able
`default_filter_line()` used a regex that scanned from a profile's
header to end-of-file looking for the next `default-filter = "..."`
line. If `[profile.default]`'s own filter line were deleted, the
regex would spill past the table boundary and match the *next*
occurrence later in the file -- `[profile.ci]`'s filter -- silently
reporting CI's value as if it belonged to default. That is a false
pass: the checker would report `[profile.default]` as excluding a
VM-dependent binary it does not actually exclude.
Rewrite `default_filter_line()` to parse `.config/nextest.toml` with
`tomllib` (already an established convention in
scripts/verify-ffi-symbols.py) and look up
`document["profile"][profile].get("default-filter", "")`. A table
lookup is inherently bounded to that table's own keys, so it cannot
spill into a sibling profile the way a naive regex scan can. A
missing profile table still raises (unchanged, more severe case,
out of scope here); a missing `default-filter` key now returns ""
and flows through the normal per-binary FAIL-reporting path instead
of borrowing a neighbor's value.
Add a regression test that deletes `[profile.default]`'s entire
`default-filter` line, confirms `profile.ci`/`profile.lane` are
untouched, and asserts the checker fails for all VM-dependent
binaries under profile.default only -- with no false FAIL under
ci/lane and no false pass under default. Also add a
`_profile_section_span()` test helper (bounding a profile's TOML
section between its own header and the next top-level header) and
use it to fix the same latent unbounded-scan pattern in the existing
`_remove_binary_exclusion_in_profile` test helper.
Verified:
- scripts/tests/test_check_sandbox_parity_coverage.py: 11/11 pass
(flake gate 3x, all pass)
- make sandbox-parity-coverage-check: 16/16 checks pass against
real repo state
- cargo nextest run -p hew-sandbox-wasm (default/ci/lane profiles,
hew-sandbox-vm/node_modules removed): 51/51 passed, 0 skipped,
each profile
- make sandbox-parity (node_modules restored): 10/10 tests pass
across parity, parity_ratchet, playground, ios_subset
- cargo fmt --check: clean
- actionlint .github/workflows/ci.yml: clean
- scripts/check-preflight-ci-parity.sh: 16/16, 11/11, 5/5
- scripts/preflight-parity-selftest.sh: 4/4 PASS
The previous fix for the [profile.default]/[profile.ci] filter boundary bug used the stdlib `tomllib` module. `tomllib` requires Python 3.11+; this project's tooling baseline is Python 3.10, and nothing provisions a newer interpreter for this script. That silently raised the effective floor for anyone or any CI runner still on 3.10, with no dependency or interpreter change declared to match. Replace it with `_profile_table_span()`: a plain regex-based scan that locates `[profile.<name>]`'s own header line and bounds the lookup to the text between that header and the very next header line of any kind (a sibling profile's `[profile.<other>]`, or that same profile's own `[[profile.<name>.overrides]]` array-of-tables entries). This mirrors real TOML table semantics without a parser: once any new header line appears, keys that follow no longer belong to the previously open table. Every profile in .config/nextest.toml declares its `default-filter` before its own first override block, so bounding at "the next header line of any kind" is correct and requires no top-level/nested distinction. `default_filter_line()` now calls this helper and searches only within the returned span, so a missing `default-filter` key returns "" and can never read a sibling profile's value -- the exact bug this already-existing regression test proves. No new dependency added (no third-party TOML library, no `tomllib` import); the script runs on a bare Python 3.10 interpreter same as before this whole exercise started. Added `test_script_stays_python_3_10_compatible_with_no_new_dependency`, asserting no `import tomllib`/`import toml`/`import tomli` statement is present and that the section-bounded lookup still resolves `binary(parity_ratchet)` correctly for profile.default/ci/lane against the real repo file. Retained the existing `test_deleting_profile_default_filter_line_entirely_fails_default_only` sabotage proof (deletes profile.default's own default-filter line, confirms ci/lane are untouched, confirms the checker fails for profile.default only) and all prior VM-topology self-tests unchanged. Verified: - scripts/tests/test_check_sandbox_parity_coverage.py: 12/12 pass (flake gate 3x, all pass) - make sandbox-parity-coverage-check: 16/16 checks pass against real repo state - cargo nextest run -p hew-sandbox-wasm (default/ci/lane profiles, hew-sandbox-vm/node_modules removed): 51/51 passed, 0 skipped, each profile - make sandbox-parity (node_modules restored): 10/10 tests pass across parity, parity_ratchet, playground, ios_subset - cargo fmt --check: clean - actionlint .github/workflows/ci.yml: clean - scripts/check-preflight-ci-parity.sh: 16/16, 11/11, 5/5 - scripts/preflight-parity-selftest.sh: 4/4 PASS
…nd/ios_subset parity
Implement parity-correct array repeat, scalar cast, and Result/Option postfix-try emission while keeping map literals fail-closed until the VM has map semantics.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The sandbox VM's parity story had two gaps. First, the consumer-driven coverage work from #2623 (playground manifest, iOS corpus, graduation ratchet, VM-dependent-test classification) went stale against main before it could land — this PR replaces and completes that stack; I'll close #2623 after this merges. Second, three expression forms that real playground/iOS programs use —
[value; count]array repeat, numericascasts, and?on Result/Option — still trapped in the sandbox instead of executing, and two verified native↔sandbox divergences existed underneath them: f32 arithmetic ran at f64 precision, and isize/usize width was unpinned.What
scripts/check-sandbox-parity-coverage.pycoverage guard with a 12-case self-test; whole-binary VM-dependent-test classification with nextest exclusion acrossdefault/ci/laneprofiles (thesandbox-paritytarget still runs all four binaries); CI coverage-check step; 44-entry playground manifest parity; 40-source iOS subset corpus; 17-file graduation-corpus ratchet.ArrayRepeatlowers to a counted loop with the element evaluated once before the count; numericCastemitsnumeric.castwith explicit source/target type names (saturating float→int with NaN→0, matchingfptosi.sat/fptoui.sat; f32 via fround);PostfixTryon Result/Option extracts the success payload or early-returns the error/None, reusing the extractedemit_early_returnpath.MapLiteralstays fail-closed — the VM has no map semantics, so it refuses rather than approximating. Error-converting?(mismatched error types) also stays fail-closed.Math.froundat every operation, not just at literals/casts, with the divergent input pinned as a fixture.Test
trap_residual.hewgraduation fixture with an exact-stdout native contract, asserted equal native↔sandbox (covers saturating casts, sign/zero extension,?success and early-return paths, array-repeat evaluation order).hew-sandbox-vm/test/numeric-cast-parity.test.mjs: int/float/bool/char cast boundary cases against nativeassemantics.parity_ratchetbehavioural gate: every promoted construct must run clean on the real VM; every NotYetRunnable construct must trap.scripts/tests/test_check_sandbox_parity_coverage.py: 12 sabotage cases (bypass, sibling-profile borrow, Python 3.10 compatibility).make sandbox-parity,make fixtures-check,parity_ratchet, full workspace nextest.Quality Checklist
.ok()?orunwrap_or_default()in codegen without// JUSTIFIEDcomment// WASM-TODO(#NNN):marker, and newhew_*exports are classified inscripts/jit-symbol-classification.tomlOut of scope
MapLiteralsandbox emission — deliberately fail-closed until the VM grows map semantics.int as charcasts trap at runtime in the VM rather than failing closed at compile time; tightening that to compile-time refusal is a follow-up.