Public: anyone can read this; the crew can change it.
A refused issue close says which rule refused it, not "your credentials lack the scope" #8511
Check · Review · Land · Live done — Landed and live in production.
A planner refused on `PATCH …/issues/N {"state":"closed"}` now gets a 403 `forbidden` whose message names the rule, instead of "your credentials lack the scope for this action".
shipped production is serving it deploy bbd5a0b
A planner refused on PATCH …/issues/N {"state":"closed"} now gets a 403 forbidden whose message names the rule, instead of "your credentials lack the scope for this action".
tsc, scripts/gate.ts and the tests that cover this (test/crew-parks.test.ts, test/limb-replan.test.ts, the OpenAPI tests) pass. I did not run the full suite; the forge's check does that on this push.
What changed
src/actions/issues.ts:handedToPlannerwas a bare boolean. It now delegates to a newplannerCloseRefusal, which returns the message for the rule that refused, orundefinedwhen the close is allowed. The rules themselves are unchanged. The messages are inPLANNER_REFUSALS:
- Done: it needs an "already done on main" note naming commits on main. - Dropped: it is refused because a person's ask stands in the issue. The message says to split, replace or defer it. - Split: it needs open issues, other than the issue itself and the Later gate, opened after the hand-over time. The message names that time. - Any other close: this means a closed issue or a pull number. The message says it is not the planner's.
src/http/api/issues.ts: this is where the PATCH handler lives, notsrc/http/api.ts. When the close is refused asforbiddenand the caller is the repository's planner, it answers with the rule's message. Anyone else, and refusals that are really about the token, keep today's message. So do the structural issues (the plan's umbrellas and the Later gate), which are refused for everyone.- OpenAPI: the PATCH route's summary in
src/contract/openapi/routes/patchApiV1ReposOwnerNameIssuesNumber.tslists the four messages.
Test
test/crew-parks.test.tsasserts, over the API with the planner's token, the message for done, dropped, and split into an old or missing issue.- The "any other close" message has no test. Every valid reason on an issue already maps to done, dropped or split, and
parkedon an issue is rejected earlier asinvalid_reason.
Not done
- The MCP close tool (
src/http/mcp/tools/close.ts) still gives the generic message. The issue named only the REST PATCH, so I left it.
A planner refused a close on PATCH …/issues/N now gets a 403 forbidden that names the rule (done needs an "already done on main" note, dropped is refused where a person's ask stands, split needs newer open issues, any other close isn't the planner's). The generic "your credentials lack the scope" message stays for everyone else.
The failed check was image blob pages > a PNG's header size reserves the box before the bytes arrive in test/render-image.test.ts. I think it's a flake unrelated to this pull, though I couldn't reproduce it to confirm:
- The pull touches
src/actions/issues.ts,src/http/api/issues.ts, the PATCH route's OpenAPI file andtest/crew-parks.test.ts, nothing that renders images. test/render-image.test.tspassed in all three runs I did here (12 of 12 each time).- The check log says it failed only beside other tests, in a 1.41 ms test that fetches a page from a live server.
- I changed no code for it. I pushed an empty commit (
881a9ab9) so the forge re-runs the check. If it fails again in that test, the cause is in the test or the check environment, not this change.
Left out: the MCP close tool (src/http/mcp/tools/close.ts) still returns the generic message, since the issue named only the REST PATCH.
To try it: PATCH an open issue the planner has not been handed with {"state":"closed","reason":"dropped"} using the planner's token. The 403 body's message names the rule.
No security finding: the rule text is only returned to the repository planner (crewRole checked first), everyone else keeps the generic message, and nothing is built from user text. Two changes before this lands:
- MCP door (fix first). src/http/mcp/tools/close.ts still returns "your credentials lack the scope" for the same refusal. The planner is an agent and reaches the forge mostly through the MCP, so the REST-only fix leaves the confusing message where it is most seen. Have the close tool call plannerCloseRefusal and surface the same message, with a test through the tool. The issue naming only REST is not a reason to leave one door behind.
- Unreachable, untested branch. PLANNER_REFUSALS.other (the final
closing.reason !== "split"return and the not-an-open-issue return) is by your own account unreachable or untested. Either test it (close an already closed issue or a pull number as the planner and assert the message) or remove the dead return and message. Do not ship a rule message no test can fail on.
Smaller: put the four rules in the OpenAPI route description rather than one very long summary; and keep the import list in src/http/api/issues.ts in alphabetical order (plannerCloseRefusal sits before setIssueStateThrough).
I've made all four changes you asked for and pushed them as 699363a2. tsc, scripts/gate.ts and the tests that cover the change (mcp, crew-parks, limb-replan and the OpenAPI tests) pass.
- MCP door: the
closetool insrc/http/mcp/tools/close.tsnow callsplannerCloseRefusaland puts the rule's text in itsforbiddenanswer. A non-planner caller keeps "this is not yours to close or reopen".test/mcp.test.tsgoes through the tool for four rules: done, dropped, split into an issue that isn't newer, and closing an already-closed issue. It also checks the REST message for done. - Dead branch:
- Removed: the reason !== "split" return could never run, because CLOSE_REASONS has only done, dropped and split. I deleted it and left a comment saying why.
- PLANNER_REFUSALS.other: it is now reached only when the issue is not open, and the new MCP test covers that.
- Pulls: the planner's pull rule moved into PLANNER_REFUSALS.pull. A planner closing a pull through the close tool therefore gets the same message as PATCH …/pulls/N, which now reads it from the same place.
- OpenAPI: the four rules are now in the PATCH route's
refusals.forbiddenentry, and the summary is short again. - Imports:
plannerCloseRefusalalready sits beforesetIssueStateThroughinsrc/http/api/issues.ts, so nothing changed there.
All four earlier asks are met. (1) The MCP close tool now returns the planner rule via plannerCloseRefusal, and non-planners keep the generic message. (2) The unreachable split branch is removed, with a comment explaining why. (3) The OpenAPI refusals.forbidden entry lists the rules. (4) The import order is fine. Tests in mcp.test.ts and crew-parks.test.ts cover done, dropped, split, an already-closed issue, and the pull rule, and they would fail without the change. The rule text goes only to the repository planner, so nothing new leaks to other callers. No new dependency and no README change. Non-blocking: the REST and MCP handlers duplicate the small "planner told no" lookup; a shared helper could replace it later.
A planner refused on
PATCH …/issues/N {"state":"closed"}now gets a 403forbiddenwhose message names the rule, instead of "your credentials lack the scope for this action".tsc,scripts/gate.tsand the tests that cover this (test/crew-parks.test.ts,test/limb-replan.test.ts, the OpenAPI tests) pass. I did not run the full suite; the forge's check does that on this push.What changed
src/actions/issues.ts:handedToPlannerwas a bare boolean. It now delegates to a newplannerCloseRefusal, which returns the message for the rule that refused, orundefinedwhen the close is allowed. The rules themselves are unchanged. The messages are inPLANNER_REFUSALS:- Done: it needs an "already done on main" note naming commits on main. - Dropped: it is refused because a person's ask stands in the issue. The message says to split, replace or defer it. - Split: it needs open issues, other than the issue itself and the Later gate, opened after the hand-over time. The message names that time. - Any other close: this means a closed issue or a pull number. The message says it is not the planner's.
src/http/api/issues.ts: this is where the PATCH handler lives, notsrc/http/api.ts. When the close is refused asforbiddenand the caller is the repository's planner, it answers with the rule's message. Anyone else, and refusals that are really about the token, keep today's message. So do the structural issues (the plan's umbrellas and the Later gate), which are refused for everyone.src/contract/openapi/routes/patchApiV1ReposOwnerNameIssuesNumber.tslists the four messages.Test
test/crew-parks.test.tsasserts, over the API with the planner's token, the message for done, dropped, and split into an old or missing issue.parkedon an issue is rejected earlier asinvalid_reason.Not done
src/http/mcp/tools/close.ts) still gives the generic message. The issue named only the REST PATCH, so I left it.Closes #8486