mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-23 03:27:50 +09:00
fix: record skill runs under OpenCode and report unmeasured skills honestly
Under OpenCode the skill-health dashboard had no writer and no honest fallback, so it reported every discovered skill as healthy (#2463). Root cause: `post:skill:track` is only wired for Claude Code, where `posttooluse-dispatcher.js` fans the event out to `skill-run-tracker.js`. The OpenCode plugin re-implements the harness events instead of reusing that dispatcher, so nothing ever wrote `skill-runs.jsonl` — and the tracker's matcher only accepted the exact Claude Code tool name `Skill`, while OpenCode calls the tool `skill`, so even wiring it would have dropped every event. Commands that locate ECC through the inlined resolver in `scripts/lib/resolve-ecc-root.js` also fell back to `~/.claude`, which does not contain ECC, so they could not find their scripts. Changes: - `.opencode/plugins/ecc-hooks.ts`: dispatch `post:skill:track` from `tool.execute.after`. The tracker module is imported lazily and behind try/catch, matching the existing `changed-files-store` pattern: this file is OpenCode's startup entry point, so a static import failure would crash the session before any hook runs (#2530). The module is reused rather than reimplemented, keeping its identifier validation, length bounds and "never persist prompt text" guarantees. - `scripts/hooks/skill-run-tracker.js`: match the tool name case-insensitively so `Skill` and `skill` are both accepted. - `scripts/lib/skill-evolution/health.js`: split the summary into measured/healthy/declining/unmeasured instead of deriving `healthy = total - declining`, which turned "no telemetry" into "everything is fine" for every skill. - `scripts/lib/skill-evolution/dashboard.js`: show the breakdown and warn explicitly when skills have no recorded runs, since absence of telemetry is not a health signal. Under OpenCode the resolver also needs `CLAUDE_PLUGIN_ROOT`, which only the plugin can supply: `shell.env` exports it when a root is found, without replacing a root the user set or one an earlier plugin already put in `output.env`. Root discovery is per capability and probes required runtime files — never optional content. Probing for `skills/continuous-learning-v2` (the `workflow-quality` module) rejected supported modular installs that ship the plugin and tracker without that optional module, disabling telemetry and the root export for exactly those users. Also fixes an incidental bug: the compiled plugin bundle is ESM, where `__dirname` is undefined, so the ECC version lookup silently fell back to its default. The module directory is now derived from `import.meta.url`. Tests: `tests/opencode-plugin-hooks.test.js` covers the telemetry dispatch (skill tool recorded, other tools ignored, minimal profile respected), a modular install without the optional skills module, the root export and both override paths; `tests/hooks/skill-run-tracker.test.js` covers the case-insensitive matcher; `tests/lib/skill-evolution.test.js` and `tests/lib/skill-dashboard.test.js` cover the unmeasured bucket, the warning and the aged-out-coverage wording. Refs #2463, #2530
This commit is contained in:
1 parent
ef648e0189
commit
b4943a5e4b
8 files changed
+865
-22
No files matched your search
+186
-10
@@ -16,9 +16,26 @@
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import * as fs from "fs"
|
||||
import * as path from "path"
|
||||
import { fileURLToPath, pathToFileURL } from "url"
|
||||
import changedFilesTool from "../tools/changed-files.ts"
|
||||
import dependencyAnalyzerTool from "../tools/dependency-analyzer.ts"
|
||||
|
||||
/**
|
||||
* Directory of this module.
|
||||
*
|
||||
* The plugin is ESM: tsc compiles it to .opencode/dist/plugins/*.js and OpenCode
|
||||
* loads the .ts directly, so `__dirname` is not defined there. Reading it used to
|
||||
* throw inside a try/catch and silently degrade the reported ECC version to the
|
||||
* default. `import.meta.url` is the ESM source of truth; the `__dirname` check is
|
||||
* kept first only so a CJS consumer of this module still resolves correctly.
|
||||
*/
|
||||
function resolvePluginDir(): string {
|
||||
if (typeof __dirname === "string" && __dirname) return __dirname
|
||||
return path.dirname(fileURLToPath(import.meta.url))
|
||||
}
|
||||
|
||||
const PLUGIN_DIR = resolvePluginDir()
|
||||
|
||||
/**
|
||||
* Type definitions for better type safety
|
||||
*/
|
||||
@@ -51,17 +68,84 @@ interface TodoEvent {
|
||||
}
|
||||
|
||||
/**
|
||||
* Read ECC version from package.json
|
||||
* Falls back to a default if package.json cannot be read
|
||||
* Artifacts that identify an ECC root, one per capability this plugin needs.
|
||||
*
|
||||
* These are required runtime files, never optional content: `skills/` belongs to
|
||||
* the optional `workflow-quality` module, so probing for a skill rejected
|
||||
* supported modular installs (e.g. `platform-configs` + `hooks-runtime` without
|
||||
* `workflow-quality`) that still ship both the plugin and the tracker — silently
|
||||
* disabling skill telemetry and the shell root export for those users.
|
||||
*
|
||||
* Each probe is the exact artifact the caller is about to use, so detection is
|
||||
* layout detection rather than an authenticity control: a directory only
|
||||
* qualifies because it contains what we are going to read.
|
||||
*/
|
||||
function getECCVersion(): string {
|
||||
try {
|
||||
const packageJsonPath = path.resolve(__dirname, "../../package.json")
|
||||
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf-8"))
|
||||
return packageJson.version || "2.0.0"
|
||||
} catch {
|
||||
return "2.0.0"
|
||||
const ECC_ROOT_PROBES = {
|
||||
// scripts/lib/utils.js is the shared library every scripts/ module requires.
|
||||
root: [path.join("scripts", "lib", "utils.js")],
|
||||
// What the skill-run telemetry hook imports at runtime.
|
||||
skillTracker: [path.join("scripts", "hooks", "skill-run-tracker.js")],
|
||||
// What the resolver embedded in commands requires once they read
|
||||
// CLAUDE_PLUGIN_ROOT, and the version lookup for shell.env.
|
||||
resolver: [path.join("scripts", "lib", "resolve-ecc-root.js")],
|
||||
} as const
|
||||
|
||||
/**
|
||||
* Walk upwards from `startDir` looking for an ancestor that contains every
|
||||
* relative path in `probes`.
|
||||
*
|
||||
* The plugin ships in three layouts — `<root>/plugins` (installed), `<root>/dist/plugins`
|
||||
* (compiled) and `<repo>/.opencode/plugins` (repo checkout) — so the number of
|
||||
* levels back to the root is not fixed. Probing beats a hardcoded `..`: a wrong
|
||||
* root would make every command resolve against a directory that has no scripts.
|
||||
*
|
||||
* The walk is bounded to 5 levels so it cannot wander into unrelated ancestors.
|
||||
*
|
||||
* Returns null when no ancestor qualifies, so callers can leave the capability
|
||||
* disabled instead of advertising a root that does not hold the artifact.
|
||||
*/
|
||||
function findEccRootDir(startDir: string, probes: readonly string[]): string | null {
|
||||
let current = startDir
|
||||
for (let depth = 0; depth < 5; depth += 1) {
|
||||
if (probes.every((probe) => fs.existsSync(path.join(current, probe)))) {
|
||||
return current
|
||||
}
|
||||
const parent = path.dirname(current)
|
||||
if (parent === current) break
|
||||
current = parent
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the ECC version from package.json.
|
||||
*
|
||||
* Candidates are tried in order — the resolved ECC root first, then the two
|
||||
* plausible parent depths of the plugin — because the layout differs between an
|
||||
* installed root (`<root>/plugins`), the compiled bundle (`<root>/dist/plugins`)
|
||||
* and a repo checkout (`<repo>/.opencode/plugins`). Falls back to a default if
|
||||
* no package.json can be read.
|
||||
*/
|
||||
function getECCVersion(eccRootDir?: string | null): string {
|
||||
const candidates = [
|
||||
eccRootDir,
|
||||
path.resolve(PLUGIN_DIR, ".."),
|
||||
path.resolve(PLUGIN_DIR, "..", ".."),
|
||||
]
|
||||
|
||||
for (const candidate of candidates) {
|
||||
if (!candidate) continue
|
||||
try {
|
||||
const packageJson = JSON.parse(fs.readFileSync(path.join(candidate, "package.json"), "utf-8"))
|
||||
if (typeof packageJson.version === "string" && packageJson.version) {
|
||||
return packageJson.version
|
||||
}
|
||||
} catch {
|
||||
// Try the next candidate.
|
||||
}
|
||||
}
|
||||
|
||||
return "2.0.0"
|
||||
}
|
||||
|
||||
type ECCHooksPluginFn = (input: PluginInput) => Promise<Record<string, unknown>>
|
||||
@@ -135,6 +219,60 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
||||
.catch(() => {})
|
||||
}
|
||||
|
||||
// Claude Code routes post:skill:track through
|
||||
// scripts/hooks/posttooluse-dispatcher.js, which OpenCode never invokes.
|
||||
// Without a caller here, nothing ever writes skill-runs.jsonl and
|
||||
// `skills-health.js --dashboard` has no data to report on any skill (#2463).
|
||||
//
|
||||
// The tracker module is reused rather than reimplemented so its identifier
|
||||
// validation, length bounds and "never persist prompt text" guarantees stay
|
||||
// the ones that were written and audited for the Claude Code path.
|
||||
//
|
||||
// Loaded lazily, for the same reason as the store above: this file is
|
||||
// OpenCode's startup entry point, and a static import of a missing module
|
||||
// crashes the plugin -- and with it the whole session -- before any hook can
|
||||
// run (#2530). A runtime file URL also keeps tsc from resolving a relative
|
||||
// specifier that exists in no compiled layout.
|
||||
type SkillRunTrackerModule = { run?: (payload: unknown) => void }
|
||||
let recordSkillRun: ((payload: unknown) => void) | undefined
|
||||
// Detection is scoped to the tracker itself, so a modular install that omits
|
||||
// optional content still gets telemetry.
|
||||
const trackerRoot = findEccRootDir(PLUGIN_DIR, ECC_ROOT_PROBES.skillTracker)
|
||||
// The root advertised to commands is a different question: those commands
|
||||
// require the resolver, so a root without it would break them again.
|
||||
const eccRoot =
|
||||
findEccRootDir(PLUGIN_DIR, ECC_ROOT_PROBES.resolver) ?? findEccRootDir(PLUGIN_DIR, ECC_ROOT_PROBES.root)
|
||||
try {
|
||||
const trackerPath = trackerRoot
|
||||
? path.join(trackerRoot, "scripts", "hooks", "skill-run-tracker.js")
|
||||
: null
|
||||
if (!trackerPath || !fs.existsSync(trackerPath)) {
|
||||
throw new Error("skill-run-tracker.js not found")
|
||||
}
|
||||
// CJS module loaded from ESM: run() is on the namespace in Node/Bun, but a
|
||||
// transpiled default export must stay supported.
|
||||
const loaded = (await import(pathToFileURL(trackerPath).href)) as SkillRunTrackerModule & {
|
||||
default?: SkillRunTrackerModule
|
||||
}
|
||||
const entry = typeof loaded.run === "function" ? loaded : loaded.default
|
||||
if (!entry || typeof entry.run !== "function") {
|
||||
throw new Error("skill-run-tracker.js exposes no run()")
|
||||
}
|
||||
recordSkillRun = entry.run
|
||||
} catch {
|
||||
// Same deferred-log rationale as the store above: guarantee that a
|
||||
// telemetry failure can never escape this catch and abort startup.
|
||||
Promise.resolve()
|
||||
.then(() =>
|
||||
log(
|
||||
"warn",
|
||||
"[ECC] skill-run telemetry disabled: could not load scripts/hooks/skill-run-tracker.js. " +
|
||||
"Run `ecc repair --target opencode` to restore the missing files. Other ECC hooks are unaffected."
|
||||
)
|
||||
)
|
||||
.catch(() => {})
|
||||
}
|
||||
|
||||
const normalizeProfile = (value: string | undefined): HookProfile => {
|
||||
if (value === "minimal" || value === "strict") return value
|
||||
return "standard"
|
||||
@@ -264,6 +402,25 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
||||
) {
|
||||
log("info", "[ECC] PR created - check GitHub Actions status")
|
||||
}
|
||||
|
||||
// Skill telemetry (#2463). OpenCode's tool is named `skill`, so the
|
||||
// harness-side name is matched here (case-insensitively) and the payload
|
||||
// is handed over in the shape the tracker already understands.
|
||||
if (
|
||||
hookEnabled("post:skill:track", ["standard", "strict"]) &&
|
||||
typeof input.tool === "string" &&
|
||||
input.tool.trim().toLowerCase() === "skill"
|
||||
) {
|
||||
try {
|
||||
recordSkillRun?.({
|
||||
tool_name: "Skill",
|
||||
tool_input: input.args ?? {},
|
||||
tool_response: output,
|
||||
})
|
||||
} catch {
|
||||
// Telemetry is best-effort; never let it affect the tool result.
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
@@ -483,7 +640,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
||||
*/
|
||||
"shell.env": async (_input: { cwd: string }, output: { env: Record<string, string> }) => {
|
||||
const env: Record<string, string> = {
|
||||
ECC_VERSION: getECCVersion(),
|
||||
ECC_VERSION: getECCVersion(eccRoot),
|
||||
ECC_PLUGIN: "true",
|
||||
ECC_HOOK_PROFILE: currentProfile,
|
||||
ECC_DISABLED_HOOKS: process.env.ECC_DISABLED_HOOKS || "",
|
||||
@@ -523,6 +680,25 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
||||
env.PRIMARY_LANGUAGE = detected[0]
|
||||
}
|
||||
|
||||
// resolve-ecc-root.js only probes ~/.claude and Claude Code's plugin
|
||||
// cache, so under OpenCode it fell back to ~/.claude and every command
|
||||
// carrying the inlined resolver failed to find its scripts. That resolver
|
||||
// reads CLAUDE_PLUGIN_ROOT first, so exporting the root here fixes them
|
||||
// through the mechanism ECC already provides -- rather than editing the
|
||||
// command files that embed the locator.
|
||||
//
|
||||
// Only set when a root actually exists, and never replace a root someone
|
||||
// else already chose: neither the process environment (set by the user)
|
||||
// nor `output.env` (set by an earlier plugin). The final merge below
|
||||
// overwrites output.env, so checking it only in `env` would clobber it.
|
||||
const rootAlreadySet = Boolean(
|
||||
(process.env.CLAUDE_PLUGIN_ROOT || "").trim() ||
|
||||
(output.env?.CLAUDE_PLUGIN_ROOT || "").trim()
|
||||
)
|
||||
if (eccRoot && !rootAlreadySet) {
|
||||
env.CLAUDE_PLUGIN_ROOT = eccRoot
|
||||
}
|
||||
|
||||
// OpenCode reads the supplied output object and ignores callback return values.
|
||||
output.env = { ...output.env, ...env }
|
||||
},
|
||||
|
||||
@@ -16,8 +16,9 @@
|
||||
* skill id is an identifier, not free text, so anything that does not look like
|
||||
* one is dropped rather than written through.
|
||||
*
|
||||
* Best-effort: never blocks tool execution. Runs under the PostToolUse
|
||||
* dispatcher, which owns stdin, pass-through, and exit codes.
|
||||
* Best-effort: never blocks tool execution. Under Claude Code it runs via the
|
||||
* PostToolUse dispatcher, which owns stdin, pass-through, and exit codes; under
|
||||
* OpenCode the ecc-hooks plugin calls run() directly with an equivalent payload.
|
||||
*
|
||||
* Cross-platform (Windows, macOS, Linux); CommonJS.
|
||||
*/
|
||||
@@ -102,6 +103,28 @@ function deriveOutcome(payload) {
|
||||
return 'success';
|
||||
}
|
||||
|
||||
const SKILL_TOOL_NAME = 'skill';
|
||||
|
||||
/**
|
||||
* Report whether a hook payload describes a skill tool invocation.
|
||||
*
|
||||
* Claude Code names the tool `Skill`; OpenCode and other harnesses name it
|
||||
* `skill`. Comparing the raw string meant a case difference silently disabled
|
||||
* the whole write side again — the hook ran, dropped every event, and the
|
||||
* dashboard kept reporting unmeasured skills with no hint why.
|
||||
*
|
||||
* @param {{tool_name?: unknown} | null} payload
|
||||
* @returns {boolean}
|
||||
*/
|
||||
function isSkillToolCall(payload) {
|
||||
if (!payload || typeof payload !== 'object') {
|
||||
return false;
|
||||
}
|
||||
|
||||
const toolName = payload.tool_name;
|
||||
return typeof toolName === 'string' && toolName.trim().toLowerCase() === SKILL_TOOL_NAME;
|
||||
}
|
||||
|
||||
function buildRecord(payload) {
|
||||
const skillId = extractSkillId(payload.tool_input);
|
||||
if (!skillId) {
|
||||
@@ -130,12 +153,22 @@ function buildRecord(payload) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Record a skill invocation from a hook payload.
|
||||
*
|
||||
* Accepts the raw JSON string Claude Code's dispatcher reads from stdin, or an
|
||||
* already-parsed payload for in-process callers such as the OpenCode plugin.
|
||||
* Non-skill tools and unparsable input are ignored.
|
||||
*
|
||||
* @param {string | object | null | undefined} rawInput
|
||||
* @returns {void}
|
||||
*/
|
||||
function run(rawInput) {
|
||||
try {
|
||||
const payload = typeof rawInput === 'string'
|
||||
? (rawInput.trim() ? JSON.parse(rawInput) : {})
|
||||
: rawInput;
|
||||
if (payload && typeof payload === 'object' && payload.tool_name === 'Skill') {
|
||||
if (isSkillToolCall(payload)) {
|
||||
const record = buildRecord(payload);
|
||||
if (record) {
|
||||
recordSkillExecution(record);
|
||||
@@ -146,4 +179,4 @@ function run(rawInput) {
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = { buildRecord, deriveOutcome, extractSkillId, run };
|
||||
module.exports = { buildRecord, deriveOutcome, extractSkillId, isSkillToolCall, run };
|
||||
@@ -328,6 +328,18 @@ function renderVersionTimelinePanel(skillsById, options = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the full skill-health dashboard: header, coverage warning and panels.
|
||||
*
|
||||
* Coverage is stated explicitly because the 30-day window can empty out while
|
||||
* runs are still retained, so "unmeasured" must not be reported as "never
|
||||
* recorded" (#2463).
|
||||
*
|
||||
* @param {object} [options]
|
||||
* @param {string} [options.now] ISO timestamp used as the evaluation instant
|
||||
* @param {string} [options.panel] render only this panel
|
||||
* @returns {{text: string, data: object}}
|
||||
*/
|
||||
function renderDashboard(options = {}) {
|
||||
const now = options.now || new Date().toISOString();
|
||||
const nowMs = Date.parse(now);
|
||||
@@ -359,12 +371,31 @@ function renderDashboard(options = {}) {
|
||||
const header = [
|
||||
'ECC Skill Health Dashboard',
|
||||
`Generated: ${now}`,
|
||||
`Skills: ${summary.total_skills} total, ${summary.healthy_skills} healthy, ${summary.declining_skills} declining`,
|
||||
`Skills: ${summary.total_skills} total, ${summary.measured_skills} measured, ${summary.healthy_skills} healthy, ${summary.declining_skills} declining, ${summary.unmeasured_skills} unmeasured`,
|
||||
'',
|
||||
];
|
||||
|
||||
textParts.push(header.join('\n'));
|
||||
|
||||
if (summary.unmeasured_skills > 0) {
|
||||
// The window can empty out while runs are still retained, so "unmeasured"
|
||||
// must not be reported as "never recorded" (#2463). Three distinct states,
|
||||
// because "no recent runs" alone does not say which one applies.
|
||||
const recentRuns = health.filterRecordsWithinDays(records, nowMs, 30).length;
|
||||
const coverageNote = records.length === 0
|
||||
? 'Nothing has ever been recorded, so the panels below are not a signal about skill health.'
|
||||
: recentRuns === 0
|
||||
? `All ${records.length} retained run${records.length === 1 ? '' : 's'} are older than the 30-day window: coverage aged out rather than never existing. The panels below still show that history.`
|
||||
: `Coverage is partial: ${recentRuns} recent run${recentRuns === 1 ? '' : 's'} inside the window, ${records.length - recentRuns} older one${records.length - recentRuns === 1 ? '' : 's'} retained outside it. Only the measured skills can be judged.`;
|
||||
|
||||
textParts.push([
|
||||
`WARNING: ${summary.unmeasured_skills} of ${summary.total_skills} skills have no recorded run in the last 30 days.`,
|
||||
'This dashboard can say nothing about them: no telemetry is an absence of evidence,',
|
||||
'not a healthy result. Their success rate reads "n/a", which means unknown.',
|
||||
coverageNote,
|
||||
].join('\n'));
|
||||
}
|
||||
|
||||
if (selectedPanel) {
|
||||
const result = panelRenderers[selectedPanel]();
|
||||
panels[selectedPanel] = result.data;
|
||||
|
||||
@@ -26,15 +26,36 @@ function formatRate(value) {
|
||||
return `${Math.round(value * 100)}%`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Summarise a health report into counting buckets.
|
||||
*
|
||||
* A skill with no runs in the window is not healthy, it is unmeasured: the
|
||||
* trend helpers return 'stable' when both rates are null, so deriving health
|
||||
* from "not declining" turned a missing telemetry writer into a green
|
||||
* dashboard across every skill (#2463). A declining skill always has runs, so
|
||||
* the buckets partition the total.
|
||||
*
|
||||
* @param {{skills: Array<{declining?: boolean, run_count_30d?: number}>}} report
|
||||
* @returns {{total_skills: number, measured_skills: number, healthy_skills: number, declining_skills: number, unmeasured_skills: number}}
|
||||
*/
|
||||
function summarizeHealthReport(report) {
|
||||
const totalSkills = report.skills.length;
|
||||
// A skill with no runs in the window is not healthy, it is unmeasured.
|
||||
// Folding it into "healthy" turned a missing telemetry writer into a green
|
||||
// dashboard across every skill (#2463) — false confidence that never triggers
|
||||
// an alert. A declining skill always has runs, so the buckets partition the
|
||||
// total.
|
||||
const unmeasuredSkills = report.skills.filter(skill => !skill.run_count_30d).length;
|
||||
const decliningSkills = report.skills.filter(skill => skill.declining).length;
|
||||
const healthySkills = totalSkills - decliningSkills;
|
||||
const measuredSkills = totalSkills - unmeasuredSkills;
|
||||
const healthySkills = measuredSkills - decliningSkills;
|
||||
|
||||
return {
|
||||
total_skills: totalSkills,
|
||||
measured_skills: measuredSkills,
|
||||
healthy_skills: healthySkills,
|
||||
declining_skills: decliningSkills,
|
||||
unmeasured_skills: unmeasuredSkills,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -230,7 +251,7 @@ function formatHealthReport(report, options = {}) {
|
||||
const lines = [
|
||||
'ECC skill health',
|
||||
`Generated: ${report.generated_at}`,
|
||||
`Skills: ${summary.total_skills} total, ${summary.healthy_skills} healthy, ${summary.declining_skills} declining`,
|
||||
`Skills: ${summary.total_skills} total, ${summary.measured_skills} measured, ${summary.healthy_skills} healthy, ${summary.declining_skills} declining, ${summary.unmeasured_skills} unmeasured`,
|
||||
'',
|
||||
'skill version 7d 30d trend pending last run',
|
||||
'--------------------------------------------------------------------------',
|
||||
|
||||
@@ -15,7 +15,7 @@ const fs = require('fs');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
|
||||
const { buildRecord, deriveOutcome, extractSkillId, run } = require('../../scripts/hooks/skill-run-tracker');
|
||||
const { buildRecord, deriveOutcome, extractSkillId, isSkillToolCall, run } = require('../../scripts/hooks/skill-run-tracker');
|
||||
const { readHooksConfig } = require('../../scripts/lib/hooks-config');
|
||||
const {
|
||||
MAX_RUN_RECORDS,
|
||||
@@ -152,6 +152,57 @@ test('run ignores non-Skill tools and malformed input without throwing', () => {
|
||||
assert.doesNotThrow(() => run(''));
|
||||
});
|
||||
|
||||
// ── tool-name tolerance ───────────────────────────────────────────────────────
|
||||
// OpenCode names the tool `skill`. A case-sensitive matcher meant the hook ran,
|
||||
// dropped every event, and the dashboard kept reporting "unmeasured" with no
|
||||
// indication that a writer existed at all.
|
||||
|
||||
test('isSkillToolCall accepts both harness spellings and rejects everything else', () => {
|
||||
assert.strictEqual(isSkillToolCall({ tool_name: 'Skill' }), true);
|
||||
assert.strictEqual(isSkillToolCall({ tool_name: 'skill' }), true);
|
||||
assert.strictEqual(isSkillToolCall({ tool_name: ' SKILL ' }), true);
|
||||
assert.strictEqual(isSkillToolCall({ tool_name: 'Bash' }), false);
|
||||
assert.strictEqual(isSkillToolCall({ tool_name: '' }), false);
|
||||
assert.strictEqual(isSkillToolCall({ tool_name: 42 }), false);
|
||||
assert.strictEqual(isSkillToolCall({}), false);
|
||||
assert.strictEqual(isSkillToolCall(null), false);
|
||||
});
|
||||
|
||||
test('run records a run for a lowercase skill tool name', () => {
|
||||
withTempHome(homeDir => {
|
||||
const previousHome = process.env.HOME;
|
||||
const previousUserProfile = process.env.USERPROFILE;
|
||||
process.env.HOME = homeDir;
|
||||
process.env.USERPROFILE = homeDir;
|
||||
try {
|
||||
run(payload({ tool_name: 'skill' }));
|
||||
|
||||
const records = readSkillExecutionRecords({ homeDir });
|
||||
assert.strictEqual(records.length, 1);
|
||||
assert.strictEqual(records[0].skill_id, 'code-review');
|
||||
} 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;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
test('run still records nothing for a non-skill tool', () => {
|
||||
withTempHome(homeDir => {
|
||||
const previousHome = process.env.HOME;
|
||||
const previousUserProfile = process.env.USERPROFILE;
|
||||
process.env.HOME = homeDir;
|
||||
process.env.USERPROFILE = homeDir;
|
||||
try {
|
||||
run(payload({ tool_name: 'bash' }));
|
||||
assert.strictEqual(readSkillExecutionRecords({ homeDir }).length, 0);
|
||||
} 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;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── JSONL sink bounds ────────────────────────────────────────────────────────
|
||||
|
||||
test('recordSkillExecution writes the sink owner-only', function () {
|
||||
|
||||
@@ -358,6 +358,179 @@ function runTests() {
|
||||
}, /Unknown panel/);
|
||||
})) passed++; else failed++;
|
||||
|
||||
// A panel that reports every never-used skill as healthy is worse than an
|
||||
// empty one: it looks like evidence and never raises an alert (#2463).
|
||||
if (test('renderDashboard warns that unmeasured skills say nothing about health', () => {
|
||||
const result = dashboard.renderDashboard({
|
||||
skillsRoot,
|
||||
learnedRoot,
|
||||
importedRoot,
|
||||
homeDir,
|
||||
runsFilePath: path.join(homeDir, '.claude', 'state', 'no-such-runs.jsonl'),
|
||||
now,
|
||||
warnThreshold: 0.1,
|
||||
});
|
||||
|
||||
const summary = result.data.summary;
|
||||
assert.ok(summary.unmeasured_skills > 0, 'expected unmeasured skills');
|
||||
assert.ok(
|
||||
result.text.includes('WARNING'),
|
||||
'expected an explicit warning when skills have no telemetry'
|
||||
);
|
||||
assert.match(result.text, /unmeasured/);
|
||||
assert.strictEqual(
|
||||
summary.healthy_skills,
|
||||
0,
|
||||
'skills without runs must not be counted as healthy'
|
||||
);
|
||||
})) passed++; else failed++;
|
||||
|
||||
// Isolated roots so the full-coverage case is guaranteed rather than
|
||||
// conditional: every discovered skill needs a run inside the 30-day window.
|
||||
if (test('renderDashboard omits the warning once every skill is measured', () => {
|
||||
const coveredRoot = createTempDir('ecc-dashboard-covered-');
|
||||
const coveredLearned = createTempDir('ecc-dashboard-covered-');
|
||||
const coveredImported = createTempDir('ecc-dashboard-covered-');
|
||||
const coveredRuns = path.join(homeDir, '.claude', 'state', 'covered-skill-runs.jsonl');
|
||||
try {
|
||||
for (const name of ['covered-a', 'covered-b']) {
|
||||
createSkill(coveredRoot, name, `# ${name}\n`);
|
||||
}
|
||||
appendJsonl(coveredRuns, ['covered-a', 'covered-b'].map(skill_id => ({
|
||||
skill_id,
|
||||
skill_version: 'v1',
|
||||
task_description: `Skill invocation: ${skill_id}`,
|
||||
outcome: 'success',
|
||||
recorded_at: now,
|
||||
})));
|
||||
|
||||
const result = dashboard.renderDashboard({
|
||||
skillsRoot: coveredRoot,
|
||||
learnedRoot: coveredLearned,
|
||||
importedRoot: coveredImported,
|
||||
homeDir,
|
||||
runsFilePath: coveredRuns,
|
||||
now,
|
||||
warnThreshold: 0.1,
|
||||
});
|
||||
|
||||
assert.strictEqual(result.data.summary.unmeasured_skills, 0);
|
||||
assert.strictEqual(result.data.summary.measured_skills, 2);
|
||||
assert.ok(
|
||||
!result.text.includes('WARNING'),
|
||||
'no warning expected when every skill has a run in the window'
|
||||
);
|
||||
assert.strictEqual(
|
||||
result.data.summary.total_skills,
|
||||
result.data.summary.measured_skills + result.data.summary.unmeasured_skills,
|
||||
'measured and unmeasured must partition the total'
|
||||
);
|
||||
} finally {
|
||||
cleanupTempDir(coveredRoot);
|
||||
cleanupTempDir(coveredLearned);
|
||||
cleanupTempDir(coveredImported);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// The 30-day window can empty out while runs are still retained. Saying
|
||||
// "nothing has ever been recorded" then is factually wrong.
|
||||
if (test('the unmeasured warning distinguishes aged-out coverage from no history', () => {
|
||||
const agedRuns = path.join(homeDir, '.claude', 'state', 'aged-skill-runs.jsonl');
|
||||
const agedOut = new Date(Date.parse(now) - (60 * 24 * 60 * 60 * 1000)).toISOString();
|
||||
appendJsonl(agedRuns, [{
|
||||
skill_id: 'alpha',
|
||||
skill_version: 'v1',
|
||||
task_description: 'Skill invocation: alpha',
|
||||
outcome: 'failure',
|
||||
recorded_at: agedOut,
|
||||
}]);
|
||||
|
||||
const result = dashboard.renderDashboard({
|
||||
skillsRoot,
|
||||
learnedRoot,
|
||||
importedRoot,
|
||||
homeDir,
|
||||
runsFilePath: agedRuns,
|
||||
now,
|
||||
warnThreshold: 0.1,
|
||||
});
|
||||
|
||||
assert.ok(result.data.summary.unmeasured_skills > 0, 'expected unmeasured skills');
|
||||
assert.ok(result.text.includes('WARNING'), 'expected the warning');
|
||||
assert.ok(
|
||||
result.text.includes('aged out'),
|
||||
'retained history outside the window must be described as aged out'
|
||||
);
|
||||
assert.ok(
|
||||
!result.text.includes('Nothing has ever been recorded'),
|
||||
'must not claim there is no history when runs are retained'
|
||||
);
|
||||
})) passed++; else failed++;
|
||||
|
||||
// Mixed coverage is the common case: some skills used recently, others not.
|
||||
// It must not be described as aged out, nor as having no history at all.
|
||||
if (test('the unmeasured warning describes partial coverage as partial', () => {
|
||||
const mixedRuns = path.join(homeDir, '.claude', 'state', 'mixed-skill-runs.jsonl');
|
||||
appendJsonl(mixedRuns, [
|
||||
{
|
||||
skill_id: 'alpha',
|
||||
skill_version: 'v1',
|
||||
task_description: 'Skill invocation: alpha',
|
||||
outcome: 'success',
|
||||
recorded_at: now,
|
||||
},
|
||||
{
|
||||
skill_id: 'beta',
|
||||
skill_version: 'v1',
|
||||
task_description: 'Skill invocation: beta',
|
||||
outcome: 'success',
|
||||
recorded_at: new Date(Date.parse(now) - (60 * 24 * 60 * 60 * 1000)).toISOString(),
|
||||
},
|
||||
]);
|
||||
|
||||
const result = dashboard.renderDashboard({
|
||||
skillsRoot,
|
||||
learnedRoot,
|
||||
importedRoot,
|
||||
homeDir,
|
||||
runsFilePath: mixedRuns,
|
||||
now,
|
||||
warnThreshold: 0.1,
|
||||
});
|
||||
|
||||
assert.ok(result.text.includes('WARNING'), 'expected the warning');
|
||||
assert.ok(
|
||||
result.text.includes('Coverage is partial'),
|
||||
'mixed recent and aged runs must be reported as partial coverage'
|
||||
);
|
||||
assert.ok(
|
||||
!result.text.includes('aged out'),
|
||||
'coverage is not aged out while recent runs exist'
|
||||
);
|
||||
assert.ok(
|
||||
!result.text.includes('Nothing has ever been recorded'),
|
||||
'runs are retained, so history is not missing'
|
||||
);
|
||||
})) passed++; else failed++;
|
||||
|
||||
// With no runs at all the stronger statement is the accurate one.
|
||||
if (test('the unmeasured warning says nothing was ever recorded when there is no history', () => {
|
||||
const result = dashboard.renderDashboard({
|
||||
skillsRoot,
|
||||
learnedRoot,
|
||||
importedRoot,
|
||||
homeDir,
|
||||
runsFilePath: path.join(homeDir, '.claude', 'state', 'definitely-absent.jsonl'),
|
||||
now,
|
||||
warnThreshold: 0.1,
|
||||
});
|
||||
|
||||
assert.ok(
|
||||
result.text.includes('Nothing has ever been recorded'),
|
||||
'an empty record set must be reported as missing telemetry'
|
||||
);
|
||||
})) passed++; else failed++;
|
||||
|
||||
console.log('\nCLI integration:');
|
||||
|
||||
if (test('CLI --dashboard --json returns valid JSON with all panels', () => {
|
||||
|
||||
@@ -559,19 +559,67 @@ function runTests() {
|
||||
const summary = health.summarizeHealthReport(report);
|
||||
assert.deepStrictEqual(summary, {
|
||||
total_skills: 6,
|
||||
healthy_skills: 5,
|
||||
measured_skills: 2,
|
||||
healthy_skills: 1,
|
||||
declining_skills: 1,
|
||||
unmeasured_skills: 4,
|
||||
});
|
||||
|
||||
const human = health.formatHealthReport(report, { json: false });
|
||||
assert.match(human, /alpha/);
|
||||
assert.match(human, /worsening/);
|
||||
assert.match(
|
||||
human,
|
||||
new RegExp(`Skills: ${summary.total_skills} total, ${summary.healthy_skills} healthy, ${summary.declining_skills} declining`)
|
||||
// Built from values rather than interpolated into a RegExp: the numbers are
|
||||
// internally generated, but a literal comparison needs no pattern engine.
|
||||
assert.ok(
|
||||
human.includes(
|
||||
`Skills: ${summary.total_skills} total, ${summary.measured_skills} measured, ` +
|
||||
`${summary.healthy_skills} healthy, ${summary.declining_skills} declining, ` +
|
||||
`${summary.unmeasured_skills} unmeasured`
|
||||
),
|
||||
'summary line must report measured and unmeasured counts'
|
||||
);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not report unmeasured skills as healthy (#2463)', () => {
|
||||
// A runs file that does not exist: skills are still discovered, so this
|
||||
// is exactly the "discovered but never executed" state.
|
||||
const emptyReport = health.collectSkillHealth({
|
||||
repoRoot,
|
||||
homeDir,
|
||||
runsFilePath: path.join(homeDir, '.claude', 'state', 'no-such-runs.jsonl'),
|
||||
now,
|
||||
warnThreshold: 0.1,
|
||||
});
|
||||
|
||||
const summary = health.summarizeHealthReport(emptyReport);
|
||||
assert.ok(summary.total_skills > 0, 'expected discovered skills');
|
||||
assert.strictEqual(summary.total_skills, summary.unmeasured_skills);
|
||||
assert.strictEqual(summary.measured_skills, 0);
|
||||
assert.strictEqual(summary.declining_skills, 0);
|
||||
assert.strictEqual(
|
||||
summary.healthy_skills,
|
||||
0,
|
||||
'a skill with no runs is unmeasured, never healthy'
|
||||
);
|
||||
|
||||
// Healthy + declining must account for every measured skill, so a partial
|
||||
// coverage state cannot over-report.
|
||||
const partial = health.summarizeHealthReport({
|
||||
skills: [
|
||||
{ skill_id: 'a', run_count_30d: 4, declining: false },
|
||||
{ skill_id: 'b', run_count_30d: 2, declining: true },
|
||||
{ skill_id: 'c', run_count_30d: 0, declining: false },
|
||||
],
|
||||
});
|
||||
assert.deepStrictEqual(partial, {
|
||||
total_skills: 3,
|
||||
measured_skills: 2,
|
||||
healthy_skills: 1,
|
||||
declining_skills: 1,
|
||||
unmeasured_skills: 1,
|
||||
});
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('treats an unsnapshotted SKILL.md as v1 and orders last_run by actual time', () => {
|
||||
const gammaSkillDir = createSkill(skillsRoot, 'gamma', '# Gamma v1\n');
|
||||
const offsetRunsFile = path.join(homeDir, '.claude', 'state', 'offset-skill-runs.jsonl');
|
||||
|
||||
@@ -79,6 +79,90 @@ async function withTempProject(files, fn) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a fake ECC installation root and copy the compiled plugin into it.
|
||||
*
|
||||
* Reproduces a *modular* install: the runtime scripts the plugin needs are
|
||||
* present, but `skills/` is absent because that directory belongs to the optional
|
||||
* `workflow-quality` module. `options.withSkills` adds it back so the same helper
|
||||
* can prove the difference is what matters.
|
||||
*
|
||||
* @param {{withSkills?: boolean}} [options]
|
||||
* @returns {string} path to the fake installation root
|
||||
*/
|
||||
function createModularInstallRoot(options = {}) {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "ecc-opencode-install-"))
|
||||
const distSource = path.join(__dirname, "..", ".opencode", "dist")
|
||||
const repoRoot = path.join(__dirname, "..")
|
||||
|
||||
for (const dir of ["plugins", "tools"]) {
|
||||
fs.cpSync(path.join(distSource, dir), path.join(root, dir), { recursive: true })
|
||||
}
|
||||
|
||||
// The runtime files an install without `workflow-quality` still ships. Copied
|
||||
// whole rather than file-by-file: scripts/lib has transitive requires (utils.js
|
||||
// alone pulls in agent-data-home), and a partial copy would fail the import for
|
||||
// reasons unrelated to what this test is about.
|
||||
for (const dir of ["hooks", "lib"]) {
|
||||
fs.cpSync(path.join(repoRoot, "scripts", dir), path.join(root, "scripts", dir), {
|
||||
recursive: true,
|
||||
})
|
||||
}
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(root, "package.json"),
|
||||
JSON.stringify({ name: "ecc-universal", version: "0.0.0-test" })
|
||||
)
|
||||
|
||||
// The plugin's tools import @opencode-ai/plugin, which resolves through
|
||||
// node_modules — a real installation has it, an isolated temp dir would not.
|
||||
fs.symlinkSync(path.join(repoRoot, "node_modules"), path.join(root, "node_modules"), "junction")
|
||||
|
||||
if (options.withSkills) {
|
||||
fs.mkdirSync(path.join(root, "skills", "continuous-learning-v2"), { recursive: true })
|
||||
fs.writeFileSync(path.join(root, "skills", "continuous-learning-v2", "SKILL.md"), "# sentinel\n")
|
||||
}
|
||||
|
||||
return root
|
||||
}
|
||||
|
||||
/** Load the compiled plugin from a fake install root instead of the repo dist. */
|
||||
async function loadPluginFrom(root) {
|
||||
return import(pathToFileURL(path.join(root, "plugins", "ecc-hooks.js")).href)
|
||||
}
|
||||
|
||||
/**
|
||||
* Restore environment variables saved with Object.fromEntries of name -> value.
|
||||
*
|
||||
* @param {Record<string, string | undefined>} saved
|
||||
* @returns {void}
|
||||
*/
|
||||
function restoreEnv(saved) {
|
||||
for (const [key, value] of Object.entries(saved)) {
|
||||
if (value === undefined) delete process.env[key]
|
||||
else process.env[key] = value
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the skill-runs sink the tracker wrote under `homeDir`.
|
||||
*
|
||||
* The tracker resolves the sink from the *current* home, so the telemetry tests
|
||||
* move HOME/USERPROFILE to a temp dir and read from there.
|
||||
*
|
||||
* @param {string} homeDir
|
||||
* @returns {Array<Record<string, unknown>>}
|
||||
*/
|
||||
function readRecordedRuns(homeDir) {
|
||||
const runsFilePath = path.join(homeDir, ".claude", "state", "skill-runs.jsonl")
|
||||
if (!fs.existsSync(runsFilePath)) return []
|
||||
return fs
|
||||
.readFileSync(runsFilePath, "utf8")
|
||||
.split("\n")
|
||||
.filter(Boolean)
|
||||
.map((line) => JSON.parse(line))
|
||||
}
|
||||
|
||||
async function main() {
|
||||
console.log("\n=== Testing OpenCode plugin hooks ===\n")
|
||||
|
||||
@@ -381,6 +465,232 @@ async function main() {
|
||||
}
|
||||
),
|
||||
],
|
||||
[
|
||||
"skill telemetry records a run through the shared tracker (#2463)",
|
||||
async () => withTempProject([], async (projectDir) => {
|
||||
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), "ecc-opencode-skill-home-"))
|
||||
const previousHome = process.env.HOME
|
||||
const previousUserProfile = process.env.USERPROFILE
|
||||
process.env.HOME = homeDir
|
||||
process.env.USERPROFILE = homeDir
|
||||
try {
|
||||
const client = createClient()
|
||||
const $ = createFailingShell()
|
||||
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||
|
||||
assert.ok(
|
||||
!client.logs.some(
|
||||
(entry) =>
|
||||
entry.level === "warn" && entry.message.includes("skill-run telemetry disabled")
|
||||
),
|
||||
"Did not expect telemetry to be disabled when scripts/hooks/skill-run-tracker.js is present"
|
||||
)
|
||||
|
||||
// OpenCode names the tool `skill`; the id arrives on `name`.
|
||||
await hooks["tool.execute.after"]({ tool: "skill", args: { name: "code-review" } }, {})
|
||||
|
||||
const records = readRecordedRuns(homeDir)
|
||||
|
||||
assert.strictEqual(records.length, 1, "Expected exactly one recorded skill run")
|
||||
assert.strictEqual(records[0].skill_id, "code-review")
|
||||
assert.strictEqual(records[0].outcome, "success")
|
||||
} finally {
|
||||
restoreEnv({ HOME: previousHome, USERPROFILE: previousUserProfile })
|
||||
fs.rmSync(homeDir, { recursive: true, force: true })
|
||||
}
|
||||
}),
|
||||
],
|
||||
[
|
||||
"skill telemetry records nothing for other tools",
|
||||
async () => withTempProject([], async (projectDir) => {
|
||||
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), "ecc-opencode-skill-home-"))
|
||||
const previousHome = process.env.HOME
|
||||
const previousUserProfile = process.env.USERPROFILE
|
||||
process.env.HOME = homeDir
|
||||
process.env.USERPROFILE = homeDir
|
||||
try {
|
||||
const client = createClient()
|
||||
const $ = createFailingShell()
|
||||
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||
|
||||
await hooks["tool.execute.after"]({ tool: "edit", args: { path: "src/a.ts" } }, {})
|
||||
await hooks["tool.execute.after"]({ tool: "bash", args: { command: "ls" } }, {})
|
||||
|
||||
assert.strictEqual(readRecordedRuns(homeDir).length, 0)
|
||||
} finally {
|
||||
restoreEnv({ HOME: previousHome, USERPROFILE: previousUserProfile })
|
||||
fs.rmSync(homeDir, { recursive: true, force: true })
|
||||
}
|
||||
}),
|
||||
],
|
||||
[
|
||||
"skill telemetry respects the minimal hook profile",
|
||||
async () => withTempProject([], async (projectDir) => {
|
||||
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), "ecc-opencode-skill-home-"))
|
||||
const previousHome = process.env.HOME
|
||||
const previousUserProfile = process.env.USERPROFILE
|
||||
const previousProfile = process.env.ECC_HOOK_PROFILE
|
||||
process.env.HOME = homeDir
|
||||
process.env.USERPROFILE = homeDir
|
||||
process.env.ECC_HOOK_PROFILE = "minimal"
|
||||
try {
|
||||
const client = createClient()
|
||||
const $ = createFailingShell()
|
||||
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||
|
||||
await hooks["tool.execute.after"]({ tool: "skill", args: { name: "code-review" } }, {})
|
||||
|
||||
assert.strictEqual(readRecordedRuns(homeDir).length, 0)
|
||||
} finally {
|
||||
restoreEnv({
|
||||
HOME: previousHome,
|
||||
USERPROFILE: previousUserProfile,
|
||||
ECC_HOOK_PROFILE: previousProfile,
|
||||
})
|
||||
fs.rmSync(homeDir, { recursive: true, force: true })
|
||||
}
|
||||
}),
|
||||
],
|
||||
[
|
||||
"shell.env exports CLAUDE_PLUGIN_ROOT so commands can resolve the ECC root",
|
||||
async () => withTempProject([], async (projectDir) => {
|
||||
const previousRoot = process.env.CLAUDE_PLUGIN_ROOT
|
||||
delete process.env.CLAUDE_PLUGIN_ROOT
|
||||
try {
|
||||
const client = createClient()
|
||||
const $ = createFailingShell()
|
||||
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||
|
||||
const output = { env: {} }
|
||||
await hooks["shell.env"]({ cwd: projectDir }, output)
|
||||
|
||||
assert.deepStrictEqual($.calls, [], `Unexpected shell probes: ${$.calls.join(", ")}`)
|
||||
assert.ok(output.env.CLAUDE_PLUGIN_ROOT, "Expected CLAUDE_PLUGIN_ROOT to be exported")
|
||||
|
||||
// The resolver consumes this variable verbatim, so it must be a
|
||||
// directory that actually carries the ECC script tree.
|
||||
const root = output.env.CLAUDE_PLUGIN_ROOT
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(root, "scripts", "lib", "resolve-ecc-root.js")),
|
||||
`Expected ${root} to contain scripts/lib/resolve-ecc-root.js`
|
||||
)
|
||||
} finally {
|
||||
restoreEnv({ CLAUDE_PLUGIN_ROOT: previousRoot })
|
||||
}
|
||||
}),
|
||||
],
|
||||
[
|
||||
"shell.env does not override a CLAUDE_PLUGIN_ROOT the user set",
|
||||
async () => withTempProject([], async (projectDir) => {
|
||||
const explicitRoot = path.join(projectDir, "explicit-root")
|
||||
const previousRoot = process.env.CLAUDE_PLUGIN_ROOT
|
||||
process.env.CLAUDE_PLUGIN_ROOT = explicitRoot
|
||||
try {
|
||||
const client = createClient()
|
||||
const $ = createFailingShell()
|
||||
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||
|
||||
const output = { env: {} }
|
||||
await hooks["shell.env"]({ cwd: projectDir }, output)
|
||||
|
||||
assert.strictEqual(
|
||||
output.env.CLAUDE_PLUGIN_ROOT,
|
||||
undefined,
|
||||
"the plugin must not inject its own root over one the user set"
|
||||
)
|
||||
assert.strictEqual(
|
||||
process.env.CLAUDE_PLUGIN_ROOT,
|
||||
explicitRoot,
|
||||
"the user's value must survive untouched"
|
||||
)
|
||||
} finally {
|
||||
restoreEnv({ CLAUDE_PLUGIN_ROOT: previousRoot })
|
||||
}
|
||||
}),
|
||||
],
|
||||
[
|
||||
"shell.env does not clobber a root another plugin already put in output.env",
|
||||
async () => withTempProject([], async (projectDir) => {
|
||||
const previousRoot = process.env.CLAUDE_PLUGIN_ROOT
|
||||
delete process.env.CLAUDE_PLUGIN_ROOT
|
||||
try {
|
||||
const client = createClient()
|
||||
const $ = createFailingShell()
|
||||
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||
|
||||
// An earlier plugin chose a root. The final merge below spreads our env
|
||||
// over output.env, so a guard that only checked process.env would
|
||||
// replace this value.
|
||||
const output = { env: { CLAUDE_PLUGIN_ROOT: path.join(projectDir, "other-plugin-root") } }
|
||||
await hooks["shell.env"]({ cwd: projectDir }, output)
|
||||
|
||||
assert.strictEqual(
|
||||
output.env.CLAUDE_PLUGIN_ROOT,
|
||||
path.join(projectDir, "other-plugin-root"),
|
||||
"an existing output.env root must be preserved"
|
||||
)
|
||||
} finally {
|
||||
restoreEnv({ CLAUDE_PLUGIN_ROOT: previousRoot })
|
||||
}
|
||||
}),
|
||||
],
|
||||
[
|
||||
"telemetry and root export work in a modular install without optional skills",
|
||||
async () => {
|
||||
// Regression: root detection probed skills/continuous-learning-v2, which
|
||||
// belongs to the optional workflow-quality module. An install with
|
||||
// platform-configs + hooks-runtime but without workflow-quality still
|
||||
// ships the plugin and the tracker, and it lost both fixes.
|
||||
const root = createModularInstallRoot()
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "ecc-opencode-modular-"))
|
||||
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), "ecc-opencode-skill-home-"))
|
||||
const previousHome = process.env.HOME
|
||||
const previousUserProfile = process.env.USERPROFILE
|
||||
const previousRoot = process.env.CLAUDE_PLUGIN_ROOT
|
||||
process.env.HOME = homeDir
|
||||
process.env.USERPROFILE = homeDir
|
||||
delete process.env.CLAUDE_PLUGIN_ROOT
|
||||
try {
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(root, "skills")),
|
||||
"the fixture must not contain skills/, or it would not test the modular case"
|
||||
)
|
||||
|
||||
const { ECCHooksPlugin: modularPlugin } = await loadPluginFrom(root)
|
||||
const client = createClient()
|
||||
const hooks = await modularPlugin({
|
||||
client,
|
||||
$: createFailingShell(),
|
||||
directory: projectDir,
|
||||
})
|
||||
|
||||
assert.ok(
|
||||
!client.logs.some(
|
||||
(entry) =>
|
||||
entry.level === "warn" && entry.message.includes("skill-run telemetry disabled")
|
||||
),
|
||||
"telemetry must stay enabled when the optional skills module is absent"
|
||||
)
|
||||
|
||||
await hooks["tool.execute.after"]({ tool: "skill", args: { name: "code-review" } }, {})
|
||||
assert.strictEqual(readRecordedRuns(homeDir).length, 1, "expected the run to be recorded")
|
||||
|
||||
const output = { env: {} }
|
||||
await hooks["shell.env"]({ cwd: projectDir }, output)
|
||||
assert.strictEqual(output.env.CLAUDE_PLUGIN_ROOT, root)
|
||||
assert.strictEqual(output.env.ECC_VERSION, "0.0.0-test")
|
||||
} finally {
|
||||
restoreEnv({
|
||||
HOME: previousHome,
|
||||
USERPROFILE: previousUserProfile,
|
||||
CLAUDE_PLUGIN_ROOT: previousRoot,
|
||||
})
|
||||
fs.rmSync(root, { recursive: true, force: true })
|
||||
fs.rmSync(projectDir, { recursive: true, force: true })
|
||||
fs.rmSync(homeDir, { recursive: true, force: true })
|
||||
}
|
||||
},
|
||||
],
|
||||
]
|
||||
|
||||
let passed = 0
|
||||
|
||||
Reference in new issue
Block a user