Unverified 提交 ea36c831 authored 作者: wwwillchen-bot's avatar wwwillchen-bot 提交者: GitHub

fix: check staged changes before committing to prevent 'nothing to commit' error (#2991)

## Summary - Fixes a bug where the response processor would attempt `git commit` even when there were no staged changes, causing "nothing to commit" errors - Adds a `hasStagedChanges` utility function to `git_utils.ts` that checks whether there are actually staged files before committing - Updates the commit flow in `response_processor.ts` to check for staged changes first, skipping the commit when unnecessary ## Test plan - [x] All 897 existing tests pass - [x] Lint, formatting, and type checks pass - Verify that file write/rename/delete operations still commit correctly when changes are staged - Verify that no error is thrown when git operations result in no staged changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2991" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> --------- Co-authored-by: 's avatarClaude <noreply@anthropic.com> Co-authored-by: 's avatarclaude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: 's avatarWill Chen <willchen90@gmail.com>
上级 69a731c3
......@@ -88,12 +88,18 @@ Commit any uncommitted changes, run lint checks, fix any issues, and push the cu
git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null
```
If this succeeds (e.g., returns `origin/my-branch` or `someuser/my-branch`), the branch already has an upstream. Just push:
If this succeeds (e.g., returns `origin/my-branch` or `someuser/my-branch`), the branch already has an upstream. Push to it:
```
git push --force-with-lease
```
**Permission fallback:** If the push fails with a permission error (e.g., the branch tracks `upstream` but the current account lacks write access to `dyad-sh/dyad`), fall back to pushing to `origin` instead:
```
git push --force-with-lease -u origin HEAD
```
b. If there is NO upstream, check if a PR already exists and determine which remote it was opened from:
First, get the PR's head repository as `owner/repo`:
......@@ -181,6 +187,8 @@ Commit any uncommitted changes, run lint checks, fix any issues, and push the cu
gh pr edit --add-label "cc:request"
```
**Permission fallback:** If this fails with a permission error (common for bot/fork accounts that lack label permissions on the upstream repo), skip label addition silently and note in the summary that the label could not be added. Do NOT fail the workflow over a label.
9. **Remove review-issue label:**
After pushing, remove the `needs-human:review-issue` label if it exists (this label indicates the issue needed human review before work started, which is now complete):
......
......@@ -6,6 +6,14 @@ When pushing changes and creating PRs:
2. If the branch hasn't been pushed before, default to pushing to `origin` (the fork `wwwillchen/dyad`), then create a PR from the fork to the upstream repo (`dyad-sh/dyad`).
3. If you cannot push to the fork due to permissions, push directly to `upstream` (`dyad-sh/dyad`) as a last resort.
**Bot account push permissions:** The `wwwillchen-bot` account does NOT have write access to `upstream` (`dyad-sh/dyad`). If a branch tracks `upstream` (e.g., `upstream/claude/...`), pushing will fail with a permission error. In this case, push to `origin` (the bot's fork at `wwwillchen-bot/dyad`) instead:
```bash
git push --force-with-lease -u origin HEAD
```
This overrides the branch's tracking remote. Always check which remote `origin` points to (`git remote -v`) — for bot workspaces, `origin` is typically the bot's fork, not the upstream repo.
## `gh pr create` branch detection
If `gh pr create` says `you must first push the current branch to a remote` even though `git push -u` succeeded, create the PR with an explicit head ref:
......@@ -60,12 +68,16 @@ gh api graphql --input .claude/tmp/resolve_thread.json
## Adding labels to PRs
`gh pr edit --add-label` fails with a GraphQL "Projects (classic)" deprecation error on repos that had classic projects. Use the REST API instead:
`gh pr edit --add-label` can fail for two reasons:
1. **GraphQL "Projects (classic)" deprecation error** on repos that had classic projects. Use the REST API instead:
```bash
gh api repos/dyad-sh/dyad/issues/{PR_NUMBER}/labels -f "labels[]=label-name"
```
2. **Bot account permission errors:** The `wwwillchen-bot` account (and similar bot/fork accounts) may not have permission to add labels on the upstream repo (`dyad-sh/dyad`). Both `gh pr edit --add-label` and the REST API will fail with 403/permission errors. In this case, skip label addition and note it in the PR summary rather than failing the workflow. Labels can be added later by a maintainer with appropriate permissions.
## CI file access (claude-code-action)
In CI, `claude-code-action` restricts file access to the repo working directory (e.g., `/home/runner/work/dyad/dyad`). Skills that save intermediate files (like PR diffs) must use `./filename` (current working directory), **never** `/tmp/`. Using `/tmp/` causes errors like: `cat in '/tmp/pr_*_diff.patch' was blocked. For security, Claude Code may only concatenate files from the allowed working directories`.
......
......@@ -56,6 +56,7 @@ vi.mock("../ipc/utils/git_utils", () => ({
gitSetRemoteUrl: vi.fn(),
gitStatus: vi.fn().mockResolvedValue([]),
getGitUncommittedFiles: vi.fn().mockResolvedValue([]),
hasStagedChanges: vi.fn().mockResolvedValue(true),
}));
// Mock paths module to control getDyadAppPath
......
......@@ -26,6 +26,7 @@ import {
gitRemove,
gitAddAll,
getGitUncommittedFiles,
hasStagedChanges,
} from "../utils/git_utils";
import { readSettings } from "@/main/settings";
import { writeMigrationFile } from "../utils/file_utils";
......@@ -558,45 +559,59 @@ export async function processFullResponseActions(
let message = chatSummary
? `[dyad] ${chatSummary} - ${changes.join(", ")}`
: `[dyad] ${changes.join(", ")}`;
// Use chat summary, if provided, or default for commit message
let commitHash = await gitCommit({
path: appPath,
message,
});
logger.log(`Successfully committed changes: ${changes.join(", ")}`);
// Check for any uncommitted changes after the commit
uncommittedFiles = await getGitUncommittedFiles({ path: appPath });
// Verify there are actual staged changes before committing.
// Files may be "written" with identical content (e.g. regenerated by the AI),
// resulting in no actual git diff and causing "nothing to commit" errors.
// We check staged changes specifically (not the entire working tree) to avoid
// false positives from unrelated unstaged/untracked files.
const staged = await hasStagedChanges({ path: appPath });
if (!staged) {
logger.log(
"No actual git changes detected after staging (files may have been rewritten with identical content), skipping commit",
);
hasChanges = false;
} else {
// Use chat summary, if provided, or default for commit message
let commitHash = await gitCommit({
path: appPath,
message,
});
logger.log(`Successfully committed changes: ${changes.join(", ")}`);
if (uncommittedFiles.length > 0) {
// Stage all changes
await gitAddAll({ path: appPath });
try {
commitHash = await gitCommit({
path: appPath,
message: message + " + extra files edited outside of Dyad",
amend: true,
});
logger.log(
`Amend commit with changes outside of dyad: ${uncommittedFiles.join(", ")}`,
);
} catch (error) {
// Just log, but don't throw an error because the user can still
// commit these changes outside of Dyad if needed.
logger.error(
`Failed to commit changes outside of dyad: ${uncommittedFiles.join(", ")}`,
);
extraFilesError = (error as any).toString();
// Check for any uncommitted changes after the commit
uncommittedFiles = await getGitUncommittedFiles({ path: appPath });
if (uncommittedFiles.length > 0) {
// Stage all changes
await gitAddAll({ path: appPath });
try {
commitHash = await gitCommit({
path: appPath,
message: message + " + extra files edited outside of Dyad",
amend: true,
});
logger.log(
`Amend commit with changes outside of dyad: ${uncommittedFiles.join(", ")}`,
);
} catch (error) {
// Just log, but don't throw an error because the user can still
// commit these changes outside of Dyad if needed.
logger.error(
`Failed to commit changes outside of dyad: ${uncommittedFiles.join(", ")}`,
);
extraFilesError = (error as any).toString();
}
}
}
// Save the commit hash to the message
await db
.update(messages)
.set({
commitHash: commitHash,
})
.where(eq(messages.id, messageId));
// Save the commit hash to the message
await db
.update(messages)
.set({
commitHash: commitHash,
})
.where(eq(messages.id, messageId));
}
}
logger.log("mark as approved: hasChanges", hasChanges);
// Update the message to approved
......
......@@ -259,6 +259,29 @@ export async function isGitStatusClean({
}
}
export async function hasStagedChanges({
path,
}: {
path: string;
}): Promise<boolean> {
const settings = readSettings();
if (settings.enableNativeGit) {
// git diff --cached --quiet exits with 1 if there are staged changes, 0 if none
const result = await execGit(["diff", "--cached", "--quiet"], path);
if (result.exitCode !== 0 && result.exitCode !== 1) {
throw new Error(
`Failed to check staged changes: ${result.stderr.trim() || result.stdout.trim()}`,
);
}
return result.exitCode === 1;
} else {
const statusMatrix = await git.statusMatrix({ fs, dir: path });
// row[1] = HEAD status, row[3] = stage status
// If stage differs from HEAD, there are staged changes
return statusMatrix.some((row) => row[3] !== row[1]);
}
}
export async function gitCommit({
path,
message,
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论