[Port to dtq-dev] Issue dspace-customers#903: make REST root version prefix configurable

[jr-rk]

Problem

The version string exposed at the REST root (GET /server/api → dspaceVersion, rendered by the UI as <meta name="Generator">) is hardcoded to the "DSpace " prefix in RootConverter. CLARIN deployments need the "CLARIN-DSpace" identity, which was introduced by #985 and then accidentally dropped by the #1031 7.6.5 upgrade merge. Hardcoding it back would mis-brand the nine non-customer branches that should read "DSpace".

Port of dataquest-dev/dspace-customers#903 (item 6). References customer/lindat 563ef4f366/0fea1de43c for where the CLARIN prefix lived; reimplemented as a configurable property rather than a hardcode.

Root cause

RootConverter.convert() built the version string as the literal "DSpace " + getSourceVersion(), so the prefix could only be changed by forking the converter per deployment — which is exactly what caused the identity to be lost across a merge.

Change set

• RootConverter.java — read the prefix from a new dspace.version.prefix property, defaulting to "DSpace", with a blank-value guard: StringUtils.defaultIfBlank(configurationService.getProperty("dspace.version.prefix", "DSpace"), "DSpace"). getProperty(key, default) only returns the default when the key is absent, so the defaultIfBlank wrapper also collapses a present-but-empty value back to "DSpace" (avoids a malformed leading-space version string). Behaviour is byte-for-byte unchanged unless a deployment sets a non-blank prefix.
• dspace.cfg — document the new property (commented-out default) next to dspace.name.
• RootConverterTest.java — stub the new property in setUp (existing assertions unchanged) and add testConfigurableVersionPrefix, asserting a "CLARIN-DSpace" override.

Scope: this PR makes only the REST-root generator string configurable (item 6's target). Other DSpace-branded surfaces (AIP/METS agent name, OpenAIRE User-Agent) remain hardcoded and are out of scope. The dtq-dev HttpServletRequest/dspace.server.ssr.url SSR-URL logic in RootConverter is untouched.

Test evidence

$ mvn -pl dspace-server-webapp test -Dtest=RootConverterTest -DskipUnitTests=false
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0 — in org.dspace.app.rest.converter.RootConverterTest
BUILD SUCCESS
  (5 = 4 existing REST-root tests, still green with the default "DSpace" prefix,
   + new testConfigurableVersionPrefix asserting a "CLARIN-DSpace" override)

$ mvn -pl dspace-server-webapp checkstyle:check
You have 0 Checkstyle violations.
BUILD SUCCESS

Note: the blank/absent fallback path is not yet covered by a dedicated test (the tests mock the 2-arg getProperty overload directly).

Risk & rollback

Minimal. Default path is unchanged ("DSpace"). Revert = single commit revert; the config property is inert when unset.

Notes / assumptions

Property name dspace.version.prefix chosen to sit alongside dspace.name/dspace.shortname. CLARIN branches opt in via local.cfg (dspace.version.prefix = CLARIN-DSpace); no code fork required going forward.

Follow-up (not in this PR): customer/lindat still hardcodes "CLARIN-DSpace " + getSourceVersion() in its RootConverter and sets dspace.version.prefix nowhere. Until it is migrated to consume the new property (drop the hardcode + set the value in cfg), this PR restores the CLARIN identity on zero deployments and the next dtq-dev → lindat merge will conflict on the same line again. That migration should be tracked as a separate task under dataquest-dev/dspace-customers#903.

[jr-rk]

dspace-skills improvements suggestions

One efficiency pass over the skills, helper scripts, and reference facts, ranked by how often the bottleneck is hit times what it costs each time; the recurring theme is redundant git fetch/Maven/gh work in the fan-out paths and a handful of unconfirmed facts that drive wrong-variant backports.

High impact

1. audit-coverage.sh re-fetches every remote on every call — and the security audit calls it per-patch.
skills/shared/audit-backport-coverage/scripts/audit-coverage.sh:46 runs git fetch --all --prune unconditionally. audit-upstream-security (SKILL step 6) invokes it "one run per patch per repo," and --all drags in the huge upstream (DSpace/DSpace) remote that step 1 already fetched. A 10-patch × 2-repo audit = ~20 full fetches of a large repo.
Fix: add a shared ensure_fetched() in skills/shared/scripts/git-refs.sh that fetches origin once per process-tree via a TTL stamp file in .git/ (e.g. skip if fetched < 300s ago), honor an AUDIT_SKIP_FETCH=1 opt-out, and fetch origin not --all (upstream is already current from step 1).
Payoff: cuts the dominant network cost of a security audit from ~20 large fetches to 1-2; minutes → seconds per re-run.

2. Backend verify.sh falls back to a full-reactor mvn package when the module is knowable from the cherry-pick.
skills/backend/be-backport/scripts/verify.sh:20: with no [module] arg it runs mvn package — the entire reactor (~15+ modules) plus all tests, on a repo where that is 20-40 min. But be-backport just cherry-picked a commit, so the touched module is trivially derivable.
Fix: in verify.sh, when $module is empty, derive it from git -C "$wt" show --name-only --format= HEAD (top-level dir → module), then run mvn checkstyle:check -pl <mods> + mvn test -pl <mods> -am; keep bare mvn package only as an explicit last resort.
Payoff: removes a 20-40 min footgun on every BE branch where the agent didn't pass the module; makes the fast path the default.

3. The facts that decide how to backport are still marked unconfirmed.
The 8 theme: {verify: true} in customers/registry.yml are cosmetic, but four fe_backport_class: {verify: true} (sav :64, vsb-tuo :74, zcu-data :104, jcu :114) directly drive fe-backport's decision tree — an unconfirmed class means applying the wrong file set (FULL vs overlay-only vs hydration) to a live customer PR. Separately, customer/jcu is flagged "confirm/push" (:116-122), so a jcu backport can fail at setup-worktree on a missing base branch; and reference/backend-stack.md:25-33,67 leaves the whole BE command block "confirm current" with JDK/Maven versions unpinned, so every BE task re-derives them and risks a wrong-JDK build failure.
Fix: one confirmation pass — resolve the 4 fe_backport_class values from the live branches, confirm/push customer/jcu (or mark n/a), and pin exact JDK/Maven + validation order from pom.xml/CI into backend-stack.md; drop each marker as it is verified.
Payoff: eliminates the highest-consequence errors (wrong-variant PR, hard setup failure, build-env mismatch); these are hit on every fan-out and upstream/BE task.

4. A fan-out has no resumable state — it lives only in agent context and worktrees.
fe-backport/be-backport run 8 sequential heavy builds (FE ~8 GB each). The coverage matrix and per-branch readiness exist only in the conversation; the skills even admit state is reconstructed from git worktree list (fe-backport step 7). A dropped session mid-fan-out loses the matrix and forces re-recon + re-build.
Fix: have setup-worktree/verify/open-pr append a per-branch ledger row to $parent/.dspace-backport-state.tsv (branch, base, worktree, verified, pr-url); the coordinator reads it to resume at the next unverified branch and to print the matrix without re-probing.
Payoff: a resumed session skips already-verified branches instead of repeating multi-minute builds.

Medium impact

5. setup-worktree.sh re-fetches per branch; recon.sh reads possibly-stale refs.
skills/shared/scripts/setup-worktree.sh:43 runs git fetch origin --prune on every worktree creation (8× in a fan-out), while recon.sh (step 1, run before any setup) never fetches and classifies off whatever origin/customer/* happens to be local.
Fix: route both through the ensure_fetched() helper from #1 (recon fetches once up front; setup skips within the TTL).
Payoff: one fetch per fan-out instead of nine; recon can't misclassify on stale refs.

6. The front-door and link scripts make duplicate gh/API round-trips.
skills/shared/triage-issue/scripts/classify-issue.sh:43-51 defines gh_field() and calls it three times (plus a fourth access-check at :47) — four gh issue view fetches of the same issue. link-prs-to-issue.sh:88 and :94 paginate the full issue-comments list twice to get an id and then a body.
Fix: fetch once into a variable and extract fields locally (classify-issue.sh: one --json title,body,labels; link-prs-to-issue.sh: one --paginate capture, pull id+body from it).
Payoff: ~3× fewer API calls on the most frequently-run skill (triage); less rate-limit exposure.

7. recon.sh resolves each branch's ref seven times.
The classification loop resolves once (skills/frontend/fe-backport/scripts/recon.sh:32), then the blob-hash matrix re-calls resolve_customer_ref inside the inner loop (:64) for each of 6 files × each branch. resolve_customer_ref shells out to the awk registry parser (twice) + several git rev-parse each time — subprocess-heavy on Windows Git Bash where fork is slow (~8 branches × 6 = 48 redundant resolves).
Fix: resolve each branch once into an array/temp map and reuse it in the matrix loop.
Payoff: removes dozens of slow process spawns per recon.

8. harvest-upstream.sh fires one search per keyword.
skills/shared/audit-upstream-security/scripts/harvest-upstream.sh:71-74 loops the 13 strong keywords into 13 separate gh pr list --search calls (+ label pass) per repo — ~28 search calls across both repos, each pulling up to 200 PRs, against GitHub's ~30/min secondary search limit.
Fix: batch keywords into a few OR queries (--search "XSS OR CSRF OR SSRF OR injection OR … merged:>=$since"), keep the label:security pass separate.
Payoff: ~28 → ~6 calls; avoids secondary-rate-limit stalls, faster harvest.

9. audit-upstream-security omits the accidental-push guard its sibling has.
assess-upstream-contribution/SKILL.md:68 hardens the upstream remote with git remote set-url --push upstream DISABLED; the identical remote-setup in audit-upstream-security/SKILL.md (step 1, lines ~52-57) does not, despite both being strictly read-only on DSpace/*.
Fix: add the same set-url --push … DISABLED line to audit-upstream-security step 1.
Payoff: closes an inconsistent-safety gap on the skill that touches upstream most.

Low impact

• Patch-id work is recomputed per branch. audit-coverage.sh:54 computes the target commit's patch-id inside verdict_sha, which runs per branch; hoist tpid out of the loop. The inner equivalence loop (:57-61) can compute up to AUDIT_LOOKBACK_COMMITS (400) patch-ids per branch — already file-scoped, but document the cost / consider a lower default.
• gawk-only constructs + hand-rolled YAML. git-refs.sh:66 and classify-issue.sh:113 use match(str, re, arr) (a GNU-awk extension) and a whitespace-sensitive awk parser for registry.yml. Fine on Git Bash (gawk) but breaks on BSD awk and if the registry's - name:-first formatting drifts; add a gawk guard or a one-line note pinning the assumption.
• Doc drift. backend-stack.md:27 leads with mvn package while profiles/backend.md:18 says mvn -DskipTests install for the compile step; and the upstream-remote-add snippet is duplicated verbatim across the two upstream skills. Reconcile to one canonical command and factor the snippet into a shared reference.

Coverage gaps (workflows with no skill)

• review-expert agent. green-light.md:15-16 makes "review-expert run + findings addressed" a merge-gate item, then admits it is "on the roadmap but not yet authored." Every backport PR currently clears this gate manually. Highest-value missing piece — it is already wired into the contract (backport-model.md:25, flow.html).
• verify-registry skill. The registry carries 12 verify: true markers and the README calls accuracy "a guardrail" ("a wrong registry silently misroutes a security backport"), yet there is no skill to reconcile it against live branches (git branch -r on both origins, confirm pairing/versions, drop markers). Cheap to build, directly retires findings Revert "test commit" #3.
• 7.x → 9.x forward-port skill. assess-upstream-contribution and the dtq-dev-9-base work reference recurring jakarta/Hibernate-6/Angular-standalone porting with no skill to drive it.

Quick wins (do these first)

1. Shared ensure_fetched() helper in git-refs.sh — retires findings Test issue #1 and Testtests #5 (biggest network savings, small change).
2. Auto-derive the module in BE verify.sh:20 — kills the full-reactor fallback (test commit #2).
3. One gh fetch in classify-issue.sh (and single paginate in link-prs-to-issue.sh) — CICD pipeline #6, trivial.
4. Add set-url --push … DISABLED to audit-upstream-security step 1 — Auto assign issue to projects #9, a one-line safety fix.
5. Confirm the 4 fe_backport_class markers + customer/jcu existence — the subset of Revert "test commit" #3 that prevents wrong-variant PRs and hard setup failures.

---

_{Posted from the issue dataquest-dev/dspace-customers#903 port session — a background agent audited the dspace-skills repo in parallel with the backend backports.}

[Copilot]

Pull request overview

This PR makes the REST-root dspaceVersion prefix configurable via a new dspace.version.prefix property, allowing CLARIN/customer deployments to brand the version string without forking RootConverter, while keeping the default behavior unchanged for standard deployments.

Changes:

• Update RootConverter to read the version prefix from configuration (default: "DSpace").
• Document the new dspace.version.prefix property in dspace.cfg.
• Extend RootConverterTest with a new test verifying a "CLARIN-DSpace" override.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
dspace/config/dspace.cfg	Documents the new dspace.version.prefix configuration option and its default behavior.
dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/RootConverter.java	Builds the REST-root version string using a configurable prefix instead of a hardcoded literal.
dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/RootConverterTest.java	Stubs the new config property and adds coverage for a custom prefix override.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jr-rk]

Addressed the Copilot review: dspace.version.prefix now falls back to the "DSpace" default when set to a blank value (commit 6fe6678). RootConverterTest green (5/5).

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/RootConverter.java:49

• "DSpace" is duplicated as both the configuration default and the blank fallback. Using a single local constant avoids accidental drift if the default ever changes and makes the intent clearer.

        String versionPrefix = StringUtils.defaultIfBlank(
                configurationService.getProperty("dspace.version.prefix", "DSpace"), "DSpace");
        rootRest.setDspaceVersion(versionPrefix + " " + getSourceVersion());

dspace/config/dspace.cfg:48

• The comment says the property defaults when "unset", but the implementation also falls back when the property is present but blank. Updating the wording will match the actual behavior and help operators avoid confusion.

# Prefix for the version string exposed at the REST root (/server/api), rendered by the UI as
# the HTML <meta name="Generator"> value. Defaults to "DSpace" when unset; CLARIN/customer
# deployments may set e.g. "CLARIN-DSpace".
# dspace.version.prefix = DSpace

dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/RootConverterTest.java:100

• The new blank-value guard (defaultIfBlank) is a key behavior change but isn’t directly covered by a test (current tests only cover default + non-blank override). Add a unit test asserting that an empty configured prefix falls back to the default "DSpace" prefix.

    @Test
    public void testConfigurableVersionPrefix() throws Exception {
        when(configurationService.getProperty("dspace.version.prefix", "DSpace")).thenReturn("CLARIN-DSpace");
        request.setScheme("https");
        request.setServerName("dspace-rest");
        request.setServerPort(443);
        request.setRequestURI("/server/api");
        RootRest rootRest = rootConverter.convert(request);
        assertEquals("CLARIN-DSpace " + Util.getSourceVersion(), rootRest.getDspaceVersion());
    }

[milanmajchrak]

Change it to "CLARIN-DSpace" as it is here: https://github.com/dataquest-dev/DSpace/pull/985/changes#diff-755dfa87d0fb57ba5cca52d0544119a7425a1669150dee2bcf60f7ccebd0c032R39