test(executor-e2e): cover conclusion work-item filing for signal safe outputs

docs/conclusion.md

@@ -103,6 +103,20 @@ Conclusion reports deduplicate by rendered work-item title. The job
 searches for an existing open work item with the same title; if it finds
 one, it appends a comment. Otherwise it creates a new work item.
 
+## Testing
+
+Unit coverage lives in
+`scripts/ado-script/src/conclusion/__tests__/index.test.ts` (manifest parsing,
+signal rendering, per-tool config).
+
+End-to-end coverage lives in the deterministic executor suite
+([`tests/executor-e2e/`](../tests/executor-e2e/README.md)): the `conclusion-*`
+scenarios run `ado-aw execute` for a `noop` / `missing-tool` / `missing-data`
+signal, then run the compiled `conclusion.js` over the resulting
+`safe-outputs-executed.ndjson`, and assert the filed Azure DevOps work item
+(title, type, tags, body), the append-on-duplicate-title path, and the
+`report-as-work-item: false` opt-out.
+
 ## Relationship to gh-aw
 
 This mirrors gh-aw's conclusion-job pattern: a single always-running

scripts/ado-script/src/executor-e2e/__tests__/conclusion-cli.test.ts

@@ -0,0 +1,93 @@
+import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+import { afterEach, describe, expect, it } from "vitest";
+
+import { CONCLUSION_BUNDLE_ENV, resolveConclusionBundle, runConclusion } from "../conclusion-cli.js";
+import { SkipError } from "../scenario.js";
+
+const originalBundle = process.env[CONCLUSION_BUNDLE_ENV];
+
+afterEach(() => {
+  if (originalBundle === undefined) delete process.env[CONCLUSION_BUNDLE_ENV];
+  else process.env[CONCLUSION_BUNDLE_ENV] = originalBundle;
+});
+
+describe("resolveConclusionBundle", () => {
+  it("skips the scenario when the bundle env var is unset", () => {
+    delete process.env[CONCLUSION_BUNDLE_ENV];
+    expect(() => resolveConclusionBundle()).toThrow(SkipError);
+  });
+
+  it("skips the scenario when the configured bundle does not exist", () => {
+    process.env[CONCLUSION_BUNDLE_ENV] = join(tmpdir(), "definitely-missing-conclusion.js");
+    expect(() => resolveConclusionBundle()).toThrow(SkipError);
+  });
+});
+
+describe("runConclusion", () => {
+  it("passes the safe-output dir, pipeline name and per-tool config to the bundle", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "ado-aw-conclusion-cli-"));
+    try {
+      // Fake bundle: echo the env the harness handed it, so the test pins the
+      // env-var contract shared with the compiler-generated Conclusion job.
+      const bundle = join(dir, "fake-conclusion.js");
+      await writeFile(
+        bundle,
+        `const keys = ["AW_SAFE_OUTPUT_DIR","AW_PIPELINE_NAME","AW_AGENT_RESULT",` +
+          `"AW_NOOP_TITLE_PREFIX","SYSTEM_TEAMPROJECT","SYSTEM_COLLECTIONURI","BUILD_BUILDID"];\n` +
+          `console.log(JSON.stringify(Object.fromEntries(keys.map((k) => [k, process.env[k]]))));\n`,
+        "utf8",
+      );
+      process.env[CONCLUSION_BUNDLE_ENV] = bundle;
+
+      const result = await runConclusion({
+        safeOutputDir: join(dir, "out"),
+        pipelineName: "ado-aw-det-1-conclusion-noop",
+        orgUrl: "https://dev.azure.com/org/",
+        project: "P",
+        token: "t",
+        buildId: "1",
+        config: { AW_NOOP_TITLE_PREFIX: "[prefix]" },
+        log: () => {},
+      });
+
+      expect(JSON.parse(result.stdout.trim())).toEqual({
+        AW_SAFE_OUTPUT_DIR: join(dir, "out"),
+        AW_PIPELINE_NAME: "ado-aw-det-1-conclusion-noop",
+        AW_AGENT_RESULT: "Succeeded",
+        AW_NOOP_TITLE_PREFIX: "[prefix]",
+        SYSTEM_TEAMPROJECT: "P",
+        SYSTEM_COLLECTIONURI: "https://dev.azure.com/org/",
+        BUILD_BUILDID: "1",
+      });
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it("fails when the bundle exits non-zero", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "ado-aw-conclusion-cli-"));
+    try {
+      const bundle = join(dir, "crashing-conclusion.js");
+      await writeFile(bundle, `console.error("boom");\nprocess.exit(3);\n`, "utf8");
+      process.env[CONCLUSION_BUNDLE_ENV] = bundle;
+
+      await expect(
+        runConclusion({
+          safeOutputDir: dir,
+          pipelineName: "p",
+          orgUrl: "https://dev.azure.com/org/",
+          project: "P",
+          token: "t",
+          buildId: "1",
+          config: {},
+          log: () => {},
+        }),
+      ).rejects.toThrow(/conclusion\.js exited 3/);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+});

scripts/ado-script/src/executor-e2e/__tests__/runner.test.ts

@@ -315,3 +315,116 @@ fs.writeFileSync(path.join(out, "safe-outputs-executed.ndjson"), [
     }
   });
 });
+
+/**
+ * `postExecute` runs a post-Stage-3 consumer (the Conclusion reporter) against
+ * the manifest the executor just wrote, before `assert`. These tests pin the
+ * ordering, the safe-output dir it is handed, and the failure/skip handling.
+ */
+describe("runScenario post-execute phase", () => {
+  /** Fake `ado-aw` that reports the primary tool as succeeded. */
+  async function writeOkBin(dir: string): Promise<string> {
+    const bin = join(dir, "ok-ado-aw.js");
+    await writeFile(
+      bin,
+      `#!/usr/bin/env node
+const fs = require("node:fs");
+const path = require("node:path");
+const out = process.argv[process.argv.indexOf("--safe-output-dir") + 1];
+fs.writeFileSync(
+  path.join(out, "safe-outputs-executed.ndjson"),
+  JSON.stringify({ name: "noop", status: "succeeded", result: {} }) + "\\n",
+);
+`,
+      { encoding: "utf8", mode: 0o755 },
+    );
+    return bin;
+  }
+
+  function postExecuteScenario(
+    postExecute: Scenario<unknown>["postExecute"],
+    order: string[],
+  ): Scenario<unknown> {
+    return {
+      id: "post-execute",
+      tool: "noop",
+      config: () => ({}),
+      setup: async () => ({}),
+      ndjson: async () => ({}),
+      postExecute,
+      assert: async () => {
+        order.push("assert");
+      },
+      cleanup: async () => {
+        order.push("cleanup");
+      },
+    };
+  }
+
+  it("runs before assert and receives the executor's safe-output dir and records", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "ado-aw-runner-post-"));
+    try {
+      const bin = await writeOkBin(dir);
+      const order: string[] = [];
+      let seenDir = "";
+      let seenRecords: ExecutedRecord[] = [];
+      const res = await runScenario(
+        { ...fakeCtx(), adoAwBin: bin, workDir: dir },
+        postExecuteScenario(async (_ctx, _state, run) => {
+          order.push("post-execute");
+          seenDir = run.safeOutputDir;
+          seenRecords = run.records;
+          // The executed manifest must be readable from the handed-over dir.
+          await readFile(join(run.safeOutputDir, "safe-outputs-executed.ndjson"), "utf8");
+        }, order),
+      );
+
+      expect(res.ok).toBe(true);
+      expect(order).toEqual(["post-execute", "assert", "cleanup"]);
+      expect(seenDir).toBe(join(dir, "post-execute", "out"));
+      expect(seenRecords.map((r) => r.name)).toEqual(["noop"]);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it("records a post-execute failure without running assert, but still cleans up", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "ado-aw-runner-post-"));
+    try {
+      const bin = await writeOkBin(dir);
+      const order: string[] = [];
+      const res = await runScenario(
+        { ...fakeCtx(), adoAwBin: bin, workDir: dir },
+        postExecuteScenario(async () => {
+          throw new Error("conclusion.js exited 3");
+        }, order),
+      );
+
+      expect(res.ok).toBe(false);
+      expect(res.phase).toBe("post-execute");
+      expect(res.message).toBe("conclusion.js exited 3");
+      expect(order).toEqual(["cleanup"]);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  it("treats a SkipError from post-execute as a skip", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "ado-aw-runner-post-"));
+    try {
+      const bin = await writeOkBin(dir);
+      const order: string[] = [];
+      const res = await runScenario(
+        { ...fakeCtx(), adoAwBin: bin, workDir: dir },
+        postExecuteScenario(async () => {
+          throw new SkipError("conclusion bundle not built");
+        }, order),
+      );
+
+      expect(res).toMatchObject({ ok: true, skipped: true, phase: "skipped" });
+      expect(order).toEqual(["cleanup"]);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+});

scripts/ado-script/src/executor-e2e/conclusion-cli.ts

@@ -0,0 +1,112 @@
+/**
+ * Wrapper around the compiled `conclusion.js` bundle for the deterministic E2E
+ * harness.
+ *
+ * Production shape: the Conclusion job runs `node conclusion.js` after the
+ * SafeOutputs job, reading `safe-outputs-executed.ndjson` from the downloaded
+ * `safe_outputs` artifact and filing/appending Azure DevOps work items for the
+ * diagnostic signals it finds (`noop`, `missing-tool`, `missing-data`) and for
+ * upstream job failures. The compiler passes its configuration as flat env vars
+ * (`AW_<TOOL>_TITLE_PREFIX`, `AW_<TOOL>_TAGS`, …) — see
+ * `src/compile/agentic_pipeline.rs` and `docs/conclusion.md`.
+ *
+ * This module reproduces exactly that invocation against the manifest a real
+ * `ado-aw execute` run just wrote, so the harness covers the whole
+ * signal → manifest → work-item path rather than stopping at Stage 3.
+ *
+ * Test-harness module; not shipped in `ado-script.zip`.
+ */
+import { existsSync } from "node:fs";
+
+import { partialOutput, spawnCollect } from "./execute-cli.js";
+import { SkipError } from "./scenario.js";
+
+/** Env var carrying the path to the compiled `conclusion.js` bundle. */
+export const CONCLUSION_BUNDLE_ENV = "EXECUTOR_E2E_CONCLUSION_BUNDLE";
+
+export interface RunConclusionOptions {
+  /** Directory holding `safe-outputs-executed.ndjson`. */
+  safeOutputDir: string;
+  /** Pipeline name the reporter renders into titles and the stats block. */
+  pipelineName: string;
+  orgUrl: string;
+  project: string;
+  token: string;
+  buildId: string;
+  /** Conclusion-specific `AW_*` config vars (title prefix, tags, opt-outs). */
+  config: Record<string, string>;
+  log: (msg: string) => void;
+}
+
+export interface RunConclusionResult {
+  exitCode: number;
+  stdout: string;
+  stderr: string;
+}
+
+/**
+ * Resolve the compiled bundle path, or skip the scenario when it is absent.
+ *
+ * The bundle is a build artifact (`npm run build:conclusion`), not a checked-in
+ * file, so a harness run that was not given one must skip rather than fail —
+ * the same contract the optional-precondition scenarios use.
+ */
+export function resolveConclusionBundle(): string {
+  const configured = process.env[CONCLUSION_BUNDLE_ENV]?.trim();
+  if (!configured) {
+    throw new SkipError(
+      `${CONCLUSION_BUNDLE_ENV} is not set; run 'npm run build:conclusion' and point it at conclusion.js`,
+    );
+  }
+  if (!existsSync(configured)) {
+    throw new SkipError(`${CONCLUSION_BUNDLE_ENV}='${configured}' does not exist`);
+  }
+  return configured;
+}
+
+/**
+ * Run the conclusion reporter once over `safeOutputDir`.
+ *
+ * `conclusion.js` is deliberately fail-open (it exits 0 even when work-item
+ * filing fails, so post-pipeline housekeeping can never fail an otherwise green
+ * build). A non-zero exit therefore means the bundle itself crashed, which we
+ * surface as an error; a filing failure is caught by the scenario's assertion
+ * against the ADO REST API instead of by the exit code.
+ */
+export async function runConclusion(
+  opts: RunConclusionOptions,
+): Promise<RunConclusionResult> {
+  const bundle = resolveConclusionBundle();
+  const env: NodeJS.ProcessEnv = {
+    ...process.env,
+    SYSTEM_ACCESSTOKEN: opts.token,
+    SYSTEM_COLLECTIONURI: opts.orgUrl,
+    SYSTEM_TEAMPROJECT: opts.project,
+    BUILD_BUILDID: opts.buildId,
+    AW_SAFE_OUTPUT_DIR: opts.safeOutputDir,
+    AW_PIPELINE_NAME: opts.pipelineName,
+    // The upstream job results the compiler wires in. All succeeded, so the
+    // pipeline-failure signal stays silent and only the diagnostic signals in
+    // the manifest are reported.
+    AW_AGENT_RESULT: "Succeeded",
+    AW_DETECTION_RESULT: "Succeeded",
+    AW_SAFEOUTPUTS_RESULT: "Succeeded",
+    ...opts.config,
+  };
+
+  opts.log(`[conclusion] running: node ${bundle} (AW_SAFE_OUTPUT_DIR=${opts.safeOutputDir})`);
+  const { exitCode, stdout, stderr } = await spawnCollect(
+    process.execPath,
+    [bundle],
+    env,
+    "conclusion.js",
+  );
+  if (stdout.trim()) opts.log(`[conclusion] stdout:\n${stdout.trim()}`);
+  if (stderr.trim()) opts.log(`[conclusion] stderr:\n${stderr.trim()}`);
+  if (exitCode !== 0) {
+    throw new Error(
+      `conclusion.js exited ${exitCode}${partialOutput(stdout, stderr)}`,
+    );
+  }
+  return { exitCode, stdout, stderr };
+}

scripts/ado-script/src/executor-e2e/execute-cli.ts

@@ -120,6 +120,13 @@ export interface RunExecuteResult {
   records: ExecutedRecord[];
   /** The record matching `tool` (dashes -> underscores), if any. */
   record?: ExecutedRecord;
+  /**
+   * Directory holding `safe_outputs.ndjson` and the executor-written
+   * `safe-outputs-executed.ndjson`. Exposed so a post-execute phase (e.g. the
+   * conclusion reporter, which consumes the executed manifest) can run against
+   * exactly the files this invocation produced.
+   */
+  safeOutputDir: string;
 }
 
 /** Parse `safe-outputs-executed.ndjson` content into typed records. */
@@ -233,7 +240,7 @@ export async function runExecute(opts: RunExecuteOptions): Promise<RunExecuteRes
   const snake = opts.tool.replaceAll("-", "_");
   const record = records.find((r) => r.name === snake);
 
-  return { exitCode, stdout, stderr, records, record };
+  return { exitCode, stdout, stderr, records, record, safeOutputDir };
 }
 
 /** Append a truncated snapshot of a subprocess's output to a timeout message. */
@@ -244,12 +251,18 @@ export function partialOutput(stdout: string, stderr: string): string {
   return parts.join("");
 }
 
-function spawnCollect(
+/**
+ * Spawn a child process, collect stdout/stderr, and reject when it exceeds the
+ * harness timeout. Shared with the conclusion-bundle runner so both child
+ * processes get identical hang protection and output capture.
+ */
+export function spawnCollect(
   cmd: string,
   args: string[],
   env: NodeJS.ProcessEnv,
+  label = "ado-aw execute",
 ): Promise<{ exitCode: number; stdout: string; stderr: string }> {
-  // Guard against a hung `ado-aw execute` blocking the whole suite: kill the
+  // Guard against a hung child blocking the whole suite: kill the
   // child after a bounded timeout and surface a meaningful error instead of
   // waiting for the ADO job-level timeout.
   const timeoutMs = Number(process.env.EXECUTOR_E2E_EXECUTE_TIMEOUT_MS) || 600_000;
@@ -273,7 +286,7 @@ function spawnCollect(
       if (timedOut) {
         // Include any accumulated output so a hung run is diagnosable from the
         // error/issue body rather than only from the raw ADO logs.
-        reject(new Error(`ado-aw execute timed out after ${timeoutMs}ms${partialOutput(stdout, stderr)}`));
+        reject(new Error(`${label} timed out after ${timeoutMs}ms${partialOutput(stdout, stderr)}`));
         return;
       }
       resolve({ exitCode: code ?? -1, stdout, stderr });