refactor: remove unused guards and tests, restructure tool utilities

- Deleted unused guard functions (isString, isFunction, isBigInt) from guards.ts.
- Removed tool-loader.spec.ts and tool-validator.test.ts as they were no longer needed.
- Consolidated tool-related exports into a new tools/index.ts file for better organization.
- Introduced ToolErrorHandler for standardized error handling during tool execution.
- Implemented DefaultToolFactory for loading tool modules dynamically.
- Added comprehensive tests for ToolLoader, DefaultToolRegistrar, and tool metadata validation.
- Updated tools-manifest.json to reflect changes in tool parameters and descriptions.

Author: Coderrob

.eslintrc.json

@@ -26,6 +26,7 @@
   },
   "rules": {
     "@typescript-eslint/no-explicit-any": "error",
+    "@typescript-eslint/explicit-function-return-type": "error",
     "@typescript-eslint/strict-boolean-expressions": "warn",
     "@typescript-eslint/no-unused-vars": [
       "error",

jest.config.js

@@ -1,9 +1,13 @@
 /** @type {import('ts-jest').JestConfigWithTsJest} **/
-module.exports = {
-  preset: 'ts-jest',
+export default {
+  preset: 'ts-jest/presets/default-esm',
   testEnvironment: 'node',
+  extensionsToTreatAsEsm: ['.ts'],
+  moduleNameMapper: {
+    '^(\\.{1,2}/.*)\\.js$': '$1',
+  },
   transform: {
-    '^.+\\.tsx?$': ['ts-jest', { tsconfig: 'tsconfig.spec.json' }],
+    '^.+\\.tsx?$': ['ts-jest', { useESM: true, tsconfig: 'tsconfig.spec.json' }],
   },
   testPathIgnorePatterns: ['<rootDir>/dist/'],
   collectCoverageFrom: ['src/**/*.{ts,js}', '!src/**/*.spec.ts', '!src/**/*.test.ts', '!src/**/__fixtures__/**/*'],

package.json

@@ -1,5 +1,6 @@
 {
   "author": "Robert Lindley",
+  "type": "module",
   "dependencies": {
     "@backstage/catalog-client": "^1.9.1",
     "@backstage/catalog-model": "^1.7.3",

src/api/backstage-catalog-api.ts

@@ -18,21 +18,17 @@ import {
 import { CompoundEntityRef, Entity, stringifyEntityRef } from '@backstage/catalog-model';
 import axios, { AxiosInstance, isAxiosError } from 'axios';
 
-import { type AuthConfig } from '../auth';
-import { AuthManager } from '../auth/auth-manager';
-import { securityAuditor, SecurityEventType } from '../auth/security-auditor';
-import { CacheManager } from '../cache/cache-manager';
-import { logger } from '../utils';
-import { isString } from '../utils/guards';
-import { JsonApiDocument, JsonApiFormatter } from '../utils/jsonapi-formatter';
-import { PaginationHelper, PaginationParams } from '../utils/pagination-helper';
+import { AuthConfig, AuthManager, securityAuditor } from '../auth';
+import { CacheManager } from '../cache';
+import { IBackstageCatalogApi, JsonApiDocument, PaginationParams, SecurityEventType } from '../types';
+import { isNonEmptyString, isNumber, isString, JsonApiFormatter, logger, PaginationHelper } from '../utils';
 
 interface BackstageCatalogApiOptions {
   baseUrl: string;
   auth: AuthConfig;
 }
 
-export class BackstageCatalogApi {
+export class BackstageCatalogApi implements IBackstageCatalogApi {
   private readonly client: AxiosInstance;
   private readonly authManager: AuthManager;
   private readonly cacheManager: CacheManager;
@@ -60,16 +56,13 @@ export class BackstageCatalogApi {
 
         // Add authorization header if provided
         const authHeader = await this.authManager.getAuthorizationHeader();
-        if (typeof authHeader === 'string' && authHeader.length > 0) {
+        if (isNonEmptyString(authHeader)) {
           config.headers.Authorization = authHeader;
         }
 
         // Log security event with explicit fallbacks
-        const resource = typeof config.url === 'string' && config.url.length > 0 ? String(config.url) : 'unknown';
-        const action =
-          typeof config.method === 'string' && config.method.length > 0
-            ? String(config.method).toUpperCase()
-            : 'UNKNOWN';
+        const resource = isNonEmptyString(config.url) ? String(config.url) : 'unknown';
+        const action = isNonEmptyString(config.method) ? String(config.method).toUpperCase() : 'UNKNOWN';
 
         securityAuditor.logEvent({
           type: SecurityEventType.AUTH_SUCCESS,
@@ -85,20 +78,11 @@ export class BackstageCatalogApi {
         let resource: string = 'unknown';
         let action = 'UNKNOWN';
         if (isAxiosError(error)) {
-          resource =
-            typeof error.config?.url === 'string' && error.config?.url!.length > 0
-              ? String(error.config!.url)
-              : resource;
-          action =
-            typeof error.config?.method === 'string' && error.config?.method!.length > 0
-              ? String(error.config!.method).toUpperCase()
-              : action;
+          resource = isNonEmptyString(error.config?.url) ? String(error.config!.url) : resource;
+          action = isNonEmptyString(error.config?.method) ? String(error.config!.method).toUpperCase() : action;
         } else {
-          resource = typeof config.url === 'string' && config.url.length > 0 ? String(config.url) : resource;
-          action =
-            typeof config.method === 'string' && config.method.length > 0
-              ? String(config.method).toUpperCase()
-              : action;
+          resource = isNonEmptyString(config.url) ? String(config.url) : resource;
+          action = isNonEmptyString(config.method) ? String(config.method).toUpperCase() : action;
         }
 
         securityAuditor.logEvent({
@@ -118,14 +102,10 @@ export class BackstageCatalogApi {
       (error) => {
         // Log security events for certain error types. Narrow to axios errors first
         if (isAxiosError(error)) {
-          const resource =
-            typeof error.config?.url === 'string' && error.config?.url!.length > 0
-              ? String(error.config!.url)
-              : 'unknown';
-          const action =
-            typeof error.config?.method === 'string' && error.config?.method!.length > 0
-              ? String(error.config!.method).toUpperCase()
-              : 'UNKNOWN';
+          const resource = isNonEmptyString(error.config?.url) ? String(error.config!.url) : 'unknown';
+          const action = isNonEmptyString(error.config?.method)
+            ? String(error.config!.method).toUpperCase()
+            : 'UNKNOWN';
 
           if (error.response?.status === 401) {
             securityAuditor.logEvent({
@@ -148,19 +128,15 @@ export class BackstageCatalogApi {
           // Fallback for non-axios errors that may still have a response shape
           const maybe = error as unknown as { response?: { status?: number } } | undefined;
           const maybeResponse = maybe && maybe.response ? maybe.response : undefined;
-          if (maybeResponse !== undefined && typeof maybeResponse.status === 'number' && maybeResponse.status === 401) {
+          if (isNumber(maybeResponse?.status) && maybeResponse.status === 401) {
             securityAuditor.logEvent({
               type: SecurityEventType.UNAUTHORIZED_ACCESS,
               resource: 'unknown',
               action: 'UNKNOWN',
               success: false,
               errorMessage: 'Unauthorized access',
             });
-          } else if (
-            maybeResponse !== undefined &&
-            typeof maybeResponse.status === 'number' &&
-            maybeResponse.status === 429
-          ) {
+          } else if (isNumber(maybeResponse?.status) && maybeResponse.status === 429) {
             securityAuditor.logEvent({
               type: SecurityEventType.RATE_LIMIT_EXCEEDED,
               resource: 'unknown',
@@ -344,7 +320,6 @@ export class BackstageCatalogApi {
    */
   async getEntitiesJsonApi(request?: GetEntitiesRequest & PaginationParams): Promise<JsonApiDocument> {
     const entities = await this.getEntities(request);
-
     // Convert to JSON:API format
     return JsonApiFormatter.entitiesToDocument(entities.items ?? []);
   }

src/api/index.ts

@@ -0,0 +1 @@
+export { BackstageCatalogApi } from './backstage-catalog-api';

src/auth/auth-manager.ts

@@ -1,6 +1,6 @@
 import axios, { AxiosResponse } from 'axios';
 
-import { logger } from '../utils';
+import { isNonEmptyString, isNumber, logger } from '../utils';
 
 export interface AuthConfig {
   type: 'bearer' | 'oauth' | 'api-key' | 'service-account';
@@ -92,7 +92,7 @@ export class AuthManager {
   }
 
   private async handleBearerToken(): Promise<TokenInfo> {
-    if (typeof this.config.token !== 'string' || this.config.token.length === 0) {
+    if (!isNonEmptyString(this.config.token)) {
       throw new Error('Bearer token not configured');
     }
     return {
@@ -103,21 +103,14 @@ export class AuthManager {
 
   private async handleOAuthRefresh(): Promise<TokenInfo> {
     if (
-      typeof this.config.clientId !== 'string' ||
-      this.config.clientId.length === 0 ||
-      typeof this.config.clientSecret !== 'string' ||
-      this.config.clientSecret.length === 0 ||
-      typeof this.config.tokenUrl !== 'string' ||
-      this.config.tokenUrl.length === 0
+      !isNonEmptyString(this.config.clientId) ||
+      !isNonEmptyString(this.config.clientSecret) ||
+      !isNonEmptyString(this.config.tokenUrl)
     ) {
       throw new Error('OAuth configuration incomplete');
     }
 
-    if (
-      !this.tokenInfo ||
-      typeof this.tokenInfo.refreshToken !== 'string' ||
-      this.tokenInfo.refreshToken.length === 0
-    ) {
+    if (!isNonEmptyString(this.tokenInfo?.refreshToken)) {
       throw new Error('No refresh token available for OAuth');
     }
 
@@ -132,7 +125,7 @@ export class AuthManager {
   }
 
   private async handleApiKey(): Promise<TokenInfo> {
-    if (typeof this.config.apiKey !== 'string' || this.config.apiKey.length === 0) {
+    if (!isNonEmptyString(this.config.apiKey)) {
       throw new Error('API key not configured');
     }
     return {
@@ -142,7 +135,7 @@ export class AuthManager {
   }
 
   private async handleServiceAccount(): Promise<TokenInfo> {
-    if (typeof this.config.serviceAccountKey !== 'string' || this.config.serviceAccountKey.length === 0) {
+    if (!isNonEmptyString(this.config.serviceAccountKey)) {
       throw new Error('Service account key not configured');
     }
 
@@ -162,15 +155,13 @@ export class AuthManager {
       token_type?: string;
     };
     const expiresAt =
-      typeof data.expires_in === 'number' && !Number.isNaN(data.expires_in)
-        ? Date.now() + data.expires_in * 1000
-        : undefined;
+      isNumber(data.expires_in) && !Number.isNaN(data.expires_in) ? Date.now() + data.expires_in * 1000 : undefined;
 
     return {
       accessToken: data.access_token,
       refreshToken: data.refresh_token,
       expiresAt,
-      tokenType: typeof data.token_type === 'string' && data.token_type.length > 0 ? data.token_type : 'Bearer',
+      tokenType: isNonEmptyString(data.token_type) ? data.token_type : 'Bearer',
     };
   }
 

src/auth/index.ts

@@ -1,4 +1,4 @@
 /* eslint-disable import/no-unused-modules */
 export { type AuthConfig, AuthManager, type TokenInfo } from './auth-manager';
 export { InputSanitizer, inputSanitizer } from './input-sanitizer';
-export { SecurityAuditor, securityAuditor, type SecurityEvent, SecurityEventType } from './security-auditor';
+export { SecurityAuditor, securityAuditor } from './security-auditor';

src/auth/input-sanitizer.ts

@@ -1,5 +1,7 @@
 import { z } from 'zod';
 
+import { isObject, isString } from '../utils';
+
 export class InputSanitizer {
   private readonly maxStringLength = 10000;
   private readonly maxArrayLength = 1000;
@@ -9,7 +11,7 @@ export class InputSanitizer {
    * Sanitizes a string input
    */
   sanitizeString(input: string, fieldName: string): string {
-    if (typeof input !== 'string') {
+    if (!isString(input)) {
       throw new Error(`Invalid input type for ${fieldName}: expected string, got ${typeof input}`);
     }
 
@@ -58,11 +60,11 @@ export class InputSanitizer {
   sanitizeEntityRef(
     entityRef: string | { kind: string; namespace: string; name: string }
   ): string | { kind: string; namespace: string; name: string } {
-    if (typeof entityRef === 'string') {
+    if (isString(entityRef)) {
       return this.sanitizeString(entityRef, 'entityRef');
     }
 
-    if (typeof entityRef === 'object' && entityRef !== null) {
+    if (isObject(entityRef)) {
       return {
         kind: this.sanitizeString(entityRef.kind, 'entityRef.kind'),
         namespace: this.sanitizeString(entityRef.namespace, 'entityRef.namespace'),
@@ -134,4 +136,5 @@ export class InputSanitizer {
 }
 
 // Global input sanitizer instance
+/* eslint-disable-next-line import/no-unused-modules */
 export const inputSanitizer = new InputSanitizer();

src/auth/security-auditor.ts

@@ -1,27 +1,6 @@
 import { z } from 'zod';
 
-export enum SecurityEventType {
-  AUTH_SUCCESS = 'auth_success',
-  AUTH_FAILURE = 'auth_failure',
-  TOKEN_REFRESH = 'token_refresh',
-  RATE_LIMIT_EXCEEDED = 'rate_limit_exceeded',
-  INVALID_REQUEST = 'invalid_request',
-  UNAUTHORIZED_ACCESS = 'unauthorized_access',
-}
-
-export interface SecurityEvent {
-  id: string;
-  timestamp: Date;
-  type: SecurityEventType;
-  userId?: string;
-  ipAddress?: string;
-  userAgent?: string;
-  resource: string;
-  action: string;
-  success: boolean;
-  details?: Record<string, unknown>;
-  errorMessage?: string;
-}
+import { ISecurityEvent, SecurityEventType } from '../types';
 
 const SecurityEventSchema = z.object({
   id: z.string(),
@@ -38,11 +17,11 @@ const SecurityEventSchema = z.object({
 });
 
 export class SecurityAuditor {
-  private events: SecurityEvent[] = [];
+  private events: ISecurityEvent[] = [];
   private readonly maxEvents = 10000; // Keep last 10k events in memory
 
-  logEvent(event: Omit<SecurityEvent, 'id' | 'timestamp'>): void {
-    const fullEvent: SecurityEvent = {
+  logEvent(event: Omit<ISecurityEvent, 'id' | 'timestamp'>): void {
+    const fullEvent: ISecurityEvent = {
       ...event,
       id: this.generateEventId(),
       timestamp: new Date(),
@@ -61,7 +40,7 @@ export class SecurityAuditor {
     // Log to console in development (use warn so eslint no-console rule permits it)
     if (process.env.NODE_ENV !== 'production') {
       console.warn(
-        `[SECURITY] ${event.type}: ${event.resource} - ${event.action} (${event.success ? 'SUCCESS' : 'FAILED'})`
+        `[SECURITY] ${event.type}: ${event.resource} - ${event.action} (${event.success === true ? 'SUCCESS' : 'FAILED'})`
       );
     }
 
@@ -73,15 +52,15 @@ export class SecurityAuditor {
     return `sec_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
   }
 
-  private persistEvent(_event: SecurityEvent): void {
+  private persistEvent(_event: ISecurityEvent): void {
     // In a real implementation, this would:
     // - Send to SIEM system
     // - Write to audit database
     // - Send to monitoring service
     // For now, we'll just keep in memory
   }
 
-  getEvents(filter?: { type?: SecurityEventType; userId?: string; since?: Date; limit?: number }): SecurityEvent[] {
+  getEvents(filter?: { type?: SecurityEventType; userId?: string; since?: Date; limit?: number }): ISecurityEvent[] {
     let filtered = this.events;
 
     if (filter && filter.type !== undefined) {
@@ -110,7 +89,7 @@ export class SecurityAuditor {
     authSuccessCount: number;
     authFailureCount: number;
     rateLimitCount: number;
-    recentEvents: SecurityEvent[];
+    recentEvents: ISecurityEvent[];
   } {
     const recentEvents = this.events.slice(-100); // Last 100 events
 

src/cache/cache-manager.ts

@@ -1,18 +1,5 @@
-/* eslint-disable import/no-unused-modules */
-import { logger } from '../utils/logger';
-
-export interface CacheEntry<T = unknown> {
-  data: T;
-  timestamp: number;
-  ttl: number; // Time to live in milliseconds
-  hits: number;
-}
-
-export interface CacheConfig {
-  defaultTtl: number; // Default TTL in milliseconds
-  maxSize: number; // Maximum number of entries
-  cleanupInterval: number; // Cleanup interval in milliseconds
-}
+import { CacheConfig, CacheEntry } from '../types';
+import { isNumber, logger } from '../utils';
 
 export class CacheManager {
   private cache = new Map<string, CacheEntry>();
@@ -63,12 +50,12 @@ export class CacheManager {
    */
   set<T>(key: string, value: T, ttl?: number): void {
     // Evict oldest entries if at capacity
-    if (typeof this.config.maxSize === 'number' && this.cache.size >= this.config.maxSize) {
+    if (isNumber(this.config.maxSize) && this.cache.size >= this.config.maxSize) {
       logger.debug('Cache at capacity, evicting oldest entry');
       this.evictOldest();
     }
 
-    const finalTtl = typeof ttl === 'number' && !Number.isNaN(ttl) ? ttl : this.config.defaultTtl;
+    const finalTtl = isNumber(ttl) && !Number.isNaN(ttl) ? ttl : this.config.defaultTtl;
     this.cache.set(key, {
       data: value,
       timestamp: Date.now(),
@@ -180,6 +167,3 @@ export class CacheManager {
     }
   }
 }
-
-// Global cache instance
-export const cacheManager = new CacheManager();