Repository navigation
feat(cli): let wrappers set the integration tag with FIRECRAWL_INTEGRATION - #336
Conversation
…fer downloadable templates
…ate and Windows tests
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Shadow auto-approve: would auto-approve with 4 open P3 issues. Adds an opt-in FIRECRAWL_INTEGRATION env var to tag CLI API requests (defaulting to cli) across commands, with tests and README docs; bounded because only _-prefixed values are honored and default behavior is unchanged.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Adds an opt-in FIRECRAWL_INTEGRATION env var so CLI wrappers can tag API requests, defaulting to cli and honoring only underscore-prefixed values; focused tests and README docs cover the behavior.
View guided diff | Turn on auto-fix | Re-trigger cubic
…' into capy/bump-firecrawl-sdk-4-49-1
…egration-env # Conflicts: # package.json
|
@cubic-dev-ai review |
@capy-ai[bot] I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 28 files
Confidence score: 4/5
src/commands/setup.tsstill prints skip notices and the summary whenquietis set, soinitcan lose its compact output for env-backed keys. Gate those messages onquiet.package.jsonbumps the exact-pinnedfirecrawlSDK from 4.44.0 to 4.49.1 without explaining the reason. Document why the integration needs this version or split the bump into its own PR.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:88">
P3: This bumps the exact-pinned `firecrawl` SDK from 4.44.0 to 4.49.1, which the PR description never mentions. Either document why the new integration tag needs this SDK version, or split the bump into its own PR so it can be reviewed and rolled back separately.</violation>
</file>
<file name="src/commands/setup.ts">
<violation number="1" location="src/commands/setup.ts:777">
P2: The skip notices and the summary block print even when `quiet` is set. `init` calls `installMcp` with `quiet: true` and `agent: 'all'` when the key is env-backed, so the compact init output gets the full summary. Gate these logs on `!options.quiet`, and keep the `console.error` failure output.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
…ent test accurately
…' into capy/bump-firecrawl-sdk-4-49-1
|
@cubic-dev-ai review |
@capy-ai[bot] I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 28 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Adds opt-in FIRECRAWL_INTEGRATION for CLI requests; default stays cli and _-prefixed values are trimmed and validated, with tests. SDK/setup changes in the diff are the stacked #326/#337 changes named in the merge order.
View guided diff | Turn on auto-fix | Re-trigger cubic
…nt the stored-key prerequisite; restore an absent PATH in tests
…' into capy/bump-firecrawl-sdk-4-49-1
… the exported-key note
…' into capy/bump-firecrawl-sdk-4-49-1
|
@cubic-dev-ai review |
@capy-ai[bot] I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 28 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Adds opt-in FIRECRAWL_INTEGRATION tagging for CLI requests; default stays cli, only _-prefixed values pass through, with tests and docs. SDK/setup/init changes are the stacked #326/#337 changes scheduled to merge first.
View guided diff | Turn on auto-fix | Re-trigger cubic
…ent names; make the all-clients test portable to Windows
…' into capy/bump-firecrawl-sdk-4-49-1
|
@cubic-dev-ai review |
@capy-ai[bot] I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 28 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Adds opt-in FIRECRAWL_INTEGRATION tagging for CLI requests; default stays cli and only _-prefixed values pass, with tests/docs. SDK/setup/init changes are the stacked #326/#337 work merging first.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review |
@capy-ai[bot] I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 28 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Adds opt-in FIRECRAWL_INTEGRATION tagging across CLI commands; default stays cli and only _-prefixed values pass, matching the API schema, with tests and docs. Delta since prior review is only the firecrawl SDK bump, which belongs to stacked #337.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Opt-in FIRECRAWL_INTEGRATION replaces the hardcoded 'cli' tag across commands; default stays cli and only _-prefixed values are sent, with tests and docs. Delta since prior approval is just a README clarification.
View guided diff | Turn on auto-fix | Re-trigger cubic
…dk-4-49-1 # Conflicts: # package.json # pnpm-lock.yaml
…egration-env # Conflicts: # package.json # src/__tests__/commands/feedback.test.ts
…ation-env # Conflicts: # package.json
…every API request The body integration stays cli. The CLI keeps no list of names: the API ignores a value it does not recognize, so the tag can never fail a request.
There was a problem hiding this comment.
All reported issues were addressed across 27 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
… example and keyless header comment
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Opt-in FIRECRAWL_INTEGRATION adds X-Integration to all CLI API requests while body/query stay cli; default is unchanged, values are sanitized, and tests plus README cover the feature, making it bounded and clearly beneficial.
View guided diff | Turn on auto-fix | Re-trigger cubic
1 participant
Summary
Every command hardcodes
integration: 'cli', so tools that wrap the CLI always show up ascli. The CLI now readsFIRECRAWL_INTEGRATION, the same variablefirecrawl-mcpreads, and sends it as theX-Integrationrequest header. The body (and query)integrationstayscli.src/utils/integration.ts:integrationHeaders()returns{ 'X-Integration': <trimmed value> }when the variable is set. There is no underscore rule and no list of names in the CLI: the API attributes a recognized name and silently ignores anything else, so the tag can never make a request fail. The only values dropped client side are ones that cannot be sent as a header at all (control or non-ASCII characters), which would otherwise makefetchthrow.http), viawithIntegrationHeader()ingetClient();keylessRequest,keylessGet);fetchcalls: feedback, search-feedback, interact, parse, monitor, credit-usage, status, doctor, list and terms.withIntegrationHeader()adds it to the SDK client's axios defaults. The CLI already reaches into the samehttpmember for research and developer search. The smallest proper fix is aheaders?: Record<string, string>option onFirecrawlClientOptions, merged into the axios defaults inHttpClient; once an SDK release has it, the CLI can pass the header through that option instead.originand the keylessX-Originheader are unchanged.vitest.config.mjssetsFIRECRAWL_INTEGRATIONto empty for test runs, so a value exported in the shell doesn't change request assertions.The API side: a valid
X-Integrationheader must beat a body or query value ofclion every endpoint the CLI calls. That rule lands in firecrawl/firecrawl#5260; until it ships, requests keep being attributed tocli, and nothing fails either way.Testing
pnpm type-check,pnpm format:check,pnpm buildpass;pnpm test: 768 passed, with and withoutFIRECRAWL_INTEGRATION=hermesexported.src/__tests__/utils/integration.test.tsrun against a local HTTP server:rtk,hermesand_rtkgo out asX-Integrationon SDK client POST and GET requests while the body keepsintegration: cli; a junk value (not a valid tag!) is sent and the request succeeds; a value that cannot be a header is dropped and the request still succeeds.feedback.test.tschecks the header on feedback and search feedback.firecrawl scrape https://example.comexits 0 withFIRECRAWL_INTEGRATIONset tohermes,_rtkandnot a valid tag!.Merge order
Merge after #337 (merged, 1.26.14) and once firecrawl/firecrawl#5260 is deployed, so the README matches the API. Merging publishes 1.26.15.