From 216bcfb6193f6a75a2f3cf62bf4b5132ca6c87e7 Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:06:08 -0700 Subject: [PATCH 1/2] patch: Make Admin status checks independent (#1954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Opening Integrations reparses native configuration files and queues every card behind one configuration lock. A large VS Code model file makes unrelated cards wait. Local provider reachability results also wait for the slowest check because they share one response. ## How Decode ordinary JSON with the standard parser while retaining JSON5 support and duplicate/nonfinite validation. Give each integration concurrent reader access and exclusive writer access, preserving configuration publication ordering, generation checks, multi-file consistency, and worker ownership through cancellation. Return local reachability results per provider and render each result independently. Track integration requests by their configuration so replies from an earlier load cannot overwrite newer results or clear their loading state.

RetriggerConfidence Score: 5/5

No outstanding finding or merge blocker was identified.
Summary Admin status checks run independently, so a slow integration or local provider check does not delay other results. The latest change synchronizes the browser test’s local-provider assertions.
Reviews (2) · Last reviewed commit: ["test: Synchronize local provider status ..."](https://github.com/alishahryar1/free-claude-code/commit/b89b44b2a3f67b8637368b4ca0fbe08f03b43116) --- e2e/test_admin_apply.py | 14 +- e2e/test_admin_providers.py | 49 ++- e2e/test_admin_status_independence.py | 139 +++++++ src/free_claude_code/api/admin_routes.py | 21 +- .../api/admin_static/admin.js | 123 +++--- src/free_claude_code/core/async_rwlock.py | 52 +++ src/free_claude_code/core/async_tasks.py | 6 +- .../harnesses/claude_desktop_integration.py | 9 +- .../harnesses/claude_integration.py | 8 +- src/free_claude_code/harnesses/config_file.py | 31 ++ .../harnesses/jetbrains_acp_integration.py | 7 +- .../harnesses/vscode_chat_integration.py | 6 +- .../runtime/integration_service.py | 374 ++++++++++-------- tests/api/test_admin.py | 54 ++- tests/api/test_admin_configuration_io.py | 2 +- .../test_integration_status_independence.py | 53 +++ tests/api/test_local_provider_status.py | 60 +++ tests/core/test_async_rwlock.py | 133 +++++++ tests/harnesses/test_native_config_json.py | 75 ++++ .../test_integration_status_concurrency.py | 283 +++++++++++++ 20 files changed, 1184 insertions(+), 315 deletions(-) create mode 100644 e2e/test_admin_status_independence.py create mode 100644 src/free_claude_code/core/async_rwlock.py create mode 100644 tests/api/test_integration_status_independence.py create mode 100644 tests/api/test_local_provider_status.py create mode 100644 tests/core/test_async_rwlock.py create mode 100644 tests/harnesses/test_native_config_json.py create mode 100644 tests/runtime/test_integration_status_concurrency.py diff --git a/e2e/test_admin_apply.py b/e2e/test_admin_apply.py index 2550d029a4..e2e9a8e3c1 100644 --- a/e2e/test_admin_apply.py +++ b/e2e/test_admin_apply.py @@ -107,7 +107,8 @@ def test_unverified_warning_survives_apply( ): availability: list[Route] = [] page.route( - "**/admin/api/providers/local-status", lambda route: availability.append(route) + "**/admin/api/providers/lmstudio/local-status", + lambda route: availability.append(route), ) page.route( "**/admin/api/config/apply", @@ -148,16 +149,17 @@ def test_unverified_warning_survives_apply( expect(page.locator("#messageArea")).to_contain_text("Verification unavailable.") current = page.locator('[data-provider-check-result="lmstudio"]') - with page.expect_response("**/admin/api/providers/local-status") as response: + with page.expect_response( + "**/admin/api/providers/lmstudio/local-status" + ) as response: old = availability.pop(0) if restart: old.fulfill(status=503, json={"detail": "Old check failed"}) else: payload = old.fetch().json() - for provider in payload["providers"]: - provider.update( - status="offline", label="Offline", message="Old availability result" - ) + payload.update( + status="offline", label="Offline", message="Old availability result" + ) old.fulfill(json=payload) response.value.finished() page.evaluate("() => new Promise(requestAnimationFrame)") diff --git a/e2e/test_admin_providers.py b/e2e/test_admin_providers.py index 009581ea31..f8d98bc07c 100644 --- a/e2e/test_admin_providers.py +++ b/e2e/test_admin_providers.py @@ -221,7 +221,8 @@ def test_local_model_discovery_takes_precedence_over_reachability( availability: list[Route] = [] page.route("**/admin/api/status", lambda route: startup.append(route)) page.route( - "**/admin/api/providers/local-status", lambda route: availability.append(route) + "**/admin/api/providers/lmstudio/local-status", + lambda route: availability.append(route), ) _open_admin(page, admin_base_url, {"width": 1280, "height": 720}) page.wait_for_function("!!state.startupRequest && !!state.localStatusRequest") @@ -298,9 +299,12 @@ def test_admin_loading_finishes_before_local_availability_checks( page: Page, admin_base_url: str ) -> None: pending: list[Route] = [] - page.route( - "**/admin/api/providers/local-status", lambda route: pending.append(route) - ) + + def hold(route): + pending.append(route) + page.evaluate("window.localChecks = (window.localChecks || 0) + 1") + + page.route("**/admin/api/providers/*/local-status", hold) _open_admin(page, admin_base_url, {"width": 1280, "height": 720}) open_provider(page, "nvidia_nim") key = page.locator("#field-NVIDIA_NIM_API_KEY") @@ -308,12 +312,14 @@ def test_admin_loading_finishes_before_local_availability_checks( expect(page.locator("#dirtyState")).to_have_text("No changes") expect(page.locator("#saveProvider")).to_be_enabled() - route = pending.pop() - payload = route.fetch().json() - providers = {provider["provider_id"]: provider for provider in payload["providers"]} - providers["llamacpp"].update(status="offline", label="Offline", status_code=503) - providers["ollama"].update(status="missing_url", label="Missing URL", base_url="") - route.fulfill(json=payload) + page.wait_for_function("window.localChecks === 3") + for route in pending: + payload = route.fetch().json() + if payload["provider_id"] == "llamacpp": + payload.update(status="offline", label="Offline", status_code=503) + elif payload["provider_id"] == "ollama": + payload.update(status="missing_url", label="Missing URL", base_url="") + route.fulfill(json=payload) expect(page.locator('[data-provider-check-result="lmstudio"]')).to_have_text( "Reachable: http://localhost:1234/v1" ) @@ -339,9 +345,10 @@ def test_local_availability_failure_does_not_fail_admin_loading( errors: list[str] = [] page.on("pageerror", lambda error: errors.append(str(error))) page.route( - "**/admin/api/providers/local-status", lambda route: pending.append(route) + "**/admin/api/providers/lmstudio/local-status", + lambda route: pending.append(route), ) - with page.expect_request("**/admin/api/providers/local-status"): + with page.expect_request("**/admin/api/providers/lmstudio/local-status"): page.goto(f"{admin_base_url}/admin") open_provider(page, "nvidia_nim") expect(page.locator("#field-NVIDIA_NIM_API_KEY")).to_be_editable() @@ -353,9 +360,12 @@ def test_local_availability_failure_does_not_fail_admin_loading( for provider_id in ("lmstudio", "llamacpp", "ollama"): card = page.locator(f'[data-provider="{provider_id}"]') - expect(card.locator(".provider-check-result")).to_have_text( - "Availability check failed. Use Test to retry." - ) + if provider_id == "lmstudio": + expect(card.locator(".provider-check-result")).to_have_text( + "Availability check failed. Use Test to retry." + ) + else: + expect(card.locator(".provider-check-result")).to_contain_text("Reachable:") expect(card.get_by_role("button", name="Edit", exact=True)).to_have_class( "secondary-button" ) @@ -377,7 +387,8 @@ def test_manual_provider_test_takes_precedence_over_automatic_availability( availability: list[Route] = [] manual: list[Route] = [] page.route( - "**/admin/api/providers/local-status", lambda route: availability.append(route) + "**/admin/api/providers/lmstudio/local-status", + lambda route: availability.append(route), ) page.route( "**/admin/api/providers/lmstudio/test", lambda route: manual.append(route) @@ -401,7 +412,9 @@ def test_manual_provider_test_takes_precedence_over_automatic_availability( "Unavailable: Could not refresh this provider's models." ) - with page.expect_response("**/admin/api/providers/local-status") as response: + with page.expect_response( + "**/admin/api/providers/lmstudio/local-status" + ) as response: if manual_finished: availability.pop().fulfill(status=503, json={"detail": "Check failed"}) else: @@ -413,7 +426,7 @@ def test_manual_provider_test_takes_precedence_over_automatic_availability( expect(result).to_have_text( "Unavailable: Could not refresh this provider's models." ) - expect(other).to_have_text("Availability check failed. Use Test to retry.") + expect(other).to_have_text("Reachable: http://localhost:11434") else: expect(result).to_have_text("Checking...") expect(other).to_have_text("Reachable: http://localhost:11434") diff --git a/e2e/test_admin_status_independence.py b/e2e/test_admin_status_independence.py new file mode 100644 index 0000000000..2c1549a0f8 --- /dev/null +++ b/e2e/test_admin_status_independence.py @@ -0,0 +1,139 @@ +import pytest +from playwright.sync_api import expect + + +@pytest.mark.parametrize( + "held_id,held_button", + [ + ("claude-vscode", "openClaudeIntegration"), + ("vscode-chat", "openVSCodeChatIntegration"), + ("codex", "openCodexIntegration"), + ("claude-desktop", "openClaudeDesktopIntegration"), + ("jetbrains-acp", "openJetBrainsIntegration"), + ], +) +def test_each_integration_finishes_without_slowest_card( + page, admin_base_url, held_id, held_button +): + pending = [] + + def hold(route): + pending.append(route) + page.evaluate("window.heldChecks = (window.heldChecks || 0) + 1") + + page.route(f"**/admin/api/integrations/{held_id}", hold) + page.goto(f"{admin_base_url}/admin/integrations") + for visit in range(2): + if visit: + page.get_by_role("button", name="Providers", exact=True).click() + page.get_by_role("button", name="Integrations", exact=True).click() + page.wait_for_function("count => window.heldChecks === count", arg=visit + 1) + expect(page.locator(f"#{held_button}")).to_have_text("Loading…") + for button in [ + "openClaudeIntegration", + "openVSCodeChatIntegration", + "openCodexIntegration", + "openClaudeDesktopIntegration", + "openJetBrainsIntegration", + ]: + if button != held_button: + expect(page.locator(f"#{button}")).to_have_text("Connect") + expect(page.locator(f"#{button}")).to_be_enabled() + pending.pop().fulfill(status=503, json={"detail": "This check failed"}) + expect(page.locator(f"#{held_button}")).to_have_text("Retry") + expect(page.locator(f"#{held_button}")).to_be_enabled() + + +def test_local_cards_render_before_slowest_check(page, admin_base_url): + pending = [] + + def hold(route): + pending.append(route) + page.evaluate("window.heldLocalCheck = true") + + page.route( + "**/admin/api/providers/ollama/local-status", + hold, + ) + page.route( + "**/admin/api/providers/lmstudio/local-status", + lambda route: route.fulfill( + json={ + "provider_id": "lmstudio", + "status": "reachable", + "base_url": "http://localhost:1234/v1", + } + ), + ) + page.route( + "**/admin/api/providers/llamacpp/local-status", + lambda route: route.fulfill(status=503, json={"detail": "Check failed"}), + ) + with ( + page.expect_response( + "**/admin/api/providers/lmstudio/local-status" + ) as lmstudio_response, + page.expect_response( + "**/admin/api/providers/llamacpp/local-status" + ) as llamacpp_response, + ): + page.goto(f"{admin_base_url}/admin") + page.wait_for_function("window.heldLocalCheck === true") + lmstudio_response.value.finished() + llamacpp_response.value.finished() + expect(page.locator('[data-provider-check-result="lmstudio"]')).to_have_text( + "Reachable: http://localhost:1234/v1" + ) + expect(page.locator('[data-provider-check-result="llamacpp"]')).to_have_text( + "Availability check failed. Use Test to retry." + ) + assert len(pending) == 1 + expect(page.locator('[data-provider-check-result="ollama"]')).to_be_hidden() + pending.pop().fulfill( + json={ + "provider_id": "ollama", + "status": "reachable", + "base_url": "http://localhost:11434", + } + ) + expect(page.locator('[data-provider-check-result="ollama"]')).to_have_text( + "Reachable: http://localhost:11434" + ) + + +def test_integration_check_from_previous_config_cannot_overwrite_new_result( + page, admin_base_url +): + pending = [] + + def hold(route): + pending.append(route) + page.evaluate("window.statusChecks = (window.statusChecks || 0) + 1") + + page.route("**/admin/api/integrations/vscode-chat", hold) + page.goto(f"{admin_base_url}/admin/integrations") + page.wait_for_function("window.statusChecks === 1") + expect(page.locator("#openJetBrainsIntegration")).to_be_enabled() + page.evaluate("void load()") + page.wait_for_function("window.statusChecks === 2", timeout=2000) + old, current = pending + current.fulfill( + json={ + "connected": True, + "paths": None, + "update": {"state": "ready", "changed": False, "message": None}, + } + ) + expect(page.locator("#openVSCodeChatIntegration")).to_have_text("Disconnect") + with page.expect_response("**/admin/api/integrations/vscode-chat") as response: + old.fulfill( + json={ + "connected": False, + "paths": None, + "update": {"state": "ready", "changed": False, "message": None}, + } + ) + response.value.finished() + page.evaluate("() => new Promise(requestAnimationFrame)") + expect(page.locator("#openVSCodeChatIntegration")).to_have_text("Disconnect") + expect(page.locator("#openVSCodeChatIntegration")).to_be_enabled() diff --git a/src/free_claude_code/api/admin_routes.py b/src/free_claude_code/api/admin_routes.py index 59c58de910..e3fefe4811 100644 --- a/src/free_claude_code/api/admin_routes.py +++ b/src/free_claude_code/api/admin_routes.py @@ -1,6 +1,5 @@ """Local admin UI routes and APIs.""" -import asyncio from collections.abc import Awaitable, Callable, Mapping from pathlib import Path @@ -161,26 +160,22 @@ async def admin_status( return await services.admin.admin_status() -@router.get("/admin/api/providers/local-status") +@router.get("/admin/api/providers/{provider_id}/local-status") async def local_provider_status( - request: Request, services: ApiServices = Depends(get_services) + provider_id: str, request: Request, services: ApiServices = Depends(get_services) ): require_loopback_admin(request) + if provider_id not in LOCAL_PROVIDER_PATHS: + raise HTTPException(status_code=404, detail="Local provider not found") values = { key: entry.value or "" for key, entry in (await services.admin.admin_values()).items() } - checks = await asyncio.gather( - *( - _check_local_provider( - provider_id, - _local_provider_url(provider_id, values), - path, - ) - for provider_id, path in LOCAL_PROVIDER_PATHS.items() - ) + return await _check_local_provider( + provider_id, + _local_provider_url(provider_id, values), + LOCAL_PROVIDER_PATHS[provider_id], ) - return {"providers": checks} @router.post("/admin/api/providers/{provider_id}/test") diff --git a/src/free_claude_code/api/admin_static/admin.js b/src/free_claude_code/api/admin_static/admin.js index 74dc9a1d5c..2afb166de3 100644 --- a/src/free_claude_code/api/admin_static/admin.js +++ b/src/free_claude_code/api/admin_static/admin.js @@ -1390,41 +1390,26 @@ async function refreshLocalStatus(config) { }; state.localStatusRequest = request; try { - const result = await api("/admin/api/providers/local-status"); - if (state.localStatusRequest !== request) return; - result.providers.forEach((provider) => { - if (!request.providerIds.has(provider.provider_id) || provider.status === "missing_url") return; - if (provider.status === "reachable") { - updateProviderCheckResult( - provider.provider_id, - "ok", - `Reachable: ${provider.base_url}`, - "availability", - ); - return; + await Promise.all([...request.providerIds].map(async (providerId) => { + try { + const provider = await api(`/admin/api/providers/${providerId}/local-status`); + if (state.localStatusRequest !== request || !request.providerIds.has(providerId)) return; + if (provider.status === "missing_url") return; + if (provider.status === "reachable") { + updateProviderCheckResult(providerId, "ok", `Reachable: ${provider.base_url}`, "availability"); + return; + } + const detail = provider.message + ? provider.message + : provider.status_code + ? `${provider.base_url} returned HTTP ${provider.status_code}` + : "The local provider did not respond."; + updateProviderCheckResult(providerId, "error", `Unavailable: ${detail}`, "availability"); + } catch { + if (state.localStatusRequest !== request || !request.providerIds.has(providerId)) return; + updateProviderCheckResult(providerId, "error", "Availability check failed. Use Test to retry.", "availability"); } - const detail = provider.message - ? provider.message - : provider.status_code - ? `${provider.base_url} returned HTTP ${provider.status_code}` - : "The local provider did not respond."; - updateProviderCheckResult( - provider.provider_id, - "error", - `Unavailable: ${detail}`, - "availability", - ); - }); - } catch { - if (state.localStatusRequest !== request) return; - request.providerIds.forEach((providerId) => { - updateProviderCheckResult( - providerId, - "error", - "Availability check failed. Use Test to retry.", - "availability", - ); - }); + })); } finally { if (state.localStatusRequest === request) state.localStatusRequest = null; } @@ -1605,6 +1590,26 @@ function integrationUpdating(integration, id) { state.startup?.startup?.integrations?.[id]?.state === "starting"; } +function beginIntegrationCheck(integration) { + if (integration.busy && (!integration.checkRequest || integration.checkRequest.config === state.config)) return null; + const request = { config: state.config }; + integration.checkRequest = request; + integration.busy = true; + return request; +} + +function currentIntegrationCheck(integration, request) { + return integration.checkRequest === request && request.config === state.config; +} + +function finishIntegrationCheck(integration, request, render) { + if (integration.checkRequest !== request) return; + integration.checkRequest = null; + integration.busy = false; + render(); + if (request.config === state.config && integration.update?.state === "starting") void refreshStartup(); +} + function refreshIntegrationUpdates(previous, current) { if (state.activeView !== "integrations") return; for (const [id, integration, refresh, messageId, message] of [ @@ -1660,25 +1665,25 @@ function renderClaudeIntegration() { async function refreshClaudeIntegration(retry = false, { background = false } = {}) { if (!background) integrationMessage("claudeIntegrationMessage", ""); - if (claudeIntegration.busy) return; - claudeIntegration.busy = true; + const request = beginIntegrationCheck(claudeIntegration); + if (!request) return; renderClaudeIntegration(); try { if (retry) await api(`${claudeIntegrationPath}/refresh`, { method: "POST" }); const result = await api(claudeIntegrationPath); + if (!currentIntegrationCheck(claudeIntegration, request)) return; claudeIntegration.connected = result.connected; claudeIntegration.paths = result.paths; claudeIntegration.update = result.update; if (result.update?.state === "failed") integrationMessage("claudeIntegrationMessage", result.update.message, true); else if (byId("claudeIntegrationMessage").classList.contains("error")) integrationMessage("claudeIntegrationMessage", ""); } catch (error) { + if (!currentIntegrationCheck(claudeIntegration, request)) return; claudeIntegration.connected = null; claudeIntegration.update = null; integrationMessage("claudeIntegrationMessage", error.message, true); } finally { - claudeIntegration.busy = false; - renderClaudeIntegration(); - if (claudeIntegration.update?.state === "starting") void refreshStartup(); + finishIntegrationCheck(claudeIntegration, request, renderClaudeIntegration); } } @@ -1758,25 +1763,25 @@ function renderVSCodeChatIntegration() { async function refreshVSCodeChatIntegration(retry = false, { background = false } = {}) { if (!background) integrationMessage("vscodeChatIntegrationMessage", ""); - if (vscodeChatIntegration.busy) return; - vscodeChatIntegration.busy = true; + const request = beginIntegrationCheck(vscodeChatIntegration); + if (!request) return; renderVSCodeChatIntegration(); try { if (retry) await api(`${vscodeChatIntegrationPath}/refresh`, { method: "POST" }); const result = await api(vscodeChatIntegrationPath); + if (!currentIntegrationCheck(vscodeChatIntegration, request)) return; vscodeChatIntegration.connected = result.connected; vscodeChatIntegration.paths = result.paths; vscodeChatIntegration.update = result.update; if (result.update?.state === "failed") integrationMessage("vscodeChatIntegrationMessage", result.update.message, true); else if (byId("vscodeChatIntegrationMessage").classList.contains("error")) integrationMessage("vscodeChatIntegrationMessage", ""); } catch (error) { + if (!currentIntegrationCheck(vscodeChatIntegration, request)) return; vscodeChatIntegration.connected = null; vscodeChatIntegration.update = null; integrationMessage("vscodeChatIntegrationMessage", error.message, true); } finally { - vscodeChatIntegration.busy = false; - renderVSCodeChatIntegration(); - if (vscodeChatIntegration.update?.state === "starting") void refreshStartup(); + finishIntegrationCheck(vscodeChatIntegration, request, renderVSCodeChatIntegration); } } @@ -1860,25 +1865,25 @@ function renderCodexIntegration() { async function refreshCodexIntegration(retry = false, { background = false } = {}) { if (!background) integrationMessage("codexIntegrationMessage", ""); - if (codexIntegration.busy) return; - codexIntegration.busy = true; + const request = beginIntegrationCheck(codexIntegration); + if (!request) return; renderCodexIntegration(); try { if (retry) await api(`${codexIntegrationPath}/refresh`, { method: "POST" }); const result = await api(codexIntegrationPath); + if (!currentIntegrationCheck(codexIntegration, request)) return; codexIntegration.connected = result.connected; codexIntegration.paths = result.paths; codexIntegration.update = result.update; if (result.update?.state === "failed") integrationMessage("codexIntegrationMessage", result.update.message, true); else if (byId("codexIntegrationMessage").classList.contains("error")) integrationMessage("codexIntegrationMessage", ""); } catch (error) { + if (!currentIntegrationCheck(codexIntegration, request)) return; codexIntegration.connected = null; codexIntegration.update = null; integrationMessage("codexIntegrationMessage", error.message, true); } finally { - codexIntegration.busy = false; - renderCodexIntegration(); - if (codexIntegration.update?.state === "starting") void refreshStartup(); + finishIntegrationCheck(codexIntegration, request, renderCodexIntegration); } } @@ -1957,25 +1962,25 @@ function renderJetBrainsIntegration() { async function refreshJetBrainsIntegration(retry = false, { background = false } = {}) { if (!background) integrationMessage("jetBrainsIntegrationMessage", ""); - if (jetBrainsIntegration.busy) return; - jetBrainsIntegration.busy = true; + const request = beginIntegrationCheck(jetBrainsIntegration); + if (!request) return; renderJetBrainsIntegration(); try { if (retry) await api(`${jetBrainsIntegrationPath}/refresh`, { method: "POST" }); const result = await api(jetBrainsIntegrationPath); + if (!currentIntegrationCheck(jetBrainsIntegration, request)) return; jetBrainsIntegration.connected = result.connected; jetBrainsIntegration.paths = result.paths; jetBrainsIntegration.update = result.update; if (result.update?.state === "failed") integrationMessage("jetBrainsIntegrationMessage", result.update.message, true); else if (byId("jetBrainsIntegrationMessage").classList.contains("error")) integrationMessage("jetBrainsIntegrationMessage", ""); } catch (error) { + if (!currentIntegrationCheck(jetBrainsIntegration, request)) return; jetBrainsIntegration.connected = null; jetBrainsIntegration.update = null; integrationMessage("jetBrainsIntegrationMessage", error.message, true); } finally { - jetBrainsIntegration.busy = false; - renderJetBrainsIntegration(); - if (jetBrainsIntegration.update?.state === "starting") void refreshStartup(); + finishIntegrationCheck(jetBrainsIntegration, request, renderJetBrainsIntegration); } } @@ -2056,12 +2061,13 @@ function renderClaudeDesktopIntegration() { async function refreshClaudeDesktopIntegration(retry = false, { background = false } = {}) { if (!background) integrationMessage("claudeDesktopIntegrationMessage", ""); - if (claudeDesktopIntegration.busy) return; - claudeDesktopIntegration.busy = true; + const request = beginIntegrationCheck(claudeDesktopIntegration); + if (!request) return; renderClaudeDesktopIntegration(); try { if (retry) await api(`${claudeDesktopIntegrationPath}/refresh`, { method: "POST" }); const result = await api(claudeDesktopIntegrationPath); + if (!currentIntegrationCheck(claudeDesktopIntegration, request)) return; claudeDesktopIntegration.connected = result.connected; claudeDesktopIntegration.disconnectPending = result.disconnect_pending; claudeDesktopIntegration.paths = result.paths; @@ -2069,14 +2075,13 @@ async function refreshClaudeDesktopIntegration(retry = false, { background = fal if (result.update?.state === "failed") integrationMessage("claudeDesktopIntegrationMessage", result.update.message, true); else if (byId("claudeDesktopIntegrationMessage").classList.contains("error")) integrationMessage("claudeDesktopIntegrationMessage", ""); } catch (error) { + if (!currentIntegrationCheck(claudeDesktopIntegration, request)) return; claudeDesktopIntegration.connected = null; claudeDesktopIntegration.disconnectPending = false; claudeDesktopIntegration.update = null; integrationMessage("claudeDesktopIntegrationMessage", error.message, true); } finally { - claudeDesktopIntegration.busy = false; - renderClaudeDesktopIntegration(); - if (claudeDesktopIntegration.update?.state === "starting") void refreshStartup(); + finishIntegrationCheck(claudeDesktopIntegration, request, renderClaudeDesktopIntegration); } } diff --git a/src/free_claude_code/core/async_rwlock.py b/src/free_claude_code/core/async_rwlock.py new file mode 100644 index 0000000000..4c9f65ff07 --- /dev/null +++ b/src/free_claude_code/core/async_rwlock.py @@ -0,0 +1,52 @@ +"""Concurrent readers and exclusive writers with preference for queued writes.""" + +import asyncio +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager + +from .async_tasks import wait_owned + + +class AsyncReadWriteLock: + def __init__(self) -> None: + self._condition = asyncio.Condition() + self._readers = 0 + self._writer = False + self._waiting_writers = 0 + + @asynccontextmanager + async def read(self) -> AsyncIterator[None]: + async with self._condition: + await self._condition.wait_for( + lambda: not self._writer and not self._waiting_writers + ) + self._readers += 1 + try: + yield + finally: + await wait_owned(asyncio.create_task(self._release(writer=False))) + + @asynccontextmanager + async def write(self) -> AsyncIterator[None]: + async with self._condition: + self._waiting_writers += 1 + try: + await self._condition.wait_for( + lambda: not self._writer and not self._readers + ) + self._writer = True + finally: + self._waiting_writers -= 1 + self._condition.notify_all() + try: + yield + finally: + await wait_owned(asyncio.create_task(self._release(writer=True))) + + async def _release(self, *, writer: bool) -> None: + async with self._condition: + if writer: + self._writer = False + else: + self._readers -= 1 + self._condition.notify_all() diff --git a/src/free_claude_code/core/async_tasks.py b/src/free_claude_code/core/async_tasks.py index f5fb8edb17..81afae291a 100644 --- a/src/free_claude_code/core/async_tasks.py +++ b/src/free_claude_code/core/async_tasks.py @@ -8,7 +8,11 @@ async def run_sync_owned[T](function: Callable[[], T]) -> T: """Drain a finite worker before propagating cancellation; never abandon it.""" - worker = asyncio.create_task(to_thread.run_sync(function)) + return await wait_owned(asyncio.create_task(to_thread.run_sync(function))) + + +async def wait_owned[T](worker: asyncio.Task[T]) -> T: + """Finish owned work before propagating any caller cancellation.""" cancellation: asyncio.CancelledError | None = None while not worker.done(): try: diff --git a/src/free_claude_code/harnesses/claude_desktop_integration.py b/src/free_claude_code/harnesses/claude_desktop_integration.py index 129612ee8d..3977cc1788 100644 --- a/src/free_claude_code/harnesses/claude_desktop_integration.py +++ b/src/free_claude_code/harnesses/claude_desktop_integration.py @@ -11,11 +11,9 @@ from urllib.parse import urlsplit from uuid import NAMESPACE_URL, uuid5 -import json5 - from free_claude_code.config.server_urls import same_proxy_url from free_claude_code.core.json_types import JsonObject -from free_claude_code.harnesses.config_file import atomic_write_text +from free_claude_code.harnesses.config_file import atomic_write_text, decode_json _IDENTITY = ( "https://github.com/Alishahryar1/free-claude-code/integrations/claude-desktop" @@ -111,14 +109,11 @@ def check_unmanaged() -> None: def _read(path: Path) -> JsonObject | None: try: - value = json5.loads( - path.read_text(encoding="utf-8-sig"), allow_duplicate_keys=False - ) + value = decode_json(path.read_text(encoding="utf-8-sig")) except FileNotFoundError: return None if not isinstance(value, dict): raise ValueError("Desktop settings must be objects") - json.dumps(value, allow_nan=False) return cast(JsonObject, value) diff --git a/src/free_claude_code/harnesses/claude_integration.py b/src/free_claude_code/harnesses/claude_integration.py index 64c4c4af16..48f45ec869 100644 --- a/src/free_claude_code/harnesses/claude_integration.py +++ b/src/free_claude_code/harnesses/claude_integration.py @@ -6,12 +6,10 @@ from pathlib import Path from typing import cast -import json5 - from free_claude_code.config.server_urls import same_proxy_url from free_claude_code.core.json_types import JsonObject from free_claude_code.harnesses.claude import claude_proxy_values -from free_claude_code.harnesses.config_file import atomic_write_text +from free_claude_code.harnesses.config_file import atomic_write_text, decode_json _ENV = "claudeCode.environmentVariables" _LOGIN = "claudeCode.disableLoginPrompt" @@ -41,11 +39,9 @@ def _read_object(path: Path) -> JsonObject: source = path.read_text(encoding="utf-8-sig") except FileNotFoundError: return {} - document = json5.loads(source, allow_duplicate_keys=False) + document = decode_json(source) if not isinstance(document, dict): raise ValueError("Settings must be an object") - # Also reject non-finite JSON5 numbers before any operation or status result. - json.dumps(document, allow_nan=False) return cast(JsonObject, document) diff --git a/src/free_claude_code/harnesses/config_file.py b/src/free_claude_code/harnesses/config_file.py index 9d59d5c8e0..b438c41550 100644 --- a/src/free_claude_code/harnesses/config_file.py +++ b/src/free_claude_code/harnesses/config_file.py @@ -1,9 +1,40 @@ """Atomic text-file replacement for local client configuration.""" +import json import os import stat import tempfile from pathlib import Path +from typing import cast + +import json5 + +from free_claude_code.core.json_types import JsonObject, JsonValue + + +def _unique_object(pairs: list[tuple[str, JsonValue]]) -> JsonObject: + document: JsonObject = {} + for key, value in pairs: + if key in document: + raise ValueError("Duplicate configuration key") + document[key] = value + return document + + +def _reject_constant(value: str) -> JsonValue: + raise ValueError("Nonfinite configuration value") + + +def decode_json(source: str) -> JsonValue: + """Decode native JSON/JSON5 without duplicate or nonfinite values.""" + try: + document = json.loads( + source, object_pairs_hook=_unique_object, parse_constant=_reject_constant + ) + except json.JSONDecodeError: + document = json5.loads(source, allow_duplicate_keys=False) + json.dumps(document, allow_nan=False) + return cast(JsonValue, document) def ensure_private_permissions(path: Path) -> None: diff --git a/src/free_claude_code/harnesses/jetbrains_acp_integration.py b/src/free_claude_code/harnesses/jetbrains_acp_integration.py index 56a331e1ef..f9ef352972 100644 --- a/src/free_claude_code/harnesses/jetbrains_acp_integration.py +++ b/src/free_claude_code/harnesses/jetbrains_acp_integration.py @@ -9,11 +9,9 @@ from pathlib import Path from typing import cast -import json5 - from free_claude_code.core.json_types import JsonObject from free_claude_code.harnesses.claude import claude_proxy_values -from free_claude_code.harnesses.config_file import atomic_write_text +from free_claude_code.harnesses.config_file import atomic_write_text, decode_json _NAME = "Claude Code (FCC)" _MARKER = "FCC_JETBRAINS_ACP" @@ -71,8 +69,7 @@ def _read(path: Path) -> JsonObject: source = path.read_text(encoding="utf-8-sig") except FileNotFoundError: return {} - value = _object(json5.loads(source, allow_duplicate_keys=False)) - json.dumps(value, allow_nan=False) + value = _object(decode_json(source)) return value diff --git a/src/free_claude_code/harnesses/vscode_chat_integration.py b/src/free_claude_code/harnesses/vscode_chat_integration.py index 6bd261cb94..c1c7cbebf4 100644 --- a/src/free_claude_code/harnesses/vscode_chat_integration.py +++ b/src/free_claude_code/harnesses/vscode_chat_integration.py @@ -5,8 +5,6 @@ from pathlib import Path from typing import cast -import json5 - from free_claude_code.application.model_catalog import ( CatalogModel, context_window_for_client, @@ -20,6 +18,7 @@ from free_claude_code.harnesses.claude_integration import settings_path from free_claude_code.harnesses.config_file import ( atomic_write_text, + decode_json, ensure_private_permissions, ) from free_claude_code.harnesses.model_policy import ( @@ -65,10 +64,9 @@ def _read(path: Path) -> tuple[list[JsonObject], JsonObject | None]: source = path.read_text(encoding="utf-8-sig") except FileNotFoundError: return [], None - document = json5.loads(source, allow_duplicate_keys=False) + document = decode_json(source) if not isinstance(document, list) or any(not isinstance(g, dict) for g in document): raise ValueError("Model configuration must be an array of groups") - json.dumps(document, allow_nan=False) groups = cast(list[JsonObject], document) owned = [g for g in groups if g.get(_MARKER) == "vscode"] if len(owned) > 1: diff --git a/src/free_claude_code/runtime/integration_service.py b/src/free_claude_code/runtime/integration_service.py index d6e3ffea52..1b261cad9d 100644 --- a/src/free_claude_code/runtime/integration_service.py +++ b/src/free_claude_code/runtime/integration_service.py @@ -2,7 +2,7 @@ import asyncio from collections.abc import Awaitable, Callable -from dataclasses import dataclass +from dataclasses import dataclass, field from functools import partial from typing import Literal @@ -21,6 +21,7 @@ ) from free_claude_code.config.server_urls import local_proxy_root_url from free_claude_code.config.settings import Settings +from free_claude_code.core.async_rwlock import AsyncReadWriteLock from free_claude_code.core.async_tasks import run_sync_owned from free_claude_code.core.json_types import JsonObject from free_claude_code.harnesses import ( @@ -42,6 +43,7 @@ class _IntegrationUpdate: changed: bool = False message: str | None = None task: asyncio.Task[None] | None = None + access: AsyncReadWriteLock = field(default_factory=AsyncReadWriteLock) def snapshot(self) -> JsonObject: return {"state": self.state, "changed": self.changed, "message": self.message} @@ -161,16 +163,22 @@ async def drain() -> None: async def _vscode_chat(self, action: IntegrationAction) -> JsonObject: try: + if action == "status": + return await self._read_status( + self._vscode_update, + lambda url, token, ready: vscode_chat_integration.status( + vscode_chat_integration.config_path() + ), + ) if action == "refresh": - async with self._config_lock: - self._check_integration_available() - status = await run_sync_owned( - lambda: vscode_chat_integration.status( - vscode_chat_integration.config_path() - ) - ) - if not status["connected"]: - return {"changed": False} + status = await self._read_status( + self._vscode_update, + lambda url, token, ready: vscode_chat_integration.status( + vscode_chat_integration.config_path() + ), + ) + if not status["connected"]: + return {"changed": False} while True: snapshot = ( await self.provider_manager.wait_for_catalog() @@ -178,7 +186,7 @@ async def _vscode_chat(self, action: IntegrationAction) -> JsonObject: else None ) revision = self.provider_manager.catalog_status()["catalog_revision"] - async with self._config_lock: + async with self._config_lock, self._vscode_update.access.write(): self._check_integration_available() if snapshot is not None and ( snapshot.current_settings() is not self.settings @@ -190,11 +198,13 @@ async def _vscode_chat(self, action: IntegrationAction) -> JsonObject: catalog = ( read_model_catalog(snapshot) if snapshot is not None else None ) - settings = self.settings + url = local_proxy_root_url(self.settings) + token = self.settings.proxy_auth_token def operate( catalog: ModelCatalog | None = catalog, - settings: Settings = settings, + url: str = url, + token: str = token, ) -> JsonObject: path = vscode_chat_integration.config_path() changed = False @@ -203,8 +213,8 @@ def operate( elif catalog is not None: changed = vscode_chat_integration.configure( path, - local_proxy_root_url(settings), - settings.proxy_auth_token, + url, + token, catalog.models, only_existing=action == "refresh", ) @@ -215,8 +225,6 @@ def operate( result = await run_sync_owned(operate) if action in {"connect", "disconnect"}: self._vscode_update.complete() - if action == "status": - result["update"] = self._vscode_update.snapshot() return result except ValueError, UnicodeError: raise InvalidRequestError( @@ -242,6 +250,61 @@ def _check_integration_available(self) -> None: "Wait for FCC to restart before changing the integration." ) + async def _read_status( + self, + update: _IntegrationUpdate, + operation: Callable[[str, str, bool], JsonObject], + *, + mask_when_unready: bool = False, + ) -> JsonObject: + for _ in range(2): + async with update.access.read(): + self._check_integration_available() + generation = self.provider_manager.current_generation_id + url = local_proxy_root_url(self.settings) + token = self.settings.proxy_auth_token + ready = update.state == "ready" + failure: ValueError | OSError | None = None + result: JsonObject = {} + try: + result = await run_sync_owned(partial(operation, url, token, ready)) + except (ValueError, OSError) as exc: + failure = exc + self._check_integration_available() + if generation != self.provider_manager.current_generation_id or ( + mask_when_unready and not ready and update.state == "ready" + ): + continue + if failure is not None: + raise failure + if mask_when_unready and update.state != "ready": + result["connected"] = None + result["update"] = update.snapshot() + return result + raise ApplicationUnavailableError( + "FCC configuration changed during the check. Retry shortly." + ) + + async def _run_integration( + self, + update: _IntegrationUpdate, + action: IntegrationAction, + operation: Callable[[str, str, bool], JsonObject], + *, + mask_when_unready: bool = False, + ) -> JsonObject: + if action == "status": + return await self._read_status( + update, operation, mask_when_unready=mask_when_unready + ) + async with self._config_lock, update.access.write(): + self._check_integration_available() + url = local_proxy_root_url(self.settings) + token = self.settings.proxy_auth_token + result = await run_sync_owned(partial(operation, url, token, True)) + update.complete(action == "refresh" and result.get("changed") is True) + return result + def _start_integration_update( self, update: _IntegrationUpdate, @@ -294,43 +357,30 @@ async def refresh_jetbrains_acp(self) -> JsonObject: ) async def _jetbrains_acp(self, action: IntegrationAction) -> JsonObject: - async with self._config_lock: - self._check_integration_available() - settings = self.settings - - def operate() -> JsonObject: - path = jetbrains_acp_integration.config_path() - url = local_proxy_root_url(settings) - if action == "refresh": - return { - "changed": jetbrains_acp_integration.refresh_connected( - path, url, settings.proxy_auth_token - ) - } - return jetbrains_acp_integration.configure( - path, - url, - settings.proxy_auth_token, - None if action == "status" else action == "connect", - ) + def operate(url: str, token: str, ready: bool) -> JsonObject: + path = jetbrains_acp_integration.config_path() + if action == "refresh": + return { + "changed": jetbrains_acp_integration.refresh_connected( + path, url, token + ) + } + return jetbrains_acp_integration.configure( + path, url, token, None if action == "status" else action == "connect" + ) - try: - result = await run_sync_owned(operate) - except jetbrains_acp_integration.SetupError as exc: - raise InvalidRequestError(str(exc)) from None - except ValueError, UnicodeError: - raise InvalidRequestError( - "Could not read JetBrains ACP configuration. Check the JSON in acp.json and the installed Claude Agent metadata." - ) from None - except OSError: - raise ApplicationUnavailableError( - "Could not access JetBrains ACP files. Finish any configuration edits, check file permissions, and retry." - ) from None - if action in {"connect", "disconnect"}: - self._jetbrains_update.complete() - if action == "status": - result["update"] = self._jetbrains_update.snapshot() - return result + try: + return await self._run_integration(self._jetbrains_update, action, operate) + except jetbrains_acp_integration.SetupError as exc: + raise InvalidRequestError(str(exc)) from None + except ValueError, UnicodeError: + raise InvalidRequestError( + "Could not read JetBrains ACP configuration. Check the JSON in acp.json and the installed Claude Agent metadata." + ) from None + except OSError: + raise ApplicationUnavailableError( + "Could not access JetBrains ACP files. Finish any configuration edits, check file permissions, and retry." + ) from None async def claude_desktop_status(self) -> JsonObject: return await self._claude_desktop("status") @@ -347,108 +397,88 @@ async def refresh_claude_desktop(self) -> JsonObject: ) async def _claude_desktop(self, action: IntegrationAction) -> JsonObject: - async with self._config_lock: - self._check_integration_available() - settings = self.settings - - def operate() -> JsonObject: - root = claude_desktop_integration.config_root() - url = local_proxy_root_url(settings) - if action == "refresh": - return { - "changed": claude_desktop_integration.refresh_connected( - root, - url, - settings.proxy_auth_token, - disconnect_path=claude_desktop_disconnect_path(), - ) - } - return claude_desktop_integration.configure( - root, - url, - settings.proxy_auth_token, - None if action == "status" else action == "connect", - disconnect_path=claude_desktop_disconnect_path(), - ) + def operate(url: str, token: str, ready: bool) -> JsonObject: + root = claude_desktop_integration.config_root() + if action == "refresh": + return { + "changed": claude_desktop_integration.refresh_connected( + root, + url, + token, + disconnect_path=claude_desktop_disconnect_path(), + ) + } + return claude_desktop_integration.configure( + root, + url, + token, + None if action == "status" else action == "connect", + disconnect_path=claude_desktop_disconnect_path(), + ) - try: - result = await run_sync_owned(operate) - except claude_desktop_integration.ManagedDesktopError: - raise InvalidRequestError( - "Claude Desktop is managed by an organization, or its policy could not be read. FCC can configure only unmanaged Desktop installations." - ) from None - except claude_desktop_integration.PendingDisconnectError: - raise InvalidRequestError( - "Finish disconnecting Claude Desktop before connecting again." - ) from None - except claude_desktop_integration.PendingMigrationError: - raise InvalidRequestError( - "Claude Desktop has data in its previous Windows location. Launch Claude Desktop once so it can migrate that data, fully quit it, then retry Connect." - ) from None - except ValueError, UnicodeError: - raise InvalidRequestError( - "Could not configure Claude Desktop. Check its configuration JSON and FCC disconnect record, and ensure FCC uses a localhost address and a nonempty managed token." - ) from None - except OSError: - raise ApplicationUnavailableError( - "Could not access Claude Desktop settings or the FCC disconnect record. Fully quit Claude Desktop, check file permissions, and retry." - ) from None - if action in {"connect", "disconnect"}: - self._desktop_update.complete() - if action == "status": - if self._desktop_update.state != "ready": - result["connected"] = None - result["update"] = self._desktop_update.snapshot() - return result + try: + return await self._run_integration( + self._desktop_update, action, operate, mask_when_unready=True + ) + except claude_desktop_integration.ManagedDesktopError: + raise InvalidRequestError( + "Claude Desktop is managed by an organization, or its policy could not be read. FCC can configure only unmanaged Desktop installations." + ) from None + except claude_desktop_integration.PendingDisconnectError: + raise InvalidRequestError( + "Finish disconnecting Claude Desktop before connecting again." + ) from None + except claude_desktop_integration.PendingMigrationError: + raise InvalidRequestError( + "Claude Desktop has data in its previous Windows location. Launch Claude Desktop once so it can migrate that data, fully quit it, then retry Connect." + ) from None + except ValueError, UnicodeError: + raise InvalidRequestError( + "Could not configure Claude Desktop. Check its configuration JSON and FCC disconnect record, and ensure FCC uses a localhost address and a nonempty managed token." + ) from None + except OSError: + raise ApplicationUnavailableError( + "Could not access Claude Desktop settings or the FCC disconnect record. Fully quit Claude Desktop, check file permissions, and retry." + ) from None async def _claude_vscode(self, action: IntegrationAction) -> JsonObject: - async with self._config_lock: - self._check_integration_available() - settings = self.settings - - def operate() -> JsonObject: - path = claude_integration.settings_path() - state_path = claude_integration.claude_state_path() - if action == "status" and self._claude_update.state != "ready": - return { - "connected": None, - "paths": { - "vscode_settings": str(path.resolve()), - "claude_state": str(state_path.resolve()), - }, - } - if action == "refresh": - return { - "changed": claude_integration.refresh_connected( - path, - state_path, - local_proxy_root_url(settings), - settings.proxy_auth_token, - ) - } - return claude_integration.configure( - path, - state_path, - local_proxy_root_url(settings), - settings.proxy_auth_token, - None if action == "status" else action == "connect", - ) + def operate(url: str, token: str, ready: bool) -> JsonObject: + path = claude_integration.settings_path() + state_path = claude_integration.claude_state_path() + if action == "status" and not ready: + return { + "connected": None, + "paths": { + "vscode_settings": str(path.resolve()), + "claude_state": str(state_path.resolve()), + }, + } + if action == "refresh": + return { + "changed": claude_integration.refresh_connected( + path, state_path, url, token + ) + } + return claude_integration.configure( + path, + state_path, + url, + token, + None if action == "status" else action == "connect", + ) - try: - result = await run_sync_owned(operate) - except ValueError, UnicodeError: - raise InvalidRequestError( - "Could not read Claude integration settings. Check the JSON in VS Code settings.json and .claude.json." - ) from None - except OSError: - raise ApplicationUnavailableError( - "Could not access VS Code settings.json or .claude.json. Check file permissions and try again." - ) from None - if action in {"connect", "disconnect"}: - self._claude_update.complete() - if action == "status": - result["update"] = self._claude_update.snapshot() - return result + try: + return await self._run_integration( + self._claude_update, action, operate, mask_when_unready=True + ) + except ValueError, UnicodeError: + raise InvalidRequestError( + "Could not read Claude integration settings. Check the JSON in VS Code settings.json and .claude.json." + ) from None + except OSError: + raise ApplicationUnavailableError( + "Could not access VS Code settings.json or .claude.json. Check file permissions and try again." + ) from None async def codex_integration_status(self) -> JsonObject: return await self._codex_integration("status") @@ -463,23 +493,40 @@ async def _codex_integration(self, action: IntegrationAction) -> JsonObject: wait = InitializationWait(None) if action == "refresh" else InitializationWait() needs_catalog = action in {"connect", "refresh"} try: + if action == "status": + + def status(url: str, token: str, ready: bool) -> JsonObject: + path = codex_integration.config_path() + if not ready: + return { + "connected": None, + "paths": {"codex_config": str(path.resolve())}, + } + return codex_integration.configure( + path, codex_model_catalog_path(), url + ) + + return await self._read_status( + self._codex_update, status, mask_when_unready=True + ) if action == "refresh": - async with self._config_lock: - self._check_integration_available() - url = local_proxy_root_url(self.settings) - if not await run_sync_owned( - lambda: codex_integration.recognizes_connection( + existing = await self._read_status( + self._codex_update, + lambda url, token, ready: { + "connected": codex_integration.recognizes_connection( codex_integration.config_path(), url ) - ): - return {"changed": False} + }, + ) + if not existing["connected"]: + return {"changed": False} while True: generation_id = ( await self.provider_manager.wait_for_catalog_file(wait) if needs_catalog else None ) - async with self._config_lock: + async with self._config_lock, self._codex_update.access.write(): self._check_integration_available() if needs_catalog and ( generation_id != self.provider_manager.current_generation_id @@ -490,11 +537,6 @@ async def _codex_integration(self, action: IntegrationAction) -> JsonObject: def operate(url: str = url) -> JsonObject: path = codex_integration.config_path() - if action == "status" and self._codex_update.state != "ready": - return { - "connected": None, - "paths": {"codex_config": str(path.resolve())}, - } if action == "refresh": return { "changed": codex_integration.refresh_connected( @@ -505,14 +547,12 @@ def operate(url: str = url) -> JsonObject: path, codex_model_catalog_path(), url, - None if action == "status" else action == "connect", + action == "connect", ) result = await run_sync_owned(operate) if action in {"connect", "disconnect"}: self._codex_update.complete() - if action == "status": - result["update"] = self._codex_update.snapshot() return result except ValueError, UnicodeError: raise InvalidRequestError( diff --git a/tests/api/test_admin.py b/tests/api/test_admin.py index 93e7ab9c0c..a39333f591 100644 --- a/tests/api/test_admin.py +++ b/tests/api/test_admin.py @@ -1,4 +1,3 @@ -import asyncio import mimetypes from dataclasses import replace from pathlib import Path @@ -2163,7 +2162,10 @@ def test_admin_apply_preserves_false_and_numeric_zero(monkeypatch, tmp_path): assert "HTTP_WRITE_TIMEOUT=0" in managed -def test_admin_local_provider_status_reports_reachable(monkeypatch, tmp_path): +@pytest.mark.parametrize("provider_id", ["lmstudio", "llamacpp", "ollama"]) +def test_admin_local_provider_status_reports_reachable( + monkeypatch, tmp_path, provider_id +): _set_home(monkeypatch, tmp_path) _clear_process_config(monkeypatch) app = create_test_app() @@ -2182,24 +2184,24 @@ async def get(self, url: str): return httpx.Response(200, json={"data": []}) with patch("free_claude_code.api.admin_routes.httpx.AsyncClient", FakeAsyncClient): - response = _local_client(app).get("/admin/api/providers/local-status") + response = _local_client(app).get( + f"/admin/api/providers/{provider_id}/local-status" + ) assert response.status_code == 200 - providers = response.json()["providers"] - assert {provider["status"] for provider in providers} == {"reachable"} + assert response.json()["provider_id"] == provider_id + assert response.json()["status"] == "reachable" -def test_admin_local_provider_status_checks_all_providers_concurrently( +def test_admin_local_provider_status_checks_only_requested_provider( monkeypatch, tmp_path ): _set_home(monkeypatch, tmp_path) _clear_process_config(monkeypatch) app = create_test_app() calls = 0 - active = 0 - max_active = 0 - class SlowAsyncClient: + class CountingAsyncClient: def __init__(self, *args, **kwargs): pass @@ -2210,20 +2212,17 @@ async def __aexit__(self, *args): return None async def get(self, url: str): - nonlocal active, calls, max_active + nonlocal calls calls += 1 - active += 1 - max_active = max(max_active, active) - await asyncio.sleep(0.01) - active -= 1 return httpx.Response(200, json={"data": []}) - with patch("free_claude_code.api.admin_routes.httpx.AsyncClient", SlowAsyncClient): - response = _local_client(app).get("/admin/api/providers/local-status") + with patch( + "free_claude_code.api.admin_routes.httpx.AsyncClient", CountingAsyncClient + ): + response = _local_client(app).get("/admin/api/providers/lmstudio/local-status") assert response.status_code == 200 - assert calls == 3 - assert max_active == 3 + assert calls == 1 def test_admin_config_exposes_structured_provider_configuration_targets( @@ -2271,18 +2270,17 @@ async def get(self, url: str): "free_claude_code.api.admin_routes.httpx.AsyncClient", return_value=FailingAsyncClient(), ): - response = _local_client(app).get("/admin/api/providers/local-status") + response = _local_client(app).get("/admin/api/providers/lmstudio/local-status") assert response.status_code == 200 - providers = response.json()["providers"] - assert {provider["status"] for provider in providers} == {"offline"} - for provider in providers: - assert provider["message"] == ( - "Could not connect. Verify the URL and that the local provider is running." - ) - assert "CREDENTIAL[unrecognized-format-987654321]" not in provider["message"] - assert "RuntimeError" not in provider["message"] - assert "error_type" not in provider + provider = response.json() + assert provider["status"] == "offline" + assert provider["message"] == ( + "Could not connect. Verify the URL and that the local provider is running." + ) + assert "CREDENTIAL[unrecognized-format-987654321]" not in provider["message"] + assert "RuntimeError" not in provider["message"] + assert "error_type" not in provider @pytest.mark.parametrize( diff --git a/tests/api/test_admin_configuration_io.py b/tests/api/test_admin_configuration_io.py index 52016cc754..a9043f10f1 100644 --- a/tests/api/test_admin_configuration_io.py +++ b/tests/api/test_admin_configuration_io.py @@ -12,7 +12,7 @@ @pytest.mark.asyncio @pytest.mark.parametrize( - "path", ["config", "status", "providers/local-status", "config/apply"] + "path", ["config", "status", "providers/lmstudio/local-status", "config/apply"] ) async def test_admin_storage_wait_allows_health_and_authenticated_requests(path): app = create_test_app() diff --git a/tests/api/test_integration_status_independence.py b/tests/api/test_integration_status_independence.py new file mode 100644 index 0000000000..2cb1b5285b --- /dev/null +++ b/tests/api/test_integration_status_independence.py @@ -0,0 +1,53 @@ +import asyncio +import threading + +import httpx +import pytest + +from free_claude_code.harnesses import vscode_chat_integration as vscode +from tests.api.support import create_test_app, runtime_for_app + + +@pytest.mark.asyncio +async def test_integration_endpoints_finish_while_one_worker_is_held(monkeypatch): + app = create_test_app() + runtime = runtime_for_app(app) + entered, release = asyncio.Event(), threading.Event() + loop = asyncio.get_running_loop() + + def held(path): + loop.call_soon_threadsafe(entered.set) + assert release.wait(5) + return {"connected": False, "paths": {}} + + monkeypatch.setattr(vscode, "status", held) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app, client=("127.0.0.1", 50000)), + base_url="http://127.0.0.1", + ) as client: + task = asyncio.create_task(client.get("/admin/api/integrations/vscode-chat")) + try: + await asyncio.wait_for(entered.wait(), 5) + responses = await asyncio.wait_for( + asyncio.gather( + *( + client.get(f"/admin/api/integrations/{integration}") + for integration in [ + "claude-vscode", + "claude-desktop", + "codex", + "jetbrains-acp", + ] + ) + ), + 1, + ) + assert all( + r.status_code == 200 and r.json()["connected"] is False + for r in responses + ) + assert not task.done() + finally: + release.set() + await asyncio.gather(task, return_exceptions=True) + await runtime.close() diff --git a/tests/api/test_local_provider_status.py b/tests/api/test_local_provider_status.py new file mode 100644 index 0000000000..68304329f9 --- /dev/null +++ b/tests/api/test_local_provider_status.py @@ -0,0 +1,60 @@ +import asyncio +from unittest.mock import AsyncMock, patch + +import httpx +import pytest + +from tests.api.support import create_test_app + + +@pytest.mark.asyncio +async def test_local_provider_results_complete_independently(): + app = create_test_app() + entered, release = asyncio.Event(), asyncio.Event() + + async def check(provider_id, url, path): + if provider_id == "ollama": + entered.set() + await release.wait() + return {"provider_id": provider_id, "status": "reachable"} + + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app, client=("127.0.0.1", 50000)), + base_url="http://127.0.0.1", + ) as client: + with patch( + "free_claude_code.api.admin_routes._check_local_provider", side_effect=check + ): + slow = asyncio.create_task( + client.get("/admin/api/providers/ollama/local-status") + ) + try: + await asyncio.wait_for(entered.wait(), 1) + fast = await asyncio.wait_for( + client.get("/admin/api/providers/lmstudio/local-status"), 1 + ) + assert fast.status_code == 200 + assert fast.json()["provider_id"] == "lmstudio" + assert not slow.done() + finally: + release.set() + await asyncio.gather(slow, return_exceptions=True) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("provider_id", ["openai", "unknown"]) +async def test_nonlocal_provider_status_is_rejected(provider_id): + app = create_test_app() + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app, client=("127.0.0.1", 50000)), + base_url="http://127.0.0.1", + ) as client: + with patch( + "free_claude_code.api.admin_routes._check_local_provider", + new_callable=AsyncMock, + ) as check: + response = await client.get( + f"/admin/api/providers/{provider_id}/local-status" + ) + assert response.status_code == 404 + check.assert_not_awaited() diff --git a/tests/core/test_async_rwlock.py b/tests/core/test_async_rwlock.py new file mode 100644 index 0000000000..011916cfd0 --- /dev/null +++ b/tests/core/test_async_rwlock.py @@ -0,0 +1,133 @@ +import asyncio + +import pytest + +from free_claude_code.core.async_rwlock import AsyncReadWriteLock + + +@pytest.mark.asyncio +async def test_readers_overlap_and_writer_waits(): + lock = AsyncReadWriteLock() + entered, release = asyncio.Event(), asyncio.Event() + + async def reader(): + async with lock.read(): + entered.set() + await release.wait() + + task = asyncio.create_task(reader()) + try: + await entered.wait() + async with lock.read(): + writer_entered = asyncio.Event() + + async def writer(): + async with lock.write(): + writer_entered.set() + + writer_task = asyncio.create_task(writer()) + await asyncio.sleep(0) + assert not writer_entered.is_set() + assert not writer_entered.is_set() + release.set() + await asyncio.wait_for(asyncio.gather(task, writer_task), 5) + assert writer_entered.is_set() + finally: + release.set() + await task + + +@pytest.mark.asyncio +@pytest.mark.parametrize("cancel_writer", [False, True]) +async def test_queued_writer_precedes_later_reader(cancel_writer): + lock = AsyncReadWriteLock() + queued, entered, release = asyncio.Event(), asyncio.Event(), asyncio.Event() + order = [] + + async def writer(): + queued.set() + async with lock.write(): + order.append("writer") + entered.set() + await release.wait() + + async def reader(): + async with lock.read(): + order.append("reader") + + async with lock.read(): + writer_task = asyncio.create_task(writer()) + await queued.wait() + reader_task = asyncio.create_task(reader()) + await asyncio.sleep(0) + assert order == [] + if cancel_writer: + writer_task.cancel() + with pytest.raises(asyncio.CancelledError): + await writer_task + await asyncio.wait_for(reader_task, 5) + assert order == ["reader"] + if not cancel_writer: + await asyncio.wait_for(entered.wait(), 5) + assert order == ["writer"] + release.set() + await asyncio.wait_for(asyncio.gather(writer_task, reader_task), 5) + assert order == ["writer", "reader"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("mode", ["read", "write"]) +async def test_exception_releases_access(mode): + lock = AsyncReadWriteLock() + with pytest.raises(ValueError): + async with getattr(lock, mode)(): + raise ValueError("failed operation") + async with lock.write(): + pass + + +@pytest.mark.asyncio +@pytest.mark.parametrize("mode", ["read", "write"]) +async def test_cancelled_queued_access_does_not_strand_waiters(mode): + lock = AsyncReadWriteLock() + queued = asyncio.Event() + + async def operation(): + queued.set() + async with getattr(lock, mode)(): + pytest.fail("Cancelled waiter entered") + + async with lock.write(): + task = asyncio.create_task(operation()) + await queued.wait() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + async with lock.write(): + pass + + +@pytest.mark.asyncio +@pytest.mark.parametrize("mode", ["read", "write"]) +async def test_repeated_cancellation_drains_contended_release(mode): + lock = AsyncReadWriteLock() + entered, finish = asyncio.Event(), asyncio.Event() + + async def operation(): + async with getattr(lock, mode)(): + entered.set() + await finish.wait() + + task = asyncio.create_task(operation()) + await entered.wait() + # Hold admission bookkeeping so releasing access must await cleanup. + async with lock._condition: + task.cancel() + await asyncio.sleep(0) + task.cancel() + await asyncio.sleep(0) + assert not task.done() + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(task, 5) + async with lock.write(): + pass diff --git a/tests/harnesses/test_native_config_json.py b/tests/harnesses/test_native_config_json.py new file mode 100644 index 0000000000..1c2604b5e8 --- /dev/null +++ b/tests/harnesses/test_native_config_json.py @@ -0,0 +1,75 @@ +import json + +import pytest + +from free_claude_code.harnesses import ( + claude_desktop_integration as desktop, +) +from free_claude_code.harnesses import ( + claude_integration as claude, +) +from free_claude_code.harnesses import ( + jetbrains_acp_integration as jetbrains, +) +from free_claude_code.harnesses import ( + vscode_chat_integration as vscode, +) + + +@pytest.fixture(params=["claude", "vscode", "desktop", "jetbrains"]) +def native_reader(request): + readers = { + "claude": claude._read_object, + "vscode": lambda path: vscode._read(path)[0], + "desktop": desktop._read, + "jetbrains": jetbrains._read, + } + return readers[request.param], request.param == "vscode" + + +def source_for(source, array): + return f"[{source}]" if array else source + + +def test_standard_json_does_not_use_json5(native_reader, tmp_path, monkeypatch): + read, array = native_reader + document = {"nested": {"value": 1}, "models": [{"id": str(i)} for i in range(999)]} + expected = [document] if array else document + path = tmp_path / "native.json" + path.write_text("\ufeff" + json.dumps(expected), encoding="utf-8") + + def slow_decoder(*args, **kwargs): + raise AssertionError("Standard JSON must not enter the JSON5 parser") + + monkeypatch.setattr("json5.loads", slow_decoder) + assert read(path) == expected + + +def test_json5_native_syntax_still_works(native_reader, tmp_path): + read, array = native_reader + path = tmp_path / "native.json" + path.write_text(source_for("{/* comment */ nested: {'value': 1,},}", array)) + expected = {"nested": {"value": 1}} + assert read(path) == ([expected] if array else expected) + + +@pytest.mark.parametrize( + "source", + [ + '{"x": 1, "x": 2}', + '{"x": 1, "\\u0078": 2}', + '{"nested": {"x": 1, "x": 2}}', + "{nested: {x: 1, x: 2,},}", + '{"x": NaN}', + '{"x": Infinity}', + '{"x": -Infinity}', + '{"x": 1e999}', + "{x: +Infinity,}", + ], +) +def test_invalid_native_values_remain_errors(native_reader, tmp_path, source): + read, array = native_reader + path = tmp_path / "native.json" + path.write_text(source_for(source, array)) + with pytest.raises(ValueError): + read(path) diff --git a/tests/runtime/test_integration_status_concurrency.py b/tests/runtime/test_integration_status_concurrency.py new file mode 100644 index 0000000000..583f6a6f03 --- /dev/null +++ b/tests/runtime/test_integration_status_concurrency.py @@ -0,0 +1,283 @@ +import asyncio +import threading +from unittest.mock import AsyncMock + +import pytest + +from free_claude_code.application.errors import ApplicationUnavailableError +from free_claude_code.harnesses import claude_integration as claude +from free_claude_code.harnesses import vscode_chat_integration as vscode +from tests.runtime.test_integration_startup import runtime as runtime + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "other", + [ + "claude_vscode_status", + "codex_integration_status", + "claude_desktop_status", + "jetbrains_acp_status", + ], +) +async def test_slow_status_does_not_block_other_integration( + runtime, monkeypatch, other +): + entered, release = asyncio.Event(), threading.Event() + loop = asyncio.get_running_loop() + + def held(path): + loop.call_soon_threadsafe(entered.set) + assert release.wait(5) + return {"connected": False, "paths": {}} + + monkeypatch.setattr(vscode, "status", held) + task = asyncio.create_task(runtime.vscode_chat_status()) + try: + await asyncio.wait_for(entered.wait(), 5) + result = await asyncio.wait_for(getattr(runtime, other)(), 1) + assert result["connected"] is False + assert not task.done() + finally: + release.set() + await asyncio.gather(task, return_exceptions=True) + await runtime.close() + + +@pytest.mark.asyncio +async def test_cancelled_reader_drains_before_writer_enters(runtime, monkeypatch): + entered, writer_entered, release = ( + asyncio.Event(), + asyncio.Event(), + threading.Event(), + ) + loop = asyncio.get_running_loop() + status, configure = vscode.status, vscode.configure + + def held(path): + if not entered.is_set(): + loop.call_soon_threadsafe(entered.set) + assert release.wait(5) + return status(path) + + def record_write(*args, **kwargs): + loop.call_soon_threadsafe(writer_entered.set) + return configure(*args, **kwargs) + + monkeypatch.setattr(vscode, "status", held) + monkeypatch.setattr(vscode, "configure", record_write) + reader = asyncio.create_task(runtime.vscode_chat_status()) + writer = None + try: + await asyncio.wait_for(entered.wait(), 5) + reader.cancel() + await asyncio.sleep(0) + reader.cancel() + writer = asyncio.create_task(runtime.connect_vscode_chat()) + await runtime.provider_manager.wait_for_catalog() + await asyncio.sleep(0) + assert not reader.done() + assert not writer_entered.is_set() + assert (await asyncio.wait_for(runtime.jetbrains_acp_status(), 1))[ + "connected" + ] is False + release.set() + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(reader, 5) + assert (await asyncio.wait_for(writer, 5))["connected"] is True + assert writer_entered.is_set() + finally: + release.set() + await asyncio.gather( + reader, *([writer] if writer else []), return_exceptions=True + ) + await runtime.close() + + +@pytest.mark.asyncio +async def test_status_cannot_observe_partial_multifile_write(runtime, monkeypatch): + entered, release = asyncio.Event(), threading.Event() + loop = asyncio.get_running_loop() + atomic = claude.atomic_write_text + + def held(path, content): + atomic(path, content) + if path == claude.claude_state_path(): + loop.call_soon_threadsafe(entered.set) + assert release.wait(5) + + monkeypatch.setattr(claude, "atomic_write_text", held) + writer = asyncio.create_task(runtime.connect_claude_vscode()) + reader = None + try: + await asyncio.wait_for(entered.wait(), 5) + assert claude.claude_state_path().exists() + assert not claude.settings_path().exists() + reader = asyncio.create_task(runtime.claude_vscode_status()) + assert (await asyncio.wait_for(runtime.jetbrains_acp_status(), 1))[ + "connected" + ] is False + assert not reader.done() + release.set() + assert (await asyncio.wait_for(writer, 5))["connected"] is True + assert (await asyncio.wait_for(reader, 5))["connected"] is True + finally: + release.set() + await asyncio.gather( + writer, *([reader] if reader else []), return_exceptions=True + ) + await runtime.close() + + +@pytest.mark.asyncio +async def test_continuously_changed_generation_has_bounded_retry(runtime, monkeypatch): + entered, release = asyncio.Queue(), threading.Semaphore(0) + loop = asyncio.get_running_loop() + + def held(path, state_path, url, token, connected=None): + loop.call_soon_threadsafe(entered.put_nowait, token) + assert release.acquire(timeout=5) + return {"connected": False, "paths": {}} + + monkeypatch.setattr(claude, "configure", held) + task = asyncio.create_task(runtime.claude_vscode_status()) + try: + for attempt in range(2): + await asyncio.wait_for(entered.get(), 5) + await runtime.provider_manager.replace( + runtime.settings.model_copy( + update={"proxy_auth_token": f"rotated-{attempt}"} + ), + commit=AsyncMock(), + ) + release.release() + with pytest.raises(ApplicationUnavailableError, match="configuration changed"): + await asyncio.wait_for(task, 5) + assert entered.empty() + finally: + release.release() + await asyncio.gather(task, return_exceptions=True) + await runtime.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("change", ["restart", "shutdown"]) +async def test_availability_is_rechecked_after_status_worker( + runtime, monkeypatch, change +): + entered, release = asyncio.Event(), threading.Event() + loop = asyncio.get_running_loop() + + def held(path): + loop.call_soon_threadsafe(entered.set) + assert release.wait(5) + return {"connected": False, "paths": {}} + + monkeypatch.setattr(vscode, "status", held) + task = asyncio.create_task(runtime.vscode_chat_status()) + try: + await asyncio.wait_for(entered.wait(), 5) + if change == "shutdown": + await asyncio.wait_for(runtime.close(), 1) + else: + runtime._pending_fields = ["PORT"] + release.set() + with pytest.raises(ApplicationUnavailableError): + await asyncio.wait_for(task, 5) + finally: + release.set() + await asyncio.gather(task, return_exceptions=True) + await runtime.close() + + +@pytest.mark.asyncio +async def test_startup_finishing_during_skipped_status_repeats_inspection( + runtime, monkeypatch +): + update = runtime._integrations._claude_update + update.state = "starting" + loop = asyncio.get_running_loop() + path = claude.settings_path() + reads = 0 + + def resolve_path(): + nonlocal reads + reads += 1 + if reads == 1: + loop.call_soon_threadsafe(update.complete) + return path + + monkeypatch.setattr(claude, "settings_path", resolve_path) + monkeypatch.setattr( + claude, "configure", lambda *args: {"connected": True, "paths": {}} + ) + try: + result = await runtime.claude_vscode_status() + assert result["connected"] is True + assert result["update"]["state"] == "ready" + assert reads == 2 + finally: + await runtime.close() + + +@pytest.mark.asyncio +async def test_same_integration_status_readers_overlap(runtime, monkeypatch): + both_entered, release = asyncio.Event(), threading.Event() + loop = asyncio.get_running_loop() + count = 0 + count_lock = threading.Lock() + + def held(path): + nonlocal count + with count_lock: + count += 1 + if count == 2: + loop.call_soon_threadsafe(both_entered.set) + assert release.wait(5) + return {"connected": False, "paths": {}} + + monkeypatch.setattr(vscode, "status", held) + tasks = [asyncio.create_task(runtime.vscode_chat_status()) for _ in range(2)] + try: + await asyncio.wait_for(both_entered.wait(), 1) + assert all(not task.done() for task in tasks) + finally: + release.set() + await asyncio.gather(*tasks, return_exceptions=True) + await runtime.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("first_failure", [False, True]) +async def test_generation_change_discards_old_status_outcome( + runtime, monkeypatch, first_failure +): + entered, release = asyncio.Event(), threading.Event() + loop = asyncio.get_running_loop() + tokens = [] + initial_token = runtime.settings.proxy_auth_token + + def held(path, state_path, url, token, connected=None): + tokens.append(token) + if len(tokens) == 1: + loop.call_soon_threadsafe(entered.set) + assert release.wait(5) + if first_failure: + raise ValueError("Old configuration read failed") + return {"connected": token == "rotated", "paths": {}} + + monkeypatch.setattr(claude, "configure", held) + task = asyncio.create_task(runtime.claude_vscode_status()) + try: + await asyncio.wait_for(entered.wait(), 5) + await runtime.provider_manager.replace( + runtime.settings.model_copy(update={"proxy_auth_token": "rotated"}), + commit=AsyncMock(), + ) + release.set() + assert (await asyncio.wait_for(task, 5))["connected"] is True + assert tokens == [initial_token, "rotated"] + finally: + release.set() + await asyncio.gather(task, return_exceptions=True) + await runtime.close() From 7195cefd5d0c197a9809e851a65b4a940918cee8 Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:52:30 -0700 Subject: [PATCH 2/2] patch: Format development versions for ChatGPT model discovery (#1956) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why ChatGPT model discovery rejects Python development versions such as `6.5.9.dev3+gb89b44b2a` with HTTP 400. This leaves a connected account unable to load its models when FCC runs from a development checkout. ## How Convert the model-list `client_version` parameter to SemVer using the existing `packaging.Version` parser. Preserve the development counter and local build metadata, producing values such as `6.5.9-dev.3+gb89b44b2a`, and give source-only installs a valid development version.

RetriggerConfidence Score: 4/5

The inaccurate version parameter is a non-blocking concern; the observed model request still succeeded with the mocked response.

Fix All in CodexFindings

1. P2 **Release candidate is omitted** ▶
Summary The PR converts package versions for the Codex model-list request and adds version-formatting tests. For release-candidate builds, the conversion omits the release-candidate identifier, so the request reports a different version.
Reviews (1) · Last reviewed commit: ["patch: Format development versions for C..."](https://github.com/alishahryar1/free-claude-code/commit/b4a6b0886b7585ab3c1f4b5cb369ca9c3c7721c4) --- .../providers/openai_codex/provider.py | 18 +++++++- tests/providers/test_openai_codex_provider.py | 46 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/free_claude_code/providers/openai_codex/provider.py b/src/free_claude_code/providers/openai_codex/provider.py index 24dedca6d1..f99f6a4395 100644 --- a/src/free_claude_code/providers/openai_codex/provider.py +++ b/src/free_claude_code/providers/openai_codex/provider.py @@ -9,6 +9,7 @@ import httpx2 from openai import AsyncOpenAI +from packaging.version import Version from free_claude_code.application.model_metadata import ProviderModelInfo from free_claude_code.core.anthropic.models import MessagesRequest @@ -126,7 +127,9 @@ async def _list_models_payload(self) -> Any: payload = await client.get( "models", cast_to=object, - options={"params": {"client_version": FCC_VERSION}}, + options={ + "params": {"client_version": _model_list_client_version()} + }, ) await attempt.accept() execution.succeed() @@ -199,6 +202,19 @@ def stream_responses( ) +def _model_list_client_version() -> str: + """Format FCC's release and VCS development versions for the catalog API.""" + if FCC_VERSION == "dev": + return "0.0.0-dev" + parsed = Version(FCC_VERSION) + formatted = f"{parsed.major}.{parsed.minor}.{parsed.micro}" + if parsed.dev is not None: + formatted += f"-dev.{parsed.dev}" + if parsed.local is not None: + formatted += f"+{parsed.local}" + return formatted + + async def _endpoint_required() -> str: raise RuntimeError( "Codex requests require request-scoped subscription credentials." diff --git a/tests/providers/test_openai_codex_provider.py b/tests/providers/test_openai_codex_provider.py index 9dfdddb79d..e361972fda 100644 --- a/tests/providers/test_openai_codex_provider.py +++ b/tests/providers/test_openai_codex_provider.py @@ -23,6 +23,7 @@ from free_claude_code.core.reasoning import ReasoningEffort, ReasoningPolicy from free_claude_code.providers.admission import ProviderAdmissionController from free_claude_code.providers.base import ProviderConfig +from free_claude_code.providers.openai_codex import provider as codex_provider_module from free_claude_code.providers.openai_codex.auth import ( OpenAIAccess, OpenAIAuthManager, @@ -256,6 +257,51 @@ def handler(request: httpx2.Request) -> httpx2.Response: assert auth.recovery_calls == 0 +@pytest.mark.asyncio +@pytest.mark.parametrize( + "package_version,client_version", + [ + ("6.5.9", "6.5.9"), + ("6.5.9.dev0", "6.5.9-dev.0"), + ("6.5.9.dev3+gb89b44b2a", "6.5.9-dev.3+gb89b44b2a"), + ("6.5.9.dev3+gb89b44b2a.d20260930", "6.5.9-dev.3+gb89b44b2a.d20260930"), + ("6.5.9+d20260930", "6.5.9+d20260930"), + ("dev", "0.0.0-dev"), + ], +) +async def test_model_discovery_formats_package_version_for_upstream( + monkeypatch: pytest.MonkeyPatch, package_version: str, client_version: str +) -> None: + monkeypatch.setattr(codex_provider_module, "FCC_VERSION", package_version) + requests: list[httpx2.Request] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + requests.append(request) + if request.url.params.get("client_version") != client_version: + return httpx2.Response( + 400, json={"detail": "Invalid client_version format"} + ) + return httpx2.Response( + 200, json={"models": [{"slug": "gpt-visible", "visibility": "list"}]} + ) + + provider = OpenAICodexProvider( + _config(), + auth=_FakeAuth(), + admission=immediate_admission(max_attempts=1), + transport=httpx2.MockTransport(handler), + ) + try: + infos = await provider.list_model_infos() + finally: + await provider.cleanup() + + assert {info.model_id for info in infos} == {"gpt-visible"} + assert len(requests) == 1 + assert requests[0].headers["version"] == package_version + assert requests[0].headers["user-agent"] == f"codex_cli_rs/{package_version}" + + @pytest.mark.asyncio async def test_provider_uses_subscription_headers_and_visible_model_catalog() -> None: requests: list[httpx2.Request] = []