paired: decouple unsafe host mode from fresh reviewer sessions
This commit is contained in:
@@ -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';
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -54,6 +54,7 @@ export interface AppConfig {
|
||||
arbiterAgentType?: AgentType;
|
||||
arbiterServiceId: string | null;
|
||||
carryForwardLatestOwnerFinal: boolean;
|
||||
forceFreshClaudeReviewerSessionInUnsafeHostMode: boolean;
|
||||
agentLanguage: string;
|
||||
arbiterDeadlockThreshold: number;
|
||||
maxRoundTrips: number;
|
||||
|
||||
@@ -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 &&
|
||||
|
||||
@@ -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({
|
||||
|
||||
Reference in New Issue
Block a user