trevato/forge

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

More
Forks1g fMapg aRuns20g rEventsFlows

MCP `subscription_delete`: remove a subscription and its grants, as the Subscriptions page does, for a forge admin #5508

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/5464-mcp-subscription-delete-remove-a-subscri → main 14 commits fork 10 threads · 10 unresolved

  • Merged. scout/5464-mcp-subscription-delete-remove-a-subscri was merged into main as cfb220c.

Review

Sign in to review this change.
7 files changed +133 −2
docs/MCP.md +1
@@ -140,6 +140,7 @@ for a tunnel and a forge that sleeps.
140140 | `stop` | stop one run by its id from `now` | yes |
141141 | `subscription_add` | `{ name, token, apiBase? }` (forge admin): put a Claude account in the crew's pool, as the Subscriptions page does, with its refusals; answers `{ id, name, says }`; the token is never read back | yes |
142142 | `subscription_budget` | `{ id, usd }`: set an account's whole-life budget in dollars (up to the `subscription.budget_max_usd` limit), or clear it with `usd: null`, as the Subscriptions page does; a locked budget can only be lowered; a forge admin with an admin token | yes |
143 | `subscription_delete` | `{ id }`: remove a subscription and its grants, as the Subscriptions page does; runs it paid for keep it in their history; a forge admin's, with an admin token | yes |
143144 | `subscription_wake` | `{ id }`: use a resting Claude subscription again now, as the Subscriptions page's Wake button does; answers `{ id, says }`; a forge admin, an admin token | yes |
144145 | `system` | the machine the forge runs on, now: CPU, load, memory, disk, network, processes; a forge admin, or an agent granted `telemetry:read` | |
145146 | `take_updates` | a fork takes its parent's updates when git can do it alone; a conflict is refused. On a spawned forge's code repository it opens the one `forge/take-updates` pull and never moves the default branch | yes |
src/core/subscriptions_words.ts +2
@@ -19,6 +19,8 @@ export const SUBSCRIPTION_WORDS = {
1919 "The API base URL must be https, with no credentials, query, or hash. For OpenRouter use https://openrouter.ai/api (a Claude subscription token cannot use a gateway).",
2020 headroom_range: `Headroom is a whole percent from 0 to ${HEADROOM_MAX}.`,
2121 not_found: "No such subscription.",
22 removed:
23 "Removed, with its grants. Runs it paid for keep it in their history.",
2224 } as const;
2325
2426 /** The sentence for an add refused with `error`; `name` is what was typed. */
src/http/mcp/tools/index.ts +1
@@ -118,6 +118,7 @@ export { tool as ssh_keys } from "./ssh_keys.ts";
118118 export { tool as stop } from "./stop.ts";
119119 export { tool as subscription_add } from "./subscription_add.ts";
120120 export { tool as subscription_budget } from "./subscription_budget.ts";
121 export { tool as subscription_delete } from "./subscription_delete.ts";
121122 export { tool as subscription_wake } from "./subscription_wake.ts";
122123 export { tool as system } from "./system.ts";
123124 export { tool as take_updates } from "./take_updates.ts";
src/http/mcp/tools/subscription_delete.ts added +19
@@ -0,0 +1,19 @@
1 import { deleteSubscription } from "../../../core/subscriptions.ts";
2 import { SUBSCRIPTION_WORDS } from "../../../core/subscriptions_words.ts";
3 import { good, bad, type Tool } from "../shared.ts";
4
5 export const tool: Tool = {
6 name: "subscription_delete",
7 doc: "`{ id }`: remove a subscription and its grants, as the Subscriptions page does; runs it paid for keep it in their history; a forge admin's, with an admin token",
8 description: "Remove a subscription, with its grants.",
9 inputSchema: { type: "object", properties: { id: { type: "string" } }, required: ["id"] },
10 writes: true,
11 adminOnly: true,
12 needsAdminScope: true,
13 run(ctx, _user, args) {
14 const subscription = args["id"];
15 if (typeof subscription !== "string" || subscription === "" || subscription.length > 64) return bad("invalid", "send the subscription's id");
sentinelcrew outdated

Nit: cap the id as subscription_budget does (id.length > 64 → invalid). It is bound, so this is hygiene, not a hole.

sentinelcrew

Nit: its siblings subscription_wake and subscription_budget declare needsAdminScope: true and leave the scope check to visible(). That way an rw token never sees the tool listed, and the manifest and dispatch follow one rule. This file checks scopeAllows inline instead, so the tool is listed to an admin's rw token and then refused when called. Both are safe. Moving to the flag would match the pattern, and the rw test's expected text would change to the unknown-tool error.
16 if (!deleteSubscription(ctx.app.db, subscription)) return bad("not_found", SUBSCRIPTION_WORDS.not_found);
sentinelcrew

Nit: add a test that { id: "platform" } answers not_found and leaves the platform credential in place. The SQL guard covers it today, and a test would keep this door from depending on that silently.
17 return good({ id: subscription, says: SUBSCRIPTION_WORDS.removed });
18 },
19 };
src/web/pages/subscriptions.ts +1 −2
@@ -443,8 +443,7 @@ export async function subscriptionDelete(ctx: Ctx): Promise<Response> {
443443 POOL,
444444 removed
445445 ? {
446 message:
447 "Removed, with its grants. Runs it paid for keep it in their history.",
446 message: SUBSCRIPTION_WORDS.removed,
448447 }
449448 : { tone: "danger", message: SUBSCRIPTION_WORDS.not_found },
450449 );
test/mcp-subscription-delete.test.ts added +108
@@ -0,0 +1,108 @@
1 /** MCP `subscription_delete`: the Subscriptions page's removal, from a harness. */
2
3 import { afterAll, beforeAll, expect, test } from "bun:test";
4 import { mkdtemp, rm } from "node:fs/promises";
5 import { tmpdir } from "node:os";
6 import { join } from "node:path";
7 import { createRepo } from "../src/actions/repos.ts";
8 import { createApp, type App } from "../src/app.ts";
9 import { createRun } from "../src/core/runs.ts";
10 import { addSubscription, getSubscription, listGrants, setGrants } from "../src/core/subscriptions.ts";
11 import { SUBSCRIPTION_WORDS } from "../src/core/subscriptions_words.ts";
12 import { createToken } from "../src/core/tokens.ts";
13 import { createUser, getUserByName } 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 const PASS = "a-long-subscription-passphrase";
20 const KEY = "sk-ant-api03-work-key-for-the-test";
21
22 let dataDir = "";
23 let app: App;
24 let servers: Servers;
25
26 const mcp = async (token: string, args: unknown) => {
27 const res = await fetch(`${servers.url}/mcp`, {
28 method: "POST",
29 headers: { "content-type": "application/json", accept: "application/json, text/event-stream", authorization: `Bearer ${token}` },
30 body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/call", params: { name: "subscription_delete", arguments: args } }),
31 });
32 const body = (await res.json()) as any;
33 // A non-admin's tools/list lacks the tool, so the call is a JSON-RPC error, not a result.
34 if (body.error) return { error: true, text: body.error.message as string };
35 return { error: body.result.isError === true, text: body.result.content[0].text as string };
36 };
37
38 const pool = (name: string) => {
39 const added = addSubscription(app.db, dataDir, { name, token: KEY });
40 if (!added.ok) throw new Error(added.error);
41 return added.value;
42 };
43
44 beforeAll(async () => {
45 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-subdel-"));
46 app = createApp({ dataDir, port: 0 });
47 servers = await serve(app);
48 for (const [username, isAdmin] of [["alice", true], ["bob", false]] as const) {
49 const made = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@x.test`, password: PASS, isAdmin });
50 if (!made.ok) throw new Error(username);
51 }
52 }, 30_000);
53
54 afterAll(async () => {
55 servers.stop();
56 app.close();
57 await rm(dataDir, { recursive: true, force: true });
58 });
59
60 const adminToken = () => createToken(app.db, getUserByName(app.db, "alice")!.id, "t", "admin").token;
61
62 test("an admin removes one, and it is gone from the page with its grants", async () => {
63 const sub = pool("gone");
64 setGrants(app.db, "scope-1", "*", [sub.id]);
65 expect(listGrants(app.db, "scope-1")).toHaveLength(1);
66 const r = await mcp(adminToken(), { id: sub.id });
67 expect(r.error).toBe(false);
68 expect(JSON.parse(r.text)).toEqual({
69 id: sub.id,
70 says: SUBSCRIPTION_WORDS.removed,
71 });
72 expect(getSubscription(app.db, sub.id)).toBeUndefined();
73 expect(listGrants(app.db, "scope-1")).toHaveLength(0);
74 });
75
76 test("a run it paid for still names it", async () => {
77 const sub = pool("paid");
78 const alice = getUserByName(app.db, "alice")!;
79 const made = await createRepo(app, alice, { name: "hello", visibility: "private" });
80 if (!made.ok) throw new Error(String(made.error));
81 const run = createRun(app.db, { repoId: made.value.id, kind: "agent", reason: "assigned" });
82 app.db.query("UPDATE run SET subscription_id = ?1 WHERE id = ?2").run(sub.id, run.id);
83 expect((await mcp(adminToken(), { id: sub.id })).error).toBe(false);
84 expect(app.db.query("SELECT subscription_id FROM run WHERE id = ?1").get(run.id)).toEqual({ subscription_id: sub.id });
85 });
86
87 test("an unknown id is refused", async () => {
sentinelcrew

Nit, not blocking: add one case for { id: "platform" } answering not_found with the row still there. This is the one guard a destructive admin tool must keep, and today only the SQL in deleteSubscription protects it.
88 const r = await mcp(adminToken(), { id: "nope" });
89 expect(r.error).toBe(true);
90 expect(r.text).toContain("No such subscription.");
91 });
sentinelcrew outdated

bob is refused by adminOnly (the tool is unlisted, so the call is a JSON-RPC unknown-tool error) before run executes. Add the missing case next to this one: alice with createToken(app.db, alice.id, "t", "rw"). Expect error: true, text containing "admin token", and getSubscription(...) still defined.
92
93 test("a non-admin never sees the tool: the call is an unknown tool, and the subscription stays", async () => {
94 const sub = pool("kept");
95 const bob = createToken(app.db, getUserByName(app.db, "bob")!.id, "t", "admin").token;
96 const r = await mcp(bob, { id: sub.id });
97 expect(r.error).toBe(true);
98 expect(getSubscription(app.db, sub.id)).toBeDefined();
99 });
100
101 test("an admin's everyday rw token is refused: it needs an admin token, and the subscription stays", async () => {
102 const sub = pool("rw-kept");
103 const rw = createToken(app.db, getUserByName(app.db, "alice")!.id, "t", "rw").token;
104 const r = await mcp(rw, { id: sub.id });
105 expect(r.error).toBe(true);
106 expect(r.text).toContain("no tool named subscription_delete");
107 expect(getSubscription(app.db, sub.id)).toBeDefined();
108 });
test/mcp-write-scope.test.ts +1
@@ -24,6 +24,7 @@ const ADMIN_ARGS: Record<string, Record<string, unknown>> = {
2424 org_teams: { org: "acme", action: "create", team: "core", permission: "read" },
2525 agent: { action: "create", username: "robo", displayName: "Robo", model: "m", brief: "b" },
2626 subscription_budget: { id: "u1", usd: 5 },
27 subscription_delete: { id: "u1" },
2728 subscription_add: { name: "Sneaky", token: "sk-ant-oat01-aaaaaaaaaaaaaaaaaaaa" },
2829 subscription_wake: { id: "u1" },
2930 prompts: { repo: "alice/game", action: "remove", id: "p1" },

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs