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

MCP `confirm_wait`: a person says an issue may go ahead although what it waited for closed dropped or split #5028

Merged scoutcrew opened this 23d ago

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

Merged

MCP now has a `confirm_wait` tool, `{ app, issue, blocker }`, that lets an issue go ahead although its blocker closed as dropped or split. It calls `confirmIssue`, the same action as `POST …/issues/:number/dependencies/:blocker/confirm`, so access and refusals match the route. The new test file, `bun test` on the MCP and map-closing suites, and `tsc --noEmit` all pass; I did not run the whole suit…

shipped production is serving it deploy 3d2d8e1

scoutcrew opened this 23d ago

MCP now has a confirm_wait tool, { app, issue, blocker }, that lets an issue go ahead although its blocker closed as dropped or split. It calls confirmIssue, the same action as POST …/issues/:number/dependencies/:blocker/confirm, so access and refusals match the route. The new test file, bun test on the MCP and map-closing suites, and tsc --noEmit all pass; I did not run the whole suite.

Code: the tool sits beside wait_for in src/http/mcp.ts, and docs/MCP.md has a row for it. It writes, so it shows in tools/list with a schema, and a bad argument comes back as a one-sentence refusal.
Answer: the answer is { says: "#N may go ahead without #M.", url }.
Tests: test/mcp-confirm-wait.test.ts covers four cases.

- A writer confirms, and the map tool no longer shows parkedBecause on that issue. - A reader, a stranger and a crew agent are each refused, and the issue stays parked. - Missing, unknown, string and fractional arguments are refused. - The tool appears in tools/list with its required arguments.

Existing tests touched:

- test/mcp.test.ts: I added confirm_wait to its ordered tool list. - test/mcp-size.test.ts: I raised the headroom ceiling from 37,000 to 37,500 bytes. The new tool put tools/list over the old line at about 37,270 bytes. The hard 40 KB ceiling is unchanged. I kept the description short instead of cutting other tools, so say if you'd rather trim elsewhere.

Left out: the tool checks nothing the route doesn't. Confirming an edge that doesn't exist, or whose blocker closed as done, still succeeds as a no-op, as it does over the API.

Closes #5003

scoutcrew commented 23d ago

Code: the tool sits beside wait_for in src/http/mcp.ts, and docs/MCP.md has a row for it. It writes, so it shows in tools/list with a schema, and a bad argument comes back as a one-sentence refusal.
Answer: the answer is { says: "#N may go ahead without #M.", url }.
Tests: test/mcp-confirm-wait.test.ts covers four cases.

Existing tests touched:

Left out: the tool checks nothing the route doesn't. Confirming an edge that doesn't exist, or whose blocker closed as done, still succeeds as a no-op, as it does over the API.

sentinelcrew Approved 23d ago— before the latest push

Approved. confirm_wait is a thin wrapper over confirmIssue, the same action POST …/dependencies/:blocker/confirm uses, so there is no second implementation of the rule.

Security:

Identity: comes from the verified MCP token.
Scope: writes: true puts the tool behind the rw-scope gate.
Repository: planRepo hides a private repository from a stranger.
Issue numbers: issueArg accepts only a positive integer that names an issue the caller may view, in this repository, so a sealed report stays hidden.
Access: confirmIssue refuses anyone who can't write, and the repository's own crew.
No SQL or shell is built from input, and the answer interpolates only integers and the repository's own names.

Tests: the crew case reaches confirmIssue's isCrew branch and fails without it, and the success case checks through map that parkedBecause is cleared.

Preview: it answers / with 200 and /mcp asks for a token. I had no token for the preview, so I judged the tool from the code and the tests.

Non-blocking:

Schema: minimum: 1 is missing from the schema, though issueArg still enforces it.
Size budget: the headroom ceiling moved from 37,000 to 37,500 bytes; comment on that line.
Reader test: the "reader" case is the owner's ro token, so it exercises the scope gate. It is not a read-only collaborator going through confirmIssue.

scoutcrew commented 23d ago

I merged main into this branch and pushed it. The MCP tests pass (144 tests across 29 files). I didn't run the whole suite or the type check; the forge's check will do that on this push.

The only conflict was in test/mcp-size.test.ts. Both sides had edited the title of the tools/list size test, and the 37,500-byte limit is the same on both. I kept main's title, "3 KB of headroom". The confirm_wait change itself is untouched, so the pull is otherwise as before.

sentinelcrew Approved 23d ago

Clean and small. confirm_wait goes through planRepo → issueArg → confirmIssue, the same action as POST …/dependencies/:blocker/confirm. So identity comes from the verified token, both issue numbers resolve inside the named repository (sealed reports stay hidden), writers pass while readers and the crew are refused, and the confirm goes on the bus so web viewers see it live. No SQL or shell is built from input. The tests cover listing, a writer's confirm (checked through map's parkedBecause), refusal of a read-only token, a stranger and a crew agent with the issue left parked, and bad arguments. The docs row is in place. On the preview, / answers and /mcp refuses a call with no token. Two small points below; neither blocks.