From ff95d0c2694f64d68e9c28af8ef72113ee95079b Mon Sep 17 00:00:00 2001 From: Sash Zats Date: Fri, 17 Jul 2026 06:37:35 -0400 Subject: [PATCH] improve(ci): replace boundary guards with focused Oxlint rules (#95368) * perf(ci): move boundary guards to oxlint Co-authored-by: Sash Zats * fix(ci): avoid dead editor URL export Co-authored-by: Sash Zats * fix(ci): declare path-loaded boundary guard roots Co-authored-by: Sash Zats * fix(ci): model Oxlint plugin export in Knip Co-authored-by: Sash Zats --------- Co-authored-by: Peter Steinberger --- config/knip.all-exports.config.ts | 2 + config/knip.config.ts | 2 + config/knip.scripts-exports.config.ts | 2 + config/oxlint/boundary-guards.json | 10 +++ package.json | 4 +- scripts/check-no-raw-window-open.mjs | 83 ------------------- scripts/check-no-register-http-handler.mjs | 43 ---------- scripts/oxlint-boundary-guards.mjs | 66 +++++++++++++++ scripts/run-oxlint.mjs | 16 ++-- .../raw-window-open-violation.ts | 12 +++ .../register-http-handler-violation.ts | 7 ++ test/scripts/check-no-raw-window-open.test.ts | 40 --------- test/scripts/oxlint-boundary-guards.test.ts | 41 +++++++++ ui/src/components/app-sidebar-menus.ts | 7 +- ui/src/lib/editor-links.ts | 7 +- .../chat/components/chat-sidebar.test.ts | 59 +++++++------ ui/src/pages/chat/components/chat-sidebar.ts | 5 +- ui/src/pages/sessions/sessions-page.ts | 8 +- 18 files changed, 204 insertions(+), 210 deletions(-) create mode 100644 config/oxlint/boundary-guards.json delete mode 100644 scripts/check-no-raw-window-open.mjs delete mode 100644 scripts/check-no-register-http-handler.mjs create mode 100644 scripts/oxlint-boundary-guards.mjs create mode 100644 test/fixtures/oxlint-boundary-guards/raw-window-open-violation.ts create mode 100644 test/fixtures/oxlint-boundary-guards/register-http-handler-violation.ts delete mode 100644 test/scripts/check-no-raw-window-open.test.ts create mode 100644 test/scripts/oxlint-boundary-guards.test.ts diff --git a/config/knip.all-exports.config.ts b/config/knip.all-exports.config.ts index 915a67861896..71eed6689332 100644 --- a/config/knip.all-exports.config.ts +++ b/config/knip.all-exports.config.ts @@ -62,6 +62,8 @@ const ROOT_TEST_ENTRY_GLOBS = [ "test/helpers/config/bundled-channel-config-runtime.ts!", // The topology analyzer owns these as an intentionally self-contained graph. "test/fixtures/ts-topology/basic/**/*.{js,mjs,cjs,ts,mts,cts}!", + // The focused Oxlint test invokes these deliberate violations by path. + "test/fixtures/oxlint-boundary-guards/*.ts!", ] as const; const workspaces = Object.fromEntries( diff --git a/config/knip.config.ts b/config/knip.config.ts index 64e3630ff02c..20ec6574992c 100644 --- a/config/knip.config.ts +++ b/config/knip.config.ts @@ -63,6 +63,8 @@ const repositoryScriptEntries = [ "scripts/mcp-code-mode-gateway-e2e.ts!", "scripts/openclaw-release-clawhub-plan.ts!", "scripts/openclaw-release-clawhub-runtime-state.ts!", + // Oxlint loads this JS plugin by path from config/oxlint/boundary-guards.json. + "scripts/oxlint-boundary-guards.mjs!", "scripts/plugin-prerelease-liveish-matrix.mjs!", "scripts/pr-gates-lock.mjs!", "scripts/pr-lib/process-group-runner.mjs!", diff --git a/config/knip.scripts-exports.config.ts b/config/knip.scripts-exports.config.ts index 95302cff3459..265c6118578f 100644 --- a/config/knip.scripts-exports.config.ts +++ b/config/knip.scripts-exports.config.ts @@ -52,6 +52,8 @@ const config = { "enumMembers", "namespaceMembers", ], + // Oxlint consumes this required default export through a JSON config path. + "scripts/oxlint-boundary-guards.mjs": ["exports"], "scripts/repro/code-mode-namespace-live.ts": [ "exports", "nsExports", diff --git a/config/oxlint/boundary-guards.json b/config/oxlint/boundary-guards.json new file mode 100644 index 000000000000..dee7b04325a1 --- /dev/null +++ b/config/oxlint/boundary-guards.json @@ -0,0 +1,10 @@ +{ + "$schema": "../../node_modules/oxlint/configuration_schema.json", + "plugins": [], + "jsPlugins": ["../../scripts/oxlint-boundary-guards.mjs"], + "categories": { "correctness": "off" }, + "rules": { + "openclaw-boundaries/no-raw-window-open-call": "error", + "openclaw-boundaries/no-register-http-handler-call": "error" + } +} diff --git a/package.json b/package.json index 643026c3e967..bf6ab2b39615 100644 --- a/package.json +++ b/package.json @@ -1704,7 +1704,7 @@ "lint:plugins:no-extension-src-imports": "node --import tsx scripts/check-no-extension-src-imports.ts", "lint:plugins:no-extension-test-core-imports": "node --import tsx scripts/check-no-extension-test-core-imports.ts", "lint:plugins:no-monolithic-plugin-sdk-entry-imports": "node --import tsx scripts/check-no-monolithic-plugin-sdk-entry-imports.ts", - "lint:plugins:no-register-http-handler": "node scripts/check-no-register-http-handler.mjs", + "lint:plugins:no-register-http-handler": "node scripts/run-oxlint.mjs --openclaw-focused-config --config config/oxlint/boundary-guards.json src extensions", "lint:plugins:plugin-sdk-subpaths-exported": "node scripts/check-plugin-sdk-subpath-exports.mjs", "lint:scripts": "pnpm lint:docker-e2e && pnpm lint:tmp:no-raw-http2-imports && node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.scripts.json scripts", "lint:swift": "./scripts/lint-swift.sh", @@ -1719,7 +1719,7 @@ "lint:tmp:sqlite-transaction-boundary": "node scripts/check-sqlite-transaction-boundary.mjs", "lint:tmp:tsgo-core-boundary": "node scripts/check-tsgo-core-boundary.mjs", "lint:ui:i18n": "pnpm ui:i18n:verify", - "lint:ui:no-raw-window-open": "node scripts/check-no-raw-window-open.mjs", + "lint:ui:no-raw-window-open": "node scripts/run-oxlint.mjs --openclaw-focused-config --config config/oxlint/boundary-guards.json ui/src", "lint:web-fetch-provider-boundaries": "node scripts/check-web-fetch-provider-boundaries.mjs", "lint:web-search-provider-boundaries": "node scripts/check-web-search-provider-boundaries.mjs", "lint:webhook:no-low-level-body-read": "node scripts/check-webhook-auth-body-order.mjs", diff --git a/scripts/check-no-raw-window-open.mjs b/scripts/check-no-raw-window-open.mjs deleted file mode 100644 index 8f1239a4e8bf..000000000000 --- a/scripts/check-no-raw-window-open.mjs +++ /dev/null @@ -1,83 +0,0 @@ -#!/usr/bin/env node - -// Ensures UI code opens external URLs through the safe helper. -import { promises as fs } from "node:fs"; -import path from "node:path"; -import ts from "typescript"; -import { - collectTypeScriptFiles, - resolveRepoRoot, - runAsScript, - toLine, - unwrapExpression, -} from "./lib/ts-guard-utils.mjs"; - -const repoRoot = resolveRepoRoot(import.meta.url); -const uiSourceDir = path.join(repoRoot, "ui", "src", "ui"); -const allowedCallsites = new Set([path.join(uiSourceDir, "open-external-url.ts")]); - -function isRawWindowOpenCall(expression) { - const propertyAccess = unwrapExpression(expression); - if (!ts.isPropertyAccessExpression(propertyAccess) || propertyAccess.name.text !== "open") { - return false; - } - - const receiver = unwrapExpression(propertyAccess.expression); - return ( - ts.isIdentifier(receiver) && (receiver.text === "window" || receiver.text === "globalThis") - ); -} - -/** - * Finds raw `window.open(...)` or `globalThis.open(...)` call lines. - */ -export function findRawWindowOpenLines(content, fileName = "source.ts") { - const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); - const lines = []; - - const visit = (node) => { - if (ts.isCallExpression(node) && isRawWindowOpenCall(node.expression)) { - lines.push(toLine(sourceFile, node.expression)); - } - ts.forEachChild(node, visit); - }; - - visit(sourceFile); - return lines; -} - -/** - * Runs the raw window.open guard. - */ -export async function main() { - const files = await collectTypeScriptFiles(uiSourceDir, { - extraTestSuffixes: [".browser.test.ts", ".node.test.ts"], - ignoreMissing: true, - }); - const violations = []; - - for (const filePath of files) { - if (allowedCallsites.has(filePath)) { - continue; - } - - const content = await fs.readFile(filePath, "utf8"); - for (const line of findRawWindowOpenLines(content, filePath)) { - const relPath = path.relative(repoRoot, filePath); - violations.push(`${relPath}:${line}`); - } - } - - if (violations.length === 0) { - return; - } - - console.error("Found raw window.open usage outside safe helper:"); - for (const violation of violations) { - console.error(`- ${violation}`); - } - console.error("Use openExternalUrlSafe(...) from ui/src/lib/open-external-url.ts instead."); - process.exit(1); -} - -runAsScript(import.meta.url, main); diff --git a/scripts/check-no-register-http-handler.mjs b/scripts/check-no-register-http-handler.mjs deleted file mode 100644 index bca5a61c4744..000000000000 --- a/scripts/check-no-register-http-handler.mjs +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env node - -// Blocks deprecated plugin `registerHttpHandler` callsites. -import ts from "typescript"; -import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; -import { - collectCallExpressionLines, - runAsScript, - unwrapExpression, -} from "./lib/ts-guard-utils.mjs"; - -const sourceRoots = ["src", "extensions"]; - -function isDeprecatedRegisterHttpHandlerCall(expression) { - const callee = unwrapExpression(expression); - return ts.isPropertyAccessExpression(callee) && callee.name.text === "registerHttpHandler"; -} - -/** - * Finds deprecated `registerHttpHandler(...)` call lines. - */ -function findDeprecatedRegisterHttpHandlerLines(content, fileName = "source.ts") { - const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); - return collectCallExpressionLines(ts, sourceFile, (node) => - isDeprecatedRegisterHttpHandlerCall(node.expression) ? node.expression : null, - ); -} - -/** - * Runs the deprecated HTTP handler API guard. - */ -export async function main() { - await runCallsiteGuard({ - importMetaUrl: import.meta.url, - sourceRoots, - findCallLines: findDeprecatedRegisterHttpHandlerLines, - header: "Found deprecated plugin API call registerHttpHandler(...):", - footer: - "Use registerHttpRoute({ path, auth, match, handler }) and registerPluginHttpRoute for dynamic webhook paths.", - }); -} - -runAsScript(import.meta.url, main); diff --git a/scripts/oxlint-boundary-guards.mjs b/scripts/oxlint-boundary-guards.mjs new file mode 100644 index 000000000000..5257f14b2cf5 --- /dev/null +++ b/scripts/oxlint-boundary-guards.mjs @@ -0,0 +1,66 @@ +const EXPRESSION_WRAPPER_RE = + /^(?:ChainExpression|ParenthesizedExpression|TSAsExpression|TSNonNullExpression|TSTypeAssertion)$/; +const TEST_FILE_SUFFIXES = [".test.ts", ".test-utils.ts", ".test-harness.ts", ".e2e-harness.ts"]; + +function unwrapExpression(node) { + let current = node; + while (EXPRESSION_WRAPPER_RE.test(current.type)) { + current = current.expression; + } + return current; +} + +function restrictedCallRule({ allowedFiles = [], message, objects, property, roots }) { + return { + create(context) { + const filename = context.physicalFilename.replaceAll("\\", "/"); + const cwd = context.cwd.replaceAll("\\", "/"); + const repoPath = filename.startsWith(`${cwd}/`) ? filename.slice(cwd.length + 1) : filename; + if ( + !filename.endsWith(".ts") || + !roots.some((root) => repoPath === root || repoPath.startsWith(`${root}/`)) || + TEST_FILE_SUFFIXES.some((suffix) => filename.endsWith(suffix)) || + allowedFiles.includes(repoPath) + ) { + return {}; + } + return { + CallExpression(node) { + const callee = unwrapExpression(node.callee); + if ( + callee.type !== "MemberExpression" || + callee.computed || + callee.property.type !== "Identifier" || + callee.property.name !== property + ) { + return; + } + const receiver = unwrapExpression(callee.object); + if (objects && (receiver.type !== "Identifier" || !objects.includes(receiver.name))) { + return; + } + context.report({ message, node: node.callee }); + }, + }; + }, + }; +} + +export default { + meta: { name: "openclaw-boundaries" }, + rules: { + "no-raw-window-open-call": restrictedCallRule({ + allowedFiles: ["ui/src/lib/editor-links.ts", "ui/src/lib/open-external-url.ts"], + roots: ["ui/src", "test/fixtures/oxlint-boundary-guards"], + property: "open", + objects: ["window", "globalThis"], + message: "Use openExternalUrlSafe(...) from ui/src/lib/open-external-url.ts instead.", + }), + "no-register-http-handler-call": restrictedCallRule({ + roots: ["src", "extensions", "test/fixtures/oxlint-boundary-guards"], + property: "registerHttpHandler", + message: + "Use registerHttpRoute({ path, auth, match, handler }) and registerPluginHttpRoute for dynamic webhook paths.", + }), + }, +}; diff --git a/scripts/run-oxlint.mjs b/scripts/run-oxlint.mjs index 3c285449ee47..0a43f98c7347 100644 --- a/scripts/run-oxlint.mjs +++ b/scripts/run-oxlint.mjs @@ -39,6 +39,7 @@ const OXLINT_VALUE_FLAGS = new Set([ "--tsconfig", "--warn", ]); +const OPENCLAW_FOCUSED_CONFIG_FLAG = "--openclaw-focused-config"; /** * Returns whether oxlint args need package-boundary declaration artifacts first. @@ -206,10 +207,14 @@ async function prepareExtensionPackageBoundaryArtifacts(env) { * Applies wrapper policy and runs oxlint with the final argument list. */ export async function main(argv = process.argv.slice(2), runtimeEnv = process.env) { - const { args: policyArgs, env } = applyLocalOxlintPolicy( - argv, - resolveLocalHeavyCheckEnv(runtimeEnv), - ); + const focusedConfig = argv.includes(OPENCLAW_FOCUSED_CONFIG_FLAG); + const oxlintArgs = argv.filter((arg) => arg !== OPENCLAW_FOCUSED_CONFIG_FLAG); + const localEnv = resolveLocalHeavyCheckEnv(runtimeEnv); + // Focused configs are syntax-only guards; keep wrapper process handling + // without the broad type-aware policy or package artifact preparation. + const { args: policyArgs, env } = focusedConfig + ? { args: oxlintArgs, env: localEnv } + : applyLocalOxlintPolicy(oxlintArgs, localEnv); const sparseTargets = filterSparseMissingOxlintTargets(policyArgs); const finalArgs = sparseTargets.args; if (sparseTargets.skippedTargets.length > 0) { @@ -229,7 +234,7 @@ export async function main(argv = process.argv.slice(2), runtimeEnv = process.en } const releaseLock = - env.OPENCLAW_OXLINT_SKIP_LOCK === "1" + env.OPENCLAW_OXLINT_SKIP_LOCK === "1" || focusedConfig ? () => {} : shouldAcquireLocalHeavyCheckLockForOxlint(finalArgs, { cwd: process.cwd(), @@ -244,6 +249,7 @@ export async function main(argv = process.argv.slice(2), runtimeEnv = process.en try { if ( + !focusedConfig && env.OPENCLAW_OXLINT_SKIP_PREPARE !== "1" && shouldPrepareExtensionPackageBoundaryArtifacts(finalArgs) ) { diff --git a/test/fixtures/oxlint-boundary-guards/raw-window-open-violation.ts b/test/fixtures/oxlint-boundary-guards/raw-window-open-violation.ts new file mode 100644 index 000000000000..a61b5fcb80a7 --- /dev/null +++ b/test/fixtures/oxlint-boundary-guards/raw-window-open-violation.ts @@ -0,0 +1,12 @@ +window.open("https://example.com"); +globalThis.open("https://example.com"); +(window as Window).open("https://example.com"); +(window.open as typeof window.open)("https://example.com"); +window?.open?.("https://example.com"); +const open = window.open; +const { open: openWindow } = window; +window["open"]("https://example.com"); +app.window.open("https://example.com"); +(window satisfies Window).open("https://example.com"); +// window.open("https://example.com"); +const text = "window.open('https://example.com')"; diff --git a/test/fixtures/oxlint-boundary-guards/register-http-handler-violation.ts b/test/fixtures/oxlint-boundary-guards/register-http-handler-violation.ts new file mode 100644 index 000000000000..051b202b10ed --- /dev/null +++ b/test/fixtures/oxlint-boundary-guards/register-http-handler-violation.ts @@ -0,0 +1,7 @@ +plugin.registerHttpHandler(() => {}); +(plugin.registerHttpHandler as (handler: () => void) => void)(() => {}); +plugin?.registerHttpHandler?.(() => {}); +const register = plugin.registerHttpHandler; +const { registerHttpHandler } = plugin; +plugin["registerHttpHandler"](() => {}); +(plugin.registerHttpHandler satisfies (handler: () => void) => void)(() => {}); diff --git a/test/scripts/check-no-raw-window-open.test.ts b/test/scripts/check-no-raw-window-open.test.ts deleted file mode 100644 index c66ad411ca87..000000000000 --- a/test/scripts/check-no-raw-window-open.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -// Check No Raw Window Open tests cover check no raw window open script behavior. -import { describe, expect, it } from "vitest"; -import { findRawWindowOpenLines } from "../../scripts/check-no-raw-window-open.mjs"; - -describe("check-no-raw-window-open", () => { - it("finds direct window.open calls", () => { - const source = ` - function openDocs() { - window.open("https://docs.openclaw.ai"); - } - `; - expect(findRawWindowOpenLines(source)).toEqual([3]); - }); - - it("finds globalThis.open calls", () => { - const source = ` - function openDocs() { - globalThis.open("https://docs.openclaw.ai"); - } - `; - expect(findRawWindowOpenLines(source)).toEqual([3]); - }); - - it("ignores mentions in strings and comments", () => { - const source = ` - // window.open("https://example.com") - const text = "window.open('https://example.com')"; - `; - expect(findRawWindowOpenLines(source)).toStrictEqual([]); - }); - - it("handles parenthesized and asserted window references", () => { - const source = ` - const openRef = (window as Window).open; - openRef("https://example.com"); - (window as Window).open("https://example.com"); - `; - expect(findRawWindowOpenLines(source)).toEqual([4]); - }); -}); diff --git a/test/scripts/oxlint-boundary-guards.test.ts b/test/scripts/oxlint-boundary-guards.test.ts new file mode 100644 index 000000000000..c4662d6a8468 --- /dev/null +++ b/test/scripts/oxlint-boundary-guards.test.ts @@ -0,0 +1,41 @@ +import { spawnSync } from "node:child_process"; +import { describe, expect, it } from "vitest"; + +const FIXTURES = "test/fixtures/oxlint-boundary-guards"; +const cases = [ + { + rule: "openclaw-boundaries/no-register-http-handler-call", + violation: `${FIXTURES}/register-http-handler-violation.ts`, + violations: 3, + }, + { + rule: "openclaw-boundaries/no-raw-window-open-call", + violation: `${FIXTURES}/raw-window-open-violation.ts`, + violations: 5, + }, +]; + +function runGuard(target: string) { + return spawnSync( + process.execPath, + [ + "scripts/run-oxlint.mjs", + "--openclaw-focused-config", + "--config", + "config/oxlint/boundary-guards.json", + target, + ], + { encoding: "utf8" }, + ); +} + +describe("oxlint boundary guards", () => { + it.each(cases)("matches legacy call-only semantics for $rule", (testCase) => { + const violation = runGuard(testCase.violation); + const output = `${violation.stdout}${violation.stderr}`; + expect(violation.status).toBe(1); + expect(output.split(`${testCase.rule.replace("/", "(")})`)).toHaveLength( + testCase.violations + 1, + ); + }); +}); diff --git a/ui/src/components/app-sidebar-menus.ts b/ui/src/components/app-sidebar-menus.ts index c5919239a845..518278b90063 100644 --- a/ui/src/components/app-sidebar-menus.ts +++ b/ui/src/components/app-sidebar-menus.ts @@ -9,8 +9,9 @@ import { } from "../app-navigation.ts"; import { pathForRoute } from "../app-route-paths.ts"; import { normalizeAgentLabel } from "../lib/agents/display.ts"; -import { editorOpenUrl } from "../lib/editor-links.ts"; +import { openEditor } from "../lib/editor-links.ts"; import { isGatewayMethodAdvertised } from "../lib/gateway-methods.ts"; +import { openExternalUrlSafe } from "../lib/open-external-url.ts"; import { searchForSession } from "../lib/sessions/index.ts"; import { canArchiveSessionRow, @@ -435,10 +436,10 @@ export abstract class AppSidebarMenusElement extends AppSidebarSessionGroupsElem this.selectSession(session.key); break; case "open-pr": - window.open(action.url, "_blank", "noopener"); + openExternalUrlSafe(action.url); break; case "open-in": - window.open(editorOpenUrl(action.editor, action.path)); + openEditor(action.editor, action.path); break; case "toggle-pin": void this.patchSession(session, { pinned: !session.pinned }); diff --git a/ui/src/lib/editor-links.ts b/ui/src/lib/editor-links.ts index 4370afd32258..1019cf94df06 100644 --- a/ui/src/lib/editor-links.ts +++ b/ui/src/lib/editor-links.ts @@ -11,7 +11,7 @@ export const EDITOR_LABELS: Record = { zed: "Zed", }; -export function editorOpenUrl(editor: EditorId, absPath: string, line?: number | null): string { +function editorOpenUrl(editor: EditorId, absPath: string, line?: number | null): string { const normalizedPath = absPath.replaceAll("\\", "/"); const urlPath = normalizedPath.startsWith("/") ? normalizedPath : `/${normalizedPath}`; const encodedPath = urlPath @@ -22,3 +22,8 @@ export function editorOpenUrl(editor: EditorId, absPath: string, line?: number | .join("/"); return `${editor}://file${encodedPath}${line ? `:${line}` : ""}`; } + +export function openEditor(editor: EditorId, path: string, line?: number | null) { + // Typed editor IDs plus encoded paths make this custom-scheme handoff safe. + return window.open(editorOpenUrl(editor, path, line)); +} diff --git a/ui/src/pages/chat/components/chat-sidebar.test.ts b/ui/src/pages/chat/components/chat-sidebar.test.ts index c951e4c47ddf..bc13a5a3071d 100644 --- a/ui/src/pages/chat/components/chat-sidebar.test.ts +++ b/ui/src/pages/chat/components/chat-sidebar.test.ts @@ -1,7 +1,7 @@ /* @vitest-environment jsdom */ import { describe, expect, it, vi } from "vitest"; -import { editorOpenUrl } from "../../../lib/editor-links.ts"; +import { openEditor } from "../../../lib/editor-links.ts"; import { hasUniformLineEndings } from "./chat-sidebar.ts"; describe("hasUniformLineEndings", () => { @@ -19,35 +19,42 @@ describe("hasUniformLineEndings", () => { }); }); -describe("editorOpenUrl", () => { - it("creates a custom editor URL for a plain path", () => { - expect(editorOpenUrl("cursor", "/workspace/src/foo.ts")).toBe( +describe("openEditor", () => { + it.each([ + [ + "plain path", + "cursor", + "/workspace/src/foo.ts", + undefined, "cursor://file/workspace/src/foo.ts", - ); - }); - - it("encodes spaces in paths", () => { - expect(editorOpenUrl("vscode", "/workspace/My File.ts")).toBe( + ], + [ + "spaces", + "vscode", + "/workspace/My File.ts", + undefined, "vscode://file/workspace/My%20File.ts", - ); - }); - - it("appends a target line", () => { - expect(editorOpenUrl("zed", "/workspace/src/foo.ts", 42)).toBe( - "zed://file/workspace/src/foo.ts:42", - ); - }); - - it("normalizes Windows paths", () => { - expect(editorOpenUrl("vscode", "C:\\workspace\\src\\foo.ts", 42)).toBe( + ], + ["target line", "zed", "/workspace/src/foo.ts", 42, "zed://file/workspace/src/foo.ts:42"], + [ + "Windows path", + "vscode", + "C:\\workspace\\src\\foo.ts", + 42, "vscode://file/C:/workspace/src/foo.ts:42", - ); - }); - - it("encodes URL-significant path characters", () => { - expect(editorOpenUrl("windsurf", "/workspace/#notes?.md")).toBe( + ], + [ + "URL-significant characters", + "windsurf", + "/workspace/#notes?.md", + undefined, "windsurf://file/workspace/%23notes%3F.md", - ); + ], + ] as const)("opens the encoded custom URL for %s", (_name, editor, path, line, expected) => { + const open = vi.spyOn(window, "open").mockReturnValue(null); + openEditor(editor, path, line); + expect(open).toHaveBeenCalledWith(expected); + open.mockRestore(); }); }); diff --git a/ui/src/pages/chat/components/chat-sidebar.ts b/ui/src/pages/chat/components/chat-sidebar.ts index 04f4c38c993b..f420ccb71ce7 100644 --- a/ui/src/pages/chat/components/chat-sidebar.ts +++ b/ui/src/pages/chat/components/chat-sidebar.ts @@ -18,7 +18,7 @@ import { type EmbedSandboxMode, } from "../../../lib/chat/tool-display.ts"; import { copyToClipboard } from "../../../lib/clipboard.ts"; -import { type EditorId, editorOpenUrl } from "../../../lib/editor-links.ts"; +import { type EditorId, openEditor } from "../../../lib/editor-links.ts"; import { OpenClawLightDomElement } from "../../../lit/openclaw-element.ts"; import "./session-diff-panel.ts"; import { renderChatSidebarEditorMenu } from "./chat-sidebar-editor-menu.ts"; @@ -937,8 +937,7 @@ class ChatDetailPanel extends OpenClawLightDomElement { return; } this.fileEditorMenuOpen = false; - // A custom-scheme window hands off to the OS without navigating this page. - window.open(editorOpenUrl(editor, absPath, content.line)); + openEditor(editor, absPath, content.line); }; private readonly copyFileContents = () => { diff --git a/ui/src/pages/sessions/sessions-page.ts b/ui/src/pages/sessions/sessions-page.ts index a094ca65ee0e..367e3189460b 100644 --- a/ui/src/pages/sessions/sessions-page.ts +++ b/ui/src/pages/sessions/sessions-page.ts @@ -17,8 +17,9 @@ import type { SessionMenuAction, SessionMenuWork } from "../../components/sessio import { isStoppableCloudWorkerPlacement } from "../../components/session-row-badges.ts"; import { renderSettingsWorkspace } from "../../components/settings-workspace.ts"; import { t } from "../../i18n/index.ts"; -import { editorOpenUrl } from "../../lib/editor-links.ts"; +import { openEditor } from "../../lib/editor-links.ts"; import { isGatewayMethodAdvertised } from "../../lib/gateway-methods.ts"; +import { openExternalUrlSafe } from "../../lib/open-external-url.ts"; import { isWorkboardEnabledInConfigSnapshot } from "../../lib/plugin-activation.ts"; import type { SessionsGroupBy } from "../../lib/sessions/grouping.ts"; import { @@ -1096,11 +1097,10 @@ class SessionsPage extends OpenClawLightDomElement { context.navigate("chat", { search: searchForSession(row.key), hash: "" }); break; case "open-pr": - window.open(action.url, "_blank", "noopener"); + openExternalUrlSafe(action.url); break; case "open-in": - // A custom-scheme window hands off to the OS without navigating this page. - window.open(editorOpenUrl(action.editor, action.path)); + openEditor(action.editor, action.path); break; case "toggle-pin": void this.patchSession(row.key, { pinned: row.pinned !== true });