Correct skill installation and monitoring cost - #215
Conversation
1c135e4 to
5f1f016
Compare
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/docs-search.ts">
<violation number="1" location="src/commands/docs-search.ts:73">
P2: When the custom API URL comes from configuration rather than this command’s `--api-url`, this expression sends the stored API key to that custom endpoint. Check the effective normalized `apiUrl` instead, while retaining an explicitly supplied `options.apiKey` for authenticated self-hosted requests.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| explicitApiUrl || normalizeApiUrl(config.apiUrl || DEFAULT_API_URL); | ||
| const apiKey = | ||
| options.apiKey || | ||
| (explicitApiUrl && explicitApiUrl !== DEFAULT_API_URL |
There was a problem hiding this comment.
P2: When the custom API URL comes from configuration rather than this command’s --api-url, this expression sends the stored API key to that custom endpoint. Check the effective normalized apiUrl instead, while retaining an explicitly supplied options.apiKey for authenticated self-hosted requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/docs-search.ts, line 73:
<comment>When the custom API URL comes from configuration rather than this command’s `--api-url`, this expression sends the stored API key to that custom endpoint. Check the effective normalized `apiUrl` instead, while retaining an explicitly supplied `options.apiKey` for authenticated self-hosted requests.</comment>
<file context>
@@ -0,0 +1,157 @@
+ explicitApiUrl || normalizeApiUrl(config.apiUrl || DEFAULT_API_URL);
+ const apiKey =
+ options.apiKey ||
+ (explicitApiUrl && explicitApiUrl !== DEFAULT_API_URL
+ ? undefined
+ : config.apiKey);
</file context>
5f1f016 to
edecc80
Compare
edecc80 to
819fa9a
Compare
hmishra2250
left a comment
There was a problem hiding this comment.
Approved — the install scope and monitoring-pricing wording now match the implemented behavior and published docs.
Nit (non-blocking follow-up): now that build skills are separate from init, surface firecrawl setup build in the post-init next-steps copy and, ideally, the interactive bare firecrawl setup picker so the command is not easy to miss.
Why
The bundled install guidance says
initinstalls build skills, but build skills require a separate setup step. The monitor skill also omits the published cost model for recurring checks.Summary
initinstalls the CLI, core skills, and workflow skills.firecrawl setup buildas the separate build-skill step.Test Plan
pnpm test, 433 passing testspnpm run type-checkpnpm run format:checkpnpm run buildgit diff --check