fix: close validator and path edge cases

This commit is contained in:
haelyra
2026-08-29 15:36:59 -04:00
parent 703163275d
commit 1bdda4bdac
6 changed files with 101 additions and 18 deletions
+21 -2
View File
@@ -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 };
}
+3 -6
View File
@@ -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
+13 -6
View File
@@ -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 \
+28
View File
@@ -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');
+8 -3
View File
@@ -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++;
@@ -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++;