Skip to content

feat: add repo check to re-enable disabled workflows - #712

Merged
feanil merged 4 commits into
openedx:masterfrom
salman2013:salman/repo-check
Jun 12, 2026
Merged

feanil merged 4 commits into
openedx:masterfrom
salman2013:salman/repo-check

Conversation

@salman2013

@salman2013 salman2013 commented May 26, 2026 •

Copy link
Copy Markdown
Contributor

Description:

GitHub can automatically disable workflows (e.g. cron jobs on inactive forked repos). The new EnsureWorkflowsEnabled check detects all non-active workflows in a repo and re-enables them.

See: https://github.com/orgs/community/discussions/59547
Ticket: #596

How to test

Details can be found here https://github.com/openedx/repo-tools/blob/master/edx_repo_tools/repo_checks/README.rst

Testing results

  • Testing disabled workflow list output

command

uv run --extra repo_checks repo_checks --dry-run -c EnsureWorkflowsEnabled -r forum

Output on terminal

Screenshot 2026-06-01 at 12 51 16 PM

Verification of output on github

Screenshot 2026-06-01 at 12 51 05 PM
  • Testing to re-enable workflow

command

uv run --extra repo_checks repo_checks --no-dry-run -c EnsureWorkflowsEnabled -r forum
Screenshot 2026-06-01 at 12 52 42 PM

Verify that all workflows have been re-enabled

Screenshot 2026-06-01 at 12 53 07 PM Screenshot 2026-06-01 at 12 52 31 PM

GitHub can automatically disable workflows (e.g. cron jobs on inactive
forked repos). The new EnsureWorkflowsEnabled check detects all
non-active workflows in a repo and re-enables them.

See: https://github.com/orgs/community/discussions/59547

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels May 26, 2026
@openedx-webhooks

openedx-webhooks commented May 26, 2026 •

Copy link
Copy Markdown

Thanks for the pull request, @salman2013!

This repository is currently maintained by @openedx/axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions May 26, 2026
@salman2013 salman2013 closed this May 26, 2026
@salman2013 salman2013 reopened this May 26, 2026
@github-project-automation github-project-automation Bot moved this from Needs Triage to Done in Contributions May 26, 2026
@salman2013
salman2013 marked this pull request as ready for review May 26, 2026 12:00
@salman2013 salman2013 closed this May 26, 2026
@salman2013 salman2013 reopened this May 26, 2026
- Replace all_paged_items with a direct API call since list_repo_workflows
  returns {"total_count": N, "workflows": [...]} not a plain list, causing
  paged() to loop indefinitely
- Print all workflow names alongside the "All workflows are enabled" message

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new repository check to detect disabled GitHub Actions workflows and re-enable them automatically (helping recover from GitHub auto-disabling workflows due to inactivity, etc.).

Changes:

  • Introduces EnsureWorkflowsEnabled check to list workflows and flag any non-active ones.
  • Implements fix() to call the GitHub API to re-enable workflows found disabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread edx_repo_tools/repo_checks/repo_checks.py Outdated
Comment thread edx_repo_tools/repo_checks/repo_checks.py Outdated
Comment thread edx_repo_tools/repo_checks/repo_checks.py Outdated
Comment thread edx_repo_tools/repo_checks/repo_checks.py

@irfanuddinahmad irfanuddinahmad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent Review — Issues Copilot Missed

I also reviewed Copilot's four inline comments. All are valid:

  • State filter (comments 1 & 2): confirmed — state != "active" is too broad and will hit deleted/disabled_manually.
  • Pagination (comment 3): confirmed — only the first page is fetched; the repo already has all_paged_items() at line 42 that should be used here.
  • Duplicate api.repos.get() calls (comment 4): valid, though this is a pre-existing pattern shared by other checks. Worth a follow-up issue rather than blocking this PR.

Beyond those, a few more things caught my eye:


1. No tests added

The PR adds significant new behaviour but no test coverage. tests/test_repo_checks.py already shows exactly how to mock GhApi with MagicMock. At minimum I'd expect:

  • check() returns (False, …) when disabled workflows exist
  • check() returns (True, …) when all workflows are active
  • fix() calls api.actions.enable_workflow() for each disabled workflow
  • dry_run() does not call api.actions.enable_workflow()
  • is_relevant() returns False for security forks and empty repos

2. disabled_manually policy should be explicit

The PR description says the goal is to re-enable automatically disabled workflows, but state != "active" silently catches disabled_manually too. Re-enabling a manually-disabled workflow overrides an intentional admin decision. Even if the team decides to include that state, the docstring and the filter should make it explicit.


3. No error handling in fix() — partial re-enable risk

If enable_workflow() raises for any workflow (e.g. an unexpected state slips through the filter), the loop aborts and the repo is left in a partially re-enabled state with no indication of which workflows succeeded. Either fix the state filter defensively (which resolves this too) or wrap the call in a try/except that logs the failure and continues to the next workflow.


4. Success message lists every workflow name (minor)

When all workflows are enabled, check() returns every workflow name. For repos with many workflows this produces very verbose output. A count-based summary (e.g. "All 14 workflows are enabled") would be more consistent with other checks.

- Filter only disabled_inactivity and disabled_fork states to avoid
  re-enabling manually disabled workflows (intentional admin decisions)
- Add per_page=100 to avoid missing workflows on large repos
- Show count-based success message; include manually disabled count only
  when non-zero
- Wrap enable_workflow() in try/except to continue on failure
- Add tests covering check(), fix(), dry_run(), and is_relevant()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@feanil feanil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion otherwise, looks good to me.

)

def check(self) -> tuple[bool, str]:
response = self.api.actions.list_repo_workflows(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use all_paged_items so we don't miss anything. It's very unlikely that we'll have more than a 100 workflows but not impossible. We should write the code to be resilient, especially since the helper already exists.

False,
f"Some workflows are disabled:\n\t\t" + "\n\t\t".join(names),
)
manually_disabled = [w for w in response.workflows if w.state == "disabled_manually"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract disabled_manually into a name string so we can add comments near it if needed.

- Use all_paged_items-compatible pagination via a wrapper function that
  extracts .workflows from list_repo_workflows response, since the
  endpoint returns {total_count, workflows} rather than a flat list
- Extract DISABLED_MANUALLY_STATE class constant with explanatory comment
- Fix enabled count in message: report "X of Y enabled (Z manually
  disabled)" instead of incorrectly showing total as enabled count
- Update make_workflows_api mock to use side_effect for correct
  pagination simulation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@feanil
feanil merged commit e269ac6 into openedx:master Jun 12, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants