diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 08569ca7..dee40da7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -77,7 +77,7 @@ jobs: run: | set -euo pipefail cd scripts - zip -r ../ado-script.zip ado-script/gate.js ado-script/import.js ado-script/exec-context-pr.js ado-script/exec-context-pr-synth.js ado-script/exec-context-manual.js ado-script/exec-context-pipeline.js ado-script/exec-context-ci-push.js ado-script/exec-context-workitem.js ado-script/exec-context-schedule.js ado-script/exec-context-pr-checks.js ado-script/exec-context-repo.js + zip -r ../ado-script.zip ado-script/gate.js ado-script/import.js ado-script/exec-context-pr.js ado-script/exec-context-pr-synth.js ado-script/exec-context-manual.js ado-script/exec-context-pipeline.js ado-script/exec-context-ci-push.js ado-script/exec-context-workitem.js ado-script/exec-context-schedule.js ado-script/exec-context-pr-checks.js ado-script/exec-context-repo.js ado-script/approval-summary.js - name: Upload release assets env: diff --git a/AGENTS.md b/AGENTS.md index 7759c11f..81c57c92 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,6 +39,15 @@ Every compiled pipeline runs as three sequential jobs: 3. **SafeOutputs (Stage 3)** — a non-agent executor applies approved safe outputs using a write-capable ADO token that the agent never sees. +**Optional manual review.** When a safe output is configured with +`require-approval` (see [`docs/safe-outputs.md`](docs/safe-outputs.md)), an +agentless `ManualReview` job (`pool: server`, `ManualValidation@1`) is inserted +between Detection and SafeOutputs to pause for human approval. With a mix of +gated and non-gated outputs, Stage 3 splits into an automatic `SafeOutputs` job +(applies non-gated outputs immediately) and a `SafeOutputs_Reviewed` job (gated +behind `ManualReview`, publishes `safe_outputs_reviewed`). The gate is +fail-closed and only pauses when the agent actually proposed a reviewed output. + ### Architecture ``` @@ -50,7 +59,7 @@ Every compiled pipeline runs as three sequential jobs: │ ├── compile/ # Pipeline compilation module │ │ ├── mod.rs # Module entry point and Compiler trait │ │ ├── common.rs # Shared helpers across targets -│ │ ├── agentic_pipeline.rs # Canonical Setup → Agent → Detection → SafeOutputs → Teardown shape (shared by every target); BuiltPipelineContext, build_pipeline_context, build_canonical_jobs, per-job builders, fold_agent_conditions, agent_job_variables_hoist +│ │ ├── agentic_pipeline.rs # Canonical Setup → Agent → Detection → (ManualReview?) → SafeOutputs(+SafeOutputs_Reviewed?) → Teardown shape (shared by every target); BuiltPipelineContext, build_pipeline_context, build_canonical_jobs, per-job builders incl. build_manual_review_job + SafeOutputsVariant split, fold_agent_conditions, agent_job_variables_hoist │ │ ├── standalone.rs # Standalone pipeline compiler │ │ ├── standalone_ir.rs # Standalone target typed-IR builder │ │ ├── onees.rs # 1ES Pipeline Template compiler @@ -249,6 +258,7 @@ Every compiled pipeline runs as three sequential jobs: │ ├── exec-context-schedule/ # Scheduled-run context source (bundled to exec-context-schedule.js) │ ├── exec-context-pr-checks/ # PR validation checks context source (bundled to exec-context-pr-checks.js) │ ├── exec-context-repo/ # Repository identity context source (bundled to exec-context-repo.js) +│ ├── approval-summary/ # Safe-outputs summary renderer (bundled to approval-summary.js; end-of-Agent-job summary tab) │ └── shared/ # Shared modules across bundles (auth, ado-client, env-facts, types.gen.ts) ├── tests/ # Integration tests and fixtures ├── docs/ # Per-concept reference documentation (see index below) diff --git a/docs/ado-script.md b/docs/ado-script.md index 97cc1ef4..efbe4ff6 100644 --- a/docs/ado-script.md +++ b/docs/ado-script.md @@ -3,7 +3,7 @@ `ado-script` is the umbrella name for the TypeScript workspace at [`scripts/ado-script/`](../scripts/ado-script/). It produces small, ncc-bundled Node programs that the **compiler injects into every emitted -pipeline** as runtime helpers. Today it produces eleven bundles: +pipeline** as runtime helpers. Today it produces twelve bundles: - `gate.js` — trigger-filter gate evaluator (Setup job). - `import.js` — runtime prompt resolver described in @@ -45,6 +45,13 @@ pipeline** as runtime helpers. Today it produces eleven bundles: branch, SHA, last release tag, and commits-since-tag facts under `aw-context/repo/` (Agent job; see [`execution-context.md`](execution-context.md)). +- `approval-summary.js` — Safe-outputs summary renderer that runs at the + **end of the Agent job** (after proposals are collected). It reads the + proposed safe outputs from `safe_outputs.ndjson`, renders a sanitized + per-tool markdown summary (pending-approval proposals first when manual + review is configured), and attaches it to the build's + `ado-aw-safe-outputs` summary tab via `##vso[task.uploadsummary]`. See + [`safe-outputs.md`](safe-outputs.md). > **Internal-only.** `ado-script` is not a user-facing front-matter > feature. Authors never write an `ado-script:` block in their agent diff --git a/docs/cli.md b/docs/cli.md index beea16b6..39a970fe 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -36,6 +36,8 @@ Global flags (apply to all subcommands): `--verbose, -v` (enable info-level logg - `--ado-org-url ` - Azure DevOps organization URL override - `--ado-project ` - Azure DevOps project name override - `--dry-run` - Validate inputs but skip ADO API calls (useful for local testing and QA review) + - `--only ` - Execute only these safe-output tools (repeatable). Used by the manual-review split for the approval-gated `SafeOutputs_Reviewed` job. + - `--exclude ` - Skip these safe-output tools (repeatable). Used by the manual-review split so the automatic `SafeOutputs` job applies non-gated outputs while reviewed ones wait. See [`docs/safe-outputs.md`](safe-outputs.md#manual-review-require-approval). - `configure` *(deprecated; hidden in --help)* - Alias forwarding to `secrets set GITHUB_TOKEN`. Existing scripts keep working but get a stderr warning. diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index 7edd76e0..21c3ad00 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -37,6 +37,123 @@ safe-outputs: Safe output configurations are passed to Stage 3 execution and used when processing safe outputs. +### Manual review (`require-approval`) + +High-impact safe outputs can be gated behind a human approval step +(`ManualValidation@1`) that pauses the run until a reviewer approves or rejects +in the Azure DevOps UI. This lets agents propose more consequential actions +(PRs, branches, queued builds, work items) safely. + +Set `require-approval` at the **section level** for a pipeline-wide default, +and/or inside an **individual tool** to override the default for that tool: + +```yaml +safe-outputs: + require-approval: true # global default: every output below needs review + create-pull-request: + target-branch: main + add-pr-comment: + require-approval: false # …except low-impact comments, which auto-apply +``` + +`require-approval` accepts either a bare boolean or an object for finer control: + +```yaml +safe-outputs: + create-pull-request: + require-approval: + approvers: ["[MyOrg]\\release-team"] # who may approve (empty → anyone with run permission) + notify-users: ["ops@example.com"] # who is emailed (empty → no email) + timeout-minutes: 120 # pending period before on-timeout fires (omit → pipeline default) + on-timeout: reject # reject (default, fail-closed) | resume + instructions: "Verify the proposed PR before approving." +``` + +Resolution per tool: the tool's own `require-approval` wins; otherwise the +section-level `require-approval` applies; otherwise the tool is **not** gated. + +**Defaults (bare `require-approval: true`)** — the run pauses on a Review panel; +**anyone with run permission** can approve or reject; **no** notification emails +are sent; and the validation **fails closed** on timeout (`on-timeout: reject`), +so un-approved outputs are never applied. + +**Timeout (`timeout-minutes` / `on-timeout`)** — `timeout-minutes` bounds the +`ManualValidation@1` task's pending period; when it elapses the task applies +`on-timeout` (`reject` by default, or `resume` to auto-approve). The agentless +`ManualReview` job carries a slightly larger outer timeout as a hard bound, so a +job-level cancellation never preempts the task's graceful `on-timeout` handling +(in particular, `on-timeout: resume` reliably auto-approves rather than being +cancelled). Omit `timeout-minutes` to inherit the pipeline default. + +**Reviewer message** — set `instructions` to control the text shown in the +Review panel and notification emails. It is plain text and supports pipeline +variable (`$(...)`) interpolation. When omitted, ado-aw generates a default +message listing the reviewed safe-output type(s) awaiting approval. A run uses a +**single** `ManualReview` gate covering every reviewed tool: the gate message +**lists every reviewed tool** and aggregates **all** author-supplied per-tool +`instructions` (grouped when identical), so no tool's note is dropped when +several are gated. A single reviewed tool with its own `instructions` shows that +message verbatim; set `instructions` on the section-level `require-approval` to +apply one note to every tool. + +**Execution shape** — manual review changes the compiled pipeline: + +- A new agentless `ManualReview` job (`pool: server`) runs `ManualValidation@1` + between Detection and the safe-output execution. +- It only pauses when Detection cleared the run (no prompt-injection / secret + leak) **and** the agent actually proposed a reviewed-type output (a Detection + step sets a `HasReviewedProposals` flag) — so the run never pauses for + nothing. +- When some tools are gated and others are not, execution **splits**: an + automatic `SafeOutputs` job applies the non-gated outputs immediately + (independent of the review outcome), while a separate `SafeOutputs_Reviewed` + job — gated behind `ManualReview` — applies the approved outputs and publishes + a distinct `safe_outputs_reviewed` artifact. A rejected or timed-out review + fails closed: the reviewed job is skipped while the automatic outputs are + unaffected. +- When **every** configured tool requires approval (no automatic tools), + execution is **not** split — the single `SafeOutputs` job is gated behind + `ManualReview` in its entirety. Note this also defers the always-enabled + diagnostic outputs (`noop`, `report_incomplete`, `missing-tool`, + `missing-data`) until after approval, since they share that one job. If you + want diagnostics to apply without waiting on a human, leave at least one + low-impact tool (e.g. `add-pr-comment`) non-gated so the automatic split job + is created. + +The Detection threat gate always runs first, so a flagged run applies nothing — +automatic or reviewed. + +### Safe-outputs summary tab + +Every run that proposes safe outputs publishes a human-readable **build summary +tab** titled **`ado-aw-safe-outputs`**, listing what the agent proposed. This is +always on — it does **not** require `require-approval` — so non-elevated runs get +the same transparency, and it is the panel a reviewer reads before approving a +gated run. + +- The summary is rendered at the **end of the Agent job** (the job that produced + the proposals) by the `approval-summary` ado-script bundle, and attached via + `##vso[task.uploadsummary]`. It is **not** produced by the Detection + (threat-analysis) stage, whose only job is inspecting proposals for threats. +- Each proposal is shown with per-tool key fields (e.g. PR title + target branch, + work-item title) plus a truncated excerpt of any long body. All content is + **agent-generated** and is sanitized for display (markdown/HTML escaped, code + fences neutralised, control characters stripped, long values truncated) so a + proposal cannot forge UI or break the layout. +- When manual review is configured, the **pending-approval** proposals are listed + first (under a `⏳ Pending approval` heading), followed by the automatic ones. + With no approval configured, a single list is shown. The default review + message points approvers at this tab. +- Rendering is best-effort: if it fails it is logged as a warning and never fails + the build or blocks the review gate. + +**Coexistence with your own summary tabs.** ADO derives a summary section's title +from the uploaded file's base name and does not de-duplicate, so this feature uses +a namespaced base name (`ado-aw-safe-outputs.md` → the `ado-aw-safe-outputs` +section). It is additive and build-scoped: it appears as one extra section +alongside any `task.uploadsummary` tabs your own steps publish (including under +`target: job` / `target: stage`), and never collides with them. + ### Executor authentication All write-bearing safe outputs (e.g. `create-pull-request`, diff --git a/scripts/ado-script/.gitignore b/scripts/ado-script/.gitignore index 275e17a3..73629344 100644 --- a/scripts/ado-script/.gitignore +++ b/scripts/ado-script/.gitignore @@ -11,5 +11,6 @@ exec-context-workitem.js exec-context-schedule.js exec-context-pr-checks.js exec-context-repo.js +approval-summary.js schema *.tsbuildinfo diff --git a/scripts/ado-script/package.json b/scripts/ado-script/package.json index 39121319..00f8d61a 100644 --- a/scripts/ado-script/package.json +++ b/scripts/ado-script/package.json @@ -7,8 +7,8 @@ "node": ">=20.0.0" }, "scripts": { - "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo", - "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); for (const n of ['gate','import','exec-context-pr','exec-context-pr-synth','exec-context-manual','exec-context-pipeline','exec-context-ci-push','exec-context-workitem','exec-context-schedule','exec-context-pr-checks','exec-context-repo']) fs.rmSync(n+'.js',{force:true});\"", + "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:approval-summary", + "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); for (const n of ['gate','import','exec-context-pr','exec-context-pr-synth','exec-context-manual','exec-context-pipeline','exec-context-ci-push','exec-context-workitem','exec-context-schedule','exec-context-pr-checks','exec-context-repo','approval-summary']) fs.rmSync(n+'.js',{force:true});\"", "build:gate": "ncc build src/gate/index.ts -o .ado-build/gate -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/gate/index.js','gate.js'); fs.rmSync('.ado-build/gate',{recursive:true,force:true});\"", "build:import": "ncc build src/import/index.ts -o .ado-build/import -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/import/index.js','import.js'); fs.rmSync('.ado-build/import',{recursive:true,force:true});\"", "build:exec-context-pr": "ncc build src/exec-context-pr/index.ts -o .ado-build/exec-context-pr -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-pr/index.js','exec-context-pr.js'); fs.rmSync('.ado-build/exec-context-pr',{recursive:true,force:true});\"", @@ -20,10 +20,11 @@ "build:exec-context-schedule": "ncc build src/exec-context-schedule/index.ts -o .ado-build/exec-context-schedule -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-schedule/index.js','exec-context-schedule.js'); fs.rmSync('.ado-build/exec-context-schedule',{recursive:true,force:true});\"", "build:exec-context-pr-checks": "ncc build src/exec-context-pr-checks/index.ts -o .ado-build/exec-context-pr-checks -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-pr-checks/index.js','exec-context-pr-checks.js'); fs.rmSync('.ado-build/exec-context-pr-checks',{recursive:true,force:true});\"", "build:exec-context-repo": "ncc build src/exec-context-repo/index.ts -o .ado-build/exec-context-repo -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-repo/index.js','exec-context-repo.js'); fs.rmSync('.ado-build/exec-context-repo',{recursive:true,force:true});\"", + "build:approval-summary": "ncc build src/approval-summary/index.ts -o .ado-build/approval-summary -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/approval-summary/index.js','approval-summary.js'); fs.rmSync('.ado-build/approval-summary',{recursive:true,force:true});\"", "build:check": "ls -lh gate.js && wc -c gate.js", "codegen": "node -e \"require('node:fs').mkdirSync('schema', { recursive: true })\" && cargo run --quiet --manifest-path ../../Cargo.toml -- export-gate-schema --output schema/gate-spec.schema.json && npx json2ts schema/gate-spec.schema.json -o src/shared/types.gen.ts --bannerComment \"// AUTO-GENERATED from Rust IR via cargo run -- export-gate-schema. Do not edit; run npm run codegen.\"", "test": "vitest run", - "test:smoke": "npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && vitest run -c vitest.config.smoke.ts", + "test:smoke": "npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:approval-summary && vitest run -c vitest.config.smoke.ts", "lint": "echo TODO", "typecheck": "tsc --noEmit" }, diff --git a/scripts/ado-script/src/approval-summary/__tests__/index.test.ts b/scripts/ado-script/src/approval-summary/__tests__/index.test.ts new file mode 100644 index 00000000..eb874f24 --- /dev/null +++ b/scripts/ado-script/src/approval-summary/__tests__/index.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { mkdtempSync, readFileSync, writeFileSync, rmSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { main, parseReviewed } from "../index.js"; + +const dirs: string[] = []; +function freshDir(): string { + const d = mkdtempSync(join(tmpdir(), "approval-summary-")); + dirs.push(d); + return d; +} + +afterEach(() => { + for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }); +}); + +describe("parseReviewed", () => { + it("splits a newline-delimited list, trims, and drops empties", () => { + const set = parseReviewed(" create-pull-request \n \n add-pr-comment "); + expect([...set].sort()).toEqual(["add-pr-comment", "create-pull-request"]); + }); + + it("does not split on commas (a comma may appear in a YAML map key)", () => { + const set = parseReviewed("weird,tool-name"); + expect([...set]).toEqual(["weird,tool-name"]); + }); + + it("returns an empty set for undefined/empty", () => { + expect(parseReviewed(undefined).size).toBe(0); + expect(parseReviewed("").size).toBe(0); + }); +}); + +describe("main", () => { + it("writes a summary and returns 0 when proposals exist", () => { + const dir = freshDir(); + const ndjsonPath = join(dir, "safe_outputs.ndjson"); + const outPath = join(dir, "ado-aw-safe-outputs.md"); + writeFileSync( + ndjsonPath, + JSON.stringify({ name: "create-pull-request", title: "T" }) + "\n", + "utf8", + ); + const rc = main({ + AW_SAFE_OUTPUTS_NDJSON: ndjsonPath, + AW_APPROVAL_SUMMARY_OUT: outPath, + AW_REVIEWED_TOOLS: "create-pull-request", + } as NodeJS.ProcessEnv); + expect(rc).toBe(0); + expect(existsSync(outPath)).toBe(true); + expect(readFileSync(outPath, "utf8")).toContain("Pending approval (1)"); + }); + + it("is a no-op (exit 0, no file) when the proposals file is missing", () => { + const dir = freshDir(); + const outPath = join(dir, "ado-aw-safe-outputs.md"); + const rc = main({ + AW_SAFE_OUTPUTS_NDJSON: join(dir, "does-not-exist.ndjson"), + AW_APPROVAL_SUMMARY_OUT: outPath, + } as NodeJS.ProcessEnv); + expect(rc).toBe(0); + expect(existsSync(outPath)).toBe(false); + }); + + it("is a no-op when the proposals file has no valid records", () => { + const dir = freshDir(); + const ndjsonPath = join(dir, "safe_outputs.ndjson"); + const outPath = join(dir, "ado-aw-safe-outputs.md"); + writeFileSync(ndjsonPath, "\n\nnot json\n", "utf8"); + const rc = main({ + AW_SAFE_OUTPUTS_NDJSON: ndjsonPath, + AW_APPROVAL_SUMMARY_OUT: outPath, + } as NodeJS.ProcessEnv); + expect(rc).toBe(0); + expect(existsSync(outPath)).toBe(false); + }); + + it("returns 0 without writing when required env is missing", () => { + const rc = main({} as NodeJS.ProcessEnv); + expect(rc).toBe(0); + }); +}); diff --git a/scripts/ado-script/src/approval-summary/__tests__/render.test.ts b/scripts/ado-script/src/approval-summary/__tests__/render.test.ts new file mode 100644 index 00000000..e45eb349 --- /dev/null +++ b/scripts/ado-script/src/approval-summary/__tests__/render.test.ts @@ -0,0 +1,251 @@ +import { describe, it, expect } from "vitest"; + +import { + BODY_MAX_CHARS, + parseProposals, + renderSummary, + sanitizeBlock, + sanitizeInline, + type Proposal, +} from "../render.js"; + +function ndjson(...records: Record[]): string { + return records.map((r) => JSON.stringify(r)).join("\n") + "\n"; +} + +describe("parseProposals", () => { + it("parses one proposal per non-blank line with a string name", () => { + const text = ndjson( + { name: "create-pull-request", title: "T" }, + { name: "add-pr-comment", content: "C" }, + ); + const out = parseProposals(text); + expect(out.map((p) => p.name)).toEqual([ + "create-pull-request", + "add-pr-comment", + ]); + expect(out.map((p) => p.index)).toEqual([0, 1]); + }); + + it("skips blank lines, malformed JSON, non-objects, and records with no name", () => { + const text = [ + "", + "not json", + JSON.stringify([1, 2, 3]), + JSON.stringify({ noName: true }), + JSON.stringify({ name: "" }), + JSON.stringify({ name: "noop", context: "ok" }), + " ", + ].join("\n"); + const out = parseProposals(text); + expect(out).toHaveLength(1); + expect(out[0]?.name).toBe("noop"); + }); +}); + +describe("sanitizeInline", () => { + it("escapes markdown/HTML/table metacharacters so content renders literally", () => { + const out = sanitizeInline("**bold** [x](y) | cell `code`"); + expect(out).not.toContain("**bold**"); + expect(out).toContain("\\*\\*bold\\*\\*"); + expect(out).toContain("\\|"); + // `<`/`>` are HTML-entity-encoded (renderer-agnostic), not backslash-escaped. + expect(out).toContain("<img>"); + expect(out).not.toContain("\\ { + const out = sanitizeInline("line1\nline2\tcol\u0000\u0007"); + expect(out).not.toMatch(/[\n\t\u0000\u0007]/); + expect(out).toContain("line1 line2 col"); + }); + + it("renders arrays as comma-joined values", () => { + expect(sanitizeInline(["a", "b", "c"])).toBe("a, b, c"); + }); + + it("truncates very long values", () => { + const out = sanitizeInline("x".repeat(5000)); + expect(out.length).toBeLessThan(5000); + expect(out).toContain("(truncated)"); + }); + + it("entity-encodes & so agent-supplied entities are shown literally", () => { + const out = sanitizeInline("Tom & Jerry <tag>"); + // The ampersands are encoded, so a browser cannot decode `<` back to `<`. + expect(out).toContain("&amp;"); + expect(out).toContain("&lt;"); + expect(out).not.toMatch(/<tag/); + }); +}); + +describe("sanitizeBlock", () => { + it("neutralises embedded code fences so the body cannot escape the block", () => { + const out = sanitizeBlock("before\n```\nbreakout\n```\nafter"); + expect(out).not.toContain("```"); + expect(out).toContain("breakout"); + }); + + it("preserves newlines but strips other control characters", () => { + const out = sanitizeBlock("a\nb\u0000\u0007c"); + expect(out).toContain("a\nb"); + expect(out).not.toMatch(/[\u0000\u0007]/); + }); + + it("truncates bodies longer than BODY_MAX_CHARS", () => { + const out = sanitizeBlock("y".repeat(BODY_MAX_CHARS + 500)); + expect(out.length).toBeLessThan(BODY_MAX_CHARS + 500); + expect(out).toContain("(truncated)"); + }); +}); + +describe("renderSummary — grouping/ordering", () => { + const proposals: Proposal[] = parseProposals( + ndjson( + { name: "add-pr-comment", pull_request_id: 5, content: "auto comment" }, + { name: "create-pull-request", title: "Reviewed PR", source_branch: "feat/x" }, + { name: "create-work-item", title: "Reviewed WI" }, + ), + ); + + it("lists pending-approval proposals BEFORE automatic ones", () => { + const reviewed = new Set(["create-pull-request", "create-work-item"]); + const md = renderSummary(proposals, reviewed); + const pendingIdx = md.indexOf("Pending approval"); + const autoIdx = md.indexOf("Automatic"); + expect(pendingIdx).toBeGreaterThan(-1); + expect(autoIdx).toBeGreaterThan(-1); + expect(pendingIdx).toBeLessThan(autoIdx); + // Reviewed tools appear in the pending section (before Automatic heading). + const pendingBlock = md.slice(pendingIdx, autoIdx); + expect(pendingBlock).toContain("create-pull-request"); + expect(pendingBlock).toContain("create-work-item"); + expect(pendingBlock).not.toContain("add-pr-comment"); + }); + + it("counts the pending and automatic groups", () => { + const reviewed = new Set(["create-pull-request", "create-work-item"]); + const md = renderSummary(proposals, reviewed); + expect(md).toContain("Pending approval (2)"); + expect(md).toContain("Automatic (1)"); + }); + + it("renders a single 'All proposals' list when nothing is reviewed", () => { + const md = renderSummary(proposals, new Set()); + expect(md).toContain("All proposals (3)"); + expect(md).not.toContain("Pending approval"); + expect(md).not.toContain("Automatic ("); + }); + + it("returns an empty string for no proposals", () => { + expect(renderSummary([], new Set())).toBe(""); + }); +}); + +describe("renderSummary — per-tool detail", () => { + it("uses tailored fields + a fenced body for a known tool", () => { + const md = renderSummary( + parseProposals( + ndjson({ + name: "create-pull-request", + title: "My PR", + source_branch: "feat/x", + repository: "self", + description: "Body line one\nBody line two", + }), + ), + new Set(), + ); + expect(md).toContain("Create pull request"); + expect(md).toContain("| Title | My PR |"); + expect(md).toContain("| Source branch | feat/x |"); + expect(md).toContain("```text"); + expect(md).toContain("Body line one"); + }); + + it("falls back to generic scalar fields for an unmapped tool", () => { + const md = renderSummary( + parseProposals( + ndjson({ name: "future-tool", alpha: "a", zeta: 9, obj: { x: 1 } }), + ), + new Set(), + ); + // Title-cased fallback heading. + expect(md).toContain("Future tool"); + // Scalar fields surfaced in sorted order; nested object skipped. + expect(md).toContain("| alpha | a |"); + expect(md).toContain("| zeta | 9 |"); + expect(md).not.toContain("obj"); + }); + + it("surfaces diagnostic-tool free-text in a fenced body", () => { + const md = renderSummary( + parseProposals( + ndjson( + { name: "noop", context: "Nothing to do.\nAll inputs were valid." }, + { name: "report-incomplete", reason: "Ran out of API quota." }, + { name: "missing-tool", tool_name: "kubectl", context: "needed for deploy" }, + { name: "missing-data", data_type: "schema", reason: "not provided" }, + ), + ), + new Set(), + ); + expect(md).toContain("`noop`"); + expect(md).toContain("```text"); + // noop's multi-line context goes in a fenced body, not a truncated cell. + expect(md).toContain("Nothing to do."); + expect(md).toContain("All inputs were valid."); + // report-incomplete surfaces its reason. + expect(md).toContain("`report-incomplete`"); + expect(md).toContain("Ran out of API quota."); + // missing-tool shows the tool field + context body. + expect(md).toContain("| Tool | kubectl |"); + // missing-data shows the data-type field + reason body. + expect(md).toContain("| Data type | schema |"); + }); +}); + +describe("renderSummary — security", () => { + it("does not let a crafted tool name break the heading code span", () => { + const md = renderSummary( + parseProposals(ndjson({ name: "foo\nbar`baz", title: "x" })), + new Set(), + ); + const heading = md.split("\n").find((l) => l.startsWith("#### ")); + expect(heading).toBeDefined(); + // Newline and backtick stripped from the name → it renders as a single + // clean code span on one line (if a newline survived, the heading would be + // split across lines and this exact span would not appear). + expect(heading).toContain("`foobarbaz`"); + }); + + it("does not let agent content forge UI or break out of the layout", () => { + const hostile = + "Looks fine | ✅ APPROVED | \n```\n## Fake heading"; + const md = renderSummary( + parseProposals( + ndjson({ + name: "create-pull-request", + title: hostile, + description: hostile, + }), + ), + new Set(["create-pull-request"]), + ); + // Inline title escaped: no raw pipe (would add a table column) or raw tag. + const titleRow = md.split("\n").find((l) => l.startsWith("| Title |")); + expect(titleRow).toBeDefined(); + expect(titleRow).toContain("\\|"); + // The tag is HTML-entity-encoded (renderer-agnostic), so no raw `<`/`>`. + expect(titleRow).toContain("<script>"); + expect(titleRow).not.toMatch(/