fix(settings): read the two remaining LOOPOVER_* flags via the codebase truthy convention - #10097
Conversation
…se truthy convention isDuplicateWinnerEnabledGlobally and isOpenPrFileCollisionEnabledGlobally compared their env flag against the exact string "true", while every other LOOPOVER_* flag reader uses the trimmed, case-insensitive /^(1|true|yes|on)$/i convention (e.g. selfTuneFlagOn). Both functions' JSDoc even claimed the exact-"true" form WAS that convention. So LOOPOVER_DUPLICATE_WINNER=1 -- the value the regex accepts first and env.d.ts publishes for a sibling flag -- read as OFF, as did a TRUE spelling or a .env value with trailing whitespace, with no warning: the flag simply behaved as unset. Route both through /^(1|true|yes|on)$/i.test((env.X ?? "").trim()), byte-identical in shape to repository-settings.ts's selfTuneFlagOn. Correct both JSDoc claims and the third restatement in advisory.ts to name the resolver instead of the raw comparison. This only widens the accepted truthy set; "true" still enables, unset/empty still disables, and resolveDuplicateWinnerEnabled / resolveOpenPrFileCollisionEnabled keep their exact inherit/off/enabled behaviour. Closes JSONbored#10054
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 07:01:06 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10097 +/- ##
===========================================
- Coverage 91.88% 79.69% -12.20%
===========================================
Files 930 285 -645
Lines 113852 59040 -54812
Branches 27473 8808 -18665
===========================================
- Hits 104615 47051 -57564
- Misses 7940 11694 +3754
+ Partials 1297 295 -1002
Flags with carried forward coverage won't be shown. Click here to find out more.
|
What & why
This codebase has one documented truthy convention for a
LOOPOVER_*boolean env flag — trimmed, case-insensitive/^(1|true|yes|on)$/i— used by ~12 flag readers and documented insrc/env.d.tsfor a sibling flag. Two flags did not follow it, yet each asserted in its own JSDoc that its=== "true"form was the convention:So
LOOPOVER_DUPLICATE_WINNER=1— the value the convention regex accepts first, and the formenv.d.tspublishes as valid for a sibling flag — read as OFF. ATRUEspelling, or a.envvalue with trailing whitespace (routine in Docker/compose), also read as OFF, with no warning: the flag simply behaved as unset. The observable effect is a silent policy difference (every duplicate sibling closed instead of the earliest claimant spared) that an operator who set=1has no signal for.The fix
Route both through the exact convention, byte-identical in shape to
selfTuneFlagOn(src/settings/repository-settings.ts:12):/^(1|true|yes|on)$/ias the convention (asautomation-bot-skip.tsdoes) instead of claiming exact-"true"is it.src/rules/advisory.ts's third restatement now namesisDuplicateWinnerEnabledGloballyas the resolver rather than re-teaching the raw comparison."true"still enables; unset/empty still disables;resolveDuplicateWinnerEnabled/resolveOpenPrFileCollisionEnabled'sinherit/off/enabledresolution and the already-"true"wrangler.jsoncvalues keep their exact current behaviour. No new shared helper is introduced and the ~12 convention-following call sites are untouched (out of scope, per the issue).Tests
isDuplicateWinnerEnabledGloballyandisOpenPrFileCollisionEnabledGloballyeach returntruefor"1","true","TRUE","yes","on"," true "andfalseforundefined,"","0","false","off","no","maybe"— the old "ON only for exacttrue" / "OFF for truthy-looking values" tests are replaced (they fail onmainwith the new convention).resolveDuplicateWinnerEnabled(isDuplicateWinnerEnabledGlobally({ LOOPOVER_DUPLICATE_WINNER: "1" }), "inherit")istrue— the=1form enables the feature end-to-end through the resolver, not just the global reader.Validation
?? ""nullish arm is exercised on both the absent and present sides);advisory.tsis a JSDoc-only edit.npm run typecheckclean for these files (the only local errors are a pre-existing missing-release-please-dep phantom in unrelated release tooling, present on baremain).git diff --checkclean; no schema/route/env-reference/wrangler change, so no generated artifact is affected.Closes #10054