Skip to content

feat: add firecrawl plugin - #60

Merged
kyle-sexton merged 2 commits into
mainfrom
feat/publish-firecrawl
Jul 11, 2026
Merged

feat: add firecrawl plugin#60
kyle-sexton merged 2 commits into
mainfrom
feat/publish-firecrawl

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Closes melodic-software/medley#1296.

Publishes the firecrawl plugin per docs/MIGRATION-PLAYBOOK.md (per-plugin gate + acceptance security review).

What ships

  • plugins/firecrawl/ — one skill (/firecrawl:firecrawl): wraps the firecrawl-cli binary for scrape/search/crawl/map/parse/interact/agent/monitor with the core write-to-disk-then-Read pattern (results land in tempfiles via -o; the agent reads only the needed slice — 32–35× token savings vs the MCP per the Scalekit benchmark cited in the skill).
  • context/commands.md (full flag tables, carried verbatim), context/configuration.md + context/update-flow.md (adapted repo-agnostic).
  • Maintainer-facing scripts/update.sh (--check read-only CLI-version + upstream-SHA drift report; --apply npm upgrade + UPSTREAM.md rewrite behind approval gates; never touches SKILL.md — content integration is a gated manual step) with a new self-contained update.test.sh (17 checks, network-free).
  • UPSTREAM.md sidecar (SHA-tracking sync state + rollback version).
  • Marketplace entry: category: utilities. Explicit version: 0.1.0 in plugin.json only.

Gate evidence

  • claude plugin validate ./plugins/firecrawl --strict — PASS; catalog --strict — PASS
  • claude plugin details token cost: ~253 tok always-on, ~4.9k on-invoke
  • update.test.sh 17/17 PASS; shellcheck (repo rcfile) clean; shfmt clean; markdownlint 0 errors; typos clean; editorconfig-checker clean
  • Smoke test via --plugin-dir — PASS: skill body loaded, answered the write-to-disk flag (-o) from the body, namespace confirmed as firecrawl:firecrawl. The smoke run surfaced two real headless-permission defects that are fixed in this PR: a credential-shaped printenv FIRECRAWL_API_KEY preamble line was rejected by permission preflight (removed — firecrawl --status already reports auth), and the status/sync preamble lines needed narrow allowed-tools grants (Bash(command -v firecrawl*), Bash(firecrawl --status*), Bash(grep -m1 *UPSTREAM.md*)). Caveat: in a fully sandboxed scratch dir the preamble lines may still fall back to their error text; the skill body loads and functions regardless.

Security review (playbook acceptance)

  • No hooks, no MCP servers, no agents, no userConfig.
  • Data egress (normal invocation): the user-installed firecrawl-cli calls api.firecrawl.dev (or a self-hosted FIRECRAWL_API_URL) — that is the plugin's documented purpose. The credential is the consumer-owned FIRECRAWL_API_KEY env var; never stored in or written by the plugin. Env-var auth is preferred over firecrawl login/config explicitly to avoid a second credential store.
  • Data egress (update path, maintainer-invoked only): registry.npmjs.org (version metadata) + www.firecrawl.dev (upstream skill source, hashed in a tmpdir). npm install -g and any SKILL.md rewrite sit behind two explicit approval gates; --check is read-only.
  • The skill hard-prohibits firecrawl init (would install a parallel MCP + skill copy) and documents the login/config divergence risk.
  • Cache isolation: script resolves paths relative to itself; preamble reads its own UPSTREAM.md via the skill-dir substitution; no ../ reach-outs.
  • No PII / secrets.

Merge note: open PRs #53, #56, #57, #59 also touch marketplace.json + root README.md — merge serially; later ones need a conflict-merge of the two shared files.


Note

Medium Risk
New plugin steers agents toward external Firecrawl API egress and credit use via a user-installed CLI and FIRECRAWL_API_KEY; maintainer --apply runs npm install -g, but there are no hooks/MCP and updates are gated.

Overview
Adds a new firecrawl plugin to the marketplace and root catalog, shipping /firecrawl:firecrawl as the maintained firecrawl-cli integration (scrape, search, crawl, map, parse, interact, agent, monitor) instead of the Firecrawl MCP.

The skill centers on write-to-disk then Read (-o tempfiles) with escalation tables for when to use Firecrawl vs WebFetch, narrow allowed-tools for the status/sync preamble (firecrawl --status, UPSTREAM.md grep), and prohibitions against firecrawl init / login-style config drift. Supporting context/ docs cover flags, env defaults, and the maintainer update pipeline; UPSTREAM.md records upstream SHA and CLI rollback versions.

Maintainer tooling: scripts/update.sh (--check read-only drift vs npm + upstream skill; --apply global npm install + UPSTREAM.md rewrite only, not SKILL.md) and network-free update.test.sh regression checks.

Reviewed by Cursor Bugbot for commit 3184b4e. Bugbot is set up for automated code reviews on this repo. Configure here.

Wraps the firecrawl-cli binary as an agent skill: scrape, search,
crawl, map, parse, interact, agent, and monitor commands behind the
write-to-disk-then-Read pattern that keeps large results out of
context, plus a gated maintainer update flow (UPSTREAM.md SHA tracking,
two approval gates, recorded rollback version). Source-repo references
generalized; update script resolves paths relative to itself and gains
a source-guard plus a new self-contained regression suite. Preamble
hardened for headless permission preflight (redundant credential-probe
line dropped; narrow allowed-tools grants for the status and sync
lines).

Co-Authored-By: Claude <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 11, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_02c72919-0a68-4536-b9a2-40b901dc061b)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 7s —— View job


PR Review: feat/publish-firecrawl

  • Gather context and read changed files
  • Review plugin manifest and marketplace entry
  • Review SKILL.md for correctness and security
  • Review update scripts (update.sh / update.test.sh)
  • Review context docs
  • Post final findings

Overall this is a high-quality plugin submission. The structure, security posture, and documentation are solid. A few actionable issues below, ordered by severity.


Medium — date +%s%N is GNU-only; macOS examples produce malformed filenames

File: plugins/firecrawl/skills/firecrawl/SKILL.md

NONCE=$(date +%s%N)
firecrawl scrape "..." --format markdown -o "/tmp/fc-${NONCE}.md"

%N (nanoseconds) is a GNU date extension. On macOS/BSD, date +%s%N emits the literal string %N after the Unix timestamp, producing filenames like /tmp/fc-1720694123%N.md. The file is still created and functional, but the nonce provides no collision-avoidance on macOS, and the %N in the path could confuse shell scripts that parse it. The Gotchas section mentions Windows/Git Bash path behaviour but is silent on macOS.

Recommended fix — replace all three NONCE=$(date +%s%N) occurrences with mktemp, which is portable and atomic:

OUTFILE=$(mktemp /tmp/fc-XXXXXX.md)
firecrawl scrape "https://www.gnu.org/software/bash/manual/bash.html" \
  --format markdown -o "$OUTFILE"
# Then: Read "$OUTFILE" with offset/limit as needed

If keeping the NONCE pattern is preferred for readability, $(date +%s)_$$ (seconds + PID) is portable and avoids the %N literal.

Fix this →


Low — --apply carries no internal gate; safety relies solely on the caller

File: plugins/firecrawl/skills/firecrawl/scripts/update.sh, line 188

if ! npm install -g "${NPM_PKG}@latest"; then

SKILL.md documents "Gate 1 covers the binary install" and states "--apply never runs npm install -g without asking first." But the script itself has no read/prompt — it runs npm install -g unconditionally. The gate lives entirely in the skill body's conversational flow, which calls --apply only after the user approves. A maintainer who invokes scripts/update.sh --apply from the shell directly bypasses Gate 1 silently.

Adding a one-line warning to the --apply branch header would close the gap without breaking automated callers:

run_apply() {
  log "Gate 1 confirmed by caller — proceeding with npm install."
  ...

Or more firmly, check for a --confirmed flag before running npm:

--apply|--apply-confirmed)
  [[ "$mode" == "--apply-confirmed" ]] || {
    err "Pass --apply-confirmed to acknowledge this mutates the system (npm install -g + UPSTREAM.md rewrite)"
    exit 2
  }

The current design is not wrong, but the safety claim in SKILL.md is stronger than what the script enforces, which will confuse future maintainers.


Low — --pretty note in commands.md may mislead users

File: plugins/firecrawl/skills/firecrawl/context/commands.md, line 36

No --format or --pretty flag — use --json for structured output, pipe through jq for pretty-printing.

configuration.md lists --pretty as a global flag available to every subcommand. A reader of commands.md in isolation will think firecrawl search ... --pretty doesn't work, when it may well be valid. The intent is clearly "the search command has no command-specific format flag", but the wording implies --pretty doesn't exist at all.

Suggested rewording: "No command-specific --format flag. Use --json for structured output; --pretty (global flag) pretty-prints it, or pipe through jq."


Low — allowed-tools grep glob is path-unconstrained

File: plugins/firecrawl/skills/firecrawl/SKILL.md, line 7

allowed-tools: … Bash(grep -m1 *UPSTREAM.md*)

The glob *UPSTREAM.md* permits a grep against any file whose name contains UPSTREAM.md, not only the skill's own UPSTREAM.md. The preamble command is:

grep -m1 '^- Last sync:' "${CLAUDE_SKILL_DIR}/UPSTREAM.md"

which is benign, but the grant allows the same pattern against an attacker-controlled filename if a prompt-injection were to construct a path containing UPSTREAM.md. A narrower grant might be Bash(grep -m1 *${CLAUDE_SKILL_DIR}/UPSTREAM.md*) if variable expansion is supported in allowed-tools globs; otherwise a comment noting the accepted breadth would suffice.


Positive notes

  • Manifest: plugin.json is schema-valid with explicit version: 0.1.0; marketplace entry category (utilities) and tags are appropriate.
  • SKILL.md frontmatter: All required fields present (name, description, argument-hint, user-invocable: true, disable-model-invocation: false). The description single-line format with Use when: and skip guidance is exactly right.
  • Preamble grants are tight: Bash(command -v firecrawl*), Bash(firecrawl --status*), Bash(grep -m1 *UPSTREAM.md*) are the minimum needed for the three preamble lines — no broad shell grant.
  • Write-to-disk pattern: Enforced consistently across all non-trivial examples; direct stdout reserved only for tiny results with explicit justification. The "32–35× token savings" claim is cited.
  • update.sh: --fail on every curl call prevents silently hashing a 404 error page as if it were real SKILL.md content — this is the most important guard in the update path and it's correctly applied on both fetch calls (lines 91, 102).
  • update.test.sh: Source-guard pattern correctly keeps main from running during source; SOURCED_TMPDIR cleanup at line 120 avoids leaking the script's own TMPDIR_RUN after sourcing.
  • No PII/secrets: No hardcoded keys, no FIRECRAWL_API_KEY values, no printenv invocations. The PR description notes a previous printenv preamble line was caught and removed before merge.
  • Repo-agnostic: All paths go through ${CLAUDE_SKILL_DIR}, ${SCRIPT_DIR}/.., or /tmp/. No hardcoded repo-specific values.
  • firecrawl init prohibition is explicit and prominent in both the Gotchas section and Prerequisites.

The macOS date +%s%N issue is the one worth fixing before merge — the other three are documentation/safety hygiene. The rest of the implementation is clean.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Risk: medium. Cursor Bugbot did not complete (check skipped; usage limit reached), so automated review is incomplete and I am not approving. Human review is needed before merge; no reviewers were assigned because no non-author maintainers were identified in this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59db20d4f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/firecrawl/skills/firecrawl/scripts/update.sh
# Conflicts:
#	.claude-plugin/marketplace.json
#	README.md
@cursor

cursor Bot commented Jul 11, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_34982183-f427-48f8-9c30-1162644f2dc2)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 19s —— View job


PR Review: feat/publish-firecrawl

  • Gather context and read changed files
  • Review plugin manifest and marketplace entry
  • Review SKILL.md for correctness and security
  • Review update scripts (update.sh / update.test.sh)
  • Review context docs
  • Post final findings

Trigger: This synchronize event is a merge of origin/main (5d7724f — fable-5-playbook) into the branch. The firecrawl plugin files are unchanged from 59db20d. The four items from the first Claude review and one Codex finding remain unaddressed; findings below include those plus two new observations.


Medium — date +%s%N is GNU-only (unaddressed from prior review)

SKILL.md:55,64,75

NONCE=$(date +%s%N)
firecrawl scrape "..." --format markdown -o "/tmp/fc-${NONCE}.md"

%N (nanoseconds) is a GNU date extension. On macOS/BSD, date +%s%N emits the literal string %N after the Unix timestamp — producing /tmp/fc-1720694123%N.md. The file is still created and functional, but the nonce provides zero collision-avoidance on macOS. Three occurrences at lines 55, 64, 75.

Portable fix — replace all three occurrences with mktemp:

OUTFILE=$(mktemp /tmp/fc-XXXXXX.md)
firecrawl scrape "https://www.gnu.org/software/bash/manual/bash.html" \
  --format markdown -o "$OUTFILE"
# Then: Read "$OUTFILE" with offset/limit as needed

If the NONCE pattern is preferred for readability, $(date +%s)_$$ (seconds + PID) is portable.

Fix this →


Low — Upstream SHA written before Gate 2 (Codex P2, confirmed)

update.sh:212

run_apply() calls rewrite_upstream_md immediately after npm install -g — recording the new upstream SHA before the maintainer has integrated (or even reviewed) the upstream SKILL.md content. update-flow.md step 7 documents UPSTREAM.md update as the final step after Gate-2 content integration, but the script writes it as part of the npm install phase:

# update.sh run_apply():
npm install -g ...              # Gate 1 (conversational)
rewrite_upstream_md "$sha" ...  # ← SHA stamped here
# --- Gate 2 content integration happens externally in skill body ---

If the maintainer runs --apply but then rejects Gate 2 or never integrates the upstream content, UPSTREAM.md now shows the new SHA, and the next --check reports "upstream unchanged" even though SKILL.md still has old content. The sidecar's drift-tracking purpose is defeated.

Suggested approaches:

  1. Move rewrite_upstream_md out of update.sh into a --finalize mode the skill body calls explicitly after Gate 2 succeeds.
  2. Or: record a - Pending upstream SHA: <sha> field during --apply, promoting it to - Upstream SHA256: only when --finalize is called.

This contradicts the pipeline documented in update-flow.md step 7 and is the most substantive logic issue in the PR.


Low — --apply carries no internal guard (unaddressed from prior review)

update.sh:188

SKILL.md documents Gate 1 as "never runs npm install -g without asking first," but the script runs it unconditionally when --apply is passed. A maintainer invoking scripts/update.sh --apply directly silently bypasses Gate 1. Minimum fix — add a warning or --confirmed flag requirement to make the safety story match the documented guarantees:

run_apply() {
  log "WARNING: This installs firecrawl-cli@latest globally (npm install -g)."
  log "Invoked from the skill body only after Gate-1 approval. Ctrl-C to abort."
  ...

Fix this →


Low — --pretty wording in commands.md misleads readers (unaddressed from prior review)

commands.md:36

No --format or --pretty flag — use --json for structured output, pipe through jq for pretty-printing.

--pretty is listed in configuration.md as a global flag available to every subcommand. A reader of commands.md in isolation concludes firecrawl search ... --pretty is invalid when it's actually valid.

Suggested rewording: "No command-specific --format flag. Use --json for structured output; --pretty (global flag, see configuration.md) pretty-prints it, or pipe through jq."

Fix this →


Low — allowed-tools grep glob is path-unconstrained (unaddressed from prior review)

SKILL.md:7

allowed-tools: … Bash(grep -m1 *UPSTREAM.md*)

*UPSTREAM.md* permits grep against any file whose name contains UPSTREAM.md. The actual preamble command is fixed (${CLAUDE_SKILL_DIR}/UPSTREAM.md), making this benign in practice. A narrow comment documenting the accepted breadth would close the ambiguity for future maintainers. If variable expansion is supported in allowed-tools globs: Bash(grep -m1 *${CLAUDE_SKILL_DIR}/UPSTREAM.md*) would tighten it.


Info — recorded_field() uses $1 in unquoted regex context

update.sh:113

grep -m1 "^- $1:" "$UPSTREAM_MD" | awk '{print $NF}'

$1 is interpolated into a regex without escaping. Current callers only pass 'Upstream SHA256' and 'CLI version at sync' — no metacharacters, no real risk. Worth a note if the function's callers ever expand, since a pattern like 'SHA.256' would match differently than intended.


Positive notes

  • Manifest: plugin.json schema-valid with explicit version: 0.1.0; marketplace category: utilities and tags are appropriate; $schema on SchemaStore.
  • Frontmatter: All required fields present. Single-line description with Use when: and skip guidance is exactly right. user-invocable: true, disable-model-invocation: false correct.
  • Narrow preamble grants: Bash(command -v firecrawl*), Bash(firecrawl --status*), Bash(grep -m1 *UPSTREAM.md*) are the minimum for the three preamble lines — no broad shell grant. The printenv credential preamble was correctly removed before submission.
  • Write-to-disk pattern: Enforced consistently across all non-trivial examples with explicit justification for the direct-stdout exception. The "32–35× token savings" claim is cited.
  • --fail on all curl calls: --fail is correctly applied to both curl calls (lines 91, 102), preventing error pages from being silently hashed as if they were valid SKILL.md content. This is the most important guard in the update path.
  • Cross-platform SHA: sha256() wrapper correctly falls back to shasum -a 256 on macOS.
  • Portable date in rewrite_upstream_md: date -u +%Y-%m-%d (not %N) is portable. No macOS issue here.
  • Source-guard: Correctly prevents main from running during source; SOURCED_TMPDIR cleanup in the test avoids leaking the script's own TMPDIR_RUN.
  • firecrawl init prohibition: Explicit and prominent in both Gotchas and Prerequisites.
  • Repo-agnostic: All paths go through ${CLAUDE_SKILL_DIR}, ${SCRIPT_DIR}/.., or /tmp/. No hardcoded repo-specific values.
  • No PII/secrets: Correct; FIRECRAWL_API_KEY never stored or printed by the plugin.
  • context/configuration.md: Correctly disambiguates CLI env vars from MCP-era FIRECRAWL_RETRY_* and FIRECRAWL_CREDIT_*_THRESHOLD vars — good defensive documentation.

Merge recommendation: The date +%s%N portability issue (macOS) and the upstream-SHA-before-Gate-2 logic issue are both worth fixing before merge. The SHA issue in particular makes UPSTREAM.md misleading in the case of a Gate-2 rejection. The other three items are documentation and safety-messaging hygiene.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: medium. Cursor Bugbot did not complete successfully (check skipped; usage limit reached), so automated review is incomplete and I am not approving. Human review is needed before merge; no reviewers were assigned because no non-author maintainers were identified in this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit 214eed1 into main Jul 11, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the feat/publish-firecrawl branch July 11, 2026 11:51

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3184b4e3f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

## agent — natural-language web research task

```bash
firecrawl agent "<prompt>" --model <spark-1-mini|spark-1-pro> -o <path>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add --wait when saving agent results

For natural-language research tasks, the Firecrawl CLI starts an agent job asynchronously by default—the current CLI docs describe basic agent usage as returning a job ID immediately and show persisted results with --wait -o. As written, this reference tells Claude to run firecrawl agent ... -o <path> and then read the file, so the agent path will read only the job status/ID instead of the completed research output whenever this command is used; include --wait here or document an explicit poll step.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant