Skip to content

feat(*): add hermes as a cold-start import source - #264

Merged
Kendrick-Song merged 65 commits into
mainfrom
feat/hermes_cold_start_import
Aug 4, 2026
Merged

feat(*): add hermes as a cold-start import source#264
Kendrick-Song merged 65 commits into
mainfrom
feat/hermes_cold_start_import

Conversation

@Kendrick-Song

@Kendrick-Song Kendrick-Song commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Hermes as a cold-start import source, alongside the existing Claude Code
scanner, across three kinds of data.

Memory files. MEMORY.md and USER.md are discovered through the same
profile resolution Hermes itself uses (HERMES_HOME, then active_profile
redirecting to <root>/profiles/<name>). USER.md entries are additionally
mirrored into the native user_memory/profile/user.md: six host consumers
(Curator, Personalizer, four Sentinel producers) read that file directly and
never see EverOS-only content. An LLM picks each entry's H2 heading; every
failure path lands the entry under ## Notes, which injection always includes,
so a misclassification costs tokens rather than visibility.

Conversations. Sessions are enumerated by shelling out to
hermes sessions export --dry-run, whose listing is capped at 100 rows. The
scanner walks time windows until every session is covered rather than silently
importing the first 100. Only ended sessions are candidates, and
reopen_session() can clear ended_at, so the window reconciliation documents
the resulting race in both directions instead of assuming monotonicity.

Skills. Provenance is classified from on-disk evidence -- the
.bundled_manifest name-to-md5 map, .hub/lock.json, and .usage.json -- with
a package_hash byte-identical to Hermes' own _dir_hash, so a skill still
matching its factory content is left alone and only user-authored or
curator-managed skills are copied into the local skill pool. Skills never travel
as ScanResults, so the menus, the confirm line and the final summary all state
the skill count explicitly; without it a run that installs 12 of 82 reads as
having dropped 70.

Failure isolation: scan_all now reports a failing scanner through an
on_error callback and keeps the other platforms' results, and a Hermes run
whose conversations cannot be enumerated still returns its memory files.

Two pre-existing defects found while working here are fixed in the same branch:
a multi-line recall hit escaped its bullet in the context segment renderer, and
a recalled EverOS profile was rendered as a Python repr and left uncapped.

Also in this branch, from hands-on testing of the finished flow:

  • raven import run could never reach its interactive path. The selectors
    called questionary's synchronous ask() from inside asyncio.run(), and
    prompt_toolkit drives Application.run() through asyncio.run(), which
    raises inside a running loop. No test covered it: every case passed
    --platform and --tier, bypassing the selectors.
  • Import outcomes now render as one block after the progress display closes.
    Printing them from _build_and_run sent them to this module's Console while
    onboard's live progress belongs to its themed Console, and Rich's Live can
    only hold back writes on its own console, so they landed mid-bar and left the
    bar duplicated.
  • The import run selectors use the shared prompt style and glyphs instead of
    questionary's defaults; onboard's foreground import gains the phase reporter
    it never had, so the multi-second USER.md mirror is no longer silent behind a
    bar reading 100%.

The EverOS backend moves with it. Agent-track recall turned out to be dead:
agent-track HYBRID fuses agent_case and agent_skill through a rerank
cross-encoder, and with no rerank provider the server refuses the request
outright, which recall catches and turns into an empty list against a
file-only logger. A live 1.1.3 probe returns RuntimeError: owner_type='agent' with method='hybrid' requires a rerank provider. The adapter now reads
capabilities.rerank from /health and asks for the LLM lane only when the
cross-encoder is genuinely absent -- always sending it would make every
correctly configured user pay an LLM call per recall, never sending it leaves
the track dead for anyone who skipped the optional rerank role in onboard.

That capability is only readable from everos 1.2.1, so the pin moves 1.1.3 ->
1.2.1, skipping 1.2.0 (built from a branch missing the 1.1.4 fixes, including a
knowledge-upload path traversal). Memory endpoints move to /api/v2, canonical
since 1.2.0; /api/v1 still resolves to the same handlers but is documented as
a legacy alias. Only everos endpoints move -- the Skill Hub and OpenRouter
clients have their own v1 surfaces.

That upgrade also broke an assumption Raven was making. "Server running" and
"server can recall" used to be the same statement; 1.2.1 boots with [llm] alone
instead of aborting, so a misconfigured provider answers 200 and quietly serves
less. Raven decided a role was configured by reading model + api_key out of
everos.toml, which cannot see that. Capabilities are now read from /health at
all three points where it matters: the memory step (right after the server it
already starts), raven doctor (which had no EverOS check at all), and
backend.start() -- the only one of the three on the path every session takes,
which is where an expired key actually surfaces. It follows the precedent already
set there for native Windows and prints to stderr rather than a log nobody reads,
but does not drop to a no-op: writes still land, and everos cascade backfill
gives those rows their vectors once the provider is fixed.

The same reading makes the embedding role genuinely optional. It was not before:
recall sends no method, so it took the server default of HYBRID, which everos
refuses without an embedding provider -- skipping the role produced a memory that
stored everything and returned nothing. The adapter now asks for KEYWORD when no
embedding capability is reported, searching the same rows lexically instead of
semantically, and that also rules out the rerank fallback because KEYWORD's agent
path never reaches the cross-encoder. So the wizard stops requiring embedding and
states what each role buys before asking for it -- llm alone gets keyword recall,
embedding adds meaning, rerank sharpens ordering -- and skipping embedding is
reported in colour rather than dim, because skipping rerank costs ordering while
skipping embedding costs semantic recall entirely.

Fault and degradation are now distinguished rather than lumped together: only a
missing llm sets doctor's exit code, and raven doctor names what each
unconfigured optional role costs, per role, since "optional" alone does not help
anyone decide. The wizard also stops asking whether to enable memory at all --
everos is the only backend, so the question framed a choice that does not exist,
and both of its answers were described wrongly (declining was said to use "native
Markdown memory", which does not exist: raven/plugin/memory/ holds only everos,
and ContextAssembler.owns_compaction is a hardcoded True that stops the
consolidator from ever updating user.md). Backing out is still possible and now
states the real consequence in colour. The memory LLM field is pre-filled with
the user's own main model rather than a recommended id, because a recommendation
is only reachable if their key carries it.

Two rounds of review on this branch found defects worth naming, since each was
invisible for the same reason -- no test covered the path:

  • Skills were copied before the run's own Proceed? gate. The two early
    returns that install a skills-only import sit above it, so a machine whose only
    importable data is skills had directories copied before being asked anything.
    Both paths now name the count and ask; the wizard's Start? gate is further
    down still, so its two call sites ask as well. Every covering test passed
    --yes, which is what hid it.
  • A cancelled run still ran both post-phases. run_import returns normally
    once it sees the cancel file, so raven import stop halted the conversation
    loop and then started an LLM call per USER.md entry plus a full skill copy.
  • Org-mirror skills were imported past a gate Hermes applies itself. Hermes
    token-gates _org/: with no .active_org marker it prunes the whole mirror,
    and with one it descends only into the named org, so a stale mirror is content
    Hermes no longer loads. Discovery now mirrors that. Excluding _org outright
    would have been the opposite error -- it drops the active org's skills.
  • doctor and the wizard probed the default address. Both took
    DEFAULT_EVEROS_BASE_URL while the backend reads the configured base_url, so
    a user who moved everos off port 18791 was told it was not running -- by the
    checks added to make a degraded server visible.
  • An unbuilt multimodal role was reported by nobody. The section-to-capability
    mapping had an entry both consumers skipped.
  • The USER.md progress line reached 100% before the last call. Each report
    counted the call about to start; on a measured 9.4s run that left ~3s sitting at
    100%, the exact symptom the phase reporter was added to remove.
  • Registry-name collisions were only checked within one run, so a name an
    earlier import or the user's own skill already claimed produced the invisible
    skill that get(name) never returns.
  • "All platforms" plus "run in background" could not work. import run has no
    all-platforms flag, so the child reached its platform picker with DEVNULL on
    both streams and no terminal, after the wizard reported the import as started.
    It now runs in the foreground and says so.

The e2e tests this branch made dead are fixed too: identity moved to the host, so
config={"agent_id": ...} is ignored, and the agent-track test wrote under the
services default while querying the tag -- hits was empty, its per-hit
assertions vacuous, and an xfail absorbed it. The plugin manifest moves to
1.1.0, which the upgrade SOP in this same diff asks for.

Not in scope: Codex, KimiCode and OpenClaw remain placeholders; cold-start
import stays an EverOS-only path with no native fallback.

Type

  • Feature
  • Fix

Verification

uv run --all-extras pytest -q       5405 passed, 30 skipped, 13 deselected
uv run ruff check .                 All checks passed!
uv run ruff format --check .        873 files already formatted
make check-large-files              passed

Beyond the suite, the importer was run against a real Hermes install at
--tier full: memory files, conversations and skills all landed, EverOS
extracted agent cases from the imported conversations (quality_score=1.0 in
the server log), and a second run over the same data reported
Submitted: 0, Skipped: 4, confirming the checkpointing is idempotent.

The everos upgrade was verified against a live server holding real imported
data: on 1.1.3 the agent track returns 500 and /api/v2 returns 404; on 1.2.1
with this change the agent track returns an agent_case and the user track is
unchanged at three episodes. Re-verified after killing the server and cold
starting it, with the resolved binary printed to confirm which build answered.
raven doctor and the wizard both report llm and embedding available with
rerank absent against that server.

The suite was also run with HOME pointed at an empty directory, which is how the
memory-probe and import-step tests were caught reading the developer's own
configuration instead of their own fixtures. That pass also found the test suite
waiting out ensure_everos_server's 30s readiness timeout on every machine
without a running server -- always, on CI -- which cost one file 34 of its 35
seconds and is now stubbed.

The interactive flow was verified under a pty, comparing the two console
wirings directly, and each new assertion was mutation-tested: every behavioural
change on this branch had its fix reverted and the covering test confirmed red,
and only that test. Where a fix had a plausible wrong shape, that shape was
mutated too -- excluding _org wholesale rather than token-gating it, reporting
progress after the await instead of before, seeding the registry-name check from
the pool without ordering it after the target resolution. Two of those mutations
survived and were the more useful result: one showed a guard clause could not
fire (_target_for only ever returns a path that does not exist, so excluding it
from the pool lookup was dead code, now removed), the other showed the wizard's
own confirmation sits below the returns it was supposed to cover.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally

User-facing docs are unchanged: this adds a platform to an existing documented
command rather than a new surface. The developer docs move with the code --
docs/memory-plugin-architecture.md (the everos pin, and the upgrade SOP now
stating that a one-way migration belongs in the release notes) and
scripts/README.md (two deleted scripts).

Risk

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

The import writes to EverOS and to user_memory/profile/user.md, and copies
skill directories into the local skill pool. EverOS has no delete endpoint, so
an unwanted import is not undone by reverting this branch; the skill copies and
the profile edits are plain files and can be removed by hand. Import state
lives in ~/.raven/import_state.json and can be deleted to start over.

Behaviour changes for existing users: the onboard wizard's import step and
raven import run both look different (shared prompt style, skill counts,
one summary block). raven import run's interactive path changes from
"crashes immediately" to working. Rolling back restores the crash.

Three more, from the review round: a skills-only import now asks before copying
(one extra prompt, skippable with --yes), an all-platforms import chosen with
"run in background" runs in the foreground instead and says so, and skills under
a stale _org/ mirror are no longer imported -- Hermes does not load them
either, but anyone who had them imported by an earlier build keeps them, since
nothing here deletes.

Two consequences of the everos upgrade that operators need to know:

  • The index migration is one-way. everos 1.2.1 migrates the LanceDB schema
    on first start and prunes older manifest versions, so once a machine has run
    this, its memory index can no longer be read by an older everos. Reverting
    the branch does not undo it.
  • A missing provider no longer aborts startup. 1.2.1 boots with [llm]
    alone. A misconfigured embedding provider used to fail loudly; it now degrades
    to keyword search, and rows written in that state carry no vector until
    everos cascade backfill is run. everos itself only logs one warning line,
    which is why the wizard, raven doctor and backend.start() all surface it --
    the last of those runs every session, so an expired key is reported rather
    than discovered.
  • Behaviour change for users who skip embedding. Recall degrades to keyword
    matching instead of failing, and the wizard no longer stops them from getting
    there. That is the point of the change, but it means a user can now end up with
    a working memory that is measurably worse at recall than the previous
    all-or-nothing setup would have allowed.

Related Issues

N/A

Kendrick-Song and others added 30 commits July 30, 2026 11:27
Store side read user_id/agent_id from the plugin config slice while the
recall side read them from config.memory, so a user editing only one of
the two silently split store and recall onto different owner_ids and
every written memory became unrecallable. Identity is host policy, so it
now travels through ServiceLocator as required fields.

Also corrects the PluginContext docstring: config_schema is parsed but
never validated and its defaults are never applied.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…g_schema

Identity now comes from ServiceLocator. A leftover user_id/agent_id in the
plugin config slice is ignored, and warned about only when it differs from
the host value. start() rejects identities EverOS cannot accept, because they
become directory segments on its write path and would otherwise fail late
with a 422 buried in the server log.

The manifest config_schema block is removed: it was never validated, its
agent_id default disagreed with the code and contained a colon the write path
rejects, and it declared only one of the three keys the factory actually
reads. The surviving comment now states that rather than promising validation
that is no longer planned.

The default base URL is consolidated into _server.py, which owns the server
lifecycle and is imported by backend.py rather than the reverse. It had a
third copy as ensure_everos_server's default parameter, which onboard relies
on by calling it with no argument, so a port change would previously have made
onboard probe one port while the backend used another. config/update.py keeps
its own copy on purpose: config must not import from a plugin.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
_validate_identity reported memory.user_id, which is the Python attribute
rather than the userId key the user actually has in config.json, so the
message pointed at a string they could not find. _warn_stale_identity_keys in
the same file already used the camelCase key; the two now agree.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
include_profile defaults to false server-side, so the branch that flattens
data.profiles into Memory rows was dead and every extracted user profile was
unreachable. Profiles are a direct fetch: unranked, not counted against
top_k, at most one row, so the flag is always on for user-track recall.
Agent owners ignore it, so it is not sent for agent_id.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
_flatten_profile was written before anything exercised it. Once profiles
actually started coming back, its f"{k}: {v}" emitted 11414 characters of
Python repr into every turn's system prompt, because the two substantial
fields are lists of dicts.

Rendering now uses an allowlist: category or trait as the label, description
as the body, one bullet per item. Everything else in an item is dropped,
including evidence and basis, which narrate how a trait was inferred rather
than stating a fact about the user. Keys ending in _ms are machine fields and
are skipped. Scalars keep the old key: value form.

Measured against a live server the same payload renders as 5172 characters of
prose.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The rendering rule states a precedence but nothing asserted it, so an item
carrying both fields could have silently started using trait.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Second platform for cold-start import. Home resolution mirrors Hermes' own:
HERMES_HOME when set and non-blank, else the platform default. A missing home
means Hermes is not installed, which is not an error.

Only the resolved home is scanned. Hermes also supports named profiles under
<root>/profiles/<name>, each with its own memories/ and skills/, but importing
every profile would need a cross-profile collision policy no current use case
calls for.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Entries split on the exact newline-delimited section-sign sequence so an entry
whose own text contains that character is not torn in half, a trap Hermes' own
parser calls out.

USER.md holds what the assistant learned about the user, so its entries are
user turns. MEMORY.md holds the assistant's own notes written in its first
person -- one real entry reads "the user corrected me about the model name" --
so its entries are assistant turns. That file therefore needs a leading user
preamble, which is load-bearing rather than decorative: EverOS' user-track
extraction skips a memcell containing no user sender outright, so a run of
pure assistant turns would extract nothing.

Preamble language samples the whole file rather than is_cjk's 200-char
default, because these files are a short list of atomic facts where the first
entry's language does not represent the rest.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Six host consumers read user_memory/profile/user.md directly -- the Curator,
the Personalizer and four Sentinel producers -- and the Sentinel daily plan
takes that file as its entire input, so content that only reaches EverOS is
invisible to all of them.

One entry becomes one H2 section because injection keeps the top two whole
sections by lexical overlap plus Notes as a catchall, so a single large section
would be all-or-nothing and would occupy one of only two slots.

The model picks only the heading; entry text is written verbatim, and every
failure path -- a raised exception, a finish_reason of error, or no provider at
all -- lands the entry under Notes, which injection always includes. A
misclassification therefore costs tokens, never visibility.

Idempotency is keyed on the entry text rather than the heading, and an existing
section is appended to rather than replaced. Keying it on the heading lost
content: the seeded template already defines Preferences, Work Context, Topics
of Interest and Basic Information, four of the seven headings the prompt
offers, so on a fresh install -- the cold-start case this feature exists for --
an entry classified into any of them was skipped and landed nowhere.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The idempotency guard read "stripped and stripped in current", where the first
clause only existed to stop an empty string matching everything. A blank entry
therefore fell through to the classifier and created a section with an empty
body. Blank entries carry nothing to import, so they are dropped outright.

Also corrects the docstring: entry text is written unchanged apart from
surrounding whitespace, and the return value is one heading per entry written
rather than one per distinct section.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
build_scanners now returns the Hermes scanner alongside Claude Code, and the
run path mirrors Hermes USER.md into the native user_memory/profile/user.md
after the EverOS pass.

The mirror is additive, so a failure in it is reported without reversing an
import that already succeeded -- the comment claiming that property previously
sat above code that did not have it. loguru is redirected to a file during
run, so the warning also goes to the console where the user will see it.

Provider construction is fault-tolerant: make_lazy_provider runs a credential
check that raises when no LLM is configured, and cold-start import has to work
in that case, so the mirror falls back to provider=None and lands everything
under the Notes catchall.

The delimiter rule now lives in one place: split_memory_entries is public on
the scanner module and used by both the scanner and the CLI, instead of the
CLI reaching across for a private constant.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
A real run flooded the terminal with LiteLLM DEBUG records and raw response
JSON, defeating the point of redirecting loguru to a file so the run shows only
the progress bar.

Cause is ordering, not configuration. redirect_loguru_to_file already strips TTY
StreamHandlers, but litellm was imported lazily on the first heading call, well
after that, and reattached its own stderr handler. The records reached both the
file sink via propagation and the terminal via that handler.

The provider is now built eagerly so litellm is imported inside
_make_hermes_provider, and the handlers are stripped again immediately after. A
one-shot command gains nothing from deferring the import.

Also drops a noqa for BLE001, which this project's ruff select never enables.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Skills cannot travel the memory pipeline: a skill is a directory of SKILL.md
plus references, scripts, templates and assets, while an everos agent-skill row
carries only text, so store() would drop every attachment.

package_hash reproduces Hermes' own _dir_hash byte-for-byte, including the
try/except wrapping the whole loop rather than each file. Faithfulness is the
requirement, not robustness: Hermes compares its manifest md5 against a digest
computed this exact way, so any deviation would classify every skill as
user-modified. Cross-checked equal on all 82 skill directories of a real
install and all 70 md5 values in skills/.bundled_manifest; the golden-digest
test pins the format, since Hermes is not a dependency a test could import.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The suite could not tell abort-on-first-failure apart from skip-the-bad-file:
the only blocked file was alphabetically last, so both algorithms produced the
same digest. Nor was sorted() load-bearing -- this filesystem's raw rglob order
happened to match sorted order for the old fixtures. Both are exactly the
'more robust' refactors that would silently break parity with Hermes and
misclassify every skill as user-modified.

The golden fixture now holds a top-level file that sorts after a nested one, so
rglob order and sorted order differ, and a second golden pins the digest of a
package with one unreadable attachment. Both constants were cross-checked
against Hermes' own _dir_hash, and both mutations now fail.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The bundled manifest plus a whole-package MD5, then the hub lock, then the usage
file. Hashing only SKILL.md would call a skill whose attachments the user edited
pristine. A missing manifest proves nothing, so everything reads unknown and is
therefore imported; if the hash ever diverges from upstream the failure direction
is redundancy, never data loss.

Manifest keys are tried frontmatter-name first and directory name second, the
order Hermes writes them in. Two of the 82 skills on a real install carry a
frontmatter name that differs from their directory name, so a single-key lookup
would misread them, and the reverse order would compare one skill against a
same-named directory belonging to another.

Discovery mirrors Hermes' own exclusion set rather than skipping .hub alone.
Without .archive we would resurrect precisely the skills the curator retired,
and without the support-dir rule we would import the documentation copies of old
skills that archive workflows preserve under a live skill's references/.

Measured on a real install: 82 skills, 70 pristine and skipped, 12 imported.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Hermes' usage record field is created_by: agent, which reads like provenance
but upstream consumes as a curator-management opt-in: hermes curator adopt
stamps the same marker on a skill the user wrote by hand, and upstream's own
_is_curator_managed_record docstring says to prefer is_curator_managed at call
sites rather than propagate the misleading name. Keeping AGENT_CREATED would
have told a user that skills they authored were written by an agent as soon as
the import summary surfaced the label. It also missed the older agent_created
flag that upstream still accepts.

Also pins the one shape where the support-dir index guard changes the answer.
Both guards were previously mutation-proof: no test failed when either was
removed.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Only bundled_pristine is skipped: importing too much leaves unused entries in a
pool that injects at most two skills per turn by relevance and that the user can
delete wholesale, while importing too little is silent. The browsing category
level is dropped and a single hermes/ level interposed, because the registry
reads the first level below a layer root as the source label.

Two situations that look alike are kept apart. A name claimed earlier in the
same run is a flatten collision to disambiguate; a target that already exists on
disk is the user's, and is never overwritten nor duplicated under another name.
Depth is not fixed either -- a skill can sit at Hermes' skills root with no
category to borrow -- so that case falls back to a numeric suffix rather than an
invented category word in a directory the user browses.

The summary keeps factory content in its own field: a run that installs 12 of 82
must not read as though it dropped 70.

Verified on a real install: 12 of 82 skills landed, 224 KB, and the registry
resolves all 12 under source hermes.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
copytree fails partway through, and by then SKILL.md is usually already in
place. Two things followed from leaving that directory behind: the skill pool
served an entry whose attachments were missing, and every later run read the
leftover as already present and skipped it, so the one console warning from the
first run was the only notice the user would ever get. That is the silent
under-import the module's own docstring names as the failure to avoid.

Removing the partial restores the retry, since a failed entry is not submitted,
and restores the invariant that a directory in the pool is a whole skill.

The failure branch had no test at all: deleting the entire try/except left the
suite green. It now fails both when the cleanup is dropped and when the whole
guard is removed.

Also drops a test named for the removed root-prefix behaviour whose assertion
was already true before the fix it claimed to guard.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
hermes prints only the first 100 rows of its export dry run and appends a
count of the rest, so parsing that listing alone would have reported success
while dropping every session past the hundredth. Above the cap the started_at
axis is partitioned into windows and probed recursively until each window is
small enough to be listed in full.

Three constraints drive the shape and none are guessable from the output:
--dry-run needs an output path and at least one filter; --after and --before
filter started_at and silently truncate to the minute, so bounds must be
minute-aligned or adjacent windows overlap and import the same session twice;
and only ended sessions are ever candidates, which is why hermes sessions list
can show more than the export dry run does.

The partition floor is the epoch rather than a recent date because a session
older than the floor lands in no window at all, and the empty halves left of
real data cost one probe each. The window results are reconciled against the
count the root probe reported, since a partition that misses a range returns a
perfectly well-formed short list that nothing else would notice.

Verified against the real CLI: 2 sessions, one probe, no windowing.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Images become a marker rather than travelling: ImportMessage.content is a str
and the everos adapter reduces list content to its text parts anyway, so a
multi-megabyte inlined image would be carried the whole way only to be dropped,
after exceeding the importer's 30,000-character batch limit by orders of
magnitude and being posted as a body that size.

Hermes stores a text part two different ways. An input_text part keeps its text
under content rather than text, so reading only the text field discards real
user prose and, worse, counts it as an image. Both shapes are handled, only the
known image types are counted, and a part of an unrecognised shape gives up its
text instead of being labelled an image.

Neither real export sampled here has list content at all -- the image-heavy
session on that install had not ended, and only ended sessions are exportable --
so that path rests on the shapes hermes documents rather than on observed data.
active and compacted need no filtering: hermes' exporter already excludes
inactive rows, so rewound turns never reach us.

Also pins three properties on the session enumerator that no test held: the
listing cap boundary at exactly 100, the ceiling's buffer past the current
minute, and the role filter itself, which real session_meta rows never exercised
because their empty content dropped them first.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…roperties

The reconciliation docstring claimed ended_at never goes back, so a shortfall
had to mean a coverage bug rather than a race. That is wrong: reopen_session
clears ended_at when a session is resumed, and it is called from six places, so
a session genuinely leaves the candidate set mid-scan. The race does run both
ways, as the original wording said. The error came from reading a field name
instead of checking what writes it.

The image docstring likewise kept a size claim nothing in this tree can support;
it now cites only the batch limit, which is a constant here.

Five properties were asserted nowhere, each verified by mutation: the ceiling
buffer past the current minute (reachable only with a frozen clock, which is why
the previous test passed with the buffer removed), tool_call_id staying absent on
non-tool roles, tool_calls arriving as a tuple rather than the list hermes hands
over, sender_id's split, and a zero timestamp being dropped as EverOS requires.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Sessions are enumerated through list_exportable_sessions, which pages around
hermes' 100-row listing cap, and fetched one at a time by --session-id. Export
goes to stdout rather than a temp file: a real session is one JSON object on one
line with nothing human-readable mixed in, so there is no temp directory to
create, no partial file to clean up, and one seam serves both the dry run and
the export, since the output path is just another argument.

scan_all no longer lets one platform take the others down with it. The hermes
scanner has to shell out to enumerate conversations, and it raises when the
binary is off PATH -- which, through a bare gather, meant a user who had once
installed hermes could import nothing at all, not even their Claude Code
history. Failures are now isolated per scanner and handed to an optional
callback, because the logger is silenced during scan and file-only during run,
so without it a failed platform would look exactly like a platform with no data.

filter_by_tier needs no change: conversations carry SourceKind.CONVERSATION and
the default tier already keeps memory files only.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
scan() probes for conversations, which shells out, so a test that forgot to
supply a fake would spawn the developer's own hermes and pass or fail depending
on what sessions it happened to hold. Safety was a convention -- remember the
stub -- rather than a property. An autouse fixture now replaces the default
runner with one that names the mistake, and the single test that wants the real
runner takes the reference captured before the guard is installed.

Verified by adding a stub-less test: it fails with the diagnostic instead of
invoking the binary.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The _run helper used asyncio.get_event_loop().run_until_complete, which only
finds a loop when some earlier test in the session created one. The file
therefore passed in a full run and failed 22 of 22 on its own, and from Python
3.12 on get_event_loop no longer creates a loop at all, so it raises outright --
on a 3.14 interpreter these were 22 hard failures.

asyncio.run owns the loop it uses, which makes each call self-contained.

Unrelated to the hermes import work around it; separable if this branch should
stay single-topic.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Both files hard-failed on a checkout without the extras installed, which made
the result depend on the developer's environment rather than on the code.

test_sandbox_debug_server claimed in its own docstring to run without boxlite,
and it nearly does: every reference is a patch of boxlite.Boxlite, so the real
package is never called into. But mock.patch has to import its target and the
server probes availability with a bare import, so 22 tests failed instead of
exercising anything. A scoped stand-in carrying the two names raven actually
reaches, Boxlite and Options, makes the docstring true; skipping instead would
have cost 22 tests on the platform where they are cheapest to run. The two tests
that assert the not-installed path patch builtins.__import__ themselves.

test_channels_errors needs the real slack_sdk in exactly one of its six tests,
because the classifier isinstance-checks SlackApiError, so that case now uses
importorskip the way test_channels_slack already does. A module-level skip would
have taken the five unrelated cases with it.

Verified both ways with a meta-path blocker standing in for a fresh worktree:
extras absent goes from 47 failed to 4769 passed and 4 skipped, extras present
stays at 4804 passed, and each file passes on its own.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
A text part carrying its prose under "text" was thrown away. run_agent reads
that key for input_text exactly as it does for text, and the canonical
flattener tries "content" only second, so the mapping this branch shipped had
it backwards and a message whose only part was such a text part was dropped
whole. Both keys are now accepted. The non-text set also missed image, audio
and input_audio, so an Anthropic-native image message flattened to nothing and
disappeared rather than leaving a marker; it now mirrors upstream's set exactly,
and the marker says media rather than image because audio reaches it too.

Session rows are read structurally instead of by guessing the id's shape. Cron
ids are cron_<job>_<stamp> and ACP ids are bare uuids, so the old pattern
silently discarded every session either subsystem ever created. The header
count is now reconciled on the short path as well, which is the path nearly
every install takes and where an unrecognised row was previously invisible, and
more rows than the header counted is an error rather than an invented session.

A missing hermes binary no longer costs the memory files, which are read off
disk and never needed the CLI. The reason still reaches the user: a scanner can
report a deliberate partial result and scan_all forwards it down the same path
a total failure takes, since the logger is silenced during scan and file-only
during run. Onboarding was the one scan_all caller left without that callback,
so this branch had turned its loud failure into No importable data found.

Also pins the identity plumbing: swapping user_id and agent_id at the only
production wiring point left all 4804 tests green, which for a refactor whose
whole purpose is preventing that mismatch is the assertion that mattered most.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
resolve_hermes_home read the platform-default root while hermes itself points
HERMES_HOME at the sticky active profile when one is set. Since HERMES_HOME is
not in raven's environment, a user on a named profile would have had their
default profile imported instead of the data they actually use -- upstream warns
loudly about exactly that read rather than treating it as an equal fallback. The
Windows fallback also now matches upstream's AppData location.

The user.md mirror held the profile write lock across every classification call,
which is blocking and untimed, so a long run stalled every other writer of
user.md. The consolidator documents the opposite pattern for the same reason:
classification never depends on file content, so headings are picked first and
only the read-check-write of each section is serialized. Idempotency also moves
off a whole-file substring test, which dropped a short entry that happened to
appear inside unrelated text, and a genuine skip is now counted and returned
rather than logged to a file nobody reads.

Skill provenance reads .usage.json by the frontmatter name it is keyed on, and
DiscoveredSkill now carries the name the pool identifies a skill by. A
registry-name collision cannot be renamed away, because that name rides inside
the copied SKILL.md: the second skill would sit on disk and never be what
get(name) returns. It is skipped and counted instead, and the directory-rename
path is left to the case it can actually fix -- two skills whose declared names
differ, which is the shape two of the 82 skills on a real install have.

Deletes ImportMessage.sender_id and DiscoveredSkill.size, both written by every
producer and read by none; the everos adapter synthesises its own sender from
role plus the host identity.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…ct stale docs

The everos profile recalls as a single Memory whose score falls back to 1.0,
which places it above every similarity-scored episode, and everos grows it
monotonically with no server-side limit. Turning include_profile on therefore
handed one unranked blob an unbounded and growing share of every turn's prompt.
It is now capped client-side at roughly the combined budget of a full episode
batch, truncated on a line boundary with a visible marker. The flag's comment
said it costs nothing; that is true of the server and false of the prompt, and
now says which.

An install whose only importable data is skills was told there was nothing to
import. Skills are directories rather than message sources, so they never appear
as ScanResults, and both the empty-scan and empty-tier paths returned before the
skill phase could run. The scan test that covered the empty case had to stub
discover() as well -- it was reading the developer's own Hermes install, so its
result depended on the machine.

MemoryConfig's docstring and the plugin architecture doc still instructed users
to keep memory.userId in sync with a per-plugin copy, which is the duplication
this branch removed to stop a silent split between writes and reads. Both now
describe ctx.services as the single path. The config docstring also still
claimed a plugin's slice is validated against its manifest config_schema, which
was never true and is why that block was deleted.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…ults

Both everos scripts import _RealEverosAdapter and read _mode, neither of which
survived the move to an HTTP-only backend, so they raise on import. That matters
because the plugin architecture doc prescribed the roundtrip script as the
EverOS-upgrade smoke step and the scripts README claimed it verified the
identity wiring end-to-end; a maintainer following either would have hit a
TypeError and had no smoke test at all.

The USER.md mirror printed nothing on success while the skill phase printed its
own line, so a mirror that worked looked like one that had not run. It now says
what landed and what was already there.

A conversation carries no cost hermes will state up front -- the listing gives an
id and a source -- so the table showed 0 files at 0 B, which reads as an empty
conversation rather than an unmeasured one. Unmeasured now renders as a dash.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The everos user profile recalls as prose, so a hit is not always one line, and
the renderer emitted it as a single dash followed by unindented continuations --
which read as body text that escaped the list rather than as part of that entry.

Also corrects the note beside the new profile ceiling. It described the score
falling back to 1.0 as though it were a second exposure; it sorts the profile
above every episode, but the profile is a direct fetch that never counts against
top_k and the caller renders every hit, so nothing is displaced by it being
first. Length was the whole problem, and length is what is capped.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Kendrick-Song and others added 2 commits August 4, 2026 11:03
The placeholders rendered in the same colour as the platforms that actually have
data, so a menu of two real choices read as a menu of six. They were selectable
too: picking one printed "not yet supported" and redrew the step, a round trip
that told the user what the label already said.

They now pass `disabled`, which greys the row through RAVEN_STYLE's `disabled`
class -- defined since the theme landed and never once used -- and makes the
arrow keys skip it, so the branch handling a pick of one is gone.

`disabled=True` rather than a reason string: questionary appends the reason in
its own hardcoded " (...)", and the label already carries the full-width pair the
rest of the Chinese copy uses. The "- " prefix it puts on a disabled row is
hardcoded in both of its branches and stays.

Sorted so the pickable platforms come first. In enum order the two kinds
interleave, which left Hermes sandwiched between two placeholders.

The scripted-questionary fake in the tests kept dropping `**kwargs`, so
`disabled=True` could be deleted with the suite still green; it records the flag
now, and the ordering, the flag and the full-width label each have a test that a
mutation confirmed fails without them.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Recording `disabled` in the previous commit fixed one instance of a general
defect rather than the defect. The fake still dropped every other keyword a
`Choice` is given, and dropped all of `select`'s -- so no test could observe
`style` or `qmark`, and any prompt in the wizard could lose `style=RAVEN_STYLE`,
fall back to questionary's own colours and marker mid-flow, and leave the suite
green. `test_cli_import_commands` already records them; this file did not.

Both now keep everything they are handed. That makes the assertion possible, so
it is written: every prompt the import step raises carries the shared style and
marker. Mutation-checked in both directions -- dropping the chrome from a prompt
fails it, and so does the fake going back to discarding keywords.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@Kendrick-Song
Kendrick-Song requested a review from 0xKT August 4, 2026 03:15
@Kendrick-Song Kendrick-Song self-assigned this Aug 4, 2026
Kendrick-Song and others added 5 commits August 4, 2026 13:59
The wizard's two early returns -- no scan result at all, and none surviving
the tier filter -- printed "nothing to import" and stopped there. Skills are
directories rather than message sources, so they never arrive as ScanResults
and neither return had anything of theirs to act on: an install whose only
importable data is skills was told there was nothing to import. `raven import
run` already covered both paths, and the wizard is the one every new user
takes.

Both entry points now share a single skill counter as well. The wizard kept
its own copy and called it inside the platform loop, so returning to that
prompt re-walked the whole Hermes skill tree; the shared counter runs once,
outside the loop.

Also corrects two docstrings claiming `_memory_enabled` gates on llm and
embedding. It has gated on llm alone since embedding became optional, and a
maintainer reading either docstring would conclude the opposite.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
`start()` called the capability probe inline. The probe is synchronous httpx
with a 5s timeout and the config read behind it blocks too, so a wedged server
stalled the loop every session begins on for the whole timeout. The sibling
`_probe_health` in the same package already goes through `asyncio.to_thread`.

The bool-only parse of `/health` also existed twice -- here, and in the adapter
that picks its search lane from the same map -- along with two definitions of
the timeout. Parsed two ways, `raven doctor` and recall could disagree about
the same server, so both now call `parse_capabilities`.

Records why `_flatten_profile` drops `*_ms` keys while here: they are the
store's bookkeeping about when it learned something, not a fact about the user.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
`partial_failure` is mutable instance state that `scan()` set but never reset,
and `scan_all(scanners=...)` accepts instances a caller may scan more than
once. The first run's failure then travelled with the second run's results,
reporting a complete scan as one that came up short.

Drops two `except HermesExportError: raise` clauses on the way past. The clause
below each catches OSError, which cannot match a RuntimeError subclass, so
neither branch changed any behaviour.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Cold-start import writes into the same `user.md` the consolidator owns, and
reached its parser as `_parse_user_md_sections`, aliasing the underscore away
at the import site. Renaming it drops that fiction: parsing the file a second
way is how the two sides would come to disagree about what a section is.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
everos 1.2.1 migrates the LanceDB schema and prunes older manifest versions on
first start, and reverting the pin undoes neither. That was recorded only in a
pull-request description, which is known to whoever read it. The upgrade SOP
now says to take a copy of ~/.everos/ beforehand, and to put a one-way
migration in the release notes.

Corrects two stale versions in the same section: the heading pinned 1.0.0, and
the bump example installed 1.2.0 -- the one release that must be skipped, for a
path-traversal regression fixed in 1.2.1.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@gloryfromca

Copy link
Copy Markdown
Contributor

Adversarial review. I read the full diff and pulled every suspicion back onto the
branch source to check it. I did not run the suite, so 5370 passed is taken
on trust; everything below comes from reading the code.

Line numbers are on feat/hermes_cold_start_import.

The engineering is unusually careful -- _uncovered's arithmetic reconciliation,
parse_dry_run_listing refusing to read "cannot parse" as "no sessions", the
rmtree after a partial copytree. Three findings below are real defects, two of
which write to disk and spend tokens without the user having agreed to either.

Confirmed defects

1. Skills are installed before the Proceed? confirmation

raven/cli/import_commands.py, _run_async has two early returns:

  • L537 if not all_results: -> _install_skills_without_a_scan(platform_filter)
  • L572 if not filtered: -> the same

The confirmation gate if not yes: typer.confirm("Proceed?") is at L587 and
neither path reaches it. On a machine whose only importable data is Hermes skills,
raven import run has already shutil.copytree'd dozens of skill directories into
workspace/skills/hermes/ and written import state before the user is asked
anything.

The Risk section says the skill copies "can be removed by hand" -- being
irreversible is the reason this step needs the confirmation more, not less.

Both covering tests (test_run_installs_skills_when_the_scan_finds_nothing,
test_run_installs_skills_when_the_tier_keeps_nothing) pass --yes, so the gap
is invisible to them. That is the same blind spot this PR describes fixing for the
selectors ("No test covered it: every case passed --platform and --tier").

2. A cancelled import still runs both post-phases

_build_and_run, L128-142:

summary = await run_import(..., cancel_path=cancel_path)   # may be cancelled=True
profile = await _land_hermes_user_md(...)   # one LLM call per USER.md entry
skills  = await _install_hermes_skills(...) # full skill tree copy

run_import returns normally with cancelled=True once it sees the cancel file,
and both phases then run unconditionally. So raven import stop halts the
conversation loop and immediately starts an unbounded LLM classification run plus a
complete skill install. Both need an if not summary.cancelled guard.

3. doctor and the wizard may probe a different server than the one in use

  • raven/cli/doctor_commands.py L204 -> probe_capabilities()
  • onboard_commands._report_everos_capabilities -> probe_capabilities()

Both take the DEFAULT_EVEROS_BASE_URL default, while backend.start() correctly
passes self._config.get("base_url"). A user who moved everos off port 18791 gets
Server: not running from doctor and silence from the wizard -- the two surfaces
this PR added specifically so a degraded server is visible. Read
plugins.config["everos-memory"].base_url and pass it through.

Worth fixing, not urgent

4. The progress bar reaches 100% before the work is done.
raven/importer/hermes_user_md.py L111-112:

on_progress(index, len(kept))                 # reported
headings.append(await _pick_heading(...))     # then done

index starts at 1, so with three entries the bar shows 3/3 before the third LLM
call is even issued. On the measured 9.4s/3-entry run that is ~3s sitting at 100% --
exactly the symptom _make_phase_reporter was added to remove. Report after the
await, or report index - 1. tests/test_importer_hermes_user_md.py has no
assertion on on_progress at all.

5. Registry-name collision detection only covers the current run.
raven/importer/skills/installer.py L61/68/104: claimed_registry_names starts
empty every run and is only populated after a successful install. A skill whose
frontmatter name collides with one installed by a previous run, or with the
user's own skill in the pool, passes the guard, lands under a non-colliding
directory name, and becomes precisely the skill get(name) never returns -- the
loss the guard exists to prevent. Seeding the set from the existing pool closes it.

6. The _validate_identity ValueError lands where nobody reads it.
The path-traversal guard is right, but every backend.start() call site wraps it in
except Exception: logger.exception(...) (agent_commands.py L403/L501,
tui_commands.py L657, gateway_commands.py L370), and loguru is file-only under
the TUI and the agent. An invalid memory.userId therefore turns memory off
silently. That contradicts the argument this PR makes for
_warn_if_recall_cannot_work ("prints to stderr rather than a log nobody reads") --
same class of fault, two different visibility decisions. Either validate at config
load, or print it the same way.

Questions and observations

7. The window ceiling is naive local time

In scanners/hermes.py, _PARTITION_FLOOR = datetime(1970, 1, 1) and
_ceil_to_minute(datetime.now()) are both naive and formatted straight into
--after / --before.

If hermes parses those bounds as UTC and the user sits at a negative UTC offset, the
ceiling falls behind real "now" and the most recent hours of sessions drop out of
every window. _uncovered then reports them as unaccounted for, which reads like
the documented reopen_session race rather than a bug.

Omitting --before on the rightmost window removes the whole class. Has the hermes
side's parsing been confirmed?

8. Cost of the session enumeration

Simulating the partition (the empty halves left of real data do return on their
first probe, so this is better than it first looks):

sessions span hermes subprocess invocations
150 180d 18
500 180d 28
2000 365d 78

Each is a cold Python CLI start, and _collect_window is sequential
(await left then await right). raven import scan and raven import run each
pay a full enumeration. asyncio.gather on the two halves is a cheap improvement if
hermes' SQLite reads tolerate the concurrency.

9. One /health, three readers

_server._probe_health, _health.probe_capabilities (sync httpx), and
_HttpEverosAdapter._probe_capabilities (async, on the client that already exists).
start() has ensure_everos_server prove reachability, then probes again
off-thread, and the adapter probes a third time on first recall.

Neither of the latter two sends the Authorization header the adapter uses
elsewhere -- if /health is ever auth-gated, both read as "no capabilities".

10. Menu padding implemented twice with different width functions

onboard_commands._tier_choice_label uses the CJK-aware _cell_len;
import_commands._pick_tier / _platform_choice_label reimplement the same padding
rule with bare len. Both sides are ASCII today so nothing is visibly wrong, but it
is one layout rule with two implementations.

11. `partial_failure` is an implicit protocol

scanners/__init__.py reads it via getattr(scanner, "partial_failure", None).
Putting it on the Scanner protocol with a None default makes it type-checkable
and makes "I can fail partially" an explicit part of the scanner contract.

What is good here

  • _uncovered reconciling window coverage by arithmetic. A partition that misses a
    range returns a perfectly well-formed short list; counting is the only thing that
    can catch it, and that judgement is correct.
  • parse_dry_run_listing treating an unrecognised header as an error rather than a
    zero. A silent 0 would read as success.
  • The rmtree after a failed copytree, restoring both the retry and the invariant
    that a directory in the pool is a complete skill.
  • _flatten_profile using an allowlist rather than a denylist for evidence /
    basis.
  • The multi-line recall bullet fix in render.py, and dropping
    ImportMessage.sender_id -- grepping the whole repo confirms it was dead.

On the size

57 files / +6597 carries at least five separable changes: the Hermes importer, the
everos 1.1.3 -> 1.2.1 upgrade, the wizard redesign, two incidental pre-existing bug
fixes, and the identity single-source refactor. Nothing in AGENTS.md forbids that,
but the identity refactor (two non-defaulted fields on ServiceLocator, 20+ test
construction sites touched) and the everos upgrade each deserve their own review
lens -- and the latter carries the one-way index migration this PR itself flags. If
anything can still be split out, the everos upgrade plus the identity refactor is
the seam I would cut on.

@0xKT

0xKT commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Second review pass, complementary to the earlier comment rather than a repeat of it.
That comment's items are referenced below as G1..G11 (its own numbering 1..11); mine are
R1..R8. Line numbers are on feat/hermes_cold_start_import at 47dfd5d. Every item below
carries a concrete patch; the snippets are illustrative, not literally applied.

Rebase note first: the branch now conflicts with main. #266 landed after this PR's base
(c89460f) and touches the same two files -- raven/cli/onboard_commands.py auto-merges,
tests/test_cli_onboard_commands.py conflicts. Two of #266's tests then apply to this
branch: the contract test that every credential prompt raises on Ctrl+C, and the AST guard
test_no_call_site_translates_a_cancelled_address_prompt. That guard only resolves
_prompt_local_api_base call sites, and the selects this PR adds (the give-up menu in
_config_everos_role, the platform / tier / mode prompts in the import step) are in the
chain #266 deliberately left alone, so nothing here contradicts it -- but the suite is
worth re-running on the rebased tree rather than on this one.

Scope note: I re-verified every upstream claim this branch makes against the Hermes
source and against both everos wheels (1.1.3 and the 1.2.1 this PR pins). All of them
hold -- the section-sign delimiter, the ended_at IS NOT NULL candidate filter, the
half-open --after/--before semantics, the candidates[:100] cap and row format, the
positional - for stdout, package_hash being byte-identical to _dir_hash, /api/v2
plus the /api/v1 alias, the /health capability key names, extra="forbid" on
SearchRequest, and the owner_type='agent' rerank requirement. No need to re-check any
of those. What follows is what did not hold.

Before merge

R1 -- org-shared skills are imported past a gate Hermes itself applies

raven/importer/skills/hermes.py:42-59

_EXCLUDED_DIRS mirrors upstream EXCLUDED_SKILL_DIRS, but upstream gates the org mirror
separately from that set. agent/skill_utils.py:62-63 defines
ORG_MIRROR_DIR_NAME = "_org" / ORG_ACTIVE_MARKER = ".active_org", and the walk at
agent/skill_utils.py:877-890 prunes _org entirely when no marker exists and otherwise
descends only into the marked org -- its docstring: "leave an org and its skills stop
resolving, without any manual cleanup". Upstream tests the stale case
(tests/agent/test_org_skill_namespace.py, org-OLD/stale-y).

Here discover()'s root.rglob("SKILL.md") (L85) has no _org handling in
_is_discoverable (L101-110), nothing in _classify (L113-136) stops the LOCAL_UNKNOWN
verdict, and LOCAL_UNKNOWN is copied (installer.py:32,55). So every org mirror on disk
-- including orgs whose marker is gone, which Hermes will not load -- is copied into
workspace/skills/hermes/, and there is no delete path for an unwanted import.

patch:

# raven/importer/skills/hermes.py
_ORG_MIRROR_DIR = "_org"
_ORG_ACTIVE_MARKER = ".active_org"


def _read_active_org(root: Path) -> str | None:
    """Mirrors Hermes' ``read_active_org_id`` (agent/skill_utils.py).

    No marker means no org mirror resolves at all; that is the gate, not a
    default. A blank marker counts as absent.
    """
    try:
        return (root / _ORG_MIRROR_DIR / _ORG_ACTIVE_MARKER).read_text(encoding="utf-8").strip() or None
    except OSError:
        return None


# in discover(), before the loop:
        active_org = _read_active_org(root)
        for skill_md in sorted(root.rglob("SKILL.md")):
            if not _is_discoverable(skill_md, root, active_org):
                continue

# in _is_discoverable(skill_md, root, active_org), first check:
    if parts and parts[0] == _ORG_MIRROR_DIR:
        # Token-gated upstream: only the marked org's mirror resolves, so a
        # stale mirror is content Hermes itself no longer loads.
        if active_org is None or len(parts) < 2 or parts[1] != active_org:
            return False

Do NOT just add _org to _EXCLUDED_DIRS: that also drops the active org's skills,
which Hermes does load, and that is the "import too little" direction installer.py:4-7
argues is the costly one.

verify: skills/_org/org-OLD/stale/SKILL.md with no marker -> not discovered; then
.active_org containing org-1 plus _org/org-1/x/SKILL.md and _org/org-2/y/SKILL.md
-> only x discovered.

R2 -- the agent-track e2e test can no longer fail

tests/integration/test_everos_backend_e2e.py:32-40

This PR moved identity to the host (backend.py:308-310 now reads
self._services.agent_id), but the test still passes it the old way,
config={"agent_id": agent_id}, and hardcodes ServiceLocator(..., agent_id="default").
The config key is ignored and additionally trips this PR's own stale-key warning
(backend.py:346-359).

Consequence: assistant/tool rows are stamped from services (backend.py:704), everos
derives the agent-track owner from that sender_id (everos 1.2.1 memory/models.py:280-282),
so writes land under "default" while be.recall(..., agent_id=ids.agent_id) (L115)
queries a-<tag>. hits is empty, the per-hit assertions are vacuous, and the
pytest.xfail at L127 absorbs it. The docstring claims to prove user_id/agent_id
routing; today it proves nothing and cannot go red.

patch:

-def _backend(tmp_path: Path, *, agent_id: str) -> EverosBackend:
+def _backend(tmp_path: Path, *, user_id: str, agent_id: str) -> EverosBackend:
     be = EverosBackend(
         PluginContext(
-            config={"agent_id": agent_id},
-            services=ServiceLocator(workspace=tmp_path, user_id="default", agent_id="default"),
+            config={},
+            services=ServiceLocator(workspace=tmp_path, user_id=user_id, agent_id=agent_id),
         )
     )

-    be = _backend(tmp_path, agent_id=ids.agent_id)
+    be = _backend(tmp_path, user_id=ids.user_id, agent_id=ids.agent_id)

Three call sites in this file (L74, L106, L146). _stamp_user can stay as-is -- with the
services user_id set correctly the fallback would do the same thing, but the explicit
stamp still documents the owner rule.

tests/integration/test_everos_skill_evolution_e2e.py:58-63 carries the same dead config
key and needs the same two-line change (there the store goes through
everos.service.memorize directly, so its recall already matched -- the argument was
merely inert).

verify: run with a real everos and confirm the skill assertions execute instead of
xfailing, and that the stale-key warning is gone from the log.

R3 -- plugin manifest version not bumped, against this PR's own SOP

docs/memory-plugin-architecture.md:351, inside the upgrade procedure this PR edits,
says: "Finalize: bump the manifest version". The everos pin moved 1.1.3 -> 1.2.1, the
API prefix moved v1 -> v2, and the manifest itself changed, but the version is still
1.0.0. It is pinned in four places, all of which move together:

raven/plugin/memory/everos/raven-plugin.toml:3   version            = "1.0.0"
raven/plugin/memory/everos/__init__.py:14        __version__ = "1.0.0"
tests/test_everos_plugin_discovery.py:51         assert ... .__version__ == "1.0.0"
tests/test_everos_plugin_discovery.py:112        assert record.manifest.version == "1.0.0"
tests/test_plugin_command.py:60                  assert "1.0.0" in result.stdout

1.1.0 fits what changed (identity source moved, config_schema removed, v2 prefix) --
__init__.__version__ and the manifest must not drift apart.

Description -- must land before the squash, since the PR body becomes the commit body

R4 -- two factual errors, plus two metadata items

  • Summary says the memory files discovered are HERMES.md and USER.md. The code reads
    USER.md and MEMORY.md (raven/importer/scanners/hermes.py:31), matching upstream
    (tools/memory_tool.py:223-224). There is no HERMES.md.
    Fix: "MEMORY.md and USER.md are discovered through the same profile resolution ...".
  • Verification ends with "Docs are unchanged", but the diff touches
    docs/memory-plugin-architecture.md and scripts/README.md. The unchecked
    docs/screenshots box is right; the sentence is not.
    Fix: "User-facing docs are unchanged; the developer docs (memory-plugin-architecture.md,
    scripts/README.md) move with the code."
  • Type has both Feature and Fix checked. AGENTS.md 3.7 asks for one box mirroring the
    commit type, which is feat. Uncheck Fix.
  • Title scope is (importer), but the diff spans raven/importer,
    raven/plugin/memory/everos, raven/cli, raven/config, raven/context_engine,
    raven/memory_engine. AGENTS.md 3.1: multiple scopes -> omit the scope or use (*).
    feat(*): add hermes as a cold-start import source is 58 chars, so it still clears the
    header limit with the (#264) the squash appends. Re-run the title check afterwards.

Follow-up

R5 -- a configured-but-unbuilt multimodal role is reported by nobody

raven/plugin/memory/everos/_health.py:33 maps multimodal -> multimodal_llm, but
DEGRADING_SECTIONS (L43) omits it, and both consumers iterate only
(*REQUIRED_SECTIONS, *DEGRADING_SECTIONS) (doctor_commands.py:203,243,
onboard_commands.py:4116). multimodal is real config surface --
raven/config/update_everos.py:42 lists it writable and the wizard writes it
(onboard_commands.py:3247,3295,3878) -- so the mapping entry is dead and the role can
fail to build with nobody saying so.

patch:

# raven/plugin/memory/everos/_health.py
-DEGRADING_SECTIONS = ("embedding", "rerank")
+DEGRADING_SECTIONS = ("embedding", "rerank", "multimodal")

# raven/cli/doctor_commands.py, _degradation_note()
     return {
         "embedding": "  (recall matches keywords, not meaning)",
         "rerank": "  (agent-track recall uses the LLM lane instead of a cross-encoder)",
+        "multimodal": "  (images / PDFs / audio stay out of memory)",
     }.get(section, "")

The warning text needs adjusting with it: doctor_commands.py:261-263 says "so recall
runs degraded", which is wrong for multimodal -- what degrades there is
understand_media and multimodal ingest. "so memory runs degraded" covers all three, or
split the sentence per role.

R6 -- "All platforms" plus "Run in background" starts an import that cannot run

raven/cli/onboard_commands.py:4652-4663 builds raven import run --tier <t> --yes and
adds --platform only when a single platform was chosen, then spawns it with
stdout/stderr on DEVNULL and start_new_session=True. The child reaches
import_commands.py:546-555, which calls _pick_platform (questionary, L683) whenever
platform is None and more than one platform has results: an interactive prompt in a
session with no controlling terminal and no visible output. The wizard prints "Import
started in background" and nothing is imported.

Not a regression -- main is equally broken here (there _pick_platform used a sync
ask() inside asyncio.run and crashed) -- but the description says the interactive path
went from "crashes immediately" to working, and this is the one place it still cannot. The
root cause is that import run has no way to say "all platforms" non-interactively.

patch (smallest correct option -- fall through to the foreground, which already works):

         else:
             platform_flag = selected_platform if selected_platform != "all" else None
+            if platform_flag is None and len(by_platform) > 1:
+                # The child would have to ask which platform to use, and a
+                # detached process has no terminal to ask on.
+                console.print(_t(
+                    "  [yellow]Background mode cannot run an all-platforms import; "
+                    "running in the foreground instead.[/yellow]",
+                    "  [yellow]<zh>[/yellow]",
+                ))
+                exec_mode = "foreground"
+            else:
                 cmd = [raven_bin, "import", "run", "--tier", selected_tier.value, "--yes"]
                 ...

The alternative is a real all-platforms flag (plus making _run_async skip the picker for
it) or one child per platform; both are larger.

verify: a test asserting the spawned argv needs no TTY -- the Popen branch has no coverage
at all today.

R7 -- residual on the window bounds (this closes G7)

G7 asked whether hermes parses the naive bounds as UTC. It does not:
hermes_cli/session_filters.py:63-71 does datetime.fromisoformat(s) and returns
dt.timestamp() when tzinfo is None, docstring "naive = local time". That matches this
branch's naive datetime.now() (scanners/hermes.py:573) and naive _PARTITION_FLOOR
(L405), so no window is lost to an offset. G7 can be closed.

One narrower residual survives: --after "1970-01-01 00:00" reaches dt.timestamp()
inside hermes, and at a positive UTC offset that is a pre-epoch local time, which raises
OSError on Windows. It needs Windows plus more than 100 sessions plus a positive offset,
and it fails loudly (hermes exits non-zero -> HermesExportError -> partial_failure,
scanners/hermes.py:145-152), so it is not a silent-loss class.

patch:

-_PARTITION_FLOOR = datetime(1970, 1, 1)
+# 1970-01-02, not 1970-01-01: the bound is formatted as naive local time and
+# hermes parses it the same way, so an epoch-day floor is a pre-epoch local
+# timestamp east of UTC, which raises OSError on Windows.
+_PARTITION_FLOOR = datetime(1970, 1, 2)

Omitting --after on the leftmost window also works but is not a deletion:
_collect_window:521 needs a concrete start to compute midpoints, so the floor has to
stay for the arithmetic while None is passed as the bound.

Nit

R8 -- log path hardcoded right next to the helper introduced to prevent that

raven/cli/import_commands.py:124 prints a literal ~/.raven/logs/everos-server.log.
This PR added server_log_path() precisely so that name cannot drift, and uses it
everywhere else (doctor_commands.py:283, backend.py:433). The line is pre-existing and
currently accurate.

-        console.print("[dim]Check the server log: ~/.raven/logs/everos-server.log[/dim]")
+        from raven.plugin.memory.everos._server import server_log_path
+
+        console.print(f"[dim]Check the server log: {server_log_path()}[/dim]")

Corrections to the earlier comment

G1's fix belongs at the call sites, and wants the count. Do not prompt inside
_install_skills_without_a_scan: onboard_commands.py:4546 also calls it and already has
its own "Start?" flow, so it would double-prompt the wizard. Hoisting discovery gives the
user a number to consent to, and returning True on a decline keeps the caller from also
printing "No importable data found":

-async def _install_skills_without_a_scan(platform_filter: Platform | None) -> bool:
+async def _install_skills_without_a_scan(platform_filter: Platform | None, *, assume_yes: bool) -> bool:
     if platform_filter not in (None, Platform.HERMES):
         return False
+    count = await _importable_skill_count(platform_filter)
+    if not count:
+        return False
+    console.print(f"\nAbout to import {count} Hermes skills.")
+    if not assume_yes and not typer.confirm("Proceed?", default=True):
+        return True  # handled: the user declined, nothing left to report
     summary = await install_skills(HermesSkillSource(), load_config().workspace_path, _default_state())
-    if summary.total == 0:
-        return False

Callers: assume_yes=yes at import_commands.py:537,572; assume_yes=True at the two
onboard sites, which have already confirmed. A test that does not pass --yes is the part
that was missing.

G4's fix, both variants incomplete. Reporting after the await leaves the first call
with no progress at all; reporting index - 1 never reaches N/N. Report completed count
before each call, then close the bar:

-    for index, (entry, _) in enumerate(kept, start=1):
+    for index, (entry, _) in enumerate(kept):
         if on_progress is not None:
             on_progress(index, len(kept))
         headings.append(await _pick_heading(entry, provider=provider, model=model))
+    if on_progress is not None and kept:
+        on_progress(len(kept), len(kept))

G5's fix has a trap. Seeding claimed_registry_names from the existing pool makes an
idempotent re-run trip the duplicate guard on a skill's own name, ahead of the
plain.exists() "already present" path at installer.py:138, which misclassifies and
mis-counts it. Resolve the target first (that check already handles "already on disk"),
then test the name against the pool minus that target:

-        if skill.registry_name in claimed_registry_names:
-            ...skip...
         target = _target_for(skill, dest_root, claimed)
         if target is None:
             skipped += 1
             continue
+        if skill.registry_name in claimed_registry_names | _pool_registry_names(dest_root, exclude=target):
+            ...skip...

where _pool_registry_names reads the frontmatter name of each */SKILL.md already under
dest_root. The underlying defect is real.

G9 is real but hygiene only. everos ships no built-in auth (settings.py:91-92, the
shipped default.toml:21-23; no auth middleware under entrypoints/api), so /health
cannot be auth-gated today except behind a user-supplied proxy. Worth
headers=self._headers() on the adapter probe (backend.py:192); the module-level
probe_capabilities has no key to send without new plumbing, so leave it.

G10 and G11 can be dropped. The import_commands padding sites only ever pad ASCII
labels (import_commands.py:709, display names at L43-49), so bare len equals the cell
width there and unifying on _cell_len would import rich.Text for nothing.
getattr(scanner, "partial_failure", None) is a deliberate optional-capability idiom --
only HermesScanner has the attribute (scanners/hermes.py:130) -- and putting it on the
Protocol forces it onto ClaudeCodeScanner.

The sender_id observation is right, confirmed. origin/main's
orchestrator._to_store_dict never forwarded sender_id into the dict handed to
backend.store(), so the field was written by scanners and read by nobody. Removing it is
behaviour-neutral with no migration consequence -- worth stating so nobody goes looking
for one.

One blind spot to record rather than fix

REQUIRED_SECTIONS = ("llm",) (_health.py:38) plus MemoryInfo.broken and the exit-2
branch (doctor_commands.py:83-93,127) are unreachable against everos 1.2.1: its
/health hardcodes llm=True (entrypoints/api/routes/health.py:61) because the server
cannot boot without an llm (lifespans/llm.py:23). So broken is always empty, and a
genuinely broken llm surfaces as "Server: not running (starts on demand)"
(doctor_commands.py:231) with exit 0 -- the benign wording for the worst case.

I would not change code for this: telling "not started yet" from "crashes at boot" needs
either starting the server or reading its log, both outside doctor's zero-network design,
and everos' own comment says that hardcoded literal may become a real probe later. A
comment at the constant would stop the next reader taking the branch as live.

Suggested order

  1. R2 -- this PR created the dead test; four lines, and while it sits the xfail reads as
    intended behaviour.
  2. R1 -- data correctness of the new feature, and an import users cannot undo.
  3. G2 then G1 -- both are broken promises of the same command, both small.
  4. R4 and R3 -- description and manifest, needed before the squash.
  5. G5, G3, R5, R6, R7, G6, G8, G10-cost -- follow-ups, roughly in that order of user
    impact.
  6. G4, R8, G9 -- batch or skip.

Kendrick-Song and others added 9 commits August 4, 2026 16:49
…t_import

Conflict in tests/test_cli_onboard_commands.py: both sides appended tests at the
end of the file, so both are kept -- #266's Ctrl+C contract test and its AST
guard, plus this branch's role-cost and import-step tests.

The AST guard resolves `_prompt_local_api_base` call sites only, and the selects
this branch adds are in the chain #266 left alone, so it passes unchanged.
Four paths of one command that no test reached.

The two early returns that install a skills-only import sit above the run's own
`Proceed?` gate, so a machine whose only importable data is skills had directories
copied into its skill pool before being asked anything -- and copying is the part
nothing undoes. Both now name the count and ask. The wizard's `Start?` gate turned
out to sit below its own two call sites, so those ask as well rather than
inheriting a confirmation that comes later. Every covering test passed `--yes`,
which is exactly what hid the missing gate.

`run_import` returns normally once it sees the cancel file, so nothing downstream
noticed: `raven import stop` halted the conversation loop and then started an LLM
call per USER.md entry plus a full skill-tree copy. Both post-phases are now
guarded on `summary.cancelled`.

"All platforms" plus "run in background" could not work. `import run` has no
all-platforms flag, so with more than one platform holding data the child reached
its platform picker -- on DEVNULL, in a session with no controlling terminal,
after the wizard had already reported the import as started. It now falls back to
the foreground and says why.

Also replaces a hardcoded `~/.raven/logs/everos-server.log` with
`server_log_path()`, the helper added to stop that name from drifting.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
`raven doctor` and the wizard both called `probe_capabilities()` with its default
base URL while the backend reads `plugins.config[...].base_url`. Someone who moved
everos off port 18791 was therefore told it was not running -- by the two surfaces
added in this branch specifically so that a degraded server would be visible.
`configured_base_url` reads the same two keys `_resolve_plugin_config_slice` does,
without building a registry neither caller has another reason to build.

The multimodal role was in the section-to-capability map and in neither consumer's
iteration, so its mapping entry was dead and the role could fail to build with
nothing saying so. It is config surface the wizard writes, so it joins
DEGRADING_SECTIONS with a note of its own cost -- images, PDFs and audio staying
out of memory, which also makes "so recall runs degraded" the wrong summary for
the group; it now reads "so memory runs degraded".

Records why nothing reaches doctor's exit-2 branch on everos 1.2.1: its `/health`
reports llm as a hardcoded True, because a server that answered at all has one.
The section list is the contract, and everos' own comment says that literal may
become a real probe, so the branch stays.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
`_EXCLUDED_DIRS` mirrors Hermes' EXCLUDED_SKILL_DIRS, but Hermes gates its org
mirror separately and by token rather than by name: with no `.active_org` marker it
prunes `_org` entirely, and with one it descends only into the org the marker
names (`agent/skill_utils.py`, "leave an org and its skills stop resolving,
without any manual cleanup"). Discovery walked the mirror regardless, so every org
on disk was imported -- including orgs whose marker is gone, whose skills Hermes
itself will not load, and for which there is no delete path once imported.

Adding `_org` to the excluded set would have been the opposite error: it drops the
active org's skills, which Hermes does load, and under-importing is the costlier
direction here.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
`claimed_registry_names` started empty every run and was filled only on a
successful install, so a skill whose frontmatter name collided with one a previous
run installed -- or with the user's own skill in the pool -- passed the guard,
landed under a non-colliding directory name, and became precisely the skill
`get(name)` never returns. That is the loss the guard exists to report.

Ordered after `_target_for` so "already on disk" stays its answer to give: a
re-run of the same skill is a no-op, not a dropped duplicate. That ordering also
makes excluding the resolved target from the lookup unnecessary -- `_target_for`
returns a path only when it does not exist yet, so the target is never among the
pool's names. A first version excluded it anyway; mutation testing showed the
exclusion could not change any outcome.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The progress line reported the call it was about to make, so with three entries it
read 3/3 while the third LLM call was still in flight -- on a measured 9.4s run,
about three seconds sitting at 100%, which is the symptom the phase reporter was
added to remove. Reporting after the await instead leaves the first call with no
progress at all, and reporting index-1 never reaches N/N; each report is now a
count of what is done, with one close after the loop.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The window bounds are formatted as naive local time and hermes parses them the
same way (`hermes_cli/session_filters.py`, "naive = local time", then
`dt.timestamp()`). East of UTC, a floor of 1970-01-01 00:00 local is therefore a
pre-epoch instant, and a naive `timestamp()` goes through mktime, which raises
OSError for those on Windows. One day of headroom covers every real offset, and no
hermes session predates it.

Reachable only on the paged path -- Windows, plus more than the 100 sessions the
dry-run listing caps at, plus a positive offset -- and it fails loudly there
rather than silently dropping sessions, which is why this is a floor change and
not a rework of the bounds.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Identity moved to the host in this branch, so `config={"agent_id": ...}` is
ignored and additionally trips the branch's own stale-key warning. The
agent-track test therefore stamped its assistant and tool rows from the services
default while `recall` queried the tag: `hits` came back empty, every per-hit
assertion ran over an empty list, and the `xfail` below absorbed the miss. The
test claimed to prove user_id/agent_id routing and could not go red.

The skill-evolution test carries the same dead key. There the store goes through
`everos.service.memorize` directly, so its recall already matched and the argument
was merely inert -- but nothing reads it, and the warning is noise.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The upgrade SOP this branch edits asks for it: the everos pin moved 1.1.3 ->
1.2.1, the API prefix moved v1 -> v2, identity moved to the host, and
`config_schema` is gone, while the manifest still said 1.0.0. Pinned in four
places that have to move together, plus the two tests asserting them.

Two corrections to the same package while here. The health probe on the adapter
now sends the headers every other call on that client sends: everos ships no auth
today, but a deployment behind a proxy that adds it would read an unauthenticated
probe as a server with no capabilities. And `feedback`'s docstring named everos
1.0.0 as the version with no endpoint to consume the signals -- still true on
1.2.1, whose routes are get / health / knowledge / memorize / metrics / ome /
search, so the statement is now dated to the version actually pinned.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@Kendrick-Song Kendrick-Song changed the title feat(importer): add hermes as a cold-start import source feat(*): add hermes as a cold-start import source Aug 4, 2026
@Kendrick-Song

Copy link
Copy Markdown
Contributor Author

Both reviews are addressed. Thank you both -- between them they found nine
behavioural defects, a dead test and a stale manifest, and every one of the nine was
invisible for the same reason: no test reached the path.
CI is 9/9 green at 723ac26, the branch is merged up to the latest main
(5e6cdad), and the suite is 5405 passed / 30 skipped.

Fixed

Item Commit
G1 -- skills copied before the run's own Proceed? gate 02e61fc
G2 -- a cancelled run still ran both post-phases 02e61fc
R6 -- "all platforms" + background could not work 02e61fc
R8 -- hardcoded server log path 02e61fc
G3 -- doctor and the wizard probed the default address 8caaad0
R5 -- an unbuilt multimodal role was reported by nobody 8caaad0
R1 -- org-mirror skills imported past Hermes' own token gate f11879c
G5 -- registry-name collisions only checked within one run adbb429
G4 -- USER.md progress reached 100% before the last call bac49bf
R7 -- partition floor was a pre-epoch local time bf27da2
R2 -- the agent-track e2e test could not go red 337e4f9
R3 -- plugin manifest still 1.0.0 723ac26
G9 -- health probe sent no auth header (adapter side) 723ac26
R4 -- PR description and title description updated

Every behavioural change had its fix reverted and the covering test confirmed red.
Where a fix had a plausible wrong shape, that shape was mutated too -- excluding
_org wholesale rather than token-gating it, reporting progress after the await
instead of before.

G10 and G11 dropped as advised. The REQUIRED_SECTIONS blind spot is now a comment
at the constant. sender_id confirmed dead before removal, as noted.

Not fixed (2), deliberately

G6 -- _validate_identity's ValueError lands where nobody reads it. Real, and
the reasoning holds. Not taken here because all four call sites
(agent_commands.py, tui_commands.py, gateway_commands.py) are files this
branch does not touch, and the fix is either a config-load-time validation or four
changed except blocks -- a separate change with its own review lens. Tracking it
separately rather than growing a branch two reviews have already called large.

G8 -- session-enumeration cost. Measured rather than estimated, and it comes out
against the change. A dry-run probe is 0.15s here, not a heavy CLI start (the
dry-run path does not load the agent stack), and 8 concurrent probes ran clean.
Driving the real _collect_window against synthetic stores:

sessions span probes depth serial gathered saved
150 180d 17 9 2.5s 1.3s 1.2s
500 180d 27 11 4.0s 1.6s 2.4s
2000 365d 79 12 11.8s 1.8s 10.1s
5000 365d 159 13 23.8s 1.9s 21.9s

Probe counts land within one of the review's own table, which is a useful
cross-check. What the table does not show is the fan-out: _collect_window is
recursive, so asyncio.gather on the two halves expands the tree exponentially
rather than running two at a time. Peak concurrent probes are 2 / 6 / 28 / 72
for those four rows. Trading an unbounded process fan-out for 10-22s, against a
full-tier import that is hours of LLM extraction, is the wrong side of the deal --
enumeration is under 1% of it. A bounded version (Semaphore(4)) keeps most of the
gain and is the shape to use if this ever matters, but its correctness would rest
on synthetic data: this branch has never run the paged path on a real store, which
is the first of the two unverified paths the description flags.

Three places I ended up disagreeing

1. G5's exclude=target is dead code, and mutation testing is what showed it.
The correction is right that seeding from the pool must not break an idempotent
re-run, and I implemented the guard as suggested. Then mutating the exclusion away
left every test green. _target_for returns a path only when it does not exist yet,
so the resolved target is never among the pool's names and the re-run never meets
its own. Ordering the check after _target_for is the whole fix; the exclusion was
defence against a case that cannot arise, and is gone.

2. My own first fix for the wizard's two call sites was wrong. I passed
assume_yes=True there, reasoning its own confirmation already covered the copy.
It does not: the wizard's Start? gate is at onboard_commands.py:4627 (post-fix),
below both early returns at :4369 and :4559. Both now ask, with a test for the declining path.

3. G7 is closed with a second, independent check. Alongside the source reading
in R7, the bounds were probed against a real hermes: --before "2026-07-27 11:04"
returns the 11:03 session, --before "2026-07-27 03:04" does not, so the bounds are
read as naive local time and no window is lost to an offset. Worth noting the
original concern had the direction inverted -- east of UTC, a bound misread as UTC
lands in the future, which is the safe side. The one real residual was the Windows
pre-epoch floor, fixed in bf27da2.

On splitting: agreed in principle, and the everos upgrade plus the identity refactor
is the right seam. Not doing it now because the cost has moved into the tests --
test_everos_backend.py and test_everos_http_adapter.py now assert against both
the 1.2.1 /health shape and the adapter changes in the same cases, so splitting
means rewriting tests rather than moving them. Recorded as a lesson for the next
upgrade of this kind: an irreversible data migration should start life in its own PR.

@0xKT

0xKT commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The two items left deliberately unfixed are now tracked, so nothing here needs to grow the
branch:

Bare references, not Fixes: neither is closed by this PR.

Nothing else outstanding on my side. All eight items I raised are fixed at 723ac26, and I
re-read each fix rather than taking the table on trust -- including the three places the
reply disagreed with my suggested shape, where the reply is right on all three: the
exclude=target guard is unreachable (_target_for only ever returns a path that does not
exist yet, so the target is never among the pool's names), the wizard's Start? gate does
sit below both early returns so assume_yes=True there was my error, and the live
--before probe is a stronger check on the bounds than my source reading was.

Spec side, re-verified on the new head rather than carried over: the eight new commits are
all Conventional, all-ASCII, headers 55-72 chars, each carrying the Co-authored-by
trailer; the title is now feat(*) and passes the title check; Type is a single box; the
description is ASCII-only and its Verification section lists commands anyone can run; no
lockfile or asset drift in the new commits; CI 9/9.

One thing for whoever merges: the everos index migration is one-way, so per this PR's own
addition to the upgrade SOP it belongs in the release notes, not only in a description that
is known to whoever read it.

@Kendrick-Song
Kendrick-Song merged commit 5b26c83 into main Aug 4, 2026
9 of 12 checks passed
@Kendrick-Song
Kendrick-Song deleted the feat/hermes_cold_start_import branch August 4, 2026 12:44
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.

4 participants