diff --git a/src/mcp.ts b/src/mcp.ts index 6ee9f91..8f91941 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -60,6 +60,14 @@ export function resolveProxyOrigin(): string { const TOOL_TIMEOUT_MS = 60_000; const CONVERSATION_FROM_ENV = process.env.CLAUDE_CODE_SESSION_ID?.trim() || process.env.BILI_CONVERSATION_ID?.trim() || undefined; const IDENTITY_BINDING = Boolean(process.env.CLAUDE_CODE_SESSION_ID?.trim()); +// #656: hosts that resume a session (claude --resume forks a NEW session id) +// do so after MCP children were spawned — the env-captured id goes stale and +// every tool call 404s forever. When that exact failure is seen, adopt the +// proxy's most-recent active conversation (status?fallback=latest) and retry +// once. Only armed for identity-bound hosts (claude code); opt out with +// BILI_MCP_NO_ORPHAN_ADOPT=1 when several host sessions share one proxy and +// the resumed one must not adopt a sibling's conversation. +const ORPHAN_ADOPT = IDENTITY_BINDING && process.env.BILI_MCP_NO_ORPHAN_ADOPT !== "1"; let manifestTools: McpToolDef[] = []; let conversationId = CONVERSATION_FROM_ENV; let registered = false; @@ -98,21 +106,55 @@ function ensureManifest(): Promise { } export async function forwardTool(tool: string, args: unknown, timeoutMs: number = TOOL_TIMEOUT_MS): Promise { - let res: Response; + for (let attempt = 0; ; attempt++) { + let res: Response; + try { + res = await fetch(`${resolveProxyOrigin()}/__bili/plugin/tool`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ conversationId, tool, args }), + signal: AbortSignal.timeout(timeoutMs), + }); + } catch (err) { + if (err instanceof Error && err.name === "TimeoutError") throw new Error(`tool forward timed out after ${timeoutMs}ms: ${tool}`); + throw err; + } + const data = (await res.json()) as { ok?: boolean; result?: string; error?: string }; + if (res.ok && data.ok) return data.result ?? ""; + // #656: the shim's captured id was never registered — the host likely + // resumed its session and forked a new id after this shim spawned. + // Adopt the proxy's latest active conversation and retry once. + if ( + res.status === 404 && attempt === 0 && ORPHAN_ADOPT && conversationId && + typeof data.error === "string" && data.error.includes("no model request has arrived") + ) { + if (await adoptLatestActiveConversation()) continue; + } + throw new Error(data.error ?? `tool forward failed: ${res.status}`); + } +} + +/** One-shot recovery for a stale shim id (#656): resolve the proxy's + * most-recent ACTIVE conversation via the status endpoint's fallback=latest + * and adopt it for all subsequent tool calls. Returns true when an adoption + * happened (caller should retry the tool call). */ +async function adoptLatestActiveConversation(): Promise { try { - res = await fetch(`${resolveProxyOrigin()}/__bili/plugin/tool`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ conversationId, tool, args }), - signal: AbortSignal.timeout(timeoutMs), - }); - } catch (err) { - if (err instanceof Error && err.name === "TimeoutError") throw new Error(`tool forward timed out after ${timeoutMs}ms: ${tool}`); - throw err; + const res = await fetch( + `${resolveProxyOrigin()}/__bili/plugin/status?conversationId=${encodeURIComponent(conversationId ?? "")}&fallback=latest`, + { signal: AbortSignal.timeout(5000) }, + ); + if (!res.ok) return false; + const data = (await res.json()) as { ok?: boolean; conversationId?: string; fallback?: boolean }; + if (data.ok !== true || data.fallback !== true || !data.conversationId || data.conversationId === conversationId) return false; + process.stderr.write( + `[bili-mcp] conversation id no longer known by the proxy (host resumed its session?); adopting latest active conversation ${data.conversationId}\n`, + ); + conversationId = data.conversationId; + return true; + } catch { + return false; } - const data = (await res.json()) as { ok?: boolean; result?: string; error?: string }; - if (!res.ok || !data.ok) throw new Error(data.error ?? `tool forward failed: ${res.status}`); - return data.result ?? ""; } const ERR_TOOL = -32602; diff --git a/src/plugin.ts b/src/plugin.ts index d83ede5..8a62ba6 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -359,12 +359,31 @@ export type PluginToolDeps = { log: (level: string, msg: string) => void; }; +/** Reverse-lookup the conversation id bound to a session id. #656: the + * status endpoint's fallback branch picks the latest active SESSION, but a + * caller that needs to ADOPT it (an MCP shim whose captured conversation id + * went stale after the host resumed) must be told the session's conversation + * id, not have its own stale id echoed back. Most-recently-seen binding wins + * when several conversations share one session. */ +function conversationIdForSession(sessionId: string): string | undefined { + let bestId: string | undefined; + let bestSeen = -Infinity; + for (const [cid, entry] of conversations) { + if (entry.sessionId === sessionId && entry.lastSeen > bestSeen) { + bestId = cid; + bestSeen = entry.lastSeen; + } + } + return bestId; +} + /** Context-level visibility for plugin UIs (status bars / slash commands): * the same usage the nudge decision sees, keyed by conversation id. */ export function handlePluginStatus(conversationId: string, res: import("node:http").ServerResponse, deps: PluginToolDeps, fallbackLatest = false): void { let entry = conversations.get(conversationId); let session = entry ? peekSession(entry.sessionId) : undefined; let viaFallback = false; + let resolvedConversationId = conversationId; if ((!entry || !session) && fallbackLatest) { // #404: only sessions with real activity in THIS process qualify. // Before the fix every boot-restored session carried lastSeen = @@ -376,6 +395,9 @@ export function handlePluginStatus(conversationId: string, res: import("node:htt if (latest) { session = latest; viaFallback = true; + // #656: name the conversation that was actually resolved — the + // caller asked with a stale id and must learn the real one. + resolvedConversationId = conversationIdForSession(latest.id) ?? conversationId; } else { res.writeHead(404, { "content-type": "application/json" }); res.end(JSON.stringify({ ok: false, error: "no session with activity since boot — issue a model request or pass the conversation id" })); @@ -441,7 +463,7 @@ export function handlePluginStatus(conversationId: string, res: import("node:htt res.writeHead(200, { "content-type": "application/json" }); res.end(JSON.stringify({ ok: true, - conversationId, + conversationId: resolvedConversationId, fallback: viaFallback || undefined, label: session.meta.label ?? null, pluginAgent: session.metadata.pluginAgent ?? null, @@ -481,8 +503,18 @@ export async function handlePluginTool( const entry = conversations.get(conversationId); const session = entry ? peekSession(entry.sessionId) : undefined; if (!entry || !session) { + // #656: two distinct failures shared one message before. An id that was + // NEVER registered is the classic stale-shim-id case (host resumed its + // session after the MCP shim captured CLAUDE_CODE_SESSION_ID) — say so, + // and log it: these 404s used to be invisible in bili.log. + deps.log("warn", `[plugin] tool "${tool}" rejected for conversation ${conversationId}: ${entry ? "id registered but session not resident in this proxy instance" : "id never registered (stale shim session id after host resume?)"}`); res.writeHead(404, { "content-type": "application/json" }); - res.end(JSON.stringify({ ok: false, error: "unknown plugin conversation (no model request has arrived with this conversation id yet)" })); + res.end(JSON.stringify({ + ok: false, + error: !entry + ? "unknown plugin conversation (no model request has arrived with this conversation id yet)" + : "unknown plugin conversation (id registered but its session is not resident in this proxy instance — a fresh model request re-binds it)", + })); return; } // Absorb enablement is per-session (last resolved config), so the gate diff --git a/tests/plugin-protocol.test.ts b/tests/plugin-protocol.test.ts index e304a4b..ac352cb 100644 --- a/tests/plugin-protocol.test.ts +++ b/tests/plugin-protocol.test.ts @@ -186,6 +186,25 @@ async function callPluginAnthropic( return { raw, events: parseAnthropicSse(raw), json: undefined }; } +async function claudeIdentityRequest(h: Harness, sessionId: string, messages: AnthropicMessage[]): Promise { + const resp = await fetch(`http://127.0.0.1:${h.proxyPort}/bili/http://127.0.0.1:${h.upstreamPort}/v1/messages`, { + method: "POST", + headers: { + "content-type": "application/json", + "x-claude-code-session-id": sessionId, + }, + body: JSON.stringify({ + model: "claude-test", + max_tokens: 1024, + stream: true, + system: UPSTREAM_SYSTEM, + messages, + }), + }); + if (resp.body) for await (const _ of resp.body) {} + return resp.status; +} + test("plugin manifest serves the exact wire tool schemas, headers and version", async () => { const h = await startHarness([textScript()]); try { @@ -405,6 +424,85 @@ test("plugin tool API error paths: bad JSON, unknown tool, unknown conversation" const errJson = (await unknownConv.json()) as { ok: boolean; error: string }; assert.equal(errJson.ok, false); assert.match(errJson.error, /unknown plugin conversation/i); + // #656: a never-registered id must say so explicitly (stale shim id + // after host resume) — not the generic wording. + assert.match(errJson.error, /no model request has arrived/); + } finally { + await h.close(); + } +}); + +test("#656: status fallback=latest resolves the active conversation for a stale shim id, and the adopted id then works for tool calls", async () => { + const h = await startHarness([textScript()]); + try { + const conv = "plug-conv-after-resume"; + // The host resumed and now sends traffic under a NEW id; the shim + // still holds the pre-resume one ("stale-shim-id"). + await callPluginAnthropic(h, conv, [{ role: "user", content: "hello after resume" }]); + + // Shim adoption step 1: status asked with the stale id falls back to + // the latest active conversation and reports the adoption. + const statusResp = await fetch(`http://127.0.0.1:${h.proxyPort}/__bili/plugin/status?conversationId=stale-shim-id&fallback=latest`); + assert.equal(statusResp.status, 200); + const status = (await statusResp.json()) as { ok: boolean; conversationId: string; fallback?: boolean }; + assert.equal(status.ok, true); + assert.equal(status.fallback, true); + assert.equal(status.conversationId, conv); + + // Shim adoption step 2: the retry with the adopted id succeeds. + const adopted = await fetch(`http://127.0.0.1:${h.proxyPort}/__bili/plugin/tool`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ conversationId: conv, tool: "acp_status", args: {} }), + }); + assert.equal(adopted.status, 200); + const adoptedJson = (await adopted.json()) as { ok: boolean }; + assert.equal(adoptedJson.ok, true); + } finally { + await h.close(); + } +}); + +test("#656 identity binding: a stale x-claude-code-session-id recovers via status fallback=latest", async () => { + const h = await startHarness([textScript()]); + try { + const OLD = "claude-session-before-resume"; + const NEW = "claude-session-after-resume"; + // Identity binding (#162): the shell registers its captured id; after + // a resume the host forks NEW and the shell re-registers it, leaving + // the stale OLD id with no model traffic. + queuePluginRegister(OLD, "mcp", true); + queuePluginRegister(NEW, "mcp", true); + assert.equal(await claudeIdentityRequest(h, NEW, [{ role: "user", content: "after resume" }]), 200); + + const toolOld = await fetch(`http://127.0.0.1:${h.proxyPort}/__bili/plugin/tool`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ conversationId: OLD, tool: "acp_status", args: {} }), + }); + assert.equal(toolOld.status, 404); + const toolOldJson = (await toolOld.json()) as { ok: boolean; error: string }; + assert.equal(toolOldJson.ok, false); + assert.match(toolOldJson.error, /no model request has arrived/); + + // Adoption step 1: status with the stale id falls back to the latest + // active conversation and reports NEW (the resolved id), not the stale one. + const statusResp = await fetch(`http://127.0.0.1:${h.proxyPort}/__bili/plugin/status?conversationId=${OLD}&fallback=latest`); + assert.equal(statusResp.status, 200); + const status = (await statusResp.json()) as { ok: boolean; conversationId: string; fallback?: boolean }; + assert.equal(status.ok, true); + assert.equal(status.fallback, true); + assert.equal(status.conversationId, NEW); + + // Adoption step 2: the retry with the adopted id succeeds. + const toolNew = await fetch(`http://127.0.0.1:${h.proxyPort}/__bili/plugin/tool`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ conversationId: NEW, tool: "acp_status", args: {} }), + }); + assert.equal(toolNew.status, 200); + const toolNewJson = (await toolNew.json()) as { ok: boolean }; + assert.equal(toolNewJson.ok, true); } finally { await h.close(); }