Skip to content

Add musl import compatibility for Linux CLI#1601

Closed
Yuxin-Qiao wants to merge 4 commits into
steipete:mainfrom
Yuxin-Qiao:codex/musl-cli-import-compat
Closed

Add musl import compatibility for Linux CLI#1601
Yuxin-Qiao wants to merge 4 commits into
steipete:mainfrom
Yuxin-Qiao:codex/musl-cli-import-compat

Conversation

@Yuxin-Qiao

@Yuxin-Qiao Yuxin-Qiao commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Refs #1524

Summary

This is a first-stage source compatibility PR for building the CLI-facing code with Swift Static Linux SDK musl targets. It does not add or change release artifacts, and it does not claim to fully solve the static Linux release work in #1524.

Root Cause

The CLI and CLI-shared core sources currently assume non-Darwin C APIs come from the Glibc Swift module. Swift Static Linux SDK musl targets expose libc through the Musl module instead, so #else import Glibc and qualified calls such as Glibc.exit, Glibc.signal, and Glibc.close block musl source compilation. SOCK_STREAM also differs between Glibc and Musl, where the existing SOCK_STREAM.rawValue shape is Glibc-specific.

Scope

  • Adds Darwin / Glibc / Musl import branches in CodexBarCLI and CLI-shared CodexBarCore source files.
  • Adds Musl branches for the few qualified C API calls in CLI code: exit, signal, and close.
  • Keeps the existing Darwin and Glibc branches unchanged.
  • Handles the CLI local HTTP server socket type difference by keeping SOCK_STREAM.rawValue for Glibc and using SOCK_STREAM for Musl.
  • Keeps posix_spawn_file_actions_addclosefrom_np on Glibc, and adds a small Musl fallback that enumerates /proc/self/fd and adds explicit close actions for child process fd cleanup.
  • Updates the Linux test import guard so the Linux test source can compile under either Glibc or Musl.

Explicit Non-Goals

  • No release workflow changes.
  • No release matrix or artifact changes.
  • No SQLite vendoring.
  • No SweetCookieKit changes.
  • No full static release implementation.

Validation

  • swiftformat <changed files>: passed; no final formatting changes.
  • swiftlint --strict --cache-path /private/tmp/codexbar-1524-swiftlint.cache: passed, 0 violations.
  • env HOME=/private/tmp/codexbar-1524/.home CLANG_MODULE_CACHE_PATH=/private/tmp/codexbar-1524/.clang-module-cache swift test --filter SpawnedProcessGroupTests: passed, 10 tests.
  • env HOME=/private/tmp/codexbar-1524/.home CLANG_MODULE_CACHE_PATH=/private/tmp/codexbar-1524/.clang-module-cache swift test --filter AntigravityCLISessionTests: passed, 48 tests.
  • env HOME=/private/tmp/codexbar-1524/.home CLANG_MODULE_CACHE_PATH=/private/tmp/codexbar-1524/.clang-module-cache swift test --filter CLIEntryTests: passed, 22 tests.
  • env HOME=/private/tmp/codexbar-1524/.home CLANG_MODULE_CACHE_PATH=/private/tmp/codexbar-1524/.clang-module-cache swift test --filter CLIServeRouterTests: passed, 36 tests.
  • env HOME=/private/tmp/codexbar-1524/.home CLANG_MODULE_CACHE_PATH=/private/tmp/codexbar-1524/.clang-module-cache swift test: built and ran, but failed on 5 existing environment/timing-sensitive issues outside this PR's touched code:
    • CommandCodeUsageFetcherTests: 2 cancellation grace timing assertions.
    • DeepSeekUsageFetcherTests: 1 optional summary cancellation timing assertion.
    • CodexLoginRunnerTests: 1 detached child pipe drain assertion.
    • GeminiStatusProbeAPITests: 1 fnm stdout timing assertion.

Musl Verification

Locally verified with the Swift Static Linux SDK through Swift source compilation. This is not a complete static release artifact verification: the build now reaches the link stage and stops on the expected SQLite static-link blocker, which is outside this PR scope.

Installed/used locally:

  • Swift toolchain: ~/Library/Developer/Toolchains/swift-6.2.1-RELEASE.xctoolchain (Apple Swift version 6.2.1 (swift-6.2.1-RELEASE)).
  • Swift SDK: swift-6.2.1-RELEASE_static-linux-0.0.1.

Command:

env HOME=/private/tmp/codexbar-1524/.home \
  CLANG_MODULE_CACHE_PATH=/private/tmp/codexbar-1524/.clang-module-cache \
  ~/Library/Developer/Toolchains/swift-6.2.1-RELEASE.xctoolchain/usr/bin/swift \
  build -c release --product CodexBarCLI \
  --swift-sdk swift-6.2.1-RELEASE_static-linux-0.0.1

Result:

[376/378] Compiling CodexBarCore AppGroupSupport.swift
[4/6] Compiling CodexBarCLI CLICacheCommand.swift
[5/6] Linking CodexBarCLI
ld.lld: error: unable to find library -lsqlite3

That result confirms the Darwin/glibc-safe Musl source compatibility changes compile under the Swift Static Linux SDK until the known static SQLite dependency/linking work described in #1524, which remains intentionally out of scope here.

@clawsweeper

clawsweeper Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed June 17, 2026, 2:13 PM ET / 18:13 UTC.

Summary
The PR adds Musl import and libc-call branches across Linux CLI/core sources so Swift Static Linux SDK musl source compilation can progress.

Reproducibility: Do we have a high-confidence way to reproduce the issue? Yes from source: the widened Linux test expects fd 200 to be closed, while the Musl branch skips the only close-from spawn action that closes arbitrary fds.

Review metrics: 1 noteworthy metric.

  • Files touched: 21 files, 84 additions, 29 deletions. The change spans enough CLI/core imports that process-spawn behavior needs explicit review before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Add a Musl-compatible close-from replacement or equivalent fd cleanup for spawned children.
  • Run or document a focused check that the Linux descriptor-inheritance test passes under the intended Musl SDK once linking allows tests.

Risk before merge

  • [P1] Musl child processes can inherit unrelated parent file descriptors because the Glibc close-from call is removed without a replacement.
  • [P1] The PR remains only a source-compatibility step; the broader static Linux artifact and static SQLite decisions stay in the linked issue.

Maintainer options:

  1. Restore Musl FD Isolation (recommended)
    Add a Musl-compatible way to close unrelated descriptors before spawn and keep the widened Linux descriptor test meaningful for both libc modules.
  2. Pause The Source Slice
    If maintainers do not want to choose the Musl descriptor strategy in this PR, pause or narrow it until the broader static Linux work decides the spawn contract.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Fix the Musl spawn path so AntigravityPTYProcessLauncher and SpawnedProcessGroup do not inherit unrelated parent file descriptors when `posix_spawn_file_actions_addclosefrom_np` is unavailable; keep Darwin and Glibc behavior unchanged and add/update focused Linux tests for the Musl branch where feasible.

Next step before merge

  • There is a narrow code-level blocker: restore Musl child-process fd cleanup while preserving Darwin/Glibc behavior.

Security
Needs attention: The diff introduces a concrete file-descriptor inheritance concern in the Musl child-process paths.

Review findings

  • [P2] Restore close-from behavior for Musl spawns — Sources/CodexBarCore/Providers/Antigravity/AntigravityCLISession.swift:887-889
  • [P2] Preserve descriptor cleanup in the shared spawn path — Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift:253-255
Review details

Best possible solution:

Land this source-compatibility slice only after adding a Musl-compatible descriptor-closing strategy while leaving release artifact and SQLite decisions to the broader static Linux issue.

Do we have a high-confidence way to reproduce the issue?

Do we have a high-confidence way to reproduce the issue? Yes from source: the widened Linux test expects fd 200 to be closed, while the Musl branch skips the only close-from spawn action that closes arbitrary fds.

Is this the best way to solve the issue?

Is this the best way to solve the issue? Not yet; the import shims are the right narrow direction, but the Musl spawn path needs an equivalent descriptor-closing strategy before the compatibility slice is merge-ready.

Full review comments:

  • [P2] Restore close-from behavior for Musl spawns — Sources/CodexBarCore/Providers/Antigravity/AntigravityCLISession.swift:887-889
    The PR makes the Linux descriptor test compile under Musl, but this branch now runs posix_spawn_file_actions_addclosefrom_np only for Glibc. On a Musl build the fd duplicated at 200 remains open in the child, so the test's closed assertion and the Antigravity child-process isolation contract are broken unless Musl gets an equivalent close strategy.
    Confidence: 0.88
  • [P2] Preserve descriptor cleanup in the shared spawn path — Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift:253-255
    This shared launcher also loses the broad close-from action on Musl and only closes the known pipe descriptors. Any unrelated parent fd can then leak into spawned CLI helpers, which is the behavior the recent Linux descriptor-isolation work was preventing.
    Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 05545feba362.

Label changes

Label changes:

  • add P2: This is a bounded Linux CLI compatibility improvement with a concrete child-process correctness blocker before merge.
  • add merge-risk: 🚨 security-boundary: Merging as-is could make Musl child processes inherit unrelated parent file descriptors that existing Linux code tries to close.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-change terminal output from a Swift Static Linux SDK build showing source compilation reached the known SQLite link blocker; it does not remove the need to fix the descriptor-isolation regression.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes after-change terminal output from a Swift Static Linux SDK build showing source compilation reached the known SQLite link blocker; it does not remove the need to fix the descriptor-isolation regression.

Label justifications:

  • P2: This is a bounded Linux CLI compatibility improvement with a concrete child-process correctness blocker before merge.
  • merge-risk: 🚨 security-boundary: Merging as-is could make Musl child processes inherit unrelated parent file descriptors that existing Linux code tries to close.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes after-change terminal output from a Swift Static Linux SDK build showing source compilation reached the known SQLite link blocker; it does not remove the need to fix the descriptor-isolation regression.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-change terminal output from a Swift Static Linux SDK build showing source compilation reached the known SQLite link blocker; it does not remove the need to fix the descriptor-isolation regression.
Evidence reviewed

Security concerns:

  • [medium] Musl child processes may inherit parent descriptors — Sources/CodexBarCore/Providers/Antigravity/AntigravityCLISession.swift:887
    Changing the close-from spawn actions to Glibc-only removes broad descriptor cleanup for Musl without a replacement, which can expose unrelated parent descriptors to spawned CLI children.
    Confidence: 0.84

Acceptance criteria:

  • [P1] swift test --filter AntigravityProcessLauncherLinuxTests.
  • [P1] swift test --filter CLIEntryTests.
  • [P1] swift test --filter CLIServeRouterTests.
  • [P1] make check.

What I checked:

  • Repository policy read: AGENTS.md was read fully; its guidance to keep changes small, prefer focused CLI tests, avoid live provider/Keychain validation, and run check/lint for code changes applies to this review. (AGENTS.md:1, 05545feba362)
  • Current main still has Glibc-only CLI source: Current main still has #else import Glibc and qualified Glibc calls in affected CLI/core files, so the PR is not already implemented on main. (Sources/CodexBarCLI/CLILocalHTTPServer.swift:2, 05545feba362)
  • Musl compile proof in PR body: The PR body reports a Swift Static Linux SDK build of CodexBarCLI that compiles to the link stage and stops at the known -lsqlite3 static-link blocker, matching the stated first-stage source-compatibility scope. (a7d5663d0744)
  • Musl test now promises descriptor isolation: The PR widens the Linux Antigravity process launcher test to canImport(Glibc) || canImport(Musl) while the test still duplicates fd >=200 and expects the child to report it as closed. (TestsLinux/AntigravityProcessLauncherLinuxTests.swift:1, a7d5663d0744)
  • Close-from cleanup becomes Glibc-only: The PR changes the broad posix_spawn_file_actions_addclosefrom_np cleanup from non-Darwin to Glibc-only in both Antigravity and shared process-spawn paths, leaving Musl without an equivalent cleanup path. (Sources/CodexBarCore/Providers/Antigravity/AntigravityCLISession.swift:887, a7d5663d0744)
  • Related static Linux issue remains broader: The linked issue tracks fully static Linux artifacts, artifact strategy, and static SQLite/SweetCookieKit decisions; this PR intentionally handles only source import compatibility.

Likely related people:

  • Peter Steinberger: Recent path history ties the shared spawn cleanup and Antigravity descriptor-test fixture changes to this person. (role: recent area contributor; confidence: high; commits: 3286934b8be3, 38f0592431c6; files: Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift, TestsLinux/AntigravityProcessLauncherLinuxTests.swift)
  • Ellis Nieuwpoort: Path history shows the Antigravity CLI fallback introduced the provider/session area now affected by the Musl import and spawn cleanup changes. (role: feature introducer; confidence: medium; commits: cd201b9e5035; files: Sources/CodexBarCore/Providers/Antigravity/AntigravityCLISession.swift, Sources/CodexBarCLI/CLILocalHTTPServer.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jun 17, 2026
@Yuxin-Qiao

Yuxin-Qiao commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the ClawSweeper fd-inheritance finding in a6a17e3.

  • Glibc still uses posix_spawn_file_actions_addclosefrom_np.
  • Musl now enumerates /proc/self/fd and adds explicit close actions for descriptors at/above the close-from boundary, preserving child fd cleanup without relying on the unavailable glibc _np API.
  • Darwin behavior remains unchanged.

Validation:

  • swift test --filter SpawnedProcessGroupTests passed, 10 tests.
  • swift test --filter AntigravityCLISessionTests passed, 48 tests.
  • swiftlint --strict --cache-path /private/tmp/codexbar-1524-swiftlint.cache passed, 0 violations.
  • Swift Static Linux SDK musl build still reaches the known -lsqlite3 link blocker after Swift source compilation.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

This was referenced Jun 18, 2026
Merged
Merged
@steipete steipete mentioned this pull request Jun 18, 2026
Merged
@steipete

steipete commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Landed the repaired musl source-compatibility change in d8ce8698 via #1620.

The replacement keeps your contributor credit and adds fail-closed descriptor handling plus regression coverage. This branch could not be updated because maintainer edits were disabled, so I recreated the patch locally. Thanks @Yuxin-Qiao. Enabling Allow edits by maintainers on future PRs lets us repair the original branch directly.

@steipete steipete closed this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Yuxin-Qiao @steipete