No way to suppress a WP001 finding that is a deliberate, documented choice #20

Closed
opened 2026-08-13 19:16:18 +03:00 by issuer-agent · 1 comment
Collaborator

Why: A fleet sweep flagged ~/projects/forrest/.woodpecker/check.yaml for WP001 (unscoped push + unscoped pull_request). It is a TRUE POSITIVE of the rule and simultaneously CORRECT configuration: the file carries an extensive comment explaining that push-to-any-branch plus all-PRs is deliberate, citing three separate incidents it exists to prevent — anton/forrest#584 (develop merge commits were a CI blind spot; two individually-green PRs can conflict semantically), anton/forrest#1127 (cancel_previous_pipeline_events was cancelling back-to-back develop merges' runs), and anton/forrest#1017 (a hand-carried hotfix pushed straight to production was never typechecked).

Applying WP001's remediation there would reintroduce documented blind spots.

pipetree has no suppression mechanism, so this repo will be flagged forever, and once emmett's pre-push hook cuts over to pipetree lint (oleks/emmett#380) it would HARD-FAIL every push to that repo. A blocking check that misfires on correct config is the exact failure mode oleks/emmett#317 records — it trains people to --no-verify.

Suggested shape (design decision, not prescribed): an inline comment directive the linter honours, e.g. # pipetree:ignore WP001 -- <reason> on or above the when: block, with the reason REQUIRED so a suppression is self-documenting and reviewable. Constitution Principle V's spirit argues the reason should be mandatory rather than optional.

Acceptance:

  • A fixture that suppresses WP001 and one that does not, both covered by tests
  • The suppression is reported somewhere (a suppressed-count in the JSON output) rather than being invisible, so a repo cannot silently suppress everything

Links: Surfaced during a fleet-wide pipetree lint sweep on 2026-08-13 (oleks/pipetree at 9083db9). Related: oleks/emmett#380, oleks/emmett#317, anton/forrest#584, anton/forrest#1127, anton/forrest#1017.

**Why**: A fleet sweep flagged `~/projects/forrest/.woodpecker/check.yaml` for WP001 (unscoped push + unscoped pull_request). It is a TRUE POSITIVE of the rule and simultaneously CORRECT configuration: the file carries an extensive comment explaining that push-to-any-branch plus all-PRs is deliberate, citing three separate incidents it exists to prevent — anton/forrest#584 (develop merge commits were a CI blind spot; two individually-green PRs can conflict semantically), anton/forrest#1127 (cancel_previous_pipeline_events was cancelling back-to-back develop merges' runs), and anton/forrest#1017 (a hand-carried hotfix pushed straight to production was never typechecked). Applying WP001's remediation there would reintroduce documented blind spots. pipetree has no suppression mechanism, so this repo will be flagged forever, and once emmett's pre-push hook cuts over to `pipetree lint` (oleks/emmett#380) it would HARD-FAIL every push to that repo. A blocking check that misfires on correct config is the exact failure mode oleks/emmett#317 records — it trains people to --no-verify. Suggested shape (design decision, not prescribed): an inline comment directive the linter honours, e.g. `# pipetree:ignore WP001 -- <reason>` on or above the `when:` block, with the reason REQUIRED so a suppression is self-documenting and reviewable. Constitution Principle V's spirit argues the reason should be mandatory rather than optional. **Acceptance**: - [ ] A fixture that suppresses WP001 and one that does not, both covered by tests - [ ] The suppression is reported somewhere (a suppressed-count in the JSON output) rather than being invisible, so a repo cannot silently suppress everything **Links**: Surfaced during a fleet-wide `pipetree lint` sweep on 2026-08-13 (oleks/pipetree at 9083db9). Related: oleks/emmett#380, oleks/emmett#317, anton/forrest#584, anton/forrest#1127, anton/forrest#1017.
Owner

Shipped in 757e81a on main.

# pipetree:ignore <RULE> -- <reason> on the flagged line or above it. A
whole-line directive governs the next line that is neither blank nor a
comment, so it can sit anywhere inside an explanatory comment block above
the when: it excuses; a trailing directive governs its own line. One
directive names one rule — there is no wildcard or file-level blanket form.

Both acceptance criteria:

  • Fixtures both ways, covered by tests. Six new fixtures against one
    unchanging config (wp001_fires.yaml's), so the directive is the only
    variable: wp001_suppressed_clean.yaml and
    wp001_suppressed_trailing_clean.yaml are honoured; the four _fires
    ones — missing reason, unregistered rule ID, a directive naming a
    different rule, and one anchored to a line with no finding — each assert
    the finding survives and the run still exits blocking. woodpecker-cli
    accepts all six (oracle golden regenerated at 24 fixtures).
  • Suppressions are reported, not invisible. New suppressions array
    in the JSON (always present, additive per constitution III — the contract
    and its golden are updated), and one stderr line per directive in text
    mode. Applied, rejected and stale directives are all listed, so
    "clean" and "clean because fourteen were suppressed" stay distinguishable,
    and a rejected directive reads as "still blocking despite an attempt to
    silence it".

The reason is required, as the issue argued: a bare # pipetree:ignore WP001 is rejected. Every rejection fails open — it does not suppress,
does not error, and says on stderr why it was ignored. Turning a typo in a
comment into a blocked push would recreate oleks/emmett#317 one level up.

Anchoring is by line rather than yaml.Node comment attachment: yaml.v3
shifts a comment's head/line/foot assignment when neighbouring keys are
edited, and a suppression that silently detaches because someone reordered
a key above it is worse than no suppression. Findings already anchor by
line, so the two agree by construction.

Class-2 findings are deliberately not suppressible this way. They are
repo-level facts about branch protection on the server, so a comment in a
config file is the wrong place to record a decision about them, and
silencing one would hide a genuinely unsatisfiable required check rather
than an over-eager pattern match.

Verified end-to-end on a copy of the real forrest/.woodpecker/check.yaml
(the founding case; the live file was not touched): exit 1 → exit 0, with
check.yaml:40: suppressed [WP001] -- the doubled run is deliberate ...
on stderr. Applying the directive to forrest itself is a separate change in
a separate repo and is not part of this.

Shipped in `757e81a` on `main`. `# pipetree:ignore <RULE> -- <reason>` on the flagged line or above it. A whole-line directive governs the next line that is neither blank nor a comment, so it can sit anywhere inside an explanatory comment block above the `when:` it excuses; a trailing directive governs its own line. One directive names one rule — there is no wildcard or file-level blanket form. Both acceptance criteria: - [x] **Fixtures both ways, covered by tests.** Six new fixtures against one unchanging config (`wp001_fires.yaml`'s), so the directive is the only variable: `wp001_suppressed_clean.yaml` and `wp001_suppressed_trailing_clean.yaml` are honoured; the four `_fires` ones — missing reason, unregistered rule ID, a directive naming a different rule, and one anchored to a line with no finding — each assert the finding survives and the run still exits blocking. `woodpecker-cli` accepts all six (oracle golden regenerated at 24 fixtures). - [x] **Suppressions are reported, not invisible.** New `suppressions` array in the JSON (always present, additive per constitution III — the contract and its golden are updated), and one stderr line per directive in text mode. Applied, *rejected* and stale directives are all listed, so "clean" and "clean because fourteen were suppressed" stay distinguishable, and a rejected directive reads as "still blocking despite an attempt to silence it". The reason is required, as the issue argued: a bare `# pipetree:ignore WP001` is rejected. Every rejection **fails open** — it does not suppress, does not error, and says on stderr why it was ignored. Turning a typo in a comment into a blocked push would recreate oleks/emmett#317 one level up. Anchoring is by line rather than `yaml.Node` comment attachment: yaml.v3 shifts a comment's head/line/foot assignment when neighbouring keys are edited, and a suppression that silently detaches because someone reordered a key above it is worse than no suppression. Findings already anchor by line, so the two agree by construction. Class-2 findings are deliberately **not** suppressible this way. They are repo-level facts about branch protection on the server, so a comment in a config file is the wrong place to record a decision about them, and silencing one would hide a genuinely unsatisfiable required check rather than an over-eager pattern match. Verified end-to-end on a **copy** of the real `forrest/.woodpecker/check.yaml` (the founding case; the live file was not touched): exit 1 → exit 0, with `check.yaml:40: suppressed [WP001] -- the doubled run is deliberate ...` on stderr. Applying the directive to forrest itself is a separate change in a separate repo and is not part of this.
oleks closed this issue 2026-08-13 20:13:13 +03:00
Sign in to join this conversation.