From dfb5da59fb8fed14bade180c1f348711417c8e89 Mon Sep 17 00:00:00 2001 From: Deepu S Nath Date: Fri, 31 Jul 2026 23:18:03 +0530 Subject: [PATCH] fix(skill-stocktake): surface find errors instead of swallowing them Following up on the -L fix: find -L can now traverse symlinks, but a broken symlink target or an unreadable directory makes find skip that entry and exit non-zero. Both scripts previously redirected find's stderr to /dev/null and never checked its exit status, so a scan could silently under-count skills with no indication anything was wrong. Capture find's exit status and stderr in both scripts; on failure, print a warning (with the underlying find error) to stderr while still emitting the best-effort results for whatever was found. Verified with a permission-denied skill directory: real BSD find exits 1 and reports "Permission denied" on stderr, now surfaced as an explicit warning instead of silently dropped. Addresses CodeRabbit review feedback on PR #2640. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FhDjpSfrbPpnpqT3CZBEX1 --- skills/skill-stocktake/scripts/quick-diff.sh | 13 ++++++++++++- skills/skill-stocktake/scripts/scan.sh | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index 75dbac273..a2177b39a 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -51,6 +51,17 @@ i=0 process_dir() { local dir="$1" + local find_out="$tmpdir/.find-stdout" + local find_err="$tmpdir/.find-stderr" + # Capture find's exit status and stderr instead of discarding them: with -L, + # a broken symlink or unreadable directory makes find skip that entry AND + # exit non-zero, which would otherwise silently under-count skills. + if ! find -L "$dir" -name "SKILL.md" -type f >"$find_out" 2>"$find_err"; then + echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 + cat "$find_err" >&2 + fi + sort -o "$find_out" "$find_out" + while IFS= read -r file; do local mtime dp is_new mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) @@ -74,7 +85,7 @@ process_dir() { '{path:$path,mtime:$mtime,is_new:$is_new}' \ > "$tmpdir/$i.json" i=$((i+1)) - done < <(find -L "$dir" -name "SKILL.md" -type f 2>/dev/null | sort) + done < "$find_out" } [[ -d "$GLOBAL_DIR" ]] && process_dir "$GLOBAL_DIR" diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index 9a5aca497..4197fff96 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -95,6 +95,17 @@ scan_dir_to_json() { fi local i=0 + local find_out="$tmpdir/.find-stdout" + local find_err="$tmpdir/.find-stderr" + # Capture find's exit status and stderr instead of discarding them: with -L, + # a broken symlink or unreadable directory makes find skip that entry AND + # exit non-zero, which would otherwise silently under-count skills. + if ! find -L "$dir" -name "SKILL.md" -type f >"$find_out" 2>"$find_err"; then + echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 + cat "$find_err" >&2 + fi + sort -o "$find_out" "$find_out" + while IFS= read -r file; do local name desc mtime u7 u30 dp name=$(extract_field "$file" "name") @@ -118,7 +129,7 @@ scan_dir_to_json() { '{path:$path,name:$name,description:$description,use_7d:$use_7d,use_30d:$use_30d,mtime:$mtime}' \ > "$tmpdir/$i.json" i=$((i+1)) - done < <(find -L "$dir" -name "SKILL.md" -type f 2>/dev/null | sort) + done < "$find_out" if [[ $i -eq 0 ]]; then echo "[]"