fix(scripts): address CodeRabbit PR #85 review — 4 findings (#86)

- generate-marketplace.sh: set -o pipefail (added in #85) made
  find|wc -l on a missing skills/ dir abort the whole generator.
  Append || true so a plugin lacking skills/ reports count 0.
- generate-marketplace.sh: reject an empty marketplace (plugin_count=0)
  before publishing — all-skipped runs previously produced {"plugins":[]}
  violating the schema minItems. Now exits 1.
- review-skill.sh: strip YAML trailing comment/whitespace BEFORE quotes
  so 'value' # comment and "value" # comment both parse; the prior order
  left a stray closing quote on single-quoted-with-comment values.
- sync-plugins.sh: fold mv into the jq && jq && mv chain so an mv failure
  routes to the else-cleanup branch instead of aborting under set -e.

All verified: pipefail missing-dir survives; empty-mp guard fires;
quote-strip handles 5 forms; sync dry-run green; validators green.
This commit is contained in:
Eddie
2026-08-08 16:43:56 +02:00
committed by GitHub
parent fb6e3f1c2b
commit b9627cae5d
3 changed files with 25 additions and 11 deletions
+14 -2
View File
@@ -143,8 +143,11 @@ while IFS= read -r plugin_dir; do
# Track category count
echo "$plugin_name" >> "$TEMP_DIR/$category"
# Count skills in this plugin (for info only, not included in marketplace)
skill_count=$(find "$plugin_dir/skills" -name "SKILL.md" 2>/dev/null | wc -l | tr -d ' ')
# Count skills in this plugin (for info only, not included in marketplace).
# `find` exits non-zero if the directory doesn't exist; under `set -o pipefail`
# that would abort the whole generator. Append `|| true` so a plugin lacking a
# skills/ subdir simply reports count 0 instead of crashing the run.
skill_count=$(find "$plugin_dir/skills" -name "SKILL.md" 2>/dev/null | wc -l | tr -d ' ' || true)
# Add comma before each entry except the first
if [ "$first" = true ]; then
@@ -205,6 +208,15 @@ if command -v jq &> /dev/null; then
if jq empty "$MARKETPLACE_JSON" 2>/dev/null; then
plugin_count=$(jq '.plugins | length' "$MARKETPLACE_JSON")
# Reject an empty marketplace before doing anything else. If every plugin
# was skipped (misconfigured PLUGINS_DIR, all plugin.json invalid, etc.)
# the file would contain `"plugins": []`, violating the marketplace schema
# (minItems) and silently publishing a broken artifact. Fail loudly instead.
if [ "$plugin_count" -eq 0 ]; then
echo "❌ ERROR: Generated marketplace has 0 plugins (all skipped?). Aborting; $MARKETPLACE_JSON left as-is or empty." >&2
exit 1
fi
# Sync top-level metadata.version from plugin entries and ENFORCE lockstep.
# Previously this read `.plugins[0].version` (always the alphabetically-first
# plugin), which silently masked version drift instead of detecting it — the
+5 -3
View File
@@ -212,11 +212,13 @@ check_yaml_frontmatter() {
# Check for last_verified
if echo "$frontmatter" | grep -q "last_verified:"; then
local last_verified=$(echo "$frontmatter" | grep "last_verified:" | sed 's/.*last_verified:[[:space:]]*//')
# Strip surrounding quotes / whitespace so a YAML-quoted scalar like
# `last_verified: "2024-01-15"` parses cleanly.
# Strip trailing YAML comments / whitespace FIRST, then surrounding quotes,
# so values like `last_verified: "2024-01-15" # comment` or
# `'2024-01-15' # note` reduce to a parseable date. Stripping quotes before
# the comment left a stray closing quote in single-quoted-with-comment cases.
last_verified="${last_verified%% *}"
last_verified="${last_verified#\"}"; last_verified="${last_verified%\"}"
last_verified="${last_verified#\'}"; last_verified="${last_verified%\'}"
last_verified="${last_verified%% *}"
# Parse the YYYY-MM-DD date to epoch seconds portably. `date -d` is
# GNU-only and errors on macOS/BSD (silently yielding epoch 0, which
# flagged every skill as ~56 years stale). Use BSD's `date -jf` when on
+6 -6
View File
@@ -728,13 +728,13 @@ while IFS= read -r codex_json; do
fi
codex_count=$((codex_count + 1))
if [ "$DRY_RUN" = false ]; then
# Write to .tmp, VALIDATE with jq, then promote. Previously a failed jq
# left a .tmp behind and still incremented the updated counter (reporting
# "N/N synced" when some had failed), and a malformed result could destroy
# the original via mv with no validation step in between.
# Write to .tmp, VALIDATE with jq, then promote — all in one `&&` chain so
# that a failure at ANY step (jq write, jq validate, OR mv) routes to the
# else branch for cleanup. Previously `mv` was a standalone command inside
# `then`, so an mv failure aborted under set -e instead of being reported.
if jq --arg v "$GLOBAL_VERSION" '.version = $v' "$codex_json" > "$codex_json.tmp" \
&& jq '.' "$codex_json.tmp" > /dev/null 2>&1; then
mv "$codex_json.tmp" "$codex_json"
&& jq '.' "$codex_json.tmp" > /dev/null 2>&1 \
&& mv "$codex_json.tmp" "$codex_json"; then
codex_updated=$((codex_updated + 1))
else
rm -f "$codex_json.tmp"