Sign up
View stargazers13Sign up

fix(server): hide browser and device tools when agent access is off#24314

open6
main
fix/issue-17202
opened1d agobyJJulius Marminge
3 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

With Agent browser access and Agent device access off, a fresh agent session still discovers preview and device tools that fail when called. Reproduced in the real web dev app with a fresh Claude session: 19 preview tools, four device tools and both preview controls remained advertised.

Change

Gate standard preview/device tools, their custom image tools and preview controls using the authenticated credential's existing capabilities at MCP registration. Discovery and calls use the same gate; hidden tools return tool-not-found. Existing handler capability checks remain in place. Other tool registrations are unchanged.

Scope and approval

Fixes https://github.com/pingdotgg/t3code/issues/17202

GitCafe: https://git.cafe/pingdotgg/t3code/issues/17202

Triaged bug via the automated issue flow. This fixes discovery for the existing browser/device access settings without adding settings, capabilities or persisted state.

Verification

  • Red first: the real HTTP tools/list regression failed without the required capabilities; the preview-controls follow-up also failed before its fix. Original red log, controls red/green.
  • After merging current main, vp test run apps/server/src/mcp/McpHttpServer.test.ts apps/server/src/mcp/McpDeviceToolkit.test.ts apps/server/src/mcp/toolkits/previewControls/handlers.test.ts: 26 passed across three files. Tests cover neither capability, preview only, device only and both, including image tools, both controls and an ordinary tool. Post-merge log.
  • CI follow-up: supplied the missing PreviewManager mock required by the preview-controls registration in the HTTP test fixture. Scoped server typecheck now exits 0; 48 focused tests across McpHttpServer, McpDeviceToolkit and PreviewAutomationBroker pass. Targeted lint/format pass. Production code is unchanged by this follow-up. Three rounds of isolated reviews found no remaining defects. Proof audit closure.
Runtime evidenceBeforeAfter
Real isolated web dev server; fresh Claude Code 2.1.303 / Opus 5.5 session; both access settings offTranscript: preview/device tools advertised, calls denied. Build 3b95e7831b.Transcript and raw offered catalog + successful ordinary call/result: no preview/device tools or controls, ordinary MCP works. Build 43104e9349.

All affected files are identical between the captured fixed build and merge head a44ddee637. Before-build affected files are identical to the original main base. Evidence index, fixture matrix.

Published head also includes the test-fixture-only service mock correction; the runtime implementation remains identical to the captured fixed build.

Visual matrix is empty: server registration and tests only, with no client render changes. No screenshot or pixel-sameness claim. Live enabled-permission sessions and every provider/transport permutation were not checked; the four capability combinations are covered over real HTTP on the shared server boundary.

Models: Claude Opus 5.5 and GPT-6.1-sol. Harness: T3 Code with Claude Code and Codex.

Checks

All checks passed

Added 5 commits

  • test(server): assert preview_snapshot and device_screenshot visibility per capability0e0ce78
  • fix(server): preview tab list and close tools follow the preview capability43104e9
  • Merge remote-tracking branch 'origin/main' into fix/issue-17202a44ddee
  • fix(server): agents only see preview and device tools when their access is ona6c3bc1
  • test(server): reproduces issue 17202, MCP lists preview and device tools without their capabilitiesf8cdae1
MmacroscopeappBot1d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR changes authenticated MCP tool visibility and availability based on credential capabilities, affecting browser and device access on the production request path. Although the implementation is focused and backed by HTTP-level tests, the authorization impact warrants human review.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot1d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire4.9 KiB5.0 KiB+23 B (+0.5%)6.8 KiB✅
CodexThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
CodexLive turn WebSocket wire1.2 KiB1.2 KiB+23 B (+1.9%)2.0 KiB✅
CodexLive turn WebSocket decoded20.8 KiB20.9 KiB+41 B (+0.2%)29.3 KiB✅
CodexLive turn messages12+1 (+100.0%)8✅
ClaudeTotal thread wire5.0 KiB5.0 KiB0 B (0.0%)6.8 KiB✅
ClaudeThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
ClaudeLive turn WebSocket wire1.2 KiB1.2 KiB0 B (0.0%)2.0 KiB✅
ClaudeLive turn WebSocket decoded21.2 KiB21.2 KiB0 B (0.0%)29.3 KiB✅
ClaudeLive turn messages220 (0.0%)8✅

Baseline: 26c3625 · PR result: a3424bf · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Added 1 commit

  • test(server): provide PreviewManager to the MCP tool discovery testa3424bf
CcoderabbitaiBot1d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 26c3625f484eadd4d5b6ca5a3dd1f8f8bd5533b4 and a3424bfae625ab19759d099a36d7a601846bb170.

External image Files selected for processing: 3
  • apps/server/src/mcp/McpDeviceToolkit.test.ts
  • apps/server/src/mcp/McpHttpServer.test.ts
  • apps/server/src/mcp/McpHttpServer.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

MCP tool registration now checks invocation capabilities for preview and device tools. HTTP integration tests cover tool visibility across four capability combinations. A device test checks the error message from a denied call.

Changes: MCP capability gating

Layer / File(s)Summary
Capability-gated registration
apps/server/src/mcp/McpHttpServer.ts		Registration helpers add capability checks to tool registrations when a capability is supplied. Preview and device standard tools and image tools require their respective capabilities.
Capability visibility tests
apps/server/src/mcp/McpHttpServer.test.ts, apps/server/src/mcp/McpDeviceToolkit.test.ts		HTTP tests check shared, preview, and device tool visibility across four capability combinations. The denied device call test now checks for a "not found" message.

Priority: External image Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant MCPClient
3 participant McpHttpServer
4 participant InvocationContext
5 MCPClient->>McpHttpServer: Authenticate and initialize session
6 MCPClient->>McpHttpServer: Request tools/list
7 McpHttpServer->>InvocationContext: Check granted capabilities
8 McpHttpServer-->>MCPClient: Return capability-eligible tools

Possibly related issues

  • pingdotgg/t3code#17202: The PR gates preview and device tools by credential capabilities, addressing the reported tool visibility when access is off.

Suggested reviewers: macroscopeapp, t3dotgg

Merge Risk: External image Minimal · up to a3424

The capability-gated tool visibility change is ready to merge after normal checks.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: hiding browser and device tools when agent access is disabled.
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.
Description checkPassedThe pull request description follows the required template. It explains the problem, implementation, scope, linked issue, regression evidence, focused test results, runtime verification, limitations, …
Full details: Description check

Explanation

The pull request description follows the required template. It explains the problem, implementation, scope, linked issue, regression evidence, focused test results, runtime verification, limitations, and agent details. The scope section does not include a direct maintainer approval comment, but it explains the triaged bug and why the change does not require new approval.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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