trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns13g rEventsFlows

MCP `steer` refuses in the run page's words, not `refused: <code>` #7610

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/7598-mcp-steer-refuses-in-the-run-page-s-word → main 4 commits fork

  • Merged. scout/7598-mcp-steer-refuses-in-the-run-page-s-word was merged into main as 9b58c63.

Review

Sign in to review this change.
7 files changed +104 −22
src/actions/steer_words.ts added +20
@@ -0,0 +1,20 @@
1 /**
2 * What a refused steer says (#7598): one sentence per code `steerRepoRun`
3 * returns, so the run page's steer box and MCP `steer` word it the same.
4 */
5 import { SAY_SOMETHING_WORDS } from "./tell_words.ts";
6 import { VERIFY_FIRST } from "./verify_words.ts";
7
8 const REFUSED: Readonly<Record<string, string>> = {
9 empty: SAY_SOMETHING_WORDS,
10 not_agent: "Only an agent's run can be steered.",
11 not_found: "There is no such run here.",
12 unauthorized: "Sign in to steer a run.",
13 unverified: `${VERIFY_FIRST[0]!.toUpperCase()}${VERIFY_FIRST.slice(1)} to steer a run.`,
14 forbidden: "You need write access to steer a run.",
15 };
16
17 /** The sentence for a refused steer; an unknown code reads as no access. */
18 export function steerRefusedWords(code: string): string {
19 return REFUSED[code] ?? REFUSED["forbidden"]!;
20 }
src/http/mcp/tools/steer.ts +2 −7
@@ -1,4 +1,5 @@
11 import { STEER_SAYS, steerRepoRun } from "../../../actions/runs.ts";
2 import { steerRefusedWords } from "../../../actions/steer_words.ts";
23 import { good, bad, type Tool, str, REPO_ARG, repoArg, isAnswer } from "../shared.ts";
34
45 export const tool: Tool = {
@@ -15,13 +16,7 @@ export const tool: Tool = {
1516 const repo = repoArg(ctx, user, args);
1617 if (isAnswer(repo)) return repo;
1718 const steered = steerRepoRun(ctx.app, user, repo, str(args["run"]), str(args["text"]));
18 if (!steered.ok)
19 return bad(
20 steered.error,
21 steered.error === "empty" || steered.error === "not_agent"
22 ? "say something, to an agent's run"
23 : `refused: ${steered.error}`,
24 );
19 if (!steered.ok) return bad(steered.error, steerRefusedWords(steered.error));
2520 return good({
2621 says: STEER_SAYS[steered.value],
2722 outcome: steered.value,
src/tui/screens/runlist.ts +3 −4
@@ -11,7 +11,7 @@
1111 */
1212
1313 import { STEER_SAYS, steerRepoRun } from "../../actions/runs.ts";
14 import { SAY_SOMETHING_WORDS } from "../../actions/tell_words.ts";
14 import { steerRefusedWords } from "../../actions/steer_words.ts";
1515 import { placementWords } from "../../checks/placement.ts";
1616 import { getIssuesByIds, type Issue } from "../../core/issues.ts";
1717 import { canWrite, type Repo } from "../../core/repos.ts";
@@ -25,7 +25,6 @@ import type { Session } from "../session.ts";
2525 import {
2626 ListState,
2727 drawList,
28 errorText,
2928 hitList,
3029 metaOf,
3130 promptScreen,
@@ -219,8 +218,8 @@ export function steerPrompt(
219218 onSubmit: (text) => {
220219 const steered = steerRepoRun(f.app, f.user, repo, run.id, text);
221220 if (!steered.ok) {
222 if (steered.error === "empty") return SAY_SOMETHING_WORDS;
223 ctx.toast(`Could not steer: ${errorText(steered.error)}.`, "danger");
221 if (steered.error === "empty") return steerRefusedWords("empty");
222 ctx.toast(steerRefusedWords(steered.error), "danger");
224223 return undefined;
225224 }
226225 ctx.toast(`${steered.value === "sent" ? "Sent" : "Commented"}. ${STEER_SAYS[steered.value]}`);
src/web/pages/runs.ts +2 −7
@@ -11,7 +11,7 @@
1111 * id, so the redirect itself says nothing about what exists.
1212 */
1313
14 import { SAY_SOMETHING_WORDS } from "../../actions/tell_words.ts";
14 import { steerRefusedWords } from "../../actions/steer_words.ts";
1515 import { readLooks } from "../../checks/looks.ts";
1616 import type { Database } from "bun:sqlite";
1717 import {
@@ -347,12 +347,7 @@ export async function runSteer(ctx: Ctx): Promise<Response> {
347347 if (steered.error === "not_found") return notFoundPage(ctx);
348348 return flashTo(ctx, dest, {
349349 tone: "danger",
350 message:
351 steered.error === "empty"
352 ? SAY_SOMETHING_WORDS
353 : steered.error === "not_agent"
354 ? "Only an agent's run can be steered."
355 : "You need write access to steer a run.",
350 message: steerRefusedWords(steered.error),
356351 });
357352 }
358353 return flashTo(ctx, dest, {
test/mcp-steer.test.ts +32
@@ -7,6 +7,7 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test";
77 import { mkdtemp, rm } from "node:fs/promises";
88 import { tmpdir } from "node:os";
99 import { join } from "node:path";
10 import { steerRefusedWords } from "../src/actions/steer_words.ts";
1011 import { createApp, type App } from "../src/app.ts";
1112 import { newApp } from "../src/actions/apps.ts";
1213 import { openIssue } from "../src/actions/issues.ts";
@@ -106,4 +107,35 @@ describe("MCP steer", () => {
106107 expect(said()).not.toContain("no");
107108 expect(said()).not.toContain("reader words");
108109 });
110
111 test("each refusal is worded as the run page's steer box words it", async () => {
112 const said = (r: { error?: string }, code: string) => {
113 expect(r.error).toContain(code);
114 expect(r.error).toContain(steerRefusedWords(code));
115 };
116 said(await tool(bobRw, { repo: "alice/hello", run: agentRun.id, text: "x" }), "forbidden");
117 said(await tool(aliceRw, { repo: "alice/hello", run: agentRun.id, text: " " }), "empty");
118 said(await tool(aliceRw, { repo: "alice/hello", run: deployRun.id, text: "hi" }), "not_agent");
119 said(await tool(aliceRw, { repo: "alice/hello", run: "nope", text: "hi" }), "not_found");
120 // The run page: the flash it redirects with carries the same sentence.
121 for (const [token, code] of [[bobRw, "forbidden"], [aliceRw, "empty"]] as const) {
122 const page = await fetch(`${servers.url}/alice/hello/runs/${agentRun.id}/steer`, {
123 method: "POST",
124 redirect: "manual",
125 headers: {
126 authorization: `Bearer ${token}`,
127 "content-type": "application/x-www-form-urlencoded",
128 origin: servers.url,
129 },
130 body: new URLSearchParams({ text: code === "empty" ? " " : "x" }),
131 });
132 expect(page.status).toBe(303);
133 const flash = decodeURIComponent(
134 `${page.headers.get("location")} ${page.headers.get("set-cookie")}`.replace(/\+/g, " "),
135 );
136 expect(flash).toContain(steerRefusedWords(code));
137 }
138 for (const code of ["empty", "not_agent", "not_found", "unauthorized", "unverified", "forbidden"])
139 expect(steerRefusedWords(code)).not.toContain(code);
140 });
109141 });
test/tell-words.test.ts +18 −2
@@ -21,10 +21,10 @@ const SENTENCES = {
2121 };
2222
2323 const DOORS = [
24 "src/actions/steer_words.ts",
2425 "src/web/studio/actions.ts",
2526 "src/tui/screens/tell.ts",
2627 "src/tui/screens/report.ts",
27 "src/web/pages/runs.ts",
2828 "src/web/studio/toolbar.ts",
2929 "src/web/client.ts",
3030 ];
@@ -40,7 +40,7 @@ describe("tell words", () => {
4040 test(`${door} types none of them out`, () => {
4141 const text = read(door);
4242 for (const sentence of Object.values(SENTENCES)) expect(text).not.toContain(sentence);
43 expect(text).toContain("actions/tell_words.ts");
43 expect(text).toMatch(/tell_words\.ts/);
4444 });
4545 }
4646
@@ -48,4 +48,20 @@ describe("tell words", () => {
4848 const { shellJs } = await import("../src/web/client.ts");
4949 expect(shellJs).toContain(`note.textContent = '${NOT_TOLD_WORDS}';`);
5050 });
51
52 const STEER_DOORS = ["src/web/pages/runs.ts", "src/tui/screens/runlist.ts", "src/http/mcp/tools/steer.ts"];
53 for (const door of STEER_DOORS) {
54 test(`${door} reads the steer refusals from steer_words.ts, typing none out`, () => {
55 const text = read(door);
56 for (const sentence of [
57 "Only an agent's run can be steered.",
58 "You need write access to steer a run.",
59 "Sign in to steer a run.",
60 "There is no such run here.",
61 ...Object.values(SENTENCES),
62 ])
63 expect(text).not.toContain(sentence);
64 expect(text).toContain("actions/steer_words.ts");
65 });
66 }
5167 });
test/tui-steer.test.ts +27 −2
@@ -3,16 +3,17 @@ import { afterAll, beforeAll, expect, test } from "bun:test";
33 import { mkdtemp, rm } from "node:fs/promises";
44 import { tmpdir } from "node:os";
55 import { join } from "node:path";
6 import { steerRefusedWords } from "../src/actions/steer_words.ts";
67 import { addToCrew, createAgent } from "../src/actions/crew.ts";
78 import { openIssue } from "../src/actions/issues.ts";
89 import { openPull } from "../src/actions/pulls.ts";
910 import { commitToBranch, createRepo as makeRepo } from "../src/actions/repos.ts";
1011 import { createApp, type App as Forge } from "../src/app.ts";
1112 import { listComments, type Issue } from "../src/core/issues.ts";
12 import { addTeamMember, createOrg, createTeam } from "../src/core/orgs.ts";
13 import { addTeamMember, createOrg, createTeam, removeTeamMember } from "../src/core/orgs.ts";
1314 import { canWrite, createRepo, type Repo } from "../src/core/repos.ts";
1415 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
15 import type { User } from "../src/core/users.ts";
16 import { createUser, type User } from "../src/core/users.ts";
1617 import { listenForSteer } from "../src/crew/runner.ts";
1718 import { createForgeApp } from "../src/tui/forge.ts";
1819 import { issueScreen } from "../src/tui/screens/issue.ts";
@@ -222,3 +223,27 @@ test("an agent session that may write is not offered steering", async () => {
222223 stop();
223224 h.app.stop();
224225 });
226
227 test("a refusal reads as the run page and MCP read it", async () => {
228 const { repo, issue } = await seed();
229 const c = await createUser(forge.db, {
230 origin: "seed",
231 trusted: true,
232 username: "carol",
233 email: "[email protected]",
234 password: "a-long-passphrase",
235 });
236 if (!c.ok) throw new Error("carol failed");
237 const writers = createTeam(forge.db, orgId, "Carol's", "write");
238 if (!writers.ok) throw new Error("team failed");
239 addTeamMember(forge.db, writers.value, c.value.id);
240 const h = await open(c.value, (s, ctx) => runsScreen(s, ctx, repo));
241 await h.press("S");
242 await h.type("hello");
243 // Her write access goes after the prompt opened: the action refuses.
244 removeTeamMember(forge.db, writers.value, c.value.id);
245 const text = await h.press("enter");
246 expect(text).toContain(steerRefusedWords("forbidden"));
247 expect(listComments(forge.db, issue.id)).toHaveLength(0);
248 h.app.stop();
249 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs