diff --git a/docs/conclusion.md b/docs/conclusion.md index a89db81b..a3968a0c 100644 --- a/docs/conclusion.md +++ b/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 diff --git a/scripts/ado-script/src/executor-e2e/__tests__/conclusion-cli.test.ts b/scripts/ado-script/src/executor-e2e/__tests__/conclusion-cli.test.ts new file mode 100644 index 00000000..4f6b8218 --- /dev/null +++ b/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 }); + } + }); +}); diff --git a/scripts/ado-script/src/executor-e2e/__tests__/runner.test.ts b/scripts/ado-script/src/executor-e2e/__tests__/runner.test.ts index 665e0ea0..0d520779 100644 --- a/scripts/ado-script/src/executor-e2e/__tests__/runner.test.ts +++ b/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 { + 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["postExecute"], + order: string[], + ): Scenario { + 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 }); + } + }); +}); diff --git a/scripts/ado-script/src/executor-e2e/conclusion-cli.ts b/scripts/ado-script/src/executor-e2e/conclusion-cli.ts new file mode 100644 index 00000000..4f597d3e --- /dev/null +++ b/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__TITLE_PREFIX`, `AW__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; + 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 { + 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 }; +} diff --git a/scripts/ado-script/src/executor-e2e/execute-cli.ts b/scripts/ado-script/src/executor-e2e/execute-cli.ts index 7e1fa021..f4c58daf 100644 --- a/scripts/ado-script/src/executor-e2e/execute-cli.ts +++ b/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 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 }); diff --git a/scripts/ado-script/src/executor-e2e/runner.ts b/scripts/ado-script/src/executor-e2e/runner.ts index 725415ab..c6da281f 100644 --- a/scripts/ado-script/src/executor-e2e/runner.ts +++ b/scripts/ado-script/src/executor-e2e/runner.ts @@ -154,6 +154,23 @@ export async function runScenario( }); } + // ---- post-execute (optional; e.g. the Conclusion reporter) ---- + if (scenario.postExecute) { + ctx.log(`[${scenarioId}] post-execute`); + try { + await scenario.postExecute(ctx, state, { + safeOutputDir: result.safeOutputDir, + records: result.records, + }); + } catch (err) { + if (err instanceof SkipError) { + ctx.log(`[${scenarioId}] SKIPPED: ${err.message}`); + return finish({ ok: true, skipped: true, phase: "skipped", message: err.message }); + } + return finish({ ok: false, phase: "post-execute", message: errMessage(err) }); + } + } + // ---- assert ---- try { await scenario.assert(ctx, state, result.record, result.records); diff --git a/scripts/ado-script/src/executor-e2e/scenario.ts b/scripts/ado-script/src/executor-e2e/scenario.ts index be8b3315..d57eb43c 100644 --- a/scripts/ado-script/src/executor-e2e/scenario.ts +++ b/scripts/ado-script/src/executor-e2e/scenario.ts @@ -94,6 +94,14 @@ export interface ScenarioSource { readonly prefix: (tool: string) => string; } +/** Files and records produced by one `ado-aw execute` run, handed to `postExecute`. */ +export interface PostExecuteRun { + /** Directory holding `safe_outputs.ndjson` + `safe-outputs-executed.ndjson`. */ + readonly safeOutputDir: string; + /** Every parsed record from the executed manifest. */ + readonly records: ExecutedRecord[]; +} + /** * A single deterministic executor scenario. * @@ -161,6 +169,20 @@ export interface Scenario { * child process (e.g. BUILD_SOURCESDIRECTORY pointing at a git checkout). */ env?(ctx: ScenarioContext, state: State): Promise>; + /** + * Optional phase that runs **after** a successful `ado-aw execute` and + * before `assert`. + * + * This exists for post-Stage-3 consumers of the executor's output — the + * Conclusion job reads `safe-outputs-executed.ndjson` from the same + * safe-output directory and files diagnostic work items from it. Running it + * here reproduces the production ordering (SafeOutputs → Conclusion) against + * a real manifest instead of a fixture. + * + * A throw records the scenario as failed in the `post-execute` phase; + * `cleanup()` still runs. + */ + postExecute?(ctx: ScenarioContext, state: State, run: PostExecuteRun): Promise; /** * Some scenarios intentionally submit invalid staged output and should pass * only when the executor rejects it with the expected failure. @@ -194,7 +216,7 @@ export interface Scenario { export interface ScenarioResult { tool: string; ok: boolean; - /** "setup" | "execute" | "assert" | "cleanup" | "skipped". */ + /** "setup" | "execute" | "post-execute" | "assert" | "cleanup" | "skipped". */ phase?: string; message?: string; durationMs: number; diff --git a/scripts/ado-script/src/executor-e2e/scenarios/conclusion.ts b/scripts/ado-script/src/executor-e2e/scenarios/conclusion.ts new file mode 100644 index 00000000..86a231c3 --- /dev/null +++ b/scripts/ado-script/src/executor-e2e/scenarios/conclusion.ts @@ -0,0 +1,313 @@ +/** + * Conclusion-job scenarios: work-item filing for the diagnostic signals. + * + * The signal safe-outputs (`noop`, `missing-tool`, `missing-data`) have no ADO + * write path of their own — the executor only records them in + * `safe-outputs-executed.ndjson`. Their *observable* effect is produced one job + * later by the Conclusion job (`conclusion.js`), which reads that manifest and + * files (or appends to) an Azure DevOps work item per signal — see + * `docs/conclusion.md`. + * + * The scenarios in `signals.ts` stop at the executor record, so nothing covered + * the signal → manifest → work-item path end to end. These scenarios close that + * gap: each runs the real executor, then the real conclusion bundle over the + * manifest it just wrote, and asserts the resulting work item via the ADO REST + * API. + * + * Coverage per scenario: + * - `conclusion-noop` — a work item is created for a `noop` signal, carrying + * the configured title, type, tags and rendered body. + * - `conclusion-missing-tool` — same for `missing-tool`, and the second + * conclusion run appends a comment instead of creating a duplicate + * (title deduplication). + * - `conclusion-missing-data` — same for `missing-data`, including the + * reported data type and reason. + * - `conclusion-report-as-work-item-false` — the per-tool opt-out files + * nothing at all. + * + * Test-harness module; not shipped in `ado-script.zip`. + */ +import { runConclusion } from "../conclusion-cli.js"; +import type { PostExecuteRun, Scenario, ScenarioContext } from "../scenario.js"; + +/** Work item type used for every conclusion scenario (the reporter's default). */ +const WORK_ITEM_TYPE = "Task"; + +/** Title prefix handed to the reporter; the rendered title appends the pipeline name. */ +const TITLE_PREFIX = "[ado-aw-e2e conclusion]"; + +interface ConclusionState { + /** Value of `AW_PIPELINE_NAME`; unique per build and scenario. */ + pipelineName: string; + /** The title the reporter is expected to render: ` `. */ + title: string; + /** Tag applied to created work items (also used for cleanup diagnostics). */ + tag: string; + /** Populated in `postExecute` once the work item is observed. */ + workItemId?: number; + /** stdout of the last conclusion run, asserted by the opt-out scenario. */ + stdout?: string; +} + +function conclusionState(ctx: ScenarioContext, scenarioId: string): ConclusionState { + const pipelineName = ctx.prefix(scenarioId); + return { + pipelineName, + title: `${TITLE_PREFIX} ${pipelineName}`, + tag: `ado-aw-e2e-${ctx.buildId}`, + }; +} + +/** Per-tool conclusion env, mirroring the flat `AW__*` vars the compiler emits. */ +function toolConfig( + envPrefix: string, + state: ConclusionState, + extra: Record = {}, +): Record { + return { + [`${envPrefix}_TITLE_PREFIX`]: TITLE_PREFIX, + [`${envPrefix}_WORK_ITEM_TYPE`]: WORK_ITEM_TYPE, + [`${envPrefix}_TAGS`]: JSON.stringify([state.tag]), + ...extra, + }; +} + +/** + * Wait for the work item to become visible to WIQL. + * + * `findWorkItemByTitle` goes through the WIQL endpoint, whose index lags work + * item creation by a second or two. Polling here (rather than reading once) + * keeps the assertion deterministic, and it also guarantees the *reporter's* + * own dedup query can see the item before a second run is asked to append. + */ +async function waitForWorkItem( + ctx: ScenarioContext, + title: string, + timeoutMs = 90_000, +): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + const id = await ctx.rest.findWorkItemByTitle(title); + if (id !== undefined) return id; + if (Date.now() >= deadline) { + throw new Error( + `no work item titled '${title}' became visible within ${timeoutMs}ms`, + ); + } + await new Promise((resolve) => setTimeout(resolve, 3_000)); + } +} + +/** Read a work item field as a string (missing/non-string fields fail loudly). */ +function fieldText(fields: Record, name: string): string { + const value = fields[name]; + if (typeof value !== "string") { + throw new Error(`work item field ${name} is not a string (got ${JSON.stringify(value)})`); + } + return value; +} + +/** + * Assert the shared shape of a conclusion-filed work item: title, type, tag and + * the substrings the reporter is expected to render into the description. + * Returns the asserted work item id so callers can make further checks against + * it without re-deriving (and re-guarding) it. + * + * Substrings are chosen to be free of `<`, `>` and `&` so the check holds + * whether or not Azure DevOps stores the body as Markdown or re-encodes it. + */ +async function assertFiledWorkItem( + ctx: ScenarioContext, + state: ConclusionState, + expectedBodySubstrings: readonly string[], +): Promise { + const workItemId = state.workItemId; + if (workItemId === undefined) { + throw new Error("postExecute did not record a work item id"); + } + const item = await ctx.rest.getWorkItem(workItemId); + const title = fieldText(item.fields, "System.Title"); + if (title !== state.title) { + throw new Error(`work item title is '${title}', expected '${state.title}'`); + } + const type = fieldText(item.fields, "System.WorkItemType"); + if (type !== WORK_ITEM_TYPE) { + throw new Error(`work item type is '${type}', expected '${WORK_ITEM_TYPE}'`); + } + const tags = fieldText(item.fields, "System.Tags"); + if (!tags.split(";").map((t) => t.trim()).includes(state.tag)) { + throw new Error(`work item tags '${tags}' do not include '${state.tag}'`); + } + const description = fieldText(item.fields, "System.Description"); + for (const expected of expectedBodySubstrings) { + if (!description.includes(expected)) { + throw new Error( + `work item description does not contain '${expected}': ${description.slice(0, 800)}`, + ); + } + } + return workItemId; +} + +/** Best-effort teardown: delete the filed work item (resolving it by title if needed). */ +async function cleanupWorkItem(ctx: ScenarioContext, state: ConclusionState): Promise { + const id = state.workItemId ?? (await ctx.rest.findWorkItemByTitle(state.title)); + if (id === undefined) return; + await ctx.rest.deleteWorkItem(id); +} + +/** Run the reporter once against the manifest the executor just wrote. */ +async function reportOnce( + ctx: ScenarioContext, + state: ConclusionState, + run: PostExecuteRun, + config: Record, +): Promise { + const result = await runConclusion({ + safeOutputDir: run.safeOutputDir, + pipelineName: state.pipelineName, + orgUrl: ctx.orgUrl, + project: ctx.project, + token: ctx.token, + buildId: ctx.buildId, + config, + log: ctx.log, + }); + state.stdout = result.stdout; + return result.stdout; +} + +export const conclusionNoop: Scenario = { + id: "conclusion-noop", + tool: "noop", + config: () => ({}), + setup: async (ctx) => conclusionState(ctx, "conclusion-noop"), + ndjson: async (ctx) => ({ + context: `deterministic conclusion e2e noop for build ${ctx.buildId}`, + }), + postExecute: async (ctx, state, run) => { + await reportOnce(ctx, state, run, toolConfig("AW_NOOP", state)); + state.workItemId = await waitForWorkItem(ctx, state.title); + ctx.log(`[conclusion-noop] filed work item #${state.workItemId}`); + }, + assert: async (ctx, state) => { + await assertFiledWorkItem(ctx, state, [ + "noop", + "Occurrences: 1", + `deterministic conclusion e2e noop for build ${ctx.buildId}`, + `Build ID: ${ctx.buildId}`, + ]); + }, + cleanup: cleanupWorkItem, +}; + +export const conclusionMissingTool: Scenario = { + id: "conclusion-missing-tool", + tool: "missing-tool", + config: () => ({}), + setup: async (ctx) => conclusionState(ctx, "conclusion-missing-tool"), + ndjson: async (ctx) => ({ + tool_name: `ado-aw-det-${ctx.buildId}-bash`, + context: `deterministic conclusion e2e missing-tool for build ${ctx.buildId}`, + }), + postExecute: async (ctx, state, run) => { + const config = toolConfig("AW_MISSING_TOOL", state); + await reportOnce(ctx, state, run, config); + state.workItemId = await waitForWorkItem(ctx, state.title); + ctx.log(`[conclusion-missing-tool] filed work item #${state.workItemId}`); + // Second run over the same manifest: the reporter must dedup on the + // rendered title and append a comment rather than file a duplicate. + await reportOnce(ctx, state, run, config); + }, + assert: async (ctx, state) => { + const workItemId = await assertFiledWorkItem(ctx, state, [ + "missing_tool", + `ado-aw-det-${ctx.buildId}-bash`, + `deterministic conclusion e2e missing-tool for build ${ctx.buildId}`, + ]); + // Exactly one: the title is unique to this build and scenario, so the work + // item is always freshly created by the first conclusion run (which files, + // and does not comment). A second comment would mean the reporter appended + // twice; zero would mean it filed a duplicate work item instead. + const comments = await ctx.rest.getWorkItemComments(workItemId); + if (comments.length !== 1) { + throw new Error( + `expected exactly one appended comment after the second conclusion run, got ${comments.length}`, + ); + } + const commentText = comments[0]?.text ?? ""; + if (!commentText.includes("missing_tool")) { + throw new Error( + `appended comment does not describe the missing_tool signal: ${commentText.slice(0, 400)}`, + ); + } + }, + cleanup: cleanupWorkItem, +}; + +export const conclusionMissingData: Scenario = { + id: "conclusion-missing-data", + tool: "missing-data", + config: () => ({}), + setup: async (ctx) => conclusionState(ctx, "conclusion-missing-data"), + ndjson: async (ctx) => ({ + data_type: "deterministic-conclusion-e2e-data-type", + reason: `deterministic conclusion e2e missing-data for build ${ctx.buildId}`, + }), + postExecute: async (ctx, state, run) => { + await reportOnce(ctx, state, run, toolConfig("AW_MISSING_DATA", state)); + state.workItemId = await waitForWorkItem(ctx, state.title); + ctx.log(`[conclusion-missing-data] filed work item #${state.workItemId}`); + }, + assert: async (ctx, state) => { + await assertFiledWorkItem(ctx, state, [ + "missing_data", + "deterministic-conclusion-e2e-data-type", + `deterministic conclusion e2e missing-data for build ${ctx.buildId}`, + ]); + }, + cleanup: cleanupWorkItem, +}; + +export const conclusionOptOut: Scenario = { + id: "conclusion-report-as-work-item-false", + tool: "noop", + config: () => ({}), + setup: async (ctx) => conclusionState(ctx, "conclusion-report-as-work-item-false"), + ndjson: async (ctx) => ({ + context: `deterministic conclusion e2e report-as-work-item-false for build ${ctx.buildId}`, + }), + postExecute: async (ctx, state, run) => { + await reportOnce( + ctx, + state, + run, + toolConfig("AW_NOOP", state, { AW_NOOP_REPORT_AS_WORK_ITEM: "false" }), + ); + }, + assert: async (ctx, state) => { + const stdout = state.stdout ?? ""; + if (!stdout.includes("report-as-work-item is false")) { + throw new Error( + `conclusion did not log the per-tool opt-out: ${stdout.slice(0, 800)}`, + ); + } + // The absence check is secondary to the log assertion above: WIQL lags + // creation, so a filed item might not be visible yet. It still catches a + // regression where the opt-out is ignored on a later run of the suite. + const id = await ctx.rest.findWorkItemByTitle(state.title); + if (id !== undefined) { + throw new Error( + `work item #${id} was filed despite report-as-work-item: false`, + ); + } + }, + cleanup: cleanupWorkItem, +}; + +export const conclusionScenarios: Scenario[] = [ + conclusionNoop, + conclusionMissingTool, + conclusionMissingData, + conclusionOptOut, +]; diff --git a/scripts/ado-script/src/executor-e2e/scenarios/index.ts b/scripts/ado-script/src/executor-e2e/scenarios/index.ts index e2d277ab..35dd16ca 100644 --- a/scripts/ado-script/src/executor-e2e/scenarios/index.ts +++ b/scripts/ado-script/src/executor-e2e/scenarios/index.ts @@ -4,6 +4,7 @@ */ import type { Scenario } from "../scenario.js"; import { buildScenarios } from "./build.js"; +import { conclusionScenarios } from "./conclusion.js"; import { createPullRequestScenarios } from "./create-pull-request.js"; import { crossOrgScenarios } from "./cross-org.js"; import { gitScenarios } from "./git.js"; @@ -16,6 +17,7 @@ import { workItemScenarios } from "./work-item.js"; /** Every scenario, in a deterministic run order. */ export const allScenarios: Scenario[] = [ ...signalScenarios, + ...conclusionScenarios, ...workItemScenarios, ...wikiScenarios, ...prScenarios, diff --git a/tests/executor-e2e/README.md b/tests/executor-e2e/README.md index bd1f52f4..a6350584 100644 --- a/tests/executor-e2e/README.md +++ b/tests/executor-e2e/README.md @@ -48,6 +48,10 @@ All deterministically-assertable ADO-write safe outputs plus the flagship - **Signals:** `noop`, `missing-tool`, `missing-data`, `report-incomplete` (no ADO write path; assert that the executor emits the expected status) +- **Conclusion work-item filing:** `conclusion-noop`, + `conclusion-missing-tool`, `conclusion-missing-data` and + `conclusion-report-as-work-item-false` — see [Conclusion + scenarios](#conclusion-scenarios) below - **Work items:** `create-work-item`, `assign-work-item`, `update-work-item`, `comment-on-work-item`, `link-work-items`, `upload-workitem-attachment`, plus two rendering-fidelity scenarios (see [Rendering @@ -133,6 +137,36 @@ definition/queue-time variable first, then falls back to > now-deleted per-tool agentic smoke pipelines. Adding them here closes > the coverage gap while keeping the test deterministic. +## Conclusion scenarios + +The signal safe-outputs have no ADO write path of their own: `ado-aw execute` +only records them in `safe-outputs-executed.ndjson`. Their user-visible effect +is produced one job later by the **Conclusion job**, which reads that manifest +and files (or appends to) an Azure DevOps work item per signal — see +[`docs/conclusion.md`](../../docs/conclusion.md). + +These scenarios extend the harness past Stage 3: after `ado-aw execute` +succeeds, the runner's `postExecute` phase runs the **real compiled +`conclusion.js`** against the manifest that run just wrote, with the same flat +`AW_*` env contract the compiler emits. The work item is then asserted (and +deleted) through the ADO REST API. + +| Scenario id | Signal | What it proves | +| --- | --- | --- | +| `conclusion-noop` | `noop` | a work item is created with the configured title, type and tags, and its description carries the rendered noop report plus the conclusion stats block | +| `conclusion-missing-tool` | `missing-tool` | same for `missing-tool`, including the reported tool name; a **second** conclusion run over the same manifest appends one comment instead of filing a duplicate (title deduplication) | +| `conclusion-missing-data` | `missing-data` | same for `missing-data`, including the reported data type and reason | +| `conclusion-report-as-work-item-false` | `noop` | the per-tool `report-as-work-item: false` opt-out files nothing | + +Each scenario uses a title unique to the build +(`[ado-aw-e2e conclusion] ado-aw-det--`) so concurrent runs +never dedup into each other's work item, and deletes it in `cleanup`. + +The bundle is a build artifact, not a checked-in file: the scenarios read its +path from `EXECUTOR_E2E_CONCLUSION_BUNDLE` and **skip** when that is unset or +points at a missing file. The pipeline builds it with `npm run +build:conclusion` alongside the harness. + ## GitHub issue scenarios `create-github-issue` and `set-github-issue-type` had **zero runtime @@ -273,6 +307,8 @@ Some scenarios need optional infrastructure and **skip** (rather than fail) when it is not available: - `queue-build` — needs a target pipeline id in `E2E_QUEUE_PIPELINE_ID`. +- The four `conclusion-*` scenarios — need a compiled `conclusion.js` in + `EXECUTOR_E2E_CONCLUSION_BUNDLE`. - `create-wiki-page` / `update-wiki-page` — need a wiki in the project. The harness auto-discovers the first wiki; set `E2E_WIKI_NAME` to force one. When no wiki exists, both skip. @@ -303,13 +339,15 @@ You need a write-capable ADO token (PAT) and a checkout-built binary: ```bash cargo build --release --bin ado-aw -cd scripts/ado-script && npm ci && npm run build:executor-e2e && cd ../.. +cd scripts/ado-script && npm ci && npm run build:executor-e2e && npm run build:conclusion && cd ../.. export SYSTEM_COLLECTIONURI="https://dev.azure.com/msazuresphere/" export SYSTEM_TEAMPROJECT="AgentPlayground" export SYSTEM_ACCESSTOKEN="" export EXECUTOR_E2E_ADO_AW_BIN="$PWD/target/release/ado-aw" export EXECUTOR_E2E_ADO_REPO="agent-definitions" +# Enables the conclusion work-item scenarios (they skip when unset): +export EXECUTOR_E2E_CONCLUSION_BUNDLE="$PWD/scripts/ado-script/conclusion.js" # Optional: # export EXECUTOR_E2E_GITHUB_TOKEN="" # export EXECUTOR_E2E_ISSUE_REPO="jamesadevine/ado-aw-issues" diff --git a/tests/executor-e2e/azure-pipelines.yml b/tests/executor-e2e/azure-pipelines.yml index 2b6a7bd7..a63fa012 100644 --- a/tests/executor-e2e/azure-pipelines.yml +++ b/tests/executor-e2e/azure-pipelines.yml @@ -24,6 +24,7 @@ pr: - src/sanitize.rs - src/sanitize/** - src/safe_outputs/** + - scripts/ado-script/src/conclusion/** - scripts/ado-script/src/executor-e2e/** - tests/executor-e2e/** @@ -119,8 +120,12 @@ steps: set -euo pipefail npm ci npm run build:executor-e2e + # The conclusion scenarios run the real Conclusion reporter over the + # manifest the executor writes, so the bundle must be built too. + npm run build:conclusion + echo "##vso[task.setvariable variable=CONCLUSION_BUNDLE]$(Build.SourcesDirectory)/scripts/ado-script/conclusion.js" workingDirectory: scripts/ado-script - displayName: Build executor-e2e harness (not shipped in ado-script.zip) + displayName: Build executor-e2e harness + conclusion bundle (not shipped in ado-script.zip) - task: AzureCLI@2 displayName: Acquire ADO token (SC_WRITE_TOKEN) @@ -145,6 +150,9 @@ steps: # explicitly so the harness (and the ado-aw binary it spawns) can write. SYSTEM_ACCESSTOKEN: $(SC_WRITE_TOKEN) EXECUTOR_E2E_ADO_AW_BIN: $(ADO_AW_BIN) + # Compiled conclusion.js driven by the conclusion work-item scenarios. + # When unset those scenarios skip rather than fail. + EXECUTOR_E2E_CONCLUSION_BUNDLE: $(CONCLUSION_BUNDLE) EXECUTOR_E2E_ADO_REPO: $(EFFECTIVE_EXECUTOR_E2E_ADO_REPO) EXECUTOR_E2E_ISSUE_REPO: $(EXECUTOR_E2E_ISSUE_REPO) # Secret PAT for filing failure issues on the configured repository.