From 540a9bdf9707717e4dd461d63bec81b4a17470a7 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Thu, 12 Nov 2026 10:40:27 -0500 Subject: [PATCH 1/3] Invalidate cached numbered title fragments Signed-off-by: Hampton Lintorn-Catlin --- spec/requests/plan_content_editing_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/plan_content_editing_spec.rb b/spec/requests/plan_content_editing_spec.rb index fe68bb5f..9e128faf 100644 --- a/spec/requests/plan_content_editing_spec.rb +++ b/spec/requests/plan_content_editing_spec.rb @@ -41,7 +41,7 @@ renderer_version = CoPlan::MarkdownHelper::RENDER_CACHE_VERSION # Simulate a persistent fragment written by the renderer before title aliases. - stub_const("CoPlan::MarkdownHelper::RENDER_CACHE_VERSION", 24) + stub_const("CoPlan::MarkdownHelper::RENDER_CACHE_VERSION", 29) get content_body_plan_path(plan) expect(response).to have_http_status(:ok) fragment_key = written_keys.find { |key| key.to_s.include?("coplan/plan-content-body-with-title") } From 858f0ea3caa4bd08a3221f2360933660f8020c72 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Thu, 12 Nov 2026 17:21:37 -0500 Subject: [PATCH 2/3] Invalidate rendered content with bundle releases --- docs/HOST_APP_GUIDE.md | 26 ++++ engine/app/helpers/coplan/markdown_helper.rb | 6 - engine/app/helpers/coplan/plans_helper.rb | 4 +- .../app/helpers/coplan/references_helper.rb | 3 +- engine/app/services/coplan/render_cache.rb | 46 +++++++ .../views/coplan/comments/_comment.html.erb | 4 +- .../views/coplan/plans/_content_body.html.erb | 6 +- spec/helpers/plans_helper_spec.rb | 12 ++ spec/helpers/references_helper_spec.rb | 13 ++ spec/requests/comments_spec.rb | 8 +- spec/requests/plan_content_editing_spec.rb | 11 +- spec/services/render_cache_spec.rb | 124 ++++++++++++++++++ 12 files changed, 239 insertions(+), 24 deletions(-) create mode 100644 engine/app/services/coplan/render_cache.rb create mode 100644 spec/services/render_cache_spec.rb diff --git a/docs/HOST_APP_GUIDE.md b/docs/HOST_APP_GUIDE.md index 61465b60..b2a24eb0 100644 --- a/docs/HOST_APP_GUIDE.md +++ b/docs/HOST_APP_GUIDE.md @@ -4,6 +4,32 @@ Coplan is a Rails engine that manages collaborative planning documents. It owns its own `CoPlan::User` model and handles authentication internally via a callback you configure. +## Rendered-content caching + +Rendered plan bodies, comments, previews, and citations use the installed +bundle as their cache namespace. Coplan hashes `Gemfile.lock` once per process. +An engine gem upgrade, a Git pin change, or a rendering dependency upgrade +automatically starts a new namespace, even when document content is unchanged. +Git installs use the revision in the lockfile; no Git command or checkout +directory name is involved. Without Bundler, the installed engine gem version +is the namespace. + +Rails' standard `RAILS_APP_VERSION` or `RAILS_CACHE_ID` also prefixes these +keys. Set it to the build or release ID when host code changes rendering +without changing the bundle, such as an overridden helper or template. + +Bundles with editable path dependencies cannot identify a release from their +lockfile alone. They bypass these rendered-content caches unless the host +sets `RAILS_APP_VERSION` or `RAILS_CACHE_ID`. This includes this repository's +path-based example host. Development code reloading always bypasses them, so +Ruby and template edits appear immediately. Other application caches keep +their normal behavior. + +Upgrading from the old numeric renderer version ignores those older entries; +no cache flush or data deletion is needed. Old entries expire under the host's +normal cache policy. An unchanged release can reuse entries across processes +and restarts; a changed bundle fills its own entries lazily on first use. + ## User settings and host integrations Coplan owns the settings shown on its Settings page. The app theme, light/dark diff --git a/engine/app/helpers/coplan/markdown_helper.rb b/engine/app/helpers/coplan/markdown_helper.rb index f9dd4c00..178b18c5 100644 --- a/engine/app/helpers/coplan/markdown_helper.rb +++ b/engine/app/helpers/coplan/markdown_helper.rb @@ -42,12 +42,6 @@ module MarkdownHelper # (ordered-list or blockquoted tasks) stay disabled. TASK_LINE_PATTERN = /\A\s*[*+-]\s+\[[ xX]\]\s/ - # Fragment caches of rendered markdown are keyed on content SHA plus this - # version. Bump it whenever the rendering pipeline changes output for the - # same input (new tags, attribute changes, checkbox wiring, etc.), or - # stale HTML will be served from cache. - RENDER_CACHE_VERSION = 32 - # Matches `[@username](mention:username)` where the bracket text and link # target encode the same username. Username allows letters, digits, dots, # dashes, and underscores. The pattern must round-trip exactly so that diff --git a/engine/app/helpers/coplan/plans_helper.rb b/engine/app/helpers/coplan/plans_helper.rb index 97a4a35b..a8b5a0a9 100644 --- a/engine/app/helpers/coplan/plans_helper.rb +++ b/engine/app/helpers/coplan/plans_helper.rb @@ -159,9 +159,9 @@ def plan_content_preview(plan, limit: 200) # full Commonmarker + Nokogiri parse per plan without an AI summary. # The document body itself is fetched only on a cache miss — list pages # preload just the stub (id + sha), never the MEDIUMTEXT columns. - cache_key = [ "coplan/plan-preview", MarkdownHelper::RENDER_CACHE_VERSION, plan.id, + cache_key = [ "coplan/plan-preview", plan.id, stub.content_sha256 || plan.current_revision ] - plain = Rails.cache.fetch(cache_key) do + plain = RenderCache.fetch(cache_key) do markdown_to_plain_text(PlanVersion.where(id: stub.id).pick(:content_markdown)) end return nil if plain.blank? diff --git a/engine/app/helpers/coplan/references_helper.rb b/engine/app/helpers/coplan/references_helper.rb index bd323845..216ced82 100644 --- a/engine/app/helpers/coplan/references_helper.rb +++ b/engine/app/helpers/coplan/references_helper.rb @@ -41,9 +41,8 @@ def plan_citation_back_matter(plan, references) references.map { |reference| "#{reference.id}:#{reference.updated_at&.to_f}" }.join("|") ) signature = [ plan.current_revision, reference_digest ] - cached = Rails.cache.fetch([ + cached = RenderCache.fetch([ "coplan/plan-citation-back-matter", - MarkdownHelper::RENDER_CACHE_VERSION, plan.id, *signature ]) do diff --git a/engine/app/services/coplan/render_cache.rb b/engine/app/services/coplan/render_cache.rb new file mode 100644 index 00000000..91579b05 --- /dev/null +++ b/engine/app/services/coplan/render_cache.rb @@ -0,0 +1,46 @@ +require "digest" + +module CoPlan + # Rendered HTML belongs to the installed bundle, just as assets belong to + # a build. A lockfile records Git revisions and all gem versions, including + # the renderer's dependencies. No source scan or Git checkout is needed. + class RenderCache + def self.version + return if Rails.application.config.reloading_enabled? + + @version ||= begin + if defined?(Bundler) && Bundler.default_lockfile.file? + # Path sources can change without changing the lockfile. Only an + # explicit host release ID makes them safe to cache persistently. + mutable = Bundler.locked_gems.sources.any? { |source| mutable_source?(source) } + return if mutable && application_version.blank? + + Digest::SHA256.file(Bundler.default_lockfile).hexdigest + else + spec = Gem.loaded_specs.fetch("coplan-engine") + return if spec.respond_to?(:source) && mutable_source?(spec.source) && application_version.blank? + + "coplan-engine-#{spec.version}" + end + end + end + + def self.fetch(key, &block) + return yield unless (release = version) + + # Use Rails' standard RAILS_CACHE_ID / RAILS_APP_VERSION prefix too. + # The view cache helper already adds this prefix to fragment keys. + Rails.cache.fetch(ActiveSupport::Cache.expand_cache_key([ "coplan/render", release, *key ]), &block) + end + + def self.application_version + ENV["RAILS_CACHE_ID"].presence || ENV["RAILS_APP_VERSION"].presence + end + private_class_method :application_version + + def self.mutable_source?(source) + defined?(Bundler::Source::Path) && source.is_a?(Bundler::Source::Path) && !source.is_a?(Bundler::Source::Git) + end + private_class_method :mutable_source? + end +end diff --git a/engine/app/views/coplan/comments/_comment.html.erb b/engine/app/views/coplan/comments/_comment.html.erb index 96a592f8..0162e6bc 100644 --- a/engine/app/views/coplan/comments/_comment.html.erb +++ b/engine/app/views/coplan/comments/_comment.html.erb @@ -29,8 +29,8 @@ <%= time_ago_in_words(comment.created_at) %> ago<% if comment.updated_at > comment.created_at %> · edited<% end %>
- <%# The record version invalidates edits; the renderer version invalidates cached HTML. %> - <%= cache(["comment-body-v2", CoPlan::MarkdownHelper::RENDER_CACHE_VERSION, comment], skip_digest: true) do %> + <%# The record version invalidates edits; the bundle release invalidates renderer changes. %> + <%= cache_if(CoPlan::RenderCache.version.present?, ["comment-body-v2", CoPlan::RenderCache.version, comment], skip_digest: true) do %> <%# footnote_prefix keeps footnote ids unique per comment — the plan body and every other comment render in the same DOM. Comment images don't expand: the comments panel and footnote previews diff --git a/engine/app/views/coplan/plans/_content_body.html.erb b/engine/app/views/coplan/plans/_content_body.html.erb index d316d49c..29d4269f 100644 --- a/engine/app/views/coplan/plans/_content_body.html.erb +++ b/engine/app/views/coplan/plans/_content_body.html.erb @@ -5,8 +5,8 @@ Cached per content SHA: rendering is Commonmarker + two Nokogiri passes + sanitize, and this partial is re-rendered on every show AND every content-mutation broadcast. skip_digest avoids the template-digest cost - on write paths; the explicit RENDER_CACHE_VERSION key handles renderer - changes instead. -%> -<% cache [CoPlan::EmbedDomain.order(:hostname).pluck(:hostname), request&.host, "coplan/plan-content-body-with-title", CoPlan::MarkdownHelper::RENDER_CACHE_VERSION, plan.id, plan.current_plan_version&.content_sha256 || plan.current_revision], skip_digest: true do %> + on write paths; the bundle release namespace handles renderer and template + changes. Editable installs bypass this cache without a host release ID. -%> +<% cache_if CoPlan::RenderCache.version.present?, [CoPlan::EmbedDomain.order(:hostname).pluck(:hostname), request&.host, "coplan/plan-content-body-with-title", CoPlan::RenderCache.version, plan.id, plan.current_plan_version&.content_sha256 || plan.current_revision], skip_digest: true do %> <%= render_plan_document(plan.current_content) %> <% end %> diff --git a/spec/helpers/plans_helper_spec.rb b/spec/helpers/plans_helper_spec.rb index cea91907..d66c3ea5 100644 --- a/spec/helpers/plans_helper_spec.rb +++ b/spec/helpers/plans_helper_spec.rb @@ -16,6 +16,18 @@ describe "#plan_content_preview" do let(:plan) { create(:plan, :considering) } + it "reuses previews until the installed bundle changes" do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-a") + allow(helper).to receive(:markdown_to_plain_text).and_return("First rendering", "New rendering") + + expect(helper.plan_content_preview(plan)).to eq("First rendering") + expect(helper.plan_content_preview(plan)).to eq("First rendering") + allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-b") + expect(helper.plan_content_preview(plan)).to eq("New rendering") + expect(helper).to have_received(:markdown_to_plain_text).twice + end + it "strips markdown formatting and returns a plain-text preview" do set_version_fixture(plan.current_plan_version, content_markdown: "# Heading\n\nA **bold** intro with [a link](https://example.com)." diff --git a/spec/helpers/references_helper_spec.rb b/spec/helpers/references_helper_spec.rb index 1d6042bc..e32ee75c 100644 --- a/spec/helpers/references_helper_spec.rb +++ b/spec/helpers/references_helper_spec.rb @@ -20,6 +20,19 @@ plan end + it "reuses citations until the installed bundle changes" do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-a") + allow(helper).to receive(:render_markdown).and_call_original + + original = helper.plan_citation_back_matter(plan, plan.references) + expect(helper.plan_citation_back_matter(plan, plan.references)).to eq(original) + expect(helper).to have_received(:render_markdown).once + allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-b") + expect(helper.plan_citation_back_matter(plan, plan.references)).to eq(original) + expect(helper).to have_received(:render_markdown).twice + end + it "joins Markdown citation context to structured source identity" do reference = plan.references.find_by!(url: "https://docs.google.com/spreadsheets/d/sample") reference.update!(title: "Rollout experiment results") diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb index fd45d8c9..1328ba66 100644 --- a/spec/requests/comments_spec.rb +++ b/spec/requests/comments_spec.rb @@ -48,6 +48,7 @@ it "does not calculate a template digest while broadcasting a reply" do original_perform_caching = ActionController::Base.perform_caching ActionController::Base.perform_caching = true + allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-release") allow(ActionView::Digestor).to receive(:digest).and_call_original allow(Rails.cache).to receive(:read).and_call_original @@ -61,7 +62,7 @@ ActionController::Base.perform_caching = original_perform_caching end - it "refreshes cached comment links when the Markdown renderer version changes" do + it "refreshes cached comment links when the bundle release changes" do comment = create(:comment, comment_thread: thread_record, author_id: alice.id, body_markdown: "[Open chat](codex://threads/123)") original_caching = CoPlan::ApplicationController.perform_caching @@ -69,18 +70,17 @@ memory = ActiveSupport::Cache::MemoryStore.new allow(Rails).to receive(:cache).and_return(memory) allow_any_instance_of(CoPlan::ApplicationController).to receive(:cache_store).and_return(memory) - renderer_version = CoPlan::MarkdownHelper::RENDER_CACHE_VERSION link_scrubber = CoPlan::LinkScrubber render_comment = -> { CoPlan::ApplicationController.render(partial: "coplan/comments/comment", locals: { comment: comment }) } - stub_const("CoPlan::MarkdownHelper::RENDER_CACHE_VERSION", renderer_version - 1) + allow(CoPlan::RenderCache).to receive(:version).and_return("old-bundle") stub_const("CoPlan::LinkScrubber", Rails::HTML::PermitScrubber) expect(render_comment.call).not_to include('href="codex://threads/123"') stub_const("CoPlan::LinkScrubber", link_scrubber) # The old renderer's cached fragment really is still being served. expect(render_comment.call).not_to include('href="codex://threads/123"') - stub_const("CoPlan::MarkdownHelper::RENDER_CACHE_VERSION", renderer_version) + allow(CoPlan::RenderCache).to receive(:version).and_return("new-bundle") expect(render_comment.call).to include('href="codex://threads/123"') ensure CoPlan::ApplicationController.perform_caching = original_caching diff --git a/spec/requests/plan_content_editing_spec.rb b/spec/requests/plan_content_editing_spec.rb index 9e128faf..5d2eeadf 100644 --- a/spec/requests/plan_content_editing_spec.rb +++ b/spec/requests/plan_content_editing_spec.rb @@ -38,10 +38,9 @@ written_keys << key write.call(key, *args, **options) end - renderer_version = CoPlan::MarkdownHelper::RENDER_CACHE_VERSION - - # Simulate a persistent fragment written by the renderer before title aliases. - stub_const("CoPlan::MarkdownHelper::RENDER_CACHE_VERSION", 29) + # Both deployed and candidate renderers used version 29. Reproduce the + # real persistent entry, then migrate to a release namespace with no edit. + allow(CoPlan::RenderCache).to receive(:version).and_return(29) get content_body_plan_path(plan) expect(response).to have_http_status(:ok) fragment_key = written_keys.find { |key| key.to_s.include?("coplan/plan-content-body-with-title") } @@ -51,10 +50,12 @@ get content_body_plan_path(plan) expect(Nokogiri::HTML.fragment(response.body).at_css("#section-1")).to be_nil - stub_const("CoPlan::MarkdownHelper::RENDER_CACHE_VERSION", renderer_version) + allow(CoPlan::RenderCache).to receive(:version).and_return("candidate-bundle") get content_body_plan_path(plan) expect(response).to have_http_status(:ok) expect(Nokogiri::HTML.fragment(response.body).at_css("h1#plan-title #section-1")).to be_present + expect(cache.read(fragment_key)).to eq(stale_html) + expect(written_keys.uniq.size).to eq(2) end it "returns the latest rendered body and revision without caching" do diff --git a/spec/services/render_cache_spec.rb b/spec/services/render_cache_spec.rb new file mode 100644 index 00000000..4379950e --- /dev/null +++ b/spec/services/render_cache_spec.rb @@ -0,0 +1,124 @@ +require "rails_helper" +require "tmpdir" + +RSpec.describe CoPlan::RenderCache do + # Each instance represents a separate boot of a release, with its own memo. + let(:release) { Class.new(described_class) } + + around do |example| + original = ENV.to_h.slice("RAILS_CACHE_ID", "RAILS_APP_VERSION") + ENV.delete("RAILS_CACHE_ID") + ENV.delete("RAILS_APP_VERSION") + Dir.mktmpdir do |directory| + @lockfile = Pathname.new(directory).join("Gemfile.lock") + example.run + end + ensure + ENV.delete("RAILS_CACHE_ID") + ENV.delete("RAILS_APP_VERSION") + original.each { |name, value| ENV[name] = value } + end + + before do + allow(Rails.application.config).to receive(:reloading_enabled?).and_return(false) + allow(Bundler).to receive(:default_lockfile).and_return(@lockfile) + allow(Bundler).to receive(:locked_gems) { Bundler::LockfileParser.new(@lockfile.read) } + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + end + + def write_lock(version: "1.1.0", revision: nil, path: nil, parser_version: "2.4.0") + engine = " coplan-engine (#{version})\n" + source = if revision + "GIT\n remote: https://github.com/block/coplan.git\n revision: #{revision}\n specs:\n#{engine}\n" + elsif path + "PATH\n remote: #{path}\n specs:\n#{engine}\n" + else + "" + end + gems = "GEM\n remote: https://rubygems.org/\n specs:\n" + gems += engine unless revision || path + gems += " commonmarker (#{parser_version})\n\nDEPENDENCIES\n coplan-engine#{'!' if revision || path}\n" + @lockfile.write(source + gems) + end + + it "reuses rendered output for the same installed gem release without a host variable" do + write_lock + expect(release.fetch([ "example", "content-sha" ]) { "first render" }).to eq("first render") + expect(Class.new(described_class).fetch([ "example", "content-sha" ]) { "second render" }).to eq("first render") + end + + it "invalidates persistent output when the installed engine gem version changes" do + write_lock + release.fetch([ "example", "unchanged-content" ]) { "old HTML" } + write_lock(version: "1.2.0") + expect(Class.new(described_class).fetch([ "example", "unchanged-content" ]) { "new HTML" }).to eq("new HTML") + end + + it "invalidates Git-pinned engine updates even when the gem version stays the same" do + write_lock(revision: "b40c2c32681e1ad4d7fadbb80392510d90cb08e2") + release.fetch([ "example", "unchanged-content" ]) { "deployed HTML" } + write_lock(revision: "f2693895b48fab820c579728715ce90daaf0dd1a") + expect(Class.new(described_class).fetch([ "example", "unchanged-content" ]) { "candidate HTML" }).to eq("candidate HTML") + end + + it "does not ask Git for a revision or depend on an installed checkout directory name" do + write_lock(revision: "f2693895b48fab820c579728715ce90daaf0dd1a") + expect_any_instance_of(Bundler::Source::Git).not_to receive(:revision) + expect(release.version).to be_present + end + + it "invalidates output when a rendering dependency changes" do + write_lock + original = release.version + write_lock(parser_version: "2.5.0") + expect(Class.new(described_class).version).not_to eq(original) + end + + it "does not read the lockfile again on later lookups in the same process" do + write_lock + original = release.version + expect(Digest::SHA256).not_to receive(:file) + expect(release.version).to equal(original) + end + + it "bypasses rendered caches for editable path installs without a release ID" do + write_lock(path: "engine") + expect(release.version).to be_nil + expect(Rails.cache).not_to receive(:fetch) + expect(release.fetch([ "example" ]) { "first render" }).to eq("first render") + expect(release.fetch([ "example" ]) { "updated renderer" }).to eq("updated renderer") + end + + it "also bypasses caches when another dependency is an editable path source" do + write_lock + @lockfile.write("PATH\n remote: local-renderer\n specs:\n custom-renderer (1.0)\n\n" + @lockfile.read) + expect(release.version).to be_nil + end + + %w[RAILS_APP_VERSION RAILS_CACHE_ID].each do |name| + it "uses Rails' #{name} namespace for path-based production releases" do + write_lock(path: "engine") + ENV[name] = "release-a" + release.fetch([ "example" ]) { "release A" } + expect(release.fetch([ "example" ]) { "cached" }).to eq("release A") + ENV[name] = "release-b" + expect(Class.new(described_class).fetch([ "example" ]) { "release B" }).to eq("release B") + end + end + + it "does not cache while development reloading is enabled, even with a release ID" do + write_lock + ENV["RAILS_APP_VERSION"] = "release-a" + allow(Rails.application.config).to receive(:reloading_enabled?).and_return(true) + expect(release.version).to be_nil + expect(Rails.cache).not_to receive(:fetch) + expect(release.fetch([ "example" ]) { "fresh HTML" }).to eq("fresh HTML") + end + + it "falls back to the installed gem version without Bundler or a lockfile" do + hide_const("Bundler") + gem = Gem::Specification.new { |spec| spec.name = "coplan-engine"; spec.version = "1.2.0" } + allow(Gem).to receive(:loaded_specs).and_return("coplan-engine" => gem) + expect(release.version).to eq("coplan-engine-1.2.0") + end +end From cc3b3413176a8ba4b5f5d10f1f6604a3669edd01 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Thu, 12 Nov 2026 18:23:15 -0500 Subject: [PATCH 3/3] Use standard Rails caching with expiry --- docs/HOST_APP_GUIDE.md | 28 +--- engine/app/helpers/coplan/plans_helper.rb | 2 +- .../app/helpers/coplan/references_helper.rb | 4 +- engine/app/services/coplan/render_cache.rb | 46 ------- .../views/coplan/comments/_comment.html.erb | 3 +- .../views/coplan/plans/_content_body.html.erb | 5 +- spec/helpers/plans_helper_spec.rb | 9 +- spec/helpers/references_helper_spec.rb | 9 +- spec/requests/comments_spec.rb | 15 ++- spec/requests/plan_content_editing_spec.rb | 38 +++--- spec/services/render_cache_spec.rb | 124 ------------------ 11 files changed, 54 insertions(+), 229 deletions(-) delete mode 100644 engine/app/services/coplan/render_cache.rb delete mode 100644 spec/services/render_cache_spec.rb diff --git a/docs/HOST_APP_GUIDE.md b/docs/HOST_APP_GUIDE.md index b2a24eb0..675acd52 100644 --- a/docs/HOST_APP_GUIDE.md +++ b/docs/HOST_APP_GUIDE.md @@ -6,29 +6,11 @@ Coplan is a Rails engine that manages collaborative planning documents. It owns ## Rendered-content caching -Rendered plan bodies, comments, previews, and citations use the installed -bundle as their cache namespace. Coplan hashes `Gemfile.lock` once per process. -An engine gem upgrade, a Git pin change, or a rendering dependency upgrade -automatically starts a new namespace, even when document content is unchanged. -Git installs use the revision in the lockfile; no Git command or checkout -directory name is involved. Without Bundler, the installed engine gem version -is the namespace. - -Rails' standard `RAILS_APP_VERSION` or `RAILS_CACHE_ID` also prefixes these -keys. Set it to the build or release ID when host code changes rendering -without changing the bundle, such as an overridden helper or template. - -Bundles with editable path dependencies cannot identify a release from their -lockfile alone. They bypass these rendered-content caches unless the host -sets `RAILS_APP_VERSION` or `RAILS_CACHE_ID`. This includes this repository's -path-based example host. Development code reloading always bypasses them, so -Ruby and template edits appear immediately. Other application caches keep -their normal behavior. - -Upgrading from the old numeric renderer version ignores those older entries; -no cache flush or data deletion is needed. Old entries expire under the host's -normal cache policy. An unchanged release can reuse entries across processes -and restarts; a changed bundle fills its own entries lazily on first use. +Rendered plan bodies, comments, previews, and citations use standard Rails +caching with a 15-minute expiry. Content edits change the cache key immediately. +Renderer code changes appear as entries expire; no version bump or release +configuration is required. The host's normal cache store and fragment-caching +settings apply. ## User settings and host integrations diff --git a/engine/app/helpers/coplan/plans_helper.rb b/engine/app/helpers/coplan/plans_helper.rb index a8b5a0a9..efce52e0 100644 --- a/engine/app/helpers/coplan/plans_helper.rb +++ b/engine/app/helpers/coplan/plans_helper.rb @@ -161,7 +161,7 @@ def plan_content_preview(plan, limit: 200) # preload just the stub (id + sha), never the MEDIUMTEXT columns. cache_key = [ "coplan/plan-preview", plan.id, stub.content_sha256 || plan.current_revision ] - plain = RenderCache.fetch(cache_key) do + plain = Rails.cache.fetch(cache_key, expires_in: 15.minutes) do markdown_to_plain_text(PlanVersion.where(id: stub.id).pick(:content_markdown)) end return nil if plain.blank? diff --git a/engine/app/helpers/coplan/references_helper.rb b/engine/app/helpers/coplan/references_helper.rb index 216ced82..7e81eb19 100644 --- a/engine/app/helpers/coplan/references_helper.rb +++ b/engine/app/helpers/coplan/references_helper.rb @@ -41,11 +41,11 @@ def plan_citation_back_matter(plan, references) references.map { |reference| "#{reference.id}:#{reference.updated_at&.to_f}" }.join("|") ) signature = [ plan.current_revision, reference_digest ] - cached = RenderCache.fetch([ + cached = Rails.cache.fetch([ "coplan/plan-citation-back-matter", plan.id, *signature - ]) do + ], expires_in: 15.minutes) do result = build_plan_citation_back_matter(plan, references) [ result[:html].to_s, result[:cited_urls].to_a, result[:count] ] end diff --git a/engine/app/services/coplan/render_cache.rb b/engine/app/services/coplan/render_cache.rb deleted file mode 100644 index 91579b05..00000000 --- a/engine/app/services/coplan/render_cache.rb +++ /dev/null @@ -1,46 +0,0 @@ -require "digest" - -module CoPlan - # Rendered HTML belongs to the installed bundle, just as assets belong to - # a build. A lockfile records Git revisions and all gem versions, including - # the renderer's dependencies. No source scan or Git checkout is needed. - class RenderCache - def self.version - return if Rails.application.config.reloading_enabled? - - @version ||= begin - if defined?(Bundler) && Bundler.default_lockfile.file? - # Path sources can change without changing the lockfile. Only an - # explicit host release ID makes them safe to cache persistently. - mutable = Bundler.locked_gems.sources.any? { |source| mutable_source?(source) } - return if mutable && application_version.blank? - - Digest::SHA256.file(Bundler.default_lockfile).hexdigest - else - spec = Gem.loaded_specs.fetch("coplan-engine") - return if spec.respond_to?(:source) && mutable_source?(spec.source) && application_version.blank? - - "coplan-engine-#{spec.version}" - end - end - end - - def self.fetch(key, &block) - return yield unless (release = version) - - # Use Rails' standard RAILS_CACHE_ID / RAILS_APP_VERSION prefix too. - # The view cache helper already adds this prefix to fragment keys. - Rails.cache.fetch(ActiveSupport::Cache.expand_cache_key([ "coplan/render", release, *key ]), &block) - end - - def self.application_version - ENV["RAILS_CACHE_ID"].presence || ENV["RAILS_APP_VERSION"].presence - end - private_class_method :application_version - - def self.mutable_source?(source) - defined?(Bundler::Source::Path) && source.is_a?(Bundler::Source::Path) && !source.is_a?(Bundler::Source::Git) - end - private_class_method :mutable_source? - end -end diff --git a/engine/app/views/coplan/comments/_comment.html.erb b/engine/app/views/coplan/comments/_comment.html.erb index 0162e6bc..d2f57500 100644 --- a/engine/app/views/coplan/comments/_comment.html.erb +++ b/engine/app/views/coplan/comments/_comment.html.erb @@ -29,8 +29,7 @@ <%= time_ago_in_words(comment.created_at) %> ago<% if comment.updated_at > comment.created_at %> · edited<% end %>
- <%# The record version invalidates edits; the bundle release invalidates renderer changes. %> - <%= cache_if(CoPlan::RenderCache.version.present?, ["comment-body-v2", CoPlan::RenderCache.version, comment], skip_digest: true) do %> + <%= cache(["comment-body-v2", comment], expires_in: 15.minutes, skip_digest: true) do %> <%# footnote_prefix keeps footnote ids unique per comment — the plan body and every other comment render in the same DOM. Comment images don't expand: the comments panel and footnote previews diff --git a/engine/app/views/coplan/plans/_content_body.html.erb b/engine/app/views/coplan/plans/_content_body.html.erb index 29d4269f..8eaf612b 100644 --- a/engine/app/views/coplan/plans/_content_body.html.erb +++ b/engine/app/views/coplan/plans/_content_body.html.erb @@ -5,8 +5,7 @@ Cached per content SHA: rendering is Commonmarker + two Nokogiri passes + sanitize, and this partial is re-rendered on every show AND every content-mutation broadcast. skip_digest avoids the template-digest cost - on write paths; the bundle release namespace handles renderer and template - changes. Editable installs bypass this cache without a host release ID. -%> -<% cache_if CoPlan::RenderCache.version.present?, [CoPlan::EmbedDomain.order(:hostname).pluck(:hostname), request&.host, "coplan/plan-content-body-with-title", CoPlan::RenderCache.version, plan.id, plan.current_plan_version&.content_sha256 || plan.current_revision], skip_digest: true do %> + on write paths; expiry picks up renderer changes within 15 minutes. -%> +<% cache [CoPlan::EmbedDomain.order(:hostname).pluck(:hostname), request&.host, "coplan/plan-content-body-with-title", plan.id, plan.current_plan_version&.content_sha256 || plan.current_revision], expires_in: 15.minutes, skip_digest: true do %> <%= render_plan_document(plan.current_content) %> <% end %> diff --git a/spec/helpers/plans_helper_spec.rb b/spec/helpers/plans_helper_spec.rb index d66c3ea5..672b00cb 100644 --- a/spec/helpers/plans_helper_spec.rb +++ b/spec/helpers/plans_helper_spec.rb @@ -1,6 +1,10 @@ require "rails_helper" RSpec.describe CoPlan::PlansHelper, type: :helper do + include ActiveSupport::Testing::TimeHelpers + + after { travel_back } + describe "#render_plan_document" do before { helper.extend(CoPlan::SlideshowsHelper) } it "preserves the numbered title fragment alongside the canonical title anchor" do @@ -16,14 +20,13 @@ describe "#plan_content_preview" do let(:plan) { create(:plan, :considering) } - it "reuses previews until the installed bundle changes" do + it "reuses previews for 15 minutes" do allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) - allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-a") allow(helper).to receive(:markdown_to_plain_text).and_return("First rendering", "New rendering") expect(helper.plan_content_preview(plan)).to eq("First rendering") expect(helper.plan_content_preview(plan)).to eq("First rendering") - allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-b") + travel 15.minutes + 1.second expect(helper.plan_content_preview(plan)).to eq("New rendering") expect(helper).to have_received(:markdown_to_plain_text).twice end diff --git a/spec/helpers/references_helper_spec.rb b/spec/helpers/references_helper_spec.rb index e32ee75c..8dbba114 100644 --- a/spec/helpers/references_helper_spec.rb +++ b/spec/helpers/references_helper_spec.rb @@ -1,6 +1,10 @@ require "rails_helper" RSpec.describe CoPlan::ReferencesHelper, type: :helper do + include ActiveSupport::Testing::TimeHelpers + + after { travel_back } + describe "#plan_citation_back_matter" do let(:user) { create(:coplan_user) } let(:plan) do @@ -20,15 +24,14 @@ plan end - it "reuses citations until the installed bundle changes" do + it "reuses citations for 15 minutes" do allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) - allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-a") allow(helper).to receive(:render_markdown).and_call_original original = helper.plan_citation_back_matter(plan, plan.references) expect(helper.plan_citation_back_matter(plan, plan.references)).to eq(original) expect(helper).to have_received(:render_markdown).once - allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-b") + travel 15.minutes + 1.second expect(helper.plan_citation_back_matter(plan, plan.references)).to eq(original) expect(helper).to have_received(:render_markdown).twice end diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb index 1328ba66..84664f7a 100644 --- a/spec/requests/comments_spec.rb +++ b/spec/requests/comments_spec.rb @@ -1,6 +1,10 @@ require "rails_helper" RSpec.describe "Comments", type: :request do + include ActiveSupport::Testing::TimeHelpers + + after { travel_back } + let(:alice) { create(:coplan_user, :admin) } let(:plan) { create(:plan, :considering, created_by_user: alice) } let(:thread_record) { create(:comment_thread, plan: plan, plan_version: plan.current_plan_version, created_by_user: alice) } @@ -48,7 +52,6 @@ it "does not calculate a template digest while broadcasting a reply" do original_perform_caching = ActionController::Base.perform_caching ActionController::Base.perform_caching = true - allow(CoPlan::RenderCache).to receive(:version).and_return("bundle-release") allow(ActionView::Digestor).to receive(:digest).and_call_original allow(Rails.cache).to receive(:read).and_call_original @@ -62,7 +65,7 @@ ActionController::Base.perform_caching = original_perform_caching end - it "refreshes cached comment links when the bundle release changes" do + it "refreshes cached comment links after 15 minutes" do comment = create(:comment, comment_thread: thread_record, author_id: alice.id, body_markdown: "[Open chat](codex://threads/123)") original_caching = CoPlan::ApplicationController.perform_caching @@ -73,14 +76,13 @@ link_scrubber = CoPlan::LinkScrubber render_comment = -> { CoPlan::ApplicationController.render(partial: "coplan/comments/comment", locals: { comment: comment }) } - allow(CoPlan::RenderCache).to receive(:version).and_return("old-bundle") stub_const("CoPlan::LinkScrubber", Rails::HTML::PermitScrubber) expect(render_comment.call).not_to include('href="codex://threads/123"') stub_const("CoPlan::LinkScrubber", link_scrubber) # The old renderer's cached fragment really is still being served. expect(render_comment.call).not_to include('href="codex://threads/123"') - allow(CoPlan::RenderCache).to receive(:version).and_return("new-bundle") + travel 15.minutes + 1.second expect(render_comment.call).to include('href="codex://threads/123"') ensure CoPlan::ApplicationController.perform_caching = original_caching @@ -104,7 +106,10 @@ def update_comment(body = "Revised **answer**") headers: { "Accept" => "text/vnd.turbo-stream.html" } end - it "updates the author's comment and broadcasts a requestless replacement" do + it "updates the author's cached comment and broadcasts a requestless replacement" do + allow_any_instance_of(CoPlan::ApplicationController).to receive(:perform_caching).and_return(true) + allow_any_instance_of(CoPlan::ApplicationController).to receive(:cache_store).and_return(ActiveSupport::Cache::MemoryStore.new) + expect(CoPlan::ApplicationController.render(partial: "coplan/comments/comment", locals: { comment: comment })).to include("Original") allow(CoPlan::Broadcaster).to receive(:replace_to) expect(CoPlan::Broadcaster).to receive(:replace_to).with(plan, target: ActionView::RecordIdentifier.dom_id(comment), partial: "coplan/comments/comment", locals: { comment: comment }) diff --git a/spec/requests/plan_content_editing_spec.rb b/spec/requests/plan_content_editing_spec.rb index 5d2eeadf..7ef6e7b5 100644 --- a/spec/requests/plan_content_editing_spec.rb +++ b/spec/requests/plan_content_editing_spec.rb @@ -1,6 +1,10 @@ require "rails_helper" RSpec.describe "Plan content editing (web UI)", type: :request do + include ActiveSupport::Testing::TimeHelpers + + after { travel_back } + let(:author) { create(:coplan_user) } let(:other_user) { create(:coplan_user) } let(:plan) { create(:plan, :considering, created_by_user: author) } @@ -28,34 +32,26 @@ end describe "GET content_body" do - it "invalidates pre-alias rendered fragments without changing the plan" do + it "refreshes stale rendered fragments after 15 minutes without changing the plan" do set_version_fixture(plan.current_plan_version, content_markdown: "# 1 Overview\n\nOpening text.\n") cache = ActiveSupport::Cache::MemoryStore.new allow_any_instance_of(CoPlan::PlansController).to receive(:perform_caching).and_return(true) allow_any_instance_of(CoPlan::PlansController).to receive(:cache_store).and_return(cache) - written_keys = [] - allow(cache).to receive(:write).and_wrap_original do |write, key, *args, **options| - written_keys << key - write.call(key, *args, **options) + # Simulate an older renderer's output, retaining the real cache expiry. + strip_alias = true + allow(cache).to receive(:write).and_wrap_original do |write, key, value, options| + value = value.gsub(/]*data-heading-alias[^>]*><\/span>/, "") if strip_alias + write.call(key, value, options) end - # Both deployed and candidate renderers used version 29. Reproduce the - # real persistent entry, then migrate to a release namespace with no edit. - allow(CoPlan::RenderCache).to receive(:version).and_return(29) get content_body_plan_path(plan) - expect(response).to have_http_status(:ok) - fragment_key = written_keys.find { |key| key.to_s.include?("coplan/plan-content-body-with-title") } - expect(fragment_key).to be_present - stale_html = response.body.gsub(/]*data-heading-alias[^>]*><\/span>/, "") - cache.write(fragment_key, stale_html) + strip_alias = false get content_body_plan_path(plan) expect(Nokogiri::HTML.fragment(response.body).at_css("#section-1")).to be_nil - allow(CoPlan::RenderCache).to receive(:version).and_return("candidate-bundle") + travel 15.minutes + 1.second get content_body_plan_path(plan) expect(response).to have_http_status(:ok) expect(Nokogiri::HTML.fragment(response.body).at_css("h1#plan-title #section-1")).to be_present - expect(cache.read(fragment_key)).to eq(stale_html) - expect(written_keys.uniq.size).to eq(2) end it "returns the latest rendered body and revision without caching" do @@ -93,7 +89,12 @@ end describe "PATCH update_content" do - it "creates a new human-authored version through ReplaceContent" do + it "creates a new human-authored version and immediately refreshes cached content" do + allow_any_instance_of(CoPlan::PlansController).to receive(:perform_caching).and_return(true) + allow_any_instance_of(CoPlan::PlansController).to receive(:cache_store).and_return(ActiveSupport::Cache::MemoryStore.new) + get content_body_plan_path(plan) + expect(response.body).to include("Original body.") + expect { patch update_content_plan_path(plan), params: { content: "# Plan\n\nEdited body.\n", @@ -109,6 +110,9 @@ expect(version.actor_type).to eq("human") expect(version.actor_id).to eq(author.id) expect(version.change_summary).to eq("Tightened wording") + get content_body_plan_path(plan) + expect(response.body).to include("Edited body.") + expect(response.body).not_to include("Original body.") end it "defaults the change summary" do diff --git a/spec/services/render_cache_spec.rb b/spec/services/render_cache_spec.rb deleted file mode 100644 index 4379950e..00000000 --- a/spec/services/render_cache_spec.rb +++ /dev/null @@ -1,124 +0,0 @@ -require "rails_helper" -require "tmpdir" - -RSpec.describe CoPlan::RenderCache do - # Each instance represents a separate boot of a release, with its own memo. - let(:release) { Class.new(described_class) } - - around do |example| - original = ENV.to_h.slice("RAILS_CACHE_ID", "RAILS_APP_VERSION") - ENV.delete("RAILS_CACHE_ID") - ENV.delete("RAILS_APP_VERSION") - Dir.mktmpdir do |directory| - @lockfile = Pathname.new(directory).join("Gemfile.lock") - example.run - end - ensure - ENV.delete("RAILS_CACHE_ID") - ENV.delete("RAILS_APP_VERSION") - original.each { |name, value| ENV[name] = value } - end - - before do - allow(Rails.application.config).to receive(:reloading_enabled?).and_return(false) - allow(Bundler).to receive(:default_lockfile).and_return(@lockfile) - allow(Bundler).to receive(:locked_gems) { Bundler::LockfileParser.new(@lockfile.read) } - allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) - end - - def write_lock(version: "1.1.0", revision: nil, path: nil, parser_version: "2.4.0") - engine = " coplan-engine (#{version})\n" - source = if revision - "GIT\n remote: https://github.com/block/coplan.git\n revision: #{revision}\n specs:\n#{engine}\n" - elsif path - "PATH\n remote: #{path}\n specs:\n#{engine}\n" - else - "" - end - gems = "GEM\n remote: https://rubygems.org/\n specs:\n" - gems += engine unless revision || path - gems += " commonmarker (#{parser_version})\n\nDEPENDENCIES\n coplan-engine#{'!' if revision || path}\n" - @lockfile.write(source + gems) - end - - it "reuses rendered output for the same installed gem release without a host variable" do - write_lock - expect(release.fetch([ "example", "content-sha" ]) { "first render" }).to eq("first render") - expect(Class.new(described_class).fetch([ "example", "content-sha" ]) { "second render" }).to eq("first render") - end - - it "invalidates persistent output when the installed engine gem version changes" do - write_lock - release.fetch([ "example", "unchanged-content" ]) { "old HTML" } - write_lock(version: "1.2.0") - expect(Class.new(described_class).fetch([ "example", "unchanged-content" ]) { "new HTML" }).to eq("new HTML") - end - - it "invalidates Git-pinned engine updates even when the gem version stays the same" do - write_lock(revision: "b40c2c32681e1ad4d7fadbb80392510d90cb08e2") - release.fetch([ "example", "unchanged-content" ]) { "deployed HTML" } - write_lock(revision: "f2693895b48fab820c579728715ce90daaf0dd1a") - expect(Class.new(described_class).fetch([ "example", "unchanged-content" ]) { "candidate HTML" }).to eq("candidate HTML") - end - - it "does not ask Git for a revision or depend on an installed checkout directory name" do - write_lock(revision: "f2693895b48fab820c579728715ce90daaf0dd1a") - expect_any_instance_of(Bundler::Source::Git).not_to receive(:revision) - expect(release.version).to be_present - end - - it "invalidates output when a rendering dependency changes" do - write_lock - original = release.version - write_lock(parser_version: "2.5.0") - expect(Class.new(described_class).version).not_to eq(original) - end - - it "does not read the lockfile again on later lookups in the same process" do - write_lock - original = release.version - expect(Digest::SHA256).not_to receive(:file) - expect(release.version).to equal(original) - end - - it "bypasses rendered caches for editable path installs without a release ID" do - write_lock(path: "engine") - expect(release.version).to be_nil - expect(Rails.cache).not_to receive(:fetch) - expect(release.fetch([ "example" ]) { "first render" }).to eq("first render") - expect(release.fetch([ "example" ]) { "updated renderer" }).to eq("updated renderer") - end - - it "also bypasses caches when another dependency is an editable path source" do - write_lock - @lockfile.write("PATH\n remote: local-renderer\n specs:\n custom-renderer (1.0)\n\n" + @lockfile.read) - expect(release.version).to be_nil - end - - %w[RAILS_APP_VERSION RAILS_CACHE_ID].each do |name| - it "uses Rails' #{name} namespace for path-based production releases" do - write_lock(path: "engine") - ENV[name] = "release-a" - release.fetch([ "example" ]) { "release A" } - expect(release.fetch([ "example" ]) { "cached" }).to eq("release A") - ENV[name] = "release-b" - expect(Class.new(described_class).fetch([ "example" ]) { "release B" }).to eq("release B") - end - end - - it "does not cache while development reloading is enabled, even with a release ID" do - write_lock - ENV["RAILS_APP_VERSION"] = "release-a" - allow(Rails.application.config).to receive(:reloading_enabled?).and_return(true) - expect(release.version).to be_nil - expect(Rails.cache).not_to receive(:fetch) - expect(release.fetch([ "example" ]) { "fresh HTML" }).to eq("fresh HTML") - end - - it "falls back to the installed gem version without Bundler or a lockfile" do - hide_const("Bundler") - gem = Gem::Specification.new { |spec| spec.name = "coplan-engine"; spec.version = "1.2.0" } - allow(Gem).to receive(:loaded_specs).and_return("coplan-engine" => gem) - expect(release.version).to eq("coplan-engine-1.2.0") - end -end