diff --git a/README.md b/README.md index 00d3a8ca7..2431bd418 100644 --- a/README.md +++ b/README.md @@ -555,7 +555,10 @@ Do not copy the raw repo `hooks/hooks.json` into `~/.claude/settings.json` or `~ bash ./install.sh --target claude --modules hooks-runtime --enable-hooks ``` -That writes resolved hooks to `~/.claude/hooks/hooks.json` and leaves any existing `~/.claude/settings.json` untouched. +That installs the hook scripts under `~/.claude/` and registers the resolved +hook entries in `~/.claude/settings.json`. Existing user settings and hooks are +preserved; ECC-owned entries are tracked by stable ID for idempotent updates +and safe uninstall. If you installed ECC via `/plugin install`, do not copy those hooks into `settings.json`. Claude Code v2.1+ already auto-loads plugin `hooks/hooks.json`, and duplicating them in `settings.json` causes duplicate execution and cross-platform hook conflicts. diff --git a/hooks/README.md b/hooks/README.md index 510dfa755..e540b2d24 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -33,7 +33,11 @@ bash ./install.sh --target claude --modules hooks-runtime --enable-hooks pwsh -File .\install.ps1 --target claude --modules hooks-runtime --enable-hooks ``` -That installs resolved hooks to `~/.claude/hooks/hooks.json`. On Windows, the Claude config root is `%USERPROFILE%\\.claude`. +That installs the hook scripts under `~/.claude/` and registers the resolved +hook entries in `~/.claude/settings.json`. Existing user settings and hook +entries are preserved, while ECC-owned entries are tracked by stable ID for +idempotent updates and safe uninstall. On Windows, the Claude config root is +`%USERPROFILE%\\.claude`. ### PreToolUse Hooks diff --git a/schemas/hooks.schema.json b/schemas/hooks.schema.json index 4d1192973..c325d9712 100644 --- a/schemas/hooks.schema.json +++ b/schemas/hooks.schema.json @@ -122,6 +122,11 @@ "hooks" ], "properties": { + "id": { + "type": "string", + "pattern": "\\S", + "description": "Stable identifier for a matcher entry. Required and globally unique in wrapped object format." + }, "matcher": { "oneOf": [ { diff --git a/schemas/install-state.schema.json b/schemas/install-state.schema.json index 976d5129b..9b827e144 100644 --- a/schemas/install-state.schema.json +++ b/schemas/install-state.schema.json @@ -213,9 +213,92 @@ "contentSha256": { "type": "string", "pattern": "^[a-fA-F0-9]{64}$" + }, + "managedHooks": { + "type": "object", + "minProperties": 1, + "propertyNames": { + "enum": [ + "SessionStart", + "UserPromptSubmit", + "PreToolUse", + "PermissionRequest", + "PostToolUse", + "PostToolUseFailure", + "Notification", + "SubagentStart", + "Stop", + "SubagentStop", + "PreCompact", + "InstructionsLoaded", + "TeammateIdle", + "TaskCompleted", + "ConfigChange", + "WorktreeCreate", + "WorktreeRemove", + "SessionEnd" + ] + }, + "additionalProperties": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "required": ["id", "hooks"], + "properties": { + "id": { + "type": "string", + "pattern": "\\S" + } + } + } + } + } + }, + "allOf": [ + { + "if": { + "properties": { + "kind": { "const": "update-claude-settings" } + } + }, + "then": { + "required": ["managedHooks"], + "properties": { + "moduleId": { "const": "hooks-runtime" }, + "sourceRelativePath": { "const": "hooks/hooks.json" } + } + } + } + ] + } + } + }, + "allOf": [ + { + "if": { + "properties": { + "operations": { + "contains": { + "type": "object", + "properties": { + "kind": { "const": "update-claude-settings" } + }, + "required": ["kind"] + } + } + } + }, + "then": { + "properties": { + "target": { + "properties": { + "target": { "enum": ["claude", "claude-project"] } + }, + "required": ["target"] } } } } - } + ] } diff --git a/scripts/ci/validate-hooks.js b/scripts/ci/validate-hooks.js index bc1da8020..779555a44 100644 --- a/scripts/ci/validate-hooks.js +++ b/scripts/ci/validate-hooks.js @@ -154,8 +154,17 @@ function validateHooks() { // Support both object format { hooks: {...} } and array format const hooks = data.hooks || data; + const requiresStableIds = Boolean( + data + && typeof data === 'object' + && !Array.isArray(data) + && data.hooks + && typeof data.hooks === 'object' + && !Array.isArray(data.hooks) + ); let hasErrors = false; let totalMatchers = 0; + const matcherIdLocations = new Map(); if (typeof hooks === 'object' && !Array.isArray(hooks)) { // Object format: { EventType: [matchers] } @@ -179,20 +188,32 @@ function validateHooks() { hasErrors = true; continue; } + const matcherLabel = `${eventType}[${i}]`; + if (requiresStableIds && !isNonEmptyString(matcher.id)) { + console.error(`ERROR: ${matcherLabel} missing or invalid 'id' field`); + hasErrors = true; + } else if (requiresStableIds && matcherIdLocations.has(matcher.id)) { + console.error( + `ERROR: ${matcherLabel} has duplicate id '${matcher.id}' (already used by ${matcherIdLocations.get(matcher.id)})` + ); + hasErrors = true; + } else if (requiresStableIds) { + matcherIdLocations.set(matcher.id, matcherLabel); + } if (!('matcher' in matcher) && !EVENTS_WITHOUT_MATCHER.has(eventType)) { - console.error(`ERROR: ${eventType}[${i}] missing 'matcher' field`); + console.error(`ERROR: ${matcherLabel} missing 'matcher' field`); hasErrors = true; } else if ('matcher' in matcher && typeof matcher.matcher !== 'string' && (typeof matcher.matcher !== 'object' || matcher.matcher === null)) { - console.error(`ERROR: ${eventType}[${i}] has invalid 'matcher' field`); + console.error(`ERROR: ${matcherLabel} has invalid 'matcher' field`); hasErrors = true; } if (!matcher.hooks || !Array.isArray(matcher.hooks)) { - console.error(`ERROR: ${eventType}[${i}] missing 'hooks' array`); + console.error(`ERROR: ${matcherLabel} missing 'hooks' array`); hasErrors = true; } else { // Validate each hook entry for (let j = 0; j < matcher.hooks.length; j++) { - if (validateHookEntry(matcher.hooks[j], `${eventType}[${i}].hooks[${j}]`)) { + if (validateHookEntry(matcher.hooks[j], `${matcherLabel}.hooks[${j}]`)) { hasErrors = true; } } diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index c10b1cfe3..c5ece3504 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -20,6 +20,15 @@ const { getLegacyOpencodeLocation, inspectLegacyOpencodeState, } = require('./install/opencode-legacy-migration'); +const { + acquireSettingsLock, + inspectManagedHooks, + materializeManagedHooks, + repairManagedHooks, + uninstallManagedHooks, + updateSettingsAtomic, + validateManagedHooks, +} = require('./install/claude-settings'); const { adaptAntigravityAgent } = require('./install/antigravity-agent'); const { buildInstallIndex, rewriteRelativeLinks } = require('./install/link-rewrite'); const { getInstallTargetAdapter, listInstallTargetAdapters } = require('./install-targets/registry'); @@ -523,6 +532,24 @@ function readJsonNoFollow(filePath) { return JSON.parse(readFileNoFollow(filePath, 'utf8')); } +function expectedClaudeSettingsPath(targetRoot) { + return path.join(targetRoot, 'settings.json'); +} + +function assertClaudeSettingsDestination(operation, trustedRoot, target = null) { + if (target && target !== 'claude' && target !== 'claude-project') { + throw new Error('Refusing to manage Claude hooks for a non-Claude target.'); + } + if (path.resolve(operation.destinationPath) !== path.resolve( + expectedClaudeSettingsPath(trustedRoot) + )) { + throw new Error( + `Refusing to manage Claude hooks outside the canonical settings file: ` + + `${operation.destinationPath}` + ); + } +} + function writeContainedFile(destinationPath, content, trustedRoot, action, mode) { const preparedDestination = prepareContainedWriteDestination(destinationPath, trustedRoot, action); const finalDestination = getManagedDestination( @@ -689,6 +716,24 @@ function deepRemoveJsonSubset(currentValue, managedValue) { function hydrateRecordedOperations(repoRoot, operations) { return operations.map(operation => { + if (operation.kind === 'update-claude-settings') { + const sourcePath = resolveOperationSourcePath(repoRoot, operation); + if (!sourcePath || !fs.existsSync(sourcePath)) { + throw new Error( + `Missing source file for repair: ${sourcePath || operation.sourceRelativePath}` + ); + } + return { + ...operation, + sourcePath, + previousManagedHooks: operation.managedHooks, + managedHooks: materializeManagedHooks( + readJsonNoFollow(sourcePath), + path.dirname(operation.destinationPath) + ), + }; + } + if (operation.kind !== 'copy-file') { return { ...operation }; } @@ -717,7 +762,14 @@ function shouldRepairFromRecordedOperations(state) { return getManagedOperations(state).some(operation => operation.kind !== 'copy-file'); } -function executeRepairOperation(repoRoot, operation, trustedRoot, linkIndex = null) { +function executeRepairOperation( + repoRoot, + operation, + trustedRoot, + linkIndex = null, + target = null, + settingsLockHeld = false +) { // Install-state is attacker-controllable; never write/delete outside the // adapter-derived trusted root, regardless of what the state file claims // (GHSA-hfpv-w6mp-5g95). @@ -770,6 +822,35 @@ function executeRepairOperation(repoRoot, operation, trustedRoot, linkIndex = nu return operation.destinationPath; } + if (operation.kind === 'update-claude-settings') { + assertClaudeSettingsDestination(operation, trustedRoot, target); + const managedHooks = validateManagedHooks(operation.managedHooks); + const previousManagedHooks = operation.previousManagedHooks + ? validateManagedHooks(operation.previousManagedHooks, 'previous managed hooks') + : null; + const existingDestination = getContainedExistingPath( + operation.destinationPath, + trustedRoot, + 'repair' + ); + const settingsPath = existingDestination + ? getManagedDestination(existingDestination, trustedRoot, 'repair').managedPath + : prepareContainedWriteDestination(operation.destinationPath, trustedRoot, 'repair'); + updateSettingsAtomic( + settingsPath, + currentSettings => repairManagedHooks(currentSettings, managedHooks, { + previousManagedHooks, + }), + { + lockHeld: settingsLockHeld, + beforeCommit() { + getManagedDestination(settingsPath, trustedRoot, 'repair'); + }, + } + ); + return operation.destinationPath; + } + if (operation.kind === 'remove') { const removedPath = removeContainedPath( operation.destinationPath, @@ -938,6 +1019,45 @@ function executeUninstallOperation(operation, trustedRoot, options = {}) { }; } + if (operation.kind === 'update-claude-settings') { + assertClaudeSettingsDestination(operation, trustedRoot, options.target); + const existingDestination = getContainedExistingPath( + operation.destinationPath, + trustedRoot, + 'uninstall' + ); + if (!existingDestination) { + return { + removedPaths: [], + cleanupTargets: [] + }; + } + + const settingsPath = getManagedDestination( + existingDestination, + trustedRoot, + 'uninstall' + ).managedPath; + const uninstalled = updateSettingsAtomic( + settingsPath, + currentSettings => uninstallManagedHooks(currentSettings, operation.managedHooks), + { + lockHeld: Boolean(options.settingsLockHeld), + beforeCommit() { + getManagedDestination(settingsPath, trustedRoot, 'uninstall'); + }, + } + ); + + return { + removedPaths: [], + cleanupTargets: [], + retainedPaths: uninstalled.retained.length > 0 + ? [operation.destinationPath] + : [] + }; + } + if (operation.kind === 'remove') { const previousContent = getOperationPreviousContent(operation); if (previousContent !== null) { @@ -966,7 +1086,7 @@ function executeUninstallOperation(operation, trustedRoot, options = {}) { throw new Error(`Unsupported uninstall operation kind: ${operation.kind}`); } -function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = null) { +function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = null, target = null) { const destinationPath = operation.destinationPath; if (!destinationPath) { return { @@ -1147,6 +1267,48 @@ function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = n }; } + if (operation.kind === 'update-claude-settings') { + try { + assertClaudeSettingsDestination(operation, trustedRoot, target); + } catch (_error) { + return { + status: 'unsafe-destination', + operation, + destinationPath, + reason: 'non-canonical-claude-settings' + }; + } + let managedHooks; + try { + managedHooks = validateManagedHooks(operation.managedHooks); + } catch (_error) { + return { + status: 'unverified', + operation, + destinationPath + }; + } + + try { + const inspection = inspectManagedHooks( + readJsonNoFollow(inspectedPath), + managedHooks + ); + return { + status: inspection.status, + operation, + destinationPath, + managedHookInspection: inspection + }; + } catch (_error) { + return { + status: 'drifted', + operation, + destinationPath + }; + } + } + return { status: 'unverified', operation, @@ -1154,11 +1316,17 @@ function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = n }; } -function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations) { +function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations, target = null) { const linkIndex = buildLinkIndexForOperations(operations, trustedRoot); return operations.reduce( (summary, operation) => { - const inspection = inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex); + const inspection = inspectManagedOperation( + repoRoot, + trustedRoot, + operation, + linkIndex, + target + ); if (inspection.status === 'missing') { summary.missing.push(inspection); } else if (inspection.status === 'drifted') { @@ -1185,6 +1353,12 @@ function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations) { ); } +function hookRepairOperations(operationHealth) { + return operationHealth.drifted + .filter(entry => entry.operation.kind === 'update-claude-settings') + .map(entry => ({ ...entry.operation })); +} + function getUnsafeManagedDestinationError(operationHealth) { const hasFinalSymlink = operationHealth.unsafeDestination.some( inspection => inspection.reason === 'final-symlink' @@ -1467,7 +1641,8 @@ function analyzeRecord(record, context) { const operationHealth = summarizeManagedOperationHealth( context.repoRoot, record.targetRoot, - managedOperations + managedOperations, + record.adapter.target ); const missingManagedOperations = operationHealth.missing; @@ -1757,7 +1932,18 @@ function repairInstalledStates(options = {}) { }; } + let releaseSettingsLock = null; try { + if ( + !options.dryRun + && getManagedOperations(record.state || {}).some( + operation => operation.kind === 'update-claude-settings' + ) + ) { + releaseSettingsLock = acquireSettingsLock( + path.join(record.targetRoot, 'settings.json') + ); + } const needsOpencodeBuild = record.adapter.target === 'opencode' && hasOpencodeBuildError(getOpencodeBuildValidationIssues(context)); const opencodeBuildRepairPath = path.join(context.repoRoot, OPENCODE_BUILD_ARTIFACT); @@ -1829,7 +2015,8 @@ function repairInstalledStates(options = {}) { const operationHealth = summarizeManagedOperationHealth( context.repoRoot, record.targetRoot, - desiredPlan.operations + desiredPlan.operations, + record.adapter.target ); const unsafeOperationResult = getUnsafeOperationResult( record, @@ -1876,7 +2063,8 @@ function repairInstalledStates(options = {}) { const operationHealth = summarizeManagedOperationHealth( context.repoRoot, record.targetRoot, - desiredPlan.operations + desiredPlan.operations, + record.adapter.target ); const unsafeOperationResult = getUnsafeOperationResult( @@ -1899,7 +2087,23 @@ function repairInstalledStates(options = {}) { }; } - const repairOperations = [...operationHealth.missing.map(entry => ({ ...entry.operation })), ...operationHealth.drifted.map(entry => ({ ...entry.operation }))]; + const repairOperations = [ + ...operationHealth.missing.map(entry => ({ ...entry.operation })), + ...operationHealth.drifted.map(entry => ({ ...entry.operation })), + ...hookRepairOperations({ + drifted: desiredPlan.operations + .filter(operation => ( + operation.kind === 'update-claude-settings' + && operation.previousManagedHooks + && JSON.stringify(operation.previousManagedHooks) + !== JSON.stringify(operation.managedHooks) + )) + .map(operation => ({ operation })), + }), + ].filter((operation, index, items) => items.findIndex(candidate => ( + candidate.kind === operation.kind + && candidate.destinationPath === operation.destinationPath + )) === index); const repairLinkIndex = buildLinkIndexForOperations(desiredPlan.operations, record.targetRoot); const legacyMigrationPaths = migration.legacyOperationsToRemove.map( operation => operation.destinationPath @@ -1934,7 +2138,9 @@ function repairInstalledStates(options = {}) { context.repoRoot, operation, record.targetRoot, - repairLinkIndex + repairLinkIndex, + record.adapter.target, + Boolean(releaseSettingsLock) ); if (repairedPath) { repairedPaths.push(repairedPath); @@ -1986,6 +2192,8 @@ function repairInstalledStates(options = {}) { plannedRepairs: [], error: error.message }; + } finally { + if (releaseSettingsLock) releaseSettingsLock(); } }); @@ -2100,15 +2308,23 @@ function uninstallInstalledStates(options = {}) { }; } + let releaseSettingsLock = null; try { const removedPaths = []; const cleanupTargets = []; const retainedPaths = []; const operations = getManagedOperations(state); + if (operations.some(operation => operation.kind === 'update-claude-settings')) { + releaseSettingsLock = acquireSettingsLock( + path.join(record.targetRoot, 'settings.json') + ); + } for (const operation of operations) { const outcome = executeUninstallOperation(operation, record.targetRoot, { preserveDriftedCopies: true, + target: record.adapter.target, + settingsLockHeld: Boolean(releaseSettingsLock), }); removedPaths.push(...outcome.removedPaths); cleanupTargets.push(...outcome.cleanupTargets); @@ -2153,6 +2369,8 @@ function uninstallInstalledStates(options = {}) { plannedRemovals, error: error.message }; + } finally { + if (releaseSettingsLock) releaseSettingsLock(); } }); diff --git a/scripts/lib/install-state.js b/scripts/lib/install-state.js index a0aa3bbe6..805943f92 100644 --- a/scripts/lib/install-state.js +++ b/scripts/lib/install-state.js @@ -1,5 +1,6 @@ const fs = require('fs'); const path = require('path'); +const { validateManagedHooks } = require('./install/claude-settings'); // Dependency-free, self-contained validation. The installer closure must not // require any non-builtin package (enterprise supply-chain vetting: the vetted @@ -209,6 +210,38 @@ function createFallbackValidator() { ) { pushError(`${instancePath}/contentSha256`, 'must be a SHA-256 hex digest'); } + if (operation.kind === 'update-claude-settings') { + if (!['claude', 'claude-project'].includes(state.target && state.target.target)) { + pushError(`${instancePath}/kind`, 'is only valid for Claude targets'); + } + if (operation.moduleId !== 'hooks-runtime') { + pushError(`${instancePath}/moduleId`, 'must equal hooks-runtime'); + } + if (String(operation.sourceRelativePath).replace(/\\/g, '/') !== 'hooks/hooks.json') { + pushError(`${instancePath}/sourceRelativePath`, 'must equal hooks/hooks.json'); + } + if ( + isNonEmptyString(state.target && state.target.root) + && isNonEmptyString(operation.destinationPath) + ) { + const expectedDestination = path.resolve(state.target.root, 'settings.json'); + const actualDestination = path.resolve(operation.destinationPath); + const pathsMatch = process.platform === 'win32' + ? expectedDestination.toLowerCase() === actualDestination.toLowerCase() + : expectedDestination === actualDestination; + if (!pathsMatch) { + pushError( + `${instancePath}/destinationPath`, + 'must equal the canonical Claude settings path' + ); + } + } + try { + validateManagedHooks(operation.managedHooks); + } catch (error) { + pushError(`${instancePath}/managedHooks`, error.message); + } + } } } diff --git a/scripts/lib/install-targets/claude-home.js b/scripts/lib/install-targets/claude-home.js index 3729b50c8..0ff84a160 100644 --- a/scripts/lib/install-targets/claude-home.js +++ b/scripts/lib/install-targets/claude-home.js @@ -1,3 +1,4 @@ +const fs = require('fs'); const path = require('path'); const { @@ -8,6 +9,39 @@ const { } = require('./helpers'); const CLAUDE_ECC_NAMESPACE = 'ecc'; +const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; + +function planClaudeHooksOperations(adapter, module, input) { + const sourceHooksRoot = path.join(input.repoRoot || '', 'hooks'); + const operations = [ + createRemappedOperation( + adapter, + module.id, + CLAUDE_HOOKS_CONFIG_PATH, + path.join(adapter.resolveRoot(input), 'settings.json'), + { + kind: 'update-claude-settings', + strategy: 'merge-hook-ids', + } + ), + ]; + + if (!input.repoRoot || !fs.existsSync(sourceHooksRoot)) { + return operations; + } + + return [ + ...operations, + ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) + .filter(entry => entry.name !== 'hooks.json') + .sort((left, right) => left.name.localeCompare(right.name)) + .map(entry => adapter.createScaffoldOperation( + module.id, + path.join('hooks', entry.name), + input + )), + ]; +} function getClaudeManagedDestinationPath(adapter, sourceRelativePath, input) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); @@ -66,7 +100,14 @@ module.exports = createInstallTargetAdapter({ const paths = Array.isArray(module.paths) ? module.paths : []; return paths .filter(p => !isForeignPlatformPath(p, adapter.target)) - .map(sourceRelativePath => { + .flatMap(sourceRelativePath => { + if ( + module.id === 'hooks-runtime' + && normalizeRelativePath(sourceRelativePath) === 'hooks' + ) { + return planClaudeHooksOperations(adapter, module, planningInput); + } + const managedDestinationPath = getClaudeManagedDestinationPath( adapter, sourceRelativePath, @@ -74,16 +115,16 @@ module.exports = createInstallTargetAdapter({ ); if (managedDestinationPath) { - return createRemappedOperation( + return [createRemappedOperation( adapter, module.id, sourceRelativePath, managedDestinationPath, { strategy: 'preserve-relative-path' } - ); + )]; } - return adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput); + return [adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput)]; }); }); }, diff --git a/scripts/lib/install-targets/claude-project.js b/scripts/lib/install-targets/claude-project.js index 051b0ae26..4c5f23a32 100644 --- a/scripts/lib/install-targets/claude-project.js +++ b/scripts/lib/install-targets/claude-project.js @@ -1,3 +1,4 @@ +const fs = require('fs'); const path = require('path'); const { @@ -8,6 +9,39 @@ const { } = require('./helpers'); const CLAUDE_ECC_NAMESPACE = 'ecc'; +const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; + +function planClaudeHooksOperations(adapter, module, input) { + const sourceHooksRoot = path.join(input.repoRoot || '', 'hooks'); + const operations = [ + createRemappedOperation( + adapter, + module.id, + CLAUDE_HOOKS_CONFIG_PATH, + path.join(adapter.resolveRoot(input), 'settings.json'), + { + kind: 'update-claude-settings', + strategy: 'merge-hook-ids', + } + ), + ]; + + if (!input.repoRoot || !fs.existsSync(sourceHooksRoot)) { + return operations; + } + + return [ + ...operations, + ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) + .filter(entry => entry.name !== 'hooks.json') + .sort((left, right) => left.name.localeCompare(right.name)) + .map(entry => adapter.createScaffoldOperation( + module.id, + path.join('hooks', entry.name), + input + )), + ]; +} function getClaudeManagedDestinationPath(adapter, sourceRelativePath, input) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); @@ -66,7 +100,14 @@ module.exports = createInstallTargetAdapter({ const paths = Array.isArray(module.paths) ? module.paths : []; return paths .filter(p => !isForeignPlatformPath(p, 'claude')) - .map(sourceRelativePath => { + .flatMap(sourceRelativePath => { + if ( + module.id === 'hooks-runtime' + && normalizeRelativePath(sourceRelativePath) === 'hooks' + ) { + return planClaudeHooksOperations(adapter, module, planningInput); + } + const managedDestinationPath = getClaudeManagedDestinationPath( adapter, sourceRelativePath, @@ -74,16 +115,16 @@ module.exports = createInstallTargetAdapter({ ); if (managedDestinationPath) { - return createRemappedOperation( + return [createRemappedOperation( adapter, module.id, sourceRelativePath, managedDestinationPath, { strategy: 'preserve-relative-path' } - ); + )]; } - return adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput); + return [adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput)]; }); }); }, diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index e755586a8..1c5d4900d 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -8,8 +8,16 @@ const { hasExplicitCommitAttributionPreference, withCommitAttributionDisabled, } = require('../claude-commit-attribution'); -const { writeInstallState } = require('../install-state'); +const { readInstallState, writeInstallState } = require('../install-state'); const { assertHookConsentReady, planMaterializesHookRuntime } = require('./hook-consent'); +const { + acquireSettingsLock, + mergeManagedHooks, + readSettings, + uninstallManagedHooks, + updateSettingsAtomic, + validateManagedHooks, +} = require('./claude-settings'); const { filterMcpConfig, parseDisabledMcpServers } = require('../mcp-config'); const { assertWithinTrustedRoot } = require('../path-safety'); const { @@ -200,69 +208,21 @@ function shouldSetClaudeCommitAttributionPreference(plan) { }); } -function writeClaudeCommitAttributionPreference(settingsPath) { - // Read once rather than probing with existsSync first. Checking for the file and - // then writing it is a file system race (CodeQL js/file-system-race), and a - // missing file is simply the fresh-install case. - let settings; +function writeClaudeCommitAttributionPreference(settingsPath, options = {}) { try { - settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); - } catch (error) { - if (error.code !== 'ENOENT') { - // Unreadable or malformed settings belong to the user; leave them untouched. - return false; - } - settings = {}; - } - - if (!settings || typeof settings !== 'object' || Array.isArray(settings)) { + let changed = false; + updateSettingsAtomic(settingsPath, settings => { + if (hasExplicitCommitAttributionPreference(settings)) { + return { settings }; + } + changed = true; + return { settings: withCommitAttributionDisabled(settings) }; + }, options); + return changed; + } catch (_error) { + // Unreadable or malformed settings belong to the user; leave them untouched. return false; } - - if (hasExplicitCommitAttributionPreference(settings)) { - return false; - } - - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - fs.writeFileSync( - settingsPath, - formatJson(withCommitAttributionDisabled(settings)), - 'utf8' - ); - return true; -} - -function replacePluginRootPlaceholders(value, pluginRoot) { - if (!pluginRoot) { - return value; - } - - if (typeof value === 'string') { - return value.split('${CLAUDE_PLUGIN_ROOT}').join(pluginRoot); - } - - if (Array.isArray(value)) { - return value.map(item => replacePluginRootPlaceholders(item, pluginRoot)); - } - - if (value && typeof value === 'object') { - return Object.fromEntries( - Object.entries(value).map(([key, nestedValue]) => [ - key, - replacePluginRootPlaceholders(nestedValue, pluginRoot), - ]) - ); - } - - return value; -} - -function findHooksOperation(plan, hooksDestinationPath) { - return plan.operations.find(item => ( - item.destinationPath === hooksDestinationPath - && item.moduleId === 'hooks-runtime' - && typeof item.sourcePath === 'string' - )); } function isMcpConfigPath(filePath) { @@ -302,40 +262,135 @@ function assertSafeInstallOperation(plan, operation) { } } -function buildResolvedClaudeHooks(plan) { - if (!plan.adapter || (plan.adapter.target !== 'claude' && plan.adapter.target !== 'claude-project')) { +function readPreviousInstallState(plan) { + if (!fs.existsSync(plan.installStatePath)) { + return null; + } + return readInstallState(plan.installStatePath); +} + +function comparablePath(filePath) { + const resolved = path.resolve(filePath); + return process.platform === 'win32' ? resolved.toLowerCase() : resolved; +} + +function findPreviousManagedHooks(previousState, plan, operation) { + if ( + !previousState + || previousState.target.id !== plan.adapter.id + || comparablePath(previousState.target.root) !== comparablePath(plan.targetRoot) + || comparablePath(previousState.target.installStatePath) !== comparablePath(plan.installStatePath) + ) { return null; } - const pluginRoot = plan.targetRoot; - const hooksDestinationPath = path.join(plan.targetRoot, 'hooks', 'hooks.json'); - const hooksOperation = findHooksOperation(plan, hooksDestinationPath); - if (!hooksOperation) { - return null; - } - const hooksSourcePath = hooksOperation.sourcePath; - if (!fs.existsSync(hooksSourcePath)) { + const previousOperation = (previousState.operations || []).find(candidate => ( + candidate.kind === operation.kind + && candidate.destinationPath === operation.destinationPath + )); + if (!previousOperation || !previousOperation.managedHooks) { return null; } - const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config'); - const resolvedHooks = replacePluginRootPlaceholders(hooksConfig.hooks, pluginRoot); - if (!resolvedHooks || typeof resolvedHooks !== 'object' || Array.isArray(resolvedHooks)) { - throw new Error(`Invalid hooks config at ${hooksSourcePath}: expected "hooks" to be a JSON object`); + return validateManagedHooks( + previousOperation.managedHooks, + 'previous managed hooks' + ); +} + +function preflightClaudeSettingsOperations(plan) { + const settingsOperations = plan.operations.filter(operation => ( + operation.kind === 'update-claude-settings' + || operation.kind === 'remove-claude-settings-hooks' + )); + if (settingsOperations.length === 0) { + return new Map(); } + const previousState = readPreviousInstallState(plan); + return new Map(settingsOperations.map(operation => { + assertSafeInstallOperation(plan, operation); + const managedHooks = validateManagedHooks(operation.managedHooks); + const settings = readSettings(operation.destinationPath); + const previousManagedHooks = findPreviousManagedHooks(previousState, plan, operation); + if (operation.kind === 'remove-claude-settings-hooks') { + const removal = uninstallManagedHooks(settings, managedHooks); + if (removal.retained.length > 0) { + throw new Error( + `Refusing to disable modified Claude hooks in ${operation.destinationPath}; ` + + 'run the ECC uninstaller to review retained entries.' + ); + } + } else { + mergeManagedHooks(settings, managedHooks, { previousManagedHooks }); + } + return [operation, { managedHooks, previousManagedHooks }]; + })); +} + +function prepareHookConsentMigration(plan, migration) { + if (plan.hookConsent !== 'declined') { + return migration; + } + const previousState = readPreviousInstallState(plan); + if (!previousState) { + return migration; + } + + const removals = (previousState.operations || []) + .filter(operation => operation.kind === 'update-claude-settings') + .map(operation => ({ + ...operation, + kind: 'remove-claude-settings-hooks', + strategy: 'remove-hook-ids', + scaffoldOnly: false, + })); + if (removals.length === 0) { + return migration; + } + const removalDestinations = new Set(removals.map(operation => comparablePath( + operation.destinationPath + ))); return { - hooksOperation, - hooksDestinationPath, - resolvedHooksConfig: { - ...hooksConfig, - hooks: resolvedHooks, + ...migration, + // Disable hooks only after every ordinary install operation succeeds so a + // partial reinstall cannot silently revoke working hooks before failing. + appliedOperations: [...migration.appliedOperations, ...removals], + finalState: { + ...migration.finalState, + operations: migration.finalState.operations.filter(operation => !( + operation.kind === 'update-claude-settings' + && removalDestinations.has(comparablePath(operation.destinationPath)) + )), }, + bridgeState: { + ...migration.bridgeState, + request: { + ...migration.bridgeState.request, + hookConsent: 'enabled', + }, + resolution: { + ...migration.bridgeState.resolution, + selectedModules: [...new Set([ + ...migration.bridgeState.resolution.selectedModules, + 'hooks-runtime', + ])], + }, + }, + requiresBridgeState: true, }; } function previewInstallPlan(plan) { - const migration = prepareClaudeSkillMigration(plan); + const migration = prepareHookConsentMigration( + plan, + prepareClaudeSkillMigration(plan) + ); + const appliedPlan = { + ...plan, + operations: migration.appliedOperations, + }; + preflightClaudeSettingsOperations(appliedPlan); const hookConsentWarnings = planMaterializesHookRuntime(plan) && plan.hookConsent !== 'enabled' ? ['Applying this plan requires an explicit hook decision: --enable-hooks or --no-hooks.'] : []; @@ -356,6 +411,25 @@ function previewInstallPlan(plan) { function applyInstallPlan(plan, dependencies = {}) { assertHookConsentReady(plan); + const isClaudeManualTarget = plan.adapter + && (plan.adapter.target === 'claude' || plan.adapter.target === 'claude-project'); + const settingsPathToLock = isClaudeManualTarget + ? path.join(plan.targetRoot, 'settings.json') + : null; + if (settingsPathToLock) { + assertSafeInstallOperation(plan, { destinationPath: settingsPathToLock }); + } + const releaseSettingsLock = settingsPathToLock + ? acquireSettingsLock(settingsPathToLock) + : null; + try { + return applyInstallPlanLocked(plan, dependencies, Boolean(releaseSettingsLock)); + } finally { + if (releaseSettingsLock) releaseSettingsLock(); + } +} + +function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = false) { const persistInstallState = dependencies.writeInstallState || writeInstallState; const beforeInstallStateRead = dependencies.beforeInstallStateRead; const beforeOperationWrite = dependencies.beforeOperationWrite; @@ -363,30 +437,36 @@ function applyInstallPlan(plan, dependencies = {}) { if (typeof beforeInstallStateRead === 'function') { beforeInstallStateRead({ plan }); } - const migration = prepareClaudeSkillMigration(plan); + const migration = prepareHookConsentMigration( + plan, + prepareClaudeSkillMigration(plan) + ); const appliedPlan = { ...plan, operations: migration.appliedOperations, }; - const resolvedClaudeHooksPlan = buildResolvedClaudeHooks(appliedPlan); + const preparedClaudeSettings = preflightClaudeSettingsOperations(appliedPlan); const disabledServers = parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS); const linkIndex = buildLinkIndexForPlan(appliedPlan); const hasLegacyMigration = migration.legacyOperationsToRemove.length > 0; - - if (migration.requiresBridgeState) { - // Own every operation that may be written during a flat-skill migration - // before the first copy. A later failure is retryable and uninstall can - // clean the entire partial install, including non-skill files. During - // legacy migration the bridge also retains the prior managed operations. - if (typeof beforeInstallStateWrite === 'function') { - beforeInstallStateWrite({ plan: appliedPlan, state: migration.bridgeState }); + const hookRemovalCount = appliedPlan.operations.filter(operation => ( + operation.kind === 'remove-claude-settings-hooks' + )).length; + let completedHookRemovalCount = 0; + if (migration.requiresBridgeState) { + // Own every operation that may be written during a flat-skill migration + // before the first copy. A later failure is retryable and uninstall can + // clean the entire partial install, including non-skill files. During + // legacy migration the bridge also retains the prior managed operations. + if (typeof beforeInstallStateWrite === 'function') { + beforeInstallStateWrite({ plan: appliedPlan, state: migration.bridgeState }); + } + persistInstallState(plan.installStatePath, migration.bridgeState); } - persistInstallState(plan.installStatePath, migration.bridgeState); - } - let finalState; - try { - for (const operation of appliedPlan.operations) { + let finalState; + try { + for (const operation of appliedPlan.operations) { assertSafeInstallOperation(appliedPlan, operation); assertSafeClaudeSkillOperation(appliedPlan, operation); fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true }); @@ -399,6 +479,42 @@ function applyInstallPlan(plan, dependencies = {}) { beforeOperationWrite({ plan: appliedPlan, operation }); } + if ( + operation.kind === 'update-claude-settings' + || operation.kind === 'remove-claude-settings-hooks' + ) { + // Re-read at the write boundary so unrelated settings added after + // planning are preserved. A same-ID change still fails closed. + const prepared = preparedClaudeSettings.get(operation); + assertSafeInstallOperation(appliedPlan, operation); + updateSettingsAtomic(operation.destinationPath, latestSettings => { + const merged = operation.kind === 'remove-claude-settings-hooks' + ? uninstallManagedHooks(latestSettings, prepared.managedHooks) + : mergeManagedHooks(latestSettings, prepared.managedHooks, { + previousManagedHooks: prepared.previousManagedHooks, + }); + if ( + operation.kind === 'remove-claude-settings-hooks' + && merged.retained.length > 0 + ) { + throw new Error( + `Refusing to disable modified Claude hooks in ${operation.destinationPath}; ` + + 'run the ECC uninstaller to review retained entries.' + ); + } + return merged; + }, { + lockHeld: settingsLockHeld, + beforeCommit() { + assertSafeInstallOperation(appliedPlan, operation); + }, + }); + if (operation.kind === 'remove-claude-settings-hooks') { + completedHookRemovalCount += 1; + } + continue; + } + if (operation.kind === 'merge-json') { const payload = cloneJsonValue(operation.mergePayload); if (payload === undefined) { @@ -450,55 +566,49 @@ function applyInstallPlan(plan, dependencies = {}) { } fs.copyFileSync(operation.sourcePath, operation.destinationPath); - } - - if (resolvedClaudeHooksPlan) { - assertSafeInstallOperation(appliedPlan, resolvedClaudeHooksPlan.hooksOperation); - fs.mkdirSync(path.dirname(resolvedClaudeHooksPlan.hooksDestinationPath), { recursive: true }); - assertSafeInstallOperation(appliedPlan, resolvedClaudeHooksPlan.hooksOperation); - if (typeof beforeOperationWrite === 'function') { - beforeOperationWrite({ plan: appliedPlan, operation: resolvedClaudeHooksPlan.hooksOperation }); } - fs.writeFileSync( - resolvedClaudeHooksPlan.hooksDestinationPath, - JSON.stringify(resolvedClaudeHooksPlan.resolvedHooksConfig, null, 2) + '\n', - 'utf8' - ); - } - if (hasLegacyMigration) { - removeLegacyClaudeSkillFiles(migration, plan.targetRoot); - } + if (hasLegacyMigration) { + removeLegacyClaudeSkillFiles(migration, plan.targetRoot); + } - if (shouldSetClaudeCommitAttributionPreference(appliedPlan)) { - writeClaudeCommitAttributionPreference(path.join(plan.targetRoot, 'settings.json')); - } - - finalState = stateWithContentDigests(migration.finalState, appliedPlan); - if (typeof beforeInstallStateWrite === 'function') { - beforeInstallStateWrite({ plan: appliedPlan, state: finalState }); - } - persistInstallState(plan.installStatePath, finalState); - } catch (error) { - if (migration.requiresBridgeState) { - try { - // The bridge was committed before any writes. Refresh it with hashes of - // files that now exist so uninstall can remove only bytes this attempt - // actually installed while preserving user changes. - persistInstallState( - plan.installStatePath, - stateWithContentDigests(migration.bridgeState, appliedPlan) - ); - } catch (checkpointError) { - throw new Error( - `${error.message} Install-state checkpoint also failed: ${checkpointError.message}`, - { cause: error } + if (shouldSetClaudeCommitAttributionPreference(appliedPlan)) { + writeClaudeCommitAttributionPreference( + path.join(plan.targetRoot, 'settings.json'), + { lockHeld: settingsLockHeld } ); } + + finalState = stateWithContentDigests(migration.finalState, appliedPlan); + if (typeof beforeInstallStateWrite === 'function') { + beforeInstallStateWrite({ plan: appliedPlan, state: finalState }); + } + persistInstallState(plan.installStatePath, finalState); + } catch (error) { + if (migration.requiresBridgeState) { + try { + // The bridge was committed before any writes. Refresh it with hashes of + // files that now exist so uninstall can remove only bytes this attempt + // actually installed while preserving user changes. + persistInstallState( + plan.installStatePath, + stateWithContentDigests( + hookRemovalCount > 0 && completedHookRemovalCount === hookRemovalCount + ? migration.finalState + : migration.bridgeState, + appliedPlan + ) + ); + } catch (checkpointError) { + throw new Error( + `${error.message} Install-state checkpoint also failed: ${checkpointError.message}`, + { cause: error } + ); + } + } + throw error; } - throw error; - } - let antigravityMigrationWarnings = []; + let antigravityMigrationWarnings = []; try { const antigravityMigration = cleanupLegacyAntigravityInstall(appliedPlan); if (antigravityMigration.detected && !antigravityMigration.complete) { @@ -528,20 +638,20 @@ function applyInstallPlan(plan, dependencies = {}) { ]; } - return { - ...plan, - statePreview: finalState, - plannedOperations: [...plan.operations], - operations: migration.appliedOperations, - skippedOperations: migration.skippedOperations, - warnings: [ - ...(Array.isArray(plan.warnings) ? plan.warnings : []), - ...migration.warnings, - ...antigravityMigrationWarnings, - ...opencodeMigrationWarnings, - ], - applied: true, - }; + return { + ...plan, + statePreview: finalState, + plannedOperations: [...plan.operations], + operations: migration.appliedOperations, + skippedOperations: migration.skippedOperations, + warnings: [ + ...(Array.isArray(plan.warnings) ? plan.warnings : []), + ...migration.warnings, + ...antigravityMigrationWarnings, + ...opencodeMigrationWarnings, + ], + applied: true, + }; } module.exports = { diff --git a/scripts/lib/install/claude-settings.js b/scripts/lib/install/claude-settings.js new file mode 100644 index 000000000..90b0791ec --- /dev/null +++ b/scripts/lib/install/claude-settings.js @@ -0,0 +1,750 @@ +'use strict'; + +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); +const { isDeepStrictEqual } = require('util'); +const { writeFileAtomic } = require('../atomic-write'); + +const PLUGIN_ROOT_PLACEHOLDER = '${CLAUDE_PLUGIN_ROOT}'; +const VALID_EVENTS = new Set([ + 'SessionStart', 'UserPromptSubmit', 'PreToolUse', 'PermissionRequest', + 'PostToolUse', 'PostToolUseFailure', 'Notification', 'SubagentStart', + 'Stop', 'SubagentStop', 'PreCompact', 'InstructionsLoaded', + 'TeammateIdle', 'TaskCompleted', 'ConfigChange', 'WorktreeCreate', + 'WorktreeRemove', 'SessionEnd', +]); +const EVENTS_WITHOUT_MATCHER = new Set([ + 'UserPromptSubmit', 'Notification', 'Stop', 'SubagentStop', +]); +const VALID_HOOK_TYPES = new Set(['command', 'http', 'prompt', 'agent']); +const INVALID_LOCK_STALE_MS = 5 * 60 * 1000; + +function isJsonObject(value) { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +function cloneValue(value) { + if (Array.isArray(value)) { + return value.map(cloneValue); + } + if (isJsonObject(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, nestedValue]) => [key, cloneValue(nestedValue)]) + ); + } + return value; +} + +function isNonEmptyString(value) { + return typeof value === 'string' && value.trim() !== ''; +} + +function validateHookHandler(hook, label) { + if (!isJsonObject(hook)) { + throw new Error(`Invalid managed hook handler at ${label}: expected a JSON object`); + } + if (!VALID_HOOK_TYPES.has(hook.type)) { + throw new Error(`Invalid managed hook handler at ${label}: unsupported type`); + } + if (hook.timeout !== undefined && (typeof hook.timeout !== 'number' || hook.timeout < 0)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid timeout`); + } + + if (hook.type === 'command') { + const validCommand = isNonEmptyString(hook.command) + || (Array.isArray(hook.command) + && hook.command.length > 0 + && hook.command.every(isNonEmptyString)); + if (!validCommand) { + throw new Error(`Invalid managed hook handler at ${label}: invalid command`); + } + if (hook.async !== undefined && typeof hook.async !== 'boolean') { + throw new Error(`Invalid managed hook handler at ${label}: invalid async flag`); + } + return; + } + + if (hook.async !== undefined) { + throw new Error(`Invalid managed hook handler at ${label}: async requires command type`); + } + if (hook.type === 'http') { + if (!isNonEmptyString(hook.url)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid url`); + } + if ( + hook.headers !== undefined + && (!isJsonObject(hook.headers) + || !Object.values(hook.headers).every(value => typeof value === 'string')) + ) { + throw new Error(`Invalid managed hook handler at ${label}: invalid headers`); + } + if ( + hook.allowedEnvVars !== undefined + && (!Array.isArray(hook.allowedEnvVars) + || !hook.allowedEnvVars.every(isNonEmptyString)) + ) { + throw new Error(`Invalid managed hook handler at ${label}: invalid allowedEnvVars`); + } + return; + } + if (!isNonEmptyString(hook.prompt)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid prompt`); + } + if (hook.model !== undefined && !isNonEmptyString(hook.model)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid model`); + } +} + +function validateManagedHooks(managedHooks, label = 'managed hooks') { + if (!isJsonObject(managedHooks)) { + throw new Error(`Invalid ${label}: expected a JSON object`); + } + if (Object.keys(managedHooks).length === 0) { + throw new Error(`Invalid ${label}: expected at least one hook event`); + } + + const seenIds = new Set(); + for (const [event, entries] of Object.entries(managedHooks)) { + if (!VALID_EVENTS.has(event)) { + throw new Error(`Invalid ${label}: unsupported hook event "${event}"`); + } + if (!Array.isArray(entries)) { + throw new Error(`Invalid ${label}.${event}: expected an array`); + } + if (entries.length === 0) { + throw new Error(`Invalid ${label}.${event}: expected at least one hook entry`); + } + + entries.forEach((entry, index) => { + if (!isJsonObject(entry)) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: expected a JSON object` + ); + } + if (typeof entry.id !== 'string' || entry.id.trim() === '') { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: ` + + 'expected a non-empty unique id' + ); + } + if (seenIds.has(entry.id)) { + throw new Error(`Invalid ${label}: expected globally unique id "${entry.id}"`); + } + seenIds.add(entry.id); + if ( + !Object.prototype.hasOwnProperty.call(entry, 'matcher') + && !EVENTS_WITHOUT_MATCHER.has(event) + ) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: missing matcher` + ); + } + if ( + Object.prototype.hasOwnProperty.call(entry, 'matcher') + && typeof entry.matcher !== 'string' + && !isJsonObject(entry.matcher) + ) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: invalid matcher` + ); + } + if (!Array.isArray(entry.hooks) || entry.hooks.length === 0) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: expected hooks` + ); + } + entry.hooks.forEach((hook, hookIndex) => { + validateHookHandler(hook, `${label}.${event}[${index}].hooks[${hookIndex}]`); + }); + }); + } + + return cloneValue(managedHooks); +} + +function validateSettings(settings, label = 'Claude settings') { + if (!isJsonObject(settings)) { + throw new Error(`Invalid ${label}: expected a JSON object`); + } + + if (Object.prototype.hasOwnProperty.call(settings, 'hooks')) { + if (!isJsonObject(settings.hooks)) { + throw new Error(`Invalid ${label}: expected "hooks" to be a JSON object`); + } + for (const [event, entries] of Object.entries(settings.hooks)) { + if (!Array.isArray(entries)) { + throw new Error(`Invalid ${label}: expected hooks.${event} to be an array`); + } + } + } + + return cloneValue(settings); +} + +function replacePluginRootPlaceholders(value, pluginRoot) { + if (typeof pluginRoot !== 'string') { + throw new Error('Invalid Claude plugin root: expected a string'); + } + if (typeof value === 'string') { + return value.split(PLUGIN_ROOT_PLACEHOLDER).join(pluginRoot); + } + if (Array.isArray(value)) { + return value.map(item => replacePluginRootPlaceholders(item, pluginRoot)); + } + if (isJsonObject(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, nestedValue]) => [ + key, + replacePluginRootPlaceholders(nestedValue, pluginRoot), + ]) + ); + } + return value; +} + +function resolveManagedHookCommands(managedHooks, targetRoot) { + const encodedRoot = Buffer.from(targetRoot, 'utf8').toString('base64'); + const rootExpression = `Buffer.from('${encodedRoot}','base64').toString('utf8')`; + return Object.fromEntries( + Object.entries(managedHooks).map(([event, entries]) => [ + event, + entries.map(entry => ({ + ...entry, + hooks: entry.hooks.map(hook => ({ + ...hook, + ...(typeof hook.command === 'string' + ? { + command: hook.command + .split('var e=process.env.CLAUDE_PLUGIN_ROOT;') + .join(`var e=${rootExpression};`), + } + : {}), + })), + })), + ]) + ); +} + +function materializeManagedHooks(hooksConfig, targetRoot) { + if (!isJsonObject(hooksConfig) || !isJsonObject(hooksConfig.hooks)) { + throw new Error('Invalid hooks config: expected a JSON object with a hooks object'); + } + if (!isNonEmptyString(targetRoot)) { + throw new Error('Invalid Claude target root: expected a non-empty string'); + } + return validateManagedHooks(resolveManagedHookCommands( + replacePluginRootPlaceholders(hooksConfig.hooks, targetRoot), + targetRoot + )); +} + +function parseSettings(rawSettings, label = 'Claude settings') { + let settings; + try { + settings = JSON.parse(rawSettings); + } catch (error) { + throw new Error(`Failed to parse ${label}: ${error.message}`, { cause: error }); + } + return validateSettings(settings, label); +} + +function readSettings(settingsPath, fileSystem = fs) { + const reader = fileSystem && fileSystem.fs ? fileSystem.fs : fileSystem; + let rawSettings; + try { + rawSettings = reader.readFileSync(settingsPath, 'utf8'); + } catch (error) { + if (error && error.code === 'ENOENT') { + return {}; + } + throw error; + } + return parseSettings(rawSettings, `Claude settings at ${settingsPath}`); +} + +function readSettingsSnapshot(settingsPath) { + const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0); + let descriptor; + try { + descriptor = fs.openSync(settingsPath, flags); + } catch (error) { + if (error && error.code === 'ENOENT') { + return { exists: false, raw: null, settings: {}, mode: 0o600 }; + } + throw error; + } + + try { + const descriptorStat = fs.fstatSync(descriptor); + let pathStat; + try { + pathStat = fs.lstatSync(settingsPath); + } catch (error) { + if (error && error.code === 'ENOENT') { + error.code = 'ECC_SETTINGS_CHANGED'; + } + throw error; + } + if ( + !descriptorStat.isFile() + || !pathStat.isFile() + || pathStat.isSymbolicLink() + || descriptorStat.dev !== pathStat.dev + || descriptorStat.ino !== pathStat.ino + ) { + const error = new Error(`Refusing to read changed Claude settings at ${settingsPath}`); + error.code = 'ECC_SETTINGS_CHANGED'; + throw error; + } + const raw = fs.readFileSync(descriptor, 'utf8'); + return { + exists: true, + raw, + settings: parseSettings(raw, `Claude settings at ${settingsPath}`), + mode: descriptorStat.mode & 0o777, + dev: descriptorStat.dev, + ino: descriptorStat.ino, + }; + } finally { + fs.closeSync(descriptor); + } +} + +function assertSettingsSnapshotUnchanged(settingsPath, snapshot) { + let current; + try { + current = readSettingsSnapshot(settingsPath); + } catch (error) { + error.code = error.code || 'ECC_SETTINGS_CHANGED'; + throw error; + } + const unchanged = current.exists === snapshot.exists + && current.raw === snapshot.raw + && (!current.exists || (current.dev === snapshot.dev && current.ino === snapshot.ino)); + if (!unchanged) { + const error = new Error(`Claude settings changed during update: ${settingsPath}`); + error.code = 'ECC_SETTINGS_CHANGED'; + throw error; + } +} + +function createSettingsLock(lockPath) { + const tempPath = `${lockPath}.create-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + let descriptor; + let ownedStats; + try { + descriptor = fs.openSync(tempPath, 'wx', 0o600); + fs.writeFileSync(descriptor, `${JSON.stringify({ + pid: process.pid, + startedAt: new Date().toISOString(), + token: crypto.randomBytes(16).toString('hex'), + })}\n`); + fs.fsyncSync(descriptor); + ownedStats = fs.fstatSync(descriptor, { bigint: true }); + fs.closeSync(descriptor); + descriptor = undefined; + fs.linkSync(tempPath, lockPath); + } catch (error) { + if (descriptor !== undefined) fs.closeSync(descriptor); + fs.rmSync(tempPath, { force: true }); + throw error; + } + fs.rmSync(tempPath, { force: true }); + + let released = false; + return () => { + if (released) return; + released = true; + const quarantinePath = `${lockPath}.release-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + fs.renameSync(lockPath, quarantinePath); + const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); + if (!sameFileIdentity(quarantinedStats, ownedStats)) { + if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); + throw new Error(`Refusing to release a changed Claude settings lock: ${lockPath}`); + } + fs.rmSync(quarantinePath, { force: true }); + }; +} + +function sameFileIdentity(left, right) { + return left.dev === right.dev && left.ino === right.ino; +} + +function inspectSettingsLock(lockPath) { + const descriptor = fs.openSync(lockPath, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + try { + const stats = fs.fstatSync(descriptor, { bigint: true }); + const pathStats = fs.lstatSync(lockPath, { bigint: true }); + if ( + !stats.isFile() + || pathStats.isSymbolicLink() + || !pathStats.isFile() + || !sameFileIdentity(stats, pathStats) + ) { + return { metadata: null, stats }; + } + let metadata = null; + try { + metadata = JSON.parse(fs.readFileSync(descriptor, 'utf8')); + } catch (_error) { + // Invalid locks may be recovered only after the bounded lease below. + } + return { metadata, stats }; + } finally { + fs.closeSync(descriptor); + } +} + +function processIsAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code !== 'ESRCH'; + } +} + +function recoverSettingsLock(lockPath) { + const recoveryPath = `${lockPath}.recover`; + try { + fs.mkdirSync(recoveryPath, { mode: 0o700 }); + } catch (error) { + if (error && error.code === 'EEXIST') return null; + throw error; + } + + const quarantinePath = `${lockPath}.stale-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + try { + let inspected; + try { + inspected = inspectSettingsLock(lockPath); + } catch (error) { + if (error && error.code === 'ENOENT') return createSettingsLock(lockPath); + throw error; + } + const validOwner = Number.isSafeInteger(inspected.metadata && inspected.metadata.pid) + && inspected.metadata.pid > 0; + const stale = validOwner + ? !processIsAlive(inspected.metadata.pid) + : Date.now() - Number(inspected.stats.mtimeMs) >= INVALID_LOCK_STALE_MS; + if (!stale) return null; + + fs.renameSync(lockPath, quarantinePath); + const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); + if (!sameFileIdentity(quarantinedStats, inspected.stats)) { + if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); + return null; + } + fs.rmSync(quarantinePath, { force: true }); + return createSettingsLock(lockPath); + } finally { + fs.rmSync(recoveryPath, { recursive: true, force: true }); + fs.rmSync(quarantinePath, { force: true }); + } +} + +function acquireSettingsLock(settingsPath) { + const lockPath = `${settingsPath}.ecc.lock`; + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + try { + return createSettingsLock(lockPath); + } catch (error) { + if (!error || error.code !== 'EEXIST') { + throw error; + } + } + const recovered = recoverSettingsLock(lockPath); + if (recovered) return recovered; + throw new Error( + `Another ECC process is updating Claude settings: ${settingsPath}. ` + + `If no ECC process is active, inspect and remove ${lockPath}.` + ); +} + +function updateSettingsAtomic(settingsPath, transform, options = {}) { + const update = () => { + const maxAttempts = options.maxAttempts || 3; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + const snapshot = readSettingsSnapshot(settingsPath); + const result = transform(snapshot.settings); + if (typeof options.beforeCommit === 'function') options.beforeCommit(); + assertSettingsSnapshotUnchanged(settingsPath, snapshot); + writeFileAtomic( + settingsPath, + `${JSON.stringify(result.settings, null, 2)}\n`, + { encoding: 'utf8', mode: snapshot.mode } + ); + return result; + } catch (error) { + if (error.code !== 'ECC_SETTINGS_CHANGED' || attempt === maxAttempts) { + throw error; + } + } + } + throw new Error(`Unable to update Claude settings at ${settingsPath}`); + }; + if (options.lockHeld) { + return update(); + } + const releaseLock = acquireSettingsLock(settingsPath); + try { + return update(); + } finally { + releaseLock(); + } +} + +function reference(event, id) { + return { event, id }; +} + +function entriesMatchingId(entries, id) { + return entries + .map((entry, index) => ({ entry, index })) + .filter(candidate => isJsonObject(candidate.entry) && candidate.entry.id === id); +} + +function assertUnambiguousMatch(entries, event, id) { + const matches = entriesMatchingId(entries, id); + if (matches.length > 1) { + throw new Error( + `Claude settings contains multiple hooks for event "${event}" and id "${id}"` + ); + } + return matches[0] || null; +} + +function managedEntryFor(managedHooks, event, id) { + const entries = managedHooks && managedHooks[event]; + if (!Array.isArray(entries)) { + return null; + } + return entries.find(entry => entry.id === id) || null; +} + +function mergeManagedHooks(settings, managedHooks, options = {}) { + const validatedSettings = validateSettings(settings); + const desiredHooks = validateManagedHooks(managedHooks); + const previousHooks = options.previousManagedHooks === undefined + || options.previousManagedHooks === null + ? null + : validateManagedHooks(options.previousManagedHooks, 'previous managed hooks'); + const repair = options.mode === 'repair' || options.repair === true; + if (options.mode !== undefined && options.mode !== 'merge' && options.mode !== 'repair') { + throw new Error(`Unknown Claude settings merge mode: ${options.mode}`); + } + + let nextHooks = validatedSettings.hooks + ? cloneValue(validatedSettings.hooks) + : {}; + const added = []; + const updated = []; + const unchanged = []; + const removed = []; + + if (previousHooks) { + for (const [event, previousEntries] of Object.entries(previousHooks)) { + let eventEntries = nextHooks[event] ? cloneValue(nextHooks[event]) : []; + for (const previousEntry of previousEntries) { + if (managedEntryFor(desiredHooks, event, previousEntry.id)) continue; + const match = assertUnambiguousMatch(eventEntries, event, previousEntry.id); + if (!match) continue; + if (!isDeepStrictEqual(match.entry, previousEntry)) { + throw new Error( + `Refusing to remove Claude hook for event "${event}" and id ` + + `"${previousEntry.id}" because the previous managed entry has drifted` + ); + } + eventEntries = eventEntries.filter((_entry, index) => index !== match.index); + removed.push(reference(event, previousEntry.id)); + } + nextHooks = eventEntries.length > 0 + ? { ...nextHooks, [event]: eventEntries } + : withoutProperty(nextHooks, event); + } + } + + for (const [event, desiredEntries] of Object.entries(desiredHooks)) { + let eventEntries = nextHooks[event] ? cloneValue(nextHooks[event]) : []; + for (const desiredEntry of desiredEntries) { + const match = assertUnambiguousMatch(eventEntries, event, desiredEntry.id); + if (!match) { + eventEntries = [...eventEntries, cloneValue(desiredEntry)]; + added.push(reference(event, desiredEntry.id)); + continue; + } + if (isDeepStrictEqual(match.entry, desiredEntry)) { + unchanged.push(reference(event, desiredEntry.id)); + continue; + } + + const previousEntry = managedEntryFor(previousHooks, event, desiredEntry.id); + if (!repair && (!previousEntry || !isDeepStrictEqual(match.entry, previousEntry))) { + const driftReason = previousEntry ? ' because the previous managed entry has drifted' : ''; + throw new Error( + `Refusing to overwrite Claude hook for event "${event}" and id ` + + `"${desiredEntry.id}"${driftReason}` + ); + } + + eventEntries = eventEntries.map((entry, index) => ( + index === match.index ? cloneValue(desiredEntry) : entry + )); + updated.push(reference(event, desiredEntry.id)); + } + if (desiredEntries.length > 0) { + nextHooks = { ...nextHooks, [event]: eventEntries }; + } + } + + const nextSettings = Object.keys(nextHooks).length > 0 + ? { ...validatedSettings, hooks: nextHooks } + : validatedSettings; + return { + settings: nextSettings, + managedHooks: cloneValue(desiredHooks), + added, + updated, + unchanged, + removed, + }; +} + +function repairManagedHooks(settings, managedHooks, options = {}) { + return mergeManagedHooks(settings, managedHooks, { + ...options, + mode: 'repair', + }); +} + +function inspectManagedHooks(settings, managedHooks) { + const validatedSettings = validateSettings(settings); + const expectedHooks = validateManagedHooks(managedHooks); + const settingsHooks = validatedSettings.hooks || {}; + const managedSubset = {}; + const matched = []; + const missing = []; + const drifted = []; + + for (const [event, expectedEntries] of Object.entries(expectedHooks)) { + const actualEntries = settingsHooks[event] || []; + const foundEntries = []; + for (const expectedEntry of expectedEntries) { + const match = assertUnambiguousMatch(actualEntries, event, expectedEntry.id); + if (!match) { + missing.push(reference(event, expectedEntry.id)); + continue; + } + + foundEntries.push(cloneValue(match.entry)); + if (isDeepStrictEqual(match.entry, expectedEntry)) { + matched.push(reference(event, expectedEntry.id)); + } else { + drifted.push({ + ...reference(event, expectedEntry.id), + expected: cloneValue(expectedEntry), + actual: cloneValue(match.entry), + }); + } + } + if (foundEntries.length > 0) { + managedSubset[event] = foundEntries; + } + } + + const ok = missing.length === 0 && drifted.length === 0; + return { + status: ok ? 'ok' : (missing.length > 0 ? 'missing' : 'drifted'), + ok, + managedHooks: managedSubset, + matched, + missing, + drifted, + }; +} + +function withoutProperty(object, omittedKey) { + return Object.fromEntries( + Object.entries(object).filter(([key]) => key !== omittedKey) + ); +} + +function uninstallManagedHooks(settings, recordedManagedHooks) { + const validatedSettings = validateSettings(settings); + const recordedHooks = validateManagedHooks(recordedManagedHooks, 'recorded managed hooks'); + const currentHooks = validatedSettings.hooks || {}; + + for (const [event, recordedEntries] of Object.entries(recordedHooks)) { + const eventEntries = currentHooks[event] || []; + for (const recordedEntry of recordedEntries) { + assertUnambiguousMatch(eventEntries, event, recordedEntry.id); + } + } + + const removed = []; + const retained = []; + const missing = []; + let nextHooks = cloneValue(currentHooks); + + for (const [event, recordedEntries] of Object.entries(recordedHooks)) { + let eventEntries = nextHooks[event] || []; + for (const recordedEntry of recordedEntries) { + const match = assertUnambiguousMatch(eventEntries, event, recordedEntry.id); + if (!match) { + missing.push(reference(event, recordedEntry.id)); + continue; + } + if (!isDeepStrictEqual(match.entry, recordedEntry)) { + retained.push({ + ...reference(event, recordedEntry.id), + expected: cloneValue(recordedEntry), + actual: cloneValue(match.entry), + reason: 'modified', + }); + continue; + } + + eventEntries = eventEntries.filter((_entry, index) => index !== match.index); + removed.push(reference(event, recordedEntry.id)); + } + nextHooks = eventEntries.length > 0 + ? { ...nextHooks, [event]: eventEntries } + : withoutProperty(nextHooks, event); + } + + nextHooks = Object.fromEntries( + Object.entries(nextHooks).filter(([, entries]) => entries.length > 0) + ); + const settingsWithoutHooks = withoutProperty(validatedSettings, 'hooks'); + const nextSettings = Object.keys(nextHooks).length > 0 + ? { ...settingsWithoutHooks, hooks: nextHooks } + : settingsWithoutHooks; + + return { + settings: nextSettings, + removed, + retained, + missing, + }; +} + +module.exports = { + acquireSettingsLock, + inspectManagedHooks, + materializeManagedHooks, + mergeManagedHooks, + parseSettings, + readSettings, + repairManagedHooks, + replacePluginRootPlaceholders, + updateSettingsAtomic, + uninstallManagedHooks, + validateManagedHooks, + validateSettings, +}; diff --git a/scripts/lib/install/hook-consent.js b/scripts/lib/install/hook-consent.js index f12bd833c..883c121d7 100644 --- a/scripts/lib/install/hook-consent.js +++ b/scripts/lib/install/hook-consent.js @@ -44,7 +44,10 @@ function normalizeOperationPath(value) { } function isHookRuntimeOperation(operation = {}) { - if (operation.moduleId === HOOK_RUNTIME_MODULE_ID) { + if ( + operation.kind === 'update-claude-settings' + || operation.moduleId === HOOK_RUNTIME_MODULE_ID + ) { return true; } diff --git a/scripts/lib/install/plan.js b/scripts/lib/install/plan.js index d98ef8f0b..08173a672 100644 --- a/scripts/lib/install/plan.js +++ b/scripts/lib/install/plan.js @@ -7,6 +7,9 @@ const { execFileSync } = require('child_process'); const { resolveInstallPlan } = require('../install-manifests'); const { getInstallTargetAdapter } = require('../install-targets/registry'); const { resolveInvocationEnvironment } = require('../invocation-environment'); +const { + materializeManagedHooks, +} = require('./claude-settings'); const EXCLUDED_GENERATED_SOURCE_SUFFIXES = ['/ecc-install-state.json', '/ecc/install-state.json']; const IGNORED_DIRECTORY_NAMES = new Set([ @@ -127,7 +130,31 @@ function readJsonObject(filePath, label) { return parsed; } +function materializeClaudeSettingsOperation(sourceRoot, operation) { + const sourcePath = path.join(sourceRoot, operation.sourceRelativePath); + if (!fs.existsSync(sourcePath)) { + return []; + } + + const hooksConfig = readJsonObject(sourcePath, operation.sourceRelativePath); + const managedHooks = materializeManagedHooks( + hooksConfig, + path.dirname(operation.destinationPath) + ); + + return [{ + ...operation, + sourcePath, + scaffoldOnly: false, + managedHooks, + }]; +} + function materializeScaffoldOperation(sourceRoot, operation) { + if (operation.kind === 'update-claude-settings') { + return materializeClaudeSettingsOperation(sourceRoot, operation); + } + if (operation.kind === 'merge-json') { return [ { diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index afac4a469..a91bfe854 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -699,7 +699,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - InvalidEventType: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo hi' }] }] + InvalidEventType: [{ id: 'test:invalid-event', matcher: 'test', hooks: [{ type: 'command', command: 'echo hi' }] }] } })); @@ -714,7 +714,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ command: 'echo hi' }] }] + PreToolUse: [{ id: 'test:missing-type', matcher: 'test', hooks: [{ command: 'echo hi' }] }] } })); @@ -729,7 +729,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command' }] }] + PreToolUse: [{ id: 'test:missing-command', matcher: 'test', hooks: [{ type: 'command' }] }] } })); @@ -744,7 +744,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', async: 'yes' }] }] + PreToolUse: [{ id: 'test:invalid-async', matcher: 'test', hooks: [{ type: 'command', command: 'echo', async: 'yes' }] }] } })); @@ -759,7 +759,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: -5 }] }] + PreToolUse: [{ id: 'test:negative-timeout', matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: -5 }] }] } })); @@ -774,7 +774,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'node -e "function {"' }] }] + PreToolUse: [{ id: 'test:invalid-inline-js', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "function {"' }] }] } })); @@ -789,7 +789,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'node -e "console.log(1+2)"' }] }] + PreToolUse: [{ id: 'test:valid-inline-js', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "console.log(1+2)"' }] }] } })); @@ -803,7 +803,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ['node', '-e', 'console.log(1)'] }] }] + PreToolUse: [{ id: 'test:array-command', matcher: 'test', hooks: [{ type: 'command', command: ['node', '-e', 'console.log(1)'] }] }] } })); @@ -829,7 +829,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test' }] + PreToolUse: [{ id: 'test:missing-hooks', matcher: 'test' }] } })); @@ -1396,7 +1396,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ' \t ' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: ' \t ' }] }] } })); @@ -1411,7 +1411,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: null }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: null }] }] } })); @@ -1426,7 +1426,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 42 }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 42 }] }] } })); @@ -1605,7 +1605,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: '' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: '' }] }] } })); @@ -1620,7 +1620,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: [] }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: [] }] }] } })); @@ -1635,7 +1635,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ['node', 123, null] }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: ['node', 123, null] }] }] } })); @@ -1650,7 +1650,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 42, command: 'echo hi' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 42, command: 'echo hi' }] }] } })); @@ -1665,7 +1665,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 'fast' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 'fast' }] }] } })); @@ -1680,7 +1680,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 0 }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 0 }] }] } })); @@ -1694,7 +1694,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); // data.hooks is undefined, so fallback to data itself fs.writeFileSync(hooksFile, JSON.stringify({ - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo ok' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo ok' }] }] })); const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); @@ -1796,7 +1796,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ['node', '', 'script.js'] }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: ['node', '', 'script.js'] }] }] } })); @@ -1811,7 +1811,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo hi', timeout: -5 }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo hi', timeout: -5 }] }] } })); @@ -1826,7 +1826,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PostToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo ok', async: 'yes' }] }] + PostToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo ok', async: 'yes' }] }] } })); @@ -1847,7 +1847,7 @@ function runTests() { manyHooks.push({ type: 'command', command: '' }); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: manyHooks }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: manyHooks }] } })); @@ -1862,7 +1862,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'node -e "const x = 1 + 2; process.exit(0)"' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "const x = 1 + 2; process.exit(0)"' }] }] } })); @@ -1876,9 +1876,9 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo pre' }] }], - PostToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo post' }] }], - Stop: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo stop' }] }] + PreToolUse: [{ id: 'test:multi-event-pre', matcher: 'test', hooks: [{ type: 'command', command: 'echo pre' }] }], + PostToolUse: [{ id: 'test:multi-event-post', matcher: 'test', hooks: [{ type: 'command', command: 'echo post' }] }], + Stop: [{ id: 'test:multi-event-stop', matcher: 'test', hooks: [{ type: 'command', command: 'echo stop' }] }] } })); @@ -2227,7 +2227,7 @@ function runTests() { // After unescape chain: var a = "ok"\nconsole.log(a) (real newline) — valid JS fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "var a = \\"ok\\"\\nconsole.log(a)"' }] }] } })); @@ -2243,7 +2243,7 @@ function runTests() { // After unescape this becomes: var x = { — missing closing brace fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "var x = {"' }] }] } })); @@ -2427,7 +2427,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: { run: 'echo hi' } }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: { run: 'echo hi' } }] }] } })); @@ -2446,7 +2446,7 @@ function runTests() { // Object format: matcher entry has hooks array but NO matcher field fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ hooks: [{ type: 'command', command: 'echo ok' }] }] + PreToolUse: [{ id: 'test:missing-matcher', hooks: [{ type: 'command', command: 'echo ok' }] }] } })); @@ -2554,6 +2554,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ PreToolUse: [{ + id: 'test:round72-async', matcher: 'Write', hooks: [{ type: 'command', @@ -2574,6 +2575,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ PostToolUse: [{ + id: 'test:round72-timeout', matcher: 'Edit', hooks: [{ type: 'command', @@ -2661,8 +2663,8 @@ function runTests() { fs.writeFileSync(hooksFile, JSON.stringify({ "$schema": "https://json.schemastore.org/claude-code-settings.json", hooks: { - PreToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo ok' }] }], - PostToolUse: [{ matcher: 'Read', hooks: [{ type: 'command', command: 'echo done' }] }] + PreToolUse: [{ id: 'test:wrapped-pre', matcher: 'Write', hooks: [{ type: 'command', command: 'echo ok' }] }], + PostToolUse: [{ id: 'test:wrapped-post', matcher: 'Read', hooks: [{ type: 'command', command: 'echo done' }] }] } })); @@ -2674,6 +2676,74 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (test('rejects wrapped matcher entry missing id', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + PreToolUse: [{ + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo missing id' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1, 'Should reject wrapped matcher entries without an id'); + assert.ok(result.stderr.includes('id'), `Should report missing id, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects wrapped matcher entry with whitespace-only id', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + PreToolUse: [{ + id: ' \t', + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo blank id' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1, 'Should reject whitespace-only matcher ids'); + assert.ok(result.stderr.includes('id'), `Should report invalid id, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects duplicate wrapped matcher ids across events', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + PreToolUse: [{ + id: 'shared:matcher', + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo pre' }] + }], + PostToolUse: [{ + id: 'shared:matcher', + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo post' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1, 'Should reject matcher ids reused by another event'); + assert.ok( + result.stderr.includes("duplicate id 'shared:matcher'"), + `Should report the duplicate id, got: ${result.stderr}` + ); + assert.ok( + result.stderr.includes('PreToolUse[0]') && result.stderr.includes('PostToolUse[0]'), + `Should report both matcher locations, got: ${result.stderr}` + ); + cleanupTestDir(testDir); + })) passed++; else failed++; + // ── Round 79: validate-commands.js warnings count suffix in output ── console.log('\nRound 79: validate-commands.js (warnings count in output):'); @@ -2756,6 +2826,7 @@ function runTests() { hooks: { UserPromptSubmit: [ { + id: 'test:user-prompt-submit', hooks: [ { type: 'prompt', prompt: 'Summarize the request.' }, { type: 'agent', prompt: 'Review for security issues.', model: 'gpt-5.4' }, diff --git a/tests/lib/claude-settings.test.js b/tests/lib/claude-settings.test.js new file mode 100644 index 000000000..4fadb1c11 --- /dev/null +++ b/tests/lib/claude-settings.test.js @@ -0,0 +1,557 @@ +/** + * Focused coverage for safely managing ECC hook entries in Claude settings. + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { + inspectManagedHooks, + mergeManagedHooks, + parseSettings, + readSettings, + repairManagedHooks, + replacePluginRootPlaceholders, + uninstallManagedHooks, + updateSettingsAtomic, + validateManagedHooks, +} = require('../../scripts/lib/install/claude-settings'); + +function test(name, fn) { + try { + fn(); + console.log(` PASS ${name}`); + return true; + } catch (error) { + console.log(` FAIL ${name}`); + console.log(` Error: ${error.stack || error.message}`); + return false; + } +} + +function entry(id, command, extra = {}) { + return { + matcher: '.*', + hooks: [{ type: 'command', command }], + id, + ...extra, + }; +} + +function clone(value) { + return JSON.parse(JSON.stringify(value)); +} + +function runTests() { + console.log('\n=== Testing install/claude-settings.js ===\n'); + + let passed = 0; + let failed = 0; + + if (test('validates and clones a managed hook map without mutating it', () => { + const managed = { + SessionStart: [entry('session:start', 'node start.js')], + Stop: [entry('session:stop', 'node stop.js')], + }; + const validated = validateManagedHooks(managed); + + assert.deepStrictEqual(validated, managed); + assert.notStrictEqual(validated, managed); + assert.notStrictEqual(validated.SessionStart[0], managed.SessionStart[0]); + })) passed++; else failed++; + + if (test('strictly rejects invalid managed hook maps and globally duplicate ids', () => { + const invalidValues = [ + null, + [], + {}, + { SessionStart: [] }, + { SessionStart: {} }, + { SessionStart: [null] }, + { SessionStart: [[]] }, + { SessionStart: [{}] }, + { SessionStart: [{ id: ' ' }] }, + { BogusEvent: [entry('bad:event', 'bad')] }, + { SessionStart: [{ id: 'missing:hooks', matcher: '.*' }] }, + { SessionStart: [{ id: 'bad:command', matcher: '.*', hooks: [{ type: 'command' }] }] }, + { + SessionStart: [{ id: 'shared' }], + Stop: [{ id: 'shared' }], + }, + ]; + + for (const invalid of invalidValues) { + assert.throws(() => validateManagedHooks(invalid), /managed hooks|hook entry|unique id/i); + } + })) passed++; else failed++; + + if (test('replaces every plugin-root placeholder recursively and immutably', () => { + const source = { + SessionStart: [{ + id: 'session:start', + command: '${CLAUDE_PLUGIN_ROOT}/start.js:${CLAUDE_PLUGIN_ROOT}', + nested: ['${CLAUDE_PLUGIN_ROOT}/nested.js', 3, null], + }], + }; + const before = clone(source); + + const resolved = replacePluginRootPlaceholders(source, '/opt/ecc'); + + assert.deepStrictEqual(source, before); + assert.deepStrictEqual(resolved, { + SessionStart: [{ + id: 'session:start', + command: '/opt/ecc/start.js:/opt/ecc', + nested: ['/opt/ecc/nested.js', 3, null], + }], + }); + })) passed++; else failed++; + + if (test('parseSettings accepts an object and validates every hooks event array', () => { + assert.deepStrictEqual( + parseSettings('{"theme":"dark","hooks":{"Stop":[]}}', 'memory settings'), + { theme: 'dark', hooks: { Stop: [] } } + ); + assert.throws(() => parseSettings('{', 'memory settings'), /Failed to parse memory settings/); + assert.throws(() => parseSettings('null', 'memory settings'), /expected a JSON object/); + assert.throws(() => parseSettings('[]', 'memory settings'), /expected a JSON object/); + assert.throws( + () => parseSettings('{"hooks":{"Stop":{}}}', 'memory settings'), + /hooks\.Stop.*array/ + ); + assert.throws( + () => parseSettings('{"hooks":[]}', 'memory settings'), + /"hooks".*object/ + ); + })) passed++; else failed++; + + if (test('readSettings returns an empty object for ENOENT and rejects bad files', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-')); + try { + assert.deepStrictEqual(readSettings(path.join(tempDir, 'missing.json')), {}); + + const malformedPath = path.join(tempDir, 'malformed.json'); + fs.writeFileSync(malformedPath, '{', 'utf8'); + assert.throws(() => readSettings(malformedPath), /Failed to parse Claude settings/); + + const invalidPath = path.join(tempDir, 'invalid.json'); + fs.writeFileSync(invalidPath, '{"hooks":{"Stop":false}}', 'utf8'); + assert.throws(() => readSettings(invalidPath), /hooks\.Stop.*array/); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('readSettings propagates non-ENOENT read errors without converting them to empty settings', () => { + const denied = new Error('denied'); + denied.code = 'EACCES'; + assert.throws( + () => readSettings('/private/settings.json', { + readFileSync() { + throw denied; + }, + }), + error => error === denied + ); + })) passed++; else failed++; + + if (test('atomic settings updates retry after a concurrent change and preserve secure mode', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-atomic-')); + const settingsPath = path.join(tempDir, 'settings.json'); + let commitAttempts = 0; + try { + const result = updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, managed: true } }), + { + beforeCommit() { + commitAttempts += 1; + if (commitAttempts === 1) { + fs.writeFileSync(settingsPath, '{"theme":"concurrent"}\n', { mode: 0o600 }); + } + }, + } + ); + + assert.deepStrictEqual(result.settings, { theme: 'concurrent', managed: true }); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), result.settings); + assert.strictEqual(commitAttempts, 2); + if (process.platform !== 'win32') { + assert.strictEqual(fs.statSync(settingsPath).mode & 0o777, 0o600); + } + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('atomic settings updates recover a stale invalid lock after its lease', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-stale-lock-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + try { + fs.writeFileSync(lockPath, '', { mode: 0o600 }); + const stale = new Date(Date.now() - (10 * 60 * 1000)); + fs.utimesSync(lockPath, stale, stale); + updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, recovered: true } }) + ); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + recovered: true, + }); + assert.ok(!fs.existsSync(lockPath)); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('atomic settings updates serialize nested ECC writers and release the lock', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-lock-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + try { + updateSettingsAtomic(settingsPath, settings => { + assert.throws( + () => updateSettingsAtomic( + settingsPath, + nested => ({ settings: { ...nested, nested: true } }) + ), + /Another ECC process is updating Claude settings/ + ); + return { settings: { ...settings, outer: true } }; + }); + + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + outer: true, + }); + assert.ok(!fs.existsSync(lockPath)); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('fresh merge appends managed entries while preserving unrelated settings and hooks', () => { + const userEntry = { matcher: 'Bash', hooks: [{ type: 'command', command: 'user-hook' }] }; + const settings = { + theme: 'dark', + hooks: { + SessionStart: [userEntry], + Notification: [{ id: 'user:notification', command: 'notify' }], + }, + }; + const managed = { + SessionStart: [entry('ecc:start', 'node /opt/ecc/start.js')], + Stop: [entry('ecc:stop', 'node /opt/ecc/stop.js')], + }; + const settingsBefore = clone(settings); + const managedBefore = clone(managed); + + const result = mergeManagedHooks(settings, managed); + + assert.deepStrictEqual(settings, settingsBefore); + assert.deepStrictEqual(managed, managedBefore); + assert.deepStrictEqual(result.settings, { + theme: 'dark', + hooks: { + SessionStart: [userEntry, managed.SessionStart[0]], + Notification: settings.hooks.Notification, + Stop: managed.Stop, + }, + }); + assert.deepStrictEqual(result.added, [ + { event: 'SessionStart', id: 'ecc:start' }, + { event: 'Stop', id: 'ecc:stop' }, + ]); + assert.deepStrictEqual(result.updated, []); + })) passed++; else failed++; + + if (test('fresh merge treats a different entry with the same event and id as a conflict', () => { + const settings = { + hooks: { + Stop: [entry('ecc:stop', 'user-modified')], + }, + }; + const managed = { + Stop: [entry('ecc:stop', 'managed')], + }; + + assert.throws( + () => mergeManagedHooks(settings, managed), + /Refusing to overwrite.*Stop.*ecc:stop/ + ); + assert.deepStrictEqual(settings.hooks.Stop[0], entry('ecc:stop', 'user-modified')); + })) passed++; else failed++; + + if (test('fresh merge adopts an identical existing event and id without duplicating it', () => { + const managed = { Stop: [entry('ecc:stop', 'managed')] }; + const result = mergeManagedHooks({ hooks: clone(managed) }, managed); + + assert.deepStrictEqual(result.settings.hooks.Stop, managed.Stop); + assert.deepStrictEqual(result.unchanged, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('upgrade replaces an entry only while it still equals previous managed content', () => { + const previousManagedHooks = { Stop: [entry('ecc:stop', 'version-1')] }; + const managedHooks = { Stop: [entry('ecc:stop', 'version-2')] }; + const result = mergeManagedHooks( + { hooks: { Stop: [entry('ecc:stop', 'version-1')] } }, + managedHooks, + { previousManagedHooks } + ); + + assert.deepStrictEqual(result.settings.hooks.Stop, managedHooks.Stop); + assert.deepStrictEqual(result.updated, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('upgrade fails closed when previous managed content has drifted', () => { + const settings = { hooks: { Stop: [entry('ecc:stop', 'customer-edit')] } }; + const before = clone(settings); + + assert.throws( + () => mergeManagedHooks( + settings, + { Stop: [entry('ecc:stop', 'version-2')] }, + { previousManagedHooks: { Stop: [entry('ecc:stop', 'version-1')] } } + ), + /drifted|Refusing to overwrite/ + ); + assert.deepStrictEqual(settings, before); + })) passed++; else failed++; + + if (test('upgrade is idempotent when the desired entry is already installed', () => { + const desired = { Stop: [entry('ecc:stop', 'version-2')] }; + const result = mergeManagedHooks( + { hooks: clone(desired) }, + desired, + { previousManagedHooks: { Stop: [entry('ecc:stop', 'version-1')] } } + ); + + assert.deepStrictEqual(result.settings.hooks, desired); + assert.deepStrictEqual(result.unchanged, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('upgrade removes unchanged entries that are no longer managed', () => { + const previousManagedHooks = { + Stop: [ + entry('ecc:keep', 'version-1'), + entry('ecc:removed', 'old-command'), + ], + }; + const desired = { Stop: [entry('ecc:keep', 'version-2')] }; + const userEntry = entry('user:stop', 'keep-user'); + const result = mergeManagedHooks({ + hooks: { Stop: [userEntry, ...previousManagedHooks.Stop] }, + }, desired, { previousManagedHooks }); + + assert.deepStrictEqual(result.settings.hooks.Stop, [userEntry, desired.Stop[0]]); + assert.deepStrictEqual(result.removed, [{ event: 'Stop', id: 'ecc:removed' }]); + })) passed++; else failed++; + + if (test('upgrade removes multiple retired hooks without deleting their neighbor', () => { + const previousManagedHooks = { + Stop: [entry('ecc:a', 'a'), entry('ecc:b', 'b')], + }; + const userEntry = entry('user:c', 'keep-user'); + const result = mergeManagedHooks({ + hooks: { Stop: [...previousManagedHooks.Stop, userEntry] }, + }, { SessionStart: [entry('ecc:start', 'start')] }, { previousManagedHooks }); + + assert.deepStrictEqual(result.settings.hooks.Stop, [userEntry]); + assert.deepStrictEqual(result.removed, [ + { event: 'Stop', id: 'ecc:a' }, + { event: 'Stop', id: 'ecc:b' }, + ]); + })) passed++; else failed++; + + if (test('upgrade refuses to remove a retired entry after user drift', () => { + const previousManagedHooks = { Stop: [entry('ecc:removed', 'old-command')] }; + const settings = { hooks: { Stop: [entry('ecc:removed', 'user-edited')] } }; + + assert.throws( + () => mergeManagedHooks(settings, { SessionStart: [entry('ecc:start', 'start')] }, { + previousManagedHooks, + }), + /Refusing to remove.*ecc:removed.*drifted/ + ); + })) passed++; else failed++; + + if (test('merge fails closed when settings contains ambiguous duplicate event ids', () => { + assert.throws( + () => mergeManagedHooks( + { + hooks: { + Stop: [ + entry('ecc:stop', 'version-1'), + entry('ecc:stop', 'another-copy'), + ], + }, + }, + { Stop: [entry('ecc:stop', 'version-2')] }, + { previousManagedHooks: { Stop: [entry('ecc:stop', 'version-1')] } } + ), + /multiple.*ecc:stop/i + ); + })) passed++; else failed++; + + if (test('merge treats the same id under another event as a separate user entry', () => { + const result = mergeManagedHooks( + { hooks: { SessionStart: [entry('shared:id', 'existing')] } }, + { Stop: [entry('shared:id', 'desired')] } + ); + assert.deepStrictEqual(result.settings.hooks, { + SessionStart: [entry('shared:id', 'existing')], + Stop: [entry('shared:id', 'desired')], + }); + })) passed++; else failed++; + + if (test('repair mode overwrites a drifted same-event managed id and preserves neighbors', () => { + const userEntry = { id: 'user:hook', command: 'keep-me' }; + const result = repairManagedHooks( + { hooks: { Stop: [userEntry, entry('ecc:stop', 'drifted')] } }, + { Stop: [entry('ecc:stop', 'repaired')] } + ); + + assert.deepStrictEqual(result.settings.hooks.Stop, [ + userEntry, + entry('ecc:stop', 'repaired'), + ]); + assert.deepStrictEqual(result.updated, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('inspect reports exact, missing, and drifted managed entries plus the actual subset', () => { + const expected = { + SessionStart: [entry('ecc:start', 'start')], + Stop: [ + entry('ecc:stop', 'expected'), + entry('ecc:missing', 'missing'), + ], + }; + const actualStart = entry('ecc:start', 'start'); + const actualDrift = entry('ecc:stop', 'changed'); + const result = inspectManagedHooks({ + hooks: { + SessionStart: [actualStart, { id: 'user:start', command: 'user' }], + Stop: [actualDrift], + }, + }, expected); + + assert.strictEqual(result.status, 'missing'); + assert.deepStrictEqual(result.matched, [{ event: 'SessionStart', id: 'ecc:start' }]); + assert.deepStrictEqual(result.missing, [{ event: 'Stop', id: 'ecc:missing' }]); + assert.deepStrictEqual(result.drifted, [{ + event: 'Stop', + id: 'ecc:stop', + expected: expected.Stop[0], + actual: actualDrift, + }]); + assert.deepStrictEqual(result.managedHooks, { + SessionStart: [actualStart], + Stop: [actualDrift], + }); + })) passed++; else failed++; + + if (test('inspect fails closed on duplicate matching ids in one settings event', () => { + assert.throws( + () => inspectManagedHooks( + { hooks: { Stop: [entry('ecc:stop', 'a'), entry('ecc:stop', 'b')] } }, + { Stop: [entry('ecc:stop', 'expected')] } + ), + /multiple.*ecc:stop/i + ); + })) passed++; else failed++; + + if (test('inspect and uninstall key ownership by event plus id', () => { + const recorded = { Stop: [entry('ecc:stop', 'managed')] }; + const moved = { hooks: { SessionStart: [entry('ecc:stop', 'managed')] } }; + + const inspection = inspectManagedHooks(moved, recorded); + assert.strictEqual(inspection.status, 'missing'); + assert.deepStrictEqual(inspection.missing, [{ event: 'Stop', id: 'ecc:stop' }]); + + const uninstall = uninstallManagedHooks(moved, recorded); + assert.deepStrictEqual(uninstall.settings, moved); + assert.deepStrictEqual(uninstall.missing, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('uninstall removes exact recorded entries, retains drift, and cleans empty events', () => { + const recorded = { + SessionStart: [entry('ecc:start', 'start')], + Stop: [entry('ecc:stop', 'recorded')], + Notification: [entry('ecc:notify', 'notify')], + }; + const userEntry = { matcher: 'Bash', hooks: [{ type: 'command', command: 'user' }] }; + const driftedStop = entry('ecc:stop', 'customer-edit'); + const settings = { + theme: 'dark', + hooks: { + SessionStart: [recorded.SessionStart[0]], + Stop: [userEntry, driftedStop], + Notification: [recorded.Notification[0]], + }, + }; + const before = clone(settings); + + const result = uninstallManagedHooks(settings, recorded); + + assert.deepStrictEqual(settings, before); + assert.deepStrictEqual(result.settings, { + theme: 'dark', + hooks: { + Stop: [userEntry, driftedStop], + }, + }); + assert.deepStrictEqual(result.removed, [ + { event: 'SessionStart', id: 'ecc:start' }, + { event: 'Notification', id: 'ecc:notify' }, + ]); + assert.deepStrictEqual(result.retained, [{ + event: 'Stop', + id: 'ecc:stop', + expected: recorded.Stop[0], + actual: driftedStop, + reason: 'modified', + }]); + })) passed++; else failed++; + + if (test('uninstall removes consecutive managed hooks without deleting a user neighbor', () => { + const recorded = { Stop: [entry('ecc:a', 'a'), entry('ecc:b', 'b')] }; + const userEntry = entry('user:c', 'keep-user'); + const result = uninstallManagedHooks({ + hooks: { Stop: [...recorded.Stop, userEntry] }, + }, recorded); + + assert.deepStrictEqual(result.settings.hooks.Stop, [userEntry]); + assert.deepStrictEqual(result.removed, [ + { event: 'Stop', id: 'ecc:a' }, + { event: 'Stop', id: 'ecc:b' }, + ]); + })) passed++; else failed++; + + if (test('uninstall removes hooks entirely after the final managed event is emptied', () => { + const recorded = { Stop: [entry('ecc:stop', 'recorded')] }; + const result = uninstallManagedHooks({ theme: 'dark', hooks: clone(recorded) }, recorded); + + assert.deepStrictEqual(result.settings, { theme: 'dark' }); + assert.deepStrictEqual(result.removed, [{ event: 'Stop', id: 'ecc:stop' }]); + assert.deepStrictEqual(result.retained, []); + })) passed++; else failed++; + + if (test('all settings transforms reject non-array hook events before changing data', () => { + const settings = { hooks: { Stop: 'invalid' } }; + const managed = { Stop: [entry('ecc:stop', 'expected')] }; + + assert.throws(() => mergeManagedHooks(settings, managed), /hooks\.Stop.*array/); + assert.throws(() => repairManagedHooks(settings, managed), /hooks\.Stop.*array/); + assert.throws(() => inspectManagedHooks(settings, managed), /hooks\.Stop.*array/); + assert.throws(() => uninstallManagedHooks(settings, managed), /hooks\.Stop.*array/); + })) passed++; else failed++; + + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); + process.exit(failed > 0 ? 1 : 0); +} + +runTests(); diff --git a/tests/lib/hook-consent.test.js b/tests/lib/hook-consent.test.js index 716a91b3a..2f44361e2 100644 --- a/tests/lib/hook-consent.test.js +++ b/tests/lib/hook-consent.test.js @@ -27,10 +27,23 @@ function test(name, fn) { } function buildHookPlan() { + const managedHooks = { + SessionStart: [{ + id: 'session:start', + matcher: '.*', + hooks: [{ type: 'command', command: 'node /target/scripts/hooks/session-start.js' }], + }], + }; return { operations: [ { kind: 'copy-file', moduleId: 'rules-core', sourceRelativePath: 'rules/common.md', destinationPath: '/target/rules/common.md' }, - { kind: 'copy-file', moduleId: 'hooks-runtime', sourceRelativePath: 'hooks/hooks.json', destinationPath: '/target/hooks/hooks.json' }, + { + kind: 'update-claude-settings', + moduleId: 'hooks-runtime', + sourceRelativePath: 'hooks/hooks.json', + destinationPath: '/target/settings.json', + managedHooks, + }, { kind: 'copy-file', moduleId: 'hooks-runtime', sourceRelativePath: 'scripts/hooks/session-start.js', destinationPath: '/target/scripts/hooks/session-start.js' }, ], selectedModuleIds: ['rules-core', 'hooks-runtime'], @@ -46,7 +59,13 @@ function buildHookPlan() { }, operations: [ { kind: 'copy-file', moduleId: 'rules-core', sourceRelativePath: 'rules/common.md', destinationPath: '/target/rules/common.md' }, - { kind: 'copy-file', moduleId: 'hooks-runtime', sourceRelativePath: 'hooks/hooks.json', destinationPath: '/target/hooks/hooks.json' }, + { + kind: 'update-claude-settings', + moduleId: 'hooks-runtime', + sourceRelativePath: 'hooks/hooks.json', + destinationPath: '/target/settings.json', + managedHooks, + }, ], resolution: { selectedModules: ['rules-core', 'hooks-runtime'], skippedModules: [] }, }, @@ -69,6 +88,7 @@ function runTests() { if (test('matches hook runtime operations by module id and source path', () => { assert.strictEqual(isHookRuntimeOperation({ moduleId: 'hooks-runtime' }), true); + assert.strictEqual(isHookRuntimeOperation({ kind: 'update-claude-settings' }), true); assert.strictEqual(isHookRuntimeOperation({ sourceRelativePath: 'hooks/hooks.json' }), true); assert.strictEqual(isHookRuntimeOperation({ sourceRelativePath: '.cursor/hooks.json' }), true); assert.strictEqual(isHookRuntimeOperation({ destinationPath: '/root/.claude/hooks/hooks.json' }), true); diff --git a/tests/lib/install-executor.test.js b/tests/lib/install-executor.test.js index 4a65ce5ef..d0acad15a 100644 --- a/tests/lib/install-executor.test.js +++ b/tests/lib/install-executor.test.js @@ -9,6 +9,7 @@ const crypto = require('crypto'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { spawnSync } = require('child_process'); const { applyInstallPlan, @@ -19,6 +20,7 @@ const { listAvailableLanguages, } = require('../../scripts/lib/install-executor'); const { applyInstallPlan: applyInstallPlanDirect } = require('../../scripts/lib/install/apply'); +const { withHookConsent } = require('../../scripts/lib/install/hook-consent'); const REPO_ROOT = path.resolve(__dirname, '..', '..'); @@ -166,6 +168,98 @@ function runTests() { } })) passed++; else failed++; + if (test('Claude settings write preserves unrelated changes made after preflight', () => { + const tempDir = createTempDir('install-executor-settings-race-'); + try { + const homeDir = path.join(tempDir, 'home'); + const projectRoot = path.join(tempDir, 'project'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + const rawPlan = createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + moduleIds: ['hooks-runtime'], + }); + const plan = { + ...rawPlan, + hookConsent: 'enabled', + statePreview: { + ...rawPlan.statePreview, + request: { ...rawPlan.statePreview.request, hookConsent: 'enabled' }, + }, + }; + const settingsPath = path.join(homeDir, '.claude', 'settings.json'); + + applyInstallPlanDirect(plan, { + beforeOperationWrite({ operation }) { + if (operation.kind === 'update-claude-settings') { + fs.writeFileSync(settingsPath, '{"theme":"added-after-preflight"}\n'); + } + }, + }); + + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + assert.strictEqual(settings.theme, 'added-after-preflight'); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + } finally { + cleanup(tempDir); + } + })) passed++; else failed++; + + if (test('failed hook disable checkpoints the previous enabled consent state', () => { + const tempDir = createTempDir('install-executor-disable-failure-'); + try { + const homeDir = path.join(tempDir, 'home'); + const projectRoot = path.join(tempDir, 'project'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + const enabledPlan = withHookConsent(createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + profileId: 'core', + }), 'enabled'); + applyInstallPlanDirect(enabledPlan); + + const declinedPlan = withHookConsent(createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + profileId: 'core', + }), 'declined'); + let injectedFailure = false; + assert.throws( + () => applyInstallPlanDirect(declinedPlan, { + beforeOperationWrite({ operation }) { + if (!injectedFailure && operation.kind === 'copy-file') { + injectedFailure = true; + throw new Error('injected copy failure'); + } + }, + }), + /injected copy failure/ + ); + + const state = JSON.parse(fs.readFileSync(declinedPlan.installStatePath, 'utf8')); + assert.strictEqual(state.request.hookConsent, 'enabled'); + assert.ok(state.resolution.selectedModules.includes('hooks-runtime')); + assert.ok(state.operations.some(operation => ( + operation.kind === 'update-claude-settings' + ))); + const settings = JSON.parse(fs.readFileSync( + path.join(homeDir, '.claude', 'settings.json'), + 'utf8' + )); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + } finally { + cleanup(tempDir); + } + })) passed++; else failed++; + if (test('rejects unknown legacy install targets before planning', () => { assert.throws( () => createLegacyInstallPlan({ target: 'not-a-target' }), @@ -400,6 +494,80 @@ function runTests() { } })) passed++; else failed++; + if (test('plans one resolved Claude settings hook registration for home and project targets', () => { + const tempDir = createTempDir('install-executor-claude-hooks-'); + try { + for (const target of ['claude', 'claude-project']) { + const homeDir = path.join(tempDir, `${target} home "quoted" $dollar %percent%`); + const projectRoot = path.join(tempDir, `${target} project "quoted" $dollar %percent%`); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + + const plan = createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target, + moduleIds: ['hooks-runtime'], + }); + const expectedRoot = target === 'claude' + ? path.join(homeDir, '.claude') + : path.join(projectRoot, '.claude'); + const settingsOperations = plan.operations.filter(operation => ( + operation.kind === 'update-claude-settings' + )); + + assert.strictEqual(settingsOperations.length, 1, `${target} should plan one settings update`); + const operation = settingsOperations[0]; + assert.strictEqual(operation.moduleId, 'hooks-runtime'); + assert.strictEqual( + operation.sourceRelativePath.split(path.sep).join('/'), + 'hooks/hooks.json' + ); + assert.strictEqual(operation.destinationPath, path.join(expectedRoot, 'settings.json')); + assert.ok(operation.managedHooks); + assert.ok(operation.managedHooks.SessionStart.some(entry => ( + entry.id === 'session:start' + ))); + const commands = Object.values(operation.managedHooks) + .flat() + .flatMap(entry => entry.hooks || []) + .map(hook => hook.command) + .filter(command => typeof command === 'string'); + const encodedRoot = Buffer.from(expectedRoot, 'utf8').toString('base64'); + assert.ok(commands.some(command => command.includes(encodedRoot))); + assert.ok(commands.every(command => !command.includes(expectedRoot))); + assert.ok( + commands.every(command => !command.includes('var e=process.env.CLAUDE_PLUGIN_ROOT;')), + `${target} commands should not depend on an unset CLAUDE_PLUGIN_ROOT` + ); + if (process.platform !== 'win32') { + for (const command of commands) { + const syntaxCheck = spawnSync('/bin/sh', ['-n', '-c', command], { + encoding: 'utf8', + }); + assert.strictEqual( + syntaxCheck.status, + 0, + `${target} hook command should remain shell-safe: ${syntaxCheck.stderr}` + ); + } + } + assert.ok(!plan.operations.some(candidate => ( + candidate.kind === 'copy-file' + && candidate.sourceRelativePath.split(path.sep).join('/') === 'hooks/hooks.json' + ))); + + const stateOperation = plan.statePreview.operations.find(candidate => ( + candidate.kind === 'update-claude-settings' + )); + assert.deepStrictEqual(stateOperation.managedHooks, operation.managedHooks); + } + } finally { + cleanup(tempDir); + } + })) passed++; else failed++; + if (test('creates legacy compatibility manifest plans from language selections', () => { const projectRoot = createTempDir('install-executor-project-'); const homeDir = createTempDir('install-executor-home-'); diff --git a/tests/lib/install-lifecycle.test.js b/tests/lib/install-lifecycle.test.js index 51d39f9e1..4e45c9597 100644 --- a/tests/lib/install-lifecycle.test.js +++ b/tests/lib/install-lifecycle.test.js @@ -23,6 +23,7 @@ const { readInstallState, writeInstallState, } = require('../../scripts/lib/install-state'); +const { materializeManagedHooks } = require('../../scripts/lib/install/claude-settings'); const REPO_ROOT = path.join(__dirname, '..', '..'); const CURRENT_PACKAGE_VERSION = JSON.parse( @@ -52,6 +53,10 @@ function cleanup(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } +function formatJson(value) { + return `${JSON.stringify(value, null, 2)}\n`; +} + function writeState(filePath, options) { const state = createInstallState(options); writeInstallState(filePath, state); @@ -100,6 +105,61 @@ function writeCursorState(projectRoot, overrides = {}) { }; } +function writeClaudeState(homeDir, overrides = {}) { + const targetRoot = overrides.targetRoot || path.join(homeDir, '.claude'); + const installStatePath = overrides.installStatePath + || path.join(targetRoot, 'ecc', 'install-state.json'); + const options = { + adapter: { id: 'claude-home', target: 'claude', kind: 'home' }, + targetRoot, + installStatePath, + request: { + profile: null, + modules: [], + includeComponents: [], + excludeComponents: [], + legacyLanguages: [], + legacyMode: true, + hookConsent: 'enabled', + ...(overrides.request || {}), + }, + resolution: { + selectedModules: ['legacy-claude-install'], + skippedModules: [], + ...(overrides.resolution || {}), + }, + operations: overrides.operations || [], + source: { + repoVersion: CURRENT_PACKAGE_VERSION, + repoCommit: 'abc123', + manifestVersion: CURRENT_MANIFEST_VERSION, + ...(overrides.source || {}), + }, + }; + + writeState(installStatePath, options); + return { + targetRoot, + installStatePath, + state: options, + }; +} + +function managedHookEntry(id, command) { + return { + id, + matcher: '.*', + hooks: [{ type: 'command', command }], + }; +} + +function currentManagedHooks(targetRoot) { + return materializeManagedHooks( + JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'hooks', 'hooks.json'), 'utf8')), + targetRoot + ); +} + function createOpencodeStateOptions(homeDir, overrides = {}) { const targetRoot = overrides.targetRoot || path.join(homeDir, '.config', 'opencode'); const installStatePath = overrides.installStatePath || path.join(targetRoot, 'ecc-install-state.json'); @@ -171,8 +231,10 @@ function withTemporarilyMovedPath(filePath, callback) { function managedOperation(kind, destinationPath, overrides = {}) { const operation = { kind, - moduleId: 'test-module', - sourceRelativePath: 'rules/common/coding-style.md', + moduleId: kind === 'update-claude-settings' ? 'hooks-runtime' : 'test-module', + sourceRelativePath: kind === 'update-claude-settings' + ? 'hooks/hooks.json' + : 'rules/common/coding-style.md', destinationPath, strategy: kind, ownership: 'managed', @@ -3231,6 +3293,417 @@ function runTests() { } })) passed++; else failed++; + if (test('doctor inspects update-claude-settings hooks by event and id', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = currentManagedHooks(targetRoot); + const stopEntry = managedHooks.Stop[0]; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + Stop: [ + { id: 'user:stop', matcher: 'Bash', hooks: [{ type: 'command', command: 'user' }] }, + ...managedHooks.Stop, + ], + ...Object.fromEntries(Object.entries(managedHooks).filter(([event]) => event !== 'Stop')), + }, + })); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + let report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(report.results[0].status, 'ok'); + + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + ...managedHooks, + Stop: [ + { id: 'user:stop', matcher: 'Bash', hooks: [{ type: 'command', command: 'user' }] }, + { ...stopEntry, description: 'drifted' }, + ...managedHooks.Stop.slice(1), + ], + }, + })); + report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(report.results[0].status, 'warning'); + assert.ok(report.results[0].issues.some(issue => issue.code === 'drifted-managed-files')); + + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + ...managedHooks, + Stop: managedHooks.Stop.filter(entry => entry.id !== stopEntry.id), + }, + })); + report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(report.results[0].status, 'error'); + assert.ok(report.results[0].issues.some(issue => issue.code === 'missing-managed-files')); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('repair restores managed Claude hooks while preserving user settings and hooks', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const userHook = { + id: 'user:stop', + matcher: 'Bash', + hooks: [{ type: 'command', command: 'user-command' }], + }; + const managedHooks = currentManagedHooks(targetRoot); + const stopEntry = managedHooks.Stop[0]; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + ...managedHooks, + Stop: [ + userHook, + { ...stopEntry, description: 'drifted' }, + ...managedHooks.Stop.slice(1), + ], + }, + })); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'repaired'); + assert.ok(result.results[0].repairedPaths.includes(settingsPath)); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + hooks: { + ...managedHooks, + Stop: [userHook, ...managedHooks.Stop], + }, + }); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('repair creates missing Claude settings with private permissions', () => { + if (process.platform === 'win32') return; + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = currentManagedHooks(targetRoot); + fs.mkdirSync(targetRoot, { recursive: true }); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'repaired'); + assert.strictEqual(fs.statSync(settingsPath).mode & 0o777, 0o600); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')).hooks, managedHooks); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('repair removes retired managed hooks using the recorded ownership snapshot', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const currentHooks = currentManagedHooks(targetRoot); + const retiredHook = managedHookEntry('ecc:retired', 'node retired.js'); + const recordedHooks = { + ...currentHooks, + Stop: [...currentHooks.Stop, retiredHook], + }; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: recordedHooks, + })); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + managedHooks: recordedHooks, + }), + ], + }); + + const result = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'repaired'); + const repaired = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + assert.ok(!repaired.hooks.Stop.some(entry => entry.id === 'ecc:retired')); + assert.deepStrictEqual(repaired.hooks, currentHooks); + const state = readInstallState(path.join(targetRoot, 'ecc', 'install-state.json')); + const settingsOperation = state.operations.find(operation => ( + operation.kind === 'update-claude-settings' + )); + assert.deepStrictEqual(settingsOperation.managedHooks, currentHooks); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('uninstall removes only unchanged managed Claude hooks and reports drift as partial', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = { + SessionStart: [managedHookEntry('ecc:start', 'node managed-start.js')], + Stop: [managedHookEntry('ecc:stop', 'node managed-stop.js')], + }; + const userHook = { + id: 'user:stop', + matcher: 'Bash', + hooks: [{ type: 'command', command: 'user-command' }], + }; + const driftedHook = managedHookEntry('ecc:stop', 'node user-edited-stop.js'); + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + SessionStart: managedHooks.SessionStart, + Stop: [userHook, driftedHook], + }, + })); + const { installStatePath } = writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = uninstallInstalledStates({ + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'partial'); + assert.deepStrictEqual(result.results[0].retainedPaths, [settingsPath]); + assert.ok(fs.existsSync(installStatePath)); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + hooks: { + Stop: [userHook, driftedHook], + }, + }); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('uninstall clears empty hook containers but preserves unrelated Claude settings', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = { + Stop: [managedHookEntry('ecc:stop', 'node managed-stop.js')], + }; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: managedHooks, + })); + const { installStatePath } = writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = uninstallInstalledStates({ + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'uninstalled'); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + }); + assert.ok(!fs.existsSync(installStatePath)); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('Claude settings lifecycle refuses a final-symlink destination', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const victimPath = path.join(targetRoot, 'victim.json'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = { + Stop: [managedHookEntry('ecc:stop', 'node managed-stop.js')], + }; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(victimPath, formatJson({ sentinel: true, hooks: managedHooks })); + try { + fs.symlinkSync(victimPath, settingsPath, 'file'); + } catch { + console.log(' (file symlink unsupported on this platform; skipping)'); + return; + } + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const doctor = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + const repair = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + const uninstall = uninstallInstalledStates({ + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(doctor.results[0].status, 'error'); + assert.ok(doctor.results[0].issues.some(issue => ( + issue.code === 'unsafe-managed-destination' + || issue.code === 'invalid-install-state' + ))); + assert.strictEqual(repair.results[0].status, 'error'); + assert.match(repair.results[0].error, /final symlink/); + assert.strictEqual(uninstall.results[0].status, 'error'); + assert.match(uninstall.results[0].error, /final symlink/); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(victimPath, 'utf8')), { + sentinel: true, + hooks: managedHooks, + }); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('Claude settings lifecycle refuses a non-canonical settings destination', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const destinationPath = path.join(targetRoot, 'settings.local.json'); + const managedHooks = currentManagedHooks(targetRoot); + fs.mkdirSync(targetRoot, { recursive: true }); + assert.throws( + () => writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', destinationPath, { + managedHooks, + }), + ], + }), + /canonical Claude settings path/ + ); + assert.ok(!fs.existsSync(destinationPath)); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); } diff --git a/tests/lib/install-state.test.js b/tests/lib/install-state.test.js index 8baa6c3e5..ba8effaea 100644 --- a/tests/lib/install-state.test.js +++ b/tests/lib/install-state.test.js @@ -84,6 +84,115 @@ function runTests() { assert.strictEqual(state.operations.length, 1); })) passed++; else failed++; + if (test('validates managed hook metadata for Claude settings operations', () => { + const baseOptions = { + adapter: { id: 'claude-home', target: 'claude', kind: 'home' }, + targetRoot: '/home/test/.claude', + installStatePath: '/home/test/.claude/ecc/install-state.json', + request: { + profile: 'core', + modules: ['hooks-runtime'], + includeComponents: [], + excludeComponents: [], + legacyLanguages: [], + legacyMode: false, + hookConsent: 'enabled', + }, + resolution: { selectedModules: ['hooks-runtime'], skippedModules: [] }, + source: { repoVersion: CURRENT_PACKAGE_VERSION, repoCommit: 'abc123', manifestVersion: 1 }, + }; + const operation = { + kind: 'update-claude-settings', + moduleId: 'hooks-runtime', + sourceRelativePath: 'hooks/hooks.json', + destinationPath: '/home/test/.claude/settings.json', + strategy: 'merge-hook-ids', + ownership: 'managed', + scaffoldOnly: false, + managedHooks: { + SessionStart: [{ + id: 'session:start', + matcher: '.*', + hooks: [{ type: 'command', command: 'node start.js' }], + }], + }, + }; + + assert.doesNotThrow(() => createInstallState({ ...baseOptions, operations: [operation] })); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ ...operation, moduleId: 'not-hooks-runtime' }], + }), + /moduleId.*hooks-runtime/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ ...operation, sourceRelativePath: 'attacker.json' }], + }), + /sourceRelativePath.*hooks\/hooks\.json/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + destinationPath: '/home/test/.claude/settings.local.json', + }], + }), + /destinationPath.*canonical Claude settings path/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + adapter: { id: 'cursor-project', target: 'cursor', kind: 'project' }, + targetRoot: '/repo/.cursor', + installStatePath: '/repo/.cursor/ecc-install-state.json', + operations: [{ + ...operation, + destinationPath: '/repo/.cursor/settings.json', + }], + }), + /only valid for Claude targets/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + managedHooks: { + SessionStart: [{ + matcher: '.*', + hooks: [{ type: 'command', command: 'node start.js' }], + }], + }, + }], + }), + /managedHooks.*non-empty unique id/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + managedHooks: { + SessionStart: [{ + id: 'duplicate', + matcher: '.*', + hooks: [{ type: 'command', command: 'node start.js' }], + }], + Stop: [{ + id: 'duplicate', + hooks: [{ type: 'command', command: 'node stop.js' }], + }], + }, + }], + }), + /managedHooks.*globally unique id/ + ); + })) passed++; else failed++; + if (test('writes and reads install-state from disk', () => { const testDir = createTestDir(); const statePath = path.join(testDir, 'ecc-install-state.json'); diff --git a/tests/scripts/install-apply.test.js b/tests/scripts/install-apply.test.js index 0931d5c0a..270339cb9 100644 --- a/tests/scripts/install-apply.test.js +++ b/tests/scripts/install-apply.test.js @@ -115,6 +115,31 @@ function runTests() { assert.ok(result.stdout.includes('--modules ')); })) passed++; else failed++; + if (test('Claude hook dry-run validates settings without mutating malformed input', () => { + const homeDir = createTempDir('install-apply-home-'); + const projectDir = createTempDir('install-apply-project-'); + const claudeRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(claudeRoot, 'settings.json'); + + try { + fs.mkdirSync(claudeRoot, { recursive: true }); + fs.writeFileSync(settingsPath, '{ malformed\n'); + + const result = run( + ['--profile', 'core', '--enable-hooks', '--dry-run', '--json'], + { cwd: projectDir, homeDir } + ); + + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /Failed to parse Claude settings/); + assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ malformed\n'); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); + } finally { + cleanup(homeDir); + cleanup(projectDir); + } + })) passed++; else failed++; + if (test('guided dispatcher reports sanitized load and rejection failures', () => { for (const failureMode of ['load', 'reject']) { const result = runWithGuidedDispatcherFailure(failureMode); @@ -487,6 +512,19 @@ function runTests() { const parsed = JSON.parse(result.stdout); assert.strictEqual(parsed.dryRun, true); assert.ok(parsed.plan.selectedModuleIds.includes('workflow-quality')); + const settingsOperations = parsed.plan.operations.filter(operation => ( + operation.kind === 'update-claude-settings' + )); + assert.strictEqual(settingsOperations.length, 1); + assert.strictEqual( + settingsOperations[0].destinationPath, + path.join(homeDir, '.claude', 'settings.json') + ); + assert.ok(settingsOperations[0].managedHooks.SessionStart); + assert.ok(!parsed.plan.operations.some(operation => ( + operation.kind === 'copy-file' + && String(operation.sourceRelativePath || '').replace(/\\/g, '/') === 'hooks/hooks.json' + ))); assert.ok( parsed.plan.operations.some(operation => ( String(operation.sourceRelativePath || '').replace(/\\/g, '/').startsWith('skills/delivery-gate/') @@ -532,7 +570,8 @@ function runTests() { assert.ok(fs.existsSync(path.join(claudeRoot, 'rules', 'ecc', 'common', 'coding-style.md'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'agents', 'architect.md'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'commands', 'plan.md'))); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json'))); + assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json'))); + assert.ok(readJson(path.join(claudeRoot, 'settings.json')).hooks.SessionStart); assert.ok(fs.existsSync(path.join(claudeRoot, 'scripts', 'hooks', 'session-end.js'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'scripts', 'lib', 'session-manager.js'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'plugin.json'))); @@ -747,7 +786,7 @@ function runTests() { assert.ok(result.stderr.includes('Unknown install module: ghost-module')); })) passed++; else failed++; - if (test('installs claude hooks and defaults commit attribution off', () => { + if (test('registers Claude hooks in settings and defaults commit attribution off', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -756,58 +795,87 @@ function runTests() { assert.strictEqual(result.code, 0, result.stderr); const claudeRoot = path.join(homeDir, '.claude'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should be copied'); - assert.deepStrictEqual( - readJson(path.join(claudeRoot, 'settings.json')), - { includeCoAuthoredBy: false } + assert.strictEqual( + fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), + false, + 'hooks.json should not be copied for Claude targets' ); + const settings = readJson(path.join(claudeRoot, 'settings.json')); + assert.strictEqual(settings.includeCoAuthoredBy, false); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + + const state = readJson(path.join(claudeRoot, 'ecc', 'install-state.json')); + const settingsOperation = state.operations.find(operation => ( + operation.kind === 'update-claude-settings' + )); + assert.ok(settingsOperation, 'state should record the settings update operation'); + assert.deepStrictEqual(settingsOperation.managedHooks, settings.hooks); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('installs claude hooks with the safe plugin bootstrap contract', () => { - const homeDir = createTempDir('install-apply-home-'); - const projectDir = createTempDir('install-apply-project-'); + if (test('resolves Claude home and project hook commands to their installed roots', () => { + for (const target of ['claude', 'claude-project']) { + const homeDir = createTempDir(`install-apply-${target}-home-`); + const projectDir = createTempDir(`install-apply-${target}-project-`); - try { - const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); - assert.strictEqual(result.code, 0, result.stderr); + try { + const result = run( + ['--target', target, '--profile', 'core', '--enable-hooks'], + { cwd: projectDir, homeDir } + ); + assert.strictEqual(result.code, 0, result.stderr); - const claudeRoot = path.join(homeDir, '.claude'); - const installedHooks = readJson(path.join(claudeRoot, 'hooks', 'hooks.json')); + const claudeRoot = target === 'claude' + ? path.join(homeDir, '.claude') + : path.join(projectDir, '.claude'); + const settings = readJson(path.join(claudeRoot, 'settings.json')); + const state = readJson(path.join(claudeRoot, 'ecc', 'install-state.json')); + const installedRoot = state.target.root; + assert.strictEqual(fs.realpathSync(installedRoot), fs.realpathSync(claudeRoot)); + const installedBashDispatcherEntry = settings.hooks.PreToolUse.find( + entry => entry.id === 'pre:bash:dispatcher' + ); + assert.ok(installedBashDispatcherEntry); + const command = installedBashDispatcherEntry.hooks[0].command; + assert.ok(command.startsWith('node -e ')); + assert.ok(command.includes('plugin-hook-bootstrap.js')); + assert.ok(command.includes('pre-bash-dispatcher.js')); + assert.ok( + command.includes(Buffer.from(installedRoot, 'utf8').toString('base64')), + `${target} command should encode its absolute root without shell interpolation` + ); + assert.ok(!command.includes(claudeRoot)); + assert.ok(!command.includes('var e=process.env.CLAUDE_PLUGIN_ROOT;')); + assert.ok(!command.includes('${CLAUDE_PLUGIN_ROOT}')); - const installedBashDispatcherEntry = installedHooks.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:dispatcher'); - assert.ok(installedBashDispatcherEntry, 'hooks/hooks.json should include the consolidated Bash dispatcher hook'); - assert.strictEqual(typeof installedBashDispatcherEntry.hooks[0].command, 'string', 'hooks/hooks.json should install string-form commands for Claude Code schema compatibility'); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.startsWith('node -e '), - 'hooks/hooks.json should use the inline node bootstrap contract' - ); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.includes('plugin-hook-bootstrap.js'), - 'hooks/hooks.json should route plugin-managed hooks through the shared bootstrap' - ); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.includes('CLAUDE_PLUGIN_ROOT'), - 'hooks/hooks.json should still consult CLAUDE_PLUGIN_ROOT for runtime resolution' - ); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.includes('pre-bash-dispatcher.js'), - 'hooks/hooks.json should point the Bash preflight contract at the consolidated dispatcher' - ); - assert.ok( - !installedBashDispatcherEntry.hooks[0].command.includes('\\"'), - 'hooks/hooks.json should avoid escaped double quotes that break Windows Git Bash parsing' - ); - assert.ok( - !installedBashDispatcherEntry.hooks[0].command.includes('${CLAUDE_PLUGIN_ROOT}'), - 'hooks/hooks.json should not retain raw CLAUDE_PLUGIN_ROOT shell placeholders after install' - ); - } finally { - cleanup(homeDir); - cleanup(projectDir); + const smokeEntry = settings.hooks.PreToolUse.find( + entry => entry.id === 'pre:write:doc-file-warning' + ); + const smokeResult = spawnSync(smokeEntry.hooks[0].command, { + input: JSON.stringify({ + hook_event_name: 'PreToolUse', + tool_name: 'Write', + tool_input: { file_path: 'README.md' }, + }), + encoding: 'utf8', + cwd: projectDir, + env: { + ...process.env, + HOME: homeDir, + USERPROFILE: homeDir, + ECC_DISABLED_HOOKS: 'pre:write:doc-file-warning', + }, + shell: true, + timeout: DEFAULT_INSTALL_APPLY_TIMEOUT_MS, + }); + assert.strictEqual(smokeResult.status, 0, smokeResult.stderr); + } finally { + cleanup(homeDir); + cleanup(projectDir); + } } })) passed++; else failed++; @@ -840,12 +908,16 @@ function runTests() { assert.deepStrictEqual( settings.hooks.UserPromptSubmit, [{ matcher: '*', hooks: [{ type: 'command', command: 'echo custom-submit' }] }], - 'existing hooks should be left untouched' + 'unrelated existing hooks should be preserved' ); assert.deepStrictEqual( - settings.hooks.PreToolUse, - [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }], - 'managed Claude hooks should not be injected into settings.json' + settings.hooks.PreToolUse[0], + { matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }, + 'existing event entries should retain their order and content' + ); + assert.ok( + settings.hooks.PreToolUse.some(entry => entry.id === 'pre:bash:dispatcher'), + 'managed Claude hooks should be registered alongside user hooks' ); } finally { cleanup(homeDir); @@ -927,7 +999,7 @@ function runTests() { } })) passed++; else failed++; - if (test('reinstall keeps commit attribution disabled when only managed hooks are installed', () => { + if (test('reinstall is idempotent for managed hooks and keeps commit attribution disabled', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -938,17 +1010,17 @@ function runTests() { const secondInstall = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); assert.strictEqual(secondInstall.code, 0, secondInstall.stderr); - assert.deepStrictEqual( - readJson(path.join(homeDir, '.claude', 'settings.json')), - { includeCoAuthoredBy: false } - ); + const settings = readJson(path.join(homeDir, '.claude', 'settings.json')); + assert.strictEqual(settings.includeCoAuthoredBy, false); + const ids = Object.values(settings.hooks).flat().map(entry => entry.id); + assert.strictEqual(ids.length, new Set(ids).size, 'managed hook IDs should not duplicate'); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('reinstall leaves pre-existing hook-based settings.json untouched apart from co-author preference', () => { + if (test('reinstall preserves pre-existing hook entries while registering managed hooks', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -967,10 +1039,9 @@ function runTests() { assert.strictEqual(secondInstall.code, 0, secondInstall.stderr); const afterSecondInstall = readJson(settingsPath); - assert.deepStrictEqual(afterSecondInstall, { - ...legacySettings, - includeCoAuthoredBy: false, - }); + assert.strictEqual(afterSecondInstall.includeCoAuthoredBy, false); + assert.deepStrictEqual(afterSecondInstall.hooks.PreToolUse[0], legacySettings.hooks.PreToolUse[0]); + assert.ok(afterSecondInstall.hooks.PreToolUse.some(entry => entry.id === 'pre:bash:dispatcher')); } finally { cleanup(homeDir); cleanup(projectDir); @@ -995,7 +1066,9 @@ function runTests() { assert.strictEqual(install.code, 0, install.stderr); const afterInstall = readJson(settingsPath); - assert.deepStrictEqual(afterInstall, customSettings); + assert.strictEqual(afterInstall.includeCoAuthoredBy, true); + assert.strictEqual(afterInstall.theme, 'dark'); + assert.ok(afterInstall.hooks.SessionStart.some(entry => entry.id === 'session:start')); } finally { cleanup(homeDir); cleanup(projectDir); @@ -1022,14 +1095,17 @@ function runTests() { assert.strictEqual(install.code, 0, install.stderr); const afterInstall = readJson(settingsPath); - assert.deepStrictEqual(afterInstall, customSettings); + assert.deepStrictEqual(afterInstall.attribution, customSettings.attribution); + assert.strictEqual(afterInstall.theme, 'dark'); + assert.ok(!Object.hasOwn(afterInstall, 'includeCoAuthoredBy')); + assert.ok(afterInstall.hooks.SessionStart.some(entry => entry.id === 'session:start')); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('ignores malformed existing settings.json during claude install', () => { + if (test('malformed Claude settings aborts before any install mutation', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -1040,17 +1116,17 @@ function runTests() { fs.writeFileSync(settingsPath, '{ invalid json\n'); const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); - assert.strictEqual(result.code, 0, result.stderr); + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /Failed to parse Claude settings/); assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written'); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('ignores non-object existing settings.json during claude install', () => { + if (test('non-object Claude settings aborts before any install mutation', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -1061,76 +1137,44 @@ function runTests() { fs.writeFileSync(settingsPath, '[]\n'); const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); - assert.strictEqual(result.code, 0, result.stderr); + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /expected a JSON object/); assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written'); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('fails when source hooks.json root is not an object before copying files', () => { - const tempDir = createTempDir('install-apply-invalid-hooks-'); - const targetRoot = path.join(tempDir, '.claude'); - const installStatePath = path.join(targetRoot, 'ecc', 'install-state.json'); - const sourceHooksPath = path.join(tempDir, 'hooks.json'); + if (test('same-id Claude hook conflict aborts before any install mutation', () => { + const homeDir = createTempDir('install-apply-home-'); + const projectDir = createTempDir('install-apply-project-'); try { - fs.writeFileSync(sourceHooksPath, '[]\n'); - - assert.throws(() => { - applyInstallPlan({ - targetRoot, - installStatePath, - hookConsent: 'enabled', - statePreview: { - schemaVersion: 'ecc.install.v1', - installedAt: new Date().toISOString(), - target: { - id: 'claude-home', - kind: 'home', - root: targetRoot, - installStatePath, - }, - request: { - profile: 'core', - modules: [], - includeComponents: [], - excludeComponents: [], - legacyLanguages: [], - legacyMode: false, - }, - resolution: { - selectedModules: ['hooks-runtime'], - skippedModules: [], - }, - source: { - repoVersion: null, - repoCommit: null, - manifestVersion: 1, - }, - operations: [], - }, - adapter: { target: 'claude' }, - operations: [{ - kind: 'copy-file', - moduleId: 'hooks-runtime', - sourcePath: sourceHooksPath, - sourceRelativePath: 'hooks/hooks.json', - destinationPath: path.join(targetRoot, 'hooks', 'hooks.json'), - strategy: 'preserve-relative-path', - ownership: 'managed', - scaffoldOnly: false, + const claudeRoot = path.join(homeDir, '.claude'); + fs.mkdirSync(claudeRoot, { recursive: true }); + const settingsPath = path.join(claudeRoot, 'settings.json'); + const existing = { + theme: 'dark', + hooks: { + PreToolUse: [{ + id: 'pre:bash:dispatcher', + matcher: 'Bash', + hooks: [{ type: 'command', command: 'echo user-owned' }], }], - }); - }, /Invalid hooks config at .*expected a JSON object/); + }, + }; + fs.writeFileSync(settingsPath, `${JSON.stringify(existing, null, 2)}\n`); - assert.ok(!fs.existsSync(path.join(targetRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied when source hooks are invalid'); - assert.ok(!fs.existsSync(installStatePath), 'install state should not be written when source hooks are invalid'); + const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /Refusing to overwrite.*pre:bash:dispatcher/); + assert.deepStrictEqual(readJson(settingsPath), existing); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); } finally { - cleanup(tempDir); + cleanup(homeDir); + cleanup(projectDir); } })) passed++; else failed++; @@ -1254,6 +1298,39 @@ function runTests() { assert.strictEqual(state.request.hookConsent, 'declined'); assert.ok(!state.resolution.selectedModules.includes('hooks-runtime')); assert.ok(state.resolution.selectedModules.includes('rules-core')); + assert.ok(!state.operations.some(operation => ( + operation.kind === 'update-claude-settings' + ))); + } finally { + cleanup(homeDir); + cleanup(projectDir); + } + })) passed++; else failed++; + + if (test('--no-hooks removes hooks registered by a previous enabled install', () => { + const projectDir = createTempDir('install-apply-disable-hooks-'); + const homeDir = createTempDir('install-apply-disable-hooks-home-'); + try { + const enabled = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); + assert.strictEqual(enabled.code, 0, enabled.stderr); + + const settingsPath = path.join(homeDir, '.claude', 'settings.json'); + const settings = readJson(settingsPath); + settings.theme = 'dark'; + fs.writeFileSync(settingsPath, `${JSON.stringify(settings, null, 2)}\n`); + + const disabled = run(['--profile', 'core', '--no-hooks'], { cwd: projectDir, homeDir }); + assert.strictEqual(disabled.code, 0, disabled.stderr); + assert.deepStrictEqual(readJson(settingsPath), { + includeCoAuthoredBy: false, + theme: 'dark', + }); + + const state = readJson(path.join(homeDir, '.claude', 'ecc', 'install-state.json')); + assert.strictEqual(state.request.hookConsent, 'declined'); + assert.ok(!state.operations.some(operation => ( + operation.kind === 'update-claude-settings' + ))); } finally { cleanup(homeDir); cleanup(projectDir); diff --git a/tests/scripts/manual-hook-install-docs.test.js b/tests/scripts/manual-hook-install-docs.test.js index 8dc531efd..f86ea670f 100644 --- a/tests/scripts/manual-hook-install-docs.test.js +++ b/tests/scripts/manual-hook-install-docs.test.js @@ -47,6 +47,10 @@ function runTests() { readme.includes('%USERPROFILE%\\\\.claude'), 'README should call out the correct Windows Claude config root' ); + assert.ok( + readme.includes('registers the resolved\nhook entries in `~/.claude/settings.json`'), + 'README should explain that manual installs register hooks in Claude settings' + ); })) passed++; else failed++; if (test('hooks/README mirrors supported manual install guidance', () => { @@ -62,6 +66,10 @@ function runTests() { hooksReadme.includes('pwsh -File .\\install.ps1 --target claude --modules hooks-runtime --enable-hooks'), 'hooks/README should document the supported PowerShell hook install path' ); + assert.ok( + hooksReadme.includes('registers the resolved\nhook entries in `~/.claude/settings.json`'), + 'hooks/README should explain that manual installs register hooks in Claude settings' + ); })) passed++; else failed++; console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);