fix(stats): event-emit lands without the env var and reports every failure
event-emit returned {written:false, reason:'CLAUDE_PLUGIN_DATA unset'} whenever the harness did not export the variable - the normal case for the Bash tool - so /trekexecute's Phase 8 events never landed. It now resolves the directory through stats-append's resolveStatsDir (one resolver; opts.path still wins, so intent-approval is unchanged). The CLI uses the realpath isMain() guard (the file:// guard was a silent no-op under a symlink or a path with a space) and exits 1 on every written:false - usage error, payload-not-json, failed write - instead of always 0. KNOWN_EVENTS gains main-merge-approved, main-merge-declined and gates_mode_flag_override, which /trekexecute emits by design. Header, intent-approval docstring (comment only), docs/operations.md and the README autonomy sentence (wrong state names, 'records each transition') updated.
Red: 10 of 20 tests fail on the pre-fix code - 7 new (CLI lands with CLAUDE_PLUGIN_DATA empty / absent, unwritable target exit 1, usage error exit 1, symlink, space path, known events) and 3 inverted contracts. Regression guards (green on arrival): never throws when unwritable; no second copy of the fallback directory name.
Inverted (the contract this changes, not deleted): emit with no CLAUDE_PLUGIN_DATA (silent skip) -> lands under a pinned HOME; never throws when unwritable -> a genuinely unwritable target (env {} now resolves to the real home); resolveStatsPath({}) === null -> the fallback path; malformed --payload exit 0 -> exit 1.
Readers checked: otel-export keys on 'event', yardstick counts only user_input, cache-analyzer and research-loop-cap name event-emit only in comments. Verify 268/268 (stats-event-emit, intent-approval, doc-consistency, otel-export-validators, yardstick). Suite 1376 (1374/0/2); non-hook lines in the real trekexecute-stats.jsonl 60 before and after.
Voyage order 20261005T104513Z-1250514346, plan step 2/5.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
8a56187341
commit
27d586841a
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
5 changed files with 230 additions and 57 deletions
|
|
@ -2,18 +2,25 @@
|
|||
// Cover lib/stats/event-emit.mjs:
|
||||
// - emit appends a JSONL line with required ISO-8601 ts
|
||||
// - known_event flag distinguishes recognized vs unknown events
|
||||
// - missing CLAUDE_PLUGIN_DATA does NOT throw (stats must never block)
|
||||
// - CLI shim parses --payload JSON and writes via emit()
|
||||
// - missing CLAUDE_PLUGIN_DATA falls back to the directory stats-append and
|
||||
// scripts/yardstick.mjs use, and a failed write never throws
|
||||
// - CLI shim parses --payload JSON, writes via emit(), exits 1 on any
|
||||
// written:false, and runs through a symlink or a path with a space
|
||||
// - concurrent appends don't corrupt the file (smoke test)
|
||||
//
|
||||
// Isolation: every test that reaches the fallback pins HOME to a temp dir and
|
||||
// empties CLAUDE_PLUGIN_DATA. `env: {}` is never used - resolveStatsDir would fall
|
||||
// back to os.homedir(), the operator's REAL data directory.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { mkdtempSync, rmSync, readFileSync, existsSync } from 'node:fs';
|
||||
import { execFileSync, spawnSync } from 'node:child_process';
|
||||
import { cpSync, mkdtempSync, mkdirSync, rmSync, readFileSync, existsSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { emit, buildRecord, resolveStatsPath, KNOWN_EVENTS } from '../../lib/stats/event-emit.mjs';
|
||||
import { resolveStatsDir } from '../../lib/stats/stats-append.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const SHIM = join(HERE, '..', '..', 'lib', 'stats', 'event-emit.mjs');
|
||||
|
|
@ -24,6 +31,24 @@ function tmp(prefix = 'stats-event-emit-') {
|
|||
return mkdtempSync(join(tmpdir(), prefix));
|
||||
}
|
||||
|
||||
const ROOT = join(HERE, '..', '..');
|
||||
const DATA_REL = join('.claude', 'plugins', 'data', 'voyage-ktg-plugin-marketplace');
|
||||
|
||||
/** process.env with HOME pinned and CLAUDE_PLUGIN_DATA empty: never the real data dir. */
|
||||
function isolatedEnv(home) {
|
||||
const env = { ...process.env, CLAUDE_PLUGIN_DATA: '', HOME: home };
|
||||
delete env.CLAUDE_COORD_DIR;
|
||||
return env;
|
||||
}
|
||||
|
||||
function runShim(cli, args, env) {
|
||||
return spawnSync(process.execPath, [cli, ...args], { encoding: 'utf-8', env });
|
||||
}
|
||||
|
||||
function records(file) {
|
||||
return readFileSync(file, 'utf-8').trim().split('\n').map((l) => JSON.parse(l));
|
||||
}
|
||||
|
||||
test('KNOWN_EVENTS contains plan-v2 spec set', () => {
|
||||
for (const e of ['brief-approved', 'main-merge-gate', 'user_input']) {
|
||||
assert.ok(KNOWN_EVENTS.has(e), `missing recognized event: ${e}`);
|
||||
|
|
@ -82,27 +107,60 @@ test('emit creates the stats directory on demand', () => {
|
|||
}
|
||||
});
|
||||
|
||||
test('emit with no CLAUDE_PLUGIN_DATA returns { written: false } (silent skip)', () => {
|
||||
const r = emit('brief-approved', {}, { env: {} });
|
||||
assert.equal(r.written, false);
|
||||
assert.equal(r.path, null);
|
||||
assert.match(r.reason, /CLAUDE_PLUGIN_DATA unset/);
|
||||
// Inverted contract (was: "returns { written: false } (silent skip)"). The silent
|
||||
// skip is how Phase 8's events never landed: the Bash tool env carries no
|
||||
// CLAUDE_PLUGIN_DATA.
|
||||
test('emit with no CLAUDE_PLUGIN_DATA lands in the fallback dir under HOME', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const r = emit('brief-approved', {}, { env: { HOME: home } });
|
||||
assert.equal(r.written, true, JSON.stringify(r));
|
||||
assert.equal(r.path, join(home, DATA_REL, 'trekexecute-stats.jsonl'));
|
||||
assert.equal(records(r.path).length, 1);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// Inverted contract (was: env {} as the "unwritable" case - after the fallback an
|
||||
// empty env resolves to the real home, so it is no longer unwritable).
|
||||
test('emit never throws when stats path is unwritable', () => {
|
||||
// Pointing at a path under a non-existent dir on a readonly mount would
|
||||
// be brittle in CI; instead, force the env-resolved path to be empty
|
||||
// and confirm no exception leaks.
|
||||
let threw = false;
|
||||
try { emit('user_input', { foo: 'bar' }, { env: {} }); }
|
||||
catch { threw = true; }
|
||||
assert.equal(threw, false);
|
||||
const dir = tmp();
|
||||
try {
|
||||
const file = join(dir, 'a-file');
|
||||
writeFileSync(file, 'not a directory');
|
||||
let r;
|
||||
assert.doesNotThrow(() => { r = emit('user_input', { foo: 'bar' }, { path: join(file, 'x.jsonl') }); });
|
||||
assert.equal(r.written, false);
|
||||
assert.ok(r.reason && r.reason.length > 0, JSON.stringify(r));
|
||||
|
||||
mkdirSync(join(dir, '.claude', 'plugins', 'data'), { recursive: true });
|
||||
writeFileSync(join(dir, DATA_REL), 'not a directory');
|
||||
assert.doesNotThrow(() => { r = emit('user_input', {}, { env: { HOME: dir } }); });
|
||||
assert.equal(r.written, false);
|
||||
assert.ok(r.reason && r.reason.length > 0, JSON.stringify(r));
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('resolveStatsPath honors CLAUDE_PLUGIN_DATA env var', () => {
|
||||
// Inverted contract (was: resolveStatsPath({}) === null).
|
||||
test('resolveStatsPath honors CLAUDE_PLUGIN_DATA, else the stats-append directory', () => {
|
||||
const r = resolveStatsPath({ CLAUDE_PLUGIN_DATA: '/var/data/plugin' });
|
||||
assert.equal(r, '/var/data/plugin/trekexecute-stats.jsonl');
|
||||
assert.equal(resolveStatsPath({}), null);
|
||||
assert.equal(resolveStatsPath({ HOME: '/h' }), join(resolveStatsDir({ HOME: '/h' }), 'trekexecute-stats.jsonl'));
|
||||
assert.equal(resolveStatsPath({ HOME: '/h', CLAUDE_PLUGIN_DATA: '' }), join('/h', DATA_REL, 'trekexecute-stats.jsonl'));
|
||||
});
|
||||
|
||||
test('the Phase 8 and override events are known events', () => {
|
||||
for (const e of ['main-merge-approved', 'main-merge-declined', 'gates_mode_flag_override']) {
|
||||
assert.equal(buildRecord(e).known_event, true, `${e} is emitted by /trekexecute by design`);
|
||||
}
|
||||
});
|
||||
|
||||
// Regression guard: one resolver - event-emit carries no second copy of the fallback path.
|
||||
test('event-emit has no copy of the fallback directory name', () => {
|
||||
assert.doesNotMatch(readFileSync(SHIM, 'utf-8'), /voyage-ktg-plugin-marketplace/);
|
||||
});
|
||||
|
||||
test('CLI shim writes via emit when CLAUDE_PLUGIN_DATA is set', () => {
|
||||
|
|
@ -126,13 +184,107 @@ test('CLI shim writes via emit when CLAUDE_PLUGIN_DATA is set', () => {
|
|||
}
|
||||
});
|
||||
|
||||
test('CLI shim with malformed --payload returns reason payload-not-json (exit 0)', () => {
|
||||
const r = execFileSync(process.execPath, [
|
||||
SHIM, '--event', 'user_input', '--payload', 'not-json{{',
|
||||
], { encoding: 'utf-8' });
|
||||
const parsed = JSON.parse(r.trim());
|
||||
assert.equal(parsed.written, false);
|
||||
assert.equal(parsed.reason, 'payload-not-json');
|
||||
// Inverted contract (was: "... payload-not-json (exit 0)"): every written:false exits 1.
|
||||
test('CLI shim with malformed --payload returns reason payload-not-json (exit 1)', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const r = runShim(SHIM, ['--event', 'user_input', '--payload', 'not-json{{'], isolatedEnv(home));
|
||||
assert.equal(r.status, 1, r.stdout + r.stderr);
|
||||
const parsed = JSON.parse(r.stdout.trim());
|
||||
assert.equal(parsed.written, false);
|
||||
assert.equal(parsed.reason, 'payload-not-json');
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI: with CLAUDE_PLUGIN_DATA empty the event lands under HOME', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const r = runShim(SHIM, ['--event', 'main-merge-gate', '--payload', '{}'], isolatedEnv(home));
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.equal(JSON.parse(r.stdout.trim()).written, true, r.stdout);
|
||||
const recs = records(join(home, DATA_REL, 'trekexecute-stats.jsonl'));
|
||||
assert.equal(recs.length, 1);
|
||||
assert.equal(recs[0].event, 'main-merge-gate');
|
||||
assert.equal(recs[0].known_event, true);
|
||||
assert.deepEqual(recs[0].payload, {});
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI: with CLAUDE_PLUGIN_DATA absent the event lands under HOME', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const env = isolatedEnv(home);
|
||||
delete env.CLAUDE_PLUGIN_DATA;
|
||||
const r = runShim(SHIM, ['--event', 'main-merge-gate', '--payload', '{}'], env);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.equal(records(join(home, DATA_REL, 'trekexecute-stats.jsonl')).length, 1);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI: an unwritable target exits 1 with a reason', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
mkdirSync(join(home, '.claude', 'plugins', 'data'), { recursive: true });
|
||||
writeFileSync(join(home, DATA_REL), 'not a directory');
|
||||
const r = runShim(SHIM, ['--event', 'main-merge-gate', '--payload', '{}'], isolatedEnv(home));
|
||||
assert.equal(r.status, 1, r.stdout + r.stderr);
|
||||
const out = JSON.parse(r.stdout.trim());
|
||||
assert.equal(out.written, false);
|
||||
assert.ok(out.reason && out.reason.length > 0, r.stdout);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI: a usage error exits 1 with a reason', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const r = runShim(SHIM, [], isolatedEnv(home));
|
||||
assert.equal(r.status, 1, r.stdout + r.stderr);
|
||||
const out = JSON.parse(r.stdout.trim());
|
||||
assert.equal(out.written, false);
|
||||
assert.match(out.reason, /usage/);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI: event-emit writes when it is run through a symlink', () => {
|
||||
const home = tmp();
|
||||
const links = tmp('stats-event-emit-link-');
|
||||
try {
|
||||
const link = join(links, 'event-emit.mjs');
|
||||
symlinkSync(SHIM, link);
|
||||
const r = runShim(link, ['--event', 'main-merge-gate', '--payload', '{}'], isolatedEnv(home));
|
||||
assert.ok(r.stdout.trim().length > 0, `no result line - the CLI guard did not fire; stderr: ${r.stderr}`);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.ok(existsSync(join(home, DATA_REL, 'trekexecute-stats.jsonl')), `no record landed; stdout: "${r.stdout}"`);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
rmSync(links, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI: event-emit writes when its path contains a space', () => {
|
||||
const home = tmp();
|
||||
const spaced = tmp('stats event emit space-');
|
||||
try {
|
||||
cpSync(join(ROOT, 'lib'), join(spaced, 'with space', 'lib'), { recursive: true });
|
||||
const cli = join(spaced, 'with space', 'lib', 'stats', 'event-emit.mjs');
|
||||
const r = runShim(cli, ['--event', 'main-merge-gate', '--payload', '{}'], isolatedEnv(home));
|
||||
assert.ok(r.stdout.trim().length > 0, `no result line - the CLI guard did not fire; stderr: ${r.stderr}`);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.ok(existsSync(join(home, DATA_REL, 'trekexecute-stats.jsonl')), `no record landed; stdout: "${r.stdout}"`);
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
rmSync(spaced, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('concurrent appends do not corrupt JSONL (smoke)', async () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue