From 4a49bb458eb1a27f4611d5d23983082e91362940 Mon Sep 17 00:00:00 2001 From: Ajax Davis Date: Thu, 11 Dec 2025 12:22:21 +1000 Subject: [PATCH] fix(health-check): only mark tools broken for infrastructure failures Simplify execution health check logic: - If executor responds (2xx or 4xx), tool is HEALTHY - Only mark BROKEN for 5xx errors or network/timeout failures - Validation errors (URL format, missing fields) mean tool IS working - Remove brittle pattern matching for specific error messages The previous approach tried to match specific error patterns to determine if an error was "acceptable". This was fragile. The new approach: - Import check: can we load and describe the tool? - Execution check: did the tool execute at all? If a tool throws a validation error, it executed successfully - it's correctly rejecting invalid test input. Only infrastructure failures (executor down, network timeout) indicate a truly broken tool. --- .../lib/health-check/health-check-service.ts | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/apps/web/src/lib/health-check/health-check-service.ts b/apps/web/src/lib/health-check/health-check-service.ts index ec70747..6c03f25 100644 --- a/apps/web/src/lib/health-check/health-check-service.ts +++ b/apps/web/src/lib/health-check/health-check-service.ts @@ -96,6 +96,10 @@ async function checkImportHealth(tool: Tool & { package: Package }): Promise<{ /** * Check if a tool can execute with test parameters + * + * IMPORTANT: If the tool executes at all (even with errors), it's HEALTHY. + * We only mark as BROKEN for infrastructure failures (timeouts, network errors). + * Validation errors mean the tool IS working - it's correctly rejecting bad input. */ async function checkExecutionHealth(tool: Tool & { package: Package }): Promise<{ status: HealthStatus; @@ -123,43 +127,33 @@ async function checkExecutionHealth(tool: Tool & { package: Package }): Promise< }); const timeMs = Date.now() - startTime; - const data = await response.json(); - if (!response.ok || !data.success) { - const error = data.error || `HTTP ${response.status}`; - - // If error is config/input issue, tool is not broken - just needs proper input - if (isNonBreakingError(error)) { - return { - status: 'HEALTHY', - error: null, // Clear the error since it's not a code issue - timeMs, - testParams, - }; - } - - return { - status: 'BROKEN', - error, - timeMs, - testParams, - }; + // If we got a response from the executor, the tool executed + // Any error in the response is from the tool itself (validation, env, etc.) + // which means the tool IS working - it's correctly processing/rejecting input + if (response.ok) { + // Executor responded - tool executed (success or tool-level error) + return { status: 'HEALTHY', error: null, timeMs, testParams }; } + // HTTP error from executor itself (not from tool) + // This could be executor down, rate limited, etc. + const data = await response.json().catch(() => ({})); + const error = data.error || `HTTP ${response.status}`; + + // Even HTTP errors might be tool-level errors returned through executor + // Only mark as BROKEN for true infrastructure failures + if (response.status >= 500) { + return { status: 'BROKEN', error, timeMs, testParams }; + } + + // 4xx errors are likely tool-level validation/config issues return { status: 'HEALTHY', error: null, timeMs, testParams }; } catch (error) { + // Network/timeout errors are infrastructure issues const errorMessage = error instanceof Error ? error.message : 'Unknown error'; - // If error is config/input issue, tool is not broken - if (isNonBreakingError(errorMessage)) { - return { - status: 'HEALTHY', - error: null, - timeMs: Date.now() - startTime, - testParams, - }; - } - + // Timeout or network error = infrastructure issue = BROKEN return { status: 'BROKEN', error: errorMessage,