From 4e1c4492819f46e91a88ed84a1dc1712bad1538d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 5 Sep 2026 18:35:09 -0400 Subject: [PATCH] enh(#3811): add hooks.commit_types config surface to gsd-validate-commit (#4340) * enh(#3811): add hooks.commit_types config surface to gsd-validate-commit Extends the opt-in Conventional Commits hook with a hooks.commit_types config array that adds project-specific types to the 10 built-ins without replacing them. Configured values pass a safe-token filter before reaching the compiled regex, so a config entry can never alter the pattern's structure. The regex alternation, the human-readable error text, and a new typed valid_types JSON field all derive from one list instead of the two hand-synced copies this replaces. Co-Authored-By: Claude Sonnet 5 * chore(#3811): backfill changeset PR number Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- .changeset/jolly-ravens-parade.md | 5 ++ docs/COMMANDS.md | 8 ++ docs/FEATURES.md | 1 + docs/features/community-hooks-opt-in.md | 1 + hooks/gsd-validate-commit.sh | 70 +++++++++++++-- tests/hooks-opt-in.test.cjs | 108 +++++++++++++++++++++++- 6 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 .changeset/jolly-ravens-parade.md diff --git a/.changeset/jolly-ravens-parade.md b/.changeset/jolly-ravens-parade.md new file mode 100644 index 000000000..23c5cfb5c --- /dev/null +++ b/.changeset/jolly-ravens-parade.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 4340 +--- +**`hooks.commit_types` config surface for `gsd-validate-commit.sh`** — projects using `hooks.community: true` can now extend the Conventional Commits type allowlist with a `hooks.commit_types` array in `.planning/config.json` (e.g. `["enhance", "enh", "revert"]`), added to rather than replacing the 10 built-in types. Configured values are validated against a safe-token pattern and the regex/error text now derive from a single source of truth. (#3811) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 51db00b50..4f4f41b5b 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -2349,6 +2349,14 @@ Enable with: { "hooks": { "community": true } } ``` +`gsd-validate-commit.sh` accepts the 10 Conventional Commits types (`feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`) by default. Extend the list with `hooks.commit_types` — an array of extra type names, added to (never replacing) the built-in ten: + +```json +{ "hooks": { "community": true, "commit_types": ["enhance", "enh", "revert"] } } +``` + +Each entry must match `^[a-z][a-z0-9-]*$` (lowercase letters, digits, hyphens); non-conforming or non-string entries are dropped rather than blocking the hook. + --- ### Community Invite diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 01c37004d..e8fca97fc 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -2191,6 +2191,7 @@ Test suite that scans all agent, workflow, and command files for embedded inject | Setting | Type | Default | Description | |---------|------|---------|-------------| | `hooks.community` | boolean | `false` | Enable optional community hooks for commit validation, session state, and phase boundaries | +| `hooks.commit_types` | array of strings | `[]` | Extra Conventional Commits types `gsd-validate-commit.sh` accepts, in addition to the built-in `feat, fix, docs, style, refactor, perf, test, build, ci, chore` — never replaces them. Each entry must match `^[a-z][a-z0-9-]*$` (lowercase letters, digits, hyphens); non-conforming or non-string entries are dropped. Example: `{ "hooks": { "community": true, "commit_types": ["enhance", "enh", "revert"] } }`. | --- diff --git a/docs/features/community-hooks-opt-in.md b/docs/features/community-hooks-opt-in.md index da4b1a4f7..2f7bc2cbd 100644 --- a/docs/features/community-hooks-opt-in.md +++ b/docs/features/community-hooks-opt-in.md @@ -18,3 +18,4 @@ group: v1.32 Features | Setting | Type | Default | Description | |---------|------|---------|-------------| | `hooks.community` | boolean | `false` | Enable optional community hooks for commit validation, session state, and phase boundaries | +| `hooks.commit_types` | array of strings | `[]` | Extra Conventional Commits types `gsd-validate-commit.sh` accepts, in addition to the built-in `feat, fix, docs, style, refactor, perf, test, build, ci, chore` — never replaces them. Each entry must match `^[a-z][a-z0-9-]*$` (lowercase letters, digits, hyphens); non-conforming or non-string entries are dropped. Example: `{ "hooks": { "community": true, "commit_types": ["enhance", "enh", "revert"] } }`. | diff --git a/hooks/gsd-validate-commit.sh b/hooks/gsd-validate-commit.sh index ab82f8ce2..a6e5adf88 100755 --- a/hooks/gsd-validate-commit.sh +++ b/hooks/gsd-validate-commit.sh @@ -24,13 +24,38 @@ cleanup_temp_files() { } trap cleanup_temp_files EXIT +# The 10 built-in Conventional Commits types — the SINGLE declaration (#3811 +# review finding: this was previously hand-typed a second time inside the +# node -e script below, a generative-fix-divergence risk per CLAUDE.md's +# known-defect list). Threaded into node via an env var; reused directly by +# bash below when building COMMIT_TYPES. +BUILTIN_COMMIT_TYPES=(feat fix docs style refactor perf test build ci chore) + # Check opt-in config — exit silently if not enabled if [ -f .planning/config.json ]; then ENABLED_ERR=$(mktemp) - ENABLED=$(node -e " + # Single node invocation reads BOTH hooks.community (line 1: '1'/'0') and + # hooks.commit_types (remaining lines: one sanitized extra type per line) — + # see #3811. Sanitizing here, not in bash, keeps the safe-token check in one + # place and guarantees only [a-z][a-z0-9-]* strings ever reach the regex + # built below, so a configured value can never alter the compiled pattern's + # structure. + BUILTIN_COMMIT_TYPES_CSV=$(IFS=,; echo "${BUILTIN_COMMIT_TYPES[*]}") + CONFIG_OUT=$(GSD_BUILTIN_COMMIT_TYPES="$BUILTIN_COMMIT_TYPES_CSV" node -e " try{ const c=require('./.planning/config.json'); process.stdout.write(c.hooks?.community===true?'1':'0'); + process.stdout.write('\n'); + const raw=c.hooks?.commit_types; + const list=Array.isArray(raw)?raw:[]; + const seen=new Set((process.env.GSD_BUILTIN_COMMIT_TYPES||'').split(',').filter(Boolean)); + for (const t of list){ + if (typeof t!=='string') continue; + if (!/^[a-z][a-z0-9-]*\$/.test(t)) continue; + if (seen.has(t)) continue; + seen.add(t); + process.stdout.write(t+'\n'); + } }catch(e){ process.stderr.write('CONFIG_READ_FAILED: '+(e&&e.message?e.message:String(e))); process.exit(3); @@ -44,7 +69,16 @@ if [ -f .planning/config.json ]; then echo "gsd-validate-commit.sh: could not read .planning/config.json (opt-in check) — validator disabled for this call. $(cat "$ENABLED_ERR")" >&2 exit 0 fi + ENABLED=$(printf '%s\n' "$CONFIG_OUT" | head -1) if [ "$ENABLED" != "1" ]; then exit 0; fi + # Remaining lines (if any) are the sanitized, deduped configured commit + # types beyond the 10 built-ins (#3811). Read into a bash-3.2-safe array — + # `mapfile`/`readarray` are bash 4+ only and this hook is tested against + # bash 3.2.57 (macOS default). + EXTRA_COMMIT_TYPES=() + while IFS= read -r _extra_type; do + [ -n "$_extra_type" ] && EXTRA_COMMIT_TYPES+=("$_extra_type") + done < <(printf '%s\n' "$CONFIG_OUT" | tail -n +2) else exit 0 fi @@ -491,11 +525,37 @@ if [ "$CLASSIFY_STATUS" = "0" ]; then else SUBJECT=$(echo "$MSG" | head -1) fi + # Single source of truth for the accepted commit-type list (#3811): the + # 10 built-ins plus whatever passed the safe-token filter above. Both the + # regex alternation and the human-readable error text below are derived + # from this ONE array — no hand-synced second copy. + # + # The `"${EXTRA_COMMIT_TYPES[@]+"${EXTRA_COMMIT_TYPES[@]}"}"` form (not + # plain `"${EXTRA_COMMIT_TYPES[@]}"`) is required: on bash 3.2.57 (this + # repo's macOS test target), expanding `[@]` on an array that is declared + # but has zero elements throws "unbound variable" under `set -u` (which + # this script has via `set -euo pipefail`). Verified directly against + # /bin/bash 3.2.57 on macOS. The `${arr[@]+word}` form is the + # nounset-safe idiom for "expand if set, empty otherwise" on empty arrays. + COMMIT_TYPES=("${BUILTIN_COMMIT_TYPES[@]}" "${EXTRA_COMMIT_TYPES[@]+"${EXTRA_COMMIT_TYPES[@]}"}") + COMMIT_TYPE_ALT=$(IFS='|'; echo "${COMMIT_TYPES[*]}") + COMMIT_TYPE_LIST=$(printf '%s, ' "${COMMIT_TYPES[@]}") + COMMIT_TYPE_LIST="${COMMIT_TYPE_LIST%, }" + # Typed `valid_types` array (#3811 review finding): CONTRIBUTING.md bans + # substring/prose matching on `reason` in tests — a test needing to + # verify the accepted-type set must have a typed field, not grep prose. + # Safe to build with a bare printf (no JSON-escaping needed): every + # element of COMMIT_TYPES has already passed the `^[a-z][a-z0-9-]*$` + # safe-token filter (or is a literal built-in), so none can contain `"` + # or `\`. + COMMIT_TYPES_JSON=$(printf '"%s",' "${COMMIT_TYPES[@]}") + COMMIT_TYPES_JSON="[${COMMIT_TYPES_JSON%,}]" # Validate Conventional Commits format - if ! [[ "$SUBJECT" =~ ^(feat|fix|docs|style|refactor|perf|test|build|ci|chore)(\(.+\))?:[[:space:]].+ ]]; then - # Emit a typed `code` field alongside `reason` (#2974). Tests assert - # on the stable code string; the reason is the human-readable copy. - echo '{"decision": "block", "code": "CONVENTIONAL_COMMITS_VIOLATION", "reason": "Commit message must follow Conventional Commits: (): . Valid types: feat, fix, docs, style, refactor, perf, test, build, ci, chore. Subject must be <=72 chars, lowercase, imperative mood, no trailing period."}' + if ! [[ "$SUBJECT" =~ ^($COMMIT_TYPE_ALT)(\(.+\))?:[[:space:]].+ ]]; then + # Emit typed `code` and `valid_types` fields alongside `reason` (#2974, + # #3811). Tests assert on the stable code string and the typed array; + # the reason is the human-readable copy, never grepped by tests. + echo "{\"decision\": \"block\", \"code\": \"CONVENTIONAL_COMMITS_VIOLATION\", \"valid_types\": $COMMIT_TYPES_JSON, \"reason\": \"Commit message must follow Conventional Commits: (): . Valid types: $COMMIT_TYPE_LIST. Subject must be <=72 chars, lowercase, imperative mood, no trailing period.\"}" exit 2 fi if [ ${#SUBJECT} -gt 72 ]; then diff --git a/tests/hooks-opt-in.test.cjs b/tests/hooks-opt-in.test.cjs index 298500f50..3c586b2aa 100644 --- a/tests/hooks-opt-in.test.cjs +++ b/tests/hooks-opt-in.test.cjs @@ -67,12 +67,14 @@ function cleanup(tmpDir) { try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch {} } -function writeConfigWithHooks(tmpDir, enabled) { +function writeConfigWithHooks(tmpDir, enabled, commitTypes) { + const hooks = { community: enabled }; + if (commitTypes !== undefined) hooks.commit_types = commitTypes; fs.writeFileSync( path.join(tmpDir, '.planning', 'config.json'), JSON.stringify({ model_profile: 'balanced', - hooks: { community: enabled } + hooks }, null, 2) ); } @@ -367,6 +369,108 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION'); }); + // ───────────────────────────────────────────────────────────────────────── + // #3811 — hooks.commit_types config surface (extends, never replaces, the + // 10 built-in Conventional Commits types) + // ───────────────────────────────────────────────────────────────────────── + + describe('hooks.commit_types config surface (#3811)', () => { + test('blocks a non-built-in type when commit_types is absent (default list unchanged)', () => { + writeConfigWithHooks(tmpDir, true); + const result = runHookCmd('git commit -m "enhance(core): x"'); + assert.strictEqual(result.status, 2, `expected block, got ${result.status}`); + assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION'); + }); + + test('treats an empty commit_types array as the default list', () => { + writeConfigWithHooks(tmpDir, true, []); + const result = runHookCmd('git commit -m "enhance(core): x"'); + assert.strictEqual(result.status, 2, `expected block, got ${result.status}`); + assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION'); + }); + + test('accepts a single configured type', () => { + writeConfigWithHooks(tmpDir, true, ['enhance']); + const result = runHookCmd('git commit -m "enhance(core): x"'); + assert.strictEqual(result.status, 0, `expected pass, got ${result.status}. stderr: ${result.stderr}`); + }); + + test('still rejects an unconfigured type when only one type is configured', () => { + writeConfigWithHooks(tmpDir, true, ['enhance']); + const result = runHookCmd('git commit -m "enh(core): x"'); + assert.strictEqual(result.status, 2, `extension must not become a free-for-all, got ${result.status}`); + assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION'); + }); + + test('accepts every type in a many-entry commit_types array', () => { + writeConfigWithHooks(tmpDir, true, ['enhance', 'enh', 'revert']); + for (const type of ['enhance', 'enh', 'revert']) { + const result = runHookCmd(`git commit -m "${type}(core): x"`); + assert.strictEqual(result.status, 0, `expected pass for type "${type}", got ${result.status}. stderr: ${result.stderr}`); + } + }); + + test('dedupes a configured type that duplicates a built-in', () => { + writeConfigWithHooks(tmpDir, true, ['feat']); + const okResult = runHookCmd('git commit -m "feat(core): x"'); + assert.strictEqual(okResult.status, 0, `expected pass, got ${okResult.status}`); + const blockResult = runHookCmd('git commit -m "WIP save"'); + assert.strictEqual(blockResult.status, 2); + const { valid_types: validTypes } = JSON.parse(blockResult.stdout); + assert.deepStrictEqual( + validTypes, + ['feat', 'fix', 'docs', 'style', 'refactor', 'perf', 'test', 'build', 'ci', 'chore'], + `configuring a duplicate of a built-in must not add a second "feat", got: ${JSON.stringify(validTypes)}` + ); + }); + + test('dedupes a configured type repeated in its own array', () => { + writeConfigWithHooks(tmpDir, true, ['enhance', 'enhance']); + const blockResult = runHookCmd('git commit -m "WIP save"'); + assert.strictEqual(blockResult.status, 2); + const { valid_types: validTypes } = JSON.parse(blockResult.stdout); + assert.strictEqual( + validTypes.filter((t) => t === 'enhance').length, + 1, + `"enhance" configured twice must still appear once, got: ${JSON.stringify(validTypes)}` + ); + assert.strictEqual(new Set(validTypes).size, validTypes.length, `valid_types must have no duplicates: ${JSON.stringify(validTypes)}`); + }); + + test('drops non-string commit_types entries and keeps the valid ones', () => { + writeConfigWithHooks(tmpDir, true, [123, null, 'enhance', {}]); + const result = runHookCmd('git commit -m "enhance(core): x"'); + assert.strictEqual(result.status, 0, `expected pass, got ${result.status}. stderr: ${result.stderr}`); + }); + + test('rejects unsafe commit_types entries without corrupting the built-in regex', () => { + writeConfigWithHooks(tmpDir, true, ['feat|rm -rf /', 'a)(b', '.*', 'UPPER', '']); + // Built-ins must still work — a bad entry must not corrupt the compiled regex. + const builtinResult = runHookCmd('git commit -m "feat(core): x"'); + assert.strictEqual(builtinResult.status, 0, `built-in type must still pass, got ${builtinResult.status}. stderr: ${builtinResult.stderr}`); + // None of the unsafe entries should have been accepted as a type. + const rejectedResult = runHookCmd('git commit -m "UPPER(core): x"'); + assert.strictEqual(rejectedResult.status, 2, `unsafe entry must not be accepted as a type, got ${rejectedResult.status}`); + }); + + test('ignores a non-array commit_types value', () => { + writeConfigWithHooks(tmpDir, true, 'enhance'); + const result = runHookCmd('git commit -m "enhance(core): x"'); + assert.strictEqual(result.status, 2, `non-array commit_types must be ignored (treated as absent), got ${result.status}`); + }); + + test('valid_types includes configured types alongside the built-ins', () => { + writeConfigWithHooks(tmpDir, true, ['enhance']); + const result = runHookCmd('git commit -m "WIP save"'); + assert.strictEqual(result.status, 2); + const { valid_types: validTypes } = JSON.parse(result.stdout); + assert.deepStrictEqual( + validTypes, + ['feat', 'fix', 'docs', 'style', 'refactor', 'perf', 'test', 'build', 'ci', 'chore', 'enhance'], + `expected built-ins plus "enhance", got: ${JSON.stringify(validTypes)}` + ); + }); + }); // ─── #3816 round 8 (Major): bracket classes must not smuggle a literal `\` ─── //