From 2b32b7211993c5170878aceba3316377177fe06b Mon Sep 17 00:00:00 2001 From: ejclaw Date: Wed, 22 Apr 2026 06:47:52 +0900 Subject: [PATCH] paired: decouple unsafe host mode from fresh reviewer sessions --- src/config.test.ts | 11 +++ src/config.ts | 8 ++ src/config/load-config.ts | 4 + src/config/schema.ts | 1 + src/message-agent-executor-target.ts | 6 +- src/message-agent-executor.test.ts | 112 ++++++++++++++++++++++++++- 6 files changed, 140 insertions(+), 2 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index 6f21f11..46a0357 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -30,6 +30,7 @@ describe('config/env loading', () => { delete process.env.DISCORD_REVIEW_BOT_TOKEN; delete process.env.DISCORD_CODEX_REVIEW_BOT_TOKEN; delete process.env.PAIRED_CARRY_FORWARD_LATEST_OWNER_FINAL; + delete process.env.PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION; delete process.env.SESSION_COMMAND_USER_IDS; vi.resetModules(); }); @@ -113,6 +114,16 @@ describe('config/env loading', () => { expect(config.PAIRED_CARRY_FORWARD_LATEST_OWNER_FINAL).toBe(true); }); + it('defaults unsafe-host Claude reviewer fresh-session forcing to disabled and honors explicit opt-in', async () => { + let config = await import('./config.js'); + expect(config.PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION).toBe(false); + + vi.resetModules(); + process.env.PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION = 'true'; + config = await import('./config.js'); + expect(config.PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION).toBe(true); + }); + it('fails fast when a legacy Discord token alias is configured', async () => { process.env.DISCORD_BOT_TOKEN = 'legacy-owner-token'; diff --git a/src/config.ts b/src/config.ts index ce5272e..d194b8c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -80,6 +80,14 @@ export const ARBITER_SERVICE_ID = CONFIG.paired.arbiterServiceId; export const PAIRED_CARRY_FORWARD_LATEST_OWNER_FINAL = CONFIG.paired.carryForwardLatestOwnerFinal; +/** Whether unsafe-host Claude reviewers must always start on a fresh SDK session. Default: false. */ +export const PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION = + CONFIG.paired.forceFreshClaudeReviewerSessionInUnsafeHostMode; + +export function shouldForceFreshClaudeReviewerSessionInUnsafeHostMode(): boolean { + return PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION; +} + /** Language for agent responses. When set, a language instruction is appended to all paired room prompts. */ export const AGENT_LANGUAGE = CONFIG.paired.agentLanguage; diff --git a/src/config/load-config.ts b/src/config/load-config.ts index fa7297f..98e1740 100644 --- a/src/config/load-config.ts +++ b/src/config/load-config.ts @@ -227,6 +227,10 @@ export function loadConfig(): AppConfig { 'PAIRED_CARRY_FORWARD_LATEST_OWNER_FINAL', false, ), + forceFreshClaudeReviewerSessionInUnsafeHostMode: readBoolean( + 'PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION', + false, + ), agentLanguage: readText('AGENT_LANGUAGE') ?? '', arbiterDeadlockThreshold: readInteger('ARBITER_DEADLOCK_THRESHOLD', 2), maxRoundTrips, diff --git a/src/config/schema.ts b/src/config/schema.ts index c2f07ae..e1fbd46 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -54,6 +54,7 @@ export interface AppConfig { arbiterAgentType?: AgentType; arbiterServiceId: string | null; carryForwardLatestOwnerFinal: boolean; + forceFreshClaudeReviewerSessionInUnsafeHostMode: boolean; agentLanguage: string; arbiterDeadlockThreshold: number; maxRoundTrips: number; diff --git a/src/message-agent-executor-target.ts b/src/message-agent-executor-target.ts index c217784..e8fe08c 100644 --- a/src/message-agent-executor-target.ts +++ b/src/message-agent-executor-target.ts @@ -8,6 +8,7 @@ import { ARBITER_AGENT_TYPE, REVIEWER_AGENT_TYPE, getRoleModelConfig, + shouldForceFreshClaudeReviewerSessionInUnsafeHostMode, } from './config.js'; import { getAllTasks, @@ -119,7 +120,10 @@ export async function prepareMessageAgentExecutionTarget( const isClaudeCodeAgent = effectiveAgentType === 'claude-code'; const unsafeHostPairedMode = isUnsafeHostPairedModeEnabled(); const forceFreshClaudeReviewerSession = - reviewerMode && isClaudeCodeAgent && unsafeHostPairedMode; + reviewerMode && + isClaudeCodeAgent && + unsafeHostPairedMode && + shouldForceFreshClaudeReviewerSessionInUnsafeHostMode(); const shouldPersistSession = activeRole !== 'arbiter' && !args.forcedAgentType && diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index dd5ecd9..7b2bf63 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -1,4 +1,5 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as config from './config.js'; vi.mock('./agent-runner.js', () => ({ runAgentProcess: vi.fn(), @@ -16,9 +17,11 @@ vi.mock('./config.js', () => ({ CODEX_MAIN_SERVICE_ID: 'codex-main', CODEX_REVIEW_SERVICE_ID: 'codex-review', DATA_DIR: '/tmp/ejclaw-test-data', + PAIRED_FORCE_FRESH_CLAUDE_REVIEWER_SESSION: false, REVIEWER_AGENT_TYPE: 'claude-code', ARBITER_AGENT_TYPE: undefined, SERVICE_SESSION_SCOPE: 'claude', + shouldForceFreshClaudeReviewerSessionInUnsafeHostMode: vi.fn(() => false), isClaudeService: vi.fn(() => true), normalizeServiceId: vi.fn((serviceId: string) => serviceId === 'codex' ? 'codex-main' : serviceId, @@ -290,6 +293,9 @@ describe('runAgentForGroup room memory', () => { vi.resetAllMocks(); resetPairedFollowUpScheduleState(); delete process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE; + vi.mocked( + config.shouldForceFreshClaudeReviewerSessionInUnsafeHostMode, + ).mockReturnValue(false); vi.mocked(agentRunner.runAgentProcess).mockImplementation( async (_group, _input, _onProcess, onOutput) => { await onOutput?.({ @@ -1862,7 +1868,7 @@ describe('runAgentForGroup room memory', () => { ); }); - it('starts reviewer Claude fresh in unsafe host mode instead of resuming a stored session', async () => { + it('keeps reviewer Claude session in unsafe host mode by default', async () => { process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE = '1'; const group = { ...makeGroup(), @@ -1946,6 +1952,110 @@ describe('runAgentForGroup room memory', () => { runId: 'run-review-plan-unsafe-host', }); + expect(agentRunner.runAgentProcess).toHaveBeenCalledWith( + expect.objectContaining({ + folder: 'test-group', + agentType: 'claude-code', + }), + expect.objectContaining({ + sessionId: 'reviewer-session', + }), + expect.any(Function), + expect.any(Function), + expect.objectContaining({ + EJCLAW_UNSAFE_HOST_PAIRED_MODE: '1', + }), + ); + expect(deps.clearSession).not.toHaveBeenCalled(); + }); + + it('starts reviewer Claude fresh in unsafe host mode when explicitly forced', async () => { + process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE = '1'; + vi.mocked( + config.shouldForceFreshClaudeReviewerSessionInUnsafeHostMode, + ).mockReturnValue(true); + 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_agent_type: 'codex', + reviewer_agent_type: 'claude-code', + arbiter_agent_type: null, + 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', + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + 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', + }, + workspace: null, + envOverrides: { + EJCLAW_PAIRED_TASK_ID: 'paired-task-review', + EJCLAW_PAIRED_ROLE: 'reviewer', + EJCLAW_UNSAFE_HOST_PAIRED_MODE: '1', + CLAUDE_CONFIG_DIR: '/tmp/test-group-reviewer', + }, + }); + vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({ + status: 'success', + result: 'review ok', + newSessionId: 'review-session-new', + }); + + await runAgentForGroup(deps, { + group, + prompt: 'please review', + chatJid: 'group@test', + runId: 'run-review-plan-unsafe-host-fresh', + }); + expect(deps.clearSession).toHaveBeenCalledWith('test-group:reviewer'); expect(agentRunner.runAgentProcess).toHaveBeenCalledWith( expect.objectContaining({