mirror of
https://github.com/affaan-m/ECC.git
synced 2026-11-07 08:35:54 +00:00
fix(hooks): skip stdin listeners on require and match exact tsc paths in post-edit-typecheck
config-protection.js and post-edit-typecheck.js attached process.stdin listeners at module scope. tests/hooks/hook-input.test.js requires config-protection.js, so that test never exits while stdin stays open. Attach the listeners only when the script runs directly (require.main === module), as stop-format-typecheck.js and quality-gate.js already do. post-edit-typecheck.js picked tsc diagnostics with line.includes(candidate), so editing a root index.ts reported the errors of src/index.ts, and a Windows relative path (src\index.ts) never matched tsc's forward-slash output. Compare the file at the start of each diagnostic instead, the same check diagnosticMatchesFile() does in stop-format-typecheck.js. Refs #3821
This commit is contained in:
1 parent
b38be6a7f6
commit
00a8fe96f8
4 files changed
+268
-100
No files matched your search
@@ -232,31 +232,33 @@ function checkTarget(filePath) {
|
||||
module.exports = { run };
|
||||
|
||||
// Stdin fallback for spawnSync execution
|
||||
let truncated = /^(1|true|yes)$/i.test(String(process.env.ECC_HOOK_INPUT_TRUNCATED || ''));
|
||||
process.stdin.setEncoding('utf8');
|
||||
process.stdin.on('data', chunk => {
|
||||
if (raw.length < MAX_STDIN) {
|
||||
const remaining = MAX_STDIN - raw.length;
|
||||
raw += chunk.substring(0, remaining);
|
||||
if (chunk.length > remaining) truncated = true;
|
||||
} else {
|
||||
truncated = true;
|
||||
}
|
||||
});
|
||||
|
||||
process.stdin.on('end', () => {
|
||||
const result = run(raw, {
|
||||
truncated,
|
||||
maxStdin: Number(process.env.ECC_HOOK_INPUT_MAX_BYTES) || MAX_STDIN
|
||||
if (require.main === module) {
|
||||
let truncated = /^(1|true|yes)$/i.test(String(process.env.ECC_HOOK_INPUT_TRUNCATED || ''));
|
||||
process.stdin.setEncoding('utf8');
|
||||
process.stdin.on('data', chunk => {
|
||||
if (raw.length < MAX_STDIN) {
|
||||
const remaining = MAX_STDIN - raw.length;
|
||||
raw += chunk.substring(0, remaining);
|
||||
if (chunk.length > remaining) truncated = true;
|
||||
} else {
|
||||
truncated = true;
|
||||
}
|
||||
});
|
||||
|
||||
if (result.stderr) {
|
||||
process.stderr.write(result.stderr + '\n');
|
||||
}
|
||||
process.stdin.on('end', () => {
|
||||
const result = run(raw, {
|
||||
truncated,
|
||||
maxStdin: Number(process.env.ECC_HOOK_INPUT_MAX_BYTES) || MAX_STDIN
|
||||
});
|
||||
|
||||
if (result.exitCode === 2) {
|
||||
process.exit(2);
|
||||
}
|
||||
if (result.stderr) {
|
||||
process.stderr.write(result.stderr + '\n');
|
||||
}
|
||||
|
||||
process.stdout.write(raw);
|
||||
});
|
||||
if (result.exitCode === 2) {
|
||||
process.exit(2);
|
||||
}
|
||||
|
||||
process.stdout.write(raw);
|
||||
});
|
||||
}
|
||||
@@ -14,92 +14,107 @@ const fs = require("fs");
|
||||
const path = require("path");
|
||||
|
||||
const MAX_DIRECT_STDIN_BYTES = 16 * 1024 * 1024;
|
||||
let data = "";
|
||||
let stdinBytes = 0;
|
||||
let oversized = false;
|
||||
process.stdin.setEncoding("utf8");
|
||||
|
||||
process.stdin.on("data", (chunk) => {
|
||||
if (oversized) return;
|
||||
stdinBytes += Buffer.byteLength(chunk, "utf8");
|
||||
if (stdinBytes > MAX_DIRECT_STDIN_BYTES) {
|
||||
data = "";
|
||||
oversized = true;
|
||||
return;
|
||||
// tsc --pretty false starts each diagnostic with the file it is about:
|
||||
// `src/a.ts(1,14): error TS2322: ...`. Compare that path instead of looking
|
||||
// for the edited path anywhere in the line, which let a root `index.ts` claim
|
||||
// the errors of `src/index.ts`. Same check as diagnosticMatchesFile() in
|
||||
// stop-format-typecheck.js.
|
||||
function diagnosticMatchesFile(line, paths) {
|
||||
const location = /^(.+?)\(\d+,\d+\): /.exec(line);
|
||||
if (!location) return false;
|
||||
const file = location[1].replace(/\\/g, "/");
|
||||
for (const candidate of paths) {
|
||||
if (file === candidate.replace(/\\/g, "/")) return true;
|
||||
}
|
||||
data += chunk;
|
||||
});
|
||||
return false;
|
||||
}
|
||||
|
||||
process.stdin.on("end", () => {
|
||||
if (oversized) {
|
||||
process.exit(0);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const input = JSON.parse(data);
|
||||
const filePath = input.tool_input?.file_path;
|
||||
if (require.main === module) {
|
||||
let data = "";
|
||||
let stdinBytes = 0;
|
||||
let oversized = false;
|
||||
process.stdin.setEncoding("utf8");
|
||||
|
||||
if (filePath && /\.(ts|tsx)$/.test(filePath)) {
|
||||
const resolvedPath = path.resolve(filePath);
|
||||
if (!fs.existsSync(resolvedPath)) {
|
||||
process.stdout.write(data, () => process.exit(0));
|
||||
return;
|
||||
}
|
||||
// Find nearest tsconfig.json by walking up (max 20 levels to prevent infinite loop)
|
||||
let dir = path.dirname(resolvedPath);
|
||||
const root = path.parse(dir).root;
|
||||
let depth = 0;
|
||||
process.stdin.on("data", (chunk) => {
|
||||
if (oversized) return;
|
||||
stdinBytes += Buffer.byteLength(chunk, "utf8");
|
||||
if (stdinBytes > MAX_DIRECT_STDIN_BYTES) {
|
||||
data = "";
|
||||
oversized = true;
|
||||
return;
|
||||
}
|
||||
data += chunk;
|
||||
});
|
||||
|
||||
while (dir !== root && depth < 20) {
|
||||
if (fs.existsSync(path.join(dir, "tsconfig.json"))) {
|
||||
break;
|
||||
process.stdin.on("end", () => {
|
||||
if (oversized) {
|
||||
process.exit(0);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const input = JSON.parse(data);
|
||||
const filePath = input.tool_input?.file_path;
|
||||
|
||||
if (filePath && /\.(ts|tsx)$/.test(filePath)) {
|
||||
const resolvedPath = path.resolve(filePath);
|
||||
if (!fs.existsSync(resolvedPath)) {
|
||||
process.stdout.write(data, () => process.exit(0));
|
||||
return;
|
||||
}
|
||||
dir = path.dirname(dir);
|
||||
depth++;
|
||||
}
|
||||
// Find nearest tsconfig.json by walking up (max 20 levels to prevent infinite loop)
|
||||
let dir = path.dirname(resolvedPath);
|
||||
const root = path.parse(dir).root;
|
||||
let depth = 0;
|
||||
|
||||
if (fs.existsSync(path.join(dir, "tsconfig.json"))) {
|
||||
try {
|
||||
// Use npx.cmd on Windows to avoid shell: true which enables command injection
|
||||
const npxBin = process.platform === "win32" ? "npx.cmd" : "npx";
|
||||
execFileSync(npxBin, ["tsc", "--noEmit", "--pretty", "false"], {
|
||||
cwd: dir,
|
||||
encoding: "utf8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
timeout: 30000,
|
||||
});
|
||||
} catch (err) {
|
||||
// tsc exits non-zero when there are errors — filter to edited file
|
||||
const output = (err.stdout || "") + (err.stderr || "");
|
||||
// Compute paths that uniquely identify the edited file.
|
||||
// tsc output uses paths relative to its cwd (the tsconfig dir),
|
||||
// so check for the relative path, absolute path, and original path.
|
||||
// Avoid bare basename matching — it causes false positives when
|
||||
// multiple files share the same name (e.g., src/utils.ts vs tests/utils.ts).
|
||||
const relPath = path.relative(dir, resolvedPath);
|
||||
const candidates = new Set([filePath, resolvedPath, relPath]);
|
||||
const relevantLines = output
|
||||
.split("\n")
|
||||
.filter((line) => {
|
||||
for (const candidate of candidates) {
|
||||
if (line.includes(candidate)) return true;
|
||||
}
|
||||
return false;
|
||||
})
|
||||
.slice(0, 10);
|
||||
while (dir !== root && depth < 20) {
|
||||
if (fs.existsSync(path.join(dir, "tsconfig.json"))) {
|
||||
break;
|
||||
}
|
||||
dir = path.dirname(dir);
|
||||
depth++;
|
||||
}
|
||||
|
||||
if (relevantLines.length > 0) {
|
||||
console.error(
|
||||
"[Hook] TypeScript errors in " + path.basename(filePath) + ":",
|
||||
);
|
||||
relevantLines.forEach((line) => console.error(line));
|
||||
if (fs.existsSync(path.join(dir, "tsconfig.json"))) {
|
||||
try {
|
||||
// Use npx.cmd on Windows to avoid shell: true which enables command injection
|
||||
const npxBin = process.platform === "win32" ? "npx.cmd" : "npx";
|
||||
execFileSync(npxBin, ["tsc", "--noEmit", "--pretty", "false"], {
|
||||
cwd: dir,
|
||||
encoding: "utf8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
timeout: 30000,
|
||||
});
|
||||
} catch (err) {
|
||||
// tsc exits non-zero when there are errors — filter to edited file
|
||||
const output = (err.stdout || "") + (err.stderr || "");
|
||||
// Compute paths that uniquely identify the edited file.
|
||||
// tsc output uses paths relative to its cwd (the tsconfig dir),
|
||||
// so check for the relative path, absolute path, and original path.
|
||||
// Avoid bare basename matching — it causes false positives when
|
||||
// multiple files share the same name (e.g., src/utils.ts vs tests/utils.ts).
|
||||
const relPath = path.relative(dir, resolvedPath);
|
||||
const candidates = new Set([filePath, resolvedPath, relPath]);
|
||||
const relevantLines = output
|
||||
.split("\n")
|
||||
.filter((line) => diagnosticMatchesFile(line, candidates))
|
||||
.slice(0, 10);
|
||||
|
||||
if (relevantLines.length > 0) {
|
||||
console.error(
|
||||
"[Hook] TypeScript errors in " + path.basename(filePath) + ":",
|
||||
);
|
||||
relevantLines.forEach((line) => console.error(line));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Invalid input — pass through
|
||||
}
|
||||
} catch {
|
||||
// Invalid input — pass through
|
||||
}
|
||||
|
||||
process.stdout.write(data, () => process.exit(0));
|
||||
});
|
||||
process.stdout.write(data, () => process.exit(0));
|
||||
});
|
||||
}
|
||||
|
||||
module.exports = { diagnosticMatchesFile };
|
||||
@@ -587,6 +587,31 @@ function runTests() {
|
||||
})
|
||||
);
|
||||
|
||||
results.push(
|
||||
test('requiring config-protection in-process does not attach stdin listeners or echo stdin', () => {
|
||||
return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-require-')), tmpDir => {
|
||||
const protectedPath = path.join(tmpDir, '.eslintrc.js');
|
||||
fs.writeFileSync(protectedPath, 'module.exports = {};');
|
||||
const configProtectionPath = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'config-protection.js');
|
||||
const script = [
|
||||
'const beforeData = process.stdin.listenerCount("data");',
|
||||
'const beforeEnd = process.stdin.listenerCount("end");',
|
||||
`const mod = require(${JSON.stringify(configProtectionPath)});`,
|
||||
'if (typeof mod.run !== "function") process.exit(3);',
|
||||
'if (process.stdin.listenerCount("data") !== beforeData || process.stdin.listenerCount("end") !== beforeEnd) process.exit(4);'
|
||||
].join('');
|
||||
const result = spawnSync(process.execPath, ['-e', script], {
|
||||
input: JSON.stringify({ tool_name: 'Write', tool_input: { file_path: protectedPath } }),
|
||||
encoding: 'utf8',
|
||||
timeout: 5000
|
||||
});
|
||||
assert.strictEqual(result.status, 0, `require() should not attach stdin listeners or run the CLI handler (exit ${result.status}): ${result.stderr}`);
|
||||
assert.strictEqual(result.stdout, '', 'require() must not echo stdin');
|
||||
assert.strictEqual(result.stderr, '', 'require() must not execute the stdin fallback block');
|
||||
});
|
||||
})
|
||||
);
|
||||
|
||||
const passed = results.filter(result => result === 'passed').length;
|
||||
const failed = results.filter(result => result === 'failed').length;
|
||||
const skipped = results.filter(result => result === 'skipped').length;
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
/**
|
||||
* Tests for scripts/hooks/post-edit-typecheck.js
|
||||
*
|
||||
* Run with: node tests/hooks/post-edit-typecheck.test.js
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
const { spawnSync } = require('child_process');
|
||||
|
||||
const { diagnosticMatchesFile } = require('../../scripts/hooks/post-edit-typecheck');
|
||||
|
||||
const SCRIPT = path.resolve(__dirname, '../../scripts/hooks/post-edit-typecheck.js');
|
||||
const SKIPPED = Symbol('skipped');
|
||||
|
||||
function test(name, fn) {
|
||||
try {
|
||||
if (fn() === SKIPPED) {
|
||||
console.log(` SKIP ${name}`);
|
||||
return 'skipped';
|
||||
}
|
||||
console.log(` PASS ${name}`);
|
||||
return 'passed';
|
||||
} catch (error) {
|
||||
console.log(` FAIL ${name}`);
|
||||
console.log(` Error: ${error && error.message ? error.message : error}`);
|
||||
return 'failed';
|
||||
}
|
||||
}
|
||||
|
||||
const results = [];
|
||||
|
||||
results.push(
|
||||
test('matches forward-slash tsc paths against Windows edited paths', () => {
|
||||
const candidates = new Set(['C:\\project\\src\\index.ts', 'src\\index.ts']);
|
||||
assert.strictEqual(diagnosticMatchesFile('src/index.ts(1,14): error TS2322: Type', candidates), true);
|
||||
assert.strictEqual(diagnosticMatchesFile('src/other.ts(1,14): error TS2322: Type', candidates), false);
|
||||
})
|
||||
);
|
||||
|
||||
results.push(
|
||||
test('matches a diagnostic by the file it names, not by a substring of the line', () => {
|
||||
const rootIndex = new Set(['/project/index.ts', 'index.ts']);
|
||||
assert.strictEqual(diagnosticMatchesFile('index.ts(1,14): error TS2322: Type', rootIndex), true);
|
||||
assert.strictEqual(diagnosticMatchesFile('src/index.ts(1,14): error TS2322: Type', rootIndex), false);
|
||||
assert.strictEqual(diagnosticMatchesFile('other-index.ts(1,14): error TS2322: Type', rootIndex), false);
|
||||
const utils = new Set(['/project/src/utils.ts', 'src/utils.ts']);
|
||||
assert.strictEqual(diagnosticMatchesFile("app.ts(3,1): error TS2307: Cannot find module './src/utils.ts'", utils), false);
|
||||
assert.strictEqual(diagnosticMatchesFile(" Type 'string' is not assignable to type 'number'.", utils), false);
|
||||
assert.strictEqual(diagnosticMatchesFile('src/utils.ts(1,14): error TS2345: call(1,2): bad', utils), true);
|
||||
assert.strictEqual(diagnosticMatchesFile('app/(auth)/[...slug]/page.tsx(3,5): error TS2322', new Set(['app/(auth)/[...slug]/page.tsx'])), true);
|
||||
})
|
||||
);
|
||||
|
||||
results.push(
|
||||
test('requiring the hook does not attach stdin listeners or echo stdin', () => {
|
||||
const script = [
|
||||
'const beforeData = process.stdin.listenerCount("data");',
|
||||
'const beforeEnd = process.stdin.listenerCount("end");',
|
||||
`const mod = require(${JSON.stringify(SCRIPT)});`,
|
||||
'if (typeof mod.diagnosticMatchesFile !== "function") process.exit(3);',
|
||||
'if (process.stdin.listenerCount("data") !== beforeData || process.stdin.listenerCount("end") !== beforeEnd) process.exit(4);'
|
||||
].join('');
|
||||
const result = spawnSync(process.execPath, ['-e', script], {
|
||||
input: JSON.stringify({ tool_input: { file_path: path.join(os.tmpdir(), 'missing.ts') } }),
|
||||
encoding: 'utf8',
|
||||
timeout: 5000
|
||||
});
|
||||
assert.strictEqual(result.status, 0, `require() should not attach stdin listeners (exit ${result.status}): ${result.stderr}`);
|
||||
assert.strictEqual(result.stdout, '', 'require() must not echo stdin');
|
||||
assert.strictEqual(result.stderr, '', 'require() must not run the stdin handler');
|
||||
})
|
||||
);
|
||||
|
||||
results.push(
|
||||
test('hook reports a tsc diagnostic only under the file it names', () => {
|
||||
// On Windows the hook runs npx.cmd through execFileSync without a shell,
|
||||
// and Node cannot launch a .cmd file that way, so a stubbed npx never
|
||||
// runs there. The matcher cases above cover Windows paths.
|
||||
if (process.platform === 'win32') return SKIPPED;
|
||||
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-post-edit-typecheck-'));
|
||||
try {
|
||||
const binDir = path.join(projectDir, 'bin');
|
||||
fs.mkdirSync(path.join(projectDir, 'src'));
|
||||
fs.mkdirSync(binDir);
|
||||
fs.writeFileSync(path.join(projectDir, 'tsconfig.json'), '{}');
|
||||
fs.writeFileSync(path.join(projectDir, 'index.ts'), 'export const ok = 1;\n');
|
||||
fs.writeFileSync(path.join(projectDir, 'src', 'index.ts'), 'export const bad: number = "x";\n');
|
||||
const diagnostic = "src/index.ts(1,14): error TS2322: Type 'string' is not assignable to type 'number'.";
|
||||
const fakeTsc = path.join(binDir, 'fake-tsc.js');
|
||||
fs.writeFileSync(fakeTsc, `process.stdout.write(${JSON.stringify(`${diagnostic}\n`)});\nprocess.exitCode = 2;\n`);
|
||||
fs.writeFileSync(path.join(binDir, 'npx'), `#!/usr/bin/env node\nrequire(${JSON.stringify(fakeTsc)});\n`);
|
||||
fs.chmodSync(path.join(binDir, 'npx'), 0o755);
|
||||
|
||||
const runHook = editedFile =>
|
||||
spawnSync(process.execPath, [SCRIPT], {
|
||||
input: JSON.stringify({ tool_input: { file_path: editedFile } }),
|
||||
env: { ...process.env, PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}` },
|
||||
encoding: 'utf8',
|
||||
timeout: 10000
|
||||
});
|
||||
|
||||
const rootResult = runHook(path.join(projectDir, 'index.ts'));
|
||||
assert.strictEqual(rootResult.status, 0, rootResult.stderr);
|
||||
assert.ok(!rootResult.stderr.includes('TypeScript errors'), `root index.ts must not get src/index.ts errors:\n${rootResult.stderr}`);
|
||||
|
||||
const nestedResult = runHook(path.join(projectDir, 'src', 'index.ts'));
|
||||
assert.strictEqual(nestedResult.status, 0, nestedResult.stderr);
|
||||
assert.ok(nestedResult.stderr.includes(`TypeScript errors in index.ts:\n${diagnostic}`), nestedResult.stderr);
|
||||
} finally {
|
||||
fs.rmSync(projectDir, { recursive: true, force: true });
|
||||
}
|
||||
return undefined;
|
||||
})
|
||||
);
|
||||
|
||||
const passed = results.filter(result => result === 'passed').length;
|
||||
const failed = results.filter(result => result === 'failed').length;
|
||||
const skipped = results.filter(result => result === 'skipped').length;
|
||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}, Skipped: ${skipped}`);
|
||||
process.exitCode = failed > 0 ? 1 : 0;
|
||||
Reference in new issue
Block a user