Skip to content

feat(orb): add a typed config-push write path to the relay pipeline#7611

Merged
JSONbored merged 3 commits into
mainfrom
feat/config-push-write-path
Jul 21, 2026
Merged

feat(orb): add a typed config-push write path to the relay pipeline#7611
JSONbored merged 3 commits into
mainfrom
feat/config-push-write-path

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Adds a kind discriminator column (default 'github_webhook') to orb_relay_pending, migration 0168_orb_relay_pending_kind.sql — every existing/future webhook row is untouched.
  • New enqueueConfigPushRelay (src/orb/relay.ts) writes an operator-addressed config_push payload ({ pushId, message, capability?, deprecatesAt? }) into the same queue the webhook relay uses, sharing its TTL pruning and per-installation backlog cap but skipping the webhook-only coalesce-key derivation entirely.
  • New POST /v1/app/fleet/config-push endpoint, gated via requireAppRole(c, ["operator"]) and a .strict() zod body schema, mirroring the kill-switch endpoint's exact auth/validation pattern. Accepts an explicit installationIds list (no percentage/canary selector — none exists in this codebase to build on) and enqueues one row per target, audited as operator.config_push_enqueued.

Placed under /v1/app/* rather than the issue's illustrative /v1/internal/* example path — that prefix's own middleware requires a bearer INTERNAL_JOB_TOKEN and (per requiresApiToken's explicit /v1/internal/ exclusion) never resolves a session identity there at all, which would make requireAppRole's session-role branch unreachable dead code for a control-panel caller. /v1/app/* is where that pattern is actually meaningful, matching the kill-switch endpoint's own placement (a bearer token caller still passes requireAppRole's non-session branch either way).

Write side only, per the issue's explicit scope: does not touch enqueueWebhookByEnv or drainOrbRelayWithMonitor — the dispatch/read side is the companion issue #7523.

Test plan

  • npm run typecheck
  • npm run db:migrations:check / npm run db:schema-drift:check
  • npm run ui:openapi:check (no surface change — internal-only route, not documented, matching the kill-switch endpoint's own precedent)
  • New test/unit/routes-config-push.test.ts: multi-target enqueue, idempotency per (pushId, installation), a pre-existing webhook row's kind stays 'github_webhook' untouched, 400 on schema-invalid/malformed body, 403/401 auth matrix (non-operator session, no identity, shared MCP token), operator-session success + audit row
  • 100% patch-line coverage on the diff (verified via lcov cross-reference against origin/main)
  • Full unsharded suite green aside from two known, pre-existing, unrelated flakes (local-branch.test.ts's defaultBaseRef test and miner-attempt-worktree.test.ts's real-git-subprocess test — both reproducibly pass in isolation; resource-contention flakes under full-parallel load, confirmed unrelated to this diff)

Closes #7522

@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 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.51%. Comparing base (5420570) to head (5d0e650).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7611   +/-   ##
=======================================
  Coverage   88.50%   88.51%           
=======================================
  Files         724      724           
  Lines       75961    75992   +31     
  Branches    22612    22617    +5     
=======================================
+ Hits        67232    67263   +31     
  Misses       7681     7681           
  Partials     1048     1048           
Flag Coverage Δ
shard-1 28.49% <5.71%> (+1.57%) ⬆️
shard-2 33.93% <48.57%> (-0.35%) ⬇️
shard-3 35.04% <8.57%> (+0.17%) ⬆️
shard-4 42.75% <5.71%> (-1.67%) ⬇️
shard-5 34.55% <45.71%> (+1.99%) ⬆️
shard-6 29.72% <14.28%> (-1.32%) ⬇️

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.09% <100.00%> (+0.02%) ⬆️
src/db/schema.ts 72.97% <ø> (ø)
src/orb/broker-client.ts 99.14% <100.00%> (ø)
src/orb/relay.ts 100.00% <100.00%> (ø)
src/selfhost/metrics.ts 100.00% <ø> (ø)
src/selfhost/monitored-work.ts 100.00% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-21 04:12:16 UTC

11 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 write-only config-push path: a `kind` discriminator column (default 'github_webhook', migration 0168), a new `enqueueConfigPushRelay` that reuses the same `orb_relay_pending` queue/TTL/backlog-cap infra as the webhook path but skips coalesce-key derivation, and a `/v1/app/fleet/config-push` operator-gated endpoint that enqueues one row per installation with idempotency keyed on `(pushId, installationId)`. The migration is additive and default-preserving so existing webhook rows are untouched, and the endpoint mirrors the kill-switch route's auth/validation pattern with solid test coverage (idempotency, auth matrix, invalid body, legacy row untouched). The main issue is that the route fans out `enqueueConfigPushRelay` over up to 500 installationIds via `Promise.all`, and that function itself calls the global `pruneRelayPending(env)` on every invocation — so a single request can trigger up to 500 redundant prune scans/deletes against the shared table.

Blockers

  • src/api/routes.ts (config-push handler) calls `enqueueConfigPushRelay` inside `installationIds.map(...)` via `Promise.all`, and `enqueueConfigPushRelay` (src/orb/relay.ts) unconditionally calls `pruneRelayPending(env)` on every invocation — a single request with up to 500 installationIds triggers up to 500 redundant global TTL-prune DB scans/deletes instead of one, an N+1 pattern introduced by looping a function designed for single-webhook calls.
Nits — 5 non-blocking
  • The magic numbers `500` (max installationIds) and `120` (pushId/capability length caps) in the new `configPushSchema` (src/api/routes.ts:1024-1027) aren't named constants — consider extracting them for clarity/reuse.
  • `enqueueConfigPushRelay`'s per-installation DELETE-prune query (src/orb/relay.ts) duplicates the backlog-cap SQL shape from the webhook path inline rather than sharing a helper — minor DRY concern.
  • The handler's `Promise.all` (src/api/routes.ts) has no partial-failure handling — if one installation's insert throws, the whole request 500s with no indication of which targets already succeeded; worth a comment noting this is accepted behavior.
  • Hoist a single `await pruneRelayPending(env)` call to the top of the route handler (or before the `Promise.all`) and have `enqueueConfigPushRelay` accept an option to skip the internal prune, so the fan-out doesn't multiply prune calls.
  • Consider extracting `500`/`120` into named constants near `configPushSchema` (src/api/routes.ts) for readability and to match convention with `MAX_ORB_RELAY_REGISTER_BODY_BYTES` above it.

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 #7522
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: 23 registered-repo PR(s), 16 merged, 358 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 23 PR(s), 358 issue(s).
Improvement ✅ Minor risk: clean · value: minor
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: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 23 PR(s), 358 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: 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.
🧪 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

Adds a discriminator column (kind, default 'github_webhook') to
orb_relay_pending so the pull-drain loop can eventually tell an
operator-addressed config-push notice apart from a GitHub webhook row
(dispatch side is a separate follow-up). A new POST /v1/app/fleet/config-push
endpoint lets an operator enqueue a typed { pushId, message, capability?,
deprecatesAt? } payload for an explicit list of installation_ids, mirroring
the kill-switch endpoint's requireAppRole + zod-validated-body pattern.

Placed under /v1/app/*, not /v1/internal/* (the latter's own middleware
requires a bearer INTERNAL_JOB_TOKEN and never resolves a session identity
for it, which would make requireAppRole's session-role branch unreachable
for a control-panel caller -- see the route's own doc comment).

Write side only: enqueueConfigPushRelay shares the same TTL pruning and
per-installation backlog cap as the existing webhook path, but skips its
GitHubWebhookPayload-shaped coalesce-key derivation entirely (a config_push
payload never has that shape). No existing webhook-row behavior changes.

Closes #7522
…-webhook queue (#7615)

* feat(orb): dispatch config_push relay rows separately from the GitHub-webhook queue

drainOrbRelayWithMonitor's per-event loop now branches on kind BEFORE
calling args.enqueue (which unconditionally does JSON.parse(rawBody) as
GitHubWebhookPayload): a 'config_push' row routes to a new dedicated
handleConfigPushRelayEvent (receive-and-log only, per #4902's v1 scope --
no capability-toggle or config-mutation side effect implemented here).
Everything else -- 'github_webhook' and any old-shaped/missing kind --
falls through to the existing path unchanged.

Threads `kind` through the read side that feeds the drain loop:
pullRelayPending's SELECT + RelayPendingEvent, the /v1/orb/relay/pull
response (no code change needed there -- it just returns pullRelayPending's
output), and drainOrbRelay's HTTP response parsing (defaults a missing
`kind` to 'github_webhook' for a rolling deploy against an older Orb
server).

Closes #7523

* chore(orb): register the config_push relay metric

selfhost-metrics.test.ts scans every incr() call site for its metric
name and cross-checks it against DEFAULT_METRIC_META -- loopover_orb_config_push_received_total
(introduced in the previous commit) was missing.

* test(selfhost): cover the non-Error branch of the config_push throw log

Closes the codecov/patch gap on PR #7615 -- handleConfigPushRelayEvent's
error-logging ternary (error instanceof Error ? error.message : String(error))
only had its Error-instance arm exercised.
…t per target

enqueueConfigPushRelay pruned the shared orb_relay_pending table on every
call, but the route fans it out over up to 500 installationIds via
Promise.all -- turning one request into up to 500 redundant global
TTL-prune scans/deletes. Moves the prune to the route handler, called
once before the fan-out; enqueueConfigPushRelay no longer prunes itself.
@JSONbored
JSONbored force-pushed the feat/config-push-write-path branch from 4fe99ca to 5d0e650 Compare July 21, 2026 04:04
@JSONbored
JSONbored merged commit 590a8ef into main Jul 21, 2026
18 checks passed
@JSONbored
JSONbored deleted the feat/config-push-write-path branch July 21, 2026 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(orb): add a typed config-push write path to the relay pipeline (migration + endpoint)

1 participant