fix: drain sdk follow-up turns (#219)
This commit is contained in:
@@ -852,4 +852,53 @@ describe('agent-runner Codex goals', () => {
|
|||||||
| undefined;
|
| undefined;
|
||||||
expect(spawnEnv?.CODEX_GOALS).toBe('true');
|
expect(spawnEnv?.CODEX_GOALS).toBe('true');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('marks SDK codex processes so queued IPC follow-ups drain after the active run', async () => {
|
||||||
|
const previousCodexGoals = process.env.CODEX_GOALS;
|
||||||
|
delete process.env.CODEX_GOALS;
|
||||||
|
const onProcess = vi.fn();
|
||||||
|
const codexGroup: RegisteredGroup = {
|
||||||
|
...testGroup,
|
||||||
|
agentType: 'codex',
|
||||||
|
};
|
||||||
|
|
||||||
|
try {
|
||||||
|
const resultPromise = runAgentProcess(
|
||||||
|
codexGroup,
|
||||||
|
{
|
||||||
|
...testInput,
|
||||||
|
roomRoleContext: {
|
||||||
|
serviceId: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
ownerServiceId: 'codex-main',
|
||||||
|
reviewerServiceId: 'claude',
|
||||||
|
failoverOwner: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
onProcess,
|
||||||
|
async () => {},
|
||||||
|
{
|
||||||
|
CODEX_RUNTIME: 'sdk',
|
||||||
|
CODEX_RUNTIME_SDK_ROLES: 'owner',
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
fakeProc.emit('close', 0);
|
||||||
|
const result = await resultPromise;
|
||||||
|
expect(result.status).toBe('success');
|
||||||
|
|
||||||
|
expect(onProcess).toHaveBeenCalledWith(
|
||||||
|
fakeProc,
|
||||||
|
expect.any(String),
|
||||||
|
expect.any(String),
|
||||||
|
{ drainFollowUpsAfterRun: true },
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
if (previousCodexGoals === undefined) {
|
||||||
|
delete process.env.CODEX_GOALS;
|
||||||
|
} else {
|
||||||
|
process.env.CODEX_GOALS = previousCodexGoals;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -61,6 +61,30 @@ export interface AgentOutput {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface ActiveProcessOptions {
|
||||||
|
drainFollowUpsAfterRun?: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isCodexSdkRuntimeMode(
|
||||||
|
env: Record<string, string>,
|
||||||
|
input: AgentInput,
|
||||||
|
): boolean {
|
||||||
|
const runtime = env.CODEX_RUNTIME?.trim().toLowerCase();
|
||||||
|
if (runtime !== 'sdk') return false;
|
||||||
|
if (input.prompt.trim() === '/compact') return false;
|
||||||
|
if (input.codexGoals === true || env.CODEX_GOALS === 'true') return false;
|
||||||
|
|
||||||
|
const rawRoles = env.CODEX_RUNTIME_SDK_ROLES?.trim();
|
||||||
|
if (!rawRoles) return true;
|
||||||
|
const role = input.roomRoleContext?.role?.trim().toLowerCase();
|
||||||
|
if (!role) return false;
|
||||||
|
return rawRoles
|
||||||
|
.split(',')
|
||||||
|
.map((item) => item.trim().toLowerCase())
|
||||||
|
.filter(Boolean)
|
||||||
|
.includes(role);
|
||||||
|
}
|
||||||
|
|
||||||
function readRoomSkillOverridesForRunner(
|
function readRoomSkillOverridesForRunner(
|
||||||
chatJid: string,
|
chatJid: string,
|
||||||
): StoredRoomSkillOverride[] {
|
): StoredRoomSkillOverride[] {
|
||||||
@@ -126,6 +150,7 @@ export async function runAgentProcess(
|
|||||||
proc: ChildProcess,
|
proc: ChildProcess,
|
||||||
processName: string,
|
processName: string,
|
||||||
runtimeIpcDir: string,
|
runtimeIpcDir: string,
|
||||||
|
options?: ActiveProcessOptions,
|
||||||
) => void,
|
) => void,
|
||||||
onOutput?: (output: AgentOutput) => Promise<void>,
|
onOutput?: (output: AgentOutput) => Promise<void>,
|
||||||
envOverrides?: Record<string, string>,
|
envOverrides?: Record<string, string>,
|
||||||
@@ -186,6 +211,11 @@ export async function runAgentProcess(
|
|||||||
const safeName = group.folder.replace(/[^a-zA-Z0-9-]/g, '-');
|
const safeName = group.folder.replace(/[^a-zA-Z0-9-]/g, '-');
|
||||||
const processSuffix = input.runId || `${Date.now()}`;
|
const processSuffix = input.runId || `${Date.now()}`;
|
||||||
const processName = `ejclaw-${safeName}-${processSuffix}`;
|
const processName = `ejclaw-${safeName}-${processSuffix}`;
|
||||||
|
const activeProcessOptions: ActiveProcessOptions = {
|
||||||
|
drainFollowUpsAfterRun:
|
||||||
|
(group.agentType || 'claude-code') === 'codex' &&
|
||||||
|
isCodexSdkRuntimeMode(env, input),
|
||||||
|
};
|
||||||
const finalizeCodexAuthSessionOnce = createCodexAuthSessionFinalizer(
|
const finalizeCodexAuthSessionOnce = createCodexAuthSessionFinalizer(
|
||||||
() => codexSessionAuth,
|
() => codexSessionAuth,
|
||||||
);
|
);
|
||||||
@@ -228,7 +258,7 @@ export async function runAgentProcess(
|
|||||||
env,
|
env,
|
||||||
});
|
});
|
||||||
|
|
||||||
onProcess(proc, processName, env[EJCLAW_ENV.ipcDir]);
|
onProcess(proc, processName, env[EJCLAW_ENV.ipcDir], activeProcessOptions);
|
||||||
|
|
||||||
const runnerInput: AgentInput = {
|
const runnerInput: AgentInput = {
|
||||||
...input,
|
...input,
|
||||||
|
|||||||
@@ -37,6 +37,7 @@ export interface GroupState {
|
|||||||
pendingTasks: QueuedTask[];
|
pendingTasks: QueuedTask[];
|
||||||
process: ChildProcess | null;
|
process: ChildProcess | null;
|
||||||
processName: string | null;
|
processName: string | null;
|
||||||
|
drainFollowUpsAfterRun: boolean;
|
||||||
ipcDir: string | null;
|
ipcDir: string | null;
|
||||||
retryCount: number;
|
retryCount: number;
|
||||||
retryTimer: ReturnType<typeof setTimeout> | null;
|
retryTimer: ReturnType<typeof setTimeout> | null;
|
||||||
@@ -60,6 +61,7 @@ export function createGroupState(): GroupState {
|
|||||||
pendingTasks: [],
|
pendingTasks: [],
|
||||||
process: null,
|
process: null,
|
||||||
processName: null,
|
processName: null,
|
||||||
|
drainFollowUpsAfterRun: false,
|
||||||
ipcDir: null,
|
ipcDir: null,
|
||||||
retryCount: 0,
|
retryCount: 0,
|
||||||
retryTimer: null,
|
retryTimer: null,
|
||||||
@@ -145,6 +147,7 @@ export function resetRunState(state: GroupState, groupJid: string): void {
|
|||||||
state.startedAt = null;
|
state.startedAt = null;
|
||||||
state.process = null;
|
state.process = null;
|
||||||
state.processName = null;
|
state.processName = null;
|
||||||
|
state.drainFollowUpsAfterRun = false;
|
||||||
state.ipcDir = null;
|
state.ipcDir = null;
|
||||||
state.directTerminalDeliveries.clear();
|
state.directTerminalDeliveries.clear();
|
||||||
transitionRunPhase(state, groupJid, 'idle');
|
transitionRunPhase(state, groupJid, 'idle');
|
||||||
@@ -160,7 +163,8 @@ export function assertRunPhaseInvariants(
|
|||||||
state.currentRunId != null ||
|
state.currentRunId != null ||
|
||||||
state.runningTaskId != null ||
|
state.runningTaskId != null ||
|
||||||
state.process != null ||
|
state.process != null ||
|
||||||
state.processName != null
|
state.processName != null ||
|
||||||
|
state.drainFollowUpsAfterRun
|
||||||
) {
|
) {
|
||||||
logger.error(
|
logger.error(
|
||||||
{
|
{
|
||||||
@@ -170,6 +174,7 @@ export function assertRunPhaseInvariants(
|
|||||||
runningTaskId: state.runningTaskId,
|
runningTaskId: state.runningTaskId,
|
||||||
hasProcess: state.process != null,
|
hasProcess: state.process != null,
|
||||||
processName: state.processName,
|
processName: state.processName,
|
||||||
|
drainFollowUpsAfterRun: state.drainFollowUpsAfterRun,
|
||||||
},
|
},
|
||||||
'Invariant violation: idle phase has stale run/task ID or process',
|
'Invariant violation: idle phase has stale run/task ID or process',
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -40,8 +40,6 @@ describe('GroupQueue', () => {
|
|||||||
vi.useRealTimers();
|
vi.useRealTimers();
|
||||||
});
|
});
|
||||||
|
|
||||||
// --- Single group at a time ---
|
|
||||||
|
|
||||||
it('only runs one agent per group at a time', async () => {
|
it('only runs one agent per group at a time', async () => {
|
||||||
let concurrentCount = 0;
|
let concurrentCount = 0;
|
||||||
let maxConcurrent = 0;
|
let maxConcurrent = 0;
|
||||||
@@ -90,6 +88,7 @@ describe('GroupQueue', () => {
|
|||||||
|
|
||||||
releaseRun(true);
|
releaseRun(true);
|
||||||
await vi.advanceTimersByTimeAsync(10);
|
await vi.advanceTimersByTimeAsync(10);
|
||||||
|
expect(processMessages).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('does not pipe follow-up messages after stdin close was requested', async () => {
|
it('does not pipe follow-up messages after stdin close was requested', async () => {
|
||||||
@@ -791,3 +790,46 @@ describe('GroupQueue close reason tracking', () => {
|
|||||||
expect(queue.getCloseReasonForRun('group1@g.us', activeRunId)).toBeNull();
|
expect(queue.getCloseReasonForRun('group1@g.us', activeRunId)).toBeNull();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('GroupQueue SDK follow-up draining', () => {
|
||||||
|
let queue: GroupQueue;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.clearAllMocks();
|
||||||
|
queue = new GroupQueue();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('drains follow-up messages after active processes that cannot consume queued IPC', async () => {
|
||||||
|
const ipcDir = '/tmp/ejclaw-test-data/ipc/group-folder';
|
||||||
|
let releaseRun!: (value: boolean) => void;
|
||||||
|
let activeRun = true;
|
||||||
|
const blocker = new Promise<boolean>((resolve) => {
|
||||||
|
releaseRun = resolve;
|
||||||
|
});
|
||||||
|
|
||||||
|
const processMessages = vi.fn(async () => {
|
||||||
|
if (!activeRun) return true;
|
||||||
|
activeRun = false;
|
||||||
|
return await blocker;
|
||||||
|
});
|
||||||
|
|
||||||
|
queue.setProcessMessagesFn(processMessages);
|
||||||
|
queue.enqueueMessageCheck('group1@g.us', ipcDir);
|
||||||
|
await vi.advanceTimersByTimeAsync(10);
|
||||||
|
|
||||||
|
queue.registerProcess('group1@g.us', {} as any, 'sdk-agent', ipcDir, {
|
||||||
|
drainFollowUpsAfterRun: true,
|
||||||
|
});
|
||||||
|
expect(queue.sendMessage('group1@g.us', '후속 메시지')).toBe(true);
|
||||||
|
|
||||||
|
releaseRun(true);
|
||||||
|
await vi.advanceTimersByTimeAsync(10);
|
||||||
|
|
||||||
|
expect(processMessages).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -65,7 +65,6 @@ export class GroupQueue {
|
|||||||
this.processMessagesFn = fn;
|
this.processMessagesFn = fn;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Limit concurrency after restart to avoid API rate-limit storms. */
|
|
||||||
enterRecoveryMode(): void {
|
enterRecoveryMode(): void {
|
||||||
this.recoveryMode = true;
|
this.recoveryMode = true;
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -101,7 +100,6 @@ export class GroupQueue {
|
|||||||
|
|
||||||
const state = this.getGroup(groupJid);
|
const state = this.getGroup(groupJid);
|
||||||
|
|
||||||
// Pre-set IPC dir so sendMessage can pipe follow-ups while agent process starts
|
|
||||||
if (ipcDir && !state.ipcDir) {
|
if (ipcDir && !state.ipcDir) {
|
||||||
state.ipcDir = ipcDir;
|
state.ipcDir = ipcDir;
|
||||||
}
|
}
|
||||||
@@ -157,7 +155,6 @@ export class GroupQueue {
|
|||||||
|
|
||||||
const state = this.getGroup(groupJid);
|
const state = this.getGroup(groupJid);
|
||||||
|
|
||||||
// Prevent double-queuing: check both pending and currently-running task
|
|
||||||
if (state.runningTaskId === taskId) {
|
if (state.runningTaskId === taskId) {
|
||||||
logger.debug({ groupJid, taskId }, 'Task already running, skipping');
|
logger.debug({ groupJid, taskId }, 'Task already running, skipping');
|
||||||
return;
|
return;
|
||||||
@@ -196,7 +193,6 @@ export class GroupQueue {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run immediately
|
|
||||||
this.runTask(groupJid, { id: taskId, groupJid, fn }).catch((err) =>
|
this.runTask(groupJid, { id: taskId, groupJid, fn }).catch((err) =>
|
||||||
logger.error({ groupJid, taskId, err }, 'Unhandled error in runTask'),
|
logger.error({ groupJid, taskId, err }, 'Unhandled error in runTask'),
|
||||||
);
|
);
|
||||||
@@ -207,10 +203,12 @@ export class GroupQueue {
|
|||||||
proc: ChildProcess,
|
proc: ChildProcess,
|
||||||
processName: string,
|
processName: string,
|
||||||
ipcDir?: string,
|
ipcDir?: string,
|
||||||
|
options?: { drainFollowUpsAfterRun?: boolean },
|
||||||
): void {
|
): void {
|
||||||
const state = this.getGroup(groupJid);
|
const state = this.getGroup(groupJid);
|
||||||
state.process = proc;
|
state.process = proc;
|
||||||
state.processName = processName;
|
state.processName = processName;
|
||||||
|
state.drainFollowUpsAfterRun = options?.drainFollowUpsAfterRun === true;
|
||||||
if (ipcDir) state.ipcDir = ipcDir;
|
if (ipcDir) state.ipcDir = ipcDir;
|
||||||
logger.info(
|
logger.info(
|
||||||
{
|
{
|
||||||
@@ -241,6 +239,7 @@ export class GroupQueue {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
const filename = queueFollowUpMessage(state.ipcDir, text);
|
const filename = queueFollowUpMessage(state.ipcDir, text);
|
||||||
|
state.pendingMessages ||= state.drainFollowUpsAfterRun;
|
||||||
logger.info(
|
logger.info(
|
||||||
{
|
{
|
||||||
groupJid,
|
groupJid,
|
||||||
@@ -248,6 +247,7 @@ export class GroupQueue {
|
|||||||
ipcDir: state.ipcDir,
|
ipcDir: state.ipcDir,
|
||||||
textLength: text.length,
|
textLength: text.length,
|
||||||
filename,
|
filename,
|
||||||
|
pendingMessages: state.pendingMessages,
|
||||||
},
|
},
|
||||||
'Queued follow-up message for active agent',
|
'Queued follow-up message for active agent',
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -100,6 +100,7 @@ interface RunMessageAgentAttemptArgs {
|
|||||||
: never,
|
: never,
|
||||||
processName: string,
|
processName: string,
|
||||||
ipcDir?: string,
|
ipcDir?: string,
|
||||||
|
options?: { drainFollowUpsAfterRun?: boolean },
|
||||||
) => void;
|
) => void;
|
||||||
onOutput?: (output: AgentOutput) => Promise<void>;
|
onOutput?: (output: AgentOutput) => Promise<void>;
|
||||||
pairedExecutionLifecycle: {
|
pairedExecutionLifecycle: {
|
||||||
|
|||||||
@@ -274,8 +274,8 @@ export async function runAgentForGroup(
|
|||||||
deps.persistSession(sessionFolder, sessionId);
|
deps.persistSession(sessionFolder, sessionId);
|
||||||
currentSessionId = sessionId;
|
currentSessionId = sessionId;
|
||||||
},
|
},
|
||||||
registerProcess: (proc, processName, ipcDir) =>
|
registerProcess: (proc, processName, ipcDir, options) =>
|
||||||
deps.queue.registerProcess(chatJid, proc, processName, ipcDir),
|
deps.queue.registerProcess(chatJid, proc, processName, ipcDir, options),
|
||||||
onOutput,
|
onOutput,
|
||||||
pairedExecutionLifecycle,
|
pairedExecutionLifecycle,
|
||||||
log,
|
log,
|
||||||
|
|||||||
Reference in New Issue
Block a user