fix(settings): derive randomVersionString from release identity

[JohnMcLear]

Summary

Fixes #7213 — horizontally-scaled Etherpad deployments (ingress → service → N pods) were 404'ing on padbootstrap-<hash>.min.js because every pod generated a different hash for the same build.

Root cause

src/node/hooks/express/specialpages.ts:319-326 names the production bootstrap bundles after their esbuild content-hash:

const padSliderWrite = convertTypescript(padString);
fileNamePad = `padbootstrap-${padSliderWrite.hash}.min.js`;

The content fed to esbuild comes from src/templates/padBootstrap.js, which embeds settings.randomVersionString as a field on window.clientVars. That value was generated in src/node/utils/Settings.ts:1092:

settings.randomVersionString = randomString(4);

Regenerated on every boot → pods of the same build disagreed on the bundle filename. A client that loaded HTML from pod A asking for padbootstrap-ABCD.min.js hit pod B for the follow-up fetch and got a 404 (exactly the scenario described in the issue).

Fix

Make randomVersionString deterministic:

ETHERPAD_VERSION_STRING env  →  verbatim (integrator override)
else                         →  sha256(version + "|" + gitVersion)[:8]

• Pods of the same built artifact now produce identical hashes, so cross-pod requests work.
• The token still rotates on release (version or git SHA change), so clients' HTTP caches invalidate correctly.
• Integrators who want to tie the token to their own deploy ID can set ETHERPAD_VERSION_STRING explicitly.

Backwards-compatible: single-pod deployments see the same effective behavior — cache-bust on release.

Test plan

• Added backend regression tests in src/tests/backend/specs/settings.ts:
  • Default token is an 8-hex-char sha256 prefix.
  • ETHERPAD_VERSION_STRING env override is respected verbatim.
• pnpm run ts-check clean locally.
• Manual: start two pods of the same build; confirm /padbootstrap-*.min.js filenames match.
• Manual: start two pods of different builds (bump version); confirm filenames diverge.

Closes #7213

🤖 Generated with Claude Code

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

Review Summary by Qodo

Derive randomVersionString from release identity for multi-pod deployments

🐞 Bug fix

Walkthroughs

Description

• Make randomVersionString deterministic from version/git SHA
• Fixes 404s in horizontally-scaled deployments with multiple pods
• Respects ETHERPAD_VERSION_STRING env var for explicit override
• Added regression tests for version string stability

Diagram

flowchart LR
  A["ETHERPAD_VERSION_STRING env"] -->|explicit override| B["randomVersionString"]
  C["version + gitVersion"] -->|sha256 hash| D["8-char hex prefix"]
  D --> B
  B --> E["padbootstrap-hash.min.js filename"]
  E --> F["Stable across pods"]

Loading

File Changes

1. src/node/utils/Settings.ts 🐞 Bug fix +30/-9

Implement deterministic version string from release identity

• Replaced random string generation with deterministic hash of version and git SHA
• Added support for ETHERPAD_VERSION_STRING environment variable override
• Updated comments to explain cache-busting strategy and multi-pod issue
• Changed log message from "Random string" to "String" for accuracy

src/node/utils/Settings.ts

---

2. src/tests/backend/specs/settings.ts 🧪 Tests +35/-0

Add regression tests for version string determinism

• Added regression test suite for issue #7213 covering randomVersionString stability
• Test validates default token is 8-character hex string from sha256 hash
• Test verifies ETHERPAD_VERSION_STRING environment variable override works correctly
• Tests ensure multi-pod deployments produce consistent filenames

src/tests/backend/specs/settings.ts

---

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

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0)

1. 4-space indent in reloadSettings() 📘 Rule violation ⚙ Maintainability

Description

Newly added code in reloadSettings() uses 4+ space indentation (and deeper levels), which violates
the requirement to indent with 2 spaces only. This reduces formatting consistency and can cause
style/lint failures in environments enforcing the rule.

Code

src/node/utils/Settings.ts[R1102-1112]

+    const explicit = process.env.ETHERPAD_VERSION_STRING;
+    if (explicit) {
+        settings.randomVersionString = explicit;
+    } else {
+        const pkgVersion = require('../../package.json').version as string;
+        settings.randomVersionString = createHash('sha256')
+            .update(`${pkgVersion}|${settings.gitVersion || ''}`)
+            .digest('hex')
+            .slice(0, 8);
+    }
+    logger.info(`String used for versioning assets: ${settings.randomVersionString}`);

Evidence

PR Compliance ID 8 requires that all modified/added code use 2-space indentation. The added block
that sets settings.randomVersionString is indented with 4 spaces at the block level and 8+ spaces
on nested lines.

src/node/utils/Settings.ts[1102-1112]
Best Practice: Repository guidelines

Agent prompt

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

## Issue description
Newly added code uses indentation other than 2 spaces, violating the repo compliance formatting rule.
## Issue Context
The `randomVersionString` derivation block added in `reloadSettings()` is indented with 4+ spaces.
## Fix Focus Areas
- src/node/utils/Settings.ts[1102-1112]

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

---

2. Override test never reloads ☑ 🐞 Bug ≡ Correctness

Description

The new test sets ETHERPAD_VERSION_STRING but only calls exportedForTestingOnly.parseSettings(),
which does not recompute settings.randomVersionString; randomVersionString is computed in
reloadSettings(). Because the module is already loaded earlier in this test file via require(), the
later require() returns the cached instance and the assertion can fail.

Code

src/tests/backend/specs/settings.ts[R165-178]

+    it('honours ETHERPAD_VERSION_STRING as an explicit override', function () {
+      const {exportedForTestingOnly} = require('../../../node/utils/Settings');
+      const original = process.env.ETHERPAD_VERSION_STRING;
+      process.env.ETHERPAD_VERSION_STRING = 'integrator-1';
+      try {
+        const parsed =
+            exportedForTestingOnly.parseSettings(path.join(__dirname, 'settings.json'), true);
+        // parseSettings returns the parsed JSON, not the mutated module-scope
+        // settings object. The override lives on the singleton, which
+        // parseSettings updates as a side effect — require the module again
+        // via cjs so we pick up the current state.
+        const cjs = require('../../../node/utils/Settings');
+        assert.strictEqual(cjs.randomVersionString, 'integrator-1',
+            'ETHERPAD_VERSION_STRING should be used verbatim');

Evidence

In Settings.ts, parseSettings() only reads/parses JSON and returns an object; it does not mutate the
module-scope settings singleton. randomVersionString is set only inside reloadSettings(). In the
test file, the Settings module is required earlier (CJS compatibility tests), so the later require()
in the override test will return the already-initialized module instance without re-running
reloadSettings(), leaving randomVersionString unchanged despite the env var update.

src/node/utils/Settings.ts[85-132]
src/node/utils/Settings.ts[943-947]
src/node/utils/Settings.ts[1081-1113]
src/tests/backend/specs/settings.ts[105-149]
src/tests/backend/specs/settings.ts[165-178]

Agent prompt

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

## Issue description
The test `honours ETHERPAD_VERSION_STRING as an explicit override` sets `process.env.ETHERPAD_VERSION_STRING` but never calls `reloadSettings()`, so `settings.randomVersionString` is not recomputed. The test currently calls `exportedForTestingOnly.parseSettings()` which only parses JSON and does not mutate the module-scope singleton.
### Issue Context
- `randomVersionString` is derived inside `reloadSettings()`.
- The Settings module has already been loaded earlier in the file via `require()`, so requiring it again will not re-run module initialization.
### Fix Focus Areas
- src/tests/backend/specs/settings.ts[165-183]
### Suggested change
Update the override test to:
1) Save the original env var.
2) Set `process.env.ETHERPAD_VERSION_STRING`.
3) Call `require('../../../node/utils/Settings').reloadSettings()`.
4) Assert `randomVersionString`.
5) Restore the env var and call `reloadSettings()` again to avoid leaking state to other tests.
(Optionally also ensure the default-hash test unsets `ETHERPAD_VERSION_STRING` and calls `reloadSettings()` before asserting.)

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

---

3. ETHERPAD_VERSION_STRING undocumented override 📘 Rule violation ⚙ Maintainability

Description

A new public configuration surface (ETHERPAD_VERSION_STRING) is introduced but no corresponding
documentation update is included. This makes the new override hard to discover for integrators and
operators.

Code

src/node/utils/Settings.ts[R1097-1105]

+     * Precedence: ETHERPAD_VERSION_STRING env var (explicit integrator
+     * override) > sha256(version + "|" + gitVersion) > package.json version.
+     *
+     * For the original cache-busting rationale, see PR #3958.
    */
-    settings.randomVersionString = randomString(4);
-    logger.info(`Random string used for versioning assets: ${settings.randomVersionString}`);
+    const explicit = process.env.ETHERPAD_VERSION_STRING;
+    if (explicit) {
+        settings.randomVersionString = explicit;
+    } else {

Evidence

PR Compliance ID 7 requires documentation updates in the same PR when changing configuration/public
integration surfaces. The code introduces process.env.ETHERPAD_VERSION_STRING as an explicit
override, but the docs (for example the Docker env-var reference section) do not mention this new
variable.

src/node/utils/Settings.ts[1097-1111]
doc/docker.md[64-79]
Best Practice: Repository guidelines

Agent prompt

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

## Issue description
A new environment-variable override (`ETHERPAD_VERSION_STRING`) was added, but documentation was not updated in this PR.
## Issue Context
Operators running clustered deployments need to know how to set this override deterministically.
## Fix Focus Areas
- src/node/utils/Settings.ts[1097-1111]
- doc/docker.md[20-90]

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

---

ⓘ The new review experience is currently in Beta. Learn more

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

1. 4-space indent in reloadsettings() 📘 Rule violation ⚙ Maintainability

Newly added code in reloadSettings() uses 4+ space indentation (and deeper levels), which violates
the requirement to indent with 2 spaces only. This reduces formatting consistency and can cause
style/lint failures in environments enforcing the rule.

Agent Prompt

## Issue description
Newly added code uses indentation other than 2 spaces, violating the repo compliance formatting rule.

## Issue Context
The `randomVersionString` derivation block added in `reloadSettings()` is indented with 4+ spaces.

## Fix Focus Areas
- src/node/utils/Settings.ts[1102-1112]

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

[JohnMcLear]

Qodo round-2 on this PR:

1. Override test never reloads — already addressed on current HEAD. settings.ts override test calls settingsMod.reloadSettings() at line 177 and restores the saved token/paths in finally. CI is green.
2. 4-space indent in reloadSettings() — the surrounding reloadSettings function already uses 4-space indent throughout (see e.g. lines 1030-1050 pre-existing). Matching file style is the intent; converting just the new block to 2-space would break local consistency and not match how lint is actually enforced in this file.