fix(release): isolate per-package tarball in shared distDir (v1.9.1)#45
Merged
Conversation
The mtime grace-window heuristic in packPackage() classified prior packages' tarballs as 'fresh by this invocation' in monorepo runs, emitting one spurious 'produced N tarballs; expected 1' warning per package after the first. Snapshot the directory before the pack call and accept only files that are new in 'after' or whose mtime advanced. Empirically reproduced on DevExpGbb/zava-agent-config@v6.1.1 (run 26079513903), which packs 7 plugins into one dist/ directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes packPackage() tarball selection during monorepo releases where multiple sequential apm pack invocations share a single dist/ directory, preventing spurious “produced N tarballs; expected 1” warnings caused by an mtime grace-window heuristic.
Changes:
- Update
packPackage()to snapshot tarballs/mtimes before and afterapm pack, and select only tarballs created/modified by the current invocation. - Add a regression test covering shared
distDirmonorepo behavior. - Bump release metadata (version + changelog) and regenerate
dist/outputs.
Show a summary per file
| File | Description |
|---|---|
| src/release.ts | Switch tarball detection from time-window heuristic to before/after snapshot diff to isolate the tarball produced by the current pack call. |
| src/tests/release.test.ts | Add monorepo regression test for shared distDir tarball isolation. |
| package.json | Bump package version to 1.9.1. |
| package-lock.json | Update lockfile version fields to 1.9.1. |
| dist/release.d.ts | Regenerate typings reflecting updated packPackage() docs/behavior. |
| dist/index.js | Regenerate bundled JS reflecting updated packPackage() implementation. |
| CHANGELOG.md | Add 1.9.1 entry and update compare links for the release. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/7 changed files
- Comments generated: 2
Comment on lines
341
to
+346
| core.warning( | ||
| `apm pack in ${dir} produced ${fresh.length} tarballs; expected 1. ` | ||
| + `Using the most recently modified: ${fresh[0]}`, | ||
| `apm pack in ${dir} produced ${touched.length} tarballs; expected 1. ` | ||
| + `Using the most recently modified: ${touched.sort((a, b) => fs.statSync(b).mtimeMs - fs.statSync(a).mtimeMs)[0]}`, | ||
| ); | ||
| } | ||
| return fresh[0]; | ||
| return touched.sort((a, b) => fs.statSync(b).mtimeMs - fs.statSync(a).mtimeMs)[0]; |
Comment on lines
+341
to
+346
| const result = await packPackage(tmpDir, distDir); | ||
| expect(result).toBe(newTarball); | ||
| // Sibling files must still exist on disk (we did not delete them). | ||
| expect(fs.existsSync(priorA)).toBe(true); | ||
| expect(fs.existsSync(priorB)).toBe(true); | ||
| }); |
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
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.
TL;DR
In monorepo releases, every per-package
apm packafter the first emitted a spuriousproduced N tarballs; expected 1warning. The grace-window heuristic mistook prior packages' tarballs in the shareddist/directory as freshly produced. Snapshot before/after the pack call and accept only files this invocation actually touched.Problem (WHY)
packPackage()selected the produced tarball by mtime: any file indistDirwithmtime >= packStart - 1swas "fresh by this invocation." In a monorepo loop, sequential per-package packs complete in well under a second, so plugin N saw plugin 1..N-1's tarballs inside the grace window. Each pack after the first warnedproduced N tarballs; expected 1.Empirically reproduced on DevExpGbb/zava-agent-config@v6.1.1 (run 26079513903), which packs 7 plugins into one
dist/directory and emitted 6 spurious warnings per release.Approach (WHAT)
Snapshot the set of tarballs in
distDirand their mtimes before invokingapm pack. After the pack returns, "produced by this call" = new inafterOR existed inbeforeand mtime advanced. The warning now fires only on a genuine producer-side anomaly (one pack invocation touching multiple tarballs); the test suite's existing overwrite-regression case still passes.Implementation (HOW)
src/release.tspackPackage()rewritten to use the before/after diff (~25 LOC delta).listTarballsretained: it is still needed to enumerate the snapshot and to format error messages.returns only the tarball produced by this invocation in a shared distDir (monorepo regression): pre-seedsdistDirwith sibling-A and sibling-B tarballs, then asserts the returned path is only the freshly-produced one and the warning does not trigger.Validation evidence
npm run lint: cleannpm test: 182 passed (5 suites), including the new regressionnpm run build: dist/ regenerated via ncc 0.38.4Release
package.jsonbumped 1.9.0 → 1.9.1CHANGELOG.mdentry under[1.9.1]v1.9.1after merge;v1floating tag and GitHub release are handled automatically by.github/workflows/ci.yml's release job on tag push.How to test
The monorepo regression case (
returns only the tarball produced by this invocation in a shared distDir) is the new assertion.