Skip to content

goose doctor#8342

Merged
DOsinga merged 2 commits into
mainfrom
goose-doctor
Apr 7, 2026
Merged

goose doctor#8342
DOsinga merged 2 commits into
mainfrom
goose-doctor

Conversation

@DOsinga
Copy link
Copy Markdown
Collaborator

@DOsinga DOsinga commented Apr 6, 2026

Summary

Introduce /doctor or goose doctor

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b600b48a23

ℹ️ 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".

Comment thread crates/goose/src/doctor.rs Outdated
log.push(format!("Checking {} / {} ...", pname, mname));
match try_create_and_test(pname, mname).await {
Ok(working) => {
agent.update_provider(working, session_id).await?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve session mode when doctor verifies provider

ensure_working_provider recreates the provider and immediately calls agent.update_provider(...) even when the current provider/model already passed the health check. Replacing the live provider instance drops provider-local session state; for stateful providers (for example, Codex tracks per-session mode internally), this can silently revert a non-default approval mode right after /doctor. Keep using the existing provider on successful checks, or reapply the session mode after swapping.

Useful? React with 👍 / 👎.

Comment thread crates/goose/src/doctor.rs Outdated
Comment on lines +45 to +46
if let Ok(content) = std::fs::read_to_string(&path) {
prompt.push_str(&format!("\nLast LLM request log:\n```\n{}\n```\n", content));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Truncate LLM request logs before adding them to /doctor prompt

run reads llm_request.0.jsonl with read_to_string and injects the entire file into the prompt. That file contains full request/response payloads and can be very large, so /doctor may exceed model context limits or generate unexpectedly high token usage before diagnostics even begin. Use a bounded tail/size cap (as done for server logs) before appending this content.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1aeb131de8

ℹ️ 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".

use crate::session::SessionBuilderConfig;

pub async fn handle_doctor() -> Result<()> {
let mut session = build_session(SessionBuilderConfig {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Let goose doctor run without preconfigured provider/model

handle_doctor builds a full session before sending /doctor, but build_session aborts in resolve_provider_and_model when GOOSE_PROVIDER or GOOSE_MODEL is missing (see crates/goose-cli/src/session/builder.rs around lines 363-376). That means goose doctor exits before the new doctor logic can diagnose or recover the exact setup failures users are likely invoking it for, so the command is ineffective for broken or incomplete configs.

Useful? React with 👍 / 👎.

Comment on lines +66 to +67
let provider_name = config.get_goose_provider().ok();
let model_name = config.get_goose_model().ok();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check the active session provider, not global config

ensure_working_provider reads provider/model only from Config::global(), so /doctor ignores the provider currently loaded in the session (for example when the user started Goose with --provider/--model overrides). In that case a healthy session can be misdiagnosed as broken, and doctor may switch the session/config to a different provider unnecessarily, which is a behavior regression tied to override-based workflows.

Useful? React with 👍 / 👎.

@DOsinga DOsinga added this pull request to the merge queue Apr 7, 2026
Merged via the queue into main with commit 0028d97 Apr 7, 2026
25 checks passed
@DOsinga DOsinga deleted the goose-doctor branch April 7, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants