From dddbe52cb7ecf1fa28a2a87804b667ebec5f225b Mon Sep 17 00:00:00 2001 From: Yann Roberto <1922498827@qq.com> Date: Wed, 23 Sep 2026 11:41:37 +0800 Subject: [PATCH] fix(skill-stocktake): exclude trash and mark unmeasured usage --- skills/skill-stocktake/SKILL.md | 6 +- skills/skill-stocktake/scripts/quick-diff.sh | 3 +- skills/skill-stocktake/scripts/scan.sh | 12 +- .../scripts/skill-stocktake-discovery.test.js | 113 +++++++++++++++++- 4 files changed, 127 insertions(+), 7 deletions(-) diff --git a/skills/skill-stocktake/SKILL.md b/skills/skill-stocktake/SKILL.md index 6f207320d..7fc538243 100644 --- a/skills/skill-stocktake/SKILL.md +++ b/skills/skill-stocktake/SKILL.md @@ -20,6 +20,8 @@ The command targets the following paths **relative to the directory where it is **At the start of Phase 1, the command explicitly lists which paths were found and scanned.** +Directories named `.trash` are excluded from both scan modes: archived skills are not part of the live inventory. If an older `results.json` contains `.trash` entries, run a Full Stocktake once to replace that cache before resuming Quick Scan. + ### Targeting a specific project To include project-level skills, run from that project's root directory: @@ -74,6 +76,8 @@ Scanning: | Skill | 7d use | 30d use | Description | |-------|--------|---------|-------------| +Usage counts come from the optional `~/.claude/observations.jsonl` file (overridable with `SKILL_STOCKTAKE_OBSERVATIONS`), which Claude Code does not create by default. When the file is absent, `use_7d` and `use_30d` are JSON `null`; display them as **unmeasured** in inventory and summary tables. A numeric `0` means the file exists but contains no matching Read observations in that window. Missing usage data is never evidence for retiring a skill. + ### Phase 2 — Quality Evaluation Launch an Agent tool subagent (**general-purpose agent**) with the full inventory and checklist: @@ -110,7 +114,7 @@ Each skill is evaluated against this checklist: - [ ] Content overlap with other skills checked - [ ] Overlap with MEMORY.md / CLAUDE.md checked - [ ] Freshness of technical references verified (use WebSearch if tool names / CLI flags / APIs are present) -- [ ] Usage frequency considered +- [ ] Usage frequency considered when measured; missing observations marked unmeasured, not treated as zero ``` Verdict criteria: diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index b22d42e11..da36fa0f1 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -76,7 +76,8 @@ process_dir() { # exit non-zero, which would otherwise silently under-count skills. # NUL-delimited (-print0 / sort_nul_file / read -d '') so a path containing a # literal newline can't desync record boundaries — paths here are untrusted. - if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then + # Archived skills are not live inventory; prune the entire trash subtree. + if ! find -L "$dir" -type d -name ".trash" -prune -o -name "SKILL.md" -type f -print0 >"$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 diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index 02c9c3dab..7f404fbe8 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -78,7 +78,7 @@ date_ago() { count_obs() { local file="$1" cutoff="$2" if [[ ! -f "$OBSERVATIONS" ]]; then - echo 0 + echo null return fi jq -r --arg p "$file" --arg c "$cutoff" \ @@ -104,9 +104,12 @@ scan_dir_to_json() { # Pre-aggregate observation counts in two passes (one per window) instead of # calling jq per-file — reduces from O(n*m) to O(n+m) jq invocations. local obs_7d_counts obs_30d_counts + # Zero is meaningful only when an observations file is available. + local usage_default=null obs_7d_counts="" obs_30d_counts="" if [[ -f "$OBSERVATIONS" ]]; then + usage_default=0 obs_7d_counts=$(jq -r --arg c "$c7" \ 'select(.tool=="Read" and .timestamp>=$c) | .path' \ "$OBSERVATIONS" 2>/dev/null | sort | uniq -c) @@ -123,7 +126,8 @@ scan_dir_to_json() { # exit non-zero, which would otherwise silently under-count skills. # NUL-delimited (-print0 / sort_nul_file / read -d '') so a path containing a # literal newline can't desync record boundaries — paths here are untrusted. - if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then + # Archived skills are not live inventory; prune the entire trash subtree. + if ! find -L "$dir" -type d -name ".trash" -prune -o -name "SKILL.md" -type f -print0 >"$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 @@ -143,9 +147,9 @@ scan_dir_to_json() { # Use awk exact field match to avoid substring false-positives from grep -F. # uniq -c output format: " N /path/to/file" — path is always field 2. u7=$(echo "$obs_7d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) - u7="${u7:-0}" + u7="${u7:-$usage_default}" u30=$(echo "$obs_30d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) - u30="${u30:-0}" + u30="${u30:-$usage_default}" fi dp="${file/#$HOME/~}" diff --git a/tests/scripts/skill-stocktake-discovery.test.js b/tests/scripts/skill-stocktake-discovery.test.js index 92c13c6df..c1aa26190 100644 --- a/tests/scripts/skill-stocktake-discovery.test.js +++ b/tests/scripts/skill-stocktake-discovery.test.js @@ -46,7 +46,8 @@ console.log('\nSkill stocktake discovery tests:'); test('both scanners use canonical, error-visible, NUL-delimited discovery', () => { for (const scriptPath of [scanScript, quickDiffScript]) { const source = fs.readFileSync(scriptPath, 'utf8'); - assert.match(source, /find -L "\$dir" -name "SKILL\.md" -type f -print0/); + assert.match(source, /find -L "\$dir"/); + assert.match(source, /-name "SKILL\.md" -type f -print0/); assert.match(source, /sort_nul_file "\$find_out"/); assert.match(source, /records\.sort\(Buffer\.compare\)/); assert.doesNotMatch(source, /sort -z/, `${path.basename(scriptPath)} still requires GNU sort`); @@ -147,6 +148,116 @@ if (process.platform === 'win32') { } } + +if (process.platform !== 'win32') { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-stocktake-inventory-')); + try { + const globalSkills = path.join(tempRoot, 'global', '.claude', 'skills'); + const projectSkills = path.join(tempRoot, 'project', '.claude', 'skills'); + const liveSkills = [ + [path.join(globalSkills, 'active'), 'global-active'], + [path.join(globalSkills, '.trash-archive'), 'global-trash-archive'], + [path.join(projectSkills, 'active'), 'project-active'], + [path.join(projectSkills, '.trashcan'), 'project-trashcan'], + [path.join(projectSkills, 'space skill'), 'space-skill'], + [path.join(projectSkills, 'newline\nskill'), 'newline-skill'], + ]; + for (const [skillDir, name] of liveSkills) writeSkill(skillDir, name); + for (const [scope, skillsDir] of [['global', globalSkills], ['project', projectSkills]]) { + writeSkill(path.join(skillsDir, '.trash', 'deleted'), `${scope}-deleted`); + writeSkill(path.join(skillsDir, 'bundle', '.trash', 'nested'), `${scope}-nested-deleted`); + } + const linkedTarget = path.join(tempRoot, 'shared', 'linked'); + const linkedSkill = path.join(projectSkills, 'linked'); + writeSkill(linkedTarget, 'linked-skill'); + fs.symlinkSync(linkedTarget, linkedSkill, 'dir'); + const expectedPaths = [...liveSkills.map(([skillDir]) => path.join(skillDir, 'SKILL.md')), + path.join(linkedSkill, 'SKILL.md')].sort(); + const resultsPath = path.join(tempRoot, 'results.json'); + fs.writeFileSync(resultsPath, JSON.stringify({ evaluated_at: '2099-01-01T00:00:00Z', skills: [] })); + const emptyObservations = path.join(tempRoot, 'empty-observations.jsonl'); + fs.writeFileSync(emptyObservations, ''); + const env = { + SKILL_STOCKTAKE_GLOBAL_DIR: globalSkills, + SKILL_STOCKTAKE_PROJECT_DIR: projectSkills, + SKILL_STOCKTAKE_OBSERVATIONS: path.join(tempRoot, 'missing-observations.jsonl'), + }; + + test('scan excludes root and nested .trash directories in both scopes, preserving live skills', () => { + const result = runBash(scanScript, [], env); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.deepStrictEqual(output.skills.map(skill => skill.path).sort(), expectedPaths); + assert.strictEqual(output.scan_summary.global.count, 2); + assert.strictEqual(output.scan_summary.project.count, 5); + }); + + test('quick diff excludes .trash in both scopes while preserving similarly named and linked skills', () => { + const result = runBash(quickDiffScript, [resultsPath], env); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.deepStrictEqual(output.map(skill => skill.path).sort(), expectedPaths); + assert.ok(output.every(skill => skill.is_new === true)); + }); + + for (const name of ['project-active', 'space-skill', 'newline-skill']) { + test(`scan reports unknown usage for ${name} when observations are missing`, () => { + const result = runBash(scanScript, [], env); + assert.strictEqual(result.status, 0, result.stderr); + const skill = JSON.parse(result.stdout).skills.find(entry => entry.name === name); + assert.ok(skill, `missing live skill: ${name}`); + assert.strictEqual(skill.use_7d, null); + assert.strictEqual(skill.use_30d, null); + }); + } + + test('scan reports numeric zero for unobserved skills when an empty observation file exists', () => { + const result = runBash(scanScript, [], { ...env, SKILL_STOCKTAKE_OBSERVATIONS: emptyObservations }); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.ok(output.skills.length > 0); + for (const skill of output.skills) { + assert.strictEqual(skill.use_7d, 0, skill.path); + assert.strictEqual(skill.use_30d, 0, skill.path); + } + }); + + test('scan preserves exact usage counts and date windows for normal and whitespace paths', () => { + const observationsPath = path.join(tempRoot, 'matching-observations.jsonl'); + const observedPaths = [path.join(linkedSkill, 'SKILL.md'), + ...liveSkills.filter(([, name]) => ['space-skill', 'newline-skill'].includes(name)) + .map(([skillDir]) => path.join(skillDir, 'SKILL.md'))]; + const timestamp = days => new Date(Date.now() - days * 86400000).toISOString(); + const observations = observedPaths.flatMap(skillPath => [ + { tool: 'Read', path: skillPath, timestamp: timestamp(0) }, + { tool: 'Read', path: skillPath, timestamp: timestamp(1) }, + { tool: 'Read', path: skillPath, timestamp: timestamp(10) }, + { tool: 'Read', path: skillPath, timestamp: timestamp(40) }, + { tool: 'Write', path: skillPath, timestamp: timestamp(0) }, + { tool: 'Read', path: `${skillPath}.backup`, timestamp: timestamp(0) }, + ]); + fs.writeFileSync(observationsPath, `${observations.map(record => JSON.stringify(record)).join('\n')}\n`); + const result = runBash(scanScript, [], { ...env, SKILL_STOCKTAKE_OBSERVATIONS: observationsPath }); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + for (const skillPath of observedPaths) { + const skill = output.skills.find(entry => entry.path === skillPath); + assert.ok(skill, `missing observed skill: ${skillPath}`); + assert.strictEqual(skill.use_7d, 2, skillPath); + assert.strictEqual(skill.use_30d, 3, skillPath); + } + const unobserved = output.skills.find(entry => entry.name === 'global-active'); + assert.strictEqual(unobserved.use_7d, 0); + assert.strictEqual(unobserved.use_30d, 0); + }); + } catch (error) { + console.log(` ✗ inventory fixture setup: ${error.message}`); + failed++; + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +} + console.log(`\nPassed: ${passed}`); console.log(`Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0);