Containers: bake the patched forgejo-mcp into the image (merge_pull_request silent-success bug regressed in container mode) #32

New issue

Closed

opened 2026-04-17 21:59:49 +00:00 by claude-desktop · 0 comments

claude-desktop commented

2026-04-17 21:59:49 +00:00

Collaborator

User story

As an operator running workers in container mode, I want the image
to ship a forgejo-mcp binary that surfaces real merge failures
instead of silently returning "Pull request merged successfully" on
non-200 responses — so boss's merge_pull_request calls fail loudly
when the server rejects, and the follow-up can kick in.

Background

The upstream goern/forgejo-mcp v2.17.0 (what the Dockerfile currently
pulls) has a bug in MergePullRequestFn: it discards the bool
success flag from the SDK and checks only err, which is nil for any
HTTP response the SDK managed to parse (including 4xx / 5xx rejects).
Patched locally in-session (track record: update_issue assignee and
merge_pull_request both fixed, built at /tmp/forgejo-mcp, installed
to ~/.local/bin/forgejo-mcp).

When workers ran on the host, PATH put ~/.local/bin before /usr/bin
and the patched binary shadowed the pacman install. In container mode
there is no such shadowing: the image bakes /usr/local/bin/forgejo-mcp
at build time (curl from the upstream release), and that is the only
binary the in-container Claude CLI sees.

Observed 2026-04-17 on PR #31 (the #29 follow-up): boss claimed
"squash-merged successfully, branch dev/29 deleted" in the dashboard
event stream, but the PR was still open and the branch still present.
Forgejo was in fact returning HTTP 500 on the squash call (separate
issue — see Related below) and the buggy MCP swallowed it.

Recovered by docker cp'ing the patched binary into each running
container and completing the merge manually via REST. That recovery is
ephemeral — survives until the next just containers-rebuild.

Acceptance criteria

Dockerfile

The forgejo-mcp build step must produce a binary that returns
an error when the server rejects a merge, not "Pull request merged successfully".
Pick one mechanism (whichever upstream allows first):
1. Preferred: contribute the fix upstream, pin the image to the
  first release that carries it.
2. Otherwise: build from a known-good fork / local tarball. Commit
  the patch (diff, not a blob) to this repo under patches/ and
  apply it during the Dockerfile build, then go build.
Same check for update_issue's dropped assignee — the upstream
code comments out the assignee write ("assignee is not supported
in the current SDK" — the comment is wrong, the SDK has the
field). Both fixes in one binary.

Verification

docker run --rm claude-hooks:<tag> forgejo-mcp --version
reports a version string that makes it clear this is the patched
build (e.g. a dev suffix, or a fork's version).
A hand-exercise: post a merge that the server will reject (e.g.
a PR with a required-check still pending) and confirm the MCP
returns an error string, not the success string.
Existing dockerfile job in qa.yml continues to pass the
static audit (no new credential-leak patterns).

Docs

A brief note in README.md (or wherever the container story
lives) explaining why the image carries a patched binary, with a
pointer to the upstream issue / PR.

Out of scope

The Forgejo-side HTTP 500 on squash merges on this server. PR #31
was closed via a merge-style commit as a workaround. Separate bug,
separate story if it's worth chasing (possibly commit signing
related — the PR page flagged "This instance has no key to sign
this commit with").
The merge.md skill's fallback strategy when squash 500s. If the
server-side squash issue is persistent, merge.md could try
squash first, then fall back to merge with a note. Separate
story.

References

Parent tracking issue: #17
Upstream MCP: codeberg.org/goern/forgejo-mcp (v2.17.0 at time of
writing, patched locally at /tmp/forgejo-mcp)
Local patch sites: operation/pull/pull.go:MergePullRequestFn
(check the bool return from MergePullRequest),
operation/issue/issue.go:UpdateIssueFn (wire the assignee arg
into opt.Assignees)

Dependencies

Blocked by: none open
Blocks: reliable merge dispatch in container mode, milestone
close
Branch off: main
Full graph: #17

## User story As an **operator** running workers in container mode, I want the image to ship a `forgejo-mcp` binary that surfaces real merge failures instead of silently returning `"Pull request merged successfully"` on non-200 responses — so boss's `merge_pull_request` calls fail loudly when the server rejects, and the follow-up can kick in. ## Background The upstream `goern/forgejo-mcp` v2.17.0 (what the Dockerfile currently pulls) has a bug in `MergePullRequestFn`: it discards the `bool` success flag from the SDK and checks only `err`, which is nil for any HTTP response the SDK managed to parse (including 4xx / 5xx rejects). Patched locally in-session (track record: `update_issue` assignee and `merge_pull_request` both fixed, built at `/tmp/forgejo-mcp`, installed to `~/.local/bin/forgejo-mcp`). When workers ran on the host, PATH put `~/.local/bin` before `/usr/bin` and the patched binary shadowed the pacman install. In container mode there is no such shadowing: the image bakes `/usr/local/bin/forgejo-mcp` at build time (`curl` from the upstream release), and that is the only binary the in-container Claude CLI sees. Observed 2026-04-17 on PR #31 (the #29 follow-up): boss claimed "squash-merged successfully, branch dev/29 deleted" in the dashboard event stream, but the PR was still open and the branch still present. Forgejo was in fact returning HTTP 500 on the squash call (separate issue — see Related below) and the buggy MCP swallowed it. Recovered by `docker cp`'ing the patched binary into each running container and completing the merge manually via REST. That recovery is ephemeral — survives until the next `just containers-rebuild`. ## Acceptance criteria ### Dockerfile - [ ] The `forgejo-mcp` build step must produce a binary that returns an error when the server rejects a merge, not `"Pull request merged successfully"`. - [ ] Pick one mechanism (whichever upstream allows first): 1. **Preferred**: contribute the fix upstream, pin the image to the first release that carries it. 2. Otherwise: build from a known-good fork / local tarball. Commit the patch (diff, not a blob) to this repo under `patches/` and apply it during the Dockerfile build, then `go build`. - [ ] Same check for `update_issue`'s dropped `assignee` — the upstream code comments out the assignee write ("assignee is not supported in the current SDK" — the comment is wrong, the SDK has the field). Both fixes in one binary. ### Verification - [ ] `docker run --rm claude-hooks:<tag> forgejo-mcp --version` reports a version string that makes it clear this is the patched build (e.g. a dev suffix, or a fork's version). - [ ] A hand-exercise: post a merge that the server will reject (e.g. a PR with a required-check still pending) and confirm the MCP returns an error string, not the success string. - [ ] Existing `dockerfile` job in `qa.yml` continues to pass the static audit (no new credential-leak patterns). ### Docs - [ ] A brief note in `README.md` (or wherever the container story lives) explaining why the image carries a patched binary, with a pointer to the upstream issue / PR. ## Out of scope - The Forgejo-side HTTP 500 on squash merges on this server. PR #31 was closed via a `merge`-style commit as a workaround. Separate bug, separate story if it's worth chasing (possibly commit signing related — the PR page flagged "This instance has no key to sign this commit with"). - The `merge.md` skill's fallback strategy when squash 500s. If the server-side squash issue is persistent, `merge.md` could try `squash` first, then fall back to `merge` with a note. Separate story. ## References - Parent tracking issue: #17 - Upstream MCP: codeberg.org/goern/forgejo-mcp (v2.17.0 at time of writing, patched locally at `/tmp/forgejo-mcp`) - Local patch sites: `operation/pull/pull.go:MergePullRequestFn` (check the `bool` return from `MergePullRequest`), `operation/issue/issue.go:UpdateIssueFn` (wire the `assignee` arg into `opt.Assignees`) ## Dependencies - **Blocked by:** none open - **Blocks:** reliable merge dispatch in container mode, milestone close - **Branch off:** `main` - **Full graph:** #17