diff --git a/src/tools/delegate-task/subagent-resolver.ts b/src/tools/delegate-task/subagent-resolver.ts index 0baedf552..88117ca0f 100644 --- a/src/tools/delegate-task/subagent-resolver.ts +++ b/src/tools/delegate-task/subagent-resolver.ts @@ -141,8 +141,6 @@ Create the work plan directly - that's your job as the planning agent.`, categoryModel = variantToUse ? { ...normalized, variant: variantToUse } : normalized } } else if (resolutionSkipped && (agentOverride?.model ?? agentCategoryModel)) { - // Cold cache: resolution was skipped but user explicitly configured a model. - // Honor the user override directly — don't fall through to hardcoded fallback chain. const normalized = normalizeModelFormat((agentOverride?.model ?? agentCategoryModel)!) if (normalized) { const agentCategoryVariant = agentOverride?.category @@ -164,8 +162,6 @@ Create the work plan directly - that's your job as the planning agent.`, normalizedAgentFallbackModels, defaultProviderID, ) - // Don't assign hardcoded fallback chain when resolution was skipped (cold cache) - // — the chain may contain model IDs that don't exist in the provider yet. fallbackChain = configuredFallbackChain ?? (resolutionSkipped ? undefined : agentRequirement?.fallbackChain) // Only promote fallback-only settings when resolution actually selected a fallback model. diff --git a/src/tools/hashline-edit/edit-operations.test.ts b/src/tools/hashline-edit/edit-operations.test.ts index 40585210f..9be6dabae 100644 --- a/src/tools/hashline-edit/edit-operations.test.ts +++ b/src/tools/hashline-edit/edit-operations.test.ts @@ -227,10 +227,10 @@ describe("hashline edit operations", () => { }) it("preserves blank lines and indentation in range replace (no false unwrap)", () => { - //#given — reproduces the 애국가 bug where blank+indented lines collapse + //#given, reproduces the 애국가 bug where blank+indented lines collapse const lines = ["", "동해물과 백두산이 마르고 닳도록", "하느님이 보우하사 우리나라 만세", "", "무궁화 삼천리 화려강산", "대한사람 대한으로 길이 보전하세", ""] - //#when — replace the range with indented version (blank lines preserved) + //#when, replace the range with indented version (blank lines preserved) const result = applyReplaceLines( lines, anchorFor(lines, 1), @@ -238,7 +238,7 @@ describe("hashline edit operations", () => { ["", " 동해물과 백두산이 마르고 닳도록", " 하느님이 보우하사 우리나라 만세", "", " 무궁화 삼천리 화려강산", " 대한사람 대한으로 길이 보전하세", ""] ) - //#then — all 7 lines preserved with indentation, not collapsed to 3 + //#then, all 7 lines preserved with indentation, not collapsed to 3 expect(result).toEqual(["", " 동해물과 백두산이 마르고 닳도록", " 하느님이 보우하사 우리나라 만세", "", " 무궁화 삼천리 화려강산", " 대한사람 대한으로 길이 보전하세", ""]) }) diff --git a/src/tools/hashline-edit/formatter-trigger.test.ts b/src/tools/hashline-edit/formatter-trigger.test.ts index c631ae079..050c2b2f6 100644 --- a/src/tools/hashline-edit/formatter-trigger.test.ts +++ b/src/tools/hashline-edit/formatter-trigger.test.ts @@ -350,10 +350,10 @@ describe("runFormattersForFile", () => { }, }) - //#when — run for a .go file, but only .ts formatters registered + //#when, run for a .go file, but only .ts formatters registered await runFormattersForFile(client, "/project", "/src/main.go") - //#then — no error thrown + //#then, no error thrown }) it("runs formatter for matching extension", async () => { @@ -367,10 +367,10 @@ describe("runFormattersForFile", () => { }, }) - //#when — echo is a safe no-op command + //#when, echo is a safe no-op command await runFormattersForFile(client, "/tmp", "/tmp/test.ts") - //#then — should complete without error + //#then, should complete without error expect(client.config.get).toHaveBeenCalledTimes(1) }) }) diff --git a/src/tools/hashline-edit/validation.test.ts b/src/tools/hashline-edit/validation.test.ts index 739def9fa..c3c531bef 100644 --- a/src/tools/hashline-edit/validation.test.ts +++ b/src/tools/hashline-edit/validation.test.ts @@ -23,10 +23,10 @@ describe("parseLineRef", () => { }) it("gives specific hint when literal text is used instead of line number", () => { - //#given — model sends "LINE#HK" instead of "1#HK" + //#given, model sends "LINE#HK" instead of "1#HK" const ref = "LINE#HK" - //#when / #then — error should mention that LINE is not a valid number + //#when / #then, error should mention that LINE is not a valid number expect(() => parseLineRef(ref)).toThrow(/not a line number/i) }) @@ -39,10 +39,10 @@ describe("parseLineRef", () => { }) it("extracts valid line number from mixed prefix like LINE42 without throwing", () => { - //#given — normalizeLineRef extracts 42#VK from LINE42#VK + //#given, normalizeLineRef extracts 42#VK from LINE42#VK const ref = "LINE42#VK" - //#when / #then — should parse successfully as line 42 + //#when / #then, should parse successfully as line 42 const result = parseLineRef(ref) expect(result.line).toBe(42) expect(result.hash).toBe("VK") @@ -144,11 +144,11 @@ describe("validateLineRef", () => { }) it("suggests correct line number when hash matches a file line", () => { - //#given — model sends LINE#XX where XX is the actual hash for line 1 + //#given, model sends LINE#XX where XX is the actual hash for line 1 const lines = ["function hello() {", " return 42", "}"] const hash = computeLineHash(1, lines[0]) - //#when / #then — error should suggest the correct reference + //#when / #then, error should suggest the correct reference expect(() => validateLineRefs(lines, [`LINE#${hash}`])).toThrow(new RegExp(`1#${hash}`)) }) }) diff --git a/src/tools/hashline-edit/validation.ts b/src/tools/hashline-edit/validation.ts index aa9166c16..f09b8fb8b 100644 --- a/src/tools/hashline-edit/validation.ts +++ b/src/tools/hashline-edit/validation.ts @@ -48,7 +48,6 @@ export function parseLineRef(ref: string): LineRef { hash: match[2], } } - // normalized equals ref.trim() in all error paths — extraction only succeeds for valid refs const hashIdx = normalized.indexOf('#') if (hashIdx > 0) { const prefix = normalized.slice(0, hashIdx) diff --git a/src/tools/lsp/config.test.ts b/src/tools/lsp/config.test.ts index 85de82c37..59459cde8 100644 --- a/src/tools/lsp/config.test.ts +++ b/src/tools/lsp/config.test.ts @@ -20,8 +20,7 @@ describe("isServerInstalled", () => { afterEach(() => { try { rmSync(tempDir, { recursive: true, force: true }) - } catch (e) { - // cleanup failed — ignored + } catch { } if (process.platform === "win32") { diff --git a/src/tools/lsp/lsp-manager-process-cleanup.ts b/src/tools/lsp/lsp-manager-process-cleanup.ts index df9f299e9..4bf6b14f7 100644 --- a/src/tools/lsp/lsp-manager-process-cleanup.ts +++ b/src/tools/lsp/lsp-manager-process-cleanup.ts @@ -1,3 +1,5 @@ +import { log } from "../../shared/logger" + type ManagedClientForCleanup = { client: { stop: () => Promise; @@ -22,23 +24,32 @@ export type LspProcessCleanupHandle = { export function registerLspManagerProcessCleanup(options: ProcessCleanupOptions): LspProcessCleanupHandle { const handlers: RegisteredHandler[] = []; - // Synchronous cleanup for 'exit' event (cannot await) + const logCleanupError = (phase: string, error: unknown): void => { + log(`[lsp-manager-process-cleanup] ${phase}`, { + error: error instanceof Error ? error.message : String(error), + }); + }; + const syncCleanup = () => { for (const [, managed] of options.getClients()) { try { - // Fire-and-forget during sync exit - process is terminating - void managed.client.stop().catch(() => {}); - } catch {} + void managed.client.stop().catch((error) => { + logCleanupError("stop failed during exit cleanup", error); + }); + } catch (error) { + logCleanupError("failed to schedule exit cleanup", error); + } } options.clearClients(); options.clearCleanupInterval(); }; - // Async cleanup for signal handlers - properly await all stops const asyncCleanup = async () => { const stopPromises: Promise[] = []; for (const [, managed] of options.getClients()) { - stopPromises.push(managed.client.stop().catch(() => {})); + stopPromises.push(managed.client.stop().catch((error) => { + logCleanupError("stop failed during signal cleanup", error); + })); } await Promise.allSettled(stopPromises); options.clearClients(); @@ -52,8 +63,9 @@ export function registerLspManagerProcessCleanup(options: ProcessCleanupOptions) registerHandler("exit", syncCleanup); - // Don't call process.exit() here; other handlers (background-agent manager) handle final exit. - const signalCleanup = () => void asyncCleanup().catch(() => {}); + const signalCleanup = () => void asyncCleanup().catch((error) => { + logCleanupError("signal cleanup failed", error); + }); registerHandler("SIGINT", signalCleanup); registerHandler("SIGTERM", signalCleanup); if (process.platform === "win32") { diff --git a/src/tools/lsp/lsp-process.ts b/src/tools/lsp/lsp-process.ts index 358c5c7e5..3f7b769a2 100644 --- a/src/tools/lsp/lsp-process.ts +++ b/src/tools/lsp/lsp-process.ts @@ -2,11 +2,9 @@ import { spawn as bunSpawn } from "bun" import { spawn as nodeSpawn, type ChildProcess } from "node:child_process" import { existsSync, statSync } from "fs" import { log } from "../../shared/logger" -// Bun spawn segfaults on Windows (oven-sh/bun#25798) — unfixed as of v1.3.8+ function shouldUseNodeSpawn(): boolean { return process.platform === "win32" } -// Prevents segfaults when libuv gets a non-existent cwd (oven-sh/bun#25798) export function validateCwd(cwd: string): { valid: boolean; error?: string } { try { if (!existsSync(cwd)) { @@ -24,7 +22,6 @@ export function validateCwd(cwd: string): { valid: boolean; error?: string } { interface StreamReader { read(): Promise<{ done: boolean; value: Uint8Array | undefined }> } -// Bridges Bun Subprocess and Node.js ChildProcess under a common API export interface UnifiedProcess { stdin: { write(chunk: Uint8Array | string): void } stdout: { getReader(): StreamReader } diff --git a/src/tools/skill/tools.test.ts b/src/tools/skill/tools.test.ts index 5007857ff..bf7ebf451 100644 --- a/src/tools/skill/tools.test.ts +++ b/src/tools/skill/tools.test.ts @@ -732,14 +732,14 @@ describe("skill tool - short name resolution", () => { ] const tool = createSkillTool({ skills: loadedSkills }) - // when / then — should not resolve (ambiguous), should suggest both + // when / then, should not resolve (ambiguous), should suggest both await expect(tool.execute({ name: "debugging" }, mockContext)).rejects.toThrow( "not found" ) }) it("prefers exact match over short name match", async () => { - // given — "debugging" exists as both exact and as part of a namespace + // given, "debugging" exists as both exact and as part of a namespace const loadedSkills = [ createMockSkill("debugging"), createMockSkill("superpowers/debugging"), @@ -749,7 +749,7 @@ describe("skill tool - short name resolution", () => { // when const result = await tool.execute({ name: "debugging" }, mockContext) - // then — should match "debugging" exactly, not "superpowers/debugging" + // then, should match "debugging" exactly, not "superpowers/debugging" expect(result).toContain("## Skill: debugging") }) }) diff --git a/src/tools/task/todo-sync.test.ts b/src/tools/task/todo-sync.test.ts index d6c87c3df..bf83b732b 100644 --- a/src/tools/task/todo-sync.test.ts +++ b/src/tools/task/todo-sync.test.ts @@ -535,7 +535,7 @@ describe("syncAllTasksToTodos", () => { // when await syncAllTasksToTodos(mockCtx, tasks, "session-1", writer); - // then — no duplicates + // then, no duplicates const matching = writtenTodos.filter((t: TodoInfo) => t.content === "Task 1 (updated)"); expect(matching.length).toBe(1); expect(matching[0].status).toBe("in_progress");