mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(moa): reject half-filled MoA saves at the API boundary and hold desktop autosave until slots are complete
Follow-up hardening on top of #64158 (@DavidMetcalfe): Backend (the root-cause fix): - hermes_cli/moa_config.py: add validate_moa_payload() — strict write-time counterpart to the deliberately tolerant normalize_moa_config(). Flags half-filled slots, empty reference lists, recursive moa slots, naming the exact preset/slot. - hermes_cli/web_server.py: PUT /api/model/moa validates before normalizing and returns 422 with the specific problems instead of silently swapping the user's preset for hardcoded defaults (#64156). Also declares fanout / reference_max_tokens / reasoning_effort on the Pydantic payload so client round-trips no longer erase hand-set values. Desktop: - Replace sanitize-then-send with hold-while-incomplete: the debounced autosave is deferred (not repaired) while any slot is half-filled, and flushes once the model pick completes the edit. Mid-edit UI state is never repainted by a save response (generation guard covers held edits too). - updateMoaSlot only clears the model when the provider actually changed. - Explicit preset ops (set default / add / delete) cancel the pending autosave and invalidate in-flight responses so the two writers can't race. - Stable row keys (preset+index) so mid-edit rows don't remount; cleared model shows the 'Model' placeholder instead of vanishing. Both TS clients' MoaConfigResponse types now declare the round-tripped fields (fanout, reference_max_tokens, reasoning_effort). Tests: 12 new backend unit tests (validate_moa_payload contract incl. validate/normalize agreement), 3 new web_server endpoint tests (422 on half-filled ref/aggregator, fanout round-trip), 3 new desktop vitest cases (autosave held while half-filled, flush on completion, same-provider reselect no-op). E2E validated against a live TestClient with isolated HERMES_HOME: bug sequence now 422s with config untouched. Fixes #64156
This commit is contained in:
Teknium
parent
a61a0bc019
commit
fcdc10a0f3
8 changed files with 500 additions and 54 deletions
|
|
@ -186,3 +186,139 @@ describe('ModelSettings', () => {
|
|||
expect(await screen.findByText(/still run on/)).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
||||
describe('ModelSettings MoA preset editor', () => {
|
||||
const moaConfig = () => ({
|
||||
default_preset: 'default',
|
||||
active_preset: '',
|
||||
presets: {
|
||||
default: {
|
||||
reference_models: [
|
||||
{ provider: 'nous', model: 'hermes-4' },
|
||||
{ provider: 'openrouter', model: 'deepseek/deepseek-v4-pro' }
|
||||
],
|
||||
aggregator: { provider: 'openrouter', model: 'anthropic/claude-opus-4.8' },
|
||||
reference_temperature: 0,
|
||||
aggregator_temperature: 0,
|
||||
max_tokens: 4096,
|
||||
enabled: true
|
||||
}
|
||||
},
|
||||
reference_models: [
|
||||
{ provider: 'nous', model: 'hermes-4' },
|
||||
{ provider: 'openrouter', model: 'deepseek/deepseek-v4-pro' }
|
||||
],
|
||||
aggregator: { provider: 'openrouter', model: 'anthropic/claude-opus-4.8' },
|
||||
reference_temperature: 0,
|
||||
aggregator_temperature: 0,
|
||||
max_tokens: 4096,
|
||||
enabled: true
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
getGlobalModelOptions.mockResolvedValue({
|
||||
providers: [
|
||||
{
|
||||
name: 'Nous',
|
||||
slug: 'nous',
|
||||
models: ['hermes-4', 'hermes-4-mini'],
|
||||
authenticated: true,
|
||||
capabilities: { 'hermes-4': { reasoning: true, fast: true } }
|
||||
},
|
||||
{
|
||||
name: 'OpenRouter',
|
||||
slug: 'openrouter',
|
||||
models: ['deepseek/deepseek-v4-pro', 'anthropic/claude-opus-4.8'],
|
||||
authenticated: true
|
||||
}
|
||||
]
|
||||
})
|
||||
getMoaModels.mockResolvedValue(moaConfig())
|
||||
saveMoaModels.mockImplementation((body: unknown) => Promise.resolve(body))
|
||||
})
|
||||
|
||||
async function openReferenceEditor() {
|
||||
await renderModelSettings()
|
||||
expect(await screen.findByText('Reference 1')).toBeTruthy()
|
||||
}
|
||||
|
||||
function slotSelects() {
|
||||
// Combobox order in the MoA section (last 7 on the page): preset select,
|
||||
// then provider+model per reference (2 refs), then aggregator
|
||||
// provider+model. Reference 1's pair is therefore at -6 / -5.
|
||||
const all = screen.getAllByRole('combobox')
|
||||
|
||||
return { ref1Provider: all.at(-6)!, ref1Model: all.at(-5)! }
|
||||
}
|
||||
|
||||
it('holds the autosave while a slot is half-filled (provider changed, model pending)', async () => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true })
|
||||
|
||||
try {
|
||||
await openReferenceEditor()
|
||||
|
||||
fireEvent.click(slotSelects().ref1Provider)
|
||||
fireEvent.click(await screen.findByRole('option', { name: 'OpenRouter' }))
|
||||
|
||||
// Model was cleared by the provider change → config incomplete → the
|
||||
// debounced autosave must NOT fire, even well past the 600ms window.
|
||||
await vi.advanceTimersByTimeAsync(2000)
|
||||
expect(saveMoaModels).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('saves once the model pick completes the slot', async () => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true })
|
||||
|
||||
try {
|
||||
await openReferenceEditor()
|
||||
|
||||
fireEvent.click(slotSelects().ref1Provider)
|
||||
fireEvent.click(await screen.findByRole('option', { name: 'OpenRouter' }))
|
||||
await vi.advanceTimersByTimeAsync(700)
|
||||
|
||||
fireEvent.click(slotSelects().ref1Model)
|
||||
fireEvent.click(await screen.findByRole('option', { name: 'anthropic/claude-opus-4.8' }))
|
||||
await vi.advanceTimersByTimeAsync(700)
|
||||
|
||||
expect(saveMoaModels).toHaveBeenCalledTimes(1)
|
||||
const sent = saveMoaModels.mock.calls[0][0] as ReturnType<typeof moaConfig>
|
||||
expect(sent.presets.default.reference_models[0]).toEqual({
|
||||
provider: 'openrouter',
|
||||
model: 'anthropic/claude-opus-4.8'
|
||||
})
|
||||
// The untouched slots ride along unchanged — nothing reverts to defaults.
|
||||
expect(sent.presets.default.reference_models[1]).toEqual({
|
||||
provider: 'openrouter',
|
||||
model: 'deepseek/deepseek-v4-pro'
|
||||
})
|
||||
expect(sent.presets.default.aggregator).toEqual({
|
||||
provider: 'openrouter',
|
||||
model: 'anthropic/claude-opus-4.8'
|
||||
})
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not clear the model or save when the same provider is re-selected', async () => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true })
|
||||
|
||||
try {
|
||||
await openReferenceEditor()
|
||||
|
||||
fireEvent.click(slotSelects().ref1Provider)
|
||||
fireEvent.click(await screen.findByRole('option', { name: 'Nous' }))
|
||||
await vi.advanceTimersByTimeAsync(700)
|
||||
|
||||
// Radix treats re-picking the current value as a no-op (no
|
||||
// onValueChange), so nothing changes: no save, model still shown.
|
||||
expect(saveMoaModels).not.toHaveBeenCalled()
|
||||
expect(screen.getByText('nous · hermes-4')).toBeTruthy()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -130,6 +130,24 @@ const NO_PROVIDERS: readonly ModelOptionProvider[] = [{ name: '—', slug: '', m
|
|||
export const withActive = (models: readonly string[], active: string): readonly string[] =>
|
||||
active && !models.includes(active) ? [active, ...models] : models
|
||||
|
||||
// A slot is complete when both halves are chosen. Changing a slot's provider
|
||||
// intentionally clears its model (see updateMoaSlot), so every provider change
|
||||
// passes through an incomplete state while the user picks the new model.
|
||||
export const moaSlotComplete = (slot: MoaModelSlot): boolean => !!(slot.provider.trim() && slot.model.trim())
|
||||
|
||||
// True when every slot in every preset is fully specified — the only state
|
||||
// that is safe to persist. The backend rejects configs with half-filled slots
|
||||
// (HTTP 422) instead of silently swapping the preset for hardcoded defaults
|
||||
// (#64156), so the autosave must simply wait for the edit to finish rather
|
||||
// than trying to "repair" the payload.
|
||||
export const moaConfigComplete = (config: MoaConfigResponse): boolean =>
|
||||
Object.values(config.presets).every(
|
||||
preset =>
|
||||
preset.reference_models.length > 0 &&
|
||||
preset.reference_models.every(moaSlotComplete) &&
|
||||
moaSlotComplete(preset.aggregator)
|
||||
)
|
||||
|
||||
interface StaleAuxWarningProps {
|
||||
applying: boolean
|
||||
onReset: () => void
|
||||
|
|
@ -320,49 +338,31 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
// Guard against stale save responses overwriting newer state.
|
||||
const moaSaveGeneration = useRef(0)
|
||||
|
||||
// Strip slots with an empty model from every preset before autosave, so a
|
||||
// half-filled slot (provider selected but no model yet) is never sent to the
|
||||
// backend. Without this guard, _clean_slot rejects the empty-model slot and
|
||||
// _normalize_preset falls back to hardcoded defaults. Both reference and
|
||||
// aggregator slots are sanitized. Presets keep their empty reference_models
|
||||
// array rather than being dropped.
|
||||
const sanitizeMoaRefsForSave = useCallback((config: MoaConfigResponse): MoaConfigResponse => {
|
||||
const presets: MoaConfigResponse['presets'] = {}
|
||||
let changed = false
|
||||
|
||||
for (const [name, preset] of Object.entries(config.presets)) {
|
||||
const refs = preset.reference_models.filter(slot => slot.provider.trim() && slot.model.trim())
|
||||
if (refs.length !== preset.reference_models.length) {
|
||||
changed = true
|
||||
}
|
||||
const agg = preset.aggregator
|
||||
const aggValid = agg && agg.provider.trim() && agg.model.trim()
|
||||
const cleanAgg = aggValid ? agg : { provider: agg?.provider ?? '', model: agg?.model ?? '' }
|
||||
|
||||
presets[name] = {
|
||||
...preset,
|
||||
reference_models: refs,
|
||||
aggregator: cleanAgg
|
||||
}
|
||||
}
|
||||
|
||||
return changed ? { ...config, presets } : config
|
||||
}, [])
|
||||
|
||||
// Quiet debounced persist for inline MoA edits — mirrors the config page's
|
||||
// autosave so slot/aggregator tweaks save themselves, matching the
|
||||
// preset-level ops (set default / add / delete) that already persist on
|
||||
// click. No `applying` spinner, so selecting stays responsive.
|
||||
//
|
||||
// While any slot is half-filled (provider picked, model pending) the save is
|
||||
// HELD, not sent: the previous complete config stays on disk and the next
|
||||
// edit that completes the slot flushes the whole preset. Every edit bumps
|
||||
// the generation so an in-flight response from an older save can never
|
||||
// repaint over the user's mid-edit state.
|
||||
const scheduleMoaSave = useCallback((next: MoaConfigResponse) => {
|
||||
if (moaSaveTimer.current) {
|
||||
window.clearTimeout(moaSaveTimer.current)
|
||||
moaSaveTimer.current = null
|
||||
}
|
||||
|
||||
const generation = moaSaveGeneration.current + 1
|
||||
moaSaveGeneration.current = generation
|
||||
|
||||
if (!moaConfigComplete(next)) {
|
||||
return
|
||||
}
|
||||
|
||||
moaSaveTimer.current = window.setTimeout(() => {
|
||||
void saveMoaModels(sanitizeMoaRefsForSave(next))
|
||||
void saveMoaModels(next)
|
||||
.then(saved => {
|
||||
if (moaSaveGeneration.current === generation) {
|
||||
setMoa(saved)
|
||||
|
|
@ -374,7 +374,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
}
|
||||
})
|
||||
}, 600)
|
||||
}, [sanitizeMoaRefsForSave])
|
||||
}, [])
|
||||
|
||||
const updateMoaPreset = useCallback(
|
||||
(updater: (preset: NonNullable<typeof currentMoaPreset>) => NonNullable<typeof currentMoaPreset>) => {
|
||||
|
|
@ -402,7 +402,10 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
const updateMoaSlot = useCallback((slot: MoaModelSlot, patch: Partial<MoaModelSlot>): MoaModelSlot => {
|
||||
const next = { ...slot, ...patch }
|
||||
|
||||
if (patch.provider) {
|
||||
// Picking a new provider invalidates the model choice (models are
|
||||
// per-provider). A same-provider update must not wipe the model — Radix
|
||||
// filters same-value changes, but programmatic callers may not.
|
||||
if (patch.provider && patch.provider !== slot.provider) {
|
||||
next.model = ''
|
||||
}
|
||||
|
||||
|
|
@ -411,6 +414,16 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
|
||||
const saveMoa = useCallback(async (next: MoaConfigResponse) => {
|
||||
const epoch = profileEpoch.current
|
||||
|
||||
// Explicit preset ops (set default / add / delete) supersede any pending
|
||||
// debounced slot autosave — cancel it and invalidate in-flight responses
|
||||
// so the two writers can't race each other's state.
|
||||
if (moaSaveTimer.current) {
|
||||
window.clearTimeout(moaSaveTimer.current)
|
||||
moaSaveTimer.current = null
|
||||
}
|
||||
|
||||
moaSaveGeneration.current += 1
|
||||
setApplying(true)
|
||||
setError('')
|
||||
|
||||
|
|
@ -1036,6 +1049,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
<SelectContent>
|
||||
{withActive(moaSlotProviderOptions.map(p => p.slug || 'none'), slot.provider).map(slug => {
|
||||
const provider = moaSlotProviderOptions.find(p => (p.slug || 'none') === slug)
|
||||
|
||||
return (
|
||||
<SelectItem key={slug} value={slug}>
|
||||
{provider?.name || slug}
|
||||
|
|
@ -1083,10 +1097,10 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
}
|
||||
description={
|
||||
<span className="font-mono text-[0.68rem]">
|
||||
{slot.provider} · {slot.model}
|
||||
{slot.provider} · {slot.model || m.model}
|
||||
</span>
|
||||
}
|
||||
key={`${selectedMoaPreset}-${slot.provider}-${slot.model}-${index}`}
|
||||
key={`${selectedMoaPreset}-${index}`}
|
||||
title={`Reference ${index + 1}`}
|
||||
/>
|
||||
))}
|
||||
|
|
@ -1118,6 +1132,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
<SelectContent>
|
||||
{withActive(moaSlotProviderOptions.map(p => p.slug || 'none'), currentMoaPreset.aggregator.provider).map(slug => {
|
||||
const provider = moaSlotProviderOptions.find(p => (p.slug || 'none') === slug)
|
||||
|
||||
return (
|
||||
<SelectItem key={slug} value={slug}>
|
||||
{provider?.name || slug}
|
||||
|
|
|
|||
|
|
@ -857,6 +857,8 @@ export interface AuxiliaryModelsResponse {
|
|||
export interface MoaModelSlot {
|
||||
provider: string
|
||||
model: string
|
||||
/** Optional per-slot reasoning effort — round-tripped, not edited here. */
|
||||
reasoning_effort?: string
|
||||
}
|
||||
|
||||
export interface MoaConfigResponse {
|
||||
|
|
@ -871,6 +873,10 @@ export interface MoaConfigResponse {
|
|||
max_tokens: number
|
||||
reference_models: MoaModelSlot[]
|
||||
reference_temperature: number
|
||||
/** Optional advisor output cap — round-tripped, not edited here. */
|
||||
reference_max_tokens?: number | null
|
||||
/** Fan-out cadence (per_iteration | user_turn) — round-tripped. */
|
||||
fanout?: string
|
||||
}
|
||||
>
|
||||
aggregator: MoaModelSlot
|
||||
|
|
|
|||
|
|
@ -110,6 +110,77 @@ def _clean_slot(slot: Any) -> dict[str, Any] | None:
|
|||
return clean
|
||||
|
||||
|
||||
def _slot_problem(slot: Any) -> str | None:
|
||||
"""Return a human-readable problem for a slot ``_clean_slot`` would drop.
|
||||
|
||||
None means the slot is complete and valid. Mirrors ``_clean_slot`` exactly
|
||||
so the write-boundary validator (``validate_moa_payload``) and the
|
||||
tolerant runtime normalizer can never disagree about what is acceptable.
|
||||
"""
|
||||
if not isinstance(slot, dict):
|
||||
return "must be an object with 'provider' and 'model'"
|
||||
provider = str(slot.get("provider") or "").strip()
|
||||
model = str(slot.get("model") or "").strip()
|
||||
if not provider and not model:
|
||||
return "provider and model are required"
|
||||
if not provider:
|
||||
return "provider is required"
|
||||
if not model:
|
||||
return f"model is required (provider '{provider}' has no model selected)"
|
||||
if provider.lower() == "moa":
|
||||
return "the Mixture of Agents provider cannot be used inside a preset (recursive MoA)"
|
||||
return None
|
||||
|
||||
|
||||
def validate_moa_payload(raw: Any) -> list[str]:
|
||||
"""Return the problems ``normalize_moa_config`` would silently paper over.
|
||||
|
||||
``normalize_moa_config`` is deliberately tolerant: at *read* time a
|
||||
hand-edited config must degrade to defaults rather than crash the agent.
|
||||
That same tolerance at *write* time is a corruption engine — a client that
|
||||
sends a half-filled slot gets its whole preset silently replaced with the
|
||||
hardcoded defaults (#64156). API write paths call this first and reject
|
||||
invalid payloads loudly instead of saving something the user never chose.
|
||||
|
||||
Returns a list of human-readable problems; empty means safe to save.
|
||||
"""
|
||||
if not isinstance(raw, dict):
|
||||
return ["MoA config must be an object"]
|
||||
|
||||
presets_raw = raw.get("presets")
|
||||
if isinstance(presets_raw, dict) and presets_raw:
|
||||
presets: dict[Any, Any] = presets_raw
|
||||
else:
|
||||
# Legacy flat payload: the top-level object is the default preset.
|
||||
presets = {DEFAULT_MOA_PRESET_NAME: raw}
|
||||
|
||||
problems: list[str] = []
|
||||
for name, preset in presets.items():
|
||||
label = str(name or "").strip() or "(unnamed)"
|
||||
if not isinstance(preset, dict):
|
||||
problems.append(f"preset '{label}': must be an object")
|
||||
continue
|
||||
|
||||
refs = preset.get("reference_models")
|
||||
if not isinstance(refs, list):
|
||||
refs = [refs] if isinstance(refs, dict) else []
|
||||
complete_refs = 0
|
||||
for index, slot in enumerate(refs):
|
||||
issue = _slot_problem(slot)
|
||||
if issue:
|
||||
problems.append(f"preset '{label}' reference {index + 1}: {issue}")
|
||||
else:
|
||||
complete_refs += 1
|
||||
if not complete_refs:
|
||||
problems.append(f"preset '{label}': needs at least one complete reference model")
|
||||
|
||||
agg_issue = _slot_problem(preset.get("aggregator"))
|
||||
if agg_issue:
|
||||
problems.append(f"preset '{label}' aggregator: {agg_issue}")
|
||||
|
||||
return problems
|
||||
|
||||
|
||||
def _default_preset() -> dict[str, Any]:
|
||||
return {
|
||||
"reference_models": deepcopy(DEFAULT_MOA_REFERENCE_MODELS),
|
||||
|
|
|
|||
|
|
@ -996,6 +996,9 @@ class ModelAssignment(BaseModel):
|
|||
class MoaModelSlot(BaseModel):
|
||||
provider: str = ""
|
||||
model: str = ""
|
||||
# Optional per-slot reasoning effort. Declared so a client round-tripping
|
||||
# the GET payload doesn't have it stripped at parse time and wiped on save.
|
||||
reasoning_effort: Optional[str] = None
|
||||
|
||||
|
||||
class MoaPresetPayload(BaseModel):
|
||||
|
|
@ -1006,6 +1009,11 @@ class MoaPresetPayload(BaseModel):
|
|||
reference_temperature: Optional[float] = None
|
||||
aggregator_temperature: Optional[float] = None
|
||||
max_tokens: int = 4096
|
||||
# Newer per-preset knobs (see moa_config._normalize_preset). Optional so
|
||||
# older clients that never send them keep working; declared so clients
|
||||
# that round-trip the GET payload don't silently erase hand-set values.
|
||||
reference_max_tokens: Optional[int] = None
|
||||
fanout: Optional[str] = None
|
||||
enabled: bool = True
|
||||
|
||||
|
||||
|
|
@ -1020,6 +1028,8 @@ class MoaConfigPayload(BaseModel):
|
|||
reference_temperature: Optional[float] = None
|
||||
aggregator_temperature: Optional[float] = None
|
||||
max_tokens: int = 4096
|
||||
reference_max_tokens: Optional[int] = None
|
||||
fanout: Optional[str] = None
|
||||
enabled: bool = True
|
||||
profile: Optional[str] = None
|
||||
|
||||
|
|
@ -5701,7 +5711,23 @@ def get_moa_models(profile: Optional[str] = None):
|
|||
def set_moa_models(body: MoaConfigPayload, profile: Optional[str] = None):
|
||||
"""Persist the Mixture-of-Agents provider/model slots."""
|
||||
try:
|
||||
from hermes_cli.moa_config import normalize_moa_config
|
||||
from hermes_cli.moa_config import normalize_moa_config, validate_moa_payload
|
||||
|
||||
def _slot_dict(slot: MoaModelSlot) -> dict:
|
||||
# Drop unset optionals so saved slots stay minimal ({provider, model}).
|
||||
return {k: v for k, v in slot.dict().items() if v is not None}
|
||||
|
||||
def _preset_dict(preset: MoaPresetPayload) -> dict:
|
||||
return {
|
||||
"reference_models": [_slot_dict(slot) for slot in preset.reference_models],
|
||||
"aggregator": _slot_dict(preset.aggregator),
|
||||
"reference_temperature": preset.reference_temperature,
|
||||
"aggregator_temperature": preset.aggregator_temperature,
|
||||
"max_tokens": preset.max_tokens,
|
||||
"reference_max_tokens": preset.reference_max_tokens,
|
||||
"fanout": preset.fanout,
|
||||
"enabled": preset.enabled,
|
||||
}
|
||||
|
||||
with _profile_scope(body.profile or profile):
|
||||
cfg = load_config()
|
||||
|
|
@ -5709,27 +5735,35 @@ def set_moa_models(body: MoaConfigPayload, profile: Optional[str] = None):
|
|||
raw = {
|
||||
"default_preset": body.default_preset,
|
||||
"active_preset": body.active_preset,
|
||||
"presets": {
|
||||
name: {
|
||||
"reference_models": [slot.dict() for slot in preset.reference_models],
|
||||
"aggregator": preset.aggregator.dict(),
|
||||
"reference_temperature": preset.reference_temperature,
|
||||
"aggregator_temperature": preset.aggregator_temperature,
|
||||
"max_tokens": preset.max_tokens,
|
||||
"enabled": preset.enabled,
|
||||
}
|
||||
for name, preset in body.presets.items()
|
||||
},
|
||||
"presets": {name: _preset_dict(preset) for name, preset in body.presets.items()},
|
||||
}
|
||||
else:
|
||||
raw = {
|
||||
"reference_models": [slot.dict() for slot in body.reference_models],
|
||||
"aggregator": body.aggregator.dict(),
|
||||
"reference_temperature": body.reference_temperature,
|
||||
"aggregator_temperature": body.aggregator_temperature,
|
||||
"max_tokens": body.max_tokens,
|
||||
"enabled": body.enabled,
|
||||
}
|
||||
raw = _preset_dict(
|
||||
MoaPresetPayload(
|
||||
reference_models=body.reference_models,
|
||||
aggregator=body.aggregator,
|
||||
reference_temperature=body.reference_temperature,
|
||||
aggregator_temperature=body.aggregator_temperature,
|
||||
max_tokens=body.max_tokens,
|
||||
reference_max_tokens=body.reference_max_tokens,
|
||||
fanout=body.fanout,
|
||||
enabled=body.enabled,
|
||||
)
|
||||
)
|
||||
|
||||
# Reject-don't-repair: normalize_moa_config() silently swaps any
|
||||
# preset containing incomplete slots for the hardcoded defaults —
|
||||
# correct tolerance for hand-edited configs at READ time, silent
|
||||
# data loss at WRITE time (#64156: desktop autosave of a
|
||||
# half-filled slot replaced the user's whole preset). Refuse the
|
||||
# save loudly so no client can corrupt config through this route.
|
||||
problems = validate_moa_payload(raw)
|
||||
if problems:
|
||||
raise HTTPException(
|
||||
status_code=422,
|
||||
detail="Invalid MoA config: " + "; ".join(problems),
|
||||
)
|
||||
|
||||
normalized = normalize_moa_config(raw)
|
||||
cfg["moa"] = normalized
|
||||
save_config(cfg)
|
||||
|
|
|
|||
|
|
@ -304,3 +304,115 @@ def test_reference_max_tokens_in_flattened_view():
|
|||
active preset's reference_max_tokens."""
|
||||
cfg = normalize_moa_config(_preset(reference_max_tokens=750))
|
||||
assert cfg["reference_max_tokens"] == 750
|
||||
|
||||
|
||||
# ── validate_moa_payload (write-boundary validation, #64156) ─────────────────
|
||||
#
|
||||
# normalize_moa_config is deliberately tolerant at READ time (hand-edited
|
||||
# configs degrade to defaults). validate_moa_payload is the strict WRITE-time
|
||||
# counterpart: it must flag exactly the payloads normalize would silently
|
||||
# repair, so API save paths reject them instead of corrupting user config.
|
||||
|
||||
|
||||
def _valid_preset_payload():
|
||||
return {
|
||||
"reference_models": [{"provider": "openrouter", "model": "deepseek/deepseek-v4-pro"}],
|
||||
"aggregator": {"provider": "openrouter", "model": "anthropic/claude-opus-4.8"},
|
||||
}
|
||||
|
||||
|
||||
def test_validate_moa_payload_accepts_complete_presets():
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
assert validate_moa_payload({"presets": {"default": _valid_preset_payload()}}) == []
|
||||
|
||||
|
||||
def test_validate_moa_payload_accepts_legacy_flat_payload():
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
assert validate_moa_payload(_valid_preset_payload()) == []
|
||||
|
||||
|
||||
def test_validate_moa_payload_flags_half_filled_reference_slot():
|
||||
"""The #64156 shape: provider picked, model still empty (mid-edit autosave)."""
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
preset = _valid_preset_payload()
|
||||
preset["reference_models"].append({"provider": "kilo", "model": ""})
|
||||
problems = validate_moa_payload({"presets": {"default": preset}})
|
||||
|
||||
assert problems
|
||||
assert any("reference 2" in p and "model is required" in p for p in problems)
|
||||
|
||||
|
||||
def test_validate_moa_payload_flags_half_filled_aggregator():
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
preset = _valid_preset_payload()
|
||||
preset["aggregator"] = {"provider": "openrouter", "model": ""}
|
||||
problems = validate_moa_payload({"presets": {"default": preset}})
|
||||
|
||||
assert any("aggregator" in p and "model is required" in p for p in problems)
|
||||
|
||||
|
||||
def test_validate_moa_payload_flags_empty_references():
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
preset = _valid_preset_payload()
|
||||
preset["reference_models"] = []
|
||||
problems = validate_moa_payload({"presets": {"default": preset}})
|
||||
|
||||
assert any("at least one complete reference model" in p for p in problems)
|
||||
|
||||
|
||||
def test_validate_moa_payload_flags_recursive_moa_slot():
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
preset = _valid_preset_payload()
|
||||
preset["aggregator"] = {"provider": "MoA", "model": "default"}
|
||||
problems = validate_moa_payload({"presets": {"default": preset}})
|
||||
|
||||
assert any("recursive MoA" in p for p in problems)
|
||||
|
||||
|
||||
def test_validate_moa_payload_names_the_broken_preset():
|
||||
"""Multi-preset payloads must say WHICH preset is broken."""
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
problems = validate_moa_payload(
|
||||
{
|
||||
"presets": {
|
||||
"good": _valid_preset_payload(),
|
||||
"broken": {
|
||||
"reference_models": [{"provider": "", "model": ""}],
|
||||
"aggregator": {"provider": "a", "model": "b"},
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
assert problems
|
||||
assert all("'broken'" in p for p in problems)
|
||||
assert not any("'good'" in p for p in problems)
|
||||
|
||||
|
||||
def test_validate_moa_payload_agrees_with_clean_slot():
|
||||
"""Contract: a payload validate accepts must survive normalize UNCHANGED in
|
||||
its slots — validate and _clean_slot can never disagree (else a payload
|
||||
could pass validation and still be swapped for defaults)."""
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
payload = {"presets": {"p": _valid_preset_payload()}}
|
||||
assert validate_moa_payload(payload) == []
|
||||
|
||||
cfg = normalize_moa_config(payload)
|
||||
assert cfg["presets"]["p"]["reference_models"] == payload["presets"]["p"]["reference_models"]
|
||||
assert cfg["presets"]["p"]["aggregator"] == payload["presets"]["p"]["aggregator"]
|
||||
|
||||
|
||||
def test_validate_moa_payload_rejects_non_dict():
|
||||
from hermes_cli.moa_config import validate_moa_payload
|
||||
|
||||
assert validate_moa_payload(None)
|
||||
assert validate_moa_payload([1, 2])
|
||||
assert validate_moa_payload({"presets": {"p": "not-a-dict"}})
|
||||
|
|
|
|||
|
|
@ -861,6 +861,72 @@ class TestWebServerEndpoints:
|
|||
assert cfg["moa"]["reference_models"] == payload["reference_models"]
|
||||
assert cfg["moa"]["aggregator"] == payload["aggregator"]
|
||||
|
||||
def test_put_moa_models_rejects_half_filled_slot_with_422(self):
|
||||
"""#64156: a mid-edit autosave (provider picked, model empty) used to be
|
||||
silently normalized into the hardcoded default preset — the user's
|
||||
config was replaced without any error. The write path must reject it."""
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
original = load_config().get("moa")
|
||||
|
||||
payload = {
|
||||
"presets": {
|
||||
"default": {
|
||||
"reference_models": [{"provider": "kilo", "model": ""}],
|
||||
"aggregator": {"provider": "openrouter", "model": "anthropic/claude-opus-4.8"},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
resp = self.client.put("/api/model/moa", json=payload)
|
||||
assert resp.status_code == 422
|
||||
assert "model is required" in resp.json()["detail"]
|
||||
# Config untouched — not swapped for defaults.
|
||||
assert load_config().get("moa") == original
|
||||
|
||||
def test_put_moa_models_rejects_half_filled_aggregator_with_422(self):
|
||||
payload = {
|
||||
"presets": {
|
||||
"default": {
|
||||
"reference_models": [{"provider": "openrouter", "model": "deepseek/deepseek-v4-pro"}],
|
||||
"aggregator": {"provider": "openrouter", "model": ""},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
resp = self.client.put("/api/model/moa", json=payload)
|
||||
assert resp.status_code == 422
|
||||
assert "aggregator" in resp.json()["detail"]
|
||||
|
||||
def test_put_moa_models_round_trips_fanout_and_reference_max_tokens(self):
|
||||
"""GET → PUT round-trip must not erase newer per-preset knobs. The old
|
||||
Pydantic payload didn't declare fanout / reference_max_tokens, so any
|
||||
client save silently wiped hand-set values back to defaults."""
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
payload = {
|
||||
"presets": {
|
||||
"default": {
|
||||
"reference_models": [{"provider": "openrouter", "model": "deepseek/deepseek-v4-pro"}],
|
||||
"aggregator": {"provider": "openrouter", "model": "anthropic/claude-opus-4.8"},
|
||||
"fanout": "user_turn",
|
||||
"reference_max_tokens": 600,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
resp = self.client.put("/api/model/moa", json=payload)
|
||||
assert resp.status_code == 200
|
||||
|
||||
saved = load_config()["moa"]["presets"]["default"]
|
||||
assert saved["fanout"] == "user_turn"
|
||||
assert saved["reference_max_tokens"] == 600
|
||||
|
||||
# And the GET view carries them back to the client.
|
||||
fetched = self.client.get("/api/model/moa").json()
|
||||
assert fetched["presets"]["default"]["fanout"] == "user_turn"
|
||||
assert fetched["presets"]["default"]["reference_max_tokens"] == 600
|
||||
|
||||
# ── GET /api/media (remote image display) ───────────────────────────
|
||||
|
||||
def test_get_media_serves_image_in_root(self):
|
||||
|
|
|
|||
|
|
@ -2301,6 +2301,8 @@ export interface AuxiliaryModelsResponse {
|
|||
export interface MoaModelSlot {
|
||||
provider: string;
|
||||
model: string;
|
||||
/** Optional per-slot reasoning effort — round-tripped, not edited here. */
|
||||
reasoning_effort?: string;
|
||||
}
|
||||
|
||||
export interface MoaConfigResponse {
|
||||
|
|
@ -2312,6 +2314,10 @@ export interface MoaConfigResponse {
|
|||
reference_temperature: number;
|
||||
aggregator_temperature: number;
|
||||
max_tokens: number;
|
||||
/** Optional advisor output cap — round-tripped, not edited here. */
|
||||
reference_max_tokens?: number | null;
|
||||
/** Fan-out cadence (per_iteration | user_turn) — round-tripped. */
|
||||
fanout?: string;
|
||||
enabled: boolean;
|
||||
}>;
|
||||
reference_models: MoaModelSlot[];
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue