From eb0f093efa8874d42772fbbcc65b779a0a42e6fd Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 1 Sep 2026 09:14:44 +0200 Subject: [PATCH 1/3] fix(scripts): give the desired-state digests a writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate-manifests.sh treats the content digests in a *.desired-state.json resource as a required gate, but nothing in this repository ever wrote them. A branch that legitimately changes a bundled agent, skill, or runtime asset โ€” the daily agent-skills sync being the standing case โ€” therefore produces a manifest its own CI rejects, and re-running the sync cannot fix it. Only a hand edit could, and a hand edit does not survive: on 2026-09-01 a digest fix pushed to deps/agent-skills-update was force-pushed away by the next sync run 6 minutes later, restoring the stale value with no signal that it had happened. Add scripts/refresh-desired-state-digests.sh as that writer and run it in the same follow-up job that already bumps plugin versions on the sync branch, so the generated branch is self-consistent by construction. The generator and the validator now source their two hashing rules from scripts/sha256.lib.sh rather than each carrying a copy: a generator that disagreed with the gate about what a file hashes to would leave the branch exactly as unmergeable as having no generator at all. Fixes #179 --- .github/workflows/ci.yaml | 9 + .github/workflows/update-agent-skills.yaml | 32 ++- AGENTS.md | 10 +- scripts/refresh-desired-state-digests.sh | 186 +++++++++++++++ scripts/refresh-desired-state-digests.test.sh | 211 ++++++++++++++++++ scripts/sha256.lib.sh | 34 +++ scripts/validate-manifests.sh | 31 +-- 7 files changed, 480 insertions(+), 33 deletions(-) create mode 100755 scripts/refresh-desired-state-digests.sh create mode 100755 scripts/refresh-desired-state-digests.test.sh create mode 100644 scripts/sha256.lib.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4ccf5ef..68dcf78 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -84,6 +84,15 @@ jobs: # plugin reaching consumers. Self-contained: throwaway fixtures, no network. run: ./scripts/validate-manifests.test.sh + - name: ๐Ÿงช Self-test the desired-state digest generator + # Proves the generator writes exactly what validate-manifests.sh demands โ€” a + # generator that disagreed with the gate would leave a sync branch just as + # unmergeable as having none. Asserts the coupling against the REAL validator on + # a copy of this tree, keeps the two hashing rules distinct (definition files + # normalize CRLF, executed runtime assets do not), and proves every unresolvable + # digest fails closed rather than being written wrong. + run: ./scripts/refresh-desired-state-digests.test.sh + - name: ๐Ÿงช Self-test the bundled-skill edit guard # Proves the guard FAILS a hand-edit to a synced skill tree and names its # upstream, PASSES the programmed sync (actor AND branch), passes a wholly diff --git a/.github/workflows/update-agent-skills.yaml b/.github/workflows/update-agent-skills.yaml index 6e9b2c9..91794fb 100644 --- a/.github/workflows/update-agent-skills.yaml +++ b/.github/workflows/update-agent-skills.yaml @@ -21,13 +21,17 @@ jobs: secrets: APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} - # A skill sync changes shipped plugin content. Consumers cache plugins by version, so - # without a matching version bump the update is unreachable for everyone who already - # installed that version โ€” and the `Check version bump` CI gate rejects it. The sync - # workflow that opens the PR has no post-update hook, so the bump lands here as a - # follow-up commit on the same branch. + # A skill sync changes shipped plugin content, and two repository gates then reject the + # branch the sync produced. Consumers cache plugins by version, so without a matching + # version bump the update is unreachable for everyone who already installed that + # version, and `Check version bump` rejects it. A synced agent or skill also moves a + # content digest that `Validate manifests` pins, and nothing else in this repository + # writes those digests โ€” so without the refresh the branch is only greenable by a hand + # edit, which the next sync force-pushes away with no signal that it happened. The sync + # workflow that opens the PR has no post-update hook, so both land here as follow-up + # commits on the same branch. bump-versions: - name: Bump versions of changed plugins + name: Refresh digests and bump versions of changed plugins needs: update runs-on: ubuntu-latest permissions: @@ -54,6 +58,22 @@ jobs: continue-on-error: true id: checkout + - name: ๐Ÿ” Refresh the desired-state digests the sync moved + if: steps.checkout.outcome == 'success' + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + ./scripts/refresh-desired-state-digests.sh + if git diff --quiet; then + echo "Declared digests already match the synced content." + exit 0 + fi + # Stage only what the generator is allowed to touch, so an unrelated working-tree + # change could never ride along in this commit. + git add -- '*.desired-state.json' + git commit -m "chore(deps): refresh desired-state digests for synced content" + git push origin HEAD:deps/agent-skills-update + - name: ๐Ÿ”ข Bump every plugin whose content changed if: steps.checkout.outcome == 'success' run: | diff --git a/AGENTS.md b/AGENTS.md index fdfec6d..41af53f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,10 @@ scripts/ โ”œโ”€โ”€ guard-bundled-skill-edits.sh # Gate: refuse a hand-edit to a synced skill tree, naming its upstream โ”œโ”€โ”€ guard-bundled-skill-edits.test.sh # Self-test for the gate above โ”œโ”€โ”€ bump-plugin-version.sh # Move a plugin's version across all four manifests (the fix the gate points at) -โ””โ”€โ”€ bump-plugin-version.test.sh # Self-test for the bump helper +โ”œโ”€โ”€ bump-plugin-version.test.sh # Self-test for the bump helper +โ”œโ”€โ”€ refresh-desired-state-digests.sh # Writer: recompute every digest a *.desired-state.json pins (the fix "digest must match" points at) +โ”œโ”€โ”€ refresh-desired-state-digests.test.sh # Self-test for the generator, incl. its coupling to the validator +โ””โ”€โ”€ sha256.lib.sh # The two hashing rules, sourced by BOTH the validator and the generator so they cannot drift README.md # Human-facing index โ€” the plugin table + per-tool install instructions ``` @@ -210,6 +213,11 @@ does not currently enforce but that keeps workflow changes clean: # Fix a failure with: ./scripts/bump-plugin-version.sh [patch|minor|major] ./scripts/check-plugin-version-bump.sh origin/main HEAD +# 1c. Every content digest a desired-state resource pins must match the file it pins. +# Those digests have a writer: refresh them rather than hand-editing, or the next +# agent-skills sync force-pushes the hand edit away. --check reports without writing. +./scripts/refresh-desired-state-digests.sh --check + # 2. Validate each bundled skill against the agentskills.io spec (the matrixed CI check). Pin to the # SAME agentskills commit CI uses (AGENTSKILLS_REF in .github/workflows/ci.yaml) so local matches CI. python -m pip install "skills-ref @ git+https://github.com/agentskills/agentskills.git@8d8fcbc69e0c42e05922c2ffc287a3bbdef7b0a3#subdirectory=skills-ref" diff --git a/scripts/refresh-desired-state-digests.sh b/scripts/refresh-desired-state-digests.sh new file mode 100755 index 0000000..dcc87f1 --- /dev/null +++ b/scripts/refresh-desired-state-digests.sh @@ -0,0 +1,186 @@ +#!/usr/bin/env bash +# Recompute every content digest a *.desired-state.json resource declares, from the +# bundled files those digests pin. +# +# Why this exists: validate-manifests.sh treats those digests as a required content +# gate, but nothing ever wrote them. A branch that legitimately changes a bundled +# agent, skill, or runtime asset โ€” the daily agent-skills sync being the standing +# case โ€” therefore produces a manifest its own repository rejects, and no amount of +# re-running the sync fixes it. Only a hand edit did, and a hand edit on a generated +# branch is force-pushed away on the next sync with no signal that it happened. +# +# The digest helpers are sourced from scripts/sha256.lib.sh, the same file +# validate-manifests.sh sources, so the value written here and the value demanded +# there cannot drift apart. +# +# Operates on the current working directory (run from the repo root, exactly as CI +# does). Idempotent: a second run over an already-current tree writes nothing. +# +# Usage: +# ./scripts/refresh-desired-state-digests.sh # rewrite stale digests in place +# ./scripts/refresh-desired-state-digests.sh --check # report drift, write nothing +# +# Exit codes: +# 0 every digest is current (--check), or every stale digest was rewritten +# 1 --check found drift, or a declared digest's target file is missing +# 2 usage error, or a required tool is unavailable +set -euo pipefail + +# shellcheck source=scripts/sha256.lib.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/sha256.lib.sh" + +mode="write" +case "${1-}" in + "") ;; + --check) mode="check" ;; + *) + echo "usage: refresh-desired-state-digests.sh [--check]" >&2 + exit 2 + ;; +esac +if [ "$#" -gt 1 ]; then + echo "usage: refresh-desired-state-digests.sh [--check]" >&2 + exit 2 +fi + +for tool in jq perl awk; do + command -v "$tool" > /dev/null 2>&1 || { + echo "::error::refresh-desired-state-digests: required tool not found: $tool" >&2 + exit 2 + } +done + +drift=0 +missing=0 + +# Resolve one declared digest against the file it pins. Emits nothing and returns 1 +# when the target is absent, so a missing file fails closed here instead of being +# papered over with a digest of nothing. +digest_for() { + local target="$1" resource="$2" field="$3" + if [ ! -f "$target" ]; then + echo "::error::$resource: $field pins a file that does not exist: $target" >&2 + return 1 + fi + sha256_file "$target" +} + +while IFS= read -r resource; do + [ -n "$resource" ] || continue + if ! jq -e . "$resource" > /dev/null 2>&1; then + echo "::error::$resource: not valid JSON โ€” refusing to rewrite" >&2 + missing=1 + continue + fi + + # plugins//resources/.desired-state.json -> plugins/ + resource_dir=${resource%/*} + plugin_dir=${resource_dir%/*} + + args=() + program='.' + + entrypoint=$(jq -r '.spec.source.entrypoint // ""' "$resource") + if jq -e 'has("spec") and (.spec | has("source")) and (.spec.source | has("entrypointSha256"))' \ + "$resource" > /dev/null && [ -n "$entrypoint" ]; then + if value=$(digest_for "$plugin_dir/agents/$entrypoint.agent.md" "$resource" entrypointSha256); then + args+=(--arg entrypointSha256 "$value") + program="$program | .spec.source.entrypointSha256 = \$entrypointSha256" + else + missing=1 + fi + fi + + # Every role that pins its own definition or skill file. Driven off the keys the + # resource actually declares, so a new role's definitionSha256 inherits the generator + # without an edit. skillSha256 is deliberately not generalized: validate-manifests.sh + # resolves it to one hard-coded bundled skill, and a generator that guessed a + # different path would write a digest that gate never reads. + while IFS=$'\t' read -r role field relative; do + [ -n "$role" ] || continue + if [ "$relative" = "!UNMAPPED" ]; then + # The validator resolves each digest field to one specific bundled path. A field + # this generator cannot map to that same path would be written with a value the + # gate never checks, so refuse rather than write a plausible wrong digest. + echo "::error::$resource: $role.$field has no known source path in this generator โ€” teach it the mapping validate-manifests.sh uses" >&2 + missing=1 + continue + fi + if value=$(digest_for "$plugin_dir/$relative" "$resource" "$role.$field"); then + key="role_${role//-/_}_$field" + args+=(--arg "$key" "$value") + program="$program | .spec.roles[\"$role\"].$field = \$$key" + else + missing=1 + fi + done < <( + jq -r ' + (.spec.roles // {}) + | to_entries[] + | . as $entry + | ( + (if ($entry.value | has("definitionSha256")) + then [$entry.key, "definitionSha256", "agents/\($entry.key).agent.md"] + else empty end), + (if ($entry.value | has("skillSha256")) + then (if $entry.key == "agent-improver" + then [$entry.key, "skillSha256", "skills/agent-improvement/SKILL.md"] + else [$entry.key, "skillSha256", "!UNMAPPED"] end) + else empty end) + ) + | @tsv + ' "$resource" + ) + + # Runtime assets are hashed as exact bytes: they are executed from the checkout, so a + # checkout-only CRLF change must invalidate the digest rather than be normalized away. + asset_map='{}' + while IFS= read -r asset_path; do + [ -n "$asset_path" ] || continue + if [ ! -f "$plugin_dir/$asset_path" ]; then + echo "::error::$resource: requiredRuntimeAssets pins a file that does not exist: $asset_path" >&2 + missing=1 + continue + fi + asset_map=$( + jq -c --arg p "$asset_path" --arg s "$(sha256_bytes "$plugin_dir/$asset_path")" \ + '.[$p] = $s' <<< "$asset_map" + ) + done < <(jq -r '.spec.source.requiredRuntimeAssets[]?.path // empty' "$resource") + + if [ "$asset_map" != '{}' ]; then + args+=(--argjson assetDigests "$asset_map") + program="$program | .spec.source.requiredRuntimeAssets |= map(.sha256 = (\$assetDigests[.path] // .sha256))" + fi + + if [ "${#args[@]}" -eq 0 ]; then + continue + fi + + updated=$(jq "${args[@]}" "$program" "$resource") + + if [ "$updated" = "$(cat "$resource")" ]; then + continue + fi + + drift=1 + if [ "$mode" = "check" ]; then + echo "::error::$resource: declared digests are stale โ€” run ./scripts/refresh-desired-state-digests.sh" >&2 + continue + fi + + printf '%s\n' "$updated" > "$resource" + echo "โœ“ refreshed $resource" +done < <(find plugins -type f -path '*/resources/*.desired-state.json' | sort) + +if [ "$missing" -ne 0 ]; then + exit 1 +fi + +if [ "$mode" = "check" ] && [ "$drift" -ne 0 ]; then + exit 1 +fi + +if [ "$mode" = "write" ] && [ "$drift" -eq 0 ]; then + echo "โœ“ every declared desired-state digest is already current" +fi diff --git a/scripts/refresh-desired-state-digests.test.sh b/scripts/refresh-desired-state-digests.test.sh new file mode 100755 index 0000000..c413ca0 --- /dev/null +++ b/scripts/refresh-desired-state-digests.test.sh @@ -0,0 +1,211 @@ +#!/usr/bin/env bash +# Self-test for refresh-desired-state-digests.sh. +# +# Proves the generator writes exactly the values validate-manifests.sh demands โ€” the +# whole point of the pair, since a generator that disagrees with the gate leaves the +# branch just as unmergeable as having no generator at all. So the coupling is asserted +# against the REAL repository tree and the REAL validator, not against a restatement of +# the hashing rule: a skill change is simulated, the validator is shown to reject it, +# the generator is run, and the validator is shown to accept it. The ablation is the +# same scenario without the refresh, which must still fail. +# +# Also proves the two hashing rules stay distinct (definition files normalize CRLF, +# runtime assets do not), that the pass is idempotent and format-preserving, and that +# every unknown or unresolvable digest fails closed rather than being written wrong. +# +# Self-contained: throwaway copies, the REAL scripts, no network. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +REFRESH="$SCRIPT_DIR/refresh-desired-state-digests.sh" + +pass=0 +fail=0 +ok() { echo " โœ“ $1"; pass=$((pass + 1)); } +ko() { echo " โœ— $1"; fail=$((fail + 1)); } + +tmproot=$(mktemp -d) +trap 'rm -rf "$tmproot"' EXIT +fresh() { mktemp -d "$tmproot/case.XXXXXX"; } + +sha_norm() { + LC_ALL=C PERL5OPT='' PERL_UNICODE='' PERLIO='' perl -C0 -pe \ + 'BEGIN { binmode STDIN, ":raw"; binmode STDOUT, ":raw" } s/\r\n/\n/g' \ + < "$1" | shasum -a 256 | awk '{ print $1 }' +} +sha_raw() { shasum -a 256 "$1" | awk '{ print $1 }'; } + +ZERO=0000000000000000000000000000000000000000000000000000000000000000 + +# A minimal tree carrying only what the generator reads: an entrypoint agent, one role +# definition, one bundled skill, and one runtime asset. +make_fixture() { + local root="$1" + mkdir -p "$root/plugins/alpha/agents" \ + "$root/plugins/alpha/skills/agent-improvement" \ + "$root/plugins/alpha/scripts" \ + "$root/plugins/alpha/resources" + printf -- '---\nname: alpha-entry\n---\nentry body\n' > "$root/plugins/alpha/agents/alpha-entry.agent.md" + printf -- '---\nname: agent-improver\n---\nimprover body\n' > "$root/plugins/alpha/agents/agent-improver.agent.md" + printf -- '---\nname: agent-improvement\n---\nskill body\n' > "$root/plugins/alpha/skills/agent-improvement/SKILL.md" + printf '#!/usr/bin/env bash\necho asset\n' > "$root/plugins/alpha/scripts/asset.sh" + chmod +x "$root/plugins/alpha/scripts/asset.sh" + cat > "$root/plugins/alpha/resources/provider-neutral.desired-state.json" < /dev/null 2>&1 ); rc=$? +if [ "$rc" -eq 0 ]; then ok "rewrites a fully stale resource and exits 0"; else ko "rewrites a fully stale resource and exits 0 (got $rc)"; fi + +want=$(sha_norm "$d/plugins/alpha/agents/alpha-entry.agent.md") +if [ "$(field "$d" '.spec.source.entrypointSha256')" = "$want" ]; then + ok "entrypointSha256 is the entrypoint agent's normalized digest" +else ko "entrypointSha256 is the entrypoint agent's normalized digest"; fi + +want=$(sha_norm "$d/plugins/alpha/agents/agent-improver.agent.md") +if [ "$(field "$d" '.spec.roles["agent-improver"].definitionSha256')" = "$want" ]; then + ok "definitionSha256 resolves to agents/.agent.md" +else ko "definitionSha256 resolves to agents/.agent.md"; fi + +want=$(sha_norm "$d/plugins/alpha/skills/agent-improvement/SKILL.md") +if [ "$(field "$d" '.spec.roles["agent-improver"].skillSha256')" = "$want" ]; then + ok "skillSha256 resolves to the bundled agent-improvement skill" +else ko "skillSha256 resolves to the bundled agent-improvement skill"; fi + +want=$(sha_raw "$d/plugins/alpha/scripts/asset.sh") +if [ "$(field "$d" '.spec.source.requiredRuntimeAssets[0].sha256')" = "$want" ]; then + ok "runtime asset digest is the exact bytes" +else ko "runtime asset digest is the exact bytes"; fi + +# --- the two hashing rules must stay different ---------------------------------------- +# A definition file differing only by CRLF keeps its digest; a runtime asset does not. +# Collapsing the two would silently accept a checkout-only change to an executed script. +d=$(fresh); make_fixture "$d" +( cd "$d" && "$REFRESH" > /dev/null 2>&1 ) +before_def=$(field "$d" '.spec.roles["agent-improver"].definitionSha256') +before_asset=$(field "$d" '.spec.source.requiredRuntimeAssets[0].sha256') +perl -pe 's/\n/\r\n/' < "$d/plugins/alpha/agents/agent-improver.agent.md" > "$d/crlf.tmp" +mv "$d/crlf.tmp" "$d/plugins/alpha/agents/agent-improver.agent.md" +perl -pe 's/\n/\r\n/' < "$d/plugins/alpha/scripts/asset.sh" > "$d/crlf2.tmp" +cat "$d/crlf2.tmp" > "$d/plugins/alpha/scripts/asset.sh" +( cd "$d" && "$REFRESH" > /dev/null 2>&1 ) +if [ "$(field "$d" '.spec.roles["agent-improver"].definitionSha256')" = "$before_def" ]; then + ok "a CRLF-only change leaves a definition digest unchanged" +else ko "a CRLF-only change leaves a definition digest unchanged"; fi +if [ "$(field "$d" '.spec.source.requiredRuntimeAssets[0].sha256')" != "$before_asset" ]; then + ok "a CRLF-only change DOES change a runtime asset digest" +else ko "a CRLF-only change DOES change a runtime asset digest"; fi + +# --- idempotence and format preservation --------------------------------------------- +d=$(fresh); make_fixture "$d" +( cd "$d" && "$REFRESH" > /dev/null 2>&1 ) +cp "$d/plugins/alpha/resources/provider-neutral.desired-state.json" "$d/first.json" +( cd "$d" && "$REFRESH" > /dev/null 2>&1 ); rc=$? +if [ "$rc" -eq 0 ] && cmp -s "$d/first.json" "$d/plugins/alpha/resources/provider-neutral.desired-state.json"; then + ok "a second run over a current tree writes nothing" +else ko "a second run over a current tree writes nothing (rc=$rc)"; fi + +# Only the digest values may move: a generator that reformatted the document would make +# every sync PR an unreviewable whole-file diff. +d=$(fresh); make_fixture "$d" +cp "$d/plugins/alpha/resources/provider-neutral.desired-state.json" "$d/before.json" +( cd "$d" && "$REFRESH" > /dev/null 2>&1 ) +changed=$(diff "$d/before.json" "$d/plugins/alpha/resources/provider-neutral.desired-state.json" \ + | grep -c '^[<>]') +if [ "$changed" -eq 8 ]; then + ok "only the four digest lines change (4 removed + 4 added)" +else ko "only the four digest lines change (got $changed changed lines)"; fi + +# --- --check reports without writing --------------------------------------------------- +d=$(fresh); make_fixture "$d" +cp "$d/plugins/alpha/resources/provider-neutral.desired-state.json" "$d/before.json" +( cd "$d" && "$REFRESH" --check > /dev/null 2>&1 ); rc=$? +if [ "$rc" -eq 1 ] && cmp -s "$d/before.json" "$d/plugins/alpha/resources/provider-neutral.desired-state.json"; then + ok "--check reports drift (exit 1) and writes nothing" +else ko "--check reports drift (exit 1) and writes nothing (rc=$rc)"; fi + +( cd "$d" && "$REFRESH" > /dev/null 2>&1 ) +( cd "$d" && "$REFRESH" --check > /dev/null 2>&1 ); rc=$? +if [ "$rc" -eq 0 ]; then ok "--check exits 0 on a current tree"; else ko "--check exits 0 on a current tree (got $rc)"; fi + +# --- fail closed ----------------------------------------------------------------------- +d=$(fresh); make_fixture "$d" +rm "$d/plugins/alpha/skills/agent-improvement/SKILL.md" +( cd "$d" && "$REFRESH" > "$d/out" 2>&1 ); rc=$? +if [ "$rc" -eq 1 ] && grep -q 'pins a file that does not exist' "$d/out"; then + ok "a digest whose target file is missing fails closed" +else ko "a digest whose target file is missing fails closed (rc=$rc)"; fi + +d=$(fresh); make_fixture "$d" +J="$d/plugins/alpha/resources/provider-neutral.desired-state.json" +jq '.spec.roles["some-other-role"] = {"skillSha256": "'"$ZERO"'"}' "$J" > "$J.tmp" && mv "$J.tmp" "$J" +( cd "$d" && "$REFRESH" > "$d/out" 2>&1 ); rc=$? +if [ "$rc" -eq 1 ] && grep -q 'no known source path in this generator' "$d/out"; then + ok "a skillSha256 on an unmapped role fails closed instead of guessing" +else ko "a skillSha256 on an unmapped role fails closed instead of guessing (rc=$rc)"; fi +if [ "$(jq -r '.spec.roles["some-other-role"].skillSha256' "$J")" = "$ZERO" ]; then + ok "the unmapped role's digest is left untouched" +else ko "the unmapped role's digest is left untouched"; fi + +d=$(fresh); make_fixture "$d" +( cd "$d" && "$REFRESH" --bogus > /dev/null 2>&1 ); rc=$? +if [ "$rc" -eq 2 ]; then ok "an unknown flag exits 2"; else ko "an unknown flag exits 2 (got $rc)"; fi +( cd "$d" && "$REFRESH" --check extra > /dev/null 2>&1 ); rc=$? +if [ "$rc" -eq 2 ]; then ok "a surplus argument exits 2"; else ko "a surplus argument exits 2 (got $rc)"; fi + +# --- the coupling this pair exists for, against the REAL validator --------------------- +# Reproduces the deadlock: a synced skill change the sync workflow cannot follow up on. +d=$(fresh) +tar -cf - -C "$REPO_ROOT" --exclude='./.git' . 2> /dev/null | tar -xf - -C "$d" +SKILL="$d/plugins/agentic-engineering/skills/agent-improvement/SKILL.md" +if [ ! -f "$SKILL" ]; then + ko "fixture copy carries the bundled agent-improvement skill" +else + ( cd "$d" && ./scripts/validate-manifests.sh > /dev/null 2>&1 ); rc=$? + if [ "$rc" -eq 0 ]; then ok "the untouched copy validates clean (control)"; else ko "the untouched copy validates clean (control, rc=$rc)"; fi + + printf '\n\n' >> "$SKILL" + ( cd "$d" && ./scripts/validate-manifests.sh > "$d/v1" 2>&1 ); rc=$? + if [ "$rc" -ne 0 ] && grep -q 'agent-improvement skill digest must match the bundled skill' "$d/v1"; then + ok "ABLATION: a synced skill change without a refresh still fails the gate" + else ko "ABLATION: a synced skill change without a refresh still fails the gate (rc=$rc)"; fi + + ( cd "$d" && ./scripts/refresh-desired-state-digests.sh > /dev/null 2>&1 ); rc=$? + if [ "$rc" -eq 0 ]; then ok "the generator refreshes the real resource"; else ko "the generator refreshes the real resource (rc=$rc)"; fi + + ( cd "$d" && ./scripts/validate-manifests.sh > "$d/v2" 2>&1 ); rc=$? + if [ "$rc" -eq 0 ]; then + ok "refresh then validate is clean โ€” the deadlock is resolved without a hand edit" + else ko "refresh then validate is clean (rc=$rc): $(tail -1 "$d/v2")"; fi +fi + +echo "refresh-desired-state-digests.sh self-test: $pass passed, $fail failed" +[ "$fail" -eq 0 ] diff --git a/scripts/sha256.lib.sh b/scripts/sha256.lib.sh new file mode 100644 index 0000000..7470960 --- /dev/null +++ b/scripts/sha256.lib.sh @@ -0,0 +1,34 @@ +# shellcheck shell=bash +# Digest helpers shared by the desired-state validator and its generator. +# +# The generator writes exactly what the validator checks, so the two must hash a file +# identically. Sourcing one definition is what makes that true by construction: a +# second copy could drift silently, and the only symptom would be a required check +# that no amount of regeneration can satisfy. + +# Hash definition-file bytes after normalizing checkout-only CRLF pairs to committed LF +# bytes. Clear inherited Perl I/O controls and set both stream handles to raw bytes +# explicitly. This preserves invalid UTF-8, NULs, lone CRs, and a missing final newline +# instead of decoding or reconstructing the file as text. +sha256_file() { + if command -v sha256sum > /dev/null 2>&1; then + LC_ALL=C PERL5OPT='' PERL_UNICODE='' PERLIO='' perl -C0 -pe \ + 'BEGIN { binmode STDIN, ":raw"; binmode STDOUT, ":raw" } s/\r\n/\n/g' \ + < "$1" | sha256sum | awk '{ print $1 }' + else + LC_ALL=C PERL5OPT='' PERL_UNICODE='' PERLIO='' perl -C0 -pe \ + 'BEGIN { binmode STDIN, ":raw"; binmode STDOUT, ":raw" } s/\r\n/\n/g' \ + < "$1" | shasum -a 256 | awk '{ print $1 }' + fi +} + +# Hash the exact bytes of an executable runtime asset. Unlike definition files, +# runtime assets are executed from the checkout, so checkout-only CRLF changes +# must invalidate the declared digest instead of being normalized away. +sha256_bytes() { + if command -v sha256sum > /dev/null 2>&1; then + sha256sum "$1" | awk '{ print $1 }' + else + shasum -a 256 "$1" | awk '{ print $1 }' + fi +} diff --git a/scripts/validate-manifests.sh b/scripts/validate-manifests.sh index 5c4e303..f4ba5e1 100755 --- a/scripts/validate-manifests.sh +++ b/scripts/validate-manifests.sh @@ -28,32 +28,11 @@ CLAUDE_MANIFEST=".claude-plugin/marketplace.json" RENAME_HISTORY="scripts/marketplace-rename-history.json" README="README.md" -# Hash entrypoint bytes after normalizing checkout-only CRLF pairs to committed LF bytes. -# Clear inherited Perl I/O controls and set both stream handles to raw bytes explicitly. -# This preserves invalid UTF-8, NULs, lone CRs, and a missing final newline instead of -# decoding or reconstructing the file as text. -sha256_file() { - if command -v sha256sum > /dev/null 2>&1; then - LC_ALL=C PERL5OPT='' PERL_UNICODE='' PERLIO='' perl -C0 -pe \ - 'BEGIN { binmode STDIN, ":raw"; binmode STDOUT, ":raw" } s/\r\n/\n/g' \ - < "$1" | sha256sum | awk '{ print $1 }' - else - LC_ALL=C PERL5OPT='' PERL_UNICODE='' PERLIO='' perl -C0 -pe \ - 'BEGIN { binmode STDIN, ":raw"; binmode STDOUT, ":raw" } s/\r\n/\n/g' \ - < "$1" | shasum -a 256 | awk '{ print $1 }' - fi -} - -# Hash the exact bytes of an executable runtime asset. Unlike definition files, -# runtime assets are executed from the checkout, so checkout-only CRLF changes -# must invalidate the declared digest instead of being normalized away. -sha256_bytes() { - if command -v sha256sum > /dev/null 2>&1; then - sha256sum "$1" | awk '{ print $1 }' - else - shasum -a 256 "$1" | awk '{ print $1 }' - fi -} +# Digest helpers are shared with the desired-state digest generator, so the value this +# gate demands and the value that generator writes cannot drift apart. See +# scripts/sha256.lib.sh. +# shellcheck source=scripts/sha256.lib.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/sha256.lib.sh" # 1. A marketplace manifest must parse and carry both required top-level keys. validate_marketplace_json() { From f6cbd533f69649365ae93278db41a73a5b6cc7d1 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 1 Sep 2026 09:32:34 +0200 Subject: [PATCH 2/3] fix(scripts): fail closed on a digest nothing can resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both the same class the generator exists to remove โ€” a pass reporting success over input it never examined. A declared entrypointSha256 with an empty entrypoint, and a requiredRuntimeAssets entry carrying a digest but no path, were each skipped silently. With no other field stale the run then exited 0 and printed "every declared desired-state digest is already current", over a digest nothing had looked at. Both are now invalid input. The required-tool check also did not require a SHA-256 program. Without one, sha256_file simply failed and digest_for reported it as an absent target, so the run exited 1 blaming a file that was present โ€” a misdiagnosis costing more than the failure it hid. It is an environment error and now exits 2, as documented. Each guard has a regression test, and each was ablated to confirm its test binds to it: removing the hasher check gives exit 1 instead of 2, and restoring either silent skip reproduces the exit-0-over-an-unexamined-digest it replaced. --- scripts/refresh-desired-state-digests.sh | 27 ++++++++++++++--- scripts/refresh-desired-state-digests.test.sh | 30 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/scripts/refresh-desired-state-digests.sh b/scripts/refresh-desired-state-digests.sh index dcc87f1..0679f25 100755 --- a/scripts/refresh-desired-state-digests.sh +++ b/scripts/refresh-desired-state-digests.sh @@ -50,6 +50,14 @@ for tool in jq perl awk; do } done +# A missing hasher is an environment failure, not a finding about the tree. Without this check +# sha256_file simply fails, digest_for reports it as an absent target, and the run exits 1 blaming +# a file that is present โ€” the misdiagnosis costing more than the failure. +if ! command -v sha256sum > /dev/null 2>&1 && ! command -v shasum > /dev/null 2>&1; then + echo "::error::refresh-desired-state-digests: no SHA-256 program found (need sha256sum or shasum)" >&2 + exit 2 +fi + drift=0 missing=0 @@ -82,8 +90,13 @@ while IFS= read -r resource; do entrypoint=$(jq -r '.spec.source.entrypoint // ""' "$resource") if jq -e 'has("spec") and (.spec | has("source")) and (.spec.source | has("entrypointSha256"))' \ - "$resource" > /dev/null && [ -n "$entrypoint" ]; then - if value=$(digest_for "$plugin_dir/agents/$entrypoint.agent.md" "$resource" entrypointSha256); then + "$resource" > /dev/null; then + if [ -z "$entrypoint" ]; then + # Declared but unresolvable. Skipping it would exit 0 over a digest nothing examined โ€” the + # exact shape of failure this generator exists to remove, one level up. + echo "::error::$resource: entrypointSha256 is declared but entrypoint is empty, so nothing resolves it" >&2 + missing=1 + elif value=$(digest_for "$plugin_dir/agents/$entrypoint.agent.md" "$resource" entrypointSha256); then args+=(--arg entrypointSha256 "$value") program="$program | .spec.source.entrypointSha256 = \$entrypointSha256" else @@ -136,7 +149,13 @@ while IFS= read -r resource; do # checkout-only CRLF change must invalidate the digest rather than be normalized away. asset_map='{}' while IFS= read -r asset_path; do - [ -n "$asset_path" ] || continue + if [ -z "$asset_path" ]; then + # An entry with a declared digest and no path is unverifiable, so filtering it out would + # again exit 0 over something never examined. + echo "::error::$resource: a requiredRuntimeAssets entry declares no path, so nothing resolves its digest" >&2 + missing=1 + continue + fi if [ ! -f "$plugin_dir/$asset_path" ]; then echo "::error::$resource: requiredRuntimeAssets pins a file that does not exist: $asset_path" >&2 missing=1 @@ -146,7 +165,7 @@ while IFS= read -r resource; do jq -c --arg p "$asset_path" --arg s "$(sha256_bytes "$plugin_dir/$asset_path")" \ '.[$p] = $s' <<< "$asset_map" ) - done < <(jq -r '.spec.source.requiredRuntimeAssets[]?.path // empty' "$resource") + done < <(jq -r '.spec.source.requiredRuntimeAssets[]? | .path // ""' "$resource") if [ "$asset_map" != '{}' ]; then args+=(--argjson assetDigests "$asset_map") diff --git a/scripts/refresh-desired-state-digests.test.sh b/scripts/refresh-desired-state-digests.test.sh index c413ca0..820467b 100755 --- a/scripts/refresh-desired-state-digests.test.sh +++ b/scripts/refresh-desired-state-digests.test.sh @@ -175,6 +175,36 @@ if [ "$(jq -r '.spec.roles["some-other-role"].skillSha256' "$J")" = "$ZERO" ]; t ok "the unmapped role's digest is left untouched" else ko "the unmapped role's digest is left untouched"; fi +# A declared digest that nothing resolves must not be silently skipped: exiting 0 there would +# report "every digest is current" over one the generator never examined, which is the same +# shape of failure this generator exists to remove one level up. +d=$(fresh); make_fixture "$d" +J="$d/plugins/alpha/resources/provider-neutral.desired-state.json" +jq '.spec.source.entrypoint = ""' "$J" > "$J.tmp" && mv "$J.tmp" "$J" +( cd "$d" && "$REFRESH" > "$d/out" 2>&1 ); rc=$? +if [ "$rc" -eq 1 ] && grep -q 'entrypointSha256 is declared but entrypoint is empty' "$d/out"; then + ok "entrypointSha256 with no entrypoint fails closed instead of exiting 0" +else ko "entrypointSha256 with no entrypoint fails closed instead of exiting 0 (rc=$rc)"; fi + +d=$(fresh); make_fixture "$d" +J="$d/plugins/alpha/resources/provider-neutral.desired-state.json" +jq 'del(.spec.source.requiredRuntimeAssets[0].path)' "$J" > "$J.tmp" && mv "$J.tmp" "$J" +( cd "$d" && "$REFRESH" > "$d/out" 2>&1 ); rc=$? +if [ "$rc" -eq 1 ] && grep -q 'declares no path' "$d/out"; then + ok "a runtime asset with a digest but no path fails closed" +else ko "a runtime asset with a digest but no path fails closed (rc=$rc)"; fi + +# A missing hasher is an environment failure (exit 2), not a claim that a present file is absent. +d=$(fresh); make_fixture "$d" +mkdir -p "$d/onlybin" +for t in bash env jq perl awk find sort dirname; do + real=$(command -v "$t" 2> /dev/null) && ln -sf "$real" "$d/onlybin/$t" +done +( cd "$d" && PATH="$d/onlybin" "$REFRESH" > "$d/out" 2>&1 ); rc=$? +if [ "$rc" -eq 2 ] && grep -q 'no SHA-256 program found' "$d/out"; then + ok "no sha256sum and no shasum exits 2, not a bogus missing-file exit 1" +else ko "no sha256sum and no shasum exits 2, not a bogus missing-file exit 1 (rc=$rc): $(head -1 "$d/out")"; fi + d=$(fresh); make_fixture "$d" ( cd "$d" && "$REFRESH" --bogus > /dev/null 2>&1 ); rc=$? if [ "$rc" -eq 2 ]; then ok "an unknown flag exits 2"; else ko "an unknown flag exits 2 (got $rc)"; fi From 746612cd5237fbe00073c737e6448e070988be35 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 1 Sep 2026 10:35:43 +0200 Subject: [PATCH 3/3] fix(scripts): fail closed when the enumeration matches nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while exercising the writer as its user: run where nothing matches, it printed "every declared desired-state digest is already current" and exited 0 while `find` had written "No such file or directory" and the loop ran zero times. Reporting success over a tree it never examined is the exact failure class this script exists to remove โ€” the third instance of it in this PR. The enumeration is cwd-relative BY DESIGN (the self-test exercises the script against synthetic trees), so this does not anchor the cwd; anchoring it broke 11 of those cases. It counts what the loop actually saw and treats zero as an environment error (exit 2), matching the convention used for a missing tool or hasher. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/refresh-desired-state-digests.sh | 10 +++++++ scripts/refresh-desired-state-digests.test.sh | 26 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/scripts/refresh-desired-state-digests.sh b/scripts/refresh-desired-state-digests.sh index 0679f25..939c30d 100755 --- a/scripts/refresh-desired-state-digests.sh +++ b/scripts/refresh-desired-state-digests.sh @@ -60,6 +60,7 @@ fi drift=0 missing=0 +seen=0 # Resolve one declared digest against the file it pins. Emits nothing and returns 1 # when the target is absent, so a missing file fails closed here instead of being @@ -74,6 +75,7 @@ digest_for() { } while IFS= read -r resource; do + seen=$((seen + 1)) [ -n "$resource" ] || continue if ! jq -e . "$resource" > /dev/null 2>&1; then echo "::error::$resource: not valid JSON โ€” refusing to rewrite" >&2 @@ -192,6 +194,14 @@ while IFS= read -r resource; do echo "โœ“ refreshed $resource" done < <(find plugins -type f -path '*/resources/*.desired-state.json' | sort) +# Zero resources is never a legitimate clean run: this repository always declares at least one. +# Without this, an enumeration that matched nothing is indistinguishable from one that matched +# everything and found it current โ€” the same success-over-nothing shape guarded against above. +if [ "$seen" -eq 0 ]; then + echo "::error::refresh-desired-state-digests: no *.desired-state.json resource found under plugins/" >&2 + exit 2 +fi + if [ "$missing" -ne 0 ]; then exit 1 fi diff --git a/scripts/refresh-desired-state-digests.test.sh b/scripts/refresh-desired-state-digests.test.sh index 820467b..f32adf9 100755 --- a/scripts/refresh-desired-state-digests.test.sh +++ b/scripts/refresh-desired-state-digests.test.sh @@ -237,5 +237,31 @@ else else ko "refresh then validate is clean (rc=$rc): $(tail -1 "$d/v2")"; fi fi +# --------------------------------------------------------------------------- +# Reporting success over a tree it never examined is the exact failure this script +# exists to remove, so it must not commit that failure itself. The enumeration is +# cwd-relative BY DESIGN (the cases above exercise the script against synthetic +# trees), but its `find` runs inside a process substitution whose failure neither +# `set -e` nor the loop's exit status observes. Run where nothing matches, it +# printed "every declared desired-state digest is already current" and exited 0. +# Zero resources is never a clean run. +# --------------------------------------------------------------------------- +d=$(fresh) +out=$( cd "$d" && "$REFRESH" 2>&1 ); rc=$? +if [ "$rc" -eq 2 ] && printf '%s' "$out" | grep -q 'no \*.desired-state.json resource found'; then + ok "an enumeration matching nothing fails closed instead of reporting success" +else + ko "an enumeration matching nothing fails closed (rc=$rc): $out" +fi + +d=$(fresh) +mkdir -p "$d/plugins" +out=$( cd "$d" && "$REFRESH" --check 2>&1 ); rc=$? +if [ "$rc" -eq 2 ]; then + ok "--check also fails closed on an empty enumeration" +else + ko "--check also fails closed on an empty enumeration (rc=$rc): $out" +fi + echo "refresh-desired-state-digests.sh self-test: $pass passed, $fail failed" [ "$fail" -eq 0 ]