test(bin-e2e): dlopen the shipped js plugin cdylib for real - #370
Closed
raphaelvigee wants to merge 51 commits into
Closed
test(bin-e2e): dlopen the shipped js plugin cdylib for real#370raphaelvigee wants to merge 51 commits into
raphaelvigee wants to merge 51 commits into
Conversation
raphaelvigee
force-pushed
the
raphaelvigee/feat-plugin-js-m8-bin-e2e
branch
from
August 7, 2026 11:21
a1bfe40 to
5380ba2
Compare
raphaelvigee
marked this pull request as ready for review
August 7, 2026 12:57
raphaelvigee
force-pushed
the
raphaelvigee/feat-plugin-js-m8-bin-e2e
branch
from
August 7, 2026 12:58
5380ba2 to
0ec0d6c
Compare
raphaelvigee
force-pushed
the
raphaelvigee/feat-plugin-js-m8-bin-e2e
branch
from
August 7, 2026 16:36
0ec0d6c to
32f0499
Compare
raphaelvigee
force-pushed
the
raphaelvigee/feat-plugin-js-m8-bin-e2e
branch
from
August 7, 2026 18:31
32f0499 to
538a871
Compare
raphaelvigee
force-pushed
the
raphaelvigee/feat-plugin-js-m8-bin-e2e
branch
from
August 7, 2026 19:56
538a871 to
20f1382
Compare
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…out (#392) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…de` knob (#393) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…it in (#387) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
First-party JS/TS ecosystem plugin (M0+M1), mirroring the plugin-go / plugin-go-cdylib two-crate split (logic crate + thin stabby cdylib wrapper). - Provider: package.json discovery, pnpm-workspace.yaml + npm "workspaces" member resolution. - js_install: hermetic per-(name,version,integrity) dependency fetch, SRI verification, platform baked into the cache key, postinstall scripts off by default with an explicit allowlist enforced before any network I/O. Reviewed by feature-quality/code-quality/hermeticity; a tar-slip symlink escape and an uncacheable (host-toolchain-dependent) lifecycle-script hash were found and fixed, each with a regression test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
Temporary, will be reverted once the real cause is identified. Prints what `cargo update` would change in CI's actual environment before the --locked build, since every local reproduction attempt (native darwin, explicit --target, full --workspace, cleared cache forcing a fresh index) succeeds while CI fails identically and fast on every retry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
…ke --locked builds Revert the temporary CI diagnostics now that the root cause is confirmed.
M2 of the JS/TS plugin: fixes the deferred optionalDependencies hard-fail (platform-restricted optional deps now silently skip instead of failing Provider::get), then adds a real import-graph resolver on top of M1's package.json-declaration wiring. - oxc_parser + oxc_resolver extract and resolve import/require/dynamic-import specifiers per real Node condition-set semantics (separate ESM/CJS/types resolvers), building two distinct graphs (runtime vs type-only edges). - Phantom-dependency detection: an import resolving into a package not in the declared-dependency closure (deps + devDeps + peerDeps) is a hard error naming the file/specifier/package, hermetic against a fresh checkout (no ambient node_modules required) via a bare-specifier name check. - Conformance corpus (crates/plugin-js/src/pluginjs/conformance.rs) covering exports-map condition ordering, wildcard specificity, array fallbacks, null-blocked subpaths, self-referencing imports, and the "imports" field — cross-checked live against a real Node binary when present, self-gated otherwise. Reviewed by feature-quality/code-quality/hermeticity. Four BLOCKERs found and fixed: the phantom-dep check being a no-op without ambient node_modules, peerDependencies never counted as declared, an oxc_resolver default reading the ambient NODE_PATH env var, and a resolved-but-unclassifiable edge silently passing instead of failing closed. Each has a regression test. Explicitly deferred (named, not silent): import-equals require() and require.resolve() extraction, per-specifier `type` modifier detection, provider-lifetime resolver caching (currently rebuilt per Provider::get call — a stated perf follow-up), and pinning a hermetic Node toolchain so the conformance corpus's live cross-check is guaranteed to run in CI rather than opportunistically. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
…rde_json's Map ordering oxc_resolver (a new plugin-js dependency) requires serde_json's preserve_order feature unconditionally, which is a workspace-wide Cargo choice. That silently flipped serde_json::Map from an always-sorted BTreeMap to an insertion-order IndexMap everywhere, including in plugin-oci's manifest/config json! literals, which relied on that implicit sort for canonical, cache-key-stable output. Sort explicitly at encode time instead of depending on which Map backing happens to be active.
…uirk that changed after v18 array_exports_matched_entry_missing_on_disk_hard_fails_no_fallback's live cross-check was authored against v18.12.1's --experimental-import-meta-resolve. Later Node no longer checks file existence for array-form exports entries during resolution, deferring it to module load — so any newer Node found on PATH now resolves where the hard-coded expectation says it should throw. plugin-js's own resolver (the assertion that actually matters here) still implements the documented algorithm correctly and is unaffected; only the live Node cross-check is dropped for this one fixture.
M3 of the JS/TS plugin: a cacheable js_typecheck ManagedDriver running tsc --noEmit per package, fed by the M2 import graph rather than blind package.json declarations. - Toolchain: tstool=host (only supported mode) resolves tsc from node_modules/.bin or PATH, queried once per Provider lifetime and hashed alongside the tsconfig content — disclosed non-hermetic escape hatch, same shape as the design doc's stated M1+ gap. - Input scoping fixed through review to actually match what tsc reads: plain (non-type-only) cross-package imports, not just `import type`; third-party .d.ts inputs resolved via the same lockfile mechanism as js_install (works with no ambient node_modules, not just when one happens to exist on disk); tsconfig include/exclude honored for first-party sources; the full tsconfig extends chain declared and hashed; a shared/ancestor tsconfig with unscoped include is now a loud Provider::get error instead of a silently unsound cache key. Reviewed by feature-quality/code-quality/hermeticity — five BLOCKERs found (all variations on "the declared Input set doesn't match what tsc actually reads") and fixed, each with a regression test proving the specific divergence. Tests requiring a real tsc binary are #[ignore]d with a named reason rather than silently skipping and reading as a pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
M4 of the JS/TS plugin: a cacheable js_test ManagedDriver with one target per test file (vitest default, jest alt via a single testrunner config option), fed by the M2/M3 import graph rather than package-level caching — the differentiator the design doc calls out over Turborepo/Nx, which never get finer than per-package task caching. - Per-test-file Input scoping: the test file's own runtime-transitive closure (BFS over ImportGraph::runtime_edges, bounded to the owning package), third-party deps resolved via the same lockfile-driven mechanism js_install/js_typecheck use (no ambient node_modules dependency, per the M3 lesson), and the resolved test-runner config. - Runner-config discovery covers vitest's own fallback (vite.config.*) and jest's package.json "jest" field, not just the dedicated config filenames. Reviewed by feature-quality/code-quality/hermeticity. Three BLOCKERs found and fixed: a js_test addr's file= argument accepted an absolute path or a `..`-escape with no validation, letting a target read/exec outside the workspace and sandbox entirely (fixed at both Provider::get and defensively again in the driver's run()); vitest/jest config-referenced files (setupFiles, globalSetup, a shared base config reached via a relative import) were untracked, so editing one didn't bust the cache of every test that depends on it; and the primary runner-config file itself could go undiscovered for the vite.config.*/package.json-jest-field layouts. Each has a regression test. Explicitly deferred (named, not silent): per-test-file Provider::get rebuilds the whole package's import graph from scratch rather than caching it once per package (a real warm-cache-path cost); the cross-package one-hop trim now risks a stale test-pass cache hit, not just a missed diagnostic, for a barrel-file re-export; test-runner-specific alias resolution (vitest resolve.alias, jest moduleNameMapper) isn't fed into the resolver. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
M5 of the JS/TS plugin. Perf fix (prerequisite): deps_config, typecheck_config, and test_config each rebuilt the whole package's ImportGraph from scratch on every Provider::get call — for js_test, once per test file. Flagged independently by both the M2 and M4 reviews and deferred twice. Added a per-package memoized cache (keyed OnceCell behind a Mutex whose critical section is just the get-or-insert, so unrelated packages never serialize behind one lock) shared by all import-graph consumers, plus a call-count test proving it actually memoizes. Also added the equivalent cache for workspace-member discovery, the same O(P) redundant-walk shape found alongside it. js_lint: cacheable per-package target, oxlint default + eslint (with type-aware parserOptions.project support) via a single linter config option — same naming rule as every other driver here. Reuses the lockfile- driven third-party resolution and tsconfig-extends-chain handling already established for js_typecheck/js_test. Reviewed by feature-quality/code-quality/hermeticity — five BLOCKERs found and fixed: a fabricated package.json-config fallback that doesn't match how oxlint/eslint actually discover config and would break real invocations; an unconditional hard-fail on packages with zero lintable source files (now a clean no-op); only the first parserOptions.project entry in a multi-entry eslint config being tracked, silently dropping the rest from the cache key; an unvalidated parserOptions.project path letting a repo-controlled eslint config make heph read and hash an arbitrary host file (e.g. /etc/hostname) with no workspace-containment check — a real escape, now a hard error instead of a silent fallback; and eslint configs' own relative-path extends/imports (a shared base config) going untracked, the same class of gap already fixed once for js_test's runner config. Explicitly deferred: converging graph_cache/tsc_cache/testrunner_cache/ linter_cache onto the repo's existing hmemoizer primitive for panic containment across the ABI seam (pre-existing pattern, not a new regression, scoped as its own follow-up); a same-package concurrent-race test for the new cache (single-flight correctness argued from tokio's OnceCell contract, not yet proven under real concurrency); no bin-e2e coverage for any JS driver's dlopen/ABI-crossing seam yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
M6 of the JS/TS plugin, the final milestone: a cacheable js_bundle ManagedDriver wrapping esbuild, with format (esm/cjs) and target (node/browser) as plain addr args resolved with a flat default rather than porting the Go plugin's ancestry/universe variant machinery — this plugin's addr model is a single flat workspace, so there's no ancestor chain or cross-subtree variant-pin problem for that machinery to solve (researched and confirmed against crates/plugin-go/src/plugingo/variant.rs and this crate's own flat workspace.rs model before deciding). Whole-graph cache key by design (the entry point's full transitive closure via ImportGraph::runtime_edges, cross-package recursion unlike js_test's one-hop trim), third-party deps resolved via the same lockfile-driven mechanism as every other driver, tsconfig (plus its extends chain) declared and staged the same way js_typecheck does. Reviewed by feature-quality/code-quality/hermeticity. Two functional BLOCKERs found and fixed: the discovered third-party import closure was never wired into esbuild's --external flags, so bundling any package with a real npm runtime dependency failed outright; and the output directory didn't vary by format/target, so esm and cjs variants of the same package collided on the same declared output path. A third BLOCKER (missing tsconfig Input) meant path-aliased/JSX/decorator-configured TS entry points silently failed or cached wrong. All three fixed with regression tests, including a real-esbuild end-to-end proof for the external-deps fix. Explicitly deferred (disclosed in module docs): target=browser doesn't yet change what's resolved (only what esbuild is told to assume) since the resolver has no browser condition/main-field support; a config-reference-scanning helper shared with js_test/js_lint reads a config-referenced file before checking workspace containment rather than after (dormant for js_bundle's only supported config format today, live for js_test already, flagged as a cross-cutting follow-up not specific to this milestone); rollup/webpack/vite bundlers. Note for follow-up: the design doc's v1 scope table lists six drivers including js_format, but the Milestones list only ever defined M0-M6 for the other five (install/typecheck/test/lint/bundle) — js_format has no milestone and isn't built. Flagged in plugin-js-cdylib's module doc; needs either an M7 or a corrected scope table. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
…here Live testing turned up a fourth failure mode, distinct from the prior three ambiguity/collision fixes: some packages (ts-log, ts-interface-checker, tailwindcss-scoped-preflight, @types/react in a real workspace) have exactly one lockfile entry, and it has no integrity/resolved at all — not a dedup/collision artifact. Root cause confirmed against npm's own issue tracker: `npm install` (unlike `npm ci`) can satisfy a package from its local cache and strip resolved/integrity from an existing package-lock.json entry instead of repopulating them — a known npm CLI bug (npm/cli#4263, #4460, #6301), not a heph bug. There's nothing safe to fetch+verify against in this case, so js_install correctly still fails — but it did so with "no recognized integrity algorithm in \"\"", surfaced deep inside the SRI parser with no indication of cause. Move the check up to where the js_install spec is built and name the actual cause plus the fix (regenerate the lockfile). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…n't block Per explicit product decision: reverts the previous commit's hard-fail for a package with no integrity anywhere in the workspace's lockfiles. That fail-closed behavior is the textbook-correct hermeticity default, but the actual condition (npm/cli#4263: `npm install` stripping resolved/integrity from an existing lockfile entry when a package is satisfied from local cache — not a heph bug) hit hundreds of packages across a real monorepo, with no fix available except regenerating lockfiles the user doesn't control the timing of. js_install now installs unverified when `integrity` is empty, logging a `tracing::warn!` so it's diagnosable, rather than blocking the build. This is a deliberate, recorded hermeticity trade-off, not an oversight — see driver_install.rs's module doc and fetch_and_extract's doc for the full reasoning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…lifecycle scripts Two more live-testing bugs, both in driver_install.rs: 1. extract_tarball: a real published tarball (pngjs@7.0.0) ships an explicit directory entry (`lib/`) with mode 0666 — no execute bit — followed by files inside it. `Entry::unpack` applies that mode immediately per-entry, so the very next entry (a file inside that directory) fails to be created: Unix file creation is gated on the parent directory's execute bit, not the child's own mode. Upstream `tar::Archive::unpack` avoids this by deferring all directory permissions to the end (tar-rs#242); that API can't be used here (needs per-entry `package/` root stripping), so instead directory entries never get their archive-recorded mode applied at all — matching this codebase's own `hartifactcontent::unpack`, which already only ever adds permissions during extraction, never narrows them from untrusted archive data. 2. run_lifecycle_scripts: `Command::status()` inherits the parent's stdio by default, so an allow-listed package's own lifecycle script writing to stdout/stderr went straight to the real terminal underneath heph's TUI (which owns the alternate screen for the whole run) — corrupting it. Switched to `.output()`: captured, not inherited, and a failing script's captured output now lands in the error instead of vanishing into whatever the terminal happened to be showing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…os/arch `goos`/`goarch` were copied verbatim from the Go plugin's own vocabulary (Go's actual env var names) into every js_install/thirdparty addr, config key, and function name in this crate, despite this plugin having nothing to do with Go. Per explicit request: renamed throughout plugin-js to os/arch — addr args (`@goos=linux,goarch=amd64` -> `@os=linux,arch=amd64`), JsInstallDef/JsInstallSpec fields, thirdparty_addr/node_modules_addr params, platform::current_goos/current_goarch -> current_os/current_arch. plugin-go's own goos/goarch stays untouched — that crate genuinely is about Go, and the naming is correct there. BREAKING: this changes the js_install/thirdparty addr format (`goos=`/`goarch=` args -> `os=`/`arch=`) and JsInstallDef's cache-key field names — any addr strings or targets referencing the old args are invalidated. Explicit, requested rename; plugin-js is still pre-1.0 (milestone-numbered, not yet stably released), so this is the right time for it. Only naming values (`"linux"`, `"amd64"`, canonical Go/OCI spelling) are unchanged — those still come from `hcore::htplatform`, shared with the Go plugin for a reason (npm/pnpm's own `os`/`cpu` restriction-list values are separately mapped via `platform::npm_cpu`, untouched by this rename). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
js_install's allow-listed postinstall scripts (esbuild, sharp, etc.) run in a sandbox with only their own package present, so any script that requires a sibling dependency (esbuild's install.js requiring its own @esbuild/<platform> optionalDependency) fails even when the sibling is correctly resolved in the lockfile. Resolve each package's required + optional dependencies from the already-fetched lockfile graph and declare them as driver Inputs, materialized at sandbox_ws_dir/node_modules by the engine's default placement (an ancestor of sandbox_pkg_dir, so Node's own ancestor node_modules walk finds them with no cwd change). A self-reference symlink makes the package's own name resolvable the same way. Optional dependencies that are unresolvable or platform-mismatched are skipped and noted in the lifecycle-script failure context; required ones that are unresolvable or platform-mismatched are a hard error. Also fixes a pngjs install failure: extract_tarball was applying a tarball directory entry's own restrictive permissions immediately, blocking creation of files inside it on the next entry — now uses create_dir_all for directories instead of Entry::unpack(). And lifecycle scripts now run with captured stdio instead of inherited, since inherited stdio was corrupting the TUI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…lings Sibling node_modules materialization only walked a script-owning package's *direct* dependencies/optionalDependencies. Real postinstall scripts routinely reach further: @sentry/cli's postinstall requires `which`, and `which` itself requires `isexe` — a dependency of a dependency, never a direct edge of @sentry/cli at all. The second hop was never materialized, so the script failed with a plain MODULE_NOT_FOUND for isexe. Walk the resolved graph transitively from the script package's own edges, at every depth applying the same required-hard-fail / optional-skip asymmetry per edge. seen_names dedupes (a diamond dependency isn't fetched or declared as an Input twice) and bounds the walk (a cycle back to an already-visited name is never re-queued). A cycle back to the script-owning package by name is dropped outright — already reachable via the driver's own self-reference symlink, and would otherwise be a js_install target declaring itself as its own Input. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…nal in js_test build_test_closure bucketed a resolved import as first-party whenever the resolved path fell physically inside the owning package's own directory (starts_with(pkg_dir)) — true even for a real, ambient node_modules nested inside that same package (a common on-disk shape: version-conflict dedup, or a stray local install). test_deps_config takes closure.files straight to a raw fs:file Input with no further classification, while closure.external_files goes through classify_resolved_edge onto the lockfile-driven relocated js/node_modules:group addr — so an ambient-nested third-party file landed in both buckets, and the two targets collided on the identical sandbox path (confirmed live: @apollo/client nested inside mgmt/backoffice's own node_modules). Fix: also require thirdparty_pkg_name_from_path(...).is_none() before treating an edge as first-party, matching what classify_resolved_edge already checks — an edge under any node_modules/ path component is third-party regardless of physical containment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…es materialization, fix ambient-nested classification Two related gaps in the js_test/js_typecheck/js_bundle node_modules relocation mechanism (distinct from js_install's own lifecycle-script sibling walk fixed earlier): 1. ResolvedGraph::transitive_reachable only followed `dependencies` edges, never `optionalDependencies`. vite depends on rolldown, which ships its native binding as an optionalDependencies entry per platform — the binding was never relocated into the sandbox, so vite failed to load at require-time (not a lifecycle-script failure, ordinary module resolution, one edge past what this closure declared). Now walks a platform-matching optional edge exactly like a required one; an unresolvable or mismatched one is silently skipped, mirroring the established asymmetry elsewhere in this crate. Threaded os/arch through transitive_reachable and its three callers (resolve_transitive, resolve_transitive_closure, transitive_declared_closure) to keep the phantom-dependency check and the actual Input-wiring in agreement, per resolve_transitive's own doc on why that agreement matters. 2. build_test_closure classified a resolved import as first-party whenever it fell physically inside the owning package's own directory, even when that meant a real, ambient node_modules nested inside the same package (confirmed live: @apollo/client nested inside mgmt/backoffice's own node_modules). That bucket goes straight to a raw fs:file Input with no further classification, while the sibling bucket goes through classify_resolved_edge onto the relocated js/node_modules:group addr — so the two collided on the identical sandbox path. Now also requires thirdparty_pkg_name_from_path(...).is_none() before treating an edge as first-party. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
The node_modules relocation mechanism (deps::resolve_transitive_closure, used by js_test/js_typecheck/js_bundle) placed every third-party package flat, one node_modules/<name> per name per consuming package, chosen by whichever version a BFS reached first. Real npm/pnpm lockfiles routinely resolve the same name to different versions for different consumers — diamond dependencies, which npm itself handles via nested node_modules overrides. The flat model could only ever materialize one version, so whichever consumer needed the other one broke at test/build run time with an opaque, deep Node.js module-resolution error (confirmed live: estree-walker resolved to v2.0.2 for most consumers but v3.0.3 for @module-federation/vite/@vitest/mocker specifically, and only one version was ever wired, producing ERR_PACKAGE_PATH_NOT_EXPORTED). ResolvedGraph::transitive_reachable now runs two passes: the original flat BFS (unchanged, first-reached-wins) establishes the default placement for every name, then a second pass walks every reachable node's own dependency edges and detects divergence from that default. A genuine diamond conflict gets nested one level under the diverging node's own placement — exactly where npm's own override would live, and exactly where Node's own ancestor node_modules walk looks first. Nesting is capped at one level: if a node that is itself already an override has a further diverging edge, that would require depth-2 nesting, which is a loud, named hard error rather than a silent wrong placement. node_modules_addr's own shape is unchanged for the flat case (the new nested_under arg is only ever inserted when actually nesting), so the overwhelming majority of existing targets keep an identical addr and cache key across this change. A design review (hermeticity/product-vision/feature-quality) and a follow-up code-quality review both ran on this before landing; the code-quality pass caught a real panic in the first draft (an edge whose graph key is unresolvable due to an npm: alias mismatch was `.expect()`-ed instead of skipped like every other unresolvable-edge case in this file) — fixed and regression-tested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
The just-shipped diamond-dependency fix capped override nesting at exactly one level, hard-failing anything deeper as "not supported yet". The first real repo it ran against needed depth 2 immediately: @netskope-ui/core was itself an override (its own name resolving to a different version elsewhere in the closure), and its own dependency on @floating-ui/react diverged again from that name's flat default — exactly the shape a one-level cap can't represent, so the build simply failed instead of running. ResolvedGraph::transitive_reachable's override pass is now a worklist over (graph_key, placement path) pairs instead of a single flat/override distinction: a node whose own edge diverges from the flat default gets nested one level inside whichever placement it was reached from, and its own edges are then walked the same way, so a chain of any length resolves. A node reached as a divergent target from more than one distinct placement still gets its own independently-computed subtree at each one (the two-different-parents case the previous design already handled). The only remaining hard-fail is a genuine cycle in the "who overrides whom" relation hitting a generous (16-level) safety cap — pathological input, not a real dependency tree. node_modules_addr's chain is encoded as one dedicated Addr arg per element (n0, n1, ...) rather than a single joined string, so a scoped package name in the chain never has a re-splitting ambiguity, and the flat (no-conflict) case's addr stays exactly as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
@lingui/vite-plugin calls @lingui/conf's getConfig() at plugin-load
time, which does its own lilconfig-style filesystem walk for
lingui.config.{js,cjs,ts,mjs} — never named anywhere in vitest.config.ts's
own text, so it's invisible to resolve_runner_config_referenced_files'
static scan by construction. The config was never declared as an
Input, so it was simply absent from the sandbox: "No Lingui config
found" at plugin-load time, even on a project that has one.
Verified the exact search order against @lingui/conf's real published
source (lilconfigSync("lingui", { searchPlaces: [...] })) rather than
guessing. Only the file-based forms are handled; package.json's
"lingui" key and the .linguirc dotfile forms are out of scope for now
since they're rare in practice.
This is a narrow, precedented special case for the one plugin actually
confirmed to need it — a general escape hatch for arbitrary
cosmiconfig-based tools (postcss, tailwind, babel, ...) is a bigger,
separate design decision heph doesn't have a mechanism for yet.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
Confirmed live: the previous filename-guessing special case only
found lingui.config.{js,cjs,ts,mjs} — lingui's own default search
filenames. A project using a non-default filename (config.lingui.ts)
passes it explicitly via lingui({ configPath: '...' }), which makes
@lingui/conf's getConfig() skip its own default search entirely and
use that path verbatim — invisible to the filename table no matter
what it guesses.
Extend the existing setupFiles/globalSetup-style key scan
(RUNNER_CONFIG_FILE_KEYS) with a parallel PLUGIN_CONFIG_PATH_KEYS
list (currently just "configPath"), resolved as a plain filesystem
path rather than an import specifier — lingui's own configPath value
isn't required to have a "./" prefix the way a setupFiles import is.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…r, not workspace root Root cause of the Lingui config still not being found after two prior fixes staged it correctly: js_test invoked vitest with cwd = sandbox_ws_dir (the sandboxed workspace root), not the package's own directory. lingui() is called with no options in the reported config, so @lingui/conf's getConfig() falls through to its own ambient search from process.cwd() — and that search only ever walks *ancestor* directories. With cwd = workspace root, a config living in the package directory (a descendant, never an ancestor) could never be found no matter how correctly it was staged. Every path argument these four drivers already pass to their tool (tsconfig_abs, entry_abs, outdir_abs, config_abs, test_file_abs, and group_staged_paths' list-file entries) is already absolute, so cwd never mattered for *finding* those — it only matters for a tool's own ambient, cwd-relative behavior. sandbox_pkg_dir is also what a real, non-heph invocation actually runs with in practice (cd package && vitest, pnpm --filter pkg test), so this fixes the whole class of bug for any tool doing cosmiconfig-style ambient discovery, not just Lingui specifically. Added a regression test using a tiny always-failing shell script (no real vitest/jest needed) that reports its own $PWD through this driver's existing failure-detail path — verified it fails against the old cwd choice and passes against the new one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
test_failure_detail/tsc_failure_detail/lint_failure_detail/bundle_failure_detail all truncated stdout/stderr to their last N lines. A tool that prints its error name/message first and a long stack/diagnostic after (vitest's own error dump does this) had the actual message silently cut, leaving only orphaned trailing stack frames with no indication anything was omitted. hplugin::error::head_and_tail_lines keeps both ends, with an omission marker between, so the leading message always survives regardless of how long the tail is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
Real failure: js_test crashed with `Error: Cannot find package 'playwright'`. vitest optionally depends on @vitest/browser-playwright (walked fine via existing optionalDependencies handling), which unconditionally imports playwright at module load despite declaring it only as a peerDependencies entry. ResolvedPackage had no field for peer deps at all, so transitive_reachable could never walk to a genuinely- installed peer. - ResolvedPackage gains peer_dependencies. npm resolves it via the same ancestor-node_modules walk already used for dependencies/optionalDependencies (npm's package-lock.json never writes a resolved edge for a peer, only a bare copy of the package.json range). - transitive_reachable's flat and override passes both walk peer edges with the same platform-gated, miss-tolerant treatment as optional deps. - provider.rs's lifecycle-script sibling-resolution walk (js_install postinstall requires) gets the identical peer-edge treatment — same defect class, different call site. While verifying pnpm needed no equivalent change, found and fixed a second, unrelated, pre-existing bug: pnpm's `packages:` section keys a package with its own peerDependencies WITHOUT a peer suffix (confirmed against a real `pnpm@9.15.9 install`), but `snapshots:` keys the same package WITH one. resolved_graph() joined them on the same key, so the lookup always missed for any such package — silently leaving ALL of its dependencies empty, not just the peer edge, for any pnpm project using a package like react-dom. Fixed by driving the graph from snapshots (source of dependency edges) with metadata joined via the stripped peer suffix. Known, pre-existing, documented limitation (not introduced here, not fixed): pass 1's flat-reachability BFS has no bias toward `dependencies` edges over optional/peer edges for the same name — whichever is reached first in seed/BFS order wins the flat placement. optional_dependencies already shared this exposure; peer deps widen it since a peer's whole purpose is naming a widely-shared library likely to also have a real dependencies edge elsewhere. Sandbox materialization stays correct regardless (pass 2 re-derives the right per-consumer placement via nested overrides) — the residual risk is narrower: resolve_transitive's single-name flat-only lookup, used only for a first-party import with no declared edge of its own. Pinned with a test (transitive_reachable_flat_placement_prefers_whichever_edge_type_is_seeded_first_not_dependencies) rather than redesigned here — changing pass 1's priority is a separate, cross-cutting design change affecting already-shipped optional-dependency behavior too, not something to bundle into this fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
heph's js_test invocation is headless by construction — no TTY, no human able to answer a prompt or press a key — exactly what CI conventionally signals to a well-behaved CLI tool. Defense-in-depth alongside the existing explicit `run` subcommand (which already defeats vitest's own default watch-mode selection): vitest and jest both gate other interactive behavior (reporter live-redraw, keypress handling) on isCI/isTTY checks beyond just watch-vs-run selection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…adline CI=1 and the run subcommand only reduce the odds vitest wedges after finishing its own work (a confirmed live upstream failure mode: a failed dependency-optimizer scan leaving a background esbuild service alive) — they don't bound it. proc_exec::output otherwise waits on the child's own exit with no timeout at all, so a wedged runner hangs the whole heph run forever with no distinguishing signal from "still working". DeadlineCancellable composes the caller's own cancellation token with a deadline (fires on whichever comes first) rather than wrapping the subprocess future in a bare tokio::time::timeout, which would just drop the future and leak the orphaned child. Composing tokens instead routes through proc_exec's existing SIGINT-then-grace-then-SIGKILL teardown — the same path a real user cancellation already goes through — and the original ctoken is never itself touched, so a target that merely timed out is never mistaken upstream for a user-cancelled run. Verified proc_exec::output actually errors (not a killed-but-Ok status) once its cancellable fires, confirmed empirically rather than assumed, and regression-tested against a real subprocess that actively ignores SIGINT — the exact shape of a wedged child that won't respond to a plain interrupt. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
…nput
Mirrors plugin-go's go_src mechanism: a BUILD author marks an ordinary
codegen target labels = ["js_src"]; each of the 4 JS driver Input-
computation functions (test/typecheck/lint/bundle _deps_config) now
queries "{scope} && label(js_src) && tree_output(pkg)" and wires the
result in as a declared Input, alongside the existing import-graph-
derived ones. Because it's a declared Input, the engine schedules and
runs the codegen target automatically before whichever target needs
it, and stages its output into the sandbox before the driver's
subprocess (vitest/tsc/eslint/esbuild) runs — no change needed to
importgraph.rs's own resolution walk, since the downstream tool
resolves the generated import itself once the file physically exists,
exactly as `go list` already does for go_src.
Fixes the real reported gap: a js_test importing GraphQL-codegen
output previously required a human to remember to run the codegen
target first.
Design reviewed in parallel by product-vision/hermeticity/feature-
quality before implementation (per this repo's review-board process).
Consensus: mirror go_src's mechanism, but two things needed resolving
first, both addressed here:
- Scope: go_src's *default* scope (no explicit root override) is
already the package's subtree, not the bare package — go_src's own
history is why: a bare-package floor shipped a real bug (an embed
that resolved on some runs and failed on others, when a generator
sits in a sub-package of the consumer). js_codegen_default_scope
mirrors this exactly, confirmed sufficient for the reported case
(codegen output lands inside the consuming package itself). No
go_codegen_root equivalent (ancestor-scope-widening for codegen
landing in a *different* package) yet — deferred, disclosed in
js_src_query_addr's doc.
- Cost: js_test has per-*test-file* target granularity, not go_src's
per-*package* granularity, so the query must resolve once per
package and be shared across every test file and all 4 drivers, not
recomputed per target. js_src_query_addr is a single pure function
every call site routes through unchanged, so all 4 _deps_config
functions format the byte-identical query addr for the same package
by construction — the engine's own per-addr memoization does the
rest. Also excludes the js provider from the query (mirroring go_src
excluding go) — without it, every js_test/typecheck/lint/bundle
candidate in scope would pay a full get_spec just to be rejected on
the label check.
Disclosed, not silently inherited: a genuine dependency cycle through
a js_src-labelled target is silently dropped from the query match by
the engine's own existing query-cycle handling (the identical,
already-shipped go_src behavior, not new here) — no dedicated
diagnostic for this case yet.
Known gap, not yet covered: no in-process e2e test exists for this
(crates/e2e has no JS harness at all yet, a separate, pre-existing
infrastructure gap; real vitest also isn't available in this devenv).
Unit-tested at the provider layer: query construction (scope/label-
before-tree_output ordering/js-provider exclusion), and that all four
_deps_config functions wire the identical codegen addr for the same
package. Not yet covered: ported cycle-containment regressions
(plugin-go's own 5-test suite for this exact hazard class), failure
attribution when a js_src target itself fails, and output-collision
handling for two conflicting js_src targets.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLhaQSMoXo1ExzAeCbu3an
raphaelvigee
force-pushed
the
raphaelvigee/feat-plugin-js-m8-bin-e2e
branch
from
August 12, 2026 22:02
a44ec05 to
4de9d21
Compare
This was referenced Aug 12, 2026
Member
Author
|
Superseded by #394 — retargeting this PR's base to master was blocked by GitHub's stack tracking even after closing/unlinking, so a fresh PR was opened from the same branch directly against master instead. |
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
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.
Part 8/8 of the stack — closes a gap flagged repeatedly since M0:
plugin-js-cdylibwas lint/fmt covered (devenv.nix'squalityCrates) but never actually loaded across the real ABI seam by any test.What's here
devenv.nix'se2escript now stagesplugin-js-cdylibas a fourth artifact, symmetric with go/gha in both the local-build andHEPH_E2E_FROM/CI branches (fingerprinting, copying, macOS-portable-patching all updated together).bin-e2etests incrates/bin-e2e/tests/plugin_dylib_js.rs:shipped_js_cdylib_loads_and_answers_across_the_abi— construction + the syncinspect functionsABI call round-trips cleanly.shipped_js_cdylib_discovers_a_real_workspace_package—query -e //...against a real pnpm-shaped fixture resolves//packages/foo:package_info.shipped_js_cdylib_resolves_pnpm_workspace_glob_membership— proves a package outside thepackages/*glob is never admitted as a workspace member, across the real seam.js_plugin_construction_failure_logs_before_the_abort— the same log-sink-before-abort regression coverage the go test already has.Two things review caught worth naming
The first draft of
shipped_js_cdylib_loads_and_answers_across_the_abiassertedinspect functionsproduced completely empty stdout — wrong, and it failed deterministically: thefsbuiltin provider's own functions are always registered regardless of plugin config, so stdout is never actually empty. Fixed to assert no line starts withjs.instead (the jsProvidernever overridesfunctions(), unlike go'sbuild_addr).The discovery test's own doc comments originally claimed to exercise pnpm's workspace-glob membership resolution — they didn't. Traced it:
Provider::list/list_packagesdiscover packages bypackage.jsonpresence alone, independent ofpnpm-workspace.yamlcontent entirely; the glob-resolution path (workspace.rs'sresolve_members) is only reached fromProvider::get, which a plainquery -e //...never calls. Rather than leave an overclaiming comment standing, added the dedicatedshipped_js_cdylib_resolves_pnpm_workspace_glob_membershiptest that actually reaches that path, and corrected the original test's docs.Verified for real, not just compiled
Ran the full
e2edevenv script end to end — release build ofheph+ all three plugin cdylibs, staged, macOS-portable-patched, fullbin-e2esuite against real dlopen'd artifacts:Test plan
cargo build -p bin-e2e --testscargo clippy -p bin-e2e --all-targets -- -D warnings(clean)cargo fmt --check -p bin-e2enix-instantiate --parse devenv.nixe2erun against real staged release artifacts — 17/17 passedtst,lint,bin_e2e) on this PRStack created with GitHub Stacks CLI • Give Feedback 💬