Skip to content

Add EnsureStoredMemoriesDeleted to Foundry MemoryProvider to expose scope deletion - #619

Merged
George Adams (gdams) merged 2 commits into
microsoft:mainfrom
PratikDhanaveFork:foundry-ensure-stored-memories-deleted
Jul 29, 2026
Merged

Add EnsureStoredMemoriesDeleted to Foundry MemoryProvider to expose scope deletion#619
George Adams (gdams) merged 2 commits into
microsoft:mainfrom
PratikDhanaveFork:foundry-ensure-stored-memories-deleted

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

What

Adds an exported EnsureStoredMemoriesDeleted(ctx, session) method to the Foundry MemoryProvider. It resolves the session scope with the provider's scope callback and calls MemoryStoresClient.DeleteScope to delete all memories stored for that scope in the memory store. A 404 (no memories exist for the scope) is treated as success; any other error is returned to the caller. Success and no-op cases are logged at info, consistent with the existing provide/store operations.

Why

MemoryStoresClient.DeleteScope lives in the internal azaiprojects package and cannot be reached by external callers. The provider previously exposed only Invoking/Invoked, so there was no way to clear a scope's stored memories through the public API.

This mirrors the memory-provider surface in the .NET and Python Agent Framework, where a memory provider exposes an explicit "delete stored memories for this scope" operation (e.g. the EnsureStoredMemoriesDeletedAsync/thread-deletion hooks) so applications can honor data-deletion requests per user/tenant scope. Adding it keeps the Go provider aligned with the other SDKs.

Tests

Added tests in memory_test.go driving the method through the existing fake MemoryStoresClient transport:

  • successful DeleteScope returns nil and issues POST /memory_stores/memory:delete_scope with the resolved scope and Foundry-Features header;
  • a 404 response is swallowed and returns nil;
  • a 500 response is surfaced as an *azcore.ResponseError;
  • a blank scope panics via the provider's scope contract.

go build ./..., go vet ./provider/foundryprovider/..., and go test ./provider/foundryprovider/... all pass.

Copilot AI review requested due to automatic review settings July 23, 2026 05:43
@PratikDhanave
PratikDhanave (PratikDhanave) requested a review from a team as a code owner July 23, 2026 05:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a public deletion API to the Foundry MemoryProvider so callers can delete all stored memories for a resolved session scope (idempotent behavior: 404 is treated as success), aligning the Go SDK’s memory-provider surface with other Agent Framework SDKs.

Changes:

  • Export (*MemoryProvider).EnsureStoredMemoriesDeleted(ctx, session) to resolve scope and call MemoryStoresClient.DeleteScope, swallowing 404s.
  • Add unit tests covering success, 404-as-success, 500 surfacing as *azcore.ResponseError, and empty-scope panic behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
provider/foundryprovider/memory.go Adds EnsureStoredMemoriesDeleted and handles 404 as a no-op, logging success/failure outcomes.
provider/foundryprovider/memory_test.go Adds tests for the new deletion method across success and error scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +308 to +319
func TestMemoryProviderEnsureStoredMemoriesDeletedTreatsNotFoundAsSuccess(t *testing.T) {
transport := &recordingTransport{handle: func(req *http.Request, _ string) (*http.Response, error) {
return jsonResponse(req, http.StatusNotFound, `{"error":{"code":"NotFound","message":"scope not found"}}`), nil
}}
provider := foundryprovider.NewMemoryProvider(validEndpoint, validCredential, "memory", validScope, foundryprovider.MemoryProviderConfig{
ClientOptions: azcore.ClientOptions{Transport: transport},
})

if err := provider.EnsureStoredMemoriesDeleted(t.Context(), nil); err != nil {
t.Fatalf("EnsureStoredMemoriesDeleted error = %v", err)
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in edd6593: the not-found test now asserts the DeleteScope request was actually issued (POST /memory_stores/memory:delete_scope, Foundry-Features header, and scope body), matching the successful-delete test.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the parity-approved Go API consistency review found no parity issues label Jul 23, 2026
@PratikDhanave
PratikDhanave (PratikDhanave) force-pushed the foundry-ensure-stored-memories-deleted branch from 4ac611f to c9cdf74 Compare July 24, 2026 01:41
@github-actions github-actions Bot added the public-api-change Pull Request changes public APIs label Jul 24, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Expose scope deletion on the Foundry MemoryProvider so external callers
can clear all stored memories for a session's scope. The underlying
MemoryStoresClient.DeleteScope lives in the internal azaiprojects package
and is otherwise unreachable outside the provider. A 404 response (no
memories for the scope) is treated as success; other errors are returned.
@PratikDhanave
PratikDhanave (PratikDhanave) force-pushed the foundry-ensure-stored-memories-deleted branch from edd6593 to 1c181e9 Compare July 24, 2026 09:36
@github-actions

Copy link
Copy Markdown
Contributor

Parity Review — EnsureStoredMemoriesDeleted

Summary

This PR adds EnsureStoredMemoriesDeleted(ctx context.Context, session *agent.Session) error to the Foundry MemoryProvider. The change is semantically aligned with the upstream .NET implementation and the public-api-change + parity-approved labels are both appropriate.

Cross-repo comparison

SDK Equivalent Notes
.NET FoundryMemoryProvider.EnsureStoredMemoriesDeletedAsync(AgentSession, CancellationToken) ✅ Present in dotnet/src/Microsoft.Agents.AI.Foundry/Memory/FoundryMemoryProvider.cs. Same behavior: calls DeleteScope, swallows 404, surfaces other errors.
Python ⚠️ No ensure_stored_memories_deleted equivalent was found in python/packages/foundry/agent_framework_foundry/_memory_provider.py. The PR description cites Python parity but the Python SDK has not yet added this method. This is not a blocker for this Go PR, but a Python follow-up may be worth tracking.

Minor divergences (non-blocking)

  • 404 log level: .NET logs the "no memories to delete" path at Debug; Go logs it at Info. Neither is wrong, but they differ slightly from upstream.
  • Nil session guard: .NET explicitly throws ArgumentNullException for a null session. Go passes nil through to the scope callback which panics on empty scope. This is consistent with the existing Go contract documented on NewMemoryProvider, and the test covers this case.

Verdict

The Go implementation faithfully mirrors the .NET EnsureStoredMemoriesDeletedAsync behavior. Parity with the .NET SDK is confirmed. The parity-approved label is correct.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent · 62.2 AIC · ⌖ 5.65 AIC · ⊞ 5.9K ·

@gdams
George Adams (gdams) added this pull request to the merge queue Jul 29, 2026
Merged via the queue into microsoft:main with commit ba00109 Jul 29, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants