Skip to content

Set up CI to run the test suite #2

Closed

Description

@zanellig

Problem

The repo has no CI. gh pr checks on #1 reported "no checks reported on the 'feat/match-tsx-servers' branch", so the ship/review workflow's "CI green" step is vacuous — tests/test-cli.sh only runs when someone remembers to run it locally.

This matters more than usual here: the whole tool is regex-driven process matching, where a subtly wrong pattern silently either misses dev servers or, worse, kills unrelated processes. #1 shipped both a false-negative fix and a false-positive fix; neither would have been caught by anything automated.

Proposal

A GitHub Actions workflow on push and pull_request that runs:

  • bash tests/test-cli.sh
  • shellcheck kill-dev-servers.sh install.sh tests/test-cli.sh — the script is nontrivial bash with arrays, associative arrays, and regex; shellcheck earns its keep
  • bash -n syntax check on the same files

Notes

  • The suite stubs ss, lsof, ps, and readlink via PATH, and spawns real background PIDs so the script's /proc lookups see live processes. It needs a Linux runner with /proc — ubuntu-latest is fine, macOS is not (and is already listed as unsupported in the README).
  • Worth considering a matrix over bash versions. README claims "Bash 4 or newer"; ubuntu-latest ships bash 5, so the bash 4 claim is currently untested.
  • Installer coverage is a separate question. install.sh writes to global agent instruction files, so any CI exercise of it must set KDS_SKIP_AGENT_INSTRUCTIONS=1.

Activity

  1. zanellig commented on Sep 3, 2026

    @zanellig
    OwnerAuthor

    Done in #4 (merged).

    .github/workflows/ci.yml runs on pull_request and on pushes to main (push is scoped to main so PR branches get one run, not two), on ubuntu-latest since the suite spawns real PIDs and reads /proc:

    • bash -n over the three scripts, one file per invocation — bash -n only reads its first file argument and treats the rest as positional parameters, so the obvious one-liner would have checked only kill-dev-servers.sh
    • shellcheck kill-dev-servers.sh install.sh tests/test-cli.sh (preinstalled on the runner)
    • bash tests/test-cli.sh

    The gate paid for itself on its first run. It caught a locale-dependent bug: listener discovery sorted bindings under the ambient locale, so a process bound to both *:3002 and 127.0.0.1:3002 rendered them in one order under en_US.UTF-8 and the reverse under the runner's C.UTF-8. Fixed at the source — discovery now sorts with LC_ALL=C — rather than by loosening the assertion. Codex independently flagged the same thing as P1.

    Enabling shellcheck also required fixing SC2295 (unquoted ${BASH_REMATCH[1]} inside a ${var#pattern} pattern), SC2004, SC2190 (an associative target_indices local colliding with the top-level index array, now selected_indices), SC2086 in the tests, and a duplicated shebang line.

    Not done, both flagged in the PR:

    • Bash 4 matrix. No runner or image offers bash 4 alongside procps ps and /proc — the bash:4.x images are BusyBox/Alpine, which the README already lists as unsupported — so it would mean building bash from source in CI. The README's "Bash 4 or newer" claim remains untested; worth a separate issue if the claim matters.
    • Installer coverage, which this issue already scoped out as a separate question.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions