Repository navigation
refactor(*): seal the memory backend seam so a memory plugin needs no host edits - #414
Conversation
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: onboarding can erase a selected backend when its optional onboarding contribution is absent.
Reviewed github/main...07836b5bbbc3, the affected callers, relevant history, and the plugin/onboarding contracts. Coverage included AGENTS.md and CONTEXT terminology, the full diff, callers and compatibility behavior, test changes for weakening, and the memory-plugin architecture constraints. One concrete blocker is marked inline.
Verification: git diff --check github/main...HEAD passed. After syncing the documented Python 3.12 dev environment with uv sync --frozen --python 3.12 --extra dev --dev, the 38 changed Python test modules passed: 1,611 passed and 34 skipped in 46.75s. An initial run in the unsynced Python 3.13 environment failed because the dev/plugin packages were absent; that was an environment setup result, not treated as a branch failure. The focused reproduction described inline also confirmed the config-clearing path.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the new sub-agent memory path and legacy embedding fallback need correction.
Reviewed f114e08 against github/main and the seven-commit delta from the prior revision. Coverage included the repository rules (AGENTS.md/CLAUDE.md and the relevant CONTEXT material), the full diff, new callers and lifecycle ownership, history, compatibility for existing installs, dependency boundaries, tests and whether their mocks hid production behavior, and the plugin-seam architecture. The earlier onboarding blocker is fixed and its thread is resolved.
The three concrete regressions are marked inline. The per-record backend lifecycle also appears in dag_runner.py; the manager anchor names the shared defect.
Nonblocking follow-up, within the explicitly deferred EverOS-shaped GUI surface: settings.everos does not check memory.backend, so an installed but unselected EverOS plugin still returns available=true despite the response contract saying this state gets a note.
Verification: git diff --check github/main...HEAD, uv lock --check, and focused Ruff checks passed. Local uv run --frozen --python 3.12 pytest -q finished with 21,838 passed, 65 skipped, and one failure in untouched test_cli_theme.py::test_bold_accent_renders_styled_not_bare; CI's Python run had 22,096 passed and 52 skipped, then failed the changed-line coverage gate at 88.16% versus 90%. The TUI job failed because generated RPC types are out of sync with openrpc.json; a local TUI type-check could not start because JavaScript dependencies are not installed (tsc: not found). Page, RPC-contract, lint, bridge, wheel-smoke, repository, title, and commit-message checks passed.
f114e08 to
7416ee9
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: fresh managed onboarding must launch EverOS with the embedding endpoint it just saved.
The three findings from f114e08 are fixed, replied to, and resolved. A range-diff confirmed the earlier 25 PR commits are patch-identical after the base update; review therefore covered the complete github/main...HEAD diff and the new 7416ee980 delta, its callers and process lifecycle, history, backward compatibility, dependency and plugin boundaries, the repository rules (AGENTS.md/CLAUDE.md and relevant CONTEXT material), and whether tests were weakened. The replacement lifecycle and config-shaped tests are materially stronger.
One regression introduced by the fix commit remains inline. It meets the round-four blocker bar: the new host-owned write introduces it, choosing embedding during ordinary fresh managed onboarding reaches it, and the normal product paths leave the already-running service on the placeholder config with no way for the operator to apply the saved endpoint.
The previously named nonblocking GUI follow-up remains: settings.everos still reports available=true when EverOS is installed but another memory.backend is selected, despite its response contract describing that state as unavailable with a note. This stays deferred with the EverOS-shaped GUI surface.
Verification: the focused seven-file suite passed (1,037 tests), including all three fixed paths. git diff --check github/main...HEAD, uv lock --check, and focused Ruff checks passed. All four CI Python shards passed; the aggregate coverage gate is red at 88.97% versus 90%, and the TUI job is red because its generated RPC types are out of sync with openrpc.json. The remaining listed CI checks passed.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: onboarding still launches EverOS without the embedding endpoint it just saved.
The 7416ee9..1d4add1 delta adds sub-agent regression tests and regenerates TUI RPC types; it does not change the production onboarding path identified in the open thread at plugins-dist/everos-memory/raven_everos/onboard.py. A range-diff confirms the previously reviewed production commits are otherwise unchanged, so that blocker remains open and is not duplicated inline here.
Review coverage: repository rules (AGENTS.md, CLAUDE.md, and the applicable CONTEXT files), the full prior diff plus this exact range-diff, affected callers and history, backward compatibility, test-strengthening versus weakening, and the plugin/host seam constraints. The added tests strengthen failure and lifecycle coverage; no tests were weakened.
Verification: uv run --frozen --python 3.12 pytest -q tests/test_subagent_memory.py tests/test_subagent_manager.py tests/test_subagent_dag_runner.py passed (443 tests); focused Ruff, uv lock --check, and git diff --check github/main...HEAD passed. GitHub CI is fully green, including all four unit shards and the coverage gates.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The fresh-configuration launch failure is fixed, and its thread is resolved. One narrower rerun case remains as a nonblocking follow-up: keeping an existing host embedding does not bind it before the wizard restarts EverOS. Reconfiguring the same endpoint is a working recovery, so this does not meet the late-round blocker bar.
Review coverage: repository rules (AGENTS.md, CLAUDE.md, and applicable CONTEXT files), the full prior diff plus the exact revision range-diff, affected callers and launch history, backward compatibility, test-strengthening versus weakening, and the plugin/host seam constraints. No tests were weakened.
Verification: the focused onboarding/config suite passed (380 tests), the EverOS backend suite passed (167 tests), focused Ruff, uv lock --check, and diff checks passed. Both pytest runs reported only warning-level wall-clock ceiling notices (four each). GitHub CI is fully green, including all unit shards and coverage gates.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The new commit only documents the existing host/plugin ownership asymmetry. Its range-diff leaves every previously reviewed behavior unchanged, introduces no new defect, and does not alter the already named nonblocking onboarding follow-up. All threads opened by this review are resolved.
Review coverage: repository comment/documentation rules (AGENTS.md), the applicable CLAUDE.md and CONTEXT guidance, the exact delta and full range-diff, affected callers and history, backward compatibility, test weakening, and the plugin/host seam constraints. The added explanation records a non-obvious ownership constraint; no tests or production behavior changed.
Verification: the corrected focused command covering onboarding and both Raven config modules passed (381 tests), with four warning-only wall-clock ceiling notices. An initial command named nonexistent tests/test_config.py, collected zero tests, and was not counted. Focused Ruff, uv lock --check, and git diff --check github/main...HEAD passed. GitHub CI is fully green, including all four unit shards and coverage gates.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The 2078f0cd8773..2f3363826728 delta settles the remaining nonblocking follow-up. The child-environment builder is the spawn's sole production caller, now fills Raven's saved embedding endpoint for both fresh and keep-current onboarding, preserves any deliberate backend binding with setdefault, and shares the TOML-precedence rule through embedding_env. The write-site special case is removed, and existing production commits are otherwise unchanged by range-diff. All threads opened by this review are resolved.
Review coverage: repository rules (AGENTS.md, CLAUDE.md, and applicable CONTEXT files), the full prior diff plus this exact delta and range-diff, affected spawn/runtime callers and history, backward compatibility and precedence, test-strengthening versus weakening, and the plugin/host seam constraints. No tests were weakened.
Verification against a detached checkout of the live PR head: uv run ... python -m pytest -n0 over onboarding, EverOS config, server, and backend passed (623 tests), with two warning-only wall-clock ceiling notices. Preliminary detached-environment invocations collected no tests because their pytest process lacked the transient plugin import; they were corrected rather than counted. Focused Ruff, uv lock --check, and diff checks passed. GitHub CI is fully green, including all unit shards and coverage gates.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the new settings-routing commit against the unchanged previously reviewed range. The card now reads and writes the same endpoint home as the runtime: an operator-authored EverOS embedding section keeps precedence, while the ordinary host-owned lane uses Raven's embedding block. The web caller's partial update and redacted-key behavior remain safe because the host writer merges non-empty fields, and the extracted precedence helper preserves the prior binding behavior.
Coverage included the repository rules in AGENTS.md/CLAUDE.md and the relevant CONTEXT terms, the base diff plus range history, RPC and web callers, the OpenRPC shape, backward-compatible merge/secret behavior, the plugin/runtime ownership boundary, and the changed tests for weakening. No test was skipped or weakened to obtain a pass.
Verification: uv run --frozen --python 3.12 pytest -q tests/test_rpc_settings.py tests/test_rpc_contract_shapes.py tests/test_config_update_everos.py tests/test_everos_server.py tests/test_everos_backend.py passed 370 tests; focused Ruff and git diff --check passed. The local UI test command could not start because this reviewer worktree has no installed Vitest binary, while GitHub's page checks, UI RPC contract, all four Python unit shards, coverage, lint, and every other reported check are green. The PR is mergeable.
e9ba8b7 to
187937c
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The rebased series is patch-equivalent to the previously reviewed revision through the settings-page fix. The two new commits correctly stop treating the shipped EverOS template as an operator-authored embedding endpoint: the real template supplies a model and base URL but no key, and EverOS's provider factory requires all three. The production-shaped template test preserves that distinction, and the accompanying RPC edit only corrects its explanation.
Named follow-up (nonblocking, no open thread): preserve a complete endpoint supplied through the documented EVEROS_EMBEDDING__* environment variables. everos_has_own_embedding() now examines only TOML, and configure_embedding_env() applies the host fallback with os.environ.update(). On this head, starting from the shipped template with all three operator environment values set caused the helper to return True and replace all three with Raven's host endpoint. Treating a complete existing environment endpoint as an explicit EverOS choice, or otherwise sharing the non-overwriting rule used by _child_env(), would preserve the documented override. This revision introduces the behavior, but an operator can recover by recording the endpoint in everos.toml, so it does not meet the late-round blocker bar.
Coverage included AGENTS.md/CLAUDE.md and the relevant CONTEXT terms, the full base range plus rebase history, backend startup and server callers, RPC behavior, backward compatibility, the memory-plugin ownership seam, and the changed tests for weakening. Verification on the live head: uv run --frozen --python 3.12 pytest -q tests/test_config_update_everos.py tests/test_rpc_settings.py tests/test_everos_server.py tests/test_everos_backend.py tests/test_cli_onboard_commands.py passed 653 tests; focused Ruff and git diff --check passed. GitHub's four unit shards and all other completed checks passed; the coverage aggregation was still pending when checked.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The documented environment override is now preserved: a complete EVEROS_EMBEDDING__* endpoint wins over Raven's host block, while an incomplete fragment falls back to the complete host endpoint instead of producing a mixed triple. This settles the previous follow-up.
Named follow-up (nonblocking): everos_has_own_embedding() now selects the EverOS side of the settings read/write path for a complete environment override, but settings_everos() reads values only from everos.toml and settings_everos_set() writes them there. With the documented override exported and the shipped template on disk, the service uses the environment endpoint while the settings card displays the template as unconfigured; saves target TOML and remain outranked by the environment. Managing the exported variables is a working recovery, so this does not meet the late-round blocker bar, but the settings surface should eventually represent this third, externally managed home explicitly.
Verification: the changed config/server suites passed (129 tests) and Ruff passed. The broader focused run was not green: 3 failed, 653 passed; all three failures were the settings endpoint-home assertions encountering a complete environment endpoint. An isolated run with the complete documented override reproduced the card mismatch (1 failed). GitHub CI is fully green, and git diff --check github/main...HEAD passed.
Covered in this round: the repository rules and canonical terminology, the full base diff with emphasis on the new delta, affected callers and commit history, backward compatibility for TOML and environment precedence, test changes (none weakened), and the plugin/host seam constraints. No additional merge blockers survived refutation.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The new predicate and refusal fix the silent-save behavior for a genuine operator-exported endpoint, and the ambient-environment fixtures make the unit assumptions deterministic.
Named follow-up (nonblocking): the predicate cannot distinguish an operator's exported endpoint from the identical variables Raven writes itself. In the ordinary runtime sequence, configure_embedding_env(host_endpoint) first copies Raven's host block into os.environ; a subsequent settings.everos call then renders the template as an empty embedding card, and settings.everosSet rejects the edit as shell-managed. The reproduction returned bound_by_raven=True, an empty card, and the new “change them instead” error. A manual config edit plus restart remains a working recovery, so this introduced and ordinarily reachable defect does not satisfy the late-round requirement that an operator have no working way out. The eventual fix needs provenance for the environment triple, rather than treating every complete live-process triple as external.
Verification: uv run --frozen --python 3.12 pytest -q tests/test_config_update_everos.py tests/test_everos_server.py tests/test_everos_backend.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py passed all 657 tests; Ruff and git diff --check github/main...HEAD passed; GitHub CI is fully green. The focused tests do not catch the lifecycle sequence because the new autouse fixtures deliberately clear the process variables before each RPC test.
Covered in this round: repository rules and canonical terms, the full branch diff with emphasis on the dff7410adf68..8a707009c1f6 delta, callers and startup/settings lifecycle, commit history, environment/TOML backward compatibility, whether tests were weakened, and the plugin/host seam constraints. No merge blocker survived refutation, and all previously opened threads remain resolved.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The new _BOUND_HERE provenance closes the ordinary in-process startup/settings failure: Raven's first binding is no longer mistaken for an operator export, repeated backend starts keep using the host endpoint, and a genuine pre-existing environment endpoint is still preserved and protected from ineffective settings saves.
Named follow-up (nonblocking): the provenance is module-local while Raven's /restart uses os.execv. That restart preserves the Raven-written EVEROS_EMBEDDING__* values but reloads the module with an empty _BOUND_HERE, so the restarted gateway again classifies its own binding as external, declines the second bind, and returns to the blank/refused settings behavior. The exec-shaped reproduction produced first_bind=True, managed_before_exec=False, then managed_after_module_reset=True and second_bind=False. This is introduced and /restart is an ordinary path, but stopping and launching Raven from a clean shell or supervisor remains a working recovery, so it does not meet the late-round blocker requirement. Provenance needs to survive the exec boundary (or the binding must avoid surviving without it).
Verification: the focused onboarding/config/server/backend/RPC suites passed all 659 tests; Ruff and git diff --check github/main...HEAD passed; GitHub CI is fully green. The added tests correctly cover the in-process sequence but do not exercise environment survival across a module/process reset.
Covered in this round: repository rules and canonical terms, the full branch diff with emphasis on 8a707009c1f6..24df741f197a, affected callers and restart history, backward compatibility for operator exports and TOML precedence, whether tests were weakened, and the plugin/host seam constraints. No merge blocker survived refutation, and all previously opened threads remain resolved.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The restart provenance follow-up is fixed. Raven now carries the binding marker through its os.execv restart alongside the values it describes, reloads that provenance in the replacement interpreter, and strips the private marker from the environment handed to EverOS. The two-interpreter regression test covers the actual preservation/reset boundary; genuine operator exports still remain external, while Raven's binding remains editable and can be refreshed after restart.
Verification: the focused onboarding/config/server/backend/RPC suites passed all 660 tests; Ruff and git diff --check github/main...HEAD passed; GitHub CI is fully green.
Covered in this round: repository rules and canonical terms, the full branch diff with emphasis on 24df741f197a..72b056d4e0fd, affected startup, spawn, settings, and restart callers plus history, backward compatibility for operator exports and TOML precedence, whether tests were weakened, and the plugin/host seam constraints. No candidate finding survived a concrete-failure check, and every previously opened thread remains resolved.
…ontract on everos tests/test_plugin_boundary.py greps the host for imports of raven_everos and the plugin for imports of host-private modules, with allowlists that name the violations still standing. The MemoryBackend contract tests now run against EverosBackend through the fake adapter; that exposed recall returning top_k + 1 rows (the profile row rides outside the server's bound), fixed by truncating after the sort. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…host applies it The Protocol said recall should raise and a failed start aborts startup; the host wraps every call and degrades. The prose now matches the code. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
… import MemoryBackend gains health() -> BackendHealth | None (HealthCheck rows with ok / degraded / missing). raven doctor renders the checks verbatim and exits on missing ones; raven import gates on ready. Both stop importing the everos plugin's internals; the everos backend implements health() and the doctor scenarios move to the plugin's own tests. CONTRACTS_VERSION 16 -> 17. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…tion swap runtime.dispose() awaited backend.stop() unwrapped, so a raising plugin escaped the gateway serve loop on config reload. The contract prose is corrected alongside: start may not be awaited before the first turn on the TUI and serve hosts, and recall runs under a turn budget. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…ackend EverosSkillSource becomes BackendSkillSource(backend, agent_id, name). The router prefix, the RRF weight key and the after-turn feedback filter all read memory.backend, so a second memory plugin's skills route and feed back without host edits. The skill tools route every non-hub prefix to the on-disk lookup. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…gnostics A backend built before health() existed passes raven import instead of raising AttributeError; doctor renders an out-of-band status and treats a non-BackendHealth return like a raise. The multimodal -> multimodal_llm capability mapping regains a test. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
raven/contracts/onboard.py declares OnboardUI (the wizard shell the host lends), OnboardStep (run / configured) and StepOutcome (CONFIGURED / DISABLED / BACK). Manifests may contribute [[plugin.contributes.onboard]]; the registry builds it like a service; build_onboard_steps returns (name, step) pairs. CONTRACTS_VERSION 17 -> 18. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
The readiness check moved inside the try so backend.stop() runs when it exits; this pins the call order. Doctor gains a test for a non-BackendHealth return, and the contract prose names mechanisms instead of orchestrator line numbers. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
raven/config/update_everos.py becomes raven_everos/config.py; the config loader's everos-slice migration block is deleted (the plugin infers root and owned when absent); borrow_provider_credentials becomes the host helper lend_provider_credentials in update_providers. The plugin-side boundary allowlist is now empty. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
… OnboardStep raven/cli/onboard_everos.py becomes raven_everos/onboard.py. The screen reads the host's wizard shell from a module-level OnboardUI handle set by run(), returns a StepOutcome and never writes memory.backend; the host's step 4 runs every activated plugin's screen and records the outcome. Provider resolution the screen needs (resolve_main_model) is a host helper handed over through OnboardUI. CONTRACTS_VERSION 18 -> 19. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…isabled enabled_by_default had no opt-in counterpart, so a manifest that set it false could never be activated. The field is gone from the model (legacy manifests still parse), the CLI and the ext.list RPC; a memory.backend that names no installed plugin is now said on stderr. Co-authored-by: Claude (claude-sonnet-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…n config slice Two sites loaded the host's validated config only to find the plugin's own base_url; they now read recorded_slice(), which also falls back to the legacy everos key. The boundary guard learns raven.config.raven. Co-authored-by: Claude (claude-sonnet-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
memory-plugin-architecture.md carries health(), the onboard contribution and the failure contract; the plugin's package docstring lists what the host may still reach; the contract test base names health(). Co-authored-by: Claude (claude-sonnet-5) <noreply@anthropic.com> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…oice Manual acceptance on a real managed install: the wizard said embedding was configured, and the service it started the next moment ran keyword-only. The child had no EVEROS_EMBEDDING__ values at all, and the server logged that the embedding api_key was not configured. The deference rule asked whether everos.toml carried a model name that was not a "<...>" placeholder. The shipped template seeds every section with a real model name and an empty key, so that question is true of a root nobody has configured -- every fresh managed install looked like an operator who had deliberately chosen their own endpoint, and the host's was never bound. Model and key, which is `role_configured_in`'s criterion rather than a second one invented beside it. An empty key is what a template leaves. The tests missed it because each hand-built its "operator's own" section with a key in it. The regression copies the shipped template itself, and asserts the template still seeds a model first, so it cannot quietly stop proving anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two comments claimed the template seeds "<...>" placeholder model names. It does not: every section arrives with a real model name and an empty key. That belief is what produced the bug the previous commit fixed, and leaving it written down is how the next reader reaches the same place. The settings card keeps its "<" strip as a guard against a hand-written "<fill me>", but the comment no longer presents it as the mechanism that makes an unconfigured role read as unset -- `api_key_set` is what does that. The plugin's docstring drops the claim entirely and points at the function that now owns the question. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ce too EverOS resolves its settings env-above-file, so exporting the three EVEROS_EMBEDDING__ variables is an operator saying which endpoint memory uses, as plainly as editing the toml. The question of whether EverOS already had one of its own looked only at the file, and the host's endpoint was then applied with a plain update: the documented override worked until Raven started and then silently did not. Both homes now answer that question. All three variables or none: two of them is not an endpoint, and taking the third from the host would hand EverOS a mixture of two operators' intentions that fails in a way naming neither. That also removes the child environment's special case. It filled each missing variable rather than applying the set, on the grounds that a value already present had been bound deliberately -- which is now what the question itself answers, so there is nothing left to protect and the per-key fill is the one behaviour worth avoiding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…top reading the shell Two things the environment home left behind once it became an answer to "does EverOS already have an endpoint". The settings card still writes to everos.toml, which the exported variables outrank, so a save was accepted, written and then ignored -- the silent no-op this card was fixed for a few commits ago, arriving by the one route left. It now refuses and names the three variables, which is the only useful thing raven can say: it cannot edit a shell. The cases covering the file and host homes read the exported variables out of whoever ran them. A developer with the documented override set turned four of them into a different case, silently and only on their machine; the reviewer hit exactly that. An autouse fixture takes the variables out of the ambient shell in both files, and the cases that are about the override set them themselves afterwards. Telling the two homes apart is now its own predicate rather than a second reading of the combined one, since a surface that offers to edit a file needs to know which of the two it is looking at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… export `configure_embedding_env` puts the host endpoint into this process so the in-process EverOS imports and every child see it. From that moment all three variables are set and complete, and the predicate added last commit -- which asked only whether all three were set -- said an operator had exported them. On an ordinary running install the settings card therefore went blank and refused an edit by telling the person to change variables they had never set. The question is provenance, so the module records which of the three it set itself and answers "managed outside" only for a complete triple that is not all ours. Partly ours cannot arise, since a fragment is not an endpoint and the binding replaces it whole, and is read the conservative way if it ever does. Both new cases exercise the sequence rather than a prepared state, which is the only way to reach this: clearing the variables before each case, the thing that makes every other case deterministic, is also what hid it. The autouse fixtures clear the provenance too -- it is process-global and would otherwise carry from one case into the next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… exec `raven gateway --restart` re-launches through `os.execv`: the environment survives and every module object is rebuilt. The provenance added last commit lived in a module-level set, so the restarted gateway found raven's own three variables exported and no record of having written them -- blank settings card, a save refused by naming variables nobody had set, and no second bind, for the rest of that process's life. It now travels in a variable of its own beside the values it describes, and the child environment drops it: EverOS has no use for raven's bookkeeping. The case runs two real interpreters and hands the second the first's environment, because that is the only shape that can fail -- importing the module twice in one process would find the set still populated and prove nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`everos_source.py` held `BackendSkillSource`, which reaches whatever backend is configured through the contract and takes its prefix from `memory.backend` -- the everos name in the filename was left over from before that was true. It is `backend_source.py` now. The three sites that compared a backend name to the literal "everos" were not treating that backend as special: they answer "the default names something that is not here", which is the one case where the reader did not choose the name themselves and so has to be told what the default was. `SHIPPED_DEFAULT_BACKEND` says that, and moves with the default if it ever changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The block configures the pipeline that distils skills out of finished turns into the workspace cache. It calls no service and needs no plugin -- its own docstring said it replaced the EverOS HTTP path -- so the name told every reader of their own config the opposite of what is true: that this belonged to the memory backend and would stop working without it. `skillForge.extraction` now, with the old spelling accepted as an alias for the load that happens before the migration writes, and a run-once migration that moves the key so nobody is left reading the wrong name. A config carrying both keeps the new one and drops the retired key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three places held an endpoint that has to be one: raven's own block, the
memory plugin's everos.toml, and knowledge.embedding{Model,Provider}.
Vectors from different models cannot be compared, so a knowledge base
and a memory store embedding against two of them are two stores that
silently disagree -- and three places to change it is two places to
forget.
One block now, and it names a model and the provider that serves it
rather than carrying an address and a key of its own. The credential
stays with the provider, which is where every other feature already
reads it from, so configuring an endpoint here hands over a choice
instead of a second copy of somebody's key. A run-once migration moves
the retired pair into it and rewrites the file, so the old names stop
being read as current.
The pair is checked when it is set rather than at the first index: a
base built against a provider with no credential, or a model that does
not return vectors, used to fail with a stack of retries and a message
about vectors when the thing to fix was two fields on a page. A model
with no provider is refused for the same reason -- stored, it reads as
configured on every screen while every reader resolves it to nothing.
Changing the model is allowed and says once what it costs: everything
already indexed answers to the old one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both moves -- knowledge.embedding{Model,Provider} into the top-level
embedding block, and skillForge.everos into skillForge.extraction --
ran as always-on shims: correct in memory, and invisible on disk. So
every load went on paying for them, and the config file went on naming
a block after the backend it never belonged to, for the life of the
install.
They are one stamped generation now, applied to the file the same way
every other rename here is, and then they stop.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A stored pin spells the model the way it was chosen, and that is not always the way it is sent. Raven's own convention puts the route in front -- deepseek/deepseek-v4-pro beside provider deepseek -- and the head comes off, because the vendor never asked to be told its own name. But a slash does not have to be that: Qwen/Qwen3-Embedding-4B is the org on HuggingFace, which DeepInfra and SiliconFlow serve verbatim, and Pro/BAAI/bge-m3 puts a SiliconFlow channel in front of one. Taking a head off either sends an id that does not exist. providers.wire already owns this question for every other model id, and answers it by the only rule that is safe: a head comes off when it names the provider, and stays otherwise. Asked there rather than decided here -- and the plugin now reads the resolved endpoint through the host's own resolver instead of keeping a second copy of the resolution, which is the copy that drifts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pin names a provider and lets it hold the address and the key. A screen that collected a key and recorded only the name therefore left the pin pointing at a provider the host holds nothing for -- resolving to nothing, while the wizard reported semantic memory over a service running on keywords alone. That is the failure this endpoint's move was supposed to end, arriving from the other side. So the screen hands the key to the host before it records the choice, through a lender's mirror on the onboard contract: the plugin says whose key this is, the host decides where credentials live. Only what is missing is written -- a provider that already has a key keeps it, since the one on file is what every other feature is using. The embedding role stops offering a typed-in endpoint for the same reason. A pin can only name a provider, so an address has nowhere to go; the listed vendors are the ones this screen can record, and a self-hosted one is configured as a provider first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three surfaces still wrote the retired shape. The settings card sent an address and a key into a block that has no place for them; the whitelist still named knowledge.embeddingModel; the page's pin row still wrote under the knowledge key. All three now address the embedding block by model and provider. A borrow means something different in each home, so it is resolved by home rather than by copying: into EverOS's file a borrow lends the address and the key, into raven's block it is the provider's name, which is the whole of what the block stores. An address or key sent to raven's block is refused rather than dropped -- a value accepted and discarded reads to the caller as one that was stored -- and a save that moves the model says once what it costs. Doctor's migration offer follows the same rule: the retired block held an address, the pin names a provider, so the move only lands when a configured provider answers at that address. There is nowhere else for the key to live, and inventing a provider row from a bare URL would be a worse guess than saying nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenAI and DeepSeek state no `default_api_base`, because setup must not ask for one: their requests route through LiteLLM, which knows where they live. `usable_default_api_base` answers that question and answers it correctly. A caller of `resolve_provider_credentials` is asking a different one -- where do I POST -- and every caller it has is a bare client: the embedding endpoint's resolution, the pin check that refuses an unreachable pair, doctor's reverse lookup from an address to a provider, and the wizard deciding whether a key is already on file. None of them goes through LiteLLM, and all four were told that a config holding a real OpenAI key had no usable credential for the most common embedding provider there is. So the address falls back to the same canonical table the bare-client path in this module already uses. It is a short list of known addresses, not a guess: a vendor with neither a spec nor an entry still answers nothing, and a key alone is still not enough for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…osts Three surfaces write this block and only one of them did either. The settings page's pin row went through the raw writer: no check that the pair can embed anything, and no word about a swapped model -- while the same swap made through the memory card got both. A model changed on a page invalidates every vector already stored just as surely as one changed in the wizard. One way in now. The pin row routes through the endpoint writer, and the wizard's screen gets the sentence back through the onboard contract and prints it after the tick -- after, because the write did happen and what it costs is about the stores, not about whether the screen worked. Say it only when the model actually moved, or picking "Keep current" would cry wolf at everyone who reconfigures anything else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sentence shown when the embedding model moves told every operator to run one particular backend's command, whatever backend they run. A host message printing a plugin's command is the coupling this seam exists to remove, and it was written into the host by this branch. It says what has to happen -- re-index what the memory backend has stored -- and leaves the how to whoever ships that backend. The screen inside the plugin still names its own command, which is its to name. Three more of the same, all from earlier commits here: the lender's error and its docstrings described the group it can hand over as what one named backend's section can hold. A test now asserts the sentence carries no backend name, so this cannot creep back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
72b056d to
93b10c3
Compare
The rule was already there for one direction: a model with nobody to serve it reads as configured on every screen while every reader resolves it to nothing. A provider with nothing to run on it is the same write from the other end, and it was accepted -- the settings card's lender picker with the model box left blank sent exactly that and came back applied, having stored nothing anyone could use. Both halves or neither, so the rule is one rule. A run that changes only one of them is still judged against the block it leaves behind, so swapping a provider under a stored model is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The model/provider embedding pin is coherent through schema migration, provider resolution, onboarding, RPC, knowledge resolution, and the EverOS launch handoff. Three bounded follow-ups remain; they are named here rather than left as open threads:
- Clearing the embedding picker through its
Inheritoption is a no-op.PinRowsends{model: "", provider: ""}, butset_embedding_endpointfilters both values and returns before writing. A direct reproduction left the old pair unchanged. Editing the config is a working recovery, so this does not meet the late-round blocker bar. - A legacy-only endpoint in
everos.tomlis no longer a runtime fallback. An upgraded knowledge base remains unavailable until the operator migrates the endpoint; moreover,doctor --fixrecords the fix as applied even whenadopt_legacy_endpointreturnsFalsebecause no configured provider matches a custom legacy URL. Provider setup plus settings/doctor is a recovery, so this is also nonblocking. - The canonical SkillForge entry in
CONTEXT.mdstill says the local extraction pipeline isskillForge.everosand belongs to embedded EverOS. The code and migration now call itskillForge.extractionand explicitly make it plugin-independent.
At review round 17, none of these meets all three required conditions for a blocker: the first two have working recovery paths, and the documentation mismatch causes no runtime failure.
Coverage: I read AGENTS.md / the identical CLAUDE.md, CONTEXT-MAP.md, and the relevant Runtime terminology; reviewed git diff github/main...HEAD, the range-diff from the previously clean revision across the rebase, the ten-commit embedding redesign, and the final half-pin commit; traced callers and history; checked backward compatibility, test changes, and the plugin-seam constraints. git diff --check and Ruff pass. The focused config/onboarding/doctor/RPC/knowledge/contract run passes (910 tests). The changed real-LLM integration test was also attempted; it failed before exercising the scenario because this environment has no EverOS LLM key/base URL. The local web test could not start because vitest is not installed here; GitHub's page check is green. At posting time all completed GitHub checks are green, with the four unit shards still running.
## Summary Five upgrade-path and reporting defects in the embedding pin that #414 landed. Three were named as bounded follow-ups in that PR's final review; two came out of driving the acceptance environment afterwards, and both of those were invisible to the unit suite because every fixture already held the new shape. The two found by acceptance: - **A config still holding the retired endpoint shape does not load at all.** The block used to carry the address and the key; `EmbeddingConfig` forbids extras, so `raven doctor` ends in a traceback rather than in a degraded feature, and so does every other command that reads the extension blocks. The migration takes those keys now: the address is adopted onto whichever configured provider answers there, and when nothing answers the whole block goes rather than half of it, with the dropped address named in the log. A model left with no provider is a pin every reader resolves to nothing while every screen reads it as configured. - **A pin could not name a provider this config holds.** The provider half was checked against the registry alone, and Raven carries no spec for every vendor LiteLLM can reach. A pin the wizard had stored and every reader resolved came back from the settings page as a provider that does not exist, leaving the endpoint uneditable on the page that exists to edit it. A section the operator wrote counts as proof the vendor exists; a name nothing holds is still refused as a typo. The three from review: - **Clearing the pin was a no-op.** The picker's inherit option sends both halves empty and the writer dropped empty values before deciding anything, so the call returned applied while the file kept the old pair. - **Doctor reported a move it had not made.** The endpoint only moves when a configured provider answers at the retired address, and the return value was discarded. The unmade move stays in the remaining list now, with a line saying why. - **The canonical SkillForge glossary entry still said `skillForge.everos`** and attributed the local extraction pipeline to an embedded copy of the memory backend. ## Type - [x] Fix ## Verification - `uv run --all-extras pytest -q` -- 23058 passed, 110 skipped, 4 failed; the four are the same pre-existing failures main carries (`test_agent_loop_token_budget`, `test_agents_code_launcher`, `test_agents_oncall_launcher`, `test_ppt_engine_image_search`), matched by test id against a run on the merge base. - `uv run ruff check .` and `uv run ruff format --check .` -- clean. - `make check-commits`, `make check-source-language`, `make check-large-files` -- clean. - Every fix was reproduced before and after against a real config or a real gateway, not only through its test: the unloadable config came from the acceptance home itself and `raven doctor` was run on it before and after; the provider refusal was reproduced over a real WebSocket to a running gateway; clearing and the doctor report were each run end to end. - Each new test was checked by removing its fix and confirming it fails. - Wider acceptance on the running product, with the memory plugin and without it: a gateway with no memory plugin installed built a knowledge base whose width (4096) was measured from the live endpoint and answered a query sharing almost no keywords with the source (score 0.728); the real TUI recalled a fact stored in an earlier turn, with the recall payload in the audit artifact as evidence rather than the model's answer. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk Two changes widen what is accepted rather than narrowing it: a provider section counts alongside the registry, and a migration edits a block it previously left alone. The migration only runs for a config carrying the retired keys and is stamped, so it runs once; a config already in the current shape is untouched. Rolling back any single commit is safe -- they do not depend on each other. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
) ## Summary An EverOS role points a section at a provider: the settings card writes the model and asks `lend_provider_credentials` for that provider's key and address to go beside it. The address was copied only when there was one to copy -- the reasoning being that a section with no lender address may be holding one the reader typed by hand. A section being pointed at a new lender is holding the OLD lender's address, though, and leaving the field out of the answer keeps it. Moving the memory role from an OpenRouter model to a DeepSeek one left DeepSeek's key at OpenRouter's address, which the far end refuses. DeepSeek is not a corner case here: every vendor LiteLLM routes carries no `default_api_base`, so the lend answers with no address for all of them. The lender answers about the address either way now, empty included. An empty value reads where it lands the way an unset one does: the EverOS writer sets the field, and the section stops carrying somebody else's endpoint. Found while running the settings page's acceptance case for the EverOS roles against a real gateway. The lend rule is from #414, not from the settings work that surfaced it. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `uv run pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py tests/test_rpc_console.py -q` -- 641 passed on the branch this was written on; 191 on the two closest suites here. - The new case fails without the change: the lender answers `{"api_key": ...}` where the section needs `{"api_key": ..., "base_url": ""}`. - `uv run ruff check` and `ruff format --check` over both files -- clean. - Real gateway, temp `RAVEN_HOME`, EverOS plugin present: `settings.everosSet` with `borrow_from: "deepseek"` writes the model and DeepSeek's key and leaves no `base_url` behind; the same call with `borrow_from: "openrouter"` writes OpenRouter's address back. Before the change the first of those left `base_url = "https://openrouter.ai/api/v1"` under DeepSeek's key. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk - Two callers, both of which point a section at a provider: the EverOS card (`settings.everosSet`) and the CLI onboarding reuse step. For a lender that has an address, nothing changes. For one that has none, the borrowing section now stops carrying the previous lender's -- which is the fix. - A reader who typed an address by hand into an EverOS section and then borrowed from an addressless provider loses that typed value. That is the same event as the bug, read the other way; the section is being pointed somewhere else, and the card shows the resolved address. - Rollback: revert the commit. ## Related Issues N/A --------- Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
… host edits (#414) ## Summary Make `MemoryBackend` the only seam between the host and the memory subsystem, so a second memory plugin can be installed without editing host code. The abstraction is the goal; no second backend is being added. What the branch does: - `health()` joins the Protocol. `raven doctor` and `raven import` read a `BackendHealth(ready, checks)` the backend fills in with its own vocabulary, instead of reaching into EverOS internals. Doctor prints the checks verbatim and never interprets a label. - A new `[[plugin.contributes.onboard]]` kind, with `raven/contracts/onboard.py` as its paper. The host lends the wizard shell as one `OnboardUI` and records the result; the screen returns a `StepOutcome`. The plugin never writes `memory.backend` -- that key stays the host's. EverOS's step-4 screen and its `everos.toml` management move into the plugin. - The skill source and the after-turn feedback prefix are named from `memory.backend` rather than the literal `everos`. - `enabled_by_default` is gone from the manifest: an installed plugin the user has not disabled is active. - `tests/test_plugin_boundary.py` pins the dependency direction in both directions, with an equality assertion so the remaining exceptions cannot grow. That guard's allowlist started at five host files still speaking EverOS's wire protocol. The follow-ups below close three of them. Two things this branch itself introduced were found in manual acceptance on an install without the plugin and fixed here: a turn printed which backends were installed -- empty on exactly the install that needs the sentence -- and `settings.everos` / `settings.everosSet` let `ModuleNotFoundError` reach the dispatcher as an internal error carrying a traceback. Both now carry the note doctor and the wizard already use, naming the distribution to install. Review on this PR found four blockers, all fixed and covered. A fifth defect came out of manual acceptance rather than review, and four more out of the rounds that followed it -- every one of those in the embedding endpoint's own plumbing, each found by the round after the fix that introduced it. They are listed under Risk; the blockers are: 1. `onboard` is an optional contribution, but the wizard equated "no matching screen" with "not configured", so one `raven onboard --skip-memory` wrote `memory.backend` back to null for a third-party backend configured by hand. A backend with no screen now answers on the strength of having been selected; only one whose own screen says it is unconfigured is cleared, which is the case that rule exists for. 2. The record a sub-agent call leaves behind ran through a backend nobody had started. The shipped adapter begins in an unknown state, so the first flushing write only scheduled a readiness probe and answered false -- the record said "unavailable" while the service was up the whole time, and each per-record HTTP client was left open. One context manager now owns start and stop around a record, in both the manager and the DAG runner. 3. That record's write and the recall after it addressed different owners: a real config spells its keys in camelCase and the block reaches the backend as written, while the adapter read only the contract's snake_case, so a sub-agent's memories were filed under the host identity. The adapter reads both spellings. 4. Legacy embedding discovery had narrowed to the recorded plugin root, so an install predating root recording lost its knowledge-base endpoint with a valid file still on disk. **The five follow-ups the seam left behind are in this PR too**, rather than in three stacked ones, because they are one goal and the branch protection dismisses an approval on every push: - The embedding endpoint is raven's. A knowledge base indexes inside the gateway process and never speaks to the memory service, yet it read its endpoint out of everos.toml, so selecting another backend left it with no vectors and no message naming the cause. Raven holds the block now and hands it to the backend at start -- as a shared default, not an override: a plugin that was given an endpoint of its own keeps it, and only one that has none takes raven's. Discovery of an existing everos.toml still checks all three places the plugin itself looks, read with plain tomllib so it works where the plugin is not installed. No data migrates: a base is keyed on its model and measured width, not on the base url. - `delete()` joins the Protocol. The browser used to delete a row out of EverOS's index, in the host process, through the plugin's repository layer -- while the markdown EverOS treats as the source of truth kept the text, so the memory came back on the next rebuild, after the user was told it was gone. The adapter now uses only operations EverOS already performs, for the kinds it performs them for. - Sub-agent memory goes through the contract. The read half is a new optional `recall_session`; the write half needed no new surface, only two metadata conventions written down. `agents.<name>.config.everos` becomes `config.memory`, opaque to the host except the two keys the host reads; the old spelling still loads. - The memory and settings pages say why they are empty -- no plugin, or a backend this page does not read -- instead of four zeros and a retry button that cannot help. - The `everos` package leaves the host's dependencies for the plugin's own, at the same exact pin; the host reaches the tree through the plugin. This is the package layer only: the installers already deliver `everos-memory` as a sibling wheel from the same release, so an ordinary install still has long-term memory. The boundary guard's allowlist falls from five entries to two, both GUI surfaces still shaped around EverOS's own store. Generalising their wire is a frontend change and is deliberately not in scope. This branch was reviewed on GitLab as merge request 639 over three rounds, including a post-rebase range-diff check. It is being moved here because GitHub is now the source of truth. ## Type - [x] Refactor ## Verification - `uv run pytest` on this branch, rebased onto `main`: 22146 passed, 4 failed, 45 skipped. Three of the four (`test_agents_oncall_launcher`, `test_agent_loop_token_budget`, `test_agents_code_launcher`) were re-run on a detached checkout of that same `main` commit in this environment and fail there identically. The fourth (`test_ppt_engine_image_search`) could not run in that checkout, whose environment lacks the deck engine's extras; this branch's only change anywhere under `plugins-dist/ppt-engine` is the removal of the `enabled_by_default` manifest key, which that test does not reach. So this branch adds no regression. - `npx vitest run` in ui-web: 1680 passed across 103 files. `npm run type-check` and `npm run gen:check` clean, so the generated client matches the contract. - Every fix in the follow-up commits was mutated away and its test confirmed red, then restored and confirmed green. For the record path that is three separate mutations: dropping `start()`, dropping `stop()`, and narrowing the owner lookup back to one spelling. - `uv run ruff check .` and `uv run ruff format --check` on the changed files: clean. - `uv lock --check`: consistent. - `scripts/check_commit_messages.py`, `scripts/check_large_files.py`, `scripts/check_source_language.py` over the branch range: clean. - Manual acceptance against a real isolated install (temporary RAVEN_HOME, isolated EverOS root and port), 34 cases: `raven plugins`, `raven doctor` across four backend states, `raven onboard` interactive and non-interactive including all three step-4 lanes, `raven import` refusing and importing, an end-to-end write and recall, the web UI's memory and settings pages driven through a real browser, a sub-agent trace record, and an install with no memory plugin at all. 31 passed; 1 failed and was fixed; 1 partial; 1 carried over from the earlier round. - The failure is the one worth reading. On a fresh managed install the wizard reported embedding configured and the service it started the next moment ran keyword-only: the spawned process carried no `EVEROS_EMBEDDING__` values, and EverOS logged that the embedding key was not configured. The rule deciding whether everos.toml already held an operator's own endpoint asked whether its model name was a `"<...>"` placeholder; the shipped template seeds every section with a real model name and an empty key, so every fresh install read as a deliberate choice and the host's endpoint was never bound. It now asks the criterion the module already had -- model and key. The unit tests missed it because each built its "operator's own" section with a key in it; the regression copies the shipped template itself. - The partial: the memory page's search returned relevant results without error, but every memory in the test corpus was on one subject, so narrowing could not be observed. The carried-over case is the skill-source prefix, whose path this branch does not touch. - Findings recorded and not fixed here, none introduced by this branch: a progress spinner is still drawn after `import` reports it imported nothing; an unsupported delete surfaces as a generic internal error; the TUI log and `doctor` word "no memory plugin" differently; a turn that did reach disk still prints that it was not written. - The partial case: an induced agent skill is discovered through `find_skill` with the backend's prefix, but `read_skill` / `use_skill` on that id fail. Nothing writes the on-disk layer those tools look in, on this branch or on `main`, and no test covers it. Pre-existing; filed as a follow-up. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk User-visible changes: 1. User-track recall is now strictly bounded by `top_k`. A profile row used to ride outside that bound. 2. `raven doctor` builds the plugin factory, so a managed EverOS setup writes its env var and config templates during a doctor run. It used to be read-only. The `--json` memory section is reshaped around `health.checks`. 3. A third-party drop-in plugin without `enabled_by_default = true` used to be skipped and now loads. 4. The loader no longer back-fills the EverOS config slice's `root` / `owned`, nor strips a legacy `mode` key; a leftover `mode` now draws an unknown-key warning. 5. The wizard's recap shows the contribution name `everos` rather than `EverOS`. 6. `use_skill` with an unknown prefix reports "not found" after a disk lookup instead of "unknown source". From the follow-ups: 7. Config gains an optional top-level `embedding` block. Absent, the knowledge base reads the EverOS file as before and says so once. An operator who moves the values across changes nothing else; one who names a *different* model there gets the existing `StaleBaseError` telling them to rebuild that base. The block is a default for the memory plugin rather than an override, so an install that had configured the plugin separately keeps what it had. Note that a config carrying this block does not load on an older Raven, which rejects unknown top-level keys. 8. `agents.<name>.everos` is written back as `agents.<name>.memory`. The old spelling still loads; a per-agent `baseUrl` is ignored with a warning, having never had a user. 9. Deleting a memory now changes what EverOS treats as the source of truth, so it stays deleted. Profiles and cases report that they cannot be deleted rather than appearing to succeed. 10. `pip install raven` no longer pulls the memory engine in. `install.sh` and `install.ps1` are unaffected: both already resolve `everos_memory-*.whl` from the release they install and pass it as `--with everos-memory`, and the source path installs it editable, so those installs keep long-term memory. An install that reaches neither -- a release predating the sibling wheel, or a bare `pip install` -- now boots without memory and says which distribution to install rather than failing on an import. Found after the acceptance run, in review, all fixed here. They share one cause worth stating once: the endpoint acquired a second home, and each fix taught one more reader about it while leaving the next one behind. 11. The endpoint the wizard saved never reached the EverOS process it then started. The rule for "does everos.toml already hold an operator's own endpoint" asked whether its model name was a `"<...>"` placeholder; the shipped template seeds every section with a real model name and an empty key, so every fresh managed install read as a deliberate choice and the host's endpoint was never bound. The service came out of the wizard running keyword-only while the wizard said embedding was configured. The rule now asks the criterion the module already had: model and key. 12. The exported `EVEROS_EMBEDDING__*` variables are a third home, documented by EverOS and ranked above its file. They were being overwritten. A complete triple is now an operator's choice; an incomplete one is not an endpoint and is replaced whole rather than topped up from the host. 13. Raven writes those same variables itself at startup, so a reader that asked only whether all three were set called raven's own binding an operator's export -- a blank settings card and a save refused by naming variables nobody had set. The module now records which of them it set. 14. That record lived in a module-level set, and `raven gateway --restart` re-launches through `os.execv`: the values survive, module state does not. The record now travels in a variable beside them, and is stripped from the environment handed to EverOS. 15. A settings save the environment would outrank is refused with the variable names, rather than written and silently ignored. Four of the five arrived one per review round, each in the fix for the one before it. The tests that missed them all shared a shape: each built its "an operator configured this" fixture with a key in it, or cleared the environment before the case, so the states the product actually produces -- a template with no key, a live process that has just bound its own endpoint -- were never the input. The regressions now copy the shipped template, and run the startup sequence and a second interpreter rather than a prepared state. Rollback is a revert of the branch: the seam is additive, and `memory.backend` keeps its shipped default. - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
## Summary Five upgrade-path and reporting defects in the embedding pin that #414 landed. Three were named as bounded follow-ups in that PR's final review; two came out of driving the acceptance environment afterwards, and both of those were invisible to the unit suite because every fixture already held the new shape. The two found by acceptance: - **A config still holding the retired endpoint shape does not load at all.** The block used to carry the address and the key; `EmbeddingConfig` forbids extras, so `raven doctor` ends in a traceback rather than in a degraded feature, and so does every other command that reads the extension blocks. The migration takes those keys now: the address is adopted onto whichever configured provider answers there, and when nothing answers the whole block goes rather than half of it, with the dropped address named in the log. A model left with no provider is a pin every reader resolves to nothing while every screen reads it as configured. - **A pin could not name a provider this config holds.** The provider half was checked against the registry alone, and Raven carries no spec for every vendor LiteLLM can reach. A pin the wizard had stored and every reader resolved came back from the settings page as a provider that does not exist, leaving the endpoint uneditable on the page that exists to edit it. A section the operator wrote counts as proof the vendor exists; a name nothing holds is still refused as a typo. The three from review: - **Clearing the pin was a no-op.** The picker's inherit option sends both halves empty and the writer dropped empty values before deciding anything, so the call returned applied while the file kept the old pair. - **Doctor reported a move it had not made.** The endpoint only moves when a configured provider answers at the retired address, and the return value was discarded. The unmade move stays in the remaining list now, with a line saying why. - **The canonical SkillForge glossary entry still said `skillForge.everos`** and attributed the local extraction pipeline to an embedded copy of the memory backend. ## Type - [x] Fix ## Verification - `uv run --all-extras pytest -q` -- 23058 passed, 110 skipped, 4 failed; the four are the same pre-existing failures main carries (`test_agent_loop_token_budget`, `test_agents_code_launcher`, `test_agents_oncall_launcher`, `test_ppt_engine_image_search`), matched by test id against a run on the merge base. - `uv run ruff check .` and `uv run ruff format --check .` -- clean. - `make check-commits`, `make check-source-language`, `make check-large-files` -- clean. - Every fix was reproduced before and after against a real config or a real gateway, not only through its test: the unloadable config came from the acceptance home itself and `raven doctor` was run on it before and after; the provider refusal was reproduced over a real WebSocket to a running gateway; clearing and the doctor report were each run end to end. - Each new test was checked by removing its fix and confirming it fails. - Wider acceptance on the running product, with the memory plugin and without it: a gateway with no memory plugin installed built a knowledge base whose width (4096) was measured from the live endpoint and answered a query sharing almost no keywords with the source (score 0.728); the real TUI recalled a fact stored in an earlier turn, with the recall payload in the audit artifact as evidence rather than the model's answer. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk Two changes widen what is accepted rather than narrowing it: a provider section counts alongside the registry, and a migration edits a block it previously left alone. The migration only runs for a config carrying the retired keys and is stamped, so it runs once; a config already in the current shape is untouched. Rolling back any single commit is safe -- they do not depend on each other. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
) ## Summary An EverOS role points a section at a provider: the settings card writes the model and asks `lend_provider_credentials` for that provider's key and address to go beside it. The address was copied only when there was one to copy -- the reasoning being that a section with no lender address may be holding one the reader typed by hand. A section being pointed at a new lender is holding the OLD lender's address, though, and leaving the field out of the answer keeps it. Moving the memory role from an OpenRouter model to a DeepSeek one left DeepSeek's key at OpenRouter's address, which the far end refuses. DeepSeek is not a corner case here: every vendor LiteLLM routes carries no `default_api_base`, so the lend answers with no address for all of them. The lender answers about the address either way now, empty included. An empty value reads where it lands the way an unset one does: the EverOS writer sets the field, and the section stops carrying somebody else's endpoint. Found while running the settings page's acceptance case for the EverOS roles against a real gateway. The lend rule is from #414, not from the settings work that surfaced it. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `uv run pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py tests/test_rpc_console.py -q` -- 641 passed on the branch this was written on; 191 on the two closest suites here. - The new case fails without the change: the lender answers `{"api_key": ...}` where the section needs `{"api_key": ..., "base_url": ""}`. - `uv run ruff check` and `ruff format --check` over both files -- clean. - Real gateway, temp `RAVEN_HOME`, EverOS plugin present: `settings.everosSet` with `borrow_from: "deepseek"` writes the model and DeepSeek's key and leaves no `base_url` behind; the same call with `borrow_from: "openrouter"` writes OpenRouter's address back. Before the change the first of those left `base_url = "https://openrouter.ai/api/v1"` under DeepSeek's key. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk - Two callers, both of which point a section at a provider: the EverOS card (`settings.everosSet`) and the CLI onboarding reuse step. For a lender that has an address, nothing changes. For one that has none, the borrowing section now stops carrying the previous lender's -- which is the fix. - A reader who typed an address by hand into an EverOS section and then borrowed from an addressless provider loses that typed value. That is the same event as the bug, read the other way; the section is being pointed somewhere else, and the card shows the resolved address. - Rollback: revert the commit. ## Related Issues N/A --------- Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Summary
Make
MemoryBackendthe only seam between the host and the memory subsystem, so a second memory plugin can be installed without editing host code.The abstraction is the goal; no second backend is being added.
What the branch does:
health()joins the Protocol.raven doctorandraven importread aBackendHealth(ready, checks)the backend fills in with its own vocabulary, instead of reaching into EverOS internals. Doctor prints the checks verbatim and never interprets a label.[[plugin.contributes.onboard]]kind, withraven/contracts/onboard.pyas its paper. The host lends the wizard shell as oneOnboardUIand records the result; the screen returns aStepOutcome. The plugin never writesmemory.backend-- that key stays the host's. EverOS's step-4 screen and itseveros.tomlmanagement move into the plugin.memory.backendrather than the literaleveros.enabled_by_defaultis gone from the manifest: an installed plugin the user has not disabled is active.tests/test_plugin_boundary.pypins the dependency direction in both directions, with an equality assertion so the remaining exceptions cannot grow.That guard's allowlist started at five host files still speaking EverOS's wire protocol. The follow-ups below close three of them.
Two things this branch itself introduced were found in manual acceptance on an install without the plugin and fixed here: a turn printed which backends were installed -- empty on exactly the install that needs the sentence -- and
settings.everos/settings.everosSetletModuleNotFoundErrorreach the dispatcher as an internal error carrying a traceback. Both now carry the note doctor and the wizard already use, naming the distribution to install.Review on this PR found four blockers, all fixed and covered. A fifth defect came out of manual acceptance rather than review, and four more out of the rounds that followed it -- every one of those in the embedding endpoint's own plumbing, each found by the round after the fix that introduced it. They are listed under Risk; the blockers are:
onboardis an optional contribution, but the wizard equated "no matching screen" with "not configured", so oneraven onboard --skip-memorywrotememory.backendback to null for a third-party backend configured by hand. A backend with no screen now answers on the strength of having been selected; only one whose own screen says it is unconfigured is cleared, which is the case that rule exists for.The five follow-ups the seam left behind are in this PR too, rather than in three stacked ones, because they are one goal and the branch protection dismisses an approval on every push:
delete()joins the Protocol. The browser used to delete a row out of EverOS's index, in the host process, through the plugin's repository layer -- while the markdown EverOS treats as the source of truth kept the text, so the memory came back on the next rebuild, after the user was told it was gone. The adapter now uses only operations EverOS already performs, for the kinds it performs them for.recall_session; the write half needed no new surface, only two metadata conventions written down.agents.<name>.config.everosbecomesconfig.memory, opaque to the host except the two keys the host reads; the old spelling still loads.everospackage leaves the host's dependencies for the plugin's own, at the same exact pin; the host reaches the tree through the plugin. This is the package layer only: the installers already delivereveros-memoryas a sibling wheel from the same release, so an ordinary install still has long-term memory.The boundary guard's allowlist falls from five entries to two, both GUI surfaces still shaped around EverOS's own store. Generalising their wire is a frontend change and is deliberately not in scope.
This branch was reviewed on GitLab as merge request 639 over three rounds, including a post-rebase range-diff check. It is being moved here because GitHub is now the source of truth.
Type
Verification
uv run pyteston this branch, rebased ontomain: 22146 passed, 4 failed, 45 skipped. Three of the four (test_agents_oncall_launcher,test_agent_loop_token_budget,test_agents_code_launcher) were re-run on a detached checkout of that samemaincommit in this environment and fail there identically. The fourth (test_ppt_engine_image_search) could not run in that checkout, whose environment lacks the deck engine's extras; this branch's only change anywhere underplugins-dist/ppt-engineis the removal of theenabled_by_defaultmanifest key, which that test does not reach. So this branch adds no regression.npx vitest runin ui-web: 1680 passed across 103 files.npm run type-checkandnpm run gen:checkclean, so the generated client matches the contract.Every fix in the follow-up commits was mutated away and its test confirmed red, then restored and confirmed green. For the record path that is three separate mutations: dropping
start(), droppingstop(), and narrowing the owner lookup back to one spelling.uv run ruff check .anduv run ruff format --checkon the changed files: clean.uv lock --check: consistent.scripts/check_commit_messages.py,scripts/check_large_files.py,scripts/check_source_language.pyover the branch range: clean.Manual acceptance against a real isolated install (temporary RAVEN_HOME, isolated EverOS root and port), 34 cases:
raven plugins,raven doctoracross four backend states,raven onboardinteractive and non-interactive including all three step-4 lanes,raven importrefusing and importing, an end-to-end write and recall, the web UI's memory and settings pages driven through a real browser, a sub-agent trace record, and an install with no memory plugin at all. 31 passed; 1 failed and was fixed; 1 partial; 1 carried over from the earlier round.The failure is the one worth reading. On a fresh managed install the wizard reported embedding configured and the service it started the next moment ran keyword-only: the spawned process carried no
EVEROS_EMBEDDING__values, and EverOS logged that the embedding key was not configured. The rule deciding whether everos.toml already held an operator's own endpoint asked whether its model name was a"<...>"placeholder; the shipped template seeds every section with a real model name and an empty key, so every fresh install read as a deliberate choice and the host's endpoint was never bound. It now asks the criterion the module already had -- model and key. The unit tests missed it because each built its "operator's own" section with a key in it; the regression copies the shipped template itself.The partial: the memory page's search returned relevant results without error, but every memory in the test corpus was on one subject, so narrowing could not be observed. The carried-over case is the skill-source prefix, whose path this branch does not touch.
Findings recorded and not fixed here, none introduced by this branch: a progress spinner is still drawn after
importreports it imported nothing; an unsupported delete surfaces as a generic internal error; the TUI log anddoctorword "no memory plugin" differently; a turn that did reach disk still prints that it was not written.The partial case: an induced agent skill is discovered through
find_skillwith the backend's prefix, butread_skill/use_skillon that id fail. Nothing writes the on-disk layer those tools look in, on this branch or onmain, and no test covers it. Pre-existing; filed as a follow-up.Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
User-visible changes:
top_k. A profile row used to ride outside that bound.raven doctorbuilds the plugin factory, so a managed EverOS setup writes its env var and config templates during a doctor run. It used to be read-only. The--jsonmemory section is reshaped aroundhealth.checks.enabled_by_default = trueused to be skipped and now loads.root/owned, nor strips a legacymodekey; a leftovermodenow draws an unknown-key warning.everosrather thanEverOS.use_skillwith an unknown prefix reports "not found" after a disk lookup instead of "unknown source".From the follow-ups:
embeddingblock. Absent, the knowledge base reads the EverOS file as before and says so once. An operator who moves the values across changes nothing else; one who names a different model there gets the existingStaleBaseErrortelling them to rebuild that base. The block is a default for the memory plugin rather than an override, so an install that had configured the plugin separately keeps what it had. Note that a config carrying this block does not load on an older Raven, which rejects unknown top-level keys.agents.<name>.everosis written back asagents.<name>.memory. The old spelling still loads; a per-agentbaseUrlis ignored with a warning, having never had a user.pip install ravenno longer pulls the memory engine in.install.shandinstall.ps1are unaffected: both already resolveeveros_memory-*.whlfrom the release they install and pass it as--with everos-memory, and the source path installs it editable, so those installs keep long-term memory. An install that reaches neither -- a release predating the sibling wheel, or a barepip install-- now boots without memory and says which distribution to install rather than failing on an import.Found after the acceptance run, in review, all fixed here. They share one cause worth stating once: the endpoint acquired a second home, and each fix taught one more reader about it while leaving the next one behind.
"<...>"placeholder; the shipped template seeds every section with a real model name and an empty key, so every fresh managed install read as a deliberate choice and the host's endpoint was never bound. The service came out of the wizard running keyword-only while the wizard said embedding was configured. The rule now asks the criterion the module already had: model and key.EVEROS_EMBEDDING__*variables are a third home, documented by EverOS and ranked above its file. They were being overwritten. A complete triple is now an operator's choice; an incomplete one is not an endpoint and is replaced whole rather than topped up from the host.raven gateway --restartre-launches throughos.execv: the values survive, module state does not. The record now travels in a variable beside them, and is stripped from the environment handed to EverOS.Four of the five arrived one per review round, each in the fix for the one before it. The tests that missed them all shared a shape: each built its "an operator configured this" fixture with a key in it, or cleared the environment before the case, so the states the product actually produces -- a template with no key, a live process that has just bound its own endpoint -- were never the input. The regressions now copy the shipped template, and run the startup sequence and a second interpreter rather than a prepared state.
Rollback is a revert of the branch: the seam is additive, and
memory.backendkeeps its shipped default.Related Issues
N/A