test: address review feedback — assert node_output, test failure message format

kagura-agent · kagura-agent · commit b1c94a1ada68 · 2026-05-12T02:15:37.000+08:00
- Assert node_output field in node_skipped_prior_success event test
- Add test for empty-string fallback when node ID has undefined output
- Add dedicated test verifying failure message names failing nodes
- Update stale comment from 'no successful nodes' to new behavior
- Add edge-case test for only node_skipped_prior_success rows (no node_completed)
diff --git a/packages/core/src/db/workflow-events.test.ts b/packages/core/src/db/workflow-events.test.ts
@@ -227,6 +227,27 @@ describe('workflow-events', () => {
       );
     });
 
+    test('returns outputs when only node_skipped_prior_success rows exist (no node_completed)', async () => {
+      mockQuery.mockResolvedValueOnce(
+        createQueryResult([
+          {
+            step_name: 'node-x',
+            data: { reason: 'prior_success', node_output: 'skipped output X' },
+          },
+          {
+            step_name: 'node-y',
+            data: { reason: 'prior_success', node_output: 'skipped output Y' },
+          },
+        ])
+      );
+
+      const result = await getCompletedDagNodeOutputs('run-all-skipped');
+
+      expect(result.size).toBe(2);
+      expect(result.get('node-x')).toBe('skipped output X');
+      expect(result.get('node-y')).toBe('skipped output Y');
+    });
+
     test('parses JSON string data (SQLite path)', async () => {
       mockQuery.mockResolvedValueOnce(
         createQueryResult([
diff --git a/packages/workflows/src/dag-executor.test.ts b/packages/workflows/src/dag-executor.test.ts
@@ -2973,6 +2973,54 @@ describe('executeDagWorkflow -- resume with priorCompletedNodes', () => {
         (call[0] as { step_name: string }).step_name === 'step1'
     );
     expect(skippedEvent).toBeDefined();
+    expect(skippedEvent[0].data.node_output).toBe('prior output');
+  });
+
+  it('emits node_skipped_prior_success with empty output when node ID not in map', async () => {
+    const store = createMockStore();
+    const mockDeps = createMockDeps(store);
+    const platform = createMockPlatform();
+    const workflowRun = makeWorkflowRun('resume-empty-output');
+
+    // priorCompletedNodes has step1 but with undefined value to test the ?? '' fallback
+    const priorCompletedNodes = new Map<string, string>([
+      ['step1', undefined as unknown as string],
+    ]);
+
+    await executeDagWorkflow(
+      mockDeps,
+      platform,
+      'conv-resume',
+      testDir,
+      {
+        name: 'two-step',
+        nodes: [
+          { id: 'step1', command: 'step1' },
+          { id: 'step2', command: 'step2', depends_on: ['step1'] },
+        ],
+      },
+      workflowRun,
+      'claude',
+      undefined,
+      join(testDir, 'artifacts'),
+      join(testDir, 'logs'),
+      'main',
+      'docs/',
+      minimalConfig,
+      undefined,
+      undefined,
+      priorCompletedNodes
+    );
+
+    const eventCalls = (store.createWorkflowEvent as ReturnType<typeof mock>).mock.calls;
+    const skippedEvent = eventCalls.find(
+      (call: unknown[]) =>
+        (call[0] as { event_type: string }).event_type === 'node_skipped_prior_success' &&
+        (call[0] as { step_name: string }).step_name === 'step1'
+    );
+    expect(skippedEvent).toBeDefined();
+    // The ?? '' fallback kicks in when the map value is undefined
+    expect(skippedEvent[0].data.node_output).toBe('');
   });
 
   it('runs all nodes when priorCompletedNodes is empty', async () => {
@@ -4664,7 +4712,7 @@ describe('executeDagWorkflow -- terminal node output selection', () => {
 
     expect(store.failWorkflowRun).toHaveBeenCalled();
     // The "unknown provider" detail surfaces on the node_failed event; the
-    // workflow-level fail message is a generic "no successful nodes" summary.
+    // workflow-level fail message names the failing node(s).
     const eventCalls = (store.createWorkflowEvent as ReturnType<typeof mock>).mock.calls;
     const nodeFailedEvents = eventCalls.filter(
       (call: unknown[]) => (call[0] as Record<string, unknown>).event_type === 'node_failed'
@@ -4677,6 +4725,44 @@ describe('executeDagWorkflow -- terminal node output selection', () => {
     expect(nodeFailedData.error).toContain("unknown provider 'claud'");
   });
 
+  it('failure message names the failing node instead of generic summary', async () => {
+    const store = createMockStore();
+    const mockDeps = createMockDeps(store);
+    const platform = createMockPlatform();
+    const workflowRun = makeWorkflowRun();
+
+    await executeDagWorkflow(
+      mockDeps,
+      platform,
+      'conv-dag',
+      testDir,
+      {
+        name: 'fail-msg-test',
+        nodes: [
+          {
+            id: 'fail-node',
+            command: 'my-cmd',
+            provider: 'nonexistent',
+          },
+        ],
+      },
+      workflowRun,
+      'claude',
+      undefined,
+      join(testDir, 'artifacts'),
+      join(testDir, 'logs'),
+      'main',
+      'docs/',
+      minimalConfig
+    );
+
+    expect(store.failWorkflowRun).toHaveBeenCalled();
+    const failCall = (store.failWorkflowRun as ReturnType<typeof mock>).mock.calls[0];
+    const failMsg = failCall[1] as string;
+    expect(failMsg).toContain('fail-node failed');
+    expect(failMsg).not.toContain('no successful nodes');
+  });
+
   it('excludes intermediate nodes with dependents from terminal set (fan-in DAG)', async () => {
     let callCount = 0;
     mockSendQueryDag.mockImplementation(async function* () {
