ci: honest greeting + split PR/issue response jobs#3272
ci: honest greeting + split PR/issue response jobs#3272
Conversation
Refactor GitHub Actions workflow to improve PR and issue responses with Copilot integration.
|
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with |
There was a problem hiding this comment.
Pull request overview
This PR refactors the .github/workflows/immediate-response.yaml GitHub Actions workflow to split PR vs issue automation and add Copilot Models–powered issue triage to generate more actionable, template-aware follow-up comments.
Changes:
- Split the prior combined responder into
respond-to-pr(PR greeting) andtriage-issue(issue triage + response). - Updated PR greeting message to set expectations around Copilot + human triage.
- Added issue triage logic (template completeness checks, duplicate search, release context) and a Copilot Models API call to produce concise follow-up questions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Should work without it, but doesn't hurt to include it Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
austenstone
left a comment
There was a problem hiding this comment.
Left two comments: one on missing [MAINT] handling in the issue-type parsing, and one small fix on the bot name (github-actions[bot]).
…THUB_TOKEN env - Fix `githubactions[bot]` → `github-actions[bot]` in both jobs (austenstone) - Add `[MAINT]` to issue type detection and cleanTitle regex (austenstone) - Add `env: GITHUB_TOKEN` to github-script step for Models API auth (deiga)
- Detect GitHub's `_No response_` placeholder as a non-answer - Change greeting to "A human maintainer will review this" (drop "from the GitHub SDK team")
There was a problem hiding this comment.
I want to hold off on this PR. I view this as a cool and clever use case for what Copilot can do, but I do not want to experiment on this repository with unproven Copilot uses. This repository is already under-staffed, and potential improvements like this come with a real risk of increasing workload instead of reducing it. For now, let's implement Copilot only when it has proven its value. For example, I view CCR as a proven value.
When coming back to this PR, some non-exhaustive thoughts:
- I don't think this organization has Copilot enabled. That needs to be resolved before implementing this or CCR.
- The logic to review issues should move to a new file, and actions should call that file. That will make it easier to test and to review.
- The model chosen,
gpt-40-mini, should be a variable
|
@robert-crandall I hear the concern about experimenting on an under-staffed repo, but I'd push back a bit on the framing here. The current workflow is actively unhelpful: it promises "reviews twice a week, every Monday and Friday" which hasn't been true in a long time, and gives zero triage value. A maintainer's first touch on every issue is spent asking for basic info that the template already requested. With 40-50 new issues a month, that adds up fast. This isn't an aggressive use of Copilot. It's template validation, duplicate search, and targeted follow-up questions. I simulated it against real issues from this repo, and in every case it asked for exactly what maintainers end up asking manually, sometimes hours later. I'm happy to share those simulations. I'll monitor it closely after merge and we can roll back immediately if it creates noise instead of reducing it. But waiting for "proven value" on a workflow that replaces something already broken feels like the wrong bar. Re: the specific suggestions: extracting the logic into a separate file and making the model a variable are good improvements, but I'd treat those as follow-ups rather than blockers. The workflow works as-is, and for this use case |
stevehipwell
left a comment
There was a problem hiding this comment.
At a high level I agree that the current workflow doesn't really fit but I'm not sure about the value of adding more signal (we already have too much signal). Wouldn't a better MVP solution be to fix the current behaviour? How about an initial PR to remove this workflow and add a separate workflow for issues and PRs just adding a correct comment in the vein of the original. I'm confident that this would be non-contentious.
After there are separate MVP workflows you could re-add these suggestions as 2 seperate PRs which would be easier to review.
The `pull_request_target` trigger always runs in the context of the base repo, so `github.repository` is always `integrations/terraform-provider-github` for PRs opened against this repo. For issues, the workflow only fires on the repo's own issue tracker. The check adds no value in either case. Addresses review feedback from @stevehipwell.
|
@eyalgal Could you split this into multiple PRs with the first being least changes to make the "immediate-response" workflow useful again? |
Reverts the Copilot-powered triage additions to land them in a follow-up PR, per #3272 review discussion. This PR now only: - Replaces the misleading "reviews twice a week" greeting with an honest best-effort note. - Splits the single respond job into respond-to-pr and respond-to-issue, each scoped to its event with least-privilege permissions.
|
@deiga — splitting it up. Revised plan after thinking it through: This PR (now rescoped) — Fix the greeting + split PR/issue jobs. One small PR because the two changes share a rationale ("make the workflow useful + stop promising a cadence we don't have"), and once the messages diverge the job split is the natural shape. Concretely:
I've just force-pushed this branch to that scope (net diff: one file, +53/-14). Please take another look. Follow-up PR — Copilot-powered issue triage (the real thing). I'd originally planned a deterministic-only version as an intermediate step, but I think that's the wrong move: a rules-only triage comment (missing-field checklist + duplicate list) would look like the final product while lacking the two things that make it feel smart — asking the one specific follow-up that matters, and staying quiet when the issue is already well-described. Shipping that as an intermediate state risks proving the "noise instead of signal" concern right. So the follow-up will be the full thing, with the concerns from this thread baked in:
Does this sequencing work? |
Replace the misleading "reviews twice a week, every Monday and Friday" greeting with an honest note, and split the single
respondjob into event-scoped jobs. The Copilot-powered issue triage that was previously in this PR is deferred to a follow-up, per discussion in this thread.Before the change?
respondjob posts one greeting on both issues and PRs.After the change?
Honest greeting. The comment body no longer promises a cadence. It states that the repo is maintained by GitHub and community members on a best-effort basis, and keeps the existing
Status: Up for grabscommunity pointer.Split jobs.
respondbecomes two jobs:respond-to-pr— triggered onpull_request_target, requests onlypull-requests: write.respond-to-issue— triggered onissues, requests onlyissues: write.Each job has a tailored opener ("thank you for this contribution" vs. "thank you for opening this issue") and the same honest body.
No new dependencies, no new external calls, no template parsing. The Copilot-powered triage (template completeness checks, duplicate search, Models API analysis) will come in a follow-up PR with the concerns from this thread addressed (extracted script, model id as env var, repo-variable kill switch, quiet-on-complete-issues behavior).
Pull request checklist
Does this introduce a breaking change?