mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
Merge branch 'main' into update-flatted-3.4.2
This commit is contained in:
@@ -0,0 +1,60 @@
|
|||||||
|
import { render, screen, waitFor } from "@testing-library/react";
|
||||||
|
import userEvent from "@testing-library/user-event";
|
||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import { CopyableContentWrapper } from "#/components/shared/buttons/copyable-content-wrapper";
|
||||||
|
|
||||||
|
describe("CopyableContentWrapper", () => {
|
||||||
|
it("should hide the copy button by default", () => {
|
||||||
|
render(
|
||||||
|
<CopyableContentWrapper text="hello">
|
||||||
|
<p>content</p>
|
||||||
|
</CopyableContentWrapper>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByTestId("copy-to-clipboard")).not.toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should show the copy button on hover", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(
|
||||||
|
<CopyableContentWrapper text="hello">
|
||||||
|
<p>content</p>
|
||||||
|
</CopyableContentWrapper>,
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.hover(screen.getByText("content"));
|
||||||
|
|
||||||
|
expect(screen.getByTestId("copy-to-clipboard")).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should copy text to clipboard on click", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(
|
||||||
|
<CopyableContentWrapper text="copy me">
|
||||||
|
<p>content</p>
|
||||||
|
</CopyableContentWrapper>,
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.click(screen.getByTestId("copy-to-clipboard"));
|
||||||
|
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(navigator.clipboard.readText()).resolves.toBe("copy me"),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should show copied state after clicking", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(
|
||||||
|
<CopyableContentWrapper text="hello">
|
||||||
|
<p>content</p>
|
||||||
|
</CopyableContentWrapper>,
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.click(screen.getByTestId("copy-to-clipboard"));
|
||||||
|
|
||||||
|
expect(screen.getByTestId("copy-to-clipboard")).toHaveAttribute(
|
||||||
|
"aria-label",
|
||||||
|
"BUTTON$COPIED",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,37 @@
|
|||||||
|
import { render, screen, waitFor } from "@testing-library/react";
|
||||||
|
import userEvent from "@testing-library/user-event";
|
||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import { code as Code } from "#/components/features/markdown/code";
|
||||||
|
|
||||||
|
describe("code (markdown)", () => {
|
||||||
|
it("should render inline code without a copy button", () => {
|
||||||
|
render(<Code>inline snippet</Code>);
|
||||||
|
|
||||||
|
expect(screen.getByText("inline snippet")).toBeInTheDocument();
|
||||||
|
expect(screen.queryByTestId("copy-to-clipboard")).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should render a multiline code block with a copy button", () => {
|
||||||
|
render(<Code>{"line1\nline2"}</Code>);
|
||||||
|
|
||||||
|
expect(screen.getByText("line1 line2")).toBeInTheDocument();
|
||||||
|
expect(screen.getByTestId("copy-to-clipboard")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should render a syntax-highlighted block with a copy button", () => {
|
||||||
|
render(<Code className="language-js">{"console.log('hi')"}</Code>);
|
||||||
|
|
||||||
|
expect(screen.getByTestId("copy-to-clipboard")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should copy code block content to clipboard", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(<Code>{"line1\nline2"}</Code>);
|
||||||
|
|
||||||
|
await user.click(screen.getByTestId("copy-to-clipboard"));
|
||||||
|
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(navigator.clipboard.readText()).resolves.toBe("line1\nline2"),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -2,6 +2,7 @@ import React from "react";
|
|||||||
import { ExtraProps } from "react-markdown";
|
import { ExtraProps } from "react-markdown";
|
||||||
import { Prism as SyntaxHighlighter } from "react-syntax-highlighter";
|
import { Prism as SyntaxHighlighter } from "react-syntax-highlighter";
|
||||||
import { vscDarkPlus } from "react-syntax-highlighter/dist/esm/styles/prism";
|
import { vscDarkPlus } from "react-syntax-highlighter/dist/esm/styles/prism";
|
||||||
|
import { CopyableContentWrapper } from "#/components/shared/buttons/copyable-content-wrapper";
|
||||||
|
|
||||||
// See https://github.com/remarkjs/react-markdown?tab=readme-ov-file#use-custom-components-syntax-highlight
|
// See https://github.com/remarkjs/react-markdown?tab=readme-ov-file#use-custom-components-syntax-highlight
|
||||||
|
|
||||||
@@ -15,6 +16,7 @@ export function code({
|
|||||||
React.HTMLAttributes<HTMLElement> &
|
React.HTMLAttributes<HTMLElement> &
|
||||||
ExtraProps) {
|
ExtraProps) {
|
||||||
const match = /language-(\w+)/.exec(className || ""); // get the language
|
const match = /language-(\w+)/.exec(className || ""); // get the language
|
||||||
|
const codeString = String(children).replace(/\n$/, "");
|
||||||
|
|
||||||
if (!match) {
|
if (!match) {
|
||||||
const isMultiline = String(children).includes("\n");
|
const isMultiline = String(children).includes("\n");
|
||||||
@@ -37,29 +39,33 @@ export function code({
|
|||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<pre
|
<CopyableContentWrapper text={codeString}>
|
||||||
style={{
|
<pre
|
||||||
backgroundColor: "#2a3038",
|
style={{
|
||||||
padding: "1em",
|
backgroundColor: "#2a3038",
|
||||||
borderRadius: "4px",
|
padding: "1em",
|
||||||
color: "#e6edf3",
|
borderRadius: "4px",
|
||||||
border: "1px solid #30363d",
|
color: "#e6edf3",
|
||||||
overflow: "auto",
|
border: "1px solid #30363d",
|
||||||
}}
|
overflow: "auto",
|
||||||
>
|
}}
|
||||||
<code className={className}>{String(children).replace(/\n$/, "")}</code>
|
>
|
||||||
</pre>
|
<code className={className}>{codeString}</code>
|
||||||
|
</pre>
|
||||||
|
</CopyableContentWrapper>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<SyntaxHighlighter
|
<CopyableContentWrapper text={codeString}>
|
||||||
className="rounded-lg"
|
<SyntaxHighlighter
|
||||||
style={vscDarkPlus}
|
className="rounded-lg"
|
||||||
language={match?.[1]}
|
style={vscDarkPlus}
|
||||||
PreTag="div"
|
language={match?.[1]}
|
||||||
>
|
PreTag="div"
|
||||||
{String(children).replace(/\n$/, "")}
|
>
|
||||||
</SyntaxHighlighter>
|
{codeString}
|
||||||
|
</SyntaxHighlighter>
|
||||||
|
</CopyableContentWrapper>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,44 @@
|
|||||||
|
import React from "react";
|
||||||
|
import { CopyToClipboardButton } from "./copy-to-clipboard-button";
|
||||||
|
|
||||||
|
export function CopyableContentWrapper({
|
||||||
|
text,
|
||||||
|
children,
|
||||||
|
}: {
|
||||||
|
text: string;
|
||||||
|
children: React.ReactNode;
|
||||||
|
}) {
|
||||||
|
const [isHovering, setIsHovering] = React.useState(false);
|
||||||
|
const [isCopied, setIsCopied] = React.useState(false);
|
||||||
|
|
||||||
|
const handleCopy = async () => {
|
||||||
|
await navigator.clipboard.writeText(text);
|
||||||
|
setIsCopied(true);
|
||||||
|
};
|
||||||
|
|
||||||
|
React.useEffect(() => {
|
||||||
|
let timeout: NodeJS.Timeout;
|
||||||
|
if (isCopied) {
|
||||||
|
timeout = setTimeout(() => setIsCopied(false), 2000);
|
||||||
|
}
|
||||||
|
return () => clearTimeout(timeout);
|
||||||
|
}, [isCopied]);
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div
|
||||||
|
className="relative"
|
||||||
|
onMouseEnter={() => setIsHovering(true)}
|
||||||
|
onMouseLeave={() => setIsHovering(false)}
|
||||||
|
>
|
||||||
|
<div className="absolute top-2 right-2 z-10">
|
||||||
|
<CopyToClipboardButton
|
||||||
|
isHidden={!isHovering}
|
||||||
|
isDisabled={isCopied}
|
||||||
|
onClick={handleCopy}
|
||||||
|
mode={isCopied ? "copied" : "copy"}
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
{children}
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -123,10 +123,9 @@ async def store_llm_settings(
|
|||||||
settings.llm_api_key = existing_settings.llm_api_key
|
settings.llm_api_key = existing_settings.llm_api_key
|
||||||
if settings.llm_model is None:
|
if settings.llm_model is None:
|
||||||
settings.llm_model = existing_settings.llm_model
|
settings.llm_model = existing_settings.llm_model
|
||||||
# if llm_base_url is missing or empty, try to preserve existing or determine appropriate URL
|
if settings.llm_base_url is None:
|
||||||
if not settings.llm_base_url:
|
# Not provided at all (e.g. MCP config save) - preserve existing or auto-detect
|
||||||
if settings.llm_base_url is None and existing_settings.llm_base_url:
|
if existing_settings.llm_base_url:
|
||||||
# Not provided at all (e.g. MCP config save) - preserve existing
|
|
||||||
settings.llm_base_url = existing_settings.llm_base_url
|
settings.llm_base_url = existing_settings.llm_base_url
|
||||||
elif is_openhands_model(settings.llm_model):
|
elif is_openhands_model(settings.llm_model):
|
||||||
# OpenHands models use the LiteLLM proxy
|
# OpenHands models use the LiteLLM proxy
|
||||||
@@ -145,6 +144,9 @@ async def store_llm_settings(
|
|||||||
logger.error(
|
logger.error(
|
||||||
f'Failed to get api_base from litellm for model {settings.llm_model}: {e}'
|
f'Failed to get api_base from litellm for model {settings.llm_model}: {e}'
|
||||||
)
|
)
|
||||||
|
elif settings.llm_base_url == '':
|
||||||
|
# Explicitly cleared by the user (basic view save or advanced view clear)
|
||||||
|
settings.llm_base_url = None
|
||||||
# Keep search API key if missing or empty
|
# Keep search API key if missing or empty
|
||||||
if not settings.search_api_key:
|
if not settings.search_api_key:
|
||||||
settings.search_api_key = existing_settings.search_api_key
|
settings.search_api_key = existing_settings.search_api_key
|
||||||
|
|||||||
@@ -211,9 +211,32 @@ async def test_store_llm_settings_partial_update():
|
|||||||
assert result.llm_model == 'gpt-4'
|
assert result.llm_model == 'gpt-4'
|
||||||
# For SecretStr objects, we need to compare the secret value
|
# For SecretStr objects, we need to compare the secret value
|
||||||
assert result.llm_api_key.get_secret_value() == 'existing-api-key'
|
assert result.llm_api_key.get_secret_value() == 'existing-api-key'
|
||||||
# llm_base_url was explicitly cleared (""), so auto-detection runs
|
# llm_base_url="" is an explicit clear — must not be repopulated via auto-detection
|
||||||
# OpenAI models: litellm.get_api_base() returns https://api.openai.com
|
assert result.llm_base_url is None
|
||||||
assert result.llm_base_url == 'https://api.openai.com'
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_store_llm_settings_advanced_view_clear_removes_base_url():
|
||||||
|
"""Regression test for #13420: clearing Base URL in Advanced view must persist.
|
||||||
|
|
||||||
|
Before the fix, llm_base_url="" was treated identically to llm_base_url=None,
|
||||||
|
causing the backend to re-run auto-detection and overwrite the user's intent.
|
||||||
|
"""
|
||||||
|
settings = Settings(
|
||||||
|
llm_model='gpt-4',
|
||||||
|
llm_base_url='', # User deleted the field in Advanced view
|
||||||
|
)
|
||||||
|
|
||||||
|
existing_settings = Settings(
|
||||||
|
llm_model='gpt-4',
|
||||||
|
llm_api_key=SecretStr('my-api-key'),
|
||||||
|
llm_base_url='https://my-custom-proxy.example.com',
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await store_llm_settings(settings, existing_settings)
|
||||||
|
|
||||||
|
# The old custom URL must not come back
|
||||||
|
assert result.llm_base_url is None
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user