feat(replay vision): add MCP get tools#60127
Merged
Merged
Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
MCP UI Apps size report
|
Contributor
|
Size Change: 0 B Total Size: 80.2 MB ℹ️ View Unchanged
|
ksvat
force-pushed
the
replay-vision-mcp-get-tools
branch
3 times, most recently
from
6bccc8b to
5150414
Compare
ksvat
requested review from
a team,
TueHaulund,
arnohillen,
fasyy612 and
nicowaltz
and removed request for
a team
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR modifies OAuth scope visibility in posthog/scopes.py (removing replay_scanner from OAUTH_HIDDEN_SCOPE_OBJECTS), which is an auth/authorization contract change that triggered the deny-list gate. It has zero reviews and the scope change has security implications for what OAuth clients can discover and request. A human reviewer needs to sign off on this.
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
services/mcp/src/tools/generated/replay_vision.ts:113-116
The `session_id` check is superfluous: the Zod schema merges `VisionScannersObserveCreateBody` whose `session_id` field is required (not `.optional()`), so `params.session_id` is guaranteed to be a string by the time the handler executes. The conditional never evaluates to `false` and misleads readers into thinking the field might be absent.
```suggestion
const body: Record<string, unknown> = {
session_id: params.session_id,
}
```
Reviews (1): Last reviewed commit: "chore: update OpenAPI generated types" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+113
to
+116
| const body: Record<string, unknown> = {} | ||
| if (params.session_id !== undefined) { | ||
| body['session_id'] = params.session_id | ||
| } |
Contributor
There was a problem hiding this comment.
session_id check is superfluous: the Zod schema merges VisionScannersObserveCreateBody whose session_id field is required (not .optional()), so params.session_id is guaranteed to be a string by the time the handler executes. The conditional never evaluates to false and misleads readers into thinking the field might be absent.
Suggested change
| const body: Record<string, unknown> = {} | |
| if (params.session_id !== undefined) { | |
| body['session_id'] = params.session_id | |
| } | |
| const body: Record<string, unknown> = { | |
| session_id: params.session_id, | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/src/tools/generated/replay_vision.ts
Line: 113-116
Comment:
The `session_id` check is superfluous: the Zod schema merges `VisionScannersObserveCreateBody` whose `session_id` field is required (not `.optional()`), so `params.session_id` is guaranteed to be a string by the time the handler executes. The conditional never evaluates to `false` and misleads readers into thinking the field might be absent.
```suggestion
const body: Record<string, unknown> = {
session_id: params.session_id,
}
```
How can I resolve this? If you propose a fix, please make it concise.![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
ksvat
force-pushed
the
replay-vision-mcp-get-tools
branch
from
5150414 to
611da60
Compare
TueHaulund
approved these changes
May 27, 2026
Contributor
TueHaulund
left a comment
TueHaulund
left a comment
There was a problem hiding this comment.
LGTM - maybe check the "AI consent gate" issue raised by one of the bots though
ksvat
force-pushed
the
replay-vision-mcp-get-tools
branch
from
fd98823 to
2a5b37e
Compare
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
2 participants
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.
Problem
Changes
Add round of MCP tools for simple get/list tools for Vision/Scanners
How did you test this code?
Claude Desktop
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Publish to changelog?
Docs update
🤖 Agent context