package bitrix24 import ( "context" "encoding/json" "errors" "net/http" "net/http/httptest" "strings" "sync" "testing" "time" "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/bus" "github.com/nextlevelbuilder/goclaw/internal/store" ) // fakeMCPStore implements store.MCPServerStore for provisioner tests. // Only the methods provisionIfMissing actually exercises are implemented // with behaviour — the rest satisfy the interface with zero returns so the // test file compiles cleanly. // // All tests that touch fakeMCPStore pass a *preloaded* server row because // GetServerByName is what initMCPProvisioner calls at startup; the other // fields exist to be mutated by SetUserCredentials and observed by the // test assertions. type fakeMCPStore struct { mu sync.Mutex serversByName map[string]*store.MCPServerData serversByID map[uuid.UUID]*store.MCPServerData userCreds map[string]store.MCPUserCredentials // key = serverID + ":" + userID getUserCallCount int setUserCallCount int } func newFakeMCPStore() *fakeMCPStore { return &fakeMCPStore{ serversByName: map[string]*store.MCPServerData{}, serversByID: map[uuid.UUID]*store.MCPServerData{}, userCreds: map[string]store.MCPUserCredentials{}, } } func credKey(serverID uuid.UUID, userID string) string { return serverID.String() + ":" + userID } // --- MCPServerStore methods that matter for the provisioner -------------- func (f *fakeMCPStore) GetServerByName(_ context.Context, name string) (*store.MCPServerData, error) { f.mu.Lock() defer f.mu.Unlock() if s, ok := f.serversByName[name]; ok { return s, nil } return nil, nil // partner's contract: nil + nil when absent } func (f *fakeMCPStore) GetUserCredentials(_ context.Context, serverID uuid.UUID, userID string) (*store.MCPUserCredentials, error) { f.mu.Lock() defer f.mu.Unlock() f.getUserCallCount++ if c, ok := f.userCreds[credKey(serverID, userID)]; ok { return &c, nil } return nil, nil } func (f *fakeMCPStore) SetUserCredentials(_ context.Context, serverID uuid.UUID, userID string, creds store.MCPUserCredentials) error { f.mu.Lock() defer f.mu.Unlock() f.setUserCallCount++ f.userCreds[credKey(serverID, userID)] = creds return nil } // --- MCPServerStore methods the provisioner doesn't touch ---------------- func (f *fakeMCPStore) CreateServer(_ context.Context, _ *store.MCPServerData) error { return nil } func (f *fakeMCPStore) GetServer(_ context.Context, id uuid.UUID) (*store.MCPServerData, error) { f.mu.Lock() defer f.mu.Unlock() if s, ok := f.serversByID[id]; ok { return s, nil } return nil, nil // partner's contract: nil + nil when absent } func (f *fakeMCPStore) ListServers(_ context.Context) ([]store.MCPServerData, error) { return nil, nil } func (f *fakeMCPStore) UpdateServer(_ context.Context, _ uuid.UUID, _ map[string]any) error { return nil } func (f *fakeMCPStore) DeleteServer(_ context.Context, _ uuid.UUID) error { return nil } func (f *fakeMCPStore) GrantToAgent(_ context.Context, _ *store.MCPAgentGrant) error { return nil } func (f *fakeMCPStore) RevokeFromAgent(_ context.Context, _, _ uuid.UUID) error { return nil } func (f *fakeMCPStore) ListAgentGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { return nil, nil } func (f *fakeMCPStore) ListServerGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { return nil, nil } func (f *fakeMCPStore) GrantToUser(_ context.Context, _ *store.MCPUserGrant) error { return nil } func (f *fakeMCPStore) RevokeFromUser(_ context.Context, _ uuid.UUID, _ string) error { return nil } func (f *fakeMCPStore) CountAgentGrantsByServer(_ context.Context) (map[uuid.UUID]int, error) { return nil, nil } func (f *fakeMCPStore) ListAccessible(_ context.Context, _ uuid.UUID, _ string) ([]store.MCPAccessInfo, error) { return nil, nil } func (f *fakeMCPStore) CreateRequest(_ context.Context, _ *store.MCPAccessRequest) error { return nil } func (f *fakeMCPStore) ListPendingRequests(_ context.Context) ([]store.MCPAccessRequest, error) { return nil, nil } func (f *fakeMCPStore) ReviewRequest(_ context.Context, _ uuid.UUID, _ bool, _, _ string) error { return nil } func (f *fakeMCPStore) DeleteUserCredentials(_ context.Context, _ uuid.UUID, _ string) error { return nil } func (f *fakeMCPStore) CacheToolDescriptions(_ context.Context, _ uuid.UUID, _ map[string]store.CachedToolInfo) error { return nil } // --- Test helpers -------------------------------------------------------- // newProvisionerTestChannel builds a started Channel with provisioning // enabled against the given (fake MCP server URL, fake MCP store). Used // by the happy-path tests that need the full provisioner wired up. func newProvisionerTestChannel(t *testing.T, mcpStore *fakeMCPStore, mcpBaseURL string, botType string) *Channel { t.Helper() fs := newFakeStore() resetWebhookRouterForTest() t.Cleanup(resetWebhookRouterForTest) // Seed the fake MCP store so initMCPProvisioner's GetServerByName // succeeds. ID doesn't need to match anything real — provisioner just // caches and passes it through. serverID := uuid.New() mcpStore.serversByName["bitrix-mcp"] = &store.MCPServerData{ BaseModel: store.BaseModel{ID: serverID}, Name: "bitrix-mcp", } fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") cfgJSON := `{"portal":"p","bot_code":"c","bot_name":"n","bot_type":"` + botType + `","mcp_server_name":"bitrix-mcp","mcp_base_url":"` + mcpBaseURL + `"}` ch, err := fn("b1", nil, json.RawMessage(cfgJSON), bus.New(), nil) if err != nil { t.Fatalf("factory: %v", err) } bc := ch.(*Channel) bc.SetTenantID(store.GenNewID()) // Bypass Start() — set the wiring initMCPProvisioner would set. // Calling Start() would also try to hit Bitrix portal which we don't // want in these focused tests. bc.startMu.Lock() bc.botID = 1 bc.client = NewClient("p.bitrix24.com", nil) bc.startMu.Unlock() if err := bc.initMCPProvisioner(context.Background()); err != nil { t.Fatalf("initMCPProvisioner: %v", err) } return bc } // validAuth returns an EventAuth with every field provisionIfMissing // needs populated so individual tests can focus on the outcome, not // fixture plumbing. func validAuth() EventAuth { return EventAuth{ Domain: "acme.bitrix24.com", AccessToken: "at-tok", RefreshToken: "rt-tok", ExpiresIn: 3600, } } // --- Tests --------------------------------------------------------------- // TestProvisionIfMissing_OpenChannelBot_Skipped verifies bot_type=O short- // circuits the provisioner without touching the MCP store. This is a // Phase C skip (not failure) and is the expected outcome for every // message delivered to an Open Channel bot. func TestProvisionIfMissing_OpenChannelBot_Skipped(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() mcpStore := newFakeMCPStore() // Don't seed a server — if provisioner tries to use it we'll catch the // mistake via the nil-check, but this test is really about the early // IsOpenChannelBot() exit. fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") ch, err := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","bot_type":"O","mcp_server_name":"bitrix-mcp","mcp_base_url":"http://example.test"}`), bus.New(), nil) if err != nil { t.Fatalf("factory: %v", err) } bc := ch.(*Channel) // IS_CONNECTOR=Y (external connector customer) → skipped: not a Bitrix user. err = bc.provisionIfMissing(context.Background(), "42", true, validAuth(), "chat123") if !errors.Is(err, ErrProvisionSkippedOpenChannel) { t.Fatalf("connector message: err = %v; want ErrProvisionSkippedOpenChannel", err) } if mcpStore.getUserCallCount != 0 { t.Errorf("Open Channel connector message must not hit MCP store; got %d GetUserCredentials calls", mcpStore.getUserCallCount) } // IS_CONNECTOR=N (internal staff in an Open Channel) → must NOT connector-skip. if err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123"); errors.Is(err, ErrProvisionSkippedOpenChannel) { t.Fatalf("internal staff (IS_CONNECTOR=N) must not be connector-skipped; got %v", err) } } // TestProvisionIfMissing_Disabled verifies a channel built via the // no-MCP factory variant (or with half-config) reports Disabled without // touching anything external. func TestProvisionIfMissing_Disabled(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() // FactoryWithPortalStore (2-arg) leaves mcpStore nil → provisioning // stays disabled regardless of config fields. fn := FactoryWithPortalStore(fs, "") ch, err := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n"}`), bus.New(), nil) if err != nil { t.Fatalf("factory: %v", err) } bc := ch.(*Channel) err = bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123") if !errors.Is(err, ErrProvisionDisabled) { t.Fatalf("err = %v; want ErrProvisionDisabled", err) } } // TestProvisionIfMissing_ExistingCreds_NoHTTP ensures the warm path skips // auto-onboard entirely when the user already has credentials. This is // the hottest path — all subsequent messages from the same user — so // verifying it does NOT re-hit the HTTP endpoint protects against a // regression that'd show up as "burning 1 HTTP request per message". func TestProvisionIfMissing_ExistingCreds_NoHTTP(t *testing.T) { httpCalls := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { httpCalls++ w.WriteHeader(http.StatusInternalServerError) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") // Preload credentials for user 42 before the first provision attempt. // Warm path requires BITRIX_EXPIRES_AT meta (added 260512 C3 fix). Legacy // rows without expiry meta now actively refresh on next event to write the // meta column — see TestProvisionIfMissing_LegacyNoExpiry_RefreshHTTP. warmExpiry := time.Now().UTC().Add(30 * time.Minute).Format(time.RFC3339) mcpStore.userCreds[credKey(bc.mcpServerID, "42")] = store.MCPUserCredentials{ APIKey: "prior-key", Env: map[string]string{ "BITRIX_EXPIRES_AT": warmExpiry, }, } if err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123"); err != nil { t.Fatalf("err = %v; want nil", err) } if httpCalls != 0 { t.Errorf("warm path must not call auto-onboard; got %d HTTP calls", httpCalls) } if mcpStore.setUserCallCount != 0 { t.Errorf("warm path must not re-persist; got %d SetUserCredentials calls", mcpStore.setUserCallCount) } } // TestProvisionIfMissing_NearExpiry_RefreshHTTP locks the C3 (Phase 3) fix: // when cached BITRIX_EXPIRES_AT is within mcpCredsRefreshWindow (5 min) of // now, the provisioner MUST refresh proactively — preventing the upcoming // tool call from racing a stale token. Without this, a user chatting just // before expiry would hit 401 on the tool call and need a follow-up message // to recover. func TestProvisionIfMissing_NearExpiry_RefreshHTTP(t *testing.T) { httpCalls := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { httpCalls++ w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"api_key":"refreshed-key","user_id":"u-42","tenant_id":"t-1","created":false}`)) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") // Preload creds with expiry 2 minutes in the future (inside refresh window). nearExpiry := time.Now().UTC().Add(2 * time.Minute).Format(time.RFC3339) mcpStore.userCreds[credKey(bc.mcpServerID, "42")] = store.MCPUserCredentials{ APIKey: "stale-key", Env: map[string]string{ "BITRIX_EXPIRES_AT": nearExpiry, }, } if err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123"); err != nil { t.Fatalf("err = %v; want nil", err) } if httpCalls != 1 { t.Errorf("near-expiry path must refresh once; got %d HTTP calls", httpCalls) } if mcpStore.setUserCallCount != 1 { t.Errorf("near-expiry path must persist refreshed creds; got %d SetUserCredentials calls", mcpStore.setUserCallCount) } } // TestProvisionIfMissing_LegacyNoExpiry_RefreshHTTP locks the 260512 fix: // rows without BITRIX_EXPIRES_AT meta (legacy onboards before C3) MUST be // refreshed once to write expiry meta. Without this, mcp-bx-syn rejects with // 401 when its stored token expires (1h TTL after onboard) → loop-side // purge breaks the in-flight conversation (observed user 1 group chat 2150). func TestProvisionIfMissing_LegacyNoExpiry_RefreshHTTP(t *testing.T) { httpCalls := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { httpCalls++ w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"api_key":"refreshed-key","user_id":"u-42","tenant_id":"t-1","created":false}`)) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") // Legacy row: APIKey set, NO BITRIX_EXPIRES_AT. mcpStore.userCreds[credKey(bc.mcpServerID, "42")] = store.MCPUserCredentials{ APIKey: "legacy-key", } if err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123"); err != nil { t.Fatalf("err = %v; want nil", err) } if httpCalls != 1 { t.Errorf("legacy path must refresh once; got %d HTTP calls", httpCalls) } if mcpStore.setUserCallCount != 1 { t.Errorf("legacy path must persist refreshed creds; got %d SetUserCredentials", mcpStore.setUserCallCount) } } // TestProvisionIfMissing_WarmExpiry_NoHTTP locks the inverse of C3: when // cached expiry is comfortably beyond the refresh window (e.g. 30 min away), // provisioner MUST skip HTTP. Refreshing on every event when token is still // fresh would burn 1 HTTP per message and DDoS mcp-bx-syn. func TestProvisionIfMissing_WarmExpiry_NoHTTP(t *testing.T) { httpCalls := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { httpCalls++ w.WriteHeader(http.StatusInternalServerError) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") warmExpiry := time.Now().UTC().Add(30 * time.Minute).Format(time.RFC3339) mcpStore.userCreds[credKey(bc.mcpServerID, "42")] = store.MCPUserCredentials{ APIKey: "warm-key", Env: map[string]string{ "BITRIX_EXPIRES_AT": warmExpiry, }, } if err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123"); err != nil { t.Fatalf("err = %v; want nil", err) } if httpCalls != 0 { t.Errorf("warm-expiry path must not call HTTP; got %d HTTP calls", httpCalls) } } // TestProvisionIfMissing_MintAndPersist covers the full happy path: no // existing creds → auto-onboard call → credential saved with OAuth tokens // in Env. This is the core value-add of Phase C. func TestProvisionIfMissing_MintAndPersist(t *testing.T) { var gotReq autoOnboardRequest srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _ = json.NewDecoder(r.Body).Decode(&gotReq) w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"api_key":"minted-k","user_id":"mu","tenant_id":"mt","created":true}`)) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") before := time.Now() err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123") if err != nil { t.Fatalf("provisionIfMissing: %v", err) } // Request body reached the MCP server verbatim. if gotReq.Domain != "acme.bitrix24.com" || gotReq.BitrixUserID != "42" || gotReq.AccessToken != "at-tok" || gotReq.RefreshToken != "rt-tok" { t.Errorf("unexpected request to MCP server: %+v", gotReq) } // Credential was persisted with API key + OAuth Env. stored, ok := mcpStore.userCreds[credKey(bc.mcpServerID, "42")] if !ok { t.Fatalf("credentials were not persisted") } if stored.APIKey != "minted-k" { t.Errorf("APIKey = %q; want minted-k", stored.APIKey) } for _, key := range []string{"BITRIX_DOMAIN", "BITRIX_ACCESS_TOKEN", "BITRIX_REFRESH_TOKEN", "BITRIX_EXPIRES_AT"} { if _, has := stored.Env[key]; !has { t.Errorf("Env missing %q (full Env: %v)", key, stored.Env) } } if stored.Env["BITRIX_DOMAIN"] != "acme.bitrix24.com" { t.Errorf("Env[BITRIX_DOMAIN] = %q; want acme.bitrix24.com", stored.Env["BITRIX_DOMAIN"]) } // Sanity-check EXPIRES_AT is ~now + expires_in, not some fossil. parsed, err := time.Parse(time.RFC3339, stored.Env["BITRIX_EXPIRES_AT"]) if err != nil { t.Fatalf("BITRIX_EXPIRES_AT not RFC3339: %v", err) } expectedMin := before.Add(3599 * time.Second) expectedMax := before.Add(3601 * time.Second) if parsed.Before(expectedMin) || parsed.After(expectedMax) { t.Errorf("BITRIX_EXPIRES_AT out of expected window (%v–%v): got %v", expectedMin, expectedMax, parsed) } } // TestProvisionIfMissing_Debounce verifies that after one attempt (success // OR failure), a second attempt within mcpProvisionDebounceTTL returns // ErrProvisionDebounced without calling the MCP server again. Critical // guard against Bitrix24 webhook retry storms. func TestProvisionIfMissing_Debounce(t *testing.T) { httpCalls := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { httpCalls++ w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"api_key":"k","user_id":"u","tenant_id":"t","created":true}`)) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") // First attempt succeeds and marks the debounce. if err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123"); err != nil { t.Fatalf("first attempt: %v", err) } if httpCalls != 1 { t.Fatalf("first attempt should call HTTP once, got %d", httpCalls) } // Second attempt within TTL → debounced. We need to wipe the stored // credential first; otherwise the "existing creds" short-circuit // returns nil before the debounce check fires. (This ordering is // documented in provisionIfMissing; the test makes it observable.) mcpStore.mu.Lock() delete(mcpStore.userCreds, credKey(bc.mcpServerID, "42")) mcpStore.mu.Unlock() err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123") if !errors.Is(err, ErrProvisionDebounced) { t.Fatalf("second attempt: %v; want ErrProvisionDebounced", err) } if httpCalls != 1 { t.Errorf("debounced attempt must not hit HTTP; got %d total calls", httpCalls) } } // TestProvisionIfMissing_HTTPFailure_Surfaces ensures auto-onboard errors // propagate out of provisionIfMissing (wrapped, but distinguishable). The // caller in handle.go swallows these after logging, but tests need to see // them to assert the error path exists. func TestProvisionIfMissing_HTTPFailure_Surfaces(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) _, _ = w.Write([]byte(`{"error":"invalid_bitrix_user"}`)) })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") err := bc.provisionIfMissing(context.Background(), "42", false, validAuth(), "chat123") if err == nil { t.Fatal("401 from MCP must produce an error") } // We don't assert exact string, but do check the caller can tell this // is NOT one of the expected no-op sentinels — otherwise handle.go // would misclassify it as "skipped" and hide the 401 in Debug logs. if errors.Is(err, ErrProvisionDisabled) || errors.Is(err, ErrProvisionDebounced) || errors.Is(err, ErrProvisionSkippedOpenChannel) { t.Errorf("HTTP 401 should not match any no-op sentinel; got %v", err) } // Underlying message should mention the status so operators can debug. if !strings.Contains(err.Error(), "auto-onboard failed") { t.Errorf("err = %v; expected wrapped 'auto-onboard failed' prefix", err) } } // TestProvisionIfMissing_MissingAuthBlock catches the case where the // event somehow reached handleMessage with partial auth data. Should // fail BEFORE calling the MCP server (saves an HTTP round-trip that // we know will fail validation client-side). func TestProvisionIfMissing_MissingAuthBlock(t *testing.T) { httpCalls := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { httpCalls++ })) defer srv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, srv.URL, "B") cases := []struct { name string auth EventAuth }{ {"empty_domain", EventAuth{AccessToken: "a", RefreshToken: "r"}}, {"empty_access_token", EventAuth{Domain: "d", RefreshToken: "r"}}, {"empty_refresh_token", EventAuth{Domain: "d", AccessToken: "a"}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { before := httpCalls // Use a fresh userID per subcase so the debounce from a prior // case doesn't mask a regression. err := bc.provisionIfMissing(context.Background(), tc.name, false, tc.auth, "chat123") if err == nil { t.Fatalf("missing %s should fail", tc.name) } if httpCalls != before { t.Errorf("incomplete auth must not hit HTTP; got +%d calls", httpCalls-before) } }) } } // attachTestPortal wires a real *Portal (OAuth calls routed to oauthSrv) onto // an already-built provisioner test channel, plus a derivable encKey — needed // for the ErrUserAuthRequired branch tests below, which call // Channel.BuildUserAuthorizeURL (requires portal.PublicURL() + a valid key). func attachTestPortal(t *testing.T, bc *Channel, oauthSrv *httptest.Server) { t.Helper() bc.encKey = testOAuthEncKey portalFS := newFakeStore() portal := newTestPortal(t, oauthSrv, portalFS, bc.TenantID(), "p", store.BitrixPortalState{PublicURL: "https://goclaw.example.com"}) bc.startMu.Lock() bc.portal = portal bc.startMu.Unlock() } // TestProvisionIfMissing_NewUser_ReturnsAuthRequired covers the brand-new-user // branch (existing == nil, no auth in the event): must return // *ErrUserAuthRequired with a non-empty URL, WITHOUT calling the MCP server // or attempting a Bitrix token refresh (there's nothing to refresh). func TestProvisionIfMissing_NewUser_ReturnsAuthRequired(t *testing.T) { mcpSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Error("brand-new user with no auth in event must not hit MCP auto-onboard") })) defer mcpSrv.Close() oauthSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Error("brand-new user must not attempt a Bitrix token refresh either") })) defer oauthSrv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, mcpSrv.URL, "B") attachTestPortal(t, bc, oauthSrv) err := bc.provisionIfMissing(context.Background(), "999", false, EventAuth{}, "chat123") var authErr *ErrUserAuthRequired if !errors.As(err, &authErr) { t.Fatalf("err = %v, want *ErrUserAuthRequired", err) } if authErr.URL == "" { t.Error("ErrUserAuthRequired.URL must not be empty") } } // TestProvisionIfMissing_DeadRefreshToken_ReturnsAuthRequired_RowUntouched // covers the "existing user, refresh_token is dead" branch: Bitrix rejects // the refresh with invalid_grant → must escalate to *ErrUserAuthRequired // WITHOUT deleting the existing mcp_user_credentials row (design.md §12 — // SetUserCredentials upserts in place once the user re-authorizes; deleting // first would be unnecessary churn). func TestProvisionIfMissing_DeadRefreshToken_ReturnsAuthRequired_RowUntouched(t *testing.T) { mcpSrv := httptest.NewServer(mcpAutoOnboardHandler()) defer mcpSrv.Close() oauthSrv := httptest.NewServer(oauthTokenHandler(0, true)) // invalid_grant defer oauthSrv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, mcpSrv.URL, "B") attachTestPortal(t, bc, oauthSrv) staleCreds := store.MCPUserCredentials{ APIKey: "old-key", Env: map[string]string{ "BITRIX_DOMAIN": "portal.bitrix24.com", "BITRIX_ACCESS_TOKEN": "old-access", "BITRIX_REFRESH_TOKEN": "dead-refresh", "BITRIX_EXPIRES_AT": time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), }, } mcpStore.mu.Lock() mcpStore.userCreds[credKey(bc.mcpServerID, "1058")] = staleCreds mcpStore.mu.Unlock() err := bc.provisionIfMissing(context.Background(), "1058", false, EventAuth{}, "chat123") var authErr *ErrUserAuthRequired if !errors.As(err, &authErr) { t.Fatalf("err = %v, want *ErrUserAuthRequired", err) } mcpStore.mu.Lock() got, ok := mcpStore.userCreds[credKey(bc.mcpServerID, "1058")] mcpStore.mu.Unlock() if !ok { t.Fatal("existing row must NOT be deleted on dead-token classification") } if got.APIKey != "old-key" { t.Errorf("row was modified; APIKey = %q, want unchanged %q", got.APIKey, "old-key") } } // TestProvisionIfMissing_TransientRefreshError_NotAuthRequired ensures a // non-classified refresh failure (network/5xx — here a bare 500 with no // Bitrix `error` field, so APIError.Code == "") does NOT escalate to // *ErrUserAuthRequired — it must fall through to the existing generic error // path (handle.go's notifyUserOfMCPIssueOnce), since a retry might just work // without bothering the user for re-authorization. func TestProvisionIfMissing_TransientRefreshError_NotAuthRequired(t *testing.T) { mcpSrv := httptest.NewServer(mcpAutoOnboardHandler()) defer mcpSrv.Close() oauthSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte(`{}`)) })) defer oauthSrv.Close() mcpStore := newFakeMCPStore() bc := newProvisionerTestChannel(t, mcpStore, mcpSrv.URL, "B") attachTestPortal(t, bc, oauthSrv) staleCreds := store.MCPUserCredentials{ Env: map[string]string{ "BITRIX_DOMAIN": "portal.bitrix24.com", "BITRIX_ACCESS_TOKEN": "old-access", "BITRIX_REFRESH_TOKEN": "still-alive-refresh", "BITRIX_EXPIRES_AT": time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), }, } mcpStore.mu.Lock() mcpStore.userCreds[credKey(bc.mcpServerID, "42")] = staleCreds mcpStore.mu.Unlock() err := bc.provisionIfMissing(context.Background(), "42", false, EventAuth{}, "chat123") var authErr *ErrUserAuthRequired if errors.As(err, &authErr) { t.Fatalf("transient 5xx must NOT escalate to ErrUserAuthRequired, got %v", err) } if err == nil { t.Fatal("expected a transient error, got nil") } } // TestInitMCPProvisioner_DisabledModes covers the configurations that // leave the provisioner off at startup (all non-fatal): // - nil MCPServerStore // - mcp_server_name points at a server that doesn't exist in the store // // (Half-config — only one of mcp_server_name / mcp_base_url set — // is rejected earlier at factory load; see TestFactory_HalfConfigRejected. // Both-empty is accepted with provisioning off, covered by // TestProvisionIfMissing_Disabled.) // // Each case should leave mcpClient nil + mcpServerID zero, so // provisionIfMissing returns ErrProvisionDisabled. // // Auto-onboard auth note: there is no longer an admin-token branch to test — // the MCP server authenticates each /api/auto-onboard call via the // caller-supplied Bitrix access_token, not a shared bearer. func TestInitMCPProvisioner_DisabledModes(t *testing.T) { t.Run("nil_mcp_store", func(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() fn := FactoryWithPortalStoreAndMCP(fs, nil, "") ch, _ := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_name":"x","mcp_base_url":"http://x"}`), bus.New(), nil) bc := ch.(*Channel) if err := bc.initMCPProvisioner(context.Background()); err != nil { t.Fatalf("init: %v", err) } if bc.mcpClient != nil || bc.mcpServerID != uuid.Nil { t.Errorf("nil mcpStore should leave provisioner off") } }) t.Run("server_not_found", func(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() mcpStore := newFakeMCPStore() // Intentionally do NOT seed serversByName — GetServerByName returns nil. fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") ch, _ := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_name":"missing","mcp_base_url":"http://x"}`), bus.New(), nil) bc := ch.(*Channel) if err := bc.initMCPProvisioner(context.Background()); err != nil { t.Fatalf("init: %v", err) } if bc.mcpClient != nil || bc.mcpServerID != uuid.Nil { t.Errorf("missing server row should leave provisioner off") } }) // mcp_server_id path (v3.15+ dashboard wiring): the UUID string must // parse and resolve to a live mcp_servers row. Broken UUID or missing // row silently disables provisioning — no fallback to legacy name. t.Run("mcp_server_id_invalid_uuid", func(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() mcpStore := newFakeMCPStore() fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") ch, _ := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_id":"not-a-uuid"}`), bus.New(), nil) bc := ch.(*Channel) if err := bc.initMCPProvisioner(context.Background()); err != nil { t.Fatalf("init: %v", err) } if bc.mcpClient != nil || bc.mcpServerID != uuid.Nil { t.Errorf("invalid mcp_server_id UUID should leave provisioner off") } }) t.Run("mcp_server_id_row_not_found", func(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() mcpStore := newFakeMCPStore() // Intentionally do NOT seed serversByID — GetServer returns nil. fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") ch, _ := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_id":"00000000-0000-0000-0000-000000000042"}`), bus.New(), nil) bc := ch.(*Channel) if err := bc.initMCPProvisioner(context.Background()); err != nil { t.Fatalf("init: %v", err) } if bc.mcpClient != nil || bc.mcpServerID != uuid.Nil { t.Errorf("missing mcp_servers row for mcp_server_id should leave provisioner off") } }) } // TestDeriveAutoOnboardBaseURL guards the fix for the Phase 2 regression // where mcp_servers.url (JSON-RPC endpoint under /mcp) was passed verbatim // to mcp_client.newMCPClient — which appends "/api/auto-onboard" to it, // producing "…/mcp/api/auto-onboard" (404). The origin extraction is the // contract the Bitrix24 auto-onboard client expects. func TestDeriveAutoOnboardBaseURL(t *testing.T) { cases := []struct { name string in string want string }{ {"jsonrpc subpath", "https://mcp.example.com/mcp", "https://mcp.example.com"}, {"jsonrpc trailing slash", "https://mcp.example.com/mcp/", "https://mcp.example.com"}, {"origin trailing slash", "https://mcp.example.com/", "https://mcp.example.com"}, {"origin only", "https://mcp.example.com", "https://mcp.example.com"}, {"deep path with query", "http://localhost:8080/some/path?x=1#frag", "http://localhost:8080"}, {"prod b24 syn mcp", "https://b24-mcp-dev.synity.so/mcp", "https://b24-mcp-dev.synity.so"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { got, err := deriveAutoOnboardBaseURL(tc.in) if err != nil { t.Fatalf("deriveAutoOnboardBaseURL(%q) unexpected error: %v", tc.in, err) } if got != tc.want { t.Errorf("deriveAutoOnboardBaseURL(%q) = %q, want %q", tc.in, got, tc.want) } }) } errCases := []struct { name string in string }{ {"empty", ""}, {"whitespace only", " \t"}, {"no scheme", "mcp.example.com/mcp"}, {"no host", "https:///path"}, } for _, tc := range errCases { t.Run("err_"+tc.name, func(t *testing.T) { if _, err := deriveAutoOnboardBaseURL(tc.in); err == nil { t.Errorf("deriveAutoOnboardBaseURL(%q) expected error, got nil", tc.in) } }) } } // TestInitMCPProvisioner_MCPServerID exercises the v3.15+ id-based wiring: // mcp_server_id resolves to a live mcp_servers row whose URL becomes the // base URL for /api/auto-onboard. Legacy MCPServerName + MCPBaseURL are // ignored on this path (single source of truth). func TestInitMCPProvisioner_MCPServerID(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() mcpStore := newFakeMCPStore() serverID := uuid.MustParse("019df803-ec13-76c3-b1f5-60b0e80d3eec") // URL carries a /mcp subpath because that's how partner MCP servers // really configure the JSON-RPC endpoint. initMCPProvisioner must // strip it before handing the value to mcp_client (which appends // /api/auto-onboard). Regression guard for the Phase 2 fix. mcpStore.serversByID[serverID] = &store.MCPServerData{ BaseModel: store.BaseModel{ID: serverID}, Name: "b24-syn-mcp", URL: "https://b24-mcp.example.test/mcp", Enabled: true, // Field promoted from settings JSONB in Phase 89 — the provisioner // reads it to log accurately and downstream provisionIfMissing // respects it via manager-level checks. RequireUserCredentials: true, } fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") cfg := `{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_id":"019df803-ec13-76c3-b1f5-60b0e80d3eec"}` ch, err := fn("b1", nil, json.RawMessage(cfg), bus.New(), nil) if err != nil { t.Fatalf("factory: %v", err) } bc := ch.(*Channel) if err := bc.initMCPProvisioner(context.Background()); err != nil { t.Fatalf("init: %v", err) } if bc.mcpClient == nil { t.Fatalf("mcp_server_id path should wire mcpClient — got nil") } if bc.mcpServerID != serverID { t.Errorf("mcpServerID = %s, want %s", bc.mcpServerID, serverID) } } // newBareChannelForNotifyTest builds a Channel that's wired enough for // notifyUserOfMCPIssueOnce tests but skips MCP provisioner setup — the // function under test only touches c.notifyMu / c.notifyDebounce and then // delegates to sendChunk, which is out of scope here (send.go owns it). // // sendChunk will fail immediately with "portal not bound" because the test // Client has no Portal attached — notifyUserOfMCPIssueOnce swallows that // error via slog.Debug, so the debounce state is still the primary // observable. Tests that care about the wire-level Send behavior should // use the full newProvisionerTestChannel + portal helper instead. func newBareChannelForNotifyTest(t *testing.T) *Channel { t.Helper() fs := newFakeStore() resetWebhookRouterForTest() t.Cleanup(resetWebhookRouterForTest) fn := FactoryWithPortalStore(fs, "") ch, err := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n"}`), bus.New(), nil) if err != nil { t.Fatalf("factory: %v", err) } bc := ch.(*Channel) bc.SetTenantID(store.GenNewID()) bc.startMu.Lock() bc.botID = 1 bc.client = NewClient("p.bitrix24.com", nil) bc.startMu.Unlock() return bc } // TestNotifyUserOfMCPIssueOnce_FirstCallMarksDebounce verifies the first // notification for a user stamps the debounce map, regardless of whether // the downstream sendChunk actually delivered the message. The debounce // stamp is what prevents a webhook retry burst from flooding the user — // if we only stamped on successful Send, a Bitrix-portal outage would // simultaneously block delivery AND disable the rate limit, giving the // user a queue of identical notices once the portal recovers. func TestNotifyUserOfMCPIssueOnce_FirstCallMarksDebounce(t *testing.T) { bc := newBareChannelForNotifyTest(t) before := time.Now() bc.notifyUserOfMCPIssueOnce(context.Background(), "user-42", "chat-9") after := time.Now() bc.notifyMu.Lock() defer bc.notifyMu.Unlock() ts, ok := bc.notifyDebounce["user-42"] if !ok { t.Fatalf("first notify did not stamp debounce map for user-42") } if ts.Before(before) || ts.After(after) { t.Errorf("debounce timestamp %v out of window [%v, %v]", ts, before, after) } } // TestNotifyUserOfMCPIssueOnce_SecondCallWithinTTLIsDebounced verifies // that a second call within mcpUserNotifyDebounceTTL does NOT refresh // the stamp. Two invariants matter: // 1. Rate limit holds — user gets exactly one notice per TTL window. // 2. Timestamp stays pinned to the FIRST call, so the window rolls // forward from there (not from every subsequent silenced call). // Otherwise a sustained outage + steady webhook retry traffic // would keep bumping the stamp forward and the user would never // see a refreshed notice when the TTL legitimately expired. func TestNotifyUserOfMCPIssueOnce_SecondCallWithinTTLIsDebounced(t *testing.T) { bc := newBareChannelForNotifyTest(t) bc.notifyUserOfMCPIssueOnce(context.Background(), "user-42", "chat-9") bc.notifyMu.Lock() firstStamp := bc.notifyDebounce["user-42"] bc.notifyMu.Unlock() // Small sleep so a naive "refresh stamp every call" bug would produce // a strictly later timestamp than firstStamp. 10ms is enough resolution // on every supported platform. time.Sleep(10 * time.Millisecond) bc.notifyUserOfMCPIssueOnce(context.Background(), "user-42", "chat-9") bc.notifyMu.Lock() defer bc.notifyMu.Unlock() secondStamp := bc.notifyDebounce["user-42"] if !secondStamp.Equal(firstStamp) { t.Errorf("debounced call should not refresh stamp: first=%v second=%v", firstStamp, secondStamp) } } // TestNotifyUserOfMCPIssueOnce_ExpiredDebounceAllowsNewNotice verifies // the stamp gets refreshed once the TTL elapses. We manipulate the // debounce map directly (planting a stale timestamp) to avoid a real // 5-minute test runtime — this is the standard pattern for testing // TTL-based caches without the wall-clock penalty. func TestNotifyUserOfMCPIssueOnce_ExpiredDebounceAllowsNewNotice(t *testing.T) { bc := newBareChannelForNotifyTest(t) // Plant a stamp that's safely outside the TTL window. stale := time.Now().Add(-(mcpUserNotifyDebounceTTL + time.Minute)) bc.notifyMu.Lock() bc.notifyDebounce = map[string]time.Time{"user-42": stale} bc.notifyMu.Unlock() bc.notifyUserOfMCPIssueOnce(context.Background(), "user-42", "chat-9") bc.notifyMu.Lock() defer bc.notifyMu.Unlock() got := bc.notifyDebounce["user-42"] if !got.After(stale) { t.Errorf("expired stamp should have been refreshed: stale=%v got=%v", stale, got) } if time.Since(got) > time.Second { t.Errorf("refreshed stamp should be ~now, got age=%v", time.Since(got)) } } // TestNotifyUserOfMCPIssueOnce_EmptyInputsAreNoop guards the two // defensive branches at the top of notifyUserOfMCPIssueOnce: empty // chatID (no reply target) and empty userID (shouldn't reach here // from handle.go but cheap to defend). Either one must short-circuit // BEFORE the debounce map is touched — otherwise a webhook with a // blank FromUserID could silently poison the "" key and prevent // legitimate future notices from firing. func TestNotifyUserOfMCPIssueOnce_EmptyInputsAreNoop(t *testing.T) { cases := []struct { name string userID string chatID string }{ {"empty_chat_id", "user-42", ""}, {"whitespace_chat_id", "user-42", " "}, {"empty_user_id", "", "chat-9"}, {"whitespace_user_id", "\t", "chat-9"}, {"both_empty", "", ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { bc := newBareChannelForNotifyTest(t) bc.notifyUserOfMCPIssueOnce(context.Background(), tc.userID, tc.chatID) bc.notifyMu.Lock() defer bc.notifyMu.Unlock() if len(bc.notifyDebounce) != 0 { t.Errorf("no-op case must leave debounce map empty, got %v", bc.notifyDebounce) } }) } } // TestNotifyUserOfMCPIssueOnce_DifferentUsersIndependent verifies per- // user debounce isolation: user A hitting the rate limit must not // silence notices for user B. This matters when a single MCP outage // affects many users — each should be independently informed on their // next message. func TestNotifyUserOfMCPIssueOnce_DifferentUsersIndependent(t *testing.T) { bc := newBareChannelForNotifyTest(t) bc.notifyUserOfMCPIssueOnce(context.Background(), "alice", "chat-1") bc.notifyUserOfMCPIssueOnce(context.Background(), "bob", "chat-2") bc.notifyMu.Lock() defer bc.notifyMu.Unlock() if _, ok := bc.notifyDebounce["alice"]; !ok { t.Errorf("alice missing from debounce map: %v", bc.notifyDebounce) } if _, ok := bc.notifyDebounce["bob"]; !ok { t.Errorf("bob missing from debounce map: %v", bc.notifyDebounce) } } // TestFactory_HalfConfigRejected codifies the "both or neither" rule for // the mcp_server_name + mcp_base_url pair so admin typos fail fast at // load rather than manifesting as a silently-disabled provisioner. func TestFactory_HalfConfigRejected(t *testing.T) { fs := newFakeStore() resetWebhookRouterForTest() defer resetWebhookRouterForTest() fn := FactoryWithPortalStoreAndMCP(fs, newFakeMCPStore(), "") cases := []struct { name string cfg string }{ {"only_server_name", `{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_name":"x"}`}, {"only_base_url", `{"portal":"p","bot_code":"c","bot_name":"n","mcp_base_url":"http://x"}`}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { _, err := fn("b1", nil, json.RawMessage(tc.cfg), bus.New(), nil) if err == nil { t.Fatal("half-config must fail at factory load") } }) } }