fix: terminate failed scans in Temporal and surface the reason when following (#429)

* fix(cli): skip splash screen off a TTY (e.g. CI) * fix: terminate failed scans in Temporal and surface the reason when following * fix(cli): indent embedded newlines within failure-error segments * fix(worker): omit the Agent Breakdown section when no agents completed * fix(cli): don't reprint the failure reason when the log already showed it * fix(worker): indent embedded newlines within the workflow.log error block
2026-08-29 14:30:45 +02:00 · 2026-08-24 20:04:47 +05:30
parent 53118c6203
commit b13788d8ef
10 changed files with 261 additions and 81 deletions
@@ -1,21 +1,23 @@
 /**
 * `shannon logs` command — tail a scan's live log.
 *
- * Uses chokidar for reliable cross-platform file watching and
- * bounded synchronous reads to prevent duplicate output.
+ * The log file is streamed for its content; completion is decided by Temporal (the
+ * workflow's status), so a worker that dies mid-run can't leave the tail hanging. Uses
+ * chokidar for reliable cross-platform file watching and bounded synchronous reads to
+ * prevent duplicate output.
 */

 import fs from 'node:fs';
 import path from 'node:path';
+import { setTimeout as sleep } from 'node:timers/promises';
 import { watch } from 'chokidar';
 import { fail } from '../errors.js';
 import { getWorkspacesDir } from '../home.js';
 import { resolveRunFile } from '../paths.js';
+import { resolveWorkflowId } from '../session.js';
+import { waitForWorkflowClose } from '../temporal-client.js';
 import { stdoutIsTerminal } from '../tty.js';

-// Match the exact line the worker writes — anchored to prevent false positives from agent output
-const COMPLETION_PATTERN = /^Scan (COMPLETED|FAILED)$/m;
-
 /** Read a byte range from a file and return it as a UTF-8 string. */
 function readRange(filePath: string, start: number, end: number): string {
  const length = end - start;
@@ -62,60 +64,114 @@ export function resolveLogFile(workspaceId: string): string {
  );
 }

+export interface TailOptions {
+  /** Workflow whose Temporal status decides when the tail stops. Without it, only Ctrl-C ends the tail. */
+  readonly workflowId?: string;
+  /** Called if the tail ends because Temporal became unreachable, with the captured error. */
+  readonly onUnreachable?: (lastError: string) => void;
+}
+
+/** Outcome of a tail: whether the streamed log already contained the worker's `Scan FAILED` block. */
+export interface TailResult {
+  readonly sawFailure: boolean;
+}
+
+// The worker writes this exact line at the head of its terminal failure summary.
+const FAILURE_MARKER = /^Scan FAILED$/m;
+
 /**
- * Tail a scan's log until it reports completion, resolving when the completion marker appears
- * (or the file is gone, or Ctrl-C stops it). Never exits the process, so the caller decides what
- * happens next: plain `logs` exits 0; `start --follow` reads the workflow outcome first.
+ * Stream a scan's log to the terminal until the workflow closes (completion comes from Temporal,
+ * or Ctrl-C). A Temporal outage is warned about and, if sustained, ends the tail with a diagnostic.
+ * Never exits the process: plain `logs` exits; `start --follow` reads the workflow outcome first.
+ * Reports whether the log already showed the failure, so a caller need not print it a second time.
 */
-export function tailUntilComplete(logFile: string): Promise<void> {
+export function tailUntilComplete(logFile: string, opts: TailOptions = {}): Promise<TailResult> {
  return new Promise((resolve) => {
    let position = 0;
+    let done = false;
+    let sawFailure = false;
+    const controller = new AbortController();
+    let watcher: ReturnType<typeof watch> | undefined;

-    /**
-     * Output any new content appended since the last read.
-     * Returns true when the workflow completion marker is detected.
-     */
-    function flush(): boolean {
+    /** Output any new content appended since the last read. */
+    function flush(): void {
      try {
        const { size } = fs.statSync(logFile);
-        if (size <= position) return false;
-
+        if (size <= position) return;
        const data = readRange(logFile, position, size);
        process.stdout.write(data);
        position = size;
-
-        return COMPLETION_PATTERN.test(data);
+        if (!sawFailure && FAILURE_MARKER.test(data)) {
+          sawFailure = true;
+        }
      } catch {
-        // File deleted or unreadable — treat as done
-        return true;
+        // File not present yet or transiently unreadable — nothing to flush this round.
      }
    }

-    // 1. Output existing content
-    if (flush()) {
-      resolve();
-      return;
+    function finish(): void {
+      if (done) return;
+      done = true;
+      controller.abort();
+      if (watcher) {
+        watcher.close().finally(() => resolve({ sawFailure }));
+        // Safety net — resolve anyway if watcher.close() stalls.
+        setTimeout(() => resolve({ sawFailure }), 1000).unref();
+      } else {
+        resolve({ sawFailure });
+      }
    }

-    // 2. Watch for appended content via chokidar
-    const watcher = watch(logFile, { persistent: true });
+    // 1. Output existing content, then stream anything appended.
+    flush();
+    watcher = watch(logFile, { persistent: true });
+    watcher.on('change', () => flush());

-    const stop = (): void => {
-      watcher.close().finally(() => resolve());
-      // Safety net — resolve anyway if watcher.close() stalls
-      setTimeout(() => resolve(), 1000).unref();
-    };
+    // 2. Ctrl-C stops watching.
+    process.on('SIGINT', finish);

-    watcher.on('change', () => {
-      if (flush()) stop();
-    });
-
-    process.on('SIGINT', stop);
+    // 3. Temporal decides completion. Without a workflow id, the tail relies on Ctrl-C alone.
+    if (opts.workflowId) {
+      waitForWorkflowClose(opts.workflowId, {
+        signal: controller.signal,
+        onConnectionTrouble: (lastError) => {
+          if (!done) console.error(`\n⚠ Lost contact with Temporal, retrying… (${lastError})`);
+        },
+        onReconnected: () => {
+          if (!done) console.error('  Reconnected to Temporal.');
+        },
+      })
+        .then(async (end) => {
+          if (done) return;
+          // Flush, let a just-written final summary land, then flush the tail once more.
+          flush();
+          await sleep(750).catch(() => {});
+          flush();
+          if (end.reason === 'unreachable') {
+            console.error('\nScan watch aborted: lost contact with Temporal.');
+            console.error(`  Last error: ${end.lastError}`);
+            console.error('  Temporal may have crashed — check `docker compose logs temporal`.');
+            opts.onUnreachable?.(end.lastError);
+          }
+          finish();
+        })
+        .catch(() => {
+          // waitForWorkflowClose never rejects; guard only against an aborted race.
+        });
+    }
  });
 }

 export function logs(workspaceId: string): void {
  const logFile = resolveLogFile(workspaceId);
+  const workflowId = resolveWorkflowId(workspaceId);
  console.error(stdoutIsTerminal() ? `Tailing scan log: ${logFile}` : 'Tailing scan log');
-  tailUntilComplete(logFile).finally(() => process.exit(0));
+
+  let unreachable = false;
+  tailUntilComplete(logFile, {
+    ...(workflowId ? { workflowId } : {}),
+    onUnreachable: () => {
+      unreachable = true;
+    },
+  }).finally(() => process.exit(unreachable ? 1 : 0));
 }
@@ -24,6 +24,7 @@ import {
  resolveRepo,
  resolveRunFile,
 } from '../paths.js';
+import { indentFailureSegments } from '../scan/failure.js';
 import { resolveWorkflowId } from '../session.js';
 import { displaySplash } from '../splash.js';
 import { getTerminalOutcome } from '../temporal-client.js';
@@ -81,7 +82,10 @@ export async function start(args: StartArgs): Promise<void> {
  const config = args.config ? resolveConfig(args.config) : undefined;

  // Inputs are valid — show the splash before the Docker/Temporal setup work.
-  displaySplash(isLocal() ? undefined : args.version);
+  // Skip it off a real terminal (e.g. CI) so piped/logged output stays clean.
+  if (stdoutIsTerminal()) {
+    displaySplash(isLocal() ? undefined : args.version);
+  }

  // 4. Ensure workspaces dir is writable by container user (UID 1001)
  const workspacesDir = getWorkspacesDir();
@@ -237,12 +241,14 @@ export async function start(args: StartArgs): Promise<void> {
 }

 /**
- * Follow a just-started scan (for `--follow`, aimed at CI): stream its log to completion, then
- * exit on the workflow outcome — 0 if the assessment ran, 1 if the scan failed. That tracks
- * whether the pipeline ran, not whether vulnerabilities were found.
+ * Follow a just-started scan (for `--follow`, aimed at CI): stream its log while Temporal drives
+ * completion, then exit on the workflow outcome — 0 if the assessment ran, 1 if the scan failed.
+ * That tracks whether the pipeline ran, not whether vulnerabilities were found. On failure the
+ * root-cause message is printed so a red CI build says why.
 */
 async function followScan(workspace: string, workspacesDir: string): Promise<never> {
  const logFile = resolveRunFile(path.join(workspacesDir, workspace), 'workflow.log');
+  const workflowId = resolveWorkflowId(workspace);

  // The worker creates workflow.log as it starts; wait briefly so the first read doesn't
  // mistake a not-yet-created file for an already-finished scan.
@@ -253,18 +259,38 @@ async function followScan(workspace: string, workspacesDir: string): Promise<nev
  if (stdoutIsTerminal()) {
    console.error('\n  Following scan log (Ctrl-C to stop watching):\n');
  }
-  await tailUntilComplete(logFile);

-  const workflowId = resolveWorkflowId(workspace);
+  let temporalUnreachable = false;
+  const { sawFailure } = await tailUntilComplete(logFile, {
+    ...(workflowId && { workflowId }),
+    onUnreachable: () => {
+      temporalUnreachable = true;
+    },
+  });
+
+  // The tail already printed the diagnostic; reading the outcome would only fail the same way.
+  if (temporalUnreachable) {
+    process.exit(1);
+  }
+
  if (!workflowId) {
    fail('Scan finished but its workflow id could not be resolved from session.json.');
  }

  try {
    const outcome = await getTerminalOutcome(workflowId);
-    process.exit(outcome.kind === 'success' ? 0 : 1);
-  } catch {
-    fail('Could not reach Temporal at 127.0.0.1:7233 to read the scan outcome.');
+    if (outcome.kind === 'failed') {
+      // Print the reason only when the streamed log didn't already show the worker's failure
+      // summary — otherwise the worker crashed before writing it, and this is the only report.
+      if (!sawFailure) {
+        console.error(`\nScan failed:\n${indentFailureSegments(outcome.message)}`);
+      }
+      process.exit(1);
+    }
+    process.exit(0);
+  } catch (err) {
+    const detail = err instanceof Error ? err.message : String(err);
+    fail('Could not read the scan outcome from Temporal at 127.0.0.1:7233.', `  ${detail}`);
  }
 }