Fix recurring Codex unavailable in paired rooms (#212)
* fix: avoid codex leases for claude readonly sessions * fix: restore codex lease quality budget * fix: preserve owner finalization on codex failure * fix: wake owner after arbiter codex failure * chore: format readonly codex lease fix
This commit is contained in:
@@ -15,7 +15,13 @@ const {
|
||||
mockReadEnvFile: vi.fn<() => Record<string, string>>(),
|
||||
mockGetActiveCodexAuthPath: vi.fn<() => string | null>(),
|
||||
mockGetCodexAccountCount: vi.fn<() => number>(),
|
||||
mockClaimCodexAuthLease: vi.fn<() => null>(),
|
||||
mockClaimCodexAuthLease: vi.fn<
|
||||
() => {
|
||||
authPath: string;
|
||||
accountIndex: number;
|
||||
release: () => void;
|
||||
} | null
|
||||
>(),
|
||||
mockFindCodexAccountIndexByAuthPath: vi.fn<() => number | null>(),
|
||||
}));
|
||||
|
||||
@@ -835,6 +841,37 @@ describe('prepareReadonlySessionEnvironment codex compatibility', () => {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('does not claim a Codex auth lease for Claude read-only sessions', () => {
|
||||
vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
|
||||
mockGetCodexAccountCount.mockReturnValue(1);
|
||||
const rotatedAuthPath = path.join(tempRoot, 'rotated-auth.json');
|
||||
fs.writeFileSync(rotatedAuthPath, '{"auth_mode":"chatgpt"}\n');
|
||||
const release = vi.fn();
|
||||
mockClaimCodexAuthLease.mockReturnValue({
|
||||
authPath: rotatedAuthPath,
|
||||
accountIndex: 0,
|
||||
release,
|
||||
});
|
||||
|
||||
const sessionDir = path.join(tempRoot, 'readonly-claude-session');
|
||||
const prepared = prepareReadonlySessionEnvironment({
|
||||
sessionDir,
|
||||
chatJid: 'dc:test',
|
||||
isMain: false,
|
||||
groupFolder: 'codex-test-group',
|
||||
agentType: 'claude-code',
|
||||
memoryBriefing: 'memory briefing',
|
||||
role: 'reviewer',
|
||||
});
|
||||
|
||||
expect(prepared.codexSessionAuth).toBeNull();
|
||||
expect(mockClaimCodexAuthLease).not.toHaveBeenCalled();
|
||||
expect(release).not.toHaveBeenCalled();
|
||||
expect(fs.existsSync(path.join(sessionDir, '.codex', 'auth.json'))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it('writes matching AGENTS.md and copies host codex auth/config into the role-scoped session', () => {
|
||||
vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
|
||||
|
||||
|
||||
@@ -832,32 +832,39 @@ export function prepareReadonlySessionEnvironment(args: {
|
||||
const sessionClaudeMdPath = path.join(sessionDir, 'CLAUDE.md');
|
||||
if (sessionClaudeMd) {
|
||||
fs.writeFileSync(sessionClaudeMdPath, sessionClaudeMd + '\n');
|
||||
const sessionCodexDir = path.join(sessionDir, '.codex');
|
||||
codexSessionAuth = syncHostCodexSessionFiles(sessionCodexDir);
|
||||
fs.writeFileSync(
|
||||
path.join(sessionCodexDir, 'AGENTS.md'),
|
||||
sessionClaudeMd + '\n',
|
||||
);
|
||||
const sessionConfigPath = path.join(sessionCodexDir, 'config.toml');
|
||||
const mcpServerPath = path.join(
|
||||
projectRoot,
|
||||
'runners',
|
||||
'agent-runner',
|
||||
'dist',
|
||||
'ipc-mcp-stdio.js',
|
||||
);
|
||||
if (fs.existsSync(mcpServerPath)) {
|
||||
upsertEjclawMcpServerSection({
|
||||
sessionConfigPath,
|
||||
mcpServerPath,
|
||||
ipcDir,
|
||||
hostIpcDir,
|
||||
chatJid,
|
||||
groupFolder,
|
||||
isMain,
|
||||
agentType,
|
||||
roomRole: role,
|
||||
workDir,
|
||||
if (agentType === 'codex') {
|
||||
const sessionCodexDir = path.join(sessionDir, '.codex');
|
||||
codexSessionAuth = syncHostCodexSessionFiles(sessionCodexDir);
|
||||
fs.writeFileSync(
|
||||
path.join(sessionCodexDir, 'AGENTS.md'),
|
||||
sessionClaudeMd + '\n',
|
||||
);
|
||||
const sessionConfigPath = path.join(sessionCodexDir, 'config.toml');
|
||||
const mcpServerPath = path.join(
|
||||
projectRoot,
|
||||
'runners',
|
||||
'agent-runner',
|
||||
'dist',
|
||||
'ipc-mcp-stdio.js',
|
||||
);
|
||||
if (fs.existsSync(mcpServerPath)) {
|
||||
upsertEjclawMcpServerSection({
|
||||
sessionConfigPath,
|
||||
mcpServerPath,
|
||||
ipcDir,
|
||||
hostIpcDir,
|
||||
chatJid,
|
||||
groupFolder,
|
||||
isMain,
|
||||
agentType,
|
||||
roomRole: role,
|
||||
workDir,
|
||||
});
|
||||
}
|
||||
} else {
|
||||
fs.rmSync(path.join(sessionDir, '.codex'), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
}
|
||||
logger.info(
|
||||
|
||||
@@ -60,6 +60,44 @@ function logStreamedOutputDeliveryError(
|
||||
);
|
||||
}
|
||||
|
||||
function logStreamedAgentErrorOutput(
|
||||
output: AgentOutput,
|
||||
group: RegisteredGroup,
|
||||
input: AgentInput,
|
||||
): void {
|
||||
if (output.status !== 'error') return;
|
||||
logger.warn(
|
||||
{
|
||||
group: group.name,
|
||||
chatJid: input.chatJid,
|
||||
runId: input.runId,
|
||||
error: output.error,
|
||||
newSessionId: output.newSessionId,
|
||||
},
|
||||
'Streamed agent error output',
|
||||
);
|
||||
}
|
||||
|
||||
function chainStreamedOutputDelivery(args: {
|
||||
outputChain: Promise<void>;
|
||||
parsed: AgentOutput;
|
||||
onOutput: (output: AgentOutput) => Promise<void>;
|
||||
onTerminalStreamedOutputFlushed?: (output: AgentOutput) => void;
|
||||
group: RegisteredGroup;
|
||||
input: AgentInput;
|
||||
}): Promise<void> {
|
||||
return args.outputChain.then(async () => {
|
||||
try {
|
||||
await args.onOutput(args.parsed);
|
||||
if (isTerminalStreamedOutput(args.parsed)) {
|
||||
args.onTerminalStreamedOutputFlushed?.(args.parsed);
|
||||
}
|
||||
} catch (err) {
|
||||
logStreamedOutputDeliveryError(err, args.group, args.input);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
function writeTimeoutLog(args: {
|
||||
logsDir: string;
|
||||
input: AgentInput;
|
||||
@@ -188,27 +226,15 @@ export function runSpawnedAgentProcess(
|
||||
}
|
||||
hadStreamingOutput = true;
|
||||
resetTimeout();
|
||||
if (parsed.status === 'error') {
|
||||
logger.warn(
|
||||
{
|
||||
group: group.name,
|
||||
chatJid: input.chatJid,
|
||||
runId: input.runId,
|
||||
error: parsed.error,
|
||||
newSessionId: parsed.newSessionId,
|
||||
},
|
||||
'Streamed agent error output',
|
||||
);
|
||||
}
|
||||
outputChain = outputChain.then(async () => {
|
||||
try {
|
||||
await onOutput(parsed);
|
||||
if (isTerminalStreamedOutput(parsed)) {
|
||||
args.onTerminalStreamedOutputFlushed?.(parsed);
|
||||
}
|
||||
} catch (err) {
|
||||
logStreamedOutputDeliveryError(err, group, input);
|
||||
}
|
||||
logStreamedAgentErrorOutput(parsed, group, input);
|
||||
outputChain = chainStreamedOutputDelivery({
|
||||
outputChain,
|
||||
parsed,
|
||||
onOutput,
|
||||
onTerminalStreamedOutputFlushed:
|
||||
args.onTerminalStreamedOutputFlushed,
|
||||
group,
|
||||
input,
|
||||
});
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
|
||||
@@ -137,6 +137,72 @@ function emitOutputMarker(
|
||||
proc.stdout.push(`${OUTPUT_START_MARKER}\n${json}\n${OUTPUT_END_MARKER}\n`);
|
||||
}
|
||||
|
||||
function mockCodexLeaseEnvironment(releaseLease: () => void) {
|
||||
return vi
|
||||
.spyOn(agentRunnerEnvironment, 'prepareGroupEnvironment')
|
||||
.mockReturnValueOnce({
|
||||
env: {
|
||||
EJCLAW_IPC_DIR: '/tmp/ejclaw-test-data/ipc/test-group',
|
||||
},
|
||||
groupDir: '/tmp/ejclaw-test-groups/test-group',
|
||||
runnerDir: '/tmp/ejclaw-test-runners/codex-runner',
|
||||
codexSessionAuth: {
|
||||
canonicalAuthPath: '/tmp/codex-account/auth.json',
|
||||
sessionAuthPath: '/tmp/codex-session/auth.json',
|
||||
accountIndex: 0,
|
||||
lease: {
|
||||
accountIndex: 0,
|
||||
authPath: '/tmp/codex-account/auth.json',
|
||||
release: releaseLease,
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async function expectCodexLeaseReleasedAfterFinalOutputFlush() {
|
||||
const releaseLease = vi.fn();
|
||||
const prepareGroupEnvironmentSpy = mockCodexLeaseEnvironment(releaseLease);
|
||||
const onOutput = vi.fn(async () => {});
|
||||
const codexGroup: RegisteredGroup = {
|
||||
...testGroup,
|
||||
agentType: 'codex',
|
||||
};
|
||||
let resultPromise: Promise<AgentOutput> | undefined;
|
||||
|
||||
try {
|
||||
resultPromise = runAgentProcess(
|
||||
codexGroup,
|
||||
{
|
||||
...testInput,
|
||||
runId: 'run-codex-lease-final',
|
||||
},
|
||||
() => {},
|
||||
onOutput,
|
||||
);
|
||||
|
||||
emitOutputMarker(fakeProc, {
|
||||
status: 'success',
|
||||
result: 'TASK_DONE\n작업 완료',
|
||||
phase: 'final',
|
||||
newSessionId: 'codex-session-lease-final',
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
|
||||
expect(onOutput).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
result: 'TASK_DONE\n작업 완료',
|
||||
phase: 'final',
|
||||
}),
|
||||
);
|
||||
expect(releaseLease).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
fakeProc.emit('close', 0);
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
await resultPromise?.catch(() => undefined);
|
||||
prepareGroupEnvironmentSpy.mockRestore();
|
||||
}
|
||||
}
|
||||
|
||||
describe('agent-runner timeout behavior', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
@@ -157,23 +223,18 @@ describe('agent-runner timeout behavior', () => {
|
||||
onOutput,
|
||||
);
|
||||
|
||||
// Emit output with a result
|
||||
emitOutputMarker(fakeProc, {
|
||||
status: 'success',
|
||||
result: 'Here is my response',
|
||||
newSessionId: 'session-123',
|
||||
});
|
||||
|
||||
// Let output processing settle
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
|
||||
// Fire the hard timeout (IDLE_TIMEOUT + 30s = 1830000ms)
|
||||
await vi.advanceTimersByTimeAsync(1830000);
|
||||
|
||||
// Emit close event (as if agent was stopped by the timeout)
|
||||
fakeProc.emit('close', 137);
|
||||
|
||||
// Let the promise resolve
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
|
||||
const result = await resultPromise;
|
||||
@@ -193,10 +254,8 @@ describe('agent-runner timeout behavior', () => {
|
||||
onOutput,
|
||||
);
|
||||
|
||||
// No output emitted — fire the hard timeout
|
||||
await vi.advanceTimersByTimeAsync(1830000);
|
||||
|
||||
// Emit close event
|
||||
fakeProc.emit('close', 137);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
@@ -216,7 +275,6 @@ describe('agent-runner timeout behavior', () => {
|
||||
onOutput,
|
||||
);
|
||||
|
||||
// Emit output
|
||||
emitOutputMarker(fakeProc, {
|
||||
status: 'success',
|
||||
result: 'Done',
|
||||
@@ -225,7 +283,6 @@ describe('agent-runner timeout behavior', () => {
|
||||
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
|
||||
// Normal exit (no timeout)
|
||||
fakeProc.emit('close', 0);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
@@ -236,65 +293,7 @@ describe('agent-runner timeout behavior', () => {
|
||||
});
|
||||
|
||||
it('releases Codex auth lease after final streamed output flushes even before process close', async () => {
|
||||
const releaseLease = vi.fn();
|
||||
const prepareGroupEnvironmentSpy = vi
|
||||
.spyOn(agentRunnerEnvironment, 'prepareGroupEnvironment')
|
||||
.mockReturnValueOnce({
|
||||
env: {
|
||||
EJCLAW_IPC_DIR: '/tmp/ejclaw-test-data/ipc/test-group',
|
||||
},
|
||||
groupDir: '/tmp/ejclaw-test-groups/test-group',
|
||||
runnerDir: '/tmp/ejclaw-test-runners/codex-runner',
|
||||
codexSessionAuth: {
|
||||
canonicalAuthPath: '/tmp/codex-account/auth.json',
|
||||
sessionAuthPath: '/tmp/codex-session/auth.json',
|
||||
accountIndex: 0,
|
||||
lease: {
|
||||
accountIndex: 0,
|
||||
authPath: '/tmp/codex-account/auth.json',
|
||||
release: releaseLease,
|
||||
},
|
||||
},
|
||||
});
|
||||
const onOutput = vi.fn(async () => {});
|
||||
const codexGroup: RegisteredGroup = {
|
||||
...testGroup,
|
||||
agentType: 'codex',
|
||||
};
|
||||
let resultPromise: Promise<AgentOutput> | undefined;
|
||||
|
||||
try {
|
||||
resultPromise = runAgentProcess(
|
||||
codexGroup,
|
||||
{
|
||||
...testInput,
|
||||
runId: 'run-codex-lease-final',
|
||||
},
|
||||
() => {},
|
||||
onOutput,
|
||||
);
|
||||
|
||||
emitOutputMarker(fakeProc, {
|
||||
status: 'success',
|
||||
result: 'TASK_DONE\n작업 완료',
|
||||
phase: 'final',
|
||||
newSessionId: 'codex-session-lease-final',
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
|
||||
expect(onOutput).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
result: 'TASK_DONE\n작업 완료',
|
||||
phase: 'final',
|
||||
}),
|
||||
);
|
||||
expect(releaseLease).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
fakeProc.emit('close', 0);
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
await resultPromise?.catch(() => undefined);
|
||||
prepareGroupEnvironmentSpy.mockRestore();
|
||||
}
|
||||
await expectCodexLeaseReleasedAfterFinalOutputFlush();
|
||||
});
|
||||
|
||||
it('preserves streamed progress phase metadata', async () => {
|
||||
|
||||
@@ -623,6 +623,7 @@ class PairedExecutionLifecycleController implements PairedExecutionLifecycle {
|
||||
sawOutput: state.sawOutputForFollowUp,
|
||||
taskStatus: finishedTask?.status ?? null,
|
||||
outputSummary: this.pairedExecutionSummary,
|
||||
ownerFailureCount: finishedTask?.owner_failure_count ?? null,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -172,17 +172,17 @@ describe('message-agent-executor-rules', () => {
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('does not requeue a silent arbiter failure caused by Codex account auth expiry', () => {
|
||||
it('requeues owner follow-up after a silent arbiter Codex account failure returns the task active', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
completedRole: 'arbiter',
|
||||
executionStatus: 'failed',
|
||||
sawOutput: false,
|
||||
taskStatus: 'arbiter_requested',
|
||||
taskStatus: 'active',
|
||||
outputSummary:
|
||||
'Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.\nExecution completed without a visible terminal verdict.',
|
||||
}),
|
||||
).toBe('none');
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('resolves pending arbiter follow-up requeue after repeated owner execution failures', () => {
|
||||
@@ -207,7 +207,7 @@ describe('message-agent-executor-rules', () => {
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('does not requeue a silent owner failure caused by Codex pool unavailability', () => {
|
||||
it('requeues a silent owner failure caused by Codex pool unavailability', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
completedRole: 'owner',
|
||||
@@ -217,7 +217,7 @@ describe('message-agent-executor-rules', () => {
|
||||
outputSummary:
|
||||
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.',
|
||||
}),
|
||||
).toBe('none');
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('does not request an executor-side follow-up after successful owner output moved the task to review_ready', () => {
|
||||
|
||||
@@ -28,13 +28,19 @@ export function resolvePairedFollowUpQueueAction(args: {
|
||||
sawOutput: boolean;
|
||||
taskStatus: PairedTaskStatus | null;
|
||||
outputSummary?: string | null;
|
||||
ownerFailureCount?: number | null;
|
||||
}): PairedFollowUpQueueAction {
|
||||
if (
|
||||
args.executionStatus === 'failed' &&
|
||||
args.sawOutput === false &&
|
||||
isSilentCodexAccountFailure(args.outputSummary)
|
||||
) {
|
||||
return 'none';
|
||||
if (args.completedRole === 'arbiter' && args.taskStatus === 'active') {
|
||||
return 'pending';
|
||||
}
|
||||
if (args.completedRole !== 'owner') {
|
||||
return 'none';
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
|
||||
@@ -21,6 +21,11 @@ export type PairedFollowUpSource = Parameters<
|
||||
typeof resolveFollowUpDispatch
|
||||
>[0]['source'];
|
||||
|
||||
type PairedFollowUpTaskContext = Pick<
|
||||
PairedTask,
|
||||
'id' | 'status' | 'round_trip_count' | 'updated_at' | 'owner_failure_count'
|
||||
>;
|
||||
|
||||
export interface PairedFollowUpDecision {
|
||||
taskId: string | null;
|
||||
taskStatus: PairedTaskStatus | null;
|
||||
@@ -67,7 +72,10 @@ export function resolveLatestPairedTurnOutputContext(args: {
|
||||
}
|
||||
|
||||
export function resolvePairedFollowUpDecision(args: {
|
||||
task: Pick<PairedTask, 'id' | 'status'> | null | undefined;
|
||||
task:
|
||||
| Pick<PairedFollowUpTaskContext, 'id' | 'status' | 'owner_failure_count'>
|
||||
| null
|
||||
| undefined;
|
||||
source: PairedFollowUpSource;
|
||||
completedRole?: PairedRoomRole;
|
||||
executionStatus?: 'succeeded' | 'failed';
|
||||
@@ -93,6 +101,7 @@ export function resolvePairedFollowUpDecision(args: {
|
||||
taskStatus: args.task?.status ?? null,
|
||||
lastTurnOutputRole,
|
||||
lastTurnOutputVerdict,
|
||||
ownerFailureCount: args.task?.owner_failure_count ?? null,
|
||||
});
|
||||
const dispatch = resolveFollowUpDispatch({
|
||||
source: args.source,
|
||||
@@ -115,10 +124,7 @@ export function resolvePairedFollowUpDecision(args: {
|
||||
export function schedulePairedFollowUpIntent(args: {
|
||||
chatJid: string;
|
||||
runId: string;
|
||||
task:
|
||||
| Pick<PairedTask, 'id' | 'status' | 'round_trip_count' | 'updated_at'>
|
||||
| null
|
||||
| undefined;
|
||||
task: PairedFollowUpTaskContext | null | undefined;
|
||||
intentKind: ScheduledPairedFollowUpIntentKind;
|
||||
enqueue: () => void;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
@@ -148,6 +154,7 @@ export function schedulePairedFollowUpIntent(args: {
|
||||
taskStatus: args.task.status,
|
||||
lastTurnOutputRole: latestOutputContext.role,
|
||||
lastTurnOutputVerdict: latestOutputContext.verdict,
|
||||
ownerFailureCount: args.task.owner_failure_count ?? null,
|
||||
intentKind: args.intentKind,
|
||||
}) &&
|
||||
!(
|
||||
@@ -171,10 +178,7 @@ export function schedulePairedFollowUpIntent(args: {
|
||||
export function schedulePairedFollowUpWithMessageCheck(args: {
|
||||
chatJid: string;
|
||||
runId: string;
|
||||
task:
|
||||
| Pick<PairedTask, 'id' | 'status' | 'round_trip_count' | 'updated_at'>
|
||||
| null
|
||||
| undefined;
|
||||
task: PairedFollowUpTaskContext | null | undefined;
|
||||
intentKind: ScheduledPairedFollowUpIntentKind;
|
||||
enqueueMessageCheck: () => void;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
@@ -198,10 +202,7 @@ export function schedulePairedFollowUpWithMessageCheck(args: {
|
||||
export function dispatchPairedFollowUpForEvent(args: {
|
||||
chatJid: string;
|
||||
runId: string;
|
||||
task:
|
||||
| Pick<PairedTask, 'id' | 'status' | 'round_trip_count' | 'updated_at'>
|
||||
| null
|
||||
| undefined;
|
||||
task: PairedFollowUpTaskContext | null | undefined;
|
||||
source: PairedFollowUpSource;
|
||||
completedRole?: PairedRoomRole;
|
||||
executionStatus?: 'succeeded' | 'failed';
|
||||
@@ -270,10 +271,7 @@ export function dispatchPairedFollowUpForEvent(args: {
|
||||
export function enqueuePairedFollowUpAfterEvent(args: {
|
||||
chatJid: string;
|
||||
runId: string;
|
||||
task:
|
||||
| Pick<PairedTask, 'id' | 'status' | 'round_trip_count' | 'updated_at'>
|
||||
| null
|
||||
| undefined;
|
||||
task: PairedFollowUpTaskContext | null | undefined;
|
||||
source: PairedFollowUpSource;
|
||||
completedRole?: PairedRoomRole;
|
||||
executionStatus?: 'succeeded' | 'failed';
|
||||
|
||||
179
src/owner-final-codex-unavailable.test.ts
Normal file
179
src/owner-final-codex-unavailable.test.ts
Normal file
@@ -0,0 +1,179 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./db.js', () => {
|
||||
const updatePairedTask = vi.fn();
|
||||
return {
|
||||
getPairedTaskById: vi.fn(),
|
||||
getPairedWorkspace: vi.fn(),
|
||||
updatePairedTask,
|
||||
updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => {
|
||||
updatePairedTask(id, updates);
|
||||
return true;
|
||||
}),
|
||||
releasePairedTaskExecutionLease: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('./paired-workspace-manager.js', () => ({
|
||||
isOwnerWorkspaceRepairNeededError: vi.fn(() => false),
|
||||
markPairedTaskReviewReady: vi.fn(),
|
||||
prepareReviewerWorkspaceForExecution: vi.fn(),
|
||||
provisionOwnerWorkspaceForPairedTask: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('./logger.js', () => ({
|
||||
logger: {
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import * as config from './config.js';
|
||||
import * as db from './db.js';
|
||||
import { completePairedExecutionContext } from './paired-execution-context.js';
|
||||
import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js';
|
||||
import type { PairedTask } from './types.js';
|
||||
|
||||
const CODEX_UNAVAILABLE_SUMMARY =
|
||||
'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.';
|
||||
|
||||
function buildPairedTask(overrides: Partial<PairedTask> = {}): PairedTask {
|
||||
return {
|
||||
id: 'task-1',
|
||||
chat_jid: 'dc:test',
|
||||
group_folder: 'ejclaw',
|
||||
owner_service_id: config.CODEX_MAIN_SERVICE_ID,
|
||||
reviewer_service_id: config.REVIEWER_SERVICE_ID_FOR_TYPE,
|
||||
title: null,
|
||||
source_ref: 'HEAD',
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: 0,
|
||||
task_done_then_user_reopen_count: 0,
|
||||
empty_step_done_streak: 0,
|
||||
status: 'merge_ready',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-28T00:00:00.000Z',
|
||||
updated_at: '2026-03-28T00:00:00.000Z',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('owner final Codex unavailable handling', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(db.getPairedWorkspace).mockReturnValue(undefined);
|
||||
vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true);
|
||||
});
|
||||
|
||||
it('preserves active tasks when the first owner turn cannot start Codex', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
updated_at: '2026-03-28T00:00:05.000Z',
|
||||
}),
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'failed',
|
||||
summary: CODEX_UNAVAILABLE_SUMMARY,
|
||||
});
|
||||
|
||||
const updates = vi.mocked(db.updatePairedTask).mock.calls[0]?.[1];
|
||||
expect(updates).toEqual(
|
||||
expect.objectContaining({
|
||||
owner_failure_count: 1,
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
}),
|
||||
);
|
||||
expect(updates?.status).not.toBe('completed');
|
||||
});
|
||||
|
||||
it('preserves merge_ready when owner finalization cannot start Codex', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
updated_at: '2026-03-28T00:00:05.000Z',
|
||||
}),
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'failed',
|
||||
summary: CODEX_UNAVAILABLE_SUMMARY,
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
owner_failure_count: 1,
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('requests arbiter after repeated owner Codex account failures', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
owner_failure_count: 1,
|
||||
updated_at: '2026-03-28T00:00:05.000Z',
|
||||
}),
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'failed',
|
||||
summary: CODEX_UNAVAILABLE_SUMMARY,
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'arbiter_requested',
|
||||
owner_failure_count: 2,
|
||||
arbiter_requested_at: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('requeues owner finalization once after preserving merge_ready', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
completedRole: 'owner',
|
||||
executionStatus: 'failed',
|
||||
sawOutput: false,
|
||||
taskStatus: 'merge_ready',
|
||||
ownerFailureCount: 1,
|
||||
outputSummary: CODEX_UNAVAILABLE_SUMMARY,
|
||||
}),
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('queues arbiter after repeated owner finalization failures', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
completedRole: 'owner',
|
||||
executionStatus: 'failed',
|
||||
sawOutput: false,
|
||||
taskStatus: 'arbiter_requested',
|
||||
ownerFailureCount: 2,
|
||||
outputSummary: CODEX_UNAVAILABLE_SUMMARY,
|
||||
}),
|
||||
).toBe('pending');
|
||||
});
|
||||
});
|
||||
81
src/paired-arbiter-codex-owner-wake.test.ts
Normal file
81
src/paired-arbiter-codex-owner-wake.test.ts
Normal file
@@ -0,0 +1,81 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./db.js', () => ({
|
||||
getPairedTurnOutputs: vi.fn(() => []),
|
||||
reservePairedTurnReservation: vi.fn(() => true),
|
||||
}));
|
||||
|
||||
import { getPairedTurnOutputs } from './db.js';
|
||||
import { dispatchPairedFollowUpForEvent } from './message-runtime-follow-up.js';
|
||||
import {
|
||||
matchesExpectedPairedFollowUpIntent,
|
||||
resolveNextTurnAction,
|
||||
} from './message-runtime-rules.js';
|
||||
|
||||
describe('arbiter Codex failure owner wake', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(getPairedTurnOutputs).mockReturnValue([]);
|
||||
});
|
||||
|
||||
it('keeps owner follow-ups schedulable when active task has owner failure count', () => {
|
||||
const followUpContext = {
|
||||
taskStatus: 'active' as const,
|
||||
lastTurnOutputRole: 'owner' as const,
|
||||
lastTurnOutputVerdict: 'step_done' as const,
|
||||
ownerFailureCount: 1,
|
||||
};
|
||||
|
||||
expect(resolveNextTurnAction(followUpContext)).toEqual({
|
||||
kind: 'owner-follow-up',
|
||||
});
|
||||
expect(
|
||||
matchesExpectedPairedFollowUpIntent({
|
||||
...followUpContext,
|
||||
intentKind: 'owner-follow-up',
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('requeues owner follow-up when arbiter Codex failure returns the task active', () => {
|
||||
const enqueue = vi.fn();
|
||||
vi.mocked(getPairedTurnOutputs).mockReturnValue([
|
||||
{
|
||||
id: 1,
|
||||
task_id: 'task-arbiter-codex-unavailable',
|
||||
turn_number: 1,
|
||||
role: 'owner',
|
||||
output_text: 'STEP_DONE\nwaiting for CI',
|
||||
verdict: 'step_done',
|
||||
created_at: '2026-03-30T00:00:01.000Z',
|
||||
},
|
||||
] as any);
|
||||
|
||||
const result = dispatchPairedFollowUpForEvent({
|
||||
chatJid: 'group@test',
|
||||
runId: 'run-arbiter-codex-unavailable',
|
||||
task: {
|
||||
id: 'task-arbiter-codex-unavailable',
|
||||
status: 'active',
|
||||
round_trip_count: 1,
|
||||
owner_failure_count: 1,
|
||||
updated_at: '2026-03-30T00:00:02.000Z',
|
||||
},
|
||||
source: 'executor-recovery',
|
||||
completedRole: 'arbiter',
|
||||
executionStatus: 'failed',
|
||||
sawOutput: false,
|
||||
enqueue,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
kind: 'paired-follow-up',
|
||||
intentKind: 'owner-follow-up',
|
||||
scheduled: true,
|
||||
taskStatus: 'active',
|
||||
lastTurnOutputRole: 'owner',
|
||||
nextTurnAction: { kind: 'owner-follow-up' },
|
||||
});
|
||||
expect(enqueue).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -14,16 +14,23 @@ export function handleFailedArbiterExecution(args: {
|
||||
const { task, taskId, summary } = args;
|
||||
if (isTerminalCodexAccountFailure(summary)) {
|
||||
const now = new Date().toISOString();
|
||||
const nextOwnerFailureCount = Math.max(
|
||||
(task.owner_failure_count ?? 0) + 1,
|
||||
1,
|
||||
);
|
||||
transitionPairedTaskStatus({
|
||||
taskId,
|
||||
currentStatus: task.status,
|
||||
nextStatus: 'completed',
|
||||
nextStatus: 'active',
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
arbiter_verdict: 'escalate',
|
||||
owner_failure_count: nextOwnerFailureCount,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: 0,
|
||||
empty_step_done_streak: 0,
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: 'arbiter_codex_unavailable',
|
||||
},
|
||||
});
|
||||
logger.warn(
|
||||
@@ -31,9 +38,10 @@ export function handleFailedArbiterExecution(args: {
|
||||
taskId,
|
||||
role: 'arbiter',
|
||||
status: task.status,
|
||||
nextOwnerFailureCount,
|
||||
summary: summary?.slice(0, 200),
|
||||
},
|
||||
'Completed arbiter task after terminal Codex account failure instead of re-arming arbitration loop',
|
||||
'Returned arbiter task to owner after terminal Codex account failure',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -31,33 +31,81 @@ export function handleFailedOwnerExecution(args: {
|
||||
}): void {
|
||||
const { task, taskId, summary } = args;
|
||||
const now = new Date().toISOString();
|
||||
const nextFailureCount = (task.owner_failure_count ?? 0) + 1;
|
||||
if (isTerminalCodexAccountFailure(summary)) {
|
||||
if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) {
|
||||
requestArbiterOrEscalate({
|
||||
taskId,
|
||||
currentStatus: task.status,
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
now,
|
||||
arbiterLogMessage:
|
||||
'Owner Codex unavailable repeatedly — requesting arbiter',
|
||||
escalateLogMessage:
|
||||
'Owner Codex unavailable repeatedly — escalating to user',
|
||||
logContext: {
|
||||
taskId,
|
||||
role: 'owner',
|
||||
previousStatus: task.status,
|
||||
ownerFailureCount: nextFailureCount,
|
||||
summary: summary?.slice(0, 160),
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: nextFailureCount,
|
||||
arbiter_verdict: null,
|
||||
completion_reason: null,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const patch = {
|
||||
owner_failure_count: nextFailureCount,
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
};
|
||||
if (task.status === 'active' || task.status === 'merge_ready') {
|
||||
applyPairedTaskPatch({
|
||||
taskId,
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch,
|
||||
});
|
||||
logger.warn(
|
||||
{
|
||||
taskId,
|
||||
role: 'owner',
|
||||
status: task.status,
|
||||
ownerFailureCount: nextFailureCount,
|
||||
summary: summary?.slice(0, 200),
|
||||
},
|
||||
'Preserved owner task after terminal Codex account failure',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
transitionPairedTaskStatus({
|
||||
taskId,
|
||||
currentStatus: task.status,
|
||||
nextStatus: 'completed',
|
||||
nextStatus: 'active',
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
arbiter_verdict: 'escalate',
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: 'owner_codex_unavailable',
|
||||
},
|
||||
patch,
|
||||
});
|
||||
logger.warn(
|
||||
{
|
||||
taskId,
|
||||
role: 'owner',
|
||||
status: task.status,
|
||||
ownerFailureCount: nextFailureCount,
|
||||
summary: summary?.slice(0, 200),
|
||||
},
|
||||
'Completed owner task after terminal Codex account failure instead of retrying owner loop',
|
||||
'Reset owner task to active after terminal Codex account failure',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const nextFailureCount = (task.owner_failure_count ?? 0) + 1;
|
||||
|
||||
if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) {
|
||||
requestArbiterOrEscalate({
|
||||
taskId,
|
||||
|
||||
@@ -197,10 +197,16 @@ describe('paired execution routing loop guards', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('does not re-arm arbiter loop after terminal Codex account failure', () => {
|
||||
it('returns arbiter terminal Codex account failures to owner without re-arming arbiter', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'in_arbitration',
|
||||
owner_failure_count: 1,
|
||||
owner_step_done_streak: 3,
|
||||
finalize_step_done_count: 1,
|
||||
empty_step_done_streak: 2,
|
||||
arbiter_verdict: 'revise',
|
||||
arbiter_requested_at: '2026-03-28T00:00:04.000Z',
|
||||
updated_at: '2026-03-28T00:00:05.000Z',
|
||||
}),
|
||||
);
|
||||
@@ -216,10 +222,13 @@ describe('paired execution routing loop guards', () => {
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'completed',
|
||||
arbiter_verdict: 'escalate',
|
||||
status: 'active',
|
||||
owner_failure_count: 2,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: 0,
|
||||
empty_step_done_streak: 0,
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: 'arbiter_codex_unavailable',
|
||||
}),
|
||||
);
|
||||
});
|
||||
@@ -251,7 +260,7 @@ describe('paired execution routing loop guards', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('completes owner task after terminal Codex account failure instead of retrying owner forever', () => {
|
||||
it('preserves owner task after terminal Codex account failure instead of completing silently', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
@@ -270,10 +279,10 @@ describe('paired execution routing loop guards', () => {
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'completed',
|
||||
arbiter_verdict: 'escalate',
|
||||
owner_failure_count: 1,
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: 'owner_codex_unavailable',
|
||||
completion_reason: null,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user