fix(dist): stage package assets onto a cleared destination - #1879
Open
ScriptedAlchemy wants to merge 7 commits into
Open
ScriptedAlchemy wants to merge 7 commits into
ScriptedAlchemy wants to merge 7 commits into
Conversation
The product crate owns `crates/tracedecay/tests/fixtures`, the same path the root `tests/fixtures` asset is staged onto. The snapshot regression fixture never carried crate-local content there, so it never exercised that overlap and the gate's first real run failed instead. Seed the collision and assert the staged asset carries the root entries and nothing else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`cp -a` merges a directory into an existing directory of the same name, so staging the root `tests/fixtures` asset beside the product manifest left the package-local copy a superset of the root tree: the crate's own `tests/fixtures/impls_behavior` survived alongside it. The snapshot assertion then reported the staged asset differing from its snapshot and the gate exited before packaging. Clear each destination path before copying, in both the product and CLI asset loops, so a staged asset is exactly the validated root snapshot no matter what the package directory already holds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The step asked the extracted root package for its mcp_suite target, but cargo package publishes no integration tests and the suite requires the test-transport feature the production graph excludes, so the command could never run. The step now runs the suite from the staged source snapshot under the root-transport CI lens with the packaged CLI as the binary the suite spawns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The acceptance script runs the packaged grammar, query, root, LSP and MCP suites through nextest, and the workflow never installed it, so the first run to get past staging died on `no such command: nextest`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Asset staging rewrites package-local directories inside the staged tree (crates/tracedecay/tests/fixtures becomes the root fixtures), so the suite compiled from it could not find the package-local impls_behavior fixture it include_str!s. Keep a second copy of the snapshot before staging and run the suite from that. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The structural-rewrite proof in mcp_suite shells out to the host ast-grep CLI, which CI installs and this workflow did not, so the first run to reach the suite stopped at that test with 476 tests unrun. Run the suite without fail-fast so one run reports every gap. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The library consumer and the test-API probe are fresh manifests, so cargo resolves them from scratch, and offline resolution refuses a version that has since been yanked (bisync 0.3.0 under gix-protocol) even though the workspace lockfile pins it. Copy that lockfile in, as the extracted packages already get, so they resolve what the product resolves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
The first run of the distribution-acceptance workflow failed at "staged asset differs from its snapshot: tests/fixtures". The check stages the clean checkout into a tarball, copies eleven repository-root assets beside
crates/tracedecay/Cargo.toml, and compares each copy's digest with the staged original, so no live edit can enter the archive. The copy usedcp -a SRC DEST/, which merges a directory into an existing directory of the same name.crates/tracedecay/tests/fixturesgained crate-local content in528c3f5969(theimpls_behaviorfixture), so staging the roottests/fixturesonto it produced the union and the digests diverged. Nothing was leaking into the shipped archive; the whitelist names five fixture subpaths and that one is not among them.Both copy loops now clear the destination before copying, which covers all twelve staged assets rather than the one that collided. The existing two-second harness
scripts/test-check-distribution-snapshot.shdrives the real gate against a synthetic repository; seeding it with crate-local fixture content reproduces CI's exact failure line before the fix and passes after.Verified with the real production binary built in the lane at this head: staging, packaging every workspace crate, the required-asset and dashboard-bundle checks, and the feature-wiring comparison all pass with the packaged runtime battery skipped. The full battery, a second cold release compile plus five nextest suites and the MCPB, LSP-bridge and MCP-stdio checks, is running locally and the workflow should be dispatched again on master after this merges.
🤖 Generated with Claude Code