[cgwalters-bot]

Operator replies without a mention should reach the dispatcher without waiting for the full sweep. Add a conditional notifications fast path to the existing loop, verifying the API author and tracked open thread before reporting a news wake. Keep the sweep as the safety net.

Reworked three times after LLM review (all squashed into the one commit): unreadable threads no longer stop the poll, the work per poll is bounded and resumable, and retries are kept apart from new work so a failing backlog can't hold back later pages or the next window. The latest rework (head ba5a2d5, from run 37969725194) bounds the retained retries: one entry per thread, exponential backoff, a 24 hour lifetime and count and size caps, each reported as a fallback to the sweep; and tests/bot-supervisor.sh now runs tests/poll-notifications.test.js, so CI covers it.

Tested in that run, all exit 0: node --test tests/poll-notifications.test.js, cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace, bash tests/bot-supervisor.sh, bash tests/bot-poll-loop.sh, shellcheck -x bin/bot-poll-loop bin/poll-notifications.sh tests/bot-poll-loop.sh tests/bot-supervisor.sh. CI on this head: run 37971232368, green. Live authenticated polling was not possible on the credential-free runner.

Left for a follow-up, from the LLM review of this head: docs/scheduled-dispatcher.md and a comment in bin/bot-poll-loop still say a failed read holds the polling window back, and the tests cover few pull request and rejected-URL shapes.

Not critical: read-only API calls, no change to who can authorize actions, credentials or sandboxing. #184 is an earlier apply of the first run for the same item and is superseded by this one.

From run 37953108069 for tracker#468.

Generated-by: https://github.com/cgwalters/#llms

[cgwalters-bot]

Review by an LLM (run 37955193804), requested by the coordinator. Not a human review.

VERDICT: CHANGES — two merge blockers below. LLM-generated review by gpt-6.1-sol (praxis/gpt-6.1-sol).

1. P1, blocks merge — one unavailable comment disables the entire fast path. bin/poll-notifications.sh:59,64,75–80: any comment/issue read failure returns before publishing even already-verified operator comments or advancing the window. A deleted comment returning 404, or a notification from a repository whose access was revoked, is retried indefinitely and suppresses unrelated valid news. I reproduced two successive polls containing a valid operator comment followed by a 404: both returned 1, reported no news, and retained the same cursor. Distinguish terminal unavailability from transient errors; skip/tombstone terminal failures, deliver successfully verified news, and preserve retry state for transient failures without starving other threads. Add mixed-success/404 and transient-recovery tests.

2. P2, blocks merge — new network traversal has no work/deadline bound. bin/poll-notifications.sh:48–59,64,67 and bin/bot-poll-loop:357,381: pagination accumulates every page and then serially fetches every latest comment before the loop checks its deadline or reports other pending actions. There is no page/thread/byte cap, repeated-next detection, or overall timeout. Notification volume is partly controlled by unrelated users, and failure retains the growing window. This can consume the poll window and delay deterministic reconciliation/approval handling, despite --max-wait. Bound requests and elapsed time, stop pagination cycles, and retain resumable work without advancing past unprocessed notifications; test an over-budget feed and repeated next link.

Sound: the diff changes no workflows, permissions, action pins, sandbox/check/apply gates, or Rust code. New API calls are reads; notification URLs are restricted to GitHub API paths, bot identity and the configured operator's API author are checked, open/tracked eligibility is checked before waking, and comment bodies/titles are not used as commands or authority. State is under the existing lock; ordinary dedupe, conditional-request issuance, and restart due-time gating are covered by data-driven tests. The sweep remains a fallback, but does not repair fast-path starvation.

Checks: cargo fmt --check passed (exit 0, 0.153s); cargo clippy --workspace --all-targets -- -D warnings passed (0, 16.984s); cargo test --workspace passed (0, 30.377s; 65 tests). bash tests/bot-poll-loop.sh passed (0, 24.698s); bash tests/gh-http.sh passed (0, 0.060s); node --test tests/bot-supervisor.test.js passed (0, 21.802s; 34 tests); shellcheck -x bin/bot-poll-loop bin/poll-notifications.sh tests/bot-poll-loop.sh passed (0, 1.081s). The offline 404 reproduction passed its failure-mode assertions (0, 0.125s).

Limits: no authenticated live polling or full shell-test suite was run. The public PR currently shows one commit, no earlier review, and no description of rework; the checkout is a4c79a5. I reviewed that actual diff, but cannot verify the brief's claimed second-run fixes against an unavailable earlier review/rework. Working tree left unchanged.

Generated-by: https://github.com/cgwalters/#llms

[cgwalters-bot]

Review by an LLM (run 37958891706), requested by the coordinator. Not a human review.

VERDICT: CHANGES — one merge blocker. LLM-generated review by gpt-6.1-sol (praxis/gpt-6.1-sol).

1. P2, blocks merge — retry backlog can permanently starve acquisition. bin/poll-notifications.js:63–64,90–97,121–122: new pages are acquired only when state.queue is empty, but retries are appended to that same queue on every invocation. With more failing threads than the request budget can process, the queue never empties at the acquisition check. I reproduced 30 polls of a 100-thread first page whose comment reads return 403: the first page was fetched once, page 2 was never fetched, and all 100 threads kept recycling. Valid operator news on later pages (and eventually in later windows) is therefore indefinitely suppressed. Keep acquisition/unprocessed work separate from retries and reserve bounded acquisition capacity even when retries remain; add a multi-poll regression with a large failing first page and a valid later-page operator comment. The existing mixed-success tests use only two threads, and the over-budget test makes every read succeed, so neither catches this interaction.

Sound: reviewed head 1a06fc1 against origin/main. The rework fixes terminal 404/410/451 suppression, delivers verified news despite other failures, and introduces request/page/subprocess-byte/deadline bounds, resumable pagination and cycle detection. Those individual behaviors pass the new tests; fairness under combined failures and budget exhaustion does not. The diff changes no workflows, permissions, action pins, sandbox, check/apply gates, or Rust code. API calls are reads; subprocess arguments are not shell-expanded; bot identity, operator API author and open/tracked eligibility are checked before dispatch. Comment bodies/titles do not confer authority, and state uses the existing lock. No unauthorized-write or widened dispatch-authority path found.

Checks (all exit 0): cargo fmt --check (0.115s); cargo clippy --workspace --all-targets -- -D warnings (10.010s); cargo test --workspace (25.642s, 65 tests); node --test tests/poll-notifications.test.js tests/bot-supervisor.test.js (17.742s, 47 tests); bash tests/bot-poll-loop.sh (30.937s); shellcheck -x bin/bot-poll-loop bin/poll-notifications.sh tests/bot-poll-loop.sh (0.619s). Offline starvation reproduction: node /home/runner-sandbox/review-notifications.cjs (0.174s, exit 0; assertions confirm the failure).

Limits: no authenticated live GitHub polling or full repository shell suite. The public PR description still describes the original implementation rather than enumerating rework fixes; I verified the prior review's requested fixes against the actual code. The commit-review skill was unavailable on this runner. Working tree unchanged.

Generated-by: https://github.com/cgwalters/#llms

[cgwalters-bot]

Review by an LLM (run 37965946158), requested by the coordinator. Not a human review.

VERDICT: CHANGES — two merge blockers below. LLM-generated review by gpt-6.1-sol (praxis/gpt-6.1-sol).

1. P2, blocks merge — persistent retry work is unbounded. bin/poll-notifications.js:126–127,141–143: every transient failure is retained forever, and deduplication includes updated_at. Repeated activity on one inaccessible thread therefore creates additional retries for the same latest-comment endpoint. I reproduced 100 polls with one repeatedly updated thread returning 403: all 100 versions remained in state.retry. Eligibility is checked only after these reads, so unrelated repository activity can grow this state too. The request/page/buffer caps do not bound this accumulated state; reading, parsing, deduplicating and rewriting it run outside the subprocess deadline enforcement. Coalesce by stable thread identity, retain the necessary earliest polling timestamp, and impose an explicit retained-state size/count and retry lifetime/backoff policy with a reported fallback to the sweep. Add a multi-window regression proving the state stays bounded when failing threads keep changing.

2. P2, blocks merge — the new safety regression suite is not run by CI. tests/poll-notifications.test.js:1 (integration point: tests/bot-supervisor.sh:4): CI executes tests/*.sh, and no shell wrapper invokes this new JS file. Thus the mixed-failure, request/page-bound and backlog-fairness tests can regress while required checks remain green. Include this file in an existing suitable wrapper or add a nonexecutable shell wrapper following repository convention; no workflow edit is needed.

Verified fixes / sound: reviewed head 9587842 against fetched origin/main, including the public PR and both prior review comments. Separating acquisition from retries fixes the previously reported large-backlog starvation: the 403/429/503 later-page/later-window regressions pass. Terminal 404/410/451 failures no longer suppress verified news; transient work survives, and per-batch requests, pages, subprocess bytes and network time are capped. The description still summarizes the original feature rather than itemizing rework; these conclusions come from code and tests. No workflow or Rust changes, no changed permissions/action pins/job dependencies, and no weakening of sandbox/check/apply gates found. New API operations are reads; arguments are not shell-expanded; bot identity, operator API author and open/tracked eligibility gate wakes. Bodies/titles are not authority or commands. State uses the existing lock.

Exact checks (all exit 0): cargo fmt --check (0.15s); cargo clippy --workspace --all-targets -- -D warnings (13.71s); cargo test --workspace (30.59s, 65 tests); node --test tests/poll-notifications.test.js tests/bot-supervisor.test.js (21.46s, 50 tests); bash tests/bot-poll-loop.sh (35.29s); shellcheck -x bin/bot-poll-loop bin/poll-notifications.sh tests/bot-poll-loop.sh (0.81s). Offline retained-retry reproduction: node /home/runner-sandbox/review-notifications.cjs (0.31s, exit 0; assertions confirm the issue).

Limits: authenticated live GitHub polling and the full repository shell suite were not run. The commit-review skill is unavailable on this runner. Working tree unchanged.

Generated-by: https://github.com/cgwalters/#llms

[cgwalters-bot]

Review by an LLM (run 37971232167), requested by the coordinator. Not a human review.

VERDICT: APPROVE — safe to merge as is; no merge blockers found. LLM-generated review by gpt-6.1-sol (praxis/gpt-6.1-sol).

Reviewed checkout ba5a2d5 against fetched origin/main, and read the public PR and prior reviews. The description still references 9587842; conclusions below are from the current code, not that description.

Findings, nonblocking follow-ups:

1. P3 — stale failure documentation: docs/scheduled-dispatcher.md:80 and bin/bot-poll-loop:104 say failed reads retain the polling window. Current code advances acquisition independently and retains transient threads with their own timestamps; retries expire after 24 hours and are capped at 200/1 MiB with a sweep-fallback warning. Update these statements and the PR's now-obsolete claim that retry age is unlimited.
2. P3 — missing negative URL/identity coverage: tests/poll-notifications.test.js:28–29 and tests/bot-poll-loop.sh:394–419 primarily exercise Issue notification shapes. Add data-driven PullRequest review/comment cases, rejected pagination/subject/comment URLs, and failed or mismatched bot identity. The present URL restrictions and identity checks are sound on inspection; these are regression-coverage improvements, not demonstrated bypasses.

Verified fixes / sound: transient retries are separate from acquisition; later-page/later-window failure-backlog tests pass. Terminal 404/410/451 reads do not suppress verified news. Stable subject-URL coalescing preserves the earliest timestamp, retry age and exponential backoff; count, retained-byte and lifetime limits address the previous unbounded-backlog finding. Oversized legacy state is checked before parsing. Request/page/subprocess-output/time budgets and resumable pagination remain in place. tests/bot-supervisor.sh:4 now runs the notification suite through CI's existing tests/*.sh discovery. No workflow or Rust changes, permissions/token/action-pin/job-dependency changes, or sandbox/check/apply weakening. Reads use argument arrays, not shell expansion; bot identity, operator API author, and open/tracked eligibility gate wakes. Notification titles and comment bodies confer no authority. The existing lock protects state, pending actions bypass the notification budget, and the sweep remains the fallback. No new unauthorized-write or nonoperator-spend trigger found.

Exact checks, all exit 0: cargo fmt --check (0.146s); cargo clippy --workspace --all-targets -- -D warnings (15.926s); cargo test --workspace (29.940s, 65 tests); bash tests/bot-supervisor.sh (22.522s, 55 tests including the notification regressions); bash tests/bot-poll-loop.sh (35.747s); shellcheck -x bin/bot-poll-loop bin/poll-notifications.sh tests/bot-poll-loop.sh (0.810s); git diff --check origin/main...HEAD passed.

Limits: no authenticated live GitHub polling or full repository shell suite; cannot verify production notification delivery/token scopes on this credential-free runner. The commit-review skill was unavailable. Working tree left unchanged.

Generated-by: https://github.com/cgwalters/#llms