Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .claude/skills/pr-flow/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)`,
Expand Down
11 changes: 7 additions & 4 deletions .github/workflows/dco.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
cliffhall marked this conversation as resolved.
# 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
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 #<ISSUE_NUMBER>` 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 #<ISSUE_NUMBER>` 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`.
Expand Down
2 changes: 1 addition & 1 deletion docs/quality-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
Loading