fix: allow cold remote modules to finish fetching - #3553
Conversation
A cold esm.sh module can take longer than the previous per-attempt deadline, causing three aborted requests even though the upstream response would succeed. Extend one attempt to cover cold generation and lock the boundary with an abort-aware fetch regression. Constraint: Cold esm.sh package generation can exceed 2.5 seconds. Rejected: Add more retries | Repeating the same short deadline never lets a slow successful response finish. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the attempt timeout above realistic cold upstream generation latency. Tested: Red-green HTTP cache regression, 699-step ESM transform suite, format, lint, typecheck, isolated Deno ai-agent journey, and all 21 scaffold/build/serve/browser template-runtime cases. Not-tested: None.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHTTP module fetching now uses shared retry budgets, cancellation, progress reporting, and in-flight waiter management. Abort signals propagate through transforms and SSR module processing. Module-loading timeouts increase to 40 seconds. Runtime command construction is centralized and tested for Deno, npm, and Bun. ChangesHTTP module fetch lifecycle
Template runtime command construction
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e3fc04e9
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f260c7d11c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d7936a82
ℹ️ 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".
The retry-budget regression imports a runtime orchestrator constant from outside its directory. Use the repository import map path so the test follows the same internal import convention as nearby Veryfront modules. Constraint: Review requested the #veryfront rendering orchestrator import path. Rejected: Keep the relative import | inconsistent with internal source import conventions and the PR review request. Confidence: high Scope-risk: narrow Directive: Prefer #veryfront/* imports for cross-subtree internal source references in tests. Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/esm/http-cache.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/esm Tested: deno task fmt:check Tested: deno task lint Not-tested: Full unit suite; change is limited to a test import path.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Cold esm.sh module generation can exceed five seconds, as the full Bun template journey reproduced. Restore the ten-second fetch attempt and expand the module-loading idle window so a stalled attempt can retry before the unchanged hard cap. Constraint: Cold upstream module generation exceeded five seconds in a real Bun production build. Constraint: Module loading must allow a retry while retaining the 45 second hard cap. Rejected: Keep the five second fetch timeout | fails cold ai-agent builds. Rejected: Restore ten seconds without changing module idle timeout | render deadline wins before retry. Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep two HTTP module attempts plus retry delay within MODULE_LOAD_TIMEOUT_MS. Tested: Six-second cold response regression; transform and rendering suites (64 tests, 1086 steps); all seven templates on Node, Bun, and Deno (21 journeys). Not-tested: Native network outages exceeding the 45 second hard cap.
|
@codex review |
|
@coderabbitai review |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/transforms/esm/http-cache.test.ts (1)
101-104: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInclude retry backoff in the timeout invariant.
fetchHttpModuleappliesHTTP_MODULE_FETCH_TIMEOUT_MSper attempt and addsHTTP_MODULE_FETCH_RETRY_DELAY_MSbefore the next attempt insrc/transforms/esm/http-cache.ts:191-254. This assertion checks only the per-attempt timeout. It can pass while retry backoff consumes the remaining module-loading idle window. Include the retry delay in the first-retry deadline assertion.🤖 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 `@src/transforms/esm/http-cache.test.ts` around lines 101 - 104, Update the timeout invariant test around “keeps a retry attempt within the module-loading idle deadline” to include HTTP_MODULE_FETCH_RETRY_DELAY_MS alongside HTTP_MODULE_FETCH_TIMEOUT_MS when comparing against MODULE_LOAD_TIMEOUT_MS / 2. Preserve the existing first-retry deadline constraint while accounting for both the attempt duration and backoff.
🤖 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 `@src/transforms/esm/http-cache.test.ts`:
- Around line 101-104: Update the timeout invariant test around “keeps a retry
attempt within the module-loading idle deadline” to include
HTTP_MODULE_FETCH_RETRY_DELAY_MS alongside HTTP_MODULE_FETCH_TIMEOUT_MS when
comparing against MODULE_LOAD_TIMEOUT_MS / 2. Preserve the existing first-retry
deadline constraint while accounting for both the attempt duration and backoff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ce81730-bb29-4781-ae6d-c86e55607859
📒 Files selected for processing (4)
docs/api-reference/veryfront/utils.mdsrc/rendering/orchestrator/module-collection.test.tssrc/rendering/orchestrator/module-collection.tssrc/transforms/esm/http-cache.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf44da3b1c
ℹ️ 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".
|
Addressed the CodeRabbit retry-backoff nitpick in fbfc017 as part of the shared full-sequence retry budget. The invariant now includes every attempt and backoff. |
|
@codex review |
|
@codex review |
|
The exact-head typecheck exposed a stale generated client template. Refreshed it in bc34d85; the complete deno task typecheck now passes locally. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10f5df7b1a
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d17a9589
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
The template runtime harness installs a packed Veryfront CLI below the generated project so Deno can exercise the release artifact. Treat that hidden dependency root like node_modules during style discovery, preventing its thousands of framework files from exhausting the candidate budget. Constraint: Deno template E2E must run against the packed release artifact. Rejected: Raise the CSS candidate limit | it would mask dependency scanning and increase resource exposure. Confidence: high Scope-risk: narrow Directive: Keep .veryfront-packed-cli outside application style discovery unless it becomes a supported source root. Tested: style-scope-profile unit test; packed Deno multi-agent scaffold, install, serve, HTTP, and browser journey Not-tested: Full 21-case template matrix pending after push
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f51e2dd36
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Why
Cold upstream module generation can exceed the old 2.5-second deadline. The coordinated attempt, retry, in-flight wait, and module-loading budgets let valid cold responses finish without leaving canceled renders doing network or cache work. Concurrent renders can share transforms, framework transforms, and HTTP fetches safely, and useful recursive fetch progress refreshes each active render idle deadline.
Validation
Related: https://github.com/veryfront/veryfront-issue-inbox/issues/456
Summary by CodeRabbit