fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(desktop): align the connection base URL gate with the Runtime Host - #3673

Open
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate
Open

fix(desktop): align the connection base URL gate with the Runtime Host#3673
shaokeyibb wants to merge 1 commit into
apache:mainfrom
shaokeyibb:fix/connection-base-url-client-gate

Conversation

@shaokeyibb

@shaokeyibbshaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Desktop connection forms accepted endpoints the Runtime Host refuses. A service URL carrying a query string, a fragment, or embedded credentials passed the add form's field gate, passed the main-process IPC gate, and then failed at the write — where the Host's English domain error reached the renderer unclassified and fell through to actionFallback:

模型连接服务暂时不可用,请稍后重试。
The model connection service is temporarily unavailable. Try again later.

That names neither the field nor the rule, and asks for a retry that can never succeed. Both write paths were affected: adding a custom relay and editing an existing endpoint.

Two validators had drifted. validateConnectionBaseUrl checked length, URL parseability, and an http/https allowlist, and documented "trim is the only canonicalization". normalizeCatalogConnectionBaseUrl additionally rejected credentials and ?/#, canonicalized through URL.toString(), and capped the canonical form at 2048 bytes.

The rule now has one implementation. canonicalizeConnectionBaseUrl holds the shape check, the scheme allowlist, the credential and query/fragment bans, both caps, and the canonical form. validateConnectionBaseUrl, normalizeConnectionBaseUrl, and the Host's normalizeCatalogConnectionBaseUrl all delegate to it and keep only their own wording — the Host's wire-facing sentences are byte-identical to before, since they are pinned by protocol tests. A rejection travels as a code (credentials, query_or_fragment, …), which is what lets the Desktop form render localized field copy from the same decision the Host makes.

Both write surfaces now check before they write: the add form marks its baseUrl field, and the detail page's endpoint row reports the rule that was broken instead of the generic service message.

Two notes on scope:

  • Canonicalization on the client is new and deliberate. The previous comment argued that rewriting https://Example.com:443/V1 could surprise whoever typed it that way. But the Host canonicalized on write regardless, so that form was never what got stored — the client was preserving a value the system discarded one hop later. Doing it here only moves the surprise to where the user can still see and correct it. The one test that pinned the old behavior is updated with that reasoning.
  • The Host contract is untouched. Relaxing it to allow query strings, which Azure-style ?api-version= gateways want, is a product and security decision for dev@maka.apache.org, not an implementation detail. This PR only makes the client agree with the contract that already exists.

Fixes#3672

Verification

npm run lint, npm run format:check, npm run build, npm run typecheck, npx knip --workspace apps/desktop, npx knip --workspace packages/ui — all clean.

SuiteResult
@maka/core651 pass / 0 fail
@maka/desktop1356 pass / 0 fail
@maka/storage906 pass / 0 fail (14 pre-existing skips)
@maka/runtime-host1119 pass / 0 fail

The reported scenario, before and after, run against the built tree:

 before after
1. add-form field gate ACCEPTED REJECTED {field: 'baseUrl', reason: 'invalid',
rejection: 'query_or_fragment'}
2. main-process IPC gate ACCEPTED REJECTED baseUrl must not contain a query or fragment
3. Runtime Host codec REJECTED REJECTED connection base URL must not contain a
query or fragment

The message the user reads at step 1 is now the rule, on the endpoint field:

en -> The service URL must not contain a query (?) or fragment (#). Use the advanced request
settings below to add parameters.
zh -> 服务地址不能包含查询参数(?)或锚点(#)。如需附加参数,请使用下方的高级请求设置。

New tests, each pinning one contract:

  • core: the four rules the client did not have (credentials, query, fragment, byte cap), plus a percent-encoded ? staying accepted — the Host reads the typed text rather than the parsed URL, and so does the client now.
  • core: the anti-drift test — a table of fifteen endpoints asserting the client gate and the Host codec agree on both the accept/reject verdict and the canonical value. The defect was drift between the two, not either one's rules, so this is the test that fails if they separate again.
  • desktop: the field gate's new invalid issues, that the rule is not relay-only (a local runtime's editable port is checked too), that Cloudflare stays exempt because its field holds an account id rather than a URL, and that a bare host, a local port, and a percent-encoded ? remain accepted.

Driven through the real app, using the settings-models e2e fixture whose seeded no-models connection is an openai-compatible relay with an editable endpoint row. Same steps in both: edit the service URL row, enter the URL above, save.

Toast
BeforeFailed to save model connection — The model connection service is temporarily unavailable. Try again later.
AfterFailed to save model connection — The service URL must not contain a query (?) or fragment (#). Use the advanced request settings below to add parameters.

The row stays open with the draft intact either way; what changes is whether the message describes the thing that went wrong. Screenshots follow in a comment.

Review focus

The canonicalization change is the one behavior change a reviewer should weigh rather than read past — it is what required editing an existing test's expectation rather than only adding to it. If the project would rather keep the client's as-typed value and fix only the misleading error, dropping the canonical form from canonicalizeConnectionBaseUrl and reverting that one assertion leaves the rest of the fix intact.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code diagnosed the drift between the two validators, implemented the shared rule across core / the Host codec / the Desktop forms and copy, and wrote the tests and verification above. I reviewed and verified the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

CopilotAI lite review requested due to automatic review settings August 24, 2026 04:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch 2 times, most recently from ca6e445 to 26b3d03CompareAugust 24, 2026 04:46

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Scope

First independent full review of exact head 26b3d031a39e3e75d84ef5eb28b64c4a7daed5f2.

I covered the shared base-URL validation/normalization contract, the Runtime Host catalog codec and canonical persisted-entry boundary, create/edit renderer-to-IPC flows, localized rejection copy, tests, and the exact-head CI result.

Findings

No P0–P3 findings in the covered scope.

The renderer and Runtime Host now use the same canonicalization authority. The codec continues to reject non-canonical or invalid persisted entries at the existing canonical decode boundary; I found no new silent-drop path or behavior that deletes legacy connections. Clear intent remains distinguishable from an omitted update, and OAuth/default endpoint handling remains explicit.

Verification

  • Exact-head CI test run 32691266671: terminal success.
  • Local @maka/core tests: 651 passed, 0 failed.
  • Changed-file Biome check: passed.
  • git diff --check: passed.

The desktop main build was not used as a PR signal because this shared workspace has pre-existing cross-package dist/type drift; its errors were outside the changed endpoint path.

Evidence gap

The PR changes user-visible endpoint error copy, but no screenshot or visual artifact is attached. The new focused tests validate behavior and copy selection, not rendered visual output. I record this as an acceptance-evidence gap, not a code finding.

@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor
beforeafter

FRI, here's before-after screenshots

@shaokeyibb
shaokeyibbforce-pushed the fix/connection-base-url-client-gate branch from 26b3d03 to 3a83833CompareAugust 24, 2026 08:43
@shaokeyibb

shaokeyibb commented Aug 24, 2026

Copy link
Copy Markdown
ContributorAuthor

rebased and resolve the conflict to match latest changes in main branch

@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch 3 times, most recently from 33a6108 to 7c860afCompareAugust 26, 2026 09:46
The Desktop forms accepted endpoints the Host refuses. A service URL
carrying a query string, a fragment, or embedded credentials passed the
add form, passed the IPC gate, and failed at the write — where the Host's
English domain error reached the renderer unclassified and fell through to
`actionFallback`: "the model connection service is temporarily
unavailable". That named neither the field nor the rule, and asked for a
retry that could never succeed.
The rule now has one implementation. `canonicalizeConnectionBaseUrl` holds
the shape check, the scheme allowlist, the credential and query/fragment
bans, the byte cap, and the canonical form; `validateConnectionBaseUrl`,
`normalizeConnectionBaseUrl`, and the Host's
`normalizeCatalogConnectionBaseUrl` all delegate to it and keep only their
own wording. A rejection travels as a code, so the Desktop form can mark
its endpoint field with localized copy while the Host keeps its
wire-facing sentences unchanged.
Both write surfaces check before they write: the add form reports on the
`baseUrl` field, and the detail page's endpoint row reports the rule
instead of the generic service message.
Canonicalization is new on the client and deliberate. The previous note
argued that rewriting `https://Example.com:443/V1` could surprise whoever
typed it — but the Host canonicalized on write regardless, so that form
was never what got stored. Doing it here only moves the surprise to where
the user can still correct it.
The Host contract is untouched. Allowing query strings, for Azure-style
`?api-version=` gateways, is a product and security decision for
dev@maka.apache.org, not an implementation detail.
Fixesapache#3672
Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@M4n5ter
M4n5terforce-pushed the fix/connection-base-url-client-gate branch from 7c860af to 565569fCompareAugust 26, 2026 09:53
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): connection base URL gate is looser than the Runtime Host contract, so a rejected endpoint reports a misleading error

3 participants

@shaokeyibb@Astro-Han