fix(server): PR watches stop burning GitHub's rate limit and giving up#16208

🟡 Medium orchestration-v2/PullRequestWatchReactor.ts:111

A watch created in the same millisecond as the prior successful group read is not read immediately, so it remains unevaluated until the snapshot changes or the ten-minute quiet reread. Because the initial-read check uses > against last.at, startedAt === last.at fails the check; use >= (or track a distinct watch generation).

Fixed in 2359cc99c9. I didn't use >=: under a frozen clock, every pass would read again. The last read now records which watches it evaluated (thread plus startedAt). Any watch outside that set gets its first read on the next pass. The shared-read runtime test now starts a third watch on the quiet PR and asserts one extra read plus a "checks passed" wake. It fails with the old > comparison.

🟡 Medium orchestration-v2/PullRequestWatchReactor.ts:324

A failed giveUp dispatch still clears readFailures, so a watch whose stop was not applied loses its due-stop state and must accumulate another READ_FAILURE_LIMIT failures before retrying. Because eachTarget catches dispatch failures internally, only clear the counter after confirming all stop dispatches succeeded.

Fixed in fb26b8ab19. eachTarget now succeeds with whether every step landed. The count is cleared only when all give-up stops land, so a failed stop is retried on the next failed read. I didn't drop the delete entirely: a watch restarted before the next pass would then inherit a count at the limit and end after one failed read.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Invalidate the group cache when a target update fails.

A quiet read marks every target evaluated before record dispatches its update. If the dispatch fails, eachTarget catches the error but leaves the cache intact. The next sweep can then skip that target until the ten-minute reread, delaying its wake. Delete the group’s cached read in this catch so the next sweep retries it.

✅ Addressed in commits fb26b8a to 9d0ba37

Fixed in 9d0ba37c61. A failed step in eachTarget now clears the group's cached read, so the next pass reads the PR again and retries the update.

@t3dotgg Thanks for the fix. The supplied code shows that eachTarget now deletes lastReads[group.key] when a target update fails. The next sweep will read the PR again and retry the update.

✅ Review thread resolved.

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include projectId in the watch-group key.

threadPullRequestKeyOf groups by host, repository, and PR number. readGroup uses the first target’s projectId for both reads. When two projects resolve the PR through different matching checkouts, a persistent non-rate-limit failure in the first context can stop every watch in the group after eight failures, including watches whose own project could read it.

✅ Addressed in commits fb26b8a to 9d0ba37

Fixed in 9d0ba37c61. The group key now includes thread.projectId, so each project reads through its own checkout and fails on its own. Sibling threads in one project still share a read, and that was the fan-out case seen on the fleet.

@t3dotgg Thanks for the clarification. The supplied code includes thread.projectId in the group key. This isolates reads and failure counts by project while preserving shared reads for sibling threads in one project.

✅ Review thread resolved.

You are interacting with an AI system.

🟡 Medium orchestration-v2/PullRequestWatchReactor.ts:351

A successful host read is cached even when a target's record dispatch fails, so that target's watch update is skipped on later passes for up to ten minutes. eachTarget only logs the failure after lastReads has been populated; retain the cache only when every target dispatch succeeds, and clear it otherwise so the group is retried.

No longer relevant as of 9d0ba37c61316a226af8a0c8d13fda58e91c5d8c