feat(apify_mcp): add Apify MCP plugin - #384
Conversation
Implements the 8 OSS operations via the hosted Apify MCP server with API token auth, selective DB caching for actors/runs/outputs, and live tests. Closes corsairdev#383
|
@Ayush7614 is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds the
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Endpoint as actors/runs/docs endpoint
participant Shared as executeApifyMcpTool
participant Client as callApifyMcpTool
participant MCP as Apify MCP Server<br/>(mcp.apify.com)
participant DB as ctx.db
Caller->>Endpoint: call(ctx, input)
Endpoint->>Shared: executeApifyMcpTool(ctx, eventName, toolName, args, options)
alt "requireAuth && !ctx.key"
Shared-->>Caller: throw AuthMissingError
end
Shared->>Client: callApifyMcpTool(toolName, args, ctx.key)
Client->>MCP: "connect (StreamableHTTP)<br/>Authorization: Bearer {apiKey}"
Client->>MCP: "callTool({ name, arguments })"
MCP-->>Client: CallToolResult
Client->>Client: normalizeToolResult → parseToolResult
alt result.isError
Client-->>Shared: throw ApifyMcpAPIError
end
Client-->>Shared: T (parsed payload)
Client->>MCP: close session
alt "cache === 'actors'"
Shared->>DB: actors.upsertByEntityId(entityId, item)
else "cache === 'actorRun'"
Shared->>DB: actorRuns.upsertByEntityId(runId, payload)
else "cache === 'actorOutput' && !isPartial"
Shared->>DB: "actorOutputs.upsertByEntityId(datasetId, { output })"
end
Shared->>Shared: "logEventFromContext(completed|failed)"
Shared-->>Caller: T
Reviews (5): Last reviewed commit: "fix(apifymcp): harden cache, docs URL, a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Enforce requireAuth fail-fast via AuthMissingError, add unknown/assertion comments per plugin conventions, use instanceof in error handlers, and gate live tests on APIFY_TOKEN.
get-actor-output was removed upstream; map getActorOutput to get-dataset-items, fix actor cache key collisions, and harden live tests.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR adds the ChangesApify MCP plugin
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ApifyPlugin
participant ApifyMCP
participant CorsairDatabase
participant CorsairEventLog
Caller->>ApifyPlugin: Invoke typed endpoint
ApifyPlugin->>ApifyMCP: Send MCP tool request
ApifyMCP-->>ApifyPlugin: Return tool response
ApifyPlugin->>CorsairDatabase: Cache recognized entities
ApifyPlugin->>CorsairEventLog: Record endpoint result
ApifyPlugin-->>Caller: Return normalized response
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 0 (Call ended without gRPC status) Comment |
Resolve constants.ts (keep apify_mcp + apilabz) and regenerate lockfile.
|
@greptile review |
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ✅ | |
| R2 — Tests with assertions | ✅ | |
| R3 — Description complete | ✅ | |
| R3 — Linked issue / claim | ✅ | |
| R4 — Demo video / recording | ✅ |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
|
Hey @Ayush7614, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
Optional improvements (P2)
// Using type assertion instead of // See above — same rationale for the Partial cast. Rule Used: What: Type assertions in the plugins folder must b... (source) 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!
// |
|
@greptile review |
|
Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/apifymcp/jest.config.cjs (1)
56-57: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
forceExit: truehides open handles from the MCP transport.The comment states that Streamable HTTP sockets stay open after
client.close().forceExitsuppresses the symptom in tests, but the same unreleased transport can leak sockets in production. Run Jest with--detectOpenHandlesonce and confirm that the client tears down its transport, then keepforceExitonly if the remaining handle comes from the SDK.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apifymcp/jest.config.cjs` around lines 56 - 57, Investigate the open-handle source around the Jest configuration and MCP client teardown by running the test suite with --detectOpenHandles; ensure the client explicitly closes its Streamable HTTP transport after client.close(). Retain forceExit only if the remaining handle is confirmed to originate from the SDK, otherwise remove it.packages/apifymcp/api.test.ts (2)
59-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the auth-guard coverage to the remaining endpoints.
The guard block covers
callActor,ragWebBrowser,getActorRun, andgetActorOutput. It omitssearchActors,fetchActorDetails,searchApifyDocs, andfetchApifyDocs. These endpoints run without a token in the live block only, so a missing auth check on them stays untested. Add the four remaining cases, or drive all eight through a table-driven test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apifymcp/api.test.ts` around lines 59 - 92, Extend the “Apify MCP endpoint guards” tests to cover searchActors, fetchActorDetails, searchApifyDocs, and fetchApifyDocs, asserting each rejects with AuthMissingError when createCtx() has no API key. Keep the existing four endpoint checks intact, or consolidate all eight cases into a table-driven test using each endpoint’s required input shape.
94-98: 📐 Maintainability & Code Quality | 🔵 TrivialLive tests call the real Apify platform and consume paid compute.
callActorandragWebBrowserstart real actor runs. Therunsblock starts a second run. Each execution consumes Apify compute units on the account that owns the token. Restrict these tests to an explicit opt-in job rather than any environment that exposesAPIFY_TOKEN, or record fixtures for the default test run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apifymcp/api.test.ts` around lines 94 - 98, Restrict the live tests in the “Apify MCP API Type Tests” suite to an explicit opt-in job instead of enabling them whenever APIFY_TOKEN is present. Update the suite’s gating configuration while preserving the existing tests for intentional live-test runs, or replace the default execution with recorded fixtures.packages/apifymcp/tsconfig.json (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude test files from the declaration build.
api.test.tsis included byinclude: ["./**/*"]. Since the build runstsc --buildwithemitDeclarationOnly, it can emit test declarations and fail on test type errors. Add"**/*.test.ts"toexclude.
references: []matches the sibling plugin configurations and does not require a change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apifymcp/tsconfig.json` around lines 17 - 19, Add "**/*.test.ts" to the exclude list in the TypeScript configuration so declaration builds omit test files such as api.test.ts; preserve the existing dist and node_modules exclusions and leave references unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/apifymcp/api.test.ts`:
- Around line 7-13: Update the test setup to use Jest’s ESM mocking: replace the
jest.mock/requireActual block with jest.unstable_mockModule for corsair/core,
then dynamically import both corsair/core and the system under test after
registering the mock. Ensure the Jest invocation, including CI’s test command,
enables --experimental-vm-modules.
In `@packages/apifymcp/client.ts`:
- Around line 10-30: Update ApifyMcpAPIError and the MCP transport flow so
Retry-After is read from the HTTP response headers via a custom fetch or
transport wrapper, parsed into a delay value, and passed through the error
options to assign retryAfter. Ensure getRetryAfter() returns this parsed value
while preserving existing StreamableHTTPError status and body handling.
In `@packages/apifymcp/endpoints/shared.ts`:
- Around line 84-131: The cacheActorOutput flow currently keys all actor-output
responses only by datasetId, allowing distinct limit, offset, or fields queries
to overwrite one another. Update executeApifyMcpTool and cacheActorOutput so
only canonical complete snapshots are cached, or derive a normalized
query-specific cache identity and persist the associated query metadata; ensure
partial and filtered getActorOutput responses cannot be stored under the plain
dataset ID.
In `@packages/apifymcp/endpoints/types.ts`:
- Around line 67-72: Add the same explicit documentation-host allowlist used by
the hosted fetch-apify-docs tool to FetchApifyDocsInputSchema, applying it after
the existing URL validation so only approved Apify or Crawlee documentation
hosts are accepted while preserving HTTPS URL parsing.
---
Nitpick comments:
In `@packages/apifymcp/api.test.ts`:
- Around line 59-92: Extend the “Apify MCP endpoint guards” tests to cover
searchActors, fetchActorDetails, searchApifyDocs, and fetchApifyDocs, asserting
each rejects with AuthMissingError when createCtx() has no API key. Keep the
existing four endpoint checks intact, or consolidate all eight cases into a
table-driven test using each endpoint’s required input shape.
- Around line 94-98: Restrict the live tests in the “Apify MCP API Type Tests”
suite to an explicit opt-in job instead of enabling them whenever APIFY_TOKEN is
present. Update the suite’s gating configuration while preserving the existing
tests for intentional live-test runs, or replace the default execution with
recorded fixtures.
In `@packages/apifymcp/jest.config.cjs`:
- Around line 56-57: Investigate the open-handle source around the Jest
configuration and MCP client teardown by running the test suite with
--detectOpenHandles; ensure the client explicitly closes its Streamable HTTP
transport after client.close(). Retain forceExit only if the remaining handle is
confirmed to originate from the SDK, otherwise remove it.
In `@packages/apifymcp/tsconfig.json`:
- Around line 17-19: Add "**/*.test.ts" to the exclude list in the TypeScript
configuration so declaration builds omit test files such as api.test.ts;
preserve the existing dist and node_modules exclusions and leave references
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e8ec87e-d9d2-4387-baaa-d61bab88513f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
packages/apifymcp/api.test.tspackages/apifymcp/client.tspackages/apifymcp/endpoints/actors.tspackages/apifymcp/endpoints/docs.tspackages/apifymcp/endpoints/index.tspackages/apifymcp/endpoints/runs.tspackages/apifymcp/endpoints/shared.tspackages/apifymcp/endpoints/types.tspackages/apifymcp/error-handlers.tspackages/apifymcp/index.tspackages/apifymcp/jest.config.cjspackages/apifymcp/package.jsonpackages/apifymcp/schema/database.tspackages/apifymcp/schema/index.tspackages/apifymcp/tsconfig.jsonpackages/apifymcp/tsup.config.tspackages/corsair/core/constants.ts
| jest.mock('corsair/core', () => { | ||
| const original = jest.requireActual('corsair/core'); | ||
| return { | ||
| ...original, | ||
| logEventFromContext: jest.fn().mockResolvedValue(null), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Jest 29 jest.mock hoisting not supported native ESM unstable_mockModule ts-jest useESM
💡 Result:
In Jest 29 and later, the standard jest.mock hoisting mechanism, which works by moving calls to the top of the file during compilation, is not supported for native ECMAScript Modules (ESM) [1][2][3]. This is because ESM evaluates static import statements before the script code itself is executed [1][4]. To mock modules in an ESM environment, you must use the jest.unstable_mockModule API [1][5]. As the name implies, this API is considered experimental/unstable and is the required alternative to jest.mock for ESM [1][3]. Key technical considerations include: 1. Dynamic Imports: Because static imports are evaluated first, you cannot use static import statements for modules you intend to mock [1][4]. Instead, you must use dynamic import calls after the jest.unstable_mockModule registration to load the system under test (SUT) and the mocked dependencies [1][6]. 2. Configuration with ts-jest: When using ts-jest, you must enable the useESM: true configuration option [7][8]. This allows ts-jest to transform code into ESM syntax [8][9]. You must also ensure your Jest configuration is compatible with ESM (e.g., using correct presets like ts-jest/presets/default-esm) [7][9]. 3. API Usage: Unlike jest.mock, jest.unstable_mockModule requires a factory function (which can be sync or async) [1][2]. The basic pattern for a test file is: - Perform necessary imports (from @jest/globals, etc.) [6]. - Call jest.unstable_mockModule('module-path', factory) [1][6]. - Use dynamic await import('./path-to-sut') to load your code [1][6]. Note: The jest.unstable_mockModule API has known limitations, including potential issues with virtual mocks being scoped correctly outside of the immediate test file [10]. For mocking CommonJS (CJS) modules within an ESM project, you may continue to use jest.mock by using createRequire to bring require into your ESM environment [1][3].
Citations:
- 1: https://jestjs.io/docs/ecmascript-modules
- 2: https://jestjs.io/docs/30.0/ecmascript-modules
- 3: https://jestjs.io/docs/29.7/ecmascript-modules
- 4: [Bug]: jest.mock not working with ESM support activated jestjs/jest#13135
- 5: https://jestjs.io/docs/next/ecmascript-modules
- 6: jest.mock does not mock an ES module without Babel jestjs/jest#10025
- 7: https://kulshekhar.github.io/ts-jest/docs/28.0/guides/esm-support
- 8: https://kulshekhar.github.io/ts-jest/docs/getting-started/options/useESM
- 9: https://kulshekhar.github.io/ts-jest/docs/29.0/guides/esm-support
- 10: [Bug]: Virtual mocks created with
jest.unstable_mockModuledo not work outside of the test file jestjs/jest#15303
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(jest\.config\.cjs|package\.json|tsconfig[^/]*\.json|api\.test\.ts|.*jest.*|.*babel.*|.*swc.*)$' | head -200
printf '%s\n' '--- Jest and module configuration references ---'
rg -n --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' \
'useESM|extensionsToTreatAsEsm|preset.*esm|transform|type[[:space:]]*:[[:space:]]*"module"|unstable_mockModule|jest\.mock|`@jest/globals`' \
jest.config.cjs package.json packages/apifymcp package.json tsconfig*.json packages 2>/dev/null | head -300
printf '%s\n' '--- target file outline and relevant contents ---'
wc -l packages/apifymcp/api.test.ts
ast-grep outline packages/apifymcp/api.test.ts || true
sed -n '1,40p' packages/apifymcp/api.test.ts
sed -n '90,180p' packages/apifymcp/api.test.ts
printf '%s\n' '--- package metadata ---'
cat package.json
if [ -f packages/apifymcp/package.json ]; then cat packages/apifymcp/package.json; fiRepository: corsairdev/corsair
Length of output: 38221
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- apifymcp Jest configuration ---'
cat -n packages/apifymcp/jest.config.cjs
printf '%s\n' '--- apifymcp TypeScript configuration ---'
cat -n packages/apifymcp/tsconfig.json
printf '%s\n' '--- shared TypeScript configurations ---'
for f in tsconfig.json tsconfig.base.json tsconfig.test.json packages/corsair/tsconfig.test.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f"
fi
done
printf '%s\n' '--- test invocation and ESM runtime settings ---'
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' \
'pnpm (exec )?jest|jest --|NODE_OPTIONS|experimental-vm-modules|--experimental-vm-modules|packages/apifymcp|turbo.*test|\"test\"[[:space:]]*:' \
package.json pnpm-workspace.yaml turbo.json .github packages/apifymcp packages/corsair 2>/dev/null | head -300
printf '%s\n' '--- comparable mock patterns in ESM-configured tests ---'
for f in packages/vercel/api.test.ts packages/confluence/operations.test.ts packages/digitalocean/operations.test.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,35p' "$f"
fi
done
printf '%s\n' '--- Jest/ts-jest versions and lockfile entries ---'
rg -n '(^|/)(jest|ts-jest)@|jest@29\.7|ts-jest@29\.4|experimental-vm-modules' pnpm-lock.yaml package.json packages/apifymcp/package.json 2>/dev/null | head -100Repository: corsairdev/corsair
Length of output: 38053
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import json
import re
root = Path(".")
pkg = json.loads((root / "packages/apifymcp/package.json").read_text())
root_pkg = json.loads((root / "package.json").read_text())
cfg = (root / "packages/apifymcp/jest.config.cjs").read_text()
test = (root / "packages/apifymcp/api.test.ts").read_text()
checks = {
"root package is ESM": root_pkg.get("type") == "module",
"plugin package is ESM": pkg.get("type") == "module",
"ts-jest useESM is enabled": "useESM: true" in cfg,
"TypeScript module is ESNext": "module: 'ESNext'" in cfg,
"TypeScript files are treated as ESM": "extensionsToTreatAsEsm: ['.ts']" in cfg,
"static core import exists": bool(re.search(
r"^\s*import\s*\{[^}]*\blogEventFromContext\b[^}]*\}\s*from\s*['\"]corsair/core['\"]",
test, re.M
)),
"mock call exists": "jest.mock('corsair/core'" in test,
}
import_pos = test.find("import { AuthMissingError, logEventFromContext } from 'corsair/core';")
mock_pos = test.find("jest.mock('corsair/core'")
checks["static import precedes mock call"] = import_pos >= 0 and mock_pos > import_pos
# Inspect only declared launcher/configuration text. Do not execute repository code.
runtime_text = "\n".join(
p.read_text()
for p in [
root / ".github/workflows/pr-checks.yml",
root / "package.json",
root / "packages/apifymcp/package.json",
root / "turbo.json",
]
if p.exists()
)
checks["searched launcher text has VM-module flag"] = bool(re.search(
r"experimental-vm-modules|NODE_OPTIONS", runtime_text
))
for name, result in checks.items():
print(f"{name}: {'PASS' if result else 'FAIL'}")
if not all(checks[k] for k in checks if k != "searched launcher text has VM-module flag"):
raise SystemExit("repository facts do not match the expected ESM/mock shape")
PY
printf '%s\n' '--- narrow CI launcher context ---'
sed -n '35,75p' .github/workflows/pr-checks.yml
printf '%s\n' '--- turbo test task ---'
sed -n '20,45p' turbo.jsonRepository: corsairdev/corsair
Length of output: 1983
Replace the CommonJS mock with an ESM-compatible mock.
ts-jest emits ESM for this package, so static imports execute before jest.mock. Use jest.unstable_mockModule and dynamic imports for corsair/core and the system under test. Replace jest.requireActual with an ESM-compatible approach. Launch Jest with --experimental-vm-modules; the CI test command does not currently set it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/apifymcp/api.test.ts` around lines 7 - 13, Update the test setup to
use Jest’s ESM mocking: replace the jest.mock/requireActual block with
jest.unstable_mockModule for corsair/core, then dynamically import both
corsair/core and the system under test after registering the mock. Ensure the
Jest invocation, including CI’s test command, enables --experimental-vm-modules.
|
@greptile review |
Maintainer review neededAutomated rounds are exhausted. Remaining findings:
|
github-actions
Bot
added
the
needs-maintainer
Dhirenderchoudhary
removed
the
needs-maintainer
|
LGTM fixed blocking changes and locally tested with api |
3 participants
Description
packages/apify_mcp/with all 8 OSS operations wrapping the hosted Apify MCP server athttps://mcp.apify.comapi_key(Authorization: Bearerheader)ctx.dbupserts foractors,actorRuns, andactorOutputs(Slack pattern)BaseProviders/ProviderDisplayNamesOperations
search-actorsfetch-actor-detailscall-actorapify/rag-web-browserget-actor-runget-actor-outputsearch-apify-docsfetch-apify-docsTest plan
pnpm typecheckpnpm build(apify_mcp)pnpm test(6 live MCP tests withAPIFY_TOKEN)Screenshots / Demos (if applicable)
Closes #383
Summary by CodeRabbit
New Features
Tests