From b3242bc04c0748177bb6ea243f65be451d42fecc Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 11 Aug 2026 22:29:54 -0400 Subject: [PATCH] fix(docs): format.signOff does not sign commits, and remediation is unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both review points were factually right and my original text was wrong. 1. `git config format.signOff true` — which I had recommended as the *better* option — only defaults the -s flag for `git format-patch`. `git commit` never reads it, and there is no `commit.signoff` equivalent. Verified all three in a throwaway repo: format.signOff → no trailer; commit.signoff → no effect; a prepare-commit-msg hook → works. Shipping that advice would have been worse than saying nothing, since it looks like a fix and silently changes nothing. Now recommends `git commit -s`, gives the verified hook for automation, and names format.signOff explicitly as a trap — the name invites exactly the mistake I made, so it is worth warning about rather than omitting. 2. The DCO app's empty remediation-commit flow needs `allowRemediationCommits.individual`. This repo ships no DCO config, so it runs with that disabled and the original unsigned commits keep failing. The escape hatch I offered does not exist here; rewriting is the only repair, so the text now says to coordinate before rewriting shared history instead. Mirror updated with both corrections. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall --- .github/copilot-instructions.md | 2 +- AGENTS.md | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f9d995d3..ffc4c38f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -103,7 +103,7 @@ Both exist and do different jobs. Theme files (`src/theme/.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 #`. - **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; `git commit -s`, or `git config format.signOff true` once per clone. Repairing pushed commits means `git rebase HEAD~ --signoff` + `git push --force-with-lease`. +- **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` (note `format.signOff` does *not* sign `git commit` — only `format-patch`; automate with a `prepare-commit-msg` hook instead). Repairing pushed commits means `git rebase HEAD~ --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 diff --git a/AGENTS.md b/AGENTS.md index 6a19dfb8..afbb05c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -208,8 +208,20 @@ All work should be driven by items on the project board. - 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 ` trailer matching its author. There is no per-PR override and no partial credit: one unsigned commit out of six fails the whole check. - - **Prevent it, don't remember it.** Commit with `git commit -s`, or — better — run `git config format.signOff true` once per clone so the trailer is added automatically and the rule stops depending on memory. This matters more than the rule itself, because the failure is invisible until after you have pushed. - - **Repairing already-pushed commits** means rewriting them: `git rebase HEAD~ --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). If others are on the branch, add an empty remediation commit instead. + - **Prevent it, don't remember it** — the failure is invisible until after you have pushed. Commit with **`git commit -s`**. To stop relying on memory, add a `prepare-commit-msg` hook, which is the only mechanism that actually signs an ordinary `git commit`: + + ```sh + cat > .git/hooks/prepare-commit-msg <<'HOOK' + #!/bin/sh + grep -q '^Signed-off-by:' "$1" || git interpret-trailers --in-place \ + --trailer "Signed-off-by: $(git config user.name) <$(git config user.email)>" "$1" + HOOK + chmod +x .git/hooks/prepare-commit-msg + ``` + + ⚠️ **`git config format.signOff true` does NOT do this** — 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, so it is a trap worth naming rather than omitting. + + - **Repairing already-pushed commits** means rewriting them: `git rebase HEAD~ --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). **There is no non-rewrite escape hatch:** the DCO app's empty "remediation commit" flow requires `allowRemediationCommits.individual`, and this repo ships no DCO config, so it runs with that disabled and the original unsigned commits keep failing. If others are working on the branch, coordinate with them before rewriting rather than reaching for a remediation commit. - 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. - 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`.