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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Orchestrator now shows emoji reaction feedback on Slack messages (πŸ‘€ β†’ πŸ”„ β†’ βœ…/❌) during workflow execution.

## [0.3.10] - 2026-04-29

Maintainer workflow suite, loop output variables, and broad workflow engine fixes
Expand Down
96 changes: 96 additions & 0 deletions packages/adapters/src/chat/slack/adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ mock.module('@archon/paths', () => ({
// Create mock functions
const mockPostMessage = mock(() => Promise.resolve(undefined));
const mockReplies = mock(() => Promise.resolve({ messages: [] }));
const mockReactionsAdd = mock(() => Promise.resolve(undefined));
const mockReactionsRemove = mock(() => Promise.resolve(undefined));
const mockEvent = mock(() => {});
const mockStart = mock(() => Promise.resolve(undefined));
const mockStop = mock(() => Promise.resolve(undefined));
Expand All @@ -38,6 +40,10 @@ const mockApp = {
conversations: {
replies: mockReplies,
},
reactions: {
add: mockReactionsAdd,
remove: mockReactionsRemove,
},
},
event: mockEvent,
start: mockStart,
Expand Down Expand Up @@ -366,4 +372,94 @@ describe('SlackAdapter', () => {
);
});
});

describe('addReaction', () => {
let adapter: SlackAdapter;

beforeEach(() => {
mockReactionsAdd.mockClear();
adapter = new SlackAdapter('xoxb-fake', 'xapp-fake');
});

test('should add reaction to valid conversation ID', async () => {
await adapter.addReaction('C123:1234567890.123456', 'eyes');

expect(mockReactionsAdd).toHaveBeenCalledWith({
channel: 'C123',
name: 'eyes',
timestamp: '1234567890.123456',
});
});

test('should handle different emoji names', async () => {
await adapter.addReaction('C456:1111111111.111111', 'white_check_mark');
await adapter.addReaction('C789:2222222222.222222', 'x');

expect(mockReactionsAdd).toHaveBeenCalledTimes(2);
expect(mockReactionsAdd).toHaveBeenNthCalledWith(1, {
channel: 'C456',
name: 'white_check_mark',
timestamp: '1111111111.111111',
});
expect(mockReactionsAdd).toHaveBeenNthCalledWith(2, {
channel: 'C789',
name: 'x',
timestamp: '2222222222.222222',
});
});

test('should silently handle invalid conversation ID format', async () => {
// No colon separator - should be ignored
await adapter.addReaction('invalid-id-no-colon', 'eyes');

expect(mockReactionsAdd).not.toHaveBeenCalled();
});

test('should silently handle missing timestamp', async () => {
await adapter.addReaction('C123:', 'eyes');

expect(mockReactionsAdd).not.toHaveBeenCalled();
});
});

describe('removeReaction', () => {
let adapter: SlackAdapter;

beforeEach(() => {
mockReactionsRemove.mockClear();
adapter = new SlackAdapter('xoxb-fake', 'xapp-fake');
});

test('should remove reaction from valid conversation ID', async () => {
await adapter.removeReaction('C123:1234567890.123456', 'eyes');

expect(mockReactionsRemove).toHaveBeenCalledWith({
channel: 'C123',
name: 'eyes',
timestamp: '1234567890.123456',
});
});

test('should handle different emoji names', async () => {
await adapter.removeReaction('C456:1111111111.111111', 'arrows_counterclockwise');

expect(mockReactionsRemove).toHaveBeenCalledWith({
channel: 'C456',
name: 'arrows_counterclockwise',
timestamp: '1111111111.111111',
});
});

test('should silently handle invalid conversation ID format', async () => {
await adapter.removeReaction('invalid-id-no-colon', 'eyes');

expect(mockReactionsRemove).not.toHaveBeenCalled();
});

test('should silently handle missing timestamp', async () => {
await adapter.removeReaction('C123:', 'eyes');

expect(mockReactionsRemove).not.toHaveBeenCalled();
});
});
});
52 changes: 52 additions & 0 deletions packages/adapters/src/chat/slack/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,58 @@ export class SlackAdapter implements IPlatformAdapter {
return `${event.channel}:${event.ts}`;
}

/**
* Add a reaction (emoji) to a message
* @param conversationId - Slack: "channel:timestamp"
* @param reaction - Emoji shortname (e.g., "eyes", "white_check_mark", "x")
*/
async addReaction(conversationId: string, reaction: string): Promise<void> {
// Parse conversationId: "channel:timestamp" format
const [channelId, timestamp] = conversationId.split(':');
if (!channelId || !timestamp) {
getLog().warn({ conversationId, reaction }, 'slack.reaction_invalid_conversation_id');
return;
}

try {
await this.app.client.reactions.add({
channel: channelId,
name: reaction,
timestamp: timestamp,
});
getLog().debug({ channelId, timestamp, reaction }, 'slack.reaction_added');
} catch (error) {
const err = error as Error;
getLog().warn({ err, channelId, timestamp, reaction }, 'slack.reaction_failed');
}
}

/**
* Remove a reaction from a message
* @param conversationId - Slack: "channel:timestamp"
* @param reaction - Emoji shortname to remove (e.g., "eyes", "arrows_counterclockwise")
*/
async removeReaction(conversationId: string, reaction: string): Promise<void> {
// Parse conversationId: "channel:timestamp" format
const [channelId, timestamp] = conversationId.split(':');
if (!channelId || !timestamp) {
getLog().warn({ conversationId, reaction }, 'slack.remove_reaction_invalid_conversation_id');
return;
}

try {
await this.app.client.reactions.remove({
channel: channelId,
name: reaction,
timestamp: timestamp,
});
getLog().debug({ channelId, timestamp, reaction }, 'slack.reaction_removed');
} catch (error) {
const err = error as Error;
getLog().warn({ err, channelId, timestamp, reaction }, 'slack.remove_reaction_failed');
}
}

/**
* Strip bot mention from message text and normalize Slack formatting
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"./state/*": "./src/state/*.ts"
},
"scripts": {
"test": "bun test src/handlers/command-handler.test.ts && bun test src/handlers/clone.test.ts && bun test src/db/adapters/postgres.test.ts && bun test src/db/connection.test.ts && bun test src/db/adapters/sqlite.test.ts src/db/codebases.test.ts src/db/conversations.test.ts src/db/env-vars.test.ts src/db/isolation-environments.test.ts src/db/messages.test.ts src/db/sessions.test.ts src/db/workflow-events.test.ts src/db/workflows.test.ts src/utils/defaults-copy.test.ts src/utils/worktree-sync.test.ts src/utils/conversation-lock.test.ts src/utils/credential-sanitizer.test.ts src/utils/port-allocation.test.ts src/utils/error.test.ts src/utils/error-formatter.test.ts src/utils/github-graphql.test.ts src/config/ src/state/ && bun test src/utils/path-validation.test.ts && bun test src/services/cleanup-service.test.ts && bun test src/services/title-generator.test.ts && bun test src/workflows/ && bun test src/operations/workflow-operations.test.ts && bun test src/operations/isolation-operations.test.ts && bun test src/orchestrator/orchestrator.test.ts && bun test src/orchestrator/orchestrator-agent.test.ts && bun test src/orchestrator/orchestrator-isolation.test.ts",
"test": "bun test src/handlers/command-handler.test.ts && bun test src/handlers/clone.test.ts && bun test src/db/adapters/postgres.test.ts && bun test src/db/connection.test.ts && bun test src/db/adapters/sqlite.test.ts src/db/codebases.test.ts src/db/conversations.test.ts src/db/env-vars.test.ts src/db/isolation-environments.test.ts src/db/messages.test.ts src/db/sessions.test.ts src/db/workflow-events.test.ts src/db/workflows.test.ts src/utils/defaults-copy.test.ts src/utils/worktree-sync.test.ts src/utils/conversation-lock.test.ts src/utils/credential-sanitizer.test.ts src/utils/port-allocation.test.ts src/utils/error.test.ts src/utils/error-formatter.test.ts src/utils/github-graphql.test.ts src/config/ src/state/ && bun test src/utils/path-validation.test.ts && bun test src/services/cleanup-service.test.ts && bun test src/services/title-generator.test.ts && bun test src/workflows/ && bun test src/operations/workflow-operations.test.ts && bun test src/operations/isolation-operations.test.ts && bun test src/orchestrator/orchestrator.test.ts && bun test src/orchestrator/orchestrator-agent.test.ts && bun test src/orchestrator/orchestrator-isolation.test.ts && bun test src/orchestrator/safe-reaction.test.ts",
"type-check": "bun x tsc --noEmit",
"build": "echo 'No build needed - Bun runs TypeScript directly'"
},
Expand Down
65 changes: 65 additions & 0 deletions packages/core/src/orchestrator/orchestrator-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,40 @@ const MAX_BATCH_ASSISTANT_CHUNKS = 20;
/** Max total chunks (assistant + tool) to keep in batch mode */
const MAX_BATCH_TOTAL_CHUNKS = 200;

// ─── Reaction Helpers ───────────────────────────────────────────────────────

/**
* Safely add a reaction to a message.
* Errors are logged but not thrown to avoid breaking core message handling.
*/
export async function safeAddReaction(
platform: IPlatformAdapter,
conversationId: string,
reaction: string
): Promise<void> {
try {
await platform.addReaction?.(conversationId, reaction);
} catch (error) {
getLog().warn({ err: error, conversationId, reaction }, 'orchestrator.reaction_add_failed');
}
}

/**
* Safely remove a reaction from a message.
* Errors are logged but not thrown to avoid breaking core message handling.
*/
export async function safeRemoveReaction(
platform: IPlatformAdapter,
conversationId: string,
reaction: string
): Promise<void> {
try {
await platform.removeReaction?.(conversationId, reaction);
} catch (error) {
getLog().warn({ err: error, conversationId, reaction }, 'orchestrator.reaction_remove_failed');
}
}
Comment on lines +72 to +104
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

Add direct orchestrator coverage for the new reaction helpers.

Slack adapter tests cover the Slack SDK wiring, but they do not prove the contract introduced here: no-op when a platform has no reaction hooks, errors are swallowed and logged, and handleMessage() drives the intended πŸ‘€ β†’ πŸ”„ β†’ βœ…/❌ sequence. That orchestrator-level behavior is still untested in the provided changes.

I can sketch a small dedicated test file for these helper/lifecycle cases if helpful.

πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/orchestrator/orchestrator-agent.ts` around lines 72 - 104,
Add orchestrator-level tests that exercise safeAddReaction and
safeRemoveReaction and the lifecycle in handleMessage: create a fake
IPlatformAdapter instance with (a) no addReaction/removeReaction to assert
no-ops, (b) addReaction/removeReaction that throw to assert errors are swallowed
and getLog().warn is called, and (c) a normal adapter to assert handleMessage
triggers the reaction sequence (eyes β†’ spinner β†’ success/fail) in order; use
spies/mocks on platform methods and on getLog().warn to verify calls and
ordering.


// ─── Types ──────────────────────────────────────────────────────────────────

export interface WorkflowInvocation {
Expand Down Expand Up @@ -547,6 +581,9 @@ export async function handleMessage(
try {
getLog().debug({ conversationId }, 'orchestrator_message_received');

// React with πŸ‘€ to acknowledge message received
await safeAddReaction(platform, conversationId, 'eyes');

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// 1. Get/create conversation and inherit thread context
let conversation = await db.getOrCreateConversation(
platform.getPlatformType(),
Expand Down Expand Up @@ -836,6 +873,9 @@ export async function handleMessage(
const aiClient = getAgentProvider(conversation.ai_assistant_type);
getLog().debug({ assistantType: conversation.ai_assistant_type }, 'sending_to_ai');

// React with πŸ”„ when work begins
await safeAddReaction(platform, conversationId, 'arrows_counterclockwise');

// Reuse the config already loaded during workflow discovery (avoids a second disk read).
// Fall back to loadConfig only when no codebase is scoped (discoveredConfig is undefined).
const config = discoveredConfig ?? (await loadConfig());
Expand Down Expand Up @@ -905,9 +945,34 @@ export async function handleMessage(
}

getLog().debug({ conversationId }, 'orchestrator_message_completed');

// Reaction cleanup and final status (ensures even on early returns)
try {
// Remove intermediate reactions before adding final status
await safeRemoveReaction(platform, conversationId, 'eyes');
await safeRemoveReaction(platform, conversationId, 'arrows_counterclockwise');

// React with βœ… on success
await safeAddReaction(platform, conversationId, 'white_check_mark');
} catch {
// Silently ignore reaction errors
}
} catch (error) {
const err = toError(error);
getLog().error({ err, conversationId }, 'orchestrator_message_failed');

// Reaction cleanup and final status (ensures even on exceptions)
try {
// Remove intermediate reactions before adding final status
await safeRemoveReaction(platform, conversationId, 'eyes');
await safeRemoveReaction(platform, conversationId, 'arrows_counterclockwise');

// React with ❌ on failure
await safeAddReaction(platform, conversationId, 'x');
} catch {
// Silently ignore reaction errors
}

const userMessage = classifyAndFormatError(err);
try {
await platform.sendMessage(conversationId, userMessage);
Expand Down
72 changes: 72 additions & 0 deletions packages/core/src/orchestrator/safe-reaction.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { describe, it, expect, mock, beforeEach } from 'bun:test';
import type { IPlatformAdapter } from '../types';
import { safeAddReaction, safeRemoveReaction } from './orchestrator-agent';

// Minimal mock of IPlatformAdapter for reaction testing
const createMockPlatform = (): IPlatformAdapter => {
const platform: IPlatformAdapter = {
getPlatformType: mock(() => 'slack'),
sendMessage: mock(() => Promise.resolve()),
};
return platform;
};

describe('Reaction Helpers', () => {
let mockPlatform: IPlatformAdapter;

beforeEach(() => {
mockPlatform = createMockPlatform();
});

describe('safeAddReaction', () => {
it('calls platform.addReaction when it exists', async () => {
const addReactionSpy = mock(() => Promise.resolve());
mockPlatform.addReaction = addReactionSpy;

await safeAddReaction(mockPlatform, 'channel:123', 'eyes');

expect(addReactionSpy).toHaveBeenCalledWith('channel:123', 'eyes');
});

it('gracefully returns when platform.addReaction is undefined', async () => {
mockPlatform.addReaction = undefined;

await expect(safeAddReaction(mockPlatform, 'channel:123', 'eyes')).resolves.toBeUndefined();
});

it('catches and logs errors without throwing', async () => {
const error = new Error('API error');
mockPlatform.addReaction = mock(() => Promise.reject(error));

await expect(safeAddReaction(mockPlatform, 'channel:123', 'eyes')).resolves.toBeUndefined();
});
});

describe('safeRemoveReaction', () => {
it('calls platform.removeReaction when it exists', async () => {
const removeReactionSpy = mock(() => Promise.resolve());
mockPlatform.removeReaction = removeReactionSpy;

await safeRemoveReaction(mockPlatform, 'channel:123', 'eyes');

expect(removeReactionSpy).toHaveBeenCalledWith('channel:123', 'eyes');
});

it('gracefully returns when platform.removeReaction is undefined', async () => {
mockPlatform.removeReaction = undefined;

await expect(
safeRemoveReaction(mockPlatform, 'channel:123', 'eyes')
).resolves.toBeUndefined();
});

it('catches and logs errors without throwing', async () => {
const error = new Error('API error');
mockPlatform.removeReaction = mock(() => Promise.reject(error));

await expect(
safeRemoveReaction(mockPlatform, 'channel:123', 'eyes')
).resolves.toBeUndefined();
});
});
});
14 changes: 14 additions & 0 deletions packages/core/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,20 @@ export interface IPlatformAdapter {

/** Retract previously streamed text (used when workflow routing intercepts) */
emitRetract?(conversationId: string): Promise<void>;

/**
* Add a reaction (emoji) to a message
* @param conversationId - The conversation ID (for Slack: "channel:timestamp")
* @param reaction - Emoji name (Slack shortname, e.g., "eyes", "white_check_mark", "x")
*/
addReaction?(conversationId: string, reaction: string): Promise<void>;

/**
* Remove a reaction from a message
* @param conversationId - The conversation ID (for Slack: "channel:timestamp")
* @param reaction - Emoji name to remove
*/
removeReaction?(conversationId: string, reaction: string): Promise<void>;
}

/**
Expand Down
Loading