fix: pre-seed kubo Routing config so pkc-js init does not restart kubo - #144
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe daemon now passes HTTP router options to Kubo startup. Startup pre-seeds matching routing configuration before Kubo spawns. A regression test verifies that a fresh daemon start performs one Kubo start without a restart. ChangesHTTP router startup configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The daemon now persists routing settings before starting Kubo, avoiding an unnecessary restart during initialization. Because the live configuration file is rewritten directly, an interrupted write or concurrent local change could leave Kubo unable to start until the configuration is repaired; this is a bounded mergeable risk requiring owner awareness. Sequence Diagram(s)sequenceDiagram
participant Daemon
participant startKuboNode
participant KuboConfig
participant Kubo
participant pkc-js
Daemon->>startKuboNode: pass HTTP router options
startKuboNode->>KuboConfig: pre-seed Routing configuration
startKuboNode->>Kubo: spawn Kubo
pkc-js->>Kubo: initialize router configuration
Kubo-->>Daemon: remain running without restart
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
src/cli/commands/daemon.tsOops! Something went wrong! :( ESLint: 8.27.0 Error: ESLint configuration in --config » eslint-config-oclif is invalid:
Referenced from: /.eslintrc src/ipfs/startIpfs.tsESLint skipped: the matched ESLint configuration already failed (config-incompatibility). test/cli/daemon-no-kubo-restart-on-fresh-start.test.tsESLint skipped: the matched ESLint configuration already failed (config-incompatibility). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Problem
pkc-js (>= 0.0.46, still true in 0.0.89) rewrites the connected kubo node's
Routingconfig during its init (setupKuboHttpRouters) and POSTs/shutdownto kubo whenever the router endpoint set changed — always true on a repo pkc-js hasn't configured yet. The daemon'skeepKuboUprestarts kubo as pkc-js expects, but that opens a multi-second window right after the ready banner where kubo's API refuses connections. Early CLI commands can burn their whole budget inside it: on windows-latest CI (run 33471620931),bitsocial community createblocked on pkc-js's signer-key import retryingECONNREFUSEDagainst the restarting kubo and blew the completion-time test's 10s budget.Fix
pkc-js builds its desired
Routingvalue deterministically fromhttpRoutersOptionsand only shuts kubo down when the endpoint set differs. The daemon knows those options (it passes them to pkc-js), sostartKuboNodenow writes the equivalentRoutingconfig (plusProvide.DHT.SweepEnabled=false) into the kubo config file before spawning kubo, on every start (likeensureIpnsPubsubEnabled) — covering fresh repos and router-list changes on existing repos alike. pkc-js's endpoint comparison then sees no change and never issues the shutdown.Notes:
_mergeRouterConfigsstarts from its own Routers set) — so this preserves no less user state than pkc-js itself would.keepKuboUpand the restart machinery stay: still needed for external kubo nodes and as a safety net. If a pkc-js upgrade changes its Routing mapping, behavior degrades gracefully back to a one-time restart, and the new regression test catches that at upgrade time (pkc-js versions are pinned exactly).Test
test/cli/daemon-no-kubo-restart-on-fresh-start.test.ts: fresh daemon start + a fullcommunity create(which forces pkc-js through its kubo interactions), then asserts the daemon log has no "Will attempt to restart it" and exactly one "Started kubo ipfs process with pid". Red before the fix (restart observed ~50ms after the ready banner), green after. pkc-js's own router-setup log lines never reach the daemon log file (its bundled logger doesn't pick up the daemon's debug config), so the assertions anchor on the daemon's own logging.Full suite: 44 files, 342 passed, 1 skipped.
Closes #143
Summary by CodeRabbit
Bug Fixes
Tests