Worktree feature+hermes research doc - #4297
Closed
huangyunhua-neolix wants to merge 46 commits into
Closed
Conversation
- hermes-self-evolution.md: full research snapshot (2026-06-10) covering runtime learning loop and offline evolution pipeline of Nous Research's Hermes Agent, with mapping to nanobot's existing surfaces. - hermes-evolution/roadmap.md: 5-milestone plan (M1 foundations → M2 skill_manage → M3 Curator; M1 → M4 offline skeleton → M5 Darwinian Evolver). M1 brainstorming in progress. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
M1 introduces the data layer that all later self-evolution work depends on: agent-skill directory isolation, skill telemetry (.telemetry.json + filelock), provenance frontmatter for agent-authored skills, and an auxiliary provider config plumbing that lets background tasks (Curator, Dream rubric) run without poisoning the main prompt cache. Approach B (schema + read-side counter hooks). No skill_manage, no Curator behavior, no slash commands — those land in M2/M3. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Apply 7 blockers + 6 warnings + 3 suggestions from the spec review: - §3.1: add explicit source/origin terminology mapping table - §4.1: rename first_seen → entry_created_at; clarify views semantics (agent-context only); add schema_version forward-compat rules - §4.2: replace bump_views/uses/patches with single bump(name, kind); document list_skills_with_shadows() rules (disabled filter, no cache) - §4.3: rewrite concurrency model — two-layer locks (threading.Lock + filelock), RMW merge with incremental counter accumulation to prevent lost updates across processes - §4.4: codify reconcile-vs-bump boundary; lazy-init for unknown names - §2: recommend .gitignore entries for telemetry artifacts - §5: explicit frontmatter read path via _get_skill_meta(...) - §6.4: replace runtime smoke-test with Pydantic model_validator - §7: add subagent.py / webui/skills_api.py call sites; require SkillsLoader to take optional telemetry param so WebUI never bumps; pin hook locations with explicit table - §8: update tests for bump(name, kind), add asyncio + multi-process concurrency tests, schema-version compat, lazy-init coverage; move 10-round real LLM run to optional manual checklist (§8.4) - §9: add decisions 13–21 covering terminology, concurrency, API shape - §10.1: reword agent-name collision risk; add new risks for terminology drift, multi-process lost-update, subagent loader duplication - §11: rename contract from bump_patches to bump(name, "patch"); add list_skills_with_shadows / provenance read entry / schema_version contracts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RED fixes:
- R1 §4.3: full flush() pseudocode (3 phases); _last_synced_counts
lifecycle table; merge rule branches for "entry only in snapshot"
(don't resurrect reconcile-deleted), corruption-reset, max(.., 0)
delta clamp; advance _last_synced only after successful rename
- R2 §1.1#6 / §7: pin view/use hooks inside SkillsLoader function
bodies, gated on `self.telemetry is not None`; WebUI constructs
loader with telemetry=None (physical guarantee, not convention);
add current-call-sites baseline table
- R3 §4.3 RMW table: branch origin/shadowed merge by writer
("reconcile"/"bump") and by snapshot.origin=="unknown"; ensures
lazy-init "unknown" cannot clobber reconciled real origin
- R4 §9 decision #6: rename first_seen → entry_created_at with
back-reference to decision #17
- R5 §3.1: clarify SkillsLoader.source stays 2-valued for legacy
compat (WebUI unbroken); new code uses origin (3-valued) only
via _infer_origin_from_path; two vocabularies coexist in memory
but never mix
YELLOW fixes:
- Y1 §1.1#2: list_skills_with_shadows also a M3 surface
- Y2 §12: README "Workspace requirements" section task
- Y3 §4.3: unify WARN throttling (100-failure dedup) for all
filelock/atomic-write failures, not just NFS
- Y4 §4.3: explicit "bumps during flush land in next flush"
delay note + invariant comment
- Y5 §4.3: fsync parent dir step in atomic_write
- Y6 §4.4: constraint table redone with writer-tag column;
three invariants (counter not lost, unknown not clobber,
orphan not resurrect)
- Y7 §8.1: concurrency tests pinned — asyncio with to_thread,
multiprocessing with spawn + top-level worker
- Y8 §6.2/§11: rename ProviderClient → LLMProvider (real type)
- Y10 §11: rephrase agent-dir row as precondition, not interface
- Y11 §1.1#3: mention threading.Lock in summary
- Y12 §4.4: explicit disabled_skills semantics (entry frozen,
not orphan-deleted); new test in §8.1
- Y13 §7: source-comment note on _get_skill_meta stability
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- §4.2 SkillTelemetry: add Writer Literal; reconcile scrubs "unknown" and calls flush(writer="reconcile") - §4.2 disabled_skills: split (a)/(b) pre-existing vs new wording - §4.3 RMW: split reconcile branch (unknown vs known); consolidate bump branch; add _last_synced_counts scope note + updated_at clock-skew note - §4.3 atomic_write: os.rename → os.replace (Windows cross-platform safety) [R-C] - §4.3 add .tmp residue cleanup owner (SkillTelemetry.__init__, glob .telemetry.json.tmp*) [Y-C] - §4.3 lifecycle table: add "process restart with intact disk" row [Y-H] - §4.3 add parent-dir auto-create note (mkdir parents=True, exist_ok=True) [Y-D] - §6.1 add model_validator placement clarification on root Config [Y-E] - §7 SkillsLoader.__init__: pin telemetry as keyword-only (*, telemetry=None) [R-B] - §7 runner row: spell out startup sequence contract; flush() always no-args [R-A residual + Y-B] - §8 new test file test_runner_telemetry_startup.py - §9 add decisions #22-#27 covering the above
REDs:
- §7 caller table: actual sites are context.py:92,96 + subagent.py:365
(was wrongly attributed to loop.py/runner.py); runner row now distinguishes
"construction point + flush trigger" from "build_skills_summary callsite"
- §4.3 add flush() single-flight contract (self._flush_lock, distinct from
self._lock); concurrent re-entry is no-op; invariant 4 documented
- §4.4 step 2: explicit "已存在条目" branch (incl. bump-lazy-init) keeps
entry_created_at unchanged on subsequent reconcile
- §4.4 step 4 + §4.3 inline note: reconcile read+write must occur inside the
same filelock window (forbid read-without-lock then write)
YELLOWs:
- §5 add "_get_skill_meta already unwraps metadata.nanobot" clarification
- §6.1 explicit Field(alias="modelPreset") + populate_by_name=True hint
- §4.2 SkillEntry docstring: name is dict-key on disk, field in list shape
- §4.2 introduce TelemetryEntrySnapshot + TelemetrySnapshot TypedDicts;
snapshot() typed return; §11 contract row updated
- §4.2 list_skills_with_shadows(): forbid calling _get_skill_meta (cache
discipline); disabled_skills is __init__ snapshot (no live-reload contract)
- §4.3 RMW: split "entry only in snapshot" by writer ("bump" skip vs
"reconcile" first-insert); WARN throttle dimension = (process, kind)
- §4.3 lifecycle "normal restart" row: explicit "__init__ does not hydrate
from disk" with double-counting failure mode worked out
- §4.1 schema forward-compat: passthrough scope (top-level + entries[name]
fields, not entries shape itself)
- §10.1 risk row for missing preset: replace "smoke-test" wording with
model_validator + runtime defence (matches decision #19)
- §8.1 new test bullets: flush single-flight; writer="reconcile" wins on
conflict; WARN throttle math (250 fails → exactly 2 WARNs); atexit-only
flush; get_auxiliary_client both-presets-missing ConfigError; agent path
as file-not-dir collision
- §8.1 multi-proc tests now pin spawn workspace-as-arg + explicit ordering
(Event-based) + coverage.py multiprocessing caveat
- §8.1 fsync(dir) test: @pytest.mark.skipif(win32)
- §8.1 integration test: extend tests/agent/test_loop_runner_integration.py
_make_loop helper rather than rolling new harness
- §7 keyword-only test signature: SkillsLoader(workspace, None, None, ts)
must TypeError (precise positional case)
- §9 add decisions HKUDS#28-HKUDS#36 covering all of the above
5 phases (A-E), 33 TDD tasks covering: - Phase A (A1-A12): SkillTelemetry core — double-lock concurrency, RMW merge with writer tag, atomic write + single-flight flush, WARN throttle, reconcile, no-hydrate invariant, multiproc + atexit + corruption tests - Phase B (B1-B8): SkillsLoader 3-source merge, _infer_origin_from_path, list_skills_with_shadows TypedDict, keyword-only telemetry kwarg, gated bump hooks inside build_skills_summary + load_skills_for_context - Phase C (C1-C6): Upper-layer wiring — ContextBuilder/SubagentManager forwarding, AgentLoop owns SkillTelemetry + atexit, reconcile-before- consume ordering, _run_agent_loop flush, explicit WebUI telemetry=None - Phase D (D1-D4): AuxiliaryConfig schema + camelCase alias, root-level model_validator(mode=after) for preset existence, get_auxiliary_client factory with main-preset fallback - Phase E (E1-E3): Integration tests — startup ordering invariant, subagent telemetry reuse, WebUI + agent multi-process coexistence Each task follows bite-sized 2-5 min TDD pattern: failing test → run (expect fail) → minimal implementation → run (expect pass) → commit. No placeholders, exact file paths, complete code snippets throughout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Also bundles A1 review fix: hoists inline stdlib imports to module top and restores module-level pytest import (now genuinely used by pytest.raises). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…assthrough (M1 Task A4)
… (M1 Task A5) Wires SkillTelemetry together: snapshot → filelock-guarded RMW write → advance last_synced. Three-phase pipeline preserves concurrent cross-process writes by re-reading on-disk under filelock and merging deltas via _rmw_merge. Single- flight gate (threading.Lock, non-blocking) makes overlapping flush() calls no-op. Filelock timeouts (3 retries @ 0.2s) and atomic write IO errors are counted in _failure_counts and warn-coalesced every WARN_COALESCE_EVERY (100). A4 review fixes bundled into this commit: - Extract COUNTER_KEYS module-level constant; use it in _rmw_merge and flush. - WARN on negative per-counter delta in _rmw_merge (last_synced > snapshot is an invariant violation; clamp behavior preserved, observability added). - Promote `import time` to top-of-file (drop lazy import in _epoch_ms). Spec interpretation note: _rmw_merge's "bump cannot resurrect orphan" rule now fires only when `name in last_synced` (the canonical "we previously synced N events; disk lost it → reconcile killed it" signature). A never- synced entry under bump is treated as first-landing so flush() can persist new entries created via bump(). The A4 orphan-skip test was updated to pass a non-empty last_synced reflecting the prior-sync precondition the rule actually targets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…concile-equivalent disk state The A5 implementation weakened the _rmw_merge bump-orphan-skip rule to require `name in last_synced` for the skip to fire. This conflicted with spec §4.3 line 373, invariant 3, decision HKUDS#31, and the canonical orphan test at line 703 of docs/hermes-evolution/specs/m1-foundations.md, which all encode "reconcile is the only legitimate creator of new entries". Revert _rmw_merge to the strict spec form. A5 tests that previously exercised "fresh workspace bump+flush writes to disk" now seed disk via a small test helper that simulates the post-reconcile state — A7 will land the real reconcile() call, but A5's flush semantics can be tested against that contract today. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds two tests for the flush() filelock-timeout path: - _dirty and _last_synced_counts are preserved when every retry times out (no disk file created, in-memory state untouched, next flush retries). - _note_failure coalesces WARN logs to once per 100 failures of the same kind: 250 timed-out flushes emit exactly 2 warnings (at 100/200). Loguru-stdlib bridge: - Adds a `loguru_caplog` fixture that propagates loguru records into stdlib logging so pytest's caplog can observe them. Required because the project logs via loguru and caplog only sees stdlib records. Bundled cleanups (from A5 review feedback): - Docstrings on flush(), _write_phase(), _note_failure() documenting the 3-phase pipeline, single-flight gate, RMW merge, and WARN coalescing. - Type hints on _write_phase parameters (snapshot, last_synced_snapshot). - Renames test_flush_rmw_preserves_concurrent_process_writes → test_flush_rmw_merges_external_disk_changes_between_flushes (the test is sequential, not concurrent — old name overpromised). - Hoists `import json` from inside _seed_disk to the top of the test file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eeze (M1 Task A7) Adds SkillTelemetry.reconcile(known_skills, disabled_skills=None) per spec §4.4: * Orphan removal: entries no longer in known_skills (and not in disabled_skills) are popped from in-memory state and propagated to disk via flush. * New entries get zero counters with the real origin and a fresh entry_created_at timestamp. * Existing entries only have origin/shadowed patched — counters and timestamps are never touched here (spec §4.4 invariant). * Disabled skills are FROZEN: neither deleted nor updated. * Lazy-init "unknown" origin entries are corrected to the real origin once the skill becomes known. Spec gap closed in _rmw_merge: the original A4 design only iterated snapshot entries during merge, so disk-only entries were unconditionally preserved. Add a writer="reconcile" branch that also drops disk entries missing from the snapshot — required for orphan removal to actually reach disk. writer="bump" behavior is unchanged (still preserves disk entries from concurrent writers). Bundled cleanups from A6 code review: * Move loguru_caplog fixture from test_skills_telemetry.py to tests/agent/ conftest.py for reuse across the agent test suite. * DRY the AlwaysTimeout class to a module-level helper (was duplicated inline across two tests). * Drive the WARN throttle test off WARN_COALESCE_EVERY arithmetic instead of hard-coded 250 / 2 magic numbers. 5 new tests cover: zero-init on arrival, orphan removal end-to-end (in-memory + disk), disabled-skills freeze, counter preservation across origin updates, and lazy-init origin correction. 25/25 tests pass; full agent suite (1141 tests) green; ruff clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…1 Task A8) Bundled cleanups from A7 review: - Tighten Step 3 comment in reconcile() to clarify defense-in-depth role. - Add test_reconcile_freezes_disabled_lazy_unknown_entry to cover the corner case where a skill was lazy-init bumped (origin='unknown') before being disabled with no intervening known-skills reconcile. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…orward-compat (M1 Task A12) - Add `register_atexit()` method that registers `flush` for interpreter shutdown. AgentLoop will call this after construction (Task C3). Idempotent under A5's single-flight gate and the _dirty=False short-circuit. - Pin existing corruption-rebuild and schema-version forward-compat behavior with two new tests (no production change required for those — A3 + A4 already implemented backup-on-corrupt and unknown-field passthrough). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r (M1 Task B2) list_skills() gains a minimal one-line extension to surface skills under <workspace>/skills/agent/foo/. Priority logic + collision detection are deferred to B3, which rewrites this block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
list_skills() now enforces user > agent > builtin priority. SkillsLoader construction eagerly walks all three sources once via _detect_collisions_once and logs one WARN per colliding name (no spam on repeated list_skills calls). Replaces the B2 minimal extension with the proper merge. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Task B4) Returns one record per visible skill with effective_origin (user|agent|builtin) and shadowed_origins (the rest, in user→agent→builtin scan order). Used by SkillTelemetry.reconcile to compute the canonical 3-value origin without parsing frontmatter. Filters disabled skills consistent with list_skills. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds optional `telemetry: SkillTelemetry | None = None` keyword-only kwarg to SkillsLoader.__init__. Forward reference via TYPE_CHECKING avoids a runtime cross-module import (skills_telemetry stays loader-agnostic). Default None preserves the WebUI/CLI no-telemetry construction pattern; agent runtime construction will pass a real telemetry instance starting in C3. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When telemetry is wired, every visible skill (including unavailable ones) gets a view bump per build_skills_summary call. Excluded skills are skipped before the bump, matching spec semantics: view = "agent saw this skill in the summary". No bump when self.telemetry is None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ess (M1 Task B7) Rewrites the list-comprehension form into an imperative for-loop so we can side-effect (telemetry bump) per iteration. Bump fires only after a successful load (markdown is not None) and only when self.telemetry is wired. Missing skills generate zero telemetry — they don't even produce an entry in the snapshot. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… _get_skill_meta (M1 Task B8) Adds a regression test that calls list_skills / list_skills_with_shadows / load_skill 10x each and asserts the telemetry snapshot has zero bumps — spec §7 hook table forbids these methods from bumping. Also adds a contract comment above _get_skill_meta marking it as a stable provenance read entry per spec §5/§11; signature must not drift in later milestones. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… (M1 Task C1) ContextBuilder now accepts an optional `telemetry: SkillTelemetry | None` keyword-only kwarg and threads it into the SkillsLoader it owns. Default None preserves the WebUI/CLI no-telemetry path; the agent-loop runtime will start passing a real telemetry instance in C3. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oader (M1 Task C2) Adds keyword-only `telemetry: SkillTelemetry | None = None` kwarg to SubagentManager.__init__, stored as self.telemetry and threaded into the SkillsLoader constructed inside _build_subagent_prompt. The telemetry instance is shared (not owned) — one per agent process tree per M1 spec. TYPE_CHECKING avoids runtime import of skills_telemetry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… Task C3) AgentLoop.__init__ now constructs a SkillTelemetry tied to the loop's workspace, registers atexit-flush, and threads the SAME instance into ContextBuilder and SubagentManager. One telemetry per agent process tree per spec section 3. atexit registration is idempotent — multiple AgentLoop constructions in tests are harmless. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AgentLoop.run() now calls self.telemetry.reconcile(...) immediately after MCP connect and before entering the inbound-message consume loop. The reconcile uses list_skills_with_shadows() so origin and shadowed_origins are populated correctly on first persist. Required by spec section 4.4 — first bump landing on disk must already know the correct origin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…KUDS#49) Spec §4.3 RMW table previously had a single row for "entry only in on_disk" that said "keep — another process is managing it". That hid the writer-tag divergence already present in production (`_rmw_merge` lines 197-207): bump preserves, reconcile DELETEs. Patch: - Split the row into two writer-tag branches matching implementation - Add invariant 4: reconcile is the sole orphan deleter; bump never modifies the entry set Pairs with existing invariant 3 ("bump never resurrects") to make the "reconcile owns entry-set changes" contract symmetric and explicit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…te_prompt_tokens
`LLMProvider` declares `get_default_model` but not `estimate_prompt_tokens`
— production code accesses the latter via `getattr(provider, "...", None)`
in `nanobot/utils/helpers.py:510`. A spec-limited `MagicMock(spec=LLMProvider)`
therefore rejects the attribute-access chain
provider.estimate_prompt_tokens.return_value = (10_000, "test")
with `AttributeError("Mock object has no attribute 'estimate_prompt_tokens'")`.
Fix: assign a fresh `MagicMock(return_value=...)` directly to the attribute
name. Direct assignment bypasses spec attribute-access enforcement; both
spec=True and spec=False callers now get a callable returning
`(10_000, "test")`.
Add `test_conftest_helpers.py` with 4 regression tests covering both
spec values, default model override, and generation settings shape.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The E3 multiprocess test imported `from pathlib import Path as P` inside both worker functions, triggering ruff N817 (CamelCase imported as acronym). The `as P` rebind served no purpose — `Path` itself is short. Rename to plain `Path` and update the 3 callsites. Pure style cleanup; behavior unchanged. Test still passes in ~1.6s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…HKUDS#50) `register_atexit` previously bound `flush` directly. If shutdown raced with an in-flight flush (single-flight gate held) or the cross-process filelock was held by a peer, the atexit hook silently returned and any unflushed bumps were lost — with no visible signal. New behavior: - `register_atexit` now binds `_atexit_flush`, a wrapper that: 1. calls `flush()` once 2. if `_dirty` remains True, sleeps `_ATEXIT_RETRY_DELAY_S` (0.05s) to let peer holders release locks, then calls `flush()` again 3. if still dirty, emits a single `atexit_flush_skipped` WARN including `_failure_counts` so operators can correlate - `_ATEXIT_RETRY_DELAY_S` constant added next to existing tunables - `time` already imported 5 new test cases cover: - register_atexit binds the wrapper (not bare flush) - clean state → no WARN, no retry - dirty after first attempt → exactly one retry with the configured delay - both attempts fail → exactly one atexit_flush_skipped WARN with payload - first attempt succeeds → no sleep, no WARN Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
YELLOW-1: _failure_counts race in _atexit_flush `_note_failure` (skills_telemetry.py:460-461) mutates `self._failure_counts` lock-free for performance. The atexit reader then did `dict(self._failure_counts)` outside `_lock`, risking `RuntimeError: dictionary changed size during iteration` if a peer flush worker was still alive at shutdown — and our own docstring already concedes other threads may be live. Fix: snapshot the dict inside the existing `with self._lock:` block that reads `_dirty`. Cost: one extra dict copy (≤10 keys typical). YELLOW-2: E3 multiprocess head-start flake `time.sleep(0.05)` between agent_proc.start() and webui_proc.start() assumed spawn-context Python re-import (nanobot + filelock + loguru) completes in <50ms. On loaded CI this can take 100-300ms, in which case WebUI would start before agent's first flush — test still passes (WebUI tolerates missing file), but no longer proves "concurrent with active producer". Fix: poll for .telemetry.json existence with 5s timeout. Also break early if agent dies, so the assertion surfaces the real failure. MINOR: drop redundant `from pathlib import Path` from _agent_worker and _webui_worker — module-level import (line 13) propagates via spawn's module re-execute. All 40 telemetry + integration tests pass; ruff clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Jun 12, 2026
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.
No description provided.