refactor(memory): delete dead sidecar infrastructure and HardScope plumbing#586
Merged
Conversation
…umbing Audience is the only security axis for memory access. Lexical/facet/anchor matching is the only relevance signal. Everything else (recall sidecar, observation sidecar, HardScope, domain-derived boundary inference) was dead or vestigial and has been removed in a single pass. Sidecar infrastructure deleted (dead in production since ~March 12, 2026): - SessionSidecarRunner.cs (entire file) — LLM sidecar call executor, only invoked by the recall fallback path that production never hits - SidecarMemoryObserver class and MemoryObservationRequest + subtypes — replaced by SessionMemoryObserverActor.RunDistillationAsync ~March 24 - MemoryObservationCompleted + its handler in LlmSessionActor — registered but no senders existed - RecallPlanningCompleted — zero callers - MemorySidecarPromptBuilder reduced to just BuildClassificationRules (the only method still used by the live distillation path) - The entire sidecar fallback path in SQLiteMemoryRecallCoordinator.RecallAsync (BuildPlanAsync, LogRecallTrace, TokenizeTerms, FallbackSearchTerms), including the IChatClientProvider / SidecarRecallPlanner / RecallPlanGate / SessionConfig constructor parameters that were only used by that path - SessionTuning.MemorySidecarsEnabled flag + its config binding + JSON schema entry + the SessionConfigDefaultsTests.Memory_sidecars_enabled_by_default assertion HardScope / domain-derived-boundary plumbing deleted: - DeterministicRetrievalRequestPlanner.ResolveHardScope and IsTransportScopedSession — session-ID prefix parsing that the coordinator bypassed because it populated HardScopeOverride first - HardScope field on DeterministicRetrievalRequestPlan, HardScopeOverride field on AutomaticRecallRequest, NormalizeRequest on the coordinator - All SecurityPolicyDefaults.InferLegacyBoundaryFromDomain call sites replaced with SecurityPolicyDefaults.TrustedInstanceBoundary. Netclaw is a single-instance trusted-boundary system; the "derive boundary from domain prefix" logic was dead code because ToMemoryDomain always returns project:default anyway - MemoryPolicyScopeResolver.ResolveBoundary simplified — no session-ID or domain heuristics, always returns TrustedInstanceBoundary when no explicit boundary is provided Search method cleanup: - SQLiteMemoryStore.SearchByPlanAsync (domain-scoped variant) collapsed into SearchAcrossDomainsByPlanAsync — the sole remaining search method - WHERE d.domain = ? / WHERE r.domain = ? filters removed from the SQL - domain parameter dropped from SidecarRecallPlanner.BuildRequest and RecallPlanningRequest Planner anchor-hint filter: - The filter that strips sentence-start capitalized stopwords from anchor hints (landed in #582) stays and expands slightly Test fallout cleaned up across ~20 test files: - MemorySidecarPromptBuilderTests.cs and SessionSidecarRunnerTests.cs deleted outright - Sidecar_observation_promotes_strong_user_assertion_into_memory test and the memory-observation-sidecar branch of the fake chat client removed from LlmSessionIntegrationTests - MemorySidecarsEnabled literals stripped from 17 test SessionTuning constructions - HardScopeOverride / plan.HardScope references removed from DeterministicRetrievalPlanningTests, DeterministicCandidateSelectorTests, MemoryRedesignedEvalSuiteTests, MemoryRecallScenarioTests - InferLegacyBoundaryFromDomain call sites in SQLiteMemoryStoreTests and MemoryRedesignedEvalSuiteTests updated to TrustedInstanceBoundary - Score_geometry_domain_affinity_boosts_same_domain_match test replaced with Domain_is_not_a_scoring_signal assertion Intentionally NOT touched (deferred to a follow-up PR): - ToMemoryDomain() method and its ~14 call sites — still returns the constant DefaultMemoryDomain, which is still written to the domain column - The Domain field on SQLiteMemoryDocument, SQLiteMemoryHydratedItem, SQLiteMemorySearchResult, SQLiteMemoryCurationOperation, SQLiteMemoryAnchor - The domain column on memory_documents, memory_records, memory_anchors, memory_edges tables - NormalizeLegacyBoundariesAsync (still reads the domain column) - SecurityPolicyDefaults.DefaultMemoryDomain and InferLegacyBoundaryFromDomain (the latter is now dead code but kept alongside DefaultMemoryDomain for the followup) Those are a separate, larger cleanup that requires schema-migration work and is riskier to land in the same commit. Tracked as the next step in #584. Net: ~1,400 lines deleted, ~60 added. All 1,967 tests green across every test project. Slopwatch clean.
Follow-on cleanup pass from code review of the previous commit. - Drop dead audience and sessionId parameters from MemoryPolicyScopeResolver.ResolveBoundary. Boundary is a single-valued constant (audience is the security axis), so neither parameter was read. Updated all 7 call sites across the curation pipeline, policy gates, and the SQLite memory tools. - Inline the private ResolveBoundary helper in SQLiteMemoryRecallCoordinator — it duplicated MemoryPolicyScopeResolver.ResolveBoundary after the simplification. Coordinator now uses the shared resolver directly. - Delete the MemoryObservationFailed and RecallPlanningFailed message records from LlmMessages.cs. Zero senders and zero receivers after the previous commit removed the sidecar handler. - Delete two WHERE 1 = 1 placeholder artifacts in SearchAcrossDomainsByPlanAsync — leftover scaffolding from the inlined domain-filter clauses that got collapsed in the previous commit. - Trim narrating-history comments down to the non-obvious WHY, or remove outright where the code already says what the comment restated. Net another ~30 lines deleted. All 840 Actors tests and the full 1,967-test solution suite still pass. Slopwatch clean.
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
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
Part one of a multi-step cleanup of the Netclaw memory subsystem. This PR deletes dead code only — no behavior changes for any test that was already passing, no schema migrations, no API surface that users could reach. It's a pure net-removal refactor that leaves the memory system easier to reason about before checkpoint 2 (the
Domainfield + schema migration) lands.Background
While closing #582 I discovered that the memory subsystem had two large pieces of dead infrastructure sitting as "scheduled for removal" tombstones since late March:
DeterministicCandidateSelectorfor recall and bySessionMemoryObserverActor.RunDistillationAsyncfor memory formation within ~2 weeks. The infrastructure got an explicit "Scheduled for removal" annotation on 2026-03-25 (PR refactor: consolidate SessionConfig — reduce 16 properties to ~5 meaningful settings #414) but the deletion never shipped. Zero production code paths reach any of it.HardScope/HardScopeOverride/ domain-derived boundary inference.DeterministicRetrievalRequestPlanner.ResolveHardScopeparsed a session ID prefix into a per-project domain, but the coordinator always populatedHardScopeOverridefirst viaProtocol.SessionId.ToMemoryDomain(), which unconditionally returnsSecurityPolicyDefaults.DefaultMemoryDomain. The session-ID parsing branch was unreachable. Domain affinity inDeterministicCandidateSelectorwas disabled in fix(memory): add composite-score floor to recall ranker (#582) #585 for the same reason.Beyond those two, the code review pass also flagged a handful of small residuals: dead parameters on
MemoryPolicyScopeResolver.ResolveBoundary, a duplicate private helper in the coordinator, dead message records inLlmMessages, and twoWHERE 1 = 1SQL placeholders. Those are cleaned up in the second commit.Design decision: "audience + lexical/facet/anchor matching, nothing else"
Before touching anything I verified with the project owner that the mental model going forward is:
Public⊂Team⊂Personal) is the only security / isolation axis.MemoryPolicyEvaluator.AllowedAudienceWireValuesalready does the correct walled-garden check, including the case where an operator in a privileged DM context should be able to reference memories formed in a team channel.That framing is the basis for every deletion in this PR. If a concept only existed to prop up one of the three things above (hard scope, LLM-driven sidecars, domain-derived boundaries) it's gone.
What this PR does
Sidecar infrastructure deleted
src/Netclaw.Actors/Sessions/SessionSidecarRunner.cssrc/Netclaw.Actors/Sessions/MemorySidecarPromptBuilder.csBuildClassificationRuleshelper still used by the liveSessionMemoryObserverActordistillation pathsrc/Netclaw.Actors/Sessions/SidecarRecallPlanner.csSidecarMemoryObserverclass deleted (observation sidecar).SidecarRecallPlannerclass retained — still used by the liveSqliteFindMemoriesToolto build structural request plans without any LLM involvement.domainparameter dropped fromBuildRequestsignature.src/Netclaw.Actors/Sessions/MemorySidecarContracts.csMemoryObservationRequest,MemoryObservationCurrentTurn,MemoryObservationRecentContext,MemoryObservationPolicyScope,MemoryObservationCompleted,RecallPlanningCompletedall deleted.MemoryProposal,MemoryAnchor,MemoryRelation,MemoryProposalOperation,RecallPlanningRequest,RecallQueryPlanretained — all live via the distillation path or thefind_memoriestool.src/Netclaw.Actors/Sessions/SQLiteMemoryRecallCoordinator.csif (!_sessionTuning.MemorySidecarsEnabled) …fallback branch inRecallAsync, plusBuildPlanAsync,LogRecallTrace,TokenizeTerms,FallbackSearchTerms,NormalizeRequest. Constructor parameters reduced from 6 (store, logger, clientProvider, sidecarPlanner, recallPlanGate, sessionTuning, sessionConfig) to 3 (store, logger, sessionTuning).src/Netclaw.Actors/Sessions/LlmSessionActor.csCommand<MemoryObservationCompleted>handler deleted — registered handler with zero senders.src/Netclaw.Actors/Sessions/LlmMessages.csMemoryObservationFailedandRecallPlanningFailedrecords deleted — also had no senders.src/Netclaw.Configuration/SessionTuning.csMemorySidecarsEnabledflag deleted. Config binding inSessionConfig, JSON schema entry innetclaw-config.v1.schema.json, and theMemory_sidecars_enabled_by_defaultdefault-check test inSessionConfigDefaultsTestsall removed to match.HardScope / domain plumbing deleted
src/Netclaw.Actors/Sessions/DeterministicRetrievalPlanning.csResolveHardScopeandIsTransportScopedSessiondeleted.HardScopeproperty removed fromDeterministicRetrievalRequestPlan.src/Netclaw.Actors/Sessions/IMemoryRecallCoordinator.csHardScopeOverridefield removed fromAutomaticRecallRequest.src/Netclaw.Actors/Sessions/Pipelines/SessionRecallManager.csHardScopeOverride: sessionId.ToMemoryDomain()argument from the recall request construction.Boundary resolution simplified
All Netclaw memory runs inside a single trusted-instance daemon.
InferLegacyBoundaryFromDomainultimately returnedTrustedInstanceBoundaryfor every input the system actually produces. Rather than thread that fiction through 7 call sites:SecurityPolicyDefaults.InferLegacyBoundaryFromDomain(...)in the recall path replaced withSecurityPolicyDefaults.TrustedInstanceBoundarydirectly. Curation-pipeline call sites are still on the old function and are deferred to checkpoint 2.MemoryPolicyScopeResolver.ResolveBoundarysimplified to the single-value case: if a caller-supplied boundary is present, trim and return it; otherwise returnTrustedInstanceBoundary. TheaudienceandsessionIdparameters became dead weight and were dropped from the signature and every call site (curation pipeline, policy gates, all four SQLite memory tools).SQLiteMemoryRecallCoordinator.ResolveBoundaryhelper was byte-identical to the scope-resolver method after simplification — inlined at the call site and deleted.Search method collapse
SQLiteMemoryStore.SearchByPlanAsync(the domain-scoped variant) andSearchAcrossDomainsByPlanAsyncwere merged into the single cross-domain method. Thedomainparameter is gone, theWHERE d.domain = $domain/WHERE r.domain = $domainSQL clauses are gone, and twoWHERE 1 = 1scaffolding artifacts left behind by the inlining were also removed.Test fallout
MemorySidecarPromptBuilderTests.cs(tested the deleted prompt builder methods) andSessionSidecarRunnerTests.cs(tested the deleted runner).Sidecar_observation_promotes_strong_user_assertion_into_memoryfromLlmSessionIntegrationTests, plus the "memory observation sidecar" branch of the fake chat client that only existed to serve that test.MemorySidecarsEnabled = false/trueliterals stripped from everySessionTuningconstructor call.HardScopeOverride: "user:aaron"dropped fromDeterministicRetrievalPlanningTests,MemoryRedesignedEvalSuiteTests.plan.HardScopeassertions removed.DeterministicCandidateSelectorTestsfixture update:MakePlanlost itshardScopeparameter. TheScore_geometry_domain_affinity_boosts_same_domain_matchtest from fix(memory): add composite-score floor to recall ranker (#582) #585 was replaced withDomain_is_not_a_scoring_signalwhich asserts same-domain and cross-domain candidates score identically.SQLiteMemoryStoreTestsandMemoryRedesignedEvalSuiteTestsswitched fromSearchByPlanAsync(..., "project:test", ..., InferLegacyBoundaryFromDomain(...), ...)toSearchAcrossDomainsByPlanAsync(..., TrustedInstanceBoundary, ...).What this PR deliberately does NOT do
These are all tracked as checkpoint 2 work:
Protocol.SessionId.ToMemoryDomain()is still a trivial constant passthrough with ~14 call sites. Inlining it is mechanical churn acrossLlmSessionActor,SessionMemoryObserverActor, the SQLite memory tools,SubAgentActor, andSessionToolExecutionPipeline.Domainfield onSQLiteMemoryDocument,SQLiteMemoryHydratedItem,SQLiteMemorySearchResult,SQLiteMemoryCurationOperation,SQLiteMemoryAnchor,AutomaticRecallItem, andMemoryCheckpointPayloadstays for now. Removing it touches every writer and reader of every memory record type.domaincolumn onmemory_documents,memory_records,memory_anchors,memory_edgesstays. Dropping it needs an idempotent schema migration path, and schema migration risk is better decoupled from this deletion-heavy PR.NormalizeLegacyBoundariesAsyncstill reads thedomaincolumn to bucket pre-existing data. Dead once the column is gone.SecurityPolicyDefaults.InferLegacyBoundaryFromDomainstill has two callers inMemoryCurationPipeline(lines 319, 508) that haven't been migrated yet.SecurityPolicyDefaults.DefaultMemoryDomainstays as the constant the column receives.openspec/specs/netclaw-agent-memory/spec.mdstill referencesdomainas part of the memory policy envelope and has a scenario aboutdomain=businessfiltering. The OpenSpec update will land alongside the checkpoint 2 PR so the spec describes the final landed state in one pass.Net effect
~1,450 lines deleted, ~80 added across two commits. All 1,967 tests across 7 test projects pass locally. Slopwatch clean (0 issues).
dotnet buildclean (0 warnings).Closes #584 in spirit — the decision captured there is "domain scoping is the wrong mental model; audience handles it." The formal issue close will follow the checkpoint 2 PR that finishes removing the vestigial column and field.
Test plan
dotnet buildgreen across all projects (0 warnings, 0 errors)dotnet test src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj— 840 passed / 0 faileddotnet testfull solution — 1,967 passed / 0 failed across 7 test projectsdotnet slopwatch analyze— 0 issues