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

Merged
dev-agent merged 2 commits from flaky-stale-approval into main 2026-10-18 13:23:25 +00:00
2 changed files with 47 additions and 3 deletions

No files matched your search

+5 -2
View File
@@ -782,8 +782,11 @@ if the implementation differs.
with code's list of the calls they replace, become a `compaction` event
that renders in place of its range; a later one folds in the earlier.
Failed notes are logged (`context.compaction_failed`) and fitting takes
over; `context.compacted` logs each one. Not yet in nisse-dev's helper
loop.
over; `context.compacted` logs each one. Not in nisse-dev's helper loop,
on purpose (operator, 2026-10-18): its token cap (`delegation.max_tokens`,
200k, every call's whole prompt counted) ends a delegation at a prompt of
about 36k tokens, well short of `compact_at` on any window in use. Wire
`Compactor` in if that cap is raised several-fold.
- **L6 fitting order:** first shorten old tool results to their first and last
lines, then drop the oldest events with a note saying so; never the system
prompt, the current plan or the most recent turns. View labels:
+42 -1
View File
@@ -1,9 +1,12 @@
"""The trust dial's hard wall, on a real Gitea (D4, D5; M3.3): with the
protection a `merge: approved` project has, the agent's own token can't
merge its PR until an operator approves the PR as it is now; it can then;
and a commit pushed after the approval needs approving again."""
and a commit pushed after the approval needs approving again: at once by the
agent's own check, and by Gitea once it has dismissed the approval."""
import base64
import re
import time
import uuid
from pathlib import Path
@@ -15,6 +18,9 @@ from nisse_dev.config import AgentConfig
pytestmark = pytest.mark.integration
# How long Gitea gets to dismiss an approval after a push (#162).
DISMISS_WAIT_S = 30.0
def add_file(gitea: GiteaClient, repo: str, branch: str, path: str) -> None:
gitea._request(
@@ -34,6 +40,34 @@ def approve(operator: GiteaClient, repo: str, pr: int) -> None:
)
def wait_for_dismissal(gitea: GiteaClient, repo: str, pr: int) -> None:
"""Gitea dismisses a stale approval in a job it queues on the push, not
in the push itself, and its merge check counts what isn't dismissed yet.
A merge tried before that job has run can still go through (#162)."""
deadline = time.monotonic() + DISMISS_WAIT_S
Review

This loop makes up to DISMISS_WAIT_S / 0.5 ≈ 60 GET calls, so it has far more exposure to a transient Gitea error (a 5xx, a dropped connection) than any single call elsewhere in the test. A one-off GiteaError inside the window would fail the test even though the fix is working — an ironic new flake source in a PR whose whole purpose is to remove one. The premise is that Gitea can be busy/slow on CI, and a busy Gitea is also the likelier to return a transient error.

The sibling live test (test_end_to_end_loop.py, merge()) already absorbs a known transient Gitea response ("Please try again later") by retrying to a deadline rather than failing. You might do the same here: catch GiteaError inside the loop and, while under the deadline, sleep and re-check instead of propagating (ideally only for transient ones, letting a genuine 4xx fail fast). A persistent failure would still hit the deadline and fail, so nothing real is masked — the wait just becomes robust.

This loop makes up to DISMISS_WAIT_S / 0.5 ≈ 60 GET calls, so it has far more exposure to a transient Gitea error (a 5xx, a dropped connection) than any single call elsewhere in the test. A one-off `GiteaError` inside the window would fail the test even though the fix is working — an ironic new flake source in a PR whose whole purpose is to remove one. The premise is that Gitea can be busy/slow on CI, and a busy Gitea is also the likelier to return a transient error. The sibling live test (`test_end_to_end_loop.py`, `merge()`) already absorbs a known transient Gitea response ("Please try again later") by retrying to a deadline rather than failing. You might do the same here: catch `GiteaError` inside the loop and, while under the deadline, sleep and re-check instead of propagating (ideally only for transient ones, letting a genuine 4xx fail fast). A persistent failure would still hit the deadline and fail, so nothing real is masked — the wait just becomes robust.
Review

Agreed, done in a1eeab1. The loop now asks again after a dropped connection or a 5xx (transient()), up to the same 30 s deadline; any other GiteaError, such as a 4xx, is raised at once. In practice the loop usually finishes on the first or second call (the job takes about 0.2 s), but a busy Gitea is exactly when it would matter.

Agreed, done in a1eeab1. The loop now asks again after a dropped connection or a 5xx (`transient()`), up to the same 30 s deadline; any other `GiteaError`, such as a 4xx, is raised at once. In practice the loop usually finishes on the first or second call (the job takes about 0.2 s), but a busy Gitea is exactly when it would matter.
while True:
try:
if not gitea.approvals(repo, pr): # dismissed ones aren't listed
return
except GiteaError as exc:
# A busy Gitea may drop a call or answer 5xx; ask again. Anything
# else, such as a 4xx, is a real failure.
if not transient(exc) or time.monotonic() > deadline:
raise
assert time.monotonic() < deadline, (
f"Gitea didn't dismiss the approval within {DISMISS_WAIT_S}s"
)
time.sleep(0.5)
def transient(exc: GiteaError) -> bool:
"""A dropped connection or a 5xx: worth asking again."""
text = str(exc)
return text.startswith("gitea request failed") or bool(
re.search(r" returned 5\d\d:", text)
)
def test_merging_needs_the_operators_approval_of_the_pr_as_it_is(
tmp_path: Path, live_config: AgentConfig, gitea: GiteaClient
) -> None:
@@ -64,12 +98,19 @@ def test_merging_needs_the_operators_approval_of_the_pr_as_it_is(
name, head="m-2", base="main", title="b", body=""
)
approve(operator, name, second.number)
approved = gitea.pull_request(name, second.number).head_sha
add_file(gitea, name, "m-2", "c.txt") # a fix pushed after the approval
# At once, with no waiting: the PR's head is read from the branch,
# so the agent sees the new commit, and the approval is for the old
# one. Its own check (an approval at the head, not stale) holds off.
after = gitea.pull_request(name, second.number).head_sha
assert after != approved
assert not [
a
for a in gitea.approvals(name, second.number)
if a.commit == after and not a.stale
]
# Gitea's own refusal comes once it has dismissed the approval.
wait_for_dismissal(gitea, name, second.number)
with pytest.raises(GiteaError):
gitea.merge_pull_request(name, second.number)