Add commit + push funcionality to git

[Campalans03]

Summary

Adds a button in GitCommitDialog.fxml that allows committing and pushing in a single action. Refactored GitCommitDialogViewModel with new commitAndPush methods that reuse the existing commit and push implementations, and added custom error messages for partial failures (e.g. commit succeeds but push fails) and total failures.

Steps to test

1. Open JabRef with a configured Git repository and pending changes.
2. Open the Git Commit Dialog.
3. Verify the new button to commit and push in a single action appears.
4. Confirm both operations complete successfully in a normal flow.
5. Trigger a failure (e.g. no connection, remote conflict) to verify the custom error messages, for both partial and total failure cases.
6. See attached screenshots.

Related issues and pull requests

Closes #16339

AI usage

Claude Sonnet 5 — used to search for already-implemented functions in the codebase (existing commit/push logic) to avoid duplication. The code was manually written, reviewed and tested.

AI CHECKLIST.md walkthrough

1. Code self-review

Nullability and control flow

• No == null / != null checks — commitOn() now uses StringUtil.isBlank(message).
• No Objects.requireNonNull(...) — none introduced.
• [/] New classes annotated with @NullMarked. ResolvedRepository, CommitOutcome, and PushFailedException are new types introduced in this PR and are still not annotated.
• [/] Optional consumed with ifPresent/ifPresentOrElse/map/orElseThrow. Not applicable: the isEmpty() + .get() block in resolveRepository() is pre-existing code, unchanged in this diff (moved verbatim from the original doCommit()).
• StringUtil.isBlank(...) used instead of s == null || s.isBlank().

Exceptions

• No catch (Exception e) — only specific exceptions caught (JabRefException | GitAPIException | IOException).
• No throw new RuntimeException(...) / IllegalStateException(...).
• [/] Logged exceptions passed as last logger argument — not applicable, no LOGGER calls added.

Style and idioms

• [/] New BibEntry objects built with withers — not applicable, no BibEntry created.
• [/] Modern Java (List.of(), Path.of(), text blocks, etc.) — not applicable, none introduced.
• [/] Regexes use precompiled Pattern — not applicable, no regex used.
• Background work uses BackgroundTask — BackgroundTask.wrap(...) used for both commit and commit+push.
• No commented-out code, trivial comments, or AI-disclosure comments in source.
• Markdown Javadoc (///) uses Markdown syntax, not JavaDoc inline tags — used correctly on CommitOutcome and PushFailedException.

User-facing text

• All user-facing text localized (Localization.lang / % prefix in FXML) — new strings "Commit and push" and "Committed and pushed successfully" added to JabRef_en.properties.
• Sentence case; no trailing !; labels don't end with : — new strings follow sentence case.
• [/] Variance expressed with placeholders — not applicable, no interpolated strings added.

Security

• [/] HTML-escaping of user-controlled data — not applicable, no HTML output involved.

Tests

• [/] Behavior changes in org.jabref.model / org.jabref.logic have tests — not applicable: all new code lives in org.jabref.gui.git (ViewModel/View layer). The reused push logic (GitSyncService) is pre-existing and already covered by GitSyncServiceTest.
• New/updated tests for the changed behavior — no GitCommitDialogViewModelTest was added for the new commitAndPush flow.

2. Verification commands

• ./gradlew :jablib:check / ./gradlew check
• ./gradlew checkstyleMain checkstyleTest checkstyleJmh
• ./gradlew modernizer
• ./gradlew --no-configuration-cache :rewriteDryRun
• ./gradlew javadoc
• [/] npx markdownlint-cli2 "docs/**/*.md" "*.md" — not applicable, no Markdown changed except CHANGELOG.md

(not run in this review pass — must be run locally before opening the PR)

3. Documentation

• CHANGELOG.md entry added, end-user wording, links issue Add Commit and Push Feature #16339.
• Searched jabref/issues for a related issue — confidently linked to Add Commit and Push Feature #16339.
• Requirement added to docs/requirements/<area>.md — no git.md exists under docs/requirements; this is a new user-facing feature, consider adding one.
• [/] Developer documentation under docs/ — not applicable, no architecture change (wires existing GitSyncService).

4. Pull request

• PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
• All checklist items kept and marked.
• All HTML comments removed from the PR body — verify before submitting.
• PR created with gh pr create --body-file <file>.
• [/] CHANGELOG.md TODO placeholder replaced — not applicable, issue number was already known.

Images

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
• I manually tested my changes in running JabRef (always required)
• [/] I added JUnit tests for changes (if applicable)
• I added screenshots in the PR description (if change is visible to the user)
• I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
• I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
• [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

[github-actions]

Hey @Campalans03! 👋

Thank you for contributing to JabRef!

We have automated checks in place, based on which you will soon get feedback if any of them are failing. We also use Qodo for review assistance. It will update your pull request description with a review help and offer suggestions to improve the pull request.

After all automated checks pass, a maintainer will also review your contribution. Once that happens, you can go through their comments in the "Files changed" tab and act on them, or reply to the conversation if you have further inputs. You can read about the whole pull request process in our contribution guide.

Please ensure that your pull request is in line with our AI Usage Policy and make necessary disclosures.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add one-click Commit + Push action to Git Commit dialog

✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

AI Description

• Add a "Commit and push" button to run both Git operations in one action.
• Refactor the dialog ViewModel to reuse existing commit/push logic with clearer outcomes.
• Show push-specific error messaging when commit succeeds but push fails.

Diagram

graph TD
  F["GitCommitDialog.fxml"] --> V["GitCommitDialogView"] --> VM["GitCommitDialogViewModel"]
  VM --> H(["GitHandler (commit)"])
  VM --> S(["GitSyncService (push)"]) --> R[("Remote Git repo")]
  VM --> P(["GuiPreferences"])
  VM --> D(["DialogService"])

  subgraph Legend
    direction LR
    _ui["UI / File"] ~~~ _svc(["Service"]) ~~~ _ext[("External/Remote")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Move commit+push orchestration into logic layer (service method)

• ➕ Keeps GUI ViewModel thin; orchestration becomes reusable for other UIs/commands
• ➕ Easier to unit test business flow without JavaFX/mvvmfx scaffolding
• ➖ Requires introducing/choosing the right service abstraction boundary (GitHandler vs GitSyncService vs new orchestrator)
• ➖ Slightly more refactoring than needed for a UI-only feature

2. Add dedicated ViewModel tests for commitAndPush outcomes

• ➕ Locks in behavior for partial failures (commit succeeds, push fails) and success/no-op push cases
• ➕ Reduces regression risk around exception-to-dialog-title mapping
• ➖ Test harness may require additional mocking of GitHandlerRegistry/GitSyncService construction
• ➖ More upfront work for a relatively small feature

Recommendation: The PR’s approach (reusing existing commit logic and GitSyncService.push, with explicit outcome modeling and a push-specific exception) is a good fit for a UI-driven feature and avoids duplicating sync logic. The main follow-up worth considering is adding focused ViewModel tests for the new commitAndPush path; moving orchestration into the logic layer is optional unless other entry points will soon need the same combined operation.

Files changed (5) +94 / -10

Enhancement (4) +93 / -10

GitCommitDialogView.javaWire dialog button to commit vs commit+push actions +10/-2

Wire dialog button to commit vs commit+push actions

• Adds a new injected GuiPreferences dependency and a commitAndPushButton reference. Updates the result converter to call either commit() or commitAndPush() depending on which button was pressed.

jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogView.java

GitCommitDialogViewModel.javaImplement commit+push workflow with outcome-based messaging +80/-8

Implement commit+push workflow with outcome-based messaging

• Refactors commit execution into a shared commitAction that runs BackgroundTasks returning a CommitOutcome. Adds repository resolution reuse, a push step using GitSyncService, and a PushFailedException to distinguish push failures (commit already done) for user-facing error titles and messages.

jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java

GitCommitDialog.fxmlAdd Commit and push button to Git commit dialog +1/-0

Add Commit and push button to Git commit dialog

• Introduces a new ButtonType (APPLY) labeled via localization key "%Commit and push" alongside the existing Commit and Cancel buttons.

jabgui/src/main/resources/org/jabref/gui/git/GitCommitDialog.fxml

JabRef_en.propertiesAdd English strings for commit+push UI and success message +2/-0

Add English strings for commit+push UI and success message

• Adds localized strings for the new "Commit and push" button and the "Committed and pushed successfully" notification.

jablib/src/main/resources/l10n/JabRef_en.properties

Documentation (1) +1 / -0

CHANGELOG.mdDocument new Commit and push action +1/-0

Document new Commit and push action

• Adds a changelog entry announcing the new combined commit+push button and its Git error handling behavior.

CHANGELOG.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. New types lack @NullMarked ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

Newly added types (ResolvedRepository, CommitOutcome, PushFailedException) are introduced
without @NullMarked, weakening the default non-null contract expected for new code. This reduces
static null-safety and makes nullability expectations less explicit.

Code

jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[R190-205]

+    private record ResolvedRepository(BibDatabaseContext database, Path bibFilePath, GitHandler gitHandler) {
+    }
+
+    /// What a finished dialog action actually achieved, so the user can be told the truth about it.
+    private enum CommitOutcome {
+        COMMITTED,
+        COMMITTED_AND_PUSHED,
+        COMMITTED_WITHOUT_PUSH
+    }
+
+    /// Marks a failure that happened once the commit already existed, so the user is not told the commit itself failed.
+    private static class PushFailedException extends JabRefException {
+        PushFailedException(Throwable cause) {
+            super(cause.getLocalizedMessage(), cause);
+        }
+    }

Evidence

PR Compliance ID 7 requires new types to be annotated with @NullMarked to enforce explicit
nullability defaults. The PR adds ResolvedRepository, CommitOutcome, and PushFailedException
without any @NullMarked annotation in their declarations.

AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs: AGENTS.md: Enforce explicit nullability with JSpecify and prohibit null in new public APIs
jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[190-205]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Newly introduced types are missing `@NullMarked`, so their nullability defaults are not explicitly declared.
## Issue Context
The compliance checklist requires new classes/types to be explicit about nullability and to default to non-null via JSpecify (`@NullMarked`).
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[190-205]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Null push failure message ✓ Resolved 🐞 Bug ◔ Observability

Description

PushFailedException sets its message to cause.getLocalizedMessage(), which is allowed to be null;
commitAction then passes ex.getMessage() as the dialog content. This can produce a push-failure
dialog with no meaningful error description even though the underlying exception is available as the
cause.

Code

jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[R200-204]

+    /// Marks a failure that happened once the commit already existed, so the user is not told the commit itself failed.
+    private static class PushFailedException extends JabRefException {
+        PushFailedException(Throwable cause) {
+            super(cause.getLocalizedMessage(), cause);
+        }

Evidence

The dialog content is populated from ex.getMessage(), and the newly introduced
PushFailedException can produce a null message by calling cause.getLocalizedMessage() directly;
the dialog service then sets that content string onto the UI dialog.

jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[74-88]
jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[200-205]
jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[254-260]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PushFailedException` uses `cause.getLocalizedMessage()` as the exception message. Since `getLocalizedMessage()` may be `null`, the error dialog content (fed by `ex.getMessage()`) may end up empty/unhelpful for some push failures.
## Issue Context
- `commitAction` shows the error dialog with `ex.getMessage()` as the content.
- `PushFailedException` currently sets its message to `cause.getLocalizedMessage()` without a fallback.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[74-88]
- jabgui/src/main/java/org/jabref/gui/git/GitCommitDialogViewModel.java[200-205]
## Suggested change
In `PushFailedException`, compute a non-null, non-blank message, e.g.:
- Prefer `cause.getLocalizedMessage()`
- Fallback to `cause.getMessage()`
- Final fallback to `cause.toString()` (or a localized generic message)
This ensures the dialog always has meaningful content while still preserving the original cause.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[wanling0000]

LGTM. Tested it locally and works fine. When I broke the remote on purpose, it showed "Git Push Failed" and the commit was still saved. That partial-failure handling is nice

[wanling0000]

nitpick: the enum values are already self-explanatory here

[InAnYan]

Ok, that's nice!

Control-flow using exceptions is not really in the JabRef style, but I think that it is good to have a distinction between errors

[github-actions]

Your pull request conflicts with the target branch.

Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.

[LoayTarek5]

Can noop() happen here? ithink it is only true for UP_TO_DATE, but commitOn() above throws unless a commit was created, so the branch should always be AHEAD,
so if itis unreachable, drop COMMITTED_WITHOUT_PUSH, and if not, the message contradicts the commit that just happened

[LoayTarek5]

push failure shows "Git Push Failed" + the cause, but nothing says the commit was made so it reads as if the whole thing failed, so if the user retries, they then get "Nothing to commit.", which is confusing, right?

[LoayTarek5]

@Campalans03, any updates?

[Campalans03]

@Campalans03, any updates?

Yeah, sorry for not responding! I totally forgot. I’m actually not home until the 19th, so unfortunately I can’t change it right now. I’ll change it as soon as I get home.

[LoayTarek5]

@Campalans03, any updates?

Yeah, sorry for not responding! I totally forgot. I’m actually not home until the 19th, so unfortunately I can’t change it right now. I’ll change it as soon as I get home.

No worries, i hope all is well with you

[calixtus]

Thank you @ThinkerDesigns for your interest in JabRef and trying to craft a summary of the previous discussion. However, I think you missed the point of last week comments: @LoayTarek5 made comments the original contributor agreed to address. Also there are currently merge conflicts with main branch, so I cannot consent to your verdict for approval in the current state. Thus is I think your comment is premature. I also have the suspicion that your comment was automatically generated with a LLM. If this is true, please avoid posting in our repository again, as we already have Qodo as an AI bot integrated in our workflows and a less qualified verdict is confusing.

@Campalans03 : Thank you for your work so far. Looking forward to have this finished and merged.

---

Edit for context: This was the comment i answered to.