FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(web): only record meaningful MCP activity by brendan-kellam · Pull Request #1613 · sourcebot-dev/sourcebot · GitHub

/ sourcebot Public

fix(web): only record meaningful MCP activity - #1613

Open
brendan-kellam wants to merge 2 commits into
mainfrom
brendan/fix-SOU-2040
ConversationCommits (2)ChecksFiles changed
Open

fix(web): only record meaningful MCP activity#1613
brendan-kellam wants to merge 2 commits into
mainfrom
brendan/fix-SOU-2040

Conversation

brendan-kellam commented Aug 21, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Fixes SOU-2040

Summary

  • record user activity only for valid MCP tools/call requests
  • exclude MCP lifecycle, discovery, malformed, and session teardown traffic from activity tracking
  • prevent protocol-only requests from activating pending memberships while preserving credential usage tracking
  • add focused auth, MCP classification, and route regression coverage

Testing

  • yarn workspace @sourcebot/web vitest run src/middleware/withAuth.test.ts src/app/api/(server)/ee/mcp/route.test.ts src/ee/features/mcp/activity.test.ts
  • yarn workspace @sourcebot/web eslint ...
  • yarn workspace @sourcebot/web tsc --noEmit
  • git diff --check

Note

Medium Risk
Touches auth activity and pending-membership activation, which feed seat usage. Misclassification could under-count real usage or still activate seats on protocol-only MCP traffic.

Overview
Stops MCP handshake, discovery, malformed, and session-close traffic from counting as user activity. Only valid JSON-RPC tools/call requests (including batches) now bump last-active timestamps and activate pending memberships.

Auth wrappers gain an opt-in recordActivity flag (default true). The MCP POST path clones the body to classify the message; DELETE always skips activity. Credential last-used tracking is unchanged.

Reviewed by Cursor Bugbot for commit faa5906. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • MCP protocol messages no longer incorrectly mark users as active.
    • User activity is now recorded only for valid MCP tool calls.
    • Session closure and other non-activity MCP messages no longer update activity status.

  • Tests

    • Added coverage for tool calls, protocol messages, malformed requests, batched messages, and session closure.

This comment has been minimized.

Sign in to view

coderabbitai Bot commented Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9406e417-d091-4546-bcb6-4fc72a92dd5c

📥 Commits

Reviewing files that changed from the base of the PR and between 3f98f44 and faa5906.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • packages/web/src/app/api/(server)/ee/mcp/route.test.ts
  • packages/web/src/app/api/(server)/ee/mcp/route.ts
  • packages/web/src/ee/features/mcp/activity.test.ts
  • packages/web/src/ee/features/mcp/activity.ts
  • packages/web/src/middleware/withAuth.test.ts
  • packages/web/src/middleware/withAuth.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

The change adds conditional activity recording to authentication. MCP POST requests record activity only for tool calls. Protocol messages, malformed payloads, and DELETE requests skip activity recording. Tests cover classification, routes, and authentication behavior.

Changes

MCP activity recording

Layer / File(s) Summary
Authentication activity option
packages/web/src/middleware/withAuth.ts, packages/web/src/middleware/withAuth.test.ts		 recordActivity defaults to enabled. When disabled, authentication skips activity updates, membership activation, transactions, and Lighthouse synchronization.
MCP activity classification
packages/web/src/ee/features/mcp/activity.ts, packages/web/src/ee/features/mcp/activity.test.ts		 isMcpActivityMessage detects single and batched JSON-RPC tools/call requests and rejects other or malformed messages.
MCP route integration
packages/web/src/app/api/(server)/ee/mcp/route.ts, packages/web/src/app/api/(server)/ee/mcp/route.test.ts, CHANGELOG.md		 The POST route parses request bodies and passes the activity flag to authentication. DELETE disables activity recording. Route tests and the changelog cover the behavior.

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

Merge Risk: ⚪ Minimal · up to faa59

This change narrows activity tracking to meaningful MCP tool calls while preserving credential usage tracking; no actionable merge-blocking risk remains after normal checks and review.

🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 6 files. (1 skipped: 1 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: limiting MCP activity recording to meaningful activity.
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.
✨ Finishing Touches 💡 1 📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch brendan/fix-SOU-2040

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.

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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL