feat(cli): add --check flag to goose info for provider testing#8289
Conversation
![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: e61936675d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "Auth:", | ||
| &style("ok").green().to_string(), | ||
| label_padding, | ||
| ); |
There was a problem hiding this comment.
This marks Auth: ok immediately after provider construction, before any authenticated request is made, so invalid credentials can still be reported as successful. For providers like OpenAI-compatible ones, construction can succeed without validating the key, and the first real auth check happens in fetch_supported_models(), which may then return 401 while the output still says auth is ok. Since --check is intended to validate provider setup, this is a false positive and can mislead users into thinking their auth is healthy when it is not.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52e7c8e72a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| print_aligned( | ||
| "Auth:", | ||
| &style("ok").green().to_string(), | ||
| label_padding, | ||
| ); |
There was a problem hiding this comment.
When fetch_supported_models() returns an error that does not match the string checks for 401/Authentication/Unauthorized (for example NetworkError, RateLimitExceeded, or other request failures), this branch still prints Auth: ok (and immediately after, Connection: ok). That creates a false positive for goose info --check even though the verification call failed, so users can be told their provider is healthy during outages or connectivity failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44e140d551
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Ok(model_config) => { | ||
| match goose::providers::create(&provider, model_config, Vec::new()).await { | ||
| Ok(p) => { | ||
| let elapsed = start.elapsed(); |
There was a problem hiding this comment.
The Connection: ok (verified in …) value is computed before fetch_supported_models().await, so the reported verification time excludes the credentialed network call that actually validates connectivity. In slow or degraded networks this can print a near-zero "verified" duration even though the check took much longer, which makes the new health signal inaccurate for troubleshooting.
Useful? React with 👍 / 👎.
| match goose::providers::create(&provider, model_config, Vec::new()).await { | ||
| Ok(p) => { | ||
| let elapsed = start.elapsed(); | ||
| match p.fetch_supported_models().await { |
There was a problem hiding this comment.
this sounds like the right way to do it, since it is free for most providers, but it is not. for custom providers that specify their own models and have fetching disabled, this will always work. for openrouter, this will also always work. you should probably just send an actual test message
There was a problem hiding this comment.
Good call -- switched from fetch_supported_models() to a minimal complete() call (Message::user("Say 'ok'") with no tools). This actually exercises the auth path, so it'll catch bad keys on custom providers and OpenRouter where model listing always succeeds.
Replace fetch_supported_models() with a minimal completion call (Message::user 'Say ok' with max 1 response). This catches auth failures on custom providers with fetching disabled and on OpenRouter, where model listing always succeeds regardless of credentials. Addresses review feedback from @DOsinga on PR aaif-goose#8289.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f83fc853f3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| _ => { | ||
| println!( | ||
| " {} No provider configured. Run '{}' first.", | ||
| style("⚠").yellow(), | ||
| style("goose configure").cyan() | ||
| ); | ||
| } |
There was a problem hiding this comment.
The fallback arm for match (provider_name, model_name) collapses every error into the same "No provider configured" message, so goose info --check misreports malformed/unreadable config files (or other retrieval failures) as missing setup. In those cases users are directed to reconfigure instead of seeing the real failure, which makes troubleshooting provider checks much harder. Please only use this message for actual NotFound cases and print other errors explicitly.
Useful? React with 👍 / 👎.
|
Addressed Codex findings in 818dacd:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 818dacdd08
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| print_aligned("Provider:", &provider, label_padding); | ||
| print_aligned("Model:", &model, label_padding); | ||
|
|
||
| match goose::model::ModelConfig::new(&model) { |
There was a problem hiding this comment.
goose info --check builds the probe config with ModelConfig::new(&model) but never calls .with_canonical_limits(&provider), unlike the normal agent path. That leaves max_output_tokens at the fallback from ModelConfig::max_output_tokens() (4096), and provider request builders (e.g. OpenAI/Anthropic formatters) send that value directly; models with lower output caps then fail the probe even when auth and connectivity are valid. This creates false negatives for valid configurations and makes --check less reliable than an actual session.
Useful? React with 👍 / 👎.
|
Fixed in 5d76deb - added |
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
The feature is useful and addresses real user pain. A few things to fix:
1. Match on ProviderError variants instead of string-searching
complete() returns Result<_, ProviderError>, which already has structured variants like ProviderError::Authentication(_), ProviderError::NetworkError(_), etc. The current code does:
if err_str.contains("401")
|| err_str.contains("Authentication")
|| err_str.contains("Unauthorized")This is brittle and will break if error messages change. Instead:
use goose::providers::errors::ProviderError;
match e {
ProviderError::Authentication(_) => { /* auth failed */ }
ProviderError::NetworkError(_) => { /* network issue */ }
_ => { /* other failure */ }
}2. Collapse the duplicated config-error arms
The (Err(e), _) and (_, Err(e)) branches do the same thing. You can combine them:
(Ok(provider), Ok(model)) => { /* ... */ }
_ => {
// print whichever is missing
}Or even simpler — just try to get both and bail on the first error, since you can't do anything useful without both.
3. Flatten the nesting
The happy path is 5+ levels deep. Consider extracting the provider-check logic into a helper function that returns a Result, then match on that result at the top level for display. This would make the output formatting much easier to follow.
DOsinga
added
the
needs_human
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97f28e92fe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Addressed all three items in 97f28e9:
|
|
Hi @mvanhorn, Thanks for the careful iteration here. The refactor into Two Codex comments from the last pass still look open: returning a non-zero exit code when the check fails (P1), and separating provider-init errors from auth failures (P2) could you address those (or reply why they don't apply) and rebase onto main? Happy to take another look after that. |
…n check failure Addresses the two open Codex items @lifeizhou-ap flagged on aaif-goose#8289: - Split `ProviderCheckError::ProviderCreate`: when `show_api_key_hint` is true the failure is reported as `Auth: FAILED` (missing or invalid credential), otherwise as `Provider: FAILED`. The previous code blanket-labeled every construction failure as auth, which misdirected troubleshooting when the real issue was an unknown provider or malformed provider config. - Return `Err(anyhow!(...))` from `handle_info` when the check fails so `goose info --check` exits non-zero. Automation (CI scripts, install checks, health probes) can now rely on the exit code. Verified: cargo check, cargo clippy --deny warnings, and cargo test -p goose-cli --lib (163 pass).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db1ec06e80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let model_config = provider_client.get_model_config(); | ||
| let start = std::time::Instant::now(); | ||
| provider_client | ||
| .complete(&model_config, "check", "", &[test_msg], &[]) |
There was a problem hiding this comment.
Passing the hard-coded session id "check" causes every goose info --check probe to present the same user/session identity to providers. In openrouter::stream, session_id is copied into the user field specifically for attribution/rate-limiting, so this change can create cross-user throttling and intermittent false failures even when an individual user’s credentials are valid. Generating a per-invocation id (or leaving it empty for a pre-session probe) avoids this shared-rate-limit coupling.
Useful? React with 👍 / 👎.
|
Addressed both items in db1ec06: [P1] Return non-zero status on check failure
[P2] Separate provider-init errors from auth failures
Verified: |
|
Hi @mvanhorn Thank you so much for the quick reply! There are some merge conflicts, would you please resolve it? |
…ting Adds a --check flag to `goose info` that tests the configured provider connection and reports auth status, initialization time, and available model count. On failure, prints the error with an actionable hint. Users currently discover broken provider configurations only after starting a session. This flag lets them verify setup works before committing to a session. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
The initial implementation used config.get_param("provider") which
reads from the PROVIDER env var. The correct accessors are
get_goose_provider() and get_goose_model() which read from
GOOSE_PROVIDER/GOOSE_MODEL and the config file.
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Addresses Codex review feedback: provider construction can succeed without validating credentials, so Auth: ok was a false positive. Now fetch_supported_models() runs first, and auth status reflects whether the credentialed call actually passed. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Non-auth errors (network, rate limit) from fetch_supported_models() were still printing Auth: ok. Now any verification failure shows Check: FAILED with the error message. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Replace fetch_supported_models() with a minimal completion call (Message::user 'Say ok' with max 1 response). This catches auth failures on custom providers with fetching disabled and on OpenRouter, where model listing always succeeds regardless of credentials. Addresses review feedback from @DOsinga on PR aaif-goose#8289.
Previously the catch-all arm showed a generic "No provider configured" message. Now it reports the actual config error so users can diagnose whether the provider or model key is missing or malformed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ModelConfig::new() alone uses a fallback max_output_tokens (4096) which can exceed the actual model's limit, causing false probe failures. Apply with_canonical_limits() to match the normal agent session path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract provider-check logic into a helper function, replace brittle string searching with match on ProviderError variants, and collapse duplicated config-error branches. Keeps user-facing output unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…n check failure Addresses the two open Codex items @lifeizhou-ap flagged on aaif-goose#8289: - Split `ProviderCheckError::ProviderCreate`: when `show_api_key_hint` is true the failure is reported as `Auth: FAILED` (missing or invalid credential), otherwise as `Provider: FAILED`. The previous code blanket-labeled every construction failure as auth, which misdirected troubleshooting when the real issue was an unknown provider or malformed provider config. - Return `Err(anyhow!(...))` from `handle_info` when the check fails so `goose info --check` exits non-zero. Automation (CI scripts, install checks, health probes) can now rely on the exit code. Verified: cargo check, cargo clippy --deny warnings, and cargo test -p goose-cli --lib (163 pass). Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
mvanhorn
force-pushed
the
feat/info-provider-check
branch
from
db1ec06 to
9784419
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9784419171
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .map_err(|e| { | ||
| let error = e.to_string(); | ||
| ProviderCheckError::ProviderCreate { | ||
| show_api_key_hint: error.contains("not found") || error.contains("API_KEY"), |
There was a problem hiding this comment.
The show_api_key_hint heuristic treats any provider-create error containing "not found" as an auth failure, which misclassifies non-credential setup errors as Auth: FAILED and gives the wrong remediation. Fresh evidence: Azure provider construction reads required non-auth fields (AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_DEPLOYMENT_NAME) via get_param in crates/goose/src/providers/azure.rs lines 78-79, so missing endpoint/deployment currently triggers this auth branch even when API keys are fine. This makes goose info --check diagnostics misleading and should be restricted to explicit credential-key failures.
Useful? React with 👍 / 👎.
|
Rebased onto main and force-pushed. Two conflicts: |
|
Appreciate getting --check on goose info, @lifeizhou-ap. |
3 participants
Summary
Adds a
--checkflag togoose infothat tests the configured provider connection. Reports auth status, initialization time, and available model count. On failure, prints the error with a hint to rungoose configure.Demo
Three scenes: (1)
goose infobefore this change (paths only), (2)goose info --checkwith no provider configured (warning + hint), (3)goose info --checkwith invalid credentials (auth failure correctly detected after credentialed API call).Why this matters
Users discover broken provider configurations only after starting a session. The auth error appears after the session banner has already printed, and the error message gives no guidance on how to fix it. Several issues trace back to this first-use friction:
This flag provides a quick pre-flight check without starting a full session.
Changes
Two files changed in
crates/goose-cli/:src/cli.rs(+2 lines): Addedcheck: boolfield to theInfocommand variant. Updated dispatch to passcheckand.awaitthe now-asynchandle_info.src/commands/info.rs(+93 lines): Madehandle_infoasync. When--checkis passed, reads provider/model fromConfig::get_goose_provider()/get_goose_model(), creates the provider viagoose::providers::create(), then callsfetch_supported_models()as the credentialed API call. Auth status is reported based on whether that call succeeds or returns a 401/auth error. Non-auth errors (network, rate limit) are reported separately under Models.Three output paths:
goose configurehintAuth: FAILEDwith actionable hintTesting
cargo fmt --checkpassescargo clippy -p goose-cli --all-targets -- -D warningspasses (zero warnings)cargo test -p goose-clipassesRelated Issues
Relates to #6287, #6893, #7903, #8029
This contribution was developed with AI assistance (Codex + Claude Code).