modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(docs): correct the DCO rules against the validator source

Browse source 
Round 5 was right on all three counts. Read dcoapp/app/lib/dco.js this time
rather than the README, which is where the earlier errors came from:

  const authors = [commit.author.name, commit.committer.name]
  const emails  = [commit.author.email, commit.committer.email]
  if (isMerge || ...) continue          // parents.length > 1
  else if (author && author.type === 'Bot') continue

So, corrected:

- Scope. Merge commits and bot-authored commits are skipped, not gated. "Every
  commit" overstated it and could prompt a needless history rewrite.
- Matching. The trailer may match the author OR the committer, not the author
  alone. A cherry-picker's own signoff is accepted.
- Certification. The DCO expressly covers submitting work created by others
  that you have the right to submit, so signing off a cherry-pick is legitimate.
  My "never sign off someone else's commit" would have told a maintainer not to
  do a correct thing. The real prohibition is narrower: never fabricate a
  trailer in another person's name.

This also retracts the mechanical argument in bd6c1072 for dropping the hook —
it does satisfy the check, since you are the committer. The recommendation
against it stands on honest grounds now: the trailer is a certification, a hook
makes it for every commit automatically including work merely applied on
someone else's behalf, and it provably cannot tell the difference. A judgment
about deliberateness, not a claim that it breaks.

Mirror corrected to match.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-11 23:22:22 -04:00
commit c158ad809f
2 changed files with 4 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.github/copilot-instructions.md vendored
View file

@ -103,7 +103,7 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
- **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged.
- **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.
- **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch.
- **Every commit carries a `Signed-off-by:` trailer.** The DCO check is a hard merge gate and fails on a single unsigned commit. Use `git commit -s` (`format.signOff` does *not* sign `git commit` — only `format-patch` — and a `prepare-commit-msg` hook signs with the committer's identity, not the author's, so it breaks on cherry-picks). Repairing pushed commits means `git rebase HEAD~<n> --signoff` + `git push --force-with-lease`; remediation commits are not enabled on this repo.
- **Commits carry a `Signed-off-by:` trailer.** The DCO check is a hard merge gate and fails on a single unsigned commit; it matches the trailer against the author *or* committer, and skips only merge and bot commits. Use `git commit -s` — note `format.signOff` does *not* sign `git commit` (only `format-patch`). Repairing pushed commits means `git rebase HEAD~<n> --signoff` + `git push --force-with-lease`; remediation commits are not enabled on this repo.
- Update the relevant `README.md` / `AGENTS.md` when a change adds, removes, renames, or repurposes a file or folder, changes the structure or tech stack, or introduces a command, dependency, or architectural pattern.
## What to prioritize in review

8
AGENTS.md
View file

@ -207,16 +207,14 @@ All work should be driven by items on the project board.
- **Branch names start with the target version segment.** The first path segment must be the version whose base branch the PR targets — `v2/` for work on `v2/main`, `v1/` for work on `v1/main` — followed by the usual type and slug: `v2/ci/restore-claude-workflow`, `v2/fix/oauth-scope-union`, `v1/fix/proxy-ssrf-pin`. Not `ci/restore-claude-workflow`. This keeps the two lines legible in `git branch -a` and in the PR list once v1 and v2 branches coexist on the same remote, and it matches the base branches themselves (`v2/main`, `v1/main`).
- When work is complete:
- Run `npm run ci` from the root — the mandatory pre-push gate (see [Mandatory pre-push gate](#mandatory-pre-push-gate)). `npm run validate` is the fast inner-loop check and is **not** a substitute: it runs no coverage gate, no smokes, and no Storybook tests.
- **Sign off every commit — the DCO check is a hard merge gate.** The repo runs the [probot DCO app](https://probot.github.io/apps/dco/), which fails the PR unless **every** commit in it carries a `Signed-off-by: Author Name <author@example.com>` trailer matching its author. There is no partial credit: one unsigned commit out of six fails the whole check. The app *does* expose an override button to anyone with write access, but treat it as unavailable — see the repair bullet below for why.
- **Sign off your commits — the DCO check is a hard merge gate.** The repo runs the [probot DCO app](https://probot.github.io/apps/dco/), which fails the PR unless each commit carries a `Signed-off-by: Name <email>` trailer whose name **and** email match either the commit's **author or its committer** (the validator checks both, so a cherry-picker's own signoff is accepted). Its only exemptions are **merge commits** and **bot-authored** commits, which it skips; everything else counts, and there is no partial credit — one unsigned commit out of six fails the whole check. The app *does* expose an override button to anyone with write access, but treat it as unavailable — see the repair bullet below for why.
- **Prevent it with `git commit -s`** — the failure is invisible until after you have pushed, so make the flag habitual. Two things that look like automation and are not:
- ⚠️ **`git config format.signOff true` does nothing here.** Despite the name it only defaults the `-s` flag for `git format-patch`; `git commit` never reads it, and there is no `commit.signoff` equivalent. Setting it looks like a fix and silently changes nothing.
- ⚠️ **Don't reach for a `prepare-commit-msg` hook either**, however tempting. A hook can only sign with your *configured* identity, while the DCO check requires a trailer matching the commit **author** — and the two diverge exactly when you least want a wrong answer. Verified: on `git cherry-pick`, the resulting commit keeps the original author but the hook signs as you, producing a commit that both still fails the gate and certifies someone else's work. `git var GIT_AUTHOR_IDENT` does **not** resolve the true author inside that hook, so an author-aware guard doesn't rescue it.
If you want automation anyway, put it somewhere that knows the author — a CI pre-merge check, or a shell alias you invoke deliberately — not a blanket commit hook.
- ⚠️ **A `prepare-commit-msg` hook works, but think before installing one.** It would satisfy the check — a hook signs with your configured identity, and you are the committer, which the validator accepts. The objection is not mechanical but what it means: the trailer is a certification, and a hook makes it on your behalf for *every* commit, including work you merely cherry-picked or applied on someone else's behalf. Verified that the hook cannot distinguish those cases: inside `prepare-commit-msg`, `git var GIT_AUTHOR_IDENT` returns your config identity rather than the preserved author, so it cannot even tell that it is signing for someone else's authorship. Prefer `-s`, where each certification is a deliberate act.
- **Repairing already-pushed commits** means rewriting them: `git rebase HEAD~<n> --signoff`, then `git push --force-with-lease`. Use `--force-with-lease` rather than `--force` so a concurrent push can't be silently clobbered, and only rewrite when you are the sole author and nobody else has based work on the branch — the usual [perils of rebasing](https://git-scm.com/book/en/v2/Git-Branching-Rebasing). **Rewriting is the only legitimate repair, and the two apparent alternatives are not.** The app's empty "remediation commit" flow requires `allowRemediationCommits.individual`; this repo ships no `.github/dco.yml`, so it runs with that disabled and the original unsigned commits keep failing. The app *does* also give anyone with write access an override button that forces the check green — but it only silences the check, it does not make the author certify anything, and the signoff is the certification (see below). Using it on your own unsigned commits asserts nothing while looking like compliance, so don't: sign the commits instead. If others are working on the branch, coordinate with them before rewriting.
- The signoff is a **Developer Certificate of Origin** assertion in the author's own name, so it must be that author's identity — never sign off someone else's commit.
- The signoff is a **[Developer Certificate of Origin](https://developercertificate.org/)** assertion, made in **your own name**. It does not claim you wrote the code — the DCO expressly covers submitting work created by others that you have the right to submit — so signing off a commit you cherry-picked is legitimate. What is never acceptable is fabricating *someone else's* certification: don't add a trailer bearing another person's name or email. If a commit needs their signoff, they add it.
- Open a PR against the matching base branch (`v1/main` for v1, `v2/main` for v2) and set the item's Status to **In Review**
- **Attach screenshots as proof of functionality.** Any change to the web UI or the TUI must show its result: capture before/after screenshots (or a short GIF for an interaction) and put them in a **`pr-screenshots/` folder off the repo root**, creating it if it doesn't exist. That folder is **gitignored** — the images are working artifacts staged for upload, never committed to the source tree — so attach them to the PR body from there rather than referencing an in-repo path. Name them for what they show (`tools-tab-before.png`, `tools-tab-after.png`), not `Screenshot 2026-07-31 at 14.02.11.png`.
- **Link the PR to its issue — mandatory for every PR, from anyone.** No PR is opened without an issue to reference; if one doesn't exist yet, create it first (labeled and on the board) rather than opening the PR and backfilling. Note also that only the **repo maintainers** open PRs at all (see [Contributing](#contributing)) — everyone else files a detailed issue. The PR body's **first line must be `Closes #<ISSUE_NUMBER>`**. ⚠️ Note: closing keywords only auto-link/auto-close for PRs targeting the repo's **default branch** (`main`). Because v2 PRs target `v2/main` (a non-default branch), `Closes #N` there is only a cross-reference — it will **not** create a hard link or close the issue on merge. (There is no `gh` flag for manual linking — `gh pr edit` has no `--add-issue`; closing keywords are the only mechanism GitHub exposes, and they're gated to the default branch.)