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

[Copilot]

Summary

The signal safe outputs (noop, missing-tool, missing-data) have no ADO write path — the executor only records them in safe-outputs-executed.ndjson. Their user-visible effect is produced one job later by the Conclusion job, which files work items from that manifest. The harness stopped at the executor record, so nothing exercised signal → manifest → work item end to end.

Changes:

• Post-execute phase (scenario.ts, runner.ts, execute-cli.ts) — new optional postExecute(ctx, state, run) hook that runs after a successful ado-aw execute and before assert, handed the safe-output dir and parsed records. Failures report phase post-execute; SkipError skips; cleanup still runs. runExecute now returns safeOutputDir, and spawnCollect is exported (with a label) so both child processes share the same hang protection.

• conclusion-cli.ts — spawns the compiled conclusion.js with the same flat AW_* env contract the compiler emits (AW_SAFE_OUTPUT_DIR, AW_PIPELINE_NAME, AW_<TOOL>_TITLE_PREFIX, …). The bundle is a build artifact, so scenarios skip when EXECUTOR_E2E_CONCLUSION_BUNDLE is unset or missing rather than failing.

• scenarios/conclusion.ts — four scenarios asserting the filed work item via ADO REST:

  Scenario	Proves
  conclusion-noop	work item created with configured title/type/tags, body carries the rendered report + stats
  conclusion-missing-tool	same incl. reported tool name; a second reporter run appends exactly one comment instead of filing a duplicate
  conclusion-missing-data	same incl. data type and reason
  conclusion-report-as-work-item-false	per-tool opt-out files nothing

  Titles are unique per build ([ado-aw-e2e conclusion] ado-aw-det-<buildId>-<scenario>) so concurrent runs never dedup into each other's item; each is deleted in cleanup. Work-item lookup polls because WIQL lags creation — which also guarantees the reporter's own dedup query can see the item before the append run.

• Pipeline / docs — azure-pipelines.yml builds conclusion.js, passes its path, and adds scripts/ado-script/src/conclusion/** to the PR path filter; coverage documented in tests/executor-e2e/README.md and docs/conclusion.md.

Test plan

• npm run typecheck and npm test (1190 tests) in scripts/ado-script.
• New unit tests: three runner tests pinning post-execute ordering, failure and skip handling; conclusion-cli.test.ts pinning the env contract, the skip paths, and non-zero bundle exit.
• Manually ran the freshly built conclusion.js over a hand-written manifest to confirm the signal-detection and opt-out log lines the scenarios assert on.
• The live ADO assertions run only in the executor-e2e pipeline (AgentPlayground); they skip locally without a built bundle.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟢 Approval recommended

The changes are well-scoped to test harness + pipeline wiring, with added unit tests and no apparent contract mismatches in the Conclusion env/config handling.

Pull request overview

Extends the deterministic executor-e2e harness to exercise the real Conclusion reporter (conclusion.js) end-to-end for signal safe outputs, ensuring the “signal → executed manifest → filed work item” path is covered in CI and documented.

Changes:

• Added an optional postExecute phase to scenarios and runner plumbing so post-Stage-3 consumers (Conclusion) can run before assertions.
• Introduced a conclusion-cli wrapper plus four new conclusion-* E2E scenarios that run the compiled bundle, then assert/cleanup the resulting work items via ADO REST.
• Updated the executor-e2e pipeline and docs to build/pass the conclusion.js artifact and describe the new coverage.

File summaries

File	Description
tests/executor-e2e/README.md	Documents new Conclusion scenarios and local env requirements.
tests/executor-e2e/azure-pipelines.yml	Builds conclusion.js, passes its path to the harness, and updates PR path filters.
scripts/ado-script/src/executor-e2e/scenarios/index.ts	Registers the new Conclusion scenarios in deterministic order.
scripts/ado-script/src/executor-e2e/scenarios/conclusion.ts	Adds four end-to-end Conclusion scenarios asserting work-item filing/dedup/opt-out.
scripts/ado-script/src/executor-e2e/scenario.ts	Adds PostExecuteRun and optional postExecute hook to the scenario contract.
scripts/ado-script/src/executor-e2e/runner.ts	Executes postExecute after a successful ado-aw execute, with skip/failure handling.
scripts/ado-script/src/executor-e2e/execute-cli.ts	Returns safeOutputDir and exports spawnCollect for shared hang protection.
scripts/ado-script/src/executor-e2e/conclusion-cli.ts	New wrapper that runs compiled conclusion.js with the compiler’s flat AW_* env contract and skips when absent.
scripts/ado-script/src/executor-e2e/tests/runner.test.ts	Adds unit tests pinning postExecute ordering, error handling, and skip behavior.
scripts/ado-script/src/executor-e2e/tests/conclusion-cli.test.ts	Adds unit tests for bundle resolution, env contract passing, and non-zero exit behavior.
docs/conclusion.md	Documents unit vs end-to-end test coverage for Conclusion reporting.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

✅ TypeScript Code Quality Reviewer completed the TypeScript code quality review.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer

[github-actions]

✅ PR Security Reviewer completed the security review.

🔒 Security review by PR Security Reviewer

[github-actions]

✅ Compiler Contract Reviewer completed the compiler contract review.

Compiler Contract Reviewer: no findings. PR #2110 only touches the executor-e2e TypeScript test harness (post-execute hook in runner.ts/scenario.ts/execute-cli.ts, new conclusion-cli.ts spawner, scenarios/conclusion.ts) plus docs/conclusion.md and the pipeline path filter. No front-matter grammar, safe-output Params types, typed IR, generated ShellScript bodies, or codegen-tracked files (types.gen.ts / fact-catalog.gen.json) were touched, so none of the drift or contract checks in my remit apply. Docs update (docs/conclusion.md) correctly accompanies the new test coverage.

🏗️ Compiler contract review by Compiler Contract Reviewer

[github-actions]

✅ Rust Code Quality Reviewer completed the Rust code quality review.

No Rust files (src/, ado-aw-derive/, Rust tests, Cargo.toml) appear in this PR's diff — all changes are TypeScript (scripts/ado-script/src/executor-e2e/**), plus docs/conclusion.md and tests/executor-e2e/azure-pipelines.yml/README.md. This falls entirely under the TypeScript reviewer's remit, so no Rust code quality review action was taken.

🦀 Rust code quality review by Rust Code Quality Reviewer

[github-actions]

✅ Test Quality Sentinel completed the test quality analysis.

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

Security review: this PR only adds/extends the executor-e2e test harness (scripts/ado-script/src/executor-e2e/**, tests/executor-e2e/**), which is explicitly not shipped in ado-script.zip, plus a doc update. No compiler code (src/compile/**), sanitizer/validator (src/sanitize.rs, src/validate.rs), typed IR, or ado_bundle.rs token-projection logic is touched.

The new runConclusion() in conclusion-cli.ts passes SYSTEM_ACCESSTOKEN to the child process purely via env, following the exact same pattern already used for ado-aw execute in execute-cli.ts — it's never interpolated into a command string or logged (only stdout/stderr of the conclusion.js process itself are logged, which is expected harness output, not the token). The azure-pipelines.yml change reuses the existing SC_WRITE_TOKEN service-connection pattern for the new EXECUTOR_E2E_CONCLUSION_BUNDLE build step, with no new secret exposure.

No injection into generated ADO YAML, no path-traversal-prone new file params, no widened network allowlist, and no new safe-output write path bypassing Stage 3 — this is test-only tooling that exercises the existing signal → manifest → work-item flow rather than changing it. Security-neutral.

🔒 Security review by PR Security Reviewer · auto · 27.7 AIC · ⌖ 10.6 AIC · ⊞ 11.7K
_{Comment /review to run again}

[github-actions]

TypeScript review — scripts/ado-script/src/executor-e2e/

Scoped to the new test-harness code (conclusion-cli.ts, runner.ts, execute-cli.ts, scenarios/conclusion.ts). This is not shipped in the production ado-script.zip bundle.

Overall solid: SkipError paths are handled correctly, postExecute failures are distinguished from skips, cleanup always runs via finally, and external work-item fields are validated with fieldText before use rather than blindly cast. No unhandled rejections or any leakage found.

One advisory note left inline about verbatim stdout/stderr logging of the conclusion bundle's output combined with token injection into its env — low risk today since conclusion.js doesn't echo secrets, but worth a defensive redaction given this is a credential-bearing subprocess.

Note: the ts-critic sub-agent returned no output after the blocking wait, so this review reflects my own pass only.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer · auto · 67.5 AIC · ⌖ 2.61 AIC · ⊞ 11.6K
_{Comment /review to run again}

Comments that could not be inline-anchored

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

stdout/stderr of the conclusion bundle are logged verbatim to the harness log. Since runConclusion injects SYSTEM_ACCESSTOKEN into the child's env, any future error path in conclusion.js that echoes env or a failed request URL containing the token would leak it into CI logs here.

<details><summary>💡 Consider redacting</summary>

A simple mitigation is to strip/replace opts.token occurrences before logging, e.g. stdout.replaceAll(opts.token, "***"), mirroring how other e2e log paths …

[github-actions]

Solid test additions for the runner-level postExecute hook — the three runner.test.ts cases correctly pin ordering (post-execute → assert → cleanup), failure (no assert, cleanup still runs), and skip handling, and conclusion-cli.test.ts pins the flat AW_* env contract with a fake bundle plus the non-zero-exit failure path.

The gap is in scenarios/conclusion.ts itself (313 new lines, zero unit tests): the shared assertion helper assertFiledWorkItem, the dedup/append-comment-count check in conclusionMissingTool, and toolConfig/fieldText are only exercised by the live ADO lane, which the PR body says skips locally. Other scenario files (e.g. work-item.ts) have a companion __tests__/*-scenarios.test.ts that unit-tests these pure/stubbable helpers against a fakeCtx(); this file doesn't get that treatment. Left as advisory since the live E2E lane does cover the behavior end-to-end, but a local/fast regression signal is missing for a fairly intricate dedup assertion.

🧪 Test quality analysis by Test Quality Sentinel · auto · 66.2 AIC · ⌖ 2.68 AIC · ⊞ 11.3K
_{Comment /review to run again}

Comments that could not be inline-anchored

scripts/ado-script/src/executor-e2e/scenarios/conclusion.ts:119

assertFiledWorkItem, fieldText, toolConfig, conclusionState and the dedup-comment logic in conclusionMissingTool.assert have no deterministic unit test — this whole 313-line file is only exercised by the live ADO E2E lane, which the PR description says "skip[s] locally without a built bundle," so a regression here (e.g. a typo in a field name, or the dedup-count check) would ship silently until the nightly AgentPlayground run.

<details><summary>💡 Suggestion</summary>

Other scenario…

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

The timeout error path for spawnCollect's new label parameter (used for conclusion.js) is untested — only the exit-code-3 case is covered in conclusion-cli.test.ts. A regression that dropped the label back to the hardcoded "ado-aw execute" string, or broke the partialOutput interpolation for this call site, would ship silently.

<details><summary>💡 Suggestion</summary>

Add a case mirroring the existing exit-code test but with a bundle that sleeps past a very short `EXECUTOR_E2E_EX…