From 3af23c1c4cd323a4227384fc47d1371ed113e258 Mon Sep 17 00:00:00 2001 From: Bee <68532117+abeatrix@users.noreply.github.com> Date: Tue, 4 Aug 2026 13:28:18 -0700 Subject: [PATCH] fix(cli,core): stop duplicate connector launches (#12770) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: prevent duplicate connector launches during doctor/connect Mark connectors as starting before the hub daemon spawns so autostart skips in-flight instances, and improve doctor process filtering with container-aware namespace/cgroup checks plus detached log rotation. * Update apps/cli/src/connectors/common.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * feat(hub): supervise connector processes * feat(connectors): enable tools by default, and stop replaying the Slack greeting Tools were on by default only for Telegram (via --no-tools); Slack, Discord, Linear, Google Chat and WhatsApp all required an explicit --enable-tools. All six now default to tools on and opt out with --no-tools. --enable-tools still parses everywhere, including Telegram which never accepted it, so deployed scripts, systemd units and persisted autostart arguments keep working. Passing both resolves to the safer answer: --no-tools wins. This also affects hub/webview starts, which never emitted a tools flag and so ran those five connectors with tools off. Slack no longer posts the "Connected to Cline." first-contact message. It was gated on per-thread welcomeSentAt, so a connector restart or a cleared history made the next user message look like first contact and replayed the greeting. The host mechanism is unchanged and the other adapters still greet. Co-Authored-By: Claude Opus 5 * fix(connectors): recover a thread whose session is wedged mid-run A connector thread keeps a long-lived mapping to a hub session. When that session's runtime still had a run in flight and no abort had been requested, every message in the thread came back as "SessionRuntime.shutdown called while a run is in progress" instead of an answer, and stayed that way until someone cleared the binding by hand. Observed on the Cline Mom Slack bot after a stack restart. The connector host already recovers from a session the hub no longer knows about: it forgets the mapping and replays the turn once against a fresh session. This widens the trigger from "session not found" to "session cannot serve another turn" via isUnusableSessionError, so a wedged runtime takes the same path. The shutdown error now carries a stable code (SessionRunInProgressError, session_run_in_progress) so callers can recognise it structurally. The predicate also matches on message, because an error reaching a connector has crossed the hub's JSON boundary and arrives as a bare message - and because a host commonly runs a hub and CLI of different versions. Ordinary run failures still propagate untouched: replacing the session on those would hide real errors and drop the conversation. Co-Authored-By: Claude Opus 5 * fix(connectors): serialise turns that share a session Answering "what happens if I message the bot in another thread while it is still replying": channel threads were already independent, but DMs were not. findBindingForThread deliberately reuses one binding — and therefore one runtime session — for every message in a DM channel, so a DM stays one continuous conversation. The turn queue, though, was keyed by thread id, and a DM thread id carries the message timestamp. Two messages in flight in the same DM therefore got two independent queues and ran concurrently against a single session, which fails with "shutdown called while a run is in progress" or interleaves two conversations in one session history. The queue key now follows the same identity rule as the binding lookup, via resolveThreadTurnQueueKey next to findBindingForThread so the two cannot drift. DM messages queue behind each other on the shared session; channel threads keep their own key and still run in parallel. Applied to all six adapters, which all had the same mismatch. Co-Authored-By: Claude Opus 5 * fix(core): abort an in-flight run before tearing its session down Where the Slack bot's "plugin-sandbox process exited (code=null, signal=SIGTERM)" came from, and its "shutdown called while a run is in progress" sibling: both are one event, a session released while a run was still going. stopSession aborts the agent first "so shutdown can proceed", but callers that reach shutdownSession or releaseSessionRuntime another way did not - hub dispose() on a restart being the one that hurt. Without an abort the runtime refuses to shut down, that error is rethrown from the cleanup, and the plugin sandbox is SIGTERMed while tool calls are still pending, so those calls reject with "plugin-sandbox process exited". A connector turn awaiting the run reports whichever surfaced first instead of answering. Both paths now abort and let the run drain before shutting the agent, runtime and sandbox down, guarded on session.aborting so callers that already aborted do not abort twice. Co-Authored-By: Claude Opus 5 * fix(connectors): stop announcing "Steering current task." Every follow-up sent while the bot was replying added an acknowledgement line to the thread, and the wording overstated what happens: the host treats delivery "steer" the same as "queue", enqueuing the prompt for the session rather than injecting it into the loop already running. The follow-up is now handed over silently. Co-Authored-By: Claude Opus 5 * fix(core): retire a dead supervised entry before replacing it A start arriving while an instance sat in backoff left the old entry's restart timer live. The timer closes over the old entry object, so when it fired it spawned a second process for the same (channel, instanceId) - untracked by the supervisor's map, so invisible to list() and unreachable by stop() - two connectors holding one bot token, which is the exact failure supervision exists to prevent. Its exit handler then kept reaping the live instance's state and rescheduling restarts. The same window exists before the timer is even scheduled: the exit-cleanup chain runs first, and a replacement made mid-chain would be followed by a restart scheduled for the retired entry. start() now retires a dead existing entry explicitly - cancel its timer, mark it stopped, drop its exit listener. Both the timer callback and the cleanup chain already stand down on "stopped", so one mark covers both phases. * fix(core): serialise supervisor start/stop and wait for stopped processes to die Found by exercising a hub restart against a live webhook connector: the new hub's boot reconnect restarts the adopted survivor - which suspends inside stop() on the CLI cleanup - while the user's `cline connect` arrives as connector.start. With no per-instance serialisation the two starts interleaved across that suspension and both spawned. The map tracked one process while the other lived on untracked, holding the connector's webhook port; the tracked chain crash-looped on EADDRINUSE through all five attempts and ended state=failed, while the ghost kept running with no way to reach it through list() or stop(). Two changes: - start/stop (and the backoff-restart spawn) now run under a per- instance-key promise queue, so one instance has exactly one lifecycle operation in flight. The exit-cleanup chain also stands down when its entry is no longer the one in the map. - stop() waits for the process to actually die after SIGTERM (bounded, then SIGKILL) instead of returning while it still holds its listen port - the race that turned the double-spawn into a crash loop, and that could burn a backoff cycle on any webhook connector restart. process.kill is now injectable (killProcess), which also stops the test suite from signalling arbitrary real pids like 600 on the host. Verified live: the same kill-hub-then-reconnect sequence now converges to one tracked running process, with the concurrent user start correctly answered "already running under the hub". --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 Co-authored-by: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com> --- apps/cli/src/commands/connect-via-hub.test.ts | 304 ++++++++ apps/cli/src/commands/connect-via-hub.ts | 289 +++++++ apps/cli/src/commands/connect.test.ts | 200 +++++ apps/cli/src/commands/connect.ts | 144 +++- apps/cli/src/commands/doctor.test.ts | 176 ++++- apps/cli/src/commands/doctor.ts | 133 +++- apps/cli/src/connectors/adapters/discord.ts | 129 ++-- apps/cli/src/connectors/adapters/gchat.ts | 102 +-- apps/cli/src/connectors/adapters/linear.ts | 102 +-- apps/cli/src/connectors/adapters/slack.ts | 139 ++-- apps/cli/src/connectors/adapters/telegram.ts | 102 ++- .../connectors/adapters/tools-default.test.ts | 100 +++ apps/cli/src/connectors/adapters/whatsapp.ts | 103 +-- apps/cli/src/connectors/base.test.ts | 26 +- apps/cli/src/connectors/base.ts | 128 +++- apps/cli/src/connectors/common.test.ts | 37 +- apps/cli/src/connectors/common.ts | 25 + .../cli/src/connectors/connector-host.test.ts | 133 +++- apps/cli/src/connectors/connector-host.ts | 19 +- apps/cli/src/connectors/thread-bindings.ts | 20 + .../src/connectors/thread-turn-queue.test.ts | 132 ++++ apps/cli/src/connectors/types.ts | 6 + apps/cli/src/index.ts | 12 +- apps/cli/src/main.test.ts | 63 ++ apps/cli/src/main.ts | 27 +- apps/examples/desktop-app/sidecar/index.ts | 6 +- apps/examples/menubar/sidecar/index.ts | 14 +- .../core/src/hub/daemon/entry.test.ts | 12 +- sdk/packages/core/src/hub/daemon/entry.ts | 22 + .../handlers/connector-handlers.test.ts | 195 +++++ .../hub/server/handlers/connector-handlers.ts | 82 +- .../src/hub/server/hub-server-transport.ts | 3 + sdk/packages/core/src/index.ts | 21 +- .../runtime/host/local-runtime-host.test.ts | 86 +++ .../src/runtime/host/local-runtime-host.ts | 25 + .../src/runtime/host/runtime-host.test.ts | 82 ++ .../core/src/runtime/host/runtime-host.ts | 41 + .../session-runtime-orchestrator.ts | 28 +- .../connectors/connector-autostart.ts | 3 + .../connectors/connector-child-env.ts | 39 + .../connectors/connector-cleanup.test.ts | 128 ++++ .../services/connectors/connector-cleanup.ts | 115 +++ .../connectors/connector-supervisor.test.ts | 722 ++++++++++++++++++ .../connectors/connector-supervisor.ts | 682 +++++++++++++++++ .../daemon-connector-reconnect.test.ts | 366 ++++----- .../connectors/daemon-connector-reconnect.ts | 175 ++--- .../shared/src/connectors/supervision.ts | 75 ++ sdk/packages/shared/src/hub.ts | 11 +- sdk/packages/shared/src/index.ts | 21 +- .../shared/src/runtime/hub-daemon-env.test.ts | 143 +++- .../shared/src/runtime/hub-daemon-env.ts | 147 +++- sdk/packages/shared/src/storage/index.ts | 1 + sdk/packages/shared/src/storage/paths.ts | 21 + 53 files changed, 5247 insertions(+), 670 deletions(-) create mode 100644 apps/cli/src/commands/connect-via-hub.test.ts create mode 100644 apps/cli/src/commands/connect-via-hub.ts create mode 100644 apps/cli/src/connectors/adapters/tools-default.test.ts create mode 100644 apps/cli/src/connectors/thread-turn-queue.test.ts create mode 100644 sdk/packages/core/src/runtime/host/runtime-host.test.ts create mode 100644 sdk/packages/core/src/services/connectors/connector-child-env.ts create mode 100644 sdk/packages/core/src/services/connectors/connector-cleanup.test.ts create mode 100644 sdk/packages/core/src/services/connectors/connector-cleanup.ts create mode 100644 sdk/packages/core/src/services/connectors/connector-supervisor.test.ts create mode 100644 sdk/packages/core/src/services/connectors/connector-supervisor.ts create mode 100644 sdk/packages/shared/src/connectors/supervision.ts diff --git a/apps/cli/src/commands/connect-via-hub.test.ts b/apps/cli/src/commands/connect-via-hub.test.ts new file mode 100644 index 0000000000..dc4e938fdd --- /dev/null +++ b/apps/cli/src/commands/connect-via-hub.test.ts @@ -0,0 +1,304 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ConnectIo } from "../connectors/types"; + +const mocks = vi.hoisted(() => ({ + ensureDetachedHubServer: vi.fn(), + readHubDiscovery: vi.fn(), + connect: vi.fn(), + command: vi.fn(), + close: vi.fn(), + clientOptions: vi.fn(), +})); + +vi.mock("@cline/core", () => ({ + ensureDetachedHubServer: mocks.ensureDetachedHubServer, + readHubDiscovery: mocks.readHubDiscovery, + resolveProductionHubOwnerContext: () => ({ + ownerId: "hub-production", + discoveryPath: "/tmp/production.json", + }), + resolveSharedHubOwnerContext: () => ({ + ownerId: "hub-owner", + discoveryPath: "/tmp/owner.json", + }), + NodeHubClient: class { + constructor(options: unknown) { + mocks.clientOptions(options); + } + connect = mocks.connect; + command = mocks.command; + close = mocks.close; + }, +})); + +import { startConnectorViaHub, stopConnectorsViaHub } from "./connect-via-hub"; + +describe("startConnectorViaHub", () => { + const io: ConnectIo = { writeln: vi.fn(), writeErr: vi.fn() }; + + beforeEach(() => { + vi.clearAllMocks(); + mocks.ensureDetachedHubServer.mockResolvedValue({ + url: "ws://127.0.0.1:25463/hub", + authToken: "token", + }); + mocks.readHubDiscovery.mockResolvedValue({ + url: "ws://127.0.0.1:25463/hub", + capabilities: ["session.create", "connector.start"], + }); + mocks.connect.mockResolvedValue(undefined); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + function startRequest(overrides: Record = {}) { + return { + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + io, + cwd: "/workspace", + ...overrides, + }; + } + + it("hands the start to the hub and reports supervision", async () => { + mocks.command.mockResolvedValue({ + version: "v1", + ok: true, + payload: { + started: true, + record: { pid: 4242, state: "running" }, + }, + }); + + await expect(startConnectorViaHub(startRequest())).resolves.toEqual({ + delegated: true, + exitCode: 0, + }); + expect(mocks.command).toHaveBeenCalledWith("connector.start", { + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + restart: false, + }); + expect(io.writeln).toHaveBeenCalledWith( + expect.stringContaining("started under hub supervision pid=4242"), + ); + expect(mocks.close).toHaveBeenCalled(); + }); + + it("passes a restart through", async () => { + mocks.command.mockResolvedValue({ + version: "v1", + ok: true, + payload: { started: true, record: { state: "running" } }, + }); + + await startConnectorViaHub(startRequest({ restart: true })); + + expect(mocks.command).toHaveBeenCalledWith( + "connector.start", + expect.objectContaining({ restart: true }), + ); + }); + + it("treats an already-running instance as success", async () => { + mocks.command.mockResolvedValue({ + version: "v1", + ok: true, + payload: { + started: false, + reason: "already_running", + record: { pid: 99, state: "running" }, + }, + }); + + await expect(startConnectorViaHub(startRequest())).resolves.toEqual({ + delegated: true, + exitCode: 0, + }); + expect(io.writeln).toHaveBeenCalledWith( + expect.stringContaining("already running under the hub"), + ); + }); + + it("falls back when the hub cannot be reached", async () => { + mocks.ensureDetachedHubServer.mockRejectedValue(new Error("EADDRINUSE")); + + const outcome = await startConnectorViaHub(startRequest()); + + expect(outcome.delegated).toBe(false); + expect(mocks.command).not.toHaveBeenCalled(); + }); + + it("falls back when a running hub predates connector supervision", async () => { + // The normal state of a long-lived host mid-upgrade: a new CLI, an old hub. + mocks.readHubDiscovery.mockResolvedValue({ + url: "ws://127.0.0.1:25463/hub", + capabilities: ["session.create"], + }); + + const outcome = await startConnectorViaHub(startRequest()); + + expect(outcome).toEqual({ + delegated: false, + reason: "hub does not support connector supervision", + }); + expect(mocks.command).not.toHaveBeenCalled(); + }); + + it("falls back when the hub reports supervision unavailable", async () => { + mocks.command.mockResolvedValue({ + version: "v1", + ok: false, + error: { + code: "connector_command_failed", + message: "connector supervision is unavailable in this hub", + }, + }); + + const outcome = await startConnectorViaHub(startRequest()); + + expect(outcome.delegated).toBe(false); + }); + + it("falls back when the hub command throws", async () => { + mocks.command.mockRejectedValue(new Error("socket closed")); + + const outcome = await startConnectorViaHub(startRequest()); + + expect(outcome.delegated).toBe(false); + expect(mocks.close).toHaveBeenCalled(); + }); + + it("surfaces a genuine start refusal instead of starting locally", async () => { + mocks.command.mockResolvedValue({ + version: "v1", + ok: false, + error: { + code: "connector_command_failed", + message: "instanceId is required", + }, + }); + + await expect(startConnectorViaHub(startRequest())).resolves.toEqual({ + delegated: true, + exitCode: 1, + }); + expect(io.writeErr).toHaveBeenCalledWith( + expect.stringContaining("hub refused to start slack"), + ); + }); + + it("reports a hub that accepted the command but did not start anything", async () => { + mocks.command.mockResolvedValue({ + version: "v1", + ok: true, + payload: { + started: false, + record: { state: "failed", lastError: "bad token" }, + }, + }); + + await expect(startConnectorViaHub(startRequest())).resolves.toEqual({ + delegated: true, + exitCode: 1, + }); + expect(io.writeErr).toHaveBeenCalledWith( + expect.stringContaining("bad token"), + ); + }); +}); + +describe("stopConnectorsViaHub", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.readHubDiscovery.mockResolvedValue({ + url: "ws://127.0.0.1:25463/hub", + capabilities: [ + "connector.start", + "connector.stop", + "connector.supervised", + ], + }); + mocks.connect.mockResolvedValue(undefined); + }); + + it("retires every supervised instance of a channel", async () => { + mocks.command.mockImplementation(async (command: string) => { + if (command === "connector.supervised") { + return { + ok: true, + payload: { + supervised: [ + { channel: "slack", instanceId: "a" }, + { channel: "slack", instanceId: "b" }, + { channel: "telegram", instanceId: "c" }, + ], + }, + }; + } + return { ok: true, payload: { stopped: true } }; + }); + + await expect(stopConnectorsViaHub({ channel: "slack" })).resolves.toBe(2); + expect(mocks.command).toHaveBeenCalledWith("connector.stop", { + channel: "slack", + instanceId: "a", + }); + expect(mocks.command).toHaveBeenCalledWith("connector.stop", { + channel: "slack", + instanceId: "b", + }); + // A different channel is left alone. + expect(mocks.command).not.toHaveBeenCalledWith("connector.stop", { + channel: "telegram", + instanceId: "c", + }); + }); + + it("retires only the requested instance", async () => { + mocks.command.mockImplementation(async (command: string) => { + if (command === "connector.supervised") { + return { + ok: true, + payload: { + supervised: [ + { channel: "slack", instanceId: "a" }, + { channel: "slack", instanceId: "b" }, + ], + }, + }; + } + return { ok: true, payload: { stopped: true } }; + }); + + await expect( + stopConnectorsViaHub({ channel: "slack", instanceId: "b" }), + ).resolves.toBe(1); + expect(mocks.command).toHaveBeenCalledWith("connector.stop", { + channel: "slack", + instanceId: "b", + }); + }); + + it("reports nothing to stop when the hub supervises none of them", async () => { + mocks.command.mockResolvedValue({ ok: true, payload: { supervised: [] } }); + + await expect(stopConnectorsViaHub({ channel: "slack" })).resolves.toBe(0); + }); + + it("returns undefined when the hub cannot supervise", async () => { + mocks.readHubDiscovery.mockResolvedValue({ + url: "ws://127.0.0.1:25463/hub", + capabilities: ["session.create"], + }); + + await expect( + stopConnectorsViaHub({ channel: "slack" }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/apps/cli/src/commands/connect-via-hub.ts b/apps/cli/src/commands/connect-via-hub.ts new file mode 100644 index 0000000000..43481d8f5b --- /dev/null +++ b/apps/cli/src/commands/connect-via-hub.ts @@ -0,0 +1,289 @@ +import { + ensureDetachedHubServer, + NodeHubClient, + readHubDiscovery, + resolveProductionHubOwnerContext, + resolveSharedHubOwnerContext, +} from "@cline/core"; +import { + type ConnectorStartResult, + resolveClineBuildEnv, + type SupervisedConnectorRecord, +} from "@cline/shared"; +import type { ConnectIo } from "../connectors/types"; + +/** + * Error codes that mean "this hub cannot supervise connectors", as opposed to + * "the start failed". Both are answers from the hub, but only the former should + * send the caller back to starting the connector itself. + */ +const UNSUPPORTED_ERROR_CODES = new Set([ + "unsupported_command", + "unsupported_connector_command", +]); +const UNSUPPORTED_MESSAGE_FRAGMENT = "connector supervision is unavailable"; + +export type HubDelegationOutcome = + /** The hub owns the connector now; `exitCode` is the command's result. */ + | { delegated: true; exitCode: number } + /** Nothing was started; the caller should start the connector locally. */ + | { delegated: false; reason: string }; + +function resolveHubOwnerContext() { + return resolveClineBuildEnv() === "production" + ? resolveProductionHubOwnerContext() + : resolveSharedHubOwnerContext(); +} + +/** + * Whether the hub at `url` advertises connector supervision. + * + * A newer CLI regularly talks to an older running hub — that is the normal state + * of a long-lived host mid-upgrade — and such a hub would reject + * `connector.start` outright. Checking the advertised capability first keeps that + * case on the local path instead of turning it into a failed start. + */ +async function hubSupportsSupervision(): Promise { + try { + const owner = resolveHubOwnerContext(); + const record = await readHubDiscovery(owner.discoveryPath); + return record?.capabilities?.includes("connector.start") === true; + } catch { + return false; + } +} + +function describeRecord(record: SupervisedConnectorRecord | undefined): string { + if (!record) { + return ""; + } + const details = [ + record.pid === undefined ? undefined : `pid=${record.pid}`, + `state=${record.state}`, + ].filter(Boolean); + return details.length > 0 ? ` ${details.join(" ")}` : ""; +} + +/** + * What the running hub is supervising, or undefined when it cannot say. + * + * Deliberately does not start a hub: this exists for diagnostics, and `cline + * doctor` reporting on the system must never change it. + */ +export async function listSupervisedConnectorsViaHub(): Promise< + SupervisedConnectorRecord[] | undefined +> { + let url: string; + let authToken: string | undefined; + try { + const owner = resolveHubOwnerContext(); + const record = await readHubDiscovery(owner.discoveryPath); + if ( + !record?.url || + !record.capabilities?.includes("connector.supervised") + ) { + return undefined; + } + url = record.url; + authToken = record.authToken; + } catch { + return undefined; + } + const client = new NodeHubClient({ + url, + ...(authToken ? { authToken } : {}), + clientType: "cli-doctor", + displayName: "doctor", + }); + try { + await client.connect(); + const reply = await client.command("connector.supervised"); + if (!reply.ok) { + return undefined; + } + const supervised = (reply.payload as { supervised?: unknown })?.supervised; + return Array.isArray(supervised) + ? (supervised as SupervisedConnectorRecord[]) + : undefined; + } catch { + return undefined; + } finally { + try { + client.close(); + } catch { + // One-shot connection; a failed close changes nothing. + } + } +} + +/** + * Ask the hub to stop supervising a channel's connectors, or one instance of it. + * + * Returns how many the hub stopped, or undefined when it cannot supervise. The + * local stop path alone is not enough: it finds processes through their state + * files, so a connector that has not written one yet — still starting, or failing + * to start — would keep running under the hub and be restarted. + */ +export async function stopConnectorsViaHub(input: { + channel: string; + instanceId?: string; +}): Promise { + const supervised = await listSupervisedConnectorsViaHub(); + if (!supervised) { + return undefined; + } + const targets = supervised.filter( + (record) => + record.channel === input.channel && + (input.instanceId === undefined || + record.instanceId === input.instanceId), + ); + if (targets.length === 0) { + return 0; + } + let url: string; + let authToken: string | undefined; + try { + const owner = resolveHubOwnerContext(); + const record = await readHubDiscovery(owner.discoveryPath); + if (!record?.url) { + return undefined; + } + url = record.url; + authToken = record.authToken; + } catch { + return undefined; + } + const client = new NodeHubClient({ + url, + ...(authToken ? { authToken } : {}), + clientType: "cli-connect", + displayName: `stop ${input.channel}`, + }); + let stopped = 0; + try { + await client.connect(); + for (const target of targets) { + const reply = await client.command("connector.stop", { + channel: target.channel, + instanceId: target.instanceId, + }); + if (reply.ok) { + stopped += 1; + } + } + return stopped; + } catch { + return stopped > 0 ? stopped : undefined; + } finally { + try { + client.close(); + } catch { + // One-shot connection; a failed close changes nothing. + } + } +} + +/** + * Ask the hub to start and own a connector. + * + * The hub spawning the connector — rather than the connector spawning itself and + * then bringing up a hub — is what makes the hub the single authority on how many + * processes hold one connector's credentials, and what lets it reap and restart + * them when they die. Every failure mode here falls back to the local path so a + * missing or older hub cannot stop a connector from starting. + */ +export async function startConnectorViaHub(input: { + channel: string; + instanceId: string; + args: string[]; + restart?: boolean; + io: ConnectIo; + cwd?: string; +}): Promise { + const cwd = input.cwd ?? process.cwd(); + let hub: { url: string; authToken: string }; + try { + hub = await ensureDetachedHubServer(cwd); + } catch (error) { + return { + delegated: false, + reason: `hub unavailable: ${ + error instanceof Error ? error.message : String(error) + }`, + }; + } + if (!(await hubSupportsSupervision())) { + return { + delegated: false, + reason: "hub does not support connector supervision", + }; + } + + const client = new NodeHubClient({ + url: hub.url, + authToken: hub.authToken, + clientType: "cli-connect", + displayName: `connect ${input.channel}`, + cwd, + }); + try { + await client.connect(); + const reply = await client.command("connector.start", { + channel: input.channel, + instanceId: input.instanceId, + args: input.args, + restart: input.restart === true, + }); + if (!reply.ok) { + const code = reply.error?.code ?? ""; + const message = reply.error?.message ?? "connector start failed"; + if ( + UNSUPPORTED_ERROR_CODES.has(code) || + message.includes(UNSUPPORTED_MESSAGE_FRAGMENT) + ) { + return { delegated: false, reason: message }; + } + input.io.writeErr( + `[connect] hub refused to start ${input.channel}: ${message}`, + ); + return { delegated: true, exitCode: 1 }; + } + const payload = reply.payload as ConnectorStartResult | undefined; + const record = payload?.record; + if (payload?.started === false && payload.reason === "already_running") { + input.io.writeln( + `[connect] ${input.channel} connector ${input.instanceId} is already running under the hub${describeRecord(record)}`, + ); + return { delegated: true, exitCode: 0 }; + } + if (payload?.started !== true) { + input.io.writeErr( + `[connect] hub could not start ${input.channel} connector ${input.instanceId}${ + record?.lastError ? `: ${record.lastError}` : "" + }`, + ); + return { delegated: true, exitCode: 1 }; + } + input.io.writeln( + `[connect] ${input.channel} connector ${input.instanceId} started under hub supervision${describeRecord(record)}`, + ); + input.io.writeln( + "[connect] the hub will restart it if it exits; use `cline connect --stop` to retire it", + ); + return { delegated: true, exitCode: 0 }; + } catch (error) { + return { + delegated: false, + reason: `hub command failed: ${ + error instanceof Error ? error.message : String(error) + }`, + }; + } finally { + try { + client.close(); + } catch { + // The connection is one-shot; a failed close changes nothing. + } + } +} diff --git a/apps/cli/src/commands/connect.test.ts b/apps/cli/src/commands/connect.test.ts index 62c7ea9329..fc2bb8e0fe 100644 --- a/apps/cli/src/commands/connect.test.ts +++ b/apps/cli/src/commands/connect.test.ts @@ -5,6 +5,7 @@ import { } from "../connectors/common"; import type { ConnectIo, ConnectRunContext } from "../connectors/types"; import { + runCleanupConnectorInstance, runConnectAdapter, runRestartConnector, runStopAllConnectors, @@ -12,6 +13,8 @@ import { } from "./connect"; const mocks = vi.hoisted(() => ({ + startConnectorViaHub: vi.fn(), + stopConnectorsViaHub: vi.fn(async () => undefined as number | undefined), disableConnectorAutostart: vi.fn(), getPersistedConnectorConnection: vi.fn(), getConnector: vi.fn(), @@ -36,6 +39,11 @@ vi.mock("../connectors/registry", () => ({ listConnectors: mocks.listConnectors, })); +vi.mock("./connect-via-hub", () => ({ + startConnectorViaHub: mocks.startConnectorViaHub, + stopConnectorsViaHub: mocks.stopConnectorsViaHub, +})); + describe("runConnectAdapter", () => { const previousDetachedChild = process.env[CLINE_CONNECTOR_DETACHED_CHILD_ENV]; const io: ConnectIo = { @@ -492,3 +500,195 @@ describe("runConnectAdapter", () => { expect(mocks.persistConnectorConnection).not.toHaveBeenCalled(); }); }); + +describe("runCleanupConnectorInstance", () => { + const io: ConnectIo = { + writeln: vi.fn(), + writeErr: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("reaps one instance without disabling its autostart", async () => { + const stopInstance = vi.fn().mockResolvedValue({ + stoppedProcesses: 0, + failedProcesses: 0, + stoppedSessions: 2, + }); + mocks.getConnector.mockResolvedValue({ + name: "slack", + description: "Slack", + run: mocks.run, + validate: mocks.validate, + showHelp: vi.fn(), + stopInstance, + }); + + await expect( + runCleanupConnectorInstance("slack", "cline-slack", io), + ).resolves.toBe(0); + + expect(stopInstance).toHaveBeenCalledWith("cline-slack", io); + // The instance crashed; it was not retired. Disabling autostart here would + // make every crash silently opt the connector out of supervision. + expect(mocks.disableConnectorAutostart).not.toHaveBeenCalled(); + }); + + it("reports a failed reap", async () => { + mocks.getConnector.mockResolvedValue({ + name: "slack", + description: "Slack", + run: mocks.run, + validate: mocks.validate, + showHelp: vi.fn(), + stopInstance: vi.fn().mockResolvedValue({ + stoppedProcesses: 0, + failedProcesses: 1, + stoppedSessions: 0, + }), + }); + + await expect( + runCleanupConnectorInstance("slack", "cline-slack", io), + ).resolves.toBe(1); + }); + + it("rejects an adapter without per-instance stop", async () => { + mocks.getConnector.mockResolvedValue({ + name: "slack", + description: "Slack", + run: mocks.run, + validate: mocks.validate, + showHelp: vi.fn(), + }); + + await expect( + runCleanupConnectorInstance("slack", "cline-slack", io), + ).resolves.toBe(1); + expect(io.writeErr).toHaveBeenCalledWith( + 'connect adapter "slack" does not support per-instance stop', + ); + }); + + it("rejects an unknown adapter", async () => { + mocks.getConnector.mockResolvedValue(undefined); + + await expect( + runCleanupConnectorInstance("nope", "instance", io), + ).resolves.toBe(1); + expect(io.writeErr).toHaveBeenCalledWith('unknown connect adapter "nope"'); + }); +}); + +describe("hub-delegated connector starts", () => { + const io: ConnectIo = { writeln: vi.fn(), writeErr: vi.fn() }; + + beforeEach(() => { + vi.clearAllMocks(); + mocks.listConnectors.mockReturnValue([]); + mocks.listActiveConnectors.mockReturnValue([]); + mocks.validate.mockResolvedValue(0); + mocks.run.mockResolvedValue(0); + mocks.getConnector.mockResolvedValue({ + name: "slack", + description: "Slack", + run: mocks.run, + validate: mocks.validate, + showHelp: vi.fn(), + resolveInstanceId: () => "cline-slack", + }); + mocks.startConnectorViaHub.mockResolvedValue({ + delegated: true, + exitCode: 0, + }); + }); + + afterEach(() => { + delete process.env.CLINE_CONNECTOR_SUPERVISED; + }); + + it("asks the hub to own a background connector and records the intent", async () => { + await expect( + runConnectAdapter("slack", ["--bot-token", "xoxb"], io), + ).resolves.toBe(0); + + expect(mocks.startConnectorViaHub).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }), + ); + // The adapter must not also run here: the hub owns the process now. + expect(mocks.run).not.toHaveBeenCalled(); + expect(mocks.persistConnectorConnection).toHaveBeenCalledWith( + "slack", + "cline-slack", + ["--bot-token", "xoxb"], + ); + }); + + it("runs locally for a foreground connector", async () => { + await runConnectAdapter("slack", ["--bot-token", "xoxb", "-i"], io); + + expect(mocks.startConnectorViaHub).not.toHaveBeenCalled(); + expect(mocks.run).toHaveBeenCalled(); + }); + + it("runs locally inside a supervised process instead of asking the hub again", async () => { + process.env.CLINE_CONNECTOR_SUPERVISED = "1"; + + await runConnectAdapter("slack", ["--bot-token", "xoxb"], io); + + // Delegating here would send the hub straight back to spawning this same + // process. + expect(mocks.startConnectorViaHub).not.toHaveBeenCalled(); + expect(mocks.run).toHaveBeenCalled(); + }); + + it("runs locally when the instance id cannot be known up front", async () => { + mocks.getConnector.mockResolvedValue({ + name: "telegram", + description: "Telegram", + run: mocks.run, + validate: mocks.validate, + showHelp: vi.fn(), + resolveInstanceId: () => undefined, + }); + + await runConnectAdapter("telegram", ["-k", "token"], io); + + expect(mocks.startConnectorViaHub).not.toHaveBeenCalled(); + expect(mocks.run).toHaveBeenCalled(); + }); + + it("falls back to a local start when the hub declines", async () => { + mocks.startConnectorViaHub.mockResolvedValue({ + delegated: false, + reason: "hub does not support connector supervision", + }); + + await runConnectAdapter("slack", ["--bot-token", "xoxb"], io); + + expect(mocks.run).toHaveBeenCalled(); + }); + + it("does not validate or delegate a help invocation", async () => { + await runConnectAdapter("slack", ["--help"], io); + + expect(mocks.startConnectorViaHub).not.toHaveBeenCalled(); + expect(mocks.validate).not.toHaveBeenCalled(); + }); + + it("reports a validation failure without contacting the hub", async () => { + mocks.validate.mockResolvedValue(2); + + await expect( + runConnectAdapter("slack", ["--bot-token", "bad"], io), + ).resolves.toBe(2); + expect(mocks.startConnectorViaHub).not.toHaveBeenCalled(); + expect(mocks.run).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/cli/src/commands/connect.ts b/apps/cli/src/commands/connect.ts index f0c50bd60d..4a881125dc 100644 --- a/apps/cli/src/commands/connect.ts +++ b/apps/cli/src/commands/connect.ts @@ -5,6 +5,10 @@ import { persistConnectorConnection, removePersistedConnectorConnection, } from "@cline/core"; +import { + isSupervisedConnectorProcess, + setStartingConnectorInstance, +} from "@cline/shared"; import { CLINE_CONNECTOR_DETACHED_CHILD_ENV, CONNECT_ALREADY_RUNNING_EXIT_CODE, @@ -15,6 +19,7 @@ import type { ConnectRunContext, ConnectStopResult, } from "../connectors/types"; +import { startConnectorViaHub, stopConnectorsViaHub } from "./connect-via-hub"; const HELP_FLAGS = new Set(["-h", "--help"]); const INTERACTIVE_FLAGS = new Set(["-i", "--interactive"]); @@ -83,6 +88,20 @@ export async function runStopConnector( io.writeErr(`connect adapter "${adapterName}" does not support stop`); return 1; } + // Retire it with the hub first. The local stop below finds processes through + // their state files, so a supervised connector that has not written one yet + // would survive and be restarted. + const stoppedByHub = await stopConnectorsViaHub({ + channel: connector.name, + ...(options.instanceId === undefined + ? {} + : { instanceId: options.instanceId }), + }); + if (stoppedByHub) { + io.writeln( + `[connect] hub stopped supervising ${stoppedByHub} ${connector.name} connector${stoppedByHub === 1 ? "" : "s"}`, + ); + } const result = await stop(); if (!result) { io.writeErr(`connect adapter "${adapterName}" does not support stop`); @@ -97,6 +116,39 @@ export async function runStopConnector( return result.failedProcesses === 0 ? 0 : 1; } +/** + * Reap one connector instance that is no longer running. + * + * Invoked by the hub supervisor when it observes a connector die. It clears the + * same things a normal stop does — process state file, thread→session bindings, + * the instance's hub sessions — but deliberately leaves the autostart record + * intact: the instance crashed, it was not retired, so the supervisor still + * intends to restart it. `runStopConnector` with `autostart: "disable"` would + * make every crash silently opt the connector out of recovery. + */ +export async function runCleanupConnectorInstance( + adapterName: string, + instanceId: string, + io: ConnectIo, +): Promise { + const connector = await getConnector(adapterName); + if (!connector) { + io.writeErr(`unknown connect adapter "${adapterName}"`); + return 1; + } + if (!connector.stopInstance) { + io.writeErr( + `connect adapter "${adapterName}" does not support per-instance stop`, + ); + return 1; + } + const result = await connector.stopInstance(instanceId, io); + io.writeln( + `[connect] ${connector.name} instance=${instanceId} cleaned processes=${result.stoppedProcesses} failed=${result.failedProcesses} sessions=${result.stoppedSessions}`, + ); + return result.failedProcesses === 0 ? 0 : 1; +} + export async function runRestartConnector( adapterName: string, passthroughArgs: string[], @@ -132,6 +184,21 @@ export async function runRestartConnector( if (validationExitCode !== 0) { return validationExitCode; } + // The supervisor replaces an instance in one step, so let it do the whole + // restart rather than stopping here and racing it to start the replacement. + // Only when the target is the instance these arguments describe: a + // `--restart-instance` pointing elsewhere is not ours to reinterpret. + if ( + requestedInstanceId === undefined || + connector.resolveInstanceId?.(passthroughArgs) === requestedInstanceId + ) { + const delegated = await tryDelegateToHub(connector, passthroughArgs, io, { + restart: true, + }); + if (delegated !== undefined) { + return delegated; + } + } const previousConnection = getPersistedConnectorConnection( adapterName, instanceId, @@ -208,6 +275,14 @@ async function runConnectAdapterWithResult( }, setPersistenceInstanceId: (instanceId) => { persistenceInstanceId = instanceId; + // Adapters report their instance id before they build a Cline core, so + // this lands in the environment before the hub daemon is spawned and + // inherited by it. Without it the daemon's autostart pass cannot tell + // that this instance is mid-startup and launches a second copy of it. + setStartingConnectorInstance({ + channel: connector.name, + instanceId, + }); }, }; const exitCode = await connector.run(passthroughArgs, io, context); @@ -218,8 +293,12 @@ async function runConnectAdapterWithResult( const isInteractiveInvocation = passthroughArgs.some((arg) => INTERACTIVE_FLAGS.has(arg), ); + // A supervised process is the hub's own connector, not a user invocation, so + // it makes the same autostart bookkeeping choices as a detached child: the + // process that asked for the start already recorded the intent. const isDetachedChild = - process.env[CLINE_CONNECTOR_DETACHED_CHILD_ENV] === "1"; + process.env[CLINE_CONNECTOR_DETACHED_CHILD_ENV] === "1" || + isSupervisedConnectorProcess(); if ( exitCode === 0 && !isHelpInvocation && @@ -242,11 +321,74 @@ async function runConnectAdapterWithResult( return { exitCode, instanceId: persistenceInstanceId }; } +/** + * Hand a background connector start to the hub, when that is possible. + * + * Returns the exit code once the hub owns the connector, or undefined to mean + * "start it locally instead". Delegation is skipped for foreground (`-i`) runs, + * which are attached to the user's terminal, and for connectors the hub itself + * launched, which would otherwise ask the hub to start them again. + */ +async function tryDelegateToHub( + connector: { + name: string; + validate: (args: string[], io: ConnectIo) => Promise; + resolveInstanceId?: (args: string[]) => string | undefined; + }, + passthroughArgs: string[], + io: ConnectIo, + options: { restart?: boolean } = {}, +): Promise { + if ( + passthroughArgs.some((arg) => HELP_FLAGS.has(arg)) || + passthroughArgs.some((arg) => INTERACTIVE_FLAGS.has(arg)) || + process.env[CLINE_CONNECTOR_DETACHED_CHILD_ENV] === "1" || + isSupervisedConnectorProcess() + ) { + return undefined; + } + // Without an instance id the hub cannot enforce one process per connector, + // which is the entire point of routing through it. + const instanceId = connector.resolveInstanceId?.(passthroughArgs); + if (!instanceId) { + return undefined; + } + // Check the arguments here rather than after handing off: a bad token should + // fail in front of the user instead of becoming a supervised crash loop. + const validationExitCode = await connector.validate(passthroughArgs, io); + if (validationExitCode !== 0) { + return validationExitCode; + } + const outcome = await startConnectorViaHub({ + channel: connector.name, + instanceId, + args: passthroughArgs, + ...(options.restart === undefined ? {} : { restart: options.restart }), + io, + }); + if (!outcome.delegated) { + return undefined; + } + if (outcome.exitCode === 0) { + // Recorded here rather than in the hub-spawned process: this is the + // invocation that expressed the intent to keep the connector running. + persistConnectorConnection(connector.name, instanceId, passthroughArgs); + } + return outcome.exitCode; +} + export async function runConnectAdapter( adapterName: string, passthroughArgs: string[], io: ConnectIo, ): Promise { + const connector = await getConnector(adapterName); + if (connector) { + const delegated = await tryDelegateToHub(connector, passthroughArgs, io); + if (delegated !== undefined) { + return delegated; + } + } const result = await runConnectAdapterWithResult( adapterName, passthroughArgs, diff --git a/apps/cli/src/commands/doctor.test.ts b/apps/cli/src/commands/doctor.test.ts index 0a5a92bf06..5fbe92eafc 100644 --- a/apps/cli/src/commands/doctor.test.ts +++ b/apps/cli/src/commands/doctor.test.ts @@ -24,6 +24,7 @@ const { mockEnsureFileExists, mockListActiveConnectors, mockStopAllConnectors, + mockListSupervisedConnectors, } = vi.hoisted(() => ({ mockSpawnSync: vi.fn(), mockResolveClineDataDir: vi.fn(() => "/tmp/cline-data"), @@ -58,6 +59,7 @@ const { stoppedSessions: 0, executed: 0, })), + mockListSupervisedConnectors: vi.fn(async () => undefined as unknown), })); vi.mock("node:child_process", () => ({ @@ -84,7 +86,11 @@ vi.mock("./connect", () => ({ stopAllConnectors: mockStopAllConnectors, })); -import { createDoctorCommand, runDoctorCommand } from "./doctor"; +vi.mock("./connect-via-hub", () => ({ + listSupervisedConnectorsViaHub: mockListSupervisedConnectors, +})); + +import { __test__, createDoctorCommand, runDoctorCommand } from "./doctor"; describe("runDoctorCommand", () => { const tempDirs: string[] = []; @@ -450,3 +456,171 @@ describe("createDoctorCommand log subcommand", () => { expect(errors[0]).toContain("open failed"); }); }); + +describe("container-aware process filtering", () => { + const { decideForeignContainer, CONTAINER_CGROUP_PATTERN } = __test__; + + it("treats a process in a different pid namespace as foreign", () => { + expect( + decideForeignContainer({ + platform: "linux", + namespacePairs: [ + ["pid:[4026531836]", "pid:[4026532500]"], + [undefined, undefined], + ], + ownContainerId: undefined, + otherContainerId: undefined, + }), + ).toBe(true); + }); + + it("keeps a sibling process in our own namespaces", () => { + expect( + decideForeignContainer({ + platform: "linux", + namespacePairs: [ + ["pid:[4026531836]", "pid:[4026531836]"], + ["mnt:[4026531840]", "mnt:[4026531840]"], + ], + ownContainerId: undefined, + otherContainerId: undefined, + }), + ).toBe(false); + }); + + it("falls back to cgroup container ids when namespaces are unreadable", () => { + expect( + decideForeignContainer({ + platform: "linux", + namespacePairs: [[undefined, undefined]], + ownContainerId: undefined, + otherContainerId: "7c6ffadc42f0bc0bc7c6ca47de4cd702", + }), + ).toBe(true); + // Same container: our own sibling process, not something to retire. + expect( + decideForeignContainer({ + platform: "linux", + namespacePairs: [[undefined, undefined]], + ownContainerId: "7c6ffadc42f0bc0bc7c6ca47de4cd702", + otherContainerId: "7c6ffadc42f0bc0bc7c6ca47de4cd702", + }), + ).toBe(false); + }); + + it("never filters off Linux, where containers cannot share our pid space", () => { + expect( + decideForeignContainer({ + platform: "darwin", + namespacePairs: [["pid:[1]", "pid:[2]"]], + ownContainerId: undefined, + otherContainerId: "abcdef123456", + }), + ).toBe(false); + }); + + it("extracts container ids from real cgroup paths", () => { + const docker = + "0::/system.slice/docker-7c6ffadc42f0bc0bc7c6ca47de4cd702206e79b4068d172d8c2a2350063913ad.scope"; + expect(docker.match(CONTAINER_CGROUP_PATTERN)?.[1]).toBe( + "7c6ffadc42f0bc0bc7c6ca47de4cd702206e79b4068d172d8c2a2350063913ad", + ); + // A plain host session must not look like a container. + expect( + "0::/user.slice/user-1001.slice/session-121.scope".match( + CONTAINER_CGROUP_PATTERN, + ), + ).toBeNull(); + }); +}); + +describe("doctor supervision reporting", () => { + const { formatSupervisedConnector } = __test__; + + afterEach(() => { + vi.clearAllMocks(); + mockListSupervisedConnectors.mockResolvedValue(undefined); + }); + + async function runDoctorJson(): Promise> { + const output: string[] = []; + await runDoctorCommand( + { cwd: "/workspace", json: true }, + { + writeln: (text) => { + output.push(text ?? ""); + }, + writeErr: () => {}, + }, + ); + return JSON.parse(output[0] || "{}") as Record; + } + + it("reports what the hub is supervising", async () => { + mockListSupervisedConnectors.mockResolvedValue([ + { + channel: "slack", + instanceId: "cline-slack", + state: "backoff", + origin: "spawned", + restarts: 3, + }, + ]); + + await expect(runDoctorJson()).resolves.toMatchObject({ + supervisedConnectors: [ + { channel: "slack", instanceId: "cline-slack", state: "backoff" }, + ], + }); + }); + + it("omits supervision when the hub cannot report it", async () => { + mockListSupervisedConnectors.mockResolvedValue(undefined); + + const status = await runDoctorJson(); + + expect(status.supervisedConnectors).toBeUndefined(); + }); + + it("stays usable when the supervision query fails", async () => { + mockListSupervisedConnectors.mockRejectedValue(new Error("hub gone")); + + // Diagnostics must degrade quietly rather than fail. + const status = await runDoctorJson(); + + expect(status.supervisedConnectors).toBeUndefined(); + expect(status).toHaveProperty("hubHealthy"); + }); + + it("formats restart and failure state so a crash loop is visible", () => { + expect( + formatSupervisedConnector({ + channel: "slack", + instanceId: "cline-slack", + state: "failed", + origin: "adopted", + pid: 42, + restarts: 5, + lastExitCode: 1, + lastError: "invalid token", + }), + ).toBe( + "slack | instance=cline-slack | state=failed | origin=adopted | pid=42 | restarts=5 | lastExit=1 | error=invalid token", + ); + }); + + it("leaves out fields that do not apply to a healthy connector", () => { + expect( + formatSupervisedConnector({ + channel: "telegram", + instanceId: "cline_bot", + state: "running", + origin: "spawned", + pid: 7, + restarts: 0, + }), + ).toBe( + "telegram | instance=cline_bot | state=running | origin=spawned | pid=7", + ); + }); +}); diff --git a/apps/cli/src/commands/doctor.ts b/apps/cli/src/commands/doctor.ts index 7e17c61f0f..b0264c1243 100644 --- a/apps/cli/src/commands/doctor.ts +++ b/apps/cli/src/commands/doctor.ts @@ -1,5 +1,5 @@ import { spawnSync } from "node:child_process"; -import { existsSync, readFileSync, rmSync } from "node:fs"; +import { existsSync, readFileSync, readlinkSync, rmSync } from "node:fs"; import { dirname, join } from "node:path"; import { clearHubDiscovery, @@ -16,6 +16,7 @@ import { type ActiveConnectorRecord, formatUptime, resolveClineBuildEnv, + type SupervisedConnectorRecord, } from "@cline/shared"; import { Command } from "commander"; import { version as cliVersion } from "../../package.json"; @@ -24,6 +25,7 @@ import { getCliBuildInfo } from "../utils/common"; import open from "../utils/open"; import { c, writeln } from "../utils/output"; import { stopAllConnectors } from "./connect"; +import { listSupervisedConnectorsViaHub } from "./connect-via-hub"; type DoctorIo = { writeln: (text?: string) => void; @@ -64,6 +66,8 @@ type DoctorStatus = { staleCliPids: number[]; staleSidecarPids: number[]; activeConnectors: ActiveConnectorRecord[]; + /** Undefined when the running hub cannot report supervision. */ + supervisedConnectors?: SupervisedConnectorRecord[]; recentSpawnedProcesses: SpawnedProcessRecord[]; }; @@ -72,6 +76,13 @@ type ProcessRecord = { command: string; }; +// Container id inside a cgroup path, e.g. +// "0::/system.slice/docker-<64-hex>.scope" (docker/containerd/podman) or +// "/kubepods/.../<64-hex>" (kubernetes). Captures the id so two different +// containers can be told apart, not merely "is containerised". +const CONTAINER_CGROUP_PATTERN = + /(?:docker[-/]|containerd[-/]|libpod[-/]|crio[-/]|lxc[-/.])([0-9a-f]{12,64})/; + function parsePids(raw: string): number[] { return raw .split(/\r?\n/) @@ -79,6 +90,77 @@ function parsePids(raw: string): number[] { .filter((pid) => Number.isInteger(pid) && pid > 0); } +function tryReadLink(target: string): string | undefined { + try { + return readlinkSync(target); + } catch { + return undefined; + } +} + +function readContainerCgroupId(pid: number | "self"): string | undefined { + let raw: string; + try { + raw = readFileSync(`/proc/${pid}/cgroup`, "utf8"); + } catch { + return undefined; + } + return raw.match(CONTAINER_CGROUP_PATTERN)?.[1]; +} + +/** + * Decide whether a process belongs to a container other than our own. + * + * Namespace identity is the reliable signal: a containerised process has + * different PID/mount namespaces than the host process running the scan. + * Container ids parsed from cgroup paths are the fallback for kernels where the + * namespace links are unreadable. Unknown on both sides means "assume ours", + * preserving the previous behaviour rather than silently dropping processes the + * user does want cleaned up. + */ +function decideForeignContainer(input: { + platform: string; + namespacePairs: Array<[string | undefined, string | undefined]>; + ownContainerId: string | undefined; + otherContainerId: string | undefined; +}): boolean { + // /proc//ns exists only on Linux. Elsewhere containers run inside a VM + // and never share a pid space with us, so there is nothing to disambiguate. + if (input.platform !== "linux") { + return false; + } + for (const [own, other] of input.namespacePairs) { + if (own && other && own !== other) { + return true; + } + } + return ( + Boolean(input.otherContainerId) && + input.otherContainerId !== input.ownContainerId + ); +} + +/** + * True when `pid` belongs to a container other than this process's own. + * + * `pgrep` sees every process on the host, containers included: a Docker agent's + * hub daemon shows up beside ours, and when the container shares our uid `kill` + * on it succeeds. Those daemons are emphatically not stale — they belong to a + * live agent with its own data dir — so reporting them, and killing them in + * `doctor fix`, takes down an unrelated agent. + */ +function isForeignContainerPid(pid: number): boolean { + return decideForeignContainer({ + platform: process.platform, + namespacePairs: (["pid", "mnt"] as const).map((namespace) => [ + tryReadLink(`/proc/self/ns/${namespace}`), + tryReadLink(`/proc/${pid}/ns/${namespace}`), + ]), + ownContainerId: readContainerCgroupId("self"), + otherContainerId: readContainerCgroupId(pid), + }); +} + function listMatchingProcesses(pattern: string): ProcessRecord[] { if (process.platform === "win32") { return []; @@ -108,7 +190,8 @@ function listMatchingProcesses(pattern: string): ProcessRecord[] { pid <= 0 || !command || pid === process.pid || - pid === process.ppid + pid === process.ppid || + isForeignContainerPid(pid) ) { continue; } @@ -354,6 +437,7 @@ async function collectDoctorStatus(cwd: string): Promise { staleCliPids: listStaleCliPids(), staleSidecarPids: listStaleSidecarPids(), activeConnectors: listActiveConnectors(), + ...((await listSupervisedConnectorsSafely()) ?? {}), recentSpawnedProcesses: readRecentSpawnedProcesses(), }; } @@ -378,6 +462,39 @@ function formatRecentSpawnedProcess(record: SpawnedProcessRecord): string { return pieces.join(" | "); } +/** + * Supervision is reported by the running hub, so it is unavailable whenever + * there is no hub or it predates supervision. Diagnostics must degrade quietly + * rather than fail. + */ +async function listSupervisedConnectorsSafely(): Promise< + { supervisedConnectors: SupervisedConnectorRecord[] } | undefined +> { + try { + const supervised = await listSupervisedConnectorsViaHub(); + return supervised ? { supervisedConnectors: supervised } : undefined; + } catch { + return undefined; + } +} + +function formatSupervisedConnector(record: SupervisedConnectorRecord): string { + const pieces = [ + record.channel, + `instance=${record.instanceId}`, + `state=${record.state}`, + `origin=${record.origin}`, + record.pid === undefined ? undefined : `pid=${record.pid}`, + record.restarts > 0 ? `restarts=${record.restarts}` : undefined, + record.nextRestartAt ? `nextRestart=${record.nextRestartAt}` : undefined, + record.lastExitCode === undefined + ? undefined + : `lastExit=${record.lastExitCode}`, + record.lastError ? `error=${record.lastError}` : undefined, + ]; + return pieces.filter(Boolean).join(" | "); +} + function formatActiveConnector(record: ActiveConnectorRecord): string { const identity = record.type === "telegram" @@ -411,6 +528,12 @@ function killPids(pids: number[]): number { return killed; } +export const __test__ = { + decideForeignContainer, + CONTAINER_CGROUP_PATTERN, + formatSupervisedConnector, +}; + export async function runDoctorCommand( opts: { cwd: string; json?: boolean; fix?: boolean; verbose?: boolean }, io: DoctorIo, @@ -450,6 +573,12 @@ export async function runDoctorCommand( writeln(`- ${c.dim}${formatActiveConnector(record)}${c.reset}`); } } + if (before.supervisedConnectors?.length) { + writeln("hub-supervised connectors:"); + for (const record of before.supervisedConnectors) { + writeln(`- ${c.dim}${formatSupervisedConnector(record)}${c.reset}`); + } + } if (verbose && before.recentSpawnedProcesses.length > 0) { writeln("recent spawned processes:"); for (const record of before.recentSpawnedProcesses) { diff --git a/apps/cli/src/connectors/adapters/discord.ts b/apps/cli/src/connectors/adapters/discord.ts index 8eb5bd47bb..a106f239eb 100644 --- a/apps/cli/src/connectors/adapters/discord.ts +++ b/apps/cli/src/connectors/adapters/discord.ts @@ -56,6 +56,7 @@ import { loadThreadState, persistMergedThreadState, readBindings, + resolveThreadTurnQueueKey, } from "../thread-bindings"; import type { ConnectCommandDefinition, @@ -730,60 +731,69 @@ class DiscordConnector extends ConnectorBase< } protected override createCommand(): Command { - return super - .createCommand() - .usage("--base-url [options]") - .option("--user-name ", "Discord bot username label") - .option("--application-id ", "Discord application id") - .option("--app-id ", "Alias for --application-id") - .option("--bot-token ", "Discord bot token") - .option("--token ", "Alias for --bot-token") - .option("--public-key ", "Discord application public key") - .option( - "--owner-user-id ", - "Discord user id that should be marked as connector owner", - ) - .option("--ignore-bot-authors", "Ignore messages from other Discord bots") - .option( - "--mention-role-ids ", - "Comma-separated role IDs that should trigger mention handlers", - ) - .option("--provider ", "Provider override") - .option("--model ", "Model override") - .option("--api-key ", "Provider API key override") - .option("--system ", "System prompt override") - .option("--cwd ", "Workspace / cwd for runtime") - .option("--mode ", "Agent mode", "act") - .option("-i, --interactive", "Keep connector in foreground") - .option("--enable-tools", "Enable tools for Discord sessions") - .option( - "--hook-command ", - "Run a shell command for connector events", - ) - .option( - "--rpc-address ", - "RPC address", - process.env.CLINE_RPC_ADDRESS?.trim() || resolveDefaultCliRpcAddress(), - ) - .option("--host ", "Webhook listen host") - .option("--port ", "Webhook listen port") - .option( - "--base-url ", - "Public base URL for Discord interactions webhook", - ) - .addHelpText( - "after", - [ - "", - "Environment:", - " DISCORD_APPLICATION_ID Discord application id", - " DISCORD_BOT_TOKEN Discord bot token", - " DISCORD_PUBLIC_KEY Discord application public key", - " DISCORD_OWNER_USER_ID Optional connector owner user id", - " DISCORD_IGNORE_BOT_AUTHORS Set to 1 to ignore messages from other bots", - " DISCORD_MENTION_ROLE_IDS Optional comma-separated role ids", - ].join("\n"), - ); + return ( + super + .createCommand() + .usage("--base-url [options]") + .option("--user-name ", "Discord bot username label") + .option("--application-id ", "Discord application id") + .option("--app-id ", "Alias for --application-id") + .option("--bot-token ", "Discord bot token") + .option("--token ", "Alias for --bot-token") + .option("--public-key ", "Discord application public key") + .option( + "--owner-user-id ", + "Discord user id that should be marked as connector owner", + ) + .option( + "--ignore-bot-authors", + "Ignore messages from other Discord bots", + ) + .option( + "--mention-role-ids ", + "Comma-separated role IDs that should trigger mention handlers", + ) + .option("--provider ", "Provider override") + .option("--model ", "Model override") + .option("--api-key ", "Provider API key override") + .option("--system ", "System prompt override") + .option("--cwd ", "Workspace / cwd for runtime") + .option("--mode ", "Agent mode", "act") + .option("-i, --interactive", "Keep connector in foreground") + .option("--no-tools", "Disable tools for Discord sessions") + // Retained so existing invocations and persisted autostart arguments + // keep parsing; tools are on unless --no-tools is passed. + .option("--enable-tools", "Enable tools (default)") + .option( + "--hook-command ", + "Run a shell command for connector events", + ) + .option( + "--rpc-address ", + "RPC address", + process.env.CLINE_RPC_ADDRESS?.trim() || + resolveDefaultCliRpcAddress(), + ) + .option("--host ", "Webhook listen host") + .option("--port ", "Webhook listen port") + .option( + "--base-url ", + "Public base URL for Discord interactions webhook", + ) + .addHelpText( + "after", + [ + "", + "Environment:", + " DISCORD_APPLICATION_ID Discord application id", + " DISCORD_BOT_TOKEN Discord bot token", + " DISCORD_PUBLIC_KEY Discord application public key", + " DISCORD_OWNER_USER_ID Optional connector owner user id", + " DISCORD_IGNORE_BOT_AUTHORS Set to 1 to ignore messages from other bots", + " DISCORD_MENTION_ROLE_IDS Optional comma-separated role ids", + ].join("\n"), + ) + ); } protected override readOptions(command: Command): ConnectDiscordOptions { @@ -805,6 +815,7 @@ class DiscordConnector extends ConnectorBase< mode?: string; interactive?: boolean; enableTools?: boolean; + tools?: boolean; rpcAddress?: string; hookCommand?: string; port?: string; @@ -857,7 +868,7 @@ class DiscordConnector extends ConnectorBase< systemPrompt: opts.system, mode: this.parseMode(opts.mode), interactive: Boolean(opts.interactive), - enableTools: Boolean(opts.enableTools), + enableTools: opts.tools !== false, rpcAddress: opts.rpcAddress?.trim() || process.env.CLINE_RPC_ADDRESS?.trim() || @@ -974,6 +985,12 @@ class DiscordConnector extends ConnectorBase< return 0; } + protected override instanceIdFromOptions( + options: ConnectDiscordOptions, + ): string | undefined { + return options.applicationId; + } + protected override async runWithOptions( options: ConnectDiscordOptions, rawArgs: string[], @@ -1120,7 +1137,7 @@ class DiscordConnector extends ConnectorBase< isSubscribedThreadMessage?: boolean; }, ) => { - const queueKey = thread.id; + const queueKey = resolveThreadTurnQueueKey(thread); const enqueueTurn = (work: () => Promise) => enqueueThreadTurn(threadQueues, queueKey, work); const runTurn = async () => { diff --git a/apps/cli/src/connectors/adapters/gchat.ts b/apps/cli/src/connectors/adapters/gchat.ts index e8b03de48d..aff82b57f2 100644 --- a/apps/cli/src/connectors/adapters/gchat.ts +++ b/apps/cli/src/connectors/adapters/gchat.ts @@ -51,6 +51,7 @@ import { loadThreadState, persistMergedThreadState, readBindings, + resolveThreadTurnQueueKey, } from "../thread-bindings"; import type { ConnectCommandDefinition, @@ -235,48 +236,54 @@ class GoogleChatConnector extends ConnectorBase< } protected override createCommand(): Command { - return super - .createCommand() - .usage("--base-url [options]") - .option("--user-name ", "Google Chat bot username label") - .option("--provider ", "Provider override") - .option("--model ", "Model override") - .option("--api-key ", "Provider API key override") - .option("--system ", "System prompt override") - .option("--cwd ", "Workspace / cwd for runtime") - .option("--mode ", "Agent mode", "act") - .option("-i, --interactive", "Keep connector in foreground") - .option("--enable-tools", "Enable tools for Google Chat sessions") - .option( - "--hook-command ", - "Run a shell command for connector events", - ) - .option( - "--rpc-address ", - "RPC address", - process.env.CLINE_RPC_ADDRESS?.trim() || resolveDefaultCliRpcAddress(), - ) - .option("--host ", "Webhook listen host") - .option("--port ", "Webhook listen port") - .option("--base-url ", "Public base URL for webhook configuration") - .option( - "--pubsub-topic ", - "Optional Pub/Sub topic for all-message events", - ) - .option("--impersonate-user ", "Optional delegation user email") - .option("--use-adc", "Use Google Application Default Credentials") - .option("--credentials-json ", "Service account credentials JSON") - .addHelpText( - "after", - [ - "", - "Environment:", - " GOOGLE_CHAT_CREDENTIALS Service account JSON", - " GOOGLE_CHAT_USE_ADC=true Use Application Default Credentials", - " GOOGLE_CHAT_PUBSUB_TOPIC Optional Pub/Sub topic", - " GOOGLE_CHAT_IMPERSONATE_USER Optional delegation user", - ].join("\n"), - ); + return ( + super + .createCommand() + .usage("--base-url [options]") + .option("--user-name ", "Google Chat bot username label") + .option("--provider ", "Provider override") + .option("--model ", "Model override") + .option("--api-key ", "Provider API key override") + .option("--system ", "System prompt override") + .option("--cwd ", "Workspace / cwd for runtime") + .option("--mode ", "Agent mode", "act") + .option("-i, --interactive", "Keep connector in foreground") + .option("--no-tools", "Disable tools for Google Chat sessions") + // Retained so existing invocations and persisted autostart arguments + // keep parsing; tools are on unless --no-tools is passed. + .option("--enable-tools", "Enable tools (default)") + .option( + "--hook-command ", + "Run a shell command for connector events", + ) + .option( + "--rpc-address ", + "RPC address", + process.env.CLINE_RPC_ADDRESS?.trim() || + resolveDefaultCliRpcAddress(), + ) + .option("--host ", "Webhook listen host") + .option("--port ", "Webhook listen port") + .option("--base-url ", "Public base URL for webhook configuration") + .option( + "--pubsub-topic ", + "Optional Pub/Sub topic for all-message events", + ) + .option("--impersonate-user ", "Optional delegation user email") + .option("--use-adc", "Use Google Application Default Credentials") + .option("--credentials-json ", "Service account credentials JSON") + .addHelpText( + "after", + [ + "", + "Environment:", + " GOOGLE_CHAT_CREDENTIALS Service account JSON", + " GOOGLE_CHAT_USE_ADC=true Use Application Default Credentials", + " GOOGLE_CHAT_PUBSUB_TOPIC Optional Pub/Sub topic", + " GOOGLE_CHAT_IMPERSONATE_USER Optional delegation user", + ].join("\n"), + ) + ); } protected override readOptions(command: Command): ConnectGoogleChatOptions { @@ -290,6 +297,7 @@ class GoogleChatConnector extends ConnectorBase< mode?: string; interactive?: boolean; enableTools?: boolean; + tools?: boolean; rpcAddress?: string; hookCommand?: string; port?: string; @@ -316,7 +324,7 @@ class GoogleChatConnector extends ConnectorBase< systemPrompt: opts.system, mode: this.parseMode(opts.mode), interactive: Boolean(opts.interactive), - enableTools: Boolean(opts.enableTools), + enableTools: opts.tools !== false, rpcAddress: opts.rpcAddress?.trim() || process.env.CLINE_RPC_ADDRESS?.trim() || @@ -463,6 +471,12 @@ class GoogleChatConnector extends ConnectorBase< } } + protected override instanceIdFromOptions( + options: ConnectGoogleChatOptions, + ): string | undefined { + return options.userName; + } + protected override async runWithOptions( options: ConnectGoogleChatOptions, rawArgs: string[], @@ -614,7 +628,7 @@ class GoogleChatConnector extends ConnectorBase< thread: Thread, text: string, ) => { - const queueKey = thread.id; + const queueKey = resolveThreadTurnQueueKey(thread); const enqueueTurn = (work: () => Promise) => enqueueThreadTurn(threadQueues, queueKey, work); const runTurn = async () => { diff --git a/apps/cli/src/connectors/adapters/linear.ts b/apps/cli/src/connectors/adapters/linear.ts index 253b9ee886..ff38da9e99 100644 --- a/apps/cli/src/connectors/adapters/linear.ts +++ b/apps/cli/src/connectors/adapters/linear.ts @@ -47,6 +47,7 @@ import { loadThreadState, persistMergedThreadState, readBindings, + resolveThreadTurnQueueKey, } from "../thread-bindings"; import type { ConnectCommandDefinition, @@ -290,48 +291,54 @@ class LinearConnector extends ConnectorBase< } protected override createCommand(): Command { - return super - .createCommand() - .usage("--base-url [options]") - .option("--user-name ", "Linear bot display name") - .option("--api-key ", "Linear personal API key") - .option("--client-id ", "Linear OAuth client id") - .option("--client-secret ", "Linear OAuth client secret") - .option("--access-token ", "Pre-obtained Linear access token") - .option("--webhook-secret ", "Linear webhook signing secret") - .option("--provider ", "Provider override") - .option("--model ", "Model override") - .option("--provider-api-key ", "Provider API key override") - .option("--system ", "System prompt override") - .option("--cwd ", "Workspace / cwd for runtime") - .option("--mode ", "Agent mode", "act") - .option("-i, --interactive", "Keep connector in foreground") - .option("--enable-tools", "Enable tools for Linear sessions") - .option( - "--hook-command ", - "Run a shell command for connector events", - ) - .option( - "--rpc-address ", - "RPC address", - process.env.CLINE_RPC_ADDRESS?.trim() || resolveDefaultCliRpcAddress(), - ) - .option("--host ", "Webhook listen host") - .option("--port ", "Webhook listen port") - .option("--base-url ", "Public base URL for webhook configuration") - .addHelpText( - "after", - [ - "", - "Environment:", - " LINEAR_API_KEY Personal API key", - " LINEAR_CLIENT_ID OAuth client id", - " LINEAR_CLIENT_SECRET OAuth client secret", - " LINEAR_ACCESS_TOKEN Pre-obtained access token", - " LINEAR_WEBHOOK_SECRET Webhook signing secret", - " LINEAR_BOT_USERNAME Bot display name (default: linear-bot)", - ].join("\n"), - ); + return ( + super + .createCommand() + .usage("--base-url [options]") + .option("--user-name ", "Linear bot display name") + .option("--api-key ", "Linear personal API key") + .option("--client-id ", "Linear OAuth client id") + .option("--client-secret ", "Linear OAuth client secret") + .option("--access-token ", "Pre-obtained Linear access token") + .option("--webhook-secret ", "Linear webhook signing secret") + .option("--provider ", "Provider override") + .option("--model ", "Model override") + .option("--provider-api-key ", "Provider API key override") + .option("--system ", "System prompt override") + .option("--cwd ", "Workspace / cwd for runtime") + .option("--mode ", "Agent mode", "act") + .option("-i, --interactive", "Keep connector in foreground") + .option("--no-tools", "Disable tools for Linear sessions") + // Retained so existing invocations and persisted autostart arguments + // keep parsing; tools are on unless --no-tools is passed. + .option("--enable-tools", "Enable tools (default)") + .option( + "--hook-command ", + "Run a shell command for connector events", + ) + .option( + "--rpc-address ", + "RPC address", + process.env.CLINE_RPC_ADDRESS?.trim() || + resolveDefaultCliRpcAddress(), + ) + .option("--host ", "Webhook listen host") + .option("--port ", "Webhook listen port") + .option("--base-url ", "Public base URL for webhook configuration") + .addHelpText( + "after", + [ + "", + "Environment:", + " LINEAR_API_KEY Personal API key", + " LINEAR_CLIENT_ID OAuth client id", + " LINEAR_CLIENT_SECRET OAuth client secret", + " LINEAR_ACCESS_TOKEN Pre-obtained access token", + " LINEAR_WEBHOOK_SECRET Webhook signing secret", + " LINEAR_BOT_USERNAME Bot display name (default: linear-bot)", + ].join("\n"), + ) + ); } protected override readOptions(command: Command): ConnectLinearOptions { @@ -350,6 +357,7 @@ class LinearConnector extends ConnectorBase< mode?: string; interactive?: boolean; enableTools?: boolean; + tools?: boolean; rpcAddress?: string; hookCommand?: string; port?: string; @@ -396,7 +404,7 @@ class LinearConnector extends ConnectorBase< systemPrompt: opts.system, mode: this.parseMode(opts.mode), interactive: Boolean(opts.interactive), - enableTools: Boolean(opts.enableTools), + enableTools: opts.tools !== false, rpcAddress: opts.rpcAddress?.trim() || process.env.CLINE_RPC_ADDRESS?.trim() || @@ -488,6 +496,12 @@ class LinearConnector extends ConnectorBase< ); } + protected override instanceIdFromOptions( + options: ConnectLinearOptions, + ): string | undefined { + return options.userName; + } + protected override async runWithOptions( options: ConnectLinearOptions, rawArgs: string[], @@ -637,7 +651,7 @@ class LinearConnector extends ConnectorBase< thread: Thread, text: string, ) => { - const queueKey = thread.id; + const queueKey = resolveThreadTurnQueueKey(thread); const enqueueTurn = (work: () => Promise) => enqueueThreadTurn(threadQueues, queueKey, work); const runTurn = async () => { diff --git a/apps/cli/src/connectors/adapters/slack.ts b/apps/cli/src/connectors/adapters/slack.ts index 60c435d29f..4ebe330717 100644 --- a/apps/cli/src/connectors/adapters/slack.ts +++ b/apps/cli/src/connectors/adapters/slack.ts @@ -56,6 +56,7 @@ import { loadThreadState, persistMergedThreadState, readBindings, + resolveThreadTurnQueueKey, writeBindings, } from "../thread-bindings"; import type { @@ -64,19 +65,13 @@ import type { ConnectRunContext, ConnectStopResult, } from "../types"; -import { - getConnectorFirstContactMessage, - getConnectorSystemPrompt, - getConnectorSystemRules, -} from "./prompts"; +import { getConnectorSystemPrompt, getConnectorSystemRules } from "./prompts"; const SLACK_SYSTEM_RULES = getConnectorSystemRules( "Slack", "You can respond to user messages in threads and DMs, and you can use tools according to user's requests and your capabilities.", ); -const SLACK_FIRST_CONTACT_MESSAGE = getConnectorFirstContactMessage(); - type SlackThreadState = ConnectorThreadState & { teamId?: string; }; @@ -501,62 +496,68 @@ class SlackConnector extends ConnectorBase< } protected override createCommand(): Command { - return super - .createCommand() - .usage("--base-url [options]") - .option("--user-name ", "Slack bot username label") - .option( - "--bot-token ", - "Slack bot token for single-workspace mode", - ) - .option("--signing-secret ", "Slack signing secret") - .option("--app-token ", "Slack app-level token for socket mode") - .option("--client-id ", "Slack OAuth client id") - .option("--client-secret ", "Slack OAuth client secret") - .option( - "--encryption-key ", - "Base64 32-byte key for encrypted installations", - ) - .option( - "--installation-key-prefix ", - "Override stored installation key prefix", - ) - .option("--provider ", "Provider override") - .option("--model ", "Model override") - .option("--api-key ", "Provider API key override") - .option("--system ", "System prompt override") - .option("--cwd ", "Workspace / cwd for runtime") - .option("--mode ", "Agent mode", "act") - .option("-i, --interactive", "Keep connector in foreground") - .option("--enable-tools", "Enable tools for Slack sessions") - .option( - "--hook-command ", - "Run a shell command for connector events", - ) - .option( - "--rpc-address ", - "RPC address", - process.env.CLINE_RPC_ADDRESS?.trim() || resolveDefaultCliRpcAddress(), - ) - .option("--host ", "Webhook listen host") - .option("--port ", "Webhook listen port") - .option( - "--base-url ", - "Public base URL for webhooks and OAuth callback", - ) - .addHelpText( - "after", - [ - "", - "Environment:", - " SLACK_BOT_TOKEN Single-workspace bot token", - " SLACK_SIGNING_SECRET Slack signing secret", - " SLACK_APP_TOKEN App-level token for socket mode", - " SLACK_CLIENT_ID OAuth client id", - " SLACK_CLIENT_SECRET OAuth client secret", - " SLACK_ENCRYPTION_KEY Optional installation encryption key", - ].join("\n"), - ); + return ( + super + .createCommand() + .usage("--base-url [options]") + .option("--user-name ", "Slack bot username label") + .option( + "--bot-token ", + "Slack bot token for single-workspace mode", + ) + .option("--signing-secret ", "Slack signing secret") + .option("--app-token ", "Slack app-level token for socket mode") + .option("--client-id ", "Slack OAuth client id") + .option("--client-secret ", "Slack OAuth client secret") + .option( + "--encryption-key ", + "Base64 32-byte key for encrypted installations", + ) + .option( + "--installation-key-prefix ", + "Override stored installation key prefix", + ) + .option("--provider ", "Provider override") + .option("--model ", "Model override") + .option("--api-key ", "Provider API key override") + .option("--system ", "System prompt override") + .option("--cwd ", "Workspace / cwd for runtime") + .option("--mode ", "Agent mode", "act") + .option("-i, --interactive", "Keep connector in foreground") + .option("--no-tools", "Disable tools for Slack sessions") + // Retained so existing invocations and persisted autostart arguments + // keep parsing; tools are on unless --no-tools is passed. + .option("--enable-tools", "Enable tools (default)") + .option( + "--hook-command ", + "Run a shell command for connector events", + ) + .option( + "--rpc-address ", + "RPC address", + process.env.CLINE_RPC_ADDRESS?.trim() || + resolveDefaultCliRpcAddress(), + ) + .option("--host ", "Webhook listen host") + .option("--port ", "Webhook listen port") + .option( + "--base-url ", + "Public base URL for webhooks and OAuth callback", + ) + .addHelpText( + "after", + [ + "", + "Environment:", + " SLACK_BOT_TOKEN Single-workspace bot token", + " SLACK_SIGNING_SECRET Slack signing secret", + " SLACK_APP_TOKEN App-level token for socket mode", + " SLACK_CLIENT_ID OAuth client id", + " SLACK_CLIENT_SECRET OAuth client secret", + " SLACK_ENCRYPTION_KEY Optional installation encryption key", + ].join("\n"), + ) + ); } protected override readOptions(command: Command): ConnectSlackOptions { @@ -577,6 +578,7 @@ class SlackConnector extends ConnectorBase< mode?: string; interactive?: boolean; enableTools?: boolean; + tools?: boolean; rpcAddress?: string; hookCommand?: string; port?: string; @@ -643,7 +645,7 @@ class SlackConnector extends ConnectorBase< systemPrompt: opts.system, mode: this.parseMode(opts.mode), interactive: Boolean(opts.interactive), - enableTools: Boolean(opts.enableTools), + enableTools: opts.tools !== false, rpcAddress: opts.rpcAddress?.trim() || process.env.CLINE_RPC_ADDRESS?.trim() || @@ -733,6 +735,12 @@ class SlackConnector extends ConnectorBase< ); } + protected override instanceIdFromOptions( + options: ConnectSlackOptions, + ): string | undefined { + return options.userName; + } + protected override async runWithOptions( options: ConnectSlackOptions, rawArgs: string[], @@ -892,7 +900,7 @@ class SlackConnector extends ConnectorBase< bindingsPath, startRequest, ); - const queueKey = thread.id; + const queueKey = resolveThreadTurnQueueKey(thread); const enqueueTurn = (work: () => Promise) => enqueueThreadTurn(threadQueues, queueKey, work); const runTurn = async () => { @@ -919,7 +927,6 @@ class SlackConnector extends ConnectorBase< hookCommand: options.hookCommand, systemRules: SLACK_SYSTEM_RULES, errorLabel: "Slack", - firstContactMessage: SLACK_FIRST_CONTACT_MESSAGE, userInstructionService, chatCommandHost, activeTurns, diff --git a/apps/cli/src/connectors/adapters/telegram.ts b/apps/cli/src/connectors/adapters/telegram.ts index 5bf91987da..0ecfb80820 100644 --- a/apps/cli/src/connectors/adapters/telegram.ts +++ b/apps/cli/src/connectors/adapters/telegram.ts @@ -47,6 +47,7 @@ import { loadThreadState, persistMergedThreadState, readBindings, + resolveThreadTurnQueueKey, } from "../thread-bindings"; import type { ConnectCommandDefinition, @@ -466,47 +467,53 @@ class TelegramConnector extends ConnectorBase< } protected override createCommand(): Command { - return super - .createCommand() - .usage("-k [options]") - .option( - "-m, --bot-username ", - "Telegram bot username; fetched from token if omitted", - ) - .option("-k, --bot-token ", "Telegram bot token") - .option("--provider ", "Provider override") - .option("--model ", "Model override") - .option("--api-key ", "Provider API key override") - .option("--system ", "System prompt override") - .option("--cwd ", "Workspace / cwd for runtime") - .option("--mode ", "Agent mode", "act") - .option("-i, --interactive", "Keep connector in foreground") - .option("--no-tools", "Disable tools for Telegram sessions") - .option( - "--allowed-user-id ", - "Only allow this Telegram user ID to use the bot", - ) - .option( - "--hook-command ", - "Run a shell command for connector events", - ) - .option( - "--rpc-address ", - "RPC address", - process.env.CLINE_RPC_ADDRESS?.trim() || resolveDefaultCliRpcAddress(), - ) - .addHelpText( - "after", - [ - "", - "Notes:", - " - Without -i, the connector is launched in the background.", - " - Tools are enabled by default for Telegram sessions.", - " - Use --allowed-user-id or `cline connect` to restrict Telegram access.", - " - Bot username is discovered from the Telegram bot token when omitted.", - " - Provider/model default to the CLI's last-used provider settings.", - ].join("\n"), - ); + return ( + super + .createCommand() + .usage("-k [options]") + .option( + "-m, --bot-username ", + "Telegram bot username; fetched from token if omitted", + ) + .option("-k, --bot-token ", "Telegram bot token") + .option("--provider ", "Provider override") + .option("--model ", "Model override") + .option("--api-key ", "Provider API key override") + .option("--system ", "System prompt override") + .option("--cwd ", "Workspace / cwd for runtime") + .option("--mode ", "Agent mode", "act") + .option("-i, --interactive", "Keep connector in foreground") + .option("--no-tools", "Disable tools for Telegram sessions") + // Retained so existing invocations and persisted autostart arguments + // keep parsing; tools are on unless --no-tools is passed. + .option("--enable-tools", "Enable tools (default)") + .option( + "--allowed-user-id ", + "Only allow this Telegram user ID to use the bot", + ) + .option( + "--hook-command ", + "Run a shell command for connector events", + ) + .option( + "--rpc-address ", + "RPC address", + process.env.CLINE_RPC_ADDRESS?.trim() || + resolveDefaultCliRpcAddress(), + ) + .addHelpText( + "after", + [ + "", + "Notes:", + " - Without -i, the connector is launched in the background.", + " - Tools are enabled by default for Telegram sessions.", + " - Use --allowed-user-id or `cline connect` to restrict Telegram access.", + " - Bot username is discovered from the Telegram bot token when omitted.", + " - Provider/model default to the CLI's last-used provider settings.", + ].join("\n"), + ) + ); } protected override readOptions(command: Command): ConnectTelegramOptions { @@ -680,6 +687,17 @@ class TelegramConnector extends ConnectorBase< } } + /** + * Only knowable up front when `--bot-username` was supplied; otherwise the + * username is resolved from Telegram's API during startup, and the caller + * has to start this connector locally instead of through the hub. + */ + protected override instanceIdFromOptions( + options: ConnectTelegramOptions, + ): string | undefined { + return options.botUsername; + } + protected override async runWithOptions( inputOptions: ConnectTelegramOptions, rawArgs: string[], @@ -865,7 +883,7 @@ class TelegramConnector extends ConnectorBase< thread: Thread, text: string, ) => { - const queueKey = thread.id; + const queueKey = resolveThreadTurnQueueKey(thread); const enqueueTurn = (work: () => Promise) => enqueueThreadTurn(threadQueues, queueKey, work); const runTurn = async () => { diff --git a/apps/cli/src/connectors/adapters/tools-default.test.ts b/apps/cli/src/connectors/adapters/tools-default.test.ts new file mode 100644 index 0000000000..8ff9a88542 --- /dev/null +++ b/apps/cli/src/connectors/adapters/tools-default.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vitest"; +import { discordConnector } from "./discord"; +import { gchatConnector } from "./gchat"; +import { linearConnector } from "./linear"; +import { slackConnector } from "./slack"; +import { telegramConnector } from "./telegram"; +import { whatsappConnector } from "./whatsapp"; + +/** + * Every connector runs with tools enabled unless the operator opts out, so this + * has to hold for all of them at once rather than per adapter — the whole point + * is that there is no adapter where the default is different. + */ +const connectors: Array<{ + name: string; + connector: unknown; + /** Minimal arguments that parse for this adapter. */ + baseArgs: string[]; +}> = [ + { + name: "slack", + connector: slackConnector, + baseArgs: ["--user-name", "bot"], + }, + { + name: "discord", + connector: discordConnector, + baseArgs: ["--application-id", "app-1", "--bot-token", "token"], + }, + { + name: "linear", + connector: linearConnector, + baseArgs: [ + "--user-name", + "bot", + "--api-key", + "key", + "--webhook-secret", + "secret", + ], + }, + { + name: "gchat", + connector: gchatConnector, + baseArgs: ["--user-name", "bot"], + }, + { + name: "whatsapp", + connector: whatsappConnector, + baseArgs: ["--user-name", "bot", "--phone-number-id", "123"], + }, + { + name: "telegram", + connector: telegramConnector, + baseArgs: ["--bot-token", "123:token"], + }, +]; + +function parse( + connector: unknown, + rawArgs: string[], +): { enableTools: boolean } { + return ( + connector as { + parseArgs(rawArgs: string[]): { enableTools: boolean }; + } + ).parseArgs(rawArgs); +} + +describe("connector tools default", () => { + for (const { name, connector, baseArgs } of connectors) { + it(`${name}: enables tools when nothing is passed`, () => { + expect(parse(connector, baseArgs).enableTools).toBe(true); + }); + + it(`${name}: disables tools with --no-tools`, () => { + expect(parse(connector, [...baseArgs, "--no-tools"]).enableTools).toBe( + false, + ); + }); + + it(`${name}: an explicit --no-tools beats --enable-tools`, () => { + // Ambiguous input resolves to the safer answer. + expect( + parse(connector, [...baseArgs, "--enable-tools", "--no-tools"]) + .enableTools, + ).toBe(false); + }); + } + + it("keeps accepting --enable-tools so existing invocations still parse", () => { + // Persisted autostart arguments and deployed scripts carry this flag; it is + // redundant now but must not become an unknown-option error. + for (const { connector, baseArgs } of connectors) { + expect( + parse(connector, [...baseArgs, "--enable-tools"]).enableTools, + ).toBe(true); + } + }); +}); diff --git a/apps/cli/src/connectors/adapters/whatsapp.ts b/apps/cli/src/connectors/adapters/whatsapp.ts index c3184dac8a..d2dc9624c3 100644 --- a/apps/cli/src/connectors/adapters/whatsapp.ts +++ b/apps/cli/src/connectors/adapters/whatsapp.ts @@ -51,6 +51,7 @@ import { loadThreadState, persistMergedThreadState, readBindings, + resolveThreadTurnQueueKey, } from "../thread-bindings"; import type { ConnectCommandDefinition, @@ -273,47 +274,53 @@ class WhatsAppConnector extends ConnectorBase< } protected override createCommand(): Command { - return super - .createCommand() - .usage("--base-url [options]") - .option("--user-name ", "WhatsApp bot username label") - .option("--phone-number-id ", "WhatsApp Business phone number id") - .option("--access-token ", "Meta access token") - .option("--app-secret ", "Meta app secret") - .option("--verify-token ", "Webhook verify token") - .option("--api-version ", "Graph API version", "v21.0") - .option("--provider ", "Provider override") - .option("--model ", "Model override") - .option("--api-key ", "Provider API key override") - .option("--system ", "System prompt override") - .option("--cwd ", "Workspace / cwd for runtime") - .option("--mode ", "Agent mode", "act") - .option("-i, --interactive", "Keep connector in foreground") - .option("--enable-tools", "Enable tools for WhatsApp sessions") - .option( - "--hook-command ", - "Run a shell command for connector events", - ) - .option( - "--rpc-address ", - "RPC address", - process.env.CLINE_RPC_ADDRESS?.trim() || resolveDefaultCliRpcAddress(), - ) - .option("--host ", "Webhook listen host") - .option("--port ", "Webhook listen port") - .option("--base-url ", "Public base URL for webhook configuration") - .addHelpText( - "after", - [ - "", - "Environment:", - " WHATSAPP_ACCESS_TOKEN Meta access token", - " WHATSAPP_APP_SECRET Meta app secret", - " WHATSAPP_PHONE_NUMBER_ID WhatsApp Business phone number id", - " WHATSAPP_VERIFY_TOKEN Webhook verification token", - " WHATSAPP_BOT_USERNAME Bot username label", - ].join("\n"), - ); + return ( + super + .createCommand() + .usage("--base-url [options]") + .option("--user-name ", "WhatsApp bot username label") + .option("--phone-number-id ", "WhatsApp Business phone number id") + .option("--access-token ", "Meta access token") + .option("--app-secret ", "Meta app secret") + .option("--verify-token ", "Webhook verify token") + .option("--api-version ", "Graph API version", "v21.0") + .option("--provider ", "Provider override") + .option("--model ", "Model override") + .option("--api-key ", "Provider API key override") + .option("--system ", "System prompt override") + .option("--cwd ", "Workspace / cwd for runtime") + .option("--mode ", "Agent mode", "act") + .option("-i, --interactive", "Keep connector in foreground") + .option("--no-tools", "Disable tools for WhatsApp sessions") + // Retained so existing invocations and persisted autostart arguments + // keep parsing; tools are on unless --no-tools is passed. + .option("--enable-tools", "Enable tools (default)") + .option( + "--hook-command ", + "Run a shell command for connector events", + ) + .option( + "--rpc-address ", + "RPC address", + process.env.CLINE_RPC_ADDRESS?.trim() || + resolveDefaultCliRpcAddress(), + ) + .option("--host ", "Webhook listen host") + .option("--port ", "Webhook listen port") + .option("--base-url ", "Public base URL for webhook configuration") + .addHelpText( + "after", + [ + "", + "Environment:", + " WHATSAPP_ACCESS_TOKEN Meta access token", + " WHATSAPP_APP_SECRET Meta app secret", + " WHATSAPP_PHONE_NUMBER_ID WhatsApp Business phone number id", + " WHATSAPP_VERIFY_TOKEN Webhook verification token", + " WHATSAPP_BOT_USERNAME Bot username label", + ].join("\n"), + ) + ); } protected override readOptions(command: Command): ConnectWhatsAppOptions { @@ -332,6 +339,7 @@ class WhatsAppConnector extends ConnectorBase< mode?: string; interactive?: boolean; enableTools?: boolean; + tools?: boolean; rpcAddress?: string; hookCommand?: string; port?: string; @@ -364,7 +372,7 @@ class WhatsAppConnector extends ConnectorBase< systemPrompt: opts.system, mode: this.parseMode(opts.mode), interactive: Boolean(opts.interactive), - enableTools: Boolean(opts.enableTools), + enableTools: opts.tools !== false, rpcAddress: opts.rpcAddress?.trim() || process.env.CLINE_RPC_ADDRESS?.trim() || @@ -455,6 +463,15 @@ class WhatsAppConnector extends ConnectorBase< ); } + protected override instanceIdFromOptions( + options: ConnectWhatsAppOptions, + ): string | undefined { + return resolveInstanceKey({ + phoneNumberId: options.phoneNumberId, + userName: options.userName, + }); + } + protected override async runWithOptions( options: ConnectWhatsAppOptions, rawArgs: string[], @@ -608,7 +625,7 @@ class WhatsAppConnector extends ConnectorBase< thread: Thread, text: string, ) => { - const queueKey = thread.id; + const queueKey = resolveThreadTurnQueueKey(thread); const enqueueTurn = (work: () => Promise) => enqueueThreadTurn(threadQueues, queueKey, work); const runTurn = async () => { diff --git a/apps/cli/src/connectors/base.test.ts b/apps/cli/src/connectors/base.test.ts index 49595ba74a..83006bf6dd 100644 --- a/apps/cli/src/connectors/base.test.ts +++ b/apps/cli/src/connectors/base.test.ts @@ -116,7 +116,29 @@ describe("ConnectorBase background launch", () => { await expect(new TestConnector().runBackground(io)).resolves.toBe(1); expect(io.writeErr).toHaveBeenCalledWith( - "launch failed: child exited before becoming ready", + expect.stringContaining( + "launch failed: child exited before becoming ready", + ), + ); + }); + + it("points at the child log so a startup failure is diagnosable", async () => { + mocks.spawnDetachedConnector.mockReturnValue(42); + mocks.isProcessRunning.mockReturnValue(false); + + await new TestConnector().runBackground(io); + + const [message] = vi.mocked(io.writeErr).mock.calls[0] ?? []; + expect(message).toContain("logs/connectors/test/test-connector.log"); + expect(mocks.spawnDetachedConnector).toHaveBeenCalledWith( + ["connect", "test"], + ["--token", "secret"], + "CLINE_TEST_CONNECT_CHILD", + expect.objectContaining({ + logPath: expect.stringContaining( + "logs/connectors/test/test-connector.log", + ), + }), ); }); @@ -129,7 +151,7 @@ describe("ConnectorBase background launch", () => { expect(mocks.terminateProcess).toHaveBeenCalledWith(42); expect(io.writeErr).toHaveBeenCalledWith( - "launch failed: timed out after 0ms", + expect.stringContaining("launch failed: timed out after 0ms"), ); }); diff --git a/apps/cli/src/connectors/base.ts b/apps/cli/src/connectors/base.ts index d40f2d0d29..024258354f 100644 --- a/apps/cli/src/connectors/base.ts +++ b/apps/cli/src/connectors/base.ts @@ -1,12 +1,21 @@ -import { existsSync, readdirSync } from "node:fs"; -import { join } from "node:path"; +import { + closeSync, + existsSync, + openSync, + readdirSync, + readSync, + statSync, +} from "node:fs"; +import { basename, join } from "node:path"; import { resolveClineDataDir } from "@cline/core"; +import { isSupervisedConnectorProcess } from "@cline/shared"; import { Command, CommanderError } from "commander"; import { CONNECT_ALREADY_RUNNING_EXIT_CODE, isProcessRunning, readJsonFile, removeFile, + resolveConnectorDebugLogPath, spawnDetachedConnector, terminateProcess, writeJsonFile, @@ -21,6 +30,65 @@ import type { const SHOW_HELP_ERROR = "__SHOW_HELP__"; const CONNECTOR_STARTUP_TIMEOUT_MS = 15_000; const CONNECTOR_STARTUP_POLL_MS = 100; +const CHILD_LOG_TAIL_BYTES = 8_192; +const CHILD_LOG_TAIL_LINES = 3; +const ESC = String.fromCharCode(27); +const BEL = String.fromCharCode(7); +const ANSI_SEQUENCE_PATTERN = new RegExp( + `${ESC}(?:[@-Z\\\\-_]|\\[[0-?]*[ -/]*[@-~]|\\][^${BEL}]*(?:${BEL}|${ESC}\\\\))`, + "g", +); + +function stripAnsiCodes(text: string): string { + return text.replace(ANSI_SEQUENCE_PATTERN, ""); +} + +/** + * Surface why a detached connector child died. The child's own error is the + * useful part; the parent only knows that it exited, so quote the tail of the + * child's log and point at the file for the rest. + */ +function formatChildLogHint(logPath: string): string { + const suffix = ` See ${logPath} for details.`; + let handle: number | undefined; + try { + const { size } = statSync(logPath); + if (size === 0) { + return suffix; + } + const length = Math.min(size, CHILD_LOG_TAIL_BYTES); + const buffer = Buffer.alloc(length); + handle = openSync(logPath, "r"); + const read = readSync(handle, buffer, 0, length, size - length); + const lines = buffer + .subarray(0, read) + .toString("utf8") + // A partial first line is likely when starting mid-file. + .split("\n") + .slice(size > length ? 1 : 0) + .map((line) => stripAnsiCodes(line).trim()) + .filter((line) => line.length > 0) + .slice(-CHILD_LOG_TAIL_LINES); + if (lines.length === 0) { + return suffix; + } + return ` Last output from the child:\n${lines + .map((line) => ` ${line}`) + .join("\n")}\n${suffix.trimStart()}`; + } catch { + // The log is best-effort: a missing or unreadable file must never turn a + // startup failure into a crash. + return suffix; + } finally { + if (handle !== undefined) { + try { + closeSync(handle); + } catch { + // Nothing actionable if the descriptor is already gone. + } + } + } +} export abstract class ConnectorBase implements ConnectCommandDefinition @@ -87,6 +155,31 @@ export abstract class ConnectorBase return this.runWithOptions(options, rawArgs, io, context); } + /** + * The instance id `rawArgs` would run as, when that is knowable from the + * arguments alone. + * + * The hub keys supervision by (channel, instanceId), so it needs the id + * before anything is spawned. Adapters that can only determine it with a side + * effect — Telegram resolves its bot username from the API when the flag is + * omitted — return undefined, and the caller falls back to starting the + * connector locally. + */ + resolveInstanceId(rawArgs: string[]): string | undefined { + let options: Options; + try { + options = this.parseArgs(rawArgs); + } catch { + return undefined; + } + const instanceId = this.instanceIdFromOptions(options); + return instanceId?.trim() ? instanceId.trim() : undefined; + } + + protected instanceIdFromOptions(_options: Options): string | undefined { + return undefined; + } + async validate(rawArgs: string[], io: ConnectIo): Promise { let options: Options; try { @@ -170,6 +263,19 @@ export abstract class ConnectorBase return undefined; } + /** + * Where a detached child's stdout/stderr is captured. Without this the + * child is spawned with stdio "ignore", so a child that dies during startup + * takes its only diagnostic with it and the parent can report nothing but + * "child exited before becoming ready". + */ + protected resolveDetachedLogPath(statePath: string): string { + return resolveConnectorDebugLogPath( + this.name, + basename(statePath, ".json") || this.name, + ); + } + protected async maybeRunInBackground(input: { rawArgs: string[]; io: ConnectIo; @@ -184,7 +290,13 @@ export abstract class ConnectorBase launchFailureMessage: string; startupTimeoutMs?: number; }): Promise { - if (input.interactive || process.env[input.childEnvVar] === "1") { + if ( + input.interactive || + process.env[input.childEnvVar] === "1" || + // A supervised connector is the process the hub is tracking, so it must + // run the adapter here instead of handing off to a detached child. + isSupervisedConnectorProcess() + ) { return undefined; } const runningState = input.readState(input.statePath); @@ -192,10 +304,16 @@ export abstract class ConnectorBase input.io.writeln(input.formatAlreadyRunningMessage(runningState)); return CONNECT_ALREADY_RUNNING_EXIT_CODE; } + const logPath = this.resolveDetachedLogPath(input.statePath); const pid = spawnDetachedConnector( ["connect", this.name], input.rawArgs, input.childEnvVar, + { + logPath, + component: `${this.name}-connect`, + metadata: { statePath: input.statePath }, + }, ); if (!pid) { input.io.writeErr(input.launchFailureMessage); @@ -212,7 +330,7 @@ export abstract class ConnectorBase } if (!isProcessRunning(pid)) { input.io.writeErr( - `${input.launchFailureMessage}: child exited before becoming ready`, + `${input.launchFailureMessage}: child exited before becoming ready.${formatChildLogHint(logPath)}`, ); return 1; } @@ -222,7 +340,7 @@ export abstract class ConnectorBase } await terminateProcess(pid); input.io.writeErr( - `${input.launchFailureMessage}: timed out after ${timeoutMs}ms`, + `${input.launchFailureMessage}: timed out after ${timeoutMs}ms.${formatChildLogHint(logPath)}`, ); return 1; } diff --git a/apps/cli/src/connectors/common.test.ts b/apps/cli/src/connectors/common.test.ts index c76516d569..a96bf3b844 100644 --- a/apps/cli/src/connectors/common.test.ts +++ b/apps/cli/src/connectors/common.test.ts @@ -1,4 +1,6 @@ -import { dirname, resolve } from "node:path"; +import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; import { @@ -189,3 +191,36 @@ describe("readSessionReplyText", () => { ).resolves.toBe(2); }); }); + +describe("detached connector log rotation", () => { + it("keeps one generation once the log grows past the cap", () => { + const dir = mkdtempSync(join(tmpdir(), "cline-connector-log-")); + const logPath = join(dir, "cline-slack.log"); + writeFileSync(logPath, "x".repeat(__test__.DETACHED_LOG_MAX_BYTES + 1)); + + __test__.rotateOversizedLog(logPath); + + expect(existsSync(logPath)).toBe(false); + expect(readFileSync(`${logPath}.1`, "utf8").length).toBe( + __test__.DETACHED_LOG_MAX_BYTES + 1, + ); + }); + + it("leaves a small log in place so restarts keep their history", () => { + const dir = mkdtempSync(join(tmpdir(), "cline-connector-log-")); + const logPath = join(dir, "cline-slack.log"); + writeFileSync(logPath, "recent failure"); + + __test__.rotateOversizedLog(logPath); + + expect(readFileSync(logPath, "utf8")).toBe("recent failure"); + expect(existsSync(`${logPath}.1`)).toBe(false); + }); + + it("does nothing when there is no log yet", () => { + const dir = mkdtempSync(join(tmpdir(), "cline-connector-log-")); + expect(() => + __test__.rotateOversizedLog(join(dir, "missing.log")), + ).not.toThrow(); + }); +}); diff --git a/apps/cli/src/connectors/common.ts b/apps/cli/src/connectors/common.ts index 8af5caf66f..61e01bf4a2 100644 --- a/apps/cli/src/connectors/common.ts +++ b/apps/cli/src/connectors/common.ts @@ -4,7 +4,9 @@ import { existsSync, openSync, readFileSync, + renameSync, rmSync, + statSync, writeFileSync, } from "node:fs"; import { join } from "node:path"; @@ -28,6 +30,9 @@ export const CLINE_CONNECTOR_DETACHED_CHILD_ENV = */ export const CONNECT_ALREADY_RUNNING_EXIT_CODE = 75; +/** Rotate a detached connector log once it passes this size. */ +const DETACHED_LOG_MAX_BYTES = 8 * 1024 * 1024; + export function parseBooleanFlag(rawArgs: string[], flag: string): boolean { return rawArgs.includes(flag); } @@ -164,12 +169,30 @@ export function resolveConnectorDebugLogPath( ); } +/** + * Connectors are long-lived and restart often, so an append-only log would grow + * without bound on a host that runs them for weeks. Keep one previous + * generation and start fresh once the current one gets large. + */ +function rotateOversizedLog(path: string): void { + try { + if (statSync(path).size < DETACHED_LOG_MAX_BYTES) { + return; + } + rmSync(`${path}.1`, { force: true }); + renameSync(path, `${path}.1`); + } catch { + // No log yet, or it cannot be rotated: appending is still fine. + } +} + function tryOpenDetachedLogFd(path: string | undefined): number | undefined { if (!path?.trim()) { return undefined; } try { ensureParentDir(path); + rotateOversizedLog(path); return openSync(path, "a"); } catch { return undefined; @@ -269,6 +292,8 @@ export const __test__ = { buildDetachedConnectorArgs, buildDetachedConnectorCommand, buildDetachedConnectorEnv, + rotateOversizedLog, + DETACHED_LOG_MAX_BYTES, }; export function readJsonFile(path: string, fallback: T): T { diff --git a/apps/cli/src/connectors/connector-host.test.ts b/apps/cli/src/connectors/connector-host.test.ts index 9f0befc039..1654e4a9ce 100644 --- a/apps/cli/src/connectors/connector-host.test.ts +++ b/apps/cli/src/connectors/connector-host.test.ts @@ -178,6 +178,52 @@ describe("handleConnectorUserTurn", () => { } }); + it("posts no greeting when the adapter configures none", async () => { + // Slack deliberately configures no first-contact message: the greeting is + // gated on per-thread state, so a restart or a cleared history replayed it + // on the user's next message. + const dir = mkdtempSync(join(tmpdir(), "connector-host-test-")); + tempDirs.push(dir); + const bindingsPath = join(dir, "threads.json"); + const { thread, posts } = createThread({ + enableTools: false, + autoApproveTools: false, + cwd: "/tmp/work", + workspaceRoot: "/tmp/work", + participantKey: "slack:user:alice", + participantLabel: "alice", + }); + + await handleConnectorUserTurn({ + thread: thread as never, + client: {} as never, + pendingApprovals: new Map(), + baseStartRequest: baseStartRequest() as never, + explicitSystemPrompt: undefined, + clientId: "client-1", + logger: { + core: { debug: vi.fn(), log: vi.fn(), error: vi.fn() }, + } as never, + transport: "slack", + botUserName: "cline-slack", + requestStop: vi.fn(), + bindingsPath, + systemRules: "rules", + errorLabel: "Slack", + getSessionMetadata: () => ({}), + reusedLogMessage: "reused", + text: "/whereami", + }); + + expect( + posts.filter((message) => messageText(message).includes("Connected")), + ).toEqual([]); + // The turn itself still answers. + expect(messageText(posts.at(-1))).toContain( + "participantKey=slack:user:alice", + ); + }); + it("sends a first-contact message only once per persisted thread state", async () => { const dir = mkdtempSync(join(tmpdir(), "connector-host-test-")); tempDirs.push(dir); @@ -644,6 +690,81 @@ describe("handleConnectorUserTurn", () => { ).toBe(false); }); + it("recovers when the bound session is wedged on a run that never drained", async () => { + // Cline Mom's failure: the thread pointed at a session whose runtime still + // had a run in flight, so every message came back as "SessionRuntime.shutdown + // called while a run is in progress" instead of answering. + const dir = mkdtempSync(join(tmpdir(), "connector-host-test-")); + tempDirs.push(dir); + const bindingsPath = join(dir, "threads.json"); + const { thread, posts, getState } = createThread({ + enableTools: false, + autoApproveTools: false, + cwd: "/tmp/work", + workspaceRoot: "/tmp/work", + sessionId: "wedged-session", + welcomeSentAt: new Date().toISOString(), + }); + + const runtime = createRuntimeClient("recovered reply"); + runtime.getSession.mockImplementation(async (sessionId: string) => ({ + sessionId, + })); + runtime.startRuntimeSession.mockResolvedValue({ + sessionId: "fresh-session", + }); + runtime.sendRuntimeSession.mockImplementation(async (sessionId: string) => { + if (sessionId === "wedged-session") { + // Crossing the hub's JSON boundary strips the error class, so only the + // message survives — which is exactly what the connector sees. + throw new Error( + "SessionRuntime.shutdown called while a run is in progress (agentId=agent_123)", + ); + } + return { + result: { + text: "recovered reply", + finishReason: "stop", + iterations: 1, + }, + }; + }); + + await handleConnectorUserTurn({ + thread: thread as never, + text: "are you there?", + client: runtime.client as never, + pendingApprovals: new Map(), + baseStartRequest: baseStartRequest() as never, + explicitSystemPrompt: undefined, + clientId: "client-1", + logger: { + core: { debug: vi.fn(), log: vi.fn(), error: vi.fn() }, + } as never, + transport: "slack", + botUserName: "ClineAdapterBot", + requestStop: vi.fn(), + bindingsPath, + systemRules: "rules", + errorLabel: "Slack", + getSessionMetadata: () => ({}), + reusedLogMessage: "reused", + startedLogMessage: "started", + }); + + expect( + runtime.sendRuntimeSession.mock.calls.map((call) => call[0]), + ).toEqual(["wedged-session", "fresh-session"]); + // The stale mapping is replaced, so the thread is not wedged next time. + expect(getState().sessionId).toBe("fresh-session"); + expect(messageText(posts.at(-1))).toBe("recovered reply"); + expect( + posts.some((message) => + messageText(message).includes("run is in progress"), + ), + ).toBe(false); + }); + it("does not retry forever when the replacement session is also missing", async () => { const dir = mkdtempSync(join(tmpdir(), "connector-host-test-")); tempDirs.push(dir); @@ -1777,7 +1898,11 @@ describe("handleConnectorUserTurn", () => { }), { timeoutMs: null }, ); - expect(posts.at(-1)).toEqual({ raw: "Steering current task." }); + // Handing the follow-up to the running session is silent: no acknowledgement + // line is added to the thread. + expect( + posts.some((message) => messageText(message).includes("Steering")), + ).toBe(false); }); it("steers when the same session is active under a different turn key", async () => { @@ -1829,7 +1954,11 @@ describe("handleConnectorUserTurn", () => { }), { timeoutMs: null }, ); - expect(posts.at(-1)).toEqual({ raw: "Steering current task." }); + // Handing the follow-up to the running session is silent: no acknowledgement + // line is added to the thread. + expect( + posts.some((message) => messageText(message).includes("Steering")), + ).toBe(false); }); it("starts a normal turn when the active session is in a different thread", async () => { diff --git a/apps/cli/src/connectors/connector-host.ts b/apps/cli/src/connectors/connector-host.ts index 4ebeee0354..525ef36f3f 100644 --- a/apps/cli/src/connectors/connector-host.ts +++ b/apps/cli/src/connectors/connector-host.ts @@ -6,7 +6,7 @@ import type { HubSessionClient, UserInstructionConfigService, } from "@cline/core"; -import { isSessionNotFoundError } from "@cline/core"; +import { isUnusableSessionError } from "@cline/core"; import type { SentMessage, Thread } from "chat"; import type { CliLoggerAdapter } from "../logging/adapter"; import { buildUserInputMessage, resolveSystemPrompt } from "../runtime/prompt"; @@ -975,10 +975,12 @@ export async function handleConnectorUserTurn< { timeoutMs: null }, ); } catch (error) { - if (!isSessionNotFoundError(error)) { + if (!isUnusableSessionError(error)) { throw error; } - // The tracked turn points at a session the hub no longer knows about. + // The tracked turn points at a session that can no longer serve it — + // the hub does not know it, or its runtime is stuck on a run that never + // drained. // Remove only the entry we attempted to steer, then route recovery // through the normal per-thread queue. Concurrent messages that saw // the same stale turn will line up behind this one instead of creating @@ -1002,11 +1004,10 @@ export async function handleConnectorUserTurn< ); return; } - await postConnectorText( - input.thread, - input.transport, - "Steering current task.", - ); + // No acknowledgement: the follow-up is handed to the running session and its + // effect shows up in the answer. Announcing it added a line to every thread + // and overstated what happens, since the prompt is queued for the session + // rather than injected into the loop already running. return; } @@ -1104,7 +1105,7 @@ async function runConnectorRuntimeTurnWithRecovery< }); break; } catch (error) { - if (!allowStaleSessionRetry || !isSessionNotFoundError(error)) { + if (!allowStaleSessionRetry || !isUnusableSessionError(error)) { throw error; } allowStaleSessionRetry = false; diff --git a/apps/cli/src/connectors/thread-bindings.ts b/apps/cli/src/connectors/thread-bindings.ts index cf05f89650..3539fe6cc5 100644 --- a/apps/cli/src/connectors/thread-bindings.ts +++ b/apps/cli/src/connectors/thread-bindings.ts @@ -153,6 +153,26 @@ export function writeBindings( writeJsonFile(path, bindings); } +/** + * Key under which turns for `thread` must be serialised. + * + * This has to follow the same identity rule as {@link findBindingForThread}, + * because whatever shares a session has to share a queue. A DM reuses one + * binding — and therefore one runtime session — for every message in the + * channel, so keying the queue by thread id would let two messages in the same + * DM run against that one session concurrently. That surfaces as + * "SessionRuntime.shutdown called while a run is in progress", or as two + * conversations interleaved in one session's history. + * + * Channel threads each own their binding, so they keep their own key and go on + * running independently of one another. + */ +export function resolveThreadTurnQueueKey( + thread: Pick, +): string { + return thread.isDM ? `dm:${thread.channelId}` : thread.id; +} + export function findBindingForThread( bindings: ConnectorBindingStore, thread: ConnectorBindingThreadIdentity, diff --git a/apps/cli/src/connectors/thread-turn-queue.test.ts b/apps/cli/src/connectors/thread-turn-queue.test.ts new file mode 100644 index 0000000000..ba43520fe8 --- /dev/null +++ b/apps/cli/src/connectors/thread-turn-queue.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from "vitest"; +import { enqueueThreadTurn } from "./chat-runtime"; +import { resolveThreadTurnQueueKey } from "./thread-bindings"; + +describe("resolveThreadTurnQueueKey", () => { + it("gives every channel thread its own key", () => { + // Channel threads each own a binding and a session, so they run in parallel. + const first = resolveThreadTurnQueueKey({ + id: "slack:C1:1111.1", + channelId: "slack:C1", + isDM: false, + }); + const second = resolveThreadTurnQueueKey({ + id: "slack:C1:2222.2", + channelId: "slack:C1", + isDM: false, + }); + + expect(first).not.toBe(second); + expect(first).toBe("slack:C1:1111.1"); + }); + + it("collapses every message in one DM onto a single key", () => { + // findBindingForThread reuses one binding for a whole DM channel, so those + // messages share a session and must not run concurrently. + const first = resolveThreadTurnQueueKey({ + id: "slack:D1:1111.1", + channelId: "slack:D1", + isDM: true, + }); + const second = resolveThreadTurnQueueKey({ + id: "slack:D1:2222.2", + channelId: "slack:D1", + isDM: true, + }); + + expect(first).toBe(second); + }); + + it("keeps separate DM channels separate", () => { + expect( + resolveThreadTurnQueueKey({ + id: "slack:D1:1111.1", + channelId: "slack:D1", + isDM: true, + }), + ).not.toBe( + resolveThreadTurnQueueKey({ + id: "slack:D2:1111.1", + channelId: "slack:D2", + isDM: true, + }), + ); + }); +}); + +describe("thread turn scheduling", () => { + function deferred() { + let resolve!: () => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; + } + + it("runs two messages in the same DM one after the other", async () => { + const queues = new Map>(); + const dm = { id: "slack:D1:1.1", channelId: "slack:D1", isDM: true }; + const later = { id: "slack:D1:2.2", channelId: "slack:D1", isDM: true }; + const order: string[] = []; + const first = deferred(); + + const firstTurn = enqueueThreadTurn( + queues, + resolveThreadTurnQueueKey(dm), + async () => { + order.push("first:start"); + await first.promise; + order.push("first:end"); + }, + ); + const secondTurn = enqueueThreadTurn( + queues, + resolveThreadTurnQueueKey(later), + async () => { + order.push("second:start"); + }, + ); + + // The second message must not touch the shared session until the first + // message's run has finished. + await new Promise((resolve) => setImmediate(resolve)); + expect(order).toEqual(["first:start"]); + + first.resolve(); + await Promise.all([firstTurn, secondTurn]); + expect(order).toEqual(["first:start", "first:end", "second:start"]); + }); + + it("runs two channel threads at the same time", async () => { + const queues = new Map>(); + const threadA = { id: "slack:C1:1.1", channelId: "slack:C1", isDM: false }; + const threadB = { id: "slack:C1:2.2", channelId: "slack:C1", isDM: false }; + const order: string[] = []; + const blocked = deferred(); + + const turnA = enqueueThreadTurn( + queues, + resolveThreadTurnQueueKey(threadA), + async () => { + order.push("a:start"); + await blocked.promise; + order.push("a:end"); + }, + ); + const turnB = enqueueThreadTurn( + queues, + resolveThreadTurnQueueKey(threadB), + async () => { + order.push("b:start"); + }, + ); + + // B answers while A is still working: separate threads, separate sessions. + await new Promise((resolve) => setImmediate(resolve)); + expect(order).toEqual(["a:start", "b:start"]); + + blocked.resolve(); + await Promise.all([turnA, turnB]); + expect(order).toEqual(["a:start", "b:start", "a:end"]); + }); +}); diff --git a/apps/cli/src/connectors/types.ts b/apps/cli/src/connectors/types.ts index f1e65e8130..8e2d27f0ee 100644 --- a/apps/cli/src/connectors/types.ts +++ b/apps/cli/src/connectors/types.ts @@ -24,6 +24,12 @@ export interface ConnectCommandDefinition { ): Promise; validate(args: string[], io: ConnectIo): Promise; showHelp(io: ConnectIo): void; + /** + * Instance id `args` would run as, when it is knowable without side effects. + * The hub keys connector supervision by (channel, instanceId) and needs it + * before spawning; undefined sends the caller to the local start path. + */ + resolveInstanceId?(args: string[]): string | undefined; stopAll?(io: ConnectIo): Promise; stopInstance?(instanceId: string, io: ConnectIo): Promise; } diff --git a/apps/cli/src/index.ts b/apps/cli/src/index.ts index 71a854367e..547703d65c 100755 --- a/apps/cli/src/index.ts +++ b/apps/cli/src/index.ts @@ -2,9 +2,10 @@ import { isMainThread } from "node:worker_threads"; import { + claimHubDaemonProcess, + claimSupervisedConnectorProcess, disposeAll, initVcr, - isHubDaemonProcess, setConnectorCliLaunchSpec, } from "@cline/shared"; import { logCliProcessError } from "./logging/errors"; @@ -22,11 +23,18 @@ initVcr(process.env.CLINE_VCR); if (!isMainThread) { // Worker imports of the bundled CLI entrypoint should not start the CLI. -} else if (isHubDaemonProcess()) { +} else if (claimHubDaemonProcess()) { + // Claim rather than read: the sentinel is consumed here so the processes a + // daemon-hosted session spawns do not inherit it and try to become daemons. // The hub daemon owns its process-level abort handling. Installing the CLI's // fatal rejection handler first would make expected abort rejections exit it. void import("@cline/core/hub/daemon-entry"); } else { + // Same reasoning as the daemon sentinel above: consume the supervised-connector + // marker so the processes an agent session spawns cannot inherit it and mistake + // themselves for the connector the hub is tracking. + claimSupervisedConnectorProcess(); + const cliLaunchSpec = resolveCliLaunchSpec({ debugRole: "connector" }); if (cliLaunchSpec) { setConnectorCliLaunchSpec({ diff --git a/apps/cli/src/main.test.ts b/apps/cli/src/main.test.ts index 71b6e0f2e9..64ba0db003 100644 --- a/apps/cli/src/main.test.ts +++ b/apps/cli/src/main.test.ts @@ -66,6 +66,7 @@ const dashboardMocks = vi.hoisted(() => ({ })); const connectMocks = vi.hoisted(() => ({ formatAdapterList: vi.fn(() => ""), + runCleanupConnectorInstance: vi.fn(async () => 0), runConnectAdapter: vi.fn(async () => 0), runRestartConnector: vi.fn(async () => 0), runStopAllConnectors: vi.fn(async () => 0), @@ -396,6 +397,68 @@ describe("runCli lightweight command dispatch", () => { expect(connectMocks.runStopConnector).not.toHaveBeenCalled(); }); + it("routes a supervised cleanup to one connector instance", async () => { + connectMocks.runCleanupConnectorInstance.mockClear(); + connectMocks.runConnectAdapter.mockClear(); + process.argv = [ + "bun", + "src/index.ts", + "connect", + "--cleanup-instance", + "cline-slack", + "slack", + ]; + + const { runCli } = await import("./main"); + + await expect(runCli()).resolves.toBeUndefined(); + expect(process.exitCode).toBe(0); + expect(connectMocks.runCleanupConnectorInstance).toHaveBeenCalledWith( + "slack", + "cline-slack", + expect.any(Object), + ); + expect(connectMocks.runConnectAdapter).not.toHaveBeenCalled(); + }); + + it("rejects combining cleanup with another connect mode", async () => { + connectMocks.runCleanupConnectorInstance.mockClear(); + connectMocks.runStopConnector.mockClear(); + process.argv = [ + "bun", + "src/index.ts", + "connect", + "--cleanup-instance", + "cline-slack", + "--stop", + "slack", + ]; + + const { runCli } = await import("./main"); + + await runCli(); + expect(process.exitCode).toBe(1); + expect(connectMocks.runCleanupConnectorInstance).not.toHaveBeenCalled(); + expect(connectMocks.runStopConnector).not.toHaveBeenCalled(); + }); + + it("requires a channel for a supervised cleanup", async () => { + connectMocks.runCleanupConnectorInstance.mockClear(); + process.argv = [ + "bun", + "src/index.ts", + "connect", + "--cleanup-instance", + "x", + ]; + + const { runCli } = await import("./main"); + + await runCli(); + expect(process.exitCode).toBe(1); + expect(connectMocks.runCleanupConnectorInstance).not.toHaveBeenCalled(); + }); + it("routes a targeted connector restart to one instance", async () => { process.argv = [ "bun", diff --git a/apps/cli/src/main.ts b/apps/cli/src/main.ts index e495c9ea4d..32cf09ccd3 100644 --- a/apps/cli/src/main.ts +++ b/apps/cli/src/main.ts @@ -384,6 +384,10 @@ export async function runCli(): Promise { "--restart-instance ", "Restart one connector instance (used by daemon recovery)", ) + .option( + "--cleanup-instance ", + "Reap one dead connector instance, preserving autostart (used by hub supervision)", + ) .allowUnknownOption() .passThroughOptions() .addHelpText( @@ -393,15 +397,34 @@ export async function runCli(): Promise { .action(async (adapter: string | undefined) => { const { formatAdapterList, + runCleanupConnectorInstance, runConnectAdapter, runRestartConnector, runStopAllConnectors, runStopConnector, } = await import("./commands/connect"); const opts = connectCmd.opts(); - if (opts.stop && (opts.restart || opts.restartInstance)) { - io.writeErr("connect accepts only one of --stop or --restart"); + const exclusiveModes = [ + opts.stop, + opts.restart || opts.restartInstance, + opts.cleanupInstance, + ].filter(Boolean).length; + if (exclusiveModes > 1) { + io.writeErr( + "connect accepts only one of --stop, --restart or --cleanup-instance", + ); ctx.exitCode = 1; + } else if (opts.cleanupInstance) { + if (!adapter) { + io.writeErr("connect --cleanup-instance requires a channel"); + ctx.exitCode = 1; + } else { + ctx.exitCode = await runCleanupConnectorInstance( + adapter, + opts.cleanupInstance, + io, + ); + } } else if (opts.stop) { if (adapter) { ctx.exitCode = await runStopConnector(adapter, io); diff --git a/apps/examples/desktop-app/sidecar/index.ts b/apps/examples/desktop-app/sidecar/index.ts index 520c6a04f3..7cf3177f5b 100644 --- a/apps/examples/desktop-app/sidecar/index.ts +++ b/apps/examples/desktop-app/sidecar/index.ts @@ -1,6 +1,6 @@ import { homedir } from "node:os"; import { setHomeDirIfUnset } from "@cline/core"; -import { isHubDaemonProcess } from "@cline/shared"; +import { claimHubDaemonProcess } from "@cline/shared"; import { prewarmWorkspaceMetadata } from "./chat-session"; import { configureConnectorCliLaunch } from "./connectors"; import { @@ -137,7 +137,9 @@ async function main() { } async function runEntrypoint(): Promise { - if (isHubDaemonProcess()) { + // Claim rather than read: consuming the sentinel keeps daemon-hosted sessions + // from handing it to every process they spawn. + if (claimHubDaemonProcess()) { await import("@cline/core/hub/daemon-entry"); return; } diff --git a/apps/examples/menubar/sidecar/index.ts b/apps/examples/menubar/sidecar/index.ts index 4b4fc5197f..659365167f 100644 --- a/apps/examples/menubar/sidecar/index.ts +++ b/apps/examples/menubar/sidecar/index.ts @@ -10,7 +10,11 @@ import { stopLocalHubServerGracefully, toHubStatusUrl, } from "@cline/core"; -import type { HubUINotifyPayload, SessionRecord } from "@cline/shared"; +import { + claimHubDaemonProcess, + type HubUINotifyPayload, + type SessionRecord, +} from "@cline/shared"; import { configureMenubarConnectorCliLaunch } from "./connector-cli-launch"; interface TrackedClient { @@ -940,7 +944,13 @@ async function main(): Promise { }); } -if (isBundledDaemonEntryInvocation()) { +// Claim unconditionally, before the personality is decided: the spawn path sets +// the sentinel on every daemon it launches, and this host selects the daemon by +// argv. Leaving the variable in the environment would hand it to every process a +// daemon-hosted session spawns — agent shell commands, MCP servers, hooks — each +// of which would then try to become a hub daemon and die on EADDRINUSE. +const claimedDaemonSentinel = claimHubDaemonProcess(); +if (claimedDaemonSentinel || isBundledDaemonEntryInvocation()) { await import("@cline/core/hub/daemon-entry"); } else { main().catch((err) => { diff --git a/sdk/packages/core/src/hub/daemon/entry.test.ts b/sdk/packages/core/src/hub/daemon/entry.test.ts index 0fa204ee0b..9df95e72ca 100644 --- a/sdk/packages/core/src/hub/daemon/entry.test.ts +++ b/sdk/packages/core/src/hub/daemon/entry.test.ts @@ -57,10 +57,14 @@ const { }; }); -vi.mock("@cline/shared", () => ({ - initVcr: mockInitVcr, - resolveClineBuildEnv: () => "production", -})); +vi.mock("@cline/shared", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + initVcr: mockInitVcr, + resolveClineBuildEnv: () => "production", + }; +}); vi.mock("@cline/agents", () => ({ AgentRuntimeAbortError: class AgentRuntimeAbortError extends Error {}, diff --git a/sdk/packages/core/src/hub/daemon/entry.ts b/sdk/packages/core/src/hub/daemon/entry.ts index ba076f8435..d89b69395a 100644 --- a/sdk/packages/core/src/hub/daemon/entry.ts +++ b/sdk/packages/core/src/hub/daemon/entry.ts @@ -1,5 +1,10 @@ import { AgentRuntimeAbortError } from "@cline/agents"; import { initVcr, resolveClineBuildEnv } from "@cline/shared"; +import { cleanupConnectorInstanceViaCli } from "../../services/connectors/connector-cleanup"; +import { + ConnectorSupervisor, + setActiveConnectorSupervisor, +} from "../../services/connectors/connector-supervisor"; import { reconnectDaemonConnectors } from "../../services/connectors/daemon-connector-reconnect"; import { createLocalHubScheduleRuntimeHandlers } from "../daemon/runtime-handlers"; import { resolveHubEndpointOptions } from "../discovery/defaults"; @@ -104,7 +109,20 @@ async function main(): Promise { throw error; } + // Owns connector processes for this hub's lifetime: one instance per + // (channel, instanceId), reaping and backoff restarts when they die. + const supervisor = new ConnectorSupervisor({ + cleanupInstance: (channel, instanceId) => + cleanupConnectorInstanceViaCli(channel, instanceId), + }); + setActiveConnectorSupervisor(supervisor); + const shutdown = async (): Promise => { + // Stop supervising but leave the connectors running: they are detached on + // purpose so a hub restart does not disconnect Slack/Telegram, and the next + // hub adopts them from their state files. + supervisor.dispose(); + setActiveConnectorSupervisor(undefined); await server.close(); await daemonTelemetry.dispose().catch(() => undefined); process.exit(0); @@ -161,6 +179,10 @@ async function main(): Promise { resolveHubDaemonReady(); try { + // Adopt first: connectors that outlived the previous hub have to be known + // before recovery runs, so they are restarted onto this hub's session + // instead of being started a second time alongside themselves. + supervisor.adoptRunningConnectors(); await reconnectDaemonConnectors(); } catch (error) { const message = diff --git a/sdk/packages/core/src/hub/server/handlers/connector-handlers.test.ts b/sdk/packages/core/src/hub/server/handlers/connector-handlers.test.ts index 95863b47bc..b1d4456d0a 100644 --- a/sdk/packages/core/src/hub/server/handlers/connector-handlers.test.ts +++ b/sdk/packages/core/src/hub/server/handlers/connector-handlers.test.ts @@ -8,6 +8,10 @@ import { withConnectorStore, } from "@cline/shared/db"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + type ConnectorSupervisor, + setActiveConnectorSupervisor, +} from "../../../services/connectors/connector-supervisor"; import { __test__, handleConnectorCommand } from "./connector-handlers"; import type { HubTransportContext } from "./context"; @@ -374,3 +378,194 @@ describe("connector hub handlers", () => { }); }); }); + +describe("supervised connector hub commands", () => { + afterEach(() => { + setActiveConnectorSupervisor(undefined); + vi.clearAllMocks(); + }); + + function createHubContext(): HubTransportContext { + return { + clients: new Map(), + sessionState: new Map(), + pendingApprovals: new Map(), + pendingCapabilityRequests: new Map(), + suppressNextTerminalEventBySession: new Map(), + telemetry: { capture: vi.fn() } as never, + sessionHost: {} as never, + publish: vi.fn(), + buildEvent: vi.fn() as never, + requestCapability: vi.fn() as never, + }; + } + + function connectorCommand( + command: HubCommandEnvelope["command"], + payload?: Record, + ): HubCommandEnvelope { + return { + version: "v1", + requestId: `req-${command}`, + command, + payload, + }; + } + + function useSupervisor(): { started: unknown[]; stopped: unknown[] } { + const started: unknown[] = []; + const stopped: unknown[] = []; + const supervisor = { + start: async (request: unknown) => { + started.push(request); + return { + started: true, + record: { + channel: "slack", + instanceId: "cline-slack", + state: "running", + origin: "spawned", + restarts: 0, + }, + }; + }, + stop: async (request: unknown) => { + stopped.push(request); + return true; + }, + list: () => [ + { + channel: "slack", + instanceId: "cline-slack", + state: "running" as const, + origin: "spawned" as const, + restarts: 2, + }, + ], + } as unknown as ConnectorSupervisor; + setActiveConnectorSupervisor(supervisor); + return { started, stopped }; + } + + it("starts a connector through the supervisor", async () => { + const { started } = useSupervisor(); + + const reply = await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.start", { + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }), + ); + + expect(reply.ok).toBe(true); + expect(started).toEqual([ + { + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + restart: false, + }, + ]); + }); + + it("passes the restart intent through", async () => { + const { started } = useSupervisor(); + + await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.start", { + channel: "slack", + instanceId: "cline-slack", + args: [], + restart: true, + }), + ); + + expect((started[0] as { restart: boolean }).restart).toBe(true); + }); + + it("stops a connector and disables autostart by default", async () => { + const { stopped } = useSupervisor(); + + const reply = await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.stop", { + channel: "slack", + instanceId: "cline-slack", + }), + ); + + expect(reply.ok).toBe(true); + expect(stopped).toEqual([ + { channel: "slack", instanceId: "cline-slack", disableAutostart: true }, + ]); + }); + + it("keeps autostart when the caller asks it to", async () => { + const { stopped } = useSupervisor(); + + await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.stop", { + channel: "slack", + instanceId: "cline-slack", + disableAutostart: false, + }), + ); + + expect((stopped[0] as { disableAutostart: boolean }).disableAutostart).toBe( + false, + ); + }); + + it("lists supervised connectors", async () => { + useSupervisor(); + + const reply = await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.supervised"), + ); + + expect(reply.ok).toBe(true); + expect( + (reply.payload as { supervised: Array<{ restarts: number }> }).supervised, + ).toEqual([ + { + channel: "slack", + instanceId: "cline-slack", + state: "running", + origin: "spawned", + restarts: 2, + }, + ]); + }); + + it("rejects a start with no instance id", async () => { + useSupervisor(); + + const reply = await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.start", { channel: "slack" }), + ); + + expect(reply.ok).toBe(false); + expect(reply.error?.message).toContain("instanceId is required"); + }); + + it("reports clearly when the hub has no supervisor", async () => { + const reply = await handleConnectorCommand( + createHubContext(), + connectorCommand("connector.start", { + channel: "slack", + instanceId: "cline-slack", + }), + ); + + expect(reply.ok).toBe(false); + expect(reply.error?.message).toContain( + "connector supervision is unavailable", + ); + }); +}); diff --git a/sdk/packages/core/src/hub/server/handlers/connector-handlers.ts b/sdk/packages/core/src/hub/server/handlers/connector-handlers.ts index 873facde38..3c2bb6add9 100644 --- a/sdk/packages/core/src/hub/server/handlers/connector-handlers.ts +++ b/sdk/packages/core/src/hub/server/handlers/connector-handlers.ts @@ -16,6 +16,7 @@ import { } from "@cline/shared"; import { withConnectorStore } from "@cline/shared/db"; import { listActiveConnectors } from "../../../services/connectors/active-connectors"; +import { getActiveConnectorSupervisor } from "../../../services/connectors/connector-supervisor"; import { captureToolUsage } from "../../../services/telemetry/core-events"; import { errorReply, type HubTransportContext, okReply } from "./context"; @@ -165,11 +166,75 @@ function deleteConnectorConfig(payload: unknown): ConnectorChannelsResponse { return connectorChannelsPayload(); } +function asStringArray(value: unknown): string[] { + return Array.isArray(value) + ? value.filter((entry): entry is string => typeof entry === "string") + : []; +} + +/** + * The hub is the single authority on which connector processes may run, so the + * supervisor has to exist before these commands mean anything. It is absent in + * hosts that embed the hub server without the daemon entrypoint. + */ +function requireSupervisor() { + const supervisor = getActiveConnectorSupervisor(); + if (!supervisor) { + throw new Error( + "connector supervision is unavailable in this hub; start connectors with the CLI instead", + ); + } + return supervisor; +} + +function parseInstanceTarget(payload: unknown): { + channel: string; + instanceId: string; +} { + if (!isRecord(payload)) { + throw new Error("payload must be an object."); + } + const channel = asString(payload.channel); + if (!channel) { + throw new Error("channel is required"); + } + const instanceId = asString(payload.instanceId); + if (!instanceId) { + throw new Error("instanceId is required"); + } + return { channel, instanceId }; +} + +async function startSupervisedConnector(payload: unknown) { + const { channel, instanceId } = parseInstanceTarget(payload); + const record = isRecord(payload) ? payload : {}; + return await requireSupervisor().start({ + channel, + instanceId, + args: asStringArray(record.args), + restart: record.restart === true, + }); +} + +async function stopSupervisedConnector(payload: unknown) { + const { channel, instanceId } = parseInstanceTarget(payload); + const record = isRecord(payload) ? payload : {}; + const stopped = await requireSupervisor().stop({ + channel, + instanceId, + disableAutostart: record.disableAutostart !== false, + }); + return { stopped, channel, instanceId }; +} + function isStateMutatingConnectorCommand( command: HubCommandEnvelope["command"], ) { return ( - command === "connector.configure" || command === "connector.delete_config" + command === "connector.configure" || + command === "connector.delete_config" || + command === "connector.start" || + command === "connector.stop" ); } @@ -206,6 +271,19 @@ export async function handleConnectorCommand( captureConnectorCommandUsage(ctx, envelope, true); return okReply(envelope, payload); } + if (envelope.command === "connector.start") { + const payload = await startSupervisedConnector(envelope.payload); + captureConnectorCommandUsage(ctx, envelope, true); + return okReply(envelope, payload); + } + if (envelope.command === "connector.stop") { + const payload = await stopSupervisedConnector(envelope.payload); + captureConnectorCommandUsage(ctx, envelope, true); + return okReply(envelope, payload); + } + if (envelope.command === "connector.supervised") { + return okReply(envelope, { supervised: requireSupervisor().list() }); + } return errorReply( envelope, "unsupported_connector_command", @@ -225,4 +303,6 @@ export const __test__ = { configureConnector, connectorChannelsPayload, deleteConnectorConfig, + startSupervisedConnector, + stopSupervisedConnector, }; diff --git a/sdk/packages/core/src/hub/server/hub-server-transport.ts b/sdk/packages/core/src/hub/server/hub-server-transport.ts index 7b6d89bc48..c4e1b4e6a1 100644 --- a/sdk/packages/core/src/hub/server/hub-server-transport.ts +++ b/sdk/packages/core/src/hub/server/hub-server-transport.ts @@ -411,6 +411,9 @@ export class HubServerTransport implements NativeHubTransport { case "connector.channels": case "connector.configure": case "connector.delete_config": + case "connector.start": + case "connector.stop": + case "connector.supervised": return await handleConnectorCommand(this.ctx, envelope); case "settings.get": case "settings.patch": diff --git a/sdk/packages/core/src/index.ts b/sdk/packages/core/src/index.ts index 2bc4188f44..ca06e37f4a 100644 --- a/sdk/packages/core/src/index.ts +++ b/sdk/packages/core/src/index.ts @@ -460,6 +460,7 @@ export type { } from "./runtime/host/runtime-host"; export { isSessionNotFoundError, + isUnusableSessionError, SESSION_NOT_FOUND_ERROR_CODE, SessionNotFoundError, splitCoreSessionConfig, @@ -504,6 +505,21 @@ export { reconnectPersistedConnectors, removePersistedConnectorConnection, } from "./services/connectors/connector-autostart"; +export { buildConnectorChildEnv } from "./services/connectors/connector-child-env"; +export { cleanupConnectorInstanceViaCli } from "./services/connectors/connector-cleanup"; +export { + ADOPTED_POLL_INTERVAL_MS, + ConnectorSupervisor, + type ConnectorSupervisorDeps, + getActiveConnectorSupervisor, + RESTART_BASE_DELAY_MS, + RESTART_COUNTER_RESET_MS, + RESTART_GIVE_UP_AFTER, + RESTART_MAX_DELAY_MS, + setActiveConnectorSupervisor, + STOP_SIGKILL_TIMEOUT_MS, + STOP_SIGTERM_TIMEOUT_MS, +} from "./services/connectors/connector-supervisor"; export { FeatureFlagsService, type FeatureFlagsServiceOptions, @@ -654,7 +670,10 @@ export { SqliteTeamStore, type SqliteTeamStoreOptions, } from "./services/storage/team-store"; -export { resolveCoreDeviceId, resolveCoreDistinctId } from "./services/telemetry"; +export { + resolveCoreDeviceId, + resolveCoreDistinctId, +} from "./services/telemetry"; export type { CaptureAgentUnexpectedReasoningTokensInput, CaptureCompactionExecutedProperties, diff --git a/sdk/packages/core/src/runtime/host/local-runtime-host.test.ts b/sdk/packages/core/src/runtime/host/local-runtime-host.test.ts index 429d6547a7..33f346d9ad 100644 --- a/sdk/packages/core/src/runtime/host/local-runtime-host.test.ts +++ b/sdk/packages/core/src/runtime/host/local-runtime-host.test.ts @@ -6212,4 +6212,90 @@ describe("LocalRuntimeHost", () => { expect(emissions[0]).toMatchObject({ source: "shutdown" }); }); }); + + describe("LocalRuntimeHost releasing a session mid-run", () => { + it("aborts and drains the run before shutting the sandbox down", async () => { + // A hub restart disposes its sessions. Tearing one down while a run is in + // flight used to reject shutdown ("a run is in progress") and SIGTERM the + // plugin sandbox with tool calls still pending, so a connector turn awaiting + // the run got an error instead of an answer. + const order: string[] = []; + let running = true; + const agent = { + run: vi.fn().mockResolvedValue(createResult()), + continue: vi.fn().mockResolvedValue(createResult()), + getMessages: vi.fn().mockReturnValue([]), + getAgentId: vi.fn().mockReturnValue("agent-mid-run"), + getConversationId: vi.fn().mockReturnValue("conv-mid-run"), + abort: vi.fn(() => { + order.push("agent.abort"); + running = false; + }), + subscribeEvents: vi.fn().mockReturnValue(() => {}), + // Reports "busy" until the abort lands, like a live run. + canStartRun: vi.fn(() => !running), + shutdown: vi.fn(async () => { + order.push("agent.shutdown"); + }), + }; + const runtimeShutdown = vi.fn(async () => { + order.push("runtime.shutdown"); + if (running) { + throw new Error( + "SessionRuntime.shutdown called while a run is in progress (agentId=agent-mid-run)", + ); + } + }); + const runtimeBuilder = { + build: vi + .fn() + .mockReturnValue({ tools: [], shutdown: runtimeShutdown }), + }; + const manager = new RuntimeHostUnderTest({ + distinctId, + sessionService: new FileSessionService( + join(isolatedHomeDir, "sessions"), + ), + runtimeBuilder: runtimeBuilder as never, + createAgent: () => agent as never, + }); + + const started = await manager.startSession({ + config: { + providerId: "mock-provider", + modelId: "mock-model", + systemPrompt: "You are a test agent", + enableTools: false, + enableSpawnAgent: false, + enableAgentTeams: false, + }, + }); + const session = ( + manager as unknown as { + sessions: Map< + string, + { pluginSandboxShutdown?: () => Promise } + >; + } + ).sessions.get(started.sessionId); + if (!session) { + throw new Error("session was not registered"); + } + session.pluginSandboxShutdown = async () => { + order.push("sandbox.shutdown"); + }; + + // dispose() is what a hub restart runs. + await manager.dispose("hub_restart"); + + // The abort has to come first, so the runtime drains instead of refusing and + // the sandbox is only killed once no tool call can be pending. + expect(order[0]).toBe("agent.abort"); + expect(order).toContain("sandbox.shutdown"); + expect(order.indexOf("agent.abort")).toBeLessThan( + order.indexOf("sandbox.shutdown"), + ); + expect(agent.abort).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/sdk/packages/core/src/runtime/host/local-runtime-host.ts b/sdk/packages/core/src/runtime/host/local-runtime-host.ts index e0118f72f5..30f4dc5cda 100644 --- a/sdk/packages/core/src/runtime/host/local-runtime-host.ts +++ b/sdk/packages/core/src/runtime/host/local-runtime-host.ts @@ -2012,6 +2012,18 @@ export class LocalRuntimeHost implements RuntimeHost { } notifyTeamRunWaiters(session); + // Drain an in-flight run before tearing anything down. `stopSession` aborts + // first for exactly this reason; callers that arrive here another way — hub + // `dispose()` on a restart, most notably — otherwise hit two failures at + // once: the runtime refuses to shut down while a run is in progress, and the + // plugin sandbox is SIGTERMed with tool calls still pending, so those calls + // reject with "plugin-sandbox process exited". A connector turn awaiting the + // run sees whichever surfaced first instead of an answer. + if (!session.aborting && !session.agent.canStartRun()) { + session.aborting = true; + session.agent.abort(new Error(input.shutdownReason)); + } + const cleanupErrors: unknown[] = []; const recordCleanupError = (stage: string, error: unknown) => { cleanupErrors.push(error); @@ -2104,6 +2116,19 @@ export class LocalRuntimeHost implements RuntimeHost { }); }; + // Drain an in-flight run before tearing anything down, the same way + // stopSession does for its non-interactive path. + // + // Without this, releasing a session that is mid-run fails twice over: the + // runtime refuses to shut down ("a run is in progress") and that error is + // rethrown below, and the plugin sandbox is SIGTERMed while tool calls are + // still pending, so those calls reject with "plugin-sandbox process exited". + // A connector turn awaiting the run sees whichever surfaced first instead of + // an answer — which is what a hub restart looked like from Slack. + if (!session.aborting && !session.agent.canStartRun()) { + session.aborting = true; + session.agent.abort(new Error(reason)); + } try { await session.agent.shutdown(reason); } catch (error) { diff --git a/sdk/packages/core/src/runtime/host/runtime-host.test.ts b/sdk/packages/core/src/runtime/host/runtime-host.test.ts new file mode 100644 index 0000000000..580f17a098 --- /dev/null +++ b/sdk/packages/core/src/runtime/host/runtime-host.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import { SessionRunInProgressError } from "../orchestration/session-runtime-orchestrator"; +import { + isSessionNotFoundError, + isUnusableSessionError, + SessionNotFoundError, +} from "./runtime-host"; + +describe("isUnusableSessionError", () => { + it("matches a missing session", () => { + expect(isUnusableSessionError(new SessionNotFoundError("sess_1"))).toBe( + true, + ); + expect( + isUnusableSessionError( + Object.assign(new Error("session not found: sess_1"), { + code: "session_not_found", + }), + ), + ).toBe(true); + }); + + it("matches a session wedged on a run that never drained", () => { + expect( + isUnusableSessionError(new SessionRunInProgressError("agent_1")), + ).toBe(true); + }); + + it("matches the wedged-run error by message once the code is gone", () => { + // Errors reaching a connector have crossed the hub's JSON boundary, which + // leaves only the message — and hub and CLI are often different versions. + expect( + isUnusableSessionError( + new Error( + "SessionRuntime.shutdown called while a run is in progress (agentId=agent_1)", + ), + ), + ).toBe(true); + expect( + isUnusableSessionError({ + message: + "SessionRuntime.shutdown called while a run is in progress (agentId=agent_1)", + }), + ).toBe(true); + }); + + it("leaves ordinary run failures alone", () => { + // Replacing the session would hide a real error and lose the conversation. + expect(isUnusableSessionError(new Error("provider returned 500"))).toBe( + false, + ); + expect(isUnusableSessionError(new Error("aborted by user"))).toBe(false); + expect(isUnusableSessionError(undefined)).toBe(false); + expect(isUnusableSessionError("some string")).toBe(false); + }); + + it("keeps the narrower missing-session check narrow", () => { + // Callers that specifically mean "the hub forgot this session" must not + // start matching wedged runtimes. + expect( + isSessionNotFoundError(new SessionRunInProgressError("agent_1")), + ).toBe(false); + }); +}); + +describe("SessionRunInProgressError", () => { + it("carries a stable code and keeps the original message", () => { + const error = new SessionRunInProgressError("agent_1"); + + expect(error.code).toBe("session_run_in_progress"); + expect(error.message).toBe( + "SessionRuntime.shutdown called while a run is in progress (agentId=agent_1)", + ); + expect(error).toBeInstanceOf(Error); + }); + + it("reads sensibly without an agent id", () => { + expect(new SessionRunInProgressError().message).toBe( + "SessionRuntime.shutdown called while a run is in progress", + ); + }); +}); diff --git a/sdk/packages/core/src/runtime/host/runtime-host.ts b/sdk/packages/core/src/runtime/host/runtime-host.ts index bb1502848b..194c5fffc8 100644 --- a/sdk/packages/core/src/runtime/host/runtime-host.ts +++ b/sdk/packages/core/src/runtime/host/runtime-host.ts @@ -51,6 +51,47 @@ export function isSessionNotFoundError( ); } +function errorMessageOf(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + if (typeof error === "object" && error !== null && "message" in error) { + const message = (error as { message?: unknown }).message; + return typeof message === "string" ? message : ""; + } + return typeof error === "string" ? error : ""; +} + +/** + * A session that cannot serve another turn, whatever the caller does with it. + * + * Two distinct causes, one remedy: the session is gone (`session_not_found`, + * after a hub restart, a deletion, or retention cleanup), or its runtime is stuck + * with a run that never drained (`session_run_in_progress`). A caller holding a + * long-lived mapping to that session — a connector thread, for instance — has to + * replace the session rather than keep retrying against it. + * + * Errors reaching a connector have crossed the hub's JSON boundary, so the code + * may be gone and only the message survives; both are checked, which also keeps + * this working when the hub and the CLI are different versions. + */ +export function isUnusableSessionError(error: unknown): boolean { + if (isSessionNotFoundError(error)) { + return true; + } + if ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { code?: unknown }).code === "session_run_in_progress" + ) { + return true; + } + return errorMessageOf(error).includes( + "shutdown called while a run is in progress", + ); +} + type LocalOnlyCoreSessionConfigKeys = | "hooks" | "logger" diff --git a/sdk/packages/core/src/runtime/orchestration/session-runtime-orchestrator.ts b/sdk/packages/core/src/runtime/orchestration/session-runtime-orchestrator.ts index 262bc51e32..64ffd0b460 100644 --- a/sdk/packages/core/src/runtime/orchestration/session-runtime-orchestrator.ts +++ b/sdk/packages/core/src/runtime/orchestration/session-runtime-orchestrator.ts @@ -76,6 +76,30 @@ import { LoopDetectionTracker } from "../safety/loop-detection"; import { MistakeTracker } from "../safety/mistake-tracker"; import { RuntimeEventAdapter } from "./runtime-event-adapter"; +export const SESSION_RUN_IN_PROGRESS_ERROR_CODE = "session_run_in_progress"; + +/** + * A session was asked to shut down while one of its runs was still in flight and + * no abort had been requested. + * + * Carries a code so callers can recognise it structurally after it crosses the + * hub's JSON boundary, where an `Error` arrives as a bare message. Connectors use + * it to tell "this thread's session is unusable" apart from a genuine run failure, + * and to recover by starting a fresh session instead of wedging the thread. + */ +export class SessionRunInProgressError extends Error { + readonly code = SESSION_RUN_IN_PROGRESS_ERROR_CODE; + + constructor(readonly agentId?: string) { + super( + `SessionRuntime.shutdown called while a run is in progress${ + agentId ? ` (agentId=${agentId})` : "" + }`, + ); + this.name = "SessionRunInProgressError"; + } +} + function formatToolResultError(output: unknown): string { if (typeof output === "string") { return output; @@ -628,9 +652,7 @@ export class SessionRuntime { async shutdown(_reason?: string, _timeoutMs?: number): Promise { if (this.running) { if (!this.abortRequested || !this.activeRunPromise) { - throw new Error( - `SessionRuntime.shutdown called while a run is in progress (agentId=${this.agentId})`, - ); + throw new SessionRunInProgressError(this.agentId); } await this.activeRunPromise; } diff --git a/sdk/packages/core/src/services/connectors/connector-autostart.ts b/sdk/packages/core/src/services/connectors/connector-autostart.ts index 4061bfe51f..b66f3f644a 100644 --- a/sdk/packages/core/src/services/connectors/connector-autostart.ts +++ b/sdk/packages/core/src/services/connectors/connector-autostart.ts @@ -126,6 +126,9 @@ export async function reconnectPersistedConnectors( for (const target of candidates) { const { channel, instanceId } = target; if (options.isHealthy?.(target)) { + log( + `[connect] skipping ${channel} connector ${instanceId}: already live in this host`, + ); continue; } log(`[connect] reconnecting ${channel} connector ${instanceId}`); diff --git a/sdk/packages/core/src/services/connectors/connector-child-env.ts b/sdk/packages/core/src/services/connectors/connector-child-env.ts new file mode 100644 index 0000000000..dff9964d6b --- /dev/null +++ b/sdk/packages/core/src/services/connectors/connector-child-env.ts @@ -0,0 +1,39 @@ +import { + CLINE_CONNECTOR_STARTING_INSTANCE_ENV, + CLINE_RUN_AS_HUB_DAEMON_ENV, +} from "@cline/shared"; + +/** + * Env markers a connector sets on its own detached child: the shared + * `CLINE_CONNECTOR_DETACHED_CHILD` plus one per adapter + * (`CLINE_SLACK_CONNECT_CHILD`, `CLINE_TELEGRAM_CONNECT_CHILD`, ...). They are + * owned by the CLI, so match them by shape rather than importing upward. + */ +const CONNECTOR_CHILD_MARKER_PATTERN = + /^CLINE_(?:CONNECTOR_DETACHED_CHILD|[A-Z0-9]+_CONNECT_CHILD)$/; + +/** + * Environment for a CLI process the hub daemon launches. + * + * The daemon inherits the environment of whichever connector spawned it, and + * those inherited markers are actively harmful downstream: the daemon sentinel + * would make the child try to become a hub, and a child marker tells a connector + * "you are already the detached child", which makes it skip its own + * already-running check and start alongside a live instance holding the same + * credentials. + */ +export function buildConnectorChildEnv( + env: NodeJS.ProcessEnv = process.env, +): NodeJS.ProcessEnv { + const childEnv = { ...env }; + delete childEnv[CLINE_RUN_AS_HUB_DAEMON_ENV]; + delete childEnv[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; + for (const key of Object.keys(childEnv)) { + if (CONNECTOR_CHILD_MARKER_PATTERN.test(key)) { + delete childEnv[key]; + } + } + return childEnv; +} + +export const __test__ = { CONNECTOR_CHILD_MARKER_PATTERN }; diff --git a/sdk/packages/core/src/services/connectors/connector-cleanup.test.ts b/sdk/packages/core/src/services/connectors/connector-cleanup.test.ts new file mode 100644 index 0000000000..47618fd724 --- /dev/null +++ b/sdk/packages/core/src/services/connectors/connector-cleanup.test.ts @@ -0,0 +1,128 @@ +import { EventEmitter } from "node:events"; +import type { ConnectorCliLaunchSpec } from "@cline/shared"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanupConnectorInstanceViaCli } from "./connector-cleanup"; + +const spec: ConnectorCliLaunchSpec = { + launcher: "/usr/local/bin/bun", + connectArgsPrefix: ["/repo/apps/cli/src/index.ts", "connect"], + cwd: "/workspace", +}; + +class FakeChild extends EventEmitter { + stderr = new EventEmitter() as EventEmitter & { + setEncoding: (encoding: string) => void; + }; + kill = vi.fn(); + + constructor() { + super(); + this.stderr.setEncoding = vi.fn(); + } +} + +describe("cleanupConnectorInstanceViaCli", () => { + afterEach(() => { + vi.clearAllMocks(); + delete process.env.CLINE_SLACK_CONNECT_CHILD; + delete process.env.CLINE_RUN_AS_HUB_DAEMON; + }); + + it("invokes the CLI cleanup path with flags before the channel", async () => { + const child = new FakeChild(); + const spawnProcess = vi.fn(() => child); + + const pending = cleanupConnectorInstanceViaCli("slack", "cline-slack", { + launchSpec: spec, + spawnProcess: spawnProcess as never, + }); + child.emit("close", 0); + await expect(pending).resolves.toBeUndefined(); + + expect(spawnProcess).toHaveBeenCalledWith( + "/usr/local/bin/bun", + [ + "/repo/apps/cli/src/index.ts", + "connect", + // `connect` uses passThroughOptions, so a flag after the channel would + // be handed to the adapter instead of the connect command. + "--cleanup-instance", + "cline-slack", + "slack", + ], + expect.objectContaining({ cwd: "/workspace" }), + ); + }); + + it("strips inherited daemon and connector-child markers", async () => { + process.env.CLINE_RUN_AS_HUB_DAEMON = "1"; + process.env.CLINE_SLACK_CONNECT_CHILD = "1"; + const child = new FakeChild(); + let env: NodeJS.ProcessEnv = {}; + const spawnProcess = vi.fn( + ( + _launcher: string, + _args: string[], + options: { env: NodeJS.ProcessEnv }, + ) => { + env = options.env; + return child; + }, + ); + + const pending = cleanupConnectorInstanceViaCli("slack", "cline-slack", { + launchSpec: spec, + spawnProcess: spawnProcess as never, + }); + child.emit("close", 0); + await pending; + + expect(env.CLINE_RUN_AS_HUB_DAEMON).toBeUndefined(); + expect(env.CLINE_SLACK_CONNECT_CHILD).toBeUndefined(); + }); + + it("reports a failing cleanup with the CLI's stderr", async () => { + const child = new FakeChild(); + const spawnProcess = vi.fn(() => child); + + const pending = cleanupConnectorInstanceViaCli("slack", "cline-slack", { + launchSpec: spec, + spawnProcess: spawnProcess as never, + }); + child.stderr.emit("data", "unknown instance"); + child.emit("close", 1); + + await expect(pending).rejects.toThrow( + "cleanup exited with code 1: unknown instance", + ); + }); + + it("fails fast when no launch specification is available", async () => { + await expect( + cleanupConnectorInstanceViaCli("slack", "cline-slack", { + launchSpec: undefined, + }), + ).rejects.toThrow("connector CLI launch information is unavailable"); + }); + + it("kills and reports a cleanup that hangs", async () => { + vi.useFakeTimers(); + try { + const child = new FakeChild(); + const pending = cleanupConnectorInstanceViaCli("slack", "cline-slack", { + launchSpec: spec, + spawnProcess: (() => child) as never, + timeoutMs: 50, + }); + const assertion = expect(pending).rejects.toThrow( + "cleanup timed out after 50ms", + ); + await vi.advanceTimersByTimeAsync(60); + await assertion; + // A wedged cleanup must not block the supervisor's restart path forever. + expect(child.kill).toHaveBeenCalledWith("SIGKILL"); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/sdk/packages/core/src/services/connectors/connector-cleanup.ts b/sdk/packages/core/src/services/connectors/connector-cleanup.ts new file mode 100644 index 0000000000..36691c3c71 --- /dev/null +++ b/sdk/packages/core/src/services/connectors/connector-cleanup.ts @@ -0,0 +1,115 @@ +import { spawn } from "node:child_process"; +import { + type ConnectorCliLaunchSpec, + readConnectorCliLaunchSpec, +} from "@cline/shared"; +import { buildConnectorChildEnv } from "./connector-child-env"; + +const CLEANUP_TIMEOUT_MS = 15_000; + +export interface CleanupConnectorInstanceOptions { + launchSpec?: ConnectorCliLaunchSpec | undefined; + spawnProcess?: typeof spawn; + timeoutMs?: number; +} + +/** + * Reap one dead connector instance through the CLI. + * + * What has to be cleaned up — the process state file, thread→session bindings, + * and the hub sessions the connector owned — is all connector-specific knowledge + * that lives in the CLI's adapters. Rather than duplicate those conventions in + * the hub, the supervisor shells back into the CLI's own stop path, which + * already does exactly this for a process that is no longer running. + * + * `--cleanup-instance` deliberately preserves the autostart record: the instance + * died, it was not retired, so the supervisor still intends to restart it. + */ +export async function cleanupConnectorInstanceViaCli( + channel: string, + instanceId: string, + options: CleanupConnectorInstanceOptions = {}, +): Promise { + const spec = + "launchSpec" in options ? options.launchSpec : readConnectorCliLaunchSpec(); + if (!spec) { + throw new Error("connector CLI launch information is unavailable"); + } + const spawnProcess = options.spawnProcess ?? spawn; + const timeoutMs = options.timeoutMs ?? CLEANUP_TIMEOUT_MS; + + await new Promise((resolve, reject) => { + let settled = false; + let timer: ReturnType | undefined; + const finish = (error?: Error) => { + if (settled) { + return; + } + settled = true; + if (timer) { + clearTimeout(timer); + } + if (error) { + reject(error); + } else { + resolve(); + } + }; + + let child: ReturnType; + try { + child = spawnProcess( + spec.launcher, + [ + ...spec.connectArgsPrefix, + // Flags must precede the channel: `connect` uses passThroughOptions, + // so anything after the channel name is handed to the adapter. + "--cleanup-instance", + instanceId, + channel, + ], + { + cwd: spec.cwd, + env: buildConnectorChildEnv(), + stdio: ["ignore", "ignore", "pipe"], + windowsHide: true, + }, + ); + } catch (error) { + finish(error instanceof Error ? error : new Error(String(error))); + return; + } + + let stderr = ""; + child.stderr?.setEncoding("utf8"); + child.stderr?.on("data", (chunk: unknown) => { + stderr += String(chunk); + }); + timer = setTimeout(() => { + // A hung cleanup must not wedge the supervisor's restart path. + try { + child.kill("SIGKILL"); + } catch { + // Nothing further to do; the timeout is reported either way. + } + finish(new Error(`cleanup timed out after ${timeoutMs}ms`)); + }, timeoutMs); + timer.unref?.(); + child.once("error", (error: unknown) => { + finish(error instanceof Error ? error : new Error(String(error))); + }); + child.once("close", (code: number | null) => { + if (code === 0 || code === null) { + finish(); + return; + } + finish( + new Error( + `cleanup exited with code ${code}${ + stderr.trim() ? `: ${stderr.trim()}` : "" + }`, + ), + ); + }); + }); +} diff --git a/sdk/packages/core/src/services/connectors/connector-supervisor.test.ts b/sdk/packages/core/src/services/connectors/connector-supervisor.test.ts new file mode 100644 index 0000000000..d7c6cda86f --- /dev/null +++ b/sdk/packages/core/src/services/connectors/connector-supervisor.test.ts @@ -0,0 +1,722 @@ +import { EventEmitter } from "node:events"; +import type { ConnectorCliLaunchSpec } from "@cline/shared"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + ConnectorSupervisor, + getActiveConnectorSupervisor, + RESTART_BASE_DELAY_MS, + RESTART_GIVE_UP_AFTER, + RESTART_MAX_DELAY_MS, + setActiveConnectorSupervisor, + STOP_SIGTERM_TIMEOUT_MS, +} from "./connector-supervisor"; + +const mocks = vi.hoisted(() => ({ + disableConnectorAutostart: vi.fn(), + getPersistedConnectorConnection: vi.fn(), +})); + +vi.mock("./connector-autostart", () => ({ + disableConnectorAutostart: mocks.disableConnectorAutostart, + getPersistedConnectorConnection: mocks.getPersistedConnectorConnection, +})); + +const spec: ConnectorCliLaunchSpec = { + launcher: "/usr/local/bin/bun", + connectArgsPrefix: ["/repo/apps/cli/src/index.ts", "connect"], + cwd: "/workspace", +}; + +class FakeChild extends EventEmitter { + unref = vi.fn(); + constructor(public pid: number | undefined = 4242) { + super(); + } + exit(code: number | null, signal: string | null = null): void { + this.emit("exit", code, signal); + } +} + +/** + * Deterministic clock + timer queue: restart backoff is the behaviour under + * test, so nothing here may depend on real time. + */ +function createHarness( + overrides: Partial<{ + children: FakeChild[]; + autostartEnabled: boolean; + launchSpec: ConnectorCliLaunchSpec | undefined; + active: Array<{ type: string; instanceId: string; pid: number }>; + killProcess: ( + pid: number, + signal: NodeJS.Signals, + alivePids: Set, + ) => void; + }> = {}, +) { + let now = 1_000_000; + const timers: Array<{ id: number; at: number; callback: () => void }> = []; + let nextTimerId = 1; + const spawned: Array<{ + launcher: string; + args: string[]; + options: { cwd: string; env: NodeJS.ProcessEnv; detached: boolean }; + }> = []; + const children = overrides.children ?? []; + let childIndex = 0; + const alivePids = new Set(); + const cleanups: string[] = []; + const logs: string[] = []; + const kills: string[] = []; + + const spawnProcess = vi.fn( + (launcher: string, args: string[], options: Record) => { + spawned.push( + options as unknown as (typeof spawned)[number] extends infer T + ? T + : never, + ); + spawned[spawned.length - 1] = { + launcher, + args, + options: options as unknown as { + cwd: string; + env: NodeJS.ProcessEnv; + detached: boolean; + }, + }; + const child = children[childIndex++] ?? new FakeChild(5000 + childIndex); + if (child.pid !== undefined) { + alivePids.add(child.pid); + child.once("exit", () => { + if (child.pid !== undefined) { + alivePids.delete(child.pid); + } + }); + } + return child as never; + }, + ); + + const supervisor = new ConnectorSupervisor({ + launchSpec: () => ("launchSpec" in overrides ? overrides.launchSpec : spec), + spawnProcess: spawnProcess as never, + isProcessRunning: (pid) => alivePids.has(pid), + // Injected so tests never signal real pids; the default drops the pid so + // a stop's wait-for-exit resolves the way a real SIGTERM would. + killProcess: (pid, signal) => { + kills.push(`${pid}:${signal}`); + if (overrides.killProcess) { + overrides.killProcess(pid, signal, alivePids); + return; + } + alivePids.delete(pid); + }, + listActive: () => (overrides.active ?? []) as never, + cleanupInstance: async (channel, instanceId) => { + cleanups.push(`${channel}:${instanceId}`); + }, + isAutostartEnabled: () => overrides.autostartEnabled ?? true, + log: (message) => logs.push(message), + now: () => now, + setTimer: (callback, delayMs) => { + const id = nextTimerId++; + timers.push({ id, at: now + delayMs, callback }); + return id; + }, + clearTimer: (handle) => { + const index = timers.findIndex((timer) => timer.id === handle); + if (index >= 0) { + timers.splice(index, 1); + } + }, + }); + + return { + supervisor, + spawned, + cleanups, + logs, + kills, + alivePids, + spawnProcess, + advance(ms: number) { + now += ms; + const due = timers.filter((timer) => timer.at <= now); + for (const timer of due) { + const index = timers.indexOf(timer); + if (index >= 0) { + timers.splice(index, 1); + } + timer.callback(); + } + }, + setNow(value: number) { + now = value; + }, + pendingTimers: () => timers.length, + nextDelay: () => (timers[0] ? timers[0].at - now : undefined), + }; +} + +async function flush(): Promise { + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setImmediate(resolve)); +} + +describe("ConnectorSupervisor", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getPersistedConnectorConnection.mockReturnValue(undefined); + }); + + it("spawns a connector detached and reports it as running", async () => { + const harness = createHarness(); + + const result = await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + expect(result.started).toBe(true); + expect(result.record.state).toBe("running"); + expect(result.record.origin).toBe("spawned"); + expect(harness.spawned[0]?.launcher).toBe("/usr/local/bin/bun"); + expect(harness.spawned[0]?.args).toEqual([ + "/repo/apps/cli/src/index.ts", + "connect", + "slack", + "--bot-token", + "xoxb", + ]); + // Detached: a hub restart must not take the connector down with it. + expect(harness.spawned[0]?.options.detached).toBe(true); + }); + + it("refuses a second instance of a live connector", async () => { + const harness = createHarness(); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + const second = await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + expect(second.started).toBe(false); + expect(second.reason).toBe("already_running"); + // The whole point: one process per instance, so one socket per token. + expect(harness.spawnProcess).toHaveBeenCalledTimes(1); + }); + + it("replaces a live instance when restart is requested", async () => { + const harness = createHarness(); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "old"], + }); + + const restarted = await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "new"], + restart: true, + }); + + expect(restarted.started).toBe(true); + expect(harness.spawnProcess).toHaveBeenCalledTimes(2); + expect(harness.spawned[1]?.args).toContain("new"); + // A restart must not disable the autostart record. + expect(mocks.disableConnectorAutostart).not.toHaveBeenCalled(); + }); + + it("strips daemon and connector-child markers from the child environment", async () => { + process.env.CLINE_RUN_AS_HUB_DAEMON = "1"; + process.env.CLINE_SLACK_CONNECT_CHILD = "1"; + process.env.CLINE_CONNECTOR_DETACHED_CHILD = "1"; + process.env.CLINE_CONNECTOR_STARTING_INSTANCE = '{"channel":"slack"}'; + try { + const harness = createHarness(); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + const env = harness.spawned[0]?.options.env ?? {}; + expect(env.CLINE_RUN_AS_HUB_DAEMON).toBeUndefined(); + expect(env.CLINE_SLACK_CONNECT_CHILD).toBeUndefined(); + expect(env.CLINE_CONNECTOR_DETACHED_CHILD).toBeUndefined(); + expect(env.CLINE_CONNECTOR_STARTING_INSTANCE).toBeUndefined(); + expect(env.PATH).toBe(process.env.PATH); + } finally { + delete process.env.CLINE_RUN_AS_HUB_DAEMON; + delete process.env.CLINE_SLACK_CONNECT_CHILD; + delete process.env.CLINE_CONNECTOR_DETACHED_CHILD; + delete process.env.CLINE_CONNECTOR_STARTING_INSTANCE; + } + }); + + it("reaps a dead connector and restarts it with exponential backoff", async () => { + const first = new FakeChild(101); + const second = new FakeChild(102); + const harness = createHarness({ children: [first, second] }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + first.exit(1); + await flush(); + + // Reaping is what keeps Slack threads from pointing at dead sessions. + expect(harness.cleanups).toEqual(["slack:cline-slack"]); + expect(harness.supervisor.list()[0]?.state).toBe("backoff"); + expect(harness.nextDelay()).toBe(RESTART_BASE_DELAY_MS); + + harness.advance(RESTART_BASE_DELAY_MS); + await flush(); + + expect(harness.spawnProcess).toHaveBeenCalledTimes(2); + const record = harness.supervisor.list()[0]; + expect(record?.state).toBe("running"); + expect(record?.restarts).toBe(1); + }); + + it("backs off further on each successive crash and caps the delay", async () => { + const children = Array.from( + { length: 4 }, + (_, i) => new FakeChild(200 + i), + ); + const harness = createHarness({ children }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + const delays: Array = []; + for (let attempt = 0; attempt < 3; attempt += 1) { + children[attempt]?.exit(1); + await flush(); + delays.push(harness.nextDelay()); + harness.advance(harness.nextDelay() ?? 0); + await flush(); + } + + expect(delays).toEqual([ + RESTART_BASE_DELAY_MS, + RESTART_BASE_DELAY_MS * 2, + RESTART_BASE_DELAY_MS * 4, + ]); + expect(delays.every((delay) => (delay ?? 0) <= RESTART_MAX_DELAY_MS)).toBe( + true, + ); + }); + + it("gives up after too many consecutive restarts", async () => { + const children = Array.from( + { length: RESTART_GIVE_UP_AFTER + 2 }, + (_, i) => new FakeChild(300 + i), + ); + const harness = createHarness({ children }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + for (let attempt = 0; attempt <= RESTART_GIVE_UP_AFTER; attempt += 1) { + children[attempt]?.exit(1); + await flush(); + harness.advance(harness.nextDelay() ?? 0); + await flush(); + } + + const record = harness.supervisor.list()[0]; + expect(record?.state).toBe("failed"); + expect(record?.restarts).toBe(RESTART_GIVE_UP_AFTER); + // A revoked token must not become an endless spawn loop. + expect(harness.pendingTimers()).toBe(0); + expect(harness.logs.some((line) => line.includes("giving up"))).toBe(true); + }); + + it("clears the restart counter after a run that stayed up", async () => { + const children = Array.from( + { length: 3 }, + (_, i) => new FakeChild(400 + i), + ); + const harness = createHarness({ children }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + children[0]?.exit(1); + await flush(); + harness.advance(RESTART_BASE_DELAY_MS); + await flush(); + expect(harness.supervisor.list()[0]?.restarts).toBe(1); + + // Second run stays healthy for well over the reset window before dying. + harness.advance(10 * 60_000); + children[1]?.exit(1); + await flush(); + + expect(harness.nextDelay()).toBe(RESTART_BASE_DELAY_MS); + expect(harness.supervisor.list()[0]?.restarts).toBe(1); + }); + + it("cancels a pending backoff restart when a new start replaces the entry", async () => { + // A user runs `cline connect` while the instance is waiting out its + // backoff. The old entry's timer must not survive the replacement: it + // holds a closure over the old entry, so firing it would spawn a second + // process for the same instance — untracked by the map, so invisible to + // list() and unreachable by stop() — two processes on one bot token. + const children = [ + new FakeChild(1300), + new FakeChild(1301), + new FakeChild(1302), + ]; + const harness = createHarness({ children }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + children[0]?.exit(1); + await flush(); + expect(harness.supervisor.list()[0]?.state).toBe("backoff"); + expect(harness.pendingTimers()).toBe(1); + + const second = await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + expect(second.started).toBe(true); + expect(harness.pendingTimers()).toBe(0); + + // Even well past every possible backoff, nothing else may spawn. + harness.advance(RESTART_MAX_DELAY_MS); + await flush(); + expect(harness.spawnProcess).toHaveBeenCalledTimes(2); + expect(harness.supervisor.list()).toHaveLength(1); + expect(harness.supervisor.list()[0]?.state).toBe("running"); + }); + + it("serialises a boot-time restart with a concurrent user start", async () => { + // Observed live: a new hub's boot reconnect restarts an adopted survivor + // — which suspends inside stop() waiting on the CLI cleanup — while a + // user `cline connect` for the same instance arrives over the hub. + // Unserialised, both spawned: the map tracked one process while the other + // lived on untracked, holding the connector's webhook port, and the + // tracked chain crash-looped on EADDRINUSE until it gave up. + mocks.getPersistedConnectorConnection.mockReturnValue({ + channel: "slack", + instanceId: "cline-slack", + connectArgs: ["--bot-token", "stored"], + lastSuccessfulArgs: [], + enabled: true, + updatedAt: "", + lastConnectedAt: "", + }); + const harness = createHarness({ + active: [{ type: "slack", instanceId: "cline-slack", pid: 700 }], + }); + harness.alivePids.add(700); + harness.supervisor.adoptRunningConnectors(); + + const [restarted, userStart] = await Promise.all([ + harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "stored"], + restart: true, + }), + harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "stored"], + }), + ]); + + expect(restarted.started).toBe(true); + expect(userStart.started).toBe(false); + expect(userStart.reason).toBe("already_running"); + // The survivor was actually stopped, exactly one replacement exists, and + // the supervisor tracks it. + expect(harness.kills).toContain("700:SIGTERM"); + expect(harness.spawnProcess).toHaveBeenCalledTimes(1); + expect(harness.supervisor.list()).toHaveLength(1); + expect(harness.supervisor.list()[0]?.state).toBe("running"); + }); + + it("waits for a stopped process to die, escalating to SIGKILL", async () => { + // A replacement spawned while the old process still holds the + // connector's listen port fails on EADDRINUSE, so stop must not return + // until the process is actually gone. + const child = new FakeChild(800); + const harness = createHarness({ + children: [child], + killProcess: (pid, signal, alivePids) => { + // This process ignores SIGTERM and lingers on its port. + if (signal === "SIGKILL") { + alivePids.delete(pid); + } + }, + }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + let resolved = false; + const pending = harness.supervisor + .stop({ channel: "slack", instanceId: "cline-slack" }) + .then((stopped) => { + resolved = true; + return stopped; + }); + + // Drive the exit poll past the SIGTERM window. + const iterations = STOP_SIGTERM_TIMEOUT_MS / 100 + 5; + for (let step = 0; step < iterations && !resolved; step += 1) { + await flush(); + harness.advance(100); + } + await flush(); + + await expect(pending).resolves.toBe(true); + expect(harness.kills).toContain("800:SIGTERM"); + expect(harness.kills).toContain("800:SIGKILL"); + expect(harness.alivePids.has(800)).toBe(false); + expect(harness.supervisor.list()).toEqual([]); + }); + + it("does not schedule a restart for an entry replaced while its cleanup was in flight", async () => { + // Between a crash and its restart timer there is a window where the + // exit-cleanup chain is still running and no timer exists yet to cancel. + // A replacement made in that window must make the chain stand down + // instead of scheduling a restart for the retired entry. + const children = [new FakeChild(1400), new FakeChild(1401)]; + const harness = createHarness({ children }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + + children[0]?.exit(1); + // No flush: the cleanup chain has not completed when the start arrives. + const second = await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: ["--bot-token", "xoxb"], + }); + await flush(); + + expect(second.started).toBe(true); + expect(harness.pendingTimers()).toBe(0); + expect(harness.spawnProcess).toHaveBeenCalledTimes(2); + expect(harness.supervisor.list()).toHaveLength(1); + }); + + it("restarts a connector that was started with no arguments", async () => { + // A connector configured entirely through the connector store launches with + // an empty argv. Emptiness must not be mistaken for "argv unknown", or the + // hub can never bring it back. + mocks.getPersistedConnectorConnection.mockReturnValue(undefined); + const children = [new FakeChild(1100), new FakeChild(1101)]; + const harness = createHarness({ children }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + children[0]?.exit(1); + await flush(); + harness.advance(RESTART_BASE_DELAY_MS); + await flush(); + + expect(harness.spawnProcess).toHaveBeenCalledTimes(2); + expect(harness.supervisor.list()[0]?.state).toBe("running"); + expect(harness.spawned[1]?.args).toEqual([ + "/repo/apps/cli/src/index.ts", + "connect", + "slack", + ]); + }); + + it("does not restart a connector whose autostart is disabled", async () => { + const child = new FakeChild(500); + const harness = createHarness({ + children: [child], + autostartEnabled: false, + }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + child.exit(0); + await flush(); + + expect(harness.cleanups).toEqual(["slack:cline-slack"]); + expect(harness.pendingTimers()).toBe(0); + expect(harness.supervisor.list()).toEqual([]); + }); + + it("stops a connector on request without restarting it", async () => { + const child = new FakeChild(600); + const harness = createHarness({ children: [child] }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + const stopped = await harness.supervisor.stop({ + channel: "slack", + instanceId: "cline-slack", + }); + child.exit(null, "SIGTERM"); + await flush(); + + expect(stopped).toBe(true); + expect(mocks.disableConnectorAutostart).toHaveBeenCalledWith( + "slack", + "cline-slack", + ); + expect(harness.supervisor.list()).toEqual([]); + expect(harness.pendingTimers()).toBe(0); + }); + + it("adopts connectors that predate this hub and polls them for death", async () => { + mocks.getPersistedConnectorConnection.mockReturnValue({ + channel: "slack", + instanceId: "cline-slack", + connectArgs: ["--bot-token", "stored"], + lastSuccessfulArgs: [], + enabled: true, + updatedAt: "", + lastConnectedAt: "", + }); + const harness = createHarness({ + active: [{ type: "slack", instanceId: "cline-slack", pid: 700 }], + }); + harness.alivePids.add(700); + + const adopted = harness.supervisor.adoptRunningConnectors(); + expect(adopted).toHaveLength(1); + expect(adopted[0]?.origin).toBe("adopted"); + expect(adopted[0]?.pid).toBe(700); + + // Adopted processes have no child handle, so death is only visible by poll. + harness.alivePids.delete(700); + harness.advance(10_000); + await flush(); + + expect(harness.cleanups).toEqual(["slack:cline-slack"]); + harness.advance(RESTART_BASE_DELAY_MS); + await flush(); + + // Restart args come from the persisted record: an adopted process's argv + // is not ours to reconstruct. + expect(harness.spawned[0]?.args).toEqual([ + "/repo/apps/cli/src/index.ts", + "connect", + "slack", + "--bot-token", + "stored", + ]); + }); + + it("does not adopt an instance it already supervises", async () => { + const harness = createHarness({ + active: [{ type: "slack", instanceId: "cline-slack", pid: 800 }], + }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + expect(harness.supervisor.adoptRunningConnectors()).toEqual([]); + expect(harness.supervisor.list()).toHaveLength(1); + }); + + it("fails a start when no launch specification is available", async () => { + const harness = createHarness({ launchSpec: undefined }); + + const result = await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + expect(result.started).toBe(false); + expect(result.record.state).toBe("failed"); + expect(harness.spawnProcess).not.toHaveBeenCalled(); + }); + + it("cannot restart an adopted connector with no stored arguments", async () => { + mocks.getPersistedConnectorConnection.mockReturnValue(undefined); + const harness = createHarness({ + active: [{ type: "slack", instanceId: "cline-slack", pid: 900 }], + }); + harness.alivePids.add(900); + harness.supervisor.adoptRunningConnectors(); + + harness.alivePids.delete(900); + harness.advance(10_000); + await flush(); + harness.advance(RESTART_BASE_DELAY_MS); + await flush(); + + expect(harness.spawnProcess).not.toHaveBeenCalled(); + expect(harness.supervisor.list()[0]?.state).toBe("failed"); + }); + + it("leaves running connectors alone when disposed so the next hub can adopt them", async () => { + const child = new FakeChild(1000); + const harness = createHarness({ children: [child] }); + await harness.supervisor.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + + harness.supervisor.dispose(); + + expect(harness.alivePids.has(1000)).toBe(true); + expect(harness.supervisor.list()).toEqual([]); + // A post-dispose exit must not trigger cleanup or a restart. + child.exit(1); + await flush(); + expect(harness.cleanups).toEqual([]); + expect(harness.pendingTimers()).toBe(0); + }); + + it("exposes the active supervisor to hub command handlers", () => { + const harness = createHarness(); + expect(getActiveConnectorSupervisor()).toBeUndefined(); + setActiveConnectorSupervisor(harness.supervisor); + expect(getActiveConnectorSupervisor()).toBe(harness.supervisor); + setActiveConnectorSupervisor(undefined); + expect(getActiveConnectorSupervisor()).toBeUndefined(); + }); +}); diff --git a/sdk/packages/core/src/services/connectors/connector-supervisor.ts b/sdk/packages/core/src/services/connectors/connector-supervisor.ts new file mode 100644 index 0000000000..3d51874c53 --- /dev/null +++ b/sdk/packages/core/src/services/connectors/connector-supervisor.ts @@ -0,0 +1,682 @@ +import { type ChildProcess, spawn } from "node:child_process"; +import { openSync } from "node:fs"; +import { + CLINE_CONNECTOR_SUPERVISED_ENV, + type ConnectorCliLaunchSpec, + type ConnectorStartRequest, + type ConnectorStartResult, + readConnectorCliLaunchSpec, + type SupervisedConnectorOrigin, + type SupervisedConnectorRecord, + type SupervisedConnectorState, +} from "@cline/shared"; +import { + ensureParentDir, + resolveConnectorLogPath, +} from "@cline/shared/storage"; +import { listActiveConnectors } from "./active-connectors"; +import { + disableConnectorAutostart, + getPersistedConnectorConnection, +} from "./connector-autostart"; +import { buildConnectorChildEnv } from "./connector-child-env"; + +export const RESTART_BASE_DELAY_MS = 1_000; +export const RESTART_MAX_DELAY_MS = 60_000; +/** Consecutive failed restarts before the hub stops trying. */ +export const RESTART_GIVE_UP_AFTER = 5; +/** + * A run that lasts this long is treated as healthy, clearing the restart + * counter. Without it a connector that stays up for hours and then dies would + * inherit stale failures and be given up on immediately. + */ +export const RESTART_COUNTER_RESET_MS = 60_000; +/** How often adopted connectors (no child handle) are checked for liveness. */ +export const ADOPTED_POLL_INTERVAL_MS = 5_000; +/** How long a stop waits for SIGTERM to land before escalating to SIGKILL. */ +export const STOP_SIGTERM_TIMEOUT_MS = 5_000; +/** How long a stop waits for SIGKILL to land before giving up on the wait. */ +export const STOP_SIGKILL_TIMEOUT_MS = 2_000; +const EXIT_POLL_INTERVAL_MS = 100; + +export interface ConnectorSupervisorDeps { + launchSpec?: () => ConnectorCliLaunchSpec | undefined; + spawnProcess?: typeof spawn; + isProcessRunning?: (pid: number) => boolean; + killProcess?: (pid: number, signal: NodeJS.Signals) => void; + listActive?: typeof listActiveConnectors; + /** + * Reap a dead instance's leftovers: state file, thread bindings and hub + * sessions. Delegated because all of that is connector-specific and owned by + * the CLI; the supervisor only knows a process died. + */ + cleanupInstance?: (channel: string, instanceId: string) => Promise; + isAutostartEnabled?: (channel: string, instanceId: string) => boolean; + log?: (message: string) => void; + now?: () => number; + setTimer?: (callback: () => void, delayMs: number) => unknown; + clearTimer?: (handle: unknown) => void; +} + +interface SupervisedEntry { + channel: string; + instanceId: string; + args: string[]; + /** + * Whether `args` is this instance's real argv. An adopted process's argv is + * not ours to reconstruct, and an empty argv is legitimate for a connector + * configured entirely through the connector store — so emptiness cannot + * stand in for "unknown". + */ + argsKnown: boolean; + origin: SupervisedConnectorOrigin; + state: SupervisedConnectorState; + pid?: number; + child?: ChildProcess; + startedAt?: number; + restarts: number; + nextRestartAt?: number; + restartTimer?: unknown; + lastExitCode?: number; + lastExitSignal?: string; + lastError?: string; +} + +function instanceKey(channel: string, instanceId: string): string { + return `${channel}\u0000${instanceId}`; +} + +function defaultIsProcessRunning(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function toIsoString(value: number | undefined): string | undefined { + return value === undefined ? undefined : new Date(value).toISOString(); +} + +/** + * Owns the lifecycle of connector processes on behalf of the hub. + * + * Three responsibilities, in order of why this exists: + * + * 1. **Single instance per (channel, instanceId).** The in-memory map is the + * authority, so two connectors can never hold the same bot token because two + * processes raced on a state file. + * 2. **Reaping.** When a connector dies its state file, thread bindings and hub + * sessions would otherwise linger until the next manual start, leaving + * Slack threads bound to sessions that no longer exist. + * 3. **Restart with backoff.** Replaces external watchdogs, and refuses to + * spin forever on a connector that cannot start (a revoked token). + * + * Connectors are spawned detached so a hub restart does not take them down; a + * later hub adopts the survivors by pid from their state files. That is why exit + * detection has two paths: child events for processes this hub spawned, pid + * polling for adopted ones. + */ +export class ConnectorSupervisor { + private readonly entries = new Map(); + /** + * Tail of the in-flight start/stop chain per instance key. `start` suspends + * on `stop` (which shells into the CLI for cleanup, taking seconds), and two + * unserialised starts interleaving across that suspension each spawn their + * own process — the map ends up tracking one while the other survives as an + * untracked ghost holding the connector's credentials and ports. + */ + private readonly instanceLocks = new Map>(); + private pollTimer: unknown; + private disposed = false; + + private readonly launchSpec: () => ConnectorCliLaunchSpec | undefined; + private readonly spawnProcess: typeof spawn; + private readonly isProcessRunning: (pid: number) => boolean; + private readonly killProcess: (pid: number, signal: NodeJS.Signals) => void; + private readonly listActive: typeof listActiveConnectors; + private readonly cleanupInstance?: ( + channel: string, + instanceId: string, + ) => Promise; + private readonly isAutostartEnabled: ( + channel: string, + instanceId: string, + ) => boolean; + private readonly log: (message: string) => void; + private readonly now: () => number; + private readonly setTimer: (callback: () => void, delayMs: number) => unknown; + private readonly clearTimer: (handle: unknown) => void; + + constructor(deps: ConnectorSupervisorDeps = {}) { + this.launchSpec = deps.launchSpec ?? readConnectorCliLaunchSpec; + this.spawnProcess = deps.spawnProcess ?? spawn; + this.isProcessRunning = deps.isProcessRunning ?? defaultIsProcessRunning; + this.killProcess = + deps.killProcess ?? ((pid, signal) => process.kill(pid, signal)); + this.listActive = deps.listActive ?? listActiveConnectors; + this.cleanupInstance = deps.cleanupInstance; + this.isAutostartEnabled = + deps.isAutostartEnabled ?? + ((channel, instanceId) => + getPersistedConnectorConnection(channel, instanceId)?.enabled === true); + this.log = + deps.log ?? + ((message) => process.stderr.write(`[hub-daemon] ${message}\n`)); + this.now = deps.now ?? (() => Date.now()); + this.setTimer = + deps.setTimer ?? + ((callback, delayMs) => { + const timer = setTimeout(callback, delayMs); + // Never let a pending restart hold the process open. + timer.unref?.(); + return timer; + }); + this.clearTimer = + deps.clearTimer ?? + ((handle) => clearTimeout(handle as ReturnType)); + } + + /** + * Take over connectors that are already running, so a replacement hub reaps + * and restarts the processes it inherited instead of ignoring them. + */ + adoptRunningConnectors(): SupervisedConnectorRecord[] { + const adopted: SupervisedConnectorRecord[] = []; + for (const record of this.listActive()) { + const key = instanceKey(record.type, record.instanceId); + if (this.entries.has(key)) { + continue; + } + const entry: SupervisedEntry = { + channel: record.type, + instanceId: record.instanceId, + // Reconstructed from the autostart record on the first restart. + args: [], + argsKnown: false, + origin: "adopted", + state: "running", + pid: record.pid, + startedAt: record.startedAt ? Date.parse(record.startedAt) : this.now(), + restarts: 0, + }; + this.entries.set(key, entry); + adopted.push(this.toRecord(entry)); + this.log( + `[connect] adopted running ${record.type} connector ${record.instanceId} pid=${record.pid}`, + ); + } + if (adopted.length > 0) { + this.ensurePolling(); + } + return adopted; + } + + /** + * Serialise start/stop work per instance key. Both operations suspend + * mid-flight — a stop waits for the process to die and for the CLI cleanup, + * a start may embed a stop — and interleaving two of them across those + * suspensions is how the map ends up tracking one process while another + * lives on untracked. One instance, one queue. + */ + private withInstanceLock(key: string, task: () => Promise): Promise { + const previous = this.instanceLocks.get(key) ?? Promise.resolve(); + const run = previous.then(task, task); + const tail = run.then( + () => undefined, + () => undefined, + ); + this.instanceLocks.set(key, tail); + void tail.then(() => { + if (this.instanceLocks.get(key) === tail) { + this.instanceLocks.delete(key); + } + }); + return run; + } + + async start(request: ConnectorStartRequest): Promise { + return this.withInstanceLock( + instanceKey(request.channel, request.instanceId), + () => this.startLocked(request), + ); + } + + private async startLocked( + request: ConnectorStartRequest, + ): Promise { + const { channel, instanceId } = request; + const key = instanceKey(channel, instanceId); + const existing = this.entries.get(key); + if (existing && this.isEntryAlive(existing)) { + if (!request.restart) { + return { + started: false, + reason: "already_running", + record: this.toRecord(existing), + }; + } + await this.stopLocked({ channel, instanceId, disableAutostart: false }); + } else if (existing) { + // A dead entry can still act: a pending backoff timer would spawn a + // second process for this instance once it fires — untracked, so + // unstoppable — and an exit-cleanup chain still in flight would + // schedule that timer after this replacement is made. Retire the old + // entry explicitly; both paths check for "stopped" and stand down. + this.cancelRestart(existing); + existing.state = "stopped"; + existing.child?.removeAllListeners("exit"); + } + const entry: SupervisedEntry = { + channel, + instanceId, + args: [...request.args], + argsKnown: true, + origin: "spawned", + state: "running", + restarts: existing?.restarts ?? 0, + }; + this.entries.set(key, entry); + this.spawnEntry(entry); + if (entry.state !== "running") { + return { started: false, record: this.toRecord(entry) }; + } + return { started: true, record: this.toRecord(entry) }; + } + + async stop(request: { + channel: string; + instanceId: string; + disableAutostart?: boolean; + }): Promise { + return this.withInstanceLock( + instanceKey(request.channel, request.instanceId), + () => this.stopLocked(request), + ); + } + + private async stopLocked(request: { + channel: string; + instanceId: string; + disableAutostart?: boolean; + }): Promise { + const key = instanceKey(request.channel, request.instanceId); + const entry = this.entries.get(key); + if (request.disableAutostart !== false) { + disableConnectorAutostart(request.channel, request.instanceId); + } + if (!entry) { + return false; + } + this.cancelRestart(entry); + // Mark before signalling so the exit handler does not restart it. + entry.state = "stopped"; + const pid = entry.child?.pid ?? entry.pid; + if (pid && this.isProcessRunning(pid)) { + this.signal(pid, "SIGTERM"); + // Wait for the process to actually die. A replacement spawned while + // the old process still holds the connector's listen port or socket + // fails on it — observed as an EADDRINUSE crash loop when a webhook + // connector was restarted for a new hub session. + if (!(await this.waitForProcessExit(pid, STOP_SIGTERM_TIMEOUT_MS))) { + this.signal(pid, "SIGKILL"); + await this.waitForProcessExit(pid, STOP_SIGKILL_TIMEOUT_MS); + } + } + await this.runCleanup(entry); + this.entries.delete(key); + return true; + } + + private signal(pid: number, signal: NodeJS.Signals): void { + try { + this.killProcess(pid, signal); + } catch { + // Already gone, or not ours to signal. + } + } + + private async waitForProcessExit( + pid: number, + timeoutMs: number, + ): Promise { + const deadline = this.now() + timeoutMs; + while (this.isProcessRunning(pid)) { + if (this.now() >= deadline) { + return false; + } + await new Promise((resolve) => { + this.setTimer(resolve, EXIT_POLL_INTERVAL_MS); + }); + } + return true; + } + + list(): SupervisedConnectorRecord[] { + return [...this.entries.values()] + .map((entry) => this.toRecord(entry)) + .sort( + (left, right) => + left.channel.localeCompare(right.channel) || + left.instanceId.localeCompare(right.instanceId), + ); + } + + /** + * Stop supervising without touching the processes: they are detached and + * outlive this hub on purpose, and the next hub adopts them. + */ + dispose(): void { + this.disposed = true; + for (const entry of this.entries.values()) { + this.cancelRestart(entry); + entry.child?.removeAllListeners("exit"); + } + if (this.pollTimer !== undefined) { + this.clearTimer(this.pollTimer); + this.pollTimer = undefined; + } + this.entries.clear(); + this.instanceLocks.clear(); + } + + private spawnEntry(entry: SupervisedEntry): void { + const spec = this.launchSpec(); + if (!spec) { + entry.state = "failed"; + entry.lastError = "connector CLI launch information is unavailable"; + this.log( + `[connect] cannot start ${entry.channel} connector ${entry.instanceId}: ${entry.lastError}`, + ); + return; + } + const logPath = resolveConnectorLogPath(entry.channel, entry.instanceId); + let stdio: ["ignore", "ignore" | number, "ignore" | number] = [ + "ignore", + "ignore", + "ignore", + ]; + try { + ensureParentDir(logPath); + const fd = openSync(logPath, "a"); + stdio = ["ignore", fd, fd]; + } catch { + // Without a log the connector still runs; only diagnostics are lost. + } + try { + const child = this.spawnProcess( + spec.launcher, + [...spec.connectArgsPrefix, entry.channel, ...entry.args], + { + cwd: spec.cwd, + env: { + ...buildConnectorChildEnv(), + // Tells the connector to run in this process rather than asking + // the hub to start it (which would loop back here) or spawning + // its own detached child and exiting (which would leave us + // holding a handle to a process that is already gone). + [CLINE_CONNECTOR_SUPERVISED_ENV]: "1", + }, + // Detached so it survives this hub, but not unref'd from our + // listener: we still want the exit event while we are alive. + detached: true, + stdio, + windowsHide: true, + }, + ); + entry.child = child; + entry.pid = child.pid ?? undefined; + entry.startedAt = this.now(); + entry.state = "running"; + entry.lastError = undefined; + child.unref?.(); + child.once("error", (error: unknown) => { + entry.lastError = + error instanceof Error ? error.message : String(error); + this.handleExit(entry, undefined, undefined); + }); + child.once("exit", (code: number | null, signal: string | null) => { + this.handleExit(entry, code ?? undefined, signal ?? undefined); + }); + this.log( + `[connect] started ${entry.channel} connector ${entry.instanceId} pid=${entry.pid}`, + ); + } catch (error) { + entry.state = "backoff"; + entry.lastError = error instanceof Error ? error.message : String(error); + this.log( + `[connect] failed to spawn ${entry.channel} connector ${entry.instanceId}: ${entry.lastError}`, + ); + this.scheduleRestart(entry); + } + } + + private handleExit( + entry: SupervisedEntry, + code: number | undefined, + signal: string | undefined, + ): void { + if (this.disposed || entry.state === "stopped") { + return; + } + entry.child = undefined; + entry.pid = undefined; + entry.lastExitCode = code; + entry.lastExitSignal = signal; + const ranLongEnough = + entry.startedAt !== undefined && + this.now() - entry.startedAt >= RESTART_COUNTER_RESET_MS; + if (ranLongEnough) { + entry.restarts = 0; + } + this.log( + `[connect] ${entry.channel} connector ${entry.instanceId} exited` + + `${code === undefined ? "" : ` code=${code}`}` + + `${signal ? ` signal=${signal}` : ""}`, + ); + void this.runCleanup(entry).then(() => { + const key = instanceKey(entry.channel, entry.instanceId); + if ( + this.disposed || + entry.state === "stopped" || + // A concurrent start may have replaced this entry while cleanup was + // running; the retired generation must not restart or delete the + // replacement's map entry. + this.entries.get(key) !== entry + ) { + return; + } + if (!this.isAutostartEnabled(entry.channel, entry.instanceId)) { + this.log( + `[connect] not restarting ${entry.channel} connector ${entry.instanceId}: autostart is disabled`, + ); + this.entries.delete(key); + return; + } + this.scheduleRestart(entry); + }); + } + + private scheduleRestart(entry: SupervisedEntry): void { + if (entry.restarts >= RESTART_GIVE_UP_AFTER) { + entry.state = "failed"; + entry.nextRestartAt = undefined; + this.log( + `[connect] giving up on ${entry.channel} connector ${entry.instanceId} after ${entry.restarts} failed restarts` + + `${entry.lastError ? `: ${entry.lastError}` : ""}`, + ); + return; + } + const delayMs = Math.min( + RESTART_BASE_DELAY_MS * 2 ** entry.restarts, + RESTART_MAX_DELAY_MS, + ); + entry.restarts += 1; + entry.state = "backoff"; + entry.nextRestartAt = this.now() + delayMs; + this.log( + `[connect] restarting ${entry.channel} connector ${entry.instanceId} in ${delayMs}ms (attempt ${entry.restarts})`, + ); + const key = instanceKey(entry.channel, entry.instanceId); + entry.restartTimer = this.setTimer(() => { + entry.restartTimer = undefined; + entry.nextRestartAt = undefined; + // Under the instance lock: a restart spawn must not interleave with a + // start or stop in flight for the same instance. + void this.withInstanceLock(key, async () => { + if ( + this.disposed || + entry.state === "stopped" || + this.entries.get(key) !== entry + ) { + return; + } + const args = this.resolveRestartArgs(entry); + if (!args) { + entry.state = "failed"; + this.log( + `[connect] cannot restart ${entry.channel} connector ${entry.instanceId}: no stored launch arguments`, + ); + return; + } + entry.args = args; + entry.argsKnown = true; + entry.origin = "spawned"; + this.spawnEntry(entry); + }); + }, delayMs); + } + + /** + * An adopted connector has no argv of its own here, so its restart arguments + * come from the persisted autostart record. + */ + private resolveRestartArgs(entry: SupervisedEntry): string[] | undefined { + if (entry.argsKnown) { + return entry.args; + } + const persisted = getPersistedConnectorConnection( + entry.channel, + entry.instanceId, + ); + return persisted?.connectArgs?.length ? persisted.connectArgs : undefined; + } + + private cancelRestart(entry: SupervisedEntry): void { + if (entry.restartTimer !== undefined) { + this.clearTimer(entry.restartTimer); + entry.restartTimer = undefined; + } + entry.nextRestartAt = undefined; + } + + private async runCleanup(entry: SupervisedEntry): Promise { + if (!this.cleanupInstance) { + return; + } + try { + await this.cleanupInstance(entry.channel, entry.instanceId); + } catch (error) { + this.log( + `[connect] cleanup failed for ${entry.channel} connector ${entry.instanceId}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + + private isEntryAlive(entry: SupervisedEntry): boolean { + if (entry.state === "backoff" || entry.state === "failed") { + return false; + } + const pid = entry.child?.pid ?? entry.pid; + return pid !== undefined && this.isProcessRunning(pid); + } + + /** + * Adopted connectors have no child handle, so their death is only visible by + * polling. Processes this hub spawned report their own exit and are skipped. + */ + private ensurePolling(): void { + if (this.pollTimer !== undefined || this.disposed) { + return; + } + const tick = () => { + this.pollTimer = undefined; + if (this.disposed) { + return; + } + for (const entry of [...this.entries.values()]) { + if ( + entry.origin !== "adopted" || + entry.state !== "running" || + entry.child + ) { + continue; + } + if (entry.pid === undefined || !this.isProcessRunning(entry.pid)) { + this.handleExit(entry, undefined, undefined); + } + } + if (this.hasAdoptedRunning()) { + this.pollTimer = this.setTimer(tick, ADOPTED_POLL_INTERVAL_MS); + } + }; + this.pollTimer = this.setTimer(tick, ADOPTED_POLL_INTERVAL_MS); + } + + private hasAdoptedRunning(): boolean { + for (const entry of this.entries.values()) { + if (entry.origin === "adopted" && entry.state === "running") { + return true; + } + } + return false; + } + + private toRecord(entry: SupervisedEntry): SupervisedConnectorRecord { + return { + channel: entry.channel, + instanceId: entry.instanceId, + state: entry.state, + origin: entry.origin, + restarts: entry.restarts, + ...(entry.pid === undefined ? {} : { pid: entry.pid }), + ...(entry.startedAt === undefined + ? {} + : { startedAt: toIsoString(entry.startedAt) }), + ...(entry.nextRestartAt === undefined + ? {} + : { nextRestartAt: toIsoString(entry.nextRestartAt) }), + ...(entry.lastExitCode === undefined + ? {} + : { lastExitCode: entry.lastExitCode }), + ...(entry.lastExitSignal === undefined + ? {} + : { lastExitSignal: entry.lastExitSignal }), + ...(entry.lastError === undefined ? {} : { lastError: entry.lastError }), + }; + } +} + +let activeSupervisor: ConnectorSupervisor | undefined; + +/** + * The hub daemon's supervisor. Hub command handlers reach it through here + * because they are invoked per-request and have no other shared state. + */ +export function setActiveConnectorSupervisor( + supervisor: ConnectorSupervisor | undefined, +): void { + activeSupervisor = supervisor; +} + +export function getActiveConnectorSupervisor(): + | ConnectorSupervisor + | undefined { + return activeSupervisor; +} diff --git a/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.test.ts b/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.test.ts index 05c8c10a3c..56df439dc7 100644 --- a/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.test.ts +++ b/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.test.ts @@ -1,35 +1,11 @@ -import { EventEmitter } from "node:events"; -import { - CLINE_RUN_AS_HUB_DAEMON_ENV, - type ConnectorCliLaunchSpec, -} from "@cline/shared"; +import { CLINE_CONNECTOR_STARTING_INSTANCE_ENV } from "@cline/shared"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { - __test__, - reconnectDaemonConnectors, -} from "./daemon-connector-reconnect"; +import type { ConnectorSupervisor } from "./connector-supervisor"; +import { reconnectDaemonConnectors } from "./daemon-connector-reconnect"; const mocks = vi.hoisted(() => ({ - listActiveConnectors: vi.fn(), - readConnectorCliLaunchSpec: vi.fn(), reconnectPersistedConnectors: vi.fn(), - spawnProcess: vi.fn(), -})); - -vi.mock("node:child_process", () => ({ - spawn: mocks.spawnProcess, -})); - -vi.mock("@cline/shared", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - readConnectorCliLaunchSpec: mocks.readConnectorCliLaunchSpec, - }; -}); - -vi.mock("./active-connectors", () => ({ - listActiveConnectors: mocks.listActiveConnectors, + getActiveConnectorSupervisor: vi.fn(), })); vi.mock("./connector-autostart", async (importOriginal) => { @@ -40,212 +16,200 @@ vi.mock("./connector-autostart", async (importOriginal) => { }; }); -class FakeConnectorCliChild extends EventEmitter { - stderr = new EventEmitter() as EventEmitter & { - setEncoding: (encoding: string) => void; +vi.mock("./connector-supervisor", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getActiveConnectorSupervisor: mocks.getActiveConnectorSupervisor, }; +}); - constructor() { - super(); - this.stderr.setEncoding = vi.fn(); - } +type StartArgs = { + channel: string; + instanceId: string; + args: string[]; + restart?: boolean; +}; + +function createSupervisor( + options: { + supervised?: Array<{ channel: string; instanceId: string }>; + started?: boolean; + reason?: "already_running"; + } = {}, +) { + const starts: StartArgs[] = []; + const supervisor = { + list: () => + (options.supervised ?? []).map((entry) => ({ + ...entry, + state: "running" as const, + origin: "adopted" as const, + restarts: 0, + })), + start: vi.fn(async (request: StartArgs) => { + starts.push(request); + return { + started: options.started ?? true, + ...(options.reason ? { reason: options.reason } : {}), + record: { + channel: request.channel, + instanceId: request.instanceId, + state: "running" as const, + origin: "spawned" as const, + restarts: 0, + }, + }; + }), + } as unknown as ConnectorSupervisor; + return { supervisor, starts }; } -describe("daemon connector CLI launcher", () => { - const originalDaemonFlag = process.env[CLINE_RUN_AS_HUB_DAEMON_ENV]; - const spec: ConnectorCliLaunchSpec = { - launcher: "/usr/local/bin/bun", - connectArgsPrefix: ["/repo/apps/cli/src/index.ts", "connect"], - cwd: "/workspace", - }; +describe("reconnectDaemonConnectors", () => { + const originalStartingInstance = + process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; afterEach(() => { vi.clearAllMocks(); - if (originalDaemonFlag === undefined) { - delete process.env[CLINE_RUN_AS_HUB_DAEMON_ENV]; + if (originalStartingInstance === undefined) { + delete process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; } else { - process.env[CLINE_RUN_AS_HUB_DAEMON_ENV] = originalDaemonFlag; + process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV] = + originalStartingInstance; } }); - it("launches reconnect through the CLI without the daemon sentinel", async () => { - process.env[CLINE_RUN_AS_HUB_DAEMON_ENV] = "1"; - const child = new FakeConnectorCliChild(); - const spawnProcess = vi.fn(() => child); - const log = vi.fn(); - - const pending = __test__.runConnectorCli( - spec, - "telegram", - ["-k", "token"], - { log, spawnProcess }, - ); - child.emit("close", 0); - - await expect(pending).resolves.toBe(true); - expect(spawnProcess).toHaveBeenCalledWith( - "/usr/local/bin/bun", - ["/repo/apps/cli/src/index.ts", "connect", "telegram", "-k", "token"], - expect.objectContaining({ - cwd: "/workspace", - env: expect.not.objectContaining({ - [CLINE_RUN_AS_HUB_DAEMON_ENV]: "1", - }), - }), - ); - expect(log).not.toHaveBeenCalled(); - }); - - it("reports non-zero CLI reconnect exits", async () => { - const child = new FakeConnectorCliChild(); - const spawnProcess = vi.fn(() => child); - const log = vi.fn(); - - const pending = __test__.runConnectorCli( - spec, - "telegram", - ["-k", "token"], - { log, spawnProcess }, - ); - child.stderr.emit("data", "invalid token"); - child.emit("close", 1); - - await expect(pending).resolves.toBe(false); - expect(log).toHaveBeenCalledWith( - "[connect] telegram reconnect exited with code 1: invalid token", - ); - }); - - it("restarts a surviving connector so it binds to the new hub session", async () => { - const child = new FakeConnectorCliChild(); - mocks.readConnectorCliLaunchSpec.mockReturnValue(spec); - mocks.listActiveConnectors.mockReturnValue([ - { - id: "telegram:cline_bot", - type: "telegram", - instanceId: "cline_bot", - pid: 123, - hubUrl: "ws://127.0.0.1:4317", - botUsername: "cline_bot", - }, - ]); - mocks.spawnProcess.mockImplementation(() => { - queueMicrotask(() => child.emit("close", 0)); - return child; - }); + it("starts a persisted connector through the supervisor", async () => { + delete process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; + const { supervisor, starts } = createSupervisor(); mocks.reconnectPersistedConnectors.mockImplementation(async (options) => { - const target = { + const ok = await options.start({ channel: "telegram", instanceId: "cline_bot", args: ["-k", "token"], - }; - const ok = await options.start(target); + }); return [{ channel: "telegram", instanceId: "cline_bot", ok }]; }); - const log = vi.fn(); - await expect(reconnectDaemonConnectors(log)).resolves.toEqual([ + await expect( + reconnectDaemonConnectors(vi.fn(), supervisor), + ).resolves.toEqual([ { channel: "telegram", instanceId: "cline_bot", ok: true }, ]); + expect(starts).toEqual([ + { + channel: "telegram", + instanceId: "cline_bot", + args: ["-k", "token"], + restart: false, + }, + ]); + }); - expect(mocks.spawnProcess).toHaveBeenCalledWith( - "/usr/local/bin/bun", - [ - "/repo/apps/cli/src/index.ts", - "connect", - "--restart-instance", - "cline_bot", - "telegram", - "-k", - "token", - ], - expect.objectContaining({ cwd: "/workspace" }), - ); + it("restarts a connector that survived the previous hub", async () => { + delete process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; + const { supervisor, starts } = createSupervisor({ + supervised: [{ channel: "telegram", instanceId: "cline_bot" }], + }); + const log = vi.fn(); + mocks.reconnectPersistedConnectors.mockImplementation(async (options) => { + await options.start({ + channel: "telegram", + instanceId: "cline_bot", + args: ["-k", "token"], + }); + return []; + }); + + await reconnectDaemonConnectors(log, supervisor); + + // A survivor authenticated against the dead hub's token, so it has to come + // back rather than keep running. + expect(starts[0]?.restart).toBe(true); expect(log).toHaveBeenCalledWith( "[connect] restarting surviving telegram connector cline_bot for the new hub session", ); }); - it("restarts multiple surviving instances independently", async () => { - mocks.readConnectorCliLaunchSpec.mockReturnValue(spec); - mocks.listActiveConnectors.mockReturnValue([ - { - id: "telegram:first_bot", - type: "telegram", - instanceId: "first_bot", - pid: 123, - hubUrl: "ws://127.0.0.1:4317", - botUsername: "first_bot", - }, - { - id: "telegram:second_bot", - type: "telegram", - instanceId: "second_bot", - pid: 456, - hubUrl: "ws://127.0.0.1:4317", - botUsername: "second_bot", - }, - ]); - mocks.spawnProcess.mockImplementation(() => { - const child = new FakeConnectorCliChild(); - queueMicrotask(() => child.emit("close", 0)); - return child; + it("does not reconnect the connector instance that is starting this daemon", async () => { + process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV] = JSON.stringify({ + channel: "telegram", + instanceId: "cline_bot", }); + const { supervisor } = createSupervisor(); + let isHealthy: + | ((target: { channel: string; instanceId: string }) => boolean) + | undefined; mocks.reconnectPersistedConnectors.mockImplementation(async (options) => { - const targets = [ - { - channel: "telegram", - instanceId: "first_bot", - args: ["-k", "first-token"], - }, - { - channel: "telegram", - instanceId: "second_bot", - args: ["-k", "second-token"], - }, - ]; - return await Promise.all( - targets.map(async (target) => ({ - channel: target.channel, - instanceId: target.instanceId, - ok: await options.start(target), - })), - ); + isHealthy = options.isHealthy; + return []; + }); + + await reconnectDaemonConnectors(vi.fn(), supervisor); + + expect(isHealthy?.({ channel: "telegram", instanceId: "cline_bot" })).toBe( + true, + ); + // A different instance of the same channel still needs reconnecting. + expect(isHealthy?.({ channel: "telegram", instanceId: "other_bot" })).toBe( + false, + ); + expect(isHealthy?.({ channel: "slack", instanceId: "cline_bot" })).toBe( + false, + ); + }); + + it("reconnects every persisted instance when no connector is starting", async () => { + delete process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; + const { supervisor } = createSupervisor(); + let isHealthy: + | ((target: { channel: string; instanceId: string }) => boolean) + | undefined; + mocks.reconnectPersistedConnectors.mockImplementation(async (options) => { + isHealthy = options.isHealthy; + return []; + }); + + await reconnectDaemonConnectors(vi.fn(), supervisor); + + expect(isHealthy?.({ channel: "telegram", instanceId: "cline_bot" })).toBe( + false, + ); + }); + + it("reports an already-running instance without treating it as started", async () => { + delete process.env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; + const { supervisor } = createSupervisor({ + started: false, + reason: "already_running", }); const log = vi.fn(); + mocks.reconnectPersistedConnectors.mockImplementation(async (options) => { + const ok = await options.start({ + channel: "slack", + instanceId: "cline-slack", + args: [], + }); + return [{ channel: "slack", instanceId: "cline-slack", ok }]; + }); - await expect(reconnectDaemonConnectors(log)).resolves.toEqual([ - { channel: "telegram", instanceId: "first_bot", ok: true }, - { channel: "telegram", instanceId: "second_bot", ok: true }, + await expect(reconnectDaemonConnectors(log, supervisor)).resolves.toEqual([ + { channel: "slack", instanceId: "cline-slack", ok: false }, ]); - - expect(mocks.spawnProcess).toHaveBeenNthCalledWith( - 1, - "/usr/local/bin/bun", - [ - "/repo/apps/cli/src/index.ts", - "connect", - "--restart-instance", - "first_bot", - "telegram", - "-k", - "first-token", - ], - expect.any(Object), + expect(log).toHaveBeenCalledWith( + "[connect] slack connector cline-slack is already running under this hub", ); - expect(mocks.spawnProcess).toHaveBeenNthCalledWith( - 2, - "/usr/local/bin/bun", - [ - "/repo/apps/cli/src/index.ts", - "connect", - "--restart-instance", - "second_bot", - "telegram", - "-k", - "second-token", - ], - expect.any(Object), + }); + + it("does nothing when no supervisor is active", async () => { + const log = vi.fn(); + mocks.getActiveConnectorSupervisor.mockReturnValue(undefined); + + await expect(reconnectDaemonConnectors(log)).resolves.toEqual([]); + expect(mocks.reconnectPersistedConnectors).not.toHaveBeenCalled(); + expect(log).toHaveBeenCalledWith( + "[connect] cannot reconnect connectors: no connector supervisor is active", ); }); }); diff --git a/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.ts b/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.ts index c75bfa2682..e881d881bd 100644 --- a/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.ts +++ b/sdk/packages/core/src/services/connectors/daemon-connector-reconnect.ts @@ -1,152 +1,69 @@ -import { spawn } from "node:child_process"; -import { - CLINE_RUN_AS_HUB_DAEMON_ENV, - type ConnectorCliLaunchSpec, - readConnectorCliLaunchSpec, -} from "@cline/shared"; -import { listActiveConnectors } from "./active-connectors"; +import { readStartingConnectorInstance } from "@cline/shared"; import { type ReconnectAttempt, reconnectPersistedConnectors, } from "./connector-autostart"; - -type ConnectorCliChild = { - stderr?: { - setEncoding: (encoding: string) => void; - on: (event: "data", listener: (chunk: unknown) => void) => void; - }; - once: (event: "error" | "close", listener: (value: unknown) => void) => void; -}; - -type SpawnConnectorCli = ( - launcher: string, - args: string[], - options: { - cwd: string; - env: NodeJS.ProcessEnv; - stdio: ["ignore", "ignore", "pipe"]; - windowsHide: boolean; - }, -) => ConnectorCliChild; - -async function runConnectorCli( - spec: ConnectorCliLaunchSpec, - channel: string, - args: string[], - options: { - restartInstanceId?: string; - log: (message: string) => void; - spawnProcess?: SpawnConnectorCli; - }, -): Promise { - const { log, restartInstanceId } = options; - const spawnProcess = options.spawnProcess ?? (spawn as SpawnConnectorCli); - const childEnv = { ...process.env }; - delete childEnv[CLINE_RUN_AS_HUB_DAEMON_ENV]; - - return await new Promise((resolve) => { - let stderr = ""; - let settled = false; - const finish = (ok: boolean, message?: string) => { - if (settled) { - return; - } - settled = true; - if (message) { - log(message); - } - resolve(ok); - }; - - try { - const child = spawnProcess( - spec.launcher, - [ - ...spec.connectArgsPrefix, - ...(restartInstanceId - ? ["--restart-instance", restartInstanceId] - : []), - channel, - ...args, - ], - { - cwd: spec.cwd, - env: childEnv, - stdio: ["ignore", "ignore", "pipe"], - windowsHide: true, - }, - ); - child.stderr?.setEncoding("utf8"); - child.stderr?.on("data", (chunk) => { - stderr += String(chunk); - }); - child.once("error", (error) => { - const message = error instanceof Error ? error.message : String(error); - finish( - false, - `[connect] failed to launch ${channel} reconnect: ${message}`, - ); - }); - child.once("close", (exitCode) => { - const code = typeof exitCode === "number" ? exitCode : 1; - finish( - code === 0, - code === 0 - ? undefined - : `[connect] ${channel} reconnect exited with code ${code}${ - stderr.trim() ? `: ${stderr.trim()}` : "" - }`, - ); - }); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - finish( - false, - `[connect] failed to launch ${channel} reconnect: ${message}`, - ); - } - }); -} +import type { ConnectorSupervisor } from "./connector-supervisor"; +import { getActiveConnectorSupervisor } from "./connector-supervisor"; /** - * Restore connectors from the daemon entrypoint through a host-provided CLI - * launch specification. This keeps connector implementations in the CLI app - * while allowing the package-owned daemon entrypoint to supervise recovery. + * Bring persisted connectors back under the new hub's supervision. + * + * Called once per daemon boot, after the supervisor has adopted whatever + * survived the previous hub. Survivors still have to be *restarted* rather than + * left running: their hub client authenticated against the old hub's token and + * cannot re-authenticate against this one, so the process has to come back to + * attach to the new session. + * + * Spawning itself belongs to the supervisor, which is the single authority on + * how many processes may hold one connector's credentials. */ export async function reconnectDaemonConnectors( log: (message: string) => void = (message) => process.stderr.write(`[hub-daemon] ${message}\n`), + supervisor: ConnectorSupervisor | undefined = getActiveConnectorSupervisor(), ): Promise { - const launchSpec = readConnectorCliLaunchSpec(); - const activeInstances = new Set(); - for (const record of listActiveConnectors()) { - activeInstances.add(`${record.type}\0${record.instanceId}`); + if (!supervisor) { + log( + "[connect] cannot reconnect connectors: no connector supervisor is active", + ); + return []; } + const supervisedBefore = new Set( + supervisor + .list() + .map((record) => `${record.channel}\0${record.instanceId}`), + ); + // The connector that spawned this daemon is mid-startup and attaches itself. + // Starting it here would put a second process on the same credentials, and + // for socket-mode adapters both would hold a live connection and split + // incoming events between them. + const startingInstance = readStartingConnectorInstance(); + return await reconnectPersistedConnectors({ + isHealthy: ({ channel, instanceId }) => + startingInstance?.channel === channel && + startingInstance.instanceId === instanceId, start: async ({ channel, instanceId, args }) => { - if (!launchSpec) { - log( - `[connect] cannot reconnect ${channel} instance ${instanceId}: connector CLI launch information is unavailable`, - ); - return false; - } - const restartInstanceId = activeInstances.has(`${channel}\0${instanceId}`) - ? instanceId - : undefined; - if (restartInstanceId) { + const survived = supervisedBefore.has(`${channel}\0${instanceId}`); + if (survived) { log( `[connect] restarting surviving ${channel} connector ${instanceId} for the new hub session`, ); } - return await runConnectorCli(launchSpec, channel, args, { - restartInstanceId, - log, + const result = await supervisor.start({ + channel, + instanceId, + args, + restart: survived, }); + if (!result.started && result.reason === "already_running") { + log( + `[connect] ${channel} connector ${instanceId} is already running under this hub`, + ); + } + return result.started; }, log, }); } - -export const __test__ = { - runConnectorCli, -}; diff --git a/sdk/packages/shared/src/connectors/supervision.ts b/sdk/packages/shared/src/connectors/supervision.ts new file mode 100644 index 0000000000..51e086fd99 --- /dev/null +++ b/sdk/packages/shared/src/connectors/supervision.ts @@ -0,0 +1,75 @@ +/** + * Types for hub-owned connector supervision. + * + * A connector is a separate process holding third-party credentials (a Slack + * socket, a Telegram poll loop). The hub supervises those processes: it is the + * single authority on which instances may run, it reaps their state when they + * die, and it restarts them with backoff. These records are what it reports + * about that work. + */ + +/** How a supervised connector process came under the hub's watch. */ +export type SupervisedConnectorOrigin = + /** The current hub spawned it, so it has a live child handle. */ + | "spawned" + /** It predates the current hub, which adopted it by pid from its state file. */ + | "adopted"; + +export type SupervisedConnectorState = + /** Process is alive as far as the hub can tell. */ + | "running" + /** Died and is waiting out its restart backoff. */ + | "backoff" + /** Died too many times in a row; the hub has given up restarting it. */ + | "failed" + /** Stopped on request; the hub will not restart it. */ + | "stopped"; + +export type SupervisedConnectorRecord = { + channel: string; + instanceId: string; + state: SupervisedConnectorState; + origin: SupervisedConnectorOrigin; + pid?: number; + startedAt?: string; + /** Consecutive restarts that have not yet been cleared by a stable run. */ + restarts: number; + /** When the next restart attempt is due, while in "backoff". */ + nextRestartAt?: string; + lastExitCode?: number; + lastExitSignal?: string; + lastError?: string; +}; + +export type ConnectorStartRequest = { + channel: string; + instanceId: string; + /** Connector CLI arguments, excluding the channel name itself. */ + args: string[]; + /** + * Replace a running instance instead of reporting it as already running. + * Used by `connect --restart`. + */ + restart?: boolean; +}; + +export type ConnectorStartResult = { + /** False when an instance was already running and `restart` was not set. */ + started: boolean; + record: SupervisedConnectorRecord; + /** Why `started` is false, when it is. */ + reason?: "already_running"; +}; + +export type ConnectorStopRequest = { + channel: string; + instanceId: string; + /** Stop auto-restarting this instance as well. Defaults to true. */ + disableAutostart?: boolean; +}; + +export type ConnectorStopResultPayload = { + stopped: boolean; + channel: string; + instanceId: string; +}; diff --git a/sdk/packages/shared/src/hub.ts b/sdk/packages/shared/src/hub.ts index 94edcfb094..b256e65be4 100644 --- a/sdk/packages/shared/src/hub.ts +++ b/sdk/packages/shared/src/hub.ts @@ -20,7 +20,10 @@ export type HubCapabilityName = | "schedule.create" | "schedule.list" | "settings.get" - | "settings.set"; + | "settings.set" + | "connector.start" + | "connector.stop" + | "connector.supervised"; export const HUB_CAPABILITIES: readonly HubCapabilityName[] = [ "client.register", @@ -34,6 +37,9 @@ export const HUB_CAPABILITIES: readonly HubCapabilityName[] = [ "schedule.list", "settings.get", "settings.set", + "connector.start", + "connector.stop", + "connector.supervised", ]; export interface HubProtocolMetadata { @@ -455,6 +461,9 @@ export type HubCommandName = | "connector.channels" | "connector.configure" | "connector.delete_config" + | "connector.start" + | "connector.stop" + | "connector.supervised" | "cron.event.ingest" | "cron.event.list" | "cron.event.get" diff --git a/sdk/packages/shared/src/index.ts b/sdk/packages/shared/src/index.ts index 222140e991..5b88967ad6 100644 --- a/sdk/packages/shared/src/index.ts +++ b/sdk/packages/shared/src/index.ts @@ -38,6 +38,15 @@ export { mergeConnectorConnectArgs, shouldIncludeConnectorField, } from "./connectors/platforms"; +export type { + ConnectorStartRequest, + ConnectorStartResult, + ConnectorStopRequest, + ConnectorStopResultPayload, + SupervisedConnectorOrigin, + SupervisedConnectorRecord, + SupervisedConnectorState, +} from "./connectors/supervision"; export type { AutomationEventEnvelope, CronEventSpec, @@ -441,13 +450,23 @@ export { getClineEnvironmentConfig, resolveClineEnvironment, } from "./runtime/cline-environment"; -export type { ConnectorCliLaunchSpec } from "./runtime/hub-daemon-env"; +export type { + ConnectorCliLaunchSpec, + ConnectorInstanceRef, +} from "./runtime/hub-daemon-env"; export { CLINE_CONNECTOR_CLI_LAUNCH_ENV, + CLINE_CONNECTOR_STARTING_INSTANCE_ENV, + CLINE_CONNECTOR_SUPERVISED_ENV, CLINE_RUN_AS_HUB_DAEMON_ENV, + claimHubDaemonProcess, + claimSupervisedConnectorProcess, isHubDaemonProcess, + isSupervisedConnectorProcess, readConnectorCliLaunchSpec, + readStartingConnectorInstance, setConnectorCliLaunchSpec, + setStartingConnectorInstance, } from "./runtime/hub-daemon-env"; export type { CaptureAgentUnexpectedReasoningTokensInput, diff --git a/sdk/packages/shared/src/runtime/hub-daemon-env.test.ts b/sdk/packages/shared/src/runtime/hub-daemon-env.test.ts index 59f5331688..2cf71ee663 100644 --- a/sdk/packages/shared/src/runtime/hub-daemon-env.test.ts +++ b/sdk/packages/shared/src/runtime/hub-daemon-env.test.ts @@ -1,10 +1,17 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; import { CLINE_CONNECTOR_CLI_LAUNCH_ENV, + CLINE_CONNECTOR_STARTING_INSTANCE_ENV, + CLINE_CONNECTOR_SUPERVISED_ENV, CLINE_RUN_AS_HUB_DAEMON_ENV, + claimHubDaemonProcess, + claimSupervisedConnectorProcess, isHubDaemonProcess, + isSupervisedConnectorProcess, readConnectorCliLaunchSpec, + readStartingConnectorInstance, setConnectorCliLaunchSpec, + setStartingConnectorInstance, } from "./hub-daemon-env"; describe("hub daemon environment helpers", () => { @@ -35,6 +42,35 @@ describe("hub daemon environment helpers", () => { expect(env[CLINE_CONNECTOR_CLI_LAUNCH_ENV]).toBe(JSON.stringify(spec)); }); + it("round-trips the connector instance that is starting", () => { + const env: Record = {}; + const ref = { channel: "slack", instanceId: "cline-slack" }; + + setStartingConnectorInstance(ref, env); + + expect(readStartingConnectorInstance(env)).toEqual(ref); + expect(env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]).toBe( + JSON.stringify(ref), + ); + }); + + it("reports no starting connector instance when the marker is absent or malformed", () => { + expect(readStartingConnectorInstance({})).toBeUndefined(); + expect( + readStartingConnectorInstance({ + [CLINE_CONNECTOR_STARTING_INSTANCE_ENV]: "not json", + }), + ).toBeUndefined(); + expect( + readStartingConnectorInstance({ + [CLINE_CONNECTOR_STARTING_INSTANCE_ENV]: JSON.stringify({ + channel: "slack", + instanceId: " ", + }), + }), + ).toBeUndefined(); + }); + it("rejects malformed connector CLI launch specifications", () => { expect( readConnectorCliLaunchSpec({ @@ -47,3 +83,108 @@ describe("hub daemon environment helpers", () => { ).toBeUndefined(); }); }); + +describe("claiming the hub daemon sentinel", () => { + const original = process.env[CLINE_RUN_AS_HUB_DAEMON_ENV]; + + afterEach(() => { + if (original === undefined) { + delete process.env[CLINE_RUN_AS_HUB_DAEMON_ENV]; + } else { + process.env[CLINE_RUN_AS_HUB_DAEMON_ENV] = original; + } + // Reset the module latch so cases do not leak into each other. + claimHubDaemonProcess({}); + }); + + it("removes the sentinel so spawned children cannot inherit it", () => { + const env: Record = { + [CLINE_RUN_AS_HUB_DAEMON_ENV]: "1", + PATH: "/usr/bin", + }; + + expect(claimHubDaemonProcess(env)).toBe(true); + expect(CLINE_RUN_AS_HUB_DAEMON_ENV in env).toBe(false); + // Unrelated environment is untouched. + expect(env.PATH).toBe("/usr/bin"); + }); + + it("still reports daemon mode after the sentinel is gone", () => { + process.env[CLINE_RUN_AS_HUB_DAEMON_ENV] = "1"; + + claimHubDaemonProcess(); + + expect(process.env[CLINE_RUN_AS_HUB_DAEMON_ENV]).toBeUndefined(); + // The guards that stop a daemon spawning another daemon rely on this. + expect(isHubDaemonProcess()).toBe(true); + }); + + it("reports non-daemon mode when the sentinel was never set", () => { + delete process.env[CLINE_RUN_AS_HUB_DAEMON_ENV]; + + expect(claimHubDaemonProcess()).toBe(false); + expect(isHubDaemonProcess()).toBe(false); + }); + + it("reads an explicitly passed environment verbatim, ignoring the latch", () => { + process.env[CLINE_RUN_AS_HUB_DAEMON_ENV] = "1"; + claimHubDaemonProcess(); + + expect(isHubDaemonProcess({})).toBe(false); + expect(isHubDaemonProcess({ [CLINE_RUN_AS_HUB_DAEMON_ENV]: "1" })).toBe( + true, + ); + }); +}); + +describe("supervised connector marker", () => { + const original = process.env[CLINE_CONNECTOR_SUPERVISED_ENV]; + + afterEach(() => { + if (original === undefined) { + delete process.env[CLINE_CONNECTOR_SUPERVISED_ENV]; + } else { + process.env[CLINE_CONNECTOR_SUPERVISED_ENV] = original; + } + // Reset the module latch so cases do not leak into each other. + claimSupervisedConnectorProcess({}); + }); + + it("removes the marker so spawned children cannot inherit it", () => { + const env: Record = { + [CLINE_CONNECTOR_SUPERVISED_ENV]: "1", + PATH: "/usr/bin", + }; + + expect(claimSupervisedConnectorProcess(env)).toBe(true); + expect(CLINE_CONNECTOR_SUPERVISED_ENV in env).toBe(false); + expect(env.PATH).toBe("/usr/bin"); + }); + + it("still reports supervision after the marker is gone", () => { + process.env[CLINE_CONNECTOR_SUPERVISED_ENV] = "1"; + + claimSupervisedConnectorProcess(); + + expect(process.env[CLINE_CONNECTOR_SUPERVISED_ENV]).toBeUndefined(); + // The connector still has to run in-process rather than delegating. + expect(isSupervisedConnectorProcess()).toBe(true); + }); + + it("reports no supervision when the marker was never set", () => { + delete process.env[CLINE_CONNECTOR_SUPERVISED_ENV]; + + expect(claimSupervisedConnectorProcess()).toBe(false); + expect(isSupervisedConnectorProcess()).toBe(false); + }); + + it("reads an explicitly passed environment verbatim, ignoring the latch", () => { + process.env[CLINE_CONNECTOR_SUPERVISED_ENV] = "1"; + claimSupervisedConnectorProcess(); + + expect(isSupervisedConnectorProcess({})).toBe(false); + expect( + isSupervisedConnectorProcess({ [CLINE_CONNECTOR_SUPERVISED_ENV]: "1" }), + ).toBe(true); + }); +}); diff --git a/sdk/packages/shared/src/runtime/hub-daemon-env.ts b/sdk/packages/shared/src/runtime/hub-daemon-env.ts index 6ba5d49f0f..202219a235 100644 --- a/sdk/packages/shared/src/runtime/hub-daemon-env.ts +++ b/sdk/packages/shared/src/runtime/hub-daemon-env.ts @@ -1,5 +1,8 @@ export const CLINE_RUN_AS_HUB_DAEMON_ENV = "CLINE_RUN_AS_HUB_DAEMON"; export const CLINE_CONNECTOR_CLI_LAUNCH_ENV = "CLINE_CONNECTOR_CLI_LAUNCH"; +export const CLINE_CONNECTOR_STARTING_INSTANCE_ENV = + "CLINE_CONNECTOR_STARTING_INSTANCE"; +export const CLINE_CONNECTOR_SUPERVISED_ENV = "CLINE_CONNECTOR_SUPERVISED"; export interface ConnectorCliLaunchSpec { launcher: string; @@ -7,10 +10,59 @@ export interface ConnectorCliLaunchSpec { cwd: string; } -export function isHubDaemonProcess( +/** Identifies one connector instance: an adapter channel plus its instance id. */ +export interface ConnectorInstanceRef { + channel: string; + instanceId: string; +} + +/** + * Latched result of {@link claimHubDaemonProcess}, so the sentinel can be + * removed from the environment while the process still knows what it is. + */ +let claimedHubDaemonProcess: boolean | undefined; + +/** + * Take the daemon sentinel out of the environment, remembering its value. + * + * The sentinel selects which personality the shared CLI binary boots, so it must + * not outlive that decision: the hub daemon hosts session runtimes, and every + * process a session spawns - agent shell commands, MCP servers, hooks, plugin + * sandboxes - inherits its environment. An inherited sentinel makes each of + * those try to become a hub daemon instead of running the command, and they die + * on EADDRINUSE against the real hub. Observed as every `cline` invocation from + * a Slack connector agent failing, `cline --help` included, because the + * personality is chosen before any argument parsing. + * + * Call this once from an entrypoint, in place of {@link isHubDaemonProcess}. + * Spawn paths that deliberately start a daemon set the variable explicitly on + * the child environment, so they are unaffected. + */ +export function claimHubDaemonProcess( env: Record = process.env, ): boolean { - return env[CLINE_RUN_AS_HUB_DAEMON_ENV] === "1"; + claimedHubDaemonProcess = env[CLINE_RUN_AS_HUB_DAEMON_ENV] === "1"; + delete env[CLINE_RUN_AS_HUB_DAEMON_ENV]; + return claimedHubDaemonProcess; +} + +/** + * Whether this process is the hub daemon. + * + * Reads the latch first so callers still get the right answer after + * {@link claimHubDaemonProcess} has scrubbed the environment - notably the + * guards that stop a daemon from spawning another daemon. An explicitly passed + * environment is always read verbatim. + */ +export function isHubDaemonProcess( + env?: Record, +): boolean { + if (env) { + return env[CLINE_RUN_AS_HUB_DAEMON_ENV] === "1"; + } + return ( + claimedHubDaemonProcess ?? process.env[CLINE_RUN_AS_HUB_DAEMON_ENV] === "1" + ); } export function setConnectorCliLaunchSpec( @@ -20,6 +72,97 @@ export function setConnectorCliLaunchSpec( env[CLINE_CONNECTOR_CLI_LAUNCH_ENV] = JSON.stringify(spec); } +/** Latched result of {@link claimSupervisedConnectorProcess}. */ +let claimedSupervisedConnectorProcess: boolean | undefined; + +/** + * Take the supervised-connector marker out of the environment, remembering it. + * + * Same hazard as {@link claimHubDaemonProcess}: a supervised connector hosts + * agent sessions, and everything they spawn — shell commands, MCP servers, hooks + * — inherits its environment. An inherited marker makes a nested `cline connect` + * think it is the process the hub is tracking, so it runs the connector in the + * foreground of that shell command instead of handing it to the hub. + * + * Call once from an entrypoint, in place of + * {@link isSupervisedConnectorProcess}. The supervisor sets the marker + * explicitly on the child environment, so it is unaffected. + */ +export function claimSupervisedConnectorProcess( + env: Record = process.env, +): boolean { + claimedSupervisedConnectorProcess = + env[CLINE_CONNECTOR_SUPERVISED_ENV] === "1"; + delete env[CLINE_CONNECTOR_SUPERVISED_ENV]; + return claimedSupervisedConnectorProcess; +} + +/** + * True in a connector process the hub supervisor launched. + * + * Such a process must run the connector itself rather than doing what a + * user-invoked background `connect` does — asking the hub to start it (which + * would loop straight back here) or spawning its own detached child and exiting + * (which would leave the supervisor holding a handle to a process that is + * already gone). + * + * Reads the latch first so callers still get the right answer after + * {@link claimSupervisedConnectorProcess} has scrubbed the environment. An + * explicitly passed environment is always read verbatim. + */ +export function isSupervisedConnectorProcess( + env?: Record, +): boolean { + if (env) { + return env[CLINE_CONNECTOR_SUPERVISED_ENV] === "1"; + } + return ( + claimedSupervisedConnectorProcess ?? + process.env[CLINE_CONNECTOR_SUPERVISED_ENV] === "1" + ); +} + +/** + * Announce the connector instance this process is in the middle of starting. + * + * A connector starts its own hub daemon, and the daemon then reconnects every + * persisted connector. The instance doing the starting is not yet registered as + * active when the daemon boots, so without this marker the daemon launches a + * second copy of it - two processes holding the same bot token. The daemon + * inherits this variable from the connector that spawned it, so it can tell + * "the connector that is bringing me up" apart from "a connector left over from + * a previous hub session", which genuinely does need restarting. + */ +export function setStartingConnectorInstance( + ref: ConnectorInstanceRef, + env: Record = process.env, +): void { + env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV] = JSON.stringify(ref); +} + +export function readStartingConnectorInstance( + env: Record = process.env, +): ConnectorInstanceRef | undefined { + const raw = env[CLINE_CONNECTOR_STARTING_INSTANCE_ENV]; + if (!raw) { + return undefined; + } + try { + const parsed = JSON.parse(raw) as Partial; + if ( + typeof parsed.channel !== "string" || + !parsed.channel.trim() || + typeof parsed.instanceId !== "string" || + !parsed.instanceId.trim() + ) { + return undefined; + } + return { channel: parsed.channel, instanceId: parsed.instanceId }; + } catch { + return undefined; + } +} + export function readConnectorCliLaunchSpec( env: Record = process.env, ): ConnectorCliLaunchSpec | undefined { diff --git a/sdk/packages/shared/src/storage/index.ts b/sdk/packages/shared/src/storage/index.ts index 8c49511fe4..ba9331b870 100644 --- a/sdk/packages/shared/src/storage/index.ts +++ b/sdk/packages/shared/src/storage/index.ts @@ -23,6 +23,7 @@ export { resolveClineDir, resolveConfiguredPluginModulePaths, resolveConnectorDataDir, + resolveConnectorLogPath, resolveConnectorSettingsPath, resolveConnectorsDbPath, resolveCronDbPath, diff --git a/sdk/packages/shared/src/storage/paths.ts b/sdk/packages/shared/src/storage/paths.ts index 2f23e0f8af..332cacc794 100644 --- a/sdk/packages/shared/src/storage/paths.ts +++ b/sdk/packages/shared/src/storage/paths.ts @@ -179,6 +179,27 @@ export function resolveConnectorDataDir(): string { return join(resolveClineDataDir(), "connectors"); } +/** + * Where a connector instance's stdout/stderr is captured. Both the CLI (which + * spawns detached connectors directly) and the hub supervisor (which spawns and + * reaps them) need to agree on this path, so it lives here rather than in + * either one. + */ +export function resolveConnectorLogPath( + channel: string, + instanceKey: string, +): string { + const safeChannel = channel.replace(/[^a-zA-Z0-9._-]+/g, "_"); + const safeKey = instanceKey.replace(/[^a-zA-Z0-9._-]+/g, "_"); + return join( + resolveClineDataDir(), + "logs", + "connectors", + safeChannel, + `${safeKey}.log`, + ); +} + export function resolveConnectorSettingsPath(): string { const explicitPath = process.env.CLINE_CONNECTOR_SETTINGS_PATH?.trim(); if (explicitPath) {