From 0241970e3c7635038f21a59d8becaaaac1124c94 Mon Sep 17 00:00:00 2001 From: vinodvx <8429554+vinodvx@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:08:25 -0700 Subject: [PATCH] refactor: unify conversation constructors as NewConversation with deprecated aliases --- .gitignore | 2 ++ README.md | 6 ++--- cmd/main.go | 2 +- docs/advanced/worker-separation.mdx | 2 +- docs/features/conversation.mdx | 2 +- docs/getting-started/streaming.mdx | 2 +- examples/agent_with_conversation/main.go | 2 +- .../agent_with_stream_conversation/main.go | 2 +- pkg/agent/config.go | 4 ++-- pkg/conversation/config.go | 2 +- pkg/conversation/config_test.go | 8 +++---- pkg/conversation/inmem/conversation.go | 10 +++++++-- pkg/conversation/inmem/conversation_test.go | 22 +++++++++---------- pkg/conversation/redis/conversation.go | 9 ++++++-- pkg/conversation/redis/conversation_test.go | 18 +++++++-------- 15 files changed, 53 insertions(+), 40 deletions(-) diff --git a/.gitignore b/.gitignore index f69f2b3..eeb4a91 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,8 @@ pnpm-debug.log* .DS_Store +.a2a-server.pid + # Python / DeepEval (eval-harness/deepeval) eval-harness/deepeval/.venv/ eval-harness/deepeval/.pytest_cache/ diff --git a/README.md b/README.md index 315308f..e5f10ea 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,7 @@ import ( "github.com/agenticenv/agent-sdk-go/pkg/conversation/inmem" ) -conv := inmem.NewInMemoryConversation(inmem.WithMaxSize(100)) +conv := inmem.NewConversation(inmem.WithMaxSize(100)) a, _ := agent.NewAgent( agent.WithTemporalConfig(...), agent.WithLLMClient(...), @@ -415,13 +415,13 @@ a.Run(ctx, "What's my name?", opts) **Remote workers** — configure Redis on both processes; set `SaveOnIteration: true` on the worker when live updates are needed: ```go -convW, _ := redis.NewRedisConversation(redis.WithAddr("localhost:6379")) +convW, _ := redis.NewConversation(redis.WithAddr("localhost:6379")) w, _ := agent.NewAgentWorker( agent.WithTemporalConfig(...), agent.WithConversation(conversation.Config{Conversation: convW, Size: 20, SaveOnIteration: true}), ) -convA, _ := redis.NewRedisConversation(redis.WithAddr("localhost:6379")) +convA, _ := redis.NewConversation(redis.WithAddr("localhost:6379")) a, _ := agent.NewAgent( agent.WithTemporalConfig(...), agent.DisableLocalWorker(), diff --git a/cmd/main.go b/cmd/main.go index a274803..65ee488 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -75,7 +75,7 @@ func main() { log.Fatalf("mcp config: %v", err) } - conv := inmem.NewInMemoryConversation(inmem.WithMaxSize(100)) + conv := inmem.NewConversation(inmem.WithMaxSize(100)) // Single stdin reader: avoids conflict between main loop and approval handler after timeout lineCh := make(chan string) diff --git a/docs/advanced/worker-separation.mdx b/docs/advanced/worker-separation.mdx index 3e163c5..e06cc97 100644 --- a/docs/advanced/worker-separation.mdx +++ b/docs/advanced/worker-separation.mdx @@ -121,7 +121,7 @@ This starts Temporal's remote event path for out-of-process event delivery. In-memory conversation **fails at build time** with remote workers. Use Redis or another distributed backend on both processes: ```go -conv, _ := redis.NewRedisConversation(redis.WithAddr("localhost:6379")) +conv, _ := redis.NewConversation(redis.WithAddr("localhost:6379")) convCfg := conversation.Config{Conversation: conv, Size: 20, SaveOnIteration: true} w, _ := agent.NewAgentWorker(/* ... */, agent.WithConversation(convCfg)) diff --git a/docs/features/conversation.mdx b/docs/features/conversation.mdx index 151e6b8..26e24c5 100644 --- a/docs/features/conversation.mdx +++ b/docs/features/conversation.mdx @@ -20,7 +20,7 @@ import ( "github.com/agenticenv/agent-sdk-go/pkg/conversation/inmem" ) -conv := inmem.NewInMemoryConversation(inmem.WithMaxSize(100)) +conv := inmem.NewConversation(inmem.WithMaxSize(100)) a, err := agent.NewAgent( agent.WithLLMClient(llmClient), diff --git a/docs/getting-started/streaming.mdx b/docs/getting-started/streaming.mdx index b5bbafd..93a5202 100644 --- a/docs/getting-started/streaming.mdx +++ b/docs/getting-started/streaming.mdx @@ -167,7 +167,7 @@ Handle tool approvals with [`WithApprovalHandler`](/features/approvals) — the Pass `ConversationOptions` to share history across turns while streaming. The same session ID links messages across calls: ```go -conv := inmem.NewInMemoryConversation(inmem.WithMaxSize(100)) +conv := inmem.NewConversation(inmem.WithMaxSize(100)) a, err := agent.NewAgent( agent.WithLLMClient(llmClient), diff --git a/examples/agent_with_conversation/main.go b/examples/agent_with_conversation/main.go index 6899f50..fe189c2 100644 --- a/examples/agent_with_conversation/main.go +++ b/examples/agent_with_conversation/main.go @@ -26,7 +26,7 @@ func main() { } // Redis conversation (start with task infra:redis:up or docker compose redis service). - conv, err := redis.NewRedisConversation( + conv, err := redis.NewConversation( redis.WithAddr(redisAddrFromEnv()), redis.WithMaxSize(100), ) diff --git a/examples/agent_with_stream_conversation/main.go b/examples/agent_with_stream_conversation/main.go index d122eb0..6ec4058 100644 --- a/examples/agent_with_stream_conversation/main.go +++ b/examples/agent_with_stream_conversation/main.go @@ -27,7 +27,7 @@ func main() { log.Fatalf("failed to create LLM client: %v", err) } - conv := inmem.NewInMemoryConversation(inmem.WithMaxSize(100)) + conv := inmem.NewConversation(inmem.WithMaxSize(100)) reg := agent.NewToolRegistry() if err := agent.RegisterTools(reg, diff --git a/pkg/agent/config.go b/pkg/agent/config.go index d65177f..f2bd83e 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -452,8 +452,8 @@ func WithDisableFingerprintCheck(disable bool) Option { // Pass [conversation.DefaultConfig] for SDK defaults or a custom [conversation.Config]. // // Choose implementation based on deployment: -// - Single process: use inmem.NewInMemoryConversation -// - Remote workers: use redis.NewRedisConversation (in-memory cannot be used across processes) +// - Single process: use inmem.NewConversation +// - Remote workers: use redis.NewConversation (in-memory cannot be used across processes) // // The application owns Clear on the conversation when required; the agent runtime does not call Clear. // Agent and worker must use the same conversation config and ID when using remote workers. diff --git a/pkg/conversation/config.go b/pkg/conversation/config.go index 2e2b4ac..d8b63ea 100644 --- a/pkg/conversation/config.go +++ b/pkg/conversation/config.go @@ -38,7 +38,7 @@ func (c Config) Validate() error { // ValidateDistributed returns an error when conv is not distributed but remote workers require it. func ValidateDistributed(conv interfaces.Conversation, remoteWorkers bool) error { if remoteWorkers && conv != nil && !conv.IsDistributed() { - return errors.New("in-memory conversation cannot be used with remote workers: use distributed storage such as redis.NewRedisConversation") + return errors.New("in-memory conversation cannot be used with remote workers: use distributed storage such as redis.NewConversation") } return nil } diff --git a/pkg/conversation/config_test.go b/pkg/conversation/config_test.go index 6440a20..257e19e 100644 --- a/pkg/conversation/config_test.go +++ b/pkg/conversation/config_test.go @@ -10,7 +10,7 @@ import ( ) func TestDefaultConfig(t *testing.T) { - conv := inmem.NewInMemoryConversation() + conv := inmem.NewConversation() cfg := conversation.DefaultConfig(conv) if err := cfg.Validate(); err != nil { t.Fatal(err) @@ -21,7 +21,7 @@ func TestDefaultConfig(t *testing.T) { } func TestConfig_WithDefaults(t *testing.T) { - cfg := (conversation.Config{Conversation: inmem.NewInMemoryConversation()}).WithDefaults() + cfg := (conversation.Config{Conversation: inmem.NewConversation()}).WithDefaults() if cfg.Size != conversation.DefaultSize { t.Fatalf("size = %d", cfg.Size) } @@ -34,7 +34,7 @@ func TestConfig_Validate_missingConversation(t *testing.T) { } func TestConfig_ListOptions(t *testing.T) { - cfg := conversation.DefaultConfig(inmem.NewInMemoryConversation()) + cfg := conversation.DefaultConfig(inmem.NewConversation()) opts := cfg.ListOptions() if len(opts) != 1 { t.Fatalf("opts len = %d", len(opts)) @@ -42,7 +42,7 @@ func TestConfig_ListOptions(t *testing.T) { } func TestValidateDistributed_inmemRemoteWorkers(t *testing.T) { - conv := inmem.NewInMemoryConversation() + conv := inmem.NewConversation() if err := conversation.ValidateDistributed(conv, true); err == nil { t.Fatal("expected distributed error") } diff --git a/pkg/conversation/inmem/conversation.go b/pkg/conversation/inmem/conversation.go index 8951ca9..83992ca 100644 --- a/pkg/conversation/inmem/conversation.go +++ b/pkg/conversation/inmem/conversation.go @@ -15,12 +15,13 @@ type InMemoryConversation struct { mu sync.RWMutex } -func NewInMemoryConversation(options ...Option) *InMemoryConversation { +// NewConversation creates a new in-memory conversation store. +func NewConversation(opts ...Option) *InMemoryConversation { c := &InMemoryConversation{ messages: make(map[string][]interfaces.Message), maxSize: 100, } - for _, option := range options { + for _, option := range opts { option(c) } if c.maxSize <= 0 { @@ -29,6 +30,11 @@ func NewInMemoryConversation(options ...Option) *InMemoryConversation { return c } +// Deprecated: use NewConversation instead. +func NewInMemoryConversation(opts ...Option) *InMemoryConversation { + return NewConversation(opts...) +} + type Option func(*InMemoryConversation) // WithMaxSize sets the maximum number of messages to store per conversation. diff --git a/pkg/conversation/inmem/conversation_test.go b/pkg/conversation/inmem/conversation_test.go index b8fdacb..3cff8d6 100644 --- a/pkg/conversation/inmem/conversation_test.go +++ b/pkg/conversation/inmem/conversation_test.go @@ -8,25 +8,25 @@ import ( ) func TestInMemoryConversation_IsDistributed(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() if c.IsDistributed() { t.Error("in-memory should not be distributed") } } -func TestNewInMemoryConversation_MaxSizeDefault(t *testing.T) { - c := NewInMemoryConversation(WithMaxSize(0)) +func TestNewConversation_MaxSizeDefault(t *testing.T) { + c := NewConversation(WithMaxSize(0)) if c.maxSize != 100 { t.Fatalf("maxSize = %d, want 100", c.maxSize) } - c2 := NewInMemoryConversation(WithMaxSize(-1)) + c2 := NewConversation(WithMaxSize(-1)) if c2.maxSize != 100 { t.Fatalf("maxSize = %d, want 100", c2.maxSize) } } func TestInMemoryConversation_AddMessage_EmptyID(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() ctx := context.Background() if err := c.AddMessage(ctx, "", interfaces.Message{Role: interfaces.MessageRoleUser, Content: "x"}); err != nil { t.Fatal(err) @@ -41,7 +41,7 @@ func TestInMemoryConversation_AddMessage_EmptyID(t *testing.T) { } func TestInMemoryConversation_ListMessages_EmptyID(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() msgs, err := c.ListMessages(context.Background(), "") if err != nil { t.Fatal(err) @@ -52,7 +52,7 @@ func TestInMemoryConversation_ListMessages_EmptyID(t *testing.T) { } func TestInMemoryConversation_TrimToMaxSize(t *testing.T) { - c := NewInMemoryConversation(WithMaxSize(3)) + c := NewConversation(WithMaxSize(3)) ctx := context.Background() id := "conv-1" for i := 0; i < 5; i++ { @@ -73,7 +73,7 @@ func TestInMemoryConversation_TrimToMaxSize(t *testing.T) { } func TestInMemoryConversation_ListMessages_LimitOffset(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() ctx := context.Background() id := "c" for _, ch := range []string{"a", "b", "c", "d", "e"} { @@ -100,7 +100,7 @@ func TestInMemoryConversation_ListMessages_LimitOffset(t *testing.T) { } func TestInMemoryConversation_ListMessages_RoleFilter(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() ctx := context.Background() id := "c" if err := c.AddMessage(ctx, id, interfaces.Message{Role: interfaces.MessageRoleSystem, Content: "sys"}); err != nil { @@ -119,7 +119,7 @@ func TestInMemoryConversation_ListMessages_RoleFilter(t *testing.T) { } func TestInMemoryConversation_Clear(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() ctx := context.Background() id := "x" if err := c.AddMessage(ctx, id, interfaces.Message{Role: interfaces.MessageRoleUser, Content: "1"}); err != nil { @@ -138,7 +138,7 @@ func TestInMemoryConversation_Clear(t *testing.T) { } func TestInMemoryConversation_Clear_EmptyID(t *testing.T) { - c := NewInMemoryConversation() + c := NewConversation() if err := c.Clear(context.Background(), ""); err != nil { t.Fatal(err) } diff --git a/pkg/conversation/redis/conversation.go b/pkg/conversation/redis/conversation.go index f7a1774..d706c7e 100644 --- a/pkg/conversation/redis/conversation.go +++ b/pkg/conversation/redis/conversation.go @@ -75,10 +75,10 @@ func (c *RedisConversation) getKey(id string) string { return fmt.Sprintf("%s:%s:messages", p, id) } -// NewRedisConversation creates a Redis-backed conversation from options. +// NewConversation creates a Redis-backed conversation from options. // Use WithClient to provide your own client; otherwise addr is required to create a client. // Call Close() when done if you did not use WithClient. -func NewRedisConversation(opts ...Option) (*RedisConversation, error) { +func NewConversation(opts ...Option) (*RedisConversation, error) { c := &RedisConversation{maxSize: 100} for _, opt := range opts { opt(c) @@ -106,6 +106,11 @@ func NewRedisConversation(opts ...Option) (*RedisConversation, error) { return c, nil } +// Deprecated: use NewConversation instead. +func NewRedisConversation(opts ...Option) (*RedisConversation, error) { + return NewConversation(opts...) +} + // Close releases the Redis connection only when we own it (not using WithClient). func (c *RedisConversation) Close() error { if c.ownClient && c.client != nil { diff --git a/pkg/conversation/redis/conversation_test.go b/pkg/conversation/redis/conversation_test.go index 6c2dc48..f0949a0 100644 --- a/pkg/conversation/redis/conversation_test.go +++ b/pkg/conversation/redis/conversation_test.go @@ -14,7 +14,7 @@ func newTestConversation(t *testing.T, opts ...Option) (*RedisConversation, *min t.Helper() s := miniredis.RunT(t) base := []Option{WithAddr(s.Addr())} - c, err := NewRedisConversation(append(base, opts...)...) + c, err := NewConversation(append(base, opts...)...) if err != nil { t.Fatal(err) } @@ -22,19 +22,19 @@ func newTestConversation(t *testing.T, opts ...Option) (*RedisConversation, *min return c, s } -func TestNewRedisConversation_RequiresAddr(t *testing.T) { - _, err := NewRedisConversation() +func TestNewConversation_RequiresAddr(t *testing.T) { + _, err := NewConversation() if err == nil || err.Error() != "addr is required when not using WithClient" { t.Fatalf("got %v", err) } } -func TestNewRedisConversation_WithClient_CloseDoesNotOwn(t *testing.T) { +func TestNewConversation_WithClient_CloseDoesNotOwn(t *testing.T) { s := miniredis.RunT(t) rdb := goredis.NewClient(&goredis.Options{Addr: s.Addr()}) t.Cleanup(func() { _ = rdb.Close() }) - c, err := NewRedisConversation(WithClient(rdb)) + c, err := NewConversation(WithClient(rdb)) if err != nil { t.Fatal(err) } @@ -170,7 +170,7 @@ func TestRedisConversation_WithKeyPrefix(t *testing.T) { s := miniredis.RunT(t) rdb := goredis.NewClient(&goredis.Options{Addr: s.Addr()}) t.Cleanup(func() { _ = rdb.Close() }) - c, err := NewRedisConversation(WithAddr(s.Addr()), WithKeyPrefix("myapp")) + c, err := NewConversation(WithAddr(s.Addr()), WithKeyPrefix("myapp")) if err != nil { t.Fatal(err) } @@ -194,7 +194,7 @@ func TestRedisConversation_ListMessages_UnmarshalError(t *testing.T) { s := miniredis.RunT(t) rdb := goredis.NewClient(&goredis.Options{Addr: s.Addr()}) t.Cleanup(func() { _ = rdb.Close() }) - c, err := NewRedisConversation(WithClient(rdb)) + c, err := NewConversation(WithClient(rdb)) if err != nil { t.Fatal(err) } @@ -212,7 +212,7 @@ func TestRedisConversation_ListMessages_UnmarshalError(t *testing.T) { func TestRedisConversation_WithMaxSizeZeroUsesDefault(t *testing.T) { s := miniredis.RunT(t) - c, err := NewRedisConversation(WithAddr(s.Addr()), WithMaxSize(0)) + c, err := NewConversation(WithAddr(s.Addr()), WithMaxSize(0)) if err != nil { t.Fatal(err) } @@ -233,7 +233,7 @@ func TestRoleIn(t *testing.T) { func TestRedisConversation_TTLExpiresKey(t *testing.T) { s := miniredis.RunT(t) - c, err := NewRedisConversation(WithAddr(s.Addr()), WithTTL(10*time.Minute)) + c, err := NewConversation(WithAddr(s.Addr()), WithTTL(10*time.Minute)) if err != nil { t.Fatal(err) }