Skip to content

refactor(ci): converge two diverged PR-comment upserts; ssot-coverage appends duplicates past 30 comments #12984

Open

Description

@mrveiss

Problem

Two workflows independently implement "upsert my report comment on the PR", and
they have already diverged in ways that matter.

ssot-coverage.yml:122-145 ai-security-review.yml
Identifies its comment by visible heading substring invisible HTML marker
Pagination listComments unpaginated github.paginate

Discovered while auditing #12973 — the newer implementation was written without
checking for the existing one (Core Rule 1 / Rule 2). Two implementations of one
concept is the violation; the newer being more correct makes it worse, because
the divergence is a silent upgrade the older caller never receives.

Bug in the existing implementation

ssot-coverage.yml:122 calls listComments without pagination. GitHub returns
30 comments per page by default, so on a PR with more than 30 comments the
existing report falls off page one and the workflow appends a new comment on
every run rather than updating in place.

Its identification is also brittle: it matches the visible heading substring
'SSOT Configuration Compliance', so changing that heading orphans the previous
comment and silently restarts the append behaviour.

Not hypothetical here — long-running PRs in this repo accumulate bot comments
from several workflows.

Proposed fix

Extract a composite action, e.g. .github/actions/upsert-pr-comment, taking a
marker string and a body file, and converge both callers on it. Fixing
ssot-coverage.yml's pagination and marker identification falls out of the
convergence rather than needing a separate patch.

.github/actions/ already hosts free-disk-space and setup-python-ci, so the
pattern is established — nothing new is being invented.

Note the other five workflows that create PR comments (phase_validation.yml,
issue-triage.yml, auto-close-issues.yml, dependabot-retarget.yml,
dependabot-auto-merge.yml) are fire-and-forget createComment calls, not
upserts. Only convert one if it is genuinely meant to update in place —
converting a deliberate append into an upsert would be a behaviour regression.

Acceptance criteria

  • .github/actions/upsert-pr-comment exists, marker-based and paginated
  • ssot-coverage.yml and ai-security-review.yml both use it, with no inline upsert left
  • ssot-coverage.yml no longer appends a duplicate on PRs with >30 comments
  • Fire-and-forget createComment callers are left unchanged unless individually justified
  • actionlint passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions