fix(orb): exclude contributor self-closes from the public homepage counter - #1632
Conversation
…unter recordOrbPrOutcome (the central Orb's terminal-outcome capture feeding the public homepage merged/closed/total counter) recorded a contributor closing their OWN unmerged PR as an authoritative 'closed' — a contributor could inflate the public counter by opening + self-closing PRs. Add the same anti-poisoning guard the cloud recordPrOutcome path uses: when the close is unmerged, the actor is not a Bot, and sender.login === pull_request.user.login, early-return (record nothing, leave any prior authoritative row untouched). Merges stay trusted; maintainer + bot closes remain authoritative.
|
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 #1632 +/- ##
=======================================
Coverage 95.53% 95.53%
=======================================
Files 204 204
Lines 22084 22089 +5
Branches 7972 7975 +3
=======================================
+ Hits 21097 21102 +5
Misses 412 412
Partials 575 575
🚀 New features to boost your workflow:
|
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review — safe to merge
✅ Approved — safe to merge Review summary
Nits — 5 non-blocking
Review context
Contributor next steps
Signal definitions
Review detailsGenerated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative. Straightforward parity port of the cloud `recordPrOutcome` anti-poisoning guard to the Orb path. The guard logic is correct: `.toLowerCase()` comparison handles GitHub's case-insensitive logins, the `botWasActor` exemption correctly bypasses the guard for Bot-typed senders even when their login matches the PR author, and the `senderLogin && authorLogin` truthiness checks fail-open (record) when either identity is absent — the right call for a public integrity counter. Tests cover every new branch with real-path payloads and no fabricated impossible states; no schema change means no migration needed. Nits (5)
🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Summary
recordOrbPrOutcome(the central Orb's terminal-PR-outcome capture feeding the public homepage merged/closed/total counter) recorded a contributor closing their own unmerged PR as an authoritativeclosed. A contributor can inflate the public counter (or flip a repo's stats) by opening + self-closing PRs — the same poisoning the cloudrecordPrOutcomepath already guards against, here on the more-visible metric.Add the same anti-poisoning guard: when the close is unmerged, the actor is not a Bot, and
sender.login === pull_request.user.login, early-return (record nothing, leave any prior authoritative row untouched). Merges stay trusted (GitHub enforces merge permission); maintainer and bot closes are authoritative and still recorded.Surfaced by the 2026-06-27 review-pipeline hardening audit (accuracy).
Scope
orb/outcomes.ts+ its test).site//CNAME/Pages; followsCONTRIBUTING.md.Validation
git diff --check·actionlint·typechecktest:coverage— new tests cover every arm of the guard: self-close (records nothing + never overwrites a prior maintainer row), maintainer close (sender≠author → recorded), bot-actor close (recorded even when login matches), close with no author field (not a self-close → recorded); the existing merged + no-sender closed tests cover the!mergedand falsy-sender short-circuits.test:workers·build:mcp·test:mcp-pack·ui:*·npm audit --audit-level=moderateIf any required check was skipped, explain why:
Safety
Notes
gittensory-api), which auto-deploys on merge.