Add tests for reaction-based PR merge handler

Dockerfile

@@ -35,8 +35,8 @@ RUN ARCH=$(dpkg --print-architecture) && \
 ENV PATH="/usr/local/go/bin:/home/bot/go/bin:${PATH}"
 ENV GOPATH="/home/bot/go"
 
-# Install Claude Code CLI
-RUN npm install -g @anthropic-ai/claude-code
+# Install Claude Code CLI and TypeScript
+RUN npm install -g @anthropic-ai/claude-code typescript
 
 # Create non-root user
 RUN useradd -m -s /bin/bash bot \

src/bot/commands/deploy.ts

@@ -1,6 +1,5 @@
 import { WebClient } from '@slack/web-api';
 import { ThreadPRManager } from '../../manager/thread-pr';
-import { ThreadCompletionManager } from '../../manager/thread-completion';
 import { SessionManager } from '../../orchestrator/session';
 import { WorkspaceManager } from '../../workspace/manager';
 import { BranchManager } from '../../git/branch';
@@ -16,7 +15,6 @@ export function createDeployHandler(
   threadPRManager: ThreadPRManager,
   sessionManager: SessionManager,
   workspaceManager: WorkspaceManager,
-  threadCompletionManager: ThreadCompletionManager,
   botName: string = 'silverback'
 ) {
   return async (command: any, client: WebClient): Promise<void> => {
@@ -148,8 +146,6 @@ export function createDeployHandler(
         text: `PR created: ${pr.url}\n\nBranch \`${branch}\` pushed to \`${session.repository}\`.`,
       });
 
-      // Mark thread as complete: add ✅ reaction and clean up resources
-      await threadCompletionManager.markComplete(client, channelId, threadTs);
     } catch (error) {
       const errMsg = error instanceof Error ? error.message : 'Unknown error';
       logger.error('Deploy failed', { threadTs, error });

src/bot/events/__tests__/reaction.test.ts

@@ -0,0 +1,252 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { registerReactionHandler } from '../reaction';
+
+vi.mock('../../../git/pr', () => ({
+  PRManager: {
+    getStatus: vi.fn(),
+    merge: vi.fn(),
+  },
+}));
+
+vi.mock('../../../logging/logger', () => ({
+  Logger: class {
+    debug = vi.fn();
+    info = vi.fn();
+    warn = vi.fn();
+    error = vi.fn();
+  },
+}));
+
+import { PRManager } from '../../../git/pr';
+
+function createMockClient() {
+  return {
+    chat: {
+      postMessage: vi.fn().mockResolvedValue({}),
+    },
+    reactions: {
+      add: vi.fn().mockResolvedValue({}),
+    },
+  };
+}
+
+function createMockEvent(overrides: Record<string, unknown> = {}) {
+  return {
+    reaction: 'approved',
+    item: {
+      type: 'message',
+      channel: 'C123',
+      ts: '1234567890.000001',
+    },
+    ...overrides,
+  };
+}
+
+describe('registerReactionHandler', () => {
+  let reactionHandler: Function;
+  let mockApp: { event: ReturnType<typeof vi.fn> };
+  let mockSessionManager: { getSession: ReturnType<typeof vi.fn> };
+  let mockThreadPRManager: { getByThread: ReturnType<typeof vi.fn> };
+  let mockThreadCompletionManager: { cleanup: ReturnType<typeof vi.fn> };
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    mockApp = {
+      event: vi.fn((eventName: string, handler: Function) => {
+        if (eventName === 'reaction_added') {
+          reactionHandler = handler;
+        }
+      }),
+    };
+
+    mockSessionManager = {
+      getSession: vi.fn(),
+    };
+
+    mockThreadPRManager = {
+      getByThread: vi.fn(),
+    };
+
+    mockThreadCompletionManager = {
+      cleanup: vi.fn().mockResolvedValue(undefined),
+    };
+
+    registerReactionHandler(
+      mockApp as any,
+      mockSessionManager as any,
+      mockThreadPRManager as any,
+      mockThreadCompletionManager as any,
+    );
+  });
+
+  it('ignores non-approved reactions', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent({ reaction: 'thumbsup' });
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockSessionManager.getSession).not.toHaveBeenCalled();
+    expect(mockClient.chat.postMessage).not.toHaveBeenCalled();
+  });
+
+  it('ignores non-message items', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent({ item: { type: 'file', channel: 'C123', ts: '1234567890.000001' } });
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockSessionManager.getSession).not.toHaveBeenCalled();
+    expect(mockClient.chat.postMessage).not.toHaveBeenCalled();
+  });
+
+  it('returns silently when no session exists', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue(null);
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockSessionManager.getSession).toHaveBeenCalledWith('1234567890.000001');
+    expect(mockClient.chat.postMessage).not.toHaveBeenCalled();
+    expect(mockThreadPRManager.getByThread).not.toHaveBeenCalled();
+  });
+
+  it('posts message when no PR mapping found', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue(null);
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockClient.chat.postMessage).toHaveBeenCalledWith({
+      channel: 'C123',
+      thread_ts: '1234567890.000001',
+      text: 'No PR found for this thread. Use `/sb-deploy` to create a PR first.',
+    });
+    expect(vi.mocked(PRManager.getStatus)).not.toHaveBeenCalled();
+  });
+
+  it('posts message when PR mapping has no prNumber', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prUrl: 'https://github.com/org/repo/pull/42' });
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockClient.chat.postMessage).toHaveBeenCalledWith({
+      channel: 'C123',
+      thread_ts: '1234567890.000001',
+      text: 'No PR found for this thread. Use `/sb-deploy` to create a PR first.',
+    });
+  });
+
+  it('posts message when no workspace path on session', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: null });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prNumber: 42, prUrl: 'https://github.com/org/repo/pull/42' });
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockClient.chat.postMessage).toHaveBeenCalledWith({
+      channel: 'C123',
+      thread_ts: '1234567890.000001',
+      text: 'Cannot merge: workspace not found for this thread.',
+    });
+    expect(vi.mocked(PRManager.getStatus)).not.toHaveBeenCalled();
+  });
+
+  it('adds :merged: reaction and returns when PR is already merged', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prNumber: 42, prUrl: 'https://github.com/org/repo/pull/42' });
+    vi.mocked(PRManager.getStatus).mockResolvedValue('MERGED');
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(vi.mocked(PRManager.getStatus)).toHaveBeenCalledWith('/workspace', 42);
+    expect(mockClient.reactions.add).toHaveBeenCalledWith({
+      channel: 'C123',
+      timestamp: '1234567890.000001',
+      name: 'merged',
+    });
+    expect(vi.mocked(PRManager.merge)).not.toHaveBeenCalled();
+    expect(mockClient.chat.postMessage).not.toHaveBeenCalled();
+    expect(mockThreadCompletionManager.cleanup).not.toHaveBeenCalled();
+  });
+
+  it('does not throw when already_reacted on merged PR', async () => {
+    const mockClient = createMockClient();
+    mockClient.reactions.add.mockRejectedValue(new Error('already_reacted'));
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prNumber: 42, prUrl: 'https://github.com/org/repo/pull/42' });
+    vi.mocked(PRManager.getStatus).mockResolvedValue('MERGED');
+
+    await expect(reactionHandler({ event, client: mockClient })).resolves.not.toThrow();
+  });
+
+  it('posts error message when PR is closed', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prNumber: 42, prUrl: 'https://github.com/org/repo/pull/42' });
+    vi.mocked(PRManager.getStatus).mockResolvedValue('CLOSED');
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockClient.chat.postMessage).toHaveBeenCalledWith({
+      channel: 'C123',
+      thread_ts: '1234567890.000001',
+      text: 'Cannot merge: PR #42 is closed.',
+    });
+    expect(vi.mocked(PRManager.merge)).not.toHaveBeenCalled();
+    expect(mockThreadCompletionManager.cleanup).not.toHaveBeenCalled();
+  });
+
+  it('merges PR, adds reaction, posts confirmation, and calls cleanup on happy path', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prNumber: 42, prUrl: 'https://github.com/org/repo/pull/42' });
+    vi.mocked(PRManager.getStatus).mockResolvedValue('OPEN');
+    vi.mocked(PRManager.merge).mockResolvedValue(undefined);
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(vi.mocked(PRManager.merge)).toHaveBeenCalledWith('/workspace', 42);
+    expect(mockClient.reactions.add).toHaveBeenCalledWith({
+      channel: 'C123',
+      timestamp: '1234567890.000001',
+      name: 'merged',
+    });
+    expect(mockClient.chat.postMessage).toHaveBeenCalledWith({
+      channel: 'C123',
+      thread_ts: '1234567890.000001',
+      text: 'PR #42 merged. :merged:',
+    });
+    expect(mockThreadCompletionManager.cleanup).toHaveBeenCalledWith('1234567890.000001');
+  });
+
+  it('posts error message when merge throws', async () => {
+    const mockClient = createMockClient();
+    const event = createMockEvent();
+    mockSessionManager.getSession.mockResolvedValue({ workspacePath: '/workspace' });
+    mockThreadPRManager.getByThread.mockResolvedValue({ prNumber: 42, prUrl: 'https://github.com/org/repo/pull/42' });
+    vi.mocked(PRManager.getStatus).mockResolvedValue('OPEN');
+    vi.mocked(PRManager.merge).mockRejectedValue(new Error('merge conflict'));
+
+    await reactionHandler({ event, client: mockClient });
+
+    expect(mockClient.chat.postMessage).toHaveBeenCalledWith({
+      channel: 'C123',
+      thread_ts: '1234567890.000001',
+      text: 'Failed to merge PR #42: merge conflict',
+    });
+    expect(mockThreadCompletionManager.cleanup).not.toHaveBeenCalled();
+  });
+});

src/bot/events/reaction.ts

@@ -0,0 +1,107 @@
+import { App } from '@slack/bolt';
+import { ThreadPRManager } from '../../manager/thread-pr';
+import { ThreadCompletionManager } from '../../manager/thread-completion';
+import { SessionManager } from '../../orchestrator/session';
+import { PRManager } from '../../git/pr';
+import { Logger } from '../../logging/logger';
+
+const logger = new Logger('reaction-handler');
+
+export function registerReactionHandler(
+  app: App,
+  sessionManager: SessionManager,
+  threadPRManager: ThreadPRManager,
+  threadCompletionManager: ThreadCompletionManager,
+): void {
+  app.event('reaction_added', async ({ event, client }) => {
+    // Only handle :approved: reaction
+    if (event.reaction !== 'approved') return;
+
+    // The item must be a message
+    if (event.item.type !== 'message') return;
+
+    const channelId = event.item.channel;
+    const threadTs = event.item.ts;
+
+    // Look up the session for this thread
+    const session = await sessionManager.getSession(threadTs);
+    if (!session) {
+      logger.debug('No session found for reacted message', { threadTs });
+      return;
+    }
+
+    // Look up PR mapping
+    const mapping = await threadPRManager.getByThread(threadTs);
+    if (!mapping?.prNumber || !mapping?.prUrl) {
+      logger.info('No PR found for thread, ignoring approved reaction', { threadTs });
+      await client.chat.postMessage({
+        channel: channelId,
+        thread_ts: threadTs,
+        text: 'No PR found for this thread. Use `/sb-deploy` to create a PR first.',
+      });
+      return;
+    }
+
+    // Need workspace to run gh commands
+    if (!session.workspacePath) {
+      logger.warn('No workspace path for session', { threadTs });
+      await client.chat.postMessage({
+        channel: channelId,
+        thread_ts: threadTs,
+        text: 'Cannot merge: workspace not found for this thread.',
+      });
+      return;
+    }
+
+    try {
+      // Check PR status first
+      const status = await PRManager.getStatus(session.workspacePath, mapping.prNumber);
+      if (status === 'MERGED') {
+        logger.info('PR already merged', { prNumber: mapping.prNumber, threadTs });
+        // Add merged reaction anyway in case it's missing
+        try {
+          await client.reactions.add({ channel: channelId, timestamp: threadTs, name: 'merged' });
+        } catch { /* already_reacted */ }
+        return;
+      }
+      if (status === 'CLOSED') {
+        await client.chat.postMessage({
+          channel: channelId,
+          thread_ts: threadTs,
+          text: `Cannot merge: PR #${mapping.prNumber} is closed.`,
+        });
+        return;
+      }
+
+      // Merge the PR
+      await PRManager.merge(session.workspacePath, mapping.prNumber);
+      logger.info('PR merged via approved reaction', { prNumber: mapping.prNumber, threadTs });
+
+      // Add :merged: reaction
+      await client.reactions.add({
+        channel: channelId,
+        timestamp: threadTs,
+        name: 'merged',
+      });
+
+      // Post confirmation
+      await client.chat.postMessage({
+        channel: channelId,
+        thread_ts: threadTs,
+        text: `PR #${mapping.prNumber} merged. :merged:`,
+      });
+
+      // Clean up thread resources
+      await threadCompletionManager.cleanup(threadTs);
+      logger.info('Thread completed via approved reaction', { threadTs });
+    } catch (error) {
+      const errMsg = error instanceof Error ? error.message : String(error);
+      logger.error('Failed to merge PR via reaction', { threadTs, prNumber: mapping.prNumber, error: errMsg });
+      await client.chat.postMessage({
+        channel: channelId,
+        thread_ts: threadTs,
+        text: `Failed to merge PR #${mapping.prNumber}: ${errMsg}`,
+      });
+    }
+  });
+}