diff --git a/backend/pkg/providers/helpers.go b/backend/pkg/providers/helpers.go index e040b953..1445e575 100644 --- a/backend/pkg/providers/helpers.go +++ b/backend/pkg/providers/helpers.go @@ -44,6 +44,33 @@ type dummyMessage struct { Message string `json:"message"` } +// wrapToolCallIDTemplateError annotates an error returned by +// Provider.GetToolCallIDTemplate so the user sees an actionable hint when a +// known upstream limitation blocks provider initialization. Currently it +// covers the Ollama "model does not support tools" case (issue #280): the +// underlying error is returned by the Ollama API itself and surfaces five +// wraps deep, which previously left the user with a cryptic "failed to +// determine tool call ID template" message. This helper is shared by the +// flow and assistant provider paths, so the wording stays context-neutral. +// +// The function preserves the original error chain via %w so downstream code +// (logs, langfuse spans, errors.Is/As) keeps working. +func wrapToolCallIDTemplateError(err error) error { + if err == nil { + return nil + } + if strings.Contains(err.Error(), "does not support tools") { + return fmt.Errorf( + "failed to determine tool call ID template: the selected model "+ + "does not support tool/function calling, which PentAGI requires "+ + "for tool execution in flows and assistant sessions; select an "+ + "Ollama model whose metadata advertises tool/function calling "+ + "support and update the provider configuration: %w", + err) + } + return fmt.Errorf("failed to determine tool call ID template: %w", err) +} + type reflectorRetryContextKey struct{} // isReflectorRetry checks if we are already in a reflector retry cycle diff --git a/backend/pkg/providers/helpers_test.go b/backend/pkg/providers/helpers_test.go index 1b5e201a..1b4099b8 100644 --- a/backend/pkg/providers/helpers_test.go +++ b/backend/pkg/providers/helpers_test.go @@ -2,7 +2,10 @@ package providers import ( "encoding/json" + "errors" + "fmt" "slices" + "strings" "sync" "sync/atomic" "testing" @@ -1174,3 +1177,99 @@ func TestExecutionMonitorDetector_TotalCallsSequence(t *testing.T) { func mockToolCall(name string) llms.ToolCall { return llms.ToolCall{FunctionCall: &llms.FunctionCall{Name: name}} } + +func TestWrapToolCallIDTemplateError(t *testing.T) { + t.Parallel() + + // Mirror the real upstream wrap chain so the test traps regressions in + // substring detection if the inner wraps ever change. + ollamaErr := fmt.Errorf( + "all sample collection attempts failed: %w", + fmt.Errorf("failed to call LLM: %w", + errors.New("400 Bad Request: registry.ollama.ai/library/gemma3:27b-it-q4_K_M does not support tools")), + ) + genericErr := errors.New("connection refused") + + cases := []struct { + name string + input error + wantNil bool + wantContains []string + wantUnwrapsTo error + wantNotContain []string + }{ + { + name: "nil error passes through", + input: nil, + wantNil: true, + }, + { + name: "ollama tools error gets actionable hint", + input: ollamaErr, + wantContains: []string{ + "failed to determine tool call ID template", + "does not support tool/function calling", + "PentAGI requires", + "flows and assistant sessions", + "metadata advertises tool/function calling", + "does not support tools", + }, + wantNotContain: []string{ + // The hint must not name specific model tags as + // tool-capable, because Ollama model capabilities are + // not verified by PentAGI and any concrete list ages + // poorly. + "llama3.1", + "qwen2.5", + "mistral-nemo", + // The helper is shared by both the flow and assistant + // provider paths, so the message must not imply this + // is a flow-only failure mode. + "flow execution", + "flow creation", + }, + wantUnwrapsTo: ollamaErr, + }, + { + name: "non-tools error keeps existing wrap", + input: genericErr, + wantContains: []string{ + "failed to determine tool call ID template", + "connection refused", + }, + wantNotContain: []string{ + "does not support tool/function calling", + "metadata advertises tool/function calling", + }, + wantUnwrapsTo: genericErr, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := wrapToolCallIDTemplateError(tc.input) + if tc.wantNil { + assert.Nil(t, got) + return + } + if !assert.NotNil(t, got) { + return + } + msg := got.Error() + for _, want := range tc.wantContains { + assert.Truef(t, strings.Contains(msg, want), + "expected error message to contain %q; got %q", want, msg) + } + for _, banned := range tc.wantNotContain { + assert.Falsef(t, strings.Contains(msg, banned), + "expected error message NOT to contain %q; got %q", banned, msg) + } + if tc.wantUnwrapsTo != nil { + assert.True(t, errors.Is(got, tc.wantUnwrapsTo), + "expected wrapped error to unwrap to original via errors.Is") + } + }) + } +} diff --git a/backend/pkg/providers/providers.go b/backend/pkg/providers/providers.go index 4e6e49cc..68207afa 100644 --- a/backend/pkg/providers/providers.go +++ b/backend/pkg/providers/providers.go @@ -445,7 +445,7 @@ func (pc *providerController) NewFlowProvider( tcIDTemplate, err := prv.GetToolCallIDTemplate(ctx, prompter) if err != nil { - return nil, fmt.Errorf("failed to determine tool call ID template: %w", err) + return nil, wrapToolCallIDTemplateError(err) } fp := &flowProvider{ @@ -591,7 +591,7 @@ func (pc *providerController) NewAssistantProvider( tcIDTemplate, err := prv.GetToolCallIDTemplate(ctx, prompter) if err != nil { - return nil, fmt.Errorf("failed to determine tool call ID template: %w", err) + return nil, wrapToolCallIDTemplateError(err) } ap := &assistantProvider{