Synchronize browser specs with capybara-lockstep

Gemfile

@@ -87,5 +87,6 @@ group :test do
   gem "factory_bot_rails"
   # Use system testing [https://guides.rubyonrails.org/testing.html#system-testing]
   gem "capybara"
+  gem "capybara-lockstep"
   gem "selenium-webdriver"
 end

Gemfile.lock

@@ -136,6 +136,9 @@ GEM
       rack-test (>= 0.6.3)
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
+    capybara-lockstep (2.4.0)
+      activesupport (>= 7.0)
+      capybara (>= 3.0)
     commonmarker (2.6.3-aarch64-linux)
     commonmarker (2.6.3-arm-linux)
     commonmarker (2.6.3-arm64-darwin)
@@ -540,6 +543,7 @@ DEPENDENCIES
   brakeman
   bundler-audit
   capybara
+  capybara-lockstep
   commonmarker
   coplan-engine!
   coplan-slack!
@@ -598,6 +602,7 @@ CHECKSUMS
   builder (3.3.0) sha256=497918d2f9dca528fdca4b88d84e4ef4387256d984b8154e9d5d3fe5a9c8835f
   bundler-audit (0.9.3) sha256=81c8766c71e47d0d28a0f98c7eed028539f21a6ea3cd8f685eb6f42333c9b4e9
   capybara (3.40.0) sha256=42dba720578ea1ca65fd7a41d163dd368502c191804558f6e0f71b391054aeef
+  capybara-lockstep (2.4.0) sha256=a95893430c5bb7d7802eff37703425a48820478665e29a62854605990496ac12
   commonmarker (2.6.3-aarch64-linux) sha256=73795e80ab5ef1e4b5b83ada6f082bccb0ed7eae0b910232e13af1b2d71b14d6
   commonmarker (2.6.3-arm-linux) sha256=62b9f32d7d3f85d47988a4a98a2e66e60ca42b894687047db8332f1e80caff7b
   commonmarker (2.6.3-arm64-darwin) sha256=d6c1e4955619da3f68fed22de99dec49a24925611770c039bf870823846c8b21

app/views/layouts/application.html.erb

@@ -1,6 +1,7 @@
 <!DOCTYPE html>
 <html lang="<%= I18n.locale %>">
   <head>
+    <%= capybara_lockstep if defined?(Capybara::Lockstep) %>
     <title><%= content_for(:title) || "Coplan" %></title>
     <meta name="viewport" content="width=device-width,initial-scale=1">
     <meta name="color-scheme" content="light dark">

config/environments/test.rb

@@ -4,6 +4,8 @@
 # and recreated between test runs. Don't rely on the data there!
 
 Rails.application.configure do
+  config.middleware.insert_before 0, Capybara::Lockstep::Middleware
+
   # Settings specified here will take precedence over those in config/application.rb.
 
   # While tests run files are not watched, reloading is not necessary.

docs/BROWSER_TESTS.md

@@ -0,0 +1,108 @@
+# Browser test synchronization
+
+System specs run Chrome through Capybara and Selenium. The test-only
+[`capybara-lockstep`](https://github.com/makandra/capybara-lockstep) gem waits
+for browser work around normal Capybara commands. Both page layouts include its
+snippet before application scripts. A test-only head wrapper covers ActiveAdmin's
+separate layout too. The test environment also uses its Rack
+middleware to wait for server requests.
+
+Write normal interactions and assertions:
+
+```ruby
+click_button "Post comment"
+expect(page).to have_css("mark.anchor-highlight", text: "Selected text")
+```
+
+Lockstep tracks fetch/XHR requests and resource loading. The shared snippet in
+`spec/support/browser_synchronization.js` also tracks each received Turbo Stream
+through its render promise, including renderers wrapped by Stimulus controllers.
+That covers both HTTP responses and streams received over Action Cable. A
+cancelled or failed renderer releases its work count.
+
+The gem uses Capybara's current wait budget. By default a synchronization timeout
+is logged, and synchronization is tried again on the next command. This matters
+when a full document navigation discards the previous page's probe. A logged
+timeout is not proof of readiness: feature assertions must still pass. The
+cancellation/failure regressions explicitly wait for the browser's idle callback
+and fail if the work count leaks, regardless of this logging policy.
+
+## Readiness has a scope
+
+Stimulus connects controllers, but does not know when every controller's async
+work has finished. `document.readyState` describes the initial document load,
+not later changes to a live page. Lockstep supplies test coordination, not a
+permanent promise that the document cannot change.
+
+Keep Capybara's retrying assertions. A future broadcast can replace elements
+after any readiness check. When asserting a collection, put the entire read
+inside a Capybara filter so a detached element retries the whole read:
+
+```ruby
+expect(page).to have_css("#plan-content-body") do |body|
+  body.all("mark.anchor-highlight", count: 2).map(&:text) == [ "relation", "links" ]
+end
+```
+
+For a changed form control, assert against the page by its accessible name:
+
+```ruby
+check "Task: Write documentation", enable_aria_label: true
+expect(page).to have_checked_field("Task: Write documentation", enable_aria_label: true)
+```
+
+Avoid inspecting a saved element handle after the action. A Turbo response can
+replace that element even when the action succeeds.
+
+Synthetic native event sequences need the same ordering as a real gesture.
+For example, dispatch `drop` and `dragend` together before allowing Lockstep to
+wait for the drop's network response and navigation. The folder spring-loading
+specs demonstrate this without pausing synchronization for the whole example.
+
+An open WebSocket is not unfinished work. The shared hook waits for a stream
+that has arrived; it cannot predict a future message. Specs that initiate work
+from Ruby must first observe the expected delivery and can explicitly call
+`Capybara::Lockstep.synchronize` to join its rendering. The real cable regression
+in `spec/system/browser_synchronization_spec.rb` demonstrates this boundary.
+
+Timers, animations, service workers, and application promises outside tracked
+requests are not automatically idle conditions. Assert their visible result or
+use a specific completion signal. Do not add sleeps to guess when a page is ready.
+
+## Test unfinished work deliberately
+
+Progress, overlapping edits, held pointer gestures, and delayed-response tests
+must interact while work is unfinished. Mark those examples explicitly:
+
+```ruby
+it "keeps the save indicator visible while the response is held", browser_sync: :manual do
+  # Hold the response, assert progress, then release it and assert completion.
+end
+```
+
+This pauses automatic Lockstep synchronization for that example. Capybara's
+normal assertion retries remain active. The shared setup restores automatic
+mode after every example, including failures. Do not use this metadata to hide
+an unexplained synchronization timeout.
+
+For a small observation within an otherwise synchronized test, use
+`Capybara::Lockstep.with_mode(:manual) { ... }`, then explicitly synchronize once
+the held work is released. Prefer the smallest scope that preserves the race
+the regression is meant to exercise.
+
+## Try the integration
+
+Use a dedicated test database so other checkouts retain their data:
+
+```sh
+bundle install
+RAILS_ENV=test DATABASE_URL=mysql2://root@localhost/coplan_browser_sync_tryout bin/rails db:create db:schema:load
+RAILS_ENV=test DATABASE_URL=mysql2://root@localhost/coplan_browser_sync_tryout bundle exec rspec spec/system/browser_synchronization_spec.rb spec/system/math_rendering_spec.rb
+```
+
+Expect the delayed-fetch, real-cable, cancelled/failed-renderer, and math cases
+to pass. The command starts its own test server. Product navigation and visual
+styles are unchanged.
+
+Set `BROWSER_SYNC_DEBUG=1` on a spec command to log Lockstep's synchronization
+steps and tracked work while diagnosing a timeout.

docs/CI.md

@@ -92,6 +92,10 @@ runs once in its original defined order.
 
 ## Run locally
 
+Browser specs share the synchronization setup described in
+[Browser test synchronization](BROWSER_TESTS.md), including explicit manual mode
+for tests that inspect unfinished requests or gestures.
+
 Preview a browser shard without a database or Rails:
 
 ```sh

docs/CI_RELIABILITY.md

@@ -41,6 +41,60 @@ Older failures of removed editing flows and shipped regressions are historical
 evidence, not reasons to alter the current assertions. Validate current behavior
 with full suites as well as the focused repeated runs below.
 
+## November 11 post-merge follow-up
+
+These are the original attempts; a later green run does not erase a failure.
+
+| Merge | Original main run | Result |
+| --- | --- | --- |
+| #311 — recovered titles and numbered title links (`cd5dfdc0`) | [43087241467, attempt 1](https://github.com/block/coplan/actions/runs/43087241467/attempts/1) | Browser shard 4 failed the non-empty paragraph Backspace case in `editor_lists_spec.rb`, seed 8443. The paragraph remained, but the caret did not reach the expected offset. Shards 1 and 3 were cancelled by fail-fast. |
+| #305 — preserve review history (`0b3c0dd0`) | [43087376842](https://github.com/block/coplan/actions/runs/43087376842) | Full main CI passed. |
+| #324 — keep linked comment windows open (`3b1cdcb2`) | [43087536377, attempt 1](https://github.com/block/coplan/actions/runs/43087536377/attempts/1) | Browser shard 4 failed the math comment anchor case, seed 41209, with `StaleElementReferenceError` while reading highlight text. Shard 2 was cancelled by fail-fast. |
+| #321 — expired-session commit failure event (`06fecace`) | [43090004657](https://github.com/block/coplan/actions/runs/43090004657) | Full main CI passed. |
+
+The math failure reproduced naturally on the isolated Mac/MySQL checkout at
+`06fecace`: the fifth fresh-process run failed at seed 41213 with the same stale
+element exception. The screenshot retained both correct highlights, and the
+stored anchor/source assertions had passed. The test held WebDriver elements
+across separate text reads while thread updates could replace their marks.
+The first correction read ordered text in one browser turn inside Capybara's
+retry loop; ten fresh-process runs with seeds 41209–41218 passed. Hampton then
+requested shared browser synchronization instead of a JavaScript snapshot in
+the feature spec. The replacement uses `capybara-lockstep` with test-only Turbo
+Stream render tracking. The ordered text assertion is Ruby inside Capybara's
+retry loop, retaining the exact count, text, source range, formula exclusion,
+and reload checks. See [Browser test synchronization](BROWSER_TESTS.md) for its
+scope and the explicit exceptions for tests of unfinished work.
+
+During that integration, a strict timeout setting rejected probes discarded by
+full document navigation during sign-in. A time-origin check confirmed the
+document replacement. The final integration keeps Lockstep's default timeout
+logging and next-command synchronization. Production navigation and theme
+initialization are unchanged. Feature assertions and the explicit stream idle
+callback regression remain the gates, rather than a logged timeout counting as
+readiness.
+
+The first Linux CI run with Lockstep exposed two checkbox specs that inspected
+old element handles after a version broadcast replaced the document body. Both
+stale-element failures reproduced locally with seed 49204. The assertions now
+re-query the named task fields, retaining checked/unchecked state, exact saved
+content, revision, attribution, and provenance checks.
+
+The next full run completed the previously cancelled browser shard and exposed
+a spring-loaded-folder test that sent `drop` and `dragend` as separate browser
+commands. Lockstep waited for the move response between them, so navigation
+removed the drag source before the second command. The synthetic helper now
+sends that pair together, retaining the tunneled-folder, saved-placement, and
+final-path assertions. Normal synchronization resumes after the gesture.
+
+The editor caret failure did not reproduce in five focused fresh-process runs,
+seeds 8443–8447, on the same isolated MySQL database. Its screenshot showed the
+caret one character into `Outside`. It remains unexplained; the later green runs
+and these local passes do not establish its cause or prove a product fix.
+Only one failed-job retry per original failing run was requested. The original
+cancelled shards were not selected for retry, and their incomplete coverage must
+remain visible when assessing those merges.
+
 ## Reproduce and preserve evidence
 
 Use a separate test database when another checkout is running specs. Initialize

engine/app/views/layouts/coplan/application.html.erb

@@ -1,6 +1,7 @@
 <!DOCTYPE html>
 <html lang="<%= I18n.locale %>" data-app-theme="<%= coplan_app_theme %>" data-app-theme-choice="<%= coplan_app_theme_choice %>"<% if signed_in? && current_user.theme_preference != "system" %> data-theme="<%= current_user.theme_preference %>"<% end %>>
   <head>
+    <%= capybara_lockstep if defined?(Capybara::Lockstep) %>
     <title><%= content_for(:title) || "Coplan" %></title>
     <meta name="viewport" content="width=device-width,initial-scale=1">
     <meta name="color-scheme" content="<%= signed_in? && current_user.theme_preference != 'system' ? current_user.theme_preference : 'light dark' %>">

spec/support/browser_synchronization.js

@@ -0,0 +1,29 @@
+// A received Turbo Stream still has a repaint and possibly an async renderer
+// ahead of it. Track that work for HTTP responses and WebSocket broadcasts.
+document.addEventListener("turbo:before-stream-render", event => {
+  const lockstep = window.CapybaraLockstep
+  let pending = true
+  const finish = () => {
+    if (!pending) return
+    pending = false
+    lockstep.stopWork("Turbo Stream")
+  }
+
+  lockstep.startWork("Turbo Stream")
+  // Other listeners may wrap the renderer or cancel the stream. Capture their
+  // final renderer after dispatch, before Turbo's next-repaint render step.
+  queueMicrotask(() => {
+    if (event.defaultPrevented) {
+      finish()
+      return
+    }
+    const render = event.detail.render
+    event.detail.render = async stream => {
+      try {
+        return await render(stream)
+      } finally {
+        finish()
+      }
+    }
+  })
+}, { capture: true })

spec/support/browser_synchronization.rb

@@ -0,0 +1,29 @@
+# Extend the gem's early head snippet without shipping test instrumentation as
+# a production asset. Both the host and engine layouts use the same helper.
+module BrowserSynchronization
+  SCRIPT = File.read(File.join(__dir__, "browser_synchronization.js")).freeze
+
+  def capybara_lockstep_js(options = {})
+    super + "\n" + SCRIPT
+  end
+end
+
+Capybara::Lockstep::Helper.prepend(BrowserSynchronization)
+# Keep the gem's timeout behavior: a document replacement can discard a probe;
+# it logs that timeout and tries again on the next command. Assertions still fail
+# normally when their expected result does not arrive.
+Capybara::Lockstep.debug = true if ENV["BROWSER_SYNC_DEBUG"] == "1"
+
+# ActiveAdmin owns a separate layout. Wrap its head only in this test host.
+ActionController::Base.prepend_view_path(File.join(__dir__, "views"))
+
+RSpec.configure do |config|
+  # Loading/progress regressions intentionally inspect unfinished work. Keep
+  # their normal Capybara assertions, but don't wait for the held request.
+  config.around(:each, type: :system) do |example|
+    Capybara::Lockstep.mode = example.metadata.fetch(:browser_sync, :auto)
+    example.run
+  ensure
+    Capybara::Lockstep.mode = :auto
+  end
+end

spec/support/system_test_config.rb

@@ -15,6 +15,8 @@
     # :selenium_chrome_headless is a Capybara-registered driver that
     # ignores screen_size — the :selenium/using form respects it.
     driven_by :selenium, using: :headless_chrome, screen_size: [ 1400, 900 ] do |options|
+      options.unhandled_prompt_behavior = "ignore"
+      options.logging_prefs = { browser: "ALL" }
       # Use Chrome for Testing (provisioned by selenium-manager), NOT the
       # system Chrome. On managed machines the system Chrome is enrolled in
       # Chrome Browser Cloud Management, which force-installs IT extensions

spec/support/views/active_admin/_html_head.html.erb

@@ -0,0 +1,2 @@
+<%= capybara_lockstep %>
+<%= render file: ActiveAdmin::Engine.root.join("app/views/active_admin/_html_head.html.erb") %>

spec/system/admin_spec.rb

@@ -13,6 +13,7 @@
     click_button "Menu"
     click_link "Administration"
     expect(page).to have_current_path("/_/admin")
+    expect(page.evaluate_script("typeof CapybaraLockstep")).to eq("object")
     expect(page).to have_content("Notification delivery · last 7 days")
 
     click_link "View delivery attempts"