Skip to content

refactor(*): ask who runs everos once and drop discovery-time ownership - #343

Open
gloryfromca wants to merge 4 commits into
mainfrom
fix/everos_self_managed_ownership
Open

refactor(*): ask who runs everos once and drop discovery-time ownership#343
gloryfromca wants to merge 4 commits into
mainfrom
fix/everos_self_managed_ownership

Conversation

@gloryfromca

@gloryfromca gloryfromca commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #310 that grew into removing the thing both of its leftover findings
came from. Ownership of an EverOS root was treated as something the wizard could
observe: discovery reported it per candidate, and five nested menus each
re-derived what raven was allowed to do from whatever happened to be on disk.
Every fix in this area has landed on the least-used of those states, and the two
that opened this branch were the same shape again -- a menu that did not consult
ownership, and a leftover root adopted before any menu appeared.

It is a decision, not an observation. Step 4 now asks one question -- raven runs
EverOS, the user runs it, or neither today -- and ownership follows from the
answer, so nothing downstream reads it back off a directory. The managed lane
takes over whatever memory directory it finds, whatever an earlier run recorded
there, and builds its own when there is none. RootState.owned is gone.

The found-root question also moved in front of the work it authorises.
Convergence used to stop and restart the service before asking, so an install
that only wanted confirming was moved to another port unprompted. "Use it as it
is" now touches nothing and keeps the address the service already answers on;
relocating is what "reconfigure" means. A refused self-managed address returns to
the lane question instead of ending the step, and skipping is an answer of its
own rather than something reached by backing out of a lane.

Two states have no way forward and are reported rather than worked around: a
directory held by something that serves no HTTP (pid and command line), and a
start that failed (the reason EverOS gave). One memory directory admits one
engine, so there is nothing to offer there.

Also kept from the original branch: _proc_locks_pid's companion port lookup no
longer depends on lsof. LockHolder.port falls back to /proc/net/tcp matched
against the process's own socket inodes, and the /proc/locks branch is asked
first because it can tell a holder from a blocked waiter, which lsof -t cannot.
That lookup is now what decides whether a busy directory can be taken over at
all.

Removed: _enabled_unowned_menu, _reuse_unowned_root, _converge_owned_root,
_adopt_or_move, _adopt_running_address, _stop_failure_line, _same_address,
RootState.owned. _record_root and _adopt_own_root collapse into
_adopt_root. Net about 480 lines lighter. The runtime is untouched: backend,
tools and doctor read the recorded owned field, whose meaning is unchanged
(raven may write and start it, or connect only).

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

uv run pytest tests/ -q
1 failed, 6643 passed, 33 skipped, 13 deselected in 122.58s

uv run pytest tests/test_cli_onboard_commands.py tests/test_everos_discover.py -q
267 passed in 3.18s

uv run ruff check raven/ tests/            # All checks passed
uv run ruff format --check raven/ tests/   # 830 files already formatted

The single failure is tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare.
It reproduces with every change on this branch stashed, when that file runs as a
whole rather than as a single node -- ordering pollution inside it, macOS-local,
green in CI on Linux. This branch introduces no failures.

Two test-isolation faults surfaced while rewriting and are fixed in the memory
fixture: a real bind test and real discovery decided which branch a test took, so
a machine already running everos on 18791 met an unscripted prompt and died on
EOF, and every managed-lane test probed the developer's own /health. The onboard
file now runs in 3s instead of 100s.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Risk

User-visible behaviour changes:

  • Step 4 opens with one question and three answers (managed / self-managed /
    skip). The separate keep-or-reconfigure screens for an already-configured
    install are gone; the same three answers cover them.
  • Taking over a found root asks before acting. "Use it as it is" no longer stops,
    moves or reconfigures the service, which also means a pre-upgrade port survives
    a reuse -- moving to the standard port is what "reconfigure" does.
  • A root recorded as the user's is taken over by the managed lane instead of being
    offered as read-only reuse. Read-only use of an EverOS someone else runs is the
    self-managed lane, which records an address and no root.
  • A refused self-managed address no longer turns memory off; it re-asks.

Not verified here: the Windows branch (unchanged, still skips and disables), and
the report path for a real everos demo holding the lock (covered with a stubbed
lock holder, not against a live process).

Removing the recorded-root-owned-false state means raven no longer reads a
user-managed root's everos.toml, so capability reporting for that setup relies
on /health alone, and make_understand_media_tool still exports EVEROS_ROOT
from the fallback root there -- a finding from #310's review that this branch
does not address either.

Rollback: revert the branch. No config keys are added, removed or renamed; the
owned field keeps its meaning and its readers.

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

Follow-up to #310. Still open from its review, not addressed here: doctor's
retrieval line cannot tell "server down" from "server too old to report"; the
win32 branch leaves the service state at UNKNOWN so every write counts as
dropped; drain_backend_stores neither cancels nor counts writes that outlive
its budget before stop() reads the total; and MemoryBackend's contract test
gained no assertion for store() -> bool, so a backend returning None still
passes it.

@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review

No blockers; this can merge as far as I can tell.

Delta reviewed: the new commit fbebbc2 ("ask who runs everos once and drop discovery-time ownership"), on top of the two previously-reviewed fix commits. This is a substantial rewrite of onboard_everos.py and _discover.py (removes _enabled_unowned_menu, _reuse_unowned_root, _converge_owned_root, _adopt_or_move, _adopt_running_address, _stop_failure_line, _same_address, RootState.owned; collapses _record_root/_adopt_own_root into _adopt_root). raven/plugin/memory/everos/_server.py and tests/test_everos_server.py are untouched by this commit, so the earlier nonblocking notes on the lock/port code (inode-only match, lsof-first ordering) still stand as-is and don't need re-raising.

What I checked:

  • Traced _step4_memory's new single-question flow (_memory_source_menu → managed/self/skip) end to end, including the discovery sub-loop (_found_root_menu → reuse/redo/back) and both places _adopt_root runs.
  • Confirmed the double-ask this PR's earlier review comments flagged (picking "hand over to Raven" landing back on the same source question) is gone: source == "self"/"managed"/"skip" now each resolve to a single terminal action or fall straight into role configuration — no path loops back to _memory_source_menu after a lane is taken.
  • Confirmed _adopt_root's theirs = ...get("owned") is False retraction (the port-inheritance fix from the previous round) survived the rewrite and is exercised by both discovery paths (test_the_managed_lane_does_not_inherit_their_address, test_a_root_recorded_as_the_users_is_taken_over_all_the_same).
  • Grepped for dangling references to the removed symbols (_enabled_unowned_menu, _converge_owned_root, RootState.owned, etc.) across raven/ and tests/ — none found; _discover has exactly one caller (onboard_everos.py), so dropping RootState.owned breaks nothing else.
  • Read through the new/changed tests in TestTheLaneDecidesOwnership, TestALeftoverRootIsOnlyTakenOverOnPurpose, and the reconfigure/reuse test classes — these assert on recorded config state and captured output (pid/cmdline reporting, port numbers), not on source text, so the "asserts on source, not behaviour" issue flagged on the prior round's test doesn't reappear here.
  • Commit message: ASCII-clean, Conventional-Commits header (refactor(*): ...) within length, Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com> matches AGENTS.md 3.3 (the non-conforming trailer form flagged on the earlier commits is unchanged by this commit and stays the author's call per 3.4).
  • No dependency, asset, or large-file changes in this diff.

Not verified: I could not execute uv run pytest in this sandbox (the uv/pytest invocation itself required interactive approval that wasn't available here) — I relied on static tracing of the control flow and the config-state assertions in the test file rather than an actual run. CI's own test job was still in progress at review time.

View job run

Comment thread raven/cli/onboard_everos.py Outdated
# An explicit handover. Recorded here rather than left to the branch that
# builds the root, because everything downstream -- _memory_enabled(),
# owned_everos_root() -- has to see the new answer first.
_record_root(default_everos_root(), owned=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] When a self-managed install picks "Let Raven run its own EverOS instead" and no leftover raven root exists yet (the common case for a first handover), found stays None and _memory_enabled() is still False here (the new root has no [llm] configured), so control falls into the if found is None and not _memory_enabled(): block below (line 2138) and re-asks "Where should long-term memory come from?" -- the same managed-vs-self choice the user just answered one screen earlier.

It's not destructive either way (answering "managed" again is a no-op; answering "self" cleanly reverts via _use_self_managed_everos(), which overwrites owned/base_url and drops root), but it undercuts "the handover is an answer rather than a side effect" from the PR description -- the user can end up second-guessing a decision they already confirmed. Worth considering an early _set_memory_backend/skip of that second prompt once the handover is recorded here, so "managed" falls straight through to configuring roles.

Note: this same double-ask already exists for the sibling _OWN_ROOT_INSTEAD path (line 2130), so it may be a pre-existing quirk this PR simply extends to a new entry point rather than a fresh regression -- flagging for awareness rather than as a blocker.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and independently reproduced: driving _step4_memory with a self-managed slice and answering managed yields two selects, the second offering ['managed', 'self']. Same conclusion on severity too -- the recovery paths are clean, and the sibling _OWN_ROOT_INSTEAD path at 2130 already does this, so it is an extended quirk rather than a regression. It is the same point as my note one line up at 2109.

One correction to "it's not destructive either way": on this exact path it is, just not through the prompt. _record_root merges, so the handover leaves the user's base_url in the slice, and _ask_managed_port derives the managed port from it (urlparse(base_url).port, since port was removed on the way in). Answering "managed" a second time therefore configures raven's own server on the user's port -- silently on 8000 when their server is stopped, or with Port 8000 is already in use by something else plus an 8000 default when it is still up. Details and a fix in the inline note at 2114.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking on one thing: the self-managed -> managed handover keeps the user's address in the config, so raven's own server ends up configured on the user's port -- inline at raven/cli/onboard_everos.py:2114. Everything else here is nonblocking, and the shape of the change is right.

Reviewed as the author, so I traced the new paths rather than trusting the reasoning in the comments.

What I covered: the diff; the callers and surrounding flow (_step4_memory ordering, _ask_managed_port, _configured_target_url, _converge_owned_root, lock_holder); whether the changed/added tests assert behaviour; backward compatibility of recorded-config shapes; AGENTS.md rules. Not covered: anything on a real Linux host -- the /proc code was read, not run.

The two fixes themselves hold up. Gating ahead of discovery is the right place: with a owned: false slice the leftover-root test never even reaches pick(), which is the point. On the server side the /proc/net/tcp field indices check out against a real row (inode at index 9, port as hex in field 1, 0A for LISTEN); tcp6's header line survives the concatenation but the 0A test drops it. The /proc/locks-first reorder is a genuine improvement -- lsof -t does list waiters alongside the holder -- and it is a no-op on darwin, where /proc/locks is absent.

Test state, honestly:

  • uv run pytest tests/test_everos_server.py -> 61 passed.
  • uv run pytest tests/test_cli_onboard_commands.py -> 11 failed, 257 total on this host, and none of it is this PR. A real EverOS is listening on 18791 on this machine, so _port_is_free returns false and _ask_managed_port injects a port prompt into 11 tests that never stubbed one. Neutralizing that single host fact (autouse _port_is_free -> True) turns the file green: 257 passed. That host-dependence is pre-existing and on untouched lines, so it is not counted against this branch -- but it is worth its own fix, since any developer running everos locally sees 11 red tests.

Two small notes, neither a blocker:

  1. Both commits carry Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>, while AGENTS.md 3.3 and every co-authored commit on main use Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>. Squash collects the trailer, so the non-conforming form is what lands on main. Rewriting a message needs your say-so per 3.4, so it stays yours to call.
  2. _proc_locks_pid matches the inode only, dropping the MAJOR:MINOR half of what /proc/locks records. That was harmless while lsof answered first on Linux; it is now the primary source, so a file with the same inode number on another filesystem holding a lock returns a stranger's pid, lock_holder discards it on the cmdline check, and a healthy server reads as "no holder" -- the same class of misdiagnosis this PR is fixing, arriving through another door. Long odds, and the fix is one line (lock.stat().st_dev against os.makedev of the two fields). Take it or leave it.

Branch name, commit types and scopes, ASCII, test file placement and the no-assets rule are all fine.

Comment thread raven/cli/onboard_everos.py Outdated
# An explicit handover. Recorded here rather than left to the branch that
# builds the root, because everything downstream -- _memory_enabled(),
# owned_everos_root() -- has to see the new answer first.
_record_root(default_everos_root(), owned=True)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handover records raven's own root but leaves the user's base_url behind, so the managed server inherits the user's port.

_record_root merges, so after this line the slice is {owned: true, root: <raven root>, base_url: http://127.0.0.1:8000} -- the address the screen above just promised raven would stop using. _ask_managed_port then reads it: port was removed on the way in by _use_self_managed_everos, so current falls back to urlparse(base_url).port, i.e. 8000. _configured_target_url refuses exactly that fallback on purpose ("Deliberately not falling back to the recorded address"); _ask_managed_port does not, and after this handover the recorded address is no longer raven's.

Traced by driving _step4_memory with a self-managed slice at http://127.0.0.1:8000 and answering managed:

  • Their server stopped (they shut it down, then re-ran onboard -- the natural order). Every port looks free, _ask_managed_port returns 8000 without a word, and the final slice is {"owned": true, "base_url": "http://localhost:8000", "port": 8000, "root": <raven root>}. Raven's managed EverOS is now parked on their port: the next time they start their own server the two collide, and 18791 was never mentioned.
  • Their server still up (what the handover text explicitly promises: "That server and its data are untouched"). Output is ! Port 8000 is already in use by something else. -- that "something else" is their own EverOS -- and the port prompt then defaults to 8000, so an Enter records 8000 and the start a few lines later fails.

The pre-existing TestReusingAnEverosTheUserManages case lands the same way (its http://localhost:8000 is carried into a managed root); it just never asserts on base_url, so it stays green.

Smallest fix I see is to drop the stale address in the same write, mirroring how _use_self_managed_everos clears root/port on the way in:

set_plugin_config_fields(
    "everos-memory",
    {"root": str(default_everos_root()), "owned": True},
    remove=("base_url", "port"),
)

so _ask_managed_port falls through to the 18791 default. The other option -- teaching _ask_managed_port to ignore an address that is not raven's -- needs it to know ownership just flipped, which is more moving parts for the same outcome. Your pick.

Comment thread raven/cli/onboard_everos.py Outdated
# so an abandoned managed root -- the normal leftover after switching --
# would be picked and adopted before any menu appeared. A recorded
# ownership decision outranks a directory that happens to still exist.
if _enabled_unowned_menu() is not _SWITCH_TO_MANAGED:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonblocking: taking the handover asks the same question twice.

The user picks "Let Raven run its own EverOS instead"; the next screen is "Where should long-term memory come from? / Let Raven run EverOS for me / I run my own EverOS". Cause is two lines down: _record_root(default_everos_root(), owned=True) leaves _memory_enabled() false for a fresh root (no llm role yet), so the found is None and not _memory_enabled() block fires the source question. Traced: two selects, the second one offering ['managed', 'self'].

This is the same shape as the pre-existing _OWN_ROOT_INSTEAD path, so it is not new behaviour -- but there the preceding screen asked something else, whereas here the second screen is verbatim the question just answered, which reads as if the answer did not register. If you want it gone the handover has to tell the source question it is already settled (a local flag, or fall straight into the managed build). Fine to leave for later.

Comment thread tests/test_cli_onboard_commands.py Outdated
from raven.cli import onboard_everos

src = inspect.getsource(onboard_everos._step4_memory)
assert "_enabled_unowned_menu" in src, "the enabled branch does not consult ownership"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test asserts on the source text of _step4_memory, not on behaviour.

Delete the _config_everos_role loop outright and it still passes; rename the helper without changing anything and it fails. It also declares everos_isolated and monkeypatch without using either, and its name plus docstring claim a coverage ("a managed install still gets the model roles") that the assertion does not provide.

TestReusingAnEverosTheUserManages already drives the real path and asserts len(reached) == 4, so the coverage is not missing -- this file just gains a green line that would not catch the regression it is named after. Either do the same here with an owned: true slice, or drop it.


def _listening_port(pid: int) -> int | None:
"""The TCP port ``pid`` listens on, or ``None`` if it serves no HTTP."""
port = _lsof_listening_port(pid)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonblocking, and it is this commit's own argument turned around: _lock_holder_pid was reordered because "asking lsof first meant that branch never ran on a Linux box that has lsof -- including the one it was validated on". Behind lsof, _proc_net_listening_port is in the same position: it runs only on a Linux host with no lsof, so no ordinary dev box or CI runner ever executes it, and both new tests stub out the two /proc readers it depends on (_proc_net_rows, _socket_inodes_of) -- the parsing is covered, the reads are not.

Nothing is wrong today: both sources agree wherever both work. Reading /proc first would exercise the new path where it is actually developed and skip a subprocess with a 5s timeout. test_lsof_is_still_used_when_present pins the current order deliberately, so this is a decision, not an oversight -- flagging it so it is a conscious one.

gloryfromca and others added 3 commits August 17, 2026 21:38
Configuring models is not an action that exists for a server the user runs --
that is raven writing into a root it owns, which by definition it does not. The
enabled menu did not look at ownership, so a self-managed install got the
managed one, and "Reconfigure" was the only plausible button for changing the
address. It answered by walking the four model roles and recording raven's own
root, flipping owned to true and overwriting the address, none of it confirmed;
backing out of the roles then turned memory off after the config had already
been rewritten.

A self-managed install now gets its own three answers: keep it, point raven at
a different address, or hand the job to raven. The handover is an answer rather
than a side effect, and it says what it costs first -- their server and its data
are untouched, raven just stops reading them.

The branch sits ahead of discovery, which is the other half of the same fault.
discover() marks raven's own roots owned=True unconditionally, so an abandoned
managed root -- the ordinary leftover after switching -- was picked and adopted
before any menu appeared. A recorded ownership decision outranks a directory
that happens to still exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_proc_locks_pid exists because minimal Linux images routinely omit lsof, and
the port lookup beside it used lsof and nothing else. Without it
LockHolder.port is None, which the type documents as "holds the lock but serves
no HTTP" -- so in a container a healthy raven-managed server is described as a
squatter, with the wizard reporting its own service as "already in use by
something else" and convergence telling the user to stop a server that is
serving fine. The port now falls back to /proc/net/tcp matched against the
process's own socket inodes, needing no external binary.

The lock lookup also asked lsof first, which meant the /proc/locks branch never
ran on a Linux box that has lsof -- including the one it was validated on. That
branch is the one that tells a holder from a blocked waiter, while `lsof -t`
lists both and its first line is not reliably the holder, so it is now the
second choice rather than the first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… root

Handing memory over from a self-managed everos recorded raven's root and
ownership with a merging write, so the user's base_url stayed in the slice.
_ask_managed_port reads exactly that as where raven is meant to listen: port
was removed on the way in, so it falls back to urlparse(base_url).port. Raven's
own service was therefore configured on the user's port, one screen after
promising to stop using it -- silently when their server is stopped, and with
"already in use by something else" naming their own everos when it is not.

_adopt_own_root retracts base_url and port in the same write, and only when the
record says the address was theirs. Declining to share a discovered root reaches
the same line with a managed port the user deliberately moved to, and dropping
that one would offer the shipped default again on the next run.

test_declining_falls_through_to_ravens_own_root now stubs _port_is_free: with
the address retracted, the managed default decides its outcome, which made it
depend on whether the host happens to run an everos of its own.

Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>
@gloryfromca
gloryfromca force-pushed the fix/everos_self_managed_ownership branch from 31c2609 to c7f03d0 Compare August 17, 2026 13:39
@gloryfromca
gloryfromca requested a review from 0xKT August 17, 2026 13:40

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned. The port inheritance I blocked on is fixed, and I re-verified it independently rather than reading the new tests.

Delta since my last pass: the branch was rebased onto current main and gained one commit, c7f03d0e. git diff 31c26095 7bbf0109 over the four touched files is empty, so the previously reviewed content came through the rebase untouched -- the only new material is _adopt_own_root.

The fix holds. Re-running my own harness against this head, both variants now land on 18791: their server stopped -> {"owned": true, "root": <raven root>, "base_url": "http://localhost:18791", "port": 18791}; their server still listening on 8000 -> same result and no port prompt at all, where before it printed already in use by something else and defaulted the prompt to their port.

owned is False is the right discriminator, and the else branch is not over-broad -- I checked all three ways into that line:

  • the handover (record still says owned: false) -> retracts, which is the fix;
  • a fresh install (no slice at all, .get("owned") is None) -> remove=(), byte-identical to the old _record_root;
  • declining to share a discovered root with a managed port the user moved to -> kept, and unit-tested directly.

Mechanics check out too: remove=() is safe (pop(key, None) per key, and the log line's if remove handles the empty tuple), and dropping the default_everos_root import from _step4_memory leaves no dangling reference -- the only remaining use is inside the new helper, which is also what makes the test's monkeypatch.setattr(ue, "default_everos_root", ...) bite.

The test change is a strengthening, not a weakening. test_declining_falls_through_to_ravens_own_root now stubs _port_is_free: the port was never that test's subject, the new base_url/port assertions pin the outcome that matters, and the stub removes a dependency on whether the host happens to be running an EverOS. That is the same root-cause fix the 11 host-dependent tests below still need.

Test state on this host:

  • uv run pytest tests/test_cli_onboard_commands.py tests/test_everos_server.py -> 11 failed, 310 passed. All 11 are the identical pre-existing host-dependence from my last pass (a real EverOS listening on 18791 here makes _ask_managed_port inject an unstubbed prompt); the set of failing names is unchanged by this commit.
  • Neutralizing that one host fact (autouse _port_is_free -> True) -> tests/test_cli_onboard_commands.py 260 passed, i.e. the three new tests pass and nothing else moved. tests/test_everos_server.py -> 61 passed on its own.

Still open, all nonblocking and already argued -- not repeating the detail: the double-ask after the handover (now acknowledged in a comment in the new test), test_a_managed_install_still_gets_the_model_roles asserting on source text, _proc_net_listening_port sitting behind lsof, and the inode-only /proc/locks match. None of them has to move for this to land.

One bookkeeping note: c7f03d0e uses the AGENTS.md 3.3 trailer form, but the two rebased commits still carry Co-Authored-By: Claude Opus 5 (1M context). Squash collects every commit's trailer, so both forms end up on the squash commit. Rewriting messages needs your say-so per 3.4, so it stays your call.

Covered this pass: the delta, all three call sites of the new helper, set_plugin_config_fields remove semantics, rebase integrity of the previously reviewed content, the new and modified tests, and AGENTS.md. Not covered: a real Linux host -- the /proc code is still read, not run.

Ownership was a property the wizard tried to observe: discovery reported it
per candidate root, and five nested menus each re-derived what raven was
allowed to do from whatever happened to be on disk. Every fix in this area
landed on the least-used of those states, and the last two were the same
shape again -- a menu that did not consult ownership, and a leftover root
adopted before any menu appeared.

It is a decision, not an observation. The step now asks one question -- raven
runs everos, the user runs it, or neither today -- and ownership follows from
the answer, so nothing downstream reads it back off a directory. The managed
lane takes over whatever memory directory it finds, whatever an earlier run
recorded there, and builds its own when there is none.

The found-root question also moved in front of the work it authorises.
Convergence used to stop and restart the service before asking, so an install
that only wanted confirming was moved to another port unprompted; "use it as
it is" now touches nothing and keeps the address the service already answers
on, while relocating is what "reconfigure" means. A refused self-managed
address returns to the lane question instead of ending the step, and skipping
is an answer rather than something reached by backing out of a lane.

Two states have no way forward and are reported rather than worked around:
a directory held by something that serves no HTTP (pid and command line), and
a start that failed (the reason everos gave). One memory directory admits one
engine, so there is nothing to offer there.

Removed: _enabled_unowned_menu, _reuse_unowned_root, _converge_owned_root,
_adopt_or_move, _adopt_running_address, _stop_failure_line, _same_address and
RootState.owned. _record_root and _adopt_own_root collapse into _adopt_root.
The runtime is untouched: backend, tools and doctor read the recorded owned
field, whose meaning is unchanged.

Two test-isolation faults surfaced while rewriting: the memory fixture let a
real bind test and real discovery decide which branch a test took, so a box
already running everos on 18791 met an unscripted prompt and died on EOF, and
every managed-lane test probed the developer's own /health. Both are pinned in
the fixture; the file now runs in 3s instead of 100s.

Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>
@gloryfromca gloryfromca changed the title fix(cli): keep a self-managed everos out of the managed reconfigure path refactor(*): ask who runs everos once and drop discovery-time ownership Aug 17, 2026

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the managed lane takes over, reconfigures and restarts an EverOS the config records as one the user runs -- inline at raven/plugin/memory/everos/_discover.py:138. Everything else in this refactor is an improvement, and I would sign off the moment that one case is handled.

Reviewed the whole rewrite rather than the delta, since it replaces the step wholesale (-963/+484, eight helpers and RootState.owned gone).

What I covered: the new _step4_memory and all three lanes; _memory_source_menu / _found_root_menu / _use_found_root / _adopt_root; the discover() and pick() change and every remaining reader of ownership (doctor_commands.py via everos_owned(), loader.py migration, owned_everos_root); the deleted tests against the code they covered; AGENTS.md; and whether the fixture changes hide anything. Not covered: a real Linux host, and the runtime/session path beyond confirming it reads the same recorded owned field.

The refactor lands three things I raised, properly:

  • The double-ask is gone -- one lane question, no second screen re-asking it.
  • test_a_managed_install_still_gets_the_model_roles, the test that asserted on the source text of _step4_memory, is deleted rather than patched.
  • The host-dependence I flagged is fixed at the root: pinning _port_is_free and discover in the everos_isolated fixture. On this machine, with a real EverOS still listening on 18791, tests/test_cli_onboard_commands.py + test_everos_discover.py + test_everos_server.py + test_config_update_everos.py -> 371 passed, 0 failed, where the same host produced 11 failures two revisions ago. The pin is a default, not a blanket: six tests still patch _port_is_free to False, so the occupied-port branch is genuinely exercised.

On the deleted tests. _same_address and _adopt_running_address tests went with the functions they tested, which is right. test_reuse_records_the_address_and_writes_nothing was replaced by test_a_root_recorded_as_the_users_is_taken_over_all_the_same, which asserts the opposite outcome -- that is an honest specification change, pinned by a test, not a test weakened to look green. It is that specification I am disputing in the inline note, not the way it was tested.

Also checked and clean: RootState.owned has no readers left anywhere (doctor reads the recorded field, whose meaning is unchanged); root_is_raven_owned is still live in loader.py and everos_owned(), so nothing became dead code; _describe sets alive=False whenever no address is declared, so _use_found_root's str(state.declared_url) cannot stringify None; _adopt_root's retraction still only fires on owned is False, and the "managed port the user moved to survives" case keeps its unit test. "Reuse" reaches _config_everos_role nowhere, which is the promise the new menu makes.

Still open from earlier, unchanged and nonblocking: _proc_net_listening_port sitting behind lsof in _server.py -- untouched by this commit, already argued, no need to move it here.

root = Path(str(recorded)).expanduser()
owned = bool(slice_["owned"]) if "owned" in slice_ else root_is_raven_owned(root)
add(root, owned=owned)
add(Path(str(recorded)).expanduser())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the managed lane will take over, write into, and restart an EverOS the config says the user runs.

discover() now adds the recorded root unconditionally and pick() returns the first configured candidate -- which is the recorded one, since it is added first. When the recorded slice is {root: <the user's dir>, owned: false}, the managed lane offers that directory as "a memory directory Raven can take over", and "Reconfigure it (port and models)" writes into it.

Traced end to end with real discovery doing the picking (only default_everos_root / applicable_legacy_root redirected somewhere harmless, and their /health answering):

config:  {"root": "<theirs>", "owned": false, "base_url": "http://localhost:8000"}
theirs/everos.toml:  [llm] model = "their-private-model", api_key = "their-secret-key"
                     [api] host = "localhost", port = 8000

  v Found a memory directory Raven can take over
      memory dir  <theirs>
      state       running at http://localhost:8000
  -> answer "redo"

ensure_everos_home       -> <theirs>        # templates written into their directory
configure_everos_env     -> <theirs>
model roles written into -> llm, embedding, rerank, multimodal @ <theirs>
stop_for_reload          -> <theirs>        # their running server signalled
server started at        -> http://localhost:18791
final slice              -> {"root": "<theirs>", "owned": true, "port": 18791}

Their [llm]/[embedding]/[rerank]/[multimodal] sections and the keys in them are replaced, their process is stopped, and the service is moved off the port they chose. That is the case EverosRootNotOwnedError exists for, and the test this commit deleted asserted it must not happen ("adopted the root the user declined to share"). The docstring immediately above this line still promises it: "An EverOS the user runs is never discovered."

I read the commit message, so I know the takeover is deliberate -- "the managed lane takes over whatever memory directory it finds, whatever an earlier run recorded there" -- and for a root that is raven's own that is exactly right. My objection is only to the case where the record says the directory is not raven's.

Reachability, stated plainly. _use_self_managed_everos retracts root, so no config written by current raven holds this shape; the migration at raven/config/loader.py:516-523 fills a missing root with fallback_everos_root(), which is raven's own. The exposure is pre-#310 configs, which recorded whatever root was active including the user's -- exactly what the comment on the deleted test said ("A recorded unowned root only reaches this code from a config written before self-managed setups stopped recording one"). A bounded population, not a hypothetical one. If you can establish that no such config exists in the wild, this drops to fixing the docstring.

Two ways out, both small:

  1. Exclude it in discovery -- skip the recorded root when the recorded slice says owned is False. The managed lane then finds raven's own root or none and builds there, the self-managed lane is untouched, and the docstring above becomes true again. Cost: someone who genuinely wants to hand their directory over has to point the managed lane at a fresh root, which is what the old design implied anyway.
  2. Keep the takeover, make it informed -- have _found_root_menu recognise owned: false and say it: this is the directory you told raven you run, reconfiguring replaces the models and keys in it, and the running service will be stopped. Cost: the wizard keeps reading owned for routing, which is part of what this refactor set out to delete -- though _adopt_root reads it anyway, three lines later, for the address retraction.

I would take the first: one condition restores the invariant, whereas the second leaves a destructive answer one keystroke away. Your call either way.

oc.console.print()
oc.console.print(
oc._t(
f" [green]v Found a memory directory Raven can take over[/green]\n"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Narrower point, and it stands whichever way you resolve the other note: "Use it as it is" changes hands without saying so.

_adopt_root(found.root) runs before the branch, so on a root the record marks owned: false the safest-sounding answer -- the one that genuinely touches nothing this run -- still writes owned: true against the user's directory. From then on everos_owned() is true, so doctor reports the directory as raven's, and the runtime and the next raven onboard are free to write to it and to start and stop it. The screen says "Found a memory directory Raven can take over" and never mentions that this is the one raven was told not to touch.

Compounding it slightly: the lane question is now asked cold, with no line stating the current setup. The previous revision opened with "Long-term memory uses the EverOS you run at http://..."; a self-managed user re-running the wizard now sees three lanes with "Let Raven run EverOS for me" first, and two Enters reach this screen. If you keep the takeover design, this is the line where the change of hands should be stated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant