add the greeting msg

go-api/internal/httpserver/gatewaylog_test.go

@@ -0,0 +1,171 @@
+package httpserver
+
+// Unit tests for the operator's side of a GatewayFailure.
+//
+// The user-facing sentence tells the reader whether retrying can work. These
+// assert the other half: that the deployment says WHICH fault it was, to the
+// only audience that can act on it. Four of the five gateway faults need an
+// administrator, and until this line existed a deployment failing every run
+// emitted a stream of 200s and nothing else.
+//
+// Internal rather than httpserver_test because the function under test is
+// unexported. Pure: a result goes in and a log record comes out — no server
+// wiring, no database.
+
+import (
+	"bytes"
+	"encoding/json"
+	"log/slog"
+	"strings"
+	"testing"
+
+	"github.com/krow/krow-backend/go-api/internal/authctx"
+	"github.com/krow/krow-backend/go-api/internal/gateway"
+	"github.com/krow/krow-backend/go-api/internal/runtime"
+)
+
+// logging builds a Server that logs into a buffer, and a reader for the records
+// it wrote.
+func logging(t *testing.T) (*Server, func() []map[string]any) {
+	t.Helper()
+	var buf bytes.Buffer
+	s := &Server{log: slog.New(slog.NewJSONHandler(&buf, nil))}
+
+	return s, func() []map[string]any {
+		var out []map[string]any
+		for _, line := range strings.Split(strings.TrimSpace(buf.String()), "\n") {
+			if line == "" {
+				continue
+			}
+			var rec map[string]any
+			if err := json.Unmarshal([]byte(line), &rec); err != nil {
+				t.Fatalf("log line is not JSON: %v", err)
+			}
+			out = append(out, rec)
+		}
+		return out
+	}
+}
+
+func gatewayResult(term runtime.Termination, cause error) *runtime.ExecutionResult {
+	return &runtime.ExecutionResult{
+		RunID:        "run_1",
+		AgentID:      "activity-agent",
+		AgentVersion: 3,
+		Termination:  term,
+		Error: &runtime.RuntimeError{
+			Code:    "runtime." + strings.ToLower(string(term)),
+			Message: "internal wording",
+			Cause:   cause,
+		},
+	}
+}
+
+// The code is what distinguishes the retryable fault from the four that need an
+// administrator, so it is the field that must survive into the log.
+func TestGatewayFailureIsLoggedWithItsCode(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		cause      error
+		wantCode   string
+		wantStatus float64
+	}{
+		{
+			name:     "no credential configured",
+			cause:    &gateway.Error{Code: gateway.CodeNotConfigured, Message: "no model credentials"},
+			wantCode: gateway.CodeNotConfigured,
+		},
+		{
+			name:       "credential rejected",
+			cause:      &gateway.Error{Code: gateway.CodeUnauthorized, Message: "refused", Status: 401},
+			wantCode:   gateway.CodeUnauthorized,
+			wantStatus: 401,
+		},
+		{
+			name:       "rate limited",
+			cause:      &gateway.Error{Code: gateway.CodeRateLimited, Message: "slow down", Status: 429},
+			wantCode:   gateway.CodeRateLimited,
+			wantStatus: 429,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			s, records := logging(t)
+			s.logGatewayFailure(authctx.Identity{OrgID: "org_1"},
+				gatewayResult(runtime.TerminationGatewayFailure, tc.cause))
+
+			recs := records()
+			if len(recs) != 1 {
+				t.Fatalf("wrote %d log records, want 1: %v", len(recs), recs)
+			}
+			rec := recs[0]
+
+			if rec["level"] != "ERROR" {
+				t.Errorf("level = %v, want ERROR — a deployment that cannot reach its model is an outage", rec["level"])
+			}
+			if got := rec["gateway_code"]; got != tc.wantCode {
+				t.Errorf("gateway_code = %v, want %q", got, tc.wantCode)
+			}
+			if got := rec["gateway_status"]; got != tc.wantStatus {
+				t.Errorf("gateway_status = %v, want %v", got, tc.wantStatus)
+			}
+
+			// §10: every line carries these four.
+			for _, field := range []string{"run_id", "tenant_id", "agent_key", "agent_version"} {
+				if rec[field] == nil {
+					t.Errorf("log record has no %s", field)
+				}
+			}
+			// §10 again: no model or document text in the log store. The
+			// gateway's message can quote the provider's body, so it stays out.
+			if strings.Contains(strings.ToLower(rec["msg"].(string)), "refused") {
+				t.Errorf("msg = %q, want no provider text", rec["msg"])
+			}
+			for k, v := range rec {
+				if str, ok := v.(string); ok && strings.Contains(str, "slow down") {
+					t.Errorf("field %s leaked the provider message: %q", k, str)
+				}
+			}
+		})
+	}
+}
+
+// A cause that is not a gateway error leaves the code visibly empty rather than
+// guessed. "Which fault was it" is the whole point of the line, and a wrong
+// answer to it is worse than a gap.
+func TestGatewayFailureWithoutACauseLogsAnEmptyCode(t *testing.T) {
+	s, records := logging(t)
+	s.logGatewayFailure(authctx.Identity{OrgID: "org_1"},
+		gatewayResult(runtime.TerminationGatewayFailure, nil))
+
+	recs := records()
+	if len(recs) != 1 {
+		t.Fatalf("wrote %d log records, want 1", len(recs))
+	}
+	if got := recs[0]["gateway_code"]; got != "" {
+		t.Errorf("gateway_code = %v, want empty", got)
+	}
+}
+
+// Every other termination is silent here. A run that hit its budget or was
+// refused is not a gateway outage, and logging it as one would make the signal
+// useless exactly when it is being read.
+func TestOnlyGatewayFailureIsLogged(t *testing.T) {
+	for _, term := range []runtime.Termination{
+		runtime.TerminationCompleted,
+		runtime.TerminationBudgetExceeded,
+		runtime.TerminationDeadline,
+		runtime.TerminationConfirmationPending,
+		runtime.TerminationToolFailure,
+		runtime.TerminationRefused,
+	} {
+		t.Run(string(term), func(t *testing.T) {
+			s, records := logging(t)
+			s.logGatewayFailure(authctx.Identity{OrgID: "org_1"},
+				gatewayResult(term, &gateway.Error{Code: gateway.CodeRateLimited}))
+
+			if recs := records(); len(recs) != 0 {
+				t.Errorf("wrote %d log records for %s, want none: %v", len(recs), term, recs)
+			}
+		})
+	}
+}

go-api/internal/httpserver/gatewaymessage_test.go

@@ -0,0 +1,170 @@
+package httpserver
+
+// Unit tests for the wording a GatewayFailure produces.
+//
+// Internal rather than httpserver_test because the function under test is the
+// mapping itself, and the mapping is unexported. Pure: no server, no database,
+// no fixture — a cause goes in and a sentence comes out.
+//
+// What these assert is one property, and it is the one the old wording broke:
+// a reader is told to retry EXACTLY when retrying can work. A rate limit clears
+// on its own; a rejected credential, a model id the endpoint does not have, and
+// an unconfigured deployment do not, and telling somebody to wait a minute for
+// any of those is a loop with no exit.
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/krow/krow-backend/go-api/internal/gateway"
+	"github.com/krow/krow-backend/go-api/internal/runtime"
+)
+
+// invitesRetry reports whether a sentence tells the reader to try again.
+//
+// Deliberately looser than an equality check on the whole string: what must
+// hold is the ADVICE, not the copy, so rewording a sentence does not fail a
+// test that was never about the words.
+func invitesRetry(message string) bool {
+	m := strings.ToLower(message)
+	return strings.Contains(m, "ask again") || strings.Contains(m, "try again")
+}
+
+func TestGatewayFailureMessageInvitesRetryOnlyWhenRetryingCanWork(t *testing.T) {
+	cases := []struct {
+		name  string
+		cause error
+		retry bool
+	}{
+		{
+			name:  "a rate limit clears on its own",
+			cause: &gateway.Error{Code: gateway.CodeRateLimited, Status: 429},
+			retry: true,
+		},
+		{
+			name:  "a provider 5xx is worth another attempt",
+			cause: &gateway.Error{Code: gateway.CodeUpstream, Status: 503},
+			retry: true,
+		},
+		{
+			name:  "a rejected credential will be rejected again",
+			cause: &gateway.Error{Code: gateway.CodeUnauthorized, Status: 401},
+			retry: false,
+		},
+		{
+			name:  "a model the endpoint does not have stays absent",
+			cause: &gateway.Error{Code: gateway.CodeInvalidRequest, Status: 404},
+			retry: false,
+		},
+		{
+			name:  "an unconfigured deployment cannot answer at all",
+			cause: &gateway.Error{Code: gateway.CodeNotConfigured},
+			retry: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := gatewayFailureMessage(tc.cause)
+			if got == "" {
+				t.Fatal("a failed run must say something")
+			}
+			if invitesRetry(got) != tc.retry {
+				t.Errorf("retry advice = %v, want %v\n  message: %q",
+					invitesRetry(got), tc.retry, got)
+			}
+			// Nothing ran, so nothing can have been written. The reassurance is
+			// the whole reason this termination is not frightening.
+			if !strings.Contains(got, "Nothing was changed") {
+				t.Errorf("message must say nothing was changed: %q", got)
+			}
+		})
+	}
+}
+
+// The three that need a person are the three that used to be indistinguishable
+// from load. Each must point at one, or the reader has no idea what to do next.
+func TestGatewayFailureMessageNamesAnAdministratorWhenOneIsNeeded(t *testing.T) {
+	for _, code := range []string{
+		gateway.CodeUnauthorized,
+		gateway.CodeInvalidRequest,
+		gateway.CodeNotConfigured,
+	} {
+		got := gatewayFailureMessage(&gateway.Error{Code: code})
+		if !strings.Contains(strings.ToLower(got), "administrator") {
+			t.Errorf("%s: must send the reader to an administrator: %q", code, got)
+		}
+	}
+}
+
+// Vendor names, model ids and HTTP statuses are for the trajectory, not for a
+// venue manager — they cannot act on any of them.
+func TestGatewayFailureMessageLeaksNoOperatorDetail(t *testing.T) {
+	for _, code := range []string{
+		gateway.CodeRateLimited,
+		gateway.CodeUpstream,
+		gateway.CodeUnauthorized,
+		gateway.CodeInvalidRequest,
+		gateway.CodeNotConfigured,
+	} {
+		got := gatewayFailureMessage(&gateway.Error{
+			Code:    code,
+			Status:  429,
+			Message: "gemini-3.5-flash-lite quota exceeded for project 12345",
+		})
+		for _, leak := range []string{"gemini", "429", "quota", "http", "12345"} {
+			if strings.Contains(strings.ToLower(got), leak) {
+				t.Errorf("%s: message carries operator detail %q: %q", code, leak, got)
+			}
+		}
+	}
+}
+
+// A cause that is not a gateway error — lost, wrapped away, or a non-gateway
+// failure that reached this termination — still has to produce a sentence.
+func TestGatewayFailureMessageFallsBackWithoutAGatewayError(t *testing.T) {
+	for _, cause := range []error{nil, errors.New("something else entirely")} {
+		if got := gatewayFailureMessage(cause); got == "" {
+			t.Errorf("cause %v produced no message", cause)
+		}
+	}
+}
+
+// The cause arrives wrapped in a RuntimeError, which is how the surface
+// actually receives it. If Unwrap ever stopped reaching the gateway error,
+// every failure would silently fall back to "usually it is busy" — the exact
+// bug this change exists to fix, reintroduced without a compile error.
+func TestGatewayFailureMessageReadsThroughARuntimeError(t *testing.T) {
+	wrapped := &runtime.RuntimeError{
+		Code:  "runtime.gatewayfailure",
+		Cause: &gateway.Error{Code: gateway.CodeUnauthorized, Status: 401},
+	}
+
+	got := gatewayFailureMessage(wrapped)
+	if invitesRetry(got) {
+		t.Errorf("a wrapped credential failure must not invite a retry: %q", got)
+	}
+}
+
+// The other terminations are unchanged by the new parameter: they ignore the
+// cause, so passing one must not alter a single word.
+func TestTerminationMessageIgnoresTheCauseElsewhere(t *testing.T) {
+	cause := &gateway.Error{Code: gateway.CodeUnauthorized}
+
+	for _, term := range []runtime.Termination{
+		runtime.TerminationBudgetExceeded,
+		runtime.TerminationDeadline,
+		runtime.TerminationConfirmationPending,
+		runtime.TerminationToolFailure,
+		runtime.TerminationRefused,
+	} {
+		if terminationMessage(term, nil) != terminationMessage(term, cause) {
+			t.Errorf("%s: wording changed with the cause", term)
+		}
+	}
+
+	if terminationMessage(runtime.TerminationCompleted, nil) != "" {
+		t.Error("a completed run has nothing to say")
+	}
+}

go-api/internal/httpserver/runs.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/krow/krow-backend/go-api/internal/authctx"
 	"github.com/krow/krow-backend/go-api/internal/domain"
+	"github.com/krow/krow-backend/go-api/internal/gateway"
 	"github.com/krow/krow-backend/go-api/internal/runtime"
 	"github.com/krow/krow-backend/go-api/internal/tools"
 )
@@ -163,6 +164,7 @@ func (s *Server) handleAgentRun(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	s.logUnsaved(ident, res)
+	s.logGatewayFailure(ident, res)
 
 	writeJSON(w, http.StatusOK, buildRunResponse(res))
 }
@@ -180,6 +182,43 @@ func (s *Server) logUnsaved(ident authctx.Identity, res *runtime.ExecutionResult
 	}
 }
 
+// logGatewayFailure is the operator's record of the model provider not
+// answering, and it exists because the reader of the message cannot report what
+// the message does not say.
+//
+// Four of the five gateway faults need an administrator and will fail
+// identically on every retry — a rejected credential, a model id this
+// deployment cannot use, no key at all, and a 4xx from the endpoint. The person
+// in the chat panel is told, correctly, that retrying will not help; but
+// nothing until now told the side that CAN fix it. A deployment failing every
+// run produced a stream of 200s and no error line, so the only record of which
+// fault it was lived in a trajectory somebody had to know to go and read.
+//
+// Logged at Error because that is what it is: on a rate limit it is a capacity
+// decision worth seeing, and on the other four it is an outage. It carries the
+// gateway's code and status and NOT the message — §10 keeps model and document
+// text out of the log store, and the code is the part that is actionable
+// anyway.
+func (s *Server) logGatewayFailure(ident authctx.Identity, res *runtime.ExecutionResult) {
+	if res.Termination != runtime.TerminationGatewayFailure {
+		return
+	}
+
+	// Empty rather than invented when the cause did not survive: "which fault
+	// was it" is the whole point of this line, and a guessed answer to it is
+	// worse than a visible gap.
+	code, status := "", 0
+	var gwErr *gateway.Error
+	if errors.As(res.Error, &gwErr) {
+		code, status = gwErr.Code, gwErr.Status
+	}
+
+	s.log.Error("gateway failure",
+		"run_id", res.RunID, "tenant_id", ident.OrgID,
+		"agent_key", res.AgentID, "agent_version", res.AgentVersion,
+		"gateway_code", code, "gateway_status", status)
+}
+
 // buildRunResponse turns a runtime result into the client's shape.
 //
 // Every termination answers 200. That looks wrong at first and is not: the
@@ -206,7 +245,7 @@ func buildRunResponse(res *runtime.ExecutionResult) runResponse {
 		},
 	}
 	if res.Termination != runtime.TerminationCompleted {
-		out.Message = terminationMessage(res.Termination)
+		out.Message = terminationMessage(res.Termination, res.Error)
 	}
 	return out
 }
@@ -222,7 +261,9 @@ func buildRunResponse(res *runtime.ExecutionResult) runResponse {
 // Every one of the seven is spelled out. A default that said "something went
 // wrong" would be the place where a Refused run and a ToolFailure became
 // indistinguishable to the person best placed to tell us which it was.
-func terminationMessage(t runtime.Termination) string {
+// `cause` is the run's own error, carried so GatewayFailure can say which
+// gateway failure it was. Every other termination ignores it.
+func terminationMessage(t runtime.Termination, cause error) string {
 	switch t {
 	case runtime.TerminationCompleted:
 		return ""
@@ -239,16 +280,70 @@ func terminationMessage(t runtime.Termination) string {
 	case runtime.TerminationRefused:
 		return "The agent declined to answer this one."
 	case runtime.TerminationGatewayFailure:
-		// The one termination where "try again" is honest advice: the
-		// dominant cause is a rate limit that clears within a minute, and
-		// nothing about the question itself was the problem.
-		return "The model behind this agent did not answer — usually it is busy. " +
-			"Wait a minute and ask again. Nothing was changed."
+		return gatewayFailureMessage(cause)
 	default:
 		return "The agent did not finish."
 	}
 }
 
+// gatewayFailureMessage tells a GatewayFailure apart from the four others it
+// used to be indistinguishable from.
+//
+// GatewayFailure is everything the gateway can raise except a refusal and a
+// timeout, which have terminations of their own. That is five different faults,
+// and the one sentence they all produced was "usually it is busy — wait a
+// minute and ask again".
+//
+// For a rate limit that is true and useful. For the other three it is advice
+// that CANNOT work: a credential the provider rejected, a model id that is not
+// on the configured endpoint, and no key at all will each fail identically on
+// every retry, forever. Telling somebody to wait a minute for a misconfigured
+// deployment sends them round a loop with no exit, and hides an operator
+// problem behind what reads as a transient one — the reader retries instead of
+// reporting it, so nobody with access to fix it ever hears.
+//
+// So each says what it is, and only the two that clear on their own invite a
+// retry. The wording stays free of vendor names and status codes: the person
+// reading it cannot act on "429 from the model endpoint", and the code is in
+// the trajectory for the person who can.
+func gatewayFailureMessage(cause error) string {
+	var gwErr *gateway.Error
+	if !errors.As(cause, &gwErr) {
+		// No gateway error to read — either the cause was lost or something
+		// non-gateway reached this termination. The old sentence is still the
+		// best guess, so it is what an unknown falls back to.
+		return "The model behind this agent did not answer — usually it is busy. " +
+			"Wait a minute and ask again. Nothing was changed."
+	}
+
+	switch gwErr.Code {
+	case gateway.CodeRateLimited:
+		return "The model behind this agent is busy right now. " +
+			"Wait a minute and ask again. Nothing was changed."
+	case gateway.CodeUpstream:
+		if gwErr.Status >= 500 {
+			return "The model behind this agent is having trouble. " +
+				"Try again in a few minutes. Nothing was changed."
+		}
+		return "The model behind this agent could not be reached, and retrying is " +
+			"unlikely to help. This needs an administrator. Nothing was changed."
+	case gateway.CodeUnauthorized:
+		return "This deployment's model credentials were rejected, so the agent " +
+			"cannot answer. Retrying will not help — it needs an administrator. " +
+			"Nothing was changed."
+	case gateway.CodeInvalidRequest:
+		return "The agent is pointed at a model this deployment cannot use, so it " +
+			"cannot answer. Retrying will not help — it needs an administrator. " +
+			"Nothing was changed."
+	case gateway.CodeNotConfigured:
+		return "No model is configured for this deployment, so the agent cannot " +
+			"answer. It needs an administrator. Nothing was changed."
+	default:
+		return "The model behind this agent did not answer — usually it is busy. " +
+			"Wait a minute and ask again. Nothing was changed."
+	}
+}
+
 // runLoadError maps a pre-run failure onto the API's error vocabulary.
 //
 // These are the errors from LoadExecutableAgent, raised before any run began —
@@ -351,6 +446,7 @@ func (s *Server) streamAgentRun(w http.ResponseWriter, r *http.Request, ident au
 			return
 		}
 		s.logUnsaved(ident, res)
+		s.logGatewayFailure(ident, res)
 		writeJSON(w, http.StatusOK, buildRunResponse(res))
 		return
 	}
@@ -399,6 +495,7 @@ func (s *Server) streamAgentRun(w http.ResponseWriter, r *http.Request, ident au
 	}
 
 	s.logUnsaved(ident, res)
+	s.logGatewayFailure(ident, res)
 	send(map[string]any{"run": buildRunResponse(res)})
 	fmt.Fprint(w, "data: [DONE]\n\n")
 	flusher.Flush()