fix(memory): rescue skill extraction, disable foresight, tighten APIs (#393)
* feat(ome): exponential backoff + jitter between retry attempts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(events): carry case body + vector on agent skill chain
* feat(strategies): populate extended agent-skill chain events
* feat(md): AgentSkillReader.list_by_cluster for md-first skill enum
* fix(strategies): rescue extract_agent_skill from cascade-lag dead-letter
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(agentic): honor radius, use kind-shaped rerank, non-empty passage
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(api/ome): expose dispatched + runs, distinguish not_dispatched
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(release): v1.2.3 — agent-skill rescue + OME/agentic contracts
* fix(agentic): drop inert radius plumbing; correct release docs
* fix(md): sanitize LLM-generated skill names against path traversal
AgentSkillFrontmatter.name comes straight from LLM output
(memory.strategies.extract_agent_skill) and was concatenated
unsanitized into the skills/skill_<name>/ directory segment on both
the write path (agent_skill_writer._skill_dir) and the read path
(agent_skill_reader._skill_dir). Given a sufficiently long ../ prefix,
the write target could escape the memory root (CWE-22). A live run
also produced a skill name containing spaces and CJK characters,
proving the "keep snake_case" docstring convention on
AgentSkillFrontmatter.name is not enforced at runtime.
This is the same class of defect already fixed for knowledge-upload
titles/categories. Promote that fix's sanitizer
(knowledge_writer._sanitize_dirname) to a shared primitive,
everos.core.persistence.markdown.sanitize_dirname, so there is one
CWE-22 defense for md directory names instead of two independently
maintained copies:
- New core/persistence/markdown/path_safety.py holds sanitize_dirname
(idempotent: sanitize(sanitize(x)) == sanitize(x)), exported through
the markdown + persistence facades.
- SkillPathMixin gains skill_dir_name(), the single sanitization point
both AgentSkillWriter._skill_dir and AgentSkillReader._skill_dir now
derive from, replacing their previous independent string
concatenation.
- KnowledgeWriter now imports the shared sanitize_dirname instead of
keeping its own private copy.
- AgentSkillFrontmatter.name gains a field_validator rejecting path
separators / ".." as defence in depth, so a hand-edited SKILL.md is
caught on parse rather than silently relocating the skill on the
next write.
Idempotency is what keeps the reader and writer in agreement even
though they recover a skill_name from different sources:
list_by_cluster derives it from the on-disk (already-sanitized)
directory name, while extract_agent_skill._hydrate_algo_skills
re-reads using the frontmatter's raw name field. A regression test
(test_agent_skill_reader.py) seeds a skill whose frontmatter name
contains CJK + a space and asserts both routes resolve to the same
file.
No data migration: agent-skill extraction has never once succeeded
before this branch (the cascade-lag defect this branch fixes meant
.skills/ was never created), so there is no legacy skill corpus whose
directory names would change under the new sanitizer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(e2e): make the agent-skill chain assertion real
tests/conftest.py's autouse fixture pins embedding + rerank capability
to unavailable for hermeticity, and this test never opted back in.
trigger_skill_clustering and extract_agent_skill both body-guard on
get_embedding_capability().available and return early, so the module
docstring's "real embedder" claim was false and the skill chain never
ran: measured log counts were skill_cluster_updated=0,
agent_skills_extracted=0, strategy_gated_off_embedding_unavailable=10.
The three skill assertions were assert len(...) >= 0 — always true —
with a comment blaming "LLM-dependent" flakiness for a count that was
in fact deterministically zero. This is why a defect that made
agent-skill extraction fail 4/4 in production reached a release: there
was no working e2e coverage of the chain.
- New _opt_in_real_embedding_and_rerank autouse fixture, scoped to this
file only, resets everos.component.embedding.accessor._capability and
everos.component.rerank.accessor._capability to None (the mechanism
the global fixture's own docstring prescribes) so both capabilities
rebuild from the real .env credentials tests/e2e/conftest.py already
loads. Restores to None on teardown; every other test keeps its
hermetic default.
- Replaced the three vacuous per-agent assertions with one aggregate
floor across all three agents (>= 1 total skill). A per-agent floor
would be flaky: extract_agent_skill has no cluster-size gate, only
everalgo's per-case skip_quality_threshold, so a single low-quality
trajectory can legitimately yield 0 skills for one agent.
- Added a sharper, defect-specific check: assert no dead-lettered
extract_agent_skill run in OME's run_record (via
OfflineEngine.list_runs), since a dead-letter (retries exhausted)
is unambiguously a failure, unlike a quality-gated 0-skill outcome.
- Corrected the module docstring's "real embedder" claim and the old
"# 4.5" comment's reasoning: extract_agent_skill has no cluster-size
gate, only everalgo's per-case quality threshold.
Unexecuted: this test is slow + live_llm and this machine has no
provider credentials (the verification .env was deleted), so make ci
does not run it and it could not be run here. Verified by inspection
instead: ran the test file with -m "" to override the marker
deselection and confirmed it proceeds past the new fixture and
through app lifespan startup without error, failing only at the
expected point — LLMNotConfiguredError from the missing API key —
which confirms the fixture and imports are wired correctly and the
only blocker is the missing credentials, not a bug in this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(strategies): sanitize skill name before frontmatter construction
Follow-up to afe1609 (path-traversal sanitization). That commit added a
field_validator on AgentSkillFrontmatter.name rejecting path separators
or "..", intended as a read-side defence for hand-edited SKILL.md
files. But _persist_skill (memory/strategies/extract_agent_skill.py)
constructs AgentSkillFrontmatter(name=skill.name) with skill.name
straight from LLM output — the raw, unsanitized string — so the
validator actually fires on the write path too. Sanitization already
makes the on-disk path safe (SkillPathMixin.skill_dir_name), so an
LLM emitting a traversal-shaped name (reachable via prompt injection,
since the LLM's input is user conversation content) gained nothing
from the validator except a new failure mode: ValidationError ->
strategy raises -> OME retries with backoff -> dead-letter -> that
case's skill is permanently lost. A DoS vector introduced by a
security fix, and it also contradicted the validator's own docstring
("catches a hand-edited SKILL.md").
Also fixes a latent second bug in the validator itself, caught by the
new tests below: it rejected any name containing the substring "..",
but sanitize_dirname keeps "." as a safe character, so
"../" * 8 + "tmp/pwned" sanitizes to "................tmppwned" —
still containing ".." many times over. The validator would have
rejected the sanitizer's own safe output. Narrowed the check to actual
path separators or the name being exactly ".." (the only case where
".." functions as a real traversal component when there's no separator
left to combine it with).
Fix:
- SkillPathMixin gains sanitize_skill_name(skill_name) — the bare
sanitized name (no skill_ prefix), factored out of skill_dir_name so
both share one sanitizer call.
- _persist_skill now sanitizes skill.name via sanitize_skill_name
once, up front, and uses that same sanitized string for
AgentSkillFrontmatter.id, .name, and the writer.write_main() call.
A traversal-shaped LLM name is now made filesystem-safe before it
ever reaches the frontmatter constructor, instead of tripping the
validator.
- The validator's docstring now describes actual behaviour: the write
path pre-sanitizes, so the validator only fires for a name that
bypassed the writer (e.g. a hand-edited file, or any other direct
AgentSkillFrontmatter construction that skips pre-sanitization).
Bonus: with the write path pre-sanitizing, frontmatter.name becomes
byte-identical to the directory-derived name for LLM-written skills —
an identity, not merely an idempotency argument. This also closes the
gap in the previous commit's reader/writer test, which proved
idempotency generically but never drove an adversarial name through
the actual production write path end-to-end.
Tests:
- test_agent_skill.py: constructing AgentSkillFrontmatter with a
pre-sanitized adversarial name (mirroring _persist_skill's own call
shape) succeeds and yields a separator-free name; the read-side
rejection test for bypassed/hand-edited names is unchanged and still
passes with the narrowed check.
- test_agent_skill_writer.py: new parametrized identity test — for
both an adversarial and a CJK/space raw name, sanitize once, write
via that sanitized name, and assert frontmatter.name equals the
directory-derived name exactly.
- test_agent_skill_reader.py: docstring updated to clarify its
existing round-trip test now covers the bypass case (a caller that
writes via a raw, unsanitized name directly through the writer,
skipping _persist_skill's pre-sanitization) rather than the normal
production path, which is proven as an identity by the writer test
above.
- Existing test_extract_agent_skill.py strategy tests (snake_case
fixture names) are unaffected — sanitize_dirname is the identity
function for already-safe names.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(md): reject degenerate sanitizer results, read globbed paths
Security review of 0c6820f found it did not close the DoS it was meant
to close: sanitize_dirname("../") returns ".." verbatim, because "."
is a safe character and is not stripped by the character-class filter
— only the leading "/" is removed. sanitize_skill_name("../") ->
sanitize_dirname("../") -> "..", and AgentSkillFrontmatter(name="..")
still raises ValidationError (name == ".." is exactly the case the
narrowed validator rejects). Same dead-letter DoS as 0c6820f, just a
shorter payload; the previous commit's tests only exercised the long
"../" * 8 + "tmp/pwned" payload, which happens to sanitize past the
fixpoint. The same fixpoint is a real one-level directory escape on
the knowledge path, which has no skill_ prefix to protect it:
Path(root) / sanitize_dirname("../", "Others") / "doc_123" resolved to
root/doc_123, skipping the category directory entirely.
Fix (path_safety.py): sanitize_dirname now falls back on "", ".", or
".." instead of only "". This one change closes both the skill
dead-letter DoS and the knowledge one-level escape, since both callers
already route through this single primitive. Also NFC-normalizes the
input before the character filter (unicodedata.normalize("NFC", raw)),
so an NFD-decomposed accented character (base letter + combining
mark, which is not \w) no longer silently loses its accent. Rewrote
the docstring, which previously claimed ".. sequences are always
stripped" (false — "." is explicitly a safe character) and "cannot
escape the directory it is concatenated into" (false for an unprefixed
caller before this fix); it now states what actually holds: no
separator survives, so the result is always exactly one path
component, and it is never "", ".", or "..".
Also fixes (per review, cheap and worth doing alongside):
- AgentSkillReader.list_by_cluster previously globbed skill_*/SKILL.md,
stripped the prefix to recover a name, then called read_main(name),
which re-derives (and re-sanitizes) the path from that name. Any
on-disk directory whose suffix was not already a sanitizer fixpoint
(e.g. "skill_My Skill", a raw space) re-derived to a path that
doesn't exist and was silently dropped. Since list_by_cluster is the
documented strong-consistency existence check, a dropped skill would
make the LLM emit add() for a skill that already exists, duplicating
it at the sanitized path and orphaning the original. Fixed by having
list_by_cluster read each globbed path directly (new _read_path
helper, shared with read_main) instead of round-tripping through a
recovered name — the reader never derives a path at all on this
route, which is a stronger guarantee than the idempotency argument
the docstrings previously leaned on.
- e2e test: made fixture ordering explicit — the embedding opt-in
fixture now takes _reset_embedding_capability_singleton and
_reset_rerank_capability_singleton as parameters so pytest's
dependency graph guarantees correct ordering, rather than relying on
collection order between conftest files. Added a positive
"extract_agent_skill actually ran" assertion (any status) before the
dead-letter check — without it, the dead-letter assertion alone is
vacuously satisfied by a strategy that never executed at all; it was
only meaningful before because the skill-count floor happened to run
first. Dropped the rerank capability opt-in and the module
docstring's "real reranker credentials" claim: nothing on the
agent-skill write path touches rerank, so opting it in only widened
the credential surface with no coverage benefit.
- CHANGELOG: corrected the validator description (rejects a path
separator or being exactly "..", not any string containing ".."),
and added the previously-missing user-visible fact that
AgentSkillFrontmatter.name and the agent_skill LanceDB primary key
now hold the sanitized name, not the raw LLM output.
Tests: parametrized the sanitize -> construct -> (write, for the
writer-level test) tests over a boundary family instead of one long
payload: "..", "../", "/../", ".", "./", "!!!" (empty), "a" * 200
(truncation), a CJK+space name, and the original "../" * 8 +
"tmp/pwned". Each case asserts the sanitized name is a single
component, is never "" / "." / "..", frontmatter construction
succeeds, and (writer-level) frontmatter.name is byte-identical to the
directory-derived suffix. New test_path_safety.py cases pin the
degenerate-fixpoint fallback directly, the knowledge-style unprefixed
one-level-escape repro, and NFC normalization. New
test_list_by_cluster_finds_skill_whose_directory_suffix_has_a_space
reproduces the exact list_by_cluster drop bug against a directory
written outside the writer entirely.
Explicitly not in scope (per review): the collision behaviour where
"fix django" and "fix_django" now map to the same directory is a real
product-decision question the reviewer is raising separately, not
touched here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(md): record the skill-name collision trade-off
No behaviour change. Documents a product decision the coordinator made
explicit: sanitize_dirname is lossy, so distinct raw skill names can
collapse onto the same directory ("fix django" and "fix_django" both
become "fix_django"; "fix!django" and "fixdjango" both become
"fixdjango"; names differing only past the 50-char cap also collide).
Because AgentSkillWriter.write_main is a full-file replace and the
LanceDB primary key is f"{agent_id}_{sanitized_name}", a collision
means the later skill silently overwrites the earlier one, losing its
accumulated source_case_ids, maturity_score, and body.
This is accepted rather than mitigated: the LLM's add/update decision
for a skill is keyed on the name it sees, so a collision usually reads
as an intended update anyway; and adding a disambiguating suffix would
break the frontmatter.name == directory-suffix identity the
reader/writer seam (from 0c6820f) relies on.
- SkillPathMixin.sanitize_skill_name docstring now states the
collision consequence and the two reasons it is accepted, so a
reader does not have to derive them.
- sanitize_dirname's docstring gains one line: the function is lossy
and not injective; callers that need distinct outputs for distinct
inputs must disambiguate themselves. General primitive — the
knowledge path calls it too.
- CHANGELOG: added the collision consequence to the existing
path-traversal entry, next to the already-documented fact that name
/ the LanceDB key hold the sanitized value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(md): return skill bodies from list_by_cluster; correct docs
Security review of 1b8cf11 + add842d found the list_by_cluster fix was
incomplete: it stopped its own enumeration from dropping a skill whose
directory suffix wasn't a sanitizer fixpoint, but the caller
(_hydrate_algo_skills) still re-read each selected skill by
fm.name via read_main, which re-derives (and re-sanitizes) a path from
that name and drops it there instead. Reproduced on a real
filesystem: skill_My Skill/ enumerates fine, but
read_main("My Skill") re-derives to skill_My_Skill/ and misses. The
drop moved one layer downstream; existing_relevant_skills was empty
before and after the prior fix.
Fix: list_by_cluster now returns (frontmatter, body) pairs instead of
frontmatter alone, so the caller never needs a second, name-based
read. _select_existing_skills / _rank_skills_by_relevance updated to
carry (fm, body) tuples through selection; _hydrate_algo_skills is
deleted — the body is already in hand, so there's nothing left for it
to do. This closes the drop for real, removes the second disk read
(the 2n-read concern carried since Task 4), and makes "the reader
never derives a path" true end-to-end rather than true only for
list_by_cluster's own enumeration step.
New end-to-end regression test
(test_select_existing_skills... / test_existing_skills_reaches_llm_for_skill_whose_directory_has_a_space)
seeds a skill_My Skill/ directory directly on disk (bypassing the
writer) and runs the real extract_agent_skill strategy against it,
asserting the skill reaches existing_relevant_skills with non-empty
content — the property the previous commit's test docstring claimed
but the code didn't yet deliver. The reader-level regression test
gained the same body assertion.
Also, per review:
- path_safety.py: corrected the NFC docstring claim, which was wrong
for the ~1,082 Unicode composition-exclusion codepoints (e.g.
Devanagari क़/ख़, U+0958/U+0959) — NFC decomposes an
already-precomposed exclusion character instead of preserving it, so
the combining mark is stripped either way. Scoped the claim to
"best-effort for the common case", not a guarantee for every script.
New test pins this directly.
- Dropped the e2e test's unused _reset_rerank_capability_singleton
fixture parameter: the reviewer adjudicated the earlier instruction
conflict the other way — ordering is only meaningful between
fixtures that touch the same state, and this fixture never reads or
writes the rerank capability at all.
- Widened the skill-name collision documentation (SkillPathMixin.sanitize_skill_name,
CHANGELOG) beyond dropped-punctuation / space-collapse / truncation
to the larger case: every combining mark is non-\w and is stripped
regardless of script, so e.g. Devanagari "किताब" and "कताब" both
collapse to "कतब" (same for Thai tone marks, Hebrew niqqud, Arabic
harakat).
- Corrected the collision justification: because _persist_skill
sanitizes before frontmatter construction, the LLM sees the
already-sanitized name in existing_relevant_skills, so a colliding
raw name is an *affirmative* decision that two skills are different,
not a probable intended update. The decision to accept collisions
still stands, but on its real grounds: a disambiguating suffix would
break the fm.name == directory-suffix identity the reader/writer
seam relies on, and detecting-and-raising would reintroduce the
dead-letter DoS.
- Merged two consecutive "# -- Internals --" banners in
agent_skill_reader.py into one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(md): skip unparseable SKILL.md instead of failing the cluster
`list_by_cluster` only handled a missing file, so a ValidationError from
`_read_path` aborted the whole enumeration. That enumeration is what
feeds `extract_agent_skill` its existing skills, so one bad SKILL.md
starved every skill in the cluster and dead-lettered that cluster's
extraction on every subsequent run -- the permanent-failure mode this
md-first read path was introduced to eliminate. The write path already
pre-sanitizes to avoid exactly this; the read path was left open.
The trigger surface is the whole schema, not just the traversal
validator this PR added: `_read_path` validates the full
`AgentSkillFrontmatter`, so a field a later revision makes required
would take out every existing file at once. Reproduced both ways.
`read_main` still propagates -- a caller naming one specific skill needs
an error, since `None` already means "not created yet" and reusing it
for "exists but is corrupt" would let an upsert overwrite the damage.
Also moves this route's `OfflineEngine` import under TYPE_CHECKING. It
is used only to annotate `_summarize_runs`, and an eager import
contradicted the deferred `_get_engine` import a few lines below. Note
this saves nothing at startup today: `service.memorize` imports the
engine eagerly to construct it, so any app process pays the ~750ms
apscheduler cost regardless.
* docs(md): correct two docstrings, add the case-collision dimension
Three fixes to claims that did not match behavior:
- `_rank_skills_by_relevance` claimed no skill is silently dropped from
the prompt. The backfill loop is capped at MAX_SKILLS_IN_PROMPT, and
the function only runs when the cluster already exceeds that budget, so
skills beyond K are dropped by design. Reworded to what the backfill
actually guarantees: a lagging index cannot under-fill the prompt.
- `sanitize_skill_name` enumerated collision causes in detail but omitted
case, the dimension an LLM varies most freely. "Fix Django" and "fix
django" sanitize to two distinct names -- two LanceDB rows, but one
directory on a case-insensitive filesystem (macOS APFS, Windows NTFS
defaults), so the index advertises a name whose content was overwritten.
- The same docstring justified accepting collisions partly on a
disambiguating suffix breaking the `frontmatter.name` = directory-suffix
identity. It would not: writing "fix_django_2" into both keeps that
intact. Replaced with the real reason it is deferred rather than
dismissed -- it needs a collision probe and a case-folding rule.
Adds the knowledge-writer sanitization tests that were missing entirely:
swapping in the shared primitive changed NFD input ("Résumé" no longer
degrades to "Resume") and made a "." / ".." topic fall back. Knowledge
upload predates this PR, so unlike skills it has a corpus whose
directory names those first cases affect. Both tests verified red against
the 1.2.2 sanitizer.
* docs(changelog): scope the migration claim, record run_record growth
Three corrections to the 1.2.3 entry:
- "No data migration" was asserted for the whole sanitization change but
only holds for agent skills, which have no corpus because extraction
never succeeded. Knowledge upload predates this release and does have
one: NFD topics and `.`/`..` topics resolve to a different directory
now. Scoped the claim and spelled out both cases.
- Added the case dimension to the collision list, and replaced the
"disambiguating suffix breaks the name = directory identity" reason
with the accurate one -- it does not break it, it just needs a probe
and a case-folding rule, so it is deferred rather than rejected.
- Recorded that `SkillClusterUpdated` now persists a 1024-dim vector in
`run_record.event_payload`: ~0.8 KB to ~14 KB per record, ~14 MB per
strategy at the default 1000-record ring buffer. Operators sizing
ome.db need this number, and it was not stated anywhere.
* fix(strategies): ship extract_foresight disabled by default
The sender scan reads `m.role` off every memcell item, but only
ChatMessage carries it: ToolCallRequest has `sender_id` and no `role`,
ToolCallResult has neither. So any memcell holding a tool call raises
AttributeError before the first sender resolves -- correct on plain user
chat, guaranteed to fail on agent trajectories, where it burns its
max_retries budget and dead-letters on output nothing consumes today.
Flipped the decorator rather than `default_ome.toml`, because `everos
init` skips an existing `~/.everos/ome.toml` (init_cmd.py:85), so a
template edit would reach new installs only. The toml opt-in is left
working on purpose -- a chat-only deployment does get correct
foresights -- and documented in both the module docstring and the
template comment.
This is a stop-gap. The fix is per-episode extraction, like
atomic_fact, which needs an everalgo entry point that does not exist
yet.
The one test that used foresight as its UserPipelineStarted subscriber
now opts back in through that same toml key, so the opt-in path is
covered rather than worked around. It has to wait for the override to
reach the registry first: ConfigReloader.start() fires its initial load
as a task, so engine.start() returns before ome.toml is applied, and an
emit inside that window is judged against the coded defaults and
dropped by the enabled gate with no redelivery.
* fix(ome): hold engine_sem per attempt, not across the retry chain
The backoff this PR added slept inside the semaphore block, so a run
waiting to retry kept its concurrency slot. That turns a partial outage
into a total stall: with max_concurrent_runs slots and a 1s/2s/4s
backoff, enough simultaneously-failing runs park every slot in
asyncio.sleep and starve strategies that would have succeeded.
The cap exists to bound concurrent strategy work -- LLM calls,
embeddings, storage IO -- and a sleeping coroutine consumes none of it.
Backpressure on the failing work is intended; backpressure on everything
else is not. Semantics change is deliberate and stated in the docstring:
the cap still applies to execution, no longer to waiting.
The guard uses a single-permit semaphore so locked() is unambiguous, and
asserts a second waiter actually acquires -- locked() alone would pass on
an implementation that freed the slot but left waiters unable to take it.
Verified red against the previous structure.
* fix(strategies): reap the directory a renamed skill leaves behind
everalgo treats a name change as a first-class update: _apply_update
preserves prior.id while swapping the name, so _persist_skill wrote the
skill to a new skill_<new_name>/ and the old directory survived with the
same cluster_id.
That is not a cosmetic leak now. Since existing skills are read from
markdown rather than LanceDB, the orphan returns in the next run's
existing_relevant_skills as a duplicate of a skill the LLM already
renamed -- feeding exactly the add-instead-of-update full-replace clobber
this PR set out to close, once more per rename. Reconciliation keys off
skill.id, the only field that survives a rename (a fresh add mints a
uuid4 and can never match), and never deletes a name another emitted
skill just claimed.
Also in this pass:
- AgentSkillWriter.delete_skill, the one destructive operation here. It
fails closed: a directory it cannot resolve by the writer's own path
rule is left alone rather than targeted by anything looser.
- reference_name / script_filename now sanitized on both reader and
writer. They are appended after the skill_<name> segment, so
skill_dir_name never covered them. Zero callers in src/ today; closing
it before progressive disclosure wires them up.
- Agentic case rows with every passage field empty fall back to a
placeholder instead of raising ValueError in everalgo's _format_docs
and 500ing a whole search the row merely appears in.
- The retire op is documented as unimplemented rather than left implied.
aextract returns a flat list with no discriminator, so a retirement
arrives as an ordinary low-confidence skill and is written back like
any other. Honouring it means either giving an LLM confidence score
authority to delete the source of truth, or a retired flag that the
enumeration, cascade, and search all learn to filter on -- a design
decision, deferred.
- The embedding body-guard comment no longer claims to protect a local
embed call; this strategy stopped embedding when the vector moved onto
the event.
* fix(strategies): stop extract_foresight crashing on tool-call memcells
The sender scan read m.role off every memcell item, but only ChatMessage
carries it: ToolCallRequest has sender_id without it, ToolCallResult has
neither. The first tool call raised AttributeError before any sender was
resolved, so the strategy was correct on plain user chat and
dead-lettered every time on agent trajectories.
everalgo contracts for exactly this input -- user_memory/_render
.chat_messages says "the caller need not pre-filter; an
AgentMemCell-shaped MemCell is acceptable input" -- and every other
user-memory extractor gets that for free by delegating. This strategy was
the one place the filter was hand-rolled, and it was hand-rolled wrong.
It stays disabled by default, but for the correct reason: nothing in
EverOS reads foresights yet, so running it spends one LLM call per sender
per memcell on write-only data. The earlier justification (per-episode
extraction needs an everalgo entry point that does not exist) confused
extraction granularity with the crash; granularity is still open, the
crash was one line. Fixing it is what makes the documented ome.toml
opt-in actually usable.
The guard pins both directions: a pure agent trajectory extracts nothing
and never reaches the LLM, and a mixed memcell extracts for human senders
only -- an implementation that stopped raising but scanned tool-call
sender_id values would invent "agent" as a user.
CHANGELOG also records that the stale-index clobber is fully closed only
for clusters at or below MAX_SKILLS_IN_PROMPT; above it LanceDB orders
the markdown candidates, and the skill a lagging index omits is the one
written most recently.
* docs(changelog): fold the merged cascade work into the 1.2.3 entry
#392 landed on main with its entries under [Unreleased] and no version
bump. Since 1.2.3 ships that code, leaving them there would have the
release notes disclaim work the release contains. Merged section by
section into [1.2.3] and dated it to the actual release day.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kendrick-Song
GitHub
parent
8024fe576e
commit
9d48544280
239
CHANGELOG.md
239
CHANGELOG.md
|
|
@ -7,25 +7,167 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- **`[cascade]` settings section** — the four maintenance cadences
|
||||
(`optimize_heartbeat_seconds`, `optimize_prune_interval_seconds`,
|
||||
`optimize_prune_retention_seconds`, `optimize_rebuild_interval_seconds`) are
|
||||
now configurable. They were already constructor arguments on `CascadeWorker`,
|
||||
but `CascadeConfig` did not carry them and no production path passed one, so
|
||||
the defaults were unreachable — which is why the 12h rebuild sweep could not
|
||||
be exercised by any soak run shorter than half a day. The deadlines that
|
||||
bound a hung call are deliberately **not** exposed: they are hang-catchers
|
||||
sized from measured durations, where too low manufactures failures on a
|
||||
healthy table and too high leaves a wedged one invisible for longer. Note
|
||||
`optimize_prune_retention_seconds` has a second effect worth reading before
|
||||
tuning — it also decides how long index files keep a manifest naming them,
|
||||
and below LanceDB's 7-day unverified window they then wait out the full 7
|
||||
days.
|
||||
## [1.2.3] - 2026-08-07
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Agent skill extraction is no longer stuck in a retry-then-dead-letter
|
||||
loop.** Target case data now travels on `SkillClusterUpdated` and existing
|
||||
skills for the cluster are read from markdown (strong-consistency), so the
|
||||
strategy never races cascade indexing. Prior to this fix, running a fresh
|
||||
agent trajectory produced zero `SKILL.md` files — `.skills/` did not exist.
|
||||
**The related stale-index clobber is fully closed only for clusters at or
|
||||
below `MAX_SKILLS_IN_PROMPT` (10).** Above it, markdown still supplies the
|
||||
candidate set but LanceDB orders it, and the skill a lagging index omits is
|
||||
by definition the one written most recently — the one most likely to need
|
||||
`update` — so it can be ranked out of the prompt and re-added instead. The
|
||||
window is narrow (it needs a cluster over 10 skills *and* an index that has
|
||||
not caught up) and the consequence is the pre-existing full-replace, not a
|
||||
new failure mode.
|
||||
- **`POST /api/v2/ome/trigger` no longer masks strategy state.** The `status`
|
||||
field now distinguishes `not_dispatched` (all dispatch gates rejected the
|
||||
strategy — usually a missing `"force": true`) from `ok` (dispatched and
|
||||
settled). The new `runs` field surfaces dead-lettered strategy runs that
|
||||
were previously invisible to the caller. **If your client matches
|
||||
`status` exhaustively (Python `Literal`, TypeScript union), add a
|
||||
`not_dispatched` branch.**
|
||||
- **Agentic search on agent memory now uses the skill-shaped rerank
|
||||
passage.** The cross-encoder previously saw only the raw `description`
|
||||
field instead of the `name + description + skill instruction` triple that
|
||||
the HYBRID lane uses. A skill with empty `description` (a legal everalgo
|
||||
output — see `everalgo/agent_memory/skill_ops.py:294`) no longer causes
|
||||
HTTP 500 during the LLM sufficiency check.
|
||||
- **OME strategy retries now back off between attempts.** A retry-class error
|
||||
(e.g. waiting on eventually-consistent state) previously exhausted its
|
||||
`max_retries` budget in milliseconds; the loop now sleeps
|
||||
`min(base * 2**(attempt-1), cap)` plus up to `jitter` seconds
|
||||
(defaults: `1s` base / `10s` cap / `0.5s` jitter — code-only defaults,
|
||||
not currently exposed via `everos.toml` or `ome.toml`). **`engine_sem` is
|
||||
now held per attempt rather than across the whole retry chain**, so the
|
||||
backoff sleep does not occupy a concurrency slot. The cap bounds
|
||||
concurrent strategy *work* — LLM calls, embeddings, storage IO — and a
|
||||
coroutine waiting to retry consumes none of it; holding the slot would
|
||||
have turned a partial outage into a total stall, since enough
|
||||
simultaneously-failing runs park every one of the
|
||||
`max_concurrent_runs` slots in `asyncio.sleep` and starve strategies that
|
||||
would have succeeded. Backpressure on failing work is intended;
|
||||
backpressure on everything else is not.
|
||||
- **Path-traversal hardening for LLM-generated agent-skill names (CWE-22).**
|
||||
`AgentSkillFrontmatter.name` comes straight from LLM output
|
||||
(`extract_agent_skill`) and was concatenated unsanitized into the
|
||||
`skills/skill_<name>/` directory segment on both the write and read
|
||||
paths; given a sufficiently long `../` prefix, the write target could
|
||||
escape the memory root. This is the same class of defect previously
|
||||
fixed for knowledge-upload titles/categories (see `knowledge_writer.py`
|
||||
in an earlier 1.2.x). The sanitizer is now a single shared helper
|
||||
(`everos.core.persistence.markdown.sanitize_dirname`) used by both
|
||||
`KnowledgeWriter` and the new `SkillPathMixin.skill_dir_name()` /
|
||||
`sanitize_skill_name()`, instead of two independently maintained copies.
|
||||
`extract_agent_skill` now sanitizes the LLM-emitted name *before*
|
||||
constructing `AgentSkillFrontmatter`, so **`AgentSkillFrontmatter.name`
|
||||
and the LanceDB `agent_skill` primary key now hold the sanitized name**
|
||||
(spaces become `_`, characters outside `[\w\-.]` are dropped, capped at
|
||||
50 chars), not the raw LLM output — a user-visible change for anything
|
||||
that reads a skill's `name` field expecting the verbatim LLM string.
|
||||
`AgentSkillFrontmatter.name` also gained a validator rejecting a name
|
||||
containing a path separator, or being exactly `..`, so a hand-edited
|
||||
`SKILL.md` that bypasses the writer's sanitization is caught on read
|
||||
rather than silently relocated (the substring form, e.g. a name that
|
||||
merely *contains* `..`, is deliberately allowed — sanitized output can
|
||||
legitimately contain runs of literal dots). `sanitize_dirname` itself
|
||||
falls back (not just on an empty result, but also on `.` or `..`) so a
|
||||
short input that is itself a sanitizer fixpoint — e.g. `"../"` sanitizes
|
||||
to `".."` verbatim without this fallback — cannot resolve to the same
|
||||
directory or its parent; this closes both the agent-skill case and an
|
||||
equivalent one-level escape on the knowledge-upload path, which has no
|
||||
`skill_`-style prefix protecting its sanitized segment. **No data
|
||||
migration is needed for agent skills**: extraction has never
|
||||
successfully produced a `SKILL.md` before this release (see the
|
||||
cascade-lag fix above), so there is no legacy skill corpus whose
|
||||
directory names would change. **Knowledge documents do have a
|
||||
pre-existing corpus**, and two inputs resolve to a different directory
|
||||
than before: a decomposed (NFD) topic or category now keeps its
|
||||
combining marks (`"Résumé"` no longer degrades to `"Resume"`) because
|
||||
the shared helper NFC-normalizes first, and a topic or category of
|
||||
exactly `.` or `..` now falls back instead of resolving onto the
|
||||
parent directory. Precomposed input — including CJK — is unaffected;
|
||||
the character class is unchanged from the previous private copy.
|
||||
Sanitizing is lossy: skills whose raw names
|
||||
differ only in characters the sanitizer drops or replaces (e.g.
|
||||
`"fix django"` vs. `"fix_django"`) now share one `SKILL.md`, and so do
|
||||
names differing only in a combining mark regardless of script (e.g.
|
||||
Devanagari `"किताब"` vs. `"कताब"` — a combining mark alone is not `\w`
|
||||
and is stripped either way; same for Thai tone marks, Hebrew niqqud,
|
||||
Arabic harakat). The later write wins — the earlier skill's
|
||||
`source_case_ids`, `maturity_score`, and body are silently lost, not
|
||||
merged. Case is *not* folded, so `"Fix Django"` and `"fix django"` stay
|
||||
two distinct sanitized names — two LanceDB rows, but one directory on a
|
||||
case-insensitive filesystem (macOS APFS and Windows NTFS defaults),
|
||||
where the index then advertises a name whose content was overwritten.
|
||||
This is accepted for now rather than mitigated: detecting a collision
|
||||
and raising would reintroduce the dead-letter DoS the sanitizer was
|
||||
built to avoid, and a disambiguating suffix — the workable option —
|
||||
needs a collision probe plus a case-folding rule, so it is deferred to
|
||||
a deliberate pass rather than added here.
|
||||
- **A renamed skill no longer leaves an orphan directory that pollutes the
|
||||
next extraction.** everalgo treats a name change as a first-class update
|
||||
(`skill_ops._apply_update` preserves `prior.id` while swapping the name),
|
||||
so the emitted skill was written to a new `skill_<new_name>/` while the
|
||||
old directory survived carrying the same `cluster_id`. Because existing
|
||||
skills are now read from markdown rather than LanceDB, that orphan did
|
||||
not merely sit on disk — it came back in the next run's
|
||||
`existing_relevant_skills` as a duplicate of a skill the LLM had already
|
||||
renamed, feeding exactly the `add`-instead-of-`update` full-replace
|
||||
clobber this release set out to close, once more per rename. The old
|
||||
directory is now reaped after the new one is written, keyed on the
|
||||
skill's `id` (the only thing that survives a rename; a fresh `add` mints
|
||||
a uuid and can never match). A prior name that another skill in the same
|
||||
batch just claimed is never deleted.
|
||||
- **`extract_agent_skill` retire ops are documented as unimplemented rather
|
||||
than silently mispersisted.** `AgentSkillExtractor.aextract` returns a
|
||||
flat list with no op discriminator, so a retirement arrives as an
|
||||
ordinary skill with `confidence < retire_confidence` and was written back
|
||||
like any other — staying in markdown, in the next prompt, and in search.
|
||||
The behaviour is unchanged; the module docstring no longer claims retire
|
||||
is handled. Honouring it is a design decision (delete the directory, or
|
||||
add a `retired` flag that the enumeration, cascade, and search all
|
||||
filter on) deferred to its own change.
|
||||
- **`reference_name` and `script_filename` are sanitized.** Both are
|
||||
appended *after* the `skill_<name>` segment, so `skill_dir_name` never
|
||||
covered them; they now go through the same `sanitize_dirname` primitive
|
||||
on both the reader and the writer. No caller in `src/` reaches them
|
||||
today, so nothing was exploitable — this closes the gap before
|
||||
progressive disclosure wires them up.
|
||||
- **A single unparseable `SKILL.md` no longer disables skill extraction
|
||||
for its whole cluster.** `AgentSkillReader.list_by_cluster` propagated
|
||||
any frontmatter `ValidationError`, which aborted the enumeration that
|
||||
feeds `extract_agent_skill` its existing skills — so one hand-edited
|
||||
file (or, after a future schema revision adds a required field, every
|
||||
existing file at once) dead-lettered that cluster's extraction on every
|
||||
run. Offending files are now logged and skipped. `read_main` still
|
||||
raises, since a caller naming one specific skill needs to hear about
|
||||
corruption rather than receive the `None` that already means "not
|
||||
created yet".
|
||||
merged. This is accepted, not mitigated, on two grounds: a
|
||||
disambiguating suffix would break the `name` ≡ directory-suffix
|
||||
identity the reader/writer relies on, and detecting a collision and
|
||||
raising would reintroduce the dead-letter DoS the sanitizer was built
|
||||
to avoid.
|
||||
`"fix django"` vs. `"fix_django"`) now share one `SKILL.md`, and the
|
||||
later write wins — the earlier skill's `source_case_ids`,
|
||||
`maturity_score`, and body are silently lost, not merged. This is
|
||||
accepted, not mitigated: the LLM's add/update decision is keyed on the
|
||||
name it sees, so a collision usually reads as an intended update
|
||||
anyway.
|
||||
under the new sanitizer.
|
||||
`KnowledgeWriter` and the new `SkillPathMixin.skill_dir_name()`, instead
|
||||
of two independently maintained copies. `AgentSkillFrontmatter.name`
|
||||
also gained a validator rejecting path separators / `..` so a
|
||||
hand-edited `SKILL.md` is caught on read rather than silently
|
||||
relocated. No data migration: agent-skill extraction has never
|
||||
successfully produced a `SKILL.md` before this release (see the
|
||||
cascade-lag fix above), so there is no legacy skill corpus whose
|
||||
directory names would change under the new sanitizer.
|
||||
- **Reads now carry a deadline** (`count` / `get_by_id` / `find_where` /
|
||||
`find_where_paginated` / `search`). The write-side deadline work skipped them
|
||||
on the reasoning that a read takes no lock and so blocks no writer — true, but
|
||||
|
|
@ -134,8 +276,73 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
near the legitimate hold turns a slow migration into startup crashes for
|
||||
every waiting process.
|
||||
|
||||
### Added
|
||||
|
||||
- `OfflineEngine.trigger_manual` now returns
|
||||
`tuple[BaseEvent, list[tuple[StrategyMeta, str]]]` instead of `None`,
|
||||
enabling the `dispatched`/`runs` fields below.
|
||||
- `TriggerResponse` gains `dispatched: int` and `runs: list[RunSummary]`.
|
||||
- `OMEConfig` gains `retry_backoff_base_seconds`, `retry_backoff_cap_seconds`,
|
||||
and `retry_jitter_seconds` for the retry-loop sleep.
|
||||
- `AgentSkillReader.list_by_cluster()` enumerates the cluster's SKILL.md
|
||||
files from markdown (strong-consistency existence check).
|
||||
- **`[cascade]` settings section** — the four maintenance cadences
|
||||
(`optimize_heartbeat_seconds`, `optimize_prune_interval_seconds`,
|
||||
`optimize_prune_retention_seconds`, `optimize_rebuild_interval_seconds`) are
|
||||
now configurable. They were already constructor arguments on `CascadeWorker`,
|
||||
but `CascadeConfig` did not carry them and no production path passed one, so
|
||||
the defaults were unreachable — which is why the 12h rebuild sweep could not
|
||||
be exercised by any soak run shorter than half a day. The deadlines that
|
||||
bound a hung call are deliberately **not** exposed: they are hang-catchers
|
||||
sized from measured durations, where too low manufactures failures on a
|
||||
healthy table and too high leaves a wedged one invisible for longer. Note
|
||||
`optimize_prune_retention_seconds` has a second effect worth reading before
|
||||
tuning — it also decides how long index files keep a manifest naming them,
|
||||
and below LanceDB's 7-day unverified window they then wait out the full 7
|
||||
days.
|
||||
|
||||
### Changed
|
||||
|
||||
- **`extract_foresight` now ships disabled** (`enabled=False`). Not because
|
||||
it is broken — the crash below is fixed — but because it is one LLM call
|
||||
per sender per memcell whose output nothing in EverOS reads today: no
|
||||
search route surfaces foresights and no prompt slot consumes them. Until
|
||||
something does, running it by default spends tokens on write-only data.
|
||||
**Re-enable per install** in `ome.toml` (hot-reloaded, no restart):
|
||||
|
||||
```toml
|
||||
[strategies.extract_foresight]
|
||||
enabled = true
|
||||
```
|
||||
|
||||
Editing `default_ome.toml` alone would not have reached existing installs
|
||||
— `everos init` does not overwrite an existing `~/.everos/ome.toml` — so
|
||||
the code default is what changed.
|
||||
- **`extract_foresight` no longer crashes on a memcell containing tool
|
||||
calls.** The sender scan read `m.role` off every item, but only
|
||||
`ChatMessage` carries it (`ToolCallRequest` has `sender_id` without it,
|
||||
`ToolCallResult` has neither), so the first tool call raised
|
||||
`AttributeError` — before any sender was resolved. The strategy was
|
||||
correct on plain user chat and dead-lettered every time on agent
|
||||
trajectories. everalgo explicitly contracts for the mixed case
|
||||
(`user_memory/_render.chat_messages`: the caller need not pre-filter),
|
||||
and every other user-memory extractor gets that for free by delegating;
|
||||
this was the one place the filter was hand-rolled. The scan now tests
|
||||
`isinstance(m, ChatMessage)`, so a pure agent trajectory yields no senders
|
||||
and returns without an LLM call. Matters even with the strategy off by
|
||||
default: it is what makes the opt-in above actually usable.
|
||||
- **`SkillClusterUpdated` carries the case's 1024-dim embedding, growing the
|
||||
OME `run_record` table.** The event payload is persisted verbatim in
|
||||
`run_record.event_payload` (and in the APScheduler jobstore while a job is
|
||||
queued), so a `skill_cluster_updated` record goes from roughly 0.8 KB to
|
||||
14 KB. At the default `max_records_per_strategy = 1000` ring buffer that is
|
||||
~14 MB for this one strategy instead of ~0.8 MB. **Operators sizing
|
||||
`~/.everos/.index/sqlite/ome.db` should expect this.** The vector is only
|
||||
read when a cluster holds more skills than `MAX_SKILLS_IN_PROMPT`, so it
|
||||
usually rides along unused; trimming it from the persisted copy is not a
|
||||
local change, because crash recovery replays `event_payload` to rebuild the
|
||||
event and a trimmed payload would silently take the recovered run down a
|
||||
different branch than the original. Tracked as a follow-up.
|
||||
- **`cascade_lancedb_optimize_conflict` now records `pruned`** — which
|
||||
maintenance beat lost the commit race. Lance labels both beats' commit the
|
||||
same way (`This Rewrite transaction was preempted by concurrent transaction
|
||||
|
|
|
|||
26
docs/api.md
26
docs/api.md
|
|
@ -1070,8 +1070,32 @@ Manually trigger a registered OME strategy.
|
|||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
| `status` | `"ok" \| "timeout"` | Whether the strategy completed within the timeout |
|
||||
| `status` | `"ok" \| "timeout" \| "not_dispatched"` | `ok` = every dispatched run settled — a dead-lettered run still counts as settled (see `runs[*].error`); `timeout` = at least one run had not settled when `timeout` elapsed; `not_dispatched` = no strategy was dispatched (see below) |
|
||||
| `name` | `string` | Echoes the requested strategy name |
|
||||
| `dispatched` | `int` | Number of strategy routes enqueued. `0` iff `status == "not_dispatched"` |
|
||||
| `runs` | `list[RunSummary]` | One entry per strategy run *attempt*, not per dispatched route: `{run_id: string, status: string, error?: string}`. A strategy that retried before settling contributes multiple entries sharing one `event_id`. `status` is one of `running` / `success` / `failed` / `dead_letter` / `crashed`. Includes dead-lettered runs |
|
||||
|
||||
**`not_dispatched`** means every subscriber was rejected by one of the
|
||||
four dispatch gates (`_routes_to` / `enabled` / `applies_to` /
|
||||
`Counter`). The most common cause is forgetting `"force": true` on a
|
||||
strategy that is `enabled=false` in `ome.toml` — e.g. triggering
|
||||
`reflect_episodes` without `force` while it is disabled in config
|
||||
returns `{"status": "not_dispatched", "dispatched": 0, "runs": []}`
|
||||
instead of an error.
|
||||
|
||||
> `status: "ok"` means all dispatched strategy runs settled — including
|
||||
> runs that dead-lettered (their errors are in `runs[*].error`). It does
|
||||
> **not** mean the LanceDB index has caught up. Markdown is written
|
||||
> synchronously; the index syncs asynchronously (see
|
||||
> [Eventual consistency](#eventual-consistency)).
|
||||
>
|
||||
> If you need read-your-write semantics, poll `GET /health`'s
|
||||
> `cascade.pending` field until it reads `0` on two consecutive samples
|
||||
> (a single zero can be a false convergence — the watcher-input window
|
||||
> can briefly report an empty queue between md write and enqueue).
|
||||
|
||||
See [docs/openapi.json](openapi.json) for the exact generated schema
|
||||
(`TriggerResponse` / `RunSummary`) behind this table.
|
||||
|
||||
#### Errors
|
||||
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
"info": {
|
||||
"title": "everos",
|
||||
"description": "md-first memory extraction framework",
|
||||
"version": "1.2.2"
|
||||
"version": "1.2.3"
|
||||
},
|
||||
"paths": {
|
||||
"/health": {
|
||||
|
|
@ -222,7 +222,7 @@
|
|||
"ome"
|
||||
],
|
||||
"summary": "Trigger",
|
||||
"description": "Manually trigger a registered OME strategy and wait for completion.",
|
||||
"description": "Manually trigger a registered OME strategy and wait for its runs to\nsettle. Returns without waiting for the LanceDB index — the response\nreflects markdown state only; poll ``GET /health``'s ``cascade.pending``\nfor index convergence (two consecutive zero samples to guard against the\nwatcher-input window). See docs/api.md#eventual-consistency.",
|
||||
"operationId": "trigger_api_v1_ome_trigger_post",
|
||||
"requestBody": {
|
||||
"content": {
|
||||
|
|
@ -982,7 +982,7 @@
|
|||
"ome"
|
||||
],
|
||||
"summary": "Trigger",
|
||||
"description": "Manually trigger a registered OME strategy and wait for completion.",
|
||||
"description": "Manually trigger a registered OME strategy and wait for its runs to\nsettle. Returns without waiting for the LanceDB index — the response\nreflects markdown state only; poll ``GET /health``'s ``cascade.pending``\nfor index convergence (two consecutive zero samples to guard against the\nwatcher-input window). See docs/api.md#eventual-consistency.",
|
||||
"operationId": "trigger_api_v2_ome_trigger_post",
|
||||
"requestBody": {
|
||||
"content": {
|
||||
|
|
@ -3089,6 +3089,36 @@
|
|||
],
|
||||
"title": "MessageItemDTO"
|
||||
},
|
||||
"RunSummary": {
|
||||
"properties": {
|
||||
"run_id": {
|
||||
"type": "string",
|
||||
"title": "Run Id"
|
||||
},
|
||||
"status": {
|
||||
"type": "string",
|
||||
"title": "Status"
|
||||
},
|
||||
"error": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "string"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
],
|
||||
"title": "Error"
|
||||
}
|
||||
},
|
||||
"type": "object",
|
||||
"required": [
|
||||
"run_id",
|
||||
"status"
|
||||
],
|
||||
"title": "RunSummary",
|
||||
"description": "One strategy run within a trigger response."
|
||||
},
|
||||
"SearchAgentCaseItem": {
|
||||
"properties": {
|
||||
"id": {
|
||||
|
|
@ -4006,15 +4036,28 @@
|
|||
"name": {
|
||||
"type": "string",
|
||||
"title": "Name"
|
||||
},
|
||||
"dispatched": {
|
||||
"type": "integer",
|
||||
"title": "Dispatched"
|
||||
},
|
||||
"runs": {
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/RunSummary"
|
||||
},
|
||||
"type": "array",
|
||||
"title": "Runs",
|
||||
"default": []
|
||||
}
|
||||
},
|
||||
"type": "object",
|
||||
"required": [
|
||||
"status",
|
||||
"name"
|
||||
"name",
|
||||
"dispatched"
|
||||
],
|
||||
"title": "TriggerResponse",
|
||||
"description": "Response body for ``POST /api/v2/ome/trigger``."
|
||||
"description": "Response body for ``POST /api/v2/ome/trigger``.\n\n``status`` distinguishes three outcomes that were previously masked as\na single ``ok``:\n\n- ``ok``: at least one strategy was dispatched and all runs settled\n within ``timeout``. Individual run outcomes are in ``runs`` (a\n ``dead_letter`` there is still ``ok`` at this level — the strategy\n *ran*, it just failed permanently).\n- ``timeout``: at least one strategy was dispatched but the engine did\n not go idle within ``timeout``. Runs may be partially complete;\n poll ``GET /health`` for cascade convergence separately.\n- ``not_dispatched``: no strategy was dispatched — the subscriber was\n rejected by one of the dispatch gates (``_routes_to`` / ``enabled`` /\n ``applies_to`` / ``Counter``). Common cause: forgetting\n ``force=true`` on a strategy that is ``enabled=false`` in ome.toml."
|
||||
},
|
||||
"UnprocessedMessageDTO": {
|
||||
"properties": {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
[project]
|
||||
name = "everos"
|
||||
version = "1.2.2"
|
||||
version = "1.2.3"
|
||||
description = "EverOS — local-first markdown memory framework for AI agents and user chats; lightweight, dev-friendly, small-team"
|
||||
license = {text = "Apache-2.0"}
|
||||
readme = "README.md"
|
||||
|
|
|
|||
|
|
@ -29,8 +29,12 @@
|
|||
# Atomic fact extraction runs per memcell. Always enabled — search
|
||||
# depends on atomic facts for MaxSim retrieval.
|
||||
|
||||
# Foresight extraction (runs per memcell). Heavy LLM call — set
|
||||
# enabled = false to skip in evaluation / benchmark runs.
|
||||
# Foresight extraction (runs per memcell). Heavy LLM call.
|
||||
#
|
||||
# DISABLED BY DEFAULT since 1.2.3: nothing in EverOS reads foresights yet
|
||||
# — no search route surfaces them, no prompt slot consumes them — so
|
||||
# running it spends one LLM call per sender per memcell on write-only
|
||||
# data. Uncomment to opt in; it works on agent and chat input alike.
|
||||
# [strategies.extract_foresight]
|
||||
# enabled = true
|
||||
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@ External usage:
|
|||
# Frontmatter schema chassis
|
||||
BaseFrontmatter, UserScopedFrontmatter, AgentScopedFrontmatter,
|
||||
DailyLogPathMixin, SkillPathMixin,
|
||||
# Path safety
|
||||
sanitize_dirname,
|
||||
# Async SQLite (SQLModel / SA 2.0)
|
||||
create_system_engine, create_session_factory, session_scope,
|
||||
SQLModel, Field, Relationship, BaseTable, RepoBase,
|
||||
|
|
@ -49,6 +51,7 @@ from .markdown import find_entry as find_entry
|
|||
from .markdown import parse_frontmatter as parse_frontmatter
|
||||
from .markdown import parse_structured_entry as parse_structured_entry
|
||||
from .markdown import render_structured_entry as render_structured_entry
|
||||
from .markdown import sanitize_dirname as sanitize_dirname
|
||||
from .markdown import split_entries as split_entries
|
||||
from .memory_root import MemoryRoot as MemoryRoot
|
||||
from .memory_root import app_dir_name as app_dir_name
|
||||
|
|
@ -100,6 +103,7 @@ __all__ = [
|
|||
"project_dir_name",
|
||||
"project_id_from_dir",
|
||||
"render_structured_entry",
|
||||
"sanitize_dirname",
|
||||
"session_scope",
|
||||
"split_entries",
|
||||
"touch",
|
||||
|
|
|
|||
|
|
@ -21,6 +21,9 @@ External usage (frontmatter schema chassis):
|
|||
KnowledgeScopedMixin, KnowledgeDocumentPathMixin,
|
||||
KnowledgeTopicPathMixin,
|
||||
)
|
||||
|
||||
External usage (path safety):
|
||||
from everos.core.persistence.markdown import sanitize_dirname
|
||||
"""
|
||||
|
||||
from .entries import Entry as Entry
|
||||
|
|
@ -42,6 +45,7 @@ from .frontmatter import UserScopedFrontmatter as UserScopedFrontmatter
|
|||
from .frontmatter import dump_frontmatter as dump_frontmatter
|
||||
from .frontmatter import parse_frontmatter as parse_frontmatter
|
||||
from .parsed import ParsedMarkdown as ParsedMarkdown
|
||||
from .path_safety import sanitize_dirname as sanitize_dirname
|
||||
from .reader import MarkdownReader as MarkdownReader
|
||||
from .writer import MarkdownWriter as MarkdownWriter
|
||||
|
||||
|
|
@ -66,5 +70,6 @@ __all__ = [
|
|||
"parse_frontmatter",
|
||||
"parse_structured_entry",
|
||||
"render_structured_entry",
|
||||
"sanitize_dirname",
|
||||
"split_entries",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -36,6 +36,8 @@ from typing import Any, ClassVar, Literal
|
|||
import yaml
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
from .path_safety import sanitize_dirname
|
||||
|
||||
# ── YAML helpers ────────────────────────────────────────────────────────
|
||||
|
||||
_DELIM = "---"
|
||||
|
|
@ -229,6 +231,15 @@ class SkillPathMixin:
|
|||
SKILL_DIR_PREFIX: ClassVar[str] = "skill_"
|
||||
SKILL_MAIN_FILENAME: ClassVar[str] = "SKILL.md"
|
||||
...
|
||||
|
||||
``skill_dir_name`` / ``sanitize_skill_name`` are the single
|
||||
sanitization point both ``AgentSkillWriter`` and ``AgentSkillReader``
|
||||
derive their ``skill_<name>`` directory segment from, and that
|
||||
``memory.strategies.extract_agent_skill._persist_skill`` uses to
|
||||
sanitize LLM-emitted ``skill_name`` *before* constructing
|
||||
``AgentSkillFrontmatter`` — ``skill_name`` is LLM output and must not
|
||||
reach the filesystem, or the frontmatter's traversal validator,
|
||||
unsanitized (CWE-22).
|
||||
"""
|
||||
|
||||
SKILLS_CONTAINER_NAME: ClassVar[str]
|
||||
|
|
@ -244,6 +255,83 @@ class SkillPathMixin:
|
|||
f"{cls.SKILL_DIR_PREFIX}*/{cls.SKILL_MAIN_FILENAME}"
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def sanitize_skill_name(cls, skill_name: str) -> str:
|
||||
"""Bare sanitized skill name (no ``skill_`` prefix).
|
||||
|
||||
The single sanitization point for a skill's ``name`` value itself —
|
||||
as opposed to :meth:`skill_dir_name`, which additionally prefixes
|
||||
it for the directory segment. Callers building
|
||||
``AgentSkillFrontmatter.name`` from LLM output (see
|
||||
``memory.strategies.extract_agent_skill._persist_skill``) route
|
||||
through this *before* constructing the frontmatter, so
|
||||
``frontmatter.name`` ends up byte-identical to the directory-derived
|
||||
name rather than merely idempotent-if-resanitized.
|
||||
|
||||
This is lossy: distinct raw names can collapse onto the same
|
||||
sanitized name. Dropped punctuation, space/underscore collapse, and
|
||||
the 50-character cap are the visible cases (``"fix django"`` and
|
||||
``"fix_django"`` both become ``"fix_django"``; ``"fix!django"`` and
|
||||
``"fixdjango"`` both become ``"fixdjango"``). The larger case is
|
||||
every combining mark: a combining mark alone is not ``\\w``, so it
|
||||
is stripped regardless of script, and two names that differ only in
|
||||
their marks collide — e.g. Devanagari ``"किताब"`` and ``"कताब"``
|
||||
both sanitize to ``"कतब"``; the same holds for Thai tone marks,
|
||||
Hebrew niqqud, and Arabic harakat.
|
||||
|
||||
Case is *not* folded, which makes the collision above
|
||||
filesystem-dependent rather than universal, and is the dimension an
|
||||
LLM varies most freely: ``"Fix Django"`` → ``"Fix_Django"`` and
|
||||
``"fix django"`` → ``"fix_django"`` are two distinct sanitized
|
||||
names, so they are two rows in LanceDB (a case-sensitive Python
|
||||
string key) but one directory on a case-insensitive filesystem —
|
||||
macOS APFS and Windows NTFS in their default configurations. That
|
||||
splits the invariant this seam otherwise maintains: the surviving
|
||||
``SKILL.md`` carries one of the two names in its frontmatter while
|
||||
the index still advertises both, so a search hit on the shadowed
|
||||
name resolves to the other skill's content. On a case-sensitive
|
||||
filesystem the same pair simply stays two independent skills.
|
||||
|
||||
Because ``AgentSkillWriter.write_main`` is a full-file replace and
|
||||
the LanceDB primary key is ``f"{agent_id}_{sanitized_name}"``, a
|
||||
collision means the later skill silently overwrites the earlier
|
||||
one — its accumulated ``source_case_ids``, ``maturity_score``, and
|
||||
body are lost, not merged.
|
||||
|
||||
This is deliberate, not an oversight — but not because a collision
|
||||
"usually reads as an intended update". ``_persist_skill`` sanitizes
|
||||
*before* constructing the frontmatter, so the LLM is shown the
|
||||
already-sanitized name in ``existing_relevant_skills``; when it
|
||||
then emits a raw name like ``"fix django"`` after having just been
|
||||
shown ``"fix_django"``, it has affirmatively treated them as two
|
||||
different skills, and the write silently merges them anyway.
|
||||
|
||||
What justifies accepting it is narrower: the two alternatives are
|
||||
both worse here. Detecting a collision and raising would
|
||||
reintroduce the dead-letter DoS this sanitizer was built to avoid —
|
||||
LLM output would again decide whether a run survives. Appending a
|
||||
disambiguating suffix is the real candidate and is left for a
|
||||
deliberate design pass, not dismissed: it does *not* break the
|
||||
``frontmatter.name`` ≡ directory-suffix identity (writing
|
||||
``"fix_django_2"`` into both keeps that intact), but it does need a
|
||||
collision probe on a path that currently touches no other skill,
|
||||
and a rule for the case-insensitive-filesystem variant above where
|
||||
the probe must compare case-folded while the key stays exact.
|
||||
"""
|
||||
return sanitize_dirname(skill_name, fallback="unnamed")
|
||||
|
||||
@classmethod
|
||||
def skill_dir_name(cls, skill_name: str) -> str:
|
||||
"""Sanitized ``skill_<name>`` directory segment (traversal-safe).
|
||||
|
||||
Idempotent in ``skill_name``: calling this again on an already
|
||||
sanitized name (e.g. one recovered by walking the directory tree)
|
||||
returns the same segment, so a reader deriving ``skill_name`` from
|
||||
the on-disk directory and a writer deriving it from raw LLM output
|
||||
land on the same path.
|
||||
"""
|
||||
return f"{cls.SKILL_DIR_PREFIX}{cls.sanitize_skill_name(skill_name)}"
|
||||
|
||||
|
||||
class ProfilePathMixin:
|
||||
"""Path strategy for single-file profile markdown.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,83 @@
|
|||
"""``sanitize_dirname`` — the single path-safety primitive for md directory names.
|
||||
|
||||
Several markdown layouts turn free-text into a filesystem path segment:
|
||||
knowledge document/category titles, and agent-skill names. Both sources are
|
||||
untrusted in the same way — knowledge titles come from parsed source
|
||||
documents, skill names come straight from LLM output — so a name containing
|
||||
``../`` or a path separator must never survive into a directory segment
|
||||
(CWE-22 path traversal).
|
||||
|
||||
This module is the one place that decision is made. Callers that need a
|
||||
filesystem-safe segment from a free-text string route through
|
||||
:func:`sanitize_dirname` rather than keeping a private regex copy — see
|
||||
``writers/knowledge_writer.py`` and
|
||||
:meth:`SkillPathMixin.skill_dir_name() <.frontmatter.SkillPathMixin.skill_dir_name>`.
|
||||
Some callers (``knowledge_writer.py``) concatenate the result directly under a
|
||||
shared directory with no per-caller prefix, so the guarantee below has to hold
|
||||
on its own, without relying on a prefix like ``skill_`` to absorb a degenerate
|
||||
result.
|
||||
|
||||
``sanitize_dirname`` is idempotent (``sanitize_dirname(sanitize_dirname(x),
|
||||
fb) == sanitize_dirname(x, fb)``): a name built by re-sanitizing an
|
||||
already-sanitized segment (e.g. one derived by walking the directory tree)
|
||||
lands on the same string as sanitizing the original raw name. That property
|
||||
is what lets a reader and a writer agree on a path even when one side has
|
||||
only the raw name and the other only the on-disk directory name.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import unicodedata
|
||||
|
||||
_MAX_DIRNAME_LEN = 50
|
||||
_SAFE_CHARS = re.compile(r"[^\w\-.]", re.UNICODE)
|
||||
_DEGENERATE = frozenset({"", ".", ".."})
|
||||
|
||||
|
||||
def sanitize_dirname(raw: str, fallback: str) -> str:
|
||||
"""Produce a safe directory/file name segment from free-text input.
|
||||
|
||||
* NFC-normalize first. For an ordinary decomposed (NFD) input — a base
|
||||
letter plus a combining mark, e.g. ``"e"`` + combining acute accent —
|
||||
this collapses to the precomposed form before the character filter
|
||||
runs, so the accent survives (a combining mark alone is not ``\\w``
|
||||
and would otherwise be silently stripped). This is best-effort, not a
|
||||
guarantee: for the ~1,082 Unicode *composition exclusion* codepoints
|
||||
(e.g. Devanagari ``क़``/``ख़``, U+0958/U+0959), NFC does the
|
||||
opposite — it *decomposes* an already-precomposed exclusion
|
||||
character, because recomposing it is explicitly excluded from the
|
||||
NFC algorithm, and the resulting combining mark is then stripped just
|
||||
the same. Normalizing here improves fidelity for the common case; it
|
||||
does not make every Unicode script round-trip losslessly.
|
||||
* Replace spaces with underscores.
|
||||
* Strip characters outside ``[a-zA-Z0-9_\\-.]`` (``\\w`` is Unicode-aware,
|
||||
so CJK and other non-ASCII scripts survive readably). Note that ``.``
|
||||
is a *safe* character, not stripped — a run of literal dots is a legal
|
||||
result of this step.
|
||||
* Truncate to 50 characters.
|
||||
* Fall back to *fallback* if the result is empty, ``"."``, or ``".."``.
|
||||
|
||||
Every path separator (``/``, ``\\``) is stripped by the character-class
|
||||
filter, so no separator survives and the result is always exactly one
|
||||
path component — it can never be split into multiple segments by a
|
||||
downstream ``Path(...) / result``. The fallback on ``""`` / ``"."`` /
|
||||
``".."`` closes the remaining gap: those are the only single components
|
||||
that resolve to *no new child* (``""`` and ``"."`` both mean "this same
|
||||
directory", ``".."`` means "its parent") rather than a genuinely new
|
||||
entry. With both guarantees together, ``Path(some_dir) / sanitize_dirname(raw, fb)``
|
||||
can never escape ``some_dir`` and never silently collapses back onto it
|
||||
or its parent — unconditionally, including for a caller with no
|
||||
additional prefix (like ``skill_``) protecting the segment.
|
||||
|
||||
This function is lossy and not injective: distinct inputs can sanitize
|
||||
to the same output (dropped characters, space/underscore collapse, and
|
||||
truncation are all many-to-one). A caller that needs distinct outputs
|
||||
for distinct inputs must disambiguate before or after calling this —
|
||||
the function itself makes no such guarantee.
|
||||
"""
|
||||
slug = unicodedata.normalize("NFC", raw)
|
||||
slug = slug.replace(" ", "_")
|
||||
slug = _SAFE_CHARS.sub("", slug)
|
||||
slug = slug[:_MAX_DIRNAME_LEN]
|
||||
return slug if slug not in _DEGENERATE else fallback
|
||||
|
|
@ -2,12 +2,23 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from fastapi import APIRouter
|
||||
from pydantic import BaseModel
|
||||
|
||||
from everos.core.errors import NotFoundError
|
||||
from everos.core.observability.logging import get_logger
|
||||
|
||||
if TYPE_CHECKING:
|
||||
# Type-only — used solely to annotate `_summarize_runs`. Importing the
|
||||
# engine eagerly costs ~750ms (apscheduler + aiosqlite, 26 modules), so
|
||||
# keep it out of the runtime path even though `service.memorize` already
|
||||
# imports it eagerly and today's app startup pays that cost regardless:
|
||||
# this router's own `_get_engine` import is deliberately deferred (see
|
||||
# `trigger`), and an eager import here would contradict it.
|
||||
from everos.infra.ome.engine import OfflineEngine
|
||||
|
||||
router = APIRouter(prefix="/ome", tags=["ome"])
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
|
@ -21,27 +32,89 @@ class TriggerRequest(BaseModel):
|
|||
force: bool = False
|
||||
|
||||
|
||||
class RunSummary(BaseModel):
|
||||
"""One strategy run within a trigger response."""
|
||||
|
||||
run_id: str
|
||||
status: str
|
||||
"""One of: running / success / failed / dead_letter / crashed."""
|
||||
error: str | None = None
|
||||
|
||||
|
||||
class TriggerResponse(BaseModel):
|
||||
"""Response body for ``POST /api/v2/ome/trigger``."""
|
||||
"""Response body for ``POST /api/v2/ome/trigger``.
|
||||
|
||||
``status`` distinguishes three outcomes that were previously masked as
|
||||
a single ``ok``:
|
||||
|
||||
- ``ok``: at least one strategy was dispatched and all runs settled
|
||||
within ``timeout``. Individual run outcomes are in ``runs`` (a
|
||||
``dead_letter`` there is still ``ok`` at this level — the strategy
|
||||
*ran*, it just failed permanently).
|
||||
- ``timeout``: at least one strategy was dispatched but the engine did
|
||||
not go idle within ``timeout``. Runs may be partially complete;
|
||||
poll ``GET /health`` for cascade convergence separately.
|
||||
- ``not_dispatched``: no strategy was dispatched — the subscriber was
|
||||
rejected by one of the dispatch gates (``_routes_to`` / ``enabled`` /
|
||||
``applies_to`` / ``Counter``). Common cause: forgetting
|
||||
``force=true`` on a strategy that is ``enabled=false`` in ome.toml.
|
||||
"""
|
||||
|
||||
status: str
|
||||
"""One of: ok / timeout / not_dispatched."""
|
||||
name: str
|
||||
dispatched: int
|
||||
"""Number of strategy routes that were enqueued. ``0`` iff status is
|
||||
``not_dispatched``."""
|
||||
runs: list[RunSummary] = []
|
||||
"""One entry per strategy run *attempt*, not per dispatched route: a
|
||||
strategy that retried before settling contributes multiple entries
|
||||
sharing one ``event_id``. Includes dead-lettered runs whose errors
|
||||
would otherwise be invisible to the caller (they live in the SQLite
|
||||
``run_record`` table with no HTTP surface until this field was
|
||||
added)."""
|
||||
|
||||
|
||||
@router.post("/trigger", response_model=TriggerResponse)
|
||||
async def trigger(req: TriggerRequest) -> TriggerResponse:
|
||||
"""Manually trigger a registered OME strategy and wait for completion."""
|
||||
"""Manually trigger a registered OME strategy and wait for its runs to
|
||||
settle. Returns without waiting for the LanceDB index — the response
|
||||
reflects markdown state only; poll ``GET /health``'s ``cascade.pending``
|
||||
for index convergence (two consecutive zero samples to guard against the
|
||||
watcher-input window). See docs/api.md#eventual-consistency.
|
||||
"""
|
||||
# Deferred: avoid importing heavy OME engine at module level.
|
||||
from everos.service.memorize import _get_engine
|
||||
|
||||
engine = _get_engine()
|
||||
try:
|
||||
await engine.trigger_manual(req.name, force=req.force)
|
||||
event, routes = await engine.trigger_manual(req.name, force=req.force)
|
||||
except KeyError:
|
||||
raise NotFoundError(f"strategy '{req.name}' not found") from None
|
||||
logger.info("ome_trigger_manual", strategy=req.name)
|
||||
|
||||
if not routes:
|
||||
logger.info("ome_trigger_manual_not_dispatched", strategy=req.name)
|
||||
return TriggerResponse(
|
||||
status="not_dispatched", name=req.name, dispatched=0, runs=[]
|
||||
)
|
||||
|
||||
logger.info("ome_trigger_manual", strategy=req.name, dispatched=len(routes))
|
||||
idle = await engine.wait_idle(timeout=req.timeout)
|
||||
runs = await _summarize_runs(engine, event.event_id)
|
||||
if not idle:
|
||||
logger.warning("ome_trigger_timeout", strategy=req.name, timeout=req.timeout)
|
||||
return TriggerResponse(status="timeout", name=req.name)
|
||||
return TriggerResponse(status="ok", name=req.name)
|
||||
return TriggerResponse(
|
||||
status="timeout", name=req.name, dispatched=len(routes), runs=runs
|
||||
)
|
||||
return TriggerResponse(
|
||||
status="ok", name=req.name, dispatched=len(routes), runs=runs
|
||||
)
|
||||
|
||||
|
||||
async def _summarize_runs(engine: OfflineEngine, event_id: str) -> list[RunSummary]:
|
||||
"""Fetch and shape run records for one event into the response DTO."""
|
||||
records = await engine.list_runs_by_event_id(event_id)
|
||||
return [
|
||||
RunSummary(run_id=r.run_id, status=r.status.value, error=r.error)
|
||||
for r in records
|
||||
]
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ therefore be safe to re-execute with the same payload.
|
|||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import random
|
||||
import traceback
|
||||
from collections.abc import Awaitable, Callable
|
||||
from typing import TYPE_CHECKING
|
||||
|
|
@ -32,6 +33,7 @@ from everos.core.observability.logging import get_logger
|
|||
from everos.core.observability.tracing import memory_span, use_traceparent
|
||||
from everos.infra.ome._dispatch._state import _CURRENT_STRATEGY
|
||||
from everos.infra.ome._stores.run_record import RunRecordStore
|
||||
from everos.infra.ome.config import OMEConfig
|
||||
from everos.infra.ome.decorator import StrategyMeta
|
||||
from everos.infra.ome.events import BaseEvent
|
||||
from everos.infra.ome.exceptions import EmitNotDeclaredError, StrategyContractError
|
||||
|
|
@ -98,12 +100,14 @@ class Runner:
|
|||
run_record_store: RunRecordStore,
|
||||
engine_sem: asyncio.Semaphore,
|
||||
emit_hook: Callable[[BaseEvent], Awaitable[None]],
|
||||
config: OMEConfig,
|
||||
on_dead_letter: Callable[[RunRecord], None] | None = None,
|
||||
engine: OfflineEngine,
|
||||
) -> None:
|
||||
self._rec = run_record_store
|
||||
self._sem = engine_sem
|
||||
self._emit_hook = emit_hook
|
||||
self._config = config
|
||||
self._on_dead_letter = on_dead_letter
|
||||
self._engine = engine
|
||||
|
||||
|
|
@ -118,23 +122,34 @@ class Runner:
|
|||
) -> None:
|
||||
"""Execute ``meta.func(event, ctx)`` with the attempt retry loop.
|
||||
|
||||
Holds ``engine_sem`` for the full retry chain so concurrency cap
|
||||
applies end-to-end. Each attempt gets a fresh ``run_id`` after
|
||||
the first, so the run history records every try.
|
||||
``engine_sem`` is held per *attempt*, not across the whole retry
|
||||
chain: the backoff sleep happens outside it. The cap exists to
|
||||
bound concurrent strategy work — LLM calls, embeddings, storage
|
||||
IO — and a coroutine sleeping between attempts consumes none of
|
||||
that. Holding the slot across the sleep turned a partial outage
|
||||
into a total stall: with ``max_concurrent_runs`` slots and a
|
||||
``1s → 2s → 4s`` backoff, enough simultaneously-failing runs
|
||||
park every slot in ``asyncio.sleep`` and starve strategies that
|
||||
would have succeeded. Backpressure on the failing work is
|
||||
wanted; backpressure on everything else is not.
|
||||
|
||||
Each attempt gets a fresh ``run_id`` after the first, so the run
|
||||
history records every try.
|
||||
"""
|
||||
if max_retries_snapshot < 0:
|
||||
raise ValueError(
|
||||
f"max_retries_snapshot must be >= 0, got {max_retries_snapshot}"
|
||||
)
|
||||
|
||||
async with self._sem:
|
||||
event_topic = type(event).topic()
|
||||
event_payload = event.model_dump_json()
|
||||
current_run_id = run_id
|
||||
event_topic = type(event).topic()
|
||||
event_payload = event.model_dump_json()
|
||||
current_run_id = run_id
|
||||
|
||||
for attempt in range(max_retries_snapshot + 1):
|
||||
if attempt > 0:
|
||||
current_run_id = uuid4().hex
|
||||
for attempt in range(max_retries_snapshot + 1):
|
||||
if attempt > 0:
|
||||
await self._sleep_backoff(attempt)
|
||||
current_run_id = uuid4().hex
|
||||
async with self._sem:
|
||||
terminated = await self._run_one_attempt(
|
||||
meta=meta,
|
||||
event=event,
|
||||
|
|
@ -145,8 +160,25 @@ class Runner:
|
|||
max_retries_snapshot=max_retries_snapshot,
|
||||
traceparent=traceparent,
|
||||
)
|
||||
if terminated:
|
||||
return
|
||||
if terminated:
|
||||
return
|
||||
|
||||
async def _sleep_backoff(self, attempt: int) -> None:
|
||||
"""Sleep before retry ``attempt`` (1-indexed): ``base * 2**(attempt-1)``,
|
||||
capped at ``retry_backoff_cap_seconds``, plus up to
|
||||
``retry_jitter_seconds`` of uniform jitter. ``retry_backoff_base_seconds
|
||||
== 0.0`` disables backoff entirely (used by tests that don't
|
||||
monkeypatch ``asyncio.sleep``).
|
||||
"""
|
||||
base = self._config.retry_backoff_base_seconds
|
||||
if base <= 0.0:
|
||||
return
|
||||
cap = self._config.retry_backoff_cap_seconds
|
||||
jitter_max = self._config.retry_jitter_seconds
|
||||
sleep_seconds = min(base * (2 ** (attempt - 1)), cap)
|
||||
if jitter_max > 0.0:
|
||||
sleep_seconds += random.uniform(0.0, jitter_max)
|
||||
await asyncio.sleep(sleep_seconds)
|
||||
|
||||
async def _run_one_attempt(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -109,6 +109,35 @@ class OMEConfig(BaseModel):
|
|||
"0 disables retries.",
|
||||
),
|
||||
] = 1
|
||||
retry_backoff_base_seconds: Annotated[
|
||||
float,
|
||||
Field(
|
||||
ge=0.0,
|
||||
description=(
|
||||
"Base seconds for exponential retry backoff (sleep between "
|
||||
"attempts). attempt N waits base * 2**(N-1), capped at "
|
||||
"retry_backoff_cap_seconds, plus up to retry_jitter_seconds "
|
||||
"of random jitter. 0.0 disables backoff."
|
||||
),
|
||||
),
|
||||
] = 1.0
|
||||
retry_backoff_cap_seconds: Annotated[
|
||||
float,
|
||||
Field(
|
||||
ge=0.0,
|
||||
description="Upper bound on the exponential backoff sleep before jitter.",
|
||||
),
|
||||
] = 10.0
|
||||
retry_jitter_seconds: Annotated[
|
||||
float,
|
||||
Field(
|
||||
ge=0.0,
|
||||
description=(
|
||||
"Uniform [0, retry_jitter_seconds] noise added to each "
|
||||
"backoff sleep to spread retry storms."
|
||||
),
|
||||
),
|
||||
] = 0.5
|
||||
max_records_per_strategy: Annotated[
|
||||
int,
|
||||
Field(
|
||||
|
|
|
|||
|
|
@ -319,6 +319,7 @@ class OfflineEngine:
|
|||
run_record_store=self._run_record_store,
|
||||
engine_sem=self._engine_sem,
|
||||
emit_hook=self._dispatch_event,
|
||||
config=self._config,
|
||||
on_dead_letter=self._on_dead_letter,
|
||||
engine=self,
|
||||
)
|
||||
|
|
@ -614,7 +615,7 @@ class OfflineEngine:
|
|||
*,
|
||||
event: BaseEvent | None = None,
|
||||
force: bool = False,
|
||||
) -> None:
|
||||
) -> tuple[BaseEvent, list[tuple[StrategyMeta, str]]]:
|
||||
"""Manually trigger one strategy.
|
||||
|
||||
- ``event=None`` → engine self-emits ``ManualTick(strategy_name=name)``
|
||||
|
|
@ -624,6 +625,14 @@ class OfflineEngine:
|
|||
Routes through :meth:`EventDispatcher.dispatch` with
|
||||
``strategy_filter=name`` so the same three-gate logic is applied
|
||||
as for engine-driven dispatch.
|
||||
|
||||
Returns:
|
||||
Tuple of ``(event, routes)``:
|
||||
- ``event``: the event that was dispatched (either supplied
|
||||
or the engine-generated ``ManualTick``).
|
||||
- ``routes``: the ``(meta, run_id)`` pairs that were
|
||||
enqueued. Empty list when every dispatch gate rejected
|
||||
the strategy.
|
||||
"""
|
||||
if not self._started:
|
||||
raise OMEError("trigger_manual: engine not started")
|
||||
|
|
@ -636,6 +645,7 @@ class OfflineEngine:
|
|||
)
|
||||
for meta, run_id in routes:
|
||||
self._enqueue_run(meta, event, run_id)
|
||||
return event, routes
|
||||
|
||||
def _enqueue_run(self, meta: StrategyMeta, event: BaseEvent, run_id: str) -> None:
|
||||
"""Add a one-shot APScheduler job that hands the event to Runner.
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ class StrategyTestHarness:
|
|||
config_watch=False,
|
||||
max_concurrent_runs=20,
|
||||
max_retries=1,
|
||||
retry_backoff_base_seconds=0.0,
|
||||
)
|
||||
self._engine = OfflineEngine(config=cfg)
|
||||
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ from __future__ import annotations
|
|||
import datetime as _dt
|
||||
from typing import ClassVar, Literal
|
||||
|
||||
from pydantic import field_validator
|
||||
|
||||
from everos.core.persistence.markdown import (
|
||||
AgentScopedFrontmatter,
|
||||
SkillPathMixin,
|
||||
|
|
@ -38,8 +40,44 @@ class AgentSkillFrontmatter(SkillPathMixin, AgentScopedFrontmatter):
|
|||
|
||||
name: str
|
||||
"""Skill identifier — also the directory suffix
|
||||
(``skills/skill_<name>/``). Keep snake_case so it is filesystem-safe
|
||||
and ID-stable."""
|
||||
(``skills/skill_<name>/``, sanitized via
|
||||
:meth:`SkillPathMixin.skill_dir_name`). Keep snake_case so it stays
|
||||
readable and ID-stable; the directory segment is sanitized regardless."""
|
||||
|
||||
@field_validator("name")
|
||||
@classmethod
|
||||
def _reject_path_traversal(cls, value: str) -> str:
|
||||
"""Catch a frontmatter ``name`` that bypassed the writer's sanitizer.
|
||||
|
||||
The normal write path
|
||||
(``memory.strategies.extract_agent_skill._persist_skill``) sanitizes
|
||||
LLM-emitted ``skill_name`` via
|
||||
:meth:`SkillPathMixin.sanitize_skill_name` *before* constructing this
|
||||
model, so ``name`` is traversal-free by the time it gets here on that
|
||||
path — this validator should not normally fire for LLM output at
|
||||
all. It exists for the case that does bypass the writer: a
|
||||
hand-edited ``SKILL.md`` (or any other direct
|
||||
``AgentSkillFrontmatter`` construction that skips pre-sanitization)
|
||||
whose ``name`` contains a path separator, or is exactly ``".."`` —
|
||||
raise loudly rather than silently relocating the skill on next
|
||||
write.
|
||||
|
||||
The check is deliberately narrower than "contains ``..``": a
|
||||
sanitized name may legitimately contain a run of literal dots
|
||||
(``sanitize_dirname`` keeps ``.`` as a safe character, so
|
||||
``"../" * 8 + "tmp/pwned"`` sanitizes to
|
||||
``"................tmppwned"``, which still contains the substring
|
||||
``".."`` many times over). With no path separator left, that string
|
||||
is one opaque filename component, not a ``..`` traversal segment —
|
||||
rejecting on substring containment would make this validator
|
||||
reject the sanitizer's own safe output.
|
||||
"""
|
||||
if "/" in value or "\\" in value or value == "..":
|
||||
raise ValueError(
|
||||
f"skill name {value!r} must not contain path separators, "
|
||||
"and must not be exactly '..'"
|
||||
)
|
||||
return value
|
||||
|
||||
description: str
|
||||
"""One-line summary surfaced at Tier-1 prompt injection. Short — the
|
||||
|
|
|
|||
|
|
@ -5,16 +5,44 @@ Pairs with :class:`AgentSkillWriter`:
|
|||
- :meth:`read_main` reads ``SKILL.md`` and returns the caller's
|
||||
:class:`AgentSkillFrontmatter` subclass instance + the Tier-2 body, so
|
||||
the caller never deals with raw dicts.
|
||||
- :meth:`list_by_cluster` walks every ``skill_*/SKILL.md`` under an agent
|
||||
and returns ``(frontmatter, body)`` for the ones whose parsed
|
||||
``cluster_id`` matches. This is the strong-consistency source of truth
|
||||
for cluster membership — LanceDB is cascade-lagged and must not be used
|
||||
for that check.
|
||||
- :meth:`read_reference` / :meth:`read_script` are plain text reads;
|
||||
no frontmatter, no schema.
|
||||
|
||||
All three return ``None`` when the target is missing — readers do not
|
||||
raise on absence, since "skill not yet created" is a normal state for
|
||||
the upsert-style workflow. Callers that need to distinguish "missing"
|
||||
from "empty body" check for ``None`` explicitly.
|
||||
``read_main``, ``read_reference``, and ``read_script`` return ``None`` when
|
||||
the target is missing — readers do not raise on absence, since "skill not
|
||||
yet created" is a normal state for the upsert-style workflow. Callers that
|
||||
need to distinguish "missing" from "empty body" check for ``None``
|
||||
explicitly.
|
||||
|
||||
``reference_name`` / ``script_filename`` are appended after the skill
|
||||
directory, so ``skill_dir_name`` does not cover them; both go through
|
||||
:func:`sanitize_dirname` here exactly as :class:`AgentSkillWriter` does.
|
||||
The two sides must agree on *every* segment — sanitizing one side only
|
||||
would route a write and its matching read to different paths.
|
||||
|
||||
Path resolution mirrors :class:`AgentSkillWriter` and reads the same
|
||||
ClassVars off :class:`AgentSkillFrontmatter`.
|
||||
ClassVars off :class:`AgentSkillFrontmatter`, including
|
||||
:meth:`AgentSkillFrontmatter.skill_dir_name` for the traversal-safe
|
||||
directory segment. ``read_main`` / ``read_reference`` / ``read_script``
|
||||
take a caller-supplied ``skill_name`` and re-derive the path from it, so
|
||||
the reader and writer must never diverge on how a ``skill_name`` maps to
|
||||
a directory. ``list_by_cluster`` never derives a path at all: it reads
|
||||
each globbed ``SKILL.md`` path directly and hands back the body it
|
||||
already read, rather than recovering a name from the directory and
|
||||
leaving the caller to re-derive a path from that name for a second read.
|
||||
A caller that discarded the body and re-read by name would recreate
|
||||
exactly the re-sanitization risk this method exists to avoid — a
|
||||
directory whose suffix isn't itself a sanitizer fixpoint (e.g. one
|
||||
containing a raw space) would silently miss on that second, name-based
|
||||
read even though the first, path-based read found it just fine. Returning
|
||||
the body is what makes "the reader never derives a path" a property of
|
||||
the full ``list_by_cluster`` → caller flow, not just of the enumeration
|
||||
step in isolation.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -23,13 +51,17 @@ from pathlib import Path
|
|||
from typing import TypeVar
|
||||
|
||||
import anyio
|
||||
from pydantic import ValidationError
|
||||
|
||||
from everos.core.persistence import MarkdownReader, MemoryRoot
|
||||
from everos.core.observability.logging import get_logger
|
||||
from everos.core.persistence import MarkdownReader, MemoryRoot, sanitize_dirname
|
||||
|
||||
from ..mds import AgentSkillFrontmatter
|
||||
|
||||
T = TypeVar("T", bound=AgentSkillFrontmatter)
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
|
||||
class AgentSkillReader:
|
||||
"""Single-skill reader for the directory + progressive-disclosure layout."""
|
||||
|
|
@ -63,12 +95,88 @@ class AgentSkillReader:
|
|||
is stripped to give the *logical* body back.
|
||||
"""
|
||||
path = self._main_path(agent_id, skill_name, app_id, project_id)
|
||||
if not await anyio.Path(path).is_file():
|
||||
return None
|
||||
parsed = await MarkdownReader.read(path)
|
||||
frontmatter = schema.model_validate(parsed.frontmatter)
|
||||
body = parsed.body.rstrip("\n")
|
||||
return frontmatter, body
|
||||
return await self._read_path(path, schema=schema)
|
||||
|
||||
async def list_by_cluster(
|
||||
self,
|
||||
agent_id: str,
|
||||
cluster_id: str,
|
||||
*,
|
||||
app_id: str = "default",
|
||||
project_id: str = "default",
|
||||
) -> list[tuple[AgentSkillFrontmatter, str]]:
|
||||
"""Enumerate this agent's ``SKILL.md`` files whose ``cluster_id`` matches.
|
||||
|
||||
Walks ``skills/skill_*/SKILL.md`` under the agent's memory root and
|
||||
returns ``(frontmatter, body)`` for each match. Skills whose
|
||||
frontmatter has ``cluster_id is None`` (or a different cluster) are
|
||||
filtered out.
|
||||
|
||||
This is the strong-consistency source of truth for "which skills
|
||||
belong to this cluster" — LanceDB is cascade-lagged and must not be
|
||||
used for existence checks. Each glob match is read directly by its
|
||||
already-resolved ``path`` (see :meth:`_read_path`), *not* by
|
||||
recovering a ``skill_name`` from the directory and calling
|
||||
:meth:`read_main` to re-derive the same path — the reader never
|
||||
derives a path at all on this route, so this enumeration cannot drop
|
||||
a skill whose on-disk directory suffix is not itself a sanitizer
|
||||
fixpoint (e.g. one written with a raw, unsanitized name containing a
|
||||
space). Returning the body here (rather than frontmatter alone) is
|
||||
load-bearing, not a convenience: a caller that discarded it and
|
||||
re-read by ``frontmatter.name`` would reintroduce the same
|
||||
name-based re-derivation this method exists to avoid, one call
|
||||
later.
|
||||
|
||||
A ``SKILL.md`` whose frontmatter fails schema validation is logged
|
||||
and skipped, not propagated. Isolating it matters because the
|
||||
blast radius of propagating is the whole cluster, not the one
|
||||
file: :meth:`_read_path` validates the full
|
||||
:class:`AgentSkillFrontmatter` schema, so *any* constraint can
|
||||
raise — a hand-edited ``name``, but equally a field that a later
|
||||
schema revision made required and existing files therefore lack.
|
||||
A single bad file would otherwise abort the enumeration, starve
|
||||
``extract_agent_skill`` of every existing skill in the cluster,
|
||||
and dead-letter that cluster's extraction on every subsequent
|
||||
run — the exact permanent-failure mode this md-first read path
|
||||
exists to eliminate.
|
||||
|
||||
Args:
|
||||
agent_id: Owning agent.
|
||||
cluster_id: Cluster to filter on.
|
||||
app_id: App scope; defaults to ``"default"``.
|
||||
project_id: Project scope; defaults to ``"default"``.
|
||||
|
||||
Returns:
|
||||
``(frontmatter, body)`` pairs sorted by skill directory path.
|
||||
Empty if the agent has no skill directory yet, none match, or
|
||||
every candidate failed validation.
|
||||
"""
|
||||
skills_dir = self._skills_root(agent_id, app_id, project_id)
|
||||
if not await anyio.Path(skills_dir).is_dir():
|
||||
return []
|
||||
pattern = (
|
||||
f"{AgentSkillFrontmatter.SKILL_DIR_PREFIX}*"
|
||||
f"/{AgentSkillFrontmatter.SKILL_MAIN_FILENAME}"
|
||||
)
|
||||
paths = await anyio.to_thread.run_sync(lambda: sorted(skills_dir.glob(pattern)))
|
||||
matches: list[tuple[AgentSkillFrontmatter, str]] = []
|
||||
for path in paths:
|
||||
try:
|
||||
parsed = await self._read_path(path, schema=AgentSkillFrontmatter)
|
||||
except ValidationError as exc:
|
||||
logger.warning(
|
||||
"agent_skill.list_by_cluster.unparseable_skill_skipped",
|
||||
path=str(path),
|
||||
cluster_id=cluster_id,
|
||||
error=str(exc),
|
||||
)
|
||||
continue
|
||||
if parsed is None:
|
||||
continue
|
||||
frontmatter, body = parsed
|
||||
if frontmatter.cluster_id == cluster_id:
|
||||
matches.append((frontmatter, body))
|
||||
return matches
|
||||
|
||||
async def read_reference(
|
||||
self,
|
||||
|
|
@ -114,16 +222,41 @@ class AgentSkillReader:
|
|||
|
||||
# ── Internals — same shape as AgentSkillWriter ────────────────────────────
|
||||
|
||||
def _skill_dir(
|
||||
self, agent_id: str, skill_name: str, app_id: str, project_id: str
|
||||
) -> Path:
|
||||
async def _read_path(self, path: Path, *, schema: type[T]) -> tuple[T, str] | None:
|
||||
"""Read + parse an already-resolved ``SKILL.md`` path.
|
||||
|
||||
Shared by :meth:`read_main` (path derived from a caller-supplied
|
||||
``skill_name``) and :meth:`list_by_cluster` (path taken directly
|
||||
from a directory glob, never re-derived from a name).
|
||||
|
||||
Raises:
|
||||
ValidationError: the file's frontmatter violates *schema*.
|
||||
Propagated to the caller — ``read_main`` asked for one
|
||||
specific skill and a corrupt answer is not a substitute,
|
||||
while ``list_by_cluster`` catches it per file so one bad
|
||||
file cannot starve the rest of the cluster.
|
||||
"""
|
||||
if not await anyio.Path(path).is_file():
|
||||
return None
|
||||
parsed = await MarkdownReader.read(path)
|
||||
frontmatter = schema.model_validate(parsed.frontmatter)
|
||||
body = parsed.body.rstrip("\n")
|
||||
return frontmatter, body
|
||||
|
||||
def _skills_root(self, agent_id: str, app_id: str, project_id: str) -> Path:
|
||||
return (
|
||||
self._root.agents_dir(app_id, project_id)
|
||||
/ agent_id
|
||||
/ AgentSkillFrontmatter.SKILLS_CONTAINER_NAME
|
||||
/ f"{AgentSkillFrontmatter.SKILL_DIR_PREFIX}{skill_name}"
|
||||
)
|
||||
|
||||
def _skill_dir(
|
||||
self, agent_id: str, skill_name: str, app_id: str, project_id: str
|
||||
) -> Path:
|
||||
return self._skills_root(
|
||||
agent_id, app_id, project_id
|
||||
) / AgentSkillFrontmatter.skill_dir_name(skill_name)
|
||||
|
||||
def _main_path(
|
||||
self, agent_id: str, skill_name: str, app_id: str, project_id: str
|
||||
) -> Path:
|
||||
|
|
@ -143,7 +276,7 @@ class AgentSkillReader:
|
|||
return (
|
||||
self._skill_dir(agent_id, skill_name, app_id, project_id)
|
||||
/ AgentSkillFrontmatter.SKILL_REFERENCES_DIR_NAME
|
||||
/ f"{reference_name}.md"
|
||||
/ f"{sanitize_dirname(reference_name, 'reference')}.md"
|
||||
)
|
||||
|
||||
def _script_path(
|
||||
|
|
@ -157,5 +290,5 @@ class AgentSkillReader:
|
|||
return (
|
||||
self._skill_dir(agent_id, skill_name, app_id, project_id)
|
||||
/ AgentSkillFrontmatter.SKILL_SCRIPTS_DIR_NAME
|
||||
/ script_filename
|
||||
/ sanitize_dirname(script_filename, "script")
|
||||
)
|
||||
|
|
|
|||
|
|
@ -22,14 +22,30 @@ This writer is intentionally distinct from :class:`BaseDailyWriter`:
|
|||
Path resolution comes from :class:`MemoryRoot` + the ClassVars on
|
||||
:class:`AgentSkillFrontmatter` (``SKILLS_CONTAINER_NAME`` /
|
||||
``SKILL_DIR_PREFIX`` / etc.). The writer + reader pair is the single
|
||||
addressing API for skills.
|
||||
addressing API for skills. ``skill_name`` is LLM output (see
|
||||
``memory.strategies.extract_agent_skill``), so the directory segment is
|
||||
built via :meth:`AgentSkillFrontmatter.skill_dir_name` — the shared,
|
||||
traversal-safe path-safety point both this writer and
|
||||
:class:`AgentSkillReader` derive from.
|
||||
|
||||
``reference_name`` and ``script_filename`` are appended *after* that
|
||||
segment, so ``skill_dir_name`` does not cover them; both route through
|
||||
:func:`sanitize_dirname` separately. Nothing in ``src/`` calls those two
|
||||
methods yet, but they are public API and their inputs will come from the
|
||||
same untrusted place the skill name does once progressive disclosure is
|
||||
wired up. :class:`AgentSkillReader` sanitizes them identically — the two
|
||||
sides must agree on every segment, not just the skill directory, or a
|
||||
write and its matching read resolve to different paths.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
from everos.core.persistence import MarkdownWriter, MemoryRoot
|
||||
import anyio
|
||||
|
||||
from everos.core.persistence import MarkdownWriter, MemoryRoot, sanitize_dirname
|
||||
|
||||
from ..mds import AgentSkillFrontmatter
|
||||
|
||||
|
|
@ -135,6 +151,40 @@ class AgentSkillWriter:
|
|||
)
|
||||
return await self._writer.write(path, _ensure_trailing_newline(content))
|
||||
|
||||
async def delete_skill(
|
||||
self,
|
||||
agent_id: str,
|
||||
skill_name: str,
|
||||
*,
|
||||
app_id: str = "default",
|
||||
project_id: str = "default",
|
||||
) -> bool:
|
||||
"""Remove ``skills/skill_<name>/`` and everything under it.
|
||||
|
||||
The one destructive operation on this writer. It exists for
|
||||
reconciliation, not for expiry: when an update renames a skill,
|
||||
the new name is written to a new directory and the old one has
|
||||
to go, or it survives as a duplicate that
|
||||
:meth:`AgentSkillReader.list_by_cluster` keeps feeding back into
|
||||
the next extraction's prompt.
|
||||
|
||||
Returns:
|
||||
``True`` if the directory existed and was removed, ``False``
|
||||
if it was already absent. Absence is not an error — the
|
||||
caller reconciles against markdown it enumerated earlier, so
|
||||
a concurrent delete is a benign race, and a directory whose
|
||||
on-disk name is not a fixpoint of
|
||||
:meth:`AgentSkillFrontmatter.skill_dir_name` simply is not
|
||||
found. Failing closed here (leaving an orphan) is the safe
|
||||
direction; the destructive alternative would be resolving the
|
||||
target by anything looser than the writer's own path rule.
|
||||
"""
|
||||
skill_dir = self._skill_dir(agent_id, skill_name, app_id, project_id)
|
||||
if not await anyio.Path(skill_dir).is_dir():
|
||||
return False
|
||||
await anyio.to_thread.run_sync(lambda: shutil.rmtree(skill_dir))
|
||||
return True
|
||||
|
||||
# ── Path API (callers that need to echo paths in responses) ──────────
|
||||
|
||||
def main_path(
|
||||
|
|
@ -157,7 +207,7 @@ class AgentSkillWriter:
|
|||
self._root.agents_dir(app_id, project_id)
|
||||
/ agent_id
|
||||
/ AgentSkillFrontmatter.SKILLS_CONTAINER_NAME
|
||||
/ f"{AgentSkillFrontmatter.SKILL_DIR_PREFIX}{skill_name}"
|
||||
/ AgentSkillFrontmatter.skill_dir_name(skill_name)
|
||||
)
|
||||
|
||||
def _main_path(
|
||||
|
|
@ -179,7 +229,7 @@ class AgentSkillWriter:
|
|||
return (
|
||||
self._skill_dir(agent_id, skill_name, app_id, project_id)
|
||||
/ AgentSkillFrontmatter.SKILL_REFERENCES_DIR_NAME
|
||||
/ f"{reference_name}.md"
|
||||
/ f"{sanitize_dirname(reference_name, 'reference')}.md"
|
||||
)
|
||||
|
||||
def _script_path(
|
||||
|
|
@ -193,7 +243,7 @@ class AgentSkillWriter:
|
|||
return (
|
||||
self._skill_dir(agent_id, skill_name, app_id, project_id)
|
||||
/ AgentSkillFrontmatter.SKILL_SCRIPTS_DIR_NAME
|
||||
/ script_filename
|
||||
/ sanitize_dirname(script_filename, "script")
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -15,11 +15,18 @@ path rather than binding to :class:`MemoryRoot`. The service layer
|
|||
resolves ``knowledge_dir`` from ``MemoryRoot.knowledge_dir(app, project)``
|
||||
and passes it in. This keeps the writer decoupled from the root-resolution
|
||||
logic and easier to test.
|
||||
|
||||
``category_id`` / ``topic`` come from parsed source documents (untrusted
|
||||
free text), so both are routed through
|
||||
:func:`everos.core.persistence.markdown.sanitize_dirname` before becoming
|
||||
a directory/file segment — the same shared helper
|
||||
``AgentSkillFrontmatter.skill_dir_name`` uses for LLM-generated skill
|
||||
names, so there is one CWE-22 path-traversal defense for md directory
|
||||
names, not two independently maintained copies.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -28,12 +35,10 @@ import yaml
|
|||
from everalgo.types import KnowledgeMemory
|
||||
|
||||
from everos.core.observability.logging import get_logger
|
||||
from everos.core.persistence.markdown import sanitize_dirname
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
_MAX_DIRNAME_LEN = 50
|
||||
_SAFE_CHARS = re.compile(r"[^\w\-.]", re.UNICODE)
|
||||
|
||||
|
||||
# ── Writer ────────────────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -113,26 +118,12 @@ def _split_root_and_topics(
|
|||
return root, topics
|
||||
|
||||
|
||||
def _sanitize_dirname(raw: str, fallback: str) -> str:
|
||||
"""Produce a safe directory/file name segment.
|
||||
|
||||
* Replace spaces with underscores.
|
||||
* Strip characters outside ``[a-zA-Z0-9_\\-.]``.
|
||||
* Truncate to 50 characters.
|
||||
* Fall back to *fallback* if the result is empty.
|
||||
"""
|
||||
slug = raw.replace(" ", "_")
|
||||
slug = _SAFE_CHARS.sub("", slug)
|
||||
slug = slug[:_MAX_DIRNAME_LEN]
|
||||
return slug if slug else fallback
|
||||
|
||||
|
||||
def _resolve_doc_dir(knowledge_dir: Path, root: KnowledgeMemory) -> Path:
|
||||
"""Build the document directory path from category, title, and doc_id."""
|
||||
category = _sanitize_dirname(
|
||||
category = sanitize_dirname(
|
||||
root.category_id if root.category_id else "Others", "Others"
|
||||
)
|
||||
title_slug = _sanitize_dirname(root.topic, "doc")
|
||||
title_slug = sanitize_dirname(root.topic, "doc")
|
||||
dir_name = f"{title_slug}_{root.doc_id}"
|
||||
return knowledge_dir / category / dir_name
|
||||
|
||||
|
|
@ -232,7 +223,7 @@ async def _write_topic(
|
|||
doc_id: str,
|
||||
) -> None:
|
||||
"""Write a numbered topic md file with frontmatter and content body."""
|
||||
slug = _sanitize_dirname(node.topic, f"topic_{node.topic_index}")
|
||||
slug = sanitize_dirname(node.topic, f"topic_{node.topic_index}")
|
||||
filename = f"{node.topic_index}_{slug}.md"
|
||||
fm = _build_topic_frontmatter(node, doc_id)
|
||||
body = _ensure_trailing_newline(node.content)
|
||||
|
|
|
|||
|
|
@ -65,18 +65,26 @@ class EpisodeExtracted(BaseEvent):
|
|||
class AgentCaseExtracted(BaseEvent):
|
||||
"""Fired by ``extract_agent_case`` after the AgentCase md is written.
|
||||
|
||||
Carries ``task_intent`` so the skill-clustering strategy can embed it
|
||||
directly, and ``quality_score`` so the strategy can short-circuit
|
||||
before any embedding work when the case is below algo's quality floor
|
||||
(``AgentCaseExtractor`` also short-circuits internally; this is the
|
||||
upstream gate that saves an LLM call too). ``case_timestamp_ms``
|
||||
drives the algo-side ``Cluster.last_ts`` for the time-window filter
|
||||
in :func:`everalgo.clustering.cluster_by_geometry`.
|
||||
Carries the full case body (``task_intent`` / ``approach`` / ``key_insight``)
|
||||
so downstream strategies do not need to read LanceDB — they receive strong-
|
||||
consistency data on the event bus, avoiding the cascade lag that made
|
||||
``extract_agent_skill`` retry-then-dead-letter on every run.
|
||||
|
||||
``quality_score`` lets ``trigger_skill_clustering`` short-circuit before any
|
||||
embedding work when the case is below algo's quality floor.
|
||||
``case_timestamp_ms`` drives the algo-side ``Cluster.last_ts`` for the
|
||||
time-window filter in :func:`everalgo.clustering.cluster_by_geometry`.
|
||||
"""
|
||||
|
||||
memcell_id: str
|
||||
case_entry_id: str
|
||||
task_intent: str
|
||||
approach: str = ""
|
||||
"""Case's Approach section verbatim. Defaults empty for back-compat with
|
||||
pending 1.2.2 events in the OME run_record queue."""
|
||||
key_insight: str | None = None
|
||||
"""Case's optional KeyInsight section. Defaults None for the same
|
||||
back-compat reason as ``approach``."""
|
||||
quality_score: float
|
||||
case_timestamp_ms: int
|
||||
agent_id: str
|
||||
|
|
@ -103,8 +111,11 @@ class SkillClusterUpdated(BaseEvent):
|
|||
"""Fired after the agent-case cluster strategy has merged a new
|
||||
case into a cluster.
|
||||
|
||||
Drives the agent-skill extraction strategy; ``cluster_id`` is the
|
||||
new or merged cluster the source case now belongs to.
|
||||
Drives the agent-skill extraction strategy. Carries a snapshot of the
|
||||
triggering case body (``task_intent`` / ``approach`` / ``key_insight`` /
|
||||
``quality_score`` / ``case_timestamp_ms``) plus ``case_vector`` (already
|
||||
embedded by ``trigger_skill_clustering``) so ``extract_agent_skill`` can
|
||||
build its algo input without a LanceDB probe that races cascade.
|
||||
"""
|
||||
|
||||
case_entry_id: str
|
||||
|
|
@ -112,3 +123,14 @@ class SkillClusterUpdated(BaseEvent):
|
|||
agent_id: str
|
||||
app_id: str = "default"
|
||||
project_id: str = "default"
|
||||
task_intent: str = ""
|
||||
"""Case task_intent for algo-side rendering. Default empty for back-compat
|
||||
with 1.2.2 payloads in the OME run_record queue; 1.2.3+ emitters populate it."""
|
||||
approach: str = ""
|
||||
key_insight: str | None = None
|
||||
quality_score: float = 0.0
|
||||
case_timestamp_ms: int = 0
|
||||
case_vector: list[float] | None = None
|
||||
"""Case task_intent embedding, produced by trigger_skill_clustering when it
|
||||
embeds for cluster matching. Passed through so extract_agent_skill does not
|
||||
need a second embedding call for the > MAX_SKILLS_IN_PROMPT top-k branch."""
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ from __future__ import annotations
|
|||
|
||||
import datetime as _dt
|
||||
from collections.abc import Awaitable, Callable
|
||||
from typing import TYPE_CHECKING, Any
|
||||
from typing import TYPE_CHECKING, Any, Literal
|
||||
|
||||
from everalgo.rank.agentic import aagentic_retrieve
|
||||
from everalgo.rank.hybrid import ahybrid_retrieve
|
||||
|
|
@ -30,7 +30,12 @@ from everalgo.types import Candidate
|
|||
|
||||
from everos.component.utils.datetime import from_timestamp, to_timestamp_ms
|
||||
from everos.core.observability.tracing import memory_span
|
||||
from everos.memory.search.callbacks import build_rerank_fn
|
||||
from everos.memory.search.callbacks import (
|
||||
_format_case_passage_from_metadata,
|
||||
_format_skill_passage_from_metadata,
|
||||
build_case_rerank_fn,
|
||||
build_skill_rerank_fn,
|
||||
)
|
||||
from everos.memory.search.shaper import (
|
||||
shape_agent_case_from_candidate,
|
||||
shape_agent_skill_from_candidate,
|
||||
|
|
@ -58,27 +63,41 @@ _MULTI_QUERY_COUNT: int = 3 # num_queries
|
|||
_REFINEMENT_STRATEGY: str = "multi_query"
|
||||
|
||||
|
||||
_EMPTY_PASSAGE = "(empty)"
|
||||
"""Stand-in body for a row whose every passage field is blank. Keeps
|
||||
``everalgo.rank.agentic._format_docs`` from raising on an empty string."""
|
||||
|
||||
|
||||
def _to_everalgo_doc_metadata(
|
||||
metadata: dict[str, Any], *, text_field: str
|
||||
metadata: dict[str, Any], *, format_passage: Callable[[dict[str, Any]], str]
|
||||
) -> dict[str, Any]:
|
||||
"""Bridge agent recall metadata to the everalgo ``_format_docs`` contract.
|
||||
|
||||
``aagentic_retrieve`` renders round-1 candidates into the sufficiency /
|
||||
multi-query LLM prompt via ``everalgo.rank.agentic._format_docs``, which
|
||||
reads ``metadata["episode"]`` as a dict with ``subject`` + ``content`` and
|
||||
a ms-epoch ``metadata["timestamp"]``. Agent-kind rows carry their body in
|
||||
``text_field`` (``task_intent`` / ``skill``) and the time in ``timestamp``
|
||||
(datetime); without this bridge ``_format_docs`` raises ``TypeError``.
|
||||
a ms-epoch ``metadata["timestamp"]``. Agent-kind rows carry their body
|
||||
across several fields (``name``/``description`` for skills,
|
||||
``task_intent``/``approach`` for cases), so ``format_passage`` is the
|
||||
same kind-shaped formatter the reranker uses — this keeps the passage the
|
||||
LLM sufficiency check sees identical to the passage the reranker scores.
|
||||
``content`` falls back to ``_EMPTY_PASSAGE`` when the formatter yields
|
||||
nothing. That happens only when *both* source fields are empty — a
|
||||
degenerate row, but a reachable one on the case side, where nothing
|
||||
guarantees ``task_intent`` is populated (the skill side is safe: the
|
||||
sanitizer floors ``name`` at ``"unnamed"``). Without the fallback
|
||||
``_format_docs`` raises ``ValueError`` on the empty string and the whole
|
||||
search request 500s, so one malformed row would take out a result set it
|
||||
merely happens to appear in. A placeholder is strictly better: the LLM
|
||||
sees a row it will rank last instead of the caller seeing nothing at all.
|
||||
Mirrors the episode path's bridge in ``agentic.py``.
|
||||
``_restore_shaper_metadata`` reverts it before DTO shaping.
|
||||
"""
|
||||
bridged = dict(metadata)
|
||||
content = metadata.get(text_field)
|
||||
if isinstance(content, str):
|
||||
bridged["episode"] = {
|
||||
"subject": metadata.get("subject", ""),
|
||||
"content": content,
|
||||
}
|
||||
bridged["episode"] = {
|
||||
"subject": metadata.get("subject", ""),
|
||||
"content": format_passage(metadata) or _EMPTY_PASSAGE,
|
||||
}
|
||||
timestamp = metadata.get("timestamp")
|
||||
if isinstance(timestamp, _dt.datetime):
|
||||
bridged["timestamp"] = to_timestamp_ms(timestamp)
|
||||
|
|
@ -137,6 +156,7 @@ async def search_agent_cases_agentic(
|
|||
reranker=reranker,
|
||||
llm=llm,
|
||||
top_k=top_k,
|
||||
kind="case",
|
||||
)
|
||||
return [
|
||||
item
|
||||
|
|
@ -178,6 +198,7 @@ async def search_agent_skills_agentic(
|
|||
reranker=reranker,
|
||||
llm=llm,
|
||||
top_k=top_k,
|
||||
kind="skill",
|
||||
)
|
||||
return [
|
||||
item
|
||||
|
|
@ -196,6 +217,7 @@ async def _run_agentic_retrieve(
|
|||
reranker: RerankProvider,
|
||||
llm: LLMClient,
|
||||
top_k: int,
|
||||
kind: Literal["case", "skill"],
|
||||
) -> list[Candidate]:
|
||||
"""Shared flat agentic retrieve pipeline for agent memory kinds.
|
||||
|
||||
|
|
@ -203,7 +225,18 @@ async def _run_agentic_retrieve(
|
|||
hands it to ``aagentic_retrieve`` with hyperparameters aligned to the
|
||||
memsys_opensource ``AgenticConfig`` defaults.
|
||||
No cluster or MaxSim step: agent memory is small enough for a flat pass.
|
||||
|
||||
``kind`` selects the passage formatter and the rerank fn together — the
|
||||
passage the LLM sufficiency check sees and the passage the cross-encoder
|
||||
scores must be the same shape, or the two stages silently disagree on
|
||||
what "relevant" means.
|
||||
"""
|
||||
if kind == "case":
|
||||
passage_formatter = _format_case_passage_from_metadata
|
||||
rerank_fn = build_case_rerank_fn(reranker)
|
||||
else:
|
||||
passage_formatter = _format_skill_passage_from_metadata
|
||||
rerank_fn = build_skill_rerank_fn(reranker)
|
||||
|
||||
async def _dense(q: str, k: int) -> list[Candidate]:
|
||||
vec = await embed_query_fn(q)
|
||||
|
|
@ -236,15 +269,13 @@ async def _run_agentic_retrieve(
|
|||
c.model_copy(
|
||||
update={
|
||||
"metadata": _to_everalgo_doc_metadata(
|
||||
c.metadata, text_field=recaller.text_field
|
||||
c.metadata, format_passage=passage_formatter
|
||||
)
|
||||
}
|
||||
)
|
||||
for c in hits
|
||||
]
|
||||
|
||||
rerank_fn = build_rerank_fn(reranker, text_field=recaller.text_field)
|
||||
|
||||
candidates, _decision = await aagentic_retrieve(
|
||||
query,
|
||||
base_retrieve=hybrid_full,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
"""Callback factories handed to ``everalgo.rank.arank``.
|
||||
|
||||
Three callbacks the rank pipeline expects:
|
||||
Four callbacks the rank pipeline expects:
|
||||
|
||||
* :func:`build_rerank_fn` — cross-encoder scorer used by ``agentic``
|
||||
Round-1 + final rerank, and by ``rrf`` / ``lr`` when LLM rerank is
|
||||
|
|
@ -12,10 +12,20 @@ Three callbacks the rank pipeline expects:
|
|||
shape doesn't fit the single-``text_field`` contract above) and uses
|
||||
a skill-specific instruction. Mirrors memsys_opensource
|
||||
``_rerank_skill_items``.
|
||||
* :func:`build_case_rerank_fn` — case-shaped variant, mirrors
|
||||
:func:`build_skill_rerank_fn` for ``"Agent Case: {task_intent} -
|
||||
{approach}"`` passages.
|
||||
* :func:`build_retrieve_fn` — Round-2 recall callback for ``agentic``.
|
||||
Re-runs the sparse + dense recall path for a refined query and fuses
|
||||
the two routes with RRF (``k=60``) before handing back to the agentic
|
||||
loop.
|
||||
|
||||
``_format_skill_passage_from_metadata`` / ``_format_case_passage_from_metadata``
|
||||
take the raw ``Candidate.metadata`` dict rather than a ``Candidate`` so the
|
||||
metadata bridge in ``agentic_agent.py`` (which formats a passage before a
|
||||
``Candidate`` wrapping it exists) can reuse the exact same formatting logic
|
||||
the rerank step uses — one implementation, no drift between what the LLM
|
||||
sufficiency check sees and what the reranker sees.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -90,11 +100,10 @@ _SKILL_RERANK_INSTRUCTION = (
|
|||
)
|
||||
|
||||
|
||||
def _format_skill_passage(candidate: Candidate) -> str:
|
||||
def _format_skill_passage_from_metadata(meta: dict[str, object]) -> str:
|
||||
"""``"Agent Skill: {name}"`` + ``" - {description}"`` when present.
|
||||
Mirrors opensource ``extract_text_from_hit`` for AGENT_SKILL.
|
||||
"""
|
||||
meta = candidate.metadata
|
||||
name = str(meta.get("name", "") or "")
|
||||
description = str(meta.get("description", "") or "")
|
||||
if not name:
|
||||
|
|
@ -104,6 +113,11 @@ def _format_skill_passage(candidate: Candidate) -> str:
|
|||
return f"Agent Skill: {name}"
|
||||
|
||||
|
||||
def _format_skill_passage(candidate: Candidate) -> str:
|
||||
"""``Candidate``-shaped wrapper over :func:`_format_skill_passage_from_metadata`."""
|
||||
return _format_skill_passage_from_metadata(candidate.metadata)
|
||||
|
||||
|
||||
def build_skill_rerank_fn(provider: RerankProvider) -> RerankFn:
|
||||
"""Skill-shaped ``RerankFn``: multi-field passage +
|
||||
:data:`_SKILL_RERANK_INSTRUCTION`. Output stays score-comparable
|
||||
|
|
@ -136,6 +150,66 @@ def build_skill_rerank_fn(provider: RerankProvider) -> RerankFn:
|
|||
return _rerank
|
||||
|
||||
|
||||
# Mirrors _SKILL_RERANK_INSTRUCTION: biases the reranker toward methodology /
|
||||
# domain match for agent cases rather than generic Q-A relevance.
|
||||
_CASE_RERANK_INSTRUCTION = (
|
||||
"Determine whether the case's task and approach are applicable to the "
|
||||
"query, preferring same-domain cases with directly relevant methodology."
|
||||
)
|
||||
|
||||
|
||||
def _format_case_passage_from_metadata(meta: dict[str, object]) -> str:
|
||||
"""``"Agent Case: {task_intent}"`` + ``" - {approach}"`` when present.
|
||||
|
||||
Mirrors ``_format_skill_passage_from_metadata``. Falls back to
|
||||
``task_intent`` alone when ``approach`` is empty (which is legal per the
|
||||
cascade handler — no non-empty guard on ``approach``).
|
||||
"""
|
||||
task_intent = str(meta.get("task_intent", "") or "")
|
||||
approach = str(meta.get("approach", "") or "")
|
||||
if not task_intent:
|
||||
return approach
|
||||
if approach:
|
||||
return f"Agent Case: {task_intent} - {approach}"
|
||||
return f"Agent Case: {task_intent}"
|
||||
|
||||
|
||||
def _format_case_passage(candidate: Candidate) -> str:
|
||||
"""``Candidate``-shaped wrapper over :func:`_format_case_passage_from_metadata`."""
|
||||
return _format_case_passage_from_metadata(candidate.metadata)
|
||||
|
||||
|
||||
def build_case_rerank_fn(provider: RerankProvider) -> RerankFn:
|
||||
"""Case-shaped ``RerankFn``: multi-field passage + :data:`_CASE_RERANK_INSTRUCTION`.
|
||||
Mirrors :func:`build_skill_rerank_fn`.
|
||||
"""
|
||||
|
||||
async def _rerank(
|
||||
query: str,
|
||||
candidates: Sequence[Candidate],
|
||||
) -> list[Candidate]:
|
||||
items = list(candidates)
|
||||
if not items:
|
||||
return []
|
||||
passages = [_format_case_passage(c) for c in items]
|
||||
with memory_span(
|
||||
"everos.search.rank",
|
||||
observation_type="span",
|
||||
metadata={"phase": "cross_encoder_case"},
|
||||
):
|
||||
results = await provider.rerank(
|
||||
query, passages, instruction=_CASE_RERANK_INSTRUCTION
|
||||
)
|
||||
out: list[Candidate] = []
|
||||
for r in results:
|
||||
if not 0 <= r.index < len(items):
|
||||
continue
|
||||
out.append(items[r.index].model_copy(update={"score": float(r.score)}))
|
||||
return out
|
||||
|
||||
return _rerank
|
||||
|
||||
|
||||
def build_retrieve_fn(
|
||||
recaller: KindRecaller,
|
||||
*,
|
||||
|
|
|
|||
|
|
@ -100,6 +100,8 @@ async def extract_agent_case(event: AgentPipelineStarted, ctx: StrategyContext)
|
|||
memcell_id=event.memcell_id,
|
||||
case_entry_id=eid.format(),
|
||||
task_intent=case.task_intent,
|
||||
approach=case.approach,
|
||||
key_insight=case.key_insight,
|
||||
quality_score=case.quality_score,
|
||||
case_timestamp_ms=case.timestamp,
|
||||
agent_id=case.owner_id,
|
||||
|
|
|
|||
|
|
@ -3,28 +3,64 @@
|
|||
Triggered by :class:`SkillClusterUpdated` after ``trigger_skill_clustering``
|
||||
has assigned the fresh case to its cluster. The strategy:
|
||||
|
||||
1. Selects the ``existing_relevant_skills`` slice for this cluster:
|
||||
1. Reconstructs the target case directly from the event payload
|
||||
(:func:`_to_algo_case_from_event`) — no LanceDB read. This is the
|
||||
cascade-lag rescue: the previous implementation probed
|
||||
``agent_case_repo.find_by_owner_entry`` for the freshly-written case
|
||||
and raised a retry-class error when cascade hadn't indexed it yet,
|
||||
which under sustained cascade lag meant the run died after
|
||||
``max_retries`` and OME dead-lettered it — the case was never
|
||||
distilled into a skill. The case body now travels on the event bus,
|
||||
so the strategy never races cascade indexing.
|
||||
2. Selects the ``existing_relevant_skills`` slice for this cluster,
|
||||
**md-first** (:func:`_select_existing_skills`):
|
||||
|
||||
* cluster size ``≤ MAX_SKILLS_IN_PROMPT`` → scalar fetch (ranking
|
||||
would be pointless on a fully-inclusive set);
|
||||
* cluster size ``> MAX_SKILLS_IN_PROMPT`` and the target case has a
|
||||
usable vector (either persisted on the row or re-embedded
|
||||
on-the-fly from ``task_intent``) → cosine top-K against the
|
||||
cluster;
|
||||
* cluster size ``> MAX_SKILLS_IN_PROMPT`` but no vector signal is
|
||||
obtainable → scalar fetch capped at K (logged warning so
|
||||
truncation without ranking is observable).
|
||||
2. Hydrates ``supporting_cases`` from the chosen skills'
|
||||
* ``AgentSkillReader.list_by_cluster`` is the source of truth for
|
||||
"which skills exist in this cluster" — md is strongly consistent,
|
||||
LanceDB is cascade-lagged and must not be used for existence
|
||||
checks (a stale index previously made the LLM emit ``add()`` for a
|
||||
skill that already existed in md, silently clobbering it on
|
||||
write-back);
|
||||
* cluster size ``≤ MAX_SKILLS_IN_PROMPT`` → every md skill is used
|
||||
(ranking would be pointless on a fully-inclusive set);
|
||||
* cluster size ``> MAX_SKILLS_IN_PROMPT`` and the event carries a
|
||||
``case_vector`` → LanceDB ranks by cosine relevance, md hydrates
|
||||
the winning ids' content (LanceDB is a ranking index here, never
|
||||
an existence check);
|
||||
* cluster size ``> MAX_SKILLS_IN_PROMPT`` but no ``case_vector`` is
|
||||
available (pre-1.2.3 event, or embedding was unavailable upstream)
|
||||
→ md ordering capped at K (logged warning so truncation without
|
||||
ranking is observable).
|
||||
3. Hydrates ``supporting_cases`` from the chosen skills'
|
||||
``source_case_ids`` lineage. The algo prompt joins each existing
|
||||
skill to its ``source_case_ids`` via the ``supporting_cases`` map;
|
||||
cases that do not back any of the chosen skills would just inflate
|
||||
the prompt without informing the LLM. Hydrated cases are then
|
||||
ranked ``(quality_score desc, timestamp desc)`` and capped at
|
||||
``MAX_SUPPORTING_CASES`` to keep the prompt bounded as a cluster
|
||||
grows.
|
||||
3. Feeds the target + existing + supporting trio to
|
||||
grows. Unlike the target case and existing skills, this lineage read
|
||||
stays LanceDB-backed: an un-indexed supporting case only means a
|
||||
thinner prompt this run (non-corrupting; the next run catches up),
|
||||
not a wrong write.
|
||||
4. Feeds the target + existing + supporting trio to
|
||||
:class:`everalgo.agent_memory.AgentSkillExtractor`, then writes the
|
||||
emitted skills back via :class:`AgentSkillWriter`.
|
||||
emitted skills back via :class:`AgentSkillWriter` and reaps the
|
||||
directory an update left behind when it renamed a skill (see
|
||||
:func:`_reap_renamed_skills`).
|
||||
|
||||
**Retire is not implemented.** ``AgentSkillExtractor.aextract`` returns a
|
||||
flat ``list[AgentSkill]`` with no op discriminator; its retire branch
|
||||
(``skill_ops._apply_update``, taken when ``confidence <
|
||||
retire_confidence``, default ``0.1``) is an ordinary skill carrying a
|
||||
lowered confidence and nothing else. This strategy writes every emitted
|
||||
skill back the same way, so a retirement persists as a normal skill: it
|
||||
stays in markdown, stays in the next run's prompt, and stays searchable.
|
||||
Honouring it means choosing between deleting the directory — handing an
|
||||
LLM-produced confidence score the authority to destroy the source of
|
||||
truth — and a ``retired`` frontmatter flag, which only works if the
|
||||
enumeration, cascade, and search all learn to filter on it. That is a
|
||||
design decision, not an omission to patch over, so it is deferred and
|
||||
stated here rather than left implied by a docstring listing three ops.
|
||||
|
||||
Per-case granularity (one strategy run per fresh case) — algo
|
||||
short-circuits low-quality cases internally via its own
|
||||
|
|
@ -34,16 +70,14 @@ short-circuits low-quality cases internally via its own
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Mapping, Sequence
|
||||
|
||||
from everalgo.agent_memory import AgentSkillExtractor
|
||||
from everalgo.types import AgentCase as AlgoAgentCase
|
||||
from everalgo.types import AgentSkill as AlgoAgentSkill
|
||||
|
||||
from everos.component.embedding import (
|
||||
EmbeddingServiceError,
|
||||
get_embedding_capability,
|
||||
)
|
||||
from everos.component.embedding import get_embedding_capability
|
||||
from everos.component.llm import get_llm_client
|
||||
from everos.core.errors import ProviderNotConfiguredError
|
||||
from everos.core.observability.logging import get_logger
|
||||
from everos.core.persistence import MemoryRoot
|
||||
from everos.infra.ome.context import StrategyContext
|
||||
|
|
@ -52,15 +86,13 @@ from everos.infra.ome.triggers import Immediate
|
|||
from everos.infra.persistence.lancedb import (
|
||||
AgentCase as LanceAgentCase,
|
||||
)
|
||||
from everos.infra.persistence.lancedb import (
|
||||
AgentSkill as LanceAgentSkill,
|
||||
)
|
||||
from everos.infra.persistence.lancedb import (
|
||||
agent_case_repo,
|
||||
agent_skill_repo,
|
||||
)
|
||||
from everos.infra.persistence.markdown import (
|
||||
AgentSkillFrontmatter,
|
||||
AgentSkillReader,
|
||||
AgentSkillWriter,
|
||||
)
|
||||
from everos.infra.persistence.sqlite import cluster_repo
|
||||
|
|
@ -74,8 +106,9 @@ MAX_SKILLS_IN_PROMPT = 10
|
|||
|
||||
The algo library expects the caller to pre-filter
|
||||
``existing_relevant_skills`` to a relevant subset (cosine top-K over
|
||||
the target case's ``task_intent`` embedding) so the prompt stays
|
||||
bounded as a cluster grows."""
|
||||
``SkillClusterUpdated.case_vector``, embedded upstream by
|
||||
``trigger_skill_clustering``) so the prompt stays bounded as a cluster
|
||||
grows."""
|
||||
|
||||
MAX_SUPPORTING_CASES = 9
|
||||
"""Upper bound on ``supporting_cases`` after lineage hydration.
|
||||
|
|
@ -92,11 +125,8 @@ class _ClusterMissingError(RuntimeError):
|
|||
"""Race with the cluster strategy; OME retry will catch up."""
|
||||
|
||||
|
||||
class _CaseNotYetIndexedError(RuntimeError):
|
||||
"""The target case is in md but not yet in LanceDB; OME retry will catch up."""
|
||||
|
||||
|
||||
_writer: AgentSkillWriter | None = None
|
||||
_reader: AgentSkillReader | None = None
|
||||
|
||||
|
||||
def _get_writer() -> AgentSkillWriter:
|
||||
|
|
@ -106,6 +136,13 @@ def _get_writer() -> AgentSkillWriter:
|
|||
return _writer
|
||||
|
||||
|
||||
def _get_reader() -> AgentSkillReader:
|
||||
global _reader
|
||||
if _reader is None:
|
||||
_reader = AgentSkillReader(root=MemoryRoot.resolve())
|
||||
return _reader
|
||||
|
||||
|
||||
@offline_strategy(
|
||||
name="extract_agent_skill",
|
||||
trigger=Immediate(on=[SkillClusterUpdated]),
|
||||
|
|
@ -113,13 +150,16 @@ def _get_writer() -> AgentSkillWriter:
|
|||
max_retries=3,
|
||||
)
|
||||
async def extract_agent_skill(event: SkillClusterUpdated, ctx: StrategyContext) -> None:
|
||||
# Body-guard: capability is checked here for defensive degradation.
|
||||
# Belt-and-suspenders even though the upstream
|
||||
# trigger_skill_clustering already gates on the same capability — a
|
||||
# direct emit of SkillClusterUpdated (tests, future features) should
|
||||
# still degrade cleanly without an owner lock or OME retry pressure.
|
||||
# Tier upgrades require a server restart; this guard is not a
|
||||
# hot-reload mechanism.
|
||||
# Body-guard: this strategy no longer embeds anything (the query
|
||||
# vector rides the event), so the guard is not protecting a local
|
||||
# call — it keeps the whole agent-skill track consistent with the
|
||||
# tier the deployment is actually running. Upstream
|
||||
# trigger_skill_clustering gates on the same capability, so no
|
||||
# SkillClusterUpdated normally exists without an embedder; the guard
|
||||
# covers a direct emit (tests, future features), which should degrade
|
||||
# cleanly rather than take an owner lock and produce skills the
|
||||
# ranking half of the pipeline cannot serve. Tier upgrades require a
|
||||
# server restart; this guard is not a hot-reload mechanism.
|
||||
#
|
||||
# ``debug`` level (not ``info``) is intentional; see the body-guard
|
||||
# in :func:`everos.memory.strategies.trigger_profile_clustering` for
|
||||
|
|
@ -143,44 +183,46 @@ async def extract_agent_skill(event: SkillClusterUpdated, ctx: StrategyContext)
|
|||
# 1. Check the cluster row exists.
|
||||
await _ensure_cluster_exists(event.cluster_id, event.case_entry_id)
|
||||
|
||||
# 2. Load the target AgentCase from LanceDB (scoped to space).
|
||||
target_lance = await _load_target_case(
|
||||
event.agent_id,
|
||||
event.case_entry_id,
|
||||
app_id=event.app_id,
|
||||
project_id=event.project_id,
|
||||
)
|
||||
# 2. Reconstruct the target AgentCase from the event payload — no
|
||||
# LanceDB probe, so the strategy never races cascade indexing.
|
||||
target = _to_algo_case_from_event(event)
|
||||
|
||||
# 3. Pick the top-K relevant existing skills in this cluster.
|
||||
# 3. Pick the top-K relevant existing skills in this cluster, md-first.
|
||||
# (Cluster-scoped queries are implicitly space-scoped: cluster_id
|
||||
# is globally unique to one (app, project, owner) cluster set.)
|
||||
existing_lance = await _select_existing_skills(
|
||||
existing_skills = await _select_existing_skills(
|
||||
agent_id=event.agent_id,
|
||||
cluster_id=event.cluster_id,
|
||||
target=target_lance,
|
||||
app_id=event.app_id,
|
||||
project_id=event.project_id,
|
||||
case_vector=event.case_vector,
|
||||
)
|
||||
|
||||
# 4. Pull the supporting cases referenced by those skills.
|
||||
supporting_lance = await _select_supporting_cases(
|
||||
existing_lance,
|
||||
existing_skills,
|
||||
agent_id=event.agent_id,
|
||||
exclude_entry_id=event.case_entry_id,
|
||||
app_id=event.app_id,
|
||||
project_id=event.project_id,
|
||||
)
|
||||
|
||||
# 5. Run the LLM extractor → add / update / retire skill operations.
|
||||
# 5. Run the LLM extractor. Emits add / update ops; a retire op comes
|
||||
# back as an ordinary skill with confidence < retire_confidence and
|
||||
# is NOT honoured here — see the module docstring.
|
||||
extractor = AgentSkillExtractor(llm=get_llm_client())
|
||||
emitted_skills = await extractor.aextract(
|
||||
_to_algo_case(target_lance),
|
||||
existing_relevant_skills=[_to_algo_skill(s) for s in existing_lance],
|
||||
target,
|
||||
existing_relevant_skills=existing_skills,
|
||||
supporting_cases=[_to_algo_case(c) for c in supporting_lance],
|
||||
)
|
||||
|
||||
# 6. Write each emitted skill back to its SKILL.md.
|
||||
# 6. Write each emitted skill back to its SKILL.md, then reap the
|
||||
# directories that a rename left behind.
|
||||
writer = _get_writer()
|
||||
written_names: dict[str, str] = {}
|
||||
for skill in emitted_skills:
|
||||
await _persist_skill(
|
||||
written_names[skill.id] = await _persist_skill(
|
||||
writer,
|
||||
skill,
|
||||
agent_id=event.agent_id,
|
||||
|
|
@ -188,6 +230,14 @@ async def extract_agent_skill(event: SkillClusterUpdated, ctx: StrategyContext)
|
|||
app_id=event.app_id,
|
||||
project_id=event.project_id,
|
||||
)
|
||||
await _reap_renamed_skills(
|
||||
writer,
|
||||
written_names,
|
||||
existing_skills=existing_skills,
|
||||
agent_id=event.agent_id,
|
||||
app_id=event.app_id,
|
||||
project_id=event.project_id,
|
||||
)
|
||||
logger.info(
|
||||
"agent_skills_extracted",
|
||||
case_entry_id=event.case_entry_id,
|
||||
|
|
@ -210,104 +260,104 @@ async def _ensure_cluster_exists(cluster_id: str, case_entry_id: str) -> None:
|
|||
)
|
||||
|
||||
|
||||
async def _load_target_case(
|
||||
agent_id: str,
|
||||
case_entry_id: str,
|
||||
*,
|
||||
app_id: str,
|
||||
project_id: str,
|
||||
) -> LanceAgentCase:
|
||||
"""Pull the target case row, raising a retry-class error on cascade lag."""
|
||||
target = await agent_case_repo.find_by_owner_entry(
|
||||
agent_id, case_entry_id, app_id=app_id, project_id=project_id
|
||||
)
|
||||
if target is None:
|
||||
# Cascade hasn't indexed the freshly-written md yet.
|
||||
raise _CaseNotYetIndexedError(
|
||||
f"AgentCase entry_id={case_entry_id} not in LanceDB yet; retrying"
|
||||
)
|
||||
return target
|
||||
|
||||
|
||||
async def _select_existing_skills(
|
||||
*,
|
||||
agent_id: str,
|
||||
cluster_id: str,
|
||||
target: LanceAgentCase,
|
||||
) -> list[LanceAgentSkill]:
|
||||
app_id: str,
|
||||
project_id: str,
|
||||
case_vector: list[float] | None,
|
||||
) -> list[AlgoAgentSkill]:
|
||||
"""Pick at most ``MAX_SKILLS_IN_PROMPT`` existing skills for the prompt.
|
||||
|
||||
See module docstring for the three-branch routing rationale.
|
||||
"""
|
||||
total = await agent_skill_repo.count_in_cluster(
|
||||
owner_id=agent_id, cluster_id=cluster_id
|
||||
)
|
||||
if total <= MAX_SKILLS_IN_PROMPT:
|
||||
return await agent_skill_repo.find_in_cluster(
|
||||
owner_id=agent_id, cluster_id=cluster_id, limit=MAX_SKILLS_IN_PROMPT
|
||||
)
|
||||
md is the source of truth for existence — this avoids the
|
||||
stale-index clobber where a skill was written last run but hadn't
|
||||
been indexed into LanceDB yet, causing the LLM to see no existing
|
||||
skill and emit ``add()`` for one that already exists. LanceDB is
|
||||
only consulted for relevance ordering when the cluster's md skill
|
||||
count exceeds ``MAX_SKILLS_IN_PROMPT`` and ``case_vector`` is
|
||||
available; when it isn't (pre-1.2.3 event, or embedding was
|
||||
unavailable upstream), fall back to md ordering.
|
||||
|
||||
query_vector = await _resolve_query_vector(target)
|
||||
if query_vector:
|
||||
return await agent_skill_repo.find_topk_relevant_in_cluster(
|
||||
owner_id=agent_id,
|
||||
``list_by_cluster`` returns each skill's frontmatter *and* body
|
||||
together, so there is no second, name-based read to hydrate
|
||||
``content`` — a prior version re-read each selected skill by
|
||||
``fm.name`` via ``read_main``, which re-derives (and re-sanitizes) a
|
||||
path from that name and would silently miss a skill whose on-disk
|
||||
directory suffix isn't itself a sanitizer fixpoint, even though the
|
||||
first, path-based enumeration found it. See
|
||||
``AgentSkillReader.list_by_cluster``'s docstring.
|
||||
"""
|
||||
reader = _get_reader()
|
||||
md_skills = await reader.list_by_cluster(
|
||||
agent_id, cluster_id, app_id=app_id, project_id=project_id
|
||||
)
|
||||
if not md_skills:
|
||||
return []
|
||||
|
||||
if len(md_skills) <= MAX_SKILLS_IN_PROMPT:
|
||||
selected = md_skills
|
||||
elif case_vector is not None:
|
||||
selected = await _rank_skills_by_relevance(
|
||||
md_skills,
|
||||
agent_id=agent_id,
|
||||
cluster_id=cluster_id,
|
||||
query_vector=query_vector,
|
||||
top_k=MAX_SKILLS_IN_PROMPT,
|
||||
case_vector=case_vector,
|
||||
)
|
||||
|
||||
logger.warning(
|
||||
"agent_skill_topk_no_query_vector_scalar_fallback",
|
||||
agent_id=agent_id,
|
||||
cluster_id=cluster_id,
|
||||
cluster_size=total,
|
||||
)
|
||||
return await agent_skill_repo.find_in_cluster(
|
||||
owner_id=agent_id, cluster_id=cluster_id, limit=MAX_SKILLS_IN_PROMPT
|
||||
)
|
||||
|
||||
|
||||
async def _resolve_query_vector(target: LanceAgentCase) -> list[float]:
|
||||
"""Return a usable query vector for cosine top-K, ``[]`` if unobtainable.
|
||||
|
||||
Order of preference:
|
||||
|
||||
1. ``target.vector`` if cascade has already populated the column —
|
||||
this is the exact vector the recall path uses, so reusing it
|
||||
keeps ranking semantics identical across reads.
|
||||
2. Compute on the fly from ``target.task_intent`` via the configured
|
||||
embedder — matches the cascade handler's own vectorisation
|
||||
contract (``cascade/handlers/agent_case.py``), so the two paths
|
||||
agree on what "the case embedding" means.
|
||||
|
||||
Returns ``[]`` only when both options are unavailable (no persisted
|
||||
vector, no ``task_intent`` text, or the embedder is not configured /
|
||||
fails). The caller decides the policy for that case.
|
||||
"""
|
||||
if target.vector:
|
||||
return list(target.vector)
|
||||
if not target.task_intent:
|
||||
return []
|
||||
# ``.require()`` is defensive: the strategy body-guard checks
|
||||
# ``.available`` before we reach this helper, so this branch will
|
||||
# not raise ``ProviderNotConfiguredError`` in normal operation. The
|
||||
# catch stays so a misconfiguration surfacing later (or a direct
|
||||
# unit-test call to this helper without the guard) still degrades
|
||||
# to ``[]`` instead of blowing up mid-strategy.
|
||||
try:
|
||||
embedder = get_embedding_capability().require()
|
||||
return list(await embedder.embed(target.task_intent))
|
||||
except (ProviderNotConfiguredError, EmbeddingServiceError) as exc:
|
||||
else:
|
||||
logger.warning(
|
||||
"agent_skill_query_embed_failed",
|
||||
case_entry_id=target.entry_id,
|
||||
error=str(exc),
|
||||
"agent_skill_topk_no_query_vector_md_fallback",
|
||||
agent_id=agent_id,
|
||||
cluster_id=cluster_id,
|
||||
md_count=len(md_skills),
|
||||
)
|
||||
return []
|
||||
selected = md_skills[:MAX_SKILLS_IN_PROMPT]
|
||||
|
||||
return [_md_to_algo_skill(fm, body) for fm, body in selected]
|
||||
|
||||
|
||||
async def _rank_skills_by_relevance(
|
||||
md_skills: list[tuple[AgentSkillFrontmatter, str]],
|
||||
*,
|
||||
agent_id: str,
|
||||
cluster_id: str,
|
||||
case_vector: list[float],
|
||||
) -> list[tuple[AgentSkillFrontmatter, str]]:
|
||||
"""Ask LanceDB to rank the md skills by cosine relevance, capped at K.
|
||||
|
||||
LanceDB is used purely as a ranking index here, never as the
|
||||
existence check — the candidate set is always the md list. A LanceDB
|
||||
row with no matching md name is stale and skipped; md skills LanceDB
|
||||
didn't return (also stale index) then backfill in md order until the
|
||||
``MAX_SKILLS_IN_PROMPT`` budget is full. That backfill keeps a lagging
|
||||
index from *under*-filling the prompt; it does not make the selection
|
||||
lossless — this function only runs when the cluster already holds more
|
||||
skills than the budget admits, so skills beyond K are dropped by
|
||||
design either way.
|
||||
"""
|
||||
md_by_name = {fm.name: (fm, body) for fm, body in md_skills}
|
||||
ranked_lance = await agent_skill_repo.find_topk_relevant_in_cluster(
|
||||
owner_id=agent_id,
|
||||
cluster_id=cluster_id,
|
||||
query_vector=case_vector,
|
||||
top_k=MAX_SKILLS_IN_PROMPT,
|
||||
)
|
||||
selected: list[tuple[AgentSkillFrontmatter, str]] = []
|
||||
seen_names: set[str] = set()
|
||||
for lance_row in ranked_lance:
|
||||
pair = md_by_name.get(lance_row.name)
|
||||
if pair is not None and pair[0].name not in seen_names:
|
||||
selected.append(pair)
|
||||
seen_names.add(pair[0].name)
|
||||
for fm, body in md_skills:
|
||||
if fm.name not in seen_names and len(selected) < MAX_SKILLS_IN_PROMPT:
|
||||
selected.append((fm, body))
|
||||
seen_names.add(fm.name)
|
||||
return selected
|
||||
|
||||
|
||||
async def _select_supporting_cases(
|
||||
skills: list[LanceAgentSkill],
|
||||
skills: list[AlgoAgentSkill],
|
||||
*,
|
||||
agent_id: str,
|
||||
exclude_entry_id: str,
|
||||
|
|
@ -341,7 +391,7 @@ async def _select_supporting_cases(
|
|||
|
||||
|
||||
def _collect_supporting_entry_ids(
|
||||
skills: list[LanceAgentSkill], *, exclude: str
|
||||
skills: list[AlgoAgentSkill], *, exclude: str
|
||||
) -> list[str]:
|
||||
"""Dedup ``source_case_ids`` across ``skills``, preserving first-seen order."""
|
||||
seen: list[str] = []
|
||||
|
|
@ -359,7 +409,11 @@ def _collect_supporting_entry_ids(
|
|||
|
||||
|
||||
def _to_algo_case(lance: LanceAgentCase) -> AlgoAgentCase:
|
||||
"""Project the LanceDB row onto the algo-side AgentCase type."""
|
||||
"""Project the LanceDB row onto the algo-side AgentCase type.
|
||||
|
||||
Used only for ``supporting_cases`` — that lineage read stays
|
||||
LanceDB-backed (see module docstring, point 3).
|
||||
"""
|
||||
return AlgoAgentCase(
|
||||
id=lance.entry_id,
|
||||
timestamp=int(lance.timestamp.timestamp() * 1000),
|
||||
|
|
@ -370,24 +424,100 @@ def _to_algo_case(lance: LanceAgentCase) -> AlgoAgentCase:
|
|||
)
|
||||
|
||||
|
||||
def _to_algo_skill(lance: LanceAgentSkill) -> AlgoAgentSkill:
|
||||
"""Project the LanceDB row onto the algo-side AgentSkill type.
|
||||
def _to_algo_case_from_event(event: SkillClusterUpdated) -> AlgoAgentCase:
|
||||
"""Reconstruct the target case from event fields.
|
||||
|
||||
``cluster_id`` rides along even though algo doesn't read it on input —
|
||||
keeps the model fully populated for any consumer that introspects.
|
||||
Strong-consistency: the case body travels on the event bus, so the
|
||||
strategy never races cascade indexing. Pre-1.2.3 events default
|
||||
missing fields to empty — those runs won't have useful data but also
|
||||
won't crash.
|
||||
"""
|
||||
return AlgoAgentCase(
|
||||
id=event.case_entry_id,
|
||||
timestamp=event.case_timestamp_ms,
|
||||
task_intent=event.task_intent,
|
||||
approach=event.approach,
|
||||
quality_score=event.quality_score,
|
||||
key_insight=event.key_insight or "",
|
||||
)
|
||||
|
||||
|
||||
def _md_to_algo_skill(fm: AgentSkillFrontmatter, body: str) -> AlgoAgentSkill:
|
||||
"""Project a SKILL.md frontmatter + body onto everalgo's AgentSkill type.
|
||||
|
||||
``body`` populates ``AlgoAgentSkill.content`` so the extractor prompt's
|
||||
existing-skills block (``everalgo.agent_memory.skill_ops._format_existing_skills``)
|
||||
carries the real skill definition, not an empty placeholder — without it
|
||||
the LLM cannot distinguish "add new" from "update existing".
|
||||
"""
|
||||
return AlgoAgentSkill(
|
||||
id=lance.id,
|
||||
cluster_id=lance.cluster_id or "",
|
||||
name=lance.name,
|
||||
description=lance.description,
|
||||
content=lance.content,
|
||||
confidence=lance.confidence,
|
||||
maturity_score=lance.maturity_score,
|
||||
source_case_ids=list(lance.source_case_ids),
|
||||
id=fm.id,
|
||||
cluster_id=fm.cluster_id or "",
|
||||
name=fm.name,
|
||||
description=fm.description,
|
||||
content=body,
|
||||
confidence=fm.confidence,
|
||||
maturity_score=fm.maturity_score,
|
||||
source_case_ids=list(fm.source_case_ids),
|
||||
)
|
||||
|
||||
|
||||
async def _reap_renamed_skills(
|
||||
writer: AgentSkillWriter,
|
||||
written_names: Mapping[str, str],
|
||||
*,
|
||||
existing_skills: Sequence[AlgoAgentSkill],
|
||||
agent_id: str,
|
||||
app_id: str,
|
||||
project_id: str,
|
||||
) -> None:
|
||||
"""Delete the directory an update left behind when it renamed a skill.
|
||||
|
||||
everalgo treats a name change as a first-class update
|
||||
(``skill_ops._apply_update`` computes ``name_changed`` and returns
|
||||
``prior.model_copy(update={"name": eff_name, ...})``), so the emitted
|
||||
skill keeps ``prior.id`` while carrying the new name. ``_persist_skill``
|
||||
writes it to ``skill_<new_name>/`` and the old directory would survive
|
||||
with the same ``cluster_id``.
|
||||
|
||||
That matters more here than it looks. Since this release the input to
|
||||
the next extraction is the markdown enumeration, not LanceDB — so a
|
||||
surviving pre-rename directory does not merely sit on disk, it comes
|
||||
back in the next run's ``existing_relevant_skills`` as a duplicate of a
|
||||
skill the LLM already renamed. Shown its own stale copy, the LLM emits
|
||||
``add`` for something that exists, and ``write_main`` full-replaces —
|
||||
which is precisely the clobber this release set out to close. Left
|
||||
unreaped, every rename adds another one.
|
||||
|
||||
Identity comes from ``skill.id``, which is the only thing that survives
|
||||
a rename: ``_apply_update`` preserves ``prior.id`` while ``_apply_add``
|
||||
mints a fresh ``uuid4().hex``, so an id present in the enumerated set is
|
||||
an update by construction and a new skill can never match.
|
||||
|
||||
A prior name that some *other* emitted skill just claimed is never
|
||||
deleted — with two ops in one batch (rename ``a``→``b`` while another
|
||||
op writes ``a``) the reap would otherwise remove a file written
|
||||
moments earlier in the same loop.
|
||||
"""
|
||||
claimed = set(written_names.values())
|
||||
for prior in existing_skills:
|
||||
new_name = written_names.get(prior.id)
|
||||
prior_name = AgentSkillFrontmatter.sanitize_skill_name(prior.name)
|
||||
if new_name is None or new_name == prior_name or prior_name in claimed:
|
||||
continue
|
||||
removed = await writer.delete_skill(
|
||||
agent_id, prior_name, app_id=app_id, project_id=project_id
|
||||
)
|
||||
logger.info(
|
||||
"agent_skill_renamed_directory_reaped",
|
||||
agent_id=agent_id,
|
||||
skill_id=prior.id,
|
||||
old_name=prior_name,
|
||||
new_name=new_name,
|
||||
removed=removed,
|
||||
)
|
||||
|
||||
|
||||
async def _persist_skill(
|
||||
writer: AgentSkillWriter,
|
||||
skill: AlgoAgentSkill,
|
||||
|
|
@ -396,12 +526,29 @@ async def _persist_skill(
|
|||
cluster_id: str,
|
||||
app_id: str,
|
||||
project_id: str,
|
||||
) -> None:
|
||||
"""Write one ``SKILL.md`` with the post-stamped ``cluster_id``."""
|
||||
) -> str:
|
||||
"""Write one ``SKILL.md`` with the post-stamped ``cluster_id``.
|
||||
|
||||
Returns the sanitized name it wrote under, so the caller can
|
||||
reconcile renames without re-deriving it.
|
||||
|
||||
``skill.name`` is LLM output and is sanitized once, up front, via
|
||||
:meth:`AgentSkillFrontmatter.sanitize_skill_name` — the same helper
|
||||
:meth:`AgentSkillFrontmatter.skill_dir_name` uses for the directory
|
||||
segment. Sanitizing here (rather than handing the raw name to the
|
||||
frontmatter constructor) keeps ``frontmatter.name`` byte-identical to
|
||||
the on-disk directory name, and means a traversal-shaped LLM name
|
||||
(reachable via prompt injection, since the LLM's input is user
|
||||
conversation content) is made filesystem-safe *before* it reaches
|
||||
``AgentSkillFrontmatter``, instead of tripping the read-side traversal
|
||||
validator and dead-lettering the whole extraction run for a name the
|
||||
writer would have sanitized safely anyway.
|
||||
"""
|
||||
sanitized_name = AgentSkillFrontmatter.sanitize_skill_name(skill.name)
|
||||
frontmatter = AgentSkillFrontmatter(
|
||||
id=f"{agent_id}_{skill.name}",
|
||||
id=f"{agent_id}_{sanitized_name}",
|
||||
agent_id=agent_id,
|
||||
name=skill.name,
|
||||
name=sanitized_name,
|
||||
description=skill.description,
|
||||
confidence=skill.confidence,
|
||||
maturity_score=skill.maturity_score,
|
||||
|
|
@ -410,9 +557,10 @@ async def _persist_skill(
|
|||
)
|
||||
await writer.write_main(
|
||||
agent_id,
|
||||
skill.name,
|
||||
sanitized_name,
|
||||
frontmatter=frontmatter,
|
||||
body=skill.content,
|
||||
app_id=app_id,
|
||||
project_id=project_id,
|
||||
)
|
||||
return sanitized_name
|
||||
|
|
|
|||
|
|
@ -10,6 +10,36 @@ Per-owner batching: each sender's full foresight list is appended in
|
|||
one batched ``append_entries`` call rather than ``N`` single appends,
|
||||
dropping IO complexity to ``O(N)`` per owner and narrowing the
|
||||
per-path lock window.
|
||||
|
||||
**Disabled by default** (``enabled=False``). Not because it is broken —
|
||||
the crash it used to have is fixed below — but because it is one LLM
|
||||
call per sender per memcell whose output nothing in EverOS reads today:
|
||||
no search route surfaces foresights, no prompt slot consumes them. Until
|
||||
something does, running it by default spends tokens on write-only data.
|
||||
Re-enable per install in ``ome.toml`` (hot-reloaded, ~2s):
|
||||
|
||||
.. code-block:: toml
|
||||
|
||||
[strategies.extract_foresight]
|
||||
enabled = true
|
||||
|
||||
The opt-in has to actually work, so the sender scan below filters on
|
||||
``isinstance(m, ChatMessage)`` rather than reaching for ``m.role``.
|
||||
Only ``ChatMessage`` carries ``role``: ``ToolCallRequest`` has
|
||||
``sender_id`` without it, ``ToolCallResult`` has neither — so the old
|
||||
attribute test raised ``AttributeError`` on the first tool call, making
|
||||
the strategy correct on plain user chat and guaranteed to dead-letter on
|
||||
agent trajectories. everalgo contracts for exactly this mixed input
|
||||
(``user_memory/_render.chat_messages``: "the caller need not pre-filter;
|
||||
an AgentMemCell-shaped MemCell is acceptable input"), and every other
|
||||
user-memory extractor gets it for free by delegating; this strategy was
|
||||
the one place that hand-rolled the filter. A pure agent trajectory now
|
||||
yields no senders and returns without an LLM call.
|
||||
|
||||
Extraction granularity is a separate, still-open question: this runs per
|
||||
memcell, while ``atomic_fact`` runs per episode. Moving it needs an
|
||||
everalgo entry point that does not exist yet — unrelated to the crash,
|
||||
and not what the default-off above is about.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -17,6 +47,7 @@ from __future__ import annotations
|
|||
from collections import defaultdict
|
||||
from collections.abc import Mapping
|
||||
|
||||
from everalgo.types import ChatMessage
|
||||
from everalgo.user_memory import ForesightExtractor
|
||||
|
||||
from everos.component.llm import get_llm_client
|
||||
|
|
@ -47,11 +78,18 @@ def _get_writer() -> ForesightWriter:
|
|||
trigger=Immediate(on=[UserPipelineStarted]),
|
||||
emits=[],
|
||||
max_retries=2,
|
||||
enabled=False,
|
||||
)
|
||||
async def extract_foresight(event: UserPipelineStarted, ctx: StrategyContext) -> None:
|
||||
# 1. List the user senders in this memcell.
|
||||
memcell = event.memcell
|
||||
sender_ids = sorted({m.sender_id for m in memcell.items if m.role == "user"})
|
||||
sender_ids = sorted(
|
||||
{
|
||||
m.sender_id
|
||||
for m in memcell.items
|
||||
if isinstance(m, ChatMessage) and m.role == "user"
|
||||
}
|
||||
)
|
||||
extractor = ForesightExtractor(llm=get_llm_client()) if sender_ids else None
|
||||
|
||||
# 2. Run the LLM extractor once per sender (prompt is per-sender).
|
||||
|
|
|
|||
|
|
@ -134,6 +134,12 @@ async def trigger_skill_clustering(
|
|||
agent_id=event.agent_id,
|
||||
app_id=event.app_id,
|
||||
project_id=event.project_id,
|
||||
task_intent=event.task_intent,
|
||||
approach=event.approach,
|
||||
key_insight=event.key_insight,
|
||||
quality_score=event.quality_score,
|
||||
case_timestamp_ms=event.case_timestamp_ms,
|
||||
case_vector=vector_list,
|
||||
)
|
||||
)
|
||||
logger.info(
|
||||
|
|
|
|||
|
|
@ -2,7 +2,13 @@
|
|||
|
||||
Drives the full HTTP route through to storage, exercising the agent-track
|
||||
pipeline (boundary → memcell → extract_agent_case → trigger_skill_clustering
|
||||
→ extract_agent_skill) with real LLM and real embedder credentials.
|
||||
→ extract_agent_skill) with real LLM and real embedder credentials — this
|
||||
module's own ``_opt_in_real_embedding`` fixture opts the embedding
|
||||
capability back in (see its docstring for why that is necessary and why
|
||||
it does not weaken the global hermeticity fixture). Rerank is
|
||||
deliberately left at its hermetic default: nothing on this write path
|
||||
touches rerank, so opting it in would only widen the credential surface
|
||||
with no coverage benefit.
|
||||
|
||||
Mixed tenancy by design (sender_id alignment from fixture):
|
||||
|
||||
|
|
@ -24,21 +30,26 @@ White-box assertions (audit trail of internal surfaces touched):
|
|||
- sqlite ``memcell`` rows per session_id
|
||||
- filesystem ``<root>/agents/<agent>/.cases/*.md`` presence
|
||||
- LanceDB ``agent_case`` rows by ``owner_id`` (count + session_id set)
|
||||
- LanceDB ``agent_skill`` rows by ``owner_id`` (soft — LLM-dependent)
|
||||
- LanceDB ``agent_skill`` rows by ``owner_id`` (aggregate floor — see
|
||||
``test_agent_pipeline_e2e_mixed_tenancy``'s section 4.5)
|
||||
- OME ``run_record``: no dead-lettered ``extract_agent_skill`` run
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
from collections.abc import Awaitable, Callable
|
||||
from collections.abc import Awaitable, Callable, Iterator
|
||||
from pathlib import Path
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
import everos.component.embedding.accessor as _embedding_accessor
|
||||
from everos.infra.ome.records import RunStatus
|
||||
from everos.infra.persistence.lancedb import agent_case_repo, agent_skill_repo
|
||||
from everos.infra.persistence.markdown import AgentCaseDailyFrontmatter
|
||||
from everos.service.memorize import _get_engine
|
||||
|
||||
_FIXTURE_DIR = Path(__file__).resolve().parents[1] / "fixtures" / "agent_trajectories"
|
||||
|
||||
|
|
@ -64,6 +75,41 @@ _DRAIN_TIMEOUT_SECONDS = 300.0
|
|||
_DRAIN_INTER_ROUND_SLEEP_SECONDS = 5.0
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _opt_in_real_embedding(
|
||||
_reset_embedding_capability_singleton: None,
|
||||
) -> Iterator[None]:
|
||||
"""Opt this module's test into a real embedding capability.
|
||||
|
||||
``tests/conftest.py``'s ``_reset_embedding_capability_singleton``
|
||||
autouse fixture pins the capability to unavailable for every test
|
||||
(hermeticity); its docstring says a test may "explicitly opt in by
|
||||
re-assigning ``acc._capability``". This test needs it:
|
||||
``trigger_skill_clustering`` and ``extract_agent_skill`` both
|
||||
body-guard on ``get_embedding_capability().available`` and return
|
||||
early when it is false, so without opting in here the skill chain
|
||||
would never run — exactly the coverage gap this fixture closes.
|
||||
Scoped to this file only (not the global fixture) so every other
|
||||
test keeps its hermetic default.
|
||||
|
||||
Requesting ``_reset_embedding_capability_singleton`` as a parameter
|
||||
— rather than relying on collection/declaration order between this
|
||||
file's conftest chain and the root conftest — makes pytest's
|
||||
dependency graph guarantee this fixture's setup runs after it and its
|
||||
teardown before it. Rerank is left untouched (see the module
|
||||
docstring): this fixture reads and writes only the embedding
|
||||
capability, so there is nothing to order it against.
|
||||
|
||||
Setting ``_capability = None`` (rather than constructing a capability
|
||||
object directly) makes the accessor rebuild lazily from
|
||||
``load_settings()`` on next call, picking up the real ``.env``
|
||||
credentials ``tests/e2e/conftest.py`` loads at import time.
|
||||
"""
|
||||
_embedding_accessor._capability = None
|
||||
yield
|
||||
_embedding_accessor._capability = None
|
||||
|
||||
|
||||
def _load_fixture(session_id: str) -> dict:
|
||||
return json.loads((_FIXTURE_DIR / f"{session_id}.json").read_text())
|
||||
|
||||
|
|
@ -174,21 +220,56 @@ async def test_agent_pipeline_e2e_mixed_tenancy(
|
|||
f"want {set(_DJANGO_SESSIONS)}"
|
||||
)
|
||||
|
||||
# 4.5 agent_skill — soft: emission depends on LLM clustering quality
|
||||
# gate (skip_quality_threshold + cluster size). pytest/sympy are
|
||||
# single-case clusters and may legitimately yield 0 skills. django
|
||||
# has 3 cases and should aggregate into ≥1 cluster of size ≥2,
|
||||
# producing ≥1 skill — but we keep this informational (LLM-dependent)
|
||||
# rather than a hard floor to avoid flaky CI signal.
|
||||
# 4.5 agent_skill — aggregate floor across all three agents. Per-agent
|
||||
# emission depends on everalgo's per-case quality gate
|
||||
# (skip_quality_threshold, see everalgo/agent_memory/skill_ops.py) —
|
||||
# extract_agent_skill itself has no cluster-size gate, so a per-agent
|
||||
# floor would be genuinely flaky (a single low-quality trajectory can
|
||||
# legitimately yield 0 skills for that agent). Empirically, on this
|
||||
# branch with real credentials, 1 django trajectory alone produced 1
|
||||
# SKILL.md (extract_agent_skill status "success", no retries); driving
|
||||
# 5 trajectories across 3 agents should clear an aggregate floor of 1
|
||||
# even if any single agent's cluster is quality-gated to 0.
|
||||
pytest_skills = await agent_skill_repo.find_where(f"owner_id = '{_AGENT_PYTEST}'")
|
||||
sympy_skills = await agent_skill_repo.find_where(f"owner_id = '{_AGENT_SYMPY}'")
|
||||
django_skills = await agent_skill_repo.find_where(f"owner_id = '{_AGENT_DJANGO}'")
|
||||
# Hard sanity: counts non-negative (the repo isn't broken).
|
||||
assert len(pytest_skills) >= 0
|
||||
assert len(sympy_skills) >= 0
|
||||
assert len(django_skills) >= 0
|
||||
total_skills = len(pytest_skills) + len(sympy_skills) + len(django_skills)
|
||||
assert total_skills >= 1, (
|
||||
"agent-skill chain produced nothing — the strategy chain "
|
||||
"(extract_agent_case → trigger_skill_clustering → extract_agent_skill) "
|
||||
"is broken or gated off "
|
||||
f"(pytest={len(pytest_skills)}, sympy={len(sympy_skills)}, "
|
||||
f"django={len(django_skills)})"
|
||||
)
|
||||
|
||||
# 4.6 strict md ↔ LanceDB parity across every cascade kind
|
||||
# 4.6 no dead-lettered extract_agent_skill run. Sharper signal than
|
||||
# the skill-count floor above and targets this branch's actual defect
|
||||
# directly: a dead-letter means the chain attempted and failed
|
||||
# (exhausted retries), as opposed to a quality-gated 0-skill outcome,
|
||||
# which is not a failure.
|
||||
#
|
||||
# The dead-letter check alone is vacuous if the strategy never ran at
|
||||
# all — zero dead-letters is also what a never-executed strategy
|
||||
# looks like. Assert (any status) runs exist first, so the dead-letter
|
||||
# assertion below is only non-vacuous because of this check, not
|
||||
# because the skill-count floor in 4.5 happened to run first.
|
||||
engine = _get_engine()
|
||||
all_skill_runs = await engine.list_runs("extract_agent_skill")
|
||||
assert all_skill_runs, (
|
||||
"extract_agent_skill never ran at all — the dead-letter check "
|
||||
"below would be vacuously satisfied by a strategy that never "
|
||||
"executed"
|
||||
)
|
||||
|
||||
dead_letters = await engine.list_runs(
|
||||
"extract_agent_skill", status=RunStatus.DEAD_LETTER
|
||||
)
|
||||
assert not dead_letters, (
|
||||
"extract_agent_skill dead-lettered "
|
||||
f"{len(dead_letters)} run(s): {[r.error for r in dead_letters]}"
|
||||
)
|
||||
|
||||
# 4.7 strict md ↔ LanceDB parity across every cascade kind
|
||||
#
|
||||
# The per-owner counts above are loose (LLM-emission-dependent); this
|
||||
# check enforces byte-exact id-set + content_sha256 parity across
|
||||
|
|
|
|||
|
|
@ -141,12 +141,35 @@ async def test_emit_dispatches_both_strategies_to_success(
|
|||
|
||||
# Ensure the sqlite dir exists before the engine creates ome.db.
|
||||
(tmp_path / ".index" / "sqlite").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "ome.toml").write_text("# test\n")
|
||||
# `extract_foresight` now ships disabled (see its module docstring).
|
||||
# This test needs a `UserPipelineStarted` subscriber to cover the
|
||||
# second trigger route, so it opts back in through the very `ome.toml`
|
||||
# key that docstring points users at — which makes the opt-in path
|
||||
# itself covered, rather than working around the new default.
|
||||
(tmp_path / "ome.toml").write_text(
|
||||
"[strategies.extract_foresight]\nenabled = true\n"
|
||||
)
|
||||
await _setup_system_db_schema(monkeypatch)
|
||||
|
||||
engine = svc._get_engine()
|
||||
await engine.start()
|
||||
try:
|
||||
# `ConfigReloader.start()` fires its initial load as a task, so
|
||||
# `engine.start()` returns before `ome.toml` has been applied.
|
||||
# An emit inside that window is judged against the coded defaults
|
||||
# and silently dropped by the enabled gate — the event is not
|
||||
# redelivered once the override lands. Wait for the override to
|
||||
# be visible in the registry before emitting.
|
||||
for _ in range(50):
|
||||
if any(
|
||||
m.name == "extract_foresight" and m.enabled
|
||||
for m in engine._registry.all()
|
||||
):
|
||||
break
|
||||
await asyncio.sleep(0.1)
|
||||
else:
|
||||
pytest.fail("ome.toml override never reached the registry")
|
||||
|
||||
# Foresight still subscribes to UserPipelineStarted.
|
||||
await engine.emit(
|
||||
UserPipelineStarted(
|
||||
|
|
|
|||
|
|
@ -155,6 +155,39 @@ def test_skill_path_glob() -> None:
|
|||
assert _AgentSkill.path_glob() == "*/*/agents/*/skills/skill_*/SKILL.md"
|
||||
|
||||
|
||||
def test_skill_dir_name_sanitizes_traversal_payload() -> None:
|
||||
"""``skill_dir_name`` is the sanitization point ``AgentSkillWriter`` /
|
||||
``AgentSkillReader`` both derive from — a traversal payload must not
|
||||
survive into the directory segment.
|
||||
"""
|
||||
|
||||
class _AgentSkill(SkillPathMixin, AgentScopedFrontmatter):
|
||||
SKILLS_CONTAINER_NAME: ClassVar[str] = "skills"
|
||||
SKILL_DIR_PREFIX: ClassVar[str] = "skill_"
|
||||
SKILL_MAIN_FILENAME: ClassVar[str] = "SKILL.md"
|
||||
type: Literal["_agent_skill_dirname"] = "_agent_skill_dirname"
|
||||
|
||||
segment = _AgentSkill.skill_dir_name("../" * 8 + "tmp/pwned")
|
||||
assert segment.startswith("skill_")
|
||||
assert "/" not in segment
|
||||
assert "\\" not in segment
|
||||
# No separator survives, so the dots left behind form one opaque
|
||||
# component — not a ``..`` path-traversal segment.
|
||||
assert segment.split("/") == [segment]
|
||||
|
||||
|
||||
def test_skill_dir_name_preserves_cjk_and_spaces() -> None:
|
||||
class _AgentSkill(SkillPathMixin, AgentScopedFrontmatter):
|
||||
SKILLS_CONTAINER_NAME: ClassVar[str] = "skills"
|
||||
SKILL_DIR_PREFIX: ClassVar[str] = "skill_"
|
||||
SKILL_MAIN_FILENAME: ClassVar[str] = "SKILL.md"
|
||||
type: Literal["_agent_skill_dirname_cjk"] = "_agent_skill_dirname_cjk"
|
||||
|
||||
segment = _AgentSkill.skill_dir_name("修复 Django 自动重载问题")
|
||||
assert "修复" in segment
|
||||
assert "Django" in segment
|
||||
|
||||
|
||||
def test_strategy_mixin_overrides_base_via_mro() -> None:
|
||||
"""Strategy mixin placed first in the parent list wins over abstract base."""
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,156 @@
|
|||
"""Unit tests for :func:`sanitize_dirname` — the shared CWE-22 path-safety helper.
|
||||
|
||||
Pins the properties the callers rely on:
|
||||
|
||||
- traversal payloads collapse to a single opaque path component with no
|
||||
separator (the "no separator survives" half of the safety property);
|
||||
- short inputs that are themselves sanitizer fixpoints — bare ``".."``, or
|
||||
anything that strips down to ``".."`` / ``"."`` once a separator is
|
||||
removed — fall back rather than being returned as-is (the other half:
|
||||
without this, ``sanitize_dirname("../", fb)`` returns ``".."`` verbatim,
|
||||
which is a real one-level escape for a caller with no additional prefix
|
||||
protecting the segment, like ``KnowledgeWriter``);
|
||||
- non-ASCII input (CJK, spaces, NFD-decomposed accents) survives readably
|
||||
rather than being sanitized down to the empty-string fallback;
|
||||
- the function is idempotent, which is what lets a reader (deriving a name
|
||||
from an on-disk directory) and a writer (deriving it from raw input)
|
||||
agree on the same path — see :mod:`.test_frontmatter`'s
|
||||
``SkillPathMixin.skill_dir_name`` coverage for the consumer side.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import unicodedata
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from everos.core.persistence.markdown import sanitize_dirname
|
||||
|
||||
|
||||
def test_traversal_payload_has_no_separator() -> None:
|
||||
"""No path separator survives — a run of literal dots (``....``) is a
|
||||
single opaque filename component, not a ``..`` path-traversal segment,
|
||||
since there is no ``/`` left to divide it into components.
|
||||
"""
|
||||
payload = "../" * 8 + "tmp/pwned"
|
||||
sanitized = sanitize_dirname(payload, fallback="unnamed")
|
||||
|
||||
assert "/" not in sanitized
|
||||
assert "\\" not in sanitized
|
||||
assert sanitized != ".."
|
||||
|
||||
|
||||
def test_traversal_payload_resolved_path_stays_under_root(tmp_path: Path) -> None:
|
||||
payload = "../" * 8 + "tmp/pwned"
|
||||
sanitized = sanitize_dirname(payload, fallback="unnamed")
|
||||
|
||||
resolved = (tmp_path / sanitized).resolve()
|
||||
assert resolved.is_relative_to(tmp_path.resolve())
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw", ["..", "../", "/../", ".", "./"])
|
||||
def test_degenerate_fixpoints_fall_back_instead_of_escaping(raw: str) -> None:
|
||||
"""A short input that strips down to exactly ``".."`` or ``"."`` must
|
||||
fall back, not be returned as-is.
|
||||
|
||||
Regression guard for the actual bug: ``"."`` is a *safe* character
|
||||
(kept, not stripped), so ``"../"`` and ``"."``+``"/"`` both collapse to
|
||||
``".."`` / ``"."`` once the separator is removed — a fixpoint of the
|
||||
old (empty-only) fallback check, since neither is the empty string.
|
||||
Without this fallback, a caller with no extra prefix protecting the
|
||||
segment (``KnowledgeWriter``, unlike the skill writer's ``skill_``
|
||||
prefix) resolves one directory level up or sideways instead of into a
|
||||
new child.
|
||||
"""
|
||||
sanitized = sanitize_dirname(raw, fallback="unnamed")
|
||||
assert sanitized == "unnamed"
|
||||
|
||||
|
||||
def test_knowledge_style_unprefixed_concatenation_stays_under_root() -> None:
|
||||
"""The one-level escape the coordinator reproduced on the (unprefixed)
|
||||
knowledge path: ``Path(root) / sanitize_dirname("../", fb) / "doc_123"``
|
||||
must resolve under ``root``, not to ``root``'s parent.
|
||||
"""
|
||||
root = Path("/root/knowledge")
|
||||
resolved = root / sanitize_dirname("../", "Others") / "doc_123"
|
||||
assert resolved == Path("/root/knowledge/Others/doc_123")
|
||||
|
||||
|
||||
def test_nfc_normalizes_decomposed_accents() -> None:
|
||||
"""An NFD-decomposed accented character (base letter + combining mark)
|
||||
must sanitize to the same result as its NFC (precomposed) form —
|
||||
without normalization, the combining mark is not ``\\w`` and gets
|
||||
silently stripped, losing the accent instead of preserving it.
|
||||
"""
|
||||
nfc = "café"
|
||||
nfd = unicodedata.normalize("NFD", nfc)
|
||||
assert nfc != nfd # sanity: the two forms really are distinct strings
|
||||
|
||||
sanitized_nfc = sanitize_dirname(nfc, fallback="unnamed")
|
||||
sanitized_nfd = sanitize_dirname(nfd, fallback="unnamed")
|
||||
|
||||
assert sanitized_nfc == sanitized_nfd == "café"
|
||||
|
||||
|
||||
def test_nfc_does_not_help_composition_exclusions() -> None:
|
||||
"""Pins the documented exception: for Unicode "composition exclusion"
|
||||
codepoints, NFC normalization does not help — it decomposes an
|
||||
already-precomposed character, and the resulting combining mark is
|
||||
stripped either way.
|
||||
|
||||
U+0958 / U+0959 (Devanagari letters formed from a base letter + nukta)
|
||||
are composition exclusions: their canonical decomposition is excluded
|
||||
from NFC recomposition, so ``normalize("NFC", precomposed)`` yields the
|
||||
*decomposed* form, not the precomposed one.
|
||||
"""
|
||||
precomposed = "क़ख़"
|
||||
assert unicodedata.normalize("NFC", precomposed) != precomposed
|
||||
|
||||
sanitized = sanitize_dirname(precomposed, fallback="unnamed")
|
||||
|
||||
# The nukta (combining mark, U+093C) is lost: NFC decomposes the
|
||||
# precomposed input into base + nukta, and the nukta is then stripped
|
||||
# (not \w) -- the opposite of what NFC does for an ordinary NFD accent.
|
||||
assert sanitized == "कख"
|
||||
|
||||
|
||||
def test_cjk_and_space_input_preserved_readably() -> None:
|
||||
raw = "修复 Django 自动重载问题"
|
||||
sanitized = sanitize_dirname(raw, fallback="unnamed")
|
||||
|
||||
assert "修复" in sanitized
|
||||
assert "Django" in sanitized
|
||||
assert "_" in sanitized # spaces became underscores, not stripped
|
||||
assert " " not in sanitized
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"raw",
|
||||
[
|
||||
"../" * 8 + "tmp/pwned",
|
||||
"修复 Django 自动重载问题",
|
||||
"normal_skill",
|
||||
"../../etc/passwd",
|
||||
" ",
|
||||
"!!!@@@###",
|
||||
"..",
|
||||
"../",
|
||||
"/../",
|
||||
".",
|
||||
"./",
|
||||
],
|
||||
)
|
||||
def test_sanitize_is_idempotent(raw: str) -> None:
|
||||
once = sanitize_dirname(raw, fallback="unnamed")
|
||||
twice = sanitize_dirname(once, fallback="unnamed")
|
||||
assert once == twice
|
||||
|
||||
|
||||
def test_empty_result_falls_back() -> None:
|
||||
assert sanitize_dirname("!!!@@@###", fallback="unnamed") == "unnamed"
|
||||
|
||||
|
||||
def test_truncates_to_max_length() -> None:
|
||||
sanitized = sanitize_dirname("a" * 200, fallback="unnamed")
|
||||
assert len(sanitized) == 50
|
||||
|
|
@ -0,0 +1,131 @@
|
|||
"""Route tests for ``POST /api/v1/ome/trigger``.
|
||||
|
||||
Pins the widened ``TriggerResponse`` contract from Task 7: a ``dispatched``
|
||||
count and a per-run ``runs`` list are always present, and ``status`` gains
|
||||
``not_dispatched`` for strategies rejected by a dispatch gate (as opposed to
|
||||
``ok``/``timeout``, which mean the strategy was actually enqueued and either
|
||||
settled or didn't within the wait window).
|
||||
|
||||
Each test wires a bare FastAPI app carrying only the ``ome`` router to a
|
||||
real, started ``OfflineEngine`` (no lifespan, no LanceDB, no LLM) — the
|
||||
route's deferred ``_get_engine()`` import resolves to whatever
|
||||
``everos.service.memorize._ome_engine`` holds, which is patched per test.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
from collections.abc import AsyncIterator
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
|
||||
from everos.entrypoints.api.routes.ome import router as ome_router
|
||||
from everos.infra.ome.config import OMEConfig
|
||||
from everos.infra.ome.context import StrategyContext
|
||||
from everos.infra.ome.decorator import offline_strategy
|
||||
from everos.infra.ome.engine import OfflineEngine
|
||||
from everos.infra.ome.events import ManualTick
|
||||
from everos.infra.ome.triggers import Immediate
|
||||
|
||||
|
||||
async def _client_for(
|
||||
engine: OfflineEngine, monkeypatch: pytest.MonkeyPatch
|
||||
) -> AsyncClient:
|
||||
"""FastAPI app exposing only the ome router, wired to ``engine``."""
|
||||
svc = importlib.import_module("everos.service.memorize")
|
||||
monkeypatch.setattr(svc, "_ome_engine", engine, raising=False)
|
||||
app = FastAPI()
|
||||
app.include_router(ome_router, prefix="/api/v1")
|
||||
return AsyncClient(transport=ASGITransport(app=app), base_url="http://test")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def gated_off_engine(tmp_path: Path) -> AsyncIterator[OfflineEngine]:
|
||||
"""Engine with one strategy registered but ``enabled=False``."""
|
||||
|
||||
@offline_strategy(
|
||||
name="gated_off_strategy",
|
||||
trigger=Immediate(on=[ManualTick]),
|
||||
emits=[],
|
||||
enabled=False,
|
||||
)
|
||||
async def _s(event: ManualTick, ctx: StrategyContext) -> None:
|
||||
return None
|
||||
|
||||
engine = OfflineEngine(
|
||||
config=OMEConfig(jobstore_path=tmp_path / "ome.db", config_watch=False)
|
||||
)
|
||||
engine.register(_s)
|
||||
await engine.start()
|
||||
try:
|
||||
yield engine
|
||||
finally:
|
||||
await engine.stop()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def always_fails_engine(tmp_path: Path) -> AsyncIterator[OfflineEngine]:
|
||||
"""Engine with a strategy that raises unconditionally.
|
||||
|
||||
``max_retries=0`` reaches ``dead_letter`` on the very first attempt —
|
||||
no retry backoff sleep, so the test stays fast under the default runner.
|
||||
"""
|
||||
|
||||
@offline_strategy(
|
||||
name="always_fails",
|
||||
trigger=Immediate(on=[ManualTick]),
|
||||
emits=[],
|
||||
max_retries=0,
|
||||
)
|
||||
async def _s(event: ManualTick, ctx: StrategyContext) -> None:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
engine = OfflineEngine(
|
||||
config=OMEConfig(jobstore_path=tmp_path / "ome.db", config_watch=False)
|
||||
)
|
||||
engine.register(_s)
|
||||
await engine.start()
|
||||
try:
|
||||
yield engine
|
||||
finally:
|
||||
await engine.stop()
|
||||
|
||||
|
||||
async def test_trigger_returns_not_dispatched_when_strategy_gated_off(
|
||||
gated_off_engine: OfflineEngine, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A strategy disabled by config, triggered without ``force``, yields
|
||||
``dispatched=0`` and ``status='not_dispatched'`` with no runs."""
|
||||
async with await _client_for(gated_off_engine, monkeypatch) as client:
|
||||
resp = await client.post(
|
||||
"/api/v1/ome/trigger", json={"name": "gated_off_strategy"}
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert body["status"] == "not_dispatched"
|
||||
assert body["dispatched"] == 0
|
||||
assert body["runs"] == []
|
||||
|
||||
|
||||
async def test_trigger_returns_runs_including_dead_letter(
|
||||
always_fails_engine: OfflineEngine, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A strategy that raises through all retries dead-letters; the run
|
||||
still appears in ``runs`` with its error, and the top-level ``status``
|
||||
stays ``ok`` — dispatch happened and the run settled (dead-letter is a
|
||||
settled state, not an in-flight one)."""
|
||||
async with await _client_for(always_fails_engine, monkeypatch) as client:
|
||||
resp = await client.post(
|
||||
"/api/v1/ome/trigger",
|
||||
json={"name": "always_fails", "force": True, "timeout": 15},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert body["status"] == "ok"
|
||||
assert body["dispatched"] == 1
|
||||
assert len(body["runs"]) == 1
|
||||
assert body["runs"][0]["status"] == "dead_letter"
|
||||
assert body["runs"][0]["error"]
|
||||
|
|
@ -95,6 +95,75 @@ def test_skill_extra_fields_still_allowed() -> None:
|
|||
assert dumped["last_indexed_at"] == "2026-05-07T08:00:00Z"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"bad_name",
|
||||
[
|
||||
"../../../etc/passwd",
|
||||
"skills/../../escape",
|
||||
"a/b",
|
||||
"a\\b",
|
||||
"..",
|
||||
],
|
||||
)
|
||||
def test_skill_name_rejects_path_traversal(bad_name: str) -> None:
|
||||
"""Defence in depth: a hand-edited ``SKILL.md`` with a traversal-shaped
|
||||
``name`` is caught on parse rather than silently relocating the skill
|
||||
on the next write (see :mod:`.frontmatter`'s ``skill_dir_name``, which
|
||||
sanitizes the directory segment independently of this validator).
|
||||
"""
|
||||
with pytest.raises(ValidationError, match="path separators"):
|
||||
AgentSkillFrontmatter(**_kwargs(name=bad_name)) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_skill_name_allows_cjk_and_spaces() -> None:
|
||||
"""Non-ASCII / whitespace names are legitimate — only traversal shapes
|
||||
are rejected."""
|
||||
fm = AgentSkillFrontmatter(**_kwargs(name="修复 Django 自动重载问题")) # type: ignore[arg-type]
|
||||
assert fm.name == "修复 Django 自动重载问题"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"raw_name",
|
||||
[
|
||||
"..",
|
||||
"../",
|
||||
"/../",
|
||||
".",
|
||||
"./",
|
||||
"!!!", # sanitizes to empty -> fallback
|
||||
"a" * 200, # truncation
|
||||
"修复 Django 自动重载问题", # CJK + space
|
||||
"../" * 8 + "tmp/pwned",
|
||||
],
|
||||
)
|
||||
def test_frontmatter_accepts_presanitized_boundary_names(raw_name: str) -> None:
|
||||
"""Mirrors ``extract_agent_skill._persist_skill``'s write path: the
|
||||
caller sanitizes ``skill_name`` via
|
||||
:meth:`AgentSkillFrontmatter.sanitize_skill_name` *before* constructing
|
||||
the frontmatter, so a traversal-shaped or degenerate LLM name never
|
||||
reaches the validator above as a raw, unsanitized string — construction
|
||||
succeeds and the resulting ``name`` is a single, non-degenerate
|
||||
component, rather than raising and dead-lettering the extraction run
|
||||
for a name the sanitizer would have handled safely anyway.
|
||||
|
||||
Covers the boundary family that a single long traversal payload does
|
||||
not exercise: bare ``".."``, and inputs that strip down to ``".."`` or
|
||||
``"."`` once a leading/trailing separator is removed (``"." is a safe
|
||||
character, so it is not itself stripped) — these are sanitizer
|
||||
fixpoints, not just substrings, and are the exact inputs the fallback
|
||||
in ``sanitize_dirname`` exists to catch.
|
||||
"""
|
||||
sanitized = AgentSkillFrontmatter.sanitize_skill_name(raw_name)
|
||||
|
||||
assert "/" not in sanitized
|
||||
assert "\\" not in sanitized
|
||||
assert sanitized not in ("", ".", "..")
|
||||
|
||||
fm = AgentSkillFrontmatter(**_kwargs(name=sanitized))
|
||||
|
||||
assert fm.name == sanitized
|
||||
|
||||
|
||||
def test_skill_directory_shape_classvars() -> None:
|
||||
"""Path-shape ClassVars pin the wiki layout for the writer/reader pair."""
|
||||
assert AgentSkillFrontmatter.SKILLS_CONTAINER_NAME == "skills"
|
||||
|
|
|
|||
|
|
@ -127,3 +127,244 @@ async def test_read_script_round_trip(
|
|||
|
||||
async def test_read_script_returns_none_when_missing(reader: AgentSkillReader) -> None:
|
||||
assert await reader.read_script("agent_x", "alpha", "ghost.py") is None
|
||||
|
||||
|
||||
async def test_list_by_cluster_returns_matching_skills(
|
||||
writer: AgentSkillWriter, reader: AgentSkillReader
|
||||
) -> None:
|
||||
"""Enumerates SKILL.md under the agent, filters by frontmatter cluster_id."""
|
||||
await writer.write_main(
|
||||
"a1",
|
||||
"revive_replica",
|
||||
frontmatter=_make_fm(
|
||||
id="a1_revive_replica",
|
||||
agent_id="a1",
|
||||
name="revive_replica",
|
||||
cluster_id="cl1",
|
||||
),
|
||||
body="b",
|
||||
)
|
||||
await writer.write_main(
|
||||
"a1",
|
||||
"drain_queue",
|
||||
frontmatter=_make_fm(
|
||||
id="a1_drain_queue", agent_id="a1", name="drain_queue", cluster_id="cl1"
|
||||
),
|
||||
body="b",
|
||||
)
|
||||
await writer.write_main(
|
||||
"a1",
|
||||
"rotate_secrets",
|
||||
frontmatter=_make_fm(
|
||||
id="a1_rotate_secrets",
|
||||
agent_id="a1",
|
||||
name="rotate_secrets",
|
||||
cluster_id="cl2",
|
||||
),
|
||||
body="b",
|
||||
)
|
||||
|
||||
results = await reader.list_by_cluster("a1", "cl1")
|
||||
|
||||
names = sorted(fm.name for fm, _body in results)
|
||||
assert names == ["drain_queue", "revive_replica"]
|
||||
assert all(body == "b" for _fm, body in results)
|
||||
|
||||
|
||||
async def test_list_by_cluster_ignores_skills_without_cluster_id(
|
||||
writer: AgentSkillWriter, reader: AgentSkillReader
|
||||
) -> None:
|
||||
"""Skills whose frontmatter cluster_id is None never leak into any bucket."""
|
||||
await writer.write_main(
|
||||
"a1",
|
||||
"orphan",
|
||||
frontmatter=_make_fm(id="a1_orphan", agent_id="a1", name="orphan"),
|
||||
body="b",
|
||||
)
|
||||
await writer.write_main(
|
||||
"a1",
|
||||
"assigned",
|
||||
frontmatter=_make_fm(
|
||||
id="a1_assigned", agent_id="a1", name="assigned", cluster_id="cl1"
|
||||
),
|
||||
body="b",
|
||||
)
|
||||
|
||||
results = await reader.list_by_cluster("a1", "cl1")
|
||||
assert [fm.name for fm, _body in results] == ["assigned"]
|
||||
assert await reader.list_by_cluster("a1", "cl_missing") == []
|
||||
|
||||
|
||||
async def test_list_by_cluster_missing_dir_returns_empty(
|
||||
reader: AgentSkillReader,
|
||||
) -> None:
|
||||
"""New agent with no skill dir yet — returns [] without raising."""
|
||||
assert await reader.list_by_cluster("a_new", "cl1") == []
|
||||
|
||||
|
||||
async def test_list_by_cluster_finds_skill_whose_directory_suffix_has_a_space(
|
||||
root: MemoryRoot, reader: AgentSkillReader
|
||||
) -> None:
|
||||
"""Regression guard: before the fix, ``list_by_cluster`` recovered
|
||||
``skill_name`` from the directory suffix and called ``read_main``,
|
||||
which re-derives (and re-sanitizes) the path from that name. A
|
||||
directory whose suffix is not itself already a sanitizer fixpoint —
|
||||
e.g. ``skill_My Skill`` (a raw space, never passed through
|
||||
``sanitize_dirname``) — re-derived to ``skill_My_Skill``, a path that
|
||||
does not exist, so the skill was silently dropped from the result.
|
||||
``list_by_cluster`` is documented as the strong-consistency existence
|
||||
check for cluster membership, so a dropped skill here would make the
|
||||
LLM emit ``add()`` for a skill that already exists, duplicating it at
|
||||
the sanitized path and orphaning the original.
|
||||
|
||||
Writes the ``SKILL.md`` directly to the filesystem (bypassing both
|
||||
``AgentSkillWriter`` and ``_persist_skill``) to reproduce a directory
|
||||
that was never sanitized in the first place — the scenario the fix
|
||||
(reading the globbed path directly, never re-deriving it) must cover
|
||||
regardless of how such a directory came to exist. Asserts the body
|
||||
too, not just enumeration: a fix that stopped at "the frontmatter is
|
||||
found" but still forced a caller to re-read by name (re-derive, and
|
||||
re-sanitize, the same path) would have moved the drop one layer
|
||||
downstream rather than closing it — see
|
||||
``test_extract_agent_skill.test_select_existing_skills_finds_skill_whose_directory_suffix_has_a_space``
|
||||
for the end-to-end version of this same property.
|
||||
"""
|
||||
skill_dir = root.agents_dir() / "a1" / "skills" / "skill_My Skill"
|
||||
skill_dir.mkdir(parents=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"id: a1_My Skill\n"
|
||||
"type: agent_skill\n"
|
||||
"agent_id: a1\n"
|
||||
"track: agent\n"
|
||||
"name: My Skill\n"
|
||||
"description: d\n"
|
||||
"confidence: 0.5\n"
|
||||
"maturity_score: 0.5\n"
|
||||
"cluster_id: cl1\n"
|
||||
"---\n"
|
||||
"The real skill body.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
results = await reader.list_by_cluster("a1", "cl1")
|
||||
|
||||
assert len(results) == 1
|
||||
fm, body = results[0]
|
||||
assert fm.name == "My Skill"
|
||||
assert body == "The real skill body."
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"frontmatter_lines",
|
||||
[
|
||||
# Any schema constraint can fail, not just the traversal validator —
|
||||
# a field a later schema revision makes required is the case existing
|
||||
# files on disk would hit at upgrade time, all at once.
|
||||
pytest.param("name: broken\ncluster_id: cl1\n", id="missing_required_field"),
|
||||
# The traversal validator, whose whole stated purpose is catching a
|
||||
# hand-edited name — i.e. a file that reaches exactly this route.
|
||||
pytest.param(
|
||||
"name: ..\ndescription: d\nconfidence: 0.5\n"
|
||||
"maturity_score: 0.5\ncluster_id: cl1\n",
|
||||
id="traversal_name",
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_list_by_cluster_skips_unparseable_skill(
|
||||
root: MemoryRoot,
|
||||
reader: AgentSkillReader,
|
||||
frontmatter_lines: str,
|
||||
) -> None:
|
||||
"""One unparseable ``SKILL.md`` must not starve the whole cluster.
|
||||
|
||||
``_read_path`` validates the full :class:`AgentSkillFrontmatter` schema,
|
||||
so a single malformed file used to abort the entire enumeration — and
|
||||
the enumeration is what feeds ``extract_agent_skill`` its existing
|
||||
skills. Propagating would leave that strategy raising on every run for
|
||||
the whole cluster, i.e. permanently dead-lettered: the exact failure
|
||||
mode the md-first read path was introduced to eliminate. The good files
|
||||
on either side of the bad one pin that the skip is per-file rather than
|
||||
"stop at the first error" — sorted glob order puts ``skill_bad`` between
|
||||
them, so a fix that merely stopped raising would still lose ``zzz``.
|
||||
"""
|
||||
skills = root.agents_dir() / "a1" / "skills"
|
||||
|
||||
def _write(dirname: str, body_frontmatter: str, body: str) -> None:
|
||||
d = skills / dirname
|
||||
d.mkdir(parents=True)
|
||||
(d / "SKILL.md").write_text(
|
||||
f"---\ntype: agent_skill\nagent_id: a1\ntrack: agent\n"
|
||||
f"id: a1_{dirname}\n{body_frontmatter}---\n{body}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
good = "description: d\nconfidence: 0.5\nmaturity_score: 0.5\ncluster_id: cl1\n"
|
||||
_write("skill_aaa", f"name: aaa\n{good}", "body aaa")
|
||||
_write("skill_bad", frontmatter_lines, "body bad")
|
||||
_write("skill_zzz", f"name: zzz\n{good}", "body zzz")
|
||||
|
||||
results = await reader.list_by_cluster("a1", "cl1")
|
||||
|
||||
assert [fm.name for fm, _ in results] == ["aaa", "zzz"]
|
||||
assert [body for _, body in results] == ["body aaa", "body zzz"]
|
||||
|
||||
|
||||
async def test_read_main_propagates_validation_error(
|
||||
root: MemoryRoot, reader: AgentSkillReader
|
||||
) -> None:
|
||||
"""``read_main`` keeps raising — the skip is scoped to enumeration.
|
||||
|
||||
A caller naming one specific skill gets an error rather than ``None``:
|
||||
``None`` means "not created yet", a normal state this reader's callers
|
||||
branch on, and silently reusing it for "exists but is corrupt" would let
|
||||
an upsert overwrite the damaged file instead of surfacing it.
|
||||
"""
|
||||
skill_dir = root.agents_dir() / "a1" / "skills" / "skill_aaa"
|
||||
skill_dir.mkdir(parents=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\ntype: agent_skill\nagent_id: a1\ntrack: agent\n"
|
||||
"id: a1_aaa\nname: aaa\n---\nbody\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
with pytest.raises(ValidationError):
|
||||
await reader.read_main("a1", "aaa", schema=AgentSkillFrontmatter)
|
||||
|
||||
|
||||
async def test_read_main_rederivation_from_raw_name_is_idempotent_safe(
|
||||
writer: AgentSkillWriter, reader: AgentSkillReader
|
||||
) -> None:
|
||||
"""``read_main`` re-derives a path from a caller-supplied name; that
|
||||
re-derivation must land on the same file a direct write produced, even
|
||||
when the name it's given is raw and unsanitized.
|
||||
|
||||
No production caller currently re-derives a path from
|
||||
``list_by_cluster``'s output — it returns each skill's body directly
|
||||
(see ``AgentSkillReader.list_by_cluster``'s docstring), so
|
||||
``extract_agent_skill`` never calls ``read_main`` in that flow anymore.
|
||||
``read_main`` remains a general single-skill lookup on the reader's
|
||||
public API, so this test pins its re-derivation as a property of the
|
||||
method itself: writing via a *raw*, unsanitized name directly through
|
||||
the writer, then reading back via that same raw name, must resolve to
|
||||
the same file (idempotent-safe), for any future caller that does pass
|
||||
a raw name.
|
||||
"""
|
||||
space_name = "修复 Django 自动重载问题"
|
||||
await writer.write_main(
|
||||
"a1",
|
||||
space_name,
|
||||
frontmatter=_make_fm(
|
||||
id="a1_django_reload_fix",
|
||||
agent_id="a1",
|
||||
name=space_name,
|
||||
cluster_id="cl1",
|
||||
),
|
||||
body="The fix body.",
|
||||
)
|
||||
|
||||
out = await reader.read_main("a1", space_name, schema=AgentSkillFrontmatter)
|
||||
assert out is not None
|
||||
fm_out, body = out
|
||||
assert fm_out.name == space_name
|
||||
assert body == "The fix body."
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import pytest
|
|||
from everos.core.persistence import MarkdownReader, MemoryRoot
|
||||
from everos.infra.persistence.markdown import (
|
||||
AgentSkillFrontmatter,
|
||||
AgentSkillReader,
|
||||
AgentSkillWriter,
|
||||
)
|
||||
|
||||
|
|
@ -135,6 +136,74 @@ def test_main_path_does_not_create_anything(
|
|||
assert not root.agents_dir().exists()
|
||||
|
||||
|
||||
def test_main_path_sanitizes_traversal_skill_name(
|
||||
root: MemoryRoot, writer: AgentSkillWriter
|
||||
) -> None:
|
||||
"""A ``../``-laden ``skill_name`` (raw LLM output) must not escape the agent dir.
|
||||
|
||||
CWE-22 regression guard: prior to sanitization, ``skill_name`` was
|
||||
concatenated straight into the path, so a sufficiently long ``../``
|
||||
prefix resolved outside ``root.agents_dir()`` entirely. ``main_path``
|
||||
is a pure resolver (no frontmatter involved, no IO), matching how the
|
||||
traversal was originally measured.
|
||||
"""
|
||||
traversal_name = "../" * 8 + "tmp/pwned"
|
||||
|
||||
path = writer.main_path("agent_x", traversal_name)
|
||||
|
||||
assert path.resolve().is_relative_to(root.agents_dir().resolve())
|
||||
assert path.name == "SKILL.md"
|
||||
assert "/" not in path.parent.name
|
||||
assert path.parent.parent == root.agents_dir() / "agent_x" / "skills"
|
||||
|
||||
|
||||
_BOUNDARY_RAW_NAMES = [
|
||||
"..",
|
||||
"../",
|
||||
"/../",
|
||||
".",
|
||||
"./",
|
||||
"!!!", # sanitizes to empty -> fallback
|
||||
"a" * 200, # truncation
|
||||
"修复 Django 自动重载问题", # CJK + space
|
||||
"../" * 8 + "tmp/pwned",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw_name", _BOUNDARY_RAW_NAMES)
|
||||
async def test_presanitized_name_identical_to_directory_segment(
|
||||
root: MemoryRoot, writer: AgentSkillWriter, raw_name: str
|
||||
) -> None:
|
||||
"""Mirrors ``extract_agent_skill._persist_skill``: sanitize
|
||||
``skill_name`` once, up front, then use that same sanitized string for
|
||||
both the frontmatter ``name`` field and the writer's ``skill_name``
|
||||
argument. Covers the boundary family that previously slipped through
|
||||
the sanitizer as a fixpoint (``".."`` alone, or with a leading/trailing
|
||||
separator that strips down to it; ``"."`` likewise) in addition to the
|
||||
empty/truncation/CJK/traversal cases already covered.
|
||||
|
||||
For each input: the sanitized name is a single path component
|
||||
(contains no separator), is never ``""`` / ``"."`` / ``".."``,
|
||||
constructing ``AgentSkillFrontmatter`` with it succeeds, and
|
||||
``frontmatter.name`` is byte-identical (an identity, not merely
|
||||
idempotent-if-resanitized) to the directory segment actually written.
|
||||
"""
|
||||
sanitized_name = AgentSkillFrontmatter.sanitize_skill_name(raw_name)
|
||||
|
||||
assert "/" not in sanitized_name
|
||||
assert "\\" not in sanitized_name
|
||||
assert sanitized_name not in ("", ".", "..")
|
||||
|
||||
fm = _make_fm(name=sanitized_name, id=f"agent_x_{sanitized_name}")
|
||||
|
||||
path = await writer.write_main("agent_x", sanitized_name, frontmatter=fm, body="b")
|
||||
|
||||
dir_derived_name = path.parent.name.removeprefix(
|
||||
AgentSkillFrontmatter.SKILL_DIR_PREFIX
|
||||
)
|
||||
assert fm.name == dir_derived_name
|
||||
|
||||
|
||||
async def test_write_main_normalises_trailing_newline(
|
||||
root: MemoryRoot, writer: AgentSkillWriter
|
||||
) -> None:
|
||||
|
|
@ -145,3 +214,58 @@ async def test_write_main_normalises_trailing_newline(
|
|||
root.agents_dir() / "agent_x" / "skills" / "skill_alpha" / "SKILL.md"
|
||||
).read_text(encoding="utf-8")
|
||||
assert text.endswith("no-newline-end\n")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("reference_name", "script_filename"),
|
||||
[
|
||||
pytest.param("../" * 6 + "etc/passwd", "../" * 6 + "evil.sh", id="traversal"),
|
||||
pytest.param("..", "..", id="dotdot_fixpoint"),
|
||||
pytest.param("", "", id="empty"),
|
||||
pytest.param("notes/../../x", "run/../../x.sh", id="embedded_separators"),
|
||||
],
|
||||
)
|
||||
async def test_reference_and_script_segments_cannot_escape_the_skill_dir(
|
||||
root: MemoryRoot,
|
||||
writer: AgentSkillWriter,
|
||||
reference_name: str,
|
||||
script_filename: str,
|
||||
) -> None:
|
||||
"""These two segments are appended *after* ``skill_dir_name``.
|
||||
|
||||
``skill_dir_name`` only sanitizes the ``skill_<name>`` component, so it
|
||||
offers these no protection at all — they need their own pass through
|
||||
``sanitize_dirname``. Nothing in ``src/`` calls them yet; they are
|
||||
covered now because they are public API whose inputs will come from the
|
||||
same untrusted place the skill name does once progressive disclosure is
|
||||
wired up, and because the traversal fix would otherwise read as
|
||||
repo-wide when it is not.
|
||||
"""
|
||||
skill_dir = root.agents_dir() / "agent_x" / "skills" / "skill_alpha"
|
||||
|
||||
ref = await writer.write_reference("agent_x", "alpha", reference_name, "x")
|
||||
script = await writer.write_script("agent_x", "alpha", script_filename, "x")
|
||||
|
||||
for path in (ref, script):
|
||||
assert path.is_relative_to(skill_dir)
|
||||
assert ".." not in path.parts
|
||||
assert path.is_file()
|
||||
|
||||
|
||||
async def test_reader_resolves_the_same_sanitized_reference_and_script_paths(
|
||||
root: MemoryRoot, writer: AgentSkillWriter
|
||||
) -> None:
|
||||
"""Reader and writer must sanitize every segment identically.
|
||||
|
||||
Sanitizing only one side would silently split a write from its matching
|
||||
read — the write lands on the safe path, the read looks at the raw one
|
||||
and reports the file missing. This is the same reader/writer symmetry
|
||||
``skill_dir_name`` maintains for the skill directory, extended to the
|
||||
two segments appended after it.
|
||||
"""
|
||||
reader = AgentSkillReader(root)
|
||||
await writer.write_reference("agent_x", "alpha", "my notes!", "ref body")
|
||||
await writer.write_script("agent_x", "alpha", "run this.sh", "echo hi\n")
|
||||
|
||||
assert await reader.read_reference("agent_x", "alpha", "my notes!") == "ref body"
|
||||
assert await reader.read_script("agent_x", "alpha", "run this.sh") == "echo hi"
|
||||
|
|
|
|||
|
|
@ -218,6 +218,59 @@ async def test_empty_slug_fallback_for_title(tmp_path: Path) -> None:
|
|||
assert (doc_dir / "index.md").is_file()
|
||||
|
||||
|
||||
async def test_decomposed_accent_survives_in_directory_name(tmp_path: Path) -> None:
|
||||
"""A decomposed (NFD) title keeps its accent in the directory name.
|
||||
|
||||
Pins a deliberate behavior change: this writer's private sanitizer was
|
||||
replaced by the shared ``core.persistence.markdown.path_safety``
|
||||
primitive, which NFC-normalizes before filtering. The character class
|
||||
(``[^\\w\\-.]``, Unicode-aware) is unchanged, so precomposed input —
|
||||
including CJK — resolved identically before and after; NFD input did
|
||||
not. A bare combining mark is not ``\\w``, so ``"e"`` + U+0301 used to
|
||||
lose the accent and land in ``Résumé_…`` → ``Resume_…``.
|
||||
|
||||
This is the one directory-name change with a pre-existing corpus behind
|
||||
it: knowledge upload shipped before this fix, so a document whose topic
|
||||
arrived decomposed resolves to a *different* directory now than the one
|
||||
already on disk. Kept because titles reaching here are overwhelmingly
|
||||
precomposed already; pinned because nothing else in the suite would
|
||||
notice a regression back to the stripping behavior.
|
||||
"""
|
||||
nfd_topic = "Re" + "́" + "sume" + "́" # "Résumé", decomposed
|
||||
doc_id = "d_nfd0001"
|
||||
doc_dir = await KnowledgeWriter.write(
|
||||
[_root_node(doc_id=doc_id, topic=nfd_topic)], tmp_path
|
||||
)
|
||||
|
||||
assert doc_dir == tmp_path / "Sports" / f"Résumé_{doc_id}"
|
||||
assert (doc_dir / "index.md").is_file()
|
||||
|
||||
|
||||
async def test_dot_only_topic_falls_back_instead_of_escaping(tmp_path: Path) -> None:
|
||||
"""A ``".."`` topic must not resolve the document dir to its parent.
|
||||
|
||||
``".."`` is a fixpoint of the character filter (``.`` is a safe
|
||||
character), so before the fallback on degenerate results it survived
|
||||
intact — and because this writer appends no prefix of its own, the
|
||||
resulting ``<knowledge_dir>/<category>/.._<doc_id>`` was one literal
|
||||
component but ``<category>`` itself was not, letting a ``".."``
|
||||
*category* climb a level. Asserts containment rather than the exact
|
||||
fallback string: the property that matters is that no path component
|
||||
can walk back out of ``tmp_path``. Both assertions are needed and
|
||||
neither implies the other — ``is_relative_to`` is prefix arithmetic
|
||||
that a ``".."`` component would satisfy while still escaping, and the
|
||||
``parts`` check alone says nothing about where the path is rooted.
|
||||
"""
|
||||
doc_id = "d_dots001"
|
||||
doc_dir = await KnowledgeWriter.write(
|
||||
[_root_node(doc_id=doc_id, topic="..", category_id="..")], tmp_path
|
||||
)
|
||||
|
||||
assert doc_dir.is_relative_to(tmp_path)
|
||||
assert ".." not in doc_dir.parts
|
||||
assert (doc_dir / "index.md").is_file()
|
||||
|
||||
|
||||
async def test_empty_category_id_fallback_to_others(tmp_path: Path) -> None:
|
||||
memories = [_root_node(category_id="")]
|
||||
doc_dir = await KnowledgeWriter.write(memories, tmp_path)
|
||||
|
|
|
|||
|
|
@ -9,8 +9,9 @@ import pytest
|
|||
from everos.infra.ome._dispatch.runner import Runner
|
||||
from everos.infra.ome._stores.run_record import RunRecordStore
|
||||
from everos.infra.ome._stores.storage import OMEStorage
|
||||
from everos.infra.ome.config import OMEConfig
|
||||
from everos.infra.ome.context import StrategyContext
|
||||
from everos.infra.ome.decorator import offline_strategy
|
||||
from everos.infra.ome.decorator import StrategyMeta, offline_strategy
|
||||
from everos.infra.ome.events import BaseEvent
|
||||
from everos.infra.ome.records import RunStatus
|
||||
from everos.infra.ome.triggers import Immediate
|
||||
|
|
@ -26,12 +27,13 @@ async def setup(tmp_path: Path):
|
|||
await storage.init()
|
||||
rec_store = RunRecordStore(storage=storage, max_records_per_strategy=1000)
|
||||
sem = asyncio.Semaphore(20)
|
||||
return rec_store, sem
|
||||
config = OMEConfig(jobstore_path=tmp_path / "ome.db")
|
||||
return rec_store, sem, config
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_success_marks_record(setup) -> None:
|
||||
rec_store, sem = setup
|
||||
rec_store, sem, config = setup
|
||||
|
||||
@offline_strategy(name="ok", trigger=Immediate(on=[_E]), emits=[])
|
||||
async def s(event: _E, ctx: StrategyContext) -> None:
|
||||
|
|
@ -41,6 +43,7 @@ async def test_runner_success_marks_record(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
await runner.run(
|
||||
|
|
@ -55,8 +58,18 @@ async def test_runner_success_marks_record(setup) -> None:
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_retries_on_failure(setup) -> None:
|
||||
rec_store, sem = setup
|
||||
async def test_runner_retries_on_failure(
|
||||
setup, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
# Retry loop now sleeps between attempts (real backoff config from
|
||||
# ``setup``); fake the sleep so this test stays fast — it asserts
|
||||
# retry *counting*, not backoff timing (see test_runner_applies_
|
||||
# exponential_backoff_between_attempts for that).
|
||||
monkeypatch.setattr(
|
||||
"everos.infra.ome._dispatch.runner.asyncio.sleep",
|
||||
_instant_sleep,
|
||||
)
|
||||
rec_store, sem, config = setup
|
||||
calls = {"n": 0}
|
||||
|
||||
@offline_strategy(
|
||||
|
|
@ -74,6 +87,7 @@ async def test_runner_retries_on_failure(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
await runner.run(
|
||||
|
|
@ -94,8 +108,14 @@ async def test_runner_retries_on_failure(setup) -> None:
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_dead_letter_after_exhaust(setup) -> None:
|
||||
rec_store, sem = setup
|
||||
async def test_runner_dead_letter_after_exhaust(
|
||||
setup, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setattr(
|
||||
"everos.infra.ome._dispatch.runner.asyncio.sleep",
|
||||
_instant_sleep,
|
||||
)
|
||||
rec_store, sem, config = setup
|
||||
|
||||
@offline_strategy(
|
||||
name="bad",
|
||||
|
|
@ -112,6 +132,7 @@ async def test_runner_dead_letter_after_exhaust(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
on_dead_letter=lambda r: dl_calls.append(r),
|
||||
engine=MagicMock(),
|
||||
)
|
||||
|
|
@ -131,7 +152,7 @@ async def test_runner_dead_letter_after_exhaust(setup) -> None:
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_emit_must_be_declared(setup) -> None:
|
||||
rec_store, sem = setup
|
||||
rec_store, sem, config = setup
|
||||
|
||||
class _Other(BaseEvent):
|
||||
pass
|
||||
|
|
@ -148,6 +169,7 @@ async def test_runner_emit_must_be_declared(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
await runner.run(
|
||||
|
|
@ -167,7 +189,7 @@ async def test_runner_negative_max_retries_raises(setup) -> None:
|
|||
constrains the user-supplied source to ``>= 0``), so the framework
|
||||
fails fast rather than silently no-op the run.
|
||||
"""
|
||||
rec_store, sem = setup
|
||||
rec_store, sem, config = setup
|
||||
|
||||
@offline_strategy(name="ok", trigger=Immediate(on=[_E]), emits=[])
|
||||
async def s(event: _E, ctx: StrategyContext) -> None:
|
||||
|
|
@ -177,6 +199,7 @@ async def test_runner_negative_max_retries_raises(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
with pytest.raises(ValueError, match=r"max_retries_snapshot must be >= 0"):
|
||||
|
|
@ -198,7 +221,7 @@ async def test_runner_aborts_silently_when_mark_running_fails(
|
|||
crash recovery to pick up, so re-execution via recovery is
|
||||
impossible. The emergency log is the only audit trail.
|
||||
"""
|
||||
rec_store, sem = setup
|
||||
rec_store, sem, config = setup
|
||||
called = {"n": 0}
|
||||
|
||||
@offline_strategy(name="ok", trigger=Immediate(on=[_E]), emits=[])
|
||||
|
|
@ -214,6 +237,7 @@ async def test_runner_aborts_silently_when_mark_running_fails(
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
# Must NOT raise; the framework swallows + logs.
|
||||
|
|
@ -230,11 +254,17 @@ async def _no_emit(event: BaseEvent) -> None:
|
|||
return None
|
||||
|
||||
|
||||
async def _instant_sleep(seconds: float) -> None:
|
||||
"""Drop-in ``asyncio.sleep`` replacement for tests that exercise the
|
||||
retry loop but don't care about backoff timing."""
|
||||
return None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_emits_ome_agent_span(setup) -> None:
|
||||
"""Runner wraps the strategy body in an everos.ome.<name> agent span
|
||||
(its own trace — runs in an APScheduler task, no request context)."""
|
||||
rec_store, sem = setup
|
||||
rec_store, sem, config = setup
|
||||
|
||||
@offline_strategy(name="traced_strat", trigger=Immediate(on=[_E]), emits=[])
|
||||
async def s(event: _E, ctx: StrategyContext) -> None:
|
||||
|
|
@ -263,6 +293,7 @@ async def test_runner_emits_ome_agent_span(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
await runner.run(s.meta, _E(), run_id="r_trace", max_retries_snapshot=1)
|
||||
|
|
@ -282,7 +313,7 @@ async def test_runner_emits_ome_agent_span(setup) -> None:
|
|||
async def test_runner_ome_span_links_to_upstream_traceparent(setup) -> None:
|
||||
"""Given a traceparent (captured where a request span was active), the
|
||||
everos.ome.<name> span nests under that upstream trace, not a new root."""
|
||||
rec_store, sem = setup
|
||||
rec_store, sem, config = setup
|
||||
|
||||
@offline_strategy(name="linked_strat", trigger=Immediate(on=[_E]), emits=[])
|
||||
async def s(event: _E, ctx: StrategyContext) -> None:
|
||||
|
|
@ -318,6 +349,7 @@ async def test_runner_ome_span_links_to_upstream_traceparent(setup) -> None:
|
|||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
await runner.run(
|
||||
|
|
@ -332,3 +364,165 @@ async def test_runner_ome_span_links_to_upstream_traceparent(setup) -> None:
|
|||
]
|
||||
assert ome.context.trace_id == parent_tid # same trace as the request
|
||||
assert ome.parent is not None # child, not a fresh root
|
||||
|
||||
|
||||
async def _make_runner_with_transient_failing_strategy(
|
||||
tmp_path: Path,
|
||||
*,
|
||||
max_retries: int,
|
||||
backoff_base: float = 1.0,
|
||||
backoff_cap: float = 10.0,
|
||||
jitter: float = 0.5,
|
||||
) -> tuple[Runner, StrategyMeta]:
|
||||
"""Build a ``Runner`` wired to a strategy that raises on every attempt.
|
||||
|
||||
Drives the retry loop to exhaustion so the backoff sleep fires between
|
||||
each of the ``max_retries`` retries, for tests asserting on
|
||||
``asyncio.sleep`` call arguments.
|
||||
"""
|
||||
storage = OMEStorage(db_path=tmp_path / "ome.db")
|
||||
await storage.init()
|
||||
rec_store = RunRecordStore(storage=storage, max_records_per_strategy=1000)
|
||||
sem = asyncio.Semaphore(20)
|
||||
config = OMEConfig(
|
||||
jobstore_path=tmp_path / "ome.db",
|
||||
retry_backoff_base_seconds=backoff_base,
|
||||
retry_backoff_cap_seconds=backoff_cap,
|
||||
retry_jitter_seconds=jitter,
|
||||
)
|
||||
|
||||
@offline_strategy(
|
||||
name="transient_failing",
|
||||
trigger=Immediate(on=[_E]),
|
||||
emits=[],
|
||||
max_retries=max_retries,
|
||||
)
|
||||
async def s(event: _E, ctx: StrategyContext) -> None:
|
||||
raise RuntimeError("transient")
|
||||
|
||||
runner = Runner(
|
||||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
return runner, s.meta
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_applies_exponential_backoff_between_attempts(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Each attempt after the first sleeps ~ base * 2**(attempt-1), capped at
|
||||
cap_seconds, with up to jitter_seconds added. Verifies that a strategy
|
||||
raising a transient exception gets real wall-clock breathing room."""
|
||||
sleeps: list[float] = []
|
||||
|
||||
async def fake_sleep(seconds: float) -> None:
|
||||
sleeps.append(seconds)
|
||||
|
||||
monkeypatch.setattr("everos.infra.ome._dispatch.runner.asyncio.sleep", fake_sleep)
|
||||
|
||||
runner, meta = await _make_runner_with_transient_failing_strategy(
|
||||
tmp_path, max_retries=3
|
||||
)
|
||||
await runner.run(meta, _E(), run_id="r1", max_retries_snapshot=3)
|
||||
|
||||
# attempts 1, 2, 3 (0 has no preceding sleep); base=1, cap=10, jitter=0.5
|
||||
assert len(sleeps) == 3
|
||||
assert 1.0 <= sleeps[0] <= 1.5 # ~1s + jitter
|
||||
assert 2.0 <= sleeps[1] <= 2.5 # ~2s + jitter
|
||||
assert 4.0 <= sleeps[2] <= 4.5 # ~4s + jitter
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_releases_engine_sem_across_backoff_sleep(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""The concurrency slot must be free while a run waits to retry.
|
||||
|
||||
``engine_sem`` bounds concurrent strategy *work* — LLM calls,
|
||||
embeddings, storage IO — and a coroutine sleeping between attempts
|
||||
consumes none of it. Holding the slot across the sleep turns a
|
||||
partial outage into a total stall: with N slots and a 1s/2s/4s
|
||||
backoff, N simultaneously-failing runs park every slot in
|
||||
``asyncio.sleep`` and starve strategies that would have succeeded.
|
||||
|
||||
Uses a single-permit semaphore so ``locked()`` is unambiguous, and
|
||||
asserts a second waiter actually acquires — ``locked()`` alone would
|
||||
pass on an implementation that released the slot but left a waiter
|
||||
unable to take it.
|
||||
"""
|
||||
storage = OMEStorage(db_path=tmp_path / "ome.db")
|
||||
await storage.init()
|
||||
rec_store = RunRecordStore(storage=storage, max_records_per_strategy=1000)
|
||||
sem = asyncio.Semaphore(1)
|
||||
config = OMEConfig(
|
||||
jobstore_path=tmp_path / "ome.db",
|
||||
retry_backoff_base_seconds=1.0,
|
||||
retry_backoff_cap_seconds=10.0,
|
||||
retry_jitter_seconds=0.0,
|
||||
)
|
||||
|
||||
held_during_sleep: list[bool] = []
|
||||
acquired_during_sleep: list[bool] = []
|
||||
|
||||
async def fake_sleep(seconds: float) -> None:
|
||||
held_during_sleep.append(sem.locked())
|
||||
try:
|
||||
async with asyncio.timeout(0.5):
|
||||
await sem.acquire()
|
||||
except TimeoutError:
|
||||
acquired_during_sleep.append(False)
|
||||
else:
|
||||
acquired_during_sleep.append(True)
|
||||
sem.release()
|
||||
|
||||
monkeypatch.setattr("everos.infra.ome._dispatch.runner.asyncio.sleep", fake_sleep)
|
||||
|
||||
@offline_strategy(
|
||||
name="sem_probe_failing",
|
||||
trigger=Immediate(on=[_E]),
|
||||
emits=[],
|
||||
max_retries=2,
|
||||
)
|
||||
async def s(event: _E, ctx: StrategyContext) -> None:
|
||||
raise RuntimeError("transient")
|
||||
|
||||
runner = Runner(
|
||||
run_record_store=rec_store,
|
||||
engine_sem=sem,
|
||||
emit_hook=_no_emit,
|
||||
config=config,
|
||||
engine=MagicMock(),
|
||||
)
|
||||
await runner.run(s.meta, _E(), run_id="r1", max_retries_snapshot=2)
|
||||
|
||||
assert held_during_sleep == [False, False]
|
||||
assert acquired_during_sleep == [True, True]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_backoff_caps_at_configured_maximum(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Backoff cap prevents unbounded growth on high max_retries."""
|
||||
sleeps: list[float] = []
|
||||
|
||||
async def fake_sleep(seconds: float) -> None:
|
||||
sleeps.append(seconds)
|
||||
|
||||
monkeypatch.setattr("everos.infra.ome._dispatch.runner.asyncio.sleep", fake_sleep)
|
||||
|
||||
runner, meta = await _make_runner_with_transient_failing_strategy(
|
||||
tmp_path,
|
||||
max_retries=6,
|
||||
backoff_base=1.0,
|
||||
backoff_cap=3.0,
|
||||
jitter=0.0,
|
||||
)
|
||||
await runner.run(meta, _E(), run_id="r1", max_retries_snapshot=6)
|
||||
|
||||
# 1, 2, 3, 3, 3, 3 -- all capped after attempt 3
|
||||
assert sleeps == [1.0, 2.0, 3.0, 3.0, 3.0, 3.0]
|
||||
|
|
|
|||
|
|
@ -4,7 +4,12 @@ import pydantic
|
|||
import pytest
|
||||
from everalgo.types import ChatMessage, MemCell
|
||||
|
||||
from everos.memory.events import AgentPipelineStarted, UserPipelineStarted
|
||||
from everos.memory.events import (
|
||||
AgentCaseExtracted,
|
||||
AgentPipelineStarted,
|
||||
SkillClusterUpdated,
|
||||
UserPipelineStarted,
|
||||
)
|
||||
|
||||
|
||||
def _sample_memcell() -> MemCell:
|
||||
|
|
@ -83,3 +88,44 @@ def test_user_pipeline_started_nested_roundtrip_json() -> None:
|
|||
assert restored.memcell.items[0].id == "m1"
|
||||
assert restored.memcell.items[1].content == "hi back"
|
||||
assert restored.memcell.timestamp == 1_700_000_001_000
|
||||
|
||||
|
||||
def test_agent_case_extracted_new_fields_default() -> None:
|
||||
"""approach/key_insight default so a pre-1.2.3 event payload deserializes."""
|
||||
payload = {
|
||||
"memcell_id": "m1",
|
||||
"case_entry_id": "c1",
|
||||
"task_intent": "cook risotto",
|
||||
"quality_score": 0.8,
|
||||
"case_timestamp_ms": 1_700_000_000_000,
|
||||
"agent_id": "a1",
|
||||
}
|
||||
event = AgentCaseExtracted.model_validate(payload)
|
||||
assert event.approach == ""
|
||||
assert event.key_insight is None
|
||||
|
||||
|
||||
def test_skill_cluster_updated_new_fields_default() -> None:
|
||||
"""All 6 pass-through fields default; a pre-1.2.3 payload deserializes."""
|
||||
payload = {"case_entry_id": "c1", "cluster_id": "cl1", "agent_id": "a1"}
|
||||
event = SkillClusterUpdated.model_validate(payload)
|
||||
assert event.task_intent == ""
|
||||
assert event.approach == ""
|
||||
assert event.key_insight is None
|
||||
assert event.quality_score == 0.0
|
||||
assert event.case_timestamp_ms == 0
|
||||
assert event.case_vector is None
|
||||
|
||||
|
||||
def test_skill_cluster_updated_carries_case_vector() -> None:
|
||||
"""When set, case_vector round-trips through JSON serialization."""
|
||||
payload = {
|
||||
"case_entry_id": "c1",
|
||||
"cluster_id": "cl1",
|
||||
"agent_id": "a1",
|
||||
"case_vector": [0.1, 0.2, 0.3],
|
||||
}
|
||||
event = SkillClusterUpdated.model_validate_json(
|
||||
SkillClusterUpdated.model_validate(payload).model_dump_json()
|
||||
)
|
||||
assert event.case_vector == [0.1, 0.2, 0.3]
|
||||
|
|
|
|||
|
|
@ -1,7 +1,16 @@
|
|||
"""Unit tests for ``memory.search.agentic_agent``.
|
||||
|
||||
White-box: patches ``aagentic_retrieve`` to assert benchmark hyperparameters
|
||||
are wired correctly, plus a shaping test to verify DTOs are built correctly.
|
||||
Two groups of tests:
|
||||
|
||||
* White-box (patches ``aagentic_retrieve``): assert benchmark hyperparameters
|
||||
are wired correctly, plus a shaping test to verify DTOs are built
|
||||
correctly. These never execute the real ``everalgo._format_docs`` /
|
||||
rerank_fn wiring — they are dead coverage for the metadata bridge.
|
||||
* Black-box (does NOT patch ``aagentic_retrieve``): exercises the real
|
||||
``_format_docs`` prompt-rendering path and the real kind-shaped rerank_fn
|
||||
via ``everalgo.testing.fake_llm.FakeLLMClient``. These pin the regression
|
||||
fixed by the metadata-bridge refactor (empty-description skill -> 500)
|
||||
and the skill/case rerank-fn swap.
|
||||
|
||||
The skill verify step has been removed from production code; this test
|
||||
module covers the agentic retrieve flow only.
|
||||
|
|
@ -10,6 +19,7 @@ module covers the agentic retrieve flow only.
|
|||
from __future__ import annotations
|
||||
|
||||
import datetime as _dt
|
||||
import json
|
||||
from typing import Any, ClassVar
|
||||
from unittest.mock import patch
|
||||
|
||||
|
|
@ -17,10 +27,15 @@ from everalgo.rank.protocols import AgenticDecision
|
|||
from everalgo.testing.fake_llm import FakeLLMClient
|
||||
from everalgo.types import Candidate
|
||||
|
||||
from everos.component.rerank import RerankResult
|
||||
from everos.memory.search.agentic_agent import (
|
||||
search_agent_cases_agentic,
|
||||
search_agent_skills_agentic,
|
||||
)
|
||||
from everos.memory.search.callbacks import (
|
||||
_CASE_RERANK_INSTRUCTION,
|
||||
_SKILL_RERANK_INSTRUCTION,
|
||||
)
|
||||
from everos.memory.search.dto import SearchAgentCaseItem, SearchAgentSkillItem
|
||||
|
||||
# ── Stubs ────────────────────────────────────────────────────────────────
|
||||
|
|
@ -270,3 +285,211 @@ async def test_search_agent_skills_agentic_shapes_result() -> None:
|
|||
assert isinstance(result[0], SearchAgentSkillItem)
|
||||
assert result[0].id == "s_1"
|
||||
assert result[0].name == "skill_s_1"
|
||||
|
||||
|
||||
# ── Black-box tests: real _format_docs + real kind-shaped rerank_fn ────────
|
||||
#
|
||||
# These deliberately do NOT patch ``aagentic_retrieve`` (unlike every test
|
||||
# above), so the metadata bridge (``_to_everalgo_doc_metadata``) and the
|
||||
# kind-shaped ``rerank_fn`` (``build_skill_rerank_fn`` / ``build_case_rerank_fn``)
|
||||
# actually run.
|
||||
|
||||
# JSON body a real LLM would return for the sufficiency-check prompt; parsed
|
||||
# by ``everalgo.rank.agentic._call_llm_for_sufficiency``. ``is_sufficient=True``
|
||||
# short-circuits Round 2, so one LLM call is enough for every test below.
|
||||
_SUFFICIENT_LLM_RESPONSE = json.dumps(
|
||||
{
|
||||
"is_sufficient": True,
|
||||
"reasoning": "single relevant candidate",
|
||||
"key_information_found": [],
|
||||
"missing_information": [],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
class _StubSkillRecallerAsym:
|
||||
"""Like ``_StubSkillRecaller`` but with independently controllable
|
||||
dense/sparse routes, needed to pin an exact fused score."""
|
||||
|
||||
kind: ClassVar[str] = "agent_skill"
|
||||
everalgo_memory_type: ClassVar[str] = "skill"
|
||||
text_field: ClassVar[str] = "description"
|
||||
|
||||
def __init__(
|
||||
self, *, dense: list[Candidate], sparse: list[Candidate] | None = None
|
||||
) -> None:
|
||||
self._dense = dense
|
||||
self._sparse = sparse if sparse is not None else list(dense)
|
||||
|
||||
async def sparse_recall(self, *_: Any, **__: Any) -> list[Candidate]:
|
||||
return list(self._sparse)
|
||||
|
||||
async def dense_recall(self, *_: Any, **__: Any) -> list[Candidate]:
|
||||
return list(self._dense)
|
||||
|
||||
|
||||
class _StubCaseRecallerAsym:
|
||||
"""Case-kind counterpart of :class:`_StubSkillRecallerAsym`."""
|
||||
|
||||
kind: ClassVar[str] = "agent_case"
|
||||
everalgo_memory_type: ClassVar[str] = "case"
|
||||
text_field: ClassVar[str] = "task_intent"
|
||||
|
||||
def __init__(
|
||||
self, *, dense: list[Candidate], sparse: list[Candidate] | None = None
|
||||
) -> None:
|
||||
self._dense = dense
|
||||
self._sparse = sparse if sparse is not None else list(dense)
|
||||
|
||||
async def sparse_recall(self, *_: Any, **__: Any) -> list[Candidate]:
|
||||
return list(self._sparse)
|
||||
|
||||
async def dense_recall(self, *_: Any, **__: Any) -> list[Candidate]:
|
||||
return list(self._dense)
|
||||
|
||||
|
||||
class _IdentityReranker:
|
||||
"""Rerank stub that preserves input order; accepts ``instruction`` like
|
||||
a real :class:`RerankProvider` (unlike ``_StubReranker`` above, which
|
||||
only satisfies the white-box tests where rerank_fn is never called)."""
|
||||
|
||||
async def rerank(
|
||||
self, query: str, passages: list[str], *, instruction: str | None = None
|
||||
) -> list[RerankResult]:
|
||||
return [RerankResult(index=i, score=1.0) for i in range(len(passages))]
|
||||
|
||||
|
||||
def _skill_metadata(*, name: str, description: str) -> dict[str, Any]:
|
||||
return {
|
||||
"owner_id": "agent_a",
|
||||
"owner_type": "agent",
|
||||
"name": name,
|
||||
"description": description,
|
||||
"content": "some remediation content",
|
||||
"confidence": 0.9,
|
||||
"maturity_score": 0.6,
|
||||
"source_case_ids": [],
|
||||
}
|
||||
|
||||
|
||||
async def test_agentic_survives_name_only_skill() -> None:
|
||||
"""A skill with an empty description is a valid everalgo output (see
|
||||
everalgo ``agent_memory/skill_ops.py`` — the guard is ``if not name and
|
||||
not description``). The metadata bridge must produce a non-empty
|
||||
passage for ``_format_docs``, otherwise everalgo raises ``ValueError``
|
||||
and the request 500s. This is the regression test for that defect."""
|
||||
skill = Candidate(
|
||||
id="s_name_only",
|
||||
score=0.9,
|
||||
source="vector",
|
||||
metadata=_skill_metadata(name="rotate_secrets", description=""),
|
||||
)
|
||||
recaller = _StubSkillRecallerAsym(dense=[skill])
|
||||
|
||||
result = await search_agent_skills_agentic(
|
||||
"how to rotate credentials",
|
||||
where="owner_id = 'agent_a' AND owner_type = 'agent'",
|
||||
skill_recaller=recaller,
|
||||
embed_query_fn=_fake_embed,
|
||||
reranker=_IdentityReranker(),
|
||||
llm=FakeLLMClient(responses=[_SUFFICIENT_LLM_RESPONSE]),
|
||||
top_k=5,
|
||||
)
|
||||
|
||||
assert len(result) == 1
|
||||
assert result[0].name == "rotate_secrets"
|
||||
|
||||
|
||||
async def test_agentic_uses_skill_rerank_passage() -> None:
|
||||
"""Rerank input passages must be the skill-shaped multi-field format
|
||||
(``build_skill_rerank_fn``'s ``"Agent Skill: {name} - {description}"``),
|
||||
not the raw single-field text the generic ``build_rerank_fn`` would use.
|
||||
Proves the rerank_fn swap in ``_run_agentic_retrieve`` actually happened."""
|
||||
captured_passages: list[str] = []
|
||||
captured_instruction: str | None = None
|
||||
|
||||
class _SpyReranker:
|
||||
async def rerank(
|
||||
self,
|
||||
query: str,
|
||||
passages: list[str],
|
||||
*,
|
||||
instruction: str | None = None,
|
||||
) -> list[RerankResult]:
|
||||
nonlocal captured_instruction
|
||||
captured_passages[:] = passages
|
||||
captured_instruction = instruction
|
||||
return [RerankResult(index=i, score=1.0) for i in range(len(passages))]
|
||||
|
||||
skill = Candidate(
|
||||
id="s_revive",
|
||||
score=0.9,
|
||||
source="vector",
|
||||
metadata=_skill_metadata(name="revive_replica", description="restart node"),
|
||||
)
|
||||
recaller = _StubSkillRecallerAsym(dense=[skill])
|
||||
|
||||
await search_agent_skills_agentic(
|
||||
"how to bring a replica back",
|
||||
where="owner_id = 'agent_a' AND owner_type = 'agent'",
|
||||
skill_recaller=recaller,
|
||||
embed_query_fn=_fake_embed,
|
||||
reranker=_SpyReranker(),
|
||||
llm=FakeLLMClient(responses=[_SUFFICIENT_LLM_RESPONSE]),
|
||||
top_k=5,
|
||||
)
|
||||
|
||||
assert captured_passages == ["Agent Skill: revive_replica - restart node"]
|
||||
assert captured_instruction == _SKILL_RERANK_INSTRUCTION
|
||||
|
||||
|
||||
async def test_agentic_uses_case_rerank_passage() -> None:
|
||||
"""Symmetric to ``test_agentic_uses_skill_rerank_passage`` for the
|
||||
agent_case kind: passages must be ``"Agent Case: {task_intent} -
|
||||
{approach}"`` with ``_CASE_RERANK_INSTRUCTION``."""
|
||||
captured_passages: list[str] = []
|
||||
captured_instruction: str | None = None
|
||||
|
||||
class _SpyReranker:
|
||||
async def rerank(
|
||||
self,
|
||||
query: str,
|
||||
passages: list[str],
|
||||
*,
|
||||
instruction: str | None = None,
|
||||
) -> list[RerankResult]:
|
||||
nonlocal captured_instruction
|
||||
captured_passages[:] = passages
|
||||
captured_instruction = instruction
|
||||
return [RerankResult(index=i, score=1.0) for i in range(len(passages))]
|
||||
|
||||
case = Candidate(
|
||||
id="c_restart",
|
||||
score=0.9,
|
||||
source="vector",
|
||||
metadata={
|
||||
"owner_id": "agent_a",
|
||||
"owner_type": "agent",
|
||||
"session_id": "sess_b",
|
||||
"timestamp": _ts(),
|
||||
"task_intent": "restart the pod",
|
||||
"approach": "kubectl rollout restart",
|
||||
"quality_score": 0.8,
|
||||
},
|
||||
)
|
||||
recaller = _StubCaseRecallerAsym(dense=[case])
|
||||
|
||||
await search_agent_cases_agentic(
|
||||
"how to restart a stuck pod",
|
||||
where="owner_id = 'agent_a' AND owner_type = 'agent'",
|
||||
case_recaller=recaller,
|
||||
embed_query_fn=_fake_embed,
|
||||
reranker=_SpyReranker(),
|
||||
llm=FakeLLMClient(responses=[_SUFFICIENT_LLM_RESPONSE]),
|
||||
top_k=5,
|
||||
)
|
||||
|
||||
assert captured_passages == [
|
||||
"Agent Case: restart the pod - kubectl rollout restart"
|
||||
]
|
||||
assert captured_instruction == _CASE_RERANK_INSTRUCTION
|
||||
|
|
|
|||
|
|
@ -225,6 +225,44 @@ async def test_fans_out_per_assistant_sender(
|
|||
assert matching[0]["fanout"] == 2
|
||||
|
||||
|
||||
async def test_emit_includes_approach_and_key_insight(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""1.2.3+ emitters populate the new case-body fields on AgentCaseExtracted."""
|
||||
monkeypatch.setattr(mod, "_writer", None, raising=False)
|
||||
case = _algo_case(
|
||||
task_intent="restore MongoDB replica",
|
||||
approach="1. stop node 2. resync 3. verify",
|
||||
quality_score=0.75,
|
||||
key_insight="watch oplog lag",
|
||||
)
|
||||
with (
|
||||
patch(
|
||||
"everos.memory.strategies.extract_agent_case.get_llm_client",
|
||||
return_value=object(),
|
||||
),
|
||||
patch(
|
||||
"everos.memory.strategies.extract_agent_case.AgentCaseExtractor"
|
||||
) as mock_cls,
|
||||
patch(
|
||||
"everos.memory.strategies.extract_agent_case.AgentCaseWriter"
|
||||
) as mock_wcls,
|
||||
):
|
||||
mock_cls.return_value.aextract = AsyncMock(return_value=[case])
|
||||
mock_wcls.return_value.append_entry = AsyncMock(return_value=_fake_eid())
|
||||
ctx = FakeStrategyContext()
|
||||
|
||||
await extract_agent_case(_event(), ctx)
|
||||
|
||||
emitted = [e for e in ctx.emitted if isinstance(e, AgentCaseExtracted)]
|
||||
assert len(emitted) == 1
|
||||
event = emitted[0]
|
||||
assert event.approach == "1. stop node 2. resync 3. verify"
|
||||
assert event.key_insight == "watch oplog lag"
|
||||
assert event.task_intent == "restore MongoDB replica"
|
||||
assert event.quality_score == 0.75
|
||||
|
||||
|
||||
async def test_omits_key_insight_section_when_empty(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -5,7 +5,15 @@ from unittest.mock import AsyncMock, patch
|
|||
|
||||
import pytest
|
||||
import structlog.testing
|
||||
from everalgo.types import ChatMessage, Foresight, MemCell
|
||||
from everalgo.types import (
|
||||
ChatMessage,
|
||||
Foresight,
|
||||
MemCell,
|
||||
ToolCall,
|
||||
ToolCallFunction,
|
||||
ToolCallRequest,
|
||||
ToolCallResult,
|
||||
)
|
||||
|
||||
from everos.infra.ome.testing import FakeStrategyContext
|
||||
from everos.memory.events import UserPipelineStarted
|
||||
|
|
@ -229,3 +237,105 @@ async def test_skips_when_memcell_has_no_messages(
|
|||
assert matching, "log line should still fire (count=0)"
|
||||
assert matching[0]["count"] == 0
|
||||
mock_wcls.return_value.append_entries.assert_not_called()
|
||||
|
||||
|
||||
# ── mixed agent/user memcells (tool calls) ──────────────────────────────
|
||||
|
||||
|
||||
def _tool_call_memcell(*, with_user_message: bool) -> MemCell:
|
||||
"""A memcell shaped the way an agent trajectory arrives.
|
||||
|
||||
``ToolCallRequest`` carries ``sender_id`` but no ``role``;
|
||||
``ToolCallResult`` carries neither. Only ``ChatMessage`` has ``role``,
|
||||
which is why a bare ``m.role`` test raised on the first tool call.
|
||||
"""
|
||||
items: list[object] = [
|
||||
ToolCallRequest(
|
||||
id="t1",
|
||||
sender_id="agent",
|
||||
timestamp=1_700_000_000_000,
|
||||
tool_calls=[
|
||||
ToolCall(
|
||||
id="c1",
|
||||
function=ToolCallFunction(name="read_file", arguments="{}"),
|
||||
)
|
||||
],
|
||||
),
|
||||
ToolCallResult(
|
||||
id="t2",
|
||||
timestamp=1_700_000_001_000,
|
||||
tool_call_id="c1",
|
||||
content="file contents",
|
||||
),
|
||||
]
|
||||
if with_user_message:
|
||||
items.insert(
|
||||
0,
|
||||
ChatMessage(
|
||||
id="m1",
|
||||
role="user",
|
||||
content="please fix the autoreloader",
|
||||
timestamp=1_699_999_999_000,
|
||||
sender_id="u_alice",
|
||||
),
|
||||
)
|
||||
return MemCell(items=items, timestamp=1_700_000_000_000)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("with_user_message", "expected_senders"),
|
||||
[
|
||||
pytest.param(False, [], id="pure_agent_trajectory"),
|
||||
pytest.param(True, ["u_alice"], id="mixed_user_and_tool_calls"),
|
||||
],
|
||||
)
|
||||
async def test_tool_calls_do_not_crash_the_sender_scan(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
with_user_message: bool,
|
||||
expected_senders: list[str],
|
||||
) -> None:
|
||||
"""A memcell holding tool calls must not raise, and must not over-extract.
|
||||
|
||||
Regression guard: the scan used to read ``m.role`` off every item, so
|
||||
the first ``ToolCallRequest`` raised ``AttributeError`` — before any
|
||||
sender was resolved, before any LLM call. That made the strategy sound
|
||||
on plain user chat and guaranteed to dead-letter on agent
|
||||
trajectories, which is the shape ``/add`` receives from an agent
|
||||
session. everalgo contracts for the mixed case
|
||||
(``user_memory/_render.chat_messages``), so the fix is to honour that
|
||||
contract rather than pre-filter by hand.
|
||||
|
||||
Both directions are pinned: a pure agent trajectory extracts nothing
|
||||
and never reaches the LLM, and a mixed memcell extracts for the human
|
||||
senders only — an implementation that merely stopped raising but
|
||||
scanned tool-call ``sender_id`` values would invent ``"agent"`` as a
|
||||
user.
|
||||
"""
|
||||
event = UserPipelineStarted(
|
||||
memcell_id="mc_tool",
|
||||
session_id="s1",
|
||||
memcell=_tool_call_memcell(with_user_message=with_user_message),
|
||||
)
|
||||
monkeypatch.setattr(mod, "_writer", None, raising=False)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"everos.memory.strategies.extract_foresight.get_llm_client",
|
||||
return_value=object(),
|
||||
),
|
||||
patch(
|
||||
"everos.memory.strategies.extract_foresight.ForesightExtractor"
|
||||
) as mock_cls,
|
||||
patch(
|
||||
"everos.memory.strategies.extract_foresight.ForesightWriter"
|
||||
) as mock_wcls,
|
||||
):
|
||||
mock_cls.return_value.aextract = AsyncMock(return_value=[])
|
||||
mock_wcls.return_value.append_entries = AsyncMock(return_value=[])
|
||||
await extract_foresight(event, FakeStrategyContext())
|
||||
|
||||
called_senders = [
|
||||
call.kwargs["sender_id"]
|
||||
for call in mock_cls.return_value.aextract.await_args_list
|
||||
]
|
||||
assert called_senders == expected_senders
|
||||
|
|
|
|||
|
|
@ -54,11 +54,15 @@ def _event(
|
|||
agent_id: str = "agent_42",
|
||||
task_intent: str = "summarise the doc",
|
||||
case_timestamp_ms: int = 1_700_000_001_000,
|
||||
approach: str = "",
|
||||
key_insight: str | None = None,
|
||||
) -> AgentCaseExtracted:
|
||||
return AgentCaseExtracted(
|
||||
memcell_id="mc_a",
|
||||
case_entry_id=case_entry_id,
|
||||
task_intent=task_intent,
|
||||
approach=approach,
|
||||
key_insight=key_insight,
|
||||
quality_score=quality_score,
|
||||
case_timestamp_ms=case_timestamp_ms,
|
||||
agent_id=agent_id,
|
||||
|
|
@ -170,6 +174,62 @@ async def test_creates_new_cluster_when_no_existing(
|
|||
assert emitted[0].agent_id == "agent_42"
|
||||
|
||||
|
||||
async def test_emit_passes_through_case_body_and_vector(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""1.2.3+ trigger_skill_clustering passes case body verbatim and includes
|
||||
the embedding it just computed so extract_agent_skill doesn't need to
|
||||
re-embed for the top-k branch.
|
||||
"""
|
||||
fake_vector = [0.1] * 1024
|
||||
embedder = MagicMock()
|
||||
embedder.embed = AsyncMock(return_value=fake_vector)
|
||||
_install_embedder(monkeypatch, embedder)
|
||||
ctx = FakeStrategyContext()
|
||||
|
||||
incoming = _event(
|
||||
task_intent="restore replica",
|
||||
approach="stop, resync, verify",
|
||||
key_insight="watch oplog",
|
||||
quality_score=0.8,
|
||||
case_timestamp_ms=1_700_000_000_000,
|
||||
agent_id="a1",
|
||||
case_entry_id="c1",
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"everos.memory.strategies.trigger_skill_clustering.get_llm_client",
|
||||
return_value=object(),
|
||||
),
|
||||
patch(
|
||||
"everos.memory.strategies.trigger_skill_clustering.cluster_repo"
|
||||
) as mock_repo,
|
||||
patch(
|
||||
"everos.memory.strategies.trigger_skill_clustering.cluster_by_llm",
|
||||
new=AsyncMock(return_value=None),
|
||||
),
|
||||
patch(
|
||||
"everos.memory.strategies.trigger_skill_clustering.mint_cluster_id",
|
||||
return_value="cl_newxxxx0001",
|
||||
),
|
||||
):
|
||||
mock_repo.list_for_owner = AsyncMock(return_value=[])
|
||||
mock_repo.upsert_with_members = AsyncMock(return_value=None)
|
||||
|
||||
await trigger_skill_clustering(incoming, ctx)
|
||||
|
||||
emitted = [e for e in ctx.emitted if isinstance(e, SkillClusterUpdated)]
|
||||
assert len(emitted) == 1
|
||||
ev = emitted[0]
|
||||
assert ev.task_intent == "restore replica"
|
||||
assert ev.approach == "stop, resync, verify"
|
||||
assert ev.key_insight == "watch oplog"
|
||||
assert ev.quality_score == 0.8
|
||||
assert ev.case_timestamp_ms == 1_700_000_000_000
|
||||
assert ev.case_vector == fake_vector
|
||||
|
||||
|
||||
async def test_merges_into_existing_cluster_when_algo_matches(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
|
|
|
|||
Loading…
Reference in New Issue