Fix Linux posix_spawn compatibility - #2531
Conversation
Signed-off-by: Emre Koca <110906681+kocaemre@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9f6e754c4
ℹ️ 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".
|
|
||
| enum PosixSpawnFileActionsCompatibility { | ||
| static func addChangeDirectory( | ||
| _ fileActions: inout posix_spawn_file_actions_t, |
There was a problem hiding this comment.
Accept Darwin's optional file-action handle
On Darwin, the call sites in this commit still declare fileActions as posix_spawn_file_actions_t?, matching the imported posix_spawn_file_actions_* APIs for the opaque spawn handle. This wrapper now requires a non-optional inout posix_spawn_file_actions_t, so macOS builds hit an inout optional/non-optional mismatch at the changed addChangeDirectory(&fileActions, ...) calls even though the old direct C calls compiled; add a Darwin-specific overload/typealias that accepts the optional handle.
Useful? React with 👍 / 👎.
|
Codex review: needs maintainer review before merge. Reviewed August 1, 2026, 7:42 AM ET / 11:42 UTC. ClawSweeper reviewWhat this changesThe PR adds a Glibc declaration shim for two GNU Merge readinessThis remains necessary: current Priority: P1 Review scores
Verification
How this fits togetherCodexBar launches local CLI and PTY subprocesses for provider integrations. The launchers assemble POSIX spawn file actions for working-directory and descriptor setup before starting each child process. flowchart LR
A[Provider CLI request] --> B[Process or PTY launcher]
B --> C[POSIX spawn file actions]
C --> D[Platform compatibility wrapper]
D --> E[Darwin or Linux C library]
E --> F[Child process starts]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge the narrow wrapper after a macOS build confirms both platform handle representations compile, preserving direct platform behavior while isolating Swift Glibc-import gaps in one place. Do we have a high-confidence way to reproduce the issue? Yes. Current main directly imports the two GNU extensions that Swift’s Glibc module omits, and the contributor supplied after-fix Swift 6.2 Ubuntu 22.04 build and test output from the revised PR head. Is this the best way to solve the issue? Yes. A small typed compatibility wrapper is the narrowest maintainable fix because it preserves existing launcher behavior while isolating only the missing Glibc declarations. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9bb9c42fb6f0. LabelsLabel justifications:
EvidenceWhat 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 (2 earlier review cycles) |
Signed-off-by: Emre Koca <110906681+kocaemre@users.noreply.github.com>
|
@clawsweeper re-review I pushed What changed:
Linux verification from Ubuntu 22.04 / Swift 6.2: Boundary note: I also re-ran |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
posix_spawn_file_actions_addchdir_npandposix_spawn_file_actions_addclosefrom_np.Why
On Ubuntu 22.04 with Swift 6.2,
swift test listfailed during compilation because the symbols exist in glibc but are not exposed by Swift's Glibc module:The new declarations are limited to the Glibc build and keep the existing runtime behavior.
Testing
PATH=/opt/swift-6.2/usr/bin:$PATH swift test listPATH=/opt/swift-6.2/usr/bin:$PATH swift buildPATH=/opt/swift-6.2/usr/bin:$PATH ./Scripts/test.shgit diff --checkPATH=/opt/swift-6.2/usr/bin:$PATH .build/lint-tools/bin/swiftformat Sources/CodexBarCore/Host/Process/PosixSpawnFileActionsCompatibility.swift Sources/CodexBarCore/Host/Process/PosixSpawnFileActionsCloseFrom.swift Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift Sources/CodexBarCore/Providers/Antigravity/AntigravityCLISession.swift --lintNote: I also ran
make check; its portable checks passed, but the full command stopped in repository-wide SwiftFormat lint with pre-existingenumNamespacestimeouts in unrelated provider files before SwiftLint ran.