From 46eb48a766b55f857af3c467dada9bdd2acc1280 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 5 Aug 2026 15:37:54 +0000 Subject: [PATCH] fix: run the whole test suite in CI, and repair the break it was hiding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 19 of 42 test files were present in the repo and referenced nowhere in .github/workflows/shell.yml — nearly half the suite, run only when someone remembered to run it by hand. That is not theoretical. #334 changed how the Kimaki lock port reaches a systemd unit, updated tests/kimaki-multi-instance.sh to match, merged with 20/20 checks green, and left that test failing on main, because it was one of the 19. CI reported green over red. The only reason it surfaced was an unrelated full-suite run afterwards. The break itself: #334 removed the `--lock-port` argument that bridge_render_systemd used to append to ExecStart, in favour of an Environment= line. But the renderer only interpolates the env block its caller hands it, so the guarantee moved from something the RENDERER enforced unconditionally to something every CALLER has to remember. A caller that renders with an env block lacking the line silently drops the port, and the instance falls back to the default — two Kimaki instances then contend for one lock, which is the exact failure multi-instance support exists to prevent. The renderer now re-asserts the line when KIMAKI_LOCK_PORT is set and the block does not already carry it, at the same layer that always owned it. The suites are wired in as a matrix rather than 21 more hand-written jobs, so adding a test is a one-line change. They run through `bash` because several of these files do not carry the executable bit — invoking them as ./tests/x.sh reports a Permission denied that reads exactly like a test failure. tests/ci-coverage.sh is the part that matters. A workflow that lists jobs by hand fails silently and permanently: the person adding a test is the same person who must remember to wire it, and nothing complains when they do not. The guard fails loudly instead, and has an EXCLUDED list so that deliberately skipping a test is a visible decision rather than an omission. It checks for the test name anywhere in the workflow rather than one exact invocation, so a matrix, a composite job, or a future restructure all still satisfy it — the claim is "CI knows about this file", not "CI runs it this particular way". Local suite is green apart from three environment-only failures that also fail on origin/main: this host has a real /usr/bin/kimaki, which two tests resolve instead of their fixtures, and no studio CLI for a third. --- .github/workflows/shell.yml | 36 +++++++++++++ bridges/kimaki.sh | 13 +++++ tests/ci-coverage.sh | 105 ++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+) create mode 100755 tests/ci-coverage.sh diff --git a/.github/workflows/shell.yml b/.github/workflows/shell.yml index 78787a8..0010963 100644 --- a/.github/workflows/shell.yml +++ b/.github/workflows/shell.yml @@ -194,3 +194,39 @@ jobs: - uses: actions/checkout@v4 - name: Run tests/claude-code-hook-scope.sh run: ./tests/claude-code-hook-scope.sh + + # Suites that had no CI job until #335. Run as a matrix so adding a test is a + # one-line change and tests/ci-coverage.sh stays satisfied; invoked through + # `bash` because several of these files do not carry the executable bit. + suites: + name: ${{ matrix.test }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + test: + - agents-md-backup-retention + - carried-claude-code-plugin + - ci-coverage + - codex-runtime + - datamachine-worker + - detect-site-domain + - homeboy-codebox-canary + - homeboy-components + - homeboy-dmc-provider + - homeboy-project-id + - kimaki-launchd-start + - kimaki-managed-plugin-rig + - kimaki-multi-instance + - kimaki-system-message-patch + - opencode-claude-auth-refresh-hardening + - path-helpers + - post-upgrade-restore + - runtime-signature + - upgrade-opencode-auth-plugin-sync + - worktree-context-projections + - wp-config-permissions + steps: + - uses: actions/checkout@v4 + - name: Run tests/${{ matrix.test }}.sh + run: bash tests/${{ matrix.test }}.sh diff --git a/bridges/kimaki.sh b/bridges/kimaki.sh index ef669b5..1ed9e97 100644 --- a/bridges/kimaki.sh +++ b/bridges/kimaki.sh @@ -1190,6 +1190,19 @@ bridge_render_systemd() { local skill_filter_args skill_filter_args="$(_kimaki_skill_filter_args_shell)" _kimaki_validate_lock_port + + # The lock port reaches the unit through Environment= rather than the + # --lock-port argument this used to append (#334). That moved a guarantee the + # RENDERER held unconditionally into data the CALLER supplies, so a caller + # that passes an env block without the line silently drops the port and the + # instance falls back to the default — two instances then fight over one lock. + # Re-assert it here, where it was always enforced. + if [ -n "${KIMAKI_LOCK_PORT:-}" ] && \ + ! printf '%s\n' "$env_block" | grep -q '^Environment=KIMAKI_LOCK_PORT='; then + env_block="$env_block +Environment=KIMAKI_LOCK_PORT=$KIMAKI_LOCK_PORT" + fi + local stale_worker_cleanup="# User-wide stale-worker cleanup omitted for instance isolation." if [ "$unit" = "kimaki.service" ]; then stale_worker_cleanup='ExecStartPre=-/usr/bin/pkill -TERM -u '$SERVICE_USER' -f "opencode-ai/bin/.*serve"' diff --git a/tests/ci-coverage.sh b/tests/ci-coverage.sh new file mode 100755 index 0000000..9194904 --- /dev/null +++ b/tests/ci-coverage.sh @@ -0,0 +1,105 @@ +#!/bin/bash +# tests/ci-coverage.sh — every test in tests/ is actually run by CI. +# +# WHY THIS EXISTS +# +# On 2026-08-05, 19 of 42 test files — nearly half the suite — were present in +# the repo and referenced nowhere in .github/workflows/shell.yml. They were only +# ever run by someone remembering to run them by hand. +# +# The consequence was not theoretical. #334 changed how the Kimaki lock port +# reaches a systemd unit, updated tests/kimaki-multi-instance.sh to match, +# merged with 20/20 checks green, and left that test failing on main — because +# it was one of the 19. CI reported green over red, and the only reason anyone +# noticed was an unrelated full-suite run. +# +# A workflow that lists jobs by hand fails this way silently and permanently: the +# person adding a test is the same person who has to remember to wire it, and +# nothing complains when they don't. This test makes the omission loud, which is +# the only property that actually holds over time. +# +# It deliberately checks for the test NAME anywhere in the workflow rather than +# an exact `./tests/.sh` invocation, so a matrix entry, a composite job, or +# a future restructuring all still satisfy it. The claim being made is "CI knows +# about this file", not "CI invokes it in one particular way". +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")/.." && pwd)" +cd "$SCRIPT_DIR" + +WORKFLOW=".github/workflows/shell.yml" +FAILED=0 + +if [ ! -f "$WORKFLOW" ]; then + echo " FAIL $WORKFLOW not found" + exit 1 +fi + +# Tests deliberately not wired into CI, each with the reason it cannot run there. +# Adding a name here is a decision that should be visible in review; an empty +# list is the healthy state. +# +# (none) +# +EXCLUDED="" + +is_excluded() { + local name="$1" e + for e in $EXCLUDED; do + [ "$name" = "$e" ] && return 0 + done + return 1 +} + +echo "ci-coverage: every tests/*.sh is referenced by $WORKFLOW" + +workflow_text="$(cat "$WORKFLOW")" +missing="" + +for path in tests/*.sh; do + name="$(basename "$path" .sh)" + if is_excluded "$name"; then + echo " skip $name (documented exclusion)" + continue + fi + case "$workflow_text" in + *"$name"*) ;; + *) + missing="$missing $name" + FAILED=$((FAILED + 1)) + ;; + esac +done + +if [ -n "$missing" ]; then + echo " FAIL these tests exist but CI never runs them:" + for name in $missing; do + echo " tests/$name.sh" + done + echo "" + echo " Add a job to $WORKFLOW, or add the name to EXCLUDED in this file" + echo " with the reason it cannot run in CI." +else + echo " ok all $(ls tests/*.sh | wc -l | tr -d ' ') shell tests are referenced" +fi + +# Non-shell suites are invoked directly by their own jobs; assert the two that +# exist stay wired, since they are the easiest to lose in a restructure. +for other in tests/dm-agent-sync.mjs tests/smoke-cli-transport.php; do + [ -e "$other" ] || continue + case "$workflow_text" in + *"$(basename "$other")"*) echo " ok $(basename "$other") is referenced" ;; + *) + echo " FAIL $other exists but CI never runs it" + FAILED=$((FAILED + 1)) + ;; + esac +done + +echo "" +if [ "$FAILED" -eq 0 ]; then + echo "ci-coverage: all assertions passed" +else + echo "ci-coverage: $FAILED assertion(s) failed" + exit 1 +fi