Skip to content

feat(gemini): A GitHub Action that identifies stale branches based on inactivity and notifies their owners via PR comments or new issues.#4262

Open
polsala wants to merge 1 commit intomainfrom
ai/gemini-20260419-0850
Open

feat(gemini): A GitHub Action that identifies stale branches based on inactivity and notifies their owners via PR comments or new issues.#4262
polsala wants to merge 1 commit intomainfrom
ai/gemini-20260419-0850

Conversation

@polsala
Copy link
Copy Markdown
Owner

@polsala polsala commented Apr 19, 2026

Implementation Summary

  • Utility: nightly-branch-rot-monitor
  • Provider: gemini
  • Location: github-actions/nightly-nightly-branch-rot-monitor
  • Files Created: 5
  • Description: A GitHub Action that identifies stale branches based on inactivity and notifies their owners via PR comments or new issues.

Rationale

  • Automated proposal from the Gemini generator delivering a fresh community utility.
  • This utility was generated using the gemini AI provider.

Why safe to merge

  • Utility is isolated to github-actions/nightly-nightly-branch-rot-monitor.
  • README + tests ship together (see folder contents).
  • No secrets or credentials touched.
  • All changes are additive and self-contained.

Test Plan

  • Follow the instructions in the generated README at github-actions/nightly-nightly-branch-rot-monitor/README.md
  • Run tests located in github-actions/nightly-nightly-branch-rot-monitor/tests/

Links

  • Generated docs and examples committed alongside this change.

Mock Justification

  • Not applicable; generator did not introduce new mocks.

… inactivity and notifies their owners via PR comments or new issues.
@polsala
Copy link
Copy Markdown
Owner Author

polsala commented Apr 19, 2026

🤖 Review by GROQ Agent

✅ What's solid

  • Clear separation of concerns – the action lives under its own folder (github‑actions/nightly‑nightly‑branch‑rot-monitor) and does not touch any other codebase.
  • Inputs & outputs are declared in action.yml and consumed via @actions/core, making the action composable in any workflow.
  • Graceful error handling – the try/catch wrapper calls core.setFailed on any exception, ensuring the workflow fails visibly.
  • Loggingcore.info statements give useful runtime context (e.g., which branches are being skipped, the stale‑days cutoff).
  • Deterministic staleness calculation – the cutoff date is derived from the current date once at the start of the run, avoiding subtle off‑by‑one bugs when iterating.
  • Test coverage – the test suite exercises the main paths (no stale branches, stale with PR, stale without PR, multiple stale branches, API error, protected‑branch skipping).

🧪 Tests

  • Mocking strategy is solidoctokit.rest methods (listBranches, getCommit, listPulls, createIssue, createComment) are all stubbed, keeping the tests fast and deterministic.
  • Date control – the test suite sets a fixed “now” date (via jest.useFakeTimers or a manual mock) before each test, ensuring the stale‑day calculation is repeatable.
  • Actionable suggestions
    • Add a test for the stale‑days input validation (e.g., non‑numeric or negative values). This will protect against accidental mis‑configuration.
    • Verify output type – assert that core.setOutput receives a string (GitHub Actions coerces numbers to strings). Example:
      expect(core.setOutput).toHaveBeenCalledWith('stale-branches-count', expect.any(String));
    • Edge‑case branch without a commit SHA – simulate a branch object missing commit.sha and ensure the action skips it gracefully rather than throwing.
    • Test pagination – the current implementation requests per_page: 100. Add a unit test that returns >100 branches and verifies the action would handle pagination (or explicitly documents the limitation).

🔒 Security

  • Token scope – the repo-token input is required and passed directly to github.getOctokit. The README correctly warns about the needed permissions (contents: read, pull‑requests: write, issues: write).
  • Potential over‑privilege – the default GITHUB_TOKEN in a workflow has read/write access to the repository, which is sufficient. If a custom PAT is used, advise users to restrict it to the minimal scopes (repopublic_repo/repo:status as appropriate).
  • Input sanitisationstale-days is parsed with parseInt. Add a guard to reject NaN or values ≤ 0, e.g.:
    if (isNaN(staleDays) || staleDays <= 0) {
      core.setFailed('`stale-days` must be a positive integer');
      return;
    }
  • Branch name handling – branch names are interpolated only into API calls (no shell commands), so injection risk is low. Still, consider trimming whitespace to avoid accidental malformed names.
  • Error surface – the catch block forwards the raw error message to core.setFailed. For production use, you might want to mask tokens or sensitive data if they ever appear in error strings (unlikely here, but a good habit).

🧩 Docs / Developer Experience

  • README completeness – the overview, inputs, outputs, and usage example are well‑written.
  • Installation path – the example workflow uses polsala/ApocalypsAI/utils/nightly-branch-rot-monitor@main. Since the action lives under github-actions/..., either rename the repository path or clarify that the action should be referenced as ./github-actions/nightly-nightly-branch-rot-monitor for local actions.
  • Development instructions – the “Development & Testing” section points to cd utils/nightly-branch-rot-monitor, but the actual folder is github-actions/nightly-nightly-branch-rot-monitor. Align the path to avoid confusion.
  • Versioning – consider adding a version input or tagging releases, so downstream workflows can pin a stable version instead of tracking @main.
  • Changelog – a short “What’s new” note in the README would help future contributors understand breaking changes.

🧱 Mocks / Fakes

  • Consistent mock naming – the test file uses mockListBranches, mockGetCommit, etc., which mirrors the real Octokit method names. This improves readability.
  • Reuse of mock objects – the suite re‑creates the same mock implementations across many tests. Extract a helper like setupBranchMocks({branches, commits, pulls}) to reduce duplication and make each test’s intent clearer.
  • Avoid leaking mock state – ensure jest.resetAllMocks() (or afterEach) is called so one test’s mock does not affect another. The current file appears to do this, but double‑check.
  • Fake date handling – the comment “Mock rationale: Control current date for deterministic staleness checks in tests.” suggests a manual date injection. Consider exposing a now parameter to run() (defaulting to new Date()) and passing a fixed date from tests; this removes reliance on global timer mocks and makes the production code easier to reason about.

Quick win checklist

  • Add validation for stale-days (positive integer).
  • Align README paths (utils/… → actual folder).
  • Provide a pagination note or implement multi‑page handling.
  • Refactor test mocks into a reusable helper.
  • Document version pinning strategy for downstream users.

Overall, the action is well‑structured, the core logic is sound, and the test suite gives confidence in the primary workflows. The suggestions above will tighten edge‑case handling, improve developer ergonomics, and future‑proof the utility.

@polsala
Copy link
Copy Markdown
Owner Author

polsala commented Apr 19, 2026

🤖 Review by GROQ Agent

✅ What's solid

  • Clear purpose & scope – the action is nicely isolated under github-actions/nightly-nightly-branch-rot-monitor and does one thing (detect stale branches and notify owners).
  • User‑facing documentation – the README explains the problem, the workflow, inputs/outputs, and gives a ready‑to‑copy example.
  • Action metadataaction.yml correctly declares inputs, outputs, and the Node runtime (node16).
  • Core logic – the implementation follows the described steps: list branches, filter protected ones, compare commit dates, then either comment on an existing PR or open an issue.
  • Test coverage – the test suite exercises the main paths (no stale branches, stale with PR, stale without PR, multiple stale branches, error handling, protected‑branch skipping).

🧪 Tests

Observation Recommendation
Tests mock the Octokit methods directly (mockListBranches, mockGetCommit, etc.) and verify the expected calls. Keep the mocks close to the public API (octokit.rest.repos.listBranches, octokit.rest.repos.getCommit, octokit.rest.pulls.list, octokit.rest.issues.createComment, octokit.rest.issues.create). This makes the test less fragile if the internal implementation changes.
The staleness cutoff is calculated from new Date() inside the action, but the tests rely on a hard‑coded “mockDate” comment that never actually overrides Date. Inject the current date (or a “now” function) via an optional input or environment variable. In tests you can then pass a deterministic value, e.g.:
js\nconst now = () => new Date(process.env.TEST_NOW);\nconst cutoff = new Date(now().setDate(now().getDate() - staleDays));\n
core.setOutput, core.info, and core.setFailed are asserted but never mocked/spied on in the test file. Add jest.spyOn(core, 'setOutput') (and the other methods) in a beforeEach block and restore them after each test. This prevents “undefined is not a function” errors if the real @actions/core is imported.
Pagination is limited to per_page: 100. The test suite only uses a handful of branches, so the limitation isn’t exercised. Add a test that returns more than 100 branches and verify that the action uses octokit.paginate (or loops) to fetch all pages. This will surface any hidden bugs before they hit production.
Error handling test only checks that core.setFailed is called. Also assert that the action does not attempt to set any output after a failure, and that the error message is logged (core.error). This makes the failure path clearer.

Sample test improvement (injecting a deterministic date):

// test-setup.js
process.env.TEST_NOW = '2023-10-30T00:00:00Z';
jest.spyOn(Date, 'now').mockImplementation(() => new Date(process.env.TEST_NOW).valueOf());

// In the action (src/index.js)
const now = new Date(Date.now());
const cutoffDate = new Date(now);
cutoffDate.setDate(now.getDate() - staleDays);

🔒 Security

  • Token handling – the action receives repo-token as an input and passes it to github.getOctokit. No token is ever printed to the console, which is good.
  • Least‑privilege recommendation – the README mentions the default GITHUB_TOKEN but does not advise on the minimal required scopes. Consider adding:
    yaml\npermissions:\n contents: read\n issues: write\n pull-requests: write\n
    to the workflow example, so downstream users can lock down the token.
  • Input validationstale-days is parsed with parseInt but there is no guard against non‑numeric or negative values. Add a validation step:
if (isNaN(staleDays) || staleDays <= 0) {
  core.setFailed('`stale-days` must be a positive integer');
  return;
}
  • Rate‑limit awareness – the action makes a separate API call per branch (getCommit) and possibly per branch for PR lookup. For large repos this could hit GitHub rate limits. Consider using the GraphQL API to batch commit dates, or at least add a back‑off/retry strategy.

🧩 Docs / Developer Experience

Area Feedback
README usage path The example workflow uses polsala/ApocalypsAI/utils/nightly-branch-rot-monitor@main, but the actual repository path is github-actions/nightly-nightly-branch-rot-monitor. Update the example to reflect the correct location, e.g.:
yaml\nuses: your-org/your-repo/github-actions/nightly-nightly-branch-rot-monitor@main\n
Installation instructions The “Development & Testing” section tells users to cd utils/nightly-branch-rot-monitor, which again mismatches the folder name. Align the path with the actual github-actions/nightly-nightly-branch-rot-monitor.
Versioning Consider adding a version input (or a release tag) so downstream users can pin a specific version of the action rather than always pulling @main.
Output description The output stale-branches-count is documented, but the README does not show how to reference it (steps.monitor.outputs.stale-branches-count). Add a short snippet in the “Example Usage” section that demonstrates reading the output.
Contribution guidelines Since this is a generated utility, a brief “How to contribute” note (e.g., run npm test, open PRs) would help future maintainers.
License The repository likely already has a top‑level LICENSE, but the action folder does not mention it. Adding a LICENSE badge or a note in the README clarifies reuse rights.

🧱 Mocks / Fakes

  • The test suite creates manual mocks for each Octokit endpoint (mockListBranches, mockGetCommit, etc.). This works but can become cumbersome as the action grows.
  • Recommendation: Use @actions/github’s built‑in jest mock helper or a library like nock to intercept HTTP calls. Example with nock:
nock('https://api.github.com')
  .get('/repos/test-owner/test-repo/branches')
  .reply(200, [{ name: 'stale-feature', commit: { sha: 'abc' } }]);

This keeps the test file concise and mirrors real network interactions.

  • Ensure that all side‑effects (logging, output, failure) are spied on, not just the Octokit calls. This gives a full picture of the action’s observable behavior.

TL;DR Action is functional and well‑documented, but a few refinements will make it more robust, secure, and developer‑friendly:

  1. Validate inputs (stale-days must be a positive integer).
  2. Inject a deterministic “now” for reliable staleness calculations in tests.
  3. Mock core methods and consider using nock/Octokit pagination for more realistic tests.
  4. Add minimal permission hints and a permissions: block in the example workflow.
  5. Fix README path mismatches and show how to consume the output.
  6. Plan for pagination & rate‑limit handling for repos with many branches.

Implementing these suggestions will tighten the action’s reliability, improve its security posture, and smooth the onboarding experience for future contributors. Happy coding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant