Skip to content

feat: add opensea health diagnostic command (DIS-144)#35

Open
ckorhonen wants to merge 3 commits intomainfrom
devin/1772640267-add-health-command
Open

feat: add opensea health diagnostic command (DIS-144)#35
ckorhonen wants to merge 3 commits intomainfrom
devin/1772640267-add-health-command

Conversation

@ckorhonen
Copy link

@ckorhonen ckorhonen commented Mar 4, 2026

feat: add opensea health diagnostic command (DIS-144)

Summary

Adds a new opensea health command that checks API connectivity before running expensive operations. Makes a lightweight GET /api/v2/collections?limit=1 call and outputs structured JSON:

{ "status": "ok", "key_prefix": "8d02...", "message": "Connectivity is working" }

On failure (auth error, API error, or network error), outputs the same shape with "status": "error" and a descriptive message, then exits with code 1.

Note: The collections endpoint returns 200 regardless of API key validity, so this command verifies connectivity — not key authentication. The success message reflects this.

Changes:

  • src/client.ts: Added getApiKeyPrefix() to safely expose first 4 chars of the API key
  • src/health.ts: New shared checkHealth(client) function containing all health-check logic (used by both CLI and SDK)
  • src/commands/health.ts: Thin CLI wrapper — delegates to checkHealth, formats output, exits with code 1 on error
  • src/sdk.ts: Added HealthAPI class with check() method for programmatic consumers, also delegates to checkHealth
  • src/types/index.ts: Added HealthResult interface
  • Registered command in barrel export (src/commands/index.ts), CLI (src/cli.ts), and package entry (src/index.ts)
  • test/mocks.ts: Added getApiKeyPrefix to MockClient type (no more Record<string, unknown> cast)
  • Full test coverage across command, SDK, and client layers (162 tests passing)

Confidence: 🟡 Medium-High — All 162 tests pass, lint and type-check clean. However, the command hasn't been verified against the live OpenSea API.

Resolves DIS-144

Updates since last revision

Addressed all code review feedback from @ckorhonen:

  1. Fixed misleading success message — Changed from "API key is valid and connectivity is working" to "Connectivity is working" since the endpoint returns 200 for any key
  2. DRYed up duplicated logic — Extracted shared checkHealth() into src/health.ts; both CLI command and SDK HealthAPI delegate to it
  3. Fixed typeinterfaceHealthResult now uses interface for consistency with other types in src/types/
  4. Fixed MockClient type — Added getApiKeyPrefix to MockClient in test/mocks.ts instead of using Record<string, unknown> cast
  5. Fixed SDK test mocking — Used vi.importActual to pass through real OpenSeaAPIError class so instanceof works; added SDK tests for auth error (401) and API error (500) branches

Review & Testing Checklist for Human

  • Verify real-world behavior: Run npx . health --api-key <valid_key> and npx . health --api-key invalidkey against the live API. Both should return "status": "ok" (since the endpoint doesn't validate keys). Confirm the output shape matches { status, key_prefix, message }
  • Confirm SDK path works: Instantiate OpenSeaCLI and call sdk.health.check() to verify the shared checkHealth function works through the SDK as well as the CLI

Notes


Open with Devin

Co-Authored-By: Chris K <ckorhonen@gmail.com>
@devin-ai-integration
Copy link
Contributor

Original prompt from Chris K
Please work on ticket "[opensea-cli] Add opensea health diagnostic command" ([DIS-144](https://linear.app/opensea/issue/DIS-144/opensea-cli-add-opensea-health-diagnostic-command))

PLAYBOOK_md:
# Ticket to PR

## Overview

This playbook guides the process of taking a Linear ticket from initial scoping through implementation to final review. The workflow ensures proper context gathering, quality implementation, and thorough code review before delivery. The agent uses the Linear MCP to manage ticket status and communication throughout.

## What's Needed From User

- Linear ticket URL or ticket ID (e.g., `ENG-123` or `https://linear.app/team/issue/ENG-123/...`)
- Repository access for the codebase where changes will be made

<phase name="Disambiguation" id="1">
## Disambiguation Phase

Think about the full user intent. Tickets are sometimes sparse. Make sure you disambiguate to the full scope that the user intended.

1. Fetch the ticket details using the Linear MCP `get_issue` tool with the ticket ID
2. Before diving into code: use the devin MCP to get a high-level understanding of the relevant systems and architecture. Use `ask_question` to learn about the relevant systems – send queries for multiple repos that could be relevant to get the full picture. Use `read_wiki_contents` to then get a better understanding how different parts of the codebase connect to each other.
3. Gather additional context to understand what the ticket means and refers to:
   - Look at past tickets in the same project and from the same author to understand patterns and terminology
   - Search for related commits and PRs (by author and content) that may provide context on the affected systems
   - Check any linked documents, designs, or parent tickets
   - Investigate the actual code
4. Identify any ambiguity in what the ticket refers to or asks for, including jargon or project-specific terms and use all means necessary to answer this yourself
5. Consult your smart friend: pass in the raw cont... (8102 chars truncated...)

@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration bot marked this pull request as ready for review March 4, 2026 16:09
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Copy link
Author

@ckorhonen ckorhonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Overall: Request changes — All 160 tests pass, but the health check doesn't actually validate API keys.

Critical Issue: Health check passes with invalid API keys

Confirmed via E2E testing:

$ npx . health --api-key invalidkey123
{"status":"ok","key_prefix":"inva...","message":"API key is valid and connectivity is working"}

GET /api/v2/collections?limit=1 returns 200 regardless of key validity. The health check doesn't validate authentication. The success message should be updated to reflect that this verifies connectivity, not key validity.

Other Issues

  1. SDK test mocking breaks instanceof — In test/sdk.test.ts, OpenSeaAPIError is mocked as vi.fn(), so instanceof OpenSeaAPIError never matches in the SDK's HealthAPI.check(). The auth/API error branches are untested at the SDK level. Fix: use vi.importActual to pass the real class through.

  2. type HealthResult should be interface HealthResult — Every other object shape in src/types/ uses interface. This should match for consistency.

  3. MockClient type hack — The health test patches getApiKeyPrefix onto the mock via Record<string, unknown> cast. Should be added to the MockClient type in test/mocks.ts instead.

  4. Duplicated error-handling logic between src/commands/health.ts and src/sdk.ts HealthAPI.check(). Consider having the CLI command be a thin wrapper per the project's design rules.

I've pushed a commit addressing these issues.

- 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>
@devin-ai-integration
Copy link
Contributor

Addressed all feedback in 5a7d6c2:

  1. Success message → "Connectivity is working" (no longer claims key validation)
  2. DRY → Extracted checkHealth() into src/health.ts; CLI and SDK both delegate to it
  3. typeinterface for HealthResult
  4. MockClient type → Added getApiKeyPrefix to the type, removed Record<string, unknown> cast
  5. SDK test mockingvi.importActual passes real OpenSeaAPIError through; added auth (401) and API (500) error branch tests

All 162 tests pass, lint and type-check clean.

The health check now performs two steps:
1. Connectivity check via /api/v2/collections (public endpoint)
2. Auth validation via /api/v2/listings (requires valid API key)

Previously the health check only hit a public endpoint that returned
200 regardless of API key validity, giving false "ok" results for
invalid keys.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 8 additional findings in Devin Review.

Open in Devin Review

message: `Authentication failed (${error.statusCode}): invalid API key`,
}
}
// Non-auth error on events endpoint — connectivity works but auth is unverified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Stale comment references "events endpoint" but code calls listings endpoint

The comment on line 50 says "Non-auth error on events endpoint" but the actual API call on line 29 is to /api/v2/listings/collection/boredapeyachtclub/all — a listings endpoint, not an events endpoint. This appears to be left over from a previous iteration where the health check used the events endpoint.

Root Cause

The comment was written when the Step 2 auth check used an events endpoint, but the implementation was later changed to use the listings endpoint (/api/v2/listings/collection/boredapeyachtclub/all at src/health.ts:29). The comment was not updated to match.

Impact: Minor — misleading for developers maintaining this code. No runtime effect.

Suggested change
// Non-auth error on events endpoint — connectivity works but auth is unverified
// Non-auth error on listings endpoint — connectivity works but auth is unverified
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — this is in @ckorhonen's commit (b3c0300). I can fix it if desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant