Skip to content

feat(cli): submit keyless job feedback - #221

Open
Max17190 wants to merge 24 commits into
mainfrom
max/enable-keyless-feedback
Open

Max17190 wants to merge 24 commits into
mainfrom
max/enable-keyless-feedback

Conversation

@Max17190

@Max17190 Max17190 commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

Why

Let agents discover and optionally submit evidence for keyless Search, Scrape, and Parse using the API contract in #4616.

Summary

  • Extend the existing firecrawl feedback command with task, assessment, observation JSON/files, and Parse document class. Keyless submissions omit Authorization.
  • Preserve job references and invitations in JSON output and stderr, including failed jobs, while preserving normal content on stdout.
  • Keep a short optional reminder in command help and stderr. Bundled skills ask agents to submit concise keyless feedback on observed result quality or missing coverage when the host permits it, especially for wrong, incomplete, blocked, or failed results. Feedback is not a completion requirement, and declined or rejected feedback is not retried.
  • Authenticated Search feedback instructions keep their existing wording, scoped to authenticated callers.
  • Default keyless Search to web results while retaining authenticated defaults, explicit sources, and the Map feedback auth gate.
  • Preserve API validation details and retry timing for keyless rejections. Preserve authenticated feedback contracts and preferences. Keep the preference helper in the existing command module.

Release after the API is deployed and Docs #1414 is published.

Test Plan

  • Current head 739a19f: synced with main; all 704 tests across 38 files passed, plus build, TypeScript checks, formatting, and diff whitespace checks.

  • Coverage includes keyless requests, same-request references on success/failure, evidence flags and help, JSON output, API validation/retry details, case-normalized auth gates, authenticated Search defaults, and feedback preferences.

  • All 861 API/OpenAPI schema parity cases passed across the aligned API and Docs branches.

  • Clarified submission requirements, the invitation deadline, and the same-caller-IP requirement. Tests restore the original feedback preference environment variable.

  • Built-client exercises against the staging API passed for Search, Scrape, Parse, failed-job references, accepted feedback, and duplicate retries.

After an eligible call, print the job reference and a one-line optional
reminder pointing to `firecrawl feedback <endpoint> <jobId> --help` instead of
the full server message and flag template. Remove the daily submission rule and
the exact attempt rate from help, the README, and bundled skills, and drop the
retired DAILY_LIMIT_REACHED code.
Resolve conflicts with Alexandria session feedback and the Alexandria-only
search command:

- Send Alexandria session fields for endpoint alexandria and keyless task,
  assessment, document class, and observations for job feedback.
- Keep the keyless feedback help beside the updated search action.
- Keep the Alexandria guidance and relocated completion criteria in the
  bundled skills alongside the keyless feedback contract.
- Describe keyless feedback as requested in exchange for free keyless use in
  the stderr reminder, help, README, and skills.
Point to feedback when a result is wrong, incomplete, blocked, or an
error, matching the API invitation and MCP wording, instead of framing
keyless use as an exchange for feedback. Updates the reminder, command
help, README and skills.
@Max17190
Max17190 marked this pull request as ready for review October 1, 2026 16:08

@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.

Review completed against the latest diff

Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic

Comment thread src/commands/feedback.ts
Comment thread src/index.ts
Comment thread src/commands/search.ts
Comment thread skills/firecrawl-search/SKILL.md Outdated
Comment thread src/commands/feedback.ts
Comment thread src/commands/scrape.ts
Comment thread README.md Outdated
Comment thread src/__tests__/commands/parse.test.ts Outdated
Comment thread src/commands/feedback.ts Outdated
Comment thread src/commands/scrape.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.

All reported issues were addressed across 18 files

Shadow auto-approve: would not auto-approve. Auto-approval blocked because this review re-detected 1 unresolved P0–P2 issue already reported by Cubic.

Re-trigger cubic

@Max17190

Max17190 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@cubic-dev-ai review this PR in full at the current head.

Use the API contract in firecrawl/firecrawl#4616 as the source of truth. Keyless feedback is optional for Search, Scrape, and Parse. Map feedback remains authenticated. Keyless evidence requirements are checked before submission; authenticated fields and preferences are retained. Search JSON preserves references even for empty results. Successful Scrape invitations are in data.metadata; failed jobs carry top-level metadata and are handled by the shared keyless request path, including HTTP 200 responses with success: false.

Please recheck the complete diff and the prior findings against the current implementation.

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR in full at the current head.

Use the API contract in firecrawl/firecrawl#4616 as the source of truth. Keyless feedback is optional for Search, Scrape, and Parse. Map feedback remains authenticated. Keyless evidence requirements are checked before submission; authenticated fields and preferences are retained. Search JSON preserves references even for empty results. Successful Scrape invitations are in data.metadata; failed jobs carry top-level metadata and are handled by the shared keyless request path, including HTTP 200 responses with success: false.

Please recheck the complete diff and the prior findings against the current implementation.
...

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

@Max17190

Max17190 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@cubic-dev-ai review this PR in full at the current head.

Use the API contract in firecrawl/firecrawl#4616 as the source of truth. Keyless feedback is optional for Search, Scrape, and Parse. Map feedback remains authenticated. Keyless evidence requirements are checked before submission; authenticated fields and preferences are retained. Search JSON preserves references even for empty results. Successful Scrape invitations are in data.metadata; failed jobs carry top-level metadata and are handled by the shared keyless request path, including HTTP 200 responses with success: false.

The latest revision also displays keyless validation details and retry timing while preserving authenticated error output.

Please recheck the complete diff and the prior findings against the current implementation.

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR in full at the current head.

Use the API contract in firecrawl/firecrawl#4616 as the source of truth. Keyless feedback is optional for Search, Scrape, and Parse. Map feedback remains authenticated. Keyless evidence requirements are checked before submission; authenticated fields and preferences are retained. Search JSON preserves references even for empty results. Successful Scrape invitations are in data.metadata; failed jobs carry top-level metadata and are handled by the shared keyless request path, including HTTP 200 responses with success: false.

The latest revision also displays keyless validation details and retry timing while preserving authenticated error output.
...

@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 19 files

Confidence score: 5/5

  • In parse.test.ts, the keyless parse path never reads FIRECRAWL_NO_ENDPOINT_FEEDBACK or sends an opt-out header, so this test doesn't verify the env var's effect. Update it to assert behavior the keyless path actually supports.
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/__tests__/commands/parse.test.ts">

<violation number="1" location="src/__tests__/commands/parse.test.ts:77">
P3: This test stubs `FIRECRAWL_NO_ENDPOINT_FEEDBACK` and asserts the request headers, but the keyless parse path never reads that env var and never sends an opt-out header, so the stub has no effect and the assertion is an exact duplicate of the "posts to /v2/parse without auth" test's header check. Either remove the test or make it assert the actual opt-out-relevant behavior of this PR: the keyless parse still reports the feedback invitation on stderr even when the opt-out env is set (as `feedback-invitation.test.ts` pins down).</violation>
</file>

Shadow auto-approve: would require human review. Adds keyless feedback submissions with new required fields and an auth bypass for feedback on search/scrape/parse; needs human review of the authorization and API contract changes.

Fix all with cubic | Re-trigger cubic

Comment thread src/__tests__/commands/parse.test.ts Outdated
@Max17190

Max17190 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@cubic-dev-ai review the latest changes at the current head.

Removed the redundant Parse opt-out test reported in the latest review and the equivalent shared-request header tests. Invitation behavior remains covered by the helper and command tests plus actual API staging flows. The feedback auth gate now uses the same case normalization as the existing endpoint parser; regression tests verify that Search, Scrape, and Parse case variants remain keyless while Map variants retain authentication.

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review the latest changes at the current head.

Removed the redundant Parse opt-out test reported in the latest review and the equivalent shared-request header tests. Invitation behavior remains covered by the helper and command tests plus actual API staging flows. The feedback auth gate now uses the same case normalization as the existing endpoint parser; regression tests verify that Search, Scrape, and Parse case variants remain keyless while Map variants retain authentication.

@Max17190 A review is already in progress. Try again after it finishes.

@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 4 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 require human review. Adds keyless feedback submission for search/scrape/parse, a new required-field contract, and an auth-gate bypass for feedback; this is an authorization/data-handling change tied to an unreleased API contract.

Re-trigger cubic

@Max17190

Max17190 commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

@cubic-dev-ai review the complete current PR diff after syncing with main. Use firecrawl/firecrawl#4616 as the source of truth for optional keyless feedback. Verify the 24-hour window, evidence requirements, preserved job references on success and failure, ownership, one submission per job, separate attempt limits, no keyless refunds or quota resets, and preservation of authenticated feedback. Recheck prior findings against the current code.

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review the complete current PR diff after syncing with main. Use firecrawl/firecrawl#4616 as the source of truth for optional keyless feedback. Verify the 24-hour window, evidence requirements, preserved job references on success and failure, ownership, one submission per job, separate attempt limits, no keyless refunds or quota resets, and preservation of authenticated feedback. Recheck prior findings against the current code.

@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 18 files

Confidence score: 4/5

  • The README.md can be read as starting the 24-hour window when the reference arrives, but the API measures it from job creation. Clarify that submissions are due within 24 hours of job creation.
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="README.md">

<violation number="1" location="README.md:476">
P2: This can imply a fresh 24-hour window after receiving the reference, but the API deadline is based on the job timestamp. State that submissions must be made within 24 hours of job creation.</violation>
</file>

Shadow auto-approve: would not auto-approve because issues were found.

Fix all with cubic | Re-trigger cubic

Comment thread skills/firecrawl-search/SKILL.md Outdated
Comment thread README.md Outdated
Comment thread README.md
Comment thread src/__tests__/utils/feedback-invitation.test.ts 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 4 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 not auto-approve. Auto-approval blocked by 1 unresolved P0–P2 issue from previous reviews.

Re-trigger cubic

…ck guidance

Keyless guidance now asks agents to submit concise feedback on observed
result quality or missing coverage when the host permits it, especially
for wrong, incomplete, blocked, or failed results. It states that feedback
does not determine whether the task is complete and should not be retried
after a decline or rejection.

The search skill no longer lists sending feedback as a completion
criterion. The authenticated search feedback instructions return to their
existing wording, scoped to authenticated callers.

@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.

2 issues found across 3 files (changes from recent commits).

Confidence score: 4/5

  • In skills/firecrawl/SKILL.md, the keyless requirements list omits --doc-class, but executeEndpointFeedback enforces it for keyless Parse and will reject the call without it — add it to the list so agents stop tripping that error.
  • The sentence that starts 'For non-search endpoint jobs' then lists search as supported contradicts itself; rewrite it so agents can tell whether firecrawl feedback search <id> is valid.
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="skills/firecrawl/SKILL.md">

<violation number="1" location="skills/firecrawl/SKILL.md:120">
P2: The keyless requirement list omits `--doc-class`, which `executeEndpointFeedback` enforces for keyless Parse: it requires task, assessment, observations, and `--doc-class` when `endpoint === 'parse'`, throwing `Keyless feedback requires --doc-class.` when missing (src/commands/feedback.ts, required map). README.md also states "Keyless Parse also requires `--doc-class born_digital|scanned|mixed|unknown`." This sentence claims to cover keyless Search, Scrape, and Parse, so an agent following it for a Parse job is rejected by the CLI. Add the Parse-only requirement to the flag list.</violation>

<violation number="2" location="skills/firecrawl/SKILL.md:128">
P3: This sentence is self-contradictory: it says "For non-search endpoint jobs" but then lists `search` among the supported endpoints. An agent will be unable to tell whether `firecrawl feedback search <id>` is valid. The prior wording described `feedback <endpoint>` without the "non-search" qualifier; spell out the scrape/parse/map scoping explicitly, or drop "non-search" and keep the endpoint list.</violation>
</file>

Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread skills/firecrawl/SKILL.md
After using search results, send `firecrawl search-feedback` (the first feedback per search refunds 1 credit). The full pattern, guard, and rules live in [firecrawl-search](../firecrawl-search/SKILL.md).
**Authenticated:** After using search results, send `firecrawl search-feedback` (the first feedback per search refunds 1 credit). The full pattern, guard, and rules live in [firecrawl-search](../firecrawl-search/SKILL.md).

**Keyless:** Submit concise feedback on observed result quality or missing coverage when a keyless Search, Scrape, or Parse result includes a feedback invitation and the host permits it, especially if the result is wrong, incomplete, blocked, or an error. Submit it with `firecrawl feedback <endpoint> <jobId>` with `--rating`, `--task`, `--assessment`, and `--observations-file`, using the job reference and invitation returned in metadata or stderr. Feedback does not determine whether the task is complete. If it is unavailable, declined, or rejected, continue without retries. Run `firecrawl feedback --help` for category-specific observations.

@cubic-dev-ai cubic-dev-ai Bot Oct 3, 2026 •

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.

P2: The keyless requirement list omits --doc-class, which executeEndpointFeedback enforces for keyless Parse: it requires task, assessment, observations, and --doc-class when endpoint === 'parse', throwing Keyless feedback requires --doc-class. when missing (src/commands/feedback.ts, required map). README.md also states "Keyless Parse also requires --doc-class born_digital|scanned|mixed|unknown." This sentence claims to cover keyless Search, Scrape, and Parse, so an agent following it for a Parse job is rejected by the CLI. Add the Parse-only requirement to the flag list.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. 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. At skills/firecrawl/SKILL.md, line 120:

<comment>The keyless requirement list omits `--doc-class`, which `executeEndpointFeedback` enforces for keyless Parse: it requires task, assessment, observations, and `--doc-class` when `endpoint === 'parse'`, throwing `Keyless feedback requires --doc-class.` when missing (src/commands/feedback.ts, required map). README.md also states "Keyless Parse also requires `--doc-class born_digital|scanned|mixed|unknown`." This sentence claims to cover keyless Search, Scrape, and Parse, so an agent following it for a Parse job is rejected by the CLI. Add the Parse-only requirement to the flag list.</comment>

<file context>
@@ -115,15 +115,17 @@ Single format outputs raw content. Multiple formats (e.g., `--format markdown,li
-Consider submitting feedback, especially if a keyless Search, Scrape, or Parse result is wrong, incomplete, blocked, or an error. Include specific evidence to help improve Firecrawl. Submit it with `firecrawl feedback <endpoint> <jobId>` with `--rating`, `--task`, `--assessment`, and `--observations-file`. Use the job reference and invitation returned in metadata or stderr. Run `firecrawl feedback --help` for category-specific observations.
+**Authenticated:** After using search results, send `firecrawl search-feedback` (the first feedback per search refunds 1 credit). The full pattern, guard, and rules live in [firecrawl-search](../firecrawl-search/SKILL.md).
+
+**Keyless:** Submit concise feedback on observed result quality or missing coverage when a keyless Search, Scrape, or Parse result includes a feedback invitation and the host permits it, especially if the result is wrong, incomplete, blocked, or an error. Submit it with `firecrawl feedback <endpoint> <jobId>` with `--rating`, `--task`, `--assessment`, and `--observations-file`, using the job reference and invitation returned in metadata or stderr. Feedback does not determine whether the task is complete. If it is unavailable, declined, or rejected, continue without retries. Run `firecrawl feedback --help` for category-specific observations.
 
 For Alexandria feedback about a provider result or coverage gap, see [firecrawl-alexandria](../firecrawl-alexandria/SKILL.md).
</file context>
Fix with cubic

Comment thread skills/firecrawl/SKILL.md

The two authentication modes use different request contracts; do not add credentials to submit feedback for a keyless job.

For non-search endpoint jobs, use `firecrawl feedback <endpoint> <jobId>` to send concise job-level feedback through `/v2/feedback`. Supported endpoints are `search`, `scrape`, `parse`, and `map`. The following example is for authenticated feedback:

@cubic-dev-ai cubic-dev-ai Bot Oct 3, 2026 •

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.

P3: This sentence is self-contradictory: it says "For non-search endpoint jobs" but then lists search among the supported endpoints. An agent will be unable to tell whether firecrawl feedback search <id> is valid. The prior wording described feedback <endpoint> without the "non-search" qualifier; spell out the scrape/parse/map scoping explicitly, or drop "non-search" and keep the endpoint list.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. 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. At skills/firecrawl/SKILL.md, line 128:

<comment>This sentence is self-contradictory: it says "For non-search endpoint jobs" but then lists `search` among the supported endpoints. An agent will be unable to tell whether `firecrawl feedback search <id>` is valid. The prior wording described `feedback <endpoint>` without the "non-search" qualifier; spell out the scrape/parse/map scoping explicitly, or drop "non-search" and keep the endpoint list.</comment>

<file context>
@@ -115,15 +115,17 @@ Single format outputs raw content. Multiple formats (e.g., `--format markdown,li
+The two authentication modes use different request contracts; do not add credentials to submit feedback for a keyless job.
 
-Authenticated callers can use `firecrawl feedback <endpoint> <jobId>` with the existing issue/note fields for `search`, `scrape`, `parse`, and `map`. The following example is for authenticated feedback:
+For non-search endpoint jobs, use `firecrawl feedback <endpoint> <jobId>` to send concise job-level feedback through `/v2/feedback`. Supported endpoints are `search`, `scrape`, `parse`, and `map`. The following example is for authenticated feedback:
 
 ```bash
</file context>
Suggested change
For non-search endpoint jobs, use `firecrawl feedback <endpoint> <jobId>` to send concise job-level feedback through `/v2/feedback`. Supported endpoints are `search`, `scrape`, `parse`, and `map`. The following example is for authenticated feedback:
For endpoint jobs, use `firecrawl feedback <endpoint> <jobId>` to send concise job-level feedback through `/v2/feedback`. Supported endpoints are `search`, `scrape`, `parse`, and `map`. The following example is for authenticated feedback:
Fix with cubic

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