Update sync-issue-labels-compute.yml#13
Update sync-issue-labels-compute.yml#13manishdait24-bot wants to merge 12 commits intomanishdait:mainfrom
Conversation
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: manishdait24-bot <manishdait24@gmail.com>
Up to standards ✅🟢 Issues
|
Signed-off-by: manishdait24-bot <manishdait24@gmail.com>
github-actions
Bot
added
automated
broken-markdown-links
enhancement
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe GitHub Actions workflow for syncing issue labels is refactored to remove manual dispatch capability, consolidate job structure, add concurrency control, simplify linked-issue parsing, and streamline label computation logic with updated artifact handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c54291ad-47bb-4e57-aa37-915cd4bbd1ab
📒 Files selected for processing (1)
.github/workflows/sync-issue-labels-compute.yml
| script: | | ||
| const MAX_LINKED_ISSUES = Number(process.env.MAX_LINKED_ISSUES || "20"); | ||
|
|
||
| function extractLabels(labelData) { | ||
| const result = []; | ||
| for (const item of labelData) { | ||
| const name = typeof item === "string" ? item : item && item.name; | ||
| if (name && name.trim()) result.push(name.trim()); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| function extractLinkedIssueNumbers(prBody, owner, repo) { | ||
| const numbers = new Set(); | ||
| const closingRefRegex = /(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)\s+(?:([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+))?#(\d+)\b/gi; | ||
| const lines = String(prBody || "").split(/\r?\n/); | ||
| for (const line of lines) { | ||
| let m; | ||
| while ((m = closingRefRegex.exec(line)) !== null) { | ||
| const refOwner = (m[1] || "").toLowerCase(); | ||
| const refRepo = (m[2] || "").toLowerCase(); | ||
| if (refOwner && refRepo && (refOwner !== owner.toLowerCase() || refRepo !== repo.toLowerCase())) continue; | ||
| numbers.add(Number(m[3])); | ||
| } | ||
| } | ||
| const all = Array.from(numbers); | ||
| if (all.length > MAX_LINKED_ISSUES) { | ||
| console.log(`[sync] Limiting linked issue refs from ${all.length} to ${MAX_LINKED_ISSUES}.`); | ||
| } | ||
| return all.slice(0, MAX_LINKED_ISSUES); | ||
| } | ||
|
|
||
| const prNumber = Number(process.env.PR_NUMBER); | ||
| if (!prNumber) { | ||
| core.setOutput('has_labels', 'false'); | ||
| core.setOutput('labels', '[]'); | ||
| core.setOutput('pr_number', ''); | ||
| core.setOutput('dry_run', 'true'); | ||
| core.setOutput('is_fork_pr', String(process.env.IS_FORK_PR || 'false')); | ||
| core.setOutput('source_event', context.eventName); | ||
| return; | ||
| } | ||
| const prNumber = context.payload.pull_request.number; | ||
| console.log(`--- Processing PR #${prNumber} ---`); | ||
|
|
||
| const { data: prData } = await github.rest.pulls.get({ | ||
| owner: context.repo.owner, repo: context.repo.repo, pull_number: prNumber | ||
| }); | ||
|
|
||
| const prAuthor = (prData.user && prData.user.login) || ""; | ||
| const prAuthor = prData.user.login; | ||
| if (/\[bot\]$/i.test(prAuthor) || /dependabot/i.test(prAuthor)) { | ||
| console.log(`[sync] Skipping bot-authored PR from ${prAuthor}.`); | ||
| core.setOutput('has_labels', 'false'); | ||
| core.setOutput('labels', '[]'); | ||
| core.setOutput('pr_number', String(prNumber)); | ||
| core.setOutput('dry_run', 'true'); | ||
| core.setOutput('is_fork_pr', String(process.env.IS_FORK_PR || 'false')); | ||
| core.setOutput('source_event', context.eventName); | ||
| console.log(`Skipping bot-authored PR (Author: ${prAuthor})`); | ||
| return; | ||
| } | ||
|
|
||
| const linkedIssues = extractLinkedIssueNumbers(prData.body || "", context.repo.owner, context.repo.repo); | ||
| if (!linkedIssues.length) { | ||
| console.log("[sync] No linked issue references found in PR body."); | ||
| core.setOutput('has_labels', 'false'); | ||
| core.setOutput('labels', '[]'); | ||
| core.setOutput('pr_number', String(prNumber)); | ||
| core.setOutput('dry_run', 'true'); | ||
| core.setOutput('is_fork_pr', String(process.env.IS_FORK_PR || 'false')); | ||
| core.setOutput('source_event', context.eventName); | ||
| const regex = /(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)[:\s]+\s*#(\d+)\b/gi; | ||
| const issueNumbers = new Set(); | ||
| let match; | ||
| while ((match = regex.exec(prData.body || "")) !== null) { | ||
| issueNumbers.add(Number(match[1])); | ||
| } | ||
|
|
||
| if (issueNumbers.size === 0) { | ||
| console.log("No linked issues found in the PR description."); | ||
| return; | ||
| } | ||
|
|
||
| console.log(`[sync] Linked issues: ${linkedIssues.map(n => '#' + n).join(', ')}`); | ||
| console.log(`Detected linked issues: #${Array.from(issueNumbers).join(', #')}`); | ||
|
|
||
| const allLabels = []; | ||
| for (const num of linkedIssues) { | ||
| const discoveredLabels = new Set(); | ||
| for (const num of issueNumbers) { | ||
| try { | ||
| const { data } = await github.rest.issues.get({ | ||
| const { data: issue } = await github.rest.issues.get({ | ||
| owner: context.repo.owner, repo: context.repo.repo, issue_number: num | ||
| }); | ||
| if (data.pull_request) { console.log(`[sync] Skipping #${num}: is a PR reference.`); continue; } | ||
| const labels = extractLabels(data.labels || []); | ||
| console.log(`[sync] Issue #${num} labels: ${labels.length ? labels.join(', ') : '(none)'}`); | ||
| allLabels.push(...labels); | ||
| } catch (err) { | ||
| if (err && err.status === 404) { console.log(`[sync] Issue #${num} not found. Skipping.`); continue; } | ||
| throw err; | ||
| if (!issue.pull_request) { | ||
| const names = (issue.labels || []).map(l => typeof l === 'string' ? l : l.name); | ||
| console.log(`Found labels on issue #${num}: [${names.join(', ')}]`); | ||
| names.forEach(l => discoveredLabels.add(l)); | ||
| } else { | ||
| console.log(`Skipping #${num} because it is a Pull Request, not an Issue.`); | ||
| } | ||
| } catch (e) { | ||
| console.log(`Error fetching labels for issue #${num}: ${e.message}`); | ||
| } | ||
| } | ||
|
|
||
| const existing = extractLabels(prData.labels || []); | ||
| const existingSet = new Set(existing); | ||
| const deduped = Array.from(new Set(allLabels)); | ||
| const toAdd = deduped.filter(l => !existingSet.has(l)); | ||
|
|
||
| console.log(`[sync] Existing: ${existing.length ? existing.join(', ') : '(none)'}`); | ||
| console.log(`[sync] To add: ${toAdd.length ? toAdd.join(', ') : '(none)'}`); | ||
| const currentLabels = (prData.labels || []).map(l => typeof l === 'string' ? l : l.name); | ||
| console.log(`Current PR labels: [${currentLabels.join(', ')}]`); | ||
|
|
||
| const labels = toAdd; | ||
| const hasLabels = labels.length > 0; | ||
| core.setOutput('has_labels', String(hasLabels)); | ||
| core.setOutput('labels', JSON.stringify(labels)); | ||
| core.setOutput('pr_number', String(prNumber)); | ||
| core.setOutput('dry_run', String(process.env.REQUESTED_DRY_RUN || 'true')); | ||
| core.setOutput('is_fork_pr', String(process.env.IS_FORK_PR || 'false')); | ||
| core.setOutput('source_event', context.eventName); | ||
| return { has_labels: hasLabels, labels, pr_number: String(prNumber), dry_run: process.env.REQUESTED_DRY_RUN, is_fork_pr: process.env.IS_FORK_PR, source_event: context.eventName }; | ||
| const newLabels = Array.from(discoveredLabels).filter(label => !currentLabels.includes(label)); | ||
|
|
||
| if (newLabels.length > 0) { | ||
| console.log(`New labels to be added: [${newLabels.join(', ')}]`); | ||
| } else { | ||
| console.log("No new labels discovered (all found labels are already present)."); | ||
| } | ||
|
|
||
| - name: Write labels artifact payload | ||
| env: | ||
| LABELS_JSON: ${{ steps.compute.outputs.labels }} | ||
| PR_NUMBER: ${{ steps.compute.outputs.pr_number }} | ||
| IS_FORK_PR: ${{ steps.compute.outputs.is_fork_pr }} | ||
| DRY_RUN: ${{ steps.compute.outputs.dry_run }} | ||
| SOURCE_EVENT: ${{ steps.compute.outputs.source_event }} | ||
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 | ||
| with: | ||
| script: | | ||
| const fs = require('fs'); | ||
| const parsed = JSON.parse(process.env.LABELS_JSON || '[]'); | ||
| const payload = { | ||
| pr_number: Number(process.env.PR_NUMBER || 0), | ||
| labels: Array.isArray(parsed) ? parsed : [], | ||
| is_fork_pr: /^true$/i.test(process.env.IS_FORK_PR || ''), | ||
| dry_run: /^true$/i.test(process.env.DRY_RUN || ''), | ||
| source_event: process.env.SOURCE_EVENT || '', | ||
| }; | ||
| fs.writeFileSync('labels.json', JSON.stringify(payload)); | ||
| console.log(`Wrote labels artifact payload for PR #${payload.pr_number}: ${payload.labels.length} labels`); | ||
|
|
||
| - name: Upload labels artifact | ||
| const result = { pr_number: prNumber, labels: newLabels }; | ||
| fs.writeFileSync('labels.json', JSON.stringify(result)); | ||
| console.log(`Calculated labels: ${newLabels.join(', ')}`); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Move the label-computation logic into .github/scripts/.
This block now owns parsing, filtering, API iteration, and artifact generation. Extracting it to a dedicated script will keep the workflow YAML focused on orchestration and make the logic easier to test.
As per coding guidelines, "Non-trivial logic belongs in dedicated scripts under .github/scripts/, keeping the workflow YAML focused on orchestration." and "Flag large run: | blocks, inline loops or conditionals, and API calls embedded directly in YAML steps."
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: manishdait24-bot <manishdait24@gmail.com>
Signed-off-by: Manish Dait <90558243+manishdait@users.noreply.github.com>
manishdait
removed
enhancement
github-actions
Bot
added
automated
broken-markdown-links
enhancement
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/sync-issue-labels-compute.yml (1)
28-88: 🧹 Nitpick | 🔵 TrivialMove the label-computation logic into
.github/scripts/.This large inline script block handles parsing, filtering, API iteration, and artifact generation. Extracting it to a dedicated script would keep the workflow YAML focused on orchestration and make the logic easier to test.
As per coding guidelines, "Non-trivial logic belongs in dedicated scripts under
.github/scripts/, keeping the workflow YAML focused on orchestration."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70577347-eee3-436f-bacb-609d378371c4
📒 Files selected for processing (1)
.github/workflows/sync-issue-labels-compute.yml
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: manishdait24-bot <manishdait24@gmail.com>
Signed-off-by: Manish Dait <90558243+manishdait@users.noreply.github.com>
manishdait
removed
enhancement
Signed-off-by: manishdait24-bot <manishdait24@gmail.com>
github-actions
Bot
added
broken-markdown-links
enhancement
Signed-off-by: Manish Dait <90558243+manishdait@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 03cdea42-97e1-4c6b-b77c-fa3c5cd08929
📒 Files selected for processing (1)
.github/workflows/sync-issue-labels-compute.yml
| name: Sync Linked Issue Labels - Compute | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, edited, reopened, synchronize, ready_for_review] | ||
| workflow_dispatch: | ||
| inputs: | ||
| pr_number: | ||
| description: "PR number to sync labels for" | ||
| required: true | ||
| type: number | ||
| dry-run-enabled: | ||
| description: "Dry run (log only, do not apply labels)" | ||
| required: false | ||
| type: boolean | ||
| default: true | ||
| types: [opened, edited, reopened, synchronize] | ||
|
|
||
| permissions: | ||
| actions: write | ||
| pull-requests: read | ||
| issues: read | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| compute-labels: | ||
| concurrency: | ||
| group: sync-issue-labels-compute-pr-${{ github.event.pull_request.number || github.event.inputs.pr_number }} | ||
| cancel-in-progress: true | ||
| sync: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| pr_number: ${{ steps.compute.outputs.pr_number }} | ||
| dry_run: ${{ steps.compute.outputs.dry_run }} | ||
| is_fork_pr: ${{ steps.compute.outputs.is_fork_pr }} | ||
| steps: | ||
| - name: Harden the runner | ||
| - name: Harden Runner | ||
| uses: step-security/harden-runner@f808768d1510423e83855289c910610ca9b43176 # v2.17.0 | ||
| with: | ||
| egress-policy: audit | ||
|
|
||
| - name: Compute linked issue labels | ||
| - name: Compute Labels |
There was a problem hiding this comment.
This change does not appear to fix issue #7.
Issue #7 is about broken markdown links/Lychee output, but this PR only changes the label-sync workflow. Please split this refactor into a separate PR or include the actual markdown/link-check fixes before closing that issue.
As per coding guidelines, "Focus feedback on the PR's stated scope. Flag out-of-scope issues as irrelevant and suggest opening a separate PR — do not block for them."
| const regex = /(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)[:\s]+\s*#(\d+)\b/gi; | ||
| const issueNumbers = new Set(); | ||
| let match; | ||
| while ((match = regex.exec(prData.body || "")) !== null) { | ||
| issueNumbers.add(Number(match[1])); | ||
| } | ||
|
|
||
| if (issueNumbers.size === 0) { | ||
| console.log("No linked issues found in the PR description."); | ||
| return; | ||
| } | ||
|
|
||
| console.log(`[sync] Linked issues: ${linkedIssues.map(n => '#' + n).join(', ')}`); | ||
| console.log(`Detected linked issues: #${Array.from(issueNumbers).join(', #')}`); | ||
|
|
||
| const allLabels = []; | ||
| for (const num of linkedIssues) { | ||
| const discoveredLabels = new Set(); | ||
| for (const num of issueNumbers) { | ||
| try { | ||
| const { data } = await github.rest.issues.get({ | ||
| const { data: issue } = await github.rest.issues.get({ | ||
| owner: context.repo.owner, repo: context.repo.repo, issue_number: num | ||
| }); | ||
| if (data.pull_request) { console.log(`[sync] Skipping #${num}: is a PR reference.`); continue; } | ||
| const labels = extractLabels(data.labels || []); | ||
| console.log(`[sync] Issue #${num} labels: ${labels.length ? labels.join(', ') : '(none)'}`); | ||
| allLabels.push(...labels); | ||
| } catch (err) { | ||
| if (err && err.status === 404) { console.log(`[sync] Issue #${num} not found. Skipping.`); continue; } | ||
| throw err; | ||
| if (!issue.pull_request) { | ||
| const names = (issue.labels || []).map(l => typeof l === 'string' ? l : l.name); | ||
| console.log(`Found labels on issue #${num}: [${names.join(', ')}]`); | ||
| names.forEach(l => discoveredLabels.add(l)); | ||
| } else { | ||
| console.log(`Skipping #${num} because it is a Pull Request, not an Issue.`); | ||
| } | ||
| } catch (e) { | ||
| console.log(`Error fetching labels for issue #${num}: ${e.message}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "sync-issue-labels-compute.yml" -type fRepository: manishdait/hiero-sdk-python
Length of output: 120
🏁 Script executed:
cat -n ./.github/workflows/sync-issue-labels-compute.ymlRepository: manishdait/hiero-sdk-python
Length of output: 4779
Introduce a hard cap on linked issues before iterating.
prData.body is untrusted input from the GitHub event payload. The current code extracts all fixes #... references from the PR description via regex without any limit, then makes one API call (issues.get) per matched issue number. A PR description with many linked issues (or many edits across edited/synchronize events) can exhaust API quota and waste runner time.
🔧 Suggested change
+ const MAX_LINKED_ISSUES = 20;
const regex = /(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)[:\s]+\s*#(\d+)\b/gi;
const issueNumbers = new Set();
let match;
while ((match = regex.exec(prData.body || "")) !== null) {
issueNumbers.add(Number(match[1]));
+ if (issueNumbers.size > MAX_LINKED_ISSUES) {
+ throw new Error(`Refusing to process more than ${MAX_LINKED_ISSUES} linked issues from the PR body.`);
+ }
}| for (const num of issueNumbers) { | ||
| try { | ||
| const { data } = await github.rest.issues.get({ | ||
| const { data: issue } = await github.rest.issues.get({ | ||
| owner: context.repo.owner, repo: context.repo.repo, issue_number: num | ||
| }); | ||
| if (data.pull_request) { console.log(`[sync] Skipping #${num}: is a PR reference.`); continue; } | ||
| const labels = extractLabels(data.labels || []); | ||
| console.log(`[sync] Issue #${num} labels: ${labels.length ? labels.join(', ') : '(none)'}`); | ||
| allLabels.push(...labels); | ||
| } catch (err) { | ||
| if (err && err.status === 404) { console.log(`[sync] Issue #${num} not found. Skipping.`); continue; } | ||
| throw err; | ||
| if (!issue.pull_request) { | ||
| const names = (issue.labels || []).map(l => typeof l === 'string' ? l : l.name); | ||
| console.log(`Found labels on issue #${num}: [${names.join(', ')}]`); | ||
| names.forEach(l => discoveredLabels.add(l)); | ||
| } else { | ||
| console.log(`Skipping #${num} because it is a Pull Request, not an Issue.`); | ||
| } | ||
| } catch (e) { | ||
| console.log(`Error fetching labels for issue #${num}: ${e.message}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "sync-issue-labels.*\.yml" .github/workflows/Repository: manishdait/hiero-sdk-python
Length of output: 162
🏁 Script executed:
cat -n .github/workflows/sync-issue-labels-compute.ymlRepository: manishdait/hiero-sdk-python
Length of output: 4779
🏁 Script executed:
cat -n .github/workflows/sync-issue-labels-add.ymlRepository: manishdait/hiero-sdk-python
Length of output: 2395
Fail the workflow on transient API errors, not partial results.
The catch block silently logs errors and continues the loop, allowing discoveredLabels to remain incomplete. If any issues are successfully fetched before a transient error (403, 429, 5xx), labels.json is still written with partial data. The downstream sync-issue-labels-add.yml workflow treats a successful run as complete computation and applies that incomplete label set.
Only catch and skip the specific case you intend to handle (404 for deleted issues); re-throw all other errors to fail the job:
Suggested change
} catch (e) {
- console.log(`Error fetching labels for issue #${num}: ${e.message}`);
+ if (e.status === 404) {
+ console.log(`Skipping missing issue #${num}.`);
+ continue;
+ }
+ throw e;
}2 participants
fixes: #7