feat(api): drive the route-spec ratchet to zero and delete the baseline (#9531) - #9581
Merged
Conversation
Contributor
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-28 19:09:10 UTC
Review summary Blockers
Nits — 5 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agentCI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
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.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 72de26e | Commit Preview URL Branch Preview URL |
Jul 28 2026, 06:53 PM |
Bundle ReportChanges will increase total bundle size by 120.83kB (1.58%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
JSONbored
force-pushed
the
feat/route-ratchet-9531
branch
from
July 28, 2026 12:21
9b7465f to
115952e
Compare
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (98.83%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #9581 +/- ##
==========================================
- Coverage 89.84% 89.03% -0.81%
==========================================
Files 875 878 +3
Lines 110980 111029 +49
Branches 26402 26409 +7
==========================================
- Hits 99706 98856 -850
- Misses 9992 11155 +1163
+ Partials 1282 1018 -264
Flags with carried forward coverage won't be shown. Click here to find out more.
|
JSONbored
force-pushed
the
feat/route-ratchet-9531
branch
from
July 28, 2026 12:46
115952e to
76b0029
Compare
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
JSONbored
force-pushed
the
feat/route-ratchet-9531
branch
from
July 28, 2026 13:13
76b0029 to
c7dab65
Compare
JSONbored
force-pushed
the
feat/route-ratchet-9531
branch
from
July 28, 2026 13:35
c7dab65 to
e8450af
Compare
The published document described 151 of the 244 operations createApp() actually serves. It now describes all 244, and src/openapi/unspecced-routes-baseline.json is gone -- there is no longer anywhere to record an exception. The 93 newly-described routes include the entire ORB management surface (/v1/orb/*, /v1/internal/orb/*), the fleet kill-switch and config-push, the per-repo BYO-key routes, and the 32 internal job endpoints. Each carries a stable operationId, real tags, and an auth level that DERIVES its security stanza rather than having one bolted on afterwards by path prefix. Three fixes the work forced: - RouteAuth gained 'orb' and 'webhook'. The ORB ingress genuinely does not authenticate the way the rest of the API does, and the old model published it as needing no credential at all. It needs a different one: an ORB-issued bearer for the relay and token endpoints, an HMAC signature header for the webhook. Both now have their own security scheme. - registerRouteSpec never emitted path parameters, so every templated segment it registered was a Cloudflare 30046 schema-validation warning and a hole in any generated client. Derived from the path now, because there is no case where a path parameter is optional. - test/unit/openapi.test.ts asserted /v1/internal/jobs/generate-signal-snapshots /run must be ABSENT from the document as a removed route. It is live. The assertion was requiring the document to stay wrong. The ratchet's other direction earned its keep immediately: it rejected two job routes I had assumed existed, because two of the internal jobs are operator-triggered repairs with a /run form and no enqueue sibling.
The document emitted tags: [] on all 244 operations and an operationId on none, which collapses every one of them into a single flat namespace for a generated client or a doc explorer -- and makes every path edit a breaking rename, since the id would otherwise be slugified from method+path. 126 literal registerPath calls carry both inline. The five families registered from a table derive them in-loop from the same rules, because those tables exist so that adding a sibling route is a one-line change and a second hand-written entry per route would give that up. All 244 now carry both, with no duplicate ids. That is the precondition for deleting isProtectedPath/applySecurityMetadata: the parallel security model can only go once every operation declares enough to derive its own stanza.
…e the parallel model Review blockers on #9581, both correct: the document still emitted the identical LoopOverBearer+SessionCookie pair on every operation regardless of its declared auth -- a bearer requirement on the ORB ingress whose own comment says it takes a shared-secret header, and on public routes that answer 200 anonymously. Two causes, both fixed: - applySecurityMetadata OVERWROTE whatever an operation declared. The seam was producing differentiated stanzas and the post-pass clobbered all of them with the generic pair. It now fills in only operations that declared nothing -- which required `public` to emit `[]` (OpenAPI's explicit "no credential") rather than undefined, so a deliberately open route is distinguishable from a legacy registerPath call that never said. - The fill-in itself now derives from requiresApiToken, the predicate the app gates on, extracted to src/auth/route-auth.ts (pure, no cloudflare: imports, so the plain-Node spec builder can load it). isProtectedPath -- the second, path-prefix model that had already drifted (it published the whole /v1/public/decision-ledger/* family as bearer-gated) -- is deleted. The published stanzas now split six ways instead of one: the generic pair (136), bearer-only internal (63), legacy-undeclared (28), explicitly public (11), the ORB instance bearer (5), and the webhook body signature (1). Deriving from the real gate surfaced one route BOTH former models had wrong: POST /v1/auth/github/token 403s a bearer-only caller (the handler checks identity.kind !== "session"), yet was published as bearer-or-cookie by one model and would be published as open by the other. It now declares LoopOverSessionCookie alone. The new parity meta-test asserts, against the real gate: no ungated route advertises the ordinary credential, no gated route claims to be open, the auth levels stay distinct rather than collapsing back to one stanza, and every referenced scheme is declared. Its own first two drafts each misread an exempt family as ungated -- /v1/internal/* has its own INTERNAL_JOB_TOKEN middleware, and the session-only route above is gated in its handler -- which is recorded in the test because those exemptions ARE the map of where auth is enforced other than the token gate.
JSONbored
force-pushed
the
feat/route-ratchet-9531
branch
from
July 28, 2026 18:32
bce3d09 to
c220fbd
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.
Part of #9531. #9575 and #9579 have merged, so this now stacks directly on . Delivers the ratchet-to-zero deliverable, the ORB surface, / on every operation, and the parallel-security-model deletion; the remaining #9531 deliverables are listed at the bottom and I have not closed the issue.
The ratchet is at zero
The published document described 151 of the 244 operations
createApp()actually serves. It now describes all 244, andsrc/openapi/unspecced-routes-baseline.jsonis deleted — there is no longer anywhere to record an exception, which is the point.The 93 newly-described routes include the whole surface #9522 needs:
/v1/orb/*,/v1/internal/orb/*, the fleet kill-switch and config-push, the per-repo BYO-key routes, and the 32 internal job endpoints. Each carries a stableoperationId, realtags, and anauthlevel that derives its security stanza instead of having one bolted on afterwards by path prefix.The review's blockers, fixed — and they were right twice over
The AI review flagged that the generated document still emitted the identical
LoopOverBearer+LoopOverSessionCookiestanza on every operation regardless of its declaredauth— a bearer requirement on the ORB ingress whose own comment says it takes a shared-secret header. Correct, and the cause was layered:applySecurityMetadataoverwrote whatever an operation declared. The seam produced differentiated stanzas; the post-pass clobbered all of them with the generic pair. It now fills in only operations that declared nothing — which requiredpublicto emit[](OpenAPI's explicit "no credential") rather thanundefined, so a deliberately open route is distinguishable from a legacyregisterPathcall that never said.The fill-in now derives from the real gate.
requiresApiToken— the predicate the app actually gates on — moved tosrc/auth/route-auth.ts(pure, nocloudflare:imports, so the plain-Node spec builder can load it), andisProtectedPathis deleted: it was a second path-prefix model of the same policy that had already drifted, publishing the whole/v1/public/decision-ledger/*family as bearer-gated when every one of those routes answers 200 anonymously.The stanzas now split six ways instead of one: the generic pair (136), bearer-only internal (63), legacy-undeclared (28), explicitly public (11), the ORB instance bearer (5), the webhook body signature (1).
Deriving from the real gate surfaced a route both former models had wrong.
POST /v1/auth/github/token403s a bearer-only caller — the handler checksidentity.kind !== "session"— yet one model published it as bearer-or-cookie and the other would publish it as open. It now declaresLoopOverSessionCookiealone.The new parity meta-test (
test/unit/openapi-security-parity.test.ts) asserts against the real gate: no ungated route advertises the ordinary credential, no gated route claims to be open, the auth levels stay distinct rather than collapsing back to one stanza, and every referenced scheme is declared. Its own first two drafts each misread an exempt family as ungated (/v1/internal/*has its ownINTERNAL_JOB_TOKENmiddleware; the session-only route above is gated in its handler) — recorded in the test, because those exemptions are the map of where auth is enforced other than the token gate.operationId + tags on all 244
The document previously emitted
tags: []everywhere and anoperationIdnowhere — one flat namespace for a generated client, and every path edit a breaking rename. All 244 operations now carry a unique id and a real tag; the five table-registered families derive theirs in-loop from the same rules, so adding a sibling route stays a one-line change.Three fixes the work forced
RouteAuthgainedorbandwebhook. The ORB ingress genuinely does not authenticate the way the rest of the API does —requiresApiToken()exempts it — and the old model therefore published it as needing no credential at all. It needs a different one: an ORB-issued bearer for the relay and token endpoints, an HMAC signature header for the webhook. Both now have their own security scheme in the document.registerRouteSpecnever emitted path parameters. Every templated segment it registered was a Cloudflare 30046 schema-validation warning and a hole in any generated client — it had no{owner}/{repo}/{id}to fill. Now derived from the path itself, since there is no case where a path parameter is optional. Caught by the repo's own existing parameter test the moment the first templated route went through the seam.A test was requiring the document to stay wrong.
test/unit/openapi.test.tslisted/v1/internal/jobs/generate-signal-snapshots/runamong paths that must be absent as removed routes.src/api/routes.ts:5167registers it. Removed from that list with the reason recorded inline.The ratchet's other direction earned its keep
It rejected two job routes I had assumed existed. I had modelled the internal jobs as uniformly having both an enqueue form and a
/runsibling; two of them —backfill-contributor-gate-historyandrefresh-installation-health— are operator-triggered repairs with only the/runform. Publishing an operation no route serves is strictly worse than omitting one, because a generated client compiles a call that 404s at runtime. That direction never had a baseline and now never will.I also replaced the deleted baseline's two tests with one that asserts the surface is non-trivial (>200 live routes, >200 operations). Both remaining assertions are satisfied by two empty lists; this is what stops them from passing by describing nothing.
Still open on #9531 — not done here
I am not closing the issue. What remains:
operationIdandtags— the pre-existing hand-registered ones inspec.ts. The 101 that went through the seam have both.isProtectedPath/applySecurityMetadata, is blocked on the above: the parallel security model can only go once every operation declares its own auth.@loopover/contract.check-openapi-settings-parity.ts, which becomes obsolete by construction only after requirement 3.control-planespec and the four self-host infra endpoints.Those are sequenced — 5 depends on 4, 6 depends on 3 — so they want their own PRs rather than one that changes 244 operations at once.
Validation
npx vitest run test/unit test/contract— 23,521 passed, 6 skipped, 0 failed.npm run typecheckclean across root and all three stricter packages.ui:openapi:check,ui:openapi:settings-parity,docs:drift-check,dead-source-files:check,import-specifiers:checkgreen.