Fix force-push detection in Check PR Modifications workflow

[koppor]

Related issues and pull requests

Context: #16074 (comment)

PR Description

The no-force-push job in the Check PR Modifications workflow decided force-push vs. regular push with git cat-file -e <before>, assuming the pre-push commit is unfetchable after a force-push. Because actions/checkout runs with fetch-depth: 0 against GitHub (which retains unreachable objects and serves arbitrary SHAs), the before commit is frequently still present locally, so the check reported "Regular push detected" for genuine force-pushes — the job passed and ghprcomment never posted the force-push notice. This switches detection to an ancestry check (git merge-base --is-ancestor <before> HEAD), which is reliable regardless of whether the orphaned before object was fetched.

Steps to test

This is a CI workflow change. To verify: on a PR, push normally (job stays green, "Regular push detected") and then force-push (job fails with "Force push detected", and ghprcomment posts the force-push notice). Empirically, PR #16074 had 15+ head_ref_force_pushed events on 2026-06-24 where every run wrongly reported "Regular push detected"; the new ancestry check fails in that scenario.

Analogies

Like honey, this fix is a small, sticky one-liner that sweetens an otherwise sour contributor experience. Like chocolate, it melts away a subtle bitterness — the silent passing of force-pushes — that was easy to miss until you tasted it. And like the moon, the orphaned before commit was still up there, visible to git cat-file long after it should have set; checking ancestry instead simply stops mistaking moonlight for daylight.

jabref-contrib-policy:4.2:reviewed​:ok

AI usage

Claude Code (model claude-opus-4-8). Investigation and the one-line workflow fix were AI-assisted; reviewed and owned by the contributor.

AI CHECKLIST.md walkthrough

This change touches only .github/workflows/pr-modifications.yml (a CI workflow). No Java, FXML, Markdown, or build inputs changed, so the Java/build/docs items below are not applicable.

Code rules

• [/] JSpecify annotations used instead of == null checks.
• [/] New classes annotated with @NullMarked (org.jspecify.annotations.NullMarked).
• [/] org.jabref.logic.util.strings.StringUtil.isBlank(java.lang.String) used instead of == null || ...isBlank().
• [/] No catch (Exception e) — only specific exceptions caught.
• No commented-out code left behind.
• [/] User-facing text is localized (Localization.lang in Java, % prefix in FXML).
• [/] New BibEntry objects created with withers (withField, not setField).
• [/] Markdown Javadoc (///) uses Markdown syntax, not JavaDoc inline tags: `code` instead of {@code}, [ClassName] instead of {@link}.
• [/] User-controlled data (request params, entry fields, file contents) is HTML-escaped before being written into any text/html response — including exception/error messages, not just the success body (XSS).
• [/] Tests added or updated for changed behavior in org.jabref.model / org.jabref.logic.

Verification commands

• [/] ./gradlew :jablib:check (or ./gradlew check for all modules) passes.
• [/] ./gradlew checkstyleMain checkstyleTest checkstyleJmh passes.
• [/] ./gradlew modernizer passes.
• [/] ./gradlew --no-configuration-cache :rewriteDryRun reports no changes (run ./gradlew rewriteRun to fix).
• [/] ./gradlew javadoc passes.
• [/] npx markdownlint-cli2 "docs/**/*.md" "*.md" passes.
• [/] docker run -v $(pwd):/github/workspace ghcr.io/leventebajczi/intellij-format:master "*.java" "" ".idea/codeStyles/Project.xml" executed to ensure proper formatting.

Documentation

• [/] CHANGELOG.md entry added if the change is visible to the user (end-user wording, no extra blank lines). Use TODO as the issue/PR reference placeholder when no issue is known and the PR is not yet created — never a fake number.
• Searched jabref/issues and jabref-koppor/issues for a related issue; linked only on a confident match, otherwise kept TODO (no closes/fixes for merely-similar issues).
• [/] Requirement added to docs/requirements/<area>.md if the change is a new feature or significant bug fix (skip for refactors, minor fixes, and internal changes).
• [/] Developer documentation under docs/ updated if behavior or architecture changed.

Pull request

• PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
• All checklist items kept and marked [x], [ ], or [/].
• All HTML comments removed from the PR body.
• PR created with gh pr create --body-file <file> (not --body).
• [/] If CHANGELOG.md used a TODO placeholder, it was replaced with the real PR-number link after PR creation, then committed and pushed.

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) — CI workflow change; not testable in the running app
• [/] I added JUnit tests for changes (if applicable) — not applicable to a CI workflow change
• [/] I added screenshots in the PR description (if change is visible to the user) — not user-visible
• [/] 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 — not user-visible
• [/] 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) — not user-visible
• [/] I checked the user documentation for up to dateness — no user-facing behavior changed

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

PR Summary by Qodo

Support HTTP import into any open library + fix force-push detection in CI workflow
✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Description

• Allow REST import endpoints to target any currently open library, not just current.
• Add optional group assignment for imported entries, creating explicit groups when missing.
• Fix CI force-push detection and improve server error responses and standalone-mode behavior.

Diagram

graph TD
  C[HTTP client] --> R["REST resources\nEntries/Citations"] --> H[UiMessageHandler] --> F["JabRefFrame\nViewModel"] --> D["ImportEntries\nDialog"] --> V["ImportEntries\nViewModel"] --> B[("BibDatabaseContext\n(open library)")] --> G[GroupsHelper]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Pass library id (string) in UiCommand, resolve path in GUI

• ➕ Avoids leaking filesystem paths into cross-module command types
• ➕ Allows targeting path-less libraries (e.g., demo) explicitly if desired
• ➕ Keeps server-side resolution consistent with /libraries ids
• ➖ Requires a GUI-side id→tab/context resolver and shared id scheme knowledge
• ➖ More invasive change across server↔GUI boundary

2. Centralize {id} resolution into a single shared helper for all resources

• ➕ Eliminates duplicated resolveTargetLibrary/resolveTargetContext logic
• ➕ Reduces risk of future endpoints drifting in semantics (404 vs 400 precedence, demo handling)
• ➖ Requires some refactoring churn now; might be better as a follow-up

3. Group assignment via explicit import pipeline hook instead of onFinish callback

• ➕ Group assignment becomes part of the import transaction/flow
• ➕ Less reliance on concurrency/ordering assumptions
• ➖ Likely requires deeper changes in ImportHandler/ImportEntry pipeline
• ➖ Harder to keep behavior consistent across dialog-driven and REST-driven imports

Recommendation: The PR’s approach (server resolves a target open library, GUI switches tab if needed, and group assignment is applied after async import completes using a tracker) is pragmatic and fits the existing architecture. The most valuable follow-up would be to centralize library-id resolution (and 404/400 precedence rules) so EntriesResource and CitationsResource can’t diverge again; passing library ids through UiCommand could be considered later if path-based targeting becomes limiting.

Files changed (24) +809 / -175

Enhancement (9) +263 / -97

ArgumentProcessor.javaSwitch CLI append commands to library-aware UiCommands +2/-2

Switch CLI append commands to library-aware UiCommands

• Updates CLI argument processing to emit 'AppendFilesToLibrary' and 'AppendBibTeXToLibrary' instead of current-library-only variants, aligning CLI/GUI command handling with library-targeting support.

jabgui/src/main/java/org/jabref/cli/ArgumentProcessor.java

JabRefFrameViewModel.javaSupport appending/importing into a selected or newly opened library tab +51/-34

Support appending/importing into a selected or newly opened library tab

• Adds tab selection/opening based on an optional target library path carried in UiCommands. Threads group assignment through the import dialog and routes group creation/assignment through the new 'GroupsHelper' for direct imports.

jabgui/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java

ImportEntriesDialog.javaPreselect and fallback to a REST-requested target group on import +48/-7

Preselect and fallback to a REST-requested target group on import

• Adds an optional 'targetGroup' to the dialog, preselecting an existing matching group and using it as fallback when the user does not explicitly choose a group. Ensures explicit group selection overrides REST-supplied group intent.

jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java

ImportEntriesViewModel.javaAssign imported entries to a group after async import completes +18/-1

Assign imported entries to a group after async import completes

• Extends 'importEntries' with an optional target group and uses 'EntryImportHandlerTracker' to assign the group to the actual inserted/merged entry copies once import processing finishes.

jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java

UiCommand.javaAdd library-targeted append commands and nullable group convenience APIs +23/-8

Add library-targeted append commands and nullable group convenience APIs

• Replaces current-library-only append commands with 'AppendFilesToLibrary' and 'AppendBibTeXToLibrary', carrying an Optional<Path> target library and normalizing nullable/blank group inputs into Optional values.

jablib/src/main/java/org/jabref/logic/UiCommand.java

UiMessageHandler.javaIntroduce UiMessageHandler.NONE and GUI-connected detection +20/-0

Introduce UiMessageHandler.NONE and GUI-connected detection

• Adds a null-object handler for standalone server mode plus 'isGuiConnected()' for reliable GUI-only endpoint gating. Ensures missing GUI wiring fails as a clear error rather than DI construction failures.

jablib/src/main/java/org/jabref/logic/UiMessageHandler.java

GroupsHelper.javaAdd helper to create/reuse explicit groups and assign entries +30/-0

Add helper to create/reuse explicit groups and assign entries

• Introduces 'GroupsHelper.assignEntriesToGroup' to ensure group-tree root exists, create/reuse a top-level explicit group, and assign imported entries using the configured keyword separator.

jablib/src/main/java/org/jabref/logic/groups/GroupsHelper.java

CitationsResource.javaResolve target library by id and append cached citations to that library +40/-19

Resolve target library by id and append cached citations to that library

• Removes 'current'-only restriction by resolving a target open library context and append target. Ensures add-from-cache rejects non-open/path-less libraries (e.g., demo) with 404, and forwards library-targeted UiCommands with optional group assignment.

jabsrv/src/main/java/org/jabref/http/server/resources/CitationsResource.java

EntriesResource.javaAllow POST /libraries/{id}/entries to target any open library and optional group +31/-26

Allow POST /libraries/{id}/entries to target any open library and optional group

• Adds library-id resolution ('current' vs specific open library id), routes imports through library-targeted UiCommands, and consistently rejects standalone mode via 'isGuiConnected()'. Improves blank-body validation and refactors temp file creation.

jabsrv/src/main/java/org/jabref/http/server/resources/EntriesResource.java

Bug fix (5) +58 / -32

pr-modifications.ymlDetect force-pushes via commit ancestry instead of object existence +5/-1

Detect force-pushes via commit ancestry instead of object existence

• Replaces 'git cat-file -e <before>' with 'git merge-base --is-ancestor <before> HEAD' to reliably distinguish regular pushes from force-pushes even when the old commit object is still present after checkout.

.github/workflows/pr-modifications.yml

EntryImportHandlerTracker.javaMake import tracking thread-safe and expose imported-entry snapshot +32/-18

Make import tracking thread-safe and expose imported-entry snapshot

• Replaces AtomicInteger counters with synchronized state to ensure 'onFinish' runs exactly once under concurrent imports. Adds immutable snapshots for selected/imported entries, enabling safe post-import operations (e.g., group assignment).

jabgui/src/main/java/org/jabref/gui/externalfiles/EntryImportHandlerTracker.java

GlobalExceptionMapper.javaPreserve WebApplicationException entities and surface messages as text/plain +15/-1

Preserve WebApplicationException entities and surface messages as text/plain

• Keeps responses that already have an entity (e.g., HTML-escaped error bodies) unchanged. For bodyless WebApplicationExceptions, returns the exception message as a text/plain entity to improve client diagnostics.

jabsrv/src/main/java/org/jabref/http/dto/GlobalExceptionMapper.java

Server.javaBind UiMessageHandler.NONE in standalone mode and require handler in GUI mode +5/-11

Bind UiMessageHandler.NONE in standalone mode and require handler in GUI mode

• Ensures standalone server mode binds the null-object UiMessageHandler so resource construction succeeds and GUI-only endpoints can respond with clean 400 errors. Simplifies GUI entry point to always bind a non-null handler.

jabsrv/src/main/java/org/jabref/http/server/Server.java

Command.javaGate command execution on GUI connectivity +1/-1

Gate command execution on GUI connectivity

• Extends the guard to reject requests when UiMessageHandler is missing or is the standalone null object, preventing GUI-only commands from being executed in CLI/standalone mode.

jabsrv/src/main/java/org/jabref/http/server/command/Command.java

Refactor (1) +2 / -3

HttpServerThread.javaMake UiMessageHandler required for GUI server thread +2/-3

Make UiMessageHandler required for GUI server thread

• Removes nullable semantics and clarifies the thread wrapper is GUI-only, ensuring a concrete UiMessageHandler is always supplied for GUI-mode server startup.

jabsrv/src/main/java/org/jabref/http/manager/HttpServerThread.java

Tests (4) +328 / -0

ImportHandlerTest.javaTest that group assignment targets inserted entry copies, not originals +43/-0

Test that group assignment targets inserted entry copies, not originals

• Adds a regression-style test using a real database context to verify the import tracker exposes the inserted entries (copies) and that group assignment updates only the database entries.

jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java

GroupsHelperTest.javaAdd tests for GroupsHelper group creation and reuse +67/-0

Add tests for GroupsHelper group creation and reuse

• Covers creating a top-level explicit group on first assignment and reusing an existing group on subsequent assignments without duplicating group nodes.

jablib/src/test/java/org/jabref/logic/groups/GroupsHelperTest.java

CitationsResourceTest.javaAdd tests for add-from-cache targeting and 404/410 behavior +81/-0

Add tests for add-from-cache targeting and 404/410 behavior

• Verifies that non-open/path-less ids (demo) and unknown ids return 404, while a valid open library id proceeds to cache lookup and returns 410 when the key is expired/missing.

jabsrv/src/test/java/org/jabref/http/server/CitationsResourceTest.java

EntriesResourceTest.javaAdd tests for targeted import, unknown ids, and standalone-mode precedence +137/-0

Add tests for targeted import, unknown ids, and standalone-mode precedence

• Covers successful import command emission with correct Optional<Path> semantics, 404 for unknown ids without emitting commands, 400 for empty BibTeX, and 400 precedence in standalone mode even when the library id is unknown.

jabsrv/src/test/java/org/jabref/http/server/EntriesResourceTest.java

Documentation (4) +157 / -43

CHANGELOG.mdDocument expanded HTTP server import capabilities +2/-1

Document expanded HTTP server import capabilities

• Adds entries describing new HTTP server behavior: querying libraries and importing into any open library, including group-related improvements. Also reorders/clarifies the existing HTTP server entry.

CHANGELOG.md

http-client.env.jsonAdd IntelliJ HTTP client environment variables +6/-0

Add IntelliJ HTTP client environment variables

• Provides a localhost environment configuration with host and default library id for manual REST testing via IntelliJ's HTTP client.

jabsrv/src/test/http-client.env.json

import.httpAdd REST import examples including targeted-library and group flows +146/-0

Add REST import examples including targeted-library and group flows

• Adds an IntelliJ HTTP client script demonstrating importing BibTeX/plain citations/RIS into either the current or a specific open library, plus error cases and the citations lookup/add-from-cache flow.

jabsrv/src/test/import.http

rest-api.httpTidy REST API test script and remove duplicated creation section +3/-42

Tidy REST API test script and remove duplicated creation section

• Normalizes headings/sections and removes older creation examples now superseded by the new import.http script.

jabsrv/src/test/rest-api.http

Other (1) +1 / -0

build.gradle.ktsEnable JUnit params module for server tests +1/-0

Enable JUnit params module for server tests

• Adds 'org.junit.jupiter.params' to test module requirements so parameterized tests compile and run in the server module.

jabsrv/build.gradle.kts

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

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (4) 📜 Skill insights (0)

1. Javadoc tags in /// 📘 Rule violation ⚙ Maintainability

Description

ImportEntriesDialog introduces {@code ...} and {@link ...} inside /// Markdown doc comments,
which violates the required Markdown-doc style. This reduces consistency and can break the project’s
documentation conventions.

Code

jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[R118-125]

+    /// Variant that pre-selects {@code targetGroup} in the group picker. The group is created and
+    /// assigned by the caller only after the dialog is confirmed (see {@link #getImportedEntries()}
+    /// / {@link #getImportTarget()}).
+    ///
+    /// @param database    the database to import into
+    /// @param task        the task executed for parsing the selected files(s).
+    /// @param targetGroup name of the group to pre-select, or {@code null} for none
+    public ImportEntriesDialog(BibDatabaseContext database, BackgroundTask<ParserResult> task, @Nullable String targetGroup) {

Evidence

PR Compliance ID 17 requires Markdown syntax in /// doc comments. The added comment block uses
{@code targetGroup} and {@link #getImportedEntries()}/{@link #getImportTarget()} instead of
Markdown equivalents.

AGENTS.md: Use Markdown Javadoc Comments (///) for Multi-line Comments and Use Markdown Syntax
jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[118-125]

Agent prompt

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

## Issue description
`ImportEntriesDialog` uses classic Javadoc inline tags (`{@code ...}`, `{@link ...}`) inside `///` Markdown doc comments.

## Issue Context
This repo requires Markdown-style documentation for `///` comments (backticks for code, `[#method()]`/`[ClassName]` for links).

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[118-125]

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

---

2. orElse(null) passed to method 📘 Rule violation ≡ Correctness

Description

The PR introduces new call sites that pass null as an argument (e.g.,
targetGroup().orElse(null)), creating implicit null contracts and increasing NPE risk. This
conflicts with the guideline to avoid passing null at call sites.

Code

jabgui/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java[R205-212]

+                          selectLibraryTab(importBibTex.library());
+                          // selectLibraryTab may have just opened the target library, which loads in
+                          // the background; wait for it so the import targets the loaded tab.
+                          waitForLoadingFinished(() -> importBibtexStringAndOpen(
+                                  importBibTex.bibtex(),
+                                  // importBibtexStringAndOpen accepts null targetGroup to indicate "no group assignment"
+                                  importBibTex.targetGroup().orElse(null)));
+                      });

Evidence

PR Compliance ID 12 forbids introducing call sites that pass null as an argument. The updated UI
command handling passes null via importBibTex.targetGroup().orElse(null), and related code also
forwards null (e.g., addParserResult(parserResult, null) and importEntries(..., null)).

AGENTS.md: Do Not Pass null as an Argument
jabgui/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java[202-212]
jabgui/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java[444-447]
jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java[190-195]

Agent prompt

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

## Issue description
New/modified code passes `null` as an argument (including via `Optional.orElse(null)`), which violates the project rule to avoid `null` arguments.

## Issue Context
`targetGroup` is currently modeled as `@Nullable String` in several APIs. To comply, prefer modeling absence as `Optional<String>` (or provide overloads) so callers never pass `null`.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java[202-212]
- jabgui/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java[444-451]
- jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java[187-224]

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

---

3. UiMessageHandler.NONE throws unchecked 📘 Rule violation ☼ Reliability

Description

UiMessageHandler.NONE throws UnsupportedOperationException, introducing an unchecked exception
path that can tear down request handling if reached. This violates the rule against adding unchecked
exceptions for error handling paths.

Code

jablib/src/main/java/org/jabref/logic/UiMessageHandler.java[R19-21]

+    UiMessageHandler NONE = uiCommands -> {
+        throw new UnsupportedOperationException("No GUI is connected to the JabRef HTTP server");
+    };

Evidence

PR Compliance ID 15 disallows introducing unchecked exceptions for error handling. The new
null-object UiMessageHandler.NONE throws UnsupportedOperationException when invoked.

AGENTS.md: Do Not Throw Unchecked Exceptions (RuntimeException/IllegalStateException)
jablib/src/main/java/org/jabref/logic/UiMessageHandler.java[9-21]

Agent prompt

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

## Issue description
The new `UiMessageHandler.NONE` implementation throws an unchecked exception (`UnsupportedOperationException`).

## Issue Context
The compliance rule forbids introducing unchecked exceptions for error handling; in a server context, this can cause request failures and instability.

## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/UiMessageHandler.java[9-21]

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

---

View more (1)

4. Empty selected-groups crash 🐞 Bug ≡ Correctness

Description

ImportEntriesDialog calls stateManager.getSelectedGroups(selectedDatabaseContext).getFirst() when an
ExplicitGroup is selected, but selected-groups for a non-active library can be empty, causing a
NoSuchElementException and aborting the import.

Code

jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[R331-335]

+                    BibDatabaseContext selectedDatabaseContext = libraryListView.getSelectionModel().getSelectedItem();
+                    GroupTreeNode prevSelectedGroup = stateManager.getSelectedGroups(selectedDatabaseContext).getFirst();
+                    stateManager.setSelectedGroups(selectedDatabaseContext, List.of(selectedGroup));
+                    viewModel.importEntries(selectedEntries, downloadLinkedOnlineFiles.isSelected());
+                    stateManager.setSelectedGroups(selectedDatabaseContext, List.of(prevSelectedGroup));

Evidence

The dialog unconditionally uses getFirst() to capture the previous group selection for the
selected library, but the state manager returns an empty list by default for contexts that were
never active; only the active database is ensured to have a default selected group. Therefore
selecting an explicit group in a non-active library can throw at confirmation time.

jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[321-340]
jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[264-283]
jabgui/src/main/java/org/jabref/gui/JabRefGuiStateManager.java[139-147]
jabgui/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java[165-182]

Agent prompt

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

## Issue description
`ImportEntriesDialog` restores the previous group selection using `getFirst()` on `stateManager.getSelectedGroups(selectedDatabaseContext)`. For non-active libraries this list can be empty, causing a `NoSuchElementException` when the user selects an explicit group and confirms the import.

## Issue Context
`StateManager#getSelectedGroups` returns an empty list by default for a given database context; only the *active* database gets a default selection initialized by `GroupTreeViewModel`.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[321-340]
- jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java[264-298]

### Implementation guidance
- Replace the single `prevSelectedGroup = ...getFirst()` with a safe snapshot, e.g. `List<GroupTreeNode> prev = List.copyOf(stateManager.getSelectedGroups(ctx));`.
- Restore with `stateManager.setSelectedGroups(ctx, prev);` (works even if `prev` is empty), instead of assuming a single element.
- Consider selecting a default group in `updateGroupList()` when groups exist but no selection is present, to avoid `selectedGroup == null` surprises.

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

---

5. Public importEntries has boolean 📘 Rule violation ⚙ Maintainability

Description

A new public overload importEntries(..., boolean shouldDownloadFiles, ...) adds/extends a
boolean-flag API, which makes call sites ambiguous and harder to read. The rule recommends replacing
boolean flags with clearer method names or distinct entry points.

Code

jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java[R190-196]

    public void importEntries(List<BibEntry> entriesToImport, boolean shouldDownloadFiles) {
+        importEntries(entriesToImport, shouldDownloadFiles, null);
+    }
+
+    /// @param targetGroup name of a group the imported entries are additionally assigned to. If it is non-blank and no group with that name exists yet, it is created as a top-level explicit group. A blank/null value assigns no group.
+    public void importEntries(List<BibEntry> entriesToImport, boolean shouldDownloadFiles, @Nullable String targetGroup) {
        // Remember the selection in the dialog

Evidence

PR Compliance ID 9 prohibits introducing/expanding public methods that take boolean flags to change
behavior. The PR adds a new public overload of importEntries that includes the boolean parameter
shouldDownloadFiles.

AGENTS.md: Avoid Boolean Parameters in Public Methods
jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java[190-196]

Agent prompt

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

## Issue description
A new public method overload introduces a boolean flag parameter (`shouldDownloadFiles`) in a public API.

## Issue Context
Compliance requires avoiding boolean parameters in public methods to keep call sites intention-revealing.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java[187-196]

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

---

6. Null UiMessageHandler startup crash 🐞 Bug ☼ Reliability

Description

HttpServerManager.start(preferences, srvStateManager, uri) still delegates to start(..., null, uri),
but HttpServerThread/Server.run now assume a non-null UiMessageHandler and unconditionally bind it,
so calling the 3-arg overload (or otherwise passing null) can crash starting the HTTP server.

Code

jabsrv/src/main/java/org/jabref/http/server/Server.java[R109-113]

+    public HttpServer run(SrvStateManager srvStateManager, UiMessageHandler uiMessageHandler, URI uri) {
        ServiceLocator serviceLocator = ServiceLocatorUtilities.createAndPopulateServiceLocator();
        ServiceLocatorUtilities.addOneConstant(serviceLocator, srvStateManager, "statemanager", SrvStateManager.class);
-        if (uiMessageHandler != null) {
-            ServiceLocatorUtilities.addOneConstant(serviceLocator, uiMessageHandler, "uimessagehandler", UiMessageHandler.class);
-        }
-
+        ServiceLocatorUtilities.addOneConstant(serviceLocator, uiMessageHandler, "uimessagehandler", UiMessageHandler.class);
        return startServer(serviceLocator, uri);

Evidence

The manager still passes null via its overload, while the thread and server entry point now treat
the handler as required and bind it without a null guard. This creates a concrete null-to-nonnull
mismatch that can crash server startup when the null-passing overload is used.

jabsrv/src/main/java/org/jabref/http/manager/HttpServerManager.java[25-37]
jabsrv/src/main/java/org/jabref/http/manager/HttpServerThread.java[28-45]
jabsrv/src/main/java/org/jabref/http/server/Server.java[108-114]
jablib/src/main/java/org/jabref/logic/UiMessageHandler.java[9-21]

Agent prompt

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

## Issue description
`UiMessageHandler` was made mandatory for the GUI server entry point (`Server.run(..., UiMessageHandler, ...)`) and `HttpServerThread` now stores it as non-null. However, `HttpServerManager` still has public overloads that pass `null`, which can lead to NPEs / startup failures when the HTTP server is started without a GUI handler.

## Issue Context
The PR introduces `UiMessageHandler.NONE` as a supported null object for standalone/no-GUI mode.

## Fix Focus Areas
- jabsrv/src/main/java/org/jabref/http/manager/HttpServerManager.java[25-37]
- jabsrv/src/main/java/org/jabref/http/manager/HttpServerThread.java[28-45]
- jabsrv/src/main/java/org/jabref/http/server/Server.java[108-114]
- jablib/src/main/java/org/jabref/logic/UiMessageHandler.java[9-29]

### Implementation guidance
- In `HttpServerManager.start(..., @Nullable UiMessageHandler uiMessageHandler, ...)`, map null to `UiMessageHandler.NONE` before constructing `HttpServerThread`.
- Update the 3-arg `start(preferences, srvStateManager, uri)` overload to pass `UiMessageHandler.NONE` (or remove that overload if it is no longer valid).
- Optionally add `Objects.requireNonNull(uiMessageHandler)` at the `HttpServerThread`/`Server.run` boundary to fail fast with a clear message if null still leaks through.

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

---