Public: anyone can read this; the crew can change it.
MCP `here` takes an app URL and answers what its /__forge/ door shows: app, environment, pull, who is working, and the links home #7011
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5858-mcp-here-takes-an-app-url-and-answers-wh → main 8 commits fork 4 threads · 4 unresolved
docs/MCP.md +1
| @@ -84,6 +84,7 @@ for a tunnel and a forge that sleeps. | ||
| 84 | 84 | | `forges` | an admin's list of the forges sent from here: name, address, whom for, apps, claimed or not, last activity | | |
| 85 | 85 | | `front_page` | `{ pinned?, hidden?, weights?, dryRun? }`: the front page as a stranger sees it, ranked, with numbers; a site admin sets what is pinned (first public one featured), hidden, and what counts (weights of forks, ships, use, fresh, 0 to 10) | yes | |
| 86 | 86 | | `health` | how this forge is: the version and commit it runs, draining or not, the rollout and its incidents | | |
| 87 | | `here` | `{ url }`: on a forge page, what you may do there as who you are, each action with its route and tool; on the link of one of this forge's app hosts, what its `/__forge/` door shows: `{ app, repo, environment, pull: { number, title } or null, working: [names], links: { home, repo, report } }`; a foreign host, or a private app you may not enter, is refused like a missing page | | | |
| 87 | 88 | | `hold` | `{ app, pull | path, why?, until?, release?, dryRun? }`: hold a pull so no rule reviews or lands it, or let it go; a person who can write, never an agent; or with `path` (no `pull`) hold new work on a file so the picker starts no build whose issue names it, or list the holds and who set each; an admin of the app; plans unless `dryRun: false` | yes | |
| 88 | 89 | | `hooks` | `{ repo, action, url, secret, kinds, id }`: `list` (pages: `limit` 1–200, `cursor`; `{ items, next_cursor }`), `add` (`secret`, 16–200 characters, is required) or `remove` the repository's webhooks, as `…/hooks`; an admin-scope token on a repository you administer, and a secret is never shown | yes | |
| 89 | 90 | | `invites` | `{ action, id? }`: `list` the outstanding invites (state and expiry, never a link) or `revoke` one by `id`; a person admin with an admin token, stricter than the API's list, which an `ro` token may read | yes | |
src/apps/crew.ts +26 −14
| @@ -27,9 +27,21 @@ export function crewNow( | ||
| 27 | 27 | /** When given, the reads are named on it: `app`, `pull`, `working`. */ |
| 28 | 28 | request?: Request, |
| 29 | 29 | ): CrewNow | undefined { |
| 30 | return crewAll(db, forgeUrl, app, env, viewerId, request)[0]; | |
| 31 | } | |
| 32 | ||
| 33 | /** Everyone working on the app now, by the same rules as `crewNow`. */ | |
| 34 | export function crewAll( | |
| 35 | db: Database, | |
| 36 | forgeUrl: string, | |
| 37 | app: AppRecord, | |
| 38 | env: string, | |
| 39 | viewerId: string | undefined, | |
| 40 | request?: Request, | |
| 41 | ): CrewNow[] { | |
| 30 | 42 | const phase = <T>(name: string, work: () => T): T => |
| 31 | 43 | request === undefined ? work() : timePhase(request, name, work); |
| 32 | if (viewerId === undefined) return undefined; | |
| 44 | if (viewerId === undefined) return []; | |
| 33 | 45 | const seen = phase("app", () => { |
| 34 | 46 | const viewer = getUserById(db, viewerId); |
| 35 | 47 | const repo = getRepoById(db, app.repoId); |
| @@ -37,29 +49,29 @@ export function crewNow( | ||
| 37 | 49 | if (!canRead(db, repo, viewer)) return undefined; |
| 38 | 50 | return { viewer, repo }; |
| 39 | 51 | }); |
| 40 | if (seen === undefined) return undefined; | |
| 52 | if (seen === undefined) return []; | |
| 41 | 53 | const { viewer, repo } = seen; |
| 42 | 54 | const number = /^pull-(\d+)$/.exec(env)?.[1]; |
| 43 | if (number === undefined && env !== "production") return undefined; | |
| 55 | if (number === undefined && env !== "production") return []; | |
| 44 | 56 | const pulled = phase("pull", () => { |
| 45 | 57 | if (number === undefined) return { issueId: undefined }; |
| 46 | 58 | const pull = getIssue(db, repo.id, Number(number)); |
| 47 | 59 | if (pull === undefined || !mayView(db, repo, pull, viewer)) return undefined; |
| 48 | 60 | return { issueId: pull.id as string | undefined }; |
| 49 | 61 | }); |
| 50 | if (pulled === undefined) return undefined; | |
| 62 | if (pulled === undefined) return []; | |
| 51 | 63 | // A run on a sealed issue is not the viewer's to know of. |
| 52 | const run = phase("working", () => | |
| 53 | workingAgentRuns(db, repo.id, pulled.issueId).find((r) => | |
| 54 | mayViewId(db, repo, r.issueId, viewer), | |
| 55 | ), | |
| 64 | return phase("working", () => | |
| 65 | workingAgentRuns(db, repo.id, pulled.issueId) | |
| 66 | .filter((r) => mayViewId(db, repo, r.issueId, viewer)) | |
| 67 | .map((run) => { | |
| 68 | const actor = run.actorId === null ? undefined : getUserById(db, run.actorId); | |
| 69 | return { | |
| 70 | who: actor?.username ?? "the crew", | |
| 71 | url: new URL(`/${repo.ownerName}/${repo.name}/runs/${run.id}`, forgeUrl).href, | |
| 72 | }; | |
| 73 | }), | |
| 56 | 74 | ); |
| 57 | if (run === undefined) return undefined; | |
| 58 | const actor = run.actorId === null ? undefined : getUserById(db, run.actorId); | |
| 59 | return { | |
| 60 | who: actor?.username ?? "the crew", | |
| 61 | url: new URL(`/${repo.ownerName}/${repo.name}/runs/${run.id}`, forgeUrl).href, | |
| 62 | }; | |
| 63 | 75 | } |
| 64 | 76 | |
| 65 | 77 | /** |
src/apps/here.ts added +64
| @@ -0,0 +1,64 @@ | ||
| 1 | /** | |
| 2 | * What an app's `/__forge/` door shows, for an agent holding the app's link: | |
| 3 | * the same facts (`pillWhere`, `crewAll`, `reportUrl`) the door and pill | |
| 4 | * render from, as plain data. Anyone the door would turn away is refused. | |
| 5 | */ | |
| 6 | ||
| 7 | import { getRepoById, canEnterApp } from "../core/repos.ts"; | |
| 8 | import { mayView } from "../core/sealed.ts"; | |
| 9 | import { getIssue } from "../core/issues.ts"; | |
| 10 | import type { User } from "../core/users.ts"; | |
| 11 | import type { AppsRuntime } from "./runtime.ts"; | |
| 12 | import { crewAll } from "./crew.ts"; | |
| 13 | import { isHostPort, pillWhere, reportUrl, resolveAppHostname } from "./proxy.ts"; | |
| 14 | ||
| 15 | export type Here = { | |
| 16 | app: string; | |
| 17 | repo: string | null; | |
| 18 | environment: string; | |
| 19 | pull: { number: number; title: string } | null; | |
| 20 | working: string[]; | |
| 21 | links: { home: string; repo: string | null; report: string | null }; | |
| 22 | }; | |
| 23 | ||
| 24 | /** `undefined` is the door's refusal: not an app host here, or not yours to enter. */ | |
| 25 | export function hereOf(runtime: AppsRuntime, link: string, user: User): Here | undefined { | |
| 26 | let host: string; | |
| 27 | try { | |
| 28 | const url = new URL(link); | |
| 29 | if (url.protocol !== "http:" && url.protocol !== "https:") return undefined; | |
| 30 | host = url.host; | |
| 31 | } catch { | |
| 32 | return undefined; | |
| 33 | } | |
| 34 | if (!isHostPort(host)) return undefined; | |
| 35 | const app = runtime.app; | |
| 36 | const target = resolveAppHostname(app, runtime.options.domain, host); | |
| 37 | if (target === undefined || target === "unknown" || target.nested) return undefined; | |
| 38 | const repo = getRepoById(app.db, target.app.repoId); | |
| 39 | if (repo === undefined || repo.archivedAt != null) return undefined; | |
| 40 | if (!canEnterApp(app.db, repo, target.app, user)) return undefined; | |
| 41 | const number = /^pull-(\d+)$/.exec(target.env)?.[1]; | |
| 42 | if (number !== undefined) { | |
| 43 | const pull = getIssue(app.db, repo.id, Number(number)); | |
| 44 | if (pull?.sealed === true && !mayView(app.db, repo, pull, user)) return undefined; | |
Sealed-pull refusal is untested. Seal the preview's pull and assert that bob gets the same error as |
||
| 45 | } | |
| 46 | const where = pillWhere(runtime, target, { id: user.id, name: user.username, role: "user" }); | |
| 47 | const forgeUrl = runtime.options.forgeUrl; | |
| 48 | const report = reportUrl(runtime, target); | |
| 49 | return { | |
| 50 | app: where.app, | |
| 51 | repo: where.repo ?? null, | |
| 52 | environment: target.env, | |
| 53 | pull: | |
| 54 | where.pull !== undefined && where.title !== undefined | |
| 55 | ? { number: where.pull, title: where.title } | |
| 56 | : null, | |
| 57 | working: crewAll(app.db, forgeUrl, target.app, target.env, user.id).map((c) => c.who), | |
No test exercises this with a running agent run. Add one where a run is in flight and its actor's name appears, and one where a run on a sealed issue the caller may not view is left out. The second is the leak this filter exists to stop. |
||
| 58 | links: { | |
| 59 | home: forgeUrl, | |
| 60 | repo: where.repo === undefined ? null : new URL(`/${where.repo}`, forgeUrl).href, | |
| 61 | report: report?.href ?? null, | |
| 62 | }, | |
| 63 | }; | |
| 64 | } |
src/apps/proxy.ts +1 −1
| @@ -636,7 +636,7 @@ function admit( | ||
| 636 | 636 | * form (`actions/reports.ts`), when both the app and the repository are |
| 637 | 637 | * public. A private one offers nothing, and a nested host is its guest's. |
| 638 | 638 | */ |
| 639 | function reportUrl(runtime: AppsRuntime, target: AppTarget): URL | undefined { | |
| 639 | export function reportUrl(runtime: AppsRuntime, target: AppTarget): URL | undefined { | |
| 640 | 640 | if (target.nested || target.app.access !== "public") return undefined; |
| 641 | 641 | const repo = getRepoById(runtime.app.db, target.app.repoId); |
| 642 | 642 | if (repo === undefined || !canRead(runtime.app.db, repo, undefined)) |
src/http/mcp/tools/here.ts +7 −1
| @@ -1,10 +1,13 @@ | ||
| 1 | import { appsRuntime } from "../../../apps/runtime.ts"; | |
| 2 | import { hereOf } from "../../../apps/here.ts"; | |
| 1 | 3 | import { manifestFor } from "../../../actions/here.ts"; |
| 2 | 4 | import { good, bad, type Tool, str } from "../shared.ts"; |
| 3 | 5 | |
| 4 | 6 | export const tool: Tool = { |
| 5 | 7 | name: "here", |
| 6 | 8 | description: |
| 7 | "What you can do on a forge page: each open action with its HTTP route and MCP tool.", | |
| 9 | "What you may do on a forge page; given an app's link, what its /__forge/ door shows.", | |
| 10 | doc: "`{ url }`: on a forge page, what you may do there as who you are, each action with its route and tool; on the link of one of this forge's app hosts, what its `/__forge/` door shows: `{ app, repo, environment, pull: { number, title } or null, working: [names], links: { home, repo, report } }`; a foreign host, or a private app you may not enter, is refused like a missing page", | |
| 8 | 11 | inputSchema: { |
| 9 | 12 | type: "object", |
| 10 | 13 | properties: { url: { type: "string", maxLength: 2000 } }, |
| @@ -13,6 +16,9 @@ export const tool: Tool = { | ||
| 13 | 16 | writes: false, |
| 14 | 17 | run(ctx, user, args) { |
| 15 | 18 | const raw = str(args["url"]); |
| 19 | const runtime = appsRuntime(ctx.app); | |
| 20 | const door = runtime === undefined || raw.length > 2000 ? undefined : hereOf(runtime, raw, user); | |
| 21 | if (door !== undefined) return good(door); | |
| 16 | 22 | const parsed = URL.parse(raw, ctx.url.origin); |
| 17 | 23 | if (parsed === null || parsed.origin !== ctx.url.origin) |
| 18 | 24 | return bad("not_found", "not a page of this forge"); |
test/mcp-docs.test.ts −1
| @@ -18,7 +18,6 @@ quietLogs(); | ||
| 18 | 18 | * The test fails until a documented tool's line is deleted here. |
| 19 | 19 | */ |
| 20 | 20 | const undocumented = new Set<string>([ |
| 21 | "here", | |
| 22 | 21 | "asks", |
| 23 | 22 | "asks_act", |
| 24 | 23 | "spend", |
test/mcp-here.test.ts added +199
| @@ -0,0 +1,199 @@ | ||
| 1 | /** | |
| 2 | * The `here` tool answers what an app's `/__forge/` door shows, for a link: | |
| 3 | * production and a preview, and the door's refusal for a foreign host and | |
| 4 | * for a private app the caller may not enter. | |
| 5 | */ | |
| 6 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 7 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 8 | import { tmpdir } from "node:os"; | |
| 9 | import { join } from "node:path"; | |
| 10 | import { createApp, type App } from "../src/app.ts"; | |
| 11 | import { AppsRuntime } from "../src/apps/runtime.ts"; | |
| 12 | import { newApp } from "../src/actions/apps.ts"; | |
| 13 | import { openPull } from "../src/actions/pulls.ts"; | |
| 14 | import { commitToBranch } from "../src/actions/repos.ts"; | |
| 15 | import { getRepo } from "../src/core/repos.ts"; | |
| 16 | import { createRun, claimRun, finishRun } from "../src/core/runs.ts"; | |
| 17 | import { createIssue, getIssue } from "../src/core/issues.ts"; | |
| 18 | import { setSealed } from "../src/core/sealed.ts"; | |
| 19 | import { createUser, getUserByName } from "../src/core/users.ts"; | |
| 20 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 21 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 22 | import { promote } from "./helpers/members.ts"; | |
| 23 | import { tokenFor } from "./helpers/tokens.ts"; | |
| 24 | ||
| 25 | describe("MCP here", () => { | |
| 26 | let dataDir: string; | |
| 27 | let app: App; | |
| 28 | let servers: Servers; | |
| 29 | let runtime: AppsRuntime; | |
| 30 | let aliceRw: string; | |
| 31 | let bobRw: string; | |
| 32 | let pull = 0; | |
| 33 | let id = 1; | |
| 34 | ||
| 35 | const tool = async (token: string, args: unknown) => { | |
| 36 | const res = await fetch(`${servers.url}/mcp`, { | |
| 37 | method: "POST", | |
| 38 | headers: { | |
| 39 | "content-type": "application/json", | |
| 40 | accept: "application/json, text/event-stream", | |
| 41 | authorization: `Bearer ${token}`, | |
| 42 | }, | |
| 43 | body: JSON.stringify({ | |
| 44 | jsonrpc: "2.0", | |
| 45 | id: id++, | |
| 46 | method: "tools/call", | |
| 47 | params: { name: "here", arguments: args }, | |
| 48 | }), | |
| 49 | }); | |
| 50 | const body: any = await res.json(); | |
| 51 | const text: string = body.result.content[0].text; | |
| 52 | return body.result.isError === true ? { error: text } : { value: JSON.parse(text) }; | |
| 53 | }; | |
| 54 | ||
| 55 | beforeAll(async () => { | |
| 56 | dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-app-deploy-")); | |
| 57 | app = createApp({ dataDir, port: 0 }); | |
| 58 | servers = await serve(app); | |
| 59 | runtime = new AppsRuntime(app, { | |
| 60 | domain: "apps.test", | |
| 61 | scheme: "http", | |
| 62 | publicPort: servers.port, | |
| 63 | forgeUrl: servers.url, | |
| 64 | }); | |
| 65 | await runtime.start(); | |
| 66 | await seedAdmin(app, { username: "alice", email: "[email protected]", password: "a-good-passphrase" }); | |
| 67 | const b = await createUser(app.db, { | |
| 68 | origin: "seed", | |
| 69 | trusted: true, | |
| 70 | username: "bob", | |
| 71 | email: "[email protected]", | |
| 72 | password: "a-good-passphrase", | |
| 73 | }); | |
| 74 | if (!b.ok) throw new Error(b.error); | |
| 75 | promote(app.db, b.value); | |
| 76 | const alice = getUserByName(app.db, "alice")!; | |
| 77 | const made = await newApp(app, alice, { name: "hello", visibility: "public" } as never); | |
| 78 | if (!made.ok) throw new Error(String(made.error)); | |
| 79 | const repo = getRepo(app.db, "alice", "hello")!; | |
| 80 | const committed = await commitToBranch(app, alice, repo, { | |
| 81 | branch: "greeting", | |
| 82 | base: repo.defaultBranch, | |
| 83 | message: "Say hello", | |
| 84 | files: [{ path: "GREETING.md", content: "hi\n" }], | |
| 85 | }); | |
| 86 | if (!committed.ok) throw new Error(JSON.stringify(committed.error)); | |
| 87 | const opened = await openPull(app, alice, repo, { | |
| 88 | title: "Say hello", | |
| 89 | baseBranch: repo.defaultBranch, | |
| 90 | headBranch: "greeting", | |
| 91 | }); | |
| 92 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 93 | pull = opened.value.number; | |
| 94 | const secret = await newApp(app, alice, { name: "vault", visibility: "private" } as never); | |
| 95 | if (!secret.ok) throw new Error(String(secret.error)); | |
| 96 | aliceRw = tokenFor(app, "alice", "rw"); | |
| 97 | bobRw = tokenFor(app, "bob", "rw"); | |
| 98 | }, 30_000); | |
| 99 | ||
| 100 | afterAll(async () => { | |
| 101 | await runtime.stop(); | |
| 102 | servers.stop(); | |
| 103 | app.close(); | |
| 104 | await rm(dataDir, { recursive: true, force: true }); | |
| 105 | }); | |
| 106 | ||
| 107 | const host = (label: string) => `http://${label}.apps.test:${servers.port}/anything`; | |
| 108 | ||
| 109 | test("production: app, repo, no pull, the links home", async () => { | |
| 110 | const r = await tool(bobRw, { url: host("hello") }); | |
| 111 | expect(r.value.app).toBe("hello"); | |
| 112 | expect(r.value.repo).toBe("alice/hello"); | |
outdated
Every case asserts an empty |
||
| 113 | expect(r.value.environment).toBe("production"); | |
| 114 | expect(r.value.pull).toBeNull(); | |
| 115 | expect(r.value.working).toEqual([]); | |
| 116 | expect(r.value.links.home).toBe(servers.url); | |
| 117 | expect(r.value.links.repo).toBe(`${servers.url}/alice/hello`); | |
| 118 | expect(r.value.links.report).toBe(`${servers.url}/alice/hello/report`); | |
| 119 | }); | |
| 120 | ||
| 121 | test("a preview names its pull", async () => { | |
| 122 | const r = await tool(bobRw, { url: host(`hello--${pull}`) }); | |
| 123 | expect(r.value.environment).toBe(`pull-${pull}`); | |
| 124 | expect(r.value.pull).toEqual({ number: pull, title: "Say hello" }); | |
| 125 | }); | |
| 126 | ||
| 127 | test("a foreign host and a bad link are refused in the forge's words", async () => { | |
| 128 | for (const url of ["https://example.com/", "ftp://hello.apps.test/", host("nothing")]) { | |
| 129 | const r = await tool(aliceRw, { url }); | |
| 130 | expect(r.error).toMatch(/not a page of this forge|no such page here/); | |
| 131 | } | |
| 132 | }); | |
| 133 | ||
| 134 | test("a private app the caller cannot enter is refused like a missing one", async () => { | |
| 135 | const owner = await tool(aliceRw, { url: host("vault") }); | |
| 136 | expect(owner.value.repo).toBe("alice/vault"); | |
| 137 | const stranger = await tool(bobRw, { url: host("vault") }); | |
| 138 | const missing = await tool(bobRw, { url: host("nothing") }); | |
| 139 | expect(stranger.error).toBe(missing.error); | |
| 140 | }); | |
| 141 | ||
| 142 | describe("who is working, and what is sealed or archived", () => { | |
| 143 | const start = (issueId: string, actorName: string) => { | |
| 144 | const actor = getUserByName(app.db, actorName)!; | |
| 145 | const repo = getRepo(app.db, "alice", "hello")!; | |
| 146 | const run = createRun(app.db, { repoId: repo.id, kind: "agent", issueId, actorId: actor.id, reason: "assigned" }); | |
| 147 | claimRun(app.db, run.id); | |
| 148 | return run.id; | |
| 149 | }; | |
| 150 | const end = (runId: string) => finishRun(app.db, runId, { state: "ok", summary: "done" }); | |
| 151 | const pullIssue = () => getIssue(app.db, getRepo(app.db, "alice", "hello")!.id, pull)!; | |
| 152 | ||
| 153 | test("a running agent run on the pull is named, and every one is listed", async () => { | |
| 154 | const a = start(pullIssue().id, "bob"); | |
| 155 | const b = start(pullIssue().id, "alice"); | |
| 156 | const r = await tool(aliceRw, { url: host(`hello--${pull}`) }); | |
| 157 | expect([...r.value.working].sort()).toEqual(["alice", "bob"]); | |
| 158 | end(a); | |
| 159 | end(b); | |
| 160 | expect((await tool(aliceRw, { url: host(`hello--${pull}`) })).value.working).toEqual([]); | |
| 161 | }); | |
| 162 | ||
| 163 | test("a run on a sealed issue is left out for who may not view it", async () => { | |
| 164 | const repo = getRepo(app.db, "alice", "hello")!; | |
| 165 | const made = createIssue(app.db, { repoId: repo.id, authorId: getUserByName(app.db, "alice")!.id, title: "secret", kind: "issue" }); | |
| 166 | if (!made.ok) throw new Error(made.error); | |
| 167 | setSealed(app.db, made.value.id, true); | |
| 168 | const run = start(made.value.id, "alice"); | |
| 169 | expect((await tool(bobRw, { url: host("hello") })).value.working).toEqual([]); | |
| 170 | expect((await tool(aliceRw, { url: host("hello") })).value.working).toEqual(["alice"]); | |
| 171 | end(run); | |
| 172 | }); | |
| 173 | ||
| 174 | test("a sealed pull is refused like a missing page to who may not view it", async () => { | |
| 175 | setSealed(app.db, pullIssue().id, true); | |
| 176 | try { | |
| 177 | const missing = await tool(bobRw, { url: host("nothing") }); | |
| 178 | const bob = await tool(bobRw, { url: host(`hello--${pull}`) }); | |
| 179 | expect(bob.error).toBe(missing.error); | |
| 180 | const alice = await tool(aliceRw, { url: host(`hello--${pull}`) }); | |
| 181 | expect(alice.value.pull).toEqual({ number: pull, title: "Say hello" }); | |
| 182 | } finally { | |
| 183 | setSealed(app.db, pullIssue().id, false); | |
| 184 | } | |
| 185 | }); | |
| 186 | ||
| 187 | test("an archived repository's app is refused like a missing page", async () => { | |
| 188 | const repo = getRepo(app.db, "alice", "hello")!; | |
| 189 | app.db.query("UPDATE repo SET archived_at = ?2 WHERE id = ?1").run(repo.id, new Date().toISOString()); | |
| 190 | try { | |
| 191 | const missing = await tool(aliceRw, { url: host("nothing") }); | |
| 192 | expect((await tool(aliceRw, { url: host("hello") })).error).toBe(missing.error); | |
| 193 | } finally { | |
| 194 | app.db.query("UPDATE repo SET archived_at = NULL WHERE id = ?1").run(repo.id); | |
| 195 | } | |
| 196 | expect((await tool(aliceRw, { url: host("hello") })).value.repo).toBe("alice/hello"); | |
| 197 | }); | |
| 198 | }); | |
| 199 | }); |
Good: this gate is the same set of checks as
stillEntitledin proxy.ts (canEnterApp plus mayView on a sealed pull), so the MCP door can't drift looser than the HTTP one. If either changes, consider making them share one helper.