fix(merge): resolve dev conflicts for openclaw branch
This commit is contained in:
GeonWoo Jeon (Jay)
@@ -1,5 +1,5 @@
|
||||
import { describe, test, expect, beforeEach, afterEach } from "bun:test"
|
||||
import { mkdtempSync, writeFileSync, rmSync } from "node:fs"
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"
|
||||
import { join } from "node:path"
|
||||
import { tmpdir } from "node:os"
|
||||
import {
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
clearCompactionAgentConfigCheckpoint,
|
||||
setCompactionAgentConfigCheckpoint,
|
||||
} from "../../shared/compaction-agent-config-checkpoint"
|
||||
import { PART_STORAGE } from "../../shared"
|
||||
|
||||
describe("isCompactionAgent", () => {
|
||||
describe("#given agent name variations", () => {
|
||||
@@ -73,6 +74,7 @@ describe("findNearestMessageExcludingCompaction", () => {
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tempDir, { force: true, recursive: true })
|
||||
rmSync(join(PART_STORAGE, "msg_test_background_compaction_marker"), { force: true, recursive: true })
|
||||
clearCompactionAgentConfigCheckpoint("ses_checkpoint")
|
||||
})
|
||||
|
||||
@@ -116,6 +118,30 @@ describe("findNearestMessageExcludingCompaction", () => {
|
||||
expect(result?.agent).toBe("sisyphus")
|
||||
})
|
||||
|
||||
test("skips JSON messages whose part storage contains a compaction marker", () => {
|
||||
// given
|
||||
const compactionMessageID = "msg_test_background_compaction_marker"
|
||||
const partDir = join(PART_STORAGE, compactionMessageID)
|
||||
writeFileSync(join(tempDir, "002.json"), JSON.stringify({
|
||||
id: compactionMessageID,
|
||||
agent: "atlas",
|
||||
model: { providerID: "anthropic", modelID: "claude-opus-4-6" },
|
||||
}))
|
||||
writeFileSync(join(tempDir, "001.json"), JSON.stringify({
|
||||
id: "msg_001",
|
||||
agent: "sisyphus",
|
||||
model: { providerID: "anthropic", modelID: "claude-opus-4-6" },
|
||||
}))
|
||||
mkdirSync(partDir, { recursive: true })
|
||||
writeFileSync(join(partDir, "prt_0001.json"), JSON.stringify({ type: "compaction" }))
|
||||
|
||||
// when
|
||||
const result = findNearestMessageExcludingCompaction(tempDir)
|
||||
|
||||
// then
|
||||
expect(result?.agent).toBe("sisyphus")
|
||||
})
|
||||
|
||||
test("falls back to partial agent/model match", () => {
|
||||
// given
|
||||
const messageWithAgentOnly = {
|
||||
@@ -256,4 +282,28 @@ describe("resolvePromptContextFromSessionMessages", () => {
|
||||
tools: { bash: true },
|
||||
})
|
||||
})
|
||||
|
||||
test("skips SDK messages that only exist to mark compaction", () => {
|
||||
// given
|
||||
const messages = [
|
||||
{
|
||||
id: "msg_compaction",
|
||||
info: { agent: "atlas", model: { providerID: "openai", modelID: "gpt-5" } },
|
||||
parts: [{ type: "compaction" }],
|
||||
},
|
||||
{ info: { agent: "sisyphus" } },
|
||||
{ info: { model: { providerID: "anthropic", modelID: "claude-opus-4-1" } } },
|
||||
{ info: { tools: { bash: true } } },
|
||||
]
|
||||
|
||||
// when
|
||||
const result = resolvePromptContextFromSessionMessages(messages)
|
||||
|
||||
// then
|
||||
expect(result).toEqual({
|
||||
agent: "sisyphus",
|
||||
model: { providerID: "anthropic", modelID: "claude-opus-4-1" },
|
||||
tools: { bash: true },
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2,8 +2,16 @@ import { readdirSync, readFileSync } from "node:fs"
|
||||
import { join } from "node:path"
|
||||
import type { StoredMessage } from "../hook-message-injector"
|
||||
import { getCompactionAgentConfigCheckpoint } from "../../shared/compaction-agent-config-checkpoint"
|
||||
import {
|
||||
hasCompactionPartInStorage,
|
||||
isCompactionAgent,
|
||||
isCompactionMessage,
|
||||
} from "../../shared/compaction-marker"
|
||||
|
||||
export { isCompactionAgent } from "../../shared/compaction-marker"
|
||||
|
||||
type SessionMessage = {
|
||||
id?: string
|
||||
info?: {
|
||||
agent?: string
|
||||
model?: {
|
||||
@@ -15,10 +23,7 @@ type SessionMessage = {
|
||||
modelID?: string
|
||||
tools?: StoredMessage["tools"]
|
||||
}
|
||||
}
|
||||
|
||||
export function isCompactionAgent(agent: string | undefined): boolean {
|
||||
return agent?.trim().toLowerCase() === "compaction"
|
||||
parts?: Array<{ type?: string }>
|
||||
}
|
||||
|
||||
function hasFullAgentAndModel(message: StoredMessage): boolean {
|
||||
@@ -35,6 +40,10 @@ function hasPartialAgentOrModel(message: StoredMessage): boolean {
|
||||
}
|
||||
|
||||
function convertSessionMessageToStoredMessage(message: SessionMessage): StoredMessage | null {
|
||||
if (isCompactionMessage(message)) {
|
||||
return null
|
||||
}
|
||||
|
||||
const info = message.info
|
||||
if (!info) {
|
||||
return null
|
||||
@@ -138,7 +147,11 @@ export function findNearestMessageExcludingCompaction(
|
||||
for (const file of files) {
|
||||
try {
|
||||
const content = readFileSync(join(messageDir, file), "utf-8")
|
||||
messages.push(JSON.parse(content) as StoredMessage)
|
||||
const parsed = JSON.parse(content) as StoredMessage & { id?: string }
|
||||
if (hasCompactionPartInStorage(parsed.id) || isCompactionAgent(parsed.agent)) {
|
||||
continue
|
||||
}
|
||||
messages.push(parsed)
|
||||
} catch {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -218,6 +218,10 @@ function getRootDescendantCounts(manager: BackgroundManager): Map<string, number
|
||||
return (manager as unknown as { rootDescendantCounts: Map<string, number> }).rootDescendantCounts
|
||||
}
|
||||
|
||||
function getPreStartDescendantReservations(manager: BackgroundManager): Set<string> {
|
||||
return (manager as unknown as { preStartDescendantReservations: Set<string> }).preStartDescendantReservations
|
||||
}
|
||||
|
||||
function getQueuesByKey(
|
||||
manager: BackgroundManager
|
||||
): Map<string, Array<{ task: BackgroundTask; input: import("./types").LaunchInput }>> {
|
||||
@@ -1144,7 +1148,18 @@ describe("BackgroundManager.notifyParentSession - notifications toggle", () => {
|
||||
prompt: promptMock,
|
||||
promptAsync: promptMock,
|
||||
abort: async () => ({}),
|
||||
messages: async () => ({ data: [] }),
|
||||
messages: async () => ({
|
||||
data: [{
|
||||
info: {
|
||||
agent: "explore",
|
||||
model: {
|
||||
providerID: "anthropic",
|
||||
modelID: "claude-opus-4-6",
|
||||
variant: "high",
|
||||
},
|
||||
},
|
||||
}],
|
||||
}),
|
||||
},
|
||||
}
|
||||
const manager = new BackgroundManager(
|
||||
@@ -1177,6 +1192,101 @@ describe("BackgroundManager.notifyParentSession - notifications toggle", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("BackgroundManager.notifyParentSession - variant propagation", () => {
|
||||
test("should prefer parent session variant over child task variant in parent notification promptAsync body", async () => {
|
||||
//#given
|
||||
const promptCalls: Array<{ body: Record<string, unknown> }> = []
|
||||
const client = {
|
||||
session: {
|
||||
prompt: async () => ({}),
|
||||
promptAsync: async (args: { path: { id: string }; body: Record<string, unknown> }) => {
|
||||
promptCalls.push({ body: args.body })
|
||||
return {}
|
||||
},
|
||||
abort: async () => ({}),
|
||||
messages: async () => ({
|
||||
data: [{
|
||||
info: {
|
||||
agent: "explore",
|
||||
model: {
|
||||
providerID: "anthropic",
|
||||
modelID: "claude-opus-4-6",
|
||||
variant: "max",
|
||||
},
|
||||
},
|
||||
}],
|
||||
}),
|
||||
},
|
||||
}
|
||||
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||
const task: BackgroundTask = {
|
||||
id: "task-parent-variant-wins",
|
||||
sessionID: "session-child",
|
||||
parentSessionID: "session-parent",
|
||||
parentMessageID: "msg-parent",
|
||||
description: "task with mismatched variant",
|
||||
prompt: "test",
|
||||
agent: "explore",
|
||||
status: "completed",
|
||||
startedAt: new Date(),
|
||||
completedAt: new Date(),
|
||||
model: { providerID: "anthropic", modelID: "claude-opus-4-6", variant: "high" },
|
||||
}
|
||||
getPendingByParent(manager).set("session-parent", new Set([task.id]))
|
||||
|
||||
//#when
|
||||
await (manager as unknown as { notifyParentSession: (task: BackgroundTask) => Promise<void> })
|
||||
.notifyParentSession(task)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(1)
|
||||
expect(promptCalls[0].body.variant).toBe("max")
|
||||
|
||||
manager.shutdown()
|
||||
})
|
||||
|
||||
test("should not include variant in promptAsync body when task has no variant", async () => {
|
||||
//#given
|
||||
const promptCalls: Array<{ body: Record<string, unknown> }> = []
|
||||
const client = {
|
||||
session: {
|
||||
prompt: async () => ({}),
|
||||
promptAsync: async (args: { path: { id: string }; body: Record<string, unknown> }) => {
|
||||
promptCalls.push({ body: args.body })
|
||||
return {}
|
||||
},
|
||||
abort: async () => ({}),
|
||||
messages: async () => ({ data: [] }),
|
||||
},
|
||||
}
|
||||
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||
const task: BackgroundTask = {
|
||||
id: "task-no-variant",
|
||||
sessionID: "session-child",
|
||||
parentSessionID: "session-parent",
|
||||
parentMessageID: "msg-parent",
|
||||
description: "task without variant",
|
||||
prompt: "test",
|
||||
agent: "explore",
|
||||
status: "completed",
|
||||
startedAt: new Date(),
|
||||
completedAt: new Date(),
|
||||
model: { providerID: "anthropic", modelID: "claude-opus-4-6" },
|
||||
}
|
||||
getPendingByParent(manager).set("session-parent", new Set([task.id]))
|
||||
|
||||
//#when
|
||||
await (manager as unknown as { notifyParentSession: (task: BackgroundTask) => Promise<void> })
|
||||
.notifyParentSession(task)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(1)
|
||||
expect(promptCalls[0].body.variant).toBeUndefined()
|
||||
|
||||
manager.shutdown()
|
||||
})
|
||||
})
|
||||
|
||||
describe("BackgroundManager.injectPendingNotificationsIntoChatMessage", () => {
|
||||
test("should prepend queued notifications to first text part and clear queue", () => {
|
||||
// given
|
||||
@@ -1437,6 +1547,7 @@ describe("BackgroundManager.tryCompleteTask", () => {
|
||||
|
||||
const task = createMockTask({
|
||||
id: "task-zombie-session",
|
||||
sessionID: "session-zombie-placeholder",
|
||||
parentSessionID: "parent-zombie",
|
||||
status: "pending",
|
||||
agent: "explore",
|
||||
@@ -1779,10 +1890,10 @@ describe("BackgroundManager.resume model persistence", () => {
|
||||
expect(getSessionPromptParams("session-advanced")).toEqual({
|
||||
temperature: 0.25,
|
||||
topP: 0.55,
|
||||
maxOutputTokens: 8192,
|
||||
options: {
|
||||
reasoningEffort: "high",
|
||||
thinking: { type: "disabled" },
|
||||
maxTokens: 8192,
|
||||
},
|
||||
})
|
||||
})
|
||||
@@ -2379,6 +2490,46 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
||||
expect(retryTask.status).toBe("pending")
|
||||
})
|
||||
|
||||
test("should only roll back the failed task reservation once when siblings still exist", async () => {
|
||||
// given
|
||||
const concurrencyKey = "test-agent"
|
||||
const task = createMockTask({
|
||||
id: "task-single-reservation-rollback",
|
||||
sessionID: "session-single-reservation-rollback",
|
||||
parentSessionID: "session-root",
|
||||
status: "pending",
|
||||
agent: "test-agent",
|
||||
rootSessionID: "session-root",
|
||||
})
|
||||
delete (task as Partial<BackgroundTask>).sessionID
|
||||
|
||||
const input = {
|
||||
description: task.description,
|
||||
prompt: task.prompt,
|
||||
agent: task.agent,
|
||||
parentSessionID: task.parentSessionID,
|
||||
parentMessageID: task.parentMessageID,
|
||||
}
|
||||
|
||||
getTaskMap(manager).set(task.id, task)
|
||||
getQueuesByKey(manager).set(concurrencyKey, [{ task, input }])
|
||||
getRootDescendantCounts(manager).set("session-root", 2)
|
||||
getPreStartDescendantReservations(manager).add(task.id)
|
||||
stubNotifyParentSession(manager)
|
||||
|
||||
;(manager as unknown as {
|
||||
startTask: (item: { task: BackgroundTask; input: typeof input }) => Promise<void>
|
||||
}).startTask = async () => {
|
||||
throw new Error("session create failed")
|
||||
}
|
||||
|
||||
// when
|
||||
await processKeyForTest(manager, concurrencyKey)
|
||||
|
||||
// then
|
||||
expect(getRootDescendantCounts(manager).get("session-root")).toBe(1)
|
||||
})
|
||||
|
||||
test("should keep the next queued task when the first task is cancelled during session creation", async () => {
|
||||
// given
|
||||
const firstSessionID = "ses-first-cancelled-during-create"
|
||||
|
||||
@@ -422,10 +422,6 @@ export class BackgroundManager {
|
||||
this.concurrencyManager.release(key)
|
||||
}
|
||||
|
||||
if (item.task.rootSessionID) {
|
||||
this.unregisterRootDescendant(item.task.rootSessionID)
|
||||
}
|
||||
|
||||
removeTaskToastTracking(item.task.id)
|
||||
|
||||
// Abort the orphaned session if one was created before the error
|
||||
@@ -1783,6 +1779,7 @@ export class BackgroundManager {
|
||||
let agent: string | undefined = task.parentAgent
|
||||
let model: { providerID: string; modelID: string } | undefined
|
||||
let tools: Record<string, boolean> | undefined = task.parentTools
|
||||
let promptContext: ReturnType<typeof resolvePromptContextFromSessionMessages> = null
|
||||
|
||||
if (this.enableParentSessionNotifications) {
|
||||
try {
|
||||
@@ -1796,7 +1793,7 @@ export class BackgroundManager {
|
||||
tools?: Record<string, boolean | "allow" | "deny" | "ask">
|
||||
}
|
||||
}>)
|
||||
const promptContext = resolvePromptContextFromSessionMessages(
|
||||
promptContext = resolvePromptContextFromSessionMessages(
|
||||
messages,
|
||||
task.parentSessionID,
|
||||
)
|
||||
@@ -1840,6 +1837,8 @@ export class BackgroundManager {
|
||||
const isTaskFailure = task.status === "error" || task.status === "cancelled" || task.status === "interrupt"
|
||||
const shouldReply = allComplete || isTaskFailure
|
||||
|
||||
const variant = promptContext?.model?.variant
|
||||
|
||||
try {
|
||||
await this.client.session.promptAsync({
|
||||
path: { id: task.parentSessionID },
|
||||
@@ -1847,6 +1846,7 @@ export class BackgroundManager {
|
||||
noReply: !shouldReply,
|
||||
...(agent !== undefined ? { agent } : {}),
|
||||
...(model !== undefined ? { model } : {}),
|
||||
...(variant !== undefined ? { variant } : {}),
|
||||
...(resolvedTools ? { tools: resolvedTools } : {}),
|
||||
parts: [createInternalAgentTextPart(notification)],
|
||||
},
|
||||
|
||||
@@ -400,10 +400,10 @@ describe("background-agent spawner fallback model promotion", () => {
|
||||
expect(getSessionPromptParams("session-123")).toEqual({
|
||||
temperature: 0.4,
|
||||
topP: 0.7,
|
||||
maxOutputTokens: 4096,
|
||||
options: {
|
||||
reasoningEffort: "high",
|
||||
thinking: { type: "disabled" },
|
||||
maxTokens: 4096,
|
||||
},
|
||||
})
|
||||
})
|
||||
@@ -466,4 +466,58 @@ describe("background-agent spawner fallback model promotion", () => {
|
||||
})
|
||||
expect(promptCalls[0]?.body?.variant).toBe("medium")
|
||||
})
|
||||
|
||||
test("strips leading zwsp from prompt body agent before promptAsync", async () => {
|
||||
//#given
|
||||
const promptCalls: Array<{ body?: { agent?: string } }> = []
|
||||
|
||||
const client = {
|
||||
session: {
|
||||
get: async () => ({ data: { directory: "/parent/dir" } }),
|
||||
create: async () => ({ data: { id: "ses_child_clean_agent" } }),
|
||||
promptAsync: async (args?: { body?: { agent?: string } }) => {
|
||||
promptCalls.push(args ?? {})
|
||||
return {}
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const task = createTask({
|
||||
description: "Test task",
|
||||
prompt: "Do work",
|
||||
agent: "\u200Bsisyphus-junior",
|
||||
parentSessionID: "ses_parent",
|
||||
parentMessageID: "msg_parent",
|
||||
})
|
||||
|
||||
const item = {
|
||||
task,
|
||||
input: {
|
||||
description: task.description,
|
||||
prompt: task.prompt,
|
||||
agent: task.agent,
|
||||
parentSessionID: task.parentSessionID,
|
||||
parentMessageID: task.parentMessageID,
|
||||
parentModel: task.parentModel,
|
||||
parentAgent: task.parentAgent,
|
||||
model: task.model,
|
||||
},
|
||||
}
|
||||
|
||||
const ctx = {
|
||||
client,
|
||||
directory: "/fallback",
|
||||
concurrencyManager: { release: () => {} },
|
||||
tmuxEnabled: false,
|
||||
onTaskError: () => {},
|
||||
}
|
||||
|
||||
//#when
|
||||
await startTask(item as any, ctx as any)
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(1)
|
||||
expect(promptCalls[0]?.body?.agent).toBe("sisyphus-junior")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -6,6 +6,7 @@ import { applySessionPromptParams } from "../../shared/session-prompt-params-hel
|
||||
import { subagentSessions } from "../claude-code-session-state"
|
||||
import { getTaskToastManager } from "../task-toast-manager"
|
||||
import { isInsideTmux } from "../../shared/tmux"
|
||||
import { stripAgentListSortPrefix } from "../../shared/agent-display-names"
|
||||
import type { ConcurrencyManager } from "./concurrency"
|
||||
|
||||
export const FALLBACK_AGENT = "general"
|
||||
@@ -168,11 +169,12 @@ export async function startTask(
|
||||
}
|
||||
: undefined
|
||||
const launchVariant = input.model?.variant
|
||||
const normalizedAgent = stripAgentListSortPrefix(input.agent)
|
||||
|
||||
applySessionPromptParams(sessionID, input.model)
|
||||
|
||||
const promptBody = {
|
||||
agent: input.agent,
|
||||
agent: normalizedAgent,
|
||||
...(launchModel ? { model: launchModel } : {}),
|
||||
...(launchVariant ? { variant: launchVariant } : {}),
|
||||
system: input.skillContent,
|
||||
@@ -180,7 +182,7 @@ export async function startTask(
|
||||
task: false,
|
||||
call_omo_agent: true,
|
||||
question: false,
|
||||
...getAgentToolRestrictions(input.agent),
|
||||
...getAgentToolRestrictions(normalizedAgent),
|
||||
},
|
||||
parts: [createInternalAgentTextPart(input.prompt)],
|
||||
}
|
||||
|
||||
@@ -1,6 +1,14 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import type { OpencodeClient } from "./constants"
|
||||
import { resolveSubagentSpawnContext } from "./subagent-spawn-limits"
|
||||
import {
|
||||
resolveSubagentSpawnContext,
|
||||
getMaxSubagentDepth,
|
||||
DEFAULT_MAX_SUBAGENT_DEPTH,
|
||||
createSubagentDepthLimitError,
|
||||
createSubagentDescendantLimitError,
|
||||
getMaxRootSessionSpawnBudget,
|
||||
DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET,
|
||||
} from "./subagent-spawn-limits"
|
||||
|
||||
function createMockClient(sessionGet: OpencodeClient["session"]["get"]): OpencodeClient {
|
||||
return {
|
||||
@@ -41,4 +49,177 @@ describe("resolveSubagentSpawnContext", () => {
|
||||
await expect(result).rejects.toThrow(/background_task\.maxDescendants cannot be enforced safely.*No session data returned/)
|
||||
})
|
||||
})
|
||||
|
||||
describe("depth calculation smoke tests (regression guard)", () => {
|
||||
test("root session (no parentID) reports depth 0 and childDepth 1", async () => {
|
||||
// given - a root session with no parent
|
||||
const client = createMockClient(async (opts) => {
|
||||
if (opts.path.id === "root-session") {
|
||||
return { data: { id: "root-session", parentID: undefined } }
|
||||
}
|
||||
return { error: "not found", data: undefined }
|
||||
})
|
||||
|
||||
// when
|
||||
const result = await resolveSubagentSpawnContext(client, "root-session")
|
||||
|
||||
// then
|
||||
expect(result.rootSessionID).toBe("root-session")
|
||||
expect(result.parentDepth).toBe(0)
|
||||
expect(result.childDepth).toBe(1)
|
||||
})
|
||||
|
||||
test("depth-1 child reports childDepth 2", async () => {
|
||||
// given - child -> root chain
|
||||
const client = createMockClient(async (opts) => {
|
||||
if (opts.path.id === "child-1") {
|
||||
return { data: { id: "child-1", parentID: "root-session" } }
|
||||
}
|
||||
if (opts.path.id === "root-session") {
|
||||
return { data: { id: "root-session", parentID: undefined } }
|
||||
}
|
||||
return { error: "not found", data: undefined }
|
||||
})
|
||||
|
||||
// when
|
||||
const result = await resolveSubagentSpawnContext(client, "child-1")
|
||||
|
||||
// then
|
||||
expect(result.rootSessionID).toBe("root-session")
|
||||
expect(result.parentDepth).toBe(1)
|
||||
expect(result.childDepth).toBe(2)
|
||||
})
|
||||
|
||||
test("depth-2 grandchild reports childDepth 3", async () => {
|
||||
// given - grandchild -> child -> root chain
|
||||
const client = createMockClient(async (opts) => {
|
||||
const sessions: Record<string, { id: string; parentID?: string }> = {
|
||||
"grandchild": { id: "grandchild", parentID: "child" },
|
||||
"child": { id: "child", parentID: "root" },
|
||||
"root": { id: "root", parentID: undefined },
|
||||
}
|
||||
const session = sessions[opts.path.id]
|
||||
if (session) return { data: session }
|
||||
return { error: "not found", data: undefined }
|
||||
})
|
||||
|
||||
// when
|
||||
const result = await resolveSubagentSpawnContext(client, "grandchild")
|
||||
|
||||
// then
|
||||
expect(result.rootSessionID).toBe("root")
|
||||
expect(result.parentDepth).toBe(2)
|
||||
expect(result.childDepth).toBe(3)
|
||||
})
|
||||
|
||||
test("depth at DEFAULT_MAX_SUBAGENT_DEPTH reports exact max childDepth", async () => {
|
||||
// given - chain of exactly DEFAULT_MAX_SUBAGENT_DEPTH depth
|
||||
// With default=3: session-3 -> session-2 -> session-1 -> root
|
||||
const sessions: Record<string, { id: string; parentID?: string }> = {
|
||||
"root": { id: "root" },
|
||||
}
|
||||
for (let i = 1; i <= DEFAULT_MAX_SUBAGENT_DEPTH; i++) {
|
||||
sessions[`session-${i}`] = {
|
||||
id: `session-${i}`,
|
||||
parentID: i === 1 ? "root" : `session-${i - 1}`,
|
||||
}
|
||||
}
|
||||
|
||||
const client = createMockClient(async (opts) => {
|
||||
const session = sessions[opts.path.id]
|
||||
if (session) return { data: session }
|
||||
return { error: "not found", data: undefined }
|
||||
})
|
||||
|
||||
// when - resolve from the deepest session
|
||||
const deepest = `session-${DEFAULT_MAX_SUBAGENT_DEPTH}`
|
||||
const result = await resolveSubagentSpawnContext(client, deepest)
|
||||
|
||||
// then - childDepth should be DEFAULT_MAX_SUBAGENT_DEPTH + 1 (exceeds limit)
|
||||
expect(result.childDepth).toBe(DEFAULT_MAX_SUBAGENT_DEPTH + 1)
|
||||
expect(result.parentDepth).toBe(DEFAULT_MAX_SUBAGENT_DEPTH)
|
||||
})
|
||||
|
||||
test("detects parent cycle and throws", async () => {
|
||||
// given - A -> B -> A (cycle)
|
||||
const client = createMockClient(async (opts) => {
|
||||
const sessions: Record<string, { id: string; parentID?: string }> = {
|
||||
"session-a": { id: "session-a", parentID: "session-b" },
|
||||
"session-b": { id: "session-b", parentID: "session-a" },
|
||||
}
|
||||
const session = sessions[opts.path.id]
|
||||
if (session) return { data: session }
|
||||
return { error: "not found", data: undefined }
|
||||
})
|
||||
|
||||
// when
|
||||
const result = resolveSubagentSpawnContext(client, "session-a")
|
||||
|
||||
// then
|
||||
await expect(result).rejects.toThrow(/session parent cycle/)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("getMaxSubagentDepth", () => {
|
||||
test("returns DEFAULT_MAX_SUBAGENT_DEPTH when no config", () => {
|
||||
expect(getMaxSubagentDepth()).toBe(DEFAULT_MAX_SUBAGENT_DEPTH)
|
||||
expect(getMaxSubagentDepth(undefined)).toBe(DEFAULT_MAX_SUBAGENT_DEPTH)
|
||||
})
|
||||
|
||||
test("returns config.maxDepth when provided", () => {
|
||||
expect(getMaxSubagentDepth({ maxDepth: 5 })).toBe(5)
|
||||
expect(getMaxSubagentDepth({ maxDepth: 1 })).toBe(1)
|
||||
expect(getMaxSubagentDepth({ maxDepth: 0 })).toBe(0)
|
||||
})
|
||||
|
||||
test("default is 3", () => {
|
||||
expect(DEFAULT_MAX_SUBAGENT_DEPTH).toBe(3)
|
||||
})
|
||||
})
|
||||
|
||||
describe("getMaxRootSessionSpawnBudget", () => {
|
||||
test("returns DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET when no config", () => {
|
||||
expect(getMaxRootSessionSpawnBudget()).toBe(DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET)
|
||||
})
|
||||
|
||||
test("returns config.maxDescendants when provided", () => {
|
||||
expect(getMaxRootSessionSpawnBudget({ maxDescendants: 10 })).toBe(10)
|
||||
})
|
||||
|
||||
test("default is 50", () => {
|
||||
expect(DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET).toBe(50)
|
||||
})
|
||||
})
|
||||
|
||||
describe("createSubagentDepthLimitError", () => {
|
||||
test("includes childDepth, maxDepth, and session IDs in message", () => {
|
||||
const error = createSubagentDepthLimitError({
|
||||
childDepth: 4,
|
||||
maxDepth: 3,
|
||||
parentSessionID: "parent-123",
|
||||
rootSessionID: "root-456",
|
||||
})
|
||||
|
||||
expect(error.message).toContain("child depth 4")
|
||||
expect(error.message).toContain("maxDepth=3")
|
||||
expect(error.message).toContain("parent-123")
|
||||
expect(error.message).toContain("root-456")
|
||||
expect(error.message).toContain("spawn blocked")
|
||||
})
|
||||
})
|
||||
|
||||
describe("createSubagentDescendantLimitError", () => {
|
||||
test("includes descendant count, max, and root session ID", () => {
|
||||
const error = createSubagentDescendantLimitError({
|
||||
rootSessionID: "root-789",
|
||||
descendantCount: 50,
|
||||
maxDescendants: 50,
|
||||
})
|
||||
|
||||
expect(error.message).toContain("root-789")
|
||||
expect(error.message).toContain("50")
|
||||
expect(error.message).toContain("maxDescendants=50")
|
||||
expect(error.message).toContain("spawn blocked")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -650,6 +650,65 @@ describe("boulder-state", () => {
|
||||
expect(progress.completed).toBe(1)
|
||||
expect(progress.isComplete).toBe(false)
|
||||
})
|
||||
|
||||
test("should count only top-level checkboxes for simple plans with nested tasks", () => {
|
||||
// given
|
||||
const planPath = join(TEST_DIR, "simple-nested-plan.md")
|
||||
writeFileSync(planPath, `# Plan
|
||||
|
||||
- [ ] Top-level task 1
|
||||
- [x] Nested task ignored
|
||||
- [x] Top-level task 2
|
||||
* [ ] Another nested task ignored
|
||||
`)
|
||||
|
||||
// when
|
||||
const progress = getPlanProgress(planPath)
|
||||
|
||||
// then
|
||||
expect(progress.total).toBe(2)
|
||||
expect(progress.completed).toBe(1)
|
||||
expect(progress.isComplete).toBe(false)
|
||||
})
|
||||
|
||||
test("should treat final-wave-only plans as structured mode", () => {
|
||||
// given
|
||||
const planPath = join(TEST_DIR, "final-wave-only-plan.md")
|
||||
writeFileSync(planPath, `# Plan
|
||||
|
||||
## Final Verification Wave
|
||||
- [ ] F1. Top-level final review
|
||||
- [x] Nested verification detail ignored
|
||||
`)
|
||||
|
||||
// when
|
||||
const progress = getPlanProgress(planPath)
|
||||
|
||||
// then
|
||||
expect(progress.total).toBe(1)
|
||||
expect(progress.completed).toBe(0)
|
||||
expect(progress.isComplete).toBe(false)
|
||||
})
|
||||
|
||||
test("should ignore mixed indentation levels in simple plans", () => {
|
||||
// given
|
||||
const planPath = join(TEST_DIR, "simple-mixed-indentation-plan.md")
|
||||
writeFileSync(planPath, `# Plan
|
||||
|
||||
* [x] Top-level star task
|
||||
- [ ] Indented task ignored
|
||||
- [x] Tab-indented task ignored
|
||||
- [ ] Top-level dash task
|
||||
`)
|
||||
|
||||
// when
|
||||
const progress = getPlanProgress(planPath)
|
||||
|
||||
// then
|
||||
expect(progress.total).toBe(2)
|
||||
expect(progress.completed).toBe(1)
|
||||
expect(progress.isComplete).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("getPlanName", () => {
|
||||
|
||||
@@ -226,7 +226,9 @@ export function getPlanProgress(planPath: string): PlanProgress {
|
||||
const lines = content.split(/\r?\n/)
|
||||
|
||||
// Check if the plan has structured sections (## TODOs / ## Final Verification Wave)
|
||||
const hasStructuredSections = lines.some((line) => TODO_HEADING_PATTERN.test(line))
|
||||
const hasStructuredSections = lines.some(
|
||||
(line) => TODO_HEADING_PATTERN.test(line) || FINAL_VERIFICATION_HEADING_PATTERN.test(line),
|
||||
)
|
||||
|
||||
if (hasStructuredSections) {
|
||||
// Structured plan: only count top-level checkboxes with numbered labels
|
||||
@@ -291,8 +293,8 @@ function getStructuredPlanProgress(lines: string[]): PlanProgress {
|
||||
}
|
||||
|
||||
function getSimplePlanProgress(content: string): PlanProgress {
|
||||
const uncheckedMatches = content.match(/^\s*[-*]\s*\[\s*\]/gm) || []
|
||||
const checkedMatches = content.match(/^\s*[-*]\s*\[[xX]\]/gm) || []
|
||||
const uncheckedMatches = content.match(/^[-*]\s*\[\s*\]/gm) || []
|
||||
const checkedMatches = content.match(/^[-*]\s*\[[xX]\]/gm) || []
|
||||
|
||||
const total = uncheckedMatches.length + checkedMatches.length
|
||||
const completed = checkedMatches.length
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { ULW_LOOP_TEMPLATE } from "./ralph-loop"
|
||||
|
||||
describe("ULW_LOOP_TEMPLATE", () => {
|
||||
test("returns the documented iteration caps for ultrawork and normal modes", () => {
|
||||
// given
|
||||
const expectedIterationCaps = "The iteration limit is 500 for ultrawork mode, 100 for normal mode"
|
||||
|
||||
// when
|
||||
const template = ULW_LOOP_TEMPLATE
|
||||
|
||||
// then
|
||||
expect(template).toContain(expectedIterationCaps)
|
||||
})
|
||||
})
|
||||
@@ -36,7 +36,7 @@ export const ULW_LOOP_TEMPLATE = `You are starting an ULTRAWORK Loop - a self-re
|
||||
2. When you believe the work is complete, output: \`<promise>{{COMPLETION_PROMISE}}</promise>\`
|
||||
3. That does NOT finish the loop yet. The system will require Oracle verification
|
||||
4. The loop only ends after the system confirms Oracle verified the result
|
||||
5. There is no iteration limit
|
||||
5. The iteration limit is 500 for ultrawork mode, 100 for normal mode
|
||||
|
||||
## Rules
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import { join } from "node:path"
|
||||
|
||||
const originalClaudePluginsHome = process.env.CLAUDE_PLUGINS_HOME
|
||||
const temporaryDirectories: string[] = []
|
||||
const originalCwd = process.cwd()
|
||||
|
||||
function createTemporaryDirectory(prefix: string): string {
|
||||
const directory = mkdtempSync(join(tmpdir(), prefix))
|
||||
@@ -17,6 +18,14 @@ function createTemporaryDirectory(prefix: string): string {
|
||||
return directory
|
||||
}
|
||||
|
||||
function writeDatabase(pluginsHome: string, database: unknown): void {
|
||||
writeFileSync(join(pluginsHome, "installed_plugins.json"), JSON.stringify(database), "utf-8")
|
||||
}
|
||||
|
||||
function createInstallPath(prefix: string): string {
|
||||
return createTemporaryDirectory(prefix)
|
||||
}
|
||||
|
||||
describe("discoverInstalledPlugins", () => {
|
||||
beforeEach(() => {
|
||||
mock.module("../../shared/logger", () => ({
|
||||
@@ -36,6 +45,10 @@ describe("discoverInstalledPlugins", () => {
|
||||
process.env.CLAUDE_PLUGINS_HOME = originalClaudePluginsHome
|
||||
}
|
||||
|
||||
if (process.cwd() !== originalCwd) {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
|
||||
for (const directory of temporaryDirectories.splice(0)) {
|
||||
rmSync(directory, { recursive: true, force: true })
|
||||
}
|
||||
@@ -156,4 +169,488 @@ describe("discoverInstalledPlugins", () => {
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("oh-my-openagent")
|
||||
})
|
||||
|
||||
describe("#given project-scoped entries in v1 format", () => {
|
||||
it("#when cwd matches projectPath #then the plugin loads", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v1-project-match-")
|
||||
const installPath = createInstallPath("omo-v1-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 1,
|
||||
plugins: {
|
||||
"project-plugin@market": {
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
},
|
||||
})
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v1-match`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("project-plugin")
|
||||
})
|
||||
|
||||
it("#when cwd is a subdirectory of projectPath #then the plugin loads", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v1-project-sub-")
|
||||
const subdirectory = join(projectDirectory, "packages", "app")
|
||||
mkdirSync(subdirectory, { recursive: true })
|
||||
const installPath = createInstallPath("omo-v1-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 1,
|
||||
plugins: {
|
||||
"sub-plugin@market": {
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
},
|
||||
})
|
||||
process.chdir(subdirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v1-sub`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("sub-plugin")
|
||||
})
|
||||
|
||||
it("#when cwd does not match projectPath #then the plugin is skipped", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v1-project-miss-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-v1-other-")
|
||||
const installPath = createInstallPath("omo-v1-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 1,
|
||||
plugins: {
|
||||
"outside-plugin@market": {
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
},
|
||||
})
|
||||
process.chdir(otherDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v1-miss`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("#when projectPath is missing #then the plugin is skipped", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const installPath = createInstallPath("omo-v1-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 1,
|
||||
plugins: {
|
||||
"no-path-plugin@market": {
|
||||
scope: "project",
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v1-noproj`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("#when scope is user #then it always loads regardless of cwd", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const unrelatedDirectory = createTemporaryDirectory("omo-v1-unrelated-")
|
||||
const installPath = createInstallPath("omo-v1-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 1,
|
||||
plugins: {
|
||||
"user-plugin@market": {
|
||||
scope: "user",
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
},
|
||||
})
|
||||
process.chdir(unrelatedDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v1-user`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("user-plugin")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given project and local scoped entries in v2 format", () => {
|
||||
it("#when cwd matches project-scoped projectPath #then it loads while non-matching entries are dropped", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v2-project-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-v2-other-")
|
||||
const matchingInstall = createInstallPath("omo-v2-match-install-")
|
||||
const missingInstall = createInstallPath("omo-v2-miss-install-")
|
||||
const userInstall = createInstallPath("omo-v2-user-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"matching-project@market": [
|
||||
{
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath: matchingInstall,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
"other-project@market": [
|
||||
{
|
||||
scope: "project",
|
||||
projectPath: otherDirectory,
|
||||
installPath: missingInstall,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
"global-user@market": [
|
||||
{
|
||||
scope: "user",
|
||||
installPath: userInstall,
|
||||
version: "2.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v2-mix`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
const names = discovered.plugins.map((plugin) => plugin.name).sort()
|
||||
expect(names).toEqual(["global-user", "matching-project"])
|
||||
})
|
||||
|
||||
it("#when scope is local and cwd matches projectPath #then it loads", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v2-local-match-")
|
||||
const installPath = createInstallPath("omo-v2-local-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"local-plugin@market": [
|
||||
{
|
||||
scope: "local",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v2-local-match`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("local-plugin")
|
||||
})
|
||||
|
||||
it("#when scope is local and cwd does not match projectPath #then it is skipped", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v2-local-miss-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-v2-local-other-")
|
||||
const installPath = createInstallPath("omo-v2-local-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"local-plugin@market": [
|
||||
{
|
||||
scope: "local",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
process.chdir(otherDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v2-local-miss`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("#when multiple installations are present #then only the first is considered and scope filtering still applies", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v2-multi-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-v2-multi-other-")
|
||||
const primaryInstall = createInstallPath("omo-v2-multi-primary-")
|
||||
const secondaryInstall = createInstallPath("omo-v2-multi-secondary-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"multi-plugin@market": [
|
||||
{
|
||||
scope: "project",
|
||||
projectPath: otherDirectory,
|
||||
installPath: primaryInstall,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
{
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath: secondaryInstall,
|
||||
version: "2.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v2-multi`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then — existing behavior keeps only the first entry; with scope filter it is
|
||||
// (correctly) skipped because the first entry points at a different project.
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given project and local scoped entries in v3 flat-array format", () => {
|
||||
it("#when cwd matches projectPath #then projectPath flows through and the plugin loads", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v3-match-")
|
||||
const installPath = createInstallPath("omo-v3-install-")
|
||||
writeDatabase(pluginsHome, [
|
||||
{
|
||||
name: "v3-project-plugin",
|
||||
marketplace: "market",
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
])
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v3-match`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("v3-project-plugin")
|
||||
})
|
||||
|
||||
it("#when cwd does not match projectPath #then the plugin is skipped", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-v3-miss-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-v3-miss-other-")
|
||||
const installPath = createInstallPath("omo-v3-install-")
|
||||
writeDatabase(pluginsHome, [
|
||||
{
|
||||
name: "v3-skipped-plugin",
|
||||
marketplace: "market",
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
{
|
||||
name: "v3-user-plugin",
|
||||
marketplace: "market",
|
||||
scope: "user",
|
||||
installPath: createInstallPath("omo-v3-user-install-"),
|
||||
version: "2.0.0",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
])
|
||||
process.chdir(otherDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-v3-miss`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("v3-user-plugin")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given enabledPluginsOverride combined with scope filtering", () => {
|
||||
it("#when a project-scoped plugin is disabled via override #then it is still skipped even if cwd would match", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-enabled-proj-")
|
||||
const installPath = createInstallPath("omo-enabled-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"gated-plugin@market": [
|
||||
{
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-enabled-off`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
enabledPluginsOverride: { "gated-plugin@market": false },
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("#when a project-scoped plugin is enabled and cwd matches #then it loads", async () => {
|
||||
//#given
|
||||
const pluginsHome = process.env.CLAUDE_PLUGINS_HOME as string
|
||||
const projectDirectory = createTemporaryDirectory("omo-enabled-match-")
|
||||
const installPath = createInstallPath("omo-enabled-match-install-")
|
||||
writeDatabase(pluginsHome, {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"enabled-plugin@market": [
|
||||
{
|
||||
scope: "project",
|
||||
projectPath: projectDirectory,
|
||||
installPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-03-25T00:00:00Z",
|
||||
lastUpdated: "2026-03-25T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
process.chdir(projectDirectory)
|
||||
|
||||
//#when
|
||||
const { discoverInstalledPlugins } = await import(`./discovery?t=${Date.now()}-enabled-on`)
|
||||
const discovered = discoverInstalledPlugins({
|
||||
pluginsHomeOverride: pluginsHome,
|
||||
loadPluginManifestOverride: () => null,
|
||||
enabledPluginsOverride: { "enabled-plugin@market": true },
|
||||
})
|
||||
|
||||
//#then
|
||||
expect(discovered.errors).toHaveLength(0)
|
||||
expect(discovered.plugins).toHaveLength(1)
|
||||
expect(discovered.plugins[0]?.name).toBe("enabled-plugin")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -3,6 +3,7 @@ import { homedir } from "os"
|
||||
import { basename, join } from "path"
|
||||
import { fileURLToPath } from "url"
|
||||
import { log } from "../../shared/logger"
|
||||
import { shouldLoadPluginForCwd } from "./scope-filter"
|
||||
import type {
|
||||
InstalledPluginsDatabase,
|
||||
InstalledPluginEntryV3,
|
||||
@@ -132,6 +133,7 @@ function v3EntryToInstallation(entry: InstalledPluginEntryV3): PluginInstallatio
|
||||
installedAt: entry.lastUpdated,
|
||||
lastUpdated: entry.lastUpdated,
|
||||
gitCommitSha: entry.gitCommitSha,
|
||||
projectPath: entry.projectPath,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -177,6 +179,7 @@ export function discoverInstalledPlugins(options?: PluginLoaderOptions): PluginL
|
||||
const settingsEnabledPlugins = settings?.enabledPlugins
|
||||
const overrideEnabledPlugins = options?.enabledPluginsOverride
|
||||
const pluginManifestLoader = options?.loadPluginManifestOverride ?? loadPluginManifest
|
||||
const cwd = process.cwd()
|
||||
|
||||
for (const [pluginKey, installation] of extractPluginEntries(db)) {
|
||||
if (!installation) continue
|
||||
@@ -186,6 +189,14 @@ export function discoverInstalledPlugins(options?: PluginLoaderOptions): PluginL
|
||||
continue
|
||||
}
|
||||
|
||||
if (!shouldLoadPluginForCwd(installation, cwd)) {
|
||||
log(`Skipping ${installation.scope}-scoped plugin outside current cwd: ${pluginKey}`, {
|
||||
projectPath: installation.projectPath,
|
||||
cwd,
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
const { installPath, scope, version } = installation
|
||||
|
||||
if (!existsSync(installPath)) {
|
||||
|
||||
@@ -0,0 +1,244 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
|
||||
import { mkdtempSync, rmSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { shouldLoadPluginForCwd } from "./scope-filter"
|
||||
|
||||
const temporaryDirectories: string[] = []
|
||||
|
||||
function createTemporaryDirectory(prefix: string): string {
|
||||
const directory = mkdtempSync(join(tmpdir(), prefix))
|
||||
temporaryDirectories.push(directory)
|
||||
return directory
|
||||
}
|
||||
|
||||
describe("shouldLoadPluginForCwd", () => {
|
||||
afterEach(() => {
|
||||
mock.restore()
|
||||
|
||||
for (const directory of temporaryDirectories.splice(0)) {
|
||||
rmSync(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
describe("#given a user-scoped plugin", () => {
|
||||
it("#when called with any cwd #then it loads", () => {
|
||||
//#given
|
||||
const installation = { scope: "user" as const }
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, "/tmp/anywhere")
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a managed-scoped plugin", () => {
|
||||
it("#when called with any cwd #then it loads", () => {
|
||||
//#given
|
||||
const installation = { scope: "managed" as const }
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, "/tmp/anywhere")
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a project-scoped plugin without projectPath", () => {
|
||||
it("#when called with any cwd #then it is skipped", () => {
|
||||
//#given
|
||||
const installation = { scope: "project" as const }
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, "/tmp/anywhere")
|
||||
|
||||
//#then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a local-scoped plugin without projectPath", () => {
|
||||
it("#when called with any cwd #then it is skipped", () => {
|
||||
//#given
|
||||
const installation = { scope: "local" as const }
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, "/tmp/anywhere")
|
||||
|
||||
//#then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a project-scoped plugin with matching projectPath", () => {
|
||||
it("#when cwd exactly matches projectPath #then it loads", () => {
|
||||
//#given
|
||||
const projectDirectory = createTemporaryDirectory("omo-scope-")
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: projectDirectory,
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, projectDirectory)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it("#when cwd is a subdirectory of projectPath #then it loads", () => {
|
||||
//#given
|
||||
const projectDirectory = createTemporaryDirectory("omo-scope-")
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: projectDirectory,
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, join(projectDirectory, "packages", "app"))
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a project-scoped plugin with non-matching projectPath", () => {
|
||||
it("#when cwd is unrelated #then it is skipped", () => {
|
||||
//#given
|
||||
const projectDirectory = createTemporaryDirectory("omo-scope-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-other-")
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: projectDirectory,
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, otherDirectory)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it("#when cwd is the parent of projectPath #then it is skipped", () => {
|
||||
//#given
|
||||
const projectDirectory = createTemporaryDirectory("omo-scope-")
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: join(projectDirectory, "nested"),
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, projectDirectory)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a local-scoped plugin with matching projectPath", () => {
|
||||
it("#when cwd matches projectPath #then it loads", () => {
|
||||
//#given
|
||||
const projectDirectory = createTemporaryDirectory("omo-scope-")
|
||||
const installation = {
|
||||
scope: "local" as const,
|
||||
projectPath: projectDirectory,
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, projectDirectory)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a local-scoped plugin with non-matching projectPath", () => {
|
||||
it("#when cwd is unrelated #then it is skipped", () => {
|
||||
//#given
|
||||
const projectDirectory = createTemporaryDirectory("omo-scope-")
|
||||
const otherDirectory = createTemporaryDirectory("omo-other-")
|
||||
const installation = {
|
||||
scope: "local" as const,
|
||||
projectPath: projectDirectory,
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = shouldLoadPluginForCwd(installation, otherDirectory)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a project-scoped plugin with a tilde-prefixed projectPath", () => {
|
||||
let fakeHome: string
|
||||
|
||||
beforeEach(() => {
|
||||
fakeHome = createTemporaryDirectory("omo-home-")
|
||||
mock.module("node:os", () => ({
|
||||
homedir: () => fakeHome,
|
||||
tmpdir,
|
||||
}))
|
||||
mock.module("os", () => ({
|
||||
homedir: () => fakeHome,
|
||||
tmpdir,
|
||||
}))
|
||||
})
|
||||
|
||||
it("#when the expanded home matches cwd #then it loads", async () => {
|
||||
//#given
|
||||
const { shouldLoadPluginForCwd: freshShouldLoad } = await import(
|
||||
`./scope-filter?t=${Date.now()}-tilde-match`
|
||||
)
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: "~/workspace/proj-a",
|
||||
}
|
||||
const cwd = join(fakeHome, "workspace", "proj-a")
|
||||
|
||||
//#when
|
||||
const result = freshShouldLoad(installation, cwd)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it("#when the expanded home does not match cwd #then it is skipped", async () => {
|
||||
//#given
|
||||
const { shouldLoadPluginForCwd: freshShouldLoad } = await import(
|
||||
`./scope-filter?t=${Date.now()}-tilde-mismatch`
|
||||
)
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: "~/workspace/proj-a",
|
||||
}
|
||||
const cwd = join(fakeHome, "workspace", "proj-b")
|
||||
|
||||
//#when
|
||||
const result = freshShouldLoad(installation, cwd)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it("#when projectPath is exactly ~ and cwd equals fake home #then it loads", async () => {
|
||||
//#given
|
||||
const { shouldLoadPluginForCwd: freshShouldLoad } = await import(
|
||||
`./scope-filter?t=${Date.now()}-tilde-root`
|
||||
)
|
||||
const installation = {
|
||||
scope: "project" as const,
|
||||
projectPath: "~",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = freshShouldLoad(installation, fakeHome)
|
||||
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,29 @@
|
||||
import { homedir } from "os"
|
||||
import { join } from "path"
|
||||
import { containsPath } from "../../shared/contains-path"
|
||||
import type { PluginInstallation } from "./types"
|
||||
|
||||
function expandTilde(inputPath: string): string {
|
||||
if (inputPath === "~") {
|
||||
return homedir()
|
||||
}
|
||||
if (inputPath.startsWith("~/") || inputPath.startsWith("~\\")) {
|
||||
return join(homedir(), inputPath.slice(2))
|
||||
}
|
||||
return inputPath
|
||||
}
|
||||
|
||||
export function shouldLoadPluginForCwd(
|
||||
installation: Pick<PluginInstallation, "scope" | "projectPath">,
|
||||
cwd: string = process.cwd(),
|
||||
): boolean {
|
||||
if (installation.scope !== "project" && installation.scope !== "local") {
|
||||
return true
|
||||
}
|
||||
|
||||
if (!installation.projectPath) {
|
||||
return false
|
||||
}
|
||||
|
||||
return containsPath(expandTilde(installation.projectPath), cwd)
|
||||
}
|
||||
@@ -18,6 +18,12 @@ export interface PluginInstallation {
|
||||
lastUpdated: string
|
||||
gitCommitSha?: string
|
||||
isLocal?: boolean
|
||||
/**
|
||||
* Claude Code records this on project/local-scoped installations.
|
||||
* Absolute path (or `~`-prefixed) of the project the plugin was installed for.
|
||||
* Used to filter project/local plugins that do not belong to the current cwd.
|
||||
*/
|
||||
projectPath?: string
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -51,6 +57,11 @@ export interface InstalledPluginEntryV3 {
|
||||
installPath: string
|
||||
lastUpdated: string
|
||||
gitCommitSha?: string
|
||||
/**
|
||||
* Claude Code records this on project/local-scoped installations.
|
||||
* Absolute path (or `~`-prefixed) of the project the plugin was installed for.
|
||||
*/
|
||||
projectPath?: string
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
getMainSessionID,
|
||||
registerAgentName,
|
||||
isAgentRegistered,
|
||||
resolveRegisteredAgentName,
|
||||
_resetForTesting,
|
||||
} from "./state"
|
||||
|
||||
@@ -140,6 +141,15 @@ describe("claude-code-session-state", () => {
|
||||
expect(isAgentRegistered("Atlas - Plan Executor")).toBe(true)
|
||||
})
|
||||
|
||||
test("should resolve config keys back to the registered raw agent name", () => {
|
||||
// given
|
||||
registerAgentName("\u200B\u200B\u200B\u200BAtlas - Plan Executor")
|
||||
|
||||
// when / then
|
||||
expect(resolveRegisteredAgentName("atlas")).toBe("\u200B\u200B\u200B\u200BAtlas - Plan Executor")
|
||||
expect(resolveRegisteredAgentName("Atlas - Plan Executor")).toBe("\u200B\u200B\u200B\u200BAtlas - Plan Executor")
|
||||
})
|
||||
|
||||
describe("#given atlas display name with zero-width prefix", () => {
|
||||
describe("#when checking registration without the zero-width prefix", () => {
|
||||
test("#then it treats the display name as registered", () => {
|
||||
|
||||
@@ -14,6 +14,7 @@ export function getMainSessionID(): string | undefined {
|
||||
}
|
||||
|
||||
const registeredAgentNames = new Set<string>()
|
||||
const registeredAgentAliases = new Map<string, string>()
|
||||
|
||||
const ZERO_WIDTH_CHARACTERS_REGEX = /[\u200B\u200C\u200D\uFEFF]/g
|
||||
|
||||
@@ -28,10 +29,16 @@ function normalizeStoredAgentName(name: string): string {
|
||||
export function registerAgentName(name: string): void {
|
||||
const normalizedName = normalizeRegisteredAgentName(name)
|
||||
registeredAgentNames.add(normalizedName)
|
||||
if (!registeredAgentAliases.has(normalizedName)) {
|
||||
registeredAgentAliases.set(normalizedName, name)
|
||||
}
|
||||
|
||||
const configKey = normalizeRegisteredAgentName(getAgentConfigKey(name))
|
||||
if (configKey !== normalizedName) {
|
||||
registeredAgentNames.add(configKey)
|
||||
if (!registeredAgentAliases.has(configKey)) {
|
||||
registeredAgentAliases.set(configKey, name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,6 +46,15 @@ export function isAgentRegistered(name: string): boolean {
|
||||
return registeredAgentNames.has(normalizeRegisteredAgentName(name))
|
||||
}
|
||||
|
||||
export function resolveRegisteredAgentName(name: string | undefined): string | undefined {
|
||||
if (typeof name !== "string") {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const normalizedName = normalizeRegisteredAgentName(name)
|
||||
return registeredAgentAliases.get(normalizedName) ?? normalizeStoredAgentName(name)
|
||||
}
|
||||
|
||||
/** @internal For testing only */
|
||||
export function _resetForTesting(): void {
|
||||
_mainSessionID = undefined
|
||||
@@ -46,6 +62,7 @@ export function _resetForTesting(): void {
|
||||
syncSubagentSessions.clear()
|
||||
sessionAgentMap.clear()
|
||||
registeredAgentNames.clear()
|
||||
registeredAgentAliases.clear()
|
||||
}
|
||||
|
||||
const sessionAgentMap = new Map<string, string>()
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
generatePartId,
|
||||
injectHookMessage,
|
||||
} from "./injector"
|
||||
import { PART_STORAGE } from "../../shared"
|
||||
import { isSqliteBackend, resetSqliteBackendCache } from "../../shared/opencode-storage-detection"
|
||||
|
||||
//#region Mocks
|
||||
@@ -53,6 +54,7 @@ function createMockClient(messages: Array<{
|
||||
tools?: Record<string, boolean>
|
||||
time?: { created?: number }
|
||||
}
|
||||
parts?: Array<{ type?: string }>
|
||||
}>): {
|
||||
session: {
|
||||
messages: (opts: { path: { id: string } }) => Promise<{ data: typeof messages }>
|
||||
@@ -176,6 +178,24 @@ describe("findNearestMessageWithFieldsFromSDK", () => {
|
||||
|
||||
expect(result?.agent).toBe("newest-by-time")
|
||||
})
|
||||
|
||||
it("skips compaction marker user messages when resolving nearest message", async () => {
|
||||
const mockClient = createMockClient([
|
||||
{
|
||||
id: "msg_compaction",
|
||||
info: { agent: "atlas", model: { providerID: "openai", modelID: "gpt-5" }, time: { created: 200 } },
|
||||
parts: [{ type: "compaction" }],
|
||||
},
|
||||
{
|
||||
id: "msg_real",
|
||||
info: { agent: "sisyphus", model: { providerID: "anthropic", modelID: "claude-opus-4" }, time: { created: 100 } },
|
||||
},
|
||||
])
|
||||
|
||||
const result = await findNearestMessageWithFieldsFromSDK(mockClient as any, "ses_123")
|
||||
|
||||
expect(result?.agent).toBe("sisyphus")
|
||||
})
|
||||
})
|
||||
|
||||
describe("findNearestMessageWithFields JSON backend ordering", () => {
|
||||
@@ -197,6 +217,34 @@ describe("findNearestMessageWithFields JSON backend ordering", () => {
|
||||
|
||||
expect(result?.agent).toBe("newest-by-time")
|
||||
})
|
||||
|
||||
it("skips JSON messages whose parts contain a compaction marker", () => {
|
||||
mockIsSqliteBackend.mockReturnValue(false)
|
||||
const messageDir = createMessageDir()
|
||||
const compactionMessageID = "msg_test_injector_compaction_marker"
|
||||
const partDir = join(PART_STORAGE, compactionMessageID)
|
||||
tempDirs.push(partDir)
|
||||
|
||||
writeFileSync(join(messageDir, "msg_0001.json"), JSON.stringify({
|
||||
id: compactionMessageID,
|
||||
agent: "atlas",
|
||||
model: { providerID: "openai", modelID: "gpt-5" },
|
||||
time: { created: 200 },
|
||||
}))
|
||||
mkdirSync(partDir, { recursive: true })
|
||||
writeFileSync(join(partDir, "prt_0001.json"), JSON.stringify({ type: "compaction" }))
|
||||
|
||||
writeFileSync(join(messageDir, "msg_0002.json"), JSON.stringify({
|
||||
id: "msg_0002",
|
||||
agent: "sisyphus",
|
||||
model: { providerID: "anthropic", modelID: "claude-opus-4" },
|
||||
time: { created: 100 },
|
||||
}))
|
||||
|
||||
const result = findNearestMessageWithFields(messageDir)
|
||||
|
||||
expect(result?.agent).toBe("sisyphus")
|
||||
})
|
||||
})
|
||||
|
||||
describe("findFirstMessageWithAgentFromSDK", () => {
|
||||
@@ -222,6 +270,17 @@ describe("findFirstMessageWithAgentFromSDK", () => {
|
||||
expect(result).toBe("earliest-agent")
|
||||
})
|
||||
|
||||
it("skips compaction marker user messages when resolving first agent", async () => {
|
||||
const mockClient = createMockClient([
|
||||
{ id: "msg_compaction", info: { agent: "atlas", time: { created: 10 } }, parts: [{ type: "compaction" }] },
|
||||
{ id: "msg_real", info: { agent: "sisyphus", time: { created: 20 } } },
|
||||
])
|
||||
|
||||
const result = await findFirstMessageWithAgentFromSDK(mockClient as any, "ses_123")
|
||||
|
||||
expect(result).toBe("sisyphus")
|
||||
})
|
||||
|
||||
it("skips messages without agent field", async () => {
|
||||
const mockClient = createMockClient([
|
||||
{ info: {} },
|
||||
|
||||
@@ -7,6 +7,7 @@ import type { MessageMeta, OriginalMessageContext, TextPart, ToolPermission } fr
|
||||
import { log } from "../../shared/logger"
|
||||
import { isSqliteBackend } from "../../shared/opencode-storage-detection"
|
||||
import { createInternalAgentTextPart, normalizeSDKResponse } from "../../shared"
|
||||
import { hasCompactionPartInStorage, isCompactionMessage } from "../../shared/compaction-marker"
|
||||
|
||||
export interface StoredMessage {
|
||||
agent?: string
|
||||
@@ -32,6 +33,7 @@ interface SDKMessage {
|
||||
created?: number
|
||||
}
|
||||
}
|
||||
parts?: Array<{ type?: string }>
|
||||
}
|
||||
|
||||
const processPrefix = randomBytes(4).toString("hex")
|
||||
@@ -39,6 +41,10 @@ let messageCounter = 0
|
||||
let partCounter = 0
|
||||
|
||||
function convertSDKMessageToStoredMessage(msg: SDKMessage): StoredMessage | null {
|
||||
if (isCompactionMessage(msg)) {
|
||||
return null
|
||||
}
|
||||
|
||||
const info = msg.info
|
||||
if (!info) return null
|
||||
|
||||
@@ -164,22 +170,38 @@ export function findNearestMessageWithFields(messageDir: string): StoredMessage
|
||||
return {
|
||||
fileName,
|
||||
msg,
|
||||
hasCompactionMarker: hasCompactionPartInStorage(
|
||||
typeof (msg as { id?: unknown }).id === "string" ? (msg as { id?: string }).id : undefined,
|
||||
),
|
||||
createdAt: typeof msg.time?.created === "number" ? msg.time.created : Number.NEGATIVE_INFINITY,
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
})
|
||||
.filter((entry): entry is { fileName: string; msg: StoredMessage & { time?: { created?: number } }; createdAt: number } => entry !== null)
|
||||
.filter((entry): entry is {
|
||||
fileName: string
|
||||
msg: StoredMessage & { time?: { created?: number } }
|
||||
hasCompactionMarker: boolean
|
||||
createdAt: number
|
||||
} => entry !== null)
|
||||
.sort((left, right) => right.createdAt - left.createdAt || right.fileName.localeCompare(left.fileName))
|
||||
|
||||
for (const entry of messages) {
|
||||
if (entry.hasCompactionMarker || isCompactionMessage({ agent: entry.msg.agent })) {
|
||||
continue
|
||||
}
|
||||
|
||||
if (entry.msg.agent && entry.msg.model?.providerID && entry.msg.model?.modelID) {
|
||||
return entry.msg
|
||||
}
|
||||
}
|
||||
|
||||
for (const entry of messages) {
|
||||
if (entry.hasCompactionMarker || isCompactionMessage({ agent: entry.msg.agent })) {
|
||||
continue
|
||||
}
|
||||
|
||||
if (entry.msg.agent || (entry.msg.model?.providerID && entry.msg.model?.modelID)) {
|
||||
return entry.msg
|
||||
}
|
||||
@@ -216,16 +238,28 @@ export function findFirstMessageWithAgent(messageDir: string): string | null {
|
||||
return {
|
||||
fileName,
|
||||
msg,
|
||||
hasCompactionMarker: hasCompactionPartInStorage(
|
||||
typeof (msg as { id?: unknown }).id === "string" ? (msg as { id?: string }).id : undefined,
|
||||
),
|
||||
createdAt: typeof msg.time?.created === "number" ? msg.time.created : Number.POSITIVE_INFINITY,
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
})
|
||||
.filter((entry): entry is { fileName: string; msg: StoredMessage & { time?: { created?: number } }; createdAt: number } => entry !== null)
|
||||
.filter((entry): entry is {
|
||||
fileName: string
|
||||
msg: StoredMessage & { time?: { created?: number } }
|
||||
hasCompactionMarker: boolean
|
||||
createdAt: number
|
||||
} => entry !== null)
|
||||
.sort((left, right) => left.createdAt - right.createdAt || left.fileName.localeCompare(right.fileName))
|
||||
|
||||
for (const entry of messages) {
|
||||
if (entry.hasCompactionMarker || isCompactionMessage({ agent: entry.msg.agent })) {
|
||||
continue
|
||||
}
|
||||
|
||||
if (entry.msg.agent) {
|
||||
return entry.msg.agent
|
||||
}
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
import type { OAuthTokenData } from "./storage"
|
||||
|
||||
/**
|
||||
* Per-server OAuth refresh mutex to prevent concurrent refresh race conditions.
|
||||
*
|
||||
* When multiple operations need to refresh a token for the same server,
|
||||
* this ensures only one refresh request is made and all waiters receive
|
||||
* the same result.
|
||||
*/
|
||||
|
||||
const ongoingRefreshes = new Map<string, Promise<OAuthTokenData>>()
|
||||
|
||||
/**
|
||||
* Execute a token refresh with per-server mutual exclusion.
|
||||
*
|
||||
* If a refresh is already in progress for the given server, this will
|
||||
* return the same promise to all concurrent callers. Once the refresh
|
||||
* completes (success or failure), the lock is released.
|
||||
*
|
||||
* @param serverUrl - The OAuth server URL (used as mutex key)
|
||||
* @param refreshFn - The actual refresh operation to execute
|
||||
* @returns Promise that resolves to the new token data
|
||||
*/
|
||||
export async function withRefreshMutex(
|
||||
serverUrl: string,
|
||||
refreshFn: () => Promise<OAuthTokenData>,
|
||||
): Promise<OAuthTokenData> {
|
||||
const existing = ongoingRefreshes.get(serverUrl)
|
||||
if (existing) {
|
||||
return existing
|
||||
}
|
||||
|
||||
const refreshPromise = refreshFn().finally(() => {
|
||||
ongoingRefreshes.delete(serverUrl)
|
||||
})
|
||||
|
||||
ongoingRefreshes.set(serverUrl, refreshPromise)
|
||||
return refreshPromise
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a refresh is currently in progress for a server.
|
||||
*
|
||||
* @param serverUrl - The OAuth server URL
|
||||
* @returns true if a refresh operation is active
|
||||
*/
|
||||
export function isRefreshInProgress(serverUrl: string): boolean {
|
||||
return ongoingRefreshes.has(serverUrl)
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the number of servers currently undergoing token refresh.
|
||||
*
|
||||
* @returns Number of active refresh operations
|
||||
*/
|
||||
export function getActiveRefreshCount(): number {
|
||||
return ongoingRefreshes.size
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"
|
||||
import { chmodSync, existsSync, mkdirSync, readFileSync, renameSync, unlinkSync, writeFileSync } from "node:fs"
|
||||
import { dirname, join } from "node:path"
|
||||
import { getOpenCodeConfigDir } from "../../shared"
|
||||
|
||||
@@ -82,8 +82,10 @@ function writeStore(store: TokenStore): boolean {
|
||||
mkdirSync(dir, { recursive: true })
|
||||
}
|
||||
|
||||
writeFileSync(filePath, JSON.stringify(store, null, 2), { encoding: "utf-8", mode: 0o600 })
|
||||
chmodSync(filePath, 0o600)
|
||||
const tempPath = `${filePath}.tmp.${Date.now()}`
|
||||
writeFileSync(tempPath, JSON.stringify(store, null, 2), { encoding: "utf-8", mode: 0o600 })
|
||||
chmodSync(tempPath, 0o600)
|
||||
renameSync(tempPath, filePath)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterAll, afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
|
||||
import { afterAll, afterEach, beforeEach, describe, expect, it, mock, test } from "bun:test"
|
||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import type { SkillMcpClientInfo, SkillMcpManagerState } from "./types"
|
||||
|
||||
@@ -89,11 +89,15 @@ function createState(): SkillMcpManagerState {
|
||||
return state
|
||||
}
|
||||
|
||||
function createClientInfo(serverName: string): SkillMcpClientInfo {
|
||||
function createClientInfo(
|
||||
serverName: string,
|
||||
scope?: SkillMcpClientInfo["scope"],
|
||||
): SkillMcpClientInfo {
|
||||
return {
|
||||
serverName,
|
||||
skillName: "env-skill",
|
||||
sessionID: "session-env",
|
||||
...(scope !== undefined ? { scope } : {}),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -125,6 +129,68 @@ afterEach(async () => {
|
||||
})
|
||||
|
||||
describe("getOrCreateClient env var expansion", () => {
|
||||
describe("#given a scope-sensitive stdio skill MCP config", () => {
|
||||
test.each([
|
||||
["opencode-project", "Authorization:Bearer "],
|
||||
["local", "Authorization:Bearer "],
|
||||
["user", "Authorization:Bearer xoxp-scope-token"],
|
||||
["builtin", "Authorization:Bearer xoxp-scope-token"],
|
||||
] satisfies Array<[NonNullable<SkillMcpClientInfo["scope"]>, string]>) (
|
||||
"#when creating the client for %s scope #then args expand to %s",
|
||||
async (scope, expectedAuthorizationHeader) => {
|
||||
// given
|
||||
process.env.SLACK_USER_TOKEN = "xoxp-scope-token"
|
||||
const state = createState()
|
||||
const info = createClientInfo(`scope-${scope}`, scope)
|
||||
const clientKey = createClientKey(info)
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
command: "npx",
|
||||
args: [
|
||||
"-y",
|
||||
"mcp-remote",
|
||||
"https://mcp.slack.com/mcp",
|
||||
"--header",
|
||||
"Authorization:Bearer ${SLACK_USER_TOKEN}",
|
||||
],
|
||||
}
|
||||
|
||||
// when
|
||||
await getOrCreateClient({ state, clientKey, info, config })
|
||||
|
||||
// then
|
||||
expect(createdStdioTransports).toHaveLength(1)
|
||||
expect(createdStdioTransports[0]?.options.args?.[4]).toBe(expectedAuthorizationHeader)
|
||||
},
|
||||
)
|
||||
|
||||
it("#when creating the client without scope #then env vars remain trusted for backward compatibility", async () => {
|
||||
// given
|
||||
process.env.SLACK_USER_TOKEN = "xoxp-undefined-scope-token"
|
||||
const state = createState()
|
||||
const info = createClientInfo("scope-undefined")
|
||||
const clientKey = createClientKey(info)
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
command: "npx",
|
||||
args: [
|
||||
"-y",
|
||||
"mcp-remote",
|
||||
"https://mcp.slack.com/mcp",
|
||||
"--header",
|
||||
"Authorization:Bearer ${SLACK_USER_TOKEN}",
|
||||
],
|
||||
}
|
||||
|
||||
// when
|
||||
await getOrCreateClient({ state, clientKey, info, config })
|
||||
|
||||
// then
|
||||
expect(createdStdioTransports).toHaveLength(1)
|
||||
expect(createdStdioTransports[0]?.options.args?.[4]).toBe(
|
||||
"Authorization:Bearer xoxp-undefined-scope-token",
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a stdio skill MCP config with sensitive env vars in args", () => {
|
||||
it("#when creating the client #then sensitive env vars in args are expanded", async () => {
|
||||
// given
|
||||
|
||||
@@ -95,6 +95,7 @@ function createClientInfo(sessionID: string): SkillMcpClientInfo {
|
||||
serverName: "race-server",
|
||||
skillName: "race-skill",
|
||||
sessionID,
|
||||
scope: "builtin",
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -14,6 +14,8 @@ function removeClientIfCurrent(state: SkillMcpManagerState, clientKey: string, c
|
||||
}
|
||||
}
|
||||
|
||||
const PROJECT_SCOPES = new Set(["project", "opencode-project", "local"])
|
||||
|
||||
export async function getOrCreateClient(params: {
|
||||
state: SkillMcpManagerState
|
||||
clientKey: string
|
||||
@@ -38,7 +40,8 @@ export async function getOrCreateClient(params: {
|
||||
return pending
|
||||
}
|
||||
|
||||
const expandedConfig = expandEnvVarsInObject(config, { trusted: true })
|
||||
const isTrusted = !PROJECT_SCOPES.has(info.scope ?? "")
|
||||
const expandedConfig = expandEnvVarsInObject(config, { trusted: isTrusted })
|
||||
let currentConnectionPromise!: Promise<Client>
|
||||
state.inFlightConnections.set(info.sessionID, (state.inFlightConnections.get(info.sessionID) ?? 0) + 1)
|
||||
currentConnectionPromise = (async () => {
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
// Redacts sensitive tokens from error messages to prevent credential exposure
|
||||
// Follows same patterns as env-cleaner.ts for consistency
|
||||
|
||||
const SENSITIVE_PATTERNS: RegExp[] = [
|
||||
// API keys and tokens in common formats
|
||||
/[a-zA-Z0-9_-]*(?:api[_-]?key|apikey)["\s]*[:=]["\s]*([a-zA-Z0-9_-]{16,})/gi,
|
||||
/[a-zA-Z0-9_-]*(?:auth[_-]?token|authtoken)["\s]*[:=]["\s]*([a-zA-Z0-9_-]{16,})/gi,
|
||||
/[a-zA-Z0-9_-]*(?:access[_-]?token|accesstoken)["\s]*[:=]["\s]*([a-zA-Z0-9_-]{16,})/gi,
|
||||
/[a-zA-Z0-9_-]*(?:secret)["\s]*[:=]["\s]*([a-zA-Z0-9_-]{16,})/gi,
|
||||
/[a-zA-Z0-9_-]*(?:password)["\s]*[:=]["\s]*([a-zA-Z0-9_-]{8,})/gi,
|
||||
|
||||
// Bearer tokens
|
||||
/bearer\s+([a-zA-Z0-9_-]{20,})/gi,
|
||||
|
||||
// Common token prefixes
|
||||
/sk-[a-zA-Z0-9]{20,}/g, // OpenAI-style secret keys
|
||||
/gh[pousr]_[a-zA-Z0-9]{20,}/gi, // GitHub tokens
|
||||
/glpat-[a-zA-Z0-9_-]{20,}/gi, // GitLab tokens
|
||||
/[A-Za-z0-9_]{20,}-[A-Za-z0-9_]{10,}-[A-Za-z0-9_]{10,}/g, // Common JWT-like patterns
|
||||
]
|
||||
|
||||
const REDACTION_MARKER = "[REDACTED]"
|
||||
|
||||
/**
|
||||
* Redacts sensitive tokens from a string.
|
||||
* Used for error messages that may contain command-line arguments or environment info.
|
||||
*/
|
||||
export function redactSensitiveData(input: string): string {
|
||||
let result = input
|
||||
|
||||
for (const pattern of SENSITIVE_PATTERNS) {
|
||||
result = result.replace(pattern, REDACTION_MARKER)
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
/**
|
||||
* Redacts sensitive data from an Error object, returning a new Error.
|
||||
* Preserves the stack trace but redacts the message.
|
||||
*/
|
||||
export function redactErrorSensitiveData(error: Error): Error {
|
||||
const redactedMessage = redactSensitiveData(error.message)
|
||||
const redactedError = new Error(redactedMessage)
|
||||
redactedError.stack = error.stack ? redactSensitiveData(error.stack) : undefined
|
||||
return redactedError
|
||||
}
|
||||
@@ -0,0 +1,162 @@
|
||||
import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test"
|
||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import type { OAuthTokenData } from "../mcp-oauth/storage"
|
||||
import type { SkillMcpClientInfo, SkillMcpServerContext } from "./types"
|
||||
|
||||
const mockGetOrCreateClient = mock(async () => {
|
||||
throw new Error("not used")
|
||||
})
|
||||
|
||||
const mockGetOrCreateClientWithRetryImpl = mock(async () => ({
|
||||
callTool: mock(async () => ({ content: [{ type: "text", text: "unused" }] })),
|
||||
close: mock(async () => {}),
|
||||
}))
|
||||
|
||||
type ManagerModule = typeof import("./manager")
|
||||
|
||||
async function importFreshManagerModule(): Promise<ManagerModule> {
|
||||
mock.module("./connection", () => ({
|
||||
getOrCreateClient: mockGetOrCreateClient,
|
||||
getOrCreateClientWithRetryImpl: mockGetOrCreateClientWithRetryImpl,
|
||||
}))
|
||||
|
||||
mock.module("../mcp-oauth/provider", () => ({
|
||||
McpOAuthProvider: class MockMcpOAuthProvider {},
|
||||
}))
|
||||
|
||||
return await import(new URL(`./manager.ts?oauth-retry-test=${Date.now()}-${Math.random()}`, import.meta.url).href)
|
||||
}
|
||||
|
||||
function createInfo(): SkillMcpClientInfo {
|
||||
return {
|
||||
serverName: "oauth-server",
|
||||
skillName: "oauth-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
}
|
||||
|
||||
function createContext(): SkillMcpServerContext {
|
||||
return {
|
||||
skillName: "oauth-skill",
|
||||
config: {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
oauth: { clientId: "test-client" },
|
||||
} satisfies ClaudeCodeMcpServer,
|
||||
}
|
||||
}
|
||||
|
||||
afterAll(() => {
|
||||
mock.restore()
|
||||
})
|
||||
|
||||
describe("SkillMcpManager post-request OAuth retry", () => {
|
||||
beforeEach(() => {
|
||||
mockGetOrCreateClient.mockClear()
|
||||
mockGetOrCreateClientWithRetryImpl.mockClear()
|
||||
})
|
||||
|
||||
it("retries the operation after a 401 refresh succeeds", async () => {
|
||||
// given
|
||||
const { SkillMcpManager } = await importFreshManagerModule()
|
||||
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
||||
const manager = new SkillMcpManager({
|
||||
createOAuthProvider: () => ({
|
||||
tokens: () => ({ accessToken: "stale-token", refreshToken: "refresh-token" }),
|
||||
login: mock(async () => ({ accessToken: "login-token" } satisfies OAuthTokenData)),
|
||||
refresh,
|
||||
}),
|
||||
})
|
||||
const callTool = mock(async () => {
|
||||
if (callTool.mock.calls.length === 1) {
|
||||
throw new Error("401 Unauthorized")
|
||||
}
|
||||
|
||||
return { content: [{ type: "text", text: "success" }] }
|
||||
})
|
||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
||||
|
||||
// when
|
||||
const result = await manager.callTool(createInfo(), createContext(), "test-tool", {})
|
||||
|
||||
// then
|
||||
expect(result).toEqual([{ type: "text", text: "success" }])
|
||||
expect(refresh).toHaveBeenCalledTimes(1)
|
||||
expect(callTool).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it("retries the operation after a 403 refresh succeeds without step-up scope", async () => {
|
||||
// given
|
||||
const { SkillMcpManager } = await importFreshManagerModule()
|
||||
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
||||
const manager = new SkillMcpManager({
|
||||
createOAuthProvider: () => ({
|
||||
tokens: () => ({ accessToken: "stale-token", refreshToken: "refresh-token" }),
|
||||
login: mock(async () => ({ accessToken: "login-token" } satisfies OAuthTokenData)),
|
||||
refresh,
|
||||
}),
|
||||
})
|
||||
const callTool = mock(async () => {
|
||||
if (callTool.mock.calls.length === 1) {
|
||||
throw new Error("403 Forbidden")
|
||||
}
|
||||
|
||||
return { content: [{ type: "text", text: "success" }] }
|
||||
})
|
||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
||||
|
||||
// when
|
||||
const result = await manager.callTool(createInfo(), createContext(), "test-tool", {})
|
||||
|
||||
// then
|
||||
expect(result).toEqual([{ type: "text", text: "success" }])
|
||||
expect(refresh).toHaveBeenCalledTimes(1)
|
||||
expect(callTool).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it("propagates the auth error without retry when refresh fails", async () => {
|
||||
// given
|
||||
const { SkillMcpManager } = await importFreshManagerModule()
|
||||
const refresh = mock(async () => {
|
||||
throw new Error("refresh failed")
|
||||
})
|
||||
const manager = new SkillMcpManager({
|
||||
createOAuthProvider: () => ({
|
||||
tokens: () => ({ accessToken: "stale-token", refreshToken: "refresh-token" }),
|
||||
login: mock(async () => ({ accessToken: "login-token" } satisfies OAuthTokenData)),
|
||||
refresh,
|
||||
}),
|
||||
})
|
||||
const callTool = mock(async () => {
|
||||
throw new Error("401 Unauthorized")
|
||||
})
|
||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
||||
|
||||
// when / then
|
||||
await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized")
|
||||
expect(refresh).toHaveBeenCalledTimes(1)
|
||||
expect(callTool).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it("only attempts one refresh when the retried operation returns 401 again", async () => {
|
||||
// given
|
||||
const { SkillMcpManager } = await importFreshManagerModule()
|
||||
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
||||
const manager = new SkillMcpManager({
|
||||
createOAuthProvider: () => ({
|
||||
tokens: () => ({ accessToken: "stale-token", refreshToken: "refresh-token" }),
|
||||
login: mock(async () => ({ accessToken: "login-token" } satisfies OAuthTokenData)),
|
||||
refresh,
|
||||
}),
|
||||
})
|
||||
const callTool = mock(async () => {
|
||||
throw new Error("401 Unauthorized")
|
||||
})
|
||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
||||
|
||||
// when / then
|
||||
await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized")
|
||||
expect(refresh).toHaveBeenCalledTimes(1)
|
||||
expect(callTool).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
@@ -65,6 +65,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "test-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {}
|
||||
|
||||
@@ -80,6 +81,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "my-mcp",
|
||||
skillName: "data-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {}
|
||||
|
||||
@@ -95,6 +97,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "custom-server",
|
||||
skillName: "custom-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {}
|
||||
|
||||
@@ -112,6 +115,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "http-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
type: "http",
|
||||
@@ -130,6 +134,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "sse-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
type: "sse",
|
||||
@@ -148,6 +153,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "inferred-http",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://example.com/mcp",
|
||||
@@ -165,6 +171,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "stdio-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
type: "stdio",
|
||||
@@ -184,6 +191,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "inferred-stdio",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
command: "node",
|
||||
@@ -202,6 +210,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "mixed-config",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
type: "stdio",
|
||||
@@ -224,6 +233,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "bad-url-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
type: "http",
|
||||
@@ -242,6 +252,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "http-error-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://nonexistent.example.com/mcp",
|
||||
@@ -259,6 +270,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "hint-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://nonexistent.example.com/mcp",
|
||||
@@ -276,6 +288,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "mock-test-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://example.com/mcp",
|
||||
@@ -302,6 +315,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "missing-command",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
type: "stdio",
|
||||
@@ -320,6 +334,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "test-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
command: "nonexistent-command-xyz",
|
||||
@@ -338,6 +353,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "test-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
command: "nonexistent-command",
|
||||
@@ -358,11 +374,13 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "server1",
|
||||
skillName: "skill1",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const session2Info: SkillMcpClientInfo = {
|
||||
serverName: "server1",
|
||||
skillName: "skill1",
|
||||
sessionID: "session-2",
|
||||
scope: "builtin",
|
||||
}
|
||||
|
||||
// when
|
||||
@@ -396,6 +414,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "signal-server",
|
||||
skillName: "signal-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://example.com/mcp",
|
||||
@@ -423,11 +442,12 @@ describe("SkillMcpManager", () => {
|
||||
describe("isConnected", () => {
|
||||
it("returns false for unconnected server", () => {
|
||||
// given
|
||||
const info: SkillMcpClientInfo = {
|
||||
serverName: "unknown",
|
||||
skillName: "test",
|
||||
sessionID: "session-1",
|
||||
}
|
||||
const info: SkillMcpClientInfo = {
|
||||
serverName: "$1",
|
||||
skillName: "$2",
|
||||
sessionID: "$3",
|
||||
scope: "builtin",
|
||||
}
|
||||
|
||||
// when / #then
|
||||
expect(manager.isConnected(info)).toBe(false)
|
||||
@@ -448,6 +468,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "test-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const configWithoutEnv: ClaudeCodeMcpServer = {
|
||||
command: "node",
|
||||
@@ -471,6 +492,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "test-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-2",
|
||||
scope: "builtin",
|
||||
}
|
||||
const configWithEnv: ClaudeCodeMcpServer = {
|
||||
command: "node",
|
||||
@@ -498,6 +520,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "auth-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://example.com/mcp",
|
||||
@@ -526,6 +549,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "no-auth-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://example.com/mcp",
|
||||
@@ -546,6 +570,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "retry-server",
|
||||
skillName: "retry-skill",
|
||||
sessionID: "session-retry-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const context: SkillMcpServerContext = {
|
||||
config: {
|
||||
@@ -584,6 +609,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "fail-server",
|
||||
skillName: "fail-skill",
|
||||
sessionID: "session-fail-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const context: SkillMcpServerContext = {
|
||||
config: {
|
||||
@@ -615,6 +641,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "error-server",
|
||||
skillName: "error-skill",
|
||||
sessionID: "session-error-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const context: SkillMcpServerContext = {
|
||||
config: {
|
||||
@@ -653,6 +680,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "oauth-server",
|
||||
skillName: "oauth-skill",
|
||||
sessionID: "session-oauth-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -679,6 +707,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "oauth-no-token",
|
||||
skillName: "oauth-skill",
|
||||
sessionID: "session-oauth-2",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -705,6 +734,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "oauth-with-headers",
|
||||
skillName: "oauth-skill",
|
||||
sessionID: "session-oauth-3",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -734,6 +764,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "oauth-refresh",
|
||||
skillName: "oauth-skill",
|
||||
sessionID: "session-oauth-refresh",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -766,6 +797,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "oauth-refresh-fallback",
|
||||
skillName: "oauth-skill",
|
||||
sessionID: "session-oauth-refresh-fallback",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -799,6 +831,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "no-oauth-server",
|
||||
skillName: "test-skill",
|
||||
sessionID: "session-no-oauth",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -824,6 +857,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "stepup-server",
|
||||
skillName: "stepup-skill",
|
||||
sessionID: "session-stepup-1",
|
||||
scope: "builtin",
|
||||
}
|
||||
const config: ClaudeCodeMcpServer = {
|
||||
url: "https://mcp.example.com/mcp",
|
||||
@@ -869,6 +903,7 @@ describe("SkillMcpManager", () => {
|
||||
serverName: "no-stepup-server",
|
||||
skillName: "no-stepup-skill",
|
||||
sessionID: "session-no-stepup",
|
||||
scope: "builtin",
|
||||
}
|
||||
const context: SkillMcpServerContext = {
|
||||
config: {
|
||||
|
||||
@@ -4,7 +4,7 @@ import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import { McpOAuthProvider } from "../mcp-oauth/provider"
|
||||
import { disconnectAll, disconnectSession, forceReconnect } from "./cleanup"
|
||||
import { getOrCreateClient, getOrCreateClientWithRetryImpl } from "./connection"
|
||||
import { handleStepUpIfNeeded } from "./oauth-handler"
|
||||
import { handlePostRequestAuthError, handleStepUpIfNeeded } from "./oauth-handler"
|
||||
import type {
|
||||
OAuthProviderFactory,
|
||||
SkillMcpClientInfo,
|
||||
@@ -110,6 +110,7 @@ export class SkillMcpManager {
|
||||
): Promise<T> {
|
||||
const maxRetries = 3
|
||||
let lastError: Error | null = null
|
||||
const refreshAttempted = new Set<string>()
|
||||
|
||||
for (let attempt = 1; attempt <= maxRetries; attempt++) {
|
||||
try {
|
||||
@@ -130,6 +131,17 @@ export class SkillMcpManager {
|
||||
continue
|
||||
}
|
||||
|
||||
const postRequestRefreshHandled = await handlePostRequestAuthError({
|
||||
error: lastError,
|
||||
config,
|
||||
authProviders: this.state.authProviders,
|
||||
createOAuthProvider: this.state.createOAuthProvider,
|
||||
refreshAttempted,
|
||||
})
|
||||
if (postRequestRefreshHandled) {
|
||||
continue
|
||||
}
|
||||
|
||||
if (!errorMessage.includes("not connected")) {
|
||||
throw lastError
|
||||
}
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
import { describe, expect, it, mock } from "bun:test"
|
||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import type { OAuthTokenData } from "../mcp-oauth/storage"
|
||||
import type { OAuthProviderFactory, OAuthProviderLike } from "./types"
|
||||
|
||||
type OAuthHandlerModule = typeof import("./oauth-handler")
|
||||
|
||||
async function importFreshOAuthHandlerModule(): Promise<OAuthHandlerModule> {
|
||||
mock.module("../mcp-oauth/provider", () => ({
|
||||
McpOAuthProvider: class MockMcpOAuthProvider {},
|
||||
}))
|
||||
|
||||
return await import(new URL(`./oauth-handler.ts?oauth-handler-test=${Date.now()}-${Math.random()}`, import.meta.url).href)
|
||||
}
|
||||
|
||||
type Deferred<TValue> = {
|
||||
promise: Promise<TValue>
|
||||
resolve: (value: TValue) => void
|
||||
}
|
||||
|
||||
function createDeferred<TValue>(): Deferred<TValue> {
|
||||
let resolvePromise: ((value: TValue) => void) | null = null
|
||||
const promise = new Promise<TValue>((resolve) => {
|
||||
resolvePromise = resolve
|
||||
})
|
||||
|
||||
if (!resolvePromise) {
|
||||
throw new Error("Failed to create deferred promise")
|
||||
}
|
||||
|
||||
return { promise, resolve: resolvePromise }
|
||||
}
|
||||
|
||||
function createConfig(serverUrl: string): ClaudeCodeMcpServer {
|
||||
return {
|
||||
url: serverUrl,
|
||||
oauth: {
|
||||
clientId: "test-client",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
describe("oauth-handler refresh mutex wiring", () => {
|
||||
it("deduplicates concurrent pre-request refresh attempts for the same server", async () => {
|
||||
// given
|
||||
const { buildHttpRequestInit } = await importFreshOAuthHandlerModule()
|
||||
const deferred = createDeferred<OAuthTokenData>()
|
||||
const refresh = mock(() => deferred.promise)
|
||||
const provider: OAuthProviderLike = {
|
||||
tokens: () => ({
|
||||
accessToken: "expired-token",
|
||||
refreshToken: "refresh-token",
|
||||
expiresAt: Math.floor(Date.now() / 1000) - 60,
|
||||
}),
|
||||
login: mock(async () => ({ accessToken: "login-token" } satisfies OAuthTokenData)),
|
||||
refresh,
|
||||
}
|
||||
const authProviders = new Map<string, OAuthProviderLike>()
|
||||
const createOAuthProvider: OAuthProviderFactory = () => provider
|
||||
|
||||
// when
|
||||
const firstRequest = buildHttpRequestInit(createConfig("https://same.example.com/mcp"), authProviders, createOAuthProvider)
|
||||
const secondRequest = buildHttpRequestInit(createConfig("https://same.example.com/mcp"), authProviders, createOAuthProvider)
|
||||
|
||||
// then
|
||||
expect(refresh).toHaveBeenCalledTimes(1)
|
||||
deferred.resolve({ accessToken: "refreshed-token" })
|
||||
await expect(firstRequest).resolves.toEqual({ headers: { Authorization: "Bearer refreshed-token" } })
|
||||
await expect(secondRequest).resolves.toEqual({ headers: { Authorization: "Bearer refreshed-token" } })
|
||||
})
|
||||
|
||||
it("allows different servers to refresh independently after request auth errors", async () => {
|
||||
// given
|
||||
const { handlePostRequestAuthError } = await importFreshOAuthHandlerModule()
|
||||
const firstDeferred = createDeferred<OAuthTokenData>()
|
||||
const secondDeferred = createDeferred<OAuthTokenData>()
|
||||
const firstProvider: OAuthProviderLike = {
|
||||
tokens: () => ({ accessToken: "expired-a", refreshToken: "refresh-a" }),
|
||||
login: mock(async () => ({ accessToken: "login-a" } satisfies OAuthTokenData)),
|
||||
refresh: mock(() => firstDeferred.promise),
|
||||
}
|
||||
const secondProvider: OAuthProviderLike = {
|
||||
tokens: () => ({ accessToken: "expired-b", refreshToken: "refresh-b" }),
|
||||
login: mock(async () => ({ accessToken: "login-b" } satisfies OAuthTokenData)),
|
||||
refresh: mock(() => secondDeferred.promise),
|
||||
}
|
||||
const providers = new Map([
|
||||
["https://server-a.example.com/mcp", firstProvider],
|
||||
["https://server-b.example.com/mcp", secondProvider],
|
||||
])
|
||||
|
||||
// when
|
||||
const firstAttempt = handlePostRequestAuthError({
|
||||
error: new Error("401 Unauthorized"),
|
||||
config: createConfig("https://server-a.example.com/mcp"),
|
||||
authProviders: providers,
|
||||
})
|
||||
const secondAttempt = handlePostRequestAuthError({
|
||||
error: new Error("403 Forbidden"),
|
||||
config: createConfig("https://server-b.example.com/mcp"),
|
||||
authProviders: providers,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(firstProvider.refresh).toHaveBeenCalledTimes(1)
|
||||
expect(secondProvider.refresh).toHaveBeenCalledTimes(1)
|
||||
firstDeferred.resolve({ accessToken: "refreshed-a" })
|
||||
secondDeferred.resolve({ accessToken: "refreshed-b" })
|
||||
await expect(firstAttempt).resolves.toBe(true)
|
||||
await expect(secondAttempt).resolves.toBe(true)
|
||||
})
|
||||
|
||||
it("allows a new refresh after the previous same-server refresh completes", async () => {
|
||||
// given
|
||||
const { handlePostRequestAuthError } = await importFreshOAuthHandlerModule()
|
||||
const refresh = mock(async () => ({ accessToken: `refreshed-${refresh.mock.calls.length + 1}` } satisfies OAuthTokenData))
|
||||
const provider: OAuthProviderLike = {
|
||||
tokens: () => ({ accessToken: "expired-token", refreshToken: "refresh-token" }),
|
||||
login: mock(async () => ({ accessToken: "login-token" } satisfies OAuthTokenData)),
|
||||
refresh,
|
||||
}
|
||||
const authProviders = new Map<string, OAuthProviderLike>([["https://same.example.com/mcp", provider]])
|
||||
|
||||
// when
|
||||
const firstResult = await handlePostRequestAuthError({
|
||||
error: new Error("401 Unauthorized"),
|
||||
config: createConfig("https://same.example.com/mcp"),
|
||||
authProviders,
|
||||
})
|
||||
const secondResult = await handlePostRequestAuthError({
|
||||
error: new Error("401 Unauthorized"),
|
||||
config: createConfig("https://same.example.com/mcp"),
|
||||
authProviders,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(firstResult).toBe(true)
|
||||
expect(secondResult).toBe(true)
|
||||
expect(refresh).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import { McpOAuthProvider } from "../mcp-oauth/provider"
|
||||
import { withRefreshMutex } from "../mcp-oauth/refresh-mutex"
|
||||
import type { OAuthTokenData } from "../mcp-oauth/storage"
|
||||
import { isStepUpRequired, mergeScopes } from "../mcp-oauth/step-up"
|
||||
import type { OAuthProviderFactory, OAuthProviderLike } from "./types"
|
||||
@@ -52,14 +53,15 @@ export async function buildHttpRequestInit(
|
||||
}
|
||||
}
|
||||
|
||||
if (tokenData && isTokenExpired(tokenData)) {
|
||||
try {
|
||||
tokenData = tokenData.refreshToken
|
||||
? await provider.refresh(tokenData.refreshToken)
|
||||
: await provider.login()
|
||||
} catch {
|
||||
if (tokenData && isTokenExpired(tokenData)) {
|
||||
try {
|
||||
tokenData = await provider.login()
|
||||
const refreshToken = tokenData.refreshToken
|
||||
tokenData = refreshToken
|
||||
? await withRefreshMutex(config.url, () => provider.refresh(refreshToken))
|
||||
: await provider.login()
|
||||
} catch {
|
||||
try {
|
||||
tokenData = await provider.login()
|
||||
} catch {
|
||||
tokenData = null
|
||||
}
|
||||
@@ -116,3 +118,43 @@ export async function handleStepUpIfNeeded(params: {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
export async function handlePostRequestAuthError(params: {
|
||||
error: Error
|
||||
config: ClaudeCodeMcpServer
|
||||
authProviders: Map<string, OAuthProviderLike>
|
||||
createOAuthProvider?: OAuthProviderFactory
|
||||
refreshAttempted?: Set<string>
|
||||
}): Promise<boolean> {
|
||||
const { error, config, authProviders, createOAuthProvider, refreshAttempted = new Set() } = params
|
||||
|
||||
if (!config.oauth || !config.url) {
|
||||
return false
|
||||
}
|
||||
|
||||
const statusMatch = /\b(401|403)\b/.exec(error.message)
|
||||
if (!statusMatch) {
|
||||
return false
|
||||
}
|
||||
|
||||
const provider = getOrCreateAuthProvider(authProviders, config.url, config.oauth, createOAuthProvider)
|
||||
const tokenData = provider.tokens()
|
||||
|
||||
if (!tokenData?.refreshToken) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (refreshAttempted.has(config.url)) {
|
||||
return false
|
||||
}
|
||||
|
||||
refreshAttempted.add(config.url)
|
||||
|
||||
try {
|
||||
const refreshToken = tokenData.refreshToken
|
||||
await withRefreshMutex(config.url, () => provider.refresh(refreshToken))
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"
|
||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import { createCleanMcpEnvironment } from "./env-cleaner"
|
||||
import { registerProcessCleanup, startCleanupTimer } from "./cleanup"
|
||||
import { redactSensitiveData } from "./error-redaction"
|
||||
import type { ManagedClient, SkillMcpClientConnectionParams } from "./types"
|
||||
|
||||
function getStdioCommand(config: ClaudeCodeMcpServer, serverName: string): string {
|
||||
@@ -45,10 +46,13 @@ export async function createStdioClient(params: SkillMcpClientConnectionParams):
|
||||
}
|
||||
|
||||
const errorMessage = error instanceof Error ? error.message : String(error)
|
||||
const fullCommand = `${command} ${args.join(" ")}`
|
||||
const safeCommand = redactSensitiveData(fullCommand)
|
||||
const safeErrorMessage = redactSensitiveData(errorMessage)
|
||||
throw new Error(
|
||||
`Failed to connect to MCP server "${info.serverName}".\n\n` +
|
||||
`Command: ${command} ${args.join(" ")}\n` +
|
||||
`Reason: ${errorMessage}\n\n` +
|
||||
`Command: ${safeCommand}\n` +
|
||||
`Reason: ${safeErrorMessage}\n\n` +
|
||||
`Hints:\n` +
|
||||
` - Ensure the command is installed and available in PATH\n` +
|
||||
` - Check if the MCP server package exists\n` +
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdi
|
||||
import type { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"
|
||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||
import type { McpOAuthProvider } from "../mcp-oauth/provider"
|
||||
import type { SkillScope } from "../opencode-skill-loader/types"
|
||||
|
||||
export type SkillMcpConfig = Record<string, ClaudeCodeMcpServer>
|
||||
|
||||
@@ -10,6 +11,7 @@ export interface SkillMcpClientInfo {
|
||||
serverName: string
|
||||
skillName: string
|
||||
sessionID: string
|
||||
scope?: SkillScope | "local"
|
||||
}
|
||||
|
||||
export interface SkillMcpServerContext {
|
||||
|
||||
@@ -44,12 +44,22 @@ mock.module("./action-executor", () => ({
|
||||
mock.module("../../shared/tmux", () => ({
|
||||
isInsideTmux: mockIsInsideTmux,
|
||||
getCurrentPaneId: mockGetCurrentPaneId,
|
||||
isServerRunning: mock(async () => true),
|
||||
resetServerCheck: mock(() => {}),
|
||||
markServerRunningInProcess: mock(() => {}),
|
||||
getPaneDimensions: mock(async () => ({ width: 220, height: 44 })),
|
||||
spawnTmuxPane: mock(async () => ({ success: true, paneId: "%1" })),
|
||||
closeTmuxPane: mock(async () => ({ success: true })),
|
||||
replaceTmuxPane: mock(async () => ({ success: true, paneId: "%1" })),
|
||||
applyLayout: mock(async () => ({ success: true })),
|
||||
enforceMainPaneWidth: mock(async () => ({ success: true })),
|
||||
POLL_INTERVAL_BACKGROUND_MS: 10,
|
||||
SESSION_READY_POLL_INTERVAL_MS: 10,
|
||||
SESSION_READY_TIMEOUT_MS: 50,
|
||||
SESSION_MISSING_GRACE_MS: 1_000,
|
||||
spawnTmuxWindow: mockSpawnTmuxWindow,
|
||||
spawnTmuxSession: mockSpawnTmuxSession,
|
||||
SESSION_TIMEOUT_MS: 600_000,
|
||||
}))
|
||||
|
||||
afterAll(() => { mock.restore() })
|
||||
|
||||
Reference in New Issue
Block a user