notify slack and triage labels#2211
Conversation
|
👋 @ofek1weiss |
📝 WalkthroughWalkthroughBoth GitHub Actions workflows are refactored to improve security and implementation approach. The notify Slack workflow adds explicit permissions, refactors input handling, and reimplements Slack notification logic using shell commands and Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/notify_slack.yml:
- Line 43: The workflow always constructs RUN_URL using inputs.run_id but the
input is optional, causing broken links when omitted; either mark the input as
required in the notify_slack.yml inputs block (set inputs.run_id required: true)
or change the RUN_URL assignment to guarded/default behavior so it only builds
when inputs.run_id exists (fallback to github.run_id or omit the URL) — update
the RUN_URL environment entry and/or the inputs.run_id declaration accordingly
to ensure a valid run link is produced.
In @.github/workflows/triage-labels.yml:
- Around line 27-32: The removeLabel call (github.rest.issues.removeLabel) can
throw a 404 if the "Awaiting Response" label is already gone; wrap the await
github.rest.issues.removeLabel({ owner, repo, issue_number, name: "Awaiting
Response" }) in a try/catch and silently ignore errors where err.status === 404
(or the Not Found message), but rethrow all other errors so genuine failures
still surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 380c55ce-881b-4a2b-9a1a-2be82030c88b
📒 Files selected for processing (2)
.github/workflows/notify_slack.yml.github/workflows/triage-labels.yml
| } | ||
| SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
| WORKFLOW_NAME: ${{ inputs.workflow_name }} | ||
| RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ inputs.run_id }} |
There was a problem hiding this comment.
run_id is treated as required at runtime but optional in inputs.
Line 43 always builds RUN_URL from inputs.run_id. If callers omit it, Slack gets a broken run link. Either make run_id required or add explicit fallback/guard logic.
Proposed fix (make input contract explicit)
on:
workflow_call:
inputs:
run_id:
type: string
- required: false
+ required: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/notify_slack.yml at line 43, The workflow always
constructs RUN_URL using inputs.run_id but the input is optional, causing broken
links when omitted; either mark the input as required in the notify_slack.yml
inputs block (set inputs.run_id required: true) or change the RUN_URL assignment
to guarded/default behavior so it only builds when inputs.run_id exists
(fallback to github.run_id or omit the URL) — update the RUN_URL environment
entry and/or the inputs.run_id declaration accordingly to ensure a valid run
link is produced.
| await github.rest.issues.removeLabel({ | ||
| owner, | ||
| repo, | ||
| issue_number, | ||
| name: "Awaiting Response", | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub REST API: For issues.removeLabel, what status/error is returned when the target label is not present on the issue?
💡 Result:
The GitHub REST API endpoint for removing a label from an issue (DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}, corresponding to issues.removeLabel in client libraries like Octokit) returns a 404 Not Found status code when the target label is not present on the issue.
Citations:
- 1: https://docs.github.com/rest/issues/labels
- 2: https://github.apidog.io/api-3489030
- 3: https://docs.github.com/v3/issues/labels
- 4: https://docs.github.com/en/rest/issues/labels?apiVersion=2026-03-10
- 5: Remove a label from an issue: Clarify 404 status code github/docs#35553
- 6: https://docs.github.com/en/rest/issues/labels
Make label removal idempotent to prevent flaky failures.
The removeLabel call will fail with a 404 if "Awaiting Response" was already removed by another workflow run or manual edit, causing the job to fail despite the desired end state being achieved. Wrap the call in a try/catch to ignore 404 errors.
Proposed patch
await github.rest.issues.addLabels({
owner,
repo,
issue_number,
labels: ["Triage 👀"],
});
- await github.rest.issues.removeLabel({
- owner,
- repo,
- issue_number,
- name: "Awaiting Response",
- });
+ try {
+ await github.rest.issues.removeLabel({
+ owner,
+ repo,
+ issue_number,
+ name: "Awaiting Response",
+ });
+ } catch (error) {
+ if (error.status !== 404) throw error;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await github.rest.issues.removeLabel({ | |
| owner, | |
| repo, | |
| issue_number, | |
| name: "Awaiting Response", | |
| }); | |
| await github.rest.issues.addLabels({ | |
| owner, | |
| repo, | |
| issue_number, | |
| labels: ["Triage 👀"], | |
| }); | |
| try { | |
| await github.rest.issues.removeLabel({ | |
| owner, | |
| repo, | |
| issue_number, | |
| name: "Awaiting Response", | |
| }); | |
| } catch (error) { | |
| if (error.status !== 404) throw error; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/triage-labels.yml around lines 27 - 32, The removeLabel
call (github.rest.issues.removeLabel) can throw a 404 if the "Awaiting Response"
label is already gone; wrap the await github.rest.issues.removeLabel({ owner,
repo, issue_number, name: "Awaiting Response" }) in a try/catch and silently
ignore errors where err.status === 404 (or the Not Found message), but rethrow
all other errors so genuine failures still surface.
Summary by CodeRabbit