Skip to content

NAMS Phase 10d: session-restore asserted live test#144

Merged
joslat merged 2 commits into
mainfrom
nams/phase10d-session-restore-test
Jul 17, 2026
Merged

NAMS Phase 10d: session-restore asserted live test#144
joslat merged 2 commits into
mainfrom
nams/phase10d-session-restore-test

Conversation

@joslat

@joslat joslat commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Formalizes what AgentMemory.Sample.NamsAgent's console demo only showed informally: that a MAF AgentSession's NAMS conversation identity survives SerializeSessionAsync/DeserializeSessionAsync.
  • Drives a REAL ChatClientAgent (via AsAIAgent) wired to a REAL, DI-resolved NamsMemoryContextProvider against the live NAMS SaaS — only the model call is stubbed (NSubstitute IChatClient, canned response), so there's no real LLM dependency, but genuine MAF session-serialization behavior is exercised.
  • A probe of the real serialized-session JSON (done before writing any test code) revealed ChatClientAgent's own state-bag serialization already embeds the memory identity (user_id/session_id/conversation_id/application_id) alongside its own turn history — so this adds two tests: the sample's documented "re-apply identity after restore" recipe, and a second proving the identity survives restore even without re-applying it.
  • Extends NamsLiveFixture with AddAgentMemoryFramework()/AddNamsAgentMemoryFramework() (additive; verified non-breaking for all 15 pre-existing NAMS live tests) and adds the Microsoft.Agents.AI package + a project reference to AgentMemory.AgentFramework.Nams.

See docs/reviews/NAMS_Phase10d_SessionRestoreTest_PlanningAndImplementationPlan.md for full design reasoning, including why the tests deliberately do NOT assert on the stub chat client's received messages (MAF's own built-in chat history would make that ambiguous), and the self-review findings/fixes below.

Self-review

2 parallel independent reviewers found and fixed:

  • HIGH: the "re-apply identity" test originally re-stamped the restored session with the same closed-over identity values used before serialization, making it unable to actually prove restore preserved anything. Fixed by asserting the restored session's identity via GetMemoryIdentity() independently, before re-applying it.
  • 3 low/medium conventions findings (missing [CallerMemberName] orphan-tracing, duplicated PollUntilAsync, minor redundancy) — all fixed.

Test plan

  • dotnet build AgentMemory.slnx -c Release — 0 warnings, 0 errors
  • dotnet test tests/AgentMemory.Tests.Unit — 3262/3262 green
  • dotnet test tests/AgentMemory.Tests.Integration --filter "FullyQualifiedName~Nams" — 17/17 live against real NAMS SaaS (15 previous + 2 new)

🤖 Generated with Claude Code

joslat and others added 2 commits July 18, 2026 00:24
Formalizes what AgentMemory.Sample.NamsAgent's console demo only showed
informally: that a MAF AgentSession's NAMS conversation identity survives
SerializeSessionAsync/DeserializeSessionAsync. Drives a real ChatClientAgent
(stubbed model only) against the live NAMS SaaS via a real, DI-resolved
NamsMemoryContextProvider.

A probe of the real serialized-session JSON showed ChatClientAgent's own
state-bag serialization already carries the memory identity through
automatically (alongside its own InMemoryChatHistoryProvider turn history),
so this adds two tests: the documented re-apply-identity-after-restore
recipe, and a second proving the state bag survives restore even without
re-applying it.

Extends NamsLiveFixture with AddAgentMemoryFramework()/
AddNamsAgentMemoryFramework() (additive) and adds the Microsoft.Agents.AI
package plus a project reference to AgentMemory.AgentFramework.Nams.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix a HIGH-severity vacuousness bug: the "with re-applied identity" test
re-stamped the restored session using the same closed-over identity values
already used before serialization, so it could never actually prove restore
preserved anything -- reintroducing, one layer down, the exact triviality
the design was meant to avoid. Fixed by reading the restored session's
identity back via GetMemoryIdentity() and asserting it independently BEFORE
re-applying WithMemoryIdentity.

Also: extract PollUntilAsync into a shared NamsLiveTestHelpers (was
duplicated verbatim from NamsLiveConnectivityTests.cs), and switch
UniqueIds() to a UniqueIdentity() returning NamsConversationIdentity
directly with a [CallerMemberName] embedded in UserId for orphan tracing,
matching sibling test files' conventions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 17, 2026 22:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@joslat
joslat merged commit 17427e8 into main Jul 17, 2026
4 checks passed
@joslat
joslat deleted the nams/phase10d-session-restore-test branch July 17, 2026 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants