[DOJ-2175/2177/2178] feat: rewrite MarketplaceService with real API calls by dbejarano820 · Pull Request #6 · DojoCodingLabs/dojo-marketplace-mcp

dbejarano820 · 2026-02-23T19:27:36Z

Summary

DOJ-2177: Wires per-request API key injection from HTTP transport to MarketplaceService via setApiKey(). Removes apiKey parameter threading from install() and private helpers — all methods now use the instance field.
DOJ-2175: Adds private callApi() method that POSTs to the marketplace-mcp-api Edge Function. Replaces all stub service methods (search, listCategories, getDetails, getItemDetail, uninstall, listInstalled) with real API calls. Updates MarketplaceCategory type to { name, slug, description }.
DOJ-2178: Implements marketplace_list_categories with real data — maps Edge Function's ['skill', 'plugin', 'tool'] response to full category objects with display names and descriptions.

Linear

Closes DOJ-2175, DOJ-2177, DOJ-2178

Test plan

npm test passes (24 tests)
npm run typecheck passes
npm run build succeeds
HTTP transport injects API key per-request
Service methods call real Edge Function endpoints

🤖 Generated with Claude Code

Add apiKey field and setApiKey() method to MarketplaceService so the HTTP transport can inject the caller's Bearer token per request. Remove the apiKey parameter from install() — all methods now use the instance field, falling back to DOJO_API_KEY env var at construction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add private callApi() method that POSTs to the marketplace-mcp-api Edge Function. Replace search, listCategories, getDetails, getItemDetail, uninstall, and listInstalled stubs with real callApi() invocations. Private helpers (resolveSlug, getVersionRecord, incrementDownloadCount) now use this.apiKey instead of parameter threading. Update tests to mock fetch for all API-calling methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

greptile-apps · 2026-02-23T19:30:38Z

Greptile Summary

Replaces MarketplaceService stub methods with real Edge Function API calls via a new callApi() helper. Adds MethodOptions parameter to all public methods for per-request API key injection. Updates MarketplaceCategory type from {id, name, itemCount} to {name, slug, description}. Adds 30s timeouts to all fetch calls.

Critical issue: DOJ-2177's per-request API key injection is incomplete. http.ts:27 stores authCtx.apiKey in res.locals.dojoApiKey, but tool handlers never retrieve it or pass it to service methods via the options parameter. All service calls use this.apiKey (the instance default from env var) instead of the per-request key, defeating the race condition fix.

Implemented real API calls for search, listCategories, getDetails, getItemDetail, uninstall, listInstalled
Added AbortSignal.timeout(30_000) to prevent hanging requests
Tests updated to mock API responses and verify correct action payloads
Empty string guard added for category slug capitalization (line 254)

Confidence Score: 1/5

Critical bug in HTTP mode: per-request credentials are validated but never used
DOJ-2177 claims to wire per-request credential injection, but the stored value is never retrieved. Tool handlers call service methods without passing the per-request value via options parameter, so all requests use the shared instance field. This creates the exact race condition DOJ-2177 intended to fix. HTTP mode will fail with auth errors or leak credentials between concurrent requests.
src/transports/http.ts must inject the validated credential into the service instance after line 27, or tool handlers must be refactored to pass it via MCP request metadata

Important Files Changed

Filename	Overview
src/transports/http.ts	Critical bug: per-request API key stored but never used, defeating the race condition fix
src/services/marketplace.service.ts	Replaces stub methods with real API calls, adds MethodOptions parameter, adds timeouts and empty string guards
src/index.ts	Passes service parameter to startHttpServer for per-request API key injection

Sequence Diagram

sequenceDiagram
    participant Client
    participant HTTP as HTTP Middleware
    participant Service as MarketplaceService
    participant API as Edge Function

    Client->>HTTP: POST /mcp + Authorization header
    HTTP->>HTTP: Extract & validate API key
    HTTP->>HTTP: Store in res.locals.dojoApiKey
    Note over HTTP,Service: ❌ API key NOT passed to service
    HTTP->>Service: Call tool handler
    Service->>Service: Use this.apiKey (default/env)
    Service->>API: POST with wrong API key
    API-->>Service: Response
    Service-->>HTTP: Result
    HTTP-->>Client: Response

_{Last reviewed commit: a4f85da}

greptile-apps

_{5 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T19:30:41Z

+
+    return data.map((slug) => {
+      const display = CATEGORY_DISPLAY[slug] ?? {
+        name: slug.charAt(0).toUpperCase() + slug.slice(1),


Empty string handling - if slug is an empty string, .charAt(0) returns empty string and capitalization fails silently

Suggested change

name: slug.charAt(0).toUpperCase() + slug.slice(1),

name: slug ? slug.charAt(0).toUpperCase() + slug.slice(1) : "Unknown",

Prompt To Fix With AI

This is a comment left during a code review. Path: src/services/marketplace.service.ts Line: 244 Comment: Empty string handling - if `slug` is an empty string, `.charAt(0)` returns empty string and capitalization fails silently ```suggestion name: slug ? slug.charAt(0).toUpperCase() + slug.slice(1) : "Unknown", ``` How can I resolve this? If you propose a fix, please make it concise.

- Add per-call apiKey parameter to all public MarketplaceService methods to prevent API key leakage between concurrent HTTP requests - Add AbortSignal.timeout(30s) to resolveSlug, getVersionRecord, and incrementDownloadCount fetch calls - Add empty string guard in listCategories slug mapping [DOJ-2175] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace `declare global { namespace Express }` with `res.locals` to satisfy @typescript-eslint/no-namespace ESLint rule. [DOJ-2175] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dbejarano820 · 2026-02-23T22:56:56Z

@greptile review again

greptile-apps

_{5 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T23:00:26Z

-export async function startHttpServer(server: McpServer): Promise<void> {
+export async function startHttpServer(
+  server: McpServer,
+  _service: MarketplaceService,


_service parameter is marked as unused but the race condition fix is incomplete

The underscore prefix indicates this parameter is intentionally unused, but DOJ-2177's goal was to inject the per-request API key. Since tool handlers don't access res.locals.dojoApiKey, calling _service.setApiKey(authCtx.apiKey) here would fix the race condition (though still has timing issues if multiple tools are called in parallel within one request).

Prompt To Fix With AI

This is a comment left during a code review. Path: src/transports/http.ts Line: 11 Comment: `_service` parameter is marked as unused but the race condition fix is incomplete The underscore prefix indicates this parameter is intentionally unused, but DOJ-2177's goal was to inject the per-request API key. Since tool handlers don't access `res.locals.dojoApiKey`, calling `_service.setApiKey(authCtx.apiKey)` here would fix the race condition (though still has timing issues if multiple tools are called in parallel within one request). How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-23T23:00:27Z

      return;
    }
+    // Store on res.locals — no shared-state mutation, no type augmentation
+    res.locals.dojoApiKey = authCtx.apiKey;


stored value is never used — tool handlers don't pass it to service methods

The value stored on line 27 is not retrieved or passed to MarketplaceService methods. Tool handlers in browse.tools.ts and install.tools.ts call service methods like service.search(args) without passing the per-request value via options, so they'll use the default instance field instead.

Fix: call _service.setApiKey(authCtx.apiKey) after line 27 to inject the per-request value into the service instance.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/transports/http.ts Line: 27 Comment: stored value is never used — tool handlers don't pass it to service methods The value stored on line 27 is not retrieved or passed to `MarketplaceService` methods. Tool handlers in `browse.tools.ts` and `install.tools.ts` call service methods like `service.search(args)` without passing the per-request value via options, so they'll use the default instance field instead. Fix: call `_service.setApiKey(authCtx.apiKey)` after line 27 to inject the per-request value into the service instance. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-23T23:00:28Z

+
+    return data.map((slug) => {
+      const display = CATEGORY_DISPLAY[slug] ?? {
+        name: slug ? slug.charAt(0).toUpperCase() + slug.slice(1) : "Unknown",


empty string guard added (already noted in previous review thread)

The ternary slug ? ... : "Unknown" handles the empty string case mentioned in the previous review. Consider whether "Unknown" is the right fallback or if empty strings should be filtered from the API response entirely.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review. Path: src/services/marketplace.service.ts Line: 254 Comment: empty string guard added (already noted in previous review thread) The ternary `slug ? ... : "Unknown"` handles the empty string case mentioned in the previous review. Consider whether "Unknown" is the right fallback or if empty strings should be filtered from the API response entirely. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-23T23:00:29Z

-        status: response.status,
-        statusText: response.statusText,
-      });
+      const error = await response.json().catch(() => ({}));


error parsing assumes JSON — will throw if Edge Function returns HTML or plain text on error

.json().catch(() => ({})) falls back to empty object, so the error message becomes generic API error: {status} instead of showing the actual error text. Consider .text() fallback for better error messages.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/services/marketplace.service.ts Line: 200 Comment: error parsing assumes JSON — will throw if Edge Function returns HTML or plain text on error `.json().catch(() => ({}))` falls back to empty object, so the error message becomes generic `API error: {status}` instead of showing the actual error text. Consider `.text()` fallback for better error messages. How can I resolve this? If you propose a fix, please make it concise.

dbejarano820 · 2026-02-27T00:01:56Z

🤖 This PR has had no activity for 3 days.

If it's still needed, please:

Resolve any conflicts
Address review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-02-27T00:02:11Z

@greptileai what is missing for this to be a 5/5 PR?

greptile-apps · 2026-02-27T00:03:04Z

Looking at the PR and review comments, here's what's missing for 5/5:

Critical Issue:
The per-request API key injection (DOJ-2177) is incomplete. The code stores authCtx.apiKey in res.locals.dojoApiKey (http.ts:27) but never uses it. Tool handlers call service methods without passing the per-request key, so all requests share the same instance field — the race condition still exists.

Fix: Call service.setApiKey(authCtx.apiKey) in the middleware after line 27, or pass { apiKey: res.locals.dojoApiKey } from tool handlers to service methods.

Secondary Issues:

Error handling (marketplace.service.ts:200): Falls back to empty object when response isn't JSON, losing error details. Use .text() fallback for better error messages.
Empty string guard (marketplace.service.ts:254): Works but consider filtering empty slugs at the API level rather than mapping them to "Unknown".

Test Plan:
The two unchecked items won't pass until the API key injection is fixed:

☐ HTTP transport injects API key per-request
☐ Service methods call real Edge Function endpoints

Once the API key wiring is complete and those items are verified, this will be a solid 5/5.

dbejarano820 · 2026-03-03T00:04:06Z

🤖 This PR has had no activity for 7 days.

If it's still needed, please:

Resolve any conflicts
Address review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-03-06T00:02:46Z

🤖 This PR has had no activity for 10 days.

If it's still needed, please:

Resolve any conflicts
Address review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-03-10T18:13:54Z

🤖 This PR has had no activity for 15 days.

If it's still needed, please:

Resolve any conflicts
Address review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-03-11T00:04:12Z

🤖 This PR has had no activity for 15 days.

If it's still needed, please:

Resolve any conflicts
Address review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-03-14T00:02:59Z

🤖 This PR has had no activity for 3 days.

If it's still needed, please:

Address Greptile review feedback (score: 1/5)
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-03-17T00:04:35Z

🤖 This PR has had no activity for 3 days.

If it's still needed, please:

Address Greptile review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 · 2026-03-20T00:04:57Z

🤖 This PR has had no activity for 24+ days.

If it's still needed, please:

Resolve any conflicts
Address review feedback
Push an update

If it's no longer needed, please close it.

— Dojito (automated PR gardener)

dbejarano820 and others added 2 commits February 23, 2026 13:24

greptile-apps Bot reviewed Feb 23, 2026

View reviewed changes

dbejarano820 and others added 2 commits February 23, 2026 16:17

fix: use res.locals instead of namespace for ESLint compliance

a4f85da

Replace `declare global { namespace Express }` with `res.locals` to satisfy @typescript-eslint/no-namespace ESLint rule. [DOJ-2175] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

greptile-apps Bot reviewed Feb 23, 2026

View reviewed changes

	name: slug.charAt(0).toUpperCase() + slug.slice(1),
	name: slug ? slug.charAt(0).toUpperCase() + slug.slice(1) : "Unknown",

Conversation

dbejarano820 commented Feb 23, 2026

Summary

Linear

Test plan

Uh oh!

greptile-apps Bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 1/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

dbejarano820 commented Feb 23, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

dbejarano820 commented Feb 27, 2026

Uh oh!

dbejarano820 commented Feb 27, 2026

Uh oh!

greptile-apps Bot commented Feb 27, 2026

Uh oh!

dbejarano820 commented Mar 3, 2026

Uh oh!

dbejarano820 commented Mar 6, 2026

Uh oh!

dbejarano820 commented Mar 10, 2026

Uh oh!

dbejarano820 commented Mar 11, 2026

Uh oh!

dbejarano820 commented Mar 14, 2026

Uh oh!

dbejarano820 commented Mar 17, 2026

Uh oh!

dbejarano820 commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Feb 23, 2026 •

edited

Loading