fix: check staged changes before committing to prevent 'nothing to commit' error#2991
Conversation
…ng to commit" error When the AI regenerates a file with identical content, fs.writeFileSync succeeds but git has no actual diff to commit. The response_processor previously determined hasChanges based on whether files were processed (written/renamed/deleted) rather than whether git actually had staged changes, causing "Failed to create commit. nothing to commit, working tree clean" errors. Now checks isGitStatusClean() after staging files but before calling gitCommit(). If the working tree is clean (no actual diff), we skip the commit gracefully instead of throwing an error. Fixes dyad-sh#2805 https://claude.ai/code/session_015X6u6UE1z6GLUr2MvLL1yk
…tting Replace isGitStatusClean (git status --porcelain) with new hasStagedChanges (git diff --cached --quiet) to correctly detect whether there are staged changes to commit. The previous approach would fail when unrelated unstaged/untracked files existed in the working tree. Co-Authored-By: Claude Opus 4.5 <[email protected]>
The test mock was missing the new hasStagedChanges function, causing vitest to fail with 'No export defined on mock' error. Co-Authored-By: Claude Opus 4.5 <[email protected]>
git diff --cached --quiet can return exit codes >1 for errors (e.g., not inside a repo, I/O failure). Previously these were conflated with exit code 1 (has staged changes), silently masking errors. Now explicitly throw for unexpected exit codes, matching the pattern used by other execGit calls in this file. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The import was flagged by oxlint as unused, causing CI build failure. Co-Authored-By: Claude Opus 4.6 <[email protected]>
wwwillchen-bot
had a problem deploying
to
ai-bots
GitHub Actions
Failure
|
@BugBot run |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the application's Git integration by preventing unnecessary commit attempts. It introduces a pre-commit check to ensure that Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug where git commit could be called without any staged changes, leading to an error. The introduction of the hasStagedChanges utility is a clean solution, and the implementation for both native git and isomorphic-git appears correct. My review includes a couple of suggestions to improve maintainability and test coverage for the new logic, with one suggestion emphasizing readability as per repository guidelines.
| gitSetRemoteUrl: vi.fn(), | ||
| gitStatus: vi.fn().mockResolvedValue([]), | ||
| getGitUncommittedFiles: vi.fn().mockResolvedValue([]), | ||
| hasStagedChanges: vi.fn().mockResolvedValue(true), |
There was a problem hiding this comment.
While this mock is necessary, the pull request would be strengthened by adding a new test case to verify the new behavior. Specifically, a test that mocks hasStagedChanges to return false and asserts that gitCommit is not called, ensuring the commit-skipping logic is working as intended.
| 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]); |
There was a problem hiding this comment.
The logic statusMatrix.some((row) => row[3] !== row[1]) is correct but a bit cryptic. For better maintainability, consider adding a more detailed comment explaining why this comparison works for detecting staged changes with isomorphic-git's status matrix. This would help future developers understand the different cases (new, modified, deleted files) and the meaning of the numeric status codes.
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. This works because of the
// status codes used by isomorphic-git's statusMatrix:
// - head: 0=absent, 1=present
// - stage: 0=absent, 1=identical to HEAD, 2=added, 3=modified
// This correctly covers new (0 vs 2), modified (1 vs 3), and deleted (1 vs 0)
// staged files, while ignoring unstaged changes.
return statusMatrix.some((row) => row[3] !== row[1]);References
- This comment aligns with the principle of favoring readability over conciseness, as detailed in the rule about explicit if/else blocks, by suggesting adding comments to clarify cryptic logic for better maintainability.
wwwillchen-bot
had a problem deploying
to
ai-bots
GitHub Actions
Failure
|
@BugBot run |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes a bug where Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[processFullResponseActions] --> B{hasChanges?\nwrittenFiles / renamedFiles /\ndeletedFiles / packages}
B -- No --> Z[Return updatedFiles: false]
B -- Yes --> C[gitAdd staged files\ngitRemove deleted files\ngitAdd renamed files]
C --> D{hasStagedChanges?}
D -- No staged diff\ne.g. AI wrote identical content --> E[Log: skipping commit\nhasChanges = false]
E --> Z
D -- Yes staged diff --> F[gitCommit initial commit]
F --> G{getGitUncommittedFiles?\nfiles changed outside Dyad}
G -- None --> H[Save commitHash to DB]
G -- Found --> I[gitAddAll\ngitCommit --amend]
I --> J{amend succeeded?}
J -- Yes --> H
J -- No --> K[Log error\nset extraFilesError]
K --> H
H --> L[Return updatedFiles: true]
Last reviewed commit: d343737 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
- There is a concrete regression risk in
src/ipc/processors/response_processor.ts: derivingupdatedFilesfrom staged Git state can misclassify ignored-file writes as unchanged, which can affect user-visible update handling. - Given the issue’s medium severity (6/10) and high confidence (9/10), this carries some real merge risk rather than a purely cosmetic concern.
- Pay close attention to
src/ipc/processors/response_processor.ts-updatedFileschange detection may miss ignored-file modifications.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/ipc/processors/response_processor.ts">
<violation number="1" location="src/ipc/processors/response_processor.ts:568">
P2: Don't derive `updatedFiles` from staged git state; ignored-file writes will now be treated as if nothing changed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // 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 }); |
There was a problem hiding this comment.
P2: Don't derive updatedFiles from staged git state; ignored-file writes will now be treated as if nothing changed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ipc/processors/response_processor.ts, line 568:
<comment>Don't derive `updatedFiles` from staged git state; ignored-file writes will now be treated as if nothing changed.</comment>
<file context>
@@ -558,45 +559,59 @@ export async function processFullResponseActions(
+ // 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(
</file context>
github-actions
bot
added
the
needs-human:review-issue
🎭 Playwright Test Results❌ Some tests failed
Summary: 785 passed, 3 failed, 7 flaky, 246 skipped Failed Tests🪟 Windows
|
3 participants
Summary
git commiteven when there were no staged changes, causing "nothing to commit" errorshasStagedChangesutility function togit_utils.tsthat checks whether there are actually staged files before committingresponse_processor.tsto check for staged changes first, skipping the commit when unnecessaryTest plan
🤖 Generated with Claude Code