Skip to content

fix(mcp): keep unavailable value/secret scopes out of Sentry - #964

Merged
kody-bot merged 3 commits into
mainfrom
cursor/sentry-triage-7634425298-9184
Jul 26, 2026
Merged

kody-bot merged 3 commits into
mainfrom
cursor/sentry-triage-7634425298-9184

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Jul 26, 2026 •

Copy link
Copy Markdown
Member

Summary

Sentry 7634425298: value_set threw a plain Error when scope "session" was requested without a session binding (the capability defaults to session). That is a caller mistake — MCP observability should keep it on mcp-event logs, not open a platform bug issue.

Fix

  • Throw McpCallerError from getOrCreateValueBucket / getOrCreateSecretBucket when the requested scope has no binding in the current storage context (same pattern as recent job/MCP caller-error triage PRs).
  • Clarify value_set scope docs: use "user" when there is no session context.
  • Unit tests cover unavailable session/app value scopes and session/package secret scopes.

Risk

composes / low — error-class wiring only; no auth, isolation, billing, or migration surface. Intended to squash-merge after CI + AI review.

Siblings

Prefetched unresolved siblings (updateCurrentEntry, workflow timeout, search ValidationError) do not share this root cause.

System recap — composes existing primitives (low risk)

Mode: recap · Base: main @ 01399f13 · Head: 90f324af

Classification: composes — reuses the existing McpCallerError carve-out so unavailable session/app/package scopes stay off Sentry; no primitive shape or contract change.

Primitives touched

Primitive Group Impact
mcp-server surfaces composes — throw McpCallerError for unavailable value/secret scopes
values assistant composes — clarify value_set scope description

System map

value_set / secret save hit the values/secrets services; unavailable scope bindings now surface as caller errors so MCP observability skips Sentry.

Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).

flowchart LR
	valuesCap["values<br/>Values"]:::touched
	mcpServer["mcp-server<br/>MCP endpoint"]:::touched
	observability["mcp observability<br/>caller-error carve-out"]:::untouched
	valuesCap -->|"value_set scope binding"| mcpServer
	mcpServer -->|"McpCallerError skips Sentry"| observability
	classDef touched fill:#1a7f37,color:#fff
	classDef extended fill:#9a6700,color:#fff
	classDef added fill:#cf222e,color:#fff
	classDef untouched fill:#57606a,color:#fff
Loading

Change flow

sequenceDiagram
	participant Agent
	participant value_set
	participant saveValue
	participant Sentry
	Agent->>value_set: scope=session (default), no sessionId
	value_set->>saveValue: getOrCreateValueBucket
	saveValue-->>value_set: McpCallerError
	value_set-->>Agent: caller-facing error
	Note over Sentry: not captured (isMcpCallerError)
Loading
Open in Web Open in Cursor 

Summary by CodeRabbit

  • Documentation
    • Clarified storage semantics for scope: session-scoped items require an active session, while user-scoped items are available without one.
  • Bug Fixes
    • Improved handling when requested storage scope isn’t available for the current context for both values and secrets.
    • These now return a consistent McpCallerError with clear, scope-specific messages instead of a generic failure.
  • Tests
    • Added/updated unit tests to assert McpCallerError type and exact error messages for unavailable session-scope and related scenarios.

value_set defaults to scope "session", so MCP calls without a session
binding threw a plain Error and opened platform-looking Sentry issues
(7634425298). Throw McpCallerError for unavailable value and secret
scopes so observability keeps them on mcp-event logs only.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Scoped storage errors

Layer / File(s) Summary
Value storage error handling
packages/worker/src/mcp/values/service.ts, packages/worker/src/mcp/values/service.node.test.ts, packages/worker/src/mcp/capabilities/values/value-set.ts
Unavailable app or session value storage now throws McpCallerError; tests verify the exact type and message, and the scope description clarifies session and user storage.
Secret storage error handling
packages/worker/src/mcp/secrets/service.ts, packages/worker/src/mcp/secrets/service.node.test.ts
Unavailable session or package secret storage now throws McpCallerError, with tests covering missing session context.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: routing unavailable value and secret scope errors away from Sentry via McpCallerError.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/sentry-triage-7634425298-9184

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kody-bot
kody-bot marked this pull request as ready for review July 26, 2026 17:55
@github-actions

github-actions Bot commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-964.kody-a99.workers.dev

Worker: kody-pr-964
D1: kody-pr-964-db
KV: kody-pr-964-oauth-kv

Mocks:

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/worker/src/mcp/secrets/service.node.test.ts`:
- Around line 379-406: Extend the saveSecret unavailable-scope tests to cover a
null package binding alongside the existing session case. Invoke saveSecret with
scope "package" and packageId: null, then assert it rejects with McpCallerError
and the exact message 'Secret scope "package" is unavailable in this context.'
while preserving the existing session assertion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a6c4755-195e-443e-88f0-7921e9264800

📥 Commits

Reviewing files that changed from the base of the PR and between 01399f1 and f3b98c9.

📒 Files selected for processing (5)
  • packages/worker/src/mcp/capabilities/values/value-set.ts
  • packages/worker/src/mcp/secrets/service.node.test.ts
  • packages/worker/src/mcp/secrets/service.ts
  • packages/worker/src/mcp/values/service.node.test.ts
  • packages/worker/src/mcp/values/service.ts

cursoragent and others added 2 commits July 26, 2026 17:59
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Address CodeRabbit feedback on the McpCallerError carve-out.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@kody-bot
kody-bot merged commit ca18364 into main Jul 26, 2026
10 checks passed
@kody-bot
kody-bot deleted the cursor/sentry-triage-7634425298-9184 branch July 26, 2026 18:04
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.

3 participants