From 65f3c589fe7b65bb6c2b610b605fb92a4e78eaa7 Mon Sep 17 00:00:00 2001 From: Rinse Date: Tue, 1 Sep 2026 10:14:59 +0000 Subject: [PATCH] fix: pre-seed kubo Routing config so pkc-js init does not restart kubo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pkc-js (>= 0.0.46) rewrites the connected kubo node's Routing config during its init and POSTs /shutdown to kubo whenever the router endpoint set changed — always true on a repo pkc-js hasn't configured yet — expecting the daemon's keepKuboUp to restart it. That restart opens a multi-second window right after the ready banner where kubo's API refuses connections, and early CLI commands can burn their whole budget inside it (observed as a community-create timeout on windows CI). Write the equivalent Routing config (plus Provide.DHT.SweepEnabled=false) into the kubo config file before spawning kubo, on every start: pkc-js's endpoint comparison then sees no change and never issues the shutdown. Running on every start (like ensureIpnsPubsubEnabled) also covers router list changes on existing repos. Routing is effectively owned by pkc-js — it overwrites the section unconditionally at init — so this preserves no less user state than pkc-js itself would. If a pkc-js upgrade changes its mapping, behavior degrades back to a one-time restart and the new regression test catches it at upgrade time. --- src/cli/commands/daemon.ts | 3 +- src/ipfs/startIpfs.ts | 57 +++++++++- ...mon-no-kubo-restart-on-fresh-start.test.ts | 101 ++++++++++++++++++ 3 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 test/cli/daemon-no-kubo-restart-on-fresh-start.test.ts 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); + }); +});