Repository navigation
feat(plugins): add the everos-cloud-memory plugin and its host wiring - #883
Conversation
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: prevent RPC-configured endpoint redirects and enforce byte-accurate cloud request limits.
Two blocking findings are marked inline. I reviewed the full github/main...HEAD diff and relevant surrounding code, including plugin discovery/activation, the MemoryBackend callers, onboarding, RPC and generated schema wiring, web settings behavior, release/install contracts, and commit history. I also checked the repository rules and Runtime/Web domain terminology, backward compatibility for the existing EverOS backend, and the test changes for weakened assertions. The unchanged three-plugin default install was verified as an explicit design constraint rather than reported as a defect.
Verification:
- After
uv sync --frozen --extra dev, the focused Python run passed: 569 tests. The initial unsynced invocation failed collection becauseraven_everoswas not installed. - Web settings tests passed: 56 tests; the fixture-shape gate passed: 5 tests.
- Web type-check and generated-client drift check passed.
- Targeted Ruff checks passed.
- Source-language and large-file checks passed when run directly;
makeitself is unavailable in this environment. - A direct CJK payload probe reproduced the inline HTTP 413 failure.
|
Both blocking findings are addressed on the branch head (e64b56b):
Local checks on the head: targeted pytest for the touched suites green, ruff/format clean, commitlint per sha, source-language gate on 6d509a8..HEAD exit 0. CI is running on the new head. |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the complete delta from a9c5813b4e38 through e64b56b2b748, including the manifest/admission contract, settings RPC caller and security boundary, cloud batching behavior, updated specifications, and regression tests. Both previously reported defects are fixed, the tests were strengthened rather than weakened, and no new plain error remains in the revision. The two review threads are resolved.
Verification:
- Focused Python suite: 570 passed.
- Original CJK reproduction: 300077-byte item rejected locally, zero HTTP requests issued.
- Targeted Ruff checks: passed.
- Source-language and large-file checks: passed.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the complete delta from the previously clean e64b56b2b748 head. It adds only the planned ponytail marker explaining the existing deliberate single-message ceiling and naming the future chunking path; runtime behavior, callers, compatibility, and tests are unchanged. The comment matches the repository rule by documenting why the drop exists rather than restating the code, and both earlier review threads remain resolved.
Verification:
uv run pytest tests/test_everos_cloud_backend.py -x: 29 passed.- Targeted Ruff check: passed.
- Source-language check: passed.
|
Self-review round (three clean-context reviewers: correctness/completeness, appropriateness/maintainability, credentials/evidence) on top of the first external review. Pushed as three commits on the branch head (01a51d4):
Credentials reviewer: no finding; key value never logged, printed or returned; write path confined to settable fields of the plugin's own slice; archived acceptance evidence holds only fake keys. Local checks on the head: targeted pytest 1003 passed / 6 skipped; ui-web Roles+source 57 passed, tsc clean, make build-ui boot snapshots match; make lint / lint-types clean; source-language and large-file gates on 6d509a8..HEAD exit 0; commitlint and check_commit_messages per sha. |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: preserve each recalled session episode's text with its own identity after score sorting.
The three-commit delta is otherwise coherent, including the malformed-timeout fallback, refusal sweep behavior, chooser navigation, cloud-only role filtering, opt-out marker, and added boundary tests. The inline issue meets the late-round blocker bar: this delta introduces it, ordinary multi-episode session data reaches it whenever result scores change source order, and the caller receives silently misidentified memory content with no operator-side recovery.
Covered the repository rules (AGENTS.md and relevant CONTEXT guidance), the delta and its callers/history, backward compatibility, whether tests were weakened, and the plugin/host architecture. Verification: 548 focused Python tests passed; the Roles Vitest file passed 36 tests; Ruff passed; the source-language checker passed. A direct two-episode probe reproduced high -> LOW - low body and low -> HIGH - high body.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The score/source-order fix is correct and narrowly scoped: session episodes are now rendered directly from their own rows, preserving both identity and chronological service order. The new regression test covers the exact inversion that previously swapped texts. The finding thread is resolved, and all reviewer-opened threads are now settled.
Covered the repository rules, the current delta and affected caller/history, backward compatibility, test strength, and the plugin/host contract. Verification: the original two-episode probe now returns the correct id/text pairs; 44 backend and contract tests passed; Ruff and git diff --check passed.
|
Not a blocker -- six smaller things from the same acceptance pass, each checked on this head merged onto main b56da14. None of them holds the merge on its own; the three blocking threads above do. Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.
|
|
Blocking: the three separate review threads remain outstanding on this revision. The six items in the acceptance note remain recorded as nonblocking follow-ups. They do not supply counterevidence for, or otherwise settle, the oversized-message retry loop, credential exposure, or stale CONTEXT anchor. |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: address deterministic oversize retry duplication, malformed-key exposure, and the stale CONTEXT chooser anchor.
The newly supplied evidence changes the standing clean assessment. The oversized-message behavior is pinned by current tests: sendable messages land, store() returns False, and both callers retry the same slice even though the dropped message can never fit. The credential finding also reproduces through the real HTTP stack: a trailing-space key is quoted in h11's exception and returned verbatim in the health hint, creating the described UI/log exposure path. The CONTEXT entry names a symbol absent from the implementation.
These are the other reviewer's open threads and remain theirs to close. The six separately listed follow-ups remain nonblocking. Verification on the unchanged head: 213 affected backend, RPC, onboarding, discovery, and fake-cloud tests passed; that green run does not refute the findings because the oversize tests explicitly assert the problematic partial-write/False contract and no malformed-key regression exists.
|
On the six non-blocking items (head 95d2cc6):
Local checks on 95d2cc6: touched pytest suites 1008 passed / 6 skipped; ui-web settings 220 passed, tsc clean, gen:check in sync; make lint / lint-types clean; commitlint and check_commit_messages per sha; source-language gate on 6d509a8..HEAD exit 0. |
95d2cc6 to
90d1ce8
Compare
90d1ce8 to
f2b47b5
Compare
|
No blockers; suggestions only, and they are marked inline. Acceptance items 1-3 and 5-6 are implemented and covered by the focused checks. Item 4 removes the blocking wait, but the claimed first-load checking state does not arrive automatically: The retry memo also has a narrow content-collision follow-up: because its digest set is consulted during the same call and ignores timestamps, two identical batches in one slice share a digest. A direct probe with two identical 130 KB rows returned |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The three blocking findings are addressed on this tree: deterministic oversize drops no longer trigger futile retries, malformed keys are normalized before Authorization construction, and the canonical chooser term points to the live symbol. The existing external threads remain for their opener to resolve.
Two named nonblocking follow-ups remain. First, the detached loadEverosCloud() completion only updates a module variable; it does not publish a new settings snapshot, so the cloud card is absent on the first settings load and appears after a second load/reopen rather than showing a live checking state. Second, the retry memo uses a per-session digest set during the initial call as well as retries, so two identical split batches collide; a probe with two identical 130 KB rows returned True after sending only one row. The first has a reopen workaround, and the second needs an unusual repeated-large-message slice, so neither meets the late-round blocker bar.
Covered AGENTS/CONTEXT rules, the complete revision delta, affected callers and history, backward compatibility, test-strength changes, and plugin/host boundaries. Verification: 217 focused Python tests and 58 settings UI tests passed; Ruff, source-language, and git diff --check passed; real-HTTP key normalization and the original oversize behavior were reproduced successfully. GitHub force-rewrote the two commits during review, but f2b47b5af565 has the exact same tree as the tested worktree.
|
Both follow-ups from the last pass are on the head (2dfd058):
Local: ui-web 63 files / 426 tests incl. the gates, tsc clean; touched pytest suites green; make lint / lint-types clean; source-language gate on 6d509a8..HEAD exit 0; both headers under 100 characters and commitlint-clean before the push this time. |
|
No blockers; this can merge as far as I am concerned. Verified both follow-ups at 2dfd058. Two identical 130 KB rows now produce two one-message adds, while the existing transient-failure test still proves a landed batch is skipped on retry. The settings source exposes the in-flight probe, and the store regression proves its completion patches |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The two follow-ups from the previous revision are correctly addressed. Batch position is now part of the retry identity, so identical batches within one slice remain distinct while stable re-splitting still suppresses already-landed batches on retry. The detached cloud probe is exposed to the settings store, whose completion patches the current snapshot without requiring a reopen. No new plain errors were found in the delta.
Covered the repository rules, the complete two-commit delta, affected callers and history, backward compatibility, test strength, and plugin/host boundaries. Verification: the original duplicate-batch probe now sends both rows; 218 focused Python tests and 75 settings UI tests passed; TypeScript, Ruff, source-language, and git diff --check passed. All threads opened from this review session remain resolved; the other reviewer owns the three older external threads.
|
Live service: with EVEROS_CLOUD_API_KEY set (the owner's key, loaded from the login shell), tests/integration/test_everos_cloud_real_cloud.py passes 6/6 against api.evermind.ai on head 2dfd058 -- add + flush + search round trip, get filtered by session, flush with nothing pending (200 no_extraction), a wrong key (401, health says 'API key rejected (401)'), /get page_size 1 with and without the key (200 / 401), and an add nobody flushed searchable about 12 s later (EVEROS_CLOUD_SLOW=1). The PR description's 'no request has reached the real service' is updated accordingly. |
|
Merge conflict: @gloryfromca this PR no longer merges onto the current main.
Your latest No blockers verdict names 2dfd058, not this head. Once a rebase onto main (or a merge of main) resolves the conflicts and the new head has a verdict, the board queues it again by itself. |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: resolve the two content conflicts against current main before this revision can merge.
Independently reproduced at 40b54b923e54: git merge-tree --write-tree HEAD github/main against current main 289426c32dfe exits nonzero with content conflicts in ui-web/scripts/gates/fixture-shape.test.mjs and ui-web/src/rpc/generated.ts; GitHub also reports mergeable=false / mergeable_state=dirty. The generated RPC file needs a merged schema regeneration that preserves both this change’s cloud-memory surface and main’s Harness/Persona surface, and the fixture-shape set needs both sides’ additions.
The revision delta from 2dfd05832e20 is otherwise clean: it only adds the module-level slow marker to the live-cloud integration tests, without weakening their assertions. git diff --check 2dfd05832e20..HEAD passed; with the credential deliberately removed, uv run pytest tests/integration/test_everos_cloud_real_cloud.py -q collected all six tests and skipped all six under their existing opt-in guards.
Coverage this round included the repository rules, the branch diff and one-commit delta, conflict-sensitive generated/gate history, relevant callers, backward compatibility, test-strength changes, and architecture constraints. No additional code finding emerged.
A separately distributed memory backend for EverOS Cloud: a thin REST client over httpx with one API key (the config slice or EVEROS_CLOUD_API_KEY), owner ids from the host's ServiceLocator, adds in agent mode, and a flush only on an explicit end (sub-agent handoff, importer final batch) and at stop. Ships its own onboard screen, a fake cloud for the suites (a MockTransport and an HTTP face for the real-host acceptance), the plugin boundary and coverage gates grown to scan both memory plugins, the release workflow building its wheel, and the design, acceptance and plan documents. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…and settings The wizard's step 4 asks which memory backend when more than one plugin contributes a screen (Off clears the backend; a cancel exits 1 and writes nothing). The memory page reads whichever backend memory.backend names, with the cloud slice's key as a Bearer header and no /health probe for the cloud. settings.everos reports reason other_backend for any non-EverOS backend so the Model page hides the three EverOS-only roles (embedding stays, knowledge bases use it); settings.set admits plugins.config.<id>.<field> against the plugin's manifest; settings.everosCloud reports the cloud card's key source and health; provider and model saves no longer restart the local EverOS when another backend is active. ui-web gains the cloud card with its key row, and the fixture gate pins the four optional fields the offline library never sends. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
Both sides of the rebase added methods to the contract; the generated client is rebuilt from the merged schema (207 methods). Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
The 250 KB ceiling was measured with len() on the JSON text, which counts code points; a long CJK turn passed the guard three times over and the service answered 413, so the slice was never stored. The measure is now the encoded length, and a two-byte-character boundary test pins it. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
….set settings.set is reachable from any RPC client with no confirmation step, and it admitted every field a plugin's config_schema declared, including the cloud plugin's base_url: a client could point the stored API key at another host. A field is now writable here only when its manifest entry carries settable = true; the cloud manifest marks api_key and leaves base_url to the config file, the same rule that keeps tools.web.proxy off the whitelist. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
The plan promised the ceiling a ponytail marker naming its upgrade path (chunking one message across adds); the comment carries it. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…weep past a refusal Four self-review findings on the cloud backend: a malformed RAVEN_EVEROS_CLOUD_FLUSH_TIMEOUT_S made store() raise (the contract says it never does); an assistant turn with None content was stored as the word "None", unlike the local plugin's empty string; recall_session handed the sub-agent memory record the 200-character summary instead of the subject and the whole episode; and the stop sweep abandoned every remaining session on the first refused flush and blamed the budget. Each has a test that fails on the old code. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…ooser, add zh strings settings.everos now answers reason other_backend before it checks whether the local plugin is installed, and the Model page draws an EverOS row only when the answer still describes it in sections -- so a cloud-only install (the install this plugin exists for) no longer shows three rows saying the plugin is not installed, and the server alone decides which rows survive. The memory-backend chooser gains the Back entry the other pickers have. The chooser's and the cloud screen's strings join the zh catalogue, and the screen uses the local plugin's check and cross marks. The settable manifest marker is documented in CONTEXT.md's Config-with-cargo entry and on the manifest field; spec C9 names the one deliberate change to the memory-off arm (no restart on a provider save). Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
… pinned, driver output ignored The discovery fixture keeps a single opt-out (the everos_cloud marker, carried module-wide by the plugin's test files); the settings.set refusal test also sends a dotted field and an empty plugin id; the fake's docstring says its ledger keeps headers verbatim and which of its faces belong to the local plugin's acceptance cases; .playwright-cli/ is ignored by the repository, not only by a developer's global rules. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
session_data_to_memories matched the score-sorted search mapping to the service-ordered rows by position, so two episodes with reordering scores swapped texts and the sub-agent memory record carried misidentified history. Episodes are now rendered row by row in the service's order, each text with its own id; a two-episode regression with differing source and score orders fails on the old code. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…retries skip landed adds Three findings from the second review pass. A slice holding one message over the 250 KB ceiling answered False after the other messages had landed, and both retrying callers (StorePipeline, the importer) re-sent the whole slice on every attempt: the drop is deterministic, so the rest was stored again and an import source never completed. The slice now reports landed; the warning that names the session and the message index stays. resolve_api_key strips whitespace and refuses a key with a control character, because the HTTP layer quotes the whole Authorization value in its error and that text reached the health hint, the settings card and the gateway log. A retry of the same slice skips the adds the service already accepted (their digests are kept until the slice lands), so a transient failure mid-slice stores nothing twice; the digest ignores the fill-in timestamp. A lone surrogate in a message is replaced so the size check and the send never raise. A parity test holds convert_messages to the local plugin's. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
… stop waiting on the probe CONTEXT.md's memory backend chooser entry named the function the branch had renamed; it now anchors to _choose_memory_backend, the plan and the design sketch follow, and the EverOS entry counts the cloud plugin among the distributed members. The settings page no longer waits on the cloud probe (up to five seconds against a slow service): the card shows its checking state until the answer lands. The card says when the key comes from EVEROS_CLOUD_API_KEY. The spec and acceptance rows for an oversize message say the slice lands with a warning, and the add row records the retry memo. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
Two identical batches in one slice shared a digest, so the second was skipped as already landed within the same store call; the index of the batch in the slice is part of the identity now (a retry re-splits the slice the same way, so it stays stable), and a twin-batch test sends both. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
load() no longer waits on the probe, but its answer only reached a module variable: the card stayed on its checking state until the dialog was opened again. The source now exposes everosCloud(), the probe it started, and the store patches the answer into the snapshot when it resolves; the test harness answers it from the fixture, and partial sources in store tests may leave it out. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…ce's extraction Run with a real key, the six live tests pass but trip the suite's idle ceiling: the wait between an add and the search that reads it back is the service extracting, not a delay the suite can shorten. The slow mark says so. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
… page The card landed there because it stood in for the three EverOS role rows the cloud hides, but standing in for a model row is not the same as being one: a memory backend's endpoint and credential answer neither question that page asks. No page mounts it now, and where it belongs is the Memory section's to say once that section has a design. The component and its data stay. The four cases that only exercised it move out of the roles tests into CloudCard.test.tsx, which renders it directly; the three that also assert the roles page hides the EverOS rows under a non-everos backend keep that half, which is unaffected. Until the Memory section grows a surface, the key is entered through raven onboard or EVEROS_CLOUD_API_KEY. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Every product under agents/ pinned its memory at the stock EverOS address, and only raven-design read the host's. The address is not a constant: the wizard moves a managed server when the port is taken, and a server the user runs themselves is wherever they put it, so the other four addressed whatever answered on 18791 -- or nothing. Which store memories live in is not the product's choice either. The host reads a sub-agent's memories back through its own backend, so a product writing anywhere else leaves that record empty with nothing reporting it. That is also what makes the hosted backend reachable from a product at all: memory.backend now travels, with the endpoint and, when the product holds none itself, the credential. Identity never travels. memory.userId / agentId and the local slice's user_id / agent_id are the product's own, and they are what keeps five products apart inside one store. owned is forced off whenever the local backend is in force. A product infers ownership from a slice naming no root, the shape all five ship, so it used to answer a host that was merely not serving by starting a second server against a root of its own -- stranding every memory written so far. It now reports the service as unavailable, which is what the host does. The pairing of a backend name to the distribution that contributes it moves beside the constant that already stated it for everos, so the launcher can resolve a slice before any registry exists. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
40b54b9 to
8ab2e3b
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The rebase conflict is resolved: the rewritten 16-commit series is equivalent apart from the expected fixture allowlist merge and regenerated RPC count, git merge-tree --write-tree HEAD github/main succeeds, and GitHub reports the head mergeable. The two new commits also preserve the documented identity boundary and keep the unmounted cloud card directly tested.
One nonblocking follow-up is inline: explicit host memory.backend: null currently falls back to the product backend. It is introduced and ordinarily reachable, but plugins.disabled remains a working operator escape hatch, so it does not meet this late round’s blocker bar.
Verification: 408 focused Python tests passed with 1 platform skip; 76 focused settings tests passed; the fixture-shape gate passed 5/5; RPC generation matches the 228-method contract; TypeScript type-check and git diff --check passed. Coverage included AGENTS/CONTEXT rules, the full branch diff and range history, conflict resolution, callers, backward compatibility, test-strength changes, and architecture constraints.
The launcher reads which distribution contributes a backend before any plugin is loaded, and it took that from raven.core.plugin_stack -- an edge from raven.config into raven.core, which already imports raven.config back. That pair was inside the cycle budget's ceiling: 14 mutual pairs against 13, and 22 subpackages drawn into a cycle against 18, red in CI and green under make lint-imports, whose ten contracts say nothing about this one. The pairing and the default backend name now live in raven.contracts.memory, the kernel module both readers already import, and plugin_stack derives EVEROS_PLUGIN_DISTRIBUTION from the pairing and re-exports both names so every importer reads them where it read them before. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
… budget The previous commit put it in raven.contracts, which broke the cycle but spent a reviewed budget on something that is not a contract: the papers ceiling is 3700 lines and the move took them to 3719, and the test says in as many words that a new paper is a reviewed change to that ceiling rather than a bump in passing. It belongs beside the two fields it joins instead. MemoryConfig.backend names a contribution and PluginsConfig.config is keyed by the distribution that provides it, both in raven/config/raven.py, which raven.core may import and which imports nothing back. plugin_stack re-exports the two names its own callers read from it and no longer carries the pairing, which it never read. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The two new commits preserve the public imports while moving the backend/distribution pairing out of the raven.config <-> raven.core cycle and below the contracts line budget. The previous explicit-null behavior is unchanged; it remains the named nonblocking follow-up from the prior review, and its thread is now resolved so it is not mistaken for a merge objection.
Verification: 269 focused tests passed with 1 platform skip, including the import-cycle budget, product rendering, plugin boundary/bootstrap, CLI plugin/doctor, and import-sync suites. A direct compatibility probe confirmed the old raven.core.plugin_stack imports and new config constants agree; git diff --check and git merge-tree --write-tree HEAD github/main passed. make lint-imports could not run because make is unavailable in this environment; the repository’s dedicated import-cycle budget test passed instead.
Coverage included AGENTS/CONTEXT rules, the revision delta and branch diff, import callers and history, backward-compatible exports, test-strength changes, and architecture constraints. No new finding emerged.
…ducts too
memory.backend null is how an operator disables long-term memory, and the
inheritance read it with .get(), which cannot tell that null from an absent
key. A host with memory off therefore left every packaged agent on its own
shipped backend, still writing, through a path the operator's own raven no
longer builds. Presence now decides: a key the host carries travels whatever
its value, an absent one leaves the product's default standing.
Reported on the pull request, reproduced directly before the change (host
{"memory": {"backend": null}} returned everos and added the local slice) and
after (returns None, writes null, adds no slice).
Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The explicit-null follow-up is fixed: the host’s off switch now propagates to rendered product configs, while an absent key retains the product default and a selected cloud backend still inherits its connection settings. No collateral issue emerged from the delta.
Verification: 373 focused tests passed with 1 platform-specific skip; independent probes covered explicit null, absent backend, and cloud selection; git diff --check and git merge-tree --write-tree HEAD github/main passed. GitHub reports the head mergeable, and the review-thread sweep found no open threads.
Coverage included AGENTS/CONTEXT rules, the revision and branch diffs, callers and history, backward compatibility, test-strength changes, and architecture constraints.
|
Not a blocker. The cloud-security surface holds: no RPC path redirects the credential endpoint (settings.set of base_url to an attacker URL is refused), the api_key never reaches logs / argv / the health output, egress is gated on the key (no key -> store() does nothing and recall() returns [], with zero network calls observed), and no TLS verification is disabled. I confirmed the egress gate (_flush returns on Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.
|
…owns it The pairing this branch introduced says in its own docstring that its readers are the launcher and anything reporting on a backend whose distribution is not installed, and the cloud reporters were spelling the name themselves instead: the memory page's unavailable note, its two slice lookups, and the settings card's hint and slice lookup. They agreed with the map, so nothing was broken; a third backend added to the map and missed here would have been misreported. The import package stays spelled where it is used. It is not in the pairing -- that answers "which distribution", not "which module to import" -- and the local backend keeps its own beside find_spec for the same reason. Unifying those is wider than this change: config/update.py, updates/upgrade.py and knowledge/_embedding.py each carry one too, all predating this branch. Raised on the pull request as non-blocking. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…e other two The entry listing the distributions beside everos-memory said they contribute "a deck-building toolchain, a visual-design engine rather than memory", and this branch inserted the new plugin into that list -- four lines above its own entry defining it as the second memory backend. The clause is true of the two engines and false of this one, so it now names all three for what they are. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
|
Both checked and both hold. Fixed at 1 (one-definition). Right, and the docstring naming those readers is mine, so this was a promise the code did not keep. Five sites were spelling the distribution themselves, not two: I deliberately left the import package spelled where it is used. It is not in the pairing -- that answers "which distribution", not "which module to import" -- and the local backend keeps its own beside 2 (nit). Correct and self-contradicting four lines apart. The clause is true of the two engines and false of this plugin; the entry now names all three for what they are. Verification on the new head: Thanks for the security pass -- the egress gate and the absent |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the delta from 517f5a365318 and the full github/main...HEAD change, including the affected RPC callers and history, backward compatibility, test intent, and the AGENTS/CONTEXT/import-layer constraints. The five cloud distribution-name reads now follow MEMORY_BACKEND_DISTRIBUTIONS without conflating the separate import-package name, and the CONTEXT wording now describes all three distributed plugins consistently. This delta changes no tests, so it does not weaken prior coverage.
Verification: uv run pytest tests/test_rpc_memory.py tests/test_rpc_settings.py tests/test_config_product_render.py tests/test_core_plugin_stack.py tests/test_import_cycle_budget.py tests/test_kernel_budget.py -q passed 305 tests with one expected case-sensitive-filesystem skip; git diff --check github/main...HEAD passed; git merge-tree --write-tree github/main HEAD completed without conflicts. No open review threads remain from this reviewer.
…b key (#927) ## Summary Two gaps in how Raven stops its cli sub-agents and what environment it starts them with. **Stopping a cli sub-agent on Windows.** A cli sub-agent's timeout, cancel and stream-failure paths, and the `kimi` commands Raven runs to explain a failed Kimi Code connect, ended the process group with `os.killpg`, which Windows does not have. The `AttributeError` replaced the timeout error or the cancellation, and the CLI was left running. Where `killpg` is missing, the tree is now ended from the launcher's pid with `taskkill /T /F` while the launcher is still alive, and the launcher is then killed itself, the way `raven/ops/transport.py` already ends a command on Windows. Once the launcher has exited its pid may name another process, so nothing is killed by number. POSIX keeps the group kill. **A web key rotated through `settings.set`.** `settings.set` rewrites `~/.raven/env` when a web credential changes, but a POSIX cli or acp sub-agent reads that file only through the login-shell capture, which a running Raven takes once and keeps. Every later spawn started with the key the write had replaced, until a Connect, a Test, "Check again" or a restart took the capture again. A mirrored key's write now takes the capture again before the reply. Captures also run one at a time: a refresh that read the shell before the write, a Connect's for instance, could otherwise land after the write's own and put the replaced key back, and readers that arrive while the first capture runs now wait for it rather than each running the shell. Windows is unchanged for keys. Nothing sources `~/.raven/env` there, and a Windows sub-agent's environment is Raven's own with the registry's PATH, so a key held only in config does not reach it; #912 states this in the code. On POSIX the keys reach a sub-agent only through a `source` line in the user's shell startup files, which the onboard wizard adds only with the user's agreement, and a Windows delivery path would need a consent step of its own, so this PR does not add one. Not changed here: an acp sub-agent that is already connected keeps the environment it was launched with until its next launch; a key written by `raven onboard`, which runs as its own process, reaches a running Raven's sub-agents at the next Connect, Test, "Check again" or restart; and on Windows the wizard still prints the POSIX `source` line when `$SHELL` is not bash or zsh. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Linux, Python 3.12, rebased onto main 182daca: - `uv run --frozen --python 3.12 --all-extras pytest -q` with the proxy variables unset: 3 failed, 29667 passed, 158 skipped. The same 3 fail on main on this machine (an npm on PATH, a root-only permission check, a LibreOffice conversion); 0 introduced. - `uv run --frozen --python 3.12 --all-extras pytest tests/test_subagent_third_party.py tests/test_subagent_kimi_code.py tests/test_subagent_acp.py tests/test_subagent_host_env.py tests/test_rpc_console.py tests/test_rpc_settings.py tests/test_rpc_subagents.py -q`: 1080 passed. - Windows stop: `tests/_windows_kill.py` leaves a Windows host as it is, where the real `taskkill` runs, and elsewhere removes `os.killpg` and stands in for `taskkill` by ending the process group it is given, the launcher's tree here; either way it records each call. The launchers, their workers and the Kimi stand-in are Python started on `sys._base_executable`, because a Windows venv's `python.exe` is a redirector that runs the base interpreter as its child and returns a pid the script never sees. The tests have not run on a Windows host, and no CI job runs these test files there. - Windows stop, measured here: the six tests that use the helper pass. With `os.killpg` removed for the whole session and a `taskkill` on PATH that ends the group it is given, they pass again through that `taskkill` (3 calls), and the POSIX-only Kimi test beside them is skipped. With `sys.executable` replaced by a wrapper that runs Python as its child, as a venv's `python.exe` does on Windows, both modes stay green; started on that wrapper instead, the two cancel tests fail on the pid they compare. - Windows stop, against the fix: the six fail with `AttributeError` on main. Removing the alive check in the cli backend, the `taskkill` call or the kill after it in either module, or the cli backend's handling of a `taskkill` that times out, turns at least one of them red. Removing the POSIX group kill from the Kimi Code commands turns only the new test whose stand-in `kimi` starts a process of its own red (1 failed, 347 passed over the two test files). - Rotated key: `test_a_rotated_key_reaches_the_capture_the_next_spawn_reads` saves a key twice through `settings.set` and reads the capture after each save, through a real bash that sources the mirror; on main the second read still returns the first key. Removing the refresh turns 12 tests red; refreshing before the mirror is rewritten turns that test red. Two existing tests that write a web key now stub the refresh, so they do not run the host's login shell. - Concurrency: three tests force the interleavings with events: a refresh that read the shell first and lands after a later one, a second reader that arrives while the first capture runs, and readers waiting on a first capture that fails. Each failed in 3 of 3 runs on main and passes with the lock; removing the lock from either capture path, or either re-check inside the first one, turns one of them red. - With `--cov=raven --cov=raven_everos --cov-branch` added to the full run, `python scripts/coverage_gate.py diff --base-ref origin/main --threshold 90`: 93.22% of the changed lines (55 of 59). The four left are except clauses for a process group or a process that is already gone. - `ruff check`, `ruff format --check`, `lint-imports`, `ty check` and `deptry` over the Makefile targets, `commitlint`, the three repository gate scripts and `pre-commit run --from-ref origin/main --to-ref HEAD`: passed. - `git merge-file` of `raven/rpc/methods/console.py` with each open PR that touches it (#883, #900, #911, #912, #923, #925), and of `raven/agent/subagent/backends/env.py` with #912 in both orders: no conflicts. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed No user-facing docs describe either behavior. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes Saving a web key now waits for the user's login shell to start before the reply, as Connect and Test already do; the capture's 15 s timeout bounds it, and it can also wait for a capture already in flight. A capture that fails keeps the previous one and does not fail the save. No credential reaches a sub-agent that did not before; it is the same shell reading the same file, read again. On Windows a stop now waits up to 5 s for `taskkill`, and a worker that a launcher started and left behind after exiting outlives the stop there, since Windows has no group to address and an exited launcher's pid is not trusted. If `taskkill` is missing, fails or does not answer within those 5 s, only the launcher is killed, and a cli stop then waits for any worker still holding the launcher's output pipes to exit. Revert this PR to roll back. ## Related Issues #908, #912
Summary
Adds
everos-cloud-memory, a separately distributed memory plugin for EverOS Cloud, and wires the host so a second memory plugin is a first-class choice.The plugin (
plugins-dist/everos-cloud-memory) is a thin REST client over httpx: one API key (the config sliceplugins.config.everos-cloud-memory.api_key, orEVEROS_CLOUD_API_KEY), owner ids from the host'sServiceLocator(never from the slice), every add inagentmode, and a flush only when a caller marks the conversation over (metadata.flush/metadata.is_final: the sub-agent handoff and the importer's final batch) and once at stop. It ships its own onboard screen (env or on-file key used without a prompt; 401 / 403 / unreachable get their own sentences with Re-enter or Skip),deleteanswers False (the cloud deletes by scope, nothing calls it), andhealth()classifies seven answers forraven doctorand the settings card.Host changes, each guarded so
memory.backendeverosandnullbehave exactly as before:raven onboardstep 4 asks "Which memory backend?" when more than one plugin contributes a screen (preselects the current backend,Offclears it, cancel exits 1 and writes nothing); with one plugin the step is byte-identical to today.memory.backendnames: the cloud slice's key goes out as a Bearer header and the/healthprobe is skipped for the cloud; a missing distribution is named in the page's note.settings.everosreportsreason: "other_backend"for any non-EverOS backend, so the Model page hides the three EverOS-only roles (embedding stays: knowledge bases use that pin);settings.everosSetrefuses those three sections for another backend;settings.setadmitsplugins.config.<id>.<field>against the plugin's manifest through the same admission door channels use; newsettings.everosCloudfeeds the card (key source, endpoint, health).settings.everosClouddata behind it. No page mounts the card: it drew on Model settings until the owner ruled that a memory backend's endpoint and credential answer neither question that page asks, and where it belongs is the Memory section's to say once that section has a design. Until then the key is entered throughraven onboardorEVEROS_CLOUD_API_KEY. The fixture gate pins the four optional fields the offline library never sends.raven_everos, and a conftest fixture keeps the second plugin out of entry-point discovery for the existing onboarding tests (they describe a one-plugin install).A second topic rides along, because it is what makes the hosted backend reachable from an agent product at all. Every product under
agents/pinned its memory at the stock EverOS address and onlyraven-designread the host's, so the other four addressed whatever answered on 18791 -- the wizard moves a managed server when the port is taken, and a server the user runs themselves is wherever they put it.inherit_memory_backendinraven/config/product_render.pynow carriesmemory.backend, the endpoint, and (only when the product holds none itself) the credential from the host into every render, beside the host denials and plugin opt-outs that already travel there;raven-design's private copy is deleted and the five product configs stop naming an address. Identity never travels, which is what keeps five products apart inside one store.memory.backend: nulltravels like any other value, because that null is how an operator turns long-term memory off and a product that kept writing would be the one surface ignoring them; the product's own default stands only when the host carries no key at all. Which store memories live in is not a product's choice either: the host reads a sub-agent's memories back through its own backend, so a product writing anywhere else left that record empty with nothing reporting it.ownedis forced off whenever the local backend is in force, because a product infers ownership from a slice naming no root -- the shape all five ship -- and so answered a host that was merely not serving by starting a second server against a root of its own, stranding every memory written so far.Decisions worth a reviewer's eye: the fake cloud (
tests/_everos_cloud_fake.py) honours or refuses every field the backend can send and serves both an in-process transport and an HTTP face for the real-host acceptance; the live tests (tests/integration/test_everos_cloud_real_cloud.py) skip withoutEVEROS_CLOUD_API_KEY; run with a real key on 2026-10-09 all six pass against api.evermind.ai (round trip, get by session, empty flush, 401 wording, probe with and without a key, and extraction without a flush within two minutes), so the documentation and the service agree on everything the plugin sends. Design, acceptance cases and plan are underdocs/specs/2026-10-08-everos-cloud-memory-*.mdanddocs/plans/.Type
Verification
Rebased onto main
182dacac0on 2026-10-13. What that rebase and the four commits on top were verified with:uv run --frozen --python 3.12 --extra dev pytest -q-> 29748 passed, 8 failed in 4m10s. All eight fail identically with these four commits' sources reverted in-tree at the same base, so none of them is this branch's: the twotest_the_products_tool_face_*cases in the code and oncall launchers (an extraweb_searchin the visible tool set),test_agent_loop_token_budget,test_rpc_files::test_a_host_without_libreoffice_says_so,test_ppt_engine_image_search,test_shell_background, and the twotest_simulation_*cases.agents/raven-research/run.py) against an isolatedRAVEN_HOMEwhose EverOS address is19701, not the stock port. Before: the product renderedbase_url: http://localhost:18791with noownedkey, ignored the host, and started an EverOS of its own -- an orphanedeveros server start --root <its own state root>/everosholding 18791, which is the stranding this change exists to stop. After: it rendered{"base_url": "http://localhost:19701", "owned": false}and said so itself, "the EverOS you manage is not running at http://localhost:19701 ... Raven does not start or stop it", withpsandlsofconfirming nothing was started. On the cloud the product renderedmemory.backend: everos-cloudwith the host's key and left the out-of-force local slice untouched, andraven doctorover that rendered config answeredeveros-cloud: ok https://api.evermind.ai, so the credential reached the service. Identity stayedraven-research-ngthroughout, and a fingerprint of the operator's own EverOS root was identical before and after.tests/test_import_cycle_budget.py.inherit_memory_backendread the backend-to-distribution pairing fromraven.core.plugin_stack, and that edge fromraven.configintoraven.core-- which importsraven.configback -- took mutual pairs from 13 to 14 and packages inside a cycle from 18 to 22.make lint-importskeeps ten contracts and none of them covers that pair, so it was green throughout. The pairing now lives inraven/config/raven.pybeside the two fields it joins (MemoryConfig.backendnames a contribution,PluginsConfig.configis keyed by the distribution), whichraven.coremay import and which imports nothing back;plugin_stackre-exports the two names its own callers read from it. An intermediate commit put it inraven.contractsinstead, which broke the cycle but took the papers ceiling from 3700 to 3719 lines --tests/test_kernel_budget.pysays a new paper is a reviewed change to that ceiling, so it moved again rather than raising it.memory.backendwith.get(), which collapses an explicitnullinto an absent key, so a host with long-term memory off left every packaged agent on its own shipped backend and still writing. Reproduced by calling the function directly before the change and after, now read on presence, and pinned by a case whose mutation (restoring the.get()) turns it and nothing else red.tests/test_rpc_settings.pyrun beforetests/test_cli_onboard_commands.pyleavesraven.home._current_config_pathpointing at a dead tmp config, so the onboard cases write somewhere else. With main's own copies of both files and main's conftest in the tree it is 57 failures; with this branch's, 19. Which cases turn red changes run to run as xdist distributes them differently.cd ui-web && npx vitest run-> 876 files, 3728 passed, 0 failed;npm run lint-> 0 errors (4 warnings, all in files this change does not touch);npm run gen:check-> in sync (228 methods);npm run type-check-> clean;cd ui-tui && npm run lint:i18n && npm run lint:rpc-> in syncmake lint-python/lint-imports(10 contracts kept) /lint-types/lint-deps-> all pass;uv lock --check-> exit 0 with both sides' new packages present;scripts/check_source_language.pyandscripts/check_large_files.pyongithub/main..HEAD-> exit 0; commitlint over every commit -> okvision.model/vision.providersimple keys meeting this branch's newplugins.config.<id>.<field>branch insettings_set; nothing in the suite exercisesvision.*through that method, so it was run directly ->{'applied': True}and{"model": "..."}on disk. The subtraction decisions were re-checked against the new base: no new caller of a memory backend'sdelete, main's new onboard cases are about TLS and not the memory step,inherit_everos_addressexists nowhere, andDEFAULT_EVEROS_BASE_URLis still defined.owned: false, letting the host's credential overwrite the product's own, not writing the host's backend name, and carrying every memory slice instead of the one in force each turn their own cases red; making the cloud card render nothing turns 4 of its 5 new cases red and leaves the fifth ("draws nothing when not selected") green.From the earlier round, on base
6d509a8c5:uv run --frozen --python 3.12 --extra dev pytest -p no:cacheprovider -q tests/test_everos_cloud_fake.py tests/test_everos_cloud_backend.py tests/test_everos_cloud_backend_contract.py tests/test_everos_cloud_onboard.py tests/test_everos_cloud_discover.py tests/test_cli_onboard_commands.py tests/test_rpc_memory.py tests/test_rpc_settings.py tests/test_plugin_boundary.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py tests/test_release_plugin_list.py tests/integration/test_everos_cloud_real_cloud.py-> 1001 passed, 6 skipped (the six live tests, no key)cd ui-web && npx vitest run-> 214 files, 3211 passed;npm run lint-> 0 errors (4 warnings, all in files this change does not touch);npm run gen:check-> in sync (207 methods);cd ui-tui && npm run lint:rpc && npm run lint:i18n-> in syncmake lint(ruff, ruff format, ui tsc, deptry) -> ok;make lint-types-> All checks passed;make build-ui-> boot snapshots match;scripts/check_large_files.pyandscripts/check_source_language.pyon6d509a8c5..HEAD-> exit 0; commitlint andscripts/check_commit_messages.pyon each commit -> okgit diffclean): a host import of the plugin turns the boundary test red;import raven_everosin the plugin turns the import-isolation test red; removing the chooser's cancel exit, the Bearer header, the restart guard, the slice-admission guard, the Roles filter and the fake's two fixes each turn their own test reddocs/specs/2026-10-08-everos-cloud-memory-acceptance.md) run against a fake EverOS Cloud: the wizard in tmux (chooser, key typed / from env / on file, 401 / 403 / unreachable wording, back, Off, Ctrl-C, non-interactive, base-tree transcript byte-identical with one plugin),raven agentturns (user and agent-track recall, agent-mode adds, 4 s recall bound, a 300 KB message dropped with a warning, a sub-agent trace stored under its own ids, flushed and read back, one exit flush per unflushed session),raven import run(3 adds then one flush; a hung flush retried 4x and the exit sweep defeated),raven doctorunder seven cloud conditions (seven sentences, exit 2 only for the five faults) and with the distribution hidden, and the web page through playwright-cli (memory page counts and the fake's episode, empty and refused stores, the Model page hiding the three EverOS-only roles while the embedding row stays, the cloud card rotating a key and showing the 401 wording, three refused settings.set frames over the gateway WebSocket, the local everos / null / mem0 backends drawing the page as before, and a provider-key save that restarts the local EverOS on the base tree and does not on this branch). Evidence and the per-case verdicts are kept outside the repo. The live service was reached with a real key after the acceptance run: six tests, six passes, the service's replies archived with the evidence.Risk
User-visible changes: the wizard's step 4 chooser (only when two memory plugins are installed); Model settings hides the three EverOS-only role rows when
memory.backendis noteveros/nulland draws no cloud card; the memory page sends a Bearer header for the cloud backend.Two behaviour changes for agent products, both on the
agents/lane only. An agent product now follows the host'smemory.backendand endpoint instead of its own, which is a fix for the four products that never followed the host and a change forraven-design, which followed the endpoint but not the backend. And a product no longer starts an EverOS server of its own: where it used to answer a host that was not serving by starting one against an empty root of its own, it now reports long-term memory as unavailable, which is what the host does. Rollback for this part is the one commit; the product configs it edits are shipped files, not user config. The plugin is not installed by default (its wheel is built by the release workflow; nothing changes for an install without it). Rollback: revert the squash commit; the config schema gains no new key (the slice is a plugin slice), so an older raven reads a config written by this one.Credentials: the cloud key is written only by
set_plugin_config_fieldsinto the plugin's slice; the settings page's key row goes throughsettings.setwith a manifest-declared field, andsettings.everosCloudreportsapi_key_set/key_source, never the value.Related Issues
N/A