Skip to content

fix(build): isolate re-bundled runtime entries - #636

Merged
ScriptedAlchemy merged 7 commits into
mainfrom
fix/runtime-rebundle-boundary
Sep 5, 2026
Merged

fix(build): isolate re-bundled runtime entries#636
ScriptedAlchemy merged 7 commits into
mainfrom
fix/runtime-rebundle-boundary

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Closes the remaining runtime re-bundling boundary item from #566.

Summary

  • build every compiler-rebundled runtime entry in a dedicated runtime-node Rslib compilation, separated from the public graph that dynamically imports the MCP runtime
  • remove the growing private-sibling entry list
  • run the packed deleted-source stdio journey against both the normal release tarball and an isolated synthetic-private-sibling tarball; the fixture build has its own dist and cache and never mutates publishable output

Tests

  • pnpm build && pnpm typecheck && pnpm lint && pnpm test:unit && pnpm test:packed (4,123 unit passed; 35 packed passed, 1 skipped)
  • node --input-type=module -e "await import(\"./packages/agent-bundle/dist/mcp-server-runtime.js\"); ..." (normal post-packed dist does not execute the fixture)

Deslop

Deslop: GPT-5.6 Sol, 0 edits after reviewing the complete branch diff.

Self-review

Reviewer: Claude Fable 5.1 Thinking High (change-risk-reviewer).

First pass findings and dispositions:

  1. Release-blocking fixture bytes could survive test:packed:release into publish — fixed in 5bcd2b7: the packed runner now builds the fixture into an isolated dist and stages a separate tarball; the normal release dist and tarball are never mutated.
  2. splitChunks: false was overridden by Rslib and therefore inert — fixed in 5bcd2b7: removed it and documented the actual compilation boundary.
  3. Packed test title did not prove the sibling executed; reviewer suggested an emitted-source scan — fixed without a scanner in 5bcd2b7: the synthetic sibling sets a process marker and the packed MCP env-probe observes it over real stdio after source deletion.
  4. Both libs emitted declarations into the same directory — fixed in 5bcd2b7: only the public lib emits declarations.
  5. Separate compilations duplicate some private package-build code — accepted as the deliberate re-bundling boundary; @agent-bundle/runtime remains external and its shared class-identity graph/heavy subpaths are unchanged.
  6. app.js is compiler-rebundled but remained public — fixed in 5bcd2b7: moved it into runtime-node.

Subsequent passes fixed every concrete finding: 0cd885f isolated fixture dist and tarball ownership; 85b43f6 scoped fixture metadata to the stdio proof and made unsupported ad-hoc fixture requests fail clearly; 7f784aa retained the normal release-tarball stdio case alongside the synthetic case and isolated the fixture cache. Final pass: no blocking merge risks. The optional emitted-source scanner was declined because both regressions intentionally prove behavior through real packed processes.

@changeset-bot

changeset-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7f784aa

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 Patch

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@636
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@636
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/rsc-markdown-stream@636
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@636

commit: 7f784aa

@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.

P1: the synthetic packed-test build can become the release output

Static review at ce66e4386257fe80b48ea0e9113b65f5f857c620; no release or tests executed by this reviewer.

The structural direction is aligned with #566: a dedicated runtime compilation removes the growing private-sibling entry list. Keep that simplification. The fixture must not replace the canonical publishable build.

In scripts/run-packed-tests.mjs, every invocation now runs the workspace pnpm build with AGENT_BUNDLE_RUNTIME_REBUNDLE_FIXTURE=1, including --release. That rewrites packages/agent-bundle/dist with tests/fixtures/runtime-rebundle/mcp-server-runtime.ts as its runtime entry. The finally block deletes only the temporary tarball directory; it does not restore the production dist.

The root scripts compose as:

pnpm release
  -> pnpm check:release
     -> pack:dry-run          (normal build)
     -> lint:release
     -> test:packed:release   (overwrites workspace dist with fixture build)
  -> changeset publish       (can consume the fixture-bearing dist)

So the release gate both tests substituted package bytes rather than the ordinary candidate and leaves that substitution in the directory the publisher uses. This is a concrete code-path risk, not evidence that an affected package has already been published.

Required correction

Build the synthetic-private-sibling case into an isolated output/project and pack a separate test tarball. Keep the normal shared packed suite and release candidate built from production entries. Do not rely on the developer remembering to rebuild after tests, or on an environment variable merely being unset in the later publish process—the files have already been changed.

Add a regression that runs the packed gate and confirms the normal publishable runtime entry is still the production build, while the dedicated synthetic fixture demonstrably crossed package and artifact compilation. Preserve the deleted-source stdio/process test and state identity checks. Re-run declaration/public-export validation after the two-lib split.

No emitted-JavaScript dependency scanner is needed to fix this ownership problem. Isolate test artifacts from release artifacts.

@ScriptedAlchemy
ScriptedAlchemy marked this pull request as ready for review September 5, 2026 18:06
@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

Copy link
Copy Markdown
Owner Author

Addressed the owner P1 in 0cd885f, 85b43f6, and 7f784aa. The runner now creates the ordinary release build/tarball first, builds the synthetic sibling into an isolated --dist-path with an isolated Rspack cache, stages it into a separate package image/tarball, and gives only the synthetic stdio case that descriptor. The ordinary release tarball still runs the same deleted-source stdio process journey as a separate case. No publishable workspace output is overwritten, and the fixture is observed from inside the spawned MCP process rather than by scanning emitted source.

@ScriptedAlchemy
ScriptedAlchemy merged commit 00467a4 into main Sep 5, 2026
16 checks passed
ScriptedAlchemy added a commit that referenced this pull request Sep 5, 2026
@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Follow-up receipt: this review was against ce66e438; the merged head 00467a486fe91f411d37eed0b9d0d1bc40aee55f already moved the fixture build to --dist-path <packed-temp>/runtime-rebundle-dist, an isolated Rspack cache, and a separately staged tarball. The normal workspace dist and release tarball are built first and never replaced. I am opening a follow-up to make that ownership mechanically undeniable: hash the workspace dist before/after test:packed, and make dist freshness/release gates reject the fixture marker.

@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Follow-up complete in #645 (a5ceb957ee8e41a5d23a2f164a37917c93255b51). test:packed now hashes the canonical packages/agent-bundle/dist tree before and after the packed pool, both release gates run a post-packed freshness check, and freshness rejects any dist containing the synthetic runtime marker. The isolated fixture build/tarball from the final #636 merge remains unchanged.

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