Skip to content

fix(test): clean Swift toolchain temporary directories - #1664

Merged
thymikee merged 3 commits into
mainfrom
agent/fix-swiftpm-tmpdir-leaks
Aug 7, 2026
Merged

fix(test): clean Swift toolchain temporary directories#1664
thymikee merged 3 commits into
mainfrom
agent/fix-swiftpm-tmpdir-leaks

Conversation

@thymikee

@thymikee thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Prevent Apple build lanes from leaving SwiftPM TemporaryDirectory.* markers and lock files in the Darwin user temp directory.

Xcode 26.2 / Swift 6.2.3 retained these artifacts even after successful swift build and xcodebuild build-for-testing commands. The affected build-lane invocations now run with an owned TMPDIR that is removed deterministically while preserving exit codes and forwarding termination signals. Production runner launches remain unchanged.

Adds regression coverage for successful and failing child processes and pins the macOS-helper and XCTest build lanes to the cleanup wrapper. Four files changed; scope stayed within Apple build/test tooling.

Follow-up to #1595.

Validation

Before the fix, isolated successful runs retained six marker directories plus four locks for pnpm build:macos-helper, and two markers plus one lock for pnpm build:xcuitest:ios. After the fix, both commands succeeded with empty outer probe directories.

The regression was observed failing 3/3 before implementation and passing 3/3 afterward. pnpm test:smoke, the full Node integration lane, and pnpm check:affected --run passed; the affected aggregate covered formatting, lint, typecheck, layering, Fallow, package verification, build, changed-line coverage, integration progress, and replay compatibility.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.98 MB 1.98 MB 0 B
JS gzip 641.3 kB 641.3 kB 0 B
npm tarball 769.2 kB 769.2 kB +21 B
npm unpacked 2.69 MB 2.69 MB +171 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.3 ms 26.9 ms -0.4 ms
CLI --help 65.7 ms 64.8 ms -0.9 ms

Top changed chunks: no changes in the largest emitted chunks.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-07 14:14 UTC

@thymikee

thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

P1: scripts/swift-toolchain-tmpdir.ts deletes the owned TMPDIR before the toolchain child has exited on SIGINT/SIGTERM. The handler calls child.kill(signal) and immediately process.exit(...); the exit hook removes the directory while xcodebuild/Swift or descendants may still be flushing, and the existing tests cover only normal exit 0/17. Forward the signal once, await child close/wait before cleanup and exit (with a bounded forced-kill policy if needed), and add a delayed-SIGTERM regression proving TMPDIR remains until the child exits.

@thymikee
thymikee force-pushed the agent/fix-swiftpm-tmpdir-leaks branch from b6b740c to e91b651 Compare August 7, 2026 11:46
@thymikee

thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head e91b651: all CI is green, and the Apple build-lane evidence is adequate for this tooling-only change. One P1 still blocks readiness.

The signal fix waits for and force-kills only the direct swift/xcodebuild child. A spawned compiler/build descendant can outlive that parent; once the direct child closes, the wrapper exits and deletes its owned TMPDIR underneath the surviving descendant. The new delayed-shutdown test exercises only one child, so it cannot catch this production shape.

Please launch the toolchain under an owned process group, forward and escalate signals to that group, retain TMPDIR until the group is quiescent with a bounded policy, and add a red/green regression where the direct child exits on SIGTERM while a delayed descendant continues to use/check TMPDIR.

@thymikee

thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 348cf8c4: the descendant/process-group P1 is fixed. The wrapper creates an owned process group, forwards and escalates signals to the group, waits boundedly for group quiescence before exit-hook cleanup, and the delayed-descendant regression is non-vacuous—it fails if only the direct child is signaled or TMPDIR is deleted after that child exits. Code review is clean and this is ready for human review.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 7, 2026
@thymikee
thymikee marked this pull request as ready for review August 7, 2026 14:13
@thymikee
thymikee merged commit 7b4e461 into main Aug 7, 2026
29 of 31 checks passed
@thymikee
thymikee deleted the agent/fix-swiftpm-tmpdir-leaks branch August 7, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant