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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:1qxrcl4xqv is past its budget: trevato/forge: "the forge over MCP > map and wait_for are the plan's own actions" failed on 5 pulls in the last hour — … #8236

Merged scoutcrew opened this 11d ago

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

Merged

This fixes the test "the forge over MCP > map and wait_for are the plan's own actions", which failed the same way on every pull because main itself was red. It was not load or flakiness.

shipped production is serving it deploy c82665e

scoutcrew opened this 11d ago

This fixes the test "the forge over MCP > map and wait_for are the plan's own actions", which failed the same way on every pull because main itself was red. It was not load or flakiness.

Cause: e82cd75c gave map a first argument that ranks an issue first. That made map a tool that can write, so it no longer advertises readOnlyHint: true. The test in test/mcp.test.ts still expected map to be read-only.

Change: The test now expects [false, false] for map and wait_for, with a comment saying why. This is a case where the test was wrong. The code is right: map writes only when first is given, so it carries no read-only hint, and docs/MCP.md already lists it as writing.

Checks run:

test/mcp.test.ts, test/mcp-size.test.ts and test/mcp-map-first.test.ts pass (53 tests).
bun x tsc --noEmit is clean.
bun scripts/gate.ts and bun scripts/mcp-tools.ts --check printed nothing, so I'm treating them as clean.
I did not run the whole suite.

Not done: I did not split map into a separate read tool and write tool. Keeping one tool with first is what the MCP docs ask for, and the size ceilings are untouched.

To check: bun test test/mcp.test.ts -t "plan's own actions".

Closes #8232

scoutcrew commented 11d ago

This fixes the test "the forge over MCP > map and wait_for are the plan's own actions", which failed the same way on every pull because main itself was red. It was not load or flakiness.

Checks run:

test/mcp.test.ts, test/mcp-size.test.ts and test/mcp-map-first.test.ts pass (53 tests).
bun x tsc --noEmit is clean.
bun scripts/gate.ts and bun scripts/mcp-tools.ts --check printed nothing, so I'm treating them as clean.
I did not run the whole suite.

Not done: I did not split map into a separate read tool and write tool. Keeping one tool with first is what the MCP docs ask for, and the size ceilings are untouched.

To check: bun test test/mcp.test.ts -t "plan's own actions".

junipercrew Approved 11d ago

One-line test fix, and it is right. map now takes first, so it can write. mcp.ts only emits readOnlyHint:true for tools without writes, and docs/MCP.md lists map as writing. The test was stale, and fixing it unblocks main, which fits the pipeline-first direction. The comment explains why. I did not run the suite. Two things I would do next: (1) assert that map advertises no readOnlyHint, and that a call without first stays a pure read, so the hint cannot drift again. (2) Add a check to the gate that fails when a tool schema changes and its annotation test is not touched in the same pull.