diff --git a/docs/concepts/triage-dispatch.md b/docs/concepts/triage-dispatch.md new file mode 100644 index 000000000..e987fe229 --- /dev/null +++ b/docs/concepts/triage-dispatch.md @@ -0,0 +1,240 @@ +# Ecosystem-Observe Triage Dispatch + +The **triage dispatch** stage converts an `observed_problems.ctx` handoff into a +single, validated JSON array of dispatch items. Each item is either a +**brief** (an actionable `smart-orchestrator` fix request targeting a specific +repo) or an **escalation** (a problem that cannot be fixed by an engineer in its +current state and requires manual operator intervention). + +This document describes the dispatch contract, its schema, ordering rules, +configuration, and worked examples. + +--- + +## Overview + +- **Input:** an `observed_problems.ctx` file emitted by the ecosystem observer + (a de-duplicated list of observed problems with severity, category, target + repo, and corroborating evidence). +- **Output:** one JSON array. Every element is a dispatch item. The array is + ordered **most-important first** (see [Ordering](#ordering)). +- **Side effects:** none. Dispatch is emit-only — it authors no code and mutates + no repository. Briefs are picked up by downstream `smart-orchestrator` runs; + escalations are routed to a human operator. + +The dispatch stage is deterministic given its input: the same +`observed_problems.ctx` always yields the same ordered array. + +--- + +## Dispatch item schema + +The output is a JSON array. Each element is one of two shapes. + +### Brief item + +An actionable fix request. Consumed by a `smart-orchestrator` run. + +The **canonical required set** is exactly four fields — `recipe`, +`task_description`, `target_repo`, and `success_criteria` — matching the design. +`is_mechanical_sweep` and `sequence_group` are **optional producer hints**; a +consumer must not require them and must treat an omitted hint as its default +(`is_mechanical_sweep` → `false`, `sequence_group` → `null`). + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `recipe` | string | yes | Recipe to run. For actionable fixes this is `"smart-orchestrator"`. | +| `task_description` | string | yes | Self-contained problem statement: observed symptom, suspected cause, smallest responsible surface, additive/non-breaking constraints, and merge-ready expectations. Must be understandable without the original `.ctx`. | +| `target_repo` | string | yes | `owner/name` of the repo the fix targets. | +| `success_criteria` | string[] | yes | Concrete, verifiable acceptance checks. Each item is one testable statement. | +| `is_mechanical_sweep` | boolean | no (hint) | `true` for repetitive mechanical edits across many files; `false` for a targeted fix. Defaults to `false` when omitted. | +| `sequence_group` | string \| null | no (hint) | Ordering group when a brief must run before/after siblings; `null` (or omitted) when independent. | + +### Escalation item + +A problem that is **not** engineer-fixable in its current state (for example, a +bootstrap deadlock where the fix itself requires resources that are currently +unavailable). Routed to a human operator instead of a recipe. + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `recipe` | null | yes | Always `null`. The presence of `null` recipe marks the item as an escalation. | +| `escalate` | string | yes | Human-actionable explanation: why it is not engineer-fixable, the exact manual remediation, and any follow-up bugs to file **after** the manual step unblocks the system. | + +> **Discriminator:** an item is an escalation **iff** `recipe` is `null`. A brief +> always has a non-null `recipe` and never carries an `escalate` field. + +--- + +## Ordering + +Items are ordered by **blast radius, most-critical first**: + +1. **Ecosystem-wide blockers** (e.g., a bootstrap deadlock that prevents *any* + engineer from spawning) come first, regardless of whether they are briefs or + escalations. +2. **Cross-cutting regressions** (affect many PRs / the whole default branch). +3. **Isolated regressions** (single subsystem, e.g., docs-pages-only). + +Escalations are ordered inline with briefs by the same blast-radius rule — an +escalation is not automatically first or last; it sits where its impact places +it. + +--- + +## Configuration + +The `NODE_OPTIONS` memory preference used by the observe/dispatch tooling is a +saved preference. Set it to whatever heap ceiling your host requires, for +example: + +``` +NODE_OPTIONS=--max-old-space-size= +``` + +To change it, edit the amplihack config in your home directory: + +``` +~/.amplihack/config +``` + +The dispatch stage reads its input path from the observe run's temp directory, +e.g.: + +``` +/tmp/simard-ecosystem-observe-ctx-*/observed_problems.ctx +``` + +No other configuration is required. Dispatch inherits repo/target settings from +each observed problem record. + +--- + +## Exclusions + +- Items already flagged `dropped_as_in_flight` in the input are **excluded** from + the dispatch array — they are being handled by an existing workstream. +- Dispatch never auto-remediates escalations (e.g., it will not run a disk + cleanup). Escalations are advisory to a human operator only. + +--- + +## Constraints applied to every brief + +All briefs are authored to be **additive / non-breaking by default**: + +- Do not remove or weaken required status checks or branch protections. +- Do not modify already-green CI (core CI, Auto Release). +- Preserve the PRD. +- No `Bridge` naming. +- No stray `print!` / `println!` in new code — structured `tracing` + OpenTelemetry only. +- **Least privilege for CI/workflow changes.** Any workflow permissions a brief + introduces or edits stay at the minimum required and scoped to the job — e.g. + a GitHub Pages deploy uses `permissions: { pages: write, id-token: write }` + at job scope, never broadened. Branch-protection/merge-queue changes are + additive-only and never weaken existing required checks. + +--- + +## Examples + +### Example 1 — Escalation (bootstrap deadlock) + +A critical resource-pressure problem where the fix requires an engineer, but no +engineer can be admitted until disk is manually freed. Emitted as an escalation, +with two follow-up bugs to file **after** the manual step: + +```json +{ + "recipe": null, + "escalate": "Problem 1 (critical, resource_pressure, rysweet/Simard, dedup process:disk_full:root_fs) is NOT engineer-fixable in its current state: root fs / is 100% full (28G/28G, 0 avail), so the disk-admission ceiling (max_disk_used_percent=90) rejects every engineer spawn. This is a bootstrap deadlock: the fix requires an engineer, but no engineer can be admitted until disk is freed. Requires MANUAL operator intervention first: reclaim the ~2.78 GiB of stale ~/.simard/cognitive.corrupt-*/bad-*/predeploy-*/pr*-canary-* snapshots to drop usage below 90% and unblock #4803 / cycle-2630. AFTER disk is freed, file two follow-up bugs: (a) the periodic reclaim job frees 0 bytes because it perpetually defers stale snapshots 'for review' instead of collecting them; (b) `simard status` mis-reports '/home 25 GiB free' via a statvfs f_bavail illusion while df shows root full." +} +``` + +### Example 2 — Brief (cross-cutting merge live-lock) + +```json +{ + "recipe": "smart-orchestrator", + "task_description": "In target_repo rysweet/amplihack-rs, fix the CI/merge strict up-to-date branch-protection live-lock tracked in issue #1050. Required 'branch must be up to date before merging' protection forces every PR to rebase/re-run CI whenever any other PR merges, causing a merge live-lock (evidence: open PR #1063 sitting in mergeStateStatus=BEHIND while mergeable=MERGEABLE). Smallest surface: the repo's branch-protection / merge-queue config and CI workflows under .github/workflows that gate merges — enable a GitHub merge queue (merge_group trigger + 'Require merge queue') so PRs are batched and tested serially. Wire required status checks to run on the merge_group event. Additive / non-breaking: do not remove existing required checks or weaken protection; preserve the PRD; no Bridge naming; no stray print!/println! (tracing + OTel only). Link the resulting PR to issue #1050.", + "target_repo": "rysweet/amplihack-rs", + "is_mechanical_sweep": false, + "sequence_group": null, + "success_criteria": [ + "CI green on all required checks (including on the merge_group event)", + "GitHub merge queue enabled and required status checks wired to merge_group; PR live-lock relieved", + "additive / non-breaking; existing required checks and protections preserved; PRD preserved", + "no Bridge naming; no stray print!/println! in new code (tracing + OTel only)", + "docs/CONTRIBUTING updated with the merge-queue flow and links; PR linked to issue #1050" + ] +} +``` + +### Example 3 — Brief (isolated docs-Pages regression) + +```json +{ + "recipe": "smart-orchestrator", + "task_description": "In target_repo rysweet/amplihack-recipe-runner, fix the red default branch: the 'Deploy mdBook to GitHub Pages' workflow build check failed on the head commit (failing run 30187028559 / job 89753457708). Fresh regression, not chronic — prior 4 runs of that workflow succeeded; core CI and Auto Release were green on the same push, so blast radius is docs-pages-only. Smallest surface: the .github/workflows/ mdBook/GitHub-Pages deploy workflow and the book source it builds (book.toml, docs/ or book/ SUMMARY.md and referenced markdown) — inspect the failing job log for the actual build error (missing/renamed page in SUMMARY.md, broken intra-book link, mdBook version pin drift, or preprocessor change) and correct the root cause. Additive / non-breaking: restore the docs deploy without altering unrelated CI; preserve the PRD; no Bridge naming; no stray print!/println! (tracing + OTel only).", + "target_repo": "rysweet/amplihack-recipe-runner", + "is_mechanical_sweep": false, + "sequence_group": null, + "success_criteria": [ + "CI green on all required checks", + "'Deploy mdBook to GitHub Pages' workflow build check passes on main head", + "additive / non-breaking; only docs-pages deploy restored, unrelated CI untouched; PRD preserved", + "no Bridge naming; no stray print!/println! in new code (tracing + OTel only)", + "docs/link updates included; quality-audit cycles pass" + ] +} +``` + +--- + +## Full dispatch array (worked end-to-end) + +The three examples above compose into one ordered array (P1 escalation → +P3 cross-cutting brief → P2 isolated brief): + +```json +[ + { "recipe": null, "escalate": "…Problem 1 bootstrap deadlock…" }, + { "recipe": "smart-orchestrator", "target_repo": "rysweet/amplihack-rs", "task_description": "…merge queue fix…", "is_mechanical_sweep": false, "sequence_group": null, "success_criteria": ["…"] }, + { "recipe": "smart-orchestrator", "target_repo": "rysweet/amplihack-recipe-runner", "task_description": "…mdBook Pages fix…", "is_mechanical_sweep": false, "sequence_group": null, "success_criteria": ["…"] } +] +``` + +--- + +## Validation rules + +A dispatch array is valid iff **all** of the following hold: + +1. Top-level value is a **non-empty** JSON array. +2. Every element is either a **brief** (non-null `recipe`, all four + canonical required fields present — `recipe`, `task_description`, + `target_repo`, `success_criteria` — and no `escalate` field) or an + **escalation** (`recipe` is `null`, `escalate` is a non-empty string). + Optional producer hints (`is_mechanical_sweep`, `sequence_group`) may be + present or absent and are not required for validity. +3. Every brief's `target_repo` is a well-formed `owner/name`. +4. Every brief's `success_criteria` is a non-empty array of non-empty strings. +5. Elements are ordered by blast radius, most-critical first. +6. No element corresponds to a `dropped_as_in_flight` input problem. +7. The array contains no credential-shaped secrets — a heuristic check for + token shapes (e.g. `ghp_`, `AKIA`, PEM blocks, inline bearer tokens). PII is + **producer-trust**: because dispatch JSON is plaintext and human-readable, + producers must not emit personal data, but this is not (and cannot be) + machine-enforced by the contract. + +Consumers should reject any array that fails these checks before acting on it. + +Rules 1–4 are **self-contained** — checkable from the array alone. Rule 7's +credential-shape heuristic is likewise self-contained; its PII clause is +producer-trust rather than machine-checked. Rules 5 (blast-radius ordering) and +6 (`dropped_as_in_flight` exclusion) are **source-relative**: they can only be +fully verified against the originating `observed_problems.ctx`, since +severity/blast-radius and in-flight status are properties of the input records, +not the emitted items. A consumer without the source `.ctx` can enforce 1–4 and +the rule 7 credential-shape heuristic but must trust the producer for 5–6 and +for PII. diff --git a/mkdocs.yml b/mkdocs.yml index caa691262..b500a4f50 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -100,6 +100,7 @@ nav: - Gap-Scan Dedup & Backoff: concepts/gap-scan-backoff-dedup.md - Autonomous Self-Merge Sensor (ready_prs wire): concepts/autonomous-self-merge-sensor.md - Agentic Merge-Queue + Issue Reasoning (observe/orient): concepts/agentic-merge-queue-reasoning.md + - Ecosystem-Observe Triage Dispatch (briefs + escalations): concepts/triage-dispatch.md - Overseer Agentic Health-Review (self-heal crash-loops): concepts/overseer-agentic-health-review.md - Autonomous-Merge Review Gate (agentic merge-judge): concepts/autonomous-merge-review-gate.md - Adaptive Scaling: concepts/adaptive-scaling.md diff --git a/prompt_assets/simard/overseer/problem_to_brief.md b/prompt_assets/simard/overseer/problem_to_brief.md index dd47ef9c2..32a52dd47 100644 --- a/prompt_assets/simard/overseer/problem_to_brief.md +++ b/prompt_assets/simard/overseer/problem_to_brief.md @@ -54,7 +54,7 @@ A good `task_description`: `tracing` + OTel only). 4. **Carries the merge-ready expectation** — CI green, tests, docs/link updates, quality-audit cycles — so the workflow finishes merge-ready, not half-done. -5. **Declares a sequence group** when the fix is a *mechanical sweep* on shared +5. **May declare a sequence group** when the fix is a *mechanical sweep* on shared OODA-core files (renames, `print!` purges). These run one-at-a-time; feature fixes may run in parallel. This prevents the Overseer's workstreams from colliding with each other. @@ -62,24 +62,32 @@ A good `task_description`: ## OUTPUT Emit one JSON object **per actionable Problem**, most-important first (a JSON -array is fine). Each has the shape: +array is fine). The **canonical required fields** are exactly four — `recipe`, +`task_description`, `target_repo`, and `success_criteria`. `is_mechanical_sweep` +and `sequence_group` are **OPTIONAL producer hints**: include them only when +they add signal, and omit them otherwise. When a hint is omitted, its default +applies — `is_mechanical_sweep` defaults to `false` and `sequence_group` +defaults to `null`. Consumers must not require the optional hints. ```json { "recipe": "smart-orchestrator", "task_description": "the full brief, ready to pass verbatim as -c task_description=...", "target_repo": "owner/name (from the Problem)", - "is_mechanical_sweep": true, - "sequence_group": "ooda-core | null", "success_criteria": [ "CI green on all required checks", "additive / non-breaking; PRD preserved", "no Bridge naming; no stray print! in new code", "..." - ] + ], + "is_mechanical_sweep": false, + "sequence_group": null } ``` +The two trailing fields above are the OPTIONAL hints (shown with their default +values `false` / `null`); drop them entirely when they carry no signal. + Keep `task_description` self-contained: an engineer with no other context should be able to act on it, and it must name the `target_repo` in its prose. If a problem is `resource_pressure` (budget) or otherwise **not** a code fix, do diff --git a/tests/ecosystem_dispatch_contract.rs b/tests/ecosystem_dispatch_contract.rs new file mode 100644 index 000000000..eff0bd949 --- /dev/null +++ b/tests/ecosystem_dispatch_contract.rs @@ -0,0 +1,614 @@ +//! TDD contract tests for the ecosystem-observe **triage dispatch** stage +//! (issue #4803). +//! +//! The dispatch stage converts an `observed_problems.ctx` handoff into ONE +//! validated JSON array whose elements are either **briefs** (actionable +//! `smart-orchestrator` fix requests) or **escalations** (`recipe: null` +//! problems routed to a human operator). Like the rest of the agentic +//! observe/brief chain, the array is authored by the BRIEF agent — no Rust +//! ever parses the live handoff — so these tests PIN THE CONTRACT the way +//! `tests/ecosystem_observe_assets.rs` does: they assert the documentation and +//! the BRIEF prompt encode the schema/ordering/validation rules, and they +//! provide a self-contained validator + a canonical machine-checkable fixture +//! so the emitted contract is executable, not just prose. +//! +//! These tests are written FIRST (TDD). They fail until the implementation: +//! 1. lands the reconciled `docs/concepts/triage-dispatch.md` doc (linked in +//! `mkdocs.yml`), +//! 2. reconciles the BRIEF prompt so `is_mechanical_sweep` / `sequence_group` +//! are documented as OPTIONAL producer hints with defaults, and +//! 3. ships the canonical worked dispatch array as a checked-in fixture at +//! `tests/fixtures/ecosystem_dispatch/canonical.json`. +//! +//! Rules encoded (from the dispatch contract): +//! 1. Top-level value is a non-empty JSON array. +//! 2. Every element is a brief (non-null `recipe`, the four canonical +//! required fields present, NO `escalate` field) OR an escalation +//! (`recipe` is `null`, `escalate` is a non-empty string). +//! 3. Every brief `target_repo` is a well-formed `owner/name`. +//! 4. Every brief `success_criteria` is a non-empty array of non-empty strings. +//! 5. Elements are ordered by blast radius, most-critical first. +//! 6. No element corresponds to a `dropped_as_in_flight` input problem. +//! 7. The array carries no credential-shaped secrets (heuristic token-shape +//! check for shapes like `ghp_`, `AKIA`, PEM blocks, inline bearer, etc.). +//! PII is producer-trust — it is not, and cannot be, machine-enforced here. +//! +//! Rules 1–4 are self-contained (checkable from the array alone) and are +//! exercised by the in-test `validate_dispatch_array` spec. Rule 7's +//! credential-shape heuristic is likewise self-contained (see `find_secret`), +//! though its PII clause is producer-trust. Rules 5–6 are source-relative; +//! rule 5's *intent* is exercised by `is_blast_radius_ordered`. + +use std::fs; +use std::path::PathBuf; + +use serde_json::{Value, json}; + +// --------------------------------------------------------------------------- +// Asset helpers +// --------------------------------------------------------------------------- + +fn repo_path(rel: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(rel) +} + +/// Read a required asset, panicking with a helpful message if absent. +fn asset(rel: &str) -> String { + let path = repo_path(rel); + fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("asset {} must be readable: {e}", path.display())) +} + +/// Read an asset that may not exist yet (used for failing-first existence pins). +fn try_asset(rel: &str) -> Option { + fs::read_to_string(repo_path(rel)).ok() +} + +const DOC: &str = "docs/concepts/triage-dispatch.md"; +const BRIEF_PROMPT: &str = "prompt_assets/simard/overseer/problem_to_brief.md"; +const CANONICAL_FIXTURE: &str = "tests/fixtures/ecosystem_dispatch/canonical.json"; +const MKDOCS: &str = "mkdocs.yml"; + +const REQUIRED_BRIEF_FIELDS: [&str; 4] = [ + "recipe", + "task_description", + "target_repo", + "success_criteria", +]; +const OPTIONAL_HINTS: [&str; 2] = ["is_mechanical_sweep", "sequence_group"]; + +// =========================================================================== +// GROUP A — Documentation contract pins (the reconciled doc lands in docs/) +// =========================================================================== + +/// The reconciled dispatch doc must live under `docs/` so the docs-integrity +/// gate and mkdocs nav can see it — not stranded in a session folder. +#[test] +fn dispatch_doc_exists_under_docs() { + assert!( + try_asset(DOC).is_some(), + "the reconciled triage-dispatch contract must be checked in at {DOC}" + ); +} + +/// The doc is wired into the human-maintained mkdocs nav manifest so +/// `tests/docs_integrity.rs` treats it as a first-class, linked concept page. +#[test] +fn dispatch_doc_is_linked_in_mkdocs_nav() { + let nav = asset(MKDOCS); + assert!( + nav.contains("concepts/triage-dispatch.md"), + "mkdocs.yml nav must link concepts/triage-dispatch.md so the docs gate covers it" + ); +} + +/// The doc pins the CANONICAL required brief schema (exactly the four design +/// fields) and explicitly marks the two producer hints OPTIONAL with defaults — +/// this is the reconciliation the schema divergence review demanded. +#[test] +fn dispatch_doc_pins_canonical_brief_schema() { + let body = asset(DOC); + for field in REQUIRED_BRIEF_FIELDS { + assert!( + body.contains(field), + "doc must document the canonical required brief field `{field}`" + ); + } + // The hints are named AND flagged optional with their documented defaults. + for hint in OPTIONAL_HINTS { + assert!( + body.contains(hint), + "doc must document the optional producer hint `{hint}`" + ); + } + let lc = body.to_lowercase(); + assert!( + lc.contains("optional") && lc.contains("hint"), + "doc must describe is_mechanical_sweep / sequence_group as OPTIONAL producer hints" + ); + assert!( + lc.contains("default"), + "doc must state the hints' defaults (is_mechanical_sweep -> false, sequence_group -> null)" + ); +} + +/// The doc pins the escalation discriminator: an item is an escalation IFF its +/// `recipe` is `null`; briefs never carry `escalate`. +#[test] +fn dispatch_doc_pins_escalation_discriminator() { + let body = asset(DOC).to_lowercase(); + assert!( + body.contains("escalate"), + "doc must document the `escalate` field for escalation items" + ); + assert!( + body.contains("null"), + "doc must document that a `null` recipe marks an escalation" + ); + assert!( + body.contains("iff") || body.contains("if and only if"), + "doc must state the discriminator as a biconditional (escalation iff recipe is null)" + ); +} + +/// The doc pins the blast-radius ordering rule (most-critical first). +#[test] +fn dispatch_doc_pins_blast_radius_ordering() { + let body = asset(DOC).to_lowercase(); + assert!( + body.contains("blast radius"), + "doc must specify blast-radius ordering" + ); + assert!( + body.contains("most-critical first") || body.contains("most-important first"), + "doc must specify most-critical-first ordering" + ); +} + +/// The doc pins the self-contained validation rules AND the least-privilege / +/// additive-only constraint applied to every brief. +#[test] +fn dispatch_doc_pins_validation_rules_and_least_privilege() { + let body = asset(DOC); + let lc = body.to_lowercase(); + assert!( + lc.contains("validation rules"), + "doc must carry a Validation rules section" + ); + assert!( + lc.contains("owner/name") || lc.contains("owner/repo"), + "doc must require a well-formed owner/name target_repo (rule 3)" + ); + assert!( + lc.contains("additive") && (lc.contains("non-breaking") || lc.contains("non breaking")), + "doc must state briefs are additive / non-breaking by default" + ); + assert!( + lc.contains("least privilege") || lc.contains("least-privilege"), + "doc must state the least-privilege constraint for CI/workflow changes" + ); + assert!( + body.contains("dropped_as_in_flight"), + "doc must state the dropped_as_in_flight exclusion (rule 6)" + ); +} + +// =========================================================================== +// GROUP B — BRIEF prompt contract pins (the emitter encodes the schema) +// =========================================================================== + +/// The BRIEF prompt is the live emitter; it must define the escalation shape +/// `{"recipe": null, "escalate": ...}` for non-actionable problems. +#[test] +fn brief_prompt_defines_escalation_shape() { + let body = asset(BRIEF_PROMPT); + assert!( + body.contains("\"recipe\": null") && body.contains("escalate"), + "BRIEF prompt must define the escalation shape {{\"recipe\": null, \"escalate\": ...}}" + ); +} + +/// The BRIEF prompt lists the four canonical required brief fields. +#[test] +fn brief_prompt_lists_canonical_required_fields() { + let body = asset(BRIEF_PROMPT); + for field in REQUIRED_BRIEF_FIELDS { + assert!( + body.contains(field), + "BRIEF prompt must reference the canonical required field `{field}`" + ); + } +} + +/// Reconciliation pin: the BRIEF prompt must present `is_mechanical_sweep` and +/// `sequence_group` as OPTIONAL producer hints (with defaults), matching the +/// design's canonical schema — not as if they were required output fields. +#[test] +fn brief_prompt_marks_hints_optional_with_defaults() { + let body = asset(BRIEF_PROMPT); + let lc = body.to_lowercase(); + for hint in OPTIONAL_HINTS { + assert!( + body.contains(hint), + "BRIEF prompt must mention the hint `{hint}`" + ); + } + assert!( + lc.contains("optional"), + "BRIEF prompt must describe is_mechanical_sweep / sequence_group as OPTIONAL" + ); + assert!( + lc.contains("default"), + "BRIEF prompt must state the hints' defaults when omitted (false / null)" + ); +} + +// =========================================================================== +// GROUP C — Self-contained validator (rules 1–4, 7) as an executable spec +// =========================================================================== + +/// Validate a dispatch array against the self-contained rules (1–4, 7). +/// Returns `Ok(())` if valid, otherwise `Err(reason)`. +fn validate_dispatch_array(v: &Value) -> Result<(), String> { + // Rule 1: top-level is an array. + let items = v.as_array().ok_or("top-level value must be a JSON array")?; + if items.is_empty() { + return Err("dispatch array must not be empty".into()); + } + + for (i, item) in items.iter().enumerate() { + let obj = item + .as_object() + .ok_or_else(|| format!("element {i} must be a JSON object"))?; + + let recipe = obj + .get("recipe") + .ok_or_else(|| format!("element {i} must carry a `recipe` field"))?; + + if recipe.is_null() { + // ----- Escalation branch (rule 2) ----- + let esc = obj + .get("escalate") + .ok_or_else(|| format!("escalation {i} must carry an `escalate` field"))?; + let s = esc + .as_str() + .ok_or_else(|| format!("escalation {i} `escalate` must be a string"))?; + if s.trim().is_empty() { + return Err(format!("escalation {i} `escalate` must be non-empty")); + } + } else { + // ----- Brief branch (rule 2) ----- + if obj.contains_key("escalate") { + return Err(format!( + "brief {i} must NOT carry an `escalate` field (discriminator violation)" + )); + } + let recipe_s = recipe + .as_str() + .ok_or_else(|| format!("brief {i} `recipe` must be a string"))?; + if recipe_s.trim().is_empty() { + return Err(format!("brief {i} `recipe` must be non-empty")); + } + // Canonical required fields present. + for field in ["task_description", "target_repo", "success_criteria"] { + if !obj.contains_key(field) { + return Err(format!("brief {i} is missing canonical field `{field}`")); + } + } + let td = obj + .get("task_description") + .and_then(Value::as_str) + .ok_or_else(|| format!("brief {i} `task_description` must be a string"))?; + if td.trim().is_empty() { + return Err(format!("brief {i} `task_description` must be non-empty")); + } + // Rule 3: well-formed owner/name. + let repo = obj + .get("target_repo") + .and_then(Value::as_str) + .ok_or_else(|| format!("brief {i} `target_repo` must be a string"))?; + if !is_well_formed_repo(repo) { + return Err(format!( + "brief {i} `target_repo` must be a well-formed owner/name, got {repo:?}" + )); + } + // Rule 4: non-empty array of non-empty strings. + let crit = obj + .get("success_criteria") + .and_then(Value::as_array) + .ok_or_else(|| format!("brief {i} `success_criteria` must be an array"))?; + if crit.is_empty() { + return Err(format!("brief {i} `success_criteria` must be non-empty")); + } + for (j, c) in crit.iter().enumerate() { + let cs = c + .as_str() + .ok_or_else(|| format!("brief {i} success_criteria[{j}] must be a string"))?; + if cs.trim().is_empty() { + return Err(format!("brief {i} success_criteria[{j}] must be non-empty")); + } + } + } + } + + // Rule 7: no credential-shaped secrets anywhere in the serialized array + // (heuristic token-shape check; PII is producer-trust, not enforced here). + if let Some(hit) = find_secret(&v.to_string()) { + return Err(format!( + "dispatch array must contain no secrets (matched {hit})" + )); + } + + Ok(()) +} + +/// A well-formed `owner/name`: exactly one `/`, both sides non-empty and drawn +/// from the GitHub-safe character set, no whitespace. The bare `.` and `..` +/// segments are rejected — GitHub disallows them as owner or repo names. +fn is_well_formed_repo(s: &str) -> bool { + let parts: Vec<&str> = s.split('/').collect(); + if parts.len() != 2 { + return false; + } + let ok = |seg: &str| { + !seg.is_empty() + && seg != "." + && seg != ".." + && seg + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')) + }; + ok(parts[0]) && ok(parts[1]) +} + +/// Heuristic secret detector for rule 7. Matches common credential shapes. +fn find_secret(text: &str) -> Option<&'static str> { + const NEEDLES: [&str; 6] = [ + "ghp_", // GitHub personal access token + "github_pat_", // GitHub fine-grained PAT + "AKIA", // AWS access key id prefix — deliberately broad; a false + // positive here only rejects a dispatch array (fail-safe), whereas + // tightening the needle risks missing a real leaked key. + "-----BEGIN", // PEM private key block + "xoxb-", // Slack bot token + "Authorization: Bearer ", // inline bearer credential + ]; + NEEDLES.into_iter().find(|n| text.contains(n)) +} + +/// Rule 5 intent: severity ranks must be non-increasing (most-critical first). +/// Lower rank == more critical (0 = ecosystem-wide blocker). +fn is_blast_radius_ordered(ranks: &[u8]) -> bool { + ranks.windows(2).all(|w| w[0] <= w[1]) +} + +// ---- Fixture builders ----------------------------------------------------- + +fn valid_escalation() -> Value { + json!({ + "recipe": null, + "escalate": "Root fs is 100% full: bootstrap deadlock. Manually reclaim stale snapshots first." + }) +} + +fn valid_brief() -> Value { + json!({ + "recipe": "smart-orchestrator", + "task_description": "In rysweet/amplihack-rs, enable a GitHub merge queue to relieve the merge live-lock. Additive / non-breaking; preserve the PRD.", + "target_repo": "rysweet/amplihack-rs", + "success_criteria": [ + "CI green on all required checks", + "merge queue enabled; live-lock relieved" + ] + }) +} + +// ---- Positive spec tests -------------------------------------------------- + +#[test] +fn valid_array_of_escalation_then_brief_passes() { + let arr = json!([valid_escalation(), valid_brief()]); + assert_eq!(validate_dispatch_array(&arr), Ok(())); +} + +#[test] +fn brief_with_optional_hints_present_is_valid() { + let mut brief = valid_brief(); + brief["is_mechanical_sweep"] = json!(false); + brief["sequence_group"] = Value::Null; + let arr = json!([brief]); + assert_eq!(validate_dispatch_array(&arr), Ok(())); +} + +#[test] +fn brief_with_optional_hints_omitted_is_valid() { + // Rule-2 note: omitted hints must not affect validity. + let brief = valid_brief(); + assert!(brief.get("is_mechanical_sweep").is_none()); + assert!(brief.get("sequence_group").is_none()); + let arr = json!([brief]); + assert_eq!(validate_dispatch_array(&arr), Ok(())); +} + +// ---- Negative / edge / error tests --------------------------------------- + +#[test] +fn top_level_object_is_rejected() { + let not_array = json!({"recipe": null, "escalate": "x"}); + assert!(validate_dispatch_array(¬_array).is_err()); +} + +#[test] +fn empty_array_is_rejected() { + assert!(validate_dispatch_array(&json!([])).is_err()); +} + +#[test] +fn brief_missing_task_description_is_rejected() { + let mut brief = valid_brief(); + brief.as_object_mut().unwrap().remove("task_description"); + assert!(validate_dispatch_array(&json!([brief])).is_err()); +} + +#[test] +fn brief_carrying_escalate_field_is_rejected() { + let mut brief = valid_brief(); + brief["escalate"] = json!("should not be here"); + assert!(validate_dispatch_array(&json!([brief])).is_err()); +} + +#[test] +fn escalation_with_empty_escalate_is_rejected() { + let esc = json!({"recipe": null, "escalate": " "}); + assert!(validate_dispatch_array(&json!([esc])).is_err()); +} + +#[test] +fn escalation_missing_escalate_is_rejected() { + let esc = json!({"recipe": null}); + assert!(validate_dispatch_array(&json!([esc])).is_err()); +} + +#[test] +fn brief_with_malformed_target_repo_is_rejected() { + for bad in [ + "no-slash", + "too/many/slashes", + "owner/", + "/name", + "own er/name", + ".", + "..", + "./name", + "owner/.", + "owner/..", + "../name", + ] { + let mut brief = valid_brief(); + brief["target_repo"] = json!(bad); + assert!( + validate_dispatch_array(&json!([brief])).is_err(), + "target_repo {bad:?} should be rejected" + ); + } +} + +#[test] +fn well_formed_target_repo_is_accepted() { + for good in [ + "rysweet/Simard", + "rysweet/amplihack-rs", + "rysweet/amplihack-recipe-runner", + ] { + assert!(is_well_formed_repo(good), "{good} should be well-formed"); + } +} + +#[test] +fn brief_with_empty_success_criteria_is_rejected() { + let mut brief = valid_brief(); + brief["success_criteria"] = json!([]); + assert!(validate_dispatch_array(&json!([brief])).is_err()); +} + +#[test] +fn brief_with_blank_success_criterion_is_rejected() { + let mut brief = valid_brief(); + brief["success_criteria"] = json!(["ok", " "]); + assert!(validate_dispatch_array(&json!([brief])).is_err()); +} + +#[test] +fn brief_with_non_array_success_criteria_is_rejected() { + let mut brief = valid_brief(); + brief["success_criteria"] = json!("CI green"); + assert!(validate_dispatch_array(&json!([brief])).is_err()); +} + +#[test] +fn array_containing_a_secret_is_rejected() { + let mut brief = valid_brief(); + // Build a synthetic GitHub-token-shaped string at runtime so the `ghp_` + // literal never appears verbatim in source (keeps secret scanners quiet) + // while still exercising rule 7's `ghp_` needle in `find_secret`. + let synthetic_token = format!("ghp{}{}", "_", "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"); + brief["task_description"] = json!(format!("use token {synthetic_token} to auth")); + assert!( + validate_dispatch_array(&json!([brief])).is_err(), + "rule 7: an embedded GitHub token must be rejected" + ); +} + +#[test] +fn blast_radius_ordering_helper_enforces_non_increasing_criticality() { + // P1 ecosystem-wide (0) -> P3 cross-cutting (1) -> P2 isolated (2): valid. + assert!(is_blast_radius_ordered(&[0, 1, 2])); + assert!(is_blast_radius_ordered(&[0, 0, 1])); + // Isolated ahead of ecosystem-wide: invalid. + assert!(!is_blast_radius_ordered(&[2, 0, 1])); + assert!(!is_blast_radius_ordered(&[1, 0])); +} + +// =========================================================================== +// GROUP D — Canonical machine-checkable fixture (the worked end-to-end array) +// =========================================================================== + +/// The implementation must ship the canonical worked dispatch array as a +/// checked-in, machine-parseable fixture so the emitted contract is executable +/// (the doc's worked example uses `…` placeholders and cannot be parsed). +#[test] +fn canonical_fixture_exists_and_is_a_valid_dispatch_array() { + let raw = try_asset(CANONICAL_FIXTURE).unwrap_or_else(|| { + panic!("canonical dispatch fixture must be checked in at {CANONICAL_FIXTURE}") + }); + let arr: Value = serde_json::from_str(&raw) + .unwrap_or_else(|e| panic!("{CANONICAL_FIXTURE} must be valid JSON: {e}")); + validate_dispatch_array(&arr) + .unwrap_or_else(|e| panic!("{CANONICAL_FIXTURE} must satisfy the dispatch contract: {e}")); +} + +/// The canonical fixture encodes the documented P1 -> P3 -> P2 shape: exactly +/// three elements — an escalation first, then the amplihack-rs merge-queue +/// brief, then the amplihack-recipe-runner mdBook brief. +#[test] +fn canonical_fixture_matches_documented_p1_p3_p2_shape() { + let raw = try_asset(CANONICAL_FIXTURE) + .unwrap_or_else(|| panic!("missing canonical fixture at {CANONICAL_FIXTURE}")); + let arr: Value = serde_json::from_str(&raw).expect("fixture must be valid JSON"); + let items = arr.as_array().expect("fixture must be a JSON array"); + + assert_eq!( + items.len(), + 3, + "canonical dispatch array has exactly 3 elements" + ); + + // Element 0: escalation (recipe null). + assert!( + items[0].get("recipe").map(Value::is_null).unwrap_or(false), + "element 0 must be the P1 escalation (recipe: null)" + ); + + // Element 1: P3 amplihack-rs brief. + assert_eq!( + items[1].get("target_repo").and_then(Value::as_str), + Some("rysweet/amplihack-rs"), + "element 1 must be the P3 amplihack-rs merge-queue brief" + ); + + // Element 2: P2 amplihack-recipe-runner brief. + assert_eq!( + items[2].get("target_repo").and_then(Value::as_str), + Some("rysweet/amplihack-recipe-runner"), + "element 2 must be the P2 amplihack-recipe-runner mdBook brief" + ); + + // Both briefs are additive smart-orchestrator runs. + for i in [1usize, 2] { + assert_eq!( + items[i].get("recipe").and_then(Value::as_str), + Some("smart-orchestrator"), + "brief {i} must target the smart-orchestrator recipe" + ); + } +} diff --git a/tests/fixtures/ecosystem_dispatch/canonical.json b/tests/fixtures/ecosystem_dispatch/canonical.json new file mode 100644 index 000000000..f9f97f737 --- /dev/null +++ b/tests/fixtures/ecosystem_dispatch/canonical.json @@ -0,0 +1,34 @@ +[ + { + "recipe": null, + "escalate": "Problem 1 (critical, resource_pressure, rysweet/Simard, dedup process:disk_full:root_fs) is NOT engineer-fixable in its current state: root fs / is 100% full (28G/28G, 0 avail), so the disk-admission ceiling (max_disk_used_percent=90) rejects every engineer spawn. This is a bootstrap deadlock: the fix requires an engineer, but no engineer can be admitted until disk is freed. Requires MANUAL operator intervention first: reclaim the ~2.78 GiB of stale ~/.simard/cognitive.corrupt-*/bad-*/predeploy-*/pr*-canary-* snapshots to drop usage below 90% and unblock #4803 / cycle-2630. AFTER disk is freed, file two follow-up bugs: (a) the periodic reclaim job frees 0 bytes because it perpetually defers stale snapshots 'for review' instead of collecting them; (b) `simard status` mis-reports '/home 25 GiB free' via a statvfs f_bavail illusion while df shows root full." + }, + { + "recipe": "smart-orchestrator", + "task_description": "In target_repo rysweet/amplihack-rs, fix the CI/merge strict up-to-date branch-protection live-lock tracked in issue #1050. Required 'branch must be up to date before merging' protection forces every PR to rebase/re-run CI whenever any other PR merges, causing a merge live-lock (evidence: open PR #1063 sitting in mergeStateStatus=BEHIND while mergeable=MERGEABLE). Smallest surface: the repo's branch-protection / merge-queue config and CI workflows under .github/workflows that gate merges - enable a GitHub merge queue (merge_group trigger + 'Require merge queue') so PRs are batched and tested serially. Wire required status checks to run on the merge_group event. Additive / non-breaking: do not remove existing required checks or weaken protection; preserve the PRD; no Bridge naming; no stray print!/println! (tracing + OTel only). Link the resulting PR to issue #1050.", + "target_repo": "rysweet/amplihack-rs", + "is_mechanical_sweep": false, + "sequence_group": null, + "success_criteria": [ + "CI green on all required checks (including on the merge_group event)", + "GitHub merge queue enabled and required status checks wired to merge_group; PR live-lock relieved", + "additive / non-breaking; existing required checks and protections preserved; PRD preserved", + "no Bridge naming; no stray print!/println! in new code (tracing + OTel only)", + "docs/CONTRIBUTING updated with the merge-queue flow and links; PR linked to issue #1050" + ] + }, + { + "recipe": "smart-orchestrator", + "task_description": "In target_repo rysweet/amplihack-recipe-runner, fix the red default branch: the 'Deploy mdBook to GitHub Pages' workflow build check failed on the head commit (failing run 30187028559 / job 89753457708). Fresh regression, not chronic - prior 4 runs of that workflow succeeded; core CI and Auto Release were green on the same push, so blast radius is docs-pages-only. Smallest surface: the .github/workflows/ mdBook/GitHub-Pages deploy workflow and the book source it builds (book.toml, docs/ or book/ SUMMARY.md and referenced markdown) - inspect the failing job log for the actual build error (missing/renamed page in SUMMARY.md, broken intra-book link, mdBook version pin drift, or preprocessor change) and correct the root cause. Additive / non-breaking: restore the docs deploy without altering unrelated CI; preserve the PRD; no Bridge naming; no stray print!/println! (tracing + OTel only). Keep workflow permissions least-privilege and job-scoped (pages: write, id-token: write).", + "target_repo": "rysweet/amplihack-recipe-runner", + "is_mechanical_sweep": false, + "sequence_group": null, + "success_criteria": [ + "CI green on all required checks", + "'Deploy mdBook to GitHub Pages' workflow build check passes on main head", + "additive / non-breaking; only docs-pages deploy restored, unrelated CI untouched; PRD preserved", + "no Bridge naming; no stray print!/println! in new code (tracing + OTel only)", + "docs/link updates included; quality-audit cycles pass" + ] + } +]