fix(ci): restore sweep integration test against hardened core (cli-client 4.0.3 + rate-limit)

[JohnMcLear]

Problem

Backend-tests CI has been red on every PR since ~2026-05-19; the last green run was #111 on 2026-05-16. The Run sweep integration test step fails with:

AssertionError: expected 0 to be greater than 0
 ❯ tests/sim/integration.test.ts:35:35   // expect(s.latencyMs.count).toBeGreaterThan(0)

This surfaced while looking at #121 (the actions/checkout 4→7 bump) — that bump is fine; its checkout steps pass. The red check is an unrelated, pre-existing, repo-wide failure.

Root cause (not in this repo)

ether/etherpad#7773 — "harden: reject USER_CHANGES inserts without an author attribute" — merged 2026-05-16 17:23, immediately after the last green run. It tightened server-side changeset validation (reject inserts with no author attribute; reject changesets that strand the trailing \n) and bumped its own etherpad-cli-client dependency to 4.0.3 to comply.

This repo was still pinned to etherpad-cli-client 4.0.2, whose append() splices at text.length (after the trailing \n) with an empty apool and no author attribute — exactly the two shapes #7773 now rejects as badChangeset. Result: zero ACCEPT_COMMITs, so every sweep step records latencyMs.count === 0 and the test fails.

Fix (two parts, both verified against a local core build at develop HEAD)

1. Bump etherpad-cli-client 4.0.2 → 4.0.3 — sends author-attributed inserts and preserves the trailing newline.
2. Raise commitRateLimiting on the SUT in the sweep step. The sweep drives 2..4 authors from a single IP; core's default 10 changes/s/IP rate-limits the 4-author step (~20/s), and a rate-limited USER_CHANGES never gets an ACCEPT_COMMIT, stalling the 4.0.3 client's in-flight slot → count=0, errors=4. Mirrors the importExportRateLimiting bump runnerLoadTest.sh already applies.

Verification

Built ether/etherpad at develop HEAD (includes #7773) and ran the sweep against it:

client	rate limit	step=2	step=4
4.0.2	default	count=0 ❌	count=0 ❌
4.0.3	default (10/s)	count=20 ✅	count=0, errors=4 ❌
4.0.3	raised	count=20 ✅	count=40 ✅

pnpm test:integration passes end-to-end with both changes; pnpm test (48 tests) green.

Follow-up (separate)

etherpad-cli-client 4.0.3 permanently stalls its in-flight slot when a USER_CHANGES is rate-limited (no ACCEPT_COMMIT ever clears it). Out of scope here, but worth hardening in that client.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

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

PR Summary by Qodo

fix(ci): restore sweep integration test on hardened Etherpad core
🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Description

• Bump etherpad-cli-client to 4.0.3 to satisfy hardened USER_CHANGES validation.
• Raise Etherpad commitRateLimiting in CI sweep step to avoid rate-limit stalls.
• Update GitHub Actions workflows to use actions/checkout@v7.

Diagram

graph TD
  A["GitHub Actions"] --> B["backend-tests.yml"] --> C["Start Etherpad core"] --> D["settings.json"] --> E{Rate limit hit?}
  B --> F["Sweep integration test"] --> G["etherpad-cli-client 4.0.3"] --> C
  D --> E -->|No| F
  E -->|Yes| H["No ACCEPT_COMMIT"]
  subgraph Legend
    direction LR
    _ci["CI workflow"] ~~~ _cfg["Config file"] ~~~ _dec{"Decision"}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Throttle the sweep to stay under default commitRateLimiting

• ➕ Keeps SUT closer to default production settings
• ➕ Avoids mutating settings.json during CI
• ➖ Changes the test’s load profile and reduces coverage of multi-author throughput
• ➖ May still be sensitive to timing variance and intermittent limiter hits

2. Disable rate limiting only for loopback/CI environment via config overlay

• ➕ More explicit than an inline sed; easier to audit and evolve
• ➕ Can be reused by other CI jobs that boot core
• ➖ Requires maintaining an extra CI-only config artifact/patching mechanism
• ➖ Slightly more repo complexity than a one-liner sed

3. Pin Etherpad core ref used by the sweep to a known-compatible commit

• ➕ Fastest way to get green without changing limits or client
• ➕ Stabilizes CI against upstream changes
• ➖ Doesn’t validate against current core behavior; delays discovery of breaking changes
• ➖ Accrues drift and future catch-up cost

Recommendation: Keep the PR’s approach. Updating etherpad-cli-client to 4.0.3 aligns the test client with hardened core validation, and raising commitRateLimiting in the sweep step preserves the intended throughput-oriented test behavior. Alternatives either weaken the load coverage (throttling) or trade short-term green builds for long-term drift (pinning core).

Files changed (7) +23 / -15

Bug fix (1) +10 / -2

backend-tests.ymlRaise SUT commit rate limit and bump checkout action +10/-2

Raise SUT commit rate limit and bump checkout action

• Updates actions/checkout usage to v7. Adds an inline settings.json edit to significantly raise Etherpad commitRateLimiting during the sweep integration test so multi-author steps don’t stall on rate limiting.

.github/workflows/backend-tests.yml

Other (6) +13 / -13

codeql.ymlBump actions/checkout to v7 for CodeQL workflow +1/-1

Bump actions/checkout to v7 for CodeQL workflow

• Updates the CodeQL workflow to use actions/checkout@v7.

.github/workflows/codeql.yml

cpu-profile.ymlBump actions/checkout to v7 for CPU profile workflow +2/-2

Bump actions/checkout to v7 for CPU profile workflow

• Updates both repository checkouts in the CPU profile workflow to actions/checkout@v7.

.github/workflows/cpu-profile.yml

npmpublish.ymlBump actions/checkout to v7 for npm publish workflow +2/-2

Bump actions/checkout to v7 for npm publish workflow

• Updates the test and publish jobs to use actions/checkout@v7.

.github/workflows/npmpublish.yml

scaling-dive.ymlBump actions/checkout to v7 for scaling dive workflow +2/-2

Bump actions/checkout to v7 for scaling dive workflow

• Updates both repository checkouts in the scaling dive workflow to actions/checkout@v7.

.github/workflows/scaling-dive.yml

package.jsonBump etherpad-cli-client to 4.0.3 +1/-1

Bump etherpad-cli-client to 4.0.3

• Updates the etherpad-cli-client dependency from ^4.0.2 to ^4.0.3 to remain compatible with hardened Etherpad core changeset validation.

package.json

pnpm-lock.yamlLockfile update for etherpad-cli-client 4.0.3 +5/-5

Lockfile update for etherpad-cli-client 4.0.3

• Updates pnpm lockfile entries to resolve etherpad-cli-client at 4.0.3, including integrity metadata and snapshot keys.

pnpm-lock.yaml

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

Code Review by Qodo

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

1. Greedy sed corrupts JSONC ✓ Resolved 🐞 Bug ☼ Reliability

Description

The backend sweep step edits settings.json with s/"points":.*/"points": 1000000/, which deletes
the trailing comma and any inline comments after the points value. This can produce invalid
JSON/JSONC (and prevent Etherpad from starting) if commitRateLimiting has additional keys after
points, unlike the safer comma-preserving patterns used in other workflows here.

Code

.github/workflows/backend-tests.yml[88]

+          sed -e '/^ *"commitRateLimiting":/,/^ *\}/ s/"points":.*/"points": 1000000/' -i settings.json

Evidence

backend-tests.yml uses a greedy .* replacement that can delete commas/comments on the points line,
while scaling-dive.yml and cpu-profile.yml in this same repo use a non-greedy [^,]* pattern to
preserve commas, indicating the greedy replacement is error-prone for these settings files.

.github/workflows/backend-tests.yml[81-89]
.github/workflows/scaling-dive.yml[95-110]
.github/workflows/cpu-profile.yml[84-92]

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 workflow’s sed replacement uses a greedy `.*` which can remove required punctuation (comma) and any trailing content (e.g., JSONC inline comments) on the `"points"` line inside `commitRateLimiting`. This risks generating an invalid settings file and failing to start Etherpad for the integration sweep.
## Issue Context
Other workflows in this repo patch rate-limiting with a safer pattern that stops before the comma, preserving valid JSON/JSONC structure.
## Fix Focus Areas
- .github/workflows/backend-tests.yml[81-89]
## Suggested change
Replace the greedy substitution with a comma-preserving one (similar to scaling-dive/cpu-profile), for example:

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

---

[JohnMcLear]

Thanks @qodo — actioned in 7e4208f. Switched to the comma-preserving s!"points":[^,]*!"points": 1000000! substitution (matching the existing scaling-dive.yml / cpu-profile.yml convention) so the edit can't strip a trailing comma or JSONC comment, and added a grep -q '"points": 1000000' settings.json fail-fast assertion so a future template change surfaces loudly instead of silently booting core with the default 10/s limiter. Verified the full sed chain against the current settings.json.template produces valid output and the assertion passes.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

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

Thanks @qodo — actioned in 7e4208f. Switched to the comma-preserving s!"points":[^,]*!"points": 1000000! substitution (matching the existing scaling-dive.yml / cpu-profile.yml convention) so the edit can't strip a trailing comma or JSONC comment, and added a grep -q '"points": 1000000' settings.json fail-fast assertion so a future template change surfaces loudly instead of silently booting core with the default 10/s limiter. Verified the full sed chain against the current settings.json.template produces valid output and the assertion passes.

Thanks — that looks like a solid fix.

• The comma-preserving sed form is the safer choice for settings.json.template / JSONC-style content.
• The grep -q check is a good guardrail so future template changes fail fast instead of silently leaving the limiter at the default.
• Verifying the full chain against the current template was the right call.

No further concerns from me.