diff --git a/src/cli/commands/daemon.ts b/src/cli/commands/daemon.ts index acdd880..22b80d6 100644 --- a/src/cli/commands/daemon.ts +++ b/src/cli/commands/daemon.ts @@ -429,7 +429,8 @@ export default class Daemon extends Command { liveKuboPids.add(pid); process.once("exit", () => liveKuboPids.delete(pid)); } - } + }, + mergedPkcOptions.httpRoutersOptions ); pendingKuboStart = startPromise; let startedProcess: ChildProcessWithoutNullStreams | undefined; diff --git a/src/ipfs/startIpfs.ts b/src/ipfs/startIpfs.ts index cc8de79..280c58e 100644 --- a/src/ipfs/startIpfs.ts +++ b/src/ipfs/startIpfs.ts @@ -85,6 +85,58 @@ export async function ensureIpnsPubsubEnabled(log: any, ipfsConfigPath: string) log("Enabled Ipns.UsePubsub in IPFS config (replaces deprecated --enable-namesys-pubsub flag).", ipfsConfigPath); } +// pkc-js (>= 0.0.46) rewrites the connected kubo node's Routing config during its init from the +// httpRoutersOptions we pass it, and POSTs /shutdown to kubo when the router endpoint set changed +// — always true on a repo pkc-js hasn't configured yet — expecting the daemon to restart kubo +// (which keepKuboUp does). That restart opens a multi-second window right after the daemon's +// ready banner where kubo's API refuses connections (issue #143). Writing the equivalent config +// before kubo spawns makes pkc-js's endpoint comparison a no-op, so the shutdown never happens. +// +// This must mirror pkc-js's setupKuboHttpRouters exactly — including the HttpRouterNotSupported +// sentinel, whose endpoint participates in the comparison. If a pkc-js upgrade changes that +// mapping, behavior degrades back to a one-time restart; the regression test in +// test/cli/daemon-no-kubo-restart-on-fresh-start.test.ts catches that on upgrade. +export function buildKuboRoutingConfigForHttpRouters(httpRoutersOptions: string[]) { + const httpRouterUrls = [...httpRoutersOptions].sort(); + const parallelRouters: { RouterName: string; IgnoreErrors: boolean; Timeout: string }[] = []; + const routers: Record = { + HttpRoutersParallel: { Type: "parallel", Parameters: { Routers: parallelRouters } }, + HttpRouterNotSupported: { Type: "http", Parameters: { Endpoint: "http://kubohttprouternotsupported" } } + }; + for (const [i, httpRouterUrl] of httpRouterUrls.entries()) { + const RouterName = `HttpRouter${i + 1}`; + routers[RouterName] = { Type: "http", Parameters: { Endpoint: httpRouterUrl } }; + parallelRouters[i] = { RouterName, IgnoreErrors: true, Timeout: "10s" }; + } + return { + Type: "custom", + Methods: { + "find-providers": { RouterName: "HttpRoutersParallel" }, + provide: { RouterName: "HttpRoutersParallel" }, + "find-peers": { RouterName: "HttpRouterNotSupported" }, + "get-ipns": { RouterName: "HttpRouterNotSupported" }, + "put-ipns": { RouterName: "HttpRouterNotSupported" } + }, + Routers: routers + }; +} + +// Runs on every start (not just fresh init): a release or flag change can alter the router list +// on an existing repo, which would otherwise re-trigger pkc-js's shutdown. Routing is effectively +// owned by pkc-js — it overwrites the section unconditionally at init — so replacing it here +// preserves no less user state than pkc-js itself would. pkc-js also sets +// Provide.DHT.SweepEnabled=false alongside; seed it too so kubo boots with its final config. +export async function ensureKuboRoutingConfigMatchesHttpRouters(log: any, ipfsConfigPath: string, httpRoutersOptions: string[]) { + if (!Array.isArray(httpRoutersOptions) || httpRoutersOptions.length === 0) return; + const config = JSON.parse((await fsPromises.readFile(ipfsConfigPath)).toString()); + const desiredRouting = buildKuboRoutingConfigForHttpRouters(httpRoutersOptions); + if (remeda.isDeepEqual(config.Routing, desiredRouting) && config.Provide?.DHT?.SweepEnabled === false) return; + config.Routing = desiredRouting; + config.Provide = { ...(config.Provide ?? {}), DHT: { ...(config.Provide?.DHT ?? {}), SweepEnabled: false } }; + await fsPromises.writeFile(ipfsConfigPath, JSON.stringify(config, null, 4)); + log("Pre-seeded kubo Routing config for the configured http routers so pkc-js init does not restart kubo.", ipfsConfigPath); +} + // use this custom function instead of spawnSync for better logging // also spawnSync might have been causing crash on start on windows @@ -221,7 +273,8 @@ export async function startKuboNode( apiUrl: URL, gatewayUrl: URL, dataPath: string, - onSpawn?: (process: ChildProcessWithoutNullStreams) => void + onSpawn?: (process: ChildProcessWithoutNullStreams) => void, + httpRoutersOptions?: string[] ): Promise { // Preparation phase runs as plain awaits so any failure rejects the returned promise. // It must NOT live inside the new Promise() executor below: an async executor swallows @@ -262,6 +315,8 @@ export async function startKuboNode( // Replaces the deprecated `--enable-namesys-pubsub` daemon flag; must run for existing repos too. await ensureIpnsPubsubEnabled(log, ipfsConfigPath); + if (httpRoutersOptions) await ensureKuboRoutingConfigMatchesHttpRouters(log, ipfsConfigPath, httpRoutersOptions); + try { await _spawnAsync(log, kuboExePath, ["repo", "migrate"], { env, hideWindows: true }); log("Ensured IPFS repository is migrated to the latest supported version."); diff --git a/test/cli/daemon-no-kubo-restart-on-fresh-start.test.ts b/test/cli/daemon-no-kubo-restart-on-fresh-start.test.ts new file mode 100644 index 0000000..962bd78 --- /dev/null +++ b/test/cli/daemon-no-kubo-restart-on-fresh-start.test.ts @@ -0,0 +1,101 @@ +// Regression test for issue #143: a fresh daemon start must not go through a kubo +// shutdown/restart cycle. +// +// pkc-js (>= 0.0.46) rewrites the connected kubo node's Routing config during its init and, when +// the router endpoint set changed — previously always true on a fresh repo — POSTs /shutdown to +// kubo, expecting the daemon's keepKuboUp to restart it. That restart opens a multi-second window +// where kubo's API refuses connections, and early CLI commands (e.g. `community create`) can burn +// their whole budget inside it (observed on windows-latest CI, run 33471620931). +// +// The daemon now pre-seeds the equivalent Routing config into the kubo config file before +// spawning kubo, so pkc-js's endpoint comparison is a no-op and no shutdown is issued. This test +// also guards against pkc-js upgrades changing the Routing mapping (which would silently bring +// the restart back): the pre-seed must keep matching what pkc-js computes. +// +// pkc-js's own router-setup log lines don't reach the daemon log (its bundled logger doesn't pick +// up the daemon's debug config), so the assertions anchor on the daemon's own logging instead: +// "Kubo node with pid (...) exited. Will attempt to restart it" and the count of +// "Started kubo ipfs process with pid" lines, both logged by the default `bitsocial*` namespace. +import { spawn } from "child_process"; +import { describe, it, expect, afterAll } from "vitest"; +import { directory as randomDirectory } from "tempy"; +import fsPromise from "fs/promises"; +import path from "path"; +import dns from "node:dns"; +import { + type ManagedChildProcess, + stopPkcDaemon, + startPkcDaemonWithDynamicPorts, + waitForCondition, + ensureKuboNodeStopped +} from "../helpers/daemon-helpers.js"; +dns.setDefaultResultOrder("ipv4first"); // to be able to resolve localhost + +const runBitsocialCommand = (args: string[], timeoutMs: number): Promise<{ stdout: string; stderr: string; exitCode: number | null }> => + new Promise((resolve, reject) => { + const proc = spawn("node", ["./bin/run", ...args], { stdio: ["pipe", "pipe", "pipe"] }); + let stdout = ""; + let stderr = ""; + proc.stdout.on("data", (data: Buffer) => (stdout += data.toString())); + proc.stderr.on("data", (data: Buffer) => (stderr += data.toString())); + const timer = setTimeout(() => { + proc.kill("SIGKILL"); + reject(new Error(`Command timed out after ${timeoutMs}ms: bitsocial ${args.join(" ")}\nstdout: ${stdout}\nstderr: ${stderr}`)); + }, timeoutMs); + proc.on("close", (exitCode) => { + clearTimeout(timer); + resolve({ stdout, stderr, exitCode }); + }); + }); + +describe("fresh daemon start does not restart kubo (issue #143)", () => { + let daemonProcess: ManagedChildProcess | undefined; + let kuboApiUrl: string | undefined; + + afterAll(async () => { + if (daemonProcess) await stopPkcDaemon(daemonProcess); + if (kuboApiUrl) await ensureKuboNodeStopped(kuboApiUrl); + }, 60_000); + + it("pkc-js init finds the pre-seeded Routing config and never shuts kubo down", { timeout: 180_000 }, async () => { + const logDir = randomDirectory(); + const readDaemonLog = async (): Promise => { + const files = (await fsPromise.readdir(logDir).catch(() => [] as string[])).filter((f) => f.endsWith(".log")); + let combined = ""; + for (const file of files) combined += await fsPromise.readFile(path.join(logDir, file), "utf8"); + return combined; + }; + + const daemon = await startPkcDaemonWithDynamicPorts((e) => [ + "--logPath", + logDir, + "--pkcOptions.dataPath", + randomDirectory(), + "--pkcRpcUrl", + e.rpcWsUrl + ]); + daemonProcess = daemon.daemonProcess; + kuboApiUrl = daemon.kuboApiUrl; + + // A full `community create` forces pkc-js through its kubo interactions (routing setup, + // signer key import), so by the time it returns, the shutdown — if pkc-js decided on one — + // has long been issued and the daemon has logged the restart. + const createResult = await runBitsocialCommand( + ["community", "create", "--description", "issue 143 regression", "--pkcRpcUrl", daemon.rpcWsUrl], + 90_000 + ); + expect(createResult.exitCode, `stderr: ${createResult.stderr}\nstdout: ${createResult.stdout}`).toBe(0); + + // Bounded observation window: a restart in flight surfaces in the log within milliseconds + // of kubo's exit, so 5 quiet seconds after a successful create means no restart happened. + const restartAppeared = await waitForCondition( + async () => (await readDaemonLog()).includes("Will attempt to restart it"), + 5_000, + 250 + ); + const logContent = await readDaemonLog(); + expect(restartAppeared, "daemon restarted kubo during a fresh start (pkc-js issued a shutdown)").toBe(false); + const kuboStarts = logContent.match(/Started kubo ipfs process with pid/g) ?? []; + expect(kuboStarts, "daemon started kubo more than once during a fresh start").toHaveLength(1); + }); +});