orb(mcp): the ORB management tool family — ops, fleet, hosted tenants, instance diagnostics (#9522) - #9604
Merged
Merged
Conversation
Contributor
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-29 02:53:49 UTC
Review summary Nits — 7 non-blocking
CI checks failing
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. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 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.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
loopover-ui | 8b5c4e8 | Jul 29 2026, 01:23 AM |
Bundle ReportChanges will increase total bundle size by 3.13kB (0.04%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9604 +/- ##
==========================================
+ Coverage 90.02% 90.05% +0.03%
==========================================
Files 888 896 +8
Lines 111983 112372 +389
Branches 26570 26634 +64
==========================================
+ Hits 100810 101197 +387
- Misses 9843 9844 +1
- Partials 1330 1331 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The management capabilities existed; they were just scattered across interfaces an agent could not reach uniformly. This adds the ops family (dead-letter list/replay/delete/purge, kill-switch get/set, operator dashboard) and the fleet family (instances, installations, enrollments), each wrapping the SAME service function its HTTP route calls so the two transports cannot drift into two behaviors with one audit trail between them. The instance and installation handlers lived inline in routes.ts, so they move to src/orb/fleet-admin.ts and both transports now call that. auth is enforced per tool from the registry's own declaration: requireOperator mirrors requireAppRole(["operator"]) and deliberately refuses the shared LOOPOVER_MCP_TOKEN, which is an end-user-obtainable CLI credential; requireInternal mirrors the /v1/internal/* middleware's bearer check. Destructive tools take a z.literal(true) confirm -- a plain boolean would let an omitted field read as false and pass -- and, where the client supports elicitation, ask before acting. A decline is a structured non-error result, and an elicitation that errors is treated as a decline rather than an implicit yes. Releasing the kill switch is the direction that needs the ceremony; engaging it is fail-safe and must not be slowed down. Two findings fixed in passing. MCP_TOOL_CATEGORIES was a hand-maintained 112-entry name-to-category map beside a registry that already carried category on every tool; it now derives, which is also what lets this issue's three new categories land in one place. Its exact-sync test loses the stale-entry half, which only had meaning while the map was hand-kept -- the map now indexes all three servers' tools, since locality says where a tool's work happens rather than which server exposes it, and a dozen local-git tools are registered on the remote server too. The half that actually caught drift, every registered tool resolving a category, stays. The map had already fallen a tool behind: loopover_admin_rotate_secret is live and tested but had no contract entry, so validate:mcp never saw it -- now migrated, with its secret name as a closed enum rather than an open string, since rotation writes to a host path. loopover_fleet_get_analytics is deliberately not added: loopover_get_fleet_analytics already wraps computeFleetAnalytics behind the same gate. Two names for one capability is how a catalog rots. The incumbent keeps its name and moves into the fleet category.
The first pass excluded three job routes as "not valid queue message types" and left them HTTP-only. That was a wrong reading of a real finding: nothing is invalid about them, they are PARAMETERIZED, and two simply do not use their route path as their message type. rag-index enqueues rag-index-repo, regate-pr enqueues agent-regate-pr, and backfill-contributor-gate-history and refresh-installation-health have no queue message at all because they are run-only. Deriving the message from the job name -- which the first pass would have done for the seventeen it kept -- would have silently enqueued messages the dispatcher drops for the two renamed ones. INTERNAL_JOB_SPEC now records, per job, the queue message type it really sends (null when run-only) and the modes it really offers. The tool forwards a payload, dispatches enqueue through JOBS.send and run through processJob -- the same dispatcher the queue consumer uses, so an inline run and a queued run cannot diverge -- and routes the two run-only jobs to the same functions their own /run routes call. All 20 are reachable; none are excluded. The parity test now asserts the full route table with nothing filtered out, checks each job's modes and its real message type against the route that sends it, and fails if a run-only job has no inline runner wired. The compile-time assertion widened to JobMessage["type"] | null so a bad messageType still fails the build. The management deny-path suite separates two auth contracts that are genuinely different rather than forcing one shape on both: the categorical requireOperator/requireInternal this issue added, and the pre-existing requireOperatorAccess, which honors the established wildcard opt-in (#2455) where an operator explicitly unscoping MCP_READ_REPO_ALLOWLIST declares that token may read everything. The latter is pinned to its real boundary -- a SCOPED allowlist is still refused -- instead of being exempted or "fixed" into a behavior change nobody asked for.
…ient validate:mcp caught the gap this closes: the four tenant tools were in the registry but registered nowhere, which its own assertion calls "a promised capability with nothing behind it". They needed a client the Worker did not have. src/orb/control-plane-client.ts is that client, mirroring the miner's tenant-client posture rather than sharing its code -- the miner reads process.env in a Node CLI, this takes the Worker's Env binding, and the two products' control-plane credentials are separately namespaced on purpose. What is shared is the contract: control-plane/src/http-app.ts owns the wire shapes and both clients speak exactly its routes. Every call fails loud. Tenant create and destroy are deliberate admin actions, so unreachable, non-2xx, and malformed all throw rather than letting a caller read success into something that did not happen; the non-2xx error carries the status only, never the operator-adjacent body. One bounded request per call and no retry, because a create is not idempotent. "Not configured" is answered as a structured result instead, since having no hosted tenants to administer is the normal state for every deployment but one. Also documents the maintenance-job surface: the self-host-maintenance workflow gains no duties and stays the scheduled path, with loopover_fleet_run_job as the interactive one running the identical dispatcher, and the operations doc now shows all three routes to the same job. The extracted fleet-admin functions get direct tests to 100% line and branch, including the `rows.results ?? []` arms that the real in-memory D1 can never reach.
…tenant-list output codecov/patch caught that the deny-path suite only ever reached the auth throw, so every handler's actual behavior was ungraded. These drive each family past the gate: the structured unavailable/not-configured answers that stand in for a capability a deployment lacks, the kill switch's asymmetric confirm, the fleet registry round-trip, both fleet_run_job dispatch paths including the renamed message type, and every instance diagnostic including its reader-throws degradation. That found a real contract defect: TenantListOutput required `tenants`, which the not-configured answer has no list to supply, so a perfectly valid response failed its own output schema with -32602. Made optional alongside an explicit `configured`.
codecov/patch was 86.29% with 58 lines missing in src/mcp/server.ts. The deny-path and not-configured suites reached every guard but almost none of the work behind them: the dead-letter tools all answered "unavailable" because a test JOBS binding has no dead-letter admin, and the tenant tools all answered "not configured". These supply the missing halves — a JOBS binding with the self-host queue's admin surface, a stubbed control plane, seeded installation rows, and a client that actually advertises elicitation. That last one is what finally reaches confirmDestructive: without the capability the server falls through on the schema-level confirm alone, so the decline path and the "an elicitation that errors is a decline, not an implicit yes" rule had no test. One real gap fell out of it. FleetRegisterInstallationInput never declared `registered`, so the opt-OUT half of the onboarding gate was unreachable over MCP -- an operator could register an installation but never un-register it, even though the route supports both. Declared, with the description spelling out what opting out actually does. loopover_fleet_backfill_installations' success return stays uncovered: reconciling against GitHub needs real Orb App credentials, and faking them to reach one return statement would buy a covered line at the cost of a test that asserts nothing. Its failure path is covered instead, which is the behavior that matters -- it fails loud rather than reporting a backfill that never ran.
…ement families #9521 landed its generator, so the two READMEs and the UI tool-reference module are now generated from the contract registry. The 25 tools this branch adds flow through it with no hand-edits, which is what that issue built it for.
JSONbored
force-pushed
the
feat/orb-management-tools-9522
branch
from
July 29, 2026 01:18
59b778b to
8b5c4e8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9522.
What changed
The ORB management capabilities existed; they were scattered across interfaces an agent could not reach uniformly. This adds 25 tools across four families, every one wrapping the same service function its HTTP route calls — one implementation behind two transports, so they cannot drift into two behaviors with one audit trail between them.
Ops (7) — dead-letter list/replay/delete/purge, kill-switch get/set, operator dashboard.
availability: both: on a deployment whose queue backend has no dead-letter admin these answerunavailable: truerather than vanishing from the catalog, because "this backend has no DLQ" is an answer to the question.Fleet (11) — instances, installations, enrollments, config push, and one
loopover_fleet_run_jobreplacing the ~30 bespoke per-job tools a route-by-route mapping would have produced.Hosted tenants (4) — create/list/set-installation/destroy, over a new
src/orb/control-plane-client.ts.Self-host diagnostics (4) —
status,doctor(ORB had none; the miner has had one for years), boundedtail_logs,backup_status, behind a Workers-safe nullable capability registry that only the self-host entry fills.requireOperatormirrorsrequireAppRole(["operator"])and deliberately refuses the sharedLOOPOVER_MCP_TOKEN— an end-user-obtainable CLI credential that must not reach the kill switch.requireInternalmirrors the/v1/internal/*bearer gate. Destructive tools takez.literal(true)(a plain boolean lets an omitted field read as false and pass) and elicit confirmation where the client supports it; a decline is a structured non-error, and an elicitation that errors is treated as a decline rather than an implicit yes. Releasing the kill switch needs the ceremony; engaging it is fail-safe and must not be slowed down.Things this found and fixed
loopover_admin_rotate_secrethad no contract entry. Live, registered, with its own test file — contract(remote): migrate the remote MCP server's tools to @loopover/contract — typed handlers, real output schemas #9518's migration missed exactly one tool, sovalidate:mcpnever saw it. Migrated, with its secret name a closed enum rather than an open string, since rotation writes to a host path.MCP_TOOL_CATEGORIESwas a hand-maintained 112-entry map beside a registry that already carriedcategoryon every tool. Diffed first — zero disagreements — then derived. Its exact-sync test loses the stale-entry half, which only had meaning while the map was hand-kept; the half that actually caught drift stays.rag-index→rag-index-repo,regate-pr→agent-regate-pr) and two more are run-only with no message at all. Deriving the message from the job name — the obvious implementation — silently enqueues messages the dispatcher drops, i.e. a job that reports "queued" and never runs.INTERNAL_JOB_SPECrecords each job's real message type and real modes; a compile-time assertion pins every one toJobMessage["type"] | null, and the parity test checks each against the route that actually sends it.loopover_fleet_get_analyticswas not added:loopover_get_fleet_analyticsalready wraps the same function behind the same gate. Two names for one capability is how a catalog rots. The incumbent keeps its name and moves into thefleetcategory.Deliberately not here
loopover_tenant_rollout. Its route is an explicit 501 whose own comment is blunt about why — the previous implementation no-opped on all four of its promised effects, and "rollback is the control you least want to discover is fake". The image-side mechanism it needs is scoped on #9143 by this issue's own instruction. A tool on top of a route that changes nothing observable would ship exactly the fake control #9143 removed.Validation
Full typecheck clean.
validate:mcppasses — including its assertion that everyremote-locality non-self-host tool is actually served, which is what caught the tenant tools being declared but unregistered before the client existed. 159 MCP/ORB test files pass. Deny-path coverage is generated from the registry (48 assertions), so a new management tool is covered the moment it is added; the pre-existingrequireOperatorAccessfamily is pinned to its real#2455wildcard-opt-in boundary rather than forced into the categorical shape. Extracted code is at 100% line and branch, including therows.results ?? []arms the real in-memory D1 can never reach.