Fix codexbar cost SIGSEGV on Linux (Bundle.allBundles in isRunningTests) - #3059
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3361502320
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3361502 to
faf65fc
Compare
isRunningTests falls back to Bundle.allBundles to detect loaded .xctest bundles. On Linux (swift-corelibs-foundation) Bundle.allBundles crashes in _CFIsSwift (via CFBundleGetAllBundles -> CFArrayGetCount), so `codexbar cost` segfaults before producing output. On Linux, detect the test process from the main executable path instead: SwiftPM builds test executables with a `.xctest` suffix, so the intent survives without enumerating bundles. macOS behavior is unchanged. Fixes steipete#3058
faf65fc to
d9ce35c
Compare
|
Codex review: needs changes before merge. Reviewed August 18, 2026, 10:42 PM ET / August 19, 2026, 02:42 UTC. ClawSweeper reviewWhat this changesThe PR replaces Linux bundle enumeration in two cost-related test detectors with executable-path detection and adds Linux safety tests. Regression provenancePossible regression — probable (reproduction; failure trace). No predecessor PR is attributed. Merge readinessKeep open for one focused correction: the platform split addresses both Linux crash call paths and the PR includes real Linux CLI output, but the added regression tests never execute the new fallback because the test process has global test markers. Priority: P1 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherThe cross-platform CLI builds cost summaries from local usage and pricing data. Cost initialization and the optional OpenCodex usage-log source both check whether they are running under tests before touching user-local paths. flowchart LR
A[Linux cost command] --> B[Cost pricing initialization]
A --> C[Optional OpenCodex log setting]
B --> D[Test-process detection]
C --> D
D -->|macOS| E[Inspect loaded test bundles]
D -->|Linux| F[Inspect executable path]
E --> G[Cost JSON output]
F --> G
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Keep the two Linux-safe guards and add a deterministic sanitized-process or injectable detector test that actually reaches each fallback without accessing real user data. Do we have a high-confidence way to reproduce the issue? Yes: a Linux release CLI invocation of Is this the best way to solve the issue? Yes for the implementation: retaining macOS bundle inspection while using the executable path on Linux is the narrowest compatible repair. The regression coverage is not yet the best validation because it short-circuits before the new branch. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a32989c750e6. 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
History |
|
Deep review found one remaining Linux crash path before merge.
I recommend applying the same platform split there, preserving the existing environment and #if os(macOS)
return Bundle.allBundles.contains { $0.bundlePath.hasSuffix(".xctest") }
#else
return Bundle.main.executableURL?.path.hasSuffix(".xctest") ?? false
#endifThe current Linux CI is useful but does not prove recovery: its release smoke runs Independent read-only intent, security, reliability, and coverage reviews all reproduced this call chain. Local focused tests on this head also pass: 6 |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
codexbar costcrashes with SIGSEGV on Linux.usageandsessionsare unaffected.Root cause
CostUsageCustomPricing.isRunningTestsandOpenCodexUsageLog.isRunningTestsfall back toBundle.allBundlesto detect loaded.xctestbundles:On Linux (swift-corelibs-foundation)
Bundle.allBundlescrashes in_CFIsSwift(viaCFBundleGetAllBundles→CFArrayGetCount). Thecostcommand reaches this path during pricing initialization (CostUsageScanner.codexPricingKey→CostUsageCustomPricing.load) as well as whenopenCodexUsageLogsEnabledis true withoutOPENCODEX_HOMEset (CLICostCommand.loadOpenCodexCostPayload→OpenCodexUsageLog.usageLogURL), so it segfaults before producing output. Under test,XCTestConfigurationFilePath/SWIFT_TESTINGis already present in the environment, soisRunningTestsreturnstrueearlier — which is why CI/test runs didn't hit it, but the production binary does.Backtrace (0.53.0, release build):
Fix
Keep the
Bundle.allBundles.xctestcheck on macOS, and on Linux detect the test process from the main executable path instead across bothCostUsageCustomPricingandOpenCodexUsageModels. SwiftPM builds Linux test executables with a.xctestsuffix, so this keeps the original intent without enumerating bundles:macOS behavior is unchanged.
Verification
codexbar cost --format json --json-only→ exit 139 (SIGSEGV), no stdout.XCTestConfigurationFilePath=/tmp/x codexbar cost ...→ exit 0, valid JSON (short-circuitsisRunningTestsat the first environment check, confirming the bug is theBundle.allBundlesfallback).CodexParserHash.generated.swift(the parser-hash gate covers the editedCostUsagefile).PlatformGatingTestsforCostUsageCustomPricing.load(environment: [:])andOpenCodexUsageLog.usageLogURL(environment: [:]).Post-fix verification (patched Linux CLI)
Built locally with Swift 6.3.3 release toolchain on Linux (
x86_64) from branchfix/linux-cost-segfault(commita6b4e3401).Pure production invocation tested without any test environment flags or workarounds (no
XCTestConfigurationFilePathor testing markers set).Execution summary
codexbar 0.53.0)codexbar cost --format json --json-only --days 30139(SIGSEGV)a6b4e34)CodexBarCLI cost --format json --json-only --days 300a6b4e34)CodexBarCLI usage --format json --json-only0Patched binary version
Redacted cost output (
jqsummary)Fixes #3058