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
This commit is contained in:
YeonGyu-Kim
2026-02-08 15:31:32 +09:00
parent 119e18c810
commit c7122b4127
9 changed files with 91 additions and 71 deletions

View File

@@ -19,7 +19,7 @@ export function buildAgent(
browserProvider?: BrowserAutomationProvider,
disabledSkills?: Set<string>
): AgentConfig {
const base = isFactory(source) ? source(model) : source
const base = isFactory(source) ? source(model) : { ...source }
const categoryConfigs: Record<string, CategoryConfig> = categories
? { ...DEFAULT_CATEGORIES, ...categories }
: DEFAULT_CATEGORIES

View File

@@ -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 {

View File

@@ -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 ?? {}) }

View File

@@ -35,10 +35,13 @@ export async function addAuthPlugins(config: InstallConfig): Promise<ConfigMerge
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 plugins: string[] = existingConfig?.plugin ?? []

View File

@@ -22,11 +22,13 @@ export async function runBunInstallWithDetails(): Promise<BunInstallResult> {
stderr: "pipe",
})
const timeoutPromise = new Promise<"timeout">((resolve) =>
setTimeout(() => resolve("timeout"), BUN_INSTALL_TIMEOUT_MS)
)
let timeoutId: ReturnType<typeof setTimeout>
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 {

View File

@@ -9,7 +9,7 @@ const NPM_FETCH_TIMEOUT_MS = 5000
export async function fetchNpmDistTags(packageName: string): Promise<NpmDistTags | null> {
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

View File

@@ -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 {

View File

@@ -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<string, "modified" | "added" | "deleted">()
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<string>()
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

View File

@@ -21,6 +21,9 @@ function createMockPluginInput(options: {
prompt: async (input: unknown) => {
promptCalls.push({ input })
},
promptAsync: async (input: unknown) => {
promptCalls.push({ input })
},
},
},
}