Flaky: test_merging_needs_the_operators_approval_of_the_pr_as_it_is (Gitea let a stale approval merge) #162

New Issue

2026-10-17T20:31:00Z

dev-agent commented

2026-10-17 20:31:00 +00:00

CI run 667 (attempt 2, PR #161) failed tests/integration/test_trust_live.py::test_merging_needs_the_operators_approval_of_the_pr_as_it_is:

approve(operator, name, second.number)
add_file(gitea, name, "m-2", "c.txt")  # a fix pushed after the approval
...  # gitea.approvals() already shows no current approval at the new head
with pytest.raises(GiteaError):
    gitea.merge_pull_request(name, second.number)
E   Failed: DID NOT RAISE GiteaError

The same commit passed this test in attempt 1, and it passed 13/13 runs locally against the same Gitea (28.1.0). Likely a race: Gitea's branch protection still counted the approval at merge time, just after the push. Dismissing stale approvals may be async (queued on the push), even though the reviews API already reported the approval as stale.

nisse-dev's own safety doesn't rest on this: it checks for an operator approval at the current head (not stale) before asking Gitea to merge. Only the test's claim that Gitea refuses a merge on its own is timing-dependent.

Options: retry the merge refusal check for a few seconds after the push; or assert on our own pre-merge check and treat Gitea's refusal as best-effort.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

CI run 667 (attempt 2, PR #161) failed `tests/integration/test_trust_live.py::test_merging_needs_the_operators_approval_of_the_pr_as_it_is`: ``` approve(operator, name, second.number) add_file(gitea, name, "m-2", "c.txt") # a fix pushed after the approval ... # gitea.approvals() already shows no current approval at the new head with pytest.raises(GiteaError): gitea.merge_pull_request(name, second.number) E Failed: DID NOT RAISE GiteaError ``` The same commit passed this test in attempt 1, and it passed 13/13 runs locally against the same Gitea (28.1.0). Likely a race: Gitea's branch protection still counted the approval at merge time, just after the push. Dismissing stale approvals may be async (queued on the push), even though the reviews API already reported the approval as stale. nisse-dev's own safety doesn't rest on this: it checks for an operator approval at the current head (not stale) before asking Gitea to merge. Only the test's claim that Gitea refuses a merge on its own is timing-dependent. Options: retry the merge refusal check for a few seconds after the push; or assert on our own pre-merge check and treat Gitea's refusal as best-effort. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

dev-agent referenced a pull request that will close this issue

2026-10-18 04:28:00 +00:00

Trust test: wait for Gitea to dismiss a stale approval before merging (#162) #164

dev-agent referenced this issue from a commit

2026-10-18 04:28:00 +00:00

Trust test: wait for Gitea to dismiss a stale approval before merging (#162)

dev-agent commented

2026-10-18 04:28:55 +00:00

Cause found, fix in #164.

Cause: Gitea v28.1.0 dismisses a stale approval in a job it queues on the push (AddTestPullRequestTask), not in the push itself. The merge check (GetGrantedApprovalsCount) counts approvals that are not dismissed yet. Measured here, the approval stays in place for 0.15–0.23 s after a push, so a merge in that window goes through.
My earlier guess was wrong: the reviews API had not reported the approval as stale. The old assertion compared commits and never looked at stale.
It failed twice, not once: also on main after the #160 merge (run 672).
nisse-dev is not affected: the PR API reads the head from the branch itself, so the agent sees the new commit at once, and its own check needs an approval at that commit.

🤖 Generated with Claude Code

Cause found, fix in #164. - **Cause:** Gitea v28.1.0 dismisses a stale approval in a job it queues on the push (`AddTestPullRequestTask`), not in the push itself. The merge check (`GetGrantedApprovalsCount`) counts approvals that are not dismissed yet. Measured here, the approval stays in place for 0.15–0.23 s after a push, so a merge in that window goes through. - **My earlier guess was wrong:** the reviews API had not reported the approval as stale. The old assertion compared commits and never looked at `stale`. - **It failed twice, not once:** also on `main` after the #160 merge (run 672). - **nisse-dev is not affected:** the PR API reads the head from the branch itself, so the agent sees the new commit at once, and its own check needs an approval at that commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dev-agent closed this issue

2026-10-18 13:23:25 +00:00

dev-agent referenced this issue from a commit

2026-10-18 13:23:25 +00:00

Merge pull request 'Trust test: wait for Gitea to dismiss a stale approval before merging (#162)' (#164) from flaky-stale-approval into main

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dev/nisse#162