fix(lib): record success events for 9 tools misusing trackMCP - #424
Merged
gaurav-singh-9227 merged 2 commits intoSep 17, 2026
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited) Review profile: ASSERTIVE Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
gaurav-singh-9227
previously approved these changes
Sep 17, 2026
…n trackMCP calls Same misuse as the other seven call sites, missed because these two were single-line calls. Sweep of src/ confirms no trackMCP call now passes config in the error slot. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
gaurav-singh-9227
approved these changes
Sep 17, 2026
gaurav-singh-9227
pushed a commit
that referenced
this pull request
Sep 17, 2026
…d createTestCasesFromFile Two tools were invisible in MCPInstrumentation: - setupBrowserStackAppAutomateTests never called trackMCP on either path. Add the standard entry (success) and catch (failure) calls. - createTestCasesFromFile called trackMCP without config, so the event went out with no Authorization header and Rails dropped it. Pass config on both paths. Tests assert both paths call trackMCP with the tool name, client info, error slot, and config, so a regression to the config-in-error-slot shape (PR #424) is caught. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Nine tools call
trackMCP(name, clientInfo, config)on the success path, dropping theerrorargument.trackMCP's signature is(toolName, clientInfo, error?, config?), so on every successful call:configlands in theerrorslot →success=falseconfigitself isundefined→ noAuthorizationheader → the event is never storedResult: 30 days of
MCPInstrumentationdata contain zerosuccess=truerows for these tools, so analytics show them failing 100% of the time (see #proj-mcp thread from Bharti onfetchBuildInsights/runTestsOnBrowserStack). Correctly instrumented tools (getFailureLogs,listSessions, …) show both outcomes in the same window.Affected call sites:
src/tools/build-insights.ts—fetchBuildInsightssrc/tools/bstack-sdk.ts—setupBrowserStackAutomateTestssrc/tools/percy-sdk.ts— all seven Percy tools (including the single-linelistTestFilesandrunPercyScancalls flagged in review)Changes
undefinedexplicitly as theerrorargument at the nine success-path call sites.runTestsOnBrowserStack→setupBrowserStackAutomateTestsVisualTestIntegrationAgent→percyVisualTestIntegrationAgentsetupPercyVisualTesting→expandPercyVisualTestingNo behavioural change for end users; telemetry only.
Heads-up for analytics
Dashboards/sheets keyed on the three old names will see the new names from the release that ships this. Historic success counts for these tools cannot be backfilled.
Test plan
npm run build(lint + format + test + tsc) green on Node 22: 47 files / 649 teststrackMCP(calls passconfigin the third positionhandleMCPErrorname for every tool in the three files🤖 Generated with Claude Code