Skip to content

Commit 0b0cdbd

Browse files
authored
NAMS: well-known endpoint, X-Workspace-Id fix, live-integration test scaffold (#136)
* feat: NAMS well-known endpoint, X-Workspace-Id header fix, live-integration test scaffold A live NAMS SaaS account became available mid-session, revealing that an account-wide API key needs X-Workspace-Id (a workspace-scoped key carries it implicitly) -- NamsOptions.WorkspaceId was captured but never attached. Adds NamsWellKnown.Endpoint (the public SaaS base URL, non-secret) and a skip-cleanly live-integration test scaffold that validates the full Phase 4-6 pipeline (identity resolution -> persistence -> recall) against the real service for the first time. * fix: self-review findings from live-validation branch (WorkspaceId validation, poll resilience, conventions) - Add NamsOptionValidator.HasValidWorkspaceId: fail fast on control characters instead of a lazy FormatException on the first HTTP call. - PollUntilAsync now swallows exceptions from the polled condition itself (not just the delay), matching Neo4jIntegrationFixture.WaitForVectorIndexesAsync's established pattern -- a transient blip against the live service no longer aborts the whole bounded poll. - Drop the redundant second Category trait, remove ConfigureAwait(false) from test code, and match NamsLiveTestCollection's shape to its one direct precedent (Neo4jIntegrationCollection).
1 parent 9d57b26 commit 0b0cdbd

14 files changed

Lines changed: 449 additions & 0 deletions
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# NAMS Live Validation & Integration Test Scaffold — Planning and Implementation Plan
2+
3+
**Prepared:** 2026-07-17
4+
**Branch:** `nams/live-validation-integration-tests`
5+
**Purpose:** Not one of the engineering plan's numbered phases -- this is a small, opportunistic follow-up
6+
that became possible mid-session when a live NAMS SaaS account and API key were provisioned. It closes two
7+
concrete gaps Phases 1-6 could only leave as documented unknowns (no live sandbox existed yet), and adds a
8+
skip-cleanly live-integration test scaffold so the whole Phase 4-6 pipeline (identity resolution -> recall ->
9+
persistence) is now verified against the real service, not just deterministic fakes.
10+
11+
## 1. Trigger and scope
12+
13+
Mid-session, a live NAMS SaaS account became available (`memory.neo4jlabs.com`), including a dedicated,
14+
isolated `agent-memory-dotnet-dev` workspace (with its own free, auto-expiring managed database) created
15+
specifically so live testing never touches a production/real workspace. Comparing the actual authenticated
16+
API docs against the existing `AgentMemory.Nams` client surfaced two small, concrete findings:
17+
18+
1. The real public SaaS base URL/auth contract matches Phase 2's guessed-from-public-docs shapes almost
19+
exactly (conversations, messages, messages/bulk, entities/search, context) -- no client-shape bugs found.
20+
2. `NamsOptions.WorkspaceId` was captured in configuration but never actually attached as a request header --
21+
harmless for a workspace-scoped API key (which carries its workspace implicitly), but a real gap for an
22+
account-wide (admin) key, which needs `X-Workspace-Id` to target a specific workspace.
23+
24+
**In scope:**
25+
- `NamsWellKnown.Endpoint` -- a non-secret constant for the public SaaS base URL, so consuming code/tests
26+
don't need an env var for something that's the same for everyone.
27+
- The `X-Workspace-Id` header fix in `NamsClientFactory.ConfigureHttpClient`.
28+
- A live-integration test scaffold (env-var-gated, skips cleanly and never runs in CI) exercising the full
29+
public surface (`INamsConversationResolver` -> `INamsPersistenceService` -> `INamsRecallService`) against
30+
the real, isolated dev workspace.
31+
32+
**Explicitly out of scope** (not triggered by this work):
33+
- Engineering plan Phase 7 (backend-neutral convergence) -- still gated on its own entry criteria, unrelated
34+
to this.
35+
- Phase 8 (MCP/capability-aware tools), Phase 9 (observability), the rest of Phase 10 (contract tests, HTTP
36+
simulation, TCK Platinum), Phase 11 (sample) -- untouched by this branch.
37+
- `SearchMessagesAsync`/`POST /v1/conversations/{id}/search` -- now confirmed to exist in the live API (it
38+
was dropped from Phase 2's `INamsClient` for lack of confirmation), but adding it is a separate, later
39+
decision, not bundled into this opportunistic fix.
40+
41+
## 2. Changes
42+
43+
- `src/AgentMemory.Nams/NamsWellKnown.cs` (new) -- `public static class NamsWellKnown { public static readonly
44+
Uri Endpoint = new("https://memory.neo4jlabs.com/v1"); }`.
45+
- `src/AgentMemory.Nams/Client/NamsClientFactory.cs` (modified) -- `ConfigureHttpClient` now adds an
46+
`X-Workspace-Id` default request header when `NamsOptions.WorkspaceId` is set (a static header, unlike the
47+
per-request-refreshed `Authorization` header `NamsAuthenticationHandler` attaches).
48+
- `tests/AgentMemory.Tests.Unit/Nams/NamsWellKnownTests.cs` (new) -- 1 test.
49+
- `tests/AgentMemory.Tests.Unit/Nams/NamsClientFactoryTests.cs` (modified) -- 2 new tests (header present when
50+
`WorkspaceId` set; absent when not).
51+
- `tests/AgentMemory.Tests.Integration/Nams/` (new) -- live-integration test scaffold:
52+
- `NamsLiveCredentials.cs` -- reads `NAMS_API_KEY`/`NAMS_DEV_WORKSPACE_ID` from the environment, with a
53+
Windows `EnvironmentVariableTarget.User` registry fallback so a credential set mid-session doesn't
54+
require restarting the IDE/terminal to become visible.
55+
- `LiveNamsFactAttribute.cs` -- a `[Fact]` variant that sets `Skip` at discovery time when credentials
56+
aren't available. CI still discovers these tests (same `Category=Integration` trait as every other
57+
integration test) but they report as Skipped there and never fail the build.
58+
- `NamsLiveFixture.cs` + `NamsLiveTestCollection.cs` -- a real DI container wired via
59+
`AddNamsAgentMemory`, exactly as a consuming application would.
60+
- `NamsLiveConnectivityTests.cs` -- 3 tests: conversation creation, persist-then-recall round trip, and a
61+
message-with-a-known-entity round trip (bounded polling for NAMS's asynchronous extraction, never
62+
unbounded per the plan's own Phase 10 release gate). Every identity uses a fresh GUID ("unique test user
63+
prefix" per Phase 10).
64+
- `tests/AgentMemory.Tests.Integration/AgentMemory.Tests.Integration.csproj` (modified) -- added a
65+
`ProjectReference` to `AgentMemory.Nams`.
66+
67+
## 3. Verification so far
68+
69+
- `dotnet test tests/AgentMemory.Tests.Unit --filter "FullyQualifiedName~Nams"` -- 162/162 green.
70+
- `dotnet build tests/AgentMemory.Tests.Integration` -- 0 warnings, 0 errors.
71+
- `dotnet test tests/AgentMemory.Tests.Integration --filter "FullyQualifiedName~Nams.NamsLiveConnectivityTests"`
72+
-- **all 3 live tests passed against the real NAMS SaaS**, ~10s total. First-ever live validation of the
73+
Phase 4-6 pipeline end to end.
74+
75+
## 4. Self-review findings and fixes
76+
77+
3 parallel reviewers (correctness / cross-file impact / cleanup-conventions), matching the Phase 4-6 pattern:
78+
79+
- **Correctness (2 fixed):** `PollUntilAsync` didn't catch exceptions from the polled condition itself (only
80+
from the delay) -- a single transient blip against the live service during the 30-60s window would abort
81+
the whole poll instead of retrying through the remaining budget. Fixed to swallow and keep polling, matching
82+
`Neo4jIntegrationFixture.WaitForVectorIndexesAsync`'s established pattern. Separately, `NamsOptions.WorkspaceId`
83+
had no fail-fast validation like every other option -- a stray control character (e.g. a pasted trailing
84+
newline) would throw lazily on the first HTTP request instead of a clear `OptionsValidationException` at
85+
startup. Added `NamsOptionValidator.HasValidWorkspaceId` + 2 tests.
86+
- **Cross-file impact (0 fixed, 1 clarified):** confirmed no existing `AddNamsAgentMemory` caller sets
87+
`WorkspaceId`, so the new header is a no-op for every pre-existing path; confirmed the manifest/CI
88+
consistency check is directory-driven (no update needed for a file added inside an existing package);
89+
re-ran and confirmed the doc's own test-count claims. Flagged (not a bug) that the new tests' trait would
90+
make CI *discover* them, not silently exclude them -- addressed by removing the redundant second trait (see
91+
below) and correcting this doc's wording.
92+
- **Cleanup/conventions (3 fixed):** dropped the redundant `[Trait("Category","LiveNams")]` (every other test
93+
class in the project uses exactly one Category trait); removed `ConfigureAwait(false)` from the new test
94+
code (CA2007 is `src/`-only, no sibling integration test uses it); changed `NamsLiveTestCollection` to the
95+
brace-bodied non-sealed form matching its one direct precedent, `Neo4jIntegrationCollection`.
96+
97+
Re-verified after fixes: 164/164 Nams unit tests green (162 + 2 new `WorkspaceId` validation tests), live
98+
suite still 3/3 green.
99+
100+
## 5. Definition of done
101+
102+
- [x] Live connectivity confirmed.
103+
- [x] `X-Workspace-Id` gap fixed and covered.
104+
- [x] `NamsWellKnown` added and covered.
105+
- [x] Live-integration test scaffold built and passing against the real service.
106+
- [x] Self-reviewed and fixes applied.
107+
- [ ] PR opened, CI green (live tests are discovered but report Skipped there -- no credentials in CI),
108+
merged to `main`.

src/AgentMemory.Nams/Client/NamsClientFactory.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ public static void ConfigureHttpClient(HttpClient httpClient, NamsOptions option
88
{
99
httpClient.BaseAddress = NormalizeBaseAddress(options.Endpoint);
1010
httpClient.Timeout = options.RequestTimeout;
11+
12+
// Only an account-wide (admin) API key needs this -- a workspace-scoped key already carries its
13+
// workspace implicitly. Static for the client's lifetime (unlike Authorization, which is refreshed
14+
// per-request by NamsAuthenticationHandler): a workspace binding never expires mid-session.
15+
if (!string.IsNullOrWhiteSpace(options.WorkspaceId))
16+
httpClient.DefaultRequestHeaders.Add("X-Workspace-Id", options.WorkspaceId);
1117
}
1218

1319
/// <summary>

src/AgentMemory.Nams/Internal/NamsOptionValidator.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,14 @@ options.Endpoint is null
5151
/// all can never successfully authenticate a request.
5252
/// </summary>
5353
public static bool HasApiKey(NamsOptions options) => !string.IsNullOrWhiteSpace(options.ApiKey);
54+
55+
/// <summary>
56+
/// True when <see cref="NamsOptions.WorkspaceId"/> is unset (it's optional -- a workspace-scoped API key
57+
/// carries its workspace implicitly; only an account-wide key needs this set) or contains no control
58+
/// characters. It is sent verbatim as the <c>X-Workspace-Id</c> header by
59+
/// <see cref="Client.NamsClientFactory.ConfigureHttpClient"/>; a stray CR/LF would otherwise throw a
60+
/// <see cref="FormatException"/> lazily on the first HTTP request instead of failing fast here at startup.
61+
/// </summary>
62+
public static bool HasValidWorkspaceId(NamsOptions options) =>
63+
options.WorkspaceId is null || !options.WorkspaceId.Any(char.IsControl);
5464
}

src/AgentMemory.Nams/NamsServiceCollectionExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public static IServiceCollection AddNamsAgentMemory(
3939
.Validate(NamsOptionValidator.HasNonNegativeMaxRetryAttempts, "NamsOptions.MaxRetryAttempts must be non-negative.")
4040
.Validate(NamsOptionValidator.HasNonNegativeInitialRetryDelay, "NamsOptions.InitialRetryDelay must be non-negative.")
4141
.Validate(NamsOptionValidator.HasApiKey, "NamsOptions.ApiKey must be provided (JWT/Auth0 authentication is not yet supported).")
42+
.Validate(NamsOptionValidator.HasValidWorkspaceId, "NamsOptions.WorkspaceId must not contain control characters.")
4243
.ValidateOnStart();
4344

4445
// Idempotent by construction: TryAddSingleton means a second AddNamsAgentMemory call is a
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
namespace AgentMemory.Nams;
2+
3+
/// <summary>
4+
/// Well-known, non-secret values for the public NAMS SaaS at <c>memory.neo4jlabs.com</c>. Only applicable
5+
/// when targeting that public service -- a self-hosted or otherwise-deployed NAMS-compatible endpoint must
6+
/// set <see cref="NamsOptions.Endpoint"/> to its own address instead.
7+
/// </summary>
8+
public static class NamsWellKnown
9+
{
10+
/// <summary>The public NAMS SaaS REST API base endpoint.</summary>
11+
public static readonly Uri Endpoint = new("https://memory.neo4jlabs.com/v1");
12+
}

tests/AgentMemory.Tests.Integration/AgentMemory.Tests.Integration.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
<ProjectReference Include="..\..\src\AgentMemory.Abstractions\AgentMemory.Abstractions.csproj" />
3535
<ProjectReference Include="..\..\src\AgentMemory.McpServer\AgentMemory.McpServer.csproj" />
3636
<ProjectReference Include="..\..\src\AgentMemory.AgentFramework\AgentMemory.AgentFramework.csproj" />
37+
<ProjectReference Include="..\..\src\AgentMemory.Nams\AgentMemory.Nams.csproj" />
3738
<ProjectReference Include="..\..\tools\AgentMemory.TckBridge\AgentMemory.TckBridge.csproj" />
3839
</ItemGroup>
3940

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
namespace AgentMemory.Tests.Integration.Nams;
2+
3+
/// <summary>
4+
/// A <see cref="FactAttribute"/> that skips at discovery time when <see cref="NamsLiveCredentials.IsAvailable"/>
5+
/// is <see langword="false"/>. These tests call the real NAMS SaaS and must never run in CI or on a machine
6+
/// without an isolated dev workspace configured (engineering plan Phase 10: "never run against production
7+
/// customer workspaces").
8+
/// </summary>
9+
public sealed class LiveNamsFactAttribute : FactAttribute
10+
{
11+
public LiveNamsFactAttribute()
12+
{
13+
if (!NamsLiveCredentials.IsAvailable)
14+
Skip = "Live NAMS credentials not configured (NAMS_API_KEY / NAMS_DEV_WORKSPACE_ID) -- skipping live NAMS test.";
15+
}
16+
}
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
using System.Runtime.CompilerServices;
2+
using Microsoft.Extensions.DependencyInjection;
3+
using FluentAssertions;
4+
using AgentMemory.Nams.Identity;
5+
using AgentMemory.Nams.Persistence;
6+
using AgentMemory.Nams.Recall;
7+
8+
namespace AgentMemory.Tests.Integration.Nams;
9+
10+
/// <summary>
11+
/// Live round-trip tests against the real NAMS SaaS (<c>memory.neo4jlabs.com</c>), exercised entirely
12+
/// through the public surface (<see cref="INamsConversationResolver"/>, <see cref="INamsPersistenceService"/>,
13+
/// <see cref="INamsRecallService"/>) -- the same contract <c>NamsMemoryContextProvider</c> (Phase 6) uses
14+
/// internally. Every identity is a fresh GUID (engineering plan Phase 10: "unique test user prefix"), and
15+
/// these only ever run against the isolated <c>agent-memory-dotnet-dev</c> workspace -- never a production
16+
/// customer workspace. <see cref="LiveNamsFactAttribute"/> skips cleanly when
17+
/// NAMS_API_KEY/NAMS_DEV_WORKSPACE_ID aren't configured -- CI still discovers these tests (they carry the
18+
/// same <c>Category=Integration</c> trait as every other integration test), but they report as Skipped
19+
/// there and never fail the build.
20+
/// </summary>
21+
[Collection("NAMS Live")]
22+
[Trait("Category", "Integration")]
23+
public sealed class NamsLiveConnectivityTests
24+
{
25+
private readonly NamsLiveFixture _fixture;
26+
27+
public NamsLiveConnectivityTests(NamsLiveFixture fixture) => _fixture = fixture;
28+
29+
[LiveNamsFact]
30+
public async Task ResolveAsync_NewIdentity_CreatesConversation()
31+
{
32+
var resolver = _fixture.Services!.GetRequiredService<INamsConversationResolver>();
33+
34+
var result = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
35+
36+
result.WasCreated.Should().BeTrue();
37+
result.NamsConversationId.Should().NotBeNullOrWhiteSpace();
38+
}
39+
40+
[LiveNamsFact]
41+
public async Task PersistTurnAsync_ThenRecallAsync_EventuallyReturnsThePersistedMessage()
42+
{
43+
var services = _fixture.Services!;
44+
var resolver = services.GetRequiredService<INamsConversationResolver>();
45+
var persistence = services.GetRequiredService<INamsPersistenceService>();
46+
var recall = services.GetRequiredService<INamsRecallService>();
47+
48+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
49+
50+
var marker = $"NAMS live test marker {Guid.NewGuid():N}: John works at Acme Corp in Denver.";
51+
var persistResult = await persistence.PersistTurnAsync(
52+
conversation.NamsConversationId,
53+
userMessages: [new NamsMessageToPersist(marker)],
54+
assistantMessages: [],
55+
CancellationToken.None);
56+
57+
persistResult.Outcome.Should().Be(NamsPersistenceOutcome.Persisted);
58+
59+
// Bounded poll -- NAMS ingestion/extraction is asynchronous. Never unbounded (Phase 10 release gate
60+
// explicitly rejects unbounded eventual-consistency waits).
61+
var found = await PollUntilAsync(
62+
async () =>
63+
{
64+
var recalled = await recall.RecallAsync(conversation.NamsConversationId, marker, CancellationToken.None);
65+
return recalled.Items.Any(i => i.Content.Contains(marker, StringComparison.Ordinal));
66+
},
67+
timeout: TimeSpan.FromSeconds(30));
68+
69+
found.Should().BeTrue("the just-persisted message should surface in recall's recent-messages tier");
70+
}
71+
72+
[LiveNamsFact]
73+
public async Task PersistTurnAsync_MessageWithKnownEntity_EventuallyExtractsIt()
74+
{
75+
var services = _fixture.Services!;
76+
var resolver = services.GetRequiredService<INamsConversationResolver>();
77+
var persistence = services.GetRequiredService<INamsPersistenceService>();
78+
var recall = services.GetRequiredService<INamsRecallService>();
79+
80+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
81+
var entityName = $"Acme-{Guid.NewGuid():N}".Substring(0, 12);
82+
83+
await persistence.PersistTurnAsync(
84+
conversation.NamsConversationId,
85+
userMessages: [new NamsMessageToPersist($"John works at {entityName} Corp in Denver.")],
86+
assistantMessages: [],
87+
CancellationToken.None);
88+
89+
// Entity extraction runs asynchronously server-side and can lag well behind ingestion.
90+
var found = await PollUntilAsync(
91+
async () =>
92+
{
93+
var recalled = await recall.RecallAsync(conversation.NamsConversationId, entityName, CancellationToken.None);
94+
return recalled.Items.Any(i => i.Content.Contains(entityName, StringComparison.Ordinal));
95+
},
96+
timeout: TimeSpan.FromSeconds(60));
97+
98+
found.Should().BeTrue($"an entity named '{entityName}' should eventually be extracted and recallable");
99+
}
100+
101+
private static NamsConversationIdentity UniqueIdentity([CallerMemberName] string testName = "") => new()
102+
{
103+
ApplicationId = "agent-memory-dotnet-live-tests",
104+
UserId = $"test-{testName}-{Guid.NewGuid():N}",
105+
SessionId = Guid.NewGuid().ToString("N"),
106+
LocalConversationId = Guid.NewGuid().ToString("N")
107+
};
108+
109+
/// <summary>
110+
/// Bounded poll helper -- never waits longer than <paramref name="timeout"/>. Swallows exceptions from
111+
/// <paramref name="condition"/> itself (a live HTTP call can hit a transient blip mid-poll) and keeps
112+
/// retrying within the remaining budget, matching <c>Neo4jIntegrationFixture.WaitForVectorIndexesAsync</c>'s
113+
/// established pattern for this repo's other eventual-consistency polls.
114+
/// </summary>
115+
private static async Task<bool> PollUntilAsync(Func<Task<bool>> condition, TimeSpan timeout)
116+
{
117+
using var cts = new CancellationTokenSource(timeout);
118+
while (!cts.IsCancellationRequested)
119+
{
120+
try
121+
{
122+
if (await condition())
123+
return true;
124+
}
125+
catch { /* transient failure against a live external service -- ignore and keep polling */ }
126+
127+
try
128+
{
129+
await Task.Delay(TimeSpan.FromSeconds(2), cts.Token);
130+
}
131+
catch (OperationCanceledException)
132+
{
133+
return false;
134+
}
135+
}
136+
return false;
137+
}
138+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
namespace AgentMemory.Tests.Integration.Nams;
2+
3+
/// <summary>
4+
/// Reads live NAMS SaaS credentials for the isolated dev/test workspace from the environment. On Windows,
5+
/// falls back to a User-scope read when the process environment block doesn't have the variable --
6+
/// <see cref="Environment.GetEnvironmentVariable(string)"/> only sees what existed when the current process
7+
/// started, so a credential set after the IDE/terminal launched would otherwise require a full restart to
8+
/// become visible here.
9+
/// </summary>
10+
internal static class NamsLiveCredentials
11+
{
12+
public static string? ApiKey => Read("NAMS_API_KEY");
13+
14+
public static string? WorkspaceId => Read("NAMS_DEV_WORKSPACE_ID");
15+
16+
/// <summary>
17+
/// <see langword="true"/> only when both credentials are present. Tests gate on this via
18+
/// <see cref="LiveNamsFactAttribute"/> so they skip cleanly instead of failing on machines/CI runs that
19+
/// (correctly) have no live NAMS access -- these must never run against a shared/production workspace.
20+
/// </summary>
21+
public static bool IsAvailable => !string.IsNullOrWhiteSpace(ApiKey) && !string.IsNullOrWhiteSpace(WorkspaceId);
22+
23+
private static string? Read(string name)
24+
{
25+
var value = Environment.GetEnvironmentVariable(name);
26+
if (!string.IsNullOrWhiteSpace(value)) return value;
27+
return OperatingSystem.IsWindows() ? Environment.GetEnvironmentVariable(name, EnvironmentVariableTarget.User) : null;
28+
}
29+
}

0 commit comments

Comments
 (0)