Create "snack" issue from PR labeled "snack-it". #4304
Conversation
github-actions
bot
added
internal
htahir1
added
snack-it
x-squad
htahir1
added
the
no-release-notes
|
@claude /full-review |
Documentation Link Check Results✅ Absolute links check passed |
|
Claude finished @strickvl's task —— View job Code Review: Snack-it WorkflowI've completed a comprehensive review of the new GitHub Actions workflow. Here's my detailed feedback: SummaryThis workflow creates a "snack" issue from PRs labeled with "snack-it", using Claude AI to generate concise issue descriptions. The issue inherits PR labels, gets added to the ZenML Roadmap project with "In Review" status, and has the PR branch linked to it. Critical Issues1. Duplicate API Key Parameter (.github/workflows/snack-it.yml:47-50)env:
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
with:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}The ANTHROPIC_API_KEY is set both as an environment variable AND as a 2. Regex Parsing Fragility (.github/workflows/snack-it.yml:99-108)const titleMatch = claudeOutput.match(/ISSUE_TITLE:\s*\n(.+?)(?=\n\nISSUE_BODY:)/s);
const bodyMatch = claudeOutput.match(/ISSUE_BODY:\s*\n([\s\S]+?)$/);The regex patterns assume very specific formatting from Claude (exactly two newlines between sections). If Claude adds extra whitespace or formats slightly differently, parsing will fail silently and fall back to using the PR title/body. Consider:
3. Race Condition in Issue Retrieval (.github/workflows/snack-it.yml:165-172)const issues = await github.rest.issues.listForRepo({
owner: context.repo.owner,
repo: context.repo.repo,
labels: 'snack',
sort: 'created',
direction: 'desc',
per_page: 1
});Instead of querying for the most recent "snack" issue, this should use the const issueNumber = ${{ steps.create-issue.outputs.issue_number }};
const issue = await github.rest.issues.get({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber
});
const issueNodeId = '${{ steps.create-issue.outputs.issue_node_id }}';Security Issues4. Token Scope Concern (.github/workflows/snack-it.yml:160)github-token: ${{ secrets.GH_PAT_TOKEN_FOR_SNACK_IT }}Using a PAT is necessary for project operations, but:
5. Command Injection via PR Title (.github/workflows/snack-it.yml:56)The PR title is interpolated directly into the prompt: PR Title: ${{ github.event.pull_request.title }}While this is passed to Claude (not a shell), if the title contains characters that could confuse the YAML parser or prompt format (like backticks, quotes, etc.), it could cause issues. Consider escaping or sanitizing, or better yet, have Claude fetch the PR details directly via Logic & Correctness Issues6. Inconsistent Field Name Matching (.github/workflows/snack-it.yml:226-228)const inReviewOption = statusField?.options.find(opt =>
opt.name === 'In Review' || opt.name === 'In review'
);Checking for both "In Review" and "In review" is good defensive programming, but indicates uncertainty about the actual field name in the project. Consider:
7. Error Handling Creates Silent Partial Success (.github/workflows/snack-it.yml:327-336)The workflow catches errors when adding to the project but continues anyway: } catch (error) {
console.error(...);
// Don't fail the workflow, just log the error
}While this prevents the workflow from failing after successfully creating an issue, it means the issue won't be tracked in the project board. Consider:
8. Branch Linking May Fail Silently (.github/workflows/snack-it.yml:291-323)The branch linking logic attempts to create a linked branch using GraphQL, but if it fails, it only logs a message. The
Consider whether this feature is critical enough to warrant better error handling or whether it should be optional. Code Quality & Best Practices9. Hardcoded Organization and Project Number (.github/workflows/snack-it.yml:185-186)const projectNumber = 1;
const orgName = 'zenml-io';These are hardcoded but should probably be workflow inputs or repository variables to make the workflow more reusable. Consider moving to: env:
PROJECT_NUMBER: 1
ORG_NAME: zenml-io10. Missing Input Validation (.github/workflows/snack-it.yml:86)const claudeOutput = process.env.CLAUDE_OUTPUT || '';There's no validation that Claude actually returned output. If
11. Prompt Could Be More Specific (.github/workflows/snack-it.yml:52-78)The prompt asks Claude to output in a specific format but:
Suggested improvement: prompt: |
Generate a snack issue from PR #${{ github.event.pull_request.number }}.
Steps:
1. Run: gh pr view ${{ github.event.pull_request.number }} --json title,body,author,headRefName,files
2. Run: gh pr diff ${{ github.event.pull_request.number }} (if needed)
3. Analyze what was accomplished
4. Output exactly in this format (no code blocks, no extra text):
ISSUE_TITLE:
A concise, actionable title (max 80 chars)
ISSUE_BODY:
A brief summary (2-3 sentences) explaining what this snack accomplishes.
Key changes:
- Change 1
- Change 2
References PR #${{ github.event.pull_request.number }}
CRITICAL: Output ONLY the above format. No markdown code blocks. No commentary.12. Checkout Depth Optimization (.github/workflows/snack-it.yml:41)fetch-depth: 1Good! Shallow checkout is appropriate here since Claude only needs to view the PR diff, not the full history. 13. Comment Message References Non-existent Feature (.github/workflows/snack-it.yml:147)body: `🍿 Snack issue created: #${issue.data.number}\n\nThis issue has been added to the [ZenML Roadmap](https://github.com/orgs/zenml-io/projects/1) project and set to "In Review".\n\n_Generated with Claude AI ✨_`This message claims the issue was added to the project and set to "In Review", but that happens in the next step which might fail. If step 3 fails, this comment will be misleading. Consider:
What's Done Well✅ Good conditional execution: Using ✅ Comprehensive error logging: Good use of console.log throughout for debugging ✅ Graceful degradation: Fallback to PR title/body if Claude fails ✅ Security: Appropriate use of different tokens for different permission levels ✅ Documentation: Excellent inline documentation explaining the workflow purpose and requirements ✅ Label inheritance: Smart to copy PR labels to the issue Minor Improvements
RecommendationsMust fix before merge:
Should fix before merge:
Nice to have:
|
|
@claude full-review |
|
Claude finished @htahir1's task —— View job Full Review of Snack-it WorkflowSummaryThis workflow automates the creation of "snack" issues from PRs labeled with "snack-it", using Claude AI to generate concise descriptions. The implementation is largely solid with good error handling and graceful degradation. I've identified several improvements (detailed below) that would make it more robust, but none are blocking issues. ✅ What's Working Well
🔴 Critical IssuesNone! All critical issues from my previous review have been addressed.
|
|
🍿 Snack issue created: #4306 This issue has been added to the ZenML Roadmap project and set to "In Review". Generated with Claude AI ✨ |
|
🍿 Snack issue created: #4307 This issue has been added to the ZenML Roadmap project and set to "In Review". Generated with Claude AI ✨ |
|
🍿 Snack issue created: #4308 This issue has been added to the ZenML Roadmap project and set to "In Review". |
|
@strickvl ready for your review |
.github/workflows/snack-it.yml
Outdated
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, labeled, unlabeled] |
There was a problem hiding this comment.
| types: [opened, synchronize, reopened, labeled, unlabeled] | |
| types: [opened, labeled] |
I think maybe I'd restrict it to this, as otherwise the issues will be created every time someone pushes to the branch. I'd even possibly suggest deleting opened as well.
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| // Get PR details (handle both workflow_dispatch and pull_request events) |
There was a problem hiding this comment.
If we include opened, I'd maybe include something like this to ensure we're not creating duplicates? I think this would be the right place?
// Before creating the issue
const owner = context.repo.owner;
const repo = context.repo.repo;
const query = `repo:${owner}/${repo} type:issue label:snack "PR #${pr.number}"`;
const searchResult = await github.rest.search.issuesAndPullRequests({ q: query });
const existing = searchResult.data.items[0];
if (existing) {
console.log(`Existing snack issue #${existing.number} found for PR #${pr.number}, not creating a new one.`);
core.setOutput('issue_number', existing.number);
core.setOutput('issue_node_id', existing.node_id);
core.setOutput('pr_number', pr.number);
return existing.number;
}
There was a problem hiding this comment.
There are a bunch of different ways you can check to see that we only proceed when the snack-it label is added.
.github/workflows/snack-it.yml
Outdated
| - name: Checkout repository | ||
| if: steps.check-label.outputs.snack_it == 'true' | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
| - name: Checkout repository | |
| if: steps.check-label.outputs.snack_it == 'true' | |
| uses: actions/checkout@v5 | |
| with: | |
| fetch-depth: 1 |
I think this isn't used anywhere?
.github/workflows/snack-it.yml
Outdated
| // Get full issue and PR details for later use | ||
| const issue = await github.rest.issues.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: issueNumber | ||
| }); | ||
|
|
||
| const pr = await github.rest.pulls.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: prNumber | ||
| }); | ||
|
|
||
| try { | ||
| // GraphQL query to add item to project | ||
| // Project number from URL: https://github.com/orgs/zenml-io/projects/1 | ||
| const projectNumber = 1; | ||
| const orgName = 'zenml-io'; |
There was a problem hiding this comment.
| // Get full issue and PR details for later use | |
| const issue = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: issueNumber | |
| }); | |
| const pr = await github.rest.pulls.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: prNumber | |
| }); | |
| try { | |
| // GraphQL query to add item to project | |
| // Project number from URL: https://github.com/orgs/zenml-io/projects/1 | |
| const projectNumber = 1; | |
| const orgName = 'zenml-io'; | |
| try { | |
| // GraphQL query to add item to project | |
| // Project number from URL: https://github.com/orgs/zenml-io/projects/1 | |
| const projectNumber = 1; | |
| const orgName = 'zenml-io'; | |
| // Get full issue and PR details for later use | |
| const issue = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: issueNumber | |
| }); | |
| const pr = await github.rest.pulls.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: prNumber | |
| }); |
Formatting of this is probably off, but this will prevent silent failures in case it wasn't able to get the issue or PR (the workflow would otherwise just run through without failing in the outer closure).
strickvl
left a comment
strickvl
left a comment
There was a problem hiding this comment.
I guess it's fine for what it is, but probably the .js script is getting a bit long now so normally I'd say it makes sense for that to live in the scripts/ dir and then we just run / trigger it from here with a node step etc.
|
@strickvl id suggest we merge it in for now it isnt a big use-case |
- Tighten permissions: pull-requests now read-only (was write) - Add label check for workflow_dispatch triggers (was always proceeding) - Add try-catch error handling for manual PR lookup - Guard branch linking for same-repo PRs only (skip for forks) - Remove unused issue fetch variable
|
@strickvl wdyt |
- Bump project fields pagination from 20 to 50 to avoid missing Status field
- Add optional chaining on options lookup to prevent runtime errors
- Add issue_reused output to distinguish new vs existing issues
- Gate PR comment on issue_reused to avoid duplicate comments on re-runs
- Switch to ${{ github.token }} for non-PAT scripts (clearer idiom)
- Add override_label_check input for workflow_dispatch to allow testing
- Clarify header comments about manual trigger behavior
I just fixed directly. It'll be a pain if this isn't working right / if it's too spammy etc. |
Co-authored-by: Alex Strick van Linschoten <stricksubscriptions@fastmail.fm> (cherry picked from commit ba8a0ed)
Describe changes
This PR adds a GitHub Actions workflow that automates issue creation and project management when PRs are labeled with
snack-it.What it does:
snack-itlabel) or manual workflow dispatchsnacklabelsnack-ititself)Security & robustness improvements:
pull-requestsusesreadinstead ofwriteworkflow_dispatchchecks if the PR has thesnack-itlabel${{ github.token }}for non-PAT scriptsIdempotence & noise reduction (latest commit):
issue_reusedoutput to detect existing snack issuesoptionslookup to prevent runtime errorsoverride_label_checkinput allows manual runs on PRs without thesnack-itlabelPre-requisites
Please ensure you have done the following:
developand the open PR is targetingdevelop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes