Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions agent/common_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions tool/loadartifactstool/load_artifacts_tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
69 changes: 69 additions & 0 deletions tool/loadartifactstool/load_artifacts_tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Loading