mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-03 04:49:26 +00:00
Tool-request prompts fail closed on installability
TOOL_REQUESTED now carries the registry's verdict (installable/version/summary). GUI offers Install only when the event says a pinned build exists.
This commit is contained in:
+15
-1
@@ -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),
|
||||
|
||||
@@ -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", {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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 || "",
|
||||
},
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user