fix(labeling): execute canonical label sync in workflow#450
Conversation
|
Warning Review limit reached
More reviews will be available in 38 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
🔍 Reviewer Summary for PR #450CI Status: ✅ Recommendations
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the label synchronization script (label-sync.js) to support running as an executable CLI in CI, and updates the documentation accordingly. The feedback suggests a safer way to parse the repository owner and name from environment variables to prevent potential runtime crashes when github.context.repo is accessed without a properly configured environment.
| const owner = | ||
| process.env.GITHUB_REPOSITORY_OWNER || github.context.repo.owner; | ||
| const repo = | ||
| process.env.GITHUB_REPOSITORY?.split("/")[1] || github.context.repo.repo; |
There was a problem hiding this comment.
Accessing github.context.repo directly can throw an unhandled exception if the GITHUB_REPOSITORY environment variable is missing or malformed (for example, when running or testing the script locally). Since github.context.repo simply parses process.env.GITHUB_REPOSITORY under the hood, we can safely extract the owner and repository directly from the environment variables. This avoids potential runtime crashes and allows our custom error handling on line 487 to gracefully report the missing configuration.
| const owner = | |
| process.env.GITHUB_REPOSITORY_OWNER || github.context.repo.owner; | |
| const repo = | |
| process.env.GITHUB_REPOSITORY?.split("/")[1] || github.context.repo.repo; | |
| const [repoOwner, repoName] = (process.env.GITHUB_REPOSITORY || "").split("/"); | |
| const owner = process.env.GITHUB_REPOSITORY_OWNER || repoOwner; | |
| const repo = repoName; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae96ef2391
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const canonicalLabels = await loadCanonicalLabels(labelsPath); | ||
| const octokit = github.getOctokit(token); | ||
|
|
||
| const syncReport = await syncLabelsWithCanonical( |
There was a problem hiding this comment.
The new CLI path now executes syncLabelsWithCanonical()/validateRepoLabels() in CI, but those helpers only request a single listLabelsForRepo page with per_page: 100; Octokit’s pagination docs note per_page tops out at 100. This repository’s .github/labels.yml already contains 149 canonical labels, so once a repo has the full canonical set the CLI only sees the first 100 existing labels, treats the rest as missing, and the validation step fails every labeling run even though the labels are present. Please fetch all pages via octokit.paginate(...) before validating or syncing.
Useful? React with 👍 / 👎.
🔍 Reviewer Summary for PR #450CI Status: ✅ Recommendations
|
github-actions
Bot
added
area:ci
github-actions
Bot
added
meta:needs-changelog
1 participant
Summary
scripts/agents/includes/label-sync.jsexecutable when run directly in CIGITHUB_TOKENand deterministicDRY_RUNbehaviour into the labeling workflow sync stepWhy
Issue #66 requires canonical label and seeding workflow alignment. The workflow was invoking
node scripts/agents/includes/label-sync.js, but that file had no CLI entrypoint, so sync was effectively a no-op.Changes
label-sync.jsthat:.github/labels.ymlGITHUB_TOKEN.github/workflows/labeling.ymlsync step env:GITHUB_TOKENfrom workflow secretDRY_RUN=trueonpull_requesteventsfalsewhen not provided)docs/ISSUE_LABELS.mdautomation section for the new sync execution contract.Validation
node scripts/validation/validate-labeling-configs.cjsnpx eslint scripts/agents/includes/label-sync.jsnpx markdownlint-cli2 docs/ISSUE_LABELS.mdnpx spectral lint .github/workflows/labeling.yml --ruleset .spectral-workflows.cjsLinks