feat: add paired execution freshness guard
This commit is contained in:
@@ -8,6 +8,7 @@ import {
|
|||||||
_initTestDatabase,
|
_initTestDatabase,
|
||||||
_initTestDatabaseFromFile,
|
_initTestDatabaseFromFile,
|
||||||
applyPairedEvent,
|
applyPairedEvent,
|
||||||
|
cancelSupersededPairedExecutions,
|
||||||
claimServiceHandoff,
|
claimServiceHandoff,
|
||||||
completeServiceHandoffAndAdvanceTargetCursor,
|
completeServiceHandoffAndAdvanceTargetCursor,
|
||||||
createPairedApproval,
|
createPairedApproval,
|
||||||
@@ -42,6 +43,7 @@ import {
|
|||||||
listPairedApprovalsForTask,
|
listPairedApprovalsForTask,
|
||||||
listPairedArtifactsForTask,
|
listPairedArtifactsForTask,
|
||||||
listPairedEventsForTask,
|
listPairedEventsForTask,
|
||||||
|
listPairedExecutionsForTask,
|
||||||
listPairedWorkspacesForTask,
|
listPairedWorkspacesForTask,
|
||||||
markWorkItemDelivered,
|
markWorkItemDelivered,
|
||||||
markWorkItemDeliveryRetry,
|
markWorkItemDeliveryRetry,
|
||||||
@@ -1003,6 +1005,71 @@ describe('paired task state', () => {
|
|||||||
),
|
),
|
||||||
).toEqual(['/tmp/reviewer-v2']);
|
).toEqual(['/tmp/reviewer-v2']);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('persists execution checkpoints and cancels only superseded running executions', () => {
|
||||||
|
createPairedTask({
|
||||||
|
id: 'paired-task-3',
|
||||||
|
chat_jid: 'dc:paired',
|
||||||
|
group_folder: 'paired-room',
|
||||||
|
owner_service_id: 'codex-main',
|
||||||
|
reviewer_service_id: 'codex-review',
|
||||||
|
title: null,
|
||||||
|
source_ref: 'HEAD',
|
||||||
|
task_policy: 'autonomous',
|
||||||
|
risk_level: 'low',
|
||||||
|
plan_status: 'approved',
|
||||||
|
review_requested_at: null,
|
||||||
|
status: 'active',
|
||||||
|
created_at: '2026-03-29T01:00:00.000Z',
|
||||||
|
updated_at: '2026-03-29T01:00:00.000Z',
|
||||||
|
});
|
||||||
|
createPairedExecution({
|
||||||
|
id: 'paired-exec-3a',
|
||||||
|
task_id: 'paired-task-3',
|
||||||
|
service_id: 'codex-review',
|
||||||
|
role: 'reviewer',
|
||||||
|
workspace_id: 'paired-task-3:reviewer',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-v1',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-29T01:01:00.000Z',
|
||||||
|
started_at: '2026-03-29T01:01:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
});
|
||||||
|
createPairedExecution({
|
||||||
|
id: 'paired-exec-3b',
|
||||||
|
task_id: 'paired-task-3',
|
||||||
|
service_id: 'codex-review',
|
||||||
|
role: 'reviewer',
|
||||||
|
workspace_id: 'paired-task-3:reviewer',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-v2',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-29T01:02:00.000Z',
|
||||||
|
started_at: '2026-03-29T01:02:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
});
|
||||||
|
|
||||||
|
const cancelled = cancelSupersededPairedExecutions({
|
||||||
|
taskId: 'paired-task-3',
|
||||||
|
role: 'reviewer',
|
||||||
|
exceptExecutionId: 'paired-exec-3b',
|
||||||
|
note: 'Superseded by a newer review checkpoint.',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(cancelled).toBe(1);
|
||||||
|
expect(getPairedExecutionById('paired-exec-3a')).toMatchObject({
|
||||||
|
status: 'cancelled',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-v1',
|
||||||
|
});
|
||||||
|
expect(getPairedExecutionById('paired-exec-3b')).toMatchObject({
|
||||||
|
status: 'running',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-v2',
|
||||||
|
});
|
||||||
|
expect(
|
||||||
|
listPairedExecutionsForTask('paired-task-3').map((execution) => execution.id),
|
||||||
|
).toEqual(['paired-exec-3a', 'paired-exec-3b']);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// --- LIMIT behavior ---
|
// --- LIMIT behavior ---
|
||||||
|
|||||||
63
src/db.ts
63
src/db.ts
@@ -295,6 +295,7 @@ function createSchema(database: Database.Database): void {
|
|||||||
service_id TEXT NOT NULL,
|
service_id TEXT NOT NULL,
|
||||||
role TEXT NOT NULL,
|
role TEXT NOT NULL,
|
||||||
workspace_id TEXT,
|
workspace_id TEXT,
|
||||||
|
checkpoint_fingerprint TEXT,
|
||||||
status TEXT NOT NULL DEFAULT 'pending',
|
status TEXT NOT NULL DEFAULT 'pending',
|
||||||
summary TEXT,
|
summary TEXT,
|
||||||
created_at TEXT NOT NULL,
|
created_at TEXT NOT NULL,
|
||||||
@@ -478,6 +479,14 @@ function createSchema(database: Database.Database): void {
|
|||||||
/* column already exists */
|
/* column already exists */
|
||||||
}
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
database.exec(
|
||||||
|
`ALTER TABLE paired_executions ADD COLUMN checkpoint_fingerprint TEXT`,
|
||||||
|
);
|
||||||
|
} catch {
|
||||||
|
/* column already exists */
|
||||||
|
}
|
||||||
|
|
||||||
database.exec(`
|
database.exec(`
|
||||||
UPDATE scheduled_tasks
|
UPDATE scheduled_tasks
|
||||||
SET agent_type = COALESCE(
|
SET agent_type = COALESCE(
|
||||||
@@ -2149,13 +2158,14 @@ export function createPairedExecution(execution: PairedExecution): void {
|
|||||||
service_id,
|
service_id,
|
||||||
role,
|
role,
|
||||||
workspace_id,
|
workspace_id,
|
||||||
|
checkpoint_fingerprint,
|
||||||
status,
|
status,
|
||||||
summary,
|
summary,
|
||||||
created_at,
|
created_at,
|
||||||
started_at,
|
started_at,
|
||||||
completed_at
|
completed_at
|
||||||
)
|
)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||||
`,
|
`,
|
||||||
).run(
|
).run(
|
||||||
execution.id,
|
execution.id,
|
||||||
@@ -2163,6 +2173,7 @@ export function createPairedExecution(execution: PairedExecution): void {
|
|||||||
execution.service_id,
|
execution.service_id,
|
||||||
execution.role,
|
execution.role,
|
||||||
execution.workspace_id,
|
execution.workspace_id,
|
||||||
|
execution.checkpoint_fingerprint ?? null,
|
||||||
execution.status,
|
execution.status,
|
||||||
execution.summary,
|
execution.summary,
|
||||||
execution.created_at,
|
execution.created_at,
|
||||||
@@ -2184,7 +2195,12 @@ export function updatePairedExecution(
|
|||||||
updates: Partial<
|
updates: Partial<
|
||||||
Pick<
|
Pick<
|
||||||
PairedExecution,
|
PairedExecution,
|
||||||
'workspace_id' | 'status' | 'summary' | 'started_at' | 'completed_at'
|
| 'workspace_id'
|
||||||
|
| 'checkpoint_fingerprint'
|
||||||
|
| 'status'
|
||||||
|
| 'summary'
|
||||||
|
| 'started_at'
|
||||||
|
| 'completed_at'
|
||||||
>
|
>
|
||||||
>,
|
>,
|
||||||
): void {
|
): void {
|
||||||
@@ -2195,6 +2211,10 @@ export function updatePairedExecution(
|
|||||||
fields.push('workspace_id = ?');
|
fields.push('workspace_id = ?');
|
||||||
values.push(updates.workspace_id);
|
values.push(updates.workspace_id);
|
||||||
}
|
}
|
||||||
|
if (updates.checkpoint_fingerprint !== undefined) {
|
||||||
|
fields.push('checkpoint_fingerprint = ?');
|
||||||
|
values.push(updates.checkpoint_fingerprint);
|
||||||
|
}
|
||||||
if (updates.status !== undefined) {
|
if (updates.status !== undefined) {
|
||||||
fields.push('status = ?');
|
fields.push('status = ?');
|
||||||
values.push(updates.status);
|
values.push(updates.status);
|
||||||
@@ -2275,6 +2295,45 @@ export function listPairedWorkspacesForTask(taskId: string): PairedWorkspace[] {
|
|||||||
.all(taskId) as PairedWorkspace[];
|
.all(taskId) as PairedWorkspace[];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function listPairedExecutionsForTask(taskId: string): PairedExecution[] {
|
||||||
|
return db
|
||||||
|
.prepare(
|
||||||
|
'SELECT * FROM paired_executions WHERE task_id = ? ORDER BY created_at, id',
|
||||||
|
)
|
||||||
|
.all(taskId) as PairedExecution[];
|
||||||
|
}
|
||||||
|
|
||||||
|
export function cancelSupersededPairedExecutions(args: {
|
||||||
|
taskId: string;
|
||||||
|
role: PairedExecution['role'];
|
||||||
|
exceptExecutionId?: string;
|
||||||
|
note?: string | null;
|
||||||
|
}): number {
|
||||||
|
const now = new Date().toISOString();
|
||||||
|
const result = db
|
||||||
|
.prepare(
|
||||||
|
`
|
||||||
|
UPDATE paired_executions
|
||||||
|
SET status = 'cancelled',
|
||||||
|
summary = COALESCE(summary, ?),
|
||||||
|
completed_at = COALESCE(completed_at, ?)
|
||||||
|
WHERE task_id = ?
|
||||||
|
AND role = ?
|
||||||
|
AND status = 'running'
|
||||||
|
AND (? IS NULL OR id <> ?)
|
||||||
|
`,
|
||||||
|
)
|
||||||
|
.run(
|
||||||
|
args.note ?? null,
|
||||||
|
now,
|
||||||
|
args.taskId,
|
||||||
|
args.role,
|
||||||
|
args.exceptExecutionId ?? null,
|
||||||
|
args.exceptExecutionId ?? null,
|
||||||
|
);
|
||||||
|
return result.changes;
|
||||||
|
}
|
||||||
|
|
||||||
export function createPairedApproval(
|
export function createPairedApproval(
|
||||||
approval: Omit<PairedApproval, 'id'>,
|
approval: Omit<PairedApproval, 'id'>,
|
||||||
): number {
|
): number {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
|
|||||||
|
|
||||||
vi.mock('./db.js', () => ({
|
vi.mock('./db.js', () => ({
|
||||||
applyPairedEvent: vi.fn(),
|
applyPairedEvent: vi.fn(),
|
||||||
|
cancelSupersededPairedExecutions: vi.fn(() => 0),
|
||||||
createPairedArtifact: vi.fn(),
|
createPairedArtifact: vi.fn(),
|
||||||
createPairedExecution: vi.fn(),
|
createPairedExecution: vi.fn(),
|
||||||
createPairedTask: vi.fn(),
|
createPairedTask: vi.fn(),
|
||||||
@@ -11,6 +12,7 @@ vi.mock('./db.js', () => ({
|
|||||||
getPairedWorkspace: vi.fn(),
|
getPairedWorkspace: vi.fn(),
|
||||||
listPairedArtifactsForTask: vi.fn(),
|
listPairedArtifactsForTask: vi.fn(),
|
||||||
listPairedEventsForTask: vi.fn(),
|
listPairedEventsForTask: vi.fn(),
|
||||||
|
listPairedExecutionsForTask: vi.fn(),
|
||||||
updatePairedExecution: vi.fn(),
|
updatePairedExecution: vi.fn(),
|
||||||
updatePairedTask: vi.fn(),
|
updatePairedTask: vi.fn(),
|
||||||
upsertPairedProject: vi.fn(),
|
upsertPairedProject: vi.fn(),
|
||||||
@@ -79,6 +81,7 @@ async function importExecutionContextForService(serviceId: string) {
|
|||||||
|
|
||||||
const dbModule = {
|
const dbModule = {
|
||||||
applyPairedEvent: vi.fn(),
|
applyPairedEvent: vi.fn(),
|
||||||
|
cancelSupersededPairedExecutions: vi.fn(() => 0),
|
||||||
createPairedArtifact: vi.fn(),
|
createPairedArtifact: vi.fn(),
|
||||||
createPairedExecution: vi.fn(),
|
createPairedExecution: vi.fn(),
|
||||||
createPairedTask: vi.fn(),
|
createPairedTask: vi.fn(),
|
||||||
@@ -88,6 +91,7 @@ async function importExecutionContextForService(serviceId: string) {
|
|||||||
getPairedWorkspace: vi.fn(),
|
getPairedWorkspace: vi.fn(),
|
||||||
listPairedArtifactsForTask: vi.fn(),
|
listPairedArtifactsForTask: vi.fn(),
|
||||||
listPairedEventsForTask: vi.fn(),
|
listPairedEventsForTask: vi.fn(),
|
||||||
|
listPairedExecutionsForTask: vi.fn(),
|
||||||
updatePairedExecution: vi.fn(),
|
updatePairedExecution: vi.fn(),
|
||||||
updatePairedTask: vi.fn(),
|
updatePairedTask: vi.fn(),
|
||||||
upsertPairedProject: vi.fn(),
|
upsertPairedProject: vi.fn(),
|
||||||
@@ -103,6 +107,7 @@ async function importExecutionContextForService(serviceId: string) {
|
|||||||
dbModule.getPairedWorkspace.mockReturnValue(undefined);
|
dbModule.getPairedWorkspace.mockReturnValue(undefined);
|
||||||
dbModule.listPairedArtifactsForTask.mockReturnValue([]);
|
dbModule.listPairedArtifactsForTask.mockReturnValue([]);
|
||||||
dbModule.listPairedEventsForTask.mockReturnValue([]);
|
dbModule.listPairedEventsForTask.mockReturnValue([]);
|
||||||
|
dbModule.listPairedExecutionsForTask.mockReturnValue([]);
|
||||||
|
|
||||||
const pairedWorkspaceManagerModule = {
|
const pairedWorkspaceManagerModule = {
|
||||||
PLAN_REVIEW_REQUIRED_BLOCK_MESSAGE:
|
PLAN_REVIEW_REQUIRED_BLOCK_MESSAGE:
|
||||||
@@ -175,12 +180,14 @@ describe('paired execution context', () => {
|
|||||||
event: { id: 1, ...event },
|
event: { id: 1, ...event },
|
||||||
result: onApply ? onApply() : null,
|
result: onApply ? onApply() : null,
|
||||||
}));
|
}));
|
||||||
|
vi.mocked(db.cancelSupersededPairedExecutions).mockReturnValue(0);
|
||||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(undefined);
|
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(undefined);
|
||||||
vi.mocked(db.getPairedExecutionById).mockReturnValue(undefined);
|
vi.mocked(db.getPairedExecutionById).mockReturnValue(undefined);
|
||||||
vi.mocked(db.getPairedTaskById).mockReturnValue(undefined);
|
vi.mocked(db.getPairedTaskById).mockReturnValue(undefined);
|
||||||
vi.mocked(db.getPairedWorkspace).mockReturnValue(undefined);
|
vi.mocked(db.getPairedWorkspace).mockReturnValue(undefined);
|
||||||
vi.mocked(db.listPairedArtifactsForTask).mockReturnValue([]);
|
vi.mocked(db.listPairedArtifactsForTask).mockReturnValue([]);
|
||||||
vi.mocked(db.listPairedEventsForTask).mockReturnValue([]);
|
vi.mocked(db.listPairedEventsForTask).mockReturnValue([]);
|
||||||
|
vi.mocked(db.listPairedExecutionsForTask).mockReturnValue([]);
|
||||||
vi.mocked(
|
vi.mocked(
|
||||||
pairedWorkspaceManager.provisionOwnerWorkspaceForPairedTask,
|
pairedWorkspaceManager.provisionOwnerWorkspaceForPairedTask,
|
||||||
).mockReturnValue({
|
).mockReturnValue({
|
||||||
@@ -240,13 +247,63 @@ describe('paired execution context', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('auto-requests review when a low-risk owner execution completes', () => {
|
it('does not self-cancel when the same execution resumes', () => {
|
||||||
|
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({
|
||||||
|
id: 'task-1',
|
||||||
|
chat_jid: 'dc:test',
|
||||||
|
group_folder: group.folder,
|
||||||
|
owner_service_id: 'codex-main',
|
||||||
|
reviewer_service_id: 'codex-review',
|
||||||
|
title: null,
|
||||||
|
source_ref: 'HEAD',
|
||||||
|
task_policy: 'autonomous',
|
||||||
|
risk_level: 'low',
|
||||||
|
plan_status: 'not_requested',
|
||||||
|
review_requested_at: null,
|
||||||
|
status: 'active',
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
updated_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
});
|
||||||
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
||||||
id: 'run-1:codex-main',
|
id: 'run-1:codex-main',
|
||||||
task_id: 'task-1',
|
task_id: 'task-1',
|
||||||
service_id: 'codex-main',
|
service_id: 'codex-main',
|
||||||
role: 'owner',
|
role: 'owner',
|
||||||
workspace_id: 'task-1:owner',
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
});
|
||||||
|
|
||||||
|
preparePairedExecutionContext({
|
||||||
|
group,
|
||||||
|
chatJid: 'dc:test',
|
||||||
|
runId: 'run-1',
|
||||||
|
roomRoleContext: ownerContext,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(db.cancelSupersededPairedExecutions).toHaveBeenCalledWith({
|
||||||
|
taskId: 'task-1',
|
||||||
|
role: 'owner',
|
||||||
|
exceptExecutionId: 'run-1:codex-main',
|
||||||
|
note: 'Superseded by a newer execution for the same task and role.',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('auto-requests review when a low-risk owner execution completes', () => {
|
||||||
|
vi.mocked(pairedWorkspaceManager.resolvePairedTaskSourceFingerprint).mockReturnValue(
|
||||||
|
'fingerprint-2',
|
||||||
|
);
|
||||||
|
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
||||||
|
id: 'run-1:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
status: 'running',
|
status: 'running',
|
||||||
summary: null,
|
summary: null,
|
||||||
created_at: '2026-03-28T00:00:00.000Z',
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
@@ -331,8 +388,8 @@ describe('paired execution context', () => {
|
|||||||
event: expect.objectContaining({
|
event: expect.objectContaining({
|
||||||
task_id: 'task-1',
|
task_id: 'task-1',
|
||||||
event_type: 'request_review',
|
event_type: 'request_review',
|
||||||
source_fingerprint: 'fingerprint-1',
|
source_fingerprint: 'fingerprint-2',
|
||||||
dedupe_key: 'auto-request-review:fingerprint-1',
|
dedupe_key: 'auto-request-review:fingerprint-2',
|
||||||
}),
|
}),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
@@ -553,6 +610,157 @@ describe('paired execution context', () => {
|
|||||||
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not reopen review from a cancelled stale owner execution', () => {
|
||||||
|
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
||||||
|
id: 'run-1:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
|
status: 'cancelled',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
});
|
||||||
|
|
||||||
|
completePairedExecutionContext({
|
||||||
|
executionId: 'run-1:codex-main',
|
||||||
|
status: 'succeeded',
|
||||||
|
summary: 'stale',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(db.updatePairedExecution).toHaveBeenCalledWith(
|
||||||
|
'run-1:codex-main',
|
||||||
|
expect.objectContaining({
|
||||||
|
status: 'cancelled',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(db.applyPairedEvent).not.toHaveBeenCalled();
|
||||||
|
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not reopen review from an older owner execution when a newer owner execution already failed', () => {
|
||||||
|
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
||||||
|
id: 'run-1:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
});
|
||||||
|
vi.mocked(db.listPairedExecutionsForTask).mockReturnValue([
|
||||||
|
{
|
||||||
|
id: 'run-1:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'run-2:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-2',
|
||||||
|
status: 'failed',
|
||||||
|
summary: 'newer',
|
||||||
|
created_at: '2026-03-28T00:01:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:01:00.000Z',
|
||||||
|
completed_at: '2026-03-28T00:02:00.000Z',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
completePairedExecutionContext({
|
||||||
|
executionId: 'run-1:codex-main',
|
||||||
|
status: 'succeeded',
|
||||||
|
summary: 'stale-after-failed',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(db.updatePairedExecution).toHaveBeenCalledWith(
|
||||||
|
'run-1:codex-main',
|
||||||
|
expect.objectContaining({
|
||||||
|
status: 'succeeded',
|
||||||
|
summary: 'stale-after-failed',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(db.applyPairedEvent).not.toHaveBeenCalled();
|
||||||
|
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not reopen review from an older owner execution when a newer owner execution already succeeded', () => {
|
||||||
|
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
||||||
|
id: 'run-1:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
});
|
||||||
|
vi.mocked(db.listPairedExecutionsForTask).mockReturnValue([
|
||||||
|
{
|
||||||
|
id: 'run-1:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-1',
|
||||||
|
status: 'running',
|
||||||
|
summary: null,
|
||||||
|
created_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:00:00.000Z',
|
||||||
|
completed_at: null,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'run-2:codex-main',
|
||||||
|
task_id: 'task-1',
|
||||||
|
service_id: 'codex-main',
|
||||||
|
role: 'owner',
|
||||||
|
workspace_id: 'task-1:owner',
|
||||||
|
checkpoint_fingerprint: 'fingerprint-2',
|
||||||
|
status: 'succeeded',
|
||||||
|
summary: 'newer',
|
||||||
|
created_at: '2026-03-28T00:01:00.000Z',
|
||||||
|
started_at: '2026-03-28T00:01:00.000Z',
|
||||||
|
completed_at: '2026-03-28T00:02:00.000Z',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
completePairedExecutionContext({
|
||||||
|
executionId: 'run-1:codex-main',
|
||||||
|
status: 'succeeded',
|
||||||
|
summary: 'stale-after-succeeded',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(db.updatePairedExecution).toHaveBeenCalledWith(
|
||||||
|
'run-1:codex-main',
|
||||||
|
expect.objectContaining({
|
||||||
|
status: 'succeeded',
|
||||||
|
summary: 'stale-after-succeeded',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(db.applyPairedEvent).not.toHaveBeenCalled();
|
||||||
|
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it('records a new checkpoint and returns the task to review_pending when owner changes arrive during in_review', () => {
|
it('records a new checkpoint and returns the task to review_pending when owner changes arrive during in_review', () => {
|
||||||
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
vi.mocked(db.getPairedExecutionById).mockReturnValue({
|
||||||
id: 'run-1:codex-main',
|
id: 'run-1:codex-main',
|
||||||
@@ -621,6 +829,11 @@ describe('paired execution context', () => {
|
|||||||
review_requested_at: expect.any(String),
|
review_requested_at: expect.any(String),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
expect(db.cancelSupersededPairedExecutions).toHaveBeenCalledWith({
|
||||||
|
taskId: 'task-1',
|
||||||
|
role: 'reviewer',
|
||||||
|
note: 'Superseded by a newer review checkpoint.',
|
||||||
|
});
|
||||||
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
expect(pairedWorkspaceManager.markPairedTaskReviewReady).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import crypto from 'crypto';
|
|||||||
import { SERVICE_ID, normalizeServiceId } from './config.js';
|
import { SERVICE_ID, normalizeServiceId } from './config.js';
|
||||||
import {
|
import {
|
||||||
applyPairedEvent,
|
applyPairedEvent,
|
||||||
|
cancelSupersededPairedExecutions,
|
||||||
createPairedArtifact,
|
createPairedArtifact,
|
||||||
createPairedExecution,
|
createPairedExecution,
|
||||||
createPairedTask,
|
createPairedTask,
|
||||||
@@ -13,6 +14,7 @@ import {
|
|||||||
getPairedWorkspace,
|
getPairedWorkspace,
|
||||||
listPairedArtifactsForTask,
|
listPairedArtifactsForTask,
|
||||||
listPairedEventsForTask,
|
listPairedEventsForTask,
|
||||||
|
listPairedExecutionsForTask,
|
||||||
updatePairedExecution,
|
updatePairedExecution,
|
||||||
updatePairedTask,
|
updatePairedTask,
|
||||||
upsertPairedProject,
|
upsertPairedProject,
|
||||||
@@ -119,20 +121,32 @@ function ensureExecutionRecord(args: {
|
|||||||
roomRoleContext: RoomRoleContext;
|
roomRoleContext: RoomRoleContext;
|
||||||
task: PairedTask;
|
task: PairedTask;
|
||||||
workspace?: PairedWorkspace;
|
workspace?: PairedWorkspace;
|
||||||
|
checkpointFingerprint?: string | null;
|
||||||
}): PairedExecution {
|
}): PairedExecution {
|
||||||
const executionId = `${args.runId}:${args.roomRoleContext.serviceId}`;
|
const executionId = `${args.runId}:${args.roomRoleContext.serviceId}`;
|
||||||
const existing = getPairedExecutionById(executionId);
|
const existing = getPairedExecutionById(executionId);
|
||||||
const now = new Date().toISOString();
|
const now = new Date().toISOString();
|
||||||
|
|
||||||
|
cancelSupersededPairedExecutions({
|
||||||
|
taskId: args.task.id,
|
||||||
|
role: args.roomRoleContext.role,
|
||||||
|
exceptExecutionId: executionId,
|
||||||
|
note: 'Superseded by a newer execution for the same task and role.',
|
||||||
|
});
|
||||||
|
|
||||||
if (existing) {
|
if (existing) {
|
||||||
updatePairedExecution(existing.id, {
|
updatePairedExecution(existing.id, {
|
||||||
workspace_id: args.workspace?.id ?? existing.workspace_id,
|
workspace_id: args.workspace?.id ?? existing.workspace_id,
|
||||||
|
checkpoint_fingerprint:
|
||||||
|
args.checkpointFingerprint ?? existing.checkpoint_fingerprint ?? null,
|
||||||
status: 'running',
|
status: 'running',
|
||||||
started_at: existing.started_at ?? now,
|
started_at: existing.started_at ?? now,
|
||||||
});
|
});
|
||||||
return {
|
return {
|
||||||
...existing,
|
...existing,
|
||||||
workspace_id: args.workspace?.id ?? existing.workspace_id,
|
workspace_id: args.workspace?.id ?? existing.workspace_id,
|
||||||
|
checkpoint_fingerprint:
|
||||||
|
args.checkpointFingerprint ?? existing.checkpoint_fingerprint ?? null,
|
||||||
status: 'running',
|
status: 'running',
|
||||||
started_at: existing.started_at ?? now,
|
started_at: existing.started_at ?? now,
|
||||||
};
|
};
|
||||||
@@ -144,6 +158,7 @@ function ensureExecutionRecord(args: {
|
|||||||
service_id: args.roomRoleContext.serviceId,
|
service_id: args.roomRoleContext.serviceId,
|
||||||
role: args.roomRoleContext.role,
|
role: args.roomRoleContext.role,
|
||||||
workspace_id: args.workspace?.id ?? null,
|
workspace_id: args.workspace?.id ?? null,
|
||||||
|
checkpoint_fingerprint: args.checkpointFingerprint ?? null,
|
||||||
status: 'running',
|
status: 'running',
|
||||||
summary: null,
|
summary: null,
|
||||||
created_at: now,
|
created_at: now,
|
||||||
@@ -154,6 +169,90 @@ function ensureExecutionRecord(args: {
|
|||||||
return execution;
|
return execution;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getLatestReviewCheckpointFingerprint(taskId: string): string | null {
|
||||||
|
const events = listPairedEventsForTask(taskId);
|
||||||
|
for (let index = events.length - 1; index >= 0; index -= 1) {
|
||||||
|
const event = events[index];
|
||||||
|
if (
|
||||||
|
event?.event_type === 'request_review' &&
|
||||||
|
event.source_fingerprint?.trim()
|
||||||
|
) {
|
||||||
|
return event.source_fingerprint.trim();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return getPairedWorkspace(taskId, 'reviewer')?.snapshot_source_fingerprint ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function resolveExecutionCheckpointFingerprint(args: {
|
||||||
|
taskId: string;
|
||||||
|
role: RoomRoleContext['role'];
|
||||||
|
workspace: PairedWorkspace | null;
|
||||||
|
}): string | null {
|
||||||
|
if (args.role === 'reviewer') {
|
||||||
|
return (
|
||||||
|
args.workspace?.snapshot_source_fingerprint ??
|
||||||
|
getLatestReviewCheckpointFingerprint(args.taskId)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return resolvePairedTaskSourceFingerprint(args.taskId);
|
||||||
|
}
|
||||||
|
|
||||||
|
function isExecutionFreshForTaskStateWrite(args: {
|
||||||
|
execution: PairedExecution;
|
||||||
|
}): boolean {
|
||||||
|
if (args.execution.status === 'cancelled') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (args.execution.role === 'owner') {
|
||||||
|
const executionOrderKey = [
|
||||||
|
args.execution.started_at ?? args.execution.created_at,
|
||||||
|
args.execution.created_at,
|
||||||
|
args.execution.id,
|
||||||
|
] as const;
|
||||||
|
const hasNewerOwnerExecution = listPairedExecutionsForTask(
|
||||||
|
args.execution.task_id,
|
||||||
|
).some((candidate) => {
|
||||||
|
if (candidate.role !== 'owner' || candidate.id === args.execution.id) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const candidateOrderKey = [
|
||||||
|
candidate.started_at ?? candidate.created_at,
|
||||||
|
candidate.created_at,
|
||||||
|
candidate.id,
|
||||||
|
] as const;
|
||||||
|
if (candidateOrderKey[0] > executionOrderKey[0]) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (candidateOrderKey[0] < executionOrderKey[0]) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (candidateOrderKey[1] > executionOrderKey[1]) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (candidateOrderKey[1] < executionOrderKey[1]) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return candidateOrderKey[2] > executionOrderKey[2];
|
||||||
|
});
|
||||||
|
return !hasNewerOwnerExecution;
|
||||||
|
}
|
||||||
|
|
||||||
|
const executionCheckpoint = args.execution.checkpoint_fingerprint ?? null;
|
||||||
|
if (!executionCheckpoint) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
const latestCheckpoint = getLatestReviewCheckpointFingerprint(
|
||||||
|
args.execution.task_id,
|
||||||
|
);
|
||||||
|
if (!latestCheckpoint) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return executionCheckpoint === latestCheckpoint;
|
||||||
|
}
|
||||||
|
|
||||||
export interface PreparedPairedExecutionContext {
|
export interface PreparedPairedExecutionContext {
|
||||||
task: PairedTask;
|
task: PairedTask;
|
||||||
execution: PairedExecution;
|
execution: PairedExecution;
|
||||||
@@ -202,6 +301,11 @@ export function preparePairedExecutionContext(args: {
|
|||||||
roomRoleContext,
|
roomRoleContext,
|
||||||
task,
|
task,
|
||||||
workspace: workspace ?? undefined,
|
workspace: workspace ?? undefined,
|
||||||
|
checkpointFingerprint: resolveExecutionCheckpointFingerprint({
|
||||||
|
taskId: task.id,
|
||||||
|
role: roomRoleContext.role,
|
||||||
|
workspace,
|
||||||
|
}),
|
||||||
});
|
});
|
||||||
const envOverrides: Record<string, string> = {
|
const envOverrides: Record<string, string> = {
|
||||||
EJCLAW_PAIRED_TASK_ID: task.id,
|
EJCLAW_PAIRED_TASK_ID: task.id,
|
||||||
@@ -394,19 +498,18 @@ function markTaskReviewPendingWithoutSnapshot(taskId: string): void {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function maybeAutoRequestReviewForCompletedExecution(args: {
|
function maybeAutoRequestReviewForCompletedExecution(args: {
|
||||||
executionId: string;
|
execution: PairedExecution;
|
||||||
status: 'succeeded' | 'failed';
|
status: 'succeeded' | 'failed';
|
||||||
}): void {
|
}): void {
|
||||||
if (args.status !== 'succeeded') {
|
if (args.status !== 'succeeded') {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const execution = getPairedExecutionById(args.executionId);
|
if (args.execution.role !== 'owner') {
|
||||||
if (!execution || execution.role !== 'owner') {
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const task = getPairedTaskById(execution.task_id);
|
const task = getPairedTaskById(args.execution.task_id);
|
||||||
if (!task) {
|
if (!task) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -446,7 +549,7 @@ function maybeAutoRequestReviewForCompletedExecution(args: {
|
|||||||
|
|
||||||
const roomRoleContext = buildRoomRoleContextFromExecution({
|
const roomRoleContext = buildRoomRoleContextFromExecution({
|
||||||
task,
|
task,
|
||||||
execution,
|
execution: args.execution,
|
||||||
});
|
});
|
||||||
const reviewIntent = applyRoomIntent({
|
const reviewIntent = applyRoomIntent({
|
||||||
task,
|
task,
|
||||||
@@ -460,10 +563,15 @@ function maybeAutoRequestReviewForCompletedExecution(args: {
|
|||||||
});
|
});
|
||||||
|
|
||||||
if (reviewIntent.applied) {
|
if (reviewIntent.applied) {
|
||||||
|
cancelSupersededPairedExecutions({
|
||||||
|
taskId: task.id,
|
||||||
|
role: 'reviewer',
|
||||||
|
note: 'Superseded by a newer review checkpoint.',
|
||||||
|
});
|
||||||
logger.info(
|
logger.info(
|
||||||
{
|
{
|
||||||
taskId: task.id,
|
taskId: task.id,
|
||||||
executionId: execution.id,
|
executionId: args.execution.id,
|
||||||
sourceFingerprint,
|
sourceFingerprint,
|
||||||
},
|
},
|
||||||
'Automatically requested review for low-risk owner checkpoint',
|
'Automatically requested review for low-risk owner checkpoint',
|
||||||
@@ -476,13 +584,43 @@ export function completePairedExecutionContext(args: {
|
|||||||
status: 'succeeded' | 'failed';
|
status: 'succeeded' | 'failed';
|
||||||
summary?: string | null;
|
summary?: string | null;
|
||||||
}): void {
|
}): void {
|
||||||
|
const execution = getPairedExecutionById(args.executionId);
|
||||||
|
if (!execution) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const completedAt = new Date().toISOString();
|
||||||
|
const preservedStatus =
|
||||||
|
execution.status === 'cancelled' ? execution.status : args.status;
|
||||||
|
|
||||||
updatePairedExecution(args.executionId, {
|
updatePairedExecution(args.executionId, {
|
||||||
status: args.status,
|
status: preservedStatus,
|
||||||
summary: args.summary ?? null,
|
summary: args.summary ?? null,
|
||||||
completed_at: new Date().toISOString(),
|
completed_at: completedAt,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const completedExecution: PairedExecution = {
|
||||||
|
...execution,
|
||||||
|
status: preservedStatus,
|
||||||
|
summary: args.summary ?? null,
|
||||||
|
completed_at: completedAt,
|
||||||
|
};
|
||||||
|
|
||||||
|
if (!isExecutionFreshForTaskStateWrite({ execution: completedExecution })) {
|
||||||
|
logger.info(
|
||||||
|
{
|
||||||
|
taskId: completedExecution.task_id,
|
||||||
|
executionId: completedExecution.id,
|
||||||
|
role: completedExecution.role,
|
||||||
|
checkpointFingerprint: completedExecution.checkpoint_fingerprint,
|
||||||
|
},
|
||||||
|
'Skipped task state write from stale paired execution',
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
maybeAutoRequestReviewForCompletedExecution({
|
maybeAutoRequestReviewForCompletedExecution({
|
||||||
executionId: args.executionId,
|
execution: completedExecution,
|
||||||
status: args.status,
|
status: args.status,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -535,6 +673,14 @@ export function markRoomReviewReady(args: {
|
|||||||
onApply: () => markPairedTaskReviewReady(task.id),
|
onApply: () => markPairedTaskReviewReady(task.id),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
if (reviewIntent.applied) {
|
||||||
|
cancelSupersededPairedExecutions({
|
||||||
|
taskId: task.id,
|
||||||
|
role: 'reviewer',
|
||||||
|
note: 'Superseded by a newer review checkpoint.',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
const latestTask = getPairedTaskById(task.id) ?? task;
|
const latestTask = getPairedTaskById(task.id) ?? task;
|
||||||
const { ownerWorkspace: persistedOwnerWorkspace, reviewerWorkspace } =
|
const { ownerWorkspace: persistedOwnerWorkspace, reviewerWorkspace } =
|
||||||
getPersistedReviewReadyState(
|
getPersistedReviewReadyState(
|
||||||
|
|||||||
@@ -125,6 +125,7 @@ export interface PairedExecution {
|
|||||||
service_id: string;
|
service_id: string;
|
||||||
role: PairedWorkspaceRole;
|
role: PairedWorkspaceRole;
|
||||||
workspace_id: string | null;
|
workspace_id: string | null;
|
||||||
|
checkpoint_fingerprint?: string | null;
|
||||||
status: PairedExecutionStatus;
|
status: PairedExecutionStatus;
|
||||||
summary: string | null;
|
summary: string | null;
|
||||||
created_at: string;
|
created_at: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user