watchpr: only alert on mergeability lost after the baseline #20

Merged

benvin merged 6 commits from benvin/watchpr-baseline-from-first-poll into main 2026-09-26 21:26:49 +10:00

Conversation 3 Commits 6 Files Changed 7

+912 -68

unkin-agent commented 2026-09-26 18:46:06 +10:00

Member

Copy Link

Copy Source

watchpr exited on its first poll for any PR already conflicted when watching began. Gitea sends mergeable: false both for a real conflict and while it recomputes the merge base after a push, so false alone decides nothing.

• a conflict alerts only once this watch has seen a merge computation start: the PR was mergeable, or its head or base SHA moved
• movement arms the rule and nothing else: it never resets the run, and a push never alerts
• the run of falses must then span 2m of wall clock, because --interval ranges from seconds to hours
• emit the baseline on stderr in both modes, as a JSON record under --json
• document the one conflict never reported: landed by the push before the watch, head and base then never moving

watchpr exited on its first poll for any PR already conflicted when watching began. Gitea sends `mergeable: false` both for a real conflict and while it recomputes the merge base after a push, so `false` alone decides nothing. - a conflict alerts only once this watch has seen a merge computation start: the PR was mergeable, or its head or base SHA moved - movement arms the rule and nothing else: it never resets the run, and a push never alerts - the run of `false`s must then span 2m of wall clock, because `--interval` ranges from seconds to hours - emit the baseline on stderr in both modes, as a JSON record under `--json` - document the one conflict never reported: landed by the push before the watch, head and base then never moving

unkin-agent added 1 commit 2026-09-26 18:46:07 +10:00

watchpr: only alert on mergeability lost after the baseline ... 6988be9ff3

Track mergeability across the whole run and decode it as a tri-state, so a PR already conflicted when watching starts is polled on instead of reported.

unkin-agent commented 2026-09-26 18:59:16 +10:00

Author

Member

Copy Link

Copy Source

• internal/agent/watch.go:185 — a MergeNo baseline disarms the conflict rule for the whole run, but Gitea also returns mergeable=false while it recomputes the merge base after a push (the rationale at watch.go:144), which is exactly when watchpr is started; if that push introduced a real conflict, every later poll reads MergeNo, everMergeable never becomes true, and the conflict is never reported (checked: non-mergeable baseline + 100 conflicted polls, no alert) -> re-arm on a HeadSHA change, or take the mergeability baseline from a settled read rather than the single baseline poll.
• cmd/watchpr/main.go:131 — the baseline line prints only a PR count, so a PR now suppressed as already-conflicted / already-CI-failing produces no output at all (ran watchpr --interval 5s: one line, no state) -> print each ref's baseline state so the condition being deliberately ignored is visible.
• internal/agent/watch.go:20-22, AGENTS.md:227 — "Gitea's mergeable is tri-state (absent/null = not yet computed)" does not hold on Gitea 1.26.2: the field is a plain bool and is always present (every payload for this repo's PRs 1-20 carries true/false), and the not-yet-computed value is false, so MergeUnknown is unreachable and the stated reason for the tri-state is wrong -> correct the claim; keep the decoding as defensive if you like, but do not document null as the recompute signal.
• nit: internal/agent/watch.go:167-176 — conflicts is reset only by MergeYes, never by MergeUnknown and never by a failed poll, so two non-adjacent MergeNo observations satisfy a rule documented as "consecutive" (No + 50 Unknown + No alerts on poll 52) -> reset the run on an unknown or failed observation.
• nit: cmd/watchpr/main.go:1-5 — the package doc still lists only the old benign set; README and AGENTS.md now also exempt baseline conditions and the agent's own pushes.
• nit: internal/agent/watch_test.go:888 — no multi-ref case for the new tracker; a conflicted run on one ref must not arm or end another ref's watch.

- internal/agent/watch.go:185 — a `MergeNo` baseline disarms the conflict rule for the whole run, but Gitea also returns `mergeable=false` while it recomputes the merge base after a push (the rationale at watch.go:144), which is exactly when watchpr is started; if that push introduced a real conflict, every later poll reads `MergeNo`, `everMergeable` never becomes true, and the conflict is never reported (checked: non-mergeable baseline + 100 conflicted polls, no alert) -> re-arm on a `HeadSHA` change, or take the mergeability baseline from a settled read rather than the single baseline poll. - cmd/watchpr/main.go:131 — the baseline line prints only a PR count, so a PR now suppressed as already-conflicted / already-CI-failing produces no output at all (ran `watchpr --interval 5s`: one line, no state) -> print each ref's baseline state so the condition being deliberately ignored is visible. - internal/agent/watch.go:20-22, AGENTS.md:227 — "Gitea's `mergeable` is tri-state (absent/null = not yet computed)" does not hold on Gitea 1.26.2: the field is a plain bool and is always present (every payload for this repo's PRs 1-20 carries `true`/`false`), and the not-yet-computed value is `false`, so `MergeUnknown` is unreachable and the stated reason for the tri-state is wrong -> correct the claim; keep the decoding as defensive if you like, but do not document null as the recompute signal. - nit: internal/agent/watch.go:167-176 — `conflicts` is reset only by `MergeYes`, never by `MergeUnknown` and never by a failed poll, so two non-adjacent `MergeNo` observations satisfy a rule documented as "consecutive" (`No` + 50 `Unknown` + `No` alerts on poll 52) -> reset the run on an unknown or failed observation. - nit: cmd/watchpr/main.go:1-5 — the package doc still lists only the old benign set; README and AGENTS.md now also exempt baseline conditions and the agent's own pushes. - nit: internal/agent/watch_test.go:888 — no multi-ref case for the new tracker; a conflicted run on one ref must not arm or end another ref's watch.

unkin-agent added 2 commits 2026-09-26 20:48:38 +10:00

watchpr: arm the conflict rule on a new merge computation ... 54c1d868d3

A mergeable=false baseline meant a conflict could never be reported, since
Gitea sends false while it recomputes after a push. Arm on a head or base SHA
change as well as on a mergeable poll, break the conflict run on unknown and
failed polls, and print the baseline with the conditions it suppresses.

docs: mergeable is a plain bool, not a tri-state on the wire 7baa194c52

unkin-agent commented 2026-09-26 21:01:01 +10:00

Author

Member

Copy Link

Copy Source

• internal/agent/watch.go:199-204 — the arming poll's own false is folded into the run: mergeInputsChanged zeroes conflicts, then track(cur) sets it back to 1 on that same snapshot, so a head or base move plus one further poll fires. Gitea returns mergeable=false for the whole post-push/post-base-move recheck (live: argocd-apps#172 read false then true seconds apart with head 3d85105a and base 53412535 both unchanged, right after main moved to 53412535), so an armed watch at an interval shorter than that window reports PR lost mergeability (conflict) on a PR that is mergeable — contradicting cmd/watchpr/main.go:5 "the agent's own pushes ... are ignored" -> do not count the arming poll's own observation toward the run, or debounce on elapsed time rather than a poll count.
• internal/agent/watch.go:199-202 — base.sha is the base branch tip, so it moves for every open PR on every push to main (verified: all open argocd-apps PRs report the same base.sha, tracking the tip). The branch that arms also resets conflicts, so where the base moves faster than --interval a genuine conflict never completes a run (armed watch, 20 conflicted polls with base moving each poll: no alert) -> arm on merge-input movement without resetting the run of consecutive false observations.
• cmd/watchpr/main.go:134 — --json drops the baseline entirely, so the only notice that the watch is deliberately silent about a pre-existing conflict is missing in the mode automation uses; the baseline goes to stderr and cannot corrupt the stdout stream. README.md:91 states the baseline line unconditionally -> emit the baseline as a JSON record on stderr under --json.
• internal/agent/watch_test.go:962-967 — the comment names "watchpr is normally started right after the agent pushes, so the baseline can land inside that window", but the scripted sequence moves the head after the baseline. The scenario it names is the one that stays silent for the whole run (MergeNo baseline on the already-pushed head, 50 stable polls, no alert) -> describe what is actually scripted.
• nit: AGENTS.md:223-226, README.md:89-91 — "keeps being watched" does not say that a conflict introduced by the push immediately before the watch is never reported, however long the run lasts -> state the residual gap.

- internal/agent/watch.go:199-204 — the arming poll's own `false` is folded into the run: `mergeInputsChanged` zeroes `conflicts`, then `track(cur)` sets it back to 1 on that same snapshot, so a head or base move plus **one** further poll fires. Gitea returns `mergeable=false` for the whole post-push/post-base-move recheck (live: argocd-apps#172 read `false` then `true` seconds apart with head `3d85105a` and base `53412535` both unchanged, right after main moved to `53412535`), so an armed watch at an interval shorter than that window reports `PR lost mergeability (conflict)` on a PR that is mergeable — contradicting cmd/watchpr/main.go:5 "the agent's own pushes ... are ignored" -> do not count the arming poll's own observation toward the run, or debounce on elapsed time rather than a poll count. - internal/agent/watch.go:199-202 — `base.sha` is the base branch tip, so it moves for every open PR on every push to main (verified: all open argocd-apps PRs report the same `base.sha`, tracking the tip). The branch that arms also resets `conflicts`, so where the base moves faster than `--interval` a genuine conflict never completes a run (armed watch, 20 conflicted polls with base moving each poll: no alert) -> arm on merge-input movement without resetting the run of consecutive `false` observations. - cmd/watchpr/main.go:134 — `--json` drops the baseline entirely, so the only notice that the watch is deliberately silent about a pre-existing conflict is missing in the mode automation uses; the baseline goes to stderr and cannot corrupt the stdout stream. README.md:91 states the baseline line unconditionally -> emit the baseline as a JSON record on stderr under `--json`. - internal/agent/watch_test.go:962-967 — the comment names "watchpr is normally started right after the agent pushes, so the baseline can land inside that window", but the scripted sequence moves the head *after* the baseline. The scenario it names is the one that stays silent for the whole run (`MergeNo` baseline on the already-pushed head, 50 stable polls, no alert) -> describe what is actually scripted. - nit: AGENTS.md:223-226, README.md:89-91 — "keeps being watched" does not say that a conflict introduced by the push immediately before the watch is never reported, however long the run lasts -> state the residual gap.

unkin-agent added 3 commits 2026-09-26 21:14:31 +10:00

watchpr: debounce conflicts on elapsed time, not poll count ... 27e48ac45e

Merge-input movement arms the conflict rule and no longer clears the run of
non-mergeable observations, so an arming poll's own false cannot be half the
evidence and a base branch moving under every poll cannot starve a real
conflict. The run must now span conflictWindow, measured on the tick that
fired each poll.

watchpr: emit the baseline under --json ... 15c527ee56

The baseline is the only notice that a watch started against an already
conflicted PR, and it was missing from the mode automation uses. It goes to
stderr as a JSON record, leaving stdout a single result record.

docs: name the conflict watchpr will never report 91509cb9b2

benvin merged commit 7ffa123e3d into main 2026-09-26 21:26:49 +10:00

benvin deleted branch benvin/watchpr-baseline-from-first-poll 2026-09-26 21:26:49 +10:00

unkin-agent commented 2026-09-26 21:46:21 +10:00

Author

Member

Copy Link

Copy Source

• internal/agent/watch.go:187-199, 218-228 — the run of falses accumulates while the watch is still disarmed, and observe arms and then evaluates confirmed(now) against that pre-arming conflictSince, so the first head/base move confirms a run it played no part in, on its own recompute poll. Baseline MergeNo + three stable MergeNo polls at 60s + one poll whose base.sha moved alerts PR lost mergeability (conflict) on that fourth poll (verified through Watch with spacedTicks); same with head.sha moving, which contradicts cmd/watchpr/main.go:5 and AGENTS.md:244 "a push never alerts". On this Gitea base.sha is the base-branch tip (live: every open argocd-apps PR reports base.sha = main's tip 426a399f), so this fires on any watch baselined non-mergeable as soon as anyone merges to main -> start the window at the later of the run's first observation and the armed false→true transition, and add the sequence to TestWatchConflictSequences, which has no case where falses precede the arming move.
• README.md:95-104 — "the non-mergeable polls have then run unbroken for two minutes" describes a window measured from arming; the code measures from the run's first observation, armed or not (above). The "one case therefore never reported" paragraph also omits that base.sha is the base-branch tip, so its "head and base never move again" precondition ends at the next push to main -> state the rule the code implements, and say the gap closes when the head or base next moves.
• nit: cmd/watchpr/main.go:140,143 and 105 — under --json stderr carries NDJSON baseline records interleaved with plain-text warning: lines, so stderr is not parseable as a stream; a caller that reads it as NDJSON breaks on the first poll error or a missing token -> emit the warnings as records too under --json, or document stderr as human-readable with JSON records embedded.
• nit: internal/agent/watch.go:171,190,211 — the zero time.Time is both "no run in progress" and a legal clock value, so a ticks channel delivering zero times never confirms a conflict however long it runs (verified: 50 conflicted polls, no alert) -> track the run with an explicit flag, or state the non-zero-clock requirement on Watch's doc comment.
• nit: cmd/watchpr/main.go:234-243 — suppressedAtBaseline names MergeNo and CI red, but a MergeUnknown baseline is equally disarmed and produces no notice -> report it too.

- internal/agent/watch.go:187-199, 218-228 — the run of `false`s accumulates while the watch is still disarmed, and `observe` arms and then evaluates `confirmed(now)` against that pre-arming `conflictSince`, so the first head/base move confirms a run it played no part in, on its own recompute poll. Baseline `MergeNo` + three stable `MergeNo` polls at 60s + one poll whose `base.sha` moved alerts `PR lost mergeability (conflict)` on that fourth poll (verified through `Watch` with `spacedTicks`); same with `head.sha` moving, which contradicts cmd/watchpr/main.go:5 and AGENTS.md:244 "a push never alerts". On this Gitea `base.sha` is the base-branch tip (live: every open argocd-apps PR reports `base.sha` = main's tip `426a399f`), so this fires on any watch baselined non-mergeable as soon as anyone merges to main -> start the window at the later of the run's first observation and the `armed` false→true transition, and add the sequence to `TestWatchConflictSequences`, which has no case where falses precede the arming move. - README.md:95-104 — "the non-mergeable polls have **then** run unbroken for two minutes" describes a window measured from arming; the code measures from the run's first observation, armed or not (above). The "one case therefore never reported" paragraph also omits that `base.sha` is the base-branch tip, so its "head and base never move again" precondition ends at the next push to main -> state the rule the code implements, and say the gap closes when the head or base next moves. - nit: cmd/watchpr/main.go:140,143 and 105 — under `--json` stderr carries NDJSON baseline records interleaved with plain-text `warning:` lines, so stderr is not parseable as a stream; a caller that reads it as NDJSON breaks on the first poll error or a missing token -> emit the warnings as records too under `--json`, or document stderr as human-readable with JSON records embedded. - nit: internal/agent/watch.go:171,190,211 — the zero `time.Time` is both "no run in progress" and a legal clock value, so a `ticks` channel delivering zero times never confirms a conflict however long it runs (verified: 50 conflicted polls, no alert) -> track the run with an explicit flag, or state the non-zero-clock requirement on `Watch`'s doc comment. - nit: cmd/watchpr/main.go:234-243 — `suppressedAtBaseline` names `MergeNo` and CI red, but a `MergeUnknown` baseline is equally disarmed and produces no notice -> report it too.

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/agent-tools#20