ci: swap deprecated ep_readonly_guest for ep_guest in plugin matrix

[JohnMcLear]

Summary

ep_readonly_guest is archived (read-only on GitHub) and its authenticate hook unconditionally swaps req.session.user with a read-only guest, even when the request carries an HTTP Authorization header. That silently demoted admin login attempts and stalled the anonymizeAuthorSocket tests for 14 min/run on every with-plugins CI matrix (#7795).

The pre-fix theory in #7795 blamed ep_hash_auth.handleMessage — that's a red herring. The hook is only invoked by PadMessageHandler.ts:559 and never fires on the /settings namespace. The deprecation warning in the issue trace came from an unrelated earlier test (rate-limited by warnDeprecated's 10-min stack dedupe in pad_utils.ts:141).

Root cause

ep_readonly_guest/index.js:31-39:

exports.authenticate = (hookName, {req}, cb) => {
  if (req.path === endpoint('forceauth')) return cb([]);
  req.session.user = user;            // guest, is_admin=false
  return cb([true]);                  // hard-claims auth
};

Plus it sorts itself to run first in expressCreateServer. So when the test does GET /admin/ with Authorization: Basic …, ep_readonly_guest runs first, ignores the header, sets the guest session, returns [true]. The Set-Cookie returned to the test is for a guest. Socket connects to /settings; adminsettings.ts:25 checks is_admin → false → handler bails without binding socket.on('authorLoad', …). Every subsequent emit() has no listener → mocha 60s timeout × 7.

Fix

ep_guest@1.0.72 (the maintained successor by the same authors) already defers when the request targets /admin* or carries an Authorization header — see its index.js:50-51, added in response to its own issue #85. Swapping ep_readonly_guest → ep_guest in the plugin matrix unblocks anonymizeAuthorSocket on Linux, Windows, and the upgrade-from-latest-release workflow.

The runtime probe added in #7796 stays in place — it still catches any other authenticate-hook plugin that rejects the test's plain-text credentials (e.g. a future ep_hash_auth-style hashed-only plugin). Its comment is reattributed so future readers don't chase the ep_hash_auth red herring.

Verification

Reproduced in a worktree off develop@f6ab8561a (Node 24) with the exact CI plugin list:

• Before: all 7 anonymizeAuthorSocket tests skip via the probe (Successful authentication … for user guest in logs).
• After swap: all 7 pass in ~12s.
• Full tests/backend/specs/admin + tests/backend/specs/api under the swapped matrix: 321 passing, 13 pending, 0 failing.

Files

• .github/workflows/backend-tests.yml — Linux + Windows with-plugins matrices
• .github/workflows/frontend-tests.yml — UI + admin with-plugins matrices
• .github/workflows/upgrade-from-latest-release.yml
• .github/workflows/load-test.yml
• src/tests/backend/specs/admin/anonymizeAuthorSocket.ts — reattribute the probe-skip warning so it points at ep_readonly_guest (the historical offender) instead of ep_hash_auth.

Closes #7795.

Test plan

• Linux with Plugins matrix is green
• Windows with Plugins matrix is green
• Upgrade from latest release passes (it installs the same plugin set)
• Frontend with Plugins matrices stay green (no behavior change in the guest plugin from the user's POV — ep_guest is a drop-in)

🤖 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]

Review Summary by Qodo

Replace deprecated ep_readonly_guest with ep_guest in CI workflows

🐞 Bug fix

Walkthroughs

Description

• Replace archived ep_readonly_guest with maintained ep_guest plugin
• Fixes anonymizeAuthorSocket tests stalling 14 min/run in CI
• ep_guest properly defers to basic auth and admin paths
• Clarify probe comment to reference actual offender ep_readonly_guest

Diagram

flowchart LR
  A["ep_readonly_guest<br/>archived plugin"] -->|"swap for"| B["ep_guest<br/>maintained successor"]
  B -->|"defers to"| C["Basic auth &<br/>admin paths"]
  C -->|"unblocks"| D["anonymizeAuthorSocket<br/>tests pass"]

Loading

File Changes

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

Update plugin matrix for backend tests

• Replace ep_readonly_guest with ep_guest in Linux with-plugins matrix
• Replace ep_readonly_guest with ep_guest in Windows with-plugins matrix

.github/workflows/backend-tests.yml

---

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

Update plugin matrix for frontend tests

• Replace ep_readonly_guest with ep_guest in UI with-plugins matrix
• Replace ep_readonly_guest with ep_guest in admin with-plugins matrix

.github/workflows/frontend-tests.yml

---

3. .github/workflows/load-test.yml ⚙️ Configuration changes +1/-1

Update plugin matrix for load tests

• Replace ep_readonly_guest with ep_guest in load test plugin matrix

.github/workflows/load-test.yml

---

View more (2)

4. .github/workflows/upgrade-from-latest-release.yml ⚙️ Configuration changes +1/-1

Update plugin matrix for upgrade workflow

• Replace ep_readonly_guest with ep_guest in upgrade workflow plugin matrix

.github/workflows/upgrade-from-latest-release.yml

---

5. src/tests/backend/specs/admin/anonymizeAuthorSocket.ts 📝 Documentation +14/-9

Clarify probe comment and root cause analysis

• Reattribute probe comment to identify ep_readonly_guest as historical offender
• Clarify that any authenticate-hook plugin can block basic auth
• Update event names in comment from load/settings to authorLoad/results:authorLoad
• Improve explanation of why probe is needed and how it works

src/tests/backend/specs/admin/anonymizeAuthorSocket.ts

---

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

Code Review by Qodo

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

Context used

✅ Tickets: 🎫 ep_hash_auth's handleMessage hook hangs admin socket events on /settings namespace

1. Suite still skips on failure 📎 Requirement gap ☼ Reliability

Description

The anonymizeAuthorSocket suite still calls this.skip() when the admin socket probe fails, which
can let plugin-enabled CI go green without exercising the 7 /settings socket tests. This also
means the PR does not provide a failing regression signal if the authenticate-hook interaction
regresses in the future.

Code

src/tests/backend/specs/admin/anonymizeAuthorSocket.ts[R139-146]

    if (!probe.ok) {
      console.warn(
          `[anonymizeAuthorSocket] admin socket probe failed (${probe.reason}); ` +
-          'skipping suite — likely an authenticate-hook plugin (e.g. ep_hash_auth) ' +
-          'rejecting the test\'s plain-text admin credentials. Tracked in #7795.');
+          'skipping suite — an authenticate-hook plugin (e.g. ep_readonly_guest, ' +
+          'or an ep_hash_auth-style plugin requiring hashed credentials) is ' +
+          'rejecting the test\'s plain-text admin credentials.');
      this.skip();
      return;

Evidence

PR Compliance ID 5 requires removing the temporary skip once the issue is fixed so the suite runs in
the with-plugins matrix, and PR Compliance ID 6 requires a regression test that would fail if the
issue returned. The changed warning message shows the suite still skips on probe failure rather than
failing, which can mask regressions.

Temporary mitigation: skip anonymizeAuthorSocket suite when ep_hash_auth is loaded (until fixed), then remove skip
src/tests/backend/specs/admin/anonymizeAuthorSocket.ts[139-146]
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
`src/tests/backend/specs/admin/anonymizeAuthorSocket.ts` currently skips the entire suite when the admin socket probe fails. Per the compliance checklist, this mitigation should be removed once the underlying issue is addressed, and bug fixes should be backed by a regression test that fails when the issue reappears.

## Issue Context
The probe is intended to prevent long CI hangs, but `this.skip()` allows CI to pass without validating `/settings` emit/reply behavior under plugins.

## Fix Focus Areas
- src/tests/backend/specs/admin/anonymizeAuthorSocket.ts[139-146]

## Implementation notes
- Replace `this.skip()` with a failing assertion (e.g., `assert.fail(...)`) or `throw new Error(...)` so CI fails fast (within the probe budget) instead of silently skipping.
- Ensure the failure message clearly identifies that an authenticate-hook plugin prevented admin session establishment and caused missing `/settings` handlers.

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

---

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

1. Suite still skips on failure 📎 Requirement gap ☼ Reliability

The anonymizeAuthorSocket suite still calls this.skip() when the admin socket probe fails, which
can let plugin-enabled CI go green without exercising the 7 /settings socket tests. This also
means the PR does not provide a failing regression signal if the authenticate-hook interaction
regresses in the future.

Agent Prompt

## Issue description
`src/tests/backend/specs/admin/anonymizeAuthorSocket.ts` currently skips the entire suite when the admin socket probe fails. Per the compliance checklist, this mitigation should be removed once the underlying issue is addressed, and bug fixes should be backed by a regression test that fails when the issue reappears.

## Issue Context
The probe is intended to prevent long CI hangs, but `this.skip()` allows CI to pass without validating `/settings` emit/reply behavior under plugins.

## Fix Focus Areas
- src/tests/backend/specs/admin/anonymizeAuthorSocket.ts[139-146]

## Implementation notes
- Replace `this.skip()` with a failing assertion (e.g., `assert.fail(...)`) or `throw new Error(...)` so CI fails fast (within the probe budget) instead of silently skipping.
- Ensure the failure message clearly identifies that an authenticate-hook plugin prevented admin session establishment and caused missing `/settings` handlers.

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