Skip to content

fix(review): velocity-aware exception in the unlinked-issue-match escalation for machine-cadence (miner) authors#4543

Merged
JSONbored merged 1 commit into
mainfrom
feat/unlinked-issue-velocity-exception
Jul 10, 2026
Merged

fix(review): velocity-aware exception in the unlinked-issue-match escalation for machine-cadence (miner) authors#4543
JSONbored merged 1 commit into
mainfrom
feat/unlinked-issue-velocity-exception

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • resolveUnlinkedIssueMatchDisposition's repeat-escalation (a confirmed unlinked-issue match by the same author, correlated across every repo in the install within a 90-day window) straight-closes on a SECOND occurrence, calibrated for human abuse cadence — a genuine tooling bug in a miner's own PR-body template could recur within hours, converting one bug into a cross-repo close of otherwise-correct work.
  • Adds a velocity exception gated on CONFIRMED official-miner identity (via a new, minimal isConfirmedOfficialMiner cached lookup, independent of processors.ts's version to avoid a circular import): a repeat within the last hour from a verified miner holds again (distinct message) instead of closing. An unverified account repeating just as fast still escalates to close exactly as before — deliberately NOT a blanket "fast repeats are fine" loophole, since a fast-and-unverified repeat is the more suspicious case, not less.
  • Added mostRecentAuditEventForOtherTarget (timestamp-returning sibling of the existing hasRecentAuditEventForOtherTarget) since the velocity check needs to know how recently the prior match happened, not just whether one exists.

Test plan

  • test/unit/unlinked-issue-guardrail.test.ts — 7 new tests: velocity exception holds within the window, still closes once the gap exceeds it, does NOT apply to an unconfirmed/not-found author, does NOT apply when the Gittensor API itself is unavailable (fail-safe), a third same-miner match hits the detection cache instead of re-fetching, and both cache-read/cache-write failure paths fail safe
  • All 16 pre-existing tests in the file still pass unchanged
  • 100% branch / 100% line coverage on unlinked-issue-guardrail.ts
  • npx tsc --noEmit clean (one pre-existing, unrelated error in miner-opportunity-ranker.test.ts from upstream drift, not touched by this PR)

Fixes #4512

…alation for machine-cadence (miner) authors

The repeat-escalation path was calibrated for human abuse cadence: a
confirmed repeat by the same author within 90 days, correlated across
every repo in the install, straight-closes on the theory that a human
is unlikely to repeat the exact same "forgot to link the issue"
mistake twice in that window. A systematic bug in a miner's own
PR-body template could recur within hours rather than months,
converting one tooling bug into cross-repo closure of otherwise-
correct, properly-targeted work.

Adds a velocity exception gated on CONFIRMED official-miner identity,
not on speed alone: a repeat within the last hour from a verified
Gittensor miner holds again (with a distinct message) instead of
closing, while an unverified account repeating just as fast still
escalates to close exactly as before -- fast-and-unverified is the
MORE suspicious case, not less, so leniency only applies where an
independent identity check backs it.

Fixes #4512
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 10, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.04%. Comparing base (95e9ec6) to head (bcbe525).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4543      +/-   ##
==========================================
- Coverage   94.04%   94.04%   -0.01%     
==========================================
  Files         422      422              
  Lines       37600    37617      +17     
  Branches    13736    13740       +4     
==========================================
+ Hits        35362    35376      +14     
- Misses       1583     1586       +3     
  Partials      655      655              
Files with missing lines Coverage Δ
src/db/repositories.ts 96.56% <100.00%> (-0.17%) ⬇️
src/review/unlinked-issue-guardrail.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-10 00:46:50 UTC

3 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a narrow, identity-gated velocity exception to the unlinked-issue repeat-escalation logic in resolveUnlinkedIssueMatchDisposition (src/review/unlinked-issue-guardrail.ts): a repeat match within 60 minutes from a CONFIRMED official miner now holds with a distinct message instead of auto-closing, while an unconfirmed/unverified author repeating just as fast still escalates to close exactly as before. The new isConfirmedOfficialMiner lookup reuses the existing official_miner_detections cache table/TTLs (via getFreshOfficialMinerDetection/upsertOfficialMinerDetection, not newly added here) and is fail-safe on every error path (cache read failure falls back to a fresh fetch, cache write failure still uses the freshly-fetched status, API unavailability never resolves to confirmed). The new mostRecentAuditEventForOtherTarget repository function is a straightforward timestamp-returning sibling of the existing boolean check, and the gapMs >= 0 guard correctly excludes clock-skew edge cases from ever granting leniency. Tests are thorough — they cover the window boundary, the confirmed/unconfirmed split, both cache failure modes, API unavailability, and cache hit on a third call — and none of them fabricate an unreachable state; they exercise the real fetch→cache→resolve path. No schema/migration change is needed since the miner-detection cache table already exists for processors.ts's own use.

Nits — 6 non-blocking
  • src/review/unlinked-issue-guardrail.ts: OFFICIAL_MINER_DETECTION_TTL_MS and OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS are duplicated literals from processors.ts's own miner-detection cache TTLs — if that TTL ever changes there, this file silently drifts out of sync; consider hoisting both constants into db/repositories.ts (or a shared constants module) so both callers import the same value.
  • src/review/unlinked-issue-guardrail.ts: the close-path reason/comment gives no signal that a velocity exception was evaluated and rejected (e.g. unconfirmed author, gap too large, API unavailable) — a one-word audit-log tag on which branch fired would make future incident triage easier without changing contributor-facing text.
  • src/db/repositories.ts: mostRecentAuditEventForOtherTarget duplicates hasRecentAuditEventForOtherTarget's query almost verbatim aside from the select/limit/orderBy — consider having the boolean variant call the timestamp variant (`(await mostRecentAuditEventForOtherTarget(...)) !== null`) to avoid maintaining two near-identical WHERE clauses.
  • Consider hoisting the shared TTL constants (nit above) into repositories.ts next to getFreshOfficialMinerDetection/upsertOfficialMinerDetection so processors.ts and this file can't drift.
  • A short structured-log line (mirroring the linked-issue-label-propagation module's `console.log(JSON.stringify({event: ...}))` convention) when the velocity exception is evaluated and rejected would help future debugging of miner-identity edge cases.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4512
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 48 registered-repo PR(s), 40 merged, 363 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 363 issue(s).
Gate result ✅ Passing No configured blocker found.
Linked issue satisfaction

Partially addressed
The PR adds a well-tested, narrowly-scoped velocity exception gated on confirmed miner identity plus a sub-hour gap, with an unconfirmed/fail-safe path that still escalates to close, directly fulfilling the issue's core ask and the majority of its test-coverage requirements. However, the diff shows no changes to the guardrail's config documentation, leaving the issue's explicit 'Docs: note the cad

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 48 PR(s), 363 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 10, 2026
@JSONbored
JSONbored merged commit c6a5504 into main Jul 10, 2026
11 checks passed
@JSONbored
JSONbored deleted the feat/unlinked-issue-velocity-exception branch July 10, 2026 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

fix(review): velocity-aware exception in the unlinked-issue-match escalation for machine-cadence (miner) authors

1 participant