ViolationIdentifier includes strict, but violations rebuilt from package_todo.yml are always given strict: false (pack.rs#L195). A found violation in a strict pack has strict: true, so it can never equal its recorded entry.
That breaks all three comparisons in CheckAllBuilder at once. The same recorded violation is reported as new, as a strict-mode violation, and as a stale todo.
Repro
tests/fixtures/uses_strict_mode already has the shape: packs/foo/package_todo.yml records ::Bar for privacy and dependency, and packs/foo/app/services/foo.rb references it.
$ pks --project-root tests/fixtures/uses_strict_mode check
2 violation(s) detected:
[...]
There were stale violations found, please run `packs update`
packs/foo cannot have dependency violations on packs/bar because strict mode is enabled [...]
packs/foo cannot have privacy violations on packs/bar because strict mode is enabled [...]
Two of those three look wrong whatever strict mode is meant to do: the violation is recorded, so it isn't new, and its todo entry isn't stale.
The third one is a policy question
#166 already described the divergence:
packwerk will not update todo files with strict violations and will also not report packwerk check strict violations if they have a corresponding todo entry. Supporting pks check ignoring strict todo violations is outside the scope of this issue.
packwerk made that tolerance its default in Shopify/packwerk#368. Matching it is what makes enforce_privacy: strict adoptable on a pack that already has recorded violations.
Why we hit it
A Rails monolith, 15.6k files, 65 packs, two of them strict with 66 recorded todo entries between them. Stock 0.4.0 reports 185 strict violations and 135 stale todos on a tree that packwerk check passes clean, so we could not move off packwerk without granting every existing caller a blanket enforcement_globs_ignore exemption.
PR to follow.
ViolationIdentifierincludesstrict, but violations rebuilt frompackage_todo.ymlare always givenstrict: false(pack.rs#L195). A found violation in a strict pack hasstrict: true, so it can never equal its recorded entry.That breaks all three comparisons in
CheckAllBuilderat once. The same recorded violation is reported as new, as a strict-mode violation, and as a stale todo.Repro
tests/fixtures/uses_strict_modealready has the shape:packs/foo/package_todo.ymlrecords::Barfor privacy and dependency, andpacks/foo/app/services/foo.rbreferences it.Two of those three look wrong whatever strict mode is meant to do: the violation is recorded, so it isn't new, and its todo entry isn't stale.
The third one is a policy question
#166 already described the divergence:
packwerk made that tolerance its default in Shopify/packwerk#368. Matching it is what makes
enforce_privacy: strictadoptable on a pack that already has recorded violations.Why we hit it
A Rails monolith, 15.6k files, 65 packs, two of them strict with 66 recorded todo entries between them. Stock 0.4.0 reports 185 strict violations and 135 stale todos on a tree that
packwerk checkpasses clean, so we could not move off packwerk without granting every existing caller a blanketenforcement_globs_ignoreexemption.PR to follow.