From dc3199ff359b8eee476a81224200f7c7603c4a1f Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:06:30 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat(governance):=20ADR-003=20enforcement?= =?UTF-8?q?=20hooks=20=E2=80=94=20commit-msg,=20branch-naming,=20pickup-is?= =?UTF-8?q?sue=20skill=20(#186)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the three "Planned" ADR-003 enforcement rows (docs/decisions/ ADR-003-contribution-governance.md enforcement table) as offline-capable, no-new-dependency hooks + a Claude Code skill: - commit-msg hook (Tier 0): scripts/hooks/check-commit-msg.mjs rejects a commit whose message carries no issue reference (Refs/Fixes/Closes #N, the GitHub closing-keyword family + Ref/Refs). Wired as a commit-msg-type local hook; `commit-msg` added to default_install_hook_types. A bare `#N` with no keyword does not satisfy the gate. - branch-name hook (pre-push): scripts/hooks/check-branch-name.mjs rejects a branch not matching (feat|fix|chore|docs)/-. Exempts `main`, `dependabot/*`, and the detached-HEAD sentinel (ADR did not enumerate exemptions; minimal set documented in the ADR + PR body). - pickup-issue skill: docs/abca-plugin/skills/pickup-issue/SKILL.md — an agent-workflow gate that hard-fails without an approved, assigned issue before implementation. Advertised in the plugin SessionStart hook + README. Both scripts are plain Node ESM with node --test unit tests (23 cases, adversarial: missing ref, wrong prefix, missing issue number, comment-only ref). No new hook-runner or third-party tool added — reuses the existing prek/ pre-commit framework and Node. Deferred AC (flagged, not dropped): the pre-push Tier 1 `approved`-label `gh` check remains "Planned" — it needs a network call at push time, which this offline hook set intentionally avoids. The pickup-issue skill covers the approved-label gate at the agent-workflow layer in the interim. Docs: flipped the three implemented ADR-003 rows Planned -> Implemented (#186), documented exemptions + reference forms; updated CONTRIBUTING.md hooks list; regenerated Starlight mirrors via docs:sync. Closes #186 Co-authored-by: Claude Opus 4.8 --- .pre-commit-config.yaml | 19 ++- CONTRIBUTING.md | 5 +- docs/abca-plugin/README.md | 2 + docs/abca-plugin/hooks/hooks.json | 2 +- docs/abca-plugin/skills/pickup-issue/SKILL.md | 136 ++++++++++++++++++ .../ADR-003-contribution-governance.md | 14 +- .../Adr-003-contribution-governance.md | 14 +- .../docs/developer-guide/Contributing.md | 5 +- scripts/hooks/check-branch-name.mjs | 122 ++++++++++++++++ scripts/hooks/check-branch-name.test.mjs | 76 ++++++++++ scripts/hooks/check-commit-msg.mjs | 110 ++++++++++++++ scripts/hooks/check-commit-msg.test.mjs | 93 ++++++++++++ 12 files changed, 584 insertions(+), 14 deletions(-) create mode 100644 docs/abca-plugin/skills/pickup-issue/SKILL.md create mode 100644 scripts/hooks/check-branch-name.mjs create mode 100644 scripts/hooks/check-branch-name.test.mjs create mode 100644 scripts/hooks/check-commit-msg.mjs create mode 100644 scripts/hooks/check-commit-msg.test.mjs diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0b2f7a7ca..787642420 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ # Git hooks via https://github.com/j178/prek (installed by `mise run install` in a Git checkout; re-run `mise run hooks:install` after edits). # Config format matches pre-commit; run hooks with `prek` from mise (`mise.toml` [tools]). -default_install_hook_types: [pre-commit, pre-push] +default_install_hook_types: [commit-msg, pre-commit, pre-push] fail_fast: false exclude: ^\.threat-composer/ @@ -22,6 +22,23 @@ repos: - repo: local hooks: + # ADR-003 Tier 0: every commit must reference an issue (Refs/Fixes/Closes #N). + # prek passes the commit-message file path as the positional arg; forward it. + - id: adr003-commit-msg + name: ADR-003 commit-msg issue reference + entry: bash -lc 'cd "$(git rev-parse --show-toplevel)" && node scripts/hooks/check-commit-msg.mjs "$@"' -- + language: system + stages: [commit-msg] + + # ADR-003: feature branch must match (feat|fix|chore|docs)/-*. + # Runs at pre-push (branch name is stable by then); reads the current branch. + - id: adr003-branch-name + name: ADR-003 branch naming convention + entry: bash -lc 'cd "$(git rev-parse --show-toplevel)" && node scripts/hooks/check-branch-name.mjs' + language: system + pass_filenames: false + stages: [pre-push] + - id: gitleaks name: gitleaks (staged) entry: bash -lc 'cd "$(git rev-parse --show-toplevel)" && mise run security:secrets:staged' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 672bdc1a9..e50423124 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,8 +93,11 @@ PRs labeled `auto-approve` are approved automatically by the `auto-approve` work `mise run install` automatically installs [prek](https://github.com/j178/prek) git hooks. These run on every commit and push: +- **commit-msg** - ADR-003 Tier 0: rejects a commit message with no issue reference (`Refs #N` / `Fixes #N` / `Closes #N`). See [ADR-003](./docs/decisions/ADR-003-contribution-governance.md). - **pre-commit** - Whitespace/EOF checks, gitleaks on staged changes, linters (ESLint, Ruff, astro check) for touched files. -- **pre-push** - Security scans (`mise run hooks:pre-push:security`) and tests across all packages (`mise run hooks:pre-push:tests`). +- **pre-push** - ADR-003 branch-name check (branch must match `(feat|fix|chore|docs)/-*`; `main`, `dependabot/*`, and detached `HEAD` are exempt), security scans (`mise run hooks:pre-push:security`), and tests across all packages (`mise run hooks:pre-push:tests`). + +The ADR-003 governance hooks (`commit-msg`, branch-name) are plain Node scripts under `scripts/hooks/`; unit-test them with `node --test scripts/hooks/check-commit-msg.test.mjs scripts/hooks/check-branch-name.test.mjs`. If `prek install` fails with "refusing to install hooks with `core.hooksPath` set", another tool owns your hooks. Either unset it (`git config --unset-all core.hooksPath`) or integrate these checks into your hook manager. diff --git a/docs/abca-plugin/README.md b/docs/abca-plugin/README.md index bc2f98a8c..e08e9eba9 100644 --- a/docs/abca-plugin/README.md +++ b/docs/abca-plugin/README.md @@ -22,6 +22,7 @@ Or add to your project's `.claude/settings.json`: | Skill | Trigger | Description | |-------|---------|-------------| +| `/pickup-issue` | Start work, implement, claim issue | ADR-003 governance gate — verify an approved, assigned issue exists before writing code (hard-fails otherwise) | | `/setup` | First-time setup, prerequisites | Walk through prerequisites, toolchain, and first deployment | | `/deploy` | Deploy, diff, destroy | Deploy, diff, or destroy the CDK stack | | `/onboard-repo` | Add a repository | Onboard a GitHub repo via Blueprint CDK construct | @@ -51,6 +52,7 @@ docs/abca-plugin/ hooks/ hooks.json # SessionStart capability advertisement skills/ + pickup-issue/SKILL.md # ADR-003 governance gate (approved+assigned issue) setup/SKILL.md # First-time setup workflow deploy/SKILL.md # CDK deployment management onboard-repo/SKILL.md # Repository onboarding diff --git a/docs/abca-plugin/hooks/hooks.json b/docs/abca-plugin/hooks/hooks.json index d7e87cbc5..050fbe7d2 100644 --- a/docs/abca-plugin/hooks/hooks.json +++ b/docs/abca-plugin/hooks/hooks.json @@ -3,7 +3,7 @@ "SessionStart": [ { "type": "prompt", - "prompt": "The ABCA plugin is active. Available skills:\n- /setup — first-time setup walkthrough\n- /deploy — deploy, diff, or destroy the CDK stack\n- /onboard-repo — add a GitHub repository\n- /submit-task — submit a coding task (guided or quick mode)\n- /troubleshoot — diagnose deployment, auth, or task issues\n- /status — platform health check (stack, tasks, build)\n\nAvailable agents: cdk-expert (CDK infrastructure), agent-debugger (task failure investigation).\n\nInteractive task commands (suggest when user is monitoring or steering a running task):\n- bgagent watch — stream progress events in real time\n- bgagent nudge \"\" — steer the agent mid-run\n- bgagent trace download — download full execution trace\n- bgagent webhook create/list/revoke — manage webhook integrations\n- --trace flag on submit — enable detailed tracing for a task\n- --verbose flag — HTTP debug output on any command\n\nSuggest relevant skills when the user's request matches." + "prompt": "The ABCA plugin is active. Available skills:\n- /pickup-issue — ADR-003 gate: verify an approved, assigned issue BEFORE implementing (hard-fails otherwise)\n- /setup — first-time setup walkthrough\n- /deploy — deploy, diff, or destroy the CDK stack\n- /onboard-repo — add a GitHub repository\n- /submit-task — submit a coding task (guided or quick mode)\n- /troubleshoot — diagnose deployment, auth, or task issues\n- /status — platform health check (stack, tasks, build)\n\nAvailable agents: cdk-expert (CDK infrastructure), agent-debugger (task failure investigation).\n\nInteractive task commands (suggest when user is monitoring or steering a running task):\n- bgagent watch — stream progress events in real time\n- bgagent nudge \"\" — steer the agent mid-run\n- bgagent trace download — download full execution trace\n- bgagent webhook create/list/revoke — manage webhook integrations\n- --trace flag on submit — enable detailed tracing for a task\n- --verbose flag — HTTP debug output on any command\n\nSuggest relevant skills when the user's request matches." } ] } diff --git a/docs/abca-plugin/skills/pickup-issue/SKILL.md b/docs/abca-plugin/skills/pickup-issue/SKILL.md new file mode 100644 index 000000000..97cdfc818 --- /dev/null +++ b/docs/abca-plugin/skills/pickup-issue/SKILL.md @@ -0,0 +1,136 @@ +--- +name: pickup-issue +description: >- + ADR-003 governance gate — verify an approved, assigned GitHub issue exists + BEFORE writing any code. Invoke before starting implementation. Hard-fails if + there is no valid `approved` issue. Use when the user says "start work", + "implement this", "pick up an issue", "begin the task", "let's build X", + "go ahead and code", "start coding", "claim issue", or directs implementation + without first pointing to an approved issue. +argument-hint: +--- + +# Pick Up an Issue (ADR-003 Governance Gate) + +You are enforcing the ABCA contribution-governance gate defined in +[ADR-003](../../../decisions/ADR-003-contribution-governance.md). **No code is +written until a durable, approved, assigned issue exists.** This is a hard gate, +not advice — if any check below fails, STOP and do not begin implementation. + +> **Why this exists:** The most common governance bypass is treating +> conversational momentum ("yes, go ahead") as authorization. Conversations are +> ephemeral; issues are auditable. This skill forces the check that the branch +> and commit hooks cannot: that the issue is *approved* and *assigned* before a +> single file changes. See ADR-003 "Conversational approval is NOT issue +> approval". + +## When to hard-fail (STOP — do not implement) + +- No issue number was provided and none can be identified. +- The referenced issue does not exist. +- The issue lacks the `approved` label. +- The issue is closed. +- The issue is unassigned, or is assigned to someone other than the acting + identity without declared intentionality (multiple assignees need + intentionality per ADR-003 "Assignments"). + +In any of these cases, respond with the specific failure and the remediation +(create the issue / request the `approved` label from an admin / self-assign), +then STOP. Do NOT create branches, write files, or run implementation commands. + +## Step 1: Identify the issue + +Determine the target issue number from the user's request or the current branch +name (which, per ADR-003, encodes it as `(feat|fix|chore|docs)/-*`). + +```bash +# From an explicit number the user gave, or extract from the branch: +git rev-parse --abbrev-ref HEAD # e.g. feat/186-adr003-hooks -> 186 +``` + +If no issue number can be determined, **hard-fail**: ask the user to create an +issue with acceptance criteria and obtain the `approved` label first. + +## Step 2: Verify the issue is approved and workable + +Query GitHub. The issue must exist, be OPEN, carry the `approved` label, and be +assigned. + +```bash +gh issue view --json number,title,state,labels,assignees \ + --jq '{number,title,state,labels:[.labels[].name],assignees:[.assignees[].login]}' +``` + +Validate the response: + +| Field | Required | Hard-fail if | +|-------|----------|--------------| +| `state` | `OPEN` | closed | +| `labels` | contains `approved` | missing `approved` | +| `assignees` | contains the acting identity | empty (unassigned) or assigned only to others | + +If `assignees` is empty, self-assign before proceeding: + +```bash +gh issue edit --add-assignee @me +# then re-read to confirm sole ownership (self-assignment is not atomic — +# ADR-003 warns concurrent agents may race; verify after claiming) +gh issue view --json assignees --jq '[.assignees[].login]' +``` + +## Step 3: Pre-start synthesis (ADR-003 "Pre-start review") + +Before implementing, synthesize context so the body + thread are unambiguous: + +- **Read the full thread** — body, comments, replies. Surface any inconsistency + between the body (primary directive) and later clarifications. +- **Check for blockers** — any `**UNRESOLVED:** ` in the body or + thread blocks implementation. `**DEFERRED:** — tracked in #N` does + not block. +- **Predecessor validation** — the dependency graph is authoritative: + + ```bash + gh api graphql -f query=' + query($owner:String!,$repo:String!,$num:Int!){ + repository(owner:$owner,name:$repo){ + issue(number:$num){ + title + trackedInIssues(first:20){ nodes{ number title state } } # blockedBy + } + } + }' -f owner= -f repo= -F num= + ``` + + If any blocking issue is OPEN, this issue is **not ready** — hard-fail. +- **Priority evaluation** — if asked to work a lower-priority item while higher + `p0`/`p1` items are unassigned, challenge before proceeding. +- **Cross-reference audit** — search open issues/PRs (including drafts) for + duplicates or conflicts; flag overlaps. + +## Step 4: Final gate + +Only if ALL checks pass: + +1. Comment "Starting implementation." on the issue (the durable start signal). +2. Confirm to the user that the gate passed and implementation may begin. + +```bash +gh issue comment --body "Starting implementation." +``` + +If any check failed, you have already stopped at that step. Do not reach Step 4. + +## Relationship to the git hooks + +This skill is the **agent-workflow** layer of ADR-003 enforcement. It complements +but does not replace the git hooks (which every contributor, human or agent, +also gets): + +- **commit-msg hook** (`scripts/hooks/check-commit-msg.mjs`) — rejects commits + with no `Refs #N` / `Fixes #N` / `Closes #N` reference. +- **branch-name hook** (`scripts/hooks/check-branch-name.mjs`, pre-push) — + rejects branches not matching `(feat|fix|chore|docs)/-*`. + +The hooks catch *unreferenced* work mechanically; this skill catches +*unapproved* work before it starts (the hooks cannot query the `approved` label +without network access at commit time). diff --git a/docs/decisions/ADR-003-contribution-governance.md b/docs/decisions/ADR-003-contribution-governance.md index 06a0d7fdd..b9e73e773 100644 --- a/docs/decisions/ADR-003-contribution-governance.md +++ b/docs/decisions/ADR-003-contribution-governance.md @@ -95,15 +95,19 @@ Prose governance is necessary but insufficient. The following enforcement points | Mechanism | Layer | What it catches | Status | |-----------|-------|-----------------|--------| | AGENTS.md directive | Agent prompt | Explicit instruction: "Do NOT begin implementation without an approved issue, even if the user says 'go ahead' in conversation" | Implemented | -| Branch name convention | Git workflow | Branch must match `(feat|fix|chore|docs)/-*` — rejects branches without issue reference | Planned | -| Commit-msg hook (Tier 0) | Pre-commit | Rejects commits without `Refs #N` or `Fixes #N` | Planned | -| Pre-push hook (Tier 1) | Pre-push | Validates referenced issue exists and has `approved` label via `gh` API | Planned | +| Branch name convention | Pre-push | Branch must match `(feat\|fix\|chore\|docs)/-*` — rejects branches without issue reference | Implemented (#186) — `scripts/hooks/check-branch-name.mjs` | +| Commit-msg hook (Tier 0) | Commit-msg | Rejects commits without `Refs #N` / `Fixes #N` / `Closes #N` | Implemented (#186) — `scripts/hooks/check-commit-msg.mjs` | +| Pre-push hook (Tier 1) | Pre-push | Validates referenced issue exists and has `approved` label via `gh` API | Planned (deferred from #186 — needs network at push time; follow-up) | | Claude Code hook (`PreToolUse: Write`) | Agent runtime | Blocks file creation in governed paths without declared issue context | Planned | -| Skill gate: `pickup-issue` | Agent workflow | Agent must invoke before implementation — hard-fails without valid issue | Planned | +| Skill gate: `pickup-issue` | Agent workflow | Agent must invoke before implementation — hard-fails without valid issue | Implemented (#186) — `docs/abca-plugin/skills/pickup-issue/SKILL.md` | **Transition:** Branch naming and commit-msg rules apply to branches created after the corresponding hooks are deployed. Existing branches (including this PR's) pre-date enforcement. -**Progressive enforcement:** Start with the commit-msg hook (cheapest, catches all contributors). Add pre-push validation next. Skill gates enforce at the agent-workflow level (see ADR-012, proposed, for the skill model). +**Branch-name exemptions:** The branch-name hook exempts `main` (the trunk is not a feature branch), `dependabot/*` (bot-authored upgrade branches), and the detached-`HEAD` sentinel. These are not enumerated elsewhere in this ADR; they are the minimal set required so the trunk and machine-generated branches are not falsely rejected. + +**Commit-msg reference forms:** The Tier 0 hook accepts the GitHub closing-keyword family (`Close`/`Closes`/`Closed`, `Fix`/`Fixes`/`Fixed`, `Resolve`/`Resolves`/`Resolved`) plus `Ref`/`Refs`, each followed by `#N`, `GH-N`, or `owner/repo#N`. A bare `#N` mention with no keyword does **not** satisfy the gate — the reference must be an intentional issue link, not incidental prose. + +**Progressive enforcement:** Start with the commit-msg hook (cheapest, catches all contributors). Add pre-push validation next. Skill gates enforce at the agent-workflow level (see ADR-012, proposed, for the skill model). The pre-push Tier 1 `approved`-label check (row above) is deferred — it needs a network `gh` call at push time, which the offline-capable hook set intentionally avoids; the `pickup-issue` skill covers the `approved`-label gate at the agent-workflow layer in the interim. ## Consequences diff --git a/docs/src/content/docs/decisions/Adr-003-contribution-governance.md b/docs/src/content/docs/decisions/Adr-003-contribution-governance.md index f8938626f..4f4b4f413 100644 --- a/docs/src/content/docs/decisions/Adr-003-contribution-governance.md +++ b/docs/src/content/docs/decisions/Adr-003-contribution-governance.md @@ -99,15 +99,19 @@ Prose governance is necessary but insufficient. The following enforcement points | Mechanism | Layer | What it catches | Status | |-----------|-------|-----------------|--------| | AGENTS.md directive | Agent prompt | Explicit instruction: "Do NOT begin implementation without an approved issue, even if the user says 'go ahead' in conversation" | Implemented | -| Branch name convention | Git workflow | Branch must match `(feat|fix|chore|docs)/-*` — rejects branches without issue reference | Planned | -| Commit-msg hook (Tier 0) | Pre-commit | Rejects commits without `Refs #N` or `Fixes #N` | Planned | -| Pre-push hook (Tier 1) | Pre-push | Validates referenced issue exists and has `approved` label via `gh` API | Planned | +| Branch name convention | Pre-push | Branch must match `(feat\|fix\|chore\|docs)/-*` — rejects branches without issue reference | Implemented (#186) — `scripts/hooks/check-branch-name.mjs` | +| Commit-msg hook (Tier 0) | Commit-msg | Rejects commits without `Refs #N` / `Fixes #N` / `Closes #N` | Implemented (#186) — `scripts/hooks/check-commit-msg.mjs` | +| Pre-push hook (Tier 1) | Pre-push | Validates referenced issue exists and has `approved` label via `gh` API | Planned (deferred from #186 — needs network at push time; follow-up) | | Claude Code hook (`PreToolUse: Write`) | Agent runtime | Blocks file creation in governed paths without declared issue context | Planned | -| Skill gate: `pickup-issue` | Agent workflow | Agent must invoke before implementation — hard-fails without valid issue | Planned | +| Skill gate: `pickup-issue` | Agent workflow | Agent must invoke before implementation — hard-fails without valid issue | Implemented (#186) — `docs/abca-plugin/skills/pickup-issue/SKILL.md` | **Transition:** Branch naming and commit-msg rules apply to branches created after the corresponding hooks are deployed. Existing branches (including this PR's) pre-date enforcement. -**Progressive enforcement:** Start with the commit-msg hook (cheapest, catches all contributors). Add pre-push validation next. Skill gates enforce at the agent-workflow level (see ADR-012, proposed, for the skill model). +**Branch-name exemptions:** The branch-name hook exempts `main` (the trunk is not a feature branch), `dependabot/*` (bot-authored upgrade branches), and the detached-`HEAD` sentinel. These are not enumerated elsewhere in this ADR; they are the minimal set required so the trunk and machine-generated branches are not falsely rejected. + +**Commit-msg reference forms:** The Tier 0 hook accepts the GitHub closing-keyword family (`Close`/`Closes`/`Closed`, `Fix`/`Fixes`/`Fixed`, `Resolve`/`Resolves`/`Resolved`) plus `Ref`/`Refs`, each followed by `#N`, `GH-N`, or `owner/repo#N`. A bare `#N` mention with no keyword does **not** satisfy the gate — the reference must be an intentional issue link, not incidental prose. + +**Progressive enforcement:** Start with the commit-msg hook (cheapest, catches all contributors). Add pre-push validation next. Skill gates enforce at the agent-workflow level (see ADR-012, proposed, for the skill model). The pre-push Tier 1 `approved`-label check (row above) is deferred — it needs a network `gh` call at push time, which the offline-capable hook set intentionally avoids; the `pickup-issue` skill covers the `approved`-label gate at the agent-workflow layer in the interim. ## Consequences diff --git a/docs/src/content/docs/developer-guide/Contributing.md b/docs/src/content/docs/developer-guide/Contributing.md index 86f5f0b3e..40cc3cc95 100644 --- a/docs/src/content/docs/developer-guide/Contributing.md +++ b/docs/src/content/docs/developer-guide/Contributing.md @@ -97,8 +97,11 @@ PRs labeled `auto-approve` are approved automatically by the `auto-approve` work `mise run install` automatically installs [prek](https://github.com/j178/prek) git hooks. These run on every commit and push: +- **commit-msg** - ADR-003 Tier 0: rejects a commit message with no issue reference (`Refs #N` / `Fixes #N` / `Closes #N`). See [ADR-003](/sample-autonomous-cloud-coding-agents/architecture/adr-003-contribution-governance). - **pre-commit** - Whitespace/EOF checks, gitleaks on staged changes, linters (ESLint, Ruff, astro check) for touched files. -- **pre-push** - Security scans (`mise run hooks:pre-push:security`) and tests across all packages (`mise run hooks:pre-push:tests`). +- **pre-push** - ADR-003 branch-name check (branch must match `(feat|fix|chore|docs)/-*`; `main`, `dependabot/*`, and detached `HEAD` are exempt), security scans (`mise run hooks:pre-push:security`), and tests across all packages (`mise run hooks:pre-push:tests`). + +The ADR-003 governance hooks (`commit-msg`, branch-name) are plain Node scripts under `scripts/hooks/`; unit-test them with `node --test scripts/hooks/check-commit-msg.test.mjs scripts/hooks/check-branch-name.test.mjs`. If `prek install` fails with "refusing to install hooks with `core.hooksPath` set", another tool owns your hooks. Either unset it (`git config --unset-all core.hooksPath`) or integrate these checks into your hook manager. diff --git a/scripts/hooks/check-branch-name.mjs b/scripts/hooks/check-branch-name.mjs new file mode 100644 index 000000000..3f95bcced --- /dev/null +++ b/scripts/hooks/check-branch-name.mjs @@ -0,0 +1,122 @@ +#!/usr/bin/env node +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/** + * ADR-003 enforcement: branch-name validation (pre-push). + * + * ADR-003 (docs/decisions/ADR-003-contribution-governance.md, "No branches + * without an Issue" + enforcement table) requires a feature branch to match + * `(feat|fix|chore|docs)/-` — a branch without an issue + * reference is unauthorized work. + * + * Exemptions (branches that are not contributor feature work): + * - `main` — the trunk itself is not a feature branch. + * - `dependabot/*` — bot-authored dependency-upgrade branches. + * - `HEAD` — the sentinel `git rev-parse --abbrev-ref HEAD` + * returns in a detached-HEAD state; do not block. + * ADR-003 does not enumerate exemptions in prose; these are the minimal set + * needed so the trunk and machine-generated branches are not falsely rejected. + * See the PR body for this note. + * + * Wiring: a `pre-push`-type local hook in `.pre-commit-config.yaml`. + * + * Usage: + * node scripts/hooks/check-branch-name.mjs [branch-name] + * When no argument is given, the current branch is read via git plumbing + * (`git rev-parse --abbrev-ref HEAD`). + */ + +import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; + +const BRANCH_PATTERN = /^(?:feat|fix|chore|docs)\/\d+-.+/; +const EXEMPT_EXACT = new Set(['main', 'HEAD', '']); + +/** + * @param {string} branch + * @returns {boolean} + */ +function isExempt(branch) { + if (EXEMPT_EXACT.has(branch)) return true; + if (branch.startsWith('dependabot/')) return true; + return false; +} + +/** + * @param {string} branch current branch name + * @returns {{ ok: boolean, reason?: string }} + */ +export function validateBranchName(branch) { + const name = branch ?? ''; + if (isExempt(name)) { + return { ok: true }; + } + if (BRANCH_PATTERN.test(name)) { + return { ok: true }; + } + return { + ok: false, + reason: + `Branch "${name}" does not match the ADR-003 convention ` + + '`(feat|fix|chore|docs)/-` ' + + '(e.g. `feat/123-short-description`). A branch without an issue reference ' + + 'is unauthorized work — rename it: `git branch -m `.', + }; +} + +/** + * Resolve the current branch via git plumbing. Fail loud on error rather than + * defaulting to a pass, so a broken git invocation cannot silently disable the + * gate. + * @returns {string} + */ +function currentBranch() { + return execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { + encoding: 'utf8', + }).trim(); +} + +function main(argv) { + let branch; + if (argv[2]) { + branch = argv[2]; + } else { + try { + branch = currentBranch(); + } catch (err) { + console.error(`check-branch-name: could not determine current branch: ${err.message}`); + return 2; + } + } + + const result = validateBranchName(branch); + if (result.ok) { + return 0; + } + + console.error(`❌ ${result.reason}`); + console.error(' ADR-003: docs/decisions/ADR-003-contribution-governance.md'); + return 1; +} + +// Run only when invoked directly, not when imported by the test suite. +if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { + process.exit(main(process.argv)); +} diff --git a/scripts/hooks/check-branch-name.test.mjs b/scripts/hooks/check-branch-name.test.mjs new file mode 100644 index 000000000..034d09502 --- /dev/null +++ b/scripts/hooks/check-branch-name.test.mjs @@ -0,0 +1,76 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { validateBranchName } from './check-branch-name.mjs'; + +// ADR-003: a feature branch must match `(feat|fix|chore|docs)/-*`. + +test('accepts feat/-', () => { + assert.equal(validateBranchName('feat/186-adr003-hooks').ok, true); +}); + +test('accepts fix/-', () => { + assert.equal(validateBranchName('fix/456-bug-name').ok, true); +}); + +test('accepts chore/ and docs/ prefixes', () => { + assert.equal(validateBranchName('chore/12-tidy').ok, true); + assert.equal(validateBranchName('docs/191-agents-md-split').ok, true); +}); + +test('rejects a prefix outside the allowed set', () => { + const r = validateBranchName('feature/186-thing'); + assert.equal(r.ok, false); + assert.match(r.reason, /feat\|fix\|chore\|docs/); +}); + +test('rejects a branch with no issue number', () => { + const r = validateBranchName('feat/adr003-hooks'); + assert.equal(r.ok, false); +}); + +test('rejects a branch missing the description', () => { + // `-` requires a hyphen + at least one desc char. + assert.equal(validateBranchName('feat/186').ok, false); + assert.equal(validateBranchName('feat/186-').ok, false); +}); + +test('rejects a bare branch name with no prefix', () => { + assert.equal(validateBranchName('my-random-branch').ok, false); +}); + +// Exemptions — branches that are not contributor feature work. +test('exempts main', () => { + assert.equal(validateBranchName('main').ok, true); +}); + +test('exempts dependabot/* branches', () => { + assert.equal(validateBranchName('dependabot/npm_and_yarn/foo-1.2.3').ok, true); +}); + +test('exempts the HEAD detached / empty sentinel', () => { + // git rev-parse --abbrev-ref HEAD returns "HEAD" when detached; do not block. + assert.equal(validateBranchName('HEAD').ok, true); +}); + +test('does not exempt a lookalike prefix (maindev)', () => { + assert.equal(validateBranchName('maindev').ok, false); +}); diff --git a/scripts/hooks/check-commit-msg.mjs b/scripts/hooks/check-commit-msg.mjs new file mode 100644 index 000000000..8a0bd21fc --- /dev/null +++ b/scripts/hooks/check-commit-msg.mjs @@ -0,0 +1,110 @@ +#!/usr/bin/env node +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/** + * ADR-003 Tier 0 enforcement: commit-msg hook. + * + * Rejects a commit whose message carries no issue reference. ADR-003 + * (docs/decisions/ADR-003-contribution-governance.md, enforcement table) + * specifies the rule as "Rejects commits without `Refs #N` or `Fixes #N`". + * We accept the full GitHub closing-keyword family (Closes/Fixes/Resolves and + * their inflections) plus `Refs`/`Ref`, since those are what actually link a + * commit to an issue on GitHub. A bare `#N` mention with no keyword does NOT + * satisfy the gate — the reference must be intentional, not incidental prose. + * + * Wiring: a `commit-msg`-type local hook in `.pre-commit-config.yaml`. prek / + * pre-commit invoke commit-msg hooks with the path to the commit message file + * (`.git/COMMIT_EDITMSG`) as the sole positional argument. + * + * Usage: + * node scripts/hooks/check-commit-msg.mjs + */ + +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +// Keyword family that links a commit to an issue on GitHub, plus `Refs`/`Ref`. +// Reference forms accepted after the keyword: `#N`, `GH-N`, or `owner/repo#N`. +const ISSUE_REF = + /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?)\b\s+(?:[\w.-]+\/[\w.-]+)?(?:#|gh-)\d+/i; + +/** + * Strip git comment lines (those beginning with `#`), which git removes before + * storing the commit. A keyword that only appears in a comment must not count. + * Note: `#N` issue references never start a line with the keyword, so removing + * whole `#`-leading lines cannot hide a legitimate reference. + */ +function stripComments(message) { + return message + .split('\n') + .filter((line) => !line.startsWith('#')) + .join('\n'); +} + +/** + * @param {string} message raw commit message + * @returns {{ ok: boolean, reason?: string }} + */ +export function validateCommitMessage(message) { + const body = stripComments(message ?? ''); + if (ISSUE_REF.test(body)) { + return { ok: true }; + } + return { + ok: false, + reason: + 'Commit message is missing an issue reference. ADR-003 (Tier 0) requires ' + + 'a keyword + issue link, e.g. `Refs #N`, `Fixes #N`, or `Closes #N`. ' + + 'Add one referencing the approved issue this commit implements.', + }; +} + +function main(argv) { + const msgPath = argv[2]; + if (!msgPath) { + console.error( + 'check-commit-msg: no commit message file path given. This hook must be ' + + 'wired as a `commit-msg`-type hook so the message file path is passed.', + ); + return 2; + } + + let message; + try { + message = readFileSync(msgPath, 'utf8'); + } catch (err) { + console.error(`check-commit-msg: could not read ${msgPath}: ${err.message}`); + return 2; + } + + const result = validateCommitMessage(message); + if (result.ok) { + return 0; + } + + console.error(`❌ ${result.reason}`); + console.error(' ADR-003: docs/decisions/ADR-003-contribution-governance.md'); + return 1; +} + +// Run only when invoked directly, not when imported by the test suite. +if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { + process.exit(main(process.argv)); +} diff --git a/scripts/hooks/check-commit-msg.test.mjs b/scripts/hooks/check-commit-msg.test.mjs new file mode 100644 index 000000000..d5de1b689 --- /dev/null +++ b/scripts/hooks/check-commit-msg.test.mjs @@ -0,0 +1,93 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { validateCommitMessage } from './check-commit-msg.mjs'; + +// ADR-003 Tier 0: a commit message must carry an issue reference — +// `Refs #N`, `Fixes #N`, or `Closes #N` (case-insensitive, GitHub's own +// closing-keyword family). These accept anywhere in the message body. + +test('accepts a message with Closes #N', () => { + const r = validateCommitMessage('feat(x): thing\n\nBody\n\nCloses #186\n'); + assert.equal(r.ok, true); +}); + +test('accepts a message with Fixes #N', () => { + const r = validateCommitMessage('fix: bug\n\nFixes #42'); + assert.equal(r.ok, true); +}); + +test('accepts a message with Refs #N', () => { + const r = validateCommitMessage('chore: tidy\n\nRefs #7'); + assert.equal(r.ok, true); +}); + +test('rejects a bare (#N) with no keyword even on the subject line', () => { + const r = validateCommitMessage('docs: update guide (#191)'); + // Bare (#N) alone is NOT a governance keyword — must be Refs/Fixes/Closes. + assert.equal(r.ok, false); +}); + +test('accepts the keyword+reference on the subject line', () => { + const r = validateCommitMessage('docs: update guide, Closes #191'); + assert.equal(r.ok, true); +}); + +test('accepts GitHub closing synonyms (Resolves, Close, Fix, Ref)', () => { + for (const kw of ['Resolves', 'Resolve', 'Close', 'Closed', 'Fix', 'Fixed', 'Ref']) { + const r = validateCommitMessage(`feat: x\n\n${kw} #99`); + assert.equal(r.ok, true, `expected "${kw} #99" to pass`); + } +}); + +test('is case-insensitive on the keyword', () => { + const r = validateCommitMessage('feat: x\n\ncloses #12'); + assert.equal(r.ok, true); +}); + +test('rejects a message with no issue reference', () => { + const r = validateCommitMessage('feat: add a thing without any reference'); + assert.equal(r.ok, false); + assert.match(r.reason, /Refs #N|Fixes #N|Closes #N/); +}); + +test('rejects a bare "#186" without a keyword', () => { + const r = validateCommitMessage('feat: mentions #186 in prose but no keyword'); + assert.equal(r.ok, false); +}); + +test('rejects an empty message', () => { + const r = validateCommitMessage(''); + assert.equal(r.ok, false); +}); + +test('ignores comment lines (git # comments) when scanning', () => { + // Lines beginning with '#' are git scissors/comments and are stripped by + // git before the commit is stored; a keyword hidden only in a comment must + // NOT satisfy the gate. + const r = validateCommitMessage('feat: x\n\n# Closes #5 (this is a comment)\n'); + assert.equal(r.ok, false); +}); + +test('accepts owner/repo#N cross-repo references', () => { + const r = validateCommitMessage('fix: x\n\nFixes aws-samples/sample#3'); + assert.equal(r.ok, true); +}); From 62b041e50ebaf4f52dd73a70fca27b5f908a44de Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:11:52 +0000 Subject: [PATCH 2/4] test(governance): wire hook tests into CI build DAG + cover GH-N/CRLF/boundary (#186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the /review_pr blocking finding: the new hook unit tests were orphaned — no CI job or mise task ran scripts/hooks/*.test.mjs, so the Tier-0 governance regexes could silently regress. - Add mise task `test:hooks` (node --test scripts/hooks/*.test.mjs; explicit glob avoids Node 22's spurious directory-arg failure) and add it to the `build` DAG so `mise run build` (which CI runs) executes it. - Add the cheap test cases the reviewers flagged: GH-N reference form (a documented-but-untested accepted form), no-separator rejection (Closes#5), word-boundary rejection (refixes), and CRLF handling (body ref survives, comment-only ref does not). 23 -> 27 tests, all green. Refs #186 --- mise.toml | 8 ++++++++ scripts/hooks/check-commit-msg.test.mjs | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/mise.toml b/mise.toml index f416aea13..4f6690cc6 100644 --- a/mise.toml +++ b/mise.toml @@ -94,6 +94,13 @@ run = "node --experimental-strip-types scripts/check-coverage-thresholds-sync.ts description = "Test the standalone Jira Forge app-actor proxy" run = "npm test --prefix integrations/jira-forge-app" +[tasks."test:hooks"] +description = "Unit-test the ADR-003 governance hook scripts (#186) via node --test" +# Explicit glob (not the directory) so Node reports real pass/fail — a directory +# arg on Node 22 emits a spurious top-level failure. Runs in the build DAG so the +# governance gate's own regression tests cannot silently rot. +run = "node --test scripts/hooks/*.test.mjs" + [tasks."sync:abca-commands"] description = "Generate commands stubs from .abca/commands for multiple AI assistants" run = "node scripts/sync-abca-commands.mjs" @@ -238,6 +245,7 @@ depends = [ "//cli:build", "//docs:build", ":test:jira-forge-app", + ":test:hooks", ":drift-prevention" ] diff --git a/scripts/hooks/check-commit-msg.test.mjs b/scripts/hooks/check-commit-msg.test.mjs index d5de1b689..3499a44ef 100644 --- a/scripts/hooks/check-commit-msg.test.mjs +++ b/scripts/hooks/check-commit-msg.test.mjs @@ -91,3 +91,26 @@ test('accepts owner/repo#N cross-repo references', () => { const r = validateCommitMessage('fix: x\n\nFixes aws-samples/sample#3'); assert.equal(r.ok, true); }); + +test('accepts the GH-N reference form', () => { + // Documented accepted form (source comment): `#N`, `GH-N`, or `owner/repo#N`. + const r = validateCommitMessage('fix: x\n\nFixes GH-42'); + assert.equal(r.ok, true); +}); + +test('rejects keyword with no separator (Closes#5)', () => { + // The `\s+` between keyword and reference is deliberate — no space, no match. + assert.equal(validateCommitMessage('feat: x\n\nCloses#5').ok, false); +}); + +test('rejects a keyword embedded in a longer word (refixes)', () => { + // Word boundary (\b) prevents matching a keyword inside another token. + assert.equal(validateCommitMessage('feat: x\n\nrefixes #5').ok, false); +}); + +test('handles CRLF line endings (comment stripped, body ref survives)', () => { + // Real COMMIT_EDITMSG files may carry \r\n (Windows / core.autocrlf). + assert.equal(validateCommitMessage('feat: x\r\n\r\nCloses #7\r\n').ok, true); + // A ref that appears ONLY in a CRLF comment line must not count. + assert.equal(validateCommitMessage('feat: x\r\n\r\n# Closes #7\r\n').ok, false); +}); From ae118f574388be6f8190be1b56b54c50e3c9e942 Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:19:16 +0000 Subject: [PATCH 3/4] fix(governance): exempt merge/revert/fixup commits from commit-msg gate (#186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Tier 0 commit-msg hook rejected any message without an issue reference, including git-generated merge and revert commits. That made a routine `git merge origin/main` — which CONTRIBUTING instructs contributors to run to refresh a branch — impossible to complete. Exempt auto-generated subjects (Merge/Revert/fixup!/squash!/amend!), keyed off the subject line only so a normal commit that merely mentions "merge" in prose still requires a ref. Mirrors the default ignore set of conventional commit-msg linters. Refs #186. Co-authored-by: Claude Opus 4.8 --- scripts/hooks/check-commit-msg.mjs | 18 +++++++++++++++++ scripts/hooks/check-commit-msg.test.mjs | 26 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/scripts/hooks/check-commit-msg.mjs b/scripts/hooks/check-commit-msg.mjs index 8a0bd21fc..8c4995346 100644 --- a/scripts/hooks/check-commit-msg.mjs +++ b/scripts/hooks/check-commit-msg.mjs @@ -29,6 +29,13 @@ * commit to an issue on GitHub. A bare `#N` mention with no keyword does NOT * satisfy the gate — the reference must be intentional, not incidental prose. * + * Exempt commits (git-generated, not authored contribution work): merge + * commits (`Merge ...`), reverts (`Revert ...`), and fixup!/squash! commits. + * These carry auto-generated subjects with no issue link; requiring one would + * make a routine `git merge origin/main` — which the contribution guide tells + * contributors to run — impossible. This mirrors the default ignore set of + * conventional commit-msg linters (commitlint et al.). + * * Wiring: a `commit-msg`-type local hook in `.pre-commit-config.yaml`. prek / * pre-commit invoke commit-msg hooks with the path to the commit message file * (`.git/COMMIT_EDITMSG`) as the sole positional argument. @@ -45,6 +52,11 @@ import { fileURLToPath } from 'node:url'; const ISSUE_REF = /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?)\b\s+(?:[\w.-]+\/[\w.-]+)?(?:#|gh-)\d+/i; +// Git-generated / non-authored commits whose subject lines are auto-produced +// and carry no issue link. Matched on the first non-comment line only. +const EXEMPT_SUBJECT = + /^(?:Merge\b|Revert\b|fixup!|squash!|amend!)/i; + /** * Strip git comment lines (those beginning with `#`), which git removes before * storing the commit. A keyword that only appears in a comment must not count. @@ -64,6 +76,12 @@ function stripComments(message) { */ export function validateCommitMessage(message) { const body = stripComments(message ?? ''); + // Exempt git-generated commits (merge / revert / fixup / squash / amend), + // keyed off the first non-blank content line (the subject). + const subject = body.split('\n').find((line) => line.trim() !== '') ?? ''; + if (EXEMPT_SUBJECT.test(subject.trim())) { + return { ok: true }; + } if (ISSUE_REF.test(body)) { return { ok: true }; } diff --git a/scripts/hooks/check-commit-msg.test.mjs b/scripts/hooks/check-commit-msg.test.mjs index 3499a44ef..86c485c9f 100644 --- a/scripts/hooks/check-commit-msg.test.mjs +++ b/scripts/hooks/check-commit-msg.test.mjs @@ -108,6 +108,32 @@ test('rejects a keyword embedded in a longer word (refixes)', () => { assert.equal(validateCommitMessage('feat: x\n\nrefixes #5').ok, false); }); +test('exempts a merge commit (auto-generated subject, no issue ref)', () => { + // `git merge origin/main` — which CONTRIBUTING tells contributors to run — + // produces this subject with no issue link and must not be blocked. + const r = validateCommitMessage( + "Merge remote-tracking branch 'origin/main' into feat/186-adr003-hooks\n", + ); + assert.equal(r.ok, true); +}); + +test('exempts a revert commit', () => { + const r = validateCommitMessage('Revert "feat: something"\n\nThis reverts commit abc123.'); + assert.equal(r.ok, true); +}); + +test('exempts fixup!/squash!/amend! commits', () => { + for (const prefix of ['fixup! feat: x', 'squash! fix: y', 'amend! chore: z']) { + assert.equal(validateCommitMessage(prefix).ok, true, `expected "${prefix}" to be exempt`); + } +}); + +test('does NOT exempt a normal commit whose body merely mentions "merge"', () => { + // Exemption keys off the SUBJECT line only, not incidental "merge" in prose. + const r = validateCommitMessage('feat: teach the merge helper a trick\n\nno reference here'); + assert.equal(r.ok, false); +}); + test('handles CRLF line endings (comment stripped, body ref survives)', () => { // Real COMMIT_EDITMSG files may carry \r\n (Windows / core.autocrlf). assert.equal(validateCommitMessage('feat: x\r\n\r\nCloses #7\r\n').ok, true); From 28e9a8a3a2e233c88b514c565343437d7234a11b Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:20:40 +0000 Subject: [PATCH 4/4] docs(governance): note merge/revert commit-msg exemption in ADR-003 (#186) Document the commit-msg hook's exemption for git-generated commits so the ADR matches the implementation. Regenerated the Starlight mirror. Refs #186. Co-authored-by: Claude Opus 4.8 --- docs/decisions/ADR-003-contribution-governance.md | 2 +- .../content/docs/decisions/Adr-003-contribution-governance.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/decisions/ADR-003-contribution-governance.md b/docs/decisions/ADR-003-contribution-governance.md index b9e73e773..f9358ad64 100644 --- a/docs/decisions/ADR-003-contribution-governance.md +++ b/docs/decisions/ADR-003-contribution-governance.md @@ -105,7 +105,7 @@ Prose governance is necessary but insufficient. The following enforcement points **Branch-name exemptions:** The branch-name hook exempts `main` (the trunk is not a feature branch), `dependabot/*` (bot-authored upgrade branches), and the detached-`HEAD` sentinel. These are not enumerated elsewhere in this ADR; they are the minimal set required so the trunk and machine-generated branches are not falsely rejected. -**Commit-msg reference forms:** The Tier 0 hook accepts the GitHub closing-keyword family (`Close`/`Closes`/`Closed`, `Fix`/`Fixes`/`Fixed`, `Resolve`/`Resolves`/`Resolved`) plus `Ref`/`Refs`, each followed by `#N`, `GH-N`, or `owner/repo#N`. A bare `#N` mention with no keyword does **not** satisfy the gate — the reference must be an intentional issue link, not incidental prose. +**Commit-msg reference forms:** The Tier 0 hook accepts the GitHub closing-keyword family (`Close`/`Closes`/`Closed`, `Fix`/`Fixes`/`Fixed`, `Resolve`/`Resolves`/`Resolved`) plus `Ref`/`Refs`, each followed by `#N`, `GH-N`, or `owner/repo#N`. A bare `#N` mention with no keyword does **not** satisfy the gate — the reference must be an intentional issue link, not incidental prose. Git-generated commits (merge, revert, and `fixup!`/`squash!`/`amend!` commits) are exempt — keyed off the subject line — so a routine `git merge origin/main` is not blocked. **Progressive enforcement:** Start with the commit-msg hook (cheapest, catches all contributors). Add pre-push validation next. Skill gates enforce at the agent-workflow level (see ADR-012, proposed, for the skill model). The pre-push Tier 1 `approved`-label check (row above) is deferred — it needs a network `gh` call at push time, which the offline-capable hook set intentionally avoids; the `pickup-issue` skill covers the `approved`-label gate at the agent-workflow layer in the interim. diff --git a/docs/src/content/docs/decisions/Adr-003-contribution-governance.md b/docs/src/content/docs/decisions/Adr-003-contribution-governance.md index 4f4b4f413..4ec27051a 100644 --- a/docs/src/content/docs/decisions/Adr-003-contribution-governance.md +++ b/docs/src/content/docs/decisions/Adr-003-contribution-governance.md @@ -109,7 +109,7 @@ Prose governance is necessary but insufficient. The following enforcement points **Branch-name exemptions:** The branch-name hook exempts `main` (the trunk is not a feature branch), `dependabot/*` (bot-authored upgrade branches), and the detached-`HEAD` sentinel. These are not enumerated elsewhere in this ADR; they are the minimal set required so the trunk and machine-generated branches are not falsely rejected. -**Commit-msg reference forms:** The Tier 0 hook accepts the GitHub closing-keyword family (`Close`/`Closes`/`Closed`, `Fix`/`Fixes`/`Fixed`, `Resolve`/`Resolves`/`Resolved`) plus `Ref`/`Refs`, each followed by `#N`, `GH-N`, or `owner/repo#N`. A bare `#N` mention with no keyword does **not** satisfy the gate — the reference must be an intentional issue link, not incidental prose. +**Commit-msg reference forms:** The Tier 0 hook accepts the GitHub closing-keyword family (`Close`/`Closes`/`Closed`, `Fix`/`Fixes`/`Fixed`, `Resolve`/`Resolves`/`Resolved`) plus `Ref`/`Refs`, each followed by `#N`, `GH-N`, or `owner/repo#N`. A bare `#N` mention with no keyword does **not** satisfy the gate — the reference must be an intentional issue link, not incidental prose. Git-generated commits (merge, revert, and `fixup!`/`squash!`/`amend!` commits) are exempt — keyed off the subject line — so a routine `git merge origin/main` is not blocked. **Progressive enforcement:** Start with the commit-msg hook (cheapest, catches all contributors). Add pre-push validation next. Skill gates enforce at the agent-workflow level (see ADR-012, proposed, for the skill model). The pre-push Tier 1 `approved`-label check (row above) is deferred — it needs a network `gh` call at push time, which the offline-capable hook set intentionally avoids; the `pickup-issue` skill covers the `approved`-label gate at the agent-workflow layer in the interim.