Skip to content

Commit 84c9867

Browse files
joslatjoslat
committed
feat: NAMS Phase 10 -- expand live scenario coverage (concurrency, cancellation, Unicode)
Adds 4 live tests to NamsLiveConnectivityTests.cs, all run against the real NAMS SaaS: concurrent resolution of the same identity reconciles to one conversation, two concurrent different users don't cross-contaminate chat history, live mid-request cancellation propagates correctly, and Unicode/emoji content round-trips byte-for-byte. Documents which of the plan's mandatory scenario-matrix areas are already covered elsewhere (HTTP-simulation for service-failure/rate-limit, Phase 9 for observability) versus explicitly deferred (multi-instance mapping, data lifecycle, DNS/TLS simulation).
1 parent 18846c6 commit 84c9867

2 files changed

Lines changed: 166 additions & 0 deletions

File tree

‎docs/reviews/NAMS_Phase10_LiveScenarioExpansion_PlanningAndImplementationPlan.md‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# NAMS Phase 10 — Live Scenario Expansion — Planning and Implementation Plan
2+
3+
**Prepared:** 2026-07-17
4+
**Branch:** `nams/phase10-expansion`
5+
**Purpose:** Expands live-integration coverage from the earlier live-validation branch's 3 scenarios toward
6+
the engineering plan's own "mandatory scenario matrix" (§7 Phase 10). Not an attempt at all 18 matrix areas
7+
in one PR -- see §2 for what's covered elsewhere already, and §3 for what's explicitly deferred.
8+
9+
## 1. What this adds
10+
11+
4 new tests in `tests/AgentMemory.Tests.Integration/Nams/NamsLiveConnectivityTests.cs`, all run against the
12+
real NAMS SaaS (`agent-memory-dotnet-dev` workspace):
13+
14+
- **`ResolveAsync_SameIdentityResolvedConcurrently_ReconcilesToOneConversation`** (matrix: Conversation ->
15+
concurrent create) -- resolves the SAME identity twice concurrently; asserts both calls agree on one
16+
conversation ID and exactly one of the two actually created it. Phase 3's `KeyedAsyncLock` reconciliation
17+
was unit-tested against a fake client; this is the first time it's proven against the real API's actual
18+
concurrent-create race behavior.
19+
- **`PersistTurnAsync_TwoConcurrentUsers_DoNotCrossContaminateChatHistory`** (matrix: Concurrency -> parallel
20+
turns, different users) -- two different identities' conversations, persisted concurrently, each recalling
21+
only its own marker text, never the other's. Deliberately scoped to the **chat-history/recent-message
22+
tier** (conversation-scoped), not entity extraction -- the `AgentMemory.Sample.NamsAgent` README already
23+
documents that NAMS's entity search is workspace-wide, not conversation-scoped, as a separate, known
24+
characteristic; this test doesn't re-litigate that, it confirms the tier that IS properly isolated.
25+
- **`RecallAsync_CancelledBeforeTheHttpRoundTripCompletes_PropagatesOperationCanceledException`** (matrix:
26+
Cancellation -> during request) -- cancels 1ms after issuing a live recall call; confirms cancellation
27+
propagates as `OperationCanceledException` against the real service, not just the fake-handler-backed unit
28+
tests.
29+
- **`PersistTurnAsync_UnicodeAndEmojiContent_RoundTripsByteForByte`** (matrix: Payload -> Unicode) --
30+
persists Japanese text + an emoji + accented characters, confirms it recalls back byte-for-byte.
31+
32+
## 2. Already covered elsewhere (not duplicated here)
33+
34+
- **Identity** (missing/valid), **Conversation** (create/reuse), **Recall** (empty/normal), **Persistence**
35+
(success) -- the original 3 live tests (this session's earlier live-validation branch).
36+
- **Service failure** (401/403/404/429/500/502, malformed JSON) -- `NamsClientExceptionMapperTests.cs`,
37+
already comprehensive via HTTP-simulation (a real network call can't deterministically produce a 500 from
38+
the live service on demand; simulation is the correct tool here, not live).
39+
- **Rate limit** (retry, `Retry-After`), **Cancellation** (before request, during retry delay) --
40+
`NamsRetryPolicyTests.cs`, HTTP-simulation.
41+
- **Observability** (expected metrics, no secrets/PII) -- Phase 9, just merged.
42+
- **Eventual consistency** (immediate absence, bounded poll, completion) -- the original 3 live tests'
43+
bounded-poll pattern, now also exercised implicitly by this PR's new tests.
44+
- **Security** (prompt injection, delimiter forgery, wrong user/workspace) -- these are properties of the
45+
MAF-layer mapping/gating logic (`NamsMafTypeMapper`, Phase 6), already unit-tested there; they're not
46+
live-NAMS-specific behavior to re-verify against the real service.
47+
48+
## 3. Explicitly deferred
49+
50+
- **Multi-instance mapping** (separate resolver processes, crash reconciliation) -- needs actual
51+
multi-process orchestration, a bigger investment than fits this increment.
52+
- **Session restore as an asserted integration test** (vs. the `NamsAgent` sample's informal demonstration)
53+
-- real value, but a distinct piece of work (building a live MAF-agent test harness), left for later.
54+
- **DNS/TLS failure simulation** -- not practically triggerable against a real external service on demand.
55+
- **MCP separation, Tools** -- N/A, Phase 8 not built yet (next in this session's queue).
56+
- **Data lifecycle** (conversation deletion, export) -- needs live testing of destructive operations and/or
57+
an organizational decision; `INamsClient` doesn't even expose a delete operation yet.
58+
- **Payload: empty / max size / multi-message / non-text** -- Unicode was judged the highest-value single
59+
addition for this pass; the rest are reasonable follow-ups, not done here.
60+
61+
## 4. Verification
62+
63+
- `dotnet build AgentMemory.slnx -c Release` -- 0 warnings, 0 errors.
64+
- `dotnet test tests/AgentMemory.Tests.Integration --filter "FullyQualifiedName~Nams.NamsLiveConnectivityTests"`
65+
-- **7/7 live, ~7s total** (3 original + 4 new).
66+
- `dotnet test tests/AgentMemory.Tests.Unit` -- full suite green, unaffected (no unit-level changes this
67+
phase).
68+
69+
## 5. Definition of done
70+
71+
- [x] 4 new live scenarios added and passing against the real NAMS SaaS.
72+
- [x] Coverage overlap and deferred scope explicitly documented (no silent gaps).
73+
- [ ] Self-reviewed, PR opened, CI green (live tests report Skipped there, as established), merged to `main`.

‎tests/AgentMemory.Tests.Integration/Nams/NamsLiveConnectivityTests.cs‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,99 @@ await persistence.PersistTurnAsync(
9898
found.Should().BeTrue($"an entity named '{entityName}' should eventually be extracted and recallable");
9999
}
100100

101+
[LiveNamsFact]
102+
public async Task ResolveAsync_SameIdentityResolvedConcurrently_ReconcilesToOneConversation()
103+
{
104+
var resolver = _fixture.Services!.GetRequiredService<INamsConversationResolver>();
105+
var identity = UniqueIdentity(); // one identity, resolved twice concurrently below
106+
107+
var task1 = resolver.ResolveAsync(identity, CancellationToken.None);
108+
var task2 = resolver.ResolveAsync(identity, CancellationToken.None);
109+
var results = await Task.WhenAll(task1, task2);
110+
111+
results[0].NamsConversationId.Should().Be(results[1].NamsConversationId,
112+
"both resolutions are for the same identity -- they must agree on one conversation");
113+
results.Count(r => r.WasCreated).Should().Be(1,
114+
"exactly one of the two concurrent resolutions should have actually created the conversation");
115+
}
116+
117+
[LiveNamsFact]
118+
public async Task PersistTurnAsync_TwoConcurrentUsers_DoNotCrossContaminateChatHistory()
119+
{
120+
var services = _fixture.Services!;
121+
var resolver = services.GetRequiredService<INamsConversationResolver>();
122+
var persistence = services.GetRequiredService<INamsPersistenceService>();
123+
var recall = services.GetRequiredService<INamsRecallService>();
124+
125+
var resolveTaskA = resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
126+
var resolveTaskB = resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
127+
await Task.WhenAll(resolveTaskA, resolveTaskB);
128+
var conversationA = await resolveTaskA;
129+
var conversationB = await resolveTaskB;
130+
131+
var markerA = $"Concurrency test A {Guid.NewGuid():N}";
132+
var markerB = $"Concurrency test B {Guid.NewGuid():N}";
133+
await Task.WhenAll(
134+
persistence.PersistTurnAsync(conversationA.NamsConversationId, [new NamsMessageToPersist(markerA)], [], CancellationToken.None),
135+
persistence.PersistTurnAsync(conversationB.NamsConversationId, [new NamsMessageToPersist(markerB)], [], CancellationToken.None));
136+
137+
var found = await PollUntilAsync(
138+
async () =>
139+
{
140+
var recalledA = await recall.RecallAsync(conversationA.NamsConversationId, markerA, CancellationToken.None);
141+
var recalledB = await recall.RecallAsync(conversationB.NamsConversationId, markerB, CancellationToken.None);
142+
var aHasOwnMarker = recalledA.Items.Any(i => i.Content.Contains(markerA, StringComparison.Ordinal));
143+
var bHasOwnMarker = recalledB.Items.Any(i => i.Content.Contains(markerB, StringComparison.Ordinal));
144+
var noCrossContamination =
145+
!recalledA.Items.Any(i => i.Content.Contains(markerB, StringComparison.Ordinal))
146+
&& !recalledB.Items.Any(i => i.Content.Contains(markerA, StringComparison.Ordinal));
147+
return aHasOwnMarker && bHasOwnMarker && noCrossContamination;
148+
},
149+
timeout: TimeSpan.FromSeconds(30));
150+
151+
found.Should().BeTrue("each conversation's recall should see only its own persisted message, never the other's");
152+
}
153+
154+
[LiveNamsFact]
155+
public async Task RecallAsync_CancelledBeforeTheHttpRoundTripCompletes_PropagatesOperationCanceledException()
156+
{
157+
var services = _fixture.Services!;
158+
var resolver = services.GetRequiredService<INamsConversationResolver>();
159+
var recall = services.GetRequiredService<INamsRecallService>();
160+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
161+
162+
using var cts = new CancellationTokenSource();
163+
cts.CancelAfter(TimeSpan.FromMilliseconds(1)); // a live round trip to NAMS never completes this fast
164+
165+
var act = () => recall.RecallAsync(conversation.NamsConversationId, "test", cts.Token);
166+
167+
await act.Should().ThrowAsync<OperationCanceledException>();
168+
}
169+
170+
[LiveNamsFact]
171+
public async Task PersistTurnAsync_UnicodeAndEmojiContent_RoundTripsByteForByte()
172+
{
173+
var services = _fixture.Services!;
174+
var resolver = services.GetRequiredService<INamsConversationResolver>();
175+
var persistence = services.GetRequiredService<INamsPersistenceService>();
176+
var recall = services.GetRequiredService<INamsRecallService>();
177+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
178+
179+
var marker = $"Unicode test {Guid.NewGuid():N}: 日本語のテスト 🎉 émoji café";
180+
await persistence.PersistTurnAsync(
181+
conversation.NamsConversationId, userMessages: [new NamsMessageToPersist(marker)], assistantMessages: [], CancellationToken.None);
182+
183+
var found = await PollUntilAsync(
184+
async () =>
185+
{
186+
var recalled = await recall.RecallAsync(conversation.NamsConversationId, marker, CancellationToken.None);
187+
return recalled.Items.Any(i => i.Content.Contains(marker, StringComparison.Ordinal));
188+
},
189+
timeout: TimeSpan.FromSeconds(30));
190+
191+
found.Should().BeTrue("Unicode/emoji content should round-trip through NAMS byte-for-byte");
192+
}
193+
101194
private static NamsConversationIdentity UniqueIdentity([CallerMemberName] string testName = "") => new()
102195
{
103196
ApplicationId = "agent-memory-dotnet-live-tests",

0 commit comments

Comments
 (0)