Rename readonly reviewer runtime remnants
This commit is contained in:
@@ -79,7 +79,7 @@ vi.mock('os', async () => {
|
||||
});
|
||||
|
||||
import {
|
||||
prepareContainerSessionEnvironment,
|
||||
prepareReadonlySessionEnvironment,
|
||||
prepareGroupEnvironment,
|
||||
} from './agent-runner-environment.js';
|
||||
import * as config from './config.js';
|
||||
@@ -341,12 +341,12 @@ describe('prepareGroupEnvironment codex auth handling', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('prepareContainerSessionEnvironment codex compatibility', () => {
|
||||
describe('prepareReadonlySessionEnvironment codex compatibility', () => {
|
||||
let tempRoot: string;
|
||||
let previousCwd: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempRoot = fs.mkdtempSync(path.join('/tmp', 'ejclaw-container-env-'));
|
||||
tempRoot = fs.mkdtempSync(path.join('/tmp', 'ejclaw-readonly-env-'));
|
||||
previousCwd = process.cwd();
|
||||
process.chdir(tempRoot);
|
||||
|
||||
@@ -369,7 +369,7 @@ describe('prepareContainerSessionEnvironment codex compatibility', () => {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('writes matching AGENTS.md and copies host codex auth/config into the container session', () => {
|
||||
it('writes matching AGENTS.md and copies host codex auth/config into the role-scoped session', () => {
|
||||
vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
|
||||
|
||||
const promptsDir = path.join(tempRoot, 'prompts');
|
||||
@@ -404,8 +404,8 @@ args = ["other.js"]
|
||||
`,
|
||||
);
|
||||
|
||||
const sessionDir = path.join(tempRoot, 'container-reviewer-session');
|
||||
prepareContainerSessionEnvironment({
|
||||
const sessionDir = path.join(tempRoot, 'readonly-reviewer-session');
|
||||
prepareReadonlySessionEnvironment({
|
||||
sessionDir,
|
||||
chatJid: 'dc:test',
|
||||
isMain: false,
|
||||
|
||||
@@ -487,7 +487,7 @@ export function prepareGroupEnvironment(
|
||||
: undefined;
|
||||
// Owner CLAUDE.md: platform rules + owner paired room rules.
|
||||
// Reviewer paired room rules are NOT included — those belong to the
|
||||
// container reviewer only (via prepareContainerSessionEnvironment).
|
||||
// Read-only reviewer/arbiter session only (via prepareReadonlySessionEnvironment).
|
||||
const sessionClaudeMd = [
|
||||
ownerCommonPlatformPrompt,
|
||||
claudePlatformPrompt,
|
||||
@@ -558,14 +558,15 @@ export function prepareGroupEnvironment(
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare the Claude session directory for a container-based reviewer.
|
||||
* Prepare a role-scoped session directory for a host-based read-only reviewer
|
||||
* or arbiter run.
|
||||
*
|
||||
* Writes CLAUDE.md (platform + paired room prompts + global memory + briefing),
|
||||
* syncs skills, and ensures settings.json exist — the same steps that
|
||||
* `prepareGroupEnvironment` does for host-mode agents, but targeted at an
|
||||
* externally provided session directory (the one mounted into the container).
|
||||
* `prepareGroupEnvironment` does for regular host-mode agents, but targeted at
|
||||
* an externally provided session directory.
|
||||
*/
|
||||
export function prepareContainerSessionEnvironment(args: {
|
||||
export function prepareReadonlySessionEnvironment(args: {
|
||||
sessionDir: string;
|
||||
chatJid: string;
|
||||
isMain: boolean;
|
||||
@@ -665,7 +666,7 @@ export function prepareContainerSessionEnvironment(args: {
|
||||
hasGlobalMemory: !!globalClaudeMemory,
|
||||
hasMemoryBriefing: !!memoryBriefing,
|
||||
},
|
||||
'Container session CLAUDE.md written',
|
||||
'Readonly session CLAUDE.md written',
|
||||
);
|
||||
} else if (fs.existsSync(sessionClaudeMdPath)) {
|
||||
fs.unlinkSync(sessionClaudeMdPath);
|
||||
|
||||
@@ -318,9 +318,9 @@ describe('agent-runner timeout behavior', () => {
|
||||
it('uses host IPC and workdir in unsafe host paired mode reviewer sessions', async () => {
|
||||
vi.useRealTimers();
|
||||
fakeProc = createFakeProcess();
|
||||
const prepareContainerSessionEnvironmentSpy = vi.spyOn(
|
||||
const prepareReadonlySessionEnvironmentSpy = vi.spyOn(
|
||||
agentRunnerEnvironment,
|
||||
'prepareContainerSessionEnvironment',
|
||||
'prepareReadonlySessionEnvironment',
|
||||
);
|
||||
|
||||
vi.mocked(fs.existsSync).mockImplementation((p: fs.PathLike) => {
|
||||
@@ -356,7 +356,7 @@ describe('agent-runner timeout behavior', () => {
|
||||
fakeProc.emit('close', 0);
|
||||
const result = await resultPromise;
|
||||
expect(result.status).toBe('success');
|
||||
expect(prepareContainerSessionEnvironmentSpy).toHaveBeenCalledWith(
|
||||
expect(prepareReadonlySessionEnvironmentSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
sessionDir: '/tmp/host-reviewer-session',
|
||||
ipcDir: '/tmp/ejclaw-test-data/ipc/test-group',
|
||||
|
||||
@@ -13,7 +13,7 @@ import {
|
||||
IDLE_TIMEOUT,
|
||||
} from './config.js';
|
||||
import {
|
||||
prepareContainerSessionEnvironment,
|
||||
prepareReadonlySessionEnvironment,
|
||||
prepareGroupEnvironment,
|
||||
} from './agent-runner-environment.js';
|
||||
import { getEnv } from './env.js';
|
||||
@@ -99,7 +99,7 @@ export async function runAgentProcess(
|
||||
(input.roomRoleContext?.role === 'reviewer' ||
|
||||
input.roomRoleContext?.role === 'arbiter')
|
||||
) {
|
||||
prepareContainerSessionEnvironment({
|
||||
prepareReadonlySessionEnvironment({
|
||||
sessionDir: envOverrides.CLAUDE_CONFIG_DIR,
|
||||
chatJid: input.chatJid,
|
||||
isMain: input.isMain,
|
||||
|
||||
@@ -1237,6 +1237,83 @@ describe('runAgentForGroup room memory', () => {
|
||||
expect(result).toBe('success');
|
||||
expect(deps.queue.enqueueMessageCheck).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('re-enqueues the group when a reviewer fails and the task remains review_ready', async () => {
|
||||
const group = { ...makeGroup(), folder: 'test-group' };
|
||||
const deps = makeDeps();
|
||||
|
||||
vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({
|
||||
chat_jid: 'group@test',
|
||||
owner_agent_type: 'claude-code',
|
||||
reviewer_agent_type: 'claude-code',
|
||||
arbiter_agent_type: null,
|
||||
owner_service_id: 'claude',
|
||||
reviewer_service_id: 'claude',
|
||||
arbiter_service_id: null,
|
||||
activated_at: null,
|
||||
reason: null,
|
||||
explicit: false,
|
||||
});
|
||||
vi.mocked(
|
||||
pairedExecutionContext.preparePairedExecutionContext,
|
||||
).mockReturnValue({
|
||||
task: {
|
||||
id: 'paired-task-review-ready',
|
||||
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: 1,
|
||||
review_requested_at: '2026-03-31T00:00:00.000Z',
|
||||
status: 'in_review',
|
||||
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: {},
|
||||
});
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue({
|
||||
id: 'paired-task-review-ready',
|
||||
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: 1,
|
||||
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(agentRunner.runAgentProcess).mockResolvedValue({
|
||||
status: 'error',
|
||||
result: null,
|
||||
error: 'SDK crashed with exit code 1',
|
||||
});
|
||||
|
||||
const result = await runAgentForGroup(deps, {
|
||||
group,
|
||||
prompt: 'please review',
|
||||
chatJid: 'group@test',
|
||||
runId: 'run-review-ready-requeue',
|
||||
forcedRole: 'reviewer',
|
||||
onOutput: async () => {},
|
||||
});
|
||||
|
||||
expect(result).toBe('error');
|
||||
expect(deps.queue.enqueueMessageCheck).toHaveBeenCalledWith('group@test');
|
||||
});
|
||||
});
|
||||
|
||||
describe('runAgentForGroup Claude rotation', () => {
|
||||
|
||||
@@ -1165,6 +1165,28 @@ export async function runAgentForGroup(
|
||||
} else if (finishedCheck?.status !== 'completed') {
|
||||
deps.queue.enqueueMessageCheck(chatJid);
|
||||
}
|
||||
} else if (pairedExecutionContext) {
|
||||
const completedRole = roomRoleContext?.role ?? 'owner';
|
||||
const finishedCheck = getPairedTaskById(pairedExecutionContext.task.id);
|
||||
const shouldRequeuePendingPairedTurn =
|
||||
(completedRole === 'reviewer' || completedRole === 'arbiter') &&
|
||||
(finishedCheck?.status === 'review_ready' ||
|
||||
finishedCheck?.status === 'in_review' ||
|
||||
finishedCheck?.status === 'arbiter_requested' ||
|
||||
finishedCheck?.status === 'in_arbitration' ||
|
||||
finishedCheck?.status === 'merge_ready');
|
||||
if (shouldRequeuePendingPairedTurn) {
|
||||
deps.queue.enqueueMessageCheck(chatJid);
|
||||
log.info(
|
||||
{
|
||||
taskId: pairedExecutionContext.task.id,
|
||||
role: completedRole,
|
||||
pairedExecutionStatus,
|
||||
taskStatus: finishedCheck?.status ?? null,
|
||||
},
|
||||
'Queued paired follow-up after failed reviewer/arbiter execution left a pending task state',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user