Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/tall-adults-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"react-doctor": patch
"deslop-js": patch
---

Harden scan orchestration and cache persistence by validating stored payloads, preserving independently written entries, and keeping workflow paths inside the repository.
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export * from "./run-oxlint.js";
export * from "./summarize-diagnostics.js";
export * from "./validate-config-types.js";
export * from "./utils/assign-fix-groups.js";
export * from "./utils/atomic-write-json.js";
export * from "./utils/build-rule-docs-url.js";
export * from "./utils/classify-package-role.js";
export * from "./utils/collect-source-file-counts-by-directory.js";
Expand Down
22 changes: 22 additions & 0 deletions packages/core/src/utils/atomic-write-file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { randomUUID } from "node:crypto";
import * as fs from "node:fs";
import * as path from "node:path";

export const atomicWriteFile = (filePath: string, contents: string): void => {
let temporaryPath: string | null = null;
try {
fs.mkdirSync(path.dirname(filePath), { recursive: true });
temporaryPath = `${filePath}.${process.pid}.${randomUUID()}.tmp`;
fs.writeFileSync(temporaryPath, contents);
fs.renameSync(temporaryPath, filePath);
temporaryPath = null;
} catch {
return;
} finally {
if (temporaryPath !== null) {
try {
fs.rmSync(temporaryPath, { force: true });
} catch {}
}
}
};
12 changes: 4 additions & 8 deletions packages/core/src/utils/atomic-write-json.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import * as fs from "node:fs";
import * as path from "node:path";
import { atomicWriteFile } from "./atomic-write-file.js";

// Writes `value` as JSON to `filePath` atomically: serialize to a
// pid-suffixed temp file in the same directory, then rename over the target so
// Writes `value` as JSON to `filePath` atomically: serialize to a unique temp
// file in the same directory, then rename over the target so
// a concurrent reader sees either the old or the new file, never a half-written
// one. Swallows every error — a cache that can't persist must not break a scan.
export const atomicWriteJson = (filePath: string, value: unknown): void => {
try {
fs.mkdirSync(path.dirname(filePath), { recursive: true });
const temporaryPath = `${filePath}.${process.pid}.tmp`;
fs.writeFileSync(temporaryPath, JSON.stringify(value));
fs.renameSync(temporaryPath, filePath);
atomicWriteFile(filePath, JSON.stringify(value));
} catch {
return;
}
Expand Down
22 changes: 2 additions & 20 deletions packages/deslop-js/src/summary-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,7 @@
// rooted at `rootDir` (matching core's whole-result dead-code cache), so
// manifest edits ABOVE the scanned root share that cache's accepted gap.
import crypto from "node:crypto";
import {
mkdirSync,
readFileSync,
readdirSync,
realpathSync,
renameSync,
statSync,
writeFileSync,
} from "node:fs";
import { readFileSync, readdirSync, realpathSync, statSync } from "node:fs";
import { createRequire } from "node:module";
import { dirname, resolve } from "node:path";
import { Minimatch } from "minimatch";
Expand All @@ -49,6 +41,7 @@ import {
SUMMARY_CACHE_MAX_BYTES,
SUMMARY_CACHE_SCHEMA_VERSION,
} from "./constants.js";
import { atomicWriteFile } from "./utils/atomic-write-file.js";
import { toPosixPath } from "./utils/to-posix-path.js";

export type PackageFactKind = "substring" | "importReference";
Expand Down Expand Up @@ -500,17 +493,6 @@ const reviveParsedSource = (persisted: unknown): ParsedSource | null => {
};
};

const atomicWriteFile = (filePath: string, contents: string): void => {
try {
mkdirSync(dirname(filePath), { recursive: true });
const temporaryPath = `${filePath}.${process.pid}.tmp`;
writeFileSync(temporaryPath, contents);
renameSync(temporaryPath, filePath);
} catch {
// A cache that cannot persist must never break the analysis.
}
};

const createSummaryCache = (cachePath: string, config: DeslopConfig): SummaryCache => {
const scopeHash = computeScopeHash(config);
const store = readPersistedStore(cachePath, scopeHash);
Expand Down
22 changes: 22 additions & 0 deletions packages/deslop-js/src/utils/atomic-write-file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { randomUUID } from "node:crypto";
import * as fs from "node:fs";
import * as path from "node:path";

export const atomicWriteFile = (filePath: string, contents: string): void => {
let temporaryPath: string | null = null;
try {
fs.mkdirSync(path.dirname(filePath), { recursive: true });
temporaryPath = `${filePath}.${process.pid}.${randomUUID()}.tmp`;
fs.writeFileSync(temporaryPath, contents);
fs.renameSync(temporaryPath, filePath);
temporaryPath = null;
} catch {
return;
} finally {
if (temporaryPath !== null) {
try {
fs.rmSync(temporaryPath, { force: true });
} catch {}
}
}
};
1 change: 0 additions & 1 deletion packages/react-doctor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
"@types/babel__code-frame": "^7.27.0",
"@types/prompts": "^2.4.9",
"@types/react": "^19.2.14",
"@xterm/headless": "^6.0.0",
"commander": "^14.0.3",
"ink": "^7.1.0",
"ink-spinner": "^5.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-doctor/src/cli/ink/run-scan-app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {
WorkspacePackage,
} from "@react-doctor/core";
import { createInvocationInspect } from "../../inspect.js";
import type { ReactDoctorInspectOptions } from "../../inspect.js";
import type { ReactDoctorInspectOptions } from "../../inspect-options.js";
import { buildNoScoreMessage } from "../utils/build-no-score-message.js";
import { hasIncompleteScoreAnalysis } from "../utils/has-incomplete-score-analysis.js";
import type { InspectFlags } from "../utils/inspect-flags.js";
Expand Down
167 changes: 167 additions & 0 deletions packages/react-doctor/src/cli/utils/finalize-inspect-result.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
import * as Console from "effect/Console";
import * as Effect from "effect/Effect";
import {
buildSkippedChecks,
filterDiagnosticsForSurface,
highlighter,
type InspectResult,
} from "@react-doctor/core";
import type { ResolvedInspectOptions } from "../../inspect-options.js";
import { buildEmptyReportMessage } from "./build-empty-report-message.js";
import { buildNoScoreMessage } from "./build-no-score-message.js";
import { filterDiagnosticsByCategories } from "./filter-diagnostics-by-categories.js";
import { hasIncompleteScoreAnalysis } from "./has-incomplete-score-analysis.js";
import { printDiagnosticsDump } from "./print-diagnostics-dump.js";
import { printFooter } from "./print-footer.js";
import { printHeadlessReport } from "./print-headless-report.js";
import { printAgentGuidance } from "./render-agent-guidance.js";
import type { CachedScanPayload } from "./scan-result-cache-payload.js";

export interface InspectExecutionCacheStats {
readonly lintCacheHitFileCount: number | null;
readonly lintCacheTotalFileCount: number | null;
readonly lintSidecarReplayedFileCount: number | null;
readonly lintSidecarTotalFileCount: number | null;
readonly deadCodeCacheHit: boolean | null;
readonly deadCodeSummaryCacheHits: number | null;
readonly deadCodeSummaryCacheMisses: number | null;
}

interface FinalizeInspectResultInput {
readonly options: ResolvedInspectOptions;
readonly elapsedMilliseconds: number;
readonly payload: CachedScanPayload;
readonly cacheStats: InspectExecutionCacheStats;
}

export const finalizeInspectResult = (
input: FinalizeInspectResultInput,
): Effect.Effect<InspectResult> =>
Effect.gen(function* () {
const { payload, cacheStats } = input;
const { skippedChecks, skippedCheckReasons } = buildSkippedChecks({
didLintFail: payload.didLintFail,
lintFailureReason: payload.lintFailureReason,
lintPartialFailures: payload.lintPartialFailures,
didDeadCodeFail: payload.didDeadCodeFail,
deadCodeFailureReason: payload.deadCodeFailureReason,
supplyChainOverlapTimedOut: payload.supplyChainOverlapTimedOut,
securityScanFailed: payload.securityScanFailed ?? false,
securityScanFailureReason: payload.securityScanFailureReason ?? null,
});
const hasSkippedChecks = skippedChecks.length > 0;
const noScoreMessage = buildNoScoreMessage({
isScoreDisabled: input.options.noScore,
isAnalysisIncomplete: hasIncompleteScoreAnalysis(skippedChecks),
disabledMessage: input.options.scoreDisabledMessage,
});
const result: InspectResult = {
diagnostics: [...payload.diagnostics],
score: payload.score,
skippedChecks,
...(Object.keys(skippedCheckReasons).length > 0 ? { skippedCheckReasons } : {}),
project: payload.project,
elapsedMilliseconds: input.elapsedMilliseconds,
scannedFileCount: payload.scannedFileCount,
scannedFilePaths: payload.scannedFilePaths,
analyzedFiles: payload.analyzedFiles ?? [],
scanElapsedMilliseconds: payload.scanElapsedMilliseconds,
...(cacheStats.lintCacheTotalFileCount !== null
? {
lintCacheHitFileCount: cacheStats.lintCacheHitFileCount,
lintCacheTotalFileCount: cacheStats.lintCacheTotalFileCount,
}
: {}),
...(cacheStats.lintSidecarTotalFileCount !== null
? {
lintSidecarReplayedFileCount: cacheStats.lintSidecarReplayedFileCount,
lintSidecarTotalFileCount: cacheStats.lintSidecarTotalFileCount,
}
: {}),
...(cacheStats.deadCodeCacheHit !== null
? { deadCodeCacheHit: cacheStats.deadCodeCacheHit }
: {}),
...(cacheStats.deadCodeSummaryCacheHits !== null &&
cacheStats.deadCodeSummaryCacheMisses !== null
? {
deadCodeSummaryCacheHits: cacheStats.deadCodeSummaryCacheHits,
deadCodeSummaryCacheMisses: cacheStats.deadCodeSummaryCacheMisses,
}
: {}),
...(payload.baselineDelta ? { baselineDelta: payload.baselineDelta } : {}),
};

if (input.options.suppressRendering) return result;

const surfaceDiagnostics = filterDiagnosticsForSurface(
[...payload.diagnostics],
input.options.outputSurface,
payload.userConfig,
);
const printedDiagnostics = filterDiagnosticsByCategories(
surfaceDiagnostics,
input.options.categoryFilters,
);

if (input.options.scoreOnly) {
if (input.options.outputDirectory !== null) {
yield* printDiagnosticsDump(
printedDiagnostics,
input.options.outputDirectory,
false,
"stderr",
);
}
if (payload.score) {
yield* Console.log(`${payload.score.score}`);
} else {
yield* Console.error(highlighter.gray(noScoreMessage));
}
return result;
}

const demotedDiagnosticCount = payload.diagnostics.length - surfaceDiagnostics.length;
if (input.options.isNonInteractiveEnvironment && input.options.outputSurface !== "prComment") {
yield* printAgentGuidance();
}

yield* printHeadlessReport({
diagnostics: printedDiagnostics,
elapsedMilliseconds: input.elapsedMilliseconds,
emptyStateMessage: buildEmptyReportMessage({
categoryFilters: input.options.categoryFilters,
demotedDiagnosticCount,
outputSurface: input.options.outputSurface,
}),
noScoreMessage,
projectName: payload.project.projectName,
scannedFileCount: payload.scannedFileCount,
scoreResult: hasSkippedChecks ? null : payload.score,
skippedChecks,
});

if (input.options.outputDirectory !== null || input.options.verbose) {
yield* printDiagnosticsDump(
printedDiagnostics,
input.options.outputDirectory,
input.options.verbose,
);
}
if (input.options.categoryFilters.size === 0 && demotedDiagnosticCount > 0) {
yield* Console.log(
highlighter.gray(
` ${demotedDiagnosticCount} demoted from the ${input.options.outputSurface} surface (e.g. design cleanup) — run \`npx react-doctor@latest .\` locally for the full list.`,
),
);
yield* Console.log("");
}

yield* printFooter({
diagnostics: printedDiagnostics,
scoreResult: payload.score,
projectName: payload.project.projectName,
isOffline: input.options.isCi || !input.options.share || payload.score === null,
});

return result;
});
28 changes: 17 additions & 11 deletions packages/react-doctor/src/cli/utils/open-workflow-pull-request.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as path from "node:path";
import { isPathInsideDirectory } from "@react-doctor/core";
import { GH_PR_LIST_MAX } from "./constants.js";
import { detectDefaultBranch } from "./detect-default-branch.js";
import { isCommandAvailable } from "./is-command-available.js";
Expand Down Expand Up @@ -41,6 +42,7 @@ export type NotAttemptedReason =
| "gh-not-installed"
| "gh-not-authenticated"
| "not-a-git-repo"
| "workflow-outside-repository"
| "no-default-branch"
| "detached-head"
// The working tree has tracked (staged or unstaged) modifications, which
Expand Down Expand Up @@ -141,7 +143,7 @@ const hasUnrelatedTrackedChanges = async (
// Async so the chain no longer blocks the event loop and the caller's `ora`
// spinner keeps animating through the slow network steps. Each step still
// runs sequentially via `await` because it depends on the previous one.
export const openWorkflowPullRequest = async (params: {
export const openWorkflowPullRequest = async (input: {
workflowPath: string;
// Override the commit message / PR title + body. Defaults describe a fresh
// install; the v1→v2 upgrade flow passes its own copy. The git/`gh` steps,
Expand All @@ -160,12 +162,12 @@ export const openWorkflowPullRequest = async (params: {
run?: CommandRunner;
checkCommandAvailable?: (command: string) => boolean;
}): Promise<OpenWorkflowPullRequestResult> => {
const workflowPath = path.resolve(params.workflowPath);
const commitMessage = params.commitMessage ?? DEFAULT_COMMIT_MESSAGE;
const prTitle = params.prTitle ?? DEFAULT_PR_TITLE;
const prBody = params.prBody ?? DEFAULT_PR_BODY;
const run = params.run ?? runCommand;
const checkCommandAvailable = params.checkCommandAvailable ?? isCommandAvailable;
const workflowPath = path.resolve(input.workflowPath);
const commitMessage = input.commitMessage ?? DEFAULT_COMMIT_MESSAGE;
const prTitle = input.prTitle ?? DEFAULT_PR_TITLE;
const prBody = input.prBody ?? DEFAULT_PR_BODY;
const run = input.run ?? runCommand;
const checkCommandAvailable = input.checkCommandAvailable ?? isCommandAvailable;

// Probe from the workflow file's directory so we resolve the repo root
// even when the CLI was invoked from a sub-package in a monorepo.
Expand All @@ -176,6 +178,9 @@ export const openWorkflowPullRequest = async (params: {
);
if (!repoRootProbe.success) return { status: "not-attempted", reason: "not-a-git-repo" };
const cwd = repoRootProbe.stdout;
if (!isPathInsideDirectory(workflowPath, cwd)) {
return { status: "not-attempted", reason: "workflow-outside-repository" };
}
Comment thread
cursor[bot] marked this conversation as resolved.
// Forward slashes so the `:!` exclude pathspec and `git add` match git's
// forward-slash-normalized repo paths on Windows (where `path.relative`
// yields backslashes, which git's magic pathspec won't treat as separators).
Expand Down Expand Up @@ -205,7 +210,7 @@ export const openWorkflowPullRequest = async (params: {
return { status: "not-attempted", reason: "working-tree-dirty" };
}

const defaultBranch = params.baseBranch ?? (await detectDefaultBranch(cwd, run));
const defaultBranch = input.baseBranch ?? (await detectDefaultBranch(cwd, run));
if (!defaultBranch) return { status: "not-attempted", reason: "no-default-branch" };

const previousBranchProbe = await run("git", ["rev-parse", "--abbrev-ref", "HEAD"], cwd);
Expand Down Expand Up @@ -283,18 +288,19 @@ export const openWorkflowPullRequest = async (params: {
// `"not-attempted"` and the file should still land in their next commit
// instead of sitting as an orphan untracked path. Returns whether the stage
// actually happened.
export const stageWorkflowFile = async (params: {
export const stageWorkflowFile = async (input: {
workflowPath: string;
run?: CommandRunner;
}): Promise<boolean> => {
const workflowPath = path.resolve(params.workflowPath);
const run = params.run ?? runCommand;
const workflowPath = path.resolve(input.workflowPath);
const run = input.run ?? runCommand;
const repoRootProbe = await run(
"git",
["rev-parse", "--show-toplevel"],
path.dirname(workflowPath),
);
if (!repoRootProbe.success) return false;
if (!isPathInsideDirectory(workflowPath, repoRootProbe.stdout)) return false;
const workflowRelative = toForwardSlashes(path.relative(repoRootProbe.stdout, workflowPath));
return (await run("git", ["add", "--", workflowRelative], repoRootProbe.stdout)).success;
};
Loading
Loading