Skip to content

.pr_agent_accepted_suggestions

Jump to bottom
qodo-merge-bot edited this page Jun 4, 2026 · 224 revisions
                     PR 15896 (2026-06-03)                    
[reliability] `inputs.gradleCommand` undefined in action
`inputs.gradleCommand` undefined in action The composite GitHub Action `.github/actions/package` no longer defines the `gradleCommand` input but still references `inputs.gradleCommand` in the Smoke test (and ShadowJar) steps, which can result in an empty/invalid command and deterministic workflow failures. This undermines consistent, configured Gradle invocation in CI.

Issue description

The composite action .github/actions/package/action.yml removed the gradleCommand input, but some steps still reference ${{ inputs.gradleCommand }}, which can evaluate to an empty string and produce an invalid Gradle command, breaking CI (including the binaries workflow).

Issue Context

The gradleCommand input was removed to standardize on a single Gradle invocation, and .github/workflows/binaries.yml no longer passes gradleCommand into the action. The action already hardcodes a Gradle invocation for the jpackage step, so the remaining ${{ inputs.gradleCommand }} usages should be updated to match that standardized approach (or the input should be reintroduced if standardization is not desired).

Fix Focus Areas

  • .github/actions/package/action.yml[4-40]
  • .github/actions/package/action.yml[71-82]
  • .github/actions/package/action.yml[129-139]
                     PR 15895 (2026-06-03)                    
[correctness] Staged/untracked changes ignored
Staged/untracked changes ignored The new check-formatting composite action is documented as failing on “uncommitted changes”, but the underlying script treats an empty `git diff` (unstaged only) as a clean tree. This means staged changes or untracked files can exist without failing CI, undermining the intent of the formatting/OpenRewrite gate.

Issue description

check-formatting relies on .github/format-diff-annotations.sh, which currently uses plain git diff and exits 0 when that output is empty. This only detects unstaged modifications, so staged changes and untracked files can still be present while CI reports “clean”.

Issue Context

The action’s description and usage implies it gates on “uncommitted changes”, so the check should cover at least:

  • unstaged changes
  • staged changes
  • (optionally) untracked files

Fix Focus Areas

  • .github/format-diff-annotations.sh[21-26]
  • .github/actions/check-formatting/action.yml[34-43]
  • .github/actions/check-formatting/action.yml[1-5]
[maintainability] Misleading annotation comment
Misleading annotation comment The script header comment still says the message appends only `lines -`, but flush() now emits `line N` for single-line hunks. This mismatch can confuse future maintenance/debugging of the annotation output format.

Issue description

The header comment describing the appended range format is now inaccurate after changing the output to use line N for single-line hunks.

Issue Context

Keeping this comment correct matters because it documents the externally-visible annotation message format that developers see in CI.

Fix Focus Areas

  • .github/format-diff-annotations.sh[9-14]
  • .github/format-diff-annotations.sh[64-75]
                     PR 15880 (2026-06-02)                    
[correctness] Wrong file on deletions
Wrong file on deletions The script only updates `file` on `+++ b/...`, so diffs that use `+++ /dev/null` (file deletions) will keep the previous file value and emit annotations against the wrong path. This can make CI annotations misleading/incorrect, especially in the OpenRewrite job that also uses this script.

Issue description

.github/format-diff-annotations.sh sets file only when matching +++ b/.... For deletion diffs (+++ /dev/null), file is never updated, so subsequent ::error file=... annotations can be emitted for the previous file.

Issue Context

This script is used in CI to annotate formatting and OpenRewrite diffs. OpenRewrite can produce diffs including file deletions, so +++ /dev/null is a realistic case to support.

Fix Focus Areas

  • Update parsing to reliably set file for every file-level diff, including deletions (e.g., parse diff --git a/... b/... and/or handle +++ /dev/null).
  • file/path focus:
  • .github/format-diff-annotations.sh[61-64]
[correctness] Add-only hunks mis-anchored
Add-only hunks mis-anchored The new hunk walker tracks only old-side line numbers and no longer parses `+newStart,+newCount`, so hunks with `oldCount=0` (pure insertions / new-file-style hunks) collapse to a single old-side line (or line 1 when oldStart=0). This breaks the “annotate only changed lines” behavior for insert-only changes by pointing at an incorrect range.

Issue description

The script only parses the old-side start ($2) and maintains only an oldLine counter. For addition-only hunks (@@ -X,0 +Y,N @@), annotations cannot be anchored to the actual changed new-side lines and instead collapse to a single old-side line number.

Issue Context

CI git diff output can include hunks where oldCount=0 (pure insertions). In those cases, the annotation should use the new-side range (newStart..newStart+N-1) or otherwise provide a meaningful range that corresponds to committed code developers can edit.

Fix Focus Areas

  • Parse both sides from the hunk header (-oldStart,oldCount and +newStart,newCount).
  • Maintain both oldLine and newLine counters while scanning hunk body.
  • When a run contains no old-side consumption (or oldCount==0), anchor the annotation on the new-side range.
  • file/path focus:
  • .github/format-diff-annotations.sh[65-82]
[correctness] Quoted paths not parsed
Quoted paths not parsed The awk parser only matches unquoted `--- a/...` and `+++ b/...` headers, so when `git diff` emits quoted headers (e.g., `--- "a/path with spaces"`) the script may keep a stale `file` value (or empty) and emit `::error` annotations against the wrong path. This makes CI annotations misleading/unactionable for changed files whose names require quoting (spaces/special characters).

Issue description

.github/format-diff-annotations.sh extracts the file path only from unquoted diff headers (--- a/... and +++ b/...). When git diff quotes paths (commonly for filenames containing spaces), those regexes don’t match, so fileRaw/file won’t be updated and subsequent annotations can point at the previous file or have an empty file=. Concrete example of headers that won’t match today:

  • --- "a/config/Eclipse Code Style.epf"
  • +++ "b/config/Eclipse Code Style.epf"

Issue Context

This repository contains tracked files with spaces in their path, so this is not purely theoretical.

Fix Focus Areas

  • .github/format-diff-annotations.sh[64-71]

What to change

  1. Add additional header matchers for quoted paths, e.g.:
  • ^--- "a/ … strip leading --- "a/ and trailing "
  • ^\+\+\+ "b/ … strip leading +++ "b/ and trailing " (Optionally handle backslash-escaped quotes within the name if you want to be extra robust.)
  1. Consider resetting fileRaw/file on ^diff --git to avoid accidentally reusing the previous file if a header isn’t recognized.
  2. Optionally, make flush() a no-op if file is empty to avoid emitting invalid annotations.
                     PR 15873 (2026-06-01)                    
[security] Whitespace collapses PR number
Whitespace collapses PR number The `Read pr_number.txt` step deletes *all* whitespace from the artifact (`tr -d '[:space:]'`), so malformed content like `12 3` becomes `123` and passes the numeric regex check. This can cause the workflow to target/comment on the wrong PR while still treating the artifact as valid.

Issue description

The workflow currently uses tr -d '[:space:]' to remove all whitespace from pr_number.txt before validating it. This can change the PR number by concatenating digits across spaces/tabs/newlines, allowing malformed content to become a different numeric value and still pass validation.

Issue Context

The artifact is produced as a single-line PR number (with a trailing newline from echo). The validation should therefore accept only a single line containing digits (optionally ending with \r\n), and reject any additional characters/lines/whitespace rather than deleting them.

Fix Focus Areas

  • .github/workflows/pr-comment.yml[64-80]

Suggested implementation direction (bash)

  • Read exactly the first line (IFS= read -r PR_NUMBER < "$ARTIFACT_FILE").
  • Strip only a trailing carriage return (PR_NUMBER="${PR_NUMBER%$'\r'}").
  • Reject if the file has more than one line (e.g., tail -n +2 "$ARTIFACT_FILE" | grep -q . → invalid).
  • Validate with a strict digits-only check (e.g., [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]).
  • Do not remove internal whitespace (no tr -d '[:space:]').
                     PR 15871 (2026-06-01)                    
[correctness] Unescaped annotation properties
Unescaped annotation properties The script emits `::error file=...,line=...,endLine=...` using raw `file` values from `git diff`, so paths containing characters that must be escaped for GitHub Actions workflow-command properties (e.g., `:`, `,`, `%`, newline) can produce malformed annotations or wrong parsing. The repo already contains a dedicated escape utility for exactly this annotation format, so this script is inconsistent with established behavior and can silently drop annotations in edge cases.

Issue description

.github/format-diff-annotations.sh prints GitHub Actions workflow commands (::error ...) but does not escape the file property (and generally any property/data field). GitHub Actions requires percent-encoding for special characters in both the message data and in key=value properties; otherwise annotations can be parsed incorrectly or ignored. The codebase already has an established escaping implementation (GitHubActionsEscape.property/data). The bash script should apply the same escaping rules when printing file=... (and any other dynamic property/data fields).

Issue Context

Existing Java-based annotation writers escape both properties and data explicitly, including : and , for properties.

Fix Focus Areas

  • .github/format-diff-annotations.sh[19-30]
  • jablib/src/main/java/org/jabref/logic/util/GitHubActionsEscape.java[15-25]

Suggested approach

  • Implement an AWK function equivalent to GitHubActionsEscape.property (escape %, \r, \n, then additionally escape : and ,).
  • Apply it to file before printing.
  • (Optional) Also escape the message data portion if it ever becomes dynamic in the future.
[correctness] Wrong range for pure additions
Wrong range for pure additions For pure-addition hunks (`oldCount==0`), the script still uses the *old* hunk range (`-oldStart,oldCount`) and can emit `line=0,endLine=0` (e.g., new files or insertions at the start), producing a meaningless “lines 0-0” message and an annotation that cannot reliably attach to source. For `oldCount==0`, the annotation should use the `+newStart,newCount` range (and clamp to 1-based lines).

Issue description

When a diff hunk has oldCount==0 (pure addition / new file), the script currently anchors the annotation using oldStart, which can be 0 and does not represent real lines in the committed file. This can yield line=0,endLine=0 and a confusing message.

Issue Context

The hunk header contains both old and new ranges: @@ -oldStart,oldCount +newStart,newCount @@. If oldCount==0, there are no old lines to annotate, so the correct anchor is the new range.

Fix Focus Areas

  • .github/format-diff-annotations.sh[21-29]

Suggested approach

  • Parse $3 (+newStart,newCount) as well as $2.
  • If oldCount > 0, keep current behavior (use old range).
  • Else (pure addition), set start=newStart and end=newStart+newCount-1.
  • Ensure start >= 1 and end >= start before emitting the annotation.
[observability] Misleading OpenRewrite annotations
Misleading OpenRewrite annotations The annotation title/text is hardcoded to “Formatting / Run the formatter”, but the workflow also uses this script to fail the OpenRewrite job when it leaves a dirty working tree. When OpenRewrite causes semantic changes or adds/removes files, the annotations will incorrectly tell contributors to “run the formatter” instead of indicating OpenRewrite output needs to be committed/fixed.

Issue description

The script always emits title=Formatting and a message telling users to run the formatter, but it is also invoked from the openrewrite job where the root cause may be OpenRewrite changes rather than formatting.

Issue Context

In .github/workflows/tests-code.yml, the OpenRewrite job runs :rewriteRun, then runs the formatter, then calls .github/format-diff-annotations.sh. Any remaining diff at that point is not necessarily a formatting issue.

Fix Focus Areas

  • .github/workflows/tests-code.yml[108-129]
  • .github/format-diff-annotations.sh[1-33]

Suggested approach

  • Add optional parameters or env vars to .github/format-diff-annotations.sh (e.g., ANNOTATION_TITLE / ANNOTATION_MESSAGE_PREFIX, or a first arg like format|openrewrite).
  • In the OpenRewrite job, set the title/message to something like OpenRewrite / Uncommitted changes after OpenRewrite; run :rewriteRun and commit results.
  • Keep the current “Formatting” messaging for the format jobs.
                     PR 15870 (2026-06-01)                    
[correctness] Immutable exporters list
Immutable exporters list ExportPreferences#getDefault passes List.of() into FXCollections.observableList, producing an ObservableList backed by an immutable list. Later calls to ExportPreferences#setCustomExporters/setAll will attempt to mutate that list (setAll), causing UnsupportedOperationException and breaking the Custom Exporters preferences flow (and other preference reset/import flows when no custom exporters are stored).

Issue description

ExportPreferences stores customExporters using FXCollections.observableList(customExporters). The default constructor passes List.of(), so the resulting ObservableList is backed by an immutable list. Any later mutation via setAll(...) / setCustomExporters(...) can throw UnsupportedOperationException.

Issue Context

This manifests in GUI when saving the “Custom Exporters” preferences tab because CustomExporterTabViewModel#storeSettings calls preferences.getExportPreferences().setCustomExporters(...), which calls customExporters.setAll(...). It also affects normal startup/import/reset flows when no custom exporters are stored, because the backing-store loader returns defaults for an empty series.

Fix

Ensure customExporters is always backed by a mutable list, e.g.:

  • In ExportPreferences constructor, use FXCollections.observableArrayList(customExporters) or FXCollections.observableList(new ArrayList<>(customExporters)).
  • Keep setCustomExporters/setAll as-is afterward.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/exporter/ExportPreferences.java[33-50]
  • jablib/src/main/java/org/jabref/logic/exporter/ExportPreferences.java[56-61]
  • jablib/src/main/java/org/jabref/logic/exporter/ExportPreferences.java[99-105]
  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[1682-1688]
  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[1771-1779]
[maintainability] `get(0)` replaces `getFirst()`
`get(0)` replaces `getFirst()` The updated export save-order persistence uses `List.get(0)` instead of the modern `SequencedCollection#getFirst()` idiom, regressing from a more modern Java style. This diverges from the project’s stated preference for modern Java 25+ APIs/idioms.

Issue description

Modern Java provides SequencedCollection#getFirst() for readability and consistency; the PR changed first-element access to get(0).

Issue Context

This code previously used getFirst() and the compliance checklist explicitly prefers modern Java 25+ idioms.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[1694-1699]
  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[1727-1731]
[maintainability] `keyWordDelimiterProperty` Javadoc wrong
`keyWordDelimiterProperty` Javadoc wrong The updated parameter comment references `BibEntryProperties#keyWordDelimiterProperty`, which does not exist (and no longer matches the renamed `keywordSeparator` concept). This creates confusing and misleading documentation for maintainers.

Issue description

A Javadoc/parameter comment references a non-existent or outdated API name, making the documentation misleading.

Issue Context

The code now uses keywordSeparator, but the Javadoc still points to BibEntryProperties#keyWordDelimiterProperty.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyPatternPreferences.java[46-46]
                     PR 15867 (2026-05-31)                    
[reliability] Date parsing can crash
Date parsing can crash ZoteroCitationMarkParser#setDate and helpers call List.getFirst() and Integer.parseInt() on unvalidated CSL "date-parts" contents, so empty inner arrays or non-numeric parts can throw NoSuchElementException/NumberFormatException. These exceptions are not caught by parse() (it only catches JsonParseException), so parsing can fail hard and bubble up to callers.

Issue description

ZoteroCitationMarkParser#setDate assumes issued.date-parts is well-formed and numeric. It uses getFirst() and Integer.parseInt() without validating inner list contents, which can throw runtime exceptions that are not caught by parse().

Issue Context

This parser is intended to consume Zotero-provided CSL JSON embedded in a reference mark name. Real-world data can be incomplete or partially missing (e.g., "date-parts":[[]]) or contain unexpected tokens; the parser should degrade gracefully (e.g., omit date fields) rather than throw.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/openoffice/ZoteroCitationMarkParser.java[39-50]
  • jablib/src/main/java/org/jabref/logic/openoffice/ZoteroCitationMarkParser.java[107-164]

What to change

  • In setDate(...):
  • Guard against issuedData == null.
  • After issuedData.dateParts non-empty, also validate the first inner list is non-null and non-empty before calling getFirst().
  • Replace direct Integer.parseInt(...) calls with safe parsing (e.g., try/catch NumberFormatException and return/skip setting month/day), so malformed values don’t crash parsing.
  • Consider broadening the parse(...) catch to include RuntimeException (or at least NoSuchElementException/NumberFormatException) and return List.of() after logging at debug level.
  • Add tests for edge cases such as "date-parts":[[]] and non-numeric date parts to prevent regressions.
                     PR 15863 (2026-05-30)                    
[correctness] `See what's new` typo
`See what's new` typo The new Russian translation contains a spelling error (`Узнайть`) which will be shown to users in the UI. This violates the UI text style guideline to keep spelling consistent and professional.

Issue description

The Russian UI translation for See what's new contains a spelling typo (Узнайть).

Issue Context

This string is user-facing and should follow UI text style guidelines (correct spelling).

Fix Focus Areas

  • jablib/src/main/resources/l10n/JabRef_ru.properties[1665-1665]
[correctness] `Move URL...` missing quote
`Move URL...` missing quote The new Russian translation has unbalanced quotes around the field name `note` for the string `Move URL in 'note' field to 'URL' field`, which makes the Cleanup checkbox label render with malformed quoting and look unpolished to users. This violates the UI text style guideline (PR Compliance ID 20) requiring consistent, correct punctuation in user-facing UI text.

Issue description

The Russian translation for Move URL in 'note' field to 'URL' field is missing the closing ' after note, causing unbalanced quotes and a malformed label shown to users.

Issue Context

This is user-facing UI text and must have correct punctuation/quoting to comply with PR Compliance ID 20 (consistent, correct UI text). The key is used as a UI label in the Cleanup entries panel (FXML), and JabRef’s localization performs %0/%1 placeholder substitution but does not interpret quotes specially, so this is purely a visible string defect.

Fix Focus Areas

  • jablib/src/main/resources/l10n/JabRef_ru.properties[1214-1214]
                     PR 15862 (2026-05-30)                    
[correctness] getDefaultPath override blocked
getDefaultPath override blocked JabRefCliPreferences#getDefaultPath is now private, so JabRefGuiPreferences#getDefaultPath annotated with @Override no longer overrides and the project should not compile. Even if the annotation is removed, GUI-mode defaults (e.g., LAST_USED_DIRECTORY and FilePreferences defaults) will fall back to "/" instead of NativeDesktop’s default directory.

Issue description

JabRefCliPreferences#getDefaultPath() was changed from an overridable method to private, which prevents JabRefGuiPreferences from overriding it (and makes its existing @Override invalid). This breaks GUI-mode default directory behavior (and likely compilation).

Issue Context

JabRefGuiPreferences expects to override getDefaultPath() to use NativeDesktop.get().getDefaultFileChooserDirectory(), but JabRefCliPreferences now defines a private method returning /.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[952-954]
  • jabgui/src/main/java/org/jabref/gui/preferences/JabRefGuiPreferences.java[1261-1265]
[reliability] Reset crashes clearing truststore
Reset crashes clearing truststore `clear()` calls `clearTruststoreFromCustomCertificates()`, which constructs a `TrustStoreManager` using `defaults.get(SSL_TRUSTSTORE_PATH).toString()`. The PR removed initialization of `SSL_TRUSTSTORE_PATH` in the `defaults` map, so reset can throw a `NullPointerException` and abort.

Issue description

Preferences reset can crash with an NPE because clearTruststoreFromCustomCertificates() still reads the truststore path from defaults, but the PR removed the defaults.put(SSL_TRUSTSTORE_PATH, ...) initialization.

Issue Context

clear() always calls clearTruststoreFromCustomCertificates(). That method should not depend on a defaults entry that may not exist; it should use getSSLPreferences().getTruststorePath() (with a safe fallback) or SSLPreferences.getDefault().getTruststorePath().

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[749-752]
  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[488-596]
                     PR 15838 (2026-05-27)                    
[correctness] `csv.begin.layout` missing `Chapter` header
`csv.begin.layout` missing `Chapter` header The new CSV header row omits standard BibTeX fields such as `chapter` and `annote`, so the exported CSV cannot include headers for all standard fields as required. This makes the export incomplete and inconsistent with the stated objective.

Issue description

The CSV header template does not include headers for all standard BibTeX fields (e.g., chapter, annote). This violates the requirement that csv.begin.layout emits a complete header row for all standard fields.

Issue Context

StandardField includes standard BibTeX fields like ANNOTE and CHAPTER, but the current csv.begin.layout header line does not include corresponding columns.

Fix Focus Areas

  • jablib/src/main/resources/resource/layout/csv/csv.begin.layout[1-1]
  • jablib/src/main/resources/resource/layout/csv/csv.layout[1-1]
[reliability] `GenericCsvExportFormatTest` lacks key test
`GenericCsvExportFormatTest` lacks key test The added tests do not verify the full required header set and do not assert citation key export behavior, so they don’t validate the key requirements of the new CSV exporter. This reduces regression protection for the new export format.

Issue description

The new CSV exporter tests only check that the header contains a handful of substrings and never assert citation key output behavior. The compliance requirement expects automated tests to validate headers (for all standard fields) and citation key behavior.

Issue Context

performExportContainsAllStandardFieldHeaders currently asserts only a few header fragments, and the tests never set a citation key (e.g., via withCitationKey(...)) to validate how it is exported.

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/exporter/GenericCsvExportFormatTest.java[46-62]
  • jablib/src/test/java/org/jabref/logic/exporter/GenericCsvExportFormatTest.java[93-111]
[correctness] Unescaped quotes in CSV
Unescaped quotes in CSV The new `csv.layout` wraps most fields in double quotes but does not escape embedded `"` characters for those fields, producing malformed CSV when values contain quotes. This can break spreadsheet import and can also violate the template README’s “one row per entry” promise when fields contain line breaks.

Issue description

The new generic CSV template quotes values (e.g., "\author") but does not apply CSV escaping for embedded double quotes (and does not normalize newlines), so exported CSV can be malformed or span multiple lines per entry.

Issue Context

CSV requires embedded " inside quoted fields to be escaped as "". JabRef already has ReplaceWithEscapedDoubleQuotes for exactly this purpose, but the new template only applies it to \citationkey.

Fix Focus Areas

  • jablib/src/main/resources/resource/layout/csv/csv.layout[1-1]
  • jablib/src/main/resources/resource/layout/csv/README[1-4]
  • jablib/src/main/java/org/jabref/logic/layout/format/ReplaceWithEscapedDoubleQuotes.java[5-18]

Suggested fix

  • Wrap each exported field in a formatter chain that at least applies ReplaceWithEscapedDoubleQuotes (and ideally also replaces \n with a space to keep one row per entry), e.g.:
  • "\format[ReplaceWithEscapedDoubleQuotes]{\author}"
  • or "\format[Replace(\n, ),ReplaceWithEscapedDoubleQuotes]{\abstract}"
  • Consider also quoting/escaping the citation key consistently (currently it is escaped but not quoted).
  • Add/extend a unit test to include a field value containing a double quote (and optionally a newline) and assert the output is valid (e.g., contains doubled quotes).
[reliability] Missing exporter test lock
Missing exporter test lock `GenericCsvExportFormatTest` lacks the `@Execution(SAME_THREAD)` / `@ResourceLock("exporter")` guards used by other exporter tests, so it may run concurrently and become flaky. This is especially risky because `TemplateExporter` mutates global `Number.serialExportNumber` during export.

Issue description

GenericCsvExportFormatTest can run in parallel with other exporter tests, but exporter code mutates global static state, which can lead to non-deterministic failures.

Issue Context

TemplateExporter sets and increments Number.serialExportNumber (a public static int) during export. Existing exporter tests (e.g., CsvExportFormatTest) protect against this by forcing same-thread execution and applying a shared ResourceLock("exporter").

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/exporter/GenericCsvExportFormatTest.java[18-33]
  • jablib/src/main/java/org/jabref/logic/exporter/TemplateExporter.java[240-246]
  • jablib/src/main/java/org/jabref/logic/layout/format/Number.java[7-19]
  • jablib/src/test/java/org/jabref/logic/exporter/CsvExportFormatTest.java[31-33]

Suggested fix

  • Add the same annotations used by CsvExportFormatTest:
  • @Execution(ExecutionMode.SAME_THREAD)
  • @ResourceLock("exporter")
  • and the required imports.
  • (Optional) Add an @AfterEach cleanup like the existing test class, for consistency.
                     PR 15837 (2026-05-26)                    
[correctness] Duplicate extension on invalid path
Duplicate extension on invalid path When Path.of(fileName) throws InvalidPathException in getBaseName, getBaseName returns the full filename (including extension), but getValidFileName then appends the extracted extension again, producing results like "name.pdf.pdf".

Issue description

getValidFileName now always rebuilds the filename as cleanedBase + '.' + extension. If getBaseName hit InvalidPathException it returns the original fileName (which can still contain the extension), leading to duplicated extensions (<cleanedNameIncludingExt>.<ext>).

Issue Context

  • getFileExtension was updated to tolerate InvalidPathException via a fallback, so the invalid-path flow is explicitly supported.
  • getBaseName still returns the raw fileName on InvalidPathException, which is not a "base name".
  • The newly added test only asserts non-empty output for an invalid-path-like input, so it would not catch .pdf.pdf.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[118-131]
  • jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[178-198]
  • jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java[409-413]

Suggested fix

In getBaseName(String) catch block, compute a best-effort base name instead of returning the full input. For example:

  • Use FilenameUtils.getBaseName(FilenameUtils.getName(fileName.trim())) (matching the strategy used in getFileExtension).

Add/strengthen test

Update getValidFileNameDoesNotThrowForInvalidPathCharacters (or add a new test) to assert the output does not contain a duplicated extension (e.g., does not end with .pdf.pdf).

[reliability] Unhandled InvalidPathException
Unhandled InvalidPathException FileUtil.getValidFileName now unconditionally calls getFileExtension(fileName), but getFileExtension uses Path.of(fileName.trim()) without handling InvalidPathException, so getValidFileName can now throw at runtime for inputs it is supposed to clean. This is a regression because getBaseName explicitly tolerates InvalidPathException and getValidFileName is used in file-export/rename flows where the input can originate from user/library data.

Issue description

FileUtil.getValidFileName(String) now always evaluates the extension via getFileExtension(fileName). getFileExtension(String) calls Path.of(fileName.trim()) without catching InvalidPathException, which means getValidFileName can throw for filenames containing characters invalid for Path parsing on the current OS (exactly the kind of inputs this method exists to sanitize).

Issue Context

  • getBaseName(String) already catches InvalidPathException and falls back to the raw string, implying invalid path strings are expected/possible.
  • getValidFileName(String) is used in export and linked-file flows, so an exception here can break rename/export operations.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[76-97]
  • jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[112-125]
  • jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[172-188]

Suggested fix approach

  • Make getFileExtension(String) resilient by catching InvalidPathException and extracting the “name” portion without Path.of (e.g., FilenameUtils.getName(trimmed)), then proceed with FilenameUtils.getExtension(...).
  • This keeps extension extraction consistent for all callers.
  • Alternatively (less ideal), wrap the getFileExtension(fileName) call inside getValidFileName with try/catch and fall back to a string-based extension parse on failure.
  • Add a unit test that passes a name containing characters that would be invalid for Path.of on Windows (e.g., "a:b.pdf") and assert getValidFileName returns a sanitized value without throwing.
[maintainability] `Stream.generate` for repeat string
`Stream.generate` for repeat string The new test builds long strings using `Stream.generate(...).limit(...).collect(Collectors.joining())` where the modern and clearer Java API `"1".repeat(n)` would be appropriate. This introduces an outdated pattern in newly added code and reduces readability.

Issue description

The new test constructs repeated-character strings using Stream.generate(...).collect(Collectors.joining()), which is less idiomatic than String#repeat(int) on modern Java.

Issue Context

The project encourages using modern Java (25+) idioms where applicable to improve readability/maintainability.

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java[401-408]
[maintainability] Misaligned chain indentation
Misaligned chain indentation The newly added test uses deeply indented chained-call formatting that deviates from the surrounding method-chaining style in the same test class. This introduces unnecessary formatting inconsistency in modified areas.

Issue description

The new test’s method-chain indentation is inconsistent with the existing formatting in FileUtilTest, creating a noisy/less consistent style.

Issue Context

This PR should preserve established formatting conventions and avoid unnecessary reformatting/stylistic divergence.

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java[401-408]
[correctness] Negative truncation length
Negative truncation length When truncating, the computed substring length can become negative if the extension length is very large, causing StringIndexOutOfBoundsException in getValidFileName. The new full-filename length check makes this branch reachable even when the base name is short but the extension is extremely long.

Issue description

getValidFileName computes MAXIMUM_FILE_NAME_LENGTH - (extensionLength + 1) and uses that directly as the substring end index. If the extension is extremely long, this value can be negative and substring(0, negative) throws.

Issue Context

This is an edge case (very long extensions are unusual), but the failure mode is a hard exception in a core utility method.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[172-188]

Suggested fix approach

  • Guard the computed max base length:
  • int maxBaseLen = MAXIMUM_FILE_NAME_LENGTH - extension.map(s -> s.length() + 1).orElse(0);
  • If maxBaseLen <= 0, avoid calling substring with a negative index.
  • Simple safe fallback: return fullCleanedName.substring(0, MAXIMUM_FILE_NAME_LENGTH) when maxBaseLen <= 0.
  • (Alternative: truncate the extension to fit, but any safe non-throwing behavior is better than an exception.)
  • Add a unit test with an extension length > 254 to assert no exception and the result length is <= 255.
                     PR 15835 (2026-05-26)                    
[maintainability] Duplicated `mutationScheduler` change code
Duplicated `mutationScheduler` change code Two cleanup jobs duplicate the same `ArrayList` + `mutationScheduler.accept(() -> entry.setFiles(...))` pattern, increasing maintenance cost and the risk of inconsistent future fixes. This violates the no-duplication requirement.

Issue description

MoveFilesCleanup and RenamePdfCleanup both contain a duplicated snippet that (1) creates a mutable list, (2) schedules an entry mutation via mutationScheduler, and (3) returns the list. This should be centralized to avoid duplication and keep behavior consistent.

Issue Context

Both classes are CleanupJob implementations that need to schedule BibEntry mutations through mutationScheduler while returning List<FieldChange>. The duplicated pattern can be replaced by a shared helper (e.g., a reusable utility method or a generic helper in CleanupJob) that runs a mutation via the scheduler and returns the produced List<FieldChange>.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java[53-56]
  • jablib/src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java[59-63]
[performance] FX-thread file serialization
FX-thread file serialization MoveFilesCleanup and RenamePdfCleanup call BibEntry#setFiles(files) inside mutationScheduler, which in the GUI is UiTaskExecutor::runAndWaitInJavaFXThread, so FILE-field serialization runs on the JavaFX thread. This can cause noticeable FX-thread stalls for entries with many linked files or long paths, even though only the actual field mutation needs to be scheduled.

Issue description

MoveFilesCleanup and RenamePdfCleanup schedule entry.setFiles(files) on mutationScheduler. In the GUI cleanup flow, this scheduler is UiTaskExecutor::runAndWaitInJavaFXThread, which means BibEntry#setFiles (and its FileFieldWriter.getStringRepresentation(files) serialization work) runs on the JavaFX thread.

Issue Context

This PR correctly moved the actual BibEntry mutation onto the scheduler thread to avoid “not on FX thread” exceptions. However, setFiles(...) does both (1) computation/serialization and (2) the mutation, so scheduling it also schedules the computation.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java[39-66]
  • jablib/src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java[36-72]

Suggested fix approach

  1. Keep file I/O and any string construction on the calling (background) thread.
  2. Precompute the new FILE field string (e.g., via FileFieldWriter.getStringRepresentation(files)) and compare to the current value off-FX.
  3. Only schedule the minimal mutation on the FX thread, e.g. entry.setField(StandardField.FILE, newValue) (or equivalent), and capture the returned FieldChange via an AtomicReference if you need to return it.
  4. Apply the same pattern in both MoveFilesCleanup and RenamePdfCleanup.
                     PR 15832 (2026-05-26)                    
[reliability] Heylogs failure masked
Heylogs failure masked The CHANGELOG lint step forces success with `|| true` and only fails based on counting `::error` lines in `heylogs.txt`, so if jbang/heylogs fails without emitting GitHub Actions `::error` annotations to stdout (e.g., stderr-only failure), CI will pass without validating CHANGELOG.md.

Issue description

The workflow masks the exit code of the heylogs invocation (|| true) and decides pass/fail only by grepping ::error lines in heylogs.txt. If heylogs/jbang fails without producing those annotations on stdout (common for runtime failures which go to stderr), error_count becomes 0 and the step passes, resulting in false-green CI.

Issue Context

You still want to ignore the specific heylogs rule all-h2-contain-a-version because of the Unreleased section, but you should not ignore unexpected command failures.

Fix Focus Areas

  • .github/workflows/tests-code.yml[188-200]

Suggested fix approach

  • Capture the pipeline exit code (via PIPESTATUS) and fail on unexpected non-zero exit codes, while still allowing the lint-error path to be handled by parsing ::error lines.
  • Also consider capturing stderr into heylogs.txt (2>&1) so runtime failures are visible and can be surfaced. Example pattern:
                     PR 15830 (2026-05-26)                    
[correctness] Git options bypass guard
Git options bypass guard `find_violation()` assumes the git subcommand is the token immediately after `git`, so any global git option placed there (e.g., `git -C … rebase`) will bypass the rebase/force-push blocking entirely. This undermines the PR’s core goal of reliably forbidding rebase/force-push for AI agents.

Issue description

The hook assumes tokens[i+1] is the git subcommand, which fails when git global options appear between git and the subcommand.

Issue Context

Git invocations can legally include global options before the subcommand; the hook should identify the actual subcommand before deciding whether to deny.

Fix Focus Areas

  • .claude/hooks/git-rebase-guard.py[18-53]

Implementation notes

  • When you encounter a git token, scan forward past recognized global options (and their arguments where applicable, e.g. -C <dir>, -c <name>=<value>, --git-dir=<path>, --work-tree=<path>) until the first non-option token, and treat that as the subcommand.
  • Then apply the existing rebase/pull/push checks against that subcommand and the remaining args.
[correctness] Pull -r bypass allowed
Pull -r bypass allowed The hook only blocks `git pull` when it sees `--rebase`/`--rebase=…`, so other rebase-enabling pull variants (e.g., `-r`) will not be denied. This allows rebase-based pulls to proceed despite the stated policy intent.

Issue description

The git pull check only detects --rebase and --rebase=... tokens, missing other rebase-enabling variants.

Issue Context

To reliably forbid rebase-based pulls, the hook should treat common short/variant flags as equivalent to --rebase.

Fix Focus Areas

  • .claude/hooks/git-rebase-guard.py[35-41]

Implementation notes

  • Extend the predicate to also match -r.
  • Consider matching any token that starts with --rebase (e.g., --rebase-merges) if the policy is “no rebase at all.”
[security] Force push refspec bypass
Force push refspec bypass The hook blocks force-push only when `--force`/`--force-with-lease`/`-f` is present, but force-updates can also be expressed via `+`-prefixed refspecs that won’t be detected. This leaves a direct path to perform a forbidden force push without triggering the guard.

Issue description

The push guard only checks for force flags and ignores refspec arguments, so +-prefixed refspec force-updates are not blocked.

Issue Context

To enforce “no force push,” the hook must also inspect non-flag arguments to git push.

Fix Focus Areas

  • .claude/hooks/git-rebase-guard.py[43-52]

Implementation notes

  • After identifying git push, scan remaining args for refspec tokens beginning with + (e.g., +HEAD:branch, +refs/heads/x:refs/heads/y) and deny if any are found.
  • Optionally also include other force-like flags (if relevant to your policy) alongside the existing set.
[maintainability] Docs/hook pull mismatch
Docs/hook pull mismatch AGENTS.md says “Do not use bare `git pull`… Use the explicit `fetch` + `merge` pair,” but the hook does not deny plain `git pull`. This creates inconsistent guidance vs. enforcement for AI agents.

Issue description

Documentation forbids bare git pull, but the enforcement hook only blocks git pull when rebase is explicitly requested.

Issue Context

This mismatch can confuse agents (and humans) about what will be blocked vs. allowed.

Fix Focus Areas

  • AGENTS.md[440-452]
  • .claude/hooks/git-rebase-guard.py[35-41]

Implementation notes

Choose one:

  • Enforce the doc: deny any git pull invocation (regardless of flags) and instruct to use fetch + merge.
  • Or adjust AGENTS.md wording to clarify the hook only blocks explicit rebase pulls, if that’s the intended enforcement scope.
                     PR 15827 (2026-05-25)                    
[correctness] `withMainFileDirectory` sets wrong property
`withMainFileDirectory` sets wrong property `FilePreferences.withMainFileDirectory` incorrectly assigns the provided path to `workingDirectory` (and even uses a misleading parameter name), so callers attempting to reset/import the main file directory do not actually update it and may instead overwrite the working directory setting. This breaks expected preference reset behavior (including the GUI reset path) and makes the API naming inconsistent and misleading.

Issue description

FilePreferences.withMainFileDirectory(...) currently updates workingDirectory instead of mainFileDirectory, and its parameter name (lastUsedDirectory) is misleading. As a result, callers (notably preference reset via JabRefCliPreferences.clear() used by the GUI reset path) silently fail to reset the main file directory and may unintentionally overwrite the working directory.

Issue Context

  • The method is intended (by name and usage) to set the main file directory, but it assigns the provided Path to the wrong field.
  • JabRefCliPreferences.clear() uses FilePreferences.getDefault().withMainFileDirectory(getDefaultPath()) to reset the main file directory.
  • In GUI mode, getDefaultPath() is overridden to the default file chooser directory, so this bug directly breaks the expected GUI preferences reset behavior.
  • The compliance checklist expectation of small, focused, naming-consistent code is violated because the method name/purpose and implementation disagree.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/FilePreferences.java[384-387]
  • jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java[991-996]
  • jabgui/src/main/java/org/jabref/gui/preferences/JabRefGuiPreferences.java[1224-1228]
[reliability] Tests mock old return
Tests mock old return FilePreferences.getMainFileDirectory() now returns Path, but some tests still mock it as Optional, which will fail compilation and block the build.

Issue description

The API change from Optional<Path> getMainFileDirectory() to Path getMainFileDirectory() was not propagated to all tests; remaining mocks returning Optional.of(...) no longer match the method signature.

Issue Context

Two test files still return Optional<Path> from getMainFileDirectory(), which should now return Path.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/FilePreferences.java[142-152]
  • jabgui/src/test/java/org/jabref/gui/desktop/os/BibDatbaseContextTest.java[27-36]
  • jabgui/src/test/java/org/jabref/gui/fieldeditors/contextmenu/ContextMenuFactoryTest.java[58-65]

Suggested fix

  • Replace thenReturn(Optional.of(path)) with thenReturn(path).
  • Remove now-unused Optional imports/usage in those tests.
[correctness] Blank directory becomes CWD
Blank directory becomes CWD LinkedFilesTabViewModel stores the main file directory via Path.of(text) and the validator does not reject blank input, so clearing the field can produce an empty Path that resolves relative to the current working directory and is treated as valid.

Issue description

The linked-files preferences UI converts the text field to a Path using Path.of(...) without handling blank input. An empty string yields an "empty" path; the validator only checks Files.exists/isDirectory and therefore can accept the current working directory when the field is cleared.

Issue Context

This behavior is new/impactful because storeSettings() now parses the path immediately (instead of storing a raw string), and the validator currently has no explicit blank-string guard.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java[64-80]
  • jabgui/src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java[116-120]

Suggested fix

  • In the validator: if the value is null/blank, return a validation error (or normalize to the desired default).
  • In storeSettings(): guard against blank input; either
  • normalize blank to the default main directory (e.g., preferences default), or
  • prevent saving and rely on validation to block closing the dialog.
  • Add a regression test covering blank input behavior.
                     PR 15823 (2026-05-24)                    
[maintainability] `AuthorListParser` formatting-only changes
`AuthorListParser` formatting-only changes Several changes in `AuthorListParser` appear to be formatting-only (notably unusually wide indentation in `Set.of(...)` declarations and comment/Javadoc reflow) and unrelated to the functional fix, creating noisy diffs and reducing readability. This violates the requirement to avoid reformatting and keep changes minimal and consistent with the existing style.

Issue description

AuthorListParser.java includes formatting-only changes (especially unusually wide indentation in Set.of(...) declarations and comment/Javadoc reflow) that are not required for the behavioral fix, creating unnecessary diff noise and deviating from the existing formatting style.

Issue Context

The PR’s functional change is in token case detection, but multiple unrelated formatting edits were introduced, including re-indentation of the AVOID_TERMS_IN_LOWER_CASE and TEX_NAMES Set.of(...) constants and reflow of comments/Javadoc into long single lines. These should be reverted or adjusted back to the standard continuation indentation used across the codebase to reduce churn and improve readability.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[23-26]
  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[36-39]
  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[111-116]
  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[200-201]
  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[420-426]
[maintainability] `CHANGELOG` mentions `namePrefix`
`CHANGELOG` mentions `namePrefix` The new changelog bullet is written with internal implementation terms (e.g., `namePrefix`, `familyName`) and starts with lowercase “we,” which makes it less end-user focused and inconsistent with the existing `- We ...` capitalization/style used in surrounding entries. This violates the changelog requirement for user-oriented language and consistent formatting in release notes.

Issue description

The added CHANGELOG.md entry is not end-user oriented (it uses internal implementation terminology like namePrefix/familyName) and it breaks the existing bullet style by starting with lowercase “we” instead of matching the surrounding - We ... capitalization.

Issue Context

This is user-facing release-note text in the ### Fixed section; surrounding bullets consistently start with - We fixed ... and avoid internal code-level field names. The current entry uses lowercase we and references internal terms (namePrefix, familyName), making it inconsistent with the established changelog style and less readable for end users.

Fix Focus Areas

  • CHANGELOG.md[31-38]
  • CHANGELOG.md[37-37]
[maintainability] Misleading tokenCase logic
Misleading tokenCase logic `getToken()` now sets `tokenCase` true for any letter that is not in Unicode category `LOWERCASE_LETTER`, which no longer matches the in-file documentation (“true if upper-case token”) and makes the condition redundant/unclear. This increases the risk of future incorrect changes because `tokenCase` drives von/last-name splitting decisions.

Issue description

tokenCase is documented as “true if upper-case token, false if lower-case”, but the new condition effectively treats any non-lowercase letter (including caseless scripts) as true. The current expression is also redundant: under the existing Character.isLetter(c) guard, Character.getType(c) != Character.LOWERCASE_LETTER already covers upper-case and Han, making isUpperCase/HAN checks unnecessary.

Issue Context

tokenCase is later used to decide whether a token starts the “von part” (!tokenCase) and where the last name begins (tokenCase), so readability and correct semantics matter.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[54-57]
  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[467-475]
  • jablib/src/main/java/org/jabref/logic/importer/AuthorListParser.java[230-276]

Suggested change

  • Replace the assignment with something explicit and self-documenting, e.g.:
  • tokenCase = !Character.isLowerCase(c);
  • or keep getType but simplify to: tokenCase = Character.getType(c) != Character.LOWERCASE_LETTER;
  • Update the field comment to match the new semantics, e.g. “true if token does not start with a lowercase letter (uppercase/caseless)”.
  • (Optional) add a small comment explaining the caseless-script rationale so the intent is preserved.
                     PR 15810 (2026-05-22)                    
[maintainability] Blank line before `req~`
Blank line before `req~` In `docs/requirements/slr.md`, each requirement heading is separated from its OpenFastTrace `req~...` identifier line by a blank line, violating the repository’s required requirements authoring format. This can impair automated requirement extraction/tracing and reduce traceability consistency.

Issue description

The OpenFastTrace/requirements authoring convention requires the requirement identifier line (e.g., req~...~1) to appear immediately under the Markdown heading with no blank line in between. In docs/requirements/slr.md, an empty line is placed between each ## ... heading and its req~... identifier, which can break or confuse automated requirement extraction and tracing.

Issue Context

PR Compliance ID 33 and the repo’s requirements guide explicitly state that IDs must be directly below headings with no empty line. Other requirement documents under docs/requirements/ follow this pattern, and CI runs gradle traceRequirements, so consistent formatting is required for tooling to parse requirements reliably.

Fix Focus Areas

  • docs/requirements/slr.md[19-41]
[reliability] Markdown trailing whitespace
Markdown trailing whitespace `docs/requirements/slr.md` contains a trailing space at the end of a list item, which violates markdownlint MD009 and can fail the CI Markdown job. This will block merging even though the change is only documentation.

Issue description

A trailing space at end-of-line violates markdownlint (MD009).

Issue Context

The CI workflow runs markdownlint over docs/**/*.md, and .markdownlint.yml keeps default rules enabled.

Fix Focus Areas

  • docs/requirements/slr.md[13-13]
                     PR 15789 (2026-05-20)                    
[maintainability] Changelog entry lacks PR link
Changelog entry lacks PR link The new CHANGELOG bullet for the `github-actions` output format does not include an issue/PR reference link, unlike surrounding entries. This reduces traceability and consistency in user-facing documentation.

Issue description

The newly added changelog entry is missing the required issue/PR reference, reducing traceability and making the changelog inconsistent.

Issue Context

In CHANGELOG.md, entries should reference an issue (#NUM) or link the PR implementing the change.

Fix Focus Areas

  • CHANGELOG.md[20-20]
                     PR 15780 (2026-05-19)                    
[reliability] Skip import on adjust error
Skip import on adjust error If `adjustLinkedFilesForTargetIfRequired` fails, the `onFailure` path marks the entry as skipped and never calls `importCleanedEntries`, so the entry is not imported at all even though only linked-file adjustment failed. This can drop entries on common filesystem/path issues during transfer (e.g., invalid paths or unexpected runtime exceptions in file operations).

Issue description

ImportHandler#importEntryWithDuplicateCheck runs linked-file adjustment in a background task. If that task fails, it marks the entry as skipped and does not import it, causing silent entry loss when adjustment fails.

Issue Context

adjustLinkedFilesForTargetIfRequired calls LinkedFileTransferHelper.adjustLinkedFilesForTarget, which performs path and filesystem operations. While many IO failures are handled internally, uncaught runtime exceptions (e.g., invalid path strings) can still happen and currently abort the entry import.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[286-296]

Suggested fix

In the .onFailure(...) of the linked-file adjustment task:

  • still proceed with importCleanedEntries(transferInformation, List.of(entryForImport)) (or the best available unadjusted entry)
  • call tracker.markImported(...) (or mark as imported-with-warning)
  • optionally show a user-visible warning/notification instead of only logging
[reliability] Dialog lock likely ineffective
Dialog lock likely ineffective `getDuplicateDecision` uses `synchronized` to prevent multiple duplicate resolver dialogs, but all `BackgroundTask` success callbacks are invoked on the JavaFX thread, so the lock does not serialize same-thread dialog requests. Because the dialog is shown via `Dialog.showAndWait()`, which runs a nested event loop, other queued FX callbacks can still reach `getDuplicateDecision` while a dialog is open, allowing additional dialogs to open before the first decision is made.

Issue description

The PR adds synchronized (duplicateDecisionLock) around dialogService.showCustomDialogAndWait(dialog) to prevent multiple dialogs. However, duplicate handling runs on the JavaFX thread, and showCustomDialogAndWait calls Dialog.showAndWait() which runs a nested event loop; additional FX callbacks can still fire and re-enter getDuplicateDecision.

Issue Context

Multiple entries are imported concurrently (importEntriesWithDuplicateCheck starts tasks in a loop). Several entries can discover duplicates and attempt to resolve them while the first dialog is still open.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[339-362]
  • jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[502-520]

Suggested fix options

Pick one approach:

  1. Batch-serialize duplicate resolution: process entries sequentially for the duplicate-resolution phase (chain background tasks) so only one dialog can be requested at a time.
  2. Queue dialog requests: maintain a single in-flight decision request and queue subsequent duplicate checks until the dialog completes, then resume with the chosen decision.
  3. If keeping a lock, replace it with an explicit non-reentrant “dialog in progress” gate + queue; synchronized alone does not prevent re-entry on the FX thread.
                     PR 15779 (2026-05-19)                    
[correctness] Crossref KEY inheritance broken
Crossref KEY inheritance broken BibEntry#getSourceField now treats StandardField.KEY as a forbidden field, so BibEntry#getResolvedFieldOrAlias(StandardField.KEY, db) will no longer inherit "key" from the crossref parent. This contradicts the existing CrossrefTest expectations that StandardField.KEY is inherited via same-name inheritance, likely breaking crossref resolution behavior and tests.

Issue description

BibEntry#getSourceField was changed to return Optional.empty() for StandardField.KEY. This disables crossref inheritance for the BibTeX key field. The repo’s crossref tests indicate StandardField.KEY is expected to be inherited (same-name inheritance). With the new forbidden check, resolving KEY via getResolvedFieldOrAlias(..., db) will return empty, causing behavior change and likely test failures.

Issue Context

  • InternalField.KEY_FIELD is the citation key; StandardField.KEY is a normal BibTeX field (used for sorting in some styles). Even if it’s “rare”, the codebase currently expects it to participate in crossref inheritance.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/model/entry/BibEntry.java[188-198]

Suggested fix

Remove StandardField.KEY from the forbidden-fields list in getSourceField (or, if the intended behavior is to stop inheriting it, update the crossref behavior/tests accordingly and document the breaking change).

                     PR 15775 (2026-05-19)                    
[maintainability] PR template instructs emoji praise
PR template instructs emoji praise The PR template now instructs adding a praise sentence ending with a `⭐` emoji, which is unprofessional and inconsistent for contributor-facing documentation. This can reduce the seriousness and clarity of PR descriptions.

Issue description

The PR template includes an instruction to add a sentence praising JabRef and end it with a ⭐ emoji. This is not professional guidance for contributor documentation and may encourage low-signal PR descriptions.

Issue Context

This change is in .github/PULL_REQUEST_TEMPLATE.md under the ### PR Description guidance.

Fix Focus Areas

  • .github/PULL_REQUEST_TEMPLATE.md[32-33]
                     PR 15773 (2026-05-19)                    
[correctness] Wrong `JabRef_en.properties` path
Wrong `JabRef_en.properties` path The new “Removing an obsolete key” section in `docs/code-howtos/localization.md` tells contributors to edit `src/main/resources/l10n/JabRef_en.properties`, but the repository’s actual English localization bundle is `jablib/src/main/resources/l10n/JabRef_en.properties`. Following the documentation as written can lead to editing/creating the wrong file, conflicting with the localization workflow and potentially leaving `LocalizationConsistencyTest` failing.

Issue description

The “Removing an obsolete key” instructions in docs/code-howtos/localization.md currently direct contributors to edit src/main/resources/l10n/JabRef_en.properties, but the correct English localization file in this repository is jablib/src/main/resources/l10n/JabRef_en.properties. Update the documentation so contributors modify the correct file path and don’t accidentally edit/create a non-existent file or leave localization tests failing.

Issue Context

  • The localization workflow (PR Compliance ID 19) requires key additions/removals to be applied to jablib/src/main/resources/l10n/JabRef_en.properties.
  • The document already references the correct jablib/.../JabRef_en.properties path earlier, but the newly added “Removing an obsolete key” step uses src/main/resources/..., creating an internal inconsistency.

Fix Focus Areas

  • docs/code-howtos/localization.md[57-69]
[maintainability] Conflicting test class name
Conflicting test class name `localization.md` now tells readers to run `org.jabref.logic.l10n.LocalizationConsistencyTest` in the new removal flow, but the existing add-key flow still references `org.jabref.logic.LocalizationConsistencyTest` (wrong package). This inconsistency can send contributors to a non-existent test class and block the workflow the doc is trying to describe.

Issue description

docs/code-howtos/localization.md contains conflicting instructions for which test to run:

  • “Adding a new key” references org.jabref.logic.LocalizationConsistencyTest (incorrect).
  • The new “Removing an obsolete key” section references org.jabref.logic.l10n.LocalizationConsistencyTest (correct). This inconsistency is confusing and can cause contributors to try to run a test class that doesn’t exist.

Issue Context

The actual test class is in package org.jabref.logic.l10n.

Fix Focus Areas

  • docs/code-howtos/localization.md[55-67]
  • jablib/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java[1-5]
                     PR 15762 (2026-05-18)                    
[reliability] Null selected tab deref
Null selected tab deref LibraryTab#setDatabaseContext calls selectedItemProperty().get().equals(this), which can throw NullPointerException when the TabPane has no selected item. This can abort library loading and leave the StateManager in an inconsistent state.

Issue description

LibraryTab#setDatabaseContext uses tabPane.getSelectionModel().selectedItemProperty().get().equals(this) which will throw if the selected item is null.

Issue Context

Other code already accounts for TabPane having no selected tab (getSelectedItem() == null), so null is a real state and should be handled here as well.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/LibraryTab.java[372-382]

Suggested fix

Replace the dereference with a null-safe comparison, e.g.:

                     PR 15759 (2026-05-18)                    
[correctness] Integrity exit code wrong
Integrity exit code wrong CheckIntegrity.execute() always returns 0 even when integrity findings exist, so `jabkit check integrity` and `jabkit check FILE` cannot signal findings via exit status. This contradicts the parent `check` command’s documented exit-code semantics and can let CI pass with integrity problems.

Issue description

CheckIntegrity.execute(...) collects integrity messages, writes them, and then unconditionally returns 0. This prevents callers (including jabkit check FILE) from using exit codes to detect integrity findings.

Issue Context

The parent check command explicitly documents 1 = findings and combines exit codes using Math.max(...), but CheckIntegrity.execute(...) never returns 1.

Fix Focus Areas

  • jabkit/src/main/java/org/jabref/toolkit/commands/CheckIntegrity.java[83-112]
  • jabkit/src/main/java/org/jabref/toolkit/commands/Check.java[50-54]

Implementation notes

  • After writing findings, return messages.isEmpty() ? 0 : 1.
  • Update the execute Javadoc to include 1 = findings for consistency with Check and CheckConsistency.
[correctness] Key findings misformatted
Key findings misformatted IntegrityCheckResultErrorFormatWriter appends a field segment for citation-key findings emitted with `StandardField.KEY`, but requirements state citation-key (entry-level) findings must carry only the citation key. This breaks the documented `citationKey[:field]` shape and can confuse parsers consuming the errorformat stream.

Issue description

IntegrityCheckResultErrorFormatWriter only suppresses the field name when field == InternalField.KEY_FIELD. Some citation-key findings use StandardField.KEY, so the writer appends :key even though the requirement says entry-level findings on the citation key must not include a field segment.

Issue Context

CitationKeyDuplicationChecker emits IntegrityMessage(..., StandardField.KEY) for duplicate citation keys.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultErrorFormatWriter.java[23-41]
  • jablib/src/main/java/org/jabref/logic/integrity/CitationKeyDuplicationChecker.java[21-32]
  • docs/requirements/cli.md[28-36]

Implementation notes

  • Treat StandardField.KEY the same as InternalField.KEY_FIELD for formatting (do not append :field).
  • Optionally, for key-level findings use parserResult.getCompleteEntryIndicator(entry) for the range, to avoid depending on field-alias resolution for StandardField.KEY.
[maintainability] Unlocalized `Check` console message
Unlocalized `Check` console message `Check.call()` prints a hardcoded user-facing message instead of using the project localization mechanism. This prevents translation and violates the localization standard for user-visible messages.

Issue description

A user-facing CLI message is printed as a hardcoded English string, bypassing JabRef localization.

Issue Context

Other jabkit commands print user-visible output via Localization.lang(...). The new check command should follow the same convention.

Fix Focus Areas

  • jabkit/src/main/java/org/jabref/toolkit/commands/Check.java[44-46]
[reliability] JBang main job missing setup
JBang main job missing setup The `jbang-main` workflow job invokes a local composite action without checking out the repository or installing JDK/JBang, so it will fail on `main`. The same workflow’s `jbang-pr` job still performs checkout and setup, making this a main-branch-only regression.

Issue description

The jbang-main job calls ./.github/actions/jbang-check but does not run actions/checkout and does not install JDK/JBang (previously done via ./.github/actions/setup-gradle). Local composite actions require the repo to be present in the workspace, and the jbang-check action assumes jbang is available.

Issue Context

jbang-pr still performs checkout and uses setup-gradle (which installs JDK and JBang), but jbang-main no longer does.

Fix Focus Areas

  • .github/workflows/tests-code.yml[381-388]
  • .github/actions/jbang-check/action.yml[1-25]
  • .github/actions/setup-gradle/action.yml[14-44]
[observability] PdfUpdate errors on stdout
PdfUpdate errors on stdout PdfUpdate prints fatal import/parse errors to stdout while returning non-zero exit codes, mixing errors into the normal output stream. Other commands in this PR route equivalent failures to stderr, so PdfUpdate remains inconsistent for scripting and tooling.

Issue description

PdfUpdate.call() prints file-open / invalid-input failures using System.out.println(...) even though these are fatal errors (exit code 2). This makes it harder to separate normal output from errors in pipelines.

Issue Context

Several other commands updated in this PR already use System.err.println(...) for the same error paths.

Fix Focus Areas

  • jabkit/src/main/java/org/jabref/toolkit/commands/PdfUpdate.java[69-83]
  • jabkit/src/main/java/org/jabref/toolkit/commands/Convert.java[59-68]

Implementation notes

  • Replace the two System.out.println(...) calls in the error paths with System.err.println(...).
  • (Optional) Consider reusing the shared importBibtexLibrary(...) helper to standardize message + exit-code handling.
[maintainability] `CheckConsistency` uses Optional.get()
`CheckConsistency` uses Optional.get() The modified consistency- and integrity-check flows use `Optional.isEmpty()` followed by `Optional.get()`, which is discouraged by the Optional-control-flow compliance rule. This pattern is unnecessarily verbose and increases the risk of unsafe `get()` usage during future edits or refactors.

Issue description

New/modified code uses Optional.isEmpty() and then Optional.get(), violating the Optional-control-flow compliance rule.

Issue Context

PR Compliance ID 29 requires idiomatic Optional control flow (e.g., map, ifPresentOrElse, orElseThrow) instead of manual presence checks followed by get(), because the latter is verbose and can be accidentally made unsafe during later changes.

Fix Focus Areas

  • jabkit/src/main/java/org/jabref/toolkit/commands/CheckConsistency.java[53-83]
  • jabkit/src/main/java/org/jabref/toolkit/commands/CheckIntegrity.java[64-105]
                     PR 15755 (2026-05-18)                    
[correctness] Wrong CLI option description
Wrong CLI option description `--importToOpen` is described as “Same as --importBibtex”, but `importToOpen` actually appends a file/URL to the current library while `--importBibtex` appends a literal BibTeX string. This mismatch will lead callers to pass the wrong argument type and trigger import failures (or confusion), especially for the native host script that uses `--importToOpen` with a temp file path.

Issue description

The help/description string for the picocli option --importToOpen claims it is the same as --importBibtex, but the code paths take different input types:

  • --importToOpen expects a file path or URL to import.
  • --importBibtex expects a BibTeX string. This makes the CLI usage message incorrect and can cause wrong usage.

Issue Context

ArgumentProcessor routes these two options to different UiCommands, and the Windows native host script demonstrates that --importToOpen is invoked with a tempfile path.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/cli/GuiCommandLine.java[19-28]
                     PR 15750 (2026-05-17)                    
[correctness] ISSN test missing assertion
ISSN test missing assertion `exportsIssnNestedUnderSerialNumber` builds an `expected` YAML output but never asserts it against the exported file contents, so ISSN serialization is not actually covered and regressions could slip through. This violates the requirement to have unit tests validating ISSN is nested under `serial-number.issn` (and not emitted as a top-level field).

Issue description

The test exportsIssnNestedUnderSerialNumber currently constructs an expected YAML output but never compares it to the actual exporter output, meaning it cannot fail and does not validate that ISSN is nested under serial-number rather than emitted as a top-level YAML field.

Issue Context

PR Compliance requires unit tests for DOI/ISBN/ISSN export verifying the identifiers are nested under serial-number and not emitted as top-level YAML fields. In the same test class, the DOI and ISBN tests use assertEquals(expected, Files.readAllLines(file));, but the ISSN test ends right after creating expected without asserting the file contents.

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/exporter/HayagrivaYamlExporterTest.java[309-333]
                     PR 15747 (2026-05-17)                    
[maintainability] Changelog entry wording unpolished
Changelog entry wording unpolished The new CHANGELOG entry has grammatical issues (`newly added entry cannot be searched`) and reads unpolished for end users. This reduces professionalism and clarity in user-facing release notes.

Issue description

The newly added CHANGELOG bullet is grammatically incorrect/unpolished and should be rewritten in professional, end-user-friendly language.

Issue Context

Compliance requires professional and consistent user-facing documentation.

Fix Focus Areas

  • CHANGELOG.md[20-24]
[maintainability] `hashcode` misspelled in comment
`hashcode` misspelled in comment The newly added comment uses `hashcode` instead of `hashCode`, which violates spelling/style conventions and reduces readability. Comments should follow standard Java terminology.

Issue description

A newly added comment misspells Java terminology as hashcode.

Issue Context

JabRef code style requires correct spelling/terminology in modified code and comments.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/search/IndexManager.java[184-186]
                     PR 15744 (2026-05-15)                    
[maintainability] `AGENTS.md` promotes `Optional.get()`
`AGENTS.md` promotes `Optional.get()` The updated guidance tells contributors to use `Optional.get()` when the value is present, which encourages unsafe and non-idiomatic Optional handling. This conflicts with the project’s requirement to use idiomatic Optional control flow (e.g., `ifPresent`, `ifPresentOrElse`, `map`, `orElseThrow`) rather than presence checks and `get()`.

Issue description

AGENTS.md currently advises using Optional.get() “if really present”, which is discouraged by the compliance rules favoring idiomatic Optional control flow.

Issue Context

This guidance is used to steer contributors/agents; recommending get() can lead to fragile code patterns and conflicts with the project’s Optional conventions.

Fix Focus Areas

  • AGENTS.md[164-165]
[maintainability] `CHECLIST.md` broken reference
`CHECLIST.md` broken reference AGENTS.md instructs readers to follow `CHECLIST.md`, but the PR adds `CHECKLIST.md`, making the reference incorrect and pointing to a non-existent document. This creates a broken documentation reference that can mislead contributors/agents and undermine checklist enforcement.

Issue description

AGENTS.md references CHECLIST.md (typo), but the checklist file added/present in the repository is CHECKLIST.md. This creates a broken/incorrect documentation reference that prevents readers/agents from finding and following the checklist.

Issue Context

The new guidance in AGENTS.md is intended to be followed after code changes, and this PR adds a new checklist file at the repository root; a wrong filename makes the instruction fail and undermines checklist enforcement.

Fix Focus Areas

  • AGENTS.md[19-19]
[reliability] Broken docker mount variable
Broken docker mount variable The documented formatter command uses `$pwd` for the Docker volume mount, but the surrounding examples use Unix-style commands; in typical bash/zsh terminals `$pwd` is not defined, so the container won’t mount the repo directory correctly and formatting won’t run on the intended files.

Issue description

The Docker formatting command uses $pwd which is PowerShell-specific. In bash/zsh (consistent with the nearby ./gradlew examples), this variable is usually unset, causing an incorrect -v mount.

Issue Context

The same command is duplicated in CHECKLIST.md, so it should be updated there too.

Fix Focus Areas

  • AGENTS.md[341-342]
  • CHECKLIST.md[6-8]
[maintainability] Misleading JSpecify checklist
Misleading JSpecify checklist CHECKLIST.md says to “Use JSpecify instead of `== null`”, but the project’s nullness ADR describes JSpecify as progressive API annotation and the codebase still uses explicit null checks; this checklist item is phrased as a hard replacement rule that contradicts established practice.

Issue description

The checklist item implies JSpecify replaces runtime null checks, which is inaccurate/misleading. JSpecify annotations complement runtime checks by documenting/validating nullness contracts; explicit null checks remain common and necessary in implementations.

Issue Context

The project ADR explicitly frames JSpecify usage as progressive annotation of APIs for compile-time checking.

Fix Focus Areas

  • CHECKLIST.md[6-6]
                     PR 15743 (2026-05-15)                    
[correctness] Truncate before normalization
Truncate before normalization `BracketedPattern.authNofMth` now truncates the raw family name after a regex replace, before later key normalization expands characters (e.g., ö→oe), so patterns like `[auth3]` / `[auth4_1]` can yield longer/different prefixes than intended. This contradicts existing expectations (e.g., tests expecting `Köning` with `[auth3]` to produce `Koe`) and can lead to unexpected key changes/collisions.

Issue description

BracketedPattern.authNofMth truncates the author family name before the citation-key normalization step that expands some Unicode characters (e.g., ö → oe). This changes the semantics of [authN] / [authN_M] / [authIniN] because truncation is done on the pre-normalized string, but normalization happens later and can increase length or alter the prefix.

Issue Context

  • BracketedPattern.getFieldValue routes auth\d+ and auth\d+_\d+ patterns to authNofMth, which performs the truncation.
  • CitationKeyGenerator.expandBracketContent subsequently calls removeUnwantedCharacters(...), which invokes StringUtil.replaceSpecialCharacters(...) using UnicodeToReadableCharMap (e.g., \u00F6 → oe).
  • Because there is no re-truncation after normalization, the final output can exceed the intended N characters and differ from prior behavior.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[960-978]

How to fix

  • Ensure the same normalization used for citation keys (at minimum StringUtil.replaceSpecialCharacters, and ideally the full unwanted/disallowed filtering logic) is applied before taking substring(0, n) in authNofMth.
  • Suggested implementation approach:
  • Replace the replaceAll(...) with a call that normalizes first, then removes unwanted/disallowed characters, then truncates.
  • Prefer reusing existing logic (e.g., extract a shared helper for removeUnwantedCharacters-style normalization) rather than hardcoding a regex character class.
  • Add/adjust a focused unit test for [auth3] / [auth4_1] with an umlauted name (e.g., Köning, Gödel) to lock in the intended behavior (truncate after normalization).
[maintainability] Unwanted chars duplicated in regex
Unwanted chars duplicated in regex The unwanted-characters set is hard-coded inside a regex literal in `authNofMth`, duplicating the canonical default unwanted-characters definition. This duplication risks future drift/inconsistency if defaults change.

Issue description

BracketedPattern.authNofMth embeds the unwanted-characters list directly in the regex literal ([-ʹ:!;?^$]*`). This duplicates the default unwanted-characters definition maintained elsewhere.

Issue Context

Duplicated business rules (like "which characters are unwanted") can easily diverge over time and make bug fixes incomplete.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[974-976]
[performance] `cleanKey()` uses `replaceAll`
`cleanKey()` uses `replaceAll` `cleanKey` calls `replaceAll("\\s", "")`, which recompiles the regex on every invocation and violates the compiled-regex requirement for non-trivial/repeated regex usage. This can add avoidable overhead in citation-key generation paths.

Issue description

CitationKeyGenerator.cleanKey currently removes whitespace via String.replaceAll("\\s", ""), which recompiles the pattern per call.

Issue Context

The compliance checklist requires using compiled Pattern instances for repeated/non-trivial regex operations.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java[151-152]
[maintainability] Default constant mis-layered
Default constant mis-layered CitationKeyPatternPreferences now sources its default unwanted-characters value from CitationKeyGenerator.DEFAULT_UNWANTED_CHARACTERS, even though CitationKeyGenerator documents that this constant should not be used directly. This increases coupling (preferences depend on generator) and contradicts the documented contract, making future refactors riskier.

Issue description

CitationKeyPatternPreferences now depends on CitationKeyGenerator.DEFAULT_UNWANTED_CHARACTERS for its default value, while CitationKeyGenerator explicitly says the constant should not be used directly. This creates an unnecessary dependency from the preferences layer to the generator layer and contradicts the comment.

Issue Context

The PR goal is to avoid duplicated defaults, but the current direction of dependency makes CitationKeyPatternPreferences require the generator class to define its own defaults.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyPatternPreferences.java[59-74]
  • jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java[28-36]

Suggested fix

  • Move the default unwanted-characters constant into CitationKeyPatternPreferences (or a small dedicated CitationKeyDefaults/CitationKeyConstants class in the same package).
  • Update CitationKeyGenerator to reference that constant (or remove/rename the misleading comment if the generator is intended to be the source-of-truth).
  • Keep only one source-of-truth, but ensure dependencies flow from preferences/constants -> generator, not the reverse.
                     PR 15728 (2026-05-12)                    
[maintainability] Optional `get()` after `isEmpty()`
Optional `get()` after `isEmpty()` `CSLCitationRanges` uses `if (range.isEmpty()) continue;` followed by multiple `range.get()` calls, which is non-idiomatic Optional control flow and risks future unsafe access if the code changes.

Issue description

New code uses Optional.isEmpty() and then calls Optional.get() multiple times. This is discouraged; prefer ifPresent, map, or unwrapping once to a local variable.

Issue Context

This occurs in the newly added CSLCitationRanges method while retrieving a reference mark anchor.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/openoffice/frontend/OOFrontend.java[302-312]
[correctness] Protects non-JabRef marks
Protects non-JabRef marks OOFrontend.CSLCitationRanges marks every LibreOffice reference mark as a protected citation range, so inserting a CSL citation can fail when the cursor is inside an unrelated reference mark (user-created cross-references/bookmarks or other extensions’ marks). This can create false “cursor is in a protected area” errors and prevent valid citation insertion.

Issue description

CSLCitationRanges currently iterates over all LibreOffice reference marks (UnoReferenceMark.getListOfNames) and treats them as citation-protected ranges. This can block citation insertion whenever the cursor happens to be inside any unrelated reference mark.

Issue Context

Other parts of the codebase already filter reference marks to JabRef-owned ones (both for JStyle and CSL flows). CSLCitationRanges should similarly only include JabRef CSL citation reference marks (and optionally also JabRef JStyle marks if you want protection across style types).

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/openoffice/frontend/OOFrontend.java[295-316]

Suggested approach

  • Filter UnoReferenceMark.getListOfNames(doc) down to JabRef CSL citation marks only (e.g., using the same token/prefix logic as CSLReferenceMarkManager.readAndUpdateExistingMarks: split by space, require parts.length >= 3, parts[0].startsWith(ReferenceMark.PREFIXES[0]) and parts[1].startsWith(ReferenceMark.PREFIXES[1])).
  • Keep skipping marks with missing anchors.
  • (Optional hardening) If you want cursor protection to work even in mixed documents, build the protected list as (JStyleCitationRanges + filtered CSLCitationRanges) instead of relying on unfiltered “all reference marks” behavior.
[maintainability] Uppercase method names added
Uppercase method names added The new methods `JStyleCitationRanges` and `CSLCitationRanges` start with an uppercase letter, which violates Java/JabRef lowerCamelCase naming conventions and reduces readability/consistency.

Issue description

Newly introduced method names use UpperCamelCase (JStyleCitationRanges, CSLCitationRanges) instead of Java/JabRef lowerCamelCase, reducing codebase consistency.

Issue Context

These methods are new/modified in the OpenOffice frontend logic and should follow JabRef naming conventions.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/openoffice/frontend/OOFrontend.java[275-316]
[maintainability] Changelog entry is unpolished
Changelog entry is unpolished The new CHANGELOG entry contains grammatical issues (e.g., `another citations`, `which break`) and reads unprofessionally for end-user documentation.

Issue description

The added CHANGELOG entry has grammatical errors and should be rewritten to be professional and user-facing.

Issue Context

CHANGELOG entries are user-facing documentation and should be polished and consistent.

Fix Focus Areas

  • CHANGELOG.md[59-59]
                     PR 15712 (2026-05-11)                    
[maintainability] `pdfPath` uses `isPresent`+`get`
`pdfPath` uses `isPresent`+`get` The new OCR action uses `if (!pdfPath.isPresent())` followed by `pdfPath.get()`, which is the non-idiomatic and error-prone Optional pattern the project forbids. This reduces readability and can lead to unsafe refactors later.

Issue description

Optional is handled using isPresent() + get(), which violates the project's Optional usage rules.

Issue Context

In OcrLinkedFileAction.execute(), the code checks presence and then calls get() to pass the Path into the OCR task.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/linkedfile/OcrLinkedFileAction.java[51-56]
[correctness] Missing OCR l10n keys
Missing OCR l10n keys New user-facing OCR UI strings are passed to `Localization.lang(...)` in `OcrLinkedFileAction`, but their translation keys are missing from `JabRef_en.properties`, so they cannot be translated. Because missing keys are silently accepted, this undermines localization completeness and consistency for the new OCR flow, including error paths.

Issue description

Two new user-facing OCR messages are looked up via Localization.lang(...) but are not present in JabRef_en.properties, preventing translation and allowing missing keys to slip through silently.

Issue Context

OcrLinkedFileAction shows an error dialog when no file is found and a notification on unexpected failure, both using new localization keys/strings that are not defined in the English resource bundle. JabRef’s localization bundle behavior returns the key itself for missing entries and does not reliably warn, making incomplete localization coverage for these OCR error paths easy to miss.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/linkedfile/OcrLinkedFileAction.java[50-88]
  • jablib/src/main/resources/l10n/JabRef_en.properties[797-808]
  • jablib/src/main/java/org/jabref/logic/l10n/Localization.java[130-168]
[correctness] OCR adds untyped file
OCR adds untyped file OcrLinkedFileAction adds the OCR output using `new LinkedFile(Path)` which creates a linked file with an empty file type and without the normal path relativization/type-guessing used elsewhere. This prevents the OCR output from being recognized as an offline PDF in the UI and reduces library portability (absolute paths).

Issue description

On OCR success, the code creates new LinkedFile(success.outputFile()), which sets an empty file type and likely stores an absolute path. JabRef has established helpers to guess file type and relativize paths against configured file directories.

Issue Context

LinkedFile(Path) explicitly sets fileType to empty. LinkedFileViewModel treats files as “offline PDF” only when fileType == pdf, so the newly added OCR file won’t behave like a PDF link. LinkedFilesEditorViewModel#fromFile shows the expected pattern: guess type by extension and relativize path.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/linkedfile/OcrLinkedFileAction.java[60-67]
  • jablib/src/main/java/org/jabref/model/entry/LinkedFile.java[96-100]
  • jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java[100-110]
  • jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[106-108]
                     PR 15708 (2026-05-09)                    
[maintainability] ADR missing Jekyll front matter
ADR missing Jekyll front matter The new ADR file does not include the standard MADR/Jekyll front matter (`---`, `nav_order`, `parent`) used by existing decision records, so it likely won’t be indexed/rendered correctly in the documentation site. This does not meet the requirement to create the ADR using the project template/required sections.

Issue description

The new ADR docs/decisions/0056-OCR-engine-selection.md is missing the standard YAML front matter (nav_order, parent) used by JabRef decision records and the provided ADR template.

Issue Context

Without the standard front matter, the ADR may not show up in the rendered Decision Records navigation/index and does not follow the project’s MADR/template conventions required by compliance.

Fix Focus Areas

  • docs/decisions/0056-OCR-engine-selection.md[1-5]
[maintainability] ADR has spelling/grammar errors
ADR has spelling/grammar errors The added ADR contains clear spelling/grammar issues (e.g., `eauations`, double punctuation), reducing the professionalism and polish of user-facing documentation. This violates the requirement to keep documentation polished and professional.

Issue description

The ADR text contains spelling/grammar/punctuation mistakes (e.g., mathematical eauations and weights..).

Issue Context

This is user-facing documentation under docs/, and should be polished and professional.

Fix Focus Areas

  • docs/decisions/0056-OCR-engine-selection.md[15-18]
  • docs/decisions/0056-OCR-engine-selection.md[182-185]
                     PR 15707 (2026-05-08)                    
[maintainability] Misleading test name
Misleading test name The method name aPACitation2Authors suggests it tests “two authors”, but it actually tests citing two different entries in one combined citation. This can mislead future maintenance and weaken understanding of coverage.

Issue description

aPACitation2Authors is named like it validates a two-author scenario, but the test actually validates a combined in-text citation for two entries.

Issue Context

The method calls generateCitation(List.of(testEntry, testEntry2), ...) and asserts the combined output.

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/citationstyle/CitationStyleGeneratorTest.java[75-84]

Suggested fix

Rename the test to something like aPACitationMultipleEntries() or aPACitationTwoEntries() (or similar) so the name matches what is asserted.

                     PR 15688 (2026-05-06)                    
[maintainability] `Reset current template` unlocalized
`Reset current template` unlocalized The new/modified FXML introduces a hard-coded user-facing string (`Reset current template`) instead of using a localization key. This breaks localization and UI text consistency across languages.

Issue description

AiTab.fxml contains a hard-coded user-facing button label (Reset current template) instead of a %... localization key.

Issue Context

Project UI strings must be localized (FXML text values should be %key).

Fix Focus Areas

  • jabgui/src/main/resources/org/jabref/gui/preferences/ai/AiTab.fxml[345-347]
[reliability] `extractCitationsUsingLLM` catches `Exception`
`extractCitationsUsingLLM` catches `Exception` `extractCitationsUsingLLM(...)` catches generic `Exception` and wraps it in a new `RuntimeException`, which can hide expected failure modes and crash callers unexpectedly. This violates the requirement for specific, minimal exception handling and avoiding unchecked exception wrapping.

Issue description

The method extractCitationsUsingLLM catches Exception broadly and rethrows new RuntimeException(e), which is against the exception-handling policy.

Issue Context

Prefer catching only expected exception types (and handle/log appropriately) rather than Exception, and avoid introducing unchecked exception wrapping in normal flows.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/importer/fileformat/pdf/CitationsFromPdf.java[27-38]
[correctness] Summary tasks not deduped
Summary tasks not deduped `SummarizationTaskAggregator.startNewTask(...)` never stores the created `GenerateSummaryTask` in `tasks`, so `start()` always creates a fresh task for the same entry. This breaks deduplication and can trigger duplicate summarization requests and inconsistent UI/task state.

Issue description

SummarizationTaskAggregator is intended to deduplicate summarization tasks, but startNewTask(...) never inserts the created task into the tasks map. As a result, getTask(...) always returns empty and deduplication never happens.

Issue Context

The aggregator maintains tasks keyed by BibEntry (by id comparator), but the map is only read and removed from—never written.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/ai/summarization/SummarizationTaskAggregator.java[46-85]

Suggested fix

  • In startNewTask(...), add tasks.put(request.fullEntry().entry(), task); before executing/returning.
  • Consider simplifying start(...) to a single computeIfAbsent flow and ensure onFinished removal matches the same key used for insertion.
[correctness] `getKind()` returns `null`
`getKind()` returns `null` `AverageTokenEstimator`, `ByCharacterTokenEstimator`, and `ByWordsTokenEstimator` return `null` from `getKind()`, which breaks the non-null contract of the API and can cause NPEs where the kind is used. These implementations should return the corresponding `TokenEstimatorKind` enum constants.

Issue description

Several TokenEstimator implementations return null from getKind(), violating null-safety expectations and risking NPEs.

Issue Context

TokenEstimatorKind defines AVERAGE, WORDS, and CHARS, so the concrete estimators should return a non-null kind.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/ai/tokenization/logic/AverageTokenEstimator.java[32-35]
  • jablib/src/main/java/org/jabref/logic/ai/tokenization/logic/ByCharacterTokenEstimator.java[31-34]
  • jablib/src/main/java/org/jabref/logic/ai/tokenization/logic/ByWordsTokenEstimator.java[31-34]
[reliability] `FileHasher` throws `RuntimeException`
`FileHasher` throws `RuntimeException` `FileHasher.computeHash(...)` logs `NoSuchAlgorithmException` but then throws a new `RuntimeException`, contradicting the method’s `Optional`-based error signaling and risking crashes in new ingestion flows. Prefer returning `Optional.empty()` (or another non-unchecked failure strategy) consistently.

Issue description

FileHasher.computeHash throws RuntimeException on NoSuchAlgorithmException, despite returning Optional<String>.

Issue Context

Compliance requires avoiding unchecked exceptions in new code where possible, and the method already models failure via Optional.empty().

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/ai/ingestion/util/FileHasher.java[41-46]
[correctness] RAG assert can crash
RAG assert can crash GenerateRagResponseTask.call() relies on an `assert` to guarantee a last USER message exists, then unconditionally calls `userMessage.get()`, which throws when the history contains no USER message (asserts are typically disabled in production). This can crash AI reply generation.

Issue description

GenerateRagResponseTask.call() uses assert userMessage.isPresent() and then calls userMessage.get(). In production JVMs assertions are usually disabled, so an empty chat history (or a history without USER messages) will throw NoSuchElementException and fail the task.

Issue Context

ChatHistoryUtils.getLastUserMessage(...) explicitly returns Optional.empty() when no USER message exists.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/ai/chatting/tasks/GenerateRagResponseTask.java[47-57]
  • jablib/src/main/java/org/jabref/logic/ai/chatting/util/ChatHistoryUtils.java[74-85]

Suggested change

Replace the assert with explicit handling, e.g.:

  • ChatMessage user = ChatHistoryUtils.getLastUserMessage(...).orElseThrow(() -> new IllegalStateException("No user message to answer"));
  • or return a ChatMessage.errorMessage(...) / fail the task with a user-friendly error, depending on your task/UI contract.
[maintainability] FXML texts end with colons
FXML texts end with colons New UI texts in FXML end with `:`, which violates JabRef’s UI wording/localization style and can look inconsistent in the UI. This should be rephrased to remove trailing colons.

Issue description

Several newly added FXML UI texts end with a trailing colon (:), which is disallowed by the UI label style rule.

Issue Context

The UI currently introduces strings like %Follow-up questions: and %Summarization algorithm:. JabRef requires labels/texts not to end with :.

Fix Focus Areas

  • jabgui/src/main/resources/org/jabref/gui/ai/chat/AiChat.fxml[80-80]
  • jabgui/src/main/resources/org/jabref/gui/ai/summary/AiSummaryParameters.fxml[15-15]
  • jabgui/src/main/resources/org/jabref/gui/ai/summary/AiSummary.fxml[26-26]
[reliability] `AiService` catches generic Exception
`AiService` catches generic Exception The new migration wrapper catches `Exception`, which is overly broad and can hide programming errors or unexpected runtime failures. Catch only the specific expected exception types (and/or handle them explicitly).

Issue description

AiService.migrateDatabase uses catch (Exception e), which is overly broad.

Issue Context

The compliance rule requires catching specific exceptions and logging them properly. While the logging call is correct, the catch type is too general and may swallow unexpected exceptions.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/ai/AiService.java[176-191]
[maintainability] Changelog entry is implementation-focused
Changelog entry is implementation-focused The CHANGELOG entry describes an internal refactoring rather than a user-visible change. Changelog entries must be end-user oriented.

Issue description

A new CHANGELOG entry is written from an implementation perspective (refactoring) instead of describing user impact.

Issue Context

Changelog entries should explain what changed for users, not how the code was changed internally.

Fix Focus Areas

  • CHANGELOG.md[39-39]
[correctness] Cancelled task becomes success
Cancelled task becomes success TrackedBackgroundTask.call() unconditionally sets status SUCCESS after perform() returns and sets ERROR for any exception, so a cancelled task can end up reported as SUCCESS/ERROR instead of CANCELLED. Code that relies on CANCELLED (e.g., summarization regenerate path) can misbehave and the UI can display the wrong task state.

Issue description

TrackedBackgroundTask.cancel() sets status to CANCELLED, but call() later overwrites status to SUCCESS after perform() completes, and overwrites to ERROR for any exception (including interruption/cancellation). This breaks logic that checks for CANCELLED and can show incorrect status in UI.

Issue Context

SummarizationTaskAggregator.start() checks task.getStatus() == CANCELLED to decide whether it may start a new regeneration task.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/ai/util/TrackedBackgroundTask.java[38-80]
  • jablib/src/main/java/org/jabref/logic/ai/summarization/SummarizationTaskAggregator.java[53-56]

Suggested fix

In call(), before setting SUCCESS, check isCancelled() / Thread.currentThread().isInterrupted() and keep/set CANCELLED. In the catch block, treat interruption/cancellation distinctly (set CANCELLED rather than ERROR) and avoid overwriting an already-cancelled status.

                     PR 15685 (2026-05-05)                    
[correctness] Autosave path must exist
Autosave path must exist When autosave is enabled, folder validation requires Files.exists(Path.of(input)) and the Connect button is gated by this validator, so selecting a new (not-yet-created) .bib file path via the Save dialog will keep Connect disabled. This contradicts the later SaveDatabaseAction.saveAs behavior, which can create a new file at the chosen path.

Issue description

When autosave is enabled, the folderValidator requires that the file path already exists, which blocks selecting a new save target via the Save dialog (common workflow). The code later saves using SaveDatabaseAction.saveAs(Path) which can create the file.

Issue Context

The Connect button is disabled based on formValidation().validProperty(). This PR added folderValidator to formValidator, so the existence check now blocks connecting.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java[137-152]

Suggested fix

Change the autosave path predicate to accept a non-empty, syntactically valid path where the parent directory exists and is writable (or at least exists), instead of requiring Files.exists(path). For example:

  • Parse with Path.of(input) inside a try/catch for InvalidPathException
  • Validate path.getParent() != null && Files.isDirectory(path.getParent())
  • Optionally validate write permission if desired
  • Keep Files.exists(path) only for the keystore validator (which really must exist).
[maintainability] Unrelated `.idea` code style edit
Unrelated `.idea` code style edit The PR includes an IDE code-style configuration change unrelated to the functional UI/validation fix, increasing diff noise and risking unintended formatting churn for other contributors. Compliance requires keeping diffs focused and avoiding unrelated formatting/config changes.

Issue description

An unrelated IntelliJ code style configuration line was added, creating non-functional diff noise.

Issue Context

This PR is about enabling/disabling fields in the shared database dialog; IDE formatting configuration changes are not required for that.

Fix Focus Areas

  • .idea/codeStyles/Project.xml[24-24]
                     PR 15678 (2026-05-04)                    
[correctness] Missing partial month tests
Missing partial month tests Month validation behavior was changed, but no regression tests were added to ensure mixed/partial values like `1abc` or `#jan# trailing` are rejected. This risks reintroducing the original bug without automated detection.

Issue description

MonthChecker behavior was tightened, but regression tests for the originally reported partial/mixed inputs are missing.

Issue Context

Compliance requires tests that fail for values that previously passed due to partial regex matching (e.g., 1abc, #jan# trailing).

Fix Focus Areas

  • jablib/src/test/java/org/jabref/logic/integrity/MonthCheckerTest.java[31-84]
[correctness] Rejects zero-padded months
Rejects zero-padded months In BibLaTeX mode, `MonthChecker` now rejects month integers with a leading zero (e.g., "01"/"02"/"09"), returning “should be an integer or normalized”. The codebase already parses and produces these values, so integrity checks/validation can start flagging existing entries as invalid after this change.

Issue description

MonthChecker's BibLaTeX integer validation regex only matches 1–12 without a leading zero. This causes values like "01", "02", "09" (which the codebase parses/produces) to be treated as invalid.

Issue Context

Month.parse explicitly supports “01 to 12”, and multiple tests construct entries with StandardField.MONTH set to zero-padded values.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/integrity/MonthChecker.java[16-20]

Suggested change

  • Update the integer regex to accept an optional leading zero for 1–9, e.g.:
  • ^(0?[1-9]|1[0-2])$
  • Add/extend unit tests (e.g., MonthCheckerTest) to cover:
  • Accept: 01, 02, 09, 10, 12
  • Reject: 00, 13, 01abc, #jan# trailing
[maintainability] Changelog uses internal classname
Changelog uses internal classname The new CHANGELOG entry references the internal class name `MonthChecker` and deviates from the surrounding end-user-focused phrasing style. This reduces readability and consistency for end users.

Issue description

CHANGELOG entry is a bit too implementation-specific and inconsistent in style.

Issue Context

Nearby entries use end-user phrasing (e.g., "We fixed...") and generally avoid internal class names.

Fix Focus Areas

  • CHANGELOG.md[107-107]
                     PR 15670 (2026-05-04)                    
[correctness] `@ValueSource` misused text block
`@ValueSource` misused text block The change replaces multi-value `@CsvSource` semantics with a single `@ValueSource(strings = """...""")` argument, likely collapsing multiple intended test cases into one string input. This weakens test coverage and can hide regressions.

Issue description

The parameterized test likely no longer iterates over the intended individual inputs because @ValueSource(strings = """...""") passes the entire text block as a single parameter value.

Issue Context

This appears to be an OpenRewrite-style conversion from @CsvSource to @ValueSource, but it changes test semantics and reduces coverage.

Fix Focus Areas

  • jablib/src/test/java/org/jabref/model/entry/AuthorTest.java[65-77]
[reliability] Unused imports break Checkstyle
Unused imports break Checkstyle Multiple files add `import java.util.function.Predicate;` but never reference `Predicate` (only the static `not(...)`), which violates the project’s Checkstyle `UnusedImports` rule and will fail CI.

Issue description

Several modified Java files import java.util.function.Predicate but never use the symbol (only the static import Predicate.not is used). This violates Checkstyle’s UnusedImports rule and fails CI.

Issue Context

The project enables Checkstyle UnusedImports, which treats unused imports as errors.

Fix Focus Areas

  • jabgui/src/main/java/org/jabref/gui/edit/CopyMoreAction.java[4-8]
  • jablib/src/main/java/org/jabref/logic/importer/fetcher/LOBIDFetcher.java[8-15]
  • jablib/src/main/java/org/jabref/logic/integrity/BooktitleContainsCountryChecker.java[3-16]
  • jablib/src/main/java/org/jabref/logic/journals/ltwa/LtwaTsvParser.java[9-17]
  • jablib/src/main/java/org/jabref/logic/layout/format/Authors.java[3-14]
  • jablib/src/main/java/org/jabref/logic/layout/format/DocBookAuthorFormatter.java[1-9]
  • jablib/src/main/java/org/jabref/logic/push/AbstractPushToApplication.java[7-25]
  • jabsrv/src/main/java/org/jabref/http/server/cayw/CitationProperties.java[2-10]

Notes

Keep import static java.util.function.Predicate.not; (since not(...) is used); remove the non-static import java.util.function.Predicate; where it’s unused.

                     PR 15669 (2026-05-03)                    
[correctness] Removed l10n key translation
Removed l10n key translation The key "Could not call executable" is still used (e.g., in AbstractPushToApplication), but its translations were removed from many non-English bundles (e.g., JabRef_de.properties), so users will see the raw key text instead of a translated message. This regression is silent because the localization bundle returns the key itself for missing entries and does not log the missing translation.

Issue description

The localization key Could not call executable is still used in code (Localization.lang("Could not call executable")), but the corresponding properties entry was removed from many non-English localization files (e.g., JabRef_de.properties). This causes non-English locales to show the raw key text (English) instead of a translation.

Issue Context

Localization loads ResourceBundle.getBundle("l10n/JabRef", locale). For missing keys in a locale file, LocalizationBundle.handleGetObject returns the key itself, and containsKey() always returns true, so the issue becomes a silent UX regression.

Fix Focus Areas

  • jablib/src/main/resources/l10n/JabRef_de.properties[121-131]
  • jablib/src/main/java/org/jabref/logic/push/AbstractPushToApplication.java[95-105]
  • jablib/src/main/resources/l10n/JabRef_en.properties[128-134]

Expected fix

Re-add the properties entry Could\ not\ call\ executable=... to each affected non-English JabRef_<lang>.properties file (ideally restoring the previous translations). If restoring per-locale translations is not possible in this PR, add at least an English placeholder value to prevent a missing-entry fallback, and ensure Crowdin will later overwrite it with proper translations.

                     PR 15666 (2026-05-03)                    
[maintainability] Malformed `PlainCitationParser` link
Malformed `PlainCitationParser` link The added Markdown documentation comment uses `[@org.jabref.logic.importer.plaincitation.PlainCitationParser]`, which is not the project’s established doc-linking style and likely won’t render as a proper reference. This violates JabRef’s code style conventions for documentation comments and reduces readability.

Issue description

A new /// documentation comment uses [@org.jabref.logic.importer.plaincitation.PlainCitationParser], which is not the established/recognized doc-link format and may not render as a link.

Issue Context

The codebase uses /// documentation comments together with {@link ...} references elsewhere; keeping the same style improves readability and tooling support.

Fix Focus Areas

  • jablib/src/main/java/org/jabref/logic/importer/fetcher/citation/CitationFetcherType.java[35-35]
                     PR 15664 (2026-05-03)                    
[reliability] Brittle patch output path
Brittle patch output path With `:rewriteDryRun` now expected to fail when rewrites are pending, the `Output diff` step will execute more often but uses a hard-coded absolute path (`/home/runner/work/jabref/jabref/...`) that can be wrong when the checkout directory differs, producing an unhelpful "file not found" error instead of the rewrite patch output.

Issue description

The OpenRewrite failure diagnostics uses a hard-coded absolute path (/home/runner/work/jabref/jabref/...) to print rewrite.patch. Now that :rewriteDryRun is intended to fail when it finds results, this step will run more often and can fail with No such file or directory when the checkout directory differs.

Issue Context

This repository’s Gradle config sets failOnDryRunResults = true, so :rewriteDryRun is expected to fail on detected rewrite results and trigger the if: failure() step.

Fix Focus Areas

  • .github/workflows/tests-code.yml[72-90]

Suggested change

Use ${{ github.workspace }} (or a relative path) and optionally guard with test -f:

Clone this wiki locally