fix: harden install and codex sync portability

This commit is contained in:
Affaan Mustafa
2026-04-01 16:04:56 -07:00
parent 401966bc18
commit dba5ae779b
5 changed files with 125 additions and 15 deletions

View File

@@ -59,6 +59,8 @@ Public ECC plugin repo for agents, skills, commands, hooks, rules, install surfa
- Direct-port candidates landed after audit:
- `#1078` hook-id dedupe for managed Claude hook reinstalls
- `#844` ui-demo skill
- `#1110` install-time Claude hook root resolution
- `#1106` portable Codex Context7 key extraction
- Port or rebuild inside ECC after full audit:
- `#894` Jira integration
- `#814` + `#808` rebuild as a single consolidated notifications lane for Opencode and cross-harness surfaces
@@ -97,3 +99,6 @@ Keep this file detailed for only the current sprint, blockers, and next actions.
- 2026-04-01: Collapsed the obvious command/skill duplicates into thin legacy shims so `skills/` now hold the maintained bodies for NanoClaw, context-budget, DevFleet, docs lookup, E2E, evals, orchestration, prompt optimization, rules distillation, TDD, and verification.
- 2026-04-01: Ported the self-contained core of `#844` directly into `main` as `skills/ui-demo/SKILL.md` and registered it under the `media-generation` install module instead of merging the PR wholesale.
- 2026-04-01: Added the first connected-workflow operator lane as ECC-native skills instead of leaving the surface as raw plugins or APIs: `workspace-surface-audit`, `customer-billing-ops`, `project-flow-ops`, and `google-workspace-ops`. These are tracked under the new `operator-workflows` install module.
- 2026-04-01: Direct-ported the real fix from the unresolved hook-path PR lane into the active installer. Claude installs now replace `${CLAUDE_PLUGIN_ROOT}` with the concrete install root in both `settings.json` and the copied `hooks/hooks.json`, which keeps PreToolUse/PostToolUse hooks working outside plugin-managed env injection.
- 2026-04-01: Replaced the GNU-only `grep -P` parser in `scripts/sync-ecc-to-codex.sh` with a portable Node parser for Context7 key extraction. Added source-level regression coverage so BSD/macOS syncs do not drift back to non-portable parsing.
- 2026-04-01: Targeted regression suite after the direct ports is green: `tests/scripts/install-apply.test.js`, `tests/scripts/sync-ecc-to-codex.test.js`, and `tests/scripts/codex-hooks.test.js`.

View File

@@ -20,16 +20,43 @@ function readJsonObject(filePath, label) {
return parsed;
}
function buildLegacyHookSignature(entry) {
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 buildLegacyHookSignature(entry, pluginRoot) {
if (!entry || typeof entry !== 'object' || Array.isArray(entry)) {
return null;
}
if (typeof entry.matcher !== 'string' || !Array.isArray(entry.hooks)) {
const normalizedEntry = replacePluginRootPlaceholders(entry, pluginRoot);
if (typeof normalizedEntry.matcher !== 'string' || !Array.isArray(normalizedEntry.hooks)) {
return null;
}
const hookSignature = entry.hooks.map(hook => JSON.stringify({
const hookSignature = normalizedEntry.hooks.map(hook => JSON.stringify({
type: hook && typeof hook === 'object' ? hook.type : undefined,
command: hook && typeof hook === 'object' ? hook.command : undefined,
timeout: hook && typeof hook === 'object' ? hook.timeout : undefined,
@@ -37,33 +64,35 @@ function buildLegacyHookSignature(entry) {
}));
return JSON.stringify({
matcher: entry.matcher,
matcher: normalizedEntry.matcher,
hooks: hookSignature,
});
}
function getHookEntryAliases(entry) {
function getHookEntryAliases(entry, pluginRoot) {
const aliases = [];
if (!entry || typeof entry !== 'object' || Array.isArray(entry)) {
return aliases;
}
if (typeof entry.id === 'string' && entry.id.trim().length > 0) {
aliases.push(`id:${entry.id.trim()}`);
const normalizedEntry = replacePluginRootPlaceholders(entry, pluginRoot);
if (typeof normalizedEntry.id === 'string' && normalizedEntry.id.trim().length > 0) {
aliases.push(`id:${normalizedEntry.id.trim()}`);
}
const legacySignature = buildLegacyHookSignature(entry);
const legacySignature = buildLegacyHookSignature(normalizedEntry, pluginRoot);
if (legacySignature) {
aliases.push(`legacy:${legacySignature}`);
}
aliases.push(`json:${JSON.stringify(entry)}`);
aliases.push(`json:${JSON.stringify(normalizedEntry)}`);
return aliases;
}
function mergeHookEntries(existingEntries, incomingEntries) {
function mergeHookEntries(existingEntries, incomingEntries, pluginRoot) {
const mergedEntries = [];
const seenEntries = new Set();
@@ -76,7 +105,7 @@ function mergeHookEntries(existingEntries, incomingEntries) {
continue;
}
const aliases = getHookEntryAliases(entry);
const aliases = getHookEntryAliases(entry, pluginRoot);
if (aliases.some(alias => seenEntries.has(alias))) {
continue;
}
@@ -84,7 +113,7 @@ function mergeHookEntries(existingEntries, incomingEntries) {
for (const alias of aliases) {
seenEntries.add(alias);
}
mergedEntries.push(entry);
mergedEntries.push(replacePluginRootPlaceholders(entry, pluginRoot));
}
return mergedEntries;
@@ -100,6 +129,7 @@ function buildMergedSettings(plan) {
return null;
}
const pluginRoot = plan.targetRoot;
const hooksDestinationPath = path.join(plan.targetRoot, 'hooks', 'hooks.json');
const hooksSourcePath = findHooksSourcePath(plan, hooksDestinationPath) || hooksDestinationPath;
if (!fs.existsSync(hooksSourcePath)) {
@@ -107,7 +137,7 @@ function buildMergedSettings(plan) {
}
const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config');
const incomingHooks = hooksConfig.hooks;
const incomingHooks = replacePluginRootPlaceholders(hooksConfig.hooks, pluginRoot);
if (!incomingHooks || typeof incomingHooks !== 'object' || Array.isArray(incomingHooks)) {
throw new Error(`Invalid hooks config at ${hooksSourcePath}: expected "hooks" to be a JSON object`);
}
@@ -126,7 +156,7 @@ function buildMergedSettings(plan) {
for (const [eventName, incomingEntries] of Object.entries(incomingHooks)) {
const currentEntries = Array.isArray(existingHooks[eventName]) ? existingHooks[eventName] : [];
const nextEntries = Array.isArray(incomingEntries) ? incomingEntries : [];
mergedHooks[eventName] = mergeHookEntries(currentEntries, nextEntries);
mergedHooks[eventName] = mergeHookEntries(currentEntries, nextEntries, pluginRoot);
}
const mergedSettings = {
@@ -137,6 +167,11 @@ function buildMergedSettings(plan) {
return {
settingsPath,
mergedSettings,
hooksDestinationPath,
resolvedHooksConfig: {
...hooksConfig,
hooks: incomingHooks,
},
};
}
@@ -149,6 +184,12 @@ function applyInstallPlan(plan) {
}
if (mergedSettingsPlan) {
fs.mkdirSync(path.dirname(mergedSettingsPlan.hooksDestinationPath), { recursive: true });
fs.writeFileSync(
mergedSettingsPlan.hooksDestinationPath,
JSON.stringify(mergedSettingsPlan.resolvedHooksConfig, null, 2) + '\n',
'utf8'
);
fs.mkdirSync(path.dirname(mergedSettingsPlan.settingsPath), { recursive: true });
fs.writeFileSync(
mergedSettingsPlan.settingsPath,

View File

@@ -106,7 +106,23 @@ extract_toml_value() {
extract_context7_key() {
local file="$1"
grep -oP -- '--key",[[:space:]]*"\K[^"]+' "$file" | head -n 1 || true
node - "$file" <<'EOF'
const fs = require('fs');
const filePath = process.argv[2];
let source = '';
try {
source = fs.readFileSync(filePath, 'utf8');
} catch {
process.exit(0);
}
const match = source.match(/--key",\s*"([^"]+)"/);
if (match && match[1]) {
process.stdout.write(`${match[1]}\n`);
}
EOF
}
generate_prompt_file() {

View File

@@ -353,6 +353,45 @@ function runTests() {
}
})) passed++; else failed++;
if (test('resolves CLAUDE_PLUGIN_ROOT placeholders in installed claude hooks', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
try {
const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(result.code, 0, result.stderr);
const claudeRoot = path.join(homeDir, '.claude');
const settings = readJson(path.join(claudeRoot, 'settings.json'));
const installedHooks = readJson(path.join(claudeRoot, 'hooks', 'hooks.json'));
const autoTmuxEntry = settings.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:auto-tmux-dev');
assert.ok(autoTmuxEntry, 'settings.json should include the auto tmux hook');
assert.ok(
autoTmuxEntry.hooks[0].command.includes(path.join(claudeRoot, 'scripts', 'hooks', 'auto-tmux-dev.js')),
'settings.json should use the installed Claude root for hook commands'
);
assert.ok(
!autoTmuxEntry.hooks[0].command.includes('${CLAUDE_PLUGIN_ROOT}'),
'settings.json should not retain CLAUDE_PLUGIN_ROOT placeholders after install'
);
const installedAutoTmuxEntry = installedHooks.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:auto-tmux-dev');
assert.ok(installedAutoTmuxEntry, 'hooks/hooks.json should include the auto tmux hook');
assert.ok(
installedAutoTmuxEntry.hooks[0].command.includes(path.join(claudeRoot, 'scripts', 'hooks', 'auto-tmux-dev.js')),
'hooks/hooks.json should use the installed Claude root for hook commands'
);
assert.ok(
!installedAutoTmuxEntry.hooks[0].command.includes('${CLAUDE_PLUGIN_ROOT}'),
'hooks/hooks.json should not retain CLAUDE_PLUGIN_ROOT placeholders after install'
);
} finally {
cleanup(homeDir);
cleanup(projectDir);
}
})) passed++; else failed++;
if (test('preserves existing settings fields and hook entries when merging hooks', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');

View File

@@ -74,6 +74,15 @@ function runTests() {
assert.ok(!source.includes('run_or_echo cp -R "$skill_dir" "$dest"'), 'skill sync cp should be removed');
})) passed++; else failed++;
if (test('sync script avoids GNU-only grep -P parsing', () => {
assert.ok(!source.includes('grep -oP'), 'sync-ecc-to-codex.sh should remain portable across BSD and GNU environments');
})) passed++; else failed++;
if (test('extract_context7_key uses a portable parser', () => {
assert.ok(source.includes('extract_context7_key() {'), 'Expected extract_context7_key helper');
assert.ok(source.includes('node - "$file"'), 'extract_context7_key should use Node-based parsing');
})) passed++; else failed++;
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
process.exit(failed > 0 ? 1 : 0);
}