From abb61990dc41a24f0a52b515cc0ce2d13f3fb104 Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Thu, 9 Jul 2026 09:33:47 +0200 Subject: [PATCH 1/7] feat(tui): wire LLM tier settings and flag config drift on connected tools Fix the Settings screen writing dead LLM_MODEL_* keys nothing reads: the runtime keys now come from services.llm_tiers.LLM_TIER_KEYS (LLM_HIGH/ MEDIUM/LOW) as the single source of truth, with friendly labels and a save-time purge of the legacy names. Connecting bakes the whole env block into each AI tool's own config, so a later Settings edit does not reach an already-connected tool. Record a fingerprint of the block baked into each client at connect time (new client_configs section in ~/.specflow/config.json) and surface drift: - ClientSetupScreen renders a connected-but-stale client in orange with a reconnect affordance; pressing enter re-bakes and clears the flag. - SettingsScreen warns and routes to the connect screen when a runtime change leaves connected tools on stale config. - 'm' on the connect screen opens tier settings before connecting. Probe deliberately never writes the fingerprint (it confirms presence, not env); a dropped connection forgets it. Correct the module docstring that claimed edits propagate to every client. --- mcp_server/tests/test_tui_app.py | 156 +++++++++++++++++++++++ mcp_server/tests/test_tui_config.py | 36 ++++++ mcp_server/tests/test_tui_mcp_clients.py | 145 +++++++++++++++++++++ mcp_server/tui/app.py | 122 +++++++++++++++--- mcp_server/tui/config.py | 27 +++- mcp_server/tui/mcp_clients.py | 119 ++++++++++++++++- 6 files changed, 579 insertions(+), 26 deletions(-) diff --git a/mcp_server/tests/test_tui_app.py b/mcp_server/tests/test_tui_app.py index 4962b6e..3bde117 100644 --- a/mcp_server/tests/test_tui_app.py +++ b/mcp_server/tests/test_tui_app.py @@ -1737,3 +1737,159 @@ async def test_verify_choice_screen_list_selects_with_keyboard(self, tmp_path): await pilot.pause() assert yes_result == [True] assert no_result == [False] + + +class TestConfigDriftReconnect: + """Config-drift: connect records a fingerprint; the setup screen flags a + connected client whose baked config no longer matches; Settings routes there.""" + + def test_row_label_orange_when_connected_but_config_drifted(self): + from rich.text import Text + + screen = tui_app.ClientSetupScreen() + screen._current_fp = "new" + screen._saved_fp = {"cursor": "old"} + row = mc.ClientRow(mc.CURSOR, installed=True, saved=mc.ClientStatus.ADDED_UNVERIFIED) + label = screen._row_label(row, mc.ClientStatus.ADDED_UNVERIFIED) + assert isinstance(label, Text) + assert mc.STALE_LABEL in label.plain + assert any("orange" in str(s.style) for s in label.spans) + + def test_row_label_not_orange_when_fingerprint_matches(self): + screen = tui_app.ClientSetupScreen() + screen._current_fp = "same" + screen._saved_fp = {"cursor": "same"} + row = mc.ClientRow(mc.CURSOR, installed=True, saved=mc.ClientStatus.ADDED_UNVERIFIED) + label = screen._row_label(row, mc.ClientStatus.ADDED_UNVERIFIED) + assert mc.STALE_LABEL not in label.plain + + @pytest.mark.asyncio + async def test_connect_flow_records_config_fingerprint(self, tmp_path): + app, (a, b, c) = _make_app(tmp_path) + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch.object( + tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock() + ), patch("pathlib.Path.home", return_value=tmp_path), patch( + "tui.app.mcp_clients.load_server_block", return_value=_BLOCK + ), patch( + "tui.app.local_env.run_command", + new=AsyncMock(return_value=local_env.CommandResult(0, "", False)), + ): + async with app.run_test() as pilot: + await pilot.pause() + screen = await _push_client_screen(app) + await pilot.pause() + with patch.object(app, "push_screen_wait", new=AsyncMock(return_value=None)): + await screen._connect_flow(mc.CURSOR) + # The block just baked in is recorded as cursor's drift baseline. + assert mc.saved_fingerprints(home=tmp_path).get("cursor") == mc.config_fingerprint(_BLOCK) + + @pytest.mark.asyncio + async def test_probe_present_does_not_touch_fingerprint(self, tmp_path): + # A present read-back confirms only presence, not env — the recorded + # fingerprint must be left as-is so drift can still surface. + with patch("pathlib.Path.home", return_value=tmp_path): + mc.save_status("claude_code", mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint("claude_code", "old-fp", home=tmp_path) + rows = [mc.ClientRow(mc.CLAUDE_CODE, installed=True, saved=mc.ClientStatus.VERIFIED)] + app, (a, b, c) = _make_app(tmp_path) + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch( + "tui.mcp_clients.client_rows", return_value=rows + ), patch("pathlib.Path.home", return_value=tmp_path), patch( + "tui.app.local_env.run_command", + new=AsyncMock(return_value=local_env.CommandResult(0, "specflow: uvx ✔", False)), + ): + async with app.run_test() as pilot: + await pilot.pause() + await _push_client_screen(app) + await pilot.pause() + await pilot.pause() # let the probe worker run + assert mc.saved_fingerprints(home=tmp_path).get("claude_code") == "old-fp" + + @pytest.mark.asyncio + async def test_probe_absent_forgets_fingerprint(self, tmp_path): + with patch("pathlib.Path.home", return_value=tmp_path): + mc.save_status("claude_code", mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint("claude_code", "old-fp", home=tmp_path) + rows = [mc.ClientRow(mc.CLAUDE_CODE, installed=True, saved=mc.ClientStatus.VERIFIED)] + app, (a, b, c) = _make_app(tmp_path) + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch( + "tui.mcp_clients.client_rows", return_value=rows + ), patch("pathlib.Path.home", return_value=tmp_path), patch( + "tui.app.local_env.run_command", + new=AsyncMock(return_value=local_env.CommandResult(1, "No such server", False)), + ): + async with app.run_test() as pilot: + await pilot.pause() + await _push_client_screen(app) + await pilot.pause() + await pilot.pause() + assert "claude_code" not in mc.saved_fingerprints(home=tmp_path) + + @staticmethod + def _write_block(tmp_path, workspace_count): + from tui import config as tui_config + + path = tui_config.config_path(tmp_path) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps( + { + "mcpServers": { + "specflow": { + "command": "uvx", + "args": ["--from", "x", "specflow-mcp"], + "env": {"WORKSPACE_COUNT": workspace_count}, + } + } + } + ) + ) + + @staticmethod + def _land_on_sessions(): + a, b, c = _gate_ready() + return ( + a, + b, + c, + patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), + patch.object(tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock()), + patch("tui.app.mcp_clients.is_any_client_connected", return_value=True), + ) + + @pytest.mark.asyncio + async def test_settings_save_routes_to_connect_screen_on_drift(self, tmp_path): + # A connected client baked in the current block; editing a runtime field + # makes it stale, so save warns and routes to the connect screen. + self._write_block(tmp_path, "3") + with patch("pathlib.Path.home", return_value=tmp_path): + baseline = mc.config_fingerprint(mc.load_server_block(tmp_path)) + mc.save_status("cursor", mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint("cursor", baseline, home=tmp_path) + a, b, c, d, e, f = self._land_on_sessions() + with a, b, c, d, e, f, patch("pathlib.Path.home", return_value=tmp_path): + app = tui_app.SpecFlowTUI(root=tmp_path, generation_id=None, poll_interval=999) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("s") + await pilot.pause() + app.screen.query_one("#field-WORKSPACE_COUNT", Input).value = "5" + await pilot.press("ctrl+s") + await pilot.pause() + assert isinstance(app.screen, tui_app.ClientSetupScreen) + + @pytest.mark.asyncio + async def test_settings_save_returns_to_sessions_when_no_connected_client(self, tmp_path): + # Same runtime edit, but nothing connected → no drift, plain pop to sessions. + self._write_block(tmp_path, "3") + a, b, c, d, e, f = self._land_on_sessions() + with a, b, c, d, e, f, patch("pathlib.Path.home", return_value=tmp_path): + app = tui_app.SpecFlowTUI(root=tmp_path, generation_id=None, poll_interval=999) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("s") + await pilot.pause() + app.screen.query_one("#field-WORKSPACE_COUNT", Input).value = "5" + await pilot.press("ctrl+s") + await pilot.pause() + assert isinstance(app.screen, tui_app.SessionsScreen) diff --git a/mcp_server/tests/test_tui_config.py b/mcp_server/tests/test_tui_config.py index 8081e39..4c41709 100644 --- a/mcp_server/tests/test_tui_config.py +++ b/mcp_server/tests/test_tui_config.py @@ -2,6 +2,7 @@ import json +from services.llm_tiers import LLM_TIER_KEYS from tui import config @@ -12,6 +13,28 @@ def _write_config(root, doc): return path +class TestEditableKeys: + def test_uses_the_real_tier_keys_not_the_dead_llm_model_names(self): + assert config.EDITABLE_KEYS == [ + "WORKSPACE_COUNT", + "LLM_HIGH", + "LLM_MEDIUM", + "LLM_LOW", + "USER_EMAIL", + "BACKEND_URL", + ] + assert not any(k.startswith("LLM_MODEL_") for k in config.EDITABLE_KEYS) + + def test_tier_slice_is_the_llm_tiers_ssot(self): + # The tier entries must BE the shared list the MCP server/backend read. + assert config.EDITABLE_KEYS[1:4] == list(LLM_TIER_KEYS) + + def test_labels_cover_every_tier_key(self): + for key in LLM_TIER_KEYS: + assert key in config.EDITABLE_LABELS + assert config.EDITABLE_LABELS[key] + + class TestLoadEnv: def test_reads_env_block(self, tmp_path): _write_config(tmp_path, {"mcpServers": {"specflow": {"env": {"USER_EMAIL": "a@b.c"}}}}) @@ -59,6 +82,19 @@ def test_clearing_a_key_removes_it(self, tmp_path): config.save_env(tmp_path, {"USER_EMAIL": ""}) assert "USER_EMAIL" not in config.load_env(tmp_path) + def test_purges_dead_legacy_tier_keys(self, tmp_path): + # A block written by the buggy build carries LLM_MODEL_* nothing reads; + # saving must sweep them so they never linger or skew a fingerprint. + _write_config( + tmp_path, + {"mcpServers": {"specflow": {"env": {"LLM_MODEL_HIGH": "old/model", "KEEP": "yes"}}}}, + ) + config.save_env(tmp_path, {"LLM_HIGH": "new/model"}) + env = config.load_env(tmp_path) + assert "LLM_MODEL_HIGH" not in env + assert env["LLM_HIGH"] == "new/model" + assert env["KEEP"] == "yes" # unrelated entries still preserved + class TestLangfuse: def test_secret_key_is_masked_but_public_and_host_are_not(self): diff --git a/mcp_server/tests/test_tui_mcp_clients.py b/mcp_server/tests/test_tui_mcp_clients.py index 4ca9e55..988eba2 100644 --- a/mcp_server/tests/test_tui_mcp_clients.py +++ b/mcp_server/tests/test_tui_mcp_clients.py @@ -235,6 +235,151 @@ def test_malformed_or_unknown_values_read_as_empty(self, tmp_path): assert mc.saved_statuses(home=tmp_path) == {} # unknown value dropped, no crash +class TestConfigFingerprint: + def test_equal_blocks_hash_equal(self): + assert mc.config_fingerprint(BLOCK) == mc.config_fingerprint(BLOCK) + + def test_env_key_order_does_not_matter(self): + a = mc.ServerBlock("uvx", ("x",), {"A": "1", "B": "2"}) + b = mc.ServerBlock("uvx", ("x",), {"B": "2", "A": "1"}) + assert mc.config_fingerprint(a) == mc.config_fingerprint(b) + + def test_changes_when_an_env_value_changes(self): + base = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "a/b"}) + changed = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "c/d"}) + assert mc.config_fingerprint(base) != mc.config_fingerprint(changed) + + def test_changes_when_command_or_args_change(self): + assert mc.config_fingerprint(BLOCK) != mc.config_fingerprint( + mc.ServerBlock("uv", BLOCK.args, BLOCK.env) + ) + assert mc.config_fingerprint(BLOCK) != mc.config_fingerprint( + mc.ServerBlock(BLOCK.command, BLOCK.args + ("--extra",), BLOCK.env) + ) + + +class TestFingerprintPersistence: + def test_empty_when_absent(self, tmp_path): + assert mc.saved_fingerprints(home=tmp_path) == {} + + def test_round_trip(self, tmp_path): + mc.save_fingerprint("claude_code", "abc", home=tmp_path) + mc.save_fingerprint("cursor", "def", home=tmp_path) + assert mc.saved_fingerprints(home=tmp_path) == {"claude_code": "abc", "cursor": "def"} + + def test_overwrites_prior_fingerprint(self, tmp_path): + mc.save_fingerprint("cursor", "old", home=tmp_path) + mc.save_fingerprint("cursor", "new", home=tmp_path) + assert mc.saved_fingerprints(home=tmp_path)["cursor"] == "new" + + def test_keys_stored_sorted(self, tmp_path): + mc.save_fingerprint("cursor", "1", home=tmp_path) + mc.save_fingerprint("claude_code", "2", home=tmp_path) + section = json.loads(mc.config_path(home=tmp_path).read_text())["client_configs"] + assert list(section) == sorted(section) + + def test_non_string_values_skipped(self, tmp_path): + path = mc.config_path(home=tmp_path) + path.parent.mkdir(parents=True) + path.write_text(json.dumps({"client_configs": {"cursor": 5, "claude_code": "ok"}})) + assert mc.saved_fingerprints(home=tmp_path) == {"claude_code": "ok"} + + def test_forget_removes_target_preserving_others(self, tmp_path): + mc.save_fingerprint("claude_code", "a", home=tmp_path) + mc.save_fingerprint("cursor", "b", home=tmp_path) + mc.forget_fingerprint("claude_code", home=tmp_path) + assert mc.saved_fingerprints(home=tmp_path) == {"cursor": "b"} + mc.forget_fingerprint("nope", home=tmp_path) # absent → no-op + assert mc.saved_fingerprints(home=tmp_path) == {"cursor": "b"} + + def test_fingerprint_and_status_sections_are_independent(self, tmp_path): + # The two sibling sections must not clobber each other. + mc.save_status("cursor", mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint("cursor", "fp", home=tmp_path) + assert mc.saved_statuses(home=tmp_path) == {"cursor": mc.ClientStatus.VERIFIED} + assert mc.saved_fingerprints(home=tmp_path) == {"cursor": "fp"} + # And writing a status after a fingerprint preserves the fingerprint. + mc.save_status("claude_code", mc.ClientStatus.CONNECTED, home=tmp_path) + assert mc.saved_fingerprints(home=tmp_path) == {"cursor": "fp"} + + def test_save_preserves_unrelated_top_level_keys(self, tmp_path): + path = mc.config_path(home=tmp_path) + path.parent.mkdir(parents=True) + path.write_text(json.dumps({"theme": "dark", "clients": {"cursor": "verified"}})) + mc.save_fingerprint("cursor", "fp", home=tmp_path) + data = json.loads(path.read_text()) + assert data["theme"] == "dark" + assert data["clients"] == {"cursor": "verified"} + assert data["client_configs"] == {"cursor": "fp"} + + +class TestIsStale: + def test_missing_baseline_is_not_stale(self): + assert mc.is_stale(None, "x") is False + + def test_matching_is_not_stale(self): + assert mc.is_stale("x", "x") is False + + def test_differing_is_stale(self): + assert mc.is_stale("x", "y") is True + + +class TestStaleConnectedIds: + def test_connected_with_differing_fingerprint_is_stale(self, tmp_path): + mc.save_status("cursor", mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint("cursor", "old", home=tmp_path) + assert mc.stale_connected_ids("new", home=tmp_path) == ["cursor"] + + def test_connected_with_matching_fingerprint_is_not_stale(self, tmp_path): + mc.save_status("cursor", mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint("cursor", "same", home=tmp_path) + assert mc.stale_connected_ids("same", home=tmp_path) == [] + + def test_legacy_connection_without_fingerprint_is_not_stale(self, tmp_path): + mc.save_status("cursor", mc.ClientStatus.VERIFIED, home=tmp_path) # no fingerprint recorded + assert mc.stale_connected_ids("anything", home=tmp_path) == [] + + def test_failed_client_is_never_stale(self, tmp_path): + mc.save_status("cursor", mc.ClientStatus.FAILED, home=tmp_path) + mc.save_fingerprint("cursor", "old", home=tmp_path) + assert mc.stale_connected_ids("new", home=tmp_path) == [] + + def test_result_is_sorted(self, tmp_path): + for cid in ("cursor", "claude_code", "gemini"): + mc.save_status(cid, mc.ClientStatus.VERIFIED, home=tmp_path) + mc.save_fingerprint(cid, "old", home=tmp_path) + assert mc.stale_connected_ids("new", home=tmp_path) == ["claude_code", "cursor", "gemini"] + + +class TestRowOverlay: + def test_stale_connected_shows_orange_reconnect(self): + assert mc.row_label(mc.ClientStatus.VERIFIED, stale=True) == mc.STALE_LABEL + assert mc.row_style(mc.ClientStatus.VERIFIED, stale=True) == mc.STALE_STYLE + + def test_non_stale_connected_renders_normally(self): + assert mc.row_label(mc.ClientStatus.VERIFIED) == mc.status_label(mc.ClientStatus.VERIFIED) + assert mc.row_style(mc.ClientStatus.VERIFIED) == mc.status_style(mc.ClientStatus.VERIFIED) + + def test_manual_never_stale_stays_dim(self): + assert mc.row_style(mc.ClientStatus.CONNECTED, stale=True, is_manual=True) == "dim" + assert mc.row_label(mc.ClientStatus.CONNECTED, stale=True, is_manual=True) == ( + mc.status_label(mc.ClientStatus.CONNECTED) + ) + + def test_failed_ignores_stale_flag(self): + assert mc.row_label(mc.ClientStatus.FAILED, stale=True) == ( + mc.status_label(mc.ClientStatus.FAILED) + ) + assert mc.row_style(mc.ClientStatus.FAILED, stale=True) == ( + mc.status_style(mc.ClientStatus.FAILED) + ) + + def test_not_configured_ignores_stale_flag(self): + assert mc.row_label(mc.ClientStatus.NOT_CONFIGURED, stale=True) == ( + mc.status_label(mc.ClientStatus.NOT_CONFIGURED) + ) + + class TestRenderCliHint: def test_lists_registry_clients_from_one_source(self): hint = mc.render_cli_hint("/proj/.specflow-local/mcp-config.json") diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index de695e7..f45ca87 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -67,6 +67,7 @@ from tui import actions, activity, mcp_clients, onboarding, render from tui.config import ( EDITABLE_KEYS, + EDITABLE_LABELS, ENV_SECRET_KEYS, LANGFUSE_KEYS, MASKED_KEYS, @@ -884,6 +885,7 @@ class ClientSetupScreen(_SpecFlowScreen): # keep them working but out of the footer. The footer carries navigation. Binding("d", "show_config", "raw config", show=False), Binding("v", "recheck", "re-scan", show=False), + Binding("m", "edit_tiers", "model tiers", show=False), Binding("s", "skip", "skip", show=False), Binding("escape", "skip", "return", show=True), ] @@ -892,6 +894,11 @@ def __init__(self) -> None: super().__init__() self._rows: list[mcp_clients.ClientRow] = [] self._status: dict[str, mcp_clients.ClientStatus] = {} + # Baked-config drift: the live block's fingerprint vs the one recorded + # for each client at connect time. None disables the overlay (unreadable + # block); recomputed each _populate so a Settings edit is reflected. + self._current_fp: str | None = None + self._saved_fp: dict[str, str] = {} def compose(self) -> ComposeResult: yield Header() @@ -899,7 +906,7 @@ def compose(self) -> ComposeResult: # navigation (esc return) so the two don't duplicate each other. yield Static( "[b]Connect SpecFlow to your AI tool[/b] " - "[dim]↑/↓ select · ↵ connect · d raw config · v re-scan[/dim]", + "[dim]↑/↓ select · ↵ connect · m model tiers · d raw config · v re-scan[/dim]", id="client-setup-title", ) yield ListView(id="client-list") @@ -916,6 +923,16 @@ def on_mount(self) -> None: async def _populate(self) -> None: self._rows = mcp_clients.client_rows() + # Recompute the drift baseline: the live block's fingerprint and each + # client's recorded one. A missing/malformed block disables the overlay + # (fail-safe) rather than crashing the screen. + try: + self._current_fp = mcp_clients.config_fingerprint( + mcp_clients.load_server_block(self.app.root) + ) + except (FileNotFoundError, KeyError, json.JSONDecodeError): + self._current_fp = None + self._saved_fp = mcp_clients.saved_fingerprints() self._status = { r.client.client_id: mcp_clients.initial_status( r.client, installed=r.installed, saved=r.saved @@ -971,26 +988,37 @@ async def _probe_verifiable(self) -> None: mcp_clients.save_status(client.client_id, mcp_clients.ClientStatus.VERIFIED) else: # Not registered now — resolve the "verifying…" placeholder to a - # plain "press ↵ to connect", and forget any stale saved connection. + # plain "press ↵ to connect", and forget any stale saved connection + # (status and its drift baseline) so a dropped client starts clean. if current in (mcp_clients.ClientStatus.VERIFIED, mcp_clients.ClientStatus.CONNECTED): mcp_clients.forget_status(client.client_id) + mcp_clients.forget_fingerprint(client.client_id) + self._saved_fp.pop(client.client_id, None) self._set_status(client.client_id, mcp_clients.ClientStatus.NOT_CONFIGURED) self._render_detail() + def _is_stale(self, client_id: str, status: mcp_clients.ClientStatus) -> bool: + """True if this client is connected but its baked config has drifted.""" + return ( + self._current_fp is not None + and status in mcp_clients._ACTED_STATUSES + and mcp_clients.is_stale(self._saved_fp.get(client_id), self._current_fp) + ) + def _row_label(self, row: mcp_clients.ClientRow, status: mcp_clients.ClientStatus) -> Text: - # The "Other / copy config" row can never be verified (no read-back), - # so it stays grey regardless of state — green is only ever "connected". + # The "Other / copy config" row can never be verified (no read-back), so + # it stays grey; a connected client whose config drifted goes orange. Both + # rules live in mcp_clients.row_label/row_style so the colour and the words + # can never disagree. Shape (●/○) still marks installed vs not. is_manual = row.client.strategy is mcp_clients.AddStrategy.MANUAL_COPY - status_style = "dim" if is_manual else mcp_clients.status_style(status) + stale = self._is_stale(row.client.client_id, status) + style = mcp_clients.row_style(status, stale=stale, is_manual=is_manual) line = Text() line.append(f"{row.client.icon} ", style="bold cyan") line.append(f"{row.client.name:<20} ", style="bold") - # The dot and the label share the status colour, so the indicator always - # matches the words: green only when connected, amber added, red failed, - # grey otherwise. Shape (●/○) still marks installed vs not. badge = "● " if row.installed else "○ " - line.append(badge, style=status_style) - line.append(mcp_clients.status_label(status), style=status_style) + line.append(badge, style=style) + line.append(mcp_clients.row_label(status, stale=stale, is_manual=is_manual), style=style) return line def _selected_client(self) -> mcp_clients.McpClient | None: @@ -1048,8 +1076,11 @@ def action_connect(self) -> None: self._show_config(client) return # An "added — confirm" client: pressing ↵ asks the user what they found - # when they inspected it, rather than blindly re-adding. - if self._status.get(cid) is mcp_clients.ClientStatus.ADDED_UNVERIFIED: + # when they inspected it, rather than blindly re-adding — unless its + # config has drifted, in which case ↵ means "reconnect" (re-bake), so we + # fall through to the connect flow to rewrite it with the current block. + status = self._status.get(cid) + if status is mcp_clients.ClientStatus.ADDED_UNVERIFIED and not self._is_stale(cid, status): self.run_worker(self._inspect_flow(client), exclusive=True) return row = self._row_by_id(cid) @@ -1061,6 +1092,17 @@ def action_connect(self) -> None: def action_recheck(self) -> None: self.call_later(self._rescan) + def action_edit_tiers(self) -> None: + # Let the user set LLM model tiers (and other runtime settings) *before* + # connecting, so tools bake the intended config rather than the defaults. + # Re-populate on return so any edit is reflected (and any resulting drift + # on already-connected clients shows immediately). + self.run_worker(self._edit_tiers_flow(), exclusive=True) + + async def _edit_tiers_flow(self) -> None: + await self.app.push_screen_wait(SettingsScreen()) + await self._populate() + async def _rescan(self) -> None: await self._populate() self.notify("Re-scanned installed clients.", severity="information") @@ -1108,14 +1150,22 @@ async def _connect_flow(self, client: mcp_clients.McpClient) -> None: else: status = await self._connect_cli(client, block, log) + connected = status in ( + mcp_clients.ClientStatus.VERIFIED, + mcp_clients.ClientStatus.ADDED_UNVERIFIED, + ) + # Record the block we just baked in as this client's drift baseline before + # rendering, so a reconnect immediately clears the stale overlay (rather + # than staying orange until the next populate re-reads the same block). + if connected: + fingerprint = mcp_clients.config_fingerprint(block) + mcp_clients.save_fingerprint(cid, fingerprint) + self._saved_fp[cid] = fingerprint self._set_status(cid, status) # Persist the real outcome — including "added (unverified)" — so next time # the screen shows actual state, never an assumed connection. mcp_clients.save_status(cid, status) - if status in ( - mcp_clients.ClientStatus.VERIFIED, - mcp_clients.ClientStatus.ADDED_UNVERIFIED, - ): + if connected: await self.app.push_screen_wait( MessageScreen(f"Connected · {client.name}", mcp_clients.success_body(client, status)) ) @@ -1606,7 +1656,9 @@ def compose(self) -> ComposeResult: yield Static("Runtime (mcp-config.json)", classes="settings-section") for key in EDITABLE_KEYS: with Horizontal(classes="settings-row"): - yield Label(f"{key:<22}", classes="settings-label") + # Show a friendly label but keep the input id on the real env + # key so save reads/writes exactly what the MCP server reads. + yield Label(f"{EDITABLE_LABELS.get(key, key):<22}", classes="settings-label") yield Input(value=str(env.get(key, "")), id=f"field-{key}") yield Static("Secrets (.env — requires backend restart)", classes="settings-section") yield from self._secret_rows(ENV_SECRET_KEYS, secrets) @@ -1621,6 +1673,17 @@ def compose(self) -> ComposeResult: def _secret_input(self, key: str) -> str: return self.query_one(f"#secret-{key}", Input).value.strip() + def _block_fingerprint(self) -> str | None: + """Fingerprint of the live server block, or None if it can't be read. + + Guarded so a first run (no config yet) or a malformed file disables the + drift warning rather than crashing the save. + """ + try: + return mcp_clients.config_fingerprint(mcp_clients.load_server_block(self.app.root)) + except (FileNotFoundError, KeyError, json.JSONDecodeError): + return None + def action_save(self) -> None: # Reject a partially-filled LangFuse set before writing anything. Uses the # effective post-save state (a blank masked key keeps its stored value), @@ -1636,6 +1699,9 @@ def action_save(self) -> None: self.notify(partial, severity="error") return + # Fingerprint the block before the write so we can tell whether the + # runtime config actually changed (and thus whether connected tools drift). + old_fp = self._block_fingerprint() new_env = { key: self.query_one(f"#field-{key}", Input).value.strip() for key in EDITABLE_KEYS } @@ -1652,6 +1718,13 @@ def action_save(self) -> None: if secret_updates: save_env_secrets(self.app.root, secret_updates) + # A runtime (mcp-config.json) change does NOT reach an already-connected + # AI tool — its config was baked in at connect time. Find the connected + # tools now on stale config so we can steer the user to reconnect them. + new_fp = self._block_fingerprint() + env_changed = old_fp is not None and new_fp is not None and old_fp != new_fp + stale = mcp_clients.stale_connected_ids(new_fp) if env_changed and new_fp else [] + # The backend reads .env only at startup, so a changed secret is not live # until it restarts. Flag it explicitly rather than letting the user # wonder why nothing changed (and never point them at the file by hand — @@ -1662,9 +1735,22 @@ def action_save(self) -> None: f"Saved to {path}. Requires restart of the backend to take effect.", severity="warning", ) - else: + elif not stale: self.notify(f"Saved settings to {path}", severity="information") + self.app.pop_screen() + if stale: + # Warn and route to the connect screen so the affected tools (shown + # orange there) can be reconnected. If we returned onto a connect + # screen (Settings was opened from it), it re-populates itself — don't + # stack a second one. + self.notify( + f"{len(stale)} connected AI tool(s) still use the old config — " + "reconnect them below.", + severity="warning", + ) + if not isinstance(self.app.screen, ClientSetupScreen): + self.app.push_screen(ClientSetupScreen()) def action_cancel(self) -> None: self.app.pop_screen() diff --git a/mcp_server/tui/config.py b/mcp_server/tui/config.py index f34c330..34a1850 100644 --- a/mcp_server/tui/config.py +++ b/mcp_server/tui/config.py @@ -15,17 +15,31 @@ from cli import _MCP_CONFIG_FILENAME, _load_mcp_config from services import local_env +from services.llm_tiers import LLM_TIER_KEYS -# Runtime settings keys, stored in mcp-config.json, in display order. +# Runtime settings keys, stored in mcp-config.json, in display order. The tier +# keys come from ``services.llm_tiers.LLM_TIER_KEYS`` — the single source of +# truth the MCP server and backend actually read (``LLM_HIGH/MEDIUM/LOW``) — so +# the Settings screen can never again drift onto names nothing consumes. EDITABLE_KEYS: list[str] = [ "WORKSPACE_COUNT", - "LLM_MODEL_HIGH", - "LLM_MODEL_MEDIUM", - "LLM_MODEL_LOW", + *LLM_TIER_KEYS, "USER_EMAIL", "BACKEND_URL", ] +# Friendly labels for the runtime keys (raw key shown when unmapped). Tier values +# may be a comma-separated model list (multi-workspace variance / round-robin). +EDITABLE_LABELS: dict[str, str] = { + "LLM_HIGH": "High tier model(s)", + "LLM_MEDIUM": "Medium tier model(s)", + "LLM_LOW": "Low tier model(s)", +} + +# Superseded tier key names an earlier build wrote into the env block; nothing +# reads them. Purged on save so they never linger or skew a config fingerprint. +_LEGACY_EDITABLE_KEYS: list[str] = ["LLM_MODEL_HIGH", "LLM_MODEL_MEDIUM", "LLM_MODEL_LOW"] + # Secret/identity keys, stored in .env (consumed by docker-compose / the backend # / the init script). Names match .env.quickstart.example so write_dotenv fills # the template entries in place rather than appending duplicates. @@ -105,8 +119,9 @@ def save_env(root: Path, env: dict[str, str]) -> Path: specflow = servers.setdefault("specflow", {}) existing_env = specflow.get("env") merged = existing_env if isinstance(existing_env, dict) else {} - # Replace only the editable keys; leave any other env entries untouched. - for key in EDITABLE_KEYS: + # Replace only the editable keys (and sweep away any dead legacy tier keys); + # leave every other env entry untouched. + for key in (*EDITABLE_KEYS, *_LEGACY_EDITABLE_KEYS): merged.pop(key, None) merged.update(cleaned) specflow["env"] = merged diff --git a/mcp_server/tui/mcp_clients.py b/mcp_server/tui/mcp_clients.py index a5f8bc8..5db4e63 100644 --- a/mcp_server/tui/mcp_clients.py +++ b/mcp_server/tui/mcp_clients.py @@ -8,14 +8,17 @@ live in exactly one place and cannot drift. The canonical input is always the *live* server block read from -``.specflow-local/mcp-config.json`` (``load_server_block``) so a later edit to -``USER_EMAIL`` / ``WORKSPACE_COUNT`` in Settings propagates to every client. +``.specflow-local/mcp-config.json`` (``load_server_block``). Connecting *bakes* +that block into each client's own config, so a later edit in Settings does NOT +reach an already-connected client until it is reconnected — drift the setup +screen surfaces via ``config_fingerprint`` / ``stale_connected_ids``. """ from __future__ import annotations import base64 import copy +import hashlib import json import shutil import urllib.parse @@ -433,6 +436,35 @@ def status_style(status: ClientStatus) -> str: return _STATUS_STYLES[status] +# A client that is genuinely connected but whose baked-in config no longer +# matches the live block is neither "connected" (green would hide the drift) nor +# "failed" (red is wrong — it still works, just on stale config). It gets its own +# amber/orange overlay so a glance says "reconnect me", not "done" or "broken". +STALE_STYLE = "orange3" +STALE_LABEL = "connected · old config (↵ to reconnect)" + + +def row_style(status: ClientStatus, *, stale: bool = False, is_manual: bool = False) -> str: + """Row colour with the staleness overlay applied. + + Manual copy has no baked config and can never be verified, so it stays dim. + The orange overlay only ever replaces a *connected* status; transient/failed + states render normally so drift never masks an in-flight or failed connect. + """ + if is_manual: + return "dim" + if stale and status in _ACTED_STATUSES: + return STALE_STYLE + return status_style(status) + + +def row_label(status: ClientStatus, *, stale: bool = False, is_manual: bool = False) -> str: + """Row label with the staleness overlay applied (see ``row_style``).""" + if stale and not is_manual and status in _ACTED_STATUSES: + return STALE_LABEL + return status_label(status) + + def initial_status( client: McpClient, *, installed: bool, saved: ClientStatus | None = None ) -> ClientStatus: @@ -610,6 +642,89 @@ def is_any_client_connected(*, home: Path | None = None) -> bool: return any(s in _ACTED_STATUSES for s in saved_statuses(home=home).values()) +# --------------------------------------------------------------------------- +# Config-drift detection (~/.specflow/config.json → "client_configs" section) +# +# Connecting bakes the *entire* env block into each client's own config (Claude +# add-json embeds it; Cursor's merge writes it to ~/.cursor/mcp.json). A later +# edit to mcp-config.json therefore does NOT reach an already-connected client — +# it must be reconnected. We record a fingerprint of the block baked into each +# client at connect time in a sibling section (the "clients" status schema is +# left untouched, so no migration), then flag drift when the live block differs. +# --------------------------------------------------------------------------- + + +def config_fingerprint(block: ServerBlock) -> str: + """Stable hash of the launch spec baked into a client at connect time. + + Order-independent for ``env`` (sorted keys) and covers command/args/env, so + the fingerprint changes iff what a client would launch changes — any runtime + setting (tiers, WORKSPACE_COUNT, USER_EMAIL, BACKEND_URL), not just LLM tiers. + """ + payload = json.dumps( + {"command": block.command, "args": list(block.args), "env": dict(block.env)}, + sort_keys=True, + separators=(",", ":"), + ) + return hashlib.sha256(payload.encode()).hexdigest() + + +def saved_fingerprints(*, home: Path | None = None) -> dict[str, str]: + """Per-client baked-config fingerprints (unknown/non-string values skipped).""" + section = _read_config(home).get("client_configs") or {} + return {cid: value for cid, value in section.items() if isinstance(value, str)} + + +def save_fingerprint(client_id: str, fingerprint: str, *, home: Path | None = None) -> None: + """Record the config fingerprint baked into ``client_id`` at connect time. + + Read-modify-write that preserves every other top-level config section (notably + the untouched ``clients`` status schema), mirroring ``save_status``. + """ + data = _read_config(home) + section = data.get("client_configs") + if not isinstance(section, dict): + section = {} + section[client_id] = fingerprint + data["client_configs"] = {cid: section[cid] for cid in sorted(section)} + _write_config(data, home) + + +def forget_fingerprint(client_id: str, *, home: Path | None = None) -> None: + """Drop ``client_id``'s baked-config fingerprint (e.g. it's no longer registered).""" + data = _read_config(home) + section = data.get("client_configs") + if isinstance(section, dict) and client_id in section: + del section[client_id] + data["client_configs"] = {cid: section[cid] for cid in sorted(section)} + _write_config(data, home) + + +def is_stale(stored_fingerprint: str | None, current_fingerprint: str) -> bool: + """True iff we have a baseline that no longer matches the live block. + + A missing baseline (a client connected before drift tracking existed, or one + reconciled by probe without a recorded fingerprint) is NOT stale — we never + nag without a real baseline; the next reconnect establishes one. + """ + return stored_fingerprint is not None and stored_fingerprint != current_fingerprint + + +def stale_connected_ids(current_fingerprint: str, *, home: Path | None = None) -> list[str]: + """Connected clients whose baked config no longer matches the live block. + + The single source of truth for both the Settings save-time warning and the + setup-screen row overlay: a client counts only if it is genuinely connected + (``_ACTED_STATUSES``) and has a recorded fingerprint that now differs. + """ + saved_fp = saved_fingerprints(home=home) + return sorted( + client_id + for client_id, status in saved_statuses(home=home).items() + if status in _ACTED_STATUSES and is_stale(saved_fp.get(client_id), current_fingerprint) + ) + + # --------------------------------------------------------------------------- # CLI registration hint — derived from the registry (replaces the hardcoded # ``cli._IDE_REGISTRATION_HINT`` so client instructions live in one place). From 5d68251be66b4b157e30582ef04570848266ea1d Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Thu, 9 Jul 2026 09:45:09 +0200 Subject: [PATCH 2/7] refactor(tui): show raw runtime key names in Settings Drop EDITABLE_LABELS and render the raw env keys (LLM_HIGH/MEDIUM/LOW, WORKSPACE_COUNT, ...) so the Settings screen matches what appears in mcp-config.json verbatim. --- mcp_server/tests/test_tui_config.py | 5 ----- mcp_server/tui/app.py | 5 +---- mcp_server/tui/config.py | 8 -------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/mcp_server/tests/test_tui_config.py b/mcp_server/tests/test_tui_config.py index 4c41709..b17862b 100644 --- a/mcp_server/tests/test_tui_config.py +++ b/mcp_server/tests/test_tui_config.py @@ -29,11 +29,6 @@ def test_tier_slice_is_the_llm_tiers_ssot(self): # The tier entries must BE the shared list the MCP server/backend read. assert config.EDITABLE_KEYS[1:4] == list(LLM_TIER_KEYS) - def test_labels_cover_every_tier_key(self): - for key in LLM_TIER_KEYS: - assert key in config.EDITABLE_LABELS - assert config.EDITABLE_LABELS[key] - class TestLoadEnv: def test_reads_env_block(self, tmp_path): diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index f45ca87..01cf278 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -67,7 +67,6 @@ from tui import actions, activity, mcp_clients, onboarding, render from tui.config import ( EDITABLE_KEYS, - EDITABLE_LABELS, ENV_SECRET_KEYS, LANGFUSE_KEYS, MASKED_KEYS, @@ -1656,9 +1655,7 @@ def compose(self) -> ComposeResult: yield Static("Runtime (mcp-config.json)", classes="settings-section") for key in EDITABLE_KEYS: with Horizontal(classes="settings-row"): - # Show a friendly label but keep the input id on the real env - # key so save reads/writes exactly what the MCP server reads. - yield Label(f"{EDITABLE_LABELS.get(key, key):<22}", classes="settings-label") + yield Label(f"{key:<22}", classes="settings-label") yield Input(value=str(env.get(key, "")), id=f"field-{key}") yield Static("Secrets (.env — requires backend restart)", classes="settings-section") yield from self._secret_rows(ENV_SECRET_KEYS, secrets) diff --git a/mcp_server/tui/config.py b/mcp_server/tui/config.py index 34a1850..5b0fdf7 100644 --- a/mcp_server/tui/config.py +++ b/mcp_server/tui/config.py @@ -28,14 +28,6 @@ "BACKEND_URL", ] -# Friendly labels for the runtime keys (raw key shown when unmapped). Tier values -# may be a comma-separated model list (multi-workspace variance / round-robin). -EDITABLE_LABELS: dict[str, str] = { - "LLM_HIGH": "High tier model(s)", - "LLM_MEDIUM": "Medium tier model(s)", - "LLM_LOW": "Low tier model(s)", -} - # Superseded tier key names an earlier build wrote into the env block; nothing # reads them. Purged on save so they never linger or skew a config fingerprint. _LEGACY_EDITABLE_KEYS: list[str] = ["LLM_MODEL_HIGH", "LLM_MODEL_MEDIUM", "LLM_MODEL_LOW"] From 669402ec8877e32113267e37a24d8cec1cfc1309 Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Thu, 9 Jul 2026 14:59:33 +0200 Subject: [PATCH 3/7] feat(tui): show model-tier note on the connect screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a persistent footer note to ClientSetupScreen listing the model tiers a connect would bake in (blank tier = backend default), the 'm' affordance to change them, and the standing caveat that a tier change reaches the sandbox only from the next run — a generation already in progress keeps its models. Makes the previously easy-to-miss 'm' binding discoverable. --- mcp_server/tests/test_tui_app.py | 28 ++++++++++++++++++++++++++++ mcp_server/tui/app.py | 31 ++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/mcp_server/tests/test_tui_app.py b/mcp_server/tests/test_tui_app.py index 3bde117..35daace 100644 --- a/mcp_server/tests/test_tui_app.py +++ b/mcp_server/tests/test_tui_app.py @@ -1763,6 +1763,34 @@ def test_row_label_not_orange_when_fingerprint_matches(self): label = screen._row_label(row, mc.ClientStatus.ADDED_UNVERIFIED) assert mc.STALE_LABEL not in label.plain + def test_tiers_note_shows_current_models_defaults_and_caveat(self): + screen = tui_app.ClientSetupScreen() + block = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "anthropic/opus", "WORKSPACE_COUNT": "3"}) + text = screen._tiers_note(block).plain + assert "high: anthropic/opus" in text + assert "medium: default" in text # an unset tier reads as the backend default + assert "press m to change" in text + assert "next run" in text + + def test_tiers_note_all_default_when_block_missing(self): + text = tui_app.ClientSetupScreen()._tiers_note(None).plain + assert "high: default" in text + assert "low: default" in text + + @pytest.mark.asyncio + async def test_m_opens_tier_settings(self, tmp_path): + app, (a, b, c) = _make_app(tmp_path) + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch.object( + tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock() + ): + async with app.run_test() as pilot: + await pilot.pause() + await _push_client_screen(app) + await pilot.pause() + await pilot.press("m") + await pilot.pause() + assert isinstance(app.screen, tui_app.SettingsScreen) + @pytest.mark.asyncio async def test_connect_flow_records_config_fingerprint(self, tmp_path): app, (a, b, c) = _make_app(tmp_path) diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index 01cf278..5db40d4 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -62,6 +62,7 @@ from cli import resolve_backend_config from services import local_env +from services.llm_tiers import LLM_TIER_KEYS from services.session import resolve_generation_id, set_project_root from services.specflow_backend import call_backend_endpoint_bytes from tui import actions, activity, mcp_clients, onboarding, render @@ -913,6 +914,9 @@ def compose(self) -> ComposeResult: log = RichLog(id="client-log", highlight=False, markup=False, wrap=True) log.display = False yield log + # Persistent footer note: the tiers a connect would bake in, how to change + # them, and the standing caveat that a change only reaches the next run. + yield Static(id="client-tiers") yield Footer() def on_mount(self) -> None: @@ -926,12 +930,13 @@ async def _populate(self) -> None: # client's recorded one. A missing/malformed block disables the overlay # (fail-safe) rather than crashing the screen. try: - self._current_fp = mcp_clients.config_fingerprint( - mcp_clients.load_server_block(self.app.root) - ) + block: mcp_clients.ServerBlock | None = mcp_clients.load_server_block(self.app.root) + self._current_fp = mcp_clients.config_fingerprint(block) except (FileNotFoundError, KeyError, json.JSONDecodeError): + block = None self._current_fp = None self._saved_fp = mcp_clients.saved_fingerprints() + self.query_one("#client-tiers", Static).update(self._tiers_note(block)) self._status = { r.client.client_id: mcp_clients.initial_status( r.client, installed=r.installed, saved=r.saved @@ -1020,6 +1025,26 @@ def _row_label(self, row: mcp_clients.ClientRow, status: mcp_clients.ClientStatu line.append(mcp_clients.row_label(status, stale=stale, is_manual=is_manual), style=style) return line + def _tiers_note(self, block: mcp_clients.ServerBlock | None) -> Text: + # Show the model tiers a connect would bake in (blank tier = backend + # default), the `m` affordance, and the standing caveat that a change + # reaches the harness sandbox only from the next run — an in-progress + # generation keeps the models it started with. + env = block.env if block is not None else {} + note = Text() + note.append("Model tiers ", style="bold") + for key in LLM_TIER_KEYS: + note.append(f"{key.removeprefix('LLM_').lower()}: ", style="dim") + note.append(env.get(key) or "default") + note.append(" ") + note.append("· press m to change\n", style="dim") + note.append( + "Tier changes take effect from the next run — a generation already " + "in progress keeps its current models.", + style="dim", + ) + return note + def _selected_client(self) -> mcp_clients.McpClient | None: listview = self.query_one("#client-list", ListView) item = listview.highlighted_child From a77e08e67abcd69756c7c1e3a31c4daedddd0c8f Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Thu, 9 Jul 2026 15:06:01 +0200 Subject: [PATCH 4/7] feat(tui): render model tiers as a bordered box with per-tier purpose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the plain one-line tiers note with a titled Panel on the connect screen: a tier/model/purpose grid (high → planning & KB init, medium → code generation, low → simple steps) plus the 'm' affordance and the next-run caveat. Blank tiers show as the backend default. --- mcp_server/tests/test_tui_app.py | 24 +++++++++----- mcp_server/tui/app.py | 55 +++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/mcp_server/tests/test_tui_app.py b/mcp_server/tests/test_tui_app.py index 35daace..6399b11 100644 --- a/mcp_server/tests/test_tui_app.py +++ b/mcp_server/tests/test_tui_app.py @@ -1763,19 +1763,27 @@ def test_row_label_not_orange_when_fingerprint_matches(self): label = screen._row_label(row, mc.ClientStatus.ADDED_UNVERIFIED) assert mc.STALE_LABEL not in label.plain - def test_tiers_note_shows_current_models_defaults_and_caveat(self): + @staticmethod + def _render(renderable) -> str: + console = Console(width=100, record=True) + console.print(renderable) + return console.export_text() + + def test_tiers_panel_shows_models_defaults_purposes_and_caveat(self): screen = tui_app.ClientSetupScreen() block = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "anthropic/opus", "WORKSPACE_COUNT": "3"}) - text = screen._tiers_note(block).plain - assert "high: anthropic/opus" in text - assert "medium: default" in text # an unset tier reads as the backend default + text = self._render(screen._tiers_panel(block)) + assert "Model tiers" in text # panel title + assert "anthropic/opus" in text # configured high-tier model + assert "default" in text # an unset tier reads as the backend default + assert "planning" in text # high-tier purpose + assert "code generation" in text # medium-tier purpose assert "press m to change" in text assert "next run" in text - def test_tiers_note_all_default_when_block_missing(self): - text = tui_app.ClientSetupScreen()._tiers_note(None).plain - assert "high: default" in text - assert "low: default" in text + def test_tiers_panel_all_default_when_block_missing(self): + text = self._render(tui_app.ClientSetupScreen()._tiers_panel(None)) + assert text.count("default") >= 3 # every tier falls back to default @pytest.mark.asyncio async def test_m_opens_tier_settings(self, tmp_path): diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index 5db40d4..8bfaf7a 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -914,8 +914,8 @@ def compose(self) -> ComposeResult: log = RichLog(id="client-log", highlight=False, markup=False, wrap=True) log.display = False yield log - # Persistent footer note: the tiers a connect would bake in, how to change - # them, and the standing caveat that a change only reaches the next run. + # Persistent box: the tiers a connect would bake in, what each drives, + # how to change them, and the caveat that a change only reaches the next run. yield Static(id="client-tiers") yield Footer() @@ -936,7 +936,7 @@ async def _populate(self) -> None: block = None self._current_fp = None self._saved_fp = mcp_clients.saved_fingerprints() - self.query_one("#client-tiers", Static).update(self._tiers_note(block)) + self.query_one("#client-tiers", Static).update(self._tiers_panel(block)) self._status = { r.client.client_id: mcp_clients.initial_status( r.client, installed=r.installed, saved=r.saved @@ -1025,25 +1025,41 @@ def _row_label(self, row: mcp_clients.ClientRow, status: mcp_clients.ClientStatu line.append(mcp_clients.row_label(status, stale=stale, is_manual=is_manual), style=style) return line - def _tiers_note(self, block: mcp_clients.ServerBlock | None) -> Text: - # Show the model tiers a connect would bake in (blank tier = backend - # default), the `m` affordance, and the standing caveat that a change - # reaches the harness sandbox only from the next run — an in-progress - # generation keeps the models it started with. + # What each tier drives (mirrors the backend WORKFLOW_TIER_MAP) so the user + # knows which model matters for which stage. A blank tier = backend default. + _TIER_PURPOSE: dict[str, str] = { + "LLM_HIGH": "planning & knowledge-base init", + "LLM_MEDIUM": "code generation", + "LLM_LOW": "simple steps (md→JSON, spec indexing)", + } + + def _tiers_panel(self, block: mcp_clients.ServerBlock | None) -> Panel: + # A bordered box showing the model tiers a connect would bake in, what + # each drives, the `m` affordance, and the standing caveat that a change + # reaches the sandbox only from the next run — an in-progress generation + # keeps the models it started with. env = block.env if block is not None else {} - note = Text() - note.append("Model tiers ", style="bold") + table = Table.grid(padding=(0, 2)) + table.add_column(style="bold cyan") # tier + table.add_column() # model(s) + table.add_column(style="dim") # purpose for key in LLM_TIER_KEYS: - note.append(f"{key.removeprefix('LLM_').lower()}: ", style="dim") - note.append(env.get(key) or "default") - note.append(" ") - note.append("· press m to change\n", style="dim") - note.append( - "Tier changes take effect from the next run — a generation already " - "in progress keeps its current models.", - style="dim", + tier = key.removeprefix("LLM_").lower() + value = env.get(key) + model = Text(value) if value else Text("default", style="italic dim") + table.add_row(tier, model, self._TIER_PURPOSE.get(key, "")) + footer = Text( + "press m to change · a change takes effect from the next run " + "(a generation already in progress keeps its models)", + style="dim italic", + ) + return Panel( + Group(table, Text(), footer), + title="Model tiers", + title_align="left", + border_style="cyan", + padding=(0, 1), ) - return note def _selected_client(self) -> mcp_clients.McpClient | None: listview = self.query_one("#client-list", ListView) @@ -1805,6 +1821,7 @@ class SpecFlowTUI(App): #client-setup-title { padding: 1 2; text-style: bold; } #client-detail { padding: 1 2; color: $text-muted; } #client-log { height: 1fr; border: round $primary; margin: 1 2; } + #client-tiers { height: auto; margin: 0 2 1 2; } .modal-panel { width: 64; height: auto; From 9052a7e8153580f5258af39d943757364de902a2 Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Thu, 9 Jul 2026 15:23:32 +0200 Subject: [PATCH 5/7] feat(tui): validate configured models against the provider catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface per-model validity in the TUI using the existing /api/v1/models/validate endpoint. Add request_model_validation_for(), which validates explicit LLM_* values from the local config block (the TUI edits mcp-config.json, not its own env, so the env-based path does not fit). Connect screen: a background worker validates on open and re-renders the tiers box with a per-tier glyph (✓ available / ✗ unsupported / • unchecked), tints each model, and lists invalid models with the backend's suggestion. Settings: each tier field carries a live ✓/✗ marker updated on mount and on submit. Both are best-effort — a missing backend or unreachable catalog leaves everything neutral, never red. --- mcp_server/services/validate_models.py | 39 ++++- mcp_server/tests/test_tui_app.py | 137 ++++++++++++++++++ mcp_server/tests/test_validate_models.py | 22 ++- mcp_server/tui/app.py | 173 +++++++++++++++++++++-- 4 files changed, 351 insertions(+), 20 deletions(-) diff --git a/mcp_server/services/validate_models.py b/mcp_server/services/validate_models.py index e863f42..b8b6506 100644 --- a/mcp_server/services/validate_models.py +++ b/mcp_server/services/validate_models.py @@ -18,7 +18,7 @@ from fastmcp import Context -from services.llm_tiers import apply_llm_tier_overrides +from services.llm_tiers import LLM_TIER_KEYS, apply_llm_tier_overrides from services.run_generation_precheck import RejectionCode from services.specflow_backend import post_form_data_to_backend @@ -27,20 +27,45 @@ VALIDATE_MODELS_ENDPOINT = "/api/v1/models/validate" -async def request_model_validation() -> dict[str, Any]: - """Call the backend validate-models endpoint with the configured LLM_* overrides. +async def _post_validation(form_data: dict[str, str]) -> dict[str, Any]: + """POST ``form_data`` to the validate-models endpoint and parse the response. - Returns the parsed ValidateModelsResponse dict. Raises on transport/parse errors - so callers can stay permissive (connect/run gate) on infrastructure failures. + The single call site for the endpoint so URL/timeout/parsing live in one place. """ - form_data: dict[str, str] = {} - apply_llm_tier_overrides(form_data) # injects LLM_HIGH/MEDIUM/LOW from env when set raw = await post_form_data_to_backend( VALIDATE_MODELS_ENDPOINT, form_data, timeout_seconds=30.0 ) return json.loads(raw) +async def request_model_validation() -> dict[str, Any]: + """Call the backend validate-models endpoint with the configured LLM_* overrides. + + Reads LLM_HIGH/MEDIUM/LOW from the MCP *process env*. Returns the parsed + ValidateModelsResponse dict. Raises on transport/parse errors so callers can + stay permissive (connect/run gate) on infrastructure failures. + """ + form_data: dict[str, str] = {} + apply_llm_tier_overrides(form_data) # injects LLM_HIGH/MEDIUM/LOW from env when set + return await _post_validation(form_data) + + +async def request_model_validation_for(tier_values: dict[str, str]) -> dict[str, Any]: + """Validate explicit LLM_* ``tier_values`` (e.g. from the local config block). + + Unlike ``request_model_validation`` (which reads the process env), this takes + the values directly — the TUI edits ``mcp-config.json`` rather than its own + environment, so it must forward what the file holds. Blank tiers are omitted + so the backend falls back to its own default for that tier. + """ + form_data = { + key: value.strip() + for key in LLM_TIER_KEYS + if (value := (tier_values.get(key) or "").strip()) + } + return await _post_validation(form_data) + + def invalid_models(response: dict[str, Any]) -> list[dict[str, Any]]: """Flatten the response into the list of confidently-invalid models (with tier).""" out: list[dict[str, Any]] = [] diff --git a/mcp_server/tests/test_tui_app.py b/mcp_server/tests/test_tui_app.py index 6399b11..ac52461 100644 --- a/mcp_server/tests/test_tui_app.py +++ b/mcp_server/tests/test_tui_app.py @@ -1929,3 +1929,140 @@ async def test_settings_save_returns_to_sessions_when_no_connected_client(self, await pilot.press("ctrl+s") await pilot.pause() assert isinstance(app.screen, tui_app.SessionsScreen) + + +class TestModelValidationUI: + """Model-tier validity feedback: pure glyph mapping, the validated tiers box, + and the live markers on the connect screen and in Settings.""" + + @staticmethod + def _render(renderable) -> str: + console = Console(width=100, record=True) + console.print(renderable) + return console.export_text() + + @staticmethod + def _response(tiers): + return { + "provider": "openrouter", + "catalog_available": True, + "all_valid": all(m["status"] == "valid" for t in tiers for m in t["models"]), + "has_blocking_tier": any(m["status"] == "invalid" for t in tiers for m in t["models"]), + "tiers": tiers, + } + + def test_model_mark_maps_statuses(self): + assert tui_app._model_mark("valid") == ("✓", "green") + assert tui_app._model_mark("invalid") == ("✗", "red") + assert tui_app._model_mark("unverified")[1] == "dim" + assert tui_app._model_mark(None) == tui_app._NEUTRAL_MARK + + def test_tier_mark_aggregates_worst_first(self): + assert tui_app._tier_mark([{"status": "valid"}, {"status": "invalid"}]) == ("✗", "red") + assert tui_app._tier_mark([{"status": "valid"}, {"status": "valid"}]) == ("✓", "green") + assert tui_app._tier_mark([{"status": "valid"}, {"status": "unverified"}]) == ( + tui_app._NEUTRAL_MARK + ) + assert tui_app._tier_mark([]) == tui_app._NEUTRAL_MARK + + def test_tier_marker_text_words(self): + assert tui_app._tier_marker_text([]).plain == "" + assert "unsupported" in tui_app._tier_marker_text([{"status": "invalid"}]).plain + assert "available" in tui_app._tier_marker_text([{"status": "valid"}]).plain + # Unverified-only: a glyph but no word we can't stand behind. + assert tui_app._tier_marker_text([{"status": "unverified"}]).plain == "•" + + def test_tiers_panel_with_validation_shows_marks_and_notes(self): + block = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "anthropic/bad"}) + resp = self._response( + [ + { + "tier": "LLM_HIGH", + "models": [ + {"configured": "anthropic/bad", "status": "invalid", + "suggestion": "anthropic/good"} + ], + } + ] + ) + text = self._render(tui_app.ClientSetupScreen()._tiers_panel(block, resp)) + assert "✗" in text + assert "not available on openrouter" in text + assert "did you mean" in text + assert "anthropic/good" in text + + @pytest.mark.asyncio + async def test_connect_screen_marks_invalid_model(self, tmp_path): + block = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "anthropic/bad"}) + resp = self._response( + [{"tier": "LLM_HIGH", "models": [{"configured": "anthropic/bad", "status": "invalid", + "suggestion": "anthropic/good"}]}] + ) + app, (a, b, c) = _make_app(tmp_path) + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch.object( + tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock() + ), patch("tui.app.mcp_clients.load_server_block", return_value=block), patch( + "tui.app.validate_models.request_model_validation_for", + new=AsyncMock(return_value=resp), + ): + async with app.run_test() as pilot: + await pilot.pause() + screen = await _push_client_screen(app) + await pilot.pause() + await pilot.pause() # let the validation worker run + re-render + # The worker fetched and stored the result (the box re-render runs on the + # same path); the box built from it shows the ✗ and the suggestion. + assert screen._validation == resp + text = self._render(screen._tiers_panel(block, screen._validation)) + assert "✗" in text + assert "anthropic/good" in text # suggestion surfaced + + @pytest.mark.asyncio + async def test_connect_screen_neutral_when_validation_fails(self, tmp_path): + # Backend unreachable → worker swallows the error, box stays free of ✓/✗. + block = mc.ServerBlock("uvx", ("x",), {"LLM_HIGH": "anthropic/x"}) + app, (a, b, c) = _make_app(tmp_path) + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch.object( + tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock() + ), patch("tui.app.mcp_clients.load_server_block", return_value=block), patch( + "tui.app.validate_models.request_model_validation_for", + new=AsyncMock(side_effect=RuntimeError("no backend")), + ): + async with app.run_test() as pilot: + await pilot.pause() + screen = await _push_client_screen(app) + await pilot.pause() + await pilot.pause() + # Worker swallowed the failure: no validation stored, box stays neutral. + assert screen._validation is None + text = self._render(screen._tiers_panel(block, screen._validation)) + assert "✗" not in text and "✓" not in text + + @pytest.mark.asyncio + async def test_settings_tier_markers_reflect_validation(self, tmp_path): + resp = self._response( + [ + {"tier": "LLM_HIGH", "models": [{"configured": "x/bad", "status": "invalid"}]}, + {"tier": "LLM_MEDIUM", "models": [{"configured": "x/ok", "status": "valid"}]}, + {"tier": "LLM_LOW", "models": [{"configured": "x/ok2", "status": "valid"}]}, + ] + ) + a, b, c = _gate_ready() + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch.object( + tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock() + ), patch("tui.app.mcp_clients.is_any_client_connected", return_value=True), patch( + "tui.app.validate_models.request_model_validation_for", new=AsyncMock(return_value=resp) + ): + app = tui_app.SpecFlowTUI(root=tmp_path, generation_id=None, poll_interval=999) + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("s") + await pilot.pause() + await pilot.pause() # on_mount validation worker runs + screen = app.screen + high = self._render(screen.query_one("#tierstatus-LLM_HIGH", tui_app.Static).render()) + medium = self._render( + screen.query_one("#tierstatus-LLM_MEDIUM", tui_app.Static).render() + ) + assert "✗" in high and "unsupported" in high + assert "✓" in medium and "available" in medium diff --git a/mcp_server/tests/test_validate_models.py b/mcp_server/tests/test_validate_models.py index 34a457f..96dd53c 100644 --- a/mcp_server/tests/test_validate_models.py +++ b/mcp_server/tests/test_validate_models.py @@ -1,6 +1,6 @@ """Tests for the MCP-side model validation service.""" import json -from unittest.mock import AsyncMock, patch +from unittest.mock import patch import pytest @@ -8,6 +8,7 @@ blocking_rejection, invalid_models, request_model_validation, + request_model_validation_for, ) @@ -47,6 +48,25 @@ async def fake_post(endpoint, form_data, timeout_seconds=30.0): assert result["all_valid"] is True +@pytest.mark.asyncio +async def test_request_model_validation_for_uses_explicit_values_and_omits_blanks(): + captured = {} + + async def fake_post(endpoint, form_data, timeout_seconds=30.0): + captured["endpoint"] = endpoint + captured["form_data"] = dict(form_data) + return json.dumps(_response()) + + with patch("services.validate_models.post_form_data_to_backend", new=fake_post): + result = await request_model_validation_for( + {"LLM_HIGH": "anthropic/opus", "LLM_MEDIUM": " ", "LLM_LOW": ""} + ) + + assert captured["endpoint"] == "/api/v1/models/validate" + assert captured["form_data"] == {"LLM_HIGH": "anthropic/opus"} # blanks dropped + assert result["all_valid"] is True + + def test_invalid_models_flattens_with_tier(): resp = _response( all_valid=False, diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index 8bfaf7a..17cc2de 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -61,7 +61,7 @@ ) from cli import resolve_backend_config -from services import local_env +from services import local_env, validate_models from services.llm_tiers import LLM_TIER_KEYS from services.session import resolve_generation_id, set_project_root from services.specflow_backend import call_backend_endpoint_bytes @@ -870,6 +870,49 @@ def set_text(self, text: Text) -> None: self._label.update(text) +# Per-model validation glyphs (shared by the connect-screen tiers box and +# Settings). Symbol-first so state reads at a glance; green only for a model the +# provider catalog confirms, red only for a confidently-absent one, neutral when +# the catalog can't be checked. Maps the backend TierValidationStatus values. +_MODEL_MARKS: dict[str, tuple[str, str]] = { + "valid": ("✓", "green"), + "invalid": ("✗", "red"), + "unverified": ("•", "dim"), +} +_NEUTRAL_MARK: tuple[str, str] = ("•", "dim") + + +def _model_mark(status: str | None) -> tuple[str, str]: + """(symbol, style) for one model's validation status.""" + return _MODEL_MARKS.get(status or "", _NEUTRAL_MARK) + + +def _tier_mark(models: list[dict]) -> tuple[str, str]: + """(symbol, style) for a whole tier: red if any model is invalid, green if + all are valid, else neutral (some/all unverified — catalog unavailable).""" + statuses = [m.get("status") for m in models] + if "invalid" in statuses: + return _MODEL_MARKS["invalid"] + if statuses and all(s == "valid" for s in statuses): + return _MODEL_MARKS["valid"] + return _NEUTRAL_MARK + + +def _tier_marker_text(models: list[dict]) -> Text: + """Short 'glyph + word' marker for a Settings tier field (blank when no models + or only unverified — we never assert availability we can't confirm).""" + if not models: + return Text("") + symbol, style = _tier_mark(models) + if any(m.get("status") == "invalid" for m in models): + word = "unsupported" + elif all(m.get("status") == "valid" for m in models): + word = "available" + else: + word = "" + return Text(f"{symbol} {word}".strip(), style=style) + + class ClientSetupScreen(_SpecFlowScreen): """Register the SpecFlow MCP server with the user's AI client(s). @@ -899,6 +942,9 @@ def __init__(self) -> None: # block); recomputed each _populate so a Settings edit is reflected. self._current_fp: str | None = None self._saved_fp: dict[str, str] = {} + # Last model-validation response (parsed) for the tiers box, or None + # before/without a result. Recomputed by a background worker each populate. + self._validation: dict | None = None def compose(self) -> ComposeResult: yield Header() @@ -936,7 +982,11 @@ async def _populate(self) -> None: block = None self._current_fp = None self._saved_fp = mcp_clients.saved_fingerprints() + # Render the tiers box immediately (neutral glyphs), then validate the + # configured models against the provider catalog in the background. + self._validation = None self.query_one("#client-tiers", Static).update(self._tiers_panel(block)) + self.run_worker(self._validate_models(block), group="validate") self._status = { r.client.client_id: mcp_clients.initial_status( r.client, installed=r.installed, saved=r.saved @@ -1033,34 +1083,100 @@ def _row_label(self, row: mcp_clients.ClientRow, status: mcp_clients.ClientStatu "LLM_LOW": "simple steps (md→JSON, spec indexing)", } - def _tiers_panel(self, block: mcp_clients.ServerBlock | None) -> Panel: - # A bordered box showing the model tiers a connect would bake in, what - # each drives, the `m` affordance, and the standing caveat that a change - # reaches the sandbox only from the next run — an in-progress generation - # keeps the models it started with. + def _model_cell(self, value: str | None, models: list[dict]) -> Text: + """The model(s) cell: raw config text, each model tinted by its validity. + + A blank tier reads as the backend default. When validation is present, + each configured model is coloured green/red/dim per its catalog status. + """ + if not value: + return Text("default", style="italic dim") + status_by_model = {m.get("configured"): m.get("status") for m in models} + cell = Text() + for i, token in enumerate([t.strip() for t in value.split(",") if t.strip()]): + if i: + cell.append(", ", style="dim") + status = status_by_model.get(token) + _, style = _model_mark(status) if status else ("", "") + cell.append(token, style=style) + return cell + + def _invalid_notes(self, validation: dict | None) -> Text | None: + """One red line per confidently-invalid model, with a suggestion if any.""" + if not validation: + return None + invalid = validate_models.invalid_models(validation) + if not invalid: + return None + provider = validation.get("provider", "the active provider") + note = Text() + for i, model in enumerate(invalid): + if i: + note.append("\n") + tier = str(model.get("tier", "")).removeprefix("LLM_").lower() + note.append("✗ ", style="red") + note.append(f"{tier}: ", style="bold red") + note.append(f"'{model.get('configured', '?')}' not available on {provider}", style="red") + if suggestion := model.get("suggestion"): + note.append(f" — did you mean '{suggestion}'?", style="red") + return note + + def _tiers_panel( + self, block: mcp_clients.ServerBlock | None, validation: dict | None = None + ) -> Panel: + # A bordered box showing the model tiers a connect would bake in, a per-tier + # validity glyph (✓/✗/•), what each drives, the `m` affordance, and the + # standing caveat that a change reaches the sandbox only from the next run. env = block.env if block is not None else {} - table = Table.grid(padding=(0, 2)) + by_tier = {t.get("tier"): t.get("models", []) for t in (validation or {}).get("tiers", [])} + table = Table.grid(padding=(0, 1)) + table.add_column() # validity glyph table.add_column(style="bold cyan") # tier table.add_column() # model(s) table.add_column(style="dim") # purpose for key in LLM_TIER_KEYS: - tier = key.removeprefix("LLM_").lower() - value = env.get(key) - model = Text(value) if value else Text("default", style="italic dim") - table.add_row(tier, model, self._TIER_PURPOSE.get(key, "")) + models = by_tier.get(key, []) + symbol, sym_style = _tier_mark(models) if models else ("", "") + table.add_row( + Text(symbol, style=sym_style), + key.removeprefix("LLM_").lower(), + self._model_cell(env.get(key), models), + self._TIER_PURPOSE.get(key, ""), + ) footer = Text( "press m to change · a change takes effect from the next run " "(a generation already in progress keeps its models)", style="dim italic", ) + parts: list[RenderableType] = [table] + if (notes := self._invalid_notes(validation)) is not None: + parts += [Text(), notes] + parts += [Text(), footer] return Panel( - Group(table, Text(), footer), + Group(*parts), title="Model tiers", title_align="left", border_style="cyan", padding=(0, 1), ) + async def _validate_models(self, block: mcp_clients.ServerBlock | None) -> None: + """Validate the configured tier models against the provider catalog, then + re-render the box with per-model glyphs. Fully best-effort: any failure + (no backend, unreachable catalog) leaves the box neutral — never red.""" + if block is None: + return + tier_values = {key: block.env.get(key, "") for key in LLM_TIER_KEYS} + try: + response = await validate_models.request_model_validation_for(tier_values) + except Exception: # noqa: BLE001 - infra failure must stay silent here + return + self._validation = response + try: + self.query_one("#client-tiers", Static).update(self._tiers_panel(block, response)) + except NoMatches: + pass # screen dismissed before validation returned + def _selected_client(self) -> mcp_clients.McpClient | None: listview = self.query_one("#client-list", ListView) item = listview.highlighted_child @@ -1698,6 +1814,10 @@ def compose(self) -> ComposeResult: with Horizontal(classes="settings-row"): yield Label(f"{key:<22}", classes="settings-label") yield Input(value=str(env.get(key, "")), id=f"field-{key}") + # Tier fields carry a live validity marker (✓/✗) updated on + # mount and when the field is submitted (Enter). + if key in LLM_TIER_KEYS: + yield Static(id=f"tierstatus-{key}", classes="tier-status") yield Static("Secrets (.env — requires backend restart)", classes="settings-section") yield from self._secret_rows(ENV_SECRET_KEYS, secrets) yield Static( @@ -1711,6 +1831,34 @@ def compose(self) -> ComposeResult: def _secret_input(self, key: str) -> str: return self.query_one(f"#secret-{key}", Input).value.strip() + def on_mount(self) -> None: + # Validate the stored tier models once the fields exist, so their ✓/✗ + # markers reflect reality the moment the screen opens. + self.run_worker(self._validate_tier_inputs(), group="tiervalidate", exclusive=True) + + def on_input_submitted(self, event: Input.Submitted) -> None: + # Re-validate when a tier field is submitted (Enter) — "when changing". + if event.input.id in {f"field-{key}" for key in LLM_TIER_KEYS}: + self.run_worker(self._validate_tier_inputs(), group="tiervalidate", exclusive=True) + + async def _validate_tier_inputs(self) -> None: + """Validate the current tier-field values against the provider catalog and + update each field's marker. Best-effort: any failure clears to neutral.""" + tier_values = { + key: self.query_one(f"#field-{key}", Input).value.strip() for key in LLM_TIER_KEYS + } + try: + response = await validate_models.request_model_validation_for(tier_values) + except Exception: # noqa: BLE001 - infra failure must stay silent here + return + by_tier = {t.get("tier"): t.get("models", []) for t in response.get("tiers", [])} + for key in LLM_TIER_KEYS: + try: + marker = self.query_one(f"#tierstatus-{key}", Static) + except NoMatches: + continue + marker.update(_tier_marker_text(by_tier.get(key, []))) + def _block_fingerprint(self) -> str | None: """Fingerprint of the live server block, or None if it can't be read. @@ -1809,6 +1957,7 @@ class SpecFlowTUI(App): .settings-section { padding: 1 2 0 2; text-style: bold; color: $accent; } .settings-row { height: 3; padding: 0 2; } .settings-label { width: 20; content-align: left middle; } + .tier-status { width: auto; content-align: left middle; padding: 0 1; } #ws-stream { height: 2fr; border: round $primary; padding: 0 1; } #ws-stats { height: 1fr; padding: 0 1; } #onboard-body { height: 1fr; padding: 0 2; } From 5f87f085c082e7fbf3da35b075faed03df5e0512 Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Thu, 9 Jul 2026 15:38:29 +0200 Subject: [PATCH 6/7] fix(tui): keep the Settings tier validity marker on-screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-tier ✓/✗ marker was correctly set but rendered off the right edge: the row's Input did not flex, so label+input already overflowed the viewport and the trailing marker landed past it. Make .settings-row inputs 1fr and give the marker a fixed width so it always stays visible. Add a regression test asserting the marker region falls within the viewport. --- mcp_server/tests/test_tui_app.py | 26 ++++++++++++++++++++++++++ mcp_server/tui/app.py | 5 ++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/mcp_server/tests/test_tui_app.py b/mcp_server/tests/test_tui_app.py index ac52461..7b70857 100644 --- a/mcp_server/tests/test_tui_app.py +++ b/mcp_server/tests/test_tui_app.py @@ -2066,3 +2066,29 @@ async def test_settings_tier_markers_reflect_validation(self, tmp_path): ) assert "✗" in high and "unsupported" in high assert "✓" in medium and "available" in medium + + @pytest.mark.asyncio + async def test_settings_tier_marker_stays_on_screen(self, tmp_path): + # Regression: the marker must render within the viewport, not be pushed + # off the right edge by a non-flexing input (it was, before the CSS fix). + resp = self._response( + [{"tier": t, "models": [{"configured": "x", "status": "invalid"}]} + for t in ("LLM_HIGH", "LLM_MEDIUM", "LLM_LOW")] + ) + width = 100 + a, b, c = _gate_ready() + with a, b, c, patch("tui.app.fetch_sessions", new=AsyncMock(return_value=[])), patch.object( + tui_app.ClientSetupScreen, "_probe_verifiable", new=AsyncMock() + ), patch("tui.app.mcp_clients.is_any_client_connected", return_value=True), patch( + "tui.app.validate_models.request_model_validation_for", new=AsyncMock(return_value=resp) + ): + app = tui_app.SpecFlowTUI(root=tmp_path, generation_id=None, poll_interval=999) + async with app.run_test(size=(width, 40)) as pilot: + await pilot.pause() + await pilot.press("s") + await pilot.pause() + await pilot.pause() + marker = app.screen.query_one("#tierstatus-LLM_HIGH", tui_app.Static) + region = marker.region + assert region.width > 0 + assert region.right <= width # fully within the viewport diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index 17cc2de..c909ad5 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -1957,7 +1957,10 @@ class SpecFlowTUI(App): .settings-section { padding: 1 2 0 2; text-style: bold; color: $accent; } .settings-row { height: 3; padding: 0 2; } .settings-label { width: 20; content-align: left middle; } - .tier-status { width: auto; content-align: left middle; padding: 0 1; } + /* Tier rows also carry a validity marker; the input must flex (1fr) so the + fixed-width marker stays on-screen instead of being pushed off the right. */ + .settings-row Input { width: 1fr; } + .tier-status { width: 16; content-align: left middle; padding: 0 1; } #ws-stream { height: 2fr; border: round $primary; padding: 0 1; } #ws-stats { height: 1fr; padding: 0 1; } #onboard-body { height: 1fr; padding: 0 2; } From a0f36a77b028d14f2a275d98099f9e843e6d5e11 Mon Sep 17 00:00:00 2001 From: Artsiom Kozak Date: Fri, 10 Jul 2026 10:07:16 +0200 Subject: [PATCH 7/7] refactor(mcp): make env LLM-tier overrides a pure function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review: apply_llm_tier_overrides mutated its argument in place (a hidden side effect). Add pure llm_tier_overrides_from_env() that returns the overrides, and keep apply_llm_tier_overrides as a thin in-place wrapper for generation_orchestrator (which accumulates several field groups into one form). request_model_validation now passes the returned dict directly — no mutation. Behavior is byte-identical, so the backend request is unchanged. --- mcp_server/services/llm_tiers.py | 22 ++++++++++++---------- mcp_server/services/validate_models.py | 6 ++---- mcp_server/tests/test_llm_tiers.py | 26 ++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/mcp_server/services/llm_tiers.py b/mcp_server/services/llm_tiers.py index 8143cf5..aca1ec9 100644 --- a/mcp_server/services/llm_tiers.py +++ b/mcp_server/services/llm_tiers.py @@ -23,15 +23,17 @@ def apply_mcp_servers_enabled_form(form_data: Dict[str, str]) -> None: form_data[MCP_SERVERS_ENABLED_ENV] = value.strip() -def apply_llm_tier_overrides(form_data: Dict[str, str]) -> None: - """Inject LLM tier overrides from the MCP environment into form_data (in-place). +def llm_tier_overrides_from_env() -> Dict[str, str]: + """Return the LLM tier overrides present in the process environment. - Reads LLM_HIGH, LLM_MEDIUM, LLM_LOW from the process environment (set via - mcp.json) and adds any that are present to the form_data dict that will be - sent to the backend API. Keys not set in the environment are left out so - the backend falls back to its own defaults. + Reads LLM_HIGH, LLM_MEDIUM, LLM_LOW (set via mcp.json) and returns only the + ones actually set, so absent tiers let the backend fall back to its own + defaults. Pure — no mutation — so callers can pass the result directly. """ - for key in LLM_TIER_KEYS: - value = os.environ.get(key) - if value: - form_data[key] = value + return {key: value for key in LLM_TIER_KEYS if (value := os.environ.get(key))} + + +def apply_llm_tier_overrides(form_data: Dict[str, str]) -> None: + """In-place variant of :func:`llm_tier_overrides_from_env` for callers that + accumulate several groups of fields into one ``form_data`` dict.""" + form_data.update(llm_tier_overrides_from_env()) diff --git a/mcp_server/services/validate_models.py b/mcp_server/services/validate_models.py index b8b6506..e990d0e 100644 --- a/mcp_server/services/validate_models.py +++ b/mcp_server/services/validate_models.py @@ -18,7 +18,7 @@ from fastmcp import Context -from services.llm_tiers import LLM_TIER_KEYS, apply_llm_tier_overrides +from services.llm_tiers import LLM_TIER_KEYS, llm_tier_overrides_from_env from services.run_generation_precheck import RejectionCode from services.specflow_backend import post_form_data_to_backend @@ -45,9 +45,7 @@ async def request_model_validation() -> dict[str, Any]: ValidateModelsResponse dict. Raises on transport/parse errors so callers can stay permissive (connect/run gate) on infrastructure failures. """ - form_data: dict[str, str] = {} - apply_llm_tier_overrides(form_data) # injects LLM_HIGH/MEDIUM/LOW from env when set - return await _post_validation(form_data) + return await _post_validation(llm_tier_overrides_from_env()) async def request_model_validation_for(tier_values: dict[str, str]) -> dict[str, Any]: diff --git a/mcp_server/tests/test_llm_tiers.py b/mcp_server/tests/test_llm_tiers.py index cba27c2..4d5aeaa 100644 --- a/mcp_server/tests/test_llm_tiers.py +++ b/mcp_server/tests/test_llm_tiers.py @@ -5,9 +5,35 @@ MCP_SERVERS_ENABLED_ENV, apply_llm_tier_overrides, apply_mcp_servers_enabled_form, + llm_tier_overrides_from_env, ) +class TestLlmTierOverridesFromEnv: + def test_returns_only_keys_present_in_env(self, monkeypatch): + monkeypatch.setenv("LLM_HIGH", "claude-opus") + monkeypatch.setenv("LLM_LOW", "claude-haiku") + monkeypatch.delenv("LLM_MEDIUM", raising=False) + assert llm_tier_overrides_from_env() == { + "LLM_HIGH": "claude-opus", + "LLM_LOW": "claude-haiku", + } + + def test_empty_when_no_env(self, monkeypatch): + for key in LLM_TIER_KEYS: + monkeypatch.delenv(key, raising=False) + assert llm_tier_overrides_from_env() == {} + + def test_apply_is_the_in_place_form_of_the_pure_helper(self, monkeypatch): + monkeypatch.setenv("LLM_HIGH", "claude-opus") + monkeypatch.delenv("LLM_MEDIUM", raising=False) + monkeypatch.delenv("LLM_LOW", raising=False) + form: dict[str, str] = {"spec_path": "specs"} + apply_llm_tier_overrides(form) + # Wrapper merges the pure result, preserving unrelated keys. + assert form == {"spec_path": "specs", "LLM_HIGH": "claude-opus"} + + class TestApplyLlmTierOverrides: def test_injects_only_keys_present_in_env(self, monkeypatch): monkeypatch.setenv("LLM_HIGH", "claude-opus")