test(runtime): install co-published extensions from the local build - #4017
Conversation
The runtime critical-flow harness packs only the root `veryfront` package and
points the fixture at that tarball. The root pins its co-published extensions to
its own exact version, so npm resolves those pins from the registry -- and on a
release-cut branch that version is not published yet, because publishing is what
the release PR unlocks. Every version bump fails identically:
npm error code ETARGET
npm error notarget No matching version found for
@veryfront/ext-bundler-esbuild@0.1.1252-rc
It passes on main only because main's version is already on npm. #3977 added the
harness after the last release cut merged, so veryfront-code#4012 is the first
bump to hit it.
Pack the co-published extensions alongside the root and name them in the fixture
manifest, so nothing is fetched from a registry. `npm exec` gets the same set,
since it resolves the CLI's dependencies into its own prefix.
The set comes from the built root manifest, not a directory listing: the build
emits 29 extension packages and the root co-publishes 6. Installing the other 23
would change what the fixture exercises.
Nothing mutates the build output. The fixture manifest is a generated test input
the harness already writes, so there is no artifact to restore afterwards.
Verified against the real build by bumping npm/ to an unpublished 0.1.9999-rc:
root tarball only npm error code ETARGET
No matching version found for
@veryfront/ext-bundler-esbuild@0.1.9999-rc
root + co-published added 64 packages in 7s
veryfront 0.1.9999-rc, ext-bundler-esbuild 0.1.9999-rc,
ext-parser-babel 0.1.9999-rc, ext-yaml 0.1.9999-rc
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12fdc5ca0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #4005 needs exact-head CI while main still carries the runtime-critical release-version failures fixed by PR #4017. Merge the reviewed #4017 head so this branch can prove its own provider-header-budget changes against a green runtime baseline. Constraint: This is an explicit temporary dependency on #4017, reviewed commit 12fdc5c Rejected: Wait for main to receive #4017 before verifying #4005 | leaves this PR red on known unrelated failures and blocks exact-head confidence Confidence: high Scope-risk: moderate Directive: When #4017 lands on main, reconcile this dependency through the normal merge path rather than reimplementing the package-runtime fix here Tested: pending post-merge exact-head CI Not-tested: local package-runtime full matrix after merge
Fresh CI for this branch would otherwise inherit the current main release-version runtime-critical failures. PR #4017 contains the reviewed fix, so this branch carries it as an explicit temporary dependency instead of duplicating the implementation. Constraint: Temporary dependency on PR #4017 head 12fdc5c for full CI signal. Rejected: Reimplement the release-version fix in this branch | would duplicate reviewed work and widen the semantic-audit PR. Confidence: high Scope-risk: moderate Directive: Drop this merge once PR #4017 lands into the base branch. Tested: gh pr view 4017 reports all substantive checks green at head 12fdc5c. Tested: #4009 semantic and coverage shard checks passed before this dependency merge. Not-tested: full #4009 CI after dependency merge until pushed.
… into fix/pack-copublished-extensions
Addresses Codex P1 and P2 on #4017. P1 -- the Deno matrix still reached the registry. `scaffoldProject` forwarded only `packed.root` to `usePackedVeryfrontDenoTasks`, which extracts that tarball and runs `deno install` inside `.veryfront-packed-cli/package`. That manifest still pins the six co-published extensions to its own version, so the sibling tarballs already on disk were never declared to that install and it resolved them from the registry. `redirectCoPublishedPins` now rewrites those pins to `file:` before the install; it only touches pins the manifest already declares, so it cannot widen what gets pulled in. P2 -- templates add first-party extensions the root does not depend on: `minimal` installs @veryfront/ext-content-mdx, `docs-agent` installs @veryfront/ext-document-kreuzberg. Those pins carry the build's version and fail the same way. `localiseTemplateExtensions` packs them from the generated manifest, so only the selected template's extensions are packed and unrelated ones stay out. It runs before the runtime-specific wiring because both paths install from that manifest. Not verifiable by this PR's CI: its head is 0.1.1251, which is published, so the release-cut path cannot reproduce there. Both fixes are aimed at the unpublished-version case that CI does not currently exercise. The two TS2307 errors in cli/utils/terminal-select.ts are pre-existing -- 2 before and after this change on the same file set.
|
Warning Review limit reached
Next review available in: 42 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Pushed P1 — the Deno matrix did still reach the registry. P2 — template-owned extensions. Added One thing worth flagging about this PR's evidence. I initially read the green That is the structural gap your own follow-up note names: a fix for release cuts that CI only ever runs against published versions. Until something exercises an unpublished version, both my changes and the original rest on manual verification rather than CI.
|
|
@codex review |
…nt' into fix/pack-copublished-extensions
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
PR #4005 needs exact-head CI that includes the reviewed runtime packaging fix from PR #4017. The earlier dependency merge used the pre-review #4017 head, so this refresh explicitly brings in the head whose review findings were resolved before re-running #4005 checks. Constraint: Main still has release-version runtime failures without the PR #4017 packaging fix Constraint: Temporary dependency must track the exact reviewed #4017 head Rejected: Rerun #4005 CI on the pre-review dependency head | would preserve the known runtime-critical failures Confidence: high Scope-risk: moderate Directive: Drop this merge when #4017 lands into the base branch Tested: pending post-merge focused test, typecheck, docs API check Not-tested: full remote CI pending after push
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Heads up on the remaining Now that this branch carries the rc bump ( All six co-published extensions, so this is the same class as the P1 finding but on the bun path specifically. The likely mechanism: If that reading is right, the fix is to constrain transitive resolution rather than only declare top-level deps: Worth noting the silver lining: this failure is only visible because the branch now includes the version bump. That closes the structural gap where CI could only ever run against a published version — the bun path was broken before too, just unobservable. I have not pushed anything for this; the branch is yours and moving. Flagging the diagnosis so the round trip is shorter. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The coverage-status branch must rerun CI against an unpublished release candidate. Merge the reviewed runtime harness fix so Bun, Deno, and Node resolve co-published extensions from local tarballs instead of the registry. Constraint: Current main identifies an unpublished release candidate Rejected: Ignore the three runtime failures | required checks must exercise the real package flow Confidence: high Scope-risk: moderate Reversibility: clean Directive: Drop this temporary dependency merge after #4017 lands on main Tested: #4017 Bun critical flow passed locally; #4017 Node and Deno critical checks passed in CI Not-tested: Exact #4015 combined head pending CI
The extension-suite branch must rerun CI against an unpublished release candidate. Merge the reviewed runtime harness fix so Bun, Deno, and Node resolve co-published extensions from local tarballs instead of the registry. Constraint: Current main identifies an unpublished release candidate Rejected: Ignore the three runtime failures | required checks must exercise the real package flow Confidence: high Scope-risk: moderate Reversibility: clean Directive: Drop this temporary dependency merge after #4017 lands on main Tested: #4017 Bun critical flow passed locally; #4017 Node and Deno critical checks passed in CI Not-tested: Exact #4018 combined head pending CI
PR #4005 temporarily depends on PR #4017 so exact-head CI includes the release-version runtime packaging fix. The prior dependency head exposed a Bun transitive-resolution gap, so this merge advances only to the confirmed final #4017 head with the Bun override fix. Constraint: Main still has release-version runtime failures without PR #4017 Constraint: Dependency must match final #4017 head 70499cb Rejected: Keep the 0292d59 dependency head | its CI reproduced the Bun transitive-resolution failure Confidence: high Scope-risk: moderate Directive: Drop this temporary dependency merge after #4017 lands into the base branch Tested: pending post-merge #4005 focused verification Not-tested: full remote #4005 CI pending after push
…' into fix/pack-copublished-extensions # Conflicts: # scripts/test/runtime-e2e-helpers.ts # scripts/test/template-runtime-e2e.ts
Nonbundled SWC builds cannot resolve the private virtual reflection module, so metadata-enabled output now inlines reflection unless the delegate is doing a real bundle. The tsconfig inheritance guard also reports cycle and depth failures without raw config paths, keeping loader-facing errors free of local filesystem layout. Constraint: bundle:false leaves imports unresolved, including private virtual specifiers Constraint: User-facing errors must not expose local filesystem paths Rejected: Always inline reflection | bundled graphs can deduplicate the virtual reflection module Confidence: high Scope-risk: narrow Directive: Keep nonbundled transform output self-contained unless a resolver explicitly owns the virtual reflection module Tested: deno fmt --check extensions/ext-bundler-swc/src/swc-bundler.ts extensions/ext-bundler-swc/src/swc-bundler.test.ts src/extensions/bundler/typescript-config.ts src/extensions/bundler/typescript-config.test.ts Tested: deno test --no-check --allow-all extensions/ext-bundler-swc/src/swc-bundler.test.ts Tested: deno task test:file src/extensions/bundler/typescript-config.test.ts Not-tested: Runtime-critical-flow pending explicit #4017 harness dependency merge
PR #4000 needs exact-head CI on an unpublished RC package version. Pull in the reviewed #4017 harness fix instead of duplicating it here, so the runtime-critical-flow jobs install local first-party extension tarballs alongside the packed root package. Constraint: Runtime-critical CI installs an unpublished RC package version Rejected: Reimplement the harness fix in this branch | #4017 owns the shared packaging behavior Confidence: high Scope-risk: moderate Directive: Drop this temporary dependency merge once #4000 is rebased past the #4017 landing commit Tested: #4017 head 12fdc5c CI succeeded, including runtime critical flow for deno, node, and bun Tested: deno fmt --check extensions/ext-bundler-swc/src/swc-bundler.ts extensions/ext-bundler-swc/src/swc-bundler.test.ts src/extensions/bundler/typescript-config.ts src/extensions/bundler/typescript-config.test.ts Tested: deno test --no-check --allow-all extensions/ext-bundler-swc/src/swc-bundler.test.ts Tested: deno task test:file src/extensions/bundler/typescript-config.test.ts Not-tested: #4000 exact-head runtime-critical-flow pending after merge
PR #4000 needs exact-head CI on an unpublished RC package version. Merge the final #4017 harness head so Deno CLI installs and template-owned extension pins both resolve from the locally packed workspace instead of the npm registry. Constraint: Runtime-critical CI installs an unpublished RC package version Constraint: #4017 owns the shared runtime harness behavior Rejected: Commit a local Deno-only bridge here | final #4017 covers the broader harness surface Confidence: high Scope-risk: moderate Directive: Drop this temporary dependency merge once #4000 is rebased past the #4017 landing commit Tested: #4017 final head 8a22b2e merged cleanly Tested: Focused #4000 SWC, tsconfig, runtime contract tests passed before this merge Not-tested: #4000 exact-head runtime-critical-flow pending after final #4017 merge
PR #4000 needs exact-head CI on an unpublished RC package version. Merge the green #4017 harness head so Bun, Deno, and npm runtime-critical-flow installs resolve extension pins from the locally packed workspace. Constraint: Runtime-critical CI installs an unpublished RC package version Constraint: #4017 owns the shared runtime harness behavior Rejected: Commit branch-local helper overrides here | #4017 owns the cross-runtime package localization fix Confidence: high Scope-risk: moderate Directive: Drop this temporary dependency merge once #4000 is rebased past the #4017 landing commit Tested: #4017 head 70499cb CI passed format, lint, typecheck, and runtime critical flow for deno, node, and bun Tested: Focused #4000 SWC, tsconfig, runtime contract tests and typecheck passed before this merge Not-tested: #4000 exact-head CI pending after push
|
@codex review |
The finalized package harness introduced three non-ASCII dashes in explanatory comments. Rephrase them with periods and commas so the runtime fix follows the repository-wide public-copy constraint without changing behavior. Constraint: AGENTS.md forbids em and en dash characters Rejected: Leave comments unchanged because they are not runtime output | repository copy rules cover source comments too Confidence: high Scope-risk: narrow Reversibility: clean Tested: Focused runtime/template tests passed (3 tests, 27 steps); format, lint, and diff checks passed Not-tested: Exact pushed head full CI pending
|
@codex review Please review exact head |
Current main advanced through the semantic-audit PR while coverage CI was running. Merge the conflict-resolved and standards-clean #4017 head so this branch remains mergeable and exercises local extension tarballs across every runtime. Constraint: #4009 merged an earlier runtime harness revision into main Rejected: Resolve the duplicated harness independently here | #4017 owns the canonical conflict resolution Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep #4017 as the source of truth for runtime harness conflicts Tested: #4017 exact head has 0 Standards findings, 0 Spec findings, and focused tests pass Not-tested: Exact #4015 combined head full CI pending
PR #4005 temporarily depends on PR #4017 so exact-head CI includes the release-version runtime packaging fix. Main advanced while validating the Bun fix, so this merge advances the dependency to the final base-current #4017 head. Constraint: Main advanced via #4009 during validation Constraint: Dependency must match final #4017 head 08f2d04 Rejected: Keep d929e87 | it depends on the previous #4017 base and is now dirty Confidence: high Scope-risk: moderate Directive: Drop this temporary dependency merge after #4017 lands into the base branch Tested: pending post-merge #4005 focused verification Not-tested: full remote #4005 CI pending after push
PR #4005 temporarily depends on PR #4017 so exact-head CI includes the release-version runtime packaging fix. PR #4017 advanced from the base-current head to satisfy repository copy rules without changing the tested runtime behavior, so this merge keeps #4005 aligned to the exact current dependency head. Constraint: #4017 remote head advanced to 932ab9d after AGENTS.md copy-rule cleanup Rejected: Push the 08f2d04 dependency head | it is no longer the remote #4017 head Confidence: high Scope-risk: moderate Directive: Drop this temporary dependency merge after #4017 lands into the base branch Tested: pending post-merge #4005 focused verification Not-tested: full remote #4005 CI pending after push
|
@codex review |
Current main advanced through the semantic-audit PR while extension-suite CI was running. Merge the conflict-resolved and standards-clean #4017 head so this branch remains mergeable and exercises local extension tarballs across every runtime. Constraint: #4009 merged an earlier runtime harness revision into main Rejected: Resolve the duplicated harness independently here | #4017 owns the canonical conflict resolution Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep #4017 as the source of truth for runtime harness conflicts Tested: #4017 exact head has 0 Standards findings, 0 Spec findings, and focused tests pass Not-tested: Exact #4018 combined head full CI pending
GitHub reported #4000 as conflicting after the exact-head push. Merging current main brings in the latest test semantic-audit baseline and CI wiring while preserving #4000's SWC changes and the final #4017 runtime harness on the two conflicting files. Constraint: PR #4000 must remain mergeable against current main before exact-head review can be trusted Constraint: The runtime harness conflict overlapped with #4017-owned packaging behavior Rejected: Take origin/main for runtime-e2e helper files | that would discard the final #4017 local extension install fix required for runtime-critical-flow Confidence: high Scope-risk: moderate Directive: Preserve the final #4017 harness behavior until #4000 is rebased past the #4017 landing commit Tested: Conflict-marker scan over scripts/test/runtime-e2e-helpers.ts and scripts/test/template-runtime-e2e.ts Not-tested: Full post-merge verification pending
PR #4000 needs exact-head CI on current main while depending on the #4017 runtime harness. Merge the final base-current #4017 head so the shared runtime-critical-flow package localization is identical to the reviewed dependency branch while preserving #4000's SWC feature changes. Constraint: #4017 final dependency head is 08f2d04 Constraint: #4000 must stay mergeable with current main after #4009 advanced the base Rejected: Depend only on 70499cb | parent reported 08f2d04 as the final base-current dependency head Confidence: high Scope-risk: moderate Directive: Remove this temporary dependency merge after #4017 lands and #4000 is rebased Tested: Focused SWC, tsconfig, runtime contract tests passed before this merge Not-tested: Post-merge verification pending
PR #4000 needs exact-head CI on current main while temporarily depending on #4017. Merge the final #4017 head after its standards-required comment cleanup so this branch carries the reviewed dependency exactly. Constraint: #4017 final dependency head is 932ab9d Constraint: #4000 should not duplicate or diverge from the shared runtime harness fix Rejected: Push the previous 08f2d04 dependency merge | parent reported 932ab9d as the final reviewed head Confidence: high Scope-risk: narrow Directive: Remove this temporary dependency merge after #4017 lands and #4000 is rebased Tested: Prior focused SWC, tsconfig, runtime contract tests and typecheck passed on 99d17a6 Not-tested: Post-merge verification pending
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Unblocks every release cut. Found while cutting #4012.
The problem
The runtime critical-flow harness packs only the root
veryfrontpackage and points the fixture atthat tarball. The root pins its co-published extensions to its own exact version, so npm resolves
those pins from the registry — and on a release-cut branch that version is not published yet, because
publishing is precisely what the release PR unlocks.
Chicken-and-egg: the test demands a published version; publishing is downstream of the PR.
It passes on
mainonly becausemain's version is already on npm. #3977 added the harness at16:35 and the previous release cut merged at 07:54, before it — so #4012 is simply the first bump
since, and every future one fails the same way.
The fix
Pack the co-published extensions alongside the root and name them in the fixture manifest, so nothing
is fetched from a registry.
npm execgets the same set via repeated--package, since it resolvesthe CLI's dependencies into its own prefix.
One-line summary: stop asking a registry for packages we just built and are holding.
Two decisions worth reviewing
The set comes from the built root manifest, not a directory listing. The build emits 29
extension packages; the root co-publishes 6. My first attempt globbed
npm/extensions/*— thatwould have been 5× the packing and would have installed 23 extensions the root never depended on,
changing what the fixture exercises.
Nothing mutates the build output. An earlier draft rewrote
npm/package.jsontofile:pathsbefore packing; it worked, but it corrupts an artifact
npm-install-smoke.shalso reads and needs afinallyto undo. The fixture manifest is a generated test input the harness already writes, sothere is nothing to restore.
Verification
Against the real build, with
npm/bumped to an unpublished0.1.9999-rc(root and the 6 extensionmanifests, as a real cut does):
npm error code ETARGET—No matching version found for @veryfront/ext-bundler-esbuild@0.1.9999-rcadded 64 packages in 7sInstalled versions confirm they came from the local tarballs, not the registry:
deno check,fmt:check,lintclean. The twoTS2307errors incli/utils/terminal-select.tsthat
deno check --config=scripts/test.deno.jsonreports are pre-existing — they appear identicallyon an untouched tree with the same file set.
Follow-up, not in scope
file:tarballs bypass real registry resolution, so this cannot catch a bad version range or abroken
publishConfig. The higher-fidelity answer is a throwaway local registry (Verdaccio) that thethree runtimes resolve against. Worth filing if packaging keeps biting — veryfront-issue-inbox#732
and #756 are the same shape: the packaged artifact is not exercised the way users consume it.