Skip to content

ci: test ep_font_color and ep_hash_auth in with-plugins matrix#7639

Merged
JohnMcLear merged 2 commits intoether:developfrom
JohnMcLear:ci/add-font-color-hash-auth
May 1, 2026
Merged

ci: test ep_font_color and ep_hash_auth in with-plugins matrix#7639
JohnMcLear merged 2 commits intoether:developfrom
JohnMcLear:ci/add-font-color-hash-auth

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

@JohnMcLear JohnMcLear commented May 1, 2026

Summary

  • Adds ep_font_color (Not able to find the subvalue colorId in null #12 by npm last-30d downloads, ~16.3k) and ep_hash_auth ([WebSocket] SYNTAX_ERR: invalid url: wss://... #14, ~10.5k) to all four plugin install lists: withpluginsLinux / withpluginsWindows in backend-tests, and both Playwright with-plugins jobs in frontend-tests.
  • These were the only top-15 plugins not already covered. Existing coverage already includes Added Makefile #1Support proxy requests #11; this closes the remaining gaps that don't require special test setup. (ep_webrtc, Support subfolders #13, is being stabilized separately and is intentionally excluded for now.)
  • Also fixes a pre-existing flake in change_user_color.spec.ts ("Own user color is shown when you enter a chat") that surfaced under the broader plugin matrix: the test left the #users popup open, which overlapped #chaticon and made showChat() retry-click for the full 90s × 5 retries until the suite timed out. Now the popup is explicitly toggled closed before opening chat (matching the close pattern used in a11y_dialogs.spec.ts).

Why these two

  • ep_font_color exercises aceEditorCSS / aceAttribsToClasses — same plugin family as the already-tested ep_font_size, ep_align.
  • ep_hash_auth exercises authenticate / handleMessage, which no other plugin in the matrix touches. Its authenticate hook is a no-op unless a Basic header is sent and settings.users[user].hash exists, so it falls through cleanly under the default test settings — no test config changes needed.

Semver

patch — CI-only change, no runtime/user-facing impact.

Test plan

  • withpluginsLinux (Node 22/24/25) green
  • withpluginsWindows (Node 22/24/25) green
  • Playwright Chrome with plugins green
  • Playwright Firefox with plugins green

🤖 Generated with Claude Code

These are the ether#12 and ether#14 most-installed Etherpad plugins on npm
(last 30d) and were the only top-15 plugins not exercised by the
withpluginsLinux / withpluginsWindows / Playwright with-plugins
jobs. Adding them broadens coverage of the plugin loader against
two real-world hooks: aceEditorCSS / aceAttribsToClasses
(ep_font_color) and authenticate / handleMessage (ep_hash_auth).

ep_hash_auth's authenticate hook is a no-op unless a Basic auth
header is sent and a matching settings.users[user].hash exists,
so it falls through cleanly with the default test settings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@JohnMcLear
Copy link
Copy Markdown
Member Author

/review

@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 1, 2026

Code Review by Qodo

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

Context used

Grey Divider


Remediation recommended

1. Duplicated plugin lists 🐞 Bug ⚙ Maintainability ⭐ New
Description
The with-plugins plugin install list is hard-coded in four workflow steps, so future changes require
multi-file edits and can easily diverge between backend and frontend matrices. This increases CI
maintenance burden and risks inconsistent test coverage when a plugin is added/removed in only some
jobs.
Code

.github/workflows/backend-tests.yml[R126-132]

          ep_align
          ep_author_hover
          ep_cursortrace
+          ep_font_color
          ep_font_size
+          ep_hash_auth
          ep_headings2
Evidence
The same pnpm add -w plugin list is repeated in backend-tests (Linux + Windows) and frontend-tests
(Playwright Chromium + Firefox). This PR had to add the same two plugins in all four locations,
demonstrating the duplication and the manual sync requirement.

.github/workflows/backend-tests.yml[123-138]
.github/workflows/backend-tests.yml[237-252]
.github/workflows/frontend-tests.yml[216-229]
.github/workflows/frontend-tests.yml[306-319]

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 with-plugins workflow plugin list is duplicated across four jobs. This requires manual synchronization and can cause the backend and frontend plugin matrices to drift.

### Issue Context
This PR updated the same list in four places, which is a recurring maintenance hazard for future plugin additions/removals.

### Fix Focus Areas
- .github/workflows/backend-tests.yml[123-138]
- .github/workflows/backend-tests.yml[237-252]
- .github/workflows/frontend-tests.yml[216-229]
- .github/workflows/frontend-tests.yml[306-319]

### Suggested approach
Create a shared source of truth (for example `.github/with-plugins.txt` or a composite action under `.github/actions/install-etherpad-plugins/`) and have each workflow step invoke it, so the list is defined once and consumed everywhere.

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


2. Unpinned CI plugin versions 🐞 Bug ☼ Reliability
Description
The workflows install plugins (including the newly-added ep_font_color and ep_hash_auth) without
version constraints, so CI will resolve whatever plugin versions are latest at runtime, making test
outcomes non-reproducible and susceptible to upstream plugin releases breaking CI. This conflicts
with the otherwise deterministic pnpm install --frozen-lockfile setup used earlier in the jobs.
Code

.github/workflows/backend-tests.yml[R129-131]

+          ep_font_color
         ep_font_size
+          ep_hash_auth
Evidence
The workflows use pnpm install --frozen-lockfile for core dependencies but then install plugins
via pnpm add -w with bare package names (no @version), meaning the plugin versions are not
pinned by the repo lockfile and can change between CI runs.

.github/workflows/backend-tests.yml[117-134]
.github/workflows/frontend-tests.yml[198-229]

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

## Issue description
GitHub Actions installs Etherpad plugins using `pnpm add -w` with no versions, so CI resolves the latest plugin versions at runtime. This makes CI results non-deterministic and allows upstream plugin releases to break the repo’s CI unexpectedly.
## Issue Context
The workflows already use `pnpm install --frozen-lockfile` for core deps, but the plugin install step bypasses version pinning.
## Fix Focus Areas
- .github/workflows/backend-tests.yml[117-139]
- .github/workflows/frontend-tests.yml[198-230]
## Suggested approach
Pick one:
1) Pin explicit versions in the `pnpm add -w` list (e.g., `ep_hash_auth@x.y.z`).
2) Add the plugins to the repo’s workspace dependencies (so they are in `pnpm-lock.yaml`), and replace `pnpm add -w ...` with `pnpm install --frozen-lockfile` (or equivalent) in CI so plugin versions are locked.
3) If you intentionally want “latest plugin” coverage, document that intent in the workflow comments and consider adding a scheduled job for latest-plugin testing while keeping PR CI pinned.

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


Grey Divider

Previous review results

Review updated until commit 5881167

Results up to commit 47ef8e2


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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Add ep_font_color and ep_hash_auth to plugin test matrix

🧪 Tests

Grey Divider

Walkthroughs

Description
• Adds ep_font_color and ep_hash_auth to plugin test matrix
• Covers top-15 npm plugins previously untested in CI
• Exercises aceEditorCSS and authentication hooks
• Applied to all four plugin installation jobs
Diagram
flowchart LR
  A["CI Workflow Jobs"] -->|Add plugins| B["backend-tests.yml"]
  A -->|Add plugins| C["frontend-tests.yml"]
  B -->|Install| D["ep_font_color<br/>ep_hash_auth"]
  C -->|Install| D
  D -->|Test coverage| E["aceEditorCSS hooks<br/>authenticate hooks"]
Loading

Grey Divider

File Changes

1. .github/workflows/backend-tests.yml ⚙️ Configuration changes +4/-0

Add two plugins to backend test matrix

• Adds ep_font_color and ep_hash_auth to withpluginsLinux job plugin list
• Adds ep_font_color and ep_hash_auth to withpluginsWindows job plugin list
• Maintains alphabetical ordering in plugin installation commands

.github/workflows/backend-tests.yml


2. .github/workflows/frontend-tests.yml ⚙️ Configuration changes +4/-0

Add two plugins to frontend test matrix

• Adds ep_font_color and ep_hash_auth to Playwright Chrome with-plugins job
• Adds ep_font_color and ep_hash_auth to Playwright Firefox with-plugins job
• Maintains consistent plugin list across all frontend test jobs

.github/workflows/frontend-tests.yml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 1, 2026

Code Review by Qodo

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

Context used

Grey Divider


Remediation recommended

1. Unpinned CI plugin versions 🐞 Bug ☼ Reliability
Description
The workflows install plugins (including the newly-added ep_font_color and ep_hash_auth) without
version constraints, so CI will resolve whatever plugin versions are latest at runtime, making test
outcomes non-reproducible and susceptible to upstream plugin releases breaking CI. This conflicts
with the otherwise deterministic pnpm install --frozen-lockfile setup used earlier in the jobs.
Code

.github/workflows/backend-tests.yml[R129-131]

+          ep_font_color
          ep_font_size
+          ep_hash_auth
Evidence
The workflows use pnpm install --frozen-lockfile for core dependencies but then install plugins
via pnpm add -w with bare package names (no @version), meaning the plugin versions are not
pinned by the repo lockfile and can change between CI runs.

.github/workflows/backend-tests.yml[117-134]
.github/workflows/frontend-tests.yml[198-229]

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

## Issue description
GitHub Actions installs Etherpad plugins using `pnpm add -w` with no versions, so CI resolves the latest plugin versions at runtime. This makes CI results non-deterministic and allows upstream plugin releases to break the repo’s CI unexpectedly.

## Issue Context
The workflows already use `pnpm install --frozen-lockfile` for core deps, but the plugin install step bypasses version pinning.

## Fix Focus Areas
- .github/workflows/backend-tests.yml[117-139]
- .github/workflows/frontend-tests.yml[198-230]

## Suggested approach
Pick one:
1) Pin explicit versions in the `pnpm add -w` list (e.g., `ep_hash_auth@x.y.z`).
2) Add the plugins to the repo’s workspace dependencies (so they are in `pnpm-lock.yaml`), and replace `pnpm add -w ...` with `pnpm install --frozen-lockfile` (or equivalent) in CI so plugin versions are locked.
3) If you intentionally want “latest plugin” coverage, document that intent in the workflow comments and consider adding a scheduled job for latest-plugin testing while keeping PR CI pinned.

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


Grey Divider

Qodo Logo

The "Own user color is shown when you enter a chat" spec leaves the
users popup open after picking a color, then calls showChat(). In the
with-plugins matrix the popup overlaps #chaticon and intercepts pointer
events, so the click in showChat() is retried until the 90s timeout
(× 5 retries ≈ 7m), failing both Firefox and Chrome with-plugins jobs.

Toggle the users button off and wait for popup-show to drop before
clicking the chat icon, matching the close pattern used in
a11y_dialogs.spec.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohnMcLear
Copy link
Copy Markdown
Member Author

/review

@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 1, 2026

Persistent review updated to latest commit 5881167

@JohnMcLear JohnMcLear merged commit 4704d80 into ether:develop May 1, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant