From 72613aece461d4e8ac99fc26976b2d66e3838bca Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 10:01:48 -0400 Subject: [PATCH 1/2] Persist explicit session IDs for reuse in subsequent commands. Fixes #7602. --- .../azure.ai.agents/internal/cmd/helpers.go | 3 + .../internal/cmd/monitor_test.go | 70 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index 25c032f5251..c441d616735 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -139,6 +139,9 @@ func resolveStoredID( generateIfMissing bool, ) (string, error) { if explicit != "" { + // Persist the explicit ID so that subsequent commands (e.g. monitor, invoke) + // can find it without the user passing it again. + saveContextValue(ctx, azdClient, agentKey, explicit, storeField) return explicit, nil } if forceNew && !generateIfMissing { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go index 93e7d1d4418..d64db3dd9b7 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go @@ -364,6 +364,76 @@ func TestValidateMonitorFlags_ErrorMessages(t *testing.T) { assert.Contains(t, err.Error(), "badtype") } +func TestExplicitSessionID_PersistedAndReusable(t *testing.T) { + t.Parallel() + + // Simulate the persistence that resolveStoredID now performs when an explicit + // session ID is provided. This verifies the round-trip: save an explicit ID, + // then load it back (as monitor's resolveMonitorSession would). + dir := t.TempDir() + configPath := filepath.Join(dir, ConfigFile) + + agentName := "my-agent" + explicitSID := "user-provided-session-id" + + // Before: no sessions stored + agentCtx := loadLocalContext(configPath) + assert.Empty(t, agentCtx.Sessions) + + // Simulate what resolveStoredID now does: persist the explicit ID + store := contextMap(agentCtx, "sessions") + store[agentName] = explicitSID + require.NoError(t, saveLocalContext(agentCtx, configPath)) + + // Reload (as monitor or a subsequent invoke would) + loaded := loadLocalContext(configPath) + require.NotNil(t, loaded.Sessions) + assert.Equal(t, explicitSID, loaded.Sessions[agentName]) +} + +func TestExplicitConversationID_PersistedAndReusable(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, ConfigFile) + + agentName := "my-agent" + explicitConvID := "user-provided-conv-id" + + agentCtx := loadLocalContext(configPath) + store := contextMap(agentCtx, "conversations") + store[agentName] = explicitConvID + require.NoError(t, saveLocalContext(agentCtx, configPath)) + + loaded := loadLocalContext(configPath) + require.NotNil(t, loaded.Conversations) + assert.Equal(t, explicitConvID, loaded.Conversations[agentName]) +} + +func TestExplicitID_OverwritesPreviousValue(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, ConfigFile) + + agentName := "my-agent" + + // Save an initial session ID + agentCtx := loadLocalContext(configPath) + store := contextMap(agentCtx, "sessions") + store[agentName] = "old-session-id" + require.NoError(t, saveLocalContext(agentCtx, configPath)) + + // Simulate a second invoke with a different explicit session ID + agentCtx = loadLocalContext(configPath) + store = contextMap(agentCtx, "sessions") + store[agentName] = "new-session-id" + require.NoError(t, saveLocalContext(agentCtx, configPath)) + + loaded := loadLocalContext(configPath) + assert.Equal(t, "new-session-id", loaded.Sessions[agentName]) +} + func TestValidateMonitorFlags_SessionDoesNotAffectValidation(t *testing.T) { t.Parallel() From b018b7656363a77f57a27b74b24aed818dcc7e0a Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 12:49:02 -0400 Subject: [PATCH 2/2] Refactor session ID persistence logic and enhance test coverage for explicit session IDs --- .../azure.ai.agents/internal/cmd/helpers.go | 69 +++++++++++++++++-- .../internal/cmd/monitor_test.go | 41 +++++------ 2 files changed, 83 insertions(+), 27 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index c441d616735..4dd0c4f0a7a 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -9,6 +9,7 @@ import ( "encoding/json" "fmt" "io" + "log" "net/http" "os" "path/filepath" @@ -103,12 +104,21 @@ func saveContextValue( } configPath, err := resolveConfigPath(ctx, azdClient) if err != nil { + log.Printf("saveContextValue: failed to resolve config path: %v", err) return } + if err := saveContextValueToPath(configPath, agentKey, value, storeField); err != nil { + log.Printf("saveContextValue: failed to persist %s for %q: %v", storeField, agentKey, err) + } +} + +// saveContextValueToPath persists a value into the named field of the local config at configPath. +// This is the path-based implementation used by saveContextValue and directly in tests. +func saveContextValueToPath(configPath, agentKey, value, storeField string) error { agentCtx := loadLocalContext(configPath) store := contextMap(agentCtx, storeField) store[agentKey] = value - _ = saveLocalContext(agentCtx, configPath) + return saveLocalContext(agentCtx, configPath) } // resolveLocalAgentKey builds the storage key for local mode from the azd project config. @@ -141,7 +151,7 @@ func resolveStoredID( if explicit != "" { // Persist the explicit ID so that subsequent commands (e.g. monitor, invoke) // can find it without the user passing it again. - saveContextValue(ctx, azdClient, agentKey, explicit, storeField) + persistExplicitID(ctx, azdClient, agentKey, explicit, storeField) return explicit, nil } if forceNew && !generateIfMissing { @@ -176,7 +186,56 @@ func resolveStoredID( return newID, nil } -// contextMap returns the named map from AgentLocalContext, initializing it if nil. +// persistExplicitID saves a user-provided explicit ID to the local config. +// Errors are logged for debug visibility but do not block the caller. +func persistExplicitID( + ctx context.Context, + azdClient *azdext.AzdClient, + agentKey string, + value string, + storeField string, +) { + saveContextValue(ctx, azdClient, agentKey, value, storeField) +} + +// resolveStoredIDFromPath is a testable variant of resolveStoredID that operates on a +// config file path directly, removing the need for an azdClient. +func resolveStoredIDFromPath( + configPath string, + agentKey string, + explicit string, + forceNew bool, + storeField string, + generateIfMissing bool, +) (string, error) { + if explicit != "" { + if err := saveContextValueToPath(configPath, agentKey, explicit, storeField); err != nil { + log.Printf("resolveStoredIDFromPath: failed to persist explicit %s for %q: %v", storeField, agentKey, err) + } + return explicit, nil + } + if forceNew && !generateIfMissing { + return "", nil + } + + agentCtx := loadLocalContext(configPath) + store := contextMap(agentCtx, storeField) + if !forceNew { + if id, ok := store[agentKey]; ok { + return id, nil + } + } + + if !generateIfMissing { + return "", nil + } + + newID := uuid.NewString() + store[agentKey] = newID + _ = saveLocalContext(agentCtx, configPath) + + return newID, nil +} func contextMap(agentCtx *AgentLocalContext, field string) map[string]string { switch field { case "sessions": @@ -295,7 +354,7 @@ func fetchOpenAPISpec( } // resolveConversationID resolves a Foundry conversation ID. -// When explicit is provided, it is returned directly. +// When explicit is provided, it is persisted and returned directly. // If no conversation is found (or forceNew is true), it attempts to create one and persist it. // Returns empty string when conversation creation fails, allowing invoke to continue without multi-turn memory. func resolveConversationID( @@ -308,6 +367,8 @@ func resolveConversationID( bearerToken string, ) (string, error) { if explicit != "" { + // Persist the explicit conversation ID so subsequent commands can find it. + saveContextValue(ctx, azdClient, agentName, explicit, "conversations") return explicit, nil } configPath, err := resolveConfigPath(ctx, azdClient) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go index d64db3dd9b7..fed7778af41 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go @@ -367,9 +367,6 @@ func TestValidateMonitorFlags_ErrorMessages(t *testing.T) { func TestExplicitSessionID_PersistedAndReusable(t *testing.T) { t.Parallel() - // Simulate the persistence that resolveStoredID now performs when an explicit - // session ID is provided. This verifies the round-trip: save an explicit ID, - // then load it back (as monitor's resolveMonitorSession would). dir := t.TempDir() configPath := filepath.Join(dir, ConfigFile) @@ -380,12 +377,13 @@ func TestExplicitSessionID_PersistedAndReusable(t *testing.T) { agentCtx := loadLocalContext(configPath) assert.Empty(t, agentCtx.Sessions) - // Simulate what resolveStoredID now does: persist the explicit ID - store := contextMap(agentCtx, "sessions") - store[agentName] = explicitSID - require.NoError(t, saveLocalContext(agentCtx, configPath)) + // Call resolveStoredIDFromPath with an explicit ID — this is the code path + // that resolveStoredID follows when the user passes --session-id. + got, err := resolveStoredIDFromPath(configPath, agentName, explicitSID, false, "sessions", false) + require.NoError(t, err) + assert.Equal(t, explicitSID, got) - // Reload (as monitor or a subsequent invoke would) + // Verify the ID was persisted (as monitor's resolveMonitorSession would load it) loaded := loadLocalContext(configPath) require.NotNil(t, loaded.Sessions) assert.Equal(t, explicitSID, loaded.Sessions[agentName]) @@ -400,10 +398,9 @@ func TestExplicitConversationID_PersistedAndReusable(t *testing.T) { agentName := "my-agent" explicitConvID := "user-provided-conv-id" - agentCtx := loadLocalContext(configPath) - store := contextMap(agentCtx, "conversations") - store[agentName] = explicitConvID - require.NoError(t, saveLocalContext(agentCtx, configPath)) + got, err := resolveStoredIDFromPath(configPath, agentName, explicitConvID, false, "conversations", false) + require.NoError(t, err) + assert.Equal(t, explicitConvID, got) loaded := loadLocalContext(configPath) require.NotNil(t, loaded.Conversations) @@ -418,17 +415,15 @@ func TestExplicitID_OverwritesPreviousValue(t *testing.T) { agentName := "my-agent" - // Save an initial session ID - agentCtx := loadLocalContext(configPath) - store := contextMap(agentCtx, "sessions") - store[agentName] = "old-session-id" - require.NoError(t, saveLocalContext(agentCtx, configPath)) - - // Simulate a second invoke with a different explicit session ID - agentCtx = loadLocalContext(configPath) - store = contextMap(agentCtx, "sessions") - store[agentName] = "new-session-id" - require.NoError(t, saveLocalContext(agentCtx, configPath)) + // Persist an initial session ID via resolveStoredIDFromPath + got, err := resolveStoredIDFromPath(configPath, agentName, "old-session-id", false, "sessions", false) + require.NoError(t, err) + assert.Equal(t, "old-session-id", got) + + // Persist a different explicit session ID — should overwrite the previous one + got, err = resolveStoredIDFromPath(configPath, agentName, "new-session-id", false, "sessions", false) + require.NoError(t, err) + assert.Equal(t, "new-session-id", got) loaded := loadLocalContext(configPath) assert.Equal(t, "new-session-id", loaded.Sessions[agentName])