diff --git a/.agents/skills/autopilot-issue-triage-worker/SKILL.md b/.agents/skills/autopilot-issue-triage-worker/SKILL.md index 46ecb58b3e..450eed63fe 100644 --- a/.agents/skills/autopilot-issue-triage-worker/SKILL.md +++ b/.agents/skills/autopilot-issue-triage-worker/SKILL.md @@ -109,14 +109,35 @@ output template differ. ## Agent roster -| Agent | Persona | Always active? | -|-------|---------|----------------| -| [DevX UX Expert](../../agents/devx-ux-expert.agent.md) | User-Need Reviewer | Yes | -| [Supply Chain Security Expert](../../agents/supply-chain-security-expert.agent.md) | Risk-Surface Reviewer | Yes | -| [APM CEO](../../agents/apm-ceo.agent.md) | Triage Arbiter | Yes (always arbitrates) | -| [OSS Growth Hacker](../../agents/oss-growth-hacker.agent.md) | Contributor-Tone Reviewer | Conditional (see below) | -| [Python Architect](../../agents/python-architect.agent.md) | Architecture Reviewer | Conditional (see below) | -| [Doc Writer](../../agents/doc-writer.agent.md) | Documentation Reviewer | Conditional (see below) | +| Agent slug | Persona | Always active? | +|------------|---------|----------------| +| `devx-ux-expert` | User-Need Reviewer | Yes | +| `supply-chain-security-expert` | Risk-Surface Reviewer | Yes | +| `apm-ceo` | Triage Arbiter | Yes (always arbitrates) | +| `oss-growth-hacker` | Contributor-Tone Reviewer | Conditional (see below) | +| `python-architect` | Architecture Reviewer | Conditional (see below) | +| `doc-writer` | Documentation Reviewer | Conditional (see below) | + +These agent files are consumer-provided companion assets, not part of this +package. For a Copilot consumer, APM deploys them under +`.github/agents/.agent.md`. Do not resolve them through a +path relative to the installed skill directory. + +Before reading persona files or producing advice, run the bundled +`scripts/check_required_agents.py` with the three always-active slugs and +every conditional slug activated for this issue, using the consumer's +`.github/agents` directory as `--agents-dir`. The script emits JSON and +returns nonzero when any required role is absent; stop and report every +missing role. Never silently omit a mandatory or activated conditional lens. +For example, the always-active roles are checked with: + +```bash +python3 .agents/skills/autopilot-issue-triage-worker/scripts/check_required_agents.py \ + --agents-dir .github/agents \ + devx-ux-expert supply-chain-security-expert apm-ceo +``` + +Add any conditional role selected for this issue to the role arguments. Skipped by default: CLI Logging Expert, Auth Expert. Triage operates on issue intent, not on diffs -- those personas are invoked downstream @@ -376,7 +397,7 @@ write/receipt step. If any check fails, redo that persona's pass and repeat the gate. Do not proceed to step 5 until the gate passes. 5. Take the **APM CEO** lens (load - `../../agents/apm-ceo.agent.md`) and arbitrate the collected + `.github/agents/apm-ceo.agent.md`) and arbitrate the collected findings into a single recommendation, useful classification, proposed brief, and reply tone. Still in your own context. CEO arbitration may run only after the completeness gate has passed. @@ -423,8 +444,9 @@ write/receipt step. For each persona, run this exact procedure in your own context: -1. Open the persona's `.agent.md` file (linked in the roster) and - read its scope, lens, anti-patterns, and required return shape. +1. Open the persona's `.agent.md` file from the consumer's + `.github/agents/` directory and read its scope, lens, + anti-patterns, and required return shape. 2. From that persona's lens, review the issue title, body, labels, author signals, and the complete prior comment history against the scope declared in the file. Reconcile with earlier advice and any diff --git a/.agents/skills/autopilot-issue-triage-worker/apm.yml b/.agents/skills/autopilot-issue-triage-worker/apm.yml index 90f3d70cd6..0436229b79 100644 --- a/.agents/skills/autopilot-issue-triage-worker/apm.yml +++ b/.agents/skills/autopilot-issue-triage-worker/apm.yml @@ -5,10 +5,9 @@ author: Microsoft license: MIT type: skill -# Stand-alone primitive: no transitive APM deps. Persona lenses are -# referenced by relative path (../../agents/*.agent.md) and resolved -# against the consumer repo's installed .apm/agents/** or -# .github/agents/** -- identical to the autopilot-pr-review-worker precedent. +# Stand-alone primitive: no transitive APM deps. Required specialist +# roles are consumer-provided companions checked in the deployed +# .github/agents/ directory before the worker gives advice. dependencies: apm: - ../autopilot-comment diff --git a/.agents/skills/autopilot-issue-triage-worker/scripts/check_required_agents.py b/.agents/skills/autopilot-issue-triage-worker/scripts/check_required_agents.py new file mode 100644 index 0000000000..bd5b143e27 --- /dev/null +++ b/.agents/skills/autopilot-issue-triage-worker/scripts/check_required_agents.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Check that required Copilot agent files exist in a consumer repository.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +_AGENT_SLUG = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*\Z") + + +def main() -> int: + """Report available and missing required agent roles as JSON.""" + parser = argparse.ArgumentParser( + description="Check required .github/agents/.agent.md files." + ) + parser.add_argument( + "--agents-dir", + type=Path, + default=Path(".github/agents"), + help="consumer agent directory (default: .github/agents)", + ) + parser.add_argument("roles", nargs="+", help="required agent role slugs") + args = parser.parse_args() + + invalid = [role for role in args.roles if not _AGENT_SLUG.fullmatch(role)] + if invalid: + parser.error(f"invalid role slug(s): {', '.join(invalid)}") + + roles = sorted(set(args.roles)) + available = [role for role in roles if (args.agents_dir / f"{role}.agent.md").is_file()] + missing = [role for role in roles if role not in available] + print( + json.dumps( + { + "agents_dir": str(args.agents_dir), + "available_roles": available, + "missing_roles": missing, + "ready": not missing, + }, + sort_keys=True, + ) + ) + if missing: + print( + f"Missing required agent roles in {args.agents_dir}: {', '.join(missing)}", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.agents/skills/autopilot-pr-review-worker/SKILL.md b/.agents/skills/autopilot-pr-review-worker/SKILL.md index 7d777e0152..bb7a6bfbbb 100644 --- a/.agents/skills/autopilot-pr-review-worker/SKILL.md +++ b/.agents/skills/autopilot-pr-review-worker/SKILL.md @@ -210,16 +210,20 @@ are active for this `panel-mode` and surface. Do not spawn | Agent | Role | When spawned | |-------|------|----------------| -| [Python Architect](.apm/agents/python-architect.agent.md) | Architectural Reviewer + mermaid in `full` | Core, unless docs/changelog-only. Optional in `lean` on tiny diffs. | -| [CLI Logging Expert](.apm/agents/cli-logging-expert.agent.md) | Output UX Reviewer | Surface-gated | -| [DevX UX Expert](.apm/agents/devx-ux-expert.agent.md) | Package-Manager UX | Surface-gated | -| [Supply Chain Security Expert](.apm/agents/supply-chain-security-expert.agent.md) | Threat-Model Reviewer | Surface-gated | -| [OSS Growth Hacker](.apm/agents/oss-growth-hacker.agent.md) | Adoption Strategist | Surface-gated | -| [Auth Expert](.apm/agents/auth-expert.agent.md) | Auth / Token Reviewer | Surface-gated | -| [Doc Writer](.apm/agents/doc-writer.agent.md) | Documentation Reviewer | Surface-gated | -| [Test Coverage Expert](.apm/agents/test-coverage-expert.agent.md) | Test-Presence Reviewer | Core unless docs-only (`src/` untouched) | -| [Performance Expert](.apm/agents/performance-expert.agent.md) | Package-Manager Performance Reviewer | Surface-gated | -| [APM CEO](.apm/agents/apm-ceo.agent.md) | Strategic Arbiter / Synthesizer | Always | +| `python-architect` | Architectural Reviewer + mermaid in `full` | Core, unless docs/changelog-only. Optional in `lean` on tiny diffs. | +| `cli-logging-expert` | Output UX Reviewer | Surface-gated | +| `devx-ux-expert` | Package-Manager UX | Surface-gated | +| `supply-chain-security-expert` | Threat-Model Reviewer | Surface-gated | +| `oss-growth-hacker` | Adoption Strategist | Surface-gated | +| `auth-expert` | Auth / Token Reviewer | Surface-gated | +| `doc-writer` | Documentation Reviewer | Surface-gated | +| `test-coverage-expert` | Test-Presence Reviewer | Core unless docs-only (`src/` untouched) | +| `performance-expert` | Package-Manager Performance Reviewer | Surface-gated | +| `apm-ceo` | Strategic Arbiter / Synthesizer | Always | + +These agent files are consumer-provided companion assets, not part of this +package. For a Copilot consumer, APM deploys them under +`.github/agents/.agent.md`. ## Topology @@ -244,6 +248,22 @@ are active for this `panel-mode` and surface. Do not spawn Caller (merge-worker, scheduler, or human) SHOULD set `panel-mode: full | lean | delta`. Omitted or unknown -> `lean`. +After resolving the active roster from the mode and surface, but before +spawning any panelist, run the bundled +`scripts/check_required_agents.py` with the active role slugs and the +consumer's `.github/agents` directory as `--agents-dir`. The script emits +JSON and returns nonzero when any active role is missing. Stop and report +all missing roles; do not skip a required role or produce a partial review. +Run it from the consumer repository root, for example: + +```bash +python3 .agents/skills/autopilot-pr-review-worker/scripts/check_required_agents.py \ + --agents-dir .github/agents python-architect test-coverage-expert apm-ceo +``` + +Replace the example roles with the exact active roster for the selected mode +and surface. + | Mode | Roster | Context | Comment | |------|--------|---------|---------| | full | Surface-gated specialists + CEO | Shared brief + owned files | yes | @@ -530,7 +550,7 @@ no comment can be rendered, an explicit `noop` (step 9) -- are emitted. 5. **Spawn the CEO synthesizer task.** Pass the spawned panelist JSON returns (omitted personas are inactive; do not invent stub JSON) to a `task` invocation that loads - `.apm/agents/apm-ceo.agent.md`. Run it as a BLOCKING task and WAIT + `.github/agents/apm-ceo.agent.md`. Run it as a BLOCKING task and WAIT for its JSON return -- do NOT spawn it detached (background mode that returns an `agent_id`) and do NOT end your turn while it runs. Its return is required to render the comment; ending the turn here is the diff --git a/.agents/skills/autopilot-pr-review-worker/apm.yml b/.agents/skills/autopilot-pr-review-worker/apm.yml index c32cb52a05..a86e667b5f 100644 --- a/.agents/skills/autopilot-pr-review-worker/apm.yml +++ b/.agents/skills/autopilot-pr-review-worker/apm.yml @@ -5,9 +5,9 @@ author: Microsoft license: MIT type: skill -# Stand-alone primitive: no transitive APM deps. The skill body honors -# the consumer repo's linting / encoding rules at runtime by activating -# whichever instructions the consumer has installed. +# Stand-alone primitive: no transitive APM deps. Specialist roles are +# consumer-provided companions checked in the deployed .github/agents/ +# directory before the worker begins the panel. dependencies: apm: - ../autopilot-comment diff --git a/.agents/skills/autopilot-pr-review-worker/assets/panelist-return-schema.json b/.agents/skills/autopilot-pr-review-worker/assets/panelist-return-schema.json index a05f7b240d..a20164f08c 100644 --- a/.agents/skills/autopilot-pr-review-worker/assets/panelist-return-schema.json +++ b/.agents/skills/autopilot-pr-review-worker/assets/panelist-return-schema.json @@ -128,11 +128,11 @@ }, "proves": { "type": "string", - "description": "One sentence in USER words naming the promise the test proves (or fails to prove if missing). MUST mirror a Scenario Evidence row from the PR body when one exists -- the rubric (.github/skills/pr-description-skill/assets/scenario-evidence-rubric.md) is the shared vocabulary." + "description": "One sentence in USER words naming the promise the test proves (or fails to prove if missing). MUST mirror a Scenario Evidence row from the PR body when one exists." }, "principles": { "type": "array", - "description": "APM principle taxonomy from the scenario-evidence rubric (Portability by manifest, Secure by default, Governed by policy, Multi-harness support, Vendor-neutral, DevX, OSS). Ranks the criticality of the proven (or missing) promise.", + "description": "APM principle taxonomy defined by the enum below. Ranks the criticality of the proven (or missing) promise.", "items": { "type": "string", "enum": [ diff --git a/.agents/skills/autopilot-pr-review-worker/scripts/check_required_agents.py b/.agents/skills/autopilot-pr-review-worker/scripts/check_required_agents.py new file mode 100644 index 0000000000..bd5b143e27 --- /dev/null +++ b/.agents/skills/autopilot-pr-review-worker/scripts/check_required_agents.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Check that required Copilot agent files exist in a consumer repository.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +_AGENT_SLUG = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*\Z") + + +def main() -> int: + """Report available and missing required agent roles as JSON.""" + parser = argparse.ArgumentParser( + description="Check required .github/agents/.agent.md files." + ) + parser.add_argument( + "--agents-dir", + type=Path, + default=Path(".github/agents"), + help="consumer agent directory (default: .github/agents)", + ) + parser.add_argument("roles", nargs="+", help="required agent role slugs") + args = parser.parse_args() + + invalid = [role for role in args.roles if not _AGENT_SLUG.fullmatch(role)] + if invalid: + parser.error(f"invalid role slug(s): {', '.join(invalid)}") + + roles = sorted(set(args.roles)) + available = [role for role in roles if (args.agents_dir / f"{role}.agent.md").is_file()] + missing = [role for role in roles if role not in available] + print( + json.dumps( + { + "agents_dir": str(args.agents_dir), + "available_roles": available, + "missing_roles": missing, + "ready": not missing, + }, + sort_keys=True, + ) + ) + if missing: + print( + f"Missing required agent roles in {args.agents_dir}: {', '.join(missing)}", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.agents/skills/autopilot-pr-triage-worker/SKILL.md b/.agents/skills/autopilot-pr-triage-worker/SKILL.md index c73b93219b..91666ab645 100644 --- a/.agents/skills/autopilot-pr-triage-worker/SKILL.md +++ b/.agents/skills/autopilot-pr-triage-worker/SKILL.md @@ -44,9 +44,9 @@ Rules: and processing / classification labels. Never assign. Never request reviewers. Do not call `gh pr comment` yourself. Do not write `status/accepted`, `status/needs-design`, or - `status/needs-triage`. Write `status/deferred` only when this - PR is not labelled `status/accepted` and has no same-repo - linked issue labelled `status/accepted`. + `status/needs-triage` or `status/deferred`. A defer + recommendation belongs only in the advisory comment; a human + decides whether to apply `status/deferred`. - `json` defaults to `off` when omitted or unknown. Omitted `json` is not a missing-field stop. - `json: off` -> no machine JSON receipt. @@ -97,14 +97,14 @@ classification labels from the contract. Never write `status/accepted`, `status/needs-design`, or `status/needs-triage`. -Auto-defer: if this PR is not labelled `status/accepted` and -has no same-repo linked issue labelled `status/accepted`, add -`status/deferred` (`write: on` only). -Do not overwrite `status/accepted` already on the PR. Linked -means `Fixes` / `Closes` / `#N` in the body or commits, or -GitHub `closingIssuesReferences`, same repository. Any one -accepted linked issue or `status/accepted` on the PR blocks -auto-defer. +When neither this PR nor a same-repo linked issue is labelled +`status/accepted`, recommend `needs-issue` and explain that a +human must review and accept scope before implementation. If +deferral is appropriate, recommend it in the advisory comment; +never write `status/deferred`. Linked means `Fixes` / `Closes` / +`#N` in the body or commits, or GitHub `closingIssuesReferences`, +same repository. An accepted linked issue or `status/accepted` on +the PR blocks the `needs-issue` recommendation. CODEOWNERS `reviewRequests` is runtime authority. Note owners in the comment. Never add or remove review requests. @@ -131,11 +131,11 @@ conversation, no-op (do not post a duplicate). `auto-handle`. 4. Neither this PR nor a linked same-repo issue is `status/accepted` -> recommend `needs-issue`, - add `status/deferred` when `write: on`, and thank the author. - Invite them to open an issue for maintainer review and - acceptance first, per CONTRIBUTING.md ("Start with an issue, - not an implementation."). Link - https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md + thank the author, and invite them to open an issue for + maintainer review and acceptance first, per CONTRIBUTING.md + ("Start with an issue, not an implementation."). Link + https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md. + Do not add `status/deferred`; any deferral is a human decision. 5. `ready-for-review` when this PR or a linked same-repo issue is `status/accepted`. It is not merge approval and is not a request to run `autopilot-pr-review-scheduler`. @@ -157,6 +157,5 @@ conversation, no-op (do not post a duplicate). - Do not run `autopilot-pr-review-worker` or `autopilot-pr-merge-worker`. - Do not contradict CODEOWNERS. - Do not treat labels or this comment as `status/accepted`. -- Do not write `status/deferred` when a linked issue is - `status/accepted` or the PR already has `status/accepted`. +- Never write `status/deferred` or any other human-decision label. - ASCII only. diff --git a/.agents/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md b/.agents/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md index 3a44bd05ae..26621180cc 100644 --- a/.agents/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md +++ b/.agents/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md @@ -44,6 +44,6 @@ Thank you for contributing this pull request. APM starts with an issue, not an implementation (https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md). Please open an issue describing the user problem so a maintainer -can review and accept the scope first. This PR is labelled -status/deferred until that happens.> +can review and accept the scope first. This PR is awaiting maintainer +review and acceptance of its scope.> ``` diff --git a/.agents/skills/pr-description-skill/SKILL.md b/.agents/skills/pr-description-skill/SKILL.md index e0c521f487..86fb357b67 100644 --- a/.agents/skills/pr-description-skill/SKILL.md +++ b/.agents/skills/pr-description-skill/SKILL.md @@ -4,9 +4,9 @@ description: >- Use this skill to write the PR description (PR body) for any pull request opened against microsoft/apm. Produces one self-sufficient GitHub-Flavored Markdown artifact: TL;DR, Problem (WHY), Approach - (WHAT), Implementation (HOW), 1-3 validated mermaid diagrams, - explicit trade-offs, validation evidence, and a How-to-test - section -- with every WHY-claim backed by a verbatim quote from + (WHAT), Implementation (HOW), validated mermaid diagrams when + structure is non-trivial, explicit trade-offs, validation evidence, + and a How-to-test section -- with every WHY-claim backed by a PROSE or Agent Skills. Activate when the user asks to "write a PR description", "draft a PR body", "open a PR", "fill in the PR template", or any equivalent. @@ -72,7 +72,7 @@ Per-section ceilings (enforced by `assets/section-rubric.md`): | Problem (WHY) | max 6 bullets, max 3 quoted anchors total | | Approach (WHAT) | a table OR 3-7 bullets; may be skipped if PR is purely additive (say "additive: see Implementation") | | Implementation (HOW) | one short paragraph per file, OR a table; no prose walls | -| Diagrams | 1-3 mermaid blocks; every diagram preceded by a one-sentence legend | +| Diagrams | Include 1-3 validated mermaid blocks only when structure or control flow is non-trivial; simple, docs-only, and mechanical PRs may omit the section | | Trade-offs | 3-5 bullets; mechanical PRs may be 1-2 | | Benefits | 3-5 numbered items, each measurable | | Validation | copy-paste real command output; do not narrate | @@ -113,11 +113,12 @@ quote, it is downgraded to a "should" with the reason given. Anchor: Agent Skills, ["Add what the agent lacks, omit what it knows"](https://agentskills.io/skill-creation/best-practices). -4. **Visual aid where structure is non-trivial.** Any change that - touches more than one file or alters control flow SHOULD include - at least one mermaid diagram. Add a second only when the - relationships are non-trivial. Never add a third unless it earns - its place. Each diagram MUST be preceded by a one-sentence legend. +4. **Visual aid where structure is non-trivial.** Changes with + non-trivial structure or control flow SHOULD include 1-3 mermaid + diagrams. Simple, docs-only, and mechanical PRs may omit the + section. Add a second only when the relationships are non-trivial. + Never add a third unless it earns its place. Each diagram MUST be + preceded by a one-sentence legend. Anchor: Agent Skills, ["agents pattern-match well against concrete structures"](https://agentskills.io/skill-creation/best-practices). @@ -176,7 +177,7 @@ scannable. | 3 | Problem (WHY) | Observed failure modes; max 6 bullets, max 3 quoted anchors | | 4 | Approach (WHAT) | Table or 3-7 bullets; may say "additive: see Implementation" | | 5 | Implementation (HOW) | One short paragraph per file or a table | -| 6 | Diagrams | 1-3 validated mermaid blocks, each with a legend; diagram type chosen per intent (`assets/mermaid-conventions.md`) | +| 6 | Diagrams | Optional for simple, docs-only, and mechanical PRs; non-trivial structure or control flow requires 1-3 validated blocks, each with a legend and a type chosen per intent (`assets/mermaid-conventions.md`) | | 7 | Trade-offs | 3-5 bullets (1-2 if mechanical) | | 8 | Benefits | 3-5 numbered, measurable items | | 9 | Validation | Real command output, ideally inside `
` if long; **MUST include the Scenario Evidence subsection** (`assets/scenario-evidence-rubric.md`) for any behavior-change PR -- maps each user-promise scenario this PR touches to the test that proves it works, tagged with the APM principle the scenario serves | @@ -230,14 +231,14 @@ Run these steps in order. Tick each before moving on. 4. [ ] Fill in the template top-to-bottom using only facts from the activation contract. Every WHY-claim gets a verbatim quoted anchor. If you cannot anchor a claim, drop it. -5. [ ] Generate 1-3 mermaid diagrams. **Before drafting any block, - load `assets/mermaid-conventions.md`** to pick the right - diagram type per intent (sequenceDiagram for execution flow, - flowchart LR for pipeline / architecture, stateDiagram-v2 for - state machines) and apply the boxing convention for NEW - behavior. Add a one-sentence legend above each diagram. -6. [ ] **Validate every mermaid block deterministically (see - below). Do NOT save the draft until every block validates.** +5. [ ] Decide whether the PR has non-trivial structure or control + flow. Simple, docs-only, and mechanical PRs may omit the + Diagrams section. Otherwise, load `assets/mermaid-conventions.md` + and generate 1-3 diagrams of the right type (sequenceDiagram + for execution flow, flowchart LR for pipeline / architecture, + stateDiagram-v2 for state machines), with one-sentence legends. +6. [ ] If the draft contains mermaid blocks, **validate every block + deterministically (see below)** and do not save until all pass. 7. [ ] Load `assets/section-rubric.md` and run the self-check pass. Validation loop pattern from Agent Skills: ["do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes."](https://agentskills.io/skill-creation/best-practices). @@ -250,15 +251,15 @@ Run these steps in order. Tick each before moving on. ## Mandatory mermaid validation step -Run every mermaid block in the draft through `mmdc` and refuse to -save until all pass. +When the draft contains mermaid blocks, run each through `mmdc` and +refuse to save until all pass. ```bash # Extract mermaid blocks and validate each one. -# Requires: npx --yes -p @mermaid-js/mermaid-cli mmdc (one-shot, no global install needed) +# Requires: npx --yes -p @mermaid-js/mermaid-cli@12.0.0 mmdc awk '/^```mermaid/{n++; f=outdir"/diag"n".mmd"; getline; while($0 != "```") {print > f; getline}}' outdir=/tmp/mermaid-check pr-body-draft.md for f in /tmp/mermaid-check/diag*.mmd; do - npx --yes -p @mermaid-js/mermaid-cli mmdc -i "$f" -o "${f%.mmd}.svg" --quiet || { echo "INVALID: $f"; exit 1; } + npx --yes -p @mermaid-js/mermaid-cli@12.0.0 mmdc -i "$f" -o "${f%.mmd}.svg" --quiet || { echo "INVALID: $f"; exit 1; } done ``` @@ -292,8 +293,8 @@ of` closing rules, round brackets in node labels, inline - The file is **UTF-8 GitHub-Flavored Markdown**. Em dashes, smart quotes, Unicode in mermaid labels, alerts, and collapsibles are all permitted and encouraged where they improve readability. -- Every mermaid block has been validated by `mmdc` and renders - without error. +- Every included mermaid block has been validated by the pinned + `mmdc` package and renders without error. - The cite-or-omit rule applies absolutely. - The TL;DR is at most four sentences. - The body ends with the trailer: @@ -312,8 +313,9 @@ of` closing rules, round brackets in node labels, inline - Diagrams without a legend, OR diagrams that fail `mmdc`. - A TL;DR longer than four sentences. - Skipping any required section because "the PR is small". A small - PR can have a one-line Implementation per file, but the section - header must still be present. + PR can have a one-line Implementation per file, but all required + section headers must still be present. The Diagrams section is + optional for simple, docs-only, and mechanical changes. - Restating the diff line-by-line in Implementation. That is what the Files Changed tab is for. - Quoting a doc out of context. The self-check pass must verify diff --git a/.agents/skills/pr-description-skill/apm.yml b/.agents/skills/pr-description-skill/apm.yml index bd72672248..cfd8c68e9c 100644 --- a/.agents/skills/pr-description-skill/apm.yml +++ b/.agents/skills/pr-description-skill/apm.yml @@ -1,6 +1,6 @@ name: pr-description-skill version: 0.1.0 -description: Authors one anchored, mermaid-validated GitHub-Flavored Markdown PR body for any microsoft/apm pull request -- TL;DR, WHY/WHAT/HOW, trade-offs, validation evidence, how-to-test. Single-thread skill; consumed by the issue/PR orchestrators at every PR-open site and usable standalone. +description: Authors one anchored GitHub-Flavored Markdown PR body with mermaid diagrams validated when included -- TL;DR, WHY/WHAT/HOW, trade-offs, validation evidence, how-to-test. Single-thread skill; consumed by the issue/PR orchestrators at every PR-open site and usable standalone. author: Microsoft license: MIT type: skill diff --git a/.agents/skills/pr-description-skill/assets/pr-body-template.md b/.agents/skills/pr-description-skill/assets/pr-body-template.md index ad8923bf18..5a43273ff6 100644 --- a/.agents/skills/pr-description-skill/assets/pr-body-template.md +++ b/.agents/skills/pr-description-skill/assets/pr-body-template.md @@ -4,8 +4,8 @@ Loaded by pr-description-skill ONLY at synthesis time, after every row of the activation contract has been filled. Replace every with content drawn from the activation contract - inputs. Drop a section's body (keep its header) only when SKILL.md - explicitly allows it for this PR shape. + inputs. Omit an optional section only when SKILL.md allows it for + this PR shape. CHARSET: this template is ASCII because it lives in the repo and is subject to .github/instructions/encoding.instructions.md. The @@ -72,8 +72,10 @@ https://github.com/microsoft/apm/blob//path#L12-L34> ## Diagrams -<1-3 mermaid blocks. Each preceded by a one-sentence legend. Every -block MUST have been validated by mmdc before saving.> + .agent.md`. Do not resolve them through a +path relative to the installed skill directory. + +Before reading persona files or producing advice, run the bundled +`scripts/check_required_agents.py` with the three always-active slugs and +every conditional slug activated for this issue, using the consumer's +`.github/agents` directory as `--agents-dir`. The script emits JSON and +returns nonzero when any required role is absent; stop and report every +missing role. Never silently omit a mandatory or activated conditional lens. +For example, the always-active roles are checked with: + +```bash +python3 .agents/skills/autopilot-issue-triage-worker/scripts/check_required_agents.py \ + --agents-dir .github/agents \ + devx-ux-expert supply-chain-security-expert apm-ceo +``` + +Add any conditional role selected for this issue to the role arguments. Skipped by default: CLI Logging Expert, Auth Expert. Triage operates on issue intent, not on diffs -- those personas are invoked downstream @@ -376,7 +397,7 @@ write/receipt step. If any check fails, redo that persona's pass and repeat the gate. Do not proceed to step 5 until the gate passes. 5. Take the **APM CEO** lens (load - `../../agents/apm-ceo.agent.md`) and arbitrate the collected + `.github/agents/apm-ceo.agent.md`) and arbitrate the collected findings into a single recommendation, useful classification, proposed brief, and reply tone. Still in your own context. CEO arbitration may run only after the completeness gate has passed. @@ -423,8 +444,9 @@ write/receipt step. For each persona, run this exact procedure in your own context: -1. Open the persona's `.agent.md` file (linked in the roster) and - read its scope, lens, anti-patterns, and required return shape. +1. Open the persona's `.agent.md` file from the consumer's + `.github/agents/` directory and read its scope, lens, + anti-patterns, and required return shape. 2. From that persona's lens, review the issue title, body, labels, author signals, and the complete prior comment history against the scope declared in the file. Reconcile with earlier advice and any diff --git a/packages/autopilot/autopilot-issue-triage-worker/apm.yml b/packages/autopilot/autopilot-issue-triage-worker/apm.yml index 90f3d70cd6..0436229b79 100644 --- a/packages/autopilot/autopilot-issue-triage-worker/apm.yml +++ b/packages/autopilot/autopilot-issue-triage-worker/apm.yml @@ -5,10 +5,9 @@ author: Microsoft license: MIT type: skill -# Stand-alone primitive: no transitive APM deps. Persona lenses are -# referenced by relative path (../../agents/*.agent.md) and resolved -# against the consumer repo's installed .apm/agents/** or -# .github/agents/** -- identical to the autopilot-pr-review-worker precedent. +# Stand-alone primitive: no transitive APM deps. Required specialist +# roles are consumer-provided companions checked in the deployed +# .github/agents/ directory before the worker gives advice. dependencies: apm: - ../autopilot-comment diff --git a/packages/autopilot/autopilot-issue-triage-worker/scripts/check_required_agents.py b/packages/autopilot/autopilot-issue-triage-worker/scripts/check_required_agents.py new file mode 100644 index 0000000000..bd5b143e27 --- /dev/null +++ b/packages/autopilot/autopilot-issue-triage-worker/scripts/check_required_agents.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Check that required Copilot agent files exist in a consumer repository.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +_AGENT_SLUG = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*\Z") + + +def main() -> int: + """Report available and missing required agent roles as JSON.""" + parser = argparse.ArgumentParser( + description="Check required .github/agents/.agent.md files." + ) + parser.add_argument( + "--agents-dir", + type=Path, + default=Path(".github/agents"), + help="consumer agent directory (default: .github/agents)", + ) + parser.add_argument("roles", nargs="+", help="required agent role slugs") + args = parser.parse_args() + + invalid = [role for role in args.roles if not _AGENT_SLUG.fullmatch(role)] + if invalid: + parser.error(f"invalid role slug(s): {', '.join(invalid)}") + + roles = sorted(set(args.roles)) + available = [role for role in roles if (args.agents_dir / f"{role}.agent.md").is_file()] + missing = [role for role in roles if role not in available] + print( + json.dumps( + { + "agents_dir": str(args.agents_dir), + "available_roles": available, + "missing_roles": missing, + "ready": not missing, + }, + sort_keys=True, + ) + ) + if missing: + print( + f"Missing required agent roles in {args.agents_dir}: {', '.join(missing)}", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/autopilot/autopilot-pr-review-worker/SKILL.md b/packages/autopilot/autopilot-pr-review-worker/SKILL.md index 7d777e0152..bb7a6bfbbb 100644 --- a/packages/autopilot/autopilot-pr-review-worker/SKILL.md +++ b/packages/autopilot/autopilot-pr-review-worker/SKILL.md @@ -210,16 +210,20 @@ are active for this `panel-mode` and surface. Do not spawn | Agent | Role | When spawned | |-------|------|----------------| -| [Python Architect](.apm/agents/python-architect.agent.md) | Architectural Reviewer + mermaid in `full` | Core, unless docs/changelog-only. Optional in `lean` on tiny diffs. | -| [CLI Logging Expert](.apm/agents/cli-logging-expert.agent.md) | Output UX Reviewer | Surface-gated | -| [DevX UX Expert](.apm/agents/devx-ux-expert.agent.md) | Package-Manager UX | Surface-gated | -| [Supply Chain Security Expert](.apm/agents/supply-chain-security-expert.agent.md) | Threat-Model Reviewer | Surface-gated | -| [OSS Growth Hacker](.apm/agents/oss-growth-hacker.agent.md) | Adoption Strategist | Surface-gated | -| [Auth Expert](.apm/agents/auth-expert.agent.md) | Auth / Token Reviewer | Surface-gated | -| [Doc Writer](.apm/agents/doc-writer.agent.md) | Documentation Reviewer | Surface-gated | -| [Test Coverage Expert](.apm/agents/test-coverage-expert.agent.md) | Test-Presence Reviewer | Core unless docs-only (`src/` untouched) | -| [Performance Expert](.apm/agents/performance-expert.agent.md) | Package-Manager Performance Reviewer | Surface-gated | -| [APM CEO](.apm/agents/apm-ceo.agent.md) | Strategic Arbiter / Synthesizer | Always | +| `python-architect` | Architectural Reviewer + mermaid in `full` | Core, unless docs/changelog-only. Optional in `lean` on tiny diffs. | +| `cli-logging-expert` | Output UX Reviewer | Surface-gated | +| `devx-ux-expert` | Package-Manager UX | Surface-gated | +| `supply-chain-security-expert` | Threat-Model Reviewer | Surface-gated | +| `oss-growth-hacker` | Adoption Strategist | Surface-gated | +| `auth-expert` | Auth / Token Reviewer | Surface-gated | +| `doc-writer` | Documentation Reviewer | Surface-gated | +| `test-coverage-expert` | Test-Presence Reviewer | Core unless docs-only (`src/` untouched) | +| `performance-expert` | Package-Manager Performance Reviewer | Surface-gated | +| `apm-ceo` | Strategic Arbiter / Synthesizer | Always | + +These agent files are consumer-provided companion assets, not part of this +package. For a Copilot consumer, APM deploys them under +`.github/agents/.agent.md`. ## Topology @@ -244,6 +248,22 @@ are active for this `panel-mode` and surface. Do not spawn Caller (merge-worker, scheduler, or human) SHOULD set `panel-mode: full | lean | delta`. Omitted or unknown -> `lean`. +After resolving the active roster from the mode and surface, but before +spawning any panelist, run the bundled +`scripts/check_required_agents.py` with the active role slugs and the +consumer's `.github/agents` directory as `--agents-dir`. The script emits +JSON and returns nonzero when any active role is missing. Stop and report +all missing roles; do not skip a required role or produce a partial review. +Run it from the consumer repository root, for example: + +```bash +python3 .agents/skills/autopilot-pr-review-worker/scripts/check_required_agents.py \ + --agents-dir .github/agents python-architect test-coverage-expert apm-ceo +``` + +Replace the example roles with the exact active roster for the selected mode +and surface. + | Mode | Roster | Context | Comment | |------|--------|---------|---------| | full | Surface-gated specialists + CEO | Shared brief + owned files | yes | @@ -530,7 +550,7 @@ no comment can be rendered, an explicit `noop` (step 9) -- are emitted. 5. **Spawn the CEO synthesizer task.** Pass the spawned panelist JSON returns (omitted personas are inactive; do not invent stub JSON) to a `task` invocation that loads - `.apm/agents/apm-ceo.agent.md`. Run it as a BLOCKING task and WAIT + `.github/agents/apm-ceo.agent.md`. Run it as a BLOCKING task and WAIT for its JSON return -- do NOT spawn it detached (background mode that returns an `agent_id`) and do NOT end your turn while it runs. Its return is required to render the comment; ending the turn here is the diff --git a/packages/autopilot/autopilot-pr-review-worker/apm.yml b/packages/autopilot/autopilot-pr-review-worker/apm.yml index c32cb52a05..a86e667b5f 100644 --- a/packages/autopilot/autopilot-pr-review-worker/apm.yml +++ b/packages/autopilot/autopilot-pr-review-worker/apm.yml @@ -5,9 +5,9 @@ author: Microsoft license: MIT type: skill -# Stand-alone primitive: no transitive APM deps. The skill body honors -# the consumer repo's linting / encoding rules at runtime by activating -# whichever instructions the consumer has installed. +# Stand-alone primitive: no transitive APM deps. Specialist roles are +# consumer-provided companions checked in the deployed .github/agents/ +# directory before the worker begins the panel. dependencies: apm: - ../autopilot-comment diff --git a/packages/autopilot/autopilot-pr-review-worker/assets/panelist-return-schema.json b/packages/autopilot/autopilot-pr-review-worker/assets/panelist-return-schema.json index a05f7b240d..a20164f08c 100644 --- a/packages/autopilot/autopilot-pr-review-worker/assets/panelist-return-schema.json +++ b/packages/autopilot/autopilot-pr-review-worker/assets/panelist-return-schema.json @@ -128,11 +128,11 @@ }, "proves": { "type": "string", - "description": "One sentence in USER words naming the promise the test proves (or fails to prove if missing). MUST mirror a Scenario Evidence row from the PR body when one exists -- the rubric (.github/skills/pr-description-skill/assets/scenario-evidence-rubric.md) is the shared vocabulary." + "description": "One sentence in USER words naming the promise the test proves (or fails to prove if missing). MUST mirror a Scenario Evidence row from the PR body when one exists." }, "principles": { "type": "array", - "description": "APM principle taxonomy from the scenario-evidence rubric (Portability by manifest, Secure by default, Governed by policy, Multi-harness support, Vendor-neutral, DevX, OSS). Ranks the criticality of the proven (or missing) promise.", + "description": "APM principle taxonomy defined by the enum below. Ranks the criticality of the proven (or missing) promise.", "items": { "type": "string", "enum": [ diff --git a/packages/autopilot/autopilot-pr-review-worker/scripts/check_required_agents.py b/packages/autopilot/autopilot-pr-review-worker/scripts/check_required_agents.py new file mode 100644 index 0000000000..bd5b143e27 --- /dev/null +++ b/packages/autopilot/autopilot-pr-review-worker/scripts/check_required_agents.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Check that required Copilot agent files exist in a consumer repository.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +_AGENT_SLUG = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*\Z") + + +def main() -> int: + """Report available and missing required agent roles as JSON.""" + parser = argparse.ArgumentParser( + description="Check required .github/agents/.agent.md files." + ) + parser.add_argument( + "--agents-dir", + type=Path, + default=Path(".github/agents"), + help="consumer agent directory (default: .github/agents)", + ) + parser.add_argument("roles", nargs="+", help="required agent role slugs") + args = parser.parse_args() + + invalid = [role for role in args.roles if not _AGENT_SLUG.fullmatch(role)] + if invalid: + parser.error(f"invalid role slug(s): {', '.join(invalid)}") + + roles = sorted(set(args.roles)) + available = [role for role in roles if (args.agents_dir / f"{role}.agent.md").is_file()] + missing = [role for role in roles if role not in available] + print( + json.dumps( + { + "agents_dir": str(args.agents_dir), + "available_roles": available, + "missing_roles": missing, + "ready": not missing, + }, + sort_keys=True, + ) + ) + if missing: + print( + f"Missing required agent roles in {args.agents_dir}: {', '.join(missing)}", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/SKILL.md b/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/SKILL.md index c73b93219b..91666ab645 100644 --- a/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/SKILL.md +++ b/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/SKILL.md @@ -44,9 +44,9 @@ Rules: and processing / classification labels. Never assign. Never request reviewers. Do not call `gh pr comment` yourself. Do not write `status/accepted`, `status/needs-design`, or - `status/needs-triage`. Write `status/deferred` only when this - PR is not labelled `status/accepted` and has no same-repo - linked issue labelled `status/accepted`. + `status/needs-triage` or `status/deferred`. A defer + recommendation belongs only in the advisory comment; a human + decides whether to apply `status/deferred`. - `json` defaults to `off` when omitted or unknown. Omitted `json` is not a missing-field stop. - `json: off` -> no machine JSON receipt. @@ -97,14 +97,14 @@ classification labels from the contract. Never write `status/accepted`, `status/needs-design`, or `status/needs-triage`. -Auto-defer: if this PR is not labelled `status/accepted` and -has no same-repo linked issue labelled `status/accepted`, add -`status/deferred` (`write: on` only). -Do not overwrite `status/accepted` already on the PR. Linked -means `Fixes` / `Closes` / `#N` in the body or commits, or -GitHub `closingIssuesReferences`, same repository. Any one -accepted linked issue or `status/accepted` on the PR blocks -auto-defer. +When neither this PR nor a same-repo linked issue is labelled +`status/accepted`, recommend `needs-issue` and explain that a +human must review and accept scope before implementation. If +deferral is appropriate, recommend it in the advisory comment; +never write `status/deferred`. Linked means `Fixes` / `Closes` / +`#N` in the body or commits, or GitHub `closingIssuesReferences`, +same repository. An accepted linked issue or `status/accepted` on +the PR blocks the `needs-issue` recommendation. CODEOWNERS `reviewRequests` is runtime authority. Note owners in the comment. Never add or remove review requests. @@ -131,11 +131,11 @@ conversation, no-op (do not post a duplicate). `auto-handle`. 4. Neither this PR nor a linked same-repo issue is `status/accepted` -> recommend `needs-issue`, - add `status/deferred` when `write: on`, and thank the author. - Invite them to open an issue for maintainer review and - acceptance first, per CONTRIBUTING.md ("Start with an issue, - not an implementation."). Link - https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md + thank the author, and invite them to open an issue for + maintainer review and acceptance first, per CONTRIBUTING.md + ("Start with an issue, not an implementation."). Link + https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md. + Do not add `status/deferred`; any deferral is a human decision. 5. `ready-for-review` when this PR or a linked same-repo issue is `status/accepted`. It is not merge approval and is not a request to run `autopilot-pr-review-scheduler`. @@ -157,6 +157,5 @@ conversation, no-op (do not post a duplicate). - Do not run `autopilot-pr-review-worker` or `autopilot-pr-merge-worker`. - Do not contradict CODEOWNERS. - Do not treat labels or this comment as `status/accepted`. -- Do not write `status/deferred` when a linked issue is - `status/accepted` or the PR already has `status/accepted`. +- Never write `status/deferred` or any other human-decision label. - ASCII only. diff --git a/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md b/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md index 3a44bd05ae..26621180cc 100644 --- a/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md +++ b/packages/autopilot/autopilot-pr-triage-worker/.apm/skills/autopilot-pr-triage-worker/assets/pr-triage-template.md @@ -44,6 +44,6 @@ Thank you for contributing this pull request. APM starts with an issue, not an implementation (https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md). Please open an issue describing the user problem so a maintainer -can review and accept the scope first. This PR is labelled -status/deferred until that happens.> +can review and accept the scope first. This PR is awaiting maintainer +review and acceptance of its scope.> ``` diff --git a/packages/pr-description-skill/SKILL.md b/packages/pr-description-skill/SKILL.md index e0c521f487..86fb357b67 100644 --- a/packages/pr-description-skill/SKILL.md +++ b/packages/pr-description-skill/SKILL.md @@ -4,9 +4,9 @@ description: >- Use this skill to write the PR description (PR body) for any pull request opened against microsoft/apm. Produces one self-sufficient GitHub-Flavored Markdown artifact: TL;DR, Problem (WHY), Approach - (WHAT), Implementation (HOW), 1-3 validated mermaid diagrams, - explicit trade-offs, validation evidence, and a How-to-test - section -- with every WHY-claim backed by a verbatim quote from + (WHAT), Implementation (HOW), validated mermaid diagrams when + structure is non-trivial, explicit trade-offs, validation evidence, + and a How-to-test section -- with every WHY-claim backed by a PROSE or Agent Skills. Activate when the user asks to "write a PR description", "draft a PR body", "open a PR", "fill in the PR template", or any equivalent. @@ -72,7 +72,7 @@ Per-section ceilings (enforced by `assets/section-rubric.md`): | Problem (WHY) | max 6 bullets, max 3 quoted anchors total | | Approach (WHAT) | a table OR 3-7 bullets; may be skipped if PR is purely additive (say "additive: see Implementation") | | Implementation (HOW) | one short paragraph per file, OR a table; no prose walls | -| Diagrams | 1-3 mermaid blocks; every diagram preceded by a one-sentence legend | +| Diagrams | Include 1-3 validated mermaid blocks only when structure or control flow is non-trivial; simple, docs-only, and mechanical PRs may omit the section | | Trade-offs | 3-5 bullets; mechanical PRs may be 1-2 | | Benefits | 3-5 numbered items, each measurable | | Validation | copy-paste real command output; do not narrate | @@ -113,11 +113,12 @@ quote, it is downgraded to a "should" with the reason given. Anchor: Agent Skills, ["Add what the agent lacks, omit what it knows"](https://agentskills.io/skill-creation/best-practices). -4. **Visual aid where structure is non-trivial.** Any change that - touches more than one file or alters control flow SHOULD include - at least one mermaid diagram. Add a second only when the - relationships are non-trivial. Never add a third unless it earns - its place. Each diagram MUST be preceded by a one-sentence legend. +4. **Visual aid where structure is non-trivial.** Changes with + non-trivial structure or control flow SHOULD include 1-3 mermaid + diagrams. Simple, docs-only, and mechanical PRs may omit the + section. Add a second only when the relationships are non-trivial. + Never add a third unless it earns its place. Each diagram MUST be + preceded by a one-sentence legend. Anchor: Agent Skills, ["agents pattern-match well against concrete structures"](https://agentskills.io/skill-creation/best-practices). @@ -176,7 +177,7 @@ scannable. | 3 | Problem (WHY) | Observed failure modes; max 6 bullets, max 3 quoted anchors | | 4 | Approach (WHAT) | Table or 3-7 bullets; may say "additive: see Implementation" | | 5 | Implementation (HOW) | One short paragraph per file or a table | -| 6 | Diagrams | 1-3 validated mermaid blocks, each with a legend; diagram type chosen per intent (`assets/mermaid-conventions.md`) | +| 6 | Diagrams | Optional for simple, docs-only, and mechanical PRs; non-trivial structure or control flow requires 1-3 validated blocks, each with a legend and a type chosen per intent (`assets/mermaid-conventions.md`) | | 7 | Trade-offs | 3-5 bullets (1-2 if mechanical) | | 8 | Benefits | 3-5 numbered, measurable items | | 9 | Validation | Real command output, ideally inside `
` if long; **MUST include the Scenario Evidence subsection** (`assets/scenario-evidence-rubric.md`) for any behavior-change PR -- maps each user-promise scenario this PR touches to the test that proves it works, tagged with the APM principle the scenario serves | @@ -230,14 +231,14 @@ Run these steps in order. Tick each before moving on. 4. [ ] Fill in the template top-to-bottom using only facts from the activation contract. Every WHY-claim gets a verbatim quoted anchor. If you cannot anchor a claim, drop it. -5. [ ] Generate 1-3 mermaid diagrams. **Before drafting any block, - load `assets/mermaid-conventions.md`** to pick the right - diagram type per intent (sequenceDiagram for execution flow, - flowchart LR for pipeline / architecture, stateDiagram-v2 for - state machines) and apply the boxing convention for NEW - behavior. Add a one-sentence legend above each diagram. -6. [ ] **Validate every mermaid block deterministically (see - below). Do NOT save the draft until every block validates.** +5. [ ] Decide whether the PR has non-trivial structure or control + flow. Simple, docs-only, and mechanical PRs may omit the + Diagrams section. Otherwise, load `assets/mermaid-conventions.md` + and generate 1-3 diagrams of the right type (sequenceDiagram + for execution flow, flowchart LR for pipeline / architecture, + stateDiagram-v2 for state machines), with one-sentence legends. +6. [ ] If the draft contains mermaid blocks, **validate every block + deterministically (see below)** and do not save until all pass. 7. [ ] Load `assets/section-rubric.md` and run the self-check pass. Validation loop pattern from Agent Skills: ["do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes."](https://agentskills.io/skill-creation/best-practices). @@ -250,15 +251,15 @@ Run these steps in order. Tick each before moving on. ## Mandatory mermaid validation step -Run every mermaid block in the draft through `mmdc` and refuse to -save until all pass. +When the draft contains mermaid blocks, run each through `mmdc` and +refuse to save until all pass. ```bash # Extract mermaid blocks and validate each one. -# Requires: npx --yes -p @mermaid-js/mermaid-cli mmdc (one-shot, no global install needed) +# Requires: npx --yes -p @mermaid-js/mermaid-cli@12.0.0 mmdc awk '/^```mermaid/{n++; f=outdir"/diag"n".mmd"; getline; while($0 != "```") {print > f; getline}}' outdir=/tmp/mermaid-check pr-body-draft.md for f in /tmp/mermaid-check/diag*.mmd; do - npx --yes -p @mermaid-js/mermaid-cli mmdc -i "$f" -o "${f%.mmd}.svg" --quiet || { echo "INVALID: $f"; exit 1; } + npx --yes -p @mermaid-js/mermaid-cli@12.0.0 mmdc -i "$f" -o "${f%.mmd}.svg" --quiet || { echo "INVALID: $f"; exit 1; } done ``` @@ -292,8 +293,8 @@ of` closing rules, round brackets in node labels, inline - The file is **UTF-8 GitHub-Flavored Markdown**. Em dashes, smart quotes, Unicode in mermaid labels, alerts, and collapsibles are all permitted and encouraged where they improve readability. -- Every mermaid block has been validated by `mmdc` and renders - without error. +- Every included mermaid block has been validated by the pinned + `mmdc` package and renders without error. - The cite-or-omit rule applies absolutely. - The TL;DR is at most four sentences. - The body ends with the trailer: @@ -312,8 +313,9 @@ of` closing rules, round brackets in node labels, inline - Diagrams without a legend, OR diagrams that fail `mmdc`. - A TL;DR longer than four sentences. - Skipping any required section because "the PR is small". A small - PR can have a one-line Implementation per file, but the section - header must still be present. + PR can have a one-line Implementation per file, but all required + section headers must still be present. The Diagrams section is + optional for simple, docs-only, and mechanical changes. - Restating the diff line-by-line in Implementation. That is what the Files Changed tab is for. - Quoting a doc out of context. The self-check pass must verify diff --git a/packages/pr-description-skill/apm.yml b/packages/pr-description-skill/apm.yml index bd72672248..cfd8c68e9c 100644 --- a/packages/pr-description-skill/apm.yml +++ b/packages/pr-description-skill/apm.yml @@ -1,6 +1,6 @@ name: pr-description-skill version: 0.1.0 -description: Authors one anchored, mermaid-validated GitHub-Flavored Markdown PR body for any microsoft/apm pull request -- TL;DR, WHY/WHAT/HOW, trade-offs, validation evidence, how-to-test. Single-thread skill; consumed by the issue/PR orchestrators at every PR-open site and usable standalone. +description: Authors one anchored GitHub-Flavored Markdown PR body with mermaid diagrams validated when included -- TL;DR, WHY/WHAT/HOW, trade-offs, validation evidence, how-to-test. Single-thread skill; consumed by the issue/PR orchestrators at every PR-open site and usable standalone. author: Microsoft license: MIT type: skill diff --git a/packages/pr-description-skill/assets/pr-body-template.md b/packages/pr-description-skill/assets/pr-body-template.md index ad8923bf18..5a43273ff6 100644 --- a/packages/pr-description-skill/assets/pr-body-template.md +++ b/packages/pr-description-skill/assets/pr-body-template.md @@ -4,8 +4,8 @@ Loaded by pr-description-skill ONLY at synthesis time, after every row of the activation contract has been filled. Replace every with content drawn from the activation contract - inputs. Drop a section's body (keep its header) only when SKILL.md - explicitly allows it for this PR shape. + inputs. Omit an optional section only when SKILL.md allows it for + this PR shape. CHARSET: this template is ASCII because it lives in the repo and is subject to .github/instructions/encoding.instructions.md. The @@ -72,8 +72,10 @@ https://github.com/microsoft/apm/blob//path#L12-L34> ## Diagrams -<1-3 mermaid blocks. Each preceded by a one-sentence legend. Every -block MUST have been validated by mmdc before saving.> + list[dict]: def test_queue_helpers_match_across_schedulers_and_worker_contract() -> None: - """Issue and PR schedulers ship one helper implementation; worker has no queue scripts.""" + """Issue and PR schedulers share queue helpers; the worker has no queue scripts.""" pr = ( ROOT / "packages/autopilot/autopilot-pr-triage-scheduler/.apm/skills/autopilot-pr-triage-scheduler" @@ -281,7 +281,9 @@ def test_queue_helpers_match_across_schedulers_and_worker_contract() -> None: assert (PACKAGE / "assets/label-contract.json").read_bytes() == ( worker / "assets/label-contract.json" ).read_bytes() - assert not (worker / "scripts").exists() + assert not (worker / "scripts/fetch_queue.py").exists() + assert not (worker / "scripts/triage_state.py").exists() + assert (worker / "scripts/check_required_agents.py").is_file() def test_trusted_gh_uses_cli_owner(monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/tests/unit/test_workflow_actor_contract.py b/tests/unit/test_workflow_actor_contract.py index d164fe0e2a..00b4fb3f64 100644 --- a/tests/unit/test_workflow_actor_contract.py +++ b/tests/unit/test_workflow_actor_contract.py @@ -3,6 +3,8 @@ from __future__ import annotations import json +import subprocess +import sys from pathlib import Path import pytest @@ -181,6 +183,120 @@ def test_review_panel_declares_origin_intent_contract() -> None: assert "Never convert a failed reviewer request into an assignee write" in skill +def test_external_persona_preflight_fails_closed_for_clean_consumers(tmp_path: Path) -> None: + """Both workers name missing companion agents before running a panel.""" + packages = ( + ( + "autopilot-issue-triage-worker", + "devx-ux-expert", + "supply-chain-security-expert", + "apm-ceo", + ), + ("autopilot-pr-review-worker", "test-coverage-expert", "apm-ceo"), + ) + for package, *roles in packages: + package_root = ROOT / f"packages/autopilot/{package}" + script = package_root / "scripts/check_required_agents.py" + agents_dir = tmp_path / package / ".github/agents" + skill = (package_root / "SKILL.md").read_text(encoding="ascii") + + assert "consumer-provided companion assets" in skill + assert ".github/agents/.agent.md" in skill + assert "check_required_agents.py" in skill + assert "stop and report" in skill.lower() + assert "](../../agents/" not in skill + if package == "autopilot-pr-review-worker": + assert "`.github/agents/apm-ceo.agent.md`" in skill + assert "`.apm/agents/apm-ceo.agent.md`" not in skill + + help_result = subprocess.run( + [sys.executable, str(script), "--help"], + check=False, + capture_output=True, + text=True, + ) + assert help_result.returncode == 0 + assert "--agents-dir" in help_result.stdout + + missing = subprocess.run( + [sys.executable, str(script), "--agents-dir", str(agents_dir), *roles], + check=False, + capture_output=True, + text=True, + ) + assert missing.returncode == 1 + missing_report = json.loads(missing.stdout) + assert missing_report["missing_roles"] == sorted(roles) + assert missing_report["ready"] is False + assert "Missing required agent roles" in missing.stderr + + agents_dir.mkdir(parents=True) + for role in roles: + (agents_dir / f"{role}.agent.md").write_text("role", encoding="ascii") + ready = subprocess.run( + [sys.executable, str(script), "--agents-dir", str(agents_dir), *roles], + check=False, + capture_output=True, + text=True, + ) + assert ready.returncode == 0 + ready_report = json.loads(ready.stdout) + assert ready_report["available_roles"] == sorted(roles) + assert ready_report["missing_roles"] == [] + assert ready_report["ready"] is True + + +def test_pr_triage_never_writes_human_deferred_label() -> None: + """PR triage's write contract agrees with the shared human-label contract.""" + worker = _ascii(WORKER_PR_TRIAGE) + template = _ascii( + ROOT / "packages/autopilot/autopilot-pr-triage-worker/.apm/skills/" + "autopilot-pr-triage-worker/assets/pr-triage-template.md" + ) + contract_path = ( + ROOT / "packages/autopilot/autopilot-pr-triage-worker/.apm/skills/" + "autopilot-pr-triage-worker/assets/label-contract.json" + ) + contract = json.loads(contract_path.read_text(encoding="ascii")) + assert contract["human_decisions"]["status/deferred"] + assert "Never write `status/deferred`" in worker + assert "defer recommendation belongs only in the advisory comment" in worker + assert "add `status/deferred` when `write: on`" not in worker + assert "status/deferred" not in template + assert "awaiting maintainer review and acceptance of its scope" in template + + +def test_pr_description_diagrams_are_conditional_and_cli_is_pinned() -> None: + """A diagram is optional for simple PRs and validated reproducibly when used.""" + package = ROOT / "packages/pr-description-skill" + skill = _ascii(package / "SKILL.md") + template = _ascii(package / "assets/pr-body-template.md") + rubric = _ascii(package / "assets/section-rubric.md") + for text in (skill, template, rubric): + assert "simple" in text.lower() + assert "non-trivial" in text + assert "touches more than one file or alters control flow" not in skill + assert "@mermaid-js/mermaid-cli@12.0.0" in skill + assert "npx --yes -p @mermaid-js/mermaid-cli mmdc" not in skill + assert "1-3 mermaid blocks for any non-doc-only PR" not in rubric + + +def test_pr_review_schema_has_no_undeclared_rubric_path() -> None: + """The return schema keeps its principle vocabulary without a stale link.""" + schema_path = ( + ROOT / "packages/autopilot/autopilot-pr-review-worker/assets/panelist-return-schema.json" + ) + schema = json.loads(schema_path.read_text(encoding="ascii")) + serialized = json.dumps(schema) + assert ".github/skills/pr-description-skill" not in serialized + assert "scenario-evidence-rubric.md" not in serialized + assert "taxonomy defined by the enum below" in serialized + principles = schema["definitions"]["finding"]["properties"]["evidence"]["properties"][ + "principles" + ] + assert principles["items"]["enum"] + + def test_review_panel_requires_complete_paginated_context_and_watermark_noop() -> None: """Fresh advice is forbidden unless the full conversation was read.""" skill = _ascii(REVIEW_SKILL) @@ -415,8 +531,8 @@ def test_schedulers_own_isolated_fanout_pools() -> None: assert "needs-issue" in worker_pr_triage assert "Never request reviewers" in worker_pr_triage assert "Do not run `autopilot-pr-review-worker`" in worker_pr_triage - assert "Write `status/deferred` only when this" in worker_pr_triage - assert "PR is not labelled `status/accepted`" in worker_pr_triage + assert "Never write `status/deferred`" in worker_pr_triage + assert "When neither this PR nor a same-repo linked issue is labelled" in worker_pr_triage assert "Neither this PR nor a linked same-repo issue" in worker_pr_triage assert "Start with an issue" in worker_pr_triage assert "Never write human decision labels" not in worker_pr_triage