watchpr: start the conflict window at the arm #21

2026-09-26T22:14:41+10:00

unkin-agent commented

2026-09-26 22:14:41 +10:00

The conflict window ran from the first non-mergeable poll even while the rule was disarmed, so the poll that armed it — a push, or base.sha moving — confirmed a run it played no part in. Any watch baselined non-mergeable alerted falsely the moment main moved.

measure the window from the later of the run start and the disarmed→armed transition
keep re-arming a no-op, so a moving base cannot reset the run
hold both times as pointers; the zero time.Time is a legal clock value
emit warnings as NDJSON under --json; cobra's terminal Error: line stays plain text
name an unknown mergeability in the baseline suppression line
document arming as a one-way latch: repeated moves do not extend the debounce

The conflict window ran from the first non-mergeable poll even while the rule was disarmed, so the poll that armed it — a push, or `base.sha` moving — confirmed a run it played no part in. Any watch baselined non-mergeable alerted falsely the moment main moved. - measure the window from the later of the run start and the disarmed→armed transition - keep re-arming a no-op, so a moving base cannot reset the run - hold both times as pointers; the zero `time.Time` is a legal clock value - emit warnings as NDJSON under `--json`; cobra's terminal `Error:` line stays plain text - name an unknown mergeability in the baseline suppression line - document arming as a one-way latch: repeated moves do not extend the debounce

unkin-agent added 1 commit 2026-09-26 22:14:42 +10:00

watchpr: start the conflict window at the arm

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

5928233a97

The window ran from the first non-mergeable poll even while the rule was
disarmed, so the poll that armed it confirmed a run it played no part in.
Measure from the later of the run's start and the arm; re-arming stays a
no-op so a base moving under every poll still confirms.

unkin-agent commented

2026-09-26 22:24:57 +10:00

Findings:

cmd/watchpr/main.go:11-12 / README.md:109 — both say a later head/base move restarts the two-minute window. arm() returns early when already armed, so only the disarmed→armed transition ever moves armedAt; a move under an already-armed watch (the mergeable-baseline case, and every move after the first) restarts nothing. Verified: baseline mergeable, run of falses from t=30s, base.sha moves at t=90s, alert fires at t=150s — 60s into that fresh recompute, not 120s. -> scope the sentence to the first arm, or drop the "restarts from itself" claim.
cmd/watchpr/main.go:111-112 — "under --json stderr carries nothing but NDJSON records" is not true: cobra still prints Error: <msg> as plain text to stderr, so a caller parsing stderr line by line meets a non-record line on exactly the failures it cares about (describeFailure, bad ref, unknown flag, no refs — all confirmed against the built binary). TestJSONModeStderrIsAllRecords asserts the invariant only over warn and emitBaselines, so its name overstates too. -> emit the terminal error as a record (SilenceErrors + warn), or scope the comment and the test name to warnings and baselines.
nit: cmd/watchpr/main.go:258 — "disarmed until Gitea answers or the commits move": a false answer does not arm; only true or movement does. -> "until Gitea reports it mergeable or the commits move".
nit: the --json warning normalisation and the MergeUnknown baseline line are independent of the window fix and would stand alone.

Findings: - `cmd/watchpr/main.go:11-12` / `README.md:109` — both say a later head/base move restarts the two-minute window. `arm()` returns early when already armed, so only the disarmed→armed transition ever moves `armedAt`; a move under an already-armed watch (the mergeable-baseline case, and every move after the first) restarts nothing. Verified: baseline mergeable, run of falses from t=30s, `base.sha` moves at t=90s, alert fires at t=150s — 60s into that fresh recompute, not 120s. -> scope the sentence to the first arm, or drop the "restarts from itself" claim. - `cmd/watchpr/main.go:111-112` — "under `--json` stderr carries nothing but NDJSON records" is not true: cobra still prints `Error: <msg>` as plain text to stderr, so a caller parsing stderr line by line meets a non-record line on exactly the failures it cares about (`describeFailure`, bad ref, unknown flag, no refs — all confirmed against the built binary). `TestJSONModeStderrIsAllRecords` asserts the invariant only over `warn` and `emitBaselines`, so its name overstates too. -> emit the terminal error as a record (`SilenceErrors` + `warn`), or scope the comment and the test name to warnings and baselines. - nit: `cmd/watchpr/main.go:258` — "disarmed until Gitea answers or the commits move": a `false` answer does not arm; only `true` or movement does. -> "until Gitea reports it mergeable or the commits move". - nit: the `--json` warning normalisation and the `MergeUnknown` baseline line are independent of the window fix and would stand alone.

unkin-agent added 1 commit 2026-09-26 22:30:22 +10:00

watchpr: describe arming as a one-way latch, not a window restart

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

ebdd25f725

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.

benvin merged commit 5488a1ddc6 into main

2026-09-26 23:45:16 +10:00

benvin deleted branch benvin/watchpr-arm-window-start

2026-09-26 23:45:16 +10:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/agent-tools#21