Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 75 additions & 1 deletion lib/admins/slack/__tests__/fetchBotMentions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ vi.mock("@/lib/admins/slack/getCutoffTs", () => ({

describe("fetchBotMentions", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.resetAllMocks();
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 7, 2026

Choose a reason for hiding this comment

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

P3: vi.resetAllMocks() deviates from the project convention (vi.clearAllMocks()) documented in CLAUDE.md and used across all other test files. If stronger isolation is intentionally desired here, consider updating the project convention; otherwise, revert to vi.clearAllMocks() for consistency.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/admins/slack/__tests__/fetchBotMentions.test.ts, line 32:

<comment>`vi.resetAllMocks()` deviates from the project convention (`vi.clearAllMocks()`) documented in CLAUDE.md and used across all other test files. If stronger isolation is intentionally desired here, consider updating the project convention; otherwise, revert to `vi.clearAllMocks()` for consistency.</comment>

<file context>
@@ -29,7 +29,7 @@ vi.mock("@/lib/admins/slack/getCutoffTs", () => ({
 describe("fetchBotMentions", () => {
   beforeEach(() => {
-    vi.clearAllMocks();
+    vi.resetAllMocks();
   });
 
</file context>
Suggested change
vi.resetAllMocks();
vi.clearAllMocks();
Fix with Cubic

});

it("throws when token is not set", async () => {
Expand Down Expand Up @@ -108,6 +108,80 @@ describe("fetchBotMentions", () => {
vi.unstubAllEnvs();
});

it("finds bot mentions inside thread replies", async () => {
vi.stubEnv("TEST_BOT_TOKEN", "xoxb-test");

vi.mocked(getBotUserId).mockResolvedValue("U_BOT");
vi.mocked(getBotChannels).mockResolvedValue([{ id: "C1", name: "general" }]);
vi.mocked(getCutoffTs).mockReturnValue(null);

// conversations.history returns a threaded parent (reply_count > 0) but no top-level mention
vi.mocked(slackGet)
.mockResolvedValueOnce({
ok: true,
messages: [
{
type: "message",
user: "U1",
text: "check this out",
ts: "1705312200.000000",
reply_count: 2,
thread_ts: "1705312200.000000",
},
],
})
// conversations.replies for the thread
.mockResolvedValueOnce({
ok: true,
messages: [
{
type: "message",
user: "U1",
text: "check this out",
ts: "1705312200.000000",
},
{
type: "message",
user: "U2",
text: "<@U_BOT> make a video for this",
ts: "1705312400.000000",
},
{
type: "message",
bot_id: "B1",
text: "here is your video https://example.com/video",
ts: "1705312500.000000",
},
],
});

vi.mocked(getSlackUserInfo).mockResolvedValue({ name: "Bob", avatar: null });

const mockFetchThreadResponses = vi.fn().mockResolvedValue([["https://example.com/video"]]);

const result = await fetchBotMentions({
tokenEnvVar: "TEST_BOT_TOKEN",
period: "all",
fetchThreadResponses: mockFetchThreadResponses,
});

expect(result).toHaveLength(1);
expect(result[0]).toMatchObject({
user_id: "U2",
user_name: "Bob",
prompt: "make a video for this",
channel_id: "C1",
channel_name: "general",
});

// Should use the thread_ts for fetching responses, not the reply ts
expect(mockFetchThreadResponses).toHaveBeenCalledWith("xoxb-test", [
{ channelId: "C1", ts: "1705312200.000000" },
]);

vi.unstubAllEnvs();
});

it("sorts tags newest first", async () => {
vi.stubEnv("TEST_BOT_TOKEN", "xoxb-test");

Expand Down
95 changes: 91 additions & 4 deletions lib/admins/slack/fetchBotMentions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,39 @@ interface ConversationsHistoryResponse {
text?: string;
ts?: string;
bot_id?: string;
reply_count?: number;
thread_ts?: string;
}>;
response_metadata?: { next_cursor?: string };
}

interface ConversationsRepliesResponse {
ok: boolean;
error?: string;
messages?: Array<{
type: string;
user?: string;
text?: string;
ts?: string;
bot_id?: string;
}>;
}

interface RawMention {
userId: string;
prompt: string;
ts: string;
threadTs: string;
channelId: string;
channelName: string;
}

interface ThreadParent {
channelId: string;
channelName: string;
ts: string;
}

interface FetchBotMentionsOptions {
tokenEnvVar: string;
period: AdminPeriod;
Expand All @@ -47,8 +68,8 @@ interface FetchBotMentionsOptions {
}

/**
* Fetches all Slack messages where a bot was mentioned.
* Generic over the bot token and thread response extractor.
* Fetches all Slack messages where a bot was mentioned, including mentions
* within thread replies. Generic over the bot token and thread response extractor.
*
* @param options - Configuration for which bot and how to extract thread responses
* @returns Array of BotTag objects representing each mention event, sorted newest first
Expand All @@ -66,6 +87,8 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis
const cutoffTs = getCutoffTs(period);
const mentions: RawMention[] = [];
const userCache: Record<string, { name: string; avatar: string | null }> = {};
const threadParents: ThreadParent[] = [];
const topLevelMentionThreads = new Set<string>();

for (const channel of channels) {
let cursor: string | undefined;
Expand All @@ -84,9 +107,19 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis

for (const msg of history.messages ?? []) {
if (msg.bot_id) continue;
if (!msg.ts) continue;

// Track threaded messages for scanning thread replies
if (msg.reply_count && msg.reply_count > 0) {
threadParents.push({
channelId: channel.id,
channelName: channel.name,
ts: msg.ts,
});
}

if (!msg.user) continue;
if (!msg.text?.includes(mentionPattern)) continue;
if (!msg.ts) continue;

if (!userCache[msg.user]) {
userCache[msg.user] = await getSlackUserInfo(token, msg.user);
Expand All @@ -96,18 +129,72 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis
userId: msg.user,
prompt: (msg.text ?? "").replace(new RegExp(`<@${botUserId}>\\s*`, "g"), "").trim(),
ts: msg.ts,
threadTs: msg.ts,
channelId: channel.id,
channelName: channel.name,
});

topLevelMentionThreads.add(`${channel.id}:${msg.ts}`);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 7, 2026

Choose a reason for hiding this comment

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

P1: Custom agent: Flag AI Slop and Fabricated Changes

topLevelMentionThreads is populated but never read — it's dead code. This appears to be an incomplete deduplication mechanism: mentions that appear both at the top level and inside thread replies will be counted twice. Either implement the deduplication check (e.g., skip thread-reply mentions whose channelId:ts is in this set) or remove the variable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/admins/slack/fetchBotMentions.ts, line 137:

<comment>`topLevelMentionThreads` is populated but never read — it's dead code. This appears to be an incomplete deduplication mechanism: mentions that appear both at the top level and inside thread replies will be counted twice. Either implement the deduplication check (e.g., skip thread-reply mentions whose `channelId:ts` is in this set) or remove the variable.</comment>

<file context>
@@ -96,18 +129,72 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis
           channelName: channel.name,
         });
+
+        topLevelMentionThreads.add(`${channel.id}:${msg.ts}`);
       }
 
</file context>
Fix with Cubic

}

cursor = history.response_metadata?.next_cursor || undefined;
} while (cursor);
}

// Scan thread replies for additional bot mentions
const THREAD_BATCH_SIZE = 5;
const THREAD_BATCH_DELAY_MS = 1100;

for (let i = 0; i < threadParents.length; i += THREAD_BATCH_SIZE) {
if (i > 0) {
await new Promise(resolve => setTimeout(resolve, THREAD_BATCH_DELAY_MS));
}
const batch = threadParents.slice(i, i + THREAD_BATCH_SIZE);
const batchResults = await Promise.allSettled(
batch.map(async tp => {
const replies = await slackGet<ConversationsRepliesResponse>(
"conversations.replies",
token,
{ channel: tp.channelId, ts: tp.ts },
);
if (!replies.ok) return [];

const threadMentions: RawMention[] = [];
for (const msg of replies.messages ?? []) {
if (msg.bot_id) continue;
if (!msg.user) continue;
if (!msg.text?.includes(mentionPattern)) continue;
if (!msg.ts) continue;
// Skip the parent message (already captured as top-level)
if (msg.ts === tp.ts) continue;

if (!userCache[msg.user]) {
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 7, 2026

Choose a reason for hiding this comment

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

P2: Race condition on userCache within the concurrent Promise.allSettled batch. If two threads in the same batch contain replies from the same uncached user, both will call getSlackUserInfo before either populates the cache. Consider pre-populating the cache or serializing the user lookups to avoid redundant Slack API calls that count against rate limits.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/admins/slack/fetchBotMentions.ts, line 171:

<comment>Race condition on `userCache` within the concurrent `Promise.allSettled` batch. If two threads in the same batch contain replies from the same uncached user, both will call `getSlackUserInfo` before either populates the cache. Consider pre-populating the cache or serializing the user lookups to avoid redundant Slack API calls that count against rate limits.</comment>

<file context>
@@ -96,18 +129,72 @@ export async function fetchBotMentions(options: FetchBotMentionsOptions): Promis
+          // Skip the parent message (already captured as top-level)
+          if (msg.ts === tp.ts) continue;
+
+          if (!userCache[msg.user]) {
+            userCache[msg.user] = await getSlackUserInfo(token, msg.user);
+          }
</file context>
Fix with Cubic

userCache[msg.user] = await getSlackUserInfo(token, msg.user);
}

threadMentions.push({
userId: msg.user,
prompt: (msg.text ?? "").replace(new RegExp(`<@${botUserId}>\\s*`, "g"), "").trim(),
ts: msg.ts,
threadTs: tp.ts,
channelId: tp.channelId,
channelName: tp.channelName,
});
}
return threadMentions;
}),
);

for (const result of batchResults) {
if (result.status === "fulfilled") {
mentions.push(...result.value);
}
}
}

const responses = await fetchThreadResponses(
token,
mentions.map(m => ({ channelId: m.channelId, ts: m.ts })),
mentions.map(m => ({ channelId: m.channelId, ts: m.threadTs })),
);

const tags: BotTag[] = mentions.map((m, i) => {
Expand Down
Loading