Skip to content

Add UISettings model for /ui-settings endpoint#103

Open
tomerqodo wants to merge 3 commits into
qodo_only-issues-20260113-qodo-grep-copilot_base_add_uisettings_model_for__ui-settings_endpoint_pr283from
qodo_only-issues-20260113-qodo-grep-copilot_head_add_uisettings_model_for__ui-settings_endpoint_pr283
Open

Add UISettings model for /ui-settings endpoint#103
tomerqodo wants to merge 3 commits into
qodo_only-issues-20260113-qodo-grep-copilot_base_add_uisettings_model_for__ui-settings_endpoint_pr283from
qodo_only-issues-20260113-qodo-grep-copilot_head_add_uisettings_model_for__ui-settings_endpoint_pr283

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#283

desertaxle and others added 3 commits January 5, 2026 11:04
Phase 1 of dynamic API URL discovery for the React UI (ui-v2). This enables
the UI to discover the API URL at runtime instead of using a hardcoded
build-time environment variable.

Changes:
- Add UISettings Pydantic model to server schemas
- Update /ui-settings endpoint to return typed response
- Update OpenAPI schema generation to include UI routes
- Regenerate TypeScript types with UISettings schema
- Create UiSettingsService in ui-v2 for runtime URL discovery

The settings service fetches /ui-settings on first access and caches the
result. In production, it uses a relative URL (same origin). In development,
it falls back to VITE_API_URL or localhost:4200.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove the ui-settings.ts service file and revert auto-generated zod changes
from orval version update.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Code Review by Qodo (Alpha)

🐞 Bugs (2) 📘 Rule Violations (0) 📎 Requirement Gaps (0) 💡 Suggestions (0)

Grey Divider


Action Required

1. UI path prefix leaks 🐞 Bug
Description
• The OSS schema generator claims it will remove any UI base-path prefix, but it copies the UI app’s
  OpenAPI path verbatim.
• Since the UI route is registered under PREFECT_UI_SERVE_BASE (via stripped_base_url),
  oss_schema.json can end up with /ui/ui-settings instead of /ui-settings, making client
  generation/configuration inconsistent.
• The substring filter ("ui-settings" in path) is also fragile and can unintentionally merge other
  future endpoints containing that substring.

correctness

Code

scripts/generate_oss_openapi_schema.py[R13-17]

+# Merge /ui-settings path from ui_app into main schema
+for path, path_item in ui_schema.get("paths", {}).items():
+    if "ui-settings" in path:
+        # Use the path without any base URL prefix
+        openapi_schema["paths"][path] = path_item
Evidence
The generator copies any UI OpenAPI path containing ui-settings without stripping a base URL, but
the UI app defines the route using stripped_base_url derived from PREFECT_UI_SERVE_BASE, so the
UI schema path can be prefixed. Meanwhile, the generated UI client types expect a stable
/ui-settings path.

scripts/generate_oss_openapi_schema.py[13-17]
src/prefect/server/api/server.py[456-484]
ui-v2/src/api/prefect.ts[2985-2994]

Agent Prompt
## Issue description
`scripts/generate_oss_openapi_schema.py` intends to merge `/ui-settings` without any UI base-path prefix, but it currently copies the UI app OpenAPI path verbatim and selects by substring match.

This makes `oss_schema.json` (and any generated clients) dependent on `PREFECT_UI_SERVE_BASE` and fragile if new routes contain `ui-settings` in their path.

## Issue Context
The UI app registers the endpoint at `f&quot;{stripped_base_url}/ui-settings&quot;`, where `stripped_base_url` comes from `PREFECT_UI_SERVE_BASE`.

## Fix Focus Areas
- scripts/generate_oss_openapi_schema.py[13-17]
- src/prefect/server/api/server.py[456-484]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation Recommended

2. Overbroad schema merge 🐞 Bug
Description
• The generator comment says it is merging the UISettings schema component, but it actually copies
  *all* schemas from the UI app into the API schema.
• It overwrites existing component schemas unconditionally, so shared schema names (e.g., FastAPI’s
  common validation schemas) can be silently replaced, changing generated client types unexpectedly.
• This makes the OSS schema sensitive to unrelated UI schema changes and can introduce hard-to-debug
  diffs in downstream generated code.

reliability

Code

scripts/generate_oss_openapi_schema.py[R19-23]

+# Merge UISettings schema component
+for schema_name, schema_def in (
+    ui_schema.get("components", {}).get("schemas", {}).items()
+):
+    openapi_schema["components"]["schemas"][schema_name] = schema_def
Evidence
The generator iterates over every UI schema and assigns into the main schema’s components.schemas
without checking for collisions or limiting to UISettings. The repo’s generated types already
contain shared/common schema names (e.g. HTTPValidationError), illustrating that collisions are
plausible and that overwriting would affect widely-used types.

scripts/generate_oss_openapi_schema.py[19-23]
ui-v2/src/api/prefect.ts[8010-8014]

Agent Prompt
## Issue description
`scripts/generate_oss_openapi_schema.py` currently merges *all* UI OpenAPI component schemas into the API schema and overwrites any existing schema with the same name.

This can silently change widely-used schemas and destabilize generated clients.

## Issue Context
The script comment indicates only `UISettings` should be merged, but the implementation copies every schema from the UI app.

## Fix Focus Areas
- scripts/generate_oss_openapi_schema.py[19-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +13 to +17
# Merge /ui-settings path from ui_app into main schema
for path, path_item in ui_schema.get("paths", {}).items():
if "ui-settings" in path:
# Use the path without any base URL prefix
openapi_schema["paths"][path] = path_item
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action Required

1. Ui path prefix leaks 🐞 Bug

• The OSS schema generator claims it will remove any UI base-path prefix, but it copies the UI app’s
  OpenAPI path verbatim.
• Since the UI route is registered under PREFECT_UI_SERVE_BASE (via stripped_base_url),
  oss_schema.json can end up with /ui/ui-settings instead of /ui-settings, making client
  generation/configuration inconsistent.
• The substring filter ("ui-settings" in path) is also fragile and can unintentionally merge other
  future endpoints containing that substring.
Agent Prompt
## Issue description
`scripts/generate_oss_openapi_schema.py` intends to merge `/ui-settings` without any UI base-path prefix, but it currently copies the UI app OpenAPI path verbatim and selects by substring match.

This makes `oss_schema.json` (and any generated clients) dependent on `PREFECT_UI_SERVE_BASE` and fragile if new routes contain `ui-settings` in their path.

## Issue Context
The UI app registers the endpoint at `f"{stripped_base_url}/ui-settings"`, where `stripped_base_url` comes from `PREFECT_UI_SERVE_BASE`.

## Fix Focus Areas
- scripts/generate_oss_openapi_schema.py[13-17]
- src/prefect/server/api/server.py[456-484]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +19 to +23
# Merge UISettings schema component
for schema_name, schema_def in (
ui_schema.get("components", {}).get("schemas", {}).items()
):
openapi_schema["components"]["schemas"][schema_name] = schema_def
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation Recommended

2. Overbroad schema merge 🐞 Bug

• The generator comment says it is merging the UISettings schema component, but it actually copies
  *all* schemas from the UI app into the API schema.
• It overwrites existing component schemas unconditionally, so shared schema names (e.g., FastAPI’s
  common validation schemas) can be silently replaced, changing generated client types unexpectedly.
• This makes the OSS schema sensitive to unrelated UI schema changes and can introduce hard-to-debug
  diffs in downstream generated code.
Agent Prompt
## Issue description
`scripts/generate_oss_openapi_schema.py` currently merges *all* UI OpenAPI component schemas into the API schema and overwrites any existing schema with the same name.

This can silently change widely-used schemas and destabilize generated clients.

## Issue Context
The script comment indicates only `UISettings` should be merged, but the implementation copies every schema from the UI app.

## Fix Focus Areas
- scripts/generate_oss_openapi_schema.py[19-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants