Repository navigation
[quality] Pin organization lgtm/sweep inheritance and optional label-value parsing against regressions #534
Description
Scope clarification
This is an assertion-strength/regression-test gap, not an observed production configuration-loss bug. Current mergeProwConfig preserves the organization lgtm and sweep sections with { ...base.section, ...over.section }. The mutations described below simulate regressions that existing assertions did not catch.
This issue is the canonical tracker; #535 supplies the regression assertions. The matching finding folded into #521 is a duplicate and does not extend that approve issue's closure requirements.
Finding
src/utils/config.ts is 100 % line- and branch-covered by npm run test:coverage and, apart from the String(e) arm at line 657 that #528 records as unreachable from the bundle, by npm run test:coverage:e2e as well (main @ 88241f4, local runs). A single-operator mutation probe over the file (258 mutants: ===/!==, &&/||, ??→||, === undefined→=== null, comparison bounds, .every/.some, .find/.findLast, Math.min/max, spread-source swaps in mergeProwConfig, true/false) against the six direct config suites left 33 alive; re-run against npx vitest related src/utils/config.ts --run (70 files / 1498 tests) two non-equivalent mutants still survive, and three more are killed by exactly one unrelated test:
| line | mutant | behaviour nothing asserts |
|---|---|---|
| 740 | lgtm: { ...base.lgtm, ...over.lgtm } → { ...over.lgtm, ...over.lgtm } |
An organization lgtm.bind_to_commit: true must survive a repository prow.yaml that has no lgtm section. The mergeProwConfig test puts lgtm only in the repo tier, so the org tier is never the one that has to survive. |
| 741 | sweep: { ...base.sweep, ...over.sweep } → { ...over.sweep, ...over.sweep } |
Same for an organization sweep.lookback: no test merges a sweep section at all (sweep: {} is the only asserted value). |
| 451 | section.exclusive === undefined → === null |
kind: { values: [...] } without exclusive — the documented optional form — must parse. Every values: mapping in the config suites sets exclusive; only labelCatalog.test.ts › "de-duplicates case-insensitively" happens to parse one. |
| 469 / 472 | value.color === undefined → !== undefined; value.color !== undefined → !== null |
{ name: bug } and { name: docs, description: … } without a color must parse (and must not be run through the color check). Every object value in the config suites carries color; again only the labelCatalog de-dup test pins it. |
| 421 | unknown.length > 0 → >= 0 |
A document whose top-level keys are all known must not emit a core.debug('…ignoring unknown top level keys: ') line. Debug-only, but free to pin next to the existing "logs them once" case. |
Equivalent survivors, not actionable: .trim() on the config input (core.getInput already trims); every ??→|| on a value the normalizers already reject when falsy (tide.*, hold.label, authorization.*, require_matching_label, users, sweep.lookback); max_request_count !== undefined → !== null (undefined < n is false either way); fetchRepoFile's ||→&& folds on the not-a-file guard (no getContent shape distinguishes them); .find→.findLast on the unknown-authorization-key message. ms <= 0 (line 659) and the 24 h cap (line 720) are pinned by __tests__/cronJobTest/sweep.test.ts; merge_on_events ?? true (line 787) by mergeGate.test.ts.
Recommendation
Add to __tests__/utils/config.test.ts:
mergeProwConfig› orglgtm/sweepsurvive a repo tier without them, and are overridden when the repo tier has them.label values› an object value withoutcolorin a list, and avalues:mapping withoutexclusive(assertingexclusiveis absent).parseProwConfig›core.debugnot called when every top-level key is known.
Test-only; no src/, dist/ or docs change. Files claimed: __tests__/utils/config.test.ts. Disjoint from every open held PR (#510–#533): none touches __tests__/utils/config.test.ts or src/utils/config.ts.
Evidence
- unit:
npm run test:coverage→ 100 / 99.75 / 100 / 100 (main@ 88241f4, local run; the four reported branches are theawaitartifact docs(contributing): record the four negative-count BRDA branches as a v8 await artifact, not a gap #511 documents) - e2e:
npm run test:coverage:e2e→ 99.96 / 98.79 / 100 / 99.95 (same revision, local run;config.tsshows only line 657, see test(bundle): cover non-Error rejection paths including auth #528). The two reports come from different transforms and are not merged. - probe: each mutant applied in turn to
src/utils/config.ts; direct suitesnpx vitest run __tests__/utils/config.test.ts __tests__/utils/configLoader.test.ts __tests__/utils/configSweep.test.ts __tests__/configurationDocs.test.ts __tests__/dogfoodConfig.test.ts __tests__/docsYamlExamples.test.ts(241 tests, ~6 s); survivors re-run withnpx vitest related src/utils/config.ts --run→ 1498 passed for lines 740, 741, 421; 1 failed (labelCatalog.test.ts) for 451, 469, 472.
Priority
- Impact: medium —
mergeProwConfigis the org/repo tiering rule every plugin reads through; a regression there silently turns off an organization-widelgtm.bind_to_commitor sweep window for every repository that ships its ownprow.yaml - Effort: low
Filed by quality agent (hold-gated mode)
🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: unknown
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88
Activity
github-actions commentedon Oct 20, 2026
Please add a kind label with
/kind feature,/kind failing-testor/kind cleanup.[-][quality] mergeProwConfig loses the organization lgtm and sweep sections when the repository prow.yaml omits them; values-without-exclusive and object-without-color pinned only by labelCatalog[/-][+][quality] Pin organization lgtm/sweep inheritance and optional label-value parsing against regressions[/+]