fix: address code review feedback on health command (DIS-144)

- Fix misleading success message: says 'Connectivity is working' instead
  of claiming API key validation (endpoint returns 200 for any key)
- Extract shared checkHealth() into src/health.ts to DRY up duplicated
  logic between CLI command and SDK HealthAPI
- Change 'type HealthResult' to 'interface HealthResult' for consistency
  with other types in src/types/
- Add getApiKeyPrefix to MockClient type instead of using Record cast
- Fix SDK test: use vi.importActual for OpenSeaAPIError so instanceof
  works correctly, enabling auth/API error branch coverage
- Add SDK tests for auth error (401) and API error (500) branches

Co-Authored-By: Chris K <ckorhonen@gmail.com>

Author: devin-ai-integration[bot]

src/commands/health.ts

@@ -1,44 +1,20 @@
 import { Command } from "commander"
-import { OpenSeaAPIError, type OpenSeaClient } from "../client.js"
+import type { OpenSeaClient } from "../client.js"
+import { checkHealth } from "../health.js"
 import type { OutputFormat } from "../output.js"
 import { formatOutput } from "../output.js"
-import type { HealthResult } from "../types/index.js"
 
 export function healthCommand(
   getClient: () => OpenSeaClient,
   getFormat: () => OutputFormat,
 ): Command {
   const cmd = new Command("health")
-    .description("Check API key validity and connectivity")
+    .description("Check API connectivity")
     .action(async () => {
       const client = getClient()
-      const keyPrefix = client.getApiKeyPrefix()
-
-      try {
-        await client.get("/api/v2/collections", { limit: 1 })
-        const result: HealthResult = {
-          status: "ok",
-          key_prefix: keyPrefix,
-          message: "API key is valid and connectivity is working",
-        }
-        console.log(formatOutput(result, getFormat()))
-      } catch (error) {
-        let message: string
-        if (error instanceof OpenSeaAPIError) {
-          message =
-            error.statusCode === 401 || error.statusCode === 403
-              ? `Authentication failed (${error.statusCode}): ${error.responseBody}`
-              : `API error (${error.statusCode}): ${error.responseBody}`
-        } else {
-          message = `Network error: ${(error as Error).message}`
-        }
-
-        const result: HealthResult = {
-          status: "error",
-          key_prefix: keyPrefix,
-          message,
-        }
-        console.log(formatOutput(result, getFormat()))
+      const result = await checkHealth(client)
+      console.log(formatOutput(result, getFormat()))
+      if (result.status === "error") {
         process.exit(1)
       }
     })

src/health.ts

@@ -0,0 +1,31 @@
+import { OpenSeaAPIError, type OpenSeaClient } from "./client.js"
+import type { HealthResult } from "./types/index.js"
+
+export async function checkHealth(
+  client: OpenSeaClient,
+): Promise<HealthResult> {
+  const keyPrefix = client.getApiKeyPrefix()
+  try {
+    await client.get("/api/v2/collections", { limit: 1 })
+    return {
+      status: "ok",
+      key_prefix: keyPrefix,
+      message: "Connectivity is working",
+    }
+  } catch (error) {
+    let message: string
+    if (error instanceof OpenSeaAPIError) {
+      message =
+        error.statusCode === 401 || error.statusCode === 403
+          ? `Authentication failed (${error.statusCode}): ${error.responseBody}`
+          : `API error (${error.statusCode}): ${error.responseBody}`
+    } else {
+      message = `Network error: ${(error as Error).message}`
+    }
+    return {
+      status: "error",
+      key_prefix: keyPrefix,
+      message,
+    }
+  }
+}

src/index.ts

@@ -1,4 +1,5 @@
 export { OpenSeaAPIError, OpenSeaClient } from "./client.js"
+export { checkHealth } from "./health.js"
 export type { OutputFormat } from "./output.js"
 export { formatOutput } from "./output.js"
 export { OpenSeaCLI } from "./sdk.js"

src/sdk.ts

@@ -1,4 +1,5 @@
-import { OpenSeaAPIError, OpenSeaClient } from "./client.js"
+import { OpenSeaClient } from "./client.js"
+import { checkHealth } from "./health.js"
 import type {
   Account,
   AssetEvent,
@@ -392,29 +393,6 @@ class HealthAPI {
   constructor(private client: OpenSeaClient) {}
 
   async check(): Promise<HealthResult> {
-    const keyPrefix = this.client.getApiKeyPrefix()
-    try {
-      await this.client.get("/api/v2/collections", { limit: 1 })
-      return {
-        status: "ok",
-        key_prefix: keyPrefix,
-        message: "API key is valid and connectivity is working",
-      }
-    } catch (error) {
-      let message: string
-      if (error instanceof OpenSeaAPIError) {
-        message =
-          error.statusCode === 401 || error.statusCode === 403
-            ? `Authentication failed (${error.statusCode}): ${error.responseBody}`
-            : `API error (${error.statusCode}): ${error.responseBody}`
-      } else {
-        message = `Network error: ${(error as Error).message}`
-      }
-      return {
-        status: "error",
-        key_prefix: keyPrefix,
-        message,
-      }
-    }
+    return checkHealth(this.client)
   }
 }

src/types/index.ts

@@ -15,7 +15,7 @@ export interface CommandOptions {
   raw?: boolean
 }
 
-export type HealthResult = {
+export interface HealthResult {
   status: "ok" | "error"
   key_prefix: string
   message: string

test/commands/health.test.ts

@@ -5,13 +5,9 @@ import { type CommandTestContext, createCommandTestContext } from "../mocks.js"
 
 describe("healthCommand", () => {
   let ctx: CommandTestContext
-  let mockGetApiKeyPrefix: ReturnType<typeof vi.fn>
 
   beforeEach(() => {
     ctx = createCommandTestContext()
-    mockGetApiKeyPrefix = vi.fn().mockReturnValue("test...")
-    ;(ctx.mockClient as Record<string, unknown>).getApiKeyPrefix =
-      mockGetApiKeyPrefix
   })
 
   afterEach(() => {
@@ -35,7 +31,7 @@ describe("healthCommand", () => {
     const output = JSON.parse(ctx.consoleSpy.mock.calls[0][0] as string)
     expect(output.status).toBe("ok")
     expect(output.key_prefix).toBe("test...")
-    expect(output.message).toBe("API key is valid and connectivity is working")
+    expect(output.message).toBe("Connectivity is working")
   })
 
   it("outputs error status on authentication failure", async () => {

test/mocks.ts

@@ -5,6 +5,7 @@ import type { OutputFormat } from "../src/output.js"
 export type MockClient = {
   get: Mock
   post: Mock
+  getApiKeyPrefix: Mock
 }
 
 export type CommandTestContext = {
@@ -18,6 +19,7 @@ export function createCommandTestContext(): CommandTestContext {
   const mockClient: MockClient = {
     get: vi.fn(),
     post: vi.fn(),
+    getApiKeyPrefix: vi.fn().mockReturnValue("test..."),
   }
   const getClient = () => mockClient as unknown as OpenSeaClient
   const getFormat = () => "json" as OutputFormat

test/sdk.test.ts

@@ -1,14 +1,18 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
-import { OpenSeaClient } from "../src/client.js"
+import { OpenSeaAPIError, OpenSeaClient } from "../src/client.js"
 import { OpenSeaCLI } from "../src/sdk.js"
 
-vi.mock("../src/client.js", () => {
+vi.mock("../src/client.js", async importOriginal => {
+  const actual = await importOriginal<typeof import("../src/client.js")>()
   const MockOpenSeaClient = vi.fn()
   MockOpenSeaClient.prototype.get = vi.fn()
   MockOpenSeaClient.prototype.post = vi.fn()
   MockOpenSeaClient.prototype.getDefaultChain = vi.fn(() => "ethereum")
   MockOpenSeaClient.prototype.getApiKeyPrefix = vi.fn(() => "test...")
-  return { OpenSeaClient: MockOpenSeaClient, OpenSeaAPIError: vi.fn() }
+  return {
+    OpenSeaClient: MockOpenSeaClient,
+    OpenSeaAPIError: actual.OpenSeaAPIError,
+  }
 })
 
 describe("OpenSeaCLI", () => {
@@ -419,12 +423,34 @@ describe("OpenSeaCLI", () => {
       })
       expect(result.status).toBe("ok")
       expect(result.key_prefix).toBe("test...")
-      expect(result.message).toBe(
-        "API key is valid and connectivity is working",
+      expect(result.message).toBe("Connectivity is working")
+    })
+
+    it("check returns error on authentication failure", async () => {
+      mockGet.mockRejectedValue(
+        new OpenSeaAPIError(401, "Unauthorized", "/api/v2/collections"),
       )
+      const result = await sdk.health.check()
+      expect(result.status).toBe("error")
+      expect(result.key_prefix).toBe("test...")
+      expect(result.message).toContain("Authentication failed (401)")
+    })
+
+    it("check returns error on API error", async () => {
+      mockGet.mockRejectedValue(
+        new OpenSeaAPIError(
+          500,
+          "Internal Server Error",
+          "/api/v2/collections",
+        ),
+      )
+      const result = await sdk.health.check()
+      expect(result.status).toBe("error")
+      expect(result.key_prefix).toBe("test...")
+      expect(result.message).toContain("API error (500)")
     })
 
-    it("check returns error when API call fails", async () => {
+    it("check returns error when network fails", async () => {
       mockGet.mockRejectedValue(new Error("fetch failed"))
       const result = await sdk.health.check()
       expect(result.status).toBe("error")