Fix ETXTBSY race in claude-swap CLI card test - #2644
Merged
Conversation
The test wrote a shell script, set mode 0755, and executed it immediately. Under swift test --parallel a concurrent fork inherits the still-open write descriptor, so execve fails with ETXTBSY and the launch reports Cocoa 256. Write the script body as data and execute a checked-in trampoline that reads it, so execve only ever touches a file no test process has written. Measured in swift:6.3.3-noble on arm64: 112 failures in 600 attempts with the old shape, 0 in 600 with the trampoline.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 482feb758b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Author
|
Note for reviewers: this flake surfaced on #2640, a two-file docs change that had Once this merges, I will rebase #2640 onto |
The first commit only touched Tests/CodexBarTests, which Package.swift builds on macOS alone. The Linux job compiles CodexBarLinuxTests from TestsLinux, so the race remained in the target that actually flaked. Convert all three TestsLinux sites through a shared FakeExecutable helper, and keep the macOS mirror in step. Verified on Linux arm64 in swift:6.3.3-noble: 355 tests in 53 suites pass.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs changes before merge. Reviewed August 4, 2026, 3:59 PM ET / 19:59 UTC. ClawSweeper reviewWhat this changesThe PR replaces directly executed temporary fake CLI scripts in Linux tests with a stable checked-in shell trampoline to avoid parallel-test ETXTBSY failures. Merge readinessKeep this PR open for a narrow cleanup: the ETXTBSY repair is credible and proven, but it leaves temporary UUID-named script sidecars behind in two Linux test paths. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s Linux provider tests create fake command-line tools and launch them through provider probes. The new helper writes each test-specific body beside a stable trampoline, which passes the resulting command output back to the probe under test. flowchart LR
A[Linux provider tests] --> B[Fake CLI body]
B --> C[Temporary sidecar script]
A --> D[Stable trampoline]
D --> E[Shell runs sidecar]
E --> F[Provider probe result]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the trampoline approach, but give the helper or both callers ownership of deleting both the symlink and its adjacent script body. Do we have a high-confidence way to reproduce the issue? Yes. The contributor reports a Linux arm64 harness that reproduced ETXTBSY 112 times in 600 attempts with the existing write/chmod/exec shape and zero times with the trampoline. Is this the best way to solve the issue? Yes, with one small correction. Executing a stable trampoline avoids executing the just-written inode, but cleanup must also remove the generated sidecar. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a82f509ea8e7. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
Owner
|
Landed. Verification before merge (independent maintainer-agent review):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a flaky
linux-arm64failure inCLICardsClaudeSwapTests. The test wrote ashell script, set mode
0755, and executed it immediately. Underswift test --parallelthat races with every other test that spawns a childprocess, and
execvefails withETXTBSY.Tests now write the script body as data and execute a checked-in trampoline that
reads it, so
execveonly ever touches a file no test process has written.Root cause
Swift Testing runs the suite concurrently inside one process, so all tests share
one file descriptor table. The sequence:
Data.write(to:)opens a write descriptor on the new script.Process.run()and forks. The child inherits thatdescriptor, because Linux Foundation has no
POSIX_SPAWN_CLOEXEC_DEFAULT.execvereturnsETXTBSY, because the inode isstill open for writing in the child.
Foundation flattens the errno and reports
NSCocoaErrorDomain error 256, which iswhy the original CI failure named no cause.
macOS never hits this. Darwin's Foundation spawns with
POSIX_SPAWN_CLOEXEC_DEFAULT, so children do not inherit the descriptor.Scope
CodexBarLinuxTests(TestsLinux) is the target the Linux job compiles;CodexBarTestsis macOS-only (Package.swift:190-204). Three sites inTestsLinuxused the racing pattern, and all three ran in the failing job:CLICardsClaudeSwapTests.swift, the test that actually flakedPlatformGatingTests.swift, which executes a fakeclaudeCLIAntigravityCLIStrategyLinuxTests.swiftThe mirrored
Tests/CodexBarTests/CLICardsClaudeSwapTests.swiftis updated too, sothe two trees stay in step. The remaining write-then-execute sites under
Tests/CodexBarTestsare macOS-only and cannot hit this race.The suites supplying the racing forks are
ShellCommandSessionLinuxTests,ProcessPipeCaptureLinuxTests,CostUsageScanExecutorLinuxTests, andHookDispatchTests.Verification
Reproduced and measured in
swift:6.3.3-nobleon arm64, matching the CI runner. Astandalone harness ran the same shape:
Data.write(to:)for the writer,Foundation.Processfor the forkers, andposix_spawnfor the launch so theerrno stays visible.
ETXTBSYAlso run:
swift test --parallelon Linux arm64 in Docker: 355 tests in 53 suites passed.swift test --filter CLICardsClaudeSwapTestson macOS: 26 tests passed.make check: 0 violations in 1756 files.Notes
The script heredocs are unchanged, so each test still shows what its fake CLI
does.
$0inside a shebang script is the path handed toexecve, that is thesymlink, on both Linux and macOS. That is verified on both platforms and is what
lets
"$0.sh"resolve per test.Found while running CI for #2640, a docs-only change that had no way to cause it.
Failing job:
https://github.com/steipete/CodexBar/actions/runs/30885238832/job/91914882363