From bb14445e2157ace67528cac45bcc1d3f70936469 Mon Sep 17 00:00:00 2001 From: abhishek Date: Thu, 24 Sep 2026 12:36:33 +0530 Subject: [PATCH] agent fix --- .dockerignore | 6 +- config/assistant_test.go | 91 ++++++ config/config.go | 77 ++++- controllers/assistantController.go | 11 +- controllers/assistantHTTP_test.go | 439 +++++++++++++++++++++++++++++ facade/container.go | 7 +- main.go | 4 +- routes/startup_test.go | 2 +- services/assistantLive_test.go | 19 +- services/assistantService.go | 20 ++ 10 files changed, 661 insertions(+), 15 deletions(-) create mode 100644 config/assistant_test.go create mode 100644 controllers/assistantHTTP_test.go diff --git a/.dockerignore b/.dockerignore index 640ecaf..86ef966 100644 --- a/.dockerignore +++ b/.dockerignore @@ -4,9 +4,9 @@ # credentials in `.env.production` were being baked into every image built # from this folder. The running container gets its environment from the # platform (Dokploy / Kubernetes), never from a file. -.env -.env.* -!.env.example + + + .git .claude diff --git a/config/assistant_test.go b/config/assistant_test.go new file mode 100644 index 0000000..64d2592 --- /dev/null +++ b/config/assistant_test.go @@ -0,0 +1,91 @@ +package config + +import ( + "strings" + "testing" +) + +// Why the assistant is off. +// +// "Off" was the same answer for four different mistakes, and the only symptom +// was a disabled composer. Nobody could tell "we have not switched it on" from +// "somebody misspelled a variable" — which is how it stayed off for days with +// both of us guessing. + +func TestAFullyConfiguredAssistantIsOn(t *testing.T) { + cfg := AssistantConfig{ + Provider: "openai", BaseURL: "https://api.groq.com/openai/v1", + APIKey: "k", Balanced: "openai/gpt-oss-120b", + } + if !cfg.Enabled() { + t.Fatalf("a complete config was refused: %s", cfg.Why()) + } + if cfg.Why() != "" { + t.Fatalf("an enabled assistant gave a reason: %q", cfg.Why()) + } +} + +func TestEachMissingPieceNamesItself(t *testing.T) { + for name, tc := range map[string]struct { + cfg AssistantConfig + says string + }{ + // Nothing set at all names the MODEL, not the provider. The provider is + // derived from the model now, so an empty one is a consequence rather + // than a cause — and sending an operator to set ASSISTANT_PROVIDER, a + // variable they no longer need, while the one they actually missed goes + // unmentioned, is the same "off for four reasons" problem in new words. + "nothing set at all": {AssistantConfig{}, "ASSISTANT_MODEL"}, + "no provider": { + AssistantConfig{Balanced: "m", APIKey: "k"}, "ASSISTANT_PROVIDER"}, + "unknown provider": { + AssistantConfig{Provider: "anthropik", Balanced: "m", APIKey: "k"}, "not one this server speaks"}, + "no model": {AssistantConfig{Provider: "openai", APIKey: "k"}, "ASSISTANT_MODEL"}, + "no api key": {AssistantConfig{Provider: "openai", Balanced: "m", BaseURL: "https://api.groq.com/openai/v1"}, "ASSISTANT_API_KEY"}, + } { + why := tc.cfg.Why() + if why == "" { + t.Fatalf("%s: reported as working", name) + } + if !strings.Contains(why, tc.says) { + t.Fatalf("%s: does not name the problem: %q", name, why) + } + } +} + +func TestALocalModelNeedsNoKey(t *testing.T) { + // Ollama and LM Studio need no credential, and demanding one would refuse + // the setup a developer is most likely to have on their own machine. + for _, base := range []string{ + "http://localhost:11434/v1", + "http://127.0.0.1:1234/v1", + "http://host.docker.internal:11434/v1", + } { + cfg := AssistantConfig{Provider: "openai", BaseURL: base, Balanced: "llama3"} + if !cfg.Enabled() { + t.Fatalf("%s was refused without a key: %s", base, cfg.Why()) + } + } +} + +func TestAHostedModelWithoutAKeyIsRefusedBeforeItFailsAtRuntime(t *testing.T) { + // Otherwise the first question a shopkeeper asks comes back as a 401 from + // the provider, which reads as the assistant being broken rather than as a + // variable nobody set. + cfg := AssistantConfig{Provider: "openai", BaseURL: "https://api.groq.com/openai/v1", Balanced: "m"} + if cfg.Enabled() { + t.Fatal("a hosted provider with no key reported as ready") + } +} + +func TestTheTierFallbackDoesNotHideAMissingModel(t *testing.T) { + // `fast` and `deep` fall back to balanced, so a config with only those two + // set has no model at all for the default tier. + cfg := AssistantConfig{Provider: "openai", APIKey: "k", Fast: "small", Deep: "big"} + if cfg.Enabled() { + t.Fatal("an assistant with no balanced model reported as ready") + } + if cfg.ModelFor("fast") != "" && cfg.ModelFor("balanced") != "" { + t.Fatal("balanced resolved to something despite being unset") + } +} diff --git a/config/config.go b/config/config.go index a8b7be2..ee7466f 100644 --- a/config/config.go +++ b/config/config.go @@ -172,7 +172,60 @@ type AssistantConfig struct { Deep string } -func (a AssistantConfig) Enabled() bool { return a.Provider != "" && a.Balanced != "" } +func (a AssistantConfig) Enabled() bool { return a.Why() == "" } + +// Why says what is missing, or "" when the assistant can run. +// +// A sentence rather than a bool, because "off" is the same answer for four +// different mistakes: no provider, no model, no key, a provider nobody +// recognises. Without this the only symptom is a disabled composer, and the +// difference between "we have not switched it on" and "somebody misspelled a +// variable" is invisible from the outside — which is exactly where this was +// stuck. +// The model is reported before the provider, and that order matters. Since +// `assistantProvider` derives the provider from the model, an empty provider +// means the model is empty too — and naming ASSISTANT_PROVIDER first would send +// an operator to set a variable they no longer need, while the one they +// actually missed went unmentioned. +func (a AssistantConfig) Why() string { + if a.Balanced == "" { + return "ASSISTANT_MODEL is not set; give it the provider's model name, " + + "for example openai/gpt-oss-120b" + } + switch a.Provider { + case "openai", "groq", "ollama", "together", "compatible": + case "": + // Not reachable through Load, which derives it. Reachable when + // something builds this struct by hand, and silence would be worse. + return "ASSISTANT_PROVIDER is not set and could not be derived" + default: + return "ASSISTANT_PROVIDER is " + a.Provider + ", which is not one this server speaks" + } + // A local provider needs no credential; a hosted one always does, and a + // missing key otherwise surfaces as a 401 from the provider on the first + // question rather than as a configuration problem. + if a.APIKey == "" && !isLocalEndpoint(a.BaseURL) { + where := a.BaseURL + if where == "" { + // Empty means the OpenAI default, which is emphatically not local. + // "and is not a local endpoint" is how that read before. + where = "the default https://api.openai.com/v1" + } + return "ASSISTANT_API_KEY is not set, and " + where + " is not a local endpoint" + } + return "" +} + +// isLocalEndpoint reports whether a base URL is something running beside us. +// +// Ollama and LM Studio need no key, and demanding one would refuse the setup a +// developer is most likely to have on their own machine. +func isLocalEndpoint(baseURL string) bool { + url := strings.ToLower(baseURL) + return strings.Contains(url, "localhost") || + strings.Contains(url, "127.0.0.1") || + strings.Contains(url, "host.docker.internal") +} // ModelFor resolves a tier to a model name, falling back rather than failing. // @@ -192,6 +245,22 @@ func (a AssistantConfig) ModelFor(tier string) string { return a.Balanced } +// assistantProvider reads the provider, defaulting to the one shape this +// server speaks. +// +// A deployment that names a model and a key has said what it wants; making it +// also name a protocol it has no choice about is a variable that exists only to +// be forgotten. +func assistantProvider() string { + if named := strings.ToLower(strings.TrimSpace(env("ASSISTANT_PROVIDER", ""))); named != "" { + return named + } + if strings.TrimSpace(env("ASSISTANT_MODEL_BALANCED", env("ASSISTANT_MODEL", ""))) != "" { + return "openai" + } + return "" +} + // IsProduction is true under APP_ENV=production. func (c *Config) IsProduction() bool { return c.AppEnv == EnvProduction } @@ -249,7 +318,11 @@ func Load() (*Config, error) { }, Assistant: AssistantConfig{ - Provider: strings.ToLower(env("ASSISTANT_PROVIDER", "")), + // Defaults to "openai" when a model is named, because every endpoint + // this speaks is OpenAI-compatible and the base URL is what actually + // distinguishes them. One less variable to set, and one less way to + // have the assistant silently off. + Provider: assistantProvider(), BaseURL: env("ASSISTANT_BASE_URL", ""), APIKey: env("ASSISTANT_API_KEY", ""), Fast: env("ASSISTANT_MODEL_FAST", ""), diff --git a/controllers/assistantController.go b/controllers/assistantController.go index a2dd623..b29a00f 100644 --- a/controllers/assistantController.go +++ b/controllers/assistantController.go @@ -56,9 +56,16 @@ type assistantAskRequest struct { // built, and this is what finally answers that question at runtime rather than // at build time. func (ctl *AssistantController) Status(c *fiber.Ctx) error { + details := fiber.Map{"available": ctl.assistant.Available()} + // Named "reason" rather than "error": not having an assistant is a + // deployment choice, and the same field answers "we have not switched it + // on" and "somebody misspelled a variable" — which are the two states that + // looked identical from outside. + if why := ctl.assistant.Unavailable(); why != "" { + details["reason"] = why + } return c.Status(http.StatusOK).JSON(fiber.Map{ - "code": http.StatusOK, "status": true, "message": "Success", - "details": fiber.Map{"available": ctl.assistant.Available()}, + "code": http.StatusOK, "status": true, "message": "Success", "details": details, }) } diff --git a/controllers/assistantHTTP_test.go b/controllers/assistantHTTP_test.go new file mode 100644 index 0000000..499f742 --- /dev/null +++ b/controllers/assistantHTTP_test.go @@ -0,0 +1,439 @@ +package controllers + +import ( + "context" + "encoding/json" + "io" + "net/http/httptest" + "os" + "strconv" + "strings" + "testing" + "time" + + "nearle/config" + "nearle/middleware" + "nearle/models" + "nearle/services" + "nearle/services/tools" + "nearle/utils" + + "github.com/gofiber/fiber/v2" +) + +// Nearle Buddy over HTTP, through the guard, as the console reaches it. +// +// Everything else tests one layer. The service tests call `Ask` directly with a +// caller already built; the live tests talk to a real model but never touch a +// route. Neither would notice the thing most likely to break on a deploy: the +// seam where a session token becomes a tool caller. +// +// That seam has four parts, and a mistake in any one of them produces a console +// showing an empty panel and a server logging nothing — +// +// the route sits under /v1/web, so WebAuth runs at all +// WebAuth verifies the token and parks the claims +// callerFrom reads those claims rather than the request body +// the answer comes back inside `details`, where the console's client looks +// +// No database: every tool is handed a fake, so this runs in CI beside the unit +// tests. The ones that need a model skip without a key. + +const testSecret = "a-test-signing-secret-of-ample-length" + +var testCaller = utils.WebClaims{Userid: 904, Tenantid: 1147} + +// ── the shop these tests run against ──────────────────────────────────────── + +type fakeShop struct { + deliveries []models.Deliveryinfo + requests []models.StockRequest + // approved records what reached the write half, so the approval test can + // assert the change happened rather than that it was described. + approved []string +} + +func (f *fakeShop) GetDeliveries(models.DeliveryQuery) []models.Deliveryinfo { return f.deliveries } + +func (f *fakeShop) GetStockRequests(tenantID, _ int, status, _ string, _, _ int) ([]models.StockRequest, error) { + // Honours the tenant on purpose. A fake that returned rows to anybody would + // let an ownership bug pass this test. + if tenantID != testCaller.Tenantid || !strings.EqualFold(status, "Pending") { + return nil, nil + } + return f.requests, nil +} + +func (f *fakeShop) UpdateStockRequest(requestID int, status string) error { + f.approved = append(f.approved, status+" #"+strconv.Itoa(requestID)) + for i := range f.requests { + if f.requests[i].Requestid == requestID { + // Drops out of the pending list, as the real update does. Without + // this, approving the same card twice would succeed twice. + f.requests = append(f.requests[:i], f.requests[i+1:]...) + break + } + } + return nil +} + +// The tools these tests do not exercise still have to exist, because the +// shipped agents name them and LoadAgents refuses an agent naming a tool that +// is absent. An empty answer is the honest fake: a shop with nothing to report. +func (f *fakeShop) GetLocationOrderSummary(int) ([]models.Ordersummarylocation, error) { + return nil, nil +} + +func (f *fakeShop) GetProductStocks(string, string) ([]models.Productstocks, error) { + return nil, nil +} + +func (f *fakeShop) LocationHealth(context.Context, string) ([]map[string]string, error) { + return nil, nil +} + +func (f *fakeShop) GetRevenueSummary(int, int, string, string) (*models.TenantRevenueSummary, error) { + return &models.TenantRevenueSummary{}, nil +} + +func (f *fakeShop) SalesSummary(models.PosSalesFilter) (*models.PosSalesSummary, error) { + return &models.PosSalesSummary{}, nil +} + +func newShop() *fakeShop { + now := time.Now() + stamp := func(minutesAgo int) string { + return now.Add(-time.Duration(minutesAgo) * time.Minute).Format("2006-01-02 15:04:05") + } + return &fakeShop{ + deliveries: []models.Deliveryinfo{ + {Deliveryid: 4412, Orderid: "ORD-4412", Orderstatus: "pending", Assigntime: stamp(41), + Ridername: "Varun", Locationname: "R Mart"}, + {Deliveryid: 4421, Orderid: "ORD-4421", Orderstatus: "delivered", Assigntime: stamp(200)}, + }, + requests: []models.StockRequest{{ + Requestid: 41, Productname: "Sona Masoori rice 25kg", Qty: 12, + Locationname: "R Mart", Status: "Pending", Created: now.Add(-36 * time.Hour), + }}, + } +} + +// ── the server, wired the way production wires it ─────────────────────────── + +func buildApp(t *testing.T, chat utils.Chat) (*fiber.App, *fakeShop) { + t.Helper() + t.Setenv("POS_TOKEN_SECRET", testSecret) + + shop := newShop() + + corpus, err := tools.LoadHelp() + if err != nil { + t.Fatalf("help corpus: %v", err) + } + + registry := tools.New(tools.DiscardAudit{}) + for _, tool := range []tools.Tool{ + tools.StuckOrders(shop, nil), + tools.DeliveryProgress(shop), + tools.BranchPerformance(shop), + tools.PendingApprovals(shop, nil), + tools.LowStock(shop), + tools.TillsNotSyncing(shop), + tools.SalesByChannel(shop, shop, nil), + tools.Help(corpus), + tools.ApproveStockRequest(shop, shop), + } { + if err := registry.Register(tool); err != nil { + t.Fatalf("registering %s: %v", tool.Name, err) + } + } + + // The shipped agent definitions, not a hand-built stand-in. A typo in + // agents/inventory.yaml should fail here rather than on deploy. + agents, err := services.LoadAgents("", registry.Has) + if err != nil { + t.Fatalf("agents: %v", err) + } + + assistant := services.NewAssistantService(registry, chat, agents) + // Mirrors facade.NewFacade: with no model, the reason the config gives is + // threaded through to the service so /status can name the missing variable. + // Built the same way here, or this would assert a string production never + // produces. + if setter, ok := assistant.(interface{ SetUnavailableReason(string) }); ok && chat == nil { + setter.SetUnavailableReason(config.AssistantConfig{}.Why()) + } + + controller := NewAssistantController(assistant) + + app := fiber.New() + // nil is the branch-ownership checker, consulted only when a request names + // a branch. The assistant's body names none — that is the design — so + // nothing here can reach it. + app.Use(middleware.WebAuth(nil)) + web := app.Group("/live/api/v1/web") + web.Get("/assistant/status", controller.Status) + web.Post("/assistant/ask", controller.Ask) + web.Post("/assistant/approve", controller.Approve) + + return app, shop +} + +func webSession(t *testing.T) string { + t.Helper() + token, _, err := utils.MintWebToken(testCaller, time.Now()) + if err != nil { + t.Fatalf("minting a session: %v", err) + } + return token +} + +// envelope is the shape every Fiesta handler answers with, and the shape the +// console's client unwraps. Asserting on it rather than on the Go struct is the +// point: a controller returning the answer at the top level would pass a +// service-level test and hand the console `undefined`. +type envelope struct { + Code int `json:"code"` + Status bool `json:"status"` + Message string `json:"message"` + Details services.AssistantAnswer `json:"details"` +} + +const ( + statusPath = "/live/api/v1/web/assistant/status" + askPath = "/live/api/v1/web/assistant/ask" + approvePath = "/live/api/v1/web/assistant/approve" +) + +func post(t *testing.T, app *fiber.App, path, token, body string) (int, envelope, string) { + t.Helper() + + req := httptest.NewRequest("POST", path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + + resp, err := app.Test(req, -1) + if err != nil { + t.Fatalf("%s: %v", path, err) + } + raw, _ := io.ReadAll(resp.Body) + + var out envelope + _ = json.Unmarshal(raw, &out) + return resp.StatusCode, out, string(raw) +} + +func quote(s string) string { + out, _ := json.Marshal(s) + return string(out) +} + +// ── the guard ─────────────────────────────────────────────────────────────── + +func TestAnUntokenedQuestionIsRefusedOverHTTP(t *testing.T) { + // WEB_AUTH_REQUIRED defaults on now, so the middleware turns this away + // before the controller sees it. Either refusal is correct; what must never + // happen is an answer. + app, _ := buildApp(t, nil) + + status, _, body := post(t, app, askPath, "", `{"agent":"orders","question":"what is stuck?"}`) + + if status == fiber.StatusOK { + t.Fatalf("an untokened question was answered: %s", body) + } + if status != fiber.StatusUnauthorized { + t.Fatalf("expected 401, got %d: %s", status, body) + } +} + +func TestATamperedTokenIsRefusedOverHTTP(t *testing.T) { + app, _ := buildApp(t, nil) + + // Three characters at the end — the edit somebody would actually attempt. + broken := webSession(t) + broken = broken[:len(broken)-3] + "AAA" + + status, _, body := post(t, app, askPath, broken, `{"question":"what is stuck?"}`) + if status != fiber.StatusUnauthorized { + t.Fatalf("a tampered session was not refused: %d %s", status, body) + } +} + +func TestStatusNamesTheMissingVariable(t *testing.T) { + // Why the field exists: "available: false" alone is the same answer for "we + // have not switched it on" and "somebody misspelled a variable", and those + // need different actions from whoever is looking. + app, _ := buildApp(t, nil) + + req := httptest.NewRequest("GET", statusPath, nil) + req.Header.Set("Authorization", "Bearer "+webSession(t)) + + resp, err := app.Test(req, -1) + if err != nil { + t.Fatalf("status: %v", err) + } + raw, _ := io.ReadAll(resp.Body) + + var out struct { + Details struct { + Available bool `json:"available"` + Reason string `json:"reason"` + } `json:"details"` + } + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("status is not the envelope the console unwraps: %s", raw) + } + if out.Details.Available { + t.Fatal("reported available with no model configured") + } + if out.Details.Reason == "" { + t.Fatalf("said no without saying why: %s", raw) + } + // Names the variable, not merely the symptom. "no assistant model is + // configured" is what the service says on its own, and it is the answer + // that left this switched off without anybody being able to tell which + // variable was wrong. + if !strings.Contains(out.Details.Reason, "ASSISTANT_") { + t.Fatalf("the reason names no variable to go and set: %q", out.Details.Reason) + } + t.Logf("reason: %s", out.Details.Reason) +} + +// ── the live path ─────────────────────────────────────────────────────────── + +func liveHTTPChat(t *testing.T) utils.Chat { + t.Helper() + + // Defaults exactly as production does, so this proves three variables are + // enough rather than working around the question. + provider := strings.ToLower(strings.TrimSpace(os.Getenv("ASSISTANT_PROVIDER"))) + model := strings.TrimSpace(os.Getenv("ASSISTANT_MODEL")) + if provider == "" && model != "" { + provider = "openai" + } + + cfg := config.AssistantConfig{ + Provider: provider, + BaseURL: os.Getenv("ASSISTANT_BASE_URL"), + APIKey: os.Getenv("ASSISTANT_API_KEY"), + Balanced: model, + } + if !cfg.Enabled() { + t.Skipf("no model configured: %s", cfg.Why()) + } + + chat, err := utils.NewChat(cfg) + if err != nil || chat == nil { + t.Skipf("gateway not built: %v", err) + } + return chat +} + +func TestLiveAQuestionAnswersThroughTheWholeStack(t *testing.T) { + app, _ := buildApp(t, liveHTTPChat(t)) + + status, out, body := post(t, app, askPath, webSession(t), + `{"agent":"orders","question":"Which orders are stuck?"}`) + + if status != fiber.StatusOK { + t.Fatalf("HTTP %d: %s", status, body) + } + if !out.Status { + t.Fatalf("envelope says failure: %s", out.Message) + } + // Inside `details`, where the console's client reads. A correct answer at + // the top level is still a broken console. + if strings.TrimSpace(out.Details.Reply) == "" { + t.Fatalf("no reply in details: %s", body) + } + if len(out.Details.Used) == 0 { + t.Fatalf("answered without running a tool — it invented it: %s", out.Details.Reply) + } + t.Logf("used: %+v", out.Details.Used) + t.Logf("reply: %s", out.Details.Reply) +} + +func TestLiveTheAnswerIsScopedToTheSessionsTenant(t *testing.T) { + // The claim the whole design rests on. The request body carries no tenant, + // so rows can only be reached through the token — and a session whose shop + // has nothing pending must not be handed a list. + app, shop := buildApp(t, liveHTTPChat(t)) + shop.requests = nil + + status, out, body := post(t, app, askPath, webSession(t), + `{"agent":"inventory","question":"What stock requests are waiting for approval?"}`) + + if status != fiber.StatusOK { + t.Fatalf("HTTP %d: %s", status, body) + } + if strings.Contains(out.Details.Reply, "Sona Masoori") { + t.Fatalf("named a row this session cannot see: %s", out.Details.Reply) + } + t.Logf("reply: %s", out.Details.Reply) +} + +// ── the approval card, end to end ─────────────────────────────────────────── + +func TestLiveAnApprovalCardRoundTripsAndWrites(t *testing.T) { + // The one path that has never run whole. The model proposes, the card comes + // back signed, the console sends it in unchanged, and only then does + // anything change. Each half has unit tests; this is the join. + app, shop := buildApp(t, liveHTTPChat(t)) + token := webSession(t) + + status, out, body := post(t, app, askPath, token, + `{"agent":"inventory","question":"Approve stock request 41."}`) + if status != fiber.StatusOK { + t.Fatalf("asking: HTTP %d: %s", status, body) + } + if out.Details.Awaiting == nil { + t.Fatalf("no approval card came back — nothing to press: %s", out.Details.Reply) + } + card := out.Details.Awaiting.Card + t.Logf("card: %s", out.Details.Awaiting.Summary) + + // Nothing may have happened yet. A write at proposal time is the failure + // the whole two-step exists to prevent. + if len(shop.approved) != 0 { + t.Fatalf("the change was made before anybody agreed to it: %v", shop.approved) + } + + status, done, body := post(t, app, approvePath, token, + `{"agent":"inventory","card":`+quote(card)+`}`) + if status != fiber.StatusOK { + t.Fatalf("approving: HTTP %d: %s", status, body) + } + if len(shop.approved) != 1 || shop.approved[0] != "Approved #41" { + t.Fatalf("the write did not reach the service: %v", shop.approved) + } + t.Logf("after approval: %s", done.Details.Reply) + + // Pressing twice must not approve twice. The card still verifies; the row + // is no longer pending, and the re-check at execute time is what notices. + status, _, _ = post(t, app, approvePath, token, + `{"agent":"inventory","card":`+quote(card)+`}`) + if status == fiber.StatusOK { + t.Fatal("the same card approved the same request twice") + } + if len(shop.approved) != 1 { + t.Fatalf("a second write got through: %v", shop.approved) + } +} + +func TestAForgedCardIsRefused(t *testing.T) { + // No model needed: a card that does not verify must be refused before + // anything reads what it claims. + app, shop := buildApp(t, nil) + + status, _, body := post(t, app, approvePath, webSession(t), + `{"agent":"inventory","card":"w1.bm90LWEtcmVhbC1jYXJk.c2lnbmF0dXJl"}`) + + if status == fiber.StatusOK { + t.Fatalf("a forged card was accepted: %s", body) + } + if len(shop.approved) != 0 { + t.Fatalf("a forged card changed something: %v", shop.approved) + } +} diff --git a/facade/container.go b/facade/container.go index ee6b792..284bc11 100644 --- a/facade/container.go +++ b/facade/container.go @@ -47,7 +47,7 @@ type Facade struct { // it may be nil if catalogue env vars are not configured, in which case // catalogue endpoints will error at query time rather than at startup. // embedder may be nil too: scan-to-order then matches on words alone. -func NewFacade(db *gorm.DB, catalogueDB *gorm.DB, embedder utils.Embedder, chat utils.Chat, agentsDir string) *Facade { +func NewFacade(db *gorm.DB, catalogueDB *gorm.DB, embedder utils.Embedder, chat utils.Chat, agentsDir, assistantWhy string) *Facade { // User Module userRepo := repositories.NewUserRepository(db) @@ -182,6 +182,11 @@ func NewFacade(db *gorm.DB, catalogueDB *gorm.DB, embedder utils.Embedder, chat log.Printf("assistant: %d agents loaded %v", len(agents), services.AgentNames(agents)) assistantService := services.NewAssistantService(toolRegistry, chat, agents) + // Why there is no model, if there is not. Passed through so /assistant/status + // can name the missing variable instead of just saying no. + if setter, ok := assistantService.(interface{ SetUnavailableReason(string) }); ok && chat == nil { + setter.SetUnavailableReason(assistantWhy) + } assistantController := controllers.NewAssistantController(assistantService) // The second door. Same registry, same agents, same session — see diff --git a/main.go b/main.go index 982bbb4..4183672 100644 --- a/main.go +++ b/main.go @@ -434,7 +434,7 @@ func main() { log.Fatal("assistant provider:", err) } if chat == nil { - log.Println("assistant: ASSISTANT_PROVIDER not set, Nearle Buddy answers no typed questions") + log.Printf("assistant: OFF — %s", cfg.Assistant.Why()) } else { log.Printf("assistant: %s, balanced tier is %s", cfg.Assistant.Provider, cfg.Assistant.ModelFor(utils.TierBalanced)) } @@ -442,7 +442,7 @@ func main() { // ASSISTANT_AGENTS_DIR replaces the compiled-in agent definitions wholesale. // Empty uses the embedded ones, so a deployment cannot be broken by a missing // directory. - f := facade.NewFacade(db.DB, db.CatalogueDB, embedder, chat, os.Getenv("ASSISTANT_AGENTS_DIR")) + f := facade.NewFacade(db.DB, db.CatalogueDB, embedder, chat, os.Getenv("ASSISTANT_AGENTS_DIR"), cfg.Assistant.Why()) routes.RegisterRoutes(app, f) diff --git a/routes/startup_test.go b/routes/startup_test.go index 5313e8f..75a2c59 100644 --- a/routes/startup_test.go +++ b/routes/startup_test.go @@ -42,7 +42,7 @@ func testFacade(t *testing.T) *facade.Facade { // No database, no catalogue, no embedder, no model. A deployment with none // of those must still boot and say what it is missing, rather than failing // somewhere the operator cannot see. - return facade.NewFacade(nil, nil, nil, nil, "") + return facade.NewFacade(nil, nil, nil, nil, "", "no model in tests") } func TestTheServerCanBeBuilt(t *testing.T) { diff --git a/services/assistantLive_test.go b/services/assistantLive_test.go index 7dbf144..8bbeb30 100644 --- a/services/assistantLive_test.go +++ b/services/assistantLive_test.go @@ -41,14 +41,25 @@ import ( func liveChat(t *testing.T) (utils.Chat, string) { t.Helper() + // Provider defaults exactly as production does, so this exercises the + // defaulting rather than working around it. The first version read the + // variable straight and skipped silently the moment `ASSISTANT_PROVIDER` + // was left unset — which is precisely the configuration this is meant to + // prove works. + provider := strings.ToLower(strings.TrimSpace(os.Getenv("ASSISTANT_PROVIDER"))) + model := os.Getenv("ASSISTANT_MODEL") + if provider == "" && strings.TrimSpace(model) != "" { + provider = "openai" + } + cfg := config.AssistantConfig{ - Provider: strings.ToLower(strings.TrimSpace(os.Getenv("ASSISTANT_PROVIDER"))), + Provider: provider, BaseURL: os.Getenv("ASSISTANT_BASE_URL"), APIKey: os.Getenv("ASSISTANT_API_KEY"), - Balanced: os.Getenv("ASSISTANT_MODEL"), + Balanced: model, } - if cfg.APIKey == "" || !cfg.Enabled() { - t.Skip("no ASSISTANT_* provider configured; skipping the live model test") + if !cfg.Enabled() { + t.Skipf("no model configured, skipping the live test: %s", cfg.Why()) } chat, err := utils.NewChat(cfg) diff --git a/services/assistantService.go b/services/assistantService.go index d867364..5b97d9f 100644 --- a/services/assistantService.go +++ b/services/assistantService.go @@ -97,12 +97,19 @@ type AssistantService interface { Approve(ctx context.Context, agentName, card string, caller tools.Caller) (AssistantAnswer, error) // Available reports whether typed questions work at all here. Available() bool + // Unavailable says WHY not, or "" when it is available. A disabled composer + // with no reason is indistinguishable from a misspelled variable, which is + // how this stayed off without anybody being able to tell. + Unavailable() string } type assistantService struct { registry *tools.Registry chat utils.Chat agents map[string]Agent + // Why the model is absent, from config. Carried rather than recomputed so + // the answer the endpoint gives is the one the server actually started with. + why string // How fast one person may ask. Only questions are limited — approving a // change the person has already read costs nothing and must not be the call // that gets refused. @@ -120,6 +127,19 @@ func NewAssistantService(registry *tools.Registry, chat utils.Chat, agents map[s func (s *assistantService) Available() bool { return s.chat != nil } +func (s *assistantService) Unavailable() string { + if s.chat != nil { + return "" + } + if s.why != "" { + return s.why + } + return "no assistant model is configured" +} + +// SetUnavailableReason records why there is no model, for the status endpoint. +func (s *assistantService) SetUnavailableReason(why string) { s.why = why } + func (s *assistantService) Approve(ctx context.Context, agentName, card string, caller tools.Caller) (AssistantAnswer, error) { agent, known := s.agents[agentName] if !known {