From ebdd25f725a224777a36c57dcd19749a47a16e4e Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 26 Sep 2026 22:30:14 +1000 Subject: [PATCH] watchpr: describe arming as a one-way latch, not a window restart The docs claimed every later head/base move restarts the conflict window. arm() returns early once armed, so only the first move sets armedAt and every move after it is a no-op. State the real trade-off instead: repeated moves do not extend the debounce, so a conflict can be confirmed while the newest recompute is younger than the window. Also narrow the --json stderr claim to the notices watchpr writes itself (cobra's terminal Error: line is plain text), rename the test to what it covers, and stop the unknown-mergeability baseline implying a false answer arms the rule. --- README.md | 16 ++++++++++++---- cmd/watchpr/main.go | 18 ++++++++++++------ cmd/watchpr/main_test.go | 10 ++++++---- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 62ef75f..a0a0590 100644 --- a/README.md +++ b/README.md @@ -105,10 +105,18 @@ reported while the commits stay put: nothing in Gitea's payload separates it fro a merge check still in flight, and the baseline line is the only notice of it. That is a narrower gap than it looks, because `base.sha` is the base branch's tip — it moves for every open PR whenever anything merges to the base branch, so -the commits rarely stay put for long. A move like that arms the rule, and the -move itself is silent: the window restarts from it, so what is eventually -reported is a conflict still standing two minutes after that fresh merge -computation began, never the run of falses that preceded it. +the commits rarely stay put for long. The first such move arms the rule, and is +itself silent: the window then runs from that arming poll rather than from the +falses that predate it, so what is eventually reported is a conflict that +outlasted the merge computation the move started. + +Arming is a one-way latch. Moves after that one — and every move seen by a watch +that was already armed at the baseline — neither re-arm nor restart the window, +because a restart on every `base.sha` move could never complete on a busy base +branch. That is the cost side of the same trade: on a busy base branch a +conflict can be confirmed while the newest merge recompute is less than two +minutes old. The window guarantees that the run of falses outlasted *a* merge +computation this watch saw start, not that it outlasted the most recent one. ```bash # Watch until something meaningful happens (default interval 60s) diff --git a/cmd/watchpr/main.go b/cmd/watchpr/main.go index 1cc858a..a5a155a 100644 --- a/cmd/watchpr/main.go +++ b/cmd/watchpr/main.go @@ -8,8 +8,12 @@ // so. A conflict introduced by the push immediately before the watch started is // not reported while the commits stay put, because nothing in Gitea's payload // separates it from a merge check still in flight; the baseline line is the only -// notice of it. A later move of the head or base arms the rule without alerting, -// and restarts the two-minute window from itself. +// notice of it. The first later move of the head or base arms the rule without +// alerting, and the window then runs from that arm rather than from the falses +// that predate it. Moves after that neither re-arm nor restart the window -- a +// restart on every base move could never complete on a busy base branch -- so a +// conflict can be confirmed while the newest merge recompute is less than two +// minutes old. // // watchpr owner/repo#12 owner/repo:15 // watchpr --once --json owner/repo#12 @@ -108,9 +112,11 @@ func clientFor(jsonMode bool) *agent.GiteaClient { return agent.NewGiteaClient(token) } -// warnRecord is the --json form of a warning. Under --json stderr carries -// nothing but NDJSON records, so a caller parsing it line by line never has to -// guess which shape a line is. +// warnRecord is the --json form of a warning. Under --json every notice watchpr +// itself writes to stderr -- warnings and the baseline -- is an NDJSON record, +// so a caller parsing them line by line never has to guess which shape a line +// is. A terminal failure is the one exception: SilenceErrors stays off, so +// cobra prints it as a plain "Error: ..." line and the exit status is non-zero. type warnRecord struct { Warning string `json:"warning"` } @@ -255,7 +261,7 @@ func suppressedAtBaseline(st agent.PRState) string { case agent.MergeNo: conds = append(conds, "already non-mergeable (a real conflict, or Gitea still recomputing)") case agent.MergeUnknown: - conds = append(conds, "mergeability unknown (the conflict rule is disarmed until Gitea answers or the commits move)") + conds = append(conds, "mergeability unknown (the conflict rule is disarmed until Gitea reports mergeable or the commits move)") } if st.CIStatus == "failure" || st.CIStatus == "error" { conds = append(conds, "CI already "+st.CIStatus) diff --git a/cmd/watchpr/main_test.go b/cmd/watchpr/main_test.go index 11ac328..62340f1 100644 --- a/cmd/watchpr/main_test.go +++ b/cmd/watchpr/main_test.go @@ -257,10 +257,12 @@ func TestBaselineIsEmittedInJSONMode(t *testing.T) { } } -// Under --json stderr is the baseline and warning channel, so it has to be one -// shape: a caller parsing it line by line must never meet a bare `warning:` -// line between two NDJSON records. -func TestJSONModeStderrIsAllRecords(t *testing.T) { +// Under --json stderr is the baseline and warning channel, so every notice +// watchpr writes there has to be one shape: a caller parsing it line by line +// must never meet a bare `warning:` line between two NDJSON records. Cobra's +// terminal `Error: ...` line is not covered here -- SilenceErrors stays off, so +// it is plain text on stderr alongside a non-zero exit. +func TestJSONModeWarningsAndBaselineAreRecords(t *testing.T) { st := agent.PRState{ Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7}, State: "open",