refactor: PR lifecycle bots#2363
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates multiple contributor/PR lifecycle scheduling bots into a single GitHub Actions workflow + Node script, aiming to reduce drift/bugs and make thresholds/skip-labels easier to configure while ensuring PRs are only auto-closed after human review.
Changes:
- Replaces three scheduled workflows and their scripts with a unified
bot-contributor-lifecycleworkflow +bot-contributor-lifecycle.js. - Adds Jest unit tests and a dedicated workflow to run them on PRs/pushes touching the bot code.
- Updates fork-testing documentation to describe configuring thresholds via workflow inputs instead of editing scripts.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/sdk_developers/training/testing_forks.md | Updates guidance for testing the new unified lifecycle bot via workflow inputs. |
| .github/workflows/test-bot-contributor-lifecycle.yml | Adds CI workflow to run Jest unit tests for the lifecycle bot. |
| .github/workflows/cron-reminder-pr-inactive.yml | Removes legacy PR inactivity reminder workflow. |
| .github/workflows/cron-reminder-issue-no-pr.yml | Removes legacy “issue assigned but no PR” reminder workflow. |
| .github/workflows/bot-inactivity-unassign.yml | Removes legacy inactivity unassign/close workflow. |
| .github/workflows/bot-contributor-lifecycle.yml | Adds unified scheduled + manually-triggerable lifecycle bot workflow. |
| .github/scripts/test-bot-inactivity-unassign.sh | Removes legacy bash test harness for the old bot. |
| .github/scripts/cron-reminder-pr-inactive.js | Removes legacy PR inactivity reminder script. |
| .github/scripts/cron-reminder-issue-no-pr.sh | Removes legacy issue reminder script. |
| .github/scripts/bot-inactivity-unassign.sh | Removes legacy inactivity unassign/close bash bot implementation. |
| .github/scripts/bot-contributor-lifecycle.js | Adds the consolidated lifecycle bot implementation (issues + linked PR logic). |
| .github/scripts/tests/bot-contributor-lifecycle.test.js | Adds unit tests validating reminder/unassign/close behavior and edge cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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:
WalkthroughThis PR replaces three contributor automation scripts/workflows with a single JavaScript lifecycle bot, adds a scheduled/manual workflow and a matching test workflow, and updates the training docs for manual runs. ChangesContributor Lifecycle Bot Consolidation
Sequence Diagram(s)sequenceDiagram
participant bot_contributor_lifecycle_yml
participant actions_github_script
participant bot_contributor_lifecycle_js
participant github_rest_graphql_apis
bot_contributor_lifecycle_yml->>actions_github_script: pass thresholds, skip_pr_labels, DRY_RUN
actions_github_script->>bot_contributor_lifecycle_js: require(...) and invoke({ github, context })
bot_contributor_lifecycle_js->>github_rest_graphql_apis: list issues, comments, reviews, commits, and linkage
github_rest_graphql_apis-->>bot_contributor_lifecycle_js: issue and PR data
bot_contributor_lifecycle_js-->>actions_github_script: logs, counters, and failures
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d240c32c-8ec1-4332-8fe0-0b698f5986c9
📒 Files selected for processing (12)
.github/scripts/__tests__/bot-contributor-lifecycle.test.js.github/scripts/bot-contributor-lifecycle.js.github/scripts/bot-inactivity-unassign.sh.github/scripts/cron-reminder-issue-no-pr.sh.github/scripts/cron-reminder-pr-inactive.js.github/scripts/test-bot-inactivity-unassign.sh.github/workflows/bot-contributor-lifecycle.yml.github/workflows/bot-inactivity-unassign.yml.github/workflows/cron-reminder-issue-no-pr.yml.github/workflows/cron-reminder-pr-inactive.yml.github/workflows/test-bot-contributor-lifecycle.ymldocs/sdk_developers/training/testing_forks.md
💤 Files with no reviewable changes (7)
- .github/workflows/bot-inactivity-unassign.yml
- .github/workflows/cron-reminder-pr-inactive.yml
- .github/scripts/bot-inactivity-unassign.sh
- .github/workflows/cron-reminder-issue-no-pr.yml
- .github/scripts/test-bot-inactivity-unassign.sh
- .github/scripts/cron-reminder-pr-inactive.js
- .github/scripts/cron-reminder-issue-no-pr.sh
d49373c to
a109278
Compare
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: aa9ea07b-0f4f-499e-8ca4-7686260f8240
📒 Files selected for processing (12)
.github/scripts/__tests__/bot-contributor-lifecycle.test.js.github/scripts/bot-contributor-lifecycle.js.github/scripts/bot-inactivity-unassign.sh.github/scripts/cron-reminder-issue-no-pr.sh.github/scripts/cron-reminder-pr-inactive.js.github/scripts/test-bot-inactivity-unassign.sh.github/workflows/bot-contributor-lifecycle.yml.github/workflows/bot-inactivity-unassign.yml.github/workflows/cron-reminder-issue-no-pr.yml.github/workflows/cron-reminder-pr-inactive.yml.github/workflows/test-bot-contributor-lifecycle.ymldocs/sdk_developers/training/testing_forks.md
💤 Files with no reviewable changes (7)
- .github/workflows/cron-reminder-pr-inactive.yml
- .github/workflows/cron-reminder-issue-no-pr.yml
- .github/scripts/cron-reminder-issue-no-pr.sh
- .github/workflows/bot-inactivity-unassign.yml
- .github/scripts/cron-reminder-pr-inactive.js
- .github/scripts/test-bot-inactivity-unassign.sh
- .github/scripts/bot-inactivity-unassign.sh
aceppaluni
left a comment
There was a problem hiding this comment.
The issue-level try/catch seems to mean a failure while processing one stale PR aborts processing of the remaining linked PRs and any subsequent un-assignments. Should we consider documenting this and per-PR error handling if we ever want best-effort processing?
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
f124e51 to
538908e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec09a0cd-d24c-45c6-9acd-001d5c5e5525
📒 Files selected for processing (12)
.github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js.github/scripts/bot-contributor-lifecycle.js.github/scripts/bot-inactivity-unassign.sh.github/scripts/cron-reminder-issue-no-pr.sh.github/scripts/cron-reminder-pr-inactive.js.github/scripts/test-bot-inactivity-unassign.sh.github/workflows/bot-contributor-lifecycle.yml.github/workflows/bot-inactivity-unassign.yml.github/workflows/cron-reminder-issue-no-pr.yml.github/workflows/cron-reminder-pr-inactive.yml.github/workflows/test-bot-contributor-lifecycle.ymldocs/sdk_developers/training/testing_forks.md
💤 Files with no reviewable changes (7)
- .github/workflows/cron-reminder-pr-inactive.yml
- .github/scripts/test-bot-inactivity-unassign.sh
- .github/scripts/bot-inactivity-unassign.sh
- .github/workflows/cron-reminder-issue-no-pr.yml
- .github/scripts/cron-reminder-pr-inactive.js
- .github/workflows/bot-inactivity-unassign.yml
- .github/scripts/cron-reminder-issue-no-pr.sh
| function specNoPR(number, assignees, assignedDaysAgo, extraComments = []) { | ||
| const logins = arr(assignees); | ||
| return { | ||
| issues: [{ number, assignees: logins.map((l) => ({ login: l })) }], | ||
| commentsByIssue: { [number]: extraComments }, | ||
| graphqlByIssue: { | ||
| [number]: gqlIssue({ | ||
| assignedEvents: logins.map((l) => ({ createdAt: daysAgo(assignedDaysAgo), assignee: { login: l } })), | ||
| }), | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover mixed-age assignees in the no-PR fixtures.
specNoPR() only models one assignedDaysAgo for everyone, so the multi-assignee tests never exercise the branch where assignees fall into different lifecycle buckets. That leaves the per-assignee age handling from the new lifecycle logic effectively untested.
Example fixture tweak
function specNoPR(number, assignees, assignedDaysAgo, extraComments = []) {
const logins = arr(assignees);
+ const ageByLogin =
+ typeof assignedDaysAgo === "number"
+ ? Object.fromEntries(logins.map((login) => [login, assignedDaysAgo]))
+ : assignedDaysAgo;
return {
issues: [{ number, assignees: logins.map((l) => ({ login: l })) }],
commentsByIssue: { [number]: extraComments },
graphqlByIssue: {
[number]: gqlIssue({
- assignedEvents: logins.map((l) => ({ createdAt: daysAgo(assignedDaysAgo), assignee: { login: l } })),
+ assignedEvents: logins.map((l) => ({ createdAt: daysAgo(ageByLogin[l]), assignee: { login: l } })),
}),
},
};
}Then add a case like { alice: 30, bob: 10 } and assert unassign+remind split behavior.
📝 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.
| function specNoPR(number, assignees, assignedDaysAgo, extraComments = []) { | |
| const logins = arr(assignees); | |
| return { | |
| issues: [{ number, assignees: logins.map((l) => ({ login: l })) }], | |
| commentsByIssue: { [number]: extraComments }, | |
| graphqlByIssue: { | |
| [number]: gqlIssue({ | |
| assignedEvents: logins.map((l) => ({ createdAt: daysAgo(assignedDaysAgo), assignee: { login: l } })), | |
| }), | |
| }, | |
| }; | |
| } | |
| function specNoPR(number, assignees, assignedDaysAgo, extraComments = []) { | |
| const logins = arr(assignees); | |
| const ageByLogin = | |
| typeof assignedDaysAgo === "number" | |
| ? Object.fromEntries(logins.map((login) => [login, assignedDaysAgo])) | |
| : assignedDaysAgo; | |
| return { | |
| issues: [{ number, assignees: logins.map((l) => ({ login: l })) }], | |
| commentsByIssue: { [number]: extraComments }, | |
| graphqlByIssue: { | |
| [number]: gqlIssue({ | |
| assignedEvents: logins.map((l) => ({ createdAt: daysAgo(ageByLogin[l]), assignee: { login: l } })), | |
| }), | |
| }, | |
| }; | |
| } |
| for (const { prNum, ageDays } of toClose) { | ||
| let prComments = []; | ||
| try { | ||
| prComments = await listIssueComments(github, owner, repo, prNum); | ||
| } catch (err) { | ||
| console.log(` [WARN] PR #${prNum}: could not list comments (close note may duplicate):`, err.message || err); | ||
| } | ||
| console.log(` [RESULT] PR #${prNum} stale (>= ${cfg.prCloseDays}d) -> close${willUnassign ? " + unassign" : " (keep assignees)"}`); | ||
| if (!hasMarker(prComments, MARK_CLOSE)) { | ||
| await postComment(github, owner, repo, prNum, prCloseBody(assignees, ageDays, willUnassign), cfg); | ||
| } | ||
| await closePR(github, owner, repo, prNum, cfg); | ||
| stats.closed++; | ||
| } | ||
|
|
||
| // Pass 2 — remind on inactive (but not yet closeable) PRs. | ||
| for (const { prNum, author, ageDays } of toRemind) { | ||
| let prComments; | ||
| try { | ||
| prComments = await listIssueComments(github, owner, repo, prNum); | ||
| } catch (err) { | ||
| console.log(` [SKIP] PR #${prNum}: could not list comments (avoid duplicate):`, err.message || err); | ||
| continue; | ||
| } | ||
| if (hasMarker(prComments, MARK_PR_REMIND)) { | ||
| console.log(` [SKIP] PR #${prNum} already has an inactivity reminder`); | ||
| continue; | ||
| } | ||
| console.log(` [RESULT] PR #${prNum} inactive (>= ${cfg.prRemindDays}d) -> remind`); | ||
| await postComment(github, owner, repo, prNum, prReminderBody(author || assignees[0], ageDays), cfg); | ||
| stats.prReminders++; | ||
| } | ||
|
|
||
| // Unassign every assignee once, only when warranted. | ||
| if (willUnassign) { | ||
| for (const assignee of assignees) { | ||
| await removeAssignee(github, owner, repo, issue.number, assignee, cfg); | ||
| stats.unassigned++; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Isolate PR-stage mutations per PR and assignee.
A failure in postComment, closePR, or one removeAssignee aborts the rest of this issue's linked-PR handling because the only surrounding catch is at the outer issue loop. On multi-PR or multi-assignee issues, one API/permission error leaves later stale PRs untouched and later assignees still assigned. Mirror the per-assignee isolation you already use in handleNoPRStage and increment stats.errors per failed PR/assignee so the run still completes the rest of the issue.
As per path instructions, ".github/scripts/**/*.js: Wrap async operations in try/catch with contextual errors" and "Handle permission failures gracefully."
Source: Path instructions
| jobs: | ||
| lifecycle: | ||
| runs-on: hl-sdk-py-lin-md |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a hosted-runner fallback for fork executions.
The updated training doc tells contributors to run this workflow from their personal forks, but this job always targets hl-sdk-py-lin-md. Forks will not have that self-hosted label, so the workflow never starts there. Use the same fork-safe fallback pattern as the test workflow, or the docs stay inaccurate and this workflow is not fork-safe.
Suggested fix
- runs-on: hl-sdk-py-lin-md
+ runs-on: ${{ github.repository_owner == 'hiero-ledger' && 'hl-sdk-py-lin-md' || 'ubuntu-latest' }}As per path instructions, ".github/workflows/**/*: Workflows must behave safely when executed from forks."
📝 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.
| jobs: | |
| lifecycle: | |
| runs-on: hl-sdk-py-lin-md | |
| jobs: | |
| lifecycle: | |
| runs-on: ${{ github.repository_owner == 'hiero-ledger' && 'hl-sdk-py-lin-md' || 'ubuntu-latest' }} |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 58-58: label "hl-sdk-py-lin-md" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 zizmor (1.26.1)
[info] 57-57: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
Source: Path instructions
| on: | ||
| push: | ||
| paths: | ||
| - ".github/scripts/bot-contributor-lifecycle.js" | ||
| - ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js" | ||
| - ".github/scripts/jest.config.js" | ||
| - ".github/scripts/package.json" | ||
| - ".github/scripts/package-lock.json" | ||
| - ".github/workflows/test-bot-contributor-lifecycle.yml" | ||
| pull_request: | ||
| paths: | ||
| - ".github/scripts/bot-contributor-lifecycle.js" | ||
| - ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js" | ||
| - ".github/scripts/jest.config.js" | ||
| - ".github/scripts/package.json" | ||
| - ".github/scripts/package-lock.json" | ||
| - ".github/workflows/test-bot-contributor-lifecycle.yml" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trigger this suite when the production workflow changes.
.github/workflows/bot-contributor-lifecycle.yml:73-80 is the only place that wires DRY_RUN, ISSUE_*, PR_*, and SKIP_PR_LABELS into the script, and the Jest harness depends on those exact names. Because these paths filters exclude the production workflow file, a wiring-only regression there can merge without this suite running.
Suggested fix
push:
paths:
+ - ".github/workflows/bot-contributor-lifecycle.yml"
- ".github/scripts/bot-contributor-lifecycle.js"
- ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js"
- ".github/scripts/jest.config.js"
- ".github/scripts/package.json"
- ".github/scripts/package-lock.json"
- ".github/workflows/test-bot-contributor-lifecycle.yml"
pull_request:
paths:
+ - ".github/workflows/bot-contributor-lifecycle.yml"
- ".github/scripts/bot-contributor-lifecycle.js"
- ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js"
- ".github/scripts/jest.config.js"
- ".github/scripts/package.json"
- ".github/scripts/package-lock.json"
- ".github/workflows/test-bot-contributor-lifecycle.yml"📝 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.
| on: | |
| push: | |
| paths: | |
| - ".github/scripts/bot-contributor-lifecycle.js" | |
| - ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js" | |
| - ".github/scripts/jest.config.js" | |
| - ".github/scripts/package.json" | |
| - ".github/scripts/package-lock.json" | |
| - ".github/workflows/test-bot-contributor-lifecycle.yml" | |
| pull_request: | |
| paths: | |
| - ".github/scripts/bot-contributor-lifecycle.js" | |
| - ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js" | |
| - ".github/scripts/jest.config.js" | |
| - ".github/scripts/package.json" | |
| - ".github/scripts/package-lock.json" | |
| - ".github/workflows/test-bot-contributor-lifecycle.yml" | |
| on: | |
| push: | |
| paths: | |
| - ".github/workflows/bot-contributor-lifecycle.yml" | |
| - ".github/scripts/bot-contributor-lifecycle.js" | |
| - ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js" | |
| - ".github/scripts/jest.config.js" | |
| - ".github/scripts/package.json" | |
| - ".github/scripts/package-lock.json" | |
| - ".github/workflows/test-bot-contributor-lifecycle.yml" | |
| pull_request: | |
| paths: | |
| - ".github/workflows/bot-contributor-lifecycle.yml" | |
| - ".github/scripts/bot-contributor-lifecycle.js" | |
| - ".github/scripts/__tests__/jest/bot-contributor-lifecycle.test.js" | |
| - ".github/scripts/jest.config.js" | |
| - ".github/scripts/package.json" | |
| - ".github/scripts/package-lock.json" | |
| - ".github/workflows/test-bot-contributor-lifecycle.yml" |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 3-19: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
Description:
Replaces three scheduling bots tackling contributor lifecycle with one github action.
Ensures configuration options are easy to change.
Changes auto close PR logic so it only auto closes after a review, and the stale time is met. It doesn't close on unreviewed PRs.
It also doesn't auto close on certain labels, which are now easier to handle (the label was changed previiously which is why it broke)
Bug fixes relating to pr linkage detection, jq over parsing and multiple assignees.
Keeps the new file as an isolated module, for now
Related issue(s):
Fixes #2362
Checklist