refactor(dev): replace dynamic updates with immutable sessions - #87
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (60)
📝 WalkthroughWalkthroughThe PR replaces generation-based development updates with immutable Supervisor-managed Sessions. It adds semantic revision preparation, in-memory generated IR publication, revised Webpack and Utoopack adapter contracts, graceful Utoopack worker shutdown, and expanded lifecycle and watcher tests. ChangesDevelopment Session architecture
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant FileWatcher
participant DevSupervisor
participant DevRevision
participant DevSession
participant BundlerAdapter
FileWatcher->>DevSupervisor: report framework input change
DevSupervisor->>DevRevision: prepare candidate revision
DevRevision-->>DevSupervisor: return semantic fingerprint and generated IR
DevSupervisor->>DevSession: close active Session
DevSession->>BundlerAdapter: close controller
DevSupervisor->>DevSession: start replacement Session
DevSession->>BundlerAdapter: start immutable controller
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Move file watching and semantic revision scheduling into a long-lived supervisor. Replace in-place bundler plan transitions with complete immutable sessions, publish generated IR directly, and align the Webpack and Utoopack controller lifecycles.
c5c4957 to
2a69045
Compare
Route Session shutdown through Utoopack's graceful cleanup and wait for an acknowledged worker exit before starting a replacement. Fail-stop on stuck shutdown instead of overlapping cache owners.
Keep Utoopack's native loader scheduler in the long-lived dev host so immutable Sessions can replace their Projects without registering stale Worker callbacks. Preserve build/dev process-mode isolation and fail-stop on loader infrastructure errors.
…supervisor # Conflicts: # packages/bundler-utoopack/tests/adapter.test.ts
![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: cada431cd8
ℹ️ 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".
| }).catch((error) => { | ||
| if (!this.closed) { | ||
| logger.error`Failed to process initial Utoopack dev build: ${error}`; | ||
| } | ||
| }); |
There was a problem hiding this comment.
When onBuildFacts rejects—for example, because a plugin's output or afterBuild hook fails—this catch only logs the error while the controller's done promise remains pending. For a client-only plan no server-stats monitor is installed, so nothing retries processBuildFacts; the default Utoopack dev server remains advertised as ready even though its canonical HTML/manifest was rolled back or never emitted, and later client rebuilds cannot recover it. Reject the controller lifecycle or otherwise schedule a retry instead of swallowing this failure.
AGENTS.md reference: AGENTS.md:L94-L96
Useful? React with 👍 / 👎.
| releaseDistLock = await writeDevDistLock(options.cwd, options.plan.distDir); | ||
| unregisterDistExitCleanup = options.registerExitCleanup(() => | ||
| releaseDistLock?.sync(), | ||
| ); | ||
|
|
||
| controller = await options.bundler.dev({ |
There was a problem hiding this comment.
In a Webpack dev session this lock is created before bundler.dev() starts, but WebpackDevSession.start() immediately removes outputPaths.rootDir (the default dist directory), deleting dist/.evjs-dev.lock that was just written. The session then runs without the dist-lock safeguard, so any operation or external integration relying on assertNoActiveDevDistLock() can incorrectly treat the output as unused and overwrite it. Preserve or recreate the lock after the adapter's startup cleanup, as the previous ordering did.
Useful? React with 👍 / 👎.
1 participant
Summary
Changes
DevSupervisorandDevSessionownership for watchers, ports, plugins, APIs, output queues, locks, and bundler controllers..evafter the old Session closes; remove full-tree dev candidate, previous, restore, and system snapshot flows.@utoo/pack, preventingWorker creator already registeredon the second Session.maininto the Supervisor lifecycle.Validation
npm run check-typesnpm run lintnpm test(17 Turbo tasks passed; EV package 782 tests; Utoopack package 91 tests)npm --workspace evjs-docs run buildgit diff --checkjinni-explorevalidation with@utoo/pack@1.5.1: changepage.config.ts, close Session A, start Session B on the same port, regenerate the manifest, and exit cleanly without cache-lock or scheduler-registration errors.Risk / rollout
@evjs/ev/_internal/buildbundler adapter contract; repository Webpack and Utoopack adapters are migrated together.ev dev.Reviewer notes
done/closebehavior across failure and late callbacks.Summary by CodeRabbit
New Features
Bug Fixes
Documentation