Address Homarr provider review feedback

This commit is contained in:
ajnart committed 2026-08-11 21:50:40 +02:00
1 parent 4ebaea29fd
commit ea9075bde5
9 files changed
+199 -88

No files matched your search

@@ -52,6 +52,8 @@ import type { AssistantProvider, AssistantProviderCategory } from "@homarr/defin
import { showErrorNotification, showSuccessNotification } from "@homarr/notifications";
import { useScopedI18n } from "@homarr/translation/client";
import { getWorkshopAssistantProviderUrl } from "~/components/workshop/workshop-client";
import classes from "./assistant-configuration.module.css";
import { formatAssistantContextWindow, getAssistantConnectionState } from "./assistant-configuration-state";
@@ -153,8 +155,13 @@ export const AssistantConfiguration = () => {
const [webSearchEnabled, setWebSearchEnabled] = useState(false);
useEffect(() => {
setProvider(configuration?.provider ?? "openrouter");
setBaseUrl(configuration?.baseUrl ?? assistantProviderPresets.openrouter.baseUrl);
const configuredProvider = configuration?.provider ?? "openrouter";
setProvider(configuredProvider);
setBaseUrl(
configuredProvider === "homarr"
? getWorkshopAssistantProviderUrl()
: (configuration?.baseUrl ?? assistantProviderPresets.openrouter.baseUrl),
);
setModelDiscoveryPath(configuration?.modelDiscoveryPath ?? "");
setModelId(configuration?.modelId ?? "");
setEnabled(configuration?.enabled ?? false);
@@ -291,7 +298,7 @@ export const AssistantConfiguration = () => {
const nextPreset = assistantProviderPresets[nextProvider];
clearDraftConnectionState();
setProvider(nextProvider);
setBaseUrl(nextPreset.baseUrl);
setBaseUrl(nextProvider === "homarr" ? getWorkshopAssistantProviderUrl() : nextPreset.baseUrl);
setModelDiscoveryPath(nextPreset.modelDiscoveryPath ?? "");
};
@@ -321,12 +328,11 @@ export const AssistantConfiguration = () => {
};
const saveConnection = (clearApiKey = false) => {
const customHeaders =
isHomarrProvider
? undefined
: headers.length > 0
? Object.fromEntries(headers.map((header) => [header.name.trim(), header.value]))
: undefined;
const customHeaders = isHomarrProvider
? undefined
: headers.length > 0
? Object.fromEntries(headers.map((header) => [header.name.trim(), header.value]))
: undefined;
updateConnection.mutate({
provider,
baseUrl,
@@ -542,76 +548,81 @@ export const AssistantConfiguration = () => {
{!isHomarrProvider && (
<Accordion variant="contained" radius="md">
<Accordion.Item value="headers">
<Accordion.Control className={classes.advancedControl} icon={<IconShieldLock size={18} />}>
<Group justify="space-between" wrap="nowrap">
<Box>
<Text size="sm" fw={600}>
{t("headers.title")}
</Text>
<Text size="xs" c="dimmed">
{t("headers.description")}
</Text>
</Box>
{configuration?.customHeadersConfigured && (
<Badge variant="light" color={clearHeaders ? "yellow" : "green"}>
{clearHeaders ? t("headers.pendingRemoval") : t("headers.savedBadge")}
</Badge>
)}
</Group>
</Accordion.Control>
<Accordion.Panel>
<Stack gap="sm">
<Group justify="space-between" align="center">
<Text size="sm" c="dimmed">
{configuration?.customHeadersConfigured ? t("headers.configured") : t("headers.empty")}
</Text>
<Button variant="light" size="compact-sm" leftSection={<IconPlus size={14} />} onClick={addHeader}>
{t("headers.add")}
</Button>
<Accordion.Control className={classes.advancedControl} icon={<IconShieldLock size={18} />}>
<Group justify="space-between" wrap="nowrap">
<Box>
<Text size="sm" fw={600}>
{t("headers.title")}
</Text>
<Text size="xs" c="dimmed">
{t("headers.description")}
</Text>
</Box>
{configuration?.customHeadersConfigured && (
<Badge variant="light" color={clearHeaders ? "yellow" : "green"}>
{clearHeaders ? t("headers.pendingRemoval") : t("headers.savedBadge")}
</Badge>
)}
</Group>
{headers.map((header) => (
<Group key={header.id} align="flex-end" wrap="wrap">
<TextInput
flex={1}
miw="12rem"
label={t("headers.name")}
value={header.name}
onChange={(event) => updateHeader(header.id, "name", event.currentTarget.value)}
placeholder="X-Provider-Header"
/>
<PasswordInput
flex={2}
miw="12rem"
label={t("headers.value")}
value={header.value}
onChange={(event) => updateHeader(header.id, "value", event.currentTarget.value)}
placeholder={t("headers.valuePlaceholder")}
autoComplete="off"
/>
<Tooltip label={t("headers.remove")}>
<ActionIcon
size="lg"
variant="subtle"
color="red"
aria-label={t("headers.remove")}
onClick={() => setHeaders((current) => current.filter((item) => item.id !== header.id))}
>
<IconTrash size={17} />
</ActionIcon>
</Tooltip>
</Accordion.Control>
<Accordion.Panel>
<Stack gap="sm">
<Group justify="space-between" align="center">
<Text size="sm" c="dimmed">
{configuration?.customHeadersConfigured ? t("headers.configured") : t("headers.empty")}
</Text>
<Button
variant="light"
size="compact-sm"
leftSection={<IconPlus size={14} />}
onClick={addHeader}
>
{t("headers.add")}
</Button>
</Group>
))}
{configuration?.customHeadersConfigured && headers.length === 0 && (
<Button
variant="light"
color={clearHeaders ? "gray" : "red"}
onClick={() => setClearHeaders(!clearHeaders)}
>
{clearHeaders ? t("headers.keep") : t("headers.clear")}
</Button>
)}
</Stack>
</Accordion.Panel>
{headers.map((header) => (
<Group key={header.id} align="flex-end" wrap="wrap">
<TextInput
flex={1}
miw="12rem"
label={t("headers.name")}
value={header.name}
onChange={(event) => updateHeader(header.id, "name", event.currentTarget.value)}
placeholder="X-Provider-Header"
/>
<PasswordInput
flex={2}
miw="12rem"
label={t("headers.value")}
value={header.value}
onChange={(event) => updateHeader(header.id, "value", event.currentTarget.value)}
placeholder={t("headers.valuePlaceholder")}
autoComplete="off"
/>
<Tooltip label={t("headers.remove")}>
<ActionIcon
size="lg"
variant="subtle"
color="red"
aria-label={t("headers.remove")}
onClick={() => setHeaders((current) => current.filter((item) => item.id !== header.id))}
>
<IconTrash size={17} />
</ActionIcon>
</Tooltip>
</Group>
))}
{configuration?.customHeadersConfigured && headers.length === 0 && (
<Button
variant="light"
color={clearHeaders ? "gray" : "red"}
onClick={() => setClearHeaders(!clearHeaders)}
>
{clearHeaders ? t("headers.keep") : t("headers.clear")}
</Button>
)}
</Stack>
</Accordion.Panel>
</Accordion.Item>
</Accordion>
)}
@@ -1,6 +1,10 @@
import { describe, expect, test } from "vitest";
import { getAssistantProviderQuotaLevel, isAssistantProviderUnavailable } from "./assistant-provider-quota";
import {
getAssistantProviderQuotaLevel,
getAssistantProviderQuotaRefreshDelay,
isAssistantProviderUnavailable,
} from "./assistant-provider-quota";
describe("getAssistantProviderQuotaLevel", () => {
test.each([
@@ -19,4 +23,10 @@ describe("getAssistantProviderQuotaLevel", () => {
expect(isAssistantProviderUnavailable({ provider: "homarr", signedIn: true, remaining: undefined })).toBe(false);
expect(isAssistantProviderUnavailable({ provider: "openrouter", signedIn: false, remaining: 0 })).toBe(false);
});
test("refreshes just after the UTC reset, or immediately when it has passed", () => {
const reset = "2026-08-12T00:00:00.000Z";
expect(getAssistantProviderQuotaRefreshDelay(reset, Date.parse("2026-08-11T23:59:00.000Z"))).toBe(61_000);
expect(getAssistantProviderQuotaRefreshDelay(reset, Date.parse("2026-08-12T00:00:01.000Z"))).toBe(1_000);
});
});
@@ -3,6 +3,9 @@ import type { AssistantProvider } from "@homarr/definitions";
export type AssistantProviderQuotaLevel = "ok" | "warning" | "bad" | "dead";
export const getAssistantProviderQuotaRefreshDelay = (resetsAt: string, now = Date.now()) =>
Math.max(0, Date.parse(resetsAt) - now) + 1_000;
export const getAssistantProviderQuotaLevel = (
usage: Pick<WorkshopAssistantUsage, "limit" | "remaining">,
): AssistantProviderQuotaLevel => {
@@ -29,7 +29,7 @@ import { createWorkshopClient } from "~/components/workshop/workshop-client";
import { AssistantContext, AssistantPreferencesContext, useAssistantPreferences } from "./assistant-context";
import { shouldAutomaticallyContinueAssistant } from "./assistant-auto-submit";
import { isAssistantProviderUnavailable } from "./assistant-provider-quota";
import { getAssistantProviderQuotaRefreshDelay, isAssistantProviderUnavailable } from "./assistant-provider-quota";
import { AssistantAutoApprovalProvider } from "./assistant-auto-approval";
import { getRunningAssistantPartType } from "./assistant-activity-state";
import { prepareAssistantRequestBody } from "./assistant-attachment-payload";
@@ -237,6 +237,12 @@ const AssistantPreferencesProvider = ({ children }: PropsWithChildren) => {
void refreshQuota();
}, [data?.provider, providerUser, refreshQuota]);
useEffect(() => {
if (data?.provider !== "homarr" || !providerUser || !quota) return;
const timeout = window.setTimeout(() => void refreshQuota(), getAssistantProviderQuotaRefreshDelay(quota.resetsAt));
return () => window.clearTimeout(timeout);
}, [data?.provider, providerUser, quota, refreshQuota]);
const value = useMemo(
() => ({
provider: data?.provider ?? null,
@@ -3,13 +3,23 @@ import { WORKSHOP_API_URL, WORKSHOP_WEB_URL } from "@homarr/workshop/schema";
let browserClient: WorkshopBackend | undefined;
export function getWorkshopApiUrl() {
const runtimeUrl =
typeof document === "undefined"
? undefined
: document.querySelector<HTMLMetaElement>('meta[name="homarr-workshop-api-url"]')?.content;
return (runtimeUrl || WORKSHOP_API_URL).replace(/\/+$/u, "");
}
export function getWorkshopAssistantProviderUrl() {
return `${getWorkshopApiUrl()}/api/ai/v1`;
}
export function createWorkshopClient() {
if (typeof document === "undefined") return new WorkshopBackend(WORKSHOP_API_URL);
if (typeof document === "undefined") return new WorkshopBackend(getWorkshopApiUrl());
if (browserClient) return browserClient;
const runtimeUrl = document.querySelector<HTMLMetaElement>('meta[name="homarr-workshop-api-url"]')?.content;
browserClient = new WorkshopBackend(runtimeUrl || WORKSHOP_API_URL);
browserClient = new WorkshopBackend(getWorkshopApiUrl());
return browserClient;
}
@@ -3,7 +3,7 @@
import { readFileSync } from "node:fs";
import { afterEach, describe, expect, it } from "vitest";
import { getWorkshopWebUrl } from "./workshop-client";
import { getWorkshopAssistantProviderUrl, getWorkshopWebUrl } from "./workshop-client";
describe("Workshop widget install flow", () => {
afterEach(() => {
@@ -17,6 +17,12 @@ describe("Workshop widget install flow", () => {
expect(getWorkshopWebUrl("widget/id")).toBe("https://preview.example/workshop/widget%2Fid");
});
it("derives the Homarr provider endpoint from the runtime Workshop API", () => {
document.head.innerHTML = '<meta name="homarr-workshop-api-url" content="https://api.preview.example/">';
expect(getWorkshopAssistantProviderUrl()).toBe("https://api.preview.example/api/ai/v1");
});
it("uses one registered modal stack for browsing, details, reports, and confirmation", () => {
const installButton = readFileSync(
`${process.cwd()}/apps/nextjs/src/components/workshop/workshop-install-button.tsx`,
+29 -4
View File
@@ -25,11 +25,14 @@ const (
defaultDailyLimit = 50
maxProviderRequestUnits = 1000
maxChatRequestBytes = 8 << 20
maxChatResponseBytes = 8 << 20
maxUpstreamErrorBytes = 1 << 20
openRouterDefaultURL = "https://openrouter.ai/api/v1"
providerRequestTimeout = 5 * time.Minute
)
var errProviderResponseTooLarge = errors.New("provider response is too large")
type homarrProvider struct {
apiKey string
baseURL string
@@ -119,6 +122,9 @@ func newHomarrProviderFromEnv() (*homarrProvider, error) {
}
func (provider *homarrProvider) models(event *core.RequestEvent) error {
if provider.apiKey == "" {
return event.JSON(http.StatusServiceUnavailable, openAIError("The Homarr provider is not configured."))
}
return event.JSON(http.StatusOK, map[string]any{
"object": "list",
"data": []map[string]any{{
@@ -245,15 +251,27 @@ func (provider *homarrProvider) chat(event *core.RequestEvent) error {
return nil
}
responseBody, readErr := io.ReadAll(upstreamResponse.Body)
usage := usageFromJSON(responseBody)
provider.finishActivity(event.App, privateRequest, publicActivity, statusForError(readErr), usage, startedAt)
responseBody, readErr := readBoundedBody(upstreamResponse.Body, maxChatResponseBytes)
if readErr != nil {
return readErr
provider.finishActivity(event.App, privateRequest, publicActivity, "failed", requestUsage{}, startedAt)
return event.JSON(http.StatusBadGateway, openAIError("The model endpoint returned an invalid response."))
}
usage := usageFromJSON(responseBody)
provider.finishActivity(event.App, privateRequest, publicActivity, "completed", usage, startedAt)
return event.Blob(upstreamResponse.StatusCode, contentType, responseBody)
}
func readBoundedBody(reader io.Reader, limit int64) ([]byte, error) {
body, err := io.ReadAll(io.LimitReader(reader, limit+1))
if err != nil {
return nil, err
}
if int64(len(body)) > limit {
return nil, errProviderResponseTooLarge
}
return body, nil
}
func countRequestUnits(messages []providerMessage) int {
units := 1
for index := len(messages) - 1; index >= 0 && messages[index].Role == "tool"; index-- {
@@ -386,6 +404,9 @@ func (provider *homarrProvider) finishActivity(
if err != nil {
return err
}
if !requestBelongsToQuotaDay(quota.GetString("day"), startedAt) {
return nil
}
quota.Set("inputTokens", quota.GetInt("inputTokens")+usage.InputTokens)
quota.Set("outputTokens", quota.GetInt("outputTokens")+usage.OutputTokens)
quota.Set("totalTokens", quota.GetInt("totalTokens")+usage.TotalTokens)
@@ -393,6 +414,10 @@ func (provider *homarrProvider) finishActivity(
})
}
func requestBelongsToQuotaDay(day string, startedAt time.Time) bool {
return day == startedAt.UTC().Format(time.DateOnly)
}
func statusForError(err error) string {
if err != nil {
return "failed"
+22
View File
@@ -2,7 +2,9 @@ package main
import (
"math"
"strings"
"testing"
"time"
)
func TestCountRequestUnits(t *testing.T) {
@@ -84,3 +86,23 @@ func TestProviderEnvironment(t *testing.T) {
t.Fatal("expected invalid daily limit to fail")
}
}
func TestReadBoundedBody(t *testing.T) {
body, err := readBoundedBody(strings.NewReader("1234"), 4)
if err != nil || string(body) != "1234" {
t.Fatalf("unexpected bounded body: %q, %v", body, err)
}
if _, err := readBoundedBody(strings.NewReader("12345"), 4); err != errProviderResponseTooLarge {
t.Fatalf("expected response size error, got %v", err)
}
}
func TestRequestBelongsToQuotaDay(t *testing.T) {
startedAt := time.Date(2026, 8, 12, 1, 30, 0, 0, time.FixedZone("CEST", 2*60*60))
if !requestBelongsToQuotaDay("2026-08-11", startedAt) {
t.Fatal("expected request to match its UTC start day")
}
if requestBelongsToQuotaDay("2026-08-12", startedAt) {
t.Fatal("request usage must not be added after the quota record resets")
}
}
+18
View File
@@ -62,6 +62,24 @@ for client_id in workshop-test-client workshop-rotated-client; do
node apps/workshop/tests/oauth-sync.integration.mjs
done
OPENROUTER_API_KEY= PB_EXPOSE_PORT="$WORKSHOP_TEST_PORT" docker compose -p "$WORKSHOP_TEST_PROJECT" \
-f apps/workshop/docker-compose.yml up -d --force-recreate workshop
for attempt in $(seq 1 60); do
if curl --fail --silent "http://127.0.0.1:$WORKSHOP_TEST_PORT/api/health" >/dev/null; then
break
fi
if [ "$attempt" -eq 60 ]; then
exit 1
fi
sleep 1
done
models_status=$(curl --silent --output /dev/null --write-out '%{http_code}' \
"http://127.0.0.1:$WORKSHOP_TEST_PORT/api/ai/v1/models")
if [ "$models_status" != "503" ]; then
echo "Expected an unconfigured Homarr provider to return 503, received $models_status"
exit 1
fi
PB_EXPOSE_PORT="$WORKSHOP_TEST_PORT" docker compose -p "$WORKSHOP_TEST_PROJECT" \
-f apps/workshop/docker-compose.yml stop workshop
printf 'y\n' | PB_EXPOSE_PORT="$WORKSHOP_TEST_PORT" docker compose -p "$WORKSHOP_TEST_PROJECT" \