fix(slack): address 6-agent PR review

Critical:
- Declare @archon/workflows as an explicit workspace dep on @archon/adapters
  (same class of fix as the providers one). Resolves today via hoisting but
  breaks under stricter installs.
- Split workflow-bridge.test.ts into its own bun test invocation so its
  irreversible mock.module() calls on @archon/core and
  @archon/workflows/event-emitter cannot leak into the slack/telegram
  batch.
- Fix "trailing-edge" debounce comments — the implementation is
  leading-edge. Document the Slack chat.update rate limit as the 500ms
  rationale.

Important:
- Wire slackBridge.detach() into the server graceful shutdown path so the
  event subscription doesn't leak and a pending chat.update can't fire
  against a closed Bolt socket.
- Drop dead `comment` plumbing through handleApprovalDecision /
  applyResolutionEdit / buildApprovalResolutionBlocks — Block Kit buttons
  have no UI to capture it.
- Widen the action-handler try/catch to also cover applyResolutionEdit so
  block-builder or chat.update failures don't bubble as unhandled
  rejections.
- Cancel-click with missing run state now logs and posts an ephemeral
  acknowledgement (using the button message's channel/ts) so the user
  isn't left wondering whether the click registered.
- Use Bolt's BlockButtonAction / ButtonAction types directly on the
  app.action() registrations instead of the ad-hoc ActionBody /
  ActionElement aliases.

Test coverage:
- Slash command silent-rejection of unauthorized users.
- triggeringMessages 1000-entry FIFO eviction at the cap boundary.
- Slash command seed-post failure → ephemeral error + handler not called.
- Single-chunk message path skips the _part i/n_ footer.
- rejectWorkflow → { cancelled: true, maxAttemptsReached: true } branch.

Docs:
- architecture.md IPlatformAdapter listing includes sendResultFooter.
- approval-nodes.md mentions the Slack in-thread Approve button.
- CLAUDE.md test-isolation batch count for @archon/adapters updated to 6
  (was 3 — pre-existing drift, now also accounts for workflow-bridge).

Polish:
- removeReactionSafe gets the same intentional-fallback comment as
  addReactionSafe (no_reaction is a normal terminal-state interleave).
- IPlatformAdapter.sendResultFooter signature uses TokenUsage directly.
- Drop "for v1" tag on the unhandled-event comment.
- Remove what-comments from blocks.ts / blocks.test.ts / adapter.ts.

Author: Wirasm

CLAUDE.md

@@ -130,7 +130,7 @@ bun test --watch            # Watch mode (single package)
 bun test packages/core/src/handlers/command-handler.test.ts  # Single file
 ```
 
-**Test isolation (mock.module pollution):** Bun's `mock.module()` permanently replaces modules in the process-wide cache — `mock.restore()` does NOT undo it ([oven-sh/bun#7823](https://github.com/oven-sh/bun/issues/7823)). To prevent cross-file pollution, packages that have conflicting `mock.module()` calls split their tests into separate `bun test` invocations: `@archon/core` (7 batches), `@archon/workflows` (5), `@archon/adapters` (3), `@archon/isolation` (3). See each package's `package.json` for the exact splits.
+**Test isolation (mock.module pollution):** Bun's `mock.module()` permanently replaces modules in the process-wide cache — `mock.restore()` does NOT undo it ([oven-sh/bun#7823](https://github.com/oven-sh/bun/issues/7823)). To prevent cross-file pollution, packages that have conflicting `mock.module()` calls split their tests into separate `bun test` invocations: `@archon/core` (7 batches), `@archon/workflows` (5), `@archon/adapters` (6), `@archon/isolation` (3). See each package's `package.json` for the exact splits.
 
 **Do NOT run `bun test` from the repo root** — it discovers all test files across all packages and runs them in one process, causing ~135 mock pollution failures. Always use `bun run test` (which uses `bun --filter '*' test` for per-package isolation).
 

bun.lock

@@ -11,7 +11,7 @@
     "./community/forge/gitlab": "./src/community/forge/gitlab/index.ts"
   },
   "scripts": {
-    "test": "bun test src/chat/ src/community/chat/ src/community/forge/gitlab/auth.test.ts src/forge/github/auth.test.ts src/utils/ && bun test src/forge/github/adapter.test.ts && bun test src/forge/github/context.test.ts && bun test src/community/forge/gitea/adapter.test.ts && bun test src/community/forge/gitlab/adapter.test.ts",
+    "test": "bun test src/chat/slack/adapter.test.ts src/chat/slack/auth.test.ts src/chat/slack/blocks.test.ts src/chat/telegram/ src/community/chat/ src/community/forge/gitlab/auth.test.ts src/forge/github/auth.test.ts src/utils/ && bun test src/chat/slack/workflow-bridge.test.ts && bun test src/forge/github/adapter.test.ts && bun test src/forge/github/context.test.ts && bun test src/community/forge/gitea/adapter.test.ts && bun test src/community/forge/gitlab/adapter.test.ts",
     "type-check": "bun x tsc --noEmit"
   },
   "dependencies": {
@@ -20,6 +20,7 @@
     "@archon/isolation": "workspace:*",
     "@archon/paths": "workspace:*",
     "@archon/providers": "workspace:*",
+    "@archon/workflows": "workspace:*",
     "@octokit/rest": "^22.0.0",
     "@slack/bolt": "^4.6.0",
     "discord.js": "^14.16.0",

packages/adapters/package.json

@@ -29,6 +29,8 @@ const mockReplies = mock(() => Promise.resolve({ messages: [] }));
 const mockEvent = mock(() => {});
 const mockStart = mock(() => Promise.resolve(undefined));
 const mockStop = mock(() => Promise.resolve(undefined));
+const mockCommand = mock(() => {});
+const mockAction = mock(() => {});
 
 const mockApp = {
   client: {
@@ -40,6 +42,8 @@ const mockApp = {
     },
   },
   event: mockEvent,
+  command: mockCommand,
+  action: mockAction,
   start: mockStart,
   stop: mockStop,
 };
@@ -365,5 +369,149 @@ describe('SlackAdapter', () => {
         })
       );
     });
+
+    test('single-chunk message is sent without _part footer', async () => {
+      await adapter.sendMessage('C1:111.0', 'short message');
+      expect(mockPostMessage).toHaveBeenCalledTimes(1);
+      const blocks = (mockPostMessage as Mock<typeof mockPostMessage>).mock.calls[0][0].blocks;
+      expect(blocks[0].text).toBe('short message');
+      expect(blocks[0].text).not.toContain('_part ');
+    });
+
+    test('multi-chunk message annotates each part with _part i/n_', async () => {
+      const paragraph1 = 'a'.repeat(10000);
+      const paragraph2 = 'b'.repeat(10000);
+      await adapter.sendMessage('C1:111.0', `${paragraph1}\n\n${paragraph2}`);
+      expect(mockPostMessage).toHaveBeenCalledTimes(2);
+      const calls = (mockPostMessage as Mock<typeof mockPostMessage>).mock.calls;
+      expect(calls[0][0].blocks[0].text).toContain('_part 1/2_');
+      expect(calls[1][0].blocks[0].text).toContain('_part 2/2_');
+    });
+  });
+
+  describe('triggering message tracking', () => {
+    test('FIFO evicts oldest entry past the cap', async () => {
+      // SlackAdapter caps the in-memory map at 1000 entries. To keep this
+      // test fast we exercise the eviction trigger by pumping app_mention
+      // events through the registered handler.
+      mockEvent.mockClear();
+      const adapter = new SlackAdapter('xoxb-fake', 'xapp-fake');
+      adapter.onMessage(async () => {
+        /* no-op handler; we only care about the trigger map side-effect */
+      });
+      await adapter.start();
+      // Find the app_mention handler we just registered.
+      const calls = (mockEvent as unknown as Mock<(t: string, h: unknown) => void>).mock.calls;
+      const mentionReg = calls.find(c => c[0] === 'app_mention');
+      expect(mentionReg).toBeDefined();
+      const handler = mentionReg![1] as (args: { event: SlackMessageEvent }) => Promise<void>;
+
+      const CAP = 1000;
+      for (let i = 0; i < CAP + 5; i++) {
+        await handler({
+          event: { text: 'hi', user: 'U1', channel: 'C1', ts: `${i}.0` },
+        });
+      }
+
+      // Oldest 5 should be evicted (channel:ts pairs 0.0 .. 4.0)
+      expect(adapter.getTriggeringMessage('C1:0.0')).toBeUndefined();
+      expect(adapter.getTriggeringMessage('C1:4.0')).toBeUndefined();
+      // Boundary: entry 5 is the new oldest and must still be tracked.
+      expect(adapter.getTriggeringMessage('C1:5.0')).toEqual({ channel: 'C1', ts: '5.0' });
+      // Newest entry must be present.
+      expect(adapter.getTriggeringMessage(`C1:${CAP + 4}.0`)).toEqual({
+        channel: 'C1',
+        ts: `${CAP + 4}.0`,
+      });
+    });
+  });
+
+  describe('slash commands', () => {
+    function findCommandHandler(name: string): (args: unknown) => Promise<void> {
+      const calls = (mockCommand as unknown as Mock<(n: string, h: unknown) => void>).mock.calls;
+      const reg = calls.find(c => c[0] === name);
+      if (!reg) throw new Error(`no handler registered for ${name}`);
+      return reg[1] as (args: unknown) => Promise<void>;
+    }
+
+    function makeSlashArgs(overrides: Record<string, unknown> = {}) {
+      const ack = mock(async () => {});
+      const respond = mock(async () => {});
+      const fakeClient = { chat: { postMessage: mockPostMessage } };
+      return {
+        ack,
+        respond,
+        client: fakeClient,
+        command: {
+          user_id: 'U123',
+          text: 'list',
+          channel_id: 'C1',
+          trigger_id: 't1',
+          ...overrides,
+        },
+      };
+    }
+
+    test('unauthorized user is silently rejected — no respond, no seed post', async () => {
+      mockPostMessage.mockClear();
+      mockCommand.mockClear();
+      const original = process.env.SLACK_ALLOWED_USER_IDS;
+      process.env.SLACK_ALLOWED_USER_IDS = 'U1ALICE';
+      try {
+        const adapter = new SlackAdapter('xoxb-fake', 'xapp-fake');
+        adapter.onMessage(async () => {});
+        await adapter.start();
+
+        const handler = findCommandHandler('/archon-workflow');
+        const args = makeSlashArgs({ user_id: 'U2BOB' });
+        await handler(args);
+
+        // ack always fires (Slack 3s deadline).
+        expect((args.ack as Mock<() => Promise<void>>).mock.calls.length).toBe(1);
+        // But the unauthorized branch must NOT speak back to the user
+        // (mirrors the app_mention silent-rejection policy).
+        expect((args.respond as Mock<() => Promise<void>>).mock.calls.length).toBe(0);
+        expect(mockPostMessage).not.toHaveBeenCalled();
+      } finally {
+        if (original === undefined) delete process.env.SLACK_ALLOWED_USER_IDS;
+        else process.env.SLACK_ALLOWED_USER_IDS = original;
+      }
+    });
+
+    test('seed-post failure surfaces ephemeral error and skips message handler', async () => {
+      mockPostMessage.mockClear();
+      mockCommand.mockClear();
+      const adapter = new SlackAdapter('xoxb-fake', 'xapp-fake');
+      const onMessage = mock(async () => {});
+      adapter.onMessage(onMessage);
+      await adapter.start();
+
+      // Reject the seed post — adapter must surface an ephemeral error and
+      // never invoke the message handler with an undefined ts.
+      const seedError = new Error('not_in_channel');
+      const failingPost = mock(async () => Promise.reject(seedError));
+      const fakeClient = { chat: { postMessage: failingPost } };
+      const ack = mock(async () => {});
+      const respond = mock(async () => {});
+
+      const handler = findCommandHandler('/archon');
+      await handler({
+        ack,
+        respond,
+        client: fakeClient,
+        command: {
+          user_id: 'U123',
+          text: 'hello',
+          channel_id: 'C_HIDDEN',
+          trigger_id: 't1',
+        },
+      });
+
+      expect(failingPost).toHaveBeenCalledTimes(1);
+      expect(onMessage).not.toHaveBeenCalled();
+      const respondCalls = (respond as Mock<() => Promise<void>>).mock.calls;
+      expect(respondCalls.length).toBe(1);
+      expect((respondCalls[0]![0] as { response_type: string }).response_type).toBe('ephemeral');
+    });
   });
 });

packages/adapters/src/chat/slack/adapter.test.ts

@@ -391,7 +391,6 @@ export class SlackAdapter implements IPlatformAdapter {
       }
     });
 
-    // Slash commands: /archon (general) and /archon-workflow (workflow control)
     this.app.command('/archon', async ({ command, ack, respond, client }) => {
       await ack();
       await this.handleSlashCommand(command, respond, client, 'archon');

packages/adapters/src/chat/slack/adapter.ts

@@ -1,6 +1,3 @@
-/**
- * Unit tests for Slack Block Kit builders and cost footer formatter.
- */
 import {
   buildApprovalBlocks,
   buildApprovalResolutionBlocks,
@@ -99,7 +96,6 @@ describe('buildApprovalResolutionBlocks', () => {
       decision: 'approved',
       actorUserId: 'U123ALICE',
       originalMessage: 'Approve the migration?',
-      comment: 'looks good',
       outcomeNote: 'workflow resumed',
     });
 
@@ -108,7 +104,6 @@ describe('buildApprovalResolutionBlocks', () => {
     expect(text).toContain('Approved');
     expect(text).toContain('<@U123ALICE>');
     expect(text).toContain('Approve the migration?');
-    expect(text).toContain('> looks good');
     expect(text).toContain('workflow resumed');
   });
 

packages/adapters/src/chat/slack/blocks.test.ts

@@ -1,9 +1,8 @@
 /**
- * Pure Block Kit builders + cost footer formatter for the Slack adapter.
- *
- * Kept side-effect-free so they can be unit-tested without touching the Bolt
- * client. The Slack adapter and workflow bridge import these and feed the
- * results into `chat.postMessage` / `chat.update`.
+ * Side-effect-free so callers can unit-test these without spinning up a
+ * Bolt client. Consumed by `adapter.ts` (`sendResultFooter`) and the
+ * workflow bridge, both of which feed the output into `chat.postMessage`
+ * / `chat.update`.
  */
 import type { types } from '@slack/bolt';
 import type { TokenUsage } from '@archon/providers/types';
@@ -148,7 +147,6 @@ export function buildApprovalResolutionBlocks(input: {
   decision: 'approved' | 'rejected';
   actorUserId: string;
   originalMessage: string;
-  comment?: string;
   outcomeNote?: string;
 }): { blocks: KnownBlock[]; fallbackText: string } {
   const icon = input.decision === 'approved' ? ':white_check_mark:' : ':x:';
@@ -158,9 +156,6 @@ export function buildApprovalResolutionBlocks(input: {
     '',
     input.originalMessage,
   ];
-  if (input.comment) {
-    lines.push('', `> ${input.comment}`);
-  }
   if (input.outcomeNote) {
     lines.push('', `_${input.outcomeNote}_`);
   }

packages/adapters/src/chat/slack/blocks.ts

@@ -327,6 +327,38 @@ describe('SlackWorkflowBridge', () => {
     expect(text).toContain('will retry');
   });
 
+  test('reject button at max attempts notes the run was cancelled', async () => {
+    const { adapter, updated, triggerMap, dispatchAction } = makeFakeAdapter();
+    triggerMap.set('C1:111.0', { channel: 'C1', ts: '111.0' });
+    mockGetConversationId.mockReturnValue('C1:111.0');
+    mockRejectWorkflow.mockResolvedValue({ cancelled: true, maxAttemptsReached: true });
+
+    new SlackWorkflowBridge(adapter as never).attach();
+    await dispatchEvent({
+      type: 'workflow_started',
+      runId: 'r1',
+      workflowName: 'assist',
+      conversationId: 'conv-db-uuid',
+    });
+    await dispatchEvent({
+      type: 'approval_pending',
+      runId: 'r1',
+      nodeId: 'review',
+      message: 'Approve?',
+    });
+
+    await dispatchAction('reject:r1:review', {
+      user: { id: 'U999' },
+      channel: { id: 'C1' },
+      message: { ts: '2.000' },
+    });
+
+    expect(mockRejectWorkflow).toHaveBeenCalledTimes(1);
+    const text = (updated[0]?.blocks?.[0] as { text?: { text?: string } } | undefined)?.text?.text;
+    expect(text).toContain('Rejected');
+    expect(text).toContain('max reject attempts reached');
+  });
+
   test('cancel button calls abandonWorkflow', async () => {
     const { adapter, triggerMap, dispatchAction } = makeFakeAdapter();
     triggerMap.set('C1:111.0', { channel: 'C1', ts: '111.0' });