Skip to content

Add Slack DM recipient picker (user-message mount model)#156

Merged
khaliqgant merged 5 commits into
mainfrom
feat/slack-dm-recipient-picker
Jun 8, 2026
Merged

Add Slack DM recipient picker (user-message mount model)#156
khaliqgant merged 5 commits into
mainfrom
feat/slack-dm-recipient-picker

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

Pear/UI-mount layer of the comprehensive Slack DM build. Makes 1:1 DMs a first-class, selectable source via the canonical user-recipient model /slack/users/<U>/messages (bare Slack user ids). Pairs with the relayfile adapter D→U materialization and the Cloud token/options wiring (separate PRs).

What changed

  • New SlackDmRecipientPicker (reuses GenericScopePicker): consumes Cloud users options, emits concrete /slack/users/<U>/messages mount paths.
  • GenericScopePicker gains defaultSelectAll (default true, preserves channel/GitHub/Linear/Notion behavior). The DM picker sets it false so a fresh picker mounts no one's DMs by default — readability requires explicitly choosing recipients (privacy + scale).
  • ProjectSettings: separate channel vs DM pending scope state, merged into a single mountPaths array via mergeSlackScopeMountPaths (each half is preserved when only the other changes; /discovery/slack and other paths kept). listenDms stays watch-only, with copy clarifying it only observes events — recipients must be selected to read/send DMs.
  • Demote the vestigial /slack/dms/* event watch glob — no adapter resource or record ever materialized there. Canonical DM watch is now /slack/users/*/messages/**; /slack/channels/D* is retained as diagnostic-only.
  • Bare U…/W… ids only — never suffixed U…__slug (adapter writeback extraction + Cloud scopeMatchesPath only bridge channel suffixes today).

Tests

  • src/renderer/src/components/settings/slack-scope.test.ts — DM id/path helpers, mount-path classifiers, and mergeSlackScopeMountPaths (select adds + preserves channels/discovery; channel change preserves DMs; clear drops DM mounts; listenDms-only invents nothing; dedupe/order). vitest 10/10.
  • src/main/__tests__/integration-event-bridge.test.ts — asserts listenDms watch globs include /slack/users/*/messages/** + /slack/channels/D*/** and exclude /slack/dms/*/**; none when listenDms is off. node --test 63/63.

Validation

  • npx vitest run …/slack-scope.test.ts → 10/10
  • node --experimental-strip-types --test …/integration-event-bridge.test.ts → 63/63
  • tsc -p tsconfig.web.json (build-matching es2022 flags) → no errors in changed files

Notes

  • Source-only — the running Pear instance was not rebuilt/restarted (per the deferred-restart decision). DM reads/sends become live once the adapter D→U materialization + Cloud wiring land and the instance restarts.
  • No component-render tests (repo has no jsdom/RTL); coverage is via the extracted pure helpers, consistent with existing renderer tests.
  • Excludes pre-existing unrelated working-tree changes (ProjectSidebar.tsx, .claude/skills/, __.mcp.json, prpm.lock).

🤖 Generated with Claude Code

Make 1:1 Slack DMs a first-class, selectable Pear source using the canonical
user-recipient model `/slack/users/<U>/messages` (bare Slack user ids). This is
the Pear/UI-mount layer of the comprehensive DM build; it pairs with the
relayfile adapter D→U materialization and Cloud token/options wiring.

- New `SlackDmRecipientPicker` (reuses `GenericScopePicker`): consumes Cloud
  `users` options and emits concrete `/slack/users/<U>/messages` mount paths.
- `GenericScopePicker` gains `defaultSelectAll` (default true; preserves channel
  behavior). DM picker sets it false so a fresh picker mounts NO one's DMs by
  default — readability requires explicitly selecting recipients.
- `ProjectSettings`: separate channel vs DM pending scope state, merged into one
  `mountPaths` via `mergeSlackScopeMountPaths` (each half preserved when the
  other changes; discovery paths kept). `listenDms` stays watch-only with copy
  clarifying it observes events; recipients must be chosen to read/send DMs.
- Demote the vestigial `/slack/dms/*` event watch glob — no adapter resource or
  record ever materialized there. Canonical DM watch is
  `/slack/users/*/messages/**`; `/slack/channels/D*` stays diagnostic-only.
- Bare `U…`/`W…` ids only; never suffixed `U…__slug` (adapter writeback and
  Cloud scope aliasing only bridge channel suffixes today).

Tests: `slack-scope.test.ts` (helpers + mount-path merge) and an
`integration-event-bridge` glob test asserting the user-message watch is canonical
and `/slack/dms/*` is dropped. Source-only; no rebuild of the running instance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 8, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 8, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Slack DM recipient selection: backend simplifies DM glob config and exports the glob builder; frontend adds Slack mount-path utilities and tests, makes GenericScopePicker opt-out-capable, creates SlackDmRecipientPicker, and integrates DM selection into ProjectSettings.

Changes

Slack Direct Message Recipient Support

Layer / File(s) Summary
Backend DM event glob updates
src/main/integration-event-bridge.ts, src/main/__tests__/integration-event-bridge.test.ts
Slack DM event glob configuration simplified to use only canonical DM surfaces (/slack/channels/D*/** and /slack/users/*/messages/**), removing legacy /slack/dms/*/**. eventPathGlobsForIntegration is exported and tested.
Slack DM mount path utilities and tests
src/renderer/src/components/settings/slack-scope.ts, src/renderer/src/components/settings/slack-scope.test.ts
Pure helpers extract Slack user IDs, build canonical DM mount segments, classify mount paths (channel vs user-message), convert picker options into resources, and merge existing/channel/DM mount paths with deduplication and stable ordering. Tests cover helpers and merge behaviors.
GenericScopePicker opt-out default selection
src/renderer/src/components/settings/scope-pickers/GenericScopePicker.tsx
Adds optional defaultSelectAll prop (default true) and ref/effect changes so consumers can avoid pre-selecting all resources when no prior selection exists.
SlackDmRecipientPicker component
src/renderer/src/components/settings/scope-pickers/SlackDmRecipientPicker.tsx, src/renderer/src/components/settings/scope-pickers/index.ts
New specialized picker wraps GenericScopePicker with DM-specific labels, description fallbacks, mounting keys, and disables "select all" by default; re-export added.
ProjectSettings DM recipient integration
src/renderer/src/components/settings/ProjectSettings.tsx
Wires DM recipient selection into Slack scope editor: adds pending DM state, loads DM recipients via pear.integrations.listOptions, derives selected DM IDs from mounts and saved scope, merges channel and DM mount paths on save, and manages save/cancel/cleanup flows.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • AgentWorkforce/pear#154: Modifies src/main/integration-event-bridge.ts for Slack DM subscription and scoping behavior with opt-in listenDms and DM event glob coverage.

Poem

A rabbit hops through Slack's DM door,
I stitch mount paths and tests on the floor,
Pick recipients, merge lists just right,
Watch globs now sparkle in morning light,
Hooray—DMs listen, tidy and bright. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% 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 accurately summarizes the main change: adding a new Slack DM recipient picker component with a user-message mount model, which is the primary focus of this PR across multiple files.
Description check ✅ Passed The description is well-related to the changeset, providing comprehensive details about what changed, the design rationale, test validation, and implementation notes that align with the file summaries.
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 feat/slack-dm-recipient-picker

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.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for selecting specific Slack DM recipients to read and send direct messages, replacing the vestigial /slack/dms paths. It adds a new SlackDmRecipientPicker component, updates ProjectSettings to manage DM scope state, and introduces robust helper functions and unit tests in slack-scope.ts to merge channel and DM-recipient mount paths. The reviewer's feedback focuses on improving robustness by adding defensive checks and nullish guards across several new helper functions and components to prevent potential runtime TypeError crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +60 to +66
export function isSlackChannelMountPath(path: string): boolean {
return /^\/slack\/channels(?:\/|$)/u.test(path.trim())
}

export function isSlackUserMessagesMountPath(path: string): boolean {
return /^\/slack\/users\/[^/]+\/messages(?:\/|$)/u.test(path.trim())
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The isSlackChannelMountPath and isSlackUserMessagesMountPath functions assume path is always a string and call path.trim(). If path is null, undefined, or of another type, this will throw a TypeError. Adding a typeof path === 'string' check prevents runtime crashes when filtering lists of mount paths that might contain nullish or malformed entries.

Suggested change
export function isSlackChannelMountPath(path: string): boolean {
return /^\/slack\/channels(?:\/|$)/u.test(path.trim())
}
export function isSlackUserMessagesMountPath(path: string): boolean {
return /^\/slack\/users\/[^/]+\/messages(?:\/|$)/u.test(path.trim())
}
export function isSlackChannelMountPath(path: string): boolean {
return typeof path === 'string' && /^\/slack\/channels(?:\/|$)/u.test(path.trim())
}
export function isSlackUserMessagesMountPath(path: string): boolean {
return typeof path === 'string' && /^\/slack\/users\/[^/]+\/messages(?:\/|$)/u.test(path.trim())
}

Comment on lines +12 to +26
function resourceField(resource: IntegrationAccessibleResource, ...keys: Array<keyof IntegrationAccessibleResource>): string {
for (const key of keys) {
const value = resource[key]
if (typeof value === 'string' && value.trim()) return value.trim()
}
return ''
}

function resourceMeta(resource: IntegrationAccessibleResource, ...keys: string[]): string {
for (const key of keys) {
const value = resource.metadata?.[key]
if (typeof value === 'string' && value.trim()) return value.trim()
}
return ''
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The resourceField and resourceMeta helper functions do not guard against a null or undefined resource object. If resource is nullish at runtime, accessing properties like resource[key] or resource.metadata will throw a TypeError. Adding a simple nullish guard at the beginning of these functions ensures robustness.

function resourceField(resource: IntegrationAccessibleResource | null | undefined, ...keys: Array<keyof IntegrationAccessibleResource>): string {
  if (!resource) return ''
  for (const key of keys) {
    const value = resource[key]
    if (typeof value === 'string' && value.trim()) return value.trim()
  }
  return ''
}

function resourceMeta(resource: IntegrationAccessibleResource | null | undefined, ...keys: string[]): string {
  if (!resource) return ''
  for (const key of keys) {
    const value = resource.metadata?.[key]
    if (typeof value === 'string' && value.trim()) return value.trim()
  }
  return ''
}

Comment on lines +30 to +39
export function slackDmUserId(resource: IntegrationAccessibleResource): string {
return resourceMeta(resource, 'userId') || resourceField(resource, 'id', 'slug', 'key', 'name')
}

/** Mount segment appended to `/slack/users` → `<U…>/messages`, yielding the
* canonical 1:1 DM path `/slack/users/<U…>/messages`. Empty when no id. */
export function slackDmMountSegment(resource: IntegrationAccessibleResource): string {
const id = slackDmUserId(resource)
return id ? `${id}/messages` : ''
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Update the signatures of slackDmUserId and slackDmMountSegment to accept IntegrationAccessibleResource | null | undefined to align with the defensive guards added to resourceField and resourceMeta.

Suggested change
export function slackDmUserId(resource: IntegrationAccessibleResource): string {
return resourceMeta(resource, 'userId') || resourceField(resource, 'id', 'slug', 'key', 'name')
}
/** Mount segment appended to `/slack/users` → `<U…>/messages`, yielding the
* canonical 1:1 DM path `/slack/users/<U…>/messages`. Empty when no id. */
export function slackDmMountSegment(resource: IntegrationAccessibleResource): string {
const id = slackDmUserId(resource)
return id ? `${id}/messages` : ''
}
export function slackDmUserId(resource: IntegrationAccessibleResource | null | undefined): string {
return resourceMeta(resource, 'userId') || resourceField(resource, 'id', 'slug', 'key', 'name')
}
/** Mount segment appended to `/slack/users` → `<U…>/messages`, yielding the
* canonical 1:1 DM path `/slack/users/<U…>/messages`. Empty when no id. */
export function slackDmMountSegment(resource: IntegrationAccessibleResource | null | undefined): string {
const id = slackDmUserId(resource)
return id ? `${id}/messages` : ''
}

Comment on lines +42 to +58
export function slackUserResourceFromOption(option: {
value: string
label: string
hint?: string
}): IntegrationAccessibleResource {
const id = option.value.trim()
const name = option.label.replace(/^@/u, '').trim() || id
return {
id,
displayName: option.label,
name,
metadata: {
userId: id,
...(option.hint ? { hint: option.hint } : {})
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The slackUserResourceFromOption function does not guard against a null or undefined option or missing string properties. If option or option.value is nullish, calling .trim() will throw a TypeError. Adding optional chaining and fallback values makes this mapper function much more resilient to malformed API responses.

Suggested change
export function slackUserResourceFromOption(option: {
value: string
label: string
hint?: string
}): IntegrationAccessibleResource {
const id = option.value.trim()
const name = option.label.replace(/^@/u, '').trim() || id
return {
id,
displayName: option.label,
name,
metadata: {
userId: id,
...(option.hint ? { hint: option.hint } : {})
}
}
}
export function slackUserResourceFromOption(option: {
value: string
label: string
hint?: string
}): IntegrationAccessibleResource {
const id = (option?.value || '').trim()
const label = option?.label || ''
const name = label.replace(/^@/u, '').trim() || id
return {
id,
displayName: label,
name,
metadata: {
userId: id,
...(option?.hint ? { hint: option.hint } : {})
}
}
}

Comment on lines +644 to +656
const listSlackDmRecipients = useCallback(async (integration: ConnectedIntegration): Promise<IntegrationAccessibleResource[]> => {
const cacheKey = `slack-users:${projectId}:${integration.integrationId}`
return cachedResources(cacheKey, async () => {
const listOptions = (pear.integrations as typeof pear.integrations & {
listOptions?: typeof pear.integrations.listOptions
}).listOptions
if (typeof listOptions !== 'function') {
throw new Error('Slack DM recipient options are not available yet.')
}
const options = await listOptions(projectId, integration.provider, 'users')
return options.map(slackUserResourceFromOption)
})
}, [cachedResources, projectId])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If listOptions returns a non-array value (e.g., due to an unexpected API response or error state), calling options.map will throw a TypeError. Adding an Array.isArray(options) check ensures the application handles unexpected API payloads gracefully.

Suggested change
const listSlackDmRecipients = useCallback(async (integration: ConnectedIntegration): Promise<IntegrationAccessibleResource[]> => {
const cacheKey = `slack-users:${projectId}:${integration.integrationId}`
return cachedResources(cacheKey, async () => {
const listOptions = (pear.integrations as typeof pear.integrations & {
listOptions?: typeof pear.integrations.listOptions
}).listOptions
if (typeof listOptions !== 'function') {
throw new Error('Slack DM recipient options are not available yet.')
}
const options = await listOptions(projectId, integration.provider, 'users')
return options.map(slackUserResourceFromOption)
})
}, [cachedResources, projectId])
const listSlackDmRecipients = useCallback(async (integration: ConnectedIntegration): Promise<IntegrationAccessibleResource[]> => {
const cacheKey = 'slack-users:' + projectId + ':' + integration.integrationId
return cachedResources(cacheKey, async () => {
const listOptions = (pear.integrations as typeof pear.integrations & {
listOptions?: typeof pear.integrations.listOptions
}).listOptions
if (typeof listOptions !== 'function') {
throw new Error('Slack DM recipient options are not available yet.')
}
const options = await listOptions(projectId, integration.provider, 'users')
return Array.isArray(options) ? options.map(slackUserResourceFromOption) : []
})
}, [cachedResources, projectId])

Comment on lines +674 to +676
dmPaths: pendingDmScopeValue
? pendingDmScopeValue.mountPaths.filter(isSlackUserMessagesMountPath)
: null
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Guard against cases where pendingDmScopeValue is defined but mountPaths is missing or nullish at runtime, preventing potential TypeError crashes when calling .filter.

Suggested change
dmPaths: pendingDmScopeValue
? pendingDmScopeValue.mountPaths.filter(isSlackUserMessagesMountPath)
: null
dmPaths: pendingDmScopeValue?.mountPaths
? pendingDmScopeValue.mountPaths.filter(isSlackUserMessagesMountPath)
: null

Comment on lines +768 to +774
const selectedSlackDmRecipientIds = Array.from(new Set([
...integration.mountPaths
.filter(isSlackUserMessagesMountPath)
.map((path) => path.split('/')[3])
.filter(Boolean),
...scopeStringList(integration.scope, 'dmUsers')
]))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If integration.mountPaths is nullish or undefined at runtime, calling .filter on it will throw a TypeError. Adding a fallback empty array (integration.mountPaths || []) ensures the component renders safely.

Suggested change
const selectedSlackDmRecipientIds = Array.from(new Set([
...integration.mountPaths
.filter(isSlackUserMessagesMountPath)
.map((path) => path.split('/')[3])
.filter(Boolean),
...scopeStringList(integration.scope, 'dmUsers')
]))
const selectedSlackDmRecipientIds = Array.from(new Set([
...(integration.mountPaths || [])
.filter(isSlackUserMessagesMountPath)
.map((path) => path.split('/')[3])
.filter(Boolean),
...scopeStringList(integration.scope, 'dmUsers')
]))

The scope-editor open/close toggle reset pendingScopeValue + pendingSlackListenDms
but not the new pendingDmScopeValue, so an unsaved DM-recipient selection could
survive closing the editor and be applied on a later Save for the same or another
Slack integration. Clear it in the toggle handler alongside the other pending
state (Cancel and save-success paths already cleared it).

Reported by event-path-codex on PR #156.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 8, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 8, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 36e10c3 to this PR. The notes below describe what changed.

Implemented the validated PR fixes for Slack DM scoping robustness.

Changes made:

  • Hardened Slack helper functions against nullish/malformed resources, options, and mount paths.
  • Guarded DM recipient option loading against non-array returns.
  • Guarded pendingDmScopeValue.mountPaths and integration.mountPaths in ProjectSettings.
  • Added regression tests for malformed/nullish helper inputs and malformed mount-path merge inputs.

Verification run:

  • npx vitest run src/renderer/src/components/settings/slack-scope.test.ts passed, 14 tests.
  • npm test -- src/main/__tests__/integration-event-bridge.test.ts passed, 94 tests.
  • npx tsc --noEmit --project tsconfig.json passed.

GitHub status check: current PR metadata reports mergeable=true, but packaged-mcp-smoke is still in_progress, so I am not marking this READY.

Addressed comments

  • codeant-ai[bot]: free trial PR review limit reached. No code change: non-actionable billing/status comment.
  • coderabbitai[bot]: review in progress/run configuration comment. No code change: no specific finding to validate.
  • gemini-code-assist[bot]: review summary/deprecation notice. No code change: summary only; inline findings below handled individually.
  • gemini-code-assist[bot]: isSlackChannelMountPath/isSlackUserMessagesMountPath should guard non-string paths. Fixed in src/renderer/src/components/settings/slack-scope.ts:66; regression in src/renderer/src/components/settings/slack-scope.test.ts:64.
  • gemini-code-assist[bot]: resourceField/resourceMeta should guard nullish resources. Fixed in src/renderer/src/components/settings/slack-scope.ts:12 and src/renderer/src/components/settings/slack-scope.ts:24; regression in src/renderer/src/components/settings/slack-scope.test.ts:34.
  • gemini-code-assist[bot]: slackDmUserId/slackDmMountSegment should accept nullish resources. Fixed in src/renderer/src/components/settings/slack-scope.ts:35 and src/renderer/src/components/settings/slack-scope.ts:41; regression in src/renderer/src/components/settings/slack-scope.test.ts:34.
  • gemini-code-assist[bot]: slackUserResourceFromOption should tolerate nullish/malformed option data. Fixed in src/renderer/src/components/settings/slack-scope.ts:47; regression in src/renderer/src/components/settings/slack-scope.test.ts:41.
  • gemini-code-assist[bot]: listSlackDmRecipients should handle non-array listOptions results. Fixed in src/renderer/src/components/settings/ProjectSettings.tsx:653.
  • gemini-code-assist[bot]: pendingDmScopeValue.mountPaths should be guarded before .filter. Fixed in src/renderer/src/components/settings/ProjectSettings.tsx:674.
  • gemini-code-assist[bot]: integration.mountPaths should be guarded before render-time filtering. Fixed in src/renderer/src/components/settings/ProjectSettings.tsx:764.

Copy link
Copy Markdown

@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 3 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="src/renderer/src/components/settings/ProjectSettings.tsx">

<violation number="1" location="src/renderer/src/components/settings/ProjectSettings.tsx:654">
P2: Malformed DM options responses are silently swallowed as an empty list, hiding integration/API failures from users.</violation>
</file>

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

Re-trigger cubic

Comment thread src/renderer/src/components/settings/ProjectSettings.tsx Outdated
Copy link
Copy Markdown

@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.

2 issues found across 8 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="src/renderer/src/components/settings/ProjectSettings.tsx">

<violation number="1" location="src/renderer/src/components/settings/ProjectSettings.tsx:666">
P2: DM recipient state can be unintentionally cleared when recipient options are empty/unavailable, because pending picker output overwrites existing `dmUsers`/DM mount paths.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

selection: pendingScopeValue?.scope.selection ?? integration.scope.selection ?? 'selected',
channels: pendingScopeValue?.scope.channels ?? integration.scope.channels ?? [],
resources: pendingScopeValue?.scope.resources ?? integration.scope.resources ?? [],
dmUsers: pendingDmScopeValue?.scope.dmUsers ?? integration.scope.dmUsers ?? [],
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: DM recipient state can be unintentionally cleared when recipient options are empty/unavailable, because pending picker output overwrites existing dmUsers/DM mount paths.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/renderer/src/components/settings/ProjectSettings.tsx, line 666:

<comment>DM recipient state can be unintentionally cleared when recipient options are empty/unavailable, because pending picker output overwrites existing `dmUsers`/DM mount paths.</comment>

<file context>
@@ -642,8 +663,18 @@ function IntegrationVisibilitySection({
       selection: pendingScopeValue?.scope.selection ?? integration.scope.selection ?? 'selected',
       channels: pendingScopeValue?.scope.channels ?? integration.scope.channels ?? [],
       resources: pendingScopeValue?.scope.resources ?? integration.scope.resources ?? [],
+      dmUsers: pendingDmScopeValue?.scope.dmUsers ?? integration.scope.dmUsers ?? [],
       listenDms
     }
</file context>

Comment thread src/renderer/src/components/settings/slack-scope.ts Outdated
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 8, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@agent-relay-code
Copy link
Copy Markdown
Contributor

Implemented fixes for the valid current findings:

  • Prevented failed/empty non-default-select pickers from emitting synthetic empty scope updates that could clear saved Slack DM mounts.
  • Made malformed Slack DM recipient option payloads surface as an error.
  • Tightened Slack DM user ID extraction so display names cannot become /slack/users/<name>/messages paths.
  • Added a regression test for the display-name case.

Local validation passed:

  • npx vitest run src/renderer/src/components/settings/slack-scope.test.ts
  • npx tsc --noEmit --project tsconfig.json
  • npm run build
  • npm run verify:mcp-resources-drift
  • Earlier also passed npm test -- src/main/__tests__/integration-event-bridge.test.ts and npx vitest run src/main/broker.test.ts

Remote PR metadata currently reports the PR as mergeable and GitHub Actions jobs complete/successful, but the Cubic AI check was still in_progress, so I am not marking this READY.

Addressed comments

  • codeant-ai[bot]: free trial review limit reached. No code change: non-actionable billing/status comment.
  • codeant-ai[bot]: free trial review limit reached. No code change: duplicate non-actionable billing/status comment.
  • codeant-ai[bot]: free trial review limit reached. No code change: duplicate non-actionable billing/status comment.
  • coderabbitai[bot]: rate-limit/walkthrough comment and docstring coverage warning. No code change: no concrete current code finding to validate; local repo checks pass.
  • gemini-code-assist[bot]: review summary/deprecation notice. No code change: summary only; inline findings handled below.
  • gemini-code-assist[bot]: isSlackChannelMountPath/isSlackUserMessagesMountPath should guard non-string paths. Stale: already handled in src/renderer/src/components/settings/slack-scope.ts:66.
  • gemini-code-assist[bot]: resourceField/resourceMeta should guard nullish resources. Stale: already handled in src/renderer/src/components/settings/slack-scope.ts:12 and src/renderer/src/components/settings/slack-scope.ts:24.
  • gemini-code-assist[bot]: slackDmUserId/slackDmMountSegment should accept nullish resources. Stale: already handled in src/renderer/src/components/settings/slack-scope.ts:35.
  • gemini-code-assist[bot]: slackUserResourceFromOption should tolerate nullish/malformed option data. Stale: already handled in src/renderer/src/components/settings/slack-scope.ts:47.
  • gemini-code-assist[bot]: listSlackDmRecipients should handle non-array listOptions results. Fixed in src/renderer/src/components/settings/ProjectSettings.tsx:654.
  • gemini-code-assist[bot]: pendingDmScopeValue.mountPaths should be guarded before .filter. Stale: already handled in src/renderer/src/components/settings/ProjectSettings.tsx:678.
  • gemini-code-assist[bot]: integration.mountPaths should be guarded before render-time filtering. Stale: already handled in src/renderer/src/components/settings/ProjectSettings.tsx:767.
  • agent-relay-code[bot]: prior reviewer status summary. No code change: summary only; I revalidated the listed fixes and tests.
  • cubic-dev-ai[bot]: malformed DM option responses were silently swallowed. Fixed in src/renderer/src/components/settings/ProjectSettings.tsx:654.
  • cubic-dev-ai[bot]: empty/unavailable DM options could clear saved DM users/mounts. Fixed in src/renderer/src/components/settings/scope-pickers/GenericScopePicker.tsx:165.
  • cubic-dev-ai[bot]: DM user ID resolution could fall back to display names. Fixed in src/renderer/src/components/settings/slack-scope.ts:36; regression in src/renderer/src/components/settings/slack-scope.test.ts:34.

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 8, 2026

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

Copy link
Copy Markdown

@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: 1

🤖 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 `@src/renderer/src/components/settings/ProjectSettings.tsx`:
- Around line 767-771: The code adds all integration.mountPaths into
selectedSlackSourceIds, which incorrectly includes DM mounts like
"/slack/users/<U>/messages"; update the logic that builds integrationMountPaths
(and thus selectedSlackSourceIds) to filter out DM-style mount paths before
creating the Set. Specifically, transform integration.mountPaths (used in the
integrationMountPaths variable) by excluding entries that match Slack DM
patterns (e.g. paths starting with "/slack/users/") so that only
channel/conversation mount paths are merged with
scopeStringList(integration.scope, 'channels') for
GenericScopePicker/SlackChannelPicker initial state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0c290339-a336-4cc1-9407-010ca09110a0

📥 Commits

Reviewing files that changed from the base of the PR and between cc3d84f and e745219.

📒 Files selected for processing (6)
  • src/main/__tests__/integration-event-bridge.test.ts
  • src/main/integration-event-bridge.ts
  • src/renderer/src/components/settings/ProjectSettings.tsx
  • src/renderer/src/components/settings/scope-pickers/GenericScopePicker.tsx
  • src/renderer/src/components/settings/slack-scope.test.ts
  • src/renderer/src/components/settings/slack-scope.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/main/tests/integration-event-bridge.test.ts
  • src/renderer/src/components/settings/slack-scope.test.ts
  • src/main/integration-event-bridge.ts
  • src/renderer/src/components/settings/scope-pickers/GenericScopePicker.tsx
  • src/renderer/src/components/settings/slack-scope.ts

Comment on lines +767 to 771
const integrationMountPaths = integration.mountPaths ?? []
const selectedSlackSourceIds = Array.from(new Set([
...integration.mountPaths,
...integrationMountPaths,
...scopeStringList(integration.scope, 'channels')
]))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Filter DM mount paths out of the channel picker seed state.

Lines 767-771 now feed every integration.mountPaths entry into selectedSlackSourceIds. After this PR, that includes DM mounts like /slack/users/<U>/messages, so a DM-only Slack integration no longer looks like “no prior channel selection” to GenericScopePicker. That changes the initial state of SlackChannelPicker and can turn a DM-only config into an apparent empty channel selection on the next save.

Suggested fix
             const slackListenDms = pendingSlackListenDms ?? savedSlackListenDms
             const integrationMountPaths = integration.mountPaths ?? []
+            const selectedSlackChannelMountPaths = integrationMountPaths.filter((path) =>
+              path.startsWith('/slack/channels/')
+            )
             const selectedSlackSourceIds = Array.from(new Set([
-              ...integrationMountPaths,
+              ...selectedSlackChannelMountPaths,
               ...scopeStringList(integration.scope, 'channels')
             ]))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const integrationMountPaths = integration.mountPaths ?? []
const selectedSlackSourceIds = Array.from(new Set([
...integration.mountPaths,
...integrationMountPaths,
...scopeStringList(integration.scope, 'channels')
]))
const integrationMountPaths = integration.mountPaths ?? []
const selectedSlackChannelMountPaths = integrationMountPaths.filter((path) =>
path.startsWith('/slack/channels/')
)
const selectedSlackSourceIds = Array.from(new Set([
...selectedSlackChannelMountPaths,
...scopeStringList(integration.scope, 'channels')
]))
🤖 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 `@src/renderer/src/components/settings/ProjectSettings.tsx` around lines 767 -
771, The code adds all integration.mountPaths into selectedSlackSourceIds, which
incorrectly includes DM mounts like "/slack/users/<U>/messages"; update the
logic that builds integrationMountPaths (and thus selectedSlackSourceIds) to
filter out DM-style mount paths before creating the Set. Specifically, transform
integration.mountPaths (used in the integrationMountPaths variable) by excluding
entries that match Slack DM patterns (e.g. paths starting with "/slack/users/")
so that only channel/conversation mount paths are merged with
scopeStringList(integration.scope, 'channels') for
GenericScopePicker/SlackChannelPicker initial state.

@khaliqgant khaliqgant merged commit 1602097 into main Jun 8, 2026
8 of 12 checks passed
@khaliqgant khaliqgant deleted the feat/slack-dm-recipient-picker branch June 8, 2026 07:30
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