Files
ECC/tests/hooks/skill-run-tracker.test.js
T
01e15490f0 fix(skill-evolution): wire Skill PostToolUse tracker so skill-health shows real runs (#2490)
* fix(skill-evolution): wire Skill PostToolUse tracker so skill-health shows real runs (#2463)

recordSkillExecution() had no production callers, so
~/.claude/state/skill-runs.jsonl was never written and
`scripts/skills-health.js --dashboard` always reported 0 runs.

Adds scripts/hooks/skill-run-tracker.js and registers it as an async
PostToolUse hook (matcher: Skill) in posttooluse-dispatcher.js, which is now
the single PostToolUse entrypoint on main.

Addresses the privacy and bounds review on #2555's sibling PR:

- No prompt text is persisted. task_description is synthesized as
  "Skill invocation: <skill_id>"; tool_input.task_description/description/
  prompt are never read.
- Every persisted string is bounded and charset-restricted. A skill id is an
  identifier, so free text, newlines, or an over-long value are dropped rather
  than truncated and written through.
- The JSONL sink is created 0600 and re-tightened on each append, repairing
  files written before this bound existed.
- The sink is capped at MAX_RUN_RECORDS (5000), trimmed oldest-first, so the
  append-only file can no longer grow without limit.

Tests cover the privacy guarantee (no prompt text reaches a record), the
identifier bounds, the file mode on POSIX, and the retention cap.

* fix(skill-evolution): re-register the tracker for PostToolUseFailure

The rebase onto current main dropped the hooks.json entry, which silently
resurrected the P1 from the earlier review round: deriveOutcome() still
branches on hook_event_name === 'PostToolUseFailure', but the PostToolUse
dispatcher does not fan that event out, so the branch was unreachable in
production. Hard Skill failures were dropped from telemetry entirely, which
inflates the dashboard success rate — the opposite of what #2463 asks for.

Restores the dedicated PostToolUseFailure entry (matcher Skill, id
post:skill:track, same run-with-flags wrapper and standard,strict gating as
the dispatcher registration). Verified end-to-end: a PostToolUseFailure
payload piped through run-with-flags now records outcome "failure".

Adds a regression test asserting the registration so a future rebase cannot
quietly drop it again.

---------

Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com>
2026-08-12 18:12:49 -04:00

260 lines
10 KiB
JavaScript

/**
* Tests for scripts/hooks/skill-run-tracker.js and the JSONL sink bounds in
* scripts/lib/skill-evolution/tracker.js (#2463).
*
* Focus: the tracker records real runs, and it never persists prompt text,
* unbounded strings, an unbounded file, or a world-readable sink.
*
* Run with: node tests/run-all.js
*/
'use strict';
const assert = require('assert');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { buildRecord, deriveOutcome, extractSkillId, run } = require('../../scripts/hooks/skill-run-tracker');
const {
MAX_RUN_RECORDS,
RUNS_FILE_MODE,
getRunsFilePath,
recordSkillExecution,
readSkillExecutionRecords,
} = require('../../scripts/lib/skill-evolution/tracker');
let passed = 0;
let failed = 0;
function test(name, fn) {
try {
fn();
console.log(` ✓ ${name}`);
passed++;
} catch (err) {
console.log(` ✗ ${name}`);
console.log(` Error: ${err.message}`);
failed++;
}
}
function withTempHome(fn) {
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-skill-runs-'));
try {
return fn(homeDir);
} finally {
fs.rmSync(homeDir, { recursive: true, force: true });
}
}
function payload(overrides = {}) {
return {
hook_event_name: 'PostToolUse',
tool_name: 'Skill',
tool_input: { skill_id: 'code-review', skill_version: '1.2.0' },
tool_response: {},
...overrides,
};
}
// ── skill id extraction and bounds ────────────────────────────────────────────
test('extractSkillId probes the field names Claude Code has used', () => {
assert.strictEqual(extractSkillId({ skill_id: 'a' }), 'a');
assert.strictEqual(extractSkillId({ skillId: 'b' }), 'b');
assert.strictEqual(extractSkillId({ skill: 'c' }), 'c');
assert.strictEqual(extractSkillId({ name: 'd' }), 'd');
assert.strictEqual(extractSkillId({ command: 'e' }), 'e');
assert.strictEqual(extractSkillId('bare-string'), 'bare-string');
});
test('extractSkillId returns null when no skill id is present', () => {
assert.strictEqual(extractSkillId({}), null);
assert.strictEqual(extractSkillId(null), null);
assert.strictEqual(extractSkillId(42), null);
});
test('extractSkillId rejects an over-long identifier rather than truncating it', () => {
assert.strictEqual(extractSkillId({ skill_id: 'x'.repeat(129) }), null);
assert.strictEqual(extractSkillId({ skill_id: 'x'.repeat(128) }), 'x'.repeat(128));
});
test('extractSkillId rejects free text that is not identifier-shaped', () => {
// A prompt smuggled into the skill field must not become a persisted id.
assert.strictEqual(extractSkillId({ skill_id: 'summarize this: my api key is sk-abc' }), null);
assert.strictEqual(extractSkillId({ skill_id: 'line\nbreak' }), null);
assert.strictEqual(extractSkillId({ skill_id: ' ' }), null);
});
// ── privacy: no prompt text is persisted ─────────────────────────────────────
test('buildRecord synthesizes task_description and never copies prompt fields', () => {
const secret = 'PROMPT-SECRET-do-not-persist';
const record = buildRecord(payload({
tool_input: {
skill_id: 'code-review',
skill_version: '1.2.0',
task_description: secret,
description: secret,
prompt: secret,
taskDescription: secret,
},
}));
assert.strictEqual(record.task_description, 'Skill invocation: code-review');
assert.ok(!JSON.stringify(record).includes(secret), 'record must not contain any prompt text');
});
test('buildRecord persists only the four dashboard fields', () => {
const record = buildRecord(payload());
assert.deepStrictEqual(
Object.keys(record).sort(),
['outcome', 'skill_id', 'skill_version', 'task_description']
);
});
test('buildRecord drops a non-identifier skill_version instead of persisting it', () => {
const record = buildRecord(payload({
tool_input: { skill_id: 'code-review', skill_version: 'v1 (as requested by the user in chat)' },
}));
assert.strictEqual(record.skill_version, 'unknown');
});
test('buildRecord returns null when the skill id is unusable', () => {
assert.strictEqual(buildRecord(payload({ tool_input: {} })), null);
});
// ── outcome derivation ───────────────────────────────────────────────────────
test('deriveOutcome reports failure for PostToolUseFailure routing', () => {
assert.strictEqual(deriveOutcome(payload({ hook_event_name: 'PostToolUseFailure' })), 'failure');
});
test('deriveOutcome reports failure for error-bearing tool responses', () => {
assert.strictEqual(deriveOutcome(payload({ tool_response: { is_error: true } })), 'failure');
assert.strictEqual(deriveOutcome(payload({ tool_response: { isError: true } })), 'failure');
assert.strictEqual(deriveOutcome(payload({ tool_response: { status: 'ERROR' } })), 'failure');
assert.strictEqual(deriveOutcome(payload({ tool_response: { error: 'boom' } })), 'failure');
});
test('deriveOutcome reports success otherwise', () => {
assert.strictEqual(deriveOutcome(payload()), 'success');
assert.strictEqual(deriveOutcome(payload({ tool_response: { status: 'ok' } })), 'success');
});
// ── hook behaviour ───────────────────────────────────────────────────────────
test('run ignores non-Skill tools and malformed input without throwing', () => {
assert.doesNotThrow(() => run(JSON.stringify(payload({ tool_name: 'Bash' }))));
assert.doesNotThrow(() => run('not json'));
assert.doesNotThrow(() => run(''));
});
// ── JSONL sink bounds ────────────────────────────────────────────────────────
test('recordSkillExecution writes the sink owner-only', function () {
if (process.platform === 'win32') {
return; // POSIX modes are not meaningful on Windows
}
withTempHome(homeDir => {
recordSkillExecution(
{ skill_id: 'code-review', skill_version: '1.0.0', task_description: 'Skill invocation: code-review', outcome: 'success' },
{ homeDir }
);
const runsFilePath = getRunsFilePath({ homeDir });
const mode = fs.statSync(runsFilePath).mode & 0o777;
assert.strictEqual(mode, RUNS_FILE_MODE, `expected mode ${RUNS_FILE_MODE.toString(8)}, got ${mode.toString(8)}`);
});
});
test('recordSkillExecution re-tightens an already world-readable sink', function () {
if (process.platform === 'win32') {
return;
}
withTempHome(homeDir => {
const runsFilePath = getRunsFilePath({ homeDir });
fs.mkdirSync(path.dirname(runsFilePath), { recursive: true });
fs.writeFileSync(runsFilePath, '', { mode: 0o644 });
recordSkillExecution(
{ skill_id: 'code-review', skill_version: '1.0.0', task_description: 'Skill invocation: code-review', outcome: 'success' },
{ homeDir }
);
assert.strictEqual(fs.statSync(runsFilePath).mode & 0o777, RUNS_FILE_MODE);
});
});
test('the JSONL sink is bounded by a retention cap', () => {
withTempHome(homeDir => {
const maxRecords = 5;
for (let i = 0; i < maxRecords + 4; i++) {
recordSkillExecution(
{ skill_id: `skill-${i}`, skill_version: '1.0.0', task_description: `Skill invocation: skill-${i}`, outcome: 'success' },
{ homeDir, maxRecords }
);
}
const records = readSkillExecutionRecords({ homeDir });
assert.strictEqual(records.length, maxRecords, 'sink must be trimmed to the cap');
// Trimming keeps the newest runs, so the dashboard still reflects recent activity.
assert.strictEqual(records[records.length - 1].skill_id, `skill-${maxRecords + 3}`);
assert.strictEqual(records[0].skill_id, `skill-${4}`);
});
});
test('the default retention cap is a finite bound', () => {
assert.ok(Number.isInteger(MAX_RUN_RECORDS) && MAX_RUN_RECORDS > 0, 'MAX_RUN_RECORDS must be a positive integer');
});
test('an end-to-end Skill hook run lands exactly one non-sensitive record', () => {
withTempHome(homeDir => {
const previousHome = process.env.HOME;
const previousUserProfile = process.env.USERPROFILE;
process.env.HOME = homeDir;
process.env.USERPROFILE = homeDir;
try {
run(JSON.stringify(payload({
tool_input: { skill_id: 'code-review', skill_version: '1.2.0', prompt: 'PROMPT-SECRET' },
})));
const records = readSkillExecutionRecords({ homeDir });
assert.strictEqual(records.length, 1);
assert.strictEqual(records[0].skill_id, 'code-review');
assert.strictEqual(records[0].skill_version, '1.2.0');
assert.strictEqual(records[0].outcome, 'success');
assert.ok(!JSON.stringify(records[0]).includes('PROMPT-SECRET'));
} finally {
if (previousHome === undefined) delete process.env.HOME; else process.env.HOME = previousHome;
if (previousUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = previousUserProfile;
}
});
});
// deriveOutcome treats PostToolUseFailure as a hard failure. That branch is
// only reachable if the hook is actually registered for the event: the
// PostToolUse dispatcher does not fan out PostToolUseFailure, so the tracker
// needs its own hooks.json entry. Without it, hard Skill failures are silently
// dropped and the dashboard's success rate is inflated.
test('the tracker is registered for PostToolUseFailure so hard failures are recorded', () => {
const hooksConfig = JSON.parse(
fs.readFileSync(path.join(__dirname, '..', '..', 'hooks', 'hooks.json'), 'utf8')
);
const entries = (hooksConfig.hooks.PostToolUseFailure || [])
.filter(entry => entry.id === 'post:skill:track');
assert.strictEqual(entries.length, 1, 'expected one post:skill:track PostToolUseFailure entry');
assert.strictEqual(entries[0].matcher, 'Skill', 'tracker must only match the Skill tool');
assert.ok(
entries[0].hooks[0].command.includes('scripts/hooks/skill-run-tracker.js'),
'entry should invoke skill-run-tracker.js'
);
});
console.log(`\nPassed: ${passed}`);
console.log(`Failed: ${failed}`);
if (failed > 0) {
process.exitCode = 1;
}