fix(start-plugin-core): re-try failed paths correctly

[roopakv]

retryCount / retryDelay on the prerender don't actually do anything right now.

We had a race condition where the service wasnt up yet so fetch failed, so adding a retry was a simple fix. The retry branch calls addCrawlPageTask(page) again, but that path is already in the seen set from the first attempt, so addCrawlPageTask early-returns and the task is never re-queued. The page just gets dropped. We never reach the failOnError throw, so the failure is swallowed and the build finishes with "Prerendered 0 pages" and exit code is 0 though some pages failed.

I ran into this in SPA mode, the shell prerender occasionally races the preview server and fails with ECONNREFUSED, and setting retryCount did nothing.

Fix is to drop the path from seen before re-queuing so the retry actually re-fetches. retriesByPath still bounds it, so it stops after retryCount attempts and falls through to failOnError.

Added a test that fails the first fetch with retryCount: 1 and checks the page is fetched twice.

Note: I used AI to debug and come up with this fix.

Summary by CodeRabbit

• Bug Fixes

  • Improved prerender retry handling so pages that fail temporarily can be crawled again on retry.
  • Fixed an issue where retried pages could be skipped after an initial failed fetch.

• Tests

  • Added coverage for prerender retries to verify failed requests are retried successfully.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change fixes a deduplication bug in prerender's crawl retry logic by clearing the page path from the "seen" set before re-adding the crawl task on retry. A new test file validates that failed page fetches are retried correctly.

Changes

Prerender Retry Fix

Layer / File(s)	Summary
Fix seen-set deduplication on retry packages/start-plugin-core/src/prerender.ts	Removes current page.path from the seen set before re-adding the crawl task, allowing retries to bypass the earlier deduplication guard.
Retry behavior test packages/start-plugin-core/tests/prerender-retry.test.ts	Adds mocks for logger and filesystem, test helpers for start config and HTML responses, and a test asserting the request handler is called twice when the first fetch fails.

Estimated code review effort: 2 (Simple) | ~10 minutes

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main fix: failed prerender paths are retried correctly in start-plugin-core.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/start-plugin-core/tests/prerender-retry.test.ts (1)

5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor: any usage weakens type safety.

vi.importActual<any> and the as any cast on makeStartConfig's return bypass type checking. As per coding guidelines, **/*.{ts,tsx} should "Use TypeScript strict mode with extensive type safety." Consider typing these against the actual module/config shapes if convenient, though this is common practice for test mocks.

Also applies to: 13-13, 50-50

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/start-plugin-core/tests/prerender-retry.test.ts` at line 5, The test
in `prerender-retry.test.ts` is weakening type safety by using
`vi.importActual<any>` and casting `makeStartConfig` to `any`. Update the mock
setup to use the real module/config types instead of `any`, ideally by typing
the imported `utils` module and the `makeStartConfig` return value against their
actual shapes. Keep the changes local to the test helpers so the existing
`vi.importActual` and `makeStartConfig` usage remain easy to locate while
removing the unsafe casts.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/start-plugin-core/tests/prerender-retry.test.ts`:
- Line 5: The test in `prerender-retry.test.ts` is weakening type safety by
using `vi.importActual<any>` and casting `makeStartConfig` to `any`. Update the
mock setup to use the real module/config types instead of `any`, ideally by
typing the imported `utils` module and the `makeStartConfig` return value
against their actual shapes. Keep the changes local to the test helpers so the
existing `vi.importActual` and `makeStartConfig` usage remain easy to locate
while removing the unsafe casts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6030041-e6ad-495d-9f01-db4996df4f17

📥 Commits

Reviewing files that changed from the base of the PR and between ecbbd9a and 7e29255.

📒 Files selected for processing (2)

• packages/start-plugin-core/src/prerender.ts
• packages/start-plugin-core/tests/prerender-retry.test.ts

[roopakv]

@schiller-manuel any chance you could take a look at this one? This is the last thing keeping us from switching to tanstack start entirely :)