Fix exit-code verification across completion paths

packages/core/src/__tests__/completion-pipeline.test.ts

@@ -70,6 +70,7 @@ vi.mock('@relaycast/sdk', () => ({
 let waitForExitFn: (ms?: number) => Promise<'exited' | 'timeout' | 'released'>;
 let waitForIdleFn: (ms?: number) => Promise<'idle' | 'timeout' | 'exited'>;
 let mockSpawnOutputs: string[] = [];
+let mockSpawnExitCodes: Array<number | undefined> = [];
 
 vi.mock('node:child_process', async () => {
   const actual = await vi.importActual<typeof import('node:child_process')>('node:child_process');
@@ -106,7 +107,7 @@ function makeMockHandle(name: string) {
   return {
     name,
     runtime: 'pty' as const,
-    exitCode: undefined as number | undefined,
+    exitCode: mockSpawnExitCodes.shift(),
     exitSignal: undefined as string | undefined,
     waitForExit: (ms?: number) => waitForExitFn(ms).then((reason) => ({ reason })),
     waitForIdle: (ms?: number) => waitForIdleFn(ms).then((reason) => ({ reason })),
@@ -348,6 +349,7 @@ describe('Completion Pipeline', () => {
     waitForExitFn = vi.fn().mockResolvedValue('exited');
     waitForIdleFn = vi.fn().mockImplementation(() => never());
     mockSpawnOutputs = [];
+    mockSpawnExitCodes = [];
     mockRelayInstance.spawnPty.mockImplementation(defaultSpawnPtyImplementation);
     eventListeners.clear();
     db = makeDb();
@@ -1728,6 +1730,7 @@ describe('Completion Pipeline', () => {
 
       // Output has no STEP_COMPLETE, no OWNER_DECISION — just normal work output
       mockSpawnOutputs = ['Implemented the auth module. All tests pass.'];
+      mockSpawnExitCodes = [0];
 
       const localDb = makeDb();
       runner = new WorkflowRunner({ db: localDb, workspaceId: 'ws-test' });
@@ -1823,6 +1826,7 @@ describe('Completion Pipeline', () => {
 
       // Output contains positive conclusion words but no explicit marker
       mockSpawnOutputs = ['Feature implemented and verified. All artifacts are correct and complete.'];
+      mockSpawnExitCodes = [0];
 
       const localDb = makeDb();
       runner = new WorkflowRunner({ db: localDb, workspaceId: 'ws-test' });
@@ -1925,6 +1929,7 @@ describe('Completion Pipeline', () => {
       mockSpawnOutputs = [
         'Implemented the feature.\nOWNER_DECISION: INCOMPLETE_RETRY\nREASON: needs more tests\n',
       ];
+      mockSpawnExitCodes = [0];
 
       const localDb = makeDb();
       runner = new WorkflowRunner({ db: localDb, workspaceId: 'ws-test' });

packages/core/src/__tests__/e2e-permissions.test.ts

@@ -184,7 +184,9 @@ function makeMockHandle(name: string) {
   return {
     name,
     runtime: 'pty' as const,
-    exitCode: undefined as number | undefined,
+    // This fixture models agents that exit cleanly; exit_code verification now
+    // consumes the recorded code instead of treating process success as implicit.
+    exitCode: 0 as number | undefined,
     exitSignal: undefined as string | undefined,
     waitForExit: (ms?: number) => waitForExitFn(ms).then((reason) => ({ reason })),
     waitForIdle: (ms?: number) => waitForIdleFn(ms).then((reason) => ({ reason })),

packages/core/src/__tests__/fixtures/permission-test.yaml

@@ -34,9 +34,11 @@ workflows:
         task: 'Verify you have read access. Check RELAY_AGENT_TOKEN is set.'
         verification:
           type: exit_code
+          value: '0'
       - name: write-step
         agent: writer
         dependsOn: [check-env]
         task: 'Verify you can write to src/tests/. Check RELAY_AGENT_TOKEN is set.'
         verification:
           type: exit_code
+          value: '0'

packages/core/src/__tests__/step-executor.test.ts

@@ -94,6 +94,47 @@ describe('StepExecutor — deterministic steps', () => {
     const result = await executor.executeOne(step, new Map());
     expect(result.status).toBe('completed');
   });
+
+  it('processSpawner path runs exit_code verification before accepting terminal success', async () => {
+    const executor = createExecutor({
+      processSpawner: mockSpawner({
+        spawnShell: vi.fn(async () => ({ output: 'claim already taken', exitCode: 78 })),
+      }),
+    });
+    const step = makeStep({
+      command: 'claim-work',
+      failOnError: false,
+      verification: { type: 'exit_code', value: '0' },
+      ...({ terminalSuccessExitCodes: [78] } as Partial<WorkflowStep>),
+    });
+
+    const result = await executor.executeOne(step, new Map());
+
+    expect(result.status).toBe('failed');
+    expect(result.error).toContain('recorded exit code "78" did not match "0"');
+  });
+
+  it('injected executeStep path runs exit_code verification before accepting terminal success', async () => {
+    const executor = createExecutor({
+      executeStep: vi.fn(async () => ({
+        status: 'completed',
+        output: 'claim already taken',
+        exitCode: 78,
+        completionReason: 'completed_early_exit' as any,
+      })),
+    });
+    const step = makeStep({
+      command: 'claim-work',
+      verification: { type: 'exit_code', value: '0' },
+      ...({ terminalSuccessExitCodes: [78] } as Partial<WorkflowStep>),
+    });
+
+    const result = await executor.executeOne(step, new Map());
+
+    expect(result.status).toBe('failed');
+    expect(result.completionReason).toBe('failed_verification');
+    expect(result.error).toContain('recorded exit code "78" did not match "0"');
+  });
 });
 
 // ── 2. Non-interactive agent step ────────────────────────────────────────────

packages/core/src/__tests__/verification.test.ts

@@ -44,18 +44,25 @@ describe('verification logic', () => {
     vi.clearAllMocks();
   });
 
-  // 1. exit_code — pass on exit 0 (implicit success)
+  // 1. exit_code — compare the recorded exit against the expected value
   describe('exit_code', () => {
-    it('should pass when agent exited successfully (exit 0 implicit)', () => {
-      const result = run({ type: 'exit_code', value: '0' }, 'some output');
+    it('should pass when the recorded exit code matches', () => {
+      const result = run({ type: 'exit_code', value: '0' }, 'some output', 'test-step', {
+        exitCode: 0,
+      });
       expect(result.passed).toBe(true);
       expect(result.completionReason).toBe('completed_verified');
     });
 
-    it('should still pass for non-zero value (exit_code is implicitly satisfied)', () => {
-      // per existing logic, exit_code case is a no-op — always passes if we reach it
-      const result = run({ type: 'exit_code', value: '1' }, 'output');
-      expect(result.passed).toBe(true);
+    it('should fail when the recorded exit code does not match', () => {
+      expect(() =>
+        run({ type: 'exit_code', value: '0' }, 'output', 'test-step', { exitCode: 78 })
+      ).toThrow('recorded exit code "78" did not match "0"');
+
+      const missing = run({ type: 'exit_code', value: '0' }, 'output', 'test-step', {
+        allowFailure: true,
+      });
+      expect(missing.passed).toBe(false);
     });
   });
 
@@ -227,6 +234,7 @@ describe('verification logic', () => {
     it('should log legacy marker message when completionMarkerFound is false', () => {
       const result = run({ type: 'exit_code', value: '0' }, 'output', 'my-step', {
         completionMarkerFound: false,
+        exitCode: 0,
       });
       expect(result.passed).toBe(true);
       expect(noopSideEffects.log).toHaveBeenCalledWith(

packages/core/src/__tests__/workflow-runner.test.ts

@@ -68,6 +68,7 @@ vi.mock('@relaycast/sdk', () => ({
 let waitForExitFn: (ms?: number) => Promise<'exited' | 'timeout' | 'released'>;
 let waitForIdleFn: (ms?: number) => Promise<'idle' | 'timeout' | 'exited'>;
 let mockSpawnOutputs: string[] = [];
+let mockSpawnExitCodes: Array<number | undefined> = [];
 const mockHarnessDriverSpawn = vi.fn(async () => mockRelayInstance);
 
 // Spawned-agent handle shaped like harness-driver's SpawnedAgentHandle, but
@@ -76,7 +77,7 @@ function makeMockHandle(name: string) {
   return {
     name,
     runtime: 'pty' as const,
-    exitCode: undefined as number | undefined,
+    exitCode: mockSpawnExitCodes.shift(),
     exitSignal: undefined as string | undefined,
     waitForExit: (ms?: number) => waitForExitFn(ms).then((reason) => ({ reason })),
     waitForIdle: (ms?: number) => waitForIdleFn(ms).then((reason) => ({ reason })),
@@ -317,6 +318,7 @@ describe('WorkflowRunner', () => {
     waitForExitFn = vi.fn().mockResolvedValue('exited');
     waitForIdleFn = vi.fn().mockImplementation(() => never());
     mockSpawnOutputs = [];
+    mockSpawnExitCodes = [];
     mockHarnessDriverSpawn.mockImplementation(async () => mockRelayInstance);
     mockRelayInstance.spawnPty.mockImplementation(defaultSpawnPtyImplementation);
     eventListeners.clear();
@@ -1005,6 +1007,48 @@ agents:
       expect(run.status, run.error).toBe('completed');
     });
 
+    it('WorkflowRunner deterministic executor path rejects terminal success when exit_code verification disagrees', async () => {
+      const executeDeterministicStep = vi.fn(async () => ({
+        output: 'claim already taken',
+        exitCode: 78,
+      }));
+      runner = new WorkflowRunner({
+        db,
+        workspaceId: 'ws-test',
+        executor: {
+          executeAgentStep: vi.fn(async () => ''),
+          executeDeterministicStep,
+        },
+      });
+
+      const run = await runner.execute(
+        makeConfig({
+          errorHandling: { strategy: 'fail-fast' },
+          agents: [],
+          workflows: [
+            {
+              name: 'default',
+              steps: [
+                {
+                  name: 'claim-gate',
+                  type: 'deterministic',
+                  command: 'claim-work',
+                  failOnError: false,
+                  verification: { type: 'exit_code', value: '0' },
+                  terminalSuccessExitCodes: [78],
+                } as any,
+              ],
+            },
+          ],
+        }),
+        'default'
+      );
+
+      expect(executeDeterministicStep).toHaveBeenCalledTimes(1);
+      expect(run.status).toBe('failed');
+      expect(run.error).toContain('recorded exit code "78" did not match "0"');
+    });
+
     it('repairs a failed deterministic gate with a workflow agent before retrying', async () => {
       const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relay-deterministic-repair-'));
       const stepDir = path.join(tmpDir, 'step-cwd');
@@ -1140,6 +1184,7 @@ agents:
 
     it('should apply verification fallback for self-owned interactive steps', async () => {
       mockSpawnOutputs = ['LEAD_DONE\n', 'REVIEW_DECISION: APPROVE\nREVIEW_REASON: verified\n'];
+      mockSpawnExitCodes = [0];
 
       const run = await runner.execute(
         makeConfig({
@@ -1237,6 +1282,7 @@ agents:
 
       try {
         mockSpawnOutputs = ['LEAD_DONE\n'];
+        mockSpawnExitCodes = [0];
 
         const run = await runner.execute(
           makeConfig({
@@ -1281,6 +1327,7 @@ agents:
 
     it('should pass canonical bypass args to interactive codex PTY spawns', async () => {
       mockSpawnOutputs = ['LEAD_DONE\n', 'REVIEW_DECISION: APPROVE\nREVIEW_REASON: verified\n'];
+      mockSpawnExitCodes = [0];
 
       const run = await runner.execute(
         makeConfig({
@@ -1441,6 +1488,10 @@ agents:
     it('should not double release the worker when the owner fails after worker completion', async () => {
       const workerRelease = vi.fn().mockResolvedValue(undefined);
       const ownerRelease = vi.fn().mockResolvedValue(undefined);
+      let markWorkerReleased!: () => void;
+      const workerReleasedSignal = new Promise<void>((resolve) => {
+        markWorkerReleased = resolve;
+      });
 
       mockRelayInstance.spawnPty.mockImplementation(
         async ({ name, task }: { name: string; task?: string }) => {
@@ -1461,7 +1512,10 @@ agents:
               // WorkflowAgentHandle destructures `reason`, so raw strings would
               // map to `undefined` and the timeout would go undetected.
               waitForExit: vi.fn().mockImplementation(async () => {
-                await Promise.resolve();
+                // Model the test's stated ordering explicitly: the worker's
+                // completion promise settles before the owner timeout fires.
+                await workerReleasedSignal;
+                await new Promise((resolve) => setTimeout(resolve, 0));
                 return { reason: 'timeout' };
               }),
               waitForIdle: vi.fn().mockResolvedValue({ reason: 'timeout' }),
@@ -1476,6 +1530,7 @@ agents:
             exitSignal: undefined,
             waitForExit: vi.fn().mockImplementation(async () => {
               await workerRelease();
+              markWorkerReleased();
               return { reason: 'released' };
             }),
             waitForIdle: vi.fn().mockImplementation(() => never()),

packages/core/src/runner.ts

@@ -4669,7 +4669,9 @@ export class WorkflowRunner {
             { exitCode: executorResult.exitCode }
           );
           const verificationResult = step.verification
-            ? this.runVerification(step.verification, output, step.name)
+            ? this.runVerification(step.verification, output, step.name, undefined, {
+                exitCode: executorResult.exitCode,
+              })
             : undefined;
           return {
             output,
@@ -4780,7 +4782,9 @@ export class WorkflowRunner {
         );
 
         const verificationResult = step.verification
-          ? this.runVerification(step.verification, output, step.name)
+          ? this.runVerification(step.verification, output, step.name, undefined, {
+              exitCode: lastExitCode,
+            })
           : undefined;
         lastCommandOutput = [commandStdout || output, commandStderr].filter(Boolean).join('\n');
 
@@ -5785,7 +5789,8 @@ export class WorkflowRunner {
             step.verification,
             specialistOutput,
             step.name,
-            promptTaskText
+            promptTaskText,
+            { exitCode: this.getStepCompletionEvidence(step.name)?.process.exitCode }
           );
           completionReason = verificationResult.completionReason;
         }
@@ -6652,6 +6657,7 @@ export class WorkflowRunner {
       ? this.runVerification(step.verification, specialistOutput, step.name, verificationTaskText, {
           allowFailure: true,
           completionMarkerFound: hasMarker,
+          exitCode: this.getStepCompletionEvidence(step.name)?.process.exitCode,
         })
       : { passed: false };
 
@@ -6959,7 +6965,7 @@ export class WorkflowRunner {
         specialistOutput,
         step.name,
         verificationTaskText,
-        { allowFailure: true }
+        { allowFailure: true, exitCode: evidence?.process.exitCode }
       );
       if (!verificationResult.passed) return null;
     }
@@ -7983,7 +7989,7 @@ export class WorkflowRunner {
             ptyOutput,
             step.name,
             preparedTask.promptTaskText,
-            { allowFailure: true }
+            { allowFailure: true, exitCode: agent?.exitCode }
           );
           if (verificationResult.passed) {
             this.log(`[${step.name}] Agent timed out but verification passed — treating as complete`);

packages/core/src/schema.ts

@@ -477,7 +477,7 @@ export interface VerificationCheck {
   /**
    * Type-specific value:
    *  - output_contains: token that must appear in the step's output
-   *  - exit_code: expected exit code (currently informational)
+   *  - exit_code: expected recorded process exit code
    *  - file_exists: path that must exist (relative to cwd or absolute)
    *  - custom: shell command to execute, or `regex:<pattern>` against output
    *  - pr_url: optional `<owner>/<repo>` qualifier to require the discovered