fix(modules): break circular import TDZ between index.ts and modules

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) <noreply@anthropic.com>
This commit is contained in:
gavrielc
2026-04-18 15:48:43 +03:00
parent a4573395d9
commit 626c565a70
5 changed files with 65 additions and 47 deletions
+17 -44
View File
@@ -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<boolean>;
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<void>;
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<void> {
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<void> {
log.info('Shutdown signal received', { signal });
for (const cb of shutdownCallbacks) {
for (const cb of getShutdownCallbacks()) {
try {
await cb();
} catch (err) {
+1 -1
View File
@@ -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';
+1 -1
View File
@@ -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';
+1 -1
View File
@@ -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';
+45
View File
@@ -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<boolean>;
const responseHandlers: ResponseHandler[] = [];
export function registerResponseHandler(handler: ResponseHandler): void {
responseHandlers.push(handler);
}
export function getResponseHandlers(): readonly ResponseHandler[] {
return responseHandlers;
}
type ShutdownCallback = () => void | Promise<void>;
const shutdownCallbacks: ShutdownCallback[] = [];
export function onShutdown(cb: ShutdownCallback): void {
shutdownCallbacks.push(cb);
}
export function getShutdownCallbacks(): readonly ShutdownCallback[] {
return shutdownCallbacks;
}