fix(desktop): treat built-in memory as built-in in provider panel (#49513)
Built-in memory (MEMORY.md/USER.md) is controlled by memory_enabled, not memory.provider — but the desktop dropdown offered 'builtin' as a normal provider-plugin value and gave it plugin-shaped affordances (config panel, OAuth connect row), and the empty sentinel rendered as '(none)' even though built-in memory was active. - Label the empty memory.provider option 'Built-in only' (all locales). - Drop the literal 'builtin' option from the desktop ENUM_OPTIONS and the backend config-schema select; _normalize_memory_provider_name already maps legacy builtin/built-in/none values to ''. A stored legacy literal stays visible via enumOptionsFor's current-value passthrough. - Gate MemoryConnect and ProviderConfigPanel behind a new isExternalMemoryProvider() helper so built-in aliases never get provider-plugin affordances.
This commit is contained in:
parent
7ced2ee394
commit
651cff4273
|
|
@ -21,7 +21,14 @@ import { PanelEmpty } from '../overlays/panel'
|
|||
import { CONTROL_TEXT, EMPTY_SELECT_VALUE, FIELD_DESCRIPTIONS, FIELD_LABELS } from './constants'
|
||||
import { FallbackModelsField } from './fallback-models-field'
|
||||
import { fieldCopyForSchemaKey } from './field-copy'
|
||||
import { enumOptionsFor, getNested, prettyName, sectionFieldEntries, setNested } from './helpers'
|
||||
import {
|
||||
enumOptionsFor,
|
||||
getNested,
|
||||
isExternalMemoryProvider,
|
||||
prettyName,
|
||||
sectionFieldEntries,
|
||||
setNested
|
||||
} from './helpers'
|
||||
import { MemoryConnect } from './memory/connect'
|
||||
import { ProviderConfigPanel } from './memory/provider-config-panel'
|
||||
import { ModelSettings, ModelSettingsSkeleton } from './model-settings'
|
||||
|
|
@ -134,7 +141,9 @@ function ConfigField({
|
|||
? (optionLabels?.[option] ?? prettyName(option))
|
||||
: schemaKey === 'display.personality'
|
||||
? c.none
|
||||
: c.noneParen}
|
||||
: schemaKey === 'memory.provider'
|
||||
? c.builtinOnly
|
||||
: c.noneParen}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
|
|
@ -457,7 +466,7 @@ export function ConfigSettings({
|
|||
<div className="scroll-mt-6 rounded-lg" id={`setting-field-${key}`} key={key}>
|
||||
<ConfigField
|
||||
descriptionExtra={
|
||||
key === 'memory.provider' && Boolean(getNested(config, key)) ? (
|
||||
key === 'memory.provider' && isExternalMemoryProvider(getNested(config, key)) ? (
|
||||
<MemoryConnect provider={String(getNested(config, key))} />
|
||||
) : undefined
|
||||
}
|
||||
|
|
@ -472,7 +481,7 @@ export function ConfigSettings({
|
|||
schemaKey={key}
|
||||
value={getNested(config, key)}
|
||||
/>
|
||||
{key === 'memory.provider' && typeof getNested(config, key) === 'string' && getNested(config, key) ? (
|
||||
{key === 'memory.provider' && isExternalMemoryProvider(getNested(config, key)) ? (
|
||||
<ProviderConfigPanel key={String(getNested(config, key))} provider={String(getNested(config, key))} />
|
||||
) : null}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -247,7 +247,10 @@ export const ENUM_OPTIONS: Record<string, string[]> = {
|
|||
'code_execution.mode': ['project', 'strict'],
|
||||
'context.engine': ['compressor', 'default', 'custom'],
|
||||
'delegation.reasoning_effort': ['', 'minimal', 'low', 'medium', 'high', 'xhigh', 'max', 'ultra'],
|
||||
'memory.provider': ['', 'builtin', 'honcho', 'hindsight'],
|
||||
// Built-in memory is not a provider plugin: the empty sentinel renders as
|
||||
// "Built-in only" and a legacy literal `builtin` value is only kept visible
|
||||
// via enumOptionsFor's current-value passthrough (#49513).
|
||||
'memory.provider': ['', 'honcho', 'hindsight'],
|
||||
// Terminal execution backends — kept in sync with the dispatch ladder in
|
||||
// tools/terminal_tool.py::_create_environment (local/docker/singularity/
|
||||
// modal/daytona/ssh). Remote backends need extra env (image, tokens, host).
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { defineFieldCopy, fieldCopyForSchemaKey, schemaKeyToFieldCopyKey } from
|
|||
import {
|
||||
enumOptionsFor,
|
||||
getNested,
|
||||
isExternalMemoryProvider,
|
||||
providerGroup,
|
||||
sectionFieldEntries,
|
||||
setNested,
|
||||
|
|
@ -17,7 +18,28 @@ describe('settings helpers', () => {
|
|||
it('lists the desktop memory provider options in their declared order', () => {
|
||||
const options = enumOptionsFor('memory.provider', '', {})
|
||||
|
||||
expect(options).toEqual(['', 'builtin', 'honcho', 'hindsight'])
|
||||
// Built-in memory is not a provider plugin; the empty sentinel is the
|
||||
// only built-in-shaped entry (#49513).
|
||||
expect(options).toEqual(['', 'honcho', 'hindsight'])
|
||||
})
|
||||
|
||||
it('keeps a legacy literal builtin value visible as the current selection', () => {
|
||||
const options = enumOptionsFor('memory.provider', 'builtin', {})
|
||||
|
||||
expect(options).toEqual(['', 'honcho', 'hindsight', 'builtin'])
|
||||
})
|
||||
|
||||
describe('isExternalMemoryProvider', () => {
|
||||
it('treats only real plugin names as external providers', () => {
|
||||
expect(isExternalMemoryProvider('honcho')).toBe(true)
|
||||
expect(isExternalMemoryProvider('hindsight')).toBe(true)
|
||||
})
|
||||
|
||||
it('treats built-in aliases and empty values as not external', () => {
|
||||
for (const value of ['', 'builtin', 'built-in', 'Builtin', 'none', ' ', undefined, null, 7]) {
|
||||
expect(isExternalMemoryProvider(value)).toBe(false)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('defineFieldCopy', () => {
|
||||
|
|
|
|||
|
|
@ -182,3 +182,16 @@ export function enumOptionsFor(
|
|||
|
||||
return current && !opts.includes(current) ? [...opts, current] : opts
|
||||
}
|
||||
|
||||
// Built-in memory (MEMORY.md/USER.md) is controlled by memory_enabled, not
|
||||
// memory.provider — only a real external plugin name gets provider-shaped
|
||||
// affordances (config panel, OAuth connect). See #49513.
|
||||
export function isExternalMemoryProvider(value: unknown): value is string {
|
||||
if (typeof value !== 'string') {
|
||||
return false
|
||||
}
|
||||
|
||||
const normalized = value.trim().toLowerCase()
|
||||
|
||||
return normalized !== '' && normalized !== 'builtin' && normalized !== 'built-in' && normalized !== 'none'
|
||||
}
|
||||
|
|
|
|||
|
|
@ -513,6 +513,7 @@ export const en: Translations = {
|
|||
config: {
|
||||
none: 'None',
|
||||
noneParen: '(none)',
|
||||
builtinOnly: 'Built-in only',
|
||||
notSet: 'Not set',
|
||||
commaSeparated: 'comma-separated values',
|
||||
loading: 'Loading Hermes configuration...',
|
||||
|
|
|
|||
|
|
@ -606,6 +606,7 @@ export const ja = defineLocale({
|
|||
config: {
|
||||
none: 'なし',
|
||||
noneParen: '(なし)',
|
||||
builtinOnly: '内蔵のみ',
|
||||
notSet: '未設定',
|
||||
commaSeparated: 'カンマ区切りの値',
|
||||
loading: 'Hermes の設定を読み込み中...',
|
||||
|
|
|
|||
|
|
@ -424,6 +424,7 @@ export interface Translations {
|
|||
config: {
|
||||
none: string
|
||||
noneParen: string
|
||||
builtinOnly: string
|
||||
notSet: string
|
||||
commaSeparated: string
|
||||
loading: string
|
||||
|
|
|
|||
|
|
@ -594,6 +594,7 @@ export const zhHant = defineLocale({
|
|||
config: {
|
||||
none: '無',
|
||||
noneParen: '(無)',
|
||||
builtinOnly: '僅內建',
|
||||
notSet: '未設定',
|
||||
commaSeparated: '逗號分隔的值',
|
||||
loading: '正在載入 Hermes 設定...',
|
||||
|
|
|
|||
|
|
@ -705,6 +705,7 @@ export const zh: Translations = {
|
|||
config: {
|
||||
none: '无',
|
||||
noneParen: '(无)',
|
||||
builtinOnly: '仅内置',
|
||||
notSet: '未设置',
|
||||
commaSeparated: '逗号分隔的值',
|
||||
loading: '正在加载 Hermes 配置...',
|
||||
|
|
|
|||
|
|
@ -622,10 +622,13 @@ def _memory_provider_options() -> List[str]:
|
|||
"""Discovered memory providers for the ``memory.provider`` select.
|
||||
|
||||
Directory-scan only (no provider imports), so it's safe at module import
|
||||
time. ``""`` (built-in) is always first; discovery failures degrade to the
|
||||
bundled defaults rather than dropping the field.
|
||||
time. ``""`` (built-in only) is always first; discovery failures degrade to
|
||||
the bundled defaults rather than dropping the field. The literal
|
||||
``builtin`` alias is deliberately NOT offered — built-in memory is not a
|
||||
provider plugin, and ``_normalize_memory_provider_name`` already maps any
|
||||
legacy ``builtin``/``built-in``/``none`` value back to ``""`` (#49513).
|
||||
"""
|
||||
options = ["", "builtin"]
|
||||
options = [""]
|
||||
try:
|
||||
from plugins.memory import list_memory_provider_names
|
||||
|
||||
|
|
|
|||
|
|
@ -4148,10 +4148,12 @@ class TestBuildSchemaFromConfig:
|
|||
assert entry["type"] == "select"
|
||||
assert entry["category"] == "memory"
|
||||
options = entry["options"]
|
||||
# Built-in sentinel first, plus at least one discovered provider.
|
||||
# Built-in-only sentinel first, plus at least one discovered provider.
|
||||
# The literal "builtin" alias must NOT be offered — built-in memory is
|
||||
# not a provider plugin (#49513).
|
||||
assert options[0] == ""
|
||||
assert "builtin" in options
|
||||
assert len(options) >= 3
|
||||
assert "builtin" not in options
|
||||
assert len(options) >= 2
|
||||
|
||||
def test_memory_provider_options_cover_discovered_providers(self):
|
||||
"""Every provider the /api/memory endpoint can activate is selectable."""
|
||||
|
|
|
|||
Loading…
Reference in New Issue