Add ways to disable onboarding#245
Conversation
…r desktop commander client
WalkthroughAdds a CLI flag (--no-onboarding) and global variable (disableOnboarding) to control onboarding. Updates onboarding logic to short-circuit when the flag is set or when the client is desktop-commander. Logs informational/debug messages accordingly. Documentation updated with new Command Line Options and usage examples. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant CLI as CLI entry (src/index.ts)
participant Global as Global State
participant Tracker as usageTracker.shouldShowOnboarding
participant Client as server.js (dynamic)
User->>CLI: Launch app with args (e.g., --no-onboarding)
CLI->>Global: Set disableOnboarding = true/false
CLI->>Tracker: shouldShowOnboarding()
alt Flag set
Tracker->>Global: Read disableOnboarding
Tracker-->>CLI: false (skip onboarding)
else Flag not set
Tracker->>Client: import server.js
alt Client name is "desktop-commander"
Tracker-->>CLI: false (skip onboarding)
else Import fails or name differs
Tracker-->>CLI: existing gating logic (stats/state/time)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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
🧹 Nitpick comments (4)
src/types.ts (1)
4-7: Type the global instead of using(global as any); augment NodeJS.GlobalDefine these globals on Node’s
Globalto get proper typing across the codebase and avoidanycasts.Apply:
declare global { - var mcpTransport: FilteredStdioServerTransport | undefined; - var disableOnboarding: boolean | undefined; + var mcpTransport: FilteredStdioServerTransport | undefined; + var disableOnboarding: boolean | undefined; + + // Provide property types on `global` for runtime access + namespace NodeJS { + interface Global { + mcpTransport?: FilteredStdioServerTransport; + disableOnboarding?: boolean; + } + } }src/utils/usageTracker.ts (2)
385-400: Use typedglobaland structured logging; add debug when client gate disables onboarding
- Avoid
(global as any); rely on the typed global.- Emit a structured debug log when onboarding is disabled due to client detection.
- Prefer
logToStderroverconsole.logto keep stdout JSON-RPC clean.+import { logToStderr } from './logger.js'; ... -// Check if onboarding is disabled via command line argument -if ((global as any).disableOnboarding) { +// Check if onboarding is disabled via command line argument +if (global.disableOnboarding) { return false; } // Check if client is desktop-commander (disable for this client) try { const { currentClient } = await import('../server.js'); if (currentClient?.name === 'desktop-commander') { + logToStderr('debug', 'Onboarding disabled for desktop-commander client'); return false; } } catch (error) { // If we can't import server, continue with other checks - console.log('[ONBOARDING DEBUG] Could not check client name, continuing...'); + logToStderr('debug', '[ONBOARDING] Could not check client name, continuing...'); }Note: There are other
console.logcalls later in this file (e.g., Lines 465, 486, 499). Consider switching them tologToStderrin a follow-up for consistency.
391-395: Avoid hard-coded client stringExtract
'desktop-commander'to a shared constant (e.g., inutils/logger.tsor aconstants.ts) to keep the identifier consistent across logging and gating logic.README.md (1)
660-697: Add debug note for client-based disabling; optional env example
- You already document the flag-based log message. Add a line noting a debug log is emitted when onboarding is disabled for the
desktop-commanderclient.- If you adopt the
DC_DISABLE_ONBOARDINGenv var, add a short example.Example additions:
- “When the MCP client is ‘desktop-commander’, onboarding is disabled (debug log: ‘Onboarding disabled for desktop-commander client’).”
- “Alternatively set DC_DISABLE_ONBOARDING=1 to disable onboarding without CLI args.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(2 hunks)src/index.ts(1 hunks)src/types.ts(1 hunks)src/utils/usageTracker.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/index.ts (1)
src/utils/logger.ts (1)
logToStderr(71-82)
src/utils/usageTracker.ts (1)
src/server.ts (1)
currentClient(132-132)
| **Command line options during setup:** | ||
| - `--debug`: Enable debugging mode for Node.js inspector | ||
| - `--no-onboarding`: Disable onboarding prompts for new users | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Confirm: does setup honor --no-onboarding?
Docs list --no-onboarding under “Command line options during setup”, but the server entrypoint short-circuits on setup and the PR wires the flag for runtime only. Verify runSetup actually persists this flag (e.g., into Claude config args); otherwise, move this bullet to the runtime CLI section.
🏁 Script executed:
#!/bin/bash
# Check if setup pipeline consumes or persists --no-onboarding
rg -nC3 -S '\-\-no-onboarding|noOnboarding|disableOnboarding' --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for '--no-onboarding' or related identifiers..."
rg -nC3 -S '\-\-no-onboarding|noOnboarding|disableOnboarding' -g '!*node_modules/*' -g '!*dist/*' -g '!*build/*' || true
echo -e "\nSearching for runSetup definitions/usages..."
rg -nC3 -S '(\brunSetup\b|function\s+runSetup\b|export\s+function\s+runSetup\b|const\s+runSetup\b|async\s+function\s+runSetup\b)' -g '!*node_modules/*' -g '!*dist/*' -g '!*build/*' || true
echo -e "\nSearching for CLI parsers (commander|yargs|minimist|meow|caporal)..."
rg -nC3 -S '\b(commander|yargs|meow|minimist|caporal)\b' -g '!*node_modules/*' -g '!*dist/*' -g '!*build/*' || true
echo -e "\nSearching for process.argv usages..."
rg -nC3 -S 'process\.argv' -g '!*node_modules/*' -g '!*dist/*' -g '!*build/*' || true
if [ -f README.md ]; then
echo -e "\nREADME.md lines 90-93:"
nl -ba README.md | sed -n '90,93p' || true
fi
if [ -f package.json ]; then
echo -e "\npackage.json scripts:"
jq -r '.scripts' package.json || true
fi
exit 0Length of output: 2395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo root:"
ls -la || true
echo -e "\nFind files with 'setup-claude' in the name:"
find . -type f -iname "*setup-claude*" -print || true
echo -e "\nSearch for 'onboard' (onboarding, onboard) and '--no-onboarding' usages (exclude common build dirs):"
grep -RIn --binary-files=without-match --exclude-dir={node_modules,.git,dist,build} -e 'onboard' -e '--no-onboarding' -e 'noOnboarding' -e 'disableOnboarding' || true
echo -e "\nSearch for 'setup' script usages and 'process.argv' (to find flag parsing):"
grep -RIn --binary-files=without-match --exclude-dir={node_modules,.git,dist,build} -e 'process.argv' -e '\\bsetup\\b' -e 'runSetup' -e 'setup-claude-server' || true
echo -e "\nIf setup-claude-server.js exists at repo root or in dist, print first 300 lines:"
for f in setup-claude-server.js dist/setup-claude-server.js; do
if [ -f "$f" ]; then
echo "---- $f ----"
sed -n '1,300p' "$f" || true
else
echo "$f: not found"
fi
done
echo -e "\nShow README.md lines 90-93 (if present):"
if [ -f README.md ]; then
sed -n '90,93p' README.md || true
else
echo "README.md not found"
fiLength of output: 26979
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/index.ts ==="
sed -n '1,240p' src/index.ts || true
echo -e "\n=== src/npm-scripts/setup.ts ==="
sed -n '1,400p' src/npm-scripts/setup.ts || true
echo -e "\n=== src/utils/usageTracker.ts (360-420) ==="
sed -n '360,420p' src/utils/usageTracker.ts || true
echo -e "\n=== grep for 'no-onboarding' / 'disableOnboarding' / 'onboard' in key files ==="
grep -nC2 --binary-files=without-match -e '--no-onboarding' -e 'disableOnboarding' -e 'onboard' setup-claude-server.js src/npm-scripts/setup.ts src/index.ts src/utils/usageTracker.ts || trueLength of output: 16029
Action: --no-onboarding is not honored during setup — update docs or persist the flag.
Verified: README lists --no-onboarding under setup (README.md lines 90–93) but code short-circuits on setup before parsing that flag (src/index.ts calls runSetup and returns). runSetup (src/npm-scripts/setup.ts) imports/executes setup-claude-server.js without passing args, and setup-claude-server.js contains no handling for --no-onboarding. Runtime handling exists (src/index.ts sets global.disableOnboarding; src/utils/usageTracker.ts reads it), so the flag only takes effect when starting the server, not during setup.
- Fix options or docs: either move the README bullet to the runtime CLI section, or make setup persist/consume the flag (pass CLI args into setup-claude-server.js or write the flag into config during setup). Files to update: README.md (90–93), src/index.ts, src/npm-scripts/setup.ts, setup-claude-server.js (and optionally src/utils/usageTracker.ts).
🤖 Prompt for AI Agents
In README.md lines 90–93 and related setup flow (src/index.ts,
src/npm-scripts/setup.ts, setup-claude-server.js, optionally
src/utils/usageTracker.ts): the --no-onboarding flag is documented as a setup
option but is ignored during setup because runSetup returns before parsing args
and setup-claude-server.js is invoked without args; either update the README to
move/remove that bullet or make the flag persist/consumed during setup by
passing CLI args from src/index.ts through src/npm-scripts/setup.ts into
setup-claude-server.js and/or writing the flag into the config/storage used by
usageTracker so global.disableOnboarding is set during setup; modify README.md
(move the flag to runtime CLI section if choosing docs), and if implementing
behavior change, update src/index.ts to forward process.argv to runSetup, update
src/npm-scripts/setup.ts to forward args to the child/require of
setup-claude-server.js, and add handling in setup-claude-server.js to read the
flag and persist it to the same config/flag that src/utils/usageTracker.ts
reads.
| // Parse command line arguments for onboarding control | ||
| const DISABLE_ONBOARDING = process.argv.includes('--no-onboarding'); | ||
| if (DISABLE_ONBOARDING) { | ||
| logToStderr('info', 'Onboarding disabled via --no-onboarding flag'); | ||
| } | ||
|
|
||
| // Set global flag for onboarding control | ||
| (global as any).disableOnboarding = DISABLE_ONBOARDING; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Set the global flag without any; optional env support
Use the typed global and drop the any cast. Optionally support an env var for headless/packaged runs.
// Parse command line arguments for onboarding control
-const DISABLE_ONBOARDING = process.argv.includes('--no-onboarding');
+const DISABLE_ONBOARDING =
+ process.argv.includes('--no-onboarding')
+ // Optional: allow env override (document if adopted)
+ || process.env.DC_DISABLE_ONBOARDING === '1'
+ || process.env.DC_DISABLE_ONBOARDING === 'true';
if (DISABLE_ONBOARDING) {
logToStderr('info', 'Onboarding disabled via --no-onboarding flag');
}
// Set global flag for onboarding control
-(global as any).disableOnboarding = DISABLE_ONBOARDING;
+global.disableOnboarding = DISABLE_ONBOARDING;Verification script (ensures no remaining unsafe casts and that the flag is wired once):
🏁 Script executed:
#!/bin/bash
rg -nC2 -S '\(global as any\)\.disableOnboarding|global\.disableOnboarding|disableOnboarding' --type tsLength of output: 749
Use the typed global (remove any) and add optional env override
src/types.ts already declares global.disableOnboarding; replace all casts and set the flag from args or an env var in src/index.ts, and update usages elsewhere.
Locations:
- src/types.ts (declare global: disableOnboarding at line ~6)
- src/index.ts (assignment at line 33) — suggested change below
- src/utils/usageTracker.ts (check at line 386) — change
if ((global as any).disableOnboarding)→if (global.disableOnboarding)
Suggested diff for src/index.ts:
// Parse command line arguments for onboarding control
-const DISABLE_ONBOARDING = process.argv.includes('--no-onboarding');
+const DISABLE_ONBOARDING =
+ process.argv.includes('--no-onboarding')
+ // Optional: allow env override (document if adopted)
+ || process.env.DC_DISABLE_ONBOARDING === '1'
+ || process.env.DC_DISABLE_ONBOARDING === 'true';
if (DISABLE_ONBOARDING) {
logToStderr('info', 'Onboarding disabled via --no-onboarding flag');
}
// Set global flag for onboarding control
-(global as any).disableOnboarding = DISABLE_ONBOARDING;
+global.disableOnboarding = DISABLE_ONBOARDING;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Parse command line arguments for onboarding control | |
| const DISABLE_ONBOARDING = process.argv.includes('--no-onboarding'); | |
| if (DISABLE_ONBOARDING) { | |
| logToStderr('info', 'Onboarding disabled via --no-onboarding flag'); | |
| } | |
| // Set global flag for onboarding control | |
| (global as any).disableOnboarding = DISABLE_ONBOARDING; | |
| // Parse command line arguments for onboarding control | |
| const DISABLE_ONBOARDING = | |
| process.argv.includes('--no-onboarding') | |
| // Optional: allow env override (document if adopted) | |
| || process.env.DC_DISABLE_ONBOARDING === '1' | |
| || process.env.DC_DISABLE_ONBOARDING === 'true'; | |
| if (DISABLE_ONBOARDING) { | |
| logToStderr('info', 'Onboarding disabled via --no-onboarding flag'); | |
| } | |
| // Set global flag for onboarding control | |
| global.disableOnboarding = DISABLE_ONBOARDING; |
1 participant
Summary by CodeRabbit
New Features
Documentation