Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions scripts/check-queue-front-matter.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
#!/usr/bin/env bash
# Consumer-side front-matter validator for file-based markdown handoff queues
# (one item per file with YAML front matter). Detects malformed items that make
# a `grep '^status:'` reconciliation report a false all-clear (#1647).
#
# scripts/check-queue-front-matter.sh <queue-dir>
#
# Not a CI gate — the queue lives outside the repository. Invoke at claim time
# when an agent decides whether work is present.
#
# Exit 0 = every item file conforms; 1 = one or more violations; 2 = usage error.
set -euo pipefail

usage() {
printf 'usage: check-queue-front-matter.sh <queue-dir>\n' >&2
exit 2
}

[[ $# -eq 1 ]] || usage
QUEUE_DIR="$1"

if [[ ! -d "$QUEUE_DIR" ]]; then
printf 'Error: queue directory not found: %s\n' "$QUEUE_DIR" >&2
exit 2
fi

VALID_STATUSES='unclaimed|claimed|in-progress|blocked|done'
VALID_PRIORITIES='low|medium|high|urgent'
Comment thread
kyle-sexton marked this conversation as resolved.
REQUIRED_KEYS=(id title status created producer)

errors=0
file_count=0
parsed_count=0

report_violation() {
printf 'VIOLATION: %s — %s\n' "$1" "$2"
errors=$((errors + 1))
}

# extract_front_matter <file> — prints front matter body or nothing.
extract_front_matter() {
awk '
NR == 1 && $0 == "---" { in_fm = 1; next }
Comment thread
kyle-sexton marked this conversation as resolved.
in_fm && $0 == "---" { exit }
in_fm { print }
Comment thread
kyle-sexton marked this conversation as resolved.
' "$1"
}

# fm_value <front_matter> <key>
fm_value() {
awk -v key="$2" '
$1 == key ":" {
sub(/^[^:]*:[[:space:]]*/, "")
print
exit
}
' <<<"$1"
}
Comment on lines +50 to +58

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important — trailing whitespace on a value causes false-positive violations.

sub(/^[^:]*:[[:space:]]*/, "") only strips the leading key: portion; it leaves any trailing whitespace on the line untouched (e.g. a stray trailing space, or a \r if the item file happens to be CRLF-terminated).

That trailing whitespace survives into every comparison that isn't itself whitespace-tolerant:

  • status '$status' enum check at line 82status: unclaimed (trailing space) fails ^(unclaimed|...)$ and is wrongly reported as an invalid status.
  • priority enum check at line 89 — same failure mode.
  • id vs filename-stem check at line 95id: 20260812-sample would be reported as a mismatch against stem 20260812-sample even though they're the same id.

The "missing required key" check at line 75 happens to be immune (it strips all whitespace before testing emptiness), which is likely why this wasn't caught by hand-testing.

A one-line fix in fm_value (e.g. also trimming trailing [[:space:]]*$ in the sub, or piping through something that trims both ends) would remove this whole class of false positive — which matters here specifically because the script's stated purpose is to avoid false readings of queue state.

Fix this →


for item in "$QUEUE_DIR"/*.md; do
[[ -e "$item" ]] || continue
base="$(basename "$item")"
[[ "$base" == README.md ]] && continue
file_count=$((file_count + 1))
stem="${base%.md}"
fm="$(extract_front_matter "$item")"
if [[ -z "${fm//[[:space:]]/}" ]]; then
report_violation "$item" 'missing or empty YAML front matter'
continue
fi
parsed_count=$((parsed_count + 1))

for key in "${REQUIRED_KEYS[@]}"; do
val="$(fm_value "$fm" "$key")"
if [[ -z "${val//[[:space:]]/}" ]]; then
report_violation "$item" "missing required key: $key"
fi
done

status="$(fm_value "$fm" status)"
if [[ -n "$status" ]]; then
if ! grep -qE "^(${VALID_STATUSES})$" <<<"$status"; then
report_violation "$item" "status '$status' not in documented set (unclaimed|claimed|in-progress|blocked|done)"
fi
fi

priority="$(fm_value "$fm" priority)"
if [[ -n "${priority//[[:space:]]/}" ]]; then
if ! grep -qE "^(${VALID_PRIORITIES})$" <<<"$priority"; then
report_violation "$item" "priority '$priority' not in documented set (low|medium|high|urgent)"
fi
fi

id="$(fm_value "$fm" id)"
if [[ -n "$id" && "$id" != "$stem" ]]; then
report_violation "$item" "id '$id' does not match filename stem '$stem'"
fi
done

printf 'Reconciliation: %d item file(s), %d with parseable front matter\n' \
"$file_count" "$parsed_count"

if ((file_count != parsed_count)); then
report_violation "$QUEUE_DIR" \
"count gap — $((file_count - parsed_count)) file(s) lack parseable front matter (never reconcile by status grep alone)"
fi

if ((errors > 0)); then
printf '\n%d violation(s).\n' "$errors" >&2
exit 1
fi

echo "Queue front matter OK."
exit 0
107 changes: 107 additions & 0 deletions scripts/check-queue-front-matter.test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
#!/usr/bin/env bash
# Unit tests for check-queue-front-matter.sh.
set -uo pipefail

SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SCRIPT="$SELF_DIR/check-queue-front-matter.sh"
Comment on lines +1 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important — this test suite doesn't appear to run anywhere.

scripts/run-plugin-tests.sh only discovers plugins/**/*.test.sh and .claude/hooks/*.test.sh (mapfile -t tests < <(find plugins .claude/hooks -type f -name '*.test.sh' ...)), so it never picks up scripts/*.test.sh. Every other scripts/check-*.sh in this repo compensates by getting an explicit run: bash scripts/<name>.test.sh step added to .github/workflows/ci.yml (see e.g. check-silent-skips.test.sh, check-plugin-manifest-presence.test.sh, check-orphaned-fixtures.test.sh). I couldn't find check-queue-front-matter anywhere in .github/workflows/, so this 107-line test file has no CI step invoking it.

The PR's decision brief says the checker itself is intentionally "not wired to CI" because the queue lives outside the repo — but that reasoning doesn't extend to the unit tests: they're fully self-contained (each test does its own mktemp -d and doesn't touch anything outside the repo), just like the sibling tests that do run in CI. As written, a future regression in check-queue-front-matter.sh's parsing logic wouldn't be caught automatically.

Fix this →


PASS=0
FAIL=0
fail() {
echo "FAIL: $*" >&2
FAIL=$((FAIL + 1))
}
ok() {
echo "ok: $*"
PASS=$((PASS + 1))
}

new_queue() {
mktemp -d
}

run_check() (
bash "$SCRIPT" "$1"
)

# --- valid item passes -------------------------------------------------------
q="$(new_queue)"
cat >"$q/20260812-sample.md" <<'EOF'
---
id: 20260812-sample
title: Sample item
status: unclaimed
created: 2026-08-12T12:00:00Z
producer: test-fixture
---
Body
EOF
if run_check "$q" >/dev/null 2>&1; then
ok "valid item passes"
else
fail "valid item should pass"
fi

# --- missing front matter fails ----------------------------------------------
q="$(new_queue)"
printf 'No front matter here\n' >"$q/20260812-bad.md"
if run_check "$q" >/dev/null 2>&1; then
fail "missing front matter should fail"
else
ok "missing front matter fails"
fi

# --- invalid status fails ----------------------------------------------------
q="$(new_queue)"
cat >"$q/20260812-open.md" <<'EOF'
---
id: 20260812-open
title: Bad status
status: open
created: 2026-08-12T12:00:00Z
producer: test-fixture
---
EOF
if run_check "$q" >/dev/null 2>&1; then
fail "invalid status should fail"
else
ok "invalid status fails"
fi

# --- id stem mismatch fails --------------------------------------------------
q="$(new_queue)"
cat >"$q/20260812-wrong.md" <<'EOF'
---
id: other-id
title: Mismatch
status: unclaimed
created: 2026-08-12T12:00:00Z
producer: test-fixture
---
EOF
if run_check "$q" >/dev/null 2>&1; then
fail "id/filename mismatch should fail"
else
ok "id stem mismatch fails"
fi

# --- README.md is ignored ----------------------------------------------------
q="$(new_queue)"
printf '# readme\n' >"$q/README.md"
cat >"$q/20260812-only.md" <<'EOF'
---
id: 20260812-only
title: Only item
status: done
created: 2026-08-12T12:00:00Z
producer: test-fixture
---
EOF
if run_check "$q" >/dev/null 2>&1; then
ok "README.md ignored"
else
fail "README should be ignored: valid sole item should pass"
fi

printf '\n%d passed, %d failed\n' "$PASS" "$FAIL"
[[ "$FAIL" -eq 0 ]]
Comment on lines +100 to +107

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion — coverage gaps relative to the checker's own logic.

The five cases here (valid item, wholesale-missing front matter, bad status, id/stem mismatch, README skip) all just assert pass/fail via exit status. A few things the checker does aren't exercised by any case:

  • No test for an individual missing required key (title/created/producer) when front matter otherwise parses — only "front matter entirely absent" is covered.
  • No test for an out-of-set priority value (the optional-key branch at check-queue-front-matter.sh:87-92).
  • No test asserting the Reconciliation: %d item file(s), %d with parseable front matter line/count-gap violation (check-queue-front-matter.sh:100-106) — this is the PR's headline fix for the "grep '^status:' false all-clear" problem (handoff-queue: consumer-side front-matter validator (a status grep reports a false all-clear on a malformed item) #1647), but nothing here verifies the counts are actually reported correctly for a mixed queue (some valid items + some with no front matter at all).

None of these are required for the PR to be correct, but since this suite won't run in CI yet (see the other comment on this file) and the checker's core value proposition is exactly the reconciliation-count behavior, it'd be worth locking that down with a test before it can silently regress.

Loading