Repository navigation
feat(reef): add a base URL field to source creation - #2024
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/reef/app/routes/source-discovery.ts">
<violation number="1" location="apps/reef/app/routes/source-discovery.ts:108">
P1: Swagger 2 sources can be installed with an incorrect base URL because their `host`, `basePath`, and `schemes` fields are ignored by the new inference. Construct the Swagger 2 base URL from those fields before falling back to an origin probe, for both JSON and YAML.</violation>
<violation number="2" location="apps/reef/app/routes/source-discovery.ts:491">
P1: Discovery can issue a server-side request to a host chosen by the fetched OpenAPI document: a `//host/path` path key escapes `sourceUrl.origin` during URL resolution. Validate the resolved probe URL's origin before fetching, or reject network-path path keys.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
A path key comes from the fetched document, and `new URL()` resolves a network-path one such as `//host/data` to a host of the document's choosing rather than to the origin we were asked to fetch. Check the resolved probe URL's origin before requesting it.
A redirect can move a descriptor to another host, and a docs host that redirects to an API host would otherwise get both a relative `servers[].url` and the origin probe resolved against the URL that was typed. Use the response's final URL for both, which leaves the non-redirect case unchanged.
The probe was bounded only by its own five-second timeout, so a caller who navigated away still left a server-side request running and a loader awaiting it. Combine the caller's signal with the timeout, and let an abort propagate to the loader's existing handling rather than being read as an origin that does not serve the path.
An absent `servers` block means "the API is served from here" in OpenAPI 3 only. Swagger 2 has no `servers` at all — it declares the base URL across `host`, `basePath` and `schemes` — so every Swagger 2 document looked like an undeclared one and got the document's origin, dropping whatever `basePath` said. Leave those for the user to fill in, which is what any document we cannot infer already does.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The probe carried its own five-second timeout on top of the loader's ten-second one, so discovery could run for fifteen. Give it the combined signal instead, and decide cancellation in the loader: a probe that times out costs only the base URL, while a caller who navigated away still gets no answer.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
simonwhitaker
approved these changes
Jul 30, 2026
The test claimed to cover a probe timeout, but its mock rejected immediately, so neither the probe's own deadline nor discovery's was exercised — deleting the timeout from the composed signal left it green. What it does cover is worth keeping and uncovered elsewhere: a probe that throws while discovery is still live falls back to a blank base URL without discarding the metadata already read from the document. Name it accordingly and reject with a network error rather than an AbortError, so the mock stops implying timeout semantics it never had. The five-second timeout stays untested: AbortSignal.timeout bypasses the global setTimeout that fake timers patch, and losing it would cost latency rather than correctness, since AbortSignal.any still bounds the probe by discovery's deadline.
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: New feature adding a base URL field to source creation, changing the UI, validation, and manifest output. Human sign-off is needed for the product and data contract decision.
Re-trigger cubic
AlbertQM
pushed a commit
that referenced
this pull request
Jul 30, 2026
🤖 I have created a release *beep* *boop* --- ## [0.8.3](v0.8.2...v0.8.3) (2026-07-30) ### Features * **desktop:** add About Coral to the app menu ([#2030](#2030)) ([815e50b](815e50b)) * **reef:** add a base URL field to source creation ([#2024](#2024)) ([bd196f4](bd196f4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: coral-release-bot[bot] <269256696+coral-release-bot[bot]@users.noreply.github.com> Co-authored-by: release-please[bot] <release-please[bot]@users.noreply.github.com>
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
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.
Summary
For OpenAPI spec sources, we need a separate base URL, to tell Coral which endpoint to hit when fetching the data; the first URL points to the OpenAPI spec, which is not what we need.
Test plan
Paste https://lordsvotes-api.parliament.uk/swagger/v1/swagger.json, check base URL is inferred correctly.
Screen.Recording.2026-07-30.at.11.35.31.mov