If a candidate path fails with a permanent error like 403 Forbidden or 401 Unauthorized (which can happen if the raw channel ID path is outside the allowed pathScope), retrying it in subsequent delay iterations is wasteful and increases latency. We should filter out candidate paths that encounter unauthorized/forbidden errors so they aren't retried in the next rounds.

        let activeCandidates = [...candidatePaths]
        for (const [index, delayMs] of readDelays.entries()) {
          if (activeCandidates.length === 0) break
          if (delayMs > 0) await delay(delayMs)
          const nextCandidates: string[] = []
          for (const candidatePath of activeCandidates) {
            try {
              return eventContextPreviewFromFile(await client.readFile(handle.workspaceId, candidatePath))
            } catch (error) {
              readFileError = error
              if (!isUnauthorizedError(error)) {
                nextCandidates.push(candidatePath)
              }
            }
          }
          activeCandidates = nextCandidates
        }

The current implementation hardcodes the provider check to exactly 'slack'. If the repository uses enterprise or custom Slack provider variants (e.g., 'slack-enterprise'), this check and the subsequent segments[0] !== 'slack' check will fail, preventing thread context paths from being mounted. We should generalize this to support any provider starting with 'slack-'.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align DM defaults across UI and bridge to keep DM listening truly opt-in.

Line 415 now defaults unset DM scope to false, but the provided UI snippets still default listenDms to true when unset. That mismatch can silently keep DM subscriptions enabled and weakens the PR’s intended privacy posture.

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/main/integration-event-bridge.ts` around lines 413 - 416, The bridge
function slackListenDms uses scopeBooleanDefault(..., false) to make DM
listening opt-in, but the UI snippets still default listenDms to true; update
the UI/snippet/default payloads so the
listenDms/listenDirectMessages/directMessages scope defaults to false (matching
slackListenDms and scopeBooleanDefault) — ensure the UI uses the same default
value or the same helper logic when constructing the integration scope so DM
listening is opt-in across both codepaths.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a test case for mounting both messages and threads for the same channel.

The coding guideline requires regression tests when touching integration notifications, not just happy-path cases. Based on integrations.test.ts line 490-507, when downloadHistoricalData is false, a single channel spec expands to both /slack/channels/C123/messages and /slack/channels/C123/threads. However, there's no test verifying that mounting both together works correctly as separate mounts without conflicts.

+  it('mounts both messages and threads for the same channel as separate mounts', async () => {
+    const manager = new IntegrationMountManager()
+
+    await manager.ensureMounted([
+      {
+        provider: 'slack',
+        mountPaths: ['/slack/channels/C123/messages', '/slack/channels/C123/threads']
+      }
+    ])
+
+    expect(mock.mountInputs).toHaveLength(2)
+    expect(mock.mountInputs[0]).toMatchObject({
+      localDir: '/tmp/pear-home/.agentworkforce/pear/relayfile/workspaces/account-workspace-id/slack/channels/C123/messages',
+      remotePath: '/slack/channels/C123/messages',
+      localLayout: 'exact',
+      syncMode: 'write-only'
+    })
+    expect(mock.mountInputs[1]).toMatchObject({
+      localDir: '/tmp/pear-home/.agentworkforce/pear/relayfile/workspaces/account-workspace-id/slack/channels/C123/threads',
+      remotePath: '/slack/channels/C123/threads',
+      localLayout: 'exact',
+      syncMode: 'mirror'
+    })
+  })

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/main/integration-mounts.test.ts` around lines 279 - 295, Add a regression
test that ensures a single Slack channel spec expands into both messages and
threads mounts when downloadHistoricalData is false: instantiate
IntegrationMountManager, call ensureMounted with a spec like { provider:
'slack', mountPaths: ['/slack/channels/C123'], downloadHistoricalData: false }
and assert that mock.mountInputs contains two entries matching remotePath
'/slack/channels/C123/messages' and '/slack/channels/C123/threads' with distinct
localDir values and both having localLayout 'exact' and syncMode 'mirror' (use
IntegrationMountManager, ensureMounted, and mock.mountInputs to locate and
verify the mounts).

Source: Coding guidelines

🚩 Slack DM listening default change is a breaking behavioral change

The slackListenDms function at src/main/integration-event-bridge.ts:403 changes its default from true to false. Any existing integration configuration that omits an explicit listenDms/listenDirectMessages/directMessages scope key will silently stop receiving DM events. This is clearly intentional (test renamed from 'can be disabled' to 'is opt-in'), but downstream consumers or existing project configurations that relied on the implicit default will be affected without any migration path or warning log.

(Refers to lines 403-416)

Was this helpful? React with 👍 or 👎 to provide feedback.