Skip to content

fix(context-engine): clone plugin engines per agent - #62374

Open
stephenschoettler wants to merge 1 commit into
NousResearch:mainfrom
stephenschoettler:pilot/42683-context-engine-clone
Open

fix(context-engine): clone plugin engines per agent#62374
stephenschoettler wants to merge 1 commit into
NousResearch:mainfrom
stephenschoettler:pilot/42683-context-engine-clone

Conversation

@stephenschoettler

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds an explicit per-agent clone contract for process-wide context-engine plugin templates and closes owned clone resources when an agent is closed or softly evicted. This is a current-main replacement for stale/conflicting #42683 and preserves the original contract direction proposed there by @the3asic.

Current main already deep-copies general-plugin context engines to prevent #42449's shared-singleton mutation. That fails for engines with locks, database handles, or other non-copyable runtime state. ContextEngine.clone_for_agent() keeps deepcopy as the default while allowing those engines to construct an isolated runtime explicitly.

Related Issue

Related: #42449. Supersedes #42683.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • agent/context_engine.py
    • Add clone_for_agent() with a deepcopy default for backward-compatible isolation.
  • agent/agent_init.py
    • Clone general-plugin templates before model/session binding; reject None, exceptions, and template aliases with safe built-in fallback.
  • run_agent.py
    • Shut down only owned per-agent clones, idempotently, on full close and soft cache eviction.
  • tests/agent/test_context_engine.py
    • Cover the default clone contract.
  • tests/run_agent/test_plugin_context_engine_init.py
    • Cover custom cloning, template isolation, alias rejection, and one-time shutdown.

How to Test

  1. Run focused and relevant lifecycle validation:

    scripts/run_tests.sh tests/agent/test_context_engine.py tests/agent/test_context_engine_host_contract.py tests/run_agent/test_plugin_context_engine_init.py tests/run_agent/test_compression_boundary_hook.py tests/run_agent/test_openai_client_lifecycle.py tests/run_agent/test_switch_model_context.py tests/run_agent/test_commit_memory_session_context_engine.py tests/run_agent/test_compress_focus_plugin_fallback.py -- -q --tb=short

    Result: 66 passed, 0 failed.

  2. Run changed-file checks:

    ruff check agent/context_engine.py agent/agent_init.py run_agent.py tests/agent/test_context_engine.py tests/run_agent/test_plugin_context_engine_init.py
    python -m py_compile agent/context_engine.py agent/agent_init.py run_agent.py tests/agent/test_context_engine.py tests/run_agent/test_plugin_context_engine_init.py
    git diff --check origin/main...HEAD

    Result: all passed.

  3. Manual exercise: not run against a live third-party context engine; focused tests exercise the real AIAgent initialization and soft-eviction lifecycle with a lock-holding custom clone engine.

Validation Status

  • Focused regression tests: passing, 66/66.
  • Broader suite: not run; eight relevant host/lifecycle files passed.
  • GitHub checks: pending publication.
  • Full-suite checklist is intentionally unchecked because the full repository suite was not run locally.
  • Tested on Linux 7.0.14 x86_64, Python 3.14.6.

Checklist

  • Commit uses Conventional Commits.
  • Existing issues/PRs and competing implementations were searched.
  • The branch contains one scoped commit and no unrelated files.
  • Regression tests were added.
  • Documentation/config/tool-schema updates are N/A.
  • Cross-platform impact considered: standard Python object lifecycle/deepcopy behavior; no platform-specific code.
  • Full pytest tests/ -q suite run locally.

Screenshots / Logs

N/A; code-only lifecycle change.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Supersedes #42683 (same title, by @the3asic) — this is the current-main replacement preserving that PR's contract direction, and it goes further by giving engines with locks/DB handles a way to construct an isolated runtime rather than relying on deepcopy alone. Related: #42449 (the shared-singleton corruption this addresses), #58512 (pre-compaction rebind, a distinct mechanism in the same family). Flagging the #42683#62374 supersession for a maintainer to pick.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for rebuilding the clone contract against current main. I found no blocking correctness issue.

Current main still takes the general-plugin singleton path through copy.deepcopy(_candidate) in agent/agent_init.py:1755-1775; a copy failure intentionally falls back to ContextCompressor. This PR replaces that failure mode with an explicit isolated-clone hook while retaining deepcopy as the default (2972c1c21b34). The scoped teardown is gated by ownership, so it does not close the process-wide registered template. The separate repo-engine loader already instantiates/collects engines per load (plugins/context_engine/__init__.py:175-194).

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 11, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two PRs address the process-wide context-engine state issue. #42683 introduced opt-in per-agent cloning but retained shared-instance aliasing by default and only handled hard-close teardown; #62374 rebuilds the contract on current main, preserves deepcopy isolation by default, rejects unsafe aliases, supports non-copyable engines, and covers both soft eviction and hard close.

Related pull requests

  • #42683 [closed] duplicate — (+194/-8) — superseded by #62374: #42683 established the per-agent clone and ownership direction, but its default clone returns the registered singleton and its teardown only runs on close. Despite the keep_open review on #42683, that same review identifies these gaps and explicitly recommends the rebuilt #62374 approach; #42683 therefore remains relevant as the original implementation and attribution source.
  • #62374 related — (+151/-40) — keep open with a salvage path: retain the explicit clone hook, deepcopy default, template-alias rejection, ownership-gated idempotent shutdown in both release_clients() and close(), and the associated isolation/lifecycle tests. This agrees with the MAINTAINER-BOT keep_open verdict, while adding the cross-PR finding that #62374 supersedes #42683 and resolves the two deficiencies identified in #42683's review.

Duplicates

#42683 and #62374 implement substantially the same per-agent context-engine cloning contract; #62374 is the current-main replacement and #42683 is the superseded predecessor.

Suggested consolidation

Keep #62374 open with the concrete salvage path of preserving its isolated clone contract and dual lifecycle teardown; close #42683 as duplicate of and superseded by #62374. No merge recommendation is made: the available evidence supports retaining #62374's implementation for further maintainer handling, while #42683's default aliasing and missing soft-eviction teardown make it the inferior duplicate.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup42683 ["PRs duplicating each other"]
        P42683["PR #42683 (closed)"]
        P62374["PR #62374 (open)"]
    end
    class P42683 closed
    class P62374 open
    class P62374 target
    click P42683 "https://github.com/NousResearch/hermes-agent/pull/42683"
    click P62374 "https://github.com/NousResearch/hermes-agent/pull/62374"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 24 kB of PR diffs, 5 kB of issue/PR text, 5 kB of discussion (8 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants