Skip to content

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Sep 16, 2025

Fixes #7012

Proposes:

  • removing all uses of long-lived GitHub API token in repo secrets, in favor of the short-lived one GitHub Actions provisions automatically (GitHub docs)
  • switching comment-triggered workflows (e.g. /gha run r-valgrind) with using workflow dispatch (manually clicking "run workflow" in the GitHub UI)
  • switches get-workflow-status.py from Python to a shell scripts using the gh CLI
    • this PR needed to change it anyway, because the logic's currently based on reading all comments from a PR and now the workflows aren't triggered by comments

And some other small improvements to the r-configure and r-valgrind workflows.

  • fixes git config for the r-configure job, so commits are tied to the official GitHub bot and not https://github.com/GitHubActionsBot
  • reduces the timeout on the "optional checks" workflow from 120 minutes to 30 minutes
    • this usually takes less than 15 seconds to run

Notes for Reviewers

Benefits of these changes

  • reduces dependency on admins manually updating any secrets
  • allows us to remove the long-lived secret with an API token in this repo (reducing the impact of accidental credential leaks)
  • simplifies CI (allows removing the triggering_comments.yml workflow)
  • makes it much easier to test changes to these workflows
    • you can trigger the workflow right on the branch with the changes, no need to merge to master first!

How I tested this

Temporarily added on.push: blocks to the r-configure and r-valgrind workflows, to trigger them by just pushing commits here.

We have to merge this to test workflow dispatch

To trigger these workflows with workflow dispatch, we'll have to merge this to master.

But once we do, we'll be able to test workflow changes on another PR from a branch on this repo, because GitHub's UI lets you choose the branch for workflow dispatch.

Screenshot 2025-09-20 at 12 59 31 AM

After this PR

We can remove the repo secret WORKFLOW and not need to ever refresh GitHub credentials again 😁

@jameslamb jameslamb changed the title WIP: [ci] use GItHub built-in token (fixes #7012) WIP: [ci] use GitHub built-in token (fixes #7012) Sep 17, 2025
@microsoft microsoft deleted a comment from github-actions bot Sep 17, 2025
Copy link

Workflow R valgrind tests has been triggered! 🚀

https://github.com/microsoft/LightGBM/actions/runs/17786041154

Status: success ✔️

@microsoft microsoft deleted a comment from github-actions bot Sep 17, 2025
Copy link

Workflow R valgrind tests has been triggered! 🚀

https://github.com/microsoft/LightGBM/actions/runs/17786462804

Status: success ✔️

Copy link

Workflow R valgrind tests has been triggered! 🚀

https://github.com/microsoft/LightGBM/actions/runs/17817251491

Status: success ✔️

@jameslamb jameslamb force-pushed the fix/comment-triggered-jobs branch from 98c2763 to 6c681d6 Compare September 20, 2025 04:31
Copy link

Workflow R valgrind tests has been triggered! 🚀

https://github.com/microsoft/LightGBM/actions/runs/17875748943

Status: success ✔️

@jameslamb jameslamb changed the title WIP: [ci] use GitHub built-in token (fixes #7012) WIP: [ci] use GitHub built-in token, switch comment-triggered jobs to workflow dispatch (fixes #7012) Sep 20, 2025
type: string
description: Pull request ID, found in the PR URL.

permissions:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used a few commits to find the minimal set of permissions needed for this workflow to run... I believe this is a tightly-scoped as is possible.

docs: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions

description: |
branch the PR was submitted from.
Branches from forks should be prefixed with the user/org they originate from,
like '{user}:{branch}'.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Proposing we merge this PR to master to test how this works with forks, but I think this is right based on what I see at https://github.com/microsoft/LightGBM/actions/workflows/r_package.yml

Screenshot 2025-09-20 at 1 09 09 AM

echo "Checking status of workflow run '${LATEST_RUN_ID}'"
gh run view \
--repo "microsoft/LightGBM" \
--exit-status \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This ensures a non-0 exit code if the last run was not successful.

@jameslamb jameslamb changed the title WIP: [ci] use GitHub built-in token, switch comment-triggered jobs to workflow dispatch (fixes #7012) [ci] use GitHub built-in token, switch comment-triggered jobs to workflow dispatch (fixes #7012) Sep 20, 2025
@jameslamb jameslamb marked this pull request as ready for review September 20, 2025 06:20
@StrikerRUS
Copy link
Collaborator

@jameslamb Could you please resolve merge conflicts?

@jameslamb
Copy link
Collaborator Author

Resolved conflicts, I think this is ready for review.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ci] commented-triggered workflows broken: access forbidden for long-lived tokens

2 participants