Skip to content

fix(mcp): keep hosted scrape read-only - #454

Merged
Max17190 merged 4 commits into
mainfrom
fix/per-surface-instructions-and-read-only-scrape
Oct 1, 2026
Merged

Max17190 merged 4 commits into
mainfrom
fix/per-surface-instructions-and-read-only-scrape

Conversation

@Max17190

@Max17190 Max17190 commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Why

Provider terms acceptance currently shares firecrawl_scrape with page and provider data retrieval. Accepting an agreement changes organization state, so the hosted tool cannot accurately advertise itself as read-only while that operation remains reachable. Named browser profiles also save changes by default unless the request explicitly disables it.

Summary

  • Refuse every firecrawl/terms/* capability except terms/show before forwarding any call, including mixed batches. Terms-required responses preserve the requirements and retry identity, expose the read-only agreement lookup, and direct an organization admin to the existing dashboard acceptance flow. Research-agent input and output guidance uses that same dashboard flow; thread approval resumes research without accepting terms.
  • Restore hosted scrape's readOnlyHint: true. Hosted scrape and search omit browser actions and force named profiles to saveChanges: false. Profile writes remain available through firecrawl_interact.
  • Preserve local browser functionality and accurately annotate local scrape and search as non-read-only because their browser actions and profile saves can change state. Provider discovery itself does not execute or accept terms.
  • Select compact server instructions for each session's actual full, search, or keyless tool surface. Reuse the shared source opt-out guidance; detailed catalogue and provider-selection guidance remains in the tool metadata. The instructions fit 1,024 characters, with the main routing in the first 512.

Tool annotations describe the available operations; client approval policies still control whether a particular call prompts the user. Hosted scrape retains provider retrieval and isolated processing of retained results without changing account, provider, or website state.

Test Plan

  • pnpm test: all 171 tests pass.
  • Hosted full and search surfaces reject direct, normalized, and mixed-batch terms writes before any API call, while terms/show remains usable. Research-agent metadata directs acceptance to the dashboard and preserves the approval ID, provider requirements, and thread continuation contract.
  • Hosted URL scrape and search requests strip browser actions and explicitly send profile.saveChanges: false; local search preserves the requested actions and saves with the correct non-read-only annotation.
  • Session instruction and tool metadata checks cover keyed, OAuth, keyless, local, and hosted surfaces. Bundled runtime and plugin contract checks pass.
  • pnpm exec tsc --noEmit, ESLint on changed source files, and git diff --check pass.
  • Frozen lockfile installation succeeds; the lockfile diff contains only the runtime patch hash.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 18 files

Re-trigger cubic

Comment thread src/instructions.ts Outdated
Comment thread src/instructions.ts
Comment thread README.md Outdated
Comment thread tests/mcp-description-budget.test.mjs
Comment thread tests/helpers/instructions.mjs
@Max17190
Max17190 force-pushed the fix/per-surface-instructions-and-read-only-scrape branch from d7e0168 to 6a80d5c Compare September 27, 2026 20:19

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread CHANGELOG.md Outdated
@Max17190
Max17190 force-pushed the fix/per-surface-instructions-and-read-only-scrape branch from 6a80d5c to cbd92d9 Compare September 27, 2026 21:29
- Serve one short instructions string per surface (full, search, keyless)
  from src/instructions.ts. Each fits in 1,024 characters, routes search and
  scrape in its first 512, and names only tools the session lists. Hosted
  /v2/mcp selects them per session through a new fastmcp
  `instructionsForSession` option (pnpm patch), so API-key and OAuth sessions
  no longer receive the keyless text. Locally, only stdio without a key or
  FIRECRAWL_API_URL gets the keyless text; the HTTP transport requires one.
- Accept Alexandria provider terms in the dashboard. firecrawl_scrape still
  reads terms with terms/show but refuses every other terms/* capability, and
  terms errors link an organization admin to requiresAction.url or the data
  sources settings page.
- Annotate hosted firecrawl_scrape with readOnlyHint: true again. Hosted
  scrape and search scrapeOptions load a named profile with saveChanges:
  false; saving browser state goes through firecrawl_interact. Local scrape
  keeps actions and writable profiles and stays readOnlyHint: false.
- Keep firecrawl_agent at readOnlyHint: false: the research agent can click,
  fill forms, and navigate interactive pages.
- Bump to 3.26.0.
@Max17190
Max17190 force-pushed the fix/per-surface-instructions-and-read-only-scrape branch from cbd92d9 to efb8392 Compare September 27, 2026 22:33
@Max17190

Copy link
Copy Markdown
Member Author

@cubic review

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

@cubic review

@Max17190 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 17 files

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Fix all with cubic | Re-trigger cubic

Comment thread tests/mcp-instructions.test.mjs
@Max17190 Max17190 changed the title Per-surface server instructions and read-only hosted scrape fix(mcp): keep hosted scrape read-only Oct 1, 2026
@Max17190

Max17190 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@cubic review

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic review

@Max17190 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 16 files

Confidence score: 3/5

  • src/index.ts rejects the documented firecrawl_agent terms-approval request through firecrawl_scrape, preventing clients from completing the flow; update the guard to allow the terms/accept request.
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="src/index.ts">

<violation number="1" location="src/index.ts:2783">
P2: This shared guard makes the documented `firecrawl_agent` terms-approval flow impossible: clients are told to call `terms/accept` through `firecrawl_scrape`, but this line rejects it. Update the agent’s terms-approval contract and instructions to use dashboard acceptance, or preserve a supported acceptance path for the agent.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/index.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0 issues found across 5 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: Auto-approval blocked by 2 unresolved P0–P2 issues from previous reviews.

Re-trigger cubic

@Max17190

Max17190 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@cubic review

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic review

@Max17190 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Auto-approved.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 17 files

Requires human review: Restricts hosted scrape to read-only by refusing terms writes, forcing read-only profiles, restoring readOnlyHint, and adding per-session instructions; docs and tests updated. This changes the public MCP contract and terms-acceptance workflow, so it needs human sign-off.

Fix all with cubic | Re-trigger cubic

Comment thread CHANGELOG.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Requires human review: Restores hosted scrape read-only by refusing terms writes, stripping hosted browser actions, forcing profile.saveChanges=false, and swapping per-surface instructions. Requires human sign-off because it changes terms-acceptance authorization and hosted tool behavior.

Re-trigger cubic

@Max17190
Max17190 merged commit 5ca2c86 into main Oct 1, 2026
2 checks passed
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