Skip to content

fix(agent-actions): merge has the same unpinned-legacy-row head-fallback gap as approve #2422

Description

@JSONbored

Parent: #1936

Problem

performAction's "merge" case (src/services/agent-action-executor.ts) computes const mergeSha = action.expectedHeadSha ?? ctx.headSha; — structurally identical to approve's (now-fixed, see #2377) approveSha fallback. When a staged merge row has no expectedHeadSha (staged by older code, or a planning pass that ran while the stored PR head SHA was transiently null — pullRequests.headSha is a nullable column), the fallback substitutes ctx.headSha, the CURRENT live head read at accept time.

The reasoning that previously excused merge from this class of bug ("GitHub's sha param 409s on mismatch") does not actually hold for the unpinned-row case: the fallback substitutes the CURRENT live head as mergeSha, so by construction it matches whatever GitHub reports as the current head at merge time — no mismatch, no 409. The 409 backstop only protects against a narrow race between reading ctx.headSha and the merge API call, not against "this row was staged with no pin at all, then the contributor force-pushed during the queue wait." An unpinned staged merge can therefore silently merge unreviewed code, the same failure mode #2377 fixed for approve.

Found via an adversarial multi-agent verification pass while implementing #2377 (the approve fix) — the initial "not exploitable / merge is safe due to a GitHub backstop" framing was checked and found incorrect for this specific scenario.

Requirements

  • The approval-queue accept flow (src/services/agent-approval-queue.ts's decidePendingAgentAction) must not replay a staged merge action that has no expectedHeadSha pin through the live-head fallback.

Deliverables

  • Extend the same accept-flow gate fix(agent-actions): pin a staged approve to its reviewed head SHA #2377 added for approve (deny with status: "rejected", a distinguishable executionOutcome, and an audit event) to merge when pending.params.expectedHeadSha is absent.
  • Regression test: a staged merge with no expectedHeadSha is denied on accept, mergePullRequest is never called, and the row ends up rejected (not accepted).
  • Confirm existing tests modeling a genuinely-live (non-replayed) unpinned merge via executeAgentMaintenanceActions directly are unaffected (this fix belongs in the accept-flow gate, not the shared executor — see fix(agent-actions): pin a staged approve to its reviewed head SHA #2377's own scoping rationale).

Acceptance criteria

  • An unpinned staged merge accepted after a force-push is refused, not silently merged against the new head.

Expected outcome

merge gets the same fail-closed treatment approve received in #2377, closing the parallel gap the adversarial review identified.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions