fix(skill-stocktake): explicitly replace full-run caches

2026-10-19 06:23:46 +02:00 · 2026-09-23 13:17:34 +08:00
parent dddbe52cb7
commit 6d9c1eb3bc
3 changed files with 208 additions and 19 deletions
@@ -20,7 +20,7 @@ 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.
+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, start a new Full Stocktake using the cache initialization below before resuming Quick Scan. An ordinary save merges entries and does not remove archived records.

 ### Targeting a specific project

@@ -61,7 +61,11 @@ Re-evaluate only skills that have changed since the last run (5–10 min).

 ### Phase 1 — Inventory

-Run: `bash ~/.claude/skills/skill-stocktake/scripts/scan.sh`
+Capture the inventory:
+
+```bash
+SCAN_JSON=$(bash ~/.claude/skills/skill-stocktake/scripts/scan.sh)
+```

 The script enumerates skill files, extracts frontmatter, and collects UTC mtimes.
 Project dir is auto-detected from `$PWD/.claude/skills`; pass it explicitly only if needed.
@@ -78,6 +82,19 @@ Scanning:

 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.

+After a successful scan, initialize the cache **once at the start of a new Full Stocktake**:
+
+```bash
+INITIAL_RESULTS=$(printf '%s\n' "$SCAN_JSON" | jq '{
+  mode: "full", skills: {},
+  batch_progress: {total: (.skills | length), evaluated: 0, status: "in_progress"}
+}')
+bash ~/.claude/skills/skill-stocktake/scripts/save-results.sh \
+  ~/.claude/skills/skill-stocktake/results.json --replace <<< "$INITIAL_RESULTS"
+```
+
+`--replace` replaces the entire cached evaluation, removing archived or deleted skills and stale metadata. Do not run initialization when resuming an `in_progress` evaluation. Later chunks, completion updates, and Quick Scans must omit `--replace` so they merge into the current run instead of losing earlier results.
+
 ### Phase 2 — Quality Evaluation

 Launch an Agent tool subagent (**general-purpose agent**) with the full inventory and checklist:
@@ -102,7 +119,7 @@ The subagent reads each skill, applies the checklist, and returns per-skill JSON

 `{ "verdict": "Keep"|"Improve"|"Update"|"Retire"|"Merge into [X]", "reason": "..." }`

-**Chunk guidance:** Process ~20 skills per subagent invocation to keep context manageable. Save intermediate results to `results.json` (`status: "in_progress"`) after each chunk.
+**Chunk guidance:** Process ~20 skills per subagent invocation to keep context manageable. Save intermediate results with `save-results.sh RESULTS_JSON` (without `--replace`, `status: "in_progress"`) after each chunk.

 After all skills are evaluated: set `status: "completed"`, proceed to Phase 3.

@@ -1,21 +1,28 @@
 #!/usr/bin/env bash
 # save-results.sh — merge evaluated skills into results.json with correct UTC timestamp
-# Usage: save-results.sh RESULTS_JSON <<< "$EVAL_JSON"
+# Usage: save-results.sh RESULTS_JSON [--replace] <<< "$EVAL_JSON"
 #
 # stdin format:
 #   { "skills": {...}, "mode"?: "full"|"quick", "batch_progress"?: {...} }
 #
 # Always sets evaluated_at to current UTC time via `date -u`.
-# Merges stdin .skills into existing results.json (new entries override old).
+# By default, merges stdin .skills into results.json (new entries override old).
 # Optionally updates .mode and .batch_progress if present in stdin.
+# --replace starts a new full evaluation; subsequent chunks must use merge mode.

 set -euo pipefail

 RESULTS_JSON="${1:-}"
+SAVE_MODE="${2:-}"

 if [[ -z "$RESULTS_JSON" ]]; then
  echo "Error: RESULTS_JSON argument required" >&2
-  echo "Usage: save-results.sh RESULTS_JSON <<< \"\$EVAL_JSON\"" >&2
+  echo "Usage: save-results.sh RESULTS_JSON [--replace] <<< \"\$EVAL_JSON\"" >&2
+  exit 1
+fi
+
+if [[ $# -gt 2 || ( -n "$SAVE_MODE" && "$SAVE_MODE" != "--replace" ) ]]; then
+  echo "Error: expected RESULTS_JSON [--replace]" >&2
  exit 1
 fi

@@ -28,29 +35,40 @@ if ! echo "$input_json" | jq empty 2>/dev/null; then
  exit 1
 fi

-if [[ ! -f "$RESULTS_JSON" ]]; then
+# A replacement must be one complete object, not empty input or a JSON stream.
+# Validate before creating a temp file or changing the existing cache.
+if [[ "$SAVE_MODE" == "--replace" ]] && ! echo "$input_json" | jq -e -s \
+  'length == 1 and (.[0] | type == "object" and (.skills | type == "object"))' >/dev/null 2>&1; then
+  echo "Error: --replace requires one JSON object with a skills object" >&2
+  exit 1
+fi
+
+if [[ ! -f "$RESULTS_JSON" && "$SAVE_MODE" != "--replace" ]]; then
  # Bootstrap: create new results.json from stdin JSON + current UTC timestamp
  echo "$input_json" | jq --arg ea "$EVALUATED_AT" \
    '. + { evaluated_at: $ea }' > "$RESULTS_JSON"
  exit 0
 fi

-# Merge: new .skills override existing ones; old skills not in input_json are kept.
-# Optionally update .mode and .batch_progress if provided.
-#
 # Use mktemp for a collision-safe temp file (concurrent runs on the same RESULTS_JSON
 # would race on a predictable ".tmp" suffix; random suffix prevents silent overwrites).
 tmp=$(mktemp "${RESULTS_JSON}.XXXXXX")
 trap 'rm -f "$tmp"' EXIT

-jq -s \
-  --arg ea "$EVALUATED_AT" \
-  '.[0] as $existing | .[1] as $new |
-   $existing |
-   .evaluated_at = $ea |
-   .skills = ($existing.skills + ($new.skills // {})) |
-   if ($new | has("mode")) then .mode = $new.mode else . end |
-   if ($new | has("batch_progress")) then .batch_progress = $new.batch_progress else . end' \
-  "$RESULTS_JSON" <(echo "$input_json") > "$tmp"
+if [[ "$SAVE_MODE" == "--replace" ]]; then
+  echo "$input_json" | jq --arg ea "$EVALUATED_AT" \
+    '. + { evaluated_at: $ea }' > "$tmp"
+else
+  # Keep previously evaluated skills during quick scans and full-run chunks.
+  jq -s \
+    --arg ea "$EVALUATED_AT" \
+    '.[0] as $existing | .[1] as $new |
+     $existing |
+     .evaluated_at = $ea |
+     .skills = ($existing.skills + ($new.skills // {})) |
+     if ($new | has("mode")) then .mode = $new.mode else . end |
+     if ($new | has("batch_progress")) then .batch_progress = $new.batch_progress else . end' \
+    "$RESULTS_JSON" <(echo "$input_json") > "$tmp"
+fi

 mv "$tmp" "$RESULTS_JSON"
@@ -0,0 +1,154 @@
+#!/usr/bin/env node
+
+'use strict';
+
+const assert = require('assert');
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { spawnSync } = require('child_process');
+
+const repoRoot = path.resolve(__dirname, '..', '..');
+const saveScript = path.join(repoRoot, 'skills', 'skill-stocktake', 'scripts', 'save-results.sh');
+let passed = 0;
+let failed = 0;
+
+function test(description, fn) {
+  const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-stocktake-save-'));
+  try {
+    fn(path.join(tempRoot, 'results.json'));
+    console.log(`  ✓ ${description}`);
+    passed++;
+  } catch (error) {
+    console.log(`  ✗ ${description}: ${error.message}`);
+    failed++;
+  } finally {
+    fs.rmSync(tempRoot, { recursive: true, force: true });
+  }
+}
+
+function save(resultsPath, input, args = []) {
+  return spawnSync('bash', [saveScript, resultsPath, ...args], {
+    encoding: 'utf8',
+    input: typeof input === 'string' ? input : JSON.stringify(input),
+    env: process.env,
+  });
+}
+
+function readResults(resultsPath) {
+  const output = JSON.parse(fs.readFileSync(resultsPath, 'utf8'));
+  assert.match(output.evaluated_at, /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/);
+  assert.ok(Math.abs(Date.now() - Date.parse(output.evaluated_at)) < 60000);
+  return output;
+}
+
+console.log('\nSkill stocktake save-results tests:');
+
+if (process.platform === 'win32') {
+  console.log('  ↷ Bash integration cases skipped on Windows');
+} else {
+  for (const mode of [undefined, 'quick']) {
+    test(`default save merges ${mode || 'legacy'} results and retains unchanged skills`, resultsPath => {
+      const unchanged = { verdict: 'Keep', reason: 'unchanged skill' };
+      fs.writeFileSync(resultsPath, JSON.stringify({
+        evaluated_at: '2000-01-01T00:00:00Z',
+        mode: 'full',
+        batch_progress: { completed: 1, total: 2 },
+        skills: { unchanged, updated: { verdict: 'Improve' } },
+      }));
+      const input = { skills: { updated: { verdict: 'Keep' }, added: { verdict: 'Keep' } } };
+      if (mode) input.mode = mode;
+      const result = save(resultsPath, input);
+      assert.strictEqual(result.status, 0, result.stderr);
+      const output = readResults(resultsPath);
+      assert.deepStrictEqual(output.skills, { unchanged, ...input.skills });
+      assert.strictEqual(output.mode, mode || 'full');
+      assert.deepStrictEqual(output.batch_progress, { completed: 1, total: 2 });
+    });
+  }
+
+  test('full-mode intermediate saves still merge completed batches for resume', resultsPath => {
+    const completed = { verdict: 'Keep' };
+    fs.writeFileSync(resultsPath, JSON.stringify({
+      mode: 'full', batch_progress: { completed: 1, total: 2 }, skills: { completed },
+    }));
+    const input = {
+      mode: 'full', batch_progress: { completed: 2, total: 2 }, skills: { next: { verdict: 'Improve' } },
+    };
+    const result = save(resultsPath, input);
+    assert.strictEqual(result.status, 0, result.stderr);
+    const output = readResults(resultsPath);
+    assert.deepStrictEqual(output.skills, { completed, ...input.skills });
+    assert.deepStrictEqual(output.batch_progress, input.batch_progress);
+  });
+
+  test('explicit replacement saves only the live inventory and drops obsolete metadata', resultsPath => {
+    fs.writeFileSync(resultsPath, JSON.stringify({
+      evaluated_at: '2000-01-01T00:00:00Z',
+      mode: 'quick',
+      batch_progress: { completed: 1, total: 2 },
+      obsolete_summary: 'old cache',
+      skills: {
+        '~/.claude/skills/.trash/deleted/SKILL.md': { verdict: 'Keep' },
+        '~/.claude/skills/removed/SKILL.md': { verdict: 'Keep' },
+      },
+    }));
+    const input = {
+      mode: 'full',
+      evaluated_at: '1999-01-01T00:00:00Z',
+      skills: { '~/.claude/skills/live/SKILL.md': { verdict: 'Keep' } },
+    };
+    const result = save(resultsPath, input, ['--replace']);
+    assert.strictEqual(result.status, 0, result.stderr);
+    const output = readResults(resultsPath);
+    assert.deepStrictEqual(output, { ...input, evaluated_at: output.evaluated_at });
+  });
+
+  test('explicit replacement with an empty inventory removes all cached skills', resultsPath => {
+    fs.writeFileSync(resultsPath, JSON.stringify({
+      skills: { stale: { verdict: 'Keep' } }, batch_progress: { completed: 1, total: 2 },
+    }));
+    const input = { mode: 'full', skills: {} };
+    const result = save(resultsPath, input, ['--replace']);
+    assert.strictEqual(result.status, 0, result.stderr);
+    const output = readResults(resultsPath);
+    assert.deepStrictEqual(output, { ...input, evaluated_at: output.evaluated_at });
+  });
+
+  test('invalid JSON during replacement preserves the existing results byte for byte', resultsPath => {
+    const original = '{\n  "skills": {"keep": {"verdict": "Keep"}}\n}\n';
+    fs.writeFileSync(resultsPath, original);
+    const result = save(resultsPath, '{"skills":', ['--replace']);
+    assert.notStrictEqual(result.status, 0);
+    assert.match(result.stderr, /not valid JSON/);
+    assert.strictEqual(fs.readFileSync(resultsPath, 'utf8'), original);
+    assert.deepStrictEqual(fs.readdirSync(path.dirname(resultsPath)), ['results.json']);
+  });
+
+  test('replacement rejects absent, non-object, or multiple inventories without altering the cache', resultsPath => {
+    const original = '{\n  "skills": {"keep": {"verdict": "Keep"}}\n}\n';
+    const invalidInputs = ['', 'null', '[]', '{}', '{"skills":[]}', '{"skills":null}',
+      '{"skills":{}}\n{"skills":{}}'];
+    for (const input of invalidInputs) {
+      fs.writeFileSync(resultsPath, original);
+      const result = save(resultsPath, input, ['--replace']);
+      assert.notStrictEqual(result.status, 0, `accepted invalid replacement: ${JSON.stringify(input)}`);
+      assert.strictEqual(fs.readFileSync(resultsPath, 'utf8'), original);
+      assert.deepStrictEqual(fs.readdirSync(path.dirname(resultsPath)), ['results.json']);
+    }
+  });
+
+  for (const args of [[], ['--replace']]) {
+    test(`${args.length ? 'replacement' : 'default'} save bootstraps a new results file`, resultsPath => {
+      const input = { mode: 'full', skills: { live: { verdict: 'Keep' } } };
+      const result = save(resultsPath, input, args);
+      assert.strictEqual(result.status, 0, result.stderr);
+      const output = readResults(resultsPath);
+      assert.deepStrictEqual(output, { ...input, evaluated_at: output.evaluated_at });
+    });
+  }
+}
+
+console.log(`\nPassed: ${passed}`);
+console.log(`Failed: ${failed}`);
+process.exit(failed > 0 ? 1 : 0);