fix(daemon): fail fast when PKC RPC port is already in use#47
Merged
Conversation
Previously the daemon silently switched into client mode when it detected another process on the RPC port (default ws://localhost:9138), then idled: it skipped kubo startup, never loaded challenge packages, never started its own daemon server, and the keepalive tick was gated off — so the second process did nothing useful while pretending to. Now the daemon checks the port up front and exits non-zero with a message that names the port and points the user at --pkcRpcUrl on the other commands (which already exists on BaseCommand and is the right escape hatch for talking to a running daemon). Removes the usingDifferentProcessRpc state and the connect-as-client branch in createOrConnectRpc; simplifies runKeepKuboUpTick deps; drops the two tests that asserted the old silent-takeover and adds a new "fails when PKC RPC port is already in use" integration test. Closes #46
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDaemon startup now fails fast if the configured PKC RPC port is occupied and removes the external-RPC attach branch and its flag; RPC creation and keepalive logic were simplified. Separately, IPFS child-process output handling was refactored with a new helper and tests for race/success cases. ChangesDaemon Port Conflict Fail-Fast
IPFS child-process output aggregation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/commands/daemon.ts`:
- Around line 434-436: The current check using tcpPortUsed.check(...) silently
returns when the RPC port is already taken, which can leave the daemon running
without its RPC/web UI; change the behavior in the port-taken branch inside the
daemon startup flow to throw a startup error instead of returning. Locate the
block that calls tcpPortUsed.check(Number(pkcRpcUrl.port), pkcRpcUrl.hostname)
and replace the early return with throwing a descriptive Error (include
pkcRpcUrl or its port/hostname in the message) so the startup fails fast when
the RPC port is occupied.
- Around line 286-294: The pre-flight port conflict check uses
tcpPortUsed.check(Number(pkcRpcUrl.port), pkcRpcUrl.hostname) which can miss
wildcard hostnames like 0.0.0.0/::; before calling tcpPortUsed.check (and
likewise before the reconnect check later) normalize the hostname via the same
toConnectableHostname() helper (or equivalent) used on the Kubo path so you pass
a connectable host instead of raw pkcRpcUrl.hostname; update the calls that
reference pkcRpcUrl.hostname to use the normalizedHost variable (keeping the
rest of the error message unchanged), and re-run npm run build && npm run
build:test after making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5cb4736-00a3-4403-98fd-6fb3032762aa
📒 Files selected for processing (3)
src/cli/commands/daemon.tstest/cli/daemon.test.tstest/cli/keep-kubo-up-tick.test.ts
💤 Files with no reviewable changes (1)
- test/cli/keep-kubo-up-tick.test.ts
Same darwin EINVAL gotcha that toConnectableHostname() already guards on the kubo path: a user passing --pkcRpcUrl ws://0.0.0.0:9138 would feed 0.0.0.0 straight to tcpPortUsed.check, which rejects it on macOS. Wrap the hostname in toConnectableHostname at all three RPC check sites — the fail-fast pre-check, createOrConnectRpc, and runKeepKuboUpTick.
createOrConnectRpc had an early `if (isRpcPortTaken) return` meant to be defensive. But startedOwnRpc already short-circuits every tick call once our server is up, so that branch is only reachable during initial startup in a TOCTOU race where the port becomes occupied between the pre-flight fail-fast check and the actual bind. Today the daemon silently continues with kubo running but no RPC server and no web UI; throw instead so startup fails fast with the port in the message.
Pulled the Promise wiring out of _spawnAsync into an exported
_gatherChildOutput so the race is unit-testable. Listen on 'close' (not
'exit'): Node guarantees 'close' fires after all stdio streams have
closed, so every stderr 'data' event has been delivered by the time we
build the rejection Error.
Why this matters: a fast-exiting child can deliver its exit signal
before its stderr drains. With the old 'exit' listener, _spawnAsync
rejected with errorMessage="". In startKuboNode the empty message
defeated the `error.message.includes("ipfs configuration file already
exists!")` suppression, turned a benign already-initialised repo into
"Failed to call ipfs init" — and because startKuboNode wraps an async
executor in `new Promise(...)`, that throw became an unhandledRejection
instead of propagating, hanging keepKuboUp() and letting the daemon
exit silently with code 0. That's the macos-latest CI failure on PR #47.
The regression test in test/kubo/gather-child-output.test.ts feeds a
fake EventEmitter the macOS event ordering (exit → data → close) and
asserts the rejection captures the stderr text. Cross-platform
deterministic, fails on the old 'exit' listener, passes on 'close'.
This was referenced May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #46.
bitsocial daemonpreviously silent-piggybacked on an existing daemon when its RPC port (defaultws://localhost:9138) was taken — connecting as a client and then idling: no kubo, no challenge loading, no web UI, keepalive gated off. Users got a second "daemon" that did nothing useful. Now it exits non-zero with a message pointing at--pkcRpcUrlon the other commands (which is the real escape hatch —BaseCommandalready exposes it forcommunity list, etc.).tcp-port-usedcheck indaemon.ts run()after flag validation and before any setup work.createOrConnectRpc, theusingDifferentProcessRpcstate, thekeepKuboUpguard, the tick deps field, and the call site.bitsocial daemon (relying on PKC RPC started by another process)describe block whose two tests asserted the removed behavior; addsfails when PKC RPC port is already in useto the existing port-validation describe.Test plan
npm run build && npm run build:testpassesnpm run test:cli— 26 files / 206 tests passingbitsocial daemonin one terminal; secondbitsocial daemonin another must exit non-zero with the new message;bitsocial community list --pkcRpcUrl ws://localhost:9138from a third terminal still works against the first daemonSummary by CodeRabbit
Bug Fixes
Refactor
Tests