Skip to content

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

Merged
ckorhonen merged 5 commits intomainfrom
devin/1772640267-add-health-command
Mar 4, 2026
Merged

feat: add opensea health diagnostic command (DIS-144)#35
ckorhonen merged 5 commits intomainfrom
devin/1772640267-add-health-command

Conversation

@ckorhonen
Copy link
Collaborator

@ckorhonen ckorhonen commented Mar 4, 2026

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

Summary

Adds a new opensea health command that validates API connectivity and authentication before running expensive operations. Performs a two-step check:

  1. Connectivity: GET /api/v2/collections?limit=1 (public endpoint — succeeds regardless of key validity)
  2. Authentication: GET /api/v2/listings/collection/boredapeyachtclub/all?limit=1 (requires valid API key)

Outputs structured JSON:

{ "status": "ok", "key_prefix": "8d02...", "authenticated": true, "rate_limited": false, "message": "Connectivity and authentication are working" }

Exit codes: 0 = success, 1 = error (auth failure, API error, network error), 3 = rate limited (429).

If step 1 succeeds but step 2 fails with a non-auth error (e.g., 500), the command returns "status": "ok" with "authenticated": false and a message indicating auth could not be verified.

Changes:

  • src/client.ts: Added getApiKeyPrefix() — returns first 4 chars of API key, or "***" for keys shorter than 8 chars to prevent leaking short keys
  • src/health.ts: New shared checkHealth(client) function with two-step connectivity + auth validation (used by both CLI and SDK)
  • src/commands/health.ts: Thin CLI wrapper — delegates to checkHealth, formats output, exits with code 1 or 3
  • src/sdk.ts: Added HealthAPI class with check() method for programmatic consumers, delegates to checkHealth
  • src/types/index.ts: Added HealthResult interface with status, key_prefix, authenticated, rate_limited, message
  • 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
  • Full test coverage across command, SDK, and client layers (167 tests passing)

Confidence: 🟡 Medium-High — All 167 tests pass, lint and type-check clean. Not verified against the live OpenSea API.

Resolves DIS-144

Review & Testing Checklist for Human

  • E2E with valid key: Run npx . health --api-key <valid_key> — should return "status": "ok", "authenticated": true
  • E2E with invalid key: Run npx . health --api-key invalidkey123 — should return "status": "error", "authenticated": false, exit code 1
  • Hardcoded collection slug: The auth check uses boredapeyachtclub — verify this collection still exists and requires auth. If it's ever removed/renamed, the health check's auth step will break. Consider whether this should be configurable or use a different endpoint.
  • SDK path: Instantiate OpenSeaCLI and call sdk.health.check() to verify the shared checkHealth function works through the SDK

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
Collaborator 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>
devin-ai-integration[bot]

This comment was marked as resolved.

@ckorhonen
Copy link
Collaborator Author

@devin CI lint is failing on this PR. Please investigate the lint errors and push a fix. You can reproduce locally with npm run lint.

Co-Authored-By: Chris K <ckorhonen@gmail.com>
@ckorhonen
Copy link
Collaborator Author

@devin Several issues to fix on this PR:

  1. CI lint failure — Run npm run check:fix to resolve Biome formatting violations in src/health.ts, test/commands/health.test.ts, and test/sdk.test.ts. Long lines need wrapping (e.g., the boredapeyachtclub URL call expressions).

  2. API key prefix leak on short keysgetApiKeyPrefix() in src/client.ts leaks the full key if it's < 8 chars. Fix:

getApiKeyPrefix(): string {
  if (this.apiKey.length < 8) return '***'
  return `${this.apiKey.slice(0, 4)}...`
}
  1. Stale commentsrc/health.ts:48 says "events endpoint" but the code calls the listings endpoint. Update the comment to match.

  2. Missing 429 exit code test — Add a test case in test/commands/health.test.ts where mockClient.get rejects with new OpenSeaAPIError(429, ...) and assert exitSpy was called with 3.

- getApiKeyPrefix() returns '***' for keys shorter than 8 chars
- checkHealth() detects 429 responses and sets rate_limited flag
- CLI exits with code 3 on rate limiting (vs 1 for other errors)
- HealthResult interface gains rate_limited boolean field
- Added tests for 429 handling in both CLI and SDK

Co-Authored-By: Chris K <ckorhonen@gmail.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 2 new potential issues.

View 8 additional findings in Devin Review.

Open in Devin Review

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 10 additional findings in Devin Review.

Open in Devin Review

@ckorhonen ckorhonen merged commit f3d2c7a into main Mar 4, 2026
5 checks passed
@ckorhonen ckorhonen deleted the devin/1772640267-add-health-command branch March 4, 2026 20:38
ckorhonen added a commit that referenced this pull request Mar 4, 2026
The healthCommand was added to src/commands/index.ts via PR #35 but
these test files were not updated to include it in their mock.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ckorhonen added a commit that referenced this pull request Mar 4, 2026
* feat: add retry with exponential backoff in client.ts (DIS-143)

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

* fix: cancel response body on retryable errors to prevent resource leak

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

* fix: create fresh AbortSignal per retry attempt to prevent stale timeout

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

* fix: address code review feedback (POST idempotency, SDK default, stream cancel safety)

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

* fix: add healthCommand mock to CLI error handler tests

The healthCommand was added to src/commands/index.ts via PR #35 but
these test files were not updated to include it in their mock.

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

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants