Tool-request card: separate the product's facts from the coworker's ask

Registry metadata (version, publisher, checksum) moves to a distinct fact strip.
Decline button renamed to say the run continues; reason capped to one sentence.
This commit is contained in:
Rohit C Prasad
2026-08-14 23:50:51 -07:00
committed by Rohit P
parent b866157778
commit 8e77d61aa1
11 changed files with 53 additions and 15 deletions
+1
View File
@@ -976,6 +976,7 @@ class TurnEngine:
"installable": info is not None, "installable": info is not None,
"version": (info or {}).get("version", ""), "version": (info or {}).get("version", ""),
"summary": (info or {}).get("summary", ""), "summary": (info or {}).get("summary", ""),
"source": (info or {}).get("source", ""),
}, },
) )
self._audit(tool_call, stage="tool_requested", reason=reason) self._audit(tool_call, stage="tool_requested", reason=reason)
+1
View File
@@ -1717,6 +1717,7 @@ def create_app(manager: SessionManager) -> FastAPI:
"version": (info or {}).get("version", ""), "version": (info or {}).get("version", ""),
"summary": (info or {}).get("summary", ""), "summary": (info or {}).get("summary", ""),
"url": (info or {}).get("url", ""), "url": (info or {}).get("url", ""),
"source": (info or {}).get("source", ""),
}, },
tool_call_id=tool_call_id, tool_call_id=tool_call_id,
) )
+5 -2
View File
@@ -18,8 +18,11 @@ from aisuite.agents import ToolMetadata, tool
def request_tool_tool() -> object: def request_tool_tool() -> object:
def request_tool(name: str, reason: str) -> dict: def request_tool(name: str, reason: str) -> dict:
"""Ask the user to install a command-line tool you need but can't find on this """Ask the user to install a command-line tool you need but can't find on this
machine (e.g. `gitleaks`, `osv-scanner`, `semgrep`). Say in `reason` what check it machine (e.g. `gitleaks`, `osv-scanner`, `semgrep`).
unlocks, so the user can judge whether it's worth installing.
Keep `reason` to ONE sentence: which check needs the tool. The prompt the user
sees already explains what the install is (pinned version, publisher, checksum)
and what happens if they decline don't restate any of that in `reason`.
Use this INSTEAD of quietly skipping a check. If the user declines, carry on with a Use this INSTEAD of quietly skipping a check. If the user declines, carry on with a
fallback (e.g. reading git history yourself instead of running gitleaks) and state fallback (e.g. reading git history yourself instead of running gitleaks) and state
+1
View File
@@ -628,6 +628,7 @@ export async function mockApi(page: import("@playwright/test").Page) {
installable: true, installable: true,
version: "8.30.1", version: "8.30.1",
summary: "scans git history and the working tree for committed secrets", summary: "scans git history and the working tree for committed secrets",
source: "github.com/gitleaks",
}); });
return; // suspended on the tool request return; // suspended on the tool request
} }
+8 -4
View File
@@ -17,10 +17,14 @@ test("request_tool surfaces a card naming the tool, the reason and the pinned ve
const card = page.locator(".dirreq-card"); const card = page.locator(".dirreq-card");
await expect(card).toContainText("gitleaks"); await expect(card).toContainText("gitleaks");
await expect(card).toContainText("scan the git history for committed secrets"); await expect(card).toContainText("scan the git history for committed secrets");
await expect(card).toContainText("8.30.1"); // The fact strip is the product's voice: version, publisher, checksum — kept apart from
await expect(card).toContainText(/checksum-verified/i); // the coworker's quoted reason (mixing them is what made the card confusing, 2026-08-14).
// Declining must read as a normal choice, not a failure. const facts = card.locator(".toolreq-facts");
await expect(card.getByTestId("toolreq-skip")).toBeVisible(); await expect(facts).toContainText("8.30.1");
await expect(facts).toContainText(/checksum-verified/i);
await expect(facts).toContainText("from github.com/gitleaks");
// Declining must read as a normal choice that continues the run, not a failure.
await expect(card.getByTestId("toolreq-skip")).toHaveText("Continue without it");
}); });
test("an event without install metadata fails CLOSED — Install disabled, skip offered", async ({ test("an event without install metadata fails CLOSED — Install disabled, skip offered", async ({
+1
View File
@@ -741,6 +741,7 @@ export function App() {
installable: d.installable === true, installable: d.installable === true,
version: d.version || "", version: d.version || "",
summary: d.summary || "", summary: d.summary || "",
source: d.source || "",
}, },
]); ]);
break; break;
@@ -22,23 +22,30 @@ export function ToolRequestCard({
</span> </span>
</div> </div>
{item.reason && <div className="dirreq-reason">{item.reason}</div>} {item.reason && <div className="dirreq-reason">{item.reason}</div>}
{/* The fact strip is the PRODUCT speaking (registry metadata), styled apart from the
coworker's quoted ask above mixing the two voices is what made the card confusing. */}
{item.installable ? ( {item.installable ? (
<div className="dirreq-reason"> <div className="toolreq-facts">
{item.summary ? `${item.summary}. ` : ""} <code>
Installs {item.tool} {item.tool}
{item.version ? ` ${item.version}` : ""} a pinned build, checksum-verified before {item.version ? ` ${item.version}` : ""}
it runs. </code>
{item.summary && <span className="toolreq-fact">{item.summary}</span>}
<span className="toolreq-fact">pinned &amp; checksum-verified</span>
{item.source && <span className="toolreq-fact">from {item.source}</span>}
</div> </div>
) : ( ) : (
<div className="dirreq-reason"> <div className="toolreq-facts">
No verified build is available for this machine install it yourself if you want <span className="toolreq-fact">
this check, or skip and the coworker will note the gap. No verified build is available for this machine install it yourself if you want
this check, or continue and the coworker will note the gap.
</span>
</div> </div>
)} )}
<div className="dirreq-actions"> <div className="dirreq-actions">
<span className="spacer" /> <span className="spacer" />
<button className="btn" data-testid="toolreq-skip" onClick={() => onRespond(false)}> <button className="btn" data-testid="toolreq-skip" onClick={() => onRespond(false)}>
Skip this check Continue without it
</button> </button>
<button <button
className="btn primary" className="btn primary"
+13
View File
@@ -794,6 +794,19 @@ button.btn.danger { color: var(--accent); }
.dirreq-head { display: flex; align-items: center; gap: 8px; font-weight: 600; font-size: 13.5px; color: var(--ink); } .dirreq-head { display: flex; align-items: center; gap: 8px; font-weight: 600; font-size: 13.5px; color: var(--ink); }
.dirreq-head .ico { color: var(--accent); } .dirreq-head .ico { color: var(--accent); }
.dirreq-reason { font-size: 13px; color: var(--muted); font-style: italic; margin: 6px 0 10px; } .dirreq-reason { font-size: 13px; color: var(--muted); font-style: italic; margin: 6px 0 10px; }
/* request_tool fact strip the product's own metadata (version, publisher, checksum),
deliberately NOT italic so it can't be misread as the coworker still talking */
.toolreq-facts {
display: flex; flex-wrap: wrap; align-items: baseline; gap: 4px 10px;
font-size: 12.5px; color: var(--muted); font-style: normal;
background: var(--paper); border: 1px solid var(--line); border-radius: 8px;
padding: 7px 10px; margin: 2px 0 10px;
}
.toolreq-facts code { color: var(--ink); font-size: 12.5px; }
.toolreq-fact + .toolreq-fact::before,
.toolreq-facts code + .toolreq-fact::before { content: "· "; color: var(--muted); }
/* a disabled Install must LOOK disabled — the whole card exists to not oversell */
.dirreq-actions .btn:disabled { opacity: 0.45; cursor: not-allowed; }
.dirreq-pathrow { display: flex; gap: 8px; align-items: center; } .dirreq-pathrow { display: flex; gap: 8px; align-items: center; }
.dirreq-path { .dirreq-path {
flex: 1; font: inherit; font-size: 13px; color: var(--ink); flex: 1; font: inherit; font-size: 13px; color: var(--ink);
+1
View File
@@ -136,6 +136,7 @@ export type Item =
installable?: boolean; installable?: boolean;
version?: string; version?: string;
summary?: string; summary?: string;
source?: string;
resolved?: "installed" | "skipped"; resolved?: "installed" | "skipped";
} }
| { | {
+1
View File
@@ -103,6 +103,7 @@ async def test_event_tells_the_truth_about_installability(tmp_path, monkeypatch)
assert data["installable"] is True assert data["installable"] is True
assert data["version"] == toolchain.MANAGED["gitleaks"].version assert data["version"] == toolchain.MANAGED["gitleaks"].version
assert data["summary"] assert data["summary"]
assert data["source"] == "github.com/gitleaks"
events = await _run(_engine(tmp_path, requester, tool="not-a-managed-tool")) 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 data = [e for e in events if e.type is EventType.TOOL_REQUESTED][0].data
+5
View File
@@ -87,6 +87,7 @@ def test_describe_surfaces_what_the_user_is_approving(monkeypatch):
info = toolchain.describe("gitleaks") info = toolchain.describe("gitleaks")
assert info and info["version"] and info["sha256"] and info["url"] assert info and info["version"] and info["sha256"] and info["url"]
assert "secret" in info["summary"].lower() assert "secret" in info["summary"].lower()
assert info["source"] == "github.com/gitleaks" # publisher, human-readable
def test_install_refuses_a_tampered_download(tmp_path, monkeypatch): def test_install_refuses_a_tampered_download(tmp_path, monkeypatch):
@@ -155,3 +156,7 @@ def test_install_writes_a_verified_binary(tmp_path, monkeypatch):
monkeypatch.setenv("PATH", "") monkeypatch.setenv("PATH", "")
monkeypatch.setattr(toolchain, "_KNOWN_DIRS", ()) monkeypatch.setattr(toolchain, "_KNOWN_DIRS", ())
assert toolchain.resolve("osv-scanner") == path assert toolchain.resolve("osv-scanner") == path
# And linked under the stable bin dir, so a shell with that dir on PATH picks the
# tool up by name the moment the install finishes — no respawn, no full paths.
linked = toolchain.bin_dir() / "osv-scanner"
assert linked.exists() and open(linked, "rb").read() == payload