You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Tool errors: expose the GitHub HTTP status and an error kind in _meta #3424
Describe the feature or problem you'd like to solve
When a write tool's GitHub call fails, the client gets isError: true and one text item. It cannot tell "GitHub refused, nothing happened" from "the write may have happened." On v1.14.0 (merge_pull_request, http mode, a fake GitHub REST API on loopback via --gh-host), these three arrive the same way:
failed to merge pull request: PUT http://…/pulls/4471/merge: 409 Head branch was modified. Review and try the merge again. []
failed to merge pull request: Put "http://…/pulls/4471/merge": EOF
failed to merge pull request: unexpected EOF
Only the first means nothing merged. In the second, the connection dropped after GitHub merged. In the third, GitHub answered 200 and the body was cut short (go-github's Do() returns the response with the read error). So the server reports a failed merge that succeeded.
This decides retry behaviour. A client that retries on isError repeats writes that happened. That is harmless for a merge, which GitHub refuses the second time, but not for add_issue_comment, create_pull_request or push_files. A client that never retries cannot act on a definitive 409 or 422. expectedHeadSha (#3182) made the merge itself safe to retry, but the client still cannot tell which case it is in, short of parsing English for a status code.
The server already has this. NewGitHubAPIErrorResponse builds a GitHubAPIError holding the *github.Response and keeps it in context for middleware (docs/error-handling.md). None of it reaches the client.
Proposed solution
In NewGitHubAPIErrorResponse, and so in NewGitHubAPIStatusErrorResponse, add a small _meta entry built from the existing GitHubAPIError. The text content stays the same:
There is precedent for data on an error result.NewToolResultAwaitingFormSubmission already returns isError: true with structured data.
REST first.GitHubGraphQLError has no response, so GraphQL would be a follow-up.
The key name is yours to choose. The repo uses unprefixed keys (ifc, ui).
Example prompts or workflows (for tools/toolsets only)
A human approves merging a PR at head a1b2c3d, and someone pushes. The merge with expectedHeadSha comes back http_error/409: the client knows nothing merged and asks for approval at the new head.
The same merge comes back transport_error or response_read_error: the client reads the PR with pull_request_read before doing anything else.
add_issue_comment comes back transport_error: the client checks the thread before posting again, instead of posting the comment twice.
push_files comes back rate_limited: the client waits and retries, because GitHub said nothing was written.
Additional context
I maintain ctrlrun, an MCP gateway that records whether each write happened. In front of this server it has to treat every isError as unknown, so a stale-SHA 409 needs a human before the agent can retarget. The runs above are from a harness with eleven scenarios against the real server and a fake GitHub: https://github.com/CTRLRun/ctrlrun/tree/fe8374ece71aa0cf20fca8e10ffc139ccaeb4831/research/github-merge-head-race. Related: #2636, where a mutation succeeds and no response returns.
I'm happy to send the PR, REST only, with a test per kind.
Describe the feature or problem you'd like to solve
When a write tool's GitHub call fails, the client gets
isError: trueand one text item. It cannot tell "GitHub refused, nothing happened" from "the write may have happened." On v1.14.0 (merge_pull_request,httpmode, a fake GitHub REST API on loopback via--gh-host), these three arrive the same way:Only the first means nothing merged. In the second, the connection dropped after GitHub merged. In the third, GitHub answered
200and the body was cut short (go-github'sDo()returns the response with the read error). So the server reports a failed merge that succeeded.This decides retry behaviour. A client that retries on
isErrorrepeats writes that happened. That is harmless for a merge, which GitHub refuses the second time, but not foradd_issue_comment,create_pull_requestorpush_files. A client that never retries cannot act on a definitive409or422.expectedHeadSha(#3182) made the merge itself safe to retry, but the client still cannot tell which case it is in, short of parsing English for a status code.The server already has this.
NewGitHubAPIErrorResponsebuilds aGitHubAPIErrorholding the*github.Responseand keeps it in context for middleware (docs/error-handling.md). None of it reaches the client.Proposed solution
In
NewGitHubAPIErrorResponse, and so inNewGitHubAPIStatusErrorResponse, add a small_metaentry built from the existingGitHubAPIError. The text content stays the same:kindis one of:http_error: a non-2xx responserate_limited/secondary_rate_limitedtransport_error: no responseresponse_read_error: a 2xx response with an errorcanceledNo schema change and no token cost. The text the model reads is unchanged.
It reports what was observed, not "not performed". A 5xx can leave the outcome unknown, and the client decides.
_metarather thanstructuredContent. That stays clear of the output-schema question in Reply tool calls with structuredContent #1929, and of the typed-output layer in feat(inventory): add typed MCP tool registration foundation #3371, which drops structured output on errors.There is precedent for data on an error result.
NewToolResultAwaitingFormSubmissionalready returnsisError: truewith structured data.REST first.
GitHubGraphQLErrorhas no response, so GraphQL would be a follow-up.The key name is yours to choose. The repo uses unprefixed keys (
ifc,ui).Example prompts or workflows (for tools/toolsets only)
a1b2c3d, and someone pushes. The merge withexpectedHeadShacomes backhttp_error/409: the client knows nothing merged and asks for approval at the new head.transport_errororresponse_read_error: the client reads the PR withpull_request_readbefore doing anything else.add_issue_commentcomes backtransport_error: the client checks the thread before posting again, instead of posting the comment twice.push_filescomes backrate_limited: the client waits and retries, because GitHub said nothing was written.Additional context
I maintain ctrlrun, an MCP gateway that records whether each write happened. In front of this server it has to treat every
isErroras unknown, so a stale-SHA409needs a human before the agent can retarget. The runs above are from a harness with eleven scenarios against the real server and a fake GitHub: https://github.com/CTRLRun/ctrlrun/tree/fe8374ece71aa0cf20fca8e10ffc139ccaeb4831/research/github-merge-head-race. Related: #2636, where a mutation succeeds and no response returns.I'm happy to send the PR, REST only, with a test per kind.