Version Packages - #411
Conversation
fa93a0b to
568a252
Compare
568a252 to
5df29e1
Compare
commit: |
9a1be2e to
4497e8d
Compare
230cda8 to
9dee189
Compare
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Release-candidate review — 00d0a42d437320e53a852ee2a7dc70eaaa792c4b
Disposition: one concrete release-integration finding; candidate qualification is not established by the current PR evidence. This is a comment review, not approval, publication authorization, or a request to hand-edit the machine-owned release branch.
Scope
Checked the complete changed-path inventory (381 paths), all four package-manifest patches, the release/changelog version declarations and breaking-change notes, and the downstream scaffolder, package qualification and release workflow. The implementation-relevant diff is four version edits; the other paths are changeset consumption and generated changelogs. I did not re-audit every implementation described by those historical changesets.
Candidate versions:
| Package | Version |
|---|---|
agent-bundle |
0.2.0 |
create-agent-bundle |
0.1.0 |
@agent-bundle/runtime |
0.1.0 |
rsc-markdown-stream |
0.1.1 |
P2 — The advertised version-qualified/local-tarball scaffold path cannot pair this release's independently versioned packages
Evidence: runtimeSpecForFramework, scaffold's actual manifest-rewrite path, and the CLI's supported flags.
The helper mirrors a registry selector onto @agent-bundle/runtime, and derives a local runtime tarball by preserving the compiler tarball's version suffix. This candidate deliberately has different compiler/runtime versions:
framework input derived runtime
0.2.0 0.2.0
^0.2.0 ^0.2.0
file:/packs/agent-bundle-0.2.0.tgz file:/packs/agent-bundle-runtime-0.2.0.tgz
None selects this candidate's runtime 0.1.0. With the candidate's ordinary version-qualified tarball pair (agent-bundle-0.2.0.tgz and agent-bundle-runtime-0.1.0.tgz), validatedRuntimeSpecForFramework attempts to open the wrong sibling filename before scaffolding. For a registry selector it writes the compiler selector into the runtime dependency without establishing compatibility; ^0.2.0 excludes 0.1.0. The flags expose no independent runtime spec to correct that pairing.
This is not a claim that registry publication has occurred or that the helper was introduced by this PR. It is a pre-existing assumption that is incompatible with the exact package set being versioned and needs disposition before qualifying that supported release-install path.
Verification performed: executed a standalone JavaScript reproduction of this head's pure helper algorithm, with type annotations removed and a local UsageError. It confirmed the three outputs above and two unaffected controls: same-SHA pkg.pr.new URLs and deliberately unversioned sibling tarball filenames. This was not an installed CLI or project-suite run.
Why current unit tests miss it: framework.test.ts asserts same-version tarball pairs and mirrored registry selectors. It does not exercise the compiler/runtime versions actually selected here.
Required correction / acceptance:
- Support an explicitly paired runtime spec or derive compatibility from authoritative release/package metadata; do not assume version equality or introduce an unrelated hand-maintained second registry.
- Keep correct same-SHA preview behavior. Do not force otherwise independent packages into lockstep solely to preserve this helper.
- Add a regression using different compiler/runtime versions and their actual version-qualified packed filenames. Run a runtime-using template through the installed scaffolder and verify the emitted dependencies and successful installation/build. Keep the static-only template independent of the runtime.
- Preserve the explicit refusal to guess a default outside preview builds; that intentional guard is not the defect reported here.
- Make the correction through a normal source PR and let Changesets regenerate this machine-owned branch. Alternatively, explicitly narrow and test the supported release path rather than continuing to advertise arbitrary mirrored selectors as coherent pairs.
Qualification evidence — not a fabricated failing test
The four pull-request workflows for this exact SHA currently report action_required: CI, Changeset, Docs, and Package preview. That is neither passing qualification nor a reproduced test failure. The existing preview comment identifies 568a252, not this head. The inspected PR body/discussion contains no current-head local gate receipt.
The repository permits local gate evidence; this review does not impose waiting for hosted CI as a new rule. Record the allowed local gate results and run the existing release qualification against the exact intended versioned tree, or obtain the equivalent workflow results. Reuse scripts/run-packed-tests.mjs and its package/digest evidence rather than inventing another pack harness. Checking every bin with --help is useful, but does not exercise the pairing failure above.
The release workflow already distinguishes version maintenance, qualification, and opt-in publishing. Preserve that separation; no publishing variable, credential, workflow authorization, or repository setting was changed by this review.
Other observations and limits
No additional concrete defect found in the four version-number edits. The runtime's workspace:^ dependency on rsc-markdown-stream is intentional source metadata for pnpm packing, not evidence by itself that the published tarball leaks a workspace protocol; the existing packed gate checks that boundary.
Full repository builds, typechecks, packed installations, and native-host tests were not run in this review. A local checkout attempt failed because this execution environment could not resolve GitHub; source and workflow inspection used the connected GitHub tools. No source files, branches, generated changelogs, or package versions were changed.
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Release review refresh — 350846a909fd4c7bc19300f5e1e7a23e0f2987f6
The new candidate does not resolve the existing P2 compiler/runtime pairing finding. This is a delta review against the earlier detailed review, not a duplicate request for a release-reporting system.
Re-read the actual candidate manifests and packages/create-agent-bundle/src/framework.ts, and checked the same helper on current main 596d3371e9a9efbd2e84561826f5f390fefb2c3d. The candidate still publishes compiler 0.2.0 versus runtime 0.1.0, while runtimeSpecForFramework mirrors the compiler selector/versioned tarball suffix:
--framework-version 0.2.0
-> @agent-bundle/runtime: 0.2.0
file:/packs/agent-bundle-0.2.0.tgz
-> file:/packs/agent-bundle-runtime-0.2.0.tgz
The candidate's runtime file/version is agent-bundle-runtime-0.1.0.tgz / 0.1.0. The source assumption remains; regenerating version files alone cannot correct it. Evidence: candidate helper, compiler manifest, runtime manifest.
Keep the requested correction in a normal source PR: explicitly pair the two package specs or derive an authoritative compatible pair; preserve same-SHA previews and static-only template independence. Add the actual differently-versioned, version-qualified tarball case to installed-scaffolder qualification. Do not force lockstep versioning, rename files only inside the test to hide the issue, or hand-edit this machine-owned branch.
Current qualification and dependency status
- GitHub reports this release PR conflicting. Let Changesets regenerate against the intended source state; do not merge with administrative bypass.
- Exact-head CI run 34080929435, Docs, Package preview and Changeset returned
action_required. This is not a reproduced test failure, but it is not successful qualification either. No new current-head local qualification receipt was present in the inspected discussion. - The repository allows local gate evidence. Run/report the existing release qualification for the exact regenerated package set; this review does not impose waiting for hosted CI as a new rule.
- #729 proposes another closed manifest revision and #732 changes rendered compatibility output. If they are included in the release, qualify their resolved/merged state in the regenerated candidate, not this older release snapshot. Their inclusion is a release-scope decision, not a requirement to block all releases on an Amp account.
- #687's qualification/outcome reporting machinery already exists. Preserve the distinction between version maintenance, qualified artifacts and authorized publication; no duplicate workflow or issue is needed.
This review checked source/package metadata and current workflow status. It did not run a repository build, packed installation, or native-host test. No approval, branch edit, merge, publication, credential change, or workflow authorization is performed.
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Cross-repository review refresh — 350846a909fd4c7bc19300f5e1e7a23e0f2987f6
The release head is unchanged from the last detailed review and is currently conflicting. I re-read runtimeSpecForFramework on current main 4036e894816eb96f02d0e42749e5205fa8507eed: the same-version selector/tarball assumption still exists. The candidate compiler 0.2.0 / runtime 0.1.0 pairing therefore remains unresolved; this is not a newly discovered duplicate finding.
The consumer reviews make the needed distinction concrete: Cargo #160 pins compiler/runtime to the same preview build, which is supported; Cargo #157 independently updates a private manifest reader and demonstrates why coherent contract versions matter. Neither consumer's passing preview test qualifies this differently versioned registry/tarball release pair.
Fix the scaffolder through a normal source PR, test the actual compiler/runtime tarball filenames with a runtime-using template, and preserve same-SHA previews and static-template independence. Then let Changesets regenerate this machine-owned branch from the chosen source state. Do not force unrelated packages into lockstep, hand-edit generated versions, or rename the test files to disguise the selection error.
If #734/#735 are selected for the release, qualify the integrated example/argv/App state as well; #734 changes shared argv validation in addition to example files. Existing release outcome/packed infrastructure already exists—do not build a second gate or reopen #687 for the same reporting work.
This refresh reviewed current source and candidate identity; it did not execute a new build, package installation, or native-host run and does not assert current-candidate qualification. No approval, conflict resolution, merge, workflow authorization, or publication was performed.
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Prior blocker disposition — reviewed candidate bc26619719e38b7f5fc1e4ed9de2653af1529a44
My earlier same-version compiler/runtime pairing finding is addressed in this candidate. Do not carry the old hold forward unchanged. I checked the candidate's actual source, package metadata and regression tests, not just the PR's regeneration.
runtimePairingFromManifest now reads the release package's declared compiler/runtime pair; runtimeSpecForFramework uses that runtime for versioned local tarballs and rejects unsupported registry selections instead of copying the compiler version. The tests explicitly cover compiler 0.2.0 with runtime 0.1.0, including agent-bundle-0.2.0.tgz → agent-bundle-runtime-0.1.0.tgz. Same-SHA previews and unversioned local pairs retain their separate paths.
The source package deliberately carries optional workspace:* peers, which are resolved for the packed release; a source-manifest read is not itself proof of the installed package metadata. Retain the existing exact-versioned packed scaffolder gate as the qualification owner. I have not run it or published anything in this review, so this is disposition of the old code defect, not a blanket release approval.
The new first-run findings #745–#753 concern target selection, named CLI input expressiveness, generated contract readiness/semantics, Workbench selection/lifecycle, default tests and the documented authoring path. Keep them scoped to their actual acceptance rather than treating each as another version-pairing bug or forcing unrelated packages into lockstep. No new release system or manual changes to this machine-generated branch are requested.
This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or setup this action to publish automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.
Releases
agent-bundle@0.2.0
create-agent-bundle@0.1.0
@agent-bundle/runtime@0.1.0
rsc-markdown-stream@0.1.1