Skip to content

fix(orb): do not un-install repos from a page-capped installation-repositories crawl - #10159

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/installed-repos-sync-truncation-10033
Jul 31, 2026
Merged

fix(orb): do not un-install repos from a page-capped installation-repositories crawl#10159
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/installed-repos-sync-truncation-10033

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

syncBrokeredInstalledRepos treats the list from fetchAllInstallationRepos as the complete, authoritative repo set and reconciles destructively against it — every locally-installed repo not in the fresh list is flipped to isInstalled: false with its installationId cleared.

But fetchAllInstallationRepos has two exits the caller can't tell apart:

repos.push(...batch);
if (batch.length < GITHUB_INSTALLATION_REPOS_PAGE_SIZE) break;  // short page → list COMPLETE
// …or the loop simply runs out of pages: page cap hit with a full page still pending → list TRUNCATED

On truncation the reconcile ran anyway: every locally-installed repo beyond item 5,000 (50 pages × 100) is absent from freshFullNames, so markRepositoriesRemovedFromInstallation un-installs it. Per this file's header, every core feature is gated on isInstalled, so those repos go silently dark — and nothing self-heals.

This is the only one of the four paginated GitHub crawls in the codebase that performs a destructive write off a possibly-truncated read, and the only one with no capped/inconclusive signal (listMigrationFilenamesAtRef returns null, fetchPullRequestFiles warns and holds, fetchPagedSegment records a "capped" status).

The fix

  • fetchAllInstallationRepos now returns { repos, truncated }, where truncated is true exactly when the loop exhausts MAX_INSTALLATION_REPOS_PAGES with a full last page, and false on a short-page exit.
  • syncBrokeredInstalledRepos skips markRepositoriesRemovedFromInstallation entirely when truncated — a partial list is valid positive evidence (still upserts every fetched repo) but not valid negative evidence. Mirrors listMigrationFilenamesAtRef's "a bound-hit is inconclusive" posture.
  • A truncated sync still returns { status: "synced", …, removedCount: 0 } (the cron must not read it as an outage) and emits one structured console.error (event: "installed_repos_sync_truncated", with installationId and repoCount) so an operator sees the suppressed reconcile.

Unchanged: MAX_INSTALLATION_REPOS_PAGES (50), GITHUB_INSTALLATION_REPOS_PAGE_SIZE (100), the isOrbBrokerModeskipped early return, the thrown-error → failed path, the InstalledReposSyncResult union members, and the complete-crawl upsert-and-reconcile behaviour including removedCount.

Tests (test/unit/orb-installed-repos-sync.test.ts)

  • fetchAllInstallationRepos reports truncated: true (5,000 repos) for 50 full pages; truncated: false for 100-then-40 (140 repos) and 100-then-0 (100 repos).
  • A truncated crawl calls markRepositoriesRemovedFromInstallation zero times (spied) but upsertRepositoryFromGitHub once per fetched repo (5,000).
  • The truncated result is { status: "synced", installationId, repoCount: 5000, removedCount: 0 } with one structured console.error.
  • REGRESSION: a repo installed by an earlier complete sync, absent from a later truncated crawl, stays isInstalled: true — not un-installed.
  • The existing complete-crawl removal test (a repo GitHub stops returning IS un-installed, removedCount: 1) is preserved unchanged.
  • All new assertions fail on main.

Validation

  • Diff coverage on src/orb/installed-repos-sync.ts is 100% line and branch (both arms of the new truncation branch, plus the preserved !res.ok / isOrbBrokerMode paths).
  • npm run typecheck clean; npm run engine-parity:drift-check passes (host-only file, not a twin); npm run dead-exports:check clean; the suite (11 tests) green.
  • git diff --check clean; no schema/migration/generated-artifact change.

Closes #10033

…ositories crawl

syncBrokeredInstalledRepos reconciled destructively against fetchAllInstallationRepos'
list, but that loop exits either on a short page (list complete) OR on hitting
MAX_INSTALLATION_REPOS_PAGES with a full page still pending (list truncated). On
truncation the reconcile ran anyway: every locally-installed repo beyond item 5,000
was absent from the fresh set, so markRepositoriesRemovedFromInstallation flipped it
to isInstalled: false and cleared its installationId -- and since every core feature is
gated on isInstalled, those repos went silently dark with no self-heal.

Make fetchAllInstallationRepos report a truncation flag (true only when the page cap is
exhausted with a full last page) and skip markRepositoriesRemovedFromInstallation
entirely when truncated -- a partial list is valid positive evidence (still upsert every
fetched repo) but not valid negative evidence. A truncated sync still returns 'synced'
(the cron must not read it as an outage) with removedCount 0, and logs one structured
error so the suppressed reconcile is observable. Mirrors listMigrationFilenamesAtRef's
'a bound-hit is inconclusive' posture. A complete crawl upserts and reconciles exactly
as before; MAX_INSTALLATION_REPOS_PAGES and the result union members are unchanged.

Closes JSONbored#10033
@shin-core
shin-core requested a review from JSONbored as a code owner July 31, 2026 10:02
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 10:20:27 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR fixes a real correctness bug: `fetchAllInstallationRepos` now distinguishes a page-capped (truncated) crawl from a genuinely complete list, and `syncBrokeredInstalledRepos` skips the destructive reconcile when truncated so installed repos beyond the 5,000-item page cap don't get silently un-installed. The truncation detection logic (`page === MAX_INSTALLATION_REPOS_PAGES` combined with a full-size last batch) is correct, and the test suite directly exercises the truncated-vs-complete boundary including the regression scenario described in the PR body. The `console.error` call on line 83 is intentional structured logging for operator visibility, not a debug leftover, despite being flagged by the automated scan.

Nits — 4 non-blocking
  • The truncation flag is only set when the very last page (page 50) is full; if GitHub returns an error or a network hiccup causes an early full-page loop exit before page 50, this wouldn't be caught, but that's an existing design constraint not introduced by this diff.
  • The exported `fetchAllInstallationRepos` return type is now a wider public API surface for this module — verify no other caller assumed the old `GitHubRepositoryPayload[]` return shape.
  • Consider adding a comment near `MAX_INSTALLATION_REPOS_PAGES` cross-referencing the new truncation semantics for future readers who only see that constant.
  • The `installed_repos_sync_truncated` log line could include a monitoring/alert hook reference so operators know where to look when this fires in production.

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 #10033
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: 78 registered-repo PR(s), 60 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 78 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
fetchAllInstallationRepos now returns {repos, truncated} with truncated true only when the page cap is exhausted on a full last page, and syncBrokeredInstalledRepos skips markRepositoriesRemovedFromInstallation while still upserting and returning status:synced with removedCount:0 and a structured console.error, matching all stated requirements.

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, Solidity, Dart, Python, CSS, PHP, Rust
  • Official Gittensor activity: 78 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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

@superagent-security

Copy link
Copy Markdown
Contributor

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

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

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.44%. Comparing base (7075e97) to head (60a2ed7).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10159   +/-   ##
=======================================
  Coverage   80.43%   80.44%           
=======================================
  Files         282      283    +1     
  Lines       58744    58773   +29     
  Branches     6963     6971    +8     
=======================================
+ Hits        47248    47277   +29     
  Misses      11205    11205           
  Partials      291      291           
Flag Coverage Δ
backend 100.00% <100.00%> (?)

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

Files with missing lines Coverage Δ
src/orb/installed-repos-sync.ts 100.00% <100.00%> (ø)

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 55cbeb9 into JSONbored:main Jul 31, 2026
8 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(installed-repos-sync): do not un-install repos from a page-capped installation-repositories list

1 participant