Repository navigation
fix(stryker-js): read package manifest exports per entry so one shape cannot refuse the whole file - #330
Merged
Conversation
…e manifest Solution doc for the import-closure decode regression found in review of PR #308: an `imports` key typed narrower than Node's grammar made a null, array or nested-condition entry fail the whole package.json decode, so exports/main/module were lost and every importing closure opened. States the refuse-at-the-granularity-of-use invariant, the grep-able smell, and the instance still left in `exports` for workspace-linked packages. Seeds CONCEPTS.md with the import closure and closure digest entries.
…' into stream-d/fix-manifest-exports-decode
…hape cannot refuse the whole file
… cannot refuse the whole file PackageManifestSchema decodes exports, imports, main and module as JSON. selectManifestTarget reads entries at any depth (source condition, else first value; arrays by their first element), treats a present entry without a target as final, and reads a top-level null exports as absent, as Node does. Plan: docs/plans/2026-10-13-0239-fix-manifest-exports-decode-plan.md (supersedes the 0230 draft).
…e its exports take Built-CLI suite: a project whose test imports a workspace-linked package runs twice around an edit to a file no test imports; the second run must reuse every verdict for flat, nested-condition, array-fallback and null-subpath exports. On main the last three refuse with closureChanged. Simplifications from ce-simplify-code: one exact-entry reader, stringOf covers absent main/module, root entries gated by Boolean.match, events decoded through RunEventWireLine.
…linked package exports Review fixes for the manifest exports unit. The root law now draws a string and a one-element array root (fails against main's decision). The built-CLI feature becomes an outline over five shapes, adding a top-level array, plus a scenario that edits only the linked package's runtime file and expects every mutant tested again: the guard that a types-first condition closing on the .d.ts never keeps a stale verdict while the runner reports testFileModules. The lesson records the exports fix and that hazard instead of the pre-fix 'one instance remains' claim.
…untime loads A condition object took @systemfsoftware/source or else its first value, so a types-first package closed its importers' closures over a .d.ts no runtime loads, and a runner without testFileModules reused stale verdicts after an edit to the package's .js (review P1 on 14a06fd). Past types/types@..., an import, node or default condition is followed; any other first condition leaves the closure open. An exports object with no '.'-prefixed key is read as the root's conditions, and a mixed one yields no target, as in Node. Gate: a closure-only runner fixture in the exports-reuse feature, whose nested and flat types-first runtime-edit rows and root-conditions row fail on the first-value rule.
…dition rule The 0239 plan still chose the first-value rule and rejected Node's runtime condition set, which f6d463b ships (review of f6d463b: correctness P3, adversarial P2). The 0441 plan restates R3 per conductor r46 (resolve to the file the runtime loads, or stay open), records the rule in KTD2 with its rejected alternatives, states Node's array continuation rule, and adds U4 (the closure-only runner gate). The missing-target law now also pins a condition object holding only types conditions (testing gap).
…he resolver chooses from PackageManifestSchema keeps the foreign package.json as its Encoded side and decodes exports, imports, main and module into PackageEntryField: EntryPath, EntryFallbacks, EntryMap (key order kept), EntryNull and EntryScalar, one variant per JSON shape, with EntryUndeclared for an absent key. The decode is total over JSON, and the generated round-trip, encode-stability and recursion laws grade it from the Type side (schema-laws rich-type-over-foreign-encoded, root ruling r49). selectManifestTarget chooses the target by dispatching exhaustively on the entry's tag; which condition, fallback and key form counts is unchanged. The laws keep writing package.json JSON and reach the decision through the production codec. Plan 0519 supersedes 0441 (KTD1, U1, scope).
…onJS condition sets Full-path review of 07f55c9 confirmed a P2: an import-first condition object closed a CommonJS importer over the ESM file it never loads. selectManifestTarget now reads each entry twice, as Node reads it, under the ESM set (import, node, module-sync, node-addons, default) and the CommonJS set (require in place of import), skipping keys outside the set. One file from both reads, or a file from one and none from the other, is the target; two different files keep the importers' closures open. Arrays skip numbers, booleans and objects no condition matches; a number or boolean inside a condition object yields no target. Laws: the fall-through, ESM/CommonJS and fallbacks laws fail on 07f55c9 and pass on this head. Plan 0546 supersedes 0519.
…ver against Node A differential spec now resolves generated exports and imports entries with a real node process (import.meta.resolve and require.resolve) and compares the import closure Stryker builds for the same project (boundary-testing pin-dependency-semantics, reported violated by #330's review ledger). Its first runs found two array rules that disagreed with Node's resolvePackageTarget (lib/internal/modules/esm/resolve.js:488-520, v24.x): an empty array yields null, so a condition holding [] blocks instead of falling through; and a null element is skipped, not final. selectManifestTarget now reads arrays that way, and the fallbacks law states it. Two-sided: the spec fails on 07f55c9 and on 72410dd, and passes on this head. Plan 0606 supersedes 0546.
… over them A delta correctness lens over 3eee2a4 (run 20261013-061107-f1739e97, zero findings) recorded that the reader took string targets Node's resolvePackageTarget rejects (resolve.js:389-423, v24.x): an exports string without ./, or one with a ., .. or node_modules segment, or an imports string starting with / or ../. selectManifestTarget judged a string only after choosing it, so an array stopped at a string Node skips, and exports {"./x": "/a.js"} closed over pkgs/lib/a.js. The reader now judges each string as it reads it: a rejected target yields no file, and inside an array it is skipped. The differential spec now draws rejected targets (a.js for exports only; in imports it names a package) and shrank the stale case on 3eee2a4; the new string-targets law fails there too. Both pass on this head. Plan 0620 supersedes 0606.
…g module past a non-string main Full-path review of e839304 (run 20261013-063937-1f11f4b8) confirmed two P2s and one pack violation. - A condition object with an array-index key was read as conditions; Node refuses it with ERR_INVALID_PACKAGE_CONFIG (resolve.js:522-530) and rethrows that inside an array (:501-506). The reader now carries the refusal in a failure channel: an object returns it, an array stops at it, and the entry yields no target. - A non-string main fell through to module, which Node never reads; on main such a manifest failed to decode and left the closure open. A present non-string main now yields no target, and module counts only when main is absent. - schema-laws tests-own-no-schemas: the differential spec's probe schemas move from tests/__fixtures__/node-resolution.schema.ts into the reference harness tests/__fixtures__/node-resolution.fixture.ts, unexported. The differential spec now also draws node-addons and the array index 0. The two new laws and the spec fail on e839304 and pass here. Plan 0656 supersedes 0620.
… law Delta lens run 20261013-070109-64000e1b over 958d299 found 0 issues and noted that the law drew only a number and an array for main, leaving the EntryMap, EntryNull and boolean arms of mainTargetOption unasserted. The law now draws a number, a boolean, null, an array and an object.
ryanleecode
approved these changes
Oct 13, 2026
ryanleecode
left a comment
ryanleecode
left a comment
Contributor
There was a problem hiding this comment.
Root verdict: e750228 green apart from inherited bench; 0 threads; up to date with main 2083ba4; full-path round 4 (20261013-063937-1f11f4b8) plus delta lenses 20261013-070109-64000e1b and 20261013-072719-49846993 cover the head on DeepSeek v4.1; both pack violations (pin-dependency-semantics, tests-own-no-schemas) fixed with commits named; built-CLI scenarios pin the manifest reading.
This was referenced Oct 13, 2026
This was referenced Oct 13, 2026
Merged
systemfsoftware-maker
added a commit
that referenced
this pull request
Oct 13, 2026
…t of the next run's inputs Takes main through b4db6d8 (#313 root-bound dry-run coverage, #314, #317, #321, #322, #326, #330, #331, #332, #334, #335, #337). Merge resolutions: - dry-run-choice: main's current-root candidate filter over the one incremental file this branch reads; prior statuses come from the verdict store, flaky ids from the kept coverage. - dry-run.cell, deferrable-dry-run.cell, plan-request.cell: pass that one report text into main's choice; the plan prices from the kept coverage. - typescript-checker: main moved the runtime into typescript-checker-runtime.ts; it carries this branch's config/program digest scope. - e2e StrykerCliRunner: main split service from layer; runScript and runInFork move into microsandbox-stryker-cli-runner.ts. - Tests: main's flaky-root CompileError scenario writes the incremental file instead of the removed incrementalSources; the relocated-project multi-report scenario and dry-run-coverage-across-roots go with incrementalSources; the version-gate test uses layerStage. - The solution doc keeps both the store key and the root binding. Fix found while validating the merge: "A run killed after it scored its first mutant spares the next run its initial test run" failed 2 of 4 runs. The fixture kills the CLI at the first mutant event (run/mutant-run.ts:264-265), which can land inside the checkpoint's atomic write of the incremental file (mutation-reporting.service.ts:754). SIGKILL skips the temp file's cleanup (replace-file-atomically.ts:44), so reports/.main.json.<hex>.tmp stays. The crawl excluded only main.json (stryker-outputs.ts, read-project.cell.ts:116-124), so the temp file became a project file and the next run refused its dry run with closureChanged. replace-file-atomically.ts now owns the temp name and the rule matching it, and strykerOutputFilesOf excludes the incremental file's temp siblings. A new scenario plants that temp file between two runs of the built CLI; on the unfixed build the second run executed 2 tests, and it now executes none. The changeset says what holds for a killed run.
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.
Follow-up to #308 (Stream D; intents r41, r46, r49). Plan:
docs/plans/2026-10-13-0656-fix-manifest-exports-decode-plan.md. It supersedes 0620, 0606, 0546, 0519, 0441 and 0239, none of which reachedmain.Outcome
On
main, a project that imports a workspace-linked package re-tests every mutant after any edit when that package'spackage.jsondeclaresexportswith nested condition objects, array fallbacks, a top-level array or anulltarget. The whole manifest fails to decode, the closure stays open, and each run refuses reuse withclosureChanged. This repo hits it:vitest-config,tsdown-config,stryker-configandoxlint-ignorer-configall declare{ ".": { "import": { "types": "...", "default": "..." } } }.After this PR, each
exports,imports,mainandmoduleentry is read on its own, the way Node's resolver reads it. An unrelated edit keeps every verdict. A closure closes only on the file Node loads for both ESM and CommonJS importers. When the two load different files, or Node loads none, the importers' closures stay open.What changes
PackageManifestSchema(import-closure.schema.ts, r49). The foreignpackage.jsonstays the Encoded side.exports,imports,mainandmoduledecode intoPackageEntryField, a tagged tree with one variant per JSON shape:EntryPath,EntryFallbacks,EntryMap(key order kept),EntryNullandEntryScalar, plusEntryUndeclaredfor an absent key. The decode is total over JSON. The generated round-trip, encode-stability and recursion laws grade it.ExportEntrySchemais deleted.selectManifestTarget(select-manifest-target.workflow.ts) reads every entry twice, under Node's ESM conditions (import,node,module-sync,node-addons,default) and its CommonJS conditions (requirein place ofimport). Its rules follow Node v24'sresolvePackageTarget(lib/internal/modules/esm/resolve.js:377-548):exportsit starts with./and has no.,..ornode_modulessegment; inimports, likewise or a bare package specifier. A rejected string yields no file.@systemfsoftware/sourceif present. Otherwise its first key in the condition set decides; every other key (types,browser,module, user conditions) is skipped.0,1, …) refuses the read, as Node'sERR_INVALID_PACKAGE_CONFIGdoes (:522-530). Node rethrows that error inside an array (:501-506), so the reader carries the refusal in a failure channel: an array stops at it instead of skipping it.null, numbers, booleans, rejected strings and unmatched objects. An empty array, or one with no file and anullor rejected element, yields none.mainrescues it.exportsobject with no.key) serve the root. Mixed keys yield no target. For the root fallback, a stringmainis the target, a present non-stringmainyields no target, andmoduleis read only whenmainis absent (Node never readsmodule).src/__tests__/select-manifest-target.workflow.property.test.ts): 23, all drawn from the productionManifestTargetCommandand written aspackage.jsonJSON through the codec. New or rewritten this PR: the condition chain at every depth; unmatched conditions falling through; ESM and CommonJS loading different files; root conditions; the source condition at its own depth; array fallbacks; rejected string targets; array-index condition keys at every depth;nullexports;main/module(every non-stringmainshape).tests/manifest-target-node-resolution.differential.test.ts,@systemfsoftware/differential-spec, already a devDependency). It generatesexportsandimportsentry trees (conditions includenode-addonsand the array index0) and writes each to a real temp project. The reference is a realnodeprocess resolving#xand@fx/lib/xwithimport.meta.resolveandrequire.resolve. The candidate isImportClosure.analyzeImportClosure, reached through the public barrel. Oracle: the closure must equal the file both of Node's answers agree on, or the one answer when the other is none; otherwise the closure must be open. Host-bound lane, 100 runs. The probe and its output decode live in the reference harnesstests/__fixtures__/node-resolution.fixture.ts.tests/workspace-package-exports-reuse.integration.test.ts, 12 rows). It spawnsdist/main.mjs runtwice around an edit and reads only exit codes and the machine-modereuseline. It covers everyexportsshape around an unrelated edit, and a runtime-only edit (types-first packages, root conditions, a dualimport/requirepackage required by its test). Rows run on thevmrunner and on an on-disk runner fixture,tests/__fixtures__/closure-only-runner/, which reports no loaded modules.docs/solutions/performance-issues/strict-optional-key-…-refuses-the-whole-manifest.md),CONCEPTS.md(import closure, closure digest), plan 0656, and a patch changeset for@systemfsoftware/stryker-js.Evidence (local, parent-run)
e75022860main(round 1)null-subpath rows fail; flat control passes14a06fdab(first-value rule)types-first runtime edits fail (stale verdicts)import/requirerow on07f55c9f9(importfollowed whatever its siblings)07f55c9f972410ddba/3eee2a48ce8393042d07f55c9f9imports{"require": "./a.js", "types": …}(Node's CommonJS side loadsa.js; closure open)72410ddba{"module-sync": [], "node": "./a.js"}(Node blocks; closure closed ona.js)3eee2a48c(with rejected strings drawn)exports"/a.js"(Node rejects; closure closed onpkgs/lib/a.js)e8393042d(with0drawn)importsobject holding"0"(Node refuses it; closure closed ona.js)stryker-jsfull suite on958d299cf's treee75022860(widens one law only)lint:tsgo,dprintpasslint:tsgo,dprint, changeset and lesson validatorsThe old sides ran in a scratch
git worktreeunder/tmp. Nothing was reverted in this checkout.Review receipts
Round 4, full path (
ce-code-review, report-only): run20261013-063937-1f11f4b8, base2083ba43a, reviewed heade8393042d. The validator confirmed two P2s and rejected the P1; the learnings ledger found one pack violation. All three are fixed in958d299cf, so the verdict one8393042dwas Not ready. As in round 3, I fixed the findings before a report leaf ran, so the run has noreview.json.correctness-reviewergeneralist-reviewergeneralist-revieweradversarial-reviewergeneralist-reviewercorrectness-reviewerPack rules on
e8393042d: 10 comply, 1 violate. The violation isschema-laws/tests-own-no-schemas: the differential spec's probe schemas sat intests/__fixtures__/node-resolution.schema.ts. Fixed in958d299cf: they moved, unexported, into the reference harnesstests/__fixtures__/node-resolution.fixture.ts.indexOf('*') + 1(resolve.js:673-681), which is monotone in the reader's prefix length. Mynodeprobe of overlapping patterns in both key orders matched the reader.958d299cf: refusal channel,∀c_ConditionObject_≡AnArrayIndexKeyRefusesTheEntryEvenInsideAnArray, and the differential spec draws0. Both fail one8393042d.maincloses the root onmodule, which Node never loads958d299cf:∀c_MainOrModule_≡ANonStringMainYieldsNoneAndModuleCountsOnlyWithoutMain(fails one8393042d);e75022860widens it to every non-string shape.Delta lenses after round 4 (correctness only, report-only; each over exactly the commits named):
20261013-070109-64000e1be8393042d..958d299cfnullor booleanmain) is closed ine75022860.20261013-072719-49846993958d299cf..e75022860(current head)mainrows fails against the pre-958d299cfmainTargetOptionand holds at head.Residual risks the
20261013-072719-49846993lens recorded, all recorded with no code change:main/moduleroot fallback is pinned only by the law. The differential spec keeps it out of its program space (plan 0656 U5). Node v24 loads<pkg>/index.jsfor a non-stringmain, while the reader keeps the closure open: safe direction.modulecloses overmodule, which Node never loads (it loadsindex.js). This behaviour already existed on themainbranch, and this PR leaves it unchanged. It is the same class as the held runner-conditions question (Decisions, item 1), which stays with root.Round 4 residual risks and gaps, and what happened to each:
node-addons; no case had an array-index key: fixed (both now drawn by the differential spec).bestMatchand throwsexportsNotFoundwhen it yields nothing (resolve.js:644-663,:739-746), as the reader does.exportspresent with no entry for the request falls back tomain, where Node throwsERR_PACKAGE_PATH_NOT_EXPORTED: pre-existing, byte-identical tomain(round 3's P3, rejected). The import fails at runtime either way.noderuns the suite (24.x in CI), and the spec spawnsnode100 times under a 240 s host bound: accepted. Pinning to the runtime CI actually ships with is the intent.@systemfsoftware/sourcewins unconditionally: accepted. The workspace enables it in every resolver it runs (tsconfigcustomConditions, Vitestresolve.conditions).exports;imports/main/moduleare proved at theanalyzeImportClosurelayer and by the laws: recorded.mainmakes Node loadindex.js, while the reader keeps the closure open: accepted. That direction costs reuse and is never stale.Round 3, full path (
ce-code-review mode:agent base:origin/main, report-only): run20261013-052410-5e2140d5, base117bff023, reviewed head07f55c9f9. The validator confirmed one P2 and rejected one P3, so the verdict on07f55c9f9was Not ready. I fixed the P2 before the report leaf ran, so the run has noreview.json.correctness-reviewergeneralist-reviewergeneralist-revieweradversarial-reviewergeneralist-reviewercorrectness-reviewerPack rules on
07f55c9f9(enrolled by.compound-engineering/config.yaml:1-4): 10 comply, 1 violate. The violation isboundary-testing/pin-dependency-semanticsatselect-manifest-target.workflow.ts:64: the reader encoded Node's resolver, and nothing pinned it against Node. Fixed in3eee2a48cby the differential spec. The complying rows:cell-architecturedecode-never-cast and pure-decision-workflows;boundary-testingno-mocks-on-internal-glue and real-system-oracles;schema-lawslaw-failure-is-a-codec-defect, recursive-schema-suspend, refusals-beside-generated-laws, rich-type-over-foreign-encoded (resolved by r49; no exception held), tagged-unions-over-state-by-presence and tests-own-no-schemas.importbranch it never loads590361469(merged as72410ddba): ESM and CommonJS reads must agree. Pinned against Node by the differential spec (3eee2a48c).exports: {}falls back tomain, an untested boundarymainbehaves byte-identically.∀c_RootEntry_≡MainDecidesTheRootOnlyWhenExportsDeclaresNoneasserts it on purpose.Delta lenses after round 3 (correctness only, report-only; each over exactly the commits named):
20261013-061107-f1739e9707f55c9f9..590361469and72410ddba..3eee2a48c(the merge ofmainexcluded)20261013-063104-4f686a1b3eee2a48c..e8393042dResidual risks those lenses recorded, and what happened to each:
e8393042d(a rejected string yields no file and is skipped inside an array), and the differential spec now draws rejected strings../%2e%2e/a.js) are accepted where Node rejects them. The named file cannot exist, so the closure stays open: slow, never stale. Out of scope, recorded in plan 0656.importstargets get no segment check (unconfirmed consequence; unchanged by this PR): recorded.No xai or Grok model appears in any lens transcript. Each round's candidate findings were merged by the orchestrator from the lens artifacts and judged by the validator leaf.
Earlier rounds (all fixed or rejected, each with its fixing commit in the PR history): round 2 (run
20261013-043819-d9c96519, headf6d463bec) found that the plan contradicted the code, fixed inf10aa825c. Round 1 (run20261013-032850-f4ff0347, head14a06fdab) found that nestedtypes-first targets keep stale verdicts (P1), fixed inf6d463bec. Its deferred array finding is now fixed, since arrays follow Node.Decisions for you
browser,module,development,--conditions, Vite's SSR conditions). The reader follows Node's default sets and skips such keys. Take a package with{ "module": "./m.js", "default": "./d.js" }on a runner that both enablesmoduleand reports no loaded modules: when onlym.jschanges, that runner can reuse a stale verdict. Vitest reports the modules each test loaded (testFileModules), and those join the closure, so the built-in runners are covered. The alternative, keeping a closure open whenever a non-Node condition precedes the chosen key, costs reuse for every{ module, default }package. My recommendation: keep Node's rule. Say if you want the conservative one.Mutation
No local mutation runs. Latest completed
mainMutation run:38349712161(add57742a). Forselect-manifest-target.workflow.tsit reports 45 killed and 4 survived:bf8fc33bfe874ae6,c1c49ef7599681d9,f00516bf7f032d1aandecd4fec1e600291a, all inpatternMatchOf's guards, which this PR does not change and these tests do not target. The new reader's mutants get their first grade on the nextmainrun.Notes
main, unstacked, per r41. No changed file is shared with any of my other open PRs.nodein tests: thewrite-gherkin-integration-testsskill refuses process spawns (WGI-CLS1). Ryan's rule for this unit requires the built CLI, and Node is the reference the differential spec pins against.