Skip to content

feat(slack): add slack-mcp-v2 flag + align mcp-v2/cli/sdk surface#4197

Merged
saddlepaddle merged 3 commits intomainfrom
various-matrix
May 7, 2026
Merged

feat(slack): add slack-mcp-v2 flag + align mcp-v2/cli/sdk surface#4197
saddlepaddle merged 3 commits intomainfrom
various-matrix

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented May 7, 2026

Summary

  • Slack agent feature flag — new slack-mcp-v2 PostHog flag (evaluated per-organization) routes the Slack agent to @superset/mcp-v2 instead of @superset/mcp. Off → v1, on → v2. Two MCP clients constructed in mcp-clients.ts; the agent picks one in runSlackAgent.
  • mcp-v2 is now tRPC-only — refactored both new tools (organization_members_list, tasks_statuses_list) to call createMcpCaller(ctx). No more direct DB imports. Matches how every other v2 tool works (tasks_*, workspaces_*, etc).
  • Nested API alignment across tRPC + mcp-v2 + CLI + SDK so the four surfaces share one shape:
    • organization.members.list/add/removeadd/remove are thin wrappers over the existing organization.addMember/removeMember logic (kept in place for the existing dashboard callers).
    • task.statuses.list — new sub-router on taskRouter.

New surface

Layer Path
tRPC organization.members.{list,add,remove}, task.statuses.list
mcp-v2 tools organization_members_list, tasks_statuses_list
CLI superset organization members list, superset tasks statuses list
SDK client.organization.members.{list,add,remove}, client.tasks.statuses.list

Slack agent specifics

  • v2 path preloads context via organization_members_list + tasks_statuses_list + hosts_list (matches v1's list_members/list_task_statuses/list_devices).
  • getActionFromToolResult handles both wire shapes (v1 { created/updated/deleted }, v2 { task, txid } / { workspace, ... }).
  • TOOL_PROGRESS_STATUS and a per-version deny list cover both naming conventions.
  • mcp-v2 in-memory client now accepts an onToolCall so PostHog mcp_tool_called events fire on the v2 Slack path too.

Test plan

  • Lint + typecheck pass (verified: bun run lint clean, bun run typecheck 29/29, trpc bun test 44 pass).
  • With flag off: Slack @mentions still work end-to-end (v1 path, no behavior change expected).
  • Flip slack-mcp-v2 on for one test org in PostHog → @mention should still work, with mcp_server: superset-v2 properties on mcp_tool_called events.
  • CLI smoke: superset organization members list, superset tasks statuses list.
  • SDK smoke: client.organization.members.list() and client.tasks.statuses.list() against staging.

Summary by cubic

Adds a per-user slack-mcp-v2 flag to route the Slack agent to @superset/mcp-v2, and aligns nested APIs across tRPC/mcp‑v2/CLI/SDK for organization members (list-only) and task statuses. Default stays on v1 for a safe rollout with consistent tooling and telemetry.

  • New Features

    • Route Slack via PostHog slack-mcp-v2 (person-level; off → v1, on → v2).
    • @superset/mcp-v2 is tRPC‑only; adds tools organization_members_list and tasks_statuses_list; in‑memory client emits mcp_tool_called.
    • Slack agent supports v1+v2 result shapes, preloads members/statuses/hosts, and uses versioned deny lists and progress labels.
    • Unified nested API: tRPC organization.members.list and task.statuses.list; CLI superset organization members list and superset tasks statuses list; SDK client.organization.members.list() (read‑only) and client.tasks.statuses.list().
  • Migration

    • No breaking changes. To try v2 in Slack, enable slack-mcp-v2 for a test user in PostHog. Organization member add/remove remain app-only.

Written for commit 4e397f4. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Feature-flagged Slack MCP V2 support for agents
    • CLI commands to list organization members and task statuses
    • SDK now exposes organization membership and task-status APIs
    • New API endpoints to list organization members and to list task statuses
  • Enhancements

    • MCP v2 tools for listing members and task statuses
    • Added feature flag to toggle Slack MCP V2 behavior

Routes the Slack agent to the v2 MCP server via per-organization
PostHog flag (`slack-mcp-v2`); off keeps existing v1 path. Adds
nested `organization.members` and `task.statuses` to tRPC, the
mcp-v2 server (now tRPC-only — no DB), the CLI, and the SDK so all
four surfaces share the same shape.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ffd186af-6206-4721-a173-ed4b598055ab

📥 Commits

Reviewing files that changed from the base of the PR and between 9c14d26 and 4e397f4.

📒 Files selected for processing (4)
  • packages/sdk/src/client.ts
  • packages/sdk/src/resources/index.ts
  • packages/sdk/src/resources/organization.ts
  • packages/trpc/src/router/organization/members.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/trpc/src/router/organization/members.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/sdk/src/resources/index.ts
  • packages/sdk/src/resources/organization.ts

📝 Walkthrough

Walkthrough

Adds MCP v2 support for Slack agents (feature-flagged), new SDK Organization/Members and TaskStatuses resources, tRPC routers and procedures, MCP v2 tool registrations, in-memory client option plumbing, CLI list commands, and branching logic in the Slack agent to select V1 vs V2 flows.

Changes

MCP V2 Slack Agent with Organization & Task Status APIs

Layer / File(s) Summary
Feature Flag
packages/shared/src/constants.ts
New SLACK_MCP_V2 feature flag controls per-organization routing between V1 and V2 MCP servers.
SDK Resource Classes
packages/sdk/src/resources/organization.ts, packages/sdk/src/resources/tasks.ts
Introduces Members API resource with list and Organization resource wiring; adds TaskStatuses resource with list and related DTO types.
Backend tRPC Routers
packages/trpc/src/router/organization/members.ts, packages/trpc/src/router/task/statuses.ts, packages/trpc/src/router/organization/organization.ts, packages/trpc/src/router/task/task.ts
Adds protected list procedures for organization members and for task statuses, and wires members/statuses sub-routers into organization/task routers.
SDK Client Wiring
packages/sdk/src/client.ts, packages/sdk/src/resources/index.ts
Wires organization and tasks.statuses into Superset client and re-exports organization/member and task status types.
MCP V2 Tool Registration
packages/mcp-v2/src/tools/organization/members/list.ts, packages/mcp-v2/src/tools/tasks/statuses/list.ts, packages/mcp-v2/src/tools/register.ts
Registers organization_members_list and tasks_statuses_list MCP tools with Zod schemas and handlers; adds registrars.
MCP In-Memory Client Configuration
packages/mcp-v2/src/in-memory.ts
Adds optional onToolCall to InMemoryClientOptions, forwards it to createMcpServer, and removes transport send-patching.
Slack Agent V1/V2 Branching
apps/api/src/app/api/integrations/slack/events/utils/run-agent/mcp-clients.ts, apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts
Adds createSupersetMcpV2Client, updates V1 factory use, fetches SLACK_MCP_V2 via PostHog in runSlackAgent to choose client/context/tools, extends tool result parsing and denied-tool sets for v2, and records V2 analytics.
CLI Commands
packages/cli/src/commands/organization/members/meta.ts, packages/cli/src/commands/organization/members/list/command.ts, packages/cli/src/commands/tasks/statuses/meta.ts, packages/cli/src/commands/tasks/statuses/list/command.ts
Adds CLI commands and metadata to list organization members and task statuses with options and table output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops excitedly through the code
A V2 pathway springs to life,
With members and statuses in sight,
Feature flags guide the sleepy Slack,
New tools hop along the dual-track,
Hop-hop-hooray for this upgrade bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.08% 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
Title check ✅ Passed The title clearly describes the main changes: adding a slack-mcp-v2 feature flag and aligning the mcp-v2/CLI/SDK surface across the system.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering summary, related changes, new surface alignment, slack agent specifics, and a detailed test plan with checklist items.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 various-matrix

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 and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR adds a slack-mcp-v2 PostHog feature flag to route the Slack agent between the existing mcp-v1 and the new mcp-v2 server, and aligns the tRPC, mcp-v2, CLI, and SDK surfaces around organization.members.{list,add,remove} and task.statuses.list.

  • Slack agent flag routing: runSlackAgent checks FEATURE_FLAGS.SLACK_MCP_V2 per-org; on error it safely falls back to v1. Both fetchAgentContext and getActionFromToolResult branch on useV2 so v1 behavior is fully preserved.
  • New tRPC sub-routers: organizationMembersRouter (list/add/remove) and taskStatusesRouter (list) are wired into the existing organizationRouter and taskRouter; both new routers use requireActiveOrgMembership for authorization.
  • mcp-v2 tools + SDK resources: organization_members_list and tasks_statuses_list tools call createMcpCaller(ctx), matching the rest of mcp-v2. New Organization and TaskStatuses SDK resources expose the same surface to external consumers.

Confidence Score: 4/5

Safe to merge after fixing the misleading error message in the remove mutation.

The flag routing, context preloading, deny-lists, and SDK types are all correct; v1 is fully preserved as the fallback. The sole defect is in the new organization.members.remove mutation: when the sole owner tries to self-remove, the isTargetSelf guard fires before the last-owner guard, so the user sees a misleading error instead of the actionable one.

packages/trpc/src/router/organization/members.ts — specifically the error-branch ordering in the remove mutation.

Important Files Changed

Filename Overview
packages/trpc/src/router/organization/members.ts New tRPC sub-router for organization member management; contains an error-message ordering bug where sole-owner self-removal shows a misleading error, and an API surface inconsistency where list uses the active org but add/remove accept an explicit organizationId.
apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts Adds PostHog flag check to route the Slack agent to mcp-v2, with correct safe fallback to v1 on error; adds v2 tool name mappings, dual deny-lists, and v2/v1 response parsing for hosts. Logic is sound.
apps/api/src/app/api/integrations/slack/events/utils/run-agent/mcp-clients.ts Adds createSupersetMcpV2Client alongside the existing v1 client; wires onToolCall for PostHog analytics. Clean separation, no issues.
packages/trpc/src/router/task/statuses.ts New tRPC sub-router returning task statuses for the active org; correctly uses requireActiveOrgMembership and orders by position. No issues.
packages/mcp-v2/src/tools/organization/members/list.ts New mcp-v2 tool wrapping the tRPC organization.members.list via createMcpCaller; returns { members: rows } compatible with fetchAgentContext's parsing. No issues.
packages/mcp-v2/src/tools/tasks/statuses/list.ts New mcp-v2 tool for listing task statuses; no inputSchema is intentional (consistent with hosts_list), as defineTool defaults to {}. Returns { statuses: rows }. No issues.
packages/mcp-v2/src/in-memory.ts Adds optional onToolCall emitter param to InMemoryClientOptions and threads it into createMcpServer. Minimal, safe change.
packages/sdk/src/resources/organization.ts New SDK resource for organization.members with list, add, remove methods; types are well-defined and match the tRPC schema. No issues.
packages/sdk/src/resources/tasks.ts Adds TaskStatuses nested resource and TaskStatus/TaskStatusListResponse types to the Tasks SDK resource. No issues.
packages/shared/src/constants.ts Adds SLACK_MCP_V2: "slack-mcp-v2" to FEATURE_FLAGS. Straightforward addition.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Slack @mention -> runSlackAgent] --> B{PostHog: slack-mcp-v2 flag}
    B -- off/error --> C[createSupersetMcpClient v1]
    B -- on --> D[createSupersetMcpV2Client v2]
    C --> E[fetchAgentContext useV2=false]
    D --> F[fetchAgentContext useV2=true]
    E --> G[list_members / list_task_statuses / list_devices]
    F --> H[organization_members_list / tasks_statuses_list / hosts_list]
    G --> I[Agent loop - DENIED_SUPERSET_TOOLS_V1 filtered out]
    H --> J[Agent loop - DENIED_SUPERSET_TOOLS_V2 filtered out]
    I --> K[getActionFromToolResult - v1 wire shape]
    J --> L[getActionFromToolResult - v2 wire shape]
    K --> M[SlackBlocks / PostHog events]
    L --> M
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/trpc/src/router/organization/members.ts:107-124
**Last-owner self-removal shows wrong error message**

The `isTargetSelf` guard (line 108) fires before the last-owner guard (line 114). When a sole owner tries to remove themselves, both conditions are true and the `isTargetSelf` branch short-circuits first, surfacing "Cannot remove yourself" instead of the far more actionable "Cannot remove the last owner. Transfer ownership first." The last-owner message is effectively unreachable for self-removal cases. Fix by evaluating the last-owner condition first.

### Issue 2 of 2
packages/trpc/src/router/organization/members.ts:47-65
**`add` and `remove` accept caller-supplied `organizationId`; `list` uses active org**

`list` resolves the org via `requireActiveOrgMembership(ctx)` (session's active org), while `add` and `remove` accept an explicit `organizationId` in their input. SDK callers who call `organization.members.list()` and then `organization.members.add({ organizationId: someOtherOrg, ... })` will silently operate on different organizations. Consider either (a) making `add`/`remove` also default to the active org, or (b) adding an `organizationId` parameter to `list` so all three are consistent.

Reviews (1): Last reviewed commit: "feat(slack): add slack-mcp-v2 flag + ali..." | Re-trigger Greptile

Comment on lines +107 to +124
if (!canRemove) {
if (isTargetSelf) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Cannot remove yourself",
});
}
if (targetMember.role === "owner" && ownerCount === 1) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Cannot remove the last owner. Transfer ownership first.",
});
}
throw new TRPCError({
code: "FORBIDDEN",
message: "You don't have permission to remove this member",
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Last-owner self-removal shows wrong error message

The isTargetSelf guard (line 108) fires before the last-owner guard (line 114). When a sole owner tries to remove themselves, both conditions are true and the isTargetSelf branch short-circuits first, surfacing "Cannot remove yourself" instead of the far more actionable "Cannot remove the last owner. Transfer ownership first." The last-owner message is effectively unreachable for self-removal cases. Fix by evaluating the last-owner condition first.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/organization/members.ts
Line: 107-124

Comment:
**Last-owner self-removal shows wrong error message**

The `isTargetSelf` guard (line 108) fires before the last-owner guard (line 114). When a sole owner tries to remove themselves, both conditions are true and the `isTargetSelf` branch short-circuits first, surfacing "Cannot remove yourself" instead of the far more actionable "Cannot remove the last owner. Transfer ownership first." The last-owner message is effectively unreachable for self-removal cases. Fix by evaluating the last-owner condition first.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +47 to +65
add: protectedProcedure
.input(
z.object({
organizationId: z.string().uuid(),
userId: z.string().uuid(),
role: z.enum(["member", "admin", "owner"]).default("member"),
}),
)
.mutation(async ({ ctx, input }) => {
await verifyOrgAdmin(ctx.session.user.id, input.organizationId);
return ctx.auth.api.addMember({
body: {
organizationId: input.organizationId,
userId: input.userId,
role: input.role,
},
headers: ctx.headers,
});
}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 add and remove accept caller-supplied organizationId; list uses active org

list resolves the org via requireActiveOrgMembership(ctx) (session's active org), while add and remove accept an explicit organizationId in their input. SDK callers who call organization.members.list() and then organization.members.add({ organizationId: someOtherOrg, ... }) will silently operate on different organizations. Consider either (a) making add/remove also default to the active org, or (b) adding an organizationId parameter to list so all three are consistent.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/organization/members.ts
Line: 47-65

Comment:
**`add` and `remove` accept caller-supplied `organizationId`; `list` uses active org**

`list` resolves the org via `requireActiveOrgMembership(ctx)` (session's active org), while `add` and `remove` accept an explicit `organizationId` in their input. SDK callers who call `organization.members.list()` and then `organization.members.add({ organizationId: someOtherOrg, ... })` will silently operate on different organizations. Consider either (a) making `add`/`remove` also default to the active org, or (b) adding an `organizationId` parameter to `list` so all three are consistent.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/trpc/src/router/organization/members.ts`:
- Around line 107-124: The error handling in the remove flow currently checks
isTargetSelf before verifying the last-owner condition, causing the "Cannot
remove yourself" message to mask the more actionable "Cannot remove the last
owner" when the sole owner tries to remove themselves; update the logic in the
remove handler (the block that examines canRemove, isTargetSelf, targetMember,
and ownerCount) to check the last-owner condition (targetMember.role === "owner"
&& ownerCount === 1) before the isTargetSelf check and throw the TRPCError with
message "Cannot remove the last owner. Transfer ownership first." so callers
receive the correct, actionable error.
- Around line 47-65: The add procedure currently lets any caller who passes
verifyOrgAdmin assign role "owner", causing privilege escalation; update the
protectedProcedure handling in the add member endpoint (the add mutation in
members.ts) to disallow assigning "owner" for callers who only pass
verifyOrgAdmin by either narrowing the accepted roles to ["member","admin"] or
adding a runtime check that if input.role === "owner" then require
verifyOrgOwner (or throw an unauthorized error); mirror the same enforcement
approach used in updateRole in organization.ts to ensure admins cannot grant
owner.
🪄 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

Run ID: e9a34392-06a5-4df2-9ce0-41a68583c6e1

📥 Commits

Reviewing files that changed from the base of the PR and between 617cebe and 8a0fe6b.

📒 Files selected for processing (19)
  • apps/api/src/app/api/integrations/slack/events/utils/run-agent/mcp-clients.ts
  • apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts
  • packages/cli/src/commands/organization/members/list/command.ts
  • packages/cli/src/commands/organization/members/meta.ts
  • packages/cli/src/commands/tasks/statuses/list/command.ts
  • packages/cli/src/commands/tasks/statuses/meta.ts
  • packages/mcp-v2/src/in-memory.ts
  • packages/mcp-v2/src/tools/organization/members/list.ts
  • packages/mcp-v2/src/tools/register.ts
  • packages/mcp-v2/src/tools/tasks/statuses/list.ts
  • packages/sdk/src/client.ts
  • packages/sdk/src/resources/index.ts
  • packages/sdk/src/resources/organization.ts
  • packages/sdk/src/resources/tasks.ts
  • packages/shared/src/constants.ts
  • packages/trpc/src/router/organization/members.ts
  • packages/trpc/src/router/organization/organization.ts
  • packages/trpc/src/router/task/statuses.ts
  • packages/trpc/src/router/task/task.ts

Comment thread packages/trpc/src/router/organization/members.ts Outdated
Comment on lines +107 to +124
if (!canRemove) {
if (isTargetSelf) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Cannot remove yourself",
});
}
if (targetMember.role === "owner" && ownerCount === 1) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Cannot remove the last owner. Transfer ownership first.",
});
}
throw new TRPCError({
code: "FORBIDDEN",
message: "You don't have permission to remove this member",
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

remove error-message ordering masks the last-owner reason.

When the sole owner tries to remove themselves, isTargetSelf is true and targetMember.role === "owner" && ownerCount === 1 is also true. Because the isTargetSelf branch fires first, the caller receives "Cannot remove yourself" instead of the actionable "Cannot remove the last owner. Transfer ownership first." The second branch is unreachable in this scenario.

🐛 Proposed fix — check last-owner before self
 if (!canRemove) {
+  if (targetMember.role === "owner" && ownerCount === 1) {
+    throw new TRPCError({
+      code: "FORBIDDEN",
+      message: "Cannot remove the last owner. Transfer ownership first.",
+    });
+  }
   if (isTargetSelf) {
     throw new TRPCError({
       code: "FORBIDDEN",
       message: "Cannot remove yourself",
     });
   }
-  if (targetMember.role === "owner" && ownerCount === 1) {
-    throw new TRPCError({
-      code: "FORBIDDEN",
-      message: "Cannot remove the last owner. Transfer ownership first.",
-    });
-  }
   throw new TRPCError({
🤖 Prompt for 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.

In `@packages/trpc/src/router/organization/members.ts` around lines 107 - 124, The
error handling in the remove flow currently checks isTargetSelf before verifying
the last-owner condition, causing the "Cannot remove yourself" message to mask
the more actionable "Cannot remove the last owner" when the sole owner tries to
remove themselves; update the logic in the remove handler (the block that
examines canRemove, isTargetSelf, targetMember, and ownerCount) to check the
last-owner condition (targetMember.role === "owner" && ownerCount === 1) before
the isTargetSelf check and throw the TRPCError with message "Cannot remove the
last owner. Transfer ownership first." so callers receive the correct,
actionable error.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/trpc/src/router/organization/members.ts">

<violation number="1" location="packages/trpc/src/router/organization/members.ts:52">
P1: Privilege escalation: an admin can assign the `"owner"` role via this procedure because `verifyOrgAdmin` permits both admin and owner callers. Restrict the enum to `["member", "admin"]` here, or add a separate `verifyOrgOwner` guard when `role === "owner"`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread packages/trpc/src/router/organization/members.ts Outdated
Aligns with the existing All Access cohort pattern (person-level email
match) used by cloud-access. The flag now keys off the linking
Superset user's id so the cohort-based gating actually applies.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts">

<violation number="1" location="apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts:556">
P1: Evaluate `slack-mcp-v2` using organization scope, not user scope, to preserve the intended per-org rollout behavior.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

let useV2 = false;
try {
useV2 = Boolean(
await posthog.getFeatureFlag(FEATURE_FLAGS.SLACK_MCP_V2, params.userId),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: Evaluate slack-mcp-v2 using organization scope, not user scope, to preserve the intended per-org rollout behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts, line 556:

<comment>Evaluate `slack-mcp-v2` using organization scope, not user scope, to preserve the intended per-org rollout behavior.</comment>

<file context>
@@ -553,11 +553,7 @@ export async function runSlackAgent(
-				params.organizationId,
-				{ groups: { organization: params.organizationId } },
-			),
+			await posthog.getFeatureFlag(FEATURE_FLAGS.SLACK_MCP_V2, params.userId),
 		);
 	} catch (error) {
</file context>
Suggested change
await posthog.getFeatureFlag(FEATURE_FLAGS.SLACK_MCP_V2, params.userId),
await posthog.getFeatureFlag(
FEATURE_FLAGS.SLACK_MCP_V2,
params.organizationId,
{ groups: { organization: params.organizationId } },
),

Member add/remove flows have UI-side concerns (invite emails,
ownership transfer, audit) that don't belong in a programmatic
API yet. Keep the read-only `list` endpoint; add/remove stay
in the app via existing organization.{addMember,removeMember}
procedures.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/sdk/src/resources/organization.ts">

<violation number="1" location="packages/sdk/src/resources/organization.ts:26">
P1: This change makes `organization.members` read-only in the SDK by removing `add/remove`, which is a breaking API change for existing SDK users.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

export class Organization extends APIResource {
/**
* Member listing for the active organization. Member add/remove
* intentionally lives in the app — the programmatic API is read-only.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: This change makes organization.members read-only in the SDK by removing add/remove, which is a breaking API change for existing SDK users.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/resources/organization.ts, line 26:

<comment>This change makes `organization.members` read-only in the SDK by removing `add/remove`, which is a breaking API change for existing SDK users.</comment>

<file context>
@@ -18,37 +18,12 @@ export class Members extends APIResource {
 	/**
-	 * Member management for the active organization.
+	 * Member listing for the active organization. Member add/remove
+	 * intentionally lives in the app — the programmatic API is read-only.
 	 */
 	members: Members = new Members(this._client);
</file context>

Tip: Review your code locally with the cubic CLI to iterate faster.

@saddlepaddle saddlepaddle merged commit 1358cb0 into main May 7, 2026
17 checks passed
This was referenced May 7, 2026
saddlepaddle added a commit that referenced this pull request May 7, 2026
Changes since v0.2.11:

- workspaces: add `superset workspaces update <id> --name "..."` for
  renaming. Branch/host moves stay desktop-only — they require host-side
  git orchestration that isn't safe to drive from the cloud directly.
  (#4189)
- organization: new `superset organization members list` command (also
  exposed via SDK + mcp-v2). Read-only — add/remove keep their
  existing UI flows. (#4197)
- tasks: new `superset tasks statuses list` command surfacing the
  organization's configured task statuses. (#4197)
- host-service: paste a closed/merged PR URL or `#N` into the v2
  new-workspace modal and the dropdown actually returns the PR instead
  of "No open pull requests found." Direct lookups now ignore the
  open-only state filter that bounds text-search results. (#4190)

Push cli-v0.2.12 after this lands to fire the release pipeline.
saddlepaddle added a commit that referenced this pull request May 7, 2026
Changes since alpha.8:

- workspaces.update accepts `{ name }` for renaming. Branch/host moves
  stay desktop-only — they require host-side git orchestration that
  isn't safe to drive from the cloud. (#4189)
- organization.members.list — read-only listing of org members.
  add/remove deliberately not exposed; they have UI-side concerns
  (invites, ownership transfer, audit) that aren't ready for a
  programmatic API. (#4197)
- task.statuses.list — list configured task statuses for the org.
  (#4197)

After merge: `cd packages/sdk && bun run build && cd dist && npm publish --access public`.
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request May 8, 2026
Recorded as integrated via -s ours after batch PRs #455-#464.

Taken via individual PRs:
- PR  1 (#455): v2 polish 前半 safe set (9 commits)
- PR  2 (#456): v2/host-service polish 中盤 (12 commits)
- PR  3 (#457): sidebar polish + jwt API (5 commits)
- PR  4 (#458): host-service tRPC retry/cache/timeout (3 commits)
- PR  5 (#459): v2 diff pane / file pane polish (2 commits)
- PR  7 (#462): host-service v2 canonical workspace.create + attachment store (PR1 superset-sh#3893 + PR2 superset-sh#3916)
- PR 11 (#463): agents API + onboarding (7 commits + 1 cleanup)
- PR 12 (#464): v1→v2 import flow rewrite (11 commits + 2 follow-ups)
- PR 13 (#460): host-service shell env probe + typo (2 commits)
- PR 16 (#461): marketplace 19 themes (1 commit)

Skipped / deferred (recorded as integrated for behind=0):
- PR  6: CLI v1 launch (superset-sh#3898 + 30+ CLI/SDK followups) — defer to dedicated migration
- PR  9: v2 PR3 (superset-sh#3940) + revert (superset-sh#4017) — net-zero pair
- PR 10: pty-daemon (superset-sh#3896, superset-sh#3971, superset-sh#4054) — fork keeps its terminal-host
- PR 14: Slack MCP-v2 (superset-sh#4197, superset-sh#4208) — depends on mcp-v2/sdk divergence
- PR 15: onboarding remaining (superset-sh#4115, superset-sh#4125, superset-sh#4214, superset-sh#4213, superset-sh#4222, superset-sh#4225) — depends on fork's deleted setup pages

Behind: 0 after this merge.
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.

1 participant