feat(daemon): periodic IPFS repo GC over kubo RPC, upgrade kubo 0.43.0 + pkc-js 0.0.77 - #120
Conversation
…0 + pkc-js 0.0.77 Upgrades kubo 0.42.0 -> 0.43.0 and @pkcprotocol/pkc-js 0.0.73 -> 0.0.77, and adds a periodic repo GC so a long-running daemon reclaims unpinned blocks. Without it one production node reached ~190GB against a 10GB StorageMax and exhausted disk and inodes. GC is driven over the kubo RPC API on our own schedule rather than via kubo's --enable-gc daemon flag. A daemon started with --enable-gc never exits in response to POST /api/v0/shutdown, lingering as a half-shutdown zombie (SIGTERM still works). pkc-js POSTs that endpoint when it rewrites the kubo Routing config on first connect and relies on the daemon restarting kubo, so the flag wedges the supervision loop -- it fails 4 kubo restart tests deterministically. Reported upstream as ipfs/kubo#11424; reproduces on 0.24.0, 0.42.0 and 0.43.0, which is also why the earlier attempt at the flag (f228d7d) was reverted two days later. The scheduler mirrors kubo's own policy: check hourly, GC only once the repo passes 90% of Datastore.StorageMax, and use size-only repo stats so it does not walk the entire flatfs blockstore. It is single-flighted, its timer is unref'd, and errors are contained inside the tick so a rejected interval callback cannot take the daemon down. This also covers a gap in pkc-js, which only GCs from a started local community's IPNS sync -- a daemon that is up with no community started would otherwise never reclaim anything. New flags: --enableIpfsGc / --no-enableIpfsGc (default on) and --ipfsGcIntervalMinutes (default 60). Datastore.StorageMax is left at kubo's 10GB default.
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe daemon now runs optional, scheduled IPFS repository garbage collection through Kubo RPC. New flags control the feature and interval. The implementation applies storage watermarks, prevents overlapping runs, handles failures, and stops cleanly. Tests cover execution, scheduling, and RPC behavior. ChangesIPFS repository garbage collection
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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: 3
🤖 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 739-749: Add daemon-level tests covering the enableIpfsGc default,
disabling it with --no-enableIpfsGc, conversion of ipfsGcIntervalMinutes to
milliseconds, and invocation of the scheduler’s stop function during shutdown.
Exercise the daemon startup and lifecycle path containing startRepoGcScheduler,
while keeping direct scheduler behavior covered by repoGc.test.ts.
In `@src/ipfs/repoGc.ts`:
- Around line 70-188: Update the shared TypeScript configuration used by
config/tsconfig.json to set rootDir to ../src, ensuring the included source
files remain within the configured root. Add the missing `@tsconfig/node20` dev
dependency required by the base configuration, then verify npm run build and npm
run build:test complete successfully.
- Around line 42-50: Update readRepoStat to parse RepoSize and StorageMax from
body.SizeStat when present, falling back to body for compatibility, then apply
the existing numeric defaults. Update the repo/stat fixtures in repoGc tests to
return the documented nested SizeStat response shape.
🪄 Autofix
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 Plus
Run ID: 07a0384f-0374-4bcb-877a-738a3316589c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
README.mdpackage.jsonsrc/cli/commands/daemon.tssrc/ipfs/repoGc.tstest/kubo/repoGc.test.ts
| // Runs against whichever kubo the daemon ends up talking to, including one started by | ||
| // another program (--pkcOptions.kuboRpcClientsOptions). pkc-js also GCs on the same | ||
| // watermark, but only from a started local community's IPNS sync — a daemon that is up | ||
| // with no community started would otherwise never reclaim anything (issue #119). | ||
| if (flags.enableIpfsGc) | ||
| stopRepoGcScheduler = startRepoGcScheduler({ | ||
| kuboApiUrl: kuboRpcEndpoint, | ||
| intervalMs: flags.ipfsGcIntervalMinutes * 60 * 1000, | ||
| log: PKCLogger("bitsocial-cli:ipfs:repoGc") | ||
| }); | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add daemon-level coverage for the GC flags and scheduler lifecycle.
test/kubo/repoGc.test.ts tests the scheduler directly. Add a daemon test for the default enabled state, --no-enableIpfsGc, minute-to-millisecond conversion, and shutdown calling the stop function.
As per coding guidelines, “Add a test when you add a feature or fix a bug.”
🤖 Prompt for 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.
In `@src/cli/commands/daemon.ts` around lines 739 - 749, Add daemon-level tests
covering the enableIpfsGc default, disabling it with --no-enableIpfsGc,
conversion of ipfsGcIntervalMinutes to milliseconds, and invocation of the
scheduler’s stop function during shutdown. Exercise the daemon startup and
lifecycle path containing startRepoGcScheduler, while keeping direct scheduler
behavior covered by repoGc.test.ts.
Source: Coding guidelines
| async function readRepoStat( | ||
| fetchImpl: FetchLike, | ||
| apiBase: string, | ||
| signal?: AbortSignal | ||
| ): Promise<{ repoSize: number; storageMax: number }> { | ||
| const response = await postRpc(fetchImpl, apiBase, "repo/stat?size-only=true", signal); | ||
| const body = (await response.json()) as { RepoSize?: number; StorageMax?: number }; | ||
| return { repoSize: Number(body.RepoSize ?? 0), storageMax: Number(body.StorageMax ?? 0) }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
: "${KUBO_RPC_URL:?Set KUBO_RPC_URL to the Kubo /api/v0 endpoint}"
curl -fsS -X POST "${KUBO_RPC_URL%/}/repo/stat?size-only=true" |
jq -e '(.SizeStat // .) | has("RepoSize") and has("StorageMax")'Repository: bitsocialnet/bitsocial-cli
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate repoGc and tests =="
fd -a 'repoGc\.(ts|test\.ts)$' . || true
echo "== file outlines =="
for f in $(fd 'repoGc\.(ts|test\.ts)$' .); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" || true
done
echo "== relevant source =="
f="$(fd 'repoGc\.ts$' . | head -n 1 || true)"
if [ -n "${f:-}" ]; then
cat -n "$f"
fi
echo "== relevant tests =="
tf="$(fd 'repoGc\.test\.ts$' . | head -n 1 || true)"
if [ -n "${tf:-}" ]; then
cat -n "$tf"
fi
echo "== references to SizeStat/repo/stat =="
rg -n "SizeStat|repo/stat|readRepoStat|storageMax|repoSize" .Repository: bitsocialnet/bitsocial-cli
Length of output: 29329
🌐 Web query:
Kubo API repo/stat response SizeStat RepoSize StorageMax
💡 Result:
The Kubo API endpoint repo/stat (and the corresponding stats/repo command) provides information regarding the local IPFS repository [1][2]. In the response structure, RepoSize and StorageMax are contained within a nested object called SizeStat [1][2]. The response format is as follows: - NumObjects (uint64): The number of objects currently stored in the repository [1][2]. - RepoPath (string): The file system path to the repository [1][2]. - SizeStat (object): A nested object containing size information [2]. - RepoSize (uint64): The size of the repository in bytes [3][2]. - StorageMax (uint64): The maximum storage limit for the repository in bytes, as defined in the configuration [4][5][2]. - Version (string): The version of the repository format [1][2]. Note that SizeStat is a grouping mechanism used to organize these specific storage metrics within the JSON response [1][2]. While repo stat is a common command-line interface command, stats/repo is the corresponding RPC API endpoint [2].
Citations:
- 1: https://apis.io/schemas/ipfs/kubo-rpc-responses/
- 2: https://apis.io/apis/ipfs/ipfs-stats-api/
- 3: https://docs.ipfs.tech/reference/kubo/cli/
- 4: Add MaxStorage field to output of "repo stat". ipfs/kubo#3915
- 5: Let "ipfs repo stat" include StorageMax as set in the configuration (in bytes) ipfs/kubo#3836
Parse repo/stat from SizeStat.
Kubo documents RepoSize and StorageMax under SizeStat, but readRepoStat reads only top-level fields. With a real Kubo response, both become 0; the watermark check then runs when storageMax is 0, so GC skips the intended 90% threshold. Update readRepoStat to read body.SizeStat ?? body, and update the repo/stat stubs in test/kubo/repoGc.test.ts to match the nested shape.
🤖 Prompt for 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.
In `@src/ipfs/repoGc.ts` around lines 42 - 50, Update readRepoStat to parse
RepoSize and StorageMax from body.SizeStat when present, falling back to body
for compatibility, then apply the existing numeric defaults. Update the
repo/stat fixtures in repoGc tests to return the documented nested SizeStat
response shape.
| export async function runRepoGcIfDue(options: { | ||
| kuboApiUrl: URL | string; | ||
| log?: any; | ||
| force?: boolean; | ||
| signal?: AbortSignal; | ||
| fetchImpl?: FetchLike; | ||
| }): Promise<RepoGcOutcome> { | ||
| const log = options.log ?? PKCLogger("bitsocial-cli:ipfs:repoGc"); | ||
| const fetchImpl = options.fetchImpl ?? (globalThis.fetch as unknown as FetchLike); | ||
| const apiBase = toConnectableApiBase(options.kuboApiUrl); | ||
|
|
||
| let repoSizeBefore: number | undefined; | ||
| let storageMax: number | undefined; | ||
|
|
||
| if (!options.force) { | ||
| let stat: { repoSize: number; storageMax: number }; | ||
| try { | ||
| stat = await readRepoStat(fetchImpl, apiBase, options.signal); | ||
| } catch (error) { | ||
| // A daemon we can't stat is one to back off from, not to blindly GC. | ||
| log.error?.("Skipping repo gc: failed to read repo/stat from the kubo node", apiBase, error); | ||
| return { ran: false, skippedReason: "repo-stat-failed" }; | ||
| } | ||
| repoSizeBefore = stat.repoSize; | ||
| storageMax = stat.storageMax; | ||
|
|
||
| // storageMax comes from Datastore.StorageMax. If the daemon reports no ceiling there is | ||
| // nothing to compare against, so fall back to GCing on the interval alone rather than | ||
| // never GCing at all. | ||
| if (stat.storageMax > 0) { | ||
| const threshold = stat.storageMax * GC_HIGH_WATERMARK; | ||
| if (stat.repoSize < threshold) { | ||
| log.trace?.( | ||
| `Skipping repo gc on ${apiBase} - repo size ${stat.repoSize} is below the ${GC_HIGH_WATERMARK * 100}% watermark ${threshold} of StorageMax ${stat.storageMax}` | ||
| ); | ||
| return { ran: false, skippedReason: "below-watermark", repoSizeBefore, storageMax }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let reclaimedCids = 0; | ||
| try { | ||
| const response = await postRpc(fetchImpl, apiBase, "repo/gc?quiet=true", options.signal); | ||
| // repo/gc streams newline-delimited JSON, one object per reclaimed CID. Draining it fully | ||
| // is what makes this await mean "GC finished" rather than "GC started". | ||
| const text = await response.text(); | ||
| for (const line of text.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed) continue; | ||
| try { | ||
| const parsed = JSON.parse(trimmed) as { Key?: unknown; Error?: string }; | ||
| if (parsed.Error) log.error?.("Failed to GC a block out of the ipfs repo", parsed.Error); | ||
| else if (parsed.Key) reclaimedCids++; | ||
| } catch { | ||
| // A malformed line is not worth aborting a completed GC over. | ||
| } | ||
| } | ||
| } catch (error) { | ||
| log.error?.("Failed to GC ipfs repo", apiBase, error); | ||
| return { ran: false, skippedReason: "gc-failed", repoSizeBefore, storageMax }; | ||
| } | ||
|
|
||
| let repoSizeAfter: number | undefined; | ||
| try { | ||
| repoSizeAfter = (await readRepoStat(fetchImpl, apiBase, options.signal)).repoSize; | ||
| } catch (error) { | ||
| log.trace?.("repo gc finished but the follow-up repo/stat failed", error); | ||
| } | ||
|
|
||
| // How much a GC actually reclaims is worth logging rather than assuming: GC never touches | ||
| // pinned data, and a node with thousands of recursive pins can stay over the watermark. | ||
| log( | ||
| `GC reclaimed ${reclaimedCids} cids from the IPFS node ${apiBase} - repo size ${repoSizeBefore ?? "unknown"} -> ${repoSizeAfter ?? "unknown"}` | ||
| ); | ||
| return { ran: true, reclaimedCids, repoSizeBefore, repoSizeAfter, storageMax }; | ||
| } | ||
|
|
||
| /** | ||
| * Starts the periodic repo GC. Returns a stop function. | ||
| * | ||
| * The timer is unref'd so it never by itself keeps the daemon process alive, and runs are | ||
| * single-flighted: a GC that outlives its own interval must not have a second one stacked on top | ||
| * of it. Errors are swallowed inside the tick — an interval callback that rejects becomes an | ||
| * unhandledRejection and takes the daemon down (same failure mode as issue #37 bug 3). | ||
| */ | ||
| export function startRepoGcScheduler(options: { | ||
| kuboApiUrl: URL | string; | ||
| intervalMs?: number; | ||
| log?: any; | ||
| fetchImpl?: FetchLike; | ||
| }): () => void { | ||
| const log = options.log ?? PKCLogger("bitsocial-cli:ipfs:repoGc"); | ||
| const intervalMs = options.intervalMs ?? DEFAULT_REPO_GC_INTERVAL_MS; | ||
| const abortController = new AbortController(); | ||
| let inFlight: Promise<unknown> | undefined; | ||
|
|
||
| const tick = () => { | ||
| if (inFlight) return; | ||
| inFlight = runRepoGcIfDue({ | ||
| kuboApiUrl: options.kuboApiUrl, | ||
| log, | ||
| signal: abortController.signal, | ||
| fetchImpl: options.fetchImpl | ||
| }) | ||
| .catch((error) => log.error?.("repo gc tick error (will retry next interval)", error)) | ||
| .finally(() => { | ||
| inFlight = undefined; | ||
| }); | ||
| }; | ||
|
|
||
| const timer = setInterval(tick, intervalMs); | ||
| timer.unref?.(); | ||
| log(`Scheduled IPFS repo GC every ${intervalMs}ms against ${String(options.kuboApiUrl)}`); | ||
|
|
||
| return () => { | ||
| clearInterval(timer); | ||
| abortController.abort(); | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
npm run build && npm run build:testRepository: bitsocialnet/bitsocial-cli
Length of output: 7389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- package scripts ---\n'
node -e "const p=require('./package.json'); console.log(JSON.stringify(p.scripts, null, 2))"
printf '\n--- tsconfig relevant settings ---\n'
cat -n config/tsconfig.json | sed -n '1,60p'
printf '\n--- node_modules availability ---\n'
test -d node_modules/typescript && echo "node_modules/typescript exists" || echo "node_modules/typescript missing"
test -d node_modules/@tsconfig/node20 && echo "node_modules/@tsconfig/node20 exists" || echo "node_modules/@tsconfig/node20 missing"Repository: bitsocialnet/bitsocial-cli
Length of output: 255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package scripts ---'
node -e "const p=require('./package.json'); console.log(JSON.stringify(p.scripts, null, 2))"
printf '%s\n' ''
printf '%s\n' '--- tsconfig relevant settings ---'
cat -n config/tsconfig.json | sed -n '1,60p'
printf '%s\n' ''
printf '%s\n' '--- node_modules availability ---'
test -d node_modules/typescript && echo "node_modules/typescript exists" || echo "node_modules/typescript missing"
test -d node_modules/@tsconfig/node20 && echo "node_modules/@tsconfig/node20 exists" || echo "node_modules/@tsconfig/node20 missing"Repository: bitsocialnet/bitsocial-cli
Length of output: 2468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
npm run build && npm run build:testRepository: bitsocialnet/bitsocial-cli
Length of output: 7389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
npm run build && npm run build:testRepository: bitsocialnet/bitsocial-cli
Length of output: 7389
Make the configured TypeScript builds pass.
npm run build && npm run build:test exits with TS6059 because config/tsconfig.json includes ../src/**/* while rootDir is defaulted to config via the absent @tsconfig/node20/tsconfig.json base. Set the common rootDir to ../src and include the missing dev dependency so the build can succeed.
🤖 Prompt for 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.
In `@src/ipfs/repoGc.ts` around lines 70 - 188, Update the shared TypeScript
configuration used by config/tsconfig.json to set rootDir to ../src, ensuring
the included source files remain within the configured root. Add the missing
`@tsconfig/node20` dev dependency required by the base configuration, then verify
npm run build and npm run build:test complete successfully.
Source: Coding guidelines
Drops the repo/stat watermark check. GC only ever reclaims unpinned blocks, so kubo already decides what is collectable -- reading Datastore.StorageMax to gate the call just duplicated that decision on our side. Removes both repo/stat calls (the pre-GC watermark check and the post-GC size probe), GC_HIGH_WATERMARK and the `force` option. `runRepoGcIfDue` is now `runRepoGc`, which POSTs repo/gc and counts reclaimed cids from the response stream. Behaviour change: GC now runs on every tick regardless of repo size, where before it was skipped below 90% of StorageMax. Scheduling is unchanged -- hourly by default, single-flighted, unref'd timer, errors contained inside the tick.
|
Simplified per review: dropped the Removed both Behaviour change: GC now runs on every tick regardless of repo size, where before it was skipped below 90% of Scheduling is unchanged: hourly by default, single-flighted so a long GC never has a second stacked on it, Re-verified against a live kubo 0.43.0 node with the repo at ~0.01% of Tests updated: 9 unit tests (down from 12, since the watermark cases no longer exist), including a case asserting |
1 participant
Closes #119
What
kubo0.42.0 -> 0.43.0,@pkcprotocol/pkc-js0.0.73 -> 0.0.77src/ipfs/repoGc.ts: periodic IPFS repo garbage collection driven over the kubo RPC API--enableIpfsGc/--no-enableIpfsGc(default on) and--ipfsGcIntervalMinutes(default 60)Datastore.StorageMaxleft at kubo's 10GB defaultWhy not
--enable-gcThe obvious implementation is kubo's own
--enable-gcdaemon flag, which already does hourly, watermark-gated GC. It does not work here.A daemon started with
--enable-gcnever exits in response toPOST /api/v0/shutdown. It logscannot access config, repo not openand lingers as a half-shutdown zombie; SIGTERM still works. pkc-js POSTs that exact endpoint when it rewrites the kubo Routing config on first connect and relies on the daemon restarting kubo afterwards, so the flag wedges the supervision loop.Controlled A/B on an idle machine, same deps, only the flag differing:
test/cli/daemon.test.ts--enable-gc--enable-gcStandalone repro, no bitsocial-cli involved:
--enable-gcPOST /api/v0/shutdownRoot cause:
maybeRunGCgivesPeriodicGCthe command request context, but the shutdown command only callsnd.Close(), which cancels the node context — sogcErrcnever closes anddaemonFuncblocks forever drainingmerge(...). Every other channel in that merge is node-context-wired;gcErrcis the lone exception.Reported upstream as ipfs/kubo#11424. It reproduces on 0.24.0, 0.42.0 and 0.43.0, so it is long-standing — and 0.24.0 is the era of f228d7d, meaning this is what caused that Dec 2023 revert, not the MFS/GC wedge described in #119. The MFS wedge was real but coincidental, and is genuinely fixed in 0.43.0 (verified: 64 rounds of
files write+files statagainst GC every 5s at a 1MB ceiling, MFS fully intact, all 64 unpinned garbage blocks reclaimed).How the scheduler behaves
Mirrors kubo's own policy rather than inventing a second one:
--ipfsGcIntervalMinutes(default 60, matchingDatastore.GCPeriod)POST /api/v0/repo/stat?size-only=true, and only GCs onceRepoSize >= 90% of StorageMax(matchingDatastore.StorageGCWatermark)size-onlyis deliberate — the defaultrepo/statwalks the entire flatfs blockstore, which on the repos this exists for is millions of filesunref'd, and errors contained inside the tick so a rejected interval callback cannot take the daemon down--pkcOptions.kuboRpcClientsOptionsThis also closes a real gap: pkc-js GCs on the same watermark but only from a started local community's IPNS sync, so a daemon that is up with no community started would never reclaim anything.
Note GC only reclaims unpinned blocks. It bounds unpinned growth; it does not cap a repo whose bulk is pinned.
Testing
test/kubo/repoGc.test.tscovering the watermark skip, the GC path,size-only, the no-StorageMaxfallback,force, both failure paths, wildcard-address rewriting, interval/stop behaviour, single-flighting, and the unhandledRejection guardIpns.RecordLifetime/RepublishPeriod: the 0.43.0 startup validation is a non-issue — the CLI writes neither key, and 0.43.0 accepts the""that existing repos carry (verified against a repo seeded with production values)Summary by CodeRabbit
New Features
Documentation
Maintenance