diff --git a/internal/analyze/lint_test.go b/internal/analyze/lint_test.go index 09d0b73..6cc51cf 100644 --- a/internal/analyze/lint_test.go +++ b/internal/analyze/lint_test.go @@ -208,6 +208,8 @@ func TestWP001_CleanCases(t *testing.T) { "testdata/wp001_clean.yaml", "testdata/wp001_eventlist_clean.yaml", "testdata/wp001_steplevel_clean.yaml", + "testdata/wp001_blockbranch_clean.yaml", + "testdata/wp001_blockpath_clean.yaml", } { if got := findingsFor(lintFixture(t, file), "WP001"); len(got) != 0 { t.Errorf("%s: expected no WP001 finding, got %+v", file, got) @@ -215,6 +217,57 @@ func TestWP001_CleanCases(t *testing.T) { } } +// TestWP001_BlockStyleScopingParsesAsScoped pins the PARSE, not just the +// verdict (oleks/pipetree#22). WP001 staying quiet is necessary but not +// sufficient: a parser that dropped `branch:` entirely would also produce a +// bare push entry, which WP001 would then be right to flag - so silence alone +// cannot distinguish "scoping understood" from "scoping lost". Assert the +// constraint actually landed in the model. +// +// This is the exact shape that defeated the xonsh predecessor this linter +// replaced (oleks/emmett#385): its key regex demanded a same-line value, so a +// block-style `branch:` vanished from the entry and a correctly-scoped config +// hard-failed the push. That check is now deleted (oleks/emmett 278a567), +// leaving pipetree as the only thing standing between this bug class and +// everyone's pre-push hook. +func TestWP001_BlockStyleScopingParsesAsScoped(t *testing.T) { + for _, tc := range []struct { + file string + field string + want string + }{ + {"testdata/wp001_blockbranch_clean.yaml", "branch", "main"}, + {"testdata/wp001_blockpath_clean.yaml", "path", "src/**"}, + } { + pl, err := parse.File(tc.file) + if err != nil { + t.Fatalf("parse.File(%s): %v", tc.file, err) + } + + var pushConds []model.Condition + for _, c := range pl.When { + if c.Fields["event"] == "push" { + pushConds = append(pushConds, c) + } + } + if len(pushConds) != 1 { + t.Fatalf("%s: expected exactly 1 push condition, got %d (%+v)", + tc.file, len(pushConds), pl.When) + } + + got, ok := pushConds[0].Fields[tc.field] + if !ok { + t.Errorf("%s: block-style %q was dropped from the push condition; "+ + "fields were %+v", tc.file, tc.field, pushConds[0].Fields) + continue + } + if !strings.Contains(got, tc.want) { + t.Errorf("%s: push condition %q = %q, want it to contain %q", + tc.file, tc.field, got, tc.want) + } + } +} + // TestWP001_RemediationCarriesClass2Caveat is the constitution-V check in // test form (FR-002): the redundancy advice must never ship without the // warning about what that advice can create. diff --git a/internal/analyze/testdata/README.md b/internal/analyze/testdata/README.md index 3712a66..baf2623 100644 --- a/internal/analyze/testdata/README.md +++ b/internal/analyze/testdata/README.md @@ -18,6 +18,20 @@ naming a different rule, and one anchored to a line that has no finding. All four assert the fail-open contract — the finding survives and still blocks. +The `wp001_block*_clean.yaml` pair pins BLOCK-style scoping — a `branch:` or +`path:` whose list is indented under it rather than written inline as +`[main]` (oleks/pipetree#22). Both spellings are the same YAML, so both must +get the same verdict; they are separated here because the check pipetree +replaced got exactly this wrong (oleks/emmett#385), and since that check was +deleted (oleks/emmett 278a567) pipetree is the only remaining guard. + +Note these two are pinned by `TestWP001_BlockStyleScopingParsesAsScoped`, not +only by the `_clean` verdict. Rule silence alone cannot distinguish "scoping +understood" from "scoping lost": a parser that drops the value still leaves +the `branch` KEY in the condition, so WP001 sees a non-`event` key and stays +quiet either way. Verified by mutation — a deliberately style-sensitive +parser leaves every other WP001 test passing and fails only that one. + Class-2 fixtures are a *pair of files*, because the rules are pure functions of `(model, protection data)`: a config directory plus a JSON snapshot of what the Gitea API returns for diff --git a/internal/analyze/testdata/wp001_blockbranch_clean.yaml b/internal/analyze/testdata/wp001_blockbranch_clean.yaml new file mode 100644 index 0000000..39e91af --- /dev/null +++ b/internal/analyze/testdata/wp001_blockbranch_clean.yaml @@ -0,0 +1,24 @@ +# Same scoping as wp001_clean.yaml, written as a BLOCK-style list instead of +# the flow-style `branch: [main]`. The two spellings are the same YAML, so +# they must get the same verdict: quiet. +# +# This is the shape that broke the xonsh predecessor (oleks/emmett#385). Its +# key regex required a value on the SAME line as the key, so a `branch:` whose +# value lives on the following lines never entered the entry's key set, the +# entry read as bare, and a correctly-scoped config got a hard-failing false +# positive. pipetree is now the sole guard for this class (the xonsh check was +# deleted in oleks/emmett 278a567), so the style is pinned here deliberately. +# +# Modelled on the real config that surfaced it: ~/projects/gitea-mcp. +when: + - event: pull_request + - event: push + branch: + - main + - event: tag + +steps: + - name: unit + image: alpine + commands: + - echo running unit tests diff --git a/internal/analyze/testdata/wp001_blockpath_clean.yaml b/internal/analyze/testdata/wp001_blockpath_clean.yaml new file mode 100644 index 0000000..f980bea --- /dev/null +++ b/internal/analyze/testdata/wp001_blockpath_clean.yaml @@ -0,0 +1,19 @@ +# Block-style `path:` scoping — the other key WP001 accepts as narrowing, and +# the other half of oleks/emmett#385's blind spot. Multi-item on purpose: a +# single-element block list can be parsed correctly by accident (first item +# wins), a two-element one cannot. +# +# Must stay quiet: the push entry is scoped, so it is not the same check +# running twice on the same commit. +when: + - event: push + path: + - src/** + - internal/** + - event: pull_request + +steps: + - name: unit + image: alpine + commands: + - echo running unit tests diff --git a/internal/oracle/testdata/verdicts.json b/internal/oracle/testdata/verdicts.json index 4a35544..36eb1b2 100644 --- a/internal/oracle/testdata/verdicts.json +++ b/internal/oracle/testdata/verdicts.json @@ -29,6 +29,18 @@ "steps": 1, "has_when": true }, + { + "file": "../analyze/testdata/wp001_blockbranch_clean.yaml", + "valid": true, + "steps": 1, + "has_when": true + }, + { + "file": "../analyze/testdata/wp001_blockpath_clean.yaml", + "valid": true, + "steps": 1, + "has_when": true + }, { "file": "../analyze/testdata/wp001_clean.yaml", "valid": true,