Skip to content

feat: resolve redirects before mapping urls#2439

Merged
amplitudesxd merged 3 commits intomainfrom
tom/eng-4022-map-doesnt-work-on-redirects-for-example-firecrawlcom
Nov 25, 2025
Merged

feat: resolve redirects before mapping urls#2439
amplitudesxd merged 3 commits intomainfrom
tom/eng-4022-map-doesnt-work-on-redirects-for-example-firecrawlcom

Conversation

@amplitudesxd
Copy link
Copy Markdown
Member

@amplitudesxd amplitudesxd commented Nov 24, 2025

Summary by cubic

Resolve and follow URL redirects before running map so sites like firecrawl.com (redirects to firecrawl.dev) are mapped correctly. Fixes ENG-4022 where map failed on redirected domains.

  • Bug Fixes
    • Added resolveRedirects that follows redirects (HEAD first, GET fallback) with a secure dispatcher.
    • Map uses resolveRedirects; test verifies links resolve to the final domain.

Written for commit b825250. Summary will update automatically on new commits.

Copy link
Copy Markdown
Contributor

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

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

Prompt for AI agents (all 2 issues)

Understand the root cause of the following 2 issues and fix them.


<file name="apps/api/src/__tests__/snips/v2/map.test.ts">

<violation number="1" location="apps/api/src/__tests__/snips/v2/map.test.ts:128">
Wrap this redirect test in concurrentIf(ALLOW_TEST_SUITE_WEBSITE) (or use a mock) so it doesn’t make real network calls when external access is disabled.</violation>
</file>

<file name="apps/api/src/lib/validateUrl.ts">

<violation number="1" location="apps/api/src/lib/validateUrl.ts:213">
Propagate AbortError instead of swallowing it so cancellation via the provided AbortSignal actually rejects the promise.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

Comment thread apps/api/src/__tests__/snips/v2/map.test.ts Outdated
Comment thread apps/api/src/lib/validateUrl.ts
@amplitudesxd
Copy link
Copy Markdown
Member Author

@cubic-dev-ai re-review

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai Bot commented Nov 24, 2025

@cubic-dev-ai re-review

@amplitudesxd I've started the AI code review. It'll take a few minutes to complete.

Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="apps/api/src/lib/validateUrl.ts">

<violation number="1" location="apps/api/src/lib/validateUrl.ts:213">
Abort signal is swallowed in resolveRedirects, so canceled map jobs keep running instead of stopping when the abort signal fires.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

Comment thread apps/api/src/lib/validateUrl.ts
@amplitudesxd
Copy link
Copy Markdown
Member Author

@cubic-dev-ai re-review

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai Bot commented Nov 24, 2025

@cubic-dev-ai re-review

@amplitudesxd I've started the AI code review. It'll take a few minutes to complete.

Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="apps/api/src/lib/validateUrl.ts">

<violation number="1" location="apps/api/src/lib/validateUrl.ts:212">
HEAD responses are returned without checking for 405/501, so servers that disallow HEAD never hit the GET fallback and their redirects still are not resolved.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

Comment thread apps/api/src/lib/validateUrl.ts
@amplitudesxd amplitudesxd requested a review from mogery November 24, 2025 23:38
Copy link
Copy Markdown
Member

@mogery mogery left a comment

Choose a reason for hiding this comment

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

LGTM

@amplitudesxd amplitudesxd merged commit 2c23e32 into main Nov 25, 2025
6 of 8 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.

2 participants