From ce119a5da8776cdf2d6c232e06472d6f446bc5a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 16:09:10 +0200 Subject: [PATCH 1/7] build(package): stop shipping Swift test sources The npm files list ships apple/macos-helper whole, so its Tests/ entered the tarball even though the install never compiles them. Excluding the directory through files alone breaks the shipped build: SwiftPM refuses any command once a declared test target's sources are missing, and a manifest that probes FileManager for the directory depends on the install working directory and manifest sandbox. Move the test targets to apple/macos-helper-tests, a development-only package whose Sources symlink compiles the shipped helper sources with @testable reach. The shipped manifest keeps only the products the install builds, so a packed tree builds unchanged and the tests keep running through pnpm test:macos-helper in CI's macos-helper gate. A repo guard holds the two manifests in parity. --- apple/macos-helper-tests/Package.swift | 29 ++++++++ apple/macos-helper-tests/Sources | 1 + .../BackgroundInteractionTests.swift | 2 +- .../RectResponseTests.swift | 0 .../ScreenshotResponseTests.swift | 0 .../MouseClickScheduleTests.swift | 0 apple/macos-helper/Package.swift | 12 ++-- package.json | 2 +- scripts/check-affected/model.ts | 3 +- .../macos-helper-package-contents.test.ts | 72 +++++++++++++++++++ 10 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 apple/macos-helper-tests/Package.swift create mode 120000 apple/macos-helper-tests/Sources rename apple/{macos-helper => macos-helper-tests}/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift (97%) rename apple/{macos-helper => macos-helper-tests}/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift (100%) rename apple/{macos-helper => macos-helper-tests}/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift (100%) rename apple/{macos-helper => macos-helper-tests}/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift (100%) create mode 100644 test/repo-guards/macos-helper-package-contents.test.ts diff --git a/apple/macos-helper-tests/Package.swift b/apple/macos-helper-tests/Package.swift new file mode 100644 index 0000000000..8f069f04b6 --- /dev/null +++ b/apple/macos-helper-tests/Package.swift @@ -0,0 +1,29 @@ +// swift-tools-version: 5.9 +import PackageDescription + +// Development-only Swift package: never shipped, never built by an install. The npm payload is +// apple/macos-helper, whose manifest declares no test targets because their sources are not +// shipped. `Sources` is a symlink to ../macos-helper/Sources, so the helper's own sources compile +// here as testable modules and `swift test --package-path apple/macos-helper-tests` covers them. +// Keep the target list in step with the shipped manifest. +let package = Package( + name: "agent-device-macos-helper-tests", + platforms: [.macOS(.v13)], + targets: [ + .target( + name: "AgentDeviceMacOSInput" + ), + .executableTarget( + name: "AgentDeviceMacOSHelper", + dependencies: ["AgentDeviceMacOSInput"] + ), + .testTarget( + name: "AgentDeviceMacOSInputTests", + dependencies: ["AgentDeviceMacOSInput"] + ), + .testTarget( + name: "AgentDeviceMacOSHelperTests", + dependencies: ["AgentDeviceMacOSHelper"] + ), + ] +) diff --git a/apple/macos-helper-tests/Sources b/apple/macos-helper-tests/Sources new file mode 120000 index 0000000000..e94dfd312b --- /dev/null +++ b/apple/macos-helper-tests/Sources @@ -0,0 +1 @@ +../macos-helper/Sources \ No newline at end of file diff --git a/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift b/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift similarity index 97% rename from apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift rename to apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift index 7b61be5fc3..d294d729ab 100644 --- a/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift +++ b/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift @@ -28,7 +28,7 @@ private struct HelperOutcomesFixture: Decodable { let deliveryMechanisms: [String] } -/// `#filePath` is `/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/`. +/// `#filePath` is `/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/`. private let repoRoot = URL(fileURLWithPath: #filePath) .deletingLastPathComponent().deletingLastPathComponent().deletingLastPathComponent() .deletingLastPathComponent().deletingLastPathComponent() diff --git a/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift b/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift similarity index 100% rename from apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift rename to apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift diff --git a/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift b/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift similarity index 100% rename from apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift rename to apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift diff --git a/apple/macos-helper/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift b/apple/macos-helper-tests/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift similarity index 100% rename from apple/macos-helper/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift rename to apple/macos-helper-tests/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift diff --git a/apple/macos-helper/Package.swift b/apple/macos-helper/Package.swift index 844c6cb2bb..89a34d0f48 100644 --- a/apple/macos-helper/Package.swift +++ b/apple/macos-helper/Package.swift @@ -1,6 +1,10 @@ // swift-tools-version: 5.9 import PackageDescription +// The npm package ships this directory and builds it on the user's machine, so it declares only +// what the helper binary needs. SwiftPM refuses to build a package whose declared test target has +// no sources, and the tests are not shipped; apple/macos-helper-tests is the manifest that owns +// them and compiles these same sources with them. let package = Package( name: "agent-device-macos-helper", platforms: [.macOS(.v13)], @@ -18,13 +22,5 @@ let package = Package( name: "AgentDeviceMacOSHelper", dependencies: ["AgentDeviceMacOSInput"] ), - .testTarget( - name: "AgentDeviceMacOSInputTests", - dependencies: ["AgentDeviceMacOSInput"] - ), - .testTarget( - name: "AgentDeviceMacOSHelperTests", - dependencies: ["AgentDeviceMacOSHelper"] - ), ] ) diff --git a/package.json b/package.json index 237c95bd81..6ab566ca81 100644 --- a/package.json +++ b/package.json @@ -116,7 +116,7 @@ "package:android-ime-helper:npm": "rm -rf android/ime-helper/dist && AGENT_DEVICE_ANDROID_HELPER=ime sh ./scripts/package-android-helper.sh $(node -p \"require('./package.json').version\") android/ime-helper/dist", "build:macos-helper": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift build -c release --package-path apple/macos-helper", "build:macos-helper:clean": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift package --package-path apple/macos-helper clean && pnpm build:macos-helper", - "test:macos-helper": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift test --package-path apple/macos-helper", + "test:macos-helper": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift test --package-path apple/macos-helper-tests", "check:macos-helper": "pnpm build:macos-helper && pnpm test:macos-helper", "prepare:publish-assets": "node scripts/prepare-publish-assets.mjs", "build:package": "pnpm build && pnpm package:xcuitest && pnpm build:macos-helper:clean && pnpm prepare:publish-assets", diff --git a/scripts/check-affected/model.ts b/scripts/check-affected/model.ts index e1ec8d0d57..8797851692 100644 --- a/scripts/check-affected/model.ts +++ b/scripts/check-affected/model.ts @@ -462,7 +462,8 @@ const BUILD_OWNERSHIP: ReadonlyArray<{ check: 'macos-helper', rule: 'own:macos-helper', detail: 'macOS helper is a separate Swift package build', - owns: (file) => file.startsWith('apple/macos-helper/'), + owns: (file) => + file.startsWith('apple/macos-helper/') || file.startsWith('apple/macos-helper-tests/'), }, { check: 'mcp-metadata', diff --git a/test/repo-guards/macos-helper-package-contents.test.ts b/test/repo-guards/macos-helper-package-contents.test.ts new file mode 100644 index 0000000000..df3deb2a20 --- /dev/null +++ b/test/repo-guards/macos-helper-package-contents.test.ts @@ -0,0 +1,72 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from 'vitest'; + +// The npm package ships `apple/macos-helper` and builds it on the user's machine, so its manifest +// may declare only the targets whose sources ship. SwiftPM refuses any build of such a package — +// including the install-time `swift build` — once a declared test target's directory is missing, +// which is what keeps the helper's tests from leaving the tarball through `files` alone. The test +// targets belong to `apple/macos-helper-tests`, a development package that compiles the same +// sources through its `Sources` symlink. Nothing in the repo compiles the shipped manifest's +// target list — its first consumer is a user's machine — so these guards hold the split. +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const shippedManifestPath = path.join(repoRoot, 'apple/macos-helper/Package.swift'); +const testPackageRoot = path.join(repoRoot, 'apple/macos-helper-tests'); + +function declaredTargetNames( + manifest: string, + kind: 'target' | 'executableTarget' | 'testTarget', +): string[] { + return [ + ...manifest.matchAll(new RegExp(String.raw`\.${kind}\(\s*name:\s*"([^"]+)"`, 'g')), + ].flatMap((match) => (match[1] === undefined ? [] : [match[1]])); +} + +function shippedManifest(): string { + return fs.readFileSync(shippedManifestPath, 'utf8'); +} + +function testManifest(): string { + return fs.readFileSync(path.join(testPackageRoot, 'Package.swift'), 'utf8'); +} + +test('the shipped macOS helper manifest declares no test targets', () => { + const manifest = shippedManifest(); + assert.doesNotMatch(manifest, /\.testTarget\(/); + assert.deepEqual(declaredTargetNames(manifest, 'target'), ['AgentDeviceMacOSInput']); + assert.deepEqual(declaredTargetNames(manifest, 'executableTarget'), ['AgentDeviceMacOSHelper']); + assert.match(manifest, /\.executable\(\s*name:\s*"agent-device-macos-helper"/); +}); + +test('the macOS helper test package compiles the shipped sources and owns the test targets', () => { + const manifest = testManifest(); + assert.equal( + fs.readlinkSync(path.join(testPackageRoot, 'Sources')), + path.join('..', 'macos-helper', 'Sources'), + 'the test package must compile the shipped sources, not a copy', + ); + assert.ok( + fs.existsSync(path.join(testPackageRoot, 'Sources/AgentDeviceMacOSHelper/main.swift')), + 'the Sources symlink must resolve into the shipped helper sources', + ); + + // Every target the shipped manifest builds must exist here too, or the tests would cover a + // tree users never compile. + const shippedTargets = [ + ...declaredTargetNames(shippedManifest(), 'target'), + ...declaredTargetNames(shippedManifest(), 'executableTarget'), + ]; + const testPackageTargets = [ + ...declaredTargetNames(manifest, 'target'), + ...declaredTargetNames(manifest, 'executableTarget'), + ]; + for (const name of shippedTargets) { + assert.ok(testPackageTargets.includes(name), `the test package must build ${name}`); + } + assert.deepEqual( + declaredTargetNames(manifest, 'testTarget').sort(), + ['AgentDeviceMacOSHelperTests', 'AgentDeviceMacOSInputTests'].sort(), + ); +}); From 449b4d48e72851e1693cd810c0f140c3314784db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 16:09:10 +0200 Subject: [PATCH 2/7] chore(gates): keep macOS helper test sources out of npm packs The publish asset guard already rejects shipped benchmark scripts; add the sibling invariant for the helper's Tests/ directory so a future files-list edit cannot re-ship sources the install never compiles. --- scripts/__tests__/size-report-package.test.ts | 11 +++++++++++ scripts/size-report-package.mjs | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/scripts/__tests__/size-report-package.test.ts b/scripts/__tests__/size-report-package.test.ts index c3841b27b9..1a526a7ae8 100644 --- a/scripts/__tests__/size-report-package.test.ts +++ b/scripts/__tests__/size-report-package.test.ts @@ -41,6 +41,17 @@ test('publish package requires both Android helpers and excludes benchmark scrip ]), /benchmark or build scripts/, ); + assert.throws( + () => + assertPublishPackageContents([ + ...fixturePack.files, + { + path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/ScheduleTests.swift', + size: 1, + }, + ]), + /macOS helper test sources/, + ); }); test('Markdown emphasizes total install size and startup without duplicate breakdowns', () => { diff --git a/scripts/size-report-package.mjs b/scripts/size-report-package.mjs index 113b2651e0..70d27d781b 100644 --- a/scripts/size-report-package.mjs +++ b/scripts/size-report-package.mjs @@ -76,6 +76,16 @@ export function assertPublishPackageContents(entries, options = {}) { if (forbiddenPaths.length > 0) { throw new Error(`npm pack includes benchmark or build scripts: ${forbiddenPaths.join(', ')}`); } + // The macOS helper ships as buildable source and the helper's own manifest declares no test + // targets, so anything under its Tests/ directory could only ever be dead weight the install + // never compiles. Its tests live in apple/macos-helper-tests, which `files` does not list. + const shippedSwiftTests = paths.filter( + (entryPath) => + entryPath === 'apple/macos-helper/Tests' || entryPath.startsWith('apple/macos-helper/Tests/'), + ); + if (shippedSwiftTests.length > 0) { + throw new Error(`npm pack includes macOS helper test sources: ${shippedSwiftTests.join(', ')}`); + } } function parseNpmPackOutput(stdout) { From 0bde27b5cf3154664dbf712ad49c9652b66ee787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 17:12:27 +0200 Subject: [PATCH 3/7] fix(gates): skip non-blob changed paths before reading them as source The coverage-crap gate died on EISDIR before measuring anything: this PR commits apple/macos-helper-tests/Sources as a symlink (mode 120000) to a directory, its diff carries the target as one added line, and the gate read the touched list as file contents. readChangedLinesDiff's parser now records the new-side object mode from the diff headers as regularFile, and both changed-line gates skip what it rules out (a symlink, a gitlink) instead of reading their working-tree entries. --- scripts/coverage-changed/model.test.ts | 32 ++++++++++- scripts/coverage-changed/model.ts | 10 +++- scripts/coverage-changed/run.test.ts | 19 +++++++ scripts/coverage-crap/model.ts | 10 ++-- scripts/coverage-crap/run.test.ts | 19 +++++++ scripts/lib/repo-git.test.ts | 73 ++++++++++++++++++++++++++ scripts/lib/repo-git.ts | 57 +++++++++++++++++++- 7 files changed, 212 insertions(+), 8 deletions(-) diff --git a/scripts/coverage-changed/model.test.ts b/scripts/coverage-changed/model.test.ts index b58742dc08..b4c645a3db 100644 --- a/scripts/coverage-changed/model.test.ts +++ b/scripts/coverage-changed/model.test.ts @@ -86,7 +86,12 @@ const coverage = parseLcov( ); function diff(path: string, added: number[], deleted = false): ChangedFileDiff { - return { path, added, deleted }; + return { path, added, deleted, regularFile: true }; +} + +/** A changed path whose new side is not a blob (symlink or gitlink); `run.ts` never reads its bytes. */ +function nonBlobDiff(path: string, added: number[]): ChangedFileDiff { + return { path, added, deleted: false, regularFile: false }; } test('gate fails when changed-line coverage is below threshold and lists offenders', () => { @@ -158,6 +163,31 @@ test('waiver forces a pass but still reports the numbers', () => { assert.equal(result.pct, 66.67); }); +test('an includable path whose new side is a symlink or gitlink is skipped, not read', () => { + // A committed symlink like `tools/helper-tests/Sources -> ../helper/Sources` shows in the + // diff as added text (its target), so it reaches this loop by path shape. Its working-tree + // entry resolves to a directory, so a content read dies on EISDIR; reading the + // object mode is what keeps the file out of both the scored and the excluded tally. + const readPaths: string[] = []; + const result = computeChangedCoverage({ + diffs: [ + nonBlobDiff('src/helper-tests/index.ts', [1]), + nonBlobDiff('vendor/lib', [1]), + diff('src/covered.ts', [1, 2, 3]), + ], + coverage, + fileLines: (path) => { + readPaths.push(path); + return null; + }, + }); + assert.deepEqual(readPaths, ['src/covered.ts']); + // 2/3 = 66.67% < 70: the skip leaves the verdict exactly what the scored file alone earns. + assert.equal(result.totalLines, 3); + assert.equal(result.passed, false); + assert.deepEqual(result.excluded.files, []); +}); + test('includable source absent from lcov counts as excluded-path code lines', () => { const result = computeChangedCoverage({ diffs: [diff('src/sdk/thing.ts', [1, 2, 3])], diff --git a/scripts/coverage-changed/model.ts b/scripts/coverage-changed/model.ts index 5ca1110301..2e8b3d7862 100644 --- a/scripts/coverage-changed/model.ts +++ b/scripts/coverage-changed/model.ts @@ -220,7 +220,15 @@ export function computeChangedCoverage(input: ComputeInput): ChangedCoverageResu const excludedFiles: ExcludedFileReport[] = []; for (const diff of input.diffs) { - if (diff.deleted || diff.added.length === 0 || !isIncludableSource(diff.path)) continue; + // A non-blob side (symlink, gitlink) is not source: its working-tree entry can + // resolve to a directory, which no content read may attempt. + if ( + diff.deleted || + !diff.regularFile || + diff.added.length === 0 || + !isIncludableSource(diff.path) + ) + continue; const cov = input.coverage.get(diff.path); if (!cov) { diff --git a/scripts/coverage-changed/run.test.ts b/scripts/coverage-changed/run.test.ts index 15382e77f2..3341002e26 100644 --- a/scripts/coverage-changed/run.test.ts +++ b/scripts/coverage-changed/run.test.ts @@ -67,6 +67,25 @@ test('passes and prints n/a for a docs-only change without touching coverage', ( assert.match(out, /0\/0 \(n\/a\)/); }); +test('a committed symlink to a directory passes the gate without an EISDIR crash', () => { + // The object mode in the diff header is what keeps a committed symlink (target recorded + // as one added line, a directory in the working tree) out of the content read below. + fs.mkdirSync(path.join(repo, 'vendor'), { recursive: true }); + write('vendor/keep.txt', 'not source\n'); + // Shaped like includable source so the gate reaches its content read; what stands + // behind the name is a directory. + fs.symlinkSync('../vendor', path.join(repo, 'src/linked.ts')); + write('README.md', '# docs\nnew line\n'); + git('add', '-A'); + git('commit', '-q', '-m', 'symlink into a directory and docs'); + writeLcov('SF:src/base.ts\nDA:1,1\nend_of_record\n'); + + const { code, out } = capture(() => run(['--base', 'main'], repo)); + + assert.equal(code, 0); + assert.match(out, /Changed-line coverage gate: PASS/); +}); + test('reads a large stacked diff beyond the subprocess default buffer', () => { write('README.md', '# large stack\n' + 'documentation line\n'.repeat(80_000)); git('add', '-A'); diff --git a/scripts/coverage-crap/model.ts b/scripts/coverage-crap/model.ts index a7235ad0f9..e683dbc356 100644 --- a/scripts/coverage-crap/model.ts +++ b/scripts/coverage-crap/model.ts @@ -185,13 +185,15 @@ export type CrapScope = 'touched' | 'all'; export type TouchedFile = Readonly<{ path: string; addedLines: number }>; /** - * The files whose lines this change edits. A pure rename, delete, or mode change adds no lines, so - * it contributes none — which is also exactly the set of files fallow's `--diff-stdin` scoping can - * charge a finding to, since that filter indexes the same added lines. + * The files whose lines this change edits. A pure rename, delete, mode change, or non-blob side + * (a symlink or a gitlink) adds no scoreable source, so it contributes none — which is also exactly + * the set of files fallow's `--diff-stdin` scoping can charge a finding to, since that filter indexes + * the same added lines. Dropping non-blob sides here is also what keeps every `TouchedFile` a path + * the working tree can be asked to hold as a readable file. */ export function readTouchedFiles(diffs: readonly ChangedFileDiff[]): TouchedFile[] { return diffs - .filter((diff) => !diff.deleted && diff.added.length > 0) + .filter((diff) => !diff.deleted && diff.regularFile && diff.added.length > 0) .map((diff) => ({ path: diff.path, addedLines: diff.added.length })); } diff --git a/scripts/coverage-crap/run.test.ts b/scripts/coverage-crap/run.test.ts index 9ad7006e07..665e9290b3 100644 --- a/scripts/coverage-crap/run.test.ts +++ b/scripts/coverage-crap/run.test.ts @@ -492,6 +492,25 @@ test('a touched file the working tree no longer holds is not held against the ru assert.match(out, /no touched function\(s\) at or above 30/); }); +test('a committed symlink to a directory is not read as a touched file', () => { + // The diff of a committed symlink (`helper-tests/Sources -> ../helper`) carries its target + // path as one added line, so the file reaches the touched list by its lines alone. The working + // tree holds a directory behind it, so reading it as source dies on EISDIR before any verdict + // exists — the object mode in the diff header is what keeps it off the list. + fs.mkdirSync(path.join(repo, 'helper-tests'), { recursive: true }); + write('helper/Helper.swift', 'func helper() {}\n'); + fs.symlinkSync('../helper', path.join(repo, 'helper-tests/Sources')); + write('src/clean.ts', `${TRIVIAL}export const also_trivial = 2;\n`); + commit('add a symlink to a directory'); + writeCoverage({ 'src/debt.ts': {}, 'src/clean.ts': { hits: 4 } }); + writeStamp(); + + const { code, out } = check(['--base', HEADS[0]!]); + + assert.equal(code, 0); + assert.match(out, /no touched function\(s\) at or above 30/); +}); + test('coverage written without a stamp is not trustworthy', () => { write('src/clean.ts', `${TRIVIAL}export const also_trivial = 2;\n`); commit('clean edit'); diff --git a/scripts/lib/repo-git.test.ts b/scripts/lib/repo-git.test.ts index aa61f52437..5ba8bf5b49 100644 --- a/scripts/lib/repo-git.test.ts +++ b/scripts/lib/repo-git.test.ts @@ -41,3 +41,76 @@ test('parseUnifiedDiff marks deleted files and keeps rename destinations', () => const renamed = files.find((f) => f.path === 'src/new.ts'); assert.deepEqual(renamed?.added, [4]); }); + +// The object modes below are what `git diff` writes in its headers, verified against +// real diffs: a symlink's added line is its target path, a gitlink's is +// `Subproject commit `, and both resolve in the working tree to something a +// content read cannot take (EISDIR for a symlink to a directory). Content readers +// skip what `regularFile` rules out, so the parse must read the mode from the +// headers rather than trust the body. +test('parseUnifiedDiff reads the new-side object mode as regularFile', () => { + const diff = [ + 'diff --git a/tools/helper-tests/Sources b/tools/helper-tests/Sources', + 'new file mode 120000', + 'index 0000000..e94dfd3', + '--- /dev/null', + '+++ b/tools/helper-tests/Sources', + '@@ -0,0 +1 @@', + '+../helper/Sources', + String.raw`\ No newline at end of file`, + 'diff --git a/vendor/lib b/vendor/lib', + 'index 40287e1..6f36720 160000', + '--- a/vendor/lib', + '+++ b/vendor/lib', + '@@ -1 +1 @@', + '-Subproject commit 40287e12bdee20e6666847781ad1e618ef185015', + '+Subproject commit 6f36720aa717698e572f0dec47f1ca11adcc190c', + 'diff --git a/src/kept.ts b/src/kept.ts', + 'index 6178079..c3219eb 100644', + '--- a/src/kept.ts', + '+++ b/src/kept.ts', + '@@ -1,0 +2 @@', + '+const kept = 1;', + ].join('\n'); + const files = parseUnifiedDiff(diff); + const link = files.find((f) => f.path === 'tools/helper-tests/Sources'); + assert.deepEqual(link?.added, [1]); + assert.equal(link?.regularFile, false); + const gitlink = files.find((f) => f.path === 'vendor/lib'); + assert.deepEqual(gitlink?.added, [1]); + assert.equal(gitlink?.regularFile, false); + const kept = files.find((f) => f.path === 'src/kept.ts'); + assert.equal(kept?.regularFile, true); +}); + +test('parseUnifiedDiff reads mode-change and deletion headers as regular-file sides', () => { + const diff = [ + 'diff --git a/tools/build.sh b/tools/build.sh', + 'old mode 100644', + 'new mode 100755', + 'index 562890b..562890b', + 'diff --git a/src/gone.ts b/src/gone.ts', + 'deleted file mode 100644', + 'index 7898192..0000000', + '--- a/src/gone.ts', + '+++ /dev/null', + '@@ -1 +0,0 @@', + '-was here', + 'diff --git a/gone-link b/gone-link', + 'deleted file mode 120000', + 'index c59d9b6..0000000', + '--- a/gone-link', + '+++ /dev/null', + '@@ -1 +0,0 @@', + '-target', + ].join('\n'); + const files = parseUnifiedDiff(diff); + // An executable-bit change stays a regular file; a deletion has no new-side + // content to read whatever the removed side's mode was. + const modes = files.find((f) => f.path === 'tools/build.sh'); + assert.equal(modes?.regularFile, true); + assert.equal(files.find((f) => f.path === 'src/gone.ts')?.deleted, true); + const goneLink = files.find((f) => f.path === 'gone-link'); + assert.equal(goneLink?.deleted, true); + assert.equal(goneLink?.regularFile, false); +}); diff --git a/scripts/lib/repo-git.ts b/scripts/lib/repo-git.ts index d04a90b670..836e3c49e5 100644 --- a/scripts/lib/repo-git.ts +++ b/scripts/lib/repo-git.ts @@ -46,9 +46,30 @@ export type ChangedFileDiff = { readonly added: readonly number[]; // `+++ /dev/null`: the file was deleted, so it contributes no changed lines. readonly deleted: boolean; + // Whether the new side is a regular git blob (mode 100644/100755), the only kind + // whose working-tree bytes are the file's content. A symlink (120000) carries its + // target path as content and its working-tree entry can resolve to a directory, + // which reading as a file dies on (EISDIR); a gitlink (160000) names a nested + // repository. Content readers must skip what this rules out. + readonly regularFile: boolean; }; -type DiffSection = { path: string; added: Set; deleted: boolean; cursor: number }; +// Git object modes the new side can carry in a diff header for something that is +// not a regular blob. `readChangedLinesDiff` reports their bodies as text lines +// (a link target, a `Subproject commit`), which no content reader may treat as +// source. Parsed rather than probed on disk: the verdict describes the commit the +// diff was cut against, not whatever the working tree holds at check time. +// `000000` is `+++ /dev/null`, the side of a deletion. +const NON_BLOB_MODES = new Set(['000000', '120000', '160000']); + +type DiffSection = { + path: string; + added: Set; + deleted: boolean; + cursor: number; + /** The new side's object mode, as the headers state it; null when they stay silent. */ + newMode: string | null; +}; function newStartOfHunk(line: string): number | null { const match = /@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/.exec(line); @@ -60,6 +81,23 @@ function pathFromDiffHeader(line: string): string { return header ? normalizePath(header[1]!) : ''; } +// Reads the new side's object mode out of the metadata lines that carry it, last +// writer wins in header order (`old mode`, `new mode`, then the `index` trailer). +// A hand-built diff may carry neither, which leaves the mode unknown and the side +// read as regular. +function foldModeLine(section: DiffSection, line: string): boolean { + const declared = /^(?:new file mode|old mode|new mode|deleted file mode) (\d{6})$/.exec(line); + if (declared) { + // `deleted file mode` describes the removed side; the new side is /dev/null. + section.newMode = line.startsWith('deleted') ? '000000' : declared[1]!; + return true; + } + if (!line.startsWith('index ')) return false; + const trailing = / (\d{6})$/.exec(line); + if (trailing) section.newMode = trailing[1]!; + return true; +} + // Fold one line of a file's diff body into its section. Renames+edits report // their added lines under the destination (`+++ b/...`); pure renames and // deletions add nothing. `--unified=0` means no context lines in practice. @@ -76,11 +114,19 @@ function foldDiffBodyLine(section: DiffSection, line: string): void { section.cursor += 1; } else if (line.startsWith('-') || line.startsWith('\\')) { // Removed line or "\ No newline": occupies no new-side number. + } else if (foldModeLine(section, line)) { + // Object-mode metadata; recorded above, carries no new-side number. } else { section.cursor += 1; } } +// The object mode of a section's new side is known only once its headers have +// been read; until then it is null, which readers treat as a regular file. +function isRegularFileSide(newMode: string | null): boolean { + return newMode === null || !NON_BLOB_MODES.has(newMode); +} + /** * `readChangedLinesDiff`'s report as per-file added-line sets — the one reading both changed-line * gates score from. Keep it beside its producer: a hand-built diff must parse the same way the @@ -95,13 +141,20 @@ export function parseUnifiedDiff(diff: string): ChangedFileDiff[] { path: section.path, added: [...section.added].sort((a, b) => a - b), deleted: section.deleted, + regularFile: isRegularFileSide(section.newMode), }); } }; for (const line of diff.split('\n')) { if (line.startsWith('diff --git ')) { flush(); - section = { path: pathFromDiffHeader(line), added: new Set(), deleted: false, cursor: 0 }; + section = { + path: pathFromDiffHeader(line), + added: new Set(), + deleted: false, + cursor: 0, + newMode: null, + }; } else if (section) { foldDiffBodyLine(section, line); } From cf63ab3df8787b25b9f75dceb6b896d6dd6556f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 18:06:44 +0200 Subject: [PATCH 4/7] fix(package): keep the macOS helper tests in the shipped package without a symlink The dev-only package apple/macos-helper-tests compiled the helper sources through a committed symlink to ../macos-helper/Sources. A symlink to a directory breaks every tool that copies the tree: Stryker's sandbox dies on copyfile (EISDIR), which failed both mutation jobs on this branch (run 39045124618), and it breaks Windows checkouts. SwiftPM rejects sources outside a package root, so no split-package layout avoids the link. Put the tests back under apple/macos-helper/Tests and keep them out of the pack with files exclusions; ship one PackStub.swift per test target, because SwiftPM refuses to load a package whose declared test target has no sources, and the install-time swift build runs on exactly that packed tree. Proof on the packed tree: swift build -c release succeeds; pnpm test:macos-helper runs all 8 XCTest cases from the checkout; the Stryker sandbox copy and full lane pass locally (snapshot-occlusion, envelope pass). --- apple/macos-helper-tests/Package.swift | 29 ------- apple/macos-helper-tests/Sources | 1 - apple/macos-helper/Package.swift | 16 +++- .../BackgroundInteractionTests.swift | 2 +- .../PackStub.swift | 7 ++ .../RectResponseTests.swift | 0 .../ScreenshotResponseTests.swift | 0 .../MouseClickScheduleTests.swift | 0 .../AgentDeviceMacOSInputTests/PackStub.swift | 7 ++ package.json | 5 +- scripts/__tests__/size-report-package.test.ts | 13 ++++ scripts/check-affected/model.ts | 3 +- scripts/size-report-package.mjs | 20 ++++- .../macos-helper-package-contents.test.ts | 77 +++++++------------ 14 files changed, 90 insertions(+), 90 deletions(-) delete mode 100644 apple/macos-helper-tests/Package.swift delete mode 120000 apple/macos-helper-tests/Sources rename apple/{macos-helper-tests => macos-helper}/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift (97%) create mode 100644 apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift rename apple/{macos-helper-tests => macos-helper}/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift (100%) rename apple/{macos-helper-tests => macos-helper}/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift (100%) rename apple/{macos-helper-tests => macos-helper}/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift (100%) create mode 100644 apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift diff --git a/apple/macos-helper-tests/Package.swift b/apple/macos-helper-tests/Package.swift deleted file mode 100644 index 8f069f04b6..0000000000 --- a/apple/macos-helper-tests/Package.swift +++ /dev/null @@ -1,29 +0,0 @@ -// swift-tools-version: 5.9 -import PackageDescription - -// Development-only Swift package: never shipped, never built by an install. The npm payload is -// apple/macos-helper, whose manifest declares no test targets because their sources are not -// shipped. `Sources` is a symlink to ../macos-helper/Sources, so the helper's own sources compile -// here as testable modules and `swift test --package-path apple/macos-helper-tests` covers them. -// Keep the target list in step with the shipped manifest. -let package = Package( - name: "agent-device-macos-helper-tests", - platforms: [.macOS(.v13)], - targets: [ - .target( - name: "AgentDeviceMacOSInput" - ), - .executableTarget( - name: "AgentDeviceMacOSHelper", - dependencies: ["AgentDeviceMacOSInput"] - ), - .testTarget( - name: "AgentDeviceMacOSInputTests", - dependencies: ["AgentDeviceMacOSInput"] - ), - .testTarget( - name: "AgentDeviceMacOSHelperTests", - dependencies: ["AgentDeviceMacOSHelper"] - ), - ] -) diff --git a/apple/macos-helper-tests/Sources b/apple/macos-helper-tests/Sources deleted file mode 120000 index e94dfd312b..0000000000 --- a/apple/macos-helper-tests/Sources +++ /dev/null @@ -1 +0,0 @@ -../macos-helper/Sources \ No newline at end of file diff --git a/apple/macos-helper/Package.swift b/apple/macos-helper/Package.swift index 89a34d0f48..b617fd28df 100644 --- a/apple/macos-helper/Package.swift +++ b/apple/macos-helper/Package.swift @@ -1,10 +1,10 @@ // swift-tools-version: 5.9 import PackageDescription -// The npm package ships this directory and builds it on the user's machine, so it declares only -// what the helper binary needs. SwiftPM refuses to build a package whose declared test target has -// no sources, and the tests are not shipped; apple/macos-helper-tests is the manifest that owns -// them and compiles these same sources with them. +// The npm package ships this directory and builds it on the user's machine. The tests under +// Tests/ stay out of the tarball through `files`, except one PackStub.swift per test target: +// SwiftPM refuses any load of a package whose declared test target has no sources, so the +// shipped tree needs a source per test target for the install-time `swift build` to succeed. let package = Package( name: "agent-device-macos-helper", platforms: [.macOS(.v13)], @@ -22,5 +22,13 @@ let package = Package( name: "AgentDeviceMacOSHelper", dependencies: ["AgentDeviceMacOSInput"] ), + .testTarget( + name: "AgentDeviceMacOSInputTests", + dependencies: ["AgentDeviceMacOSInput"] + ), + .testTarget( + name: "AgentDeviceMacOSHelperTests", + dependencies: ["AgentDeviceMacOSHelper"] + ), ] ) diff --git a/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift b/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift similarity index 97% rename from apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift rename to apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift index d294d729ab..7b61be5fc3 100644 --- a/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift +++ b/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/BackgroundInteractionTests.swift @@ -28,7 +28,7 @@ private struct HelperOutcomesFixture: Decodable { let deliveryMechanisms: [String] } -/// `#filePath` is `/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/`. +/// `#filePath` is `/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/`. private let repoRoot = URL(fileURLWithPath: #filePath) .deletingLastPathComponent().deletingLastPathComponent().deletingLastPathComponent() .deletingLastPathComponent().deletingLastPathComponent() diff --git a/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift b/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift new file mode 100644 index 0000000000..5e76819819 --- /dev/null +++ b/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift @@ -0,0 +1,7 @@ +// The npm package ships this one file and nothing else under the test targets (see `files` in +// package.json). SwiftPM refuses to load a package whose declared test target has no sources, so +// the install-time `swift build` of the packed tree needs a source here; the real tests for this +// target live beside it and are excluded from the pack. +import XCTest + +final class AgentDeviceMacOSHelperPackStubTests: XCTestCase {} diff --git a/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift b/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift similarity index 100% rename from apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift rename to apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/RectResponseTests.swift diff --git a/apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift b/apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift similarity index 100% rename from apple/macos-helper-tests/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift rename to apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/ScreenshotResponseTests.swift diff --git a/apple/macos-helper-tests/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift b/apple/macos-helper/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift similarity index 100% rename from apple/macos-helper-tests/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift rename to apple/macos-helper/Tests/AgentDeviceMacOSInputTests/MouseClickScheduleTests.swift diff --git a/apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift b/apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift new file mode 100644 index 0000000000..ad6997dffd --- /dev/null +++ b/apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift @@ -0,0 +1,7 @@ +// The npm package ships this one file and nothing else under the test targets (see `files` in +// package.json). SwiftPM refuses to load a package whose declared test target has no sources, so +// the install-time `swift build` of the packed tree needs a source here; the real tests for this +// target live beside it and are excluded from the pack. +import XCTest + +final class AgentDeviceMacOSInputPackStubTests: XCTestCase {} diff --git a/package.json b/package.json index 6ab566ca81..2f5d2c7890 100644 --- a/package.json +++ b/package.json @@ -116,7 +116,7 @@ "package:android-ime-helper:npm": "rm -rf android/ime-helper/dist && AGENT_DEVICE_ANDROID_HELPER=ime sh ./scripts/package-android-helper.sh $(node -p \"require('./package.json').version\") android/ime-helper/dist", "build:macos-helper": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift build -c release --package-path apple/macos-helper", "build:macos-helper:clean": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift package --package-path apple/macos-helper clean && pnpm build:macos-helper", - "test:macos-helper": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift test --package-path apple/macos-helper-tests", + "test:macos-helper": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift test --package-path apple/macos-helper", "check:macos-helper": "pnpm build:macos-helper && pnpm test:macos-helper", "prepare:publish-assets": "node scripts/prepare-publish-assets.mjs", "build:package": "pnpm build && pnpm package:xcuitest && pnpm build:macos-helper:clean && pnpm prepare:publish-assets", @@ -222,6 +222,9 @@ "dist", "apple/macos-helper", "!apple/macos-helper/**/.build", + "!apple/macos-helper/Tests/**", + "apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift", + "apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift", "apple/snapshot-bridge", "apple/fold-helper", "android/snapshot-helper/dist", diff --git a/scripts/__tests__/size-report-package.test.ts b/scripts/__tests__/size-report-package.test.ts index 1a526a7ae8..5c70eab37d 100644 --- a/scripts/__tests__/size-report-package.test.ts +++ b/scripts/__tests__/size-report-package.test.ts @@ -41,6 +41,19 @@ test('publish package requires both Android helpers and excludes benchmark scrip ]), /benchmark or build scripts/, ); + assert.doesNotThrow(() => + assertPublishPackageContents([ + ...fixturePack.files, + { + path: 'apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift', + size: 1, + }, + { + path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift', + size: 1, + }, + ]), + ); assert.throws( () => assertPublishPackageContents([ diff --git a/scripts/check-affected/model.ts b/scripts/check-affected/model.ts index 8797851692..e1ec8d0d57 100644 --- a/scripts/check-affected/model.ts +++ b/scripts/check-affected/model.ts @@ -462,8 +462,7 @@ const BUILD_OWNERSHIP: ReadonlyArray<{ check: 'macos-helper', rule: 'own:macos-helper', detail: 'macOS helper is a separate Swift package build', - owns: (file) => - file.startsWith('apple/macos-helper/') || file.startsWith('apple/macos-helper-tests/'), + owns: (file) => file.startsWith('apple/macos-helper/'), }, { check: 'mcp-metadata', diff --git a/scripts/size-report-package.mjs b/scripts/size-report-package.mjs index 70d27d781b..a09e1a529a 100644 --- a/scripts/size-report-package.mjs +++ b/scripts/size-report-package.mjs @@ -76,16 +76,28 @@ export function assertPublishPackageContents(entries, options = {}) { if (forbiddenPaths.length > 0) { throw new Error(`npm pack includes benchmark or build scripts: ${forbiddenPaths.join(', ')}`); } - // The macOS helper ships as buildable source and the helper's own manifest declares no test - // targets, so anything under its Tests/ directory could only ever be dead weight the install - // never compiles. Its tests live in apple/macos-helper-tests, which `files` does not list. + // The macOS helper ships as buildable source. Its manifest declares the test targets, so the + // packed tree must hold at least one source per target or the install-time `swift build` dies; + // `files` ships exactly one PackStub.swift per target and excludes every other test source. + // Anything else under Tests/ in the pack means the exclusion list leaked real tests. const shippedSwiftTests = paths.filter( (entryPath) => - entryPath === 'apple/macos-helper/Tests' || entryPath.startsWith('apple/macos-helper/Tests/'), + (entryPath === 'apple/macos-helper/Tests' || + entryPath.startsWith('apple/macos-helper/Tests/')) && + !entryPath.endsWith('/PackStub.swift'), ); if (shippedSwiftTests.length > 0) { throw new Error(`npm pack includes macOS helper test sources: ${shippedSwiftTests.join(', ')}`); } + const packStubs = paths.filter( + (entryPath) => + entryPath.startsWith('apple/macos-helper/Tests/') && entryPath.endsWith('/PackStub.swift'), + ); + if (paths.includes('apple/macos-helper/Package.swift') && packStubs.length < 2) { + throw new Error( + `npm pack is missing macOS helper test-target pack stubs: expected one PackStub.swift per test target, found ${packStubs.length}`, + ); + } } function parseNpmPackOutput(stdout) { diff --git a/test/repo-guards/macos-helper-package-contents.test.ts b/test/repo-guards/macos-helper-package-contents.test.ts index df3deb2a20..7c73313602 100644 --- a/test/repo-guards/macos-helper-package-contents.test.ts +++ b/test/repo-guards/macos-helper-package-contents.test.ts @@ -4,16 +4,17 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { test } from 'vitest'; -// The npm package ships `apple/macos-helper` and builds it on the user's machine, so its manifest -// may declare only the targets whose sources ship. SwiftPM refuses any build of such a package — -// including the install-time `swift build` — once a declared test target's directory is missing, -// which is what keeps the helper's tests from leaving the tarball through `files` alone. The test -// targets belong to `apple/macos-helper-tests`, a development package that compiles the same -// sources through its `Sources` symlink. Nothing in the repo compiles the shipped manifest's -// target list — its first consumer is a user's machine — so these guards hold the split. +// The npm package ships `apple/macos-helper` and builds it on the user's machine. Its manifest +// declares the test targets, and `files` ships only each target's PackStub.swift: SwiftPM refuses +// any load of a package whose declared test target has no sources, so the stubs are what keep the +// install-time `swift build` of the packed tree alive while the real tests stay out of the +// tarball. That forces the whole package — sources, tests, and manifest — into one directory, +// which keeps every tool that copies the tree (Stryker's sandbox, coverage gates, Windows +// checkouts) away from directory symlinks. Nothing else verifies the packed tree compiles — its +// first consumer is a user's machine — so these guards hold the layout. const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); -const shippedManifestPath = path.join(repoRoot, 'apple/macos-helper/Package.swift'); -const testPackageRoot = path.join(repoRoot, 'apple/macos-helper-tests'); +const helperRoot = path.join(repoRoot, 'apple/macos-helper'); +const shippedManifestPath = path.join(helperRoot, 'Package.swift'); function declaredTargetNames( manifest: string, @@ -24,49 +25,29 @@ function declaredTargetNames( ].flatMap((match) => (match[1] === undefined ? [] : [match[1]])); } -function shippedManifest(): string { - return fs.readFileSync(shippedManifestPath, 'utf8'); -} - -function testManifest(): string { - return fs.readFileSync(path.join(testPackageRoot, 'Package.swift'), 'utf8'); -} - -test('the shipped macOS helper manifest declares no test targets', () => { - const manifest = shippedManifest(); - assert.doesNotMatch(manifest, /\.testTarget\(/); +test('the shipped macOS helper manifest declares the helper and its test targets', () => { + const manifest = fs.readFileSync(shippedManifestPath, 'utf8'); assert.deepEqual(declaredTargetNames(manifest, 'target'), ['AgentDeviceMacOSInput']); assert.deepEqual(declaredTargetNames(manifest, 'executableTarget'), ['AgentDeviceMacOSHelper']); + assert.deepEqual(declaredTargetNames(manifest, 'testTarget').sort(), [ + 'AgentDeviceMacOSHelperTests', + 'AgentDeviceMacOSInputTests', + ]); assert.match(manifest, /\.executable\(\s*name:\s*"agent-device-macos-helper"/); }); -test('the macOS helper test package compiles the shipped sources and owns the test targets', () => { - const manifest = testManifest(); - assert.equal( - fs.readlinkSync(path.join(testPackageRoot, 'Sources')), - path.join('..', 'macos-helper', 'Sources'), - 'the test package must compile the shipped sources, not a copy', - ); - assert.ok( - fs.existsSync(path.join(testPackageRoot, 'Sources/AgentDeviceMacOSHelper/main.swift')), - 'the Sources symlink must resolve into the shipped helper sources', - ); - - // Every target the shipped manifest builds must exist here too, or the tests would cover a - // tree users never compile. - const shippedTargets = [ - ...declaredTargetNames(shippedManifest(), 'target'), - ...declaredTargetNames(shippedManifest(), 'executableTarget'), - ]; - const testPackageTargets = [ - ...declaredTargetNames(manifest, 'target'), - ...declaredTargetNames(manifest, 'executableTarget'), - ]; - for (const name of shippedTargets) { - assert.ok(testPackageTargets.includes(name), `the test package must build ${name}`); +test('every declared test target holds a pack stub and at least one real test', () => { + const manifest = fs.readFileSync(shippedManifestPath, 'utf8'); + for (const name of declaredTargetNames(manifest, 'testTarget')) { + const targetDir = path.join(helperRoot, 'Tests', name); + const sources = fs.readdirSync(targetDir); + assert.ok( + sources.includes('PackStub.swift'), + `${name} must ship a PackStub.swift or the packed tree will not load`, + ); + assert.ok( + sources.some((file) => file !== 'PackStub.swift' && file.endsWith('.swift')), + `${name} must hold real tests in the checkout or the lane would cover nothing`, + ); } - assert.deepEqual( - declaredTargetNames(manifest, 'testTarget').sort(), - ['AgentDeviceMacOSHelperTests', 'AgentDeviceMacOSInputTests'].sort(), - ); }); From 57367bce474578b351dff1533a7e140fe31c100e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 18:23:37 +0200 Subject: [PATCH 5/7] fix(gates): check the helper pack stubs per declared test target The publish guard counted stubs instead of matching them to the test targets the manifest declares, so two stubs for one target could hide a missing one. Derive the target list with one owner (readMacosHelperTestTargets, exported from the guard's own module and used by the repo guard too) and require Tests//PackStub.swift for each. The repo guard now asserts on the real npm pack --dry-run file list instead of the files config text, and the fixture test carries the helper manifest so the stub branch is covered on both the pass and the missing-stub path. Closes the three review threads on cf63ab3df8. --- scripts/__tests__/size-report-package.test.ts | 44 +++++++++++---- scripts/size-report-package.mjs | 38 ++++++++++--- .../macos-helper-package-contents.test.ts | 53 +++++++++++++------ 3 files changed, 101 insertions(+), 34 deletions(-) diff --git a/scripts/__tests__/size-report-package.test.ts b/scripts/__tests__/size-report-package.test.ts index 5c70eab37d..9ce86516cd 100644 --- a/scripts/__tests__/size-report-package.test.ts +++ b/scripts/__tests__/size-report-package.test.ts @@ -41,18 +41,40 @@ test('publish package requires both Android helpers and excludes benchmark scrip ]), /benchmark or build scripts/, ); + // The helper ships as buildable source, so the manifest in the pack and the per-target + // PackStub.swift set must agree; the fixture carries both sides of that contract. + const helperManifest = { path: 'apple/macos-helper/Package.swift', size: 1 }; + const helperStubs = [ + { + path: 'apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift', + size: 1, + }, + { + path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift', + size: 1, + }, + ]; + const helperManifestSource = [ + '.target(', + ' name: "AgentDeviceMacOSInput",', + '.executableTarget(', + ' name: "AgentDeviceMacOSHelper",', + '.testTarget(', + ' name: "AgentDeviceMacOSHelperTests",', + '.testTarget(', + ' name: "AgentDeviceMacOSInputTests",', + ].join('\n'); assert.doesNotThrow(() => - assertPublishPackageContents([ - ...fixturePack.files, - { - path: 'apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift', - size: 1, - }, - { - path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift', - size: 1, - }, - ]), + assertPublishPackageContents([...fixturePack.files, helperManifest, ...helperStubs], { + macosHelperManifestSource: helperManifestSource, + }), + ); + assert.throws( + () => + assertPublishPackageContents([...fixturePack.files, helperManifest, helperStubs[0]!], { + macosHelperManifestSource: helperManifestSource, + }), + /pack stubs: apple\/macos-helper\/Tests\/AgentDeviceMacOSInputTests\/PackStub\.swift/, ); assert.throws( () => diff --git a/scripts/size-report-package.mjs b/scripts/size-report-package.mjs index a09e1a529a..e717123403 100644 --- a/scripts/size-report-package.mjs +++ b/scripts/size-report-package.mjs @@ -17,6 +17,17 @@ export function assertSnapshotBridgeAssets(presentPaths, context) { } } +/** + * The test targets the macOS helper manifest declares — the one reading of "which test targets + * exist". SwiftPM refuses to load a package whose declared test target has no sources, so each + * name here must appear in the pack as `apple/macos-helper/Tests//PackStub.swift`. + */ +export function readMacosHelperTestTargets(manifestSource) { + return [...manifestSource.matchAll(/\.testTarget\(\s*name:\s*"([^"]+)"/g)].map( + (match) => match[1], + ); +} + export function collectNpmPack(root) { const cachePath = path.join(root, '.tmp', 'npm-cache'); fs.mkdirSync(cachePath, { recursive: true }); @@ -28,6 +39,7 @@ export function collectNpmPack(root) { const pack = parseNpmPackOutput(stdout); const entries = normalizeNpmPackEntries(pack); assertPublishPackageContents(entries, { + repoRoot: root, requireSnapshotBridge: fs.existsSync(path.join(root, 'apple', 'snapshot-bridge')), }); return { @@ -89,14 +101,26 @@ export function assertPublishPackageContents(entries, options = {}) { if (shippedSwiftTests.length > 0) { throw new Error(`npm pack includes macOS helper test sources: ${shippedSwiftTests.join(', ')}`); } - const packStubs = paths.filter( - (entryPath) => - entryPath.startsWith('apple/macos-helper/Tests/') && entryPath.endsWith('/PackStub.swift'), - ); - if (paths.includes('apple/macos-helper/Package.swift') && packStubs.length < 2) { - throw new Error( - `npm pack is missing macOS helper test-target pack stubs: expected one PackStub.swift per test target, found ${packStubs.length}`, + // Read the target list from the same manifest the pack ships, so the pack and the manifest can + // never drift onto different target sets. A pack without the manifest ships no helper at all. + if (paths.includes('apple/macos-helper/Package.swift')) { + const manifestSource = + options.macosHelperManifestSource ?? + fs.readFileSync( + path.join(options.repoRoot ?? process.cwd(), 'apple/macos-helper/Package.swift'), + 'utf8', + ); + const testTargets = readMacosHelperTestTargets(manifestSource); + const missingStubs = testTargets.filter( + (target) => !paths.includes(`apple/macos-helper/Tests/${target}/PackStub.swift`), ); + if (missingStubs.length > 0) { + throw new Error( + `npm pack is missing macOS helper test-target pack stubs: ${missingStubs + .map((target) => `apple/macos-helper/Tests/${target}/PackStub.swift`) + .join(', ')}`, + ); + } } } diff --git a/test/repo-guards/macos-helper-package-contents.test.ts b/test/repo-guards/macos-helper-package-contents.test.ts index 7c73313602..2161692da9 100644 --- a/test/repo-guards/macos-helper-package-contents.test.ts +++ b/test/repo-guards/macos-helper-package-contents.test.ts @@ -1,8 +1,10 @@ import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { test } from 'vitest'; +import { readMacosHelperTestTargets } from '../../scripts/size-report-package.mjs'; // The npm package ships `apple/macos-helper` and builds it on the user's machine. Its manifest // declares the test targets, and `files` ships only each target's PackStub.swift: SwiftPM refuses @@ -11,43 +13,62 @@ import { test } from 'vitest'; // tarball. That forces the whole package — sources, tests, and manifest — into one directory, // which keeps every tool that copies the tree (Stryker's sandbox, coverage gates, Windows // checkouts) away from directory symlinks. Nothing else verifies the packed tree compiles — its -// first consumer is a user's machine — so these guards hold the layout. +// first consumer is a user's machine — so these guards hold the layout against the real pack list, +// not the `files` text. const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); const helperRoot = path.join(repoRoot, 'apple/macos-helper'); const shippedManifestPath = path.join(helperRoot, 'Package.swift'); -function declaredTargetNames( - manifest: string, - kind: 'target' | 'executableTarget' | 'testTarget', -): string[] { +function shippedManifest(): string { + return fs.readFileSync(shippedManifestPath, 'utf8'); +} + +function declaredTargetNames(manifest: string, kind: 'target' | 'executableTarget'): string[] { return [ ...manifest.matchAll(new RegExp(String.raw`\.${kind}\(\s*name:\s*"([^"]+)"`, 'g')), ].flatMap((match) => (match[1] === undefined ? [] : [match[1]])); } +function packedPaths(): string[] { + const stdout = execFileSync('npm', ['pack', '--dry-run', '--json', '--ignore-scripts'], { + cwd: repoRoot, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + const pack = JSON.parse(stdout); + return (Array.isArray(pack) ? pack[0] : pack).files.map((entry) => entry.path); +} + test('the shipped macOS helper manifest declares the helper and its test targets', () => { - const manifest = fs.readFileSync(shippedManifestPath, 'utf8'); + const manifest = shippedManifest(); assert.deepEqual(declaredTargetNames(manifest, 'target'), ['AgentDeviceMacOSInput']); assert.deepEqual(declaredTargetNames(manifest, 'executableTarget'), ['AgentDeviceMacOSHelper']); - assert.deepEqual(declaredTargetNames(manifest, 'testTarget').sort(), [ + assert.deepEqual(readMacosHelperTestTargets(manifest).sort(), [ 'AgentDeviceMacOSHelperTests', 'AgentDeviceMacOSInputTests', ]); assert.match(manifest, /\.executable\(\s*name:\s*"agent-device-macos-helper"/); }); -test('every declared test target holds a pack stub and at least one real test', () => { - const manifest = fs.readFileSync(shippedManifestPath, 'utf8'); - for (const name of declaredTargetNames(manifest, 'testTarget')) { - const targetDir = path.join(helperRoot, 'Tests', name); - const sources = fs.readdirSync(targetDir); - assert.ok( - sources.includes('PackStub.swift'), - `${name} must ship a PackStub.swift or the packed tree will not load`, - ); +test('every declared test target holds at least one real test in the checkout', () => { + for (const name of readMacosHelperTestTargets(shippedManifest())) { + const sources = fs.readdirSync(path.join(helperRoot, 'Tests', name)); assert.ok( sources.some((file) => file !== 'PackStub.swift' && file.endsWith('.swift')), `${name} must hold real tests in the checkout or the lane would cover nothing`, ); } }); + +test('the pack ships one PackStub.swift per declared test target and no other test source', () => { + const paths = packedPaths(); + const testTargetPaths = paths.filter((entryPath) => + entryPath.startsWith('apple/macos-helper/Tests/'), + ); + // Behavior, not config text: whatever `files` says, the tarball itself must carry exactly the + // stubs — a missing one breaks the install-time `swift build`, an extra one ships dead tests. + const expected = readMacosHelperTestTargets(shippedManifest()) + .map((target) => `apple/macos-helper/Tests/${target}/PackStub.swift`) + .sort(); + assert.deepEqual([...testTargetPaths].sort(), expected); +}); From 650f9b0d90dd0cdfb01a41eb835afb8dbe47cf83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 18:25:33 +0200 Subject: [PATCH 6/7] chore(gates): type the size-report exports the helper pack guard imports The repo guard imports the plain-Node size-report module, which the typechecked program excludes, so tsc saw it as an untyped boundary. A hand-written .d.mts declares the two exports the checked code reads, keeping the script bare-node runnable, and the guard's JSON parse carries its own narrowing. --- .fallowrc.json | 6 +++++- scripts/size-report-package.d.mts | 13 +++++++++++++ .../macos-helper-package-contents.test.ts | 5 +++-- 3 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 scripts/size-report-package.d.mts diff --git a/.fallowrc.json b/.fallowrc.json index 76de910d65..02b12078c5 100644 --- a/.fallowrc.json +++ b/.fallowrc.json @@ -64,7 +64,11 @@ "scripts/maestro-conformance/corpus/**", "apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests.xctestplan", "scripts/help-conformance-sample-outputs.d.mts", - "scripts/help-conformance-runner-output.d.mts" + "scripts/help-conformance-runner-output.d.mts", + // A hand-written type surface for a plain-Node .mjs the typechecked program (and Fallow's + // graph) cannot import; its only consumer is the .mjs's real importers, resolved by tsc, not + // by an import edge. Same shape as the help-conformance declaration files above. + "scripts/size-report-package.d.mts" ], "dynamicallyLoaded": ["packages/daemon/src/execution/interaction/index.ts"], "ignoreDependencies": [ diff --git a/scripts/size-report-package.d.mts b/scripts/size-report-package.d.mts new file mode 100644 index 0000000000..d6241e686e --- /dev/null +++ b/scripts/size-report-package.d.mts @@ -0,0 +1,13 @@ +// Type surface for the plain-Node size-report module (it must stay runnable by bare `node`, so +// it is not in the typechecked program). TypeScript resolves this file when a checked module +// imports `./size-report-package.mjs`; keep it in step with the exports there. +export function readMacosHelperTestTargets(manifestSource: string): string[]; +export type NpmPackEntry = Readonly<{ path: string; size: number }>; +export function assertPublishPackageContents( + entries: readonly NpmPackEntry[], + options?: Readonly<{ + repoRoot?: string; + requireSnapshotBridge?: boolean; + macosHelperManifestSource?: string; + }>, +): void; diff --git a/test/repo-guards/macos-helper-package-contents.test.ts b/test/repo-guards/macos-helper-package-contents.test.ts index 2161692da9..7cdb0b57ac 100644 --- a/test/repo-guards/macos-helper-package-contents.test.ts +++ b/test/repo-guards/macos-helper-package-contents.test.ts @@ -35,8 +35,9 @@ function packedPaths(): string[] { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024, }); - const pack = JSON.parse(stdout); - return (Array.isArray(pack) ? pack[0] : pack).files.map((entry) => entry.path); + const parsed = JSON.parse(stdout); + const pack: { files?: { path?: unknown }[] } = Array.isArray(parsed) ? parsed[0] : parsed; + return pack.files?.map((entry) => String(entry.path)) ?? []; } test('the shipped macOS helper manifest declares the helper and its test targets', () => { From 563d35e6ea52bd3d0f634387a297b527ee478c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 17 Oct 2026 18:41:11 +0200 Subject: [PATCH 7/7] fix(gates): require the helper manifest beside its shipped sources and run npm cross-platform The pack guard warned in a comment that a manifest-less helper pack is unbuildable but only checked the stub set, so a dropped files entry that ships Sources without Package.swift passed. Require the manifest whenever any apple/macos-helper/Sources entry is packed, and let the fixture pack stand in for a real helper pack (manifest, sources, stubs) so both the missing-manifest and missing-stub failures are covered. The repo guard ran npm through execFileSync, which dies on ENOENT for npm.cmd on Windows; runCmdSync resolves PATH extensions there, as installed-package-metro does. Closes both review threads on 650f9b0d90. --- .../fixtures/size-report-npm-pack.json | 96 +++++++++++++++---- scripts/__tests__/size-report-package.test.ts | 95 +++++++++--------- scripts/size-report-package.mjs | 11 ++- .../macos-helper-package-contents.test.ts | 9 +- 4 files changed, 145 insertions(+), 66 deletions(-) diff --git a/scripts/__tests__/fixtures/size-report-npm-pack.json b/scripts/__tests__/fixtures/size-report-npm-pack.json index 5d5cf29e20..1a11bb6334 100644 --- a/scripts/__tests__/fixtures/size-report-npm-pack.json +++ b/scripts/__tests__/fixtures/size-report-npm-pack.json @@ -1,21 +1,83 @@ { "unpackedSize": 2319, "files": [ - { "path": "dist/src/index.js", "size": 401 }, - { "path": "dist/src/index.d.ts", "size": 102 }, - { "path": "dist/apple/runner/RunnerTests.swift", "size": 503 }, - { "path": "dist/apple/snapshot-presentation/Package.swift", "size": 113 }, - { "path": "apple/snapshot-bridge/SnapshotBridge.m", "size": 0 }, - { "path": "apple/snapshot-bridge/SnapshotBridgeRuntime.m", "size": 0 }, - { "path": "apple/snapshot-bridge/SnapshotBridgeRuntime.h", "size": 0 }, - { "path": "apple/snapshot-bridge/SnapshotBridgeCapture.h", "size": 0 }, - { "path": "apple/snapshot-bridge/SnapshotBridgeCapture.m", "size": 0 }, - { "path": "apple/macos-helper/Sources/main.swift", "size": 211 }, - { "path": "android/snapshot-helper/dist/helper.apk", "size": 307 }, - { "path": "android/snapshot-helper/dist/helper.manifest.json", "size": 99 }, - { "path": "android/ime-helper/dist/helper.apk", "size": 307 }, - { "path": "android/ime-helper/dist/helper.manifest.json", "size": 99 }, - { "path": "package.json", "size": 99 }, - { "path": "vendor/unknown.bin", "size": 78 } - ] + { + "path": "dist/src/index.js", + "size": 401 + }, + { + "path": "dist/src/index.d.ts", + "size": 102 + }, + { + "path": "dist/apple/runner/RunnerTests.swift", + "size": 503 + }, + { + "path": "dist/apple/snapshot-presentation/Package.swift", + "size": 113 + }, + { + "path": "apple/snapshot-bridge/SnapshotBridge.m", + "size": 0 + }, + { + "path": "apple/snapshot-bridge/SnapshotBridgeRuntime.m", + "size": 0 + }, + { + "path": "apple/snapshot-bridge/SnapshotBridgeRuntime.h", + "size": 0 + }, + { + "path": "apple/snapshot-bridge/SnapshotBridgeCapture.h", + "size": 0 + }, + { + "path": "apple/snapshot-bridge/SnapshotBridgeCapture.m", + "size": 0 + }, + { + "path": "apple/macos-helper/Package.swift", + "size": 1057 + }, + { + "path": "apple/macos-helper/Sources/main.swift", + "size": 211 + }, + { + "path": "android/snapshot-helper/dist/helper.apk", + "size": 307 + }, + { + "path": "android/snapshot-helper/dist/helper.manifest.json", + "size": 99 + }, + { + "path": "android/ime-helper/dist/helper.apk", + "size": 307 + }, + { + "path": "android/ime-helper/dist/helper.manifest.json", + "size": 99 + }, + { + "path": "package.json", + "size": 99 + }, + { + "path": "vendor/unknown.bin", + "size": 78 + }, + { + "path": "apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift", + "size": 426 + }, + { + "path": "apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift", + "size": 426 + } + ], + "entryCount": 19, + "size": 3376 } diff --git a/scripts/__tests__/size-report-package.test.ts b/scripts/__tests__/size-report-package.test.ts index 9ce86516cd..6c9397e02d 100644 --- a/scripts/__tests__/size-report-package.test.ts +++ b/scripts/__tests__/size-report-package.test.ts @@ -9,12 +9,31 @@ const fixturePack = JSON.parse( await readFile(join(import.meta.dirname, 'fixtures', 'size-report-npm-pack.json'), 'utf8'), ); +// The fixture pack ships the macOS helper (manifest, sources, and per-target PackStub.swift), +// so every assertion here names the manifest source the guard would read from the packed tree; +// without it the guard falls back to this checkout's manifest via repoRoot. +const helperManifestSource = [ + '.target(', + ' name: "AgentDeviceMacOSInput",', + '.executableTarget(', + ' name: "AgentDeviceMacOSHelper",', + '.testTarget(', + ' name: "AgentDeviceMacOSHelperTests",', + '.testTarget(', + ' name: "AgentDeviceMacOSInputTests",', +].join('\n'); + test('publish package requires both Android helpers and excludes benchmark scripts', () => { - assert.doesNotThrow(() => assertPublishPackageContents(fixturePack.files)); + assert.doesNotThrow(() => + assertPublishPackageContents(fixturePack.files, { + macosHelperManifestSource: helperManifestSource, + }), + ); assert.throws( () => assertPublishPackageContents( fixturePack.files.filter((entry) => !entry.path.startsWith('android/ime-helper/')), + { macosHelperManifestSource: helperManifestSource }, ), /android\/ime-helper/, ); @@ -31,62 +50,52 @@ test('publish package requires both Android helpers and excludes benchmark scrip assert.doesNotThrow(() => assertPublishPackageContents( fixturePack.files.filter((entry) => !entry.path.startsWith('apple/snapshot-bridge/')), + { macosHelperManifestSource: helperManifestSource }, ), ); assert.throws( () => - assertPublishPackageContents([ - ...fixturePack.files, - { path: 'scripts/ios-snapshot-benchmark/run.ts', size: 1 }, - ]), + assertPublishPackageContents( + [...fixturePack.files, { path: 'scripts/ios-snapshot-benchmark/run.ts', size: 1 }], + { macosHelperManifestSource: helperManifestSource }, + ), /benchmark or build scripts/, ); - // The helper ships as buildable source, so the manifest in the pack and the per-target - // PackStub.swift set must agree; the fixture carries both sides of that contract. - const helperManifest = { path: 'apple/macos-helper/Package.swift', size: 1 }; - const helperStubs = [ - { - path: 'apple/macos-helper/Tests/AgentDeviceMacOSHelperTests/PackStub.swift', - size: 1, - }, - { - path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift', - size: 1, - }, - ]; - const helperManifestSource = [ - '.target(', - ' name: "AgentDeviceMacOSInput",', - '.executableTarget(', - ' name: "AgentDeviceMacOSHelper",', - '.testTarget(', - ' name: "AgentDeviceMacOSHelperTests",', - '.testTarget(', - ' name: "AgentDeviceMacOSInputTests",', - ].join('\n'); - assert.doesNotThrow(() => - assertPublishPackageContents([...fixturePack.files, helperManifest, ...helperStubs], { - macosHelperManifestSource: helperManifestSource, - }), - ); + // Each declared test target needs its PackStub.swift or the install-time `swift build` of the + // packed tree dies; a missing one must name the missing path. assert.throws( () => - assertPublishPackageContents([...fixturePack.files, helperManifest, helperStubs[0]!], { - macosHelperManifestSource: helperManifestSource, - }), + assertPublishPackageContents( + fixturePack.files.filter( + (entry) => + entry.path !== 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/PackStub.swift', + ), + { macosHelperManifestSource: helperManifestSource }, + ), /pack stubs: apple\/macos-helper\/Tests\/AgentDeviceMacOSInputTests\/PackStub\.swift/, ); assert.throws( () => - assertPublishPackageContents([ - ...fixturePack.files, - { - path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/ScheduleTests.swift', - size: 1, - }, - ]), + assertPublishPackageContents( + [ + ...fixturePack.files, + { + path: 'apple/macos-helper/Tests/AgentDeviceMacOSInputTests/ScheduleTests.swift', + size: 1, + }, + ], + { macosHelperManifestSource: helperManifestSource }, + ), /macOS helper test sources/, ); + // Sources without the manifest are not a buildable Swift package; the pack must say so. + assert.throws( + () => + assertPublishPackageContents( + fixturePack.files.filter((entry) => entry.path !== 'apple/macos-helper/Package.swift'), + ), + /sources without its Package\.swift/, + ); }); test('Markdown emphasizes total install size and startup without duplicate breakdowns', () => { diff --git a/scripts/size-report-package.mjs b/scripts/size-report-package.mjs index e717123403..94f0051852 100644 --- a/scripts/size-report-package.mjs +++ b/scripts/size-report-package.mjs @@ -101,8 +101,15 @@ export function assertPublishPackageContents(entries, options = {}) { if (shippedSwiftTests.length > 0) { throw new Error(`npm pack includes macOS helper test sources: ${shippedSwiftTests.join(', ')}`); } - // Read the target list from the same manifest the pack ships, so the pack and the manifest can - // never drift onto different target sets. A pack without the manifest ships no helper at all. + // Helper sources without the manifest are not a buildable Swift package, so a dropped + // `files` entry that ships Sources but not Package.swift must fail here, not on the user's + // machine. With the manifest present, read the target list from it — the pack and the + // manifest can then never drift onto different target sets. + if (paths.some((entryPath) => entryPath.startsWith('apple/macos-helper/Sources/'))) { + if (!paths.includes('apple/macos-helper/Package.swift')) { + throw new Error('npm pack ships macOS helper sources without its Package.swift'); + } + } if (paths.includes('apple/macos-helper/Package.swift')) { const manifestSource = options.macosHelperManifestSource ?? diff --git a/test/repo-guards/macos-helper-package-contents.test.ts b/test/repo-guards/macos-helper-package-contents.test.ts index 7cdb0b57ac..ea585c336d 100644 --- a/test/repo-guards/macos-helper-package-contents.test.ts +++ b/test/repo-guards/macos-helper-package-contents.test.ts @@ -1,8 +1,8 @@ import assert from 'node:assert/strict'; -import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { runCmdSync } from '@agent-device/host-kit/command'; import { test } from 'vitest'; import { readMacosHelperTestTargets } from '../../scripts/size-report-package.mjs'; @@ -30,12 +30,13 @@ function declaredTargetNames(manifest: string, kind: 'target' | 'executableTarge } function packedPaths(): string[] { - const stdout = execFileSync('npm', ['pack', '--dry-run', '--json', '--ignore-scripts'], { + // runCmdSync resolves `npm` through the PATH extensions a Windows checkout needs; a bare + // execFileSync('npm', ...) there dies on ENOENT for npm.cmd. + const result = runCmdSync('npm', ['pack', '--dry-run', '--json', '--ignore-scripts'], { cwd: repoRoot, - encoding: 'utf8', maxBuffer: 64 * 1024 * 1024, }); - const parsed = JSON.parse(stdout); + const parsed = JSON.parse(result.stdout); const pack: { files?: { path?: unknown }[] } = Array.isArray(parsed) ? parsed[0] : parsed; return pack.files?.map((entry) => String(entry.path)) ?? []; }