feat: per-agent memory opt-out (excludeAgents / excludeSubagents) - #357
Conversation
…-out Multi-agent gateways run a mix of agents with very different memory needs: a primary assistant that wants full recall, a latency-critical voice agent where every injected token and embed round-trip is pure overhead, and ephemeral subagents that should run lean. The context engine currently treats every agent identically. This adds two opt-in config keys: - excludeAgents: string[] — sessions whose agent id (parsed from the agent:<agentId>:... session key) is listed skip ALL memory/context work: no injection, ingestion, compaction, or daemon RPCs. - excludeSubagents: boolean — when true, every subagent session (tracked via the prepareSubagentSpawn lifecycle) skips all memory/context work. For an excluded session, assemble() is a true no-op: it returns the host's messages byte-identical with an empty systemPromptAddition (no budget-fitting, which can drop messages mid-tool-protocol and trip strict providers). bootstrap/ingest/afterTurn early-return; compact() short-circuits via session ids recorded at bootstrap (compact only receives sessionId, not sessionKey). Adds JSON schema entries and unit tests covering exclusion, the daemon-untouched guarantee, the non-excluded control paths, and subagent lifecycle teardown. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds opt-in ChangesAgent/subagent exclusion feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PluginConfig
participant buildContextEngineFactory
participant ContextEngine
participant DaemonClient
PluginConfig->>buildContextEngineFactory: Provide exclusion settings
buildContextEngineFactory->>ContextEngine: Initialize exclusion state
ContextEngine->>ContextEngine: Evaluate session exclusion
ContextEngine-->>DaemonClient: Skip memory and context RPCs
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
compoodment
left a comment
compoodment
left a comment
There was a problem hiding this comment.
Vale Review — PR #357
Quality: Q4/5 — sharp
Head: dc475b3
Findings:
- src/context-engine.ts:2773 —
compact()only skips excluded sessions while their id is still present in the bootstrap-populatedexcludedSessionIdsset, but that set is capped at 1000 and evicts the oldest entry atsrc/context-engine.ts:2351. After 1001 excluded sessions, an older still-active excluded session can fall through to normal compaction and daemon work, breaking the advertised “no compaction, or daemon RPCs” guarantee. major
Proof gaps: No local gate run; source path is deterministic and current checks are still pending.
Verdict: request-changes — excluded sessions must stay inert for compaction even after the side table hits its bound.
– Vale
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/unit/context-engine.test.ts (1)
2624-2626: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that teardown really clears exclusion state.
These calls are only smoke-tested right now. If either cleanup path stops removing
excludedSubagentKeys, this test still passes. Add one post-teardown bootstrap/assertion with a non-throwing runtime and verify the samechildSessionKeyreachesbootstrapSessionKernelagain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/context-engine.test.ts` around lines 2624 - 2626, The teardown path in the context engine test is only smoke-tested and does not verify that exclusion state is actually cleared. Update the test around the `handle.rollback?.()` and `engine.onSubagentEnded(...)` cleanup so it performs one post-teardown bootstrap/assertion using a non-throwing runtime, then confirm the same `childSessionKey` is allowed through `bootstrapSessionKernel` again. Use the existing `excludedSubagentKeys`-related flow in `ContextEngine` to locate the assertion point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/context-engine.ts`:
- Around line 1703-1711: `isExcludedSession` currently only checks `sessionKey`,
so an excluded bootstrapped session can slip through when the hook omits it.
Update `isExcludedSession` in `context-engine.ts` to fall back to the recorded
`sessionId` whenever `sessionKey` is undefined, and apply the same fallback in
the other exclusion checks referenced by `sessionKey`-based logic so
`excludedAgents` and `excludedSubagentKeys` still match the session
consistently.
- Around line 2350-2357: The exclusion-tracking logic in the
isExcludedSession/sessionId path leaves stale entries in excludedSessionIds, so
a reused sessionId can keep causing compact() to return “agent excluded.” Update
the bootstrap handling around the existing isExcludedSession(args.sessionKey)
branch to clear any prior exclusion marker for that sessionId when the current
session is not excluded, and keep the add/eviction behavior only for truly
excluded sessions. Make sure the fix is applied in the same context that manages
excludedSessionIds and the compact() exclusion check so reused IDs cannot retain
old state.
---
Nitpick comments:
In `@test/unit/context-engine.test.ts`:
- Around line 2624-2626: The teardown path in the context engine test is only
smoke-tested and does not verify that exclusion state is actually cleared.
Update the test around the `handle.rollback?.()` and
`engine.onSubagentEnded(...)` cleanup so it performs one post-teardown
bootstrap/assertion using a non-throwing runtime, then confirm the same
`childSessionKey` is allowed through `bootstrapSessionKernel` again. Use the
existing `excludedSubagentKeys`-related flow in `ContextEngine` to locate the
assertion point.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15af0409-d3bb-4593-b64b-cbbad518f5a2
📒 Files selected for processing (4)
openclaw.plugin.jsonsrc/context-engine.tssrc/types.tstest/unit/context-engine.test.ts
Addresses CodeRabbit review on the per-agent memory opt-out: - isExcludedSession now falls back to the sessionId recorded at bootstrap when sessionKey is absent. sessionKey is optional on these hooks, so an excluded session could otherwise reach daemon work when the host omits it. bootstrap/ingest/assemble/afterTurn pass sessionId through. - bootstrap clears any stale excludedSessionIds marker on the non-excluded path, so a reused sessionId can no longer make compact() keep returning "agent excluded".
compoodment
left a comment
compoodment
left a comment
There was a problem hiding this comment.
Vale Review — PR #357
Quality: Q4/5 — sharp
Head: c43b7d4
Findings:
- src/context-engine.ts:2358 — excluded sessions are still tracked in a capped
excludedSessionIdsset, whilecompact()later trusts that set alone atsrc/context-engine.ts:2783. After enough excluded sessions evict an older active session id,compact()receives onlysessionId, misses the exclusion, and can fall through to normal compaction/daemon work despite the PR promising excluded sessions skip compaction and daemon RPCs. major
Proof gaps: No local gate run; source path is deterministic, but needs a regression test for more than EXCLUDED_SESSION_IDS_MAX excluded bootstraps followed by compact on the evicted session.
Verdict: request-changes — the stale-marker fix helps reused ids, but the bounded compact side table still breaks the exclusion guarantee.
– Vale
… survive side-table overflow compact() previously trusted only the bounded excludedSessionIds set. Past EXCLUDED_SESSION_IDS_MAX cumulative excluded bootstraps, an older but still-active excluded session was evicted and its next compact() fell through to real compaction and daemon RPCs — including on the on-demand /compact path, which is not preceded by an assemble() that could refresh the marker. The host already threads sessionKey through every compaction path (timeout and overflow recovery in run.ts, and the manual /compact lane in compact.queued.ts), so compact() can resolve exclusion authoritatively from the agent id instead of the evictable side table. Declare sessionKey?: string on the compact args and route the check through isExcludedSession(); this is eviction-proof and also covers excludeSubagents (the child sessionKey recorded at prepareSubagentSpawn). The sessionId side table is retained only as a best-effort fallback for the rare case where the host cannot backfill a sessionKey, and every per-turn hook that carries the authoritative sessionKey (assemble/ingest/afterTurn) now refreshes the marker (MRU) so an id evicted while idle is re-established on the next turn. Adds two regression tests: a direct sessionKey-carrying compact() after 1001 excluded bootstraps, and the sessionKey-less fallback after eviction + refresh.
|
Thanks @compoodment — the bounded-side-table finding was correct, and the root cause was deeper than the cap: Fixed authoritatively in
Regression tests (both confirmed to fail on the pre-fix code):
Only residual is the explicitly best-effort case (a |
compoodment
left a comment
compoodment
left a comment
There was a problem hiding this comment.
Vale Review — PR #357
Quality: Q4/5 — sharp
Head: 18b5a14
Findings:
- src/context-engine.ts:2803 — None: compact() now accepts sessionKey and resolves exclusion before touching the daemon client; src/context-engine.ts:1709-1728 keeps the sessionId set as fallback only, with overflow regressions covering the prior blocker. info
Proof gaps: No local gate run in this worker; host-side compact sessionKey threading is asserted from the PR discussion, not independently tested here.
Verdict: approve — current head fixes the bounded side-table compaction escape that caused the previous request-changes review.
– Vale
use @vale review
3 participants
Summary
Multi-agent OpenClaw gateways run agents with very different memory needs: a primary assistant that wants full recall, a latency-critical voice agent where every injected token and embed round-trip is pure overhead, and ephemeral subagents that should run lean on a focused task. The context engine currently treats every agent identically.
This adds two opt-in config keys so an operator can make LibraVDB fully inert for specific agents:
excludeAgentsstring[]agent:<agentId>:...session key) is listed skip all memory/context work — no injection, ingestion, compaction, or daemon RPCs.excludeSubagentsbooleantrue, every subagent session (tracked via theprepareSubagentSpawnlifecycle) skips all memory/context work.Both default off; behavior is unchanged unless configured.
Design
The plugin already parses
agentIdout of the session key for namespacing — this just consults it for behavior.assemble()is a true no-op for excluded sessions: it returns the host'smessagesbyte-identical with an emptysystemPromptAddition. It deliberately does not run them through budget-fitting — budget-fitting can drop messages mid-tool-protocol, which strict providers reject ("invalid schema/tool payload"). Context budget stays the host's responsibility.bootstrap/ingest/afterTurnearly-return a no-op result.compact()only receivessessionId(nosessionKey), so excluded sessions are recorded in a small bounded set atbootstraptime, lettingcompact()short-circuit too.prepareSubagentSpawnmarks the child session key whenexcludeSubagentsis set (and skips granting an expansion budget, since there's no memory to expand);onSubagentEnded/ the spawnrollbackclear the marker. All per-session state is cleared indispose.Testing
tsc --noEmitclean;tsc -p tsconfig.build.json+ bundle clean.getClientthrows) and returns byte-identical passthrough; non-excluded agent still reaches the daemon; subagent exclusion + lifecycle teardown;excludeSubagentsoff-by-default control.mainwith tests. Happy to test any requested changes on that box.Notes
Backward compatible and fully opt-in. Schema entries added to
openclaw.plugin.json(the schema isadditionalProperties: false, so the keys must be declared to be accepted).Summary
excludeAgentsandexcludeSubagents.assemble()returns the host messages unchanged with an emptysystemPromptAddition.bootstrapsocompact()can short-circuit later, and cleaned up exclusion state on subagent end and disposal.sessionIdrecorded atbootstrapwhensessionKeyis not provided, and cleared stale exclusion markers for non-excluded bootstrap paths.Tests
Review notes
src/context-engine.tshas additional exclusion/lifecycle branching; a cyclomatic-complexity proxy increased slightly (heuristic decision-point count +3 from the parent version, driven mainly by added ternary/conditional operators).