Refactor Claude provider planning foundations#494
Conversation
# Conflicts: # Sources/CodexBar/UsageStore.swift
ratulsarna
force-pushed
the
rat-110-claude-source-planner-consolidation
branch
from
a82e4f1 to
9565254
Compare
|
@codex review |
![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: 95652546a9
ℹ️ 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".
|
|
||
| func isAvailable(_ context: ProviderFetchContext) async -> Bool { | ||
| _ = context | ||
| return self.plannedStep.isPlausiblyAvailable |
There was a problem hiding this comment.
Returning plannedStep.isPlausiblyAvailable from ClaudePlannedFetchStrategy.isAvailable means explicit .cli mode is skipped whenever the preflight CLI probe says unavailable, so ProviderFetchPipeline never calls ClaudeCLIFetchStrategy.fetch and instead returns ProviderFetchError.noAvailableStrategy. That is a behavior regression from the previous explicit CLI path (which attempted the fetch and surfaced concrete CLI/probe errors), and it conflicts with the planner contract where explicit modes execute ordered steps regardless of availability (executionSteps is not filtered for explicit selections).
Useful? React with 👍 / 👎.
…based on source mode; add test for CLI strategy availability when planner marks it as unavailable.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f357d48aa2
ℹ️ 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".
| if let override = environment["CLAUDE_CLI_PATH"]?.trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !override.isEmpty | ||
| { | ||
| return FileManager.default.isExecutableFile(atPath: override) ? override : nil |
There was a problem hiding this comment.
When CLAUDE_CLI_PATH is set but points to a non-executable path, this branch returns nil immediately, so Claude CLI discovery stops instead of falling back to normal lookup. In environments with a stale override and a valid claude on PATH, auto planning and fetches will now incorrectly treat CLI as unavailable. This is a regression from the prior resolver path that continued through BinaryLocator/PATH-based discovery.
Useful? React with 👍 / 👎.
| if binaryName.contains("/") { | ||
| return binaryName | ||
| } | ||
| return ClaudeCLIResolver.resolvedBinaryPath(environment: environment) |
There was a problem hiding this comment.
This resolver now ignores non-path binaryName values (for example "claude-beta") and always resolves the default Claude binary, so callers that pass an alternate command name can no longer target that executable. As a result, ClaudeStatusProbe(claudeBinary: ...) may run the wrong binary unless the argument is an absolute/relative path containing /.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
# Conflicts: # Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthDelegatedRefreshCoordinator.swift # Tests/CodexBarTests/ClaudeOAuthDelegatedRefreshCoordinatorTests.swift
1 participant
Summary
This PR carries the first Claude provider refactor stack.
The main goal is to reduce Claude-provider sprawl without changing product expectations. Source planning now comes from a shared planner, plan inference and credential routing are normalized earlier at the boundaries, and the current behavior contract is characterized and documented.
Main implementation areas
ClaudeSourcePlannerand aligned descriptor, direct fetcher, debug diagnostics, and fetch-attempt reporting around it.Residual notes
ClaudeUsageFetcher(.auto)path now follows planner availability strictly. That removes the older final OAuth fallback after CLI failure, which was previously inconsistent with the declared availability probe and is now intentionally gone.