feat: pluggable context-rewrite modules — per-agent engine + host-run request_llm_summary (closes #6264)#6287
Conversation
The context engine was selected solely by the global `KernelConfig.context_engine`. An agent's manifest can now override it: a `[context_engine]` block in `agent.toml` (or `[agents.<name>.context_engine]` in a `HAND.toml`) is resolved as agent override > kernel config > compiled default, mirroring the existing per-agent `proactive_memory` / `skill_workshop` / `compaction` overrides (#5476). `context_engine_for_agent` now picks the manifest override when present and falls back to the kernel-global engine otherwise (and on a per-agent build failure, so a malformed override never silently disables recall/compaction). Per-agent engines are built lazily and deduplicated by a fingerprint of their config, so many agents sharing one override config build a single engine. The plugin-allowlist gate now checks the resolved engine. The new field is registered in `PER_AGENT_OVERRIDE_KEYS` (so a misplaced `[agents.x.context_engine]` in config.toml gets the #5476 "move to agent.toml" warning) and covered by the exhaustive override-key drift-guard test. No `KernelConfig` field changed, so config-reload classification is untouched; per-agent changes take effect on respawn, consistent with the other per-agent overrides. Scoped to per-agent selection only. The proposal's separate `RequestLlmSummary` scriptable-hook contract changes the plugin protocol and is deferred pending design sign-off from the proposer.
…eltas # Conflicts: # crates/librefang-kernel/src/kernel/tests.rs
houko
left a comment
There was a problem hiding this comment.
Style-only review — 5 CLAUDE.md violations in the new prose, all the same class.
Rule 1 — CHANGELOG sentence-per-line (CHANGELOG.md:275): the continuation line packs four sentences; see inline comment for the split.
Rule 2 — one-line-max for doc comments (CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max"): every new /// block in this PR exceeds the limit. Inline comments on each file carry condensed one-line replacements.
Affected locations:
crates/librefang-types/src/agent.rslines 1354–1372 (19-line field doc)crates/librefang-kernel/src/kernel/mod.rslines 737–746 (10-line field doc)crates/librefang-kernel/src/kernel/tools_and_skills.rslines 1386–1408, 1449–1456, 1470–1471, 1489–1491, 1678–1685 (three function docs + two inline comment blocks)crates/librefang-kernel/src/kernel/tests.rslines 11223–11227, 11248–11254, 11275–11276, 11317–11322, 11359–11361 (four function docs + one inline comment)
No logic issues found; the resolution order, dedup, fallback-on-build-failure, and allowed_plugins gate all look correct.
Generated by Claude Code
| ### Added | ||
|
|
There was a problem hiding this comment.
CLAUDE.md prose rule: "CHANGELOG bullets — one sentence per line within a bullet."
The continuation line (275) packs four sentences together.
Split to one sentence per line:
The context engine was chosen by a single global `config.toml: context_engine`.
An agent's manifest (`agent.toml`, or a `HAND.toml [agents.<name>.context_engine]` block) can now override it, resolved as agent override > kernel config > compiled default — mirroring the existing per-agent `proactive_memory` / `skill_workshop` / `compaction` overrides (#5476).
Per-agent engines are built lazily and deduplicated by config, and a malformed override falls back to the global engine rather than disabling recall/compaction.
(The proposal's separate `RequestLlmSummary` hook-protocol change is deferred pending design sign-off.)
Generated by Claude Code
| /// Per-agent override for the kernel-global `[context_engine]` | ||
| /// selection (#6264). When `Some(_)`, this agent's context engine | ||
| /// (memory recall, prompt assembly, compaction wrapping) is built | ||
| /// from this manifest config instead of `KernelConfig.context_engine`. | ||
| /// `None` (the default) inherits the kernel-global engine. | ||
| /// | ||
| /// Resolution order — identical in shape to `compaction` / | ||
| /// `proactive_memory` / `skill_workshop` (#5476): | ||
| /// 1. this field (`agent.toml` `[context_engine]`, or `HAND.toml` | ||
| /// `[agents.<name>.context_engine]`) — when `Some(_)` | ||
| /// 2. `config.toml` `[context_engine]` — kernel-global default | ||
| /// 3. compiled-in `ContextEngineTomlConfig::default()` ("default" engine) | ||
| /// | ||
| /// Use case: one agent runs a `sidecar` recall engine while the rest | ||
| /// of the daemon stays on the default in-process engine, without | ||
| /// forcing every agent onto the heavier path. Like | ||
| /// `tool_exec_backend`, a manifest change is picked up on respawn — | ||
| /// the engine is built lazily per resolved config and cached for the | ||
| /// process lifetime (#6264). |
There was a problem hiding this comment.
CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max."
This 19-line block covers the resolution order, a use-case example, and the pick-up-on-respawn behaviour — all of which are either obvious from the code or better placed in CLAUDE.md/docs.
Condense to one line keeping only the non-obvious invariant:
/// Per-agent context-engine override (`agent.toml [context_engine]`); `None` inherits the kernel-global engine; resolution: agent manifest > kernel config > compiled default (#6264).
Generated by Claude Code
| /// Lazily-built per-agent context engines (#6264). Keyed by a stable | ||
| /// fingerprint of the agent manifest's `[context_engine]` override so | ||
| /// agents that select the same engine config share one instance. Each | ||
| /// distinct override is built at most once per process and stored as a | ||
| /// `'static` reference (the engine lives for the daemon's lifetime, like | ||
| /// the kernel-global `context_engine` above), so | ||
| /// [`Self::context_engine_for_agent`] can hand back a borrow that | ||
| /// outlives the lock and survives across the agent loop's `.await`. | ||
| /// Agents without an override are served the kernel-global engine and | ||
| /// never touch this map. |
There was a problem hiding this comment.
Same rule — 10-line doc block on a private field.
The non-obvious part is the Box::leak lifetime contract; the rest is recoverable from the type and neighbouring comment on context_engine.
Condense to:
/// Per-agent engine cache; keyed by config fingerprint, `Box::leak`-ed to `'static` (bounded: one per distinct config per process).
Generated by Claude Code
| /// Resolve the effective context engine for an agent (#6264). | ||
| /// | ||
| /// Returns the context engine reference if: | ||
| /// - The agent has no `allowed_plugins` restriction (empty = all plugins), OR | ||
| /// - The configured context engine plugin name appears in the agent's allowlist. | ||
| /// Resolution order — identical in shape to the other per-agent overrides | ||
| /// (`compaction` / `proactive_memory` / `skill_workshop`, #5476): | ||
| /// 1. `manifest.context_engine` (`agent.toml` / `HAND.toml` | ||
| /// `[agents.<name>.context_engine]`) — when `Some(_)`, the agent runs | ||
| /// its own engine built from that config. | ||
| /// 2. `KernelConfig.context_engine` — the kernel-global engine built once | ||
| /// at boot (`self.context_engine`). | ||
| /// 3. Compiled default — no engine wired (the caller falls back to the | ||
| /// built-in compactor); preserved by returning `None`. | ||
| /// | ||
| /// Returns `None` if the agent's `allowed_plugins` is non-empty and the | ||
| /// context engine plugin is not in the list. | ||
| /// The per-agent engine is built lazily, deduplicated by a fingerprint of | ||
| /// the resolved override config so agents selecting the same engine share | ||
| /// one instance, and cached as a `'static` reference for the daemon's | ||
| /// lifetime (see [`Self::context_engine_overrides`]) — the kernel-global | ||
| /// engine has the same process lifetime, so handing back a borrow here is | ||
| /// sound across the agent loop's `.await`. | ||
| /// | ||
| /// Regardless of which engine is selected, the `allowed_plugins` allowlist | ||
| /// gate still applies: if the agent restricts plugins and the *resolved* | ||
| /// engine config names a plugin not in the allowlist, the agent gets no | ||
| /// context engine (`None`), exactly as before. |
There was a problem hiding this comment.
Three new functions in this file each carry multi-paragraph docs that violate the one-line-max rule.
context_engine_for_agent (this hunk, lines 1386–1408): condense to:
/// Returns the context engine for `manifest`: agent override > kernel config > compiled default; falls back to global if per-agent build fails; still gates on `allowed_plugins`.
per_agent_context_engine (lines 1449–1456): condense to:
/// Builds or fetches a per-agent engine by config fingerprint; `Box::leak`-ed to `'static`, bounded: at most one per distinct override config per process.
The two multi-line inline comments inside the body (lines 1470–1471 and 1489–1491) should each become one line.
context_engine_config_fingerprint (lines 1678–1685): condense to:
/// Within-process fingerprint of an override config (serde_json → DefaultHasher); cache key for `context_engine_overrides`; `None` on serialization failure.
Generated by Claude Code
| /// Identity of the engine `context_engine_for_agent` resolves to, by data | ||
| /// pointer. The kernel-global engine is one fixed allocation; a per-agent | ||
| /// override builds (and leaks) a distinct one and dedups by config. Comparing | ||
| /// the trait object's data address proves *which* engine an agent got without | ||
| /// needing an LLM driver to observe behavioral differences. |
There was a problem hiding this comment.
Four doc blocks in the new test section each exceed the one-line limit.
engine_addr (this hunk): /// Returns the data-pointer of the resolved engine; pointer identity proves which engine the agent got (leaked allocations are stable).
context_engine_resolution_prefers_agent_override_then_kernel_config (line 11248–11254): /// Proves resolution order: agent override > kernel config > compiled default; identical configs share one engine instance.
context_engine_resolution_compiled_default_when_no_override_set (lines 11317–11322): /// Proves that absent any non-default override, every agent resolves to the compiled-default kernel-global engine.
context_engine_override_parses_from_agent_toml (lines 11359–11361): /// Proves [context_engine] in agent.toml parses onto the manifest; absent section deserializes to None.
The two-line inline comment at line 11275–11276 inside the first test body should also collapse to one line.
Generated by Claude Code
…N, not a u64 hash review follow-up: the override-engine cache was keyed by a u64 DefaultHasher digest of the config's canonical JSON, so a hash collision between two distinct configs would silently hand an agent the wrong cached engine. Key the map by the canonical JSON string itself — distinct configs can never collide, and the dedup (one engine per identical config) is unchanged. Negligible cost for tiny, few configs.
…oks (#6264) Completes the second half of #6264. A scriptable compact hook can now return { request_llm_summary: { prompt?, summarize, keep, max_tokens? } } instead of final messages; the host runs the LLM summary with its own driver and assembles the CompactionResult (summary = LLM text, kept_messages = the script's keep list). The script chooses what to fold and how; credentials/routing/driver stay host-side. Handled at all three compact output sites (cache hit, cache miss, non-cached) via a shared try_host_summary helper. Missing/empty summarize or any LLM error falls back to the built-in compactor — never a panic. 3 tests cover the host-run path, the no-directive passthrough, and the empty-summarize fallback.
Closes #6264.
The proposal asked for pluggable context-rewrite modules. The trait + backends already shipped (
ContextEngine,ScriptableContextEngine,SidecarContextEngine); the two genuine gaps are both implemented here.1. Per-agent context-engine selection
The engine was chosen solely by the global
KernelConfig.context_engine. An agent's manifest can now override it:context_engine: Option<ContextEngineTomlConfig>onAgentManifest, alongsideproactive_memory/skill_workshop/compaction(same serde shape, same[agents.<name>.context_engine]support in aHAND.toml) — the bug(docs/config): [agents.<name>.proactive_memory] block in config.toml silently ignored — real path is agent.toml #5476 per-agent-override pattern.context_engine_for_agentresolves agent override > kernel config > compiled default; a malformed override falls back to the global engine. Per-agent engines are built lazily and deduplicated by their canonical-JSON config key (collision-free), and the plugin-allowlist gate checks the resolved engine.PER_AGENT_OVERRIDE_KEYS(a misplaced[agents.x.context_engine]inconfig.tomlgets the bug(docs/config): [agents.<name>.proactive_memory] block in config.toml silently ignored — real path is agent.toml #5476 warning) and covered by the exhaustive override-key drift-guard test.2.
request_llm_summary— host-run summarization for scriptable compact hooksPreviously a scriptable
compacthook had to return the final compacted messages itself, with no LLM access (the host owns the driver). Now the hook may return:{ "request_llm_summary": { "prompt": "…", "summarize": [..], "keep": [..], "max_tokens": 2048 } }and the host runs the LLM summary with its own driver, returning a
CompactionResult { summary: <LLM text>, kept_messages: <keep> }. The script chooses what to fold and how (prompt, budget); the driver, credentials, and routing stay host-side. Handled at all three compact output sites (cache hit / miss / non-cached) via a sharedtry_host_summaryhelper. Missing/emptysummarizeor any LLM error falls back to the built-in compactor — never a panic. The legacymessagesreturn path is unchanged.Verification
cargo check -p librefang-types -p librefang-kernel -p librefang-runtime— clean (Docker dev image).cargo clippy -p librefang-types -p librefang-kernel -p librefang-runtime --all-targets -- -D warnings— zero warnings.request_llm_summaryhost-run + keep-list, no-directive passthrough, empty-summarize fallback (3 runtime tests).Reviewer note
Per-agent override engines are built once and held for the process lifetime (
Box::leak, deduped by canonical-JSON config key) to keep the existingOption<&dyn ContextEngine>return type without a wider refactor. Alternative:Arc<dyn ContextEngine>+ signature change — flagging the trade-off.