Skip to content

fix(miner): scope the managed-PR queue row to the polled forge host - #6861

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
jeffrey701:fix-manage-poll-apibaseurl-6764
Jul 17, 2026
Merged

fix(miner): scope the managed-PR queue row to the polled forge host#6861
JSONbored merged 1 commit into
JSONbored:mainfrom
jeffrey701:fix-manage-poll-apibaseurl-6764

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Closes #6764

Problem

ensureManagedPrRow (packages/loopover-miner/lib/manage-poll.js) enqueues { repoFullName, identifier, priority: 0 } with no apiBaseUrl, so portfolio-queue.js's normalizeApiBaseUrl silently defaults the row to github.com — even when the poll two lines away ran against a different forge host (recordManagePollSnapshot already threads options.apiBaseUrl into pollCheckRuns). The queue's composite (api_base_url, repo_full_name, identifier) primary key exists specifically so two hosts serving the same owner/repo name never collide (#5563); this write path violates that invariant.

The "already exists" check is forge-blind for the same reason: listQueue(repoFullName) returns rows across every host, so the same repo + PR-number already tracked on another host suppresses row creation for this one entirely.

Fix

  • Thread options.apiBaseUrl from recordManagePollSnapshot into ensureManagedPrRow and pass it to enqueue, so the row is scoped to the host it was actually polled from. enqueue still performs its own normalization/validation.
  • Scope the existence check to the same forge host, comparing through a small resolveManagedRowApiBaseUrl fold that mirrors portfolio-queue-manager.js's existing one (omitted/blank → the github.com default, so every pre-existing single-forge caller is byte-identical).

Tests

Two regression tests in test/unit/miner-manage-poll.test.ts (which previously had zero apiBaseUrl references), both failing before the fix:

  1. polling a non-default apiBaseUrl → the resulting queue row is scoped to that host (before: silently github.com);
  2. the exact reported failure mode — a same-named repo + PR already queued on another host no longer suppresses this host's row (before: expected length 2 but got 1).

Validation

  • npx vitest run test/unit/miner-manage-poll.test.ts11/11 pass (9 existing + 2 new)
  • npm run typecheckclean (0 errors)
  • Scope: 1 source file + 1 test file

@jeffrey701
jeffrey701 requested a review from JSONbored as a code owner July 17, 2026 09:07
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.64%. Comparing base (fcf41bc) to head (492aef3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6861   +/-   ##
=======================================
  Coverage   93.64%   93.64%           
=======================================
  Files         685      685           
  Lines       68239    68241    +2     
  Branches    18707    18707           
=======================================
+ Hits        63904    63906    +2     
  Misses       3350     3350           
  Partials      985      985           
Flag Coverage Δ
shard-1 43.23% <0.00%> (-0.46%) ⬇️
shard-2 36.93% <0.00%> (+0.25%) ⬆️
shard-3 32.60% <0.00%> (-0.12%) ⬇️
shard-4 34.37% <100.00%> (-0.39%) ⬇️
shard-5 31.47% <0.00%> (+0.32%) ⬆️
shard-6 45.97% <0.00%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-miner/lib/manage-poll.js 97.93% <100.00%> (+0.04%) ⬆️

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 17, 2026
@loopover-orb

loopover-orb Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-17 09:18:36 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This fix threads `options.apiBaseUrl` through `ensureManagedPrRow` and scopes the existence check by resolved host, directly closing the reported bug where a managed-PR queue row silently defaulted to github.com and where a same repo+PR on another host suppressed row creation. The fold in `resolveManagedRowApiBaseUrl` mirrors the existing convention in `portfolio-queue-manager.js`, and the two new regression tests exercise the exact reported failure modes (host scoping and cross-host suppression) rather than fabricated states. Both tests fail before the fix per the PR description and pass after, and typecheck/full suite are green.

Nits — 3 non-blocking
  • manage-poll.js: `resolveManagedRowApiBaseUrl` duplicates logic that already exists in portfolio-queue-manager.js — consider extracting/importing a shared helper instead of maintaining two copies of the same fold.
  • The block comments above `resolveManagedRowApiBaseUrl` and `ensureManagedPrRow` are quite long for what the code does; could be trimmed to a single line each per repo comment conventions.
  • Consider exporting the existing fold from portfolio-queue-manager.js and importing it in manage-poll.js instead of redefining `resolveManagedRowApiBaseUrl`, to avoid drift if the default host ever changes.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6764
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High 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: 121 registered-repo PR(s), 59 merged, 30 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jeffrey701; Gittensor profile; 121 PR(s), 30 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff threads options.apiBaseUrl from recordManagePollSnapshot into ensureManagedPrRow and passes it to portfolioQueue.enqueue, and scopes the 'already exists' check by comparing resolved apiBaseUrl alongside identifier, matching both stated requirements.

Review context
  • Author: jeffrey701
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, Clojure, JavaScript, Rust
  • Official Gittensor activity: 121 PR(s), 30 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step in the Signals table above.
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.
🧪 Chat with LoopOver

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

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 17, 2026
@JSONbored
JSONbored merged commit de1c473 into JSONbored:main Jul 17, 2026
16 checks passed
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

manage poll's ensureManagedPrRow drops apiBaseUrl, mis-scoping managed-PR queue rows across forge hosts

2 participants