From 6be87a56ae24dc2ddc9eba2fc71580cbc8a711f0 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Tue, 28 Jul 2026 18:31:12 -0700 Subject: [PATCH 1/4] fix: clean promoted instinct sources (#2587) * fix: clean promoted instinct sources * test(instincts): normalize retained-content line endings --------- Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> --- .../scripts/instinct-cli.py | 108 +++++++++++++ tests/scripts/instinct-cli-projects.test.js | 148 ++++++++++++++++++ 2 files changed, 256 insertions(+) diff --git a/skills/continuous-learning-v2/scripts/instinct-cli.py b/skills/continuous-learning-v2/scripts/instinct-cli.py index 0a99f1fa0..9bc3b6898 100755 --- a/skills/continuous-learning-v2/scripts/instinct-cli.py +++ b/skills/continuous-learning-v2/scripts/instinct-cli.py @@ -27,6 +27,7 @@ import ipaddress import socket import urllib.parse import urllib.request +import tempfile from contextlib import contextmanager from pathlib import Path from datetime import datetime, timedelta, timezone @@ -1323,6 +1324,106 @@ def _show_promotion_candidates(project: dict) -> None: print(f" Run `instinct-cli.py promote` to promote these to global scope.\n") +def _frontmatter_scalar(lines: list[str], key: str) -> Optional[str]: + """Extract a simple scalar value from frontmatter lines.""" + for line in lines: + if ':' not in line: + continue + parsed_key, value = line.split(':', 1) + if parsed_key.strip() != key: + continue + value = value.strip() + if value.startswith('"') and value.endswith('"'): + return value[1:-1].replace('\\"', '"').replace('\\\\', '\\') + if value.startswith("'") and value.endswith("'"): + return value[1:-1].replace("''", "'") + return value + return None + + +def _remove_instinct_blocks(content: str, instinct_id: str) -> tuple[str, int]: + """Remove raw frontmatter blocks with a matching instinct ID.""" + lines = content.splitlines(keepends=True) + retained = [] + removed = 0 + index = 0 + + while index < len(lines): + if lines[index].strip() != '---': + retained.append(lines[index]) + index += 1 + continue + + block_start = index + frontmatter_end = index + 1 + while frontmatter_end < len(lines) and lines[frontmatter_end].strip() != '---': + frontmatter_end += 1 + + if frontmatter_end >= len(lines): + retained.extend(lines[block_start:]) + break + + next_block_start = frontmatter_end + 1 + while next_block_start < len(lines) and lines[next_block_start].strip() != '---': + next_block_start += 1 + + block_id = _frontmatter_scalar(lines[block_start + 1:frontmatter_end], 'id') + if block_id == instinct_id: + removed += 1 + else: + retained.extend(lines[block_start:next_block_start]) + index = next_block_start + + return ''.join(retained), removed + + +def _write_text_atomic(file_path: Path, content: str) -> None: + """Replace a text file via same-directory temp file.""" + temp_fd, temp_name = tempfile.mkstemp( + prefix=f".{file_path.name}.", + suffix=".tmp", + dir=file_path.parent, + text=True, + ) + temp_file = Path(temp_name) + try: + with os.fdopen(temp_fd, "w", encoding="utf-8") as f: + f.write(content) + f.flush() + os.fsync(f.fileno()) + os.replace(temp_file, file_path) + finally: + try: + temp_file.unlink() + except FileNotFoundError: + pass + + +def _remove_instinct_from_source(source_file_str: str, instinct_id: str) -> None: + """Strip promoted instinct blocks from the project-scoped source file.""" + source_file = Path(source_file_str) + if not source_file.exists(): + return + + try: + content = source_file.read_text(encoding="utf-8") + except OSError as exc: + print(f"Warning: Failed to read promoted instinct source {source_file}: {exc}", file=sys.stderr) + return + + remaining_content, removed = _remove_instinct_blocks(content, instinct_id) + if removed == 0: + return + + try: + if remaining_content: + _write_text_atomic(source_file, remaining_content) + else: + source_file.unlink() + except OSError as exc: + print(f"Warning: Failed to remove promoted instinct from {source_file}: {exc}", file=sys.stderr) + + def cmd_promote(args) -> int: """Promote project-scoped instincts to global scope.""" project = detect_project() @@ -1385,6 +1486,9 @@ def _promote_specific(project: dict, instinct_id: str, force: bool, dry_run: boo output_content += target.get('content', '') + "\n" output_file.write_text(output_content, encoding="utf-8") + source_file = target.get('_source_file') + if source_file: + _remove_instinct_from_source(source_file, instinct_id) print(f"\nPromoted '{instinct_id}' to global scope.") print(f" Saved to: {output_file}") return 0 @@ -1458,6 +1562,10 @@ def _promote_auto(project: dict, force: bool, dry_run: bool) -> int: output_content += inst.get('content', '') + "\n" output_file.write_text(output_content, encoding="utf-8") + for _, _, entry_inst in cand['entries']: + entry_source = entry_inst.get('_source_file') + if entry_source: + _remove_instinct_from_source(entry_source, cand['id']) promoted += 1 print(f"\nPromoted {promoted} instincts to global scope.") diff --git a/tests/scripts/instinct-cli-projects.test.js b/tests/scripts/instinct-cli-projects.test.js index 73289f73a..ae0ff27f2 100644 --- a/tests/scripts/instinct-cli-projects.test.js +++ b/tests/scripts/instinct-cli-projects.test.js @@ -63,6 +63,10 @@ function readJson(filePath) { return JSON.parse(fs.readFileSync(filePath, 'utf8')); } +function normalizeLineEndings(value) { + return value.replace(/\r\n/g, '\n'); +} + function writeInstinct(filePath, id, confidence = 0.9) { fs.mkdirSync(path.dirname(filePath), { recursive: true }); fs.writeFileSync( @@ -117,6 +121,13 @@ function runGit(cwd, args) { return result.stdout.trim(); } +function initGitProject(parentDir, name = 'repo') { + const repoDir = path.join(parentDir, name); + fs.mkdirSync(repoDir, { recursive: true }); + runGit(repoDir, ['init']); + return repoDir; +} + function runCli(root, args, options = {}) { return spawnSync(PYTHON3, [cliPath, ...args], { cwd: options.cwd || repoRoot, @@ -330,6 +341,143 @@ test('status migrates legacy no-remote linked worktree project dirs to main work } }); +test('promote removes only the promoted instinct block from project source', () => { + const root = createTempDir(); + const repoParent = createTempDir(); + try { + const repoDir = initGitProject(repoParent); + const projectId = projectHash(runGit(repoDir, ['rev-parse', '--show-toplevel'])); + const sourceFile = path.join(root, 'projects', projectId, 'instincts', 'personal', 'mixed.yaml'); + const retainedBlock = [ + '---', + 'id: keep-me', + 'trigger: "when value: contains colon"', + 'confidence: 0.72', + 'domain: workflow', + 'tags: [alpha, beta]', + '---', + '', + 'Keep this block exactly.', + '', + ].join('\n'); + fs.mkdirSync(path.dirname(sourceFile), { recursive: true }); + fs.writeFileSync( + sourceFile, + [ + '---', + 'id: promote-me', + 'trigger: "when promoting"', + 'confidence: 0.91', + 'domain: workflow', + '---', + '', + 'Promote this block.', + '', + retainedBlock, + ].join('\n') + ); + + const result = runCli(root, ['promote', 'promote-me', '--force'], { cwd: repoDir }); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(fs.existsSync(path.join(root, 'instincts', 'personal', 'promote-me.yaml'))); + assert.strictEqual(normalizeLineEndings(fs.readFileSync(sourceFile, 'utf8')), retainedBlock); + } finally { + cleanupDir(root); + cleanupDir(repoParent); + } +}); + +test('promote deletes project source file when it only contained the promoted instinct', () => { + const root = createTempDir(); + const repoParent = createTempDir(); + try { + const repoDir = initGitProject(repoParent); + const projectId = projectHash(runGit(repoDir, ['rev-parse', '--show-toplevel'])); + const sourceFile = path.join(root, 'projects', projectId, 'instincts', 'personal', 'single.yaml'); + writeInstinct(sourceFile, 'promote-single', 0.93); + + const result = runCli(root, ['promote', 'promote-single', '--force'], { cwd: repoDir }); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(fs.existsSync(path.join(root, 'instincts', 'personal', 'promote-single.yaml'))); + assert.ok(!fs.existsSync(sourceFile)); + } finally { + cleanupDir(root); + cleanupDir(repoParent); + } +}); + +test('promote preserves malformed and foreign source blocks while removing target', () => { + const root = createTempDir(); + const repoParent = createTempDir(); + try { + const repoDir = initGitProject(repoParent); + const projectId = projectHash(runGit(repoDir, ['rev-parse', '--show-toplevel'])); + const sourceFile = path.join(root, 'projects', projectId, 'instincts', 'personal', 'foreign.yaml'); + const foreignContent = [ + '---', + 'title: foreign block without id', + '---', + '', + 'Do not drop this content.', + '', + '---', + 'id: keep-foreign-neighbor', + 'trigger: "when nearby"', + 'confidence: not-a-float', + '---', + '', + 'This parse-tolerated block must also stay raw.', + '', + ].join('\n'); + fs.mkdirSync(path.dirname(sourceFile), { recursive: true }); + fs.writeFileSync( + sourceFile, + [ + foreignContent, + '---', + 'id: promote-foreign', + 'trigger: "when target appears"', + 'confidence: 0.95', + 'domain: workflow', + '---', + '', + 'Only this block should be removed.', + '', + ].join('\n') + ); + + const result = runCli(root, ['promote', 'promote-foreign', '--force'], { cwd: repoDir }); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(fs.existsSync(path.join(root, 'instincts', 'personal', 'promote-foreign.yaml'))); + assert.strictEqual(normalizeLineEndings(fs.readFileSync(sourceFile, 'utf8')), `${foreignContent}\n`); + } finally { + cleanupDir(root); + cleanupDir(repoParent); + } +}); + +test('auto-promote removes promoted source copies from every contributing project', () => { + const root = createTempDir(); + try { + const registryPath = path.join(root, 'projects.json'); + const projectOne = seedProject(root, 'proj111', { personal: ['shared-auto'] }); + const projectTwo = seedProject(root, 'proj222', { personal: ['shared-auto'] }); + writeJson(registryPath, { + proj111: { name: 'one', root: '/repo/one', remote: '', last_seen: '2026-01-01T00:00:00Z' }, + proj222: { name: 'two', root: '/repo/two', remote: '', last_seen: '2026-01-02T00:00:00Z' }, + }); + + const result = runCli(root, ['promote', '--force'], { cwd: root }); + assert.strictEqual(result.status, 0, result.stderr); + assert.match(result.stdout, /Promoted 1 instincts to global scope/); + assert.ok(fs.existsSync(path.join(root, 'instincts', 'personal', 'shared-auto.yaml'))); + assert.ok(!fs.existsSync(path.join(projectOne, 'instincts', 'personal', 'shared-auto.yaml'))); + assert.ok(!fs.existsSync(path.join(projectTwo, 'instincts', 'personal', 'shared-auto.yaml'))); + } finally { + cleanupDir(root); + } +}); + console.log(`\nPassed: ${passed}`); console.log(`Failed: ${failed}`); From 837acaf20b360302e49a9a05abf3d708f15e84db Mon Sep 17 00:00:00 2001 From: djpjronline-netizen Date: Tue, 28 Jul 2026 21:32:42 -0400 Subject: [PATCH 2/4] fix(hooks,lib): fix hook detection and parsing edge cases (#2405) * fix(hooks,lib): fix hook detection and parsing edge cases - auto-tmux-dev: dev\b -> dev(?![\w-]) so one-shot dev-build/dev-docs scripts are not detached into tmux; align command shapes (yarn run dev, bun dev) with pre-bash-dev-server-block.js DEV_PATTERN. - pre-bash-commit-quality: skip obvious non-secret placeholders (env refs, ${...}, <...>, whitelisted tokens) in the api-key rule without suppressing real high-entropy secrets; make -m message extraction quote- and escaped-quote-aware so `-m "fix: \"x\""` / apostrophes are not truncated. - pre-compact: annotate the CURRENT worktree's session (match **Worktree:** / legacy **Project:**) instead of the newest *-session.tmp across all projects, layered onto the LLM-summary flow from #2388; a present-but-blank Worktree header is treated as non-legacy (no foreign project fallback). - shell-substitution: stop double-appending a trailing backslash in an unterminated backtick span. - utils readStdinJson: on overflow, settle and resolve {} immediately (clear timer + listeners) instead of waiting for end/timeout and parsing a partial prefix; surface the overflow on stderr. Regression tests added/extended (new tests/hooks/pre-compact.test.js). Addresses review feedback on #2405. The earlier block-no-verify change was dropped: its message-value skip on merge/cherry-pick/am/rebase would let `git rebase -m --no-verify` bypass the hook (rebase's -m is the boolean --merge), a false-negative worse than the contrived false-positive it fixed. Co-Authored-By: Claude Opus 4.8 * fix(ci): align hook fixtures and drain oversized stdin --------- Co-authored-by: djpjronline-netizen <276112803+djpjronline-netizen@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> --- scripts/hooks/auto-tmux-dev.js | 15 ++- scripts/hooks/pre-bash-commit-quality.js | 52 ++++++++-- scripts/hooks/pre-compact.js | 104 ++++++++++++++++--- scripts/lib/shell-substitution.js | 24 ++++- scripts/lib/utils.js | 38 ++++++- tests/hooks/auto-tmux-dev.test.js | 8 ++ tests/hooks/hooks.test.js | 25 +++-- tests/hooks/pre-bash-commit-quality.test.js | 81 +++++++++++++++ tests/hooks/pre-compact.test.js | 105 ++++++++++++++++++++ tests/lib/shell-substitution.test.js | 30 +++++- tests/lib/utils.test.js | 88 ++++++++++++++-- 11 files changed, 517 insertions(+), 53 deletions(-) create mode 100644 tests/hooks/pre-compact.test.js diff --git a/scripts/hooks/auto-tmux-dev.js b/scripts/hooks/auto-tmux-dev.js index 2f1a7a1e5..aa61aecb8 100755 --- a/scripts/hooks/auto-tmux-dev.js +++ b/scripts/hooks/auto-tmux-dev.js @@ -36,9 +36,18 @@ function run(rawInput) { const input = typeof rawInput === 'string' ? JSON.parse(rawInput) : rawInput; const cmd = input.tool_input?.command || ''; - // Detect dev server commands: npm run dev, pnpm dev, yarn dev, bun run dev - // Use word boundary (\b) to avoid matching partial commands - const devServerRegex = /(npm run dev\b|pnpm( run)? dev\b|yarn dev\b|bun run dev\b)/; + // Detect dev server commands: npm run dev, pnpm (run) dev, yarn (run) dev, + // bun (run) dev. Trailing (?![\w-]) rather than \b: \b treats a hyphen as a + // word boundary, so `dev\b` matches the `dev` prefix of distinct scripts + // like `dev-build` / `dev-docs` and would wrongly detach those one-shot + // scripts into tmux. The lookahead still matches the dev server (`dev`, + // `dev:ssr`, ...) but not a `dev-` script. The optional `run` on + // yarn/bun mirrors the command shapes in pre-bash-dev-server-block.js + // DEV_PATTERN so the two hooks agree on what counts as a dev server. + // Flexible whitespace (\s+) and leading \b make this byte-identical to + // pre-bash-dev-server-block.js DEV_PATTERN, so a tabbed/multi-space command + // the blocker catches is also detached here (they agree exactly). + const devServerRegex = /\b(npm\s+run\s+dev|pnpm(?:\s+run)?\s+dev|yarn(?:\s+run)?\s+dev|bun(?:\s+run)?\s+dev)(?![\w-])/; if (devServerRegex.test(cmd)) { // Get session name from current directory basename, sanitize for shell safety diff --git a/scripts/hooks/pre-bash-commit-quality.js b/scripts/hooks/pre-bash-commit-quality.js index c67b0f5a0..5780c1d5b 100644 --- a/scripts/hooks/pre-bash-commit-quality.js +++ b/scripts/hooks/pre-bash-commit-quality.js @@ -57,9 +57,32 @@ function shouldCheckFile(filePath) { return checkableExtensions.some(ext => filePath.endsWith(ext)); } +/** + * Decide whether a captured api-key value is an OBVIOUS non-secret placeholder so + * the heuristic generic api-key rule does not emit a false positive. Deliberately + * narrow: only suppresses whole-value env references / interpolations / angle-bracket + * tokens and a short explicit whitelist of placeholder + env-var NAME tokens. It must + * NOT suppress arbitrary high-entropy data (uppercase-hex, base32, digit-only, mixed + * tokens), since the generic rule is the only net catching non-prefixed secrets and a + * false-negative there is the safety-critical failure this hook exists to prevent. + * @param {string} value + * @returns {boolean} + */ +function isPlaceholderSecret(value) { + const v = (value || '').trim(); + if (v.length === 0) return true; // empty value + if (/^process\.env\.[A-Za-z0-9_]+$/.test(v)) return true; // entire value is a process.env.NAME reference + if (/^\$\{[^}]*\}$/.test(v)) return true; // entire value is a ${...} interpolation + if (/^<[^<>]*>$/.test(v)) return true; // entire value is a token + // Short explicit whitelist of placeholder + env-var NAME tokens (whole-value match only). + // No general all-caps clause: real all-caps/hex/base32/digit secrets must still flag. + if (/^(REPLACE_ME|CHANGE_?ME|YOUR[_-]?API[_-]?KEY|YOUR[_-]?KEY[_-]?HERE|API[_-]?KEY|SECRET|TOKEN|KEY|TODO|TBD|FIXME|XXX+)$/i.test(v)) return true; + return false; +} + /** * Find issues in file content - * @param {string} filePath + * @param {string} filePath * @returns {object[]} Array of issues found */ function findFileIssues(filePath) { @@ -112,11 +135,21 @@ function findFileIssues(filePath) { { pattern: /sk-[a-zA-Z0-9]{20,}/, name: 'OpenAI API key' }, { pattern: /ghp_[a-zA-Z0-9]{36}/, name: 'GitHub PAT' }, { pattern: /AKIA[A-Z0-9]{16}/, name: 'AWS Access Key' }, - { pattern: /api[_-]?key\s*[=:]\s*['"][^'"]+['"]/i, name: 'API key' } + // Capture the quoted value so obvious non-secret placeholders can be excluded + { pattern: /api[_-]?key\s*[=:]\s*['"]([^'"]+)['"]/i, name: 'API key', valueGroup: 1 }, + // Unquoted form (API_KEY=..., api_key: ... without quotes). Scoped to a + // single alnum/underscore/hyphen token of 12+ chars containing at least + // one digit — real secrets are near-always alphanumeric, whereas bare + // identifiers/expressions common in this hook's checkable languages + // (config.apiKey, getApiKey(), process.env.API_KEY) are pure-alpha or + // contain '.'/'(' that fall outside the character class, so they don't + // match. Kept deliberately narrow to avoid flagging ordinary code. + { pattern: /api[_-]?key\s*[=:]\s*(?!['"])((?=[A-Za-z0-9_-]*\d)[A-Za-z0-9_-]{12,})/i, name: 'API key', valueGroup: 1 } ]; - for (const { pattern, name } of secretPatterns) { - if (pattern.test(line)) { + for (const { pattern, name, valueGroup } of secretPatterns) { + const secretMatch = line.match(pattern); + if (secretMatch && !(valueGroup && isPlaceholderSecret(secretMatch[valueGroup]))) { issues.push({ type: 'secret', message: `Potential ${name} exposed at line ${lineNum}`, @@ -139,11 +172,14 @@ function findFileIssues(filePath) { * @returns {object|null} Validation result or null if no message to validate */ function validateCommitMessage(command) { - // Extract commit message from command - const messageMatch = command.match(/(?:-m|--message)[=\s]+["']?([^"']+)["']?/); + // Extract commit message from command (quote-aware: when quoted, capture to the + // matching closing quote, consuming escaped chars (\") so an embedded escaped + // quote does not truncate the subject, and allowing the OTHER quote char inside + // the body; when unquoted, capture the full remaining tail, not just the first token) + const messageMatch = command.match(/(?:-m|--message)[=\s]+(?:"((?:\\.|[^"\\])*)"|'((?:\\.|[^'\\])*)'|([^"']+?)\s*$)/); if (!messageMatch) return null; - const message = messageMatch[1]; + const message = messageMatch[1] ?? messageMatch[2] ?? messageMatch[3]; const issues = []; // Check conventional commit format @@ -445,4 +481,4 @@ if (require.main === module) { }); } -module.exports = { run, evaluate }; +module.exports = { run, evaluate, validateCommitMessage, findFileIssues, isPlaceholderSecret }; diff --git a/scripts/hooks/pre-compact.js b/scripts/hooks/pre-compact.js index 235b2b097..2002ac3df 100644 --- a/scripts/hooks/pre-compact.js +++ b/scripts/hooks/pre-compact.js @@ -14,7 +14,7 @@ const path = require('path'); const fs = require('fs'); -const { getSessionsDir, getDateTimeString, getTimeString, findFiles, ensureDir, appendFile, readFile, writeFile, log } = require('../lib/utils'); +const { getSessionsDir, getDateTimeString, getTimeString, findFiles, ensureDir, appendFile, readFile, writeFile, getProjectName, log } = require('../lib/utils'); const { generateSessionSummary } = require('../lib/llm-summary'); const SUMMARY_START_MARKER = ''; @@ -24,22 +24,91 @@ function escapeRegExp(value) { return String(value).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } +/** + * Canonicalize a path (resolve symlinks); fall back to the input on failure. + * Mirrors session-start.js#normalizePath so worktree comparisons agree. + */ +function normalizePath(p) { + try { + return fs.realpathSync(p); + } catch { + return p; + } +} + +/** + * Pick the session file that belongs to the CURRENT worktree. + * + * The sessions dir is shared across every project/worktree, so the newest + * `*-session.tmp` is frequently a DIFFERENT project's session. Matching by + * mtime (`sessions[0]`) therefore writes the compaction summary into the wrong + * project. Match on the `**Worktree:**` header (written by session-end.js) + * against cwd, mirroring session-start.js#selectMatchingSession: + * 1. exact worktree (cwd) match — newest wins + * 2. truly legacy sessions with NO Worktree header: same **Project:** name + * 3. otherwise null — do NOT annotate a foreign worktree's session + * A present-but-blank Worktree header counts as non-legacy (never a project + * fallback), so a foreign session is not matched by name. + * + * @param {Array<{path: string}>} sessions - newest-first session list + * @param {string} cwd + * @param {string} currentProject + * @param {(p: string) => (string|null)} [readFn] + * @returns {string|null} path of the chosen session, or null if none match + */ +function selectActiveSessionPath(sessions, cwd, currentProject, readFn = readFile) { + if (!sessions || sessions.length === 0) return null; + const normalizedCwd = normalizePath(cwd); + let projectMatch = null; + + for (const session of sessions) { + const content = readFn(session.path); + if (!content) continue; + + // (.*) not (.+): an explicit but empty header (`**Worktree:**` / `**Worktree:**\n`) + // must still register as present (hasWorktreeHeader) so it does not fall back + // to project-name matching against a foreign session. + const worktreeMatch = content.match(/\*\*Worktree:\*\*\s*(.*)$/m); + const hasWorktreeHeader = Boolean(worktreeMatch); + const sessionWorktree = worktreeMatch ? worktreeMatch[1].trim() : ''; + + if (sessionWorktree && normalizePath(sessionWorktree) === normalizedCwd) { + return session.path; + } + + // Project-name fallback only for truly legacy sessions with NO Worktree + // header at all — a present-but-blank header is not treated as legacy. + if (!projectMatch && currentProject && !hasWorktreeHeader) { + const projectFieldMatch = content.match(/\*\*Project:\*\*\s*(.+)$/m); + const sessionProject = projectFieldMatch ? projectFieldMatch[1].trim() : ''; + if (sessionProject && sessionProject === currentProject) { + projectMatch = session.path; + } + } + } + + return projectMatch; +} + const MAX_STDIN = 1024 * 1024; let stdinData = ''; -process.stdin.setEncoding('utf8'); -process.stdin.on('data', chunk => { - if (stdinData.length < MAX_STDIN) { - stdinData += chunk.substring(0, MAX_STDIN - stdinData.length); - } -}); +if (require.main === module) { + process.stdin.setEncoding('utf8'); -process.stdin.on('end', () => { - main().catch(err => { - log(`[PreCompact] Error: ${err.message}`); - process.exit(0); + process.stdin.on('data', chunk => { + if (stdinData.length < MAX_STDIN) { + stdinData += chunk.substring(0, MAX_STDIN - stdinData.length); + } }); -}); + + process.stdin.on('end', () => { + main().catch(err => { + log(`[PreCompact] Error: ${err.message}`); + process.exit(0); + }); + }); +} async function main() { let transcriptPath = null; @@ -66,7 +135,14 @@ async function main() { process.exit(0); } - const activeSession = sessions[0].path; + // Select the session for THIS worktree, not merely the newest across all + // projects (the sessions dir is shared). Skip when none matches rather than + // writing the summary into a foreign project's session file. + const activeSession = selectActiveSessionPath(sessions, process.cwd(), getProjectName()); + if (!activeSession) { + log('[PreCompact] No session matches the current worktree; skipping annotation'); + process.exit(0); + } const timeStr = getTimeString(); if (!transcriptPath || !fs.existsSync(transcriptPath)) { @@ -98,3 +174,5 @@ async function main() { process.exit(0); } + +module.exports = { selectActiveSessionPath, normalizePath }; diff --git a/scripts/lib/shell-substitution.js b/scripts/lib/shell-substitution.js index 2689ccb55..0251e74e2 100644 --- a/scripts/lib/shell-substitution.js +++ b/scripts/lib/shell-substitution.js @@ -55,8 +55,12 @@ function extractCommandSubstitutions(input) { if (i + 1 < source.length) { body += source[i + 1]; i += 2; - continue; + } else { + // Trailing backslash at end of an unterminated span: advance past + // it so it is not appended a second time by the fallthrough below. + i += 1; } + continue; } if (inner === '`') { break; @@ -85,8 +89,12 @@ function extractCommandSubstitutions(input) { if (i + 1 < source.length) { body += source[i + 1]; i += 2; - continue; + } else { + // Trailing backslash at end of an unterminated span: advance past + // it so it is not appended a second time by the fallthrough below. + i += 1; } + continue; } if (inner === "'" && !bodyInDouble && innerPrev !== '\\') { bodyInSingle = !bodyInSingle; @@ -213,8 +221,12 @@ function extractSubshellGroups(input) { if (i + 1 < source.length) { body += source[i + 1]; i += 2; - continue; + } else { + // Trailing backslash at end of an unterminated span: advance past + // it so it is not appended a second time by the fallthrough below. + i += 1; } + continue; } if (inner === "'" && !bodyInDouble && innerPrev !== '\\') { bodyInSingle = !bodyInSingle; @@ -374,8 +386,12 @@ function extractBraceGroups(input) { if (i + 1 < source.length) { body += source[i + 1]; i += 2; - continue; + } else { + // Trailing backslash at end of an unterminated span: advance past + // it so it is not appended a second time by the fallthrough below. + i += 1; } + continue; } if (inner === "'" && !bodyInDouble && innerPrev !== '\\') { bodyInSingle = !bodyInSingle; diff --git a/scripts/lib/utils.js b/scripts/lib/utils.js index a201e234e..5e29868eb 100644 --- a/scripts/lib/utils.js +++ b/scripts/lib/utils.js @@ -284,6 +284,7 @@ async function readStdinJson(options = {}) { return new Promise((resolve) => { let data = ''; let settled = false; + let overflowed = false; const timer = setTimeout(() => { if (!settled) { @@ -293,7 +294,12 @@ async function readStdinJson(options = {}) { process.stdin.removeAllListeners('end'); process.stdin.removeAllListeners('error'); if (process.stdin.unref) process.stdin.unref(); - // Resolve with whatever we have so far rather than hanging + // Oversized input is always rejected. Otherwise, resolve with whatever + // arrived before the timeout rather than hanging. + if (overflowed) { + resolve({}); + return; + } try { resolve(data.trim() ? JSON.parse(data) : {}); } catch { @@ -304,15 +310,34 @@ async function readStdinJson(options = {}) { process.stdin.setEncoding('utf8'); process.stdin.on('data', chunk => { - if (data.length < maxSize) { - data += chunk; + if (settled) return; + if (overflowed) return; + // Mark oversized input as rejected and discard the buffered prefix. + // Continue consuming the stream without retaining later chunks so a + // finite parent can finish writing without EPIPE. Resolution happens at + // EOF or the existing timeout, which also bounds never-closing writers. + if (data.length + chunk.length > maxSize) { + overflowed = true; + data = ''; + process.stderr.write( + `[readStdinJson] stdin exceeded ${maxSize} bytes; input truncated and treated as empty\n` + ); + return; } + data += chunk; }); process.stdin.on('end', () => { - if (settled) return; + if (settled) { + clearTimeout(timer); + return; + } settled = true; clearTimeout(timer); + if (overflowed) { + resolve({}); + return; + } try { resolve(data.trim() ? JSON.parse(data) : {}); } catch { @@ -323,7 +348,10 @@ async function readStdinJson(options = {}) { }); process.stdin.on('error', () => { - if (settled) return; + if (settled) { + clearTimeout(timer); + return; + } settled = true; clearTimeout(timer); // Resolve with empty object so hooks don't crash on stdin errors diff --git a/tests/hooks/auto-tmux-dev.test.js b/tests/hooks/auto-tmux-dev.test.js index ac2b37fd3..3c47a8ac4 100644 --- a/tests/hooks/auto-tmux-dev.test.js +++ b/tests/hooks/auto-tmux-dev.test.js @@ -119,6 +119,14 @@ function runTests() { assert.strictEqual(output.tool_input.command, 'npm run develop'); })) passed++; else failed++; + if (test('does not transform npm run dev-build (hyphenated script)', () => { + const input = { tool_input: { command: 'npm run dev-build' } }; + const result = runScript(input); + assert.strictEqual(result.code, 0); + const output = JSON.parse(result.stdout); + assert.strictEqual(output.tool_input.command, 'npm run dev-build'); + })) passed++; else failed++; + console.log('\nEdge cases:'); if (test('handles empty input gracefully', () => { diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 82724226a..07d48dcd5 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -1247,7 +1247,9 @@ async function runTests() { // Create an active .tmp session file const sessionFile = path.join(sessionsDir, '2026-02-11-test-session.tmp'); - fs.writeFileSync(sessionFile, '# Session: 2026-02-11\n**Started:** 10:00\n'); + fs.writeFileSync(sessionFile, buildSessionStartFixture('**Started:** 10:00', { + title: '# Session: 2026-02-11' + })); try { await runScript(path.join(scriptsDir, 'pre-compact.js'), '', { @@ -3761,7 +3763,7 @@ async function runTests() { // Create a session .tmp file and a non-session .tmp file const sessionFile = path.join(sessionsDir, '2026-02-11-abc-session.tmp'); const otherTmpFile = path.join(sessionsDir, 'other-data.tmp'); - fs.writeFileSync(sessionFile, '# Session\n'); + fs.writeFileSync(sessionFile, buildSessionStartFixture('', { title: '# Session' })); fs.writeFileSync(otherTmpFile, 'some other data\n'); try { @@ -4676,11 +4678,11 @@ async function runTests() { passed++; else failed++; - // Round 41: pre-compact.js (multiple session files) + // Round 41: pre-compact.js (multiple sessions for the current worktree) console.log('\nRound 41: pre-compact.js (multiple session files):'); if ( - await asyncTest('annotates only the newest session file when multiple exist', async () => { + await asyncTest('annotates only the newest session when multiple match the current worktree', async () => { const isoHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-compact-multi-')); const sessionsDir = getCanonicalSessionsDir(isoHome); fs.mkdirSync(sessionsDir, { recursive: true }); @@ -4688,11 +4690,12 @@ async function runTests() { // Create two session files with different mtimes const olderSession = path.join(sessionsDir, '2026-01-01-older-session.tmp'); const newerSession = path.join(sessionsDir, '2026-02-11-newer-session.tmp'); - fs.writeFileSync(olderSession, '# Older Session\n'); + const olderContent = buildSessionStartFixture('', { title: '# Older Session' }); + fs.writeFileSync(olderSession, olderContent); // Small delay to ensure different mtime const now = Date.now(); fs.utimesSync(olderSession, new Date(now - 60000), new Date(now - 60000)); - fs.writeFileSync(newerSession, '# Newer Session\n'); + fs.writeFileSync(newerSession, buildSessionStartFixture('', { title: '# Newer Session' })); try { const result = await runScript(path.join(scriptsDir, 'pre-compact.js'), '', { @@ -4702,11 +4705,11 @@ async function runTests() { assert.strictEqual(result.code, 0); const newerContent = fs.readFileSync(newerSession, 'utf8'); - const olderContent = fs.readFileSync(olderSession, 'utf8'); + const updatedOlderContent = fs.readFileSync(olderSession, 'utf8'); - // findFiles sorts by mtime newest first, so sessions[0] is the newest + // findFiles sorts matches by mtime, so the newest matching worktree wins. assert.ok(newerContent.includes('Compaction occurred'), 'Should annotate the newest session file'); - assert.strictEqual(olderContent, '# Older Session\n', 'Should NOT annotate older session files'); + assert.strictEqual(updatedOlderContent, olderContent, 'Should NOT annotate older session files'); } finally { fs.rmSync(isoHome, { recursive: true, force: true }); } @@ -6208,7 +6211,9 @@ Some random content without the expected ### Context to Load section // Create a minimal session .tmp file const sessionFile = path.join(sessionsDir, '2026-01-01-test-session.tmp'); - fs.writeFileSync(sessionFile, '# Session: 2026-01-01\n'); + fs.writeFileSync(sessionFile, buildSessionStartFixture('', { + title: '# Session: 2026-01-01' + })); // Create a minimal transcript with one user message const transcriptPath = path.join(testDir, 'transcript.jsonl'); diff --git a/tests/hooks/pre-bash-commit-quality.test.js b/tests/hooks/pre-bash-commit-quality.test.js index b845f9496..ebf17d26e 100644 --- a/tests/hooks/pre-bash-commit-quality.test.js +++ b/tests/hooks/pre-bash-commit-quality.test.js @@ -235,6 +235,40 @@ if (test('blocks commits with staged secret patterns across checkable files', () }); })) passed++; else failed++; +if (test('blocks commits with an unquoted API key assignment', () => { + inTempRepo(repoDir => { + writeAndStage(repoDir, 'config.py', [ + 'API_KEY=sk_live_1234567890abcdef', + '' + ].join('\n')); + + const input = JSON.stringify({ tool_input: { command: 'git commit -m "fix: unquoted key"' } }); + const { result, stderr } = captureConsoleError(() => hook.evaluate(input)); + + assert.strictEqual(result.output, input); + assert.strictEqual(result.exitCode, 2); + assert.ok(stderr.includes('Potential API key'), `expected unquoted API key warning, got: ${stderr}`); + }); +})) passed++; else failed++; + +if (test('does not flag ordinary unquoted apiKey code references', () => { + inTempRepo(repoDir => { + writeAndStage(repoDir, 'index.js', [ + 'const apiKey = getApiKeyFromVault();', + 'this.apiKey = options.apiKey;', + 'const apiKey2 = process.env.API_KEY;', + '' + ].join('\n')); + + const input = JSON.stringify({ tool_input: { command: 'git commit -m "fix: no secret here"' } }); + const { result, stderr } = captureConsoleError(() => hook.evaluate(input)); + + assert.strictEqual(result.output, input); + assert.strictEqual(result.exitCode, 0, `expected exit 0 (no secrets), got ${result.exitCode}: ${stderr}`); + assert.ok(!stderr.includes('Potential API key'), `should not flag ordinary code as a secret, got: ${stderr}`); + }); +})) passed++; else failed++; + if (test('reports eslint pylint and golint failures from staged files', () => { inTempRepo(repoDir => { writeAndStage(repoDir, 'index.js', 'const lint = true;\n'); @@ -291,5 +325,52 @@ if (test('stdin entry point truncates oversized input and preserves pass-through assert.ok(result.stderr.includes('[Hook] Error:'), 'truncated JSON should be logged and allowed'); })) passed++; else failed++; +// --- Secret-scanner placeholder exclusion (false-positive fix, no false-negative) --- + +if (test('isPlaceholderSecret suppresses obvious non-secret placeholders', () => { + for (const v of ['process.env.API_KEY', '${API_KEY}', '', 'REPLACE_ME', 'CHANGEME', 'YOUR_API_KEY', '']) { + assert.strictEqual(hook.isPlaceholderSecret(v), true, `should suppress placeholder: ${JSON.stringify(v)}`); + } +})) passed++; else failed++; + +if (test('isPlaceholderSecret does NOT suppress real high-entropy secrets', () => { + for (const v of [ + 'sk-live-abcdef0123456789ABCDEF', // prefixed + '9F8A7B6C5D4E3F2A1B0C9D8E7F6A5B4C', // uppercase hex + 'JBSWY3DPEHPK3PXP', // base32 TOTP/HMAC seed + '1234567890123456', // digit-only token + 'PROD_7F3A9C2E_LIVE_8821', // uppercase-with-underscore token + 'AbCd1234EfGh5678' // mixed token + ]) { + assert.strictEqual(hook.isPlaceholderSecret(v), false, `must NOT suppress real secret: ${v}`); + } +})) passed++; else failed++; + +// --- Quote-aware commit-message extraction (truncation fix) --- + +if (test('captures full double-quoted -m message containing an apostrophe', () => { + const res = hook.validateCommitMessage(`git commit -m "fix: don't crash on empty input"`); + assert.ok(res, 'expected a validation result'); + assert.strictEqual(res.message, "fix: don't crash on empty input"); +})) passed++; else failed++; + +if (test('captures full single-quoted -m message containing a double quote', () => { + const res = hook.validateCommitMessage(`git commit -m 'fix: handle the "edge" case'`); + assert.strictEqual(res.message, 'fix: handle the "edge" case'); +})) passed++; else failed++; + +if (test('captures full double-quoted -m message with escaped inner quotes (not truncated)', () => { + const res = hook.validateCommitMessage('git commit -m "fix: say \\"hello\\" to the user"'); + assert.ok(res, 'expected a validation result'); + assert.strictEqual(res.message, 'fix: say \\"hello\\" to the user'); +})) passed++; else failed++; + +if (test('measures length of the full message past an apostrophe (not the truncated prefix)', () => { + const subject = "fix: it's a deliberately long commit subject that comfortably exceeds seventy-two chars"; + const res = hook.validateCommitMessage(`git commit -m "${subject}"`); + assert.strictEqual(res.message, subject); + assert.ok(res.issues.some(i => i.type === 'length'), 'full (>72) message should trigger a length issue'); +})) passed++; else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); diff --git a/tests/hooks/pre-compact.test.js b/tests/hooks/pre-compact.test.js new file mode 100644 index 000000000..c55d67b7c --- /dev/null +++ b/tests/hooks/pre-compact.test.js @@ -0,0 +1,105 @@ +'use strict'; +/** + * Tests for scripts/hooks/pre-compact.js — worktree-aware active-session + * selection. The sessions dir is shared across projects/worktrees, so the + * hook must annotate the CURRENT worktree's session, not whichever file is + * newest by mtime. selectActiveSessionPath takes an injectable reader so the + * selection logic is tested without touching the filesystem. + */ + +const assert = require('assert'); +const { selectActiveSessionPath } = require('../../scripts/hooks/pre-compact'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (err) { + console.log(` ✗ ${name}`); + console.log(` ${err.message}`); + return false; + } +} + +// Reader built from a path -> content map (returns null for unknown/unreadable). +function reader(map) { + return (p) => (Object.prototype.hasOwnProperty.call(map, p) ? map[p] : null); +} + +const A = '/ecc-pre-compact-test/work/projA'; +const B = '/ecc-pre-compact-test/work/projB'; + +if (test('selects the session matching the current worktree, not the newest', () => { + const sessions = [ + { path: '/sessions/newest-session.tmp' }, // newest, different worktree + { path: '/sessions/older-session.tmp' }, // older, our worktree + ]; + const map = { + '/sessions/newest-session.tmp': `**Project:** projB\n**Worktree:** ${B}\n`, + '/sessions/older-session.tmp': `**Project:** projA\n**Worktree:** ${A}\n`, + }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/older-session.tmp'); +})) passed++; else failed++; + +if (test('returns null when no session matches the current worktree (no foreign write)', () => { + const sessions = [{ path: '/sessions/b-session.tmp' }]; + const map = { '/sessions/b-session.tmp': `**Project:** projB\n**Worktree:** ${B}\n` }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null); +})) passed++; else failed++; + +if (test('falls back to a legacy session (no Worktree header) with matching project name', () => { + const sessions = [{ path: '/sessions/legacy-session.tmp' }]; + const map = { '/sessions/legacy-session.tmp': '**Project:** projA\n' }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/legacy-session.tmp'); +})) passed++; else failed++; + +if (test('does not project-match a session that has an explicit non-matching Worktree', () => { + const sessions = [{ path: '/sessions/x-session.tmp' }]; + const map = { '/sessions/x-session.tmp': `**Project:** projA\n**Worktree:** ${B}\n` }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null); +})) passed++; else failed++; + +if (test('does not project-match a session whose Worktree header is present but blank', () => { + // A blank/whitespace Worktree header is NOT a legacy session, so it must not + // fall back to project-name matching and attach to a foreign session. + const sessions = [{ path: '/sessions/blank-session.tmp' }]; + const map = { '/sessions/blank-session.tmp': '**Project:** projA\n**Worktree:** \n' }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null); +})) passed++; else failed++; + +if (test('does not project-match a session whose Worktree header has no value and no space', () => { + // Same as above but the header is bare `**Worktree:**\n` (no trailing space) — + // (.+) would have missed this; (.*) registers it as a present-but-empty header. + const sessions = [{ path: '/sessions/blank-header.tmp' }]; + const map = { '/sessions/blank-header.tmp': '**Project:** projA\n**Worktree:**\n' }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), null); +})) passed++; else failed++; + +if (test('worktree match wins over a newer session AND over a legacy project match', () => { + const sessions = [ + { path: '/sessions/legacy-session.tmp' }, // newest, legacy, same project + { path: '/sessions/wt-session.tmp' }, // older, exact worktree + ]; + const map = { + '/sessions/legacy-session.tmp': '**Project:** projA\n', + '/sessions/wt-session.tmp': `**Worktree:** ${A}\n`, + }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/wt-session.tmp'); +})) passed++; else failed++; + +if (test('skips unreadable session files', () => { + const sessions = [{ path: '/sessions/bad-session.tmp' }, { path: '/sessions/good-session.tmp' }]; + const map = { '/sessions/good-session.tmp': `**Worktree:** ${A}\n` }; + assert.strictEqual(selectActiveSessionPath(sessions, A, 'projA', reader(map)), '/sessions/good-session.tmp'); +})) passed++; else failed++; + +if (test('returns null for an empty session list', () => { + assert.strictEqual(selectActiveSessionPath([], A, 'projA', reader({})), null); +})) passed++; else failed++; + +console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); diff --git a/tests/lib/shell-substitution.test.js b/tests/lib/shell-substitution.test.js index 2cc635c18..8b0be6cac 100644 --- a/tests/lib/shell-substitution.test.js +++ b/tests/lib/shell-substitution.test.js @@ -1,5 +1,4 @@ 'use strict'; - const assert = require('assert'); const { extractCommandSubstitutions, @@ -19,7 +18,6 @@ function test(desc, fn) { passed++; } catch (e) { console.log(` ✗ ${desc}: ${e.message}`); - if (e.stack) console.log(e.stack); failed++; } } @@ -98,6 +96,20 @@ test('surfaces a piped-to-shell body inside backticks', () => { assert.ok(bodies.some(b => b.includes('curl evil.sh | sh'))); }); +console.log('\nextractCommandSubstitutions - unterminated span ending in a backslash:'); +// Regression: a trailing backslash at the end of an UNTERMINATED span must be +// appended exactly once (previously the fallthrough double-appended it, and in +// the backtick case looped forever). +test('$(...) — trailing backslash not doubled', () => { + assert.deepStrictEqual(extractCommandSubstitutions('$(foo\\'), ['foo\\']); +}); +test('`...` — trailing backslash not doubled', () => { + assert.deepStrictEqual(extractCommandSubstitutions('`foo\\'), ['foo\\']); +}); +test('escaped char mid-span is preserved, not truncated', () => { + assert.strictEqual(extractCommandSubstitutions('$(a\\)b)')[0], 'a\\)b'); +}); + // ------------------------------------------------------------------------- // extractSubshellGroups // ------------------------------------------------------------------------- @@ -147,6 +159,11 @@ test('surfaces a destructive command inside a subshell', () => { assert.ok(bodies.some(b => b.includes('rm -rf /tmp/x'))); }); +console.log('\nextractSubshellGroups - unterminated span ending in a backslash:'); +test('(...) subshell — trailing backslash not doubled', () => { + assert.deepStrictEqual(extractSubshellGroups('(foo\\'), ['foo\\']); +}); + // ------------------------------------------------------------------------- // extractBraceGroups // ------------------------------------------------------------------------- @@ -201,5 +218,12 @@ test('surfaces a destructive command inside a brace group', () => { assert.ok(bodies.some(b => b.includes('rm -rf /tmp/x'))); }); +console.log('\nextractBraceGroups - unterminated span ending in a backslash:'); +test('{ ...; } brace — trailing backslash not doubled', () => { + assert.deepStrictEqual(extractBraceGroups('{ foo\\'), [' foo\\']); +}); + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); -process.exit(failed > 0 ? 1 : 0); +if (failed > 0) { + process.exit(1); +} diff --git a/tests/lib/utils.test.js b/tests/lib/utils.test.js index 54fa8dfca..158db6c73 100644 --- a/tests/lib/utils.test.js +++ b/tests/lib/utils.test.js @@ -1114,16 +1114,90 @@ function runTests() { return true; } const { execFileSync } = require('child_process'); - // maxSize is a chunk-level guard: once data.length >= maxSize, no MORE chunks are added. - // A single small chunk that arrives when data.length < maxSize is added in full. - // To test multi-chunk behavior, we send >64KB (Node default highWaterMark=16KB) - // which should arrive in multiple chunks. With maxSize=100, only the first chunk(s) - // totaling under 100 bytes should be captured; subsequent chunks are dropped. + // Send enough data to cross the chunk-level cap. The child must keep + // draining stdin until EOF so the parent does not see EPIPE on macOS. const script = 'const u=require("./scripts/lib/utils");u.readStdinJson({timeoutMs:2000,maxSize:100}).then(d=>{process.stdout.write(JSON.stringify(d))})'; - // Generate 100KB of data (arrives in multiple chunks) const bigInput = '{"k":"' + 'X'.repeat(100000) + '"}'; const result = execFileSync('node', ['-e', script], { ...stdinOpts, input: bigInput }); - // Truncated mid-string → invalid JSON → resolves to {} + // Oversized input is rejected rather than parsing a partial JSON prefix. + assert.deepStrictEqual(JSON.parse(result), {}); + })) passed++; else failed++; + + if (test('readStdinJson overflow drain still exits when the writer never closes stdin', () => { + const { execFileSync } = require('child_process'); + const childScript = [ + 'const u=require("./scripts/lib/utils");', + 'u.readStdinJson({timeoutMs:100,maxSize:100})', + '.then(d=>process.stdout.write(JSON.stringify(d)));' + ].join(''); + const harness = ` + const { spawn } = require('child_process'); + const child = spawn(process.execPath, ['-e', ${JSON.stringify(childScript)}], { + cwd: process.cwd(), + stdio: ['pipe', 'pipe', 'inherit'] + }); + let stdout = ''; + child.stdout.setEncoding('utf8'); + child.stdout.on('data', chunk => { stdout += chunk; }); + child.stdin.write('X'.repeat(100000)); + const deadline = setTimeout(() => { + child.kill(); + process.exit(2); + }, 1000); + child.on('exit', code => { + clearTimeout(deadline); + if (code !== 0) process.exit(code || 1); + process.stdout.write(stdout); + }); + `; + const result = execFileSync('node', ['-e', harness], { + ...stdinOpts, + timeout: 2000 + }); + assert.deepStrictEqual(JSON.parse(result), {}); + })) passed++; else failed++; + + if (test('readStdinJson drains a slow finite oversized writer without EPIPE', () => { + const { execFileSync } = require('child_process'); + const childScript = [ + 'const u=require("./scripts/lib/utils");', + 'u.readStdinJson({timeoutMs:500,maxSize:100})', + '.then(d=>process.stdout.write(JSON.stringify(d)));' + ].join(''); + const harness = ` + const { spawn } = require('child_process'); + const child = spawn(process.execPath, ['-e', ${JSON.stringify(childScript)}], { + cwd: process.cwd(), + stdio: ['pipe', 'pipe', 'inherit'] + }); + let stdout = ''; + let writes = 0; + child.stdout.setEncoding('utf8'); + child.stdout.on('data', chunk => { stdout += chunk; }); + child.stdin.on('error', () => process.exit(3)); + const writer = setInterval(() => { + writes += 1; + child.stdin.write('X'.repeat(5000)); + if (writes === 20) { + clearInterval(writer); + child.stdin.end(); + } + }, 5); + const deadline = setTimeout(() => { + child.kill(); + process.exit(2); + }, 1500); + child.on('exit', code => { + clearInterval(writer); + clearTimeout(deadline); + if (code !== 0) process.exit(code || 1); + process.stdout.write(stdout); + }); + `; + const result = execFileSync('node', ['-e', harness], { + ...stdinOpts, + timeout: 2000 + }); assert.deepStrictEqual(JSON.parse(result), {}); })) passed++; else failed++; From afa34d1acab45a7194ae230179e4d5dbdf322b98 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:33:18 -0400 Subject: [PATCH 3/4] chore(deps): bump the cargo-minor-and-patch group (#2593) Bumps the cargo-minor-and-patch group in /ecc2 with 4 updates: [tokio](https://github.com/tokio-rs/tokio), [serde_json](https://github.com/serde-rs/json), [clap](https://github.com/clap-rs/clap) and [libc](https://github.com/rust-lang/libc). Updates `tokio` from 1.53.0 to 1.53.1 - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](https://github.com/tokio-rs/tokio/compare/tokio-1.53.0...tokio-1.53.1) Updates `serde_json` from 1.0.150 to 1.0.151 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](https://github.com/serde-rs/json/compare/v1.0.150...v1.0.151) Updates `clap` from 4.6.2 to 4.6.4 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](https://github.com/clap-rs/clap/compare/clap_complete-v4.6.2...clap_complete-v4.6.4) Updates `libc` from 0.2.186 to 0.2.189 - [Release notes](https://github.com/rust-lang/libc/releases) - [Changelog](https://github.com/rust-lang/libc/blob/0.2.189/CHANGELOG.md) - [Commits](https://github.com/rust-lang/libc/compare/0.2.186...0.2.189) --- updated-dependencies: - dependency-name: tokio dependency-version: 1.53.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch - dependency-name: serde_json dependency-version: 1.0.151 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch - dependency-name: clap dependency-version: 4.6.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch - dependency-name: libc dependency-version: 0.2.189 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- ecc2/Cargo.lock | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ecc2/Cargo.lock b/ecc2/Cargo.lock index 307fdf1d6..1dc3a77cd 100644 --- a/ecc2/Cargo.lock +++ b/ecc2/Cargo.lock @@ -236,9 +236,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.6.2" +version = "4.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd059f9da4f5c36b3787f65d38ccaab1cc315f07b01f89abc8359ee6a8205011" +checksum = "d91e0c145792ef73a6ad36d27c75ac09f1832222a3c209689d90f534685ee5b7" dependencies = [ "clap_builder", "clap_derive", @@ -258,14 +258,14 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.6.1" +version = "4.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2ce8604710f6733aa641a2b3731eaa1e8b3d9973d5e3565da11800813f997a9" +checksum = "d012d2b9d65aca7f18f4d9878a045bc17899bba951561ba5ec3c2ba1eed9a061" dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.117", + "syn 3.0.2", ] [[package]] @@ -1111,9 +1111,9 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "libc" -version = "0.2.186" +version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libgit2-sys" @@ -1954,9 +1954,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.150" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8014e44b4736ed0538adeecded0fce2a272f22dc9578a7eb6b2d9993c74cfb9" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "itoa", "memchr", @@ -2341,9 +2341,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.53.0" +version = "1.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d988bcd52dbe076d3d46903332f58c912b87a2c49b1428419a5845154762ffee" +checksum = "202caea871b69668250d242070849eb495be178ed697a3e98aebce5bc81a0bed" dependencies = [ "bytes", "libc", From 591ab5cbd3f2f65860ea91c226e410b1502c8e2e Mon Sep 17 00:00:00 2001 From: Shanujan Suresh <207912128+shanujans@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:07:41 +0530 Subject: [PATCH 4/4] fix(opencode): don't crash the whole session when plugins/lib is missing (#2538) * fix(opencode): don't crash the whole session when plugins/lib is missing (#2530) * fix: address review feedback (trailing newlines, symptom wording) * fix(opencode): also lazy-load the store in ecc-hooks.ts plugin entrypoint * test(opencode): add regression coverage for missing/present plugins/lib in ecc-hooks * fix(opencode): guard diagnostic log() against unhandled rejection; tighten test assertion * fix(opencode): only publish store after init succeeds; scrub loader errors from warning; catch sync log throws * fix(opencode): scrub raw loader error from changed-files tool; add tool-level regression tests * fix(ci): make OpenCode checks cross-platform --------- Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> --- .opencode/plugins/ecc-hooks.ts | 49 ++++++++++++---- .opencode/tools/changed-files.ts | 35 +++++++++--- TROUBLESHOOTING.md | 38 +++++++++++++ tests/opencode-plugin-hooks.test.js | 86 +++++++++++++++++++++++++++++ tests/opencode-tools.test.js | 78 ++++++++++++++++++++++++++ 5 files changed, 267 insertions(+), 19 deletions(-) diff --git a/.opencode/plugins/ecc-hooks.ts b/.opencode/plugins/ecc-hooks.ts index 6336081e9..49124c255 100644 --- a/.opencode/plugins/ecc-hooks.ts +++ b/.opencode/plugins/ecc-hooks.ts @@ -16,11 +16,6 @@ import type { PluginInput } from "@opencode-ai/plugin" import * as fs from "fs" import * as path from "path" -import { - initStore, - recordChange, - clearChanges, -} from "./lib/changed-files-store.js" import changedFilesTool from "../tools/changed-files.js" import dependencyAnalyzerTool from "../tools/dependency-analyzer.js" @@ -80,7 +75,6 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({ type HookProfile = "minimal" | "standard" | "strict" const worktreePath = worktree || directory - initStore(worktreePath) const editedFiles = new Set() @@ -110,6 +104,37 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({ const log = (level: "debug" | "info" | "warn" | "error", message: string) => client.app.log({ body: { service: "ecc", level, message } }) + // Loaded lazily (instead of via a top-level import) so that a missing or + // partially-installed `~/.opencode/plugins/lib` directory (e.g. an + // interrupted or partial ECC install on Termux/Android) only disables + // changed-files tracking, rather than throwing during module evaluation. + // This plugin is OpenCode's startup entry point, so a static import + // failure here previously crashed the whole plugin -- and with it, the + // entire OpenCode session -- before any hooks could load (see #2530). + let changedFilesStore: typeof import("./lib/changed-files-store.js") | undefined + try { + const store = await import("./lib/changed-files-store.js") + store.initStore(worktreePath) + changedFilesStore = store + } catch { + // Best-effort diagnostic only: deferred via .then() (rather than + // Promise.resolve(log(...))) so that even a *synchronous* throw inside + // log() -- not just an async rejection -- is caught here instead of + // escaping this catch block. The raw loader error is intentionally not + // included in the message since it can contain absolute filesystem + // paths; this whole block exists to guarantee startup resilience even + // when things go wrong. + Promise.resolve() + .then(() => + log( + "warn", + "[ECC] changed-files tracking disabled: could not load the changed-files store. " + + "Run `ecc repair --target opencode` to restore the missing files. Other ECC hooks are unaffected." + ) + ) + .catch(() => {}) + } + const normalizeProfile = (value: string | undefined): HookProfile => { if (value === "minimal" || value === "strict") return value return "standard" @@ -154,7 +179,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({ */ "file.edited": async (event: { path: string }) => { editedFiles.add(event.path) - recordChange(event.path, "modified") + changedFilesStore?.recordChange(event.path, "modified") // Auto-format JS/TS files if (hookEnabled("post:edit:format", ["strict"]) && event.path.match(/\.(ts|tsx|js|jsx)$/)) { @@ -198,16 +223,16 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({ ) => { const filePath = getFilePath(input.args) if (input.tool === "edit" && filePath) { - recordChange(filePath, "modified") + changedFilesStore?.recordChange(filePath, "modified") } if (input.tool === "write" && filePath) { const key = input.callID ?? `write-${++writeCounter}-${filePath}` const pending = pendingToolChanges.get(key) if (pending) { - recordChange(pending.path, pending.type) + changedFilesStore?.recordChange(pending.path, pending.type) pendingToolChanges.delete(key) } else { - recordChange(filePath, "modified") + changedFilesStore?.recordChange(filePath, "modified") } } @@ -413,7 +438,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({ if (!hookEnabled("session:end-marker", ["minimal", "standard", "strict"])) return log("info", "[ECC] Session ended - cleaning up") editedFiles.clear() - clearChanges() + changedFilesStore?.clearChanges() pendingToolChanges.clear() }, @@ -428,7 +453,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({ let changeType: "added" | "modified" | "deleted" = "modified" if (event.type === "create" || event.type === "add") changeType = "added" else if (event.type === "delete" || event.type === "remove") changeType = "deleted" - recordChange(event.path, changeType) + changedFilesStore?.recordChange(event.path, changeType) if (event.type === "change" && event.path.match(/\.(ts|tsx|js|jsx)$/)) { editedFiles.add(event.path) } diff --git a/.opencode/tools/changed-files.ts b/.opencode/tools/changed-files.ts index a6751bcc3..1150ca756 100644 --- a/.opencode/tools/changed-files.ts +++ b/.opencode/tools/changed-files.ts @@ -1,11 +1,5 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool" -import { - buildTree, - getChangedPaths, - hasChanges, - type ChangeType, - type TreeNode, -} from "../plugins/lib/changed-files-store.js" +import type { ChangeType, TreeNode } from "../plugins/lib/changed-files-store.js" const INDICATORS: Record = { added: "+", @@ -26,6 +20,32 @@ function renderTree(nodes: TreeNode[], indent: string): string { return lines.join("\n") } +// Loaded lazily (instead of via a top-level import) so that a missing or +// partially-installed `~/.opencode/plugins` directory only breaks this one +// tool when it's actually invoked, rather than throwing during module +// evaluation. `tools/index.ts` re-exports every tool from a single barrel +// file, so a static import failure here previously took down the entire +// tools module -- and with it, the whole OpenCode session -- on the very +// first tool-loading pass (see #2530). +type ChangedFilesStore = typeof import("../plugins/lib/changed-files-store.js") +let changedFilesStorePromise: Promise | undefined + +async function loadChangedFilesStore(): Promise { + if (!changedFilesStorePromise) { + changedFilesStorePromise = import("../plugins/lib/changed-files-store.js").catch(() => { + changedFilesStorePromise = undefined + throw new Error( + "changed-files tool: could not load the changed-files store. " + + "This usually means the ~/.opencode/plugins directory is missing or incomplete " + + "(an interrupted or partial ECC install can leave tools/ populated without plugins/). " + + "Run `node scripts/repair.js --target opencode` (or `ecc repair --target opencode`) " + + "from the ECC repo to restore the missing files." + ) + }) + } + return changedFilesStorePromise +} + const changedFilesTool: ToolDefinition = tool({ description: "List files changed by agents in this session as a navigable tree. Shows added (+), modified (~), and deleted (-) indicators. Use filter to show only specific change types. Returns paths for git diff.", @@ -40,6 +60,7 @@ const changedFilesTool: ToolDefinition = tool({ .describe("Output format: tree for terminal display, json for structured data (default: tree)"), }, async execute(args, context) { + const { buildTree, getChangedPaths, hasChanges } = await loadChangedFilesStore() const filter = args.filter === "all" || !args.filter ? undefined : (args.filter as ChangeType) const format = args.format ?? "tree" diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index 1681010fe..5461c0d10 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -305,6 +305,44 @@ npm pkg set packageManager="pnpm@8.15.0" rm package-lock.json # If using pnpm/yarn/bun ``` +### OpenCode Fails to Start on Termux/Android + +**Symptom:** Changed-files tracking silently stops working (a one-time +`[ECC] changed-files tracking disabled` warning appears in the OpenCode +logs), or (on older versions) `opencode` crashes on startup entirely with a +Bun `ResolveMessage`, e.g.: + +``` +ResolveMessage: Cannot find module '../plugins/lib/changed-files-store.js' from '.../.opencode/tools/changed-files.ts' +``` + +**Causes:** +- The `~/.opencode` install is missing or incomplete for this machine — + usually `tools/` and `plugins/` are present but `plugins/lib/` never + finished copying (an interrupted install, or a storage/permission hiccup + that's more common on Android's filesystem). Both the `changed-files` tool + and the `ecc-hooks` plugin depend on `plugins/lib/changed-files-store.js`; + since `ecc-hooks.ts` is OpenCode's plugin entry point (loaded once at + session startup, before `tools/index.ts`'s barrel file), a missing + dependency there used to crash the entire OpenCode session before any + hooks could load — not just the one tool. + +**Solutions:** +```bash +# From the ECC repo, check for and repair missing/incomplete managed files +ecc doctor --target opencode +ecc repair --target opencode + +# If that reports no drift but plugins/ is still missing on the device, +# re-run the ECC installer for the opencode target +``` + +**Note:** If you're also seeing `ProviderModelNotFoundError: Model not found: openai/gpt-5.5` +referencing `~/.config/opencode/oh-my-opencode-slim.json`, that file belongs to the +third-party [`oh-my-opencode-slim`](https://github.com/alvinunreal/oh-my-opencode-slim) +plugin, not ECC — ECC never writes to `~/.config/opencode/`. Fix the model prefix +(`opencode/...` instead of `openai/...`) there, or file it against that project. + --- ## Performance Issues diff --git a/tests/opencode-plugin-hooks.test.js b/tests/opencode-plugin-hooks.test.js index b0c7ad8cc..0d261ef5c 100644 --- a/tests/opencode-plugin-hooks.test.js +++ b/tests/opencode-plugin-hooks.test.js @@ -84,6 +84,92 @@ async function main() { const { ECCHooksPlugin } = await loadPlugin() const tests = [ + [ + "plugin initializes and hooks stay usable when plugins/lib is missing", + async () => withTempProject([], async (projectDir) => { + const repoRoot = path.join(__dirname, "..") + const libDir = path.join(repoRoot, ".opencode", "dist", "plugins", "lib") + const backupDir = path.join( + repoRoot, + ".opencode", + "dist", + "plugins", + "lib.missing-store-test-backup" + ) + fs.renameSync(libDir, backupDir) + try { + const client = createClient() + const $ = createFailingShell() + + // Plugin initialization must resolve even though changed-files-store.js + // cannot be found -- it must not throw and crash session startup (#2530). + const hooks = await ECCHooksPlugin({ client, $, directory: projectDir }) + + const disabledWarnings = client.logs.filter( + (entry) => + entry.level === "warn" && + entry.message.includes("[ECC] changed-files tracking disabled") && + entry.message.includes("ecc repair --target opencode") + ) + assert.strictEqual( + disabledWarnings.length, + 1, + "Expected exactly one warning when plugins/lib/changed-files-store.js cannot be loaded" + ) + + // Every hook that touches the store must remain callable and must not throw. + await hooks["file.edited"]({ path: "src/example.ts" }) + await hooks["tool.execute.after"]({ tool: "edit", args: { path: "src/other.ts" } }, {}) + await hooks["session.deleted"]() + } finally { + fs.renameSync(backupDir, libDir) + } + }), + ], + [ + "changed-files tracking records and clears through the plugin hooks", + async () => withTempProject([], async (projectDir) => { + const client = createClient() + const $ = createFailingShell() + + const hooks = await ECCHooksPlugin({ client, $, directory: projectDir }) + + assert.ok( + !client.logs.some( + (entry) => entry.level === "warn" && entry.message.includes("changed-files tracking disabled") + ), + "Did not expect a disabled warning when plugins/lib is present" + ) + + const storeUrl = pathToFileURL( + path.join(__dirname, "..", ".opencode", "dist", "plugins", "lib", "changed-files-store.js") + ).href + const store = await import(storeUrl) + + await hooks["file.edited"]({ path: "src/example.ts" }) + assert.ok( + store + .getChangedPaths() + .some( + (entry) => + entry.path === path.normalize("src/example.ts") && + entry.changeType === "modified" + ), + "Expected file.edited to record a change via the plugin hook" + ) + + await hooks["tool.execute.after"]({ tool: "edit", args: { path: "src/other.ts" } }, {}) + assert.ok( + store + .getChangedPaths() + .some((entry) => entry.path === path.normalize("src/other.ts")), + "Expected tool.execute.after to record a change for the edit tool" + ) + + await hooks["session.deleted"]() + assert.ok(!store.hasChanges(), "Expected session.deleted to clear tracked changes") + }), + ], [ "shell.env detects project markers without shelling out to test -f", async () => withTempProject( diff --git a/tests/opencode-tools.test.js b/tests/opencode-tools.test.js index a09f067dc..67eceed66 100644 --- a/tests/opencode-tools.test.js +++ b/tests/opencode-tools.test.js @@ -234,6 +234,84 @@ async function main() { ]) } + // Test changed-files tool + if (tools.changedfiles) { + tests.push([ + "changed-files: reports an actionable, scrubbed error when plugins/lib is missing", + async () => withTempProject([], async (projectDir) => { + const repoRoot = path.join(__dirname, "..") + const libDir = path.join(repoRoot, ".opencode", "dist", "plugins", "lib") + const backupDir = path.join( + repoRoot, + ".opencode", + "dist", + "plugins", + "lib.missing-store-test-backup" + ) + fs.renameSync(libDir, backupDir) + try { + const context = createMockContext(projectDir) + await assert.rejects( + () => tools.changedfiles.execute({}, context), + (error) => { + assert.ok(error instanceof Error) + assert.ok( + error.message.includes("ecc repair --target opencode"), + "Expected the error to point at the repair command" + ) + assert.ok( + !error.message.includes(repoRoot), + "Error message must not leak the local filesystem path" + ) + assert.ok( + !error.message.includes("Original error"), + "Error message must not include the raw underlying loader error" + ) + return true + } + ) + } finally { + fs.renameSync(backupDir, libDir) + } + }), + ]) + + tests.push([ + "changed-files: renders tracked changes once plugins/lib is present", + async () => withTempProject([], async (projectDir) => { + const repoRoot = path.join(__dirname, "..") + const storeUrl = pathToFileURL( + path.join(repoRoot, ".opencode", "dist", "plugins", "lib", "changed-files-store.js") + ).href + const store = await import(storeUrl) + store.initStore(projectDir) + store.clearChanges() + store.recordChange("src/example.ts", "modified") + store.recordChange("src/new-file.ts", "added") + + try { + const context = createMockContext(projectDir) + const result = await tools.changedfiles.execute({ format: "json" }, context) + const parsed = JSON.parse(result) + assert.strictEqual(parsed.changed, true) + assert.ok( + parsed.files.some( + (f) => + f.path === path.normalize("src/example.ts") && f.changeType === "modified" + ) + ) + assert.ok( + parsed.files.some( + (f) => f.path === path.normalize("src/new-file.ts") && f.changeType === "added" + ) + ) + } finally { + store.clearChanges() + } + }), + ]) + } + // Run all tests let passed = 0 let failed = 0