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

MCP `app_domains`: list, add, check and remove an app's domains, with the same rules as the API #5708

Merged opened this

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

Merged

scout/forge:scout/4753-mcp-app-domains-list-add-check-and-remov → main 23 commits fork 8 threads · 8 unresolved

  • Merged. scout/4753-mcp-app-domains-list-add-check-and-remov was merged into main as 89c6baa.

Review

Sign in to review this change.
5 files changed +179 −0
docs/MCP.md +1
@@ -36,6 +36,7 @@ for a tunnel and a forge that sleeps.
3636 | `accounts` | `{ cursor?, limit? }`: the forge's accounts, newest first: `username`, `role`, `verified`, `suspended`, `created`, and `next` (a cursor, or null); `limit` 1-50 (default 20); a person admin's | |
3737 | `agent` | `{ action, username, … }`: `create` a crew agent, `update` its `model`, `brief` or `budgetUsd`, or `remove` it; an admin's, with an `admin` token, as `/api/v1/agents` | yes |
3838 | `app_deploy` | `{ repo, env, action }`: `redeploy` or `stop` an app's `production` or a pull's `pull-N` preview; a writer's, queued as `POST …/app/:env/redeploy` and `…/stop` are; answers `{ queued, run }` or `{ stopped }` | yes |
39 | `app_domains` | `{ repo, action, host? }`: `list`, `add`, `check` (looks the DNS proof up now) or `remove` an app's own domains, with the records to publish, as `…/app/domains` answers; an admin of the repository with an admin-scope token | yes |
3940 | `app_env` | `{ repo, action, name?, value?, scope? }`: `list` an app's variable names and scopes, `set` one, or `unset` it, in `all` (default), `production` or `preview`; the API's rules and errors, an admin of the repository with an admin-scope token; a value is never read back | yes |
4041 | `app_members` | `{ repo, members?, dryRun? }`: the people a private app names, who may enter it and its previews and nothing else; a writer (never an agent) changes the list with an admin-scope token | yes |
4142 | `apps` | the apps you can see: live or not, where served, how each runs itself, what is working | |
src/http/mcp/tools/app_domains.ts added +45
@@ -0,0 +1,45 @@
1 import {
2 addAppDomain,
3 checkAppDomain,
4 listAppDomains,
5 removeAppDomain,
6 } from "../../../actions/domains.ts";
7 import { type Answer, good, bad, type Tool, REPO_ARG, repoArg, isAnswer, str } from "../shared.ts";
8
9 export const tool: Tool = {
10 name: "app_domains",
11 doc: "`{ repo, action, host? }`: `list`, `add`, `check` (looks the DNS proof up now) or `remove` an app's own domains, with the records to publish, as `…/app/domains` answers; an admin of the repository with an admin-scope token",
12 description: "App domains: list, add, check, remove.",
outdated

This tool's entry pushes tools/list to 37,176 bytes, 176 over the 37,000 headroom in test/mcp-size.test.ts, so bun test is red on this head. Fold these actions into an existing tool, or trim about 180 bytes from other tools' descriptions. Do not raise the ceiling.

13 inputSchema: {
14 type: "object",
15 properties: {
16 repo: REPO_ARG,
17 action: { type: "string" },
18 host: { type: "string" },
outdated

Nit, not blocking: app_env puts enum on action, so clients see the allowed values in the schema. Here they only show up in the bad("invalid", …) text. The enum would cost bytes against the size cap, so leaving it out is defensible.

outdated

app_env declares enum: ["list","set","unset"] and required: ["repo","action"]. Declaring the same here (enum: ["list","add","check","remove"], required: ["repo","action"]) would let clients validate before calling. The size budget may be why it was left out. Optional.

19 },
20 },
21 writes: true,
22 needsAdminScope: true,
outdated

Minor, not blocking: the admin-only tools next to this one (repo_settings, hooks, agent) declare needsAdminScope: true. That keeps them out of the manifest for ro and rw tokens, which could never call them anyway. Declaring it here would let you drop the hand-written scope check on line 29, and it would take ~230 bytes off tools/list for every non-admin token. That helps the #6217 budget.

23 async run(ctx, user, args) {
24 const repo = repoArg(ctx, user, args);
25 if (isAnswer(repo)) return repo;
26 const action = args["action"];
27 if (action !== "list" && action !== "add" && action !== "check" && action !== "remove")
28 return bad("invalid", "action is list, add, check or remove");

Non-blocking: str trims but does not lowercase. add normalizes inside addDomain, but check/remove with App.Example.com answer no_such_domain for a domain that was just added. The API route does the same thing, so lowercase the host once in ownedDomain in actions/domains.ts in a follow-up, which fixes both doors.

29 const host = str(args["host"]);
30 if (action !== "list" && host === "") return bad("invalid", "host is the domain name");
31 const answer = <T>(result: { ok: true; value: T } | { ok: false; error: string }): Answer =>
outdated

Low (not blocking): add lowercases the host, but check/remove look it up as typed. So check with App.Example.com answers no_such_domain for a domain that was just added. The API route behaves the same way. A follow-up could normalize the host once, in ownedDomain, and fix both doors at once.

32 result.ok
33 ? good(result.value ?? {})
34 : result.error === "not_found"
35 ? bad("not_found", "no such app or repository (owner/name)")
outdated

Non-blocking: list answers a bare array here. The doc says "as …/app/domains answers", but the API (and the app_env tool's list) answers { items, next_cursor: null }. Wrap the list in that shape so a caller that pages through every list tool the same way does not trip on this one.

Minor, not blocking: a no_runtime error comes back as "the domain is refused", but the real cause is that this forge serves no apps. Next time this file is touched, a separate sentence for no_runtime would read truer.

36 : bad(result.error, `the domain is refused: ${result.error}`);
37 if (action === "list") {
38 const listed = listAppDomains(ctx.app, user, repo);
39 return listed.ok ? good({ items: listed.value, next_cursor: null }) : answer(listed);
outdated

list answers a bare array. GET …/app/domains answers { items, next_cursor: null }, and so does the sibling app_env list. The doc line says "as …/app/domains answers", so for list that is not quite true. Wrap it as good({ items: listed.value, next_cursor: null }) to match the API and app_env. The test's listed.value.map(...) would then read .items.

40 }
41 if (action === "add") return answer(addAppDomain(ctx.app, user, repo, host));
42 if (action === "check") return answer(await checkAppDomain(ctx.app, user, repo, host));
43 return answer(removeAppDomain(ctx.app, user, repo, host));
44 },
45 };
src/http/mcp/tools/index.ts +1
@@ -7,6 +7,7 @@ export { tool as account_verify } from "./account_verify.ts";
77 export { tool as accounts } from "./accounts.ts";
88 export { tool as agent } from "./agent.ts";
99 export { tool as app_deploy } from "./app_deploy.ts";
10 export { tool as app_domains } from "./app_domains.ts";
1011 export { tool as app_env } from "./app_env.ts";
1112 export { tool as app_members } from "./app_members.ts";
1213 export { tool as apps } from "./apps.ts";
test/mcp-app-domains.test.ts added +131
@@ -0,0 +1,131 @@
1 /**
2 * The `app_domains` tool is the API's domain routes over MCP: the same
3 * actions, so the same host rules and the same refusals.
4 */
5 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
6 import { mkdtemp, rm } from "node:fs/promises";
7 import { tmpdir } from "node:os";
8 import { join } from "node:path";
9 import { createApp, type App } from "../src/app.ts";
10 import { newApp } from "../src/actions/apps.ts";
11 import { AppsRuntime } from "../src/apps/runtime.ts";
12 import { createToken } from "../src/core/tokens.ts";
13 import { createUser } from "../src/core/users.ts";
14 import { serve, type Servers } from "../src/http/server.ts";
15 import { quietLogs } from "./helpers/globals.ts";
16
17 quietLogs();
18
19 describe("MCP app_domains", () => {
20 let dataDir: string;
21 let app: App;
22 let servers: Servers;
23 let runtime: AppsRuntime;
24 let admin: string;
25 let rw: string;
26 let bob: string;
27 let id = 1;
28
29 const tool = async (token: string, args: unknown) => {
30 const res = await fetch(`${servers.url}/mcp`, {
31 method: "POST",
32 headers: {
33 "content-type": "application/json",
34 accept: "application/json, text/event-stream",
35 authorization: `Bearer ${token}`,
36 },
37 body: JSON.stringify({
38 jsonrpc: "2.0",
39 id: id++,
40 method: "tools/call",
41 params: { name: "app_domains", arguments: args },
42 }),
43 });
44 const body: any = await res.json();
45 if (body.error !== undefined) return { error: String(body.error.message) };
46 const text: string = body.result.content[0].text;
47 return body.result.isError === true
48 ? { error: text }
49 : { value: JSON.parse(text) };
50 };
51
52 beforeAll(async () => {
53 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-domains-"));
54 app = createApp({ dataDir, port: 0, host: "forge.test" });
55 servers = await serve(app);
56 runtime = new AppsRuntime(app, {
57 domain: "apps.test",
58 scheme: "http",
59 publicPort: servers.port,
60 forgeUrl: servers.url,
61 concurrency: 1,
62 dns: {
63 resolveTxt: () => Promise.reject(Object.assign(new Error("x"), { code: "ENODATA" })),
64 resolveCname: () => Promise.reject(Object.assign(new Error("x"), { code: "ENODATA" })),
65 },
66 });
67 await runtime.start();
68 const a = await createUser(app.db, { origin: "seed", trusted: true,
69 username: "alice",
70 email: "[email protected]",
71 password: "a-good-passphrase",
72 adminIfFirst: true,
73 });
74 if (!a.ok) throw new Error(a.error);
75 const b = await createUser(app.db, { origin: "seed", trusted: true,
76 username: "bob",
77 email: "[email protected]",
78 password: "a-good-passphrase",
79 });
80 if (!b.ok) throw new Error(b.error);
81 admin = createToken(app.db, a.value.id, "admin", "admin").token;
82 rw = createToken(app.db, a.value.id, "rw", "rw").token;
83 bob = createToken(app.db, b.value.id, "admin", "admin").token;
84 const made = await newApp(app, a.value, {
85 name: "notes",
86 description: "A place for notes",
87 visibility: "public",
88 });
89 if (!made.ok) throw new Error(String(made.error));
90 });
91
92 afterAll(async () => {
93 await runtime.stop();
94 await servers.stop();
95 await rm(dataDir, { recursive: true, force: true });
96 });
97
98 const repo = "alice/notes";
99
100 test("an admin adds, checks, lists and removes a domain", async () => {
101 const added = await tool(admin, { repo, action: "add", host: "App.Example.com" });
102 expect(added.value.host).toBe("app.example.com");
103 expect(added.value.verified).toBe(false);
104 expect(added.value.records.length).toBe(2);
105
106 const checked = await tool(admin, { repo, action: "check", host: "app.example.com" });
107 expect(checked.value.verified).toBe(false);
108 expect(checked.value.said).toContain("No TXT record");
109
110 const listed = await tool(admin, { repo, action: "list" });
111 expect(listed.value.items.map((d: any) => d.host)).toEqual(["app.example.com"]);
112
113 expect((await tool(admin, { repo, action: "remove", host: "app.example.com" })).value).toEqual({});
114 expect((await tool(admin, { repo, action: "list" })).value.items).toEqual([]);
115 expect((await tool(admin, { repo, action: "check", host: "app.example.com" })).error).toContain("no_such_domain");
116 });
117
118 test("a non-admin is refused", async () => {
119 expect((await tool(bob, { repo, action: "list" })).error).toContain("not_found");
120 expect((await tool(bob, { repo, action: "add", host: "bob.example.com" })).error).toContain("not_found");
121 expect((await tool(rw, { repo, action: "add", host: "rw.example.com" })).error).toBeDefined();
122 expect((await tool(admin, { repo, action: "list" })).value.items).toEqual([]);
123 });
124
125 test("an invalid host, a reserved one and a bad action are refused", async () => {
126 expect((await tool(admin, { repo, action: "add", host: "not a host" })).error).toContain("invalid_host");
127 expect((await tool(admin, { repo, action: "add", host: "forge.test" })).error).toContain("reserved_host");
128 expect((await tool(admin, { repo, action: "add" })).error).toContain("host");
129 expect((await tool(admin, { repo, action: "drop", host: "a.example.com" })).error).toContain("action");
130 });
131 });
test/mcp-write-scope.test.ts +1
@@ -27,6 +27,7 @@ const ADMIN_ARGS: Record<string, Record<string, unknown>> = {
2727 prompts: { repo: "alice/game", action: "remove", id: "p1" },
2828 telemetry_grant: { agent: "robo", on: true },
2929 hooks: { repo: "alice/game", action: "add", url: "https://example.com/h", secret: "0123456789abcdef0123" },
30 app_domains: { repo: "alice/game", action: "add", host: "game.example.com" },
3031 };
3132
3233 /** What `visible()` answers for a tool the token's scope does not open. */

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs