coilyco-flight-deck/ward

Fork 0

Prevent pull-request-and-merge workers from re-merging an already merged PR branch #1358

New issue

Closed

opened 2026-07-15 08:46:44 +00:00 by coilyco-ops · 3 comments

coilyco-ops commented

2026-07-15 08:46:44 +00:00

Member

Problem

A pull-request-and-merge worker mutated main after its PR had already merged, creating a redundant merge commit with no tree change and triggering an extra promote/release cycle.

Evidence from the #1242 / #1357 run:

PR #1357 merged at 2026-07-15T08:37:19Z with merge commit 369495a36e83272141e0b25879c44d0cf8196d3a.
The #1242 engineer continued running after the PR merge and produced a second main commit: 34f1232eed0f5eb3c8795f5ba151411c55ce317a.
The second commit message was Merge issue-1242: Persist broker dispatch artifacts (closes #1242).
34f1232 has parents 369495a and 95b7dbf, where 95b7dbf was the already-merged PR branch head.
git diff --stat 369495a 34f1232 was empty.
Both 369495a^{tree} and 34f1232^{tree} were e6e2a9aee1df39f9fea46515c973e5f0aea5d64b, proving this was a no-tree-change main mutation.
The redundant main mutation triggered another promote/release pair: run 8377 succeeded and run 8378 succeeded.

Expected behavior

Once a worker's PR is already merged, the finalization path must not mutate main again.

The worker may confirm the merged PR state and post the PR URL/final status, but it must not call the merge endpoint or directly push/merge the branch when the PR has already landed or when the branch tree is already contained in main.

Acceptance criteria

The pull-request-and-merge finalization path detects an already-merged PR and exits as already merged, no action rather than attempting another merge.
The same-tree/already-landed branch case cannot create a new main commit.
Tests cover an already-merged PR and a same-tree branch after PR merge.
User-facing output distinguishes already merged, no action from merge failure.
The fix preserves normal merge behavior for eligible open PRs.

## Problem A pull-request-and-merge worker mutated `main` after its PR had already merged, creating a redundant merge commit with no tree change and triggering an extra promote/release cycle. Evidence from the #1242 / #1357 run: - PR #1357 merged at `2026-07-15T08:37:19Z` with merge commit `369495a36e83272141e0b25879c44d0cf8196d3a`. - The #1242 engineer continued running after the PR merge and produced a second main commit: `34f1232eed0f5eb3c8795f5ba151411c55ce317a`. - The second commit message was `Merge issue-1242: Persist broker dispatch artifacts (closes #1242)`. - `34f1232` has parents `369495a` and `95b7dbf`, where `95b7dbf` was the already-merged PR branch head. - `git diff --stat 369495a 34f1232` was empty. - Both `369495a^{tree}` and `34f1232^{tree}` were `e6e2a9aee1df39f9fea46515c973e5f0aea5d64b`, proving this was a no-tree-change main mutation. - The redundant main mutation triggered another promote/release pair: run 8377 succeeded and run 8378 succeeded. ## Expected behavior Once a worker's PR is already merged, the finalization path must not mutate `main` again. The worker may confirm the merged PR state and post the PR URL/final status, but it must not call the merge endpoint or directly push/merge the branch when the PR has already landed or when the branch tree is already contained in `main`. ## Acceptance criteria - The pull-request-and-merge finalization path detects an already-merged PR and exits as `already merged, no action` rather than attempting another merge. - The same-tree/already-landed branch case cannot create a new `main` commit. - Tests cover an already-merged PR and a same-tree branch after PR merge. - User-facing output distinguishes `already merged, no action` from merge failure. - The fix preserves normal merge behavior for eligible open PRs.

coilyco-ops added this to the warded control plane coherence milestone

2026-07-15 08:46:44 +00:00

coilyco-ops added the

coherence-core

headless

labels

2026-07-15 08:46:44 +00:00

coilyco-ops commented

2026-07-15 08:47:05 +00:00

Author

Member

WARDED_WORKFLOW: reservation-held

reservation details

Holder: launch intent for container engineer-codex-ward-1358 on host kais-macbook-pro-2.local.

Accepted by ward agent --harness codex (reserved 2026-07-15T08:47:04Z). Concurrent ward agent runs are blocked until this intent becomes visible or the intent is released. The stale-intent fallback is still TTL-bounded (3h TTL). --override-reservation overrides.

Do not comment on or edit this issue to steer the run while it is reserved. The engineer seeded the body once at launch and never re-reads it, so a comment or edit reaches only human readers, never the running engineer. A correction goes to a new issue, dispatched fresh. That is the only channel that reaches a run in flight. Where the forge supports it, ward locks this conversation to make that a road-block rather than a convention (ward#494).

run seed context — what this run is carrying (ward#609)

Resolved: coilyco-flight-deck/ward#1358 · branch issue-1358 · harness codex · workflow pull-request-and-merge
Run: engineer-codex-ward-1358 · ward v0.710.0 · dispatched 2026-07-15T08:47:00Z
Comment thread: 0 included in the pre-flight read, 0 stripped (ward's own automated comments).

Static container doctrine and seed boilerplate are identical every run and omitted here (they ride ward v0.710.0).

— Codex, via ward agent

WARDED_WORKFLOW: reservation-held <details><summary>reservation details</summary> Holder: launch intent for container `engineer-codex-ward-1358` on host `kais-macbook-pro-2.local`. Accepted by `ward agent --harness codex` (reserved 2026-07-15T08:47:04Z). Concurrent `ward agent` runs are blocked until this intent becomes visible or the intent is released. The stale-intent fallback is still TTL-bounded (3h TTL). `--override-reservation` overrides. **Do not comment on or edit this issue to steer the run while it is reserved.** The engineer seeded the body once at launch and never re-reads it, so a comment or edit reaches only human readers, never the running engineer. A correction goes to a **new issue, dispatched fresh**. That is the only channel that reaches a run in flight. Where the forge supports it, ward locks this conversation to make that a road-block rather than a convention (ward#494). <details><summary>run seed context — what this run is carrying (ward#609)</summary> - **Resolved:** `coilyco-flight-deck/ward#1358` · branch `issue-1358` · harness `codex` · workflow `pull-request-and-merge` - **Run:** `engineer-codex-ward-1358` · ward `v0.710.0` · dispatched `2026-07-15T08:47:00Z` - **Comment thread:** 0 included in the pre-flight read, 0 stripped (ward's own automated comments). Static container doctrine and seed boilerplate are identical every run and omitted here (they ride ward v0.710.0). </details> </details>  — Codex, via `ward agent`

coilyco-ops referenced this issue from a commit

2026-07-15 08:57:40 +00:00

Prevent redundant PR merge after landing

coilyco-ops referenced this issue from a pull request that will close it,

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

Prevent redundant PR merge after landing #1361

coilyco-ops commented

2026-07-15 09:00:50 +00:00

Author

Member

WARDED_WORKFLOW: #1361

details

workflow: pull-request-and-merge; review summary: review gate intentionally skipped because the temporary ward default pending brokered QA

felt: the fix was compact once the merge-finalization path was isolated.
confidence: high
surprises: the same-tree case needed a commit-tree comparison, not just merged-state.
follow-ups: none

WARDED_WORKFLOW: https://forgejo.coilysiren.me/coilyco-flight-deck/ward/pulls/1361 <details><summary>details</summary> workflow: pull-request-and-merge; review summary: review gate intentionally skipped because the temporary ward default pending brokered QA felt: the fix was compact once the merge-finalization path was isolated. confidence: high surprises: the same-tree case needed a commit-tree comparison, not just merged-state. follow-ups: none </details>

coilyco-ops closed this issue

2026-07-15 09:01:50 +00:00

coilyco-ops commented

2026-07-15 09:02:34 +00:00

Author

Member

QA verified after PR #1361 merged.

Evidence:

PR #1361 merged at 2026-07-15T09:01:50Z with merge commit 1a61738b63a7bd2a38f2800d46db1c5c6b7d7456.
Forgejo PR test run 8391 passed.
origin/main diff from #1359 to #1361 changes only the intended PR merge/director merge/Forgejo commit helpers:
- cmd/ward/agent_director_merge.go
- cmd/ward/agent_pr_workflow.go
- cmd/ward/agent_pr_workflow_test.go
- cmd/ward/forgejo_ops.go
Focused post-merge check on origin/main passed: go test ./cmd/ward -run 'PRWorkflowMergeExec|MergeDirectorPullRequest|GetCommitTreeSHA|PartialLaunch|DispatchHealth|AgentList' -count=1.

Acceptance review:

Already-merged PRs now return already merged, no action before touching the merge endpoint.
Same-tree/already-landed branch state is checked with base/head commit tree comparison and returns the same no-op output.
Regression coverage asserts both paths make zero merge calls.
Normal eligible open PR merge behavior remains covered by the existing merge workflow tests.

No replacement issue needed from this QA pass.

QA verified after PR #1361 merged. Evidence: - PR #1361 merged at `2026-07-15T09:01:50Z` with merge commit `1a61738b63a7bd2a38f2800d46db1c5c6b7d7456`. - Forgejo PR test run 8391 passed. - `origin/main` diff from #1359 to #1361 changes only the intended PR merge/director merge/Forgejo commit helpers: - `cmd/ward/agent_director_merge.go` - `cmd/ward/agent_pr_workflow.go` - `cmd/ward/agent_pr_workflow_test.go` - `cmd/ward/forgejo_ops.go` - Focused post-merge check on `origin/main` passed: `go test ./cmd/ward -run 'PRWorkflowMergeExec|MergeDirectorPullRequest|GetCommitTreeSHA|PartialLaunch|DispatchHealth|AgentList' -count=1`. Acceptance review: - Already-merged PRs now return `already merged, no action` before touching the merge endpoint. - Same-tree/already-landed branch state is checked with base/head commit tree comparison and returns the same no-op output. - Regression coverage asserts both paths make zero merge calls. - Normal eligible open PR merge behavior remains covered by the existing merge workflow tests. No replacement issue needed from this QA pass.