8.2 KiB
PR#135: The Codex Runner Releases a Held Merge Once Every Line Comment Is Resolved
Version: minor
WARNING: This document describes only the change applied in this PR. It may already be outdated once the next PR is merged. Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
Problem
werkrevisor approves a GREEN head, but holds its merge while a line comment of werkrevisor is open.
Its result comment then says: "merge waits for N open hint(s): reply to each line comment and resolve it,
then restart the review or push a commit".
When the author resolves the last thread, nothing happens:
a human must restart the build ai-review in Werkator, or someone must push a commit.
A review request of the human from an earlier YELLOW or ORANGE round also stays open,
and Gitea refuses each merge with "There are official review requests" until werkrevisor withdraws it.
Example: PR#134, head e534706, GREEN, approved, Codex passed, all threads resolved, still waiting.
Non-Goals
- No release of a hint that stands only in the result comment; an answer in a PR comment cannot be checked, so a restart by a human stays the way there.
- No change for repositories that the Codex runner does not list, e.g. hsadmin-ng on dev.hostsharing.net.
- No webhook receiver.
Specification
Feature: release a held merge
Scenario#135.01: the last resolved line comment releases the merge
… so that a GREEN PR merges without a manual restart.
- Given a PR whose head werkrevisor approved as GREEN and held for its line comments
- When the author resolves the last line comment
- Then the merge is scheduled without a restart of the review, and the review request of the human is withdrawn
Verified by
- test_release_held_merge.ReleaseHeldMergeTest.test_the_last_resolved_line_comment_releases_the_merge
- test_release_held_merge.ReleaseHeldMergeTest.test_a_failed_withdrawal_is_tried_again_by_the_next_run
- test_release_held_merge.ReleaseHeldMergeTest.test_a_hold_after_a_release_of_the_same_head_is_released_again
- test_codexrunner.CodexRunnerTest.test_each_ready_pr_is_checked_for_a_held_merge_also_without_a_codex_mode
Scenario#135.02: an open line comment still holds the merge
… so that the runner never releases more than a restart of werkrevisor would.
- Given a PR whose head werkrevisor approved as GREEN and held for a line comment that is still open
- When the Codex runner checks the PR
- Then the merge stays unscheduled and the review request of the human stays
Verified by
- test_release_held_merge.ReleaseHeldMergeTest.test_an_open_line_comment_still_holds_the_merge
- test_codexrunner.CodexRunnerTest.test_the_codex_status_is_up_to_date_before_a_release
- test_codexrunner.CodexRunnerTest.test_no_release_when_the_codex_status_cannot_be_brought_up_to_date
Scenario#135.03: only a head that werkrevisor approved and held is released
… so that a new head always passes the review of werkrevisor first.
- Given a PR whose current head has no approval of werkrevisor, a hint only in the result comment, or a merge that werkrevisor did not hold for its line comments
- When the author resolves the last line comment
- Then the merge stays unscheduled
Verified by
- test_release_held_merge.ReleaseHeldMergeTest.test_no_release_without_the_approval_of_werkrevisor_for_the_head
- test_release_held_merge.ReleaseHeldMergeTest.test_a_new_head_waits_for_its_own_review
- test_release_held_merge.ReleaseHeldMergeTest.test_a_hint_only_in_the_result_comment_is_not_released
- test_release_held_merge.ReleaseHeldMergeTest.test_a_merge_that_was_never_held_is_not_scheduled
- test_release_held_merge.ReleaseHeldMergeTest.test_a_failed_cancel_leaves_the_release_to_a_new_review
Solution
-
werkrevisor/cli.py: a GREEN review whose merge waits only for line comments setsheld_by_line_comments; a hint that stands only in the result comment does not, since only a human can judge its answer in a PR comment. Nor does a hold whose cancel of an earlier scheduled merge failed: its failed gate check turns green only with a new review (review finding of round 2). The restart comment of a review that still waits gets the same marker. -
werkrevisor/report.py: the result comment then carries the hidden marker<!-- werkrevisor-held-by-line-comments -->. The text of the model cannot forge it: the comment escapes each<!--outside the markers. -
werkrevisor/cli.py:release_held_mergedoes what a restart of the review does for such a head, if- the result of werkrevisor for the current head is GREEN and carries the marker, or a restart comment for the head does,
- werkrevisor approved this head, and the approval is not dismissed,
- no line comment of werkrevisor is open,
- Gitea still names this head as the head of the open PR.
It calls the same
_schedule_mergeand_withdraw_review_requestas the gate, now with theGateConfiginstead of the whole config. It notes the release in a PR comment with the marker<!-- werkrevisor-release:<sha> -->; it acts again only when a later restart comment holds the head again, e.g. for a reopened thread (Codex, round 3). A refused scheduling, other than a conflict, or a failed withdrawal leaves no comment, so the next run tries again; an open review request would block the scheduled merge (review finding of round 1). -
werkrevisor/codexrunner.py: each run calls it for each ready PR of each listed repository, also with[codex] mode = "off"; a failure is logged and stops no Codex review. It releases only after it has brought the statusai-review/codexof the PR up to date, so that a reopened Codex thread holds the merge before Gitea could merge (Codex, round 4). It checks before the Codex reviews of the run, so a thread resolved during a review is released by the next run; withmax_reviews_per_run = 1and a timeout of 30 minutes, that is at most about 31 minutes later (werkrevisor, round 4). The marker proves that werkrevisor holds and schedules merges in the repository, so the runner setsautomerge. -
Tests:
tests/test_release_held_merge.py,tests/test_codexrunner.py. Red/green: with each of the four conditions above removed, its test fails. -
README.md: "Approval and automerge" and "Codex runner". -
The diagrams in
docs/need no change: the review itself has no new step or decision, and the PR workflow diagram shows that GREEN merges after the author answered the hints, not who then schedules the merge. -
docs/werkrevisor.werkbaum: node#gate.release. -
werkrevisor/__init__.pyandstatistics/build.gradle.kts: version 0.65.0.
A head that werkrevisor held before this version has no marker; it still needs one restart.
The runner cannot read the host config of werkrevisor on mih09.
After [gate] automerge is switched off, it still releases a head that werkrevisor held before (Codex, round 3);
a merge that werkrevisor scheduled before stays scheduled in Gitea likewise.
Removing the repository from config/codex-runner.toml stops the releases; the README says so.
Alternatives
- Restart the build
ai-reviewthrough the Werkator API, triggered by the runner. It reruns exactly the gate of werkrevisor. But it needs the control token of Werkator on the workstation, which can also restart, cancel and delete each build of the instance: a new and much wider authority. - The runner releases the merge itself, with the token of
ai-reviewerthat it already holds (chosen). It adds no new credential; it calls the same functions as the gate of werkrevisor, and only for a head that werkrevisor already approved. - A Gitea webhook on "review comment resolved": Gitea has no webhook event for a resolved thread, and a receiver would be a new service reachable from Gitea.
Open Questions
None.