feat: add Apify plugin - #326
Merged
Merged
Conversation
Contributor
|
@huamanraj is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Greptile SummaryThis PR introduces the
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Plugin as apify() factory
participant EndpointTree as buildEndpointTree
participant RestClient as makeApifyRequest
participant MCP as client.ts (MCP)
participant Apify as api.apify.com / mcp.apify.com
participant Logger as logEventFromContext
participant ErrHandlers as errorHandlers
Caller->>Plugin: "apify({ key }) → plugin instance"
Plugin->>EndpointTree: ApifyRestEndpoints (built at module load)
Plugin->>MCP: ActorsEndpoints, RunsEndpoints, DocsEndpoints
Caller->>EndpointTree: endpoint(ctx, input)
EndpointTree->>RestClient: makeApifyRequest(operationDef, ctx.key, input)
RestClient->>RestClient: buildBody() / buildQuery() / pickDefined()
RestClient->>Apify: request(config, requestOptions)
alt Success
Apify-->>RestClient: response
RestClient-->>EndpointTree: "response / { success: true } / { exists: bool }"
EndpointTree->>Logger: logEventFromContext(ctx, path, meta)
Logger-->>EndpointTree: (errors swallowed in try/catch)
EndpointTree-->>Caller: response
else HTTP 429
Apify-->>RestClient: ApiError(429)
RestClient-->>EndpointTree: throws ApiError
EndpointTree-->>Caller: throws ApiError
Caller->>ErrHandlers: RATE_LIMIT_ERROR.match → true
ErrHandlers-->>Caller: "{ maxRetries: 3, retryStrategy: exponential_backoff_jitter }"
else HTTP 5xx
Apify-->>RestClient: ApiError(5xx)
RestClient-->>EndpointTree: throws ApiError
ErrHandlers-->>Caller: "{ maxRetries: 2, retryStrategy: exponential_backoff }"
else MCP call (actors/runs/docs)
Caller->>MCP: ActorsEndpoints.callActor(ctx, input)
MCP->>Apify: StreamableHTTP to mcp.apify.com
Apify-->>MCP: result or StreamableHTTPError
MCP-->>Caller: McpToolResponse / throws ApifyMcpAPIError
Caller->>ErrHandlers: RATE_LIMIT_ERROR / AUTH_ERROR match
end
Reviews (13): Last reviewed commit: "merge(main): keep apify MCP and add REST..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
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 @huamanraj, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
Rule Used: What: All uses of 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! PR requirements (rules)
Optional improvements (P2)
If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
github-actions
Bot
added
bot:round-2
|
Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically. |
Collaborator
|
@greptileai review |
Maintainer review neededAutomated rounds are exhausted. Remaining findings:
|
github-actions
Bot
added
the
needs-maintainer
Collaborator
|
hey @huamanraj, few fixes pushed to your branch A couple things to keep in mind going forward:
|
ambikeesshh
reviewed
Jul 23, 2026
ambikeesshh
left a comment
ambikeesshh
left a comment
Collaborator
There was a problem hiding this comment.
addressed the review findings:
merged latest main and repaired the broken pnpm-lock.yaml (ts-jest importer) that was failing CI install
wrapped logEventFromContext so a logging failure can’t turn a successful Apify call into an error
forced DEFAULT error handler last so caller overrides stay reachable
added a regression test for the log-failure path
Collaborator
|
@greptileai review |
Contributor
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
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 selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a typed Apify REST operation catalog, authenticated request client, recursive endpoint generation, database schemas, expanded error handling, plugin integration, public REST types, package metadata, and comprehensive tests. ChangesApify integration
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ApifyRestEndpoints
participant makeApifyRequest
participant ApifyAPI
Caller->>ApifyRestEndpoints: Invoke operation with input
ApifyRestEndpoints->>makeApifyRequest: Pass operation, API key, and input
makeApifyRequest->>ApifyAPI: Send authenticated HTTP request
ApifyAPI-->>makeApifyRequest: Return response or HTTP error
makeApifyRequest-->>ApifyRestEndpoints: Return normalized output
ApifyRestEndpoints-->>Caller: Return endpoint result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (10)
packages/apify/endpoints/operations.ts (1)
769-772: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
runActorSyncGetDatasetItemsItems.The key repeats the
Itemssuffix. This key becomes part of the public endpoint tree, becausebuildEndpointTreeinpackages/apify/endpoints/index.ts(Lines 39-73) derives endpoint names directly from catalog keys. A rename after release is a breaking change for consumers. Rename it now, for example torunActorSyncPostGetDatasetItems, to match the GET variantrunActorSyncGetDatasetItemsat Line 728.🤖 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/apify/endpoints/operations.ts` around lines 769 - 772, Rename the endpoint catalog key runActorSyncGetDatasetItemsItems to runActorSyncPostGetDatasetItems, preserving its method, path, and parameters. Ensure any references to the old key are updated so buildEndpointTree exposes the corrected public endpoint name alongside runActorSyncGetDatasetItems.packages/apify/schema/database.ts (1)
132-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
z.email()for the email schema. The package uses Zod 4.1+, wherez.string().email()is deprecated.🤖 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/apify/schema/database.ts` around lines 132 - 134, Update the email field schema in the database schema definition to use Zod 4.1+'s z.email() validator instead of the deprecated z.string().email() chain, while preserving its optional behavior.packages/apify/index.ts (2)
56-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
AuthTypesannotation widens the literal type.
const defaultAuthType: AuthTypes = 'api_key' as const;produces the typeAuthTypes, not'api_key'.BaseApifyPluginthen passes the wide union astypeof defaultAuthType. Remove the annotation to keep the literal.♻️ Proposed change
-const defaultAuthType: AuthTypes = 'api_key' as const; +const defaultAuthType = 'api_key' as const satisfies AuthTypes;🤖 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/apify/index.ts` at line 56, Remove the explicit AuthTypes annotation from defaultAuthType while retaining the 'api_key' literal initializer and const assertion, so typeof defaultAuthType remains the literal type passed by BaseApifyPlugin.
27-27: 📐 Maintainability & Code Quality | 🔵 TrivialInbound webhook support is absent.
apifyWebhooksNestedis empty andpluginWebhookMatcheralways returnsfalse. Linked issue#324requests webhook events for Actor and Task run status changes, including succeeded, failed, timed out, and aborted runs. The plugin only exposes the Apify webhook management endpoints, so Corsair cannot receive Apify callbacks.Confirm whether inbound webhooks are out of scope for this PR. I can open a follow-up issue that tracks the webhook receiver work.
Also applies to: 96-100
🤖 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/apify/index.ts` at line 27, Confirm that inbound Apify webhook receiving is out of scope for this PR; do not expand the empty apifyWebhooksNested definition or pluginWebhookMatcher for this change. Track the requested Actor and Task run status callbacks, including succeeded, failed, timed out, and aborted events, in a follow-up issue.packages/apify/endpoints/index.ts (2)
54-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecord the logging failure instead of discarding it.
The empty
catch {}block protects the response path. It also hides all logging failures. Capture the error in a debug log so operators can detect a broken event pipeline.♻️ Proposed change
- } catch {} + } catch (logError) { + console.debug('apify: failed to log endpoint event', logError); + }🤖 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/apify/endpoints/index.ts` around lines 54 - 64, Update the catch block surrounding logEventFromContext in the apify operation completion path to capture the thrown error and record it with the available debug logger, while preserving the existing behavior of not interrupting the response path.
87-94: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueProtect schemas from future parameter collisions.
The current registry has no path/query collisions or reserved parameter names. If one is added, the query loop overwrites the existing schema. Keep
if (param in shape) continue;to preserve required path and reserved input fields.🤖 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/apify/endpoints/index.ts` around lines 87 - 94, Update the query-parameter loop in the operation schema construction to skip parameters already present in shape before assigning an optional unknown schema. Preserve existing required path-parameter and reserved input-field schemas by adding the collision guard, while continuing to register non-colliding query parameters.packages/apify/jest.config.cjs (1)
11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe coverage ignore pattern does not match the config file name.
The config file is
jest.config.cjs, but the exclusion lists!jest.config.ts. The config file and the test files are therefore included in coverage.♻️ Proposed change
collectCoverageFrom: [ '**/*.ts', '!**/*.d.ts', + '!**/*.test.ts', '!**/node_modules/**', '!**/dist/**', - '!jest.config.ts', + '!jest.config.cjs', '!tests/**', ],🤖 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/apify/jest.config.cjs` around lines 11 - 18, Update the collectCoverageFrom exclusions in the Jest configuration so they exclude jest.config.cjs instead of jest.config.ts, while preserving the existing test, build, declaration, and dependency exclusions.packages/apify/operations.test.ts (3)
444-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd behavior tests for the error handlers.
This test only asserts that the handler keys exist.
packages/apify/error-handlers.tscontains matching logic for status codes and message substrings, plus the retry configuration for HTTP 429. Add cases that callmatchwith anApiErrorfor 429, 401, 403, 404, and 400, and that assertRATE_LIMIT_ERROR.handlerreturnsmaxRetries: 3and the exponential backoff strategy.🤖 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/apify/operations.test.ts` around lines 444 - 449, Add behavior-focused cases to the `registers error handlers covering rate-limit and auth errors` test by invoking each handler’s `match` with `ApiError` instances for status codes 429, 401, 403, 404, and 400, asserting the expected matching outcomes. Also invoke `RATE_LIMIT_ERROR.handler` and verify it returns `maxRetries: 3` with the exponential backoff strategy.
91-100: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlso assert the reverse direction of the path-param contract.
The test checks that every URL template placeholder appears in
pathParams. It does not check that every declaredpathParamsentry appears in the URL template. A stale entry then becomes a required schema field that the request never uses.♻️ Proposed addition
for (const param of templateParams) { expect(def.pathParams).toContain(param); } + for (const param of def.pathParams) { + expect(templateParams).toContain(param); + }🤖 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/apify/operations.test.ts` around lines 91 - 100, Extend the test case “declares every path param used in the URL template” to also iterate over each operation’s def.pathParams and assert every declared parameter exists among the extracted templateParams. Preserve the existing template-to-declaration assertion so both directions of the path-parameter contract are covered.
65-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe hardcoded operation count makes the test brittle.
Every added or removed operation breaks this test even when the registry stays valid. Keep the uniqueness and prefix assertions. Derive the count from the registry, or move the exact number into a single named constant with a comment that explains the OSS listing requirement.
♻️ Proposed change
+// Locked to the operation count published in the Apify OSS listing. +const EXPECTED_OPERATION_COUNT = 113; + it('registers exactly the 113 OSS listing operations', () => { - expect(ALL_OPERATIONS.length).toBe(113); + expect(ALL_OPERATIONS.length).toBe(EXPECTED_OPERATION_COUNT); const slugs = ALL_OPERATIONS.map(({ def }) => def.slug); - expect(new Set(slugs).size).toBe(113); + expect(new Set(slugs).size).toBe(ALL_OPERATIONS.length);🤖 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/apify/operations.test.ts` around lines 65 - 72, Update the operation-count assertion in the “registers exactly the 113 OSS listing operations” test to avoid a brittle hardcoded value by deriving the expected count from the registry or defining one named constant that documents the OSS listing requirement. Preserve the existing uniqueness and APIFY_ prefix assertions.
🤖 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/apify/client.ts`:
- Around line 110-122: Update the request-options construction around mediaType,
headers, and body to validate mediaType/contentType as strings before use,
falling back to the default media type when invalid. Filter reserved header
names case-insensitively from input.headers so variants such as authorization
cannot coexist with the later Authorization header, while preserving other
headers and existing body serialization behavior.
In `@packages/apify/endpoints/operations.ts`:
- Around line 204-206: Remove the Markdown escape backslashes from the
identifier examples in the descriptions for APIFY_ACTOR_TASK_DELETE and the
corresponding entries at the other two occurrences, so the runtime metadata
displays username~taskName, username~actorName, and username~dataset-name
without a literal backslash.
In `@packages/apify/error-handlers.ts`:
- Around line 25-28: Update the non-ApiError matching logic in the error
handler’s match callback to recognize “rate-limit,” “rate limit exceeded,” and
“too many requests” message variants, while preserving the existing ApiError
status-429 check.
In `@packages/apify/tsconfig.json`:
- Around line 5-19: Add `@types/node` as a direct dependency or devDependency in
packages/apify/package.json, and update the tsconfig include/exclude
configuration to omit **/*.test.ts from declaration generation while retaining
the existing dist and node_modules exclusions. Leave references: [] unchanged.
---
Nitpick comments:
In `@packages/apify/endpoints/index.ts`:
- Around line 54-64: Update the catch block surrounding logEventFromContext in
the apify operation completion path to capture the thrown error and record it
with the available debug logger, while preserving the existing behavior of not
interrupting the response path.
- Around line 87-94: Update the query-parameter loop in the operation schema
construction to skip parameters already present in shape before assigning an
optional unknown schema. Preserve existing required path-parameter and reserved
input-field schemas by adding the collision guard, while continuing to register
non-colliding query parameters.
In `@packages/apify/endpoints/operations.ts`:
- Around line 769-772: Rename the endpoint catalog key
runActorSyncGetDatasetItemsItems to runActorSyncPostGetDatasetItems, preserving
its method, path, and parameters. Ensure any references to the old key are
updated so buildEndpointTree exposes the corrected public endpoint name
alongside runActorSyncGetDatasetItems.
In `@packages/apify/index.ts`:
- Line 56: Remove the explicit AuthTypes annotation from defaultAuthType while
retaining the 'api_key' literal initializer and const assertion, so typeof
defaultAuthType remains the literal type passed by BaseApifyPlugin.
- Line 27: Confirm that inbound Apify webhook receiving is out of scope for this
PR; do not expand the empty apifyWebhooksNested definition or
pluginWebhookMatcher for this change. Track the requested Actor and Task run
status callbacks, including succeeded, failed, timed out, and aborted events, in
a follow-up issue.
In `@packages/apify/jest.config.cjs`:
- Around line 11-18: Update the collectCoverageFrom exclusions in the Jest
configuration so they exclude jest.config.cjs instead of jest.config.ts, while
preserving the existing test, build, declaration, and dependency exclusions.
In `@packages/apify/operations.test.ts`:
- Around line 444-449: Add behavior-focused cases to the `registers error
handlers covering rate-limit and auth errors` test by invoking each handler’s
`match` with `ApiError` instances for status codes 429, 401, 403, 404, and 400,
asserting the expected matching outcomes. Also invoke `RATE_LIMIT_ERROR.handler`
and verify it returns `maxRetries: 3` with the exponential backoff strategy.
- Around line 91-100: Extend the test case “declares every path param used in
the URL template” to also iterate over each operation’s def.pathParams and
assert every declared parameter exists among the extracted templateParams.
Preserve the existing template-to-declaration assertion so both directions of
the path-parameter contract are covered.
- Around line 65-72: Update the operation-count assertion in the “registers
exactly the 113 OSS listing operations” test to avoid a brittle hardcoded value
by deriving the expected count from the registry or defining one named constant
that documents the OSS listing requirement. Preserve the existing uniqueness and
APIFY_ prefix assertions.
In `@packages/apify/schema/database.ts`:
- Around line 132-134: Update the email field schema in the database schema
definition to use Zod 4.1+'s z.email() validator instead of the deprecated
z.string().email() chain, while preserving its optional behavior.
🪄 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: 5c90cf95-c67e-4549-86c5-efe30c2856a9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
packages/apify/client.tspackages/apify/endpoints/index.tspackages/apify/endpoints/operations.tspackages/apify/endpoints/types.tspackages/apify/error-handlers.tspackages/apify/index.tspackages/apify/jest.config.cjspackages/apify/operations.test.tspackages/apify/package.jsonpackages/apify/schema/database.tspackages/apify/schema/index.tspackages/apify/tsconfig.jsonpackages/apify/tsup.config.tspackages/corsair/core/constants.ts
Comment on lines
+110
to
+122
| const mediaType = | ||
| input.mediaType ?? input.contentType ?? 'application/json; charset=utf-8'; | ||
| const headers = isRecord(input.headers) ? input.headers : undefined; | ||
|
|
||
| const requestOptions: ApiRequestOptions = { | ||
| method: operation.method, | ||
| url: operation.path, | ||
| path: pickDefined(input, operation.pathParams), | ||
| query: buildQuery(operation, input), | ||
| headers, | ||
| body, | ||
| mediaType: body === undefined ? undefined : mediaType, | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect how corsair/http `request` merges OpenAPIConfig.HEADERS with ApiRequestOptions.headers.
set -euo pipefail
fd -t f 'request.ts' packages/corsair --exec ast-grep outline {} --items all
fd -t f . packages/corsair --extension ts \
| xargs rg -n -C 6 'getHeaders|Object\.entries\(config\.HEADERS|\.\.\.options\.headers|\.\.\.headers'Repository: corsairdev/corsair
Length of output: 4450
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- packages/apify/client.ts ---'
sed -n '70,145p' packages/apify/client.ts
printf '%s\n' '--- packages/corsair/async-core/request.ts ---'
sed -n '152,205p' packages/corsair/async-core/request.ts
printf '%s\n' '--- related types and call sites ---'
rg -n -C 5 'interface ApiRequestOptions|type ApiRequestOptions|mediaType|contentType|headers:' packages/apify packages/corsair/async-coreRepository: corsairdev/corsair
Length of output: 12495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- packages/apify/client.ts (top) ---'
sed -n '1,75p' packages/apify/client.ts
printf '%s\n' '--- Apify input schema and exported input types ---'
sed -n '1,120p' packages/apify/endpoints/types.ts
sed -n '1,125p' packages/apify/endpoints/index.ts
printf '%s\n' '--- makeApifyRequest usages ---'
rg -n -C 8 'makeApifyRequest|ApifyOperationInput|ApifyOperationInputSchema' packagesRepository: corsairdev/corsair
Length of output: 20531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
function isDefined(value) {
return value !== undefined && value !== null;
}
function isString(value) {
return typeof value === 'string';
}
function getHeaders(config, options) {
const additionalHeaders = config.HEADERS;
const headers = Object.entries({
Accept: 'application/json',
...additionalHeaders,
...options.headers,
})
.filter(([, value]) => isDefined(value))
.reduce((result, [key, value]) => {
result[key] = String(value);
return result;
}, {});
const token = config.TOKEN;
if (typeof token === 'string' && token.length > 0) {
headers.Authorization = `Bearer ${token}`;
}
if (options.body !== undefined) {
if (options.mediaType) {
headers['Content-Type'] = options.mediaType;
} else if (!isString(options.body)) {
headers['Content-Type'] = 'application/json';
}
}
return new Headers(headers);
}
const cases = [
{
name: 'caller Authorization',
options: { headers: { Authorization: 'Bearer caller' } },
},
{
name: 'caller lowercase authorization',
options: { headers: { authorization: 'Bearer caller' } },
},
{
name: 'caller arbitrary header',
options: { headers: { 'X-Injected': 'value' } },
},
{
name: 'numeric media type with body',
options: { body: { value: 1 }, mediaType: 42 },
},
];
for (const test of cases) {
try {
const headers = getHeaders(
{ HEADERS: { 'Content-Type': 'application/json' }, TOKEN: 'configured' },
test.options,
);
console.log(test.name, JSON.stringify(Object.fromEntries(headers.entries())));
} catch (error) {
console.log(test.name, `${error.constructor.name}: ${error.message}`);
}
}
JSRepository: corsairdev/corsair
Length of output: 846
Filter reserved headers and validate mediaType.
Authorization is assigned after options.headers, but a case-variant such as authorization remains and Headers combines both values. Remove reserved header names case-insensitively. Validate mediaType and contentType before assigning them; a non-string value can produce an invalid Content-Type and fail during body serialization.
🤖 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/apify/client.ts` around lines 110 - 122, Update the request-options
construction around mediaType, headers, and body to validate
mediaType/contentType as strings before use, falling back to the default media
type when invalid. Filter reserved header names case-insensitively from
input.headers so variants such as authorization cannot coexist with the later
Authorization header, while preserving other headers and existing body
serialization behavior.
Comment on lines
+5
to
+19
| "types": ["node", "jest"], | ||
| "module": "ESNext", | ||
| "moduleResolution": "Bundler", | ||
| "outDir": "./dist", | ||
| "rootDir": "./", | ||
| "composite": true, | ||
| "incremental": true, | ||
| "emitDeclarationOnly": true, | ||
| "declaration": true, | ||
| "declarationMap": true, | ||
| "skipLibCheck": true | ||
| }, | ||
| "include": ["./**/*"], | ||
| "exclude": ["dist", "node_modules"], | ||
| "references": [] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare the apify tsconfig with sibling plugin tsconfigs and check `@types/node` availability.
set -euo pipefail
fd -t f 'tsconfig.json' packages -d 2 \
| grep -v node_modules \
| while IFS= read -r f; do
echo "===== $f"
cat "$f"
done
echo "===== root manifest"
jq '{devDependencies, pnpm}' package.json 2>/dev/null || true
echo "===== `@types/node` declarations across the workspace"
rg -n '"`@types/node`"' --glob '**/package.json' --glob '!**/node_modules/**'Repository: corsairdev/corsair
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== packages/apify/package.json"
cat packages/apify/package.json
echo "===== apify source and test files"
find packages/apify -maxdepth 3 -type f \
\( -name '*.ts' -o -name '*.tsx' -o -name 'jest.config.*' \) \
-print | sort
echo "===== workspace manifests containing `@types/node` or `@types/jest`"
rg -n -C 2 '"`@types/`(node|jest)"' \
--glob 'package.json' --glob '!**/node_modules/**' .
echo "===== package-manager configuration"
for f in package.json pnpm-workspace.yaml pnpm-lock.yaml .npmrc; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n -C 2 'node-linker|hoist|catalog|`@types/`(node|jest)|workspace:' "$f" || true
fi
doneRepository: corsairdev/corsair
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== apify lockfile entry"
awk '
/^ packages\/apify:/ { in_block=1; count=0 }
in_block && count++ < 45 { print }
in_block && count >= 45 { exit }
' pnpm-lock.yaml
echo "===== apify build configuration"
cat packages/apify/tsup.config.ts
cat packages/apify/jest.config.cjs
cat tsconfig.base.json
echo "===== sibling package tsconfig summary"
python3 - <<'PY'
import json
from pathlib import Path
for path in sorted(Path("packages").glob("*/tsconfig.json")):
try:
data = json.loads(path.read_text())
except Exception:
continue
options = data.get("compilerOptions", {})
include = data.get("include", [])
exclude = data.get("exclude", [])
references = data.get("references", [])
if path.parent.name in {"apify", "abstract", "airtable", "ambientweather", "neon", "supabase"}:
print(path)
print(" types:", options.get("types"))
print(" include:", include)
print(" exclude:", exclude)
print(" references:", references)
PY
echo "===== root package manager settings"
sed -n '1,75p' package.json
sed -n '1,35p' pnpm-workspace.yamlRepository: corsairdev/corsair
Length of output: 8202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
from fnmatch import fnmatch
def load_json(path):
return json.loads(path.read_text())
apify = load_json(Path("packages/apify/tsconfig.json"))
files = [
p.relative_to("packages/apify").as_posix()
for p in Path("packages/apify").rglob("*")
if p.is_file()
]
def matches_ts_include(path, pattern):
pattern = pattern.removeprefix("./")
if pattern == "**/*":
return True
if pattern.startswith("**/"):
return fnmatch(path, pattern[3:]) or fnmatch(path, pattern)
return fnmatch(path, pattern)
def matches_ts_exclude(path, pattern):
pattern = pattern.removeprefix("./")
return fnmatch(path, pattern) or fnmatch(path, pattern.removeprefix("**/"))
included = [
path for path in files
if any(matches_ts_include(path, pattern) for pattern in apify["include"])
and not any(matches_ts_exclude(path, pattern) for pattern in apify["exclude"])
]
print("operations.test.ts included:",
"operations.test.ts" in included)
print("test TypeScript files included:",
sorted(path for path in included if path.endswith(".test.ts")))
tsup = Path("packages/apify/tsup.config.ts").read_text()
print("tsup clean false:", "clean: false" in tsup)
print("tsup declaration generation disabled:", "dts: false" in tsup)
empty_refs = []
nonempty_refs = []
for path in sorted(Path("packages").glob("*/tsconfig.json")):
try:
config = load_json(path)
except Exception:
continue
refs = config.get("references", None)
if refs == []:
empty_refs.append(path.parent.name)
elif refs:
nonempty_refs.append((path.parent.name, refs))
print("plugin configs with empty references:", len(empty_refs))
print("plugin configs with non-empty references:", nonempty_refs)
PYRepository: corsairdev/corsair
Length of output: 398
Declare @types/node and exclude test files from the declaration build.
- Add
@types/nodetopackages/apify/package.json. Thetypesoption namesnode, but the package currently relies on the root dependency. - Exclude
**/*.test.tsfromtsconfig.json. The current build emitsoperations.test.d.tsintodist, andtsupdoes not remove it. - Keep
references: []. This matches the other plugin packages.
🤖 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/apify/tsconfig.json` around lines 5 - 19, Add `@types/node` as a
direct dependency or devDependency in packages/apify/package.json, and update
the tsconfig include/exclude configuration to omit **/*.test.ts from declaration
generation while retaining the existing dist and node_modules exclusions. Leave
references: [] unchanged.
This was referenced Aug 12, 2026
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Collaborator
|
@greptileai review |
ambikeesshh
reviewed
Aug 12, 2026
ambikeesshh
left a comment
ambikeesshh
left a comment
Collaborator
There was a problem hiding this comment.
folded the REST ops into the existing apify MCP plugin so we don't overwrite it. actor CRUD is under acts because MCP already owns actors
devjain32
approved these changes
Aug 12, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #324.
Adds the
@corsair-dev/apifyplugin package, integrating the Apify API v2 surface (actors, runs, builds, tasks, datasets, key-value stores, request queues, schedules, webhooks, and users) via Bearer-token API-key auth.Covers:
apifyregistered in Corsair provider constantsChecklist
pnpm lintand all checks passpnpm typecheckand there are no TypeScript errorspnpm buildand all packages build successfullypnpm testand all tests passScreenshots / Demos (if applicable)
https://github.com/user-attachments/assets/apify-demo.mp4 (placeholder — final walkthrough recording before merge)
Additional Notes
tools.browserInfoDeleterisk level corrected fromdestructivetowrite—/v2/browser-infois a read-style endpoint exposed across all HTTP verbs.main; lint, typecheck, build, validate:plugins, and the test suite pass.Summary by CodeRabbit
New Features
Bug Fixes