From 1f2f185acf8bd3bc0a8c5fd962337f8bb25e7700 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 11 Sep 2026 15:10:34 +0530 Subject: [PATCH] fix(server): correct tool hints flagged in marketplace review and make SDK setup output chat-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenAI marketplace review of v1.3.0 rejected on two points; this addresses both. Tool annotations: - startAccessibilityScan: openWorldHint false -> true. The scanner loads an arbitrary user-supplied public URL and, with a form auth config, submits a login form on that site — open-world by the reviewer's definition. - prepareSelfHealingPlan: readOnlyHint true -> false. The server writes nothing, but the tool exists to drive a code-edit workflow, so clients should confirm before invoking it. destructiveHint stays false. - Add tests/tools/tool-annotations.test.ts pinning both values. Web test case 5 (setupBrowserStackAutomateTests): - The setup banner now tells chat-only clients that cannot run commands to present every step, command and the full browserstack.yml verbatim instead of summarizing, so the reviewer sees the setup commands and configuration. - resolveVersion("latest") skipped nothing and picked "154.0 beta" for Chrome; it now prefers stable channels and only falls back to beta/dev when no stable version exists. Adds tests/lib/version-resolver.test.ts. Co-Authored-By: Claude Fable 5.1 --- src/lib/version-resolver.ts | 11 ++++- src/tools/accessibility.ts | 2 +- src/tools/sdk-utils/common/constants.ts | 3 +- src/tools/selfheal.ts | 2 +- tests/lib/version-resolver.test.ts | 28 +++++++++++++ tests/tools/tool-annotations.test.ts | 53 +++++++++++++++++++++++++ 6 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 tests/lib/version-resolver.test.ts create mode 100644 tests/tools/tool-annotations.test.ts diff --git a/src/lib/version-resolver.ts b/src/lib/version-resolver.ts index 14ca3cb0..654d73a3 100644 --- a/src/lib/version-resolver.ts +++ b/src/lib/version-resolver.ts @@ -3,14 +3,21 @@ * Else if exact match, returns that * Else picks the numerically closest (or first) */ +const PRERELEASE_CHANNEL = /\b(beta|dev|alpha|canary|nightly|preview)\b/i; + export function resolveVersion(requested: string, available: string[]): string { // strip duplicates & sort const uniq = Array.from(new Set(available)); // pick min/max if (requested === "latest" || requested === "oldest") { + // Prefer stable releases: BrowserStack lists pre-release channels such as + // "154.0 beta" / "155.0 dev" alongside stable versions, and "latest" + // should never resolve to one of those while a stable version exists. + const stable = uniq.filter((v) => !PRERELEASE_CHANNEL.test(v)); + const candidates = stable.length > 0 ? stable : uniq; // try numeric - const nums = uniq + const nums = candidates .map((v) => ({ v, n: parseFloat(v) })) .filter((x) => !isNaN(x.n)) .sort((a, b) => a.n - b.n); @@ -18,7 +25,7 @@ export function resolveVersion(requested: string, available: string[]): string { return requested === "latest" ? nums[nums.length - 1].v : nums[0].v; } // fallback lex - const lex = uniq.slice().sort(); + const lex = candidates.slice().sort(); return requested === "latest" ? lex[lex.length - 1] : lex[0]; } diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index 81b4e7b7..6621e7ad 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -472,7 +472,7 @@ export default function addAccessibilityTools( { title: "Start Accessibility Scan", readOnlyHint: false, - openWorldHint: false, + openWorldHint: true, destructiveHint: false, idempotentHint: false, }, diff --git a/src/tools/sdk-utils/common/constants.ts b/src/tools/sdk-utils/common/constants.ts index 8d97f80a..30a68f76 100644 --- a/src/tools/sdk-utils/common/constants.ts +++ b/src/tools/sdk-utils/common/constants.ts @@ -1,5 +1,6 @@ export const IMPORTANT_SETUP_WARNING = - "IMPORTANT: DO NOT SKIP ANY STEP. All the setup steps described below MUST be executed regardless of any existing configuration or setup. This ensures proper BrowserStack SDK setup."; + "IMPORTANT: DO NOT SKIP ANY STEP. All the setup steps described below MUST be executed regardless of any existing configuration or setup. This ensures proper BrowserStack SDK setup. " + + "If you cannot run commands or edit files in the user's project (e.g. a chat-only client), present every step below to the user in full — including each shell command, the package.json changes, and the complete browserstack.yml contents — instead of summarizing them."; export const SETUP_PERCY_DESCRIPTION = "Set up or expand Percy visual testing configuration with comprehensive coverage for existing projects that might have Percy integrated. This supports both Percy Web Standalone and Percy Automate. Example prompts: Expand percy coverage for this project {project_name}"; diff --git a/src/tools/selfheal.ts b/src/tools/selfheal.ts index 94169612..a58035e0 100644 --- a/src/tools/selfheal.ts +++ b/src/tools/selfheal.ts @@ -679,7 +679,7 @@ export default function addSelfHealTools( }, { title: "Prepare Self-Healing Plan", - readOnlyHint: true, + readOnlyHint: false, openWorldHint: false, destructiveHint: false, idempotentHint: true, diff --git a/tests/lib/version-resolver.test.ts b/tests/lib/version-resolver.test.ts new file mode 100644 index 00000000..04974ad6 --- /dev/null +++ b/tests/lib/version-resolver.test.ts @@ -0,0 +1,28 @@ +import { describe, it, expect } from "vitest"; +import { resolveVersion } from "../../src/lib/version-resolver"; + +describe("resolveVersion", () => { + const versions = ["152.0", "153.0", "154.0 beta", "155.0 dev"]; + + it("resolves 'latest' to the newest stable version, skipping beta/dev channels", () => { + expect(resolveVersion("latest", versions)).toBe("153.0"); + }); + + it("resolves 'oldest' to the oldest stable version", () => { + expect(resolveVersion("oldest", versions)).toBe("152.0"); + }); + + it("falls back to pre-release channels when no stable version exists", () => { + expect(resolveVersion("latest", ["154.0 beta", "155.0 dev"])).toBe( + "155.0 dev", + ); + }); + + it("still returns exact matches, including pre-release channels", () => { + expect(resolveVersion("154.0 beta", versions)).toBe("154.0 beta"); + }); + + it("matches by major version", () => { + expect(resolveVersion("152", versions)).toBe("152.0"); + }); +}); diff --git a/tests/tools/tool-annotations.test.ts b/tests/tools/tool-annotations.test.ts new file mode 100644 index 00000000..eebd7573 --- /dev/null +++ b/tests/tools/tool-annotations.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect, vi } from "vitest"; + +vi.mock("../../src/logger", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); +vi.mock("../../src/lib/instrumentation", () => ({ trackMCP: vi.fn() })); + +import addAccessibilityTools from "../../src/tools/accessibility"; +import addSelfHealTools from "../../src/tools/selfheal"; + +const mockConfig = { + "browserstack-username": "fake-user", + "browserstack-access-key": "fake-key", +}; + +// Captures the annotations object passed as the 4th argument to server.tool(). +function collectAnnotations(register: (server: any, config: any) => unknown) { + const annotations: Record = {}; + const serverMock = { + tool: vi.fn((...toolArgs: any[]) => { + annotations[toolArgs[0]] = toolArgs[3]; + }), + server: { + getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }), + getClientCapabilities: vi.fn().mockReturnValue({}), + elicitInput: vi.fn(), + }, + }; + register(serverMock, mockConfig); + return annotations; +} + +// These values were flagged in the OpenAI marketplace review of v1.3.0; the +// assertions pin the corrected hints so they cannot silently regress. +describe("tool annotations flagged in marketplace review", () => { + it("startAccessibilityScan is open-world: it loads an arbitrary public URL (and can submit login forms)", () => { + const annotations = collectAnnotations(addAccessibilityTools); + expect(annotations.startAccessibilityScan).toMatchObject({ + readOnlyHint: false, + openWorldHint: true, + destructiveHint: false, + }); + }); + + it("prepareSelfHealingPlan is not read-only: it drives a code-edit workflow", () => { + const annotations = collectAnnotations(addSelfHealTools); + expect(annotations.prepareSelfHealingPlan).toMatchObject({ + readOnlyHint: false, + openWorldHint: false, + destructiveHint: false, + }); + }); +});