diff --git a/coworker/engine.py b/coworker/engine.py index 561eee11..1e393199 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -20,6 +20,7 @@ from enum import Enum from typing import Any, AsyncIterator, Awaitable, Callable, Optional from . import compaction as _compaction +from . import toolchain as _toolchain from .events import Event, EventType from .permissions import Mode, PermissionEngine from .providers import AssistantTurn, ProviderClient, ToolCall @@ -963,7 +964,20 @@ class TurnEngine: ), } else: - yield Event(EventType.TOOL_REQUESTED, {"name": name, "reason": reason}) + # The prompt must say up front whether WE can install this (pinned build for + # this platform) — a card that offers Install for a tool we can't fetch turns + # the user's approval into a guaranteed error. Absence of metadata means NO. + info = _toolchain.describe(name) + yield Event( + EventType.TOOL_REQUESTED, + { + "name": name, + "reason": reason, + "installable": info is not None, + "version": (info or {}).get("version", ""), + "summary": (info or {}).get("summary", ""), + }, + ) self._audit(tool_call, stage="tool_requested", reason=reason) result = await self._interruptible( self.tool_requester(dict(args), tool_call.id), diff --git a/surfaces/gui/e2e/fixtures.ts b/surfaces/gui/e2e/fixtures.ts index 9e7527f1..f1da713e 100644 --- a/surfaces/gui/e2e/fixtures.ts +++ b/surfaces/gui/e2e/fixtures.ts @@ -610,6 +610,16 @@ export async function mockApi(page: import("@playwright/test").Page) { }); return; // suspended on the approval } + // The pre-fix payload shape (owner-hit 2026-08-14): no installable/version/summary + // — an older sidecar, or any surface that forgets the field. Must render NOT + // installable, never a guessed Install offer. + if (/request an unpinned tool/i.test(msg.text)) { + send("tool_requested", { + name: "somescanner", + reason: "scan the Terraform for misconfigurations", + }); + return; // suspended on the tool request + } // OPE-85: the agent hits a missing scanner and asks instead of skipping the check. if (/scan for secrets/i.test(msg.text)) { send("tool_requested", { diff --git a/surfaces/gui/e2e/toolreq.spec.ts b/surfaces/gui/e2e/toolreq.spec.ts index 67f47def..b3a879cf 100644 --- a/surfaces/gui/e2e/toolreq.spec.ts +++ b/surfaces/gui/e2e/toolreq.spec.ts @@ -23,6 +23,21 @@ test("request_tool surfaces a card naming the tool, the reason and the pinned ve await expect(card.getByTestId("toolreq-skip")).toBeVisible(); }); +test("an event without install metadata fails CLOSED — Install disabled, skip offered", async ({ + page, +}) => { + // Owner-hit 2026-08-14: the card offered "pinned build, checksum-verified" for a tool + // with no pinned build; approval could only produce an error. Absence of metadata is NO. + await page.goto("/"); + await page.getByPlaceholder(/Ask the coworker/).fill("request an unpinned tool"); + await page.getByRole("button", { name: "Send" }).click(); + const card = page.locator(".dirreq-card"); + await expect(card).toContainText("somescanner"); + await expect(card).toContainText(/no verified build/i); + await expect(card.getByTestId("toolreq-install")).toBeDisabled(); + await expect(card.getByTestId("toolreq-skip")).toBeEnabled(); +}); + test("installing runs the check; skipping still reports coverage", async ({ page }) => { await ask(page); await page.getByTestId("toolreq-install").click(); diff --git a/surfaces/gui/src/App.tsx b/surfaces/gui/src/App.tsx index aa7d7cf4..e9ae5662 100644 --- a/surfaces/gui/src/App.tsx +++ b/surfaces/gui/src/App.tsx @@ -737,7 +737,8 @@ export function App() { kind: "toolreq", tool: d.name || "", reason: d.reason || "", - installable: d.installable !== false, + // Fail CLOSED: only offer Install when the event says a pinned build exists. + installable: d.installable === true, version: d.version || "", summary: d.summary || "", }, diff --git a/tests/test_tool_request.py b/tests/test_tool_request.py index 309d6a10..8da512c5 100644 --- a/tests/test_tool_request.py +++ b/tests/test_tool_request.py @@ -17,8 +17,9 @@ from coworker.tools import ToolRegistry class ScriptedProvider(ProviderClient): """One turn that calls request_tool, then a plain reply.""" - def __init__(self): + def __init__(self, tool: str = "gitleaks"): self.calls = 0 + self.tool = tool def complete(self, *, model, messages, tools=None, **settings): self.calls += 1 @@ -29,7 +30,7 @@ class ScriptedProvider(ProviderClient): ToolCall( id="t1", name="request_tool", - arguments={"name": "gitleaks", "reason": "scan history for secrets"}, + arguments={"name": self.tool, "reason": "scan history for secrets"}, ) ], ) @@ -39,9 +40,9 @@ class ScriptedProvider(ProviderClient): return ModelCapabilities(tools=True) -def _engine(tmp_path, requester): +def _engine(tmp_path, requester, tool: str = "gitleaks"): return TurnEngine( - provider=ScriptedProvider(), + provider=ScriptedProvider(tool), registry=ToolRegistry(), permissions=PermissionEngine(workspace_root=tmp_path, mode=Mode.INTERACTIVE), model="m", @@ -85,6 +86,30 @@ async def test_declining_tells_the_agent_to_fall_back_openly(tmp_path): assert "degraded" in body or "fallback" in body +@pytest.mark.asyncio +async def test_event_tells_the_truth_about_installability(tmp_path, monkeypatch): + """Owner-hit 2026-08-14: the card offered Install for a tool with no pinned build — + the surface guessed because the event said nothing. The event must carry the + registry's verdict, and no metadata means NO.""" + from coworker import toolchain + + monkeypatch.setattr(toolchain, "_platform_key", lambda: "darwin_arm64") + + async def requester(args, tool_call_id=None): + return {"installed": False, "reason": "declined"} + + events = await _run(_engine(tmp_path, requester, tool="gitleaks")) + data = [e for e in events if e.type is EventType.TOOL_REQUESTED][0].data + assert data["installable"] is True + assert data["version"] == toolchain.MANAGED["gitleaks"].version + assert data["summary"] + + events = await _run(_engine(tmp_path, requester, tool="not-a-managed-tool")) + data = [e for e in events if e.type is EventType.TOOL_REQUESTED][0].data + assert data["installable"] is False + assert data["version"] == "" and data["summary"] == "" + + @pytest.mark.asyncio async def test_no_requester_still_returns_guidance(tmp_path): """Headless surfaces have nobody to ask — the agent must still be told to disclose