-
Notifications
You must be signed in to change notification settings - Fork 387
feat(cli): add debug subcommand + debug/performance skill sections
#624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
aidenybai
wants to merge
4
commits into
main
Choose a base branch
from
feat/debug-command
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
f2250c2
feat(cli): add `debug` subcommand and debug/performance skill sections
aidenybai d4b8c66
fix(cli): address debug-server review findings
aidenybai 707659b
refactor(cli): address thermo code-quality findings on debug command
aidenybai d1268db
refactor(cli): extract debug server into a feature module
aidenybai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "react-doctor": minor | ||
| --- | ||
|
|
||
| Add a `react-doctor debug` subcommand that starts an NDJSON logging server for evidence-based debugging. It supports an interactive mode, a `--daemon` mode (spawns a detached background server and prints one JSON line), and a `--json` mode, and is idempotent via a singleton lock file so re-running returns the existing session. The bundled `react-doctor` agent skill gains `/debug` (a runtime-instrumentation root-cause loop) and `/performance` (a LoAF + commit attribution rig) sections that drive this server. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| import { spawn } from "node:child_process"; | ||
| import { Command } from "commander"; | ||
| import { highlighter } from "@react-doctor/core"; | ||
| import { cliLogger as logger } from "../utils/cli-logger.js"; | ||
| import { createDebugServer } from "../utils/debug-server.js"; | ||
| import { spinner } from "../utils/spinner.js"; | ||
|
|
||
| interface DebugCommandOptions { | ||
| port?: number; | ||
| host: string; | ||
| sessionId?: string; | ||
| logPath?: string; | ||
| daemon?: boolean; | ||
| json?: boolean; | ||
| } | ||
|
|
||
| // `--json` is also a root-level flag, so Commander binds it to the parent | ||
| // program rather than the `debug` subcommand. Read it back from the parent | ||
| // (same interplay the `install` command handles for `--yes`). | ||
| interface DebugCommandContext { | ||
| parent?: { | ||
| opts?: () => { | ||
| json?: boolean; | ||
| }; | ||
| }; | ||
| } | ||
|
|
||
| const startDaemon = async (options: DebugCommandOptions): Promise<void> => { | ||
| const childArgs = [process.argv[1], "debug", "--json"]; | ||
| if (options.port) childArgs.push("-p", String(options.port)); | ||
| if (options.host !== "127.0.0.1") childArgs.push("-H", options.host); | ||
| if (options.sessionId) childArgs.push("-s", options.sessionId); | ||
| if (options.logPath) childArgs.push("-l", options.logPath); | ||
|
|
||
| const childProcess = spawn(process.execPath, childArgs, { | ||
| detached: true, | ||
| stdio: ["ignore", "pipe", "ignore"], | ||
| }); | ||
|
|
||
| if (!childProcess.stdout) { | ||
| logger.error("Failed to start debug server daemon."); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| let stdoutBuffer = ""; | ||
| const serverInfoLine = await new Promise<string>((resolve, reject) => { | ||
| childProcess.stdout!.on("data", (chunk: Buffer) => { | ||
| stdoutBuffer += chunk.toString(); | ||
| const newlineIndex = stdoutBuffer.indexOf("\n"); | ||
| if (newlineIndex !== -1) resolve(stdoutBuffer.slice(0, newlineIndex)); | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
|
||
| }); | ||
| childProcess.on("error", reject); | ||
| childProcess.on("exit", (code) => { | ||
| if (code !== 0) reject(new Error(`Debug server process exited with code ${code}`)); | ||
| }); | ||
| }); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| console.log(serverInfoLine); | ||
| childProcess.unref(); | ||
| process.exit(0); | ||
| }; | ||
|
|
||
| const startJson = async (options: DebugCommandOptions): Promise<void> => { | ||
| const { server, info } = await createDebugServer({ | ||
| port: options.port, | ||
| host: options.host, | ||
| sessionId: options.sessionId, | ||
| logPath: options.logPath, | ||
| }); | ||
|
|
||
| console.log(JSON.stringify(info)); | ||
|
|
||
| if (!server) { | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const shutdown = () => { | ||
| server.close(); | ||
| process.exit(0); | ||
| }; | ||
| process.on("SIGINT", shutdown); | ||
| process.on("SIGTERM", shutdown); | ||
| }; | ||
|
|
||
| const startInteractive = async (options: DebugCommandOptions): Promise<void> => { | ||
| const startSpinner = spinner("Starting React Doctor debug server...").start(); | ||
|
|
||
| const { server, info, reused } = await createDebugServer({ | ||
| port: options.port, | ||
| host: options.host, | ||
| sessionId: options.sessionId, | ||
| logPath: options.logPath, | ||
| }); | ||
|
|
||
| if (reused || !server) { | ||
| startSpinner.succeed( | ||
| `Debug server already running on port ${highlighter.bold(String(info.port))}`, | ||
| ); | ||
| logger.dim(` Endpoint: ${info.endpoint}`); | ||
| logger.dim(` Log path: ${info.logPath}`); | ||
| return; | ||
| } | ||
|
|
||
| startSpinner.succeed(`Debug server listening on port ${highlighter.bold(String(info.port))}`); | ||
| logger.dim(` Endpoint: ${info.endpoint}`); | ||
| logger.dim(` Log path: ${info.logPath}`); | ||
|
|
||
| const shutdown = () => { | ||
| server.close(); | ||
| process.exit(0); | ||
| }; | ||
| process.on("SIGINT", shutdown); | ||
| process.on("SIGTERM", shutdown); | ||
| }; | ||
|
|
||
| export const debugAction = async ( | ||
| options: DebugCommandOptions, | ||
| command?: DebugCommandContext, | ||
| ): Promise<void> => { | ||
| const isJson = options.json ?? command?.parent?.opts?.().json ?? false; | ||
| if (options.daemon) { | ||
| await startDaemon(options); | ||
| return; | ||
| } | ||
| if (isJson) { | ||
| await startJson(options); | ||
| return; | ||
| } | ||
| await startInteractive(options); | ||
| }; | ||
|
|
||
| export const registerDebugCommand = (program: Command): void => { | ||
| program | ||
| .command("debug") | ||
| .description("Start the NDJSON debug logging server for evidence-based debugging") | ||
| .option("-p, --port <number>", "port to listen on (default: random)", (value) => | ||
| parseInt(value, 10), | ||
| ) | ||
| .option("-H, --host <address>", "host to bind to", "127.0.0.1") | ||
| .option("-s, --session-id <id>", "session ID (default: random 6-char hex)") | ||
| .option("-l, --log-path <path>", "log file path (default: <tmpdir>/react-doctor-debug/...)") | ||
| .option("-d, --daemon", "start the server in the background and exit") | ||
| .option("--json", "output server info as JSON (no spinner/colors)") | ||
| .action(debugAction); | ||
| }; | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| export interface DebugServerLock { | ||
| pid: number; | ||
| host: string; | ||
| port: number; | ||
| sessionId: string; | ||
| endpoint: string; | ||
| logPath: string; | ||
| } | ||
|
|
||
| const LOCK_FILENAME = "debug-server.lock"; | ||
|
|
||
| const getLockPath = (directory: string): string => path.join(directory, LOCK_FILENAME); | ||
|
|
||
| export const readDebugServerLock = (directory: string): DebugServerLock | null => { | ||
| const lockPath = getLockPath(directory); | ||
| if (!fs.existsSync(lockPath)) return null; | ||
| try { | ||
| return JSON.parse(fs.readFileSync(lockPath, "utf-8")); | ||
| } catch { | ||
| return null; | ||
| } | ||
| }; | ||
|
|
||
| export const writeDebugServerLock = (directory: string, lockData: DebugServerLock): void => { | ||
| fs.writeFileSync(getLockPath(directory), JSON.stringify(lockData, null, 2)); | ||
| }; | ||
|
|
||
| export const removeDebugServerLock = (directory: string): void => { | ||
| const lockPath = getLockPath(directory); | ||
| if (!fs.existsSync(lockPath)) return; | ||
| try { | ||
| fs.unlinkSync(lockPath); | ||
| } catch {} | ||
| }; |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Daemon omits telemetry opt-out flag
Medium Severity
startDaemonspawns the background server with a fixed argv list and never forwards--no-score, even when the parent CLI invocation included it. Sentry initialization reads rawprocess.argvand disables crash reporting only when that flag is present, so the detached child can still initialize Sentry and report crashes after the user opted out on the parent command.Reviewed by Cursor Bugbot for commit d1268db. Configure here.