diff --git a/scripts/ci/validate-skills.js b/scripts/ci/validate-skills.js index 6e7c3a64b..47334687f 100644 --- a/scripts/ci/validate-skills.js +++ b/scripts/ci/validate-skills.js @@ -26,6 +26,7 @@ const fs = require('fs'); const path = require('path'); +const yaml = require('js-yaml'); const SKILLS_DIR = path.join(__dirname, '../../skills'); const DOCS_DIR = path.join(__dirname, '../../docs'); @@ -101,7 +102,7 @@ function stripUnquotedYamlComment(rawValue) { } function inspectFrontmatter(lines) { - const values = Object.create(null); + let values = Object.create(null); let syntaxErrors = []; let descriptionIndicator = null; let inBlockScalar = false; @@ -126,7 +127,7 @@ function inspectFrontmatter(lines) { const rawValue = match[2]; // Strip YAML comments only when # appears outside a quoted scalar. const valueNoComment = stripUnquotedYamlComment(rawValue); - values[key] = valueNoComment; + values = Object.assign(Object.create(null), values, { [key]: valueNoComment }); const isQuoted = /^"(?:[^"\\]|\\.)*"$/.test(valueNoComment) || /^'(?:[^']|'')*'$/.test(valueNoComment); @@ -164,6 +165,24 @@ function inspectFrontmatter(lines) { } } + try { + const parsed = yaml.load(lines.join('\n')); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + syntaxErrors = [...syntaxErrors, 'must be a top-level YAML mapping']; + } else { + for (const key of ['name', 'description']) { + if (!Object.prototype.hasOwnProperty.call(parsed, key)) continue; + if (typeof parsed[key] !== 'string') { + syntaxErrors = [...syntaxErrors, `${key}: value must be a string`]; + continue; + } + values = Object.assign(Object.create(null), values, { [key]: parsed[key] }); + } + } + } catch (error) { + syntaxErrors = [...syntaxErrors, `invalid YAML: ${error.reason || error.message}`]; + } + return { values, descriptionIndicator, syntaxErrors }; } diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index 418b02558..b22d42e11 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -58,9 +58,6 @@ if [[ ! "$evaluated_at" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2 exit 1 fi -# Pre-extract known paths from results.json once (O(1) lookup per file instead of O(n*m)) -known_paths=$(jq -r '.skills[].path' "$RESULTS_JSON" 2>/dev/null) - tmpdir=$(mktemp -d) # Use a function to avoid embedding $tmpdir in a quoted string (prevents injection # if TMPDIR were crafted to contain shell metacharacters). @@ -90,9 +87,9 @@ process_dir() { mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) dp="${file/#$HOME/~}" - # Check if this file is known to results.json (exact whole-line match to - # avoid substring false-positives, e.g. "python-patterns" matching "python-patterns-v2"). - if echo "$known_paths" | grep -qxF "$dp"; then + # Keep path comparison structured so literal newlines remain part of one + # JSON string instead of becoming ambiguous line-delimited records. + if jq -e --arg path "$dp" '.skills | any(.path == $path)' "$RESULTS_JSON" >/dev/null 2>&1; then is_new="false" # Known file: only emit if mtime changed (ISO 8601 string comparison is safe) [[ "$mtime" > "$evaluated_at" ]] || continue diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index e43509edc..ea2e65574 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -134,12 +134,19 @@ scan_dir_to_json() { name=$(extract_field "$file" "name") desc=$(extract_field "$file" "description") mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) - # 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}" - u30=$(echo "$obs_30d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) - u30="${u30:-0}" + if [[ "$file" == *$'\n'* ]]; then + # The aggregated fast path is line-delimited. Preserve unusual paths by + # falling back to the structured JSON matcher for this record. + u7=$(count_obs "$file" "$c7") + u30=$(count_obs "$file" "$c30") + else + # 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}" + u30=$(echo "$obs_30d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) + u30="${u30:-0}" + fi dp="${file/#$HOME/~}" jq -n \ diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index 4bcb9452a..afac4a469 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -2870,6 +2870,34 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (test('rejects malformed quoted skill frontmatter', () => { + const testDir = createTestDir(); + const skillDir = path.join(testDir, 'malformed-quote'); + fs.mkdirSync(skillDir); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: malformed-quote\ndescription: "unterminated\n---\n# Example'); + + const result = runSkillsValidator(testDir, ['--strict']); + assert.strictEqual(result.code, 1, 'Strict validation must reject malformed YAML'); + assert.ok(result.stderr.includes('invalid YAML'), + `Should report the YAML parse failure, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects an empty folded skill description', () => { + const testDir = createTestDir(); + const skillDir = path.join(testDir, 'empty-folded-description'); + fs.mkdirSync(skillDir); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: empty-folded-description\ndescription: >\n---\n# Example'); + + const result = runSkillsValidator(testDir, ['--strict']); + assert.strictEqual(result.code, 1, 'Strict validation must reject an empty folded scalar'); + assert.ok(result.stderr.includes("'description' is empty"), + `Should report the empty parsed description, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + if (test('reports an unreadable docs root deterministically', () => { const testDir = createTestDir(); const docsPath = path.join(testDir, 'docs-file'); diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 928d2a121..0dfe1d2f9 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -151,9 +151,14 @@ else failed++; if ( test('shell test runner honors a per-invocation BASH_PATH override', () => { - const missingBash = path.join(os.tmpdir(), 'ecc-missing-bash-executable'); - const result = runBash(prePushHook, { env: { BASH_PATH: missingBash } }); - assert.strictEqual(result.error?.code, 'ENOENT'); + const tempDir = createTempDir('ecc-missing-bash-'); + try { + const missingBash = path.join(tempDir, 'bash'); + const result = runBash(prePushHook, { env: { BASH_PATH: missingBash } }); + assert.strictEqual(result.error?.code, 'ENOENT'); + } finally { + cleanup(tempDir); + } }) ) passed++; diff --git a/tests/scripts/skill-stocktake-discovery.test.js b/tests/scripts/skill-stocktake-discovery.test.js index 498ed43fa..106d041ef 100644 --- a/tests/scripts/skill-stocktake-discovery.test.js +++ b/tests/scripts/skill-stocktake-discovery.test.js @@ -65,6 +65,7 @@ if (process.platform === 'win32') { const linkedTarget = path.join(tempRoot, 'shared', 'linked-skill'); const newlineSkill = path.join(projectSkills, 'newline\nskill'); const resultsPath = path.join(tempRoot, 'results.json'); + const observationsPath = path.join(tempRoot, 'observations.jsonl'); writeSkill(directSkill, 'direct-skill'); writeSkill(linkedTarget, 'linked-skill'); @@ -76,11 +77,19 @@ if (process.platform === 'win32') { resultsPath, JSON.stringify({ evaluated_at: '2099-01-01T00:00:00Z', skills: [] }), ); + fs.writeFileSync( + observationsPath, + `${JSON.stringify({ + tool: 'Read', + path: path.join(newlineSkill, 'SKILL.md'), + timestamp: new Date().toISOString(), + })}\n`, + ); const env = { SKILL_STOCKTAKE_GLOBAL_DIR: path.join(tempRoot, 'missing-global'), SKILL_STOCKTAKE_PROJECT_DIR: projectSkills, - SKILL_STOCKTAKE_OBSERVATIONS: path.join(tempRoot, 'missing-observations.jsonl'), + SKILL_STOCKTAKE_OBSERVATIONS: observationsPath, }; test('scan follows symlinked skills and ignores nested Markdown assets', () => { @@ -92,6 +101,9 @@ if (process.platform === 'win32') { output.skills.map(skill => skill.name).sort(), ['direct-skill', 'linked-skill', 'newline-skill'], ); + const newlineEntry = output.skills.find(skill => skill.name === 'newline-skill'); + assert.strictEqual(newlineEntry.use_7d, 1); + assert.strictEqual(newlineEntry.use_30d, 1); }); test('quick diff keeps newline-containing skill paths as one record', () => { @@ -105,6 +117,21 @@ if (process.platform === 'win32') { ); assert.ok(output.every(entry => entry.is_new === true)); }); + + test('quick diff recognizes a cached newline-containing path', () => { + fs.writeFileSync( + resultsPath, + JSON.stringify({ + evaluated_at: '2099-01-01T00:00:00Z', + skills: [{ path: path.join(newlineSkill, 'SKILL.md') }], + }), + ); + const result = runBash(quickDiffScript, [resultsPath], env); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.strictEqual(output.length, 2); + assert.ok(output.every(entry => !entry.path.includes('newline\nskill/SKILL.md'))); + }); } catch (error) { console.log(` ✗ fixture setup: ${error.message}`); failed++;