diff --git a/docs/users/features/channels/gitlab.md b/docs/users/features/channels/gitlab.md index 965f005ada6..6e4d9d57716 100644 --- a/docs/users/features/channels/gitlab.md +++ b/docs/users/features/channels/gitlab.md @@ -156,6 +156,12 @@ The adapter uses GitLab's Todos API as the message source: The cursor (`lastProcessedId`) advances regardless of dispatch success or failure. Failed dispatches post a ⚠️ error comment on the issue/MR and are not retried — the user can re-mention the bot to trigger a new todo. +## Response Feedback + +For an accepted comment mention (note with `#note_` anchor), the channel adds a 👀 award emoji to the note while the agent is working, then removes it when the run completes, fails, or is cancelled. Both operations are best-effort: an award emoji API or permission failure is logged and never prevents the final response. + +Description mentions (no `#note_` anchor) do not receive an award emoji because there is no specific note to react to. + ## Known Limitations - **First start skips existing pending todos.** The cursor initializes to `{ lastProcessedId: 0, initialized: false }` on first launch. On the first poll cycle, all pre-existing pending todos are marked done without dispatch (the `initialized` flag gates this one-time drain), preventing a backlog flood. diff --git a/packages/channels/gitlab/src/GitlabAdapter.test.ts b/packages/channels/gitlab/src/GitlabAdapter.test.ts index 580453aaa0d..1fb089197c1 100644 --- a/packages/channels/gitlab/src/GitlabAdapter.test.ts +++ b/packages/channels/gitlab/src/GitlabAdapter.test.ts @@ -132,6 +132,14 @@ function createMockApi() { MergeRequests: { show: vi.fn().mockResolvedValue({ description: 'MR description' }), }, + IssueNoteAwardEmojis: { + award: vi.fn().mockResolvedValue({ id: 9000 }), + remove: vi.fn().mockResolvedValue(undefined), + }, + MergeRequestNoteAwardEmojis: { + award: vi.fn().mockResolvedValue({ id: 9000 }), + remove: vi.fn().mockResolvedValue(undefined), + }, }; } @@ -645,4 +653,298 @@ describe('GitlabChannel', () => { expect(channel.cursor.lastProcessedId).toBe(2); }); }); + + describe('working reaction', () => { + class ReactingGitlabChannel extends GitlabChannel { + override async handleInbound(envelope: Envelope): Promise { + this.onPromptStart(envelope.chatId, 'session-1', envelope.messageId); + await Promise.resolve(); + this.onPromptEnd(envelope.chatId, 'session-1', envelope.messageId); + } + + protected override startPollLoop(): void {} + } + + class LiveGitlabChannel extends GitlabChannel { + setReactionForTest( + messageId: string, + entry: { + target: { iid: number; title: string; isMr: boolean }; + noteId: number; + }, + ): void { + ( + this as unknown as { + reactions: Map< + string, + { + target: { iid: number; title: string; isMr: boolean }; + noteId: number; + award?: Promise<{ awardId: number }>; + } + >; + } + ).reactions.set(messageId, entry); + } + + startPromptForTest( + chatId: string, + sessionId: string, + messageId: string, + ): void { + this.onPromptStart(chatId, sessionId, messageId); + } + + endPromptForTest( + chatId: string, + sessionId: string, + messageId: string, + ): void { + this.onPromptEnd(chatId, sessionId, messageId); + } + } + + it('drives award/remove through real pollOnce path for note mention', async () => { + const channel = new ReactingGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await channel.connect(); + channel.disconnect(); + ( + channel as unknown as { + cursor: { lastProcessedId: number; initialized: boolean }; + } + ).cursor = { + lastProcessedId: 0, + initialized: true, + }; + mockApi.TodoLists.all.mockResolvedValueOnce([makeTodo()]); + + await ( + channel as unknown as { pollOnce: () => Promise } + ).pollOnce(); + + expect(mockApi.IssueNoteAwardEmojis.award).toHaveBeenCalledWith( + 'owner/repo', + 42, + 1001, + 'eyes', + ); + await vi.waitFor(() => + expect(mockApi.IssueNoteAwardEmojis.remove).toHaveBeenCalledWith( + 'owner/repo', + 42, + 1001, + 9000, + ), + ); + }); + + it('does not award emoji for description mention via real path', async () => { + const channel = new ReactingGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await channel.connect(); + channel.disconnect(); + ( + channel as unknown as { + cursor: { lastProcessedId: number; initialized: boolean }; + } + ).cursor = { + lastProcessedId: 0, + initialized: true, + }; + mockApi.TodoLists.all.mockResolvedValueOnce([ + makeTodo({ + target_url: 'https://gitlab.com/owner/repo/-/issues/42', + body: 'Test Issue', + }), + ]); + + await ( + channel as unknown as { pollOnce: () => Promise } + ).pollOnce(); + + expect(mockApi.IssueNoteAwardEmojis.award).not.toHaveBeenCalled(); + }); + + it('acknowledges a note mention with an eyes award emoji', async () => { + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 42, title: '', isMr: false }, + noteId: 1001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + + expect(mockApi.IssueNoteAwardEmojis.award).toHaveBeenCalledWith( + 'owner/repo', + 42, + 1001, + 'eyes', + ); + }); + + it('removes the award emoji when the prompt finishes', async () => { + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 42, title: '', isMr: false }, + noteId: 1001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + await Promise.resolve(); + liveChannel.endPromptForTest('owner/repo', 'session-1', '100'); + + await vi.waitFor(() => + expect(mockApi.IssueNoteAwardEmojis.remove).toHaveBeenCalledWith( + 'owner/repo', + 42, + 1001, + 9000, + ), + ); + }); + + it('uses MergeRequestNoteAwardEmojis for MR note mentions', async () => { + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 99, title: '', isMr: true }, + noteId: 2001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + + expect(mockApi.MergeRequestNoteAwardEmojis.award).toHaveBeenCalledWith( + 'owner/repo', + 99, + 2001, + 'eyes', + ); + }); + + it('does not award twice when onPromptStart is called again', async () => { + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 42, title: '', isMr: false }, + noteId: 1001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + + expect(mockApi.IssueNoteAwardEmojis.award).toHaveBeenCalledTimes(1); + }); + + it('handles award failure as best-effort', async () => { + mockApi.IssueNoteAwardEmojis.award.mockRejectedValueOnce( + new Error('403'), + ); + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 42, title: '', isMr: false }, + noteId: 1001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + + await vi.waitFor(() => + expect(mockApi.IssueNoteAwardEmojis.award).toHaveBeenCalled(), + ); + liveChannel.endPromptForTest('owner/repo', 'session-1', '100'); + await Promise.resolve(); + expect(mockApi.IssueNoteAwardEmojis.remove).not.toHaveBeenCalled(); + }); + + it('handles remove failure as best-effort', async () => { + mockApi.IssueNoteAwardEmojis.remove.mockRejectedValueOnce( + new Error('403'), + ); + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 42, title: '', isMr: false }, + noteId: 1001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + await Promise.resolve(); + liveChannel.endPromptForTest('owner/repo', 'session-1', '100'); + + await vi.waitFor(() => + expect(mockApi.IssueNoteAwardEmojis.remove).toHaveBeenCalled(), + ); + }); + + it('waits for pending award before removing', async () => { + const { promise: awardPending, resolve: resolveAward } = + Promise.withResolvers<{ id: number }>(); + mockApi.IssueNoteAwardEmojis.award.mockReturnValueOnce(awardPending); + const liveChannel = new LiveGitlabChannel( + 'test-gitlab', + makeConfig(), + makeBridge(), + ); + await liveChannel.connect(); + liveChannel.disconnect(); + liveChannel.setReactionForTest('100', { + target: { iid: 42, title: '', isMr: false }, + noteId: 1001, + }); + + liveChannel.startPromptForTest('owner/repo', 'session-1', '100'); + liveChannel.endPromptForTest('owner/repo', 'session-1', '100'); + expect(mockApi.IssueNoteAwardEmojis.remove).not.toHaveBeenCalled(); + + resolveAward({ id: 9001 }); + await awardPending; + await vi.waitFor(() => + expect(mockApi.IssueNoteAwardEmojis.remove).toHaveBeenCalledWith( + 'owner/repo', + 42, + 1001, + 9001, + ), + ); + }); + }); }); diff --git a/packages/channels/gitlab/src/GitlabAdapter.ts b/packages/channels/gitlab/src/GitlabAdapter.ts index 1d82d8897fc..1cbd3d0d007 100644 --- a/packages/channels/gitlab/src/GitlabAdapter.ts +++ b/packages/channels/gitlab/src/GitlabAdapter.ts @@ -6,7 +6,7 @@ import type { Envelope, } from '@qwen-code/channel-base'; import { PollingChannelBase } from '@qwen-code/channel-base'; -import { Gitlab } from '@gitbeaker/rest'; +import { Gitlab, type TodoSchema } from '@gitbeaker/rest'; import { z } from 'zod'; import { testBotMention, stripBotMention } from './mention.js'; @@ -20,16 +20,10 @@ interface GitlabCursor { initialized: boolean; } -interface Todo { - id: number; - action_name: string; - target_type: string; - body: string; - target_url: string; - updated_at: string; - project: { path_with_namespace: string }; - author: { username: string }; - target: { iid: number; title: string }; +interface GitlabTarget { + iid: number; + title: string; + isMr: boolean; } const cursorSchema = z.object({ @@ -42,6 +36,14 @@ export class GitlabChannel extends PollingChannelBase { private apiHost = 'https://gitlab.com'; private botUsername = ''; private descriptionCache = new Map(); + private readonly reactions = new Map< + string, + { + target: GitlabTarget; + noteId: number; + award?: Promise<{ awardId: number }>; + } + >(); constructor( name: string, @@ -127,6 +129,52 @@ export class GitlabChannel extends PollingChannelBase { await this.createNote(chatId, targetType, Number(match[1]), text); } + protected override onPromptStart( + chatId: string, + _sessionId: string, + messageId?: string, + ): void { + if (!messageId) return; + const entry = this.reactions.get(messageId); + if (!entry || entry.award) return; + const api = entry.target.isMr + ? this.api.MergeRequestNoteAwardEmojis + : this.api.IssueNoteAwardEmojis; + entry.award = api + .award(chatId, entry.target.iid, entry.noteId, 'eyes') + .then((r) => ({ awardId: r.id })); + void entry.award.catch((err) => { + entry.award = undefined; + process.stderr.write( + `[Channel:${this.name}] failed to acknowledge note ${entry.noteId}: ${err}\n`, + ); + }); + } + + protected override onPromptEnd( + chatId: string, + _sessionId: string, + messageId?: string, + ): void { + if (!messageId) return; + const entry = this.reactions.get(messageId); + if (!entry) return; + this.reactions.delete(messageId); + if (!entry.award) return; + const api = entry.target.isMr + ? this.api.MergeRequestNoteAwardEmojis + : this.api.IssueNoteAwardEmojis; + void entry.award + .then(({ awardId }) => + api.remove(chatId, entry.target.iid, entry.noteId, awardId), + ) + .catch((err) => { + process.stderr.write( + `[Channel:${this.name}] failed to remove acknowledgement from note ${entry.noteId}: ${err}\n`, + ); + }); + } + protected async pollOnce(): Promise { const templates = (this.config as GitlabConfig).action_prompt_template; if (!templates || Object.keys(templates).length === 0) return; @@ -136,7 +184,7 @@ export class GitlabChannel extends PollingChannelBase { const allTodos = (await this.api.TodoLists.all({ state: 'pending', - })) as unknown as Todo[]; + })) as TodoSchema[]; // First poll: drain all pre-existing todos without processing them. if (!this.cursor.initialized) { @@ -164,13 +212,21 @@ export class GitlabChannel extends PollingChannelBase { for (const todo of todos) { if ( !todo.project || - !todo.target || - !todo.target.iid || (todo.target_type !== 'Issue' && todo.target_type !== 'MergeRequest') ) { await this.skipTodo(todo); continue; } + const raw = (todo.target || {}) as { iid?: number; title?: string }; + if (!raw.iid) { + await this.skipTodo(todo); + continue; + } + const target: GitlabTarget = { + iid: raw.iid, + title: raw.title ?? '', + isMr: todo.target_type === 'MergeRequest', + }; const template = this.resolveTemplate(templates, todo.action_name); if (!template) { @@ -179,11 +235,9 @@ export class GitlabChannel extends PollingChannelBase { } const chatId = todo.project.path_with_namespace; - const targetType = todo.target_type === 'MergeRequest' ? 'mr' : 'issue'; - const threadId = `${targetType}:${todo.target.iid}`; try { - await this.processTodo(todo, template, chatId, targetType, threadId); + await this.processTodo(todo, target, template, chatId); } catch (err) { process.stderr.write( `[Channel:${this.name}] error processing todo ${todo.id}: ${err}\n`, @@ -191,8 +245,8 @@ export class GitlabChannel extends PollingChannelBase { try { await this.createNote( chatId, - targetType, - todo.target.iid, + target.isMr ? 'mr' : 'issue', + target.iid, '⚠️ Failed to process this request. Please re-mention the bot to retry.', ); } catch { @@ -210,7 +264,7 @@ export class GitlabChannel extends PollingChannelBase { } } - private async skipTodo(todo: Todo): Promise { + private async skipTodo(todo: TodoSchema): Promise { try { await this.api.TodoLists.done({ todoId: todo.id }); } catch { @@ -229,15 +283,18 @@ export class GitlabChannel extends PollingChannelBase { } private async processTodo( - todo: Todo, + todo: TodoSchema, + target: GitlabTarget, template: string, chatId: string, - targetType: string, - threadId: string, ): Promise { if (todo.author.username === this.botUsername) return; - const isNoteMention = /#note_\d+$/.test(todo.target_url); + const targetType = target.isMr ? 'mr' : 'issue'; + const threadId = `${targetType}:${target.iid}`; + const noteMatch = todo.target_url.match(/#note_(\d+)$/); + const isNoteMention = noteMatch !== null; + const messageId = String(todo.id); const needsDescription = !isNoteMention || template.includes('%description%'); @@ -248,7 +305,7 @@ export class GitlabChannel extends PollingChannelBase { description = await this.fetchDescription( chatId, targetType, - todo.target.iid, + target.iid, ); } catch (err) { process.stderr.write( @@ -259,7 +316,7 @@ export class GitlabChannel extends PollingChannelBase { description = await this.fetchDescription( chatId, targetType, - todo.target.iid, + target.iid, ); } } @@ -274,10 +331,11 @@ export class GitlabChannel extends PollingChannelBase { todo.author.username, chatId, threadId, - String(todo.id), + messageId, this.buildMetadata( template, todo, + target, chatId, todo.author.username, String(todo.id), @@ -286,7 +344,17 @@ export class GitlabChannel extends PollingChannelBase { true, ); - await this.handleInbound(envelope); + if (isNoteMention) { + this.reactions.set(messageId, { + target, + noteId: Number(noteMatch![1]), + }); + } + try { + await this.handleInbound(envelope); + } finally { + this.reactions.delete(messageId); + } } private async fetchDescription( @@ -301,10 +369,10 @@ export class GitlabChannel extends PollingChannelBase { let description: string; if (targetType === 'mr') { const mr = await this.api.MergeRequests.show(chatId, iid); - description = (mr as { description?: string }).description || ''; + description = mr.description || ''; } else { const issue = await this.api.Issues.show(iid, { projectId: chatId }); - description = (issue as { description?: string }).description || ''; + description = issue.description || ''; } this.descriptionCache.set(cacheKey, description); return description; @@ -338,7 +406,8 @@ export class GitlabChannel extends PollingChannelBase { private buildMetadata( template: string, - todo: Todo, + todo: TodoSchema, + target: GitlabTarget, chatId: string, author: string, commentId: string, @@ -349,8 +418,8 @@ export class GitlabChannel extends PollingChannelBase { project_url: `${this.apiHost}/${chatId}`, author, target_type: todo.target_type, - iid: String(todo.target.iid), - title: todo.target.title, + iid: String(target.iid), + title: target.title, description, todo_id: commentId, };