fix(orb): do not un-install repos from a page-capped installation-repositories crawl - #10159
Conversation
…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
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 10:20:27 UTC
Review summary Nits — 4 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.
|
|
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 #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
Flags with carried forward coverage won't be shown. Click here to find out more.
|
What & why
syncBrokeredInstalledRepostreats the list fromfetchAllInstallationReposas the complete, authoritative repo set and reconciles destructively against it — every locally-installed repo not in the fresh list is flipped toisInstalled: falsewith itsinstallationIdcleared.But
fetchAllInstallationReposhas two exits the caller can't tell apart:On truncation the reconcile ran anyway: every locally-installed repo beyond item 5,000 (50 pages × 100) is absent from
freshFullNames, somarkRepositoriesRemovedFromInstallationun-installs it. Per this file's header, every core feature is gated onisInstalled, 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 (
listMigrationFilenamesAtRefreturnsnull,fetchPullRequestFileswarns and holds,fetchPagedSegmentrecords a"capped"status).The fix
fetchAllInstallationReposnow returns{ repos, truncated }, wheretruncatedistrueexactly when the loop exhaustsMAX_INSTALLATION_REPOS_PAGESwith a full last page, andfalseon a short-page exit.syncBrokeredInstalledReposskipsmarkRepositoriesRemovedFromInstallationentirely when truncated — a partial list is valid positive evidence (stillupserts every fetched repo) but not valid negative evidence. MirrorslistMigrationFilenamesAtRef's "a bound-hit is inconclusive" posture.{ status: "synced", …, removedCount: 0 }(the cron must not read it as an outage) and emits one structuredconsole.error(event: "installed_repos_sync_truncated", withinstallationIdandrepoCount) so an operator sees the suppressed reconcile.Unchanged:
MAX_INSTALLATION_REPOS_PAGES(50),GITHUB_INSTALLATION_REPOS_PAGE_SIZE(100), theisOrbBrokerMode→skippedearly return, the thrown-error →failedpath, theInstalledReposSyncResultunion members, and the complete-crawl upsert-and-reconcile behaviour includingremovedCount.Tests (
test/unit/orb-installed-repos-sync.test.ts)fetchAllInstallationReposreportstruncated: true(5,000 repos) for 50 full pages;truncated: falsefor 100-then-40 (140 repos) and 100-then-0 (100 repos).markRepositoriesRemovedFromInstallationzero times (spied) butupsertRepositoryFromGitHubonce per fetched repo (5,000).{ status: "synced", installationId, repoCount: 5000, removedCount: 0 }with one structuredconsole.error.isInstalled: true— not un-installed.removedCount: 1) is preserved unchanged.main.Validation
src/orb/installed-repos-sync.tsis 100% line and branch (both arms of the new truncation branch, plus the preserved!res.ok/isOrbBrokerModepaths).npm run typecheckclean;npm run engine-parity:drift-checkpasses (host-only file, not a twin);npm run dead-exports:checkclean; the suite (11 tests) green.git diff --checkclean; no schema/migration/generated-artifact change.Closes #10033