Skip to content
Closed
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
305 changes: 290 additions & 15 deletions apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { TRPCError } from "@trpc/server";
import { shell } from "electron";
import simpleGit from "simple-git";
import { z } from "zod";
import { publicProcedure, router } from "../..";
Expand Down Expand Up @@ -75,6 +74,155 @@ async function pushWithSetUpstream({
]);
}

function _buildCommitMessagePrompt({
stagedFiles,
diffStat,
detailedDiff,
}: {
stagedFiles: string;
diffStat: string;
detailedDiff: string;
}): string {
return [
"Generate a concise conventional commit message for these changes.",
"Use format: <type>(<scope>): <description>",
"Types: feat, fix, refactor, docs, style, test, chore, perf",
"Keep the description under 72 characters.",
"Only output the commit message, nothing else.",
"",
"Files changed:",
stagedFiles,
"",
"Diff stat:",
diffStat,
"",
"Diff:",
detailedDiff,
].join("\n");
}
Comment on lines +77 to +102

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n '_buildCommitMessagePrompt' --type ts --type tsx

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

rg -n '_buildCommitMessagePrompt'

Repository: superset-sh/superset

Length of output: 163


🏁 Script executed:

# Check if _buildCommitMessagePrompt is exported or used as a callback/reference anywhere
rg -n 'buildCommitMessagePrompt' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts

Repository: superset-sh/superset

Length of output: 103


🏁 Script executed:

# Check entire codebase for any usage outside the definition
rg '_buildCommitMessagePrompt' --count-matches

Repository: superset-sh/superset

Length of output: 125


🏁 Script executed:

# Examine the generateCommitMessage and generateMessageFromDiff functions to confirm they don't call _buildCommitMessagePrompt
rg -A 20 'function generateMessageFromDiff' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts

Repository: superset-sh/superset

Length of output: 895


🏁 Script executed:

# Check for any generateCommitMessage procedure that might call _buildCommitMessagePrompt
rg -A 30 'generateCommitMessage.*:.*procedure' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts | head -40

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Search for generateCommitMessage procedure definition
rg -B 5 -A 30 'generateCommitMessage' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts | head -50

Repository: superset-sh/superset

Length of output: 1221


🏁 Script executed:

# Get the complete generateCommitMessage procedure to see what it returns
rg -A 40 'generateCommitMessage: publicProcedure' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts

Repository: superset-sh/superset

Length of output: 1154


_buildCommitMessagePrompt is dead code — never called anywhere.

This function builds a prompt string but is never invoked. The generateCommitMessage procedure delegates to the heuristic-based generateMessageFromDiff instead. Remove it if it's a leftover from an earlier LLM-based approach, or wire it up if intentional.

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 77
- 102, _buildCommitMessagePrompt is unused dead code; either remove it or wire
it into the commit-message flow. If you intend to keep an LLM-based generator,
update generateCommitMessage (or the function that currently calls
generateMessageFromDiff) to call _buildCommitMessagePrompt with {stagedFiles,
diffStat, detailedDiff} and pass its result to the LLM invocation; otherwise
delete the _buildCommitMessagePrompt function and any imports/comments
referencing it to avoid unused code warnings.


interface DiffContext {
stagedFiles: string;
diffStat: string;
detailedDiff: string;
}

async function generateMessageFromDiff(ctx: DiffContext): Promise<string> {
// Parse file operations from name-status output
const lines = ctx.stagedFiles.trim().split("\n").filter(Boolean);
const ops: { type: string; file: string }[] = lines.map((line) => {
const [status, ...rest] = line.split("\t");
return { type: status.trim(), file: rest.join("\t").trim() };
});

if (ops.length === 0) {
return "chore: update files";
}

// Detect common patterns
const allAdded = ops.every((o) => o.type.startsWith("A"));
const allDeleted = ops.every((o) => o.type.startsWith("D"));
const allRenamed = ops.every((o) => o.type.startsWith("R"));
const _allModified = ops.every((o) => o.type.startsWith("M"));

// Extract scope from common directory
const paths = ops.map((o) => o.file);
const scope = getCommonScope(paths);
const scopePart = scope ? `(${scope})` : "";

// Detect type from diff content
const diffLower = ctx.detailedDiff.toLowerCase();
const isTest =
paths.some((p) => p.includes("test") || p.includes("spec")) ||
diffLower.includes("describe(") ||
diffLower.includes("it(") ||
diffLower.includes("test(");
const isDocs = paths.some(
(p) => p.endsWith(".md") || p.includes("docs/") || p.includes("README"),
);

if (ops.length === 1) {
const op = ops[0];
const fileName = op.file.split("/").pop() || op.file;
if (op.type.startsWith("A")) {
const type = isTest ? "test" : isDocs ? "docs" : "feat";
return `${type}${scopePart}: add ${fileName}`;
}
if (op.type.startsWith("D")) {
return `chore${scopePart}: remove ${fileName}`;
}
if (op.type.startsWith("R")) {
return `refactor${scopePart}: rename ${fileName}`;
}
}

// Multi-file operations
if (allAdded) {
const type = isTest ? "test" : isDocs ? "docs" : "feat";
return `${type}${scopePart}: add ${ops.length} files`;
}
if (allDeleted) {
return `chore${scopePart}: remove ${ops.length} files`;
}
if (allRenamed) {
return `refactor${scopePart}: rename ${ops.length} files`;
}

// Analyze diff for fix indicators
const isFix =
diffLower.includes("fix") ||
diffLower.includes("bug") ||
diffLower.includes("error") ||
diffLower.includes("issue");

const type = isTest ? "test" : isDocs ? "docs" : isFix ? "fix" : "feat";
Comment on lines +170 to +178

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

isFix heuristic has a high false-positive rate.

Matching "fix", "bug", "error", or "issue" against the lowercased diff content will trigger on extremely common patterns — error handling code, CSS prefixes (-webkit-→ no, but fix appears in "prefix"), import of ErrorBoundary, issue tracker URLs in comments, etc. This could mislabel many commits as fix when they're actually feat or refactor.

Consider narrowing the heuristic to only match added lines (+ prefixed lines in the diff) or to match more specific patterns like fixes #, bugfix, or commit-message-like patterns rather than raw diff content.

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 170
- 178, The isFix heuristic (diffLower and isFix) is too broad and causes false
positives; change isFix to only inspect added lines from the diff (lines
starting with '+') and match narrower patterns such as /\bfixes?\b/,
/\bbugfix\b/, /^fix:/, /fixes\s+#\d+/ or similar commit-like tokens rather than
any occurrence of "fix", "bug", "error", or "issue"; then use that tightened
isFix when computing type (alongside existing isTest and isDocs) so type
assignment remains correct (the symbols to update are isFix, diffLower usage,
and the type = isTest ? ... line).


// Generate a short description from the filenames
const fileNames = paths.map((p) => p.split("/").pop()).filter(Boolean);
const uniqueNames = [...new Set(fileNames)];
const desc =
uniqueNames.length <= 3
? `update ${uniqueNames.join(", ")}`
: `update ${uniqueNames.length} files`;

return `${type}${scopePart}: ${desc}`;
}

function getCommonScope(paths: string[]): string {
if (paths.length === 0) return "";

const parts = paths.map((p) => p.split("/"));
const minLen = Math.min(...parts.map((p) => p.length));

let commonDepth = 0;
for (let i = 0; i < minLen - 1; i++) {
const segment = parts[0][i];
if (parts.every((p) => p[i] === segment)) {
commonDepth = i + 1;
} else {
break;
}
}

if (commonDepth === 0) return "";

// Use the deepest common directory as scope
const scopePath = parts[0].slice(0, commonDepth).join("/");

// Simplify known directory patterns
if (scopePath.includes("apps/")) {
const match = scopePath.match(/apps\/([^/]+)/);
if (match) return match[1];
}
if (scopePath.includes("packages/")) {
const match = scopePath.match(/packages\/([^/]+)/);
if (match) return match[1];
}

// Return the last segment of the common path
return parts[0][commonDepth - 1];
}

function shouldRetryPushWithUpstream(message: string): boolean {
const lowerMessage = message.toLowerCase();
return (
Expand All @@ -93,6 +241,45 @@ export const createGitOperationsRouter = () => {
// NOTE: saveFile is defined in file-contents.ts with hardened path validation
// Do NOT add saveFile here - it would overwrite the secure version

generateCommitMessage: publicProcedure
.input(
z.object({
worktreePath: z.string(),
}),
)
.mutation(async ({ input }): Promise<{ message: string }> => {
assertRegisteredWorktree(input.worktreePath);

const git = simpleGit(input.worktreePath);

// Get the staged diff
const diff = await git.diff(["--cached", "--stat"]);
if (!diff.trim()) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "No staged changes to generate a message for",
});
}

// Get detailed diff (limited to avoid huge payloads)
const detailedDiff = await git.diff(["--cached"]);
const truncatedDiff =
detailedDiff.length > 8000
? `${detailedDiff.slice(0, 8000)}\n... (truncated)`
: detailedDiff;

// Get the list of staged files for context
const stagedFiles = await git.diff(["--cached", "--name-status"]);

const message = await generateMessageFromDiff({
stagedFiles,
diffStat: diff,
detailedDiff: truncatedDiff,
});

return { message };
}),

commit: publicProcedure
.input(
z.object({
Expand Down Expand Up @@ -210,10 +397,20 @@ export const createGitOperationsRouter = () => {
.input(
z.object({
worktreePath: z.string(),
title: z.string().optional(),
body: z.string().optional(),
draft: z.boolean().optional(),
baseBranch: z.string().optional(),
}),
)
.mutation(
async ({ input }): Promise<{ success: boolean; url: string }> => {
async ({
input,
}): Promise<{
success: boolean;
url: string;
number: number;
}> => {
assertRegisteredWorktree(input.worktreePath);

const git = simpleGit(input.worktreePath);
Expand All @@ -224,7 +421,6 @@ export const createGitOperationsRouter = () => {
if (!hasUpstream) {
await pushWithSetUpstream({ git, branch });
} else {
// Push any unpushed commits
try {
await git.push();
} catch (error) {
Expand All @@ -238,26 +434,105 @@ export const createGitOperationsRouter = () => {
}
}

// Get the remote URL to construct the GitHub compare URL
const remoteUrl = (await git.remote(["get-url", "origin"])) || "";
const repoMatch = remoteUrl
.trim()
.match(/github\.com[:/](.+?)(?:\.git)?$/);
// Build gh pr create arguments
const args = ["pr", "create"];

const prTitle = input.title || branch.replace(/[-_/]/g, " ").trim();
args.push("--title", prTitle);

if (input.body) {
args.push("--body", input.body);
} else {
args.push("--body", "");
}

if (input.draft) {
args.push("--draft");
}

if (!repoMatch) {
throw new Error("Could not determine GitHub repository URL");
if (input.baseBranch) {
args.push("--base", input.baseBranch);
}

const repo = repoMatch[1].replace(/\.git$/, "");
const url = `https://github.com/${repo}/compare/${branch}?expand=1`;
try {
const { stdout } = await execWithShellEnv("gh", args, {
cwd: input.worktreePath,
});
const url = stdout.trim();

// Extract PR number from URL (e.g., https://github.com/org/repo/pull/123)
const prNumberMatch = url.match(/\/pull\/(\d+)/);
const prNumber = prNumberMatch
? Number.parseInt(prNumberMatch[1], 10)
: 0;

await fetchCurrentBranch(git);

await shell.openExternal(url);
await fetchCurrentBranch(git);
return { success: true, url, number: prNumber };
} catch (error) {
const message =
error instanceof Error ? error.message : String(error);
console.error("[git/createPR] Failed to create PR:", message);

return { success: true, url };
if (message.includes("already exists")) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "A pull request already exists for this branch",
});
}

throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: `Failed to create PR: ${message}`,
});
}
},
),

generatePRBody: publicProcedure
.input(
z.object({
worktreePath: z.string(),
baseBranch: z.string().optional(),
}),
)
.query(async ({ input }): Promise<{ title: string; body: string }> => {
assertRegisteredWorktree(input.worktreePath);

const git = simpleGit(input.worktreePath);
const branch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim();
const base = input.baseBranch || "main";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded "main" fallback won't work for repos using "master" or other default branches.

When baseBranch is not provided, this falls back to "main". Repositories using master, develop, or other conventions will get an empty or incorrect commit log since origin/main..HEAD won't resolve. Consider detecting the default branch dynamically, e.g., via git remote show origin or gh repo view --json defaultBranchRef.

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` at line 504, The
fallback to a hardcoded "main" in the assignment const base = input.baseBranch
|| "main" is brittle for repos using "master" or other defaults; change logic in
the git-operations handler to resolve the remote default branch when
input.baseBranch is absent (e.g., run a git query such as `git remote show
origin` or `git rev-parse --abbrev-ref origin/HEAD` or use the GitHub API/gh CLI
to fetch defaultBranchRef) and assign that result to base, falling back to a
sensible last-resort value only if detection fails; update any callers that
expect base to be present to handle async detection and surface detection errors
via the existing error path in the surrounding function.


// Get commit log for the branch
let logOutput = "";
try {
logOutput = await git.raw([
"log",
`origin/${base}..HEAD`,
"--format=%s",
]);
} catch {
// Fall back to just the branch name
}
Comment on lines +513 to +516

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent catch block swallows errors without logging.

This violates the guideline to never swallow errors silently. If git log fails for a reason other than a missing base branch (e.g., corrupt repo, permission error), the failure is silently ignored and the user gets an empty PR body with no indication of the problem.

Proposed fix
 			} catch {
-				// Fall back to just the branch name
+				catch (error) {
+					console.warn("[git/generatePRBody] Failed to get commit log, falling back to branch name:", error);
+				}

As per coding guidelines: "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly"

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 513
- 516, The empty catch after the git log call swallows all errors; replace it
with handling that logs the failure (including error.message and error.stack)
with context (e.g., "failed to run git log to generate PR body from base branch
<baseBranch>") and only fall back to the simple branch name for the known
"missing base branch" case — for other errors rethrow (or propagate) so callers
can surface the failure; implement this change in the git log handling block in
git-operations.ts (the catch directly after the git log invocation that builds
the PR body).


const commits = logOutput.trim().split("\n").filter(Boolean);

// Generate title from branch name
const title = branch
.replace(/^(feat|fix|chore|refactor|docs|test|perf)[/-]/i, "")
.replace(/[-_/]/g, " ")
.replace(/\b\w/g, (c) => c.toUpperCase())
.trim();

// Generate body from commits
const body =
commits.length > 0
? `## Changes\n\n${commits.map((c) => `- ${c}`).join("\n")}`
: "";

return { title, body };
}),

mergePR: publicProcedure
.input(
z.object({
Expand Down
Loading