diff --git a/README.md b/README.md index 548bbdf..fb47796 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,9 @@ pass. **The floor is 86%** unless the workspace records another (`--min`). The commands live in `/scc-manifest.json` — no second config file. They run where they are worth their cost: a bare `scc validate` never touches them; the -`pre-push` hook does, which is where a branch becomes a pull request. Writing them is +`pre-push` hook does, which is where a branch becomes a pull request — and +`SCC_SKIP_CHECKS=test` (or `=1`) passes over a gate this machine cannot run for one +push, leaving every other check in place. Writing them is the agent's job — `/scc-init` derives them from the project's language, and every "no command recorded" finding says what to record *and* how to decline it. diff --git a/docs/wiki/changelog.md b/docs/wiki/changelog.md index fbdd8f2..881828e 100644 --- a/docs/wiki/changelog.md +++ b/docs/wiki/changelog.md @@ -7,6 +7,8 @@ thing it describes moved. A page named here that no longer exists is reported, so a rename is recorded as a change rather than left pointing at the old slug. +- 2026-10-18 — [[hooks]]: `SCC_SKIP_CHECKS`, the narrow escape past a delivery-gate + command this machine cannot run, beside `SCC_SKIP_HOOKS`. - 2026-09-19 — `CLAUDE.md` went back to being the entry file the binary ships, so what it carried moved to the one place that owns it: extended [[delivery-gate]] (the grouped `check` object, the distinct findings), [[integration-boundary]] (the diff --git a/docs/wiki/pages/hooks.md b/docs/wiki/pages/hooks.md index 487227e..578bd2f 100644 --- a/docs/wiki/pages/hooks.md +++ b/docs/wiki/pages/hooks.md @@ -32,6 +32,15 @@ reports success it did not verify — and they name the way past themselves (`SCC_SKIP_HOOKS=1`), because a gate with no documented escape is a gate people delete the first time it is wrong. +That escape is too wide for the commonest reason to want one: a suite that cannot +run on this machine — a database it does not have — while CI runs it fine. Skipping +the whole hook to get past one gate also drops the artifact, attribution and +pull-request checks. **`SCC_SKIP_CHECKS` is the narrow one**: at pre-push, +`SCC_SKIP_CHECKS=test` (a comma list, or `1` for all four) passes over only those +[[delivery-gate]] commands, keeps every validator that reads a file, and says on +stderr which gates it did not run — a push past the suite must not read as verified. +A misspelled gate is exit `1`, not a skip that silently did not happen. + Where the hooks go is **asked of git** (`core.hooksPath`, then `rev-parse --git-path hooks`) rather than assumed to be `.git/hooks`: writing a hook where git will not read it is the kind of failure that looks like success for weeks. diff --git a/internal/cli/hooks.go b/internal/cli/hooks.go index dfa0420..3a772d3 100644 --- a/internal/cli/hooks.go +++ b/internal/cli/hooks.go @@ -78,7 +78,9 @@ The git hooks need a git repository; outside a workspace the artifact validators have nothing to read and the checks on the record still run. A hook scc did not write is reported and left alone; --force appends to it, and only when it is a POSIX sh script. Skip every scc git hook for one command with %s=1, or use git's ---no-verify where that applies. +--no-verify where that applies. To push past only the build, format, lint or test +gate this machine cannot run, %s=[,] (or =1 for all four) skips +those gates and keeps every other check. The harness stages are entries in the harness's own settings file, spliced in beside whatever else is there and removed the same way. Only a harness with a @@ -87,7 +89,7 @@ never block a turn, and they never push or open a pull request: delivery is an act the agent takes in the open, where you can stop it. `, prog(), prog(), prog(), prog(), prog(), hooks.PreCommit.Why(), hooks.CommitMsg.Why(), hooks.PrePush.Why(), - hooks.SessionStart.Why(), hooks.UserPromptSubmit.Why(), hooks.Stop.Why(), hooks.SkipEnv) + hooks.SessionStart.Why(), hooks.UserPromptSubmit.Why(), hooks.Stop.Why(), hooks.SkipEnv, hooks.SkipChecksEnv) } // hooksReport is the frozen JSON shape for install, check and remove: one entry @@ -335,7 +337,12 @@ func runHooksRun(args []string) int { // on the command somebody reaches for to tidy up. return ExitOK } - return runGate(target, *jsonOut, true) + skip, err := skippedGates(os.Getenv(hooks.SkipChecksEnv)) + if err != nil { + render.Err(fmt.Sprintf("%s: %v", hooks.SkipChecksEnv, err)) + return ExitError + } + return runGate(target, *jsonOut, true, skip...) case hooks.StageSessionStart, hooks.StageUserPrompt, hooks.StageStop: // A different protocol downstream of the same command: the harness hands // the event in on stdin and reads a document back, where git reads an exit @@ -393,10 +400,45 @@ func pushingAnything(in io.Reader) bool { return false } +// skippedGates reads SCC_SKIP_CHECKS: empty is none, `1` or `all` is the four, and +// anything else is a comma list of gate names. An unknown name is an error rather +// than ignored, because a push that meant to skip `tests` and ran the suite anyway +// fails for the reason it was trying to get past, and says nothing about why. +func skippedGates(v string) ([]gate.Kind, error) { + v = strings.TrimSpace(v) + switch strings.ToLower(v) { + case "": + return nil, nil + case "1", "all", "true": + return gate.Kinds(), nil + } + var out []gate.Kind + for _, name := range strings.Split(v, ",") { + if strings.TrimSpace(name) == "" { + continue + } + k, err := gate.ParseKind(name) + if err != nil { + return nil, err + } + out = append(out, k) + } + return out, nil +} + // runGate is the pre-commit and pre-push check: the same run `scc validate` does, -// reported the same way, exiting on the same contract. -func runGate(root string, jsonOut, pr bool) int { - set, err := gateFindings(root, pr) +// reported the same way, exiting on the same contract. The gates in skip are not +// run, and the run says so: a push past the suite must not read as a verified one. +func runGate(root string, jsonOut, pr bool, skip ...gate.Kind) int { + if len(skip) > 0 { + names := make([]string, len(skip)) + for i, k := range skip { + names[i] = string(k) + } + render.Warn(fmt.Sprintf("%s: not running the %s gate(s) on this push — CI has to", + hooks.SkipChecksEnv, strings.Join(names, ", "))) + } + set, err := gateFindings(root, pr, validate.WithoutGates(skip...)) if err != nil { render.Err(fmt.Sprintf("validate: %v", err)) return ExitError @@ -410,10 +452,25 @@ func runGate(root string, jsonOut, pr bool) int { set.Report("validate") if !set.Empty() { render.Detail(fmt.Sprintf(" fix these, or pass %s=1 for this one command", hooks.SkipEnv)) + if gateFailed(set) { + render.Detail(fmt.Sprintf(" or, for a gate this machine cannot run, %s= skips only that gate (=1 all four)", + hooks.SkipChecksEnv)) + } } return set.ExitCode() } +// gateFailed reports whether any finding came from the delivery gate's commands, +// the only ones SCC_SKIP_CHECKS reaches. +func gateFailed(set *finding.Set) bool { + for _, f := range set.Sorted() { + if strings.HasPrefix(f.Rule, "check.") || strings.HasPrefix(f.Rule, "tests.") { + return true + } + } + return false +} + // gateFindings runs everything that applies here: every validator in a workspace, // and the checks on the record alone outside one. // diff --git a/internal/cli/hooks_test.go b/internal/cli/hooks_test.go index 837a965..27c8149 100644 --- a/internal/cli/hooks_test.go +++ b/internal/cli/hooks_test.go @@ -5,9 +5,11 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "testing" + "github.com/protonspy/spec-claude-code/internal/gate" "github.com/protonspy/spec-claude-code/internal/hooks" ) @@ -303,6 +305,45 @@ func TestPrePushRunsTheSuiteAndPreCommitDoesNot(t *testing.T) { } } +// SCC_SKIP_CHECKS is the narrow way past a gate this machine cannot run: the suite +// is not run, the push says so, and a misspelled gate is an error rather than a +// skip that silently did not happen. +func TestPrePushSkipsOnlyTheGatesNamed(t *testing.T) { + root := gitWorkspace(t) + skipRest(t, root, "test") + marker := filepath.Join(root, "ran.txt") + setTest(t, root, "echo ran > "+filepath.ToSlash(marker)) + + // Unset, the failing suite blocks the push and the hint names the narrow way out. + _, stderr, code := run(t, "hooks", "run", "pre-push", "--root", root) + if code != ExitFindings { + t.Fatalf("pre-push: exit = %d, want %d (stderr: %s)", code, ExitFindings, stderr) + } + if !strings.Contains(stderr, hooks.SkipChecksEnv) { + t.Errorf("a failed gate did not offer %s: %q", hooks.SkipChecksEnv, stderr) + } + if err := os.Remove(marker); err != nil { + t.Fatalf("remove marker: %v", err) + } + + t.Setenv(hooks.SkipChecksEnv, "test") + _, stderr, code = run(t, "hooks", "run", "pre-push", "--root", root) + if code != ExitOK { + t.Fatalf("pre-push with %s=test: exit = %d, want %d (stderr: %s)", hooks.SkipChecksEnv, code, ExitOK, stderr) + } + if _, err := os.Stat(marker); err == nil { + t.Error("the pre-push hook ran a suite it was told to skip") + } + if !strings.Contains(stderr, "test gate") { + t.Errorf("the push did not say which gate it skipped: %q", stderr) + } + + t.Setenv(hooks.SkipChecksEnv, "tests") + if _, stderr, code = run(t, "hooks", "run", "pre-push", "--root", root); code != ExitError { + t.Errorf("an unknown gate name: exit = %d, want %d (stderr: %s)", code, ExitError, stderr) + } +} + // A workspace that never wired its tests up pushes exactly as it did before. The // gate is a default here rather than something asked for by name, and a default // that blocked every push over configuration nobody chose would be scc deciding @@ -372,3 +413,29 @@ func TestTheGateNarrowsOutsideAWorkspace(t *testing.T) { t.Errorf("the document reports no findings though the command exited %d", ExitFindings) } } + +// `1` and `all` are the four, a comma list is those, blanks are nothing, and a name +// that is not a gate is refused. +func TestSkippedGatesReadsTheEnvironmentValue(t *testing.T) { + for _, tc := range []struct { + in string + want []gate.Kind + bad bool + }{ + {in: "", want: nil}, + {in: "1", want: gate.Kinds()}, + {in: " ALL ", want: gate.Kinds()}, + {in: "test", want: []gate.Kind{gate.Test}}, + {in: "lint, Test,", want: []gate.Kind{gate.Lint, gate.Test}}, + {in: "tests", bad: true}, + } { + got, err := skippedGates(tc.in) + if (err != nil) != tc.bad { + t.Errorf("skippedGates(%q): err = %v, want error %v", tc.in, err, tc.bad) + continue + } + if !slices.Equal(got, tc.want) { + t.Errorf("skippedGates(%q) = %v, want %v", tc.in, got, tc.want) + } + } +} diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index 31f9b9c..dd08732 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -147,6 +147,12 @@ const Prog = "scc" // is a gate people remove instead of skipping. const SkipEnv = "SCC_SKIP_HOOKS" +// SkipChecksEnv passes over the delivery gate's commands at pre-push for one push: +// `1` or `all` for the four, or a comma list of gate names. It is the narrow way +// past a gate this machine cannot run — a suite needing a database the checkout +// does not have — where SkipEnv would also drop every validator that reads a file. +const SkipChecksEnv = "SCC_SKIP_CHECKS" + // State is what a hook file is, from scc's point of view. type State string diff --git a/internal/validate/all.go b/internal/validate/all.go index 52d558e..dec9542 100644 --- a/internal/validate/all.go +++ b/internal/validate/all.go @@ -5,6 +5,7 @@ import ( "time" "github.com/protonspy/spec-claude-code/internal/finding" + "github.com/protonspy/spec-claude-code/internal/gate" ) // Validator is one named check over a workspace. @@ -57,6 +58,7 @@ type options struct { pr bool checks bool timeout time.Duration + skipGates []gate.Kind noAttribution bool } @@ -88,6 +90,14 @@ func WithChecks() Option { return func(o *options) { o.checks = true } } // It adds no check of its own, so it is not a third reason to pay a cost. func WithTimeout(d time.Duration) Option { return func(o *options) { o.timeout = d } } +// WithoutGates passes over the named gates as if this run had declined them. It is +// the pre-push hook's narrow way past a gate the machine cannot run — a suite that +// needs a database this checkout does not have — without dropping the validators +// that read files, which is what skipping the whole hook would cost. +func WithoutGates(kinds ...gate.Kind) Option { + return func(o *options) { o.skipGates = append(o.skipGates, kinds...) } +} + func extra(opts []Option) []Validator { var o options for _, fn := range opts { @@ -97,9 +107,9 @@ func extra(opts []Option) []Validator { // Checks before pr: they are the slow ones, and a run that is going to fail on // a broken build should say so before it spends a network round trip on the forge. if o.checks { - limit := o.timeout + limit, skip := o.timeout, o.skipGates out = append(out, Validator{Name: "checks", Run: func(root string) (*finding.Set, error) { - return checksWithin(root, limit) + return checksWithin(root, limit, skip) }}) } if o.pr { diff --git a/internal/validate/checks.go b/internal/validate/checks.go index 2de052f..fc2ec20 100644 --- a/internal/validate/checks.go +++ b/internal/validate/checks.go @@ -3,6 +3,7 @@ package validate import ( "fmt" "os" + "slices" "time" "github.com/protonspy/spec-claude-code/internal/finding" @@ -25,10 +26,11 @@ import ( // --checks` and the pre-push hook run them, and those are the two moments they // are worth their cost: the second is where a branch becomes a pull request, // which is where this methodology says the claim is actually made. -func Checks(root string) (*finding.Set, error) { return checksWithin(root, 0) } +func Checks(root string) (*finding.Set, error) { return checksWithin(root, 0, nil) } -// checksWithin is Checks with each gate bounded by limit; zero is the default. -func checksWithin(root string, limit time.Duration) (*finding.Set, error) { +// checksWithin is Checks with each gate bounded by limit, zero being the default, +// and the gates in skip passed over as if declined for this run alone. +func checksWithin(root string, limit time.Duration, skip []gate.Kind) (*finding.Set, error) { cfg, err := gate.Load(root) if err != nil { return nil, err @@ -41,7 +43,7 @@ func checksWithin(root string, limit time.Duration) (*finding.Set, error) { // A gate somebody declined is silence. A language with no formatter and no // linter is a real answer, and a project that recorded it has already had // this conversation. - if cfg.Declined(k) { + if cfg.Declined(k) || slices.Contains(skip, k) { continue } // A gate nobody has decided on is a finding, which is the one place this diff --git a/internal/validate/checks_test.go b/internal/validate/checks_test.go index 5e2bdc0..da61848 100644 --- a/internal/validate/checks_test.go +++ b/internal/validate/checks_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/protonspy/spec-claude-code/internal/assets" + "github.com/protonspy/spec-claude-code/internal/finding" "github.com/protonspy/spec-claude-code/internal/gate" sccmanifest "github.com/protonspy/spec-claude-code/internal/manifest" "github.com/protonspy/spec-claude-code/internal/paths" @@ -351,3 +352,41 @@ func TestClipSubjectKeepsTheLineReadable(t *testing.T) { t.Errorf("a clipped subject does not say it was clipped: %q", got) } } + +// A gate passed over for one run is silent the way a declined one is, and only that +// gate: the push that skips a suite the machine cannot run still has its build held. +func TestWithoutGatesPassesOverOnlyTheNamedGates(t *testing.T) { + root := gateWorkspace(t) + cfg := gate.Config{Commands: map[gate.Kind]string{}} + for _, k := range gate.Kinds() { + cfg.Commands[k] = gate.Skipped + } + cfg.Commands[gate.Build] = "exit 3" + cfg.Commands[gate.Test] = "exit 4" + if _, err := gate.Save(root, cfg); err != nil { + t.Fatalf("gate.Save: %v", err) + } + + checks := func(opts ...Option) *finding.Set { + t.Helper() + for _, v := range All(append([]Option{WithChecks()}, opts...)...) { + if v.Name == "checks" { + set, err := v.Run(root) + if err != nil { + t.Fatalf("checks: %v", err) + } + return set + } + } + t.Fatal("WithChecks added no checks validator") + return nil + } + + if set := checks(WithoutGates(gate.Build, gate.Test)); !set.Empty() { + t.Errorf("skipping both recorded gates still reported %+v", set.Sorted()) + } + set := checks(WithoutGates(gate.Test)) + if set.Len() != 1 || !strings.Contains(set.Sorted()[0].Message, "the build gate") { + t.Errorf("skipping only the test gate let the build through: %+v", set.Sorted()) + } +} diff --git a/plans/pre-push-skip-checks.md b/plans/pre-push-skip-checks.md new file mode 100644 index 0000000..ce0cc10 --- /dev/null +++ b/plans/pre-push-skip-checks.md @@ -0,0 +1,46 @@ +--- +autonomy: auto +ci: wait +--- + +# Pre push skip checks + +Give the pre-push hook a narrow way past the delivery gate's commands — +`SCC_SKIP_CHECKS` — so a machine that cannot run part of the suite can push without +`SCC_SKIP_HOOKS=1` also switching off the artifact, attribution and pull-request checks. + +## Why + +A test suite that times out locally for an environmental reason — a database the +machine does not have — leaves `SCC_SKIP_HOOKS=1` as the only way to push, and that +skips every validator, attribution included, to get past one gate CI runs anyway. +`SCC_SKIP_CHECKS=1` skips the four command gates for one push, `SCC_SKIP_CHECKS=test` +(or a comma list) skips the named ones, and the hook says which it skipped so the push +never reads as verified. Done when the pre-push hook honors it, rejects an unknown gate +name with exit 1, and the suite and lint pass. + +## Paths + +- `internal/hooks/hooks.go` +- `internal/cli/hooks.go` +- `internal/validate/checks.go` +- `internal/validate/all.go` +- `docs/wiki/pages/hooks.md` + +## Out of scope + +- `scc validate --checks` and `scc check`: asked for by name, they run what was asked. +- Recording a skip in the manifest: that is `scc check skip `, and already exists. + +## Tasks + +- [x] 1.1 (Unit) Add `validate.WithoutGates` so a run treats the named gates as declined +- [x] 1.2 (Unit) Read `SCC_SKIP_CHECKS` in the pre-push hook, report the gates it skipped, and reject an unknown gate name + _Depends 1.1_ +- [x] 1.3 (Unit) Document `SCC_SKIP_CHECKS` beside `SCC_SKIP_HOOKS` in the hooks wiki page and the hook's failure hint + _Depends 1.2_ + +## Done when + +- `go test ./...` and `golangci-lint run` exit 0. +- `scc validate --checks --pr` exits 0.