test(openapi): enforce that the published spec covers every public route

[Emmo00]

Summary

openapi.yaml documented 7 paths while the server registers ~32, and the spec is auto-published to GitHub Pages on every push — so Lens was shipping a public contract that omitted two thirds of its surface, with nothing to stop the gap widening.

This PR makes spec coverage enforceable and fills the gap:

• Boots the real route surface in a test. Extracted the Fastify wiring from src/index.ts into buildApp() (src/app.ts) so tests/openapi.test.ts can register every plugin/hook/route exactly as production does — no duplicated route list to drift. The test enumerates registered routes and fails if any non-internal path or method is missing from openapi.yaml. It failed on 17 routes before the spec was filled in.
• Documents the missing public routes in openapi.yaml — /price/*/depth, /price/twap|vwap/..., /volumes/{asset}, /spreads/{asset}, /compare/{asset}, /screener, /benchmark/{asset}, /basket, /discovery/resources, /webhooks, /usage/me, /supported, /verify, /settle, /graphql — each with parameters, a shared components/parameters/NetworkQuery (?network=) where it applies, and an explicit response schema.
• Handles internal routes explicitly via a commented allow-list in src/openapi/coverage.ts (/admin/keys*, /admin/usage*, /metrics, /ws, /graphiql*) instead of silently skipping them. The test also asserts the allow-list has no stale entries.
• Makes the generator deterministic and importable (renderOpenApiJson() / generateOpenApi()), with a test asserting the committed openapi.json matches its output (and that two runs agree byte-for-byte).
• Brings the README endpoint table in line with the full public surface and notes the internal allow-list.

Related issue

Closes #176

Type of change

• Bug fix
• New feature
• Refactor
• Docs
• Tests
• CI / tooling

Checklist

• I have read CONTRIBUTING.md
• npx tsc --noEmit passes
• npm run build passes
• I added / updated tests where relevant
• I updated docs where relevant

Notes

• Verified locally: npx tsc --noEmit exits 0; npx vitest run gives 406 passed / 1 skipped across 48 files (previously 400 passed / 1 skipped — +6 from the new suite).
• /discovery/resources's network query param is documented on its own rather than via NetworkQuery: it filters catalog entries (accepting CAIP-2 ids) and has different semantics from the shared Stellar network selector.
• No runtime behaviour change; src/index.ts keeps the DB/Redis connections, migrations and ingesters, and only delegates app construction to buildApp()

[drips-wave]

@Emmo00 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[Emmo00]

Hey @Miracle656

This is ready. Would you like me to make any changes?

[Emmo00]

Hi @Miracle656

I fixed the merge conflict

[Miracle656]

The architecture here is right and I want to say that up front: extracting buildApp() so the coverage test enumerates the real route surface instead of a hand-maintained list is the correct call, the INTERNAL_ROUTES allow-list with a reason per entry (plus the stale-entry assertion) is exactly the shape that stops rot, and making renderOpenApiJson() importable so a test pins the committed JSON to the YAML closes the generated/source drift that would otherwise reappear on the next spec edit. Going from 7 documented paths to 25 on a spec that auto-publishes to Pages is a real improvement.

Diff size, split as requested

	lines
Generated — openapi.json (+2115/-332)	2,447
Human — openapi.yaml (+1293/-186)	1,479
Human — code/tests/docs (src/app.ts 150, src/index.ts 121, tests/openapi.test.ts 81, src/openapi/coverage.ts 80, scripts/generate-openapi.ts 39, README.md 23, changeset 11)	505
Human total	1,984

So ~55% of the +3791/-640 is the regenerated JSON. I reviewed the 1,984 human lines and diffed src/app.ts against the block it replaces in src/index.ts line by line: the plugin/hook/route order is identical, including the /status onRoute override landing after rateLimit, and the only addition is the optional onRoute hook registered first so it observes encapsulated plugins. No behaviour change — confirmed.

Verification

Merged onto current origin/main (clean, no conflict in openapi.json), then: npx tsc --noEmit clean, full suite 544 passed, 1 skipped, 57 files. The one failure was tests/openapi.test.ts:75 ("matches the committed file") and it is not your bug — my checkout has core.autocrlf=true, so readFileSync('openapi.json') returns 2,856 CRLFs while renderOpenApiJson() emits LF. Normalising line endings locally: npx vitest run tests/openapi.test.ts → 6 passed. CI is green. (Worth hardening anyway — see the last note.)

What needs changing: the spec documents things that aren't true

The whole value of this PR is that the published contract matches reality, so these matter more here than they would anywhere else.

1. /screener documents a parameter that is guaranteed to 400. openapi.yaml declares a market_cap query parameter and lists market_cap in the sortBy enum. On the merge-base you branched from (0995f8a), .changeset/screener-drop-market-cap.md is already present and src/routes/screener.ts:37-39 rejects both:

if (q.market_cap !== undefined || q.sortBy === 'market_cap') {
  return reply.status(400).send({ error: MARKET_CAP_UNSUPPORTED })
}

with the sort allowlist at :41 being ['volume', 'change_24h', 'price', 'liquidity']. It was removed precisely because it was liquidity under another name — publishing it back into the contract reintroduces the thing that changeset went out of its way to kill. Drop the parameter and the enum member. (Your ScreenerResponse/ScreenerRow schemas are correct and already omit it — nice catch there.)

2. ?network= is declared on five routes that ignore it, and missing from six that honour it. This is the one I'd most want fixed, because on a dual-network price API a documented-but-ignored network selector is how a caller ends up believing a mainnet number that came from testnet rows.

Declares NetworkQuery, handler never reads req.network:

• /basket — src/routes/basket.ts:5-13, fetchAssetVWAP filters only on asset_a/asset_b and a 5-minute window, no network predicate
• /benchmark/{asset} — src/routes/benchmark.ts, no network reference at all
• /compare/{asset} — src/routes/oracle.ts:6, same
• /price/twap/{assetA}/{assetB} and /price/vwap/{assetA}/{assetB} — src/routes/price.ts; the handlers parse TwapQuerySchema/VwapQuerySchema (non-strict z.object, so network is silently stripped, not rejected) and build pairKey = [assetA, assetB].sort().join('/') with no network and no issuer

Honours network but the spec omits NetworkQuery:

• /price/{assetA}/{assetB} (src/api/rest.ts, req.network ?? activeNetwork → getAggregatedPrice(pairKey, network))
• /price/{assetA}/{assetB}/route (same, passed into getBestRoute)
• /pools, /pairs (GET), /status — all three pass network as a query parameter
• /prices/history — src/api/history.ts:43, WHERE pair = AND network =

/price/{assetA}/{assetB}/depth, /screener, /spreads/{asset}, /volumes/{asset} are correct as documented.

Two legitimate ways to resolve: add NetworkQuery to the six that honour it and remove it from the five that don't, or keep it everywhere and open a follow-up issue for the five route-side gaps (they're real bugs — /basket pooling testnet and mainnet rows into one weighted total is the same class of problem #181 is addressing on that route). Either is fine; what I don't want to publish is the current mix, where a client cannot tell from the spec which endpoints are network-scoped. Please say in the PR body which you chose.

3. The coverage test is one-directional. It fails when a registered route is missing from the spec, but not when the spec documents a path or method that no longer exists — which is structurally the same hazard, and is how #1 above could sit in the YAML unnoticed. The inverse is about four lines on top of what you already have:

it('documents only routes that exist', () => {
  const registeredPaths = new Set(registered.map(r => r.path))
  const phantom = Object.keys(spec.paths).filter(p => !registeredPaths.has(p))
  expect(phantom, `Documented but unregistered: ${JSON.stringify(phantom)}`).toEqual([])
})

Worth adding while you're in here; it makes the test a real two-way contract instead of a coverage floor.

Smaller

• README.md: the endpoint table now lists /price/twap/:assetA/:assetB and /price/vwap/:assetA/:assetB twice — the original short rows are still there above your new rows with the query strings. Delete the originals.
• tests/openapi.test.ts:75 compares raw bytes, so it fails for any contributor on Windows with core.autocrlf=true (it did for me). Either normalise in the assertion (.replace(/\r\n/g, '\n') on both sides) or, better, add a .gitattributes with *.json text eol=lf / *.yaml text eol=lf — the repo has none today. Your call which; a spec-equality test that depends on the reviewer's git config will waste someone an afternoon.

#1 and #2 are the blockers and both are confined to openapi.yaml plus a regenerate, so this should be a quick turnaround. The buildApp() extraction and the test harness I'd merge as-is.

[Emmo00]

1. /screener — removed the market_cap query param and its sortBy enum member, plus the stale market_cap property on ScreenerRow . The route 400s on all of them.

2. ?network= — went with the surgical option, so no follow-up issue. NetworkQuery is now documented only where req.network is actually read:

• Added: /price/{assetA}/{assetB} , /price/{assetA}/{assetB}/route , /pools , /pairs , /prices/history .
• Removed: /basket , /benchmark/{asset} , /compare/{asset} , /price/twap/... , /price/vwap/... .
• Left /status undocumented — it reports the instance's active network and never reads req.network , so documenting the param would be the same hazard.
• Also added the network field to the /prices/history response schema, which it returns but the spec omitted.

3. Coverage test — added the inverse check (path + method): the spec now fails if it documents a path or operation with no registered route.

Smaller — removed the two duplicate twap / vwap rows in the README and corrected its "every route accepts ?network= " claim; added .gitattributes ( *.json / *.yaml / *.yml → eol=lf ) so the openapi.json byte-equality test no longer depends on a contributor's git config.

[Miracle656]

Re-reviewed at 8165b9e. All three blockers are fixed, plus both smaller notes. Merged onto main as c01dee9, with one follow-up commit of mine (585c15b) for something that moved under you — detail at the end.

1. /screener documenting a parameter that 400s — addressed

grep market_cap openapi.yaml is empty and openapi.json has zero occurrences: the query parameter, the sortBy enum member and the stale market_cap property on ScreenerRow are all gone. That last one I hadn't asked for and should have — a response field that src/routes/screener.ts never selects would have been the next person's bug report.

2. ?network= matching the routes that honour it — addressed, and I checked it route by route

You went with the surgical option, and the result is exact. NetworkQuery is now $ref'd on precisely nine operations, and all nine read a network per request:

route	where it reads it
GET /price/{a}/{b}	src/api/rest.ts:92
GET /price/{a}/{b}/route	src/api/rest.ts:135
GET /price/{a}/{b}/depth	src/api/rest.ts:236
GET /pools	src/api/rest.ts:200
GET /pairs	src/routes/pairs.ts:21
GET /prices/history	src/api/history.ts:112
GET /screener	src/routes/screener.ts:73
GET /spreads/{asset}	src/routes/spreads.ts:89
GET /volumes/{asset}	src/routes/volumes.ts:45

And the five I asked you to strip are stripped: /basket, /benchmark/{asset}, /compare/{asset}, /price/twap/{a}/{b}, /price/vwap/{a}/{b} all declare no network now. I also checked the two routes neither of us listed last time — /candles/{a}/{b} has no network reference anywhere in src/routes/candles.ts, and /price/{a}/{b}/history queries price_aggregates on pair_key alone (src/api/rest.ts:166) — and the spec correctly omits it from both. The network parameter on /discovery/resources is the Bazaar catalog filter rather than NetworkQuery, it has its own description explaining the CAIP-2 form, and parseDiscoveryFilters → resolveNetworkFilter genuinely honours it. Correct as documented.

Adding the network field to the /prices/history response schema was a good catch on your own account — the route returns it and the spec didn't say so.

3. The one-directional coverage test — addressed, more thoroughly than I suggested

You added both halves rather than the single path-level check I sketched: documents only paths that are actually registered and documents only operations that are actually registered. The second one is the version that would actually have caught the /screener problem's sibling — a documented POST on a path that only registers GET — and it's the one I'd have forgotten. With allow-lists only routes that are actually registered alongside it, the test is now a genuine two-way contract in all three directions.

Smaller notes — both addressed

• The duplicate twap/vwap rows are gone from the README table, and the table gained the ten routes it had been missing entirely (/depth, /candles, /volumes, /spreads, /compare, /screener, /benchmark, /basket, /usage/me, the facilitator and webhook routes, /graphql). The "Every route accepts ?network=" claim is replaced with an accurate one.
• .gitattributes with *.json/*.yaml/*.yml → eol=lf is the right fix, and I verified it does the job: my checkout is still core.autocrlf=true, and after the merge openapi.json in the working tree has 0 CRLF sequences, so tests/openapi.test.ts's byte-equality assertion passes without any local normalisation. That is the first time this repo's spec test has been green on a Windows checkout.

Verification

Merged onto origin/main (clean, no conflict), then: npx tsc --noEmit clean, full suite 56 files passed / 1 skipped (57), 547 tests passed / 1 skipped (548) — up from 539 on main, which is your eight new tests. tests/openapi.test.ts on its own: 8 passed. CI on the branch is green.

The one thing I changed: /status (585c15b)

Your reasoning for leaving /status undocumented was right for the tree you branched from — on 0995f8a the handler is literally async () => {}, reads nothing from the request and filters indexer_state on nothing. But 61c1854 ("fix: scope /status to a network and record the real indexer ledger", #193) landed on main after your d308dba merge of main, and it rewrote that handler:

app.get('/status', { config: { public: true }, ... }, async (req) => {
  const network = req.network ?? activeNetwork
  const result = await pgPool.query(`… FROM indexer_state WHERE network = $1 …`, [network])
  return { ok: true, network, watchedPairs: getNetworkConfig(network).pairs.map(…), … }

So on current main /status does read req.network, filters on it, and echoes the chosen network back — and your base is five commits behind that. Your response schema for /status is already correct for the new shape (it documents network and ingestLagSeconds, which only #193 added), so it was just the parameter missing. I added the NetworkQuery $ref and regenerated openapi.json; tests/openapi.test.ts still passes 8/8.

I also finished the README's "Per-request today" list in the same commit — it predates this PR and was already missing /pairs, /spreads/:asset and /volumes/:asset, so it needed /status plus those three — and pointed it at openapi.yaml as the source of truth, since the whole point of this PR is that there now is one.

Worth noting what that near-miss says about the design you built: a parameter being declared on the wrong set of routes is still invisible to the coverage test, which checks paths and methods but not parameters. Not something to fix now, and for most parameters a test like that would be more trouble than it's worth — but network specifically is the one where a wrong answer is a wrong price, so if you ever want a fourth assertion, "every route whose handler reads req.network declares NetworkQuery, and no other route does" is the one I'd write. It would have caught this on its own.

Two real route-side bugs surfaced while I was checking, neither yours and neither in scope: /price/{a}/{b}/history reads price_aggregates filtered on pair_key alone, so on an instance with ENABLED_NETWORKS covering both chains its OHLCV buckets interleave testnet and mainnet rows — which is what your README note already says, and it is the same class as the /basket problem #181 is on. /candles looks the same. I'll open issues for both.

Thanks for this one. Going from 7 documented paths to 25 on an auto-published spec, with a test that keeps it honest in both directions, is the kind of change that stops a whole category of bug rather than one instance of it.