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

MCP `asks_act` takes back a mute: `listen` brings a muted repository's asks back, as /asks/listen does #7226

Merged opened this

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

Merged

scout/forge:scout/7219-mcp-asks-act-takes-back-a-mute-listen-br → main 2 commits fork 2 threads · 2 unresolved

  • Merged. scout/7219-mcp-asks-act-takes-back-a-mute-listen-br was merged into main as 3d22725.

Review

Sign in to review this change.
2 files changed +129 −4
src/http/mcp/tools/asks_act.ts +17 −4
@@ -1,23 +1,36 @@
11 import { applyAsksBulk, previewAsksBulk } from "../../../actions/asks_bulk.ts";
2 import { good, bad, type Tool } from "../shared.ts";
2 import { mutedRepos, removeMutes } from "../../../core/interactions.ts";
3 import { good, bad, repoArg, type Tool } from "../shared.ts";
34
45 export const tool: Tool = {
56 name: "asks_act",
67 description:
7 "Decide asks (`items`): dismiss, decline, ship, close (`reason`), mute, replan",
8 "Decide asks (`items`): dismiss, decline, ship, close (`reason`), mute, replan; `listen` (`repo`) unmutes",
89 inputSchema: {
910 type: "object",
1011 properties: {
11 action: { type: "string", enum: ["dismiss", "decline", "mute", "ship", "close", "replan"] },
12 action: { type: "string", enum: ["dismiss", "decline", "mute", "listen", "ship", "close", "replan"] },
1213 items: { type: "array", items: { type: "string" }, maxItems: 100 },
14 repo: { type: "string" },
1315 reason: { type: "string", maxLength: 300 },
1416 dryRun: { type: "boolean" },
1517 },
16 required: ["action", "items"],
18 required: ["action"],
1719 },
1820 writes: true,
1921 dryByDefault: true,
2022 async run(ctx, user, args) {
23 if (args["action"] === "listen") {
24 if (typeof args["repo"] !== "string" || args["repo"].trim() === "")
25 return bad("invalid", "`listen` needs `repo`, owner/name");
26 const repo = repoArg(ctx, user, args);
27 if ("ok" in repo || !mutedRepos(ctx.app.db, user.id).has(repo.id))
28 return bad("not_found", "no such app or repository (owner/name)");
29 const name = `${repo.ownerName}/${repo.name}`;
outdated

High: this runs whatever dryRun says. The tool is dryByDefault, so plansOnly treats a call without dryRun:false as a read, and an ro token passes the scope check in mcp.ts:174 and reaches this write. Gate it: if (args["dryRun"] !== false) return good({ wouldListen: [...] }); before removeMutes.

30 if (args["dryRun"] !== false) return good({ wouldListen: [name] });
31 removeMutes(ctx.app.db, user.id, [`repo:${repo.id}`]);
32 return good({ listening: [name] });
33 }
2134 const answer =
2235 args["dryRun"] === false
2336 ? await applyAsksBulk(ctx.app, user, args["action"], args["items"], args["reason"])
test/mcp-asks-listen.test.ts added +112
@@ -0,0 +1,112 @@
1 import { afterAll, beforeAll, expect, setDefaultTimeout, test } from "bun:test";
2 import { mkdtemp, rm } from "node:fs/promises";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { createApp, type App } from "../src/app.ts";
6 import { addToCrew, createAgent } from "../src/actions/crew.ts";
7 import { assignIssue } from "../src/actions/issues.ts";
8 import { openPull } from "../src/actions/pulls.ts";
9 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
10 import { getIssueById } from "../src/core/issues.ts";
11 import { muteRepo, mutedRepos } from "../src/core/interactions.ts";
12 import type { Repo } from "../src/core/repos.ts";
13 import { createToken } from "../src/core/tokens.ts";
14 import { createUser, getUserByName, type User } from "../src/core/users.ts";
15 import { serve, type Servers } from "../src/http/server.ts";
16 import { mcpTool } from "./helpers/mcp.ts";
17 import { quietLogs } from "./helpers/globals.ts";
18
19 quietLogs();
20 setDefaultTimeout(30_000);
21
22 let dataDir = "";
23 let app: App;
24 let servers: Servers;
25 let alice: User;
26 let bob: User;
27 let repo: Repo;
28 let aliceToken = "";
29
30 const act = (args: unknown) => mcpTool(servers.url, aliceToken, "asks_act", args);
31 const askRows = async () =>
32 ((await mcpTool(servers.url, aliceToken, "asks", {})) as any).value.rows.length as number;
33
34 beforeAll(async () => {
35 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-listen-"));
36 app = createApp({ dataDir, port: 0, disk: () => ({ freeBytes: 500e9, totalBytes: 1000e9 }) });
37 servers = await serve(app);
38 for (const name of ["alice", "bob"]) {
39 const made = await createUser(app.db, {
40 origin: "seed",
41 trusted: true,
42 username: name,
43 email: `${name}@forge.local`,
44 password: "a-long-listen-passphrase",
45 adminIfFirst: name === "alice",
46 });
47 if (!made.ok) throw new Error(made.error);
48 if (name === "alice") alice = made.value;
49 else bob = made.value;
50 }
51 aliceToken = createToken(app.db, alice.id, "rw", "rw").token;
52 await createAgent(app, alice, { username: "bot" });
53 const made = await createRepo(app, alice, { name: "listen", visibility: "public" });
54 if (!made.ok) throw new Error(String(made.error));
55 repo = made.value;
56 await commitToBranch(app, alice, repo, {
57 message: "seed",
58 files: [{ path: "README.md", content: "# listen\n" }],
59 });
60 addToCrew(app, alice, repo, "bot");
61 await commitToBranch(app, alice, repo, {
62 branch: "bot/1",
63 base: "main",
64 message: "Work",
65 files: [{ path: "F.md", content: "1\n" }],
66 });
67 const opened = await openPull(app, getUserByName(app.db, "bot")!, repo, {
68 title: "Work",
69 body: "",
70 baseBranch: "main",
71 headBranch: "bot/1",
72 });
73 if (!opened.ok) throw new Error(String(opened.error));
74 expect(assignIssue(app, alice, repo, getIssueById(app.db, opened.value.id)!, alice).ok).toBe(true);
75 });
76
77 afterAll(async () => {
78 await servers.stop();
79 await rm(dataDir, { recursive: true, force: true });
80 });
81
82 test("listen brings a muted repository's asks back, for the caller only", async () => {
83 const before = await askRows();
84 expect(before).toBeGreaterThan(0);
85 muteRepo(app.db, alice.id, repo.id);
86 muteRepo(app.db, bob.id, repo.id);
87 expect(await askRows()).toBe(0);
88
89 // A call without dryRun only plans: the mute and the hidden asks stay.
90 const planned = await act({ action: "listen", repo: "alice/listen" });
outdated

This writes without dryRun: false, so the test pins the bug. Pass dryRun: false here. Add one case showing the default call leaves the mute in place, and one showing an ro token with dryRun:false is refused.

91 expect(planned.value).toEqual({ wouldListen: ["alice/listen"] });
92 expect(mutedRepos(app.db, alice.id).has(repo.id)).toBe(true);
93 expect(await askRows()).toBe(0);
94
95 // A read-only token cannot make the real write.
96 const ro = createToken(app.db, alice.id, "ro", "ro").token;
97 const refused = await mcpTool(servers.url, ro, "asks_act", { action: "listen", repo: "alice/listen", dryRun: false });
98 expect(refused.error).toBeDefined();
99 expect(mutedRepos(app.db, alice.id).has(repo.id)).toBe(true);
100
101 const heard = await act({ action: "listen", repo: "alice/listen", dryRun: false });
102 expect(heard.value).toEqual({ listening: ["alice/listen"] });
103 expect(await askRows()).toBe(before);
104 expect(mutedRepos(app.db, alice.id).has(repo.id)).toBe(false);
105 expect(mutedRepos(app.db, bob.id).has(repo.id)).toBe(true);
106 });
107
108 test("listen refuses without a repo, and answers not_found for one not muted or unknown", async () => {
109 expect((await act({ action: "listen" })).error).toContain("invalid");
110 expect((await act({ action: "listen", repo: "alice/listen", dryRun: false })).error).toContain("not_found");
111 expect((await act({ action: "listen", repo: "alice/nope" })).error).toContain("not_found");
112 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs