Extract paired task status helpers
This commit is contained in:
@@ -48,7 +48,7 @@ import {
|
||||
schedulePairedFollowUpWithMessageCheck,
|
||||
} from './message-runtime-follow-up.js';
|
||||
import { type ScheduledPairedFollowUpIntentKind } from './paired-follow-up-scheduler.js';
|
||||
import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
|
||||
import { transitionPairedTaskStatus } from './paired-task-status.js';
|
||||
import {
|
||||
Channel,
|
||||
NewMessage,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { isArbiterEnabled } from './config.js';
|
||||
import { logger } from './logger.js';
|
||||
import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
|
||||
import { transitionPairedTaskStatus } from './paired-task-status.js';
|
||||
import type { PairedTaskStatus } from './types.js';
|
||||
|
||||
export function requestArbiterOrEscalate(args: {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
|
||||
import { logger } from './logger.js';
|
||||
import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
|
||||
import { transitionPairedTaskStatus } from './paired-task-status.js';
|
||||
import { classifyArbiterVerdict } from './paired-verdict.js';
|
||||
import type { PairedTask } from './types.js';
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
|
||||
import {
|
||||
applyPairedTaskPatch,
|
||||
transitionPairedTaskStatus,
|
||||
} from './paired-execution-context-shared.js';
|
||||
} from './paired-task-status.js';
|
||||
import { resolveOwnerCompletionSignal } from './paired-completion-signals.js';
|
||||
import {
|
||||
hasCodeChangesSinceRef,
|
||||
|
||||
@@ -2,7 +2,7 @@ import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
|
||||
import { getPairedWorkspace } from './db.js';
|
||||
import { logger } from './logger.js';
|
||||
import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
|
||||
import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
|
||||
import { transitionPairedTaskStatus } from './paired-task-status.js';
|
||||
import {
|
||||
resolveReviewerCompletionSignal,
|
||||
resolveReviewerFailureSignal,
|
||||
|
||||
@@ -1,7 +1,3 @@
|
||||
import { updatePairedTaskIfUnchanged } from './db.js';
|
||||
import { logger } from './logger.js';
|
||||
import type { PairedTaskStatus } from './types.js';
|
||||
|
||||
export { requestArbiterOrEscalate } from './paired-arbiter-request.js';
|
||||
export type { RequestArbiterOrEscalateFn } from './paired-arbiter-request.js';
|
||||
export {
|
||||
@@ -19,139 +15,13 @@ export {
|
||||
hasCodeChangesSinceRef,
|
||||
resolveCanonicalSourceRef,
|
||||
} from './paired-source-ref.js';
|
||||
|
||||
const ALLOWED_PAIRED_STATUS_TRANSITIONS: Record<
|
||||
PairedTaskStatus,
|
||||
ReadonlySet<PairedTaskStatus>
|
||||
> = {
|
||||
active: new Set(['review_ready', 'arbiter_requested', 'completed']),
|
||||
review_ready: new Set([
|
||||
'active',
|
||||
'in_review',
|
||||
'arbiter_requested',
|
||||
'completed',
|
||||
]),
|
||||
in_review: new Set([
|
||||
'active',
|
||||
'review_ready',
|
||||
'merge_ready',
|
||||
'arbiter_requested',
|
||||
'completed',
|
||||
]),
|
||||
merge_ready: new Set(['active', 'arbiter_requested', 'completed']),
|
||||
completed: new Set(),
|
||||
arbiter_requested: new Set(['in_arbitration', 'completed']),
|
||||
in_arbitration: new Set(['active', 'arbiter_requested', 'completed']),
|
||||
};
|
||||
|
||||
export function assertPairedTaskStatusTransition(args: {
|
||||
currentStatus: PairedTaskStatus;
|
||||
nextStatus: PairedTaskStatus;
|
||||
}): void {
|
||||
const { currentStatus, nextStatus } = args;
|
||||
if (currentStatus === nextStatus) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (ALLOWED_PAIRED_STATUS_TRANSITIONS[currentStatus].has(nextStatus)) {
|
||||
return;
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Invalid paired task status transition: ${currentStatus} -> ${nextStatus}`,
|
||||
);
|
||||
}
|
||||
|
||||
export function transitionPairedTaskStatus(args: {
|
||||
taskId: string;
|
||||
currentStatus: PairedTaskStatus;
|
||||
nextStatus: PairedTaskStatus;
|
||||
expectedUpdatedAt: string;
|
||||
updatedAt: string;
|
||||
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;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
};
|
||||
}): boolean {
|
||||
assertPairedTaskStatusTransition({
|
||||
currentStatus: args.currentStatus,
|
||||
nextStatus: args.nextStatus,
|
||||
});
|
||||
|
||||
const updated = updatePairedTaskIfUnchanged(
|
||||
args.taskId,
|
||||
args.expectedUpdatedAt,
|
||||
{
|
||||
...args.patch,
|
||||
status: args.nextStatus,
|
||||
updated_at: args.updatedAt,
|
||||
},
|
||||
);
|
||||
if (!updated) {
|
||||
logger.warn(
|
||||
{
|
||||
taskId: args.taskId,
|
||||
currentStatus: args.currentStatus,
|
||||
nextStatus: args.nextStatus,
|
||||
expectedUpdatedAt: args.expectedUpdatedAt,
|
||||
},
|
||||
'Skipped stale paired task status transition because the task revision changed',
|
||||
);
|
||||
}
|
||||
return updated;
|
||||
}
|
||||
|
||||
export function applyPairedTaskPatch(args: {
|
||||
taskId: string;
|
||||
expectedUpdatedAt: string;
|
||||
updatedAt: string;
|
||||
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;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
status?: PairedTaskStatus;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
};
|
||||
}): boolean {
|
||||
const updated = updatePairedTaskIfUnchanged(
|
||||
args.taskId,
|
||||
args.expectedUpdatedAt,
|
||||
{
|
||||
...args.patch,
|
||||
updated_at: args.updatedAt,
|
||||
},
|
||||
);
|
||||
if (!updated) {
|
||||
logger.warn(
|
||||
{
|
||||
taskId: args.taskId,
|
||||
expectedUpdatedAt: args.expectedUpdatedAt,
|
||||
},
|
||||
'Skipped stale paired task patch because the task revision changed',
|
||||
);
|
||||
}
|
||||
return updated;
|
||||
}
|
||||
|
||||
export type TransitionPairedTaskStatusFn = typeof transitionPairedTaskStatus;
|
||||
export type ApplyPairedTaskPatchFn = typeof applyPairedTaskPatch;
|
||||
export {
|
||||
ALLOWED_PAIRED_STATUS_TRANSITIONS,
|
||||
applyPairedTaskPatch,
|
||||
assertPairedTaskStatusTransition,
|
||||
transitionPairedTaskStatus,
|
||||
} from './paired-task-status.js';
|
||||
export type {
|
||||
ApplyPairedTaskPatchFn,
|
||||
TransitionPairedTaskStatusFn,
|
||||
} from './paired-task-status.js';
|
||||
|
||||
@@ -49,7 +49,7 @@ import {
|
||||
import {
|
||||
applyPairedTaskPatch,
|
||||
transitionPairedTaskStatus,
|
||||
} from './paired-execution-context-shared.js';
|
||||
} from './paired-task-status.js';
|
||||
import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
|
||||
import { resolveCanonicalSourceRef } from './paired-source-ref.js';
|
||||
import {
|
||||
|
||||
118
src/paired-task-status.test.ts
Normal file
118
src/paired-task-status.test.ts
Normal file
@@ -0,0 +1,118 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./db.js', () => ({
|
||||
updatePairedTaskIfUnchanged: vi.fn(() => true),
|
||||
}));
|
||||
|
||||
vi.mock('./logger.js', () => ({
|
||||
logger: {
|
||||
warn: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { updatePairedTaskIfUnchanged } from './db.js';
|
||||
import { logger } from './logger.js';
|
||||
import {
|
||||
applyPairedTaskPatch,
|
||||
assertPairedTaskStatusTransition,
|
||||
transitionPairedTaskStatus,
|
||||
} from './paired-task-status.js';
|
||||
|
||||
describe('paired task status transitions', () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(updatePairedTaskIfUnchanged).mockReset();
|
||||
vi.mocked(updatePairedTaskIfUnchanged).mockReturnValue(true);
|
||||
vi.mocked(logger.warn).mockReset();
|
||||
});
|
||||
|
||||
it('allows valid transitions and same-status no-ops', () => {
|
||||
expect(() =>
|
||||
assertPairedTaskStatusTransition({
|
||||
currentStatus: 'active',
|
||||
nextStatus: 'review_ready',
|
||||
}),
|
||||
).not.toThrow();
|
||||
expect(() =>
|
||||
assertPairedTaskStatusTransition({
|
||||
currentStatus: 'review_ready',
|
||||
nextStatus: 'review_ready',
|
||||
}),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('rejects invalid transitions', () => {
|
||||
expect(() =>
|
||||
assertPairedTaskStatusTransition({
|
||||
currentStatus: 'completed',
|
||||
nextStatus: 'active',
|
||||
}),
|
||||
).toThrow('Invalid paired task status transition: completed -> active');
|
||||
});
|
||||
|
||||
it('writes status transitions with optimistic revision guard', () => {
|
||||
const updated = transitionPairedTaskStatus({
|
||||
taskId: 'task-1',
|
||||
currentStatus: 'active',
|
||||
nextStatus: 'review_ready',
|
||||
expectedUpdatedAt: '2026-04-29T01:00:00.000Z',
|
||||
updatedAt: '2026-04-29T01:01:00.000Z',
|
||||
patch: {
|
||||
review_requested_at: '2026-04-29T01:01:00.000Z',
|
||||
},
|
||||
});
|
||||
|
||||
expect(updated).toBe(true);
|
||||
expect(updatePairedTaskIfUnchanged).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
'2026-04-29T01:00:00.000Z',
|
||||
{
|
||||
review_requested_at: '2026-04-29T01:01:00.000Z',
|
||||
status: 'review_ready',
|
||||
updated_at: '2026-04-29T01:01:00.000Z',
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('logs stale status transitions without writing success', () => {
|
||||
vi.mocked(updatePairedTaskIfUnchanged).mockReturnValue(false);
|
||||
|
||||
const updated = transitionPairedTaskStatus({
|
||||
taskId: 'task-1',
|
||||
currentStatus: 'active',
|
||||
nextStatus: 'completed',
|
||||
expectedUpdatedAt: 'old',
|
||||
updatedAt: 'new',
|
||||
patch: {
|
||||
completion_reason: 'done',
|
||||
},
|
||||
});
|
||||
|
||||
expect(updated).toBe(false);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
{
|
||||
taskId: 'task-1',
|
||||
currentStatus: 'active',
|
||||
nextStatus: 'completed',
|
||||
expectedUpdatedAt: 'old',
|
||||
},
|
||||
'Skipped stale paired task status transition because the task revision changed',
|
||||
);
|
||||
});
|
||||
|
||||
it('applies non-status patches with optimistic revision guard', () => {
|
||||
const updated = applyPairedTaskPatch({
|
||||
taskId: 'task-1',
|
||||
expectedUpdatedAt: 'old',
|
||||
updatedAt: 'new',
|
||||
patch: {
|
||||
owner_failure_count: 2,
|
||||
},
|
||||
});
|
||||
|
||||
expect(updated).toBe(true);
|
||||
expect(updatePairedTaskIfUnchanged).toHaveBeenCalledWith('task-1', 'old', {
|
||||
owner_failure_count: 2,
|
||||
updated_at: 'new',
|
||||
});
|
||||
});
|
||||
});
|
||||
139
src/paired-task-status.ts
Normal file
139
src/paired-task-status.ts
Normal file
@@ -0,0 +1,139 @@
|
||||
import { updatePairedTaskIfUnchanged } from './db.js';
|
||||
import { logger } from './logger.js';
|
||||
import type { PairedTaskStatus } from './types.js';
|
||||
|
||||
export const ALLOWED_PAIRED_STATUS_TRANSITIONS: Record<
|
||||
PairedTaskStatus,
|
||||
ReadonlySet<PairedTaskStatus>
|
||||
> = {
|
||||
active: new Set(['review_ready', 'arbiter_requested', 'completed']),
|
||||
review_ready: new Set([
|
||||
'active',
|
||||
'in_review',
|
||||
'arbiter_requested',
|
||||
'completed',
|
||||
]),
|
||||
in_review: new Set([
|
||||
'active',
|
||||
'review_ready',
|
||||
'merge_ready',
|
||||
'arbiter_requested',
|
||||
'completed',
|
||||
]),
|
||||
merge_ready: new Set(['active', 'arbiter_requested', 'completed']),
|
||||
completed: new Set(),
|
||||
arbiter_requested: new Set(['in_arbitration', 'completed']),
|
||||
in_arbitration: new Set(['active', 'arbiter_requested', 'completed']),
|
||||
};
|
||||
|
||||
export function assertPairedTaskStatusTransition(args: {
|
||||
currentStatus: PairedTaskStatus;
|
||||
nextStatus: PairedTaskStatus;
|
||||
}): void {
|
||||
const { currentStatus, nextStatus } = args;
|
||||
if (currentStatus === nextStatus) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (ALLOWED_PAIRED_STATUS_TRANSITIONS[currentStatus].has(nextStatus)) {
|
||||
return;
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Invalid paired task status transition: ${currentStatus} -> ${nextStatus}`,
|
||||
);
|
||||
}
|
||||
|
||||
export function transitionPairedTaskStatus(args: {
|
||||
taskId: string;
|
||||
currentStatus: PairedTaskStatus;
|
||||
nextStatus: PairedTaskStatus;
|
||||
expectedUpdatedAt: string;
|
||||
updatedAt: string;
|
||||
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;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
};
|
||||
}): boolean {
|
||||
assertPairedTaskStatusTransition({
|
||||
currentStatus: args.currentStatus,
|
||||
nextStatus: args.nextStatus,
|
||||
});
|
||||
|
||||
const updated = updatePairedTaskIfUnchanged(
|
||||
args.taskId,
|
||||
args.expectedUpdatedAt,
|
||||
{
|
||||
...args.patch,
|
||||
status: args.nextStatus,
|
||||
updated_at: args.updatedAt,
|
||||
},
|
||||
);
|
||||
if (!updated) {
|
||||
logger.warn(
|
||||
{
|
||||
taskId: args.taskId,
|
||||
currentStatus: args.currentStatus,
|
||||
nextStatus: args.nextStatus,
|
||||
expectedUpdatedAt: args.expectedUpdatedAt,
|
||||
},
|
||||
'Skipped stale paired task status transition because the task revision changed',
|
||||
);
|
||||
}
|
||||
return updated;
|
||||
}
|
||||
|
||||
export function applyPairedTaskPatch(args: {
|
||||
taskId: string;
|
||||
expectedUpdatedAt: string;
|
||||
updatedAt: string;
|
||||
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;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
status?: PairedTaskStatus;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
};
|
||||
}): boolean {
|
||||
const updated = updatePairedTaskIfUnchanged(
|
||||
args.taskId,
|
||||
args.expectedUpdatedAt,
|
||||
{
|
||||
...args.patch,
|
||||
updated_at: args.updatedAt,
|
||||
},
|
||||
);
|
||||
if (!updated) {
|
||||
logger.warn(
|
||||
{
|
||||
taskId: args.taskId,
|
||||
expectedUpdatedAt: args.expectedUpdatedAt,
|
||||
},
|
||||
'Skipped stale paired task patch because the task revision changed',
|
||||
);
|
||||
}
|
||||
return updated;
|
||||
}
|
||||
|
||||
export type TransitionPairedTaskStatusFn = typeof transitionPairedTaskStatus;
|
||||
export type ApplyPairedTaskPatchFn = typeof applyPairedTaskPatch;
|
||||
@@ -12,7 +12,7 @@ import {
|
||||
} from './db.js';
|
||||
import { resolvePairedTaskWorkspacePath } from './group-folder.js';
|
||||
import { logger } from './logger.js';
|
||||
import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
|
||||
import { transitionPairedTaskStatus } from './paired-task-status.js';
|
||||
import type { PairedTask, PairedWorkspace } from './types.js';
|
||||
import { ensureWorkspaceDependenciesInstalled } from './workspace-package-manager.js';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user