From 3ceed51d479f664955d2894d0f0a6322c560d45f Mon Sep 17 00:00:00 2001 From: liuedcson <332840681+liuedcson@users.noreply.github.com> Date: Wed, 23 Sep 2026 21:00:07 +0800 Subject: [PATCH] fix: bill Step Plan search_web to the plan endpoint Step Plan logins sent `search_web` to `https://api.stepfun.com/v1/mcp/web_search/mcp`, the open-platform path, which charges a pay-as-you-go API account instead of the Step Plan credit pool. The credential itself resolved correctly, so the search succeeded and the charge landed on the wrong account without any error. - Give each login profile its own endpoint: `step_plan` and `step_plan_oversea` use `/step_plan/v1/mcp/web_search/mcp`, `platform_cn` and `platform_oversea` keep `/v1/mcp/web_search/mcp`. The table is a `Record`, so a new profile is a compile error instead of silently inheriting another account's billing path. - An origin-only `STEPCODE_SEARCH_WEB_MCP_URL` now inherits the profile's path instead of a hardcoded `/v1`. That was the same bug by a second route: redirecting only the host moved a plan login onto the platform path. - Credentials with no profile (`--api-key`, `STEPCODE_SEARCH_API_KEY`, a hand-written `auth.json`) stay on the platform endpoint. A plan credential cannot reach that branch: it only comes from `/login`, which always records a profile, and a plan login from before profiles existed still resolves through the legacy `step` mapping in `readStepLoginProfile`. - Search failures now name the endpoint that was used, so a profile/endpoint mismatch surfaces in the error instead of on a bill. Checked with a mainland Step Plan key: the `initialize` handshake (which bills nothing) returns HTTP 402 `quota_exceeded` on `/v1` and HTTP 200 on `/step_plan/v1`, and an end-to-end `search_web` call now goes to `/step_plan/v1` and returns results. --- .../coding-agent/docs/step-integration.md | 36 +++++---- .../coding-agent/src/step/search-web-tool.ts | 77 ++++++++++++++----- .../coding-agent/test/step-search-web.test.ts | 67 ++++++++++++++-- 3 files changed, 141 insertions(+), 39 deletions(-) diff --git a/packages/coding-agent/docs/step-integration.md b/packages/coding-agent/docs/step-integration.md index f836c32f..99a396c7 100644 --- a/packages/coding-agent/docs/step-integration.md +++ b/packages/coding-agent/docs/step-integration.md @@ -97,20 +97,28 @@ profile. The Step tool profile also registers `search_web`, backed by the remote `stepsearch.web_search` Streamable HTTP MCP tool. The search credential is resolved from an explicit `step --api-key`, then `STEPCODE_SEARCH_API_KEY`, -`STEPCODE_SEARCH_API_KEY`, then the Step login entry in `auth.json`; it is sent - only as a Bearer header. The `step_plan_oversea` and `platform_oversea` - login profiles use `https://api.stepfun.ai/v1/mcp/web_search/mcp`; - `step_plan` and `platform_cn` use - `https://api.stepfun.com/v1/mcp/web_search/mcp`. An unrecognized profile - falls back to that same mainland endpoint. - `STEPCODE_SEARCH_WEB_MCP_URL` and `STEPCODE_SEARCH_WEB_MCP_URL` override this - profile-based selection. This adapter does not add a separate search - credential store or `integrations.search` settings surface. The - `STEP_API_KEY` environment variable is deliberately excluded from that - chain: StepCode injects it together with `STEP_BASE_URL` to reach its own - model gateway, and because the search endpoint never follows that base URL, - reusing the value would authenticate a gateway key against - `api.stepfun.com` and fail. +then the Step login entry in `auth.json`; it is sent only as a Bearer header. +Each login profile has its own endpoint, because the endpoint decides which +account the search is billed to: `step_plan` uses +`https://api.stepfun.com/step_plan/v1/mcp/web_search/mcp` and +`step_plan_oversea` uses `https://api.stepfun.ai/step_plan/v1/mcp/web_search/mcp`, +both billed to the Step Plan quota; `platform_cn` uses +`https://api.stepfun.com/v1/mcp/web_search/mcp` and `platform_oversea` uses +`https://api.stepfun.ai/v1/mcp/web_search/mcp`, both billed to the +pay-as-you-go API account. A credential with no profile falls back to the +mainland platform endpoint: a profile is absent only when the credential came +from `--api-key`, `STEPCODE_SEARCH_API_KEY`, or a hand-written `auth.json`, and +a Step Plan credential is never one of those, since it only comes from `/login`, +which always records a profile. `STEPCODE_SEARCH_WEB_MCP_URL` overrides this +profile-based selection; an override that supplies only an origin keeps the path +of the profile's own endpoint, so redirecting the host cannot move a plan user's +searches onto the billed platform path. This adapter does not add a separate +search credential store or `integrations.search` settings surface. The +`STEP_API_KEY` environment variable is deliberately excluded from that +chain: StepCode injects it together with `STEP_BASE_URL` to reach its own +model gateway, and because the search endpoint never follows that base URL, +reusing the value would authenticate a gateway key against +`api.stepfun.com` and fail. Session storage is also selected through the Step wrapper. The wrapper keeps Pi's `SessionManager` class, JSONL format, and tree operations unchanged, but diff --git a/packages/coding-agent/src/step/search-web-tool.ts b/packages/coding-agent/src/step/search-web-tool.ts index 3236ce8e..2bd19661 100644 --- a/packages/coding-agent/src/step/search-web-tool.ts +++ b/packages/coding-agent/src/step/search-web-tool.ts @@ -8,6 +8,7 @@ import { STEP_PROVIDER_ID } from "../features/step-provider/index.ts"; import { resolveStepAgentDir } from "./environment.ts"; import { readStepLoginProfile } from "./login-flow.ts"; import { invokeRemoteMcpTool, type RemoteMcpToolInvocation, type RemoteMcpToolResult } from "./mcp-client.ts"; +import type { StepLoginProfileId } from "./onboarding.ts"; export { invokeRemoteMcpTool } from "./mcp-client.ts"; @@ -15,10 +16,35 @@ export const SEARCH_WEB_SERVER_NAME = "stepsearch"; export const SEARCH_WEB_TOOL_NAME = "web_search"; export const SEARCH_WEB_MAINLAND_URL = "https://api.stepfun.com/v1/mcp/web_search/mcp"; export const SEARCH_WEB_OVERSEA_URL = "https://api.stepfun.ai/v1/mcp/web_search/mcp"; -/** Unrecognized login profiles fall back to the mainland endpoint. */ +export const SEARCH_WEB_PLAN_MAINLAND_URL = "https://api.stepfun.com/step_plan/v1/mcp/web_search/mcp"; +export const SEARCH_WEB_PLAN_OVERSEA_URL = "https://api.stepfun.ai/step_plan/v1/mcp/web_search/mcp"; +/** + * Unrecognized login profiles fall back to the mainland platform endpoint. + * + * A profile is absent only when no login wrote one: an explicit `--api-key`, a + * `STEPCODE_SEARCH_API_KEY` environment key, or a hand-written `auth.json`. + * Those carry platform keys, because a Step Plan credential is only ever + * obtained through `/login`, which always records a profile — and a plan login + * from before profiles existed still resolves, through the legacy `"step"` + * mapping in `readStepLoginProfile`. So the fallback is not a guess about an + * unknown credential; it is the only kind of credential that can arrive here. + */ export const SEARCH_WEB_DEFAULT_URL = SEARCH_WEB_MAINLAND_URL; -const SEARCH_WEB_MCP_PATH = "/v1/mcp/web_search/mcp"; +/** + * One endpoint per login profile, mirroring the `baseUrl` split in + * `./onboarding.ts`: the plan profiles bill the user's Step Plan quota through + * `/step_plan/v1`, the platform profiles bill a pay-as-you-go API + * account through `/v1`. Plan and platform must never share a row — sending a + * plan credential to the platform endpoint silently charges the API account. + */ +const SEARCH_WEB_PROFILE_URLS: Record = { + step_plan: SEARCH_WEB_PLAN_MAINLAND_URL, + step_plan_oversea: SEARCH_WEB_PLAN_OVERSEA_URL, + platform_cn: SEARCH_WEB_MAINLAND_URL, + platform_oversea: SEARCH_WEB_OVERSEA_URL, +}; + const SEARCH_WEB_RESULT_COUNT = 10; const MAX_SNIPPET_CHARS = 400; @@ -59,12 +85,7 @@ export function resolveSearchWebServerUrl( env: Record = process.env, profile?: string, ): string { - const profileDefault = - profile === "platform_oversea" || profile === "step_plan_oversea" - ? SEARCH_WEB_OVERSEA_URL - : profile === "step_plan" || profile === "platform_cn" - ? SEARCH_WEB_MAINLAND_URL - : SEARCH_WEB_DEFAULT_URL; + const profileDefault = resolveProfileSearchWebUrl(profile); const value = normalizeOptionalText(configured) ?? normalizeOptionalText(env.STEPCODE_SEARCH_WEB_MCP_URL) ?? profileDefault; @@ -76,7 +97,18 @@ export function resolveSearchWebServerUrl( } if (parsed.pathname && parsed.pathname !== "/") return value.replace(/\/+$/u, ""); - return `${parsed.origin}${SEARCH_WEB_MCP_PATH}`; + // An override that carries only an origin still has to be billed to the + // account the login belongs to, so it inherits the profile's own path. A + // hardcoded `/v1` path here would send a plan login's searches to the + // platform account whenever someone redirected the host alone. + return `${parsed.origin}${new URL(profileDefault).pathname}`; +} + +function resolveProfileSearchWebUrl(profile: string | undefined): string { + if (profile && Object.hasOwn(SEARCH_WEB_PROFILE_URLS, profile)) { + return SEARCH_WEB_PROFILE_URLS[profile as StepLoginProfileId]; + } + return SEARCH_WEB_DEFAULT_URL; } /** Resolve search credentials without ever returning a placeholder value. */ @@ -135,16 +167,25 @@ export function createSearchWebTool( ); } - const result = await invokeTool({ - serverName: SEARCH_WEB_SERVER_NAME, - serverUrl, - toolName: SEARCH_WEB_TOOL_NAME, - arguments: { query, n: SEARCH_WEB_RESULT_COUNT }, - headers: { Authorization: `Bearer ${apiKey}` }, - signal, - }); + // The endpoint decides which account the search is billed to, so every + // failure names it: a profile/endpoint mismatch is otherwise invisible + // until it shows up on a bill. The URL carries no credential. + let result: SearchWebMcpResult; + try { + result = await invokeTool({ + serverName: SEARCH_WEB_SERVER_NAME, + serverUrl, + toolName: SEARCH_WEB_TOOL_NAME, + arguments: { query, n: SEARCH_WEB_RESULT_COUNT }, + headers: { Authorization: `Bearer ${apiKey}` }, + signal, + }); + } catch (error) { + throw new Error(`${error instanceof Error ? error.message : String(error)} (endpoint ${serverUrl})`); + } if (result.isError) { - throw new Error(result.content || `MCP tool ${SEARCH_WEB_SERVER_NAME}.${SEARCH_WEB_TOOL_NAME} failed`); + const detail = result.content || `MCP tool ${SEARCH_WEB_SERVER_NAME}.${SEARCH_WEB_TOOL_NAME} failed`; + throw new Error(`${detail} (endpoint ${serverUrl})`); } return renderSearchResults(query, result); }, diff --git a/packages/coding-agent/test/step-search-web.test.ts b/packages/coding-agent/test/step-search-web.test.ts index ded15808..b14fe7c4 100644 --- a/packages/coding-agent/test/step-search-web.test.ts +++ b/packages/coding-agent/test/step-search-web.test.ts @@ -30,7 +30,7 @@ describe("Step search_web tool", () => { }), ).toBe("https://first.example.com/v1/mcp/web_search/mcp"); expect(resolveSearchWebServerUrl(undefined, {}, "step_plan")).toBe( - "https://api.stepfun.com/v1/mcp/web_search/mcp", + "https://api.stepfun.com/step_plan/v1/mcp/web_search/mcp", ); expect(resolveSearchWebServerUrl(undefined, {}, "platform_cn")).toBe( "https://api.stepfun.com/v1/mcp/web_search/mcp", @@ -39,11 +39,55 @@ describe("Step search_web tool", () => { "https://api.stepfun.ai/v1/mcp/web_search/mcp", ); expect(resolveSearchWebServerUrl(undefined, {}, "step_plan_oversea")).toBe( - "https://api.stepfun.ai/v1/mcp/web_search/mcp", + "https://api.stepfun.ai/step_plan/v1/mcp/web_search/mcp", ); expect(resolveSearchWebServerUrl(undefined, {}, "unknown")).toBe("https://api.stepfun.com/v1/mcp/web_search/mcp"); }); + it("completes an origin-only override with the path of the profile's own endpoint", () => { + // A host-only override must not silently move a plan login onto the + // platform path, which would bill the searches to the API account. + expect(resolveSearchWebServerUrl("https://proxy.example.com", {}, "step_plan")).toBe( + "https://proxy.example.com/step_plan/v1/mcp/web_search/mcp", + ); + expect( + resolveSearchWebServerUrl( + undefined, + { STEPCODE_SEARCH_WEB_MCP_URL: "https://proxy.example.com/" }, + "step_plan_oversea", + ), + ).toBe("https://proxy.example.com/step_plan/v1/mcp/web_search/mcp"); + expect(resolveSearchWebServerUrl("https://proxy.example.com", {}, "platform_cn")).toBe( + "https://proxy.example.com/v1/mcp/web_search/mcp", + ); + // A full override still wins outright, path included. + expect(resolveSearchWebServerUrl("https://proxy.example.com/custom/mcp", {}, "step_plan")).toBe( + "https://proxy.example.com/custom/mcp", + ); + }); + + it("keeps credentials that carry no login profile on the platform endpoint", async () => { + // `--api-key` and `STEPCODE_SEARCH_API_KEY` persist without a profile. + // Step Plan credentials cannot reach this path: they only come from + // `/login`, which always records one. Routing these to the plan endpoint + // would spend a stranger's plan quota on a platform key. + const root = await mkdtemp(join(tmpdir(), "step-search-web-no-profile-")); + const authPath = join(root, "auth.json"); + try { + await writeFile(authPath, JSON.stringify({ step: { type: "api_key", key: "stored-key" } }), "utf8"); + let serverUrl = ""; + const tool = createSearchWebTool({ env: {}, authPath }, async (input) => { + serverUrl = input.serverUrl; + return { structuredContent: { results: [] } }; + }); + + await tool.execute("no-profile-call", { query: "query" }, undefined, undefined, undefined as never); + expect(serverUrl).toBe("https://api.stepfun.com/v1/mcp/web_search/mcp"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + it("prefers search credentials, then auth.json, and ignores the model credential", async () => { const root = await mkdtemp(join(tmpdir(), "step-search-web-")); const authPath = join(root, "auth.json"); @@ -123,11 +167,20 @@ describe("Step search_web tool", () => { }); }); - it("selects the overseas endpoint from either persisted overseas login profile", async () => { + it("selects the endpoint of the persisted login profile and never bills plan searches to the platform", async () => { const root = await mkdtemp(join(tmpdir(), "step-search-web-profile-")); const authPath = join(root, "auth.json"); + const expectedByProfile = { + step_plan: "https://api.stepfun.com/step_plan/v1/mcp/web_search/mcp", + step_plan_oversea: "https://api.stepfun.ai/step_plan/v1/mcp/web_search/mcp", + platform_cn: "https://api.stepfun.com/v1/mcp/web_search/mcp", + platform_oversea: "https://api.stepfun.ai/v1/mcp/web_search/mcp", + // Logins written before profiles existed; `readStepLoginProfile` maps + // them to `step_plan`, so they are plan users and bill the plan pool. + step: "https://api.stepfun.com/step_plan/v1/mcp/web_search/mcp", + }; try { - for (const profile of ["platform_oversea", "step_plan_oversea"]) { + for (const [profile, expected] of Object.entries(expectedByProfile)) { await writeFile( authPath, JSON.stringify({ step: { type: "oauth", access: "search-key", profile } }), @@ -140,7 +193,7 @@ describe("Step search_web tool", () => { }); await tool.execute("profile-call", { query: "query" }, undefined, undefined, undefined as never); - expect(serverUrl).toBe("https://api.stepfun.ai/v1/mcp/web_search/mcp"); + expect(serverUrl, `profile ${profile}`).toBe(expected); } } finally { await rm(root, { recursive: true, force: true }); @@ -178,7 +231,7 @@ describe("Step search_web tool", () => { } }); - it("turns remote MCP errors into tool failures", async () => { + it("turns remote MCP errors into tool failures that name the billed endpoint", async () => { const tool = createSearchWebTool({ apiKey: "search-key" }, async () => ({ isError: true, content: "502 Bad Gateway", @@ -186,7 +239,7 @@ describe("Step search_web tool", () => { await expect( tool.execute("call-4", { query: "query" }, undefined, undefined, undefined as never), - ).rejects.toThrow("502 Bad Gateway"); + ).rejects.toThrow("502 Bad Gateway (endpoint https://api.stepfun.com/v1/mcp/web_search/mcp)"); }); it("preserves transport failures as diagnostic tool errors", async () => {