refactor: wire role agent plan into runtime
This commit is contained in:
@@ -15,6 +15,7 @@ vi.mock('./config.js', () => ({
|
|||||||
CODEX_REVIEW_SERVICE_ID: 'codex-review',
|
CODEX_REVIEW_SERVICE_ID: 'codex-review',
|
||||||
DATA_DIR: '/tmp/ejclaw-test-data',
|
DATA_DIR: '/tmp/ejclaw-test-data',
|
||||||
REVIEWER_AGENT_TYPE: 'claude-code',
|
REVIEWER_AGENT_TYPE: 'claude-code',
|
||||||
|
ARBITER_AGENT_TYPE: undefined,
|
||||||
SERVICE_SESSION_SCOPE: 'claude',
|
SERVICE_SESSION_SCOPE: 'claude',
|
||||||
isClaudeService: vi.fn(() => true),
|
isClaudeService: vi.fn(() => true),
|
||||||
normalizeServiceId: vi.fn((serviceId: string) =>
|
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.',
|
'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', () => {
|
describe('runAgentForGroup Claude rotation', () => {
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ import {
|
|||||||
} from './paired-execution-context.js';
|
} from './paired-execution-context.js';
|
||||||
import {
|
import {
|
||||||
resolveActiveRole,
|
resolveActiveRole,
|
||||||
|
resolveConfiguredRoleAgentPlan,
|
||||||
resolveEffectiveAgentType,
|
resolveEffectiveAgentType,
|
||||||
resolveSessionFolder,
|
resolveSessionFolder,
|
||||||
} from './message-runtime-rules.js';
|
} from './message-runtime-rules.js';
|
||||||
@@ -115,13 +116,17 @@ export async function runAgentForGroup(
|
|||||||
: currentLease.owner_service_id;
|
: currentLease.owner_service_id;
|
||||||
const reviewerMode = activeRole === 'reviewer';
|
const reviewerMode = activeRole === 'reviewer';
|
||||||
const arbiterMode = activeRole === 'arbiter';
|
const arbiterMode = activeRole === 'arbiter';
|
||||||
|
const roleAgentPlan = resolveConfiguredRoleAgentPlan(
|
||||||
|
currentLease.reviewer_service_id != null,
|
||||||
|
group.agentType,
|
||||||
|
);
|
||||||
|
|
||||||
const effectiveAgentType = resolveEffectiveAgentType(
|
const effectiveAgentType = resolveEffectiveAgentType(
|
||||||
activeRole,
|
activeRole,
|
||||||
group.agentType,
|
group.agentType,
|
||||||
);
|
);
|
||||||
const effectiveGroup =
|
const effectiveGroup =
|
||||||
effectiveAgentType !== (group.agentType || 'claude-code')
|
effectiveAgentType !== roleAgentPlan.ownerAgentType
|
||||||
? { ...group, agentType: effectiveAgentType }
|
? { ...group, agentType: effectiveAgentType }
|
||||||
: group;
|
: group;
|
||||||
const isClaudeCodeAgent = effectiveAgentType === 'claude-code';
|
const isClaudeCodeAgent = effectiveAgentType === 'claude-code';
|
||||||
@@ -140,7 +145,7 @@ export async function runAgentForGroup(
|
|||||||
groupName: group.name,
|
groupName: group.name,
|
||||||
}).catch(() => undefined);
|
}).catch(() => undefined);
|
||||||
|
|
||||||
const tasks = getAllTasks(group.agentType || 'claude-code');
|
const tasks = getAllTasks(roleAgentPlan.ownerAgentType);
|
||||||
writeTasksSnapshot(
|
writeTasksSnapshot(
|
||||||
group.folder,
|
group.folder,
|
||||||
isMain,
|
isMain,
|
||||||
|
|||||||
@@ -5,9 +5,13 @@ import { isTriggerAllowed, loadSenderAllowlist } from './sender-allowlist.js';
|
|||||||
import { isTaskStatusControlMessage } from './task-watch-status.js';
|
import { isTaskStatusControlMessage } from './task-watch-status.js';
|
||||||
import {
|
import {
|
||||||
ARBITER_AGENT_TYPE,
|
ARBITER_AGENT_TYPE,
|
||||||
OWNER_AGENT_TYPE,
|
|
||||||
REVIEWER_AGENT_TYPE,
|
REVIEWER_AGENT_TYPE,
|
||||||
} from './config.js';
|
} from './config.js';
|
||||||
|
import {
|
||||||
|
resolveAgentTypeForRole,
|
||||||
|
resolveRoleAgentPlan,
|
||||||
|
type RoleAgentPlan,
|
||||||
|
} from './role-agent-plan.js';
|
||||||
import {
|
import {
|
||||||
type AgentType,
|
type AgentType,
|
||||||
type Channel,
|
type Channel,
|
||||||
@@ -63,24 +67,26 @@ export function resolveCursorKey(
|
|||||||
return role === 'owner' ? chatJid : `${chatJid}:${role}`;
|
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. */
|
/** Resolve the effective agent type for a role, considering per-role overrides. */
|
||||||
export function resolveEffectiveAgentType(
|
export function resolveEffectiveAgentType(
|
||||||
role: 'owner' | 'reviewer' | 'arbiter',
|
role: 'owner' | 'reviewer' | 'arbiter',
|
||||||
groupAgentType: AgentType | undefined,
|
groupAgentType: AgentType | undefined,
|
||||||
): AgentType {
|
): AgentType {
|
||||||
const groupDefault: AgentType = groupAgentType || 'claude-code';
|
const plan = resolveConfiguredRoleAgentPlan(role !== 'owner', groupAgentType);
|
||||||
switch (role) {
|
return resolveAgentTypeForRole(plan, 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;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Session folder key for a role. Owner uses groupFolder, others use groupFolder:role. */
|
/** Session folder key for a role. Owner uses groupFolder, others use groupFolder:role. */
|
||||||
@@ -91,8 +97,9 @@ export function resolveSessionFolder(
|
|||||||
): string {
|
): string {
|
||||||
// Arbiter always gets a separate session — must never share with owner/reviewer
|
// Arbiter always gets a separate session — must never share with owner/reviewer
|
||||||
if (role === 'arbiter') return `${groupFolder}:arbiter`;
|
if (role === 'arbiter') return `${groupFolder}:arbiter`;
|
||||||
const effectiveType = resolveEffectiveAgentType(role, groupAgentType);
|
const plan = resolveConfiguredRoleAgentPlan(role !== 'owner', groupAgentType);
|
||||||
const groupDefault: AgentType = groupAgentType || 'claude-code';
|
const effectiveType = resolveAgentTypeForRole(plan, role);
|
||||||
|
const groupDefault = plan.ownerAgentType;
|
||||||
if (effectiveType === groupDefault) return groupFolder;
|
if (effectiveType === groupDefault) return groupFolder;
|
||||||
return `${groupFolder}:${role}`;
|
return `${groupFolder}:${role}`;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
import { describe, expect, it } from 'vitest';
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
import { resolveRoleAgentPlan } from './role-agent-plan.js';
|
import {
|
||||||
|
resolveAgentTypeForRole,
|
||||||
|
resolveRoleAgentPlan,
|
||||||
|
} from './role-agent-plan.js';
|
||||||
|
|
||||||
describe('resolveRoleAgentPlan', () => {
|
describe('resolveRoleAgentPlan', () => {
|
||||||
it('uses the group agent type as owner in single rooms', () => {
|
it('uses the group agent type as owner in single rooms', () => {
|
||||||
@@ -60,4 +63,28 @@ describe('resolveRoleAgentPlan', () => {
|
|||||||
arbiterAgentType: null,
|
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');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import type { AgentType } from './types.js';
|
import type { AgentType, PairedRoomRole } from './types.js';
|
||||||
|
|
||||||
export interface RoleAgentPlan {
|
export interface RoleAgentPlan {
|
||||||
ownerAgentType: AgentType;
|
ownerAgentType: AgentType;
|
||||||
@@ -39,3 +39,17 @@ export function resolveRoleAgentPlan(
|
|||||||
arbiterAgentType: args.configuredArbiter ?? null,
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user