-
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
base: main
Are you sure you want to change the base?
Changes from all commits
f2250c2
d4b8c66
707659b
d1268db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| import { spawn } from "node:child_process"; | ||
| import type { Server } from "node:http"; | ||
| import { Command } from "commander"; | ||
| import { highlighter } from "@react-doctor/core"; | ||
| import { cliLogger as logger } from "../utils/cli-logger.js"; | ||
| import { createDebugServer, type DebugServerInfo } from "../debug-server/index.js"; | ||
| import { DEBUG_DEFAULT_HOST } from "../utils/constants.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 registerShutdown = (server: Server): void => { | ||
| const shutdown = () => { | ||
| server.close(); | ||
| process.exit(0); | ||
| }; | ||
| process.on("SIGINT", shutdown); | ||
| process.on("SIGTERM", shutdown); | ||
| }; | ||
|
|
||
| const printServerDetails = (info: DebugServerInfo): void => { | ||
| logger.dim(` Endpoint: ${info.endpoint}`); | ||
| logger.dim(` Log path: ${info.logPath}`); | ||
| }; | ||
|
|
||
| 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 !== DEBUG_DEFAULT_HOST) 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 = ""; | ||
| let isSettled = false; | ||
| const serverInfoLine = await new Promise<string>((resolve, reject) => { | ||
| const settle = (action: () => void) => { | ||
| if (isSettled) return; | ||
| isSettled = true; | ||
| action(); | ||
| }; | ||
| childProcess.stdout!.on("data", (chunk: Buffer) => { | ||
| stdoutBuffer += chunk.toString(); | ||
| const newlineIndex = stdoutBuffer.indexOf("\n"); | ||
| // Strip a trailing CR so the printed line is valid JSON on Windows. | ||
| if (newlineIndex !== -1) { | ||
| settle(() => resolve(stdoutBuffer.slice(0, newlineIndex).replace(/\r$/, ""))); | ||
| } | ||
| }); | ||
| childProcess.on("error", (error) => settle(() => reject(error))); | ||
| childProcess.on("exit", (code) => | ||
| // The server child stays alive once it prints its info line, so a | ||
| // resolve always wins the race; reaching `exit` first means it died | ||
| // before printing — reject rather than hang the parent forever. | ||
| settle(() => | ||
| reject(new Error(`Debug server process exited (code ${code ?? "unknown"}) before startup`)), | ||
| ), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Daemon treats exit zero as failureMedium Severity In Reviewed by Cursor Bugbot for commit d4b8c66. Configure here. |
||
| ); | ||
| }); | ||
|
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); | ||
| } | ||
|
|
||
| registerShutdown(server); | ||
| }; | ||
|
|
||
| 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))}`, | ||
| ); | ||
| printServerDetails(info); | ||
| return; | ||
| } | ||
|
|
||
| startSpinner.succeed(`Debug server listening on port ${highlighter.bold(String(info.port))}`); | ||
| printServerDetails(info); | ||
| registerShutdown(server); | ||
| }; | ||
|
|
||
| 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); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import crypto from "node:crypto"; | ||
| import fs from "node:fs"; | ||
| import http from "node:http"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
| import { | ||
| DEBUG_DEFAULT_HOST, | ||
| DEBUG_LOG_DIRECTORY_NAME, | ||
| DEBUG_SESSION_ID_BYTE_LENGTH, | ||
| } from "../utils/constants.js"; | ||
| import { SESSION_ID_PATTERN } from "./debug-session-store.js"; | ||
| import { createIngestRequestListener } from "./ingest-request-listener.js"; | ||
| import { pingDebugServer } from "./ping-server.js"; | ||
| import { readDebugServerLock, removeDebugServerLock, writeDebugServerLock } from "./server-lock.js"; | ||
|
|
||
| export interface DebugServerOptions { | ||
| sessionId?: string; | ||
| cwd?: string; | ||
| logPath?: string; | ||
| host?: string; | ||
| port?: number; | ||
| } | ||
|
|
||
| export interface DebugServerInfo { | ||
| sessionId: string; | ||
| port: number; | ||
| endpoint: string; | ||
| logPath: string; | ||
| } | ||
|
|
||
| export interface DebugServerResult { | ||
| server: http.Server | null; | ||
| info: DebugServerInfo; | ||
| reused: boolean; | ||
| } | ||
|
|
||
| export const createDebugServer = async ( | ||
| options: DebugServerOptions = {}, | ||
| ): Promise<DebugServerResult> => { | ||
| if (options.sessionId !== undefined && !SESSION_ID_PATTERN.test(options.sessionId)) { | ||
| throw new Error( | ||
| "Invalid --session-id: only letters, digits, '-' and '_' are allowed (no path separators).", | ||
| ); | ||
| } | ||
| const sessionId = | ||
| options.sessionId || crypto.randomBytes(DEBUG_SESSION_ID_BYTE_LENGTH).toString("hex"); | ||
| const logDirectory = path.join(options.cwd || os.tmpdir(), DEBUG_LOG_DIRECTORY_NAME); | ||
| const primaryLogPath = options.logPath || path.join(logDirectory, `debug-${sessionId}.log`); | ||
| const host = options.host || DEBUG_DEFAULT_HOST; | ||
| const requestedPort = options.port || 0; | ||
|
|
||
| if (!fs.existsSync(logDirectory)) fs.mkdirSync(logDirectory, { recursive: true }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Custom log path parent missingMedium Severity Only the shared Reviewed by Cursor Bugbot for commit d1268db. Configure here. |
||
|
|
||
| const existingLock = readDebugServerLock(logDirectory); | ||
| if (existingLock) { | ||
| const isAlive = await pingDebugServer(existingLock.host, existingLock.port); | ||
| if (isAlive) { | ||
| return { | ||
| server: null, | ||
| info: { | ||
| sessionId: existingLock.sessionId, | ||
| port: existingLock.port, | ||
| endpoint: existingLock.endpoint, | ||
| logPath: existingLock.logPath, | ||
| }, | ||
| reused: true, | ||
| }; | ||
| } | ||
| removeDebugServerLock(logDirectory); | ||
| } | ||
|
|
||
| const server = http.createServer( | ||
| createIngestRequestListener({ primarySessionId: sessionId, primaryLogPath, logDirectory }), | ||
| ); | ||
|
|
||
| return new Promise<DebugServerResult>((resolve, reject) => { | ||
| server.listen(requestedPort, host, () => { | ||
| const serverAddress = server.address(); | ||
| if (!serverAddress || typeof serverAddress === "string") { | ||
| reject(new Error("Failed to get debug server address")); | ||
| return; | ||
| } | ||
|
|
||
| const info: DebugServerInfo = { | ||
| sessionId, | ||
| port: serverAddress.port, | ||
| endpoint: `http://${host}:${serverAddress.port}/ingest/${sessionId}`, | ||
| logPath: primaryLogPath, | ||
| }; | ||
|
|
||
| writeDebugServerLock(logDirectory, { | ||
| pid: process.pid, | ||
| host, | ||
| port: serverAddress.port, | ||
| sessionId, | ||
| endpoint: info.endpoint, | ||
| logPath: primaryLogPath, | ||
| }); | ||
|
|
||
| server.on("close", () => removeDebugServerLock(logDirectory)); | ||
| // SIGINT runs the CLI's `exitGracefully` handler, which calls | ||
| // `process.exit` before the server's `close` event can fire, so wire | ||
| // lock removal to `exit` (always fires) to avoid a stale lock file. | ||
| process.once("exit", () => removeDebugServerLock(logDirectory)); | ||
|
|
||
| resolve({ server, info, reused: false }); | ||
| }); | ||
| server.on("error", reject); | ||
| }); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import path from "node:path"; | ||
| import { DEBUG_MAX_DEDUP_ENTRIES } from "../utils/constants.js"; | ||
|
|
||
| export interface DebugSessionState { | ||
| logPath: string; | ||
| processedEntryIds: Set<string>; | ||
| } | ||
|
|
||
| export interface DebugSessionStore { | ||
| get(requestSessionId: string): DebugSessionState; | ||
| } | ||
|
|
||
| interface DebugSessionStoreOptions { | ||
| primarySessionId: string; | ||
| primaryLogPath: string; | ||
| logDirectory: string; | ||
| } | ||
|
|
||
| // Session ids index a per-session log filename (`debug-<id>.log`), so they | ||
| // must stay within the log directory: only word chars, dash, underscore — | ||
| // no path separators or `..`. | ||
| export const SESSION_ID_PATTERN = /^[a-zA-Z0-9_-]+$/; | ||
|
|
||
| export const parseIngestSessionId = (url: string): string | null => { | ||
| try { | ||
| const { pathname } = new URL(url, "http://localhost"); | ||
| const match = pathname.match(/^\/ingest\/([a-zA-Z0-9_-]+)\/?$/); | ||
| return match ? match[1] : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| }; | ||
|
|
||
| // Remember an entry id for de-duplication, bounding memory by dropping the | ||
| // whole set once it grows past the cap (best-effort: a repeat after a reset | ||
| // can slip through, which is acceptable for a debug session). | ||
| export const rememberProcessedEntryId = (state: DebugSessionState, entryId: string): void => { | ||
| if (state.processedEntryIds.size >= DEBUG_MAX_DEDUP_ENTRIES) { | ||
| state.processedEntryIds.clear(); | ||
| } | ||
| state.processedEntryIds.add(entryId); | ||
| }; | ||
|
|
||
| export const createDebugSessionStore = (options: DebugSessionStoreOptions): DebugSessionStore => { | ||
| const sessions = new Map<string, DebugSessionState>(); | ||
|
|
||
| const get = (requestSessionId: string): DebugSessionState => { | ||
| const existing = sessions.get(requestSessionId); | ||
| if (existing) return existing; | ||
|
|
||
| const logPath = | ||
| requestSessionId === options.primarySessionId | ||
| ? options.primaryLogPath | ||
| : path.join(options.logDirectory, `debug-${requestSessionId}.log`); | ||
| const state: DebugSessionState = { logPath, processedEntryIds: new Set() }; | ||
| sessions.set(requestSessionId, state); | ||
| return state; | ||
| }; | ||
|
|
||
| return { get }; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| export { createDebugServer } from "./create-debug-server.js"; | ||
| export type { | ||
| DebugServerInfo, | ||
| DebugServerOptions, | ||
| DebugServerResult, | ||
| } from "./create-debug-server.js"; |


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.