Github actions spam

[MichalPetryka]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "9adfbe326c430c32c8129feab5009e60ff592003",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "e5fc9669cc6a67694d46987c753cc13d995b2623",
  "last_reviewed_commit": "",
  "last_reviewed_base_ref": "",
  "last_reviewed_base_sha": "",
  "last_recorded_worker_run_id": "",
  "review_attempt_commit": "9adfbe326c430c32c8129feab5009e60ff592003",
  "review_attempt_base_ref": "main",
  "review_attempt_count": 1,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": []
}

Originally posted by @github-actions[bot] in #130872 (comment)

Some new action keeps spamming this on every single PR it seems, causing extreme github notification flood. I had like 10 pages of notifications with this when I just checked github.

[akoeplinger]

from #130339 /cc @jeffhandley @tannergooding

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

[tannergooding]

I'd defer to Jeff, but as I understand the skill looks to have performed a one time pass over all existing PRs and placed the metadata indicating a review has been done. Potentially it should've had a filter to not already run on existing PRs prior to its own last update to reduce noise or should only be an explicit trigger.

[akoeplinger]

but does that mean we'll get the same message on every new PR too?

[MichalPetryka]

I'd defer to Jeff, but as I understand the skill looks to have performed a one time pass over all existing PRs and placed the metadata indicating a review has been done. Potentially it should've had a filter to not already run on existing PRs prior to its own last update to reduce noise or should only be an explicit trigger.

It should definitely at least not print an useless, unactionable message with some JSON.

Even for the real review comment, it'd ideally only be printed when there's some actionable feedback, instead of just printing a comment with a description of what the PR does.

[tannergooding]

but does that mean we'll get the same message on every new PR too?

This should've primarily just adjusted the existing code-review trigger that was already happening on every PR and whenever a PR was updated, namely ensuring that the one GitHub runs is the same instructions/premise as if you had run it locally in the GitHub CLI or UI App.

It should've also caused a number of improvements, like helping ensure the automatic copilot feedback was no longer happening on drafts.

I'm guessing there's something subtle here that needs to get fixed/improved, but Jeff was the one driving the underlying changes. It looks like it was possibly just missing the filter that would've avoided it running on our existing backlog of 521 PRs until some first change was pushed.

The metadata its placing to prevent itself from rerunning unnecessarily (because you can't store state) could probably be hidden as part of its review instead of a separate comment.

[akoeplinger]

The metadata its placing to prevent itself from rerunning unnecessarily (because you can't store state) could probably be hidden as part of its review instead of a separate comment.

+1

[jeffhandley]

I'm working on noise reduction that moves the state tracking comment off the PRs themselves an onto a single/centralized issue in the repo for tracking the PR review state for all PRs.

To note, I had explored and ruled out all of these options for managing that state:

1. repo-storage: creates an orphan branch in the repo and requires the workflow have contents: write permission
2. PR description edits: state likely to get clobbered with authors rewriting PR descriptions
3. cache-storage: can't rely on actions/cache for durability
4. PR review: orchestration challenges, as the state needs to be managed by the orchestrator workflow but the review is posted by the worker workflow. The state needs to be captured before dispatching the worker to prevent concurrent runs and for error handling
5. Artifacts: not intended for cross-workflow-run concurrency, and prototypes found lots of reliability/concurrency issues
6. External/side-repo: requires a custom PAT

I'm closing this to be handled as part of #131031.