From cb92ebdceccace4d22ba7fed2847fd1a0615e002 Mon Sep 17 00:00:00 2001 From: Netclaw Bot Date: Tue, 30 Jun 2026 20:14:25 +0000 Subject: [PATCH 1/7] fix: accept both colon and dash prefixes in MemoryTypedId.Parse The auto-recall block injects memory IDs in dash format (e.g. doc-bd5777c...) while the update_memory tool only recognized colon format (doc:abc123). This caused deletion failures for any memory surfaced via automatic recall. Accept both doc:/doc- and rec:/rec- prefixes in the Parse method so agents can use IDs from auto-recall, find_memories, or raw database storage interchangeably. --- .../Memory/MemoryTypedIdTests.cs | 108 ++++++++++++++++++ src/Netclaw.Actors/Memory/MemoryTypedId.cs | 10 +- 2 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs new file mode 100644 index 000000000..e5b40289d --- /dev/null +++ b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs @@ -0,0 +1,108 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Netclaw.Actors.Memory; + +namespace Netclaw.Actors.Tests.Memory; + +public class MemoryTypedIdTests +{ + [Theory] + [InlineData("doc:abc123", MemoryKind.Document, "abc123")] + [InlineData("rec:xyz789", MemoryKind.Record, "xyz789")] + [InlineData("DOC:upper", MemoryKind.Document, "upper")] + [InlineData("REC:UPPER", MemoryKind.Record, "UPPER")] + [InlineData("doc-abc123", MemoryKind.Document, "abc123")] + [InlineData("rec-xyz789", MemoryKind.Record, "xyz789")] + [InlineData("DOC-upper", MemoryKind.Document, "upper")] + [InlineData("REC-UPPER", MemoryKind.Record, "UPPER")] + public void Parse_accepts_both_colon_and_dash_prefixes(string raw, MemoryKind expectedKind, string expectedId) + { + var parsed = MemoryTypedId.Parse(raw); + Assert.Equal(expectedKind, parsed.Kind); + Assert.Equal(expectedId, parsed.Id); + } + + [Fact] + public void Parse_rejects_unrecognized_prefixes() + { + var parsed = MemoryTypedId.Parse("unknown-abc123"); + Assert.Equal(MemoryKind.Unknown, parsed.Kind); + Assert.Equal("unknown-abc123", parsed.Id); + } + + [Theory] + [InlineData("doc:abc123")] + [InlineData("rec:xyz789")] + [InlineData("doc-bd5777c5860146aab6a5304310eb20c5")] + [InlineData("rec-bd5777c5860146aab6a5304310eb20c5")] + [InlineData("")] + [InlineData("no-prefix")] + public void Parse_unknown_for_invalid_prefixes(string raw) + { + var parsed = MemoryTypedId.Parse(raw); + Assert.Equal(MemoryKind.Unknown, parsed.Kind); + } + + [Fact] + public void ToWireValue_returns_colon_format() + { + var doc = new MemoryTypedId(MemoryKind.Document, "abc123"); + var rec = new MemoryTypedId(MemoryKind.Record, "xyz789"); + var unknown = new MemoryTypedId(MemoryKind.Unknown, "orphan"); + + Assert.Equal("doc:abc123", doc.ToWireValue()); + Assert.Equal("rec:xyz789", rec.ToWireValue()); + Assert.Equal("orphan", unknown.ToWireValue()); + } + + [Fact] + public void ToString_matches_ToWireValue() + { + var id = new MemoryTypedId(MemoryKind.Document, "abc123"); + Assert.Equal("doc:abc123", id.ToString()); + } + + [Fact] + public void NewDocumentId_returns_dash_format() + { + var id = MemoryTypedId.NewDocumentId(); + Assert.StartsWith("doc-", id); + Assert.Equal(36 + 4, id.Length); // "doc-" + 32-char GUID (with dashes) + } + + [Fact] + public void NewRecordId_returns_dash_format() + { + var id = MemoryTypedId.NewRecordId(); + Assert.StartsWith("rec-", id); + Assert.Equal(36 + 4, id.Length); + } + + [Fact] + public void Round_trip_dash_to_parse_to_wire() + { + // Simulates auto-recall output: agent receives "doc-{guid}" + var generated = MemoryTypedId.NewDocumentId(); // e.g. "doc-bd5777c5860146aab6a5304310eb20c5" + var parsed = MemoryTypedId.Parse(generated); + var wire = parsed.ToWireValue(); // e.g. "doc:bd5777c5860146aab6a5304310eb20c5" + + Assert.Equal(MemoryKind.Document, parsed.Kind); + Assert.Contains("bd5777c", wire); // ID portion preserved + Assert.StartsWith("doc:", wire); // wire uses colon + } + + [Fact] + public void Round_trip_wire_to_parse_to_string() + { + // Simulates find_memories output: agent receives "doc:{guid}" + var wire = "doc:abc123"; + var parsed = MemoryTypedId.Parse(wire); + var output = parsed.ToString(); + + Assert.Equal(MemoryKind.Document, parsed.Kind); + Assert.Equal("doc:abc123", output); + } +} diff --git a/src/Netclaw.Actors/Memory/MemoryTypedId.cs b/src/Netclaw.Actors/Memory/MemoryTypedId.cs index 5e021f7dc..484d32199 100644 --- a/src/Netclaw.Actors/Memory/MemoryTypedId.cs +++ b/src/Netclaw.Actors/Memory/MemoryTypedId.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -23,13 +23,17 @@ public readonly record struct MemoryTypedId(MemoryKind Kind, string Id) /// /// Parses a prefixed string like "doc:abc123" or "rec:def456" into a typed ID. + /// Also accepts the dash-separated wire representation (e.g. "doc-abc123") for + /// compatibility with auto-recall output and raw database storage. /// Returns with the raw value when the prefix is unrecognized. /// public static MemoryTypedId Parse(string raw) { - if (raw.StartsWith("doc:", StringComparison.OrdinalIgnoreCase)) + if (raw.StartsWith("doc:", StringComparison.OrdinalIgnoreCase) + || raw.StartsWith("doc-", StringComparison.OrdinalIgnoreCase)) return new MemoryTypedId(MemoryKind.Document, raw[4..]); - if (raw.StartsWith("rec:", StringComparison.OrdinalIgnoreCase)) + if (raw.StartsWith("rec:", StringComparison.OrdinalIgnoreCase) + || raw.StartsWith("rec-", StringComparison.OrdinalIgnoreCase)) return new MemoryTypedId(MemoryKind.Record, raw[4..]); return new MemoryTypedId(MemoryKind.Unknown, raw); } From 344eecea2e576346c11acb93a35406a1e9178e9a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 00:20:08 +0000 Subject: [PATCH 2/7] fix(memory): preserve stable memory handles --- .../.system/files/netclaw-memory/SKILL.md | 12 +- .../Memory/MemoryEvalSeedSuiteTests.cs | 4 +- .../Memory/MemoryRedesignedEvalSuiteTests.cs | 4 +- .../Memory/MemoryTypedIdTests.cs | 55 ++++-- .../Memory/SQLiteMemoryStoreTests.cs | 43 ++++ .../Memory/SqliteMemoryToolsTests.cs | 145 ++++++++++++++ .../DeterministicRetrievalPlanningTests.cs | 10 +- .../Sessions/MemoryRecallScenarioTests.cs | 4 +- .../Sessions/SessionMessageAssemblerTests.cs | 23 ++- src/Netclaw.Actors/Memory/MemoryTypedId.cs | 69 +++++-- .../Memory/SQLiteMemoryStore.cs | 185 ++++++++++++++++-- .../Memory/SqliteGetMemoriesTool.cs | 14 +- .../Memory/SqliteUpdateMemoryTool.cs | 116 +++++------ .../Sessions/IMemoryRecallCoordinator.cs | 11 +- .../Sessions/LlmSessionActor.cs | 2 +- .../Pipelines/SessionRecallManager.cs | 4 +- .../Sessions/SQLiteMemoryRecallCoordinator.cs | 4 +- .../Sessions/SessionMessageAssembler.cs | 4 +- src/Netclaw.Daemon/Program.cs | 4 +- 19 files changed, 565 insertions(+), 148 deletions(-) diff --git a/feeds/skills/.system/files/netclaw-memory/SKILL.md b/feeds/skills/.system/files/netclaw-memory/SKILL.md index 315e98d78..5956261ac 100644 --- a/feeds/skills/.system/files/netclaw-memory/SKILL.md +++ b/feeds/skills/.system/files/netclaw-memory/SKILL.md @@ -1,9 +1,9 @@ --- name: netclaw-memory -description: "REQUIRED when the user asks what you remember, recall, or know from past conversations, previous sessions, or cross-session memory. Also before using memory tools: find_memories, get_memories, store_memory, update_memory." +description: "REQUIRED when the user asks what you remember, recall, or know from past conversations, previous sessions, cross-session memory, memory classes, or memory types. Also before using memory tools: find_memories, get_memories, store_memory, update_memory." metadata: author: netclaw - version: "1.5.0" + version: "1.6.1" --- # Netclaw Memory @@ -40,6 +40,9 @@ Both gates must pass for memory to function. - **Explicit tools** are a manual-control layer on top of automatic recall. - Memory is SQLite-backed and cross-session only within the active domain/boundary policy envelope. +- Memory IDs shown by automatic recall, `find_memories`, and `get_memories` + are stable handles. Reuse them directly with `get_memories` or + `update_memory`; do not rewrite `doc:` / `rec:` prefixes by hand. ## When to Use Explicit Tools @@ -84,6 +87,11 @@ Automatic observation note: Use only to correct or supersede an existing memory. +Use the memory ID exactly as shown by automatic recall, `find_memories`, or +`get_memories`. For documents, prefer `new_content` when replacing a full +hydrated memory. Use `old_text` + `new_text` only when making a precise +find-and-replace edit. To delete a memory, pass `delete: true`. + ## Memory Classes | Class | Recall | Expiry | diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryEvalSeedSuiteTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryEvalSeedSuiteTests.cs index 181af38f0..c9f342ffa 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryEvalSeedSuiteTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryEvalSeedSuiteTests.cs @@ -58,7 +58,7 @@ await _store.UpsertDocumentAsync(new SQLiteMemoryDocument( MaxItems: 3), TestContext.Current.CancellationToken); Assert.False(result.Degraded); - Assert.Contains(result.Items, i => i.Id == "doc-ops"); + Assert.Contains(result.Items, i => i.Id.Value == "doc-ops"); Assert.True(result.Items.Count <= 3); } @@ -95,7 +95,7 @@ await _store.UpsertDocumentAsync(new SQLiteMemoryDocument( MaxItems: 3), TestContext.Current.CancellationToken); Assert.False(result.Degraded); - Assert.DoesNotContain(result.Items, i => i.Id == "doc-secret"); + Assert.DoesNotContain(result.Items, i => i.Id.Value == "doc-secret"); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryRedesignedEvalSuiteTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryRedesignedEvalSuiteTests.cs index 08f01f9bf..874b73560 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryRedesignedEvalSuiteTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryRedesignedEvalSuiteTests.cs @@ -261,7 +261,7 @@ await _store.ApplyCurationBatchAsync( ["where should I stay near Stir Trek"], 3), TestContext.Current.CancellationToken); - Assert.DoesNotContain(auto.Items, x => x.Id == "rec-hotel-evidence"); + Assert.DoesNotContain(auto.Items, x => x.Id.Value == "rec-hotel-evidence"); var tool = new SqliteFindMemoriesTool(_store, _timeProvider); var search = await tool.ExecuteAsync( @@ -572,7 +572,7 @@ await _store.ApplyCurationBatchAsync( var intentionalEvidenceHitRate = search.Contains("Hotel options", StringComparison.Ordinal) ? 1.0 : 0.0; var gateCorrectness = acceptedFact.Count == 1 ? 1.0 : 0.0; var explicitWriteTruthfulness = acceptedFact.Count == 1 ? 1.0 : 0.0; - var evidenceLeakage = auto.Items.Any(x => x.Id == "rec-report-evidence") ? 1.0 : 0.0; + var evidenceLeakage = auto.Items.Any(x => x.Id.Value == "rec-report-evidence") ? 1.0 : 0.0; Assert.Contains("stale=true", staleDebug); diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs index e5b40289d..493adc726 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs @@ -14,15 +14,15 @@ public class MemoryTypedIdTests [InlineData("rec:xyz789", MemoryKind.Record, "xyz789")] [InlineData("DOC:upper", MemoryKind.Document, "upper")] [InlineData("REC:UPPER", MemoryKind.Record, "UPPER")] - [InlineData("doc-abc123", MemoryKind.Document, "abc123")] - [InlineData("rec-xyz789", MemoryKind.Record, "xyz789")] - [InlineData("DOC-upper", MemoryKind.Document, "upper")] - [InlineData("REC-UPPER", MemoryKind.Record, "UPPER")] - public void Parse_accepts_both_colon_and_dash_prefixes(string raw, MemoryKind expectedKind, string expectedId) + [InlineData("doc-abc123", MemoryKind.Document, "doc-abc123")] + [InlineData("rec-xyz789", MemoryKind.Record, "rec-xyz789")] + [InlineData("DOC-upper", MemoryKind.Document, "DOC-upper")] + [InlineData("REC-UPPER", MemoryKind.Record, "REC-UPPER")] + public void Parse_accepts_canonical_and_legacy_raw_ids(string raw, MemoryKind expectedKind, string expectedId) { var parsed = MemoryTypedId.Parse(raw); Assert.Equal(expectedKind, parsed.Kind); - Assert.Equal(expectedId, parsed.Id); + Assert.Equal(expectedId, parsed.Id.Value); } [Fact] @@ -30,16 +30,13 @@ public void Parse_rejects_unrecognized_prefixes() { var parsed = MemoryTypedId.Parse("unknown-abc123"); Assert.Equal(MemoryKind.Unknown, parsed.Kind); - Assert.Equal("unknown-abc123", parsed.Id); + Assert.Equal("unknown-abc123", parsed.Id.Value); } [Theory] - [InlineData("doc:abc123")] - [InlineData("rec:xyz789")] - [InlineData("doc-bd5777c5860146aab6a5304310eb20c5")] - [InlineData("rec-bd5777c5860146aab6a5304310eb20c5")] [InlineData("")] [InlineData("no-prefix")] + [InlineData("anchor:netclaw")] public void Parse_unknown_for_invalid_prefixes(string raw) { var parsed = MemoryTypedId.Parse(raw); @@ -69,29 +66,27 @@ public void ToString_matches_ToWireValue() public void NewDocumentId_returns_dash_format() { var id = MemoryTypedId.NewDocumentId(); - Assert.StartsWith("doc-", id); - Assert.Equal(36 + 4, id.Length); // "doc-" + 32-char GUID (with dashes) + Assert.StartsWith("doc-", id.Value); + Assert.Equal(36, id.Value.Length); } [Fact] public void NewRecordId_returns_dash_format() { var id = MemoryTypedId.NewRecordId(); - Assert.StartsWith("rec-", id); - Assert.Equal(36 + 4, id.Length); + Assert.StartsWith("rec-", id.Value); + Assert.Equal(36, id.Value.Length); } [Fact] public void Round_trip_dash_to_parse_to_wire() { - // Simulates auto-recall output: agent receives "doc-{guid}" - var generated = MemoryTypedId.NewDocumentId(); // e.g. "doc-bd5777c5860146aab6a5304310eb20c5" - var parsed = MemoryTypedId.Parse(generated); - var wire = parsed.ToWireValue(); // e.g. "doc:bd5777c5860146aab6a5304310eb20c5" + var generated = MemoryTypedId.NewDocumentId(); + var parsed = MemoryTypedId.Parse(generated.Value); + var wire = parsed.ToWireValue(); Assert.Equal(MemoryKind.Document, parsed.Kind); - Assert.Contains("bd5777c", wire); // ID portion preserved - Assert.StartsWith("doc:", wire); // wire uses colon + Assert.Equal($"doc:{generated.Value}", wire); } [Fact] @@ -105,4 +100,22 @@ public void Round_trip_wire_to_parse_to_string() Assert.Equal(MemoryKind.Document, parsed.Kind); Assert.Equal("doc:abc123", output); } + + [Fact] + public void CandidateStorageIds_include_legacy_prefixed_candidate_for_bare_handle_payload() + { + var parsed = MemoryTypedId.Parse("doc:abc123"); + var candidates = parsed.CandidateStorageIds().Select(x => x.Value).ToArray(); + + Assert.Equal(["abc123", "doc-abc123"], candidates); + } + + [Fact] + public void CandidateStorageIds_preserve_legacy_raw_storage_id() + { + var parsed = MemoryTypedId.Parse("doc-abc123"); + var candidates = parsed.CandidateStorageIds().Select(x => x.Value).ToArray(); + + Assert.Equal(["doc-abc123"], candidates); + } } diff --git a/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs b/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs index accfaf5bc..ee6b76f4b 100644 --- a/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs @@ -242,6 +242,28 @@ await _store.UpsertDocumentAsync(new SQLiteMemoryDocument( Assert.Contains(personalResults, x => x.Id == "doc-personal"); } + [Fact] + public async Task ResolveMemoryHandleAsync_fails_loudly_when_canonical_handle_is_ambiguous() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + + var anchor = _store.CreateDefaultAnchor("ambiguous-memory"); + var now = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); + await _store.UpsertDocumentAsync(CreateDocument("abc", anchor, "Bare ID", now), TestContext.Current.CancellationToken); + await _store.UpsertDocumentAsync(CreateDocument("doc-abc", anchor, "Legacy ID", now), TestContext.Current.CancellationToken); + + var resolved = await _store.ResolveMemoryHandleAsync( + "doc:abc", + TrustBoundary.TrustedInstanceValue, + TrustAudience.Personal, + TestContext.Current.CancellationToken); + + Assert.False(resolved.Resolved); + Assert.Contains("ambiguous", resolved.Error, StringComparison.OrdinalIgnoreCase); + Assert.Contains("doc:abc", resolved.Error); + Assert.Contains("doc:doc-abc", resolved.Error); + } + public ValueTask InitializeAsync() => ValueTask.CompletedTask; public async ValueTask DisposeAsync() @@ -276,4 +298,25 @@ private static async Task TryDeleteDirectoryAsync(string path) // Best effort cleanup: file handles can remain briefly open on Windows CI. // Leaving temp dirs behind is preferable to failing the test run. } + + private static SQLiteMemoryDocument CreateDocument(string id, SQLiteMemoryAnchor anchor, string title, long now) + => new( + DocumentId: id, + Anchor: anchor, + MemoryClass: MemoryClass.DurableFact.ToWireValue(), + Title: title, + MarkdownBody: $"Content for {title}.", + AliasesJson: null, + FacetsJson: null, + SlotsJson: null, + UpdateSemantics: MemoryUpdateSemantics.MergeDocument.ToWireValue(), + Sensitivity: MemorySensitivity.Normal.ToWireValue(), + RecallMode: MemoryRecallMode.Auto.ToWireValue(), + Confidence: 0.9, + FreshnessAtMs: now, + ExpiresAtMs: null, + CreatedAtMs: now, + UpdatedAtMs: now, + Boundary: TrustBoundary.TrustedInstanceValue, + Audience: TrustAudience.Team.ToWireValue()); } diff --git a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs index 207d923d6..540054bd1 100644 --- a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs @@ -276,9 +276,154 @@ await _store.ApplyCurationBatchAsync( Assert.DoesNotContain("Security issue", result); } + [Fact] + public async Task GetMemories_accepts_legacy_raw_and_typed_storage_ids() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + + await StoreDocumentAsync( + "doc-auto", + "Auto recalled note", + "This came from automatic recall.", + now); + + var tool = new SqliteGetMemoriesTool(_store, _timeProvider); + var rawResult = await tool.ExecuteAsync( + new Dictionary { ["Ids"] = "doc-auto" }, + PersonalContext(), + CancellationToken.None); + var typedResult = await tool.ExecuteAsync( + new Dictionary { ["Ids"] = "doc:doc-auto" }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("Auto recalled note", rawResult); + Assert.Contains("doc:doc-auto", rawResult); + Assert.Contains("Auto recalled note", typedResult); + Assert.Contains("doc:doc-auto", typedResult); + } + + [Fact] + public async Task UpdateMemory_edits_document_from_typed_recall_id_without_checkpoint_clobber() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + await StoreDocumentAsync("doc-edit", "Favorite color", "The user's favorite color is blue.", now); + + var update = new SqliteUpdateMemoryTool(_store); + var result = await update.ExecuteAsync( + new Dictionary + { + ["id"] = "doc:doc-edit", + ["old_text"] = "blue", + ["new_text"] = "green" + }, + PersonalContext(), + CancellationToken.None); + + var get = new SqliteGetMemoriesTool(_store, _timeProvider); + var hydrated = await get.ExecuteAsync( + new Dictionary { ["ids"] = "doc:doc-edit" }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("updated", result); + Assert.Contains("green", hydrated); + Assert.DoesNotContain("blue", hydrated); + Assert.Equal(0, await _store.GetPendingCheckpointCountAsync(CancellationToken.None)); + } + + [Fact] + public async Task UpdateMemory_replaces_document_content_from_legacy_raw_id() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + await StoreDocumentAsync("doc-replace", "Working preference", "Use short replies.", now); + + var update = new SqliteUpdateMemoryTool(_store); + var result = await update.ExecuteAsync( + new Dictionary + { + ["id"] = "doc-replace", + ["new_content"] = "Use direct replies with command examples when useful." + }, + PersonalContext(), + CancellationToken.None); + + var get = new SqliteGetMemoriesTool(_store, _timeProvider); + var hydrated = await get.ExecuteAsync( + new Dictionary { ["ids"] = "doc:doc-replace" }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("updated", result); + Assert.Contains("direct replies", hydrated); + Assert.DoesNotContain("short replies", hydrated); + } + + [Fact] + public async Task UpdateMemory_tombstones_document_from_typed_recall_id() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + await StoreDocumentAsync("doc-delete", "Delete me", "This memory should be removed from search.", now); + + var update = new SqliteUpdateMemoryTool(_store); + var result = await update.ExecuteAsync( + new Dictionary + { + ["id"] = "doc:doc-delete", + ["delete"] = true + }, + PersonalContext(), + CancellationToken.None); + + var find = new SqliteFindMemoriesTool(_store, _timeProvider); + var search = await find.ExecuteAsync( + new Dictionary { ["query"] = "removed search", ["limit"] = 5 }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("tombstoned", result); + Assert.Equal("No memories found.", search); + } + public async ValueTask DisposeAsync() { await SqliteTempDirectoryCleanup.TryDeleteDirectoryAsync(_baseDir); } + private async Task StoreDocumentAsync(string id, string title, string content, long now) + { + await _store.ApplyCurationBatchAsync( + $"cp-{id}", + [ + new SQLiteMemoryCurationOperation( + Kind: MemoryKind.Document.ToWireValue(), + MemoryClass: MemoryClass.DurableFact.ToWireValue(), + MemoryId: id, + AnchorCanonicalName: title, + AnchorType: "concept", + Title: title, + Content: content, + AliasesJson: null, + FacetsJson: null, + SlotsJson: null, + Relations: null, + UpdateSemantics: MemoryUpdateSemantics.MergeDocument.ToWireValue(), + Boundary: TrustBoundary.TrustedInstanceValue, + Audience: TrustAudience.Team, + Sensitivity: MemorySensitivity.Normal.ToWireValue(), + RecallMode: MemoryRecallMode.Auto.ToWireValue(), + Confidence: 0.9, + FreshnessAtMs: now, + ExpiresAtMs: null) + ], + CancellationToken.None); + } + + private static ToolExecutionContext PersonalContext() + => new("slack/thread-1", sessionDirectory: null) { Audience = TrustAudience.Personal }; + } diff --git a/src/Netclaw.Actors.Tests/Sessions/DeterministicRetrievalPlanningTests.cs b/src/Netclaw.Actors.Tests/Sessions/DeterministicRetrievalPlanningTests.cs index e2b6a447b..102e9457d 100644 --- a/src/Netclaw.Actors.Tests/Sessions/DeterministicRetrievalPlanningTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/DeterministicRetrievalPlanningTests.cs @@ -113,7 +113,7 @@ await store.UpsertDocumentAsync(new SQLiteMemoryDocument( ThreadTitle: "Product planning"), TestContext.Current.CancellationToken); Assert.False(result.Degraded); - Assert.Contains(result.Items, x => x.Id == "doc-textforge-pricing"); + Assert.Contains(result.Items, x => x.Id.Value == "doc-textforge-pricing"); } [Fact] @@ -157,7 +157,7 @@ await store.UpsertDocumentAsync(new SQLiteMemoryDocument( MaxItems: 3, ThreadTitle: "Product planning"), TestContext.Current.CancellationToken); - var item = Assert.Single(result.Items, x => x.Id == "doc-textforge-pricing-score"); + var item = Assert.Single(result.Items, x => x.Id.Value == "doc-textforge-pricing-score"); Assert.True(item.Score > 4.0, $"Expected composite score to exceed raw lexical score, got {item.Score:F2}"); } @@ -235,7 +235,7 @@ await store.UpsertDocumentAsync(new SQLiteMemoryDocument( MaxItems: 3), TestContext.Current.CancellationToken); Assert.False(result.Degraded); - Assert.Contains(result.Items, x => x.Id == "doc-reelfarm-research"); + Assert.Contains(result.Items, x => x.Id.Value == "doc-reelfarm-research"); } [Fact] @@ -279,7 +279,7 @@ await store.UpsertDocumentAsync(new SQLiteMemoryDocument( MaxItems: 3), TestContext.Current.CancellationToken); Assert.False(result.Degraded); - Assert.Contains(result.Items, x => x.Id == "doc-company-info"); + Assert.Contains(result.Items, x => x.Id.Value == "doc-company-info"); } [Fact] @@ -324,6 +324,6 @@ await store.UpsertDocumentAsync(new SQLiteMemoryDocument( ThreadTitle: "General DM"), TestContext.Current.CancellationToken); Assert.False(result.Degraded); - Assert.Contains(result.Items, x => x.Id == "doc-textforge-business-context"); + Assert.Contains(result.Items, x => x.Id.Value == "doc-textforge-business-context"); } } diff --git a/src/Netclaw.Actors.Tests/Sessions/MemoryRecallScenarioTests.cs b/src/Netclaw.Actors.Tests/Sessions/MemoryRecallScenarioTests.cs index 7a34fba96..3fc2f484f 100644 --- a/src/Netclaw.Actors.Tests/Sessions/MemoryRecallScenarioTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/MemoryRecallScenarioTests.cs @@ -189,8 +189,8 @@ public async Task Scenario_matches_expected_and_rejects_forbidden( result.Degraded, $"[{scenarioId}] recall degraded: {result.DegradeStage}/{result.DegradeReason}"); - var returnedIds = result.Items.Select(i => i.Id).ToArray(); - var returnedWithScores = string.Join(", ", result.Items.Select(i => $"{i.Id}={i.Score:F3}")); + var returnedIds = result.Items.Select(i => i.Id.Value).ToArray(); + var returnedWithScores = string.Join(", ", result.Items.Select(i => $"{i.Id.Value}={i.Score:F3}")); foreach (var expected in expectedIds) { diff --git a/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs b/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs index 195233f23..032ee6636 100644 --- a/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs @@ -153,6 +153,27 @@ public void BuildVolatileContextBlock_composes_recall_and_slash_command() Assert.Contains("[skill] do a thing", block); } + [Fact] + public void BuildVolatileContextBlock_formats_recall_ids_as_typed_memory_handles() + { + var input = MakeInput( + SeedHistory("hi"), + new AutomaticRecallResult(Items: + [ + new AutomaticRecallItem( + Id: new Netclaw.Actors.Memory.MemoryStorageId("doc-auto"), + Title: "auto memory", + Content: "remembered content", + Sensitivity: "normal", + Score: 0.9) + ])); + + var block = SessionMessageAssembler.BuildVolatileContextBlock(input); + + Assert.Contains("[doc:doc-auto]", block); + Assert.DoesNotContain("[doc-auto]", block); + } + [Fact] public void Static_block_contains_session_id_and_attachment_hint() { @@ -484,7 +505,7 @@ private static AutomaticRecallResult FakeRecall(string content) return new AutomaticRecallResult(Items: new[] { new AutomaticRecallItem( - Id: "mem/1", + Id: new Netclaw.Actors.Memory.MemoryStorageId("mem/1"), Title: "test memory", Content: content, Sensitivity: "public", diff --git a/src/Netclaw.Actors/Memory/MemoryTypedId.cs b/src/Netclaw.Actors/Memory/MemoryTypedId.cs index 484d32199..0e22db371 100644 --- a/src/Netclaw.Actors/Memory/MemoryTypedId.cs +++ b/src/Netclaw.Actors/Memory/MemoryTypedId.cs @@ -5,45 +5,84 @@ // ----------------------------------------------------------------------- namespace Netclaw.Actors.Memory; +public readonly record struct MemoryStorageId(string Value) +{ + public bool IsEmpty => string.IsNullOrWhiteSpace(Value); + + public override string ToString() => Value; +} + /// -/// Strongly-typed memory identity with kind prefix (doc: or rec:). -/// Centralizes ID parsing, formatting, and generation for the memory subsystem. +/// Strongly-typed model-facing memory handle with kind prefix (doc: or rec:). +/// Storage IDs are opaque and may include legacy doc-/rec- prefixes. /// -public readonly record struct MemoryTypedId(MemoryKind Kind, string Id) +public readonly record struct MemoryTypedId(MemoryKind Kind, MemoryStorageId Id) { + public MemoryTypedId(MemoryKind kind, string id) + : this(kind, new MemoryStorageId(id)) + { + } + /// /// Formats as the prefixed wire representation: "doc:{id}" or "rec:{id}". /// public string ToWireValue() => Kind switch { - MemoryKind.Document => $"doc:{Id}", - MemoryKind.Record => $"rec:{Id}", - _ => Id + MemoryKind.Document => $"doc:{Id.Value}", + MemoryKind.Record => $"rec:{Id.Value}", + _ => Id.Value }; /// - /// Parses a prefixed string like "doc:abc123" or "rec:def456" into a typed ID. - /// Also accepts the dash-separated wire representation (e.g. "doc-abc123") for - /// compatibility with auto-recall output and raw database storage. + /// Formats a storage ID for model-visible output. Existing storage IDs are + /// not rewritten; the kind prefix is added as the tool handle envelope. + /// + public static string ToWireValue(MemoryKind kind, string storageId) + => new MemoryTypedId(kind, storageId).ToWireValue(); + + public static string ToWireValue(MemoryKind kind, MemoryStorageId storageId) + => new MemoryTypedId(kind, storageId).ToWireValue(); + + /// + /// Parses a model-visible handle like "doc:abc123" or "rec:def456". + /// Also accepts legacy raw storage IDs such as "doc-abc123" and "rec-def456". /// Returns with the raw value when the prefix is unrecognized. /// public static MemoryTypedId Parse(string raw) { - if (raw.StartsWith("doc:", StringComparison.OrdinalIgnoreCase) - || raw.StartsWith("doc-", StringComparison.OrdinalIgnoreCase)) + raw = raw.Trim(); + if (raw.StartsWith("doc:", StringComparison.OrdinalIgnoreCase)) return new MemoryTypedId(MemoryKind.Document, raw[4..]); - if (raw.StartsWith("rec:", StringComparison.OrdinalIgnoreCase) - || raw.StartsWith("rec-", StringComparison.OrdinalIgnoreCase)) + if (raw.StartsWith("rec:", StringComparison.OrdinalIgnoreCase)) return new MemoryTypedId(MemoryKind.Record, raw[4..]); + if (raw.StartsWith("doc-", StringComparison.OrdinalIgnoreCase)) + return new MemoryTypedId(MemoryKind.Document, raw); + if (raw.StartsWith("rec-", StringComparison.OrdinalIgnoreCase)) + return new MemoryTypedId(MemoryKind.Record, raw); return new MemoryTypedId(MemoryKind.Unknown, raw); } + public IReadOnlyList CandidateStorageIds() => Kind switch + { + MemoryKind.Document => CandidateStorageIdsFor("doc-"), + MemoryKind.Record => CandidateStorageIdsFor("rec-"), + _ => [Id] + }; + + private IReadOnlyList CandidateStorageIdsFor(string legacyPrefix) + { + if (Id.Value.StartsWith(legacyPrefix, StringComparison.OrdinalIgnoreCase)) + return [Id]; + + return [Id, new MemoryStorageId(legacyPrefix + Id.Value)]; + } + public override string ToString() => ToWireValue(); public static string AnchorId(string canonicalName) => $"anchor:{canonicalName.Trim().ToLowerInvariant().Replace(' ', '-')}"; - public static string NewDocumentId() => $"doc-{Guid.NewGuid():N}"; + public static MemoryStorageId NewDocumentId() => new($"doc-{Guid.NewGuid():N}"); - public static string NewRecordId() => $"rec-{Guid.NewGuid():N}"; + public static MemoryStorageId NewRecordId() => new($"rec-{Guid.NewGuid():N}"); } diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index fc8d4dee0..40097f2a1 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -620,17 +620,16 @@ public async Task> GetMemoriesByIdsAsync if (ids.Count == 0) return []; - var documents = ids - .Select(ParseTypedId) - .Where(x => x.Kind is MemoryKind.Document or MemoryKind.Unknown) - .Select(x => x.Id) + var resolvedIds = await ResolveMemoryHandlesAsync(ids, boundary, audience, ct); + var documents = resolvedIds + .Where(x => x.Resolved && x.Kind == MemoryKind.Document) + .Select(x => x.StorageId!.Value.Value) .Distinct(StringComparer.OrdinalIgnoreCase) .ToArray(); - var records = ids - .Select(ParseTypedId) - .Where(x => x.Kind is MemoryKind.Record or MemoryKind.Unknown) - .Select(x => x.Id) + var records = resolvedIds + .Where(x => x.Resolved && x.Kind == MemoryKind.Record) + .Select(x => x.StorageId!.Value.Value) .Distinct(StringComparer.OrdinalIgnoreCase) .ToArray(); @@ -718,6 +717,60 @@ FROM memory_records }, ct); } + public async Task> ResolveMemoryHandlesAsync( + IReadOnlyList rawIds, + string boundary, + TrustAudience audience, + CancellationToken ct = default) + { + var output = new List(rawIds.Count); + foreach (var rawId in rawIds) + output.Add(await ResolveMemoryHandleAsync(rawId, boundary, audience, ct)); + return output; + } + + public async Task ResolveMemoryHandleAsync( + string rawId, + string boundary, + TrustAudience audience, + CancellationToken ct = default) + { + var parsed = MemoryTypedId.Parse(rawId); + if (parsed.Kind is not (MemoryKind.Document or MemoryKind.Record)) + return ResolvedMemoryHandle.Failed(rawId, parsed.Kind, "ID must be prefixed with doc: or rec:."); + + var candidates = parsed.CandidateStorageIds() + .Where(x => !x.IsEmpty) + .GroupBy(x => x.Value, StringComparer.OrdinalIgnoreCase) + .Select(x => x.First()) + .ToArray(); + if (candidates.Length == 0) + return ResolvedMemoryHandle.Failed(rawId, parsed.Kind, "ID payload is required."); + + var allowedAudiences = MemoryPolicyEvaluator.AllowedAudienceWireValues(audience) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + return await WithConnectionAsync(async (conn, ct) => + { + var matches = new List(); + foreach (var candidate in candidates) + { + if (await MemoryIdVisibleAsync(conn, parsed.Kind, candidate, boundary, allowedAudiences, ct)) + matches.Add(candidate); + } + + return matches.Count switch + { + 1 => ResolvedMemoryHandle.Found(rawId, parsed.Kind, matches[0]), + 0 => ResolvedMemoryHandle.Failed(rawId, parsed.Kind, $"Memory \"{rawId}\" was not found or is not accessible from this session."), + _ => ResolvedMemoryHandle.Failed( + rawId, + parsed.Kind, + $"Memory ID \"{rawId}\" is ambiguous. Use one of: {string.Join(", ", matches.Select(x => MemoryTypedId.ToWireValue(parsed.Kind, x)))}.") + }; + }, ct); + } + public async Task> SearchByPlanAsync( IReadOnlyList queryTerms, IReadOnlyList memoryClasses, @@ -916,6 +969,52 @@ UPDATE memory_documents }, ct); } + public async Task ReplaceDocumentTextAsync(string documentId, string newText, CancellationToken ct = default) + { + return await WithConnectionAsync(async (conn, ct) => + { + await using var tx = (SqliteTransaction)await conn.BeginTransactionAsync(ct); + + await using var read = conn.CreateCommand(); + read.Transaction = tx; + read.CommandText = "SELECT title, aliases_json, facets_json, recall_mode FROM memory_documents WHERE document_id = $id;"; + read.Parameters.AddWithValue("$id", documentId); + + string title; + string? aliasesJson; + string? facetsJson; + string recallMode; + await using (var reader = await read.ExecuteReaderAsync(ct)) + { + if (!await reader.ReadAsync(ct)) + return false; + title = reader.GetString(0); + aliasesJson = reader.IsDBNull(1) ? null : reader.GetString(1); + facetsJson = reader.IsDBNull(2) ? null : reader.GetString(2); + recallMode = reader.GetString(3); + } + + await using var write = conn.CreateCommand(); + write.Transaction = tx; + write.CommandText = """ + UPDATE memory_documents + SET markdown_body = $body, + updated_at = $updatedAt + WHERE document_id = $id; + """; + write.Parameters.AddWithValue("$id", documentId); + write.Parameters.AddWithValue("$body", newText); + write.Parameters.AddWithValue("$updatedAt", _timeProvider.GetUtcNow().ToUnixTimeMilliseconds()); + var affected = await write.ExecuteNonQueryAsync(ct); + + if (affected > 0 && IsSearchableRecallMode(recallMode)) + await UpsertDocumentFtsAsync(conn, tx, documentId, title, newText, aliasesJson, facetsJson, ct); + + await tx.CommitAsync(ct); + return affected > 0; + }, ct); + } + public async Task TombstoneDocumentAsync(string documentId, CancellationToken ct = default) { return await WithConnectionAsync(async (conn, ct) => @@ -995,7 +1094,7 @@ INSERT INTO memory_records( VALUES($id, $anchorId, $memoryClass, $recordType, $payload, $supersedes, '{MemoryUpdateSemantics.SupersedeRecord.ToWireValue()}', $boundary, $audience, $sensitivity, $recallMode, $confidence, $freshnessAt, $createdAt); """; - insert.Parameters.AddWithValue("$id", newId); + insert.Parameters.AddWithValue("$id", newId.Value); insert.Parameters.AddWithValue("$anchorId", anchorId); insert.Parameters.AddWithValue("$memoryClass", memoryClass); insert.Parameters.AddWithValue("$recordType", recordType); @@ -1013,7 +1112,7 @@ INSERT INTO memory_records( await DeleteRecordFtsAsync(conn, tx, recordId, ct); if (IsSearchableRecallMode(recallMode)) - await UpsertRecordFtsAsync(conn, tx, newId, recordType, payloadJson, null, null, ct); + await UpsertRecordFtsAsync(conn, tx, newId.Value, recordType, payloadJson, null, null, ct); await tx.CommitAsync(ct); return true; @@ -1239,7 +1338,7 @@ await WithConnectionAsync(async (conn, ct) => if (operation.Kind == MemoryKind.Record.ToWireValue()) { - var recId = string.IsNullOrWhiteSpace(operation.MemoryId) ? MemoryTypedId.NewRecordId() : operation.MemoryId; + var recId = string.IsNullOrWhiteSpace(operation.MemoryId) ? MemoryTypedId.NewRecordId().Value : operation.MemoryId; await using var recordCmd = conn.CreateCommand(); recordCmd.Transaction = tx; recordCmd.CommandText = """ @@ -1299,11 +1398,11 @@ ORDER BY updated_at DESC """; lookupCmd.Parameters.AddWithValue("$anchorId", anchor.AnchorId); documentId = (string?)await lookupCmd.ExecuteScalarAsync(ct) - ?? MemoryTypedId.NewDocumentId(); + ?? MemoryTypedId.NewDocumentId().Value; } else { - documentId = MemoryTypedId.NewDocumentId(); + documentId = MemoryTypedId.NewDocumentId().Value; } await using var documentCmd = conn.CreateCommand(); @@ -1394,7 +1493,7 @@ await WithConnectionAsync(async (conn, ct) => if (operation.Kind == MemoryKind.Record.ToWireValue()) { - var recId = string.IsNullOrWhiteSpace(operation.MemoryId) ? MemoryTypedId.NewRecordId() : operation.MemoryId; + var recId = string.IsNullOrWhiteSpace(operation.MemoryId) ? MemoryTypedId.NewRecordId().Value : operation.MemoryId; await using var recordCmd = conn.CreateCommand(); recordCmd.Transaction = tx; recordCmd.CommandText = """ @@ -1457,11 +1556,11 @@ ORDER BY updated_at DESC """; lookupCmd.Parameters.AddWithValue("$anchorId", anchor.AnchorId); documentId = (string?)await lookupCmd.ExecuteScalarAsync(ct) - ?? MemoryTypedId.NewDocumentId(); + ?? MemoryTypedId.NewDocumentId().Value; } else { - documentId = MemoryTypedId.NewDocumentId(); + documentId = MemoryTypedId.NewDocumentId().Value; } await using var documentCmd = conn.CreateCommand(); @@ -1549,7 +1648,40 @@ private async Task WithConnectionAsync( await work(conn, ct); } - private static MemoryTypedId ParseTypedId(string raw) => MemoryTypedId.Parse(raw); + private static async Task MemoryIdVisibleAsync( + SqliteConnection conn, + MemoryKind kind, + MemoryStorageId storageId, + string boundary, + ISet allowedAudiences, + CancellationToken ct) + { + await using var cmd = conn.CreateCommand(); + cmd.CommandText = kind switch + { + MemoryKind.Document => """ + SELECT boundary, audience + FROM memory_documents + WHERE document_id = $id; + """, + MemoryKind.Record => """ + SELECT boundary, audience + FROM memory_records + WHERE record_id = $id; + """, + _ => throw new ArgumentOutOfRangeException(nameof(kind), kind, null) + }; + cmd.Parameters.AddWithValue("$id", storageId.Value); + + await using var reader = await cmd.ExecuteReaderAsync(ct); + if (!await reader.ReadAsync(ct)) + return false; + + var itemBoundary = reader.IsDBNull(0) ? TrustBoundary.LegacyRestrictedValue : reader.GetString(0); + var itemAudience = reader.IsDBNull(1) ? TrustAudience.Personal.ToWireValue() : reader.GetString(1); + return string.Equals(itemBoundary, boundary, StringComparison.OrdinalIgnoreCase) + && allowedAudiences.Contains(itemAudience); + } private static async Task EnsureAnchorAsync( SqliteConnection conn, @@ -1774,6 +1906,25 @@ public sealed record SQLiteMemoryHydratedItem( string Boundary = TrustBoundary.LegacyRestrictedValue, string Audience = "public"); +public sealed record ResolvedMemoryHandle( + string RawId, + MemoryKind Kind, + MemoryStorageId? StorageId, + string? Error) +{ + public bool Resolved => StorageId is not null && Error is null; + + public string WireValue => StorageId is null + ? RawId + : MemoryTypedId.ToWireValue(Kind, StorageId.Value); + + public static ResolvedMemoryHandle Found(string rawId, MemoryKind kind, MemoryStorageId storageId) + => new(rawId, kind, storageId, null); + + public static ResolvedMemoryHandle Failed(string rawId, MemoryKind kind, string error) + => new(rawId, kind, null, error); +} + public sealed record SQLiteMemoryCurationOperation( string Kind, string MemoryClass, diff --git a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs index d3e69dab3..5bf21b2e9 100644 --- a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs @@ -45,7 +45,16 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon var sessionId = string.IsNullOrWhiteSpace(context.SessionId) ? "manual/tool" : context.SessionId!; var audience = MemoryPolicyScopeResolver.ResolveAudience(context.Audience, sessionId); var boundary = MemoryPolicyScopeResolver.ResolveBoundary(context.Boundary?.Value); - var entries = await _store.GetMemoriesByIdsAsync(ids, boundary, audience, ct); + var resolved = await _store.ResolveMemoryHandlesAsync(ids, boundary, audience, ct); + var unresolved = resolved.Where(x => !x.Resolved).ToArray(); + if (unresolved.Length == resolved.Count) + return string.Join(Environment.NewLine, unresolved.Select(x => $"Error: {x.Error}")); + + var entries = await _store.GetMemoriesByIdsAsync( + resolved.Where(x => x.Resolved).Select(x => x.WireValue).ToArray(), + boundary, + audience, + ct); if (entries.Count == 0) return $"No memories found for IDs: {string.Join(", ", ids)}"; @@ -64,6 +73,9 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon sb.AppendLine(); } + foreach (var unresolvedId in unresolved) + sb.AppendLine($"Error: {unresolvedId.Error}"); + _logger.LogInformation("SQLite memory get completed: requested={Requested}, found={Found}", ids.Length, entries.Count); return sb.ToString().TrimEnd(); } diff --git a/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs b/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs index b7db2ee93..7a36b6151 100644 --- a/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs @@ -12,32 +12,31 @@ namespace Netclaw.Actors.Memory; [NetclawTool("update_memory", - "Edit or delete a memory by ID. For edits, provide old_text and new_text for find-and-replace. " - + "For deletion, set delete to \"true\". Use find_memories to discover IDs first.", + "Edit or delete a memory by ID. For document edits, provide either old_text and new_text for find-and-replace, " + + "or new_content to replace the document content. For deletion, set delete to true. Use IDs from memory recall, find_memories, or get_memories directly.", Grant = "builtin")] public sealed partial class SqliteUpdateMemoryTool : NetclawTool { private readonly SQLiteMemoryStore _store; - private readonly IMemoryCheckpointSink _checkpointSink; private readonly ILogger _logger; public record Params( - [property: Description("Memory ID to update or delete (prefix with doc: or rec:)")] + [property: Description("Memory ID to update or delete. Use the doc:/rec: ID shown by memory recall, find_memories, or get_memories.")] string Id, [property: Description("Text to find in the memory content (required for document edits)")] string? OldText = null, [property: Description("Replacement text for document edits or new payload for record supersede")] string? NewText = null, - [property: Description("Set to \"true\" to tombstone the memory")] - string? Delete = null); + [property: Description("Full replacement content for document edits. Do not combine with old_text/new_text.")] + string? NewContent = null, + [property: Description("Set to true to tombstone the memory")] + bool? Delete = null); public SqliteUpdateMemoryTool( SQLiteMemoryStore store, - IMemoryCheckpointSink checkpointSink, ILogger? logger = null) { _store = store; - _checkpointSink = checkpointSink; _logger = logger ?? (ILogger)NullLogger.Instance; } @@ -46,86 +45,65 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon if (string.IsNullOrWhiteSpace(args.Id)) return "Error: memory ID is required."; - var delete = string.Equals(args.Delete, "true", StringComparison.OrdinalIgnoreCase); - var typedId = MemoryTypedId.Parse(args.Id); - if (typedId.Kind == MemoryKind.Unknown) - return "Error: ID must be prefixed with doc: or rec:."; + var sessionId = string.IsNullOrWhiteSpace(context.SessionId) ? "manual/tool" : context.SessionId!; + var audience = MemoryPolicyScopeResolver.ResolveAudience(context.Audience, sessionId); + var boundary = MemoryPolicyScopeResolver.ResolveBoundary(context.Boundary?.Value); + var resolved = await _store.ResolveMemoryHandleAsync(args.Id, boundary, audience, ct); + if (!resolved.Resolved) + return $"Error: {resolved.Error}"; + var storageId = resolved.StorageId!.Value; + var delete = args.Delete ?? false; if (delete) { - var tombstoned = typedId.Kind == MemoryKind.Document - ? await _store.TombstoneDocumentAsync(typedId.Id, ct) - : await _store.TombstoneRecordAsync(typedId.Id, ct); + var tombstoned = resolved.Kind == MemoryKind.Document + ? await _store.TombstoneDocumentAsync(storageId.Value, ct) + : await _store.TombstoneRecordAsync(storageId.Value, ct); if (!tombstoned) - return $"Memory \"{args.Id}\" not found."; + return $"Memory \"{resolved.WireValue}\" not found."; - await EnqueueAuditCheckpoint(args, context, typedId.Kind, MemoryUpdateSemantics.Tombstone, ct); - return $"Memory \"{args.Id}\" tombstoned."; + _logger.LogInformation("SQLite update_memory tombstoned memory={MemoryId}", resolved.WireValue); + return $"Memory \"{resolved.WireValue}\" tombstoned."; } - if (typedId.Kind == MemoryKind.Document) + if (resolved.Kind == MemoryKind.Document) { + if (args.NewContent is not null) + { + if (!string.IsNullOrEmpty(args.OldText) || args.NewText is not null) + return "Error: provide either new_content OR old_text/new_text, not both."; + + var replaced = await _store.ReplaceDocumentTextAsync(storageId.Value, args.NewContent, ct); + if (!replaced) + return $"Edit failed for \"{resolved.WireValue}\". Document missing."; + + _logger.LogInformation("SQLite update_memory replaced document memory={MemoryId}", resolved.WireValue); + return $"Memory \"{resolved.WireValue}\" updated."; + } + if (string.IsNullOrEmpty(args.OldText) || args.NewText is null) - return "Error: document update requires old_text and new_text."; + return "Error: document update requires old_text and new_text, or new_content."; - var updated = await _store.UpdateDocumentTextAsync(typedId.Id, args.OldText, args.NewText, ct); + var updated = await _store.UpdateDocumentTextAsync(storageId.Value, args.OldText, args.NewText, ct); if (!updated) - return $"Edit failed for \"{args.Id}\". Document missing or old_text not found."; + return $"Edit failed for \"{resolved.WireValue}\". Document missing or old_text not found."; - await EnqueueAuditCheckpoint(args, context, typedId.Kind, MemoryUpdateSemantics.MergeDocument, ct); - return $"Memory \"{args.Id}\" updated."; + _logger.LogInformation("SQLite update_memory edited document memory={MemoryId}", resolved.WireValue); + return $"Memory \"{resolved.WireValue}\" updated."; } - if (args.NewText is null) - return "Error: record update requires new_text as replacement payload."; + var recordPayload = args.NewContent ?? args.NewText; + if (args.OldText is not null || recordPayload is null) + return "Error: record update requires new_text or new_content as replacement payload."; - var superseded = await _store.SupersedeRecordAsync(typedId.Id, args.NewText, ct); + var superseded = await _store.SupersedeRecordAsync(storageId.Value, recordPayload, ct); if (!superseded) - return $"Record \"{args.Id}\" not found."; + return $"Record \"{resolved.WireValue}\" not found."; - await EnqueueAuditCheckpoint(args, context, typedId.Kind, MemoryUpdateSemantics.SupersedeRecord, ct); - return $"Record \"{args.Id}\" superseded."; + _logger.LogInformation("SQLite update_memory superseded record memory={MemoryId}", resolved.WireValue); + return $"Record \"{resolved.WireValue}\" superseded."; } protected override Task ExecuteAsync(Params args, CancellationToken ct) => ExecuteAsync(args, ToolExecutionContext.Empty, ct); - - private async Task EnqueueAuditCheckpoint(Params args, ToolExecutionContext context, MemoryKind kind, MemoryUpdateSemantics semantics, CancellationToken ct) - { - var sessionId = string.IsNullOrWhiteSpace(context.SessionId) ? "manual/tool" : context.SessionId!; - var audience = MemoryPolicyScopeResolver.ResolveAudience(context.Audience, sessionId); - var boundary = MemoryPolicyScopeResolver.ResolveBoundary(context.Boundary?.Value); - var payload = new MemoryCheckpointPayload( - SessionId: sessionId, - TriggerType: CheckpointTriggerType.ExplicitMemoryRequest.ToWireValue(), - Source: "update_memory", - Content: args.NewText ?? args.OldText ?? args.Id, - UserContent: args.NewText ?? args.OldText ?? args.Id, - AssistantContent: null, - IsExplicitRequest: true, - HasVerifiedToolFinding: false, - IsCompactionBoundary: false, - HasAcceptedSubAgentFinding: false, - Boundary: boundary, - Audience: audience.ToWireValue(), - Sensitivity: MemorySensitivity.Normal.ToWireValue(), - RecallMode: MemoryRecallMode.Manual.ToWireValue(), - Confidence: 0.95, - MemoryId: MemoryTypedId.Parse(args.Id).Id, - UpdateOldText: args.OldText, - UpdateNewText: args.NewText, - Delete: string.Equals(args.Delete, "true", StringComparison.OrdinalIgnoreCase), - Kind: kind.ToWireValue(), - UpdateSemantics: semantics.ToWireValue(), - Title: args.Id); - - var result = await _checkpointSink.EnqueueAsync(new MemoryCheckpointRequest( - SessionId: new Protocol.SessionId(sessionId), - TurnId: null, - TriggerType: CheckpointTriggerType.ExplicitMemoryRequest, - Priority: 95, - Payload: payload), ct); - - _logger.LogInformation("SQLite update_memory audit checkpoint={CheckpointId} memory={MemoryId}", result.CheckpointId, args.Id); - } } diff --git a/src/Netclaw.Actors/Sessions/IMemoryRecallCoordinator.cs b/src/Netclaw.Actors/Sessions/IMemoryRecallCoordinator.cs index 8673dae20..74957b881 100644 --- a/src/Netclaw.Actors/Sessions/IMemoryRecallCoordinator.cs +++ b/src/Netclaw.Actors/Sessions/IMemoryRecallCoordinator.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using Netclaw.Actors.Protocol; +using Netclaw.Actors.Memory; using Netclaw.Configuration; namespace Netclaw.Actors.Sessions; @@ -43,11 +44,17 @@ public sealed record AutomaticRecallResult( /// A single memory item selected for automatic recall. /// public sealed record AutomaticRecallItem( - string Id, + MemoryStorageId Id, string Title, string Content, string Sensitivity, - double Score); + double Score) +{ + public AutomaticRecallItem(string id, string title, string content, string sensitivity, double score) + : this(new MemoryStorageId(id), title, content, sensitivity, score) + { + } +} /// /// No-op automatic recall coordinator used when recall is not configured. diff --git a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs index 0c9a76a57..73582e2f8 100644 --- a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs +++ b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs @@ -2662,7 +2662,7 @@ private void FireLlmCall(string? recallQuery = null, bool forceNoTools = false) var recallIds = resolved.Items.Count == 0 ? "-" - : string.Join(",", resolved.Items.Select(i => i.Id)); + : string.Join(",", resolved.Items.Select(i => i.Id.Value)); TurnLog().Info( "turn_memory_recall degraded={Degraded} stage={Stage} durationMs={DurationMs} itemCount={ItemCount} itemIds={ItemIds}", resolved.Degraded, diff --git a/src/Netclaw.Actors/Sessions/Pipelines/SessionRecallManager.cs b/src/Netclaw.Actors/Sessions/Pipelines/SessionRecallManager.cs index b0a171d4b..8f5e4ec6f 100644 --- a/src/Netclaw.Actors/Sessions/Pipelines/SessionRecallManager.cs +++ b/src/Netclaw.Actors/Sessions/Pipelines/SessionRecallManager.cs @@ -102,7 +102,7 @@ public AutomaticRecallResult ApplyProgressiveRecall(AutomaticRecallResult resolv if (_injectedMemoryIds.Count > 0 && resolved.Items.Count > 0) { var filtered = resolved.Items - .Where(i => !_injectedMemoryIds.Contains(i.Id)) + .Where(i => !_injectedMemoryIds.Contains(i.Id.Value)) .ToArray(); if (filtered.Length == 0 && resolved.Items.Count > 0) @@ -120,7 +120,7 @@ public AutomaticRecallResult ApplyProgressiveRecall(AutomaticRecallResult resolv // Track injected IDs for progressive recall across turns foreach (var item in resolved.Items) - _injectedMemoryIds.Add(item.Id); + _injectedMemoryIds.Add(item.Id.Value); return resolved; } diff --git a/src/Netclaw.Actors/Sessions/SQLiteMemoryRecallCoordinator.cs b/src/Netclaw.Actors/Sessions/SQLiteMemoryRecallCoordinator.cs index 23caa12d2..ade12dad2 100644 --- a/src/Netclaw.Actors/Sessions/SQLiteMemoryRecallCoordinator.cs +++ b/src/Netclaw.Actors/Sessions/SQLiteMemoryRecallCoordinator.cs @@ -108,12 +108,12 @@ public async Task RecallAsync(AutomaticRecallRequest requ deterministicItems.Length, rankedCandidates.Length - aboveFloor.Length, minimumCompositeScore, - string.Join("|", deterministicItems.Select(i => $"{i.Id}=score{i.Score:F1}"))); + string.Join("|", deterministicItems.Select(i => $"{i.Id.Value}=score{i.Score:F1}"))); logger.LogDebug( "memory_retrieval_final_detail session={SessionId} items={Items}", request.SessionId, - string.Join("|", deterministicItems.Select(i => $"{i.Id}={i.Title}"))); + string.Join("|", deterministicItems.Select(i => $"{i.Id.Value}={i.Title}"))); return new AutomaticRecallResult(deterministicItems); } diff --git a/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs b/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs index 540517abe..3193a355b 100644 --- a/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs +++ b/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using Microsoft.Extensions.AI; +using Netclaw.Actors.Memory; using Netclaw.Actors.Protocol; using Netclaw.Configuration; using AiChatMessage = Microsoft.Extensions.AI.ChatMessage; @@ -277,7 +278,8 @@ private static string FormatRecallForLlm(AutomaticRecallResult recall) sb.AppendLine("mode: automatic"); foreach (var item in recall.Items) { - sb.AppendLine($"- {item.Title} [{item.Id}] sensitivity={item.Sensitivity} score={item.Score:F2}"); + var typedId = MemoryTypedId.Parse(item.Id.Value).ToWireValue(); + sb.AppendLine($"- {item.Title} [{typedId}] sensitivity={item.Sensitivity} score={item.Score:F2}"); sb.AppendLine($" {item.Content}"); } return sb.ToString().TrimEnd(); diff --git a/src/Netclaw.Daemon/Program.cs b/src/Netclaw.Daemon/Program.cs index 1d0219560..7acb5c8bc 100644 --- a/src/Netclaw.Daemon/Program.cs +++ b/src/Netclaw.Daemon/Program.cs @@ -717,9 +717,7 @@ static void ConfigureDaemonServices( toolRegistry.Register(new SqliteFindMemoriesTool(memoryStore)); toolRegistry.Register(new SqliteGetMemoriesTool(memoryStore)); toolRegistry.Register(new SqliteStoreMemoryTool(new SQLiteMemoryCheckpointSink(memoryStore, TimeProvider.System))); - toolRegistry.Register(new SqliteUpdateMemoryTool( - memoryStore, - new SQLiteMemoryCheckpointSink(memoryStore, TimeProvider.System))); + toolRegistry.Register(new SqliteUpdateMemoryTool(memoryStore)); } services.AddSingleton(NullMemoryExtractor.Instance); From 2706228d65d1f48f78b001867cf5f8a0a9cac4f5 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 02:55:59 +0000 Subject: [PATCH 3/7] fix(memory): address review nits on stable-handle change - get_memories: hydrate via GetMemoriesByResolvedHandlesAsync so the tool no longer resolves every ID twice (once for per-ID errors, once inside GetMemoriesByIdsAsync). Adds an early-out when nothing resolved. - update_memory: reject empty/whitespace new_content instead of silently wiping a document body; point the model at delete:true for removal. - update_memory: give records a specific error when old_text is supplied (they don't support find-and-replace) instead of the generic payload error. - restore the UTF-8 BOM on MemoryTypedId.cs (matches Add-FileHeaders canonical encoding) and add it to the new MemoryTypedIdTests.cs. - add regression tests for the empty-new_content and record old_text guards. --- .../Memory/MemoryTypedIdTests.cs | 2 +- .../Memory/SqliteMemoryToolsTests.cs | 77 +++++++++++++++++++ src/Netclaw.Actors/Memory/MemoryTypedId.cs | 2 +- .../Memory/SQLiteMemoryStore.cs | 17 ++++ .../Memory/SqliteGetMemoriesTool.cs | 6 +- .../Memory/SqliteUpdateMemoryTool.cs | 7 +- 6 files changed, 103 insertions(+), 8 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs index 493adc726..0dc04e3d0 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // diff --git a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs index 540054bd1..55d713456 100644 --- a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs @@ -389,6 +389,54 @@ public async Task UpdateMemory_tombstones_document_from_typed_recall_id() Assert.Equal("No memories found.", search); } + [Fact] + public async Task UpdateMemory_rejects_empty_new_content_without_wiping_document() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + await StoreDocumentAsync("doc-guard", "Working preference", "Use short replies.", now); + + var update = new SqliteUpdateMemoryTool(_store); + var result = await update.ExecuteAsync( + new Dictionary + { + ["id"] = "doc:doc-guard", + ["new_content"] = " " + }, + PersonalContext(), + CancellationToken.None); + + var get = new SqliteGetMemoriesTool(_store, _timeProvider); + var hydrated = await get.ExecuteAsync( + new Dictionary { ["ids"] = "doc:doc-guard" }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("new_content cannot be empty", result); + Assert.Contains("Use short replies.", hydrated); + } + + [Fact] + public async Task UpdateMemory_rejects_old_text_for_record_with_specific_error() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + await StoreRecordAsync("rec-guard", "Hotel note", "Hilton Easton was recommended.", now); + + var update = new SqliteUpdateMemoryTool(_store); + var result = await update.ExecuteAsync( + new Dictionary + { + ["id"] = "rec:rec-guard", + ["old_text"] = "Hilton", + ["new_text"] = "Marriott" + }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("records do not support old_text/new_text", result); + } + public async ValueTask DisposeAsync() { await SqliteTempDirectoryCleanup.TryDeleteDirectoryAsync(_baseDir); @@ -423,6 +471,35 @@ await _store.ApplyCurationBatchAsync( CancellationToken.None); } + private async Task StoreRecordAsync(string id, string title, string content, long now) + { + await _store.ApplyCurationBatchAsync( + $"cp-{id}", + [ + new SQLiteMemoryCurationOperation( + Kind: MemoryKind.Record.ToWireValue(), + MemoryClass: MemoryClass.Evidence.ToWireValue(), + MemoryId: id, + AnchorCanonicalName: title, + AnchorType: "concept", + Title: title, + Content: content, + AliasesJson: null, + FacetsJson: null, + SlotsJson: null, + Relations: null, + UpdateSemantics: MemoryUpdateSemantics.SupersedeRecord.ToWireValue(), + Boundary: TrustBoundary.TrustedInstanceValue, + Audience: TrustAudience.Team, + Sensitivity: MemorySensitivity.Normal.ToWireValue(), + RecallMode: MemoryRecallMode.Searchable.ToWireValue(), + Confidence: 0.9, + FreshnessAtMs: now, + ExpiresAtMs: null) + ], + CancellationToken.None); + } + private static ToolExecutionContext PersonalContext() => new("slack/thread-1", sessionDirectory: null) { Audience = TrustAudience.Personal }; diff --git a/src/Netclaw.Actors/Memory/MemoryTypedId.cs b/src/Netclaw.Actors/Memory/MemoryTypedId.cs index 0e22db371..72f34fd02 100644 --- a/src/Netclaw.Actors/Memory/MemoryTypedId.cs +++ b/src/Netclaw.Actors/Memory/MemoryTypedId.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index 40097f2a1..d36d31d29 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -621,6 +621,20 @@ public async Task> GetMemoriesByIdsAsync return []; var resolvedIds = await ResolveMemoryHandlesAsync(ids, boundary, audience, ct); + return await GetMemoriesByResolvedHandlesAsync(resolvedIds, boundary, audience, ct); + } + + /// + /// Hydrates memories from handles that have already been resolved. Callers that run + /// up front (e.g. to surface per-ID errors) pass + /// the result here so the same IDs are not resolved a second time. + /// + public async Task> GetMemoriesByResolvedHandlesAsync( + IReadOnlyList resolvedIds, + string boundary, + TrustAudience audience, + CancellationToken ct = default) + { var documents = resolvedIds .Where(x => x.Resolved && x.Kind == MemoryKind.Document) .Select(x => x.StorageId!.Value.Value) @@ -633,6 +647,9 @@ public async Task> GetMemoriesByIdsAsync .Distinct(StringComparer.OrdinalIgnoreCase) .ToArray(); + if (documents.Length == 0 && records.Length == 0) + return []; + var output = new List(); var allowedAudiences = MemoryPolicyEvaluator.AllowedAudienceWireValues(audience) .ToHashSet(StringComparer.OrdinalIgnoreCase); diff --git a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs index 5bf21b2e9..fa006789c 100644 --- a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs @@ -50,11 +50,7 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon if (unresolved.Length == resolved.Count) return string.Join(Environment.NewLine, unresolved.Select(x => $"Error: {x.Error}")); - var entries = await _store.GetMemoriesByIdsAsync( - resolved.Where(x => x.Resolved).Select(x => x.WireValue).ToArray(), - boundary, - audience, - ct); + var entries = await _store.GetMemoriesByResolvedHandlesAsync(resolved, boundary, audience, ct); if (entries.Count == 0) return $"No memories found for IDs: {string.Join(", ", ids)}"; diff --git a/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs b/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs index 7a36b6151..832be83e4 100644 --- a/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs @@ -72,6 +72,8 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon { if (!string.IsNullOrEmpty(args.OldText) || args.NewText is not null) return "Error: provide either new_content OR old_text/new_text, not both."; + if (string.IsNullOrWhiteSpace(args.NewContent)) + return "Error: new_content cannot be empty. To remove a memory, set delete to true."; var replaced = await _store.ReplaceDocumentTextAsync(storageId.Value, args.NewContent, ct); if (!replaced) @@ -92,8 +94,11 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon return $"Memory \"{resolved.WireValue}\" updated."; } + if (args.OldText is not null) + return "Error: records do not support old_text/new_text find-and-replace; provide new_text or new_content as the full replacement payload."; + var recordPayload = args.NewContent ?? args.NewText; - if (args.OldText is not null || recordPayload is null) + if (string.IsNullOrWhiteSpace(recordPayload)) return "Error: record update requires new_text or new_content as replacement payload."; var superseded = await _store.SupersedeRecordAsync(storageId.Value, recordPayload, ct); From c9a8cf42f9fb93883c6fe382295acdf2485e42dd Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 03:57:47 +0000 Subject: [PATCH 4/7] refactor(memory): collapse memory ids to one canonical model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The storage layer already mints self-describing, unique-per-table primary keys (doc-{guid} / rec-{guid}). The separate "doc:" / "rec:" wire handle was a second representation that just wrapped the storage id (surfacing doc:doc-{guid} to the model), and the strip/re-add reconciliation it required is what forced the candidate-id expansion and the fail-loud ambiguity branch. Collapse to one model: the storage id IS the handle. - MemoryTypedId: drop ToWireValue (all 3 overloads) and CandidateStorageIds; ToString now returns the storage id. Parse still accepts a legacy doc:/rec: envelope but keeps the remainder as the exact key (never rewrites it). - ResolveMemoryHandleAsync: one visibility-scoped lookup on the exact key — no candidate set, no ambiguity case (a primary key is unique). - find/get/recall surface the storage id verbatim (no more doc:doc-{guid}). - ResolvedMemoryHandle.WireValue -> Handle (the storage id verbatim). - update_memory Id description + netclaw-memory skill: copy the id verbatim. Tests updated to the canonical model: id forms map to their exact keys (the old ambiguity test is now a deterministic-resolution test), and recall/get emit the storage id verbatim. Net -73 lines. --- .../.system/files/netclaw-memory/SKILL.md | 6 +- .../Memory/MemoryTypedIdTests.cs | 60 +++++-------------- .../Memory/SQLiteMemoryStoreTests.cs | 27 +++++---- .../Memory/SqliteMemoryToolsTests.cs | 8 ++- .../Sessions/SessionMessageAssemblerTests.cs | 7 ++- src/Netclaw.Actors/Memory/MemoryTypedId.cs | 49 +++------------ .../Memory/SQLiteMemoryStore.cs | 40 ++++--------- .../Memory/SqliteFindMemoriesTool.cs | 7 +-- .../Memory/SqliteGetMemoriesTool.cs | 5 +- .../Memory/SqliteUpdateMemoryTool.cs | 26 ++++---- .../Sessions/SessionMessageAssembler.cs | 4 +- 11 files changed, 83 insertions(+), 156 deletions(-) diff --git a/feeds/skills/.system/files/netclaw-memory/SKILL.md b/feeds/skills/.system/files/netclaw-memory/SKILL.md index 5956261ac..afda513cb 100644 --- a/feeds/skills/.system/files/netclaw-memory/SKILL.md +++ b/feeds/skills/.system/files/netclaw-memory/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-memory description: "REQUIRED when the user asks what you remember, recall, or know from past conversations, previous sessions, cross-session memory, memory classes, or memory types. Also before using memory tools: find_memories, get_memories, store_memory, update_memory." metadata: author: netclaw - version: "1.6.1" + version: "1.6.2" --- # Netclaw Memory @@ -41,8 +41,8 @@ Both gates must pass for memory to function. - Memory is SQLite-backed and cross-session only within the active domain/boundary policy envelope. - Memory IDs shown by automatic recall, `find_memories`, and `get_memories` - are stable handles. Reuse them directly with `get_memories` or - `update_memory`; do not rewrite `doc:` / `rec:` prefixes by hand. + (e.g. `doc-…` / `rec-…`) are stable, opaque handles. Copy them **verbatim** + into `get_memories` or `update_memory` — do not rewrite or reformat them. ## When to Use Explicit Tools diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs index 0dc04e3d0..e61c1066d 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryTypedIdTests.cs @@ -44,22 +44,10 @@ public void Parse_unknown_for_invalid_prefixes(string raw) } [Fact] - public void ToWireValue_returns_colon_format() + public void ToString_returns_storage_id_verbatim() { - var doc = new MemoryTypedId(MemoryKind.Document, "abc123"); - var rec = new MemoryTypedId(MemoryKind.Record, "xyz789"); - var unknown = new MemoryTypedId(MemoryKind.Unknown, "orphan"); - - Assert.Equal("doc:abc123", doc.ToWireValue()); - Assert.Equal("rec:xyz789", rec.ToWireValue()); - Assert.Equal("orphan", unknown.ToWireValue()); - } - - [Fact] - public void ToString_matches_ToWireValue() - { - var id = new MemoryTypedId(MemoryKind.Document, "abc123"); - Assert.Equal("doc:abc123", id.ToString()); + var id = new MemoryTypedId(MemoryKind.Document, "doc-abc123"); + Assert.Equal("doc-abc123", id.ToString()); } [Fact] @@ -79,43 +67,27 @@ public void NewRecordId_returns_dash_format() } [Fact] - public void Round_trip_dash_to_parse_to_wire() + public void Generated_storage_id_round_trips_to_the_same_key() { + // The id we surface to the model is the storage id verbatim; parsing what the model + // sends back must yield the exact same primary key. var generated = MemoryTypedId.NewDocumentId(); var parsed = MemoryTypedId.Parse(generated.Value); - var wire = parsed.ToWireValue(); Assert.Equal(MemoryKind.Document, parsed.Kind); - Assert.Equal($"doc:{generated.Value}", wire); + Assert.Equal(generated.Value, parsed.Id.Value); } [Fact] - public void Round_trip_wire_to_parse_to_string() + public void Legacy_colon_envelope_resolves_to_the_same_key_as_the_dash_id() { - // Simulates find_memories output: agent receives "doc:{guid}" - var wire = "doc:abc123"; - var parsed = MemoryTypedId.Parse(wire); - var output = parsed.ToString(); - - Assert.Equal(MemoryKind.Document, parsed.Kind); - Assert.Equal("doc:abc123", output); - } - - [Fact] - public void CandidateStorageIds_include_legacy_prefixed_candidate_for_bare_handle_payload() - { - var parsed = MemoryTypedId.Parse("doc:abc123"); - var candidates = parsed.CandidateStorageIds().Select(x => x.Value).ToArray(); - - Assert.Equal(["abc123", "doc-abc123"], candidates); - } - - [Fact] - public void CandidateStorageIds_preserve_legacy_raw_storage_id() - { - var parsed = MemoryTypedId.Parse("doc-abc123"); - var candidates = parsed.CandidateStorageIds().Select(x => x.Value).ToArray(); - - Assert.Equal(["doc-abc123"], candidates); + // Both the bare storage id and a legacy "doc:{storageId}" envelope must map to the + // one real key — this is what makes the single-lookup resolver unambiguous. + var dash = MemoryTypedId.Parse("doc-abc123"); + var enveloped = MemoryTypedId.Parse("doc:doc-abc123"); + + Assert.Equal("doc-abc123", dash.Id.Value); + Assert.Equal("doc-abc123", enveloped.Id.Value); + Assert.Equal(dash.Id.Value, enveloped.Id.Value); } } diff --git a/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs b/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs index ee6b76f4b..84537ca80 100644 --- a/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/SQLiteMemoryStoreTests.cs @@ -243,25 +243,28 @@ await _store.UpsertDocumentAsync(new SQLiteMemoryDocument( } [Fact] - public async Task ResolveMemoryHandleAsync_fails_loudly_when_canonical_handle_is_ambiguous() + public async Task ResolveMemoryHandleAsync_maps_each_id_form_to_its_exact_storage_key() { await _store.InitializeAsync(TestContext.Current.CancellationToken); - var anchor = _store.CreateDefaultAnchor("ambiguous-memory"); + var anchor = _store.CreateDefaultAnchor("distinct-memory"); var now = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); + // Two distinct rows whose keys differ only by the legacy prefix. Because the parsed id is + // used as the exact primary key, each id form resolves to exactly one row — no ambiguity. await _store.UpsertDocumentAsync(CreateDocument("abc", anchor, "Bare ID", now), TestContext.Current.CancellationToken); await _store.UpsertDocumentAsync(CreateDocument("doc-abc", anchor, "Legacy ID", now), TestContext.Current.CancellationToken); - var resolved = await _store.ResolveMemoryHandleAsync( - "doc:abc", - TrustBoundary.TrustedInstanceValue, - TrustAudience.Personal, - TestContext.Current.CancellationToken); - - Assert.False(resolved.Resolved); - Assert.Contains("ambiguous", resolved.Error, StringComparison.OrdinalIgnoreCase); - Assert.Contains("doc:abc", resolved.Error); - Assert.Contains("doc:doc-abc", resolved.Error); + var bare = await _store.ResolveMemoryHandleAsync("doc:abc", TrustBoundary.TrustedInstanceValue, TrustAudience.Personal, TestContext.Current.CancellationToken); + var dash = await _store.ResolveMemoryHandleAsync("doc-abc", TrustBoundary.TrustedInstanceValue, TrustAudience.Personal, TestContext.Current.CancellationToken); + var envelope = await _store.ResolveMemoryHandleAsync("doc:doc-abc", TrustBoundary.TrustedInstanceValue, TrustAudience.Personal, TestContext.Current.CancellationToken); + + Assert.True(bare.Resolved); + Assert.Equal("abc", bare.StorageId!.Value.Value); + Assert.True(dash.Resolved); + Assert.Equal("doc-abc", dash.StorageId!.Value.Value); + // The colon envelope over the dash key resolves to the same row as the dash key. + Assert.True(envelope.Resolved); + Assert.Equal("doc-abc", envelope.StorageId!.Value.Value); } public ValueTask InitializeAsync() => ValueTask.CompletedTask; diff --git a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs index 55d713456..0626b42c3 100644 --- a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs @@ -298,10 +298,14 @@ await StoreDocumentAsync( PersonalContext(), CancellationToken.None); + // Both the raw storage id and a legacy doc: envelope are accepted as input, and the + // output surfaces the storage id verbatim (no doc: envelope). Assert.Contains("Auto recalled note", rawResult); - Assert.Contains("doc:doc-auto", rawResult); + Assert.Contains("[doc-auto]", rawResult); + Assert.DoesNotContain("doc:doc-auto", rawResult); Assert.Contains("Auto recalled note", typedResult); - Assert.Contains("doc:doc-auto", typedResult); + Assert.Contains("[doc-auto]", typedResult); + Assert.DoesNotContain("doc:doc-auto", typedResult); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs b/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs index 032ee6636..d956f421a 100644 --- a/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs @@ -154,7 +154,7 @@ public void BuildVolatileContextBlock_composes_recall_and_slash_command() } [Fact] - public void BuildVolatileContextBlock_formats_recall_ids_as_typed_memory_handles() + public void BuildVolatileContextBlock_emits_storage_id_verbatim() { var input = MakeInput( SeedHistory("hi"), @@ -170,8 +170,9 @@ public void BuildVolatileContextBlock_formats_recall_ids_as_typed_memory_handles var block = SessionMessageAssembler.BuildVolatileContextBlock(input); - Assert.Contains("[doc:doc-auto]", block); - Assert.DoesNotContain("[doc-auto]", block); + // The recall block surfaces the storage id verbatim so it round-trips back into the tools. + Assert.Contains("[doc-auto]", block); + Assert.DoesNotContain("doc:doc-auto", block); } [Fact] diff --git a/src/Netclaw.Actors/Memory/MemoryTypedId.cs b/src/Netclaw.Actors/Memory/MemoryTypedId.cs index 72f34fd02..d2ac3eec4 100644 --- a/src/Netclaw.Actors/Memory/MemoryTypedId.cs +++ b/src/Netclaw.Actors/Memory/MemoryTypedId.cs @@ -13,8 +13,10 @@ public readonly record struct MemoryStorageId(string Value) } /// -/// Strongly-typed model-facing memory handle with kind prefix (doc: or rec:). -/// Storage IDs are opaque and may include legacy doc-/rec- prefixes. +/// A memory identity: its plus its opaque storage id +/// (the primary key, e.g. "doc-{guid}" / "rec-{guid}"). The storage id IS the +/// model-facing handle — it is surfaced verbatim and passed back verbatim, so a +/// value always round-trips to the exact row it came from. /// public readonly record struct MemoryTypedId(MemoryKind Kind, MemoryStorageId Id) { @@ -24,28 +26,10 @@ public MemoryTypedId(MemoryKind kind, string id) } /// - /// Formats as the prefixed wire representation: "doc:{id}" or "rec:{id}". - /// - public string ToWireValue() => Kind switch - { - MemoryKind.Document => $"doc:{Id.Value}", - MemoryKind.Record => $"rec:{Id.Value}", - _ => Id.Value - }; - - /// - /// Formats a storage ID for model-visible output. Existing storage IDs are - /// not rewritten; the kind prefix is added as the tool handle envelope. - /// - public static string ToWireValue(MemoryKind kind, string storageId) - => new MemoryTypedId(kind, storageId).ToWireValue(); - - public static string ToWireValue(MemoryKind kind, MemoryStorageId storageId) - => new MemoryTypedId(kind, storageId).ToWireValue(); - - /// - /// Parses a model-visible handle like "doc:abc123" or "rec:def456". - /// Also accepts legacy raw storage IDs such as "doc-abc123" and "rec-def456". + /// Resolves an id supplied by the model to its kind plus exact storage key. Storage ids + /// are self-describing via their "doc-"/"rec-" prefix; a legacy "doc:"/"rec:" envelope is + /// also accepted and stripped. The remaining string is used as the storage key verbatim — + /// it is never rewritten — so there is exactly one key per input and no ambiguity. /// Returns with the raw value when the prefix is unrecognized. /// public static MemoryTypedId Parse(string raw) @@ -62,22 +46,7 @@ public static MemoryTypedId Parse(string raw) return new MemoryTypedId(MemoryKind.Unknown, raw); } - public IReadOnlyList CandidateStorageIds() => Kind switch - { - MemoryKind.Document => CandidateStorageIdsFor("doc-"), - MemoryKind.Record => CandidateStorageIdsFor("rec-"), - _ => [Id] - }; - - private IReadOnlyList CandidateStorageIdsFor(string legacyPrefix) - { - if (Id.Value.StartsWith(legacyPrefix, StringComparison.OrdinalIgnoreCase)) - return [Id]; - - return [Id, new MemoryStorageId(legacyPrefix + Id.Value)]; - } - - public override string ToString() => ToWireValue(); + public override string ToString() => Id.Value; public static string AnchorId(string canonicalName) => $"anchor:{canonicalName.Trim().ToLowerInvariant().Replace(' ', '-')}"; diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index d36d31d29..916922023 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -754,14 +754,8 @@ public async Task ResolveMemoryHandleAsync( { var parsed = MemoryTypedId.Parse(rawId); if (parsed.Kind is not (MemoryKind.Document or MemoryKind.Record)) - return ResolvedMemoryHandle.Failed(rawId, parsed.Kind, "ID must be prefixed with doc: or rec:."); - - var candidates = parsed.CandidateStorageIds() - .Where(x => !x.IsEmpty) - .GroupBy(x => x.Value, StringComparer.OrdinalIgnoreCase) - .Select(x => x.First()) - .ToArray(); - if (candidates.Length == 0) + return ResolvedMemoryHandle.Failed(rawId, parsed.Kind, "ID must be prefixed with doc- or rec-."); + if (parsed.Id.IsEmpty) return ResolvedMemoryHandle.Failed(rawId, parsed.Kind, "ID payload is required."); var allowedAudiences = MemoryPolicyEvaluator.AllowedAudienceWireValues(audience) @@ -769,22 +763,12 @@ public async Task ResolveMemoryHandleAsync( return await WithConnectionAsync(async (conn, ct) => { - var matches = new List(); - foreach (var candidate in candidates) - { - if (await MemoryIdVisibleAsync(conn, parsed.Kind, candidate, boundary, allowedAudiences, ct)) - matches.Add(candidate); - } - - return matches.Count switch - { - 1 => ResolvedMemoryHandle.Found(rawId, parsed.Kind, matches[0]), - 0 => ResolvedMemoryHandle.Failed(rawId, parsed.Kind, $"Memory \"{rawId}\" was not found or is not accessible from this session."), - _ => ResolvedMemoryHandle.Failed( - rawId, - parsed.Kind, - $"Memory ID \"{rawId}\" is ambiguous. Use one of: {string.Join(", ", matches.Select(x => MemoryTypedId.ToWireValue(parsed.Kind, x)))}.") - }; + // The parsed id is the exact storage key (primary key, unique per table), so a single + // visibility-scoped lookup either finds it or it does not — no candidate guessing. + var visible = await MemoryIdVisibleAsync(conn, parsed.Kind, parsed.Id, boundary, allowedAudiences, ct); + return visible + ? ResolvedMemoryHandle.Found(rawId, parsed.Kind, parsed.Id) + : ResolvedMemoryHandle.Failed(rawId, parsed.Kind, $"Memory \"{rawId}\" was not found or is not accessible from this session."); }, ct); } @@ -1931,9 +1915,11 @@ public sealed record ResolvedMemoryHandle( { public bool Resolved => StorageId is not null && Error is null; - public string WireValue => StorageId is null - ? RawId - : MemoryTypedId.ToWireValue(Kind, StorageId.Value); + /// + /// The canonical model-facing handle for this memory — its opaque storage id verbatim. + /// Falls back to the raw input when resolution failed. + /// + public string Handle => StorageId?.Value ?? RawId; public static ResolvedMemoryHandle Found(string rawId, MemoryKind kind, MemoryStorageId storageId) => new(rawId, kind, storageId, null); diff --git a/src/Netclaw.Actors/Memory/SqliteFindMemoriesTool.cs b/src/Netclaw.Actors/Memory/SqliteFindMemoriesTool.cs index 1c2ee284a..cd63f1b55 100644 --- a/src/Netclaw.Actors/Memory/SqliteFindMemoriesTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteFindMemoriesTool.cs @@ -77,21 +77,18 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon var sb = new StringBuilder(); foreach (var result in results) { - var typedId = new MemoryTypedId( - MemoryDomainEnumExtensions.TryFromWireValue(result.Kind, out MemoryKind kind) ? kind : MemoryKind.Document, - result.Id); var isStaleEvidence = string.Equals(result.MemoryClass, MemoryClass.Evidence.ToWireValue(), StringComparison.OrdinalIgnoreCase) && result.ExpiresAtMs is long expiresAt && expiresAt <= _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); var snippet = BuildSnippet(result.Content); - sb.AppendLine($"[{typedId.ToWireValue()}] {result.Title}"); + sb.AppendLine($"[{result.Id}] {result.Title}"); sb.AppendLine($" class={result.MemoryClass} sensitivity={result.Sensitivity} recall={result.RecallMode}{(isStaleEvidence ? " stale=true" : string.Empty)}"); sb.AppendLine($" {snippet}"); sb.AppendLine(); } - sb.AppendLine($"Use get_memories(\"{string.Join(", ", results.Select(r => new MemoryTypedId(MemoryDomainEnumExtensions.TryFromWireValue(r.Kind, out MemoryKind k) ? k : MemoryKind.Document, r.Id).ToWireValue()))}\") to load full content."); + sb.AppendLine($"Use get_memories(\"{string.Join(", ", results.Select(r => r.Id))}\") to load full content."); _logger.LogInformation("SQLite memory find completed: query='{Query}', results={Count}, includeStale={IncludeStale}", args.Query, results.Count, includeStale); return sb.ToString().TrimEnd(); } diff --git a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs index fa006789c..0ffcc55a5 100644 --- a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs @@ -57,13 +57,10 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon var sb = new StringBuilder(); foreach (var entry in entries.OrderByDescending(e => e.UpdatedAtMs)) { - var typedId = new MemoryTypedId( - MemoryDomainEnumExtensions.TryFromWireValue(entry.Kind, out MemoryKind kind) ? kind : MemoryKind.Document, - entry.Id); var isStaleEvidence = string.Equals(entry.MemoryClass, MemoryClass.Evidence.ToWireValue(), StringComparison.OrdinalIgnoreCase) && entry.ExpiresAtMs is long expiresAt && expiresAt <= _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); - sb.AppendLine($"━━━ {entry.Title} [{typedId.ToWireValue()}] ━━━"); + sb.AppendLine($"━━━ {entry.Title} [{entry.Id}] ━━━"); sb.AppendLine($"kind={entry.Kind} class={entry.MemoryClass} sensitivity={entry.Sensitivity} recall={entry.RecallMode} semantics={entry.UpdateSemantics}{(isStaleEvidence ? " stale=true" : string.Empty)}"); sb.AppendLine(entry.Content); sb.AppendLine(); diff --git a/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs b/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs index 832be83e4..857151364 100644 --- a/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs @@ -21,7 +21,7 @@ public sealed partial class SqliteUpdateMemoryTool : NetclawTool ExecuteAsync(Params args, ToolExecutionCon ? await _store.TombstoneDocumentAsync(storageId.Value, ct) : await _store.TombstoneRecordAsync(storageId.Value, ct); if (!tombstoned) - return $"Memory \"{resolved.WireValue}\" not found."; + return $"Memory \"{resolved.Handle}\" not found."; - _logger.LogInformation("SQLite update_memory tombstoned memory={MemoryId}", resolved.WireValue); - return $"Memory \"{resolved.WireValue}\" tombstoned."; + _logger.LogInformation("SQLite update_memory tombstoned memory={MemoryId}", resolved.Handle); + return $"Memory \"{resolved.Handle}\" tombstoned."; } if (resolved.Kind == MemoryKind.Document) @@ -77,10 +77,10 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon var replaced = await _store.ReplaceDocumentTextAsync(storageId.Value, args.NewContent, ct); if (!replaced) - return $"Edit failed for \"{resolved.WireValue}\". Document missing."; + return $"Edit failed for \"{resolved.Handle}\". Document missing."; - _logger.LogInformation("SQLite update_memory replaced document memory={MemoryId}", resolved.WireValue); - return $"Memory \"{resolved.WireValue}\" updated."; + _logger.LogInformation("SQLite update_memory replaced document memory={MemoryId}", resolved.Handle); + return $"Memory \"{resolved.Handle}\" updated."; } if (string.IsNullOrEmpty(args.OldText) || args.NewText is null) @@ -88,10 +88,10 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon var updated = await _store.UpdateDocumentTextAsync(storageId.Value, args.OldText, args.NewText, ct); if (!updated) - return $"Edit failed for \"{resolved.WireValue}\". Document missing or old_text not found."; + return $"Edit failed for \"{resolved.Handle}\". Document missing or old_text not found."; - _logger.LogInformation("SQLite update_memory edited document memory={MemoryId}", resolved.WireValue); - return $"Memory \"{resolved.WireValue}\" updated."; + _logger.LogInformation("SQLite update_memory edited document memory={MemoryId}", resolved.Handle); + return $"Memory \"{resolved.Handle}\" updated."; } if (args.OldText is not null) @@ -103,10 +103,10 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon var superseded = await _store.SupersedeRecordAsync(storageId.Value, recordPayload, ct); if (!superseded) - return $"Record \"{resolved.WireValue}\" not found."; + return $"Record \"{resolved.Handle}\" not found."; - _logger.LogInformation("SQLite update_memory superseded record memory={MemoryId}", resolved.WireValue); - return $"Record \"{resolved.WireValue}\" superseded."; + _logger.LogInformation("SQLite update_memory superseded record memory={MemoryId}", resolved.Handle); + return $"Record \"{resolved.Handle}\" superseded."; } protected override Task ExecuteAsync(Params args, CancellationToken ct) diff --git a/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs b/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs index 3193a355b..eb2b99c75 100644 --- a/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs +++ b/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs @@ -4,7 +4,6 @@ // // ----------------------------------------------------------------------- using Microsoft.Extensions.AI; -using Netclaw.Actors.Memory; using Netclaw.Actors.Protocol; using Netclaw.Configuration; using AiChatMessage = Microsoft.Extensions.AI.ChatMessage; @@ -278,8 +277,7 @@ private static string FormatRecallForLlm(AutomaticRecallResult recall) sb.AppendLine("mode: automatic"); foreach (var item in recall.Items) { - var typedId = MemoryTypedId.Parse(item.Id.Value).ToWireValue(); - sb.AppendLine($"- {item.Title} [{typedId}] sensitivity={item.Sensitivity} score={item.Score:F2}"); + sb.AppendLine($"- {item.Title} [{item.Id.Value}] sensitivity={item.Sensitivity} score={item.Score:F2}"); sb.AppendLine($" {item.Content}"); } return sb.ToString().TrimEnd(); From f17cf938c4629d3ec49a93045b64586291a526c9 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 04:45:33 +0000 Subject: [PATCH 5/7] =?UTF-8?q?fix(memory):=20address=20code-review=20nits?= =?UTF-8?q?=20=E2=80=94=20stale=20schema=20text=20+=20dead=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - get_memories: the Ids param description still taught the removed doc:/rec: colon envelope; update it to the verbatim doc-…/rec-… handle contract the rest of the PR standardized on. - SQLiteMemoryStore: remove GetMemoriesByIdsAsync — after get_memories switched to ResolveMemoryHandlesAsync + GetMemoriesByResolvedHandlesAsync it has no remaining callers. --- src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs | 13 ------------- src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs | 2 +- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index 916922023..534a35ed7 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -611,19 +611,6 @@ AND COALESCE(r.audience, $fallbackAudience) IN ({whereAudiences}) }, ct); } - public async Task> GetMemoriesByIdsAsync( - IReadOnlyList ids, - string boundary, - TrustAudience audience, - CancellationToken ct = default) - { - if (ids.Count == 0) - return []; - - var resolvedIds = await ResolveMemoryHandlesAsync(ids, boundary, audience, ct); - return await GetMemoriesByResolvedHandlesAsync(resolvedIds, boundary, audience, ct); - } - /// /// Hydrates memories from handles that have already been resolved. Callers that run /// up front (e.g. to surface per-ID errors) pass diff --git a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs index 0ffcc55a5..f16c08a8f 100644 --- a/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs +++ b/src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs @@ -23,7 +23,7 @@ public sealed partial class SqliteGetMemoriesTool : NetclawTool? logger = null) From 336bccbca2d5f65e861903a1da2a801e3a13bced Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 16:19:49 +0000 Subject: [PATCH 6/7] fix(memory): resolve record handles through the supersede chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records are append-only: update_memory supersedes a record by inserting a new rec-{guid} and leaving the old row physically present, and no read path filtered superseded rows. So after editing rec-x the model's stable handle still hydrated the pre-edit row — get_memories("rec-x") returned the OLD payload while the edit lived under an id the model was never shown. (Confirmed against the live DB: a superseded record whose old row is still present.) Resolve record ids through their supersede chain to the head (latest) row at the single resolution point (ResolveMemoryHandleAsync), so both get_memories and update_memory act on the current version. Documents edit in place and are unaffected. Chain walk is a recursive CTE over supersedes_record_id; acyclic, so the deepest reachable row is the head. Adds an end-to-end regression test: a record edited twice via its original handle reads back the latest content (not the pre-edit or first-edit row). --- .../Memory/SqliteMemoryToolsTests.cs | 32 +++++++++++++++ .../Memory/SQLiteMemoryStore.cs | 41 +++++++++++++++++-- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs index 0626b42c3..ff1ffdf75 100644 --- a/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs @@ -441,6 +441,38 @@ public async Task UpdateMemory_rejects_old_text_for_record_with_specific_error() Assert.Contains("records do not support old_text/new_text", result); } + [Fact] + public async Task UpdateMemory_record_edits_stay_readable_via_the_original_handle() + { + await _store.InitializeAsync(TestContext.Current.CancellationToken); + var now = _timeProvider.GetUtcNow().ToUnixTimeMilliseconds(); + await StoreRecordAsync("rec-pref", "Coffee order", "The order is a flat white.", now); + + // Records are superseded (append-only), not edited in place. The stable handle the model + // holds (rec-pref) must keep resolving to the CURRENT content across successive edits — + // walking the supersede chain to the head — not the pre-edit or first-edit row. + var update = new SqliteUpdateMemoryTool(_store); + await update.ExecuteAsync( + new Dictionary { ["id"] = "rec-pref", ["new_text"] = "The order is a cortado." }, + PersonalContext(), + CancellationToken.None); + var second = await update.ExecuteAsync( + new Dictionary { ["id"] = "rec-pref", ["new_text"] = "The order is an espresso." }, + PersonalContext(), + CancellationToken.None); + + var get = new SqliteGetMemoriesTool(_store, _timeProvider); + var hydrated = await get.ExecuteAsync( + new Dictionary { ["ids"] = "rec-pref" }, + PersonalContext(), + CancellationToken.None); + + Assert.Contains("superseded", second); + Assert.Contains("espresso", hydrated); + Assert.DoesNotContain("flat white", hydrated); + Assert.DoesNotContain("cortado", hydrated); + } + public async ValueTask DisposeAsync() { await SqliteTempDirectoryCleanup.TryDeleteDirectoryAsync(_baseDir); diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index 534a35ed7..abd09a11b 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -750,11 +750,19 @@ public async Task ResolveMemoryHandleAsync( return await WithConnectionAsync(async (conn, ct) => { - // The parsed id is the exact storage key (primary key, unique per table), so a single + // Records are append-only: an edit inserts a new row that supersedes the old one and + // leaves the old row physically present. Follow the supersede chain to the head (latest) + // row so a stable handle the model was given earlier still resolves to the current + // content instead of the pre-edit row. Documents edit in place and have no such chain. + var storageId = parsed.Kind == MemoryKind.Record + ? await ResolveRecordHeadAsync(conn, parsed.Id, ct) + : parsed.Id; + + // The resolved id is the exact storage key (primary key, unique per table), so a single // visibility-scoped lookup either finds it or it does not — no candidate guessing. - var visible = await MemoryIdVisibleAsync(conn, parsed.Kind, parsed.Id, boundary, allowedAudiences, ct); + var visible = await MemoryIdVisibleAsync(conn, parsed.Kind, storageId, boundary, allowedAudiences, ct); return visible - ? ResolvedMemoryHandle.Found(rawId, parsed.Kind, parsed.Id) + ? ResolvedMemoryHandle.Found(rawId, parsed.Kind, storageId) : ResolvedMemoryHandle.Failed(rawId, parsed.Kind, $"Memory \"{rawId}\" was not found or is not accessible from this session."); }, ct); } @@ -1636,6 +1644,33 @@ private async Task WithConnectionAsync( await work(conn, ct); } + /// + /// Walks memory_records.supersedes_record_id forward to the head (latest) row in the + /// supersede chain. Returns the input id unchanged when it has not been superseded or does + /// not exist. Chains are acyclic (each supersede points at an older row), so the deepest + /// reachable row is the head. + /// + private static async Task ResolveRecordHeadAsync( + SqliteConnection conn, + MemoryStorageId recordId, + CancellationToken ct) + { + await using var cmd = conn.CreateCommand(); + cmd.CommandText = """ + WITH RECURSIVE chain(id, depth) AS ( + SELECT $id, 0 + UNION ALL + SELECT n.record_id, c.depth + 1 + FROM memory_records n + JOIN chain c ON n.supersedes_record_id = c.id + ) + SELECT id FROM chain ORDER BY depth DESC LIMIT 1; + """; + cmd.Parameters.AddWithValue("$id", recordId.Value); + var head = (string?)await cmd.ExecuteScalarAsync(ct); + return string.IsNullOrEmpty(head) ? recordId : new MemoryStorageId(head); + } + private static async Task MemoryIdVisibleAsync( SqliteConnection conn, MemoryKind kind, From ac6ef8701eb1bb0cc6d5dff398c4c2e3725d854b Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 1 Jul 2026 18:02:11 +0000 Subject: [PATCH 7/7] refactor(memory): resolve batch over one connection + dedupe visibility check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the remaining code-review cleanups on the stable-handle change: - get_memories resolved each id on its own SqliteConnection (N+1 opens) and rebuilt allowedAudiences per id. ResolveMemoryHandlesAsync now resolves the whole batch over a single connection with one allowedAudiences set, via a shared connection-scoped core (ResolveHandleOnConnectionAsync); the single ResolveMemoryHandleAsync reuses the same core. - The boundary/audience visibility rule was duplicated between resolution (MemoryIdVisibleAsync) and hydration (two inline checks). Extracted one IsAccessible predicate so they cannot drift. - Documented on MemoryTypedId.Parse that the kind prefix is matched case-insensitively while the storage key is matched verbatim/case-sensitively — a mis-cased key fails loud rather than silently coercing to another row (case-insensitive key matching would be a silent fallback). --- src/Netclaw.Actors/Memory/MemoryTypedId.cs | 7 +++ .../Memory/SQLiteMemoryStore.cs | 53 ++++++++++++++----- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/Netclaw.Actors/Memory/MemoryTypedId.cs b/src/Netclaw.Actors/Memory/MemoryTypedId.cs index d2ac3eec4..3facd8f19 100644 --- a/src/Netclaw.Actors/Memory/MemoryTypedId.cs +++ b/src/Netclaw.Actors/Memory/MemoryTypedId.cs @@ -32,6 +32,13 @@ public MemoryTypedId(MemoryKind kind, string id) /// it is never rewritten — so there is exactly one key per input and no ambiguity. /// Returns with the raw value when the prefix is unrecognized. /// + /// + /// The kind prefix is matched case-insensitively (a tolerant envelope), but the storage key + /// that follows is preserved verbatim and later matched case-sensitively against the canonical + /// lowercase primary key. This is deliberate: generated ids are always lowercase, so a + /// mis-cased key is treated as not-found (fail-loud) rather than silently coerced to a + /// different row. + /// public static MemoryTypedId Parse(string raw) { raw = raw.Trim(); diff --git a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs index abd09a11b..59d77a8ee 100644 --- a/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs +++ b/src/Netclaw.Actors/Memory/SQLiteMemoryStore.cs @@ -672,8 +672,7 @@ FROM memory_documents ExpiresAtMs: reader.IsDBNull(12) ? null : reader.GetInt64(12), UpdatedAtMs: reader.GetInt64(13))); - if (!string.Equals(output[^1].Boundary, boundary, StringComparison.OrdinalIgnoreCase) - || !allowedAudiences.Contains(output[^1].Audience)) + if (!IsAccessible(output[^1].Boundary, output[^1].Audience, boundary, allowedAudiences)) { output.RemoveAt(output.Count - 1); } @@ -709,8 +708,7 @@ FROM memory_records ExpiresAtMs: reader.IsDBNull(12) ? null : reader.GetInt64(12), UpdatedAtMs: reader.GetInt64(13))); - if (!string.Equals(output[^1].Boundary, boundary, StringComparison.OrdinalIgnoreCase) - || !allowedAudiences.Contains(output[^1].Audience)) + if (!IsAccessible(output[^1].Boundary, output[^1].Audience, boundary, allowedAudiences)) { output.RemoveAt(output.Count - 1); } @@ -727,10 +725,21 @@ public async Task> ResolveMemoryHandlesAsync TrustAudience audience, CancellationToken ct = default) { + if (rawIds.Count == 0) + return []; + + var allowedAudiences = MemoryPolicyEvaluator.AllowedAudienceWireValues(audience) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + // Resolve the whole batch over one connection so N ids cost a single connection open + // (and one allowedAudiences set), not N of each. + return await WithConnectionAsync(async (conn, ct) => + { var output = new List(rawIds.Count); foreach (var rawId in rawIds) - output.Add(await ResolveMemoryHandleAsync(rawId, boundary, audience, ct)); - return output; + output.Add(await ResolveHandleOnConnectionAsync(conn, rawId, boundary, allowedAudiences, ct)); + return (IReadOnlyList)output; + }, ct); } public async Task ResolveMemoryHandleAsync( @@ -738,6 +747,23 @@ public async Task ResolveMemoryHandleAsync( string boundary, TrustAudience audience, CancellationToken ct = default) + { + var allowedAudiences = MemoryPolicyEvaluator.AllowedAudienceWireValues(audience) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + return await WithConnectionAsync( + (conn, ct) => ResolveHandleOnConnectionAsync(conn, rawId, boundary, allowedAudiences, ct), + ct); + } + + // Core handle resolution against an already-open connection, so a batch (get_memories) can + // share a single connection and allowedAudiences set instead of opening a connection per id. + private static async Task ResolveHandleOnConnectionAsync( + SqliteConnection conn, + string rawId, + string boundary, + ISet allowedAudiences, + CancellationToken ct) { var parsed = MemoryTypedId.Parse(rawId); if (parsed.Kind is not (MemoryKind.Document or MemoryKind.Record)) @@ -745,11 +771,6 @@ public async Task ResolveMemoryHandleAsync( if (parsed.Id.IsEmpty) return ResolvedMemoryHandle.Failed(rawId, parsed.Kind, "ID payload is required."); - var allowedAudiences = MemoryPolicyEvaluator.AllowedAudienceWireValues(audience) - .ToHashSet(StringComparer.OrdinalIgnoreCase); - - return await WithConnectionAsync(async (conn, ct) => - { // Records are append-only: an edit inserts a new row that supersedes the old one and // leaves the old row physically present. Follow the supersede chain to the head (latest) // row so a stable handle the model was given earlier still resolves to the current @@ -764,7 +785,6 @@ public async Task ResolveMemoryHandleAsync( return visible ? ResolvedMemoryHandle.Found(rawId, parsed.Kind, storageId) : ResolvedMemoryHandle.Failed(rawId, parsed.Kind, $"Memory \"{rawId}\" was not found or is not accessible from this session."); - }, ct); } public async Task> SearchByPlanAsync( @@ -1702,10 +1722,15 @@ FROM memory_records var itemBoundary = reader.IsDBNull(0) ? TrustBoundary.LegacyRestrictedValue : reader.GetString(0); var itemAudience = reader.IsDBNull(1) ? TrustAudience.Personal.ToWireValue() : reader.GetString(1); - return string.Equals(itemBoundary, boundary, StringComparison.OrdinalIgnoreCase) - && allowedAudiences.Contains(itemAudience); + return IsAccessible(itemBoundary, itemAudience, boundary, allowedAudiences); } + // Single source of truth for the boundary/audience visibility rule, shared by handle + // resolution (MemoryIdVisibleAsync) and hydration so the two paths cannot drift. + private static bool IsAccessible(string itemBoundary, string itemAudience, string boundary, ISet allowedAudiences) + => string.Equals(itemBoundary, boundary, StringComparison.OrdinalIgnoreCase) + && allowedAudiences.Contains(itemAudience); + private static async Task EnsureAnchorAsync( SqliteConnection conn, SqliteTransaction tx,