diff --git a/docs/agent.md b/docs/agent.md index 7043c63c..685f8e62 100644 --- a/docs/agent.md +++ b/docs/agent.md @@ -186,6 +186,28 @@ header shows `[small]` so you always know which mode is live. Compare the profiles on your own endpoint with the eval harness: `python -m korvid.evals --profile small` (see below). +## Follow mode + +Small models rarely volunteer the screen tools (`open_describe`, +`open_logs`) — they call the data-returning reads and answer in text +while the TUI sits idle. Agent follow mode mirrors each successful +cluster read from a chat turn on screen, using the same mapping as MCP +follow mode: `list_resources` navigates the view, `get_resource` / +`get_events` / `diagnose_pod` open the describe view, and `get_logs` +opens the live log pane. + +Follow is **on by default**. Disable it in `config.yaml`: + +```yaml +agent: + follow: false # default: true +``` + +or toggle it live with `:ai follow off` / `:ai follow on` (bare +`:ai follow` flips the state). Mirroring never interrupts what you are +doing: failed reads (e.g. a 404) move nothing, and a mirror is refused +while an approval dialog or a describe screen you are reading is open. + ## Agent eval harness `korvid.evals` measures how well a model diagnoses cluster faults through diff --git a/src/korvid/__main__.py b/src/korvid/__main__.py index 02830230..90d752d2 100644 --- a/src/korvid/__main__.py +++ b/src/korvid/__main__.py @@ -915,6 +915,10 @@ async def fetch( helm=_build_helm(config), telepresence=_build_telepresence(config), probe_traffic_manager=_make_traffic_manager_probe(kube), + # Agent follow mirrors route through the same serialized proxy: the + # built-in agent and concurrent MCP UI calls must never interleave + # (log-pane swaps and describes are not overlap-safe). + agent_follow_bridge=ui_proxy, proposal_store=proposal_store, save_topbar=lambda expanded: save_topbar_state(DEFAULT_CONFIG_PATH, expanded=expanded), ) diff --git a/src/korvid/agent/profiles.py b/src/korvid/agent/profiles.py index d590fa74..f7272fb9 100644 --- a/src/korvid/agent/profiles.py +++ b/src/korvid/agent/profiles.py @@ -7,8 +7,9 @@ selection, they degrade with context length far below their advertised windows, and 1-2 in-context demonstrations improve their multi-step tool use more than longer instruction lists do (ReAct). The `small` profile -gives them a surface they can actually handle; `full` reproduces the -pre-profile wiring byte-for-byte so the frontier experience is unchanged. +gives them a surface they can actually handle; `full` keeps the frontier +tool surface and budgets unchanged (its prompt wording, like `small`'s, +lives in korvid.agent.prompts and evolves with observed failures). """ from __future__ import annotations @@ -17,7 +18,14 @@ from dataclasses import dataclass from typing import Any -from korvid.agent.runtime import MAX_HISTORY_CHARS, SYSTEM_PROMPT, UI_DRIVE_PROMPT +from korvid.agent.prompts import ( + SMALL_SYSTEM_PROMPT, + SMALL_TOOL_DESCRIPTIONS, + SMALL_UI_PROMPT, + SYSTEM_PROMPT, + UI_DRIVE_PROMPT, +) +from korvid.agent.runtime import MAX_HISTORY_CHARS from korvid.tools.registry import agent_tool_schemas PROFILE_NAMES = ("full", "small") @@ -40,59 +48,9 @@ #: not a suggestion. SMALL_MAX_TOOL_CALLS_PER_ITERATION = 1 -#: Short role statement, explicit grounding rules, and ONE worked example -#: (question -> tool call -> result -> grounded answer) instead of the -#: longer frontier instruction list. -SMALL_SYSTEM_PROMPT = ( - "You are korvid's Kubernetes diagnostic agent, embedded in a live TUI. " - "Use tools to inspect cluster state and cite evidence from tool results. " - "Call one tool at a time and wait for its result before deciding the " - "next step. Never invent resource names: use only names from the screen " - "context or from tool results. " - "Worked example — user: why does pod checkout-1 in namespace shop keep " - 'restarting? -> you call diagnose_pod with {"pod": "checkout-1", ' - '"namespace": "shop"} -> the result shows lastState terminated ' - "exit=137 (OOMKilled) -> you answer: checkout-1 is OOMKilled (exit " - "code 137); its container exceeds the memory limit, so raise the limit " - "or reduce usage." -) - -#: The full UI_DRIVE_PROMPT advertises all five UI tools; the small profile -#: offers only the two evidence-showing ones, and the model must never be -#: told about capabilities it was not offered. -SMALL_UI_PROMPT = ( - "You can also show evidence on the user's screen: open_logs (show a " - "pod's live logs) and open_describe (show a resource's manifest and " - "events). These change nothing in the cluster. Keep your text concise; " - "the screen carries the detail." -) - -#: The two evidence-showing UI tools the small profile offers are encoded -#: in the registry's `small_agent` surface (korvid.tools.registry). - -#: Concise description overrides for schemas that are verbose in the full -#: profile — every request retransmits the schemas, so on a 4k-token -#: serving context the wording is a real cost (EasyTool). The effect is -#: measurable per endpoint with the #69 harness (`--profile small`). -_SMALL_DESCRIPTIONS: dict[str, str] = { - "diagnose_pod": ( - "One-call diagnosis of a broken pod: container states, exit codes, " - "restart counts, failing conditions, Warning events, node/PVC " - "context, and log excerpts. Prefer this first when a pod is failing." - ), - "list_operators": ( - "List OLM operator packages and installed subscriptions with their status. Read-only." - ), - "helm_list_releases": ( - "List installed Helm releases with revision, status, chart and app " - "version. Read-only; parsed from cluster Secrets." - ), - "open_logs": "Open the live log pane for a pod on the user's screen.", - "resize_pod": ( - "Request an in-place CPU/memory resize of a running pod (Kubernetes " - "1.35+). Runs only after the user approves it in the TUI dialog." - ), -} +#: All prompt wording — the full/small role statements, UI-drive variants, +#: and the small profile's concise tool-description overrides — lives in +#: korvid.agent.prompts; this module owns budgets and surface selection. @dataclass(frozen=True) @@ -128,7 +86,7 @@ def _trim(tools: list[dict[str, Any]]) -> list[dict[str, Any]]: trimmed = copy.deepcopy(tools) for tool in trimmed: function = tool["function"] - override = _SMALL_DESCRIPTIONS.get(function["name"]) + override = SMALL_TOOL_DESCRIPTIONS.get(function["name"]) if override is not None: function["description"] = override return trimmed diff --git a/src/korvid/agent/prompts.py b/src/korvid/agent/prompts.py new file mode 100644 index 00000000..17d12f7c --- /dev/null +++ b/src/korvid/agent/prompts.py @@ -0,0 +1,163 @@ +"""Every system-prompt string the agent sends, in one place. + +Prompt text is policy (what the model is told), the runtime is mechanism +(how the loop executes); keeping all wording here means a prompt change +never touches loop code, and the full/small variants (issue #71) sit side +by side instead of split across modules. + +Composition (`compose_system_prompt`) lives here too: which clause is +appended is decided by the *armed tool set*, so the write/no-write and +UI-drive wording stays next to the rules that select it. +""" + +from __future__ import annotations + +from typing import Any + +from korvid.tools.executor import UI_TOOL_NAMES, WRITE_TOOL_NAMES + +SYSTEM_PROMPT = ( + "You are korvid's Kubernetes diagnostic agent, embedded in a live TUI the " + "user is looking at right now. You explore the cluster only through the " + "tools provided in this session: you have no shell, you cannot run " + "kubectl or any command yourself, and you know nothing about this " + "cluster beyond tool results and the screen context. " + "Explore before you conclude: list resources to discover what exists, " + "then inspect the specific objects you found. " + "Cite evidence from tool results and never guess resource state. " + "Never invent resource names or namespaces — use only names from tool " + "results or the screen context, and keep each name paired with the " + "namespace it was listed in. A 404/NotFound means the name or namespace " + "is wrong: re-list to find the right one instead of retrying the same " + "call." +) + +# Appended when no write tools are armed (readonly mode or writes not wired): +# instead of a bare refusal the agent offers the exact kubectl command. +NO_WRITE_PROMPT = ( + "You have no write tools in this session: when the user asks you to " + "modify cluster state (scale, edit, delete, restart, apply), say write " + "actions are not enabled and give the exact kubectl command they can run " + "themselves instead." +) + +# Appended only when the approval-gated write tools are armed. The armed +# tool names are prepended dynamically in `compose_system_prompt` so the +# instruction never omits a conditionally registered tool (resize_pod) or +# advertises one that was not offered. +WRITE_PROMPT = ( + "These never execute directly: each call opens an " + "approval dialog in the TUI, and the operation runs only if the user " + "approves it with a keystroke. State clearly what you are about to " + "request and why before calling a write tool, and report the outcome " + "(approved, denied, expired, or failed) afterwards. Never retry a denied " + "or expired request unless the user explicitly asks: an expired request " + "means nobody answered the dialog, and reissuing it would keep reopening " + "approval dialogs the user is not acting on." +) + +# Appended only when the runtime is armed with the UI-control tools, so the +# model is never told about capabilities the provider was not offered. +UI_DRIVE_PROMPT = ( + "You can also drive the TUI itself: navigate (switch the resource view), " + "set_filter (narrow the visible rows), open_logs (show a pod's live logs " + "on screen), open_describe (show a resource's manifest and events), and " + "drill_down (from a deployment into its replicaset history, from a " + "replicaset into its pods, or from a helm release into its revision " + "history — following ownership). " + "Prefer showing evidence on screen with these tools while you narrate — " + "for example, when you find a failing pod, open its logs or describe view " + "so the user sees exactly what you see. These screen tools change nothing " + "in the cluster. Keep your text concise; the screen carries the detail." +) + +#: Short role statement, explicit grounding rules, and ONE worked example +#: (question -> tool call -> result -> grounded answer) instead of the +#: longer frontier instruction list (issue #71). +SMALL_SYSTEM_PROMPT = ( + "You are korvid's Kubernetes exploration agent, embedded in a live TUI. " + "You act only through the provided tools: no shell, no kubectl, no " + "cluster knowledge outside tool results. " + "Call one tool at a time and wait for its result before deciding the " + "next step. Never invent resource names or namespaces: use a name and " + "namespace pair given by the user or the screen context, or discover " + "one with list_resources first. Its rows start with " + "'namespace/name' — split that into the separate namespace and name " + "fields; never paste the combined value into either field. " + "A 404/NotFound means the name or namespace is wrong — " + "re-list instead of retrying. Cite evidence from tool results. " + "Worked example — user: why does pod checkout-1 in namespace shop keep " + 'restarting? -> you call diagnose_pod with {"pod": "checkout-1", ' + '"namespace": "shop"} -> the result shows lastState terminated ' + "exit=137 (OOMKilled) -> you answer: checkout-1 is OOMKilled (exit " + "code 137); its container exceeds the memory limit, so raise the limit " + "or reduce usage." +) + +#: The full UI_DRIVE_PROMPT advertises all five UI tools; the small profile +#: offers only the two evidence-showing ones, and the model must never be +#: told about capabilities it was not offered. +SMALL_UI_PROMPT = ( + "You can also show evidence on the user's screen: open_logs (show a " + "pod's live logs) and open_describe (show a resource's manifest and " + "events). These change nothing in the cluster. Keep your text concise; " + "the screen carries the detail." +) + +#: Concise tool-description overrides for the small profile — every request +#: retransmits the schemas, so on a 4k-token serving context the wording is +#: a real cost (EasyTool). The effect is measurable per endpoint with the +#: #69 harness (`--profile small`). +SMALL_TOOL_DESCRIPTIONS: dict[str, str] = { + "diagnose_pod": ( + "One-call diagnosis of a broken pod: container states, exit codes, " + "restart counts, failing conditions, Warning events, node/PVC " + "context, and log excerpts. Prefer this first when a pod is failing." + ), + "list_operators": ( + "List OLM operator packages and installed subscriptions with their status. Read-only." + ), + "helm_list_releases": ( + "List installed Helm releases with revision, status, chart and app " + "version. Read-only; parsed from cluster Secrets." + ), + "open_logs": "Open the live log pane for a pod on the user's screen.", + "resize_pod": ( + "Request an in-place CPU/memory resize of a running pod (Kubernetes " + "1.35+). Runs only after the user approves it in the TUI dialog." + ), +} + + +def compose_system_prompt( + tools: list[dict[str, Any]], + cluster_context: str | None, + *, + system_prompt: str | None = None, + ui_prompt: str | None = None, +) -> str: + """System prompt for the armed tool set and detected environment. + + Shared by ``AgentRuntime.__init__`` and ``retarget`` so a runtime that + survives a `:ctx` switch describes the *new* cluster and tool set, not + the one it was built against. Capability profiles (issue #71) swap the + role statement and the UI-drive instruction via + `system_prompt`/`ui_prompt`; the write/no-write clause stays + conditional on what is actually armed, whichever profile. + """ + prompt = system_prompt if system_prompt is not None else SYSTEM_PROMPT + if cluster_context: + # Detected-environment note (e.g. cloud provider, issue #30): + # placed right after the role statement so provider-specific + # requests are grounded before any tool instructions. + prompt = f"{prompt} {cluster_context}" + armed = {t.get("function", {}).get("name") for t in tools} + if armed & UI_TOOL_NAMES: + prompt = f"{prompt} {ui_prompt if ui_prompt is not None else UI_DRIVE_PROMPT}" + armed_writes = sorted(armed & WRITE_TOOL_NAMES) + if armed_writes: + names = ", ".join(armed_writes) + prompt = f"{prompt} You can request cluster writes with {names}. {WRITE_PROMPT}" + else: + prompt = f"{prompt} {NO_WRITE_PROMPT}" + return prompt diff --git a/src/korvid/agent/runtime.py b/src/korvid/agent/runtime.py index ee13eb15..147847ac 100644 --- a/src/korvid/agent/runtime.py +++ b/src/korvid/agent/runtime.py @@ -16,62 +16,15 @@ ToolCallStarted, TurnComplete, ) +from korvid.agent.prompts import compose_system_prompt from korvid.tools.executor import ( READ_TOOLS, - UI_TOOL_NAMES, - WRITE_TOOL_NAMES, cap_result, compact_result, ) logger = logging.getLogger(__name__) -SYSTEM_PROMPT = ( - "You are korvid's Kubernetes diagnostic agent, embedded in a live TUI the " - "user is looking at right now. " - "Use tools to inspect cluster state, cite evidence from tool results, " - "and never guess resource state." -) - -# Appended when no write tools are armed (readonly mode or writes not wired): -# instead of a bare refusal the agent offers the exact kubectl command. -NO_WRITE_PROMPT = ( - "You have no write tools in this session: when the user asks you to " - "modify cluster state (scale, edit, delete, restart, apply), say write " - "actions are not enabled and give the exact kubectl command they can run " - "themselves instead." -) - -# Appended only when the approval-gated write tools are armed. The armed -# tool names are prepended dynamically in __init__ so the instruction never -# omits a conditionally registered tool (resize_pod) or advertises one that -# was not offered. -WRITE_PROMPT = ( - "These never execute directly: each call opens an " - "approval dialog in the TUI, and the operation runs only if the user " - "approves it with a keystroke. State clearly what you are about to " - "request and why before calling a write tool, and report the outcome " - "(approved, denied, expired, or failed) afterwards. Never retry a denied " - "or expired request unless the user explicitly asks: an expired request " - "means nobody answered the dialog, and reissuing it would keep reopening " - "approval dialogs the user is not acting on." -) - -# Appended only when the runtime is armed with the UI-control tools, so the -# model is never told about capabilities the provider was not offered. -UI_DRIVE_PROMPT = ( - "You can also drive the TUI itself: navigate (switch the resource view), " - "set_filter (narrow the visible rows), open_logs (show a pod's live logs " - "on screen), open_describe (show a resource's manifest and events), and " - "drill_down (from a deployment into its replicaset history, from a " - "replicaset into its pods, or from a helm release into its revision " - "history — following ownership). " - "Prefer showing evidence on screen with these tools while you narrate — " - "for example, when you find a failing pod, open its logs or describe view " - "so the user sees exactly what you see. These screen tools change nothing " - "in the cluster. Keep your text concise; the screen carries the detail." -) - # History is trimmed to the most recent turns to bound token cost; a turn # begins at a "user" message, so trimming never splits assistant/tool pairs. MAX_HISTORY_TURNS = 8 @@ -129,40 +82,6 @@ def _estimate_missing_usage(state: _StreamState, prompt_estimate: int) -> None: state.out_tok = _stream_output_chars(state) // 4 -def _compose_system_prompt( - tools: list[dict[str, Any]], - cluster_context: str | None, - *, - system_prompt: str | None = None, - ui_prompt: str | None = None, -) -> str: - """System prompt for the armed tool set and detected environment. - - Shared by ``__init__`` and ``retarget`` so a runtime that survives a - `:ctx` switch describes the *new* cluster and tool set, not the one it - was built against. Capability profiles (issue #71) swap the role - statement and the UI-drive instruction via `system_prompt`/`ui_prompt`; - the write/no-write clause stays conditional on what is actually armed, - whichever profile. - """ - prompt = system_prompt if system_prompt is not None else SYSTEM_PROMPT - if cluster_context: - # Detected-environment note (e.g. cloud provider, issue #30): - # placed right after the role statement so provider-specific - # requests are grounded before any tool instructions. - prompt = f"{prompt} {cluster_context}" - armed = {t.get("function", {}).get("name") for t in tools} - if armed & UI_TOOL_NAMES: - prompt = f"{prompt} {ui_prompt if ui_prompt is not None else UI_DRIVE_PROMPT}" - armed_writes = sorted(armed & WRITE_TOOL_NAMES) - if armed_writes: - names = ", ".join(armed_writes) - prompt = f"{prompt} You can request cluster writes with {names}. {WRITE_PROMPT}" - else: - prompt = f"{prompt} {NO_WRITE_PROMPT}" - return prompt - - class AgentRuntime: """Drives the provider + tools loop, emitting typed AgentEvent objects.""" @@ -192,7 +111,7 @@ def __init__( # UI-drive instruction (issue #71), not reset them to the defaults. self._system_prompt_override = system_prompt self._ui_prompt_override = ui_prompt - prompt = _compose_system_prompt( + prompt = compose_system_prompt( self._tools, cluster_context, system_prompt=system_prompt, @@ -231,7 +150,7 @@ def retarget(self, *, tools: list[dict[str, Any]], cluster_context: str | None) self._tools_chars = len(json.dumps(self._tools)) self._messages[0] = { "role": "system", - "content": _compose_system_prompt( + "content": compose_system_prompt( tools, cluster_context, system_prompt=self._system_prompt_override, diff --git a/src/korvid/core/config.py b/src/korvid/core/config.py index 6e1a5fca..b6b42a5c 100644 --- a/src/korvid/core/config.py +++ b/src/korvid/core/config.py @@ -69,6 +69,11 @@ class KorvidConfig: #: `agent.disable_in_protected` (issue #83): refuse agent prompts entirely #: while a protected context is active. agent_disable_in_protected: bool = False + #: `agent.follow`: mirror the built-in agent's successful cluster reads + #: on screen (like MCP follow, issue #153, but for the in-app chat). + #: Small local models rarely volunteer the UI tools, so this defaults + #: on; runtime toggle: `:ai follow on|off`. + agent_follow: bool = True mcp_enabled: bool = False mcp_port: int = 7878 #: `mcp.write_proposals` (issue #110): expose the external write-proposal @@ -200,6 +205,7 @@ def load_config(path: Path | None = None) -> KorvidConfig: readonly=raw.get("readonly") is True, protected_contexts=_parse_protected_contexts(raw.get("protected_contexts")), agent_disable_in_protected=agent_raw.get("disable_in_protected") is True, + agent_follow=agent_raw.get("follow") is not False, mcp_enabled=mcp_raw.get("enabled") is True, mcp_port=_parse_port(mcp_raw.get("port")), mcp_write_proposals=mcp_raw.get("write_proposals") is True, diff --git a/src/korvid/tools/executor.py b/src/korvid/tools/executor.py index fea627ea..7551e107 100644 --- a/src/korvid/tools/executor.py +++ b/src/korvid/tools/executor.py @@ -49,6 +49,25 @@ _TRUNCATION_SUFFIX = "\n… [truncated — narrow the query]" +def _reject_slash_name(value: str, field: str) -> str: + """Reject a 'namespace/name' composite before it reaches the cluster. + + Kubernetes object and namespace names can never contain '/' (DNS + subdomain rules), but models — small ones especially — paste + composites from row keys or prose into either field. The call still + consumes its agent-loop iteration like any errored tool call; what + failing locally buys is no API round-trip and recovery guidance that + teaches the split instead of a bare 404. + """ + if "/" in value: + raise ValueError( + f"invalid {field} {value!r}: Kubernetes names never contain '/'. " + "If this is 'namespace/name', pass the namespace and the name " + "separately, each in its own field." + ) + return value + + def cap_result(result: str, limit: int = MAX_RESULT_CHARS) -> str: """Enforce the tool-result ingest cap; shared by every path that feeds a result into conversation history. Profiles may pass a tighter @@ -716,8 +735,10 @@ async def _list_operators(self, args: dict[str, Any]) -> str: async def _get_resource(self, args: dict[str, Any]) -> str: kind = str(args["kind"]).strip().lower() - name = str(args["name"]) + name = _reject_slash_name(str(args["name"]), "name") namespace: str | None = args.get("namespace") + if namespace is not None: + namespace = _reject_slash_name(str(namespace), "namespace") meta = self._api_meta(kind) # A namespaced kind without a namespace would hit an invalid # cluster-scoped path — give the model an actionable error instead. @@ -728,8 +749,8 @@ async def _get_resource(self, args: dict[str, Any]) -> str: return yaml.safe_dump(manifest, default_flow_style=False, allow_unicode=True) async def _get_logs(self, args: dict[str, Any]) -> str: - pod = str(args["pod"]) - namespace = str(args["namespace"]) + pod = _reject_slash_name(str(args["pod"]), "pod") + namespace = _reject_slash_name(str(args["namespace"]), "namespace") container: str = str(args.get("container") or "") raw_tail = args.get("tail_lines", 100) tail_lines = max(1, min(500, int(raw_tail))) @@ -753,8 +774,8 @@ async def _get_logs(self, args: dict[str, Any]) -> str: async def _get_events(self, args: dict[str, Any]) -> str: kind = str(args["kind"]).strip().lower() - namespace = str(args["namespace"]) - name = str(args["name"]) + namespace = _reject_slash_name(str(args["namespace"]), "namespace") + name = _reject_slash_name(str(args["name"]), "name") meta = self._api_meta(kind) # Fetch the live object so events are scoped to this exact incarnation # (kind + UID), not merely anything sharing the name. @@ -984,8 +1005,8 @@ async def _diagnose_pod(self, args: dict[str, Any]) -> str: ``MAX_RESULT_CHARS`` without the shared prefix-truncation ever eating the final log evidence. """ - name = str(args["pod"]) - namespace = str(args["namespace"]) + name = _reject_slash_name(str(args["pod"]), "pod") + namespace = _reject_slash_name(str(args["namespace"]), "namespace") pods_meta = self._api_meta("pods") pod = await self._kube.get_object(pods_meta, namespace, name) head_sections: list[tuple[str, list[str]]] = [ diff --git a/src/korvid/ui/app.py b/src/korvid/ui/app.py index 12f12b5e..8017f205 100644 --- a/src/korvid/ui/app.py +++ b/src/korvid/ui/app.py @@ -41,7 +41,7 @@ from textual.widgets.data_table import CellDoesNotExist, RowDoesNotExist from textual.worker import Worker, get_current_worker -from korvid.agent.events import AgentError +from korvid.agent.events import AgentError, AgentEvent, ToolCallFinished, ToolCallStarted from korvid.agent.setup import AgentConfigurator, AgentSettings from korvid.core.audit import AuditLog from korvid.core.config import KorvidConfig, ViewConfig @@ -101,6 +101,7 @@ from korvid.k8s.telepresence import TelepresenceCLI, TelepresenceError from korvid.k8s.writes import WriteOps, restart_stamp from korvid.tools.executor import UIBridge +from korvid.tools.follow import FOLLOWABLE_TOOLS, mirror_read from korvid.tools.proposals import ( ProposalClosedError, ProposalLimitError, @@ -709,6 +710,7 @@ def __init__( save_topbar: Callable[[bool], None] | None = None, telepresence: TelepresenceCLI | None = None, probe_traffic_manager: Callable[[], Awaitable[bool]] | None = None, + agent_follow_bridge: UIBridge | None = None, ) -> None: super().__init__() self.config = config @@ -751,6 +753,17 @@ def __init__( #: MCP follow mode (issue #153): mirror external cluster reads in #: the TUI. Config seeds the state; `:mcp follow on|off` toggles it. self._mcp_follow: bool = config.mcp_follow + #: Agent follow: mirror the built-in agent's cluster reads on screen + #: — small models rarely volunteer the UI tools, so without this the + #: screen sits idle while the agent reads "behind its back". Config + #: seeds the state (default on); `:ai follow on|off` toggles it. + self._agent_follow: bool = config.agent_follow + #: The shared serialized UI bridge (the composition root's + #: `_UIBridgeProxy`): agent-follow mirrors route through it so they + #: serialize with the agent's own UI tools and concurrent MCP UI + #: calls - log-pane swaps and describes must never interleave. + #: None (tests, degraded wiring) falls back to a direct adapter. + self._agent_follow_bridge = agent_follow_bridge #: External MCP write proposals (issue #110): shared with the MCP #: server; None when the feature is disabled. self._proposal_store = proposal_store @@ -2882,6 +2895,9 @@ def on_unknown_command(self, message: UnknownCommand) -> None: parts = message.text.strip().split() head = parts[0] if parts else "" if head in {"ai", "agent"} and self._agent_available: + if len(parts) > 1 and parts[1].lower() == "follow": + self._handle_agent_follow_command(parts[2:]) + return self._open_agent_setup() return if head == "model" and self._agent_available: @@ -3064,6 +3080,19 @@ def _handle_mcp_follow_command(self, args: list[str]) -> None: ) self._refresh_status() + def _handle_agent_follow_command(self, args: list[str]) -> None: + """`:ai follow [on|off]`: toggle mirroring of the built-in agent's + cluster reads on screen. Bare `:ai follow` flips the state.""" + if args and args[0].lower() not in ("on", "off"): + self.notify("Usage: :ai follow [on|off]", severity="warning") + return + self._agent_follow = args[0].lower() == "on" if args else not self._agent_follow + state = "on" if self._agent_follow else "off" + self.notify( + f"Agent follow {state} — the agent's reads are " + f"{'mirrored on screen' if self._agent_follow else 'no longer mirrored'}" + ) + @property def mcp_follow_enabled(self) -> bool: """Current follow-mode state; read by the MCP server's wiring.""" @@ -8359,12 +8388,22 @@ def _screen_context(self) -> str: """What the agent is told about the screen: the focused pane in detail plus a one-line summary of the other pane (issue #48), so context stays bounded in a split workspace.""" + selected = self._selected_row_name() or "-" + selected_ns = "" + if "/" in selected: + # Row keys are 'namespace/name' composites; fed verbatim they + # teach the model to paste the whole string as a resource name + # (observed: get_resource name='default/otel-…' -> 404). Hand + # over the two fields the tool calls actually take. + selected_ns, _, selected = selected.partition("/") context = ( f"context={self.config.kube_context or '-'} " f"view={self.current_kind} scope={self.current_scope} " - f"selected={self._selected_row_name() or '-'} " - f"filter={self.filter_pattern or '-'}" + f"selected={selected}" ) + if selected_ns: + context += f" selected_ns={selected_ns}" + context += f" filter={self.filter_pattern or '-'}" if len(self._panes) == 2: other = self._panes[1 - self._focused_pane] context += f" other_pane={other.kind} other_scope={other.scope}" @@ -8381,12 +8420,48 @@ async def _run_agent_turn(self, user_text: str) -> None: # switch once; afterwards the context= field carries the truth. screen_context += f" NOTE: {self._ctx_switch_note}" self._ctx_switch_note = None + # Agent follow: started cluster reads awaiting their result, keyed + # by call id (the finish event does not carry the arguments). + pending_reads: dict[str, tuple[str, str]] = {} try: async for event in runtime.run_turn(user_text, screen_context): panel.apply_event(event) + await self._maybe_follow_agent_read(event, pending_reads) except Exception as exc: panel.apply_event(AgentError(message=str(exc))) + async def _maybe_follow_agent_read( + self, + event: AgentEvent, + pending: dict[str, tuple[str, str]], + ) -> None: + """Mirror a successful agent cluster read on screen (agent follow). + + Small local models rarely volunteer the UI tools — they call the + data-returning reads and answer in text while the screen sits + idle. With follow on, each successful read is mirrored through the + same UIBridge mapping MCP follow uses (issue #153). Best-effort: + `mirror_read` never raises, and the bridge guards (approval + dialogs, screens the user is reading) refuse rather than cover. + """ + if isinstance(event, ToolCallStarted): + if event.name in FOLLOWABLE_TOOLS: + pending[event.call_id] = (event.name, event.arguments) + return + if not isinstance(event, ToolCallFinished): + return + started = pending.pop(event.call_id, None) + if started is None or not event.ok or not self._agent_follow: + return + name, raw_arguments = started + try: + arguments = json.loads(raw_arguments) if raw_arguments else {} + except json.JSONDecodeError: + return # small models emit broken JSON; the read still answered + if not isinstance(arguments, dict): + return + await mirror_read(self._agent_follow_bridge or AppUIBridge(self), name, arguments) + # ------------------------------------------------------------------ # UIBridge implementation (spec §4.1 UI Bus): the agent drives the # exact same handlers as user keystrokes. Every method returns a @@ -8480,6 +8555,13 @@ async def agent_open_logs(self, pod: str, namespace: str, container: str | None "ERROR: an approval dialog is open — the user is deciding; " "wait for their decision before opening logs" ) + if isinstance(self.screen, DescribeScreen): + # Same user-priority rule as describe/navigate/drill: opening + # logs swaps the streams beneath the modal the user is reading. + return ( + "ERROR: a describe screen is open — the user is reading it; " + "ask them to close it (Esc) before opening logs" + ) if self._stream_logs is None: return "ERROR: log streaming unavailable in this session" pane_gen = self._log_pane_gen @@ -8633,6 +8715,15 @@ def _describe_precheck(self, kind: str, namespace: str | None) -> ResourceMeta | "ERROR: an approval dialog is open — the user is deciding; " "wait for their decision before opening screens" ) + if isinstance(self.screen, DescribeScreen): + # Same user-priority rule as agent_navigate/agent_drill_down + # (and the docs/agent.md follow contract): a describe screen on + # top is being read — covering it with another would replace + # the content mid-read. User action takes priority. + return ( + "ERROR: a describe screen is open — the user is reading it; " + "ask them to close it (Esc) before opening another" + ) if self._get_manifest is None: return "ERROR: describe unavailable in this session" meta = self.aliases.get(kind.strip().lower()) diff --git a/tests/agent/test_profiles.py b/tests/agent/test_profiles.py index e5865b51..05a7d8a5 100644 --- a/tests/agent/test_profiles.py +++ b/tests/agent/test_profiles.py @@ -10,7 +10,8 @@ AgentProfile, build_profile, ) -from korvid.agent.runtime import MAX_HISTORY_CHARS, SYSTEM_PROMPT, UI_DRIVE_PROMPT +from korvid.agent.prompts import SYSTEM_PROMPT, UI_DRIVE_PROMPT +from korvid.agent.runtime import MAX_HISTORY_CHARS from korvid.tools.executor import READ_TOOLS, RESIZE_TOOLS, UI_TOOLS, WRITE_TOOLS @@ -102,6 +103,39 @@ def test_small_profile_prompt_has_example_and_grounding_rules() -> None: assert len(profile.system_prompt) < len(SYSTEM_PROMPT) + len(UI_DRIVE_PROMPT) * 2 +def test_small_profile_prompt_pins_tools_only_and_recovery_rules() -> None: + """Observed small-model failures (issue: 404 loops): stale names reused + from earlier turns, name/namespace pairs mixed across resources, and + describe calls issued without listing first. The prompt must pin the + tools-only boundary, list-before-inspect grounding, and 404 recovery - + asserted as the defining phrases, not independent tokens a deleted + sentence could still satisfy.""" + profile = build_profile("small", readonly=False, resize_supported=True) + prompt = profile.system_prompt.lower() + # tools-only boundary: the whole defining clause + assert "only through the provided tools" in prompt + assert "no shell, no kubectl" in prompt + # list-before-inspect grounding, without reinforcing the composite bug: + # list rows are 'namespace/name' and must be split into the two fields + assert "list_resources first" in prompt + assert "split that into the separate namespace and name fields" in prompt + assert "never paste the combined value" in prompt + # 404 recovery: re-list, never retry the same call + assert "re-list instead of retrying" in prompt + + +def test_full_profile_prompt_pins_tools_only_and_grounding_rules() -> None: + """Same invariants for the frontier prompt: the agent explores only + through session tools and never fabricates names or namespaces - + pinned as the defining phrases.""" + prompt = SYSTEM_PROMPT.lower() + assert "only through the tools provided" in prompt + assert "no shell" in prompt + assert "never invent resource names or namespaces" in prompt + assert "paired with the namespace" in prompt + assert "404" in prompt or "notfound" in prompt + + def test_small_ui_prompt_names_only_the_offered_tools() -> None: """The model must never be told about UI capabilities it was not offered — the full UI_DRIVE_PROMPT advertises all five.""" diff --git a/tests/agent/test_runtime.py b/tests/agent/test_runtime.py index e1ebdd33..2ee34c9a 100644 --- a/tests/agent/test_runtime.py +++ b/tests/agent/test_runtime.py @@ -3,7 +3,8 @@ from typing import Any from korvid.agent.events import AgentError, TextDelta, ToolCallFinished, TurnComplete -from korvid.agent.runtime import MAX_HISTORY_TURNS, NO_WRITE_PROMPT, AgentRuntime +from korvid.agent.prompts import NO_WRITE_PROMPT +from korvid.agent.runtime import MAX_HISTORY_TURNS, AgentRuntime class ScriptedProvider: diff --git a/tests/core/test_config.py b/tests/core/test_config.py index 7bf382f4..7a261fb1 100644 --- a/tests/core/test_config.py +++ b/tests/core/test_config.py @@ -35,6 +35,18 @@ def test_explicit_agent_off_wins(tmp_path: Path) -> None: assert cfg.agent_enabled is False # explicit off switch (design doc §6.3-4) +def test_agent_follow_defaults_on_and_only_explicit_false_disables(tmp_path: Path) -> None: + """`agent.follow` mirrors the agent's cluster reads on screen; it is on + by default (small models rarely volunteer the UI tools) and only a + literal `false` disables it.""" + assert KorvidConfig().agent_follow is True + f = tmp_path / "config.yaml" + f.write_text("agent:\n provider: anthropic\n follow: false\n") + assert load_config(f).agent_follow is False + f.write_text("agent:\n provider: anthropic\n follow: banana\n") + assert load_config(f).agent_follow is True + + def test_readonly_defaults_false_and_loads_from_yaml(tmp_path: Path) -> None: assert KorvidConfig().readonly is False f = tmp_path / "config.yaml" diff --git a/tests/tools/test_executor.py b/tests/tools/test_executor.py index 1851df86..b57a05e3 100644 --- a/tests/tools/test_executor.py +++ b/tests/tools/test_executor.py @@ -47,6 +47,55 @@ def test_read_tools_all_have_type_function() -> None: assert "parameters" in tool["function"] +class _ExplodingKube: + """Fails the test if any cluster call is made.""" + + def __getattr__(self, name: str) -> Any: + raise AssertionError(f"cluster reached via {name} — slash guard must reject first") + + +@pytest.mark.parametrize( + ("tool", "args"), + [ + ("get_resource", {"kind": "pods", "name": "default/web-1", "namespace": "app"}), + ("get_events", {"kind": "pods", "name": "default/web-1", "namespace": "app"}), + ("get_logs", {"pod": "default/web-1", "namespace": "app"}), + ("diagnose_pod", {"pod": "default/web-1", "namespace": "app"}), + ], +) +async def test_slash_in_name_is_rejected_with_guidance_before_any_api_call( + tool: str, args: dict[str, Any] +) -> None: + """Small models paste 'namespace/name' composites (from row keys or + prose) as the name and burn an iteration on a 404. Kubernetes names + can never contain '/', so reject locally with wording that teaches + the model to split the two fields.""" + out = await make_executor(_ExplodingKube()).execute(tool, args) + assert out.startswith("ERROR:") + assert "never contain '/'" in out + assert "separately" in out + + +@pytest.mark.parametrize( + ("tool", "args"), + [ + ("get_resource", {"kind": "pods", "name": "web-1", "namespace": "default/web-1"}), + ("get_events", {"kind": "pods", "name": "web-1", "namespace": "default/web-1"}), + ("get_logs", {"pod": "web-1", "namespace": "default/web-1"}), + ("diagnose_pod", {"pod": "web-1", "namespace": "default/web-1"}), + ], +) +async def test_slash_in_namespace_is_rejected_symmetrically( + tool: str, args: dict[str, Any] +) -> None: + """The inverse paste also happens: the composite lands in the + namespace field. Namespace names can never contain '/' either - + same local rejection, same teaching, no API round-trip.""" + out = await make_executor(_ExplodingKube()).execute(tool, args) + assert out.startswith("ERROR:") + assert "never contain '/'" in out + + async def test_get_resource_masks_secret_data() -> None: kube = FakeKube() kube.manifest = { diff --git a/tests/tools/test_write_tools.py b/tests/tools/test_write_tools.py index 47c49bc3..03c7c42d 100644 --- a/tests/tools/test_write_tools.py +++ b/tests/tools/test_write_tools.py @@ -7,7 +7,8 @@ from typing import Any -from korvid.agent.runtime import SYSTEM_PROMPT, WRITE_PROMPT, AgentRuntime +from korvid.agent.prompts import SYSTEM_PROMPT, WRITE_PROMPT +from korvid.agent.runtime import AgentRuntime from korvid.tools.executor import ( READ_TOOLS, UI_TOOLS, diff --git a/tests/ui/test_agent_follow.py b/tests/ui/test_agent_follow.py new file mode 100644 index 00000000..7a052ec7 --- /dev/null +++ b/tests/ui/test_agent_follow.py @@ -0,0 +1,203 @@ +"""Agent follow mode: mirror the built-in agent's cluster reads on screen. + +Small local models rarely volunteer the UI tools (`open_describe`, +`open_logs`) — they call the data-returning cluster reads and answer in +text while the screen sits idle. With agent follow on (the default), each +successful read in a chat turn is mirrored through the same UIBridge +methods, exactly like MCP follow mode (issue #153) does for external reads. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator + +from korvid.agent.events import AgentEvent, ToolCallFinished, ToolCallStarted +from korvid.ui.messages import UnknownCommand +from korvid.ui.widgets.describe_screen import DescribeScreen +from tests.ui.test_agent_ui_drive import make_app + + +class _ScriptedRuntime: + """Duck-typed AgentRuntime replaying a fixed event script.""" + + def __init__(self, events: list[AgentEvent]) -> None: + self._events = events + + async def run_turn(self, text: str, screen_context: str) -> AsyncIterator[AgentEvent]: + for event in self._events: + yield event + + +def _read_events(*, ok: bool = True) -> list[AgentEvent]: + return [ + ToolCallStarted( + call_id="c1", + name="get_resource", + arguments='{"kind": "pods", "name": "web-1", "namespace": "default"}', + ), + ToolCallFinished(call_id="c1", name="get_resource", ok=ok, summary=""), + ] + + +async def test_successful_agent_read_is_mirrored_as_describe() -> None: + app = make_app() + app._agent_runtime = _ScriptedRuntime(_read_events()) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + await app._run_agent_turn("what is wrong with web-1?") + await pilot.pause() + assert isinstance(app.screen, DescribeScreen) + + +async def test_failed_agent_read_is_not_mirrored() -> None: + """A 404'd read must not steer the screen to a view it never loaded.""" + app = make_app() + app._agent_runtime = _ScriptedRuntime(_read_events(ok=False)) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + await app._run_agent_turn("what is wrong with web-1?") + await pilot.pause() + assert not isinstance(app.screen, DescribeScreen) + + +async def test_agent_follow_off_disables_mirroring() -> None: + app = make_app() + app._agent_runtime = _ScriptedRuntime(_read_events()) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + app._agent_follow = False + await app._run_agent_turn("what is wrong with web-1?") + await pilot.pause() + assert not isinstance(app.screen, DescribeScreen) + + +async def test_list_read_mirrors_as_navigation() -> None: + app = make_app() + events: list[AgentEvent] = [ + ToolCallStarted(call_id="c1", name="list_resources", arguments='{"kind": "deployments"}'), + ToolCallFinished(call_id="c1", name="list_resources", ok=True, summary=""), + ] + app._agent_runtime = _ScriptedRuntime(events) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + await app._run_agent_turn("list deployments") + await pilot.pause() + assert app.current_kind == "deployments" + + +async def test_malformed_tool_arguments_do_not_break_the_turn() -> None: + """Small models emit broken JSON: the mirror is skipped, never raised.""" + app = make_app() + events: list[AgentEvent] = [ + ToolCallStarted(call_id="c1", name="get_resource", arguments='{"kind": broken'), + ToolCallFinished(call_id="c1", name="get_resource", ok=True, summary=""), + ] + app._agent_runtime = _ScriptedRuntime(events) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + await app._run_agent_turn("show web-1") + await pilot.pause() + assert not isinstance(app.screen, DescribeScreen) + + +async def test_ai_follow_command_toggles_state() -> None: + app = make_app() + app._agent_available = True # command routing gates on availability + async with app.run_test() as pilot: + await pilot.pause() + assert app._agent_follow is True # default on + app.on_unknown_command(UnknownCommand("ai follow off")) + assert app._agent_follow is False + app.on_unknown_command(UnknownCommand("ai follow")) # bare toggle + assert app._agent_follow is True + + +def test_agent_follow_config_defaults_on() -> None: + from korvid.core.config import KorvidConfig + + assert KorvidConfig().agent_follow is True + + +async def test_mirror_refuses_to_cover_a_describe_screen_the_user_is_reading() -> None: + """docs/agent.md contract: 'a mirror is refused while … a describe + screen you are reading is open'. The user opens a describe modal while + a turn is in flight - a successful get_resource must not push another + describe over it.""" + app = make_app() + app._agent_runtime = _ScriptedRuntime(_read_events()) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + # The user is already reading a describe screen (e.g. pressed `d` + # after hiding the chat panel) when the agent's read lands. + first = await app.agent_open_describe("pods", "web-2", "default") + assert not first.startswith("ERROR:") + await pilot.pause() + assert isinstance(app.screen, DescribeScreen) + reading = app.screen + await app._run_agent_turn("what is wrong with web-1?") + await pilot.pause() + assert app.screen is reading # the user's screen was not covered + + +async def test_mirror_routes_through_the_injected_serialized_bridge() -> None: + """Agent-follow mirrors must go through the shared `_UIBridgeProxy` + (the composition root's serialized bridge), not a fresh AppUIBridge: + the proxy's lock is what keeps agent and MCP UI operations - log-pane + swaps, describes - from interleaving.""" + import asyncio + + from korvid.__main__ import _UIBridgeProxy + from korvid.ui.app import AppUIBridge + + from .waits import until + + app = make_app() + proxy = _UIBridgeProxy() + app._agent_follow_bridge = proxy # ctor param in production wiring + app._agent_runtime = _ScriptedRuntime(_read_events()) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + proxy.target = AppUIBridge(app) + # An in-flight MCP UI operation holds the proxy's lock: the mirror + # must queue behind it instead of interleaving. + await proxy._lock.acquire() + turn = asyncio.create_task(app._run_agent_turn("what is wrong with web-1?")) + # Condition polling (AGENTS.md): wait until the mirror is really + # blocked on the lock before asserting nothing landed. + await until( + pilot, + lambda: bool(getattr(proxy._lock, "_waiters", None)), + label="mirror queued on the proxy lock", + ) + assert not isinstance(app.screen, DescribeScreen) # still queued + proxy._lock.release() + await turn + await pilot.pause() + assert isinstance(app.screen, DescribeScreen) # landed after the lock + + +async def test_log_mirror_refuses_while_a_describe_screen_is_open() -> None: + """Same user-priority rule for the get_logs mapping: opening logs + tears down the streams beneath the describe modal the user is + reading - the mirror must refuse, not swap them.""" + app = make_app() + events: list[AgentEvent] = [ + ToolCallStarted( + call_id="c1", + name="get_logs", + arguments='{"pod": "web-1", "namespace": "default"}', + ), + ToolCallFinished(call_id="c1", name="get_logs", ok=True, summary=""), + ] + app._agent_runtime = _ScriptedRuntime(events) # type: ignore[assignment] # fake + async with app.run_test() as pilot: + await pilot.pause() + first = await app.agent_open_describe("pods", "web-2", "default") + assert not first.startswith("ERROR:") + await pilot.pause() + reading = app.screen + assert isinstance(reading, DescribeScreen) + await app._run_agent_turn("show me web-1 logs") + await pilot.pause() + assert app.screen is reading # the modal kept focus + assert not app._log_pane.display # no stream opened beneath it diff --git a/tests/ui/test_agent_wiring.py b/tests/ui/test_agent_wiring.py index 40567bf3..dfa78d34 100644 --- a/tests/ui/test_agent_wiring.py +++ b/tests/ui/test_agent_wiring.py @@ -16,6 +16,7 @@ from korvid.ui.app import KorvidApp from korvid.ui.messages import AgentPromptSubmitted from korvid.ui.widgets.agent_panel import AgentPanel +from tests.ui.waits import until def _pod(name: str) -> PodSummary: @@ -131,6 +132,36 @@ async def test_screen_context_includes_current_view() -> None: assert "scope=default" in ctx +async def test_screen_context_splits_selected_namespace_from_name() -> None: + """Row keys are 'namespace/name' composites; fed verbatim as + `selected=` they teach the model to paste the whole string as a pod + name (observed: get_resource name='default/otel-…' -> 404). The + context must hand the model the two fields it actually needs.""" + runtime = StubRuntime([TurnComplete(input_tokens=0, output_tokens=0, estimated=True)]) + app = make_app(runtime) + async with app.run_test() as pilot: + # Wait for the watch to land the row: on an empty table the context + # reads `selected=-` and this test would pass without exercising + # the split (review on #172). + await until( + pilot, + lambda: ( + "selected=web-1" in app._screen_context() + or "selected=default/web-1" in app._screen_context() + ), + label="pod row selected", + ) + await pilot.press("ctrl+a") + inp = app.query_one(AgentPanel).query_one("#agent-input", Input) + inp.value = "q" + await pilot.press("enter") + await until(pilot, lambda: runtime.calls, label="agent turn started") + ctx = runtime.calls[0][1] + assert "selected=web-1" in ctx + assert "selected_ns=default" in ctx + assert "selected=default/web-1" not in ctx + + async def test_second_submit_ignored_while_turn_running() -> None: runtime = StubRuntime([TextDelta(text="thinking")], block=True) app = make_app(runtime)