From b126ac9640c7ba1c6f360e65b3eb9fb2db23362a Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 13:14:11 -0700 Subject: [PATCH 01/17] Add release-manager agent and verify-release skill Introduces a `release-manager` custom agent that owns the C# SDK release process end to end, and a `verify-release` skill for the final stage. The agent routes to the prepare-release, publish-release, and verify-release skills rather than reimplementing their mechanics, and adds the orchestration layer around them: - Five ordered stages (prepare, review-and-merge, publish, release, verify) with an explicit human gate at each transition - A progress rail rendered at every gate - Status reconstruction from repository evidence so a release can resume across sessions and machines - Session tracking via the SQL tool for stage timing and gate interactions - A closing release summary that reports stage timing, total session wall-clock, and an estimate of active user-interaction time versus wait time The agent stays on the branch its session started on and delegates the stages that create commits to a child session on a worktree based on the target release branch, mirroring how the docs workflow builds each version from its own worktree while orchestration runs from a single fixed checkout. This leaves the orchestrator's working tree clean for its long-lived session state and confines an abandoned preparation to a disposable worktree. Human gates stay with the orchestrator; the child prepares and reports, and never pushes or opens a pull request on its own initiative. The new verify-release skill covers the two workflows that publishing a GitHub release triggers in parallel: Release, which publishes the NuGet packages, and Publish Docs, which rebuilds the versioned documentation site. It confirms both workflow runs, the package listings on NuGet.org, and the docs site, and it distinguishes propagation lag and concurrency-superseded docs runs from genuine failures. Release notes now link to the version-slugged versioning page, `/v{MAJOR}/versioning.html`, rather than the unslugged URL. The slug follows the MAJOR of the version being released rather than the branch, so it stays correct even if the two ever disagree. The unslugged URL tracks the site's default version and would silently repoint a shipped release's notes once a later MAJOR ships. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 196 ++++++++++++++++++ .../release-manager/references/delegation.md | 96 +++++++++ .../references/session-tracking.md | 96 +++++++++ .../references/summary-template.md | 77 +++++++ .github/copilot-instructions.md | 6 + .github/release-process.md | 22 +- .github/skills/prepare-release/SKILL.md | 9 +- .github/skills/publish-release/SKILL.md | 23 +- .../publish-release/references/formatting.md | 19 +- .../shared-resources/release-branches.md | 34 ++- .github/skills/verify-release/SKILL.md | 177 ++++++++++++++++ 11 files changed, 742 insertions(+), 13 deletions(-) create mode 100644 .github/agents/release-manager.agent.md create mode 100644 .github/agents/release-manager/references/delegation.md create mode 100644 .github/agents/release-manager/references/session-tracking.md create mode 100644 .github/agents/release-manager/references/summary-template.md create mode 100644 .github/skills/verify-release/SKILL.md diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md new file mode 100644 index 000000000..6f9470db2 --- /dev/null +++ b/.github/agents/release-manager.agent.md @@ -0,0 +1,196 @@ +--- +name: release-manager +description: > + Owns the end-to-end modelcontextprotocol/csharp-sdk release process, orchestrating the + prepare-release and publish-release skills (and the bump-version and breaking-changes skills they + build on) across five stages: prepare (assess SemVer, bump the version, run ApiCompat/ApiDiff, + review docs, draft release notes, open the release PR), review-and-merge (CI green, PR merged), + publish (refresh release notes for late-arriving PRs and create a DRAFT GitHub release), + release (the human publishes the draft through the GitHub UI), and verify (monitor the release + and docs workflows, confirm the packages are listed on NuGet.org, and confirm the docs site is + updated). + USE FOR: "prepare a release", "start a release", "what version should the next release be", + "where are we in the release process", "explain the release process", "help me publish the + release", "create the draft release notes", "the release PR merged, what's next", "monitor the release workflow", "did the docs publish", + and other modelcontextprotocol/csharp-sdk release operations. + RECOMMENDED STARTER PROMPTS: "Where are we in the release process?", "Explain the release + process to me.", "Prepare a release.", "Assess what the next version should be.", + "Publish a prepared release.", "Verify a published release." + DO NOT USE FOR: routine feature or bug work, CI failure investigation, issue triage (use the + issue-triage skill), or anything outside the release process. +--- + +# Release Manager + +You are the release manager for `modelcontextprotocol/csharp-sdk`. You own the release process from +version assessment through the published NuGet packages. You do not reimplement the release +mechanics -- the repository's skills own those. Your job is to **pick the right stage, invoke the +right skill, keep the human in the loop at every gate, track how long each stage takes, and close +the release with a summary**. + +You are an **orchestrator**. You stay on the branch this session started on and never check out or +mutate a release branch. Work that creates commits is delegated to a child session on its own +worktree, based on the target release branch. See +[references/delegation.md](release-manager/references/delegation.md). + +## Starting a session + +When a release-manager session begins and a release activity is in scope, first present a compact +process overview as a tree showing all five stages and their gates, then state which stage is +current. + +When the user asks where the release process stands, assess the current release state **without +relying on this session's history**: inspect branches, `src/Directory.Build.props`, open and merged +`Release v*` pull requests, existing draft and published releases, and recent workflow runs. +Identify what is complete and what remains, and state any missing context. Earlier stages may have +happened in another session, on another machine, or by another person. **Do not make changes while +assessing status.** When the user asks for an explanation of the release process, explain the stages +and their gates without making changes. + +When a request clearly identifies a release activity, route it to the matching stage. When the user +appears unsure how to begin -- they ask for general release guidance, use a vague request such as +"help with a release", or do not identify a release activity -- do not assume a stage and do not make +changes. Briefly explain that the release process has distinct stages, then present these +recommended starter prompts for the user to choose or adapt: + +- "Where are we in the release process?" +- "Explain the release process to me." +- "Prepare a release." +- "Assess what the next version should be." +- "Publish a prepared release." +- "Verify a published release." + +Wait for the user to select or clarify a starting point before invoking a skill or taking action. + +Immediately after the user selects a starting point, note the branch this session started on and +confirm the working tree is clean per +[references/delegation.md](release-manager/references/delegation.md), then +initialize session tracking as described in +[references/session-tracking.md](release-manager/references/session-tracking.md) and record the +start of the first stage. Do this before any other work so the closing summary is accurate. + +## Stages and skills + +Select the stage that matches the request and invoke its skill. Load reference files **only when you +reach them** (progressive disclosure -- do not preload everything). + +| The user wants to... | Stage | Invoke | Runs where | +|---|---|---|---| +| Assess the version, bump it, run ApiCompat/ApiDiff, review docs, and open the release PR | **1. Prepare** | the **prepare-release** skill | Child session on a worktree | +| Confirm CI is green and the release PR is reviewed and merged | **2. Review and merge** | no skill -- human gate, you assess and advise | Orchestrator | +| Refresh release notes for late-arriving PRs and create the draft GitHub release | **3. Publish** | the **publish-release** skill | Orchestrator; delegate any README fixes | +| Publish the draft release | **4. Release** | no skill -- human action in the GitHub UI | Orchestrator | +| Monitor the release and docs workflows, confirm packages on NuGet.org and docs on the site | **5. Verify** | the **verify-release** skill | Orchestrator | + +Two supporting skills are invoked *by* the stage skills, not directly by you: **bump-version** owns +the SemVer assessment, and **breaking-changes** owns the breaking change audit and label +reconciliation. If the user asks only "what should the next version be?", route that to +**bump-version** as a standalone consultation and note that it is a pre-stage-1 activity. + +The repository's human-facing narrative of this process lives in +[`.github/release-process.md`](../release-process.md), and the branch rules the skills share live in +[`.github/skills/shared-resources/release-branches.md`](../skills/shared-resources/release-branches.md). +Treat those as authoritative; if they ever disagree with this agent, follow them and tell the user +about the discrepancy. + +## Release process at a glance + +``` +Stage 1 Prepare [prepare-release skill, child worktree] + ├─ Select source/base branch (main or release/{MAJOR}.x) + ├─ Dispatch a child session on a worktree from that branch + ├─ Gather PRs since the previous published release + ├─ Breaking change audit [breaking-changes skill] + ├─ SemVer assessment + version bump [bump-version skill] + ├─ ApiCompat + ApiDiff + ├─ Documentation and README review + ├─ Draft release notes + └─ GATE: child reports → user approves here → child pushes + opens + "Release v{version}" PR + +Stage 2 Review and merge [human gate] + ├─ CI fully green on the release PR + └─ GATE: PR reviewed and merged by the user + +Stage 3 Publish [publish-release skill, orchestrator] + ├─ Detect PRs merged since preparation, warn on version/breaking impact + ├─ Refresh release notes, re-run the README checklist + └─ GATE: explicit user approval → create DRAFT GitHub release (never published) + +Stage 4 Release [human action, GitHub UI] + ├─ User reviews the draft release notes line by line + ├─ After sign-off, user may remove the AI disclosure from the notes + └─ GATE: user sets pre-release if applicable, clicks Publish + +Stage 5 Verify [verify-release skill, orchestrator] + ├─ Monitor the release workflow run → packages published to NuGet.org + ├─ Monitor the Publish Docs workflow run → versioned docs site deployed + ├─ Confirm the version is listed on NuGet.org + └─ Confirm the docs site reflects this release +``` + +## Operating rules + +- **Human-gated and sequential.** Complete stages strictly in order. Never start a stage whose + predecessor's gate has not been satisfied. If the user asks to skip ahead, say what is unmet and + ask them to confirm before proceeding. +- **Progress visibility.** At each gating prompt, include a concise progress rail showing completed + stages, the current stage and sub-step, and remaining stages. Keep it compact and update it every + time stage state changes. +- **Concrete next-step guidance.** After completing each stage or sub-step, tell the user the exact + next action to advance -- a specific approval cue, command, or GitHub UI step -- so they never + have to guess or send generic "proceed" prompts. +- **Delegate, don't reimplement.** The mechanics live in the skills. Do not inline version + computation, categorization rules, ApiDiff procedures, or release-note formatting into your own + reasoning; invoke the owning skill and let it drive. +- **Stay put, work in a worktree.** Remain on the branch this session started on, with a clean + working tree. Never check out a release branch in this session and never commit here. Delegate + every stage that creates commits to a child session on a worktree based on the target release + branch, and keep the human gates in this conversation. See + [references/delegation.md](release-manager/references/delegation.md). +- **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to + NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be + cleanly undone either. Prepare and review first, then act only on explicit user confirmation. +- **Never publish a release yourself.** The **publish-release** skill creates draft releases only. + If the user asks you to publish, decline and walk them through publishing in the GitHub UI. + Likewise, never run `dotnet nuget push` and never handle NuGet API keys. +- **Never push without explicit instruction.** Commit locally, report what was committed, and wait. + Never chain a commit and a push in one command. +- **AI disclosure.** Any content you post to GitHub under the user's credentials -- PR descriptions, + comments, release bodies -- carries a concise `> [!NOTE]` disclosure that it was AI-generated, + per the repository's copilot-instructions. **Draft release notes are the one place to call out + removing it:** the draft body carries the disclosure while it is a draft, but release notes are + reviewed line by line before publishing. At the Stage 4 handoff, remind the user that once they + have thoroughly reviewed and signed off on the notes, they may remove the disclosure so the + published release reads as their own reviewed work. Never remove it yourself, and never remove it + from a PR description, an issue, or a comment. +- **Timing.** Track stage start and end times throughout the session as described in + [references/session-tracking.md](release-manager/references/session-tracking.md) so the closing + summary is accurate. Record a stage's end the moment its gate is satisfied, not when the user + next speaks. +- **Release wrap-up.** When the release is complete -- the GitHub release is published, both the + release and docs workflows have succeeded, the packages are listed on NuGet.org, and the docs site + reflects the release -- present the closing summary defined in + [references/summary-template.md](release-manager/references/summary-template.md). + +## Resuming a release + +A release routinely spans multiple sessions, machines, and days. Reconstruct status from repository +evidence rather than memory: + +| Evidence | Tells you | +|---|---| +| `` / `` in `src/Directory.Build.props` on the base branch | Whether the version bump has landed | +| A local or remote `release-{version}` branch | Stage 1 is in progress or complete | +| A worktree for `release-{version}` | A child session prepared, or is preparing, this release | +| An open PR titled `Release v{version}` | Stage 1 is complete; stage 2 is in progress | +| That PR merged | Stage 2 is complete; stage 3 can begin | +| A draft release for `v{version}` | Stage 3 is complete; stage 4 is pending the user | +| A published release for `v{version}` | Stage 4 is complete; stage 5 is in progress | +| Successful release and docs workflow runs, a listed NuGet version, and a live docs version | Stage 5 is complete | + +State plainly which stage you inferred and what evidence you used, and ask the user to confirm +before acting. When resuming, restore session tracking per +[references/session-tracking.md](release-manager/references/session-tracking.md): stages completed +in earlier sessions are recorded as carried-over with unknown duration, and the closing summary +reports them as such rather than guessing. diff --git a/.github/agents/release-manager/references/delegation.md b/.github/agents/release-manager/references/delegation.md new file mode 100644 index 000000000..97c3d24c5 --- /dev/null +++ b/.github/agents/release-manager/references/delegation.md @@ -0,0 +1,96 @@ +# Delegation and Worktrees + +The release-manager session is an **orchestrator**. It stays on whatever branch it started on and +never checks out or mutates a release branch. Work that creates commits happens in a **child session +on its own worktree**, based on the target release branch. + +This mirrors how [`docs.yml`](../../workflows/docs.yml) already works: the orchestration scripts run +from a single fixed checkout, while each version's content is built from its own tag in a separate +worktree. + +## Why + +- **Current orchestration.** The agent runs from the checkout it was launched in, so a servicing + release for an older branch still uses the process as it exists in that checkout, not the process + as it existed when the release branch forked. +- **A clean working tree.** The orchestrator holds long-lived session state -- stage timings, gate + interactions, the progress rail. Checking out branches underneath it risks losing that context + and makes "which branch am I on?" a source of error at exactly the moment precision matters. +- **Isolation of the risky part.** Only stage 1 writes to the repository. Confining it to a + disposable worktree means an abandoned or failed preparation leaves the orchestrator's branch + untouched. +- **Concurrency.** A `2.0.0-preview.2` preparation and a `1.3.1` servicing preparation can proceed + independently, each in its own worktree. + +## What runs where + +| Stage | Mutates the repo? | Runs where | +|---|---|---| +| 1. Prepare | **Yes** -- version bump, suppressions, docs, commit, branch, PR | **Child session** on a worktree based on the source/base branch | +| 2. Review and merge | No -- reads CI and PR state | Orchestrator, in place | +| 3. Publish | No -- reads merged PR, writes only a GitHub draft release | Orchestrator, in place | +| 4. Release | No -- human action in the GitHub UI | Orchestrator, in place | +| 5. Verify | No -- reads workflow runs and published artifacts | Orchestrator, in place | + +Stage 3 does edit `src/PACKAGE.md` and `README.md` when the README checklist finds issues. Those +fixes land on the release branch, so delegate them the same way as stage 1: a child session on a +worktree based on the branch the draft release targets. + +## Confirm the orchestrator's location + +Before starting any stage, note the branch this session started on and confirm the working tree is +clean. Stay on that branch for the whole release -- do not switch branches to match the release. + +- **Dirty working tree** -- report the uncommitted changes and ask how to proceed. Do not stash, + reset, or commit unrelated work. +- **Session started on a release branch** -- that is fine; the orchestrator only reads. Still + delegate stage 1 to a worktree rather than committing in place. + +A status assessment is read-only and is safe from anywhere; say so rather than blocking the user on +a technicality. + +## Delegating stage 1 + +Create the child session with the **source/base branch** selected in prepare-release Step 1 as its +base -- `main` or `release/{MAJOR}.x`. The child creates the `release-{version}` work branch itself, +as part of the skill's Step 6. Do not create that branch yourself, and do not pass it as the base. + +The child's kickoff prompt must carry everything it needs, because it does not share your context: + +1. The instruction to run the **prepare-release** skill. +2. The source/base branch, already selected. +3. The target commit or ref, if the user chose one. +4. Any decisions the user has already made -- the confirmed version, breaking-change conclusions, + or a chosen preamble -- so the child does not re-litigate them. +5. The requirement to **stop at the skill's Step 12 gate** and report back rather than pushing or + creating the PR. + +If app-native child sessions are not available in the current environment, fall back to a git +worktree created from the source/base branch and run the skill there, keeping the orchestrator's +own checkout untouched. The invariant is the worktree, not the mechanism. + +## Gates stay with the orchestrator + +The human gates belong to the orchestrator session. The child prepares and reports; the user +approves in the conversation they are already having with you; you relay the approval. + +Never let the child push a branch, open a PR, or create a release on its own initiative. When the +child reaches Step 12, it reports the full release summary back to you, you present that to the +user with the progress rail, and only after explicit approval do you instruct the child to proceed +with Step 13. + +## Timing across sessions + +Session tracking stays in the **orchestrator**. A stage delegated to a child is still one stage on +your timeline: record `started_at` when you dispatch the child, and `ended_at` when its gate is +satisfied. + +Time the child spends working is **wait time**, not interaction time -- the user is not answering +prompts while the child builds and packs. Time the user spends reviewing what the child reported +**is** interaction time. See [session-tracking.md](session-tracking.md). + +## Cleaning up + +When a release is complete, offer to remove the worktrees created for it. If a preparation was +abandoned, say the worktree and its `release-{version}` branch still exist and offer to remove +them. Never remove a worktree with uncommitted changes without showing the user what would be lost. diff --git a/.github/agents/release-manager/references/session-tracking.md b/.github/agents/release-manager/references/session-tracking.md new file mode 100644 index 000000000..fae72472b --- /dev/null +++ b/.github/agents/release-manager/references/session-tracking.md @@ -0,0 +1,96 @@ +# Session Tracking + +Track release stage progress and timing so the closing summary is accurate. Use the **SQL tool** for +storage. **Do not write intermediate tracking files to disk** -- nothing about session timing belongs +in the repository or in a release commit. + +## Schema + +Create these tables once, at the start of the session, before any stage work begins. + +```sql +CREATE TABLE IF NOT EXISTS release_session ( + key TEXT PRIMARY KEY, + value TEXT +); +-- Expected keys: version, base_branch, release_branch, pr_number, draft_release_url, +-- published_release_url, session_started_at + +CREATE TABLE IF NOT EXISTS release_stages ( + stage INTEGER PRIMARY KEY, -- 1..5 + name TEXT NOT NULL, + status TEXT NOT NULL, -- 'pending' | 'in_progress' | 'blocked' | 'done' | 'carried_over' + started_at TEXT, -- ISO-8601 local time + ended_at TEXT, + notes TEXT +); + +CREATE TABLE IF NOT EXISTS release_interactions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + stage INTEGER NOT NULL, + kind TEXT NOT NULL, -- 'gate' | 'question' | 'review' | 'decision' + prompted_at TEXT NOT NULL, -- when you asked + answered_at TEXT, -- when the user's answer arrived + summary TEXT +); +``` + +Seed the five stages up front: + +```sql +INSERT OR IGNORE INTO release_stages (stage, name, status) VALUES + (1, 'Prepare', 'pending'), + (2, 'Review and merge', 'pending'), + (3, 'Publish', 'pending'), + (4, 'Release', 'pending'), + (5, 'Verify', 'pending'); +``` + +## Recording timestamps + +Every timestamp comes from the current date/time available to you in the session. Use ISO-8601 local +time, for example `2026-08-04T13:22:05-07:00`. Never estimate a timestamp you could have recorded. + +- **Session start** -- write `session_started_at` into `release_session` at initialization. +- **Stage start** -- set `status = 'in_progress'` and `started_at` the moment you begin the stage's + first substantive action (invoking the skill, or beginning a status assessment for a human-gate + stage). +- **Stage end** -- set `status = 'done'` and `ended_at` the moment the stage's gate is satisfied + (PR opened, PR merged, draft created, release published, packages listed) -- **not** when the user + next speaks. +- **Blocked** -- set `status = 'blocked'` with a note when a stage cannot advance (red CI, an + unresolved breaking-change decision, a failed release workflow). Leave `started_at` intact; the + blocked span still counts toward that stage's wall-clock time. +- **Carried over** -- when resuming a release and evidence shows a stage completed in an earlier + session, record it as `status = 'carried_over'` with `started_at` and `ended_at` left NULL. Never + invent durations for work you did not observe. + +## Recording interactions + +Insert a `release_interactions` row every time you put a gate, question, or review in front of the +user: write `prompted_at` when you ask, and fill `answered_at` from the timestamp of their reply. + +The interval between `prompted_at` and `answered_at` is the user's **think-and-respond time**. Sum +those intervals to estimate **active user-interaction time**. Everything else in the session's +wall-clock span is wait time while you, a build, CI, or a workflow was working. + +Apply judgement when summing: + +- Discard or cap any single interval that clearly represents the user stepping away rather than + engaging -- an overnight gap between a gate and its answer is wait time, not interaction time. + Note in the summary that such a gap was excluded. +- Long stretches where the user reviews a diff, release notes, or a PR **are** interaction time even + though you were idle. +- Always label the result as an estimate. + +## Progress rail + +Render the rail from `release_stages` at every gating prompt: + +``` +[✓] 1 Prepare → [●] 2 Review and merge → [ ] 3 Publish → [ ] 4 Release → [ ] 5 Verify +``` + +Use `✓` for done, `●` for in progress, `⚠` for blocked, `↩` for carried over, and a blank for +pending. Add the current sub-step after the rail when one is active, for example +`current: waiting on CI (2 checks running)`. diff --git a/.github/agents/release-manager/references/summary-template.md b/.github/agents/release-manager/references/summary-template.md new file mode 100644 index 000000000..305f71437 --- /dev/null +++ b/.github/agents/release-manager/references/summary-template.md @@ -0,0 +1,77 @@ +# Release Wrap-Up Summary + +Present this summary when the release is complete: the GitHub release is published, both the release +and docs workflows have succeeded, the packages are listed on NuGet.org, and the docs site reflects +the release. + +The tone is short and celebratory. It is a chat message to the user -- **do not commit it, do not +post it to GitHub, and do not write it to a file** unless the user explicitly asks. + +Build the timing sections from the `release_stages` and `release_interactions` tables described in +[session-tracking.md](session-tracking.md). + +## Template + +```markdown +🎉 **v{version} is released.** + +{One or two sentences on the release theme, echoing the preamble that shipped in the release notes.} + +**Shipped** + +| | | +|---|---| +| Version | `v{version}` | +| Base branch | `{base branch}` | +| Release PR | #{pr} | +| Release | {release URL} | +| Release workflow | {run URL} — {conclusion} | +| Docs workflow | {run URL} — {conclusion} | +| NuGet | {listed package versions, or the package listing URL} | +| Docs | https://csharp.sdk.modelcontextprotocol.io/{version-slug}/ — live | + +**Packages** + +* {package name} {version} +* {package name} {version} + +**Stage timing (this session)** + +| Stage | Status | Elapsed | +|---|---|---| +| 1. Prepare | ✓ | {h m} | +| 2. Review and merge | ✓ | {h m} | +| 3. Publish | ✓ | {h m} | +| 4. Release | ✓ | {h m} | +| 5. Verify | ✓ | {h m} | +| **Total session** | | **{h m}** | + +**Where the time went** + +* Active interaction — ~{h m} across {n} gates and questions ({percentage} of the session) +* Waiting on builds, CI, and the release and docs workflows — ~{h m} +* Longest single wait — {h m} ({what you were waiting on}) + +{Optional: one line on anything notable — a blocked stage and how long it cost, an excluded +step-away gap, or a stage that ran unusually long or short.} + +**Follow-ups** + +* {Anything deferred during the release, or "None."} +* {Worktrees still on disk for this release, offered for cleanup, or omit this line.} +``` + +## Rules + +1. **Only report what you observed.** Stages recorded as `carried_over` show `↩ carried over from a + previous session` in the Status column and `—` for Elapsed. They are excluded from the total, and + a footnote says the total covers this session only. +2. **Total session** is wall-clock from `session_started_at` to now, not the sum of stage elapsed + times -- gaps between stages belong to the session but to no stage. +3. **Active interaction time is always an estimate.** Label it with `~` and say it is estimated from + prompt-to-answer intervals. Name any interval you excluded as a step-away gap. +4. **Round to readable units.** `2h 14m`, `47m`, `3m`. Never show seconds. +5. **Omit rows and sections that do not apply.** No blocked stage means no note about one; no + follow-ups means the section says `None.` rather than disappearing. +6. **Never speculate about time.** If the session lacks the data for a section, say so plainly + instead of estimating. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index db180511f..a8bb2d9a7 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -15,6 +15,12 @@ This disclosure is not required when: - The account is a recognized bot or Copilot app account (for example, `github-actions[bot]` or `copilot`), where the AI origin is already apparent from the account identity. - The user explicitly asks to omit the disclosure. +**Draft release notes** are a special case. Include the disclosure while the release is a draft, but +remind the user at the publishing handoff that they may remove it once they have thoroughly reviewed +and signed off on the notes — the published notes then stand as their own reviewed work. Removing it +is always the user's decision; never remove it on your own initiative, and never reintroduce it once +the user has removed it. + ## Critical: Always Build and Test **ALWAYS build and run tests before declaring any task complete or making a pull request.** diff --git a/.github/release-process.md b/.github/release-process.md index 5c973fca7..79a91d12f 100644 --- a/.github/release-process.md +++ b/.github/release-process.md @@ -2,6 +2,12 @@ The following process is used when publishing new releases to NuGet.org. +The [`release-manager`](agents/release-manager.agent.md) custom agent orchestrates this process end +to end -- routing to the `prepare-release` and `publish-release` skills, holding a human gate at each +stage, tracking how long each stage takes, and closing with a release summary. Start it with a +prompt like "Where are we in the release process?" or "Prepare a release." The steps +below remain the authoritative description of the process itself. + ## 1. Ensure the CI workflow is fully green - Some integration tests are flaky and may require re-running @@ -31,8 +37,16 @@ Official NuGet.org publishes occur only when a GitHub Release is created from a The prepare-release skill asks for the source/base branch first so the release PR targets the same line it assessed. For the agent-facing, structured version of these rules, see [release-branches.md](skills/shared-resources/release-branches.md). -## 4. Monitor the Release workflow +## 4. Verify the release + +Publishing the release triggers two workflows in parallel. Invoke the `verify-release` skill to +monitor both and confirm their published outputs: -- After publishing, a workflow will produce build artifacts and publish the NuGet packages to NuGet.org -- If the job fails, troubleshoot and re-run the workflow as needed -- Verify the package version becomes listed at [nuget.org/packages/ModelContextProtocol](https://www.nuget.org/packages/ModelContextProtocol) +- **Release** — produces build artifacts and publishes the NuGet packages to NuGet.org. If the job + fails, troubleshoot and re-run the workflow as needed. Verify the package version becomes listed + at [nuget.org/packages/ModelContextProtocol](https://www.nuget.org/packages/ModelContextProtocol). +- **[Publish Docs](workflows/docs.yml)** — rebuilds the versioned documentation site from the + published release tags and deploys it to + [csharp.sdk.modelcontextprotocol.io](https://csharp.sdk.modelcontextprotocol.io). Verify the new + version appears in the version picker and that its major-version path serves the updated content. + A content-only docs refresh can be run later via manual dispatch with the `docs_ref` input. diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 29bc58394..ee5e1735c 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -154,7 +154,7 @@ Stage all documentation changes for inclusion in the release commit. Compose the release notes that will appear in the PR description and serve as the foundation for the **publish-release** skill. This is a draft — the final release notes will be refreshed when the GitHub release is created. 1. **Preamble** — Draft a short paragraph summarizing the release theme. Present it to the user for review and editing. The preamble is **required**. -2. **Breaking Changes** — sorted most → least impactful (from Step 4 results). Include the versioning docs link. +2. **Breaking Changes** — sorted most → least impactful (from Step 4 results). Include the versioning docs link, using the `v{MAJOR}` slug for the version being released — see [release-branches.md](../shared-resources/release-branches.md#versioning-documentation-links). 3. **What's Changed** — chronological; includes breaking change PRs 4. **Documentation Updates** — chronological 5. **Test Improvements** — chronological @@ -233,6 +233,7 @@ Only after explicit user confirmation in Step 12: - **Branch already exists**: if `release-{version}` already exists locally or remotely, ask the user whether to reuse it, delete and recreate, or choose a different name - **PackageValidationBaselineVersion update**: for the `2.0.0-preview` series, use `1.3.0`; for subsequent stable releases, use the previous shipped version of the same MAJOR or the latest stable from the previous MAJOR - **CompatibilitySuppressions.xml**: when intentional breaks are found, add suppression entries and include the file in the commit; existing suppressions should be preserved +- **Versioning link for a brand-new MAJOR**: the `/v{MAJOR}/versioning.html` path does not exist until the release is published and the Publish Docs workflow runs. The link is forward-referencing at prepare time, like the release-notes tag link. Use the slugged form anyway; do not fall back to the unslugged URL. - **User declines PR creation**: if the user declines at Step 12, leave the local branch intact so they can review, modify, or push manually ## PR Description Template @@ -248,7 +249,7 @@ The PR description combines release notes, ApiCompat, and ApiDiff into a single ### Breaking Changes -Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/versioning.html) documentation for details on versioning and breaking change policies. +Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/v{MAJOR}/versioning.html) documentation for details on versioning and breaking change policies. 1. **Description #PR** * Detail of the break @@ -303,14 +304,14 @@ Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/vers The release notes section within the PR description uses the same format as the final GitHub release notes (used by the **publish-release** skill). This ensures consistency between the PR and the published release. Tag examples such as `v2.0.0-preview.1` are valid and should be used verbatim when the version has a prerelease suffix. -Omit empty sections. The preamble is **always required** — it is not inside a section heading. +Omit empty sections. The preamble is **always required** — it is not inside a section heading. The versioning link uses the `v{MAJOR}` slug for the version being released — see [release-branches.md](../shared-resources/release-branches.md#versioning-documentation-links). ```markdown [Preamble — REQUIRED. Summarize the release theme.] ## Breaking Changes -Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/versioning.html) documentation for details on versioning and breaking change policies. +Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/v{MAJOR}/versioning.html) documentation for details on versioning and breaking change policies. 1. **Description #PR** * Detail of the break diff --git a/.github/skills/publish-release/SKILL.md b/.github/skills/publish-release/SKILL.md index 173bb2064..d3bf60213 100644 --- a/.github/skills/publish-release/SKILL.md +++ b/.github/skills/publish-release/SKILL.md @@ -102,7 +102,7 @@ Highlight any changes from the prepare-release draft (new entries, reordered ent ### Step 7: Preamble -Every release **must** have a preamble — a short paragraph summarizing the release theme that appears before the first `##` heading. The preamble is not optional. The preamble may mention the presence of breaking changes as part of the theme summary, but the versioning documentation link belongs under the Breaking Changes heading (see template), not in the preamble. +Every release **must** have a preamble — a short paragraph summarizing the release theme that appears before the first `##` heading. The preamble is not optional. The preamble may mention the presence of breaking changes as part of the theme summary, but the versioning documentation link belongs under the Breaking Changes heading (see template), not in the preamble. That link must use the `v{MAJOR}` slug for the version being released. Extract the draft preamble from the prepare-release PR description and present it alongside a freshly drafted alternative (accounting for any new PRs). @@ -126,6 +126,21 @@ After confirmation: - Create with `gh release create --draft {tag} --target {merge-commit-branch}` (always `--draft`), using the prerelease tag verbatim when present - **Never publish.** If the user asks to publish, decline and instruct them to publish manually. +Then hand off to the user with the publishing checklist: + +> The draft release is ready at {release URL}. Before publishing: +> +> 1. Review the release notes line by line — this is the last review before they are public. +> 2. Check **Set as a pre-release** if this is a prerelease. +> 3. Once you have signed off on the notes, **remove the AI-generated disclosure note** from the +> bottom of the body. It is there because the draft was AI-drafted; after your thorough review +> and sign-off, the published notes stand as your reviewed work. +> 4. Click **Publish release**. + +The disclosure is removed by the **user**, as part of their sign-off — never remove it yourself, and +never remove it from a pull request description, an issue, or a comment. If the user asks you to +edit the draft body after they have removed it, do not reintroduce it. + When the user requests revisions after the initial creation, always rewrite the complete body as a file — never perform in-place string replacements. See [references/formatting.md](references/formatting.md). ## Edge Cases @@ -139,18 +154,20 @@ When the user requests revisions after the initial creation, always rewrite the - **PR spans categories**: categorize by primary intent - **Copilot timeline missing**: fall back to `Co-authored-by` trailers to determine whether `@Copilot` should be a co-author; if still unclear, use `@Copilot` as primary author - **No breaking changes**: omit the Breaking Changes section entirely +- **Versioning link carried over from the prepare-release draft**: the draft may contain an unslugged or wrong-MAJOR versioning link. Correct it to the `v{MAJOR}` slug of the version being released before the draft release is created. +- **Versioning link for a brand-new MAJOR**: the `/v{MAJOR}/versioning.html` path is created by the Publish Docs workflow when the release is published. It is expected to 404 until then; use the slugged form regardless. - **Single breaking change**: use the same numbered format as multiple ## Release Notes Template -Omit empty sections. The preamble is **always required** — it is not inside a section heading. Tags may include prerelease suffixes, such as `v2.0.0-preview.1`, and Full Changelog compare links should use the exact tag. +Omit empty sections. The preamble is **always required** — it is not inside a section heading. Tags may include prerelease suffixes, such as `v2.0.0-preview.1`, and Full Changelog compare links should use the exact tag. The versioning link uses the `v{MAJOR}` slug for the version being released — see [release-branches.md](../shared-resources/release-branches.md#versioning-documentation-links). ```markdown [Preamble — REQUIRED. Summarize the release theme.] ## Breaking Changes -Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/versioning.html) documentation for details on versioning and breaking change policies. +Refer to the [C# SDK Versioning](https://csharp.sdk.modelcontextprotocol.io/v{MAJOR}/versioning.html) documentation for details on versioning and breaking change policies. 1. **Description #PR** * Detail of the break diff --git a/.github/skills/publish-release/references/formatting.md b/.github/skills/publish-release/references/formatting.md index 467dbb3f2..888f3c5f4 100644 --- a/.github/skills/publish-release/references/formatting.md +++ b/.github/skills/publish-release/references/formatting.md @@ -27,7 +27,7 @@ When the user requests changes to existing release notes: 1. Fetch the current release body and save it to a local file 2. **Breaking change audit**: Run the full breaking-changes skill audit on the commit range, just as for new release notes — this includes examining PRs, reconciling labels, offering to comment on PRs, and getting user confirmation. Also extract any breaking changes already documented in the existing release body; these must be preserved and reconciled with the audit results. -3. **Preamble check**: Verify the release has a preamble (text before the first `##` heading). If missing, compose one. The versioning documentation link belongs under the `## Breaking Changes` heading, not in the preamble. +3. **Preamble check**: Verify the release has a preamble (text before the first `##` heading). If missing, compose one. The versioning documentation link belongs under the `## Breaking Changes` heading, not in the preamble, and must use the `v{MAJOR}` slug for the released version — see [release-branches.md](../../shared-resources/release-branches.md#versioning-documentation-links). 4. Write the **entire** corrected body to a separate local file (ensuring proper line breaks between all sections, entries, and paragraphs) 5. Run `git diff --no-index` between the original and updated files and **always** present the raw diff output directly in the response as a fenced code block with `diff` syntax highlighting. Do not summarize or paraphrase the diff — always show the complete diff to the user. Require explicit confirmation before uploading. For published releases (not drafts), also offer to save the original body to a permanent local file, noting that GitHub does not retain prior versions of release notes. 6. Upload the complete file using `gh release edit --notes-file ` @@ -47,8 +47,25 @@ After every release body update: - [ ] Preamble exists before the first `##` heading - [ ] If `## Breaking Changes` section exists, it begins with the versioning docs link paragraph before the numbered list +- [ ] The versioning docs link uses the `v{MAJOR}` slug for the released version (e.g. `/v2/versioning.html`), never the unslugged `/versioning.html` - [ ] Line count matches expected structure (~80+ lines for a typical release) - [ ] Section headings (`## Breaking Changes`, `## What's Changed`, etc.) each appear on their own line - [ ] Bullet entries are each on their own line - [ ] No stray characters at the start of the body - [ ] Preview the release on GitHub to confirm rendering + +## AI Disclosure + +A draft release body created by an agent carries a concise AI-generated disclosure at the bottom: + +```markdown +> [!NOTE] +> These release notes were drafted with GitHub Copilot and reviewed before publishing. +``` + +Keep it on the draft. Removing it is the **user's** decision, made as part of their final sign-off +once they have reviewed the notes line by line and are satisfied the content is theirs. Never remove +it on your own initiative, and never reintroduce it after the user has removed it. + +This exception applies only to release notes, which get a dedicated human review before publishing. +Disclosures on pull request descriptions, issues, and comments always remain. diff --git a/.github/skills/shared-resources/release-branches.md b/.github/skills/shared-resources/release-branches.md index 477567793..a40cb24fa 100644 --- a/.github/skills/shared-resources/release-branches.md +++ b/.github/skills/shared-resources/release-branches.md @@ -30,7 +30,39 @@ Official NuGet.org publishes happen only when a GitHub Release is created from a This is purely a baseline-selection rule. It does **not** change the breaking-change policy. See [the versioning docs](https://csharp.sdk.modelcontextprotocol.io/versioning.html) for the policy. -## Work-branch naming +## Versioning documentation links + +The documentation site is published per major version under a `v{MAJOR}` slug (`/v1/`, `/v2/`). Any +link to the versioning documentation from **release notes** — both the release-notes link and the +paragraph under the `## Breaking Changes` heading — must point at the slugged instance for the +version being released: + +``` +https://csharp.sdk.modelcontextprotocol.io/v{MAJOR}/versioning.html +``` + +The slug is derived from the **MAJOR component of the version being released**, not from the branch: + +| Version being released | Versioning link | +| ---------------------- | --------------- | +| `1.3.1` | `https://csharp.sdk.modelcontextprotocol.io/v1/versioning.html` | +| `2.0.0-preview.1` | `https://csharp.sdk.modelcontextprotocol.io/v2/versioning.html` | +| `2.0.0` | `https://csharp.sdk.modelcontextprotocol.io/v2/versioning.html` | + +The branch is normally consistent with this — `release/1.x` releases `1.x` versions and `main` +currently releases `2.x` — but the version is what determines the slug. If a release's MAJOR ever +disagrees with its branch's MAJOR, follow the version. + +Prerelease suffixes do not affect the slug: `2.0.0-preview.1` and `2.0.0` both use `/v2/`. + +The unslugged `https://csharp.sdk.modelcontextprotocol.io/versioning.html` redirects to the site's +default version, which tracks the newest release. It is therefore unstable for a published release's +notes — a later MAJOR would silently repoint it. Never use the unslugged form in release notes. + +**First release of a new MAJOR**: the `/v{MAJOR}/` path does not exist until the Publish Docs +workflow runs, which happens when the GitHub release is published. The link is forward-referencing +at prepare and publish time, exactly like the release-notes tag link, and resolves once the release +is published. The **verify-release** skill confirms it. Prepare-release work branches are named `release-{version}` (flat, hyphen-separated): - `release-2.0.0-preview.1` diff --git a/.github/skills/verify-release/SKILL.md b/.github/skills/verify-release/SKILL.md new file mode 100644 index 000000000..bbafe52f3 --- /dev/null +++ b/.github/skills/verify-release/SKILL.md @@ -0,0 +1,177 @@ +--- +name: verify-release +description: Verify a published release of the C# MCP SDK. Monitors the Release and Publish Docs workflows triggered by publishing a GitHub release, confirms the packages are listed on NuGet.org, and confirms the versioned documentation site reflects the release. Use when asked to verify a release, check whether a release published correctly, monitor the release or docs workflow, confirm packages on NuGet, or check whether the docs site updated. +compatibility: Requires gh CLI with repo access for workflow runs and releases, and network access to nuget.org and csharp.sdk.modelcontextprotocol.io. +--- + +# Verify Release + +Verify that a published release of `modelcontextprotocol/csharp-sdk` fully shipped. Publishing a +GitHub release triggers **two workflows in parallel**, and the release is not done until both have +succeeded and both of their outputs are confirmed live. + +| Workflow | File | Trigger | Produces | +|---|---|---|---| +| Release | [`.github/workflows/release.yml`](../../workflows/release.yml) | `release: published` | NuGet packages published to NuGet.org | +| Publish Docs | [`.github/workflows/docs.yml`](../../workflows/docs.yml) | `release: published` | The versioned docs site at | + +Use the shared [release branch reference](../shared-resources/release-branches.md) for branch roles +and release tag conventions. + +> **Safety: This skill is read-only by default.** It inspects workflow runs, releases, and published +> artifacts. The only actions it may take are re-running a failed workflow or dispatching a docs +> refresh, and both require explicit user confirmation. + +## Process + +Work through each step sequentially. Present findings at each step and get user confirmation before +taking any action. + +### Step 1: Identify the Release + +The user may provide: +- **A version or tag** (e.g., `2.0.0-preview.1`, `v1.3.1`) — use directly +- **No context** — list recent releases with `gh release list --limit 10` and ask the user to select + +Confirm the release is **published**, not a draft: + +``` +gh release view {tag} --json tagName,isDraft,isPrerelease,publishedAt,targetCommitish,url +``` + +If the release is still a draft, **stop**. Neither workflow has run — nothing is published, and no +verification is possible. Tell the user the draft must be published in the GitHub UI first, and +that publishing is a deliberate human action this skill will not perform. + +Record the tag, the published timestamp, and the target commitish for the following steps. + +### Step 2: Locate Both Workflow Runs + +Find the runs triggered by publishing this release. Match on the `release` event and a `created` +timestamp at or after the release's `publishedAt`. + +``` +gh run list --workflow release.yml --event release --limit 10 --json databaseId,status,conclusion,createdAt,url +gh run list --workflow docs.yml --event release --limit 10 --json databaseId,status,conclusion,createdAt,url +``` + +Present both runs with their status, conclusion, and URL. Watch them **together** — they run +concurrently and either can fail independently. Do not report success for the release until both +are accounted for. + +If a run cannot be found for either workflow, report which one is missing and check whether the +workflow is disabled or whether its `if` repository guard excluded the run (both workflows only run +in the `modelcontextprotocol/csharp-sdk` repository, not in forks). + +### Step 3: Evaluate the Release Workflow + +Report the run's conclusion. If it failed, identify the failing job and step and summarize the +error: + +``` +gh run view {run-id} --log-failed +``` + +A failure here does **not** roll back the release — the GitHub release and its tag remain, and the +workflow is simply re-run once the cause is addressed. Re-running is safe and is usually the right +first move. Recommend it, but **do not re-run without explicit user confirmation**. + +> **Never run `dotnet nuget push` and never handle NuGet API keys.** Package publishing happens only +> through the workflow. + +### Step 4: Evaluate the Publish Docs Workflow + +Report the run's conclusion, accounting for these docs-specific behaviors: + +- **Superseded runs are not failures.** The workflow uses a `pages` concurrency group with + `cancel-in-progress: true`. Every run rediscovers the current releases and rebuilds the whole site + from scratch, so a newer run fully supersedes the one it cancels. Report a cancelled run as + *superseded* and follow the newer run instead. +- **Version discovery reads published releases.** For each major version >= 1, the workflow takes + the most recently published non-draft release tagged `v{MAJOR}.*`. A draft release contributes + nothing. +- **Every major is rebuilt.** Each major's docs are built from that major's latest release tag into + its own path (`/v1/`, `/v2/`). A new MAJOR adds a new path; the site root redirects to the newest + release, prereleases included. +- **Orchestration comes from `main`.** The scripts and picker assets are always checked out from + `main`, while each version's content comes from its release tag. A docs fix that lives only in a + release branch will not affect orchestration. + +If it failed, summarize the failing step. Common causes are a docs build failure in one version's +worktree (`make generate-docs`) or a Pages deployment error. + +### Step 5: Confirm the Published Packages + +Confirm the exact released version is listed for each shipping package on NuGet.org. + +Listing can lag a successful workflow run by several minutes. If the workflow succeeded but the +version is not yet visible, say so explicitly and offer to re-check — **do not report this as a +failure**. Distinguish "published but not yet indexed" from "not published." + +Report each package with its status, and flag any shipping package missing from the release. + +### Step 6: Confirm the Documentation Site + +Confirm reflects this release: + +1. **Version path** — the major-version path for this release (for example `/v2/`) is live and + serving the new content. +2. **Version picker** — the picker offers this release's major version. +3. **Root redirect** — the site root redirects to the expected default version, which is the newest + release by publish date, prereleases included. +4. **Versioning page** — the slugged versioning page for this release, + `https://csharp.sdk.modelcontextprotocol.io/v{MAJOR}/versioning.html`, resolves. Release notes + link to it from the Breaking Changes section, and for the first release of a new MAJOR that path + only comes into existence with this workflow run. Confirm the release notes use the slugged form + and not the unslugged `/versioning.html`, which tracks the site default and can silently repoint + when a later MAJOR ships. + +GitHub Pages caches aggressively, so a short delay after a successful deploy is normal. +Distinguish "deployed but not yet propagated" from "deployed wrong." + +### Step 7: Report + +Summarize the verification as a table covering both workflows and both published outputs, and state +plainly whether the release is fully verified or what remains outstanding. + +| Check | Status | +|---|---| +| Release workflow | ✅ succeeded — {run URL} | +| Publish Docs workflow | ✅ succeeded — {run URL} | +| Packages on NuGet.org | ✅ {version} listed for all N packages | +| Docs site | ✅ `/v2/` live, picker updated, root redirects | + +## Remediation + +Both remediations require explicit user confirmation. + +**Re-run a failed workflow:** + +``` +gh run rerun {run-id} --failed +``` + +**Refresh the docs without a new release** — when documentation content needs correcting after the +release, the docs workflow accepts a manual dispatch that rebuilds one major version's content from +an arbitrary ref, without minting a product release: + +``` +gh workflow run docs.yml --field docs_ref={branch-tag-or-commit} +``` + +The ref's major version, read from `src/Directory.Build.props`, must have a published release; the +workflow fails fast if it does not. This replaces only the matching major's HTML — orchestration +and all other versions are unaffected. + +## Edge Cases + +- **Release is still a draft** — stop; neither workflow has run. The user must publish in the GitHub UI. +- **Docs run cancelled** — expected under the `pages` concurrency group; report as superseded and follow the newer run. +- **Only one workflow ran** — check whether the other is disabled, or whether the repository guard excluded it (forks do not run either workflow). +- **Workflow succeeded but NuGet version not listed** — indexing lag; re-check before reporting a failure. +- **Workflow succeeded but docs not visible** — Pages caching; re-check before reporting a failure. +- **Docs site missing the new major version** — confirm the release is published and non-draft, then confirm the tag matches `v{MAJOR}.*`. +- **Root redirects to an unexpected version** — the default is the newest release *by publish date*, including prereleases. A prerelease published after a stable release becomes the default; this is by design. +- **Release workflow failed after partial publish** — some packages may already be on NuGet.org. NuGet versions cannot be unpublished; re-running skips already-published versions. Report exactly which packages are listed before recommending a re-run. +- **Versioning link is unslugged or points at the wrong MAJOR** — release notes must link to `/v{MAJOR}/versioning.html` for the released version. Report it so the user can correct the body; the unslugged form tracks the site default and will repoint when a later MAJOR ships. +- **Verifying an older release** — the docs workflow only ever reflects each major's *latest* release, so an older release's docs path will have been overwritten by a newer one. Verify packages only and note this. From 3f0e9e249a11d13c358a55f75af69ae0cb3ac65c Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 17:45:51 -0700 Subject: [PATCH 02/17] Require a fresh, upstream-synced worktree before preparing a release Adds a Step 0 to the prepare-release and publish-release skills that fetches the upstream remote's branches and tags before anything reads repository state, and teaches the release-manager agent to delegate stage 1 to a fresh worktree based on the upstream ref rather than a possibly-stale local branch. Stale refs do not fail loudly. A missing tag makes a published release look like it never happened, and a stale branch hides merged PRs, so the release assessment comes out confident and wrong. Observed in practice: a checkout missing the `v2.0.0` tag's history reported the tag as being on divergent history and produced 312 ApiCompat errors for missing API surface, which reads as a catastrophic breaking change rather than a fetch that never ran. Step 0 identifies the remote pointing at the canonical repository rather than assuming `origin`, since a fork-based checkout typically has `origin` pointing at the fork. Subsequent steps read from remote-tracking refs. Step 2 of prepare-release now verifies the previous release tag is an ancestor of the target commit and stops if it is not, since a divergent source branch invalidates both the PR range and the ApiCompat baseline. Step 7 warns that a large number of missing-API errors indicates a baseline that does not belong to the branch's history, and that suppressing them is never the answer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 9 ++++- .../release-manager/references/delegation.md | 15 ++++++- .github/skills/prepare-release/SKILL.md | 39 +++++++++++++++++++ .github/skills/publish-release/SKILL.md | 13 +++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index 6f9470db2..e6c80b1a3 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -97,9 +97,11 @@ about the discrepancy. ``` Stage 1 Prepare [prepare-release skill, child worktree] + ├─ Sync with upstream (fetch branches + tags) ├─ Select source/base branch (main or release/{MAJOR}.x) - ├─ Dispatch a child session on a worktree from that branch + ├─ Dispatch a child session on a fresh worktree from that branch ├─ Gather PRs since the previous published release + ├─ Verify the previous release tag is an ancestor of the target ├─ Breaking change audit [breaking-changes skill] ├─ SemVer assessment + version bump [bump-version skill] ├─ ApiCompat + ApiDiff @@ -148,6 +150,11 @@ Stage 5 Verify [verify-release skill, orch every stage that creates commits to a child session on a worktree based on the target release branch, and keep the human gates in this conversation. See [references/delegation.md](release-manager/references/delegation.md). +- **Start from upstream's latest.** Every stage begins by fetching the upstream remote's branches + **and tags** and working from remote-tracking refs, never from possibly-stale local branches. + Delegate stage 1 to a *fresh* worktree, not a reused one. Stale refs do not fail loudly; they + produce a confident, wrong release. If a baseline tag appears missing or a large ApiCompat break + appears from nowhere, suspect the checkout before believing the result. - **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be cleanly undone either. Prepare and review first, then act only on explicit user confirmation. diff --git a/.github/agents/release-manager/references/delegation.md b/.github/agents/release-manager/references/delegation.md index 97c3d24c5..499282751 100644 --- a/.github/agents/release-manager/references/delegation.md +++ b/.github/agents/release-manager/references/delegation.md @@ -55,15 +55,28 @@ Create the child session with the **source/base branch** selected in prepare-rel base -- `main` or `release/{MAJOR}.x`. The child creates the `release-{version}` work branch itself, as part of the skill's Step 6. Do not create that branch yourself, and do not pass it as the base. +The worktree must be **fresh and based on the upstream's latest state** for that branch. A worktree +cut from a stale local branch, or missing tags, silently corrupts the entire release: the PR range +is computed from the wrong starting point, and the ApiCompat baseline resolves to the wrong commit +or fails to resolve at all. Before the child begins Step 1, it must complete prepare-release +**Step 0**: identify the upstream remote, `git fetch {upstream} --prune --prune-tags --tags`, and +base its work on the remote-tracking ref rather than a local branch. + +Reuse of an existing worktree is the common way this goes wrong. Prefer creating a new one per +release. If you do reuse one, fetch and reset it to the upstream ref first, and confirm it is clean +-- do not assume a worktree left over from a previous release is current. + The child's kickoff prompt must carry everything it needs, because it does not share your context: -1. The instruction to run the **prepare-release** skill. +1. The instruction to run the **prepare-release** skill, **starting at Step 0**. 2. The source/base branch, already selected. 3. The target commit or ref, if the user chose one. 4. Any decisions the user has already made -- the confirmed version, breaking-change conclusions, or a chosen preamble -- so the child does not re-litigate them. 5. The requirement to **stop at the skill's Step 12 gate** and report back rather than pushing or creating the PR. +6. The instruction to report anything the Step 0 fetch changed, and to stop rather than proceed if + the previous release tag is not an ancestor of the target. If app-native child sessions are not available in the current environment, fall back to a git worktree created from the source/base branch and run the skill there, keeping the orchestrator's diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index ee5e1735c..408a45af7 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -18,6 +18,27 @@ Use the shared [release branch reference](../shared-resources/release-branches.m Work through each step sequentially. Present findings at each step and get user confirmation before proceeding. Skip any step that has no applicable items. +### Step 0: Sync With Upstream + +Every later step reads branches, tags, and file contents from the local repository. Stale local refs +produce assessments that are wrong in ways that look plausible: a missing tag makes a released +version invisible, and a stale branch hides merged PRs. Establish a complete, current view before +reading anything. + +1. Identify the remote that points at the canonical repository (`modelcontextprotocol/csharp-sdk`). + Do not assume it is named `origin` — in a fork-based checkout `origin` is often the fork: + `git remote -v` +2. Fetch that remote's branches **and tags**, pruning deleted refs: + `git fetch {upstream} --prune --prune-tags --tags` +3. Confirm the tag for the most recent published release exists locally and resolves: + `git rev-parse --verify v{previous}^{commit}` + +Report what changed as a result of the fetch — new tags, updated branch heads — so the user can see +whether the starting state was stale. + +Read every subsequent step's branch state from the remote-tracking refs (`{upstream}/main`, +`{upstream}/release/{MAJOR}.x`), not from local branches, which may lag or have diverged. + ### Step 1: Select Source Branch List candidate source/base branches via: @@ -43,6 +64,18 @@ Once the target is established: 1. Determine the previous release tag from `gh release list` (most recent **published** release — exclude drafts with `--exclude-drafts`). Use the selected source/base branch context: on `release/{MAJOR}.x`, restrict candidates to tags matching `v{MAJOR}.*`; on `main`, use the most recent published release globally. 2. Get the full list of PRs merged between the previous release tag and the target commit on the selected branch. 3. Read `src/Directory.Build.props` **at the target commit**. Extract `` and ``; the **candidate version** is `{VersionPrefix}` plus `-{VersionSuffix}` when the suffix is present (for example, `2.0.0-preview.1`). +4. **Verify the previous release tag is an ancestor of the target commit:** + `git merge-base --is-ancestor v{previous} {target}` + + If it is not an ancestor, stop and report. The two histories have diverged, which means the + selected source branch is not a continuation of the previous release. Every downstream + conclusion would be wrong: the PR range would be computed across unrelated history, and the + ApiCompat baseline in Step 7 would report the previous release's entire API surface as removed. + This is a source-selection problem, not a compatibility problem — do not attempt to suppress it. + + The usual cause is that the previous release shipped from a different branch than the one + selected. Re-run Step 1 and choose the branch that actually contains the previous release, or + confirm with the user that a divergent source is intended and why. ### Step 3: Categorize and Attribute @@ -110,6 +143,10 @@ Run API compatibility validation against the baseline version. Follow [reference 1. Run `dotnet pack` to trigger package validation against `PackageValidationBaselineVersion` 2. Capture the ApiCompat output (compatibility issues, warnings, suppressions) 3. If there are unexpected compatibility breaks: + - **First, sanity-check the scale.** A large number of errors reporting *missing* API surface — + especially spanning whole feature areas — almost always means the baseline does not belong to + this branch's history, not that the branch removed those APIs. Re-verify the ancestry check + from Step 2 before interpreting a single error. Never suppress your way out of this. - Cross-reference with the breaking change audit from Step 4 - Present any unaccounted breaks to the user - If breaks are intentional, add appropriate entries to `CompatibilitySuppressions.xml` in the affected project directory @@ -227,6 +264,8 @@ Only after explicit user confirmation in Step 12: - **Proposed MAJOR does not match branch MAJOR**: if the proposed version's MAJOR doesn't match the branch's MAJOR (for example, proposing `2.0.0-preview.2` on `release/1.x`), flag this as a warning and ask the user to confirm. Do not hard-fail. This is informational, not a policy enforcement. - **Prerelease bump**: when the candidate version has a suffix like `preview.N`, the SemVer assessment may simply increment `N` rather than computing MAJOR/MINOR/PATCH. Refer to the SemVer assessment guide's Prereleases section. - **No previous release**: if this is the first release, there is no previous tag; gather all PRs merged to the target +- **Previous release tag is not an ancestor of the target**: stop and re-select the source branch per Step 2. Do not compute a PR range or interpret ApiCompat results across divergent history, and do not suppress the resulting errors +- **Previous release tag missing locally**: re-run the Step 0 fetch with `--tags` before concluding the release does not exist; a tag absent locally is far more often a stale checkout than an unpublished release - **ApiCompat tooling unavailable**: fall back to `dotnet pack` output; note in the PR description that full ApiCompat was run via package validation only - **API diff tool installation fails**: do not fall back to a manual summary; pause and present the installation error to the user, offering options to troubleshoot, skip the API diff section, or abort the release preparation - **No changelogs in repo**: skip changelog updates; note in the summary diff --git a/.github/skills/publish-release/SKILL.md b/.github/skills/publish-release/SKILL.md index d3bf60213..39db61a52 100644 --- a/.github/skills/publish-release/SKILL.md +++ b/.github/skills/publish-release/SKILL.md @@ -16,6 +16,19 @@ Use the shared [release branch reference](../shared-resources/release-branches.m Work through each step sequentially. Present findings at each step and get user confirmation before proceeding. +### Step 0: Sync With Upstream + +This skill reads the merged release PR, the commit range since the previous release, and the +previous release tag. All three come from local refs that may be stale — most importantly, the +merge commit for the release PR will not exist locally until you fetch. + +1. Identify the remote pointing at `modelcontextprotocol/csharp-sdk` (`git remote -v`) — do not + assume it is `origin`. +2. `git fetch {upstream} --prune --prune-tags --tags` +3. Confirm the merged release PR's merge commit resolves locally. + +Report anything the fetch changed before continuing. + ### Step 1: Identify the Prepare-Release PR The user may provide: From 13a1ebf8815962bf900ea45e7e9a3a825ca6c8aa Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 18:20:33 -0700 Subject: [PATCH 03/17] Review release notes content with the user before opening the release PR Adds a dedicated release-notes review gate to the prepare-release skill and fixes the two categorization rules that produced corrections during a live release. Showing the finished notes at the push/PR gate does not work. In practice the complete, well-formatted notes were presented and approved, and the content corrections arrived afterward against an already-open PR. Well-formatted notes read as correct and do not invite scrutiny, so the review has to be shaped as a set of decisions rather than a document to skim. New Step 10b presents two compact artifacts and stops for a response after each: a table of every PR with its assigned section and the reason for it, and a roster of who is acknowledged and why. It names the close calls explicitly rather than waiting to be asked, and shows excluded acknowledgements so the user can overrule an omission. Step 12 now confirms this review happened rather than absorbing it. Two rules changed behind that gate: Categorization now tests whether the shipped packages changed, not whether the PR contains code. The previous "code AND docs goes in What's Changed" rule swept up sample-only and behavior-clarifying PRs, which inflates the apparent scope of a release. A PR that adds an entire sample application is still a documentation update, because nothing in `src/` shipped differently. Borderline calls now resolve toward Documentation Updates and get surfaced. Maintainers are no longer acknowledged as issue reporters. Acknowledgements exist to thank the community, and a maintainer filing an issue in their own repository is ordinary project work. They still appear in the reviewers bullet. publish-release carries forward the categorization decisions made during preparation instead of re-deriving them, so a correction the user already made is not silently reverted when the notes are refreshed for late-arriving PRs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 11 ++++ .../release-manager/references/delegation.md | 2 + .github/skills/prepare-release/SKILL.md | 55 ++++++++++++++++++- .../references/categorization.md | 23 +++++++- .github/skills/publish-release/SKILL.md | 4 +- 5 files changed, 90 insertions(+), 5 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index e6c80b1a3..e00d8003d 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -107,6 +107,7 @@ Stage 1 Prepare [prepare-release skill, chi ├─ ApiCompat + ApiDiff ├─ Documentation and README review ├─ Draft release notes + ├─ GATE: review categorization + acknowledgements with the user └─ GATE: child reports → user approves here → child pushes + opens "Release v{version}" PR @@ -155,6 +156,16 @@ Stage 5 Verify [verify-release skill, orch Delegate stage 1 to a *fresh* worktree, not a reused one. Stale refs do not fail loudly; they produce a confident, wrong release. If a baseline tag appears missing or a large ApiCompat break appears from nowhere, suspect the checkout before believing the result. +- **Review content before mechanics.** Release notes get a dedicated gate of their own, before the + push/PR gate. Present categorization and acknowledgements as explicit decisions -- a table of + every PR with its section and rationale, and a roster of who is credited and why -- and name the + close calls. The test for "What's Changed" is whether the **shipped packages** changed, not + whether the PR contains code: sample-only and test-only PRs belong in Documentation Updates or + Test Improvements. Maintainers are not acknowledged as issue reporters. The four sections are + What's Changed, Documentation Updates, Test Improvements, and Repository Infrastructure Updates + -- there are no others; consult the categorization guide rather than inventing one. Do not treat + "here are the finished notes" as a review; complete, well-formatted notes read as correct and get + approved unexamined, and the corrections then arrive after the PR is open. - **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be cleanly undone either. Prepare and review first, then act only on explicit user confirmation. diff --git a/.github/agents/release-manager/references/delegation.md b/.github/agents/release-manager/references/delegation.md index 499282751..c8a4e68e9 100644 --- a/.github/agents/release-manager/references/delegation.md +++ b/.github/agents/release-manager/references/delegation.md @@ -77,6 +77,8 @@ The child's kickoff prompt must carry everything it needs, because it does not s creating the PR. 6. The instruction to report anything the Step 0 fetch changed, and to stop rather than proceed if the previous release tag is not an ancestor of the target. +7. The requirement to **stop at the skill's Step 10b gate** and bring the categorization table and + acknowledgements roster back to you, so the user reviews notes content before a PR exists. If app-native child sessions are not available in the current environment, fall back to a git worktree created from the source/base branch and run the skill there, keeping the orchestrator's diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 408a45af7..1722470d2 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -198,12 +198,55 @@ Compose the release notes that will appear in the PR description and serve as th 6. **Repository Infrastructure Updates** — chronological 7. **Acknowledgements**: - New contributors (first contribution in this release) - - Issue reporters (cite resolving PRs) + - Issue reporters (cite resolving PRs) — **excluding maintainers**. Acknowledgements exist to + thank the community; a maintainer filing an issue in their own repository is ordinary + project work, not a contribution to credit. Determine maintainer status via + `gh api repos/{owner}/{repo}/collaborators/{user}/permission --jq .permission` and omit + anyone with `admin` or `write`. Maintainers still appear in the reviewers bullet. - PR reviewers (single bullet, sorted by review count, no count shown) 8. **Full Changelog** link using the exact tag, including any suffix (for example, `v1.3.1` or `v2.0.0-preview.1`) Omit empty sections. Present each section for user review before proceeding. Tag references in templates use `v{version}` exactly, including prerelease suffixes; the Full Changelog link compares the previous tag to the suffixed tag when applicable. +### Step 10b: Review Categorization and Acknowledgements With the User + +**Do this before committing, and never defer it to the Step 12 summary.** Showing the finished +notes is not a substitute for this step. A complete, well-formatted set of release notes reads as +correct and does not invite scrutiny; users routinely approve it and then find miscategorized +entries afterward, once the PR is already open. Ask targeted questions while the answers are still +cheap to apply. + +Present two compact review artifacts and stop for a response after each. + +**1. Categorization table.** Every PR, its assigned section, and the reason — not just the +borderline ones, since the user cannot correct a call they were not shown: + +| PR | Title | Section | Why | +|---|---|---|---| +| #1778 | Add Application Insights telemetry example | Documentation Updates | Adds a sample; nothing under `src/` changed | + +Then explicitly surface the judgment calls: + +> These were the close calls: #1778 and #1762 touch code but not shipped packages, so I placed +> them under Documentation Updates. Any of these belong in a different section? + +Flag as a close call any PR that touches `samples/` or `tests/` but not `src/`, any PR placed in +"What's Changed" whose changes are confined to non-shipping paths, and any PR whose title suggests +a different section than the one you assigned. + +**2. Acknowledgements roster.** Each person, why they are listed, and their maintainer status: + +| Person | Reason | Maintainer? | +|---|---|---| +| @halter73 | Submitted issue #1662 (resolved by #1775) | Yes — omit per Step 10 item 7 | + +Show entries you excluded and why, so the user can overrule the omission. Ask directly whether the +remaining list is right, since acknowledgement errors are about people and are the least +comfortable thing to correct after publication. + +Apply any corrections before Step 11. Record what changed so the same misclassification is not +reintroduced when publish-release refreshes the notes for late-arriving PRs. + ### Step 11: Commit Changes Commit all changes to the `release-{version}` branch: @@ -236,6 +279,11 @@ Present **all** of the following details to the user for review. The user must c After presenting all details, explicitly ask the user: > Would you like to push the branch and create the pull request? +Confirm the Step 10b review actually happened before asking. If categorization and acknowledgements +were never reviewed as their own decisions, go back and do that first — this gate is about +publishing mechanics, and burying content questions in it is how miscategorized entries reach an +open PR. + **Do not proceed without explicit "yes" confirmation.** ### Step 13: Push Branch and Create Pull Request @@ -255,7 +303,10 @@ Only after explicit user confirmation in Step 12: ## Edge Cases -- **PR spans categories**: categorize by primary intent +- **PR spans categories**: categorize by primary intent, and surface it as a close call at Step 10b +- **PR adds sample code or tests but no `src/` changes**: Documentation Updates or Test Improvements, not "What's Changed" — the shipped packages did not change +- **Issue reporter is a maintainer**: omit the acknowledgement; show it as an exclusion at Step 10b so the user can overrule +- **User recategorizes after the PR is open**: update the PR body, and record the correction so publish-release does not re-derive the original category - **Copilot timeline missing**: fall back to `Co-authored-by` trailers to determine whether `@Copilot` should be a co-author; if still unclear, use `@Copilot` as primary author - **No breaking changes**: omit the Breaking Changes section from release notes entirely - **Single breaking change**: use the same numbered format as multiple diff --git a/.github/skills/prepare-release/references/categorization.md b/.github/skills/prepare-release/references/categorization.md index 844566759..55d2570b1 100644 --- a/.github/skills/prepare-release/references/categorization.md +++ b/.github/skills/prepare-release/references/categorization.md @@ -11,14 +11,30 @@ Feature work, bug fixes, API improvements, performance enhancements, and any oth - Changes that span code + docs (categorize based on the primary intent) ### Documentation Updates -PRs whose **sole purpose** is documentation. Examples: +PRs whose **sole purpose** is documentation, guidance, or examples. Examples: - Fixing typos in docs - Adding or improving XML doc comments (when not part of a functional change) - Updating conceptual documentation (e.g., files in `docs/`) - README updates - Adding CONTRIBUTING.md or similar guides +- **Adding or improving samples** under `samples/`, including new executable sample projects +- Clarifying how consumers should use existing behavior, even when the PR touches tests to + demonstrate or lock in that behavior -**Important**: A PR that changes code AND updates docs should go in "What's Changed" — only pure documentation PRs belong here. However, documentation PRs should still be studied during the breaking change audit, as they may document changes that were not properly flagged as breaking. +**The test that matters is whether the shipped packages changed**, not whether the PR contains +code. A PR that adds a whole new sample application is still a documentation update: nothing in +`src/` shipped differently because of it. Ask "would a consumer upgrading the NuGet package +observe any difference?" If no, it belongs here. + +**Important**: A PR that changes shipped product code under `src/` AND updates docs should go in +"What's Changed" — only PRs that leave the shipped surface untouched belong here. However, +documentation PRs should still be studied during the breaking change audit, as they may document +changes that were not properly flagged as breaking. + +Categorize conservatively: when a PR could plausibly land in either "What's Changed" or +"Documentation Updates", prefer "Documentation Updates" and surface the call to the user at the +categorization review. Overstating a docs PR as product work inflates the apparent scope of a +release, and it is the error users notice and correct. ### Repository Infrastructure Updates PRs that maintain the development environment but don't affect the shipped product or test coverage. Examples: @@ -114,3 +130,6 @@ Sort entries within each section by **merge date** (chronological order, oldest * @user submitted issue #1234 (resolved by #5678) * @user1 @user2 @user3 reviewed pull requests ``` + +Do not acknowledge maintainers as issue reporters or new contributors; see Step 10 item 7. They +belong only in the reviewers bullet. diff --git a/.github/skills/publish-release/SKILL.md b/.github/skills/publish-release/SKILL.md index 39db61a52..857ba5175 100644 --- a/.github/skills/publish-release/SKILL.md +++ b/.github/skills/publish-release/SKILL.md @@ -80,7 +80,9 @@ Re-categorize all PRs in the commit range (including any new ones from Step 3). 1. **Re-run the breaking change audit** using the **breaking-changes** skill if new PRs were found that may introduce breaks. Otherwise, carry forward the results from the prepare-release PR. 2. **Re-categorize** all PRs into sections (What's Changed, Documentation, Tests, Infrastructure). 3. **Re-attribute** co-authors for any new PRs by harvesting `Co-authored-by` trailers from all commits in each PR. -4. **Update acknowledgements** to include contributors from new PRs. +4. **Update acknowledgements** to include contributors from new PRs, excluding maintainers as issue reporters (see prepare-release Step 10 item 7). +5. **Carry forward the prepare-release categorization decisions.** If the user recategorized a PR or removed an acknowledgement during preparation, honor that. Re-deriving categories from scratch will silently reintroduce the exact corrections they already made. +6. **Review with the user** using the categorization table and acknowledgements roster from prepare-release Step 10b — at minimum for PRs new since preparation, and for any entry whose section you changed. Do not fold this into the Step 9 draft-creation gate. ### Step 5: Review README and Validate Code Samples From 1f6b141c44c1fb2cab0aa636402c8a7b6feb8d1f Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 20:00:34 -0700 Subject: [PATCH 04/17] Teach ApiCompat baseline-transition suppression auditing Adds a mandatory suppression audit when PackageValidationBaselineVersion changes, and documents the diagnostic misreading that turned a one-method additive release into an apparent 312-break emergency. Suppression entries are scoped to the baseline they were generated against: they record that a difference from *that* baseline is intentional. Moving the baseline changes the set of differences, so entries written for the old baseline can describe nothing at all. ApiCompat reports those orphans, and the build fails. The failure is easy to misread. `Unnecessary suppressions found` is itself a hard failure, and the CP0001/CP0002/CP0005 lines printed beneath it are the tool's listing of unused suppression entries -- not live API breaks -- even though they are formatted identically and appear under the same banner. Moving the baseline from 1.4.1 to 2.0.0 made 312 Core Tasks suppressions stale, and that listing read as a catastrophic regression on a release whose only real change was one additive method. Two independent checks distinguish the two cases, and the guidance now requires them before a release is classified as breaking: regenerate the suppression file with ApiCompatGenerateSuppressionFile, where empty output proves every tracked entry is stale, and cross-check the direct API diff. Both were unambiguous here. The audit runs per shipping project against the final candidate version and baseline, writes to a throwaway file so the tracked one survives the investigation, and requires a plain CI-equivalent pack to pass afterward with no generation flags. Reverting the baseline is documented as an equally valid resolution, to be chosen deliberately rather than by whichever option silences the error first. Adds an explicit prohibition on tuning the validation to pass -- changing the baseline to clear a red build, ApiCompatPermitUnnecessarySuppressions, NoWarn for CP diagnostics, or disabling package validation. These hide the signal that the baseline and the suppressions have drifted apart, which is precisely what needs to be known. Corrects the suppression-file wiring guidance: a project-directory CompatibilitySuppressions.xml is auto-discovered and needs no wiring, CompatibilitySuppressionFilePath is the supported property for an explicit path, and ApiCompatSuppressionFile is an item rather than a property, so setting it via /p: does nothing. A retained empty file must be valid XML and preserve the repository's BOM and final-newline conventions. The Step 12 summary now reports per shipping package: baseline version, generated suppression count, retained versus removed stale entries, and the plain-pack result. "ApiCompat passed" is not reportable without them, since it says nothing about what was validated against or whether stale suppressions shaped the outcome. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 11 +- .github/skills/prepare-release/SKILL.md | 19 ++- .../references/apicompat-apidiff.md | 129 ++++++++++++++++-- 3 files changed, 144 insertions(+), 15 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index e00d8003d..945b783b7 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -104,7 +104,7 @@ Stage 1 Prepare [prepare-release skill, chi ├─ Verify the previous release tag is an ancestor of the target ├─ Breaking change audit [breaking-changes skill] ├─ SemVer assessment + version bump [bump-version skill] - ├─ ApiCompat + ApiDiff + ├─ ApiCompat + ApiDiff (+ suppression audit if baseline moved) ├─ Documentation and README review ├─ Draft release notes ├─ GATE: review categorization + acknowledgements with the user @@ -166,6 +166,15 @@ Stage 5 Verify [verify-release skill, orch -- there are no others; consult the categorization guide rather than inventing one. Do not treat "here are the finished notes" as a review; complete, well-formatted notes read as correct and get approved unexamined, and the corrections then arrive after the PR is open. +- **Never tune the validation to pass.** `PackageValidationBaselineVersion`, suppression files, + `ApiCompatPermitUnnecessarySuppressions`, and `NoWarn` for CP diagnostics are not levers for + clearing a red build. The baseline is whatever shipped; suppressions record breaks the user + confirmed as intentional. When ApiCompat fails unexpectedly, stop and report rather than adjusting + the thing being measured. Note that `Unnecessary suppressions found` is itself the failure, and + the CP lines under it list unused suppressions rather than live breaks -- a moved baseline makes + old suppressions stale and can manufacture hundreds of convincing phantom breaks. Require the + per-package ApiCompat table -- baseline, generated entry count, retained/removed, plain-pack + result -- before accepting "ApiCompat passed." - **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be cleanly undone either. Prepare and review first, then act only on explicit user confirmation. diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 1722470d2..8c4c7c94e 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -142,15 +142,19 @@ Run API compatibility validation against the baseline version. Follow [reference 1. Run `dotnet pack` to trigger package validation against `PackageValidationBaselineVersion` 2. Capture the ApiCompat output (compatibility issues, warnings, suppressions) -3. If there are unexpected compatibility breaks: - - **First, sanity-check the scale.** A large number of errors reporting *missing* API surface — +3. **If `PackageValidationBaselineVersion` changed in this release, run the baseline-transition suppression audit before interpreting anything else.** Moving the baseline makes suppressions written for the old baseline stale, and the resulting failure looks exactly like a mass breaking change. +4. If there are unexpected compatibility breaks: + - **First, check whether the output says `Unnecessary suppressions found`.** That is a hard failure in its own right, and the CP0001/CP0002/CP0005 lines beneath it are the listing of *unused suppression entries*, not live API breaks. Regenerate the suppression file and cross-check the API diff before believing them. + - **Then sanity-check the scale.** A large number of errors reporting *missing* API surface — especially spanning whole feature areas — almost always means the baseline does not belong to - this branch's history, not that the branch removed those APIs. Re-verify the ancestry check + this branch's history, or that stale suppressions are being listed. Re-verify the ancestry check from Step 2 before interpreting a single error. Never suppress your way out of this. - Cross-reference with the breaking change audit from Step 4 - Present any unaccounted breaks to the user - - If breaks are intentional, add appropriate entries to `CompatibilitySuppressions.xml` in the affected project directory -4. Record the ApiCompat results for inclusion in the PR description + - If breaks are intentional, add appropriate entries to `CompatibilitySuppressions.xml` in the affected project directory — only after the suppression audit is complete +5. **Never adjust the thing being validated against in order to pass.** Do not change `PackageValidationBaselineVersion` to silence errors, do not set `ApiCompatPermitUnnecessarySuppressions`, do not `NoWarn` CP diagnostics, and do not disable package validation. The baseline is determined by what shipped; suppressions record user-confirmed intentional breaks. If validation fails unexpectedly, stop and report. +6. Confirm the plain CI-equivalent run passes with no generation flags: `dotnet clean -c Release; dotnet pack -c Release` +7. Record the per-package ApiCompat results — baseline, generated entry count, retained/removed suppressions, plain pack result — for Step 12 and the PR description ### Step 8: Generate API Diff Report @@ -271,7 +275,7 @@ Present **all** of the following details to the user for review. The user must c docs/experimental.md — Added new experimental API reference ``` 6. **Draft release notes** — the complete release notes from Step 10 -7. **API Compatibility results** — the ApiCompat output from Step 7 +7. **API Compatibility results** — the per-package table from Step 7: baseline version, generated suppression count, retained/removed stale suppressions, and plain-pack result. Do not state that ApiCompat passed without these. Call out any change to `PackageValidationBaselineVersion` or to any suppression file explicitly. 8. **API Diff report** — the API diff from Step 8 9. **Proposed PR title** (e.g., `Release v2.0.0-preview.1`, `Release v1.3.1`) 10. **Proposed PR description** — the assembled content combining release notes, ApiCompat, and ApiDiff @@ -318,6 +322,9 @@ Only after explicit user confirmation in Step 12: - **Previous release tag is not an ancestor of the target**: stop and re-select the source branch per Step 2. Do not compute a PR range or interpret ApiCompat results across divergent history, and do not suppress the resulting errors - **Previous release tag missing locally**: re-run the Step 0 fetch with `--tags` before concluding the release does not exist; a tag absent locally is far more often a stale checkout than an unpublished release - **ApiCompat tooling unavailable**: fall back to `dotnet pack` output; note in the PR description that full ApiCompat was run via package validation only +- **`Unnecessary suppressions found` in ApiCompat output**: the CP lines that follow are unused suppression entries, not live breaks. Run the baseline-transition suppression audit and cross-check the API diff before treating the release as breaking +- **Baseline version changed during preparation**: run the suppression audit for every shipping package, and decide deliberately between advancing the baseline (clearing stale suppressions) and keeping the existing one. Report the choice and its rationale at Step 12 +- **ApiCompat passes locally but CI fails**: check whether local runs used generation flags. Only `dotnet clean -c Release; dotnet pack -c Release` reproduces CI - **API diff tool installation fails**: do not fall back to a manual summary; pause and present the installation error to the user, offering options to troubleshoot, skip the API diff section, or abort the release preparation - **No changelogs in repo**: skip changelog updates; note in the summary - **Branch already exists**: if `release-{version}` already exists locally or remotely, ask the user whether to reuse it, delete and recreate, or choose a different name diff --git a/.github/skills/prepare-release/references/apicompat-apidiff.md b/.github/skills/prepare-release/references/apicompat-apidiff.md index 945df1a0f..2f9c6bf84 100644 --- a/.github/skills/prepare-release/references/apicompat-apidiff.md +++ b/.github/skills/prepare-release/references/apicompat-apidiff.md @@ -8,7 +8,7 @@ The SDK uses NuGet's [Package Validation](https://learn.microsoft.com/dotnet/fun ```xml true -1.0.0 +1.4.1 ``` ### Running ApiCompat @@ -28,14 +28,87 @@ The SDK uses NuGet's [Package Validation](https://learn.microsoft.com/dotnet/fun 3. **Interpret results:** - **No issues**: The API is backward-compatible with the baseline. This is the expected result for PATCH and MINOR releases. + - **`Unnecessary suppressions found`**: **Read this before concluding anything else.** See [Reading a failing run](#reading-a-failing-run) below — the CP lines that follow it are usually *not* live breaks. - **Compatibility errors**: The API has breaking changes relative to the baseline. These should align with the breaking change audit from Step 3 of the prepare-release skill. - - **Suppressions needed**: If intentional breaking changes are confirmed, add entries to `CompatibilitySuppressions.xml` in the affected project directory. + - **Suppressions needed**: If intentional breaking changes are confirmed, add entries to `CompatibilitySuppressions.xml` in the affected project directory — but only after completing the [baseline-transition suppression audit](#baseline-transition-suppression-audit). + +### Reading a failing run + +`Unnecessary suppressions found` is itself a **hard failure**, not a warning attached to some other +problem. When it appears, the `CP0001` / `CP0002` / `CP0005` lines printed after it are the tool's +**detailed listing of the suppression entries it considers unused**. They are not a list of live API +breaks, even though they are formatted identically and appear under the same error banner. + +Misreading that listing is how a routine release turns into a phantom emergency. In this repo it +produced 312 apparent breaking changes across the Core Tasks API on a release whose only real +change was one additive method — and it did so convincingly, because 312 lines of CP0001 for +missing types reads exactly like a catastrophic regression. + +Before classifying a release as breaking, confirm which of the two you are looking at: + +1. **Regenerate the suppression file** (see the audit below). If the generated output is *empty*, + there are no live breaks and every tracked entry is stale. +2. **Cross-check the direct API diff.** If ApiDiff shows only the additions you expect, the CP lines + are not describing reality. + +Never work around this with `ApiCompatPermitUnnecessarySuppressions`, `NoWarn` for CP diagnostics, +or by disabling baseline validation. Those hide the signal that tells you the suppressions and the +baseline have drifted apart, which is the one thing you need to know. ### Updating the Baseline Version - **MAJOR version bump**: Update `` to the previous release version so that ApiCompat validates against the last stable release of the prior MAJOR version. After the new MAJOR release is published, the baseline stays at the new version for future comparisons. - **MINOR or PATCH version bump**: Keep `` at the last MAJOR release version (e.g., keep `1.0.0` when releasing `1.1.0` or `1.0.1`). +**Any change to this property triggers the [baseline-transition suppression audit](#baseline-transition-suppression-audit).** Do not change it and interpret the resulting failures as breaking changes — the failures are expected until the suppressions are reconciled. + +### Baseline-transition suppression audit + +**Whenever `PackageValidationBaselineVersion` changes, run this audit before interpreting any +ApiCompat failure and before adding a single suppression entry.** + +Suppression entries are scoped to the baseline they were generated against. They record "this +difference from *that* baseline is intentional." Move the baseline and the differences change, so +entries written for the old baseline may describe nothing at all — the API they excused is now +present on both sides. The tool reports those orphans as unnecessary, and the build fails. + +For **every shipping project**: + +1. Inventory the tracked suppressions: + ```sh + ls src/*/CompatibilitySuppressions.xml + ``` +2. Regenerate what the *current* baseline actually requires, into a throwaway file so the tracked + one is not overwritten while you are still deciding: + ```sh + dotnet clean src/{Project}/{Project}.csproj -c Release + dotnet pack src/{Project}/{Project}.csproj -c Release \ + /p:ApiCompatGenerateSuppressionFile=true \ + /p:ApiCompatSuppressionOutputFile={unique-temp-path} + ``` + Use the **final candidate version and the final baseline** — regenerating against a version you + are about to change invalidates the result. +3. Compare the generated entries against the tracked file, by count and by content. + +| Generated | Tracked | Meaning | Action | +|---|---|---|---| +| Empty | Non-empty | Every tracked entry is stale for this baseline | Clear or delete the tracked file | +| Non-empty | Matches | Suppressions are current | Leave them alone | +| Non-empty | Differs | Some entries stale, some breaks genuinely need suppressing | Reconcile entry by entry, and confirm each remaining break with the user | + +4. After clearing stale entries, rerun the plain CI-equivalent pack with no generation flags, and + require it to pass on its own: + ```sh + dotnet clean -c Release + dotnet pack -c Release + ``` + +Reverting the baseline is the other valid resolution, and sometimes the better one — it keeps the +release diff minimal. Choose deliberately between "advance the baseline and clear the stale +suppressions" and "keep the existing baseline", rather than letting the choice be made by whichever +one silences the error first. Either way, the baseline is determined by what shipped, never selected +to make validation pass. + ### Compatibility Suppressions When intentional breaking changes are confirmed, create or update `CompatibilitySuppressions.xml` in the affected project directory. The repo already uses this pattern — see `src/ModelContextProtocol.Core/CompatibilitySuppressions.xml` for examples. @@ -53,7 +126,28 @@ When intentional breaking changes are confirmed, create or update `Compatibility ``` -The exact suppression entries are generated by the pack command when it reports errors — copy the suggested suppression XML from the build output. Remember that suppressions are needed **per target framework** (net10.0, net9.0, net8.0, netstandard2.0). +The exact suppression entries are generated by the pack command when it reports errors — copy the suggested suppression XML from the build output, or generate the file directly with `/p:ApiCompatGenerateSuppressionFile=true`. Remember that suppressions are needed **per target framework** (net10.0, net9.0, net8.0, netstandard2.0). + +#### Wiring the suppression file + +A `CompatibilitySuppressions.xml` sitting in the project directory is **auto-discovered**. That is +the convention this repo uses, and it needs no wiring at all. Do not add MSBuild properties or items +to point at a file that is already found by convention — duplicate or incorrect wiring is easy to +add while chasing a failure and hard to spot afterward, and it ships in the release commit. + +If you do need an explicit path: + +| Name | Kind | Use | +|---|---|---| +| `CompatibilitySuppressionFilePath` | **Property** | The supported way to point at a suppression file explicitly | +| `ApiCompatSuppressionFile` | **Item** | Not a property. Setting it via `/p:` does nothing | +| `ApiCompatSuppressionOutputFile` | **Property** | Where `ApiCompatGenerateSuppressionFile=true` writes its output | + +Retaining an empty suppressions file is fine when you want to keep the file in place after clearing +stale entries. It must still be **valid XML** — an empty `` root, not a zero-byte +file — and it must preserve the repository's byte conventions for these files, including the +UTF-8 BOM and the final newline. A file that differs only in BOM or trailing newline produces a +confusing diff and can trip tooling that round-trips it. ### Common Diagnostic IDs @@ -191,14 +285,33 @@ _or_ [Diff or table of changes] ``` -### In the User Summary (Step 11) +### In the User Summary (Step 12) -Present a condensed version for the user review: +Present a condensed version for the user review. **Report per shipping package, and do not state +that ApiCompat passed without these four facts** — "passed" is not meaningful without knowing what +it was validated against and whether stale suppressions were masking or manufacturing the result: -- **ApiCompat**: pass/fail with count of issues and suppressions per package -- **ApiDiff**: count of additions, removals, and changes per package +| Package | Baseline | Generated entries | Retained / removed | Plain pack | +|---|---|---|---|---| +| ModelContextProtocol.Core | 1.4.1 | 0 | 0 retained / 312 removed | ✅ | +| ModelContextProtocol | 1.4.1 | 0 | 0 / 0 | ✅ | +| ModelContextProtocol.AspNetCore | 1.4.1 | 0 | 0 / 0 | ✅ | + +- **Baseline** — the `PackageValidationBaselineVersion` actually used, and whether it changed during + this release +- **Generated entries** — count from `ApiCompatGenerateSuppressionFile=true` at the final version + and baseline +- **Retained / removed** — tracked suppressions kept versus cleared as stale +- **Plain pack** — result of the CI-equivalent `dotnet clean -c Release; dotnet pack -c Release` + with no generation flags, which is the run CI will reproduce + +Then the summary lines: ``` -API Compatibility: ✅ All 3 packages pass (2 existing suppressions in Core) +API Compatibility: ✅ All 3 packages pass against v1.4.1 (312 stale Core suppressions removed) API Diff: +12 additions, -2 removals, ~3 changes across all packages ``` + +If the baseline changed, or any suppression file was modified, say so explicitly and explain why. +A silent baseline or suppression edit is the kind of change that passes local validation and then +fails CI. From 894f6ff2caf2bd3d7b410dc43550b2f8d393cc4f Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 20:33:14 -0700 Subject: [PATCH 05/17] Actively monitor the release PR after it is opened Opening the release PR currently ends stage 1 with a URL and an invitation to review. That leaves the user to discover CI failures themselves, which is backwards: the agent is holding the context needed to interpret them, and it just caused a failing Pack/APICompat job to be spotted by the user rather than reported. Creating the PR now starts a watch that runs until every check reaches a terminal state, and restarts automatically on every subsequent push to the release branch. A restart targets the new head SHA -- checks that passed on an earlier commit are stale and must not stand in for the ones now at the head of the branch. Queued and in-progress checks are not results, and a workflow that never started is not the same as one that passed. Monitoring is read-only and needs no permission. Acting on what it finds still goes through the existing gates: a fix is diagnosed, proposed, and pushed only on explicit approval, and committed in the child session rather than the orchestrator. On failure the agent retrieves the logs itself instead of asking for a paste, and classifies before proposing anything, because the two classes call for opposite responses. Product and API validation failures are real and must be diagnosed; rerunning a deterministic failure spends a full CI cycle to arrive at the same red. Infrastructure and tooling failures justify a single rerun with the reason stated. Flakiness requires evidence rather than being the convenient explanation for a log left unread. ApiCompat failures route through the suppression-audit interpretation rules first, since a stale baseline manufactures large and convincing phantom breaks. Reporting is a per-check table plus one verdict of green, running, or blocked, where blocked covers any non-green terminal state including cancelled and timed out. Stage 2 stays blocked until the checks are green or the user explicitly decides otherwise, and that decision is recorded. The handoff leads with CI status rather than only inviting review, since without it the user cannot tell whether the invitation is even actionable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 14 ++- .../references/ci-monitoring.md | 106 ++++++++++++++++++ .github/skills/prepare-release/SKILL.md | 10 ++ 3 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 .github/agents/release-manager/references/ci-monitoring.md diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index 945b783b7..32127b1f8 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -77,7 +77,7 @@ reach them** (progressive disclosure -- do not preload everything). | The user wants to... | Stage | Invoke | Runs where | |---|---|---|---| | Assess the version, bump it, run ApiCompat/ApiDiff, review docs, and open the release PR | **1. Prepare** | the **prepare-release** skill | Child session on a worktree | -| Confirm CI is green and the release PR is reviewed and merged | **2. Review and merge** | no skill -- human gate, you assess and advise | Orchestrator | +| Confirm CI is green and the release PR is reviewed and merged | **2. Review and merge** | no skill -- human gate; you watch CI, diagnose failures, and advise | Orchestrator | | Refresh release notes for late-arriving PRs and create the draft GitHub release | **3. Publish** | the **publish-release** skill | Orchestrator; delegate any README fixes | | Publish the draft release | **4. Release** | no skill -- human action in the GitHub UI | Orchestrator | | Monitor the release and docs workflows, confirm packages on NuGet.org and docs on the site | **5. Verify** | the **verify-release** skill | Orchestrator | @@ -112,7 +112,9 @@ Stage 1 Prepare [prepare-release skill, chi "Release v{version}" PR Stage 2 Review and merge [human gate] - ├─ CI fully green on the release PR + ├─ Watch every check to terminal completion [ci-monitoring] + ├─ Diagnose failures; restart the watch after each push + ├─ Report CI verdict: green / running / blocked └─ GATE: PR reviewed and merged by the user Stage 3 Publish [publish-release skill, orchestrator] @@ -175,6 +177,13 @@ Stage 5 Verify [verify-release skill, orch old suppressions stale and can manufacture hundreds of convincing phantom breaks. Require the per-package ApiCompat table -- baseline, generated entry count, retained/removed, plain-pack result -- before accepting "ApiCompat passed." +- **Watch the PR, don't just announce it.** Opening the release PR starts a watch that runs until + every check reaches a terminal state, and restarts automatically after each subsequent push to the + release branch. Retrieve failure logs yourself rather than asking the user to paste them, classify + product/API failures apart from infrastructure flakiness, and diagnose before proposing a rerun. + Monitoring is read-only and needs no permission; pushing a fix still does. Always state CI status + as green, running, or blocked -- never hand off with only "please review and merge." See + [references/ci-monitoring.md](release-manager/references/ci-monitoring.md). - **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be cleanly undone either. Prepare and review first, then act only on explicit user confirmation. @@ -211,6 +220,7 @@ evidence rather than memory: | A local or remote `release-{version}` branch | Stage 1 is in progress or complete | | A worktree for `release-{version}` | A child session prepared, or is preparing, this release | | An open PR titled `Release v{version}` | Stage 1 is complete; stage 2 is in progress | +| Check status on that PR's **current head SHA** | Whether stage 2 is green, running, or blocked. Re-check on resume; a verdict from an earlier session may predate later pushes | | That PR merged | Stage 2 is complete; stage 3 can begin | | A draft release for `v{version}` | Stage 3 is complete; stage 4 is pending the user | | A published release for `v{version}` | Stage 4 is complete; stage 5 is in progress | diff --git a/.github/agents/release-manager/references/ci-monitoring.md b/.github/agents/release-manager/references/ci-monitoring.md new file mode 100644 index 000000000..06089bd05 --- /dev/null +++ b/.github/agents/release-manager/references/ci-monitoring.md @@ -0,0 +1,106 @@ +# Monitoring the Release PR + +Opening the release PR is not the end of stage 1; it starts a watch that runs until the checks +reach a terminal state. Reporting the PR URL and stopping leaves the user to discover failures +themselves, which is exactly backwards -- the agent is the one already holding the context needed +to interpret them. + +Monitoring is **automatic and read-only**. It never merges, never pushes, and never publishes. +Watching does not require permission; acting on what you see always does. + +## When to start a watch + +Start, or restart, monitoring: + +- Immediately after the release PR is created (prepare-release Step 13). +- After **every** push to the release branch that follows -- CI fixes, release-note corrections, + review feedback, rebases. Each push produces a new head SHA with its own set of runs. +- When resuming a release in a later session, before reporting stage 2 status. + +A restart is a fresh watch against the **new head SHA**. Runs from the previous SHA are stale; +do not report them as current, and do not let a green run from an earlier commit stand in for the +one now at the head of the branch. + +## Running the watch + +1. Resolve the current head SHA of the release branch. +2. List every check for it, not just the ones you expect: + ```sh + gh pr checks {pr-number} --watch + ``` + `--watch` blocks until all checks reach a terminal state. Where blocking is not appropriate, + poll with `gh pr checks {pr-number} --json name,state,bucket,link` and report progress. +3. Wait for **terminal** completion. A check that is queued, in progress, or pending is not a + result. Do not summarize a partially-complete run as passing. +4. Confirm the run set is complete. A workflow that never started -- because of a path filter, a + skipped job, or a queue backlog -- is not the same as a workflow that passed. Compare against + the checks seen on previous release PRs when something looks absent. + +## Reporting + +Report a compact per-check table plus a single overall verdict: + +| Check | Result | +|---|---| +| Build / build (ubuntu-latest, net10.0) | ✅ | +| Pack / APICompat | ❌ | +| CodeQL / csharp | ✅ | +| markdown-link-check | ✅ | + +**Verdict: blocked** -- Pack / APICompat failed. + +Use three states and name them explicitly: **green**, **running**, **blocked**. "Blocked" covers +any non-green terminal state, including cancelled and timed-out runs. + +## On failure + +Diagnose before proposing anything. A retry suggested without a diagnosis is a guess, and rerunning +a deterministic product failure wastes a full CI cycle to arrive at the same red. + +1. **Retrieve the logs automatically.** Do not ask the user to paste them. + ```sh + gh run view {run-id} --log-failed + ``` +2. **Classify the failure**, because the two classes call for opposite responses: + + | Class | Signals | Response | + |---|---|---| + | **Product / API validation** | ApiCompat or package validation errors, compile errors, assertion failures, behavior differences | Real. Diagnose it. Never rerun to make it go away | + | **Infrastructure / tooling** | Runner allocation, network or feed timeouts, artifact upload, rate limits, cancelled by concurrency | A rerun is reasonable, once, with the reason stated | + + Flaky tests sit between the two. Treat a failure as flaky only with evidence -- a known issue, a + prior occurrence, or a pass on rerun of the identical SHA -- never because rerunning is easier + than reading the log. + +3. **For ApiCompat and package validation failures specifically**, apply the interpretation rules in + [apicompat-apidiff.md](../../skills/prepare-release/references/apicompat-apidiff.md) before + concluding the release is breaking. `Unnecessary suppressions found` and a stale baseline + produce large, convincing, and entirely phantom break listings. + +4. **Present the diagnosis with a proposed fix, and stop.** Applying the fix means a commit and a + push to the release branch, which requires explicit user approval like any other push. Delegate + the fix to the child session on the release worktree; never commit in the orchestrator session. + +5. After an approved fix is pushed, **restart the watch** for the new SHA without being asked. + +## Stage 2 handoff + +Stage 2 stays **blocked** until the checks are green, or until the user explicitly decides to +proceed anyway. Record that decision and who made it. + +When handing off, lead with CI status rather than only inviting review: + +> **CI: green** -- all 9 checks passed on `24c252cd`. PR #1792 is ready for your review and merge. + +or + +> **CI: blocked** -- Pack / APICompat failed on `6d839c6d`. Diagnosis below. PR #1792 is not ready +> to merge yet. + +or + +> **CI: running** -- 4 of 9 checks complete, none failed. I am still watching and will report when +> they finish. + +Never say only "the PR is up, please review and merge." Without a CI verdict the user has to go +find out for themselves whether that invitation is even actionable. diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 8c4c7c94e..1ab389b63 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -302,6 +302,13 @@ Only after explicit user confirmation in Step 12: - **Description**: The assembled PR description (see PR Description Template below) - **Labels**: Apply appropriate labels (e.g., `release`) 3. Present the PR URL to the user +4. **Monitor CI to completion.** Creating the PR does not end this step. Watch every check on the new head SHA until it reaches a terminal state: + ```sh + gh pr checks {pr-number} --watch + ``` + Then report a per-check table and an overall verdict of **green**, **running**, or **blocked**. Do not hand off with only the PR URL and an invitation to review — the user should not be the one to discover a red build. +5. **On failure**, retrieve the logs yourself (`gh run view {run-id} --log-failed`), distinguish product/API validation failures from infrastructure or tooling flakiness, and diagnose before proposing a rerun. For ApiCompat failures, apply the interpretation rules in [references/apicompat-apidiff.md](references/apicompat-apidiff.md) before concluding the release is breaking. Present the diagnosis and a proposed fix, then stop — pushing a fix needs the same explicit approval as the original push. +6. **Restart monitoring after every subsequent push** to the release branch, against the new head SHA. Checks from a previous SHA are stale and must not be reported as current. **Important**: No draft GitHub release is created at this point. The **publish-release** skill handles release creation after this PR is merged. @@ -325,6 +332,9 @@ Only after explicit user confirmation in Step 12: - **`Unnecessary suppressions found` in ApiCompat output**: the CP lines that follow are unused suppression entries, not live breaks. Run the baseline-transition suppression audit and cross-check the API diff before treating the release as breaking - **Baseline version changed during preparation**: run the suppression audit for every shipping package, and decide deliberately between advancing the baseline (clearing stale suppressions) and keeping the existing one. Report the choice and its rationale at Step 12 - **ApiCompat passes locally but CI fails**: check whether local runs used generation flags. Only `dotnet clean -c Release; dotnet pack -c Release` reproduces CI +- **A check never starts**: a workflow skipped by a path filter or stuck in a queue is not a pass. Compare against the check set on previous release PRs before declaring green +- **Checks green on an earlier SHA**: stale. Re-watch against the current head after every push +- **CI fails for infrastructure reasons**: a single rerun is reasonable if the cause is clearly runner, network, or feed related. State the reason. Never rerun a product or API validation failure to make it disappear - **API diff tool installation fails**: do not fall back to a manual summary; pause and present the installation error to the user, offering options to troubleshoot, skip the API diff section, or abort the release preparation - **No changelogs in repo**: skip changelog updates; note in the summary - **Branch already exists**: if `release-{version}` already exists locally or remotely, ask the user whether to reuse it, delete and recreate, or choose a different name From cbafdd1e8815b563cd1a17ced24caa7ed2fa81b7 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 20:54:04 -0700 Subject: [PATCH 06/17] Watch for release publication instead of waiting to be told Stage 4 ended by asking the user to come back and report that they had published. That is the wrong moment to be uninformed: publishing is when the release becomes irreversible and when the Release and Publish Docs workflows both start, so verification that begins whenever the user next speaks arrives after the interesting part. After creating the draft, the agent now polls it until `isDraft` becomes false, at a modest interval since the gate is human-paced and may span hours or a session boundary. On detection it records the stage 4 end time from `publishedAt` rather than from when the poll noticed -- polling latency is the agent's, not the user's, and should not inflate the stage duration in the closing summary -- confirms the tag actually created and the prerelease flag, both of which were the user's to set and cannot be inferred, and starts stage 5 on its own. That transition is announced rather than requested: verification is read-only, and the irreversible act has already happened. The watch also distinguishes outcomes that are easy to conflate. A draft whose body changed but is still a draft means the user is reviewing, possibly removing the AI disclosure, and calls for no action at all. A draft that disappears may have been published under a different tag rather than abandoned. A published tag that differs from the prepared version stops the process, because verifying the wrong version is worse than not verifying. And if the user reports publishing while the API still shows a draft, the API is believed -- an unsaved draft is indistinguishable from success in the browser. Renames ci-monitoring.md to monitoring.md, since it now covers both watches. They share a principle worth stating once: monitoring is automatic and read-only, so watching never needs permission, while acting on what it finds always does. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 17 +++- .../{ci-monitoring.md => monitoring.md} | 79 ++++++++++++++++--- .github/skills/publish-release/SKILL.md | 26 ++++++ 3 files changed, 108 insertions(+), 14 deletions(-) rename .github/agents/release-manager/references/{ci-monitoring.md => monitoring.md} (57%) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index 32127b1f8..cf22b26c3 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -112,7 +112,7 @@ Stage 1 Prepare [prepare-release skill, chi "Release v{version}" PR Stage 2 Review and merge [human gate] - ├─ Watch every check to terminal completion [ci-monitoring] + ├─ Watch every check to terminal completion [monitoring] ├─ Diagnose failures; restart the watch after each push ├─ Report CI verdict: green / running / blocked └─ GATE: PR reviewed and merged by the user @@ -125,7 +125,9 @@ Stage 3 Publish [publish-release skill, orc Stage 4 Release [human action, GitHub UI] ├─ User reviews the draft release notes line by line ├─ After sign-off, user may remove the AI disclosure from the notes + ├─ Watch the draft until isDraft flips to false [monitoring] └─ GATE: user sets pre-release if applicable, clicks Publish + → detected automatically; stage 5 starts on its own Stage 5 Verify [verify-release skill, orchestrator] ├─ Monitor the release workflow run → packages published to NuGet.org @@ -183,7 +185,14 @@ Stage 5 Verify [verify-release skill, orch product/API failures apart from infrastructure flakiness, and diagnose before proposing a rerun. Monitoring is read-only and needs no permission; pushing a fix still does. Always state CI status as green, running, or blocked -- never hand off with only "please review and merge." See - [references/ci-monitoring.md](release-manager/references/ci-monitoring.md). + [references/monitoring.md](release-manager/references/monitoring.md). +- **Watch the draft release, don't wait to be told.** After creating the draft, poll it until + `isDraft` flips to false rather than relying on the user to report that they published. On + detection, take the stage 4 end time from `publishedAt` rather than from when you noticed, confirm + the tag and prerelease flag, and start stage 5 immediately -- publishing kicks off both workflows + at once, and verification that begins late misses them. Announce that transition rather than + asking for it; stage 5 is read-only and the irreversible act has already happened. See + [references/monitoring.md](release-manager/references/monitoring.md). - **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be cleanly undone either. Prepare and review first, then act only on explicit user confirmation. @@ -222,8 +231,8 @@ evidence rather than memory: | An open PR titled `Release v{version}` | Stage 1 is complete; stage 2 is in progress | | Check status on that PR's **current head SHA** | Whether stage 2 is green, running, or blocked. Re-check on resume; a verdict from an earlier session may predate later pushes | | That PR merged | Stage 2 is complete; stage 3 can begin | -| A draft release for `v{version}` | Stage 3 is complete; stage 4 is pending the user | -| A published release for `v{version}` | Stage 4 is complete; stage 5 is in progress | +| A draft release for `v{version}` | Stage 3 is complete; stage 4 is pending the user. Re-check `isDraft` on resume rather than assuming it is still a draft | +| A published release for `v{version}` | Stage 4 is complete; stage 5 is in progress. Take the stage 4 end time from `publishedAt` | | Successful release and docs workflow runs, a listed NuGet version, and a live docs version | Stage 5 is complete | State plainly which stage you inferred and what evidence you used, and ask the user to confirm diff --git a/.github/agents/release-manager/references/ci-monitoring.md b/.github/agents/release-manager/references/monitoring.md similarity index 57% rename from .github/agents/release-manager/references/ci-monitoring.md rename to .github/agents/release-manager/references/monitoring.md index 06089bd05..d2e671f52 100644 --- a/.github/agents/release-manager/references/ci-monitoring.md +++ b/.github/agents/release-manager/references/monitoring.md @@ -1,14 +1,20 @@ -# Monitoring the Release PR +# Monitoring -Opening the release PR is not the end of stage 1; it starts a watch that runs until the checks -reach a terminal state. Reporting the PR URL and stopping leaves the user to discover failures -themselves, which is exactly backwards -- the agent is the one already holding the context needed -to interpret them. +Two things in this process are easy to hand off passively and should not be: the release PR after +it is opened, and the draft release after it is created. In both cases the agent has the context +needed to interpret what happens next, and the user should not have to come back and report an +outcome the agent could have observed. Monitoring is **automatic and read-only**. It never merges, never pushes, and never publishes. Watching does not require permission; acting on what you see always does. -## When to start a watch +## Monitoring the release PR + +Opening the release PR is not the end of stage 1; it starts a watch that runs until the checks +reach a terminal state. Reporting the PR URL and stopping leaves the user to discover failures +themselves, which is exactly backwards. + +### When to start a watch Start, or restart, monitoring: @@ -21,7 +27,7 @@ A restart is a fresh watch against the **new head SHA**. Runs from the previous do not report them as current, and do not let a green run from an earlier commit stand in for the one now at the head of the branch. -## Running the watch +### Running the watch 1. Resolve the current head SHA of the release branch. 2. List every check for it, not just the ones you expect: @@ -36,7 +42,7 @@ one now at the head of the branch. skipped job, or a queue backlog -- is not the same as a workflow that passed. Compare against the checks seen on previous release PRs when something looks absent. -## Reporting +### Reporting Report a compact per-check table plus a single overall verdict: @@ -52,7 +58,7 @@ Report a compact per-check table plus a single overall verdict: Use three states and name them explicitly: **green**, **running**, **blocked**. "Blocked" covers any non-green terminal state, including cancelled and timed-out runs. -## On failure +### On failure Diagnose before proposing anything. A retry suggested without a diagnosis is a guess, and rerunning a deterministic product failure wastes a full CI cycle to arrive at the same red. @@ -83,7 +89,7 @@ a deterministic product failure wastes a full CI cycle to arrive at the same red 5. After an approved fix is pushed, **restart the watch** for the new SHA without being asked. -## Stage 2 handoff +### Stage 2 handoff Stage 2 stays **blocked** until the checks are green, or until the user explicitly decides to proceed anyway. Record that decision and who made it. @@ -104,3 +110,56 @@ or Never say only "the PR is up, please review and merge." Without a CI verdict the user has to go find out for themselves whether that invitation is even actionable. + +## Monitoring the draft release + +Creating the draft release ends stage 3. Stage 4 is a human action in the GitHub UI, and the +temptation is to hand off and wait to be told it happened. Do not. Publishing is the moment the +release becomes irreversible and the moment two workflows start, so it is the least useful point in +the process to be uninformed about. + +Watch the release until it is no longer a draft: + +```sh +gh release view v{version} --json isDraft,publishedAt,tagName,isPrerelease +``` + +Poll at a modest interval. This gate is human-paced and may sit for hours or span a session, so +prefer periodic checks over a tight loop, and say that you are watching rather than going silent. + +**`isDraft: false` is the trigger.** The moment it flips: + +1. Record the stage 4 end time from `publishedAt`, not from when you noticed. The user published + when they published; polling latency is yours, not theirs, and it should not inflate the stage + duration in the closing summary. +2. Confirm the details that were the user's to choose and cannot be inferred: the tag actually + created, and whether the release was marked as a prerelease. A stable release mistakenly left + unflagged, or a prerelease flagged as stable, changes what consumers receive. +3. **Begin stage 5 immediately** via the verify-release skill. Publishing starts the Release and + Publish Docs workflows in parallel right away; waiting to be told to verify means arriving after + the interesting part. Announce the transition rather than asking permission -- stage 5 is + read-only, and the irreversible act has already occurred. + +### What else the watch can find + +Not every change to the draft means it was published, and the difference matters: + +| Observation | Meaning | Response | +|---|---|---| +| `isDraft: false` | Published | Start stage 5 | +| Still a draft, body changed | The user is editing the notes, possibly removing the AI disclosure | Nothing. Do not re-add anything they removed | +| Draft no longer exists | Deleted, or published under a different tag | Check for a published release before assuming it was abandoned; ask | +| Published with an unexpected tag | The tag differs from the prepared version | Stop and confirm before verifying. Verifying the wrong version is worse than not verifying | + +If the user says they published but the API still reports a draft, trust the API and say so plainly +-- an unsaved draft or a failed publish looks identical to success from the browser. + +### Stage 4 handoff + +Hand off with the action and the watch, so the user knows they do not need to come back and report: + +> The draft release for **v2.1.0** is ready. Review the notes line by line, set the prerelease flag +> if applicable, and click **Publish release**. Once you have signed off you may remove the AI +> disclosure from the notes. +> +> I am watching for publication and will start verification automatically when it happens. diff --git a/.github/skills/publish-release/SKILL.md b/.github/skills/publish-release/SKILL.md index 857ba5175..b949292f4 100644 --- a/.github/skills/publish-release/SKILL.md +++ b/.github/skills/publish-release/SKILL.md @@ -158,6 +158,29 @@ edit the draft body after they have removed it, do not reintroduce it. When the user requests revisions after the initial creation, always rewrite the complete body as a file — never perform in-place string replacements. See [references/formatting.md](references/formatting.md). +### Step 10: Watch for Publication + +Do not end the skill by asking the user to report back when they have published. Poll the release +until it is no longer a draft: + +```sh +gh release view v{version} --json isDraft,publishedAt,tagName,isPrerelease +``` + +Poll at a modest interval — this gate is human-paced and may span hours or a session boundary. Say +that you are watching rather than going silent. + +When `isDraft` becomes `false`: + +1. Record the publication time from `publishedAt`, not from when the poll noticed. +2. Confirm the tag that was actually created and whether the release was marked as a prerelease. + Both were the user's to set and cannot be inferred. +3. **Hand off to the verify-release skill immediately.** Publishing starts the Release and Publish + Docs workflows in parallel at that moment; verification that begins late misses them mid-flight. + +If the user reports publishing but the API still shows a draft, trust the API and say so — an +unsaved draft looks identical to a published release from the browser. + ## Edge Cases - **No new PRs since preparation**: proceed normally — the prepare-release notes are used as the foundation with no warnings @@ -172,6 +195,9 @@ When the user requests revisions after the initial creation, always rewrite the - **Versioning link carried over from the prepare-release draft**: the draft may contain an unslugged or wrong-MAJOR versioning link. Correct it to the `v{MAJOR}` slug of the version being released before the draft release is created. - **Versioning link for a brand-new MAJOR**: the `/v{MAJOR}/versioning.html` path is created by the Publish Docs workflow when the release is published. It is expected to 404 until then; use the slugged form regardless. - **Single breaking change**: use the same numbered format as multiple +- **Draft edited but not published**: the user is still reviewing, and may be removing the AI disclosure. Take no action and do not reintroduce anything they removed +- **Draft disappears without a published release**: it may have been deleted, or published under a different tag. Check for a published release before assuming it was abandoned, then ask +- **Published tag differs from the prepared version**: stop and confirm with the user before verifying. Verifying the wrong version is worse than not verifying ## Release Notes Template From 77556c10b3f235de1febee13300ec525fa6a6c8a Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 21:08:35 -0700 Subject: [PATCH 07/17] Measure release time instead of inferring it The v2.1.0 release exposed four ways the timing data went wrong. Every one of them produced a number that looked reasonable, which is worse than an obviously missing one. Interaction rows were left with NULL answered_at, or were recorded as zero-duration when the reply timestamp was not available. Close the open interaction before acting on the reply, never equate answered_at with prompted_at, and report the measured/unmeasured split so the total reads as the floor it is. Waiting was inferred by subtracting interaction time from stage wall-clock, which counts diagnosis and rework as waiting. Log unattended waits and workflow runs as their own records, and surface whatever the two do not account for as unaccounted rather than folding it into either. Stage ends were taken from when the agent noticed an event rather than when it happened, inflating a stage by the polling interval. Take them from mergedAt, publishedAt, and workflow timestamps. A single row per stage hid the shape of the time: stage 2 read as "2h 22m" with no way to tell CI from review from remediation. Stages now carry an attempt, and rework opens a new one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 7 +- .../references/session-tracking.md | 123 +++++++++++++++--- .../references/summary-template.md | 27 +++- 3 files changed, 133 insertions(+), 24 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index cf22b26c3..c6e909d14 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -212,7 +212,12 @@ Stage 5 Verify [verify-release skill, orch - **Timing.** Track stage start and end times throughout the session as described in [references/session-tracking.md](release-manager/references/session-tracking.md) so the closing summary is accurate. Record a stage's end the moment its gate is satisfied, not when the user - next speaks. + next speaks. Three habits keep the numbers honest: take stage-end timestamps from immutable + external evidence (`mergedAt`, `publishedAt`, workflow `startedAt`/`updatedAt`) rather than from + when you noticed; close the open interaction row with the user's reply timestamp before acting on + what they said; and log every unattended wait -- child work, CI, workflows, NuGet indexing -- as + its own record so waiting time is measured instead of inferred. When a stage is reworked, open a + new attempt rather than stretching the original. - **Release wrap-up.** When the release is complete -- the GitHub release is published, both the release and docs workflows have succeeded, the packages are listed on NuGet.org, and the docs site reflects the release -- present the closing summary defined in diff --git a/.github/agents/release-manager/references/session-tracking.md b/.github/agents/release-manager/references/session-tracking.md index fae72472b..6bf6ae2a6 100644 --- a/.github/agents/release-manager/references/session-tracking.md +++ b/.github/agents/release-manager/references/session-tracking.md @@ -17,33 +17,59 @@ CREATE TABLE IF NOT EXISTS release_session ( -- published_release_url, session_started_at CREATE TABLE IF NOT EXISTS release_stages ( - stage INTEGER PRIMARY KEY, -- 1..5 + stage INTEGER NOT NULL, -- 1..5 + attempt INTEGER NOT NULL DEFAULT 1, name TEXT NOT NULL, status TEXT NOT NULL, -- 'pending' | 'in_progress' | 'blocked' | 'done' | 'carried_over' started_at TEXT, -- ISO-8601 local time ended_at TEXT, - notes TEXT + notes TEXT, + PRIMARY KEY (stage, attempt) ); CREATE TABLE IF NOT EXISTS release_interactions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + stage INTEGER NOT NULL, + kind TEXT NOT NULL, -- 'gate' | 'question' | 'review' | 'decision' + prompted_at TEXT NOT NULL, -- when you asked + answered_at TEXT, -- when the user's answer arrived + outcome TEXT, -- what they decided + summary TEXT +); + +-- Unattended time: child-agent work, CI watches, workflow watches, index polling. +-- Without this, wait time can only be inferred from stage wall-clock, which +-- silently folds in discussion and rework. +CREATE TABLE IF NOT EXISTS release_waits ( id INTEGER PRIMARY KEY AUTOINCREMENT, stage INTEGER NOT NULL, - kind TEXT NOT NULL, -- 'gate' | 'question' | 'review' | 'decision' - prompted_at TEXT NOT NULL, -- when you asked - answered_at TEXT, -- when the user's answer arrived - summary TEXT + kind TEXT NOT NULL, -- 'child_work' | 'ci' | 'workflow' | 'index' | 'other' + reason TEXT NOT NULL, + started_at TEXT NOT NULL, + ended_at TEXT, + ref TEXT -- run id, PR number, package, etc. +); + +CREATE TABLE IF NOT EXISTS release_workflow_runs ( + run_id TEXT PRIMARY KEY, + stage INTEGER NOT NULL, + name TEXT NOT NULL, + head_sha TEXT, + started_at TEXT, + ended_at TEXT, + conclusion TEXT ); ``` Seed the five stages up front: ```sql -INSERT OR IGNORE INTO release_stages (stage, name, status) VALUES - (1, 'Prepare', 'pending'), - (2, 'Review and merge', 'pending'), - (3, 'Publish', 'pending'), - (4, 'Release', 'pending'), - (5, 'Verify', 'pending'); +INSERT OR IGNORE INTO release_stages (stage, attempt, name, status) VALUES + (1, 1, 'Prepare', 'pending'), + (2, 1, 'Review and merge', 'pending'), + (3, 1, 'Publish', 'pending'), + (4, 1, 'Release', 'pending'), + (5, 1, 'Verify', 'pending'); ``` ## Recording timestamps @@ -51,6 +77,20 @@ INSERT OR IGNORE INTO release_stages (stage, name, status) VALUES Every timestamp comes from the current date/time available to you in the session. Use ISO-8601 local time, for example `2026-08-04T13:22:05-07:00`. Never estimate a timestamp you could have recorded. +**Prefer immutable external evidence over your own observation.** You notice things late; the event +itself has a real timestamp. Query it and use it: + +| Event | Authoritative source | +|---|---| +| PR merged | `gh pr view {n} --json mergedAt` | +| Release published | `gh release view v{version} --json publishedAt` | +| Workflow run start/end | `gh run view {id} --json startedAt,updatedAt,conclusion` | +| Commit created | the commit's author date | + +Recording stage 4's end from the moment you noticed publication, rather than from `publishedAt`, +inflates that stage by your entire polling interval. The same applies to a merge you detect on a +later poll, and to workflow runs you attach to after they started. + - **Session start** -- write `session_started_at` into `release_session` at initialization. - **Stage start** -- set `status = 'in_progress'` and `started_at` the moment you begin the stage's first substantive action (invoking the skill, or beginning a status assessment for a human-gate @@ -64,15 +104,66 @@ time, for example `2026-08-04T13:22:05-07:00`. Never estimate a timestamp you co - **Carried over** -- when resuming a release and evidence shows a stage completed in an earlier session, record it as `status = 'carried_over'` with `started_at` and `ended_at` left NULL. Never invent durations for work you did not observe. +- **Rework** -- when a stage that reached its gate has to be revisited (CI went red after the PR was + opened, a corrective push, a re-run of a failed workflow), close the current attempt and insert a + new row with `attempt + 1` rather than reopening the old one or stretching its `ended_at`. One + aggregate row per stage hides the shape of the time: a stage 2 that reads as "2h 22m" tells you + nothing about how much was CI, how much was review, and how much was remediation. + +## Recording waits + +Insert a `release_waits` row whenever you begin waiting on something that is not the user, and close +it when the wait ends. Cover child-session work, CI watches, workflow watches, and index polling. + +This is what makes the closing summary's split honest. Without it, wait time can only be inferred by +subtracting interaction time from stage wall-clock, which quietly counts discussion, diagnosis, and +rework as waiting. With it, both halves are measured: + +``` +active interaction = sum of interaction intervals +waiting = sum of wait intervals +unaccounted = total - (active + waiting) +``` + +Report the unaccounted remainder rather than distributing it. A visible gap is information; a +silently absorbed one is a wrong number. + +Record every CI and release workflow run in `release_workflow_runs` as you watch it, using the run's +own `startedAt` and `updatedAt`. This makes the longest-wait figure in the summary a lookup instead +of a recollection. ## Recording interactions Insert a `release_interactions` row every time you put a gate, question, or review in front of the user: write `prompted_at` when you ask, and fill `answered_at` from the timestamp of their reply. +**Close the open interaction before doing anything else with the user's reply.** At most one row per +stage should have a NULL `answered_at` at any moment -- the question you are currently waiting on. +When a reply arrives, your first action is to `UPDATE` that row with `answered_at` and `outcome`; +only then act on what they said. Deferring the update is how rows end up permanently NULL, because +by the time the work is done the arrival time is gone. + +```sql +UPDATE release_interactions +SET answered_at = '{reply-timestamp}', outcome = '{what they decided}' +WHERE id = (SELECT MAX(id) FROM release_interactions WHERE answered_at IS NULL); +``` + +Two failure modes to avoid, both of which produce numbers that look fine and are wrong: + +- **Never write `answered_at` equal to `prompted_at`.** A zero-duration interaction means the reply + timestamp was unavailable, not that the user answered instantly. Leave it NULL and count the row + as unmeasured. +- **Record choice-style prompts too.** A gate answered by picking an option is still interaction; if + the mechanism gives you no reply timestamp, log the row with NULL `answered_at` so it appears in + the unmeasured count rather than vanishing. + +At wrap-up, report how many interactions were measured and how many were not. "~27m across six +gates, four more unmeasured" is an honest floor; "~27m" alone implies a precision that is not there. + The interval between `prompted_at` and `answered_at` is the user's **think-and-respond time**. Sum -those intervals to estimate **active user-interaction time**. Everything else in the session's -wall-clock span is wait time while you, a build, CI, or a workflow was working. +those intervals to estimate **active user-interaction time**. Do not treat the rest of the session as +waiting by subtraction -- take waiting from `release_waits` and report the remainder as unaccounted. Apply judgement when summing: @@ -81,11 +172,11 @@ Apply judgement when summing: Note in the summary that such a gap was excluded. - Long stretches where the user reviews a diff, release notes, or a PR **are** interaction time even though you were idle. -- Always label the result as an estimate. +- Always label the result as an estimate, and state the measured/unmeasured split alongside it. ## Progress rail -Render the rail from `release_stages` at every gating prompt: +Render the rail from the latest attempt of each stage in `release_stages` at every gating prompt: ``` [✓] 1 Prepare → [●] 2 Review and merge → [ ] 3 Publish → [ ] 4 Release → [ ] 5 Verify diff --git a/.github/agents/release-manager/references/summary-template.md b/.github/agents/release-manager/references/summary-template.md index 305f71437..2cbfa2570 100644 --- a/.github/agents/release-manager/references/summary-template.md +++ b/.github/agents/release-manager/references/summary-template.md @@ -7,8 +7,8 @@ the release. The tone is short and celebratory. It is a chat message to the user -- **do not commit it, do not post it to GitHub, and do not write it to a file** unless the user explicitly asks. -Build the timing sections from the `release_stages` and `release_interactions` tables described in -[session-tracking.md](session-tracking.md). +Build the timing sections from the `release_stages`, `release_interactions`, `release_waits`, and +`release_workflow_runs` tables described in [session-tracking.md](session-tracking.md). ## Template @@ -41,15 +41,20 @@ Build the timing sections from the `release_stages` and `release_interactions` t |---|---|---| | 1. Prepare | ✓ | {h m} | | 2. Review and merge | ✓ | {h m} | +| 2. Review and merge (attempt 2) | ✓ | {h m} | | 3. Publish | ✓ | {h m} | | 4. Release | ✓ | {h m} | | 5. Verify | ✓ | {h m} | | **Total session** | | **{h m}** | +{Include an attempt row only when a stage was reworked, and say what forced it -- "CI red, corrective +push". Aggregating rework into one row hides where the time actually went.} + **Where the time went** -* Active interaction — ~{h m} across {n} gates and questions ({percentage} of the session) +* Active interaction — ~{h m} across {n} measured gates{, plus {n} unmeasured} * Waiting on builds, CI, and the release and docs workflows — ~{h m} +* Unaccounted — {h m} * Longest single wait — {h m} ({what you were waiting on}) {Optional: one line on anything notable — a blocked stage and how long it cost, an excluded @@ -69,9 +74,17 @@ step-away gap, or a stage that ran unusually long or short.} 2. **Total session** is wall-clock from `session_started_at` to now, not the sum of stage elapsed times -- gaps between stages belong to the session but to no stage. 3. **Active interaction time is always an estimate.** Label it with `~` and say it is estimated from - prompt-to-answer intervals. Name any interval you excluded as a step-away gap. -4. **Round to readable units.** `2h 14m`, `47m`, `3m`. Never show seconds. -5. **Omit rows and sections that do not apply.** No blocked stage means no note about one; no + prompt-to-answer intervals. Report it as a floor with the unmeasured count beside it -- interactions + whose reply timestamp was unavailable are counted, not silently dropped. Name any interval you + excluded as a step-away gap. +4. **Waiting time is measured, not inferred.** Sum the `release_waits` intervals. Never derive it by + subtracting interaction time from the session total; that counts diagnosis and rework as waiting. + Show whatever the two do not account for as `Unaccounted` rather than folding it into either. +5. **Longest single wait comes from `release_waits` and `release_workflow_runs`**, not from the + longest interaction. If waits were not recorded, say the data is unavailable instead of + substituting the longest gate. +6. **Round to readable units.** `2h 14m`, `47m`, `3m`. Never show seconds. +7. **Omit rows and sections that do not apply.** No blocked stage means no note about one; no follow-ups means the section says `None.` rather than disappearing. -6. **Never speculate about time.** If the session lacks the data for a section, say so plainly +8. **Never speculate about time.** If the session lacks the data for a section, say so plainly instead of estimating. From eafb2826bd235d86e426669820629105bb05a63b Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 21:11:02 -0700 Subject: [PATCH 08/17] Show interaction time per stage in the wrap-up table The stage timing table reported elapsed time alone, so the one number a reader actually wants -- how much of the release cost them their attention -- was stranded in the narrative below it as a single session-wide figure. A stage that took 2h 22m of wall-clock but ~11m of the user's attention reads very differently from one that took 2h 22m of theirs. Splitting the two per stage makes that visible at a glance, and makes it obvious which stages are worth automating further. The roll-up depends on every interaction being attributed to a stage when it is recorded, so that requirement moves next to the schema rather than being implied by the summary. Stages keep their distinction between "~0m", meaning decisions that took no material time, and an em dash, meaning the timestamps were never captured; the total carries a "minimum" qualifier whenever either gap exists. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .../references/session-tracking.md | 16 +++++++ .../references/summary-template.md | 45 ++++++++++++------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/.github/agents/release-manager/references/session-tracking.md b/.github/agents/release-manager/references/session-tracking.md index 6bf6ae2a6..ee6d7735b 100644 --- a/.github/agents/release-manager/references/session-tracking.md +++ b/.github/agents/release-manager/references/session-tracking.md @@ -161,6 +161,22 @@ Two failure modes to avoid, both of which produce numbers that look fine and are At wrap-up, report how many interactions were measured and how many were not. "~27m across six gates, four more unmeasured" is an honest floor; "~27m" alone implies a precision that is not there. +**Always set `stage` on the interaction row.** The closing summary reports interaction time per stage +alongside each stage's elapsed time, which is what makes the table actionable -- a 2h 22m stage +costing ~11m of your attention reads very differently from one costing ~2h. That roll-up is only +possible if every interaction is attributed when it is recorded: + +```sql +SELECT stage, + SUM(strftime('%s', answered_at) - strftime('%s', prompted_at)) AS measured_seconds, + SUM(answered_at IS NULL) AS unmeasured +FROM release_interactions +GROUP BY stage; +``` + +A stage with interactions but no measurable intervals reports `—` rather than `~0m`; `~0m` means +recorded decisions that genuinely took no material time. + The interval between `prompted_at` and `answered_at` is the user's **think-and-respond time**. Sum those intervals to estimate **active user-interaction time**. Do not treat the rest of the session as waiting by subtraction -- take waiting from `release_waits` and report the remainder as unaccounted. diff --git a/.github/agents/release-manager/references/summary-template.md b/.github/agents/release-manager/references/summary-template.md index 2cbfa2570..635dbe072 100644 --- a/.github/agents/release-manager/references/summary-template.md +++ b/.github/agents/release-manager/references/summary-template.md @@ -37,15 +37,15 @@ Build the timing sections from the `release_stages`, `release_interactions`, `re **Stage timing (this session)** -| Stage | Status | Elapsed | -|---|---|---| -| 1. Prepare | ✓ | {h m} | -| 2. Review and merge | ✓ | {h m} | -| 2. Review and merge (attempt 2) | ✓ | {h m} | -| 3. Publish | ✓ | {h m} | -| 4. Release | ✓ | {h m} | -| 5. Verify | ✓ | {h m} | -| **Total session** | | **{h m}** | +| Stage | Status | Elapsed | Interactions | +|---|---|---|---| +| 1. Prepare | ✓ | {h m} | ~{h m} | +| 2. Review and merge | ✓ | {h m} | ~{h m} | +| 2. Review and merge (attempt 2) | ✓ | {h m} | ~{h m} | +| 3. Publish | ✓ | {h m} | ~{h m} | +| 4. Release | ✓ | {h m} | ~{h m} | +| 5. Verify | ✓ | {h m} | ~{h m} | +| **Total session** | | **{h m}** | **~{h m}{, minimum}** | {Include an attempt row only when a stage was reworked, and say what forced it -- "CI red, corrective push". Aggregating rework into one row hides where the time actually went.} @@ -73,18 +73,29 @@ step-away gap, or a stage that ran unusually long or short.} a footnote says the total covers this session only. 2. **Total session** is wall-clock from `session_started_at` to now, not the sum of stage elapsed times -- gaps between stages belong to the session but to no stage. -3. **Active interaction time is always an estimate.** Label it with `~` and say it is estimated from +3. **The Interactions column is per-stage active user time.** Sum that stage's + `release_interactions` prompt-to-answer intervals and prefix with `~`: `~5m`, `~1h 3m`. A stage + with recorded decisions but no material timed interaction shows `~0m`. A stage whose timestamps + are insufficient shows `—` -- never fabricate a value to fill the cell. Carried-over stages show + `—` in both time columns. +4. **The total interaction figure is a lower bound whenever any stage shows `—` or has unmeasured + interactions.** Say so in the cell -- `**~27m minimum**` -- and repeat the reason in the narrative. +5. **Active interaction time is always an estimate.** Label it with `~` and say it is estimated from prompt-to-answer intervals. Report it as a floor with the unmeasured count beside it -- interactions whose reply timestamp was unavailable are counted, not silently dropped. Name any interval you excluded as a step-away gap. -4. **Waiting time is measured, not inferred.** Sum the `release_waits` intervals. Never derive it by +6. **Reconcile the narrative with the table.** The `Active interaction` bullet must equal the table's + total interaction cell. Where the two could differ -- excluded step-away gaps, `—` stages, + zero-duration rows discarded as unmeasured -- name the discrepancy explicitly rather than letting + the reader find it. +7. **Waiting time is measured, not inferred.** Sum the `release_waits` intervals. Never derive it by subtracting interaction time from the session total; that counts diagnosis and rework as waiting. Show whatever the two do not account for as `Unaccounted` rather than folding it into either. -5. **Longest single wait comes from `release_waits` and `release_workflow_runs`**, not from the +8. **Longest single wait comes from `release_waits` and `release_workflow_runs`**, not from the longest interaction. If waits were not recorded, say the data is unavailable instead of substituting the longest gate. -6. **Round to readable units.** `2h 14m`, `47m`, `3m`. Never show seconds. -7. **Omit rows and sections that do not apply.** No blocked stage means no note about one; no - follow-ups means the section says `None.` rather than disappearing. -8. **Never speculate about time.** If the session lacks the data for a section, say so plainly - instead of estimating. +9. **Round to readable units.** `2h 14m`, `47m`, `3m`. Never show seconds. +10. **Omit rows and sections that do not apply.** No blocked stage means no note about one; no + follow-ups means the section says `None.` rather than disappearing. +11. **Never speculate about time.** If the session lacks the data for a section, say so plainly + instead of estimating. From 4e080740a262710bd0ffb61cc63fb76edabea974 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 21:12:22 -0700 Subject: [PATCH 09/17] Let the tilde carry the estimate on its own The interaction total hedged twice, rendering as "~27m minimum". The tilde already says the figure is estimated, so the extra word only cost the table its scannability -- and a timing table earns its place by being readable at a glance. Timestamp limitations still get reported, just in the prose below the table where there is room to say which stages were affected and why. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .../release-manager/references/summary-template.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/agents/release-manager/references/summary-template.md b/.github/agents/release-manager/references/summary-template.md index 635dbe072..6670f83c9 100644 --- a/.github/agents/release-manager/references/summary-template.md +++ b/.github/agents/release-manager/references/summary-template.md @@ -45,7 +45,7 @@ Build the timing sections from the `release_stages`, `release_interactions`, `re | 3. Publish | ✓ | {h m} | ~{h m} | | 4. Release | ✓ | {h m} | ~{h m} | | 5. Verify | ✓ | {h m} | ~{h m} | -| **Total session** | | **{h m}** | **~{h m}{, minimum}** | +| **Total session** | | **{h m}** | **~{h m}** | {Include an attempt row only when a stage was reworked, and say what forced it -- "CI red, corrective push". Aggregating rework into one row hides where the time actually went.} @@ -78,12 +78,14 @@ step-away gap, or a stage that ran unusually long or short.} with recorded decisions but no material timed interaction shows `~0m`. A stage whose timestamps are insufficient shows `—` -- never fabricate a value to fill the cell. Carried-over stages show `—` in both time columns. -4. **The total interaction figure is a lower bound whenever any stage shows `—` or has unmeasured - interactions.** Say so in the cell -- `**~27m minimum**` -- and repeat the reason in the narrative. +4. **`~` is the only qualifier the table needs.** Never append "minimum", "at least", or a similar + hedge to a cell -- the tilde already says the figure is estimated, and the table stays scannable. + When stages show `—` or interactions went unmeasured, explain that in the narrative prose below + rather than in the table. 5. **Active interaction time is always an estimate.** Label it with `~` and say it is estimated from - prompt-to-answer intervals. Report it as a floor with the unmeasured count beside it -- interactions - whose reply timestamp was unavailable are counted, not silently dropped. Name any interval you - excluded as a step-away gap. + prompt-to-answer intervals. In the narrative, report it as a floor with the unmeasured count + beside it -- interactions whose reply timestamp was unavailable are counted, not silently dropped. + Name any interval you excluded as a step-away gap. 6. **Reconcile the narrative with the table.** The `Active interaction` bullet must equal the table's total interaction cell. Where the two could differ -- excluded step-away gaps, `—` stages, zero-duration rows discarded as unmeasured -- name the discrepancy explicitly rather than letting From d262c02f772b76fa29a91f9c98a59ae0b9ea2187 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 21:14:56 -0700 Subject: [PATCH 10/17] Separate "took no time" from "we did not measure it" The two timing references disagreed about the same cell. summary-template said a stage with recorded decisions but no material timed interaction shows ~0m, while session-tracking said a stage with interactions but no measurable intervals shows an em dash. Those phrases read as the same condition and gave opposite answers, so the distinction they exist to protect -- zero time versus absent data -- was the first thing to collapse under it. Both now key off whether a usable reply timestamp exists: an em dash when interactions happened but none can be measured, ~0m only when the stage had no interactions or its measured intervals round to zero. That matches what the roll-up query actually returns, which is NULL rather than 0 for the unmeasured case. Also scoped the measured/unmeasured reporting to the narrative. It predated the rule that the table carries nothing but the tilde, and still argued that "~27m" alone overstates precision -- which now reads as an argument for hedging the cell that rule just cleaned up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .../release-manager/references/session-tracking.md | 10 ++++++---- .../release-manager/references/summary-template.md | 9 +++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.github/agents/release-manager/references/session-tracking.md b/.github/agents/release-manager/references/session-tracking.md index ee6d7735b..203b57487 100644 --- a/.github/agents/release-manager/references/session-tracking.md +++ b/.github/agents/release-manager/references/session-tracking.md @@ -158,8 +158,9 @@ Two failure modes to avoid, both of which produce numbers that look fine and are the mechanism gives you no reply timestamp, log the row with NULL `answered_at` so it appears in the unmeasured count rather than vanishing. -At wrap-up, report how many interactions were measured and how many were not. "~27m across six -gates, four more unmeasured" is an honest floor; "~27m" alone implies a precision that is not there. +At wrap-up, report how many interactions were measured and how many were not. That belongs in the +narrative prose, not in the timing table: "~27m across six gates, four more unmeasured" is an honest +floor, while the table's cells stay clean and carry only the `~` estimate marker. **Always set `stage` on the interaction row.** The closing summary reports interaction time per stage alongside each stage's elapsed time, which is what makes the table actionable -- a 2h 22m stage @@ -174,8 +175,9 @@ FROM release_interactions GROUP BY stage; ``` -A stage with interactions but no measurable intervals reports `—` rather than `~0m`; `~0m` means -recorded decisions that genuinely took no material time. +A stage whose interactions all lack a usable reply timestamp reports `—`, not `~0m`. `~0m` means the +stage had no interactions, or its measured intervals rounded to zero -- it asserts that the stage +cost the user no material time, which is a claim you can only make from data you actually have. The interval between `prompted_at` and `answered_at` is the user's **think-and-respond time**. Sum those intervals to estimate **active user-interaction time**. Do not treat the rest of the session as diff --git a/.github/agents/release-manager/references/summary-template.md b/.github/agents/release-manager/references/summary-template.md index 6670f83c9..1f33c80be 100644 --- a/.github/agents/release-manager/references/summary-template.md +++ b/.github/agents/release-manager/references/summary-template.md @@ -74,10 +74,11 @@ step-away gap, or a stage that ran unusually long or short.} 2. **Total session** is wall-clock from `session_started_at` to now, not the sum of stage elapsed times -- gaps between stages belong to the session but to no stage. 3. **The Interactions column is per-stage active user time.** Sum that stage's - `release_interactions` prompt-to-answer intervals and prefix with `~`: `~5m`, `~1h 3m`. A stage - with recorded decisions but no material timed interaction shows `~0m`. A stage whose timestamps - are insufficient shows `—` -- never fabricate a value to fill the cell. Carried-over stages show - `—` in both time columns. + `release_interactions` prompt-to-answer intervals and prefix with `~`: `~5m`, `~1h 3m`. Show + `~0m` when the stage had no interactions at all, or when its measured intervals round to zero. + Show `—` when the stage *did* have interactions but none of them carry a usable reply timestamp -- + that is missing data, not zero time, and must never be rendered as `~0m` or fabricated. + Carried-over stages show `—` in both time columns. 4. **`~` is the only qualifier the table needs.** Never append "minimum", "at least", or a similar hedge to a cell -- the tilde already says the figure is estimated, and the table stays scannable. When stages show `—` or interactions went unmeasured, explain that in the narrative prose below From a3d2c6969218ed03b8784f484c5b125a88eb2c98 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 21:50:29 -0700 Subject: [PATCH 11/17] Pin the draft release to the commit the user actually approved Step 9 displayed a merge commit SHA for review and then created the release with --target {merge-commit-branch}. A draft is not a snapshot: GitHub stores the target and creates the tag only at publish, which in the v2.1.0 release was hours later. Anything landing on that branch in between would have silently moved the tag to a commit nobody reviewed, while the release notes went on describing the commit that was. Targeting the reviewed SHA closes that window at no cost, since the tag still is not created until publish and the draft stays fully editable. Pulling in a later commit is now an explicit act -- repoint with gh release edit and regenerate the notes -- rather than something that happens to you. Two other corrections found alongside it. Stage 1's boundary was defined twice and disagreed: session-tracking ended it at PR creation, monitoring opened by saying that is explicitly not the end of it. Both stages then claimed the CI watch. Stage 1 now ends when the PR opens and stage 2 owns everything after, so preparation does not absorb review and remediation time. The baseline rule was stated correctly in the reference and then restated in the step that executes it as a literal from a long-past release, hard-coding the 2.0.0-preview series to 1.3.0. Refreshing those literals only restarts the rot, so the step now derives the value from what is actually published and shows its work. That also resolves a quieter conflict: the edge case insisted existing suppressions be preserved, which is exactly wrong when the baseline moved and the audit proves them stale. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 3 +++ .../release-manager/references/monitoring.md | 11 ++++++++--- .github/skills/prepare-release/SKILL.md | 6 +++--- .github/skills/publish-release/SKILL.md | 18 +++++++++++++++++- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index c6e909d14..429e5e2aa 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -196,6 +196,9 @@ Stage 5 Verify [verify-release skill, orch - **Irreversibility.** Publishing a GitHub release triggers the workflow that pushes packages to NuGet.org, and NuGet.org versions cannot be unpublished. Pushing tags and branches cannot be cleanly undone either. Prepare and review first, then act only on explicit user confirmation. + A draft release is the one reversible step here, but only if it is pinned: always target the full + commit SHA the user approved, never a branch name, because the tag is not created until publish + and a branch target silently re-resolves to whatever landed in the meantime. - **Never publish a release yourself.** The **publish-release** skill creates draft releases only. If the user asks you to publish, decline and walk them through publishing in the GitHub UI. Likewise, never run `dotnet nuget push` and never handle NuGet API keys. diff --git a/.github/agents/release-manager/references/monitoring.md b/.github/agents/release-manager/references/monitoring.md index d2e671f52..5f07e1a5b 100644 --- a/.github/agents/release-manager/references/monitoring.md +++ b/.github/agents/release-manager/references/monitoring.md @@ -10,9 +10,14 @@ Watching does not require permission; acting on what you see always does. ## Monitoring the release PR -Opening the release PR is not the end of stage 1; it starts a watch that runs until the checks -reach a terminal state. Reporting the PR URL and stopping leaves the user to discover failures -themselves, which is exactly backwards. +Opening the release PR ends stage 1 and immediately begins stage 2, which owns the watch: it runs +until every check reaches a terminal state. Reporting the PR URL and stopping leaves the user to +discover failures themselves, which is exactly backwards. + +Record the time accordingly. Stage 1 ends when the PR is created, and the CI watch that follows -- +including any red checks, corrective pushes, and re-runs -- belongs to stage 2. Attributing that +time to stage 1 makes preparation look expensive and review look cheap, which is the opposite of +what the summary should reveal. ### When to start a watch diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 1ab389b63..13bd968d7 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -131,7 +131,7 @@ After the version is confirmed: 2. Update `src/Directory.Build.props`: - Set `` to the confirmed stable component - Set `` for prerelease versions, or clear it for stable versions; add the element if it is missing - - Update `` when appropriate. For the `2.0.0-preview` series, baseline is `1.3.0` (latest shipped 1.x). For subsequent stable releases, baseline is the previous shipped version of the same MAJOR or the latest stable from the previous MAJOR. + - Update `` when appropriate, per the rule in [references/apicompat-apidiff.md](references/apicompat-apidiff.md#updating-the-baseline-version). Read the current value from `src/Directory.Build.props` and derive the correct one from the versions actually published; never copy a version from an example. Show the derivation — current value, published versions considered, resulting value, and whether it changes — and get confirmation before editing. **If the value changes, the [baseline-transition suppression audit](references/apicompat-apidiff.md#baseline-transition-suppression-audit) is mandatory.** 3. Build the solution to verify the version change compiles: `dotnet build` This step creates local changes only — nothing is committed or pushed yet. @@ -338,8 +338,8 @@ Only after explicit user confirmation in Step 12: - **API diff tool installation fails**: do not fall back to a manual summary; pause and present the installation error to the user, offering options to troubleshoot, skip the API diff section, or abort the release preparation - **No changelogs in repo**: skip changelog updates; note in the summary - **Branch already exists**: if `release-{version}` already exists locally or remotely, ask the user whether to reuse it, delete and recreate, or choose a different name -- **PackageValidationBaselineVersion update**: for the `2.0.0-preview` series, use `1.3.0`; for subsequent stable releases, use the previous shipped version of the same MAJOR or the latest stable from the previous MAJOR -- **CompatibilitySuppressions.xml**: when intentional breaks are found, add suppression entries and include the file in the commit; existing suppressions should be preserved +- **PackageValidationBaselineVersion update**: derive it per [references/apicompat-apidiff.md](references/apicompat-apidiff.md#updating-the-baseline-version) from the versions actually published, and show the derivation for confirmation. A change to this property makes the baseline-transition suppression audit mandatory +- **CompatibilitySuppressions.xml**: when intentional breaks are found, add suppression entries and include the file in the commit. Preserve existing suppressions **unless the baseline moved** — the audit may prove tracked entries stale, in which case removing them is the fix, not a regression - **Versioning link for a brand-new MAJOR**: the `/v{MAJOR}/versioning.html` path does not exist until the release is published and the Publish Docs workflow runs. The link is forward-referencing at prepare time, like the release-notes tag link. Use the slugged form anyway; do not fall back to the unslugged URL. - **User declines PR creation**: if the user declines at Step 12, leave the local branch intact so they can review, modify, or push manually diff --git a/.github/skills/publish-release/SKILL.md b/.github/skills/publish-release/SKILL.md index b949292f4..5b5a87ae1 100644 --- a/.github/skills/publish-release/SKILL.md +++ b/.github/skills/publish-release/SKILL.md @@ -138,9 +138,25 @@ Display release metadata for user review: - **Target**: merge commit SHA, its message, the merge commit's branch (the prepare-release PR base), and the prepare-release PR link After confirmation: -- Create with `gh release create --draft {tag} --target {merge-commit-branch}` (always `--draft`), using the prerelease tag verbatim when present +- Create with `gh release create --draft {tag} --target {merge-commit-sha}` (always `--draft`), using the prerelease tag verbatim when present +- **Target the full commit SHA, never a branch name.** A draft sits unpublished until a human + reviews and publishes it, which can be hours. `--target` is resolved when the tag is created -- + at publish time, not now -- so a branch name silently re-resolves to whatever landed on that + branch in the meantime. The tag would then be cut at a commit nobody reviewed, and the release + notes would describe a different commit than the one shipped. The SHA you displayed above is the + commit the user approved; pass that exact SHA. - **Never publish.** If the user asks to publish, decline and instruct them to publish manually. +Pinning the SHA costs nothing, because a draft release does not create the git tag. GitHub stores +the target and creates the tag only when the release is published, so the tag remains uncreated and +the draft fully editable while it waits. + +That is also what makes a late-arriving commit easy to absorb. If the user decides to include work +that merged after the draft was created, do not delete and recreate the release: repoint it with +`gh release edit {tag} --target {new-commit-sha}`, then regenerate the release notes for the new +range and present them for approval again. Never move the target without revising the notes to +match -- a target change silently alters what shipped. + Then hand off to the user with the publishing checklist: > The draft release is ready at {release URL}. Before publishing: From d5fea9516c34986241280c00dfe645871c2ad631 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 21:56:47 -0700 Subject: [PATCH 12/17] Audit an abandoned preparation instead of trusting or discarding it A worktree left behind by an interrupted stage 1 says only that work started. The agent treated its existence as progress, so it could resume onto a branch whose validation had never passed and whose release notes nobody had reviewed. Give it an evidence ladder: read the commit, re-run everything that leaves no trace in the repository, and put recovered decisions back through the user before building on them. Record the child's session, worktree, and branch when it is dispatched, so a later session can find the work rather than infer it. The suppression guidance had the same shape of error. It described a compatibility suppression file as append-only, which is what let 312 obsolete Core entries read as a mass breaking change. Name all three outcomes - added, retained, cleared - and say that preservation holds only while the baseline does. Stop offering a tracked file as a template for entry shape; it is legitimately empty after an audit. Replace the hard-coded three-package enumerations and baseline literals with instructions to enumerate what actually ships. The set grows, and the next package to join validation would have gone unreported. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 8 +++- .../release-manager/references/delegation.md | 45 +++++++++++++++++++ .../references/session-tracking.md | 3 +- .../references/apicompat-apidiff.md | 30 ++++++++----- 4 files changed, 72 insertions(+), 14 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index 429e5e2aa..cfb8cd061 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -235,7 +235,7 @@ evidence rather than memory: |---|---| | `` / `` in `src/Directory.Build.props` on the base branch | Whether the version bump has landed | | A local or remote `release-{version}` branch | Stage 1 is in progress or complete | -| A worktree for `release-{version}` | A child session prepared, or is preparing, this release | +| A worktree for `release-{version}` | A preparation was started here. Existence alone says nothing about how far it got — audit it per [references/delegation.md](release-manager/references/delegation.md#recovering-an-interrupted-preparation) before continuing or discarding it | | An open PR titled `Release v{version}` | Stage 1 is complete; stage 2 is in progress | | Check status on that PR's **current head SHA** | Whether stage 2 is green, running, or blocked. Re-check on resume; a verdict from an earlier session may predate later pushes | | That PR merged | Stage 2 is complete; stage 3 can begin | @@ -244,7 +244,11 @@ evidence rather than memory: | Successful release and docs workflow runs, a listed NuGet version, and a live docs version | Stage 5 is complete | State plainly which stage you inferred and what evidence you used, and ask the user to confirm -before acting. When resuming, restore session tracking per +before acting. Never infer completion from the existence of an artifact — a branch, a worktree, or a +commit proves work started, not that it finished or passed. Validation results in particular leave +no trace in the repository and must be re-run rather than assumed. + +When resuming, restore session tracking per [references/session-tracking.md](release-manager/references/session-tracking.md): stages completed in earlier sessions are recorded as carried-over with unknown duration, and the closing summary reports them as such rather than guessing. diff --git a/.github/agents/release-manager/references/delegation.md b/.github/agents/release-manager/references/delegation.md index c8a4e68e9..e06628364 100644 --- a/.github/agents/release-manager/references/delegation.md +++ b/.github/agents/release-manager/references/delegation.md @@ -84,6 +84,51 @@ If app-native child sessions are not available in the current environment, fall worktree created from the source/base branch and run the skill there, keeping the orchestrator's own checkout untouched. The invariant is the worktree, not the mechanism. +## Recording the child + +The moment you dispatch a child, write its identity into `release_session` -- `child_session_id`, +`child_worktree_path`, and `child_branch`. A release routinely outlives the session that started +it, and a worktree with no recorded owner is very hard to tell apart from the dozens of unrelated +worktrees a busy repository accumulates. + +## Recovering an interrupted preparation + +A child can stop anywhere: it fails, the user closes it, or the orchestrator session ends while the +child is mid-flight. Recovery starts from what the worktree actually contains, never from the fact +that it exists. + +**Existence is not progress.** A `release-{version}` worktree proves only that a preparation was +started. Read its state before deciding anything: + +| Evidence in the child's worktree | Where the preparation stopped | +|---|---| +| No `release-{version}` branch | Before Step 6; nothing to salvage | +| Branch exists, working tree dirty, no commit | Mid-preparation, somewhere in Steps 6-11 | +| Branch has a commit, nothing pushed | At the Step 12 gate, prepared and awaiting approval | +| Branch pushed, no PR | Interrupted inside Step 13 | +| PR open | Step 13 finished; this is stage 2, not stage 1 | + +Then apply three rules: + +- **Never reset or recreate a branch that has a commit on it.** It may hold work the user already + reviewed and corrected -- release-note categorization, acknowledgement edits, a chosen preamble -- + none of which is reproducible from the repository. Read the commit and the drafted notes and + continue from there. +- **Never inherit a validation result.** Build, pack, and ApiCompat outcomes leave no trace in git. + A commit proves the files were written, not that anything passed. Re-run the checks rather than + assuming the interrupted run got that far. +- **Prefer resuming the recorded child over launching a replacement.** It still holds the context. + If it is gone, dispatch a replacement pointed at the *existing* worktree and branch, and tell it + to audit what is already there before continuing -- not to start over. + +Report the stopping point and the evidence you read, and let the user confirm before continuing. + +Decisions the user made at a gate are the hardest thing to recover, because session tracking does +not survive the session. Their durable form is the artifact itself: the drafted release notes carry +the categorization, and the acknowledgements roster carries the exclusions. On resume, re-derive the +decisions by reading the drafted notes, and present them as *previously decided* for confirmation. +Silently re-deriving them from scratch will quietly undo corrections the user already made once. + ## Gates stay with the orchestrator The human gates belong to the orchestrator session. The child prepares and reports; the user diff --git a/.github/agents/release-manager/references/session-tracking.md b/.github/agents/release-manager/references/session-tracking.md index 203b57487..e5364dce3 100644 --- a/.github/agents/release-manager/references/session-tracking.md +++ b/.github/agents/release-manager/references/session-tracking.md @@ -14,7 +14,8 @@ CREATE TABLE IF NOT EXISTS release_session ( value TEXT ); -- Expected keys: version, base_branch, release_branch, pr_number, draft_release_url, --- published_release_url, session_started_at +-- published_release_url, session_started_at, +-- child_session_id, child_worktree_path, child_branch CREATE TABLE IF NOT EXISTS release_stages ( stage INTEGER NOT NULL, -- 1..5 diff --git a/.github/skills/prepare-release/references/apicompat-apidiff.md b/.github/skills/prepare-release/references/apicompat-apidiff.md index 2f9c6bf84..eae7be96e 100644 --- a/.github/skills/prepare-release/references/apicompat-apidiff.md +++ b/.github/skills/prepare-release/references/apicompat-apidiff.md @@ -8,16 +8,16 @@ The SDK uses NuGet's [Package Validation](https://learn.microsoft.com/dotnet/fun ```xml true -1.4.1 +{baseline} ``` +Read the current values rather than assuming them, and check whether any individual project overrides them — a project that opts out of validation still ships, and needs to be reported as unvalidated rather than quietly skipped. + ### Running ApiCompat -1. **Pack the SDK packages** to trigger validation: +1. **Pack the SDK packages** to trigger validation. Enumerate the packable projects under `src/` and pack each one; the set of shipping packages grows over time, so do not work from a remembered list: ```sh - dotnet pack src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj - dotnet pack src/ModelContextProtocol/ModelContextProtocol.csproj - dotnet pack src/ModelContextProtocol.AspNetCore/ModelContextProtocol.AspNetCore.csproj + dotnet pack src/{project}/{project}.csproj ``` Or pack all at once: ```sh @@ -111,7 +111,11 @@ to make validation pass. ### Compatibility Suppressions -When intentional breaking changes are confirmed, create or update `CompatibilitySuppressions.xml` in the affected project directory. The repo already uses this pattern — see `src/ModelContextProtocol.Core/CompatibilitySuppressions.xml` for examples. +When intentional breaking changes are confirmed, create or update `CompatibilitySuppressions.xml` in the affected project directory — the conventional location, which is auto-discovered. + +**A suppression file has three valid outcomes, not one.** Entries get *added* when a new intentional break needs suppressing, *retained* when they still describe a real break against the current baseline, and *cleared* when the baseline moved and they no longer describe anything. Treating the file as append-only is what turned 312 obsolete entries into a release-blocking failure that read as a mass breaking change. Preservation is the default only while the baseline holds still; once it moves, the [audit](#baseline-transition-suppression-audit) decides what stays, and removing entries it proves stale is the fix rather than a regression. + +Do not use a tracked file as a template for what entries should look like — it may legitimately be empty, and its contents describe whatever baseline it was generated against, not yours. Generate entries instead. ```xml @@ -287,15 +291,19 @@ _or_ ### In the User Summary (Step 12) -Present a condensed version for the user review. **Report per shipping package, and do not state +Present a condensed version for the user review. **Report every shipping package, and do not state that ApiCompat passed without these four facts** — "passed" is not meaningful without knowing what it was validated against and whether stale suppressions were masking or manufacturing the result: | Package | Baseline | Generated entries | Retained / removed | Plain pack | |---|---|---|---|---| -| ModelContextProtocol.Core | 1.4.1 | 0 | 0 retained / 312 removed | ✅ | -| ModelContextProtocol | 1.4.1 | 0 | 0 / 0 | ✅ | -| ModelContextProtocol.AspNetCore | 1.4.1 | 0 | 0 / 0 | ✅ | +| {package} | {baseline} | 0 | 0 retained / 312 removed | ✅ | +| {package} | {baseline} | 0 | 0 / 0 | ✅ | + +Enumerate the packable projects under `src/` rather than working from a remembered list; the set +grows. A package that does not participate in validation still gets a row, reporting why — a first +release has no baseline to compare against, and that is a fact worth stating rather than an absence +worth hiding. - **Baseline** — the `PackageValidationBaselineVersion` actually used, and whether it changed during this release @@ -308,7 +316,7 @@ it was validated against and whether stale suppressions were masking or manufact Then the summary lines: ``` -API Compatibility: ✅ All 3 packages pass against v1.4.1 (312 stale Core suppressions removed) +API Compatibility: ✅ All {n} packages pass against v{baseline} ({n} stale suppressions removed from {package}) API Diff: +12 additions, -2 removals, ~3 changes across all packages ``` From 891ddd09e2a97daf547f13251271758da54503af Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 22:00:05 -0700 Subject: [PATCH 13/17] Say that a trailing validation baseline is correct, not stale PackageValidationBaselineVersion deliberately stays at the last MAJOR release while VersionPrefix advances through the series, so 2.0.0 sitting beside a published 2.1.0 is the rule working. The reference stated the rule but never said what it looks like in practice, and a second independent reader has now concluded the value was stale and worth bumping. Name the trap and the cost of "fixing" it: bumping mid-series re-baselines the released API against itself, discarding the guarantee, and trips the baseline-transition audit that produced the 312-entry incident. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/skills/prepare-release/references/apicompat-apidiff.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/skills/prepare-release/references/apicompat-apidiff.md b/.github/skills/prepare-release/references/apicompat-apidiff.md index eae7be96e..78d1d13b3 100644 --- a/.github/skills/prepare-release/references/apicompat-apidiff.md +++ b/.github/skills/prepare-release/references/apicompat-apidiff.md @@ -60,6 +60,8 @@ baseline have drifted apart, which is the one thing you need to know. - **MAJOR version bump**: Update `` to the previous release version so that ApiCompat validates against the last stable release of the prior MAJOR version. After the new MAJOR release is published, the baseline stays at the new version for future comparisons. - **MINOR or PATCH version bump**: Keep `` at the last MAJOR release version (e.g., keep `1.0.0` when releasing `1.1.0` or `1.0.1`). +**A baseline that trails `VersionPrefix` is the expected steady state, not a stale value.** Through a MAJOR series the baseline deliberately stays put while `VersionPrefix` advances, so seeing `2.0.0` alongside a published `2.1.0` means the rule is being followed. Do not "fix" the gap — bumping the baseline mid-series triggers the audit below and invites the released API surface to be re-baselined against itself, silently discarding the compatibility guarantee the property exists to enforce. + **Any change to this property triggers the [baseline-transition suppression audit](#baseline-transition-suppression-audit).** Do not change it and interpret the resulting failures as breaking changes — the failures are expected until the suppressions are reconciled. ### Baseline-transition suppression audit From 2fb63bf1f086bf59114420f9d0dce5ec7093ed7b Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 22:11:52 -0700 Subject: [PATCH 14/17] Close four ways the release process could report a confident wrong answer Previous-release lookup contradicted itself inside a single sentence, saying "most recent published globally" and "highest semver" as though they were the same rule. They diverge the moment a servicing patch ships after a newer minor: v2.0.1 published after v2.1.0 is the most recent by date but is not on main at all, so date ordering picks a tag off the branch, producing a bogus PR range and an ApiCompat run that reports the entire API surface as removed. Settle on highest semver among ancestors of the target, and align the four call sites. Workflow-run correlation matched on event type plus a timestamp at or after publishedAt, which any concurrently published release also satisfies - a green run for the wrong tag. Release-event runs carry the tag in headBranch, so match on that exactly and cross-check headSha against the reviewed target. Stage 3 corrective commits were told to land on the release branch, which is merged and gone by then. They belong on the protected base branch behind their own PR. More importantly, the draft is now pinned to an approved SHA, so a fix merged afterward is not in the tag unless the draft is re-targeted - otherwise the notes describe a fixed state the tag does not contain. Wait and interaction intervals overlapped by construction, since gates get answered mid-CI-watch. Summing both against wall-clock made the parts exceed the whole: a 2h watch containing an 11m gate yielded -11m unaccounted. Keep the two disjoint while recording by closing the wait around each interaction, and refuse to publish a negative or clamped remainder. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .../release-manager/references/delegation.md | 16 +++++++++--- .../references/session-tracking.md | 26 ++++++++++++++++--- .../references/summary-template.md | 4 +++ .github/skills/bump-version/SKILL.md | 2 +- .../references/semver-assessment.md | 2 +- .github/skills/prepare-release/SKILL.md | 2 +- .github/skills/publish-release/SKILL.md | 21 +++++++++++++-- .../shared-resources/release-branches.md | 21 +++++++++++++-- .github/skills/verify-release/SKILL.md | 21 ++++++++++++--- 9 files changed, 97 insertions(+), 18 deletions(-) diff --git a/.github/agents/release-manager/references/delegation.md b/.github/agents/release-manager/references/delegation.md index e06628364..981896580 100644 --- a/.github/agents/release-manager/references/delegation.md +++ b/.github/agents/release-manager/references/delegation.md @@ -32,9 +32,19 @@ worktree. | 4. Release | No -- human action in the GitHub UI | Orchestrator, in place | | 5. Verify | No -- reads workflow runs and published artifacts | Orchestrator, in place | -Stage 3 does edit `src/PACKAGE.md` and `README.md` when the README checklist finds issues. Those -fixes land on the release branch, so delegate them the same way as stage 1: a child session on a -worktree based on the branch the draft release targets. +Stage 3 does edit `src/PACKAGE.md` and `README.md` when the README checklist finds issues. **The +release branch is already merged by this point, so those fixes cannot land on it.** They go to the +base branch the release ships from — `main` or `release/{MAJOR}.x` — which is protected, so they +need their own small PR, reviewed and merged like any other change. + +Delegate that PR the same way as stage 1: a child session on a fresh worktree based on the base +branch. Do not push directly to the base branch, and do not commit into the orchestrator's worktree. + +A corrective commit merged at this point **is not in the draft release's tag**, because the draft is +pinned to the merge commit the user approved. After the fix merges, re-target the draft to the new +head and regenerate the notes per +[publish-release Step 9](../../../skills/publish-release/SKILL.md). Skipping the re-target ships a +tag that predates the fix while the notes describe the fixed state. ## Confirm the orchestrator's location diff --git a/.github/agents/release-manager/references/session-tracking.md b/.github/agents/release-manager/references/session-tracking.md index e5364dce3..07f61ffc2 100644 --- a/.github/agents/release-manager/references/session-tracking.md +++ b/.github/agents/release-manager/references/session-tracking.md @@ -118,20 +118,38 @@ it when the wait ends. Cover child-session work, CI watches, workflow watches, a This is what makes the closing summary's split honest. Without it, wait time can only be inferred by subtracting interaction time from stage wall-clock, which quietly counts discussion, diagnosis, and -rework as waiting. With it, both halves are measured: +rework as waiting. + +**A wait records unattended time only, so it must not overlap an interaction.** These two clocks run +in the same wall-clock window — a two-hour CI watch during which the user answers an eleven-minute +gate is not two hours of waiting plus eleven minutes of interaction. Counting both in full makes the +parts exceed the whole and drives the remainder negative: + +``` +total 120m − active 11m − waiting 120m = −11m +``` + +Keep them disjoint as you record, rather than reconciling later. When a wait is running and you turn +to the user, **close the wait, handle the interaction, then open a new wait row** for the remainder. +The same watch then yields two wait rows around the gate instead of one row swallowing it: ``` active interaction = sum of interaction intervals -waiting = sum of wait intervals -unaccounted = total - (active + waiting) +waiting = sum of wait intervals, none overlapping an interaction +unaccounted = total − (active + waiting) ``` +Before reporting, check that `unaccounted` is not negative. If it is, the rows overlap and the split +is wrong — say so and report the measured totals plainly instead of publishing a negative remainder +or clamping it to zero. A clamped number looks correct and hides the defect. + Report the unaccounted remainder rather than distributing it. A visible gap is information; a silently absorbed one is a wrong number. Record every CI and release workflow run in `release_workflow_runs` as you watch it, using the run's own `startedAt` and `updatedAt`. This makes the longest-wait figure in the summary a lookup instead -of a recollection. +of a recollection. These rows are evidence about the run itself, so they are exempt from the +non-overlap rule — never sum them into `waiting`. ## Recording interactions diff --git a/.github/agents/release-manager/references/summary-template.md b/.github/agents/release-manager/references/summary-template.md index 1f33c80be..966bbe15e 100644 --- a/.github/agents/release-manager/references/summary-template.md +++ b/.github/agents/release-manager/references/summary-template.md @@ -94,6 +94,10 @@ step-away gap, or a stage that ran unusually long or short.} 7. **Waiting time is measured, not inferred.** Sum the `release_waits` intervals. Never derive it by subtracting interaction time from the session total; that counts diagnosis and rework as waiting. Show whatever the two do not account for as `Unaccounted` rather than folding it into either. + **If `Unaccounted` computes negative, wait and interaction rows overlapped** — the split is + unsound, so omit the `Unaccounted` line, report the two measured totals, and state plainly that + they overlap. Never publish a negative figure and never clamp it to zero, which would present a + broken split as a clean one. 8. **Longest single wait comes from `release_waits` and `release_workflow_runs`**, not from the longest interaction. If waits were not recorded, say the data is unavailable instead of substituting the longest gate. diff --git a/.github/skills/bump-version/SKILL.md b/.github/skills/bump-version/SKILL.md index e78915636..1127aa284 100644 --- a/.github/skills/bump-version/SKILL.md +++ b/.github/skills/bump-version/SKILL.md @@ -22,7 +22,7 @@ Read `src/Directory.Build.props` on the current branch and extract: The candidate version is `{VersionPrefix}` plus `-{VersionSuffix}` when the suffix is present (for example, `2.0.0-preview.1`). Display the current candidate version to the user. -Determine the previous release tag from `gh release list` (most recent **published** release). Draft releases must be ignored — they represent a pending release that has not yet shipped. Use `--exclude-drafts` or filter to only published releases when querying. The lookup is branch-aware: from a `release/{MAJOR}.x` branch, restrict candidates to tags matching `v{MAJOR}.*`; from `main`, use the most recent published release globally. See [release-branches.md](../shared-resources/release-branches.md) for details. +Determine the previous release tag from `gh release list` — the **highest semver** among published releases that are ancestors of the target commit, not the most recently published by date. Draft releases must be ignored — they represent a pending release that has not yet shipped. Use `--exclude-drafts` or filter to only published releases when querying. The lookup is branch-aware: from a `release/{MAJOR}.x` branch, restrict candidates to tags matching `v{MAJOR}.*`; from `main`, there is no MAJOR filter. See [release-branches.md](../shared-resources/release-branches.md#previous-release-tag-lookup) for details, including why date ordering picks the wrong tag. ### Step 2: Assess and Determine Next Version diff --git a/.github/skills/bump-version/references/semver-assessment.md b/.github/skills/bump-version/references/semver-assessment.md index f1f5f6d68..e9f4508af 100644 --- a/.github/skills/bump-version/references/semver-assessment.md +++ b/.github/skills/bump-version/references/semver-assessment.md @@ -66,7 +66,7 @@ This is purely about how to *compute* the next version. It does **not** declare ### Branch context -The "previous release" lookup is constrained to tags matching `v{MAJOR}.*` when assessing from a `release/{MAJOR}.x` servicing branch. On `main`, the lookup is unconstrained (most recent published release globally). +The "previous release" lookup selects the highest semver among published releases that are ancestors of the target commit, constrained to tags matching `v{MAJOR}.*` when assessing from a `release/{MAJOR}.x` servicing branch. On `main`, there is no MAJOR filter. It is not a date-ordered lookup; see [release-branches.md](../../shared-resources/release-branches.md#previous-release-tag-lookup). The MAJOR/MINOR/PATCH classification criteria above are unchanged regardless of branch. diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 13bd968d7..478cbb12c 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -61,7 +61,7 @@ The user may provide: - **No context** — show the last 5 commits on the selected source/base branch (noting HEAD) and offer the option to enter a branch or tag name instead Once the target is established: -1. Determine the previous release tag from `gh release list` (most recent **published** release — exclude drafts with `--exclude-drafts`). Use the selected source/base branch context: on `release/{MAJOR}.x`, restrict candidates to tags matching `v{MAJOR}.*`; on `main`, use the most recent published release globally. +1. Determine the previous release tag from `gh release list` — the **highest semver** among published releases that are ancestors of the target commit (exclude drafts with `--exclude-drafts`). Do not order by publication date; see [release-branches.md](../shared-resources/release-branches.md#previous-release-tag-lookup) for why the two differ and what breaks. On `release/{MAJOR}.x`, restrict candidates to tags matching `v{MAJOR}.*`; on `main`, there is no MAJOR filter. 2. Get the full list of PRs merged between the previous release tag and the target commit on the selected branch. 3. Read `src/Directory.Build.props` **at the target commit**. Extract `` and ``; the **candidate version** is `{VersionPrefix}` plus `-{VersionSuffix}` when the suffix is present (for example, `2.0.0-preview.1`). 4. **Verify the previous release tag is an ancestor of the target commit:** diff --git a/.github/skills/publish-release/SKILL.md b/.github/skills/publish-release/SKILL.md index 5b5a87ae1..be4d19b74 100644 --- a/.github/skills/publish-release/SKILL.md +++ b/.github/skills/publish-release/SKILL.md @@ -44,7 +44,7 @@ Verify the PR is merged. Extract: ### Step 2: Determine Version and Commit Range 1. Read `src/Directory.Build.props` at the merge commit to confirm `` and ``. The tag is `v{VersionPrefix}` plus `-{VersionSuffix}` when the suffix is present; for example, `2.0.0` + `preview.1` → `v2.0.0-preview.1`. -2. Determine the previous release tag from `gh release list` (most recent **published** release — exclude drafts with `--exclude-drafts`). The lookup is branch-aware: when the merge commit is on a `release/{MAJOR}.x` branch, restrict candidates to tags matching `v{MAJOR}.*`; on `main`, use the most recent published release globally. See [release-branches.md](../shared-resources/release-branches.md). +2. Determine the previous release tag from `gh release list` — the **highest semver** among published releases that are ancestors of the merge commit (exclude drafts with `--exclude-drafts`). Do not order by publication date. When the merge commit is on a `release/{MAJOR}.x` branch, restrict candidates to tags matching `v{MAJOR}.*`; on `main`, there is no MAJOR filter. See [release-branches.md](../shared-resources/release-branches.md#previous-release-tag-lookup). 3. Identify the full commit range: previous release tag → merge commit. ### Step 3: Check for Additional PRs @@ -96,7 +96,24 @@ Re-run the README content checklist from [../prepare-release/references/readme-c 2. **Snippet validation** -- Extract `csharp`-fenced code blocks from `src/PACKAGE.md` and `README.md`, build the temporary test project, and report results. Follow [../prepare-release/references/readme-snippets.md](../prepare-release/references/readme-snippets.md) for the full procedure. 3. **Delete** the temporary project after validation. -If issues are found, present them to the user with proposed fixes. Any fixes must be applied as a separate commit before the draft release is created. +If issues are found, present them to the user with proposed fixes. + +**Applying them is not a local commit.** The release PR is already merged, so its branch is gone; +fixes belong on the base branch this release ships from (`main` or `release/{MAJOR}.x`), which is +protected. Open a small PR for them, let CI run, and merge it — do not push to the base branch +directly, and do not amend or re-tag anything already reviewed. + +Then **re-target the draft release**, which is pinned to the previously approved merge commit and +therefore does not contain the fix: + +```sh +gh release edit v{version} --target {new-merge-commit-sha} +``` + +Regenerate the release notes afterward so the commit range covers the new PR, and re-run the Step 6 +section review for anything that changed. If the user prefers not to take the fix in this release, +that is a valid choice — leave the draft pinned where it is and note the deferred item, rather than +carrying a fix that the tag will not include. **Edge Cases:** - **Stale package closure** -- A package introduced between prepare-release and now may not be listed. Add it to `src/PACKAGE.md` and `README.md`. diff --git a/.github/skills/shared-resources/release-branches.md b/.github/skills/shared-resources/release-branches.md index a40cb24fa..298776b6a 100644 --- a/.github/skills/shared-resources/release-branches.md +++ b/.github/skills/shared-resources/release-branches.md @@ -25,8 +25,25 @@ Official NuGet.org publishes happen only when a GitHub Release is created from a ## Previous-release tag lookup -- On `main`: most recent published release globally (use `gh release list --exclude-drafts --limit 50` and pick the highest semver). No MAJOR filter. -- On `release/{MAJOR}.x`: most recent published release whose tag matches `v{MAJOR}.*`. Drafts are excluded. +Select the **highest semver** among published releases that are **ancestors of the target commit**, +excluding drafts: + +```sh +gh release list --exclude-drafts --limit 50 +``` + +- On `main`: no MAJOR filter — the highest semver ancestor wins. +- On `release/{MAJOR}.x`: restrict candidates to tags matching `v{MAJOR}.*`. + +**"Highest semver" and "most recent by date" are not the same rule, and the difference is not +hypothetical.** Ship `v2.1.0` from `main`, then a `v2.0.1` servicing patch from `release/2.0.x`, and +the most recently *published* release is `v2.0.1` while the highest semver is `v2.1.0`. Ordering by +date picks a tag that is not on `main` at all, which produces a bogus PR range and makes ApiCompat +report the entire API surface as removed. Order by version, not by publication time. + +The ancestry constraint is what makes this safe across branches, so verify it rather than assuming +the version ordering implied it — a tag can be both the highest semver and unreachable from the +target. `prepare-release` Step 2 performs this check explicitly. This is purely a baseline-selection rule. It does **not** change the breaking-change policy. See [the versioning docs](https://csharp.sdk.modelcontextprotocol.io/versioning.html) for the policy. diff --git a/.github/skills/verify-release/SKILL.md b/.github/skills/verify-release/SKILL.md index bbafe52f3..1f8a706b2 100644 --- a/.github/skills/verify-release/SKILL.md +++ b/.github/skills/verify-release/SKILL.md @@ -47,14 +47,27 @@ Record the tag, the published timestamp, and the target commitish for the follow ### Step 2: Locate Both Workflow Runs -Find the runs triggered by publishing this release. Match on the `release` event and a `created` -timestamp at or after the release's `publishedAt`. +Find the runs triggered by publishing this release. A release-event run carries the **tag name in +`headBranch`**, which is an exact identifier — use it rather than correlating on timestamps: ``` -gh run list --workflow release.yml --event release --limit 10 --json databaseId,status,conclusion,createdAt,url -gh run list --workflow docs.yml --event release --limit 10 --json databaseId,status,conclusion,createdAt,url +gh run list --workflow release.yml --event release --branch v{version} --limit 5 --json databaseId,status,conclusion,headBranch,headSha,createdAt,url +gh run list --workflow docs.yml --event release --branch v{version} --limit 5 --json databaseId,status,conclusion,headBranch,headSha,createdAt,url ``` +**Do not identify runs by "the most recent run" or "created at or after `publishedAt`."** Those +match any release published in the same window, so a concurrent or closely-following release — +including a servicing patch published from another branch minutes later — can be reported as this +release's result, showing a green run for the wrong tag. Confirm `headBranch` equals `v{version}` +on every run before evaluating it. + +Cross-check `headSha` against the release's target commitish recorded in Step 1. A mismatch means +the tag moved between drafting and publishing, and the run validated something other than what was +reviewed — stop and report it rather than evaluating the run. + +If more than one run matches the tag, the workflow was re-run; evaluate the **latest attempt** and +say that earlier attempts existed rather than silently reporting only the newest. + Present both runs with their status, conclusion, and URL. Watch them **together** — they run concurrently and either can fail independently. Do not report success for the release until both are accounted for. From 89b143f906f4318b5d374410abc2d7e42354773a Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 22:35:25 -0700 Subject: [PATCH 15/17] Say the anti-pattern plainly instead of quoting it The rule read "do not treat 'here are the finished notes' as a review", quoting a phrase that never appears as literal output and so scanned as a stray fragment. State the behavior directly; the guidance is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager.agent.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/agents/release-manager.agent.md b/.github/agents/release-manager.agent.md index cfb8cd061..50f61fbfb 100644 --- a/.github/agents/release-manager.agent.md +++ b/.github/agents/release-manager.agent.md @@ -167,9 +167,9 @@ Stage 5 Verify [verify-release skill, orch whether the PR contains code: sample-only and test-only PRs belong in Documentation Updates or Test Improvements. Maintainers are not acknowledged as issue reporters. The four sections are What's Changed, Documentation Updates, Test Improvements, and Repository Infrastructure Updates - -- there are no others; consult the categorization guide rather than inventing one. Do not treat - "here are the finished notes" as a review; complete, well-formatted notes read as correct and get - approved unexamined, and the corrections then arrive after the PR is open. + -- there are no others; consult the categorization guide rather than inventing one. Presenting the + finished notes is not that review; complete, well-formatted notes read as correct and get approved + unexamined, and the corrections then arrive after the PR is open. - **Never tune the validation to pass.** `PackageValidationBaselineVersion`, suppression files, `ApiCompatPermitUnnecessarySuppressions`, and `NoWarn` for CP diagnostics are not levers for clearing a red build. The baseline is whatever shipped; suppressions record breaks the user From 5e5a1b74f05ac7d711b7fd998dc4c6ce003e64b3 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 4 Aug 2026 23:28:53 -0700 Subject: [PATCH 16/17] Describe the review artifacts instead of populating them with real people The Step 10b tables and the CI handoff templates were filled in with actual PR numbers, commit SHAs, and contributor handles from the v2.1.0 release. A named maintainer appearing as the worked example of someone to omit from acknowledgements is the worst of these, but all of them share a defect: they read as data rather than as shape, and they go stale the moment those PRs are no longer the ones in flight. Use placeholders and say what each column turns on, so the format is legible without borrowing a real person to illustrate it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .../agents/release-manager/references/monitoring.md | 6 +++--- .github/skills/prepare-release/SKILL.md | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/agents/release-manager/references/monitoring.md b/.github/agents/release-manager/references/monitoring.md index 5f07e1a5b..b720ae516 100644 --- a/.github/agents/release-manager/references/monitoring.md +++ b/.github/agents/release-manager/references/monitoring.md @@ -101,16 +101,16 @@ proceed anyway. Record that decision and who made it. When handing off, lead with CI status rather than only inviting review: -> **CI: green** -- all 9 checks passed on `24c252cd`. PR #1792 is ready for your review and merge. +> **CI: green** -- all {n} checks passed on `{sha}`. PR #{number} is ready for your review and merge. or -> **CI: blocked** -- Pack / APICompat failed on `6d839c6d`. Diagnosis below. PR #1792 is not ready +> **CI: blocked** -- {check name} failed on `{sha}`. Diagnosis below. PR #{number} is not ready > to merge yet. or -> **CI: running** -- 4 of 9 checks complete, none failed. I am still watching and will report when +> **CI: running** -- {done} of {n} checks complete, none failed. I am still watching and will report when > they finish. Never say only "the PR is up, please review and merge." Without a CI verdict the user has to go diff --git a/.github/skills/prepare-release/SKILL.md b/.github/skills/prepare-release/SKILL.md index 478cbb12c..5e66a1637 100644 --- a/.github/skills/prepare-release/SKILL.md +++ b/.github/skills/prepare-release/SKILL.md @@ -227,12 +227,13 @@ borderline ones, since the user cannot correct a call they were not shown: | PR | Title | Section | Why | |---|---|---|---| -| #1778 | Add Application Insights telemetry example | Documentation Updates | Adds a sample; nothing under `src/` changed | +| #{number} | {title} | {section} | {what the placement turned on} | -Then explicitly surface the judgment calls: +Then explicitly surface the judgment calls, naming the PRs and the reasoning that made each one +close: -> These were the close calls: #1778 and #1762 touch code but not shipped packages, so I placed -> them under Documentation Updates. Any of these belong in a different section? +> These were the close calls: {PRs} touch code but not shipped packages, so I placed them under +> {section}. Any of these belong in a different section? Flag as a close call any PR that touches `samples/` or `tests/` but not `src/`, any PR placed in "What's Changed" whose changes are confined to non-shipping paths, and any PR whose title suggests @@ -242,7 +243,7 @@ a different section than the one you assigned. | Person | Reason | Maintainer? | |---|---|---| -| @halter73 | Submitted issue #1662 (resolved by #1775) | Yes — omit per Step 10 item 7 | +| @{handle} | {contribution or issue, and the PR that resolved it} | {yes/no — if yes, omit per Step 10 item 7} | Show entries you excluded and why, so the user can overrule the omission. Ask directly whether the remaining list is right, since acknowledgement errors are about people and are the least From 9cc808d1b1fbeb286f80563171c0f9179fcbae41 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Wed, 5 Aug 2026 14:13:37 -0700 Subject: [PATCH 17/17] Fix relative link depth in the release-manager references Both files sit three levels under .github/, so links to sibling top-level directories need ../../../. Two used ../../, resolving to .github/agents/skills and .github/agents/workflows, neither of which exists. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8535f33-08ac-44ef-84d1-aab96770ae3a --- .github/agents/release-manager/references/delegation.md | 2 +- .github/agents/release-manager/references/monitoring.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/agents/release-manager/references/delegation.md b/.github/agents/release-manager/references/delegation.md index 981896580..bcbdf3540 100644 --- a/.github/agents/release-manager/references/delegation.md +++ b/.github/agents/release-manager/references/delegation.md @@ -4,7 +4,7 @@ The release-manager session is an **orchestrator**. It stays on whatever branch never checks out or mutates a release branch. Work that creates commits happens in a **child session on its own worktree**, based on the target release branch. -This mirrors how [`docs.yml`](../../workflows/docs.yml) already works: the orchestration scripts run +This mirrors how [`docs.yml`](../../../workflows/docs.yml) already works: the orchestration scripts run from a single fixed checkout, while each version's content is built from its own tag in a separate worktree. diff --git a/.github/agents/release-manager/references/monitoring.md b/.github/agents/release-manager/references/monitoring.md index b720ae516..c5c258cbc 100644 --- a/.github/agents/release-manager/references/monitoring.md +++ b/.github/agents/release-manager/references/monitoring.md @@ -84,7 +84,7 @@ a deterministic product failure wastes a full CI cycle to arrive at the same red than reading the log. 3. **For ApiCompat and package validation failures specifically**, apply the interpretation rules in - [apicompat-apidiff.md](../../skills/prepare-release/references/apicompat-apidiff.md) before + [apicompat-apidiff.md](../../../skills/prepare-release/references/apicompat-apidiff.md) before concluding the release is breaking. `Unnecessary suppressions found` and a stale baseline produce large, convincing, and entirely phantom break listings.