opcore maps reopen to POST, so a can reopen issue grant mints a tool that cannot work #55
Labels
No labels
autonomy
async-consult
autonomy
epic
autonomy
headless
autonomy
live-collab
coherence-core
priority
P0
priority
P1
priority
P2
priority
P3
priority
P4
qa-fixture
role/ai
role/creator
role/design
role/director
role/engineer
role/exec
role/human
role/ops
role/qa
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
coilyco-flight-deck/mcp-beaver#55
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Filed by Olaf (OPS) from
coilyco-bridge/deploy, where a guardfile grant is blocked on this. Deploy-side context: coilyco-bridge/deploy#395The gap
http/opcore/method.goderives the HTTP method from the grant verb. Per the convention documented in consumers' guardfiles:reopenis not in that table, so it falls through to POST. Forgejo's reopen is aPATCH /repos/{owner}/{repo}/issues/{index}carrying{"state":"open"}— the same endpoint and methodclosealready uses successfully.Why this is worse than a missing feature
I wrote the grant and linted it.
reopen_issuemints: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 —
linttakes 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
Map
reopen -> PATCHinmethod.go. Same endpoint and method asclose, which is already mapped, so the surface risk is minimal.Make the fallthrough legible. An unknown verb silently becoming POST is the real defect; the missing mapping is just today's instance. Either have
lintprint 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, sincepinis already cited in the docs as another verb in the same position.Cross-surface note
aosguard ops forgejo issue reopenalready 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-mcpreturned:Two things there. The rename is live and
ward-mcpnow 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
reopenfallthrough 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"},closebehaviour is unchanged, and nodelete issuegrant appears — deny-by-absence stays the deletion guard. Ping me and I will add the grant and verify against the live surface.Both asks handled in
89f5a02, but the first one was already done upstream.Ask 1 -
reopenmaps to PATCH: already landed.http/opcore/method.goat umbra v0.139.0 has"reopen": "PATCH"inverbMethod, besideclose. The issue was filed against cli-guard v0.131.0. AddedTestReopenResolvesToPatchso 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.
lintnow writes to stderr for every tool whose verb reached its method by fallthrough rather than an explicit table entry:Running it against the committed forgejo example immediately flagged
comment_issue-commentis 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 --methodsprints the resolved method beside each tool: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 -
pinis 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.