MR creation not idempotent on resume — GitLab 409 when re-running after auto-merge failure #280

New issue

david · 2026-08-18T08:18:50Z

david commented

2026-08-18 08:18:50 +00:00

Summary

Follow-on bug from #274. After #274's glab mr view --json failure aborts the mr stage during auto-merge polling (i.e. after the MR was already created), resuming the run causes runMrStage to attempt MR creation again, which GitLab rejects:

[mr] ERROR stage failed (unknown): GitLab MR create for teg/ovation-technology/development/ignition-2.0-dotnet-poc failed with status 409
pi-loop failed at mr (unknown): GitLab MR create for teg/ovation-technology/development/ignition-2.0-dotnet-poc failed with status 409

Root cause

Two compounding gaps in src/mr/services/runMrStage.ts:

mr-result.json is written only after the entire resolveMrOutcome() call succeeds — including createMr() and the full runAutoMerge() block (poll + merge). If createMr() succeeds but auto-merge subsequently throws (e.g. the #274 glab --json bug, or any other transient auto-merge failure), mr-result.json is never written even though the MR/PR genuinely exists on the platform.
Resume treats a missing mr-result.json/mr-skipped.json as "mr stage not done" (findIncompleteRun, determineResumeStageWithGit), so the whole mr stage reruns from scratch. ensureBranchCheckedOut/ensureCommitted/ensurePushed are all correctly idempotent (see their doc comments), but createMr() has no equivalent "does an MR already exist for this branch?" check — it unconditionally issues POST .../merge_requests again. GitLab rejects a second MR for the same source→target branch pair with 409 ("Another open merge request already exists for this source branch"), which classifyHttpStatus maps to kind: 'unknown', retriable: false.

This is inconsistent with the research stage's shipping workflow (AGENTS.md), which is explicitly documented as idempotent: "skip when the research PR is already merged; reference an existing open PR instead of duplicating." The MR stage has no analogous check.

Likely affects GitHub (422 "A pull request already exists...") and Forgejo similarly, not just GitLab — the missing idempotency is in runMrStage's orchestration, not GitLab-specific.

Fix (both parts requested — belt and suspenders)

1. Make `createMr` idempotent across resume

Before calling client.createMr(), check whether an open MR/PR already exists for sourceBranch -> targetBranch on the platform and reuse it instead of creating a duplicate:

GitLab: GET /projects/:id/merge_requests?source_branch=...&target_branch=...&state=opened
GitHub: GET /repos/:owner/:repo/pulls?head=:owner:branch&base=...&state=open
Forgejo: equivalent PR-list-by-branch query

If found, skip the create call and use the existing MR/PR's URL/number to continue (auto-merge etc.). This should live behind the MrClient interface (e.g. a new findExistingMr method or fold the check into createMr) so all three platform clients implement it consistently, with unit tests per platform (found / not-found / API-error passthrough).

2. Persist MR-created state before entering auto-merge

Write mr-result.json (or a new durable marker) immediately after createMr() succeeds, before starting runAutoMerge(). This way:

A failure during auto-merge polling/merge no longer erases the fact that the MR was already created.
Resume can detect "MR already created, auto-merge still pending" and skip straight to the poll/merge step instead of re-running createMr().

This likely needs a schema/shape decision (e.g. mr-result.json written in two phases — created-but-not-merged, then updated with autoMerge fields once merge completes — or a separate intermediate artifact). Needs care to keep mr-result.json's schema stable per AGENTS.md's "Artifacts are the contract" rule; if the schema must change, update DESIGN.md / IMPLEMENTATION_PLAN.md and tests together.

Doing both closes the gap from either direction: idempotent creation prevents the 409 even if resume detection doesn't improve, and accurate stage-completion tracking prevents unnecessary re-creation attempts even if the idempotency check has a platform-specific gap.

Affected files

src/mr/services/runMrStage.ts (orchestration, phased artifact write)
src/mr/types/mrClient.ts (interface — possible new method)
src/gitlab/services/gitLabMrClient.ts
src/github/services/gitHubMrClient.ts
src/forgejo/services/forgejoMrClient.ts
src/run/services/findIncompleteRun.ts, src/run/services/determineResumeStage.ts (if a new intermediate artifact/marker is introduced)
Corresponding *.test.ts files for all of the above

Acceptance criteria

Re-running createMr() for a branch that already has an open MR/PR reuses the existing one instead of failing with a duplicate/conflict error (GitLab 409, GitHub 422, Forgejo equivalent), covered by unit tests for all three platforms.
MR-created state is durably recorded before auto-merge begins, so a resume after an auto-merge-time failure does not attempt to recreate the MR/PR.
Resume correctly detects "MR created, auto-merge pending" and proceeds directly to polling/merge.
No schema-breaking change to mr-result.json without updating DESIGN.md/IMPLEMENTATION_PLAN.md and tests together.
npm run lint and npm test pass.
Manually verified end-to-end: inject a failure during auto-merge polling (e.g. reproduce the #274 scenario), resume, confirm no duplicate MR is created and the run completes successfully.

#274 (the glab mr view --json bug that originally triggered this failure path)

## Summary Follow-on bug from #274. After #274's `glab mr view --json` failure aborts the `mr` stage *during auto-merge polling* (i.e. after the MR was already created), resuming the run causes `runMrStage` to attempt MR creation again, which GitLab rejects: ``` [mr] ERROR stage failed (unknown): GitLab MR create for teg/ovation-technology/development/ignition-2.0-dotnet-poc failed with status 409 pi-loop failed at mr (unknown): GitLab MR create for teg/ovation-technology/development/ignition-2.0-dotnet-poc failed with status 409 ``` ## Root cause Two compounding gaps in `src/mr/services/runMrStage.ts`: 1. **`mr-result.json` is written only after the *entire* `resolveMrOutcome()` call succeeds** — including `createMr()` **and** the full `runAutoMerge()` block (poll + merge). If `createMr()` succeeds but auto-merge subsequently throws (e.g. the #274 `glab --json` bug, or any other transient auto-merge failure), `mr-result.json` is never written even though the MR/PR genuinely exists on the platform. 2. **Resume treats a missing `mr-result.json`/`mr-skipped.json` as "mr stage not done"** (`findIncompleteRun`, `determineResumeStageWithGit`), so the whole `mr` stage reruns from scratch. `ensureBranchCheckedOut`/`ensureCommitted`/`ensurePushed` are all correctly idempotent (see their doc comments), but **`createMr()` has no equivalent "does an MR already exist for this branch?" check** — it unconditionally issues `POST .../merge_requests` again. GitLab rejects a second MR for the same source→target branch pair with `409` ("Another open merge request already exists for this source branch"), which `classifyHttpStatus` maps to `kind: 'unknown', retriable: false`. This is inconsistent with the research stage's shipping workflow (AGENTS.md), which is explicitly documented as idempotent: "skip when the research PR is already merged; reference an existing open PR instead of duplicating." The MR stage has no analogous check. Likely affects GitHub (422 "A pull request already exists...") and Forgejo similarly, not just GitLab — the missing idempotency is in `runMrStage`'s orchestration, not GitLab-specific. ## Fix (both parts requested — belt and suspenders) ### 1. Make `createMr` idempotent across resume Before calling `client.createMr()`, check whether an open MR/PR already exists for `sourceBranch -> targetBranch` on the platform and reuse it instead of creating a duplicate: - **GitLab**: `GET /projects/:id/merge_requests?source_branch=...&target_branch=...&state=opened` - **GitHub**: `GET /repos/:owner/:repo/pulls?head=:owner:branch&base=...&state=open` - **Forgejo**: equivalent PR-list-by-branch query If found, skip the create call and use the existing MR/PR's URL/number to continue (auto-merge etc.). This should live behind the `MrClient` interface (e.g. a new `findExistingMr` method or fold the check into `createMr`) so all three platform clients implement it consistently, with unit tests per platform (found / not-found / API-error passthrough). ### 2. Persist MR-created state before entering auto-merge Write `mr-result.json` (or a new durable marker) immediately after `createMr()` succeeds, **before** starting `runAutoMerge()`. This way: - A failure during auto-merge polling/merge no longer erases the fact that the MR was already created. - Resume can detect "MR already created, auto-merge still pending" and skip straight to the poll/merge step instead of re-running `createMr()`. This likely needs a schema/shape decision (e.g. `mr-result.json` written in two phases — created-but-not-merged, then updated with `autoMerge` fields once merge completes — or a separate intermediate artifact). Needs care to keep `mr-result.json`'s schema stable per `AGENTS.md`'s "Artifacts are the contract" rule; if the schema must change, update `DESIGN.md` / `IMPLEMENTATION_PLAN.md` and tests together. Doing both closes the gap from either direction: idempotent creation prevents the 409 even if resume detection doesn't improve, and accurate stage-completion tracking prevents unnecessary re-creation attempts even if the idempotency check has a platform-specific gap. ## Affected files - `src/mr/services/runMrStage.ts` (orchestration, phased artifact write) - `src/mr/types/mrClient.ts` (interface — possible new method) - `src/gitlab/services/gitLabMrClient.ts` - `src/github/services/gitHubMrClient.ts` - `src/forgejo/services/forgejoMrClient.ts` - `src/run/services/findIncompleteRun.ts`, `src/run/services/determineResumeStage.ts` (if a new intermediate artifact/marker is introduced) - Corresponding `*.test.ts` files for all of the above ## Acceptance criteria - [ ] Re-running `createMr()` for a branch that already has an open MR/PR reuses the existing one instead of failing with a duplicate/conflict error (GitLab 409, GitHub 422, Forgejo equivalent), covered by unit tests for all three platforms. - [ ] MR-created state is durably recorded before auto-merge begins, so a resume after an auto-merge-time failure does not attempt to recreate the MR/PR. - [ ] Resume correctly detects "MR created, auto-merge pending" and proceeds directly to polling/merge. - [ ] No schema-breaking change to `mr-result.json` without updating `DESIGN.md`/`IMPLEMENTATION_PLAN.md` and tests together. - [ ] `npm run lint` and `npm test` pass. - [ ] Manually verified end-to-end: inject a failure during auto-merge polling (e.g. reproduce the #274 scenario), resume, confirm no duplicate MR is created and the run completes successfully. ## Related - #274 (the `glab mr view --json` bug that originally triggered this failure path)

david added the

bug

label

2026-08-18 08:18:50 +00:00

david referenced this issue from a pull request that will close it,

2026-08-18 20:53:40 +00:00

280: MR creation not idempotent on resume — GitLab 409 when re-running after auto-merge failure #281

david closed this issue

2026-08-18 23:42:35 +00:00

david referenced this issue from a commit

2026-08-18 23:42:37 +00:00

280: MR creation not idempotent on resume — GitLab 409 when re-running after auto-merge failure (#281)