diff --git a/.claude/skills/pr-flow/SKILL.md b/.claude/skills/pr-flow/SKILL.md index cbbb8994d..fb17c0ee7 100644 --- a/.claude/skills/pr-flow/SKILL.md +++ b/.claude/skills/pr-flow/SKILL.md @@ -105,8 +105,14 @@ bot-authored commits; there is no partial credit — one unsigned commit out of fails the whole check, and the job's output names each offending commit and the repair below. -⚠️ **It is a merge gate only because it is a _required_ status check** — a -ruleset setting, not something the workflow file can declare. The job runs on +⚠️ **It is a merge gate because it is a _required_ status check** — a +ruleset setting, not something the workflow file can declare. The +`v2/main - DCO` repository ruleset (#2621) requires `DCO` on every PR into +`v2/main`, pinned to the GitHub Actions app (integration `15368`) so a commit +status someone posts by hand under the same name cannot satisfy it; it also +blocks deleting or force-pushing `v2/main`, which the push backstop below +relies on. Repository admins can bypass it. A **stacked** PR's check runs but +gates nothing until the PR is retargeted to `v2/main`. The job runs on `pull_request`, from the PR's own ref, so it reports on every v2 PR — stacked ones included, any `v2/**` base — with no wait for a milestone merge (#2616). A second job, `DCO (v2/main push)`, diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 0ecc89e73..360fe4f17 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -9,10 +9,13 @@ # # Two jobs, one script: # -# - `DCO` — on every v2 PR, over the PR's commits. This is the -# one to make a REQUIRED status check (a ruleset change in repo settings, -# not something this file can do — without it, the job going missing -# passes as silently as the app did). +# - `DCO` — on every v2 PR, over the PR's commits. This is a REQUIRED +# status check on `v2/main`, set by the `v2/main - DCO` repository ruleset +# (#2621), not by this file — no workflow can make itself required. The +# ruleset pins the check to GitHub Actions (integration 15368), so a commit +# status posted by hand under the name `DCO` cannot satisfy it. ⚠️ Renaming +# this job renames the check, and the ruleset would then wait forever on a +# name nothing reports: change both together. # - `DCO (v2/main push)` — a backstop over every push that lands on # `v2/main`, for whatever reached the branch without a passing PR check # while this file and the script stayed intact: an admin merge, a direct diff --git a/AGENTS.md b/AGENTS.md index 6ed5e9105..76db4c112 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -391,7 +391,7 @@ skills; the rules are here. - **`Incoming` ⇔ no milestone; everything past it ⇔ milestoned — on board #28.** Board #11 is exempt for the reason above: a v1 issue has no bucket to take, so its Status is set on its own and the audit's milestone checks do not apply to it. A `[GHSA-` **advisory draft** on #28 is exempt too, for a different reason: a draft card cannot carry a milestone, so its approval act is **accepting the advisory**, which moves it `Incoming` → `Todo`; its milestone arrives with the public issue after publication. The rest of the invariant is unchanged: assigning the milestone _is_ the approval act, so the two always go together. `Todo` asserts a maintainer signed off, so never park an unreviewed issue there — that erases the distinction and quietly promotes unreviewed work into the queue. An issue created through the documented flow skips `Incoming` entirely, because filing it _was_ the approval. - **`Done` means the work shipped.** Exactly two things earn a card a place in Done: its **PR merged**, or it is a **parent whose last sub-issue closed**. Anything else — duplicate, won't fix, not planned, obsolete, superseded — means nothing shipped, so the card is **deleted**. Done is read as the record of what a milestone actually delivered; a duplicate sitting there makes that record wrong in a way nobody can detect later. Deleting a card touches the board only — the issue keeps its labels and comments and stays searchable forever. - **When work begins**, assign the issue to yourself, create a feature branch and set Status to **In Progress**. **Branch names start with the target version segment** — `v2/fix/2071-oauth-resource-metadata`, `v1/fix/proxy-ssrf-pin` — matching the base branches themselves. -- **When work is complete**, run `npm run format` then `npm run local:gate`, **sign off every commit** (`git commit -s` — the repo-owned `DCO` check, `.github/workflows/dco.yml`, fails a PR on any unsigned commit with no partial credit; it gates merges only as a **required** status check, which is a ruleset setting, so a PR missing the check is an outage, not a pass), open a PR against the matching base branch with **`Closes #` as the body's first line**, and set Status to **In Review**. +- **When work is complete**, run `npm run format` then `npm run local:gate`, **sign off every commit** (`git commit -s` — the repo-owned `DCO` check, `.github/workflows/dco.yml`, fails a PR on any unsigned commit with no partial credit; it gates merges into `v2/main` as a **required** status check, set by the `v2/main - DCO` repository ruleset (#2621) and pinned to GitHub Actions so only the workflow can satisfy it; a PR showing the check as missing or "expected" is an outage, not a pass), open a PR against the matching base branch with **`Closes #` as the body's first line**, and set Status to **In Review**. - **After opening a PR, run a Copilot review loop to exhaustion — unprompted.** Request a review, wait for the round to post _or_ for Copilot's session to end without one, answer every comment, and request again whenever a fix was pushed. Stop on the **first** clean round (no confirming round "just to be sure"), a round holding only out-of-scope findings, or two rounds in a row where Copilot's session ends without posting. **Weigh each finding against the issue the PR closes and decline scope expansion** — pre-existing behavior, new capabilities, and hardening the issue did not ask for — because that is what turns a review cycle into overbuilding. The recipe is the `pr-flow` skill, step 7. - **Attach screenshots as proof of functionality** for any web-UI or TUI change. Put them in a **`pr-screenshots/`** folder off the repo root — it is **gitignored**, so the images are staged for upload and never committed — and name them for what they show. - ⚠️ Closing keywords only auto-link and auto-close for PRs targeting the **default branch** (`main`). A v2 PR targets `v2/main`, so `Closes #N` there is only a cross-reference and the card shows no linked PR. **Link it explicitly** with the `addCloseIssueReferences` GraphQL mutation right after opening the PR (recipe in `pr-flow`, step 6). **On merge, manually close the issue and move the card to Done.** Keep the line anyway, so the issues close if/when `v2/main` reaches `main`. diff --git a/docs/quality-gate.md b/docs/quality-gate.md index 3d6bec1d4..7168a962b 100644 --- a/docs/quality-gate.md +++ b/docs/quality-gate.md @@ -14,7 +14,7 @@ Each client self-validates from its own folder; the root scripts chain them. The | **GitHub CI** (`.github/workflows/main.yml`) | Automatically, on every push | `npm install`, then `validate`, `verify:skills:cli`, `verify:build-gate`, `verify:bundle-externals`, `smoke` (which includes `smoke:web:chromium`), `test:storybook` — plus `coverage` in a parallel job ([#2159](https://github.com/modelcontextprotocol/inspector/issues/2159)) | | **The local gate** (`npm run local:gate`) | By hand, before you push | Every check above (the install is yours to run; `local:validate` stands in for `validate`, see below), **plus** the Firefox engine pass (`smoke:web:firefox`) | -One more CI check runs outside that table: **`.github/workflows/dco.yml`** fails on any commit that is not signed off ([#2566](https://github.com/modelcontextprotocol/inspector/issues/2566), [#2616](https://github.com/modelcontextprotocol/inspector/issues/2616)). It has two jobs. `DCO` runs on every *pull request with a `v2/**` base*, so `v2/main` and stacked v2 PRs, over the PR's own commits; v1 PRs and milestone PRs into `main` are out of its scope. `DCO (v2/main push)` re-checks every push that lands on `v2/main`, as a backstop for anything that merged without a passing PR check. The PR job runs on `pull_request`, so it is live as soon as the workflow is on `v2/main`. The trade-off is that a PR could edit its own check, and the push job could not catch that either, since a push run reads the workflow and the script from the pushed revision. It is accepted because PRs are maintainer-only, such an edit shows in the PR's diff, and the check exists to catch a *forgotten* signoff, not a forged one. The local gate runs the same script before a push as its `local:dco` stage (below), over `origin/v2/main..HEAD`. It replaced the probot DCO app, whose check vanished unnoticed when the app was suspended because it was never required. The replacement gates merges only as a **required** status check, a ruleset setting the workflow cannot declare. +One more CI check runs outside that table: **`.github/workflows/dco.yml`** fails on any commit that is not signed off ([#2566](https://github.com/modelcontextprotocol/inspector/issues/2566), [#2616](https://github.com/modelcontextprotocol/inspector/issues/2616)). It has two jobs. `DCO` runs on every *pull request with a `v2/**` base*, so `v2/main` and stacked v2 PRs, over the PR's own commits; v1 PRs and milestone PRs into `main` are out of its scope. `DCO (v2/main push)` re-checks every push that lands on `v2/main`, as a backstop for anything that merged without a passing PR check. The PR job runs on `pull_request`, so it is live as soon as the workflow is on `v2/main`. The trade-off is that a PR could edit its own check, and the push job could not catch that either, since a push run reads the workflow and the script from the pushed revision. It is accepted because PRs are maintainer-only, such an edit shows in the PR's diff, and the check exists to catch a *forgotten* signoff, not a forged one. The local gate runs the same script before a push as its `local:dco` stage (below), over `origin/v2/main..HEAD`. It replaced the probot DCO app, whose check vanished unnoticed when the app was suspended because it was never required. The replacement gates merges into `v2/main` as a **required** status check, set by the `v2/main - DCO` repository ruleset ([#2621](https://github.com/modelcontextprotocol/inspector/issues/2621)) because a workflow cannot declare itself required. The ruleset pins the check to GitHub Actions, so a hand-posted `DCO` status cannot satisfy it, and blocks deleting or force-pushing `v2/main`, which the push job's `before..after` range assumes never happens. The local gate runs **every check** `main.yml` runs, and is not a mirror. One of its steps has no GitHub CI counterpart: