fix: preserve paired completion errors without leases (#217)

src/paired-execution-context.test.ts

@@ -1446,6 +1446,34 @@ describe('paired execution context', () => {
   });
 });
 
+describe('paired execution context completion lease cleanup', () => {
+  beforeEach(() => {
+    vi.resetAllMocks();
+  });
+
+  it('propagates completion handler errors when no execution lease was reserved', () => {
+    const transitionError = new Error('transition failed without lease');
+    vi.mocked(db.getPairedTaskById).mockReturnValue(
+      buildPairedTask({
+        status: 'merge_ready',
+      }),
+    );
+    vi.mocked(db.updatePairedTaskIfUnchanged).mockImplementationOnce(() => {
+      throw transitionError;
+    });
+
+    expect(() =>
+      completePairedExecutionContext({
+        taskId: 'task-1',
+        role: 'owner',
+        status: 'failed',
+        summary: 'push failed',
+      }),
+    ).toThrow('transition failed without lease');
+    expect(db.releasePairedTaskExecutionLease).not.toHaveBeenCalled();
+  });
+});
+
 describe('paired execution context reviewer failures', () => {
   beforeEach(() => {
     vi.resetAllMocks();

src/paired-execution-context.ts

@@ -619,15 +619,18 @@ export function preparePairedExecutionContext(args: {
   };
 }
 
-export function completePairedExecutionContext(args: {
+type CompletePairedExecutionContextArgs = {
   taskId: string;
   role: PairedRoomRole;
   status: 'succeeded' | 'failed';
   runId?: string;
   summary?: string | null;
-}): void {
+};
+
+export function completePairedExecutionContext(
+  args: CompletePairedExecutionContextArgs,
+): void {
   const { taskId, role, status } = args;
-  let completionError: unknown;
   logger.info(
     {
       taskId,
@@ -639,6 +642,28 @@ export function completePairedExecutionContext(args: {
   );
 
   try {
+    completePairedExecutionContextBody(args);
+  } catch (error) {
+    releaseExecutionLeaseAfterCompletion({
+      taskId,
+      role,
+      runId: args.runId,
+      completionError: error,
+    });
+    throw error;
+  }
+
+  releaseExecutionLeaseAfterCompletion({
+    taskId,
+    role,
+    runId: args.runId,
+  });
+}
+
+function completePairedExecutionContextBody(
+  args: CompletePairedExecutionContextArgs,
+): void {
+  const { taskId, role, status } = args;
   const task = getPairedTaskById(taskId);
   if (!task) {
     return;
@@ -711,28 +736,32 @@ export function completePairedExecutionContext(args: {
   if (role === 'arbiter') {
     handleArbiterCompletion({ task, taskId, summary: args.summary });
   }
-  } catch (error) {
-    completionError = error;
-    throw error;
-  } finally {
+}
+
+function releaseExecutionLeaseAfterCompletion(args: {
+  taskId: string;
+  role: PairedRoomRole;
+  runId?: string;
+  completionError?: unknown;
+}): void {
   if (!args.runId) {
     return;
   }
+
   try {
-      releasePairedTaskExecutionLease({ taskId, runId: args.runId });
+    releasePairedTaskExecutionLease({ taskId: args.taskId, runId: args.runId });
   } catch (releaseError) {
     logger.error(
       {
-          taskId,
-          role,
+        taskId: args.taskId,
+        role: args.role,
         runId: args.runId,
         releaseError,
       },
       'Failed to release paired task execution lease after completion',
     );
-      if (!completionError) {
+    if (!args.completionError) {
       throw releaseError;
     }
   }
-  }
 }