watchpr: fix watch mode never detecting changes #5

2026-08-15T13:23:39+10:00

unkin-agent commented

2026-08-15 13:23:39 +10:00

watchpr watch mode establishes its baseline and then never detects changes or exits: it ran 37+ minutes and silently missed two PRs merging, while --once reports state correctly. The poll loop was not the culprit -- the fetch it depends on was.

Root cause: FetchState calls GetPR (which returns merged=true for the merged PR) and then CommitStatus(pr.Head.Sha). When a PR is squash/rebase-merged and its branch is auto-deleted (default_delete_branch_after_merge is on for these repos), the original head commit becomes unreachable and the status endpoint returns HTTP 404. FetchState treated that 404 as fatal and returned an error, discarding the merged=true signal it had just fetched. The watch loop then saw only a transient poll error, logged a warning, and kept polling forever -- the observed hang. Regular merges keep the head commit reachable as a merge parent, which is why the earlier fix (PR #3) looked correct and --once on open PRs worked.

Changes:

Add a typed APIError carrying the HTTP status code so callers can detect a 404 via errors.As instead of parsing error strings.
FetchState tolerates a 404 from CommitStatus (a gone commit has no CI status) and returns the authoritative merged/closed PR state; non-404 status errors still fail.
Add regression tests: FetchState survives a 404 status endpoint, and the full watch loop -- driven through a real *GiteaClient against an httptest Gitea server -- detects a merge whose head commit is gone. Both fail/hang against the pre-fix code. A guard test keeps non-404 status errors fatal.

Validation: gofmt -l . clean, go vet ./... clean, go test -race ./... passes, make build succeeds, golangci-lint run ./... (CI image) reports 0 issues.

watchpr watch mode establishes its baseline and then never detects changes or exits: it ran 37+ minutes and silently missed two PRs merging, while `--once` reports state correctly. The poll loop was not the culprit -- the fetch it depends on was. Root cause: `FetchState` calls `GetPR` (which returns `merged=true` for the merged PR) and then `CommitStatus(pr.Head.Sha)`. When a PR is squash/rebase-merged and its branch is auto-deleted (`default_delete_branch_after_merge` is on for these repos), the original head commit becomes unreachable and the status endpoint returns HTTP 404. `FetchState` treated that 404 as fatal and returned an error, discarding the `merged=true` signal it had just fetched. The watch loop then saw only a transient poll error, logged a warning, and kept polling forever -- the observed hang. Regular merges keep the head commit reachable as a merge parent, which is why the earlier fix (PR #3) looked correct and `--once` on open PRs worked. Changes: - Add a typed `APIError` carrying the HTTP status code so callers can detect a 404 via `errors.As` instead of parsing error strings. - `FetchState` tolerates a 404 from `CommitStatus` (a gone commit has no CI status) and returns the authoritative merged/closed PR state; non-404 status errors still fail. - Add regression tests: `FetchState` survives a 404 status endpoint, and the full watch loop -- driven through a real `*GiteaClient` against an httptest Gitea server -- detects a merge whose head commit is gone. Both fail/hang against the pre-fix code. A guard test keeps non-404 status errors fatal. Validation: `gofmt -l .` clean, `go vet ./...` clean, `go test -race ./...` passes, `make build` succeeds, `golangci-lint run ./...` (CI image) reports 0 issues.

unkin-agent added 1 commit 2026-08-15 13:23:40 +10:00

watchpr: fix watch mode hanging when a merged PR's head commit is gone

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

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

Details

fda3761ead

FetchState fetched the PR (merged=true) but then failed the whole state
fetch when CommitStatus 404'd for a head commit that no longer existed
(the branch was deleted after a squash/rebase merge). The merge signal was
discarded, so the watch loop treated every post-merge poll as a transient
error and never exited -- the 37-minute hang seen in production.

- Add a typed APIError carrying the HTTP status so callers can detect a 404
  without parsing error strings.
- FetchState now tolerates a 404 from CommitStatus (commit gone => no CI
  status) and returns the authoritative merged/closed PR state.
- Regression tests: FetchState survives a 404 status; the full watch loop,
  driven through a real client, detects a merge whose head commit is gone
  (both fail/hang before the fix).

benvin merged commit 60b08f1198 into main

2026-08-15 15:00:02 +10:00

benvin deleted branch benvin/watchpr-watchloop-fix

2026-08-15 15:00:02 +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#5