Skip to content

Commit d2449ff

Browse files
authored
Fix Slack integration event context reads (#154)
* fix: harden slack integration event context * fix: address slack event review feedback
1 parent 550a4f5 commit d2449ff

7 files changed

Lines changed: 192 additions & 50 deletions

File tree

src/main/__tests__/integration-event-bridge.test.ts

Lines changed: 78 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ function makeHarness(
160160
}
161161
readFileFailuresBeforeSuccess?: number
162162
failReadFile?: boolean
163+
readFileError?: Error
163164
sendDelayMs?: number
164165
onSendStart?: (activeSends: number) => void
165166
waitForDeliveryNeverSettles?: boolean
@@ -205,6 +206,7 @@ function makeHarness(
205206
if (options.readFileFailuresBeforeSuccess && readFileAttempts <= options.readFileFailuresBeforeSuccess) {
206207
throw new Error('remote file not ready')
207208
}
209+
if (options.readFileError) throw options.readFileError
208210
if (options.failReadFile) throw new Error('remote file not ready')
209211
return options.readFileResponse?.(workspaceId, path) ?? {
210212
path,
@@ -606,25 +608,16 @@ test('integration events watch selected relayfile mount paths', async () => {
606608

607609
assert.deepEqual(harness.subscribeCalls[0].globs, [
608610
'/slack/channels/C123ABC/**',
609-
'/slack/channels/C123ABC__proj-cloud/**',
610-
'/slack/channels/D*/**',
611-
'/slack/dms/*/**',
612-
'/slack/users/*/messages/**'
611+
'/slack/channels/C123ABC__proj-cloud/**'
613612
])
614613
assert.deepEqual(harness.subscribeCalls[0].options?.pathScope, [
615614
'/slack/channels/C123ABC/**',
616-
'/slack/channels/C123ABC__proj-cloud/**',
617-
'/slack/channels/D*/**',
618-
'/slack/dms/*/**',
619-
'/slack/users/*/messages/**'
615+
'/slack/channels/C123ABC__proj-cloud/**'
620616
])
621617
assert.equal(harness.subscribeCalls[0].options?.from, 'legacy')
622618
assert.deepEqual(integrationSubscriptionSummaries([slackIntegration])[0].watches, [
623619
'.integrations/slack/channels/C123ABC/**',
624-
'.integrations/slack/channels/C123ABC__proj-cloud/**',
625-
'.integrations/slack/channels/D*/**',
626-
'.integrations/slack/dms/*/**',
627-
'.integrations/slack/users/*/messages/**'
620+
'.integrations/slack/channels/C123ABC__proj-cloud/**'
628621
])
629622

630623
const selectedPath = '/slack/channels/C123ABC__proj-cloud/messages/1780668000_000000/meta.json'
@@ -669,8 +662,7 @@ test('integration events watch selected relayfile mount paths', async () => {
669662
assert.deepEqual(harness.sent, [])
670663

671664
await harness.emit(changeEvent('/slack/channels/D123ABC/messages/1780668180_000000/meta.json', 'slack'))
672-
await waitForSent(harness, 1)
673-
assert.deepEqual(harness.sent.map((message) => message.input.to), ['alice'])
665+
assert.deepEqual(harness.sent, [])
674666
})
675667

676668
test('slack raw-id and slug alias paths with distinct revisions inject once per logical message', async () => {
@@ -800,7 +792,7 @@ test('slack raw-id and slug alias duplicates suppress when one context read is s
800792
assert.match(harness.sent[0].input.text, /Message:\nreadable Slack message/u)
801793
})
802794

803-
test('slack edits after a blind alias claim still inject once the content changes', async () => {
795+
test('slack raw-id event resolves context through mounted slug alias', async () => {
804796
let messageText = 'original Slack message'
805797
const harness = makeHarness(['alice'], {
806798
readFileResponse: (_workspaceId, path) => {
@@ -827,8 +819,8 @@ test('slack edits after a blind alias claim still inject once the content change
827819
])
828820
})
829821

830-
// Raw-id copy first: every targeted read fails and the expanded event only
831-
// carries the sparse relayfile pointer, so the injection is blind.
822+
// Raw-id copy first: the raw targeted read fails, then the bridge retries the
823+
// selected mounted slug alias so the injection has usable context.
832824
await harness.emit({
833825
...changeEvent(
834826
'/slack/channels/C123ABC/messages/1780668000_000000/meta.json',
@@ -846,10 +838,21 @@ test('slack edits after a blind alias claim still inject once the content change
846838
} as ChangeEvent)
847839
await waitForSent(harness, 1, 2_500)
848840
assert.equal(harness.sent.length, 1)
849-
assert.match(harness.sent[0].input.text, /Message: unavailable; targeted context read did not return content\./u)
841+
assert.match(harness.sent[0].input.text, /Message:\noriginal Slack message/u)
842+
assert.match(harness.sent[0].input.text, /Path: \.integrations\/slack\/channels\/C123ABC__proj-cloud\/messages\/1780668000_000000\/meta\.json/u)
843+
assert.deepEqual(harness.readFileCalls.slice(0, 2), [
844+
{
845+
workspaceId: 'workspace-id',
846+
path: '/slack/channels/C123ABC/messages/1780668000_000000/meta.json'
847+
},
848+
{
849+
workspaceId: 'workspace-id',
850+
path: '/slack/channels/C123ABC__proj-cloud/messages/1780668000_000000/meta.json'
851+
}
852+
])
850853

851-
// The slug alias copy of the same record carries content: suppressed as a
852-
// duplicate, but the claim learns the content hash.
854+
// The slug alias copy of the same record is now a duplicate of the
855+
// content-bearing raw delivery.
853856
await harness.emit(changeEvent(
854857
'/slack/channels/C123ABC__proj-cloud/messages/1780668000_000000/meta.json',
855858
'slack',
@@ -1052,7 +1055,7 @@ test('historical download subscriptions can receive older remote events', async
10521055
assert.deepEqual(harness.sent.map((message) => message.input.to), ['alice'])
10531056
})
10541057

1055-
test('slack direct message event scope can be disabled', async () => {
1058+
test('slack direct message event scope is opt-in', async () => {
10561059
const harness = makeHarness()
10571060
const slackIntegration = integration({
10581061
provider: 'slack',
@@ -1173,11 +1176,17 @@ test('slack context falls back to expanded event data when targeted remote previ
11731176
assert.match(harness.sent[0].input.text, /Slack message event/u)
11741177
assert.match(harness.sent[0].input.text, /Author: Khaliq/u)
11751178
assert.match(harness.sent[0].input.text, /Message:\nexpanded Slack context/u)
1176-
assert.equal(harness.readFileCalls.length, 4)
1177-
assert.deepEqual(harness.readFileCalls[0], {
1178-
workspaceId: 'workspace-id',
1179-
path: messagePath
1180-
})
1179+
assert.equal(harness.readFileCalls.length, 8)
1180+
assert.deepEqual(harness.readFileCalls.slice(0, 2), [
1181+
{
1182+
workspaceId: 'workspace-id',
1183+
path: messagePath
1184+
},
1185+
{
1186+
workspaceId: 'workspace-id',
1187+
path: '/slack/channels/C123ABC/messages/1780668000_000000/meta.json'
1188+
}
1189+
])
11811190
assert.equal((harness.sent[0].input.data?.contextPreview as { kind?: string } | undefined)?.kind, 'text')
11821191
assert.equal((harness.sent[0].input.data?.contextPreview as { content?: string } | undefined)?.content, undefined)
11831192
})
@@ -1193,7 +1202,7 @@ test('slack context retries targeted remote preview before falling back to spars
11931202
integrationId: 'slack-1',
11941203
mountPaths: ['/slack/channels/C123ABC__proj-cloud'],
11951204
downloadHistoricalData: false,
1196-
scope: { notifyAgents: ['alice'] }
1205+
scope: { listenDms: true, notifyAgents: ['alice'] }
11971206
})
11981207
])
11991208
})
@@ -1217,6 +1226,45 @@ test('slack context retries targeted remote preview before falling back to spars
12171226
assert.doesNotMatch(harness.sent[0].input.text, /"deleted": false/u)
12181227
})
12191228

1229+
test('slack context stops targeted remote preview retries on auth failures', async () => {
1230+
const error = new Error('http 403 forbidden') as Error & { status: number }
1231+
error.status = 403
1232+
const harness = makeHarness(['alice'], { readFileError: error })
1233+
const messagePath = '/slack/channels/C123ABC__proj-cloud/messages/1780668000_000000/meta.json'
1234+
1235+
await withMockedNow('2026-06-05T14:00:00.000Z', async () => {
1236+
await harness.bridge.reconcile('project-1', [
1237+
integration({
1238+
provider: 'slack',
1239+
integrationId: 'slack-1',
1240+
mountPaths: ['/slack/channels/C123ABC__proj-cloud'],
1241+
downloadHistoricalData: false,
1242+
scope: { notifyAgents: ['alice'] }
1243+
})
1244+
])
1245+
})
1246+
1247+
await harness.emit({
1248+
...changeEvent(messagePath, 'slack'),
1249+
expand: async () => ({
1250+
level: 'full',
1251+
path: messagePath,
1252+
data: {
1253+
text: 'expanded Slack context'
1254+
}
1255+
})
1256+
} as ChangeEvent)
1257+
await waitForSent(harness, 1, 2_500)
1258+
1259+
assert.deepEqual(harness.readFileCalls, [
1260+
{
1261+
workspaceId: 'workspace-id',
1262+
path: messagePath
1263+
}
1264+
])
1265+
assert.match(harness.sent[0].input.text, /Message:\nexpanded Slack context/u)
1266+
})
1267+
12201268
test('slack context does not inject sparse relayfile pointer fallback as message content', async () => {
12211269
const harness = makeHarness(['alice'], { failReadFile: true })
12221270
const messagePath = '/slack/channels/D123ABC/messages/1780668000_000000/meta.json'
@@ -1228,7 +1276,7 @@ test('slack context does not inject sparse relayfile pointer fallback as message
12281276
integrationId: 'slack-1',
12291277
mountPaths: ['/slack/channels/C123ABC__proj-cloud'],
12301278
downloadHistoricalData: false,
1231-
scope: { notifyAgents: ['alice'] }
1279+
scope: { listenDms: true, notifyAgents: ['alice'] }
12321280
})
12331281
])
12341282
})
@@ -1600,16 +1648,10 @@ test('integration events preserve discovery mount paths', async () => {
16001648
await harness.bridge.reconcile('project-1', [slackIntegration])
16011649

16021650
assert.deepEqual(harness.subscribeCalls[0].globs, [
1603-
'/discovery/slack/**',
1604-
'/slack/channels/D*/**',
1605-
'/slack/dms/*/**',
1606-
'/slack/users/*/messages/**'
1651+
'/discovery/slack/**'
16071652
])
16081653
assert.deepEqual(integrationSubscriptionSummaries([slackIntegration])[0].watches, [
1609-
'.integrations/discovery/slack/**',
1610-
'.integrations/slack/channels/D*/**',
1611-
'.integrations/slack/dms/*/**',
1612-
'.integrations/slack/users/*/messages/**'
1654+
'.integrations/discovery/slack/**'
16131655
])
16141656

16151657
await harness.emit(changeEvent('/discovery/slack/actions/create-message/.schema.json', 'slack'))

src/main/integration-event-bridge.ts

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,18 @@ function dedupeStrings(values: string[]): string[] {
376376
return Array.from(new Set(values.map((value) => value.trim()).filter(Boolean))).sort()
377377
}
378378

379+
function dedupeStringsInOrder(values: string[]): string[] {
380+
const seen = new Set<string>()
381+
const deduped: string[] = []
382+
for (const value of values) {
383+
const trimmed = value.trim()
384+
if (!trimmed || seen.has(trimmed)) continue
385+
seen.add(trimmed)
386+
deduped.push(trimmed)
387+
}
388+
return deduped
389+
}
390+
379391
function sameStringList(left: string[], right: string[]): boolean {
380392
return left.length === right.length && left.every((value, index) => value === right[index])
381393
}
@@ -400,7 +412,7 @@ function scopeBooleanDefault(scope: Record<string, unknown>, keys: string[], def
400412

401413
function slackListenDms(integration: ConnectedIntegration): boolean {
402414
if (!isSlackProvider(integration.provider)) return false
403-
return scopeBooleanDefault(integration.scope, ['listenDms', 'listenDirectMessages', 'directMessages'], true)
415+
return scopeBooleanDefault(integration.scope, ['listenDms', 'listenDirectMessages', 'directMessages'], false)
404416
}
405417

406418
function pathSegments(path: string): string[] {
@@ -1708,6 +1720,32 @@ function slackEventContextPath(path: string): boolean {
17081720
return /^\/slack\/(?:channels|dms|users)\/[^/]+\/(?:messages|threads)\/.+\.json$/u.test(path)
17091721
}
17101722

1723+
function slackContextReadCandidatePaths(path: string, specs: SubscriptionSpec[]): string[] {
1724+
const normalizedPath = path.startsWith('/') ? path : `/${path}`
1725+
if (!slackEventContextPath(normalizedPath)) return [normalizedPath]
1726+
1727+
const match = normalizedPath.match(/^\/slack\/channels\/([^/]+)(\/(?:messages|threads)\/.+\.json)$/u)
1728+
if (!match?.[1] || !match[2]) return [normalizedPath]
1729+
1730+
const currentChannel = match[1]
1731+
const channelId = canonicalSlackChannelSegment(currentChannel)
1732+
const tail = match[2]
1733+
const candidates = [normalizedPath]
1734+
1735+
for (const spec of specs) {
1736+
for (const mountPath of spec.mountPaths) {
1737+
const mountMatch = mountPath.match(/^\/slack\/channels\/([^/]+)(?:\/|$)/u)
1738+
const mountedChannel = mountMatch?.[1]
1739+
if (!mountedChannel || canonicalSlackChannelSegment(mountedChannel) !== channelId) {
1740+
continue
1741+
}
1742+
candidates.push(`/slack/channels/${mountedChannel}${tail}`)
1743+
}
1744+
}
1745+
1746+
return dedupeStringsInOrder(candidates)
1747+
}
1748+
17111749
function slackScopeLabel(path: string): string | undefined {
17121750
const segments = pathSegments(path)
17131751
const channelIndex = segments.indexOf('channels')
@@ -1730,8 +1768,9 @@ function formatSlackIntegrationEventMessage(
17301768
const relayfilePath = eventSummaryValue(resource.path)
17311769
if (provider !== 'slack' || !relayfilePath || !slackEventContextPath(relayfilePath)) return null
17321770

1733-
const projectPath = projectIntegrationPathForRelayfilePath(relayfilePath)
1734-
const scopeLabel = slackScopeLabel(relayfilePath)
1771+
const contextPath = contextPreview?.path || relayfilePath
1772+
const projectPath = projectIntegrationPathForRelayfilePath(contextPath)
1773+
const scopeLabel = slackScopeLabel(contextPath)
17351774
const messageText = slackPreviewText(contextPreview)
17361775
const author = slackPreviewAuthor(contextPreview)
17371776
const lines = [
@@ -2271,7 +2310,11 @@ export class IntegrationEventBridge {
22712310
)
22722311
}
22732312

2274-
private async readEventContextPreview(projectId: string, event: ChangeEvent): Promise<EventContextPreview | undefined> {
2313+
private async readEventContextPreview(
2314+
projectId: string,
2315+
event: ChangeEvent,
2316+
matchedSpecs: SubscriptionSpec[]
2317+
): Promise<EventContextPreview | undefined> {
22752318
if (event.type === 'file.deleted' || event.type === 'relayfile.changed.summary') return undefined
22762319
const path = eventSummaryValue(event.resource.path)
22772320
if (!path) return undefined
@@ -2281,14 +2324,19 @@ export class IntegrationEventBridge {
22812324
const readDelays = slackEventContextPath(path) ? [0, ...EVENT_CONTEXT_READ_RETRY_DELAYS_MS] : [0]
22822325
const handle = await this.getWorkspaceHandle()
22832326
const client = handle.client()
2327+
const candidatePaths = slackEventContextPath(path)
2328+
? slackContextReadCandidatePaths(path, matchedSpecs)
2329+
: [path]
22842330
if (typeof client.readFile === 'function') {
22852331
for (const [index, delayMs] of readDelays.entries()) {
22862332
if (delayMs > 0) await delay(delayMs)
2287-
try {
2288-
return eventContextPreviewFromFile(await client.readFile(handle.workspaceId, path))
2289-
} catch (error) {
2290-
readFileError = error
2291-
if (index === readDelays.length - 1) break
2333+
for (const candidatePath of candidatePaths) {
2334+
try {
2335+
return eventContextPreviewFromFile(await client.readFile(handle.workspaceId, candidatePath))
2336+
} catch (error) {
2337+
readFileError = error
2338+
if (isUnauthorizedError(error)) throw error
2339+
}
22922340
}
22932341
}
22942342
}
@@ -2401,7 +2449,7 @@ export class IntegrationEventBridge {
24012449
}
24022450

24032451
const eventMetadata = integrationEventMetadata(event)
2404-
const contextPreview = await this.readEventContextPreview(projectId, event)
2452+
const contextPreview = await this.readEventContextPreview(projectId, event, matchedSpecs)
24052453
const usesConcreteAgentTargets = uniqueRecipients.every((recipient) => !recipient.startsWith('#'))
24062454
const canTrackInjectedDelivery = usesConcreteAgentTargets && typeof bridge.sendMessageAndWaitForInjected === 'function'
24072455
const shouldTrackDedupe = canTrackInjectedDelivery

src/main/integration-mounts.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,24 @@ describe('IntegrationMountManager', () => {
276276
})
277277
})
278278

279+
it('mounts Slack thread context roots in mirror mode', async () => {
280+
const manager = new IntegrationMountManager()
281+
282+
await manager.ensureMounted([
283+
{
284+
provider: 'slack',
285+
mountPaths: ['/slack/channels/C123/threads']
286+
}
287+
])
288+
289+
expect(mock.mountInputs[0]).toMatchObject({
290+
localDir: '/tmp/pear-home/.agentworkforce/pear/relayfile/workspaces/account-workspace-id/slack/channels/C123/threads',
291+
remotePath: '/slack/channels/C123/threads',
292+
localLayout: 'exact',
293+
syncMode: 'mirror'
294+
})
295+
})
296+
279297
it('rejects Slack command roots with traversal segments', async () => {
280298
const manager = new IntegrationMountManager()
281299

src/main/integrations.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,9 @@ describe('IntegrationsManager', () => {
500500
})).toEqual([
501501
'/discovery/slack',
502502
'/slack/channels/C123/messages',
503+
'/slack/channels/C123/threads',
503504
'/slack/dms/D123/messages',
505+
'/slack/dms/D123/threads',
504506
'/slack/users/U123/messages'
505507
])
506508
})
@@ -534,6 +536,7 @@ describe('IntegrationsManager', () => {
534536

535537
expect(message).toContain('create writeback files under .integrations/slack/channels/C123/messages')
536538
expect(message).toContain('Writeback command roots are mounted at .integrations/slack/channels/C123/messages')
539+
expect(message).toContain('live thread context roots are mounted at .integrations/slack/channels/C123/threads')
537540
expect(message).not.toContain('create writeback files under .integrations/slack/channels/C123, not under discovery')
538541
})
539542

@@ -577,6 +580,16 @@ describe('IntegrationsManager', () => {
577580
await vi.waitFor(() => {
578581
expect(mock.integrationMountManager.ensureMounted).toHaveBeenCalled()
579582
})
583+
expect(mock.integrationMountManager.ensureMounted).toHaveBeenLastCalledWith([
584+
{
585+
provider: 'slack',
586+
mountPaths: [
587+
'/discovery/slack',
588+
'/slack/channels/C123/messages',
589+
'/slack/channels/C123/threads'
590+
]
591+
}
592+
])
580593

581594
finishMountReconcile()
582595
})

0 commit comments

Comments
 (0)