From 5259fbdc999bd4a2c579c180f9cd2ee2ee6c58b4 Mon Sep 17 00:00:00 2001 From: Lucas Carlson Date: Mon, 10 Aug 2026 15:04:56 -0700 Subject: [PATCH 1/2] fix: load the mailbox when the gem is required Mailbox was reachable only through the caller path, which loads it as a side effect of SolidObjects.client. The reminder scheduler and the effect executor enqueue through it directly and run in solid_objects start, a process that never calls the client, so both raised NameError on every attempt. Reminders never fired and effect result messages never delivered, while the supervisor replaced the dying role over and over. Every test passed because a test process has already loaded the constant through some other path. That is the whole shape of the bug, so the regression test runs a due reminder through a real worker, and a load contract test asks a fresh process what requiring the gem actually defines. A file that stops being loaded now fails that test unless it is listed as deliberately deferred with a reason. The roadmap listed reminders as implemented and tested while they were broken in the only process that runs them. --- CHANGELOG.md | 18 ++++++ Gemfile.lock | 4 +- docs/roadmap.md | 6 +- lib/solid_objects.rb | 5 ++ lib/solid_objects/version.rb | 2 +- test/dummy/prepare_cli_reminder.rb | 31 ++++++++++ test/integration/cli_test.rb | 64 ++++++++++++++++++++ test/integration/load_contract_test.rb | 83 ++++++++++++++++++++++++++ 8 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 test/dummy/prepare_cli_reminder.rb create mode 100644 test/integration/load_contract_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f4855..4507d11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## 0.10.2 - 2026-08-10 + +- Load the mailbox when the gem is required. `SolidObjects::Mailbox` was + reachable only through the caller path, which loads it as a side effect of + `SolidObjects.client`. The reminder scheduler and the effect executor enqueue + through it directly and run in `solid_objects start`, a process that never + calls the client, so both raised + `NameError: uninitialized constant SolidObjects::ReminderScheduler::Mailbox`. + Reminders never fired and effect result messages never delivered, while the + supervisor replaced the dying role over and over. Nothing caught it because + every test process has already loaded the constant through some other path. +- Add a load contract test that asks a fresh process what `require + "solid_objects"` actually defines, and fails when a file stops being loaded + unless it is listed as deliberately deferred with a reason. This is the class + of bug that only appears in the standalone worker. +- Run a due reminder through a real `solid_objects start` worker in the test + suite, rather than only in process. + ## 0.10.1 - 2026-08-10 - Support Trilogy. Adapter selection matched the client name rather than the diff --git a/Gemfile.lock b/Gemfile.lock index b261ae2..5905bba 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - solid_objects (0.10.1) + solid_objects (0.10.2) actioncable (>= 8.0) actionpack (>= 8.0) actionview (>= 8.0) @@ -383,7 +383,7 @@ CHECKSUMS rubocop-rails-omakase (1.1.0) sha256=2af73ac8ee5852de2919abbd2618af9c15c19b512c4cfc1f9a5d3b6ef009109d ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33 securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1 - solid_objects (0.10.1) + solid_objects (0.10.2) sqlite3 (2.9.5-aarch64-linux-gnu) sha256=78075b6337d3d182c6d2b4691049ed45cd220826160c9ea18946bf6a1de200dc sqlite3 (2.9.5-aarch64-linux-musl) sha256=18c801185deb4adc01ddb281e8f672a39e3d1729979ca91e39439cd3eac0402d sqlite3 (2.9.5-arm-linux-gnu) sha256=1bdfca0c7d63998c60b0f4a8e3c8df2d33800ccc4abd2d612eddbbbc92a4c48b diff --git a/docs/roadmap.md b/docs/roadmap.md index a20ad43..ddef3fc 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -18,7 +18,11 @@ status, which works on all three adapters; a future version may add narrow ready/claimed membership tables for very large outboxes, as messages already have -- One-shot and recurring reminders with `:latest` or `:all` catch-up +- One-shot and recurring reminders with `:latest` or `:all` catch-up, exercised + through a real `solid_objects start` worker as well as in process. They were + listed here while broken in that worker: the scheduler reached a constant the + caller path happened to load, so reminders never fired in production and + every in-process test still passed - Durable observable invalidations, scalar Turbo replacement, keyed ERB components, signed component locals, and authorized replace or morph refresh - Batched component refreshes: components sharing a signed `batch:` collapse to diff --git a/lib/solid_objects.rb b/lib/solid_objects.rb index c28a6af..e8c9d85 100644 --- a/lib/solid_objects.rb +++ b/lib/solid_objects.rb @@ -55,6 +55,11 @@ require "solid_objects/commit_action_registry" require "solid_objects/lease" require "solid_objects/lease_renewer" +# The reminder scheduler and the effect executor enqueue through the mailbox, +# and both run in the standalone worker where nothing else has loaded it. It +# was reachable only through the caller path, so requiring the gem was not +# enough to run a role that uses it. +require "solid_objects/mailbox" require "solid_objects/worker" require "solid_objects/effect_executor" require "solid_objects/reminder_scheduler" diff --git a/lib/solid_objects/version.rb b/lib/solid_objects/version.rb index a16de9c..54655ff 100644 --- a/lib/solid_objects/version.rb +++ b/lib/solid_objects/version.rb @@ -1,5 +1,5 @@ # rbs_inline: enabled module SolidObjects - VERSION = "0.10.1" + VERSION = "0.10.2" end diff --git a/test/dummy/prepare_cli_reminder.rb b/test/dummy/prepare_cli_reminder.rb new file mode 100644 index 0000000..c8b5625 --- /dev/null +++ b/test/dummy/prepare_cli_reminder.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +ENV["RAILS_ENV"] = "test" + +require_relative "config/environment" +require_relative "../../db/migrate/20260805000000_create_solid_objects_tables" +require_relative "../../db/migrate/20260806000000_add_state_revision_to_solid_objects_instances" + +CreateSolidObjectsTables.new.migrate(:up) +AddStateRevisionToSolidObjectsInstances.new.migrate(:up) + +# A reminder that is already due, so the scheduler claims and enqueues it on +# its first pass rather than waiting. +instance = SolidObjects::Instance.create!( + actor_type: "CliWorkerActor", + actor_id: "reminder-in-worker", + state: {}, + state_version: 1 +) +reminder = SolidObjects::Reminder.create!( + instance:, + actor_type: instance.actor_type, + actor_id: instance.actor_id, + name: "deliver_push", + message_name: "complete", + arguments: {}, + next_run_at: 1.minute.ago, + status: "scheduled" +) + +puts reminder.id diff --git a/test/integration/cli_test.rb b/test/integration/cli_test.rb index d1a10a2..8650911 100644 --- a/test/integration/cli_test.rb +++ b/test/integration/cli_test.rb @@ -44,6 +44,39 @@ class CLITest < ActiveSupport::TestCase assert_raises(SolidObjects::Unauthorized) { command.prune_messages } end + # A role that only the standalone worker runs can depend on a constant that + # the caller path happens to load first. In this process everything is + # already loaded, so the failure is only reachable from a real worker. + test "a due reminder fires in the standalone worker" do + Dir.mktmpdir("solid-objects-reminder") do |directory| + completed = File.join(directory, "completed") + environment = worker_environment(directory, completed) + dummy_root = File.expand_path("../dummy", __dir__) + prepare(environment, dummy_root, "prepare_cli_reminder.rb") + + _input, output, error_output, wait_thread = Open3.popen3( + environment, + "bundle", "exec", "solid_objects", "start", + "--workers", "1", + "--reminder-schedulers", "1", + "--effect-workers", "0", + "--broadcast-workers", "0", + chdir: dummy_root + ) + fired = wait_for_file(completed, timeout: 20) + # The pipes only reach EOF once the process is gone, so reading them for + # the failure message has to wait until after it is stopped. + Process.kill("TERM", wait_thread.pid) if wait_thread.alive? + wait_thread.join + diagnosis = [ output.read, error_output.read ].join("\n") + + assert fired, "the scheduler should have enqueued the due reminder:\n#{diagnosis}" + ensure + Process.kill("TERM", wait_thread.pid) if wait_thread&.alive? + wait_thread&.join + end + end + # Deny-by-default means an unconfigured host hits this on its first CLI # command, so it is the most likely thing a new adopter ever sees. test "a denied command reports the policy rather than a backtrace" do @@ -121,4 +154,35 @@ class CLITest < ActiveSupport::TestCase wait_thread&.join end end + + private + + def worker_environment(directory, probe) + { + "BUNDLE_GEMFILE" => File.expand_path("../../Gemfile", __dir__), + "RAILS_ENV" => "test", + "SOLID_OBJECTS_CLI_WORKER_PROBE" => probe, + "SOLID_OBJECTS_DUMMY_DATABASE" => File.join(directory, "dummy.sqlite3") + } + end + + def prepare(environment, dummy_root, script) + _output, error_output, status = Open3.capture3( + environment, + Gem.ruby, + File.join(dummy_root, script), + chdir: dummy_root + ) + assert status.success?, error_output + end + + def wait_for_file(path, timeout:) + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout + until Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline + return true if File.exist?(path) + + sleep 0.1 + end + false + end end diff --git a/test/integration/load_contract_test.rb b/test/integration/load_contract_test.rb new file mode 100644 index 0000000..5014f1f --- /dev/null +++ b/test/integration/load_contract_test.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require "test_helper" +require "open3" + +# Runtime roles run in `solid_objects start`, a process that requires the gem +# and nothing else. A constant reached only through the caller path resolves +# fine in this test process, where everything is already loaded, and raises +# NameError in that worker. Asking a fresh process what `require +# "solid_objects"` actually defines is the only way to see the difference. +class LoadContractTest < ActiveSupport::TestCase + # Each of these is deliberately not loaded by requiring the gem. Anything + # else that stops being loaded is a role waiting to fail in production, so + # this list is the place to argue that a role never reaches it. + DEFERRED = { + "application_actor_loader" => "used only by the CLI", + "caller_process" => "the caller path, required by SolidObjects.caller_process", + "cli" => "loaded by exe/solid_objects, and pulls in thor", + "client" => "the caller path, required by SolidObjects.client", + "doctor" => "an operator tool, loaded by the doctor command", + "errors" => "defines error classes individually, so no SolidObjects::Errors exists", + "sync_diagnostics" => "the caller path, required with the client", + "synchronous_invocation" => "the caller path, required with the client", + "test_helper" => "opt-in, required by host application tests" + }.freeze + + test "requiring the gem defines everything a runtime role reaches for" do + assert_equal DEFERRED.keys.sort, undefined_after_require, + "a file that stopped being loaded is only safe if no runtime role reaches it" + end + + # The reported failure: two roles enqueue through the mailbox, and requiring + # the gem did not define it. + test "the mailbox is defined by requiring the gem" do + refute_includes undefined_after_require, "mailbox", + "the reminder scheduler and effect executor enqueue through it" + end + + private + + # Computed inside the fresh process, where both the file list and what the + # require actually defined are available. + def undefined_after_require + @undefined_after_require ||= begin + output, error_output, status = Open3.capture3( + { "BUNDLE_GEMFILE" => gem_root("Gemfile") }, + Gem.ruby, + "-e", + probe, + chdir: gem_root(".") + ) + assert status.success?, error_output + output.split("\n").sort + end + end + + def probe + <<~RUBY + require "solid_objects" + + def classify(path) + path.split("/").map { |part| part.split("_").map(&:capitalize).join }.join("::") + end + + undefined = Dir.glob("**/*.rb", base: "lib/solid_objects").filter_map do |path| + file = path.delete_suffix(".rb") + next if file == "version" + + begin + Object.const_get("SolidObjects::" + classify(file)) + nil + rescue NameError + file + end + end + puts undefined + RUBY + end + + def gem_root(path) + File.expand_path("../../#{path}", __dir__) + end +end From 9d26c921fe0b22efc4799acd2268e83a7a8bcdd7 Mon Sep 17 00:00:00 2001 From: Lucas Carlson Date: Mon, 10 Aug 2026 15:11:00 -0700 Subject: [PATCH 2/2] docs: record the two rules this bug class keeps breaking Every miss in recent work reduces to a test passing for a reason unrelated to the behavior: a constant already loaded by the test process, a double without the ensure its real collaborator runs, a mock ignoring the signal under test, a skipped adapter test reading as a pass. Two rules cover almost all of it, and both are checkable in review rather than a reminder to be careful. --- AGENTS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 4dbcac2..28c0a3a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,10 @@ Every owned Ruby file must enable inline RBS with `# rbs_inline: enabled`; annot Write Minitest files as `test/**/*_test.rb`. Start behavioral changes with a focused failing test. Exercise locking, leases, fencing, and claiming against real database adapters. Synchronize races with queues, barriers, or condition variables instead of arbitrary sleeps. Host-app actor tests should include `SolidObjects::TestHelper` rather than rely on transactional tests. +Watch the test fail before making it pass, and quote the observed failure in the pull request. A test that has never failed has not been shown to test anything, and the ways it can pass while proving nothing are not exotic: a regression test written after the fix, a double that lacks the `ensure` its real collaborator runs, a mock that ignores the signal under test, or a second guard that covers for the one being removed. When a change fixes a defect, revert the fix and confirm the test fails for the expected reason rather than some other one. + +Run code in the process and environment it actually runs in. A runtime role gets only what `require "solid_objects"` defines, so verify it through `solid_objects start` rather than in a test process that has already loaded the constant it needs; `test/integration/load_contract_test.rb` enforces that boundary and is the place to record a deliberate exception. Browser modules go through a real browser as well as jsdom. Adapter behavior goes through the real database and every supported client, and because a skipped test looks exactly like a passing one in the summary line, check the skip count when a change touches an adapter. + ## Commit & Pull Request Guidelines Use concise imperative subjects, preferably under 50 characters; use prefixes such as `fix:`, `ci:`, or `chore:`. Pull requests should explain API, correctness, security, migration, and compatibility effects; list exact validation commands; link issues; and include screenshots for UI or reactive ERB changes. Never bypass hooks or add AI attribution.