feat: version update check in selftune doctor#60
Merged
WellDunDun merged 5 commits intomainfrom Mar 16, 2026
Merged
Conversation
- doctor now checks npm registry for newer versions (3s timeout, non-blocking) - Warns when installed version is behind latest with update instructions - Add "Updating" section to README explaining skill+CLI ship together - doctor() is now async; all callers updated Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cli/selftune/badge/badge.ts (1)
33-43:⚠️ Potential issue | 🔴 CriticalMake
cliMain()async before awaitingdoctor().Line 74 uses
awaitinside a non-async function, which is a parse-time error. ChangecliMain()to returnPromise<void>, then update the direct-entry guard and thebadgeroute incli/selftune/index.tsto properly await/catch the result so failures propagate correctly.🐛 Proposed fix
-export function cliMain(): void { +export async function cliMain(): Promise<void> { const { values } = parseArgs({ args: process.argv.slice(2), options: { skill: { type: "string" }, format: { type: "string" },if (import.meta.main) { - cliMain(); + cliMain().catch((err) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`[FATAL] ${message}`); + process.exit(1); + }); }case "badge": { const { cliMain } = await import("./badge/badge.js"); - cliMain(); + await cliMain(); break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/selftune/badge/badge.ts` around lines 33 - 43, Make cliMain() async and return Promise<void> so it can await doctor(): change the function signature of cliMain() to async function cliMain(): Promise<void> and update any internal awaits (e.g., await doctor()) accordingly; then update the direct-entry guard (the block that checks require.main === module) to call and await cliMain() and handle rejections (e.g., cliMain().catch(err => { /* log and process.exit(1) */ })), and likewise update the badge route in cli/selftune/index.ts to await the call to cliMain() (or handle the returned Promise) and propagate or catch errors so failures are not swallowed. Ensure references: cliMain(), doctor(), and the badge route caller are updated to use async/await or Promise.then/.catch.cli/selftune/dashboard-server.ts (1)
49-55:⚠️ Potential issue | 🔴 CriticalUpdate
statusLoadertype and await the async result.Line 103 made
computeStatusFromLogs()async, butstatusLoaderat line 54 is still typed as returningStatusResult(not a Promise). At line 488,cachedStatusResult = getStatusResult();assigns withoutawait, so whencomputeStatusFromLogsis used as the default, aPromise<StatusResult>gets stored instead of the resolved value. The type assertion at line 494 masks this mismatch, but downstream code accessingstatusResult.skillswill fail at runtime.Fix by allowing the Promise return and awaiting the assignment:
Proposed fix
export interface DashboardServerOptions { port?: number; host?: string; spaDir?: string; openBrowser?: boolean; - statusLoader?: () => StatusResult; + statusLoader?: () => StatusResult | Promise<StatusResult>; evidenceLoader?: () => EvolutionEvidenceEntry[]; overviewLoader?: () => OverviewResponse; skillReportLoader?: (skillName: string) => SkillReportResponse | null; actionRunner?: typeof runAction; } statusRefreshPromise = (async () => { - cachedStatusResult = getStatusResult(); + cachedStatusResult = await getStatusResult(); lastStatusCacheRefreshAt = Date.now(); })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/selftune/dashboard-server.ts` around lines 49 - 55, The current DashboardServerOptions.statusLoader is typed as () => StatusResult but computeStatusFromLogs is async, causing getStatusResult() to return a Promise that gets stored into cachedStatusResult without awaiting and masked by a type assertion; update DashboardServerOptions.statusLoader to return Promise<StatusResult> (i.e., () => Promise<StatusResult>), change any default assignment that uses computeStatusFromLogs to return that Promise, and in the code where cachedStatusResult = getStatusResult() (and where you read it, e.g., before accessing statusResult.skills) await getStatusResult() and remove the unsafe type assertion so cachedStatusResult holds a resolved StatusResult; ensure functions computeStatusFromLogs, getStatusResult, and any callers all use the Promise-returning signature consistently.cli/selftune/status.ts (1)
327-333:⚠️ Potential issue | 🔴 CriticalFix syntax error:
awaitrequires async function.Line 333 uses
await doctor()butcliMain()is not declaredasync. This will fail at parse time.Proposed fix
-export function cliMain(): void { +export async function cliMain(): Promise<void> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/selftune/status.ts` around lines 327 - 333, The function cliMain currently calls await doctor() but is not declared async; update the cliMain declaration to be async (e.g., async function cliMain) and change its return type to Promise<void> if applicable so awaiting doctor() is valid; ensure any places that call cliMain handle the returned Promise (await or .then) as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/selftune/observability.ts`:
- Around line 219-224: The fetch timeout handling and version comparison in the
selftune update check need fixes: move clearTimeout(timeout) into a finally
block so the abort timer is always cleared (wrap the fetch and response handling
in try { ... } finally { clearTimeout(timeout); }), ensure you still call
controller.abort() on timeout as currently done, and replace the strict equality
check against the fetched version with a semver-aware comparison (use a semver
utility like semver.compare/lt or semver.gt) when comparing the local package
version to the fetched "latest" from the registry (referencing the
AbortController instance, the timeout variable, the fetch call, and the version
equality logic) so versions like 0.2.4 are not incorrectly marked outdated vs
0.2.3.
---
Outside diff comments:
In `@cli/selftune/badge/badge.ts`:
- Around line 33-43: Make cliMain() async and return Promise<void> so it can
await doctor(): change the function signature of cliMain() to async function
cliMain(): Promise<void> and update any internal awaits (e.g., await doctor())
accordingly; then update the direct-entry guard (the block that checks
require.main === module) to call and await cliMain() and handle rejections
(e.g., cliMain().catch(err => { /* log and process.exit(1) */ })), and likewise
update the badge route in cli/selftune/index.ts to await the call to cliMain()
(or handle the returned Promise) and propagate or catch errors so failures are
not swallowed. Ensure references: cliMain(), doctor(), and the badge route
caller are updated to use async/await or Promise.then/.catch.
In `@cli/selftune/dashboard-server.ts`:
- Around line 49-55: The current DashboardServerOptions.statusLoader is typed as
() => StatusResult but computeStatusFromLogs is async, causing getStatusResult()
to return a Promise that gets stored into cachedStatusResult without awaiting
and masked by a type assertion; update DashboardServerOptions.statusLoader to
return Promise<StatusResult> (i.e., () => Promise<StatusResult>), change any
default assignment that uses computeStatusFromLogs to return that Promise, and
in the code where cachedStatusResult = getStatusResult() (and where you read it,
e.g., before accessing statusResult.skills) await getStatusResult() and remove
the unsafe type assertion so cachedStatusResult holds a resolved StatusResult;
ensure functions computeStatusFromLogs, getStatusResult, and any callers all use
the Promise-returning signature consistently.
In `@cli/selftune/status.ts`:
- Around line 327-333: The function cliMain currently calls await doctor() but
is not declared async; update the cliMain declaration to be async (e.g., async
function cliMain) and change its return type to Promise<void> if applicable so
awaiting doctor() is valid; ensure any places that call cliMain handle the
returned Promise (await or .then) as needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7e22862-2d9a-4893-bd26-49030941dfac
📒 Files selected for processing (11)
README.mdcli/selftune/badge/badge.tscli/selftune/dashboard-server.tscli/selftune/index.tscli/selftune/init.tscli/selftune/observability.tscli/selftune/orchestrate.tscli/selftune/quickstart.tscli/selftune/status.tspackage.jsontests/observability.test.ts
- Replace strict equality with semver-aware comparison (dev versions ahead of npm latest are not flagged as outdated) - Move clearTimeout into finally block so abort timer is always cleared Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update biome.json schema version 2.4.6 → 2.4.7 - Make badge.ts and status.ts cliMain async for await doctor() - Use optional chaining in evolve-body.ts and evolve.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
selftune doctornow checks npm registry for newer versions (3s timeout)doctor()is now async; all 8 callers updatedTest plan
selftune doctorshowsversion_up_to_date: passwhen currentselftune doctorshowsversion_up_to_date: warnwhen behind🤖 Generated with Claude Code