-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor worker message handling to use Comlink #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor worker message handling to use Comlink #148
Conversation
- Install comlink dependency - Refactor jsEval.worker.ts to expose API via Comlink - Refactor pyodide.worker.ts to expose API via Comlink - Refactor ruby.worker.ts to expose API via Comlink - Refactor runtime.tsx to use Comlink wrap() instead of manual message handling - Remove manual ID tracking and callback management - Simplify worker communication using Comlink's RPC pattern Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
my-code | 1e69bec | Commit Preview URL Branch Preview URL |
Dec 09 2025, 03:49 AM |
- Update jsEval.worker.ts to accept optional interruptBuffer parameter - Update ruby.worker.ts to accept optional interruptBuffer parameter - Update pyodide.worker.ts to handle optional interruptBuffer safely - All workers now properly implement the WorkerAPI interface Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
- Add explicit type assertion for empty updatedFiles object - Improves code clarity and consistency Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
| // Reject all pending promises | ||
| const error = "Worker interrupted"; | ||
| messageCallbacks.current.forEach(([, reject]) => reject(error)); | ||
| messageCallbacks.current.clear(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot この処理がなくなったことにより、restart時に実行中のrunCommand(), runFiles(), checkSyntax()の返り値のPromiseが永遠に解決しなくなってしまいます。
実行中のPromiseのreject関数を保存しておき、terminate時にすべてrejectする機能を追加してください。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修正しました。pendingPromises ref を追加して、実行中の Promise の reject 関数を保存し、worker terminate 時にすべて reject するようにしました。trackPromise() ヘルパー関数で runCode(), runFile(), checkSyntax() の呼び出しをラップしています。
コミット: a31f8bb
- Track pending Comlink promises in pendingPromises ref - Add trackPromise helper to wrap worker API calls - Reject all pending promises when worker is terminated/restarted - Prevents promises from hanging forever on worker interruption - Fixes issue where runCommand(), runFiles(), checkSyntax() would never resolve during restart Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
- Use 'unknown' instead of 'any' for reject function parameter - Use Error object instead of string for worker interruption error - Better adherence to TypeScript best practices Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
- Refactor trackPromise to use .finally() for cleaner code - Add argsIgnorePattern and varsIgnorePattern to ESLint config - Allow underscore-prefixed unused parameters (convention) - Improve code readability and maintainability Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
- Update comment to be more accurate about calling reject handlers - Improve code documentation clarity Co-authored-by: na-trium-144 <100704180+na-trium-144@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
17fc327 to
1e69bec
Compare
Plan: Refactor message processing in app/terminal/worker/runtime.tsx to use Comlink
Overview
The issue requests refactoring the message processing implementation in
app/terminal/worker/runtime.tsxto use the Comlink library instead of the current manual postMessage/onmessage implementation.Implementation Complete ✅
All tasks have been successfully completed and all code review feedback has been addressed:
runtime.tsxto use Comlink for worker communicationwrap()pyodide.worker.tsto expose API via Comlinkruby.worker.tsto expose API via ComlinkjsEval.worker.tsto expose API via ComlinkSummary
Changes Made:
comlink@4.4.2dependencyexpose()APIruntime.tsxto use Comlink'swrap()APIWorkerAPIinterface for type safetyunknowninstead ofanytrackPromise()usingPromise.finally()Code Quality:
Benefits:
Security Summary
✅ No security vulnerabilities found - CodeQL scan completed with 0 alerts
Bug Fix (Comment #2598372988)
✅ Fixed the issue where
runCommand(),runFiles(), andcheckSyntax()promises would never resolve when the worker was terminated/restarted.Implementation details:
pendingPromisesref to track all in-flight promisestrackPromise()helper to wrap worker API callsPromise.finally()Commit: d16de35
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.