fix(orb): degrade issue-side new-account label to a default instead of a non-null assertion (#8687) - #8714
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…ad of a non-null assertion The two PR-side new-account-label call sites (processors.ts:2765,3123) resolve the optional RepositorySettings.newAccountLabel defensively (`?? "new-account"`), but the issue-side site (6742) used a bare non-null assertion (`issueSettings.newAccountLabel!`) despite its own comment claiming "same contract as the PR maintenance path". A settings source that omits the field (a manifest overlay, or a future source) would label the issue with `undefined` on the issue path while the PR path silently falls back to "new-account". Use the same `?? "new-account"` fallback as the PR-side twins. A new test strips newAccountLabel from the resolved settings, fires an issues-opened webhook for a below-account-age-threshold author, and asserts the label applied is "new-account" (previously `undefined`).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8714 +/- ##
==========================================
- Coverage 93.79% 91.19% -2.60%
==========================================
Files 797 97 -700
Lines 79474 25673 -53801
Branches 24075 5015 -19060
==========================================
- Hits 74539 23412 -51127
+ Misses 3563 1993 -1570
+ Partials 1372 268 -1104
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-26 00:31:23 UTC
Review summary Nits — 3 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.
|
Problem
Closes #8687.
RepositorySettings.newAccountLabelis optional. The two PR-side call sites (src/queue/processors.ts:2765,3123) resolve it defensively —settings.newAccountLabel ?? "new-account"— but the issue-side site (:6742) used a bare non-null assertion,issueSettings.newAccountLabel!, despite its own comment claiming "same contract as the PR maintenance path".A settings source that omits the field (a manifest overlay, or a future source) would label the issue with
undefinedon the issue path while the PR path silently falls back to"new-account". It's masked today only becauseresolveRepositorySettings's DB defaults always populate the field.Fix
Use the same
?? "new-account"fallback as the already-correct PR-side twins. The PR-side sites are unchanged.Tests
A new test in
test/unit/queue-3.test.ts(mirroring the existing account-age issue-path tests) stripsnewAccountLabelfrom the resolved settings via the one settings-resolution seam, fires anissues openedwebhook for a below-account-age-threshold author, and asserts the applied label is"new-account"— previouslyundefined. Reverting the source change makes it fail. The existing "configured newAccountLabel" and default-label tests still pass (both??branches covered).git diff --checkclean.