claude-bot/EJClaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #1 from phj1081/codex/remove-explicit-paired-review-command-path

Browse Source 
refactor: remove dead paired review and output suppression paths
This commit is contained in:
Eyejoker
2026-03-30 03:35:34 +09:00
committed by GitHub
parent 3015c3ea9a e863db61a8
commit 5e4f1fa8da
15 changed files with 47 additions and 1727 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
src/message-agent-executor.test.ts
View File

@@ -14,7 +14,9 @@ vi.mock('./config.js', () => ({
CODEX_MAIN_SERVICE_ID: 'codex-main',
CODEX_REVIEW_SERVICE_ID: 'codex-review',
DATA_DIR: '/tmp/ejclaw-test-data',
REVIEWER_AGENT_TYPE: 'claude-code',
SERVICE_SESSION_SCOPE: 'claude',
isClaudeService: vi.fn(() => true),
normalizeServiceId: vi.fn((serviceId: string) =>
serviceId === 'codex' ? 'codex-main' : serviceId,
),
@@ -23,6 +25,7 @@ vi.mock('./config.js', () => ({
vi.mock('./db.js', () => ({
createServiceHandoff: vi.fn(),
getAllTasks: vi.fn(() => []),
getLatestOpenPairedTaskForChat: vi.fn(() => undefined),
}));
vi.mock('./service-routing.js', () => ({
@@ -110,7 +113,6 @@ vi.mock('./memento-client.js', () => ({
vi.mock('./paired-execution-context.js', () => ({
completePairedExecutionContext: vi.fn(),
markRoomReviewReady: vi.fn(),
preparePairedExecutionContext: vi.fn(() => undefined),
}));
@@ -222,7 +224,7 @@ describe('runAgentForGroup room memory', () => {
);
});
it('injects structured silent-output instructions into the agent prompt', async () => {
it('passes the prompt through unchanged', async () => {
const group = { ...makeGroup(), folder: 'test-group' };
await runAgentForGroup(makeDeps(), {
@@ -230,15 +232,12 @@ describe('runAgentForGroup room memory', () => {
prompt: 'hello',
chatJid: 'group@test',
runId: 'run-suppress',
suppressToken: '__TEST_SUPPRESS__',
});
expect(agentRunner.runAgentProcess).toHaveBeenCalledWith(
group,
expect.objectContaining({
prompt: expect.stringMatching(
/If you have no user-visible content to send for this turn, output exactly this JSON and nothing else: \{"ejclaw":\{"visibility":"silent"\}\}[\s\S]*If you have already emitted any visible progress, status update, or partial answer earlier in this turn, do not end with the JSON object\. Finish with a short visible final conclusion for the user instead\./,
),
prompt: 'hello',
}),
expect.any(Function),
undefined,
@@ -273,7 +272,7 @@ describe('runAgentForGroup room memory', () => {
);
});
it('adds reviewer silence guidance when the current service is the reviewer for the chat', async () => {
it('keeps the reviewer prompt unchanged when the current service is the reviewer for the chat', async () => {
const group = { ...makeGroup(), folder: 'test-group' };
vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({
chat_jid: 'group@test',
@@ -289,15 +288,12 @@ describe('runAgentForGroup room memory', () => {
prompt: 'hello',
chatJid: 'group@test',
runId: 'run-review-suppress',
suppressToken: '__TEST_SUPPRESS__',
});
expect(agentRunner.runAgentProcess).toHaveBeenCalledWith(
group,
expect.objectContaining({
prompt: expect.stringMatching(
/If you have already emitted any visible progress, status update, or partial answer earlier in this turn, do not end with the JSON object\. Finish with a short visible final conclusion for the user instead\.[\s\S]*If you have not already emitted any visible progress, status update, or partial answer in this turn and you are only agreeing, mirroring, or restating without adding a concrete correction, risk, missing prerequisite, test gap, or code change, output only the JSON object\./,
),
prompt: 'hello',
}),
expect.any(Function),
undefined,
@@ -438,6 +434,7 @@ describe('runAgentForGroup room memory', () => {
pairedExecutionContext.completePairedExecutionContext,
).toHaveBeenCalledWith({
taskId: 'paired-task-1',
role: 'owner',
status: 'succeeded',
summary: 'ok',
});
@@ -515,6 +512,7 @@ describe('runAgentForGroup room memory', () => {
pairedExecutionContext.completePairedExecutionContext,
).toHaveBeenCalledWith({
taskId: 'paired-task-1',
role: 'owner',
status: 'failed',
summary:
'Review snapshot is stale after owner changes. Retry the review once to refresh against the latest owner workspace.',

20
src/message-agent-executor.ts
View File

@@ -40,10 +40,6 @@ import {
REVIEWER_AGENT_TYPE,
isClaudeService,
} from './config.js';
import {
buildStructuredOutputPrompt,
classifySuppressTokenOutput,
} from './output-suppression.js';
import {
activateCodexFailover,
getEffectiveChannelLease,
@@ -80,7 +76,6 @@ export async function runAgentForGroup(
prompt: string;
chatJid: string;
runId: string;
suppressToken?: string;
startSeq?: number | null;
endSeq?: number | null;
onOutput?: (output: AgentOutput) => Promise<void>;
@@ -91,7 +86,6 @@ export async function runAgentForGroup(
prompt,
chatJid,
runId,
suppressToken,
startSeq,
endSeq,
onOutput,
@@ -174,12 +168,7 @@ export async function runAgentForGroup(
runId,
roomRoleContext,
});
const effectivePrompt = buildStructuredOutputPrompt(prompt, {
reviewerMode,
pairedRoom: !!pairedExecutionContext,
gateTurnKind: pairedExecutionContext?.gateTurnKind,
requiresVisibleVerdict: pairedExecutionContext?.requiresVisibleVerdict,
});
const effectivePrompt = prompt;
let pairedExecutionStatus: 'succeeded' | 'failed' = 'failed';
let pairedExecutionSummary: string | null = null;
let pairedExecutionCompleted = false;
@@ -399,14 +388,9 @@ export async function runAgentForGroup(
if (!evaluation.shouldForwardOutput) {
return;
}
const suppressState =
typeof outputText === 'string'
? classifySuppressTokenOutput(outputText, suppressToken)
: 'none';
if (
typeof outputText === 'string' &&
outputText.length > 0 &&
suppressState === 'none'
outputText.length > 0
) {
streamedState = {
...evaluation.state,

564
src/message-runtime.test.ts
View File

@@ -11,15 +11,6 @@ vi.mock('./agent-runner.js', () => ({
writeTasksSnapshot: vi.fn(),
}));
vi.mock('./output-suppression.js', async (importOriginal) => {
const actual =
await importOriginal<typeof import('./output-suppression.js')>();
return {
...actual,
createSuppressToken: vi.fn(() => '__TEST_SUPPRESS__'),
};
});
vi.mock('./config.js', () => ({
DATA_DIR: '/tmp/ejclaw-test-data',
SERVICE_ID: 'claude',
@@ -27,6 +18,7 @@ vi.mock('./config.js', () => ({
SERVICE_SESSION_SCOPE: 'claude',
CODEX_MAIN_SERVICE_ID: 'codex-main',
CODEX_REVIEW_SERVICE_ID: 'codex-review',
REVIEWER_AGENT_TYPE: 'claude-code',
normalizeServiceId: vi.fn((serviceId: string) => serviceId),
isClaudeService: vi.fn(() => true),
isReviewService: vi.fn(() => false),
@@ -36,8 +28,6 @@ vi.mock('./config.js', () => ({
vi.mock('./paired-execution-context.js', () => ({
preparePairedExecutionContext: vi.fn(() => undefined),
completePairedExecutionContext: vi.fn(),
markRoomReviewReady: vi.fn(() => null),
formatRoomReviewReadyMessage: vi.fn(() => null),
}));
vi.mock('./db.js', () => {
@@ -118,6 +108,7 @@ vi.mock('./db.js', () => {
),
getOpenWorkItem: vi.fn(() => undefined),
getPendingServiceHandoffs: vi.fn(() => []),
getLatestOpenPairedTaskForChat: vi.fn(() => undefined),
createProducedWorkItem: vi.fn((input) => ({
id: 1,
group_folder: input.group_folder,
@@ -179,10 +170,7 @@ import * as agentRunner from './agent-runner.js';
import * as db from './db.js';
import { resolveGroupIpcPath } from './group-folder.js';
import { createMessageRuntime } from './message-runtime.js';
import * as outputSuppression from './output-suppression.js';
import * as config from './config.js';
import * as pairedExecutionContext from './paired-execution-context.js';
import * as sessionCommands from './session-commands.js';
import type { Channel, RegisteredGroup } from './types.js';
function makeGroup(agentType: 'claude-code' | 'codex'): RegisteredGroup {
@@ -219,186 +207,6 @@ describe('createMessageRuntime', () => {
vi.mocked(config.isReviewService).mockReturnValue(false);
});
it('surfaces the pending review message through the message-runtime /review path', async () => {
const chatJid = 'group@test';
const group = {
...makeGroup('codex'),
workDir: '/repo/canonical',
};
const channel = makeChannel(chatJid);
const saveState = vi.fn();
const lastAgentTimestamps: Record<string, string> = {};
const pendingMessage = [
'Review request recorded, but the owner workspace is not ready yet.',
'- Task: paired-task-1',
'The task stays review_pending until the owner workspace is prepared.',
].join('\n');
vi.mocked(db.isPairedRoomJid).mockReturnValue(true);
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-review',
chat_jid: chatJid,
sender: 'me@test',
sender_name: 'Me',
content: '/review',
timestamp: '2026-03-29T00:00:00.000Z',
is_from_me: true,
},
]);
const actualSessionCommands = await vi.importActual<
typeof import('./session-commands.js')
>('./session-commands.js');
vi.mocked(sessionCommands.handleSessionCommand).mockImplementation((opts) =>
actualSessionCommands.handleSessionCommand(opts),
);
vi.mocked(pairedExecutionContext.markRoomReviewReady).mockReturnValue({
status: 'pending',
task: {
id: 'paired-task-1',
chat_jid: chatJid,
group_folder: group.folder,
owner_service_id: 'claude',
reviewer_service_id: 'codex-main',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: '2026-03-29T00:00:00.000Z',
status: 'review_ready',
created_at: '2026-03-29T00:00:00.000Z',
updated_at: '2026-03-29T00:00:00.000Z',
},
pendingReason: 'owner-workspace-not-ready',
});
vi.mocked(
pairedExecutionContext.formatRoomReviewReadyMessage,
).mockReturnValue(pendingMessage);
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-review-pending',
reason: 'messages',
});
expect(result).toBe(true);
expect(sessionCommands.handleSessionCommand).toHaveBeenCalled();
expect(pairedExecutionContext.markRoomReviewReady).toHaveBeenCalled();
expect(
pairedExecutionContext.formatRoomReviewReadyMessage,
).toHaveBeenCalled();
expect(channel.sendMessage).toHaveBeenCalledWith(chatJid, pendingMessage);
});
it('surfaces the high-risk plan gate message through the message-runtime /review path', async () => {
const chatJid = 'group@test';
const group = {
...makeGroup('codex'),
workDir: '/repo/canonical',
};
const channel = makeChannel(chatJid);
const saveState = vi.fn();
const lastAgentTimestamps: Record<string, string> = {};
const blockedMessage = [
'Plan review is required before formal review for this high-risk task.',
'- Task: paired-task-1',
'- Plan status: pending',
'Ask the owner to record a plan and have the reviewer approve it before /review.',
].join('\n');
vi.mocked(db.isPairedRoomJid).mockReturnValue(true);
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-review',
chat_jid: chatJid,
sender: 'me@test',
sender_name: 'Me',
content: '/review',
timestamp: '2026-03-29T00:00:00.000Z',
is_from_me: true,
},
]);
const actualSessionCommands = await vi.importActual<
typeof import('./session-commands.js')
>('./session-commands.js');
vi.mocked(sessionCommands.handleSessionCommand).mockImplementation((opts) =>
actualSessionCommands.handleSessionCommand(opts),
);
vi.mocked(pairedExecutionContext.markRoomReviewReady).mockReturnValue({
status: 'pending',
task: {
id: 'paired-task-1',
chat_jid: chatJid,
group_folder: group.folder,
owner_service_id: 'claude',
reviewer_service_id: 'codex-main',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: '2026-03-29T00:00:00.000Z',
updated_at: '2026-03-29T00:00:00.000Z',
},
pendingReason: 'owner-workspace-not-ready',
});
vi.mocked(
pairedExecutionContext.formatRoomReviewReadyMessage,
).mockReturnValue(blockedMessage);
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-review-blocked',
reason: 'messages',
});
expect(result).toBe(true);
expect(channel.sendMessage).toHaveBeenCalledWith(chatJid, blockedMessage);
});
it('ignores generic failure bot messages in paired rooms', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
@@ -527,7 +335,7 @@ describe('createMessageRuntime', () => {
expect(saveState).toHaveBeenCalled();
});
it('does not defer typing-on for suppress-capable review turns', async () => {
it('does not defer typing-on for review turns', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
@@ -1556,65 +1364,6 @@ describe('createMessageRuntime', () => {
expect(channel.sendAndTrack).not.toHaveBeenCalled();
});
it('does not emit a visible message when the final output is the suppress token only', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
const lastAgentTimestamps: Record<string, string> = {};
const saveState = vi.fn();
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-1',
chat_jid: chatJid,
sender: 'user@test',
sender_name: 'User',
content: 'hello',
timestamp: '2026-03-19T00:00:00.000Z',
seq: 1,
},
]);
vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({
status: 'success',
result: '__TEST_SUPPRESS__',
newSessionId: 'session-suppress-run',
});
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-suppress-only',
reason: 'messages',
});
expect(result).toBe(true);
expect(saveState).toHaveBeenCalled();
expect(lastAgentTimestamps[chatJid]).toBe('1');
expect(channel.sendMessage).not.toHaveBeenCalled();
expect(channel.sendAndTrack).not.toHaveBeenCalled();
});
it('does not emit a visible message when the final output is structured silent output', async () => {
const chatJid = 'group@test';
const group = makeGroup('claude-code');
@@ -1769,7 +1518,7 @@ describe('createMessageRuntime', () => {
}
});
it('starts typing immediately for suppress-capable turns with visible output', async () => {
it('starts typing immediately for turns with visible output', async () => {
const chatJid = 'group@test';
const group = makeGroup('claude-code');
const channel = makeChannel(chatJid);
@@ -1793,12 +1542,12 @@ describe('createMessageRuntime', () => {
await onOutput?.({
status: 'success',
phase: 'final',
result: 'visible suppress-capable reply',
result: 'visible reply',
});
return {
status: 'success',
result: null,
newSessionId: 'session-visible-suppress-capable',
newSessionId: 'session-visible-reply',
};
},
);
@@ -1826,316 +1575,19 @@ describe('createMessageRuntime', () => {
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-visible-suppress-capable',
runId: 'run-visible-reply',
reason: 'messages',
});
expect(result).toBe(true);
expect(channel.sendMessage).toHaveBeenCalledWith(
chatJid,
'visible suppress-capable reply',
'visible reply',
);
expect(channel.setTyping).toHaveBeenCalledWith(chatJid, true);
expect(channel.setTyping).toHaveBeenCalledWith(chatJid, false);
});
it('does not grant suppress-token silence authority to codex-main', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
const lastAgentTimestamps: Record<string, string> = {};
const saveState = vi.fn();
vi.mocked(config.isClaudeService).mockReturnValue(false);
vi.mocked(config.isReviewService).mockReturnValue(false);
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-1',
chat_jid: chatJid,
sender: 'user@test',
sender_name: 'User',
content: 'hello',
timestamp: '2026-03-19T00:00:00.000Z',
seq: 1,
},
]);
vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({
status: 'success',
result: 'visible codex reply',
newSessionId: 'session-codex-main',
});
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-codex-main-no-suppress',
reason: 'messages',
});
expect(result).toBe(true);
expect(outputSuppression.createSuppressToken).not.toHaveBeenCalled();
});
it('blocks malformed output when the suppress token is mixed with visible text', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
const lastAgentTimestamps: Record<string, string> = {};
const saveState = vi.fn();
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-1',
chat_jid: chatJid,
sender: 'user@test',
sender_name: 'User',
content: 'hello',
timestamp: '2026-03-19T00:00:00.000Z',
seq: 1,
},
]);
vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({
status: 'success',
result: '동의합니다 __TEST_SUPPRESS__',
newSessionId: 'session-suppress-mixed',
});
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-suppress-mixed',
reason: 'messages',
});
expect(result).toBe(true);
expect(saveState).toHaveBeenCalled();
expect(lastAgentTimestamps[chatJid]).toBe('1');
expect(channel.sendMessage).not.toHaveBeenCalled();
expect(channel.sendAndTrack).not.toHaveBeenCalled();
});
it('suppresses a leaked foreign suppress token even when it differs from the current turn token', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
const lastAgentTimestamps: Record<string, string> = {};
const saveState = vi.fn();
vi.mocked(config.isClaudeService).mockReturnValue(false);
vi.mocked(config.isReviewService).mockReturnValue(true);
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-1',
chat_jid: chatJid,
sender: 'user@test',
sender_name: 'User',
content: 'hello',
timestamp: '2026-03-19T00:00:00.000Z',
seq: 1,
},
]);
vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({
status: 'success',
result: '__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef__',
newSessionId: 'session-foreign-suppress-token',
});
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-foreign-suppress-token',
reason: 'messages',
});
expect(result).toBe(true);
expect(saveState).toHaveBeenCalled();
expect(lastAgentTimestamps[chatJid]).toBe('1');
expect(channel.sendMessage).not.toHaveBeenCalled();
expect(channel.sendAndTrack).not.toHaveBeenCalled();
});
it('suppresses a malformed leaked foreign suppress token without the closing suffix', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
const lastAgentTimestamps: Record<string, string> = {};
const saveState = vi.fn();
vi.mocked(config.isClaudeService).mockReturnValue(false);
vi.mocked(config.isReviewService).mockReturnValue(true);
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-1',
chat_jid: chatJid,
sender: 'user@test',
sender_name: 'User',
content: 'hello',
timestamp: '2026-03-19T00:00:00.000Z',
seq: 1,
},
]);
vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({
status: 'success',
result: '__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef',
newSessionId: 'session-malformed-foreign-suppress-token',
});
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-malformed-foreign-suppress-token',
reason: 'messages',
});
expect(result).toBe(true);
expect(saveState).toHaveBeenCalled();
expect(lastAgentTimestamps[chatJid]).toBe('1');
expect(channel.sendMessage).not.toHaveBeenCalled();
expect(channel.sendAndTrack).not.toHaveBeenCalled();
});
it('suppresses a malformed structured silent envelope without sending a visible message', async () => {
const chatJid = 'group@test';
const group = makeGroup('codex');
const channel = makeChannel(chatJid);
const lastAgentTimestamps: Record<string, string> = {};
const saveState = vi.fn();
vi.mocked(config.isClaudeService).mockReturnValue(false);
vi.mocked(config.isReviewService).mockReturnValue(true);
vi.mocked(db.getMessagesSince).mockReturnValue([
{
id: 'msg-1',
chat_jid: chatJid,
sender: 'user@test',
sender_name: 'User',
content: 'hello',
timestamp: '2026-03-19T00:00:00.000Z',
seq: 1,
},
]);
vi.mocked(agentRunner.runAgentProcess).mockResolvedValue({
status: 'success',
result: '{"ejclaw":{"visibility":"silent"}} extra',
newSessionId: 'session-malformed-structured-silent',
});
const runtime = createMessageRuntime({
assistantName: 'Andy',
idleTimeout: 1_000,
pollInterval: 1_000,
timezone: 'UTC',
triggerPattern: /^@Andy\b/i,
channels: [channel],
queue: {
registerProcess: vi.fn(),
closeStdin: vi.fn(),
notifyIdle: vi.fn(),
} as any,
getRegisteredGroups: () => ({ [chatJid]: group }),
getSessions: () => ({}),
getLastTimestamp: () => '',
setLastTimestamp: vi.fn(),
getLastAgentTimestamps: () => lastAgentTimestamps,
saveState,
persistSession: vi.fn(),
clearSession: vi.fn(),
});
const result = await runtime.processGroupMessages(chatJid, {
runId: 'run-malformed-structured-silent',
reason: 'messages',
});
expect(result).toBe(true);
expect(channel.sendMessage).not.toHaveBeenCalled();
expect(channel.sendAndTrack).not.toHaveBeenCalled();
});
it('resets tracked progress after a final output that becomes empty after formatting', async () => {
vi.useFakeTimers();
const chatJid = 'group@test';

12
src/message-runtime.ts
View File

@@ -20,8 +20,6 @@ import {
type WorkItem,
} from './db.js';
import {
isClaudeService,
isReviewService,
isSessionCommandSenderAllowed,
REVIEWER_AGENT_TYPE,
SERVICE_AGENT_TYPE,
@@ -45,7 +43,6 @@ import {
} from './message-runtime-rules.js';
import { runAgentForGroup } from './message-agent-executor.js';
import { MessageTurnController } from './message-turn-controller.js';
import { createSuppressToken } from './output-suppression.js';
import {
extractSessionCommand,
handleSessionCommand,
@@ -232,7 +229,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
runId: string,
onOutput?: (output: AgentOutput) => Promise<void>,
options?: {
suppressToken?: string;
startSeq?: number | null;
endSeq?: number | null;
},
@@ -251,7 +247,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
prompt,
chatJid,
runId,
suppressToken: options?.suppressToken,
startSeq: options?.startSeq,
endSeq: options?.endSeq,
onOutput,
@@ -280,10 +275,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
const { group, prompt, chatJid, runId, channel, startSeq, endSeq } = args;
const isClaudeCodeAgent =
(group.agentType || 'claude-code') === 'claude-code';
const suppressToken =
isClaudeService() || isReviewService()
? createSuppressToken()
: undefined;
const turnController = new MessageTurnController({
chatJid,
@@ -293,7 +284,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
idleTimeout: deps.idleTimeout,
failureFinalText: FAILURE_FINAL_TEXT,
isClaudeCodeAgent,
suppressToken,
clearSession: () => deps.clearSession(group.folder),
requestClose: (reason) =>
deps.queue.closeStdin(chatJid, { runId, reason }),
@@ -327,7 +317,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
chatJid,
runId,
(result) => turnController.handleOutput(result),
{ suppressToken, startSeq, endSeq },
{ startSeq, endSeq },
);
const { deliverySucceeded, visiblePhase } =

14
src/message-turn-controller.ts
View File

@@ -1,7 +1,6 @@
import { type AgentOutput } from './agent-runner.js';
import { getAgentOutputText, isSilentAgentOutput } from './agent-output.js';
import { getAgentOutputText } from './agent-output.js';
import { logger } from './logger.js';
import { classifySuppressTokenOutput } from './output-suppression.js';
import { formatOutbound } from './router.js';
import { shouldResetSessionOnAgentFailure } from './session-recovery.js';
import { TASK_STATUS_MESSAGE_PREFIX } from './task-watch-status.js';
@@ -30,7 +29,6 @@ interface MessageTurnControllerOptions {
idleTimeout: number;
failureFinalText: string;
isClaudeCodeAgent: boolean;
suppressToken?: string;
clearSession: () => void;
requestClose: (reason: string) => void;
deliverFinalText: (text: string) => Promise<boolean>;
@@ -122,8 +120,6 @@ export class MessageTurnController {
}
const raw = getAgentOutputText(result);
const silentOutput = false;
const suppressState = 'none' as const;
const text = raw ? formatOutbound(raw) : null;
if (raw) {
@@ -267,14 +263,6 @@ export class MessageTurnController {
await this.flushPendingProgress(text);
await this.finalizeProgressMessage();
await this.deliverFinalText(text);
} else if (silentOutput || suppressState !== 'none') {
const shouldPromoteVisibleProgressToFinal =
this.visiblePhase === 'progress' &&
typeof this.latestProgressTextForFinal === 'string';
await this.finalizeProgressMessage();
if (!shouldPromoteVisibleProgressToFinal) {
this.latestProgressTextForFinal = null;
}
} else if (raw) {
logger.info(
{

140
src/output-suppression.test.ts
View File

@@ -1,140 +0,0 @@
import { describe, expect, it } from 'vitest';
import {
buildStructuredOutputPrompt,
classifySuppressTokenOutput,
parseStructuredOutputEnvelope,
} from './output-suppression.js';
describe('classifySuppressTokenOutput', () => {
it('treats the current turn suppress token as exact', () => {
expect(
classifySuppressTokenOutput(
'__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef__',
'__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef__',
),
).toBe('exact');
});
it('treats a leaked foreign suppress token as exact silent output', () => {
expect(
classifySuppressTokenOutput(
'__EJ_SUPPRESS_feedfacefeedfacefeedface__',
'__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef__',
),
).toBe('exact');
});
it('treats a malformed foreign suppress token without the closing suffix as exact silent output', () => {
expect(
classifySuppressTokenOutput(
'__EJ_SUPPRESS_feedfacefeedfacefeedface',
'__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef__',
),
).toBe('exact');
});
it('treats a suppress token embedded in visible text as mixed', () => {
expect(
classifySuppressTokenOutput(
'prefix __EJ_SUPPRESS_feedfacefeedfacefeedface__ suffix',
'__EJ_SUPPRESS_deadbeefdeadbeefdeadbeef__',
),
).toBe('mixed');
});
it('treats the exact structured silent envelope as exact silent output', () => {
expect(
classifySuppressTokenOutput(
'{"ejclaw":{"visibility":"silent"}}',
undefined,
),
).toBe('exact');
});
it('treats a truncated structured silent envelope as mixed', () => {
expect(
classifySuppressTokenOutput(
'{"ejclaw":{"visibility":"silent"',
undefined,
),
).toBe('mixed');
});
it('treats a structured silent envelope mixed with extra text as mixed', () => {
expect(
classifySuppressTokenOutput(
'{"ejclaw":{"visibility":"silent"}} extra',
undefined,
),
).toBe('mixed');
});
});
describe('parseStructuredOutputEnvelope', () => {
it('parses the exact silent envelope', () => {
expect(
parseStructuredOutputEnvelope('{"ejclaw":{"visibility":"silent"}}'),
).toEqual({ visibility: 'silent' });
});
it('parses a public envelope', () => {
expect(
parseStructuredOutputEnvelope(
'{"ejclaw":{"visibility":"public","text":"hello"}}',
),
).toEqual({ visibility: 'public', text: 'hello' });
});
it('parses a public envelope with a reviewer verdict', () => {
expect(
parseStructuredOutputEnvelope(
'{"ejclaw":{"visibility":"public","verdict":"done_with_concerns","text":"**DONE_WITH_CONCERNS**"}}',
),
).toEqual({
visibility: 'public',
verdict: 'done_with_concerns',
text: '**DONE_WITH_CONCERNS**',
});
});
});
describe('buildStructuredOutputPrompt', () => {
it('prepends the structured output control block', () => {
expect(buildStructuredOutputPrompt('hello')).toContain(
'If you have no user-visible content to send for this turn, output exactly this JSON and nothing else: {"ejclaw":{"visibility":"silent"}}',
);
expect(buildStructuredOutputPrompt('hello')).toContain(
'If you have already emitted any visible progress, status update, or partial answer earlier in this turn, do not end with the JSON object. Finish with a short visible final conclusion for the user instead.',
);
});
it('tightens the reviewer silent rule when reviewer mode is enabled', () => {
expect(
buildStructuredOutputPrompt('hello', { reviewerMode: true }),
).toContain(
'If you have not already emitted any visible progress, status update, or partial answer in this turn and you are only agreeing, mirroring, or restating without adding a concrete correction, risk, missing prerequisite, test gap, or code change, output only the JSON object.',
);
});
it('requires a visible structured verdict on reviewer gate turns', () => {
expect(
buildStructuredOutputPrompt('hello', {
reviewerMode: true,
gateTurnKind: 'implementation_start',
requiresVisibleVerdict: true,
}),
).toContain(
'This turn is a paired-room gate turn for implementation_start. Silent output is forbidden.',
);
expect(
buildStructuredOutputPrompt('hello', {
reviewerMode: true,
gateTurnKind: 'implementation_start',
requiresVisibleVerdict: true,
}),
).toContain(
'Allowed verdict values are: "done", "done_with_concerns", "blocked".',
);
});
});

33
src/output-suppression.ts
View File

@@ -1,33 +0,0 @@
/**
* Output suppression was removed — harness-level protections (isOwnMessage filter,
* shouldSkipBotOnlyCollaboration, PAIRED_MAX_ROUND_TRIPS) now prevent infinite loops.
*
* Retained exports are stubs so callers compile without changes.
*/
export function createSuppressToken(): string | undefined {
return undefined;
}
export function classifySuppressTokenOutput(
_rawText: string,
_suppressToken: string | undefined,
): 'none' {
return 'none';
}
export function buildStructuredOutputPrompt(
prompt: string,
_options?: {
reviewerMode?: boolean;
pairedRoom?: boolean;
gateTurnKind?: string | null;
requiresVisibleVerdict?: boolean;
},
): string {
return prompt;
}
export function parseStructuredOutputEnvelope(_rawText: string): null {
return null;
}

284
src/paired-execution-context.test.ts
View File

@@ -33,8 +33,6 @@ vi.mock('./logger.js', () => ({
import * as db from './db.js';
import {
completePairedExecutionContext,
formatRoomReviewReadyMessage,
markRoomReviewReady,
preparePairedExecutionContext,
} from './paired-execution-context.js';
import * as pairedWorkspaceManager from './paired-workspace-manager.js';
@@ -137,78 +135,6 @@ function buildWorkspace(
};
}
async function importExecutionContextForService(serviceId: string) {
vi.resetModules();
const dbModule = {
createPairedTask: vi.fn(),
getLatestPairedTaskForChat: vi.fn(),
getLatestOpenPairedTaskForChat: vi.fn(),
getPairedTaskById: vi.fn(),
getPairedWorkspace: vi.fn(),
updatePairedTask: vi.fn(),
upsertPairedProject: vi.fn(),
};
dbModule.getLatestOpenPairedTaskForChat.mockReturnValue(undefined);
dbModule.getPairedTaskById.mockReturnValue(undefined);
dbModule.getPairedWorkspace.mockReturnValue(undefined);
const pairedWorkspaceManagerModule = {
markPairedTaskReviewReady: vi.fn(),
prepareReviewerWorkspaceForExecution: vi.fn(),
provisionOwnerWorkspaceForPairedTask: vi.fn(),
};
pairedWorkspaceManagerModule.provisionOwnerWorkspaceForPairedTask.mockReturnValue(
{
id: 'task-1:owner',
task_id: 'task-1',
role: 'owner',
workspace_dir: '/tmp/paired/task-1/owner',
snapshot_source_dir: null,
snapshot_ref: null,
status: 'ready',
snapshot_refreshed_at: null,
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
},
);
pairedWorkspaceManagerModule.prepareReviewerWorkspaceForExecution.mockReturnValue(
{
workspace: null,
autoRefreshed: false,
},
);
vi.doMock('./db.js', () => dbModule);
vi.doMock(
'./paired-workspace-manager.js',
() => pairedWorkspaceManagerModule,
);
vi.doMock('./logger.js', () => ({
logger: {
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
},
}));
vi.doMock('./config.js', async () => {
const actual =
await vi.importActual<typeof import('./config.js')>('./config.js');
return {
...actual,
SERVICE_ID: serviceId,
};
});
const module = await import('./paired-execution-context.js');
return {
dbModule,
pairedWorkspaceManagerModule,
markRoomReviewReady: module.markRoomReviewReady,
};
}
describe('paired execution context', () => {
beforeEach(() => {
vi.resetAllMocks();
@@ -325,7 +251,7 @@ describe('paired execution context', () => {
});
});
it('does not change task state for a general reviewer turn before /review', () => {
it('does not change task state when the reviewer snapshot is not ready', () => {
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
@@ -347,7 +273,7 @@ describe('paired execution context', () => {
workspace: null,
autoRefreshed: false,
blockMessage:
'Review snapshot is not ready yet. Ask the owner to run /review (or /review-ready) after preparing changes.',
'Review snapshot is not ready yet. Wait for the owner to complete a turn so the reviewer snapshot can be prepared.',
});
const result = preparePairedExecutionContext({
@@ -358,7 +284,7 @@ describe('paired execution context', () => {
});
expect(result?.blockMessage).toBe(
'Review snapshot is not ready yet. Ask the owner to run /review (or /review-ready) after preparing changes.',
'Review snapshot is not ready yet. Wait for the owner to complete a turn so the reviewer snapshot can be prepared.',
);
expect(db.updatePairedTask).not.toHaveBeenCalledWith(
'task-1',
@@ -531,208 +457,4 @@ describe('paired execution context', () => {
);
});
it('returns ready for /review on the owner service', async () => {
const {
dbModule,
pairedWorkspaceManagerModule,
markRoomReviewReady: isolatedMarkRoomReviewReady,
} = await importExecutionContextForService('codex-main');
dbModule.getLatestOpenPairedTaskForChat.mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
group_folder: group.folder,
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
});
dbModule.getPairedTaskById.mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
group_folder: group.folder,
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: '2026-03-28T00:00:00.000Z',
status: 'review_ready',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
});
pairedWorkspaceManagerModule.markPairedTaskReviewReady.mockReturnValue({
ownerWorkspace: {
id: 'task-1:owner',
task_id: 'task-1',
role: 'owner',
workspace_dir: '/tmp/paired/task-1/owner',
snapshot_source_dir: null,
snapshot_ref: null,
status: 'ready',
snapshot_refreshed_at: null,
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
},
reviewerWorkspace: {
id: 'task-1:reviewer',
task_id: 'task-1',
role: 'reviewer',
workspace_dir: '/tmp/paired/task-1/reviewer',
snapshot_source_dir: '/tmp/paired/task-1/owner',
snapshot_ref: 'fingerprint-1',
status: 'ready',
snapshot_refreshed_at: '2026-03-28T00:00:00.000Z',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
},
});
const result = isolatedMarkRoomReviewReady({
group,
chatJid: 'dc:test',
roomRoleContext: ownerContext,
});
expect(
pairedWorkspaceManagerModule.provisionOwnerWorkspaceForPairedTask,
).toHaveBeenCalledWith('task-1');
expect(
pairedWorkspaceManagerModule.markPairedTaskReviewReady,
).toHaveBeenCalledWith('task-1');
expect(result?.status).toBe('ready');
if (!result || result.status !== 'ready') {
throw new Error('expected ready review result');
}
expect(result.task.status).toBe('review_ready');
});
it('returns pending for /review on the reviewer service before the owner workspace is visible', async () => {
const {
dbModule,
pairedWorkspaceManagerModule,
markRoomReviewReady: isolatedMarkRoomReviewReady,
} = await importExecutionContextForService('codex-review');
dbModule.getLatestOpenPairedTaskForChat.mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
group_folder: group.folder,
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
});
dbModule.getPairedTaskById.mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
group_folder: group.folder,
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: '2026-03-28T00:00:00.000Z',
status: 'review_ready',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
});
pairedWorkspaceManagerModule.markPairedTaskReviewReady.mockReturnValue(
null,
);
const result = isolatedMarkRoomReviewReady({
group,
chatJid: 'dc:test',
roomRoleContext: ownerContext,
});
expect(
pairedWorkspaceManagerModule.provisionOwnerWorkspaceForPairedTask,
).not.toHaveBeenCalled();
expect(dbModule.getPairedWorkspace).toHaveBeenCalledWith('task-1', 'owner');
expect(
pairedWorkspaceManagerModule.markPairedTaskReviewReady,
).toHaveBeenCalledWith('task-1');
expect(result).toEqual({
status: 'pending',
task: expect.objectContaining({
id: 'task-1',
status: 'review_ready',
}),
pendingReason: 'owner-workspace-not-ready',
});
});
it('keeps review_ready and returns a pending result when owner workspace is not ready', () => {
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
group_folder: group.folder,
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
});
vi.mocked(db.getPairedTaskById).mockReturnValue({
id: 'task-1',
chat_jid: 'dc:test',
group_folder: group.folder,
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: '2026-03-29T00:00:00.000Z',
status: 'review_ready',
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-29T00:00:00.000Z',
});
vi.mocked(pairedWorkspaceManager.markPairedTaskReviewReady).mockReturnValue(
null,
);
const result = markRoomReviewReady({
group,
chatJid: 'dc:test',
roomRoleContext: ownerContext,
});
expect(result).toEqual({
status: 'pending',
task: expect.objectContaining({
id: 'task-1',
status: 'review_ready',
}),
pendingReason: 'owner-workspace-not-ready',
});
expect(formatRoomReviewReadyMessage(result)).toBe(
[
'Review request recorded, but the owner workspace is not ready yet.',
'- Task: task-1',
'The task stays review_pending until the owner workspace is prepared.',
].join('\n'),
);
});
});

109
src/paired-execution-context.ts
View File

@@ -5,8 +5,6 @@ import path from 'path';
import {
DATA_DIR,
SERVICE_ID,
normalizeServiceId,
PAIRED_MAX_ROUND_TRIPS,
} from './config.js';
import {
@@ -476,110 +474,3 @@ export function completePairedExecutionContext(args: {
}
}
}
// ---------------------------------------------------------------------------
// markRoomReviewReady
// ---------------------------------------------------------------------------
export type MarkRoomReviewReadyResult =
| {
status: 'ready';
task: PairedTask;
ownerWorkspace: PairedWorkspace;
reviewerWorkspace: PairedWorkspace;
}
| {
status: 'pending';
task: PairedTask;
pendingReason: 'owner-workspace-not-ready';
}
| null;
export function markRoomReviewReady(args: {
group: RegisteredGroup;
chatJid: string;
roomRoleContext?: RoomRoleContext;
}): MarkRoomReviewReadyResult {
const { group, chatJid, roomRoleContext } = args;
if (!roomRoleContext || !group.workDir) {
return null;
}
const task = ensureActiveTask(group, chatJid, roomRoleContext);
if (!task) {
return null;
}
const isOwnerService =
normalizeServiceId(SERVICE_ID) === task.owner_service_id;
const ownerWorkspace =
(isOwnerService
? provisionOwnerWorkspaceForPairedTask(task.id)
: getPairedWorkspace(task.id, 'owner')) ?? null;
if (!ownerWorkspace) {
markPairedTaskReviewReady(task.id);
return {
status: 'pending',
task: getPairedTaskById(task.id) ?? task,
pendingReason: 'owner-workspace-not-ready',
};
}
// Update task status and refresh reviewer snapshot
const reviewResult = markPairedTaskReviewReady(task.id);
const latestTask = getPairedTaskById(task.id) ?? task;
if (reviewResult) {
return {
status: 'ready',
task: latestTask,
ownerWorkspace: reviewResult.ownerWorkspace,
reviewerWorkspace: reviewResult.reviewerWorkspace,
};
}
// Snapshot refresh succeeded but result was null — try reading persisted state
const reviewerWorkspace = getPairedWorkspace(task.id, 'reviewer');
if (!reviewerWorkspace) {
return {
status: 'pending',
task: latestTask,
pendingReason: 'owner-workspace-not-ready',
};
}
return {
status: 'ready',
task: latestTask,
ownerWorkspace,
reviewerWorkspace,
};
}
// ---------------------------------------------------------------------------
// formatRoomReviewReadyMessage
// ---------------------------------------------------------------------------
export function formatRoomReviewReadyMessage(
result: MarkRoomReviewReadyResult,
): string | null {
if (!result) {
return null;
}
if (result.status === 'pending') {
return [
'Review request recorded, but the owner workspace is not ready yet.',
`- Task: ${result.task.id}`,
'The task stays review_pending until the owner workspace is prepared.',
].join('\n');
}
return [
'Review snapshot updated.',
`- Task: ${result.task.id}`,
`- Owner workspace: ${result.ownerWorkspace.workspace_dir}`,
`- Reviewer snapshot: ${result.reviewerWorkspace.workspace_dir}`,
].join('\n');
}

240
src/paired-review-command.test.ts
View File

@@ -1,240 +0,0 @@
import { execFileSync } from 'child_process';
import fs from 'fs';
import path from 'path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { RegisteredGroup, RoomRoleContext } from './types.js';
function runGit(args: string[], cwd: string): string {
return execFileSync('git', args, {
cwd,
encoding: 'utf-8',
stdio: ['ignore', 'pipe', 'pipe'],
}).trim();
}
async function loadModules() {
const db = await import('./db.js');
const executionContext = await import('./paired-execution-context.js');
return { db, executionContext };
}
const ownerContext: RoomRoleContext = {
serviceId: 'codex-main',
role: 'owner',
ownerServiceId: 'codex-main',
reviewerServiceId: 'codex-review',
failoverOwner: false,
};
const reviewerContext: RoomRoleContext = {
serviceId: 'codex-review',
role: 'reviewer',
ownerServiceId: 'codex-main',
reviewerServiceId: 'codex-review',
failoverOwner: false,
};
describe('paired /review command path', () => {
let tempRoot: string;
let previousDataDir: string | undefined;
let previousGroupsDir: string | undefined;
let previousStoreDir: string | undefined;
let previousServiceId: string | undefined;
beforeEach(() => {
tempRoot = fs.mkdtempSync(path.join('/tmp', 'ejclaw-review-command-'));
previousDataDir = process.env.EJCLAW_DATA_DIR;
previousGroupsDir = process.env.EJCLAW_GROUPS_DIR;
previousStoreDir = process.env.EJCLAW_STORE_DIR;
previousServiceId = process.env.SERVICE_ID;
process.env.EJCLAW_STORE_DIR = path.join(tempRoot, 'shared-store');
process.env.EJCLAW_GROUPS_DIR = path.join(tempRoot, 'shared-groups');
vi.resetModules();
});
afterEach(() => {
if (previousDataDir === undefined) delete process.env.EJCLAW_DATA_DIR;
else process.env.EJCLAW_DATA_DIR = previousDataDir;
if (previousGroupsDir === undefined) delete process.env.EJCLAW_GROUPS_DIR;
else process.env.EJCLAW_GROUPS_DIR = previousGroupsDir;
if (previousStoreDir === undefined) delete process.env.EJCLAW_STORE_DIR;
else process.env.EJCLAW_STORE_DIR = previousStoreDir;
if (previousServiceId === undefined) delete process.env.SERVICE_ID;
else process.env.SERVICE_ID = previousServiceId;
fs.rmSync(tempRoot, { recursive: true, force: true });
});
it('records the first owner /review event with the provisioned owner workspace fingerprint', async () => {
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'original\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const group: RegisteredGroup = {
name: 'Paired Room',
folder: 'paired-room',
trigger: '@codex',
added_at: '2026-03-28T00:00:00.000Z',
agentType: 'codex',
workDir: canonicalDir,
};
process.env.EJCLAW_DATA_DIR = path.join(tempRoot, 'data-owner');
process.env.SERVICE_ID = 'codex-main';
vi.resetModules();
const { db, executionContext } = await loadModules();
db.initDatabase();
const result = executionContext.markRoomReviewReady({
group,
chatJid: 'dc:test',
roomRoleContext: ownerContext,
});
expect(result?.status).toBe('ready');
if (!result || result.status !== 'ready') {
throw new Error('expected owner /review to prepare a ready snapshot');
}
expect(result.reviewerWorkspace.snapshot_ref).toBeTruthy();
});
it('reuses the db owner workspace when reviewer service handles /review', async () => {
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'original\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const group: RegisteredGroup = {
name: 'Paired Room',
folder: 'paired-room',
trigger: '@codex',
added_at: '2026-03-28T00:00:00.000Z',
agentType: 'codex',
workDir: canonicalDir,
};
process.env.EJCLAW_DATA_DIR = path.join(tempRoot, 'data-owner');
process.env.SERVICE_ID = 'codex-main';
vi.resetModules();
let { db, executionContext } = await loadModules();
db.initDatabase();
const ownerResult = executionContext.preparePairedExecutionContext({
group,
chatJid: 'dc:test',
runId: 'run-owner',
roomRoleContext: ownerContext,
});
expect(ownerResult?.workspace?.workspace_dir).toBeTruthy();
const ownerWorkspaceDir = ownerResult!.workspace!.workspace_dir;
fs.writeFileSync(
path.join(ownerWorkspaceDir, 'README.md'),
'owner change\n',
);
process.env.EJCLAW_DATA_DIR = path.join(tempRoot, 'data-review');
process.env.SERVICE_ID = 'codex-review';
vi.resetModules();
({ db, executionContext } = await loadModules());
db.initDatabase();
const result = executionContext.markRoomReviewReady({
group,
chatJid: 'dc:test',
roomRoleContext: reviewerContext,
});
const reviewerLocalOwnerDir = path.join(
tempRoot,
'data-review',
'workspaces',
'paired-room',
'tasks',
ownerResult!.task.id,
'owner',
);
expect(result).not.toBeNull();
expect(result?.status).toBe('ready');
if (!result || result.status !== 'ready') {
throw new Error('expected reviewer /review to prepare a ready snapshot');
}
expect(result.ownerWorkspace.workspace_dir).toBe(ownerWorkspaceDir);
expect(result.reviewerWorkspace.snapshot_source_dir).toBe(
ownerWorkspaceDir,
);
expect(
fs.readFileSync(
path.join(result.reviewerWorkspace.workspace_dir, 'README.md'),
'utf-8',
),
).toBe('owner change\n');
expect(fs.existsSync(reviewerLocalOwnerDir)).toBe(false);
});
it('keeps review_pending when reviewer service handles /review before owner workspace exists', async () => {
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'original\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const group: RegisteredGroup = {
name: 'Paired Room',
folder: 'paired-room',
trigger: '@codex',
added_at: '2026-03-28T00:00:00.000Z',
agentType: 'codex',
workDir: canonicalDir,
};
process.env.EJCLAW_DATA_DIR = path.join(tempRoot, 'data-review');
process.env.SERVICE_ID = 'codex-review';
vi.resetModules();
const { db, executionContext } = await loadModules();
db.initDatabase();
const result = executionContext.markRoomReviewReady({
group,
chatJid: 'dc:test',
roomRoleContext: reviewerContext,
});
const task = db.getLatestOpenPairedTaskForChat('dc:test');
const reviewerLocalOwnerDir = path.join(
tempRoot,
'data-review',
'workspaces',
'paired-room',
'tasks',
task!.id,
'owner',
);
expect(result).toEqual({
status: 'pending',
task: expect.objectContaining({
id: task!.id,
status: 'review_ready',
}),
pendingReason: 'owner-workspace-not-ready',
});
expect(task?.status).toBe('review_ready');
expect(task?.review_requested_at).toBeTruthy();
expect(fs.existsSync(reviewerLocalOwnerDir)).toBe(false);
});
});

274
src/paired-workspace-manager.test.ts
View File

@@ -45,7 +45,7 @@ describe('paired workspace manager', () => {
fs.rmSync(tempRoot, { recursive: true, force: true });
});
it('provisions an owner worktree and refreshes a reviewer shadow snapshot', async () => {
it('registers the owner workspace for reviewer execution when review is requested', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
@@ -113,15 +113,10 @@ describe('paired workspace manager', () => {
'utf-8',
),
).toBe('review me\n');
expect(
runGit(
['config', '--local', '--get', 'remote.origin.pushurl'],
reviewerWorkspace.workspace_dir,
),
).toBe('DISABLED_BY_EJCLAW');
expect(
runGit(['status', '--short'], reviewerWorkspace.workspace_dir),
).toContain('M README.md');
expect(reviewerWorkspace.workspace_dir).toBe(ownerWorkspace.workspace_dir);
expect(reviewerWorkspace.snapshot_source_dir).toBe(
ownerWorkspace.workspace_dir,
);
expect(db.getPairedTaskById('paired-task-1')?.status).toBe('review_ready');
expect(
db.getPairedTaskById('paired-task-1')?.review_requested_at,
@@ -129,57 +124,9 @@ describe('paired workspace manager', () => {
expect(
db.getPairedWorkspace('paired-task-1', 'reviewer')?.snapshot_refreshed_at,
).toBeTruthy();
expect(
db.getPairedWorkspace('paired-task-1', 'reviewer')?.snapshot_ref,
).toBeTruthy();
});
it('keeps review_pending when /review is requested before an owner workspace exists', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'original\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const now = '2026-03-28T00:00:00.000Z';
db.upsertPairedProject({
chat_jid: 'dc:test',
group_folder: 'paired-room',
canonical_work_dir: canonicalDir,
created_at: now,
updated_at: now,
});
db.createPairedTask({
id: 'paired-task-pending',
chat_jid: 'dc:test',
group_folder: 'paired-room',
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: now,
updated_at: now,
});
const result = manager.markPairedTaskReviewReady('paired-task-pending');
expect(result).toBeNull();
expect(db.getPairedTaskById('paired-task-pending')?.status).toBe(
'review_pending',
expect(db.getPairedWorkspace('paired-task-1', 'reviewer')?.snapshot_ref).toBe(
null,
);
expect(
db.getPairedTaskById('paired-task-pending')?.review_requested_at,
).toBeTruthy();
});
it('uses the shared DB owner workspace across service-local data dirs', async () => {
@@ -541,63 +488,7 @@ describe('paired workspace manager', () => {
).toBe('EXAMPLE=1\n');
});
it('does not auto-refresh a missing reviewer snapshot while the task is still draft', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'base\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const now = '2026-03-28T00:00:00.000Z';
db.upsertPairedProject({
chat_jid: 'dc:test',
group_folder: 'paired-room',
canonical_work_dir: canonicalDir,
created_at: now,
updated_at: now,
});
db.createPairedTask({
id: 'paired-task-5',
chat_jid: 'dc:test',
group_folder: 'paired-room',
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: now,
updated_at: now,
});
const ownerWorkspace =
manager.provisionOwnerWorkspaceForPairedTask('paired-task-5');
fs.writeFileSync(
path.join(ownerWorkspace.workspace_dir, 'README.md'),
'owner change\n',
);
const result = manager.prepareReviewerWorkspaceForExecution(
db.getPairedTaskById('paired-task-5')!,
);
expect(result.workspace).toBeNull();
expect(result.autoRefreshed).toBe(false);
expect(result.blockMessage).toBe(
'Review snapshot is not ready yet. Ask the owner to run /review (or /review-ready) after preparing changes.',
);
expect(db.getPairedTaskById('paired-task-5')?.status).toBe('draft');
});
it('auto-refreshes a missing reviewer snapshot after an explicit review request', async () => {
it('registers a reviewer workspace when an explicit review request already exists', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
@@ -645,8 +536,12 @@ describe('paired workspace manager', () => {
db.getPairedTaskById('paired-task-5b')!,
);
expect(result.autoRefreshed).toBe(true);
expect(result.autoRefreshed).toBe(false);
expect(result.blockMessage).toBeUndefined();
expect(result.workspace?.workspace_dir).toBe(ownerWorkspace.workspace_dir);
expect(result.workspace?.snapshot_source_dir).toBe(
ownerWorkspace.workspace_dir,
);
expect(
fs.readFileSync(
path.join(result.workspace!.workspace_dir, 'README.md'),
@@ -659,71 +554,7 @@ describe('paired workspace manager', () => {
);
});
it('auto-refreshes once the owner workspace appears after review_pending was already recorded', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'base\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const now = '2026-03-28T00:00:00.000Z';
db.upsertPairedProject({
chat_jid: 'dc:test',
group_folder: 'paired-room',
canonical_work_dir: canonicalDir,
created_at: now,
updated_at: now,
});
db.createPairedTask({
id: 'paired-task-5c',
chat_jid: 'dc:test',
group_folder: 'paired-room',
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: now,
updated_at: now,
});
expect(manager.markPairedTaskReviewReady('paired-task-5c')).toBeNull();
expect(db.getPairedTaskById('paired-task-5c')?.status).toBe(
'review_pending',
);
const ownerWorkspace =
manager.provisionOwnerWorkspaceForPairedTask('paired-task-5c');
fs.writeFileSync(
path.join(ownerWorkspace.workspace_dir, 'README.md'),
'owner ready now\n',
);
const result = manager.prepareReviewerWorkspaceForExecution(
db.getPairedTaskById('paired-task-5c')!,
);
expect(result.autoRefreshed).toBe(true);
expect(result.blockMessage).toBeUndefined();
expect(
fs.readFileSync(
path.join(result.workspace!.workspace_dir, 'README.md'),
'utf-8',
),
).toBe('owner ready now\n');
expect(db.getPairedTaskById('paired-task-5c')?.status).toBe('review_ready');
});
it('blocks once when an in-review snapshot becomes stale, then refreshes on retry', async () => {
it('reuses the live owner workspace during in-review turns', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
@@ -777,87 +608,22 @@ describe('paired workspace manager', () => {
'owner changed again\n',
);
const blocked = manager.prepareReviewerWorkspaceForExecution(
const result = manager.prepareReviewerWorkspaceForExecution(
db.getPairedTaskById('paired-task-6')!,
);
expect(blocked.workspace).toBeNull();
expect(blocked.autoRefreshed).toBe(false);
expect(blocked.blockMessage).toBe(
'Review snapshot is stale after owner changes. Retry the review once to refresh against the latest owner workspace.',
);
expect(db.getPairedTaskById('paired-task-6')?.status).toBe(
'review_pending',
);
expect(result.workspace?.workspace_dir).toBe(ownerWorkspace.workspace_dir);
expect(result.autoRefreshed).toBe(false);
expect(result.blockMessage).toBeUndefined();
expect(db.getPairedTaskById('paired-task-6')?.status).toBe('in_review');
expect(
db.getPairedTaskById('paired-task-6')?.review_requested_at,
).toBeTruthy();
expect(db.getPairedWorkspace('paired-task-6', 'reviewer')?.status).toBe(
'stale',
);
const refreshed = manager.prepareReviewerWorkspaceForExecution(
db.getPairedTaskById('paired-task-6')!,
);
expect(refreshed.autoRefreshed).toBe(true);
expect(refreshed.blockMessage).toBeUndefined();
expect(
fs.readFileSync(
path.join(refreshed.workspace!.workspace_dir, 'README.md'),
path.join(result.workspace!.workspace_dir, 'README.md'),
'utf-8',
),
).toBe('owner changed again\n');
expect(
db.getPairedTaskById('paired-task-6')?.review_requested_at,
).toBeTruthy();
});
it('blocks reviewer execution for a high-risk task until the plan is approved', async () => {
const { db, manager } = await loadModules();
db._initTestDatabase();
const canonicalDir = path.join(tempRoot, 'canonical');
fs.mkdirSync(canonicalDir, { recursive: true });
runGit(['init'], canonicalDir);
runGit(['config', 'user.email', 'test@example.com'], canonicalDir);
runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir);
fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'base\n');
runGit(['add', 'README.md'], canonicalDir);
runGit(['commit', '-m', 'initial'], canonicalDir);
const now = '2026-03-28T00:00:00.000Z';
db.upsertPairedProject({
chat_jid: 'dc:test',
group_folder: 'paired-room',
canonical_work_dir: canonicalDir,
created_at: now,
updated_at: now,
});
db.createPairedTask({
id: 'paired-task-plan-gate',
chat_jid: 'dc:test',
group_folder: 'paired-room',
owner_service_id: 'codex-main',
reviewer_service_id: 'codex-review',
title: null,
source_ref: 'HEAD',
plan_notes: null,
round_trip_count: 0,
review_requested_at: null,
status: 'active',
created_at: now,
updated_at: now,
});
const result = manager.prepareReviewerWorkspaceForExecution(
db.getPairedTaskById('paired-task-plan-gate')!,
);
expect(result).toEqual({
workspace: null,
autoRefreshed: false,
blockMessage:
'Plan review is required before formal review for this high-risk task.',
});
});
});

2
src/paired-workspace-manager.ts
View File

@@ -18,7 +18,7 @@ import type { PairedTask, PairedWorkspace } from './types.js';
const REVIEWER_SNAPSHOT_STALE_BLOCK_MESSAGE =
'Review snapshot is stale after owner changes. Retry the review once to refresh against the latest owner workspace.';
const REVIEWER_SNAPSHOT_NOT_READY_BLOCK_MESSAGE =
'Review snapshot is not ready yet. Ask the owner to run /review (or /review-ready) after preparing changes.';
'Review snapshot is not ready yet. Wait for the owner to complete a turn so the reviewer snapshot can be prepared.';
const REVIEWER_SNAPSHOT_DENY_SEGMENTS = new Set([
'.git',
'.claude',

4
src/service-routing.test.ts
View File

@@ -69,8 +69,8 @@ describe('service-routing failover leases', () => {
expect(getEffectiveChannelLease('dc:paired')).toMatchObject({
chat_jid: 'dc:paired',
owner_service_id: 'claude',
reviewer_service_id: 'codex-main',
owner_service_id: 'codex-main',
reviewer_service_id: 'claude',
explicit: false,
});
});

55
src/session-commands.test.ts
View File

@@ -23,20 +23,6 @@ describe('extractSessionCommand', () => {
expect(extractSessionCommand('/clear', trigger)).toBe('/clear');
});
it('detects bare /review', () => {
expect(extractSessionCommand('/review', trigger)).toBe('/review');
});
it('detects bare /deploy-complete', () => {
expect(extractSessionCommand('/deploy-complete', trigger)).toBe(
'/deploy-complete',
);
});
it('normalizes /review-ready to /review', () => {
expect(extractSessionCommand('/review-ready', trigger)).toBe('/review');
});
it('rejects /compact with extra text', () => {
expect(extractSessionCommand('/compact now please', trigger)).toBeNull();
});
@@ -97,47 +83,6 @@ describe('isSessionCommandControlMessage', () => {
).toBe(true);
});
it('matches multiline review snapshot output', () => {
expect(
isSessionCommandControlMessage(
'Review snapshot updated.\n- Task: task-1',
),
).toBe(true);
});
it('matches pending review request output', () => {
expect(
isSessionCommandControlMessage(
[
'Review request recorded, but the owner workspace is not ready yet.',
'- Task: task-1',
].join('\n'),
),
).toBe(true);
});
it('matches plan-gate control output', () => {
expect(
isSessionCommandControlMessage(
'Plan review is required before formal review for this high-risk task.\n- Task: task-1',
),
).toBe(true);
});
it('matches plan artifact control output', () => {
expect(
isSessionCommandControlMessage('Plan recorded.\n- Task: task-1'),
).toBe(true);
});
it('matches deployment finalized output', () => {
expect(
isSessionCommandControlMessage(
'Deployment finalized.\n- Task: task-1\n- Checkpoint: abc123',
),
).toBe(true);
});
it('does not match regular bot conversation', () => {
expect(
isSessionCommandControlMessage(

3
src/session-commands.ts
View File

@@ -10,9 +10,6 @@ const SESSION_COMMAND_CONTROL_PATTERNS = [
/^Failed to process messages before \/compact\. Try again\.$/,
/^\/compact failed\. The session is unchanged\.$/,
/^Conversation compacted\.$/,
/^Review snapshot updated\.(?:\n|$)/,
/^Review request recorded, but the owner workspace is not ready yet\.(?:\n|$)/,
/^Review is unavailable for this room\./,
];
function normalizeSessionCommandText(