fix(procedures): retire soft-deletes on read and stop updates duplicating - #17
Merged
Merged
Conversation
…ting Two procedure-lifecycle defects, both previously pinned in the integration suite behind KNOWN SERVER DEFECT comments rather than fixed. 1. Soft-deleted/expired procedures kept resurfacing. Procedures soft-delete (and naturally expire) by stamping `expiresAt`, but `ProcedureRepository`'s read paths had no expiry predicate, so retired procedures came straight back on the next read. `ResearchRepository.list` already carries this exact predicate (added 2026-07-20); it was never applied to procedures. Add it to `list`, and — so the leak can't just move to `/recall` — to `listSimilar` and `fullTextSearch` as well. 2. A body-changing update duplicated the procedure. `ProcedureService.update` wrote the new body onto the original node AND created a superseding clone, retiring neither, leaving two identical live nodes behind every edit (the count grew with each edit). Rewrite the body-change path to the two-node supersession model already used by Facts/Preferences: keep the old node's body intact and retire it, create a new node at v+1 carrying the new body, so only the new node stays live and the version chain remains walkable via :SUPERSEDES and audit. `ProcedureRepository.supersede` now retires the old node (sets `expiresAt`), mirroring `FactRepository.supersede` setting `validTo`. Metadata-only updates still revise in place on the same id. Add `scripts/backfill-procedure-dedup.ts` to clean up procedures duplicated by the old behavior: it collapses each :SUPERSEDES-connected lineage to a single live node (highest version, tie-broken by newest id) and retires the rest. It is idempotent and safe to re-run. Run it once, after deploying this fix. Flip the pinned assertions in the mcp/openclaw integration specs to assert the corrected behavior (retired procedures leave the listing; a body edit yields exactly one new live node).
gigabyte22
added a commit
that referenced
this pull request
Aug 6, 2026
…lugin (#32) * feat(hermes): package the adapter as an installable hermes-elephant plugin The adapter was a directory you were told to copy into a hermes checkout. This makes it a real distribution, and fixes two things that stopped the documented setup from working at all. Packaging - Distribution `hermes-elephant`, import package `hermes_elephant`. The import package is deliberately not `elephant`: that name belongs to the Electrophysiology Analysis Toolkit on PyPI and a top-level collision would break anyone with both installed. The provider is still `elephant` everywhere a user sees it — `memory.provider`, `hermes elephant`, and the installed plugin directory. - `hermes-elephant install` copies the provider into `$HERMES_HOME/plugins/elephant/`. hermes discovers memory providers by directory scan only, never pip entry points (upstream #40101), so pip alone leaves it invisible. Same two-step shape the Memori provider ships. - Declares the `hermes_agent.memory_providers` entry point so the install step becomes redundant once upstream discovery lands. Deliberately does NOT join `hermes_agent.plugins`: the general PluginManager eagerly imports that group in every hermes process and then calls register(ctx) on a context with no register_memory_provider. - Stdlib-only, asserted in the tests — installing must never add a package to the hermes runtime. Fixes - `hermes elephant ...` was dead for every directory install. hermes registers a user-installed provider's package as a synthetic shell with no __file__, so cli.py's `from . import DEFAULT_URL` raised ImportError and discover_plugin_cli_commands() returned nothing — while plugin.yaml and the README advertised the subcommands. Shared names moved to `_shared.py`; submodule imports resolve through the shell's __path__. Regression test drives the real loader shape and fails against the old import. - Episodes now carry `origin`. Without it the dreamer treats every transcript as human conversation, so a subagent's machine-written task text was mined for facts *about the operator* — elephant's extraction prompt keys off this field to suppress exactly that. Integration - `on_delegation` records a subagent's task and result against the parent session, the last unimplemented write hook. - `config_schema.py` gives the provider a native hermes dashboard panel; validated against hermes's real ProviderField/ProviderConfigSchema. - `plugin.yaml` states `kind: exclusive` rather than relying on hermes's source-text auto-coercion. - README documents wiring elephant's pull-only `:Intention` nodes to hermes cron — elephant never fires them, and hermes is the scheduler. * test(hermes): fix the procedure live test against supersession semantics test_procedure_round_trip re-read the ORIGINAL id after a body update and expected the new content. That stopped being true when #17 replaced in-place procedure updates with supersession: a body change now creates a NEW node at v+1 and retires the old one, leaving its body intact and walkable via :SUPERSEDES. The adapter already reports the new id back to the model, so nothing was broken except the test's expectation. Now asserts the actual invariant — the edit is at the new id, the old id still answers with v1, and lookup by name follows the live version so an agent holding only the name is not stranded. Full live suite against a real service: 127 passed, 1 skipped. * refactor(hermes): make the shipped provider a directory, not an allowlist Three review findings, all about the installer promising more than it verified. Shipped set is now a directory boundary. The provider moved to hermes_elephant/provider/ and install copies that tree wholesale, so copy mode, --link mode and the built wheel are the same files by construction. The hand-maintained PLUGIN_FILES allowlist and the test holding it in agreement are both gone — the test could only ever compare the allowlist against the source checkout, never against the wheel it was meant to guard. --link also stops exposing install.py to hermes's submodule exec, which was an untested asymmetry between dev and ship. Install verifies the discovery contract. hermes classifies a provider directory by scanning the first 8KB of its __init__.py for MemoryProvider / register_memory_provider. A packaging gap used to print a warning and then report success, leaving the user to reconcile "Installed" here with "Plugin: NOT installed" there. Now it fails, cleans up, and says why — and pins an upstream constraint that is otherwise invisible. status detects a stale copy. Copying rather than symlinking means `pip install -U hermes-elephant` leaves the profile running the old version with nothing to notice. The marker now records the version and status reports STALE. Also: - hermes_home() defers to hermes_constants.get_hermes_home() when importable, and otherwise honours %LOCALAPPDATA%\hermes on Windows. It previously hardcoded ~/.hermes, which on Windows installs where nothing scans and then reports success. - _is_ours() only claims a symlink that resolves to our own package, so uninstall cannot unlink one the user made pointing at their own tree. - __pycache__ and dotfiles are never copied into the plugin directory. Verified against a real elephant on a throwaway Neo4j testcontainer: 129 passed, 1 skipped. Entry-point discovery re-checked end to end against a hermes checkout — pip-only install still resolves its directory, config panel, CLI, and 34 tools.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two procedure-lifecycle defects that were previously pinned in the integration suite behind
KNOWN SERVER DEFECTcomments rather than fixed. Both are unfixed onmainand both are real:Soft-deleted / expired procedures kept resurfacing. Procedures soft-delete (and naturally expire) by stamping
expiresAt, butProcedureRepository's read paths had no expiry predicate, so retired procedures came straight back on the next read.ResearchRepository.listalready carries this exact predicate (added 2026-07-20 for the same bug); it was simply never applied to procedures.A body-changing update duplicated the procedure.
ProcedureService.updatewrote the new body onto the original node and created a superseding clone, retiring neither — leaving two identical live nodes behind every edit. The duplicate count grows with each edit, so a live graph accumulates them over time.What changed
1. Filter expired procedures on read (
ProcedureRepository)expiresAt IS NULL OR expiresAt > datetime()tolist, mirroringResearchRepository.list.listSimilarandfullTextSearchso the leak can't simply move fromlistto/recall.2. Two-node supersession on update (
ProcedureService.update+ProcedureRepository.supersede)Rewrote the body-change path to the model already used by Facts and Preferences:
expiresAt = now) — it is no longer overwritten.v+1carrying the new body; only it stays live.:SUPERSEDES(and audit).ProcedureRepository.supersedenow retires the old node, mirroringFactRepository.supersedesettingvalidTo.expiresAt) still revise in place on the same id.3. Backfill for existing duplicates (
scripts/backfill-procedure-dedup.ts)One-shot, idempotent cleanup for procedures duplicated by the old behavior:
:SUPERSEDESedge into lineages (union-find, no APOC dependency).pnpm backfill:procedure-dedup.4. Tests
Flipped the pinned
KNOWN SERVER DEFECTassertions inmcp-liveandopenclaw-liveto assert the corrected behavior: retired procedures leave the listing, and a body edit yields exactly one new live node.Behavior change for API consumers
A body-changing
PUT /procedures/:idstill returns a new id (the superseding node) — unchanged from before. What changes is that the old id is now retired: it keeps its previous body and version, remains resolvable by id, but no longer appears inlistor/recall. This matches how Facts already behave.Testing
pnpm typecheck✅ ·pnpm lint(changed files clean) ✅pnpm test— 127/127 unit ✅vitest.integration.config.ts):mcp-live43/43 ✅v1.2-thorough,openclaw-live,client-live,research— 98/98 ✅ (supersede-chain + audit-snapshot tests pass unchanged)v3; fan-out → keeps tie-broken node; standalone untouched; second run is a clean no-op).