diff --git a/docs/legacy-compat-removal-spec.md b/docs/legacy-compat-removal-spec.md index aa96edb..4554bef 100644 --- a/docs/legacy-compat-removal-spec.md +++ b/docs/legacy-compat-removal-spec.md @@ -599,6 +599,17 @@ src/ - `getAllRoomBindings()` 도입 - startup sync 제거 +하위 컷 메모: + +- 하위 컷 1. `registered_groups` 기반 owner/room-mode/snapshot 추론 제거 + - 대상: `src/service-routing.ts`, `src/db.ts`, `src/db/room-registration.ts` + - 범위: stored canonical 값이 없을 때 `registered_groups` legacy row로 owner service, room mode, snapshot 의미를 추론하는 read-path fallback 제거 + - 동반 테스트: `src/service-routing.test.ts`, `src/db.test.ts` +- 하위 컷 2. 남은 `registeredGroups` runtime consumer 제거 + - 대상: `src/index.ts`, `src/message-runtime-loop.ts`, `src/ipc.ts` + - 범위: `getAllRegisteredGroups()` read-path 소비자 제거, `getAllRoomBindings()` 도입, startup sync 제거 +- 주의: 여기서 말하는 추론 제거는 PR 4 / Phase 4의 `paired-state.ts` hydrate fallback 제거와 별개다. + ### PR 4. Paired task canonicalization - paired/channel owner/handoff/work item backfill diff --git a/setup/register.test.ts b/setup/register.test.ts index 55bfe5f..df83157 100644 --- a/setup/register.test.ts +++ b/setup/register.test.ts @@ -63,8 +63,6 @@ describe('register step', () => { 'dc:test-room', '--name', 'Test Room', - '--trigger', - '@Andy', '--folder', 'test-room', ]); @@ -73,8 +71,6 @@ describe('register step', () => { expect(assignRoomMock).toHaveBeenCalledWith('dc:test-room', { name: 'Test Room', folder: 'test-room', - trigger: '@Andy', - requiresTrigger: true, isMain: false, }); expect(mkdirSpy).toHaveBeenCalledWith('/tmp/ejclaw-groups/test-room/logs', { @@ -87,8 +83,6 @@ describe('register step', () => { NAME: 'Test Room', FOLDER: 'test-room', CHANNEL: 'discord', - TRIGGER: '@Andy', - REQUIRES_TRIGGER: true, STATUS: 'success', LOG: 'logs/setup.log', }); @@ -107,8 +101,6 @@ describe('register step', () => { 'dc:test-room', '--name', 'Test Room', - '--trigger', - '@Andy', '--folder', 'test-room', '--assistant-name', @@ -141,19 +133,14 @@ describe('register step', () => { 'dc:main-room', '--name', 'Main Room', - '--trigger', - '@Andy', '--folder', 'main-room', '--is-main', - '--no-trigger-required', ]); expect(assignRoomMock).toHaveBeenCalledWith('dc:main-room', { name: 'Main Room', folder: 'main-room', - trigger: '@Andy', - requiresTrigger: false, isMain: true, }); expect(mkdirSpy).toHaveBeenCalledTimes(1); diff --git a/setup/register.ts b/setup/register.ts index 656cc40..7901411 100644 --- a/setup/register.ts +++ b/setup/register.ts @@ -15,10 +15,8 @@ import { emitStatus } from './status.js'; interface RegisterArgs { jid: string; name: string; - trigger: string; folder: string; channel: string; - requiresTrigger: boolean; isMain: boolean; assistantNameProvided: boolean; } @@ -27,10 +25,8 @@ function parseArgs(args: string[]): RegisterArgs { const result: RegisterArgs = { jid: '', name: '', - trigger: '', folder: '', channel: 'discord', - requiresTrigger: true, isMain: false, assistantNameProvided: false, }; @@ -43,18 +39,12 @@ function parseArgs(args: string[]): RegisterArgs { case '--name': result.name = args[++i] || ''; break; - case '--trigger': - result.trigger = args[++i] || ''; - break; case '--folder': result.folder = args[++i] || ''; break; case '--channel': result.channel = (args[++i] || '').toLowerCase(); break; - case '--no-trigger-required': - result.requiresTrigger = false; - break; case '--is-main': result.isMain = true; break; @@ -71,7 +61,7 @@ function parseArgs(args: string[]): RegisterArgs { export async function run(args: string[]): Promise { const parsed = parseArgs(args); - if (!parsed.jid || !parsed.name || !parsed.trigger || !parsed.folder) { + if (!parsed.jid || !parsed.name || !parsed.folder) { emitStatus('REGISTER_CHANNEL', { STATUS: 'failed', ERROR: 'missing_required_args', @@ -115,8 +105,6 @@ export async function run(args: string[]): Promise { assignRoom(parsed.jid, { name: parsed.name, folder: parsed.folder, - trigger: parsed.trigger, - requiresTrigger: parsed.requiresTrigger, isMain: parsed.isMain, }); logger.info('Assigned room through canonical room service'); @@ -131,8 +119,6 @@ export async function run(args: string[]): Promise { NAME: parsed.name, FOLDER: parsed.folder, CHANNEL: parsed.channel, - TRIGGER: parsed.trigger, - REQUIRES_TRIGGER: parsed.requiresTrigger, STATUS: 'success', LOG: 'logs/setup.log', }); diff --git a/src/available-groups.ts b/src/available-groups.ts index 8e7ae66..aad3de9 100644 --- a/src/available-groups.ts +++ b/src/available-groups.ts @@ -3,10 +3,10 @@ import { getAllChats } from './db.js'; import type { RegisteredGroup } from './types.js'; export function listAvailableGroups( - registeredGroups: Record, + roomBindings: Record, ): AvailableGroup[] { const chats = getAllChats(); - const registeredJids = new Set(Object.keys(registeredGroups)); + const registeredJids = new Set(Object.keys(roomBindings)); return chats .filter((chat) => chat.jid !== '__group_sync__' && chat.is_group) diff --git a/src/channels/discord.test.ts b/src/channels/discord.test.ts index 84388c4..9350eb1 100644 --- a/src/channels/discord.test.ts +++ b/src/channels/discord.test.ts @@ -133,7 +133,7 @@ function createTestOpts( return { onMessage: vi.fn(), onChatMetadata: vi.fn(), - registeredGroups: vi.fn(() => ({ + roomBindings: vi.fn(() => ({ 'dc:1234567890123456': { name: 'Test Server #general', folder: 'test-server', @@ -441,7 +441,7 @@ describe('DiscordChannel', () => { it('uses sender name for DM chats (no guild)', async () => { const opts = createTestOpts({ - registeredGroups: vi.fn(() => ({ + roomBindings: vi.fn(() => ({ 'dc:1234567890123456': { name: 'DM', folder: 'dm', @@ -491,10 +491,10 @@ describe('DiscordChannel', () => { }); }); - // --- @mention translation --- + // --- bot mention handling --- - describe('@mention translation', () => { - it('translates <@botId> mention to trigger format', async () => { + describe('bot mention handling', () => { + it('passes through <@botId> mentions without rewriting them', async () => { const opts = createTestOpts(); const channel = new DiscordChannel('test-token', opts); await channel.connect(); @@ -509,12 +509,12 @@ describe('DiscordChannel', () => { expect(opts.onMessage).toHaveBeenCalledWith( 'dc:1234567890123456', expect.objectContaining({ - content: '@Andy what time is it?', + content: '<@999888777> what time is it?', }), ); }); - it('does not translate if message already matches trigger', async () => { + it('leaves mixed text and mentions untouched', async () => { const opts = createTestOpts(); const channel = new DiscordChannel('test-token', opts); await channel.connect(); @@ -531,7 +531,7 @@ describe('DiscordChannel', () => { expect(opts.onMessage).toHaveBeenCalledWith( 'dc:1234567890123456', expect.objectContaining({ - content: '@Andy hello', + content: '@Andy hello <@999888777>', }), ); }); @@ -555,7 +555,7 @@ describe('DiscordChannel', () => { ); }); - it('handles <@!botId> (nickname mention format)', async () => { + it('passes through <@!botId> nickname mentions without rewriting them', async () => { const opts = createTestOpts(); const channel = new DiscordChannel('test-token', opts); await channel.connect(); @@ -570,7 +570,7 @@ describe('DiscordChannel', () => { expect(opts.onMessage).toHaveBeenCalledWith( 'dc:1234567890123456', expect.objectContaining({ - content: '@Andy check this', + content: '<@!999888777> check this', }), ); }); diff --git a/src/channels/discord.ts b/src/channels/discord.ts index c84aad3..ed3e71a 100644 --- a/src/channels/discord.ts +++ b/src/channels/discord.ts @@ -208,7 +208,7 @@ import { export interface DiscordChannelOpts { onMessage: OnInboundMessage; onChatMetadata: OnChatMetadata; - registeredGroups: () => Record; + roomBindings: () => Record; } export class DiscordChannel implements Channel { @@ -279,29 +279,6 @@ export class DiscordChannel implements Channel { chatName = senderName; } - // Translate Discord @bot mentions into TRIGGER_PATTERN format. - // Discord mentions look like <@botUserId> — these won't match - // TRIGGER_PATTERN (e.g., ^@Andy\b), so we prepend the trigger - // when the bot is @mentioned. - if (this.client?.user) { - const botId = this.client.user.id; - const isBotMentioned = - message.mentions.users.has(botId) || - content.includes(`<@${botId}>`) || - content.includes(`<@!${botId}>`); - - if (isBotMentioned) { - // Strip the <@botId> mention to avoid visual clutter - content = content - .replace(new RegExp(`<@!?${botId}>`, 'g'), '') - .trim(); - // Prepend trigger if not already present - if (!TRIGGER_PATTERN.test(content)) { - content = `@${ASSISTANT_NAME} ${content}`; - } - } - } - // Handle attachments — transcribe voice messages, download files const isVoiceMessage = message.flags.has(MessageFlags.IsVoiceMessage); if (message.attachments.size > 0) { @@ -403,7 +380,7 @@ export class DiscordChannel implements Channel { // Only deliver full message for registered groups. Secondary role bots // are configured as outbound-only, while the owner bot receives inbound. - const group = this.opts.registeredGroups()[chatJid]; + const group = this.opts.roomBindings()[chatJid]; if (!group) { logger.debug( { chatJid, chatName }, @@ -627,7 +604,7 @@ export class DiscordChannel implements Channel { if (!jid.startsWith('dc:')) return false; if (!this.ownsDiscordJids) return false; if (!this.agentTypeFilter) return true; - const group = this.opts.registeredGroups()[jid]; + const group = this.opts.roomBindings()[jid]; if (!group) return false; const groupType = group.agentType || 'claude-code'; return groupType === this.agentTypeFilter; diff --git a/src/channels/registry.ts b/src/channels/registry.ts index ab871c3..b145c94 100644 --- a/src/channels/registry.ts +++ b/src/channels/registry.ts @@ -8,7 +8,7 @@ import { export interface ChannelOpts { onMessage: OnInboundMessage; onChatMetadata: OnChatMetadata; - registeredGroups: () => Record; + roomBindings: () => Record; } export type ChannelFactory = (opts: ChannelOpts) => Channel | null; diff --git a/src/dashboard-status-content.ts b/src/dashboard-status-content.ts index 5429cdc..c85c237 100644 --- a/src/dashboard-status-content.ts +++ b/src/dashboard-status-content.ts @@ -13,7 +13,7 @@ export interface DashboardOptions { channels: Channel[]; getSessions: () => Record; queue: GroupQueue; - registeredGroups: () => Record; + roomBindings: () => Record; statusChannelId: string; statusUpdateInterval: number; usageUpdateInterval: number; @@ -30,7 +30,7 @@ export function buildStatusContent(opts: DashboardOptions): string { if (!STATUS_SHOW_ROOMS) return ''; const sessions = opts.getSessions(); - const groups = opts.registeredGroups(); + const groups = opts.roomBindings(); const statuses = opts.queue.getStatuses(Object.keys(groups)); let totalActive = 0; diff --git a/src/dashboard.test.ts b/src/dashboard.test.ts index 77b0cc0..32074cf 100644 --- a/src/dashboard.test.ts +++ b/src/dashboard.test.ts @@ -22,7 +22,7 @@ function makeOptions(sessionId?: string): DashboardOptions { }, ], } as unknown as DashboardOptions['queue'], - registeredGroups: () => ({ + roomBindings: () => ({ 'dc:123': { name: 'clone-test', folder: 'group-folder', diff --git a/src/dashboard.ts b/src/dashboard.ts index de85418..ff73b71 100644 --- a/src/dashboard.ts +++ b/src/dashboard.ts @@ -22,7 +22,7 @@ export async function startStatusDashboard( usageUpdateInterval: opts.usageUpdateInterval, channels: opts.channels, queue: opts.queue, - registeredGroups: opts.registeredGroups, + roomBindings: opts.roomBindings, }); } diff --git a/src/db.test.ts b/src/db.test.ts index 5b497b2..3bc3b53 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -25,7 +25,7 @@ import { deleteTask, getAllChats, getChannelOwnerLease, - getAllRegisteredGroups, + getAllRoomBindings, getDueTasks, getEffectiveRoomMode, getEffectiveRuntimeRoomMode, @@ -1870,7 +1870,7 @@ describe('registered group isMain', () => { isMain: true, }); - const groups = getAllRegisteredGroups(); + const groups = getAllRoomBindings(); const group = groups['dc:main']; expect(group).toBeDefined(); expect(group.isMain).toBe(true); @@ -1885,7 +1885,7 @@ describe('registered group isMain', () => { added_at: '2024-01-01T00:00:00.000Z', }); - const groups = getAllRegisteredGroups(); + const groups = getAllRoomBindings(); const group = groups['group@g.us']; expect(group).toBeDefined(); expect(group.isMain).toBeUndefined(); @@ -1907,8 +1907,8 @@ describe('registered group isMain', () => { agentType: 'codex', }); - const claudeGroups = getAllRegisteredGroups('claude-code'); - const codexGroups = getAllRegisteredGroups('codex'); + const claudeGroups = getAllRoomBindings('claude-code'); + const codexGroups = getAllRoomBindings('codex'); expect(claudeGroups['dc:shared']?.agentType).toBe('claude-code'); expect(claudeGroups['dc:shared']?.name).toBe('Shared Room'); @@ -1946,9 +1946,9 @@ describe('room assignment writes', () => { folder: 'assigned-room', }); - const allGroups = getAllRegisteredGroups(); - const claudeGroups = getAllRegisteredGroups('claude-code'); - const codexGroups = getAllRegisteredGroups('codex'); + const allGroups = getAllRoomBindings(); + const claudeGroups = getAllRoomBindings('claude-code'); + const codexGroups = getAllRoomBindings('codex'); expect(allGroups['dc:assigned-room']).toMatchObject({ name: 'Assigned Room', @@ -1984,19 +1984,17 @@ describe('room assignment writes', () => { ownerAgentType: 'codex', }); expect( - getAllRegisteredGroups('claude-code')['dc:projection-room'], + getAllRoomBindings('claude-code')['dc:projection-room'], ).toMatchObject({ name: 'Projection Room Renamed', folder: 'projection-room', agentType: 'claude-code', }); - expect(getAllRegisteredGroups('codex')['dc:projection-room']).toMatchObject( - { - name: 'Projection Room Renamed', - folder: 'projection-room', - agentType: 'codex', - }, - ); + expect(getAllRoomBindings('codex')['dc:projection-room']).toMatchObject({ + name: 'Projection Room Renamed', + folder: 'projection-room', + agentType: 'codex', + }); }); it('does not carry an owner override config onto a different owner agent type', () => { @@ -2017,19 +2015,17 @@ describe('room assignment writes', () => { folder: 'owner-switch', }); - expect( - getAllRegisteredGroups('claude-code')['dc:owner-switch'], - ).toMatchObject({ + expect(getAllRoomBindings('claude-code')['dc:owner-switch']).toMatchObject({ agentType: 'claude-code', }); expect( - getAllRegisteredGroups('claude-code')['dc:owner-switch']?.agentConfig, + getAllRoomBindings('claude-code')['dc:owner-switch']?.agentConfig, ).toBeUndefined(); - expect(getAllRegisteredGroups('codex')['dc:owner-switch']).toMatchObject({ + expect(getAllRoomBindings('codex')['dc:owner-switch']).toMatchObject({ agentType: 'codex', }); expect( - getAllRegisteredGroups('codex')['dc:owner-switch']?.agentConfig, + getAllRoomBindings('codex')['dc:owner-switch']?.agentConfig, ).toBeUndefined(); }); @@ -2057,14 +2053,14 @@ describe('room assignment writes', () => { expect(projectionRows).toEqual([]); expect( - getAllRegisteredGroups('claude-code')['dc:assign-no-projection'], + getAllRoomBindings('claude-code')['dc:assign-no-projection'], ).toMatchObject({ name: 'Assign No Projection', folder: 'assign-no-projection', agentType: 'claude-code', }); expect( - getAllRegisteredGroups('codex')['dc:assign-no-projection'], + getAllRoomBindings('codex')['dc:assign-no-projection'], ).toMatchObject({ name: 'Assign No Projection', folder: 'assign-no-projection', @@ -2072,7 +2068,7 @@ describe('room assignment writes', () => { }); }); - it('recreates inferred room_settings when renaming a legacy projection-only room', () => { + it('does not recreate inferred room_settings when renaming a legacy projection-only room', () => { _setRegisteredGroupForTests('dc:legacy-rename', { name: 'Legacy Rename', folder: 'legacy-rename', @@ -2093,23 +2089,12 @@ describe('room assignment writes', () => { updateRegisteredGroupName('dc:legacy-rename', 'Legacy Rename Updated'); - expect(getStoredRoomSettings('dc:legacy-rename')).toMatchObject({ - chatJid: 'dc:legacy-rename', - roomMode: 'tribunal', - modeSource: 'inferred', - name: 'Legacy Rename Updated', - folder: 'legacy-rename', - }); + expect(getStoredRoomSettings('dc:legacy-rename')).toBeUndefined(); + expect(getRegisteredGroup('dc:legacy-rename')).toBeUndefined(); expect( - getAllRegisteredGroups('claude-code')['dc:legacy-rename'], - ).toMatchObject({ - name: 'Legacy Rename Updated', - agentType: 'claude-code', - }); - expect(getAllRegisteredGroups('codex')['dc:legacy-rename']).toMatchObject({ - name: 'Legacy Rename Updated', - agentType: 'codex', - }); + getAllRoomBindings('claude-code')['dc:legacy-rename'], + ).toBeUndefined(); + expect(getAllRoomBindings('codex')['dc:legacy-rename']).toBeUndefined(); }); it('requires explicit migration before init when only legacy registered_groups rows exist', () => { @@ -2169,6 +2154,36 @@ describe('room assignment writes', () => { expect(() => _initTestDatabaseFromFile(dbPath)).toThrow( /Legacy room migration required before startup/, ); + const rawDbBeforeMigration = new Database(dbPath, { readonly: true }); + expect( + rawDbBeforeMigration + .prepare( + `SELECT COUNT(*) as count + FROM room_settings`, + ) + .get(), + ).toEqual({ count: 0 }); + expect( + rawDbBeforeMigration + .prepare( + `SELECT COUNT(*) as count + FROM room_role_overrides`, + ) + .get(), + ).toEqual({ count: 0 }); + expect( + rawDbBeforeMigration + .prepare( + `SELECT jid, agent_type + FROM registered_groups + ORDER BY jid, agent_type`, + ) + .all(), + ).toEqual([ + { jid: 'dc:legacy-sql', agent_type: 'claude-code' }, + { jid: 'dc:legacy-sql', agent_type: 'codex' }, + ]); + rawDbBeforeMigration.close(); expect(migrateLegacyRoomRegistrationsInFile(dbPath)).toEqual({ migratedRooms: 1, migratedRoleOverrides: 2, @@ -2546,9 +2561,7 @@ describe('room assignment writes', () => { agentType: 'codex', }); expect(getRegisteredGroup('dc:ssot-room', 'claude-code')).toBeUndefined(); - expect( - getAllRegisteredGroups('claude-code')['dc:ssot-room'], - ).toBeUndefined(); + expect(getAllRoomBindings('claude-code')['dc:ssot-room']).toBeUndefined(); expect(getRegisteredAgentTypesForJid('dc:ssot-room')).toEqual(['codex']); }); @@ -2712,45 +2725,55 @@ describe('room assignment writes', () => { }); describe('paired room registration', () => { - it('detects when both Claude and Codex are registered on the same jid', () => { - _setRegisteredGroupForTests('dc:123', { + it('detects paired capability types from canonical tribunal room settings', () => { + assignRoom('dc:123', { name: 'Paired Room', + roomMode: 'tribunal', + ownerAgentType: 'codex', folder: 'paired-room', - trigger: '@Andy', - added_at: '2024-01-01T00:00:00.000Z', - agentType: 'claude-code', - }); - _setRegisteredGroupForTests('dc:123', { - name: 'Paired Room', - folder: 'paired-room', - trigger: '@Codex', - added_at: '2024-01-01T00:00:00.000Z', - agentType: 'codex', }); expect(getRegisteredAgentTypesForJid('dc:123').sort()).toEqual([ 'claude-code', 'codex', ]); - expect(getExplicitRoomMode('dc:123')).toBeUndefined(); + expect(getExplicitRoomMode('dc:123')).toBe('tribunal'); expect(getEffectiveRoomMode('dc:123')).toBe('tribunal'); expect(getEffectiveRuntimeRoomMode('dc:123')).toBe('tribunal'); }); - it('does not mark solo rooms as paired', () => { - _setRegisteredGroupForTests('dc:solo', { + it('does not mark canonical single rooms as paired', () => { + assignRoom('dc:solo', { name: 'Solo Claude Room', + roomMode: 'single', + ownerAgentType: 'claude-code', folder: 'solo-claude', - trigger: '@Andy', - added_at: '2024-01-01T00:00:00.000Z', - agentType: 'claude-code', }); expect(getRegisteredAgentTypesForJid('dc:solo')).toEqual(['claude-code']); expect(getEffectiveRuntimeRoomMode('dc:solo')).toBe('single'); }); - it('keeps inferred room mode available when no explicit override exists', () => { + it('keeps canonical inferred room mode available when no explicit override exists', () => { + assignRoom('dc:canonical-inferred-paired', { + name: 'Canonical Inferred Paired', + roomMode: 'tribunal', + ownerAgentType: 'codex', + folder: 'canonical-inferred-paired', + }); + + clearExplicitRoomMode('dc:canonical-inferred-paired'); + + expect(getExplicitRoomMode('dc:canonical-inferred-paired')).toBeUndefined(); + expect(getEffectiveRoomMode('dc:canonical-inferred-paired')).toBe( + 'tribunal', + ); + expect(getEffectiveRuntimeRoomMode('dc:canonical-inferred-paired')).toBe( + 'tribunal', + ); + }); + + it('ignores legacy capability rows when canonical room settings are missing', () => { _setRegisteredGroupForTests('dc:legacy-paired', { name: 'Legacy Paired', folder: 'legacy-paired', @@ -2765,10 +2788,12 @@ describe('paired room registration', () => { added_at: '2024-01-01T00:00:00.000Z', agentType: 'codex', }); + _deleteStoredRoomSettingsForTests('dc:legacy-paired'); + expect(getRegisteredAgentTypesForJid('dc:legacy-paired')).toEqual([]); expect(getExplicitRoomMode('dc:legacy-paired')).toBeUndefined(); - expect(getEffectiveRoomMode('dc:legacy-paired')).toBe('tribunal'); - expect(getEffectiveRuntimeRoomMode('dc:legacy-paired')).toBe('tribunal'); + expect(getEffectiveRoomMode('dc:legacy-paired')).toBe('single'); + expect(getEffectiveRuntimeRoomMode('dc:legacy-paired')).toBe('single'); }); it('keeps room-level metadata synced on setRegisteredGroup helper writes', () => { @@ -2901,13 +2926,12 @@ describe('paired room registration', () => { expect(getEffectiveRuntimeRoomMode('dc:explicit-tribunal')).toBe('single'); }); - it('trusts stored tribunal mode even when legacy capability rows are incomplete', () => { - _setRegisteredGroupForTests('dc:explicit-tribunal-codex', { + it('trusts stored tribunal mode without projection rows', () => { + assignRoom('dc:explicit-tribunal-codex', { name: 'Explicit Tribunal Codex', + roomMode: 'single', + ownerAgentType: 'codex', folder: 'explicit-tribunal-codex', - trigger: '@Codex', - added_at: '2024-01-01T00:00:00.000Z', - agentType: 'codex', }); setExplicitRoomMode('dc:explicit-tribunal-codex', 'tribunal'); @@ -3067,8 +3091,6 @@ describe('service handoff completion', () => { roomMode: 'tribunal', ownerAgentType: 'codex', folder: 'owner-handoff-shadow', - trigger: '@Owner', - requiresTrigger: true, }); const handoff = createServiceHandoff({ diff --git a/src/db.ts b/src/db.ts index 0f7470a..98e3779 100644 --- a/src/db.ts +++ b/src/db.ts @@ -3,7 +3,6 @@ import fs from 'fs'; import path from 'path'; import { - ASSISTANT_NAME, ARBITER_AGENT_TYPE, CLAUDE_SERVICE_ID, CODEX_MAIN_SERVICE_ID, @@ -11,7 +10,6 @@ import { DATA_DIR, normalizeServiceId, OWNER_AGENT_TYPE, - REVIEWER_AGENT_TYPE, SERVICE_ID, SERVICE_SESSION_SCOPE, } from './config.js'; @@ -31,7 +29,6 @@ import { buildLegacyRoomMigrationPlan, collectRegisteredAgentTypes, collectRegisteredAgentTypesForFolder, - collectRoomRegistrationSnapshot, deleteLegacyRegisteredGroupRowsForJid, getPendingLegacyRegisteredGroupJids as getPendingLegacyRegisteredGroupJidsFromDatabase, getStoredRoomRowsFromDatabase, @@ -223,8 +220,6 @@ export interface AssignRoomInput { roomMode?: RoomMode; ownerAgentType?: AgentType; folder?: string; - trigger?: string; - requiresTrigger?: boolean; isMain?: boolean; workDir?: string; addedAt?: string; @@ -791,9 +786,9 @@ function writeLegacyRegisteredGroupAndSyncRoomSettings( triggerPattern: existingStored?.modeSource === 'explicit' && existingStored.trigger ? existingStored.trigger - : group.trigger, + : (group.trigger ?? ''), requiresTrigger: - group.requiresTrigger ?? existingStored?.requiresTrigger ?? true, + group.requiresTrigger ?? existingStored?.requiresTrigger ?? false, isMain: group.isMain ?? existingStored?.isMain ?? false, ownerAgentType, workDir: group.workDir ?? existingStored?.workDir ?? null, @@ -885,8 +880,8 @@ export function assignRoom( const snapshot: RoomRegistrationSnapshot = { name: input.name, folder, - triggerPattern: input.trigger || existing?.trigger || `@${ASSISTANT_NAME}`, - requiresTrigger: input.requiresTrigger ?? existing?.requiresTrigger ?? true, + triggerPattern: existing?.trigger ?? '', + requiresTrigger: existing?.requiresTrigger ?? false, isMain: input.isMain ?? existing?.isMain ?? false, ownerAgentType, workDir: input.workDir ?? existing?.workDir ?? null, @@ -959,7 +954,7 @@ export function updateRegisteredGroupName(jid: string, name: string): void { })(); } -export function getAllRegisteredGroups( +export function getAllRoomBindings( agentTypeFilter?: string, ): Record { const result: Record = {}; @@ -984,21 +979,7 @@ export function getAllRegisteredGroups( export function getRegisteredAgentTypesForJid(jid: string): AgentType[] { if (!db) return []; const stored = getStoredRoomSettingsRowFromDatabase(db, jid); - if (stored) { - return resolveStoredRoomCapabilityTypes(db, stored); - } - return collectRegisteredAgentTypes(db, jid); -} - -/** - * Internal registration/backfill helper. - * This infers the stored room mode from current registrations and must not be - * treated as runtime source-of-truth by callers. - */ -function inferStoredRoomModeForJid(jid: string): RoomMode { - return inferRoomModeFromRegisteredAgentTypes( - getRegisteredAgentTypesForJid(jid), - ); + return stored ? resolveStoredRoomCapabilityTypes(db, stored) : []; } export function getStoredRoomSettings( @@ -1018,31 +999,6 @@ function getStoredRoomModeRow(chatJid: string): StoredRoomModeRow | undefined { : undefined; } -function syncStoredRoomRegistrationSnapshotForJid(chatJid: string): void { - const existingSettings = getStoredRoomSettingsRowFromDatabase(db, chatJid); - const snapshot = collectRoomRegistrationSnapshot( - db, - chatJid, - existingSettings, - ); - if (!snapshot) return; - - if (!existingSettings) { - insertStoredRoomSettings( - db, - chatJid, - inferRoomModeFromRegisteredAgentTypes( - getRegisteredAgentTypesForJid(chatJid), - ), - 'inferred', - snapshot, - ); - return; - } - - updateStoredRoomMetadata(db, chatJid, snapshot); -} - function assertNoPendingLegacyRoomMigration(): void { const pendingLegacyRows = countPendingLegacyRegisteredGroupRows(db); const hasLegacyRegisteredGroupsJson = fs.existsSync( @@ -1096,8 +1052,8 @@ function buildRoomRegistrationSnapshotFromStoredRoom( return { name, folder: resolveAssignedRoomFolder(db, stored.chatJid, name, stored.folder), - triggerPattern: stored.trigger || `@${ASSISTANT_NAME}`, - requiresTrigger: stored.requiresTrigger ?? true, + triggerPattern: stored.trigger ?? '', + requiresTrigger: stored.requiresTrigger ?? false, isMain: stored.isMain ?? false, ownerAgentType, workDir: stored.workDir ?? null, @@ -1123,19 +1079,7 @@ function buildRoomRegistrationPlanForJid( }; } - const snapshot = collectRoomRegistrationSnapshot(db, chatJid); - if (!snapshot) return undefined; - - return { - snapshot: { - ...snapshot, - name: overrides?.name ?? snapshot.name, - }, - roomMode: inferRoomModeFromRegisteredAgentTypes( - collectRegisteredAgentTypes(db, chatJid), - ), - hasStoredRoom: false, - }; + return undefined; } function upsertStoredRoomMode( @@ -1157,10 +1101,6 @@ function upsertStoredRoomMode( ).run(chatJid, roomMode, source, new Date().toISOString()); } -function syncInferredRoomModeForJid(chatJid: string): void { - upsertStoredRoomMode(chatJid, inferStoredRoomModeForJid(chatJid), 'inferred'); -} - export function getExplicitRoomMode(chatJid: string): RoomMode | undefined { const row = getStoredRoomModeRow(chatJid); return row?.source === 'explicit' ? row.roomMode : undefined; @@ -1193,28 +1133,8 @@ export function getEffectiveRoomMode(chatJid: string): RoomMode { return getStoredRoomModeRow(chatJid)?.roomMode ?? 'single'; } -function canRunTribunalFromRegisteredAgentTypes( - agentTypes: readonly AgentType[], -): boolean { - const types = new Set(agentTypes); - if (types.size === 0) return false; - return REVIEWER_AGENT_TYPE === 'claude-code' - ? types.has('claude-code') - : types.has('codex'); -} - export function getEffectiveRuntimeRoomMode(chatJid: string): RoomMode { - const stored = getStoredRoomSettings(chatJid); - if (stored) { - return stored.roomMode; - } - - return inferStoredRoomModeForJid(chatJid) === 'tribunal' && - canRunTribunalFromRegisteredAgentTypes( - getRegisteredAgentTypesForJid(chatJid), - ) - ? 'tribunal' - : 'single'; + return getStoredRoomSettings(chatJid)?.roomMode ?? 'single'; } // --- Paired task/project/workspace state --- diff --git a/src/db/room-registration.ts b/src/db/room-registration.ts index 689bdb1..b3d0cae 100644 --- a/src/db/room-registration.ts +++ b/src/db/room-registration.ts @@ -1,10 +1,6 @@ import { Database } from 'bun:sqlite'; -import { - ASSISTANT_NAME, - OWNER_AGENT_TYPE, - REVIEWER_AGENT_TYPE, -} from '../config.js'; +import { OWNER_AGENT_TYPE, REVIEWER_AGENT_TYPE } from '../config.js'; import { isValidGroupFolder } from '../group-folder.js'; import { logger } from '../logger.js'; import type { AgentType, RegisteredGroup, RoomMode } from '../types.js'; @@ -86,6 +82,10 @@ export function normalizeStoredAgentType( : undefined; } +/** + * Legacy capability-row helper for migration/write flows. + * Runtime read paths must derive capability types from canonical room tables. + */ export function collectRegisteredAgentTypes( database: Database, jid: string, @@ -154,73 +154,14 @@ export function parseRegisteredGroupRow( jid: row.jid, name: row.name, folder: row.folder, - trigger: row.trigger_pattern, added_at: row.added_at, agentConfig: row.agent_config ? JSON.parse(row.agent_config) : undefined, - requiresTrigger: - row.requires_trigger === null ? undefined : row.requires_trigger === 1, isMain: row.is_main === 1 ? true : undefined, agentType: normalizeStoredAgentType(row.agent_type), workDir: row.work_dir || undefined, }; } -export function getLegacyRegisteredGroupRows( - database: Database, - agentTypeFilter?: string, -): RegisteredGroupDatabaseRow[] { - return ( - agentTypeFilter - ? database - .prepare( - `SELECT * - FROM registered_groups - WHERE agent_type = ? - AND NOT EXISTS ( - SELECT 1 - FROM room_settings - WHERE chat_jid = registered_groups.jid - )`, - ) - .all(agentTypeFilter) - : database - .prepare( - `SELECT * - FROM registered_groups - WHERE NOT EXISTS ( - SELECT 1 - FROM room_settings - WHERE chat_jid = registered_groups.jid - )`, - ) - .all() - ) as RegisteredGroupDatabaseRow[]; -} - -export function getLegacyRegisteredGroup( - database: Database, - jid: string, - agentType?: string, -): (RegisteredGroup & { jid: string }) | undefined { - if (getStoredRoomSettingsRowFromDatabase(database, jid)) { - return undefined; - } - - const row = ( - agentType - ? database - .prepare( - 'SELECT * FROM registered_groups WHERE jid = ? AND agent_type = ?', - ) - .get(jid, agentType) - : database - .prepare('SELECT * FROM registered_groups WHERE jid = ?') - .get(jid) - ) as RegisteredGroupDatabaseRow | undefined; - - return parseRegisteredGroupRow(row); -} - export function getPendingLegacyRegisteredGroupJids( database: Database, ): string[] { @@ -605,10 +546,8 @@ export function buildRegisteredGroupFromStoredSettings( jid: stored.chatJid, name: stored.name || stored.chatJid, folder: stored.folder, - trigger: stored.trigger || `@${ASSISTANT_NAME}`, added_at: capabilityMetadata?.createdAt || new Date(0).toISOString(), agentConfig: capabilityMetadata?.agentConfig, - requiresTrigger: stored.requiresTrigger, isMain: stored.isMain ? true : undefined, agentType: resolvedAgentType, workDir: stored.workDir, @@ -855,6 +794,10 @@ export function materializeRegisteredGroupsForRoom( .run(chatJid, ...desiredTypes); } +/** + * Legacy snapshot helper for migration/write flows. + * Runtime read paths must not rebuild room metadata from registered_groups. + */ export function collectRoomRegistrationSnapshot( database: Database, jid: string, diff --git a/src/db/schema.ts b/src/db/schema.ts index 020ed68..de17b7c 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -2989,7 +2989,7 @@ export function applySchemaMigrations( WHERE status IN ('produced', 'delivery_retry'); `); - const registeredGroupsSql = ( + const roomBindingsSql = ( database .prepare( `SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'registered_groups'`, @@ -2997,8 +2997,8 @@ export function applySchemaMigrations( .get() as { sql?: string } | undefined )?.sql; if ( - registeredGroupsSql && - !registeredGroupsSql.includes('PRIMARY KEY (jid, agent_type)') + roomBindingsSql && + !roomBindingsSql.includes('PRIMARY KEY (jid, agent_type)') ) { const registeredGroupCols = database .prepare('PRAGMA table_info(registered_groups)') diff --git a/src/formatting.test.ts b/src/formatting.test.ts index ad24eec..59d3c2c 100644 --- a/src/formatting.test.ts +++ b/src/formatting.test.ts @@ -319,55 +319,3 @@ describe('formatOutbound with tool-call leaks', () => { expect(formatOutbound(input)).toBe('Key: [REDACTED]'); }); }); - -// --- Trigger gating with requiresTrigger flag --- - -describe('trigger gating (requiresTrigger interaction)', () => { - // Replicates the exact logic from processGroupMessages and startMessageLoop: - // if (!isMainGroup && group.requiresTrigger !== false) { check trigger } - function shouldRequireTrigger( - isMainGroup: boolean, - requiresTrigger: boolean | undefined, - ): boolean { - return !isMainGroup && requiresTrigger !== false; - } - - function shouldProcess( - isMainGroup: boolean, - requiresTrigger: boolean | undefined, - messages: NewMessage[], - ): boolean { - if (!shouldRequireTrigger(isMainGroup, requiresTrigger)) return true; - return messages.some((m) => TRIGGER_PATTERN.test(m.content.trim())); - } - - it('main group always processes (no trigger needed)', () => { - const msgs = [makeMsg({ content: 'hello no trigger' })]; - expect(shouldProcess(true, undefined, msgs)).toBe(true); - }); - - it('main group processes even with requiresTrigger=true', () => { - const msgs = [makeMsg({ content: 'hello no trigger' })]; - expect(shouldProcess(true, true, msgs)).toBe(true); - }); - - it('non-main group with requiresTrigger=undefined requires trigger (defaults to true)', () => { - const msgs = [makeMsg({ content: 'hello no trigger' })]; - expect(shouldProcess(false, undefined, msgs)).toBe(false); - }); - - it('non-main group with requiresTrigger=true requires trigger', () => { - const msgs = [makeMsg({ content: 'hello no trigger' })]; - expect(shouldProcess(false, true, msgs)).toBe(false); - }); - - it('non-main group with requiresTrigger=true processes when trigger present', () => { - const msgs = [makeMsg({ content: `@${ASSISTANT_NAME} do something` })]; - expect(shouldProcess(false, true, msgs)).toBe(true); - }); - - it('non-main group with requiresTrigger=false always processes (no trigger needed)', () => { - const msgs = [makeMsg({ content: 'hello no trigger' })]; - expect(shouldProcess(false, false, msgs)).toBe(true); - }); -}); diff --git a/src/host-evidence-ipc.test.ts b/src/host-evidence-ipc.test.ts index bd8826d..d6afb03 100644 --- a/src/host-evidence-ipc.test.ts +++ b/src/host-evidence-ipc.test.ts @@ -40,7 +40,7 @@ describe('host evidence IPC', () => { deps = { sendMessage: async () => {}, - registeredGroups: () => ({ 'other@g.us': OTHER_GROUP }), + roomBindings: () => ({ 'other@g.us': OTHER_GROUP }), assignRoom: () => {}, syncGroups: async () => {}, getAvailableGroups: () => [], diff --git a/src/index.ts b/src/index.ts index b73b2f7..cfe736e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -24,7 +24,7 @@ import { listAvailableGroups } from './available-groups.js'; import { type AssignRoomInput, assignRoom, - getAllRegisteredGroups, + getAllRoomBindings, getAllSessions, getAllTasks, getLatestMessageSeqAtOrBefore, @@ -139,7 +139,7 @@ export async function editFormattedTrackedChannelMessage( let lastTimestamp = ''; let sessions: Record = {}; -let registeredGroups: Record = {}; +let roomBindings: Record = {}; let lastAgentTimestamp: Record = {}; const channels: Channel[] = []; @@ -152,7 +152,7 @@ const runtime = createMessageRuntime({ triggerPattern: TRIGGER_PATTERN, channels, queue, - getRegisteredGroups: () => registeredGroups, + getRoomBindings: () => roomBindings, getSessions: () => sessions, getLastTimestamp: () => lastTimestamp, setLastTimestamp: (timestamp) => { @@ -185,10 +185,10 @@ function loadState(): void { lastAgentTimestamp = {}; } sessions = getAllSessions(); - registeredGroups = getAllRegisteredGroups(); + roomBindings = getAllRoomBindings(); logger.info( { - groupCount: Object.keys(registeredGroups).length, + groupCount: Object.keys(roomBindings).length, agentType: 'unified', }, 'State loaded', @@ -231,7 +231,7 @@ function assignRoomForIpc(jid: string, input: AssignRoomInput): void { } const { jid: _ignoredJid, ...storedGroup } = assignedGroup; - registeredGroups[jid] = storedGroup; + roomBindings[jid] = storedGroup; fs.mkdirSync(path.join(groupDir, 'logs'), { recursive: true }); } @@ -240,14 +240,21 @@ function assignRoomForIpc(jid: string, input: AssignRoomInput): void { * Returns groups ordered by most recent activity. */ export function getAvailableGroups(): import('./agent-runner.js').AvailableGroup[] { - return listAvailableGroups(registeredGroups); + return listAvailableGroups(roomBindings); } /** @internal - exported for testing */ export function _setRegisteredGroups( groups: Record, ): void { - registeredGroups = groups; + roomBindings = groups; +} + +/** @internal - exported for testing */ +export function _setRoomBindings( + groups: Record, +): void { + roomBindings = groups; } async function announceRestartRecovery( @@ -288,10 +295,7 @@ async function announceRestartRecovery( return explicitContext; } - const inferred = inferRecentRestartContext( - registeredGroups, - processStartedAtMs, - ); + const inferred = inferRecentRestartContext(roomBindings, processStartedAtMs); if (!inferred) return null; if (hasRecentRestartAnnouncement(inferred.chatJid, dedupeSince)) { @@ -334,7 +338,7 @@ async function main(): Promise { leaseRecoveryTimer = null; } const interruptedGroups = queue - .getStatuses(Object.keys(registeredGroups)) + .getStatuses(Object.keys(roomBindings)) .filter( ( status, @@ -344,14 +348,14 @@ async function main(): Promise { ) .map((status) => ({ chatJid: status.jid, - groupName: registeredGroups[status.jid]?.name || status.jid, + groupName: roomBindings[status.jid]?.name || status.jid, status: status.status, elapsedMs: status.elapsedMs, pendingMessages: status.pendingMessages, pendingTasks: status.pendingTasks, })); const writtenPaths = writeShutdownRestartContext( - registeredGroups, + roomBindings, interruptedGroups, signal, ); @@ -376,7 +380,7 @@ async function main(): Promise { const channelOpts = { onMessage: (chatJid: string, msg: NewMessage) => { // Sender allowlist drop mode: discard messages from denied senders before storing - if (!msg.is_from_me && !msg.is_bot_message && registeredGroups[chatJid]) { + if (!msg.is_from_me && !msg.is_bot_message && roomBindings[chatJid]) { const cfg = loadSenderAllowlist(); if ( shouldDropMessage(chatJid, cfg) && @@ -400,7 +404,7 @@ async function main(): Promise { channel?: string, isGroup?: boolean, ) => storeChatMetadata(chatJid, timestamp, name, channel, isGroup), - registeredGroups: () => registeredGroups, + roomBindings: () => roomBindings, }; // Create and connect all registered channels. @@ -433,7 +437,7 @@ async function main(): Promise { ); startSchedulerLoop({ - registeredGroups: () => registeredGroups, + roomBindings: () => roomBindings, getSessions: () => sessions, queue, onProcess: (groupJid, proc, processName, ipcDir) => @@ -476,7 +480,7 @@ async function main(): Promise { } }, nudgeScheduler: nudgeSchedulerLoop, - registeredGroups: () => registeredGroups, + roomBindings: () => roomBindings, assignRoom: assignRoomForIpc, syncGroups: async (force: boolean) => { await Promise.all( @@ -494,7 +498,7 @@ async function main(): Promise { const restartContext = await announceRestartRecovery(processStartedAtMs); for (const candidate of getInterruptedRecoveryCandidates( restartContext, - registeredGroups, + roomBindings, )) { queue.enqueueMessageCheck( candidate.chatJid, @@ -520,9 +524,9 @@ async function main(): Promise { usageUpdateInterval: USAGE_UPDATE_INTERVAL, channels, queue, - registeredGroups: () => registeredGroups, + roomBindings: () => roomBindings, onGroupNameSynced: (jid, name) => { - const group = registeredGroups[jid]; + const group = roomBindings[jid]; if (group) { group.name = name; } diff --git a/src/ipc-auth.test.ts b/src/ipc-auth.test.ts index 0d03bc6..adf3ab4 100644 --- a/src/ipc-auth.test.ts +++ b/src/ipc-auth.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { ASSISTANT_NAME } from './config.js'; import { _setRegisteredGroupForTests, _initTestDatabase, @@ -60,7 +59,7 @@ beforeEach(() => { deps = { sendMessage: async () => {}, nudgeScheduler: vi.fn(), - registeredGroups: () => groups, + roomBindings: () => groups, assignRoom: (jid, room) => { const assigned = assignRoom(jid, room); if (!assigned) return; @@ -440,14 +439,13 @@ describe('assign_room authorization', () => { jid: 'new@g.us', name: 'New Group', folder: 'new-group', - trigger: '@Andy', }, 'other-group', false, deps, ); - // registeredGroups should not have changed + // roomBindings should not have changed expect(groups['new@g.us']).toBeUndefined(); }); @@ -458,7 +456,6 @@ describe('assign_room authorization', () => { jid: 'new@g.us', name: 'New Group', folder: '../../outside', - trigger: '@Andy', }, 'whatsapp_main', true, @@ -494,9 +491,9 @@ describe('IPC message authorization', () => { sourceGroup: string, isMain: boolean, targetChatJid: string, - registeredGroups: Record, + roomBindings: Record, ): boolean { - const targetGroup = registeredGroups[targetChatJid]; + const targetGroup = roomBindings[targetChatJid]; return isMain || (!!targetGroup && targetGroup.folder === sourceGroup); } @@ -885,7 +882,6 @@ describe('assign_room success', () => { jid: 'new@g.us', name: 'New Group', folder: 'new-group', - trigger: '@Andy', }, 'whatsapp_main', true, @@ -896,10 +892,9 @@ describe('assign_room success', () => { expect(group).toBeDefined(); expect(group!.name).toBe('New Group'); expect(group!.folder).toBe('new-group'); - expect(group!.trigger).toBe('@Andy'); }); - it('assign_room auto-fills missing folder and trigger for single rooms', async () => { + it('assign_room auto-fills missing folder for single rooms without exposing trigger metadata', async () => { await processTaskIpc( { type: 'assign_room', @@ -914,7 +909,6 @@ describe('assign_room success', () => { const group = getRegisteredGroup('partial@g.us'); expect(group).toBeDefined(); expect(group!.folder).toMatch(/^grp_whatsapp_/); - expect(group!.trigger).toBe(`@${ASSISTANT_NAME}`); expect(getStoredRoomSettings('partial@g.us')?.roomMode).toBe('single'); }); diff --git a/src/ipc.ts b/src/ipc.ts index 4c25b49..a26ff5b 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -35,7 +35,7 @@ export interface IpcDeps { senderRole?: string, ) => Promise; nudgeScheduler?: () => void; - registeredGroups: () => Record; + roomBindings: () => Record; assignRoom: (jid: string, room: AssignRoomInput) => void; syncGroups: (force: boolean) => Promise; getAvailableGroups: () => AvailableGroup[]; @@ -65,14 +65,14 @@ export async function forwardAuthorizedIpcMessage( msg: IpcMessagePayload, sourceGroup: string, isMain: boolean, - registeredGroups: Record, + roomBindings: Record, sendMessage: IpcDeps['sendMessage'], ): Promise { if (!(msg.type === 'message' && msg.chatJid && msg.text)) { return { outcome: 'ignored', senderRole: msg.senderRole ?? null }; } - const targetGroup = registeredGroups[msg.chatJid]; + const targetGroup = roomBindings[msg.chatJid]; const isMainOverride = isMain === true; if ( !(isMainOverride || (targetGroup && targetGroup.folder === sourceGroup)) @@ -184,11 +184,11 @@ export function startIpcWatcher(deps: IpcDeps): void { return; } - const registeredGroups = deps.registeredGroups(); + const roomBindings = deps.roomBindings(); // Build folder→isMain lookup from registered groups const folderIsMain = new Map(); - for (const group of Object.values(registeredGroups)) { + for (const group of Object.values(roomBindings)) { if (group.isMain) folderIsMain.set(group.folder, true); } @@ -240,7 +240,7 @@ export function startIpcWatcher(deps: IpcDeps): void { msg, sourceGroup, isMain, - registeredGroups, + roomBindings, deps.sendMessage, ); if (forwardResult.outcome === 'sent') { @@ -353,8 +353,6 @@ export async function processTaskIpc( jid?: string; name?: string; folder?: string; - trigger?: string; - requiresTrigger?: boolean; room_mode?: RoomMode; owner_agent_type?: AgentType; isMain?: boolean; @@ -376,7 +374,7 @@ export async function processTaskIpc( isMain: boolean, // Verified from directory path deps: IpcDeps, ): Promise { - const registeredGroups = deps.registeredGroups(); + const roomBindings = deps.roomBindings(); switch (data.type) { case 'schedule_task': @@ -389,8 +387,8 @@ export async function processTaskIpc( // Resolve the target group from JID const targetJid = data.targetJid as string; const targetGroupEntry = - registeredGroups[targetJid] || - Object.values(registeredGroups).find( + roomBindings[targetJid] || + Object.values(roomBindings).find( (group) => group.folder === targetJid, ); @@ -404,9 +402,9 @@ export async function processTaskIpc( const targetFolder = targetGroupEntry.folder; const resolvedTargetJid = - registeredGroups[targetJid] !== undefined + roomBindings[targetJid] !== undefined ? targetJid - : Object.entries(registeredGroups).find( + : Object.entries(roomBindings).find( ([, group]) => group.folder === targetFolder, )?.[0]; @@ -765,8 +763,6 @@ export async function processTaskIpc( roomMode: data.room_mode, ownerAgentType: data.owner_agent_type, folder: data.folder, - trigger: data.trigger, - requiresTrigger: data.requiresTrigger, isMain: data.isMain, workDir: data.workDir, }); diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index f782d09..09c94cd 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -67,6 +67,10 @@ export function createPairedExecutionLifecycle(args: { let pairedTurnOutputPersisted = false; let pairedTurnStateFinalized = false; let leaseHeartbeatTimer: ReturnType | null = null; + const requiresVisibleVerdict = + pairedExecutionContext?.requiresVisibleVerdict === true; + const missingVisibleVerdictSummary = + 'Execution completed without a visible terminal verdict.'; const finalizePairedTurnState = ( status: 'succeeded' | 'failed', @@ -254,12 +258,31 @@ export function createPairedExecutionLifecycle(args: { return; } + const missingVisibleVerdict = + requiresVisibleVerdict && + (!pairedFinalOutput || pairedFinalOutput.length === 0); + if (missingVisibleVerdict) { + pairedExecutionSummary = missingVisibleVerdictSummary; + log.warn( + { + pairedTaskId: pairedExecutionContext?.task.id ?? null, + role: completedRole, + runId, + }, + 'Treating paired execution as failed because it ended without a visible terminal verdict', + ); + } const effectiveStatus = completedRole === 'owner' && pairedExecutionStatus === 'succeeded' && !pairedSawOutput ? 'failed' - : pairedExecutionStatus; + : missingVisibleVerdict && pairedExecutionStatus === 'succeeded' + ? 'failed' + : pairedExecutionStatus; + const sawOutputForFollowUp = missingVisibleVerdict + ? false + : pairedSawOutput; if (pairedExecutionContext && !pairedExecutionCompleted) { if (effectiveStatus === 'succeeded') { @@ -283,7 +306,10 @@ export function createPairedExecutionLifecycle(args: { pairedExecutionCompleted = true; } - finalizePairedTurnState(effectiveStatus); + finalizePairedTurnState( + effectiveStatus, + effectiveStatus === 'failed' ? pairedExecutionSummary : null, + ); if (!pairedExecutionContext) { return; @@ -314,7 +340,7 @@ export function createPairedExecutionLifecycle(args: { const queueAction = resolvePairedFollowUpQueueAction({ completedRole, executionStatus: effectiveStatus, - sawOutput: pairedSawOutput, + sawOutput: sawOutputForFollowUp, taskStatus: finishedTask?.status ?? null, }); if (queueAction !== 'pending' || !finishedTask) { @@ -328,8 +354,8 @@ export function createPairedExecutionLifecycle(args: { source: 'executor-recovery', completedRole, executionStatus: effectiveStatus, - sawOutput: pairedSawOutput, - fallbackLastTurnOutputRole: pairedSawOutput ? completedRole : null, + sawOutput: sawOutputForFollowUp, + fallbackLastTurnOutputRole: sawOutputForFollowUp ? completedRole : null, enqueueMessageCheck, }); if (followUpResult.kind !== 'paired-follow-up') { diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 0bd37bb..890a519 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -272,7 +272,7 @@ function makeDeps() { registerProcess: vi.fn(), enqueueMessageCheck: vi.fn(), }, - getRegisteredGroups: () => ({}), + getRoomBindings: () => ({}), getSessions: () => ({}), persistSession: vi.fn(), clearSession: vi.fn(), @@ -981,6 +981,108 @@ describe('runAgentForGroup room memory', () => { ]); }); + it('fails paired reviewer turns that finish without a visible terminal verdict', async () => { + const group = { + ...makeGroup(), + folder: 'test-group', + workDir: '/repo', + agentType: 'codex' as const, + }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_agent_type: 'claude-code', + reviewer_agent_type: 'codex', + arbiter_agent_type: null, + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-review-no-verdict', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: '2026-04-10T00:00:00.000Z', + plan_notes: null, + round_trip_count: 0, + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T00:00:00.000Z', + }, + claimedTaskUpdatedAt: '2026-04-10T00:00:00.000Z', + workspace: null, + envOverrides: {}, + requiresVisibleVerdict: true, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + phase: 'progress', + result: '검토 중입니다.', + output: { visibility: 'public', text: '검토 중입니다.' }, + } as any); + return { + status: 'success', + result: null, + }; + }, + ); + + const result = await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-review-no-verdict', + forcedRole: 'reviewer', + pairedTurnIdentity: { + turnId: + 'paired-task-review-no-verdict:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-review-no-verdict', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + }); + + expect(result).toBe('error'); + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 'paired-task-review-no-verdict', + role: 'reviewer', + status: 'failed', + summary: 'Execution completed without a visible terminal verdict.', + }), + ); + expect(db.failPairedTurn).toHaveBeenCalledWith({ + turnIdentity: { + turnId: + 'paired-task-review-no-verdict:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-review-no-verdict', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + error: 'Execution completed without a visible terminal verdict.', + }); + expect(db.completePairedTurn).not.toHaveBeenCalled(); + expect(db.insertPairedTurnOutput).not.toHaveBeenCalled(); + }); + it('passes paired workspace env overrides into the runner when execution metadata exists', async () => { const group = { ...makeGroup(), diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 970d004..b2f064b 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -69,7 +69,7 @@ import type { AgentType, PairedRoomRole, RegisteredGroup } from './types.js'; export interface MessageAgentExecutorDeps { assistantName: string; queue: Pick; - getRegisteredGroups: () => Record; + getRoomBindings: () => Record; getSessions: () => Record; persistSession: (groupFolder: string, sessionId: string) => void; clearSession: (groupFolder: string) => void; @@ -175,7 +175,7 @@ export async function runAgentForGroup( writeGroupsSnapshot( group.folder, isMain, - listAvailableGroups(deps.getRegisteredGroups()), + listAvailableGroups(deps.getRoomBindings()), ); let resetSessionRequested = false; @@ -194,6 +194,7 @@ export async function runAgentForGroup( runId, roomRoleContext, hasHumanMessage: args.hasHumanMessage, + pairedTurnIdentity: args.pairedTurnIdentity, }); const preparedTurnTaskUpdatedAt = pairedExecutionContext ? (pairedExecutionContext.claimedTaskUpdatedAt ?? diff --git a/src/message-runtime-dispatch.ts b/src/message-runtime-dispatch.ts index 6e97173..62b9660 100644 --- a/src/message-runtime-dispatch.ts +++ b/src/message-runtime-dispatch.ts @@ -8,7 +8,6 @@ import { advanceLastAgentCursor, filterLoopingPairedBotMessages, getProcessableMessages, - hasAllowedTrigger, resolveCursorKey, shouldSkipBotOnlyCollaboration, } from './message-runtime-rules.js'; @@ -296,18 +295,6 @@ export async function processLoopGroupMessages(args: { return; } - if ( - !hasAllowedTrigger({ - chatJid, - messages: processableGroupMessages, - group, - triggerPattern: args.triggerPattern, - hasImplicitContinuationWindow: args.hasImplicitContinuationWindow, - }) - ) { - return; - } - await processQueuedGroupDispatch({ chatJid, group, diff --git a/src/message-runtime-gating.test.ts b/src/message-runtime-gating.test.ts index 886b291..3768541 100644 --- a/src/message-runtime-gating.test.ts +++ b/src/message-runtime-gating.test.ts @@ -1,27 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { RegisteredGroup } from './types.js'; -const { handleSessionCommandMock, hasAllowedTriggerMock, loggerInfoMock } = - vi.hoisted(() => ({ - handleSessionCommandMock: vi.fn(), - hasAllowedTriggerMock: vi.fn(), - loggerInfoMock: vi.fn(), - })); +const { handleSessionCommandMock } = vi.hoisted(() => ({ + handleSessionCommandMock: vi.fn(), +})); vi.mock('./session-commands.js', () => ({ handleSessionCommand: handleSessionCommandMock, })); -vi.mock('./message-runtime-rules.js', () => ({ - hasAllowedTrigger: hasAllowedTriggerMock, -})); - -vi.mock('./logger.js', () => ({ - logger: { - info: loggerInfoMock, - }, -})); - import { handleQueuedRunGates } from './message-runtime-gating.js'; describe('message-runtime-gating', () => { @@ -44,8 +31,6 @@ describe('message-runtime-gating', () => { beforeEach(() => { handleSessionCommandMock.mockReset(); - hasAllowedTriggerMock.mockReset(); - loggerInfoMock.mockReset(); }); it('returns session-command results directly when a command is handled', async () => { @@ -57,29 +42,13 @@ describe('message-runtime-gating', () => { const result = await handleQueuedRunGates(baseArgs); expect(result).toEqual({ handled: true, success: false }); - expect(hasAllowedTriggerMock).not.toHaveBeenCalled(); }); - it('treats missing triggers as a handled no-op', async () => { + it('falls through when no session command is handled', async () => { handleSessionCommandMock.mockResolvedValue({ handled: false }); - hasAllowedTriggerMock.mockReturnValue(false); - - const result = await handleQueuedRunGates(baseArgs); - - expect(result).toEqual({ handled: true, success: true }); - expect(loggerInfoMock).toHaveBeenCalledWith( - { chatJid: 'room-1', group: 'room', runId: 'run-1' }, - 'Skipping queued run because no allowed trigger was found', - ); - }); - - it('falls through when no session command is handled and the trigger is allowed', async () => { - handleSessionCommandMock.mockResolvedValue({ handled: false }); - hasAllowedTriggerMock.mockReturnValue(true); const result = await handleQueuedRunGates(baseArgs); expect(result).toEqual({ handled: false }); - expect(loggerInfoMock).not.toHaveBeenCalled(); }); }); diff --git a/src/message-runtime-gating.ts b/src/message-runtime-gating.ts index e06ab0d..977eea1 100644 --- a/src/message-runtime-gating.ts +++ b/src/message-runtime-gating.ts @@ -1,5 +1,4 @@ import { logger } from './logger.js'; -import { hasAllowedTrigger } from './message-runtime-rules.js'; import { handleSessionCommand, type SessionCommandDeps, @@ -32,21 +31,5 @@ export async function handleQueuedRunGates(args: { return cmdResult; } - if ( - !hasAllowedTrigger({ - chatJid: args.chatJid, - messages: args.missedMessages, - group: args.group, - triggerPattern: args.triggerPattern, - hasImplicitContinuationWindow: args.hasImplicitContinuationWindow, - }) - ) { - logger.info( - { chatJid: args.chatJid, group: args.group.name, runId: args.runId }, - 'Skipping queued run because no allowed trigger was found', - ); - return { handled: true, success: true }; - } - return { handled: false }; } diff --git a/src/message-runtime-handoffs.test.ts b/src/message-runtime-handoffs.test.ts index fa28129..cd82fa1 100644 --- a/src/message-runtime-handoffs.test.ts +++ b/src/message-runtime-handoffs.test.ts @@ -124,7 +124,7 @@ describe('message-runtime-handoffs', () => { completed_at: null, last_error: null, }, - getRegisteredGroups: () => ({ + getRoomBindings: () => ({ 'group@test': makeGroup(), }), channels: [makeChannel('discord-main', true)], @@ -174,7 +174,7 @@ describe('message-runtime-handoffs', () => { completed_at: null, last_error: null, }, - getRegisteredGroups: () => ({ + getRoomBindings: () => ({ 'group@test': makeGroup(), }), channels: [ @@ -249,7 +249,7 @@ describe('message-runtime-handoffs', () => { completed_at: null, last_error: null, }, - getRegisteredGroups: () => ({ + getRoomBindings: () => ({ 'group@test': makeGroup(), }), channels: [ diff --git a/src/message-runtime-handoffs.ts b/src/message-runtime-handoffs.ts index 990dea5..20784bd 100644 --- a/src/message-runtime-handoffs.ts +++ b/src/message-runtime-handoffs.ts @@ -160,7 +160,7 @@ function failClaimedHandoff(args: { export async function processClaimedHandoff(args: { handoff: ServiceHandoff; - getRegisteredGroups: () => Record; + getRoomBindings: () => Record; channels: Channel[]; executeTurn: ExecuteTurnFn; lastAgentTimestamps: Record; @@ -175,7 +175,7 @@ export async function processClaimedHandoff(args: { enqueueMessageCheck?: ((chatJid: string) => void) | undefined; }): Promise { const { handoff } = args; - const group = args.getRegisteredGroups()[handoff.chat_jid]; + const group = args.getRoomBindings()[handoff.chat_jid]; if (!group) { failClaimedHandoff({ handoff, diff --git a/src/message-runtime-loop.ts b/src/message-runtime-loop.ts index a4536fd..ccaf240 100644 --- a/src/message-runtime-loop.ts +++ b/src/message-runtime-loop.ts @@ -27,7 +27,7 @@ export async function processMessageLoopTick(args: { triggerPattern: RegExp; timezone: string; channels: Channel[]; - getRegisteredGroups: () => Record; + getRoomBindings: () => Record; getLastTimestamp: () => string; setLastTimestamp: (timestamp: string) => void; lastAgentTimestamps: Record; @@ -54,8 +54,8 @@ export async function processMessageLoopTick(args: { labelPairedSenders: (chatJid: string, messages: NewMessage[]) => NewMessage[]; }): Promise { args.enqueuePendingHandoffs(); - const registeredGroups = args.getRegisteredGroups(); - const jids = Object.keys(registeredGroups); + const roomBindings = args.getRoomBindings(); + const jids = Object.keys(roomBindings); const { messages, newSeqCursor } = getNewMessagesBySeq( jids, args.getLastTimestamp(), @@ -81,7 +81,7 @@ export async function processMessageLoopTick(args: { } for (const [chatJid, groupMessages] of messagesByGroup) { - const group = registeredGroups[chatJid]; + const group = roomBindings[chatJid]; if (!group) continue; const channel = findChannel(args.channels, chatJid); @@ -125,7 +125,7 @@ export async function processMessageLoopTick(args: { export function recoverPendingMessages(args: { assistantName: string; channels: Channel[]; - getRegisteredGroups: () => Record; + getRoomBindings: () => Record; lastAgentTimestamps: Record; saveState: () => void; enqueueScopedGroupMessageCheck: ( @@ -133,8 +133,8 @@ export function recoverPendingMessages(args: { groupFolder: string, ) => void; }): void { - const registeredGroups = args.getRegisteredGroups(); - for (const [chatJid, group] of Object.entries(registeredGroups)) { + const roomBindings = args.getRoomBindings(); + for (const [chatJid, group] of Object.entries(roomBindings)) { const openWorkItem = getOpenWorkItemForChat(chatJid, SERVICE_SESSION_SCOPE); if (openWorkItem) { logger.info( diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index 4ddcbf5..e55c8d9 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -398,7 +398,7 @@ describe('createMessageRuntime', () => { closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -468,7 +468,7 @@ describe('createMessageRuntime', () => { closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -545,7 +545,7 @@ describe('createMessageRuntime', () => { closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -596,7 +596,7 @@ describe('createMessageRuntime', () => { registerProcess: vi.fn(), closeStdin: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -666,7 +666,7 @@ describe('createMessageRuntime', () => { notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -742,7 +742,7 @@ describe('createMessageRuntime', () => { notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -832,7 +832,7 @@ describe('createMessageRuntime', () => { notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -915,7 +915,7 @@ describe('createMessageRuntime', () => { notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1009,7 +1009,7 @@ describe('createMessageRuntime', () => { notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1130,7 +1130,7 @@ describe('createMessageRuntime', () => { notifyIdle: vi.fn(), enqueueMessageCheck: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1229,7 +1229,7 @@ describe('createMessageRuntime', () => { senderRole === 'reviewer', ), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1320,7 +1320,7 @@ describe('createMessageRuntime', () => { noteDirectTerminalDelivery, hasDirectTerminalDeliveryForRun: vi.fn(() => false), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1397,7 +1397,7 @@ describe('createMessageRuntime', () => { noteDirectTerminalDelivery, hasDirectTerminalDeliveryForRun, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1513,7 +1513,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1619,7 +1619,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1848,7 +1848,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -1976,7 +1976,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2076,7 +2076,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2161,7 +2161,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2254,7 +2254,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2349,7 +2349,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2462,7 +2462,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2578,7 +2578,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2670,7 +2670,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2759,7 +2759,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead notifyIdle: vi.fn(), enqueueMessageCheck: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2823,7 +2823,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2896,7 +2896,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -2966,7 +2966,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3085,7 +3085,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3200,7 +3200,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3219,7 +3219,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead expect(agentRunner.runAgentProcess).toHaveBeenCalledTimes(1); }); - it('allows follow-up messages without a trigger after a visible reply in non-main groups', async () => { + it('processes first and follow-up messages without trigger mentions in non-main groups', async () => { const chatJid = 'group@test'; const group: RegisteredGroup = { ...makeGroup('codex'), @@ -3236,7 +3236,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead chat_jid: chatJid, sender: 'user@test', sender_name: 'User', - content: '@Andy 첫 요청', + content: '첫 요청도 멘션 없이 보냄', timestamp: '2026-03-18T09:00:00.000Z', }, ]) @@ -3278,7 +3278,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3289,7 +3289,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead }); const first = await runtime.processGroupMessages(chatJid, { - runId: 'run-triggered-first-turn', + runId: 'run-triggerless-first-turn', reason: 'messages', }); const second = await runtime.processGroupMessages(chatJid, { @@ -3362,7 +3362,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin, notifyIdle, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3440,7 +3440,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin, notifyIdle, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3527,7 +3527,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3627,7 +3627,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3708,7 +3708,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3810,7 +3810,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin, notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -3921,7 +3921,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4025,7 +4025,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4102,7 +4102,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4163,7 +4163,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4251,7 +4251,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4325,7 +4325,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4431,7 +4431,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4552,7 +4552,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4663,7 +4663,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4743,7 +4743,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4819,7 +4819,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin, notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4902,7 +4902,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -4974,7 +4974,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead closeStdin: vi.fn(), notifyIdle: vi.fn(), } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -5034,7 +5034,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead enqueueMessageCheck, enqueueTask, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), @@ -5094,7 +5094,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead enqueueMessageCheck, enqueueTask, } as any, - getRegisteredGroups: () => ({ [chatJid]: group }), + getRoomBindings: () => ({ [chatJid]: group }), getSessions: () => ({}), getLastTimestamp: () => '', setLastTimestamp: vi.fn(), diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 00b5f92..d17ee62 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -111,7 +111,7 @@ export interface MessageRuntimeDeps { triggerPattern: RegExp; channels: Channel[]; queue: GroupQueue; - getRegisteredGroups: () => Record; + getRoomBindings: () => Record; getSessions: () => Record; getLastTimestamp: () => string; setLastTimestamp: (timestamp: string) => void; @@ -217,7 +217,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { { assistantName: deps.assistantName, queue: deps.queue, - getRegisteredGroups: deps.getRegisteredGroups, + getRoomBindings: deps.getRoomBindings, getSessions: deps.getSessions, persistSession: deps.persistSession, clearSession: deps.clearSession, @@ -272,6 +272,9 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { getEffectiveChannelLease(chatJid), resolvedDeliveryRole ?? 'owner', ); + const allowProgressReplayWithoutFinal = + args.pairedTurnIdentity?.role !== 'reviewer' && + args.pairedTurnIdentity?.role !== 'arbiter'; const turnController = new MessageTurnController({ chatJid, group, @@ -283,6 +286,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { clearSession: () => deps.clearSession(group.folder), requestClose: (reason) => deps.queue.closeStdin(chatJid, { runId, reason }), + allowProgressReplayWithoutFinal, deliveryRole: resolvedDeliveryRole, deliveryServiceId: resolvedDeliveryServiceId, pairedTurnIdentity: args.pairedTurnIdentity ?? null, @@ -426,7 +430,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { processClaimedHandoff: async (handoff) => { await processClaimedServiceHandoff({ handoff, - getRegisteredGroups: deps.getRegisteredGroups, + getRoomBindings: deps.getRoomBindings, channels: deps.channels, executeTurn, lastAgentTimestamps: deps.getLastAgentTimestamps(), @@ -444,7 +448,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { context: GroupRunContext, ): Promise => { const { runId } = context; - const group = deps.getRegisteredGroups()[chatJid]; + const group = deps.getRoomBindings()[chatJid]; if (!group) return true; const log = createScopedLogger({ chatJid, @@ -651,18 +655,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { }, formatMessages, isAdminSender: (msg) => isSessionCommandSenderAllowed(msg.sender), - canSenderInteract: (msg) => { - const hasTrigger = deps.triggerPattern.test(msg.content.trim()); - const requiresTrigger = - !isMainGroup && group.requiresTrigger !== false; - return ( - isMainGroup || - !requiresTrigger || - (hasTrigger && - (msg.is_from_me || - isTriggerAllowed(chatJid, msg.sender, loadSenderAllowlist()))) - ); - }, + canSenderInteract: () => true, resetPairedTask: () => { if (hasReviewerLease(chatJid)) { const task = getLatestOpenPairedTaskForChat(chatJid); @@ -717,7 +710,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { } messageLoopRunning = true; - logger.info(`EJClaw running (trigger: @${deps.assistantName})`); + logger.info('EJClaw running'); while (true) { try { @@ -727,7 +720,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { triggerPattern: deps.triggerPattern, timezone: deps.timezone, channels: deps.channels, - getRegisteredGroups: deps.getRegisteredGroups, + getRoomBindings: deps.getRoomBindings, getLastTimestamp: deps.getLastTimestamp, setLastTimestamp: deps.setLastTimestamp, lastAgentTimestamps: deps.getLastAgentTimestamps(), @@ -755,7 +748,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { recoverRuntimePendingMessages({ assistantName: deps.assistantName, channels: deps.channels, - getRegisteredGroups: deps.getRegisteredGroups, + getRoomBindings: deps.getRoomBindings, lastAgentTimestamps: deps.getLastAgentTimestamps(), saveState: deps.saveState, enqueueScopedGroupMessageCheck, diff --git a/src/message-turn-controller.test.ts b/src/message-turn-controller.test.ts index 2eafd85..02f2948 100644 --- a/src/message-turn-controller.test.ts +++ b/src/message-turn-controller.test.ts @@ -221,4 +221,56 @@ describe('MessageTurnController outbound audit logging', () => { ]), ); }); + + it('does not replay the last progress message as a final delivery for paired reviewer turns', async () => { + const channel = makeChannel(); + const deliverFinalText = vi.fn().mockResolvedValue(true); + const controller = new MessageTurnController({ + chatJid: 'dc:test-room', + group: makeGroup(), + runId: 'run-review-no-fake-final', + channel, + idleTimeout: 1_000, + failureFinalText: '실패', + isClaudeCodeAgent: true, + clearSession: vi.fn(), + requestClose: vi.fn(), + deliverFinalText, + allowProgressReplayWithoutFinal: false, + deliveryRole: 'reviewer', + deliveryServiceId: 'codex-review', + pairedTurnIdentity: makeTurnIdentity(), + }); + + await controller.start(); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: '확인하겠습니다.', + } as any); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: '근거를 다시 대조 중입니다.', + } as any); + await flushAsync(); + + const finishResult = await controller.finish('success'); + + expect(finishResult.visiblePhase).toBe('progress'); + expect(channel.sendAndTrack).toHaveBeenCalledTimes(1); + expect(channel.editMessage).toHaveBeenCalledWith( + 'dc:test-room', + 'progress-1', + expect.stringContaining('확인하겠습니다.'), + ); + expect(deliverFinalText).not.toHaveBeenCalled(); + expect(getAuditEntries()).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ + auditEvent: 'final-delivery-attempt', + }), + ]), + ); + }); }); diff --git a/src/message-turn-controller.ts b/src/message-turn-controller.ts index 7a5665c..09797b4 100644 --- a/src/message-turn-controller.ts +++ b/src/message-turn-controller.ts @@ -34,6 +34,7 @@ interface MessageTurnControllerOptions { clearSession: () => void; requestClose: (reason: string) => void; deliverFinalText: (text: string) => Promise; + allowProgressReplayWithoutFinal?: boolean; deliveryRole?: PairedRoomRole | null; deliveryServiceId?: string | null; pairedTurnIdentity?: PairedTurnIdentity | null; @@ -338,11 +339,18 @@ export class MessageTurnController { !this.hadError && this.latestProgressTextForFinal ) { - this.log.info( - 'Sending a separate final message from the last progress output after agent completion', - ); await this.finalizeProgressMessage(); - await this.deliverFinalText(this.latestProgressTextForFinal); + if (this.options.allowProgressReplayWithoutFinal !== false) { + this.log.info( + 'Sending a separate final message from the last progress output after agent completion', + ); + await this.deliverFinalText(this.latestProgressTextForFinal); + } else { + this.log.info( + 'Skipped replaying the last progress output as a final message for this turn', + ); + this.latestProgressTextForFinal = null; + } } else if ( this.visiblePhase === 'progress' && !this.terminalObserved() && diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index af608ec..a7568b3 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -334,6 +334,72 @@ describe('paired execution context', () => { }); }); + it('preserves the claimed reviewer turn revision for in-review continuations and requires a visible verdict', () => { + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-03-28T00:00:00.000Z', + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:05:00.000Z', + }); + vi.mocked(db.getPairedTaskById).mockReturnValue({ + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-03-28T00:00:00.000Z', + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:05:00.000Z', + }); + vi.mocked( + pairedWorkspaceManager.prepareReviewerWorkspaceForExecution, + ).mockReturnValue({ + workspace: buildWorkspace('reviewer', '/tmp/paired/task-1/reviewer'), + autoRefreshed: false, + }); + + const result = preparePairedExecutionContext({ + group, + chatJid: 'dc:test', + runId: 'run-review-continuation', + roomRoleContext: reviewerContext, + pairedTurnIdentity: { + turnId: 'task-1:2026-03-28T00:00:00.000Z:reviewer-turn', + taskId: 'task-1', + taskUpdatedAt: '2026-03-28T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + }); + + expect(result?.claimedTaskUpdatedAt).toBe('2026-03-28T00:00:00.000Z'); + expect(result?.requiresVisibleVerdict).toBe(true); + expect(db.updatePairedTask).not.toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ status: 'in_review' }), + ); + }); + it('does not change task state when the reviewer snapshot is not ready', () => { vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ id: 'task-1', diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index cdef09e..d4d0d78 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -46,6 +46,7 @@ import { prepareReviewerWorkspaceForExecution, provisionOwnerWorkspaceForPairedTask, } from './paired-workspace-manager.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; import type { AgentType, PairedRoomRole, @@ -193,6 +194,7 @@ export function preparePairedExecutionContext(args: { runId: string; roomRoleContext?: RoomRoleContext; hasHumanMessage?: boolean; + pairedTurnIdentity?: PairedTurnIdentity; }): PreparedPairedExecutionContext | undefined { const { group, chatJid, roomRoleContext } = args; if (!roomRoleContext || !group.workDir) { @@ -210,7 +212,19 @@ export function preparePairedExecutionContext(args: { } const latestTask = getPairedTaskById(task.id) ?? task; - const claimedTaskUpdatedAt = latestTask.updated_at; + const continuationTurnIdentity = args.pairedTurnIdentity; + const claimedTaskUpdatedAt = + continuationTurnIdentity && + continuationTurnIdentity.taskId === latestTask.id && + continuationTurnIdentity.role === roomRoleContext.role && + ((roomRoleContext.role === 'reviewer' && + continuationTurnIdentity.intentKind === 'reviewer-turn' && + latestTask.status === 'in_review') || + (roomRoleContext.role === 'arbiter' && + continuationTurnIdentity.intentKind === 'arbiter-turn' && + latestTask.status === 'in_arbitration')) + ? continuationTurnIdentity.taskUpdatedAt + : latestTask.updated_at; let workspace: PairedWorkspace | null = null; let blockMessage: string | undefined; const now = new Date().toISOString(); @@ -354,6 +368,8 @@ export function preparePairedExecutionContext(args: { claimedTaskUpdatedAt, workspace, envOverrides, + requiresVisibleVerdict: + roomRoleContext.role === 'reviewer' || roomRoleContext.role === 'arbiter', blockMessage, }; } diff --git a/src/restart-context.test.ts b/src/restart-context.test.ts index d1df11b..8a4a9a7 100644 --- a/src/restart-context.test.ts +++ b/src/restart-context.test.ts @@ -18,7 +18,7 @@ function makeGroup(folder: string): RegisteredGroup { describe('getInterruptedRecoveryCandidates', () => { it('returns only registered interrupted groups and deduplicates by chatJid', () => { - const registeredGroups: Record = { + const roomBindings: Record = { 'dc:1': makeGroup('group-one'), 'dc:2': makeGroup('group-two'), }; @@ -64,7 +64,7 @@ describe('getInterruptedRecoveryCandidates', () => { ], }; - expect(getInterruptedRecoveryCandidates(context, registeredGroups)).toEqual( + expect(getInterruptedRecoveryCandidates(context, roomBindings)).toEqual( [ { chatJid: 'dc:1', diff --git a/src/restart-context.ts b/src/restart-context.ts index 838976a..f2aaedc 100644 --- a/src/restart-context.ts +++ b/src/restart-context.ts @@ -53,9 +53,9 @@ function formatKoreanTimestamp(timestamp: string | number | Date): string { } function getMainGroupJid( - registeredGroups: Record, + roomBindings: Record, ): string | null { - const mainEntry = Object.entries(registeredGroups).find( + const mainEntry = Object.entries(roomBindings).find( ([, group]) => group.isMain === true, ); return mainEntry?.[0] ?? null; @@ -140,7 +140,7 @@ export function writeRestartContext( } export function writeShutdownRestartContext( - registeredGroups: Record, + roomBindings: Record, interruptedGroups: RestartInterruptedGroup[], signal: string, serviceIds: string[] = [SERVICE_ID], @@ -149,7 +149,7 @@ export function writeShutdownRestartContext( fs.mkdirSync(DATA_DIR, { recursive: true }); const mainChatJid = - getMainGroupJid(registeredGroups) ?? interruptedGroups[0].chatJid; + getMainGroupJid(roomBindings) ?? interruptedGroups[0].chatJid; const writtenPaths: string[] = []; for (const serviceId of serviceIds) { @@ -233,7 +233,7 @@ export function buildRestartAnnouncement(context: RestartContext): string { export function getInterruptedRecoveryCandidates( context: RestartContext | null, - registeredGroups: Record, + roomBindings: Record, ): RestartRecoveryCandidate[] { if (!context?.interruptedGroups?.length) return []; @@ -242,7 +242,7 @@ export function getInterruptedRecoveryCandidates( for (const interrupted of context.interruptedGroups) { if (seen.has(interrupted.chatJid)) continue; - const group = registeredGroups[interrupted.chatJid]; + const group = roomBindings[interrupted.chatJid]; if (!group) continue; seen.add(interrupted.chatJid); candidates.push({ @@ -258,10 +258,10 @@ export function getInterruptedRecoveryCandidates( } export function inferRecentRestartContext( - registeredGroups: Record, + roomBindings: Record, processStartedAtMs: number, ): InferredRestartContext | null { - const chatJid = getMainGroupJid(registeredGroups); + const chatJid = getMainGroupJid(roomBindings); if (!chatJid) return null; const latestBuildTime = getLatestDistBuildTime(); diff --git a/src/service-routing.test.ts b/src/service-routing.test.ts index eac979d..2f7dc92 100644 --- a/src/service-routing.test.ts +++ b/src/service-routing.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { + _deleteStoredRoomSettingsForTests, _setRegisteredGroupForTests, _setStoredRoomOwnerAgentTypeForTests, _initTestDatabase, @@ -235,10 +236,36 @@ describe('service-routing global failover', () => { }); }); - it('keeps the legacy claude fallback for chats without room settings', () => { + it('defaults to the configured owner service for chats without canonical room settings', () => { expect(getEffectiveChannelLease('dc:unregistered')).toMatchObject({ chat_jid: 'dc:unregistered', - owner_service_id: 'claude', + owner_service_id: 'codex-main', + reviewer_service_id: null, + owner_failover_active: false, + explicit: false, + }); + }); + + it('ignores legacy capability rows when canonical room settings are missing', () => { + _setRegisteredGroupForTests('dc:legacy-only', { + name: 'Legacy Only', + folder: 'legacy-only', + trigger: '@Andy', + added_at: '2024-01-01T00:00:00.000Z', + agentType: 'claude-code', + }); + _setRegisteredGroupForTests('dc:legacy-only', { + name: 'Legacy Only', + folder: 'legacy-only', + trigger: '@Codex', + added_at: '2024-01-01T00:00:00.000Z', + agentType: 'codex', + }); + _deleteStoredRoomSettingsForTests('dc:legacy-only'); + + expect(getEffectiveChannelLease('dc:legacy-only')).toMatchObject({ + chat_jid: 'dc:legacy-only', + owner_service_id: 'codex-main', reviewer_service_id: null, owner_failover_active: false, explicit: false, diff --git a/src/service-routing.ts b/src/service-routing.ts index fe297ae..1eb974d 100644 --- a/src/service-routing.ts +++ b/src/service-routing.ts @@ -12,7 +12,6 @@ import { clearChannelOwnerLease, getAllChannelOwnerLeases, getEffectiveRuntimeRoomMode, - getRegisteredAgentTypesForJid, getStoredRoomSettings, type ChannelOwnerLeaseRow, } from './db.js'; @@ -103,32 +102,14 @@ function normalizeLeaseRow( }; } -function inferFallbackOwnerAgentType( - hasClaude: boolean, - hasCodex: boolean, -): AgentType | undefined { - if (hasClaude && hasCodex) return OWNER_AGENT_TYPE; - if (hasCodex) return 'codex'; - if (hasClaude) return 'claude-code'; - return undefined; -} - function resolveDefaultOwnerAgentType(chatJid: string): AgentType | undefined { - const storedOwnerAgentType = getStoredRoomSettings(chatJid)?.ownerAgentType; - if (storedOwnerAgentType) { - return storedOwnerAgentType; - } - - const types = getRegisteredAgentTypesForJid(chatJid); - const hasClaude = types.includes('claude-code'); - const hasCodex = types.includes('codex'); - - return inferFallbackOwnerAgentType(hasClaude, hasCodex); + return getStoredRoomSettings(chatJid)?.ownerAgentType; } function getDefaultLease(chatJid: string): EffectiveChannelLease { const roomMode = getEffectiveRuntimeRoomMode(chatJid); - const ownerAgentType = resolveDefaultOwnerAgentType(chatJid) ?? 'claude-code'; + const ownerAgentType = + resolveDefaultOwnerAgentType(chatJid) ?? OWNER_AGENT_TYPE; const rolePlan = resolveRoleAgentPlan({ paired: roomMode === 'tribunal', groupAgentType: ownerAgentType, diff --git a/src/session-commands.test.ts b/src/session-commands.test.ts index b3a69d8..1f57c1b 100644 --- a/src/session-commands.test.ts +++ b/src/session-commands.test.ts @@ -15,8 +15,8 @@ describe('extractSessionCommand', () => { expect(extractSessionCommand('/compact', trigger)).toBe('/compact'); }); - it('detects /compact with trigger prefix', () => { - expect(extractSessionCommand('@Andy /compact', trigger)).toBe('/compact'); + it('does not treat bot-name prefixes as commands anymore', () => { + expect(extractSessionCommand('@Andy /compact', trigger)).toBeNull(); }); it('detects bare /clear', () => { diff --git a/src/session-commands.ts b/src/session-commands.ts index 794551a..6c57181 100644 --- a/src/session-commands.ts +++ b/src/session-commands.ts @@ -14,13 +14,13 @@ const SESSION_COMMAND_CONTROL_PATTERNS = [ function normalizeSessionCommandText( content: string, - triggerPattern: RegExp, + _triggerPattern: RegExp, ): string { - return content.trim().replace(triggerPattern, '').trim(); + return content.trim(); } /** - * Extract a session slash command from a message, stripping the trigger prefix if present. + * Extract a session slash command from a message. * Returns the slash command (e.g., '/compact') or null if not a session command. */ export function extractSessionCommand( diff --git a/src/task-scheduler-runtime.ts b/src/task-scheduler-runtime.ts index 1c2f864..c67e929 100644 --- a/src/task-scheduler-runtime.ts +++ b/src/task-scheduler-runtime.ts @@ -71,7 +71,7 @@ export function resolveTaskExecutionContext( const groupDir = resolveGroupFolderPath(task.group_folder); fs.mkdirSync(groupDir, { recursive: true }); - const groups = deps.registeredGroups(); + const groups = deps.roomBindings(); const group = Object.values(groups).find( (registeredGroup) => registeredGroup.folder === task.group_folder, ); diff --git a/src/task-scheduler-types.ts b/src/task-scheduler-types.ts index 51a3065..a96be07 100644 --- a/src/task-scheduler-types.ts +++ b/src/task-scheduler-types.ts @@ -5,7 +5,7 @@ import type { AgentType, RegisteredGroup } from './types.js'; export interface SchedulerDependencies { serviceAgentType?: AgentType; - registeredGroups: () => Record; + roomBindings: () => Record; getSessions: () => Record; queue: GroupQueue; onProcess: ( diff --git a/src/task-scheduler.test.ts b/src/task-scheduler.test.ts index 7f159d5..65af563 100644 --- a/src/task-scheduler.test.ts +++ b/src/task-scheduler.test.ts @@ -228,7 +228,7 @@ describe('task scheduler', () => { ); startSchedulerLoop({ - registeredGroups: () => ({}), + roomBindings: () => ({}), getSessions: () => ({}), queue: { enqueueTask } as any, onProcess: () => {}, @@ -274,7 +274,7 @@ describe('task scheduler', () => { startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -324,7 +324,7 @@ describe('task scheduler', () => { await runSchedulerTickOnce({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -372,7 +372,7 @@ describe('task scheduler', () => { await runSchedulerTickOnce({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -425,7 +425,7 @@ describe('task scheduler', () => { await runSchedulerTickOnce({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -496,7 +496,7 @@ describe('task scheduler', () => { await runSchedulerTickOnce({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'paired@g.us': { name: 'Paired', folder: 'paired-group', @@ -568,7 +568,7 @@ describe('task scheduler', () => { await runSchedulerTickOnce({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'paired@g.us': { name: 'Paired', folder: 'paired-group', @@ -608,7 +608,7 @@ describe('task scheduler', () => { startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -658,7 +658,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -751,7 +751,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'claude-code', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -850,7 +850,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'claude-code', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -928,7 +928,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'claude-code', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1021,7 +1021,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'claude-code', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1117,7 +1117,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1149,7 +1149,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1232,7 +1232,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1305,7 +1305,7 @@ Managed by host-driven watcher. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1381,7 +1381,7 @@ Managed by host-driven watcher. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1444,7 +1444,7 @@ Managed by host-driven watcher. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1513,7 +1513,7 @@ Managed by host-driven watcher. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1567,7 +1567,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1608,7 +1608,7 @@ Check the run. await runSchedulerTickOnce({ serviceAgentType: 'codex', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', @@ -1651,7 +1651,7 @@ Check the run. startSchedulerLoop({ serviceAgentType: 'claude-code', - registeredGroups: () => ({ + roomBindings: () => ({ 'shared@g.us': { name: 'Shared', folder: 'shared-group', diff --git a/src/types.ts b/src/types.ts index 2a8ce0e..3f1e091 100644 --- a/src/types.ts +++ b/src/types.ts @@ -148,10 +148,10 @@ export function toVisiblePhase(phase: AgentOutputPhase): VisiblePhase { export interface RegisteredGroup { name: string; folder: string; - trigger: string; + trigger?: string; added_at: string; agentConfig?: AgentConfig; - requiresTrigger?: boolean; // Default: true for groups, false for solo chats + requiresTrigger?: boolean; // Compatibility-only legacy field isMain?: boolean; // True for the main control group (no trigger, elevated privileges) agentType?: AgentType; workDir?: string; // Working directory for the agent (defaults to group folder) diff --git a/src/unified-dashboard.ts b/src/unified-dashboard.ts index f5fde09..9ea1c84 100644 --- a/src/unified-dashboard.ts +++ b/src/unified-dashboard.ts @@ -66,7 +66,7 @@ export interface UnifiedDashboardOptions { usageUpdateInterval: number; channels: Channel[]; queue: GroupQueue; - registeredGroups: () => Record; + roomBindings: () => Record; onGroupNameSynced?: (jid: string, name: string) => void; purgeOnStart?: boolean; } @@ -175,7 +175,7 @@ async function refreshChannelMeta( ); if (!channel?.getChannelMeta) return; - const localJids = Object.keys(opts.registeredGroups()).filter((jid) => + const localJids = Object.keys(opts.roomBindings()).filter((jid) => jid.startsWith('dc:'), ); const snapshotJids = readStatusSnapshots(STATUS_SNAPSHOT_MAX_AGE_MS) @@ -189,7 +189,7 @@ async function refreshChannelMeta( for (const [jid, meta] of channelMetaCache) { if (!meta.name) continue; - const group = opts.registeredGroups()[jid]; + const group = opts.roomBindings()[jid]; if (!group || group.name === meta.name) continue; logger.info( { jid, oldName: group.name, newName: meta.name }, @@ -230,7 +230,7 @@ function formatRoomName( } function writeLocalStatusSnapshot(opts: UnifiedDashboardOptions): void { - const groups = opts.registeredGroups(); + const groups = opts.roomBindings(); const statuses = opts.queue.getStatuses(Object.keys(groups)); writeStatusSnapshot({