Skip to content

Stabilize MCP refresh regression tests - #1772

Merged
henrypark133 merged 1 commit into
stagingfrom
fix/mcp-refresh-postmerge-tests
Mar 31, 2026
Merged

henrypark133 merged 1 commit into
stagingfrom
fix/mcp-refresh-postmerge-tests

Conversation

@henrypark133

Copy link
Copy Markdown
Collaborator

Summary

  • make the MCP proxy-refresh regression tests deterministic by isolating them from shared OAuth proxy env state
  • use a local token URL in the proxy-configured MCP refresh test and remove allocator-sensitive lock pointer assertions
  • align the proxy refresh mock response with the token metadata assertions for token_type and scope

Testing

  • env CARGO_TARGET_DIR=/tmp/ironclaw-postmerge-test cargo test --no-default-features --features libsql test_refresh_ --lib

Copilot AI review requested due to automatic review settings March 31, 2026 01:22
@github-actions github-actions Bot added scope: channel/cli TUI / CLI channel scope: tool/mcp MCP client size: S 10-49 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Stabilizes MCP OAuth proxy refresh regression tests by removing dependency on shared process env state and aligning mocked proxy responses with token metadata expectations.

Changes:

  • Make proxy-refresh regression tests deterministic by guarding and resetting OAuth proxy-related env vars during tests.
  • Use a local, dynamically generated token_url for the proxy-configured MCP refresh test.
  • Replace allocator-sensitive lock pointer assertions with Weak/Arc::ptr_eq-based assertions and align mock proxy responses to include token_type and scope.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/tools/mcp/auth.rs Makes MCP refresh/proxy tests deterministic (env isolation, local token URL) and makes refresh-lock tests robust by avoiding pointer-based assertions.
src/cli/oauth_defaults.rs Updates mock proxy token responses to include token_type and scope to match token metadata assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the test suite for OAuth and MCP authentication. It updates mock JSON responses to include token_type and scope, replaces hardcoded test URLs with dynamic values, and improves concurrency tests by implementing environment variable guards and more precise Arc pointer comparisons. I have no feedback to provide.

@henrypark133
henrypark133 merged commit ffff743 into staging Mar 31, 2026
18 checks passed
@henrypark133
henrypark133 deleted the fix/mcp-refresh-postmerge-tests branch March 31, 2026 01:43
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/cli TUI / CLI channel scope: tool/mcp MCP client size: S 10-49 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants