Skip to content

build: AB7014 from consumer-runtime evidence (option A); gate the generated-JS walk on the compile evidence record (#619 steps b–d) - #634

Merged
ScriptedAlchemy merged 8 commits into
mainfrom
feat/619-evidence-ab7014
Sep 5, 2026
Merged

build: AB7014 from consumer-runtime evidence (option A); gate the generated-JS walk on the compile evidence record (#619 steps b–d)#634
ScriptedAlchemy merged 8 commits into
mainfrom
feat/619-evidence-ab7014

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Steps (b) and (d) of #619's reordered PR 3 (owner audit 17:11), rebased onto #638 (step a). The coverage matrix (step c) is posted on #619; this PR deletes exactly what it covers and gates the rest.

AB7014 — option A

prepack judges an installed dependency used only when a consumer must have it installed:

  • a prebuilt payload declares it in runtimeDependencies (definePrebuilt);
  • a packed .d.ts references it — read with TypeScript's preProcessFile (imports, /// <reference types>, #subpath through the imports map), not a regex;
  • a consumer-side install script names or runs it (command-position bin, node_modules/<name>/ file, bare --import/--require preload) — the same grammar that escalates AB7015.

A package the build inlined is not used; the diagnostic names the dist bundles that inlined it, read from PackageBuildResult.evidence (#638) rather than a scan. prepack no longer reads packed JavaScript at all: require/createRequire/import.meta.resolve literals, bin-command strings, inline node -e programs, files traced from install scripts, and the "computed load withholds the check" rule are gone (matrix rows 8–14). pack-dependencies.ts loses ~660 lines.

Scanner deletion, gated on the record

  • Deleted: rslib.ts assertNoResidualReservedImports — a reserved specifier kept external is a package ExternalIR and fails compile-time AB6005; guardReservedExternals and reservedAliasViolation cover the function-form and alias routes (row 7).
  • Gated: the emitted-module walk (validate-artifact-modules.ts) stays, keyed on provenModules. A manifest bundle file is proven when the artifact's agent-bundle.compile-evidence.json is AB6039-clean and coverage.rewritable is false; a proven file is lexed, not parsed, and every import it still carries is held to the record: a Node built-in or one of the file's recorded externals passes, a non-literal import() fails (the form Rslib leaves verbatim), and any other literal request fails too — the compiler neither bundled nor recorded it, which is what an rspackIgnore/webpackIgnore import looks like (Rspack leaves it verbatim with no module, external, warning, or dependency object). A matching digest proves the bytes are the compiler's, never that every import was resolved. Every other module — no record, a hatch build, install.mjs, copied scripts — keeps the full acorn parse and import resolution (rows 4–6). ValidateArtifactOptions.bundleSyntaxCheck / bundleSyntaxCheckFor are removed; the record decides.
  • Kept, with a note: the non-literal import() finding for every module, and the walk over JavaScript the framework did not compile. What still needs a replacement is recorded in the matrix and in docs/entry-conventions.md.

The package build applies the same gate to dist: proven unless a tools hatch ran.

Tests

  • build.test.ts: a compiled script with import(name) fails build() with AB6005 … has a non-literal dynamic import (compiler evidence cannot see it; the walk does); import('left-pad' /* rspackIgnore: true */) and ignored missing/outside/sibling relatives fail AB6005 … loads "<request>", which the compiler neither bundled nor recorded as an external; a relocated artifact (source deleted) validates from its record alone and still reports those; hatch build parses in full.
  • package-build.test.ts: the same ignored-import and expression-import fixtures fail the dist walk and leave dist unpublished; evidence.assets[].packages attributes node_modules, linked, and nested-linked dependencies.
  • artifact-validator.test.ts: walk cases restored; record fixtures for no record / covered / rewritable: true / malformed.
  • prepack.test.ts: inlined-dependency AB7014 names the bundles; declaration, install-script, and runtimeDependencies acceptance; compile-time AB6005 wins over AB7014 when only a dist bundle imports a declared dependency.
  • generated-module-evidence.test.ts: the framework's own runtime modules resolve no package (the process-dependency record is empty).

Docs / changeset

AGENTS.md ("Proof is bytes and processes" → compiler evidence + packed-process tests), docs/diagnostics.md (AB6005, AB7014, AB6039 gate note), docs/entry-conventions.md, website en+zh validation, architecture, package-entries, mcp, targets-artifacts. One minor changeset (AB7014 evidence changes; internal bundleSyntaxCheck removal is not release-noted).

Deslop

Fable-5.1-high, rebase pass: one validateCompileEvidence (reads, parses, re-judges, and derives provenPaths) instead of two readers emitting AB6039 twice; publishedPrefix reused for the dist walk's reportedRoot; duplicate imports merged; AB6039 message forms aligned with #638.

Self-review

Reviewer: gpt-5.6-sol-medium (change-risk pass over the diff vs origin/main, 4 findings), all fixed in 9e33a0c:

  1. Workspace-linked dependencies lost ModuleIR.package (rslib.ts only recognised node_modules segments; Rspack records symlinked workspaces at their real path, so AB7014 could not name the bundle that inlined them). Fixed: declaredDependencyRoots now returns real-root → package name and packageNameOfResource consults it; package-build.test.ts adds a workspace-link fixture asserting evidence.assets[].packages attributes linked-dep.
  2. No real-build test that the pre-manifest pass engages provenPaths (the distinguishing test used synthetic evidence). Fixed: build.test.ts builds a real artifact, re-writes a compiled bundle with a syntax error and re-signs manifest + record, then asserts the record-proven artifact passes the lexed check and the same artifact with coverage.rewritable: true fails AB6005.
  3. Docs claimed four AB7014 evidence sources; pack-inventory.ts consumes three and the "process-dependency record" is empty. Fixed across AGENTS.md, docs/diagnostics.md, website en+zh validation/architecture/targets-artifacts.
  4. Changeset presented the internal ValidateArtifactOptions.bundleSyntaxCheck removal as a public breaking change. Fixed: clause removed.

Second pass (gpt-5.6-sol-medium, 2 findings), fixed in the follow-up commit:

  1. Nested linked dependency roots attributed to the parent (packageNameOfResource took the first containing root). Fixed: deepest containing root wins; package-build.test.ts adds linked-nested inside linked-dep's real root and asserts all three packages (fails without the fix).
  2. Two English passages still said "four" evidence sources (docs/diagnostics.md AB7014 row, validation.mdx). Fixed; zh already said three.

Owner re-review (19:56, at 6a1b3b8): P1 — a proven file skipped literal-import validation, so an rspackIgnore/webpackIgnore import escaped. Fixed in 2b3f21e (see the dispositions comment and the negative-test matrix on #619).

Third pass (gpt-5.6-sol-medium, 1 finding), fixed in 2014721:

  1. artifact-validator.test.ts still expected a proven bundle's bare specifier and dangling sibling to pass — the synthetic test encoded the flaw the owner found. Fixed: those are now expected as unaccounted AB6005; a recorded artifact-relative sibling and a built-in in a proven bundle are shown to pass, and the same sibling without its record entry is shown to fail. Full integration pool: 1127 passed.

Fourth pass (claude-fable-5-1-thinking-high, 3 findings), 8cd6d02:

  1. docs/diagnostics.md said the pre-manifest pass validates only filesystem entries and JSON. Fixed: it also reads the record against the planned file table (AB6039) and walks the JavaScript modules.
  2. Changeset and docs/entry-conventions.md overstated the function-form reserved-external path (guardReservedExternals intercepts it first; compile-time AB6005 is the backstop for what reaches the module graph). Fixed in both.
  3. A compiled entry's own createRequire(…)('pkg') / import.meta.resolve has no declaration path, so AB7014 fires for it. Dismissed: this is option A as the owner settled it (bundled ≠ used; the closed-world contract says a generated executable bundles everything but Node built-ins, so a resolution-only load of an installed package is outside the contract), the form is listed in coverage.unobserved, matrix row 9 records it, and the owner asked for no unused seam. A runtimeDependencies field on entry configs can be added when a fixture needs it.

Final pass (gpt-5.6-sol-medium): the three doc edits verified against the code; one low finding — two test comments in build.test.ts still described the removed residual-import scan — fixed in 3111475. No merge risks remaining.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@changeset-bot

changeset-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3111475

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-bundle Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 5, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle@634
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@634
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/rsc-markdown-stream@634
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@634

commit: 3111475

@ScriptedAlchemy ScriptedAlchemy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-audit follow-up: keep the AB7014 improvement; block premature AB6005 scanner removal

Reviewed 5870e90554425c14daf2c9d191fbb4bd5027c8c5 through GitHub source/diff only; no builds were executed for this review.

The option-A dependency policy is correct: bundled does not mean needed by the consumer. The remaining blocker is deleting validation coverage before its replacement exists.

P1 — unobserved dynamic loads still escape compilation evidence

src/build/compile-result.ts still explicitly says expression requests can be left verbatim without a module or external record. This PR removes validate-artifact-modules.ts, the package dist walk, and the residual-import reader without introducing an equivalent compiler rejection/coverage result for that case.

A fixture such as export const load = () => import(process.argv[2]) must not move from a rejected/unverified runtime edge to a successful self-containment verdict simply because the external list is empty. Generated-module happy-path tests do not establish that negative-case coverage.

Before deleting the walks: add the negative fixtures and establish the supported policy with compiler facilities or explicit declarations. Preserve the existing guard where coverage is still missing. This is not a request to revive #602 or grow another JavaScript parser.

P1 — artifact-only validation has no persisted replacement yet

The PR body defers the manifest half of #619 steps 4–5 until after #604, while this patch already removes the validate --artifact JavaScript walk. Main's compileRslibSurfaces() passes only CompileResult.assets to surface finishers; a new process reading a copied artifact does not possess the compilation's external/module report.

Keep artifact-only validation coverage until the required evidence is recorded and read, or explicitly split and document a narrower validator contract. Merely leaving AB6005 in the changeset does not make that check run for a separately loaded artifact. Evidence must be associated with the emitted file inventory/digests and its compiler/policy version; retain missing/outside/unlisted relative-target checks.

Concrete split / merge guidance

  1. The consumer-runtime AB7014 evidence work can proceed independently, preserving declaration, declared-payload, and install-script semantics with behavioral tests.
  2. Integrate #604 and the necessary persisted-evidence reader/writer contract.
  3. Delete each old validator only alongside the replacement for its demonstrated positive and negative cases. Update AGENTS/docs in that same change.

Keep the framework process-dependency record empty while there is no producer; do not add an unused seam or resurrect the deleted checkout launcher.

Required regression matrix: package external via ordinary import and node-commonjs; allowed builtin; valid/missing/outside/unlisted relative output; unobserved dynamic import; declared opaque payload; bundled-only package still AB7014; artifact-only validation after relocation without source. Scanner removal remains pending until this matrix has an explicit disposition.

@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Converting back to draft: the owner audit on #619 (17:11 UTC) reorders PR 3 — the persisted compile evidence record lands first (#638), and scanner deletion is gated on a coverage matrix. This PR will be reworked on top of #638: it keeps the AB7014 option-A re-derivation and deletes only what the matrix covers.

@ScriptedAlchemy
ScriptedAlchemy force-pushed the feat/619-evidence-ab7014 branch from 5870e90 to 6a1b3b8 Compare September 5, 2026 19:23
@ScriptedAlchemy ScriptedAlchemy changed the title feat(build): AB7014 from evidence — .d.ts references, install-script grammar, runtimeDependencies; delete every generated-JS scanner (#619 step 6) build: AB7014 from consumer-runtime evidence (option A); gate the generated-JS walk on the compile evidence record (#619 steps b–d) Sep 5, 2026
@ScriptedAlchemy
ScriptedAlchemy force-pushed the feat/619-evidence-ab7014 branch from 306edd9 to 2b3f21e Compare September 5, 2026 20:21
@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Dispositions for the owner review threads (17:25 review at 5870e90, the 19:55 blocker, and the 19:56 re-review at 6a1b3b8), against 2b3f21e:

P1 — unobserved dynamic loads escape compilation evidence. Resolved as the re-review records: the walk is not deleted. Every emitted module is still read; a module the record does not prove (no record, AB6039 findings, coverage.rewritable, install.mjs, copied scripts) keeps the full acorn parse and import resolution, and a proven module is lexed and fails on import(<expression>). Negative fixture export const load = (name) => import(name) is build.test.ts fails the build on an expression import the compiler left verbatim in a compiled script, plus the new package-output twin.

P1 — artifact-only validation has no persisted replacement. Resolved by #638 (agent-bundle.compile-evidence.json, AB6039), which this PR is rebased onto; validate --artifact reads the record from the artifact, not from a compilation. New: build.test.ts validates a relocated artifact from its record alone… renames the output root away, deletes the project source, and validates — then re-signs a bundle and shows the record-proven bundle is still judged.

P1 (19:56) — a proven file skips literal-import validation; import('left-pad' /* rspackIgnore: true */) escapes. Confirmed and fixed in 2b3f21e. Probe first: Rspack 2.2.2 leaves the ignored call verbatim with no module, no external, no warning, and no dependency object on the issuer (only a category: "unknown" dependency with no request), so the evidence plugin cannot record it and the first option is unavailable. Second option taken: accountedRequestsOf(record) maps each proven file to its recorded external requests, and the walk holds every lexed literal import of a proven file to that set — a Node built-in or a recorded external passes, anything else fails AB6005 … loads "<request>", which the compiler neither bundled nor recorded as an external; an import the build ignored is a run-time load outside the artifact. The digest proves the bytes are the compiler's, not that every import was resolved; the walk still resolves nothing the record already judged (no #602 parser). Exact fixture in build.test.ts and package-build.test.ts (left-pad, ignored missing, outside, and existing siblings). Full host/package matrix with test names: #619 comment.

Split guidance (1–3). Followed in order: #638 is (a); this PR keeps option-A AB7014 (bundled ≠ used, three evidence sources) and deletes only assertNoResidualReservedImports (replaced by compile-time AB6005 for a package external, guardReservedExternals, reservedAliasViolation) and the pack-dependencies.ts packed-JS scanning that option A makes policy; the emitted-module walk is gated, not removed. AGENTS.md and docs updated in the same change.

Process-dependency record. Kept empty; no seam added, no launcher revived. Docs now say three AB7014 sources everywhere (a stale "four" was the previous reviewer's finding).

Required regression matrix (package external via import and node-commonjs; allowed builtin; valid/missing/outside/unlisted relative output; unobserved dynamic import; declared opaque payload; bundled-only package still AB7014; artifact-only validation after relocation). Every row has a test in the #619 comment; the two node-commonjs rows are compiler-evidence.test.ts fails the node-commonjs shim form the same way in a host pack and package-build.test.ts …when a tools.rspack mutator sets externalsType to node-commonjs; bundled-only is prepack.test.ts reports a dependency only compiled dist bundles inlined as AB7014, naming the bundles.

@ScriptedAlchemy
ScriptedAlchemy marked this pull request as ready for review September 5, 2026 20:48
@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) September 5, 2026 20:48
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@ScriptedAlchemy
ScriptedAlchemy merged commit d8fa7c2 into main Sep 5, 2026
16 checks passed
ScriptedAlchemy added a commit that referenced this pull request Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant