From 626c565a70a5543e8e4fbcafe40f6239e7873f97 Mon Sep 17 00:00:00 2001 From: gavrielc Date: Sat, 18 Apr 2026 15:48:43 +0300 Subject: [PATCH] fix(modules): break circular import TDZ between index.ts and modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #3 introduced a circular-import temporal-dead-zone bug that didn't surface in unit tests but crashed the service at startup: src/index.ts imports './modules/index.js' for side effects → src/modules/interactive/index.ts calls registerResponseHandler() → that function is declared in src/index.ts → but src/index.ts's const responseHandlers = [] hasn't been initialized yet (we're in the middle of its module-init) → ReferenceError: Cannot access 'responseHandlers' before initialization Same issue for registerResponseHandler itself (the function reference resolves to undefined) and for onShutdown in the approvals module. Caught when the operator started the service and systemd flagged the process as crashing in auto-restart loop. Fix: extract responseHandlers + registerResponseHandler + shutdownCallbacks + onShutdown into src/response-registry.ts, which has no dependencies on src/index.ts or on modules. index.ts re-exports the same surface for any existing consumers; modules import directly from response-registry.js. The bug was latent because: - Unit tests import pieces, never src/index.ts's main() flow. - Host builds clean because TypeScript doesn't catch runtime circular init order. - Only surfaces when the ES module loader actually executes src/index.ts as the entry point. Verified: service boots on Linux host with approvals + interactive loaded; OneCLI handler starts via onDeliveryAdapterReady callback. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/index.ts | 61 +++++++---------------- src/modules/approvals/index.ts | 2 +- src/modules/approvals/response-handler.ts | 2 +- src/modules/interactive/index.ts | 2 +- src/response-registry.ts | 45 +++++++++++++++++ 5 files changed, 65 insertions(+), 47 deletions(-) create mode 100644 src/response-registry.ts diff --git a/src/index.ts b/src/index.ts index 6b7d79502..ffb273180 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,51 +16,24 @@ import { startHostSweep, stopHostSweep } from './host-sweep.js'; import { routeInbound } from './router.js'; import { log } from './log.js'; -/** - * Response handler registry. - * - * Button-click / question responses arrive via the channel adapter's - * `onAction` callback. Core iterates registered handlers in registration - * order; the first one that returns `true` claims the response. Unclaimed - * responses are logged and dropped. - * - * Current consumers: interactive module (pending_questions), approvals - * module (pending_approvals + OneCLI). If neither is loaded, every click - * logs "Unclaimed response". - */ -export interface ResponsePayload { - questionId: string; - value: string; - userId: string | null; - channelType: string; - platformId: string; - threadId: string | null; -} - -export type ResponseHandler = (payload: ResponsePayload) => Promise; - -const responseHandlers: ResponseHandler[] = []; - -export function registerResponseHandler(handler: ResponseHandler): void { - responseHandlers.push(handler); -} - -/** - * Shutdown callbacks. Modules with teardown needs (timers, outbound sockets) - * register here. Called in registration order during SIGTERM / SIGINT - * before core's delivery/sweep/channel teardown. - * - * Not a general-purpose registry — narrow lifecycle hook only. - */ -type ShutdownCallback = () => void | Promise; -const shutdownCallbacks: ShutdownCallback[] = []; - -export function onShutdown(cb: ShutdownCallback): void { - shutdownCallbacks.push(cb); -} +// Response + shutdown registries live in response-registry.ts to break the +// circular import cycle: src/index.ts imports src/modules/index.js for side +// effects, and the modules call registerResponseHandler/onShutdown at top +// level — which would hit a TDZ error if the arrays lived here. Re-exported +// here so existing callers see the same surface. +import { + registerResponseHandler, + getResponseHandlers, + onShutdown, + getShutdownCallbacks, + type ResponsePayload, + type ResponseHandler, +} from './response-registry.js'; +export { registerResponseHandler, onShutdown }; +export type { ResponsePayload, ResponseHandler }; async function dispatchResponse(payload: ResponsePayload): Promise { - for (const handler of responseHandlers) { + for (const handler of getResponseHandlers()) { try { const claimed = await handler(payload); if (claimed) return; @@ -202,7 +175,7 @@ function buildConversationConfigs(channelType: string): ConversationConfig[] { /** Graceful shutdown. */ async function shutdown(signal: string): Promise { log.info('Shutdown signal received', { signal }); - for (const cb of shutdownCallbacks) { + for (const cb of getShutdownCallbacks()) { try { await cb(); } catch (err) { diff --git a/src/modules/approvals/index.ts b/src/modules/approvals/index.ts index 3f3000860..e159deb79 100644 --- a/src/modules/approvals/index.ts +++ b/src/modules/approvals/index.ts @@ -11,7 +11,7 @@ * - A shutdown callback that stops the OneCLI handler cleanly. */ import { registerDeliveryAction, onDeliveryAdapterReady } from '../../delivery.js'; -import { registerResponseHandler, onShutdown } from '../../index.js'; +import { registerResponseHandler, onShutdown } from '../../response-registry.js'; import { handleAddMcpServer, handleInstallPackages, handleRequestRebuild } from './request-approval.js'; import { handleApprovalsResponse } from './response-handler.js'; import { startOneCLIApprovalHandler, stopOneCLIApprovalHandler } from './onecli-approvals.js'; diff --git a/src/modules/approvals/response-handler.ts b/src/modules/approvals/response-handler.ts index 1f4bc6802..803268a38 100644 --- a/src/modules/approvals/response-handler.ts +++ b/src/modules/approvals/response-handler.ts @@ -16,7 +16,7 @@ import { updateContainerConfig } from '../../container-config.js'; import { buildAgentGroupImage, killContainer, wakeContainer } from '../../container-runner.js'; import { getAgentGroup } from '../../db/agent-groups.js'; import { deletePendingApproval, getPendingApproval, getSession } from '../../db/sessions.js'; -import type { ResponsePayload } from '../../index.js'; +import type { ResponsePayload } from '../../response-registry.js'; import { log } from '../../log.js'; import { writeSessionMessage } from '../../session-manager.js'; import type { PendingApproval } from '../../types.js'; diff --git a/src/modules/interactive/index.ts b/src/modules/interactive/index.ts index f24794b4c..5a3b8afc1 100644 --- a/src/modules/interactive/index.ts +++ b/src/modules/interactive/index.ts @@ -13,7 +13,7 @@ import { getDb, hasTable } from '../../db/connection.js'; import { deletePendingQuestion, getPendingQuestion, getSession } from '../../db/sessions.js'; import { wakeContainer } from '../../container-runner.js'; -import { registerResponseHandler, type ResponsePayload } from '../../index.js'; +import { registerResponseHandler, type ResponsePayload } from '../../response-registry.js'; import { log } from '../../log.js'; import { writeSessionMessage } from '../../session-manager.js'; diff --git a/src/response-registry.ts b/src/response-registry.ts new file mode 100644 index 000000000..60e04c998 --- /dev/null +++ b/src/response-registry.ts @@ -0,0 +1,45 @@ +/** + * Response handler + shutdown callback registries. + * + * Extracted from index.ts so that modules calling `registerResponseHandler()` + * or `onShutdown()` at import time don't hit a TDZ error on the const-array + * declarations. index.ts imports src/modules/index.js for its side effects, + * which triggers module registrations that would otherwise happen before + * index.ts's own const initializers have run. + * + * Keep this file dependency-free (log.js is fine, but nothing from + * modules/* or index.ts itself). Any file imported here must not in turn + * import from src/index.ts, or the cycle returns. + */ + +export interface ResponsePayload { + questionId: string; + value: string; + userId: string | null; + channelType: string; + platformId: string; + threadId: string | null; +} + +export type ResponseHandler = (payload: ResponsePayload) => Promise; + +const responseHandlers: ResponseHandler[] = []; + +export function registerResponseHandler(handler: ResponseHandler): void { + responseHandlers.push(handler); +} + +export function getResponseHandlers(): readonly ResponseHandler[] { + return responseHandlers; +} + +type ShutdownCallback = () => void | Promise; +const shutdownCallbacks: ShutdownCallback[] = []; + +export function onShutdown(cb: ShutdownCallback): void { + shutdownCallbacks.push(cb); +} + +export function getShutdownCallbacks(): readonly ShutdownCallback[] { + return shutdownCallbacks; +}