Skip to content

Conversation

@alexhancock
Copy link
Collaborator

Fixes #5259

  • Implements a new CredentialStore for goose to use to persist oauth token information on both initial auth and during token exchanges
  • Ensures new refresh tokens will always be used

More context in modelcontextprotocol/rust-sdk#542

@alexhancock alexhancock requested review from Copilot and jamadeo and removed request for Copilot November 14, 2025 18:08
[workspace.dependencies]
rmcp = { version = "0.8.5", features = ["schemars", "auth"] }
# rmcp = { version = "0.8.5", features = ["schemars", "auth"] }
rmcp = { git = "https://github.com/modelcontextprotocol/rust-sdk", features = ["schemars", "auth"], branch = "alexhancock/credential-store" }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copilot finished reviewing on behalf of alexhancock November 14, 2025 18:11
Copilot AI review requested due to automatic review settings November 14, 2025 20:28
@alexhancock alexhancock force-pushed the alexhancock/credential-store branch from 6dc7144 to d6cfc64 Compare November 14, 2025 20:28
Copilot finished reviewing on behalf of alexhancock November 14, 2025 20:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a credential store for OAuth authentication in Goose, enabling persistent storage of OAuth tokens and automatic token refresh. The implementation integrates with the rmcp SDK's new CredentialStore trait.

Key changes:

  • Implements GooseCredentialStore that uses the Config system for secure credential storage
  • Updates OAuth flow to use the credential store for loading and saving tokens
  • Adds _meta field to Tool schema for MCP protocol compliance

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/goose/src/oauth/persist.rs Implements GooseCredentialStore with async trait methods for load/save/clear operations using the Config system
crates/goose/src/oauth/mod.rs Integrates credential store into OAuth flow for token persistence and refresh
crates/goose/src/agents/extension_manager.rs Adds meta: None field to Tool construction for schema compatibility
crates/goose/Cargo.toml Updates async-trait to 0.1.89
crates/goose-server/Cargo.toml Updates async-trait to 0.1.89
crates/goose-mcp/Cargo.toml Updates async-trait to 0.1.89
crates/goose-cli/Cargo.toml Updates async-trait to 0.1.89
crates/goose-bench/Cargo.toml Updates async-trait to 0.1.89
Cargo.toml Updates rmcp to use git branch with credential store support
Cargo.lock Lock file updates for dependency changes
ui/desktop/openapi.json Adds _meta field to Tool schema
ui/desktop/src/api/types.gen.ts Generates TypeScript types for new _meta field

Comment on lines 101 to 102
// Set the credential store on the manager we got from OAuthState
// The credentials were already saved during the exchange_code_for_token call
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

This comment is incorrect. Credentials haven't been saved yet because the OAuthState on line 79 was created without a credential store (second parameter is None). The old code explicitly called save_credentials() after handle_callback(), but that was removed. To fix this, pass Some(credential_store.clone()) as the second parameter to OAuthState::new() on line 79, or manually save credentials before this point. Otherwise, credentials obtained during the OAuth flow won't be persisted.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@jamadeo jamadeo left a comment

Choose a reason for hiding this comment

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

nice!

@alexhancock alexhancock force-pushed the alexhancock/credential-store branch from d6cfc64 to 2d395a2 Compare November 14, 2025 21:29
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.

MCP OAuth refresh_token not used correctly

3 participants