feat(cli): add sorokeep doctor environment/connectivity diagnostics - #662
Conversation
|
Warning Review limit reached
Next review available in: 24 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds a ChangesDoctor diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant DoctorCommand
participant runDiagnostics
participant Database
participant StellarRpcClient
CLI->>DoctorCommand: Invoke doctor
DoctorCommand->>runDiagnostics: Run checks
runDiagnostics->>Database: Check schema and alert configuration
Database-->>runDiagnostics: Return database results
runDiagnostics->>StellarRpcClient: Check RPC reachability
StellarRpcClient-->>runDiagnostics: Return RPC result
runDiagnostics-->>DoctorCommand: Return diagnostic results
DoctorCommand-->>CLI: Print statuses and set exit code
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 review |
✅ Action performedReview finished.
|
|
2 similar comments
|
|
![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/core/doctor.ts`:
- Around line 39-42: Update the dataDir validation near fs.accessSync to also
verify that the path is a directory, not merely writable; use the existing
filesystem APIs to inspect dataDir and preserve the current creation and
writable-access checks.
- Around line 57-73: The schema diagnostic around getDatabase() must inspect the
database through a non-migrating connection before migrations run, rather than
using getDatabase(), and validate migration metadata plus required
tables/columns or other expected schema objects instead of only checking
contracts. Preserve the existing pass/fail result structure, and add a
regression test proving an outdated schema is reported as failed.
- Around line 121-123: Update the health-check flow around
StellarRpcClient.checkHealth() to enforce a bounded diagnostic timeout, since
the SDK call cannot reliably accept an AbortSignal. Configure the SDK server
timeout if supported by the client, or wrap the await with a timeout that allows
the existing failure path to report “rpc reachability: FAIL” before returning.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5e3d3797-6de8-4ce3-82e0-6b0357e015ff
📒 Files selected for processing (5)
src/cli/program.tssrc/commands/doctor.tssrc/core/doctor.tstests/commands/doctor.test.tstests/core/doctor.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: build-and-test (24.x)
- GitHub Check: build-and-test (22.x)
🔇 Additional comments (4)
tests/core/doctor.test.ts (1)
36-113: LGTM!src/commands/doctor.ts (1)
5-26: LGTM!src/cli/program.ts (1)
23-23: LGTM!Also applies to: 105-105
tests/commands/doctor.test.ts (1)
14-54: LGTM!
| try { | ||
| const client = new StellarRpcClient("testnet"); | ||
| await client.checkHealth(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For @stellar/stellar-sdk version 16.2.0, what timeout or AbortSignal API does rpc.Server.getHealth support?
💡 Result:
For @stellar/stellar-sdk version 16.2.0, the rpc.Server.getHealth method does not accept or support an AbortSignal API for timeout control [1][2]. Connection timeouts in the @stellar/stellar-sdk are managed exclusively through the timeout property defined in the Options interface when instantiating the rpc.Server [1][3]. This timeout option is a numeric value representing milliseconds, where a value of 0 indicates no timeout [1][3]. While you can configure this timeout on a per-server basis during initialization, the SDK does not provide a mechanism to pass an AbortSignal to individual method calls like getHealth [1][2]. Additionally, the SDK supports a global timeout configuration via the Config.setTimeout(value) method, which affects all SDK components unless overridden by the per-server setting [4][3].
Citations:
- 1: https://github.com/stellar/js-stellar-sdk/blob/main/src/rpc/server.ts
- 2: https://cdn.jsdelivr.net/npm/@stellar/stellar-sdk@16.1.0/lib/esm/rpc/server.d.ts
- 3: https://deepwiki.com/stellar/js-stellar-sdk/4.1-rpc-server-connection
- 4: https://stellar.github.io/js-stellar-sdk/reference/cross-cutting/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect relevant files without executing repository code.
printf 'Tracked files matching doctor.ts:\n'
git ls-files | grep -E '(^|/)doctor\.ts$' || true
printf '\ndiff stat:\n'
git diff --stat || true
printf '\npackage manifests mentioning stellar-sdk:\n'
for f in package.json */package.json package-lock.json */package-lock.json pnpm-lock.yaml yarn.lock; do
[ -f "$f" ] && { echo "--- $f"; grep -n "`@stellar/stellar-sdk`\|stellar-sdk" "$f" || true; }
done
printf '\nurllib? No, read nearby source:\n'
file="$(git ls-files | grep '(^|/)src/core/doctor\.ts$' | head -n 1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,180p' "$file" | cat -n
fi
printf '\nSearch StellarRpcClient usages/definition:\n'
rg -n "class StellarRpcClient|new StellarRpcClient|checkHealth|timeout" . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 676
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked doctor files:\n'
git ls-files | grep -E '(^|/)doctor\.ts$' || true
printf '\npackage manifests mentioning stellar-sdk:\n'
for f in package.json package-lock.json; do
if [ -f "$f" ]; then
echo "--- $f"
grep -n "`@stellar/stellar-sdk`\|stellar-sdk" "$f" || true
fi
done
printf '\nInspect src/core/doctor.ts:\n'
wc -l src/core/doctor.ts
cat -n src/core/doctor.ts | sed -n '1,180p'
printf '\nInspects src/commands/doctor.ts:\n'
wc -l src/commands/doctor.ts
cat -n src/commands/doctor.ts | sed -n '1,180p'
printf '\nSearch StellarRpcClient and timeout usage:\n'
rg -n "class StellarRpcClient|new StellarRpcClient|StellarRpcClient|checkHealth|\\.timeout|timeout\\s*:" . --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!package-lock.json' || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 19498
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Inspect src/rpc/client.ts focus:\n'
wc -l src/rpc/client.ts
cat -n src/rpc/client.ts | sed -n '240,320p'
printf '\nInspect tests around StellarRpcClient health/timeout:\n'
cat -n tests/rpc/client.test.ts | sed -n '166,220p'
printf '\nBehavioral probe: parse source for StellerRpcClient constructor/health options:\n'
python3 - <<'PY'
from pathlib import Path
import re
src = Path("src/rpc/client.ts").read_text()
m = re.search(r"class StellarRpcClient[\s\S]*?\n}", src)
body = m.group(0) if m else ""
print(body[-2400:])
PYRepository: AbdulmalikAlayande/sorokeep
Length of output: 9247
Set a deadline on StellarRpcClient.getHealth().
client.checkHealth() delegates to rpc.Server.getHealth() without a bounded client timeout, and @stellar/stellar-sdk 16.2.0 does not support pass-through AbortSignal cancellation for this call. A stalled DNS lookup or socket can leave sorokeep doctor pending before it can write the rpc reachability: FAIL result. Pass an SDK-configured server timeout or wrap client.checkHealth() with a diagnostic timeout.
🤖 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/core/doctor.ts` around lines 121 - 123, Update the health-check flow
around StellarRpcClient.checkHealth() to enforce a bounded diagnostic timeout,
since the SDK call cannot reliably accept an AbortSignal. Configure the SDK
server timeout if supported by the client, or wrap the await with a timeout that
allows the existing failure path to report “rpc reachability: FAIL” before
returning.
- validate the data directory is a directory, not merely writable - inspect the database schema on a read-only non-migrating connection so an outdated/missing schema is reported instead of silently repaired - bound the RPC health check with a timeout so a stalled connection cannot hang 'sorokeep doctor' - add regression tests for outdated schema, uninitialized database, non-directory data path, and credential check skip
|
@coderabbitai review |
|
3 participants
Replaces the closed #371 with a clean, single-feature diff rebuilt from the latest upstream main.
Summary
Adds a \sorokeep doctor\ command that runs environment and connectivity diagnostics for the local Sorokeep installation (config dir, database, RPC connectivity, alert channels).
Changes
Verification