Skip to content

feat: add Slack CLI command runner workflow and installer composite action #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ewanek1 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add Slack CLI command runner workflow and installer composite action #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .github/resources/.actions/slack-cli-installer/action.yml
View file
Open in desktop
Copy link
Member

@zimeg zimeg Aug 7, 2025

Choose a reason for hiding this comment

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

👁️‍🗨️ suggestion: We might want to move these steps to action.yml as part of the compositisation of this action!

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Slack CLI Installation
description: Download and cache the Slack CLI
runs:
using: composite
steps:
- name: Cache Slack CLI
id: cache-cli
uses: actions/cache@v4
with:
path: ~/.slack/bin
key: slack-cli-${{ runner.os }}-${{ runner.arch }}-${{ env.SLACK_CLI_VERSION }}
Copy link
Contributor

@WilliamBergamin WilliamBergamin Aug 7, 2025

Choose a reason for hiding this comment

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

Nice 💯


- name: Add Slack CLI to PATH (Linux/macOS)
if: runner.os != 'Windows'
Copy link
Contributor

@WilliamBergamin WilliamBergamin Aug 7, 2025

Choose a reason for hiding this comment

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

Why do we opt to use runner.os != 'Windows' instead of runner.os == 'Linux' || runner.os == 'macOS' used in Install Slack CLI (Linux/macOS)?

Could we potentially merge these steps?

Copy link
Author

@ewanek1 ewanek1 Aug 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Yes! I can use a testing matrix and those tests run when the workflow runs. And then set those tests to run on a pull request event.
  2. I agree, I would change to a more robust parsing library like yargs and put some defaults in. There's some useful CLI flags I think make sense to automatically set to true. Thanks!
  3. Hm I used that since it was more concise. Although I believe it's better to explicitly mention which OS we support. A user could variant for their runner which may not support bash and this conditional would error. I don't think I can merge these though.

shell: bash
run: echo "$HOME/.slack/bin" >> "$GITHUB_PATH"

- name: Add Slack CLI to PATH (Windows)
if: runner.os == 'Windows'
shell: pwsh
run: Add-Content -Path $env:GITHUB_PATH -Value "$env:USERPROFILE\.slack\bin"

- name: Install Slack CLI (Linux/macOS)
if:
(runner.os == 'Linux' || runner.os == 'macOS') && steps.cache-cli.outputs.cache-hit != 'true'
shell: bash
run:
curl -fsSL https://downloads.slack-edge.com/slack-cli/install.sh | bash -s -- -v $SLACK_CLI_VERSION

- name: Install Slack CLI (Windows)
if: runner.os == 'Windows' && steps.cache-cli.outputs.cache-hit != 'true'
shell: pwsh
run: |
irm https://downloads.slack-edge.com/slack-cli/install-windows-dev.ps1 -OutFile 'install-windows-dev.ps1'
.\install-windows-dev.ps1 -Version $env:SLACK_CLI_VERSION
43 changes: 43 additions & 0 deletions .github/workflows/slack-cli-command.yml
View file
Open in desktop
Copy link
Member

@zimeg zimeg Aug 7, 2025

Choose a reason for hiding this comment

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

🧪 thought: It might be best to include this technique as part of existing tests workflow?

I'm hoping with a composite action setup we could perhaps test the CLI version command returns a recent version to start. Perhaps too we start running these tests across various os setups to improve coverage for all techniques?

Copy link
Author

@ewanek1 ewanek1 Aug 8, 2025

Choose a reason for hiding this comment

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

Ah yes definitely more testing on the next iteration 😊

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
name: Slack CLI Command Runner

on:
workflow_dispatch:
inputs:
command:
description: 'Slack CLI command to run'
required: true

jobs:
deploy:
name: "Run Slack CLI command"
runs-on: ubuntu-latest
env:
SLACK_CLI_VERSION: "3.5.2"
timeout-minutes: 10

defaults:
run:
shell: bash

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install Slack CLI
uses: ./.github/resources/.actions/slack-cli-installer

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 18

- name: Install dependencies
run: npm install shell-quote
Copy link
Contributor

@WilliamBergamin WilliamBergamin Aug 7, 2025

Choose a reason for hiding this comment

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

Based on our discussion and the way we are using this dependency, it may end up being removed, but if we do keep it then we should pin it to a specific version and find a way to easily update it. Dependabot might be useful for this


- name: Execute command via script
run: |
echo "Running command: ${{ github.event.inputs.command }}"
node src/parse-command.js ${{ github.event.inputs.command }}
env:
SLACK_SERVICE_TOKEN: ${{ secrets.SLACK_SERVICE_TOKEN }}
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
Comment on lines +12 to +43

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 5 months ago

To fix the problem, add a permissions block to the workflow or the specific job to explicitly set the minimal required permissions for the GITHUB_TOKEN. In this case, since the job only checks out code and runs scripts, it only needs contents: read permission. The best practice is to add the following block at the job level (under deploy:), but it can also be added at the root of the workflow if all jobs require the same permissions. No changes to functionality are required, and no additional imports or definitions are needed.

Suggested changeset 1
.github/workflows/slack-cli-command.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/slack-cli-command.yml b/.github/workflows/slack-cli-command.yml
--- a/.github/workflows/slack-cli-command.yml
+++ b/.github/workflows/slack-cli-command.yml
@@ -12,2 +12,4 @@
     name: "Run Slack CLI command"
+    permissions:
+      contents: read
     runs-on: ubuntu-latest
EOF
@@ -12,2 +12,4 @@
name: "Run Slack CLI command"
permissions:
contents: read
runs-on: ubuntu-latest
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Contributor

@WilliamBergamin WilliamBergamin Aug 7, 2025

Choose a reason for hiding this comment

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

In the case of slack run will the CLI override the environment variable set by the developer?

Copy link
Author

@ewanek1 ewanek1 Aug 8, 2025

Choose a reason for hiding this comment

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

Based on the docs, tokens stored in GitHub secrets or set in the workflow file takes precedence over variables stored in configuration files.

30 changes: 30 additions & 0 deletions src/parse-command.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const { parse: _parse } = require('shell-quote');
const { execFile } = require('child_process');

async function parse(rawInput) {
let tokens;
try {
tokens = _parse(rawInput);
} catch (err) {
throw Error("Invalid syntax: command contains at least one NULL character");
}

if (tokens.length === 0) {
throw Error("No command provided");
}

console.log('Executing the command:', tokens.join(' '));
Copy link
Contributor

@WilliamBergamin WilliamBergamin Aug 7, 2025

Choose a reason for hiding this comment

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

In a follow up PR it could be nice to have this script follow the same logging level approach as it does with the --verbose flag

const args = tokens.slice(1);

execFile('slack', args, (error, stdout) => {
if (error) {
throw Error(`Slack CLI Error: ${error.message}`);
}
console.log(stdout);
Copy link
Contributor

@WilliamBergamin WilliamBergamin Aug 7, 2025

Choose a reason for hiding this comment

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

Should we log stdout before we through the error?

What happens if the error does not have a message field?

Copy link
Author

@ewanek1 ewanek1 Aug 8, 2025

Choose a reason for hiding this comment

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

  1. Ah it seems throwing will exit the current function and the logging might not get reached. I'll make sure to log then throw!
  2. 'By default, the message property is an empty string,' so I think we're good.

});
}

if (require.main === module) {
const rawInput = process.argv.slice(2).join(' ');
parse(rawInput);
}
Loading