Skip to content

suppression: PR-history pagination truncates silently, so cooldowns can vanish with degraded: [] #110

Description

@debuggingfuture

listPullRequests stops at maxPages with no signal, and applies headBranchPrefix after pagination. On a repo with more PRs than the cap inside the cooldown window, the suppression primitive sees an empty history, applies no cooldowns, and reports degraded: [] — the one outcome the module's header comment promises cannot happen.

Problem

Two behaviors combine into a silent failure:

  • listPullRequests exits the pagination loop at maxPages (default 5 × 100 = 500 PRs) and returns what it collected. Hitting the cap is indistinguishable from reaching the end of the history.
  • The headBranchPrefix filter runs after collection, because GitHub's head= matches one exact branch rather than a prefix. The cap therefore bites on unfiltered rows.

So a control repo with >500 PRs touched in the 30-day window can return zero proposals — every one of them sorted below the cap — while the call looks completely successful.

checkSuppression then gets an empty priorProposals, applies no cooldowns, and returns degraded: []. That directly contradicts what suppression.ts promises in its own header:

If a source cannot be read, its half of the rule is skipped, the reason is logged at warn, and the report names it so the run can say so in its output.

The failure mode is the one the loop's process doc calls its likeliest: not a bad merge, but every previously-closed proposal being re-opened, and the team learning to ignore the loop. The degraded list exists precisely so that can't happen quietly, and this path routes around it.

Note this is not reachable through the ordering/maxPages fix in #91 — that made the fake paginate like the live read, which is what surfaced this. The live read's truncation is still silent on both sides.

Fix

maxPages truncation has to become a value the caller can see, which is a return-shape change through every layer:

  1. listPullRequests returns { prs, truncated } (or equivalent) — truncated: true when the loop exits on the page cap with a full last page, as opposed to a short page or the updatedSince stop.
  2. GithubService.pullRequestHistory carries the flag through packages/core/src/services/github.ts.
  3. packages/runtime-cf/src/github-live.ts maps it; packages/runtime-cf/src/deferred.ts supplies its degraded default.
  4. makeGithubFake in packages/core/src/fakes/github-fake.ts models it — it already walks pages under a cap, so it knows when it truncated.
  5. checkSuppression pushes a degraded entry when the flag is set, so the run says so in its PR body and digest instead of proposing duplicates.

Worth deciding at the same time whether the default cap of 5 pages is right for a control repo, and whether the prefix should narrow the query earlier (e.g. via the search API) so the cap applies to candidate rows rather than all of them.

Test

The fake can already reach the cap via historyPageSize, so this is assertable without seeding hundreds of rows: seed more rows than maxPages × historyPageSize, all matching the prefix, and assert checkSuppression reports a non-empty degraded rather than an empty suppressed.

Found during review of #91; deferred there because it changes a service contract on a branch other PRs were stacked on.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions