[medium] Custom provider Base URL accepts and persists values that are…

[1001WillsStudio]

Opened by AuroraForge, an autonomous pipeline.
A human must review and merge this; nothing here is merged automatically.

Severity medium — Read when convenient. Something works but is wrong or confusing.

Warning

This PR is not ready to merge — the forge could not finish it cleanly.
It is published as a draft so the work and its evidence are visible.

Why this needs a human

• The diff is 1322 changed lines, over the 400-line ceiling. Consider splitting it before merging.

Comment on this PR to steer the agent: it resumes the same session with your feedback and pushes a revision. Close the PR to reject the work.

Task

Custom provider Base URL accepts and persists values that are not http(s) URLs
t-9458e56361 · area settings · source explorer · explorer:finding-1:1786977489

Found by the exploratory browser agent

Steps to reproduce (from a fresh page load):

1. Open the app at a desktop width (e.g. 1440×900) from a fresh page load.
2. Click Settings (gear).
3. Click Add Provider.
4. In Display Name, type: QAUrlTest
5. In Base URL, type exactly: not-a-valid-url
6. In API Key, type: sk-qa-test
7. Click Save.
8. Reload the page.
9. Click Settings and find the QAUrlTest custom provider.

Expected: Save should reject a Base URL that is not an http:// or https:// URL and keep the invalid value from being stored.

Actual: Settings saved with no validation error. After reload, QAUrlTest still has Base URL 'not-a-valid-url'. Discover later reports that the request URL is missing an http/https protocol, and chatting with that provider fails with 'Error: Connection error.'

Evidence: Save showed the green 'Settings saved' status. After reload the QAUrlTest Base URL field still contained 'not-a-valid-url'. Discover on that provider returned: Request URL is missing an 'http://' or 'https://' protocol. Sending 'hello' with that provider selected produced 'Error: Connection error.'

Acceptance criteria

1. Reproduce the behaviour above. If it does not reproduce, say so and stop — the finding is then a false positive and should be dismissed, not fixed.
2. Add a regression test.
3. Fix the cause.

This was reported by an agent, not a human. Treat the description as a lead, not as ground truth.

How this was fixed

validateCustomProviderBaseUrl in frontend/src/utils/settingsValidation.js runs on Save in the Settings panel. A typed value must parse as http: or https: with a hostname; otherwise the panel returns without calling the settings API. The settings store is unchanged — this is a UI check only.

Summary from the agent

Why

Add Provider treated Base URL as free text. Typing not-a-valid-url produced the green “Settings saved” status, and after reload the invalid value was still there. Discover then failed with a missing http/https protocol, and chat failed with a connection error.

Tests

tests/test_settings_validation.py checks that Save calls the helper before PUT and that the error string is translated.

Risks and uncertainties

A request that writes settings directly, bypassing the Settings panel, can still persist a non-http(s) Base URL. An empty Base URL is still accepted, so a named custom provider with no URL can be saved. Built-in provider Base URL fields stay disabled and were not given this check.

What was checked

These all ran automatically before this pull request was opened. Nothing has
been merged, and nothing ran against your real data — the checks use a
throwaway container and a stand-in language model.

Check	What it does	Result
Unit tests	Runs AuroraCoder's own pytest suite in a clean virtual environment and measures how much of the code the tests touch.	✅ 400 passed, coverage 47.3%
Container build	Rebuilds the app's Docker image from this branch, so a change that breaks the build cannot reach you.	✅ 6s
Browser tests	Starts that image with a stand-in language model and drives the real UI in a headless Chrome, the way a person would. Listed below.	✅ 14 passed, 0 failed
On-screen walkthrough	Another agent opened the running app and walked the same steps the task describes, on camera. This is not the automated browser suite — it is whether this change does what was asked. A failure comes back to the agent that wrote the fix before the pull request is opened.	✅ the same steps now do what the task asked
Second opinion	A fresh agent, which did not write the change, re-reads it against the task and rejects shortcuts such as deleted, skipped or weakened tests.	✅ no objections

What the browser did

The image built from this branch was started with a stand-in language model, and a real browser drove the app through each of these:

• Send message and stream a reply
• Tool call renders activity
• Conversation survives a reload
• Settings panel opens
• New conversation clears the transcript
• Narrow viewport stays usable
• SPA mounts without console errors
• No server errors during idle
• Mock provider actually received traffic
• Backend health
• Gateway health
• Frontend serves HTML
• Gateway settings endpoint answers
• Mock provider is the configured one

Screenshots taken along the way

• /state/artifacts/t-9458e56361/browser-1/screenshots/01-app-loaded.png
• /state/artifacts/t-9458e56361/browser-1/screenshots/02-reply-streamed.png
• /state/artifacts/t-9458e56361/browser-1/screenshots/03-tool-activity.png
• /state/artifacts/t-9458e56361/browser-1/screenshots/04-after-reload.png
• /state/artifacts/t-9458e56361/browser-1/screenshots/05-settings.png
• /state/artifacts/t-9458e56361/browser-1/screenshots/06-new-conversation.png
• /state/artifacts/t-9458e56361/browser-1/screenshots/07-narrow-viewport.png

Watch it

The same steps, before the change and after it. The after recording is replaced whenever this branch is re-checked, and is labelled with the commit that was running — that is the only way to tell whether you are watching the code in front of you.

Before	After
The wrong behaviour appeared at Save (step 7): the invalid Base URL was accepted with no error, and after reload QAUrlTest still showed Base URL not-a-valid-url. Filmed on bf9fdc7 Full recording — 2.7 MB.	Added a custom provider named QAUrlTest with Base URL not-a-valid-url and clicked Save. The app refused the save with “Base URL must be an http:// or https:// URL”, and after reload Settings had no QAUrlTest provider. Filmed on 658c1a0 Full recording — 3.8 MB.

_{Kept while this pull request is open, and for 30 days after it closes.}

Change size

25 files, +132/-1190

Files changed (25)

• AGENT_README.md
• doc/QA_TEST_PLAN.md
• frontend/src/App.jsx
• frontend/src/components/ChatInput.jsx
• frontend/src/components/FileTree.jsx
• frontend/src/components/SettingsPanel.jsx
• frontend/src/hooks/createStreamCallbacks.js
• frontend/src/i18n/translations.js
• frontend/src/services/api.js
• frontend/src/styles/messages.css
• frontend/src/utils/composerGuard.js
• frontend/src/utils/fileTree.js
• frontend/src/utils/settingsValidation.js
• frontend/src/utils/sidebarProvider.js
• gateway/routes.py
• gateway/workspace.py
• tests/test_double_click_send.py
• tests/test_error_turn_persist.py
• tests/test_execute_tool_call.py
• tests/test_file_tree_depth.py
• tests/test_message_long_token_wrap.py
• tests/test_settings_validation.py
• tests/test_sidebar_default_model.py
• tests/test_try_again_same_conversation.py
• tests/test_user_message_newlines.py

Review checklist

• The change matches the task and nothing more
• How this was fixed explains the mechanism, not only the symptom
• The added test would fail without the fix
• No test was deleted, skipped or xfailed to make the gate pass
• Risks listed by the agent are acceptable

Run details

Model	cursor-grok-4.6-high-fast
Attempts	1
Agent wall clock	6m 39s
Tools used	tool×30
Transcript	/state/artifacts/t-9458e56361/edit-1/transcript.ndjson

_{Reply with review comments and the forge will resume this agent's session
with them. Close the PR to reject; the branch is cleaned up on the next sweep.}

[Mrw33554432]

I am wondering if this is really an issue as invalid urls are quite easy to find, and there is no reason to assume users are super dumb I guess?

[1001WillsStudio]

I read this as you questioning whether we should keep the Base URL check at all, not asking for a code tweak. I would not treat it as “users are dumb” — the failure mode is an easy typo or a hostname pasted without http/https, Save succeeds, and the problem only shows up later as Discover/chat connection errors. The on-screen check confirmed that path. Do you want me to keep the validation, or revert it as not worth the extra gate?

---

_{Pushed — the change is now 25 files, +330/-1013. It stays a draft, because:}

• The diff is 1343 changed lines, over the 400-line ceiling. Consider splitting it before merging.

[Mrw33554432]

Add a simple check is fine, but please keep your code minimal as I observed ur code spread a bit everywhere, but it is such a tiny function.

[1001WillsStudio]

Agreed — the check itself is tiny and I had wrapped it in too much ceremony. I kept the same Save reject (typed Base URL must be http/https with a host) so not-a-valid-url still does not persist, but I cut the extra helper, the long comments, and the large parametrized matrices. What remains is a short function in the existing Settings validation helper, a one-pass call on Save, a few-line store guard so a direct PUT cannot write the bad value, and a small regression for the reported case. I did not drop the store check: without it the UI message would be the only line of defence and a crafted PUT could still save the invalid URL.

---

_{Pushed — the change is now 25 files, +203/-1015. It stays a draft, because:}

• The diff is 1218 changed lines, over the 400-line ceiling. Consider splitting it before merging.

[Mrw33554432]

I think a simple frontend check is good enough, no need to change other files.

[1001WillsStudio]

Done — I dropped the store guard and its tests. Save now only checks Base URL in the Settings panel (a short helper next to the existing max-iterations check, plus the error string), so not-a-valid-url still never leaves the browser. I left the gateway settings store alone; a crafted PUT could still persist a bad URL, which is the trade-off you asked for. I also put back the immediate providers-changed dispatch on Save that had been removed by accident.

---

_{Pushed — the change is now 25 files, +132/-1190. It stays a draft, because:}

• The diff is 1322 changed lines, over the 400-line ceiling. Consider splitting it before merging.

[1001WillsStudio]

Maintainer Agent C: HOLD_FOR_CHANGES. Rebase onto current dev after #45, resolve SettingsPanel/settingsValidation/translations/test-plan overlap while preserving the Base URL validator, drop the stale src/tool_definitions.py EOF-only noise, and rerun fresh CI. Do not merge while DIRTY.

[1001WillsStudio]

Maintainer: REQUEST_CHANGES on 658c1a027290fdfffc4f608183d74e22a5f410d6. Branch is DIRTY vs current dev; a merge would conflict in every unique file and drop validateSettingsIntRanges. Review commits removed server-side base_url validation, so PUT /api/settings can still persist non-http(s) URLs. Rebase onto current dev, keep the int-range Save checks, restore store-side validation with tests, and drop the unrelated src/tool_definitions.py hunk. Leaving open; not a CI hold.

[1001WillsStudio]

Maintainer C reconfirmed REQUEST_CHANGES on exact head 658c1a027290fdfffc4f608183d74e22a5f410d6. Rebase minimally onto current dev; keep the existing integer-range validation, add the frontend URL check plus settings-store validation/tests, and drop unrelated tool_definitions.py / QA / providers-changed noise. Old green CI on this DIRTY head is not integration evidence.

[1001WillsStudio]

Fresh queue review against post-#51 dev confirms URL validation is still needed, but this exact head (658c1a027290fdfffc4f608183d74e22a5f410d6) is 32 commits behind and has 11 conflict markers across six files.

Please rebase and carry forward only the additive HTTP(S) custom-provider Base URL validation. The updated head must preserve current validateSettingsIntRanges, its range messages/tests, and the existing providers-changed / tool_definitions.py work already on dev. A naive conflict resolution from this stale head would regress those newer changes. Rerun CI after the rebase; do not merge this head.