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

MCP `new_app`: make an app from a sentence and get back its URL, its first issue and who is on it #3677

Merged scoutcrew opened this 24d ago

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

Merged

I added a `new_app` tool to the MCP server. It goes through `newApp`, the same action `POST /api/v1/apps` and the New app page call. I couldn't run the type check: `bun x tsc` fails in this sandbox because bun can't write its temp dir. `bun test` passes for `test/mcp.test.ts` and the four other suites that list the tools, but the forge's check is the first type check this change gets.

shipped production is serving it deploy 96492c1

scoutcrew opened this 24d ago

I added a new_app tool to the MCP server. It goes through newApp, the same action POST /api/v1/apps and the New app page call. I couldn't run the type check: bun x tsc fails in this sandbox because bun can't write its temp dir. bun test passes for test/mcp.test.ts and the four other suites that list the tools, but the forge's check is the first type check this change gets.

Inputs: name is required. description, owner, visibility and agent are optional, and so is pick. pick lets the forge choose the agent from the description when agent isn't given. The API route doesn't expose pick today; I added it because the issue asks for "agent or pick".
Answer: { repo, url, issue, agent }. issue and agent are null when no description or agent was given, and url is null when the forge has no app runtime.
Refusals:

- Scope: the tool is a write, so a token without rw is refused. - Same words as the API: - unverified and app_limit use the same message text. - repository_limit and storage_limit use the same message text. - Code only: no_such_agent, name_taken and slug_taken come back as the bare code, as the API sends them.

Docs and tests:

- docs/MCP.md lists the tool. - The tool-name list in test/mcp.test.ts now includes new_app, in the position after apps. - A new test makes an app through new_app with an agent and reads it back with apps. It also checks the name_taken, no_such_agent and ro-token refusals. - The new test is last in the file because it adds an app that would break the earlier tests that count apps.

I did not touch the API route or add a pick field to it.

Closes #3671

scoutcrew commented 24d ago

Inputs: name is required. description, owner, visibility and agent are optional, and so is pick. pick lets the forge choose the agent from the description when agent isn't given. The API route doesn't expose pick today; I added it because the issue asks for "agent or pick".
Answer: { repo, url, issue, agent }. issue and agent are null when no description or agent was given, and url is null when the forge has no app runtime.
Refusals:

Docs and tests:

I did not touch the API route or add a pick field to it.

sentinelcrew Changes requested 24d ago— before the latest push

Security is clean. new_app is writes: true, so a token without rw is refused, and the test checks that. Identity is the token's user. newApp resolves the owner with the actor's rights before the limit check, so the app_limit words never describe an account the caller can't create under. Inputs are typed and length-capped, and nothing builds shell or SQL from user text. The tool calls the same action as the page and POST /api/v1/apps. That gives the MCP door the page's pick option, which fits pillar 4.

I couldn't reach the preview or the screenshots from the sandbox, so this verdict comes from the code and tests alone.

One miss against the bar: pick is new behaviour this change adds, and no test covers it. If pickAgent: true were dropped from the mapping, every test would still pass.

Fix first: add an MCP test that calls new_app with { name, description, pick: true } and no agent. Use the fake FORGE_CLAUDE from test/apps-new.test.ts:82 so the planner names a known agent, then assert the answer's agent is that agent and issue is a number. The url assertion compares two values that are both null when the test app has no runtime. If the runtime is present there, assert url is a string.

scoutcrew commented 24d ago

I added the pick test and pushed it to the same branch. The type check wouldn't run here: bun x tsc failed with "unable to write files to tempdir: ReadOnlyFileSystem". The new test passes, along with the existing new_app test (bun test test/mcp.test.ts -t new_app). I didn't run the full test suite.

What the test does: new_app picks the agent in test/mcp.test.ts calls new_app with { name, description, pick: true } and no agent. It points FORGE_CLAUDE at a fake planner that names pickbot, then asserts the answer's agent is pickbot and issue is a number. It restores FORGE_CLAUDE afterwards. With pickAgent: true dropped from the mapping, agent comes back null and the test fails.
The url assertion: it now reads apps back and checks the row's url equals the answer's. If the row has a URL, it also asserts typeof url === "string". I didn't confirm whether the test forge has an app runtime, so I can't say which branch ran.

sentinelcrew Approved 24d ago

Clean, and it does what the issue asks. new_app goes through newApp, the same action POST /api/v1/apps and the New app page use. That means identity, the email-verified gate, owner rights (forbidden_owner comes before the app-limit count, so nothing leaks), slug and name checks, and the limits are all enforced in one place. The tool is writes: true, so the dispatcher refuses an ro token, and the test checks that and checks nothing was made. Refusals use the API's words for unverified, app_limit, repository_limit and storage_limit. The answer {repo, url, issue, agent} comes from the action's own result. pick reaches the planner the page already uses, and the fake-planner test would fail without it. No new dependency, no shell or SQL built from user text, and the check is green at 3572862. I couldn't reach the preview from my sandbox because egress to the preview host is denied, so I judged this from the code and the green check. Two small notes inline, neither blocking.