From 7f8a361f245c6588306e9c6469e6b38040c5700f Mon Sep 17 00:00:00 2001 From: Eyejoker Date: Tue, 31 Mar 2026 09:35:19 +0900 Subject: [PATCH] refactor: wire role agent plan into runtime --- src/message-agent-executor.test.ts | 62 ++++++++++++++++++++++++++++++ src/message-agent-executor.ts | 9 ++++- src/message-runtime-rules.ts | 39 +++++++++++-------- src/role-agent-plan.test.ts | 29 +++++++++++++- src/role-agent-plan.ts | 16 +++++++- 5 files changed, 135 insertions(+), 20 deletions(-) diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 2df7f9c..5a318f1 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -15,6 +15,7 @@ vi.mock('./config.js', () => ({ CODEX_REVIEW_SERVICE_ID: 'codex-review', DATA_DIR: '/tmp/ejclaw-test-data', REVIEWER_AGENT_TYPE: 'claude-code', + ARBITER_AGENT_TYPE: undefined, SERVICE_SESSION_SCOPE: 'claude', isClaudeService: vi.fn(() => true), normalizeServiceId: vi.fn((serviceId: string) => @@ -568,6 +569,67 @@ describe('runAgentForGroup room memory', () => { 'Review snapshot is stale after owner changes. Retry the review once to refresh against the latest owner workspace.', }); }); + + it('uses the role plan for reviewer execution while keeping task snapshots on the owner agent', async () => { + const group = { + ...makeGroup(), + folder: 'test-group', + agentType: 'codex' as const, + }; + const deps = { + ...makeDeps(), + getSessions: () => ({ 'test-group:reviewer': 'reviewer-session' }), + }; + + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_service_id: 'codex-main', + 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: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-03-31T00:00:00.000Z', + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-31T00:00:00.000Z', + updated_at: '2026-03-31T00:00:00.000Z', + }); + + await runAgentForGroup(deps, { + group, + prompt: 'please review', + chatJid: 'group@test', + runId: 'run-review-plan', + }); + + expect(db.getAllTasks).toHaveBeenCalledWith('codex'); + expect(agentRunner.runAgentProcess).toHaveBeenCalledWith( + expect.objectContaining({ + folder: 'test-group', + agentType: 'claude-code', + }), + expect.objectContaining({ + sessionId: 'reviewer-session', + }), + expect.any(Function), + undefined, + undefined, + ); + }); }); describe('runAgentForGroup Claude rotation', () => { diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 877bf7a..e38b571 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -29,6 +29,7 @@ import { } from './paired-execution-context.js'; import { resolveActiveRole, + resolveConfiguredRoleAgentPlan, resolveEffectiveAgentType, resolveSessionFolder, } from './message-runtime-rules.js'; @@ -115,13 +116,17 @@ export async function runAgentForGroup( : currentLease.owner_service_id; const reviewerMode = activeRole === 'reviewer'; const arbiterMode = activeRole === 'arbiter'; + const roleAgentPlan = resolveConfiguredRoleAgentPlan( + currentLease.reviewer_service_id != null, + group.agentType, + ); const effectiveAgentType = resolveEffectiveAgentType( activeRole, group.agentType, ); const effectiveGroup = - effectiveAgentType !== (group.agentType || 'claude-code') + effectiveAgentType !== roleAgentPlan.ownerAgentType ? { ...group, agentType: effectiveAgentType } : group; const isClaudeCodeAgent = effectiveAgentType === 'claude-code'; @@ -140,7 +145,7 @@ export async function runAgentForGroup( groupName: group.name, }).catch(() => undefined); - const tasks = getAllTasks(group.agentType || 'claude-code'); + const tasks = getAllTasks(roleAgentPlan.ownerAgentType); writeTasksSnapshot( group.folder, isMain, diff --git a/src/message-runtime-rules.ts b/src/message-runtime-rules.ts index e52ef1a..fc18c46 100644 --- a/src/message-runtime-rules.ts +++ b/src/message-runtime-rules.ts @@ -5,9 +5,13 @@ import { isTriggerAllowed, loadSenderAllowlist } from './sender-allowlist.js'; import { isTaskStatusControlMessage } from './task-watch-status.js'; import { ARBITER_AGENT_TYPE, - OWNER_AGENT_TYPE, REVIEWER_AGENT_TYPE, } from './config.js'; +import { + resolveAgentTypeForRole, + resolveRoleAgentPlan, + type RoleAgentPlan, +} from './role-agent-plan.js'; import { type AgentType, type Channel, @@ -63,24 +67,26 @@ export function resolveCursorKey( return role === 'owner' ? chatJid : `${chatJid}:${role}`; } +/** Resolve the effective agent type for a role, considering per-role overrides. */ +export function resolveConfiguredRoleAgentPlan( + paired: boolean, + groupAgentType: AgentType | undefined, +): RoleAgentPlan { + return resolveRoleAgentPlan({ + paired, + groupAgentType, + configuredReviewer: REVIEWER_AGENT_TYPE, + configuredArbiter: ARBITER_AGENT_TYPE, + }); +} + /** Resolve the effective agent type for a role, considering per-role overrides. */ export function resolveEffectiveAgentType( role: 'owner' | 'reviewer' | 'arbiter', groupAgentType: AgentType | undefined, ): AgentType { - const groupDefault: AgentType = groupAgentType || 'claude-code'; - switch (role) { - case 'reviewer': - return REVIEWER_AGENT_TYPE !== groupDefault - ? REVIEWER_AGENT_TYPE - : groupDefault; - case 'arbiter': - return ARBITER_AGENT_TYPE != null && ARBITER_AGENT_TYPE !== groupDefault - ? ARBITER_AGENT_TYPE - : groupDefault; - default: - return groupDefault; - } + const plan = resolveConfiguredRoleAgentPlan(role !== 'owner', groupAgentType); + return resolveAgentTypeForRole(plan, role); } /** Session folder key for a role. Owner uses groupFolder, others use groupFolder:role. */ @@ -91,8 +97,9 @@ export function resolveSessionFolder( ): string { // Arbiter always gets a separate session — must never share with owner/reviewer if (role === 'arbiter') return `${groupFolder}:arbiter`; - const effectiveType = resolveEffectiveAgentType(role, groupAgentType); - const groupDefault: AgentType = groupAgentType || 'claude-code'; + const plan = resolveConfiguredRoleAgentPlan(role !== 'owner', groupAgentType); + const effectiveType = resolveAgentTypeForRole(plan, role); + const groupDefault = plan.ownerAgentType; if (effectiveType === groupDefault) return groupFolder; return `${groupFolder}:${role}`; } diff --git a/src/role-agent-plan.test.ts b/src/role-agent-plan.test.ts index b5e3de6..a6f626a 100644 --- a/src/role-agent-plan.test.ts +++ b/src/role-agent-plan.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from 'vitest'; -import { resolveRoleAgentPlan } from './role-agent-plan.js'; +import { + resolveAgentTypeForRole, + resolveRoleAgentPlan, +} from './role-agent-plan.js'; describe('resolveRoleAgentPlan', () => { it('uses the group agent type as owner in single rooms', () => { @@ -60,4 +63,28 @@ describe('resolveRoleAgentPlan', () => { arbiterAgentType: null, }); }); + + it('falls back to owner when reviewer or arbiter are absent in the plan', () => { + const plan = resolveRoleAgentPlan({ + paired: false, + groupAgentType: 'codex', + configuredReviewer: 'claude-code', + }); + + expect(resolveAgentTypeForRole(plan, 'owner')).toBe('codex'); + expect(resolveAgentTypeForRole(plan, 'reviewer')).toBe('codex'); + expect(resolveAgentTypeForRole(plan, 'arbiter')).toBe('codex'); + }); + + it('returns explicit reviewer and arbiter agent types when present', () => { + const plan = resolveRoleAgentPlan({ + paired: true, + groupAgentType: 'codex', + configuredReviewer: 'claude-code', + configuredArbiter: 'claude-code', + }); + + expect(resolveAgentTypeForRole(plan, 'reviewer')).toBe('claude-code'); + expect(resolveAgentTypeForRole(plan, 'arbiter')).toBe('claude-code'); + }); }); diff --git a/src/role-agent-plan.ts b/src/role-agent-plan.ts index 8dce972..8c74de8 100644 --- a/src/role-agent-plan.ts +++ b/src/role-agent-plan.ts @@ -1,4 +1,4 @@ -import type { AgentType } from './types.js'; +import type { AgentType, PairedRoomRole } from './types.js'; export interface RoleAgentPlan { ownerAgentType: AgentType; @@ -39,3 +39,17 @@ export function resolveRoleAgentPlan( arbiterAgentType: args.configuredArbiter ?? null, }; } + +export function resolveAgentTypeForRole( + plan: RoleAgentPlan, + role: PairedRoomRole, +): AgentType { + switch (role) { + case 'reviewer': + return plan.reviewerAgentType ?? plan.ownerAgentType; + case 'arbiter': + return plan.arbiterAgentType ?? plan.ownerAgentType; + default: + return plan.ownerAgentType; + } +}