From 4f6e56bd726b5e5928a7cebf0a97664ea2285b87 Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:00:06 -0700 Subject: [PATCH 1/6] Collapse doctor output in bug reports (#1911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Large fcc-doctor reports make bug reports difficult to scan. ## How Automatically wrap the doctor section in a collapsed, JSON-highlighted block when a bug report is opened or edited. Preserve the report and surrounding issue text, avoid wrapping it again, and let the version validator read both original and collapsed reports.

RetriggerConfidence Score: 4/5

Not safe to merge until formatting preserves intervening author edits.

Fix All in CodexFindings

1. P1 **Concurrent edits can be lost** ▶
Summary The PR collapses valid doctor output into a JSON-highlighted block and adds parsing and formatting tests. Its new full-body update can erase an author's edit made after the workflow fetches the issue. This should be fixed before merging.
Reviews (1) · Last reviewed commit: ["Collapse doctor output in submitted bug ..."](https://github.com/alishahryar1/free-claude-code/commit/0044dc8779ce80302cfeac1402a460ef0f81673e) --- .../workflows/validate-bug-report-version.yml | 22 +++- .../test_issue_form_version_validation.py | 101 +++++++++++++++++- 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/.github/workflows/validate-bug-report-version.yml b/.github/workflows/validate-bug-report-version.yml index 7d1c6641b0..97001ae0f0 100644 --- a/.github/workflows/validate-bug-report-version.yml +++ b/.github/workflows/validate-bug-report-version.yml @@ -37,6 +37,8 @@ jobs: if (doctor.length) { if (doctor.length !== 1) return { valid: false }; let text = doctor[0].replace(/^FCC doctor output\r?\n/, "").trim(); + const details = text.match(/^
\s*[^<>\r\n]*<\/summary>\s*([\s\S]*?)\s*<\/details>$/); + if (details) text = details[1].trim(); const fence = text.match(/^```(?:json)?\r?\n([\s\S]*)\r?\n```$/); if (fence) text = fence[1]; let report; @@ -58,7 +60,25 @@ jobs: const reportedVersion = versionMatches.length === 1 ? versionMatches[0][1] : undefined; return { valid: reportedVersion !== undefined || fieldValue === "None", reportedVersion }; }; - const { reportedVersion, valid } = parseReport(issue.body || ""); + const collapseDoctorReport = (body) => { + const sections = body.split(/(?=^### )/m); + const index = sections.findIndex((section) => /^### FCC doctor output\r?\n/.test(section)); + if (index === -1 || !parseReport(body).valid) return body; + const section = sections[index]; + const text = section.replace(/^### FCC doctor output\r?\n/, "").trim(); + if (text.startsWith("
")) return body; + const newline = section.includes("\r\n") ? "\r\n" : "\n"; + const content = text.startsWith("```") ? text : `\`\`\`json${newline}${text}${newline}\`\`\``; + const wrapped = ["
", "FCC doctor output", "", content, "", "
"].join(newline); + const start = section.indexOf(text); + sections[index] = section.slice(0, start) + wrapped + section.slice(start + text.length); + return sections.join(""); + }; + const formattedBody = collapseDoctorReport(issue.body || ""); + if (formattedBody !== (issue.body || "")) { + await github.rest.issues.update({ ...repo, issue_number, body: formattedBody }); + } + const { reportedVersion, valid } = parseReport(formattedBody); const labelName = "needs-fcc-version"; const invalidMarker = ""; const outdatedMarker = ""; diff --git a/tests/contracts/test_issue_form_version_validation.py b/tests/contracts/test_issue_form_version_validation.py index d0160e8de2..db49451372 100644 --- a/tests/contracts/test_issue_form_version_validation.py +++ b/tests/contracts/test_issue_form_version_validation.py @@ -76,6 +76,21 @@ def test_bug_forms_require_description_and_post_install_diagnostics() -> None: "body, valid, version", [ ('### FCC doctor output\n\n```json\n{"version":"1.2.3"}\n```', True, "1.2.3"), + ( + '### FCC doctor output\n\n
\nFCC doctor output\n\n```json\n{"version":"1.2.3"}\n```\n\n
', + True, + "1.2.3", + ), + ( + '### FCC doctor output\r\n\r\n
\r\nDiagnostics\r\n\r\n```json\r\n{"version":"1.2.3"}\r\n```\r\n\r\n
', + True, + "1.2.3", + ), + ( + "### FCC doctor output\n\n
\nDiagnostics\n\n```json\n{invalid}\n```\n\n
", + False, + None, + ), ( '### FCC doctor output\r\n\r\n```json\r\n{"version":"1.2.3"}\r\n```', True, @@ -101,6 +116,12 @@ def test_bug_forms_require_description_and_post_install_diagnostics() -> None: ], ) def test_report_parser_uses_the_correct_form_section(body, valid, version): + result = _parse_report(body) + assert result["valid"] is valid + assert result.get("reportedVersion") == version + + +def _parse_report(body): patterns = "\n".join( f"const {name} = {json.dumps(_workflow_pattern(name))};" for name in ("fieldPattern", "versionPattern") @@ -109,9 +130,71 @@ def test_report_parser_uses_the_correct_form_section(body, valid, version): script += ( f"\nprocess.stdout.write(JSON.stringify(parseReport({json.dumps(body)})));" ) - result = _run_javascript(script) - assert result["valid"] is valid - assert result.get("reportedVersion") == version + return _run_javascript(script) + + +def test_plain_doctor_report_can_be_collapsed_without_changing_its_data( + monkeypatch, capsys +): + from free_claude_code.cli import doctor + + copied = [] + report = {"version": "1.2.3", "model_routing": {"model": "provider/```
"}} + monkeypatch.setattr(doctor, "collect_report", lambda: report) + monkeypatch.setattr(doctor, "copy_text", copied.append) + doctor.main([]) + assert json.loads(capsys.readouterr().out) == report + assert json.loads(copied[0]) == report + body = "### FCC doctor output\n\n" + copied[0] + collapsed = _collapse_report(body) + assert collapsed.startswith("### FCC doctor output\n\n
\n") + assert json.loads(collapsed.split("```json\n", 1)[1].rsplit("```", 1)[0]) == report + assert _parse_report(collapsed) == {"valid": True, "reportedVersion": "1.2.3"} + + +def _collapse_report(body): + patterns = "\n".join( + f"const {name} = {json.dumps(_workflow_pattern(name))};" + for name in ("fieldPattern", "versionPattern") + ) + script = ( + patterns + + "\n" + + _javascript_function("parseReport") + + "\n" + + _javascript_function("collapseDoctorReport") + ) + script += f"\nprocess.stdout.write(JSON.stringify(collapseDoctorReport({json.dumps(body)})));" + return _run_javascript(script) + + +@pytest.mark.parametrize("newline", ["\n", "\r\n"]) +@pytest.mark.parametrize("fenced", [False, True]) +def test_collapsing_only_changes_doctor_section_and_is_idempotent(newline, fenced): + report = '{"version":"1.2.3","model":"$&```
"}' + content = f"```json\n{report}\n```" if fenced else report + before = "### Describe the issue\n\nMy description and ![image](https://example.com/image.png)\n\n### FCC doctor output\n\n" + after = "\n\n### Other details\n\nKeep this text exactly.\n" + body = (before + content + after).replace("\n", newline) + collapsed = _collapse_report(body) + assert collapsed.startswith(before.replace("\n", newline) + "
") + assert collapsed.endswith(after.replace("\n", newline)) + assert report in collapsed + assert _collapse_report(collapsed) == collapsed + assert _parse_report(collapsed) == {"valid": True, "reportedVersion": "1.2.3"} + + +@pytest.mark.parametrize( + "body", + [ + "### Installation issue\n\nCannot install", + "### FCC version\n\n1.2.3", + "### FCC doctor output\n\n{invalid}", + '### FCC doctor output\n\n{"version":"1.2.3"}\n\n### FCC doctor output\n\n{"version":"1.2.3"}', + ], +) +def test_collapsing_leaves_other_or_invalid_reports_untouched(body): + assert _collapse_report(body) == body @pytest.mark.parametrize( @@ -248,6 +331,11 @@ def test_outdated_version_comment_is_reconciled_across_edits(form) -> None: record("getIssue", args); return { data: liveIssue }; }, + update: async (args) => { + record("updateIssue", args); + liveIssue.body = args.body; + return { data: liveIssue }; + }, getLabel: async (args) => record("getLabel", args), createLabel: async (args) => record("createLabel", args), addLabels: async (args) => { @@ -336,6 +424,7 @@ def test_outdated_version_comment_is_reconciled_across_edits(form) -> None: assert names.count("deleteComment") == 3 assert names.count("getLatestRelease") == 4 assert names.count("getIssue") == 6 + assert names.count("updateIssue") == (3 if form == "doctor" else 0) assert names.count("getLabel") == 1 assert names.count("addLabels") == 1 assert names.count("removeLabel") == 2 @@ -354,7 +443,11 @@ def test_outdated_version_comment_is_reconciled_across_edits(form) -> None: ) assert result["comments"] == [] assert "cancel-in-progress: false" in workflow - assert "github.rest.issues.update({" not in workflow + assert all( + set(call["args"]) == {"owner", "repo", "issue_number", "body"} + for call in calls + if call["name"] == "updateIssue" + ) assert 'state: "closed"' not in workflow From d59300974083ec8bcc66dcd09d2b097c4f1070a0 Mon Sep 17 00:00:00 2001 From: Tanishq-1 <49880456+Tanishq-1@users.noreply.github.com> Date: Sun, 27 Sep 2026 04:17:47 +0530 Subject: [PATCH 2/6] patch: Fix fcc-cline reconnecting to a stale Cline Hub (#1590) (#1641) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1590 ## Problem `fcc-cline` runs Cline inside an ephemeral configuration tree pointed at the FCC proxy (`localhost:8082`). However, interactive Cline passes `backendMode: "auto"` to its core (verified in the shipped `cline.exe`), overriding the launcher's `CLINE_SESSION_BACKEND_MODE=local`. With `auto`, Cline may attach to — or start — a detached **Hub daemon** that: 1. resolves provider settings from the user's **global** store (`~/.cline/data/settings/providers.json`) instead of FCC's ephemeral providers file; 2. **outlives the launcher** (it is not killed when the terminal closes); 3. is auto-discovered by the *next* `fcc-cline` run via `~/.cline/data/locks/hub/production.json`, routing traffic to the wrong provider. This is exactly the behavior reported in #1590: > Run fcc-cline again → the server might already be running, but Cline might be > connecting to the wrong OpenAI provider. ## Fix — isolation over deletion `build_cline_launcher_env()` now redirects `CLINE_HUB_DISCOVERY_PATH` to `/hub/production.json` — a path that never exists and is never written: - The child can never **discover any pre-existing Hub** (FCC-created or user-created), so a stale daemon can no longer hijack the session. - If Cline spawns a Hub of its own, its discovery file is written **inside the ephemeral tree**, which the launcher deletes on exit — no Hub state survives the session. - A user-run `cline hub` (outside FCC) keeps its own discovery file at the default location and is untouched. Defense-in-depth notes: - `CLINE_HUB_DISCOVERY_PATH` is an internal Cline env var present in the shipped binary. The root cause is tracked upstream at cline/cline#13631, with a fix in cline/cline#13637 (owner-filed, unmerged at the time of writing). If that env var is ever renamed upstream, this guard degrades to today's behavior — **no new failure mode**. - Deliberately scoped to isolation: this change does not attempt to kill foreign Hub processes. ## Testing - 2 tests updated + 2 new in `tests/cli/test_cline_launcher.py`: - `test_cline_hub_discovery_is_isolated_from_user_hub_state` — a pre-existing user discovery file is overridden, never read; - `test_cline_hub_discovery_path_lives_in_ephemeral_temp_tree` — the redirected path sits inside the ephemeral tree and does not exist at spawn time. - Quality gates: `ruff format --check`, `ruff check`, `ty check` — all green. - Full suite: **4407 passed / 148 skipped / 0 failed** (Python 3.14, Windows). - e2e (Playwright): green except `e2e/test_chat_sessions.py::test_chat_context_meter_shows_used_over_advertised_context_window`, which fails on clean `main` as well (pre-existing, unrelated to this change). - Version bumped 5.18.11 → **5.18.12** (PATCH) with `uv lock` in the same commit. ## Version 5.18.12 — PATCH above base 5.18.11 (bug fix only, per CONTRIBUTING SemVer policy).

RetriggerConfidence Score: 5/5

No outstanding findings block merging.

Summary

The PR isolates each fcc-cline launch’s Hub discovery path in its temporary configuration tree. The current code creates the path’s parent directory before starting Cline, fixing the previously reported launch issue. Reviews (17) · Last reviewed commit: ["Merge branch 'main' into fix-cline-hub-h..."](https://github.com/alishahryar1/free-claude-code/commit/2df1fefa2002bfa6c2e54eb3a8360836e6a69b20) Co-authored-by: Claude Opus 4.7 Co-authored-by: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> --- src/free_claude_code/cli/launchers/cline.py | 20 ++++++++++- tests/cli/test_cline_launcher.py | 40 +++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/free_claude_code/cli/launchers/cline.py b/src/free_claude_code/cli/launchers/cline.py index 4520957f2f..664e0d88cb 100644 --- a/src/free_claude_code/cli/launchers/cline.py +++ b/src/free_claude_code/cli/launchers/cline.py @@ -1,4 +1,10 @@ -"""Installed Cline launcher with private native provider settings.""" +"""Installed Cline launcher with private native provider settings. + +Sessions run with an isolated Hub discovery path so a stale or user-run Cline +Hub can never capture an fcc-cline session: the child resolves Hub state only +inside FCC's ephemeral configuration directory, which is deleted on exit. A +Hub started with ordinary cline keeps using its own discovery file. +""" import re from collections.abc import Sequence @@ -29,6 +35,17 @@ def _configure( ) providers_path = files.write_json("settings/providers.json", config.providers) files.write_json("settings/models.json", config.models) + # Interactive Cline passes backendMode:"auto" to core, which overrides + # CLINE_SESSION_BACKEND_MODE and may attach to (or start) a detached Hub + # that resolves provider settings from the user's global store instead of + # FCC's ephemeral files. Redirecting Hub discovery into the ephemeral tree + # makes that attachment impossible without touching user-run Hub state. + # (Verified: shipped Cline 3.0.57 honors CLINE_HUB_DISCOVERY_PATH as the + # discovery-path override in its hub resolver.) + hub_discovery_path = files.directory / "hub" / "production.json" + # Pre-create the directory, not the file: the child records Hub discovery + # state here on startup, and the Hub itself must own the file. + hub_discovery_path.parent.mkdir(parents=True, mode=0o700, exist_ok=True) return PreparedLaunch( [ctx.binary_path, "--provider", CLINE_PROVIDER_ID, *args], client_environment( @@ -37,6 +54,7 @@ def _configure( updates={ "CLINE_PROVIDER_SETTINGS_PATH": str(providers_path), "CLINE_SESSION_BACKEND_MODE": "local", + "CLINE_HUB_DISCOVERY_PATH": str(hub_discovery_path), }, ), ) diff --git a/tests/cli/test_cline_launcher.py b/tests/cli/test_cline_launcher.py index 7d2a0c47f1..c7939a3edf 100644 --- a/tests/cli/test_cline_launcher.py +++ b/tests/cli/test_cline_launcher.py @@ -139,6 +139,7 @@ def test_cline_native_configuration_is_available_to_child(launch_capture) -> Non from tests.cli.test_launcher_workflow import launch launch_ids = [] + hub_paths = [] def inspect(command, env): providers_path = Path(env["CLINE_PROVIDER_SETTINGS_PATH"]) @@ -152,6 +153,16 @@ def inspect(command, env): assert settings["model"] in models["providers"][CLINE_PROVIDER_ID]["models"] assert env["CLINE_SESSION_BACKEND_MODE"] == "local" assert command[:3] == ["cline", "--provider", "openai-native"] + hub_path = Path(env["CLINE_HUB_DISCOVERY_PATH"]) + assert hub_path.parent.name == "hub" + assert hub_path.name == "production.json" + # Hub discovery must resolve inside the same ephemeral tree as the + # private provider files. The hub/ directory is pre-created so the + # child can record discovery state; the file itself must not exist. + assert hub_path.parent.parent == providers_path.parent.parent + assert hub_path.parent.is_dir() + assert not hub_path.exists() + hub_paths.append(hub_path) launch_capture.on_start = inspect args = ["--model", "native-selection", "--data-dir", "native-data-dir"] @@ -159,3 +170,32 @@ def inspect(command, env): assert launch_capture.commands[0][-len(args) :] == args launch("cline", []) assert launch_ids[0] != launch_ids[1] + assert hub_paths[0] != hub_paths[1] + assert all(not hub_path.exists() for hub_path in hub_paths) + + +def test_cline_hub_discovery_path_overrides_stale_inherited_value( + launch_capture, monkeypatch: pytest.MonkeyPatch +) -> None: + from tests.cli.test_launcher_workflow import launch + + monkeypatch.setenv("CLINE_HUB_DISCOVERY_PATH", "stale-user-hub-discovery.json") + dir_existed_at_start = [] + + def inspect(command, env): + hub_dir = Path(env["CLINE_HUB_DISCOVERY_PATH"]).parent + dir_existed_at_start.append(hub_dir.is_dir()) + + launch_capture.on_start = inspect + launch("cline", []) + hub_path = Path(launch_capture.environments[0]["CLINE_HUB_DISCOVERY_PATH"]) + # The stale user-level discovery file must be replaced by the isolated + # ephemeral path, never inherited by the fcc-cline child. The hub/ + # directory is pre-created at launch (checked via on_start, since the + # ephemeral tree is cleaned up when the launcher exits); only the file + # itself must not be pre-created. + assert hub_path.name == "production.json" + assert hub_path.parent.name == "hub" + assert str(hub_path) != "stale-user-hub-discovery.json" + assert dir_existed_at_start == [True] + assert not hub_path.exists() From 4df2ba3e191ce1ae3777d6c15257374891a972f2 Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Sat, 26 Sep 2026 16:40:39 -0700 Subject: [PATCH 3/6] Fix conversation headers in provider smoke tests (#1912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why OpenCode Go rejects provider smoke requests that lack a conversation ID. The settings-apply Playwright test also has a timing race because its wait accepts a Promise before request interception completes. ## How Give smoke requests an FCC smoke User-Agent and a conversation ID. Reuse the ID across turns, tool continuations, and disconnect follow-ups, and generate fresh IDs for independent conversations. Apply the headers to Messages and Responses probes while preserving caller-supplied headers and authentication. Make the settings-apply browser test wait for a boolean set by the route handler after it captures the request.

RetriggerConfidence Score: 5/5

No blocking findings were identified.
Summary The PR adds conversation headers to smoke-client requests, reuses session IDs for related requests, and adds header tests. The latest change updates how a Playwright test detects an intercepted configuration request.
Reviews (2) · Last reviewed commit: ["Wait for actual Apply request intercepti..."](https://github.com/alishahryar1/free-claude-code/commit/2db361c3d534d38f28071ccd42387a19fbc41e54) --- e2e/test_code_sessions.py | 10 +- smoke/lib/e2e.py | 6 +- smoke/lib/http.py | 15 ++- smoke/prereq/test_provider_prereq_live.py | 6 +- smoke/product/test_provider_product_live.py | 13 +- tests/contracts/test_smoke_child_process.py | 142 +++++++++++++++++++- 6 files changed, 175 insertions(+), 17 deletions(-) diff --git a/e2e/test_code_sessions.py b/e2e/test_code_sessions.py index 9c2d7e2de7..10d1825b83 100644 --- a/e2e/test_code_sessions.py +++ b/e2e/test_code_sessions.py @@ -597,14 +597,18 @@ def test_settings_apply_preserves_open_creation_and_picker( page, admin_base_url, tmp_path, code_control ): pending = [] - page.route("**/admin/api/config/apply", lambda route: pending.append(route)) - page.expose_function("applyRequestIntercepted", lambda: bool(pending)) + + def capture_apply(route): + pending.append(route) + page.evaluate("window.applyRequestIntercepted = true") + + page.route("**/admin/api/config/apply", capture_apply) page.goto(f"{admin_base_url}/admin") expect(page.locator("#messageArea")).to_have_text("") page.locator("#field-PORT").fill("8081") page.get_by_role("button", name="Apply", exact=True).click() expect(page.locator("#messageArea")).to_have_text("Applying…") - page.wait_for_function("window.applyRequestIntercepted()") + page.wait_for_function("window.applyRequestIntercepted === true") assert len(pending) == 1 page.get_by_role("button", name="Code sessions", exact=True).click() page.get_by_role("button", name="New code session", exact=True).click() diff --git a/smoke/lib/e2e.py b/smoke/lib/e2e.py index 9b731dc66e..92233cb1a4 100644 --- a/smoke/lib/e2e.py +++ b/smoke/lib/e2e.py @@ -32,6 +32,7 @@ from free_claude_code.messaging.workflow import MessagingWorkflow from smoke.lib.child_process import run_captured_text from smoke.lib.config import ProviderModel, SmokeConfig, auth_headers +from smoke.lib.http import conversation_headers from smoke.lib.server import RunningServer, start_server from smoke.lib.skips import fail_missing_env, skip_if_upstream_unavailable_events @@ -88,6 +89,7 @@ def __init__(self, server: RunningServer, config: SmokeConfig) -> None: self.config = config self.messages: list[dict[str, Any]] = [] self.turns: list[ConversationTurn] = [] + self._session_id = str(uuid.uuid4()) def ask( self, @@ -120,7 +122,7 @@ def stream( *, headers: dict[str, str] | None = None, ) -> ConversationTurn: - request_headers = headers or auth_headers() + request_headers = conversation_headers(headers, session_id=self._session_id) stream_payload = {**payload, "stream": True} with httpx.stream( "POST", @@ -148,7 +150,7 @@ def stream_expect_http_error( ) -> dict[str, Any]: response = httpx.post( f"{self.server.base_url}/v1/messages", - headers=auth_headers(), + headers=conversation_headers(session_id=self._session_id), json=payload, timeout=self.config.timeout_s, ) diff --git a/smoke/lib/http.py b/smoke/lib/http.py index 7eb1759a64..e9e7fd3d11 100644 --- a/smoke/lib/http.py +++ b/smoke/lib/http.py @@ -1,5 +1,6 @@ """HTTP helpers for live smoke requests.""" +import uuid from typing import Any import httpx @@ -10,6 +11,16 @@ from .server import RunningServer +def conversation_headers( + headers: dict[str, str] | None = None, *, session_id: str | None = None +) -> dict[str, str]: + """Identify a smoke conversation while preserving explicit caller headers.""" + result = httpx.Headers(auth_headers() if headers is None else headers) + result.setdefault("user-agent", "fcc-smoke/1.0") + result.setdefault("x-opencode-session", session_id or str(uuid.uuid4())) + return dict(result) + + def message_payload( text: str, *, @@ -35,7 +46,7 @@ def post_json( *, headers: dict[str, str] | None = None, ) -> httpx.Response: - request_headers = headers or auth_headers() + request_headers = conversation_headers(headers) response = httpx.post( f"{server.base_url}{path}", headers=request_headers, @@ -52,7 +63,7 @@ def collect_message_stream( *, headers: dict[str, str] | None = None, ) -> list[SSEEvent]: - request_headers = headers or auth_headers() + request_headers = conversation_headers(headers) stream_payload = {**payload, "stream": True} with httpx.stream( "POST", diff --git a/smoke/prereq/test_provider_prereq_live.py b/smoke/prereq/test_provider_prereq_live.py index 4f203f3833..0db600cee8 100644 --- a/smoke/prereq/test_provider_prereq_live.py +++ b/smoke/prereq/test_provider_prereq_live.py @@ -8,9 +8,9 @@ text_content, thinking_content, ) -from smoke.lib.config import ProviderModel, SmokeConfig, auth_headers +from smoke.lib.config import ProviderModel, SmokeConfig from smoke.lib.e2e import ProviderMatrixDriver -from smoke.lib.http import collect_message_stream, message_payload +from smoke.lib.http import collect_message_stream, conversation_headers, message_payload from smoke.lib.server import start_server from smoke.lib.skips import ( skip_if_upstream_unavailable_events, @@ -97,7 +97,7 @@ def test_client_disconnect_mid_stream_does_not_crash_server( with httpx.stream( "POST", f"{server.base_url}/v1/messages", - headers=auth_headers(), + headers=conversation_headers(), json=message_payload(smoke_config.prompt, model="fcc-smoke-default"), timeout=smoke_config.timeout_s, ) as response: diff --git a/smoke/product/test_provider_product_live.py b/smoke/product/test_provider_product_live.py index dc9f9328da..20a770d290 100644 --- a/smoke/product/test_provider_product_live.py +++ b/smoke/product/test_provider_product_live.py @@ -9,7 +9,7 @@ SSEEvent, parse_sse_lines, ) -from smoke.lib.config import ProviderModel, SmokeConfig, auth_headers +from smoke.lib.config import ProviderModel, SmokeConfig from smoke.lib.e2e import ( ConversationDriver, ProviderMatrixDriver, @@ -19,7 +19,7 @@ echo_tool_schema, tool_use_blocks, ) -from smoke.lib.http import collect_message_stream +from smoke.lib.http import collect_message_stream, conversation_headers from smoke.lib.skips import ( skip_if_upstream_unavailable_events, skip_if_upstream_unavailable_exception, @@ -166,7 +166,7 @@ def test_provider_error_e2e(smoke_config: SmokeConfig) -> None: ): response = client.post( f"{server.base_url}/v1/messages", - headers=auth_headers(), + headers=conversation_headers(), json={ "model": "fcc-smoke-default", "max_tokens": 32, @@ -605,11 +605,12 @@ def _scenario_reasoning_tool_continuation( def _scenario_disconnect( smoke_config: SmokeConfig, provider_model: ProviderModel ) -> None: + headers = conversation_headers() with _server_for_provider(smoke_config, provider_model, "disconnect") as server: with httpx.stream( "POST", f"{server.base_url}/v1/messages", - headers=auth_headers(), + headers=headers, json={ "model": "fcc-smoke-default", "max_tokens": 512, @@ -623,7 +624,7 @@ def _scenario_disconnect( health = httpx.get(f"{server.base_url}/health", timeout=5) assert health.status_code == 200 followup = ConversationDriver(server, smoke_config).ask( - "Reply with one short sentence." + "Reply with one short sentence.", headers=headers ) _assert_provider_product_stream(followup.events) @@ -646,4 +647,4 @@ def _openai_auth_headers(smoke_config: SmokeConfig) -> dict[str, str]: token = smoke_config.settings.proxy_auth_token if token: headers["authorization"] = f"Bearer {token}" - return headers + return conversation_headers(headers) diff --git a/tests/contracts/test_smoke_child_process.py b/tests/contracts/test_smoke_child_process.py index eedf9af8be..4aeb0efbf3 100644 --- a/tests/contracts/test_smoke_child_process.py +++ b/tests/contracts/test_smoke_child_process.py @@ -1,7 +1,11 @@ import subprocess +from contextlib import nullcontext from pathlib import Path from unittest.mock import MagicMock +import httpx +import pytest + from free_claude_code.config.settings import Settings from smoke.lib import child_process from smoke.lib import e2e as smoke_e2e @@ -12,10 +16,11 @@ cmd_python_c, run_captured_text, ) -from smoke.lib.config import SmokeConfig +from smoke.lib.config import ProviderModel, SmokeConfig from smoke.lib.e2e import ConversationDriver from smoke.lib.http import collect_message_stream from smoke.lib.server import RunningServer +from smoke.product import test_provider_product_live as provider_smoke def test_fcc_server_command_uses_cli_entrypoint() -> None: @@ -116,6 +121,141 @@ def fake_stream(_method: str, _url: str, **kwargs: object): assert turn.events[-1].event == "message_stop" +def test_smoke_conversation_identity_survives_turns_and_tool_results( + monkeypatch, tmp_path: Path +) -> None: + captured = [] + + def fake_stream(_method, _url, **kwargs): + captured.append(httpx.Headers(kwargs["headers"])) + return _FakeStreamResponse() + + monkeypatch.setattr(smoke_e2e.httpx, "stream", fake_stream) + server, config = _running_server(tmp_path), _smoke_config(tmp_path) + conversation = ConversationDriver(server, config) + conversation.ask("First turn") + conversation.ask("Second turn") + conversation.stream( + { + "messages": [ + { + "role": "user", + "content": [ + { + "type": "tool_result", + "tool_use_id": "tool-1", + "content": "done", + } + ], + } + ] + } + ) + ConversationDriver(server, config).ask("Different conversation") + + assert captured[0]["x-opencode-session"] + assert len({h["x-opencode-session"] for h in captured[:3]}) == 1 + assert captured[3]["x-opencode-session"] != captured[0]["x-opencode-session"] + assert all(h["user-agent"].startswith("fcc-smoke/") for h in captured) + + +@pytest.mark.parametrize( + "helper", [smoke_http.collect_message_stream, smoke_http.post_json] +) +def test_standalone_smoke_requests_identify_themselves_without_changing_auth( + monkeypatch, tmp_path: Path, helper +) -> None: + captured = [] + + def capture(*args, **kwargs): + captured.append(httpx.Headers(kwargs["headers"])) + return _FakeStreamResponse() + + monkeypatch.setattr(smoke_http.httpx, "stream", capture) + monkeypatch.setattr(smoke_http.httpx, "post", capture) + args: list[object] = [_running_server(tmp_path)] + if helper is smoke_http.post_json: + args.append("/v1/messages") + args += [{"messages": []}, _smoke_config(tmp_path)] + explicit = { + "Authorization": "Bearer explicit", + "X-OpenCode-Session": "caller", + "User-Agent": "caller/1.0", + } + helper(*args) + helper(*args) + helper(*args, headers=explicit) + helper(*args, headers={}) + + assert captured[0]["x-opencode-session"] != captured[1]["x-opencode-session"] + assert captured[0]["user-agent"].startswith("fcc-smoke/") + assert captured[2] == httpx.Headers(explicit) + assert "authorization" not in captured[3] + assert explicit["X-OpenCode-Session"] == "caller" + + +def test_provider_responses_smoke_sends_conversation_identity(monkeypatch, tmp_path): + captured = [] + + class ResponsesStream(_FakeStreamResponse): + def iter_lines(self): + return iter( + ( + "event: response.created", + "data: {}", + "", + "event: response.output_text.delta", + 'data: {"delta":"ok"}', + "", + "event: response.completed", + "data: {}", + "", + ) + ) + + def capture(*args, **kwargs): + captured.append(httpx.Headers(kwargs["headers"])) + return ResponsesStream() + + monkeypatch.setattr( + provider_smoke, + "_server_for_provider", + lambda *args: nullcontext(_running_server(tmp_path)), + ) + monkeypatch.setattr(provider_smoke.httpx, "stream", capture) + model = ProviderModel("opencode_go", "opencode_go/minimax-m2.7", "test") + for _ in range(2): + provider_smoke.test_provider_codex_responses_text_e2e( + _smoke_config(tmp_path), model + ) + assert captured[0]["x-opencode-session"] != captured[1]["x-opencode-session"] + assert all(h["user-agent"].startswith("fcc-smoke/") for h in captured) + + +def test_disconnect_smoke_reuses_identity_for_followup(monkeypatch, tmp_path): + captured = [] + + def capture(*args, **kwargs): + captured.append(httpx.Headers(kwargs["headers"])) + return _FakeStreamResponse() + + monkeypatch.setattr( + provider_smoke, + "_server_for_provider", + lambda *args: nullcontext(_running_server(tmp_path)), + ) + monkeypatch.setattr(provider_smoke.httpx, "stream", capture) + monkeypatch.setattr( + provider_smoke.httpx, "get", lambda *args, **kwargs: httpx.Response(200) + ) + model = ProviderModel("opencode_go", "opencode_go/minimax-m2.7", "test") + provider_smoke._scenario_disconnect(_smoke_config(tmp_path), model) + + assert len(captured) == 2 + assert captured[0]["x-opencode-session"] == captured[1]["x-opencode-session"] + assert all(h["user-agent"].startswith("fcc-smoke/") for h in captured) + + def test_run_captured_text_uses_utf8_replacement(monkeypatch, tmp_path: Path) -> None: calls: dict[str, object] = {} From b8d6e6d935d3ef0f5bb50b3340cc623b9a7c56da Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:28:44 -0700 Subject: [PATCH 4/6] patch: Use named reasoning effort for NVIDIA NIM (#1914) ## Why Nemotron Ultra 550B on NVIDIA NIM rejects numeric reasoning budgets because its deployed model runner does not support them. FCC currently converts named effort such as `high` into a numeric budget, causing repeated upstream 500 errors before Claude can use a tool. Direct requests with named effort succeed. ## How Use the existing named-effort encoder in NIM's shared request finalizer for Claude Messages and Codex Responses. Pass named efforts unchanged, send `none` for explicit off, use `high` for enabled reasoning without a named effort, and omit the control for provider defaults. Budget-only requests use the enabled default instead of sending a numeric budget. Remove the obsolete numeric-budget recovery code. Accepted behavior: NVIDIA's models support different sets of names. Unsupported values remain provider errors rather than being silently remapped or removed. For example, GPT-OSS 20B rejects `none`, `minimal`, `xhigh`, and `max`. No model-name allowlist is introduced. Acceptance of a name does not imply every model implements a distinct reasoning strength for it. --- .../providers/nvidia_nim/client.py | 24 +--- .../providers/nvidia_nim/request_options.py | 18 +-- .../providers/nvidia_nim/retry.py | 16 --- tests/providers/test_nim_request_clone.py | 42 ------ tests/providers/test_nvidia_nim.py | 125 ++---------------- tests/providers/test_nvidia_nim_reasoning.py | 85 ++++++++++++ tests/providers/test_nvidia_nim_request.py | 86 ++++-------- 7 files changed, 133 insertions(+), 263 deletions(-) delete mode 100644 tests/providers/test_nim_request_clone.py create mode 100644 tests/providers/test_nvidia_nim_reasoning.py diff --git a/src/free_claude_code/providers/nvidia_nim/client.py b/src/free_claude_code/providers/nvidia_nim/client.py index 441331f940..8afb4ed3db 100644 --- a/src/free_claude_code/providers/nvidia_nim/client.py +++ b/src/free_claude_code/providers/nvidia_nim/client.py @@ -33,7 +33,6 @@ ) from .retry import ( clone_body_without_chat_template, - clone_body_without_reasoning_budget, clone_body_without_reasoning_content, ) from .tool_schema import ( @@ -61,10 +60,7 @@ def __init__(self, nim_settings: NimSettings) -> None: @property def reasoning_off_fields(self) -> tuple[tuple[str, ...], ...]: - return ( - ("extra_body", "chat_template_kwargs", "thinking"), - ("extra_body", "chat_template_kwargs", "enable_thinking"), - ) + return (("reasoning_effort",),) @property def normal_max_tokens(self) -> int | None: @@ -118,17 +114,6 @@ def retry_request_body(self, error: Exception, body: dict) -> dict | None: error_text = f"{error_text} {json.dumps(error_body, default=str)}" error_text = error_text.lower() - if _is_reasoning_budget_rejection(error_text) and ( - bad_request_like or status_code == 500 - ): - retry_body = clone_body_without_reasoning_budget(body) - if retry_body is None: - return None - logger.warning( - "NIM_STREAM: retrying without reasoning budget after upstream rejection" - ) - return retry_body - if not bad_request_like: return None @@ -215,10 +200,3 @@ def _is_degraded_function(body: Mapping[str, Any]) -> bool: and function_id and state.strip() == _DEGRADED_FUNCTION_STATE ) - - -def _is_reasoning_budget_rejection(error_text: str) -> bool: - """Return whether NIM rejected optional thinking budget control.""" - if "reasoning_budget" in error_text: - return True - return "thinking_token_budget" in error_text and "reasoning_config" in error_text diff --git a/src/free_claude_code/providers/nvidia_nim/request_options.py b/src/free_claude_code/providers/nvidia_nim/request_options.py index ea43cf79a9..8c9f045bac 100644 --- a/src/free_claude_code/providers/nvidia_nim/request_options.py +++ b/src/free_claude_code/providers/nvidia_nim/request_options.py @@ -6,14 +6,21 @@ from free_claude_code.config.nim import NimSettings from free_claude_code.core.anthropic import ReasoningReplayMode, set_if_not_none from free_claude_code.core.anthropic.models import MessagesRequest -from free_claude_code.core.reasoning import ReasoningControl, ReasoningPolicy +from free_claude_code.core.reasoning import ReasoningEffort, ReasoningPolicy from free_claude_code.providers.openai_chat import ( + NamedEffortReasoning, OpenAIChatRequestPolicy, build_openai_chat_request_body, ) from .tool_schema import sanitize_nim_tool_schemas +NIM_REASONING = NamedEffortReasoning( + tuple((effort, effort.value) for effort in ReasoningEffort), + disabled_value="none", + enabled_value="high", +) + NIM_REQUEST_POLICY = OpenAIChatRequestPolicy( provider_name="NIM", reasoning_replay=ReasoningReplayMode.REASONING_CONTENT, @@ -106,14 +113,7 @@ def apply_nim_request_options( if not request_template_kwargs: extra_body.pop("chat_template_kwargs", None) - if reasoning.control is ReasoningControl.OFF or reasoning.requests_reasoning: - chat_template_kwargs = extra_body.setdefault("chat_template_kwargs", {}) - if isinstance(chat_template_kwargs, dict): - enabled = reasoning.control is not ReasoningControl.OFF - chat_template_kwargs["thinking"] = enabled - chat_template_kwargs["enable_thinking"] = enabled - if enabled and (budget := reasoning.numeric_budget_tokens) is not None: - chat_template_kwargs["reasoning_budget"] = budget + NIM_REASONING.encode(body, reasoning) _set_extra(extra_body, "top_k", nim.top_k, ignore_value=-1) _set_extra(extra_body, "min_p", nim.min_p, ignore_value=0.0) diff --git a/src/free_claude_code/providers/nvidia_nim/retry.py b/src/free_claude_code/providers/nvidia_nim/retry.py index 60d46b39b0..08b10dd8f6 100644 --- a/src/free_claude_code/providers/nvidia_nim/retry.py +++ b/src/free_claude_code/providers/nvidia_nim/retry.py @@ -7,11 +7,6 @@ from free_claude_code.core.history_replay import reasoning_context -def clone_body_without_reasoning_budget(body: dict[str, Any]) -> dict[str, Any] | None: - """Clone a request body and strip only reasoning_budget fields.""" - return _clone_strip_extra_body(body, _strip_reasoning_budget_fields) - - def clone_body_without_chat_template(body: dict[str, Any]) -> dict[str, Any] | None: """Clone a request body and strip NIM chat-template control fields.""" return _clone_strip_extra_body(body, _strip_chat_template_fields) @@ -42,17 +37,6 @@ def _clone_strip_extra_body( return cloned_body -def _strip_reasoning_budget_fields(extra_body: dict[str, Any]) -> bool: - removed = extra_body.pop("reasoning_budget", None) is not None - chat_template_kwargs = extra_body.get("chat_template_kwargs") - if ( - isinstance(chat_template_kwargs, dict) - and chat_template_kwargs.pop("reasoning_budget", None) is not None - ): - removed = True - return removed - - def _strip_chat_template_fields(extra_body: dict[str, Any]) -> bool: removed = extra_body.pop("chat_template", None) is not None if extra_body.pop("chat_template_kwargs", None) is not None: diff --git a/tests/providers/test_nim_request_clone.py b/tests/providers/test_nim_request_clone.py deleted file mode 100644 index 0e825094de..0000000000 --- a/tests/providers/test_nim_request_clone.py +++ /dev/null @@ -1,42 +0,0 @@ -"""Tests for NVIDIA NIM request body cloning helpers.""" - -from copy import deepcopy - -from free_claude_code.providers.nvidia_nim.retry import ( - clone_body_without_reasoning_budget, -) - - -def test_clone_body_without_reasoning_budget_strips_top_level_and_nested(): - body: dict = { - "model": "x", - "extra_body": { - "reasoning_budget": 99, - "chat_template_kwargs": {"reasoning_budget": 42, "thinking": True}, - "top_k": 1, - }, - } - original_extra = deepcopy(body["extra_body"]) - out = clone_body_without_reasoning_budget(body) - - assert out is not None - assert out["extra_body"]["chat_template_kwargs"] == {"thinking": True} - assert "reasoning_budget" not in out["extra_body"] - assert body["extra_body"] == original_extra - - -def test_clone_body_without_reasoning_budget_returns_none_when_unchanged(): - body = {"model": "x", "extra_body": {"top_k": 3}} - assert clone_body_without_reasoning_budget(body) is None - - -def test_clone_body_without_reasoning_budget_returns_none_without_extra_body(): - assert clone_body_without_reasoning_budget({"model": "y"}) is None - - -def test_clone_body_drops_empty_extra_body_after_strip(): - body = {"model": "z", "extra_body": {"reasoning_budget": 7}} - out = clone_body_without_reasoning_budget(body) - assert out is not None - assert "extra_body" not in out - assert "extra_body" in body diff --git a/tests/providers/test_nvidia_nim.py b/tests/providers/test_nvidia_nim.py index 7a8df11464..fe91fe1b37 100644 --- a/tests/providers/test_nvidia_nim.py +++ b/tests/providers/test_nvidia_nim.py @@ -16,7 +16,7 @@ encode_replay, ) from free_claude_code.core.openai_responses.models import OpenAIResponsesRequest -from free_claude_code.core.reasoning import ReasoningEffort, ReasoningPolicy +from free_claude_code.core.reasoning import ReasoningPolicy from free_claude_code.providers.admission import UPSTREAM_TRANSIENT_TOTAL_ATTEMPTS from free_claude_code.providers.nvidia_nim import NvidiaNimProvider from free_claude_code.providers.nvidia_nim.client import _PROFILE as NIM_PROFILE @@ -165,7 +165,6 @@ def _alias_events(): ("responses", None), ("messages", "chat_template"), ("responses", "chat_template"), - ("messages", "reasoning_budget"), ("messages", "reasoning_content"), ], ) @@ -398,10 +397,8 @@ async def test_build_request_body(provider_config): assert body["messages"][0]["content"] == "System prompt" assert "extra_body" in body - ctk = body["extra_body"]["chat_template_kwargs"] - assert ctk["thinking"] is True - assert ctk["enable_thinking"] is True - assert "reasoning_budget" not in ctk + assert body["reasoning_effort"] == "high" + assert "chat_template_kwargs" not in body["extra_body"] assert "reasoning_budget" not in body["extra_body"] @@ -454,10 +451,8 @@ def test_responses_request_uses_nim_chat_policy(provider_config): assert parameters["properties"] == {"_fcc_arg_type": {"type": "string"}} extra_body = body["extra_body"] assert isinstance(extra_body, dict) - assert extra_body["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - } + assert body["reasoning_effort"] == "high" + assert "chat_template_kwargs" not in extra_body @pytest.mark.asyncio @@ -473,10 +468,8 @@ async def test_build_request_body_encodes_explicit_reasoning_off( body = provider._chat._build_request_body(req, reasoning=REASONING_OFF) extra = body.get("extra_body", {}) - assert extra["chat_template_kwargs"] == { - "thinking": False, - "enable_thinking": False, - } + assert body["reasoning_effort"] == "none" + assert "chat_template_kwargs" not in extra assert "reasoning_budget" not in extra @@ -713,10 +706,7 @@ async def mock_stream(): second_extra = mock_create.call_args_list[1].kwargs["extra_body"] assert first_extra["chat_template"] == "custom_template" - assert first_extra["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - } + assert "chat_template_kwargs" not in first_extra assert "reasoning_budget" not in first_extra assert "chat_template" not in second_extra @@ -737,7 +727,10 @@ async def test_stream_messages_retries_without_chat_template_kwargs_issue_993( nim_settings=NimSettings(), admission=immediate_admission(), ) - req = make_request(model="mistralai/mistral-small-4-119b-2603") + req = make_request( + model="mistralai/mistral-small-4-119b-2603", + extra_body={"chat_template_kwargs": {"custom": "value"}}, + ) mock_chunk = MagicMock() mock_chunk.choices = [ @@ -770,10 +763,7 @@ async def mock_stream(): second_kwargs = mock_create.call_args_list[1].kwargs assert "chat_template" not in first_extra - assert first_extra["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - } + assert first_extra["chat_template_kwargs"] == {"custom": "value"} second_extra = second_kwargs.get("extra_body") or {} assert "chat_template" not in second_extra assert "chat_template_kwargs" not in second_extra @@ -1330,95 +1320,6 @@ async def mock_stream(): assert json.loads(deltas[0])["run_in_background"] is False -@pytest.mark.asyncio -async def test_stream_messages_retries_without_reasoning_budget(nim_provider): - req = make_request() - - mock_chunk = MagicMock() - mock_chunk.choices = [ - MagicMock( - delta=MagicMock(content="Recovered", reasoning_content=""), - finish_reason="stop", - ) - ] - mock_chunk.usage = MagicMock(completion_tokens=5) - - async def mock_stream(): - yield mock_chunk - - error = _make_bad_request_error("Unsupported field: reasoning_budget") - - with patch.object( - nim_provider._client.chat.completions, "create", new_callable=AsyncMock - ) as mock_create: - mock_create.side_effect = [error, SDKStreamDouble(mock_stream())] - - events = [ - e - async for e in nim_provider.stream_messages( - req, - reasoning=ReasoningPolicy.on(effort=ReasoningEffort.XHIGH), - ) - ] - - assert mock_create.await_count == 2 - first_call = mock_create.await_args_list[0].kwargs - second_call = mock_create.await_args_list[1].kwargs - assert first_call["extra_body"]["chat_template_kwargs"]["reasoning_budget"] == 4096 - assert "reasoning_budget" not in second_call["extra_body"] - assert "reasoning_budget" not in second_call["extra_body"]["chat_template_kwargs"] - assert second_call["extra_body"]["chat_template_kwargs"]["enable_thinking"] is True - assert any("Recovered" in event for event in events) - assert any("message_stop" in event for event in events) - - -@pytest.mark.asyncio -async def test_stream_messages_retries_without_budget_for_thinking_token_error( - nim_provider, -): - req = make_request(model="meta/llama-3.3-70b-instruct") - - mock_chunk = MagicMock() - mock_chunk.choices = [ - MagicMock( - delta=MagicMock(content="Recovered", reasoning_content=""), - finish_reason="stop", - ) - ] - mock_chunk.usage = MagicMock(completion_tokens=5) - - async def mock_stream(): - yield mock_chunk - - error = _make_internal_server_error( - "ValueError: thinking_token_budget is set but reasoning_config is not " - "configured. Please set --reasoning-config to use thinking_token_budget." - ) - - with patch.object( - nim_provider._client.chat.completions, "create", new_callable=AsyncMock - ) as mock_create: - mock_create.side_effect = [error, SDKStreamDouble(mock_stream())] - - events = [ - e - async for e in nim_provider.stream_messages( - req, reasoning=ReasoningPolicy.on(budget_tokens=77) - ) - ] - - assert mock_create.await_count == 2 - first_call = mock_create.await_args_list[0].kwargs - second_call = mock_create.await_args_list[1].kwargs - assert first_call["extra_body"]["chat_template_kwargs"]["reasoning_budget"] == 77 - assert "reasoning_budget" not in second_call["extra_body"] - assert "reasoning_budget" not in second_call["extra_body"]["chat_template_kwargs"] - assert second_call["extra_body"]["chat_template_kwargs"]["thinking"] is True - assert second_call["extra_body"]["chat_template_kwargs"]["enable_thinking"] is True - assert any("Recovered" in event for event in events) - assert any("message_stop" in event for event in events) - - @pytest.mark.asyncio async def test_stream_messages_retries_without_reasoning_content(nim_provider): req = make_request( diff --git a/tests/providers/test_nvidia_nim_reasoning.py b/tests/providers/test_nvidia_nim_reasoning.py new file mode 100644 index 0000000000..3bba07bdfa --- /dev/null +++ b/tests/providers/test_nvidia_nim_reasoning.py @@ -0,0 +1,85 @@ +"""NIM named reasoning across both supported ingress protocols.""" + +import pytest + +from free_claude_code.core.failures import ExecutionFailure +from free_claude_code.core.reasoning import ReasoningEffort, ReasoningPolicy +from tests.providers.test_history_transports import _harness, _saved_reply +from tests.providers.test_nvidia_nim import _alias_provider, _alias_request + + +@pytest.mark.asyncio +@pytest.mark.parametrize("wire", ["messages", "responses"]) +@pytest.mark.parametrize( + "policy,expected", + [ + *((ReasoningPolicy(effort=effort), effort.value) for effort in ReasoningEffort), + (ReasoningPolicy.off(), "none"), + (ReasoningPolicy.on(), "high"), + (ReasoningPolicy.on(budget_tokens=2048), "high"), + (ReasoningPolicy.provider_default(), None), + ], +) +async def test_named_effort_reaches_nim_without_numeric_budget(wire, policy, expected): + def responder(bodies): + body = bodies[-1] + assert body.get("reasoning_effort") == expected + assert "reasoning_budget" not in body + assert "chat_template_kwargs" not in body + return 200, [ + { + "id": "response", + "object": "chat.completion.chunk", + "model": "test-model", + "choices": [ + { + "index": 0, + "delta": {"content": "Hello"}, + "finish_reason": "stop", + } + ], + } + ] + + async with _harness("chat", responder, chat_provider_factory=_alias_provider) as ( + _, + bodies, + provider, + ): + stream = ( + provider.stream_messages + if wire == "messages" + else provider.stream_responses + ) + reply = await _saved_reply(stream(_alias_request(wire), reasoning=policy), wire) + + assert reply + assert len(bodies) == 1 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("wire", ["messages", "responses"]) +@pytest.mark.parametrize( + "policy", [ReasoningPolicy.off(), ReasoningPolicy(effort=ReasoningEffort.MAX)] +) +async def test_unsupported_named_effort_is_not_silently_remapped(wire, policy): + def responder(bodies): + return 400, { + "message": "reasoning_effort must be one of low, medium, high", + "param": "reasoning_effort", + } + + async with _harness("chat", responder, chat_provider_factory=_alias_provider) as ( + _, + bodies, + provider, + ): + stream = ( + provider.stream_messages + if wire == "messages" + else provider.stream_responses + ) + with pytest.raises(ExecutionFailure, match="reasoning_effort"): + await _saved_reply(stream(_alias_request(wire), reasoning=policy), wire) + + assert len(bodies) == 1 diff --git a/tests/providers/test_nvidia_nim_request.py b/tests/providers/test_nvidia_nim_request.py index 84b22b8716..cf022cebdb 100644 --- a/tests/providers/test_nvidia_nim_request.py +++ b/tests/providers/test_nvidia_nim_request.py @@ -102,32 +102,13 @@ def test_value_set_when_valid(self): class TestBuildRequestBody: - @pytest.mark.parametrize( - ("effort", "expected_budget"), - ( - (ReasoningEffort.MINIMAL, 512), - (ReasoningEffort.LOW, 512), - (ReasoningEffort.MEDIUM, 1_024), - (ReasoningEffort.HIGH, 2_048), - (ReasoningEffort.XHIGH, 4_096), - (ReasoningEffort.MAX, 8_192), - ), - ) - def test_named_effort_enables_thinking_with_numeric_budget( - self, - req, - effort: ReasoningEffort, - expected_budget: int, - ): - policy = ReasoningPolicy(effort=effort) - - body = build_request_body(req, NimSettings(), reasoning=policy) - - assert body["extra_body"]["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - "reasoning_budget": expected_budget, - } + @pytest.mark.parametrize("effort", list(ReasoningEffort)) + def test_named_effort_passes_through(self, req, effort): + body = build_request_body( + req, NimSettings(), reasoning=ReasoningPolicy(effort=effort) + ) + assert body["reasoning_effort"] == effort.value + assert "chat_template_kwargs" not in body["extra_body"] def test_named_effort_replaces_client_reasoning_budgets(self): req = make_messages_request( @@ -150,12 +131,8 @@ def test_named_effort_replaces_client_reasoning_budgets(self): extra_body = body["extra_body"] assert "reasoning_budget" not in extra_body - assert extra_body["chat_template_kwargs"] == { - "custom": "value", - "thinking": True, - "enable_thinking": True, - "reasoning_budget": 2048, - } + assert body["reasoning_effort"] == "high" + assert extra_body["chat_template_kwargs"] == {"custom": "value"} def test_max_tokens_capped_by_nim(self, req): req.max_tokens = 100000 @@ -377,7 +354,7 @@ def test_private_alias_metadata_is_stripped_without_mutating_body(self): "Grep": {"_fcc_arg_A": "-A"} } - def test_reasoning_params_in_extra_body(self): + def test_reasoning_uses_named_effort(self): req = make_messages_request( model="test", messages=[{"role": "user", "content": "hi"}], @@ -396,10 +373,8 @@ def test_reasoning_params_in_extra_body(self): nim = NimSettings() body = build_request_body(req, nim, reasoning=REASONING_ON) extra = body["extra_body"] - assert extra["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - } + assert body["reasoning_effort"] == "high" + assert "chat_template_kwargs" not in extra assert "reasoning_budget" not in extra def test_canonicalization_removes_empty_client_reasoning_envelope(self): @@ -493,13 +468,11 @@ def test_no_chat_template_kwargs_when_thinking_disabled(self): nim = NimSettings() body = build_request_body(req, nim, reasoning=REASONING_OFF) extra = body.get("extra_body", {}) - assert extra["chat_template_kwargs"] == { - "thinking": False, - "enable_thinking": False, - } + assert body["reasoning_effort"] == "none" + assert "chat_template_kwargs" not in extra assert "reasoning_budget" not in extra - def test_reasoning_budget_respects_existing_chat_template_kwargs(self): + def test_named_effort_preserves_unrelated_chat_template_kwargs(self): req = make_messages_request( model="test", messages=[{"role": "user", "content": "hi"}], @@ -521,11 +494,8 @@ def test_reasoning_budget_respects_existing_chat_template_kwargs(self): ) body = build_request_body(req, NimSettings(), reasoning=REASONING_ON) - assert body["extra_body"]["chat_template_kwargs"] == { - "enable_thinking": True, - "custom": "value", - "thinking": True, - } + assert body["reasoning_effort"] == "high" + assert body["extra_body"]["chat_template_kwargs"] == {"custom": "value"} def test_chat_template_fields_are_provider_wide(self): req = make_messages_request( @@ -546,10 +516,8 @@ def test_chat_template_fields_are_provider_wide(self): nim = NimSettings(chat_template="custom_template") body = build_request_body(req, nim, reasoning=REASONING_ON) extra = body.get("extra_body", {}) - assert extra["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - } + assert body["reasoning_effort"] == "high" + assert "chat_template_kwargs" not in extra assert extra["chat_template"] == "custom_template" def test_no_reasoning_params_in_extra_body(self): @@ -579,12 +547,10 @@ def test_no_reasoning_params_in_extra_body(self): "reasoning_effort", ): assert param not in extra - assert extra["chat_template_kwargs"] == { - "thinking": False, - "enable_thinking": False, - } + assert body["reasoning_effort"] == "none" + assert "chat_template_kwargs" not in extra - def test_explicit_reasoning_budget_is_preserved_exactly(self): + def test_budget_only_request_uses_enabled_named_effort(self): req = make_messages_request(model="test", thinking=None) body = build_request_body( @@ -593,11 +559,9 @@ def test_explicit_reasoning_budget_is_preserved_exactly(self): reasoning=ReasoningPolicy.on(budget_tokens=321), ) - assert body["extra_body"]["chat_template_kwargs"] == { - "thinking": True, - "enable_thinking": True, - "reasoning_budget": 321, - } + assert body["reasoning_effort"] == "high" + assert "chat_template_kwargs" not in body["extra_body"] + assert "reasoning_budget" not in body["extra_body"] def test_assistant_thinking_blocks_removed_when_disabled(self): req = make_messages_request( From 2e730696b6a7272c4fc63e7148c29574aeff1ed3 Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:37:03 -0700 Subject: [PATCH 5/6] patch: Consolidate Code storage into fcc.db with numbered migrations (#1913) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why FCC's Code history lives in `code/code.db`, with schema creation, upgrades, and restart recovery mixed into its store. Move this existing data into `fcc.db` and give schema changes an ordered migration history for future consolidation. ## How - Move the existing database automatically during startup after FCC has been stopped for the update. Recover and checkpoint SQLite journals, close the database, then rename it under the existing ownership lock. - Extract the three existing upgrades into numbered migration modules. Apply only pending versions, committing each migration and its version together. Preserve sessions, transcript ordering, prompts, and the current constraints. - Keep Code restart recovery separate from schema upgrades. Preserve conflicting files on failure, resume pending migrations on restart, and retain independently retryable retired Chat cleanup.

RetriggerConfidence Score: 4/5

Do not merge until NIM preserves explicit reasoning budgets or rejects requests it cannot represent. What we checked: - T-Rex produced a proof for the posted P1 finding, including a credential-free NIM request reproduction script. T-Rex - T-Rex produced a second proof for a posted P1 finding. See the corresponding review comment for finding details. T-Rex - T-Rex executed a general-contract-validation-proof by running Nim budget repro before and after; the policy retained budget 321 with no numeric budget in both runs, and the mocked 200 OK responses do not establish live NIM acceptance. T-Rex
Summary The PR moves Code history into `fcc.db`, adds ordered SQLite migrations, and updates the smoke test to read the new database. The NIM request change drops explicit numeric reasoning budgets and must be corrected before merging.
Reviews (3) · Last reviewed commit: ["Merge branch 'main' into sqlite-fcc-stor..."](https://github.com/alishahryar1/free-claude-code/commit/1bff6bf9b66e39cbbf85647bd0e6a7d2f08b6338) --- e2e/test_code_sessions.py | 8 +- .../product/test_codex_modes_product_live.py | 7 +- src/free_claude_code/config/paths.py | 10 +- src/free_claude_code/runtime/bootstrap.py | 9 +- .../runtime/code_sessions_sqlite.py | 155 ++---- .../runtime/sqlite_database.py | 163 ++++++ .../runtime/sqlite_migrations/__init__.py | 17 + .../sqlite_migrations/v0001_code_schema.py | 97 ++++ .../v0002_code_permissions.py | 16 + .../v0003_code_context_usage.py | 10 + tests/contracts/test_codex_smoke_storage.py | 58 +++ tests/runtime/fixtures/code_schema_v0.sql | 39 ++ tests/runtime/test_sqlite_database.py | 472 ++++++++++++++++++ 13 files changed, 929 insertions(+), 132 deletions(-) create mode 100644 src/free_claude_code/runtime/sqlite_database.py create mode 100644 src/free_claude_code/runtime/sqlite_migrations/__init__.py create mode 100644 src/free_claude_code/runtime/sqlite_migrations/v0001_code_schema.py create mode 100644 src/free_claude_code/runtime/sqlite_migrations/v0002_code_permissions.py create mode 100644 src/free_claude_code/runtime/sqlite_migrations/v0003_code_context_usage.py create mode 100644 tests/contracts/test_codex_smoke_storage.py create mode 100644 tests/runtime/fixtures/code_schema_v0.sql create mode 100644 tests/runtime/test_sqlite_database.py diff --git a/e2e/test_code_sessions.py b/e2e/test_code_sessions.py index 10d1825b83..550f094ccb 100644 --- a/e2e/test_code_sessions.py +++ b/e2e/test_code_sessions.py @@ -1726,7 +1726,13 @@ def test_off_clears_effort_when_reasoning_becomes_unavailable( ): url = create_session(page, admin_base_url, tmp_path) effort = page.locator("#codeReasoning") - effort.select_option("high") + with page.expect_response( + lambda response: ( + response.request.method == "PATCH" and "/api/code/sessions/" in response.url + ) + ) as changed: + effort.select_option("high") + assert changed.value.json()["reasoning_effort"] == "high" expect(effort).to_be_enabled() page.locator("#codeComposer").fill("Preserve this draft") code_control.harness.efforts = ("off",) diff --git a/smoke/product/test_codex_modes_product_live.py b/smoke/product/test_codex_modes_product_live.py index a69d5675b8..046c80d7f2 100644 --- a/smoke/product/test_codex_modes_product_live.py +++ b/smoke/product/test_codex_modes_product_live.py @@ -22,6 +22,7 @@ atomic_write_managed_config, settings_env_keys, ) +from free_claude_code.config.paths import FCC_DATABASE_FILENAME from free_claude_code.config.provider_catalog import PROVIDER_CATALOG from smoke.lib.child_process import run_captured_text from smoke.lib.config import SmokeConfig @@ -554,7 +555,11 @@ def test_codex_child_review_local_e2e( ) assert marker.read_text().strip() == "smoke" with closing( - sqlite3.connect(tmp_path / "home" / ".fcc" / "code" / "code.db") + sqlite3.connect( + (tmp_path / "home" / ".fcc" / FCC_DATABASE_FILENAME).as_uri() + + "?mode=ro", + uri=True, + ) ) as database: root = database.execute( "SELECT native_thread_id FROM code_sessions WHERE id = ?", diff --git a/src/free_claude_code/config/paths.py b/src/free_claude_code/config/paths.py index cefcf75f60..7e353bb13d 100644 --- a/src/free_claude_code/config/paths.py +++ b/src/free_claude_code/config/paths.py @@ -17,6 +17,7 @@ FCC_TEMP_DIRNAME = "tmp" LAUNCHER_TEMP_DIRNAME = "launchers" CODE_STATE_DIRNAME = "code" +FCC_DATABASE_FILENAME = "fcc.db" CODE_DATABASE_FILENAME = "code.db" CODE_LOCK_FILENAME = "code.lock" @@ -45,8 +46,13 @@ def launcher_temp_dir_path() -> Path: return config_dir_path() / FCC_TEMP_DIRNAME / LAUNCHER_TEMP_DIRNAME -def code_database_path() -> Path: - """Return the managed Code sessions database path.""" +def fcc_database_path() -> Path: + """Return the shared FCC database path.""" + return config_dir_path() / FCC_DATABASE_FILENAME + + +def legacy_code_database_path() -> Path: + """Return the former Code database path for startup relocation.""" return config_dir_path() / CODE_STATE_DIRNAME / CODE_DATABASE_FILENAME diff --git a/src/free_claude_code/runtime/bootstrap.py b/src/free_claude_code/runtime/bootstrap.py index 3972ee207e..b8c661116e 100644 --- a/src/free_claude_code/runtime/bootstrap.py +++ b/src/free_claude_code/runtime/bootstrap.py @@ -11,8 +11,9 @@ from free_claude_code.config.loader import ManagedConfigStore from free_claude_code.config.logging_config import configure_logging from free_claude_code.config.paths import ( - code_database_path, code_lock_path, + fcc_database_path, + legacy_code_database_path, server_log_path, ) from free_claude_code.config.settings import Settings @@ -73,7 +74,11 @@ def build_asgi_app( model_catalog_publisher=CodexModelCatalogPublisher(), ) code_service = CodeService( - SQLiteCodeStore(code_database_path(), code_lock_path()), + SQLiteCodeStore( + fcc_database_path(), + code_lock_path(), + legacy_database_path=legacy_code_database_path(), + ), CodexHarnessFactory(provider_manager), ) runtime = ApplicationRuntime( diff --git a/src/free_claude_code/runtime/code_sessions_sqlite.py b/src/free_claude_code/runtime/code_sessions_sqlite.py index 036efc484d..eddb9f926e 100644 --- a/src/free_claude_code/runtime/code_sessions_sqlite.py +++ b/src/free_claude_code/runtime/code_sessions_sqlite.py @@ -25,6 +25,8 @@ ) from free_claude_code.core.interprocess_lock import InterprocessFileLock +from .sqlite_database import initialize_database + _JSON_FIELDS = frozenset( {"raw", "form", "error_details", "request_id", "native_permission_defaults"} ) @@ -299,58 +301,16 @@ def _write_prompt(connection: sqlite3.Connection, prompt: CodePrompt) -> None: ) -def _migrate_prompt_entries(connection: sqlite3.Connection) -> None: - """Give old prompts a permanent run-end position without rewriting history.""" - for row in connection.execute( - "SELECT * FROM code_prompts ORDER BY rowid" - ).fetchall(): - prompt = _record(CodePrompt, row) - run = connection.execute( - "SELECT id FROM code_runs WHERE session_id = ? AND native_turn_id = ?", - (prompt.session_id, prompt.native_turn_id), - ).fetchone() - if run is None and prompt.native_item_id is not None: - matches = connection.execute( - "SELECT DISTINCT run_id AS id FROM code_items WHERE session_id = ? AND native_item_id = ?", - (prompt.session_id, prompt.native_item_id), - ).fetchall() - if len(matches) == 1: - run = matches[0] - if run is None: - run = connection.execute( - "SELECT id FROM code_runs WHERE session_id = ? ORDER BY ordinal DESC LIMIT 1", - (prompt.session_id,), - ).fetchone() - if run is None: - raise CodeUnavailableError("A saved prompt has no saved conversation turn.") - sequence = connection.execute( - "SELECT COALESCE(MAX(sequence), 0) + 1 FROM code_items WHERE session_id = ?", - (prompt.session_id,), - ).fetchone()[0] - _insert( - connection, - "code_items", - CodeItem( - id=prompt.id, - session_id=prompt.session_id, - run_id=run["id"], - sequence=sequence, - kind="prompt", - complete=True, - ), - ) - connection.execute("CREATE TABLE code_prompts_new" + _PROMPT_COLUMNS) - connection.execute("INSERT INTO code_prompts_new SELECT * FROM code_prompts") - connection.execute("DROP TABLE code_prompts") - connection.execute("ALTER TABLE code_prompts_new RENAME TO code_prompts") - if connection.execute("PRAGMA foreign_key_check").fetchone() is not None: - raise CodeUnavailableError("Code session history has an invalid record link.") - connection.execute("PRAGMA user_version = 1") - - class SQLiteCodeStore: - def __init__(self, database_path: Path, lock_path: Path) -> None: + def __init__( + self, + database_path: Path, + lock_path: Path, + *, + legacy_database_path: Path | None = None, + ) -> None: self._path = database_path + self._legacy_path = legacy_database_path self._lock = InterprocessFileLock(lock_path) self._started = False self._lifecycle = asyncio.Lock() @@ -371,14 +331,28 @@ async def start(self) -> None: self._started = True def _initialize(self) -> None: - self._path.parent.mkdir(parents=True, exist_ok=True) if not self._lock.acquire(): raise CodeUnavailableError( "Code sessions is already owned by another FCC server." ) - if os.name != "nt": - self._path.parent.chmod(0o700) - self._execute(self._initialize_schema) + try: + self._path.parent.mkdir(parents=True, exist_ok=True) + if os.name != "nt": + self._path.parent.chmod(0o700) + initialize_database(self._path, self._legacy_path) + except sqlite3.IntegrityError as exc: + raise CodeConflictError( + "Saved Code history conflicts with its schema." + ) from exc + except sqlite3.Error as exc: + raise CodeUnavailableError( + f"Code storage initialization failed: {exc}" + ) from exc + except OSError as exc: + raise CodeUnavailableError( + "Code database files could not be initialized or moved." + ) from exc + self._execute(self._recover) if os.name != "nt": for path in ( self._path, @@ -412,30 +386,7 @@ async def _run[T](self, operation: Callable[[sqlite3.Connection], T]) -> T: raise CodeUnavailableError("Code session storage is closed.") return await anyio.to_thread.run_sync(self._execute, operation) - def _initialize_schema(self, connection: sqlite3.Connection) -> None: - connection.execute("PRAGMA journal_mode = WAL") - connection.executescript(_SCHEMA) - connection.execute("BEGIN IMMEDIATE") - if connection.execute("PRAGMA user_version").fetchone()[0] == 0: - _migrate_prompt_entries(connection) - if connection.execute("PRAGMA user_version").fetchone()[0] == 1: - for table in ("code_sessions", "code_runs"): - connection.execute( - f"ALTER TABLE {table} ADD COLUMN mode TEXT NOT NULL DEFAULT 'config' " - "CHECK(mode IN ('config','ask','auto_review','full_access'))" - ) - connection.execute( - "ALTER TABLE code_sessions ADD COLUMN native_permission_defaults TEXT " - "CHECK(native_permission_defaults IS NULL OR " - "(json_valid(native_permission_defaults) AND json_type(native_permission_defaults) = 'object'))" - ) - connection.execute("PRAGMA user_version = 2") - if connection.execute("PRAGMA user_version").fetchone()[0] == 2: - connection.execute( - "ALTER TABLE code_sessions ADD COLUMN context_used_tokens INTEGER " - "CHECK(context_used_tokens IS NULL OR context_used_tokens >= 0)" - ) - connection.execute("PRAGMA user_version = 3") + def _recover(self, connection: sqlite3.Connection) -> None: connection.execute( "UPDATE code_runs SET status = 'interrupted', finished_at = ?, error = ? " "WHERE status IN ('preparing','running','stopping')", @@ -735,51 +686,3 @@ def operation(connection: sqlite3.Connection) -> None: connection.execute("DELETE FROM code_sessions WHERE id = ?", (session_id,)) await self._run(operation) - - -_PROMPT_COLUMNS = """( - session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, - generation TEXT NOT NULL, request_id TEXT NOT NULL, native_turn_id TEXT, native_item_id TEXT, - kind TEXT NOT NULL, form TEXT NOT NULL, raw TEXT NOT NULL, - status TEXT NOT NULL CHECK(status IN ('pending','answering','resolved','expired')), response_id TEXT, error TEXT, - PRIMARY KEY(session_id,id), UNIQUE(session_id,generation,request_id), UNIQUE(session_id,response_id), - FOREIGN KEY(session_id,id) REFERENCES code_items(session_id,id) ON DELETE CASCADE -)""" - -_SCHEMA = ( - """ -CREATE TABLE IF NOT EXISTS code_sessions( - id TEXT PRIMARY KEY NOT NULL, cwd TEXT NOT NULL, model TEXT NOT NULL, reasoning_effort TEXT, - harness TEXT NOT NULL CHECK(harness = 'codex'), title TEXT NOT NULL, title_search TEXT NOT NULL, - cwd_search TEXT NOT NULL, auto_title INTEGER NOT NULL CHECK(auto_title IN (0,1)), native_thread_id TEXT, - native_may_have_input INTEGER NOT NULL CHECK(native_may_have_input IN (0,1)), - revision INTEGER NOT NULL CHECK(revision > 0), status TEXT NOT NULL CHECK(status IN ('ready','deleting','delete_uncertain')), - error TEXT, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL -); -CREATE INDEX IF NOT EXISTS code_sessions_recent ON code_sessions(updated_at DESC, id DESC); -CREATE TABLE IF NOT EXISTS code_runs( - session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, - ordinal INTEGER NOT NULL CHECK(ordinal > 0), text TEXT NOT NULL, model TEXT NOT NULL, reasoning_effort TEXT, - status TEXT NOT NULL CHECK(status IN ('preparing','running','stopping','completed','interrupted','failed')), - submission_started INTEGER NOT NULL CHECK(submission_started IN (0,1)), native_turn_id TEXT, - stop_requested INTEGER NOT NULL CHECK(stop_requested IN (0,1)), error TEXT, error_details TEXT NOT NULL, - created_at INTEGER NOT NULL, finished_at INTEGER, - PRIMARY KEY(session_id,id), UNIQUE(session_id,ordinal), UNIQUE(session_id,native_turn_id) -); -CREATE UNIQUE INDEX IF NOT EXISTS code_one_active_run ON code_runs(session_id) - WHERE status IN ('preparing','running','stopping'); -CREATE TABLE IF NOT EXISTS code_items( - session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, - run_id TEXT NOT NULL, sequence INTEGER NOT NULL CHECK(sequence > 0), native_turn_id TEXT, native_item_id TEXT, - kind TEXT NOT NULL, title TEXT NOT NULL, text TEXT NOT NULL, detail TEXT NOT NULL, - complete INTEGER NOT NULL CHECK(complete IN (0,1)), raw TEXT NOT NULL, - PRIMARY KEY(session_id,id), FOREIGN KEY(session_id,run_id) REFERENCES code_runs(session_id,id) ON DELETE CASCADE, - UNIQUE(session_id,sequence), UNIQUE(session_id,native_turn_id,native_item_id) -); -CREATE INDEX IF NOT EXISTS code_items_run ON code_items(session_id,run_id,sequence); -CREATE TABLE IF NOT EXISTS code_deleted(id TEXT PRIMARY KEY NOT NULL); -""" - + "CREATE TABLE IF NOT EXISTS code_prompts" - + _PROMPT_COLUMNS - + ";" -) diff --git a/src/free_claude_code/runtime/sqlite_database.py b/src/free_claude_code/runtime/sqlite_database.py new file mode 100644 index 0000000000..1997171daa --- /dev/null +++ b/src/free_claude_code/runtime/sqlite_database.py @@ -0,0 +1,163 @@ +"""Offline relocation and transactional schema initialization of FCC's database. + +The caller must hold the Code owner lock until the store closes. Connections +opened here never escape initialization, including when migration fails. +""" + +import sqlite3 +from contextlib import closing +from pathlib import Path + +from .sqlite_migrations import MIGRATIONS + +# Historical columns identify supported files without depending on live models. +_BASE_COLUMNS = { + "code_sessions": "id cwd model reasoning_effort harness title title_search cwd_search " + "auto_title native_thread_id native_may_have_input revision status error created_at updated_at", + "code_runs": "session_id id ordinal text model reasoning_effort status submission_started " + "native_turn_id stop_requested error error_details created_at finished_at", + "code_items": "session_id id run_id sequence native_turn_id native_item_id kind title text detail complete raw", + "code_prompts": "session_id id generation request_id native_turn_id native_item_id kind form raw status response_id error", + "code_deleted": "id", +} +_SIDECARS = ("-wal", "-shm", "-journal") + + +def _connect(path: Path, *, existing: bool = False) -> sqlite3.Connection: + connection = sqlite3.connect( + path.as_uri() + ("?mode=rw" if existing else "?mode=rwc"), + uri=True, + timeout=10, + autocommit=True, + ) + connection.row_factory = sqlite3.Row + connection.execute("PRAGMA foreign_keys = ON") + return connection + + +def _schema_version(connection: sqlite3.Connection) -> int: + version = connection.execute("PRAGMA user_version").fetchone()[0] + if not 0 <= version <= MIGRATIONS[-1][0]: + raise sqlite3.DatabaseError(f"Unsupported FCC database version {version}.") + tables = { + row[0] + for row in connection.execute( + "SELECT name FROM sqlite_schema WHERE type IN ('table','view') AND name NOT LIKE 'sqlite_%'" + ) + } + if version == 0 and not tables: + return version + if tables != _BASE_COLUMNS.keys(): + raise sqlite3.DatabaseError( + "Unrecognized FCC database schema. Saved data was preserved." + ) + for table, baseline in _BASE_COLUMNS.items(): + expected = set(baseline.split()) + if version >= 2 and table in ("code_sessions", "code_runs"): + expected.add("mode") + if version >= 2 and table == "code_sessions": + expected.add("native_permission_defaults") + if version >= 3 and table == "code_sessions": + expected.add("context_used_tokens") + actual = {row[1] for row in connection.execute(f"PRAGMA table_info({table})")} + if not expected <= actual: + raise sqlite3.DatabaseError(f"Incomplete FCC database schema in {table}.") + return version + + +def _sidecars(path: Path) -> bool: + return any( + Path(f"{path}{suffix}").exists() or Path(f"{path}{suffix}").is_symlink() + for suffix in _SIDECARS + ) + + +def _check_path(path: Path) -> None: + if path.is_symlink(): + raise sqlite3.DatabaseError( + "FCC database path is redirected. Saved data was preserved." + ) + if not path.exists() and _sidecars(path): + raise sqlite3.DatabaseError( + "FCC database has orphan journal files. Saved data was preserved." + ) + + +def _relocate(source: Path, destination: Path) -> None: + with closing(_connect(source, existing=True)) as connection: + _schema_version(connection) + if connection.execute("PRAGMA journal_mode").fetchone()[0] == "wal": + busy, pages, checkpointed = connection.execute( + "PRAGMA wal_checkpoint(TRUNCATE)" + ).fetchone() + if busy or pages != checkpointed: + raise sqlite3.DatabaseError( + "FCC database checkpoint is busy. Stop FCC before updating." + ) + if connection.execute("PRAGMA journal_mode=DELETE").fetchone()[0] != "delete": + raise sqlite3.DatabaseError( + "FCC database journals are still in use. Stop FCC before updating." + ) + if _sidecars(source): + raise sqlite3.DatabaseError( + "FCC database journal files remain. Saved data was preserved." + ) + _check_path(destination) + if destination.exists(): + raise sqlite3.DatabaseError( + "FCC has both legacy and current databases. Resolve the file conflict before starting Code." + ) + source.rename(destination) + + +def initialize_database(path: Path, legacy_path: Path | None = None) -> None: + """Apply pending migrations, relocating the legacy file first if present.""" + if tuple(version for version, _ in MIGRATIONS) != tuple( + range(1, len(MIGRATIONS) + 1) + ): + raise sqlite3.DatabaseError( + "FCC database migration versions must be contiguous." + ) + path = path.parent.resolve() / path.name + _check_path(path) + if legacy_path is not None: + legacy_path = ( + legacy_path.parent.parent.resolve() + / legacy_path.parent.name + / legacy_path.name + ) + if legacy_path.parent.resolve() != legacy_path.parent: + raise sqlite3.DatabaseError( + "Legacy Code directory is redirected. Saved data was preserved." + ) + _check_path(legacy_path) + if legacy_path.exists() and path.exists(): + raise sqlite3.DatabaseError( + "FCC has both legacy and current databases. Resolve the file conflict before starting Code." + ) + path.parent.mkdir(parents=True, exist_ok=True) + if legacy_path is not None and legacy_path.exists(): + _relocate(legacy_path, path) + with closing(_connect(path)) as connection: + version = _schema_version(connection) + if connection.execute("PRAGMA journal_mode=WAL").fetchone()[0] != "wal": + raise sqlite3.DatabaseError("FCC database could not enable WAL.") + for next_version, upgrade in MIGRATIONS: + if next_version <= version: + continue + connection.execute("BEGIN IMMEDIATE") + try: + upgrade(connection) + if ( + connection.execute("PRAGMA foreign_key_check").fetchone() + is not None + ): + raise sqlite3.DatabaseError( + "FCC database has an invalid record link." + ) + connection.execute(f"PRAGMA user_version={next_version}") + connection.execute("COMMIT") + except BaseException: + if connection.in_transaction: + connection.execute("ROLLBACK") + raise diff --git a/src/free_claude_code/runtime/sqlite_migrations/__init__.py b/src/free_claude_code/runtime/sqlite_migrations/__init__.py new file mode 100644 index 0000000000..186b3ac517 --- /dev/null +++ b/src/free_claude_code/runtime/sqlite_migrations/__init__.py @@ -0,0 +1,17 @@ +"""Ordered FCC database history. Published migrations are immutable. + +Add the next numbered module and append its upgrade function here. The runner +owns transactions and user_version. Migrations use explicit execute statements, +never executescript, and must not import mutable domain models or serializers. +""" + +import sqlite3 +from collections.abc import Callable + +from . import v0001_code_schema, v0002_code_permissions, v0003_code_context_usage + +MIGRATIONS: tuple[tuple[int, Callable[[sqlite3.Connection], None]], ...] = ( + (1, v0001_code_schema.upgrade), + (2, v0002_code_permissions.upgrade), + (3, v0003_code_context_usage.upgrade), +) diff --git a/src/free_claude_code/runtime/sqlite_migrations/v0001_code_schema.py b/src/free_claude_code/runtime/sqlite_migrations/v0001_code_schema.py new file mode 100644 index 0000000000..0e0a21129a --- /dev/null +++ b/src/free_claude_code/runtime/sqlite_migrations/v0001_code_schema.py @@ -0,0 +1,97 @@ +"""Version 1: baseline Code tables and permanent transcript entries for prompts.""" + +import sqlite3 + +_PROMPT_COLUMNS = """( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + generation TEXT NOT NULL, request_id TEXT NOT NULL, native_turn_id TEXT, native_item_id TEXT, + kind TEXT NOT NULL, form TEXT NOT NULL, raw TEXT NOT NULL, + status TEXT NOT NULL CHECK(status IN ('pending','answering','resolved','expired')), response_id TEXT, error TEXT, + PRIMARY KEY(session_id,id), UNIQUE(session_id,generation,request_id), UNIQUE(session_id,response_id), + FOREIGN KEY(session_id,id) REFERENCES code_items(session_id,id) ON DELETE CASCADE +)""" + +_SCHEMA = ( + """CREATE TABLE IF NOT EXISTS code_sessions( + id TEXT PRIMARY KEY NOT NULL, cwd TEXT NOT NULL, model TEXT NOT NULL, reasoning_effort TEXT, + harness TEXT NOT NULL CHECK(harness = 'codex'), title TEXT NOT NULL, title_search TEXT NOT NULL, + cwd_search TEXT NOT NULL, auto_title INTEGER NOT NULL CHECK(auto_title IN (0,1)), native_thread_id TEXT, + native_may_have_input INTEGER NOT NULL CHECK(native_may_have_input IN (0,1)), + revision INTEGER NOT NULL CHECK(revision > 0), status TEXT NOT NULL CHECK(status IN ('ready','deleting','delete_uncertain')), + error TEXT, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL +)""", + """CREATE INDEX IF NOT EXISTS code_sessions_recent ON code_sessions(updated_at DESC, id DESC)""", + """CREATE TABLE IF NOT EXISTS code_runs( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + ordinal INTEGER NOT NULL CHECK(ordinal > 0), text TEXT NOT NULL, model TEXT NOT NULL, reasoning_effort TEXT, + status TEXT NOT NULL CHECK(status IN ('preparing','running','stopping','completed','interrupted','failed')), + submission_started INTEGER NOT NULL CHECK(submission_started IN (0,1)), native_turn_id TEXT, + stop_requested INTEGER NOT NULL CHECK(stop_requested IN (0,1)), error TEXT, error_details TEXT NOT NULL, + created_at INTEGER NOT NULL, finished_at INTEGER, + PRIMARY KEY(session_id,id), UNIQUE(session_id,ordinal), UNIQUE(session_id,native_turn_id) +)""", + """CREATE UNIQUE INDEX IF NOT EXISTS code_one_active_run ON code_runs(session_id) + WHERE status IN ('preparing','running','stopping')""", + """CREATE TABLE IF NOT EXISTS code_items( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + run_id TEXT NOT NULL, sequence INTEGER NOT NULL CHECK(sequence > 0), native_turn_id TEXT, native_item_id TEXT, + kind TEXT NOT NULL, title TEXT NOT NULL, text TEXT NOT NULL, detail TEXT NOT NULL, + complete INTEGER NOT NULL CHECK(complete IN (0,1)), raw TEXT NOT NULL, + PRIMARY KEY(session_id,id), FOREIGN KEY(session_id,run_id) REFERENCES code_runs(session_id,id) ON DELETE CASCADE, + UNIQUE(session_id,sequence), UNIQUE(session_id,native_turn_id,native_item_id) +)""", + """CREATE INDEX IF NOT EXISTS code_items_run ON code_items(session_id,run_id,sequence)""", + """CREATE TABLE IF NOT EXISTS code_deleted(id TEXT PRIMARY KEY NOT NULL)""", + """CREATE TABLE IF NOT EXISTS code_prompts( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + generation TEXT NOT NULL, request_id TEXT NOT NULL, native_turn_id TEXT, native_item_id TEXT, + kind TEXT NOT NULL, form TEXT NOT NULL, raw TEXT NOT NULL, + status TEXT NOT NULL CHECK(status IN ('pending','answering','resolved','expired')), response_id TEXT, error TEXT, + PRIMARY KEY(session_id,id), UNIQUE(session_id,generation,request_id), UNIQUE(session_id,response_id), + FOREIGN KEY(session_id,id) REFERENCES code_items(session_id,id) ON DELETE CASCADE +)""", +) + + +def upgrade(connection: sqlite3.Connection) -> None: + for statement in _SCHEMA: + connection.execute(statement) + for prompt in connection.execute( + "SELECT * FROM code_prompts ORDER BY rowid" + ).fetchall(): + run = connection.execute( + "SELECT id FROM code_runs WHERE session_id = ? AND native_turn_id = ?", + (prompt["session_id"], prompt["native_turn_id"]), + ).fetchone() + if run is None and prompt["native_item_id"] is not None: + matches = connection.execute( + "SELECT DISTINCT run_id AS id FROM code_items WHERE session_id = ? AND native_item_id = ?", + (prompt["session_id"], prompt["native_item_id"]), + ).fetchall() + if len(matches) == 1: + run = matches[0] + if run is None: + run = connection.execute( + "SELECT id FROM code_runs WHERE session_id = ? ORDER BY ordinal DESC LIMIT 1", + (prompt["session_id"],), + ).fetchone() + if run is None: + raise sqlite3.DatabaseError( + "A saved prompt has no saved conversation turn." + ) + sequence = connection.execute( + "SELECT COALESCE(MAX(sequence), 0) + 1 FROM code_items WHERE session_id = ?", + (prompt["session_id"],), + ).fetchone()[0] + connection.execute( + "INSERT INTO code_items (session_id,id,run_id,sequence,kind,title,text,detail,complete,raw) " + "VALUES (?,?,?,?,'prompt','','','',1,'{}')", + (prompt["session_id"], prompt["id"], run["id"], sequence), + ) + connection.execute("CREATE TABLE code_prompts_new" + _PROMPT_COLUMNS) + columns = "session_id,id,generation,request_id,native_turn_id,native_item_id,kind,form,raw,status,response_id,error" + connection.execute( + f"INSERT INTO code_prompts_new ({columns}) SELECT {columns} FROM code_prompts" + ) + connection.execute("DROP TABLE code_prompts") + connection.execute("ALTER TABLE code_prompts_new RENAME TO code_prompts") diff --git a/src/free_claude_code/runtime/sqlite_migrations/v0002_code_permissions.py b/src/free_claude_code/runtime/sqlite_migrations/v0002_code_permissions.py new file mode 100644 index 0000000000..04dde1eeca --- /dev/null +++ b/src/free_claude_code/runtime/sqlite_migrations/v0002_code_permissions.py @@ -0,0 +1,16 @@ +"""Version 2: persisted permission mode and original native defaults.""" + +import sqlite3 + + +def upgrade(connection: sqlite3.Connection) -> None: + for table in ("code_sessions", "code_runs"): + connection.execute( + f"ALTER TABLE {table} ADD COLUMN mode TEXT NOT NULL DEFAULT 'config' " + "CHECK(mode IN ('config','ask','auto_review','full_access'))" + ) + connection.execute( + "ALTER TABLE code_sessions ADD COLUMN native_permission_defaults TEXT " + "CHECK(native_permission_defaults IS NULL OR " + "(json_valid(native_permission_defaults) AND json_type(native_permission_defaults) = 'object'))" + ) diff --git a/src/free_claude_code/runtime/sqlite_migrations/v0003_code_context_usage.py b/src/free_claude_code/runtime/sqlite_migrations/v0003_code_context_usage.py new file mode 100644 index 0000000000..d6b029dbf2 --- /dev/null +++ b/src/free_claude_code/runtime/sqlite_migrations/v0003_code_context_usage.py @@ -0,0 +1,10 @@ +"""Version 3: optional model context usage for Code sessions.""" + +import sqlite3 + + +def upgrade(connection: sqlite3.Connection) -> None: + connection.execute( + "ALTER TABLE code_sessions ADD COLUMN context_used_tokens INTEGER " + "CHECK(context_used_tokens IS NULL OR context_used_tokens >= 0)" + ) diff --git a/tests/contracts/test_codex_smoke_storage.py b/tests/contracts/test_codex_smoke_storage.py new file mode 100644 index 0000000000..075f90fc2b --- /dev/null +++ b/tests/contracts/test_codex_smoke_storage.py @@ -0,0 +1,58 @@ +"""Exercise the live smoke's SQL assertion without launching a harness.""" + +import sqlite3 +from contextlib import closing, nullcontext +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from free_claude_code.config.paths import FCC_DATABASE_FILENAME +from smoke.product import test_codex_modes_product_live as smoke + + +@pytest.mark.parametrize("database_exists", [False, True]) +def test_child_review_smoke_reads_canonical_database_without_creating_files( + tmp_path, monkeypatch, database_exists +): + home = tmp_path / "home/.fcc" + home.mkdir(parents=True) + (home / "code").mkdir() + database_path = home / FCC_DATABASE_FILENAME + if database_exists: + with closing(sqlite3.connect(database_path)) as connection, connection: + connection.execute("CREATE TABLE code_sessions(id, native_thread_id)") + connection.execute("INSERT INTO code_sessions VALUES ('session','parent')") + connection.execute("CREATE TABLE code_items(session_id,kind,raw)") + connection.execute( + "INSERT INTO code_items VALUES ('session','subagent_auto_review',?)", + ('{"threadId":"child"}',), + ) + (tmp_path / "codex-home").mkdir() + (tmp_path / "child-marker.txt").write_text("smoke") + monkeypatch.setattr(smoke, "_environment", lambda *_: ({}, set())) + monkeypatch.setattr( + smoke, + "_canned_provider", + lambda *_: nullcontext(("http://provider.invalid", {"reviews": 1})), + ) + monkeypatch.setattr( + smoke, + "SmokeServerDriver", + lambda *args, **kwargs: SimpleNamespace( + run=lambda: nullcontext(SimpleNamespace(base_url="http://server.invalid")) + ), + ) + + async def completed_review(*_): + return "session" + + monkeypatch.setattr(smoke, "_exercise_child_review", completed_review) + config = MagicMock(spec=smoke.SmokeConfig, timeout_s=1) + if database_exists: + smoke.test_codex_child_review_local_e2e(config, tmp_path) + else: + with pytest.raises(sqlite3.OperationalError, match="unable to open"): + smoke.test_codex_child_review_local_e2e(config, tmp_path) + assert not (home / "code/code.db").exists() + assert database_path.exists() == database_exists diff --git a/tests/runtime/fixtures/code_schema_v0.sql b/tests/runtime/fixtures/code_schema_v0.sql new file mode 100644 index 0000000000..c36e6a4531 --- /dev/null +++ b/tests/runtime/fixtures/code_schema_v0.sql @@ -0,0 +1,39 @@ +-- Frozen pre-version-1 Code schema, independent of production migrations. + +CREATE TABLE IF NOT EXISTS code_sessions( + id TEXT PRIMARY KEY NOT NULL, cwd TEXT NOT NULL, model TEXT NOT NULL, reasoning_effort TEXT, + harness TEXT NOT NULL CHECK(harness = 'codex'), title TEXT NOT NULL, title_search TEXT NOT NULL, + cwd_search TEXT NOT NULL, auto_title INTEGER NOT NULL CHECK(auto_title IN (0,1)), native_thread_id TEXT, + native_may_have_input INTEGER NOT NULL CHECK(native_may_have_input IN (0,1)), + revision INTEGER NOT NULL CHECK(revision > 0), status TEXT NOT NULL CHECK(status IN ('ready','deleting','delete_uncertain')), + error TEXT, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL +); +CREATE INDEX IF NOT EXISTS code_sessions_recent ON code_sessions(updated_at DESC, id DESC); +CREATE TABLE IF NOT EXISTS code_runs( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + ordinal INTEGER NOT NULL CHECK(ordinal > 0), text TEXT NOT NULL, model TEXT NOT NULL, reasoning_effort TEXT, + status TEXT NOT NULL CHECK(status IN ('preparing','running','stopping','completed','interrupted','failed')), + submission_started INTEGER NOT NULL CHECK(submission_started IN (0,1)), native_turn_id TEXT, + stop_requested INTEGER NOT NULL CHECK(stop_requested IN (0,1)), error TEXT, error_details TEXT NOT NULL, + created_at INTEGER NOT NULL, finished_at INTEGER, + PRIMARY KEY(session_id,id), UNIQUE(session_id,ordinal), UNIQUE(session_id,native_turn_id) +); +CREATE UNIQUE INDEX IF NOT EXISTS code_one_active_run ON code_runs(session_id) + WHERE status IN ('preparing','running','stopping'); +CREATE TABLE IF NOT EXISTS code_items( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + run_id TEXT NOT NULL, sequence INTEGER NOT NULL CHECK(sequence > 0), native_turn_id TEXT, native_item_id TEXT, + kind TEXT NOT NULL, title TEXT NOT NULL, text TEXT NOT NULL, detail TEXT NOT NULL, + complete INTEGER NOT NULL CHECK(complete IN (0,1)), raw TEXT NOT NULL, + PRIMARY KEY(session_id,id), FOREIGN KEY(session_id,run_id) REFERENCES code_runs(session_id,id) ON DELETE CASCADE, + UNIQUE(session_id,sequence), UNIQUE(session_id,native_turn_id,native_item_id) +); +CREATE INDEX IF NOT EXISTS code_items_run ON code_items(session_id,run_id,sequence); +CREATE TABLE IF NOT EXISTS code_deleted(id TEXT PRIMARY KEY NOT NULL); +CREATE TABLE IF NOT EXISTS code_prompts( + session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, id TEXT NOT NULL, + generation TEXT NOT NULL, request_id TEXT NOT NULL, native_turn_id TEXT, native_item_id TEXT, + kind TEXT NOT NULL, form TEXT NOT NULL, raw TEXT NOT NULL, + status TEXT NOT NULL CHECK(status IN ('pending','answering','resolved','expired')), response_id TEXT, error TEXT, + PRIMARY KEY(session_id,id), UNIQUE(session_id,generation,request_id), UNIQUE(session_id,response_id) +); \ No newline at end of file diff --git a/tests/runtime/test_sqlite_database.py b/tests/runtime/test_sqlite_database.py new file mode 100644 index 0000000000..6f10791fa3 --- /dev/null +++ b/tests/runtime/test_sqlite_database.py @@ -0,0 +1,472 @@ +import asyncio +import os +import sqlite3 +import subprocess +import sys +import threading +from contextlib import closing +from pathlib import Path + +import httpx +import pytest + +from free_claude_code.application.code_sessions.models import CodeUnavailableError +from free_claude_code.runtime import sqlite_database +from free_claude_code.runtime.code_sessions_sqlite import SQLiteCodeStore +from free_claude_code.runtime.sqlite_database import initialize_database + + +def historical_database(path: Path, version: int) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + schema = (Path(__file__).parent / "fixtures/code_schema_v0.sql").read_text() + with closing(sqlite3.connect(path)) as connection, connection: + connection.executescript(schema) + if version >= 1: + connection.execute("DROP TABLE code_prompts") + connection.execute( + "CREATE TABLE code_prompts(" + "session_id TEXT NOT NULL REFERENCES code_sessions(id) ON DELETE CASCADE, " + "id TEXT NOT NULL, generation TEXT NOT NULL, request_id TEXT NOT NULL, " + "native_turn_id TEXT, native_item_id TEXT, kind TEXT NOT NULL, " + "form TEXT NOT NULL, raw TEXT NOT NULL, " + "status TEXT NOT NULL CHECK(status IN ('pending','answering','resolved','expired')), " + "response_id TEXT, error TEXT, PRIMARY KEY(session_id,id), " + "UNIQUE(session_id,generation,request_id), UNIQUE(session_id,response_id), " + "FOREIGN KEY(session_id,id) REFERENCES code_items(session_id,id) ON DELETE CASCADE)" + ) + if version >= 2: + for table in ("code_sessions", "code_runs"): + connection.execute( + f"ALTER TABLE {table} ADD COLUMN mode TEXT NOT NULL DEFAULT 'config' " + "CHECK(mode IN ('config','ask','auto_review','full_access'))" + ) + connection.execute( + "ALTER TABLE code_sessions ADD COLUMN native_permission_defaults TEXT " + "CHECK(native_permission_defaults IS NULL OR " + "(json_valid(native_permission_defaults) AND json_type(native_permission_defaults) = 'object'))" + ) + if version >= 3: + connection.execute( + "ALTER TABLE code_sessions ADD COLUMN context_used_tokens INTEGER " + "CHECK(context_used_tokens IS NULL OR context_used_tokens >= 0)" + ) + connection.execute(f"PRAGMA user_version={version}") + connection.execute( + "INSERT INTO code_sessions (id,cwd,model,harness,title,title_search,cwd_search," + "auto_title,native_thread_id,native_may_have_input,revision,status,created_at,updated_at) " + "VALUES ('s','/work','provider/model','codex','Saved','saved','/work',0,'thread',1,7,'ready',100,200)" + ) + connection.execute( + "INSERT INTO code_runs (session_id,id,ordinal,text,model,status,submission_started," + "native_turn_id,stop_requested,error_details,created_at,finished_at) " + "VALUES ('s','r',1,'User input','provider/model','completed',1,'turn',0,'{}',100,150)" + ) + connection.execute( + "INSERT INTO code_items VALUES ('s','i','r',1,'turn','native-item'," + "'assistant','Title','Saved answer','Detail',1,'{\"payload\": true}')" + ) + if version >= 1: + connection.execute( + "INSERT INTO code_items VALUES ('s','p','r',2,NULL,NULL,'prompt','','','',1,'{}')" + ) + connection.execute( + "INSERT INTO code_prompts VALUES ('s','p','g','42','turn',NULL,'questions'," + "'{\"question\": \"Continue?\"}','{\"saved\": true}','resolved','response',NULL)" + ) + connection.execute("INSERT INTO code_deleted VALUES ('deleted-session')") + if version >= 2: + connection.execute( + "UPDATE code_sessions SET mode='full_access', native_permission_defaults='{\"original\": true}'" + ) + if version >= 3: + connection.execute("UPDATE code_sessions SET context_used_tokens=1234") + + +def snapshot(path: Path) -> dict[str, list[dict]]: + with closing(sqlite3.connect(path)) as connection: + connection.row_factory = sqlite3.Row + return { + table: [dict(row) for row in connection.execute(f"SELECT * FROM {table}")] + for table in ( + "code_sessions", + "code_runs", + "code_items", + "code_prompts", + "code_deleted", + ) + } + + +@pytest.mark.parametrize("version", [0, 1, 2, 3]) +def test_historical_database_moves_and_upgrades_without_losing_records( + tmp_path, version +): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, version) + before = snapshot(old) + for _ in range(2): + initialize_database(target, old) + assert not old.exists() + after = snapshot(target) + for table, rows in before.items(): + for previous, saved in zip( + rows, after[table], strict=table != "code_items" or version != 0 + ): + assert {key: saved[key] for key in previous} == previous + assert len(after["code_items"]) == 2 + assert after["code_items"][-1]["kind"] == "prompt" + with closing(sqlite3.connect(target)) as connection: + assert connection.execute("PRAGMA user_version").fetchone() == (3,) + assert connection.execute("PRAGMA journal_mode").fetchone() == ("wal",) + assert connection.execute("PRAGMA foreign_key_check").fetchall() == [] + + +def test_fresh_database_uses_only_destination_and_is_repeatable(tmp_path): + target, old = tmp_path / "fcc.db", tmp_path / "code/code.db" + initialize_database(target, old) + initialize_database(target, old) + assert not old.exists() + assert all(not rows for rows in snapshot(target).values()) + + +def test_two_databases_are_preserved_without_choosing_a_winner(tmp_path): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 1) + historical_database(target, 3) + before = [path.read_bytes() for path in (old, target)] + with pytest.raises(sqlite3.DatabaseError, match="both"): + initialize_database(target, old) + assert [path.read_bytes() for path in (old, target)] == before + + +@pytest.mark.parametrize("suffix", ["-wal", "-shm", "-journal"]) +@pytest.mark.parametrize("location", ["fcc.db", "code/code.db"]) +def test_orphan_journals_are_not_treated_as_a_fresh_database( + tmp_path, suffix, location +): + orphan = tmp_path / (location + suffix) + orphan.parent.mkdir(parents=True, exist_ok=True) + orphan.write_bytes(b"preserve") + with pytest.raises(sqlite3.DatabaseError, match="journal"): + initialize_database(tmp_path / "fcc.db", tmp_path / "code/code.db") + assert orphan.read_bytes() == b"preserve" + assert not (tmp_path / "fcc.db").exists() + + +@pytest.mark.parametrize("version", [-1, 4]) +def test_unsupported_version_is_not_moved_or_mutated(tmp_path, version): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 3) + with closing(sqlite3.connect(old)) as connection: + connection.execute(f"PRAGMA user_version={version}") + before = old.read_bytes() + with pytest.raises(sqlite3.DatabaseError, match="version"): + initialize_database(target, old) + assert old.read_bytes() == before + assert not target.exists() + + +@pytest.mark.parametrize( + "statement", ["CREATE TABLE unrelated(id)", "CREATE TABLE code_sessions(id)"] +) +def test_unrecognized_schema_is_not_blessed_or_moved(tmp_path, statement): + old, target = tmp_path / "code.db", tmp_path / "fcc.db" + with closing(sqlite3.connect(old)) as connection: + connection.execute(statement) + with pytest.raises(sqlite3.DatabaseError, match="schema"): + initialize_database(target, old) + assert old.exists() and not target.exists() + + +def test_committed_wal_survives_unclean_exit_and_relocation(tmp_path): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 3) + subprocess.run( + [ + sys.executable, + "-c", + """ +import os, sqlite3, sys +c = sqlite3.connect(sys.argv[1]) +c.execute('PRAGMA journal_mode=WAL') +c.execute("UPDATE code_items SET text='Committed in WAL' WHERE id='i'") +c.commit() +os._exit(0) +""", + str(old), + ], + check=True, + timeout=10, + ) + assert Path(f"{old}-wal").stat().st_size > 0 + initialize_database(target, old) + assert snapshot(target)["code_items"][0]["text"] == "Committed in WAL" + assert not any(old.parent.glob("code.db*")) + + +def test_failed_rename_preserves_source_and_can_retry(tmp_path, monkeypatch): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 3) + before = snapshot(old) + with monkeypatch.context() as patch: + + def fail_rename(self, target): + raise PermissionError("move blocked") + + patch.setattr(Path, "rename", fail_rename) + with pytest.raises(PermissionError): + initialize_database(target, old) + assert old.exists() and not target.exists() + assert snapshot(old) == before + initialize_database(target, old) + assert snapshot(target) == before + + +@pytest.mark.parametrize("failing_version", [1, 2]) +def test_migration_failure_after_move_rolls_back_and_retries_at_destination( + tmp_path, monkeypatch, failing_version +): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 0) + migrations = sqlite_database.MIGRATIONS + original = migrations[failing_version - 1][1] + + def fail_upgrade(connection): + original(connection) + connection.execute("CREATE TABLE must_rollback(id)") + raise sqlite3.DatabaseError("injected migration failure") + + with monkeypatch.context() as patch: + patch.setattr( + sqlite_database, + "MIGRATIONS", + tuple( + (version, fail_upgrade if version == failing_version else upgrade) + for version, upgrade in migrations + ), + ) + with pytest.raises(sqlite3.DatabaseError, match="injected"): + initialize_database(target, old) + assert target.exists() and not old.exists() + with closing(sqlite3.connect(target)) as connection: + assert connection.execute("PRAGMA user_version").fetchone() == ( + failing_version - 1, + ) + assert not connection.execute( + "SELECT name FROM sqlite_schema WHERE name='must_rollback'" + ).fetchall() + columns = { + row[1] for row in connection.execute("PRAGMA table_info(code_sessions)") + } + assert "mode" not in columns + assert ( + connection.execute("SELECT count(*) FROM code_items").fetchone()[0] + == failing_version + ) + initialize_database(target, old) + assert len(snapshot(target)["code_items"]) == 2 + + +def test_fresh_schema_failure_rolls_back_all_ddl(tmp_path, monkeypatch): + target = tmp_path / "fcc.db" + original = sqlite_database.MIGRATIONS[0][1] + + def fail_upgrade(connection): + original(connection) + raise sqlite3.DatabaseError("injected fresh failure") + + with monkeypatch.context() as patch: + patch.setattr(sqlite_database, "MIGRATIONS", ((1, fail_upgrade),)) + with pytest.raises(sqlite3.DatabaseError, match="injected"): + initialize_database(target) + with closing(sqlite3.connect(target)) as connection: + assert connection.execute("PRAGMA user_version").fetchone() == (0,) + assert not connection.execute("SELECT name FROM sqlite_schema").fetchall() + initialize_database(target) + assert all(not rows for rows in snapshot(target).values()) + + +def test_current_schema_does_not_execute_historical_migrations(tmp_path, monkeypatch): + target = tmp_path / "fcc.db" + historical_database(target, 3) + + def already_applied(connection): + pytest.fail("An applied migration was executed again") + + monkeypatch.setattr( + sqlite_database, + "MIGRATIONS", + tuple((version, already_applied) for version in (1, 2, 3)), + ) + initialize_database(target) + + +def test_busy_wal_is_not_renamed_and_retries_after_reader_closes(tmp_path, monkeypatch): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 3) + connect = sqlite_database._connect + + def no_wait(*args, **kwargs): + connection = connect(*args, **kwargs) + connection.execute("PRAGMA busy_timeout=0") + return connection + + monkeypatch.setattr(sqlite_database, "_connect", no_wait) + with ( + closing(sqlite3.connect(old)) as writer, + closing(sqlite3.connect(old)) as reader, + ): + writer.execute("PRAGMA journal_mode=WAL") + reader.execute("BEGIN") + reader.execute("SELECT * FROM code_items").fetchall() + writer.execute("UPDATE code_items SET text='newest committed' WHERE id='i'") + writer.commit() + with pytest.raises(sqlite3.DatabaseError, match="checkpoint is busy"): + initialize_database(target, old) + assert old.exists() and not target.exists() + initialize_database(target, old) + assert snapshot(target)["code_items"][0]["text"] == "newest committed" + + +@pytest.mark.parametrize("redirect", ["directory", "database"]) +def test_redirected_legacy_storage_is_not_moved(tmp_path, redirect): + elsewhere = tmp_path / "elsewhere/code.db" + historical_database(elsewhere, 3) + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + try: + if redirect == "directory": + old.parent.symlink_to(elsewhere.parent, target_is_directory=True) + else: + old.parent.mkdir() + old.symlink_to(elsewhere) + except OSError: + pytest.skip("Symlink creation is unavailable") + before = elsewhere.read_bytes() + with pytest.raises(sqlite3.DatabaseError, match="redirected"): + initialize_database(target, old) + assert elsewhere.read_bytes() == before + assert not target.exists() + + +@pytest.mark.asyncio +async def test_restart_recovery_runs_after_current_schema_relocation(tmp_path): + old, target, lock = ( + tmp_path / "code/code.db", + tmp_path / "fcc.db", + tmp_path / "code/code.lock", + ) + historical_database(old, 3) + for iteration in range(2): + current = old if iteration == 0 else target + with closing(sqlite3.connect(current)) as connection, connection: + connection.execute( + "UPDATE code_runs SET status='running', finished_at=NULL" + ) + connection.execute("UPDATE code_prompts SET status='answering'") + store = SQLiteCodeStore(target, lock, legacy_database_path=old) + try: + await store.start() + run = await store.get_run("s", "r") + assert run is not None + assert run.status == "interrupted" and run.finished_at is not None + assert (await store.prompts("s"))[0].status == "expired" + finally: + await store.close() + + +@pytest.mark.asyncio +async def test_old_owner_blocks_move_and_new_owner_uses_same_lock(tmp_path): + old, target, lock = ( + tmp_path / "code/code.db", + tmp_path / "fcc.db", + tmp_path / "code/code.lock", + ) + historical_database(old, 3) + first = SQLiteCodeStore(old, lock) + second = SQLiteCodeStore(target, lock, legacy_database_path=old) + await first.start() + try: + with pytest.raises(CodeUnavailableError, match="another FCC"): + await second.start() + assert old.exists() and not target.exists() + finally: + await first.close() + await second.start() + try: + assert (await second.get_session("s")).title == "Saved" + assert lock.exists() and not old.exists() + finally: + await second.close() + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX permission contract") +def test_relocated_database_keeps_private_permissions(tmp_path): + old, target = tmp_path / "code/code.db", tmp_path / "fcc.db" + historical_database(old, 3) + old.chmod(0o600) + initialize_database(target, old) + assert target.stat().st_mode & 0o777 == 0o600 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("fail_migration", [False, True]) +async def test_production_startup_keeps_http_available_and_chat_cleanup_independent( + monkeypatch, fail_migration +): + from free_claude_code.config import paths + from free_claude_code.config.settings import Settings + from free_claude_code.core.interprocess_lock import InterprocessFileLock + from free_claude_code.runtime import bootstrap, code_sessions_sqlite + from free_claude_code.runtime.provider_manager import ProviderRuntimeManager + from free_claude_code.runtime.retired_chat import remove_retired_chat_history + + old, target = paths.legacy_code_database_path(), paths.fcc_database_path() + historical_database(old, 3) + chat = paths.config_dir_path() / "chat/chat.db" + chat.parent.mkdir() + chat.write_bytes(b"retired history") + chat_lock = InterprocessFileLock(chat.parent / "chat.lock") + assert chat_lock.acquire() + entered, release = threading.Event(), threading.Event() + + def gated_migration(*args): + entered.set() + assert release.wait(10) + if fail_migration: + raise sqlite3.DatabaseError("injected migration failure") + initialize_database(*args) + + monkeypatch.setattr(code_sessions_sqlite, "initialize_database", gated_migration) + monkeypatch.setattr(bootstrap, "configure_logging", lambda *args, **kwargs: None) + monkeypatch.setattr( + ProviderRuntimeManager, "start_model_list_refresh", lambda _: None + ) + app = bootstrap.build_asgi_app( + Settings().model_copy(update={"messaging_platform": "none"}) + ) + code = app.runtime._code_service + assert code is not None + try: + await asyncio.wait_for(app.runtime.start(), 3) + assert await asyncio.to_thread(entered.wait, 3) + assert code.storage_status()["state"] == "starting" + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://127.0.0.1" + ) as client: + assert (await client.get("/health")).status_code == 200 + release.set() + await asyncio.wait_for(asyncio.gather(*app.runtime._startup_tasks), 5) + assert code.storage_status()["state"] == ( + "failed" if fail_migration else "ready" + ) + assert (await client.get("/health")).status_code == 200 + assert chat.exists() + assert old.exists() == fail_migration + assert target.exists() != fail_migration + finally: + release.set() + chat_lock.release() + await app.runtime.close() + remove_retired_chat_history() + assert not chat.exists() + assert (old if fail_migration else target).exists() From f3ec4746edb8a1aeefca1153721a839f9523c506 Mon Sep 17 00:00:00 2001 From: Ali Khokhar <20476625+Alishahryar1@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:34:11 -0700 Subject: [PATCH 6/6] patch: Check FCC version before running updates (#1917) ## Why `fcc-update` runs the full installer even when FCC is already current. Check the installed version against PyPI first so unnecessary reinstallations are skipped. ## How Add a shared, short-lived version checker before the native launcher handoff. Current or newer installations exit successfully, an available update shows both versions and runs the installer, and a failed check stops with an error. The checker exits before installation so FCC can replace itself safely. Installer options do not bypass the version gate. The README directs users to the installer for same-version reinstallation or optional component changes. --- README.md | 4 + pyproject.toml | 3 +- scripts/update/fcc-update | 7 ++ scripts/update/fcc-update.cmd | 3 + src/free_claude_code/updater/__init__.py | 1 + src/free_claude_code/updater/check.py | 53 +++++++++++ tests/cli/test_entrypoints.py | 1 + tests/cli/test_update_check.py | 104 ++++++++++++++++++++++ tests/cli/test_update_wrapper.py | 69 ++++++++++++-- tests/contracts/test_import_boundaries.py | 1 + uv.lock | 4 +- 11 files changed, 238 insertions(+), 12 deletions(-) create mode 100644 src/free_claude_code/updater/__init__.py create mode 100644 src/free_claude_code/updater/check.py create mode 100644 tests/cli/test_update_check.py diff --git a/README.md b/README.md index 2dadeb09fa..9425d44554 100644 --- a/README.md +++ b/README.md @@ -462,6 +462,10 @@ Stop all running FCC commands, then run: fcc-update ``` +If FCC is already up to date, the command stops without running the installer. Otherwise, it shows the installed and available versions before updating. If the version check fails, retry the command. + +To reinstall FCC or change optional components while already up to date, run the [installer](#install) directly. + For local voice support, include `--voice-local` (macOS/Linux) or `-VoiceLocal` (Windows), plus your `--torch-backend` or `-TorchBackend` option if used. If your installation does not have `fcc-update` yet, run the [installer](#install) once to add it. diff --git a/pyproject.toml b/pyproject.toml index 095dd2e90a..97226cddf2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,7 @@ license = "AGPL-3.0-only" license-files = ["LICENSE"] requires-python = "==3.14.7" dependencies = [ + "packaging>=26.3", "anyio>=4.15.1", "fastapi[standard]>=0.141.1", "uvicorn>=0.53.0", @@ -41,6 +42,7 @@ dependencies = [ ] [project.scripts] +_fcc-update-check = "free_claude_code.updater.check:main" fcc-server = "free_claude_code.cli.entrypoints:serve" fcc-doctor = "free_claude_code.cli.entrypoints:doctor" fcc-claude = "free_claude_code.cli.launchers.claude:launch" @@ -89,7 +91,6 @@ explicit = true [dependency-groups] dev = [ - "packaging>=26.3", "pytest>=9.1.1", "pytest-asyncio>=1.4.0", "pytest-cov>=7.1.0", diff --git a/scripts/update/fcc-update b/scripts/update/fcc-update index 8fb65aa740..b770951995 100644 --- a/scripts/update/fcc-update +++ b/scripts/update/fcc-update @@ -1,4 +1,11 @@ #!/bin/sh set -eu +status=0 +"$(dirname "$0")/_fcc-update-check" || status=$? +case "$status" in + 0) exit 0 ;; + 10) ;; + *) exit "$status" ;; +esac installer=$(curl -fsSL "https://raw.githubusercontent.com/Alishahryar1/free-claude-code/main/scripts/install.sh") exec sh -c "$installer" sh "$@" diff --git a/scripts/update/fcc-update.cmd b/scripts/update/fcc-update.cmd index f2f6c8d527..85f06c4de3 100644 --- a/scripts/update/fcc-update.cmd +++ b/scripts/update/fcc-update.cmd @@ -1,4 +1,7 @@ @echo off +"%~dp0_fcc-update-check.exe" +if %errorlevel%==0 exit /b 0 +if not %errorlevel%==10 exit /b %errorlevel% ( powershell.exe -NoProfile -ExecutionPolicy Bypass -Command "$ErrorActionPreference = 'Stop'; & ([scriptblock]::Create((Invoke-RestMethod 'https://raw.githubusercontent.com/Alishahryar1/free-claude-code/main/scripts/install.ps1')))" %* call exit /b %%errorlevel%% diff --git a/src/free_claude_code/updater/__init__.py b/src/free_claude_code/updater/__init__.py new file mode 100644 index 0000000000..8dbc5c6caf --- /dev/null +++ b/src/free_claude_code/updater/__init__.py @@ -0,0 +1 @@ +"""Standalone update preflight, independent of server and harness runtime.""" diff --git a/src/free_claude_code/updater/check.py b/src/free_claude_code/updater/check.py new file mode 100644 index 0000000000..d2d6368da0 --- /dev/null +++ b/src/free_claude_code/updater/check.py @@ -0,0 +1,53 @@ +"""Standalone version preflight for the native update launchers.""" + +import sys +from importlib.metadata import PackageNotFoundError, version + +import httpx +from packaging.version import Version + +PYPI_URL = "https://pypi.org/pypi/free-claude-code/json" +UPDATE_AVAILABLE = 10 + + +def main() -> int: + """Return 10 to authorize installation, 0 to skip, or 1 on check failure.""" + try: + installed = Version(version("free-claude-code")) + if str(installed) == "0+unknown": + raise ValueError("installed FCC version is unknown") + with httpx.Client(timeout=10.0) as client: + response = client.get(PYPI_URL) + response.raise_for_status() + metadata = response.json() + latest = Version(metadata["info"]["version"]) + files = metadata["urls"] + if ( + latest.is_prerelease + or latest.is_devrelease + or not isinstance(files, list) + or not any( + isinstance(file, dict) and file.get("yanked") is False for file in files + ) + ): + raise ValueError("PyPI did not return an available stable FCC release") + except ( + PackageNotFoundError, + httpx.HTTPError, + ValueError, + KeyError, + TypeError, + ) as exc: + print(f"Could not check for FCC updates: {exc}. Please retry.", file=sys.stderr) + return 1 + + if installed == latest: + print(f"FCC {installed} is already up to date.") + return 0 + if installed > latest: + print( + f"FCC {installed} is newer than the published version {latest}. No update needed." + ) + return 0 + print(f"FCC update available: {installed} -> {latest}. Running the installer.") + return UPDATE_AVAILABLE diff --git a/tests/cli/test_entrypoints.py b/tests/cli/test_entrypoints.py index 20d7ab4354..856dbe6d12 100644 --- a/tests/cli/test_entrypoints.py +++ b/tests/cli/test_entrypoints.py @@ -38,6 +38,7 @@ def test_cli_scripts_are_registered() -> None: ) assert pyproject["project"]["scripts"] == { + "_fcc-update-check": "free_claude_code.updater.check:main", "fcc-server": "free_claude_code.cli.entrypoints:serve", "fcc-doctor": "free_claude_code.cli.entrypoints:doctor", "fcc-claude": "free_claude_code.cli.launchers.claude:launch", diff --git a/tests/cli/test_update_check.py b/tests/cli/test_update_check.py new file mode 100644 index 0000000000..edeb5c6274 --- /dev/null +++ b/tests/cli/test_update_check.py @@ -0,0 +1,104 @@ +"""The updater checks published versions before authorizing installation.""" + +from importlib.metadata import PackageNotFoundError + +import httpx +import pytest + +from free_claude_code.updater import check as update_check + + +def release(version="1.10.0", *, yanked=False): + return {"info": {"version": version}, "urls": [{"yanked": yanked}]} + + +@pytest.fixture +def check(monkeypatch): + original_client = httpx.Client + + def run(installed="1.9.0", payload=None, status=200, error=None, body=None): + monkeypatch.setattr(update_check, "version", lambda name: installed) + + def respond(request): + assert str(request.url) == update_check.PYPI_URL + if error is not None: + raise error + if body is not None: + return httpx.Response(status, content=body) + return httpx.Response(status, json=payload) + + monkeypatch.setattr( + update_check.httpx, + "Client", + lambda **kwargs: original_client( + transport=httpx.MockTransport(respond), **kwargs + ), + ) + return update_check.main() + + return run + + +@pytest.mark.parametrize( + ("installed", "expected"), + [ + ("1.9.0", 10), + ("1.10.0", 0), + ("1.11.0", 0), + ("1.10.0+local", 0), + ("1.10.0.dev1", 10), + ], +) +def test_version_comparison(check, capsys, installed, expected): + assert check(installed, release()) == expected + output = capsys.readouterr() + assert installed in output.out + assert not output.err + + +@pytest.mark.parametrize( + "payload", + [ + None, + {}, + {"info": []}, + release("invalid"), + release("2.0rc1"), + release("2.0.dev1"), + release(yanked=True), + {"info": {"version": "2.0"}, "urls": []}, + {"info": {"version": "2.0"}, "urls": [{}]}, + ], +) +def test_invalid_release_does_not_authorize_installation(check, capsys, payload): + assert check(payload=payload) == 1 + assert capsys.readouterr().err + + +@pytest.mark.parametrize("installed", ["invalid", "0+unknown"]) +def test_unknown_installed_version(check, installed): + assert check(installed, release()) == 1 + + +def test_missing_distribution(monkeypatch, capsys): + def missing(name): + raise PackageNotFoundError(name) + + monkeypatch.setattr(update_check, "version", missing) + assert update_check.main() == 1 + assert capsys.readouterr().err + + +def test_http_error(check, capsys): + assert check(payload=release(), status=503) == 1 + assert capsys.readouterr().err + + +def test_timeout(check, capsys): + assert check(error=httpx.ReadTimeout("timed out")) == 1 + assert capsys.readouterr().err + + +def test_malformed_json(check, capsys): + assert check(body="not JSON") == 1 + assert capsys.readouterr().err diff --git a/tests/cli/test_update_wrapper.py b/tests/cli/test_update_wrapper.py index 288de6ac55..dcb7ac7a3d 100644 --- a/tests/cli/test_update_wrapper.py +++ b/tests/cli/test_update_wrapper.py @@ -2,6 +2,7 @@ import contextlib import http.server +import json import os import shutil import subprocess @@ -18,14 +19,23 @@ @contextlib.contextmanager -def installer_server(body, status): +def installer_server(body, status, outcome): requests = [] class Handler(http.server.BaseHTTPRequestHandler): def do_GET(self): requests.append(self.path) payload = body.encode("utf-8") - self.send_response(status) + response_status = status + if self.path == "/metadata": + version = {"current": "1.0.0", "newer-installed": "0.9.0"}.get( + outcome, "2.0.0" + ) + payload = json.dumps( + {"info": {"version": version}, "urls": [{"yanked": False}]} + ).encode() + response_status = 503 if outcome == "check-error" else 200 + self.send_response(response_status) self.send_header("Content-Type", "text/plain; charset=utf-8") self.send_header("Content-Length", str(len(payload))) self.end_headers() @@ -51,7 +61,8 @@ def fixture_wheel(area, url): with zipfile.ZipFile(wheel, "w") as archive: archive.writestr( f"{dist}.dist-info/METADATA", - "Metadata-Version: 2.1\nName: free-claude-code\nVersion: 1.0.0\n", + "Metadata-Version: 2.1\nName: free-claude-code\nVersion: 1.0.0\n" + "Requires-Dist: httpx>=0.28.1\nRequires-Dist: packaging>=26.3\n", ) archive.writestr( f"{dist}.dist-info/WHEEL", @@ -62,6 +73,19 @@ def fixture_wheel(area, url): archive.writestr( f"{dist}.data/scripts/{name}", source.replace(INSTALL_URL, url) ) + archive.writestr( + f"{dist}.dist-info/entry_points.txt", + "[console_scripts]\n_fcc-update-check = free_claude_code.updater.check:main\n", + ) + for name in ("__init__.py", "updater/__init__.py", "updater/check.py"): + source = (ROOT / "src/free_claude_code" / name).read_text(encoding="utf-8") + archive.writestr( + f"free_claude_code/{name}", + source.replace( + "https://pypi.org/pypi/free-claude-code/json", + url.removesuffix("/install") + "/metadata", + ), + ) archive.writestr( f"{dist}.dist-info/RECORD", "".join(f"{name},,\n" for name in archive.namelist()) @@ -70,7 +94,18 @@ def fixture_wheel(area, url): return wheel -@pytest.mark.parametrize("outcome", ["success", "exit", "error", "download-error"]) +@pytest.mark.parametrize( + "outcome", + [ + "success", + "exit", + "error", + "download-error", + "current", + "newer-installed", + "check-error", + ], +) def test_installed_update_delegates_and_survives_replacement(tmp_path, outcome): uv = shutil.which("uv") if uv is None: @@ -87,12 +122,18 @@ def test_installed_update_delegates_and_survives_replacement(tmp_path, outcome): FCC_TEST_LAUNCHER=str( area / "bin" / ("fcc-update.cmd" if os.name == "nt" else "fcc-update") ), + FCC_TEST_CHECKER=str( + area + / "bin" + / ("_fcc-update-check.exe" if os.name == "nt" else "_fcc-update-check") + ), ) if os.name == "nt": body = """param([switch] $VoiceLocal, [string] $TorchBackend) $answer = Read-Host 'Continue' Write-Output "installer:$VoiceLocal|$TorchBackend|$answer" [IO.File]::WriteAllText($env:FCC_TEST_LAUNCHER, 'exit /b 99') +[IO.File]::WriteAllText($env:FCC_TEST_CHECKER, 'replaced after checker exited') """ arguments = "-VoiceLocal -TorchBackend cu130" expected = "installer:True|cu130|yes" @@ -101,11 +142,12 @@ def test_installed_update_delegates_and_survives_replacement(tmp_path, outcome): "exit": "exit 23", "error": "throw 'installer failed'", "download-error": "", - }[outcome] + }.get(outcome, "") else: body = """read -r answer printf 'installer:%s|%s|%s|%s\\n' "$1" "$2" "$3" "$answer" printf 'exit 99\\n' > "$FCC_TEST_LAUNCHER" +printf 'replaced after checker exited' > "$FCC_TEST_CHECKER" """ arguments = ["--voice-local", "--torch-backend", "cu130"] expected = "installer:--voice-local|--torch-backend|cu130|yes" @@ -114,8 +156,10 @@ def test_installed_update_delegates_and_survives_replacement(tmp_path, outcome): "exit": "exit 23", "error": "exit 1", "download-error": "", - }[outcome] - with installer_server(body, 503 if outcome == "download-error" else 200) as ( + }.get(outcome, "") + with installer_server( + body, 503 if outcome == "download-error" else 200, outcome + ) as ( url, requests, ): @@ -149,8 +193,14 @@ def test_installed_update_delegates_and_survives_replacement(tmp_path, outcome): text=True, timeout=30, ) - assert requests == ["/install.ps1" if os.name == "nt" else "/install.sh"] - if outcome == "download-error": + skipped = outcome in {"current", "newer-installed", "check-error"} + assert requests == ["/metadata"] + ( + [] if skipped else ["/install.ps1" if os.name == "nt" else "/install.sh"] + ), result.stdout + result.stderr + if skipped: + assert result.returncode == (1 if outcome == "check-error" else 0) + assert "installer:" not in result.stdout + elif outcome == "download-error": assert result.returncode != 0 assert "installer:" not in result.stdout else: @@ -166,3 +216,4 @@ def test_installed_update_delegates_and_survives_replacement(tmp_path, outcome): ) assert result.returncode == 0, result.stderr assert not any((area / "bin" / name).exists() for name in SCRIPTS) + assert not any((area / "bin").glob("_fcc-update-check*")) diff --git a/tests/contracts/test_import_boundaries.py b/tests/contracts/test_import_boundaries.py index a701352a3e..3add3b4419 100644 --- a/tests/contracts/test_import_boundaries.py +++ b/tests/contracts/test_import_boundaries.py @@ -11,6 +11,7 @@ _PACKAGE_NAME = "free_claude_code" ALLOWED_PACKAGE_DEPENDENCIES: dict[str, set[str]] = { + "updater": set(), "config": {"core"}, "core": set(), "application": {"config", "core"}, diff --git a/uv.lock b/uv.lock index 57d1cd296b..7ccc1a0460 100644 --- a/uv.lock +++ b/uv.lock @@ -617,6 +617,7 @@ dependencies = [ { name = "markdown-it-py" }, { name = "nvidia-riva-client" }, { name = "openai" }, + { name = "packaging" }, { name = "pillow", marker = "sys_platform == 'darwin' or sys_platform == 'win32'" }, { name = "pydantic" }, { name = "pyperclip" }, @@ -640,7 +641,6 @@ voice-local = [ [package.dev-dependencies] dev = [ - { name = "packaging" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, @@ -671,6 +671,7 @@ requires-dist = [ { name = "markdown-it-py", specifier = ">=4.2.0" }, { name = "nvidia-riva-client", specifier = ">=2.26.0" }, { name = "openai", specifier = ">=3.18.0" }, + { name = "packaging", specifier = ">=26.3" }, { name = "pillow", marker = "sys_platform == 'darwin' or sys_platform == 'win32'", specifier = ">=12.0.0" }, { name = "pydantic", specifier = ">=2.13.5" }, { name = "pyperclip", specifier = ">=1.11.0" }, @@ -689,7 +690,6 @@ provides-extras = ["voice-local"] [package.metadata.requires-dev] dev = [ - { name = "packaging", specifier = ">=26.3" }, { name = "pytest", specifier = ">=9.1.1" }, { name = "pytest-asyncio", specifier = ">=1.4.0" }, { name = "pytest-cov", specifier = ">=7.1.0" },