fix: pin case-insensitive matching to the invariant culture - #77
Merged
Conversation
Determinism is this project's central claim, and it has to hold across machines rather than only across runs on one. Five patterns used RegexOptions.IgnoreCase without CultureInvariant, which folds case using whatever culture the process is running in. Turkish is the standard counterexample: uppercase I lowercases to a dotless i, so a pattern containing an i stops matching text that differs only in case. Every one of these patterns contains one - "sentinel", "uses". A deployment in tr-TR would have analysed the same workflow differently from one in en-US, and nothing would have reported the difference. Only the acceptance directive is load-bearing today. The others sit behind an ordinal prefix check that their case folding cannot get past. They were pinned anyway, because a guard that is unreachable today is reachable the moment someone loosens the check in front of it. A test analyses an uppercased acceptance under both en-US and tr-TR. It was written with a second case, asserting an uppercased "USES:" is still reported - that premise was wrong in both cultures, because the rule filters on "uses:" ordinally before the regex runs, and GitHub Actions keys are case-sensitive so "USES:" is not a step key at all. The test was removed rather than weakened, and why is recorded where the next reader will look. 195 infrastructure tests and 324 in total pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N7HjzX5e8Wv7PKENDo2Uj7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Determinism is this project's central claim, and it has to hold across machines, not only across runs on one.
Five patterns used
RegexOptions.IgnoreCasewithoutCultureInvariant, which folds case using whatever culture the process happens to be running in.Turkish is the standard counterexample: uppercase
Ilowercases to a dotlessı, so a pattern containing anistops matching text that differs only in case. Every one of these patterns contains one —sentinel,uses. A deployment intr-TRwould have analysed the same workflow differently from one inen-US, and nothing would have reported the difference.InvariantGlobalizationis not set on any project, so the culture is whatever the host provides.What is actually load-bearing
Only the acceptance directive. The others sit behind an ordinal prefix check their case folding cannot get past —
UnpinnedActionRulematchesuses:withStringComparison.Ordinalbefore its regex runs.They were pinned anyway: a guard that is unreachable today becomes reachable the moment someone loosens the check in front of it.
WorkflowSuppressionssentinel:acceptdirectiveUnpinnedActionRuleSensitiveDataSanitizerWorkflowPatchGeneratorA test I wrote and then deleted
The new test analyses an uppercased acceptance under both
en-USandtr-TR.It originally had a second case asserting that an uppercased
USES:is still reported as unpinned. That premise was wrong — it failed in both cultures, because the rule filters onuses:ordinally before the regex runs, and GitHub Actions keys are case-sensitive, soUSES:is not a step key at all. The rule is right to ignore it.The test was removed rather than weakened to pass, and the reason is recorded in the file where the next reader will look for it.
Verification
324 tests pass, 195 in the infrastructure suite. The new test pins the invariant under both cultures by swapping
CultureInfo.CurrentCulturearound the assertion.🤖 Generated with Claude Code
https://claude.ai/code/session_01N7HjzX5e8Wv7PKENDo2Uj7
Generated by Claude Code