diff --git a/agent/common_context.go b/agent/common_context.go index 7bd19e3c9..154bcad2b 100644 --- a/agent/common_context.go +++ b/agent/common_context.go @@ -94,7 +94,7 @@ func NewCallbackContextWithArtifactTracking(ic InvocationContext, actions *sessi Context: ic, invocationContext: ic, actions: actions, - artifacts: &trackedArtifacts{Artifacts: ic.Artifacts(), actions: actions}, + artifacts: newTrackedArtifacts(ic.Artifacts(), actions), } // wrap the commonContext in order to log information about someone using tool-context methods on a callback context wrapper := &callbackContextWrapper{ @@ -132,7 +132,7 @@ func NewToolContext(ic InvocationContext, functionCallID string, actions *sessio res.actions = actions res.functionCallID = functionCallID res.toolConfirmation = confirmation - res.artifacts = &trackedArtifacts{Artifacts: ic.Artifacts(), actions: actions} + res.artifacts = newTrackedArtifacts(ic.Artifacts(), actions) wrapper := &toolContextWrapper{ context: &res, @@ -461,6 +461,17 @@ func (c *callbackContextState) All() iter.Seq2[string, any] { return c.ctx.invocationContext.Session().State().All() } +// newTrackedArtifacts wraps inner so that each successful Save is recorded +// into the supplied EventActions.ArtifactDelta. It returns nil when inner is +// nil so that "no artifact service configured" stays observable (a nil +// Artifacts) instead of panicking on the first promoted method call. +func newTrackedArtifacts(inner Artifacts, actions *session.EventActions) Artifacts { + if inner == nil { + return nil + } + return &trackedArtifacts{Artifacts: inner, actions: actions} +} + // trackedArtifacts wraps an Artifacts to record each successful Save into the // supplied EventActions.ArtifactDelta. type trackedArtifacts struct { diff --git a/tool/loadartifactstool/load_artifacts_tool.go b/tool/loadartifactstool/load_artifacts_tool.go index cfb0fa0c7..223f2b34d 100644 --- a/tool/loadartifactstool/load_artifacts_tool.go +++ b/tool/loadartifactstool/load_artifacts_tool.go @@ -118,6 +118,9 @@ func (t *artifactsTool) Run(ctx agent.Context, args any) (map[string]any, error) // ProcessRequest processes the LLM request. It packs the tool, appends initial // instructions, and processes any load artifacts function calls. func (t *artifactsTool) ProcessRequest(ctx agent.Context, req *model.LLMRequest) error { + if ctx.Artifacts() == nil { + return fmt.Errorf("load_artifacts tool requires an artifact service to be configured") + } if err := toolutils.PackTool(req, t); err != nil { return err } diff --git a/tool/loadartifactstool/load_artifacts_tool_test.go b/tool/loadartifactstool/load_artifacts_tool_test.go index f60778e15..22e98264c 100644 --- a/tool/loadartifactstool/load_artifacts_tool_test.go +++ b/tool/loadartifactstool/load_artifacts_tool_test.go @@ -339,6 +339,75 @@ func TestLoadArtifactsTool_ProcessRequest_Artifacts_OtherFunctionCall(t *testing } } +// TestLoadArtifactsTool_ProcessRequest_NilArtifacts verifies that ProcessRequest +// returns a descriptive error instead of panicking when no artifact service is +// configured (see https://github.com/google/adk-go/issues/283). +func TestLoadArtifactsTool_ProcessRequest_NilArtifacts(t *testing.T) { + loadArtifactsTool := loadartifactstool.New() + + // Construct a context with no artifact service configured. + invocationCtx := icontext.NewInvocationContext(t.Context(), icontext.InvocationContextParams{}) + tc := agent.NewToolContext(invocationCtx, "", nil, nil) + + requestProcessor, ok := loadArtifactsTool.(toolinternal.RequestProcessor) + if !ok { + t.Fatal("loadArtifactsTool does not implement RequestProcessor") + } + + llmRequest := &model.LLMRequest{} + err := requestProcessor.ProcessRequest(tc, llmRequest) + if err == nil { + t.Fatal("ProcessRequest should return an error when no artifact service is configured, but got nil") + } + if !strings.Contains(err.Error(), "artifact service") { + t.Errorf("error should mention the missing artifact service, got: %v", err) + } +} + +// TestLoadArtifactsTool_ProcessRequest_NilArtifacts_WithFunctionCall verifies +// that the nil-artifact guard in ProcessRequest also covers the +// processLoadArtifactsFunctionCall path, not only appendInitialInstructions: +// with a request whose last content is a load_artifacts function response, +// ProcessRequest must still return the descriptive error instead of panicking +// on the unguarded ctx.Artifacts() read in that path. +func TestLoadArtifactsTool_ProcessRequest_NilArtifacts_WithFunctionCall(t *testing.T) { + loadArtifactsTool := loadartifactstool.New() + + // Construct a context with no artifact service configured. + invocationCtx := icontext.NewInvocationContext(t.Context(), icontext.InvocationContextParams{}) + tc := agent.NewToolContext(invocationCtx, "", nil, nil) + + functionResponse := &genai.FunctionResponse{ + Name: "load_artifacts", + Response: map[string]any{ + "artifact_names": []any{"doc1.txt"}, + }, + } + llmRequest := &model.LLMRequest{ + Contents: []*genai.Content{ + { + Role: "model", + Parts: []*genai.Part{ + genai.NewPartFromFunctionResponse(functionResponse.Name, functionResponse.Response), + }, + }, + }, + } + + requestProcessor, ok := loadArtifactsTool.(toolinternal.RequestProcessor) + if !ok { + t.Fatal("loadArtifactsTool does not implement RequestProcessor") + } + + err := requestProcessor.ProcessRequest(tc, llmRequest) + if err == nil { + t.Fatal("ProcessRequest should return an error when no artifact service is configured, but got nil") + } + if !strings.Contains(err.Error(), "artifact service") { + t.Errorf("error should mention the missing artifact service, got: %v", err) + } +} + func createToolContext(t *testing.T) agent.Context { t.Helper()