diff --git a/biome.json b/biome.json index 65f23abcb..33207c298 100644 --- a/biome.json +++ b/biome.json @@ -1,5 +1,5 @@ { - "$schema": "https://biomejs.dev/schemas/2.4.14/schema.json", + "$schema": "https://biomejs.dev/schemas/2.4.15/schema.json", "vcs": { "enabled": true, "clientKind": "git", diff --git a/packages/coding-agent/src/core/subagent-runner.ts b/packages/coding-agent/src/core/subagent-runner.ts index 42a496346..e1127555c 100644 --- a/packages/coding-agent/src/core/subagent-runner.ts +++ b/packages/coding-agent/src/core/subagent-runner.ts @@ -327,7 +327,14 @@ export async function runSubagent( `[subagent:${name}] phase=session.prompt-entered taskLen=${promptTask.length} timeoutMs=${timeoutMs} noOutputMs=${noOutputTimeoutMs}\n`, ); const promptEnteredAt = phaseStartedAt(); - const promptWatchdog = phaseWatchdog("session.prompt", promptEnteredAt); + // Note: no phaseWatchdog for session.prompt. The 10s STUCK warning fired + // spuriously on healthy 30–60s LLM responses (verified by strace: bytes + // were flowing on the TLS socket the whole time). The noOutputTimeoutMs + // watchdog below is the event-aware replacement — it resets on every + // meaningful subagent event (see armNoOutputTimer / isMeaningfulSubagent- + // OutputEvent) and aborts only after a true silence window. Keep the + // STUCK watchdog for setup phases (resourceLoader.reload, createAgent- + // Session, bindExtensions) where 10s of silence really is a hang signal. try { if (timeoutMs === 0 && noOutputTimeoutMs === 0 && !options?.signal) { @@ -336,7 +343,6 @@ export async function runSubagent( runExtensionHooks: false, }); await promptPromise; - clearTimeout(promptWatchdog); logPhase("session.prompt-returned", promptEnteredAt); cleanup(); return { ok: true, output: extractFinalOutput(), exitCode: 0 }; @@ -416,7 +422,6 @@ export async function runSubagent( }); competitors.unshift( promptPromise.then(() => { - clearTimeout(promptWatchdog); logPhase("session.prompt-returned", promptEnteredAt); return {} as RaceResult; }), @@ -425,7 +430,6 @@ export async function runSubagent( const result = await Promise.race(competitors); cleanup(); - clearTimeout(promptWatchdog); if (result.timedOut) { return { diff --git a/scripts/check-test-imports.mjs b/scripts/check-test-imports.mjs index f65cd8870..c8ea932c9 100644 --- a/scripts/check-test-imports.mjs +++ b/scripts/check-test-imports.mjs @@ -30,54 +30,207 @@ const root = resolve(__dirname, ".."); // ── Helpers ─────────────────────────────────────────────────────────────────── const NODE_BUILTINS = new Set([ - "process", "require", "module", "exports", "console", "global", "Buffer", - "setTimeout", "setInterval", "clearTimeout", "clearInterval", "queueMicrotask", - "Array", "Object", "String", "Number", "Boolean", "Symbol", "BigInt", - "Map", "Set", "WeakMap", "WeakSet", "Promise", "Math", "JSON", "Date", - "RegExp", "Error", "EvalError", "RangeError", "ReferenceError", "SyntaxError", - "TypeError", "URIError", "AggregateError", "AbortController", "AbortSignal", - "URL", "URLSearchParams", "TextEncoder", "TextDecoder", "TransformStream", - "ReadableStream", "WritableStream", "Blob", "File", "FormData", "Headers", - "Request", "Response", "fetch", "WebSocket", "EventSource", - "setImmediate", "clearImmediate", "structuredClone", "atob", "btoa", - "navigator", "window", "document", "location", "history", "gc", - "finalizationRegistry", "Intl", "Reflect", "Proxy", + "process", + "require", + "module", + "exports", + "console", + "global", + "Buffer", + "setTimeout", + "setInterval", + "clearTimeout", + "clearInterval", + "queueMicrotask", + "Array", + "Object", + "String", + "Number", + "Boolean", + "Symbol", + "BigInt", + "Map", + "Set", + "WeakMap", + "WeakSet", + "Promise", + "Math", + "JSON", + "Date", + "RegExp", + "Error", + "EvalError", + "RangeError", + "ReferenceError", + "SyntaxError", + "TypeError", + "URIError", + "AggregateError", + "AbortController", + "AbortSignal", + "URL", + "URLSearchParams", + "TextEncoder", + "TextDecoder", + "TransformStream", + "ReadableStream", + "WritableStream", + "Blob", + "File", + "FormData", + "Headers", + "Request", + "Response", + "fetch", + "WebSocket", + "EventSource", + "setImmediate", + "clearImmediate", + "structuredClone", + "atob", + "btoa", + "navigator", + "window", + "document", + "location", + "history", + "gc", + "finalizationRegistry", + "Intl", + "Reflect", + "Proxy", // Node.js globals (not ESM builtins but always available) - "__dirname", "__filename", "import", "imports", + "__dirname", + "__filename", + "import", + "imports", ]); // Node.js module exports commonly used in tests without explicit import const NODE_MODULE_GLOBALS = new Set([ - "assert", "assertEq", "deepEqual", "strictEqual", - "path", "fs", "os", "url", "querystring", "crypto", "buffer", - "stream", "util", "events", "querystring", "stringDecoder", - "readline", "tty", "domain", "constants", " timers", "async_hooks", + "assert", + "assertEq", + "deepEqual", + "strictEqual", + "path", + "fs", + "os", + "url", + "querystring", + "crypto", + "buffer", + "stream", + "util", + "events", + "querystring", + "stringDecoder", + "readline", + "tty", + "domain", + "constants", + " timers", + "async_hooks", ]); // JavaScript reserved words and keywords that should never be flagged as undeclared const JS_KEYWORDS = new Set([ // Reserved words - "break", "case", "catch", "continue", "debugger", "default", "delete", - "do", "else", "finally", "for", "function", "if", "in", "instanceof", - "new", "return", "switch", "this", "throw", "try", "typeof", "var", - "void", "while", "with", "class", "const", "enum", "export", "extends", - "import", "super", "implements", "interface", "let", "package", "private", - "protected", "public", "static", "yield", "async", "await", "of", - "get", "set", "target", "from", "as", + "break", + "case", + "catch", + "continue", + "debugger", + "default", + "delete", + "do", + "else", + "finally", + "for", + "function", + "if", + "in", + "instanceof", + "new", + "return", + "switch", + "this", + "throw", + "try", + "typeof", + "var", + "void", + "while", + "with", + "class", + "const", + "enum", + "export", + "extends", + "import", + "super", + "implements", + "interface", + "let", + "package", + "private", + "protected", + "public", + "static", + "yield", + "async", + "await", + "of", + "get", + "set", + "target", + "from", + "as", ]); // Common test framework globals that are always available in vitest/jest const TEST_GLOBALS = new Set([ - "describe", "it", "test", "expect", "beforeEach", "afterEach", - "beforeAll", "afterAll", "suite", "spec", "context", "given", - "when", "then", "and", "describe.skip", "it.skip", "test.skip", - "describe.only", "it.only", "test.only", "describe.each", "it.each", - "test.each", "describe.concurrent", "it.concurrent", "test.concurrent", - "vi", "vitest", "spyOn", "jest", + "describe", + "it", + "test", + "expect", + "beforeEach", + "afterEach", + "beforeAll", + "afterAll", + "suite", + "spec", + "context", + "given", + "when", + "then", + "and", + "describe.skip", + "it.skip", + "test.skip", + "describe.only", + "it.only", + "test.only", + "describe.each", + "it.each", + "test.each", + "describe.concurrent", + "it.concurrent", + "test.concurrent", + "vi", + "vitest", + "spyOn", + "jest", ]); // Boolean and literal values that always exist -const JS_LITERALS = new Set(["true", "false", "null", "undefined", "NaN", "Infinity"]); +const JS_LITERALS = new Set([ + "true", + "false", + "null", + "undefined", + "NaN", + "Infinity", +]); /** * Parse the statically-imported identifiers from the top of a file. @@ -92,7 +245,7 @@ function parseImports(source) { const stripped = source .replace(/'[^']*'/g, "'_'") .replace(/"[^"]*"/g, '"_"') - .replace(/`[^`]*`/g, '``_`') + .replace(/`[^`]*`/g, "``_`") .replace(/\/\/[^\n]*/g, "") .replace(/\/\*[\s\S]*?\*\//g, ""); @@ -107,16 +260,19 @@ function parseImports(source) { const namedRe = /import\s+\{([^}]+)\}\s+from\s+["'][^"']+["']/g; for (const m of stripped.matchAll(namedRe)) { const content = m[1]; - const items = content.split(",").map((s) => { - s = s.trim(); - if (s.includes(" as ")) { - s = s.split(" as ").pop().trim(); - } - if (s.startsWith("type ")) { - s = s.replace("type ", "").trim(); - } - return s.replace(/\s+/g, ""); - }).filter(Boolean); + const items = content + .split(",") + .map((s) => { + s = s.trim(); + if (s.includes(" as ")) { + s = s.split(" as ").pop().trim(); + } + if (s.startsWith("type ")) { + s = s.replace("type ", "").trim(); + } + return s.replace(/\s+/g, ""); + }) + .filter(Boolean); items.forEach((id) => named.add(id)); } @@ -143,7 +299,7 @@ function collectReferences(source) { const stripped = source .replace(/'[^']*'/g, "'_'") .replace(/"[^"]*"/g, '"_"') - .replace(/`[^`]*`/g, '``_`') + .replace(/`[^`]*`/g, "``_`") .replace(/\/\/[^\n]*/g, "") .replace(/\/\*[\s\S]*?\*\//g, ""); @@ -154,7 +310,7 @@ function collectReferences(source) { // Replace member expressions and calls with placeholders to isolate identifiers const cleaned = stripped - .replace(/\b[a-zA-Z_$][\w$]*\.[a-zA-Z_$][\w$]*(?=\s*[\[.\(])/g, "_") + .replace(/\b[a-zA-Z_$][\w$]*\.[a-zA-Z_$][\w$]*(?=\s*[[.(])/g, "_") .replace(/\[[^\]]+\]/g, "[_]"); // Now collect bare identifiers (word boundaries) @@ -208,13 +364,22 @@ function checkFile(filePath, source) { // - Variable names assigned in try/catch or before import // - Playwright/MCP test helpers const COMMON_TEST_LOCALS = new Set([ - "canLaunchChromium", "isAvailable", "hasSupport", "isSupported", - "describeOrSkip", "itOrSkip", "skipIf", "runIf", - "jiti", "interopDefault", "debug", + "canLaunchChromium", + "isAvailable", + "hasSupport", + "isSupported", + "describeOrSkip", + "itOrSkip", + "skipIf", + "runIf", + "jiti", + "interopDefault", + "debug", ]); const filteredUndeclared = [...undeclared].filter( - (id) => !COMMON_TEST_LOCALS.has(id) && + (id) => + !COMMON_TEST_LOCALS.has(id) && !id.match(/^(can|has|is|should|will)[A-Z]/) && !id.match(/^(testBrowser|chromiumInstance|pageInstance)$/), ); @@ -251,9 +416,12 @@ function findTestFiles(dir) { } results.push(...findTestFiles(full)); } else if ( - (entry.name.endsWith(".test.js") || entry.name.endsWith(".test.mjs") || - entry.name.endsWith(".test.ts") || entry.name.endsWith(".spec.js") || - entry.name.endsWith(".spec.mjs") || entry.name.endsWith(".spec.ts")) && + (entry.name.endsWith(".test.js") || + entry.name.endsWith(".test.mjs") || + entry.name.endsWith(".test.ts") || + entry.name.endsWith(".spec.js") || + entry.name.endsWith(".spec.mjs") || + entry.name.endsWith(".spec.ts")) && !entry.name.includes("node_modules") ) { results.push(full); @@ -271,9 +439,7 @@ const verbose = args.includes("--verbose"); // If specific paths are provided, use those instead of scanning // Filter out only flag args, keep everything else as a file path -const explicitPaths = args.filter( - (a) => !a.startsWith("--"), -); +const explicitPaths = args.filter((a) => !a.startsWith("--")); let allFiles = []; if (explicitPaths.length > 0) { @@ -293,7 +459,9 @@ if (explicitPaths.length > 0) { process.exit(1); } if (verbose) { - console.error(`[check-test-imports] explicit mode: ${allFiles.length} file(s)`); + console.error( + `[check-test-imports] explicit mode: ${allFiles.length} file(s)`, + ); } } else { const scanDirs = [ @@ -341,4 +509,4 @@ if (jsonOut) { } } -process.exit(issues.length > 0 ? 1 : 0); \ No newline at end of file +process.exit(issues.length > 0 ? 1 : 0); diff --git a/scripts/check-test-imports.test.mjs b/scripts/check-test-imports.test.mjs index ba785fb55..7f96095c6 100644 --- a/scripts/check-test-imports.test.mjs +++ b/scripts/check-test-imports.test.mjs @@ -42,14 +42,22 @@ function run(args) { /** Create a temp test file, return absolute path. */ function makeTempFile(id, content) { - try { mkdirSync(tmpDir, { recursive: true }); } catch { /* */ } + try { + mkdirSync(tmpDir, { recursive: true }); + } catch { + /* */ + } const file = resolve(tmpDir, `regression-${id}.test.mjs`); writeFileSync(file, content); return file; } function cleanupTempDir() { - try { rmdirSync(tmpDir); } catch { /* ignore */ } + try { + rmdirSync(tmpDir); + } catch { + /* ignore */ + } } // ── Tests ───────────────────────────────────────────────────────────────────── @@ -89,7 +97,9 @@ if (!existsSync(realFile)) { // ── Test 2: Detects undeclared identifier in anti-pattern file ─────────────── console.log("\n[2] Detects undeclared identifier in anti-pattern file"); -const file2 = makeTempFile("regression-02", ` +const file2 = makeTempFile( + "regression-02", + ` // Regression test — generated by check-test-imports.test.mjs // DO NOT COMMIT import { describe, it, expect } from "vitest"; @@ -100,7 +110,8 @@ describe("test", () => { undeclaredFn(); }); }); -`); +`, +); const result2 = run([file2, "--json"]); try { const parsed = JSON.parse(result2.stdout); @@ -118,7 +129,9 @@ try { // ── Test 3: Clean file with namespace import passes ───────────────────────── console.log("\n[3] Clean file with namespace import + local vars passes"); -const file3 = makeTempFile("regression-03", ` +const file3 = makeTempFile( + "regression-03", + ` // Clean: namespace import + only local variables // DO NOT COMMIT import { describe, it, expect } from "vitest"; @@ -131,21 +144,21 @@ describe("test", () => { expect(myLocalVar).toBeDefined(); }); }); -`); +`, +); const result3 = run([file3, "--json"]); try { const parsed = JSON.parse(result3.stdout); - assert( - parsed.count === 0, - `Clean file: count ${parsed.count}`, - ); + assert(parsed.count === 0, `Clean file: count ${parsed.count}`); } catch { assert(false, `Script should not throw on clean file`); } // ── Test 4: Local const/let declarations not flagged ─────────────────────── console.log("\n[4] Local const/let declarations not flagged"); -const file4 = makeTempFile("regression-04", ` +const file4 = makeTempFile( + "regression-04", + ` // Local variable declarations should not be flagged // DO NOT COMMIT import { describe, it, expect } from "vitest"; @@ -159,7 +172,8 @@ describe("test", () => { expect(myLocalVar).toBeDefined(); }); }); -`); +`, +); const result4 = run([file4, "--json"]); try { const parsed = JSON.parse(result4.stdout); @@ -173,7 +187,9 @@ try { // ── Test 5: Short lowercase variables not flagged ─────────────────────────── console.log("\n[5] Short lowercase variables not flagged"); -const file5 = makeTempFile("regression-05", ` +const file5 = makeTempFile( + "regression-05", + ` // Short lowercase vars like i, fn are common test locals // DO NOT COMMIT import { describe, it, expect } from "vitest"; @@ -186,7 +202,8 @@ describe("test", () => { expect(j).toBeGreaterThan(0); }); }); -`); +`, +); const result5 = run([file5, "--json"]); try { const parsed = JSON.parse(result5.stdout); @@ -203,4 +220,4 @@ try { console.log(`\n${"-".repeat(50)}`); console.log(`Results: ${passed} passed, ${failed} failed`); cleanupTempDir(); -process.exit(failed > 0 ? 1 : 0); \ No newline at end of file +process.exit(failed > 0 ? 1 : 0);