Cloonar/coding-lab

Fork 0

labctl: add a "pr merge" tool — no way to land a PR from the agent surface #37

New issue

Closed

opened 2026-07-08 00:58:03 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-08 00:58:03 +02:00

Owner

Problem

labctl can open and inspect pull requests but cannot merge one. The pr surface is create | view | list — write-once, then read-only. There is no pr merge, and the agent API (/agent/v1/*, mounted at internal/httpapi/server.go:375) has no PR-merge route.

Hit live this session: asked to "land" PR #36 (a clean fast-forward, CI + docs only), there was no labctl path to do it — the maintainer had to merge by hand in the forge UI.

Why it matters

Landing a green PR is a routine end-of-task step, but today an agent has only two off-surface workarounds, both wrong:

Forge merge API (POST /api/v1/repos/{owner}/{repo}/pulls/{n}/merge) — needs a forge token, which the agent session does not have (LAB_TOKEN is a lab run token; no forge/API token in env, no curl).
Direct SSH push to main — bypasses the forge merge flow and branch protection; unacceptable for a protected trunk.

Net: "land the PR" needs a human every time, even when the PR is trivially mergeable.

What exists today (the gap is uneven)

The server already has merge machinery — but only for the builtin / change-request binding:

internal/tracker/builtin implements MergeCR(...), exposed at POST /api/v1/repos/{repo}/crs/{n}/merge (operator / CSRF surface), with both fast-forward and merge-commit paths — see internal/httpapi/crs_test.go (TestCRMergeFastForwardEndToEnd, TestCRMergeCommitPathUsesRepoIdentity).

But the pieces needed for an agent-side merge are missing:

The abstract Tracker interface (internal/tracker/tracker.go:160) has CreatePull / CreateIssue / CreateComment but no MergePull — merge is not part of the binding contract.
The forgejo binding (internal/tracker/forgejo/forgejo.go) only reads merged state (derivePullState, the Merged field) — it has no merge action at all. For a forge-bound repo (this one) there is no code path to merge a PR.
The agent API exposes no merge route, and the labctl client (internal/labctl/client.go) has only PRCreate / PRView / PRList.

So this is not "build merge from scratch" for the native binding — but the forge path, the one that actually mattered here, genuinely does not exist.

Possible shape (non-binding — for triage)

labctl pr merge <n> → new POST /agent/v1/prs/{n}/merge → a new MergePull method on the Tracker interface.
Bindings: builtin wires up the existing MergeCR; forgejo calls the forge merge API using the server's forge credentials (so no token ever reaches the agent); github similarly, for parity.
Let the forge enforce required checks / branch protection and surface any rejection verbatim (e.g. "required status check not satisfied") rather than trying to reason about mergeability agent-side.

Open questions for triage

Merge method: merge / rebase / squash — fixed, or a flag?
Delete the head branch on merge?
Behavior when required checks are not green — block, or pass the forge's error through?
Scope: pair this with a pr close tool (also missing — issues have close, PRs do not) or keep it merge-only?

## Problem `labctl` can open and inspect pull requests but cannot **merge** one. The `pr` surface is `create | view | list` — write-once, then read-only. There is no `pr merge`, and the agent API (`/agent/v1/*`, mounted at `internal/httpapi/server.go:375`) has no PR-merge route. Hit live this session: asked to "land" PR #36 (a clean fast-forward, CI + docs only), there was no `labctl` path to do it — the maintainer had to merge by hand in the forge UI. ## Why it matters Landing a green PR is a routine end-of-task step, but today an agent has only two off-surface workarounds, both wrong: - **Forge merge API** (`POST /api/v1/repos/{owner}/{repo}/pulls/{n}/merge`) — needs a forge token, which the agent session does not have (`LAB_TOKEN` is a lab run token; no forge/API token in env, no `curl`). - **Direct SSH push to `main`** — bypasses the forge merge flow and branch protection; unacceptable for a protected trunk. Net: "land the PR" needs a human every time, even when the PR is trivially mergeable. ## What exists today (the gap is uneven) The server already has merge machinery — but only for the **builtin / change-request binding**: - `internal/tracker/builtin` implements `MergeCR(...)`, exposed at `POST /api/v1/repos/{repo}/crs/{n}/merge` (operator / CSRF surface), with both fast-forward and merge-commit paths — see `internal/httpapi/crs_test.go` (`TestCRMergeFastForwardEndToEnd`, `TestCRMergeCommitPathUsesRepoIdentity`). But the pieces needed for an agent-side merge are missing: - The abstract `Tracker` interface (`internal/tracker/tracker.go:160`) has `CreatePull` / `CreateIssue` / `CreateComment` but **no `MergePull`** — merge is not part of the binding contract. - The **forgejo binding** (`internal/tracker/forgejo/forgejo.go`) only *reads* merged state (`derivePullState`, the `Merged` field) — it has **no merge action at all**. For a forge-bound repo (this one) there is no code path to merge a PR. - The **agent API** exposes no merge route, and the labctl client (`internal/labctl/client.go`) has only `PRCreate` / `PRView` / `PRList`. So this is not "build merge from scratch" for the native binding — but the forge path, the one that actually mattered here, genuinely does not exist. ## Possible shape (non-binding — for triage) - `labctl pr merge <n>` → new `POST /agent/v1/prs/{n}/merge` → a new `MergePull` method on the `Tracker` interface. - Bindings: builtin wires up the existing `MergeCR`; forgejo calls the forge merge API using the **server's** forge credentials (so no token ever reaches the agent); github similarly, for parity. - Let the forge enforce required checks / branch protection and surface any rejection verbatim (e.g. "required status check not satisfied") rather than trying to reason about mergeability agent-side. ## Open questions for triage - Merge method: merge / rebase / squash — fixed, or a flag? - Delete the head branch on merge? - Behavior when required checks are not green — block, or pass the forge's error through? - Scope: pair this with a `pr close` tool (also missing — issues have `close`, PRs do not) or keep it merge-only?

dominik.polakovics added the

needs-triage

label

2026-07-08 00:58:03 +02:00

dominik.polakovics added the

enhancement

label

2026-07-08 10:12:39 +02:00

dominik.polakovics commented

2026-07-08 10:12:39 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Add labctl pr merge <n> so an agent can land a mergeable PR/CR through the tracker seam, on both bindings, without a forge token ever reaching the session.

Current behavior:
The agent PR surface is write-once-then-read-only: labctl pr is create | view | list, backed by CreatePull / Pull / Pulls on the Tracker seam and the run-token /agent/v1 routes. There is no way to merge a PR from a session. Merge machinery does exist — but only for the builtin binding and only behind the operator (cookie + CSRF) surface: the change-request merge does a fast-forward when origin/<base> is an ancestor of the head, otherwise a --no-ff merge commit built in a throwaway detached worktree, under a per-CR mutex, cancellation-immune once the git window opens, best-effort closing every Closes #N, publishing cr.changed / issue.changed (see gitx.CRMerge + store.MergeCR and the operator CR-merge handler; ADR-0011). The abstract Tracker interface has no merge method at all, and neither forge binding (Forgejo, GitHub) can merge — they only read merged state via derivePullState. Net: landing a green PR always needs a human in the forge UI.

Desired behavior:
labctl pr merge <n> merges PR/CR n and reports the result, working identically on a builtin-bound and a forge-bound repo, entirely over the run-token surface (no forge/API token in the session).

Merge method is fixed — no flag: fast-forward when possible, otherwise a merge commit. On a forge, pass a fixed merge method and let the forge apply its own configured strategy; do not attempt ff-vs-commit detection agent-side.
The head branch is NOT deleted on merge. Branch lifecycle stays owned by guarded teardown / the sweep, which GCs a merged head once the PR/CR leaves the open state. Merge must not touch the branch.
Mergeability is the forge's call, not the agent's. Do not pre-check required status checks or branch protection. Attempt the merge; if the forge (or a git push) refuses — required check not satisfied, protected branch, conflict — surface that rejection's own words verbatim as the error, with a non-zero exit. This mirrors ADR-0011's "push refusals surface git's stderr verbatim."
Builtin binding: reuse the existing CR-merge orchestration — do not reimplement it. Its ADR-0011 invariants (per-CR serialization, cancellation-immunity once the git window opens, best-effort Closes #N closure, change/issue events, convergent re-merge of an already-merged head) must hold whether the merge is invoked from the operator route or the new agent route. Expect to lift that orchestration into a shared service both surfaces call.
Forge bindings (Forgejo now; GitHub for parity): perform the merge as a server-side authenticated call using the server's forge credential. The forge token must never be exposed to the session — the run token's repo scope stays the only agent-side boundary (ADR-0014).
Convergent / idempotent: merging an already-merged PR is a no-op success naming the existing merged state, not an error (matches the builtin re-merge contract and the create-path's duplicate handling).

Key interfaces:

Tracker seam — grows a merge method (e.g. MergePull(ctx, number) (PullRef, error)) alongside CreatePull / Pull / Pulls. Callers speak PR numbers only, like the rest of the surface.
Builtin Tracker impl — its MergePull routes into the shared CR-merge service (the one the operator merge handler uses today), preserving the mutex / cancellation / close-issues / events behavior.
Forgejo and GitHub Tracker impls — gain a real merge action (today they only derive merged state). Server-credentialed forge merge call; map the forge's refusal to the seam's error verbatim.
Agent API — a new run-token-authed, repo-scoped route under /agent/v1 for merge (mirroring the existing PR routes), returning the canonical error envelope and a pinned exit code on failure.
labctl client + CLI — a pr merge <n> subcommand and a client method beside PRCreate / PRView / PRList.

Acceptance criteria:

labctl pr merge <n> merges an open, mergeable PR/CR on a builtin-bound repo via the existing CR-merge path (ff-or-merge-commit), closing every Closes #N and emitting the same events the operator merge does.
labctl pr merge <n> merges an open, mergeable PR/CR on a forge-bound repo via a server-side forge merge call, with no forge token present in the session environment.
The head branch still exists immediately after a successful merge (teardown / sweep, not merge, removes it).
A merge the forge/git refuses (required check red, protected branch, conflict) fails with the forge's/git's own message surfaced verbatim and a non-zero exit; nothing is half-recorded (no DB row claiming a merge origin didn't take).
Re-running pr merge <n> on an already-merged PR/CR succeeds as a convergent no-op rather than erroring.
No forge/API token is read by, passed to, or required in the session for any of the above.
Merge is reachable only over the run-token agent surface with repo scope enforced — no new unscoped/operator dependency on the agent path.

Out of scope:

labctl pr close, pr comment, and the whole reject → re-queue-to-AFK loop (releasing the claim, feedback onto the issue, stopping condition) — tracked as a separate enhancement; do not build reject/close semantics here.
Deleting or resetting the head branch on merge.
Selectable merge strategy (squash / rebase / a --method flag) — fixed ff-or-merge-commit only.
Agent-side mergeability reasoning (pre-flight check of required checks / branch protection) — the forge decides.
Any operator-UI / CSRF-surface change; this adds only the agent surface.

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Add `labctl pr merge <n>` so an agent can land a mergeable PR/CR through the tracker seam, on both bindings, without a forge token ever reaching the session. **Current behavior:** The agent PR surface is write-once-then-read-only: `labctl pr` is `create | view | list`, backed by `CreatePull` / `Pull` / `Pulls` on the `Tracker` seam and the run-token `/agent/v1` routes. There is no way to *merge* a PR from a session. Merge machinery does exist — but only for the builtin binding and only behind the operator (cookie + CSRF) surface: the change-request merge does a fast-forward when `origin/<base>` is an ancestor of the head, otherwise a `--no-ff` merge commit built in a throwaway detached worktree, under a per-CR mutex, cancellation-immune once the git window opens, best-effort closing every `Closes #N`, publishing `cr.changed` / `issue.changed` (see `gitx.CRMerge` + `store.MergeCR` and the operator CR-merge handler; ADR-0011). The abstract `Tracker` interface has **no merge method at all**, and neither forge binding (Forgejo, GitHub) can merge — they only *read* merged state via `derivePullState`. Net: landing a green PR always needs a human in the forge UI. **Desired behavior:** `labctl pr merge <n>` merges PR/CR `n` and reports the result, working identically on a builtin-bound and a forge-bound repo, entirely over the run-token surface (no forge/API token in the session). - **Merge method is fixed — no flag:** fast-forward when possible, otherwise a merge commit. On a forge, pass a fixed merge method and let the forge apply its own configured strategy; do **not** attempt ff-vs-commit detection agent-side. - **The head branch is NOT deleted on merge.** Branch lifecycle stays owned by guarded teardown / the sweep, which GCs a merged head once the PR/CR leaves the open state. Merge must not touch the branch. - **Mergeability is the forge's call, not the agent's.** Do not pre-check required status checks or branch protection. Attempt the merge; if the forge (or a git push) refuses — required check not satisfied, protected branch, conflict — surface that rejection's *own words* verbatim as the error, with a non-zero exit. This mirrors ADR-0011's "push refusals surface git's stderr verbatim." - **Builtin binding:** reuse the *existing* CR-merge orchestration — do not reimplement it. Its ADR-0011 invariants (per-CR serialization, cancellation-immunity once the git window opens, best-effort `Closes #N` closure, change/issue events, convergent re-merge of an already-merged head) must hold whether the merge is invoked from the operator route or the new agent route. Expect to lift that orchestration into a shared service both surfaces call. - **Forge bindings (Forgejo now; GitHub for parity):** perform the merge as a server-side authenticated call using the **server's** forge credential. The forge token must never be exposed to the session — the run token's repo scope stays the only agent-side boundary (ADR-0014). - **Convergent / idempotent:** merging an already-merged PR is a no-op success naming the existing merged state, not an error (matches the builtin re-merge contract and the create-path's duplicate handling). **Key interfaces:** - `Tracker` seam — grows a merge method (e.g. `MergePull(ctx, number) (PullRef, error)`) alongside `CreatePull` / `Pull` / `Pulls`. Callers speak PR numbers only, like the rest of the surface. - Builtin `Tracker` impl — its `MergePull` routes into the shared CR-merge service (the one the operator merge handler uses today), preserving the mutex / cancellation / close-issues / events behavior. - Forgejo and GitHub `Tracker` impls — gain a real merge action (today they only derive merged state). Server-credentialed forge merge call; map the forge's refusal to the seam's error verbatim. - Agent API — a new run-token-authed, repo-scoped route under `/agent/v1` for merge (mirroring the existing PR routes), returning the canonical error envelope and a pinned exit code on failure. - `labctl` client + CLI — a `pr merge <n>` subcommand and a client method beside `PRCreate` / `PRView` / `PRList`. **Acceptance criteria:** - [ ] `labctl pr merge <n>` merges an open, mergeable PR/CR on a **builtin**-bound repo via the existing CR-merge path (ff-or-merge-commit), closing every `Closes #N` and emitting the same events the operator merge does. - [ ] `labctl pr merge <n>` merges an open, mergeable PR/CR on a **forge**-bound repo via a server-side forge merge call, with no forge token present in the session environment. - [ ] The head branch still exists immediately after a successful merge (teardown / sweep, not merge, removes it). - [ ] A merge the forge/git refuses (required check red, protected branch, conflict) fails with the forge's/git's own message surfaced verbatim and a non-zero exit; nothing is half-recorded (no DB row claiming a merge origin didn't take). - [ ] Re-running `pr merge <n>` on an already-merged PR/CR succeeds as a convergent no-op rather than erroring. - [ ] No forge/API token is read by, passed to, or required in the session for any of the above. - [ ] Merge is reachable only over the run-token agent surface with repo scope enforced — no new unscoped/operator dependency on the agent path. **Out of scope:** - `labctl pr close`, `pr comment`, and the whole **reject → re-queue-to-AFK** loop (releasing the claim, feedback onto the issue, stopping condition) — tracked as a separate enhancement; do **not** build reject/close semantics here. - Deleting or resetting the head branch on merge. - Selectable merge strategy (squash / rebase / a `--method` flag) — fixed ff-or-merge-commit only. - Agent-side mergeability reasoning (pre-flight check of required checks / branch protection) — the forge decides. - Any operator-UI / CSRF-surface change; this adds only the agent surface.

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-07-08 10:12:39 +02:00

dominik.polakovics referenced this issue

2026-07-08 10:12:42 +02:00

AFK reject→requeue: bounce a rejected PR back to the ready queue with feedback for the next run #44

dominik.polakovics referenced this issue from a commit

2026-07-08 11:07:01 +02:00

feat(labctl): land a PR/CR from the agent surface via a shared merge service