diff options
| author | Craig Jennings <c@cjennings.net> | 2026-07-24 10:56:50 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-07-24 10:56:50 -0500 |
| commit | 267d1de7b8a8e7fd22b156433567c88216ec3d0f (patch) | |
| tree | 4289b12c333ea3a0c0c027c4de2c38224a5aced5 /scripts | |
| parent | b34fb4e413eea994f2492669d64d2e5ee9aa8110 (diff) | |
| download | rulesets-267d1de7b8a8e7fd22b156433567c88216ec3d0f.tar.gz rulesets-267d1de7b8a8e7fd22b156433567c88216ec3d0f.zip | |
fix: harden the org-file mutators and close four verified bugs
An overnight hygiene run over this repo, reviewed adversarially afterward. Every problem below was reproduced before it was fixed, and every fix was re-checked by a skeptic who was told to refute it.
cj-remove-block could silently destroy content. Its range check validated only the first and last lines, so a span from one cj block's opener to a later block's closer passed and the removal deleted everything between — prose, headings, whole tasks, with a zero exit. That is the failure the check exists to prevent, and drift is its normal case, since the calling skill edits the file while processing. It now refuses a range holding more than one block, backs up first, and writes atomically while preserving the file's mode and following symlinks to the real target.
todo-cleanup rewrote todo.org with no backup, alone among the three tools that mutate these files. It now copies to the temp dir like lint-org does. Both backup helpers suffix rather than overwrite, because a second-resolution stamp collapsed two back-to-back invocations into one backup holding already-mutated content — and running todo-cleanup twice in a row is the shipped path.
audit.bats failed about one run in eight, in teardown rather than in its assertions, so a passing test reported as a false red. git commit spawns a detached maintenance process that is still writing packs when the commit returns, racing the cleanup. The fixture disables it.
route_recommend downgraded a correct strong match to weak when two projects shared a basename. lint.sh now sweeps the scripts that get symlinked onto PATH, which were the only shell in the repo with no gate over them.
Two test defects found in review and fixed here: a suite that deleted real backups from the shared temp dir, and one that depended on that directory's contents. Isolation now lives in an autouse fixture rather than in each test's memory.
Known and filed, not closed: the range check still cannot prove the range is the block that was scanned, so drift by exactly one whole block deletes the wrong annotation. That needs a content assertion and a CLI contract change.
Diffstat (limited to 'scripts')
| -rwxr-xr-x | scripts/lint.sh | 8 | ||||
| -rw-r--r-- | scripts/tests/audit.bats | 19 | ||||
| -rw-r--r-- | scripts/tests/lint-coverage.bats | 54 |
3 files changed, 81 insertions, 0 deletions
diff --git a/scripts/lint.sh b/scripts/lint.sh index 61a27a1..82f3b34 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -114,6 +114,14 @@ for s in scripts/*.sh; do check_hook "$s" done +# Scripts `make install` symlinks onto PATH. Extensionless by convention, so +# they need their own glob — scripts/*.sh above never matched them, which left +# the most exposed shell in the repo as the only shell with no gate over it. +for s in claude-templates/bin/*; do + [ -f "$s" ] || continue + check_hook "$s" +done + # Markdown link validation across rules and skills for f in claude-rules/*.md */SKILL.md; do [ -f "$f" ] || continue diff --git a/scripts/tests/audit.bats b/scripts/tests/audit.bats index 0b062fb..c6123e1 100644 --- a/scripts/tests/audit.bats +++ b/scripts/tests/audit.bats @@ -40,9 +40,28 @@ git_init_with_ai_tracked() { local proj_dir="$1" (cd "$proj_dir" \ && git init -q \ + && git config maintenance.auto false \ + && git config gc.auto 0 \ && git add -A \ && git -c user.email=test@test -c user.name=test commit -q -m initial) } +# maintenance.auto/gc.auto are off because `git commit` otherwise spawns +# `git maintenance run --auto --quiet --detach` (traced on git 2.55). The commit +# returns while that detached process is still writing .git/objects/pack, so +# teardown's `rm -rf` intermittently failed with "Directory not empty" and bats +# reported a passing test as failed. Measured at ~8% of runs. Killing the +# background writer is the fix; a retry loop in teardown would only hide it. +# +# What triggers it: the fixture rsyncs ~170 workflow/script files before +# committing, and maintenance's loose-objects task fires at a default threshold +# of 100. (Not gc.auto's 6700 — that never fires here, which is why disabling +# gc.auto alone was never the explanation.) maintenance.auto false is what +# suppresses it on 2.55; gc.auto 0 is the portable guard for older git, where +# commit calls gc --auto directly and maintenance.auto does not exist. Either +# is sufficient on its own here. +# +# Other bats suites that git-init fixtures are currently safe only by staying +# under that 100-object threshold — an implicit dependency, not a guarantee. @test "audit: clean projects report ok with exit 0" { scaffold_synced_ai "$TEST_HOME/code/alpha" diff --git a/scripts/tests/lint-coverage.bats b/scripts/tests/lint-coverage.bats new file mode 100644 index 0000000..130212d --- /dev/null +++ b/scripts/tests/lint-coverage.bats @@ -0,0 +1,54 @@ +#!/usr/bin/env bats +# +# Coverage tests for scripts/lint.sh. +# +# lint.sh sweeps scripts/*.sh, languages/*/claude/hooks/*.sh, and +# languages/*/githooks/* through check_hook (shebang present, executable bit +# set). It never touched claude-templates/bin/ — the four scripts `make install` +# symlinks into ~/.local/bin, so the most exposed shell in the repo was the only +# shell with no gate over it (found 2026-07-24). +# +# These tests pin the coverage itself rather than the current cleanliness: they +# plant a deliberately broken file in each swept location and assert lint.sh +# complains. A location that stops being swept fails here. + +REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/../.." && pwd)" +LINT="$REPO_ROOT/scripts/lint.sh" + +teardown() { + [ -n "${PLANTED:-}" ] && rm -f "$PLANTED" + PLANTED="" +} + +@test "lint: a bin/ script missing its shebang is flagged" { + PLANTED="$REPO_ROOT/claude-templates/bin/zz-test-noshebang" + printf 'echo hi\n' > "$PLANTED" + chmod +x "$PLANTED" + run bash "$LINT" + [[ "$output" == *"zz-test-noshebang"* ]] || { + echo "lint.sh did not report the planted bin/ script — that path is unswept" + echo "$output" + return 1 + } +} + +@test "lint: a bin/ script that is not executable is flagged" { + PLANTED="$REPO_ROOT/claude-templates/bin/zz-test-noexec" + printf '#!/usr/bin/env bash\necho hi\n' > "$PLANTED" + chmod -x "$PLANTED" + run bash "$LINT" + [[ "$output" == *"zz-test-noexec"* ]] +} + +@test "lint: the existing scripts/ sweep still works (guards the regression)" { + PLANTED="$REPO_ROOT/scripts/zz-test-noshebang.sh" + printf 'echo hi\n' > "$PLANTED" + chmod +x "$PLANTED" + run bash "$LINT" + [[ "$output" == *"zz-test-noshebang.sh"* ]] +} + +@test "lint: the real tree passes (no planted file)" { + run bash "$LINT" + [ "$status" -eq 0 ] +} |
