CI: only run slash-command dispatch job for real slash commands

[arash77]

FOR CONTRIBUTOR:

• I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
• Use of AI
  • The contribution is mostly AI generated
  • The contribution has been assisted by AI
• License permits unrestricted use (educational + commercial)
• This PR adds a new tool or tool collection
• This PR updates an existing tool or tool collection
• This PR does something else (explain below)

---

CI-only change. Refs bgruening/galaxytools#1944 (item 1). Split from #8281 at @bernt-matthias' request.

Main effect: stop allocating a runner for every issue comment. on: issue_comment fires on every comment on every issue and PR in this repo, and each one previously started a runner to execute an action that immediately exits. The job is now gated on the command name appearing in the comment body. That is a strict superset of what slash-command-dispatch acts on (it requires the command at the start of the first line), so dispatch behaviour, the reaction, and the permission-denied path are all unchanged.

Also in this PR:

• The repository_owner check moves from the step to the job, so forks stop allocating a runner for a job that can never dispatch.
• The dispatch step is guarded on a non-empty PAT, so Missing required input 'token' cannot fail the run. secrets is not available in an if condition, but env is, and secrets is available in a job-level env — that mapping is the bridge, so no shell step is needed.
• The job now declares permissions: issues: write. Only the 👀 reaction uses GITHUB_TOKEN (via reaction-token → reactions.createForIssueComment); the permission check, pull lookup and dispatch all authenticate with the PAT. Note that the tempting permissions: {} would silently drop the reaction.

Scope correction from the earlier revision of this PR: the PAT guard does not fix the copy-of-this-CI case in bgruening/galaxytools. A copy of this file carries an owner check that skips the whole job before the guard is ever reached. What the guard actually covers is PAT loss or rotation within galaxyproject-owned repos. Note it only tests for a non-empty value, so an expired or revoked PAT still reaches the action and still fails.

Where a PAT is set and someone types the command, nothing changes.

.github/** is in pr.yaml's paths-ignore, so this PR does not trigger the tool CI. actionlint is clean. zizmor now reports only the pre-existing unpinned-uses on the @v5 tag, which is the convention in this repo (cf. b0bb570); the excessive-permissions finding it previously reported on this file is resolved by the permissions block.

[bernt-matthias]

Can you split the fixes for the two problems in separate PRs?

We should definitely use only one method to find the PR. Good point.

A direct push has no associated PR,

IMO commits should only be allowed to be integrated via PRs. I think we enforce this at IUC. But yes .. maybe we should not restrict how people like to work .. or?

omits open PRs whose head commit is reachable from the fork's default branch

If this is a bug in the actions, then I would prefer to fix those .. or at least to report it there .. ?

I also do not understand how this is related to "Push to the default branch aborts linting".

The push branch is kept deliberately:

I do not understand this part.

Labels are fetched at run time rather than read

Yes. This sounds right and as intended to me.

a re-run replays the original payload,

This contradicts the first sentence of the statement, or?

act, with the two steps lifted programmatically out of pr.yaml so the harness cannot drift:

I do not understand this sentence.

In summary. I'm fine with the changes wrt the slash command.

For the other part I definitely agree that it would be better to use only one method to get the PR. Also we likely need to implement this on our own .. due to the node20 problem. But I'm not convinced yet with the analysis.

[arash77]

Split done. This PR is now slash.yaml only. The pr.yaml change moved to #8281.

I was wrong about one thing, and you were right to doubt it.

I claimed the old lookup silently dropped the skip labels. It does not. sharesight/find-github-pull-request@v1.3.0 picks the method based on the event (source):

if (context.eventName === 'pull_request') {
  pullRequest = await fetchPRByNumber();
} else if (context.eventName === 'push' || context.eventName === 'workflow_run') {
  pullRequest = await fetchPRBySha();
}

So on pull_request it already looks up by number. The bug I measured is in the other action, 8BitJonny/gh-get-current-pr, and that one only runs on push, where by-SHA is the only choice anyway. So there is nothing to report upstream, and no labels are being lost today. I have removed the claim from #8281 and from the commit message.

That also answers your question about how it relates to "Push to the default branch aborts linting". It does not. Without that claim, #8281 is only:

1. failIfNotFound: true kills the lint job when a push has no PR (item 2)
2. both Node 20 actions are gone from lint (item 3), which is the "implement it ourselves" you suggested
3. four template-injection findings go away, because ${{ env.LABELS }} no longer goes into a run: block

I can also convert deploy-report off gh-get-current-pr if you want.

About the push branch. It is not about direct pushes. Those now just give no labels and run full checks instead of failing. It is there because a push to main after a merge does have a PR, and that PR's labels still matter: deploy needs lint, so a tool merged with skip-url-check would fail lint on main and block the ToolShed upload. The old allowClosed: true did the same thing. So your PR-only policy does not conflict with it. It just means the "no PR" case never happens.

About the re-run sentence. Sorry, that was two ideas in one sentence. Old and new code both call the API, so nothing changed there. The re-run point was my reason for not switching to github.event.pull_request.labels, which would be cheaper. pull_request does not fire on labeled, and a re-run replays the old payload, so the labels would be stale in the exact case they are for: check fails, you add skip-url-check, you hit re-run.

About the act sentence. .github/** is in paths-ignore, so these PRs do not run the workflow they edit. I ran the steps under act instead. "Lifted programmatically" just means a script reads the steps out of pr.yaml, so what I tested cannot drift from what gets merged. Results are in #8281.

Thanks for checking the reasoning and not just the diff.

---

Update: force-pushed a rewrite of slash.yaml, so please re-check.

• Main change now: the job no longer starts a runner for every issue comment, only for ones containing the command.
• My reason for the PAT guard was wrong. A copy of this file has its own repository_owner check that skips the job first, so it never helped galaxytools. It only covers a missing PAT in galaxyproject repos.
• Dropped the shell step. secrets works in a job-level env, and env works in if.
• Added permissions: issues: write. permissions: {} would break the 👀 reaction.

One question: with a missing PAT the run is now green with no feedback, instead of a red X. Prefer it to fail loudly? Then the guard goes away entirely.