test(signals): cover decidePublicSurface's oss_maintainer + not_checked label fallback#8495
Conversation
…ed label fallback
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-24 16:07:17 UTC
Review summary Nits — 4 non-blocking
CI checks failing
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.
|
|
LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests-merge)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
What
decidePublicSurface(src/signals/settings-preview.ts) documents itself as the "single source oftruth … the preview can never drift from real behavior." Its
willLabelcomputation (line ~149) hasan inline second disjunct handling
publicAudienceMode: "oss_maintainer"+minerStatus: "not_checked"— a real webhook combination — that duplicates
shouldApplyPrLabel's ownautoLabelEnabled && (label surface)logic while bypassing that function'soss_maintainer && !confirmed → falseguard (line 39).That branch had zero test coverage (
grep "not_checked" test/unit/settings-preview.test.tsreturnednothing), so a future change to
shouldApplyPrLabelcould silently diverge from this inline copy withno test catching it.
Change
Pure test addition (no production change) in
test/unit/settings-preview.test.ts. Since foross_maintainer+not_checkedthe first disjunct (shouldApplyPrLabel) always returnsfalse, theinline fallback alone decides
willLabel— so the new test exercises every operand of its&&chain:autoLabelEnabled: true+publicSurface: "comment_and_label"→willLabel(label action) included.autoLabelEnabled: true+publicSurface: "label_only"→ included.autoLabelEnabled: true+publicSurface: "comment_only"→ not included (the disjunct's ownpublicSurfacecheck excludes it).autoLabelEnabled: false→ not included (falls back toshouldApplyPrLabel's ownfalse).No divergence from
shouldApplyPrLabel's intent was found while writing these (the inline conditionmirrors line 40 exactly), so nothing is "fixed" here per the issue's guidance.
Validation
npx vitest run test/unit/settings-preview.test.ts→ all pass.willLabel||/&&chain now covered (verified via lcov — all BRDA arms on thedisjunct non-zero).
npm run test:changednet green (modulo the pre-existing Windows-only baseline failures thatpass on Linux CI).
Closes #8324