From d67f00bf6552cd68fe8c2ef6d81ca80235980cc5 Mon Sep 17 00:00:00 2001 From: ytnobody Date: Tue, 28 Jul 2026 10:38:53 +0900 Subject: [PATCH] feat(risk): promote control-plane paths to HIGH risk (Closes #179) internal/risk/, internal/permissions/, internal/readiness/, harness.toml, .claude/, and CLAUDE.md are the mechanisms that constrain HERMIT itself. A PR that only touches one of them was previously MEDIUM (via the internal/ prefix) or unclassified by path at all, and so could auto-merge under its own judgment -- a structurally invalid question. DefaultConfig().HighPaths now includes these six control-plane paths alongside the existing cmd/, go.mod, .github/ entries, and HighPaths is checked (and returns) before MediumPaths, so a change confined to internal/risk/ is HIGH, not MEDIUM, even at one file / one line. The list intentionally names itself: internal/risk/ and harness.toml (whose [risk] section can override HighPaths) are both included, so a PR that tries to narrow this list to weaken the guard is itself HIGH. cmd/hermit/templates/{CLAUDE.md,harness.toml}.tmpl remain covered by the pre-existing ExcludePaths entry for cmd/hermit/templates/, since they are scaffold content shipped to user projects, not HERMIT's own control plane -- unaffected by this change. Added REQUIREMENTS.md REQ-015 and TestREQ015_ControlPlanePathsAreHighRisk covering all six new HIGH paths, the MEDIUM-vs-HIGH priority ordering, a non-control-plane internal/ path staying MEDIUM, and the templates/-only exclusion staying LOW. Also updated the pre-existing TestDefaultConfig_MatchesLegacyHardcodedValues assertion, since it directly encoded the old three-entry HighPaths list that this Issue deliberately expands. Updated README.md and cmd/hermit/templates/harness.toml.tmpl to document that control-plane paths are HIGH by default. Co-Authored-By: Claude Sonnet 5 --- README.md | 6 +- REQUIREMENTS.md | 13 ++++ cmd/hermit/templates/harness.toml.tmpl | 2 +- internal/risk/evaluator.go | 30 +++++++++- internal/risk/evaluator_test.go | 15 ++++- internal/risk/req_test.go | 83 ++++++++++++++++++++++++++ 6 files changed, 144 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f2edac0..d53d79f 100644 --- a/README.md +++ b/README.md @@ -193,10 +193,12 @@ No further action is needed. HERMIT handles the entire development workflow auto | Condition | Level | |---|---| -| 20+ changed files / 500+ changed lines / changes in `cmd/`, `go.mod`, `.github/` | HIGH | +| 20+ changed files / 500+ changed lines / changes in `cmd/`, `go.mod`, `.github/`, or HERMIT's own control plane (`internal/risk/`, `internal/permissions/`, `internal/readiness/`, `harness.toml`, `.claude/`, `CLAUDE.md`) | HIGH | | 10+ changed files / 200+ changed lines / changes in `internal/` | MEDIUM | | Otherwise | LOW | +A PR touching only one of the control-plane paths above is always HIGH, even a single-file, single-line change, and takes priority over the broader `internal/` MEDIUM match. This is deliberate: these paths are the mechanisms that constrain HERMIT itself (including `evaluate_risk`'s own logic and the `harness.toml` `[risk]` section that can override `high_paths`), so a change to them can never be safely self-certified as auto-mergeable — it always needs a human to look at it. The list intentionally includes itself (`internal/risk/`, `harness.toml`) so that a PR attempting to remove a path from `high_paths` is itself HIGH. + --- ## Configuration File (`harness.toml`) @@ -220,7 +222,7 @@ superintendent = "claude-sonnet-5" # model used for the Superintendent role engineer = "claude-sonnet-5" # model used for Engineer roles # [risk] -# high_paths = ["cmd/", "go.mod", ".github/"] # HIGH risk when a changed file matches one of these prefixes +# high_paths = ["cmd/", "go.mod", ".github/", "internal/risk/", "internal/permissions/", "internal/readiness/", "harness.toml", ".claude/", "CLAUDE.md"] # HIGH risk when a changed file matches one of these prefixes; defaults include HERMIT's own control-plane paths so they're always isolated from auto-merge # medium_paths = ["internal/"] # MEDIUM risk when a changed file matches one of these prefixes # high_file_threshold = 20 # HIGH risk when this many or more files changed # high_line_threshold = 500 # HIGH risk when this many or more lines changed (additions + deletions) diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index dbf2f59..c660898 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -183,3 +183,16 @@ MADFLOW の教訓 (HERMIT.md §1, §9) に基づき、HERMIT では以下を行 - 受け入れ条件: 上記に該当する実装がコードベースに追加されていないこと (レビューで担保) - verify: manual - 実装状況: 実装済み (遵守) — 現行コードベースに該当実装は存在しない。`cmd/hermit` のサブコマンドは serve / install / init / doctor / upgrade / version 等の CLI に留まり、Claude Code のプロセス管理や LLM 呼び出しは行っていない + +## REQ-015: 制御面パスの evaluate_risk 既定 HIGH 昇格 + +HERMIT 自身を制約する制御面 (`internal/risk/`・`internal/permissions/`・`internal/readiness/`・`harness.toml`・`.claude/`・`CLAUDE.md`) への変更は、変更ファイル数・行数に関わらず既定で HIGH と判定し、MEDIUM 自動マージの対象から外す。制御面パスへの変更をその制約自身の判定で自動マージしてよいかは構造的に無効な問いであるため、機械的に隔離する。 + +- `DefaultConfig()` の `HighPaths` は `cmd/`・`go.mod`・`.github/` に加え、上記 6 つの制御面パスを含む +- `HighPaths` の判定は `MediumPaths` (`internal/`) より優先される。`internal/risk/` のように `HighPaths` と `MediumPaths` の両方に前方一致する変更は HIGH と判定される +- `internal/risk/`・`harness.toml` 自身が `HighPaths` に含まれることで、`HighPaths` からこれらのパスを除外しようとする変更それ自体が HIGH 判定に掛かり、ガードを弱める PR が自動マージされない +- `cmd/hermit/templates/CLAUDE.md.tmpl` および `cmd/hermit/templates/harness.toml.tmpl` は既存の `ExcludePaths` により引き続き除外され、今回の変更で HIGH に巻き込まれない +- `internal/git/` など制御面に該当しない `internal/` 配下の変更は従来通り MEDIUM のまま +- 受け入れ条件: `internal/risk/`・`internal/permissions/`・`internal/readiness/`・`harness.toml`・`.claude/`・`CLAUDE.md` のいずれかのみを 1 ファイル 1 行変更しても HIGH と判定されること。制御面以外の `internal/` 配下の変更や `cmd/hermit/templates/` 配下のみの変更は従来通りの判定 (MEDIUM・LOW) を維持すること +- verify: test +- 実装状況: 実装済み — `internal/risk/evaluator.go` の `DefaultConfig()`。`internal/risk/req_test.go` の `TestREQ015_ControlPlanePathsAreHighRisk` で検証 diff --git a/cmd/hermit/templates/harness.toml.tmpl b/cmd/hermit/templates/harness.toml.tmpl index f2e9d9d..38987eb 100644 --- a/cmd/hermit/templates/harness.toml.tmpl +++ b/cmd/hermit/templates/harness.toml.tmpl @@ -27,7 +27,7 @@ analyst_effort = "{{ .AnalystEffort }}" # reasoning effort: low/medium/ # branch_prefix = "hermit/your-login" # defaults to hermit/ if omitted # [risk] -# high_paths = ["cmd/", "go.mod", ".github/"] # HIGH risk when a changed file matches one of these prefixes (see exclude_paths below for exceptions) +# high_paths = ["cmd/", "go.mod", ".github/", "internal/risk/", "internal/permissions/", "internal/readiness/", "harness.toml", ".claude/", "CLAUDE.md"] # HIGH risk when a changed file matches one of these prefixes (see exclude_paths below for exceptions). The internal/risk/, internal/permissions/, internal/readiness/, harness.toml, .claude/, and CLAUDE.md entries are HERMIT's own control-plane surfaces: by default a change confined to one of them is always HIGH (never auto-merged), regardless of file/line count, because a change to the mechanism that classifies risk (or the config that can override it) can't be safely self-certified. Override only if you understand you are removing that isolation. # medium_paths = ["internal/"] # MEDIUM risk when a changed file matches one of these prefixes (see exclude_paths below for exceptions) # exclude_paths = ["cmd/hermit/templates/"] # never a path-based HIGH/MEDIUM signal even if also matched above (scaffold/template content, not program logic). *_test.go files are always excluded from path matching regardless of this list, since adding tests should not raise risk. # high_file_threshold = 20 # HIGH risk when this many or more files changed diff --git a/internal/risk/evaluator.go b/internal/risk/evaluator.go index 1c4ee19..3f398a4 100644 --- a/internal/risk/evaluator.go +++ b/internal/risk/evaluator.go @@ -56,7 +56,35 @@ type Config struct { // harness.toml, preserving backward compatibility for existing projects. func DefaultConfig() Config { return Config{ - HighPaths: []string{"cmd/", "go.mod", ".github/"}, + // HighPaths includes HERMIT's own control-plane surfaces (the risk + // evaluator itself, permissions, readiness, harness.toml, .claude/, + // and CLAUDE.md) alongside the original cmd/go.mod/.github entries. + // Evaluate/EvaluateWithConfig check HighPaths before MediumPaths and + // return as soon as HighPaths matches (see EvaluateWithConfig below), + // so these control-plane prefixes take priority over the broader + // "internal/" MEDIUM match below -- a change confined to + // internal/risk/, for example, is HIGH, not MEDIUM. + // + // Crucially, this list names itself: internal/risk/ (the file you are + // reading right now) and harness.toml (which can override HighPaths + // via its [risk] section, see Merge above) are both included. That + // means a PR that tries to narrow this list -- e.g. to remove + // harness.toml or internal/risk/ from HighPaths so a future change + // there can auto-merge -- is itself a change to internal/risk/ and/or + // harness.toml, so it always trips this same HIGH classification. + // Do not remove internal/risk/ or harness.toml from this list; doing + // so would let a PR silently weaken its own guard. + HighPaths: []string{ + "cmd/", + "go.mod", + ".github/", + "internal/risk/", + "internal/permissions/", + "internal/readiness/", + "harness.toml", + ".claude/", + "CLAUDE.md", + }, MediumPaths: []string{"internal/"}, // cmd/hermit/templates/ holds scaffold/doc content (CLAUDE.md.tmpl, // harness.toml.tmpl, command markdown copied into user projects) that diff --git a/internal/risk/evaluator_test.go b/internal/risk/evaluator_test.go index bd81287..9514e48 100644 --- a/internal/risk/evaluator_test.go +++ b/internal/risk/evaluator_test.go @@ -292,7 +292,20 @@ func TestDefaultConfig_MatchesLegacyHardcodedValues(t *testing.T) { if cfg.MediumFileThreshold != 10 || cfg.MediumLineThreshold != 200 { t.Errorf("unexpected medium thresholds: %+v", cfg) } - wantHighPaths := []string{"cmd/", "go.mod", ".github/"} + // Issue #179: HighPaths was extended beyond the original cmd/go.mod/.github + // trio to also cover HERMIT's own control-plane surfaces, so they're + // always HIGH (never auto-merged) regardless of diff size. See REQ-015. + wantHighPaths := []string{ + "cmd/", + "go.mod", + ".github/", + "internal/risk/", + "internal/permissions/", + "internal/readiness/", + "harness.toml", + ".claude/", + "CLAUDE.md", + } if len(cfg.HighPaths) != len(wantHighPaths) { t.Fatalf("unexpected high paths: %+v", cfg.HighPaths) } diff --git a/internal/risk/req_test.go b/internal/risk/req_test.go index f387f2f..2869b53 100644 --- a/internal/risk/req_test.go +++ b/internal/risk/req_test.go @@ -52,3 +52,86 @@ func TestREQ006_RiskLevelThresholds(t *testing.T) { }) } } + +// TestREQ015_ControlPlanePathsAreHighRisk verifies REQ-015: changes confined +// to HERMIT's own control-plane surfaces (internal/risk/, +// internal/permissions/, internal/readiness/, harness.toml, .claude/, +// CLAUDE.md) are classified HIGH by DefaultConfig()'s HighPaths, even when +// the diff is a single file / single line, and even though internal/ as a +// whole is a MediumPaths prefix. It also verifies that non-control-plane +// internal/ changes remain MEDIUM, and that the pre-existing +// cmd/hermit/templates/ ExcludePaths behavior for CLAUDE.md.tmpl / +// harness.toml.tmpl is unaffected. +func TestREQ015_ControlPlanePathsAreHighRisk(t *testing.T) { + tests := []struct { + name string + files []gh.PRFile + additions int + deletions int + want Level + }{ + { + name: "HIGH: internal/risk/ single-line change", + files: []gh.PRFile{{Filename: "internal/risk/evaluator.go"}}, + additions: 1, deletions: 0, + want: High, + }, + { + name: "HIGH: internal/permissions/ single-line change", + files: []gh.PRFile{{Filename: "internal/permissions/permissions.go"}}, + additions: 1, deletions: 0, + want: High, + }, + { + name: "HIGH: internal/readiness/ single-line change", + files: []gh.PRFile{{Filename: "internal/readiness/readiness.go"}}, + additions: 1, deletions: 0, + want: High, + }, + { + name: "HIGH: harness.toml only", + files: []gh.PRFile{{Filename: "harness.toml"}}, + additions: 1, deletions: 0, + want: High, + }, + { + name: "HIGH: .claude/settings.json only", + files: []gh.PRFile{{Filename: ".claude/settings.json"}}, + additions: 1, deletions: 0, + want: High, + }, + { + name: "HIGH: CLAUDE.md only", + files: []gh.PRFile{{Filename: "CLAUDE.md"}}, + additions: 1, deletions: 0, + want: High, + }, + { + name: "MEDIUM: non-control-plane internal/ path stays MEDIUM", + files: []gh.PRFile{{Filename: "internal/git/worktree.go"}}, + additions: 5, deletions: 0, + want: Medium, + }, + { + name: "LOW: cmd/hermit/templates/ scaffold-only diff is not swept into HIGH", + files: []gh.PRFile{ + {Filename: "cmd/hermit/templates/CLAUDE.md.tmpl"}, + {Filename: "cmd/hermit/templates/harness.toml.tmpl"}, + }, + additions: 20, deletions: 5, + want: Low, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + level, reasons := Evaluate(tc.files, tc.additions, tc.deletions) + if level != tc.want { + t.Errorf("Evaluate() level = %v, want %v (reasons: %v)", level, tc.want, reasons) + } + if tc.want != Low && len(reasons) == 0 { + t.Errorf("expected non-empty reasons for %v level", tc.want) + } + }) + } +}