Fix CLI resource loading through symlinks - #2890
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
@clawsweeper review |
|
@codex review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 12, 2026, 12:16 PM ET / 16:16 UTC. ClawSweeper reviewWhat this changesThe PR resolves the physical CLI executable before finding adjacent provider-plugin resources and adds symlink coverage to portable tests, packaged-app smoke checks, and release artifacts. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open for normal owner review: the focused resolver change correctly addresses the linked Homebrew-style symlink failure, with no blocking defect found. Active release-artifact jobs should complete on the current head before merge. Priority: P1 Review scores
Verification
How this fits togetherCodexBar’s CLI loads bundled provider-plugin resources from alongside its packaged helper executable. A Homebrew-style symlink must resolve to that physical helper before the CLI can load those resources and serve provider commands. flowchart LR
A[CLI invocation] --> B[Direct path or symlink]
B --> C[Resolve physical executable]
C --> D[Find adjacent resource bundle]
D --> E[Load provider plugins]
E --> F[CLI provider command]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Land the physical-executable lookup with its portable and packaged-artifact regression coverage once the current release matrix validates the exact head. Do we have a high-confidence way to reproduce the issue? Yes—current-main source establishes the old lookup boundary, and the branch’s portable plus packaged-helper smoke paths exercise the direct-versus-symlink case without provider credentials. Is this the best way to solve the issue? Yes—the physical executable path is the narrow, package-layout-stable anchor for adjacent resources, while preserving app-bundle precedence and existing fallback behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c4ed34d0e44a. LabelsLabel changes:
Label 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
History |
clawsweeper
Bot
added
merge-risk: 🚨 automation
|
CI follow-up on run https://github.com/steipete/CodexBar/actions/runs/31613541949:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
The non-publishing release CLI artifact matrix is green on exact head https://github.com/steipete/CodexBar/actions/runs/31618914017 Passed artifact jobs:
Each package job exercised both the physical @clawsweeper re-review |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: Fix CLI resource loading through symlinks This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
Landed, released, and verified. Behavior:
Proof:
Merged as Release verification:
|
1 participant
Summary
Fixes #2889
Root cause
The documented app/Homebrew CLI install invokes
CodexBarCLIthrough/usr/local/bin/codexbaror/opt/homebrew/bin/codexbar. Outside an app bundle, the resource resolver treated the uncanonicalized bundle/invocation location as the executable-adjacent directory. A symlink living outsideContents/Helperstherefore could not see the physical helper's adjacentCodexBar_CodexBarCore.bundle, so bundled JavaScript providers failed cleanly instead of loading.Implementation
CodexBarCoreResourcesnow derives the running executable URL fromBundle.executableURL, Darwin_NSGetExecutablePath, or Linux/proc/self/exe, resolves symlinks, and probes the physical parent directory. App resource lookup still runs first; missing resources still returnnil;.bundle,.resources, and the guarded development fallback remain supported.The packaged-app smoke now invokes the helper both directly and through a symlink located outside
Contents/Helperswhile all checkout reads are denied. The release CLI workflow also exercises bothCodexBarCLIand the packagedcodexbarsymlink for every macOS, glibc Linux, and static musl artifact.Validation
.bundleand.resourcesdirectoriesbash -n Scripts/verify_packaged_app_launch.shmake check: passedmake test: 841 selections across 71 groups passed with no retries or timeoutscodesign --verify --deep --strict --verbose=2 CodexBar.appspctl --assess --type execute --verbose=4 CodexBar.appThe package is signed by
Developer ID Application: Peter Steinberger (Y5PE65HELJ)and accepted by Gatekeeper. No provider credentials or network access are required for the proof.