Skip to content

fix(orb): isolate per-installation failures in config-push relay fan-out - #8927

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-8880
Jul 26, 2026
Merged

fix(orb): isolate per-installation failures in config-push relay fan-out#8927
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-8880

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

fix(orb): isolate per-installation failures in config-push relay fan-out

The POST /v1/app/fleet/config-push handler fanned out up to 500
enqueueConfigPushRelay calls inside a bare Promise.all with no per-target
guard. One installation's DB write throwing aborted the whole request --
the audit event and the response for the other ~499 targets were silently
dropped, with no partial-success report.

Wrap each target in its own try/catch (matching pollPendingAprRepoTransfers'
per-item isolation) so one bad row can't sink the batch. Each failure is
audited via a per-installation operator.config_push_target_failed event,
and the summary audit plus the response now report succeededCount and the
failedInstallationIds.

Closes #8880

Validation

Verified locally on this branch before opening:

  • npm run typecheck
  • npx turbo run build:tsc build:verify
  • npm run test:coverage — patch coverage 100.0% of changed lines

The POST /v1/app/fleet/config-push handler fanned out up to 500
enqueueConfigPushRelay calls inside a bare Promise.all with no per-target
guard. One installation's DB write throwing aborted the whole request --
the audit event and the response for the other ~499 targets were silently
dropped, with no partial-success report.

Wrap each target in its own try/catch (matching pollPendingAprRepoTransfers'
per-item isolation) so one bad row can't sink the batch. Each failure is
audited via a per-installation operator.config_push_target_failed event,
and the summary audit plus the response now report succeededCount and the
failedInstallationIds.

Closes JSONbored#8880
@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 26, 2026 13:41
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.01%. Comparing base (64383b5) to head (ca6329c).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8927      +/-   ##
==========================================
+ Coverage   90.56%   91.01%   +0.45%     
==========================================
  Files          96       97       +1     
  Lines       22490    24940    +2450     
  Branches     3884     4673     +789     
==========================================
+ Hits        20367    22700    +2333     
- Misses       1945     1975      +30     
- Partials      178      265      +87     
Flag Coverage Δ
backend 95.22% <100.00%> (?)

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

Files with missing lines Coverage Δ
src/api/routes.ts 95.22% <100.00%> (ø)

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

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-26 13:48:12 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR wraps each per-installation enqueueConfigPushRelay call in its own try/catch instead of a bare Promise.all, so one throwing installation no longer aborts the whole fan-out — each failure is audited individually via a new operator.config_push_target_failed event, and the route response/summary audit now report succeededCount and failedInstallationIds. The fix matches the described pollPendingAprRepoTransfers per-item isolation pattern, and the new test (mocking enqueueConfigPushRelay to throw for installation 222) verifies the other targets still land, the failure is audited, and the response/summary reflect the partial failure. This closes #8880 and is a narrow, well-targeted change with no schema or config-as-code surface touched.

Nits — 3 non-blocking
  • The per-failure recordAuditEvent calls in the for-loop (src/api/routes.ts, inside the new settled loop) run sequentially awaited rather than in parallel — fine at the expected scale of rare failures, but worth a comment if failure counts could ever be large.
  • The new audit event type 'operator.config_push_target_failed' has no apparent enum/type constraint shown in the diff — confirm event_type is a free-form string column and not a checked enum that would need a migration.
  • Consider capturing failedInstallationIds order guarantees explicitly in a comment near the settled loop, since the test asserts an exact array order that depends on Promise.all preserving input order (which it does, but it's an easy invariant to break if someone switches to allSettled with a different mapping).

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 #8880
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: 41 registered-repo PR(s), 32 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 41 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The diff replaces the bare Promise.all with per-installation try/catch isolation, records a per-installation operator.config_push_target_failed audit event for each failure, and reports succeededCount/failedInstallationIds in both the summary audit and response, matching the requested pattern.

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript
  • Official Gittensor activity: 41 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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 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 546b260 into JSONbored:main Jul 26, 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.

fix(orb): config-push relay fan-out has no per-target failure isolation, unlike every other batch loop in this subsystem

1 participant