Recover Codex compaction failures in paired owner flow
This commit is contained in:
@@ -118,6 +118,7 @@ export function applyBaseSchema(database: Database): void {
|
||||
plan_notes TEXT,
|
||||
review_requested_at TEXT,
|
||||
round_trip_count INTEGER NOT NULL DEFAULT 0,
|
||||
owner_failure_count INTEGER NOT NULL DEFAULT 0,
|
||||
status TEXT NOT NULL DEFAULT 'active',
|
||||
arbiter_verdict TEXT,
|
||||
arbiter_requested_at TEXT,
|
||||
|
||||
@@ -35,6 +35,7 @@ function getExpectedSchemaMigrations(): Array<{
|
||||
{ version: 8, name: 'paired_task_schema_cleanup' },
|
||||
{ version: 9, name: 'paired_workspace_project_schema_cleanup' },
|
||||
{ version: 10, name: 'paired_turn_provenance_upgrade' },
|
||||
{ version: 11, name: 'owner_failure_count' },
|
||||
];
|
||||
}
|
||||
|
||||
|
||||
23
src/db/migrations/011_owner-failure-count.ts
Normal file
23
src/db/migrations/011_owner-failure-count.ts
Normal file
@@ -0,0 +1,23 @@
|
||||
import type { Database } from 'bun:sqlite';
|
||||
|
||||
import { tableHasColumn } from './helpers.js';
|
||||
import type { SchemaMigrationDefinition } from './types.js';
|
||||
|
||||
export const OWNER_FAILURE_COUNT_MIGRATION: SchemaMigrationDefinition = {
|
||||
version: 11,
|
||||
name: 'owner_failure_count',
|
||||
apply(database: Database) {
|
||||
if (!tableHasColumn(database, 'paired_tasks', 'owner_failure_count')) {
|
||||
database.exec(`
|
||||
ALTER TABLE paired_tasks
|
||||
ADD COLUMN owner_failure_count INTEGER NOT NULL DEFAULT 0
|
||||
`);
|
||||
}
|
||||
|
||||
database.exec(`
|
||||
UPDATE paired_tasks
|
||||
SET owner_failure_count = 0
|
||||
WHERE owner_failure_count IS NULL
|
||||
`);
|
||||
},
|
||||
};
|
||||
@@ -10,6 +10,7 @@ import { RUNTIME_SERVICE_METADATA_MIGRATION } from './007_runtime-service-metada
|
||||
import { PAIRED_TASK_SCHEMA_CLEANUP_MIGRATION } from './008_paired-task-schema-cleanup.js';
|
||||
import { PAIRED_WORKSPACE_PROJECT_SCHEMA_CLEANUP_MIGRATION } from './009_paired-workspace-project-schema-cleanup.js';
|
||||
import { PAIRED_TURN_PROVENANCE_UPGRADE_MIGRATION } from './010_paired-turn-provenance-upgrade.js';
|
||||
import { OWNER_FAILURE_COUNT_MIGRATION } from './011_owner-failure-count.js';
|
||||
import type {
|
||||
SchemaMigrationArgs,
|
||||
SchemaMigrationDefinition,
|
||||
@@ -28,6 +29,7 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [
|
||||
PAIRED_TASK_SCHEMA_CLEANUP_MIGRATION,
|
||||
PAIRED_WORKSPACE_PROJECT_SCHEMA_CLEANUP_MIGRATION,
|
||||
PAIRED_TURN_PROVENANCE_UPGRADE_MIGRATION,
|
||||
OWNER_FAILURE_COUNT_MIGRATION,
|
||||
];
|
||||
|
||||
function ensureSchemaMigrationsTable(database: Database): void {
|
||||
|
||||
@@ -50,6 +50,7 @@ export type PairedTaskUpdates = Partial<
|
||||
| 'plan_notes'
|
||||
| 'review_requested_at'
|
||||
| 'round_trip_count'
|
||||
| 'owner_failure_count'
|
||||
| 'status'
|
||||
| 'arbiter_verdict'
|
||||
| 'arbiter_requested_at'
|
||||
@@ -171,6 +172,7 @@ export function createPairedTaskInDatabase(
|
||||
plan_notes,
|
||||
review_requested_at,
|
||||
round_trip_count,
|
||||
owner_failure_count,
|
||||
status,
|
||||
arbiter_verdict,
|
||||
arbiter_requested_at,
|
||||
@@ -178,7 +180,7 @@ export function createPairedTaskInDatabase(
|
||||
created_at,
|
||||
updated_at
|
||||
)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`,
|
||||
)
|
||||
.run(
|
||||
@@ -195,6 +197,7 @@ export function createPairedTaskInDatabase(
|
||||
task.plan_notes,
|
||||
task.review_requested_at,
|
||||
task.round_trip_count,
|
||||
task.owner_failure_count ?? 0,
|
||||
task.status,
|
||||
task.arbiter_verdict,
|
||||
task.arbiter_requested_at,
|
||||
@@ -279,6 +282,10 @@ export function updatePairedTaskInDatabase(
|
||||
fields.push('round_trip_count = ?');
|
||||
values.push(updates.round_trip_count);
|
||||
}
|
||||
if (updates.owner_failure_count !== undefined) {
|
||||
fields.push('owner_failure_count = ?');
|
||||
values.push(updates.owner_failure_count);
|
||||
}
|
||||
if (updates.status !== undefined) {
|
||||
fields.push('status = ?');
|
||||
values.push(updates.status);
|
||||
@@ -338,6 +345,10 @@ export function updatePairedTaskIfUnchangedInDatabase(
|
||||
fields.push('round_trip_count = ?');
|
||||
values.push(updates.round_trip_count);
|
||||
}
|
||||
if (updates.owner_failure_count !== undefined) {
|
||||
fields.push('owner_failure_count = ?');
|
||||
values.push(updates.owner_failure_count);
|
||||
}
|
||||
if (updates.status !== undefined) {
|
||||
fields.push('status = ?');
|
||||
values.push(updates.status);
|
||||
|
||||
@@ -5,7 +5,10 @@ import type { AttemptStreamedTrigger } from './agent-attempt-retry.js';
|
||||
import { runAgentProcess, type AgentOutput } from './agent-runner.js';
|
||||
import { getCodexAccountCount } from './codex-token-rotation.js';
|
||||
import type { PreparedPairedExecutionContext } from './paired-execution-context.js';
|
||||
import { shouldResetSessionOnAgentFailure } from './session-recovery.js';
|
||||
import {
|
||||
shouldResetCodexSessionOnAgentFailure,
|
||||
shouldResetSessionOnAgentFailure,
|
||||
} from './session-recovery.js';
|
||||
import type { AgentType, RegisteredGroup, RoomRoleContext } from './types.js';
|
||||
|
||||
export interface MessageAgentAttempt {
|
||||
@@ -148,6 +151,13 @@ export async function runMessageAgentAttempt(args: {
|
||||
) {
|
||||
resetSessionRequested = true;
|
||||
}
|
||||
if (
|
||||
!isClaudeCodeAgent &&
|
||||
provider === 'codex' &&
|
||||
shouldResetCodexSessionOnAgentFailure(output)
|
||||
) {
|
||||
resetSessionRequested = true;
|
||||
}
|
||||
if (
|
||||
output.newSessionId &&
|
||||
!resetSessionRequested &&
|
||||
@@ -197,7 +207,9 @@ export async function runMessageAgentAttempt(args: {
|
||||
? outputText.slice(0, 160)
|
||||
: output.error?.slice(0, 160),
|
||||
},
|
||||
'Suppressed retryable Claude session failure from chat output',
|
||||
provider === 'claude'
|
||||
? 'Suppressed retryable Claude session failure from chat output'
|
||||
: 'Suppressed retryable Codex session failure from chat output',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -12,7 +12,11 @@ import type {
|
||||
} from './agent-error-detection.js';
|
||||
import type { PairedExecutionLifecycle } from './message-agent-executor-paired.js';
|
||||
import type { MessageAgentAttempt } from './message-agent-executor-attempt-runner.js';
|
||||
import { shouldResetSessionOnAgentFailure } from './session-recovery.js';
|
||||
import {
|
||||
shouldResetCodexSessionOnAgentFailure,
|
||||
shouldResetSessionOnAgentFailure,
|
||||
shouldRetryFreshCodexSessionOnAgentFailure,
|
||||
} from './session-recovery.js';
|
||||
import { getErrorMessage } from './utils.js';
|
||||
|
||||
type AttemptResult = 'success' | 'error';
|
||||
@@ -208,6 +212,57 @@ export async function executeMessageAgentAttemptLifecycle(args: {
|
||||
};
|
||||
};
|
||||
|
||||
const isRetryableCodexSessionFailure = (
|
||||
attempt: MessageAgentAttempt,
|
||||
): boolean => {
|
||||
if (provider !== 'codex' || attempt.sawOutput) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (attempt.retryableSessionFailureDetected === true) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (attempt.error == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return shouldRetryFreshCodexSessionOnAgentFailure({
|
||||
result: null,
|
||||
error: getErrorMessage(attempt.error),
|
||||
});
|
||||
};
|
||||
|
||||
const recoverRetryableCodexSessionFailure = async (
|
||||
attempt: MessageAgentAttempt,
|
||||
): Promise<{
|
||||
attempt: MessageAgentAttempt;
|
||||
resolved: AttemptResult | null;
|
||||
}> => {
|
||||
if (!isRetryableCodexSessionFailure(attempt)) {
|
||||
return { attempt, resolved: null };
|
||||
}
|
||||
|
||||
clearStoredSession();
|
||||
clearRoleSdkSessions();
|
||||
log.warn(
|
||||
'Cleared poisoned Codex session before visible output, retrying fresh session',
|
||||
);
|
||||
|
||||
const freshAttempt = await runTrackedAttempt('codex');
|
||||
if (!isRetryableCodexSessionFailure(freshAttempt)) {
|
||||
return { attempt: freshAttempt, resolved: null };
|
||||
}
|
||||
|
||||
clearStoredSession();
|
||||
log.warn('Fresh Codex retry also hit a retryable session failure');
|
||||
log.error('Retryable Codex session failure persisted after fresh retry');
|
||||
return {
|
||||
attempt: freshAttempt,
|
||||
resolved: 'error',
|
||||
};
|
||||
};
|
||||
|
||||
const handlePrimaryAttemptFailure = async (
|
||||
attempt: MessageAgentAttempt,
|
||||
rotationMessage: string,
|
||||
@@ -294,6 +349,19 @@ export async function executeMessageAgentAttemptLifecycle(args: {
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
!isClaudeCodeAgent &&
|
||||
provider === 'codex' &&
|
||||
(resetSessionRequested || shouldResetCodexSessionOnAgentFailure(output))
|
||||
) {
|
||||
clearStoredSession();
|
||||
clearRoleSdkSessions();
|
||||
log.warn(
|
||||
{ sessionFolder },
|
||||
'Cleared poisoned Codex session after unrecoverable error',
|
||||
);
|
||||
}
|
||||
|
||||
if (output.status === 'error') {
|
||||
return handlePrimaryAttemptFailure(
|
||||
attempt,
|
||||
@@ -354,6 +422,13 @@ export async function executeMessageAgentAttemptLifecycle(args: {
|
||||
}
|
||||
primaryAttempt = recoveredSessionAttempt.attempt;
|
||||
|
||||
const recoveredCodexSessionAttempt =
|
||||
await recoverRetryableCodexSessionFailure(primaryAttempt);
|
||||
if (recoveredCodexSessionAttempt.resolved) {
|
||||
return recoveredCodexSessionAttempt.resolved;
|
||||
}
|
||||
primaryAttempt = recoveredCodexSessionAttempt.attempt;
|
||||
|
||||
if (primaryAttempt.error) {
|
||||
return handlePrimaryAttemptFailure(
|
||||
primaryAttempt,
|
||||
|
||||
@@ -172,6 +172,17 @@ describe('message-agent-executor-rules', () => {
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('resolves pending arbiter follow-up requeue after repeated owner execution failures', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
completedRole: 'owner',
|
||||
executionStatus: 'failed',
|
||||
sawOutput: false,
|
||||
taskStatus: 'arbiter_requested',
|
||||
}),
|
||||
).toBe('pending');
|
||||
});
|
||||
|
||||
it('does not request an executor-side follow-up after successful owner output moved the task to review_ready', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
|
||||
@@ -196,7 +196,9 @@ vi.mock('./agent-error-detection.js', async (importOriginal) => {
|
||||
});
|
||||
|
||||
vi.mock('./session-recovery.js', () => ({
|
||||
shouldResetCodexSessionOnAgentFailure: vi.fn(() => false),
|
||||
shouldResetSessionOnAgentFailure: vi.fn(() => false),
|
||||
shouldRetryFreshCodexSessionOnAgentFailure: vi.fn(() => false),
|
||||
shouldRetryFreshSessionOnAgentFailure: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
@@ -3330,6 +3332,70 @@ describe('runAgentForGroup Claude rotation', () => {
|
||||
expect(deps.clearSession).toHaveBeenCalledWith('test-claude');
|
||||
});
|
||||
|
||||
it('drops a poisoned Codex session id before retrying a fresh session after remote compaction failure', async () => {
|
||||
const group = {
|
||||
...makeGroup(),
|
||||
folder: 'test-codex',
|
||||
agentType: 'codex' as const,
|
||||
};
|
||||
const deps = {
|
||||
...makeDeps(),
|
||||
getSessions: () => ({ 'test-codex': 'stale-codex-session-id' }),
|
||||
};
|
||||
|
||||
vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({
|
||||
chat_jid: 'group@test',
|
||||
owner_agent_type: 'codex',
|
||||
reviewer_agent_type: null,
|
||||
arbiter_agent_type: null,
|
||||
owner_service_id: 'codex-main',
|
||||
reviewer_service_id: null,
|
||||
arbiter_service_id: null,
|
||||
activated_at: null,
|
||||
reason: null,
|
||||
explicit: false,
|
||||
});
|
||||
vi.mocked(sessionRecovery.shouldRetryFreshCodexSessionOnAgentFailure)
|
||||
.mockReturnValueOnce(true)
|
||||
.mockReturnValueOnce(false);
|
||||
vi.mocked(sessionRecovery.shouldResetCodexSessionOnAgentFailure)
|
||||
.mockReturnValueOnce(true)
|
||||
.mockReturnValueOnce(false);
|
||||
|
||||
vi.mocked(agentRunner.runAgentProcess)
|
||||
.mockImplementationOnce(async (_group, input) => {
|
||||
expect(input.sessionId).toBe('stale-codex-session-id');
|
||||
throw new Error(
|
||||
"Error running remote compact task: Unknown parameter: 'prompt_cache_retention'",
|
||||
);
|
||||
})
|
||||
.mockImplementationOnce(async (_group, input, _onProcess, onOutput) => {
|
||||
expect(input.sessionId).toBeUndefined();
|
||||
await onOutput?.({
|
||||
status: 'success',
|
||||
phase: 'final',
|
||||
result: 'fresh Codex retry success',
|
||||
});
|
||||
return {
|
||||
status: 'success',
|
||||
result: 'fresh Codex retry success',
|
||||
newSessionId: 'fresh-codex-session-id',
|
||||
};
|
||||
});
|
||||
|
||||
const result = await runAgentForGroup(deps, {
|
||||
group,
|
||||
prompt: 'hello',
|
||||
chatJid: 'group@test',
|
||||
runId: 'run-stale-codex-session-id-retry',
|
||||
onOutput: async () => {},
|
||||
});
|
||||
|
||||
expect(result).toBe('success');
|
||||
expect(agentRunner.runAgentProcess).toHaveBeenCalledTimes(2);
|
||||
expect(deps.clearSession).toHaveBeenCalledWith('test-codex');
|
||||
});
|
||||
|
||||
it('suppresses a usage-exhausted banner even when Claude already emitted progress text', async () => {
|
||||
const outputs: string[] = [];
|
||||
|
||||
|
||||
@@ -133,7 +133,7 @@ describe('message-runtime-rules', () => {
|
||||
).toEqual({ kind: 'none' });
|
||||
});
|
||||
|
||||
it('dispatches owner delivery success through a paired follow-up enqueue only for reviewer turns', () => {
|
||||
it('dispatches owner delivery success through a paired follow-up enqueue for reviewer or arbiter turns', () => {
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
source: 'owner-delivery-success',
|
||||
@@ -143,6 +143,15 @@ describe('message-runtime-rules', () => {
|
||||
kind: 'enqueue',
|
||||
queueKind: 'paired-follow-up',
|
||||
});
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
source: 'owner-delivery-success',
|
||||
nextTurnAction: { kind: 'arbiter-turn' },
|
||||
}),
|
||||
).toEqual({
|
||||
kind: 'enqueue',
|
||||
queueKind: 'paired-follow-up',
|
||||
});
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
source: 'owner-delivery-success',
|
||||
@@ -151,6 +160,21 @@ describe('message-runtime-rules', () => {
|
||||
).toEqual({ kind: 'none' });
|
||||
});
|
||||
|
||||
it('dispatches failed owner recovery through a paired follow-up when execution escalated to arbiter', () => {
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
source: 'executor-recovery',
|
||||
nextTurnAction: { kind: 'arbiter-turn' },
|
||||
completedRole: 'owner',
|
||||
executionStatus: 'failed',
|
||||
sawOutput: false,
|
||||
}),
|
||||
).toEqual({
|
||||
kind: 'enqueue',
|
||||
queueKind: 'paired-follow-up',
|
||||
});
|
||||
});
|
||||
|
||||
it('dispatches reviewer and arbiter delivery success through paired follow-up enqueue when a handoff is pending', () => {
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
|
||||
@@ -183,7 +183,8 @@ export function resolveFollowUpDispatch(args: {
|
||||
args.source === 'owner-delivery-success' ||
|
||||
args.completedRole === 'owner'
|
||||
) {
|
||||
return args.nextTurnAction.kind === 'reviewer-turn'
|
||||
return args.nextTurnAction.kind === 'reviewer-turn' ||
|
||||
args.nextTurnAction.kind === 'arbiter-turn'
|
||||
? { kind: 'enqueue', queueKind: 'paired-follow-up' }
|
||||
: { kind: 'none' };
|
||||
}
|
||||
@@ -211,6 +212,7 @@ export function resolveFollowUpDispatch(args: {
|
||||
return { kind: 'none' };
|
||||
}
|
||||
if (
|
||||
args.completedRole !== 'owner' &&
|
||||
args.completedRole !== 'reviewer' &&
|
||||
args.completedRole !== 'arbiter'
|
||||
) {
|
||||
|
||||
@@ -21,26 +21,72 @@ import {
|
||||
import type { PairedTask } from './types.js';
|
||||
|
||||
type OwnerFinalizeOutcome = 'stop' | 're_review';
|
||||
const OWNER_FAILURE_ESCALATION_THRESHOLD = 2;
|
||||
|
||||
export function handleFailedOwnerExecution(args: {
|
||||
task: PairedTask;
|
||||
taskId: string;
|
||||
summary?: string | null;
|
||||
}): void {
|
||||
const { task, taskId } = args;
|
||||
const { task, taskId, summary } = args;
|
||||
const now = new Date().toISOString();
|
||||
const nextFailureCount = (task.owner_failure_count ?? 0) + 1;
|
||||
|
||||
if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) {
|
||||
requestArbiterOrEscalate({
|
||||
taskId,
|
||||
currentStatus: task.status,
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
now,
|
||||
arbiterLogMessage:
|
||||
'Owner failed repeatedly without a visible verdict — requesting arbiter',
|
||||
escalateLogMessage:
|
||||
'Owner failed repeatedly without a visible verdict — escalating to user',
|
||||
logContext: {
|
||||
taskId,
|
||||
role: 'owner',
|
||||
previousStatus: task.status,
|
||||
ownerFailureCount: nextFailureCount,
|
||||
summary: summary?.slice(0, 160),
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: nextFailureCount,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (task.status !== 'active') {
|
||||
const now = new Date().toISOString();
|
||||
transitionPairedTaskStatus({
|
||||
taskId,
|
||||
currentStatus: task.status,
|
||||
nextStatus: 'active',
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
owner_failure_count: nextFailureCount,
|
||||
},
|
||||
});
|
||||
} else {
|
||||
applyPairedTaskPatch({
|
||||
taskId,
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
owner_failure_count: nextFailureCount,
|
||||
},
|
||||
});
|
||||
logger.info(
|
||||
{ taskId, role: 'owner', previousStatus: task.status },
|
||||
'Reset task to active after failed execution',
|
||||
);
|
||||
}
|
||||
logger.info(
|
||||
{
|
||||
taskId,
|
||||
role: 'owner',
|
||||
previousStatus: task.status,
|
||||
ownerFailureCount: nextFailureCount,
|
||||
summary: summary?.slice(0, 160),
|
||||
},
|
||||
'Reset task to active after failed owner execution',
|
||||
);
|
||||
}
|
||||
|
||||
function handleOwnerFinalizeCompletion(args: {
|
||||
@@ -90,6 +136,9 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
hasNewChanges,
|
||||
summary: summary?.slice(0, 100),
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
},
|
||||
});
|
||||
return 'stop';
|
||||
}
|
||||
@@ -102,6 +151,9 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
nextStatus: 'active',
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
},
|
||||
});
|
||||
}
|
||||
logger.info(
|
||||
@@ -126,6 +178,7 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
completion_reason: 'done',
|
||||
owner_failure_count: 0,
|
||||
},
|
||||
});
|
||||
logger.info(
|
||||
@@ -166,6 +219,7 @@ function maybeAutoTriggerReviewerAfterOwnerCompletion(args: {
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
round_trip_count: task.round_trip_count + 1,
|
||||
owner_failure_count: 0,
|
||||
},
|
||||
});
|
||||
if (hasActiveCiWatcherForChat(task.chat_jid)) {
|
||||
@@ -229,6 +283,9 @@ export function handleOwnerCompletion(args: {
|
||||
ownerVerdict,
|
||||
summary: summary?.slice(0, 100),
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -247,6 +247,7 @@ export function transitionPairedTaskStatus(args: {
|
||||
plan_notes?: string | null;
|
||||
review_requested_at?: string | null;
|
||||
round_trip_count?: number;
|
||||
owner_failure_count?: number;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
@@ -290,6 +291,7 @@ export function applyPairedTaskPatch(args: {
|
||||
plan_notes?: string | null;
|
||||
review_requested_at?: string | null;
|
||||
round_trip_count?: number;
|
||||
owner_failure_count?: number;
|
||||
status?: PairedTaskStatus;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
@@ -324,6 +326,17 @@ export function requestArbiterOrEscalate(args: {
|
||||
arbiterLogMessage: string;
|
||||
escalateLogMessage: string;
|
||||
logContext?: Record<string, unknown>;
|
||||
patch?: {
|
||||
title?: string | null;
|
||||
source_ref?: string | null;
|
||||
plan_notes?: string | null;
|
||||
review_requested_at?: string | null;
|
||||
round_trip_count?: number;
|
||||
owner_failure_count?: number;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
};
|
||||
}): boolean {
|
||||
const {
|
||||
taskId,
|
||||
@@ -342,6 +355,7 @@ export function requestArbiterOrEscalate(args: {
|
||||
expectedUpdatedAt,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
...args.patch,
|
||||
arbiter_requested_at: now,
|
||||
},
|
||||
});
|
||||
@@ -358,6 +372,7 @@ export function requestArbiterOrEscalate(args: {
|
||||
expectedUpdatedAt,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
...args.patch,
|
||||
completion_reason: 'escalated',
|
||||
},
|
||||
});
|
||||
|
||||
@@ -139,6 +139,7 @@ function buildPairedTask(overrides: Partial<PairedTask> = {}): PairedTask {
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
@@ -980,7 +981,7 @@ describe('paired execution context', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('still resets owner tasks to active after failed execution', () => {
|
||||
it('increments owner failure count and resets owner tasks to active after failed execution', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'merge_ready',
|
||||
@@ -998,11 +999,38 @@ describe('paired execution context', () => {
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'active',
|
||||
owner_failure_count: 1,
|
||||
updated_at: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('requests arbiter after repeated owner execution failures without a visible verdict', () => {
|
||||
vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true);
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
owner_failure_count: 1,
|
||||
}),
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'failed',
|
||||
summary: "Error running remote compact task: Unknown parameter: 'prompt_cache_retention'",
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'arbiter_requested',
|
||||
owner_failure_count: 2,
|
||||
arbiter_requested_at: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('releases the execution lease even when a completion handler throws', () => {
|
||||
const transitionError = new Error('transition failed');
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
|
||||
@@ -137,6 +137,7 @@ function createActiveTaskForRoom(args: {
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
@@ -369,7 +370,7 @@ export function preparePairedExecutionContext(args: {
|
||||
expectedUpdatedAt: latestTask.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
...(hasHuman ? { round_trip_count: 0 } : {}),
|
||||
...(hasHuman ? { round_trip_count: 0, owner_failure_count: 0 } : {}),
|
||||
},
|
||||
});
|
||||
} else {
|
||||
@@ -378,7 +379,7 @@ export function preparePairedExecutionContext(args: {
|
||||
expectedUpdatedAt: latestTask.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
...(hasHuman ? { round_trip_count: 0 } : {}),
|
||||
...(hasHuman ? { round_trip_count: 0, owner_failure_count: 0 } : {}),
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -551,7 +552,7 @@ export function completePairedExecutionContext(args: {
|
||||
handleFailedArbiterExecution({ task, taskId });
|
||||
return;
|
||||
}
|
||||
handleFailedOwnerExecution({ task, taskId });
|
||||
handleFailedOwnerExecution({ task, taskId, summary: args.summary });
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
shouldResetCodexSessionOnAgentFailure,
|
||||
shouldResetSessionOnAgentFailure,
|
||||
shouldRetryFreshCodexSessionOnAgentFailure,
|
||||
shouldRetryFreshSessionOnAgentFailure,
|
||||
} from './session-recovery.js';
|
||||
|
||||
@@ -99,3 +101,36 @@ describe('shouldRetryFreshSessionOnAgentFailure', () => {
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldResetCodexSessionOnAgentFailure', () => {
|
||||
it('matches remote compact task failures that mention prompt_cache_retention', () => {
|
||||
expect(
|
||||
shouldResetCodexSessionOnAgentFailure({
|
||||
result: null,
|
||||
error:
|
||||
"Error running remote compact task: Unknown parameter: 'prompt_cache_retention'",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('does not match unrelated Codex failures', () => {
|
||||
expect(
|
||||
shouldResetCodexSessionOnAgentFailure({
|
||||
result: null,
|
||||
error: 'Codex process exited with code 1',
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldRetryFreshCodexSessionOnAgentFailure', () => {
|
||||
it('retries the same remote compact task failure with a fresh session', () => {
|
||||
expect(
|
||||
shouldRetryFreshCodexSessionOnAgentFailure({
|
||||
result: null,
|
||||
error:
|
||||
"Error running remote compact task: Unknown parameter: 'prompt_cache_retention'",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,6 +17,11 @@ const SESSION_RETRY_PATTERNS = [
|
||||
/invalid[^\n]*signature[^\n]*thinking/i,
|
||||
];
|
||||
|
||||
const CODEX_SESSION_RESET_PATTERNS = [
|
||||
/Error running remote compact task/i,
|
||||
/prompt_cache_retention/i,
|
||||
];
|
||||
|
||||
function toText(value: string | object | null | undefined): string[] {
|
||||
if (!value) return [];
|
||||
if (typeof value === 'string') return [value];
|
||||
@@ -51,3 +56,21 @@ export function shouldRetryFreshSessionOnAgentFailure(
|
||||
SESSION_RETRY_PATTERNS.some((pattern) => pattern.test(text)),
|
||||
);
|
||||
}
|
||||
|
||||
export function shouldResetCodexSessionOnAgentFailure(
|
||||
output: Pick<AgentOutput, 'result' | 'output' | 'error'>,
|
||||
): boolean {
|
||||
const texts = [
|
||||
...toText(getAgentOutputText(output)),
|
||||
...toText(output.error),
|
||||
];
|
||||
return texts.some((text) =>
|
||||
CODEX_SESSION_RESET_PATTERNS.some((pattern) => pattern.test(text)),
|
||||
);
|
||||
}
|
||||
|
||||
export function shouldRetryFreshCodexSessionOnAgentFailure(
|
||||
output: Pick<AgentOutput, 'result' | 'output' | 'error'>,
|
||||
): boolean {
|
||||
return shouldResetCodexSessionOnAgentFailure(output);
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ vi.mock('./codex-token-rotation.js', () => ({
|
||||
}));
|
||||
|
||||
vi.mock('./session-recovery.js', () => ({
|
||||
shouldRetryFreshCodexSessionOnAgentFailure: vi.fn(() => false),
|
||||
shouldRetryFreshSessionOnAgentFailure: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
@@ -34,7 +35,10 @@ import {
|
||||
isClaudeAuthError,
|
||||
} from './agent-error-detection.js';
|
||||
import { detectCodexRotationTrigger } from './codex-token-rotation.js';
|
||||
import { shouldRetryFreshSessionOnAgentFailure } from './session-recovery.js';
|
||||
import {
|
||||
shouldRetryFreshCodexSessionOnAgentFailure,
|
||||
shouldRetryFreshSessionOnAgentFailure,
|
||||
} from './session-recovery.js';
|
||||
|
||||
import {
|
||||
evaluateStreamedOutput,
|
||||
@@ -91,6 +95,7 @@ beforeEach(() => {
|
||||
shouldRotate: false,
|
||||
reason: '',
|
||||
});
|
||||
vi.mocked(shouldRetryFreshCodexSessionOnAgentFailure).mockReturnValue(false);
|
||||
vi.mocked(shouldRetryFreshSessionOnAgentFailure).mockReturnValue(false);
|
||||
});
|
||||
|
||||
@@ -252,6 +257,28 @@ describe('evaluateStreamedOutput', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Codex retryable session failure suppression', () => {
|
||||
it('suppresses remote compact failures before any visible output', () => {
|
||||
vi.mocked(shouldRetryFreshCodexSessionOnAgentFailure).mockReturnValue(
|
||||
true,
|
||||
);
|
||||
|
||||
const result = evaluateStreamedOutput(
|
||||
successOutput(
|
||||
"Error running remote compact task: Unknown parameter: 'prompt_cache_retention'",
|
||||
'intermediate',
|
||||
),
|
||||
freshState(),
|
||||
codexOpts,
|
||||
);
|
||||
|
||||
expect(result.shouldForwardOutput).toBe(false);
|
||||
expect(result.suppressedRetryableSessionFailure).toBe(true);
|
||||
expect(result.state.retryableSessionFailureDetected).toBe(true);
|
||||
expect(result.state.sawOutput).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Claude auth-expired banner', () => {
|
||||
it('suppresses output and returns newTrigger', () => {
|
||||
vi.mocked(isClaudeAuthExpiredMessage).mockReturnValue(true);
|
||||
|
||||
@@ -15,7 +15,10 @@ import {
|
||||
} from './agent-error-detection.js';
|
||||
import type { AgentOutput } from './agent-runner.js';
|
||||
import { detectCodexRotationTrigger } from './codex-token-rotation.js';
|
||||
import { shouldRetryFreshSessionOnAgentFailure } from './session-recovery.js';
|
||||
import {
|
||||
shouldRetryFreshCodexSessionOnAgentFailure,
|
||||
shouldRetryFreshSessionOnAgentFailure,
|
||||
} from './session-recovery.js';
|
||||
|
||||
export interface StreamedTriggerReason {
|
||||
reason: AgentTriggerReason;
|
||||
@@ -74,6 +77,19 @@ export function evaluateStreamedOutput(
|
||||
};
|
||||
}
|
||||
|
||||
if (
|
||||
isPrimaryCodex &&
|
||||
!state.sawOutput &&
|
||||
shouldRetryFreshCodexSessionOnAgentFailure(output)
|
||||
) {
|
||||
nextState.retryableSessionFailureDetected = true;
|
||||
return {
|
||||
state: nextState,
|
||||
shouldForwardOutput: false,
|
||||
suppressedRetryableSessionFailure: true,
|
||||
};
|
||||
}
|
||||
|
||||
if (
|
||||
isPrimaryClaude &&
|
||||
output.status === 'success' &&
|
||||
|
||||
@@ -82,6 +82,7 @@ export interface PairedTask {
|
||||
plan_notes: string | null;
|
||||
review_requested_at: string | null;
|
||||
round_trip_count: number;
|
||||
owner_failure_count?: number | null;
|
||||
status: PairedTaskStatus;
|
||||
arbiter_verdict: string | null;
|
||||
arbiter_requested_at: string | null;
|
||||
|
||||
Reference in New Issue
Block a user