From 6ce482668b36fbf395ada8f7c5760ab95b2efd2f Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 13:30:00 +0900 Subject: [PATCH 1/2] refactor: extract git worktree parser from atlas hook --- src/hooks/atlas/index.ts | 113 +----------------- .../git-worktree/collect-git-diff-stats.ts | 29 +++++ .../git-worktree/format-file-changes.ts | 46 +++++++ src/shared/git-worktree/git-worktree.test.ts | 51 ++++++++ src/shared/git-worktree/index.ts | 5 + src/shared/git-worktree/parse-diff-numstat.ts | 27 +++++ .../git-worktree/parse-status-porcelain.ts | 25 ++++ src/shared/git-worktree/types.ts | 8 ++ src/shared/index.ts | 1 + 9 files changed, 195 insertions(+), 110 deletions(-) create mode 100644 src/shared/git-worktree/collect-git-diff-stats.ts create mode 100644 src/shared/git-worktree/format-file-changes.ts create mode 100644 src/shared/git-worktree/git-worktree.test.ts create mode 100644 src/shared/git-worktree/index.ts create mode 100644 src/shared/git-worktree/parse-diff-numstat.ts create mode 100644 src/shared/git-worktree/parse-status-porcelain.ts create mode 100644 src/shared/git-worktree/types.ts diff --git a/src/hooks/atlas/index.ts b/src/hooks/atlas/index.ts index ffad0459..b2608187 100644 --- a/src/hooks/atlas/index.ts +++ b/src/hooks/atlas/index.ts @@ -1,5 +1,4 @@ import type { PluginInput } from "@opencode-ai/plugin" -import { execSync } from "node:child_process" import { existsSync, readdirSync } from "node:fs" import { join } from "node:path" import { @@ -12,6 +11,7 @@ import { findNearestMessageWithFields, MESSAGE_STORAGE } from "../../features/ho import { log } from "../../shared/logger" import { createSystemDirective, SYSTEM_DIRECTIVE_PREFIX, SystemDirectiveTypes } from "../../shared/system-directive" import { isCallerOrchestrator, getMessageDir } from "../../shared/session-utils" +import { collectGitDiffStats, formatFileChanges } from "../../shared/git-worktree" import type { BackgroundManager } from "../../features/background-agent" export const HOOK_NAME = "atlas" @@ -269,113 +269,6 @@ function extractSessionIdFromOutput(output: string): string { return match?.[1] ?? "" } -interface GitFileStat { - path: string - added: number - removed: number - status: "modified" | "added" | "deleted" -} - -function getGitDiffStats(directory: string): GitFileStat[] { - try { - const output = execSync("git diff --numstat HEAD", { - cwd: directory, - encoding: "utf-8", - timeout: 5000, - stdio: ["pipe", "pipe", "pipe"], - }).trim() - - if (!output) return [] - - const statusOutput = execSync("git status --porcelain", { - cwd: directory, - encoding: "utf-8", - timeout: 5000, - stdio: ["pipe", "pipe", "pipe"], - }).trim() - - const statusMap = new Map() - for (const line of statusOutput.split("\n")) { - if (!line) continue - const status = line.substring(0, 2).trim() - const filePath = line.substring(3) - if (status === "A" || status === "??") { - statusMap.set(filePath, "added") - } else if (status === "D") { - statusMap.set(filePath, "deleted") - } else { - statusMap.set(filePath, "modified") - } - } - - const stats: GitFileStat[] = [] - for (const line of output.split("\n")) { - const parts = line.split("\t") - if (parts.length < 3) continue - - const [addedStr, removedStr, path] = parts - const added = addedStr === "-" ? 0 : parseInt(addedStr, 10) - const removed = removedStr === "-" ? 0 : parseInt(removedStr, 10) - - stats.push({ - path, - added, - removed, - status: statusMap.get(path) ?? "modified", - }) - } - - return stats - } catch { - return [] - } -} - -function formatFileChanges(stats: GitFileStat[], notepadPath?: string): string { - if (stats.length === 0) return "[FILE CHANGES SUMMARY]\nNo file changes detected.\n" - - const modified = stats.filter((s) => s.status === "modified") - const added = stats.filter((s) => s.status === "added") - const deleted = stats.filter((s) => s.status === "deleted") - - const lines: string[] = ["[FILE CHANGES SUMMARY]"] - - if (modified.length > 0) { - lines.push("Modified files:") - for (const f of modified) { - lines.push(` ${f.path} (+${f.added}, -${f.removed})`) - } - lines.push("") - } - - if (added.length > 0) { - lines.push("Created files:") - for (const f of added) { - lines.push(` ${f.path} (+${f.added})`) - } - lines.push("") - } - - if (deleted.length > 0) { - lines.push("Deleted files:") - for (const f of deleted) { - lines.push(` ${f.path} (-${f.removed})`) - } - lines.push("") - } - - if (notepadPath) { - const notepadStat = stats.find((s) => s.path.includes("notepad") || s.path.includes(".sisyphus")) - if (notepadStat) { - lines.push("[NOTEPAD UPDATED]") - lines.push(` ${notepadStat.path} (+${notepadStat.added})`) - lines.push("") - } - } - - return lines.join("\n") -} - interface ToolExecuteAfterInput { tool: string sessionID?: string @@ -750,8 +643,8 @@ export function createAtlasHook( } if (output.output && typeof output.output === "string") { - const gitStats = getGitDiffStats(ctx.directory) - const fileChanges = formatFileChanges(gitStats) + const gitStats = collectGitDiffStats(ctx.directory) + const fileChanges = formatFileChanges(gitStats) const subagentSessionId = extractSessionIdFromOutput(output.output) const boulderState = readBoulderState(ctx.directory) diff --git a/src/shared/git-worktree/collect-git-diff-stats.ts b/src/shared/git-worktree/collect-git-diff-stats.ts new file mode 100644 index 00000000..158a09d8 --- /dev/null +++ b/src/shared/git-worktree/collect-git-diff-stats.ts @@ -0,0 +1,29 @@ +import { execSync } from "node:child_process" +import { parseGitStatusPorcelain } from "./parse-status-porcelain" +import { parseGitDiffNumstat } from "./parse-diff-numstat" +import type { GitFileStat } from "./types" + +export function collectGitDiffStats(directory: string): GitFileStat[] { + try { + const diffOutput = execSync("git diff --numstat HEAD", { + cwd: directory, + encoding: "utf-8", + timeout: 5000, + stdio: ["pipe", "pipe", "pipe"], + }).trim() + + if (!diffOutput) return [] + + const statusOutput = execSync("git status --porcelain", { + cwd: directory, + encoding: "utf-8", + timeout: 5000, + stdio: ["pipe", "pipe", "pipe"], + }).trim() + + const statusMap = parseGitStatusPorcelain(statusOutput) + return parseGitDiffNumstat(diffOutput, statusMap) + } catch { + return [] + } +} diff --git a/src/shared/git-worktree/format-file-changes.ts b/src/shared/git-worktree/format-file-changes.ts new file mode 100644 index 00000000..5afb58b8 --- /dev/null +++ b/src/shared/git-worktree/format-file-changes.ts @@ -0,0 +1,46 @@ +import type { GitFileStat } from "./types" + +export function formatFileChanges(stats: GitFileStat[], notepadPath?: string): string { + if (stats.length === 0) return "[FILE CHANGES SUMMARY]\nNo file changes detected.\n" + + const modified = stats.filter((s) => s.status === "modified") + const added = stats.filter((s) => s.status === "added") + const deleted = stats.filter((s) => s.status === "deleted") + + const lines: string[] = ["[FILE CHANGES SUMMARY]"] + + if (modified.length > 0) { + lines.push("Modified files:") + for (const f of modified) { + lines.push(` ${f.path} (+${f.added}, -${f.removed})`) + } + lines.push("") + } + + if (added.length > 0) { + lines.push("Created files:") + for (const f of added) { + lines.push(` ${f.path} (+${f.added})`) + } + lines.push("") + } + + if (deleted.length > 0) { + lines.push("Deleted files:") + for (const f of deleted) { + lines.push(` ${f.path} (-${f.removed})`) + } + lines.push("") + } + + if (notepadPath) { + const notepadStat = stats.find((s) => s.path.includes("notepad") || s.path.includes(".sisyphus")) + if (notepadStat) { + lines.push("[NOTEPAD UPDATED]") + lines.push(` ${notepadStat.path} (+${notepadStat.added})`) + lines.push("") + } + } + + return lines.join("\n") +} diff --git a/src/shared/git-worktree/git-worktree.test.ts b/src/shared/git-worktree/git-worktree.test.ts new file mode 100644 index 00000000..27183018 --- /dev/null +++ b/src/shared/git-worktree/git-worktree.test.ts @@ -0,0 +1,51 @@ +/// + +import { describe, expect, test } from "bun:test" +import { formatFileChanges, parseGitDiffNumstat, parseGitStatusPorcelain } from "./index" + +describe("git-worktree", () => { + test("#given status porcelain output #when parsing #then maps paths to statuses", () => { + const porcelain = [ + " M src/a.ts", + "A src/b.ts", + "?? src/c.ts", + "D src/d.ts", + ].join("\n") + + const map = parseGitStatusPorcelain(porcelain) + expect(map.get("src/a.ts")).toBe("modified") + expect(map.get("src/b.ts")).toBe("added") + expect(map.get("src/c.ts")).toBe("added") + expect(map.get("src/d.ts")).toBe("deleted") + }) + + test("#given diff numstat and status map #when parsing #then returns typed stats", () => { + const porcelain = [" M src/a.ts", "A src/b.ts"].join("\n") + const statusMap = parseGitStatusPorcelain(porcelain) + + const numstat = ["1\t2\tsrc/a.ts", "3\t0\tsrc/b.ts", "-\t-\tbin.dat"].join("\n") + const stats = parseGitDiffNumstat(numstat, statusMap) + + expect(stats).toEqual([ + { path: "src/a.ts", added: 1, removed: 2, status: "modified" }, + { path: "src/b.ts", added: 3, removed: 0, status: "added" }, + { path: "bin.dat", added: 0, removed: 0, status: "modified" }, + ]) + }) + + test("#given git file stats #when formatting #then produces grouped summary", () => { + const summary = formatFileChanges([ + { path: "src/a.ts", added: 1, removed: 2, status: "modified" }, + { path: "src/b.ts", added: 3, removed: 0, status: "added" }, + { path: "src/c.ts", added: 0, removed: 4, status: "deleted" }, + ]) + + expect(summary).toContain("[FILE CHANGES SUMMARY]") + expect(summary).toContain("Modified files:") + expect(summary).toContain("Created files:") + expect(summary).toContain("Deleted files:") + expect(summary).toContain("src/a.ts") + expect(summary).toContain("src/b.ts") + expect(summary).toContain("src/c.ts") + }) +}) diff --git a/src/shared/git-worktree/index.ts b/src/shared/git-worktree/index.ts new file mode 100644 index 00000000..0bc363d0 --- /dev/null +++ b/src/shared/git-worktree/index.ts @@ -0,0 +1,5 @@ +export type { GitFileStatus, GitFileStat } from "./types" +export { parseGitStatusPorcelain } from "./parse-status-porcelain" +export { parseGitDiffNumstat } from "./parse-diff-numstat" +export { collectGitDiffStats } from "./collect-git-diff-stats" +export { formatFileChanges } from "./format-file-changes" diff --git a/src/shared/git-worktree/parse-diff-numstat.ts b/src/shared/git-worktree/parse-diff-numstat.ts new file mode 100644 index 00000000..3ea2b0f6 --- /dev/null +++ b/src/shared/git-worktree/parse-diff-numstat.ts @@ -0,0 +1,27 @@ +import type { GitFileStat, GitFileStatus } from "./types" + +export function parseGitDiffNumstat( + output: string, + statusMap: Map +): GitFileStat[] { + if (!output) return [] + + const stats: GitFileStat[] = [] + for (const line of output.split("\n")) { + const parts = line.split("\t") + if (parts.length < 3) continue + + const [addedStr, removedStr, path] = parts + const added = addedStr === "-" ? 0 : parseInt(addedStr, 10) + const removed = removedStr === "-" ? 0 : parseInt(removedStr, 10) + + stats.push({ + path, + added, + removed, + status: statusMap.get(path) ?? "modified", + }) + } + + return stats +} diff --git a/src/shared/git-worktree/parse-status-porcelain.ts b/src/shared/git-worktree/parse-status-porcelain.ts new file mode 100644 index 00000000..0623de5d --- /dev/null +++ b/src/shared/git-worktree/parse-status-porcelain.ts @@ -0,0 +1,25 @@ +import type { GitFileStatus } from "./types" + +export function parseGitStatusPorcelain(output: string): Map { + const map = new Map() + if (!output) return map + + for (const line of output.split("\n")) { + if (!line) continue + + const status = line.substring(0, 2).trim() + const filePath = line.substring(3) + + if (!filePath) continue + + if (status === "A" || status === "??") { + map.set(filePath, "added") + } else if (status === "D") { + map.set(filePath, "deleted") + } else { + map.set(filePath, "modified") + } + } + + return map +} diff --git a/src/shared/git-worktree/types.ts b/src/shared/git-worktree/types.ts new file mode 100644 index 00000000..eb423699 --- /dev/null +++ b/src/shared/git-worktree/types.ts @@ -0,0 +1,8 @@ +export type GitFileStatus = "modified" | "added" | "deleted" + +export interface GitFileStat { + path: string + added: number + removed: number + status: GitFileStatus +} diff --git a/src/shared/index.ts b/src/shared/index.ts index d42be5a7..4ea34697 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -41,5 +41,6 @@ export * from "./tmux" export * from "./model-suggestion-retry" export * from "./opencode-server-auth" export * from "./port-utils" +export * from "./git-worktree" export * from "./safe-create-hook" export * from "./truncate-description" From 4738379ad73d3ba98eed1d66abc54e00122d1259 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 14:34:11 +0900 Subject: [PATCH 2/2] fix(lsp): reset safety block on server restart to prevent permanent blocks (#1366) --- .../claude-code-mcp-loader/loader.test.ts | 11 ++ .../unstable-agent-babysitter/index.test.ts | 6 +- src/tools/lsp/client.test.ts | 114 +++++++++++++++++- src/tools/lsp/client.ts | 82 ++++++++++--- 4 files changed, 193 insertions(+), 20 deletions(-) diff --git a/src/features/claude-code-mcp-loader/loader.test.ts b/src/features/claude-code-mcp-loader/loader.test.ts index 8a1b9f6e..a7fffe7e 100644 --- a/src/features/claude-code-mcp-loader/loader.test.ts +++ b/src/features/claude-code-mcp-loader/loader.test.ts @@ -8,6 +8,17 @@ const TEST_DIR = join(tmpdir(), "mcp-loader-test-" + Date.now()) describe("getSystemMcpServerNames", () => { beforeEach(() => { mkdirSync(TEST_DIR, { recursive: true }) + + // Isolate tests from real user environment (e.g., ~/.claude.json). + // loader.ts reads user-level config via os.homedir() + getClaudeConfigDir(). + mock.module("os", () => ({ + homedir: () => TEST_DIR, + tmpdir, + })) + + mock.module("../../shared", () => ({ + getClaudeConfigDir: () => join(TEST_DIR, ".claude"), + })) }) afterEach(() => { diff --git a/src/hooks/unstable-agent-babysitter/index.test.ts b/src/hooks/unstable-agent-babysitter/index.test.ts index f9900e7d..355072b5 100644 --- a/src/hooks/unstable-agent-babysitter/index.test.ts +++ b/src/hooks/unstable-agent-babysitter/index.test.ts @@ -1,8 +1,9 @@ +import { afterEach, describe, expect, test } from "bun:test" import { _resetForTesting, setMainSession } from "../../features/claude-code-session-state" import type { BackgroundTask } from "../../features/background-agent" import { createUnstableAgentBabysitterHook } from "./index" -const projectDir = "/Users/yeongyu/local-workspaces/oh-my-opencode" +const projectDir = process.cwd() type BabysitterContext = Parameters[0] @@ -21,6 +22,9 @@ function createMockPluginInput(options: { prompt: async (input: unknown) => { promptCalls.push({ input }) }, + promptAsync: async (input: unknown) => { + promptCalls.push({ input }) + }, }, }, } diff --git a/src/tools/lsp/client.test.ts b/src/tools/lsp/client.test.ts index 3ca6ee3b..8c805d14 100644 --- a/src/tools/lsp/client.test.ts +++ b/src/tools/lsp/client.test.ts @@ -2,7 +2,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs" import { join } from "node:path" import { tmpdir } from "node:os" -import { describe, it, expect, spyOn, mock } from "bun:test" +import { describe, it, expect, spyOn, mock, beforeEach, afterEach } from "bun:test" mock.module("vscode-jsonrpc/node", () => ({ createMessageConnection: () => { @@ -12,10 +12,18 @@ mock.module("vscode-jsonrpc/node", () => ({ StreamMessageWriter: function StreamMessageWriter() {}, })) -import { LSPClient, validateCwd } from "./client" +import { LSPClient, lspManager, validateCwd } from "./client" import type { ResolvedServer } from "./types" describe("LSPClient", () => { + beforeEach(async () => { + await lspManager.stopAll() + }) + + afterEach(async () => { + await lspManager.stopAll() + }) + describe("openFile", () => { it("sends didChange when a previously opened file changes on disk", async () => { // #given @@ -61,6 +69,108 @@ describe("LSPClient", () => { }) }) + describe("LSPServerManager", () => { + it("recreates client after init failure instead of staying permanently blocked", async () => { + //#given + const dir = mkdtempSync(join(tmpdir(), "lsp-manager-test-")) + + const server: ResolvedServer = { + id: "typescript", + command: ["typescript-language-server", "--stdio"], + extensions: [".ts"], + priority: 0, + } + + const startSpy = spyOn(LSPClient.prototype, "start") + const initializeSpy = spyOn(LSPClient.prototype, "initialize") + const isAliveSpy = spyOn(LSPClient.prototype, "isAlive") + const stopSpy = spyOn(LSPClient.prototype, "stop") + + startSpy.mockImplementationOnce(async () => { + throw new Error("boom") + }) + startSpy.mockImplementation(async () => {}) + initializeSpy.mockImplementation(async () => {}) + isAliveSpy.mockImplementation(() => true) + stopSpy.mockImplementation(async () => {}) + + try { + //#when + await expect(lspManager.getClient(dir, server)).rejects.toThrow("boom") + + const client = await lspManager.getClient(dir, server) + + //#then + expect(client).toBeInstanceOf(LSPClient) + expect(startSpy).toHaveBeenCalledTimes(2) + expect(stopSpy).toHaveBeenCalled() + } finally { + startSpy.mockRestore() + initializeSpy.mockRestore() + isAliveSpy.mockRestore() + stopSpy.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }) + + it("resets stale initializing entry so a hung init does not permanently block future clients", async () => { + //#given + const dir = mkdtempSync(join(tmpdir(), "lsp-manager-stale-test-")) + + const server: ResolvedServer = { + id: "typescript", + command: ["typescript-language-server", "--stdio"], + extensions: [".ts"], + priority: 0, + } + + const dateNowSpy = spyOn(Date, "now") + + const startSpy = spyOn(LSPClient.prototype, "start") + const initializeSpy = spyOn(LSPClient.prototype, "initialize") + const isAliveSpy = spyOn(LSPClient.prototype, "isAlive") + const stopSpy = spyOn(LSPClient.prototype, "stop") + + // First client init hangs forever. + const never = new Promise(() => {}) + startSpy.mockImplementationOnce(async () => { + await never + }) + + // Second attempt should be allowed after stale reset. + startSpy.mockImplementationOnce(async () => {}) + startSpy.mockImplementation(async () => {}) + initializeSpy.mockImplementation(async () => {}) + isAliveSpy.mockImplementation(() => true) + stopSpy.mockImplementation(async () => {}) + + try { + //#when + dateNowSpy.mockReturnValueOnce(0) + lspManager.warmupClient(dir, server) + + dateNowSpy.mockReturnValueOnce(60_000) + + const client = await Promise.race([ + lspManager.getClient(dir, server), + new Promise((_, reject) => setTimeout(() => reject(new Error("test-timeout")), 50)), + ]) + + //#then + expect(client).toBeInstanceOf(LSPClient) + expect(startSpy).toHaveBeenCalledTimes(2) + expect(stopSpy).toHaveBeenCalled() + } finally { + dateNowSpy.mockRestore() + startSpy.mockRestore() + initializeSpy.mockRestore() + isAliveSpy.mockRestore() + stopSpy.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }) + }) + describe("validateCwd", () => { it("returns valid for existing directory", () => { // #given diff --git a/src/tools/lsp/client.ts b/src/tools/lsp/client.ts index 57f9a5c8..ae4fb9ab 100644 --- a/src/tools/lsp/client.ts +++ b/src/tools/lsp/client.ts @@ -221,6 +221,7 @@ interface ManagedClient { refCount: number initPromise?: Promise isInitializing: boolean + initializingSince?: number } class LSPServerManager { @@ -228,6 +229,7 @@ class LSPServerManager { private clients = new Map() private cleanupInterval: ReturnType | null = null private readonly IDLE_TIMEOUT = 5 * 60 * 1000 + private readonly INIT_TIMEOUT = 60 * 1000 private constructor() { this.startCleanupTimer() @@ -309,17 +311,43 @@ class LSPServerManager { const key = this.getKey(root, server.id) let managed = this.clients.get(key) + if (managed) { + const now = Date.now() + if (managed.isInitializing && managed.initializingSince !== undefined && now - managed.initializingSince >= this.INIT_TIMEOUT) { + // Stale init can permanently block subsequent calls (e.g., LSP process hang) + try { + await managed.client.stop() + } catch {} + this.clients.delete(key) + managed = undefined + } + } + if (managed) { if (managed.initPromise) { - await managed.initPromise + try { + await managed.initPromise + } catch { + // Failed init should not keep the key blocked forever. + try { + await managed.client.stop() + } catch {} + this.clients.delete(key) + managed = undefined + } } - if (managed.client.isAlive()) { - managed.refCount++ - managed.lastUsedAt = Date.now() - return managed.client + + if (managed) { + if (managed.client.isAlive()) { + managed.refCount++ + managed.lastUsedAt = Date.now() + return managed.client + } + try { + await managed.client.stop() + } catch {} + this.clients.delete(key) } - await managed.client.stop() - this.clients.delete(key) } const client = new LSPClient(root, server) @@ -328,19 +356,30 @@ class LSPServerManager { await client.initialize() })() + const initStartedAt = Date.now() this.clients.set(key, { client, - lastUsedAt: Date.now(), + lastUsedAt: initStartedAt, refCount: 1, initPromise, isInitializing: true, + initializingSince: initStartedAt, }) - await initPromise + try { + await initPromise + } catch (error) { + this.clients.delete(key) + try { + await client.stop() + } catch {} + throw error + } const m = this.clients.get(key) if (m) { m.initPromise = undefined m.isInitializing = false + m.initializingSince = undefined } return client @@ -356,21 +395,30 @@ class LSPServerManager { await client.initialize() })() + const initStartedAt = Date.now() this.clients.set(key, { client, - lastUsedAt: Date.now(), + lastUsedAt: initStartedAt, refCount: 0, initPromise, isInitializing: true, + initializingSince: initStartedAt, }) - initPromise.then(() => { - const m = this.clients.get(key) - if (m) { - m.initPromise = undefined - m.isInitializing = false - } - }) + initPromise + .then(() => { + const m = this.clients.get(key) + if (m) { + m.initPromise = undefined + m.isInitializing = false + m.initializingSince = undefined + } + }) + .catch(() => { + // Warmup failures must not permanently block future initialization. + this.clients.delete(key) + void client.stop().catch(() => {}) + }) } releaseClient(root: string, serverId: string): void {