fix: preserve owner retry failure counts
This commit is contained in:
125
src/message-runtime-queue-owner-retry.test.ts
Normal file
125
src/message-runtime-queue-owner-retry.test.ts
Normal file
@@ -0,0 +1,125 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { _initTestDatabase, createPairedTask } from './db.js';
|
||||
import { runQueuedGroupTurn } from './message-runtime-queue.js';
|
||||
import { resetPairedFollowUpScheduleState } from './paired-follow-up-scheduler.js';
|
||||
import type { Channel, PairedTask, RegisteredGroup } from './types.js';
|
||||
|
||||
function makeGroup(): RegisteredGroup {
|
||||
return {
|
||||
name: 'Test Group',
|
||||
folder: 'test-group',
|
||||
trigger: '@Andy',
|
||||
added_at: '2026-03-30T00:00:00.000Z',
|
||||
requiresTrigger: false,
|
||||
agentType: 'codex',
|
||||
workDir: '/repo',
|
||||
};
|
||||
}
|
||||
|
||||
function makeTask(overrides: Partial<PairedTask> = {}): PairedTask {
|
||||
return {
|
||||
id: 'task-owner-retry',
|
||||
chat_jid: 'group@test',
|
||||
group_folder: 'test-group',
|
||||
owner_service_id: 'codex-main',
|
||||
reviewer_service_id: 'claude',
|
||||
owner_agent_type: 'codex',
|
||||
reviewer_agent_type: 'claude-code',
|
||||
arbiter_agent_type: null,
|
||||
title: null,
|
||||
source_ref: 'HEAD',
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 1,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: 0,
|
||||
task_done_then_user_reopen_count: 0,
|
||||
empty_step_done_streak: 0,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-30T00:00:00.000Z',
|
||||
updated_at: '2026-03-30T00:00:10.000Z',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeChannel(): Channel {
|
||||
return {
|
||||
name: 'discord-owner',
|
||||
connect: vi.fn(),
|
||||
sendMessage: vi.fn(),
|
||||
isConnected: vi.fn(() => true),
|
||||
ownsJid: vi.fn(() => true),
|
||||
disconnect: vi.fn(),
|
||||
} as unknown as Channel;
|
||||
}
|
||||
|
||||
describe('message-runtime-queue owner retry turns', () => {
|
||||
beforeEach(() => {
|
||||
_initTestDatabase();
|
||||
resetPairedFollowUpScheduleState();
|
||||
});
|
||||
|
||||
it('keeps stale human messages in failed owner retries as owner follow-up', async () => {
|
||||
const task = makeTask();
|
||||
createPairedTask(task);
|
||||
const executeTurn = vi.fn(async () => ({
|
||||
outputStatus: 'error' as const,
|
||||
deliverySucceeded: false,
|
||||
visiblePhase: 'silent',
|
||||
}));
|
||||
|
||||
const outcome = await runQueuedGroupTurn({
|
||||
chatJid: task.chat_jid,
|
||||
group: makeGroup(),
|
||||
runId: 'run-owner-retry-stale-human',
|
||||
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() } as any,
|
||||
timezone: 'UTC',
|
||||
missedMessages: [
|
||||
{
|
||||
id: 'human-owner-stale-1',
|
||||
chat_jid: task.chat_jid,
|
||||
sender: 'user@test',
|
||||
sender_name: 'User',
|
||||
content: '원 요청입니다',
|
||||
timestamp: '2026-03-30T00:00:03.000Z',
|
||||
seq: 46,
|
||||
is_bot_message: false,
|
||||
},
|
||||
],
|
||||
task,
|
||||
roleToChannel: {
|
||||
owner: null,
|
||||
reviewer: makeChannel(),
|
||||
arbiter: null,
|
||||
},
|
||||
ownerChannel: makeChannel(),
|
||||
lastAgentTimestamps: {},
|
||||
saveState: vi.fn(),
|
||||
executeTurn,
|
||||
getFixedRoleChannelName: () => 'discord-review',
|
||||
labelPairedSenders: (_chatJid, messages) => messages,
|
||||
formatMessages: () => 'formatted prompt',
|
||||
});
|
||||
|
||||
expect(outcome).toBe(false);
|
||||
expect(executeTurn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
hasHumanMessage: false,
|
||||
deliveryRole: 'owner',
|
||||
forcedRole: 'owner',
|
||||
pairedTurnIdentity: {
|
||||
turnId: 'task-owner-retry:2026-03-30T00:00:10.000Z:owner-follow-up',
|
||||
taskId: 'task-owner-retry',
|
||||
taskUpdatedAt: '2026-03-30T00:00:10.000Z',
|
||||
intentKind: 'owner-follow-up',
|
||||
role: 'owner',
|
||||
},
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -56,6 +56,33 @@ function resolveQueuedTurnReservationIntent(args: {
|
||||
return 'owner-follow-up';
|
||||
}
|
||||
|
||||
function timestampMs(value: string | null | undefined): number | null {
|
||||
if (!value) return null;
|
||||
const parsed = Date.parse(value);
|
||||
return Number.isFinite(parsed) ? parsed : null;
|
||||
}
|
||||
|
||||
function isHumanMessageFreshForTask(
|
||||
task: PairedTask | null | undefined,
|
||||
message: NewMessage,
|
||||
): boolean {
|
||||
if (!isExternalHumanMessage(message)) {
|
||||
return false;
|
||||
}
|
||||
if (!task) {
|
||||
return true;
|
||||
}
|
||||
if (task.status !== 'active' || (task.owner_failure_count ?? 0) <= 0) {
|
||||
return true;
|
||||
}
|
||||
const messageTime = timestampMs(message.timestamp);
|
||||
const taskUpdatedAt = timestampMs(task.updated_at);
|
||||
if (messageTime == null || taskUpdatedAt == null) {
|
||||
return true;
|
||||
}
|
||||
return messageTime > taskUpdatedAt;
|
||||
}
|
||||
|
||||
function buildQueuedGroupTurnPrompt(args: {
|
||||
turnRole: 'owner' | 'reviewer' | 'arbiter';
|
||||
currentTask: PairedTask | null | undefined;
|
||||
@@ -248,7 +275,9 @@ export async function runQueuedGroupTurn(args: {
|
||||
args;
|
||||
let currentTask = task;
|
||||
const hasHumanMsg = task
|
||||
? missedMessages.some(isExternalHumanMessage)
|
||||
? missedMessages.some((message) =>
|
||||
isHumanMessageFreshForTask(task, message),
|
||||
)
|
||||
: !isBotOnlyPairedRoomTurn(chatJid, missedMessages);
|
||||
let fallbackMessages = missedMessages;
|
||||
if (currentTask !== undefined && hasHumanMsg) {
|
||||
|
||||
138
src/paired-execution-context-owner-follow-up.test.ts
Normal file
138
src/paired-execution-context-owner-follow-up.test.ts
Normal file
@@ -0,0 +1,138 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./db.js', () => {
|
||||
const updatePairedTask = vi.fn();
|
||||
return {
|
||||
createPairedTask: vi.fn(),
|
||||
getLatestPairedTaskForChat: vi.fn(),
|
||||
getLatestOpenPairedTaskForChat: vi.fn(),
|
||||
getPairedTaskById: vi.fn(),
|
||||
getPairedTurnOutputs: vi.fn(() => []),
|
||||
insertPairedTurnOutput: vi.fn(),
|
||||
updatePairedTask,
|
||||
updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => {
|
||||
updatePairedTask(id, updates);
|
||||
return true;
|
||||
}),
|
||||
upsertPairedProject: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('./paired-workspace-manager.js', () => ({
|
||||
isOwnerWorkspaceRepairNeededError: vi.fn(() => false),
|
||||
markPairedTaskReviewReady: vi.fn(),
|
||||
prepareReviewerWorkspaceForExecution: vi.fn(),
|
||||
provisionOwnerWorkspaceForPairedTask: vi.fn(() => ({
|
||||
id: 'task-1:owner',
|
||||
task_id: 'task-1',
|
||||
role: 'owner',
|
||||
workspace_dir: '/tmp/paired/task-1/owner',
|
||||
snapshot_source_dir: null,
|
||||
snapshot_ref: null,
|
||||
status: 'ready',
|
||||
snapshot_refreshed_at: null,
|
||||
created_at: '2026-03-28T00:00:00.000Z',
|
||||
updated_at: '2026-03-28T00:00:00.000Z',
|
||||
})),
|
||||
}));
|
||||
|
||||
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 { preparePairedExecutionContext } from './paired-execution-context.js';
|
||||
import type { PairedTask, RegisteredGroup, RoomRoleContext } from './types.js';
|
||||
|
||||
const group: RegisteredGroup = {
|
||||
name: 'Paired Room',
|
||||
folder: 'paired-room',
|
||||
trigger: '@codex',
|
||||
added_at: '2026-03-28T00:00:00.000Z',
|
||||
agentType: 'codex',
|
||||
workDir: '/repo/canonical',
|
||||
};
|
||||
|
||||
const ownerContext: RoomRoleContext = {
|
||||
serviceId: config.CODEX_MAIN_SERVICE_ID,
|
||||
role: 'owner',
|
||||
ownerServiceId: config.CODEX_MAIN_SERVICE_ID,
|
||||
reviewerServiceId: config.REVIEWER_SERVICE_ID_FOR_TYPE,
|
||||
failoverOwner: false,
|
||||
};
|
||||
|
||||
function buildPairedTask(overrides: Partial<PairedTask> = {}): PairedTask {
|
||||
return {
|
||||
id: 'task-1',
|
||||
chat_jid: 'dc:test',
|
||||
group_folder: group.folder,
|
||||
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: 'active',
|
||||
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('paired owner follow-up preparation', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(undefined);
|
||||
});
|
||||
|
||||
it('does not reset owner failure counters for scheduled owner follow-up turns', () => {
|
||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
owner_failure_count: 1,
|
||||
owner_step_done_streak: 2,
|
||||
empty_step_done_streak: 2,
|
||||
}),
|
||||
);
|
||||
|
||||
preparePairedExecutionContext({
|
||||
group,
|
||||
chatJid: 'dc:test',
|
||||
runId: 'run-owner-follow-up-no-reset',
|
||||
roomRoleContext: ownerContext,
|
||||
hasHumanMessage: true,
|
||||
pairedTurnIdentity: {
|
||||
turnId: 'task-1:2026-03-28T00:00:00.000Z:owner-follow-up',
|
||||
taskId: 'task-1',
|
||||
taskUpdatedAt: '2026-03-28T00:00:00.000Z',
|
||||
intentKind: 'owner-follow-up',
|
||||
role: 'owner',
|
||||
},
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).not.toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -412,6 +412,24 @@ export interface PairedExecutionRecoveryPlan {
|
||||
prompt: string;
|
||||
}
|
||||
|
||||
function isOwnerContinuationTurn(
|
||||
turnIdentity: PairedTurnIdentity | undefined,
|
||||
): boolean {
|
||||
return (
|
||||
turnIdentity?.role === 'owner' && turnIdentity.intentKind !== 'owner-turn'
|
||||
);
|
||||
}
|
||||
|
||||
function shouldResetOwnerLoopCounters(args: {
|
||||
hasHumanMessage?: boolean;
|
||||
pairedTurnIdentity?: PairedTurnIdentity;
|
||||
}): boolean {
|
||||
if (args.hasHumanMessage !== true) {
|
||||
return false;
|
||||
}
|
||||
return !isOwnerContinuationTurn(args.pairedTurnIdentity);
|
||||
}
|
||||
|
||||
export function preparePairedExecutionContext(args: {
|
||||
group: RegisteredGroup;
|
||||
chatJid: string;
|
||||
@@ -458,7 +476,7 @@ export function preparePairedExecutionContext(args: {
|
||||
// merge_ready is split into a fresh task before this function runs.
|
||||
// Only reset round_trip_count when a human message is present —
|
||||
// bot-only ping-pong must accumulate the counter for loop detection.
|
||||
const hasHuman = args.hasHumanMessage === true;
|
||||
const hasHuman = shouldResetOwnerLoopCounters(args);
|
||||
const needsStatusReset =
|
||||
latestTask.status === 'review_ready' || latestTask.status === 'in_review';
|
||||
if (hasHuman || needsStatusReset) {
|
||||
|
||||
Reference in New Issue
Block a user