diff --git a/.github/workflows/callable-pr-policy.yaml b/.github/workflows/callable-pr-policy.yaml index b6859a5..d7689d7 100644 --- a/.github/workflows/callable-pr-policy.yaml +++ b/.github/workflows/callable-pr-policy.yaml @@ -214,7 +214,7 @@ jobs: // FNV-1a 32-bit hash of a string — used to produce content fingerprints // for line-specific violations so changing the payload changes the - // fingerprint while shifting the same payload to a different line does not. + // fingerprint even when the violation remains on the same line. function fnv1a32(str) { let h = 2166136261; for (let i = 0; i < str.length; i++) { @@ -514,19 +514,18 @@ jobs: return null; } - // Normalize a validateWorkflowContent error string to a line-agnostic - // fingerprint. Removes the `:LINE:` segment produced by per-line errors - // so that a violation shifted to a different line in the head file does - // not appear new relative to the base. Content-identifying tokens (action - // ref, expression text) are preserved. + // Normalize a validateWorkflowContent error string to a diff fingerprint. + // Per-line locations are intentionally preserved so moving a grandfathered + // violation to a different execution path is treated as a new violation. + // Content-identifying tokens (action ref, expression text) are preserved. function normalizeViolationFingerprint(err) { - return err.replace(/:\d+:/, ':'); + return err; } - // Diff head vs base violations using normalized fingerprints with - // multiplicity. For each fingerprint, up to base-count head violations - // of that fingerprint are considered pre-existing; the remainder are new. - // Violations are returned in head-file order. + // Diff head vs base violations using location-preserving fingerprints + // with multiplicity. For each fingerprint, up to base-count head + // violations of that fingerprint are considered pre-existing; the + // remainder are new. Violations are returned in head-file order. function filterNewViolations(headErrs, baseErrs) { const baseCounts = new Map(); for (const e of baseErrs) { diff --git a/test/pr-policy.test.mjs b/test/pr-policy.test.mjs index d16f676..cad939e 100644 --- a/test/pr-policy.test.mjs +++ b/test/pr-policy.test.mjs @@ -1507,9 +1507,9 @@ describe('static YAML assertions', () => { // ── normalizeViolationFingerprint ──────────────────────────────────────────── describe('normalizeViolationFingerprint', () => { - it('strips :LINE: from per-line errors', () => { + it('preserves :LINE: in per-line errors', () => { const err = 'f.yaml:42: run: block contains expression — expressions must go through env:'; - assert.equal(v.normalizeViolationFingerprint(err), 'f.yaml: run: block contains expression — expressions must go through env:'); + assert.equal(v.normalizeViolationFingerprint(err), err); }); it('leaves uses: violations unchanged (no line number in error)', () => { @@ -1522,15 +1522,15 @@ describe('normalizeViolationFingerprint', () => { assert.equal(v.normalizeViolationFingerprint(err), err); }); - it('removes only the first :LINE: occurrence', () => { + it('does not rewrite line-like message content', () => { const err = 'f.yaml:10: escaped key: something'; - assert.equal(v.normalizeViolationFingerprint(err), 'f.yaml: escaped key: something'); + assert.equal(v.normalizeViolationFingerprint(err), err); }); - it('line 10 and line 200 normalize to the same fingerprint', () => { + it('line 10 and line 200 remain distinct fingerprints', () => { const a = v.normalizeViolationFingerprint('f.yaml:10: run: block contains expression'); const b = v.normalizeViolationFingerprint('f.yaml:200: run: block contains expression'); - assert.equal(a, b); + assert.notEqual(a, b); }); }); @@ -1571,10 +1571,10 @@ describe('filterNewViolations', () => { assert.deepEqual(v.filterNewViolations([err], [err]), []); }); - it('line shift ignored — same run expression moved to different line', () => { + it('line shift is blocked when the same run expression moves', () => { const head = FP_EXPR('w.yaml', 20); const base = FP_EXPR('w.yaml', 10); - assert.deepEqual(v.filterNewViolations([head], [base]), []); + assert.deepEqual(v.filterNewViolations([head], [base]), [head]); }); it('newly added run expression is blocked', () => { @@ -1586,7 +1586,7 @@ describe('filterNewViolations', () => { it('unchanged run expression ignored; a second new one blocked', () => { const base = FP_EXPR('w.yaml', 10); - const head1 = FP_EXPR('w.yaml', 12); + const head1 = FP_EXPR('w.yaml', 10); const head2 = FP_EXPR('w.yaml', 50); const result = v.filterNewViolations([head1, head2], [base]); assert.equal(result.length, 1); @@ -1694,7 +1694,7 @@ describe('content fingerprint: changed payload is new', () => { assert.equal(v.filterNewViolations(headErrs, baseErrs).length, 1, 'changed expression must be reported as new'); }); - it('unchanged run expression at a shifted line is grandfathered', () => { + it('unchanged run expression at a shifted line is blocked', () => { const wfA = BASE_WF('echo ${{ github.ref }}'); // wfB inserts a blank step before the run step, shifting its line number const wfB = [ @@ -1713,7 +1713,7 @@ describe('content fingerprint: changed payload is new', () => { ].join('\n'); const headErrs = v.validateWorkflowContent(wfB, 'f.yaml'); const baseErrs = v.validateWorkflowContent(wfA, 'f.yaml'); - assert.deepEqual(v.filterNewViolations(headErrs, baseErrs), [], 'same expression on a different line must be grandfathered'); + assert.equal(v.filterNewViolations(headErrs, baseErrs).length, 1, 'same expression on a different line must be reported as new'); }); it('flow-style run changed to flow-style uses is new', () => {