From c7122b412737bb1cfeeb435ff6dd2209d871bc07 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 15:31:32 +0900 Subject: [PATCH] fix: resolve all test failures and Cubic review issues - Fix unstable-agent-babysitter: add promptAsync to test mock - Fix claude-code-mcp-loader: isolate tests from user home configs - Fix npm-dist-tags: encode packageName for scoped packages - Fix agent-builder: clone source to prevent shared object mutation - Fix add-plugin-to-opencode-config: handle JSONC with leading comments - Fix auth-plugins/add-provider-config: error on parse failures - Fix bun-install: clear timeout on completion - Fix git-diff-stats: include untracked files in diff summary --- src/agents/agent-builder.ts | 2 +- .../add-plugin-to-opencode-config.ts | 2 +- src/cli/config-manager/add-provider-config.ts | 9 ++- src/cli/config-manager/auth-plugins.ts | 9 ++- src/cli/config-manager/bun-install.ts | 8 ++- src/cli/config-manager/npm-dist-tags.ts | 2 +- .../claude-code-mcp-loader/loader.test.ts | 58 ++++++---------- src/hooks/atlas/git-diff-stats.ts | 69 +++++++++++++------ .../unstable-agent-babysitter/index.test.ts | 3 + 9 files changed, 91 insertions(+), 71 deletions(-) diff --git a/src/agents/agent-builder.ts b/src/agents/agent-builder.ts index 459f18e0..63cf6962 100644 --- a/src/agents/agent-builder.ts +++ b/src/agents/agent-builder.ts @@ -19,7 +19,7 @@ export function buildAgent( browserProvider?: BrowserAutomationProvider, disabledSkills?: Set ): AgentConfig { - const base = isFactory(source) ? source(model) : source + const base = isFactory(source) ? source(model) : { ...source } const categoryConfigs: Record = categories ? { ...DEFAULT_CATEGORIES, ...categories } : DEFAULT_CATEGORIES diff --git a/src/cli/config-manager/add-plugin-to-opencode-config.ts b/src/cli/config-manager/add-plugin-to-opencode-config.ts index 0262cc53..1e8eb872 100644 --- a/src/cli/config-manager/add-plugin-to-opencode-config.ts +++ b/src/cli/config-manager/add-plugin-to-opencode-config.ts @@ -64,7 +64,7 @@ export async function addPluginToOpenCodeConfig(currentVersion: string): Promise const newContent = content.replace(pluginArrayRegex, `"plugin": [\n ${formattedPlugins}\n ]`) writeFileSync(path, newContent) } else { - const newContent = content.replace(/^(\s*\{)/, `$1\n "plugin": ["${pluginEntry}"],`) + const newContent = content.replace(/(\{)/, `$1\n "plugin": ["${pluginEntry}"],`) writeFileSync(path, newContent) } } else { diff --git a/src/cli/config-manager/add-provider-config.ts b/src/cli/config-manager/add-provider-config.ts index bc25c7a7..833c9882 100644 --- a/src/cli/config-manager/add-provider-config.ts +++ b/src/cli/config-manager/add-provider-config.ts @@ -25,10 +25,13 @@ export function addProviderConfig(config: InstallConfig): ConfigMergeResult { if (format !== "none") { const parseResult = parseOpenCodeConfigFileWithError(path) if (parseResult.error && !parseResult.config) { - existingConfig = {} - } else { - existingConfig = parseResult.config + return { + success: false, + configPath: path, + error: `Failed to parse config file: ${parseResult.error}`, + } } + existingConfig = parseResult.config } const newConfig = { ...(existingConfig ?? {}) } diff --git a/src/cli/config-manager/auth-plugins.ts b/src/cli/config-manager/auth-plugins.ts index 77a38369..7bbc8b81 100644 --- a/src/cli/config-manager/auth-plugins.ts +++ b/src/cli/config-manager/auth-plugins.ts @@ -35,10 +35,13 @@ export async function addAuthPlugins(config: InstallConfig): Promise { stderr: "pipe", }) - const timeoutPromise = new Promise<"timeout">((resolve) => - setTimeout(() => resolve("timeout"), BUN_INSTALL_TIMEOUT_MS) - ) + let timeoutId: ReturnType + const timeoutPromise = new Promise<"timeout">((resolve) => { + timeoutId = setTimeout(() => resolve("timeout"), BUN_INSTALL_TIMEOUT_MS) + }) const exitPromise = proc.exited.then(() => "completed" as const) const result = await Promise.race([exitPromise, timeoutPromise]) + clearTimeout(timeoutId!) if (result === "timeout") { try { diff --git a/src/cli/config-manager/npm-dist-tags.ts b/src/cli/config-manager/npm-dist-tags.ts index f653fc2f..67d8ca0e 100644 --- a/src/cli/config-manager/npm-dist-tags.ts +++ b/src/cli/config-manager/npm-dist-tags.ts @@ -9,7 +9,7 @@ const NPM_FETCH_TIMEOUT_MS = 5000 export async function fetchNpmDistTags(packageName: string): Promise { try { - const res = await fetch(`https://registry.npmjs.org/-/package/${packageName}/dist-tags`, { + const res = await fetch(`https://registry.npmjs.org/-/package/${encodeURIComponent(packageName)}/dist-tags`, { signal: AbortSignal.timeout(NPM_FETCH_TIMEOUT_MS), }) if (!res.ok) return null diff --git a/src/features/claude-code-mcp-loader/loader.test.ts b/src/features/claude-code-mcp-loader/loader.test.ts index 8a1b9f6e..33c5a0ae 100644 --- a/src/features/claude-code-mcp-loader/loader.test.ts +++ b/src/features/claude-code-mcp-loader/loader.test.ts @@ -4,10 +4,19 @@ import { join } from "path" import { tmpdir } from "os" const TEST_DIR = join(tmpdir(), "mcp-loader-test-" + Date.now()) +const TEST_HOME = join(TEST_DIR, "home") describe("getSystemMcpServerNames", () => { beforeEach(() => { mkdirSync(TEST_DIR, { recursive: true }) + mkdirSync(TEST_HOME, { recursive: true }) + mock.module("os", () => ({ + homedir: () => TEST_HOME, + tmpdir, + })) + mock.module("../../shared", () => ({ + getClaudeConfigDir: () => join(TEST_HOME, ".claude"), + })) }) afterEach(() => { @@ -162,7 +171,7 @@ describe("getSystemMcpServerNames", () => { it("reads user-level MCP config from ~/.claude.json", async () => { // given - const userConfigPath = join(TEST_DIR, ".claude.json") + const userConfigPath = join(TEST_HOME, ".claude.json") const userMcpConfig = { mcpServers: { "user-server": { @@ -171,53 +180,37 @@ describe("getSystemMcpServerNames", () => { }, }, } + writeFileSync(userConfigPath, JSON.stringify(userMcpConfig)) const originalCwd = process.cwd() process.chdir(TEST_DIR) try { - mock.module("os", () => ({ - homedir: () => TEST_DIR, - tmpdir, - })) - - writeFileSync(userConfigPath, JSON.stringify(userMcpConfig)) - + // when const { getSystemMcpServerNames } = await import("./loader") const names = getSystemMcpServerNames() + // then expect(names.has("user-server")).toBe(true) } finally { process.chdir(originalCwd) - rmSync(userConfigPath, { force: true }) } }) it("reads both ~/.claude.json and ~/.claude/.mcp.json for user scope", async () => { - // given: simulate both user-level config files - const userClaudeJson = join(TEST_DIR, ".claude.json") - const claudeDir = join(TEST_DIR, ".claude") - const claudeDirMcpJson = join(claudeDir, ".mcp.json") - + // given + const claudeDir = join(TEST_HOME, ".claude") mkdirSync(claudeDir, { recursive: true }) - // ~/.claude.json has server-a - writeFileSync(userClaudeJson, JSON.stringify({ + writeFileSync(join(TEST_HOME, ".claude.json"), JSON.stringify({ mcpServers: { - "server-from-claude-json": { - command: "npx", - args: ["server-a"], - }, + "server-from-claude-json": { command: "npx", args: ["server-a"] }, }, })) - // ~/.claude/.mcp.json has server-b (CLI-managed) - writeFileSync(claudeDirMcpJson, JSON.stringify({ + writeFileSync(join(claudeDir, ".mcp.json"), JSON.stringify({ mcpServers: { - "server-from-mcp-json": { - command: "npx", - args: ["server-b"], - }, + "server-from-mcp-json": { command: "npx", args: ["server-b"] }, }, })) @@ -225,20 +218,11 @@ describe("getSystemMcpServerNames", () => { process.chdir(TEST_DIR) try { - mock.module("os", () => ({ - homedir: () => TEST_DIR, - tmpdir, - })) - - // Also mock getClaudeConfigDir to point to our test .claude dir - mock.module("../../shared", () => ({ - getClaudeConfigDir: () => claudeDir, - })) - + // when const { getSystemMcpServerNames } = await import("./loader") const names = getSystemMcpServerNames() - // Both sources should be merged + // then expect(names.has("server-from-claude-json")).toBe(true) expect(names.has("server-from-mcp-json")).toBe(true) } finally { diff --git a/src/hooks/atlas/git-diff-stats.ts b/src/hooks/atlas/git-diff-stats.ts index 40494293..d154c50a 100644 --- a/src/hooks/atlas/git-diff-stats.ts +++ b/src/hooks/atlas/git-diff-stats.ts @@ -9,15 +9,6 @@ interface GitFileStat { export 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", @@ -25,12 +16,18 @@ export function getGitDiffStats(directory: string): GitFileStat[] { stdio: ["pipe", "pipe", "pipe"], }).trim() + if (!statusOutput) return [] + const statusMap = new Map() + const untrackedFiles: string[] = [] 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 === "??") { + if (status === "??") { + statusMap.set(filePath, "added") + untrackedFiles.push(filePath) + } else if (status === "A") { statusMap.set(filePath, "added") } else if (status === "D") { statusMap.set(filePath, "deleted") @@ -39,21 +36,49 @@ export function getGitDiffStats(directory: string): GitFileStat[] { } } + const output = execSync("git diff --numstat HEAD", { + cwd: directory, + encoding: "utf-8", + timeout: 5000, + stdio: ["pipe", "pipe", "pipe"], + }).trim() + const stats: GitFileStat[] = [] - for (const line of output.split("\n")) { - const parts = line.split("\t") - if (parts.length < 3) continue + const trackedPaths = new Set() - const [addedStr, removedStr, path] = parts - const added = addedStr === "-" ? 0 : parseInt(addedStr, 10) - const removed = removedStr === "-" ? 0 : parseInt(removedStr, 10) + if (output) { + for (const line of output.split("\n")) { + const parts = line.split("\t") + if (parts.length < 3) continue - stats.push({ - path, - added, - removed, - status: statusMap.get(path) ?? "modified", - }) + const [addedStr, removedStr, path] = parts + const added = addedStr === "-" ? 0 : parseInt(addedStr, 10) + const removed = removedStr === "-" ? 0 : parseInt(removedStr, 10) + trackedPaths.add(path) + + stats.push({ + path, + added, + removed, + status: statusMap.get(path) ?? "modified", + }) + } + } + + for (const filePath of untrackedFiles) { + if (trackedPaths.has(filePath)) continue + try { + const content = execSync(`wc -l < "${filePath}"`, { + cwd: directory, + encoding: "utf-8", + timeout: 3000, + stdio: ["pipe", "pipe", "pipe"], + }).trim() + const lineCount = parseInt(content, 10) || 0 + stats.push({ path: filePath, added: lineCount, removed: 0, status: "added" }) + } catch { + stats.push({ path: filePath, added: 0, removed: 0, status: "added" }) + } } return stats diff --git a/src/hooks/unstable-agent-babysitter/index.test.ts b/src/hooks/unstable-agent-babysitter/index.test.ts index f9900e7d..9fc309ec 100644 --- a/src/hooks/unstable-agent-babysitter/index.test.ts +++ b/src/hooks/unstable-agent-babysitter/index.test.ts @@ -21,6 +21,9 @@ function createMockPluginInput(options: { prompt: async (input: unknown) => { promptCalls.push({ input }) }, + promptAsync: async (input: unknown) => { + promptCalls.push({ input }) + }, }, }, }