fix: resolve TUI worker startup crash from circular dependency#47
fix: resolve TUI worker startup crash from circular dependency#47anandgupta42 merged 1 commit intomainfrom
Conversation
The TUI showed an empty screen because the worker thread crashed on startup. ModelsDev.refresh() ran at module load time and accessed Installation.USER_AGENT before the Installation module finished initializing (circular import chain: worker.ts → config.ts → models.ts → installation). Deferring the initial refresh via setTimeout(…, 0) ensures all modules are fully loaded first. Also improves worker error logging to extract the actual error message from ErrorEvent (previously logged as useless "[object ErrorEvent]"), and adds an e2e test that spawns the worker and verifies it starts without errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jontsai
left a comment
There was a problem hiding this comment.
PR #47 Review Tour Guide
fix: resolve TUI worker startup crash from circular dependency
| # | File | +/- | Purpose |
|---|---|---|---|
| 1 | src/cli/cmd/tui/thread.ts |
+1/-1 | Extract useful error info from ErrorEvent |
| 2 | src/provider/models.ts |
+11/-7 | Defer initial ModelsDev.refresh() via setTimeout(…, 0) |
| 3 | test/cli/tui/worker.test.ts |
+70 | E2E worker startup test |
Tour
Stop 1: The Fix (models.ts) — RIGOROUSLY REVIEW
The root cause is a classic circular dependency: worker.ts → config.ts → models.ts → installation, where ModelsDev.refresh() runs at module load time and hits Installation.USER_AGENT before Installation is initialized.
The setTimeout(…, 0) defers execution to the next microtask tick, allowing all modules to finish loading. This is the standard Node.js/Bun pattern for breaking circular init dependencies. The setInterval for periodic refresh is now nested inside the timeout, which is correct — no point scheduling refreshes if the first one hasn't run.
Stop 2: Error Logging (thread.ts) — Notice
Good improvement. ErrorEvent objects stringify to [object ErrorEvent], which is useless for debugging. Now extracts .message and .error?.stack.
Stop 3: E2E Test (worker.test.ts) — Notice
Spawns the actual worker and verifies it starts without errors, then checks RPC communication works. The 3-second wait for module loading is reasonable for an e2e test. Good that the test was verified to catch this exact bug when the fix is reverted.
Summary
Clean, well-diagnosed fix. The setTimeout(…, 0) is the right approach for circular dependency init ordering. No issues found. LGTM.
The TUI showed an empty screen because the worker thread crashed on startup. ModelsDev.refresh() ran at module load time and accessed Installation.USER_AGENT before the Installation module finished initializing (circular import chain: worker.ts → config.ts → models.ts → installation). Deferring the initial refresh via setTimeout(…, 0) ensures all modules are fully loaded first. Also improves worker error logging to extract the actual error message from ErrorEvent (previously logged as useless "[object ErrorEvent]"), and adds an e2e test that spawns the worker and verifies it starts without errors. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
npm install -g @altimateai/altimate-code@0.2.2: The TUI worker thread crashed on startup due to a circular dependency —ModelsDev.refresh()ran at module load time and accessedInstallation.USER_AGENTbefore theInstallationmodule finished initializing (import chain:worker.ts→config.ts→models.ts→installation). Deferred the initial refresh viasetTimeout(…, 0)so all modules are fully loaded first.worker.onerrorhandler now extractse.messageande.error.stackinstead of logging the rawErrorEventobject (which produced the useless[object ErrorEvent]).Test plan
bun test test/cli/tui/worker.test.tspasses (2 tests)TypeError: undefined is not an object (evaluating 'Installation.USER_AGENT'))altimate run "hello"works correctly[object ErrorEvent]in logs🤖 Generated with Claude Code