feat: paired review system overhaul — unified service, configurable agents, silent output removal
- Remove UNIFIED_MODE legacy: single service manages all 3 Discord bots - Add OWNER_AGENT_TYPE / REVIEWER_AGENT_TYPE env vars for configurable agent selection - Fix Discord channel routing: reviewer output goes through correct bot (discord/discord-review) - Fix channel name assignment: explicit names prevent agentTypeFilter collision - Remove silent output suppression system: harness-level protections replace prompt workarounds - Add reviewer approval detection: DONE marker on first line stops ping-pong - Include user's original message in reviewer prompt for context - Fix round_trip_count auto-reset on new user message - Fix session ID conflict: reviewer doesn't reuse owner's session - Fix pending progress text flush in runner on close sentinel - Promote buffered intermediate text to final result when result event has no text - Remove legacy files: .env.codex.example, .env.codex-review.example, migrate-unify.cjs - Update CLAUDE.md: server-side build deployment, unified architecture docs
This commit is contained in:
@@ -1,4 +1,3 @@
|
||||
import { parseStructuredOutputEnvelope } from './output-suppression.js';
|
||||
import type { StructuredAgentOutput } from './types.js';
|
||||
|
||||
export function stringifyLegacyAgentResult(
|
||||
@@ -21,10 +20,7 @@ export function getStructuredAgentOutput(output: {
|
||||
if (output.output) {
|
||||
return output.output;
|
||||
}
|
||||
if (typeof output.result !== 'string') {
|
||||
return null;
|
||||
}
|
||||
return parseStructuredOutputEnvelope(output.result.trim());
|
||||
return null;
|
||||
}
|
||||
|
||||
export function getAgentOutputText(output: {
|
||||
@@ -32,9 +28,6 @@ export function getAgentOutputText(output: {
|
||||
result?: string | object | null;
|
||||
}): string | null {
|
||||
const structured = getStructuredAgentOutput(output);
|
||||
if (structured?.visibility === 'silent') {
|
||||
return null;
|
||||
}
|
||||
if (structured?.visibility === 'public') {
|
||||
return structured.text;
|
||||
}
|
||||
@@ -51,9 +44,9 @@ export function hasAgentOutputPayload(output: {
|
||||
return output.result !== null && output.result !== undefined;
|
||||
}
|
||||
|
||||
export function isSilentAgentOutput(output: {
|
||||
export function isSilentAgentOutput(_output: {
|
||||
output?: StructuredAgentOutput;
|
||||
result?: string | object | null;
|
||||
}): boolean {
|
||||
return getStructuredAgentOutput(output)?.visibility === 'silent';
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
IDLE_TIMEOUT,
|
||||
} from './config.js';
|
||||
import { prepareGroupEnvironment } from './agent-runner-environment.js';
|
||||
import { getEnv } from './env.js';
|
||||
export {
|
||||
type AvailableGroup,
|
||||
writeGroupsSnapshot,
|
||||
@@ -74,7 +75,7 @@ export async function runAgentProcess(
|
||||
// as a host process. This provides kernel-level write protection.
|
||||
const isReviewerContainer =
|
||||
envOverrides?.EJCLAW_REVIEWER_RUNTIME === '1' &&
|
||||
process.env.REVIEWER_CONTAINER_ENABLED !== '0';
|
||||
getEnv('REVIEWER_CONTAINER_ENABLED') !== '0';
|
||||
if (isReviewerContainer) {
|
||||
const ownerWorkspaceDir =
|
||||
envOverrides?.EJCLAW_WORK_DIR || group.workDir || process.cwd();
|
||||
|
||||
@@ -184,11 +184,14 @@ export class DiscordChannel implements Channel {
|
||||
botToken: string,
|
||||
opts: DiscordChannelOpts,
|
||||
agentTypeFilter?: AgentType,
|
||||
channelName?: string,
|
||||
) {
|
||||
this.botToken = botToken;
|
||||
this.opts = opts;
|
||||
this.agentTypeFilter = agentTypeFilter;
|
||||
if (agentTypeFilter) {
|
||||
if (channelName) {
|
||||
this.name = channelName;
|
||||
} else if (agentTypeFilter) {
|
||||
this.name = `discord-${agentTypeFilter}`;
|
||||
}
|
||||
}
|
||||
@@ -723,21 +726,20 @@ registerChannel('discord', (opts: ChannelOpts) => {
|
||||
token,
|
||||
opts,
|
||||
hasCodexBot ? 'claude-code' : undefined,
|
||||
'discord',
|
||||
);
|
||||
});
|
||||
|
||||
// Register the secondary Codex bot channel.
|
||||
// In unified mode all bots register unconditionally; in legacy per-service mode
|
||||
// the codex service uses its own DISCORD_BOT_TOKEN via systemd EnvironmentFile override.
|
||||
registerChannel('discord-codex', (opts: ChannelOpts) => {
|
||||
const token = getEnv('DISCORD_CODEX_BOT_TOKEN') || '';
|
||||
if (!token) return null; // Codex Discord bot is optional
|
||||
return new DiscordChannel(token, opts, 'codex');
|
||||
if (!token) return null;
|
||||
return new DiscordChannel(token, opts, 'codex', 'discord-codex');
|
||||
});
|
||||
|
||||
// Register the review bot channel (codex agent type, separate token).
|
||||
registerChannel('discord-review', (opts: ChannelOpts) => {
|
||||
const token = getEnv('DISCORD_REVIEW_BOT_TOKEN') || '';
|
||||
if (!token) return null; // Review Discord bot is optional
|
||||
return new DiscordChannel(token, opts, 'codex');
|
||||
if (!token) return null;
|
||||
return new DiscordChannel(token, opts, 'codex', 'discord-review');
|
||||
});
|
||||
|
||||
@@ -22,9 +22,6 @@ export const SERVICE_AGENT_TYPE: AgentType =
|
||||
? 'codex'
|
||||
: 'claude-code';
|
||||
|
||||
/** Unified mode: single process manages all bots. Disable with UNIFIED_MODE=0. */
|
||||
export const UNIFIED_MODE = getEnv('UNIFIED_MODE') !== '0';
|
||||
|
||||
export function normalizeServiceId(serviceId: string): string {
|
||||
if (serviceId === 'codex') {
|
||||
return CODEX_MAIN_SERVICE_ID;
|
||||
@@ -92,6 +89,25 @@ export const RECOVERY_CONCURRENT_AGENTS = parseInt(
|
||||
);
|
||||
|
||||
// ── Paired review ─────────────────────────────────────────────────
|
||||
|
||||
/** Owner agent type. Default: codex. Set OWNER_AGENT_TYPE=claude-code to use Claude as owner. */
|
||||
const rawOwnerAgentType = getEnv('OWNER_AGENT_TYPE');
|
||||
export const OWNER_AGENT_TYPE: AgentType =
|
||||
rawOwnerAgentType === 'codex' || rawOwnerAgentType === 'claude-code'
|
||||
? rawOwnerAgentType
|
||||
: 'codex';
|
||||
|
||||
/** Reviewer agent type. Default: claude-code. Set REVIEWER_AGENT_TYPE=codex to use Codex as reviewer. */
|
||||
const rawReviewerAgentType = getEnv('REVIEWER_AGENT_TYPE');
|
||||
export const REVIEWER_AGENT_TYPE: AgentType =
|
||||
rawReviewerAgentType === 'codex' || rawReviewerAgentType === 'claude-code'
|
||||
? rawReviewerAgentType
|
||||
: 'claude-code';
|
||||
|
||||
/** Service ID for the reviewer based on agent type. */
|
||||
export const REVIEWER_SERVICE_ID_FOR_TYPE =
|
||||
REVIEWER_AGENT_TYPE === 'claude-code' ? CLAUDE_SERVICE_ID : CODEX_REVIEW_SERVICE_ID;
|
||||
|
||||
// Max owner↔reviewer round trips per task. 0 = unlimited.
|
||||
const rawMaxRoundTrips = getEnv('PAIRED_MAX_ROUND_TRIPS') || '10';
|
||||
export const PAIRED_MAX_ROUND_TRIPS =
|
||||
|
||||
12
src/db.ts
12
src/db.ts
@@ -1053,6 +1053,18 @@ export function getLastHumanMessageTimestamp(chatJid: string): string | null {
|
||||
return row?.timestamp ?? null;
|
||||
}
|
||||
|
||||
export function getLastHumanMessageContent(chatJid: string): string | null {
|
||||
const row = db
|
||||
.prepare(
|
||||
`SELECT content FROM messages
|
||||
WHERE chat_jid = ? AND is_bot_message = 0 AND is_from_me = 0
|
||||
AND content != '' AND content IS NOT NULL
|
||||
ORDER BY timestamp DESC, seq DESC LIMIT 1`,
|
||||
)
|
||||
.get(chatJid) as { content: string } | undefined;
|
||||
return row?.content ?? null;
|
||||
}
|
||||
|
||||
export function hasRecentRestartAnnouncement(
|
||||
chatJid: string,
|
||||
sinceTimestamp: string,
|
||||
|
||||
78
src/index.ts
78
src/index.ts
@@ -9,9 +9,6 @@ import {
|
||||
POLL_INTERVAL,
|
||||
SERVICE_ID,
|
||||
SERVICE_AGENT_TYPE,
|
||||
UNIFIED_MODE,
|
||||
isClaudeService,
|
||||
isReviewService,
|
||||
isSessionCommandSenderAllowed,
|
||||
STATUS_CHANNEL_ID,
|
||||
STATUS_UPDATE_INTERVAL,
|
||||
@@ -79,7 +76,6 @@ import {
|
||||
initTokenRotation,
|
||||
} from './token-rotation.js';
|
||||
import {
|
||||
shouldStartTokenRefreshLoop,
|
||||
startTokenRefreshLoop,
|
||||
stopTokenRefreshLoop,
|
||||
} from './token-refresh.js';
|
||||
@@ -189,16 +185,11 @@ function loadState(): void {
|
||||
lastAgentTimestamp = {};
|
||||
}
|
||||
sessions = getAllSessions();
|
||||
// In unified mode, load ALL groups (duplicates resolved by preferring codex).
|
||||
// In legacy per-service mode, load only this service's registrations.
|
||||
registeredGroups = UNIFIED_MODE
|
||||
? getAllRegisteredGroups()
|
||||
: getAllRegisteredGroups(SERVICE_AGENT_TYPE);
|
||||
registeredGroups = getAllRegisteredGroups();
|
||||
logger.info(
|
||||
{
|
||||
groupCount: Object.keys(registeredGroups).length,
|
||||
agentType: SERVICE_AGENT_TYPE,
|
||||
unifiedMode: UNIFIED_MODE,
|
||||
},
|
||||
'State loaded',
|
||||
);
|
||||
@@ -331,27 +322,16 @@ async function main(): Promise<void> {
|
||||
initTokenRotation();
|
||||
initCodexTokenRotation();
|
||||
|
||||
// Unified mode: start credential proxy for container isolation and clean up orphaned containers
|
||||
if (UNIFIED_MODE) {
|
||||
startCredentialProxy(CREDENTIAL_PROXY_PORT).catch((err) =>
|
||||
logger.warn(
|
||||
{ err },
|
||||
'Failed to start credential proxy (may already be running)',
|
||||
),
|
||||
);
|
||||
cleanupOrphans();
|
||||
}
|
||||
// Start credential proxy for container isolation and clean up orphaned containers
|
||||
startCredentialProxy(CREDENTIAL_PROXY_PORT).catch((err) =>
|
||||
logger.warn(
|
||||
{ err },
|
||||
'Failed to start credential proxy (may already be running)',
|
||||
),
|
||||
);
|
||||
cleanupOrphans();
|
||||
|
||||
// In unified mode, always start the Claude OAuth refresh loop.
|
||||
// In per-service mode, only the Claude service owns refresh to avoid races.
|
||||
if (UNIFIED_MODE || shouldStartTokenRefreshLoop(SERVICE_AGENT_TYPE)) {
|
||||
startTokenRefreshLoop();
|
||||
} else {
|
||||
logger.info(
|
||||
{ serviceAgentType: SERVICE_AGENT_TYPE },
|
||||
'Skipping Claude OAuth token auto-refresh for non-Claude service',
|
||||
);
|
||||
}
|
||||
startTokenRefreshLoop();
|
||||
|
||||
loadState();
|
||||
|
||||
@@ -456,27 +436,19 @@ async function main(): Promise<void> {
|
||||
}
|
||||
|
||||
// Start subsystems (independently of connection handler)
|
||||
// In unified mode always run the scheduler. In per-service mode, skip for review-only service.
|
||||
if (UNIFIED_MODE || !isReviewService()) {
|
||||
startSchedulerLoop({
|
||||
registeredGroups: () => registeredGroups,
|
||||
getSessions: () => sessions,
|
||||
queue,
|
||||
onProcess: (groupJid, proc, processName, ipcDir) =>
|
||||
queue.registerProcess(groupJid, proc, processName, ipcDir),
|
||||
sendMessage: (jid, rawText) =>
|
||||
sendFormattedChannelMessage(channels, jid, rawText),
|
||||
sendTrackedMessage: (jid, rawText) =>
|
||||
sendFormattedTrackedChannelMessage(channels, jid, rawText),
|
||||
editTrackedMessage: (jid, messageId, rawText) =>
|
||||
editFormattedTrackedChannelMessage(channels, jid, messageId, rawText),
|
||||
});
|
||||
} else {
|
||||
logger.info(
|
||||
{ serviceId: SERVICE_ID },
|
||||
'Skipping scheduler for review service',
|
||||
);
|
||||
}
|
||||
startSchedulerLoop({
|
||||
registeredGroups: () => registeredGroups,
|
||||
getSessions: () => sessions,
|
||||
queue,
|
||||
onProcess: (groupJid, proc, processName, ipcDir) =>
|
||||
queue.registerProcess(groupJid, proc, processName, ipcDir),
|
||||
sendMessage: (jid, rawText) =>
|
||||
sendFormattedChannelMessage(channels, jid, rawText),
|
||||
sendTrackedMessage: (jid, rawText) =>
|
||||
sendFormattedTrackedChannelMessage(channels, jid, rawText),
|
||||
editTrackedMessage: (jid, messageId, rawText) =>
|
||||
editFormattedTrackedChannelMessage(channels, jid, messageId, rawText),
|
||||
});
|
||||
startIpcWatcher({
|
||||
sendMessage: (jid, text) => {
|
||||
const channel = findChannel(channels, jid);
|
||||
@@ -538,8 +510,7 @@ async function main(): Promise<void> {
|
||||
purgeOnStart: true,
|
||||
});
|
||||
|
||||
if (UNIFIED_MODE || isClaudeService()) {
|
||||
leaseRecoveryTimer = setInterval(() => {
|
||||
leaseRecoveryTimer = setInterval(() => {
|
||||
if (!hasAvailableClaudeToken()) {
|
||||
return;
|
||||
}
|
||||
@@ -578,7 +549,6 @@ async function main(): Promise<void> {
|
||||
);
|
||||
}
|
||||
}, 5_000);
|
||||
}
|
||||
runtime.startMessageLoop().catch((err) => {
|
||||
logger.fatal({ err }, 'Message loop crashed unexpectedly');
|
||||
process.exit(1);
|
||||
|
||||
@@ -142,6 +142,7 @@ function makeDeps() {
|
||||
assistantName: 'Andy',
|
||||
queue: {
|
||||
registerProcess: vi.fn(),
|
||||
enqueueMessageCheck: vi.fn(),
|
||||
},
|
||||
getRegisteredGroups: () => ({}),
|
||||
getSessions: () => ({}),
|
||||
|
||||
@@ -37,6 +37,8 @@ import {
|
||||
CODEX_MAIN_SERVICE_ID,
|
||||
CODEX_REVIEW_SERVICE_ID,
|
||||
SERVICE_SESSION_SCOPE,
|
||||
REVIEWER_AGENT_TYPE,
|
||||
isClaudeService,
|
||||
} from './config.js';
|
||||
import {
|
||||
buildStructuredOutputPrompt,
|
||||
@@ -95,10 +97,41 @@ export async function runAgentForGroup(
|
||||
onOutput,
|
||||
} = args;
|
||||
const isMain = group.isMain === true;
|
||||
const isClaudeCodeAgent =
|
||||
(group.agentType || 'claude-code') === 'claude-code';
|
||||
const sessions = deps.getSessions();
|
||||
const sessionId = sessions[group.folder];
|
||||
|
||||
const currentLease = getEffectiveChannelLease(chatJid);
|
||||
|
||||
// In unified mode, determine role from the lease directly.
|
||||
// Default to owner; the auto-review trigger in completePairedExecutionContext
|
||||
// will switch to reviewer when the task is in review_ready state.
|
||||
const pairedTask = currentLease.reviewer_service_id
|
||||
? getLatestOpenPairedTaskForChat(chatJid)
|
||||
: null;
|
||||
const effectiveServiceId =
|
||||
pairedTask &&
|
||||
(pairedTask.status === 'review_ready' || pairedTask.status === 'in_review')
|
||||
? currentLease.reviewer_service_id!
|
||||
: currentLease.owner_service_id;
|
||||
const reviewerMode = effectiveServiceId === currentLease.reviewer_service_id;
|
||||
|
||||
// Override agent type based on role config (OWNER_AGENT_TYPE / REVIEWER_AGENT_TYPE).
|
||||
// When the reviewer uses a different agent type than the group default,
|
||||
// swap in the configured type for the duration of this execution.
|
||||
const reviewerAgentTypeOverride =
|
||||
reviewerMode && REVIEWER_AGENT_TYPE !== (group.agentType || 'claude-code');
|
||||
const effectiveAgentType = reviewerAgentTypeOverride
|
||||
? REVIEWER_AGENT_TYPE
|
||||
: group.agentType || 'claude-code';
|
||||
const effectiveGroup = reviewerAgentTypeOverride
|
||||
? { ...group, agentType: REVIEWER_AGENT_TYPE }
|
||||
: group;
|
||||
const isClaudeCodeAgent = effectiveAgentType === 'claude-code';
|
||||
|
||||
// When the reviewer uses a different agent type than the group default,
|
||||
// the stored session belongs to the other agent. Don't reuse it.
|
||||
const sessionId = reviewerAgentTypeOverride
|
||||
? undefined
|
||||
: sessions[group.folder];
|
||||
const memoryBriefing = sessionId
|
||||
? undefined
|
||||
: await buildRoomMemoryBriefing({
|
||||
@@ -130,20 +163,6 @@ export async function runAgentForGroup(
|
||||
let resetSessionRequested = false;
|
||||
|
||||
const canRotateToken = isClaudeCodeAgent && getTokenCount() > 1;
|
||||
const currentLease = getEffectiveChannelLease(chatJid);
|
||||
|
||||
// In unified mode, determine role from the lease directly.
|
||||
// Default to owner; the auto-review trigger in completePairedExecutionContext
|
||||
// will switch to reviewer when the task is in review_ready state.
|
||||
const pairedTask = currentLease.reviewer_service_id
|
||||
? getLatestOpenPairedTaskForChat(chatJid)
|
||||
: null;
|
||||
const effectiveServiceId =
|
||||
pairedTask &&
|
||||
(pairedTask.status === 'review_ready' || pairedTask.status === 'in_review')
|
||||
? currentLease.reviewer_service_id!
|
||||
: currentLease.owner_service_id;
|
||||
const reviewerMode = effectiveServiceId === currentLease.reviewer_service_id;
|
||||
const roomRoleContext = buildRoomRoleContext(
|
||||
currentLease,
|
||||
effectiveServiceId,
|
||||
@@ -156,11 +175,13 @@ export async function runAgentForGroup(
|
||||
});
|
||||
const effectivePrompt = buildStructuredOutputPrompt(prompt, {
|
||||
reviewerMode,
|
||||
pairedRoom: !!pairedExecutionContext,
|
||||
gateTurnKind: pairedExecutionContext?.gateTurnKind,
|
||||
requiresVisibleVerdict: pairedExecutionContext?.requiresVisibleVerdict,
|
||||
});
|
||||
let pairedExecutionStatus: 'succeeded' | 'failed' = 'failed';
|
||||
let pairedExecutionSummary: string | null = null;
|
||||
let pairedExecutionCompleted = false;
|
||||
|
||||
const shouldHandoffToCodex = (
|
||||
reason: AgentTriggerReason,
|
||||
@@ -248,6 +269,7 @@ export async function runAgentForGroup(
|
||||
status: pairedExecutionStatus,
|
||||
summary: pairedExecutionSummary,
|
||||
});
|
||||
pairedExecutionCompleted = true;
|
||||
return 'success';
|
||||
}
|
||||
|
||||
@@ -394,21 +416,22 @@ export async function runAgentForGroup(
|
||||
}
|
||||
: undefined;
|
||||
|
||||
const agentType = group.agentType || 'claude-code';
|
||||
logger.info(
|
||||
{
|
||||
chatJid,
|
||||
group: group.name,
|
||||
groupFolder: group.folder,
|
||||
runId,
|
||||
provider: agentType,
|
||||
provider: effectiveAgentType,
|
||||
reviewerMode,
|
||||
reviewerAgentTypeOverride,
|
||||
},
|
||||
`Using provider: ${agentType}`,
|
||||
`Using provider: ${effectiveAgentType}`,
|
||||
);
|
||||
|
||||
try {
|
||||
const output = await runAgentProcess(
|
||||
group,
|
||||
effectiveGroup,
|
||||
{
|
||||
...agentInput,
|
||||
sessionId: provider === 'claude' ? sessionId : undefined,
|
||||
@@ -887,7 +910,7 @@ export async function runAgentForGroup(
|
||||
pairedExecutionStatus = 'succeeded';
|
||||
return 'success';
|
||||
} finally {
|
||||
if (pairedExecutionContext) {
|
||||
if (pairedExecutionContext && !pairedExecutionCompleted) {
|
||||
const completedRole = roomRoleContext?.role ?? 'owner';
|
||||
completePairedExecutionContext({
|
||||
taskId: pairedExecutionContext.task.id,
|
||||
@@ -895,12 +918,12 @@ export async function runAgentForGroup(
|
||||
status: pairedExecutionStatus,
|
||||
summary: pairedExecutionSummary,
|
||||
});
|
||||
}
|
||||
|
||||
// After owner/reviewer completes, enqueue the next turn so
|
||||
// the message loop picks it up without waiting for a new message.
|
||||
if (pairedExecutionStatus === 'succeeded') {
|
||||
deps.queue.enqueueMessageCheck(chatJid);
|
||||
}
|
||||
// After owner/reviewer completes, enqueue the next turn so
|
||||
// the message loop picks it up without waiting for a new message.
|
||||
if (pairedExecutionContext && pairedExecutionStatus === 'succeeded') {
|
||||
deps.queue.enqueueMessageCheck(chatJid);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,6 +89,12 @@ export function hasAllowedTrigger(opts: {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Paired rooms: bot-to-bot ping-pong doesn't need trigger patterns.
|
||||
// Peer bot messages (from reviewer/owner) are always allowed.
|
||||
if (isPairedRoomJid(chatJid)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const allowlistCfg = loadSenderAllowlist();
|
||||
const hasTrigger = messages.some(
|
||||
(message) =>
|
||||
@@ -104,14 +110,10 @@ export function getProcessableMessages(
|
||||
messages: Parameters<typeof filterProcessableMessages>[0],
|
||||
channel?: Channel,
|
||||
) {
|
||||
const isPaired = isPairedRoomJid(chatJid);
|
||||
return filterProcessableMessages(
|
||||
messages,
|
||||
isPaired,
|
||||
// In paired rooms (unified mode), don't filter by isOwnMessage —
|
||||
// both bots need to see each other's messages for the ping-pong flow.
|
||||
// Loop prevention is handled by filterLoopingPairedBotMessages + round_trip_count.
|
||||
isPaired ? undefined : channel?.isOwnMessage?.bind(channel),
|
||||
isPairedRoomJid(chatJid),
|
||||
channel?.isOwnMessage?.bind(channel),
|
||||
).filter((message) => !isTaskStatusControlMessage(message.content));
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
markWorkItemDelivered,
|
||||
markWorkItemDeliveryRetry,
|
||||
getLastBotFinalMessage,
|
||||
getLastHumanMessageContent,
|
||||
isPairedRoomJid,
|
||||
getLatestOpenPairedTaskForChat,
|
||||
type ServiceHandoff,
|
||||
@@ -21,12 +22,14 @@ import {
|
||||
isClaudeService,
|
||||
isReviewService,
|
||||
isSessionCommandSenderAllowed,
|
||||
REVIEWER_AGENT_TYPE,
|
||||
SERVICE_AGENT_TYPE,
|
||||
SERVICE_ID,
|
||||
} from './config.js';
|
||||
import { GroupQueue, GroupRunContext } from './group-queue.js';
|
||||
import {
|
||||
findChannel,
|
||||
findChannelByName,
|
||||
formatMessages,
|
||||
normalizeMessageForDedupe,
|
||||
} from './router.js';
|
||||
@@ -56,10 +59,7 @@ import {
|
||||
import { Channel, NewMessage, RegisteredGroup } from './types.js';
|
||||
import { logger } from './logger.js';
|
||||
import { resolveGroupIpcPath } from './group-folder.js';
|
||||
import {
|
||||
getEffectiveChannelLease,
|
||||
shouldServiceProcessChat,
|
||||
} from './service-routing.js';
|
||||
import { getEffectiveChannelLease } from './service-routing.js';
|
||||
|
||||
/**
|
||||
* Check if a message is a duplicate of the last bot final message in a paired room.
|
||||
@@ -451,37 +451,50 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
return true;
|
||||
}
|
||||
|
||||
// For paired rooms, determine the reviewer channel for correct bot routing.
|
||||
// This is used whenever the reviewer needs to send output.
|
||||
const reviewerChannelName =
|
||||
isPairedRoomJid(chatJid) && REVIEWER_AGENT_TYPE === 'claude-code'
|
||||
? 'discord'
|
||||
: 'discord-review';
|
||||
const foundReviewerChannel = findChannelByName(
|
||||
deps.channels,
|
||||
reviewerChannelName,
|
||||
);
|
||||
const reviewerChannel = foundReviewerChannel || channel;
|
||||
if (isPairedRoomJid(chatJid)) {
|
||||
logger.info(
|
||||
{
|
||||
chatJid,
|
||||
reviewerChannelName,
|
||||
foundChannel: foundReviewerChannel?.name ?? null,
|
||||
usingChannel: reviewerChannel.name,
|
||||
availableChannels: deps.channels.map((c) => c.name),
|
||||
},
|
||||
'Paired room reviewer channel resolution',
|
||||
);
|
||||
}
|
||||
|
||||
// Deliver pending work items through the correct channel.
|
||||
// For paired rooms, check if the work item was from a reviewer turn.
|
||||
const openWorkItem = getOpenWorkItem(
|
||||
chatJid,
|
||||
(group.agentType || 'claude-code') as 'claude-code' | 'codex',
|
||||
);
|
||||
if (openWorkItem) {
|
||||
const delivered = await deliverOpenWorkItem(channel, openWorkItem);
|
||||
// Use reviewer channel if the pending task is in review state
|
||||
const pendingTask = isPairedRoomJid(chatJid)
|
||||
? getLatestOpenPairedTaskForChat(chatJid)
|
||||
: null;
|
||||
const isReviewerWorkItem =
|
||||
pendingTask &&
|
||||
(pendingTask.status === 'review_ready' ||
|
||||
pendingTask.status === 'in_review');
|
||||
const deliveryChannel = isReviewerWorkItem ? reviewerChannel : channel;
|
||||
const delivered = await deliverOpenWorkItem(deliveryChannel, openWorkItem);
|
||||
if (!delivered) return false;
|
||||
}
|
||||
|
||||
if (!shouldServiceProcessChat(chatJid, SERVICE_ID)) {
|
||||
const rawMissedMessages = getMessagesSinceSeq(
|
||||
chatJid,
|
||||
deps.getLastAgentTimestamps()[chatJid] || '0',
|
||||
deps.assistantName,
|
||||
);
|
||||
const lastIgnored = rawMissedMessages[rawMissedMessages.length - 1];
|
||||
if (lastIgnored?.seq != null) {
|
||||
advanceLastAgentCursor(
|
||||
deps.getLastAgentTimestamps(),
|
||||
deps.saveState,
|
||||
chatJid,
|
||||
lastIgnored.seq,
|
||||
);
|
||||
}
|
||||
logger.debug(
|
||||
{ chatJid, serviceId: SERVICE_ID },
|
||||
'Skipping message processing for unassigned service',
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
const isMainGroup = group.isMain === true;
|
||||
while (true) {
|
||||
const sinceSeqCursor = deps.getLastAgentTimestamps()[chatJid] || '0';
|
||||
@@ -506,15 +519,40 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
(pendingReviewTask.status === 'review_ready' ||
|
||||
pendingReviewTask.status === 'in_review')
|
||||
) {
|
||||
// Synthesize a prompt for the reviewer turn
|
||||
const reviewPrompt =
|
||||
'Review the latest owner changes. Provide feedback or approve.';
|
||||
// Build review prompt with user's original request + owner's response
|
||||
const userMessage = getLastHumanMessageContent(chatJid);
|
||||
const ownerContent = rawMissedMessages
|
||||
.filter((m) => m.is_bot_message)
|
||||
.map((m) => m.content)
|
||||
.join('\n\n');
|
||||
const parts: string[] = [];
|
||||
if (userMessage) {
|
||||
parts.push(`User request:\n---\n${userMessage}\n---`);
|
||||
}
|
||||
if (ownerContent) {
|
||||
parts.push(`Owner response:\n---\n${ownerContent}\n---`);
|
||||
}
|
||||
const reviewPrompt = parts.length > 0
|
||||
? `${parts.join('\n\n')}\n\nReview the owner's response above. Provide feedback or approve.`
|
||||
: 'Review the latest owner changes in the workspace. Provide feedback or approve.';
|
||||
|
||||
// Advance cursor past the owner's messages so they aren't re-processed
|
||||
const lastRaw = rawMissedMessages[rawMissedMessages.length - 1];
|
||||
if (lastRaw?.seq != null) {
|
||||
advanceLastAgentCursor(
|
||||
deps.getLastAgentTimestamps(),
|
||||
deps.saveState,
|
||||
chatJid,
|
||||
lastRaw.seq,
|
||||
);
|
||||
}
|
||||
|
||||
const { deliverySucceeded } = await executeTurn({
|
||||
group,
|
||||
prompt: reviewPrompt,
|
||||
chatJid,
|
||||
runId,
|
||||
channel,
|
||||
channel: reviewerChannel,
|
||||
startSeq: null,
|
||||
endSeq: null,
|
||||
});
|
||||
@@ -751,20 +789,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!shouldServiceProcessChat(chatJid, SERVICE_ID)) {
|
||||
const lastIgnored =
|
||||
processableGroupMessages[processableGroupMessages.length - 1];
|
||||
if (lastIgnored?.seq != null) {
|
||||
advanceLastAgentCursor(
|
||||
deps.getLastAgentTimestamps(),
|
||||
deps.saveState,
|
||||
chatJid,
|
||||
lastIgnored.seq,
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (
|
||||
shouldSkipBotOnlyCollaboration(chatJid, processableGroupMessages)
|
||||
) {
|
||||
@@ -906,10 +930,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!shouldServiceProcessChat(chatJid, SERVICE_ID)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const sinceSeqCursor = deps.getLastAgentTimestamps()[chatJid] || '';
|
||||
const rawPending = getMessagesSinceSeq(
|
||||
chatJid,
|
||||
|
||||
@@ -122,14 +122,9 @@ export class MessageTurnController {
|
||||
}
|
||||
|
||||
const raw = getAgentOutputText(result);
|
||||
const silentOutput = isSilentAgentOutput(result);
|
||||
const suppressState =
|
||||
raw && this.options.suppressToken
|
||||
? classifySuppressTokenOutput(raw, this.options.suppressToken)
|
||||
: raw
|
||||
? classifySuppressTokenOutput(raw, undefined)
|
||||
: 'none';
|
||||
const text = raw && suppressState === 'none' ? formatOutbound(raw) : null;
|
||||
const silentOutput = false;
|
||||
const suppressState = 'none' as const;
|
||||
const text = raw ? formatOutbound(raw) : null;
|
||||
|
||||
if (raw) {
|
||||
logger.info(
|
||||
@@ -144,31 +139,6 @@ export class MessageTurnController {
|
||||
},
|
||||
`Agent output: ${raw.slice(0, 200)}`,
|
||||
);
|
||||
if (suppressState === 'exact') {
|
||||
logger.info(
|
||||
{
|
||||
chatJid: this.options.chatJid,
|
||||
group: this.options.group.name,
|
||||
groupFolder: this.options.group.folder,
|
||||
runId: this.options.runId,
|
||||
resultStatus: result.status,
|
||||
resultPhase: result.phase,
|
||||
},
|
||||
'Suppressed exact-match silent output token',
|
||||
);
|
||||
} else if (suppressState === 'mixed') {
|
||||
logger.warn(
|
||||
{
|
||||
chatJid: this.options.chatJid,
|
||||
group: this.options.group.name,
|
||||
groupFolder: this.options.group.folder,
|
||||
runId: this.options.runId,
|
||||
resultStatus: result.status,
|
||||
resultPhase: result.phase,
|
||||
},
|
||||
'Blocked malformed output that mixed the silent output token with visible text',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const phase: AgentOutputPhase = normalizeAgentOutputPhase(result.phase);
|
||||
|
||||
@@ -1,125 +1,35 @@
|
||||
import { randomBytes } from 'crypto';
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
|
||||
import {
|
||||
CLAUDE_SERVICE_ID,
|
||||
CODEX_REVIEW_SERVICE_ID,
|
||||
normalizeServiceId,
|
||||
} from './config.js';
|
||||
import type { StructuredAgentOutput } from './types.js';
|
||||
|
||||
const ANY_SUPPRESS_TOKEN_PATTERN = /__EJ_SUPPRESS_[a-f0-9]{24,}(?:__)?/g;
|
||||
const EXACT_ANY_SUPPRESS_TOKEN_PATTERN = /^__EJ_SUPPRESS_[a-f0-9]{24,}(?:__)?$/;
|
||||
const STRUCTURED_SILENT_OUTPUT_PREFIX_PATTERN =
|
||||
/^\s*\{\s*"ejclaw"\s*:\s*\{\s*"visibility"\s*:\s*"silent"/;
|
||||
export const STRUCTURED_SILENT_OUTPUT_ENVELOPE =
|
||||
'{"ejclaw":{"visibility":"silent"}}';
|
||||
const STRUCTURED_PUBLIC_VERDICTS = new Set([
|
||||
'done',
|
||||
'done_with_concerns',
|
||||
'blocked',
|
||||
]);
|
||||
const STRUCTURED_SILENT_VERDICTS = new Set(['silent']);
|
||||
|
||||
export function createSuppressToken(): string {
|
||||
return `__EJ_SUPPRESS_${randomBytes(12).toString('hex')}__`;
|
||||
}
|
||||
|
||||
export function shouldEnableSuppressOutputForService(
|
||||
serviceId: string | undefined,
|
||||
): boolean {
|
||||
if (!serviceId) return false;
|
||||
const normalized = normalizeServiceId(serviceId);
|
||||
return (
|
||||
normalized === CLAUDE_SERVICE_ID || normalized === CODEX_REVIEW_SERVICE_ID
|
||||
);
|
||||
export function createSuppressToken(): string | undefined {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function classifySuppressTokenOutput(
|
||||
rawText: string,
|
||||
suppressToken: string | undefined,
|
||||
): 'exact' | 'mixed' | 'none' {
|
||||
const trimmed = rawText.trim();
|
||||
const structured = parseStructuredOutputEnvelope(trimmed);
|
||||
if (
|
||||
(suppressToken && trimmed === suppressToken) ||
|
||||
EXACT_ANY_SUPPRESS_TOKEN_PATTERN.test(trimmed) ||
|
||||
structured?.visibility === 'silent'
|
||||
) {
|
||||
return 'exact';
|
||||
}
|
||||
if (suppressToken && rawText.includes(suppressToken)) {
|
||||
return 'mixed';
|
||||
}
|
||||
ANY_SUPPRESS_TOKEN_PATTERN.lastIndex = 0;
|
||||
if (ANY_SUPPRESS_TOKEN_PATTERN.test(rawText)) {
|
||||
return 'mixed';
|
||||
}
|
||||
return STRUCTURED_SILENT_OUTPUT_PREFIX_PATTERN.test(trimmed)
|
||||
? 'mixed'
|
||||
: 'none';
|
||||
}
|
||||
|
||||
export function parseStructuredOutputEnvelope(
|
||||
rawText: string,
|
||||
): StructuredAgentOutput | null {
|
||||
try {
|
||||
const parsed = JSON.parse(rawText) as {
|
||||
ejclaw?: { visibility?: unknown; text?: unknown };
|
||||
};
|
||||
const envelope = parsed?.ejclaw;
|
||||
if (!envelope || typeof envelope !== 'object' || Array.isArray(envelope)) {
|
||||
return null;
|
||||
}
|
||||
if (envelope.visibility === 'silent') {
|
||||
return { visibility: 'silent' };
|
||||
}
|
||||
if (
|
||||
envelope.visibility === 'public' &&
|
||||
typeof envelope.text === 'string' &&
|
||||
envelope.text.length > 0
|
||||
) {
|
||||
return { visibility: 'public', text: envelope.text };
|
||||
}
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
||||
return null;
|
||||
_rawText: string,
|
||||
_suppressToken: string | undefined,
|
||||
): 'none' {
|
||||
return 'none';
|
||||
}
|
||||
|
||||
export function buildStructuredOutputPrompt(
|
||||
prompt: string,
|
||||
options?: {
|
||||
_options?: {
|
||||
reviewerMode?: boolean;
|
||||
pairedRoom?: boolean;
|
||||
gateTurnKind?: string | null;
|
||||
requiresVisibleVerdict?: boolean;
|
||||
},
|
||||
): string {
|
||||
const lines = [
|
||||
'[OUTPUT CONTROL]',
|
||||
`If you have no user-visible content to send for this turn, output exactly this JSON and nothing else: ${STRUCTURED_SILENT_OUTPUT_ENVELOPE}`,
|
||||
'Do not wrap the JSON in backticks or code fences.',
|
||||
'Do not combine the JSON with any other text.',
|
||||
'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.',
|
||||
];
|
||||
|
||||
if (options?.reviewerMode) {
|
||||
lines.push(
|
||||
'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.',
|
||||
);
|
||||
}
|
||||
|
||||
if (options?.reviewerMode && options.requiresVisibleVerdict) {
|
||||
lines.push(
|
||||
`This turn is a paired-room gate turn for ${options.gateTurnKind ?? 'implementation_start'}. Silent output is forbidden.`,
|
||||
);
|
||||
lines.push(
|
||||
'Your final answer must be a structured public JSON envelope with a reviewer verdict: {"ejclaw":{"visibility":"public","verdict":"done","text":"**DONE** ..."}}',
|
||||
);
|
||||
lines.push(
|
||||
'Allowed verdict values are: "done", "done_with_concerns", "blocked".',
|
||||
);
|
||||
}
|
||||
|
||||
return `${lines.join('\n')}\n\n${prompt}`;
|
||||
return prompt;
|
||||
}
|
||||
|
||||
export function parseStructuredOutputEnvelope(
|
||||
_rawText: string,
|
||||
): null {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -28,6 +28,20 @@ import type {
|
||||
RoomRoleContext,
|
||||
} from './types.js';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Reviewer approval detection
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Only check the first line for explicit status markers from the prompt's
|
||||
// completion status protocol (DONE, DONE_WITH_CONCERNS, BLOCKED, NEEDS_CONTEXT).
|
||||
// DONE = approved → stop ping-pong. Everything else = continue.
|
||||
function isReviewerApproval(summary: string | null | undefined): boolean {
|
||||
if (!summary) return false;
|
||||
const firstLine = summary.trimStart().split('\n')[0].trim();
|
||||
// Match DONE but not DONE_WITH_CONCERNS
|
||||
return /\bDONE\b/.test(firstLine) && !/\bDONE_WITH_CONCERNS\b/.test(firstLine);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -168,6 +182,14 @@ export function preparePairedExecutionContext(args: {
|
||||
const now = new Date().toISOString();
|
||||
|
||||
if (roomRoleContext.role === 'owner') {
|
||||
// New human message → new ping-pong cycle. Reset the round trip counter
|
||||
// so previous interactions don't block the auto-review trigger.
|
||||
if (latestTask.round_trip_count > 0) {
|
||||
updatePairedTask(latestTask.id, {
|
||||
round_trip_count: 0,
|
||||
updated_at: now,
|
||||
});
|
||||
}
|
||||
workspace = provisionOwnerWorkspaceForPairedTask(latestTask.id);
|
||||
} else {
|
||||
const reviewerWorkspace = prepareReviewerWorkspaceForExecution(latestTask);
|
||||
@@ -268,17 +290,29 @@ export function completePairedExecutionContext(args: {
|
||||
}
|
||||
}
|
||||
|
||||
// Reviewer finished → set task back to active so owner can respond
|
||||
// Reviewer finished → check if approved or needs more work
|
||||
if (role === 'reviewer') {
|
||||
const now = new Date().toISOString();
|
||||
updatePairedTask(taskId, {
|
||||
status: 'active',
|
||||
updated_at: now,
|
||||
});
|
||||
logger.info(
|
||||
{ taskId },
|
||||
'Reviewer completed, task set back to active for owner',
|
||||
);
|
||||
const approved = isReviewerApproval(args.summary);
|
||||
if (approved) {
|
||||
updatePairedTask(taskId, {
|
||||
status: 'completed',
|
||||
updated_at: now,
|
||||
});
|
||||
logger.info(
|
||||
{ taskId, summary: args.summary?.slice(0, 100) },
|
||||
'Reviewer approved, task completed — ping-pong stopped',
|
||||
);
|
||||
} else {
|
||||
updatePairedTask(taskId, {
|
||||
status: 'active',
|
||||
updated_at: now,
|
||||
});
|
||||
logger.info(
|
||||
{ taskId },
|
||||
'Reviewer requested changes, task set back to active for owner',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,9 +2,10 @@ import {
|
||||
CLAUDE_SERVICE_ID,
|
||||
CODEX_MAIN_SERVICE_ID,
|
||||
CODEX_REVIEW_SERVICE_ID,
|
||||
OWNER_AGENT_TYPE,
|
||||
REVIEWER_SERVICE_ID_FOR_TYPE,
|
||||
SERVICE_AGENT_TYPE,
|
||||
SERVICE_ID,
|
||||
UNIFIED_MODE,
|
||||
normalizeServiceId,
|
||||
} from './config.js';
|
||||
import {
|
||||
@@ -51,10 +52,13 @@ function getDefaultLease(chatJid: string): EffectiveChannelLease {
|
||||
const hasCodex = types.includes('codex');
|
||||
|
||||
if (hasClaude && hasCodex) {
|
||||
// Owner/reviewer service IDs derived from OWNER_AGENT_TYPE / REVIEWER_AGENT_TYPE env vars
|
||||
const ownerServiceId =
|
||||
OWNER_AGENT_TYPE === 'codex' ? CODEX_MAIN_SERVICE_ID : CLAUDE_SERVICE_ID;
|
||||
return {
|
||||
chat_jid: chatJid,
|
||||
owner_service_id: CLAUDE_SERVICE_ID,
|
||||
reviewer_service_id: CODEX_MAIN_SERVICE_ID,
|
||||
owner_service_id: ownerServiceId,
|
||||
reviewer_service_id: REVIEWER_SERVICE_ID_FOR_TYPE,
|
||||
activated_at: null,
|
||||
reason: null,
|
||||
explicit: false,
|
||||
@@ -140,16 +144,10 @@ export function isReviewerServiceForChat(
|
||||
}
|
||||
|
||||
export function shouldServiceProcessChat(
|
||||
chatJid: string,
|
||||
serviceId: string = SERVICE_ID,
|
||||
_chatJid: string,
|
||||
_serviceId: string = SERVICE_ID,
|
||||
): boolean {
|
||||
if (UNIFIED_MODE) return true;
|
||||
const normalizedServiceId = normalizeServiceId(serviceId);
|
||||
const lease = getEffectiveChannelLease(chatJid);
|
||||
return (
|
||||
normalizedServiceId === lease.owner_service_id ||
|
||||
normalizedServiceId === lease.reviewer_service_id
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
export function activateCodexFailover(chatJid: string, reason: string): void {
|
||||
|
||||
Reference in New Issue
Block a user