Skip to content

test: downstream client compatibility gate (Phase 1)#7923

Merged
JohnMcLear merged 10 commits into
developfrom
feat/downstream-client-compat-tests
Jun 9, 2026
Merged

test: downstream client compatibility gate (Phase 1)#7923
JohnMcLear merged 10 commits into
developfrom
feat/downstream-client-compat-tests

Conversation

@JohnMcLear

@JohnMcLear JohnMcLear commented Jun 9, 2026

Copy link
Copy Markdown
Member

PR Summary by Qodo

Add downstream client compatibility gate (Phase 1)
🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 20-40 Minutes

Grey Divider

Walkthroughs

User Description

What & why

Downstream clients (the Rust terminal editor etherpad-pad, the Node etherpad-cli-client, and etherpad-desktop) live in separate repos and consume core's wire protocols rather than importing core. A core PR can change the HTTP API, the socket.io message sequence, or the changeset/attribpool wire format and break them silently — their CI never runs against the new core.

This adds a two-layer compatibility gate so a PR against develop catches that before merge.

Layer A — contract tests (every PR, hermetic, in the existing mocha backend suite)

  • Golden wire-vectorssrc/tests/backend/specs/downstream/generate-vectors.ts is the single source of truth; src/tests/fixtures/wire-vectors.json is the committed fixture (regenerate with pnpm run vectors:gen). wire-vectors.ts asserts the committed fixture is byte-identical to a fresh regeneration (any drift = a deliberate wire change) and self-consistent under core's Changeset. Downstream clients reuse the same JSON with their own decoders (Phase 2).
  • Socket sequencewire-socket-sequence.ts pins handshake→CLIENT_VARS and USER_CHANGESACCEPT_COMMIT.
  • HTTP API shapeswire-http-api.ts snapshots createPad / setText+getText / getRevisionsCount response shapes.

Layer B — downstream smoke harness

  • .github/workflows/downstream-smoke.yml: boots a real Etherpad on :9003 with apikey auth, healthcheck-polls, runs an authenticated create→read self-check, generates the canonical vectors, then matrixes over the manifest — tearing the server down by PID.
  • src/tests/downstream/clients.json: manifest of the three clients pinned to specific commit SHAs (so a client's own breakage can't redden core; bumping a ref is a deliberate PR). All entries are enabled:false until their Phase-2 smoke lands, so the harness lands green on its own.

Verification

  • All 7 downstream specs pass under Node 24 (mocha --recursive tests/backend/specs/downstream).
  • Fixture regeneration is no-drift.
  • The workflow's boot → healthcheck → authenticated roundtrip → PID-teardown path was dry-run locally against a real server.

Scope

Phase 1 is core-only. Phase 2 wires each client repo's vector + smoke test and flips its manifest entry to enabled:true, one repo at a time (pad → cli → desktop). Spec + plan under docs/superpowers/.

🤖 Generated with Claude Code

AI Description
• Add mocha contract tests to pin changeset vectors, socket message sequence, and HTTP API shapes.
• Commit canonical wire-vectors fixture with a regeneration-stability guard.
• Add GitHub Actions smoke workflow scaffold to boot Etherpad and gate downstream clients via a
  manifest.
Diagram
graph TD
  PR["Core PR"] --> A["Layer A: Downstream specs"] --> F[("wire-vectors.json")]
  A --> G["generate-vectors.ts"]
  A --> P["Socket/HTTP pins"]
  PR --> W["Layer B: Smoke workflow"] --> B["Boot Etherpad :9003"] --> M["clients.json (enabled gate)"]
  B --> G --> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Run downstream repos' CI via reusable workflows
  • ➕ Closer to how each client is tested in its own repo
  • ➕ Less bespoke harness logic in core
  • ➖ Harder to pin/coordinate toolchains and inputs (vectors) consistently
  • ➖ More brittle coupling across repos; harder to keep core PRs fast/green
2. Schema-first contracts (OpenAPI + explicit socket message schemas)
  • ➕ More explicit versioning and change review than snapshots
  • ➕ Potentially better downstream tooling/documentation
  • ➖ Significant up-front effort; socket.io message schema is not currently formalized
  • ➖ Still needs integration smoke to catch behavioral issues
3. Pact-style contract testing
  • ➕ Designed for producer/consumer compatibility checks with change negotiation
  • ➕ Can scale to many clients if adopted widely
  • ➖ Overhead and complexity; requires adopting Pact tooling in multiple ecosystems (Rust/Node/Electron)
  • ➖ May be awkward for changeset/attribpool binary-ish semantics

Recommendation: The PR’s hybrid approach (golden vectors + protocol/shape pins in core, plus a smoke harness scaffold) is the best Phase 1 tradeoff: it is deterministic, runs on every PR, and creates a shared artifact (wire-vectors.json) that downstream clients can validate against without importing core. Keep the manifest pinned and disabled-by-default until each client has a stable, minimal smoke + vector test (Phase 2), and consider tightening the smoke workflow over time (e.g., artifact caching for cloned repos/toolchains and explicit timeouts/retries per client to control flake).

Grey Divider

File Changes

Tests (5)
generate-vectors.ts Add canonical wire-vector generator (Changeset + AttributePool) +77/-0

Add canonical wire-vector generator (Changeset + AttributePool)

• Creates the single source of truth for downstream wire vectors, generating changeset/pool pairs and expected results using core’s Changeset engine. Can be executed as a CLI to write the committed fixture to src/tests/fixtures/wire-vectors.json.

src/tests/backend/specs/downstream/generate-vectors.ts


wire-http-api.ts Add HTTP API response-shape contract tests (JWT auth) +54/-0

Add HTTP API response-shape contract tests (JWT auth)

• Adds mocha specs that snapshot the response envelope/shape for createPad, setText/getText roundtrip, and getRevisionsCount. Uses the existing JWT-based test harness authentication to match other API specs.

src/tests/backend/specs/downstream/wire-http-api.ts


wire-socket-sequence.ts Add socket.io message-sequence contract test (CLIENT_VARS, ACCEPT_COMMIT) +60/-0

Add socket.io message-sequence contract test (CLIENT_VARS, ACCEPT_COMMIT)

• Adds mocha specs to pin the handshake -> CLIENT_VARS flow and USER_CHANGES -> ACCEPT_COMMIT acknowledgement semantics. Includes author attribution in apool for insert operations to match server validation rules.

src/tests/backend/specs/downstream/wire-socket-sequence.ts


wire-vectors.ts Add fixture stability + self-consistency assertions for wire vectors +33/-0

Add fixture stability + self-consistency assertions for wire vectors

• Adds tests that (1) ensure the committed wire-vectors.json is byte-equivalent to a fresh regeneration and (2) validate each vector applies correctly under core’s Changeset implementation.

src/tests/backend/specs/downstream/wire-vectors.ts


wire-vectors.json Add committed golden wire-vectors fixture +62/-0

Add committed golden wire-vectors fixture

• Commits the canonical set of changeset/attribpool vectors (plain insert/delete, formatted insert, multiline insert, and attribpool reuse) that downstream clients will consume to validate decoding compatibility.

src/tests/fixtures/wire-vectors.json


Documentation (2)
2026-06-09-downstream-client-compat-tests-phase1.md Add Phase 1 implementation plan for downstream compat gate +633/-0

Add Phase 1 implementation plan for downstream compat gate

• Documents the Phase 1 task breakdown, file structure, verification commands, and commit checkpoints for adding contract tests, a golden fixture generator, a client manifest, and the downstream smoke workflow scaffold.

docs/superpowers/plans/2026-06-09-downstream-client-compat-tests-phase1.md


2026-06-09-downstream-client-compat-tests-design.md Add design spec for downstream client compatibility testing +154/-0

Add design spec for downstream client compatibility testing

• Defines the problem statement, decisions, two-layer architecture (contract tests + smoke), flakiness mitigations, and phased rollout plan for downstream client compatibility gating.

docs/superpowers/specs/2026-06-09-downstream-client-compat-tests-design.md


Other (3)
downstream-smoke.yml Add downstream smoke CI scaffold (boot/healthcheck/self-check/teardown) +105/-0

Add downstream smoke CI scaffold (boot/healthcheck/self-check/teardown)

• Introduces a new GitHub Actions workflow that boots Etherpad on port 9003 with apikey auth, polls /api/ for readiness, runs an authenticated create/getText self-check, regenerates canonical wire vectors, and (in Phase 2) will iterate enabled downstream clients from a manifest. Includes PID-based teardown to avoid killing unrelated processes.

.github/workflows/downstream-smoke.yml


package.json Add vectors:gen script for generating canonical wire vectors +1/-0

Add vectors:gen script for generating canonical wire vectors

• Adds a new pnpm script to run the TypeScript vector generator via tsx so CI and developers can regenerate src/tests/fixtures/wire-vectors.json consistently.

src/package.json


clients.json Add downstream client manifest with pinned refs (all disabled) +29/-0

Add downstream client manifest with pinned refs (all disabled)

• Introduces a manifest of downstream client repos pinned to specific commit SHAs with per-client vector and smoke commands. All entries are enabled:false until Phase 2 wires actual client runs into the workflow.

src/tests/downstream/clients.json


Grey Divider

Qodo Logo

JohnMcLear and others added 9 commits June 9, 2026 11:23
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…wn + manifest scaffold)

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

Copy link
Copy Markdown

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

qodo-free-for-open-source-projects Bot commented Jun 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

Grey Divider


Remediation recommended

1. Unverified sed settings rewrite ✓ Resolved 🐞 Bug ☼ Reliability
Description
The workflow rewrites settings.json.template via sed but never checks that the substitutions
succeeded, so a template change can silently leave port/auth unchanged and cause confusing smoke
failures.
Code

.github/workflows/downstream-smoke.yml[R52-58]

+      - name: Boot Etherpad on :9003 (apikey auth)
+        run: |
+          # The template ships a literal "port": 9001 and sso auth; rewrite both.
+          # (PORT env is ignored once the settings file specifies a port.)
+          sed -e 's#"port": 9001,#"port": 9003,#' \
+              -e 's#${AUTHENTICATION_METHOD:sso}#apikey#' \
+              settings.json.template > settings.json
Evidence
The workflow depends on matching exact literals in the template (port 9001 and the
authenticationMethod placeholder). If either literal changes, sed can produce an unmodified
settings.json, but the workflow currently has no verification step to catch that early.

.github/workflows/downstream-smoke.yml[52-63]
settings.json.template[185-197]
settings.json.template[829-835]

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 CI boot step uses `sed` to rewrite the template’s port and authentication method, but `sed` exits successfully even if the patterns don’t match. If `settings.json.template` changes format, the job can boot with the wrong port/auth and fail later (healthcheck/self-check) without a clear root cause.
### Issue Context
The workflow assumes literal strings exist in the template and does not validate the generated `settings.json`.
### Fix Focus Areas
- .github/workflows/downstream-smoke.yml[52-63]
### Suggested fix
After generating `settings.json`, validate expected settings and fail fast if missing:

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



Informational

2. Docs ignore path mismatch ✓ Resolved 🐞 Bug ➹ Performance
Description
The downstream-smoke workflow ignores only "doc/**" changes, but this PR’s documentation lives under
"docs/**", so docs-only PRs will still run the full smoke boot job unnecessarily.
Code

.github/workflows/downstream-smoke.yml[R8-13]

+on:
+  pull_request:
+    paths-ignore:
+      - "doc/**"
+  schedule:
+    - cron: '0 4 * * *'   # nightly against the default branch
Evidence
The workflow’s paths-ignore only matches doc/**, but the PR introduces documentation under
docs/superpowers/..., so those changes won’t be ignored and will trigger the workflow.

.github/workflows/downstream-smoke.yml[8-13]
docs/superpowers/specs/2026-06-09-downstream-client-compat-tests-design.md[1-6]

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 downstream smoke workflow is configured to ignore only `doc/**`, but this PR adds docs under `docs/**`. As a result, documentation-only PRs will still trigger the workflow and boot Etherpad.
### Issue Context
Repo contains both `doc/` and `docs/` trees; the new plan/spec files are under `docs/`.
### Fix Focus Areas
- .github/workflows/downstream-smoke.yml[8-13]
### Suggested fix
Update `paths-ignore` to include `docs/**` (and optionally keep `doc/**`):

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


Grey Divider

Qodo Logo

Fail fast if the template's port/auth literals drift so a no-op sed can't
silently boot the smoke server on the wrong port/auth. Also ignore docs/**
(not just doc/**) so docs-only PRs don't trigger the boot job.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JohnMcLear

Copy link
Copy Markdown
Member Author

Thanks @qodo — both addressed in 7c232a2:

  1. Unverified sed rewrite — added fail-fast grep guards after the sed so a drifted template literal can't silently boot the smoke server on the wrong port/auth; the step now errors with a clear message instead of failing opaquely at healthcheck.
  2. Docs ignore path mismatch — added docs/** to paths-ignore (the plan/spec live there), so docs-only PRs no longer trigger the boot job.

@JohnMcLear JohnMcLear merged commit 860ab68 into develop Jun 9, 2026
33 of 34 checks passed
@JohnMcLear JohnMcLear deleted the feat/downstream-client-compat-tests branch June 9, 2026 10:41
JohnMcLear added a commit to ether/etherpad-desktop that referenced this pull request Jun 9, 2026
* test: downstream wire-compat (vectors + smoke)

Phase 2 of ether/etherpad#7923. Phase 1 added a canonical wire-format
fixture that all Etherpad clients must decode identically.

The desktop/mobile apps are thin shells: they embed core's Ace editor in
a webview and load a server URL, so there is no local changeset decoder.
The vectors test is therefore a fixture-integrity guard (shape/contract
of the vendored wire-vectors.json), not a decode test. The smoke test is
a headless-light HTTP roundtrip against the server contract the shell
depends on; the full Electron e2e stays in this repo's own CI.

- packages/shell/tests/fixtures/wire-vectors.json: vendored canonical fixture
  (overridable via ETHERPAD_WIRE_VECTORS).
- packages/shell/tests/wire/vectors.spec.ts: asserts every record has the
  5 fields with correct types; pool.numToAttrib is a plain object, nextNum
  a non-negative integer; initial/resultText non-empty and \n-terminated.
- packages/shell/tests/wire/smoke.spec.ts: reads ETHERPAD_SMOKE_URL
  (default http://localhost:9003) + ETHERPAD_SMOKE_APIKEY; skips cleanly
  unless both a reachable server and a key are present. When live: create
  pad via HTTP API, fetch /p/<pad> (the URL the shell loads) -> 200,
  getText to confirm content.
- root package.json: add test:vectors / test:smoke scripts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: address Qodo review on smoke test

- Skip the reachability probe entirely when ETHERPAD_SMOKE_APIKEY is
  unset (no key => the test always skips, so the probe + timeout was
  wasted work).
- Wrap the create -> fetch -> getText roundtrip in try/finally so the
  pad is always deleted even when an assertion throws; swallow delete
  errors so cleanup never masks the real failure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: normalize trailing newline in smoke getText assertion

Etherpad guarantees a pad ends with exactly one trailing newline, so
setText("X\n") reads back as "X\n\n". Compare normalized text instead of
exact equality. Verified live against a real core on :9013.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pull Bot pushed a commit to fgreinacher/etherpad-cli-client that referenced this pull request Jun 9, 2026
Phase 2 of ether/etherpad#7923. Adds this repo's first test runner
(node:test via tsx) plus two suites:

- test:vectors — server-free, replays the canonical wire-format fixture
  through the repo's own Changeset/AttributePool decoders and asserts
  byte-for-byte text equality. All 5 vectors pass with no decoder changes.
- test:smoke — live HTTP+socket.io round-trip; skips cleanly when no
  server/API key is available.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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