Repository navigation
Conversation
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 5:16 PM ET / 21:16 UTC. Summary Reproducibility: yes. The owner reports a current-main black-box control where a logged-out fake Claude reached the interactive path and timed out, while the repaired release CLI exited promptly after only the authentication-status probe; source inspection matches both paths. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Retain the shared non-interactive authentication preflight for all Claude CLI runtime usage, with regression coverage proving logged-out rejection and authenticated success. Do we have a high-confidence way to reproduce the issue? Yes. The owner reports a current-main black-box control where a logged-out fake Claude reached the interactive path and timed out, while the repaired release CLI exited promptly after only the authentication-status probe; source inspection matches both paths. Is this the best way to solve the issue? Yes. Reusing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 68c3c760e1ef. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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
|
8199f2d to
6374107
Compare
|
Maintainer repair complete at Policy decision: CodexBar CLI runtime now intentionally fails closed when Claude cannot prove authentication through Changes beyond the original head:
Proof:
Thanks @BearHuddleston! |
clawsweeper
Bot
added
rating: 🦞 diamond lobster
![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: 78d0cb9808
ℹ️ 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".
| context.sourceMode == .auto && | ||
| ProviderInteractionContext.current == .background) | ||
| guard requiresAuthPreflight else { return true } | ||
| guard let binary = ClaudeCLIResolver.resolvedBinaryPath(environment: context.env) else { return false } |
There was a problem hiding this comment.
When a CLI invocation explicitly selects Claude CLI and claude is not installed, this guard now marks the only strategy unavailable, so ProviderFetchPipeline returns the generic ProviderFetchError.noAvailableStrategy and the CLI exits with the default failure code. Before this change the strategy stayed available and loadViaPTY/loadViaDirectCLI threw ClaudeUsageError.claudeNotInstalled, which includes the install URL and maps to exit code 2; keep explicit CLI requests available when the binary is missing so users still get the actionable installer error while using the auth preflight once a binary is present.
Useful? React with 👍 / 👎.
|
Landed via Final proof on exact head
|
* fix: guard logged-out Claude CLI usage * docs: note logged-out Claude CLI guard * test: authenticate Claude CLI deadline fixture * test: isolate Claude CLI platform fixture --------- Co-authored-by: Peter Steinberger <[email protected]>
Summary
auth status --jsonpreflight for CodexBar CLI runtime in both Auto and explicit CLI source modesRoot cause
#1742 added a fail-closed auth preflight for native-app background Auto refreshes, but intentionally left CLI runtime unchanged. Two conditions kept the command-line path unguarded:
ClaudeCLIFetchStrategy.isAvailableonly preflighted app/background/Auto.ClaudePlannedFetchStrategyreturnedtruefor CLI-runtime planned CLI steps without calling the base strategy's availability check.On Linux with Claude Code 2.1.207 logged out (
loggedIn: false,authMethod: none), CodexBar 0.42.0 therefore started the interactiveclaude --allowed-toolssession and waited through usage-probe retries instead of returning promptly.This change fails closed when CLI runtime cannot prove Claude is logged in. Native-app explicit CLI selection and user-initiated app behavior remain unchanged. Older or wrapped Claude CLIs that do not support a usable
auth status --jsonresponse will now be unavailable to CodexBar CLI runtime rather than launching an interactive process.Validation
swift test --filter PlatformGatingTestsreached the interactive fetch override, recorded the CLI strategy available, and issued no auth-status commandswift test --filter PlatformGatingTests— 9 tests passed, including Auto and explicit CLI logged-out cases./Scripts/lint.sh lint-linux— portable checks passed; SwiftLint found 0 violations in 1,374 filesswift test --parallel— 121 tests passed across 19 suitesswift build -c release --product CodexBarCLI --static-swift-stdlibauth status --jsoncalls and no interactive probe markergit diff --checkNo UI changes; screenshots are not applicable.