phase-pr reports CI green on GitLab because it queries pipelines by branch ref, never seeing merge_request_event pipelines #517

New issue

mph · 2026-09-03T09:50:00Z

mph commented

2026-09-03 09:50:00 +00:00

Found on MEDECA-453 (medeca-modul-poptavky, GitLab at gitlab.e-bs.cz).

Symptom

./iw phase-pr exited 0 for MR !444 and set review-state to awaiting_review, whose documented meaning is 'CI is green, or there is no CI'. At that moment the MR's pipeline was still running:

glab api '.../pipelines/5306'  ->  status: running,  sha == branch HEAD

Cause

The gate enumerates pipelines by branch ref:

.../pipelines?ref=MEDECA-453-phase-02   ->   []

This project's pipelines run with source: merge_request_event on ref: refs/merge-requests/444/head, where the gate never looks. An empty result means 'looked in the wrong place' as readily as 'no CI configured', so exit 0 from this gate is not evidence that anything passed.

Impact

phase-pr can hand a still-running or failing MR to phase-merge, whose contract is to merge what the gate proved. In this session the phase MR was merged after the pipeline was polled by id by hand; without that manual step the merge would have been unguarded.

Suggested fix

Resolve a GitLab MR's pipeline through the merge request (/merge_requests/:iid/pipelines) or by commit SHA, not by branch ref, and distinguish 'no pipeline found' from 'pipeline green' — the two should not share an exit code. The same reasoning applies to phase-merge and ci-gate.

Workaround used: poll the pipeline by id and read its terminal status directly.

Type: bug
Filed from: /home/mph/Devel/projects/medeca-modul-poptavky-MEDECA-453 on branch MEDECA-453

Found on MEDECA-453 (medeca-modul-poptavky, GitLab at gitlab.e-bs.cz). ## Symptom `./iw phase-pr` exited 0 for MR !444 and set review-state to `awaiting_review`, whose documented meaning is 'CI is green, or there is no CI'. At that moment the MR's pipeline was still running: glab api '.../pipelines/5306' -> status: running, sha == branch HEAD ## Cause The gate enumerates pipelines by branch ref: .../pipelines?ref=MEDECA-453-phase-02 -> [] This project's pipelines run with `source: merge_request_event` on `ref: refs/merge-requests/444/head`, where the gate never looks. An empty result means 'looked in the wrong place' as readily as 'no CI configured', so exit 0 from this gate is not evidence that anything passed. ## Impact `phase-pr` can hand a still-running or failing MR to `phase-merge`, whose contract is to merge what the gate proved. In this session the phase MR was merged after the pipeline was polled by id by hand; without that manual step the merge would have been unguarded. ## Suggested fix Resolve a GitLab MR's pipeline through the merge request (`/merge_requests/:iid/pipelines`) or by commit SHA, not by branch ref, and distinguish 'no pipeline found' from 'pipeline green' — the two should not share an exit code. The same reasoning applies to `phase-merge` and `ci-gate`. Workaround used: poll the pipeline by id and read its terminal status directly. --- **Type:** bug **Filed from:** `/home/mph/Devel/projects/medeca-modul-poptavky-MEDECA-453` on branch `MEDECA-453`

mph added the

needs-human

label

2026-09-03 20:09:52 +00:00

mph commented

2026-09-03 20:09:52 +00:00

Intake: needs-human
Workflow: diagnostic
Reason: G1 — the analysis holds 2 CLARIFY markers (which pipelines existed for MR !444; GitLab head_pipeline contract when a commit has several pipelines)
Analysis: project-management/issues/IW-517/analysis.md on branch IW-517

Intake: needs-human Workflow: diagnostic Reason: G1 — the analysis holds 2 CLARIFY markers (which pipelines existed for MR !444; GitLab head_pipeline contract when a commit has several pipelines) Analysis: project-management/issues/IW-517/analysis.md on branch IW-517

mph commented

2026-09-04 19:43:41 +00:00

Analysis review: unsound (2 blocking findings)

Defect 2 does not exist. PhasePr.scala:477 and CiGateCommand.scala:188 pass clearCiGatedSha = gatedSha.isEmpty, and PhasePr.scala:353 clears it on the failure path — on NoCi the field is already cleared.
Defect 2's named regression test passes today, so it would build behaviour that exists.

Defect 1 stands and is reproduced. Defect 2 is struck. Recorded under ## Review Notes on branch IW-517.
Verdict unchanged: needs-human, now on G1 and G7.

Analysis review: unsound (2 blocking findings) 1. Defect 2 does not exist. `PhasePr.scala:477` and `CiGateCommand.scala:188` pass `clearCiGatedSha = gatedSha.isEmpty`, and `PhasePr.scala:353` clears it on the failure path — on NoCi the field is already cleared. 2. Defect 2's named regression test passes today, so it would build behaviour that exists. Defect 1 stands and is reproduced. Defect 2 is struck. Recorded under ## Review Notes on branch IW-517. Verdict unchanged: needs-human, now on G1 and G7.

mph removed the

needs-human

label

2026-09-05 19:46:49 +00:00

claude commented

2026-09-13 21:16:59 +00:00

Intake: agent-ready
Workflow: diagnostic
Reason: all gates passed — 0 CLARIFY markers, review sound, 1 phase
Analysis: project-management/issues/IW-517/analysis.md on branch IW-517

Intake: agent-ready Workflow: diagnostic Reason: all gates passed — 0 CLARIFY markers, review sound, 1 phase Analysis: project-management/issues/IW-517/analysis.md on branch IW-517

mph added the

agent-ready

label

2026-09-13 21:17:08 +00:00

claude commented

2026-09-14 21:03:58 +00:00

Fixed on main by merge commit 6c2baaad (pull request #559, branch IW-517). Closing.

The agent-ready label was left behind by the implement loop, which does not close its issue when the final pull request merges.

Fixed on `main` by merge commit `6c2baaad` (pull request #559, branch `IW-517`). Closing. The `agent-ready` label was left behind by the implement loop, which does not close its issue when the final pull request merges.

claude closed this issue

2026-09-14 21:03:58 +00:00

Sign in to join this conversation.