mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-11-15 09:11:24 +00:00
fix(dsh): write the default backend when the selection is cleared
The backend selection is the one managed key with a default. Clearing it
wrote an empty value, and an empty value is not the default: ReMe applies
"${LLM_BACKEND:-openai}" only when the variable is absent, so the service
started with no backend at all and refused to run - while the plugin's own
probe resolved the same empty value to openai and reported a working
configuration.
The cleared selection now writes the default out, and the probe reports an
explicitly empty backend as an incomplete configuration instead of testing a
configuration the service cannot use. A file without the key keeps the
default, and an unknown backend name stays probed tolerantly: the list of
backends here is a snapshot, and ReMe may know more.
This commit is contained in:
parent
87fddc7e4c
commit
f14741cad9
5 changed files with 245 additions and 22 deletions
|
|
@ -3,15 +3,20 @@
|
|||
*
|
||||
* This module is the single source of truth for the managed keys and for which
|
||||
* of them are write-only secrets. An empty secret draft means "leave blank to
|
||||
* keep" and an empty non-secret draft is an explicit clear; both rules are
|
||||
* keep" and an empty non-secret draft is an explicit clear; the rules are
|
||||
* derived from the `secret` flag alone, so a key cannot carry a rule that
|
||||
* contradicts its own kind. The Host and the browser both derive their
|
||||
* behaviour from it, so the two can never drift apart about the same file.
|
||||
* contradicts its own kind. A key that names a default (`defaultValue`) is the
|
||||
* one exception: its empty draft means "use the default", and the default is
|
||||
* written out, because the service only falls back to it when the variable is
|
||||
* absent. The Host and the browser both derive their behaviour from it, so the
|
||||
* two can never drift apart about the same file.
|
||||
*
|
||||
* It stays browser-safe: no node built-in is imported here, and the browser
|
||||
* bundle must stay loadable without one.
|
||||
*/
|
||||
|
||||
import { DEFAULT_LLM_BACKEND } from "./llm-backend.js";
|
||||
|
||||
export interface EnvFieldMeta {
|
||||
/** The `.env` key. */
|
||||
key: string;
|
||||
|
|
@ -28,6 +33,15 @@ export interface EnvFieldMeta {
|
|||
* is off; `EMPTY` is the documented placeholder for "no embedding key".
|
||||
*/
|
||||
disabledValue?: string;
|
||||
/**
|
||||
* The value an empty selection is written as.
|
||||
*
|
||||
* An empty selection means "use the default". The default is written
|
||||
* explicitly, because ReMe's configuration only falls back to it when the
|
||||
* variable is absent: an empty value stays empty and names no backend, and
|
||||
* the service then refuses to start.
|
||||
*/
|
||||
defaultValue?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -37,7 +51,14 @@ export interface EnvFieldMeta {
|
|||
* and the browser form list the same keys in the same order.
|
||||
*/
|
||||
export const ENV_FIELDS: readonly EnvFieldMeta[] = [
|
||||
{ key: "LLM_BACKEND", secret: false },
|
||||
{
|
||||
key: "LLM_BACKEND",
|
||||
secret: false,
|
||||
// ReMe resolves `${LLM_BACKEND:-openai}` only when the variable is
|
||||
// absent, so a cleared selection must write the default out rather than
|
||||
// an empty value.
|
||||
defaultValue: DEFAULT_LLM_BACKEND,
|
||||
},
|
||||
{ key: "LLM_MODEL_NAME", secret: false },
|
||||
{ key: "LLM_API_KEY", secret: true },
|
||||
{ key: "LLM_BASE_URL", secret: false },
|
||||
|
|
@ -87,7 +108,9 @@ export function storedState(
|
|||
* Only touched keys travel: an untouched key keeps its stored value. An empty
|
||||
* secret draft is dropped rather than written, so "leave blank to keep" holds
|
||||
* even after the user typed a replacement and cleared it again. An empty
|
||||
* non-secret draft is an explicit clear and keeps that meaning.
|
||||
* non-secret draft is an explicit clear and keeps that meaning — unless the
|
||||
* key names a default, in which case the empty selection means "use the
|
||||
* default" and the default is written out.
|
||||
*
|
||||
* `stored` reports, per key, whether the file already holds a non-empty value.
|
||||
* It is what decides whether a key that must not stay empty gets its
|
||||
|
|
@ -121,6 +144,13 @@ export function envUpdates(
|
|||
updates[field.key] = field.disabledValue;
|
||||
continue;
|
||||
}
|
||||
// "Use the default": the default is written out, because the service
|
||||
// only falls back to it when the variable is absent. An empty value
|
||||
// would name no backend at all.
|
||||
if (field.defaultValue) {
|
||||
updates[field.key] = field.defaultValue;
|
||||
continue;
|
||||
}
|
||||
updates[field.key] = "";
|
||||
continue;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -278,7 +278,14 @@ export const LLM_PROTOCOLS: Record<string, LlmProtocol> = {
|
|||
/** The backend ReMe itself falls back to when `.env` names none. */
|
||||
export { DEFAULT_LLM_BACKEND };
|
||||
|
||||
/** The protocol a backend speaks; unknown backends use ReMe's default. */
|
||||
/**
|
||||
* The protocol a backend speaks; an unknown backend uses the OpenAI-style
|
||||
* request.
|
||||
*
|
||||
* The list of backends here is a snapshot of the ones the tab offers, and
|
||||
* ReMe may support more, so an unknown name is probed tolerantly rather than
|
||||
* refused.
|
||||
*/
|
||||
export function llmProtocolFor(backend: string): LlmProtocol {
|
||||
return LLM_PROTOCOLS[llmBackendOf(backend)] ?? OPENAI_PROTOCOL;
|
||||
}
|
||||
|
|
@ -353,17 +360,22 @@ except Exception as error:
|
|||
}
|
||||
|
||||
/**
|
||||
* The LLM backend the workspace `.env` selects. An unreadable or incomplete
|
||||
* file names none, which leaves the probe on ReMe's default backend.
|
||||
* The LLM backend the workspace `.env` selects.
|
||||
*
|
||||
* `undefined` means the file does not hold the key at all: the service then
|
||||
* falls back to its default. A present key is returned as its trimmed value,
|
||||
* which may be the empty string — that names no backend at all, and ReMe
|
||||
* refuses to start with it, so the caller has to know the difference.
|
||||
*/
|
||||
export function readLlmBackend(envPath: string): string {
|
||||
export function readLlmBackend(envPath: string): string | undefined {
|
||||
let text = "";
|
||||
try {
|
||||
text = readFileSync(envPath, "utf8");
|
||||
} catch {
|
||||
return "";
|
||||
return undefined;
|
||||
}
|
||||
return (parseEnvFile(text)["LLM_BACKEND"] ?? "").trim();
|
||||
const value = parseEnvFile(text)["LLM_BACKEND"];
|
||||
return value === undefined ? undefined : value.trim();
|
||||
}
|
||||
|
||||
/** Parse the JSON line the LLM test snippet prints. */
|
||||
|
|
@ -399,9 +411,19 @@ export async function testLlm(
|
|||
// The backend in the workspace `.env` decides the protocol; the probe must
|
||||
// not fall back to an OpenAI-style request for a provider that speaks its
|
||||
// own, or it would report a working configuration as broken.
|
||||
const protocol = llmProtocolFor(
|
||||
readLlmBackend(join(options.workspaceDir, ".env")),
|
||||
);
|
||||
const backend = readLlmBackend(join(options.workspaceDir, ".env"));
|
||||
// A present but empty backend names no backend. ReMe resolves its default
|
||||
// only when the variable is absent, so the service would start with no
|
||||
// backend at all and refuse to run: the honest answer is an incomplete
|
||||
// configuration, and the request is not sent.
|
||||
if (backend === "")
|
||||
return {
|
||||
ok: false,
|
||||
status: 0,
|
||||
latencyMs: 0,
|
||||
error: "LLM config incomplete (LLM_BACKEND)",
|
||||
};
|
||||
const protocol = llmProtocolFor(backend ?? DEFAULT_LLM_BACKEND);
|
||||
try {
|
||||
const result = await io.run(
|
||||
options.pythonPath,
|
||||
|
|
|
|||
|
|
@ -1,17 +1,23 @@
|
|||
/**
|
||||
* The LLM backend the workspace `.env` selects.
|
||||
*
|
||||
* ReMe's own configuration resolves an empty value to `openai`
|
||||
* (`${LLM_BACKEND:-openai}`), and the Host's probe resolves it the same way.
|
||||
* The tab has to agree: a backend that is only picked in the form and equals
|
||||
* the default is not a change to the configuration.
|
||||
* ReMe applies its default (`${LLM_BACKEND:-openai}`) only when the variable
|
||||
* is **absent**: an explicitly empty value stays empty and names no backend,
|
||||
* and the service then refuses to start. So the default has to be written out
|
||||
* rather than left blank — see `environment/env-fields.ts`, which owns that
|
||||
* rule.
|
||||
*
|
||||
* `llmBackendOf` is the *form's* reading, not the service's: an empty
|
||||
* selection there means "use the default", so it resolves to the default
|
||||
* backend. That is what the tab compares with, so picking the default in the
|
||||
* form is not a change to the configuration.
|
||||
*
|
||||
* This module stays browser-safe: no Node built-ins, because the client
|
||||
* imports it too.
|
||||
*/
|
||||
export const DEFAULT_LLM_BACKEND = "openai";
|
||||
|
||||
/** The backend a value means, with the default the configuration applies. */
|
||||
/** The backend a value means in the form, with the default the configuration applies. */
|
||||
export function llmBackendOf(value: string): string {
|
||||
return (value ?? "").trim().toLowerCase() || DEFAULT_LLM_BACKEND;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
SECRET_KEYS,
|
||||
storedState,
|
||||
} from "../dist/environment/env-fields.js";
|
||||
import { DEFAULT_LLM_BACKEND } from "../dist/environment/llm-backend.js";
|
||||
|
||||
/** A file that already holds a value for every key that needs one. */
|
||||
const STORED = { EMBEDDING_API_KEY: true };
|
||||
|
|
@ -40,8 +41,9 @@ test("only the two credentials are secret", () => {
|
|||
});
|
||||
|
||||
test("the metadata of a key carries its secret flag and placeholder", () => {
|
||||
// What an empty draft means is not metadata: it follows the `secret` flag
|
||||
// alone, which the update rules below apply.
|
||||
// What an empty draft means follows the `secret` flag, which the update
|
||||
// rules below apply. A key that names a default is the documented
|
||||
// exception.
|
||||
assert.equal(envFieldMeta("LLM_API_KEY")?.secret, true);
|
||||
assert.equal(envFieldMeta("LLM_BASE_URL")?.secret, false);
|
||||
assert.equal(envFieldMeta("EMBEDDING_API_KEY")?.disabledValue, "EMPTY");
|
||||
|
|
@ -124,3 +126,53 @@ test("a key the tab does not manage travels unchanged", () => {
|
|||
SECRET_OTHER: "value",
|
||||
});
|
||||
});
|
||||
|
||||
test("an empty backend selection is written as the default, not as an empty value", () => {
|
||||
// ReMe resolves `${LLM_BACKEND:-openai}` only when the variable is absent:
|
||||
// an explicitly empty value names no backend, and the service refuses to
|
||||
// start with it. So the cleared selection writes the default out.
|
||||
const updates = envUpdates({ LLM_BACKEND: "" }, STORED);
|
||||
assert.deepEqual(updates, { LLM_BACKEND: DEFAULT_LLM_BACKEND });
|
||||
assert.equal(updates.LLM_BACKEND, "openai");
|
||||
assert.notEqual(updates.LLM_BACKEND, "");
|
||||
});
|
||||
|
||||
test("a typed backend still travels", () => {
|
||||
const updates = envUpdates({ LLM_BACKEND: "dashscope" }, STORED);
|
||||
assert.deepEqual(updates, { LLM_BACKEND: "dashscope" });
|
||||
});
|
||||
|
||||
test("an untouched backend key does not travel", () => {
|
||||
// The stored backend survives the save, exactly like every other key.
|
||||
const updates = envUpdates({ LLM_MODEL_NAME: "qwen3.7-plus" }, STORED);
|
||||
assert.equal("LLM_BACKEND" in updates, false);
|
||||
});
|
||||
|
||||
test("the default rule does not touch the other keys", () => {
|
||||
// A blank secret still keeps the stored credential, and a key that must not
|
||||
// stay empty still gets its placeholder.
|
||||
const secret = envUpdates(
|
||||
{ LLM_API_KEY: "", LLM_BACKEND: "" },
|
||||
{ LLM_API_KEY: true, EMBEDDING_API_KEY: true },
|
||||
);
|
||||
assert.equal("LLM_API_KEY" in secret, false);
|
||||
assert.equal(secret.LLM_BACKEND, "openai");
|
||||
|
||||
const placeholder = envUpdates(
|
||||
{ EMBEDDING_API_KEY: "" },
|
||||
{ EMBEDDING_API_KEY: false },
|
||||
);
|
||||
assert.equal(placeholder.EMBEDDING_API_KEY, "EMPTY");
|
||||
|
||||
// A non-secret key without a default is still an explicit clear.
|
||||
const cleared = envUpdates({ LLM_BASE_URL: "" }, STORED);
|
||||
assert.deepEqual(cleared, { LLM_BASE_URL: "" });
|
||||
});
|
||||
|
||||
test("only the backend names a default", () => {
|
||||
// The embedding backend has no default: ReMe's embedding components are
|
||||
// commented out, so the plugin must not invent a rule for it.
|
||||
assert.equal(envFieldMeta("LLM_BACKEND")?.defaultValue, "openai");
|
||||
assert.equal(envFieldMeta("EMBEDDING_BACKEND")?.defaultValue, undefined);
|
||||
assert.equal(envFieldMeta("LLM_MODEL_NAME")?.defaultValue, undefined);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
|
|
@ -17,8 +17,30 @@ import {
|
|||
testLlm,
|
||||
validateEnvUpdate,
|
||||
} from "../dist/environment/host/workspace.js";
|
||||
import { ReMeEnvManager } from "../dist/environment/host/manager.js";
|
||||
import { MANAGED_KEYS } from "../dist/environment/env-fields.js";
|
||||
|
||||
/** A manager whose process work is replaced by facts. */
|
||||
function managerIn(t) {
|
||||
const directory = mkdtempSync(join(tmpdir(), "reme-env-save-"));
|
||||
t.after(() => rmSync(directory, { recursive: true, force: true }));
|
||||
const manager = new ReMeEnvManager(
|
||||
{ DSH_HOME: directory },
|
||||
{
|
||||
fetchHealth: async () => ({ ok: false, version: "", error: "" }),
|
||||
portOpen: async () => false,
|
||||
spawnService: async () => ({ pid: 1 }),
|
||||
killTree() {},
|
||||
probePid() {},
|
||||
processImage: async () => "python.exe",
|
||||
processCommandLine: async () => "",
|
||||
exists: () => true,
|
||||
platform: "win32",
|
||||
},
|
||||
);
|
||||
return { manager, directory };
|
||||
}
|
||||
|
||||
test("resolves the workspace directory with ~/.reme defaults", () => {
|
||||
const home = process.platform === "win32" ? "C:\\Users\\u" : "/home/u";
|
||||
assert.equal(resolveWorkspaceDir("", home), join(home, ".reme"));
|
||||
|
|
@ -390,3 +412,94 @@ test("a backend that ignores LLM_BASE_URL is probed with its own endpoint", asyn
|
|||
assert.match(openai, /missing\.append\("LLM_BASE_URL"\)/);
|
||||
assert.match(openai, /env\.get\("LLM_BASE_URL"/);
|
||||
});
|
||||
|
||||
test("a cleared backend is saved as the default, never as an empty value", (t) => {
|
||||
// ReMe resolves `${LLM_BACKEND:-openai}` only when the variable is absent.
|
||||
// An empty value would name no backend, and the service would refuse to
|
||||
// start, so the saved file must hold the default.
|
||||
const { manager, directory } = managerIn(t);
|
||||
const envPath = join(directory, ".env");
|
||||
writeFileSync(
|
||||
envPath,
|
||||
"LLM_BACKEND=dashscope\nLLM_API_KEY=sk-keep\nEMBEDDING_API_KEY=EMPTY\n",
|
||||
);
|
||||
|
||||
manager.writeEnv(directory, { LLM_BACKEND: "" });
|
||||
|
||||
const text = readFileSync(envPath, "utf8");
|
||||
assert.match(text, /^LLM_BACKEND=openai$/m);
|
||||
assert.doesNotMatch(text, /^LLM_BACKEND=$/m);
|
||||
// The other keys are untouched.
|
||||
assert.match(text, /^LLM_API_KEY=sk-keep$/m);
|
||||
assert.match(text, /^EMBEDDING_API_KEY=EMPTY$/m);
|
||||
});
|
||||
|
||||
test("the probe refuses a .env whose backend is empty", async (t) => {
|
||||
// An explicitly empty backend names no backend, and ReMe would start with
|
||||
// none at all. The honest answer is an incomplete configuration, and no
|
||||
// request is sent.
|
||||
const directory = mkdtempSync(join(tmpdir(), "reme-llm-empty-"));
|
||||
t.after(() => rmSync(directory, { recursive: true, force: true }));
|
||||
writeFileSync(
|
||||
join(directory, ".env"),
|
||||
[
|
||||
"LLM_BACKEND=",
|
||||
"LLM_MODEL_NAME=test-model",
|
||||
"LLM_API_KEY=sk-test",
|
||||
"LLM_BASE_URL=https://provider.invalid",
|
||||
].join("\n"),
|
||||
);
|
||||
const calls = [];
|
||||
const result = await testLlm(
|
||||
{
|
||||
run: async (exe, args, options) => {
|
||||
calls.push({ exe, args, options });
|
||||
return {
|
||||
code: 0,
|
||||
stdout: '{"ok": true, "status": 200, "latencyMs": 12, "error": ""}\n',
|
||||
stderr: "",
|
||||
};
|
||||
},
|
||||
},
|
||||
{ workspaceDir: directory, pythonPath: "C:\\env\\python.exe" },
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
ok: false,
|
||||
status: 0,
|
||||
latencyMs: 0,
|
||||
error: "LLM config incomplete (LLM_BACKEND)",
|
||||
});
|
||||
assert.equal(calls.length, 0, "the snippet is not run");
|
||||
});
|
||||
|
||||
test("a .env without the backend key is probed with the default protocol", async (t) => {
|
||||
// The key is absent, so ReMe applies its default; the probe does the same.
|
||||
const directory = mkdtempSync(join(tmpdir(), "reme-llm-absent-"));
|
||||
t.after(() => rmSync(directory, { recursive: true, force: true }));
|
||||
writeFileSync(
|
||||
join(directory, ".env"),
|
||||
[
|
||||
"LLM_MODEL_NAME=test-model",
|
||||
"LLM_API_KEY=sk-test",
|
||||
"LLM_BASE_URL=https://provider.invalid",
|
||||
].join("\n"),
|
||||
);
|
||||
const calls = [];
|
||||
await testLlm(
|
||||
{
|
||||
run: async (exe, args, options) => {
|
||||
calls.push({ exe, args, options });
|
||||
return {
|
||||
code: 0,
|
||||
stdout: '{"ok": true, "status": 200, "latencyMs": 12, "error": ""}\n',
|
||||
stderr: "",
|
||||
};
|
||||
},
|
||||
},
|
||||
{ workspaceDir: directory, pythonPath: "C:\\env\\python.exe" },
|
||||
);
|
||||
const snippet = String(calls[0]?.args[1] ?? "");
|
||||
assert.match(snippet, /\/chat\/completions/);
|
||||
assert.match(snippet, /"Authorization"/);
|
||||
assert.match(snippet, /Bearer /);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue