fix(ssr): keep cold module graphs alive while work progresses - #3027
Merged
Conversation
Browser artifacts now remove the module-scope closure used only by server hooks while preserving client references and unrelated side-effect imports. Scope-aware free-variable analysis covers function parameters, hoisting, loops, switch cases, destructuring, JSX, and TypeScript wrappers. Constraint: Browser stripping must not erase unrelated project import side effects Rejected: Redirect every server barrel in the browser | direct project subpath imports would still leak Confidence: high Scope-risk: moderate Directive: Add a focused scope regression before broadening the pruning analysis Tested: browser-server-exports-strip.test.ts (72 steps); deno check; deno lint; deno fmt --check
Development HTML keeps explicit page, layout, JSX runtime, and release-manifest preloads, but no longer treats the legacy SSR route manifest as a browser preload source. Constraint: SSR traversal may include server-only modules that must never reach the browser Rejected: Filter known server module names | transitive project imports are open-ended Confidence: high Scope-risk: narrow Tested: html-shell-generator.test.ts and html-shell-manifest.test.ts; deno check; deno lint; deno fmt --check
Both direct compilation and browser bundling target ES2022 so framework modules using top-level await remain valid through development hydration. Constraint: The framework already ships modules that use top-level await Rejected: Rewrite top-level await call sites | it duplicates module initialization semantics Confidence: high Scope-risk: narrow Tested: compile.test.ts and browser-module-bundler.test.ts; deno check; deno lint; deno fmt --check
The watcher now ignores OMX runtime state and logs, matching other generated output directories and avoiding unrelated HMR work during agent-driven development. Constraint: OMX writes runtime state inside the project tree Confidence: high Scope-risk: narrow Tested: file-watch-setup.test.ts; deno check; deno lint; deno fmt --check
Same-key cache misses now join one in-flight computation while different keys remain concurrent. Failed flights are cleared so later requests can recover normally. Constraint: Concurrent hydration requests can request the same uncached transform Rejected: Raise the module-load timeout alone | duplicate cold work would remain and failures would only be delayed Confidence: high Scope-risk: narrow Directive: Keep flight lifetime tied to the cache lifecycle and preserve retry after failure Tested: transform-cache.test.ts; combined focused suite (183 steps); deno task typecheck
Pages Router collection now skips only files beneath the configured pages api directory while preserving a legitimate root api page. This prevents API handlers from entering browser code splitting and SSG rendering. Constraint: pages/api.tsx remains a valid page at /api; only api directory descendants are handlers Rejected: Exclude every /api URL | that would remove legitimate page routes and couple collection to URL naming Confidence: high Scope-risk: narrow Tested: server/build-routes.test.ts (38 steps); deno task typecheck; deno check; deno lint; deno fmt --check
Project JavaScript and TypeScript modules now pass through the existing server-hook stripper before esbuild builds browser chunks. The splitter retains real source identities and owns local dependency resolution while hook-only Node and root-framework imports disappear before graph traversal. Constraint: Full browser transforms rewrite imports for module-server delivery and cannot be fed directly into the chunk bundler Rejected: Externalize Node builtins and the root framework barrel | that would hide and ship an invalid server dependency graph Confidence: high Scope-risk: moderate Directive: Keep the splitter source transform limited to boundary stripping unless a virtual module resolver is added Tested: real production CodeSplitter node:crypto/root-veryfront regression; splitter plugin tests; browser strip tests; deno task typecheck; deno check; deno lint; deno fmt --check
The production splitter can run before server bootstrap, so it now lazily registers the first-party Babel parser and verifies that lifecycle without test-only setup. Constraint: Production builds may invoke the splitter without extension bootstrap. Rejected: Keep parser registration in test setup | it masked the production lifecycle failure. Confidence: high Scope-risk: narrow Tested: Code-splitter regression suite; framework targeted suite; linked app production build.
The router and UI barrels reach runtime core and color-mode modules that DNT had left connected to Node compatibility shims. Marking those emitted modules browser-safe removes that server graph at package build time. Constraint: Browser code splitting must consume the published npm shape without externalizing all framework UI modules. Rejected: Externalize every Veryfront client subpath | hides packaging defects and prevents normal chunk optimization. Confidence: high Scope-risk: narrow Related: #3025 Tested: Browser-safe package tests; npm package build; esbuild shim scan; linked app production build.
Pages Router SSG now renders with a matching synthetic GET request and URL, allowing page and layout data hooks to receive their normal context. Dynamic route templates are excluded because this build path does not yet expand getStaticPaths and cannot render a literal bracket route. Constraint: Static generation currently has no dynamic path expansion phase. Rejected: Render bracket routes with empty params | produces invalid pages and crashes server-data hooks. Confidence: high Scope-risk: moderate Directive: Add getStaticPaths expansion before including dynamic Pages routes in SSG collection. Tested: Static-generation and route-collection tests; linked app production build and full npm run check.
Server-hook closure pruning now tracks each simple variable declarator independently, so removing a hook-only binding cannot erase a co-declared client side effect. Constraint: Browser pruning must remain conservative for destructuring declarations. Rejected: Keep the whole declaration | leaks hook-only server dependencies into browser graphs. Confidence: high Scope-risk: narrow Tested: browser-server-exports-strip suite (73 steps); focused heartbeat/framework suite (226 steps)
Module loading now uses a 10-second idle deadline that resets only after concrete transform milestones, plus a non-resettable 45-second hard cap. Shared cold transforms broadcast progress to followers, while timed-out callers detach without cancelling work still needed by other renders. Constraint: The outer render pipeline remains capped at 60 seconds. Rejected: Raise the fixed 10-second timeout | would hide genuinely stalled module graphs. Confidence: high Scope-risk: moderate Directive: Add progress marks only after meaningful work completes; never weaken the hard cap. Tested: verify:quick; 14 targeted suites (304 steps); cold six-route comparison
Cold framework work can hold the default three transform permits longer than 500ms. Wait up to five seconds for bounded capacity, preserve active same-key singleflight work, and re-check coordination after a leader failure so followers cannot stampede into duplicate retries. Constraint: Transform concurrency remains bounded by the existing semaphore and per-project limits. Rejected: Raise the default transform capacity to 200 | the same six-route workload passes with the safe default of three when queueing is allowed to drain. Confidence: high Scope-risk: moderate Directive: Do not timeout-delete live singleflight entries; only retry after the leader rejects. Tested: transform concurrency validation; SSR loader suite; six cold concurrent routes returned 200 in 7.6-14.2 seconds
Module artifact hashes come from an unpadded 32-bit hexadecimal value, so randomized transformed paths can legitimately produce fewer than eight characters. Verify the alias against the exact returned artifact instead of assuming fixed hash width. Constraint: Preserve the existing module filename and cache contract while removing a nondeterministic pre-push failure Rejected: Pad every module hash to eight characters | would change production cache identities to satisfy a test-only assumption Confidence: high Scope-risk: narrow Reversibility: clean Directive: Cycle alias tests should compare the alias target with the loader result, not infer a fixed hash width Tested: 50 consecutive module-loader suite runs; module-persistence suite; deno fmt --check Not-tested: None
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves cold concurrent SSR reliability by making module-load timeouts progress-aware, coalescing shared cold transforms, and tightening browser/production boundaries so server-only graphs do not leak into client artifacts.
Changes:
- Add progress-aware idle timeout + hard cap (
withProgressTimeoutThrow) and wire module/transform progress events through the SSR module loader and transform pipeline. - Coalesce same-key cold transform work with singleflight, broadcast leader progress to followers, and detach aborted callers without cancelling shared leaders.
- Strip server-only hook closures from browser bundles earlier in production splitting, adjust static generation route inclusion, and update build/watch tests accordingly.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/validation/002-global-state/002.5-transform-concurrency.test.ts | Updates semaphore timeout test to validate the new longer acquire patience under cold bursts. |
| src/transforms/progress.ts | Introduces a minimal progress event/listener contract for transform milestones. |
| src/transforms/pipeline/types.ts | Threads onProgress through pipeline options/context for milestone reporting. |
| src/transforms/pipeline/stages/ssr-vf-modules/transform.ts | Emits progress milestones for framework transforms and cache hits; plumbs onProgress into framework source transforms. |
| src/transforms/pipeline/stages/ssr-vf-modules/index.ts | Passes pipeline progress listener into framework module transform entrypoints and emits an entry milestone. |
| src/transforms/pipeline/stages/ssr-vf-modules/constants.ts | Adds onProgress to the stage’s transform context. |
| src/transforms/pipeline/stages/compile.ts | Raises esbuild transform target to es2022 to allow modern syntax (notably top-level await). |
| src/transforms/pipeline/stages/compile.test.ts | Adds coverage for top-level await acceptance in compiled framework server modules. |
| src/transforms/pipeline/stages/browser-server-exports-strip.ts | Makes server-only export stripping more scope-aware and more aggressive about removing hook-only imports from browser graphs. |
| src/transforms/pipeline/stages/browser-server-exports-strip.test.ts | Expands regression coverage for hook-closure import stripping, shadowing, and destructuring/default edge cases. |
| src/transforms/pipeline/index.ts | Emits progress milestones across pipeline lifecycle and stages. |
| src/transforms/pipeline/context.ts | Wires onProgress from transform options into the created context. |
| src/transforms/esm/types.ts | Adds onProgress to ESM transform options so module transforms can report milestones. |
| src/transforms/esm/transform-cache.ts | Adds cold-miss singleflight coalescing plus progress broadcast/replay and abort-detach semantics. |
| src/transforms/esm/transform-cache.test.ts | Adds tests for coalescing, progress fanout, listener isolation, abort detach, and failure cleanup. |
| src/server/shared/browser-module-bundler.ts | Raises browser bundle target to es2022 to allow top-level await in browser modules. |
| src/server/shared/browser-module-bundler.test.ts | Adds coverage for bundling browser modules that contain top-level await. |
| src/server/dev-server/file-watch-setup.ts | Ignores .omx/ output to prevent dev watch invalidation loops. |
| src/server/dev-server/file-watch-setup.test.ts | Tests .omx/ ignore behavior on POSIX and Windows paths. |
| src/server/build-routes.ts | Excludes pages/api/* descendants and dynamic Pages Router routes from static generation. |
| src/server/build-routes.test.ts | Tests the new Pages Router route filtering behavior (api descendants, dynamic routes). |
| src/rendering/utils/timeout-enforcement.test.ts | Adds behavioral coverage for idle-reset + hard-cap timeouts. |
| src/rendering/utils/stream-utils.ts | Adds withProgressTimeoutThrow and enriches TimeoutError with kind/lastProgress. |
| src/rendering/utils/index.ts | Re-exports progress-timeout types and helper. |
| src/rendering/orchestrator/pipeline.ts | Switches module loading to progress-aware timeout enforcement; plumbs progress into module loader config. |
| src/rendering/orchestrator/module-loader/module-transform-cache.ts | Propagates progress and abort signals into the transform cache and ESM transform calls. |
| src/rendering/orchestrator/module-loader/index.ts | Emits module-load progress milestones and supports cooperative cancellation via AbortSignal. |
| src/rendering/orchestrator/module-loader/index.test.ts | Adjusts cycle-alias assertions to match the actual emitted artifact naming. |
| src/rendering/orchestrator/module-collection.ts | Defines module-load idle timeout and hard-cap constants. |
| src/rendering/orchestrator/module-collection.test.ts | Verifies updated module-load timeout constants. |
| src/modules/react-loader/ssr-module-loader/loader.ts | Removes time-based in-progress retries and avoids deleting live leader state; relies on outer render deadlines. |
| src/modules/react-loader/ssr-module-loader/constants.ts | Increases semaphore acquire timeout to 5s for cold backpressure. |
| src/html/html-shell-generator.ts | Removes legacy route-manifest module preload hint generation from HTML shell. |
| src/html/html-shell-generator.test.ts | Updates assertions to ensure SSR-derived legacy manifest modules are not emitted as preloads. |
| src/extensions/parser/defaults.ts | Adds lazy registration for the default first-party CodeParser contract. |
| src/build/production-build/static-generation.ts | Passes a synthetic Request/URL context into Pages Router static rendering. |
| src/build/production-build/static-generation.test.ts | Tests synthetic request context propagation during Pages Router static generation. |
| src/build/bundler/code-splitter/splitter.ts | Ensures default parser contracts exist before splitting production bundles. |
| src/build/bundler/code-splitter/splitter.test.ts | Adds coverage that production browser chunks strip server-only page dependencies. |
| src/build/bundler/code-splitter/esbuild-plugin.ts | Adds an onLoad hook to strip server-only exports from in-project JS/TS modules during browser bundling. |
| src/build/bundler/code-splitter/esbuild-plugin.test.ts | Tests the new strip-loader behavior and handler registration count. |
| scripts/build/browser-safe-exports.test.ts | Adds coverage ensuring certain runtime shims are included in browser-safe exports. |
| scripts/build/browser-safe-exports.mjs | Adds missing client modules to the browser-safe export allowlist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aec3a640b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Keep already-aborted followers listener-free, preserve ordinary import side effects when only hook-owned bindings are stripped, and make production Pages rendering explicitly static-data-only so request hooks cannot run during builds. Constraint: Static generation may provide URL context to getStaticData but must never execute getServerData. Rejected: Treat a synthetic GET request as safe build context | it can trigger request-only data and bake private state into static output. Confidence: high Scope-risk: moderate Directive: Keep staticDataOnly as a strict getStaticData gate when extending render context. Tested: 223 focused Deno test steps across transform cache, browser export stripping, static generation, render pipeline, and data fetching. Not-tested: Full repository CI runs after the separate AG-UI work is complete.
Layout and page data hooks can execute for the same pathname, so URL-only cache keys let the first module's props leak into the next. Thread the loaded module path through data fetching and include it in static cache identity. Constraint: Preserve existing pathname and params cache behavior for callers without module identity Rejected: Disable static data caching for layouts | removes valid production caching instead of fixing its identity Confidence: high Scope-risk: narrow Directive: Every independently loaded data module must contribute its module path to static cache identity Tested: Five focused suites, 152 steps; formatting and diff checks Not-tested: Full repository suite deferred to pre-push
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 49 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/build/bundler/code-splitter/esbuild-plugin.ts:22
- Use the internal import alias here instead of a deep relative path. The rest of this file already uses
#veryfront/*, and keeping this consistent avoids brittle../../../paths when directories move.
import { stripServerOnlyExports } from "../../../transforms/pipeline/stages/browser-server-exports-strip.ts";
Direct project agents can now preserve hosted-style invoke_agent declarations as runtime-local tools, serialize that authored tool id across the project-agent boundary, and hydrate explicitly allowed first-party project-file tool schemas when the Veryfront API MCP catalog omits them. The branch also keeps the cold SSR progress and import-stripping review fixes in source and generated bridge artifacts. Constraint: Direct AG-UI routes do not run inside the hosted chat envelope Constraint: Explicit veryfront-api MCP allow policies must remain capability-minimal Rejected: Add legacy agent_* wrapper tools beside invoke_agent | it widens the orchestrator tool surface and breaks hosted skill metadata Rejected: Trust arbitrary missing remote tool names | fallback is limited to known first-party project-file primitives Confidence: high Scope-risk: moderate Tested: deno test --allow-all src/agent/runtime/agent-delegation.test.ts src/agent/factory.test.ts src/agent/project/agent-runtime.test.ts src/agent/runtime/mcp-server-tool-sources.test.ts src/agent/ag-ui/handler.test.ts src/agent/runtime/chat-stream-handler.test.ts Tested: deno task typecheck Tested: deno task build:npm Tested: linked app direct POST /api/ag-ui streamed to RunFinished without Unknown tool references
Cold concurrent renders need progress-aware idle deadlines without letting abandoned transform leaders overwrite replacement cache state. Scope this branch back to SSR loading, serialize per-key transform publication across registry resets, and use identity-aware eviction so callers remain bounded without retry stampedes. Constraint: A caller may detach at its hard deadline while shared transform work continues for other requests Rejected: Delete in-progress state at every caller timeout | permits duplicate leaders and stale cache publication Rejected: Publish cache writes without per-key ordering | an already-started stale backend write can finish after its replacement Confidence: high Scope-risk: moderate Directive: Keep stale-flight eviction, generation identity checks, and per-key cache publication ordering coupled Tested: deno test --allow-all loader.test.ts transform-cache.test.ts singleflight.test.ts (68 steps); deno check focused sources/tests; git diff --check HEAD^ Not-tested: Full repository test suite in this final review pass
Bound repeated rejected-leader retries without adding an async leader-election gap, key static data by the URL query visible to hooks, isolate stale-eviction observers, and document the browser stripping and stage-wide timeout contracts. Canonicalize the splitter project root once so the production boundary check stays cheap. Constraint: Slow healthy transforms must remain progress-aware and shared leaders must not be evicted by impatient followers Rejected: Extract the follower loop into an async helper | an extra await before leader registration allowed concurrent callers to elect multiple leaders Rejected: Sort query parameters in the cache key | URLSearchParams order is observable to getStaticData Rejected: Retry rejected leaders until the outer render deadline | deterministic failures would amplify work and surface as misleading timeouts Confidence: high Scope-risk: moderate Directive: Keep leader election synchronous when no flight exists; keep the one-replacement retry budget identity-guarded Tested: 266 focused test steps; 20 consecutive SSR loader stress runs; changed-file format, lint, typecheck, and diff checks Not-tested: Full repository pre-push gate reruns during push
Module-load heartbeats now isolate listener failures while preserving the abort check ahead of every notification. A focused regression proves a throwing observer cannot fail a transform and a pre-aborted render never invokes the observer. Constraint: Progress callbacks reset idle deadlines but are not part of transformed module semantics. Rejected: Let observer errors propagate for visibility | diagnostic callbacks must not turn healthy module work into render failure. Confidence: high Scope-risk: narrow Directive: Keep abort checks authoritative and ahead of any isolated progress notification. Tested: module-loader index test (14 steps); targeted format and lint. Not-tested: Full repository gate will run in the pre-push hook.
kojiwakayama
enabled auto-merge (squash)
July 22, 2026 21:03
kwakayama
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Cold concurrent SSR could exceed the fixed 10-second module-load deadline even while the transform graph was actively making progress. In the agentic job-submission app, six simultaneous cold requests all failed at about 10.1 seconds with module-loading timeouts.
Raising transform concurrency hid the symptom, but the default bounded queue still needed correct backpressure and liveness semantics.
What changed
Progress-aware module loading
Bounded cold-start coordination
Browser and production boundaries
Gate stability
Cold concurrency evidence
Same six cold requests, default transform capacity 3:
No module-load timeout or transform-capacity error occurred. A control run with transform capacity 200 completed in 7–14 seconds, confirming the remaining cold latency is bounded transform work rather than a deadlock. This PR does not raise the default concurrency.
Verification
deno task verify:quick: formatting, lint, boundaries, docs, and typecheck pass.deno task build:npmpasses.npm run check: production build and all 53 tests pass.Relationship to #3025
This complements #3025 rather than replacing it. #3025 rewrites value imports from the root
veryfrontbarrel to a browser-safe barrel. This PR removes hook-only server closure, prevents generated server shims/preloads from leaking into browser graphs, and fixes the cold SSR coordination and production-build paths. The branches can merge in either order.Known follow-up
Pages Router production builds still do not expand
getStaticPaths. Dynamic templates are excluded from SSG here so literal bracket routes are not emitted; full path expansion remains a separate follow-up.