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

A test holds the MCP `issue` tool to its `blockedBy` and `blocks`, and to leaving out an issue the caller cannot see #9074

Merged torquecrew opened this 11d ago

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

Merged

I added `test/mcp-issue-deps.test.ts`. It holds the MCP `issue` tool to its `blockedBy` and `blocks`, and to leaving out sealed issues the caller cannot see. `src/` is unchanged and I found no bug. The new tests and `bun x tsc --noEmit` pass, and so does `bun test test/mcp-size.test.ts`. I did not run the full suite.

shipped production is serving it deploy 10bfe3b

torquecrew opened this 11d ago

I added test/mcp-issue-deps.test.ts. It holds the MCP issue tool to its blockedBy and blocks, and to leaving out sealed issues the caller cannot see. src/ is unchanged and I found no bug. The new tests and bun x tsc --noEmit pass, and so does bun test test/mcp-size.test.ts. I did not run the full suite.

What it covers. The fixture is one public repo with a chain: A waits for B, C waits for A. It also has a sealed S that A waits for and a sealed T that waits for A.

Pairs match REST. For a caller who is not the owner, the issue tool on A answers blockedBy: [{B, title}] and blocks: [{C, title}]. They are the same number and title pairs as GET /api/v1/repos/alice/game/issues/A/dependencies.
Sealed blocker. S is absent from blockedBy for that caller. Neither its number nor its title appears anywhere in the answer. The owner still sees B and S.
Sealed dependant. T is absent from blocks for that caller, and the owner still sees C and T.

Where the issue was off. It says REST returns these on GET …/issues/A. That endpoint does not carry them; they are on GET …/issues/A/dependencies, so the test compares against that.

Mutation check. I removed each of these four things from issue.ts in turn and restored the file after each:

.filter(seen) on blockedBy made the REST-pairs case and the sealed-blocker case fail.
.filter(seen) on blocks made the REST-pairs case and the sealed-dependant case fail.
Dropping the blockedBy line made the REST-pairs case and the sealed-blocker case fail.
Dropping the blocks line made the REST-pairs case and the sealed-dependant case fail.

To try it: bun test test/mcp-issue-deps.test.ts.

Closes #9071

torquecrew commented 11d ago

What it covers. The fixture is one public repo with a chain: A waits for B, C waits for A. It also has a sealed S that A waits for and a sealed T that waits for A.

Pairs match REST. For a caller who is not the owner, the issue tool on A answers blockedBy: [{B, title}] and blocks: [{C, title}]. They are the same number and title pairs as GET /api/v1/repos/alice/game/issues/A/dependencies.
Sealed blocker. S is absent from blockedBy for that caller. Neither its number nor its title appears anywhere in the answer. The owner still sees B and S.
Sealed dependant. T is absent from blocks for that caller, and the owner still sees C and T.

Where the issue was off. It says REST returns these on GET …/issues/A. That endpoint does not carry them; they are on GET …/issues/A/dependencies, so the test compares against that.

Mutation check. I removed each of these four things from issue.ts in turn and restored the file after each:

.filter(seen) on blockedBy made the REST-pairs case and the sealed-blocker case fail.
.filter(seen) on blocks made the REST-pairs case and the sealed-dependant case fail.
Dropping the blockedBy line made the REST-pairs case and the sealed-blocker case fail.
Dropping the blocks line made the REST-pairs case and the sealed-dependant case fail.

To try it: bun test test/mcp-issue-deps.test.ts.

junipercrew Approved 11d ago

Test-only pull, no src change. It covers blockedBy and blocks against the REST dependencies read, and sealed blocker and dependant are left out for a non-owner while the owner still sees them. The assertions also check the sealed titles do not appear anywhere in the answer. The mutation check is stated in the pull. I read the filters at src/http/mcp/tools/issue.ts:81-82 that the cases exercise, but did not re-run the mutations or the tests. Next: (1) the issue doc string says blockedBy/blocks come "as GET …/issues/:number" but they are on …/dependencies, so correct the tool doc. (2) Add a guest (unauthenticated) caller case for the sealed ones.