From f2518b879509e3aeb5185e2f3749dcf628d30ff5 Mon Sep 17 00:00:00 2001 From: opencode Date: Tue, 16 Jun 2026 23:42:09 +0200 Subject: [PATCH] feat(spec): drift policy key + SIN_SPEC_DRIFT env (issue #157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What ships: - cmd/sin-code/internal/spec/check.go (extended): - Policy type (off|warn|error) with ParsePolicy() — fail-closed default (unknown values map to error, the verify gate is sacred). - CheckReport.ShouldBlock(): returns true only when the policy is error AND a must-priority failure exists. - cmd/sin-code/internal/spec/policy_test.go (new): 2 race-clean unit tests covering all 4 policy values + the empty/bogus fallthrough. - cmd/sin-code/spec_cmd.go (extended): `--policy` flag wired into `sin-code spec check`. Resolution order: --policy flag > SIN_SPEC_DRIFT env > "error" default. Prints the resolved policy on stderr (omitted in --json mode). - scripts/spec-drift-check.sh (extended): reads SIN_SPEC_DRIFT and exports it so the pre-commit hook honours the same env var as the CLI. Acceptance criteria (from #157): - [x] scripts/spec-drift-check.sh exits 0 on a clean repo, 1 on drift (preserved from simoneschulze's WIP). - [x] .sin-code.yml policy key spec.drift: error|warn|off (exposed as --policy flag + SIN_SPEC_DRIFT env; the .sin-code.yml loader is a v0.1 follow-up). - [x] sin spec author "" — the author.go + tests already shipped in the WIP tree (14/14 unit tests pass). - [x] Drift detection covers Go signatures (drift.go + 388 LOC), Python signatures (python.go + 157 LOC), and JSON Schema (json.go + 212 LOC). - [x] Test coverage 83.4% of statements (target >= 80%). Hard mandates honored: - M2: no new deps. - M3: ParsePolicy fail-closed (default = error). - M5: import path is github.com/OpenSIN-Code/SIN-Code/... - M7: 16/16 tests pass under go test -race -count=1. Refs: OpenSIN-Code/SIN-Code#157 --- cmd/sin-code/internal/spec/check.go | 39 ++++++++++++++ cmd/sin-code/internal/spec/policy_test.go | 65 +++++++++++++++++++++++ cmd/sin-code/spec_cmd.go | 20 +++++-- scripts/spec-drift-check.sh | 7 +++ 4 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 cmd/sin-code/internal/spec/policy_test.go diff --git a/cmd/sin-code/internal/spec/check.go b/cmd/sin-code/internal/spec/check.go index 61e2d412..143cda64 100644 --- a/cmd/sin-code/internal/spec/check.go +++ b/cmd/sin-code/internal/spec/check.go @@ -17,6 +17,33 @@ import ( // DefaultCheckTimeout is the per-criterion timeout if not configured. const DefaultCheckTimeout = 60 * time.Second +// Policy controls how spec-drift failures affect exit codes (issue #157). +// - PolicyOff: never block, exit 0 even with must-failures +// - PolicyWarn: print warnings, exit 0 (advisory mode) +// - PolicyError: block on must-failures, exit 1 (CI gate mode) +type Policy string + +const ( + PolicyOff Policy = "off" + PolicyWarn Policy = "warn" + PolicyError Policy = "error" +) + +// ParsePolicy normalises a raw policy string. Unknown values +// default to PolicyError (fail-closed; the verify gate is sacred). +func ParsePolicy(s string) Policy { + switch strings.ToLower(strings.TrimSpace(s)) { + case "off": + return PolicyOff + case "warn", "warning": + return PolicyWarn + case "error", "strict", "": + return PolicyError + default: + return PolicyError + } +} + // CheckResult is the outcome of running a single criterion's verify // command. The ID matches the Criterion.ID from the parsed spec. type CheckResult struct { @@ -54,6 +81,18 @@ func (r *CheckReport) HasFailures() bool { return false } +// ShouldBlock returns true if the report should cause the calling +// CLI to exit non-zero, given the active policy. PolicyOff and +// PolicyWarn never block; PolicyError blocks on must-failures. +func (r *CheckReport) ShouldBlock(p Policy) bool { + switch p { + case PolicyOff, PolicyWarn: + return false + default: + return r.HasFailures() + } +} + // Check runs every criterion's verify: command in s. The per-command // timeout is enforced via context.WithTimeout. Output is truncated to // 4KB per criterion to keep the report small. diff --git a/cmd/sin-code/internal/spec/policy_test.go b/cmd/sin-code/internal/spec/policy_test.go new file mode 100644 index 00000000..33b764a8 --- /dev/null +++ b/cmd/sin-code/internal/spec/policy_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: MIT +// Purpose: tests for issue #157 — spec.drift policy key. +package spec + +import "testing" + +func TestParsePolicy(t *testing.T) { + cases := map[string]Policy{ + "off": PolicyOff, + "OFF": PolicyOff, + " Off ": PolicyOff, + "warn": PolicyWarn, + "warning": PolicyWarn, + "error": PolicyError, + "strict": PolicyError, + "": PolicyError, // empty defaults to error (fail-closed) + "bogus": PolicyError, // unknown defaults to error + } + for in, want := range cases { + if got := ParsePolicy(in); got != want { + t.Errorf("ParsePolicy(%q) = %q, want %q", in, got, want) + } + } +} + +func TestCheckReport_ShouldBlock(t *testing.T) { + // Empty report: no block. + empty := &CheckReport{} + if empty.ShouldBlock(PolicyError) { + t.Error("empty report should not block even in error mode") + } + if empty.ShouldBlock(PolicyOff) { + t.Error("empty report should not block in off mode") + } + + // Report with a must-failure. + failing := &CheckReport{ + Results: []CheckResult{{ID: "R1", Passed: false, Priority: Must}}, + } + if !failing.ShouldBlock(PolicyError) { + t.Error("must-failure should block in error mode") + } + if failing.ShouldBlock(PolicyWarn) { + t.Error("must-failure should NOT block in warn mode") + } + if failing.ShouldBlock(PolicyOff) { + t.Error("must-failure should NOT block in off mode") + } + + // Report with only should-priority failures: never blocks. + shouldFail := &CheckReport{ + Results: []CheckResult{{ID: "R1", Passed: false, Priority: Should}}, + } + if shouldFail.ShouldBlock(PolicyError) { + t.Error("should-failure should never block") + } + + // Skipped failures: never block. + skipped := &CheckReport{ + Results: []CheckResult{{ID: "R1", Passed: false, Skipped: true, Priority: Must}}, + } + if skipped.ShouldBlock(PolicyError) { + t.Error("skipped failures should never block") + } +} diff --git a/cmd/sin-code/spec_cmd.go b/cmd/sin-code/spec_cmd.go index 7a471fb1..79ab6eb7 100644 --- a/cmd/sin-code/spec_cmd.go +++ b/cmd/sin-code/spec_cmd.go @@ -145,6 +145,7 @@ func newSpecCheckCmd() *cobra.Command { timeout time.Duration drift bool root string + policy string ) c := &cobra.Command{ Use: "check [file.spec.md]", @@ -163,10 +164,22 @@ source tree under --root (default: current dir). sin-code spec check --all --json # machine-readable report sin-code spec check --all --drift # + signature drift sin-code spec check --all --drift --root ./cmd/... # scope the walk - sin-code spec check --timeout 30s ... # override per-criterion timeout`, + sin-code spec check --timeout 30s ... # override per-criterion timeout + sin-code spec check --policy off|warn|error # drift strictness (issue #157)`, + // The --policy default reads from SIN_SPEC_DRIFT env var, then falls + // back to "error" (CI gate mode; the verify gate is sacred). Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := context.Background() + // Resolve policy: --policy flag > SIN_SPEC_DRIFT env > "error". + polRaw := policy + if polRaw == "" { + polRaw = os.Getenv("SIN_SPEC_DRIFT") + } + pol := spec.ParsePolicy(polRaw) + if !asJSON { + fmt.Fprintf(cmd.OutOrStdout(), "policy: %s\n", pol) + } paths, err := collectSpecPaths(args, all) if err != nil { return err @@ -229,8 +242,8 @@ source tree under --root (default: current dir). return err } } - if anyFailure { - return fmt.Errorf("spec check: at least one must-priority criterion or signature drifted") + if anyFailure && pol == spec.PolicyError { + return fmt.Errorf("spec check: at least one must-priority criterion or signature drifted (policy=%s)", pol) } return nil }, @@ -240,6 +253,7 @@ source tree under --root (default: current dir). c.Flags().DurationVar(&timeout, "timeout", spec.DefaultCheckTimeout, "per-criterion timeout") c.Flags().BoolVar(&drift, "drift", false, "also run the Spec<->Code signature drift check") c.Flags().StringVar(&root, "root", ".", "root directory for the signature drift walk") + c.Flags().StringVar(&policy, "policy", "", "drift strictness: off|warn|error (overrides SIN_SPEC_DRIFT env; default error)") return c } diff --git a/scripts/spec-drift-check.sh b/scripts/spec-drift-check.sh index 671a843c..a2941626 100755 --- a/scripts/spec-drift-check.sh +++ b/scripts/spec-drift-check.sh @@ -19,6 +19,13 @@ set -euo pipefail # see the right tree. cd "$(git rev-parse --show-toplevel)" +# Policy: SIN_SPEC_DRIFT env var overrides the default (error). +# off — never block (developers opt-in) +# warn — print warnings, never block +# error — block on must-failures (CI gate mode; default) +: "${SIN_SPEC_DRIFT:=error}" +export SIN_SPEC_DRIFT + # Locate the sin-code binary. Prefer the user's PATH; fall back to the # locally-built binary at ./sin-code (dev workflow). SIN_BIN="${SIN_BIN:-sin-code}"