Merge pull request #2695 from makeittech/fix/gh-2638-chat-ui-freeze
fix(server): rebind message-stream upstreams after a managed OpenCode restart (#2638)
This commit is contained in:
@@ -112,7 +112,7 @@ This module provides OpenCode server integration utilities for the web server ru
|
||||
The runtime maintains active-session count incrementally from idempotent activity phase transitions. Upstream stall-timeout and lifecycle health checks read it in O(1); the hourly cleanup removes activity phases older than 24 hours without broadcasting synthetic state transitions. Snapshot generation remains reserved for the session-activity API.
|
||||
|
||||
## Public exports (lifecycle.js)
|
||||
- `createOpenCodeLifecycleRuntime(dependencies)`: creates lifecycle runtime for managed/external OpenCode process orchestration.
|
||||
- `createOpenCodeLifecycleRuntime(dependencies)`: creates lifecycle runtime for managed/external OpenCode process orchestration. The optional `onOpenCodeRestarted` dependency (default `null`) is fired after a successful managed restart; `index.js` wires it to `messageStreamRuntime.rebindUpstream()` so event-stream readers rebind to the possibly-new port (a restart can land on a new port while an orphaned process keeps the old one, which would otherwise leave the chat UI silent — issue #2638).
|
||||
- Returned API:
|
||||
- `startOpenCode()`
|
||||
- `restartOpenCode()`
|
||||
|
||||
@@ -48,6 +48,7 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
||||
getActiveSessionCount = () => 0,
|
||||
reapManagedOrphanedProcesses = reapOrphanedProcesses,
|
||||
getWarmupDirectories = async () => [],
|
||||
onOpenCodeRestarted = null,
|
||||
now = Date.now,
|
||||
} = deps;
|
||||
|
||||
@@ -694,6 +695,17 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
||||
setupProxy(state.expressApp);
|
||||
ensureOpenCodeApiPrefix();
|
||||
}
|
||||
|
||||
// The restart may have landed on a NEW port (the old one can remain
|
||||
// occupied by an orphaned process, e.g. Windows killProcessOnPort is a
|
||||
// no-op). Upstream event readers pinned to the old process would keep
|
||||
// the UI silent forever, so rebind them to the current port. Best
|
||||
// effort: a failure here must not fail the restart itself.
|
||||
try {
|
||||
onOpenCodeRestarted?.();
|
||||
} catch (error) {
|
||||
console.warn('Failed to rebind event stream after OpenCode restart:', error?.message ?? error);
|
||||
}
|
||||
})();
|
||||
|
||||
try {
|
||||
|
||||
@@ -287,6 +287,70 @@ describe('OpenCode lifecycle', () => {
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('calls onOpenCodeRestarted after a successful managed restart', async () => {
|
||||
const close = vi.fn(async () => {});
|
||||
const replacement = createMockChild();
|
||||
const onOpenCodeRestarted = vi.fn();
|
||||
globalThis.fetch = vi.fn(async () => ({
|
||||
ok: false,
|
||||
json: async () => null,
|
||||
}));
|
||||
spawnMock.mockImplementationOnce(() => {
|
||||
queueMicrotask(() => {
|
||||
replacement.stdout.emit('data', 'opencode server listening on http://127.0.0.1:45678\n');
|
||||
});
|
||||
return replacement;
|
||||
});
|
||||
const runtime = createRuntime({ onOpenCodeRestarted }, {
|
||||
openCodePort: 45678,
|
||||
openCodeProcess: {
|
||||
pid: null,
|
||||
exitCode: 1,
|
||||
signalCode: null,
|
||||
close,
|
||||
},
|
||||
});
|
||||
|
||||
await runtime.triggerHealthCheck();
|
||||
|
||||
expect(close).toHaveBeenCalledTimes(1);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
// The restart completed on a (possibly new) port — the event-stream
|
||||
// upstreams must rebind so the UI keeps receiving events (#2638).
|
||||
expect(onOpenCodeRestarted).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not call onOpenCodeRestarted when a managed restart fails', async () => {
|
||||
const close = vi.fn(async () => {});
|
||||
const onOpenCodeRestarted = vi.fn();
|
||||
globalThis.fetch = vi.fn(async () => ({
|
||||
ok: false,
|
||||
json: async () => null,
|
||||
}));
|
||||
spawnMock.mockImplementation(() => {
|
||||
const child = createMockChild();
|
||||
queueMicrotask(() => {
|
||||
child.emit('error', new Error('spawn failed'));
|
||||
});
|
||||
return child;
|
||||
});
|
||||
const runtime = createRuntime({ onOpenCodeRestarted }, {
|
||||
openCodePort: 45678,
|
||||
openCodeProcess: {
|
||||
pid: null,
|
||||
exitCode: 1,
|
||||
signalCode: null,
|
||||
close,
|
||||
},
|
||||
});
|
||||
|
||||
// triggerHealthCheck logs instead of rethrowing; call restartOpenCode
|
||||
// directly to observe the failure result.
|
||||
await expect(runtime.restartOpenCode()).rejects.toThrow();
|
||||
|
||||
expect(onOpenCodeRestarted).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('launches managed OpenCode with the managed PATH', async () => {
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
const child = createMockChild();
|
||||
|
||||
Reference in New Issue
Block a user