From ca8753e620a58eaf330253baa4f792c8b94016a0 Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Tue, 19 May 2026 08:24:26 +0200 Subject: [PATCH] fix(release): isolate per-package tarball in shared distDir 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> --- CHANGELOG.md | 9 +++++- dist/index.js | 52 +++++++++++++++++++++++------------ dist/release.d.ts | 15 +++++++--- package-lock.json | 4 +-- package.json | 2 +- src/__tests__/release.test.ts | 24 ++++++++++++++++ src/release.ts | 47 ++++++++++++++++++++----------- 7 files changed, 111 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 208944b..ba78656 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ The floating `v1` tag tracks the latest `1.x` release. Consumers pinning ## [Unreleased] +## [1.9.1] - 2026-05-19 + +### Fixed + +- **Spurious "produced N tarballs; expected 1" warnings in monorepo releases** ([microsoft/apm#1348]). `packPackage` snapshotted the shared `distDir` only after `apm pack` returned and treated any tarball with `mtime >= packStart - 1s` as "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 and warned on every pack after the first. Fix: snapshot tarballs and their mtimes before the pack call and accept only files that are new in `after` or whose mtime advanced — the warning now fires only on a genuine producer-side anomaly. Empirically reproduced on [DevExpGbb/zava-agent-config@v6.1.1](https://github.com/DevExpGbb/zava-agent-config/releases/tag/v6.1.1) ([run 26079513903](https://github.com/DevExpGbb/zava-agent-config/actions/runs/26079513903)), which packs 7 plugins from one `dist/` and emitted 6 spurious warnings per release. + ## [1.9.0] - 2026-05-18 ### Added @@ -228,7 +234,8 @@ Initial public release. - **Marketplace name set to "Setup APM"** ([#5]). - **Microsoft OSS compliance baseline.** SECURITY.md ([#2]), CODEOWNERS, license, contributing guide, code of conduct, and CI pipeline. -[Unreleased]: https://github.com/microsoft/apm-action/compare/v1.9.0...HEAD +[Unreleased]: https://github.com/microsoft/apm-action/compare/v1.9.1...HEAD +[1.9.1]: https://github.com/microsoft/apm-action/compare/v1.9.0...v1.9.1 [1.9.0]: https://github.com/microsoft/apm-action/compare/v1.8.0...v1.9.0 [1.8.0]: https://github.com/microsoft/apm-action/compare/v1.7.3...v1.8.0 [1.7.3]: https://github.com/microsoft/apm-action/compare/v1.7.2...v1.7.3 diff --git a/dist/index.js b/dist/index.js index a00c964..d4ab7e7 100644 --- a/dist/index.js +++ b/dist/index.js @@ -41949,14 +41949,24 @@ async function runGate(workingDir) { * Pack a single package: `cd && apm pack --offline --archive -o `. * Returns the absolute path to the produced .tar.gz. * - * Selects the produced tarball by mtime (newest after pack) rather than - * diffing the directory before/after. This is robust to the case where - * `apm pack` overwrites an existing tarball of the same name -- the diff - * approach would see fresh=[] and incorrectly throw despite pack succeeding. + * Identifies the produced tarball by snapshotting `distDir` before and after + * the pack invocation. A tarball is "produced by this call" if it is new in + * `after`, or if it existed in `before` but its mtime advanced. This is + * correct under two conditions a naive mtime heuristic gets wrong: + * + * 1. Monorepo runs share `distDir`. Sequential per-package pack invocations + * complete in <1s each, so prior tarballs from the same run fall inside + * any reasonable "newer than packStart" grace window. The before/after + * diff isolates exactly the tarball this invocation touched. + * 2. Re-runs overwrite an existing tarball of the same name. A pure + * set-difference would miss this; mtime advance catches it. */ async function packPackage(dir, distDir) { external_node_fs_namespaceObject.mkdirSync(distDir, { recursive: true }); - const packStartMs = Date.now(); + const before = new Map(); + for (const p of listTarballs(distDir)) { + before.set(p, external_node_fs_namespaceObject.statSync(p).mtimeMs); + } const rc = await lib_exec/* exec */.m('apm', [ 'pack', '--offline', @@ -41972,19 +41982,25 @@ async function packPackage(dir, distDir) { + `Verify that the package has a 'dependencies:' block or primitives ` + `to bundle.`); } - // listTarballs sorts newest-first by mtime. Accept the newest tarball - // whose mtime is >= packStart (1s grace for fs mtime granularity). - const graceMs = packStartMs - 1000; - const fresh = after.filter(p => external_node_fs_namespaceObject.statSync(p).mtimeMs >= graceMs); - if (fresh.length === 0) { - throw new Error(`apm pack in ${dir} succeeded but no tarball in ${distDir} has an ` - + `mtime newer than the pack invocation. Filesystem clock skew?`); - } - if (fresh.length > 1) { - lib_core/* warning */.$e(`apm pack in ${dir} produced ${fresh.length} tarballs; expected 1. ` - + `Using the most recently modified: ${fresh[0]}`); - } - return fresh[0]; + const touched = after.filter(p => { + const prev = before.get(p); + if (prev === undefined) + return true; + return external_node_fs_namespaceObject.statSync(p).mtimeMs > prev; + }); + if (touched.length === 0) { + throw new Error(`apm pack in ${dir} succeeded but no tarball in ${distDir} was added ` + + `or modified by this invocation. Filesystem clock skew?`); + } + if (touched.length > 1) { + // One pack invocation is expected to produce or modify exactly one + // tarball. More than one is a real producer-side anomaly worth surfacing + // -- the before/after diff has already filtered out prior packages in + // the same monorepo run. + lib_core/* warning */.$e(`apm pack in ${dir} produced ${touched.length} tarballs; expected 1. ` + + `Using the most recently modified: ${touched.sort((a, b) => external_node_fs_namespaceObject.statSync(b).mtimeMs - external_node_fs_namespaceObject.statSync(a).mtimeMs)[0]}`); + } + return touched.sort((a, b) => external_node_fs_namespaceObject.statSync(b).mtimeMs - external_node_fs_namespaceObject.statSync(a).mtimeMs)[0]; } function listTarballs(dir) { if (!external_node_fs_namespaceObject.existsSync(dir)) diff --git a/dist/release.d.ts b/dist/release.d.ts index 5d61a03..c7d034b 100644 --- a/dist/release.d.ts +++ b/dist/release.d.ts @@ -100,10 +100,17 @@ export declare function runGate(workingDir: string): Promise<{ * Pack a single package: `cd && apm pack --offline --archive -o `. * Returns the absolute path to the produced .tar.gz. * - * Selects the produced tarball by mtime (newest after pack) rather than - * diffing the directory before/after. This is robust to the case where - * `apm pack` overwrites an existing tarball of the same name -- the diff - * approach would see fresh=[] and incorrectly throw despite pack succeeding. + * Identifies the produced tarball by snapshotting `distDir` before and after + * the pack invocation. A tarball is "produced by this call" if it is new in + * `after`, or if it existed in `before` but its mtime advanced. This is + * correct under two conditions a naive mtime heuristic gets wrong: + * + * 1. Monorepo runs share `distDir`. Sequential per-package pack invocations + * complete in <1s each, so prior tarballs from the same run fall inside + * any reasonable "newer than packStart" grace window. The before/after + * diff isolates exactly the tarball this invocation touched. + * 2. Re-runs overwrite an existing tarball of the same name. A pure + * set-difference would miss this; mtime advance catches it. */ export declare function packPackage(dir: string, distDir: string): Promise; /** diff --git a/package-lock.json b/package-lock.json index 1573a11..3e12f3c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "apm-action", - "version": "1.0.0", + "version": "1.9.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "apm-action", - "version": "1.0.0", + "version": "1.9.1", "license": "MIT", "dependencies": { "@actions/core": "^3.0.0", diff --git a/package.json b/package.json index 65e94a9..580bf7f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "apm-action", - "version": "1.0.0", + "version": "1.9.1", "description": "GitHub Action for APM - Agent Package Manager", "type": "module", "main": "dist/index.js", diff --git a/src/__tests__/release.test.ts b/src/__tests__/release.test.ts index 75e8d78..d36f375 100644 --- a/src/__tests__/release.test.ts +++ b/src/__tests__/release.test.ts @@ -320,6 +320,30 @@ describe('packPackage', () => { expect(result).toBe(tarballPath); expect(fs.readFileSync(tarballPath, 'utf8')).toBe('fresh'); }); + + it('returns only the tarball produced by this invocation in a shared distDir (monorepo regression)', async () => { + // Simulate a monorepo loop: distDir already contains prior packages' + // tarballs from THIS run (mtime just a few ms ago). A mtime-grace-window + // heuristic would classify them all as "fresh" and emit a spurious + // "produced N tarballs; expected 1" warning. The before/after diff + // must isolate only the file this invocation actually touched. + fs.mkdirSync(distDir, { recursive: true }); + const priorA = path.join(distDir, 'sibling-a-1.0.0.tar.gz'); + const priorB = path.join(distDir, 'sibling-b-1.0.0.tar.gz'); + fs.writeFileSync(priorA, 'a'); + fs.writeFileSync(priorB, 'b'); + + const newTarball = path.join(distDir, 'mypkg-1.0.0.tar.gz'); + mockExec.mockImplementationOnce(async () => { + fs.writeFileSync(newTarball, 'new'); + return 0; + }); + 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); + }); }); describe('runReleaseMode (integration, mocked exec)', () => { diff --git a/src/release.ts b/src/release.ts index 83fd20a..045b911 100644 --- a/src/release.ts +++ b/src/release.ts @@ -284,17 +284,27 @@ export async function runGate( * Pack a single package: `cd && apm pack --offline --archive -o `. * Returns the absolute path to the produced .tar.gz. * - * Selects the produced tarball by mtime (newest after pack) rather than - * diffing the directory before/after. This is robust to the case where - * `apm pack` overwrites an existing tarball of the same name -- the diff - * approach would see fresh=[] and incorrectly throw despite pack succeeding. + * Identifies the produced tarball by snapshotting `distDir` before and after + * the pack invocation. A tarball is "produced by this call" if it is new in + * `after`, or if it existed in `before` but its mtime advanced. This is + * correct under two conditions a naive mtime heuristic gets wrong: + * + * 1. Monorepo runs share `distDir`. Sequential per-package pack invocations + * complete in <1s each, so prior tarballs from the same run fall inside + * any reasonable "newer than packStart" grace window. The before/after + * diff isolates exactly the tarball this invocation touched. + * 2. Re-runs overwrite an existing tarball of the same name. A pure + * set-difference would miss this; mtime advance catches it. */ export async function packPackage( dir: string, distDir: string, ): Promise { fs.mkdirSync(distDir, { recursive: true }); - const packStartMs = Date.now(); + const before = new Map(); + for (const p of listTarballs(distDir)) { + before.set(p, fs.statSync(p).mtimeMs); + } const rc = await exec.exec('apm', [ 'pack', '--offline', @@ -312,23 +322,28 @@ export async function packPackage( + `to bundle.`, ); } - // listTarballs sorts newest-first by mtime. Accept the newest tarball - // whose mtime is >= packStart (1s grace for fs mtime granularity). - const graceMs = packStartMs - 1000; - const fresh = after.filter(p => fs.statSync(p).mtimeMs >= graceMs); - if (fresh.length === 0) { + const touched = after.filter(p => { + const prev = before.get(p); + if (prev === undefined) return true; + return fs.statSync(p).mtimeMs > prev; + }); + if (touched.length === 0) { throw new Error( - `apm pack in ${dir} succeeded but no tarball in ${distDir} has an ` - + `mtime newer than the pack invocation. Filesystem clock skew?`, + `apm pack in ${dir} succeeded but no tarball in ${distDir} was added ` + + `or modified by this invocation. Filesystem clock skew?`, ); } - if (fresh.length > 1) { + if (touched.length > 1) { + // One pack invocation is expected to produce or modify exactly one + // tarball. More than one is a real producer-side anomaly worth surfacing + // -- the before/after diff has already filtered out prior packages in + // the same monorepo run. 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]; } function listTarballs(dir: string): string[] {