diff --git a/.opencode/README.md b/.opencode/README.md index e239c1361..2d76b7d92 100644 --- a/.opencode/README.md +++ b/.opencode/README.md @@ -241,3 +241,8 @@ selection, and its subagents inherit the invoking primary agent's model. ## License MIT + +Home-target installations keep `plugins/index.ts` inert even with hooks enabled. +OpenCode automatically loads `plugins/ecc-hooks.ts`; loading the barrel as well +would initialize every ECC hook twice. The source and published package barrel +remain available for module imports. diff --git a/.opencode/plugins/ecc-hooks.ts b/.opencode/plugins/ecc-hooks.ts index e350adad6..86eaa7cdf 100644 --- a/.opencode/plugins/ecc-hooks.ts +++ b/.opencode/plugins/ecc-hooks.ts @@ -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 — `/plugins` (installed), `/dist/plugins` + * (compiled) and `/.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 (`/plugins`), the compiled bundle (`/dist/plugins`) + * and a repo checkout (`/.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> @@ -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 }) => { const env: Record = { - 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 } }, diff --git a/docs/design/agent-proximity.md b/docs/design/agent-proximity.md index c9d3e2b96..6aed3faad 100644 --- a/docs/design/agent-proximity.md +++ b/docs/design/agent-proximity.md @@ -149,3 +149,8 @@ the renderer draws), and pairwise `links` with risk (the edges to color). escalation wired to live session diffs. - v2: cross-machine airspace over Tailscale (teammate agents enter the same space); the recorded "N agents, M humans, zero merge conflicts" demo. + +The control pane compares each worktree against the merge base of its declared base +and HEAD. Committed, staged, and unstaged tracked edits carry changed line ranges. +Untracked files that Git does not ignore participate as whole-file working sets. +Changes made only on the base branch after the worktree diverged do not participate. diff --git a/rules/common/agents.md b/rules/common/agents.md index 9113a3143..07397f5c1 100644 --- a/rules/common/agents.md +++ b/rules/common/agents.md @@ -2,8 +2,8 @@ ## Available Agents -ECC agents ship with the `ecc@ecc` plugin, not in `~/.claude/agents/`. -They are invoked through the Agent tool with a plugin-scoped `subagent_type`: +ECC agents come either from the `ecc@ecc` plugin (plugin-scoped names, `ecc:`) or from a manual install into `~/.claude/agents/` (bare names, ``). Use the name that matches the install you have; the manual installer strips the `ecc:` prefix from copied files. +They are invoked through the Agent tool with that `subagent_type`: ```text Agent(subagent_type: "ecc:planner", prompt: "...") diff --git a/scripts/codex-git-hooks/pre-commit b/scripts/codex-git-hooks/pre-commit index b4c608c76..e5bdab661 100644 --- a/scripts/codex-git-hooks/pre-commit +++ b/scripts/codex-git-hooks/pre-commit @@ -36,10 +36,21 @@ scan_added_lines() { return 0 fi - if hits="$(printf '%s\n' "$added_lines" | rg -n --pcre2 "$regex" 2>/dev/null)"; then + if ! command -v rg >/dev/null 2>&1; then + printf '[ECC pre-commit] ripgrep (rg) is required for secret scanning; install it and ensure it is on PATH when Git runs the hook.\n' >&2 + exit 1 + fi + + if hits="$(printf '%s\n' "$added_lines" | rg -n --pcre2 -- "$regex" 2>/dev/null)"; then printf '\n[ECC pre-commit] Potential secret detected (%s) in %s\n' "$name" "$file" >&2 printf '%s\n' "$hits" | head -n 3 >&2 has_findings=1 + else + local scan_status=$? + if [[ "$scan_status" -ne 1 ]]; then + printf '[ECC pre-commit] Secret scanner failed (rg exit %s) for %s; commit blocked because scanning could not complete.\n' "$scan_status" "$file" >&2 + exit 1 + fi fi } @@ -56,7 +67,7 @@ while IFS= read -r file; do scan_added_lines "$file" "GitHub classic token" 'ghp_[A-Za-z0-9]{36}' scan_added_lines "$file" "GitHub fine-grained token" 'github_pat_[A-Za-z0-9_]{20,}' scan_added_lines "$file" "AWS access key" 'AKIA[0-9A-Z]{16}' - scan_added_lines "$file" "private key block" '-----BEGIN (RSA|EC|OPENSSH|DSA|PRIVATE) KEY-----' + scan_added_lines "$file" "private key block" '-----BEGIN (RSA |EC |OPENSSH |DSA |ENCRYPTED )?PRIVATE KEY-----' scan_added_lines "$file" "generic credential assignment" "(?i)\\b(api[_-]?key|secret|password|token)\\b\\s*[:=]\\s*['\\\"][^'\\\"]{12,}['\\\"]" done <<< "$staged_files" diff --git a/scripts/hooks/lifecycle-hook-bootstrap.js b/scripts/hooks/lifecycle-hook-bootstrap.js index d0fab3c0a..629e031c9 100644 --- a/scripts/hooks/lifecycle-hook-bootstrap.js +++ b/scripts/hooks/lifecycle-hook-bootstrap.js @@ -80,6 +80,7 @@ async function main() { writeStderr(`[Hook] lifecycle stdin exceeded ${maxStdin} bytes; forwarded a bounded prefix`); } + const timeout = resolveTimeout(timeoutValue); const result = spawnSync( process.execPath, [runner, hookId, relScriptPath, profilesCsv || 'minimal,standard,strict'], @@ -92,10 +93,11 @@ async function main() { ECC_PLUGIN_ROOT: resolvedRoot, ECC_HOOK_INPUT_MAX_BYTES: String(maxStdin), ECC_HOOK_INPUT_TRUNCATED_UPSTREAM: truncated ? '1' : '0', - ECC_HOOK_CONTEXT_JSON: JSON.stringify(hookContext) + ECC_HOOK_CONTEXT_JSON: JSON.stringify(hookContext), + ECC_HOOK_DEADLINE_MS: String(Date.now() + timeout) }, cwd: process.cwd(), - timeout: resolveTimeout(timeoutValue), + timeout, maxBuffer: 16 * 1024 * 1024, windowsHide: true } diff --git a/scripts/hooks/mcp-health-check.js b/scripts/hooks/mcp-health-check.js index 843a6ec04..6003033c3 100644 --- a/scripts/hooks/mcp-health-check.js +++ b/scripts/hooks/mcp-health-check.js @@ -47,11 +47,29 @@ function envNumber(name, fallback) { return Number.isFinite(value) && value >= 0 ? value : fallback; } +// Resolve the Claude profile directory the same way everywhere. +// +// Duplicated rather than imported on purpose: hooks ship in managed installs +// that carry top-level `scripts/` without `scripts/lib/` (see #3259), so a +// require into lib/ would make the hook unloadable there. `~` is expanded the +// way `scripts/lib/agent-data-home.js` expands it, so a profile written as +// `~/.claude-work` resolves to one place rather than two. +function claudeConfigDir() { + const configured = process.env.CLAUDE_CONFIG_DIR; + const trimmed = configured ? String(configured).trim() : ''; + if (!trimmed) return path.join(os.homedir(), '.claude'); + if (trimmed.startsWith('~')) { + const remainder = trimmed.slice(1).replace(/^[/\\]+/, ''); + return remainder ? path.join(os.homedir(), remainder) : os.homedir(); + } + return path.resolve(trimmed); +} + function stateFilePath() { if (process.env.ECC_MCP_HEALTH_STATE_PATH) { return path.resolve(process.env.ECC_MCP_HEALTH_STATE_PATH); } - return path.join(os.homedir(), '.claude', 'mcp-health-cache.json'); + return path.join(claudeConfigDir(), 'mcp-health-cache.json'); } function configPaths() { diff --git a/scripts/hooks/observe-runner.js b/scripts/hooks/observe-runner.js index c046d75f0..5213eb1e2 100644 --- a/scripts/hooks/observe-runner.js +++ b/scripts/hooks/observe-runner.js @@ -4,6 +4,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); +const { resolveWindowsBashCandidates } = require('../lib/windows-bash'); const OBSERVE_RELATIVE_PATH = path.join('skills', 'continuous-learning-v2', 'hooks', 'observe.sh'); const DEFAULT_TIMEOUT_MS = 9000; @@ -51,7 +52,9 @@ function findShellBinary() { } if (process.platform === 'win32') { - candidates.push('bash.exe', 'bash', 'sh'); + // Absolute paths, skipping WSL launchers: WSL cannot open the /c/... path + // toShellPath() hands the shell, so observe.sh would fail on every call. + candidates.push(...resolveWindowsBashCandidates(['bash.exe', 'bash', 'sh'])); } else { candidates.push('bash', 'sh'); } diff --git a/scripts/hooks/plan-canvas-pending.js b/scripts/hooks/plan-canvas-pending.js index 9ba2a9207..b9b7dee4b 100644 --- a/scripts/hooks/plan-canvas-pending.js +++ b/scripts/hooks/plan-canvas-pending.js @@ -34,10 +34,28 @@ const path = require('path'); const SERVER_TIMEOUT_MS = 1000; const MAX_ITEMS_REPORTED = 20; +// Resolve the Claude profile directory the same way everywhere. +// +// Duplicated rather than imported on purpose: hooks ship in managed installs +// that carry top-level `scripts/` without `scripts/lib/` (see #3259), so a +// require into lib/ would make the hook unloadable there. `~` is expanded the +// way `scripts/lib/agent-data-home.js` expands it, so a profile written as +// `~/.claude-work` resolves to one place rather than two. +function claudeConfigDir() { + const configured = process.env.CLAUDE_CONFIG_DIR; + const trimmed = configured ? String(configured).trim() : ''; + if (!trimmed) return path.join(os.homedir(), '.claude'); + if (trimmed.startsWith('~')) { + const remainder = trimmed.slice(1).replace(/^[/\\]+/, ''); + return remainder ? path.join(os.homedir(), remainder) : os.homedir(); + } + return path.resolve(trimmed); +} + function stateDir() { const override = process.env.ECC_PLAN_CANVAS_STATE_DIR; if (override && override.trim()) return path.resolve(override.trim()); - return path.join(os.homedir(), '.claude', 'plan-canvas'); + return path.join(claudeConfigDir(), 'plan-canvas'); } function readState() { diff --git a/scripts/hooks/plan-canvas-sessions.js b/scripts/hooks/plan-canvas-sessions.js index 4e0799f87..b2e4a1574 100644 --- a/scripts/hooks/plan-canvas-sessions.js +++ b/scripts/hooks/plan-canvas-sessions.js @@ -19,10 +19,28 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +// Resolve the Claude profile directory the same way everywhere. +// +// Duplicated rather than imported on purpose: hooks ship in managed installs +// that carry top-level `scripts/` without `scripts/lib/` (see #3259), so a +// require into lib/ would make the hook unloadable there. `~` is expanded the +// way `scripts/lib/agent-data-home.js` expands it, so a profile written as +// `~/.claude-work` resolves to one place rather than two. +function claudeConfigDir() { + const configured = process.env.CLAUDE_CONFIG_DIR; + const trimmed = configured ? String(configured).trim() : ''; + if (!trimmed) return path.join(os.homedir(), '.claude'); + if (trimmed.startsWith('~')) { + const remainder = trimmed.slice(1).replace(/^[/\\]+/, ''); + return remainder ? path.join(os.homedir(), remainder) : os.homedir(); + } + return path.resolve(trimmed); +} + function stateDir() { const override = process.env.ECC_PLAN_CANVAS_STATE_DIR; if (override && override.trim()) return path.resolve(override.trim()); - return path.join(os.homedir(), '.claude', 'plan-canvas'); + return path.join(claudeConfigDir(), 'plan-canvas'); } function openSessions() { diff --git a/scripts/hooks/plugin-hook-bootstrap.js b/scripts/hooks/plugin-hook-bootstrap.js index ddcc14118..713f882b6 100644 --- a/scripts/hooks/plugin-hook-bootstrap.js +++ b/scripts/hooks/plugin-hook-bootstrap.js @@ -6,6 +6,7 @@ const { StringDecoder } = require('string_decoder'); const { spawnSync } = require('child_process'); const { ensureAgentDataHomeEnv } = require('../lib/agent-data-home'); const { normalizePluginRootForPlatform } = require('../lib/resolve-ecc-root'); +const { resolveWindowsBashCandidates } = require('../lib/windows-bash'); const { readStdinRaw: readBoundedStdin, resolveMaxStdin } = require('./hook-input'); const { createHookContextScanner } = require('./hook-input-limits'); @@ -111,7 +112,8 @@ function findShellBinary() { // bash.exe / conhost.exe processes the way MSYS2/Git Bash does. // Note: PowerShell is only suitable for .ps1 scripts; callers that need // to run .sh scripts (e.g. observe-runner.js) must not use this function. - candidates.push('pwsh.exe', 'powershell.exe', 'bash.exe', 'bash'); + // Bash is named by absolute path so a WSL launcher earlier on PATH is skipped. + candidates.push('pwsh.exe', 'powershell.exe', ...resolveWindowsBashCandidates(['bash.exe', 'bash'])); } else { candidates.push('bash', 'sh'); } @@ -145,7 +147,9 @@ function findBashBinary() { if (process.env.BASH && process.env.BASH.trim() && !isPowerShellBin(process.env.BASH.trim())) { candidates.push(process.env.BASH.trim()); } - candidates.push('bash.exe', 'bash'); + // On Windows these resolve to absolute paths that exclude WSL launchers; + // elsewhere they stay bare names. + candidates.push(...resolveWindowsBashCandidates(['bash.exe', 'bash'])); for (const candidate of candidates) { const probe = spawnSync(candidate, ['-c', ':'], { diff --git a/scripts/hooks/post-bash-command-log.js b/scripts/hooks/post-bash-command-log.js index bdea239df..c57579d49 100644 --- a/scripts/hooks/post-bash-command-log.js +++ b/scripts/hooks/post-bash-command-log.js @@ -38,6 +38,24 @@ function appendLine(filePath, line) { fs.appendFileSync(filePath, `${line}\n`, 'utf8'); } +// Resolve the Claude profile directory the same way everywhere. +// +// Duplicated rather than imported on purpose: hooks ship in managed installs +// that carry top-level `scripts/` without `scripts/lib/` (see #3259), so a +// require into lib/ would make the hook unloadable there. `~` is expanded the +// way `scripts/lib/agent-data-home.js` expands it, so a profile written as +// `~/.claude-work` resolves to one place rather than two. +function claudeConfigDir() { + const configured = process.env.CLAUDE_CONFIG_DIR; + const trimmed = configured ? String(configured).trim() : ''; + if (!trimmed) return path.join(os.homedir(), '.claude'); + if (trimmed.startsWith('~')) { + const remainder = trimmed.slice(1).replace(/^[/\\]+/, ''); + return remainder ? path.join(os.homedir(), remainder) : os.homedir(); + } + return path.resolve(trimmed); +} + function run(rawInput, mode = 'audit') { const config = MODE_CONFIG[mode]; @@ -45,7 +63,7 @@ function run(rawInput, mode = 'audit') { if (config) { const input = String(rawInput || '').trim() ? JSON.parse(String(rawInput)) : {}; const command = sanitizeCommand(input.tool_input?.command || '?'); - appendLine(path.join(os.homedir(), '.claude', config.fileName), config.format(command)); + appendLine(path.join(claudeConfigDir(), config.fileName), config.format(command)); } } catch { // Logging must never block the calling hook. diff --git a/scripts/hooks/post-edit-accumulator.js b/scripts/hooks/post-edit-accumulator.js index c626b2dd8..2bdafba9d 100644 --- a/scripts/hooks/post-edit-accumulator.js +++ b/scripts/hooks/post-edit-accumulator.js @@ -18,15 +18,16 @@ const crypto = require('crypto'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { resolveHookSessionId } = require('../lib/hook-session'); const MAX_STDIN = 1024 * 1024; -function getAccumFile() { +function getAccumFile(rawSessionId) { const raw = - process.env.CLAUDE_SESSION_ID || + rawSessionId || crypto.createHash('sha1').update(process.cwd()).digest('hex').slice(0, 12); // Strip path separators and traversal sequences so the value is safe to embed - // directly in a filename regardless of what CLAUDE_SESSION_ID contains. + // directly in a filename regardless of the hook session ID source. const sessionId = raw.replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 64); return path.join(os.tmpdir(), `ecc-edited-${sessionId}.txt`); } @@ -37,9 +38,9 @@ function getAccumFile() { */ const JS_TS_EXT = /\.(ts|tsx|js|jsx)$/; -function appendPath(filePath) { +function appendPath(filePath, sessionId) { if (filePath && JS_TS_EXT.test(filePath)) { - fs.appendFileSync(getAccumFile(), filePath + '\n', 'utf8'); + fs.appendFileSync(getAccumFile(sessionId), filePath + '\n', 'utf8'); } } @@ -50,12 +51,13 @@ function appendPath(filePath) { function run(rawInput) { try { const input = JSON.parse(rawInput); + const sessionId = resolveHookSessionId(rawInput); // Edit / Write: single file_path - appendPath(input.tool_input?.file_path); + appendPath(input.tool_input?.file_path, sessionId); // MultiEdit: array of edits, each with its own file_path const edits = input.tool_input?.edits; if (Array.isArray(edits)) { - for (const edit of edits) appendPath(edit?.file_path); + for (const edit of edits) appendPath(edit?.file_path, sessionId); } } catch { // Invalid input — pass through diff --git a/scripts/hooks/quality-gate.js b/scripts/hooks/quality-gate.js index 37373b87c..dd5f5a0fb 100755 --- a/scripts/hooks/quality-gate.js +++ b/scripts/hooks/quality-gate.js @@ -17,6 +17,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); +const { getLinterInvocation } = require('./pre-bash-commit-quality'); const { findProjectRoot, detectFormatter, resolveFormatterBin } = require('../lib/resolve-formatter'); @@ -31,10 +32,12 @@ const MAX_STDIN = 1024 * 1024; * @returns {import('child_process').SpawnSyncReturns} */ function exec(command, args, cwd = process.cwd()) { - return spawnSync(command, args, { + // Reuse the Windows shim runner so .cmd formatters receive literal paths. + const invocation = getLinterInvocation(command, args); + return spawnSync(invocation.command, invocation.args, { + ...invocation.options, cwd, - encoding: 'utf8', - env: process.env, + env: invocation.options.env || process.env, timeout: 15000 }); } diff --git a/scripts/hooks/run-with-flags.js b/scripts/hooks/run-with-flags.js index 19f0d1b9a..00c666308 100755 --- a/scripts/hooks/run-with-flags.js +++ b/scripts/hooks/run-with-flags.js @@ -299,6 +299,21 @@ async function main() { } // Legacy path: spawn a child Node process for hooks without run() export + // Direct runner invocations also need a deadline. Preserve an earlier + // bootstrap deadline instead of granting another full 30 seconds. + const inheritedDeadline = Number(process.env.ECC_HOOK_DEADLINE_MS); + const deadline = Math.min( + Date.now() + 30000, + Number.isSafeInteger(inheritedDeadline) && inheritedDeadline > 0 + ? inheritedDeadline + : Infinity + ); + const remaining = deadline - Date.now(); + if (remaining <= 0) { + writeStderr(`[Hook] lifecycle time budget exhausted for ${hookId}; skipping legacy hook`); + exitWithStdout('', 0); + return; + } const result = spawnSync(process.execPath, [scriptPath], { input: raw, encoding: 'utf8', @@ -308,10 +323,11 @@ async function main() { ECC_PLUGIN_ROOT: pluginRoot, ECC_HOOK_ID: hookId, ECC_HOOK_INPUT_TRUNCATED: truncated ? '1' : '0', - ECC_HOOK_INPUT_MAX_BYTES: String(MAX_STDIN) + ECC_HOOK_INPUT_MAX_BYTES: String(MAX_STDIN), + ECC_HOOK_DEADLINE_MS: String(deadline) }, cwd: process.cwd(), - timeout: 30000 + timeout: remaining }); const legacyStdout = sanitizeEcho(resolveLegacySpawnStdout(result)); diff --git a/scripts/hooks/session-end-marker.js b/scripts/hooks/session-end-marker.js index 7b27a8eb3..5a432ee4c 100755 --- a/scripts/hooks/session-end-marker.js +++ b/scripts/hooks/session-end-marker.js @@ -13,6 +13,7 @@ const { stopObserverForContext, resolveSessionId } = require('../lib/observer-sessions'); +const { resolveHookSessionId } = require('../lib/hook-session'); function log(message) { process.stderr.write(`[SessionEnd] ${message}\n`); @@ -20,10 +21,10 @@ function log(message) { function run(rawInput) { const output = rawInput || ''; - const sessionId = resolveSessionId(); + const sessionId = resolveSessionId(resolveHookSessionId(output)); if (!sessionId) { - log('No CLAUDE_SESSION_ID available; skipping observer cleanup'); + log('No session ID available; skipping observer cleanup'); return output; } diff --git a/scripts/hooks/session-end.js b/scripts/hooks/session-end.js index 5c31a8e0b..c7431eea7 100644 --- a/scripts/hooks/session-end.js +++ b/scripts/hooks/session-end.js @@ -257,10 +257,16 @@ async function main() { const isPeriodicTurn = summary.totalMessages > 0 && summary.totalMessages % safeInterval === 0; if (isContextLow || isPeriodicTurn) { log(`[SessionEnd] LLM summary triggered (context: ${contextPct ?? 'unknown'}%, messages: ${summary.totalMessages})`); - llmSummary = generateSessionSummary(transcriptPath); + let skipped = false; + llmSummary = generateSessionSummary(transcriptPath, { + onSkip: reason => { + skipped = true; + log(`[SessionEnd] LLM summary skipped (${reason}); falling back to mechanical extraction`); + } + }); if (llmSummary) { log('[SessionEnd] LLM summary generated successfully'); - } else { + } else if (!skipped) { log('[SessionEnd] LLM summary failed; falling back to mechanical extraction'); } } diff --git a/scripts/hooks/session-start.js b/scripts/hooks/session-start.js index a7462c551..528b6fcd8 100644 --- a/scripts/hooks/session-start.js +++ b/scripts/hooks/session-start.js @@ -23,6 +23,7 @@ const { log } = require('../lib/utils'); const { resolveProjectContext, writeSessionLease, resolveSessionId, getHomunculusDir } = require('../lib/observer-sessions'); +const { resolveHookSessionId } = require('../lib/hook-session'); const { getPackageManager, getSelectionPrompt } = require('../lib/package-manager'); const { listAliases } = require('../lib/session-aliases'); const { detectProjectType } = require('../lib/project-detect'); @@ -181,6 +182,10 @@ function getSessionStartMode(rawInput) { return SESSION_START_MODE_INVALID; } + if (!payload || typeof payload !== 'object' || Array.isArray(payload)) { + return SESSION_START_MODE_SKIP; + } + const supportedModes = new Set(['startup', 'resume', 'clear', 'compact']); const hookName = typeof payload.hookName === 'string' ? payload.hookName.trim() : ''; if (hookName.startsWith('SessionStart:')) { @@ -649,7 +654,8 @@ async function main() { const maxContextChars = getSessionStartMaxContextChars(); const explicitContextDisabled = isSessionStartContextDisabled(); const shouldInjectContext = !explicitContextDisabled && maxContextChars !== 0; - const sessionStartMode = getSessionStartMode(fs.readFileSync(0, 'utf8')); + const rawInput = fs.readFileSync(0, 'utf8'); + const sessionStartMode = getSessionStartMode(rawInput); // Ensure directories exist ensureDir(sessionsDir); @@ -665,7 +671,7 @@ async function main() { } } - const observerSessionId = resolveSessionId(); + const observerSessionId = resolveSessionId(resolveHookSessionId(rawInput)); if (observerSessionId) { writeSessionLease(observerContext, observerSessionId, { hook: 'SessionStart', @@ -673,7 +679,7 @@ async function main() { }); log(`[SessionStart] Registered observer lease for ${observerSessionId}`); } else { - log('[SessionStart] No CLAUDE_SESSION_ID available; skipping observer lease registration'); + log('[SessionStart] No session ID available; skipping observer lease registration'); } if (explicitContextDisabled) { diff --git a/scripts/hooks/skill-run-tracker.js b/scripts/hooks/skill-run-tracker.js index eef54b859..a306245d5 100644 --- a/scripts/hooks/skill-run-tracker.js +++ b/scripts/hooks/skill-run-tracker.js @@ -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 }; diff --git a/scripts/hooks/stop-format-typecheck.js b/scripts/hooks/stop-format-typecheck.js index d6e002cf4..25341ed44 100644 --- a/scripts/hooks/stop-format-typecheck.js +++ b/scripts/hooks/stop-format-typecheck.js @@ -23,6 +23,7 @@ const os = require('os'); const path = require('path'); const { findProjectRoot, detectFormatter, resolveFormatterBin } = require('../lib/resolve-formatter'); +const { resolveHookSessionId } = require('../lib/hook-session'); const MAX_STDIN = 1024 * 1024; // Total ms budget reserved for all batches (leaves headroom below the 300s Stop timeout) @@ -76,9 +77,9 @@ function isPluginClonePath(filePath, cwd = process.cwd(), homeDir = os.homedir() }); } -function getAccumFile() { +function getAccumFile(rawSessionId) { const raw = - process.env.CLAUDE_SESSION_ID || + rawSessionId || crypto.createHash('sha1').update(process.cwd()).digest('hex').slice(0, 12); const sessionId = raw.replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 64); return path.join(os.tmpdir(), `ecc-edited-${sessionId}.txt`); @@ -127,6 +128,14 @@ function findTsConfigDir(filePath) { return null; } +function diagnosticMatchesFile(line, paths) { + const normalizedLine = line.replace(/\\/g, '/'); + for (const candidate of paths) { + if (normalizedLine.includes(candidate.replace(/\\/g, '/'))) return true; + } + return false; +} + function typecheckBatch(tsConfigDir, editedFiles, timeoutMs) { const isWin = process.platform === 'win32'; const npxBin = isWin ? 'npx.cmd' : 'npx'; @@ -163,7 +172,7 @@ function typecheckBatch(tsConfigDir, editedFiles, timeoutMs) { const relPath = path.relative(tsConfigDir, filePath); const candidates = new Set([filePath, relPath]); const relevantLines = lines - .filter(line => { for (const c of candidates) { if (line.includes(c)) return true; } return false; }) + .filter(line => diagnosticMatchesFile(line, candidates)) .slice(0, 10); if (relevantLines.length > 0) { process.stderr.write(`[Hook] TypeScript errors in ${path.basename(filePath)}:\n`); @@ -172,8 +181,8 @@ function typecheckBatch(tsConfigDir, editedFiles, timeoutMs) { } } -function main() { - const accumFile = getAccumFile(); +function main(sessionId) { + const accumFile = getAccumFile(sessionId); let raw; try { @@ -229,7 +238,7 @@ function main() { */ function run(rawInput) { try { - main(); + main(resolveHookSessionId(rawInput)); } catch (err) { process.stderr.write(`[Hook] stop-format-typecheck error: ${err.message}\n`); } @@ -266,6 +275,7 @@ if (require.main === module) { } module.exports = { + diagnosticMatchesFile, run, parseAccumulator, isPluginClonePath, diff --git a/scripts/lib/agent-data-home.js b/scripts/lib/agent-data-home.js index 98cd0a84b..48d9b4294 100644 --- a/scripts/lib/agent-data-home.js +++ b/scripts/lib/agent-data-home.js @@ -17,6 +17,7 @@ const path = require('path'); const { assertWithinTrustedRoot } = require('./path-safety'); const AGENT_DATA_HOME_ENV = 'ECC_AGENT_DATA_HOME'; +const CLAUDE_CONFIG_DIR_ENV = 'CLAUDE_CONFIG_DIR'; const DEFAULT_CLAUDE_DIR_NAME = '.claude'; const DEFAULT_CURSOR_ECC_DIR_SEGMENTS = ['.cursor', 'ecc']; const PROJECT_CONFIG_RELATIVE = path.join('.cursor', 'ecc-agent-data.json'); @@ -92,7 +93,9 @@ function getDefaultCursorAgentDataHome() { } function getDefaultClaudeAgentDataHome() { - const fromClaudeConfigDir = expandHomePath(process.env.CLAUDE_CONFIG_DIR); + // Launcher-owned CLAUDE_CONFIG_DIR moves the Claude trusted root with its + // profile. Project-provided paths must still remain inside that root. + const fromClaudeConfigDir = expandHomePath(process.env[CLAUDE_CONFIG_DIR_ENV]); if (fromClaudeConfigDir) return fromClaudeConfigDir; return path.join(getHomeDirFromEnv(), DEFAULT_CLAUDE_DIR_NAME); } @@ -232,6 +235,7 @@ function getCursorSessionEnvPayload(options = {}) { module.exports = { AGENT_DATA_HOME_ENV, + CLAUDE_CONFIG_DIR_ENV, DEFAULT_CLAUDE_DIR_NAME, DEFAULT_CURSOR_ECC_DIR_SEGMENTS, PROJECT_CONFIG_RELATIVE, diff --git a/scripts/lib/control-pane/proximity.js b/scripts/lib/control-pane/proximity.js index a958ef36f..a4525f41e 100644 --- a/scripts/lib/control-pane/proximity.js +++ b/scripts/lib/control-pane/proximity.js @@ -15,6 +15,34 @@ const { execFileSync } = require('child_process'); const { scanAirspace, buildProximityTriggers } = require('../agent-proximity'); const { buildDependencyGraph } = require('../agent-proximity/graph'); +/** Decode Git C-style path escapes as bytes before UTF-8 decoding. */ +function decodeGitPath(header) { + const value = header.replace(/\t$/, ''); + if (!value.startsWith('"')) return value; + if (!value.endsWith('"')) return null; + const parts = []; + let literal = ''; + const escaped = { a: 7, b: 8, t: 9, n: 10, v: 11, f: 12, r: 13, '"': 34, '\\': 92 }; + for (let i = 1; i < value.length - 1; i += 1) { + const character = value[i]; + if (character !== '\\') { literal += character; continue; } + if (literal) { parts.push(Buffer.from(literal, 'utf8')); literal = ''; } + const next = value[++i]; + if (/[0-7]/.test(next || '')) { + let octal = next; + while (octal.length < 3 && /[0-7]/.test(value[i + 1] || '')) octal += value[++i]; + const byte = parseInt(octal, 8); + if (byte > 255) return null; + parts.push(Buffer.from([byte])); + } else { + if (!Object.prototype.hasOwnProperty.call(escaped, next)) return null; + parts.push(Buffer.from([escaped[next]])); + } + } + if (literal) parts.push(Buffer.from(literal, 'utf8')); + return Buffer.concat(parts).toString('utf8'); +} + /** * Parse `git diff --unified=0` output into per-file NEW-side line ranges. Hunk * headers look like `@@ -a,b +c,d @@`; we keep the +c,d (new) side so the overlap @@ -28,10 +56,13 @@ function parseDiffRanges(diff) { const byFile = new Map(); let current = null; for (const line of String(diff || '').split('\n')) { - const fileMatch = line.match(/^\+\+\+ b\/(.+)$/); - if (fileMatch) { - const name = fileMatch[1].trim(); - current = name === '/dev/null' ? null : name; + if (line.startsWith('diff --git ')) { + current = null; + continue; + } + if (line.startsWith('+++ ')) { + const gitPath = decodeGitPath(line.slice(4)); + current = gitPath && gitPath.startsWith('b/') ? gitPath.slice(2) : null; if (current && !byFile.has(current)) byFile.set(current, []); continue; } @@ -45,8 +76,8 @@ function parseDiffRanges(diff) { return byFile; } -function runGitDiff(worktreePath, base, extraArgs) { - return execFileSync('git', ['-C', worktreePath, 'diff', ...extraArgs, `${base}...HEAD`], { +function runGit(worktreePath, args) { + return execFileSync('git', ['-C', worktreePath, ...args], { encoding: 'utf8', timeout: 5000, maxBuffer: 8 * 1024 * 1024, @@ -54,6 +85,20 @@ function runGitDiff(worktreePath, base, extraArgs) { }); } +function runGitDiff(worktreePath, base, extraArgs) { + const mergeBase = runGit(worktreePath, ['merge-base', '--', base, 'HEAD']).trim(); + return runGit(worktreePath, ['-c', 'core.quotePath=false', 'diff', ...extraArgs, mergeBase, '--']); +} + +function untrackedFiles(worktreePath) { + try { + return runGit(worktreePath, ['ls-files', '--others', '--exclude-standard', '-z']).split('\0').filter(Boolean); + } catch { + // Keep successful tracked-file diffs when this optional probe is unavailable. + return []; + } +} + /** * Default working-set source: a session's worktree diff against its base, with * per-file changed line ranges. Returns [{ path, lines? }], or [] on failure so @@ -65,6 +110,15 @@ function defaultWorkingSetFor(session) { const base = wt.base || 'HEAD'; try { const ranges = parseDiffRanges(runGitDiff(wt.path, base, ['--unified=0'])); + // Deletions, binary changes and quoted paths have no usable new-side hunks. + try { + for (const file of runGitDiff(wt.path, base, ['--name-only', '-z']).split('\0').filter(Boolean)) { + if (!ranges.has(file)) ranges.set(file, []); + } + } catch { + // Preserve ranges already read if the supplementary filename probe fails. + } + for (const file of untrackedFiles(wt.path)) ranges.set(file, []); return [...ranges.entries()].map(([path, lines]) => (lines.length > 0 ? { path, lines } : { path })); } catch { return []; @@ -79,10 +133,7 @@ function defaultChangedFilesFor(session) { if (!wt || !wt.path) return []; const base = wt.base || 'HEAD'; try { - return runGitDiff(wt.path, base, ['--name-only']) - .split('\n') - .map(s => s.trim()) - .filter(Boolean); + return [...new Set([...runGitDiff(wt.path, base, ['--name-only', '-z']).split('\0').filter(Boolean), ...untrackedFiles(wt.path)])]; } catch { return []; } diff --git a/scripts/lib/hook-session.js b/scripts/lib/hook-session.js new file mode 100644 index 000000000..ad53d08f1 --- /dev/null +++ b/scripts/lib/hook-session.js @@ -0,0 +1,25 @@ +'use strict'; + +/** + * Resolve a raw hook session ID from stdin before legacy environment fallbacks. + * Callers must retain their own filename sanitization at the storage boundary. + * @param {string} rawInput - Hook stdin JSON. + * @param {object} env - Environment variables; defaults to the current process. + * @returns {string} A nonblank session ID, or an empty string when unavailable. + */ +function resolveHookSessionId(rawInput, env = process.env) { + let sessionId; + try { + const input = JSON.parse(rawInput); + if (input && typeof input === 'object' && !Array.isArray(input)) { + sessionId = input.session_id; + } + } catch { + // Empty or malformed hook input can still use an environment session ID. + } + + return [sessionId, env.ECC_SESSION_ID, env.CLAUDE_SESSION_ID] + .find(value => typeof value === 'string' && value.trim().length > 0) || ''; +} + +module.exports = { resolveHookSessionId }; diff --git a/scripts/lib/install/claude-settings-lock.js b/scripts/lib/install/claude-settings-lock.js index 164e887ec..d258d00cb 100644 --- a/scripts/lib/install/claude-settings-lock.js +++ b/scripts/lib/install/claude-settings-lock.js @@ -141,22 +141,45 @@ function recoverSettingsLock(lockPath, label = 'Claude settings') { } } -function acquireSettingsLock(settingsPath, { label = 'Claude settings' } = {}) { - const lockPath = `${settingsPath}.ecc.lock`; - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - try { - return createSettingsLock(lockPath, label); - } catch (error) { - if (!error || error.code !== 'EEXIST') { - throw error; - } +/** + * Acquire exclusive settings access, optionally waiting for another writer. + * A recovery guard (`.ecc.lock.recover`) is never removed by a contender: if its + * owner crashes during recovery, inspect and remove the orphaned guard manually + * only after confirming no ECC process is active. + * @param {string} settingsPath - Settings file protected by the lock + * @param {object} options - Diagnostic label and bounded wait duration + * @returns {Function} Release callback for the acquired lock + */ +function acquireSettingsLock(settingsPath, { label = 'Claude settings', timeoutMs = 0 } = {}) { + if (!Number.isFinite(timeoutMs) || timeoutMs < 0) { + throw new TypeError('Lock timeoutMs must be a finite non-negative number.'); + } + const lockPath = `${settingsPath}.ecc.lock`; + const deadline = performance.now() + timeoutMs; + const sleeper = new Int32Array(new SharedArrayBuffer(4)); + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + while (true) { + try { + return createSettingsLock(lockPath, label); + } catch (error) { + if (!error || error.code !== 'EEXIST') throw error; + } + try { + const recovered = recoverSettingsLock(lockPath, label); + if (recovered) return recovered; + } catch (error) { + // A different writer can acquire or release between inspection and recovery. + if (!error || (error.code !== 'EEXIST' && !(timeoutMs > 0 && error.code === 'ENOENT'))) throw error; + } + const remaining = deadline - performance.now(); + if (remaining <= 0) { + throw new Error( + `Another ECC process is updating ${label}: ${settingsPath}. ` + + `If no ECC process is active, inspect and remove ${lockPath}.` + ); + } + Atomics.wait(sleeper, 0, 0, Math.min(10, remaining)); } - const recovered = recoverSettingsLock(lockPath, label); - if (recovered) return recovered; - throw new Error( - `Another ECC process is updating ${label}: ${settingsPath}. ` - + `If no ECC process is active, inspect and remove ${lockPath}.` - ); } function runWithSettingsLock(settingsPath, callback) { diff --git a/scripts/lib/install/content-transform.js b/scripts/lib/install/content-transform.js index d27e54c02..a2c48a883 100644 --- a/scripts/lib/install/content-transform.js +++ b/scripts/lib/install/content-transform.js @@ -1,5 +1,6 @@ 'use strict'; +const { stripPluginNamespace } = require('./plugin-namespace'); const { adaptAntigravityAgent } = require('./antigravity-agent'); const { adaptCopilotAgent } = require('./copilot-agent'); const { disableOpenCodeHookPluginRegistration, getDisabledOpenCodePluginContent } = require('./hook-consent'); @@ -16,6 +17,9 @@ function transformInstallContent(operation, content) { if (!operation.contentTransform) { return content; } + if (operation.contentTransform === 'claude-manual-plugin-namespace') { + return stripPluginNamespace(content); + } if (operation.contentTransform === 'antigravity-agent-frontmatter') { return adaptAntigravityAgent(content, operation.sourceRelativePath); } diff --git a/scripts/lib/install/hook-consent.js b/scripts/lib/install/hook-consent.js index f954c9016..4876b681e 100644 --- a/scripts/lib/install/hook-consent.js +++ b/scripts/lib/install/hook-consent.js @@ -174,10 +174,6 @@ function withoutOpenCodeHookActivation(operation) { }; } -function transformOpenCodeHookActivationOperations(operations) { - return (Array.isArray(operations) ? operations : []).map(withoutOpenCodeHookActivation); -} - function planSelectsHookRuntime(plan = {}) { return ( Array.isArray(plan.selectedModuleIds) @@ -189,17 +185,26 @@ function planSelectsHookRuntime(plan = {}) { } function disableUnselectedOpenCodeHooks(plan) { - if (plan.target !== 'opencode' || planSelectsHookRuntime(plan)) { + if (plan.target !== 'opencode') { return plan; } + const runtimeSelected = planSelectsHookRuntime(plan); + const transformOperations = operations => (Array.isArray(operations) ? operations : []).map(operation => { + // The source barrel is needed by the published package, but OpenCode scans + // plugins/index.ts and plugins/ecc-hooks.ts independently in home installs. + if (!runtimeSelected || normalizeOperationPath(operation.sourceRelativePath) === '.opencode/plugins/index.ts') { + return withoutOpenCodeHookActivation(operation); + } + return operation; + }); return { ...plan, - operations: transformOpenCodeHookActivationOperations(plan.operations), + operations: transformOperations(plan.operations), statePreview: plan.statePreview ? { ...plan.statePreview, - operations: transformOpenCodeHookActivationOperations(plan.statePreview.operations), + operations: transformOperations(plan.statePreview.operations), } : plan.statePreview, }; diff --git a/scripts/lib/install/plan.js b/scripts/lib/install/plan.js index ca3ba47b6..d8a8e93ff 100644 --- a/scripts/lib/install/plan.js +++ b/scripts/lib/install/plan.js @@ -287,7 +287,17 @@ function createManifestInstallPlan(options = {}) { const adapter = getInstallTargetAdapter(target); const materializedOperations = plan.operations.flatMap(operation => ( materializeScaffoldOperation(sourceRoot, operation) - )); + )).map(operation => { + // Persist this projection so install, doctor and repair compare the same + // native component names rather than restoring plugin-only references. + if (['claude', 'claude-project'].includes(target) + && operation.kind === 'copy-file' + && /\.(md|mdx|markdown)$/i.test(operation.destinationPath) + && !operation.contentTransform) { + return { ...operation, contentTransform: 'claude-manual-plugin-namespace' }; + } + return operation; + }); const ruleLanguages = Array.isArray(options.ruleLanguages) ? [...options.ruleLanguages] : []; const operations = dedupeCopyFileOperations( options.legacyMode && target === 'antigravity' diff --git a/scripts/lib/install/plugin-namespace.js b/scripts/lib/install/plugin-namespace.js new file mode 100644 index 000000000..8a9022d6d --- /dev/null +++ b/scripts/lib/install/plugin-namespace.js @@ -0,0 +1,19 @@ +'use strict'; + +// The ecc@ecc plugin exposes agents, commands and skills under an `ecc:` +// namespace (`ecc:planner`, `/ecc:plan`). A manual install +// (install-apply --target claude|claude-project) places the same files at bare +// names, so plugin-prefixed references in copied markdown must drop the prefix +// or they point at agents and commands that do not exist in that layout. +// +// The leading guard keeps `ecc@ecc` and words that merely end in "ecc" (e.g. +// "decc:") untouched; the lookahead requires a real identifier after the colon. +const PLUGIN_NAMESPACE_PREFIX = /(^|[^A-Za-z0-9_@.-])ecc:(?=[a-z][a-z0-9-]*)/gm; + +function stripPluginNamespace(content) { + return String(content).replace(PLUGIN_NAMESPACE_PREFIX, '$1'); +} + +module.exports = { + stripPluginNamespace, +}; diff --git a/scripts/lib/llm-summary.js b/scripts/lib/llm-summary.js index b53fabd89..2473b3797 100644 --- a/scripts/lib/llm-summary.js +++ b/scripts/lib/llm-summary.js @@ -18,6 +18,8 @@ const fs = require('fs'); const MAX_TRANSCRIPT_CHARS = 7000; const MAX_TURNS = 25; const LLM_TIMEOUT_MS = 90000; +// Leave the Stop hook time to persist its mechanical fallback after a timeout. +const HOOK_FINISH_RESERVE_MS = 1000; function getLLMModel() { return process.env.ECC_LLM_SUMMARY_MODEL || 'haiku'; @@ -109,12 +111,29 @@ function getContextRemainingPct(transcriptPath) { * Generate a session summary using `claude -p`. * Returns the summary string, or null on failure or when recursion guard is active. */ -function generateSessionSummary(transcriptPath) { - if (process.env.ECC_SKIP_LLM_SUMMARY) return null; +function generateSessionSummary(transcriptPath, { onSkip = () => {} } = {}) { + if (process.env.ECC_SKIP_LLM_SUMMARY) { + onSkip('ECC_SKIP_LLM_SUMMARY'); + return null; + } const conversation = extractConversationText(transcriptPath); if (!conversation) return null; + let timeout = LLM_TIMEOUT_MS; + if (process.env.ECC_HOOK_DEADLINE_MS !== undefined) { + const deadline = Number(process.env.ECC_HOOK_DEADLINE_MS); + if (!Number.isSafeInteger(deadline) || deadline <= 0) { + onSkip('invalid lifecycle deadline'); + return null; + } + timeout = Math.min(timeout, deadline - Date.now() - HOOK_FINISH_RESERVE_MS); + if (timeout <= 0) { + onSkip('insufficient lifecycle time budget'); + return null; + } + } + const prompt = [ 'Below is a conversation log from a Claude Code coding session.', 'Create a summary to help the next session quickly understand the context.', @@ -159,7 +178,7 @@ function generateSessionSummary(transcriptPath) { ECC_SKIP_LLM_SUMMARY: '1', ECC_LLM_SUMMARY_SUBPROCESS: '1' }, - timeout: LLM_TIMEOUT_MS, + timeout, shell: process.platform === 'win32' }); diff --git a/scripts/lib/plan-canvas/sessions.js b/scripts/lib/plan-canvas/sessions.js index 799cf4b36..cd254600d 100644 --- a/scripts/lib/plan-canvas/sessions.js +++ b/scripts/lib/plan-canvas/sessions.js @@ -16,10 +16,26 @@ const path = require('path'); const FEEDBACK_KINDS = new Set(['chat', 'annotation', 'verdict']); const VERDICTS = new Set(['approve', 'request-changes']); +// The producer and the SessionStart/Stop hooks must land on one directory. The +// hooks resolve CLAUDE_CONFIG_DIR (they cannot require this file: managed +// installs ship scripts/ without scripts/lib/, see #3259), so writing to +// ~/.claude here regardless would hide queued feedback from them whenever a +// session runs under a second profile. +function resolveClaudeConfigDir(env = process.env) { + const configured = env.CLAUDE_CONFIG_DIR; + const trimmed = configured ? String(configured).trim() : ''; + if (!trimmed) return path.join(os.homedir(), '.claude'); + if (trimmed.startsWith('~')) { + const remainder = trimmed.slice(1).replace(/^[/\\]+/, ''); + return remainder ? path.join(os.homedir(), remainder) : os.homedir(); + } + return path.resolve(trimmed); +} + function resolveStateDir(env = process.env) { const override = env.ECC_PLAN_CANVAS_STATE_DIR; if (override && String(override).trim()) return path.resolve(String(override).trim()); - return path.join(os.homedir(), '.claude', 'plan-canvas'); + return path.join(resolveClaudeConfigDir(env), 'plan-canvas'); } // Canonicalize so `./plan.md`, symlinks, and absolute paths all land on the diff --git a/scripts/lib/session-aliases.d.ts b/scripts/lib/session-aliases.d.ts index 3930dc722..87d4da87d 100644 --- a/scripts/lib/session-aliases.d.ts +++ b/scripts/lib/session-aliases.d.ts @@ -84,8 +84,9 @@ export function getAliasesPath(): string; export function loadAliases(): AliasStore; /** - * Save aliases to disk with atomic write (temp file + rename). - * Creates backup before writing; restores on failure. + * Atomically replace the complete snapshot under the alias transaction lock. + * A failed publication leaves the previous snapshot intact. This does not merge + * snapshots loaded before locking; use the mutation functions for updates. */ export function saveAliases(aliases: AliasStore): boolean; diff --git a/scripts/lib/session-aliases.js b/scripts/lib/session-aliases.js index bfc846315..8abe38ab8 100644 --- a/scripts/lib/session-aliases.js +++ b/scripts/lib/session-aliases.js @@ -5,10 +5,11 @@ const fs = require('fs'); const path = require('path'); +const { writeFileAtomic } = require('./atomic-write'); +const { acquireSettingsLock } = require('./install/claude-settings-lock'); const { getClaudeDir, - ensureDir, readFile, log } = require('./utils'); @@ -80,74 +81,90 @@ function loadAliases() { } } +// Reject callback reentry instead of allowing a nested write to be overwritten +// by the outer transaction's snapshot. +const activeWrites = new Set(); + /** - * Save aliases to file with atomic write - * @param {object} aliases - Aliases object to save + * Run a synchronous alias transaction with exclusive access to its snapshot. + * Lock acquisition or reentry failure returns the caller's failure value. + * Release errors are logged without replacing the callback's result or error: + * a successfully published snapshot remains committed even if cleanup fails. + * @template T + * @param {function(): T} callback - Transaction to execute while holding the lock + * @param {T} failure - Result when the transaction cannot start + * @returns {T} The transaction result, or failure if it could not start + */ +function withAliasesLock(callback, failure) { + const aliasesPath = path.resolve(getAliasesPath()); + if (activeWrites.has(aliasesPath)) { + log('[Aliases] Cannot write aliases from inside an alias transaction'); + return failure; + } + + let release; + try { + release = acquireSettingsLock(aliasesPath, { label: 'session aliases', timeoutMs: 5000 }); + } catch (error) { + log(`[Aliases] Error locking aliases: ${error.message}`); + return failure; + } + + activeWrites.add(aliasesPath); + let result; + let primaryError; + let callbackFailed = false; + let releaseError; + try { + result = callback(); + } catch (error) { + callbackFailed = true; + primaryError = error; + } finally { + try { + release(); + } catch (error) { + releaseError = error; + } + activeWrites.delete(aliasesPath); + } + if (releaseError) { + log(`[Aliases] Error releasing aliases lock: ${releaseError.message}`); + } + if (callbackFailed) throw primaryError; + return result; +} + +/** + * Publish a complete alias snapshot while the caller holds the transaction lock. + * @param {object} aliases - Snapshot to publish without mutating the caller + * @returns {boolean} Whether atomic replacement succeeded + */ +function saveAliasesUnlocked(aliases) { + try { + const snapshot = { + ...aliases, + metadata: { + totalCount: Object.keys(aliases.aliases).length, + lastUpdated: new Date().toISOString() + } + }; + writeFileAtomic(getAliasesPath(), JSON.stringify(snapshot, null, 2)); + return true; + } catch (error) { + log(`[Aliases] Error saving aliases: ${error.message}`); + return false; + } +} + +/** + * Atomically replace the complete snapshot. This does not merge snapshots + * loaded by callers before acquiring the lock; use the mutators for updates. + * @param {object} aliases - Complete aliases snapshot * @returns {boolean} Success status */ function saveAliases(aliases) { - const aliasesPath = getAliasesPath(); - const tempPath = aliasesPath + '.tmp'; - const backupPath = aliasesPath + '.bak'; - - try { - // Update metadata - aliases.metadata = { - totalCount: Object.keys(aliases.aliases).length, - lastUpdated: new Date().toISOString() - }; - - const content = JSON.stringify(aliases, null, 2); - - // Ensure directory exists - ensureDir(path.dirname(aliasesPath)); - - // Create backup if file exists - if (fs.existsSync(aliasesPath)) { - fs.copyFileSync(aliasesPath, backupPath); - } - - // Atomic write: write to temp file, then rename - fs.writeFileSync(tempPath, content, 'utf8'); - - // On Windows, rename fails with EEXIST if destination exists, so delete first. - // On Unix/macOS, rename(2) atomically replaces the destination — skip the - // delete to avoid an unnecessary non-atomic window between unlink and rename. - if (process.platform === 'win32' && fs.existsSync(aliasesPath)) { - fs.unlinkSync(aliasesPath); - } - fs.renameSync(tempPath, aliasesPath); - - // Remove backup on success - if (fs.existsSync(backupPath)) { - fs.unlinkSync(backupPath); - } - - return true; - } catch (err) { - log(`[Aliases] Error saving aliases: ${err.message}`); - - // Restore from backup if exists - if (fs.existsSync(backupPath)) { - try { - fs.copyFileSync(backupPath, aliasesPath); - log('[Aliases] Restored from backup'); - } catch (restoreErr) { - log(`[Aliases] Failed to restore backup: ${restoreErr.message}`); - } - } - - // Clean up temp file (best-effort) - try { - if (fs.existsSync(tempPath)) { - fs.unlinkSync(tempPath); - } - } catch { - // Non-critical: temp file will be overwritten on next save - } - - return false; - } + return withAliasesLock(() => saveAliasesUnlocked(aliases), false); } /** @@ -185,7 +202,7 @@ function resolveAlias(alias) { * @param {string} title - Optional title for the alias * @returns {object} Result with success status and message */ -function setAlias(alias, sessionPath, title = null) { +function setAliasUnlocked(alias, sessionPath, title = null) { // Validate alias name if (!alias || alias.length === 0) { return { success: false, error: 'Alias name cannot be empty' }; @@ -221,7 +238,7 @@ function setAlias(alias, sessionPath, title = null) { title: title || null }; - if (saveAliases(data)) { + if (saveAliasesUnlocked(data)) { return { success: true, isNew, @@ -278,7 +295,7 @@ function listAliases(options = {}) { * @param {string} alias - Alias name to delete * @returns {object} Result with success status */ -function deleteAlias(alias) { +function deleteAliasUnlocked(alias) { const data = loadAliases(); if (!data.aliases[alias]) { @@ -288,7 +305,7 @@ function deleteAlias(alias) { const deleted = data.aliases[alias]; delete data.aliases[alias]; - if (saveAliases(data)) { + if (saveAliasesUnlocked(data)) { return { success: true, alias, @@ -305,7 +322,7 @@ function deleteAlias(alias) { * @param {string} newAlias - New alias name * @returns {object} Result with success status */ -function renameAlias(oldAlias, newAlias) { +function renameAliasUnlocked(oldAlias, newAlias) { const data = loadAliases(); if (!data.aliases[oldAlias]) { @@ -340,7 +357,7 @@ function renameAlias(oldAlias, newAlias) { aliasData.updatedAt = new Date().toISOString(); data.aliases[newAlias] = aliasData; - if (saveAliases(data)) { + if (saveAliasesUnlocked(data)) { return { success: true, oldAlias, @@ -349,12 +366,8 @@ function renameAlias(oldAlias, newAlias) { }; } - // Restore old alias and remove new alias on failure - data.aliases[oldAlias] = aliasData; - delete data.aliases[newAlias]; - // Attempt to persist the rollback - saveAliases(data); - return { success: false, error: 'Failed to save renamed alias — rolled back to original' }; + // Atomic replacement leaves the original snapshot intact on failure. + return { success: false, error: 'Failed to save renamed alias' }; } /** @@ -379,7 +392,7 @@ function resolveSessionAlias(aliasOrId) { * @param {string|null} title - New title (string or null to clear) * @returns {object} Result with success status */ -function updateAliasTitle(alias, title) { +function updateAliasTitleUnlocked(alias, title) { if (title !== null && typeof title !== 'string') { return { success: false, error: 'Title must be a string or null' }; } @@ -393,7 +406,7 @@ function updateAliasTitle(alias, title) { data.aliases[alias].title = title || null; data.aliases[alias].updatedAt = new Date().toISOString(); - if (saveAliases(data)) { + if (saveAliasesUnlocked(data)) { return { success: true, alias, @@ -431,7 +444,7 @@ function getAliasesForSession(sessionPath) { * @param {Function} sessionExists - Function to check if session exists * @returns {object} Cleanup result */ -function cleanupAliases(sessionExists) { +function cleanupAliasesUnlocked(sessionExists) { if (typeof sessionExists !== 'function') { return { totalChecked: 0, removed: 0, removedAliases: [], error: 'sessionExists must be a function' }; } @@ -446,7 +459,7 @@ function cleanupAliases(sessionExists) { } } - if (removed.length > 0 && !saveAliases(data)) { + if (removed.length > 0 && !saveAliasesUnlocked(data)) { log('[Aliases] Failed to save after cleanup'); return { success: false, @@ -465,6 +478,61 @@ function cleanupAliases(sessionExists) { }; } +/** + * Create or update an alias using the latest snapshot under the transaction lock. + * @param {string} alias - Alias name (alphanumeric, dash, underscore) + * @param {string} sessionPath - Session directory path + * @param {string|null} title - Optional title for the alias + * @returns {object} Result with success status and the saved alias details + */ +function setAlias(alias, sessionPath, title = null) { + return withAliasesLock(() => setAliasUnlocked(alias, sessionPath, title), + { success: false, error: 'Failed to save alias' }); +} + +/** + * Delete an alias without overwriting other writers' completed changes. + * @param {string} alias - Alias name to delete + * @returns {object} Result with success status and the deleted session path + */ +function deleteAlias(alias) { + return withAliasesLock(() => deleteAliasUnlocked(alias), + { success: false, error: 'Failed to delete alias' }); +} + +/** + * Rename an existing alias in one locked read-modify-write transaction. + * @param {string} oldAlias - Current alias name + * @param {string} newAlias - New unused alias name + * @returns {object} Result with success status and the renamed alias details + */ +function renameAlias(oldAlias, newAlias) { + return withAliasesLock(() => renameAliasUnlocked(oldAlias, newAlias), + { success: false, error: 'Failed to save renamed alias' }); +} + +/** + * Update an existing alias title using the latest locked snapshot. + * @param {string} alias - Alias name + * @param {string|null} title - New title, or null to clear it + * @returns {object} Result with success status and the updated title + */ +function updateAliasTitle(alias, title) { + return withAliasesLock(() => updateAliasTitleUnlocked(alias, title), + { success: false, error: 'Failed to update alias title' }); +} + +/** + * Remove aliases whose sessions no longer exist in a single locked transaction. + * The synchronous callback must not write aliases; its exceptions propagate. + * @param {function(string): boolean} sessionExists - Check whether a session exists + * @returns {object} Checked count, removed aliases, and any operation failure + */ +function cleanupAliases(sessionExists) { + return withAliasesLock(() => cleanupAliasesUnlocked(sessionExists), + { success: false, totalChecked: 0, removed: 0, removedAliases: [], error: 'Failed to save after cleanup' }); +} + module.exports = { getAliasesPath, loadAliases, diff --git a/scripts/lib/skill-evolution/dashboard.js b/scripts/lib/skill-evolution/dashboard.js index 7049707bd..edd98ce74 100644 --- a/scripts/lib/skill-evolution/dashboard.js +++ b/scripts/lib/skill-evolution/dashboard.js @@ -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; diff --git a/scripts/lib/skill-evolution/health.js b/scripts/lib/skill-evolution/health.js index 309f16094..c57e23354 100644 --- a/scripts/lib/skill-evolution/health.js +++ b/scripts/lib/skill-evolution/health.js @@ -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', '--------------------------------------------------------------------------', diff --git a/scripts/lib/windows-bash.js b/scripts/lib/windows-bash.js new file mode 100644 index 000000000..3c4e3ddd9 --- /dev/null +++ b/scripts/lib/windows-bash.js @@ -0,0 +1,87 @@ +'use strict'; + +/** + * Resolve bash on Windows without landing on a WSL launcher. + * + * %SystemRoot%\System32\bash.exe and the %LOCALAPPDATA%\Microsoft\WindowsApps + * alias start a WSL distro. With a distro installed they pass a `bash -c :` + * probe, then cannot open a hook script named by a Windows (C:\...) or Git + * Bash (/c/...) path, so the hook fails (exit 127) instead of being skipped. + * Spawning a bare `bash.exe` takes the first match on PATH, and System32 is + * normally ahead of Git's bin directory. + * + * Tests live at tests/lib/windows-bash.test.js. + */ + +const fs = require('fs'); +const path = require('path'); + +const WSL_LAUNCHER_SYSTEM_DIRS = ['System32', 'SysWOW64', 'Sysnative']; + +// Windows environment names are case-insensitive; a plain env object is not. +function readEnv(env, name) { + if (env[name] !== undefined) return env[name]; + const key = Object.keys(env).find(candidate => candidate.toUpperCase() === name.toUpperCase()); + return key === undefined ? undefined : env[key]; +} + +function normalizeDir(dir) { + return path.resolve(dir).replace(/[\\/]+$/, '').toLowerCase(); +} + +function wslLauncherDirs(env) { + const systemRoot = readEnv(env, 'SystemRoot') || readEnv(env, 'windir') || 'C:\\Windows'; + const dirs = WSL_LAUNCHER_SYSTEM_DIRS.map(name => path.join(systemRoot, name)); + const localAppData = readEnv(env, 'LOCALAPPDATA'); + if (localAppData) dirs.push(path.join(localAppData, 'Microsoft', 'WindowsApps')); + return new Set(dirs.map(normalizeDir)); +} + +/** + * True when filePath is a bash.exe that starts WSL rather than a native bash. + */ +function isWslBashLauncher(filePath, env = process.env) { + return path.basename(filePath).toLowerCase() === 'bash.exe' + && wslLauncherDirs(env).has(normalizeDir(path.dirname(filePath))); +} + +function isFile(filePath) { + try { + return fs.statSync(filePath).isFile(); + } catch { + return false; + } +} + +/** + * Absolute paths of `names` found on PATH, minus WSL bash launchers. + * + * Earlier names win over later ones, then PATH order decides, so `bash` is + * preferred to `sh` wherever each sits. A name without an extension matches + * .com/.exe, as a bare spawn would. Off Windows the names are returned as + * given, leaving lookup to the platform. + */ +function resolveWindowsBashCandidates(names, options = {}) { + const env = options.env || process.env; + const platform = options.platform || process.platform; + if (platform !== 'win32') return [...names]; + + const dirs = String(readEnv(env, 'PATH') || '').split(';').filter(Boolean); + const found = []; + const seen = new Set(); + for (const name of names) { + const fileNames = path.extname(name) ? [name] : [`${name}.com`, `${name}.exe`]; + for (const dir of dirs) { + for (const fileName of fileNames) { + const candidate = path.join(dir, fileName); + const key = candidate.toLowerCase(); + if (seen.has(key) || !isFile(candidate) || isWslBashLauncher(candidate, env)) continue; + seen.add(key); + found.push(candidate); + } + } + } + return found; +} + +module.exports = { isWslBashLauncher, resolveWindowsBashCandidates }; diff --git a/tests/hooks/claude-config-dir-routing.test.js b/tests/hooks/claude-config-dir-routing.test.js new file mode 100644 index 000000000..7f41435e7 --- /dev/null +++ b/tests/hooks/claude-config-dir-routing.test.js @@ -0,0 +1,282 @@ +/** + * Tests that the hooks route profile state through CLAUDE_CONFIG_DIR + * + * Run with: node tests/hooks/claude-config-dir-routing.test.js + * + * These spawn the hooks rather than calling the shared resolver. The four hooks + * below cannot require scripts/lib/agent-data-home.js: managed installs ship a + * top-level scripts/ without scripts/lib/ (#3259), so each carries its own copy + * of the resolver. A resolver test therefore cannot show where a hook writes. + */ + +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); +const { spawnSync } = require('child_process'); + +const hooksDir = path.join(__dirname, '..', '..', 'scripts', 'hooks'); +const libSessions = path.join(__dirname, '..', '..', 'scripts', 'lib', 'plan-canvas', 'sessions.js'); + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (err) { + console.log(` ✗ ${name}`); + console.log(` Error: ${err.message}`); + return false; + } +} + +function makeTempDir() { + return fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'config-dir-routing-'))); +} + +/** + * A profile that is not the home directory, so a hook that ignored + * CLAUDE_CONFIG_DIR would write somewhere this returns. + */ +function makeProfile() { + const root = makeTempDir(); + return { + home: path.join(root, 'home'), + profile: path.join(root, 'profile', '.claude-work'), + }; +} + +// No ECC_* override is passed anywhere here: an override would route the hook +// before it reached the CLAUDE_CONFIG_DIR fallback under test. +function runHook(hookFile, { profile, home, stdin = '{}', args = [], env = {} }) { + fs.mkdirSync(home, { recursive: true }); + return spawnSync('node', [path.join(hooksDir, hookFile), ...args], { + input: stdin, + encoding: 'utf8', + env: { + ...process.env, + HOME: home, + USERPROFILE: home, + CLAUDE_CONFIG_DIR: profile, + ECC_PLAN_CANVAS_STATE_DIR: undefined, + ECC_MCP_HEALTH_STATE_PATH: undefined, + ...env, + }, + }); +} + +/** + * plan-canvas-pending.js exports run() and is required by the lifecycle hook + * bootstrap rather than spawned, so it is driven here in a child process. The + * child is what makes CLAUDE_CONFIG_DIR meaningful: requiring it in-process, + * the way tests/hooks/plan-canvas-pending-hook.test.js does with + * ECC_PLAN_CANVAS_STATE_DIR, cannot reach the fallback under test. + */ +function runPendingHook({ profile, home, payload }) { + fs.mkdirSync(home, { recursive: true }); + const result = spawnSync('node', ['-e', ` + require(${JSON.stringify(path.join(hooksDir, 'plan-canvas-pending.js'))}) + .run(${JSON.stringify(JSON.stringify(payload))}) + .then(out => process.stdout.write(out.stdout)); + `], { + encoding: 'utf8', + env: { + ...process.env, + HOME: home, + USERPROFILE: home, + CLAUDE_CONFIG_DIR: profile, + ECC_PLAN_CANVAS_STATE_DIR: undefined, + }, + }); + assert.strictEqual(result.status, 0, `pending hook failed: ${result.stderr}`); + return result.stdout; +} + +function writeSessions(stateDir, sessions) { + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(path.join(stateDir, 'sessions.json'), JSON.stringify({ sessions }, null, 2)); +} + +function runTests() { + console.log('\nTesting CLAUDE_CONFIG_DIR routing in hooks\n'); + let passed = 0; + let failed = 0; + const record = ok => { if (ok) passed++; else failed++; }; + + console.log('post-bash-command-log.js'); + + record(test('writes the audit log under CLAUDE_CONFIG_DIR', () => { + const { home, profile } = makeProfile(); + runHook('post-bash-command-log.js', { + home, + profile, + args: ['audit'], + stdin: JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'echo routed' } }), + }); + const log = path.join(profile, 'bash-commands.log'); + assert.ok(fs.existsSync(log), `expected ${log} to exist`); + assert.match(fs.readFileSync(log, 'utf8'), /echo routed/); + assert.ok( + !fs.existsSync(path.join(home, '.claude', 'bash-commands.log')), + 'the log must not also land in ~/.claude' + ); + })); + + record(test('writes the cost log under CLAUDE_CONFIG_DIR', () => { + const { home, profile } = makeProfile(); + runHook('post-bash-command-log.js', { + home, + profile, + args: ['cost'], + stdin: JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'echo costed' } }), + }); + assert.match(fs.readFileSync(path.join(profile, 'cost-tracker.log'), 'utf8'), /echo costed/); + })); + + record(test('falls back to ~/.claude when CLAUDE_CONFIG_DIR is unset', () => { + const { home } = makeProfile(); + fs.mkdirSync(home, { recursive: true }); + spawnSync('node', [path.join(hooksDir, 'post-bash-command-log.js'), 'audit'], { + input: JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'echo default' } }), + encoding: 'utf8', + env: { ...process.env, HOME: home, USERPROFILE: home, CLAUDE_CONFIG_DIR: undefined }, + }); + assert.match( + fs.readFileSync(path.join(home, '.claude', 'bash-commands.log'), 'utf8'), + /echo default/ + ); + })); + + record(test('expands a tilde profile instead of creating a "~" directory', () => { + // path.resolve('~/.claude-work') would make ./~/.claude-work under the cwd, + // splitting the hook off from scripts/lib/agent-data-home.js, which expands. + const { home } = makeProfile(); + runHook('post-bash-command-log.js', { + home, + profile: '~/.claude-work', + args: ['audit'], + stdin: JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'echo tilde' } }), + }); + assert.match( + fs.readFileSync(path.join(home, '.claude-work', 'bash-commands.log'), 'utf8'), + /echo tilde/ + ); + })); + + console.log('\nmcp-health-check.js'); + + record(test('writes the health cache under CLAUDE_CONFIG_DIR', () => { + const { home, profile } = makeProfile(); + fs.mkdirSync(home, { recursive: true }); + // Without a configured server the hook skips the probe and never writes. + // The command does not exist, so the probe fails and the result is cached. + fs.writeFileSync(path.join(home, '.claude.json'), JSON.stringify({ + mcpServers: { routing_probe: { command: 'ecc-no-such-mcp-binary', args: [] } }, + })); + runHook('mcp-health-check.js', { + home, + profile, + stdin: JSON.stringify({ tool_name: 'mcp__routing_probe__ping', tool_input: {} }), + }); + const cache = path.join(profile, 'mcp-health-cache.json'); + assert.ok(fs.existsSync(cache), `expected ${cache} to exist`); + assert.ok( + !fs.existsSync(path.join(home, '.claude', 'mcp-health-cache.json')), + 'the cache must not also land in ~/.claude' + ); + })); + + console.log('\nplan-canvas-sessions.js (SessionStart)'); + + record(test('reports open sessions recorded under CLAUDE_CONFIG_DIR', () => { + const { home, profile } = makeProfile(); + writeSessions(path.join(profile, 'plan-canvas'), { + k1: { file: '/work/plan.md', status: 'open', pendingFeedback: [] }, + }); + const result = runHook('plan-canvas-sessions.js', { home, profile }); + assert.match(result.stdout, /\/work\/plan\.md/); + })); + + record(test('does not read ~/.claude when a profile is set', () => { + const { home, profile } = makeProfile(); + // Only the home copy holds a session. A hook that ignored the profile + // would announce it; the right answer here is silence. + writeSessions(path.join(home, '.claude', 'plan-canvas'), { + k1: { file: '/work/leaked.md', status: 'open', pendingFeedback: [] }, + }); + const result = runHook('plan-canvas-sessions.js', { home, profile }); + // Silence is the assertion here, and a hook that crashed before writing is + // also silent, so the exit status has to be checked first. + assert.strictEqual(result.status, 0, `SessionStart failed: ${result.stderr}`); + assert.strictEqual(result.stdout.trim(), ''); + })); + + console.log('\nplan-canvas-pending.js (Stop)'); + + record(test('blocks on feedback queued under CLAUDE_CONFIG_DIR', () => { + const { home, profile } = makeProfile(); + const cwd = makeTempDir(); + const artifact = path.join(cwd, 'plan.md'); + fs.writeFileSync(artifact, '# plan\n'); + writeSessions(path.join(profile, 'plan-canvas'), { + k1: { + key: 'k1', + file: artifact, + status: 'feedback', + pendingFeedback: [{ kind: 'verdict', verdict: 'request-changes', text: 'tighten step 2' }], + }, + }); + const decision = JSON.parse(runPendingHook({ home, profile, payload: { cwd } })); + assert.strictEqual(decision.decision, 'block'); + assert.match(decision.reason, /tighten step 2/); + })); + + console.log('\nproducer to consumer'); + + record(test('a session the library records is found by both hooks', () => { + // The regression this guards: the library wrote ~/.claude/plan-canvas while + // the hooks read CLAUDE_CONFIG_DIR/plan-canvas, so under a second profile + // SessionStart missed open reviews and Stop let a turn end with feedback + // still queued. Both ends are exercised with no state-directory override. + const { home, profile } = makeProfile(); + const cwd = makeTempDir(); + const artifact = path.join(cwd, 'design.md'); + fs.writeFileSync(artifact, '# design\n'); + fs.mkdirSync(home, { recursive: true }); + + const producer = spawnSync('node', ['-e', ` + const store = require(${JSON.stringify(libSessions)}).createSessionStore(); + const { session } = store.open(${JSON.stringify(artifact)}); + store.queueFeedback(session.key, [{ kind: 'verdict', verdict: 'request-changes', text: 'needs a rollback plan' }]); + `], { + encoding: 'utf8', + env: { + ...process.env, + HOME: home, + USERPROFILE: home, + CLAUDE_CONFIG_DIR: profile, + ECC_PLAN_CANVAS_STATE_DIR: undefined, + }, + }); + assert.strictEqual(producer.status, 0, `producer failed: ${producer.stderr}`); + assert.ok( + fs.existsSync(path.join(profile, 'plan-canvas', 'sessions.json')), + 'the library must record the session under the profile' + ); + + const sessionStart = runHook('plan-canvas-sessions.js', { home, profile }); + assert.match(sessionStart.stdout, /design\.md/, 'SessionStart must see the open review'); + + const decision = JSON.parse(runPendingHook({ home, profile, payload: { cwd } })); + assert.strictEqual(decision.decision, 'block'); + assert.match(decision.reason, /needs a rollback plan/); + })); + + console.log(`\n${'='.repeat(50)}`); + console.log(`Passed: ${passed}`); + console.log(`Failed: ${failed}`); + console.log('='.repeat(50)); + if (failed > 0) process.exit(1); +} + +runTests(); diff --git a/tests/hooks/observer-session-lifecycle.test.js b/tests/hooks/observer-session-lifecycle.test.js new file mode 100644 index 000000000..b9a0d1568 --- /dev/null +++ b/tests/hooks/observer-session-lifecycle.test.js @@ -0,0 +1,227 @@ +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const vm = require('vm'); +const { createRequire } = require('module'); +const { spawnSync } = require('child_process'); + +const hooksDir = path.resolve(__dirname, '../../scripts/hooks'); +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + passed += 1; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` ${error.message}`); + failed += 1; + } +} + +function withFixture(fn, extraEnv = {}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-observer-lifecycle-')); + const project = path.join(root, 'project'); + const store = path.join(root, 'homunculus'); + const observerDir = path.join(store, 'projects', 'fixture-project'); + fs.mkdirSync(project, { recursive: true }); + fs.mkdirSync(observerDir, { recursive: true }); + fs.writeFileSync(path.join(store, 'projects.json'), JSON.stringify({ + fixture: { id: 'fixture-project', root: project } + })); + const overrides = { + HOME: root, + USERPROFILE: root, + ECC_AGENT_DATA_HOME: path.join(root, 'agent-data'), + CLAUDE_CONFIG_DIR: path.join(root, 'claude-config'), + CLV2_HOMUNCULUS_DIR: store, + XDG_DATA_HOME: path.join(root, 'xdg'), + CLAUDE_PROJECT_DIR: project, + CLAUDE_PACKAGE_MANAGER: 'npm', + ECC_SESSION_START_CONTEXT: 'off', + ECC_SESSION_ID: undefined, + CLAUDE_SESSION_ID: undefined, + ...extraEnv + }; + for (const key of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_COMMON_DIR', 'GIT_PREFIX']) { + overrides[key] = undefined; + } + const env = { ...process.env, ...overrides }; + for (const [key, value] of Object.entries(env)) { + if (value === undefined) delete env[key]; + } + try { + fn({ project, observerDir, overrides, env }); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +function runHook(name, input, fixture) { + const result = spawnSync(process.execPath, [path.join(hooksDir, name)], { + cwd: fixture.project, + env: fixture.env, + input, + encoding: 'utf8', + timeout: 10000 + }); + assert.ifError(result.error); + assert.strictEqual(result.status, 0, result.stderr); + return result; +} + +function leaseFile(fixture, sessionId) { + return path.join(fixture.observerDir, '.observer-sessions', `${sessionId}.json`); +} + +function seedLease(fixture, sessionId) { + const filename = leaseFile(fixture, sessionId); + fs.mkdirSync(path.dirname(filename), { recursive: true }); + fs.writeFileSync(filename, JSON.stringify({ sessionId })); +} + +function assertStartOutput(result) { + assert.deepStrictEqual(JSON.parse(result.stdout), { + hookSpecificOutput: { hookEventName: 'SessionStart', additionalContext: '' } + }); + assert.ok(!result.stderr.includes('[SessionStart] Error:'), result.stderr); +} + +function withEnv(overrides, fn) { + const previous = Object.fromEntries(Object.keys(overrides).map(key => [key, process.env[key]])); + for (const [key, value] of Object.entries(overrides)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + try { + return fn(); + } finally { + for (const [key, value] of Object.entries(previous)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } +} + +function loadEndMarkerWithStubbedStop(stops) { + const filename = path.join(hooksDir, 'session-end-marker.js'); + const localRequire = createRequire(filename); + const module = { exports: {} }; + const requireStub = id => id === '../lib/observer-sessions' + ? { ...localRequire(id), stopObserverForContext: context => { stops.push(context.projectId); return true; } } + : localRequire(id); + vm.runInNewContext(fs.readFileSync(filename, 'utf8'), { + require: requireStub, module, + process: { stderr: { write() {} }, env: process.env } + }, { filename }); + return module.exports; +} + +console.log('\n=== Observer session lifecycle tests ===\n'); + +test('SessionStart creates separate observer leases from stdin without environment IDs', () => { + withFixture(fixture => { + for (const sessionId of ['payload-one', 'payload-two']) { + assertStartOutput(runHook('session-start.js', JSON.stringify({ + session_id: sessionId, hook_event_name: 'SessionStart', source: 'startup' + }), fixture)); + const lease = JSON.parse(fs.readFileSync(leaseFile(fixture, sessionId), 'utf8')); + assert.strictEqual(lease.sessionId, sessionId); + assert.strictEqual(lease.hook, 'SessionStart'); + assert.strictEqual(lease.projectRoot, fixture.project); + } + assert.strictEqual(fs.readdirSync(path.dirname(leaseFile(fixture, 'payload-one'))).length, 2); + }); +}); + +test('SessionEnd CLI removes the payload lease, retains others, then cleans up the final observer', () => { + withFixture(fixture => { + seedLease(fixture, 'first'); + seedLease(fixture, 'second'); + const pidFile = path.join(fixture.observerDir, '.observer.pid'); + // PID 1 is rejected before process.kill, so this fixture never signals any process. + fs.writeFileSync(pidFile, '1'); + for (const sessionId of ['first', 'second']) { + const input = ` {"session_id":"${sessionId}","hook_event_name":"SessionEnd"}\n`; + assert.strictEqual(runHook('session-end-marker.js', input, fixture).stdout, input); + assert.ok(!fs.existsSync(leaseFile(fixture, sessionId))); + assert.strictEqual(fs.existsSync(pidFile), sessionId === 'first'); + if (sessionId === 'first') assert.ok(fs.existsSync(leaseFile(fixture, 'second'))); + } + }); +}); + +test('in-process SessionEnd stops the observer only after the final payload lease ends', () => { + withFixture(fixture => withEnv(fixture.overrides, () => { + seedLease(fixture, 'first'); + seedLease(fixture, 'second'); + const stops = []; + const { run } = loadEndMarkerWithStubbedStop(stops); + const first = '{"session_id":"first"}\n'; + assert.strictEqual(run(first), first); + assert.ok(!fs.existsSync(leaseFile(fixture, 'first'))); + assert.ok(fs.existsSync(leaseFile(fixture, 'second'))); + assert.deepStrictEqual(stops, []); + const second = '{"session_id":"second"}\n'; + assert.strictEqual(run(second), second); + assert.ok(!fs.existsSync(leaseFile(fixture, 'second'))); + assert.deepStrictEqual(stops, ['fixture-project']); + })); +}); + +for (const envKey of ['ECC_SESSION_ID', 'CLAUDE_SESSION_ID']) { + test(`both lifecycle hooks retain ${envKey} fallback for malformed stdin`, () => { + withFixture(fixture => { + const input = '{malformed payload\n'; + assertStartOutput(runHook('session-start.js', input, fixture)); + assert.ok(fs.existsSync(leaseFile(fixture, 'env-session'))); + assert.strictEqual(runHook('session-end-marker.js', input, fixture).stdout, input); + assert.ok(!fs.existsSync(leaseFile(fixture, 'env-session'))); + }, { [envKey]: 'env-session' }); + }); +} + +test('payload ID overrides environment IDs in both lifecycle hooks', () => { + withFixture(fixture => { + const input = '{"session_id":"payload-session","hook_event_name":"SessionStart","source":"startup"}'; + assertStartOutput(runHook('session-start.js', input, fixture)); + assert.ok(fs.existsSync(leaseFile(fixture, 'payload-session'))); + assert.ok(!fs.existsSync(leaseFile(fixture, 'ecc-session'))); + assert.ok(!fs.existsSync(leaseFile(fixture, 'claude-session'))); + seedLease(fixture, 'ecc-session'); + assert.strictEqual(runHook('session-end-marker.js', input, fixture).stdout, input); + assert.ok(!fs.existsSync(leaseFile(fixture, 'payload-session'))); + assert.ok(fs.existsSync(leaseFile(fixture, 'ecc-session'))); + }, { ECC_SESSION_ID: 'ecc-session', CLAUDE_SESSION_ID: 'claude-session' }); +}); + +test('observer lease filenames sanitize path characters and Windows reserved IDs', () => { + withFixture(fixture => { + for (const sessionId of ['../session\\alpha:*?', 'CON']) { + const input = JSON.stringify({ session_id: sessionId, hook_event_name: 'SessionStart', source: 'startup' }); + assertStartOutput(runHook('session-start.js', input, fixture)); + const leaseDir = path.dirname(leaseFile(fixture, 'unused')); + const entries = fs.readdirSync(leaseDir); + assert.strictEqual(entries.length, 1); + assert.match(entries[0], sessionId === 'CON' ? /^CON-[a-f0-9]{6}\.json$/ : /^session-alpha\.json$/); + assert.strictEqual(runHook('session-end-marker.js', input, fixture).stdout, input); + assert.deepStrictEqual(fs.readdirSync(leaseDir), []); + } + }); +}); + +test('null JSON is harmless without session IDs and SessionEnd preserves the raw payload', () => { + withFixture(fixture => { + const input = 'null\n'; + assertStartOutput(runHook('session-start.js', input, fixture)); + assert.strictEqual(runHook('session-end-marker.js', input, fixture).stdout, input); + assert.ok(!fs.existsSync(path.dirname(leaseFile(fixture, 'unused')))); + }); +}); + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/hooks/plugin-hook-bootstrap.test.js b/tests/hooks/plugin-hook-bootstrap.test.js index e1ec6529d..3b3153efb 100644 --- a/tests/hooks/plugin-hook-bootstrap.test.js +++ b/tests/hooks/plugin-hook-bootstrap.test.js @@ -14,6 +14,7 @@ const SCRIPT = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'plugin-hook const LIFECYCLE_SCRIPT = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'lifecycle-hook-bootstrap.js'); const { normalizePluginRootForPlatform, withComparisonInput } = require(SCRIPT); const { resolveTimeout } = require(LIFECYCLE_SCRIPT); +const { resolveWindowsBashCandidates } = require('../../scripts/lib/windows-bash'); function createTempDir() { return fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-hook-bootstrap-')); @@ -464,6 +465,43 @@ process.exit(7); cleanup(root); } })) passed++; else failed++; + + const [nativeBash] = resolveWindowsBashCandidates(['bash.exe']); + if (nativeBash && process.env.SystemRoot) { + if (test('shell mode runs .sh with native bash when the System32 WSL launcher is earlier on PATH', () => { + const root = createTempDir(); + try { + writeFile(root, path.join('scripts', 'hook.sh'), [ + 'input=$(cat)', + 'printf "sh:%s:%s" "$1" "$input"', + '', + ].join('\n')); + + // A bare `bash.exe` resolves to System32\bash.exe here, which starts + // WSL and cannot open the Windows script path (exit 127). + const result = run(['shell', path.join('scripts', 'hook.sh'), 'arg'], { + root, + input: 'payload', + env: { + BASH: '', + PATH: [ + `${process.env.SystemRoot}\\System32\\WindowsPowerShell\\v1.0`, + `${process.env.SystemRoot}\\System32`, + path.dirname(nativeBash), + ].join(';'), + }, + }); + + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(result.stdout, 'sh:arg:payload'); + } finally { + cleanup(root); + } + })) passed++; else failed++; + } else { + skipped += 1; + console.log(' SKIP 1 Windows WSL-launcher test: no native bash.exe on PATH'); + } } } diff --git a/tests/hooks/quality-gate.test.js b/tests/hooks/quality-gate.test.js index cd3a05c4f..dfc75f1b4 100644 --- a/tests/hooks/quality-gate.test.js +++ b/tests/hooks/quality-gate.test.js @@ -8,8 +8,10 @@ const assert = require('assert'); const path = require('path'); const os = require('os'); const fs = require('fs'); +const { spawnSync } = require('child_process'); const qualityGate = require('../../scripts/hooks/quality-gate'); +const hookPath = path.resolve(__dirname, '../../scripts/hooks/quality-gate.js'); function test(name, fn) { try { @@ -155,5 +157,105 @@ if (test('returns original input for existing file with no formatter configured' } })) passed++; else failed++; +// Exercise real executable shims: .cmd on Windows, executable shell scripts elsewhere. + +function withFormatterFixture(formatter, fallback, fn) { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'quality-gate project '))); + try { + const srcDir = path.join(root, 'source files'); + const binDir = path.join(root, fallback ? 'runner bin' : 'node_modules/.bin'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.mkdirSync(binDir, { recursive: true }); + fs.writeFileSync(path.join(root, 'package.json'), '{}'); + fs.writeFileSync(path.join(root, formatter === 'biome' ? 'biome.json' : '.prettierrc'), '{}'); + const fileName = 'edited %ECC_TEST_LITERAL% !ECC_TEST_LITERAL! & file'; + const filePath = path.join(srcDir, fileName + (formatter === 'biome' ? '.json' : '.js')); + const original = formatter === 'biome' ? '{"value":1}\n' : 'const value=1;\n'; + fs.writeFileSync(filePath, original); + const capturePath = path.join(root, 'invocation.json'); + const runnerPath = path.join(root, 'fake formatter.js'); + fs.writeFileSync(runnerPath, [ + "const fs = require('fs');", + "const path = require('path');", + 'const args = process.argv.slice(2);', + 'fs.writeFileSync(process.env.ECC_TEST_CAPTURE, JSON.stringify({ args, cwd: process.cwd() }));', + "if (args.includes('--write')) fs.appendFileSync(args.find(arg => path.isAbsolute(arg)), 'formatted\\n');", + 'process.exit(Number(process.env.ECC_TEST_EXIT || 0));' + ].join('\n')); + const binName = fallback ? 'npx' : formatter; + const shimPath = path.join(binDir, binName + (process.platform === 'win32' ? '.cmd' : '')); + const quoteSh = value => "'" + value.replace(/'/g, "'\\''") + "'"; + const shim = process.platform === 'win32' + ? `@echo off\r\n"${process.execPath}" "${runnerPath}" %*\r\n` + : `#!/bin/sh\nexec ${quoteSh(process.execPath)} ${quoteSh(runnerPath)} "$@"\n`; + fs.writeFileSync(shimPath, shim, { mode: 0o755 }); + const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => key.toUpperCase() !== 'PATH')); + Object.assign(env, { + PATH: binDir, + CLAUDE_PACKAGE_MANAGER: 'npm', + ECC_QUALITY_GATE_FIX: 'false', + ECC_QUALITY_GATE_STRICT: 'true', + ECC_TEST_CAPTURE: capturePath, + ECC_TEST_LITERAL: 'must-not-expand', + ECC_TEST_EXIT: '0' + }); + const input = JSON.stringify({ tool_input: { file_path: filePath } }); + const runHook = overrides => spawnSync(process.execPath, [hookPath], { + cwd: srcDir, input, encoding: 'utf8', timeout: 20000, + env: { ...env, ...overrides } + }); + fn({ root, filePath, original, capturePath, input, runHook }); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +function assertInvocation(fixture, result, expectedArgs) { + assert.ifError(result.error); + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(result.stdout, fixture.input, 'Hook must preserve its input'); + assert.ok(fs.existsSync(fixture.capturePath), 'Formatter shim was not executed'); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(fixture.capturePath, 'utf8')), { + args: expectedArgs, cwd: fixture.root + }); +} + +console.log('\nReal formatter subprocesses:'); +for (const formatter of ['prettier', 'biome']) { + for (const fix of [false, true]) { + if (test(`runs local ${formatter} in ${fix ? 'fix' : 'check'} mode with literal spaces and shell metacharacters`, () => { + withFormatterFixture(formatter, false, fixture => { + const result = fixture.runHook({ ECC_QUALITY_GATE_FIX: String(fix) }); + const args = formatter === 'biome' + ? ['check', fixture.filePath, ...(fix ? ['--write'] : [])] + : [fix ? '--write' : '--check', fixture.filePath]; + assertInvocation(fixture, result, args); + assert.strictEqual(result.stderr, '', 'Successful formatter should not warn'); + assert.strictEqual(fs.readFileSync(fixture.filePath, 'utf8'), fixture.original + (fix ? 'formatted\n' : '')); + }); + })) passed++; else failed++; + } + if (test(`runs ${formatter} through the package runner when no local formatter exists`, () => { + withFormatterFixture(formatter, true, fixture => { + const args = formatter === 'biome' + ? ['@biomejs/biome', 'check', fixture.filePath] + : ['prettier', '--check', fixture.filePath]; + const result = fixture.runHook(); + assertInvocation(fixture, result, args); + assert.strictEqual(result.stderr, ''); + assert.strictEqual(fs.readFileSync(fixture.filePath, 'utf8'), fixture.original); + }); + })) passed++; else failed++; +} + +if (test('reports a real formatter failure in strict mode while preserving hook input', () => { + withFormatterFixture('prettier', false, fixture => { + const result = fixture.runHook({ ECC_TEST_EXIT: '2' }); + assertInvocation(fixture, result, ['--check', fixture.filePath]); + assert.ok(result.stderr.includes(`[QualityGate] Prettier check failed for ${fixture.filePath}`)); + assert.strictEqual(fs.readFileSync(fixture.filePath, 'utf8'), fixture.original); + }); +})) passed++; else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); diff --git a/tests/hooks/session-end.test.js b/tests/hooks/session-end.test.js index 05e74eeac..93692e38a 100644 --- a/tests/hooks/session-end.test.js +++ b/tests/hooks/session-end.test.js @@ -37,11 +37,19 @@ function countOccurrences(haystack, needle) { return n; } +function isolatedHomeEnv(home) { + return { + ...process.env, HOME: home, USERPROFILE: home, + ECC_AGENT_DATA_HOME: path.join(home, '.claude'), + CLAUDE_CONFIG_DIR: path.join(home, '.claude') + }; +} + function runHook(home, transcript, env = {}) { - return spawnSync('node', [script], { + return spawnSync(process.execPath, [script], { encoding: 'utf8', input: transcript ? JSON.stringify({ transcript_path: transcript }) : '', - env: { ...process.env, HOME: home, USERPROFILE: home, CLAUDE_SESSION_ID: '', ...env }, + env: { ...isolatedHomeEnv(home), CLAUDE_SESSION_ID: '', ...env }, timeout: 10000, }); } @@ -96,7 +104,7 @@ function runTests() { const res = spawnSync('node', [script], { encoding: 'utf8', input: JSON.stringify({ transcript_path: transcript }), - env: { ...process.env, HOME: home, USERPROFILE: home, CLAUDE_SESSION_ID: '' }, + env: { ...isolatedHomeEnv(home), CLAUDE_SESSION_ID: '' }, timeout: 10000, }); assert.strictEqual(res.status || 0, 0, `hook exited ${res.status}: ${res.stderr}`); @@ -225,7 +233,7 @@ function runTests() { const res = spawnSync('node', [script], { encoding: 'utf8', input: '{not-json', - env: { ...process.env, HOME: home, USERPROFILE: home, CLAUDE_SESSION_ID: 'fallback-session-12345678', CLAUDE_TRANSCRIPT_PATH: '' }, + env: { ...isolatedHomeEnv(home), CLAUDE_SESSION_ID: 'fallback-session-12345678', CLAUDE_TRANSCRIPT_PATH: '' }, timeout: 10000, }); assert.strictEqual(res.status || 0, 0, `hook exited ${res.status}: ${res.stderr}`); @@ -237,6 +245,114 @@ function runTests() { } }) ? passed++ : failed++); + (test('persists mechanical resume state when the LLM budget is exhausted', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-session-budget-')); + try { + const uuid = '12345678-1234-4234-8234-123456789abc'; + const transcript = path.join(home, `${uuid}.jsonl`); + fs.writeFileSync(transcript, JSON.stringify({ type: 'user', content: 'Keep this task resumable' }) + '\n'); + const bin = path.join(home, 'empty-bin'); + fs.mkdirSync(bin); + const res = runHook(home, transcript, { + ECC_LLM_SUMMARY_INTERVAL: '1', ECC_HOOK_DEADLINE_MS: '1', + ECC_SKIP_LLM_SUMMARY: '', ECC_LLM_SUMMARY_SUBPROCESS: '', PATH: bin + }); + assert.strictEqual(res.status, 0, res.stderr); + assert.match(res.stderr, /LLM summary skipped.*insufficient lifecycle time budget/); + assert.doesNotMatch(res.stderr, /LLM summary failed/); + assert.match(fs.readFileSync(sessionFileFor(home, uuid), 'utf8'), /Keep this task resumable/); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }) ? passed++ : failed++); + + (test('logs an explicit LLM disable as skipped and persists mechanical resume state', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-session-skip-')); + try { + const uuid = '12345678-1234-4234-8234-123456789abc'; + const transcript = path.join(home, `${uuid}.jsonl`); + const bin = path.join(home, 'empty-bin'); + fs.mkdirSync(bin); + fs.writeFileSync(transcript, JSON.stringify({ type: 'user', content: 'Save context with LLM disabled' }) + '\n'); + const res = runHook(home, transcript, { + ECC_LLM_SUMMARY_INTERVAL: '1', ECC_SKIP_LLM_SUMMARY: '1', + ECC_LLM_SUMMARY_SUBPROCESS: '', PATH: bin + }); + assert.strictEqual(res.status, 0, res.stderr); + assert.match(res.stderr, /LLM summary skipped.*ECC_SKIP_LLM_SUMMARY/); + assert.doesNotMatch(res.stderr, /LLM summary failed/); + assert.match(fs.readFileSync(sessionFileFor(home, uuid), 'utf8'), /Save context with LLM disabled/); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }) ? passed++ : failed++); + + for (const inherited of ['', String(Date.now() + 300000), '1']) { + (test(`direct legacy runner bounds the deadline ${inherited || '(unset)'}`, () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-runner-budget-')); + try { + const relative = 'legacy.js'; + fs.writeFileSync(path.join(home, relative), "process.stdout.write(JSON.stringify({deadline:Number(process.env.ECC_HOOK_DEADLINE_MS),now:Date.now()}));\n"); + const runner = path.resolve(__dirname, '../../scripts/hooks/run-with-flags.js'); + const res = spawnSync(process.execPath, [runner, 'session:stop:session-end', relative, 'standard'], { + input: '{}', encoding: 'utf8', timeout: 5000, + env: { + ...isolatedHomeEnv(home), CLAUDE_PLUGIN_ROOT: home, + ECC_HOOK_PROFILE: 'standard', ECC_DISABLED_HOOKS: '', ECC_DRY_RUN: '0', ECC_HOOKS_ENABLED: 'true', + ECC_HOOK_DEADLINE_MS: inherited + } + }); + assert.strictEqual(res.status, 0, res.stderr); + if (inherited === '1') { + assert.strictEqual(res.stdout, ''); + assert.match(res.stderr, /time budget exhausted/); + } else { + const result = JSON.parse(res.stdout); + assert.ok(result.deadline > result.now); + assert.ok(result.deadline - result.now <= 30000, 'Inherited far-future deadlines must not extend the runner budget'); + } + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }) ? passed++ : failed++); + } + + // A local stand-in exercises the actual bootstrap -> runner -> Stop hook -> + // child timeout chain without invoking Claude or using authentication. + if (process.platform !== 'win32') { + (test('times out a slow summarizer before lifecycle expiry and saves fallback', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-session-budget-')); + try { + const uuid = '12345678-1234-4234-8234-123456789abc'; + const transcript = path.join(home, `${uuid}.jsonl`); + const bin = path.join(home, 'bin'); + fs.mkdirSync(bin); + fs.writeFileSync(transcript, JSON.stringify({ type: 'user', content: 'Persist after a slow summary' }) + '\n'); + fs.writeFileSync(path.join(bin, 'claude'), `#!${process.execPath}\nrequire('fs').writeFileSync(${JSON.stringify(path.join(home, 'started'))}, 'started');\nsetTimeout(() => console.log('late summary'), 5000);\n`, { mode: 0o755 }); + const root = path.resolve(__dirname, '../..'); + const bootstrap = path.join(root, 'scripts/hooks/lifecycle-hook-bootstrap.js'); + const res = spawnSync(process.execPath, [bootstrap, 'session:stop:session-end', 'scripts/hooks/session-end.js', 'minimal,standard,strict', '2500'], { + encoding: 'utf8', input: JSON.stringify({ transcript_path: transcript }), + env: { + ...isolatedHomeEnv(home), PATH: bin, + CLAUDE_PLUGIN_ROOT: root, ECC_HOOK_PROFILE: 'standard', + ECC_LLM_SUMMARY_INTERVAL: '1', ECC_SKIP_LLM_SUMMARY: '', + ECC_LLM_SUMMARY_SUBPROCESS: '', ECC_HOOK_DEADLINE_MS: '1', + ECC_DISABLED_HOOKS: '', ECC_DRY_RUN: '0', ECC_HOOKS_ENABLED: 'true' + }, + timeout: 7000 + }); + assert.strictEqual(res.status, 0, res.stderr); + assert.ok(fs.existsSync(path.join(home, 'started')), 'Fresh bootstrap must replace an inherited expired deadline'); + assert.match(res.stderr, /LLM summary failed; falling back/); + assert.doesNotMatch(res.stderr, /lifecycle runner failed/); + assert.match(fs.readFileSync(sessionFileFor(home, uuid), 'utf8'), /Persist after a slow summary/); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }) ? passed++ : failed++); + } + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); } diff --git a/tests/hooks/skill-run-tracker.test.js b/tests/hooks/skill-run-tracker.test.js index d3e63e247..f836ab247 100644 --- a/tests/hooks/skill-run-tracker.test.js +++ b/tests/hooks/skill-run-tracker.test.js @@ -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 () { diff --git a/tests/hooks/stop-format-typecheck.test.js b/tests/hooks/stop-format-typecheck.test.js index 2b2504ecf..e0696de8f 100644 --- a/tests/hooks/stop-format-typecheck.test.js +++ b/tests/hooks/stop-format-typecheck.test.js @@ -17,9 +17,11 @@ const { getPerBatchBudgetMs, getTotalBudgetMs, isPluginClonePath, + diagnosticMatchesFile, parseAccumulator, } = require('../../scripts/hooks/stop-format-typecheck'); + function test(name, fn) { try { fn(); @@ -38,7 +40,9 @@ let failed = 0; // Use a unique session ID for tests so we don't pollute real sessions const TEST_SESSION_ID = `test-${Date.now()}`; const origSessionId = process.env.CLAUDE_SESSION_ID; +const origEccSessionId = process.env.ECC_SESSION_ID; process.env.CLAUDE_SESSION_ID = TEST_SESSION_ID; +delete process.env.ECC_SESSION_ID; function getAccumFile() { return path.join(os.tmpdir(), `ecc-edited-${TEST_SESSION_ID}.txt`); @@ -229,6 +233,13 @@ if (test('getPerBatchBudgetMs always returns a positive child timeout', () => { assert.strictEqual(getPerBatchBudgetMs(210000, 0), 60000); })) passed++; else failed++; +if (test('matches forward-slash TypeScript paths against Windows edited paths', () => { + const candidates = new Set(['C:\\project\\src\\broken.ts', 'src\\broken.ts']); + assert.strictEqual(diagnosticMatchesFile('src/broken.ts(1,14): error TS2322', candidates), true); + assert.strictEqual(diagnosticMatchesFile('src/other.ts(1,14): error TS2322', candidates), false); + +})) passed++; else failed++; + if (test('stop hook clears accumulator after processing duplicates', () => { cleanAccumFile(); fs.writeFileSync(getAccumFile(), '/nonexistent/x.ts\n/nonexistent/x.ts\n/nonexistent/y.ts\n', 'utf8'); @@ -259,6 +270,91 @@ if (test('stop hook passes stdin through unchanged', () => { assert.strictEqual(result.toString(), input); })) passed++; else failed++; +console.log('\nsession-scoped accumulator regression (#3460)'); + +function withPayloadSessions(fn) { + const sessionA = `${TEST_SESSION_ID}-payload-a`; + const sessionB = `${TEST_SESSION_ID}-payload-b`; + const fileA = path.join(os.tmpdir(), `ecc-edited-${sessionA}.txt`); + const fileB = path.join(os.tmpdir(), `ecc-edited-${sessionB}.txt`); + try { + cleanAccumFile(); + fn({ sessionA, sessionB, fileA, fileB }); + } finally { + for (const file of [fileA, fileB]) fs.rmSync(file, { force: true }); + cleanAccumFile(); + } +} + +if (test('stdin session IDs isolate edits from sibling sessions in the same cwd', () => { + withPayloadSessions(({ sessionA, sessionB, fileA, fileB }) => { + accumulator.run(JSON.stringify({ session_id: sessionA, tool_input: { file_path: '/nonexistent/a.ts' } })); + accumulator.run(JSON.stringify({ session_id: sessionB, tool_input: { file_path: '/nonexistent/b.ts' } })); + assert.strictEqual(fs.readFileSync(fileA, 'utf8'), '/nonexistent/a.ts\n'); + assert.strictEqual(fs.readFileSync(fileB, 'utf8'), '/nonexistent/b.ts\n'); + assert.ok(!fs.existsSync(getAccumFile()), 'stdin must override the environment session'); + }); +})) passed++; else failed++; + +if (test('Stop consumes only its stdin session accumulator and preserves pass-through', () => { + withPayloadSessions(({ sessionA, sessionB, fileA, fileB }) => { + fs.writeFileSync(fileA, '/nonexistent/a.ts\n'); + fs.writeFileSync(fileB, '/nonexistent/b.ts\n'); + const stopHook = require('../../scripts/hooks/stop-format-typecheck'); + const inputA = JSON.stringify({ session_id: sessionA, stop_reason: 'end_turn' }); + assert.strictEqual(stopHook.run(inputA), inputA); + assert.ok(!fs.existsSync(fileA), 'completed session accumulator must be removed'); + assert.strictEqual(fs.readFileSync(fileB, 'utf8'), '/nonexistent/b.ts\n'); + stopHook.run(JSON.stringify({ session_id: sessionB })); + assert.ok(!fs.existsSync(fileB), 'second session should consume its own accumulator'); + }); +})) passed++; else failed++; + +if (test('MultiEdit uses the same stdin session accumulator for every edit', () => { + withPayloadSessions(({ sessionA, fileA }) => { + accumulator.run(JSON.stringify({ + session_id: sessionA, + tool_input: { edits: [{ file_path: '/nonexistent/a.ts' }, { file_path: '/nonexistent/b.js' }] } + })); + assert.strictEqual(fs.readFileSync(fileA, 'utf8'), '/nonexistent/a.ts\n/nonexistent/b.js\n'); + }); +})) passed++; else failed++; + +if (test('ECC_SESSION_ID precedes CLAUDE_SESSION_ID while preserving legacy filenames', () => { + const rawId = `${TEST_SESSION_ID}/ecc.with spaces`; + const safeId = rawId.replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 64); + const file = path.join(os.tmpdir(), `ecc-edited-${safeId}.txt`); + process.env.ECC_SESSION_ID = rawId; + try { + cleanAccumFile(); + accumulator.run(JSON.stringify({ tool_input: { file_path: '/nonexistent/ecc.ts' } })); + assert.strictEqual(fs.readFileSync(file, 'utf8'), '/nonexistent/ecc.ts\n'); + assert.ok(!fs.existsSync(getAccumFile())); + const stopHook = require('../../scripts/hooks/stop-format-typecheck'); + assert.strictEqual(stopHook.run('malformed JSON'), 'malformed JSON'); + assert.ok(!fs.existsSync(file), 'malformed Stop input must retain the environment fallback'); + } finally { + delete process.env.ECC_SESSION_ID; + fs.rmSync(file, { force: true }); + cleanAccumFile(); + } +})) passed++; else failed++; + +if (test('stdin IDs keep the existing filename sanitizer and length limit', () => { + const rawId = `${TEST_SESSION_ID}/../nested\\session ${'x'.repeat(100)}`; + const safeId = rawId.replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 64); + const file = path.join(os.tmpdir(), `ecc-edited-${safeId}.txt`); + try { + accumulator.run(JSON.stringify({ session_id: rawId, tool_input: { file_path: '/nonexistent/safe.ts' } })); + assert.strictEqual(fs.readFileSync(file, 'utf8'), '/nonexistent/safe.ts\n'); + require('../../scripts/hooks/stop-format-typecheck').run(JSON.stringify({ session_id: rawId })); + assert.ok(!fs.existsSync(file)); + } finally { + fs.rmSync(file, { force: true }); + cleanAccumFile(); + } +})) passed++; else failed++; + // --- Plugin and marketplace clones are read, not owned: never format them --- const FAKE_HOME = path.join(path.sep, 'home', 'someone'); @@ -305,6 +401,11 @@ if (origSessionId === undefined) { } else { process.env.CLAUDE_SESSION_ID = origSessionId; } +if (origEccSessionId === undefined) { + delete process.env.ECC_SESSION_ID; +} else { + process.env.ECC_SESSION_ID = origEccSessionId; +} console.log(`\n=== Test Results ===`); console.log(`Passed: ${passed}`); diff --git a/tests/lib/agent-data-home.test.js b/tests/lib/agent-data-home.test.js index 0c507c1dd..f4d255d59 100644 --- a/tests/lib/agent-data-home.test.js +++ b/tests/lib/agent-data-home.test.js @@ -570,6 +570,107 @@ function runTests() { }); })) passed++; else failed++; + if (test('getDefaultClaudeAgentDataHome follows CLAUDE_CONFIG_DIR', () => { + // Claude Code points every profile-local path at CLAUDE_CONFIG_DIR. Ignoring + // it writes this profile's session data, logs and metrics into the default + // ~/.claude, where another profile reads them back. + withIsolatedCwd(() => { + withEnv({ + CLAUDE_CONFIG_DIR: path.join(os.tmpdir(), 'ecc-profile-x'), + ECC_AGENT_DATA_HOME: undefined, + }, () => { + const agentDataHome = require('../../scripts/lib/agent-data-home'); + assert.strictEqual( + agentDataHome.getDefaultClaudeAgentDataHome(), + path.join(os.tmpdir(), 'ecc-profile-x') + ); + }); + }); + })) passed++; else failed++; + + if (test('getDefaultClaudeAgentDataHome falls back to ~/.claude without the env', () => { + withIsolatedCwd(() => { + withEnv({ CLAUDE_CONFIG_DIR: undefined, ECC_AGENT_DATA_HOME: undefined }, () => { + const agentDataHome = require('../../scripts/lib/agent-data-home'); + assert.strictEqual( + agentDataHome.getDefaultClaudeAgentDataHome(), + path.join(os.homedir(), '.claude') + ); + }); + }); + })) passed++; else failed++; + + if (test('a blank CLAUDE_CONFIG_DIR is ignored rather than resolving to cwd', () => { + // An empty or whitespace value would otherwise resolve to the process cwd and + // scatter profile data through whatever directory the session started in. + withIsolatedCwd(() => { + withEnv({ CLAUDE_CONFIG_DIR: ' ', ECC_AGENT_DATA_HOME: undefined }, () => { + const agentDataHome = require('../../scripts/lib/agent-data-home'); + assert.strictEqual( + agentDataHome.getDefaultClaudeAgentDataHome(), + path.join(os.homedir(), '.claude') + ); + }); + }); + })) passed++; else failed++; + + if (test('security: CLAUDE_CONFIG_DIR adds a trusted profile root while preserving explicit sharing', () => { + // getDefaultClaudeAgentDataHome() is not only a default: it is one of the two + // roots resolveAllowedProjectConfigHome() will accept. Following + // CLAUDE_CONFIG_DIR moves that boundary with the profile, so the containment + // it enforces has to hold at the new location and nowhere else. + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-agent-data-home-cfgdir-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-agent-data-home-cfgdir-user-')); + const profileDir = path.join(homeDir, '.claude-profile-x'); + const configPath = path.join(projectDir, '.cursor', 'ecc-agent-data.json'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + + try { + withEnv({ + ECC_AGENT_DATA_HOME: undefined, + CLAUDE_CONFIG_DIR: profileDir, + HOME: homeDir, + USERPROFILE: undefined, + }, () => { + const agentDataHome = require('../../scripts/lib/agent-data-home'); + + // Inside the relocated root: accepted. + fs.writeFileSync( + configPath, + JSON.stringify({ agentDataHome: path.join(profileDir, 'nested') }), + 'utf8' + ); + // readProjectConfigAt canonicalizes, and macOS reaches os.tmpdir() + // through a symlink, so compare against the resolved home. + assert.strictEqual( + agentDataHome.readProjectConfigAt(configPath), + path.join(fs.realpathSync(homeDir), '.claude-profile-x', 'nested') + ); + + // Preserve the explicit ~/.claude sharing root shipped in #3479. + // The profile root adds a trusted location; arbitrary home paths and + // traversal out of that profile still stay rejected. + fs.writeFileSync(configPath, JSON.stringify({ agentDataHome: path.join(homeDir, '.claude') }), 'utf8'); + assert.strictEqual(agentDataHome.readProjectConfigAt(configPath), path.join(fs.realpathSync(homeDir), '.claude')); + const outside = [ + path.join(profileDir, '..', 'escaped'), + path.join(homeDir, 'arbitrary-agent-data'), + ]; + for (const candidate of outside) { + fs.writeFileSync(configPath, JSON.stringify({ agentDataHome: candidate }), 'utf8'); + const { result, messages } = captureConsoleErrors( + () => agentDataHome.readProjectConfigAt(configPath) + ); + assert.strictEqual(result, null, `expected rejection for ${candidate}`); + assert.ok(messages.some(message => message.includes('Ignoring unsafe agent data project config'))); + } + }); + } finally { + fs.rmSync(projectDir, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + console.log(`\n=== Test Results ===\nPassed: ${passed}\nFailed: ${failed}\n`); if (failed > 0) process.exit(1); } diff --git a/tests/lib/claude-plugin-setup.test.js b/tests/lib/claude-plugin-setup.test.js index cdda9f5f0..f3715d3c5 100644 --- a/tests/lib/claude-plugin-setup.test.js +++ b/tests/lib/claude-plugin-setup.test.js @@ -4,6 +4,7 @@ const assert = require('assert'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { withoutShadowingClaude } = require('./helpers/windows-test-env'); const repoRoot = path.join(__dirname, '..', '..'); const fakeClaudeScript = path.join(repoRoot, 'tests', 'fixtures', 'fake-claude-plugin.js'); @@ -87,7 +88,7 @@ function withFixture(initialState, fn) { process.chdir(fixture.projectRoot); process.env.HOME = fixture.homeDir; process.env.USERPROFILE = fixture.homeDir; - process.env.PATH = `${fixture.binDir}${path.delimiter}${previous.PATH || ''}`; + process.env.PATH = `${fixture.binDir}${path.delimiter}${withoutShadowingClaude(previous.PATH)}`; process.env.CLAUDE_CONFIG_DIR = fixture.configDir; process.env.ECC_TEST_CLAUDE_STATE = fixture.statePath; process.env.ECC_TEST_CLAUDE_CALLS = fixture.callsPath; diff --git a/tests/lib/claude-scope-migration.test.js b/tests/lib/claude-scope-migration.test.js index 91ce940ba..c3ab6606f 100644 --- a/tests/lib/claude-scope-migration.test.js +++ b/tests/lib/claude-scope-migration.test.js @@ -4,6 +4,7 @@ const assert = require('assert'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { withoutShadowingClaude } = require('./helpers/windows-test-env'); const repoRoot = path.join(__dirname, '..', '..'); const fakeClaudeScript = path.join(repoRoot, 'tests', 'fixtures', 'fake-claude-plugin.js'); @@ -100,7 +101,7 @@ function withFixture(initialState, fn) { process.chdir(fixture.projectRoot); process.env.HOME = fixture.homeDir; process.env.USERPROFILE = fixture.homeDir; - process.env.PATH = `${fixture.binDir}${path.delimiter}${previous.PATH || ''}`; + process.env.PATH = `${fixture.binDir}${path.delimiter}${withoutShadowingClaude(previous.PATH)}`; process.env.CLAUDE_CONFIG_DIR = fixture.configDir; process.env.ECC_TEST_CLAUDE_STATE = fixture.statePath; process.env.ECC_TEST_CLAUDE_CALLS = fixture.callsPath; diff --git a/tests/lib/claude-settings-lock.test.js b/tests/lib/claude-settings-lock.test.js new file mode 100644 index 000000000..905255804 --- /dev/null +++ b/tests/lib/claude-settings-lock.test.js @@ -0,0 +1,148 @@ +'use strict'; +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawn } = require('child_process'); +const { acquireSettingsLock } = require('../../scripts/lib/install/claude-settings-lock'); + +async function owner(root, settingsPath) { + const child = spawn(process.execPath, [ + path.join(__dirname, 'helpers', 'settings-lock-worker.js'), root, settingsPath, + ], { env: { ...process.env, HOME: root, USERPROFILE: root, ECC_AGENT_DATA_HOME: path.join(root, '.claude') }, + stdio: ['ignore', 'ignore', 'pipe'] }); + let stderr = ''; + child.stderr.on('data', data => { stderr += data; }); + child.done = new Promise((resolve, reject) => { + child.on('error', reject); + child.on('close', (code, signal) => resolve({ code, signal, stderr })); + }); + child.done.catch(() => {}); + const deadline = Date.now() + 10000; + try { + while (!fs.existsSync(path.join(root, 'held'))) { + if (child.exitCode !== null || Date.now() > deadline) throw new Error('Owner failed: ' + stderr); + await new Promise(resolve => setTimeout(resolve, 10)); + } + return child; + } catch (error) { + child.kill('SIGKILL'); + await child.done; + throw error; + } +} +async function withOwner(callback) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-settings-wait-')); + const settingsPath = path.join(root, '.claude', 'session-aliases.json'); + let child; + try { + child = await owner(root, settingsPath); + await callback(root, settingsPath, child); + } finally { + if (child) { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); + await child.done; + } + fs.rmSync(root, { recursive: true, force: true }); + } +} +const cases = [ + ['default contention is fail-fast; optional timeout is bounded and leaves owner intact', () => withOwner((_root, file) => { + const before = fs.readFileSync(file + '.ecc.lock', 'utf8'); + assert.throws(() => acquireSettingsLock(file), /Another ECC process/); + const start = performance.now(); + assert.throws(() => acquireSettingsLock(file, { timeoutMs: 50 }), /Another ECC process/); + assert.ok(performance.now() - start >= 40, 'Waits for the configured deadline'); + assert.ok(performance.now() - start < 5000, 'Does not wait indefinitely'); + assert.strictEqual(fs.readFileSync(file + '.ecc.lock', 'utf8'), before); + assert.deepStrictEqual(fs.readdirSync(path.dirname(file)), ['session-aliases.json.ecc.lock']); + })], + ['waiting succeeds when the real owner releases after the first contention', () => withOwner(async (root, file, child) => { + const originalLink = fs.linkSync; + let contended = false; + fs.linkSync = function (source, target) { + try { return originalLink.apply(this, arguments); } + catch (error) { + if (target === file + '.ecc.lock' && error.code === 'EEXIST') { + contended = true; + fs.writeFileSync(path.join(root, 'release'), ''); + } + throw error; + } + }; + let release; + try { release = acquireSettingsLock(file, { timeoutMs: 2000 }); } + finally { fs.linkSync = originalLink; } + assert.ok(contended); + release(); + const result = await child.done; + assert.strictEqual(result.code, 0, result.stderr); + assert.ok(!fs.existsSync(file + '.ecc.lock')); + })], + ['a killed owner is recovered by the next alias transaction', () => withOwner(async (root, file, child) => { + child.kill('SIGKILL'); + await child.done; + const saved = { HOME: process.env.HOME, USERPROFILE: process.env.USERPROFILE, ECC_AGENT_DATA_HOME: process.env.ECC_AGENT_DATA_HOME }; + Object.assign(process.env, { HOME: root, USERPROFILE: root, ECC_AGENT_DATA_HOME: path.dirname(file) }); + try { + const aliases = require('../../scripts/lib/session-aliases'); + assert.strictEqual(aliases.setAlias('recovered', '/session').success, true); + assert.ok(aliases.loadAliases().aliases.recovered); + assert.deepStrictEqual(fs.readdirSync(path.dirname(file)), ['session-aliases.json']); + } finally { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key]; else process.env[key] = value; + } + } + })], + ['non-contention errors propagate without retry', () => withOwner((_root, file) => { + const originalLink = fs.linkSync; + const expected = Object.assign(new Error('disk failure'), { code: 'EIO' }); + let attempts = 0; + fs.linkSync = () => { attempts++; throw expected; }; + try { + assert.throws(() => acquireSettingsLock(file, { timeoutMs: 2000 }), error => error === expected); + assert.strictEqual(attempts, 1); + } finally { fs.linkSync = originalLink; } + })], + ['recovery tolerates a lock disappearing between inspection and quarantine', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-lock-disappeared-')); + const file = path.join(root, 'settings.json'); + const lock = file + '.ecc.lock'; + fs.writeFileSync(lock, '{}'); + fs.utimesSync(lock, new Date(0), new Date(0)); + const originalRename = fs.renameSync; + let raced = false; + fs.renameSync = function (source, target) { + if (source === lock && target.includes('.stale-') && !raced) { + raced = true; + fs.unlinkSync(lock); + } + return originalRename.apply(this, arguments); + }; + try { + const release = acquireSettingsLock(file, { timeoutMs: 100 }); + assert.ok(raced); + release(); + assert.deepStrictEqual(fs.readdirSync(root), []); + } finally { + fs.renameSync = originalRename; + fs.rmSync(root, { recursive: true, force: true }); + } + }], + ['invalid timeout is rejected before touching disk', () => { + for (const timeoutMs of [-1, NaN, Infinity, '1']) { + assert.throws(() => acquireSettingsLock('unused', { timeoutMs }), /timeoutMs/); + } + }], +]; +(async () => { + let passed = 0; + let failed = 0; + for (const [name, run] of cases) { + try { await run(); passed++; console.log('PASS ' + name); } + catch (error) { failed++; console.error('FAIL ' + name + ': ' + error.stack); } + } + console.log('Results: Passed: ' + passed + ', Failed: ' + failed); + process.exitCode = failed ? 1 : 0; +})(); diff --git a/tests/lib/control-pane-proximity.test.js b/tests/lib/control-pane-proximity.test.js index c91603f46..1104214bc 100644 --- a/tests/lib/control-pane-proximity.test.js +++ b/tests/lib/control-pane-proximity.test.js @@ -54,6 +54,138 @@ const changedFilesFor = session => 'no-worktree': [] })[session.id] || []; +test('real worktrees expose committed, staged, unstaged and untracked work before commit', () => { + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + const { execFileSync } = require('child_process'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-proximity-worktree-')); + const repo = path.join(dir, 'repo'); + const git = (cwd, ...args) => execFileSync('git', ['-C', cwd, ...args], { stdio: 'pipe' }); + try { + fs.mkdirSync(repo); + git(repo, 'init', '-q', '-b', 'main'); + git(repo, 'config', 'user.name', 'proximity-test'); + git(repo, 'config', 'user.email', 'proximity-test@example.com'); + fs.writeFileSync(path.join(repo, 'shared.js'), 'const value = 1;\n'); + fs.writeFileSync(path.join(repo, '.gitignore'), 'ignored.js\n'); + git(repo, 'add', '.'); + git(repo, 'commit', '-qm', 'base'); + const worktrees = ['a', 'b'].map(name => { + const wt = path.join(dir, name); + git(repo, 'worktree', 'add', '-q', '-b', name, wt, 'main'); + fs.writeFileSync(path.join(wt, 'shared.js'), `const value = '${name}';\n`); + fs.writeFileSync(path.join(wt, 'new.js'), `export const name = '${name}';\n`); + fs.writeFileSync(path.join(wt, 'ignored.js'), 'ignored\n'); + return { id: name, worktree: { path: wt, base: 'main' } }; + }); + git(worktrees[0].worktree.path, 'add', 'shared.js'); + fs.writeFileSync(path.join(repo, 'upstream.js'), 'upstream only\n'); + git(repo, 'add', '.'); + git(repo, 'commit', '-qm', 'advance base'); + const assertWorkingSets = () => { + const agents = sessionsToAgents(worktrees); + assert.strictEqual(agents.length, 2); + for (const agent of agents) { + assert.deepStrictEqual(agent.files.map(f => f.path).sort(), ['new.js', 'shared.js']); + assert.deepStrictEqual(agent.files.find(f => f.path === 'shared.js').lines, [[1, 1]]); + assert.strictEqual(agent.files.find(f => f.path === 'new.js').lines, undefined); + } + const snapshot = buildProximitySnapshot(worktrees, { repoRoot: repo, graph: { adjacency: {} } }); + assert.strictEqual(snapshot.counts.agents, 2); + assert.ok(snapshot.advisories.some(a => a.level === 'resolution')); + }; + assertWorkingSets(); + git(worktrees[0].worktree.path, 'commit', '-qm', 'commit staged edit'); + assertWorkingSets(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('real Git working sets retain tracked deletions and binary changes', () => { + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + const { execFileSync } = require('child_process'); + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-proximity-delete-')); + const git = (...args) => execFileSync('git', ['-C', repo, ...args], { stdio: 'pipe' }); + try { + git('init', '-q', '-b', 'main'); + git('config', 'user.name', 'proximity-test'); + git('config', 'user.email', 'proximity-test@example.com'); + fs.writeFileSync(path.join(repo, 'deleted.js'), 'export const value = 1;\n'); + fs.writeFileSync(path.join(repo, 'binary.bin'), Buffer.from([0, 1, 2])); + git('add', '.'); + git('commit', '-qm', 'base'); + fs.unlinkSync(path.join(repo, 'deleted.js')); + fs.writeFileSync(path.join(repo, 'binary.bin'), Buffer.from([0, 3, 4])); + const [agent] = sessionsToAgents([{ id: 'delete', worktree: { path: repo, base: 'main' } }]); + assert.ok(agent, 'an agent with only non-text tracked changes still participates'); + assert.deepStrictEqual(agent.files.map(f => f.path).sort(), ['binary.bin', 'deleted.js']); + assert.ok(agent.files.every(f => f.lines === undefined)); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } +}); + +test('quoted staged paths and untracked paths share identity, and untracked failures keep tracked edits', () => { + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + const childProcess = require('child_process'); + const originalExec = childProcess.execFileSync; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-proximity-quoted-')); + const repo = path.join(dir, 'repo'); + const git = (cwd, ...args) => originalExec('git', ['-C', cwd, ...args], { stdio: 'pipe' }); + const modulePath = require.resolve('../../scripts/lib/control-pane/proximity'); + const cachedModule = require.cache[modulePath]; + try { + fs.mkdirSync(repo); + git(repo, 'init', '-q', '-b', 'main'); + git(repo, 'config', 'user.name', 'proximity-test'); + git(repo, 'config', 'user.email', 'proximity-test@example.com'); + fs.writeFileSync(path.join(repo, 'README'), 'base\n'); + git(repo, 'add', '.'); + git(repo, 'commit', '-qm', 'base'); + const file = process.platform === 'win32' ? 'café-résumé.js' : 'café"résumé.js'; + const worktrees = ['a', 'b'].map(name => { + const wt = path.join(dir, name); + git(repo, 'worktree', 'add', '-q', '-b', name, wt, 'main'); + fs.writeFileSync(path.join(wt, file), `export const value = '${name}';\n`); + if (name === 'a') fs.writeFileSync(path.join(wt, 'README'), 'changed\n'); + return { id: name, worktree: { path: wt, base: 'main' } }; + }); + git(worktrees[0].worktree.path, 'add', file); + const snapshot = buildProximitySnapshot(worktrees, { repoRoot: repo, graph: { adjacency: {} } }); + assert.strictEqual(snapshot.counts.agents, 2); + assert.ok(snapshot.advisories.some(advisory => advisory.level === 'resolution')); + assert.ok(snapshot.agents.every(agent => agent.files.includes(file))); + for (const code of ['EACCES', 'ETIMEDOUT', 'ENOBUFS']) { + childProcess.execFileSync = (command, args, options) => { + if (args.includes('ls-files')) throw Object.assign(new Error('untracked probe unavailable'), { code }); + return originalExec(command, args, options); + }; + delete require.cache[modulePath]; + const fresh = require(modulePath); + const [tracked] = fresh.sessionsToAgents([worktrees[0]]); + assert.ok(tracked, `${code}: tracked changes must survive`); + assert.deepStrictEqual(tracked.files.map(entry => entry.path).sort(), ['README', file].sort()); + childProcess.execFileSync = (command, args, options) => { + if (args.includes('--name-only')) throw Object.assign(new Error('tracked names unavailable'), { code }); + return originalExec(command, args, options); + }; + delete require.cache[modulePath]; + const partial = require(modulePath).sessionsToAgents([worktrees[0]]); + assert.deepStrictEqual(partial[0].files.map(entry => entry.path).sort(), ['README', file].sort()); + } + } finally { + childProcess.execFileSync = originalExec; + require.cache[modulePath] = cachedModule; + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + test('sessionsToAgents: only worktree sessions with edits participate', () => { const agents = sessionsToAgents(sessions, { changedFilesFor }); assert.deepStrictEqual(agents.map(a => a.agentId).sort(), ['docs-bot', 'lead-hermes', 'worker-kb']); @@ -97,6 +229,60 @@ test('parseDiffRanges: extracts new-side line ranges per file', () => { ]); }); +test('diff headers isolate quoted/deleted hunks and preserve trailing-space paths', () => { + const ranges = parseDiffRanges([ + 'diff --git a/first.js b/first.js', '+++ b/first.js', '@@ -1 +1 @@', + 'diff --git "a/café.js" "b/café.js"', '+++ "b/café.js"', '@@ -20 +20 @@', + 'diff --git a/deleted.js b/deleted.js', '+++ /dev/null', '@@ -30 +0,0 @@', + 'diff --git a/trailing b/trailing', '+++ b/trailing ', '@@ -40 +40 @@', + ].join('\n')); + assert.deepStrictEqual(ranges.get('first.js'), [[1, 1]]); + assert.deepStrictEqual(ranges.get('trailing '), [[40, 40]]); + assert.strictEqual(ranges.has('trailing'), false); +}); + +test('quoted Git headers decode UTF-8 octal bytes, quotes and control escapes into exact paths', () => { + const ranges = parseDiffRanges([ + 'diff --git "a/café.js" "b/café.js"', + '+++ "b/caf\\303\\251\\"r\\303\\251sum\\303\\251\\tname.js"', '@@ -4,2 +4,2 @@', + 'diff --git a/other.js b/other.js', '+++ b/other.js', '@@ -8 +8 @@', + ].join('\n')); + assert.deepStrictEqual(ranges.get('café"résumé\tname.js'), [[4, 5]]); + assert.deepStrictEqual(ranges.get('other.js'), [[8, 8]]); +}); + +test('real quoted filenames retain disjoint hunk ranges rather than whole-file collisions', () => { + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + const { execFileSync } = require('child_process'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-proximity-quoted-lines-')); + const repo = path.join(dir, 'repo'); + const git = (cwd, ...args) => execFileSync('git', ['-C', cwd, ...args], { stdio: 'pipe' }); + const file = process.platform === 'win32' ? 'café-résumé.js' : 'café"résumé\tname.js'; + try { + fs.mkdirSync(repo); + git(repo, 'init', '-q', '-b', 'main'); + git(repo, 'config', 'user.name', 'proximity-test'); + git(repo, 'config', 'user.email', 'proximity-test@example.com'); + const original = Array.from({ length: 40 }, (_, i) => `const line${i} = ${i};`); + fs.writeFileSync(path.join(repo, file), `${original.join('\n')}\n`); + git(repo, 'add', '.'); git(repo, 'commit', '-qm', 'base'); + const sessions = ['a', 'b'].map((name, index) => { + const wt = path.join(dir, name); + git(repo, 'worktree', 'add', '-q', '-b', name, wt, 'main'); + const lines = original.map((line, i) => i === index * 39 ? `${line} // edit` : line); + fs.writeFileSync(path.join(wt, file), `${lines.join('\n')}\n`); + return { id: name, worktree: { path: wt, base: 'main' } }; + }); + const agents = sessionsToAgents(sessions); + assert.deepStrictEqual(agents[0].files, [{ weight: 1, path: file, lines: [[1, 1]] }]); + assert.deepStrictEqual(agents[1].files, [{ weight: 1, path: file, lines: [[40, 40]] }]); + const snapshot = buildProximitySnapshot(sessions, { repoRoot: repo, graph: { adjacency: {} } }); + assert.ok(!snapshot.advisories.some(advisory => advisory.level === 'resolution')); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}); + test('line-range channel: same file but disjoint ranges ⇒ no resolution', () => { // Two agents in the same file, far-apart functions. workingSetFor provides ranges. const workingSetFor = s => diff --git a/tests/lib/helpers/session-alias-worker.js b/tests/lib/helpers/session-alias-worker.js new file mode 100644 index 000000000..ae9cdc7df --- /dev/null +++ b/tests/lib/helpers/session-alias-worker.js @@ -0,0 +1,39 @@ +'use strict'; +const fs = require('fs'); +const path = require('path'); +const [root, role, action] = process.argv.slice(2); +const aliasesPath = path.join(root, '.claude', 'session-aliases.json'); +const pause = new Int32Array(new SharedArrayBuffer(4)); +const mark = name => fs.writeFileSync(path.join(root, name), ''); +const readFileSync = fs.readFileSync; +const linkSync = fs.linkSync; +let paused = false; +// Announce a lock attempt OR a captured read; the parent needs no scheduling guess. +fs.linkSync = function (source, target) { + if (target === aliasesPath + '.ecc.lock') mark(role + '.attempt'); + return linkSync.apply(this, arguments); +}; +fs.readFileSync = function (file) { + const result = readFileSync.apply(this, arguments); + if (file === aliasesPath && !paused) { + paused = true; + mark(role + '.attempt'); + mark(role + '.read'); + const deadline = Date.now() + 15000; + while (!fs.existsSync(path.join(root, role + '.release'))) { + if (Date.now() > deadline) throw new Error('Read barrier timed out'); + Atomics.wait(pause, 0, 0, 10); + } + } + return result; +}; +const aliases = require('../../../scripts/lib/session-aliases'); +const actions = { + set: () => aliases.setAlias('beta', '/beta'), + delete: () => aliases.deleteAlias('existing'), + rename: () => aliases.renameAlias('existing', 'renamed'), + title: () => aliases.updateAliasTitle('existing', 'Updated'), + cleanup: () => aliases.cleanupAliases(session => session !== '/existing'), +}; +const result = role === 'alpha' ? aliases.setAlias('alpha', '/alpha') : actions[action](); +process.stdout.write(JSON.stringify(result)); diff --git a/tests/lib/helpers/settings-lock-worker.js b/tests/lib/helpers/settings-lock-worker.js new file mode 100644 index 000000000..75aad5375 --- /dev/null +++ b/tests/lib/helpers/settings-lock-worker.js @@ -0,0 +1,14 @@ +'use strict'; +const fs = require('fs'); +const path = require('path'); +const { acquireSettingsLock } = require('../../../scripts/lib/install/claude-settings-lock'); +const [root, settingsPath] = process.argv.slice(2); +const release = acquireSettingsLock(settingsPath); +fs.writeFileSync(path.join(root, 'held'), ''); +const deadline = Date.now() + 15000; +const sleeper = new Int32Array(new SharedArrayBuffer(4)); +while (!fs.existsSync(path.join(root, 'release'))) { + if (Date.now() > deadline) throw new Error('Owner release barrier timed out'); + Atomics.wait(sleeper, 0, 0, 10); +} +release(); diff --git a/tests/lib/helpers/windows-test-env.js b/tests/lib/helpers/windows-test-env.js new file mode 100644 index 000000000..9ccfeefa6 --- /dev/null +++ b/tests/lib/helpers/windows-test-env.js @@ -0,0 +1,38 @@ +/** + * Keep tests independent of tools installed on a Windows developer machine. + * + * Linux CI never sees these leaks, so they surface only as local failures. + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +// On Windows, spawning a bare `claude` resolves only .com/.exe on PATH, never +// .cmd. A natively installed claude.exe therefore wins over a test's fake +// claude.cmd even when the fake's directory comes first on PATH. +const SHADOWING_CLAUDE_BINARIES = ['claude.com', 'claude.exe']; + +/** + * Drop PATH entries holding a real Claude binary that would shadow a fake + * `claude.cmd` launcher. A no-op off Windows, where PATH order is honoured. + */ +function withoutShadowingClaude(pathValue, platform = process.platform) { + if (platform !== 'win32') return pathValue || ''; + return String(pathValue || '') + .split(path.win32.delimiter) + .filter(dir => dir && !SHADOWING_CLAUDE_BINARIES.some(name => fs.existsSync(path.join(dir, name)))) + .join(path.win32.delimiter); +} + +/** + * Name an archive relative to the directory tar runs in, with forward slashes. + * GNU tar, which Git for Windows puts first on PATH, reads an archive argument + * containing a colon (C:\...) as a host:path remote spec; bsdtar does not. + */ +function toRelativeTarPath(cwd, archivePath) { + return path.relative(cwd, archivePath).split(path.sep).join('/'); +} + +module.exports = { withoutShadowingClaude, toRelativeTarPath }; diff --git a/tests/lib/hook-consent.test.js b/tests/lib/hook-consent.test.js index d802c5507..9fbade56a 100644 --- a/tests/lib/hook-consent.test.js +++ b/tests/lib/hook-consent.test.js @@ -148,7 +148,9 @@ function runTests() { const pending = withHookConsent(selected, null); assert.throws(() => assertHookConsentReady(pending), /automatic hook runtime/); const enabled = withHookConsent(selected, 'enabled'); - assert.ok(enabled.operations.every(operation => operation.contentTransform === undefined)); + for (const operation of [...enabled.operations, ...enabled.statePreview.operations]) { + assert.strictEqual(operation.contentTransform, operation.sourceRelativePath.endsWith('/index.ts') ? 'opencode-disable-plugin-entrypoint' : undefined); + } assert.doesNotThrow(() => assertHookConsentReady(enabled)); const declined = withHookConsent(selected, 'declined'); assert.strictEqual(declined.operations.length, 2); diff --git a/tests/lib/hook-session.test.js b/tests/lib/hook-session.test.js new file mode 100644 index 000000000..13bc66a35 --- /dev/null +++ b/tests/lib/hook-session.test.js @@ -0,0 +1,85 @@ +const assert = require('assert'); +const { resolveHookSessionId } = require('../../scripts/lib/hook-session'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + passed += 1; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` ${error.message}`); + failed += 1; + } +} + +console.log('\n=== Hook session ID tests ===\n'); + +test('payload session_id takes precedence over both environment IDs', () => { + assert.strictEqual(resolveHookSessionId('{"session_id":"payload-session"}', { + ECC_SESSION_ID: 'ecc-session', CLAUDE_SESSION_ID: 'claude-session' + }), 'payload-session'); +}); + +test('ECC_SESSION_ID takes precedence over CLAUDE_SESSION_ID', () => { + assert.strictEqual(resolveHookSessionId('{}', { + ECC_SESSION_ID: 'ecc-session', CLAUDE_SESSION_ID: 'claude-session' + }), 'ecc-session'); +}); + +test('CLAUDE_SESSION_ID remains the legacy fallback', () => { + assert.strictEqual(resolveHookSessionId('', { CLAUDE_SESSION_ID: 'legacy-session' }), 'legacy-session'); +}); + +test('malformed and empty input safely fall back to the environment', () => { + for (const input of ['', ' ', '{bad json', undefined, null]) { + assert.strictEqual(resolveHookSessionId(input, { ECC_SESSION_ID: 'fallback' }), 'fallback'); + } +}); + +test('nonobject JSON does not supply a session ID', () => { + for (const input of ['null', '[]', '[{"session_id":"nested"}]', '42', 'true', '"session"']) { + assert.strictEqual(resolveHookSessionId(input, { CLAUDE_SESSION_ID: 'fallback' }), 'fallback'); + } +}); + +test('invalid payload ID types and blank IDs fall back safely', () => { + for (const sessionId of [null, 42, true, {}, [], '', ' \t\n']) { + assert.strictEqual(resolveHookSessionId(JSON.stringify({ session_id: sessionId }), { + ECC_SESSION_ID: 'fallback' + }), 'fallback'); + } +}); + +test('invalid ECC environment IDs fall back to the Claude environment ID', () => { + for (const sessionId of [undefined, null, 42, true, {}, [], '', ' \t\n']) { + assert.strictEqual(resolveHookSessionId('{}', { + ECC_SESSION_ID: sessionId, CLAUDE_SESSION_ID: 'legacy-session' + }), 'legacy-session'); + } +}); + +test('missing or invalid IDs return an empty string', () => { + assert.strictEqual(resolveHookSessionId('{}', {}), ''); + assert.strictEqual(resolveHookSessionId('{"session_id":false}', { + ECC_SESSION_ID: ' ', CLAUDE_SESSION_ID: null + }), ''); +}); + +test('only the top-level session_id field is accepted', () => { + assert.strictEqual(resolveHookSessionId('{"nested":{"session_id":"nested"},"sessionId":"camel"}', {}), ''); +}); + +test('raw IDs remain intact for each caller to apply its filename sanitizer', () => { + for (const sessionId of ['../session\\alpha:*?', 'CON', 'session:one', ' session-one ', 'sesión']) { + assert.strictEqual(resolveHookSessionId(JSON.stringify({ session_id: sessionId }), {}), sessionId); + assert.strictEqual(resolveHookSessionId('', { ECC_SESSION_ID: sessionId }), sessionId); + } +}); + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/lib/install-executor.test.js b/tests/lib/install-executor.test.js index ad831605b..a47517eec 100644 --- a/tests/lib/install-executor.test.js +++ b/tests/lib/install-executor.test.js @@ -162,6 +162,7 @@ function writeManifestSourceFixture(root) { writeFile(root, path.join('src', 'stray.pyd'), 'ignored\n'); writeFile(root, path.join('src', 'nested', 'ecc-install-state.json'), '{}\n'); writeFile(root, path.join('rules', 'common', 'coding-style.md'), '# Common\n'); + writeFile(root, path.join('rules', 'common', 'agents.md'), 'Use the `ecc:planner` agent.\n'); writeFile(root, path.join('skills', 'demo', 'SKILL.md'), '# Demo\n'); writeFile(root, 'standalone.txt', 'standalone\n'); writeFile(root, path.join('runtime', 'ecc', 'install-state.json'), '{}\n'); @@ -684,6 +685,11 @@ function runTests() { assert.ok(fs.existsSync(path.join(homeDir, '.claude', 'src', 'app.js'))); assert.ok(fs.existsSync(path.join(homeDir, '.claude', 'standalone.txt'))); assert.ok(fs.existsSync(path.join(homeDir, '.claude', 'plugin.json'))); + const installedAgentsRule = fs.readFileSync( + path.join(homeDir, '.claude', 'rules', 'ecc', 'common', 'agents.md'), + 'utf8' + ); + assert.strictEqual(installedAgentsRule, 'Use the `planner` agent.\n'); const state = JSON.parse(fs.readFileSync(path.join(homeDir, '.claude', 'ecc', 'install-state.json'), 'utf8')); assert.strictEqual(state.request.profile, 'minimal'); assert.deepStrictEqual(state.resolution.selectedModules, ['fixture-core']); @@ -1161,9 +1167,13 @@ function runTests() { for (const name of OPENCODE_ENTRYPOINTS) { assert.strictEqual( fs.readFileSync(path.join(homeDir, '.config', 'opencode', 'plugins', name), 'utf8'), - fs.readFileSync(path.join(REPO_ROOT, '.opencode', 'plugins', name), 'utf8') + name === 'index.ts' ? INERT_OPENCODE_PLUGIN : fs.readFileSync(path.join(REPO_ROOT, '.opencode', 'plugins', name), 'utf8') ); } + assert.strictEqual( + enabledPlan.statePreview.operations.find(operation => operation.sourceRelativePath.split(path.sep).join('/') === '.opencode/plugins/index.ts').contentTransform, + 'opencode-disable-plugin-entrypoint' + ); applyInstallPlanDirect(declinedPlan); const declinedState = JSON.parse(fs.readFileSync(declinedPlan.installStatePath, 'utf8')); assert.strictEqual(declinedState.request.hookConsent, 'declined'); diff --git a/tests/lib/install-plugin-namespace.test.js b/tests/lib/install-plugin-namespace.test.js new file mode 100644 index 000000000..7a0d2d15e --- /dev/null +++ b/tests/lib/install-plugin-namespace.test.js @@ -0,0 +1,86 @@ +/** + * Tests for scripts/lib/install/plugin-namespace.js — manual Claude installs + * place agents, commands and skills at bare names, so plugin-prefixed + * `ecc:` references in copied markdown must lose the prefix. + */ + +'use strict'; + +const assert = require('assert'); + +const { stripPluginNamespace } = require('../../scripts/lib/install/plugin-namespace'); + +function test(name, fn) { + try { + fn(); + console.log(` PASS ${name}`); + return true; + } catch (error) { + console.log(` FAIL ${name}`); + console.log(` Error: ${error.message}`); + console.error(error.stack || error); + return false; + } +} + +function runTests() { + console.log('Running install plugin-namespace tests...\n'); + let passed = 0; + let failed = 0; + const record = ok => (ok ? passed++ : failed++); + + record(test('strips the prefix from agent names in code spans, bold and tables', () => { + const input = [ + '| ecc:planner | Planning |', + '1. Use **ecc:code-reviewer** agent', + 'Use `ecc:tdd-guide` here', + ].join('\n'); + assert.strictEqual(stripPluginNamespace(input), [ + '| planner | Planning |', + '1. Use **code-reviewer** agent', + 'Use `tdd-guide` here', + ].join('\n')); + })); + + record(test('strips the prefix from agent frontmatter and tool-call arguments', () => { + assert.strictEqual( + stripPluginNamespace('agent: ecc:security-reviewer'), + 'agent: security-reviewer' + ); + assert.strictEqual( + stripPluginNamespace('Agent(subagent_type: "ecc:planner", prompt: "...")'), + 'Agent(subagent_type: "planner", prompt: "...")' + ); + })); + + record(test('strips the prefix from slash commands', () => { + assert.strictEqual(stripPluginNamespace('see `/ecc:ecc-guide`'), 'see `/ecc-guide`'); + assert.strictEqual(stripPluginNamespace('/ecc:plan "Add auth"'), '/plan "Add auth"'); + })); + + record(test('leaves the plugin identifier ecc@ecc untouched', () => { + const input = '`/plugin install ecc@ecc` and `ecc@ecc`'; + assert.strictEqual(stripPluginNamespace(input), input); + })); + + record(test('leaves words that only end in "ecc" untouched', () => { + const input = 'the decc:thing and tecc:value'; + assert.strictEqual(stripPluginNamespace(input), input); + })); + + record(test('does not touch a colon with no identifier after it', () => { + const input = 'the `ecc:` prefix in plugin mode'; + assert.strictEqual(stripPluginNamespace(input), input); + })); + + record(test('returns content with no prefix unchanged', () => { + const input = '# Plain markdown\n\nNo namespace here.\n'; + assert.strictEqual(stripPluginNamespace(input), input); + })); + + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); + return failed === 0; +} + +const ok = runTests(); +process.exit(ok ? 0 : 1); diff --git a/tests/lib/llm-summary.test.js b/tests/lib/llm-summary.test.js index 1705fe499..f836d20c4 100644 --- a/tests/lib/llm-summary.test.js +++ b/tests/lib/llm-summary.test.js @@ -185,6 +185,21 @@ test('returns null when ECC_SKIP_LLM_SUMMARY is set', () => { else delete process.env.ECC_SKIP_LLM_SUMMARY; }); +test('reports the explicit recursion guard as a deliberate skip', () => { + const orig = process.env.ECC_SKIP_LLM_SUMMARY; + process.env.ECC_SKIP_LLM_SUMMARY = '1'; + try { + const skipped = []; + assert.strictEqual(generateSessionSummary('/nonexistent.jsonl', { + onSkip: reason => skipped.push(reason) + }), null); + assert.deepStrictEqual(skipped, ['ECC_SKIP_LLM_SUMMARY']); + } finally { + if (orig !== undefined) process.env.ECC_SKIP_LLM_SUMMARY = orig; + else delete process.env.ECC_SKIP_LLM_SUMMARY; + } +}); + test('returns null for missing transcript (no conversation to summarize)', () => { const orig = process.env.ECC_SKIP_LLM_SUMMARY; delete process.env.ECC_SKIP_LLM_SUMMARY; @@ -201,6 +216,58 @@ test('marks the spawned summarizer so its Stop hook cannot create resume state', }); // --- Results --- +// Execute the real helper with a controlled clock and child-process boundary. +// These cases never invoke Claude or use the developer's authentication. +function summaryProbe(deadline, now = 100000, spawnResult = { status: 0, stdout: 'summary' }) { + const vm = require('vm'); + const source = fs.readFileSync(path.join(__dirname, '../../scripts/lib/llm-summary.js'), 'utf8'); + const calls = []; + const env = deadline === undefined ? {} : { ECC_HOOK_DEADLINE_MS: String(deadline) }; + const sandbox = { + module: { exports: {} }, + process: { env, platform: process.platform }, + Date: { now: () => now }, + require: name => name === 'child_process' + ? { spawnSync: (...args) => { calls.push(args); return spawnResult; } } + : require(name) + }; + vm.runInNewContext(source, sandbox, { filename: path.join(__dirname, '../../scripts/lib/llm-summary.js') }); + const skipped = []; + const result = sandbox.module.exports.generateSessionSummary( + writeTranscript([userEntry('Preserve mechanical resume state')]), + { onSkip: reason => skipped.push(reason) } + ); + return { calls, skipped, result }; +} + +test('limits summarizer to remaining hook budget minus persistence reserve', () => { + const probe = summaryProbe(105000); + assert.strictEqual(probe.result, 'summary'); + assert.strictEqual(probe.calls[0][2].timeout, 4000); + assert.strictEqual(probe.skipped.length, 0); +}); + +test('retains the standalone timeout and caps longer inherited budgets', () => { + assert.strictEqual(summaryProbe(undefined).calls[0][2].timeout, 90000); + assert.strictEqual(summaryProbe(300000).calls[0][2].timeout, 90000); +}); + +for (const deadline of [99999, 100000, 100999, 101000, '', -1, 'invalid', 'Infinity', '1.5', Number.MAX_SAFE_INTEGER + 1]) { + test(`skips summarizer with exhausted or invalid deadline ${deadline}`, () => { + const probe = summaryProbe(deadline); + assert.strictEqual(probe.result, null); + assert.strictEqual(probe.calls.length, 0, 'No child should start without a safe budget'); + assert.strictEqual(probe.skipped.length, 1, 'Skip must be distinguishable from Claude failure'); + }); +} + +test('keeps process failures distinct from deliberate budget skips', () => { + const probe = summaryProbe(105000, 100000, { status: 1, stdout: '' }); + assert.strictEqual(probe.result, null); + assert.strictEqual(probe.calls.length, 1); + assert.strictEqual(probe.skipped.length, 0); +}); + console.log('\n=== Test Results ==='); console.log(`Passed: ${passed}`); console.log(`Failed: ${failed}`); diff --git a/tests/lib/session-aliases-concurrency.test.js b/tests/lib/session-aliases-concurrency.test.js new file mode 100644 index 000000000..dcec7b30a --- /dev/null +++ b/tests/lib/session-aliases-concurrency.test.js @@ -0,0 +1,87 @@ +'use strict'; +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawn } = require('child_process'); +const worker = path.join(__dirname, 'helpers', 'session-alias-worker.js'); + +async function waitFor(file, children) { + const deadline = Date.now() + 10000; + while (!fs.existsSync(file)) { + for (const child of children) { + if (child.exitCode !== null || child.signalCode !== null) throw new Error('Child exited before barrier: ' + file); + } + if (Date.now() > deadline) throw new Error('Barrier timed out: ' + file); + await new Promise(resolve => setTimeout(resolve, 10)); + } +} +function start(root, role, action) { + const child = spawn(process.execPath, [worker, root, role, action], { + env: { ...process.env, HOME: root, USERPROFILE: root, ECC_AGENT_DATA_HOME: path.join(root, '.claude') }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', data => { stdout += data; }); + child.stderr.on('data', data => { stderr += data; }); + child.done = new Promise((resolve, reject) => { + child.on('error', reject); + child.on('close', (code, signal) => { + if (code !== 0) reject(new Error('Worker failed: ' + code + '/' + signal + ' ' + stderr)); + else { + try { resolve(JSON.parse(stdout)); } catch (error) { reject(error); } + } + }); + }); + child.done.catch(() => {}); + return child; +} +async function race(action) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-alias-race-')); + const dataDir = path.join(root, '.claude'); + fs.mkdirSync(dataDir); + const aliasesPath = path.join(dataDir, 'session-aliases.json'); + fs.writeFileSync(aliasesPath, JSON.stringify({ + version: '1.0', + aliases: { existing: { sessionPath: '/existing', createdAt: '2026-01-01', title: 'Original' } }, + metadata: {}, + })); + const children = []; + const timer = setTimeout(() => children.forEach(child => child.kill('SIGKILL')), 20000); + try { + const alpha = start(root, 'alpha', action); + children.push(alpha); + await waitFor(path.join(root, 'alpha.read'), children); + const beta = start(root, 'beta', action); + children.push(beta); + await waitFor(path.join(root, 'beta.attempt'), children); + fs.writeFileSync(path.join(root, 'alpha.release'), ''); + assert.strictEqual((await alpha.done).success, true); + fs.writeFileSync(path.join(root, 'beta.release'), ''); + assert.strictEqual((await beta.done).success, true); + const data = JSON.parse(fs.readFileSync(aliasesPath, 'utf8')); + assert.ok(data.aliases.alpha, action + ' must preserve the preceding writer'); + if (action === 'set') assert.ok(data.aliases.beta); + if (['delete', 'cleanup', 'rename'].includes(action)) assert.ok(!data.aliases.existing); + if (action === 'rename') assert.ok(data.aliases.renamed); + if (action === 'title') assert.strictEqual(data.aliases.existing.title, 'Updated'); + assert.strictEqual(data.metadata.totalCount, Object.keys(data.aliases).length); + assert.deepStrictEqual(fs.readdirSync(dataDir), ['session-aliases.json']); + } finally { + clearTimeout(timer); + for (const child of children) { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); + } + await Promise.allSettled(children.map(child => child.done)); + fs.rmSync(root, { recursive: true, force: true }); + } +} +(async () => { + let failed = 0; + for (const action of ['set', 'delete', 'rename', 'title', 'cleanup']) { + try { await race(action); console.log('PASS concurrent ' + action); } + catch (error) { failed++; console.error('FAIL concurrent ' + action + ': ' + error.message); } + } + process.exitCode = failed ? 1 : 0; +})(); diff --git a/tests/lib/session-aliases.test.js b/tests/lib/session-aliases.test.js index 0b2554cb1..3ab5fa35c 100644 --- a/tests/lib/session-aliases.test.js +++ b/tests/lib/session-aliases.test.js @@ -18,6 +18,8 @@ const tmpHome = path.join(os.tmpdir(), `ecc-alias-test-${Date.now()}`); fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); const origHome = process.env.HOME; const origUserProfile = process.env.USERPROFILE; +const origAgentDataHome = process.env.ECC_AGENT_DATA_HOME; +delete process.env.ECC_AGENT_DATA_HOME; process.env.HOME = tmpHome; process.env.USERPROFILE = tmpHome; // Windows: os.homedir() uses USERPROFILE @@ -767,18 +769,16 @@ function runTests() { assert.strictEqual(resolved.title, 'Original Title'); })) passed++; else failed++; - // ── Round 33: saveAliases backup restoration ── - console.log('\nsaveAliases backup/restore (Round 33):'); + // ── Round 33: saveAliases atomic publication ── + console.log('\nsaveAliases atomic publication (Round 33):'); - if (test('saveAliases creates backup before write and removes on success', () => { + if (test('saveAliases publishes a snapshot without leaving staging files', () => { resetAliases(); aliases.setAlias('backup-test', '/path/backup'); - // After successful save, .bak file should NOT exist - const aliasesPath = path.join(tmpHome, '.claude', 'session-aliases.json'); - const backupPath = aliasesPath + '.bak'; - assert.ok(!fs.existsSync(backupPath), 'Backup should be removed after successful save'); + const aliasesPath = aliases.getAliasesPath(); assert.ok(fs.existsSync(aliasesPath), 'Main aliases file should exist'); + assert.deepStrictEqual(fs.readdirSync(path.dirname(aliasesPath)), ['session-aliases.json']); })) passed++; else failed++; if (test('saveAliases with non-serializable data returns false and preserves existing file', () => { @@ -795,7 +795,7 @@ function runTests() { const result = aliases.saveAliases(circular); assert.strictEqual(result, false, 'Should return false'); - // The file should still have the old content (restored from backup or untouched) + // The file should still have the old content (untouched) const contentAfter = fs.readFileSync(aliasesPath, 'utf8'); assert.ok(contentAfter.includes('before-fail'), 'Original aliases data should be preserved after failed save'); @@ -843,10 +843,10 @@ function runTests() { assert.strictEqual(data.metadata.totalCount, 5, 'Metadata count should match actual aliases'); })) passed++; else failed++; - // ── Round 56: Windows platform unlink-before-rename code path ── + // ── Round 56: Windows platform atomic replacement ── console.log('\nRound 56: Windows platform atomic write path:'); - if (test('Windows platform mock: unlinks existing file before rename', () => { + if (test('Windows platform mock: replaces existing file without unlinking it', () => { resetAliases(); // First create an alias so the file exists const r1 = aliases.setAlias('win-initial', '2026-01-01-abc123-session.tmp'); @@ -854,12 +854,17 @@ function runTests() { const aliasesPath = aliases.getAliasesPath(); assert.ok(fs.existsSync(aliasesPath), 'Aliases file should exist before win32 test'); - // Mock process.platform to 'win32' to trigger the unlink-before-rename path + // Check that Windows uses replacement without deleting the published file + const originalUnlink = fs.unlinkSync; + fs.unlinkSync = function (file) { + assert.notStrictEqual(file, aliasesPath, 'Never unlink the published aliases file'); + return originalUnlink.apply(this, arguments); + }; const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); try { - // This save triggers the Windows code path: unlink existing → rename temp + // Replace the snapshot while preserving the original until publication const r2 = aliases.setAlias('win-updated', '2026-02-01-def456-session.tmp'); assert.strictEqual(r2.success, true, 'setAlias should succeed under win32 mock'); @@ -875,6 +880,7 @@ function runTests() { assert.ok(!fs.existsSync(aliasesPath + '.tmp'), 'No temp file should remain'); assert.ok(!fs.existsSync(aliasesPath + '.bak'), 'No backup file should remain'); } finally { + fs.unlinkSync = originalUnlink; // Restore original platform descriptor if (origPlatform) { Object.defineProperty(process, 'platform', origPlatform); @@ -1091,9 +1097,10 @@ function runTests() { assert.strictEqual(result.success, false, 'Should fail when save is blocked'); assert.ok(result.error.includes('Failed to save after cleanup'), `Should return cleanup save failure error, got: ${result.error}`); - assert.strictEqual(result.removed, 1, 'Should report 1 removed alias'); - assert.ok(result.removedAliases.some(a => a.name === 'remove-me'), - 'Should report remove-me in removedAliases'); + assert.strictEqual(result.removed, 0, 'No cleanup runs without the transaction lock'); + assert.deepStrictEqual(result.removedAliases, []); + assert.strictEqual(result.totalChecked, 0); + assert.ok(freshAliases.resolveAlias('remove-me'), 'Published aliases stay unchanged'); } finally { try { fs.chmodSync(isoClaudeDir, 0o755); } catch { /* best-effort */ } process.env.HOME = savedHome; @@ -1210,53 +1217,34 @@ function runTests() { resetAliases(); })) passed++; else failed++; - // ── Round 90: saveAliases backup restore double failure (inner catch restoreErr) ── - console.log('\nRound 90: saveAliases (backup restore double failure):'); - - if (test('saveAliases triggers inner restoreErr catch when both save and restore fail', () => { - // session-aliases.js lines 131-137: When saveAliases fails (outer catch), - // it tries to restore from backup. If the restore ALSO fails, the inner - // catch at line 135 logs restoreErr. No existing test creates this double-fault. - if (process.platform === 'win32' || process.getuid?.() === 0) { - console.log(' (skipped — chmod ineffective on Windows/root)'); - return; - } - const isoHome = path.join(os.tmpdir(), `ecc-r90-restore-fail-${Date.now()}`); - const claudeDir = path.join(isoHome, '.claude'); - fs.mkdirSync(claudeDir, { recursive: true }); - - // Pre-create a backup file while directory is still writable - const backupPath = path.join(claudeDir, 'session-aliases.json.bak'); - fs.writeFileSync(backupPath, JSON.stringify({ aliases: {}, version: '1.0' })); - - // Make .claude directory read-only (0o555): - // 1. writeFileSync(tempPath) → EACCES (can't create file in read-only dir) — outer catch - // 2. copyFileSync(backupPath, aliasesPath) → EACCES (can't create target) — inner catch (line 135) - fs.chmodSync(claudeDir, 0o555); - - const origH = process.env.HOME; - const origP = process.env.USERPROFILE; - process.env.HOME = isoHome; - process.env.USERPROFILE = isoHome; - + // Failed publication must preserve the original bytes, even under win32. + if (test('failed replacement preserves published aliases and removes only its staging file', () => { + resetAliases(); + aliases.setAlias('preserved', '/original'); + const aliasesPath = aliases.getAliasesPath(); + const before = fs.readFileSync(aliasesPath, 'utf8'); + const originalRename = fs.renameSync; + const staged = []; + fs.renameSync = function (source, target) { + if (target === aliasesPath) { + staged.push(source); + const error = new Error('replacement denied'); + error.code = 'EPERM'; + throw error; + } + return originalRename.apply(this, arguments); + }; try { - delete require.cache[require.resolve('../../scripts/lib/session-aliases')]; - delete require.cache[require.resolve('../../scripts/lib/utils')]; - const freshAliases = require('../../scripts/lib/session-aliases'); - - const result = freshAliases.saveAliases({ aliases: { x: 1 }, version: '1.0' }); - assert.strictEqual(result, false, 'Should return false when save fails'); - - // Backup should still exist (restore also failed, so backup was not consumed) - assert.ok(fs.existsSync(backupPath), 'Backup should still exist after double failure'); + assert.strictEqual(aliases.saveAliases({ aliases: { replacement: { sessionPath: '/new' } } }), false); + assert.strictEqual(aliases.renameAlias('preserved', 'changed').success, false); + assert.strictEqual(fs.readFileSync(aliasesPath, 'utf8'), before); + assert.strictEqual(staged.length, 2, 'Each failed operation attempts one publication'); + assert.notStrictEqual(staged[0], staged[1], 'Each write has its own staging file'); + assert.deepStrictEqual(fs.readdirSync(path.dirname(aliasesPath)), ['session-aliases.json']); } finally { - process.env.HOME = origH; - process.env.USERPROFILE = origP; - delete require.cache[require.resolve('../../scripts/lib/session-aliases')]; - delete require.cache[require.resolve('../../scripts/lib/utils')]; - try { fs.chmodSync(claudeDir, 0o755); } catch { /* best-effort */ } - fs.rmSync(isoHome, { recursive: true, force: true }); + fs.renameSync = originalRename; } + assert.strictEqual(aliases.setAlias('after-failure', '/next').success, true, 'Lock is released after failure'); })) passed++; else failed++; // ── Round 95: renameAlias with same old and new name (self-rename) ── @@ -1809,6 +1797,158 @@ function runTests() { 'Object.keys includes normal alias'); })) passed++; else failed++; + if (test('cleanup rejects reentrant writes without losing the outer transaction', () => { + resetAliases(); + aliases.setAlias('existing', '/existing'); + const nested = []; + const result = aliases.cleanupAliases(() => { + nested.push(aliases.setAlias('nested', '/nested')); + nested.push(aliases.deleteAlias('existing')); + nested.push(aliases.renameAlias('existing', 'renamed')); + nested.push(aliases.updateAliasTitle('existing', 'Nested')); + nested.push(aliases.cleanupAliases(() => true)); + nested.push(aliases.saveAliases({ aliases: {} })); + return false; + }); + assert.strictEqual(result.success, true); + assert.ok(nested.slice(0, -1).every(value => value.success === false)); + assert.strictEqual(nested.at(-1), false); + assert.deepStrictEqual(aliases.loadAliases().aliases, {}); + assert.strictEqual(aliases.setAlias('after-cleanup', '/next').success, true); + })) passed++; else failed++; + + if (test('cleanup preserves callback errors and releases its lock', () => { + resetAliases(); + aliases.setAlias('existing', '/existing'); + const primary = new Error('session lookup failed'); + assert.throws(() => aliases.cleanupAliases(() => { throw primary; }), error => error === primary); + assert.ok(aliases.resolveAlias('existing')); + assert.ok(!fs.existsSync(aliases.getAliasesPath() + '.ecc.lock')); + assert.strictEqual(aliases.setAlias('after-error', '/next').success, true); + })) passed++; else failed++; + + if (test('cleanup propagates arbitrary thrown values even if lock release also fails', () => { + resetAliases(); + aliases.setAlias('existing', '/existing'); + const lockPath = aliases.getAliasesPath() + '.ecc.lock'; + const originalRename = fs.renameSync; + const errors = [null, undefined, false, 0, Object.freeze(new Error('frozen')), + new Proxy(new Error('proxy'), { set() { throw new Error('Cannot annotate error'); } })]; + for (const failRelease of [false, true]) { + for (const primary of errors) { + let caught = false; + fs.renameSync = function (source) { + if (failRelease && source === lockPath) throw new Error('release denied'); + return originalRename.apply(this, arguments); + }; + try { + aliases.cleanupAliases(() => { throw primary; }); + } catch (error) { + caught = true; + assert.strictEqual(error, primary, 'Original thrown value is preserved'); + } finally { + fs.renameSync = originalRename; + fs.rmSync(lockPath, { force: true }); + } + assert.ok(caught, 'Callback failure must propagate'); + } + } + assert.strictEqual(aliases.setAlias('after-release-error', '/next').success, true); + })) passed++; else failed++; + + if (test('successful writes keep their results when releasing the lock fails', () => { + const cases = [ + { + run: () => aliases.setAlias('added', '/added', 'Added'), + expected: { success: true, isNew: true, alias: 'added', sessionPath: '/added', title: 'Added' }, + verify: data => assert.strictEqual(data.added.sessionPath, '/added') + }, + { + run: () => aliases.deleteAlias('existing'), + expected: { success: true, alias: 'existing', deletedSessionPath: '/existing' }, + verify: data => assert.deepStrictEqual(data, {}) + }, + { + run: () => aliases.renameAlias('existing', 'renamed'), + expected: { success: true, oldAlias: 'existing', newAlias: 'renamed', sessionPath: '/existing' }, + verify: data => assert.deepStrictEqual(Object.keys(data), ['renamed']) + }, + { + run: () => aliases.updateAliasTitle('existing', 'Updated'), + expected: { success: true, alias: 'existing', title: 'Updated' }, + verify: data => assert.strictEqual(data.existing.title, 'Updated') + }, + { + run: () => aliases.cleanupAliases(() => false), + expected: { success: true, totalChecked: 1, removed: 1, + removedAliases: [{ name: 'existing', sessionPath: '/existing' }] }, + verify: data => assert.deepStrictEqual(data, {}) + }, + { + run: () => aliases.saveAliases({ aliases: { replacement: { sessionPath: '/replacement' } } }), + expected: true, + verify: data => assert.deepStrictEqual(data, { replacement: { sessionPath: '/replacement' } }) + } + ]; + for (const entry of cases) { + resetAliases(); + aliases.setAlias('existing', '/existing'); + const lockPath = aliases.getAliasesPath() + '.ecc.lock'; + const originalRename = fs.renameSync; + const originalError = console.error; + const warnings = []; + try { + fs.renameSync = function (source) { + if (source === lockPath) throw new Error('release denied'); + return originalRename.apply(this, arguments); + }; + console.error = message => warnings.push(message); + assert.deepStrictEqual(entry.run(), entry.expected); + entry.verify(aliases.loadAliases().aliases); + assert.ok(warnings.some(message => message.includes('release denied')), + 'A release failure remains visible to the caller'); + } finally { + fs.renameSync = originalRename; + console.error = originalError; + fs.rmSync(lockPath, { force: true }); + } + assert.strictEqual(aliases.setAlias('after-release-error', '/next').success, true, + 'The local reentry guard is cleared after a release failure'); + } + })) passed++; else failed++; + + if (test('a release failure preserves the original operation failure result', () => { + resetAliases(); + aliases.setAlias('existing', '/existing'); + const lockPath = aliases.getAliasesPath() + '.ecc.lock'; + const originalRename = fs.renameSync; + try { + fs.renameSync = function (source) { + if (source === lockPath) throw new Error('release denied'); + return originalRename.apply(this, arguments); + }; + assert.deepStrictEqual(aliases.deleteAlias('missing'), + { success: false, error: "Alias 'missing' not found" }); + assert.ok(aliases.resolveAlias('existing')); + } finally { + fs.renameSync = originalRename; + fs.rmSync(lockPath, { force: true }); + } + })) passed++; else failed++; + + if (test('saveAliases replaces its complete snapshot without mutating the caller', () => { + resetAliases(); + aliases.setAlias('existing', '/existing'); + const snapshot = { aliases: { replacement: { sessionPath: '/replacement' } }, metadata: { caller: true } }; + const before = JSON.stringify(snapshot); + assert.strictEqual(aliases.saveAliases(snapshot), true); + assert.strictEqual(JSON.stringify(snapshot), before); + assert.deepStrictEqual(Object.keys(aliases.loadAliases().aliases), ['replacement']); + })) passed++; else failed++; + + if (origAgentDataHome === undefined) delete process.env.ECC_AGENT_DATA_HOME; + else process.env.ECC_AGENT_DATA_HOME = origAgentDataHome; + // Cleanup — restore both HOME and USERPROFILE (Windows) process.env.HOME = origHome; if (origUserProfile !== undefined) { diff --git a/tests/lib/skill-dashboard.test.js b/tests/lib/skill-dashboard.test.js index f90052e85..d1f29d0df 100644 --- a/tests/lib/skill-dashboard.test.js +++ b/tests/lib/skill-dashboard.test.js @@ -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', () => { diff --git a/tests/lib/skill-evolution.test.js b/tests/lib/skill-evolution.test.js index bfff437bf..f2dd4e6ec 100644 --- a/tests/lib/skill-evolution.test.js +++ b/tests/lib/skill-evolution.test.js @@ -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'); diff --git a/tests/lib/state-store.test.js b/tests/lib/state-store.test.js index 80ac95ec9..a1e2f045c 100644 --- a/tests/lib/state-store.test.js +++ b/tests/lib/state-store.test.js @@ -38,12 +38,22 @@ function cleanupTempDir(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } +// The status CLI reconciles install-state found under $HOME and the OpenCode +// config dirs, so CLI runs get an empty home instead of the developer's real +// ~/.claude and ~/.config/opencode. +const ISOLATED_HOME = createTempDir('ecc-state-home-'); +process.on('exit', () => cleanupTempDir(ISOLATED_HOME)); + function runNode(scriptPath, args = [], options = {}) { return spawnSync('node', [scriptPath, ...args], { encoding: 'utf8', cwd: options.cwd || process.cwd(), env: { ...process.env, + HOME: ISOLATED_HOME, + USERPROFILE: ISOLATED_HOME, + XDG_CONFIG_HOME: path.join(ISOLATED_HOME, '.config'), + OPENCODE_CONFIG_DIR: path.join(ISOLATED_HOME, '.config', 'opencode'), ...(options.env || {}), }, }); diff --git a/tests/lib/windows-bash.test.js b/tests/lib/windows-bash.test.js new file mode 100644 index 000000000..ae1cd7315 --- /dev/null +++ b/tests/lib/windows-bash.test.js @@ -0,0 +1,115 @@ +/** + * Tests for scripts/lib/windows-bash.js + * + * Platform-independent: the Windows lookup is driven by an injected platform + * and env over a temp tree laid out like a Windows install. + * + * Run with: node tests/lib/windows-bash.test.js + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { isWslBashLauncher, resolveWindowsBashCandidates } = require('../../scripts/lib/windows-bash'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + passed++; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` Error: ${error.message}`); + failed++; + } +} + +function touch(dir, name) { + fs.mkdirSync(dir, { recursive: true }); + const filePath = path.join(dir, name); + fs.writeFileSync(filePath, ''); + return filePath; +} + +console.log('\n=== Testing windows-bash ===\n'); + +const root = fs.mkdtempSync(path.join(os.tmpdir(), 'windows-bash-')); +const windowsDir = path.join(root, 'Windows'); +const localAppData = path.join(root, 'Local'); +const system32 = path.join(windowsDir, 'System32'); +const windowsApps = path.join(localAppData, 'Microsoft', 'WindowsApps'); +const gitBin = path.join(root, 'Git', 'usr', 'bin'); +const toolsBin = path.join(root, 'tools'); + +touch(system32, 'bash.exe'); +touch(windowsApps, 'bash.exe'); +const gitBash = touch(gitBin, 'bash.exe'); +const gitSh = touch(gitBin, 'sh.exe'); +const toolsSh = touch(toolsBin, 'sh.exe'); + +// The WSL launchers sit ahead of Git on PATH, as on a default install. +const env = { + PATH: [system32, windowsApps, toolsBin, gitBin].join(';'), + SystemRoot: windowsDir, + LOCALAPPDATA: localAppData, +}; +const win32 = { env, platform: 'win32' }; + +try { + test('skips System32 and WindowsApps launchers for a native bash later on PATH', () => { + assert.deepStrictEqual(resolveWindowsBashCandidates(['bash.exe', 'bash'], win32), [gitBash]); + }); + + test('prefers earlier names over PATH order, so bash beats an earlier sh', () => { + assert.deepStrictEqual( + resolveWindowsBashCandidates(['bash.exe', 'bash', 'sh'], win32), + [gitBash, toolsSh, gitSh] + ); + }); + + test('reads Windows environment names case-insensitively', () => { + const mixedCaseEnv = { + Path: env.PATH, + SYSTEMROOT: windowsDir, + LocalAppData: localAppData, + }; + assert.deepStrictEqual( + resolveWindowsBashCandidates(['bash'], { env: mixedCaseEnv, platform: 'win32' }), + [gitBash] + ); + }); + + test('returns nothing when only WSL launchers or no PATH are present', () => { + const launchersOnly = { ...env, PATH: [system32, windowsApps].join(';') }; + assert.deepStrictEqual(resolveWindowsBashCandidates(['bash'], { env: launchersOnly, platform: 'win32' }), []); + assert.deepStrictEqual(resolveWindowsBashCandidates(['bash'], { env: {}, platform: 'win32' }), []); + }); + + test('leaves names untouched off Windows', () => { + assert.deepStrictEqual(resolveWindowsBashCandidates(['bash', 'sh'], { env, platform: 'linux' }), ['bash', 'sh']); + }); + + test('recognises launchers in System32, SysWOW64, and Sysnative regardless of case', () => { + for (const dir of ['SYSTEM32', 'SysWOW64', 'sysnative']) { + assert.ok(isWslBashLauncher(path.join(windowsDir, dir, 'BASH.EXE'), env), dir); + } + assert.ok(isWslBashLauncher(path.join(windowsApps, 'bash.exe'), env)); + }); + + test('does not flag a native bash or a non-bash binary in System32', () => { + assert.ok(!isWslBashLauncher(gitBash, env)); + assert.ok(!isWslBashLauncher(path.join(system32, 'sh.exe'), env)); + }); +} finally { + fs.rmSync(root, { recursive: true, force: true }); +} + +console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/lib/windows-test-env.test.js b/tests/lib/windows-test-env.test.js new file mode 100644 index 000000000..be888271f --- /dev/null +++ b/tests/lib/windows-test-env.test.js @@ -0,0 +1,62 @@ +/** + * Tests for tests/lib/helpers/windows-test-env.js + * + * Run with: node tests/lib/windows-test-env.test.js + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { test, banner, summary } = require('./helpers/mini-test-runner'); +const { withoutShadowingClaude, toRelativeTarPath } = require('./helpers/windows-test-env'); + +banner('windows-test-env helper'); + +let passed = 0; +let failed = 0; + +const root = fs.mkdtempSync(path.join(os.tmpdir(), 'windows-test-env-')); +const exeDir = path.join(root, 'native-install'); +const comDir = path.join(root, 'com-install'); +const cmdDir = path.join(root, 'npm-global'); +const plainDir = path.join(root, 'tools'); +for (const dir of [exeDir, comDir, cmdDir, plainDir]) fs.mkdirSync(dir); +fs.writeFileSync(path.join(exeDir, 'claude.exe'), ''); +fs.writeFileSync(path.join(comDir, 'claude.com'), ''); +// A .cmd shim cannot shadow a fake .cmd placed earlier on PATH, so it stays. +fs.writeFileSync(path.join(cmdDir, 'claude.cmd'), ''); + +try { + if (test('drops PATH entries holding claude.exe or claude.com on Windows', () => { + const pathValue = [plainDir, exeDir, cmdDir, comDir].join(';'); + assert.strictEqual(withoutShadowingClaude(pathValue, 'win32'), [plainDir, cmdDir].join(';')); + })) passed++; else failed++; + + if (test('drops empty entries and tolerates a missing PATH on Windows', () => { + assert.strictEqual(withoutShadowingClaude(`;${plainDir};;`, 'win32'), plainDir); + assert.strictEqual(withoutShadowingClaude(undefined, 'win32'), ''); + })) passed++; else failed++; + + if (test('leaves PATH untouched off Windows', () => { + const pathValue = [plainDir, exeDir].join(':'); + assert.strictEqual(withoutShadowingClaude(pathValue, 'linux'), pathValue); + assert.strictEqual(withoutShadowingClaude(undefined, 'darwin'), ''); + })) passed++; else failed++; + + if (test('names an archive relatively with forward slashes and no drive letter', () => { + const cwd = path.join(root, 'extract', 'node_modules'); + const archive = path.join(root, 'pkg-1.0.0.tgz'); + const relative = toRelativeTarPath(cwd, archive); + assert.ok(!relative.includes(':'), `tar reads a colon as host:path: ${relative}`); + assert.ok(!relative.includes('\\'), `expected forward slashes: ${relative}`); + assert.strictEqual(path.resolve(cwd, relative), archive); + })) passed++; else failed++; +} finally { + fs.rmSync(root, { recursive: true, force: true }); +} + +summary(passed, failed); diff --git a/tests/opencode-plugin-hooks.test.js b/tests/opencode-plugin-hooks.test.js index a6c3ed2ae..421f38113 100644 --- a/tests/opencode-plugin-hooks.test.js +++ b/tests/opencode-plugin-hooks.test.js @@ -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} 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>} + */ +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, fs.realpathSync(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 diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 50d0cf5f0..7889fa5d6 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -11,6 +11,7 @@ const TOML = require('@iarna/toml'); const repoRoot = path.join(__dirname, '..', '..'); const installScript = path.join(repoRoot, 'scripts', 'codex', 'install-global-git-hooks.sh'); +const preCommitHook = path.join(repoRoot, 'scripts', 'codex-git-hooks', 'pre-commit'); const prePushHook = path.join(repoRoot, 'scripts', 'codex-git-hooks', 'pre-push'); const pluginCacheCheckScript = path.join(repoRoot, 'scripts', 'codex', 'check-plugin-cache.js'); const mergeCodexConfigScript = path.join(repoRoot, 'scripts', 'codex', 'merge-codex-config.js'); @@ -164,6 +165,86 @@ if ( passed++; else failed++; +function runHermeticPreCommit({ content = 'safe text\n', filename = 'sample.txt', scanner = false, env = {} } = {}) { + const tempDir = createTempDir('codex-pre-commit-'); + try { + const bashEnv = path.join(tempDir, 'bash-env'); + const binDir = path.join(tempDir, 'bin'); + const callsPath = path.join(tempDir, 'scanner-calls'); + fs.mkdirSync(binDir); + const hookEnv = makeHermeticCodexEnv(tempDir, path.join(tempDir, '.codex'), { + PATH: process.env.PATH, + GIT_CONFIG_NOSYSTEM: '1', + MSYS_NO_PATHCONV: '1', + ECC_SKIP_GIT_HOOKS: '0', + ECC_SKIP_PRECOMMIT: '0', + ...env, + }); + const initialized = spawnSync('git', ['init', '--quiet'], { cwd: tempDir, env: hookEnv }); + assert.strictEqual(initialized.status, 0, initialized.stderr?.toString()); + if (content !== null) { + fs.writeFileSync(path.join(tempDir, filename), content); + const staged = spawnSync('git', ['add', '--', filename], { cwd: tempDir, env: hookEnv }); + assert.strictEqual(staged.status, 0, staged.stderr?.toString()); + } + // Keep real Git/awk available, while hiding every installed ripgrep binary. + fs.writeFileSync(bashEnv, ` +git_path="$(command -v git)" +awk_path="$(command -v awk)" +head_path="$(command -v head)" +cat_path="$(command -v cat)" +rg_path="$(command -v rg || true)" +git() { "$git_path" "$@"; } +awk() { "$awk_path" "$@"; } +head() { "$head_path" "$@"; } +cat() { "$cat_path" "$@"; } +unset -f rg +PATH="${toBashPath(binDir)}" +${scanner ? `rg() { +printf '%s\\n' "$*" >> "${toBashPath(callsPath)}" +${scanner === 'real' ? '"$rg_path" "$@"' : scanner === 'finding' ? "printf '1:synthetic-scanner-finding\\n'; return 0;" : scanner === 'error' ? 'return 2;' : 'return 1;'} +}` : ''} +`); + const result = runBash(preCommitHook, { + env: { ...hookEnv, BASH_ENV: toBashPath(bashEnv) }, + cwd: tempDir, + preservePath: false, + }); + result.scannerCalls = fs.existsSync(callsPath) ? fs.readFileSync(callsPath, 'utf8').trim().split('\n') : []; + return result; + } finally { + cleanup(tempDir); + } +} + +for (const [name, options, status, diagnostic] of [ + ['blocks safe staged text when ripgrep is missing', {}, 1, /ripgrep.*required/i], + ['blocks a staged token when ripgrep is missing', { content: `token = "ghp_${'a'.repeat(36)}"\n` }, 1, /ripgrep.*required/i], + ['allows a successful scan with no findings', { scanner: true }, 0, null], + ['blocks a finding reported by an invoked scanner', { scanner: 'finding' }, 1, /Potential secret detected/], + ['blocks an invoked scanner error', { scanner: 'error' }, 1, /scanner failed.*exit 2/i], + ['allows safe text through the real scanner', { scanner: 'real' }, 0, null], + ['blocks a token through the real scanner', { scanner: 'real', content: `token = "ghp_${'a'.repeat(36)}"\n` }, 1, /Potential secret detected/], + ['blocks a private key through the real scanner', { scanner: 'real', content: '-----BEGIN PRIVATE KEY-----\n' }, 1, /private key block/], + ['blocks an RSA private key through the real scanner', { scanner: 'real', content: '-----BEGIN RSA PRIVATE KEY-----\n' }, 1, /private key block/], + ['allows an empty index without ripgrep', { content: null }, 0, null], + ['allows excluded lockfiles without ripgrep', { filename: 'yarn.lock' }, 0, null], + ['preserves the pre-commit bypass warning', { env: { ECC_SKIP_PRECOMMIT: '1' } }, 0, /hook bypassed via env/], + ['preserves the global bypass warning', { env: { ECC_SKIP_GIT_HOOKS: '1' } }, 0, /hook bypassed via env/], +]) { + if (test(`pre-commit ${name}`, () => { + const result = runHermeticPreCommit(options); + assert.strictEqual(result.status, status, `${result.stdout}\n${result.stderr}`); + if (diagnostic) assert.match(result.stderr, diagnostic); + else assert.strictEqual(result.stderr, ''); + if (options.scanner) { + assert.ok(result.scannerCalls.length > 0, 'The scanner must actually execute'); + assert.ok(result.scannerCalls.every(call => call.startsWith('-n --pcre2 -- ')), 'Regex must follow the option terminator'); + } + })) passed++; + else failed++; +} + function runHermeticPrePush({ failScript = null, includeCorepack = true, diff --git a/tests/scripts/ecc-universal-bin.test.js b/tests/scripts/ecc-universal-bin.test.js index c4336d26a..27f508e95 100644 --- a/tests/scripts/ecc-universal-bin.test.js +++ b/tests/scripts/ecc-universal-bin.test.js @@ -12,6 +12,7 @@ const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); const { getNpmPackEntry } = require('../lib/npm-pack-output'); +const { toRelativeTarPath } = require('../lib/helpers/windows-test-env'); const repoRoot = path.join(__dirname, '..', '..'); const packageJson = JSON.parse( @@ -170,8 +171,8 @@ function prepareLocalPackedProject(packageManager) { }); } fs.mkdirSync(modulesDirectory, { recursive: true }); - run('tar', ['-xzf', fixture.archivePath, '-C', modulesDirectory], { - cwd: projectDirectory, + run('tar', ['-xzf', toRelativeTarPath(modulesDirectory, fixture.archivePath)], { + cwd: modulesDirectory, timeout: archiveExtractionTimeoutMs, }); fs.renameSync(extractedDirectory, packageDirectory); diff --git a/tests/scripts/eval-harness-package.test.js b/tests/scripts/eval-harness-package.test.js index b29adf69d..5d413ca8c 100644 --- a/tests/scripts/eval-harness-package.test.js +++ b/tests/scripts/eval-harness-package.test.js @@ -8,6 +8,7 @@ const path = require('path'); const { spawnSync } = require('child_process'); const { getNpmPackEntry } = require('../lib/npm-pack-output'); const { test, tempDir, cleanup, finish, runNpm } = require('../lib/eval-harness/helpers'); +const { toRelativeTarPath } = require('../lib/helpers/windows-test-env'); const repo = path.resolve(__dirname, '../..'); const work = tempDir('package smoke'); @@ -61,7 +62,7 @@ try { assert.ok(archive, 'packing must succeed before extracting'); const extract = path.join(work, 'extracted'); fs.mkdirSync(extract); - const unpack = command('tar', ['-xzf', archive, '-C', extract]); + const unpack = command('tar', ['-xzf', toRelativeTarPath(extract, archive)], { cwd: extract }); assert.strictEqual(unpack.status, 0, unpack.error?.message || unpack.stderr); const installed = path.join(extract, 'package'); assert.ok(!fs.existsSync(path.join(installed, 'node_modules'))); diff --git a/tests/scripts/harness-audit.test.js b/tests/scripts/harness-audit.test.js index f0fb56c1b..0e5da3c6e 100644 --- a/tests/scripts/harness-audit.test.js +++ b/tests/scripts/harness-audit.test.js @@ -933,8 +933,12 @@ function runTests() { ); } + // findPluginInstall also reads USERPROFILE and os.homedir() (USERPROFILE + // on Windows), so isolating HOME alone lets a real install win. const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; process.env.HOME = homeDir; + process.env.USERPROFILE = homeDir; try { const found = findPluginInstall(projectRoot); assert.ok(found); @@ -945,6 +949,11 @@ function runTests() { } else { process.env.HOME = originalHome; } + if (originalUserProfile === undefined) { + delete process.env.USERPROFILE; + } else { + process.env.USERPROFILE = originalUserProfile; + } } } finally { cleanup(homeDir); diff --git a/tests/scripts/manual-claude-namespace.test.js b/tests/scripts/manual-claude-namespace.test.js new file mode 100644 index 000000000..5ec2101bd --- /dev/null +++ b/tests/scripts/manual-claude-namespace.test.js @@ -0,0 +1,77 @@ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const cli = path.resolve(__dirname, '../../scripts/ecc.js'); +let passed = 0; +let failed = 0; + +for (const target of ['claude', 'claude-project']) { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-native-names-'))); + const home = path.join(root, 'home'); + const project = path.join(root, 'project'); + fs.mkdirSync(home); + fs.mkdirSync(project); + const env = { + ...process.env, + HOME: home, + USERPROFILE: home, + CLAUDE_CONFIG_DIR: path.join(home, '.claude'), + ECC_AGENT_DATA_HOME: path.join(home, '.claude'), + XDG_CONFIG_HOME: path.join(home, '.config'), + OPENCODE_CONFIG_DIR: path.join(home, '.config/opencode'), + CODEX_HOME: path.join(home, '.codex'), + ECC_DRY_RUN: '0', + }; + const installRoot = path.join(target === 'claude' ? home : project, '.claude'); + const rule = path.join(installRoot, 'rules/ecc/common/agents.md'); + const run = args => { + const result = spawnSync(process.execPath, [cli, ...args], { + cwd: project, env, encoding: 'utf8', timeout: 45000, + }); + assert.ifError(result.error); + assert.strictEqual(result.status, 0, result.stderr || result.stdout); + return result; + }; + const assertNativeNames = () => { + assert.match(fs.readFileSync(rule, 'utf8'), /Agent\(subagent_type: "planner"/); + assert.doesNotMatch(fs.readFileSync(rule, 'utf8'), /Agent\(subagent_type: "ecc:planner"/); + assert.ok(fs.existsSync(path.join(installRoot, 'agents/planner.md'))); + assert.ok(fs.existsSync(path.join(installRoot, 'skills/tdd-workflow/SKILL.md'))); + }; + try { + const installArgs = ['install', '--target', target, '--modules', 'rules-core', + '--with', 'agent:planner', '--with', 'skill:tdd-workflow', '--no-hooks']; + run(installArgs); + assertNativeNames(); + const report = JSON.parse(run(['doctor', '--target', target, '--json']).stdout); + assert.strictEqual(report.summary.errorCount, 0); + assert.strictEqual(report.summary.warningCount, 0); + fs.appendFileSync(rule, '\nlocal drift\n'); + run(['repair', '--target', target, '--json']); + assertNativeNames(); + assert.doesNotMatch(fs.readFileSync(rule, 'utf8'), /local drift/); + run(installArgs); + assertNativeNames(); + const userFile = path.join(installRoot, 'user-note.txt'); + fs.writeFileSync(userFile, 'keep this user file\n'); + run(['uninstall', '--target', target, '--json']); + assert.ok(!fs.existsSync(rule)); + assert.strictEqual(fs.readFileSync(userFile, 'utf8'), 'keep this user file\n'); + passed++; + console.log(` PASS ${target} install, doctor, repair, reinstall and uninstall preserve native names`); + } catch (error) { + failed++; + console.error(` FAIL ${target}: ${error.stack}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +console.log(`Passed: ${passed}`); +console.log(`Failed: ${failed}`); +process.exitCode = failed ? 1 : 0; diff --git a/tests/scripts/npm-publish-surface.test.js b/tests/scripts/npm-publish-surface.test.js index b911cb9d2..5d57cbac1 100644 --- a/tests/scripts/npm-publish-surface.test.js +++ b/tests/scripts/npm-publish-surface.test.js @@ -259,6 +259,7 @@ function main() { "scripts/lib/nasiko-release.js", "scripts/lib/memory-vault-format.js", "scripts/lib/memory-vault.js", + "scripts/lib/hook-session.js", "scripts/discussion-audit.js", "scripts/operator-readiness-dashboard.js", "scripts/preview-pack-smoke.js", diff --git a/tests/scripts/setup.test.js b/tests/scripts/setup.test.js index babc6d946..715d383c2 100644 --- a/tests/scripts/setup.test.js +++ b/tests/scripts/setup.test.js @@ -5,6 +5,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); +const { withoutShadowingClaude } = require('../lib/helpers/windows-test-env'); const repoRoot = path.join(__dirname, '..', '..'); const setupScript = path.join(repoRoot, 'scripts', 'setup.js'); const eccScript = path.join(repoRoot, 'scripts', 'ecc.js'); @@ -62,7 +63,7 @@ function runSetup(fixture, args, options = {}) { HOME: fixture.homeDir, USERPROFILE: fixture.homeDir, CLAUDE_CONFIG_DIR: fixture.configDir, - PATH: options.path || `${fixture.binDir}${path.delimiter}${process.env.PATH || ''}`, + PATH: options.path || `${fixture.binDir}${path.delimiter}${withoutShadowingClaude(process.env.PATH)}`, ECC_TEST_CLAUDE_STATE: fixture.statePath, ECC_TEST_CLAUDE_CALLS: fixture.callsPath, ...options.env, @@ -147,7 +148,7 @@ function runInteractiveEccSetup(fixture, options = {}) { HOME: fixture.homeDir, USERPROFILE: fixture.homeDir, CLAUDE_CONFIG_DIR: fixture.configDir, - PATH: `${fixture.binDir}${path.delimiter}${process.env.PATH || ''}`, + PATH: `${fixture.binDir}${path.delimiter}${withoutShadowingClaude(process.env.PATH)}`, ECC_TEST_CLAUDE_STATE: fixture.statePath, ECC_TEST_CLAUDE_CALLS: fixture.callsPath, },