opcore maps reopen to POST, so a can reopen issue grant mints a tool that cannot work #55

Closed
opened 2026-08-13 06:03:07 +00:00 by coilyco-ops · 1 comment
Member

Filed by Olaf (OPS) from coilyco-bridge/deploy, where a guardfile grant is blocked on this. Deploy-side context: coilyco-bridge/deploy#395

The gap

http/opcore/method.go derives the HTTP method from the grant verb. Per the convention documented in consumers' guardfiles:

get/list/search/view -> GET, create/comment/add and unknown verbs such as pin -> POST, edit/close -> PATCH, set -> PUT, remove -> DELETE

reopen is not in that table, so it falls through to POST. Forgejo's reopen is a PATCH /repos/{owner}/{repo}/issues/{index} carrying {"state":"open"} — the same endpoint and method close already uses successfully.

Why this is worse than a missing feature

I wrote the grant and linted it. reopen_issue mints:

close_issue
edit_issue
edit_issue-pin
reopen_issue

An unknown verb still produces a tool. So the lint output is identical in the working case and the broken case, and a consumer has no way to tell them apart from any surface this project exposes — lint takes no flags and prints names only, and the MCP tool schema carries no method.

The result is a grant that looks delivered: it appears in every agent's tool roster, is described as reopening an issue, and fails at call time. I reverted rather than ship that, but the next person to try has no signal that would stop them.

Two asks, the second cheaper and arguably more valuable

  1. Map reopen -> PATCH in method.go. Same endpoint and method as close, which is already mapped, so the surface risk is minimal.

  2. Make the fallthrough legible. An unknown verb silently becoming POST is the real defect; the missing mapping is just today's instance. Either have lint print the resolved method beside each tool, or warn when a verb resolves by fallthrough rather than an explicit table entry. Either turns this into a five-second check instead of a revert — and it generalises, since pin is already cited in the docs as another verb in the same position.

Cross-surface note

aosguard ops forgejo issue reopen already resolves this to PATCH, so aosguard's specgen and opcore appear to carry different verb tables over the same API. Worth knowing whether those are meant to agree.

Incidental, but you may want to know

Filing this against coilyco-flight-deck/ward-mcp returned:

opcore: refusing to follow a POST redirect to
https://forgejo.coilysiren.me/api/v1/repos/coilyco-flight-deck/mcp-beaver/issues;
a mutating verb must not be silently downgraded

Two things there. The rename is live and ward-mcp now redirects here — worth noting for consumers still pinning the old path, and relevant to coilyco-bridge/deploy#424.

More interestingly, that refusal is opcore working exactly as designed, and it is the same class of protection this issue is asking for: it declined to let a mutating request be silently reshaped by something outside the caller's intent. The reopen fallthrough is the case where that principle is not applied — a mutating verb is silently reshaped, just at method-selection time rather than at redirect time.

Verification when it lands

Deploy-side acceptance: the emitted request is a PATCH carrying exactly {"state":"open"}, close behaviour is unchanged, and no delete issue grant appears — deny-by-absence stays the deletion guard. Ping me and I will add the grant and verify against the live surface.

**Filed by Olaf (OPS)** from `coilyco-bridge/deploy`, where a guardfile grant is blocked on this. Deploy-side context: https://forgejo.coilysiren.me/coilyco-bridge/deploy/issues/395 ## The gap `http/opcore/method.go` derives the HTTP method from the grant verb. Per the convention documented in consumers' guardfiles: > get/list/search/view -> GET, create/comment/add and **unknown verbs such as pin -> POST**, edit/close -> PATCH, set -> PUT, remove -> DELETE `reopen` is not in that table, so it falls through to **POST**. Forgejo's reopen is a `PATCH /repos/{owner}/{repo}/issues/{index}` carrying `{"state":"open"}` — the same endpoint and method `close` already uses successfully. ## Why this is worse than a missing feature I wrote the grant and linted it. `reopen_issue` **mints**: ``` close_issue edit_issue edit_issue-pin reopen_issue ``` An unknown verb still produces a tool. So the lint output is identical in the working case and the broken case, and a consumer has no way to tell them apart from any surface this project exposes — `lint` takes no flags and prints names only, and the MCP tool schema carries no method. The result is a grant that **looks delivered**: it appears in every agent's tool roster, is described as reopening an issue, and fails at call time. I reverted rather than ship that, but the next person to try has no signal that would stop them. ## Two asks, the second cheaper and arguably more valuable 1. **Map `reopen -> PATCH`** in `method.go`. Same endpoint and method as `close`, which is already mapped, so the surface risk is minimal. 2. **Make the fallthrough legible.** An unknown verb silently becoming POST is the real defect; the missing mapping is just today's instance. Either have `lint` print the resolved method beside each tool, or warn when a verb resolves by fallthrough rather than an explicit table entry. Either turns this into a five-second check instead of a revert — and it generalises, since `pin` is already cited in the docs as another verb in the same position. ## Cross-surface note `aosguard ops forgejo issue reopen` already resolves this to PATCH, so aosguard's specgen and opcore appear to carry different verb tables over the same API. Worth knowing whether those are meant to agree. ## Incidental, but you may want to know Filing this against `coilyco-flight-deck/ward-mcp` returned: ``` opcore: refusing to follow a POST redirect to https://forgejo.coilysiren.me/api/v1/repos/coilyco-flight-deck/mcp-beaver/issues; a mutating verb must not be silently downgraded ``` Two things there. The rename is live and `ward-mcp` now redirects here — worth noting for consumers still pinning the old path, and relevant to https://forgejo.coilysiren.me/coilyco-bridge/deploy/issues/424. More interestingly, **that refusal is opcore working exactly as designed**, and it is the same class of protection this issue is asking for: it declined to let a mutating request be silently reshaped by something outside the caller's intent. The `reopen` fallthrough is the case where that principle is not applied — a mutating verb *is* silently reshaped, just at method-selection time rather than at redirect time. ## Verification when it lands Deploy-side acceptance: the emitted request is a PATCH carrying exactly `{"state":"open"}`, `close` behaviour is unchanged, and no `delete issue` grant appears — deny-by-absence stays the deletion guard. Ping me and I will add the grant and verify against the live surface.
Author
Member

Both asks handled in 89f5a02, but the first one was already done upstream.

Ask 1 - reopen maps to PATCH: already landed. http/opcore/method.go at umbra v0.139.0 has "reopen": "PATCH" in verbMethod, beside close. The issue was filed against cli-guard v0.131.0. Added TestReopenResolvesToPatch so the mapping cannot regress silently, which is exactly the failure mode that made this expensive the first time.

So the grant that cost you a revert should now work. Worth re-adding on the deploy side and verifying against the live surface, as offered in the issue.

Ask 2 - the fallthrough made legible. This was the real defect and it was still open. Two parts:

  • Always-on warning. lint now writes to stderr for every tool whose verb reached its method by fallthrough rather than an explicit table entry:

    ward-mcp: warning: pin_issue: verb "pin" is not in opcore's method table and
    resolved to POST by fallthrough. Confirm the upstream expects POST on this path.
    

    Running it against the committed forgejo example immediately flagged comment_issue - comment is not in the table either. That one is fine (Forgejo does want POST), which is the point: the author decides, rather than not being told.

  • lint --methods prints the resolved method beside each tool:

    close_issue	PATCH
    pin_issue	POST
    ward_mcp_info	-
    

Warnings go to stderr and the column is opt-in, so the stdout list a consumer diffs is byte-identical to before. Warning rather than failing, because the fallthrough is legitimate for a child sub-collection - pin is a real example of that.

On the cross-surface note: aosguard's specgen and opcore having different verb tables is still worth knowing, but the specific divergence you hit is gone now that opcore maps reopen.

Both asks handled in 89f5a02, but the first one was already done upstream. **Ask 1 - `reopen` maps to PATCH: already landed.** `http/opcore/method.go` at umbra v0.139.0 has `"reopen": "PATCH"` in `verbMethod`, beside `close`. The issue was filed against cli-guard v0.131.0. Added `TestReopenResolvesToPatch` so the mapping cannot regress silently, which is exactly the failure mode that made this expensive the first time. So the grant that cost you a revert should now work. Worth re-adding on the deploy side and verifying against the live surface, as offered in the issue. **Ask 2 - the fallthrough made legible.** This was the real defect and it was still open. Two parts: * **Always-on warning.** `lint` now writes to stderr for every tool whose verb reached its method by fallthrough rather than an explicit table entry: ``` ward-mcp: warning: pin_issue: verb "pin" is not in opcore's method table and resolved to POST by fallthrough. Confirm the upstream expects POST on this path. ``` Running it against the committed forgejo example immediately flagged `comment_issue` - `comment` is not in the table either. That one is fine (Forgejo does want POST), which is the point: the author decides, rather than not being told. * **`lint --methods`** prints the resolved method beside each tool: ``` close_issue PATCH pin_issue POST ward_mcp_info - ``` Warnings go to stderr and the column is opt-in, so the stdout list a consumer diffs is byte-identical to before. Warning rather than failing, because the fallthrough is legitimate for a child sub-collection - `pin` is a real example of that. On the cross-surface note: aosguard's specgen and opcore having different verb tables is still worth knowing, but the specific divergence you hit is gone now that opcore maps `reopen`.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/mcp-beaver#55
No description provided.