From a8a9f8691ed5a9c6c84ff300b4f4a90f4495ac30 Mon Sep 17 00:00:00 2001 From: ranxueqing Date: Wed, 9 Sep 2026 11:50:31 +0800 Subject: [PATCH 1/2] fix(mcp): recover tool calls when the shim's conversation id goes stale after host resume Fixes #656. Claude Code resumes fork a NEW session id after MCP children are spawned, so the shim's env-captured CLAUDE_CODE_SESSION_ID goes stale and every tool call 404s with 'unknown plugin conversation' for the rest of the session while the proxy keeps serving the conversation under the new id. - mcp.ts: on that exact 404, adopt the proxy's most-recent active conversation (status?fallback=latest) and retry once. Identity-bound hosts only; BILI_MCP_NO_ORPHAN_ADOPT=1 opts out for multi-session setups. - plugin.ts: status fallback now reports the RESOLVED conversation id (it used to echo the caller's stale id back, so adoption was impossible). - plugin.ts: split the tool-endpoint 404 into its two real causes (never-registered id vs session-not-resident) and log the rejection - these 404s used to be invisible in bili.log. - tests: fallback resolution + adopted-id tool call; never-registered wording assertion. --- src/mcp.ts | 68 ++++++++++++++++++++++++++++------- src/plugin.ts | 36 +++++++++++++++++-- tests/plugin-protocol.test.ts | 34 ++++++++++++++++++ 3 files changed, 123 insertions(+), 15 deletions(-) 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..86dc9cf 100644 --- a/tests/plugin-protocol.test.ts +++ b/tests/plugin-protocol.test.ts @@ -405,6 +405,40 @@ 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(); } From 919283e44b8fe387288ebdc50e1f567546fbc574 Mon Sep 17 00:00:00 2001 From: ework-agent Date: Thu, 10 Sep 2026 00:26:35 +0800 Subject: [PATCH 2/2] test(mcp): cover the identity-binding resume path for #656 stale-id recovery The existing #656 test exercises the cooperative plugin protocol (x-bili-plugin-conversation). Add a parallel test that drives the actual Claude Code identity-binding path (x-claude-code-session-id + persistent identity register): a stale pre-resume id 404s, status?fallback=latest resolves the active conversation, and the adopted id then works. --- tests/plugin-protocol.test.ts | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/plugin-protocol.test.ts b/tests/plugin-protocol.test.ts index 86dc9cf..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 { @@ -444,6 +463,51 @@ test("#656: status fallback=latest resolves the active conversation for a stale } }); +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(); + } +}); + test("plugin mode: streamed response forwards verbatim while usage is sniffed into session stats", async () => { const h = await startHarness([textScript()]); try {