Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions .vite-hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ set -e

echo "Running pre-commit checks..."

pnpm lint
pnpm fmt:check
pnpm precommit

echo "Pre-commit checks passed."
2 changes: 2 additions & 0 deletions apps/desktop/electron/desktop-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from "./hostedRepos";
import {
commitStaged,
getLastGitCommandErrorLogPath,
discardAll,
discardFile,
discardFiles,
Expand Down Expand Up @@ -92,6 +93,7 @@ export const desktopApi: DesktopApi = {
discardFiles,
discardAll,
commitStaged,
getLastGitCommandErrorLogPath,
getRepoFile,
syncLspDocument: (input) => lspSessionManager.syncDocument(input),
closeLspDocument: (input) => lspSessionManager.closeDocument(input),
Expand Down
67 changes: 63 additions & 4 deletions apps/desktop/electron/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ const MAX_BUFFER = 32 * 1024 * 1024;
const GIT_TIMEOUT_MS = 30_000;
const GIT_WRITE_RETRY_COUNT = 3;
const GIT_WRITE_RETRY_DELAY_MS = 120;
const GIT_ERROR_LOG_DIR = path.join(os.tmpdir(), "open-warden-git-logs");

let lastGitCommandErrorLog: { repoPath: string; path: string } | null = null;

class GitCommandError extends Error {
constructor(
readonly args: string[],
readonly stderr: string,
readonly code: number | null,
readonly logPath: string | null,
) {
super(stderr || `git ${args.join(" ")} failed`);
this.name = "GitCommandError";
Expand Down Expand Up @@ -71,6 +75,53 @@ function decodeUtf8(buffer: Buffer, label: string) {
}
}

function commandOutputToString(value: unknown) {
if (Buffer.isBuffer(value)) return value.toString("utf8");
if (typeof value === "string") return value;
return "";
}

function formatGitCommand(args: string[]) {
return `git ${args.join(" ")}`;
}

async function writeGitCommandErrorLog(input: {
repoPath: string;
args: string[];
stderr: string;
stdout: string;
code: number | null;
}) {
try {
await fs.mkdir(GIT_ERROR_LOG_DIR, { recursive: true });
const logPath = path.join(GIT_ERROR_LOG_DIR, `git-${Date.now()}-${process.pid}.log`);
const content = [
`> ${formatGitCommand(input.args)}`,
`cwd: ${input.repoPath}`,
`exit code: ${input.code ?? "unknown"}`,
"",
"stderr:",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

writeGitCommandErrorLog catches all errors silently. If the temp directory cannot be created or written to, users won't get any indication that error logging failed. Consider at least logging to console.error for debugging purposes, though this is acceptable for a diagnostic feature.

input.stderr || "(empty)",
"",
"stdout:",
input.stdout || "(empty)",
"",
].join("\n");

await fs.writeFile(logPath, content, "utf8");
lastGitCommandErrorLog = { repoPath: input.repoPath, path: logPath };
return logPath;
} catch {
return null;
}
}

export async function getLastGitCommandErrorLogPath(repoPath?: string) {
if (!lastGitCommandErrorLog) return null;
if (repoPath && lastGitCommandErrorLog.repoPath !== repoPath) return null;
return lastGitCommandErrorLog.path;
}

async function runGit(
repoPath: string,
args: string[],
Expand All @@ -95,16 +146,24 @@ async function runGit(

const rawCode = "code" in error ? error.code : null;
if (rawCode === "ENOENT") {
throw new GitCommandError(args, "git is not installed or not available in PATH", null);
throw new GitCommandError(args, "git is not installed or not available in PATH", null, null);
}

if ("killed" in error && error.killed) {
throw new GitCommandError(args, `git command timed out after ${GIT_TIMEOUT_MS}ms`, null);
throw new GitCommandError(
args,
`git command timed out after ${GIT_TIMEOUT_MS}ms`,
null,
null,
);
}

const stderr = "stderr" in error ? String(error.stderr ?? "").trim() : error.message;
const stdout = "stdout" in error ? commandOutputToString(error.stdout) : "";
const rawStderr = "stderr" in error ? commandOutputToString(error.stderr) : "";
const stderr = rawStderr.trim() || error.message;
const code = typeof rawCode === "number" ? rawCode : null;
const commandError = new GitCommandError(args, stderr, code);
const logPath = await writeGitCommandErrorLog({ repoPath, args, stderr, stdout, code });
const commandError = new GitCommandError(args, stderr, code, logPath);

if (options?.allowFailure) {
throw commandError;
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@base-ui/react": "^1.1.0",
"@hookform/resolvers": "^5.2.2",
"@m2d/react-markdown": "^1.0.0",
"@pierre/diffs": "1.1.15",
"@pierre/diffs": "1.2.7",
"@pierre/trees": "1.0.0-beta.3",
"@reduxjs/toolkit": "^2.9.0",
"@tanstack/react-hotkeys": "^0.1.0",
Expand Down
18 changes: 14 additions & 4 deletions apps/desktop/src/features/diff-view/DiffWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
} from "@/features/source-control/hunkOperations";
import { DiffViewer, type DiffViewerHandle } from "@/features/diff-view/components/DiffViewer";
import { useDiffCommentAnnotations } from "@/features/diff-view/hooks/useDiffCommentAnnotations";
import { useDiffDiagnostics } from "@/features/diff-view/hooks/useDiffDiagnostics";
import { useMultiDiffDiagnostics } from "@/features/diff-view/hooks/useMultiDiffDiagnostics";
import { useDiffAnnotationRenderer } from "@/features/diff-view/hooks/useDiffAnnotationRenderer";
import { type DiffLineAnnotation, type FileDiffOptions } from "@pierre/diffs";

Expand All @@ -54,6 +54,8 @@
onHunkAction?: (operation: DiffHunkOperation, payload: DiffHunkActionPayload) => void;
};

const SINGLE_DIFF_ITEM_ID = "single-diff";

function buildReturnToDiffTarget(
jumpContextKind: "changes" | "review" | "pull-request",
source: { lineNumber: number; lineIndex: string | null },
Expand Down Expand Up @@ -108,14 +110,14 @@
activePath,
commentContext,
canComment,
lspDiagnostics = [],

Check warning on line 113 in apps/desktop/src/features/diff-view/DiffWorkspace.tsx

View workflow job for this annotation

GitHub Actions / Desktop Format/Lint/Typecheck/Test/Build

react(no-object-type-as-default-prop)

Do not use an array literal as default prop value. Use a stable reference instead.
fileViewerRevision,
lspHoverDocument,
lspJumpContextKind,
focusedLineNumber = null,
focusedLineIndex = null,
focusedLineKey = null,
annotationItems = [],

Check warning on line 120 in apps/desktop/src/features/diff-view/DiffWorkspace.tsx

View workflow job for this annotation

GitHub Actions / Desktop Format/Lint/Typecheck/Test/Build

react(no-object-type-as-default-prop)

Do not use an array literal as default prop value. Use a stable reference instead.
commentMentions,
includeCurrentFileComments = true,
disableFileHeader = false,
Expand Down Expand Up @@ -155,7 +157,11 @@
getReturnToDiffTarget,
});

const diagnostics = useDiffDiagnostics(lspDiagnostics);
const diagnosticsByItem = useMemo(
() => new Map([[SINGLE_DIFF_ITEM_ID, lspDiagnostics]]),
[lspDiagnostics],
);
const diagnostics = useMultiDiffDiagnostics(diagnosticsByItem);

const comments = useDiffCommentAnnotations({
activePath,
Expand Down Expand Up @@ -245,22 +251,26 @@
enableLineSelection: canComment,
enableGutterUtility: canComment,
onTokenClick: handleTokenClick,
onTokenEnter: diagnostics.onTokenEnter,
onTokenEnter: (props) => diagnostics.onTokenEnter(SINGLE_DIFF_ITEM_ID, props),

Check warning on line 254 in apps/desktop/src/features/diff-view/DiffWorkspace.tsx

View workflow job for this annotation

GitHub Actions / Desktop Format/Lint/Typecheck/Test/Build

react-hooks(exhaustive-deps)

React Hook useMemo has a missing dependency: 'diagnostics'
onTokenLeave: diagnostics.onTokenLeave,
onLineSelected: canComment ? comments.onLineSelected : undefined,
onLineSelectionStart: canComment ? comments.onLineSelectionStart : undefined,
onLineSelectionChange: canComment ? comments.onLineSelectionChange : undefined,
onLineSelectionEnd: canComment ? comments.onLineSelectionEnd : undefined,
onPostRender: diagnostics.onPostRender,
onPostRender: (rootNode) => diagnostics.onPostRender(SINGLE_DIFF_ITEM_ID, rootNode),
}),
[
canComment,
comments.onLineSelected,
comments.onLineSelectionStart,
comments.onLineSelectionChange,
comments.onLineSelectionEnd,
diagnostics.onPostRender,
diagnostics.onTokenEnter,
diagnostics.onTokenLeave,
disableFileHeader,
handleTokenClick,
],

Check warning on line 273 in apps/desktop/src/features/diff-view/DiffWorkspace.tsx

View workflow job for this annotation

GitHub Actions / Desktop Format/Lint/Typecheck/Test/Build

react-hooks(exhaustive-deps)

React Hook useMemo has unnecessary dependency: diagnostics.onPostRender

Check warning on line 273 in apps/desktop/src/features/diff-view/DiffWorkspace.tsx

View workflow job for this annotation

GitHub Actions / Desktop Format/Lint/Typecheck/Test/Build

react-hooks(exhaustive-deps)

React Hook useMemo has unnecessary dependency: diagnostics.onTokenEnter
);

const renderHeaderMetadata = useCallback(
Expand Down
Loading
Loading