Skip to content

Commit 707659b

Browse files
aidenybaiaidenybai
committed
refactor(cli): address thermo code-quality findings on debug command
- Add a build-time SKILL.md drift guard (assertSkillCopiesInSync) so the canonical skills/ copy and the .agents/ twin can't silently diverge. - Extract DEBUG_DEFAULT_HOST constant (was a "127.0.0.1" magic string in three spots, including the daemon host check). - Split the ingest POST handler so a log-write failure returns 500 instead of being mislabeled "Invalid JSON". - De-duplicate debug.ts: shared registerShutdown / printServerDetails helpers.
1 parent d4b8c66 commit 707659b

4 files changed

Lines changed: 75 additions & 39 deletions

File tree

‎packages/react-doctor/src/cli/commands/debug.ts‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { spawn } from "node:child_process";
2+
import type { Server } from "node:http";
23
import { Command } from "commander";
34
import { highlighter } from "@react-doctor/core";
45
import { cliLogger as logger } from "../utils/cli-logger.js";
5-
import { createDebugServer } from "../utils/debug-server.js";
6+
import { DEBUG_DEFAULT_HOST } from "../utils/constants.js";
7+
import { createDebugServer, type DebugServerInfo } from "../utils/debug-server.js";
68
import { spinner } from "../utils/spinner.js";
79

810
interface DebugCommandOptions {
@@ -25,10 +27,24 @@ interface DebugCommandContext {
2527
};
2628
}
2729

30+
const registerShutdown = (server: Server): void => {
31+
const shutdown = () => {
32+
server.close();
33+
process.exit(0);
34+
};
35+
process.on("SIGINT", shutdown);
36+
process.on("SIGTERM", shutdown);
37+
};
38+
39+
const printServerDetails = (info: DebugServerInfo): void => {
40+
logger.dim(` Endpoint: ${info.endpoint}`);
41+
logger.dim(` Log path: ${info.logPath}`);
42+
};
43+
2844
const startDaemon = async (options: DebugCommandOptions): Promise<void> => {
2945
const childArgs = [process.argv[1], "debug", "--json"];
3046
if (options.port) childArgs.push("-p", String(options.port));
31-
if (options.host !== "127.0.0.1") childArgs.push("-H", options.host);
47+
if (options.host !== DEBUG_DEFAULT_HOST) childArgs.push("-H", options.host);
3248
if (options.sessionId) childArgs.push("-s", options.sessionId);
3349
if (options.logPath) childArgs.push("-l", options.logPath);
3450

@@ -88,12 +104,7 @@ const startJson = async (options: DebugCommandOptions): Promise<void> => {
88104
process.exit(0);
89105
}
90106

91-
const shutdown = () => {
92-
server.close();
93-
process.exit(0);
94-
};
95-
process.on("SIGINT", shutdown);
96-
process.on("SIGTERM", shutdown);
107+
registerShutdown(server);
97108
};
98109

99110
const startInteractive = async (options: DebugCommandOptions): Promise<void> => {
@@ -110,21 +121,13 @@ const startInteractive = async (options: DebugCommandOptions): Promise<void> =>
110121
startSpinner.succeed(
111122
`Debug server already running on port ${highlighter.bold(String(info.port))}`,
112123
);
113-
logger.dim(` Endpoint: ${info.endpoint}`);
114-
logger.dim(` Log path: ${info.logPath}`);
124+
printServerDetails(info);
115125
return;
116126
}
117127

118128
startSpinner.succeed(`Debug server listening on port ${highlighter.bold(String(info.port))}`);
119-
logger.dim(` Endpoint: ${info.endpoint}`);
120-
logger.dim(` Log path: ${info.logPath}`);
121-
122-
const shutdown = () => {
123-
server.close();
124-
process.exit(0);
125-
};
126-
process.on("SIGINT", shutdown);
127-
process.on("SIGTERM", shutdown);
129+
printServerDetails(info);
130+
registerShutdown(server);
128131
};
129132

130133
export const debugAction = async (

‎packages/react-doctor/src/cli/utils/constants.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ export const INTERNAL_ERROR_JSON_FALLBACK =
2828
export const SENTRY_DSN =
2929
"https://f253d570240a59b8dbd77b7a548ef133@o4510226365743104.ingest.us.sentry.io/4511487817809920";
3030

31+
// Loopback host the debug server binds to by default; kept local so the
32+
// NDJSON ingest endpoint is not exposed beyond the machine.
33+
export const DEBUG_DEFAULT_HOST = "127.0.0.1";
34+
3135
// Bytes of randomness for a `react-doctor debug` session id; hex-encoded
3236
// into a 6-char id that namespaces the per-session NDJSON log file.
3337
export const DEBUG_SESSION_ID_BYTE_LENGTH = 3;

‎packages/react-doctor/src/cli/utils/debug-server.ts‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import http from "node:http";
44
import os from "node:os";
55
import path from "node:path";
66
import {
7+
DEBUG_DEFAULT_HOST,
78
DEBUG_LOG_DIRECTORY_NAME,
89
DEBUG_MAX_DEDUP_ENTRIES,
910
DEBUG_SESSION_ID_BYTE_LENGTH,
@@ -68,7 +69,7 @@ export const createDebugServer = async (
6869
options.sessionId || crypto.randomBytes(DEBUG_SESSION_ID_BYTE_LENGTH).toString("hex");
6970
const logDirectory = path.join(options.cwd || os.tmpdir(), DEBUG_LOG_DIRECTORY_NAME);
7071
const primaryLogPath = options.logPath || path.join(logDirectory, `debug-${sessionId}.log`);
71-
const host = options.host || "127.0.0.1";
72+
const host = options.host || DEBUG_DEFAULT_HOST;
7273
const requestedPort = options.port || 0;
7374

7475
if (!fs.existsSync(logDirectory)) fs.mkdirSync(logDirectory, { recursive: true });
@@ -137,32 +138,40 @@ export const createDebugServer = async (
137138
let requestBody = "";
138139
request.on("data", (chunk: Buffer) => (requestBody += chunk));
139140
request.on("end", () => {
141+
let logEntry: { id?: string; sessionId?: string; timestamp?: number };
140142
try {
141-
const logEntry = JSON.parse(requestBody);
143+
logEntry = JSON.parse(requestBody);
144+
} catch {
145+
response.writeHead(400, { "Content-Type": "application/json" });
146+
response.end(JSON.stringify({ error: "Invalid JSON" }));
147+
return;
148+
}
142149

143-
if (logEntry.id && sessionState.processedEntryIds.has(logEntry.id)) {
144-
response.writeHead(200, { "Content-Type": "application/json" });
145-
response.end(JSON.stringify({ ok: true, duplicate: true }));
146-
return;
147-
}
150+
if (logEntry.id && sessionState.processedEntryIds.has(logEntry.id)) {
151+
response.writeHead(200, { "Content-Type": "application/json" });
152+
response.end(JSON.stringify({ ok: true, duplicate: true }));
153+
return;
154+
}
148155

149-
logEntry.sessionId = logEntry.sessionId || requestSessionId;
150-
logEntry.timestamp = logEntry.timestamp || Date.now();
156+
logEntry.sessionId = logEntry.sessionId || requestSessionId;
157+
logEntry.timestamp = logEntry.timestamp || Date.now();
158+
try {
151159
fs.appendFileSync(sessionState.logPath, JSON.stringify(logEntry) + "\n");
160+
} catch {
161+
response.writeHead(500, { "Content-Type": "application/json" });
162+
response.end(JSON.stringify({ error: "Failed to write log" }));
163+
return;
164+
}
152165

153-
if (logEntry.id) {
154-
if (sessionState.processedEntryIds.size >= DEBUG_MAX_DEDUP_ENTRIES) {
155-
sessionState.processedEntryIds.clear();
156-
}
157-
sessionState.processedEntryIds.add(logEntry.id);
166+
if (logEntry.id) {
167+
if (sessionState.processedEntryIds.size >= DEBUG_MAX_DEDUP_ENTRIES) {
168+
sessionState.processedEntryIds.clear();
158169
}
159-
160-
response.writeHead(200, { "Content-Type": "application/json" });
161-
response.end(JSON.stringify({ ok: true }));
162-
} catch {
163-
response.writeHead(400, { "Content-Type": "application/json" });
164-
response.end(JSON.stringify({ error: "Invalid JSON" }));
170+
sessionState.processedEntryIds.add(logEntry.id);
165171
}
172+
173+
response.writeHead(200, { "Content-Type": "application/json" });
174+
response.end(JSON.stringify({ ok: true }));
166175
});
167176
request.on("error", () => {
168177
if (!response.writableEnded) {

‎packages/react-doctor/vite.config.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,33 @@ const assertSkillManifestParseable = (manifestPath: string): void => {
3232
}
3333
};
3434

35+
// The canonical skill lives in `skills/react-doctor` (shipped to dist) but a
36+
// byte-identical copy is committed at `.agents/skills/react-doctor` so this
37+
// repo's own agents pick it up. Nothing syncs them automatically, so fail the
38+
// build if they drift — forcing both copies to be updated together.
39+
const assertSkillCopiesInSync = (canonicalSkillDir: string): void => {
40+
const agentsSkillManifest = path.resolve(
41+
packageRoot,
42+
"../../.agents/skills/react-doctor/SKILL.md",
43+
);
44+
if (!fs.existsSync(agentsSkillManifest)) return;
45+
const canonical = fs.readFileSync(path.join(canonicalSkillDir, "SKILL.md"), "utf8");
46+
const agentsCopy = fs.readFileSync(agentsSkillManifest, "utf8");
47+
if (canonical !== agentsCopy) {
48+
throw new Error(
49+
"SKILL.md drift: skills/react-doctor/SKILL.md and .agents/skills/react-doctor/SKILL.md differ. Copy the canonical skills/ version into .agents/ so they stay identical.",
50+
);
51+
}
52+
};
53+
3554
const copySkillToDist = () => {
3655
const skillSource = path.resolve(packageRoot, "../../skills/react-doctor");
3756
const skillTarget = path.resolve(packageRoot, "dist/skills/react-doctor");
3857
if (!fs.existsSync(skillSource)) {
3958
throw new Error(`Skill source missing at ${skillSource}; expected to ship dist/skills/`);
4059
}
4160
assertSkillManifestParseable(path.join(skillSource, "SKILL.md"));
61+
assertSkillCopiesInSync(skillSource);
4262
fs.rmSync(skillTarget, { recursive: true, force: true });
4363
fs.mkdirSync(skillTarget, { recursive: true });
4464
fs.cpSync(skillSource, skillTarget, { recursive: true });

0 commit comments

Comments
 (0)