Repository navigation
build(package): stop shipping Swift test sources - #3785
Conversation
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.
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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 12 files
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Size Report
Startup median (7 runs, lower is better):
|
|
|
The code looks ready at 449b4d4: dropping the Swift test sources from the package and keeping them in a symlinked test package is sound, and the guard test blocks re-adding a testTarget. CI is still pending, with Smoke Tests (3), Repo Guards & Compatibility, Coverage and Integration Tests all queued or running and none failed, so I can't attribute anything yet. I did not run swift build, swift test or npm pack, so the 8-test pass and the packed-tree build are your claims for now. SwiftPM accepting a symlinked Sources directory on the CI macOS runner is unproven until the macos-helper gate in macos.yml passes. Not blocking, and you can take or leave both: the header comment in test/repo-guards/macos-helper-package-contents.test.ts says nothing in the repo compiles the shipped manifest, but |
thymikee
added
the
ready-for-human
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.
Coverage gate: EISDIR fixedCause. The Fix. Tests. Entry-point regressions in both gates' Receipt. |
…out 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).
|
Round 2: removed the directory symlink (option B). Head The
Validation: |
There was a problem hiding this comment.
All reported issues were addressed across 14 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
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/<Target>/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 cf63ab3.
thymikee
left a comment
thymikee
left a comment
There was a problem hiding this comment.
round 3 validation running
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.
|
Round 3: cubic review threads 1–3 fixed. Head
Typecheck fallout from the guard's Validation: |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…d 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 650f9b0.
|
Round 4: the two remaining cubic threads fixed. Head
Validation: |
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/__tests__/fixtures/size-report-npm-pack.json">
<violation number="1" location="scripts/__tests__/fixtures/size-report-npm-pack.json:41">
P3: This reformat turns the fixture's unchanged entries into expanded objects, so the diff is far larger than the two added entries. Restore the compact one-line object style used before this PR so the change only shows the new `Package.swift` and `PackStub.swift` entries.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| "size": 0 | ||
| }, | ||
| { | ||
| "path": "apple/macos-helper/Package.swift", |
There was a problem hiding this comment.
P3: This reformat turns the fixture's unchanged entries into expanded objects, so the diff is far larger than the two added entries. Restore the compact one-line object style used before this PR so the change only shows the new Package.swift and PackStub.swift entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/__tests__/fixtures/size-report-npm-pack.json, line 41:
<comment>This reformat turns the fixture's unchanged entries into expanded objects, so the diff is far larger than the two added entries. Restore the compact one-line object style used before this PR so the change only shows the new `Package.swift` and `PackStub.swift` entries.</comment>
<file context>
@@ -1,21 +1,83 @@
+ "size": 0
+ },
+ {
+ "path": "apple/macos-helper/Package.swift",
+ "size": 1057
+ },
</file context>
There was a problem hiding this comment.
Assessment: true. My script rewrite of the fixture re-expanded every entry into multi-line objects; the compact one-line style should be restored, keeping only the three added entries (Package.swift + the two PackStub.swift). Not pushed this round: the coordinator closed guard-review rounds at 563d35e and asked that further threads be left for the maintainer, and a push would re-trigger this bot's review. Leaving open; uncontroversial formatting fix.
…ment * origin/main: build(package): stop shipping Swift test sources (#3785) fix(scroll): take edge hints from a same-rect scrollable ancestor (#3784) docs(agents): write agent docs in ASD-STE100 and remove misleading docs (#3723) fix(daemon): retain cleanup locks and preserve healed snapshots (#3760) # Conflicts: # packages/daemon/src/sessions/__tests__/replay-repair/session-replay-repair-record-exclusion.test.ts
…refactor/daemon-runtime-no-process-globals * origin/refactor/host-environment: build(package): stop shipping Swift test sources (#3785) fix(scroll): take edge hints from a same-rect scrollable ancestor (#3784) docs(agents): write agent docs in ASD-STE100 and remove misleading docs (#3723) fix(daemon): retain cleanup locks and preserve healed snapshots (#3760) test: say which package still reads PATH from the process in the recording scenario
1 participant
Summary
The npm package shipped
apple/macos-helper/Tests/**, but that Swift never compiles into the helper.apple/macos-helper/Package.swiftdeclares the test targets andpackage.jsonfilesexcludesapple/macos-helper/Tests/**from the pack.PackStub.swiftper test target: SwiftPM refuses to load a package whose declared test target has no sources, and the install-timeswift buildruns on the packed tree, so the stubs are load-bearing. An earlier iteration shipped a manifest without test targets behind aSourcessymlink in a dev-only test package; the symlink broke Stryker's sandbox copy (EISDIR, run 39045124618) and any tool that copies the tree, and SwiftPM rejects sources outside a package root, so no split layout avoids it.scripts/size-report-package.mjs) derives the declared test targets from the shipped manifest and requires each target'sPackStub.swiftin the pack; the repo guard asserts the same invariant against the realnpm pack --dry-runfile list, so no test source can leak back in and no stub can go missing.FileManager.fileExistscheck in the shipped manifest (depends on the manifest's working directory and sandbox); a dev-only test package compiling the shipped sources (needs a directory symlink or an out-of-rootpath:, both rejected by tooling/SwiftPM).Validation
npm pack, extracted into a clean dir, thenswift build -c release --package-path apple/macos-helper→Build complete. The pack holds no test sources beyondTests/AgentDeviceMacOSHelperTests/PackStub.swiftandTests/AgentDeviceMacOSInputTests/PackStub.swift.npm pack --dry-run --json --ignore-scripts, identical builds): 525 → 523 files, tarball 1,435,877 → 1,433,786 B (−2,091 B), unpacked 4,661,050 → 4,653,755 B (−7,295 B).pnpm test:macos-helper→ 8 XCTest cases (6 Input + 2+1+5 Helper), 0 failures. Repo guard, publish-guard, and size-report tests pass.pnpm gate mutation --modules snapshot-occlusionlocally → Stryker sandbox copy passes (no EISDIR), initial test run succeeded, lane envelope pass at stage complete. No repo tool copies a directory symlink anymore; theregularFileguard inscripts/lib/repo-git.tsstays as the general fix for non-blob diff sides.pnpm check:affected --run --head 563d35e6ea→[passed] exactCommit=true, all runnable checks green; receipt.tmp/validation/run-zVw3OY/receipt.json.