From 1c87616c440123e5d99fb8d006ccb261ddaba038 Mon Sep 17 00:00:00 2001 From: Eyejoker Date: Tue, 31 Mar 2026 14:01:31 +0900 Subject: [PATCH] fix: support same-service tribunal runtime --- src/db.test.ts | 24 +++- src/db.ts | 13 +- src/message-agent-executor.test.ts | 54 +++++++ src/message-agent-executor.ts | 1 + src/message-runtime.test.ts | 223 +++++++++++++++++++++++++++++ src/message-runtime.ts | 12 ++ src/room-role-context.test.ts | 25 ++++ src/room-role-context.ts | 19 ++- src/service-routing.test.ts | 20 ++- 9 files changed, 380 insertions(+), 11 deletions(-) diff --git a/src/db.test.ts b/src/db.test.ts index 48f7bca..9a917bd 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -853,7 +853,7 @@ describe('paired room registration', () => { expect(isPairedRoomJid('dc:explicit-single')).toBe(false); }); - it('keeps explicit tribunal configured but not runnable without dual registration', () => { + it('lets explicit tribunal become runnable when the configured reviewer can run on the solo registration', () => { setRegisteredGroup('dc:explicit-tribunal', { name: 'Explicit Tribunal Claude', folder: 'explicit-tribunal-claude', @@ -868,8 +868,8 @@ describe('paired room registration', () => { expect(getExplicitRoomMode('dc:explicit-tribunal')).toBe('tribunal'); expect(getEffectiveRoomMode('dc:explicit-tribunal')).toBe('tribunal'); - expect(getEffectiveRuntimeRoomMode('dc:explicit-tribunal')).toBe('single'); - expect(isPairedRoomJid('dc:explicit-tribunal')).toBe(false); + expect(getEffectiveRuntimeRoomMode('dc:explicit-tribunal')).toBe('tribunal'); + expect(isPairedRoomJid('dc:explicit-tribunal')).toBe(true); clearExplicitRoomMode('dc:explicit-tribunal'); @@ -878,6 +878,24 @@ describe('paired room registration', () => { expect(getEffectiveRuntimeRoomMode('dc:explicit-tribunal')).toBe('single'); expect(isPairedRoomJid('dc:explicit-tribunal')).toBe(false); }); + + it('keeps explicit tribunal non-runnable when the configured reviewer service is unavailable', () => { + setRegisteredGroup('dc:explicit-tribunal-codex', { + name: 'Explicit Tribunal Codex', + folder: 'explicit-tribunal-codex', + trigger: '@Codex', + added_at: '2024-01-01T00:00:00.000Z', + agentType: 'codex', + }); + + setExplicitRoomMode('dc:explicit-tribunal-codex', 'tribunal'); + + expect(getEffectiveRoomMode('dc:explicit-tribunal-codex')).toBe('tribunal'); + expect(getEffectiveRuntimeRoomMode('dc:explicit-tribunal-codex')).toBe( + 'single', + ); + expect(isPairedRoomJid('dc:explicit-tribunal-codex')).toBe(false); + }); }); describe('service handoff completion', () => { diff --git a/src/db.ts b/src/db.ts index 5a49424..2c728ff 100644 --- a/src/db.ts +++ b/src/db.ts @@ -9,6 +9,7 @@ import { CODEX_REVIEW_SERVICE_ID, DATA_DIR, normalizeServiceId, + REVIEWER_AGENT_TYPE, SERVICE_ID, SERVICE_SESSION_SCOPE, STORE_DIR, @@ -1978,9 +1979,19 @@ export function getEffectiveRoomMode(chatJid: string): RoomMode { return getExplicitRoomMode(chatJid) ?? inferRoomModeForJid(chatJid); } +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 { return getEffectiveRoomMode(chatJid) === 'tribunal' && - inferRoomModeForJid(chatJid) === 'tribunal' + canRunTribunalFromRegisteredAgentTypes(getRegisteredAgentTypesForJid(chatJid)) ? 'tribunal' : 'single'; } diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 5a318f1..fa379b3 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -338,6 +338,60 @@ describe('runAgentForGroup room memory', () => { ); }); + it('preserves reviewer role metadata for same-service review turns', async () => { + const group = { ...makeGroup(), folder: 'test-group' }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_service_id: 'claude', + reviewer_service_id: 'claude', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'paired-task-review', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-03-29T00:00:00.000Z', + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-29T00:00:00.000Z', + updated_at: '2026-03-29T00:00:00.000Z', + }); + + await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-same-service-reviewer', + }); + + expect(agentRunner.runAgentProcess).toHaveBeenCalledWith( + group, + expect.objectContaining({ + roomRoleContext: { + serviceId: 'claude', + role: 'reviewer', + ownerServiceId: 'claude', + reviewerServiceId: 'claude', + failoverOwner: false, + }, + }), + expect.any(Function), + undefined, + undefined, + ); + }); + it('allows silent reviewer outputs', async () => { const group = { ...makeGroup(), folder: 'test-group', workDir: '/repo' }; vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index e38b571..bd8effc 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -172,6 +172,7 @@ export async function runAgentForGroup( const roomRoleContext = buildRoomRoleContext( currentLease, effectiveServiceId, + activeRole, ); const pairedExecutionContext = preparePairedExecutionContext({ group, diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index 4d3c152..a67f702 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -180,6 +180,7 @@ import * as db from './db.js'; import { resolveGroupIpcPath } from './group-folder.js'; import { createMessageRuntime } from './message-runtime.js'; import * as config from './config.js'; +import * as serviceRouting from './service-routing.js'; import type { Channel, RegisteredGroup } from './types.js'; function makeGroup(agentType: 'claude-code' | 'codex'): RegisteredGroup { @@ -869,6 +870,228 @@ describe('createMessageRuntime', () => { ); }); + it('does not fabricate owner labels from same-service raw bot history in paired turn prompts', async () => { + const chatJid = 'group@test'; + const group = makeGroup('claude-code'); + const saveState = vi.fn(); + const lastAgentTimestamps: Record = {}; + const channel: Channel = { + ...makeChannel(chatJid), + isOwnMessage: vi.fn((msg) => msg.sender === 'shared-bot@test'), + }; + + vi.mocked(db.isPairedRoomJid).mockReturnValue(true); + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: chatJid, + owner_service_id: 'claude', + reviewer_service_id: 'claude', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'task-same-service', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: '2026-03-30T00:00:00.000Z', + round_trip_count: 1, + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-30T00:00:00.000Z', + updated_at: '2026-03-30T00:00:00.000Z', + }); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([]); + vi.mocked(db.getMessagesSince).mockReturnValue([ + { + id: 'human-1', + chat_jid: chatJid, + sender: 'user@test', + sender_name: 'User', + content: '계속 진행해줘', + timestamp: '2026-03-30T00:00:01.000Z', + is_bot_message: false, + }, + { + id: 'bot-1', + chat_jid: chatJid, + sender: 'shared-bot@test', + sender_name: 'Shared Bot', + content: 'reviewer-like reply', + timestamp: '2026-03-30T00:00:02.000Z', + is_bot_message: true, + }, + ] as any); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, input, _onProcess, onOutput) => { + expect(input.prompt).toContain( + 'reviewer-like reply', + ); + expect(input.prompt).not.toContain( + 'reviewer-like reply', + ); + await onOutput?.({ + status: 'success', + phase: 'final', + result: '같은 서비스 턴 확인', + newSessionId: 'session-same-service-fallback', + }); + return { + status: 'success', + result: '같은 서비스 턴 확인', + newSessionId: 'session-same-service-fallback', + }; + }, + ); + + const runtime = createMessageRuntime({ + assistantName: 'Andy', + idleTimeout: 1_000, + pollInterval: 1_000, + timezone: 'Asia/Seoul', + triggerPattern: /^@Andy\b/i, + channels: [channel], + queue: { + registerProcess: vi.fn(), + closeStdin: vi.fn(), + notifyIdle: vi.fn(), + } as any, + getRegisteredGroups: () => ({ [chatJid]: group }), + getSessions: () => ({}), + getLastTimestamp: () => '', + setLastTimestamp: vi.fn(), + getLastAgentTimestamps: () => lastAgentTimestamps, + saveState, + persistSession: vi.fn(), + clearSession: vi.fn(), + }); + + const result = await runtime.processGroupMessages(chatJid, { + runId: 'run-same-service-raw-history', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(agentRunner.runAgentProcess).toHaveBeenCalledTimes(1); + }); + + it('does not fabricate owner labels from same-service raw bot history in arbiter prompts', async () => { + const chatJid = 'group@test'; + const group = makeGroup('claude-code'); + const channel: Channel = { + ...makeChannel(chatJid), + isOwnMessage: vi.fn((msg) => msg.sender === 'shared-bot@test'), + }; + + vi.mocked(db.isPairedRoomJid).mockReturnValue(true); + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: chatJid, + owner_service_id: 'claude', + reviewer_service_id: 'claude', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'task-same-service-arbiter', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: '2026-03-30T00:00:00.000Z', + round_trip_count: 2, + status: 'arbiter_requested', + arbiter_verdict: null, + arbiter_requested_at: '2026-03-30T00:00:10.000Z', + completion_reason: null, + created_at: '2026-03-30T00:00:00.000Z', + updated_at: '2026-03-30T00:00:10.000Z', + }); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([]); + vi.mocked(db.getRecentChatMessages).mockReturnValue([ + { + id: 'human-1', + chat_jid: chatJid, + sender: 'user@test', + sender_name: 'User', + content: '둘 중 누가 맞는지 판단해줘', + timestamp: '2026-03-30T00:00:01.000Z', + is_bot_message: false, + }, + { + id: 'bot-1', + chat_jid: chatJid, + sender: 'shared-bot@test', + sender_name: 'Shared Bot', + content: 'reviewer-like reply', + timestamp: '2026-03-30T00:00:02.000Z', + is_bot_message: true, + }, + ] as any); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, input, _onProcess, onOutput) => { + expect(input.prompt).toContain( + 'reviewer-like reply', + ); + expect(input.prompt).not.toContain( + 'reviewer-like reply', + ); + await onOutput?.({ + status: 'success', + phase: 'final', + result: '중재 문맥 확인', + newSessionId: 'session-same-service-arbiter', + }); + return { + status: 'success', + result: '중재 문맥 확인', + newSessionId: 'session-same-service-arbiter', + }; + }, + ); + + const runtime = createMessageRuntime({ + assistantName: 'Andy', + idleTimeout: 1_000, + pollInterval: 1_000, + timezone: 'Asia/Seoul', + triggerPattern: /^@Andy\b/i, + channels: [channel], + queue: { + registerProcess: vi.fn(), + closeStdin: vi.fn(), + notifyIdle: vi.fn(), + } as any, + getRegisteredGroups: () => ({ [chatJid]: group }), + getSessions: () => ({}), + getLastTimestamp: () => '', + setLastTimestamp: vi.fn(), + getLastAgentTimestamps: () => ({}), + saveState: vi.fn(), + persistSession: vi.fn(), + clearSession: vi.fn(), + }); + + const result = await runtime.processGroupMessages(chatJid, { + runId: 'run-same-service-arbiter-fallback', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(agentRunner.runAgentProcess).toHaveBeenCalledTimes(1); + }); + it('allows follow-up messages without a trigger after a visible reply in non-main groups', async () => { const chatJid = 'group@test'; const group: RegisteredGroup = { diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 0fb2d03..c61897b 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -132,6 +132,9 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { ): NewMessage[] => { if (!isPairedRoomJid(chatJid)) return messages; const lease = getEffectiveChannelLease(chatJid); + const sharedOwnerReviewerService = + lease.reviewer_service_id !== null && + lease.owner_service_id === lease.reviewer_service_id; // Build bot-user-id → channel-name mapping from connected channels const botIdToChannelName = new Map(); for (const ch of deps.channels) { @@ -155,6 +158,15 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { if (!channelName) return msg; const serviceId = channelToService[channelName]; if (!serviceId) return msg; + // Raw channel history cannot tell owner/reviewer apart when both roles + // are delivered by the same service, so avoid fabricating a role label. + if ( + sharedOwnerReviewerService && + serviceId === lease.owner_service_id && + serviceId === lease.reviewer_service_id + ) { + return msg; + } const role = serviceId === lease.owner_service_id ? 'owner' diff --git a/src/room-role-context.test.ts b/src/room-role-context.test.ts index 44e8fae..3e5be1d 100644 --- a/src/room-role-context.test.ts +++ b/src/room-role-context.test.ts @@ -75,6 +75,31 @@ describe('buildRoomRoleContext', () => { }); }); + it('uses the preferred reviewer role when owner and reviewer share the same service', () => { + expect( + buildRoomRoleContext( + { + chat_jid: 'group@test', + owner_service_id: 'claude', + reviewer_service_id: 'claude', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }, + 'claude', + 'reviewer', + ), + ).toEqual({ + serviceId: 'claude', + role: 'reviewer', + ownerServiceId: 'claude', + reviewerServiceId: 'claude', + failoverOwner: false, + arbiterServiceId: undefined, + }); + }); + it('returns undefined for a non-paired room', () => { expect( buildRoomRoleContext( diff --git a/src/room-role-context.ts b/src/room-role-context.ts index 9df9880..529d117 100644 --- a/src/room-role-context.ts +++ b/src/room-role-context.ts @@ -3,12 +3,13 @@ import { CODEX_REVIEW_SERVICE_ID, normalizeServiceId, } from './config.js'; -import type { RoomRoleContext } from './types.js'; +import type { PairedRoomRole, RoomRoleContext } from './types.js'; import type { EffectiveChannelLease } from './service-routing.js'; export function buildRoomRoleContext( lease: EffectiveChannelLease, serviceId: string, + preferredRole?: PairedRoomRole, ): RoomRoleContext | undefined { const normalizedServiceId = normalizeServiceId(serviceId); const reviewerServiceId = lease.reviewer_service_id @@ -24,14 +25,20 @@ export function buildRoomRoleContext( ? normalizeServiceId(lease.arbiter_service_id) : undefined; - // Check arbiter role first: if this service matches the arbiter_service_id, - // it takes the arbiter role (even if it also matches owner or reviewer) + const matches = { + owner: ownerServiceId === normalizedServiceId, + reviewer: reviewerServiceId === normalizedServiceId, + arbiter: arbiterServiceId === normalizedServiceId, + }; + const role = - arbiterServiceId && arbiterServiceId === normalizedServiceId + preferredRole && matches[preferredRole] + ? preferredRole + : matches.arbiter ? 'arbiter' - : ownerServiceId === normalizedServiceId + : matches.owner ? 'owner' - : reviewerServiceId === normalizedServiceId + : matches.reviewer ? 'reviewer' : null; diff --git a/src/service-routing.test.ts b/src/service-routing.test.ts index 2d5799a..d0249b4 100644 --- a/src/service-routing.test.ts +++ b/src/service-routing.test.ts @@ -108,7 +108,7 @@ describe('service-routing global failover', () => { }); }); - it('does not create reviewer lease for explicit tribunal without dual registration capability', () => { + it('creates a same-service reviewer lease when explicit tribunal is runnable on a solo registration', () => { setRegisteredGroup('dc:explicit-tribunal', { name: 'Explicit Tribunal Claude', folder: 'explicit-tribunal-claude', @@ -121,6 +121,24 @@ describe('service-routing global failover', () => { expect(getEffectiveChannelLease('dc:explicit-tribunal')).toMatchObject({ chat_jid: 'dc:explicit-tribunal', owner_service_id: 'claude', + reviewer_service_id: 'claude', + explicit: false, + }); + }); + + it('keeps reviewer lease disabled when explicit tribunal cannot deliver the configured reviewer', () => { + setRegisteredGroup('dc:explicit-tribunal-codex', { + name: 'Explicit Tribunal Codex', + folder: 'explicit-tribunal-codex', + trigger: '@Codex', + added_at: '2024-01-01T00:00:00.000Z', + agentType: 'codex', + }); + setExplicitRoomMode('dc:explicit-tribunal-codex', 'tribunal'); + + expect(getEffectiveChannelLease('dc:explicit-tribunal-codex')).toMatchObject({ + chat_jid: 'dc:explicit-tribunal-codex', + owner_service_id: 'codex-main', reviewer_service_id: null, explicit: false, });