diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c759f4e..d1c7513 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,6 +123,11 @@ jobs: echo "Validating tracked shell scripts are executable..." ./scripts/check-script-permissions.sh + - name: Validate skill folders are self-contained + run: | + echo "Validating no skill symlink escapes its skill folder..." + ./scripts/check-skill-links.sh + - name: Validate hooks.json run: | echo "Validating hooks/hooks.json..." diff --git a/Makefile b/Makefile index 6b0d459..2a7a093 100644 --- a/Makefile +++ b/Makefile @@ -104,6 +104,8 @@ lint: | xargs shellcheck -x @echo "🔍 Checking script permissions..." @./scripts/check-script-permissions.sh + @echo "🔍 Checking skill links..." + @./scripts/check-skill-links.sh @echo "🔍 Checking bats assertions..." @./scripts/check-bats-assertions.sh @echo "✅ Lint complete!" diff --git a/scripts/check-skill-links.sh b/scripts/check-skill-links.sh new file mode 100755 index 0000000..5fb00bf --- /dev/null +++ b/scripts/check-skill-links.sh @@ -0,0 +1,68 @@ +#!/bin/bash + +# Fails if a skill ships a symlink that points outside its own skill folder. +# Per-skill-dir installers reject escaping paths, so each skill folder stays +# self-contained. Reads the index rather than the working tree — the index is +# what a fresh clone gets. Must be run from the repository root. + +set -euo pipefail + +if ! command -v git >/dev/null 2>&1; then + echo "FAIL: git is required to check skill links" + exit 1 +fi + +if ! git rev-parse --show-toplevel >/dev/null 2>&1; then + echo "FAIL: not inside a git repository" + exit 1 +fi + +failed=0 +count=0 + +while IFS=$'\t' read -r meta path; do + [ -n "$meta" ] || continue + mode="${meta%% *}" + [ "$mode" = "120000" ] || continue + count=$((count + 1)) + + skill_dir="$(echo "$path" | cut -d/ -f1-2)" + skill_abs="$(cd "$skill_dir" && pwd -P)" + if ! target="$(readlink "$path" 2>/dev/null)"; then + echo "FAIL: $path is tracked as a symlink but is not one on disk (stage your changes)" + failed=1 + continue + fi + + case "$target" in + /*) abs="$target" ;; + *) abs="$(cd "$(dirname "$path")" && pwd -P)/$target" ;; + esac + + if ! dir_canon="$(cd -P "$(dirname "$abs")" 2>/dev/null && pwd)"; then + echo "FAIL: $path points at missing location $target" + failed=1 + continue + fi + canon="$dir_canon/$(basename "$abs")" + + if [ ! -e "$canon" ]; then + echo "FAIL: $path points at missing location $target" + failed=1 + continue + fi + + case "$canon" in + "$skill_abs"/*) ;; + *) + echo "FAIL: $path escapes $skill_dir (points at $target)" + failed=1 + ;; + esac +done < <(git ls-files -s -- 'skills/*') + +if [ "$failed" -eq 1 ]; then + exit 1 +fi + +echo "PASS: $count skill symlink(s) stay inside their skill folder" diff --git a/scripts/check-skill-links.test.bats b/scripts/check-skill-links.test.bats new file mode 100644 index 0000000..a0366ef --- /dev/null +++ b/scripts/check-skill-links.test.bats @@ -0,0 +1,94 @@ +#!/usr/bin/env bats + +# Unit Tests for scripts/check-skill-links.sh +# All tests invoke as subprocess against temp fixture git repositories + +PROJECT_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)" +export PROJECT_ROOT + +load "$PROJECT_ROOT/tests/helpers/assertions.bash" + +LINKS_SCRIPT="$PROJECT_ROOT/scripts/check-skill-links.sh" + +setup() { + FIXTURE_DIR="$(mktemp -d)" + + # Build a fixture repo with one self-contained skill + mkdir -p "$FIXTURE_DIR/skills/demo/scripts" + cp "$LINKS_SCRIPT" "$FIXTURE_DIR/scripts-check-skill-links.sh" + printf '#!/bin/bash\necho ok\n' > "$FIXTURE_DIR/skills/demo/scripts/gate.sh" + printf '# Skill\n' > "$FIXTURE_DIR/skills/demo/SKILL.md" + + cd "$FIXTURE_DIR" + git init -q . + git add -A +} + +teardown() { + rm -rf "$FIXTURE_DIR" +} + +run_links_script() { + run bash "$FIXTURE_DIR/scripts-check-skill-links.sh" +} + +# --- Happy path --- + +@test "check-skill-links passes when a skill has no symlinks" { + cd "$FIXTURE_DIR" + run_links_script + [ "$status" -eq 0 ] + assert_output_contains "PASS: 0 skill symlink(s) stay inside their skill folder" +} + +@test "check-skill-links passes when a link stays inside its skill folder" { + cd "$FIXTURE_DIR" + ln -sf gate.sh "$FIXTURE_DIR/skills/demo/scripts/inner.sh" + git add -A + run_links_script + [ "$status" -eq 0 ] + assert_output_contains "PASS: 1 skill symlink(s)" +} + +# --- Escaping links --- + +@test "check-skill-links fails and names a link pointing outside its skill folder" { + cd "$FIXTURE_DIR" + mkdir -p shared + printf '#!/bin/bash\n' > shared/gate.sh + ln -sf ../../../shared/gate.sh "$FIXTURE_DIR/skills/demo/scripts/gate.sh" + git add -A + run_links_script + [ "$status" -eq 1 ] + assert_output_contains "skills/demo/scripts/gate.sh escapes skills/demo" +} + +@test "check-skill-links fails on an absolute link target" { + cd "$FIXTURE_DIR" + ln -sf /bin/sh "$FIXTURE_DIR/skills/demo/scripts/gate.sh" + git add -A + run_links_script + [ "$status" -eq 1 ] + assert_output_contains "escapes skills/demo" +} + +@test "check-skill-links fails on a dangling link" { + cd "$FIXTURE_DIR" + ln -sf nowhere.sh "$FIXTURE_DIR/skills/demo/scripts/gate.sh" + git add -A + run_links_script + [ "$status" -eq 1 ] + assert_output_contains "points at missing location" +} + +@test "check-skill-links checks each skill against its own folder" { + cd "$FIXTURE_DIR" + mkdir -p "$FIXTURE_DIR/skills/other/scripts" + printf '# Skill\n' > "$FIXTURE_DIR/skills/other/SKILL.md" + printf '#!/bin/bash\n' > "$FIXTURE_DIR/skills/other/scripts/gate.sh" + ln -sf ../../demo/scripts/gate.sh "$FIXTURE_DIR/skills/other/scripts/gate.sh" + git add -A + run_links_script + [ "$status" -eq 1 ] + assert_output_contains "skills/other/scripts/gate.sh escapes skills/other" +} diff --git a/skills/document/scripts/doc-gates.sh b/skills/document/scripts/doc-gates.sh deleted file mode 120000 index 88f79bd..0000000 --- a/skills/document/scripts/doc-gates.sh +++ /dev/null @@ -1 +0,0 @@ -../../../scripts/doc-gates.sh \ No newline at end of file diff --git a/skills/document/scripts/doc-gates.sh b/skills/document/scripts/doc-gates.sh new file mode 100755 index 0000000..2efda24 --- /dev/null +++ b/skills/document/scripts/doc-gates.sh @@ -0,0 +1,37 @@ +#!/bin/bash +# Document skill gates - checks agent output before the next batch +# Usage: doc-gates.sh [sf-dir] + +MODE="$1" +case "$MODE" in + analysis) FILES="artifacts-summary.md implementation-summary.md docs-inventory.md" ;; + generation) FILES="technical-docs.md user-docs.md" ;; + *) echo "Usage: doc-gates.sh [sf-dir]" >&2; exit 1 ;; +esac + +RESEARCH="${2:-${SF_DIR:-.sf}}/research" + +# A marker shape means an unfinished draft. A bare word like "placeholder" in a +# props table is ordinary prose. +MARKER='(TODO|TBD)[[:space:]]*:|[[<](TODO|TBD|PLACEHOLDER|INSERT)' + +# An inline code span quotes a marker, it does not leave one, so strip spans first. +has_marker() { + # shellcheck disable=SC2016 # the backticks are markdown, not a subshell + sed 's/`[^`]*`//g' "$1" | grep -qiE "$MARKER" +} + +STATUS=0 +for file in $FILES; do + if [ ! -f "$RESEARCH/$file" ]; then + echo "FAIL $file: missing from $RESEARCH" >&2 + STATUS=1 + # An empty file is a valid "no docs needed" signal. Only markers fail. + elif [ "$MODE" = generation ] && has_marker "$RESEARCH/$file"; then + echo "FAIL $file: contains a placeholder marker" >&2 + STATUS=1 + else + echo "PASS $file" + fi +done +exit $STATUS diff --git a/skills/implement/scripts/validate-implementation.sh b/skills/implement/scripts/validate-implementation.sh deleted file mode 120000 index 13185f6..0000000 --- a/skills/implement/scripts/validate-implementation.sh +++ /dev/null @@ -1 +0,0 @@ -../../../scripts/validate-implementation.sh \ No newline at end of file diff --git a/skills/implement/scripts/validate-implementation.sh b/skills/implement/scripts/validate-implementation.sh new file mode 100755 index 0000000..c2a3aa5 --- /dev/null +++ b/skills/implement/scripts/validate-implementation.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# Implementation check - counts the acceptance criteria the spec still has unchecked +# Usage: validate-implementation.sh [sf-dir] + +SF_DIR="${1:-${SF_DIR:-.sf}}" +SPEC_FILE="$SF_DIR/spec.md" +IMPL_FILE="$SF_DIR/implementation-summary.md" + +# Before both files exist there is nothing to compare. +if [ ! -f "$SPEC_FILE" ] || [ ! -f "$IMPL_FILE" ]; then exit 0; fi + +CRITERIA=$(grep -i "^\- \[" "$SPEC_FILE" 2>/dev/null) +[ -z "$CRITERIA" ] && exit 0 + +UNCHECKED=$(echo "$CRITERIA" | grep -c "^\- \[ \]") +if [ "$UNCHECKED" -gt 0 ]; then + echo "$UNCHECKED acceptance criteria unchecked in spec" >&2 + exit 1 +fi +exit 0 diff --git a/skills/spec/scripts/spec-dir.sh b/skills/spec/scripts/spec-dir.sh deleted file mode 120000 index a8b10e7..0000000 --- a/skills/spec/scripts/spec-dir.sh +++ /dev/null @@ -1 +0,0 @@ -../../../scripts/spec-dir.sh \ No newline at end of file diff --git a/skills/spec/scripts/spec-dir.sh b/skills/spec/scripts/spec-dir.sh new file mode 100755 index 0000000..77296ac --- /dev/null +++ b/skills/spec/scripts/spec-dir.sh @@ -0,0 +1,60 @@ +#!/bin/bash +# SF artifact directory setup - prepares $SF_DIR for a spec run +# Usage: spec-dir.sh [sf-dir] + +MODE="$1" +case "$MODE" in + first|update|new) ;; + *) echo "Usage: spec-dir.sh [sf-dir]" >&2; exit 1 ;; +esac + +# Resolve the artifact directory: argument, then $SF_DIR, then a marker walk +SF_DIR="${2:-$SF_DIR}" +if [ -z "$SF_DIR" ]; then + PROJECT_ROOT="$(pwd)" + while [ ! -e "$PROJECT_ROOT/.git" ] && [ ! -f "$PROJECT_ROOT/AGENTS.md" ] && [ ! -f "$PROJECT_ROOT/CLAUDE.md" ]; do + [ "$PROJECT_ROOT" = "/" ] && { echo "Error: project root not found. Looked for .git, AGENTS.md or CLAUDE.md in $(pwd) and every parent directory." >&2; exit 1; } + PROJECT_ROOT="$(dirname "$PROJECT_ROOT")" + done + SF_DIR="$PROJECT_ROOT/.sf" + [ -d "$PROJECT_ROOT/.git" ] && [ -f "$PROJECT_ROOT/.gitignore" ] && ! grep -qF '.sf/' "$PROJECT_ROOT/.gitignore" && echo '.sf/' >> "$PROJECT_ROOT/.gitignore" +fi + +mkdir -p "$SF_DIR" 2>/dev/null || { echo "Error: Cannot create or write to $SF_DIR" >&2; exit 1; } + +case "$MODE" in + "update") + cp "$SF_DIR/spec.md" "$SF_DIR/spec-backup.md" 2>/dev/null + # No trailing slash: this must remove a symlink as a symlink + rm -rf "$SF_DIR/research" + ;; + "new") + # Never reuse an archive name, even for two runs in the same second + name=$(date -u +%Y-%m-%dT%H%M%S) + suffix=1 + while [ -e "$SF_DIR/specs/$name" ]; do + name="$(date -u +%Y-%m-%dT%H%M%S)-$suffix" + suffix=$((suffix + 1)) + done + mkdir -p "$SF_DIR/specs/$name" + # A symlink already points into an older archive. Drop it and keep that archive. + if [ -L "$SF_DIR/spec.md" ]; then + rm -f "$SF_DIR/spec.md" + else + mv "$SF_DIR/spec.md" "$SF_DIR/specs/$name/" 2>/dev/null + fi + if [ -L "$SF_DIR/research" ]; then + rm -f "$SF_DIR/research" + else + mv "$SF_DIR/research" "$SF_DIR/specs/$name/" 2>/dev/null + fi + mkdir -p "$SF_DIR/specs/$name/research" + if ! ln -sfn "specs/$name/spec.md" "$SF_DIR/spec.md" || ! ln -sfn "specs/$name/research" "$SF_DIR/research"; then + rm -rf "$SF_DIR/specs/$name/" + echo "Error: Failed to create symlinks" >&2 + exit 1 + fi + ;; +esac + +mkdir -p "$SF_DIR/research" diff --git a/skills/spec/scripts/validate-spec.sh b/skills/spec/scripts/validate-spec.sh deleted file mode 120000 index c92500b..0000000 --- a/skills/spec/scripts/validate-spec.sh +++ /dev/null @@ -1 +0,0 @@ -../../../scripts/validate-spec.sh \ No newline at end of file diff --git a/skills/spec/scripts/validate-spec.sh b/skills/spec/scripts/validate-spec.sh new file mode 100755 index 0000000..4575b0c --- /dev/null +++ b/skills/spec/scripts/validate-spec.sh @@ -0,0 +1,19 @@ +#!/bin/bash +# Spec structure check - reports the sections spec.md does not have +# Usage: validate-spec.sh [sf-dir] + +SPEC_FILE="${1:-${SF_DIR:-.sf}}/spec.md" + +# No spec yet means nothing to check. +[ ! -f "$SPEC_FILE" ] && exit 0 + +MISSING="" +grep -qi "scope\|problem" "$SPEC_FILE" || MISSING="$MISSING scope/problem," +grep -qi "criteria\|acceptance" "$SPEC_FILE" || MISSING="$MISSING acceptance criteria," +grep -qi "risk" "$SPEC_FILE" || MISSING="$MISSING risks," + +if [ -n "$MISSING" ]; then + echo "Spec missing sections:${MISSING%,}" >&2 + exit 1 +fi +exit 0