From af51fcacb70d42ed52ba8111026d9b0f0e49b364 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Mon, 9 Mar 2026 22:05:35 -0700 Subject: [PATCH] fix: resolve PR 371 portability regressions --- .opencode/README.md | 2 +- README.md | 2 +- schemas/plugin.schema.json | 21 +++++- scripts/hooks/post-edit-format.js | 44 +++++++++--- scripts/hooks/pre-bash-dev-server-block.js | 21 +++--- .../scripts/detect-project.sh | 1 + tests/hooks/hooks.test.js | 71 ++++++++++++++++++- 7 files changed, 141 insertions(+), 21 deletions(-) diff --git a/.opencode/README.md b/.opencode/README.md index eaaa73a5..e8737e81 100644 --- a/.opencode/README.md +++ b/.opencode/README.md @@ -39,7 +39,7 @@ This loads the ECC OpenCode plugin module from npm: It does **not** auto-register the full ECC command/agent/instruction catalog in your project config. For the full OpenCode setup, either: - run OpenCode inside this repository, or -- copy the relevant `.opencode/commands/`, `.opencode/prompts/`, `.opencode/instructions/`, and `agent` / `command` config entries into your own project +- copy the relevant `.opencode/commands/`, `.opencode/prompts/`, `.opencode/instructions/`, and the `instructions`, `agent`, and `command` config entries into your own project After installation, the `ecc-install` CLI is also available: diff --git a/README.md b/README.md index 0426ed6c..09267ca5 100644 --- a/README.md +++ b/README.md @@ -1055,7 +1055,7 @@ It does **not** automatically add ECC's full command/agent/instruction catalog t For the full ECC OpenCode setup, either: - run OpenCode inside this repository, or -- copy the bundled `.opencode/` config assets into your project and wire the `agent` / `command` entries in `opencode.json` +- copy the bundled `.opencode/` config assets into your project and wire the `instructions`, `agent`, and `command` entries in `opencode.json` ### Documentation diff --git a/schemas/plugin.schema.json b/schemas/plugin.schema.json index 834bb984..326a5a3b 100644 --- a/schemas/plugin.schema.json +++ b/schemas/plugin.schema.json @@ -34,6 +34,25 @@ "agents": { "type": "array", "items": { "type": "string" } + }, + "features": { + "type": "object", + "properties": { + "agents": { "type": "integer", "minimum": 0 }, + "commands": { "type": "integer", "minimum": 0 }, + "skills": { "type": "integer", "minimum": 0 }, + "configAssets": { "type": "boolean" }, + "hookEvents": { + "type": "array", + "items": { "type": "string" } + }, + "customTools": { + "type": "array", + "items": { "type": "string" } + } + }, + "additionalProperties": false } - } + }, + "additionalProperties": false } diff --git a/scripts/hooks/post-edit-format.js b/scripts/hooks/post-edit-format.js index d8a1754c..08afad09 100644 --- a/scripts/hooks/post-edit-format.js +++ b/scripts/hooks/post-edit-format.js @@ -10,7 +10,7 @@ * Fails silently if no formatter is found or installed. */ -const { execFileSync } = require('child_process'); +const { execFileSync, spawnSync } = require('child_process'); const fs = require('fs'); const path = require('path'); const { getPackageManager } = require('../lib/package-manager'); @@ -50,18 +50,23 @@ process.stdin.on('data', chunk => { function findProjectRoot(startDir) { let dir = startDir; + let fallbackDir = null; while (true) { - if (PROJECT_ROOT_MARKERS.some(marker => fs.existsSync(path.join(dir, marker)))) { + if (detectFormatter(dir)) { return dir; } + if (!fallbackDir && PROJECT_ROOT_MARKERS.some(marker => fs.existsSync(path.join(dir, marker)))) { + fallbackDir = dir; + } + const parentDir = path.dirname(dir); if (parentDir === dir) break; dir = parentDir; } - return startDir; + return fallbackDir || startDir; } function detectFormatter(projectRoot) { @@ -114,6 +119,33 @@ function getFormatterCommand(formatter, filePath, projectRoot) { return null; } +function runFormatterCommand(cmd, projectRoot) { + if (process.platform === 'win32' && cmd.bin.endsWith('.cmd')) { + const result = spawnSync(cmd.bin, cmd.args, { + cwd: projectRoot, + shell: true, + stdio: 'pipe', + timeout: 15000 + }); + + if (result.error) { + throw result.error; + } + + if (typeof result.status === 'number' && result.status !== 0) { + throw new Error(result.stderr?.toString() || `Formatter exited with status ${result.status}`); + } + + return; + } + + execFileSync(cmd.bin, cmd.args, { + cwd: projectRoot, + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 15000 + }); +} + process.stdin.on('end', () => { try { const input = JSON.parse(data); @@ -126,11 +158,7 @@ process.stdin.on('end', () => { const cmd = getFormatterCommand(formatter, filePath, projectRoot); if (cmd) { - execFileSync(cmd.bin, cmd.args, { - cwd: projectRoot, - stdio: ['pipe', 'pipe', 'pipe'], - timeout: 15000 - }); + runFormatterCommand(cmd, projectRoot); } } catch { // Formatter not installed, file missing, or failed — non-blocking diff --git a/scripts/hooks/pre-bash-dev-server-block.js b/scripts/hooks/pre-bash-dev-server-block.js index 01c76c1f..9c0861b8 100755 --- a/scripts/hooks/pre-bash-dev-server-block.js +++ b/scripts/hooks/pre-bash-dev-server-block.js @@ -2,6 +2,7 @@ 'use strict'; const MAX_STDIN = 1024 * 1024; +const path = require('path'); const { splitShellSegments } = require('../lib/shell-split'); const DEV_COMMAND_WORDS = new Set([ @@ -10,13 +11,9 @@ const DEV_COMMAND_WORDS = new Set([ 'yarn', 'bun', 'npx', - 'bash', - 'sh', - 'zsh', - 'fish', 'tmux' ]); -const SKIPPABLE_PREFIX_WORDS = new Set(['env', 'command', 'builtin', 'exec', 'noglob', 'sudo']); +const SKIPPABLE_PREFIX_WORDS = new Set(['env', 'command', 'builtin', 'exec', 'noglob', 'sudo', 'nohup']); const PREFIX_OPTION_VALUE_WORDS = { env: new Set(['-u', '-C', '-S', '--unset', '--chdir', '--split-string']), sudo: new Set([ @@ -97,6 +94,12 @@ function isOptionToken(token) { return token.startsWith('-') && token.length > 1; } +function normalizeCommandWord(token) { + if (!token) return ''; + const base = path.basename(token).toLowerCase(); + return base.replace(/\.(cmd|exe|bat)$/i, ''); +} + function getLeadingCommandWord(segment) { let index = 0; let activeWrapper = null; @@ -122,8 +125,10 @@ function getLeadingCommandWord(segment) { if (/^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token)) continue; - if (SKIPPABLE_PREFIX_WORDS.has(token)) { - activeWrapper = token; + const normalizedToken = normalizeCommandWord(token); + + if (SKIPPABLE_PREFIX_WORDS.has(normalizedToken)) { + activeWrapper = normalizedToken; continue; } @@ -134,7 +139,7 @@ function getLeadingCommandWord(segment) { continue; } - return token; + return normalizedToken; } return null; diff --git a/skills/continuous-learning-v2/scripts/detect-project.sh b/skills/continuous-learning-v2/scripts/detect-project.sh index b2677538..a44a0264 100755 --- a/skills/continuous-learning-v2/scripts/detect-project.sh +++ b/skills/continuous-learning-v2/scripts/detect-project.sh @@ -43,6 +43,7 @@ _clv2_resolve_python_cmd() { } _CLV2_PYTHON_CMD="$(_clv2_resolve_python_cmd 2>/dev/null || true)" +CLV2_PYTHON_CMD="$_CLV2_PYTHON_CMD" export CLV2_PYTHON_CMD _clv2_detect_project() { diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 09be038c..a5ecaaa0 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -101,7 +101,14 @@ function readCommandLog(logFile) { return fs.readFileSync(logFile, 'utf8') .split('\n') .filter(Boolean) - .map(line => JSON.parse(line)); + .map(line => { + try { + return JSON.parse(line); + } catch { + return null; + } + }) + .filter(Boolean); } function withPrependedPath(binDir, env = {}) { @@ -873,6 +880,32 @@ async function runTests() { } })) passed++; else failed++; + if (await asyncTest('blocks env-wrapped npm run dev outside tmux on non-Windows platforms', async () => { + const stdinJson = JSON.stringify({ tool_input: { command: '/usr/bin/env npm run dev' } }); + const result = await runScript(path.join(scriptsDir, 'pre-bash-dev-server-block.js'), stdinJson); + + if (process.platform === 'win32') { + assert.strictEqual(result.code, 0, 'Windows path should pass through'); + assert.strictEqual(result.stdout, stdinJson, 'Windows path should preserve original input'); + } else { + assert.strictEqual(result.code, 2, 'Unix path should block wrapped dev servers'); + assert.ok(result.stderr.includes('BLOCKED'), 'Should explain why the command was blocked'); + } + })) passed++; else failed++; + + if (await asyncTest('blocks nohup-wrapped npm run dev outside tmux on non-Windows platforms', async () => { + const stdinJson = JSON.stringify({ tool_input: { command: 'nohup npm run dev >/tmp/dev.log 2>&1 &' } }); + const result = await runScript(path.join(scriptsDir, 'pre-bash-dev-server-block.js'), stdinJson); + + if (process.platform === 'win32') { + assert.strictEqual(result.code, 0, 'Windows path should pass through'); + assert.strictEqual(result.stdout, stdinJson, 'Windows path should preserve original input'); + } else { + assert.strictEqual(result.code, 2, 'Unix path should block wrapped dev servers'); + assert.ok(result.stderr.includes('BLOCKED'), 'Should explain why the command was blocked'); + } + })) passed++; else failed++; + // post-edit-typecheck.js tests console.log('\npost-edit-typecheck.js:'); @@ -1659,7 +1692,14 @@ async function runTests() { const formatSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-format.js'), 'utf8'); // Strip comments to avoid matching "shell: true" in comment text const codeOnly = formatSource.replace(/\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, ''); - assert.ok(!codeOnly.includes('shell:'), 'post-edit-format.js should not pass shell option in code'); + assert.ok( + !/execFileSync\([^)]*shell\s*:/.test(codeOnly), + 'post-edit-format.js should not pass shell option to execFileSync' + ); + assert.ok( + codeOnly.includes("process.platform === 'win32' && cmd.bin.endsWith('.cmd')"), + 'Windows shell execution must stay gated to .cmd shims' + ); assert.ok(formatSource.includes('npx.cmd'), 'Should use npx.cmd for Windows cross-platform safety'); })) passed++; else failed++; @@ -1710,6 +1750,33 @@ async function runTests() { assert.ok(detectProjectSource.includes('_clv2_resolve_python_cmd'), 'detect-project.sh should provide shared Python resolution'); })) passed++; else failed++; + if (await asyncTest('detect-project exports the resolved Python command for downstream scripts', async () => { + const detectProjectPath = path.join(__dirname, '..', '..', 'skills', 'continuous-learning-v2', 'scripts', 'detect-project.sh'); + const shellCommand = [ + `source "${detectProjectPath}" >/dev/null 2>&1`, + 'printf "%s\\n" "${CLV2_PYTHON_CMD:-}"' + ].join('; '); + + const shell = process.platform === 'win32' ? 'bash' : 'bash'; + const proc = spawn(shell, ['-lc', shellCommand], { + env: process.env, + stdio: ['ignore', 'pipe', 'pipe'] + }); + + let stdout = ''; + let stderr = ''; + proc.stdout.on('data', data => stdout += data); + proc.stderr.on('data', data => stderr += data); + + const code = await new Promise((resolve, reject) => { + proc.on('close', resolve); + proc.on('error', reject); + }); + + assert.strictEqual(code, 0, `detect-project.sh should source cleanly, stderr: ${stderr}`); + assert.ok(stdout.trim().length > 0, 'CLV2_PYTHON_CMD should export a resolved interpreter path'); + })) passed++; else failed++; + if (await asyncTest('matches .tsx extension for type checking', async () => { const testDir = createTestDir(); const testFile = path.join(testDir, 'component.tsx');