Replace Swagger UI with a custom API explorer - #1637
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the third-party swagger-ui-react dependency with a custom, in-house API explorer built using the native design system. The changes include the new explorer components, unit tests, and integration with the authentication store to clear persisted credentials on logout. The review feedback focuses on enhancing robustness against prototype pollution and unexpected keys by replacing direct property access and the in operator with safer checks (such as Object.prototype.hasOwnProperty.call or explicit type checks) when handling settings, schemas, and path parameters.
1ff48aa to
eb7ab94
Compare
kriszyp
left a comment
kriszyp
left a comment
There was a problem hiding this comment.
This sounds great. But can users use cookies to authenticate, without have to hand over credentials that get stored in localStorage?
🤖 Reviewed with Codex
Swap the swagger-ui-react embed on the instance/cluster "APIs" tab for a custom explorer built from the in-house design system (Radix + Tailwind tokens, Monaco, useCopyToClipboard). It reads the same runtime spec (GET /api/openapi/rest) and preserves the CORS warning + one-click enable flow verbatim. - Hierarchical sidebar: resource -> path -> method, collapsible, filterable, with an "Authorize" item that takes over the detail pane with Server + Authorization settings. - Per-operation docs (parameters, request/response schemas via a recursive model renderer) and an interactive "Try it out" runner with a JSON body editor, path/query/header inputs, live request preview, and a fetch code sample. - Auth offers Cookie (session, default), Basic, and Bearer; Basic/Bearer add an Authorization header. Server is a selector (Studio-computed URL plus the spec's declared servers) so try-it-out works even when REST isn't on the guessed port. - Server + auth selections persist to localStorage scoped per entity, and are cleared on sign-out (authStore.signOutLocally) so credentials can't outlive a session. Closes the swagger-ui-react cookie-apiKey persist TypeError (#1530) by removing the dependency, and the long auto-generated query strings that caused 431s on relationship-heavy schemas (#1580) by never auto-populating query params. Pure spec-parsing and request-building logic is extracted and unit-tested, plus a mounted-component test for the explorer. Removes swagger-ui-react/@types and its lockfile subtree. Closes #1530 Closes #1580 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eb7ab94 to
c74932c
Compare
|
Great question — yes. The default Cookie auth mode uses the browser's instance session cookie ( 🤖 Addressed by Claude Code |
2 participants
Replaces the
swagger-ui-reactembed on the instance/cluster APIs tab with a custom, in-houseAPI explorer built entirely from the Studio design system. It reads the same runtime spec
(
GET /api/openapi/rest) and preserves the CORS warning + one-click Enable CORS flow verbatim.What's new
untagged, so grouping is by path/resource rather than the useless single "default" tag.
inputs, a live request preview, a real response view, and a copy-able
fetchsnippet.settings. Auth offers Cookie (session, default), Basic, and Bearer.
works even when REST isn't on the port Studio guesses.
localStorageper entity, and clearedon sign-out.
Pure spec-parsing and request-building logic is extracted into unit-tested modules, with a mounted
component test for the explorer. Removes
swagger-ui-react+@types/swagger-ui-reactand their~1,290-line lockfile subtree.
Closes #1530 — the RUM
TypeErrorcame from inside the vendoredswagger-ui-reactAuthorize/persistbundle; deleting the dependency removes it. Our Authorize never writes
document.cookieor reads.schemaoff a security scheme.Closes #1580 — Swagger auto-populated a huge query string for
GET /Table/on relationship-heavyschemas (→
431). Our explorer starts query params empty and omits blanks, so no query string issent unless the user types one.
For the human reviewer
Start in
spec.tsandrequest.ts— the pure, unit-tested core; thecomponents render them.
APIDocs.tsxkeeps the CORS flow.The judgment calls, most-consequential first:
localStorage. Per request, the server + Basic password / Bearer token persistper entity, cleared on sign-out (
authStore.signOutLocally; full sign-out already clears all oflocalStorage), and written via a fresh read-merge-write so a concurrent tab's sign-out isn'tclobbered. Tradeoff: creds at rest between sessions (XSS/shared-machine exposure) vs. convenience
and parity with the old Swagger
persistAuthorization. Alternative: persist only server + authtype, keep secrets in memory. Chose persist-and-clear-on-signout. (The default Cookie mode
stores no secret at all — it just uses the session cookie.)
On a split-port local instance the computed URL guesses port 80 and isn't reachable, so the
reachable spec-declared server is one click away in the dropdown. The maintainer confirmed the
cloud path runs the same code (only response JSON differs); happy to flip the default to the spec
server if preferred.
readCappedBodyreadsresponse.bodyonly upto the cap and cancels the reader, so a multi-MB collection is never fully buffered or rendered.
$refresolution doesn't JSON-Pointer-unescape.Correct for Harper's generated specs (single-segment resources, simple schema names); noted as
limitations for arbitrary OpenAPI producers.
Comments were audited twice against Harper's zero-new-comments default; the remainder are deliberate
contracts/rationale (the credentialed-fetch model, the ancestor-scoped cycle detection, the
per-entity persistence invariant). Flag any specific one you'd still cut.
Verification
dev:localagainst a local Harper instance):GET /leaderboard/→ 200 OK;POST /game/{id}path-param substitution + Send gating; server selector switching to the reachable:9926; Authorize Cookie/Basic/Bearer; selections persisting across reload; sign-out clearing thestored settings. The maintainer confirmed the cloud/cluster path runs the same code — only the
spec/response JSON differs — so the local smoke is representative.
vitest run(311 files, 2541 passed),tsc -b,oxlint .,dprint check: all green. New tests cover spec parsing (incl. the example node budget),request building, streaming response cap, per-entity persistence (incl. the cross-tab stale-writer
sequence), status colors, and a mounted-component test.
Review coverage
Four cross-model pre-push rounds via the standard CLI (
--author claude), each independent:7f12099): codex (graded) + gemini. Fixed: Unicode Basic-authbtoacrash,cross-instance try-it-out state leak (per-entity remount),
generateExamplesibling-$reftruncation, response render cap, path-template param derivation, filter allocation.
7209c74,--full): gemini. Added sign-out credential clearing; lowered theresponse cap.
c22edd8): gemini + cursor-grok. Fixed:authStorein-on-non-objectcrash-safety guard,
mergeParameters$ref-param collision guard, memoized example generation.1ff48aac, convergence): gemini — no new actionable findings; only repeats ofdecided items, so iteration stopped here.
Not run: Codex was unavailable from round 2 on (a ChatGPT-workspace spend cap, not a code
issue); the domain adjudicator failed locally every round (a known zero-byte-log issue on this
machine), so outside findings were hand-triaged rather than auto-adjudicated; Cursor was
available only in round 3. Dismissed with rationale, not silence: a claimed
buildFetchSnippettemplate-literal injection (traced — every
${is escaped, so no interpolation survives a paste), an"undefined body" crash (
generateExamplehas noundefinedreturn path;bodyis always a string),and a "cookies sent to external servers" leak (browsers scope cookies to the destination origin).
Deferred as not-producible-by-Harper-specs: multi-segment resource grouping, JSON-Pointer
$refunescaping,
$refparameter resolution, and the snippet's.json()on 204 responses.Post-open, the
gemini-code-assistPR bot raised three prototype-safety points; applied ineb7ab94d(Object.hasOwnguards inforgetApiExplorerSettings,resolveRef, andfillPathTemplate) and the threads resolved.@kriszyp (Codex-assisted) then reviewed and caught five real issues, all fixed in
c74932cc:(1) the response cap only bounded rendering — now
readCappedBodystreamsresponse.bodyup to thecap and cancels the reader, so a multi-MB collection is never fully buffered; (2) a cross-tab
stale-writer could resurrect a signed-out entity's credentials — persistence now does a fresh
read-merge-write per entity (
settings.ts) plus astorage-event refresh; (3)generateExamplewas bounded against cycles but not acyclic 2^N fan-out — added a depth + node budget; (4) Send now
gates on required query/header/body inputs, not just path params; (5) the sidebar used
max-heightso its list never scrolled — now a definite height. His cookie question is answered on the thread:
the default Cookie mode already authenticates via the session cookie and stores no secret in
localStorage.
Human-Review-Need: 4 @ c74932c