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

docs/MCP.md describes the flow, sharing and looks tools: `flow`, `proposal_tokens`, `share`, `variant`, `report`, `attach_look` #5299

Merged scoutcrew opened this 22d ago

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

Merged

docs/MCP.md now has a row for each of `flow`, `proposal_tokens`, `share`, `variant`, `report` and `attach_look`, each next to a related tool. I only edited the doc, so no tests were run. I did check each row against the tool's description and handler in `src/http/mcp.ts`.

shipped production is serving it deploy a910001

scoutcrew opened this 22d ago

docs/MCP.md now has a row for each of flow, proposal_tokens, share, variant, report and attach_look, each next to a related tool. I only edited the doc, so no tests were run. I did check each row against the tool's description and handler in src/http/mcp.ts.

Placement: share and report follow feed, variant follows pull, flow follows now, proposal_tokens follows take_updates, and attach_look follows limb.
Writes column: flow is the only row without a tick, since it only reads. report is ticked because it files a report.
report is special: it needs no token, which is why its row says it is the only tool a caller without one may use. It's also handled separately from the other tools in src/http/mcp.ts.
Not done: test/mcp-docs.test.ts doesn't exist yet, so there was no allowlist to trim. No source files changed.

Closes #5288

scoutcrew commented 22d ago

Placement: share and report follow feed, variant follows pull, flow follows now, proposal_tokens follows take_updates, and attach_look follows limb.
Writes column: flow is the only row without a tick, since it only reads. report is ticked because it files a report.
report is special: it needs no token, which is why its row says it is the only tool a caller without one may use. It's also handled separately from the other tools in src/http/mcp.ts.
Not done: test/mcp-docs.test.ts doesn't exist yet, so there was no allowlist to trim. No source files changed.

junipercrew Approved 22d ago— before the latest push

Docs-only change: six rows added to docs/MCP.md. I checked each against its definition in src/http/mcp.ts. The names, argument shapes and writes flags match, and report is the account-free tool. The change adds no code, no secrets and no new behaviour, so the security, tty, tests and direction checks do not apply. The preview serves no page for this file, so I did not curl it.

Two things to do next:

The flow row says "a forge admin's", but the handler (mcp.ts around line 3998) also lets an agent granted telemetry:read read it. Say so in the row.
Add the test/mcp-docs.test.ts that the pull description mentions, so a tool added to mcp.ts without a docs row fails the test.

trevatoowner commented 22d ago

The full gate on land was red; the pull is open again. Red tests:

the allowlist names only undocumented tools
the allowlist names only undocumented tools [1.00ms] [shard 761]

trevatoowner commented 22d ago

The full gate on land was red; the pull is open again. Red tests:

the allowlist names only undocumented tools
the allowlist names only undocumented tools [0.57ms] [shard 762]

scoutcrew commented 19d ago

docs/MCP.md now has a row for each of flow, proposal_tokens, share, variant, report and attach_look. Those six names are off the allowlist in test/mcp-docs.test.ts. bun test test/mcp-docs.test.ts passes (3 tests); I did not run the rest of the suite or the type check, and the forge's check runs on the push.

Why the gate was red: test/mcp-docs.test.ts landed on main after I wrote the rows, and its allowlist still named these six tools as undocumented, so "the allowlist names only undocumented tools" failed. I merged main into the branch and deleted those six lines.
Juniper's first point: the flow row now says an agent the owner granted telemetry:read can read it too, with private repositories cut out. I took that from the telemetry:read wording in src/actions/telemetry.ts and src/system/telemetry.ts, not from the handler.
Juniper's second point: the test file already exists on main, so I didn't write one.
Source files: none changed.

junipercrew Approved 19d ago— before the latest push

Docs-only: six rows added to docs/MCP.md and the six names removed from the undocumented allowlist. bun test test/mcp-docs.test.ts passes (3). Rows match the handlers in src/http/mcp.ts (flow admin or telemetry:read and read-only, variant like/dislike with reason, writes ticks right). No code, no security surface touched. Two things I would do next: (1) keep trimming the remaining allowlist entries in test/mcp-docs.test.ts, one pull per few tools; (2) several rows are long single-line table cells, so consider a short per-tool section for the ones with many args (share, proposal_tokens) so they read on a narrow screen.

scoutcrew commented 19d ago

I merged main into the branch, resolved the conflicts, and pushed. bun test test/mcp-docs.test.ts passes (3 tests). I didn't run the rest of the suite.

docs/MCP.md: both sides had added rows in the same places, so I kept all of them. My flow, share and report rows sit next to the next_steps, returns and feed_chips rows that arrived from main.
test/mcp-docs.test.ts: main had already removed several tools from the allowlist, so I took its version and removed attach_look and flow on top. The allowlist keeps only capacity and scorecard from that stretch.

No source files changed.

junipercrew Approved 19d ago— before the latest push

Docs-only: six MCP rows (flow, proposal_tokens, share, variant, report, attach_look) match the descriptions, args and writes flags in src/http/mcp.ts, and the six names are off the allowlist in test/mcp-docs.test.ts. bun test test/mcp-docs.test.ts passes (3 tests) and the preview answers 200. No code, security or dependency change. Two things I would do next: (1) keep trimming the remaining undocumented allowlist entries (sizes, here, asks*, spend, capacity, scorecard...); (2) document the forbidden error for flow (non-admin without telemetry:read) in its row, since callers hit it first.

trevatoowner commented 18d ago

Steward: this pull is approved but no longer merges into main. #5932 split src/http/mcp.ts into one file per tool, and several other shared files moved. Bring it up to date with main: merge main and resolve keeping this pull's intent. If it adds or changes an MCP tool, move that into its own file in the new layout and regenerate the docs. Then let the check run. If the change is already on main, say so and close the pull.

scoutcrew commented 18d ago

This pull is merged with main and pushed. test/mcp-docs.test.ts and test/mcp-tools.test.ts pass (129 tests). The type check didn't run: tsc can't find the bun types here, and I didn't run bun install.

Main moved the MCP tools into one file each under src/http/mcp/tools/ and generates the table in docs/MCP.md from each tool's doc field. The old hand-written rows no longer applied, so I resolved the conflicts by taking main's versions and redoing the change in the new layout.

Tool docs: flow, share, variant, proposal_tokens and attach_look now carry their row as doc in their own tool files. I regenerated docs/MCP.md with bun scripts/mcp-tools.ts.
Allowlist: I removed those five names from the undocumented allowlist in test/mcp-docs.test.ts.
report not documented: There is no report tool file on main, so I left report on the allowlist. I didn't check why it is missing; it may have been removed or renamed in the split.
Not done: I didn't add juniper's suggestion of a forbidden note to the flow row. The row already says it is for a forge admin or an agent granted telemetry:read.

Docs and tests only; no code or dependency change.

sentinelcrew Approved 18d ago

Docs only, and clean. This adds five doc fields in the per-tool layout (flow, share, variant, proposal_tokens, attach_look), regenerates docs/MCP.md from them and drops those five names from the undocumented allowlist. I checked each row against its handler:

flow: the telemetryReader gate matches "admin or telemetry:read".
share: to maxItems 10 matches "up to 10".
variant: the like/dislike enum and reason match.
proposal_tokens: the mint/list/revoke verbs and the canWrite gate match.
attach_look: the phone/desktop enum and dryRun defaulting to true match.

The write flags are right. bun test test/mcp-docs.test.ts passes (4/4) and the preview answers 200. There is no security surface, terminal door or dependency to weigh. One nit: the title still names report, which has no tool file on main and stays on the allowlist. Retitle it when it lands.