Sign up
View stargazersSign up

fix(server): refresh MCP credentials only from trusted lifecycle#30589

open2
main
GitHub fork: draft/mcp-credential-liveness
opened6d agobyMmahruskazi
2 files changed
View on GitHubTwo-way sync enabledPR last imported 5h ago

Problem

Resolving an MCP credential refreshes its liveness, so client requests can keep it valid after trusted provider activity stops. Expected behavior is for only trusted provider lifecycle refreshes to extend that window.

Change

Make credential resolution read-only with respect to liveness. Retain trusted refresh and revocation behavior.

Scope and approval

This is a small, focused correction to the existing credential-liveness contract. No new capability or default is introduced. No prior maintainer approval is claimed. Submitted as a draft for review.

Verification

The recorded regression fails before the fix and passes afterward: a request at 99ms no longer keeps a 100ms credential valid at 101ms. Trusted refresh and revocation tests continue to pass. The prepared candidate passed vp run check:changed with 10 tests across 2 files. This publication preserves the tested tree; repo-wide CI has not been run locally. Final independent review is pending.

Models: GPT-6.1 Sol and Claude Opus 5.5. Harnesses: Codex and Claude Code in T3 Code.

Checks

21 successful · 8 skipped
All checks passed
bade789

Added 2 commits

  • fix(server): refresh MCP credentials only from trusted lifecycleccf28ab
  • Merge branch 'main' into draft/mcp-credential-livenessbade789
CcoderabbitaiBot4d ago
Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Valid token resolution no longer refreshes the credential’s liveness timestamp. The expiration test checks that the token still resolves after 99 ms and expires after 101 ms.

Changes: MCP credential liveness

Layer / File(s)Summary
Credential resolution and expiration test
apps/server/src/mcp/McpSessionRegistry.ts, apps/server/src/mcp/McpSessionRegistry.test.ts		Valid token resolution returns the recorded scope without updating lastAliveAt. The test checks resolution before the liveness window expires and expiration after it.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#16184: This change affects when requests can extend the liveness window, but it does not make retry IDs stable across credentials.

Possibly related PRs

  • pingdotgg/t3code#26352: Both changes stop McpSessionRegistry.resolve from refreshing lastAliveAt and test expiration after token resolution.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to bade7

Credential reads no longer extend validity, while active provider sessions retain trusted refreshes. No merge-blocking risk was identified.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly describes the change to MCP credential liveness.
Description checkPassedThe description covers the problem, change, scope, verification results, and agent details.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between 25b53299b75d4d8d1d0667ad47e9cfed3749776e and bade789580b108fa9f3a7f680fd4d64a4fc589a4.

External image Files selected for processing: 2

  • apps/server/src/mcp/McpSessionRegistry.test.ts
  • apps/server/src/mcp/McpSessionRegistry.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.