From 8acc8fe6251cc98696644ddcd4a872677c604d85 Mon Sep 17 00:00:00 2001
From: Hampton Lintorn-Catlin <hampton@squareup.com>
Date: Mon, 23 Feb 2026 15:22:53 -0600
Subject: [PATCH 1/5] Switch test framework from Minitest to RSpec + FactoryBot

- Replace minitest gem with rspec-rails and factory_bot_rails
- Convert all 175 tests from test/ to spec/ (models, requests, services, helpers)
- Convert YAML fixtures to FactoryBot factories with proper association chains
- Factories derive org from parent objects to keep data consistent
- Add sign_in_as helper and ActiveSupport::Testing::TimeHelpers to rails_helper
- Update AGENTS.md to document new testing conventions
- Remove old test/ directory

Amp-Thread-ID: https://ampcode.com/threads/T-019c8c51-0ef2-723d-bab8-a95e7ecbf4e0
Co-authored-by: Amp <amp@ampcode.com>
---
 .rspec                                        |   1 +
 AGENTS.md                                     |  15 +-
 Gemfile                                       |   3 +-
 Gemfile.lock                                  |  34 ++-
 spec/factories/api_tokens.rb                  |  22 ++
 spec/factories/automated_plan_reviewers.rb    |  12 +
 spec/factories/comment_threads.rb             |  19 ++
 spec/factories/comments.rb                    |   9 +
 spec/factories/edit_leases.rb                 |  11 +
 spec/factories/organizations.rb               |   7 +
 spec/factories/plan_versions.rb               |  10 +
 spec/factories/plans.rb                       |  25 +++
 spec/factories/users.rb                       |  12 +
 spec/helpers/markdown_helper_spec.rb          |  46 ++++
 spec/models/api_token_spec.rb                 |  66 ++++++
 spec/models/automated_plan_reviewer_spec.rb   | 145 ++++++++++++
 spec/models/comment_spec.rb                   |  26 +++
 spec/models/comment_thread_spec.rb            | 188 ++++++++++++++++
 spec/models/edit_lease_spec.rb                | 147 +++++++++++++
 spec/models/organization_spec.rb              |  53 +++++
 spec/models/plan_spec.rb                      |  35 +++
 spec/models/plan_version_spec.rb              |  28 +++
 spec/models/user_spec.rb                      |  63 ++++++
 spec/rails_helper.rb                          |  24 ++
 spec/requests/api/v1/comments_spec.rb         |  68 ++++++
 spec/requests/api/v1/leases_spec.rb           |  90 ++++++++
 spec/requests/api/v1/operations_spec.rb       |  87 ++++++++
 spec/requests/api/v1/plans_spec.rb            |  88 ++++++++
 spec/requests/api_tokens_spec.rb              |  38 ++++
 spec/requests/comment_threads_spec.rb         |  78 +++++++
 spec/requests/comments_spec.rb                |  22 ++
 spec/requests/plans_spec.rb                   |  79 +++++++
 spec/requests/sessions_spec.rb                |  47 ++++
 spec/services/plans/apply_operations_spec.rb  | 150 +++++++++++++
 spec/services/plans/create_spec.rb            |  27 +++
 spec/spec_helper.rb                           |  94 ++++++++
 test/application_system_test_case.rb          |   5 -
 test/controllers/.keep                        |   0
 .../api/v1/comments_controller_test.rb        |  63 ------
 .../api/v1/leases_controller_test.rb          |  82 -------
 .../api/v1/operations_controller_test.rb      |  82 -------
 .../api/v1/plans_controller_test.rb           |  73 ------
 .../controllers/api_tokens_controller_test.rb |  37 ----
 .../comment_threads_controller_test.rb        |  76 -------
 test/controllers/comments_controller_test.rb  |  21 --
 test/controllers/plans_controller_test.rb     |  76 -------
 test/controllers/sessions_controller_test.rb  |  44 ----
 test/fixtures/api_tokens.yml                  |  28 ---
 test/fixtures/automated_plan_reviewers.yml    |  61 ------
 test/fixtures/comment_threads.yml             |  18 --
 test/fixtures/comments.yml                    |  15 --
 test/fixtures/edit_leases.yml                 |   1 -
 test/fixtures/files/.keep                     |   0
 test/fixtures/organizations.yml               |  11 -
 test/fixtures/plan_versions.yml               |  21 --
 test/fixtures/plans.yml                       |  21 --
 test/fixtures/users.yml                       |  20 --
 test/helpers/.keep                            |   0
 test/helpers/markdown_helper_test.rb          |  42 ----
 test/integration/.keep                        |   0
 test/mailers/.keep                            |   0
 test/models/.keep                             |   0
 test/models/api_token_test.rb                 |  66 ------
 test/models/automated_plan_reviewer_test.rb   | 164 --------------
 test/models/comment_test.rb                   |  27 ---
 test/models/comment_thread_test.rb            | 207 ------------------
 test/models/edit_lease_test.rb                | 145 ------------
 test/models/organization_test.rb              |  52 -----
 test/models/plan_test.rb                      |  35 ---
 test/models/plan_version_test.rb              |  38 ----
 test/models/user_test.rb                      |  56 -----
 test/services/plans/apply_operations_test.rb  | 152 -------------
 test/services/plans/create_test.rb            |  27 ---
 test/system/.keep                             |   0
 test/test_helper.rb                           |  17 --
 75 files changed, 1861 insertions(+), 1791 deletions(-)
 create mode 100644 .rspec
 create mode 100644 spec/factories/api_tokens.rb
 create mode 100644 spec/factories/automated_plan_reviewers.rb
 create mode 100644 spec/factories/comment_threads.rb
 create mode 100644 spec/factories/comments.rb
 create mode 100644 spec/factories/edit_leases.rb
 create mode 100644 spec/factories/organizations.rb
 create mode 100644 spec/factories/plan_versions.rb
 create mode 100644 spec/factories/plans.rb
 create mode 100644 spec/factories/users.rb
 create mode 100644 spec/helpers/markdown_helper_spec.rb
 create mode 100644 spec/models/api_token_spec.rb
 create mode 100644 spec/models/automated_plan_reviewer_spec.rb
 create mode 100644 spec/models/comment_spec.rb
 create mode 100644 spec/models/comment_thread_spec.rb
 create mode 100644 spec/models/edit_lease_spec.rb
 create mode 100644 spec/models/organization_spec.rb
 create mode 100644 spec/models/plan_spec.rb
 create mode 100644 spec/models/plan_version_spec.rb
 create mode 100644 spec/models/user_spec.rb
 create mode 100644 spec/rails_helper.rb
 create mode 100644 spec/requests/api/v1/comments_spec.rb
 create mode 100644 spec/requests/api/v1/leases_spec.rb
 create mode 100644 spec/requests/api/v1/operations_spec.rb
 create mode 100644 spec/requests/api/v1/plans_spec.rb
 create mode 100644 spec/requests/api_tokens_spec.rb
 create mode 100644 spec/requests/comment_threads_spec.rb
 create mode 100644 spec/requests/comments_spec.rb
 create mode 100644 spec/requests/plans_spec.rb
 create mode 100644 spec/requests/sessions_spec.rb
 create mode 100644 spec/services/plans/apply_operations_spec.rb
 create mode 100644 spec/services/plans/create_spec.rb
 create mode 100644 spec/spec_helper.rb
 delete mode 100644 test/application_system_test_case.rb
 delete mode 100644 test/controllers/.keep
 delete mode 100644 test/controllers/api/v1/comments_controller_test.rb
 delete mode 100644 test/controllers/api/v1/leases_controller_test.rb
 delete mode 100644 test/controllers/api/v1/operations_controller_test.rb
 delete mode 100644 test/controllers/api/v1/plans_controller_test.rb
 delete mode 100644 test/controllers/api_tokens_controller_test.rb
 delete mode 100644 test/controllers/comment_threads_controller_test.rb
 delete mode 100644 test/controllers/comments_controller_test.rb
 delete mode 100644 test/controllers/plans_controller_test.rb
 delete mode 100644 test/controllers/sessions_controller_test.rb
 delete mode 100644 test/fixtures/api_tokens.yml
 delete mode 100644 test/fixtures/automated_plan_reviewers.yml
 delete mode 100644 test/fixtures/comment_threads.yml
 delete mode 100644 test/fixtures/comments.yml
 delete mode 100644 test/fixtures/edit_leases.yml
 delete mode 100644 test/fixtures/files/.keep
 delete mode 100644 test/fixtures/organizations.yml
 delete mode 100644 test/fixtures/plan_versions.yml
 delete mode 100644 test/fixtures/plans.yml
 delete mode 100644 test/fixtures/users.yml
 delete mode 100644 test/helpers/.keep
 delete mode 100644 test/helpers/markdown_helper_test.rb
 delete mode 100644 test/integration/.keep
 delete mode 100644 test/mailers/.keep
 delete mode 100644 test/models/.keep
 delete mode 100644 test/models/api_token_test.rb
 delete mode 100644 test/models/automated_plan_reviewer_test.rb
 delete mode 100644 test/models/comment_test.rb
 delete mode 100644 test/models/comment_thread_test.rb
 delete mode 100644 test/models/edit_lease_test.rb
 delete mode 100644 test/models/organization_test.rb
 delete mode 100644 test/models/plan_test.rb
 delete mode 100644 test/models/plan_version_test.rb
 delete mode 100644 test/models/user_test.rb
 delete mode 100644 test/services/plans/apply_operations_test.rb
 delete mode 100644 test/services/plans/create_test.rb
 delete mode 100644 test/system/.keep
 delete mode 100644 test/test_helper.rb

diff --git a/.rspec b/.rspec
new file mode 100644
index 00000000..c99d2e73
--- /dev/null
+++ b/.rspec
@@ -0,0 +1 @@
+--require spec_helper
diff --git a/AGENTS.md b/AGENTS.md
index 67775e4f..c8f91804 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -42,12 +42,15 @@ A Rails app for managing engineering design doc review, purpose-built for AI-gen
 
 ## Testing
 
-- **Minitest** — not RSpec
-- Run the full suite: `bin/rails test`
-- Test files mirror app structure: `test/models/`, `test/controllers/`, `test/services/`
-- Use **fixtures** (YAML in `test/fixtures/`) — not factories
-- Every model, service object, and controller action should have tests
-- Fixtures use hardcoded UUIDs for stable cross-references
+- **RSpec** with `rspec-rails`
+- Run the full suite: `bundle exec rspec`
+- Spec files mirror app structure: `spec/models/`, `spec/requests/`, `spec/services/`, `spec/helpers/`
+- Use **FactoryBot** (`factory_bot_rails`) — factories live in `spec/factories/`
+- Every model, service object, and controller action should have specs
+- UUID primary keys are auto-assigned by `ApplicationRecord#assign_uuid` — do **not** set `id` in factories
+- Factories derive associations from parent objects (e.g., `organization { plan.organization }`) to keep data consistent
+- FactoryBot syntax methods (`create`, `build`) are included globally via `config.include FactoryBot::Syntax::Methods`
+- `sign_in_as(user)` helper is defined in `spec/rails_helper.rb` for request specs
 
 ## Seeds
 
diff --git a/Gemfile b/Gemfile
index 59de9842..07127190 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,7 +2,6 @@ source "https://rubygems.org"
 
 # Bundle edge Rails instead: gem "rails", github: "rails/rails", branch: "main"
 gem "rails", "~> 8.1.1"
-gem "minitest", "~> 5.25"
 # The modern asset pipeline for Rails [https://github.com/rails/propshaft]
 gem "propshaft"
 # Use mysql as the database for Active Record
@@ -73,6 +72,8 @@ group :development do
 end
 
 group :test do
+  gem "rspec-rails"
+  gem "factory_bot_rails"
   # Use system testing [https://guides.rubyonrails.org/testing.html#system-testing]
   gem "capybara"
   gem "selenium-webdriver"
diff --git a/Gemfile.lock b/Gemfile.lock
index f1bea969..745baf14 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -129,6 +129,7 @@ GEM
     debug (1.11.1)
       irb (~> 1.10)
       reline (>= 0.3.8)
+    diff-lcs (1.6.2)
     diffy (3.4.4)
     dotenv (3.2.0)
     drb (2.2.3)
@@ -138,6 +139,11 @@ GEM
     et-orbi (1.4.0)
       tzinfo
     event_stream_parser (1.0.0)
+    factory_bot (6.5.6)
+      activesupport (>= 6.1.0)
+    factory_bot_rails (6.5.1)
+      factory_bot (~> 6.5)
+      railties (>= 6.1.0)
     faraday (2.14.1)
       faraday-net_http (>= 2.0, < 3.5)
       json
@@ -345,6 +351,23 @@ GEM
       actionpack (>= 7.0)
       railties (>= 7.0)
     rexml (3.4.4)
+    rspec-core (3.13.6)
+      rspec-support (~> 3.13.0)
+    rspec-expectations (3.13.5)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.13.0)
+    rspec-mocks (3.13.7)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.13.0)
+    rspec-rails (8.0.3)
+      actionpack (>= 7.2)
+      activesupport (>= 7.2)
+      railties (>= 7.2)
+      rspec-core (~> 3.13)
+      rspec-expectations (~> 3.13)
+      rspec-mocks (~> 3.13)
+      rspec-support (~> 3.13)
+    rspec-support (3.13.7)
     rubocop (1.84.2)
       json (~> 2.3)
       language_server-protocol (~> 3.17.0.2)
@@ -469,16 +492,17 @@ DEPENDENCIES
   commonmarker
   debug
   diffy
+  factory_bot_rails
   image_processing (~> 1.2)
   importmap-rails
   jbuilder
   kamal
-  minitest (~> 5.25)
   mysql2 (~> 0.5)
   propshaft
   puma (>= 5.0)
   rack-attack
   rails (~> 8.1.1)
+  rspec-rails
   rubocop-rails-omakase
   ruby-openai
   selenium-webdriver
@@ -532,6 +556,7 @@ CHECKSUMS
   csv (3.3.5) sha256=6e5134ac3383ef728b7f02725d9872934f523cb40b961479f69cf3afa6c8e73f
   date (3.5.1) sha256=750d06384d7b9c15d562c76291407d89e368dda4d4fff957eb94962d325a0dc0
   debug (1.11.1) sha256=2e0b0ac6119f2207a6f8ac7d4a73ca8eb4e440f64da0a3136c30343146e952b6
+  diff-lcs (1.6.2) sha256=9ae0d2cba7d4df3075fe8cd8602a8604993efc0dfa934cff568969efb1909962
   diffy (3.4.4) sha256=79384ab5ca82d0e115b2771f0961e27c164c456074bd2ec46b637ebf7b6e47e3
   dotenv (3.2.0) sha256=e375b83121ea7ca4ce20f214740076129ab8514cd81378161f11c03853fe619d
   drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373
@@ -540,6 +565,8 @@ CHECKSUMS
   erubi (1.13.1) sha256=a082103b0885dbc5ecf1172fede897f9ebdb745a4b97a5e8dc63953db1ee4ad9
   et-orbi (1.4.0) sha256=6c7e3c90779821f9e3b324c5e96fda9767f72995d6ae435b96678a4f3e2de8bc
   event_stream_parser (1.0.0) sha256=a2683bab70126286f8184dc88f7968ffc4028f813161fb073ec90d171f7de3c8
+  factory_bot (6.5.6) sha256=12beb373214dccc086a7a63763d6718c49769d5606f0501e0a4442676917e077
+  factory_bot_rails (6.5.1) sha256=d3cc4851eae4dea8a665ec4a4516895045e710554d2b5ac9e68b94d351bc6d68
   faraday (2.14.1) sha256=a43cceedc1e39d188f4d2cdd360a8aaa6a11da0c407052e426ba8d3fb42ef61c
   faraday-multipart (1.2.0) sha256=7d89a949693714176f612323ca13746a2ded204031a6ba528adee788694ef757
   faraday-net_http (3.4.2) sha256=f147758260d3526939bf57ecf911682f94926a3666502e24c69992765875906c
@@ -628,6 +655,11 @@ CHECKSUMS
   reline (0.6.3) sha256=1198b04973565b36ec0f11542ab3f5cfeeec34823f4e54cebde90968092b1835
   responders (3.2.0) sha256=89c2d6ac0ae16f6458a11524cae4a8efdceba1a3baea164d28ee9046bd3df55a
   rexml (3.4.4) sha256=19e0a2c3425dfbf2d4fc1189747bdb2f849b6c5e74180401b15734bc97b5d142
+  rspec-core (3.13.6) sha256=a8823c6411667b60a8bca135364351dda34cd55e44ff94c4be4633b37d828b2d
+  rspec-expectations (3.13.5) sha256=33a4d3a1d95060aea4c94e9f237030a8f9eae5615e9bd85718fe3a09e4b58836
+  rspec-mocks (3.13.7) sha256=0979034e64b1d7a838aaaddf12bf065ea4dc40ef3d4c39f01f93ae2c66c62b1c
+  rspec-rails (8.0.3) sha256=b0a440e7a10700317d898a014852e26660867298c4076dbc3baa99c768b79dc1
+  rspec-support (3.13.7) sha256=0640e5570872aafefd79867901deeeeb40b0c9875a36b983d85f54fb7381c47c
   rubocop (1.84.2) sha256=5692cea54168f3dc8cb79a6fe95c5424b7ea893c707ad7a4307b0585e88dbf5f
   rubocop-ast (1.49.0) sha256=49c3676d3123a0923d333e20c6c2dbaaae2d2287b475273fddee0c61da9f71fd
   rubocop-performance (1.26.1) sha256=cd19b936ff196df85829d264b522fd4f98b6c89ad271fa52744a8c11b8f71834
diff --git a/spec/factories/api_tokens.rb b/spec/factories/api_tokens.rb
new file mode 100644
index 00000000..f8333e5b
--- /dev/null
+++ b/spec/factories/api_tokens.rb
@@ -0,0 +1,22 @@
+FactoryBot.define do
+  factory :api_token do
+    organization
+    user { association(:user, organization: organization) }
+    sequence(:name) { |n| "Token #{n}" }
+    token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(32)) }
+
+    transient do
+      raw_token { nil }
+    end
+
+    after(:build) do |token, evaluator|
+      if evaluator.raw_token
+        token.token_digest = Digest::SHA256.hexdigest(evaluator.raw_token)
+      end
+    end
+
+    trait :revoked do
+      revoked_at { 1.day.ago }
+    end
+  end
+end
diff --git a/spec/factories/automated_plan_reviewers.rb b/spec/factories/automated_plan_reviewers.rb
new file mode 100644
index 00000000..dbe2726b
--- /dev/null
+++ b/spec/factories/automated_plan_reviewers.rb
@@ -0,0 +1,12 @@
+FactoryBot.define do
+  factory :automated_plan_reviewer do
+    organization
+    sequence(:key) { |n| "reviewer-#{n}" }
+    sequence(:name) { |n| "Reviewer #{n}" }
+    prompt_text { "You are a reviewer. Review the plan." }
+    enabled { true }
+    trigger_statuses { [] }
+    ai_provider { "openai" }
+    ai_model { "gpt-4o" }
+  end
+end
diff --git a/spec/factories/comment_threads.rb b/spec/factories/comment_threads.rb
new file mode 100644
index 00000000..6fb943e6
--- /dev/null
+++ b/spec/factories/comment_threads.rb
@@ -0,0 +1,19 @@
+FactoryBot.define do
+  factory :comment_thread do
+    plan
+    organization { plan.organization }
+    plan_version { plan.current_plan_version }
+    created_by_user { association(:user, organization: plan.organization) }
+    status { "open" }
+    out_of_date { false }
+
+    trait :with_anchor do
+      anchor_text { "some anchor text" }
+    end
+
+    trait :resolved do
+      status { "resolved" }
+      association :resolved_by_user, factory: :user
+    end
+  end
+end
diff --git a/spec/factories/comments.rb b/spec/factories/comments.rb
new file mode 100644
index 00000000..95222177
--- /dev/null
+++ b/spec/factories/comments.rb
@@ -0,0 +1,9 @@
+FactoryBot.define do
+  factory :comment do
+    comment_thread
+    organization { comment_thread.organization }
+    author_type { "human" }
+    author_id { association(:user, organization: organization).id }
+    body_markdown { "A comment body." }
+  end
+end
diff --git a/spec/factories/edit_leases.rb b/spec/factories/edit_leases.rb
new file mode 100644
index 00000000..e3e4b0e6
--- /dev/null
+++ b/spec/factories/edit_leases.rb
@@ -0,0 +1,11 @@
+FactoryBot.define do
+  factory :edit_lease do
+    plan
+    organization
+    holder_type { "local_agent" }
+    holder_id { SecureRandom.uuid }
+    lease_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(32)) }
+    expires_at { 5.minutes.from_now }
+    last_heartbeat_at { Time.current }
+  end
+end
diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb
new file mode 100644
index 00000000..4b215cac
--- /dev/null
+++ b/spec/factories/organizations.rb
@@ -0,0 +1,7 @@
+FactoryBot.define do
+  factory :organization do
+    sequence(:name) { |n| "Organization #{n}" }
+    sequence(:slug) { |n| "org-#{n}" }
+    allowed_email_domains { ["example.com"] }
+  end
+end
diff --git a/spec/factories/plan_versions.rb b/spec/factories/plan_versions.rb
new file mode 100644
index 00000000..e7e8be54
--- /dev/null
+++ b/spec/factories/plan_versions.rb
@@ -0,0 +1,10 @@
+FactoryBot.define do
+  factory :plan_version do
+    plan
+    organization { plan.organization }
+    sequence(:revision) { |n| n }
+    content_markdown { "# Plan Content\n\nSome content here." }
+    actor_type { "human" }
+    actor_id { nil }
+  end
+end
diff --git a/spec/factories/plans.rb b/spec/factories/plans.rb
new file mode 100644
index 00000000..cae0463c
--- /dev/null
+++ b/spec/factories/plans.rb
@@ -0,0 +1,25 @@
+FactoryBot.define do
+  factory :plan do
+    organization
+    created_by_user { association(:user, organization: organization) }
+    sequence(:title) { |n| "Plan #{n}" }
+    status { "brainstorm" }
+    tags { [] }
+    metadata { {} }
+
+    after(:create) do |plan|
+      unless plan.current_plan_version
+        version = create(:plan_version, plan: plan, organization: plan.organization, revision: 1, actor_id: plan.created_by_user_id)
+        plan.update_columns(current_plan_version_id: version.id, current_revision: 1)
+      end
+    end
+
+    trait :considering do
+      status { "considering" }
+    end
+
+    trait :brainstorm do
+      status { "brainstorm" }
+    end
+  end
+end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
new file mode 100644
index 00000000..d9a08ab4
--- /dev/null
+++ b/spec/factories/users.rb
@@ -0,0 +1,12 @@
+FactoryBot.define do
+  factory :user do
+    organization
+    sequence(:email) { |n| "user#{n}@example.com" }
+    sequence(:name) { |n| "User #{n}" }
+    org_role { "member" }
+
+    trait :admin do
+      org_role { "admin" }
+    end
+  end
+end
diff --git a/spec/helpers/markdown_helper_spec.rb b/spec/helpers/markdown_helper_spec.rb
new file mode 100644
index 00000000..dd8940dc
--- /dev/null
+++ b/spec/helpers/markdown_helper_spec.rb
@@ -0,0 +1,46 @@
+require "rails_helper"
+
+RSpec.describe MarkdownHelper, type: :helper do
+  describe "#render_markdown" do
+    it "converts markdown to HTML" do
+      html = helper.render_markdown("# Hello\n\n**bold** text")
+      expect(html).to include("<h1>")
+      expect(html).to include("<strong>bold</strong>")
+      expect(html).to include("markdown-rendered")
+    end
+
+    it "sanitizes dangerous HTML" do
+      html = helper.render_markdown('<script>alert("xss")</script>')
+      expect(html).not_to match(/<script>/)
+    end
+
+    it "handles nil gracefully" do
+      html = helper.render_markdown(nil)
+      expect(html).to include("markdown-rendered")
+    end
+  end
+
+  describe "#render_line_view" do
+    it "creates numbered divs" do
+      html = helper.render_line_view("line one\nline two\nline three")
+      expect(html).to include('id="L1"')
+      expect(html).to include('id="L2"')
+      expect(html).to include('id="L3"')
+      expect(html).to include('data-line="1"')
+      expect(html).to include('data-line="3"')
+      expect(html).to include("line-view")
+    end
+
+    it "handles empty lines" do
+      html = helper.render_line_view("line one\n\nline three")
+      expect(html).to include('id="L2"')
+      expect(html).to include("&nbsp;")
+    end
+
+    it "escapes HTML in content" do
+      html = helper.render_line_view('<script>alert("xss")</script>')
+      expect(html).not_to match(/<script>/)
+      expect(html).to include("&lt;script&gt;")
+    end
+  end
+end
diff --git a/spec/models/api_token_spec.rb b/spec/models/api_token_spec.rb
new file mode 100644
index 00000000..21f20274
--- /dev/null
+++ b/spec/models/api_token_spec.rb
@@ -0,0 +1,66 @@
+require "rails_helper"
+
+RSpec.describe ApiToken, type: :model do
+  let(:org) { create(:organization) }
+  let(:user) { create(:user, organization: org) }
+
+  it "is valid with valid attributes" do
+    token = create(:api_token, organization: org, user: user)
+    expect(token).to be_valid
+  end
+
+  it "requires name" do
+    token = build(:api_token, organization: org, user: user, name: nil)
+    expect(token).not_to be_valid
+    expect(token.errors[:name]).to include("can't be blank")
+  end
+
+  it "requires token_digest" do
+    token = build(:api_token, organization: org, user: user, token_digest: nil)
+    expect(token).not_to be_valid
+    expect(token.errors[:token_digest]).to include("can't be blank")
+  end
+
+  it "authenticates with valid token" do
+    create(:api_token, organization: org, user: user, raw_token: "test-token-alice")
+    result = ApiToken.authenticate("test-token-alice")
+    expect(result).not_to be_nil
+  end
+
+  it "returns nil for invalid token" do
+    expect(ApiToken.authenticate("invalid-token")).to be_nil
+  end
+
+  it "returns nil for revoked token" do
+    create(:api_token, :revoked, organization: org, user: user, raw_token: "test-token-revoked")
+    expect(ApiToken.authenticate("test-token-revoked")).to be_nil
+  end
+
+  it "returns nil for blank token" do
+    expect(ApiToken.authenticate("")).to be_nil
+    expect(ApiToken.authenticate(nil)).to be_nil
+  end
+
+  it "revoke sets revoked_at" do
+    token = create(:api_token, organization: org, user: user)
+    expect(token).not_to be_revoked
+    token.revoke!
+    expect(token).to be_revoked
+    expect(token.revoked_at).not_to be_nil
+  end
+
+  it "active? returns false when revoked" do
+    token = create(:api_token, :revoked, organization: org, user: user)
+    expect(token).not_to be_active
+  end
+
+  it "active? returns true for valid token" do
+    token = create(:api_token, organization: org, user: user)
+    expect(token).to be_active
+  end
+
+  it "generate_token returns hex string" do
+    raw = ApiToken.generate_token
+    expect(raw).to match(/\A[0-9a-f]{64}\z/)
+  end
+end
diff --git a/spec/models/automated_plan_reviewer_spec.rb b/spec/models/automated_plan_reviewer_spec.rb
new file mode 100644
index 00000000..6df3a669
--- /dev/null
+++ b/spec/models/automated_plan_reviewer_spec.rb
@@ -0,0 +1,145 @@
+require "rails_helper"
+
+RSpec.describe AutomatedPlanReviewer, type: :model do
+  let(:org) { create(:organization) }
+
+  it "is valid with valid attributes" do
+    reviewer = create(:automated_plan_reviewer, organization: org)
+    expect(reviewer).to be_valid
+  end
+
+  it "requires key" do
+    reviewer = build(:automated_plan_reviewer, key: nil)
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:key]).to include("can't be blank")
+  end
+
+  it "requires name" do
+    reviewer = build(:automated_plan_reviewer, name: nil)
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:name]).to include("can't be blank")
+  end
+
+  it "requires prompt_text" do
+    reviewer = build(:automated_plan_reviewer, prompt_text: nil)
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:prompt_text]).to include("can't be blank")
+  end
+
+  it "requires ai_model" do
+    reviewer = build(:automated_plan_reviewer, ai_model: nil)
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:ai_model]).to include("can't be blank")
+  end
+
+  it "requires organization" do
+    reviewer = build(:automated_plan_reviewer, organization: nil)
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:organization]).to include("must exist")
+  end
+
+  it "validates key format" do
+    reviewer = build(:automated_plan_reviewer, organization: org, key: "Invalid Key!")
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:key]).to include("only allows lowercase letters, numbers, and hyphens")
+  end
+
+  it "validates key uniqueness scoped to organization" do
+    existing = create(:automated_plan_reviewer, organization: org)
+    duplicate = build(:automated_plan_reviewer, organization: org, key: existing.key)
+    expect(duplicate).not_to be_valid
+    expect(duplicate.errors[:key]).to include("has already been taken")
+  end
+
+  it "allows same key in different organizations" do
+    other_org = create(:organization)
+    create(:automated_plan_reviewer, organization: org, key: "same-key")
+    reviewer = build(:automated_plan_reviewer, organization: other_org, key: "same-key")
+    expect(reviewer).to be_valid
+  end
+
+  it "stores prompt text" do
+    reviewer = create(:automated_plan_reviewer, organization: org, prompt_text: "Review for security issues")
+    expect(reviewer.prompt_text).to include("security")
+  end
+
+  it "triggers_on_status? returns true for matching status" do
+    reviewer = create(:automated_plan_reviewer, organization: org, trigger_statuses: ["considering"])
+    expect(reviewer.triggers_on_status?("considering")).to be true
+  end
+
+  it "triggers_on_status? returns false for non-matching status" do
+    reviewer = create(:automated_plan_reviewer, organization: org, trigger_statuses: ["considering"])
+    expect(reviewer.triggers_on_status?("brainstorm")).to be false
+  end
+
+  it "enabled scope returns only enabled reviewers" do
+    enabled_reviewer = create(:automated_plan_reviewer, organization: org, enabled: true)
+    disabled_reviewer = create(:automated_plan_reviewer, organization: org, enabled: false)
+    enabled = AutomatedPlanReviewer.enabled
+    expect(enabled).to include(enabled_reviewer)
+    expect(enabled).not_to include(disabled_reviewer)
+  end
+
+  it "validates ai_provider inclusion" do
+    reviewer = build(:automated_plan_reviewer, organization: org, ai_provider: "unknown-provider")
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:ai_provider]).to include("is not included in the list")
+  end
+
+  it "accepts valid ai_providers" do
+    AutomatedPlanReviewer::AI_PROVIDERS.each do |provider|
+      reviewer = build(:automated_plan_reviewer, organization: org, ai_provider: provider)
+      expect(reviewer).to be_valid, "Expected #{provider} to be valid"
+    end
+  end
+
+  it "defaults ai_provider to openai" do
+    reviewer = AutomatedPlanReviewer.new
+    expect(reviewer.ai_provider).to eq("openai")
+  end
+
+  it "validates trigger_statuses against Plan::STATUSES" do
+    reviewer = build(:automated_plan_reviewer, organization: org, trigger_statuses: ["considering", "invalid-status"])
+    expect(reviewer).not_to be_valid
+    expect(reviewer.errors[:trigger_statuses].any? { |e| e.include?("invalid-status") }).to be true
+  end
+
+  it "accepts valid trigger_statuses" do
+    reviewer = build(:automated_plan_reviewer, organization: org, trigger_statuses: Plan::STATUSES.dup)
+    expect(reviewer).to be_valid
+  end
+
+  it "accepts empty trigger_statuses" do
+    reviewer = build(:automated_plan_reviewer, organization: org, trigger_statuses: [])
+    expect(reviewer).to be_valid
+  end
+
+  it "defaults trigger_statuses to empty array" do
+    reviewer = AutomatedPlanReviewer.new
+    expect(reviewer.trigger_statuses).to eq([])
+  end
+
+  it "defaults enabled to true" do
+    reviewer = AutomatedPlanReviewer.new
+    expect(reviewer.enabled).to be true
+  end
+
+  it "create_defaults_for creates default reviewers for an organization" do
+    org = Organization.create!(name: "Test Org", slug: "test-defaults-org-spec")
+    reviewers = org.automated_plan_reviewers
+    expect(reviewers.count).to eq(3)
+    expect(reviewers.pluck(:key).sort).to eq(%w[routing-reviewer scalability-reviewer security-reviewer])
+    reviewers.each do |r|
+      expect(r.prompt_text).to be_present
+      expect(r.ai_model).to be_present
+    end
+  end
+
+  it "create_defaults_for is idempotent" do
+    AutomatedPlanReviewer.create_defaults_for(org)
+    initial_count = org.automated_plan_reviewers.count
+    AutomatedPlanReviewer.create_defaults_for(org)
+    expect(org.automated_plan_reviewers.count).to eq(initial_count)
+  end
+end
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
new file mode 100644
index 00000000..57631fe0
--- /dev/null
+++ b/spec/models/comment_spec.rb
@@ -0,0 +1,26 @@
+require "rails_helper"
+
+RSpec.describe Comment, type: :model do
+  it "is valid with valid attributes" do
+    comment = create(:comment)
+    expect(comment).to be_valid
+  end
+
+  it "validates body_markdown presence" do
+    comment = build(:comment, body_markdown: "")
+    expect(comment).not_to be_valid
+    expect(comment.errors[:body_markdown]).to include("can't be blank")
+  end
+
+  it "validates author_type inclusion" do
+    comment = build(:comment, author_type: "unknown")
+    expect(comment).not_to be_valid
+    expect(comment.errors[:author_type]).to include("is not included in the list")
+  end
+
+  it "belongs to comment_thread" do
+    thread = create(:comment_thread)
+    comment = create(:comment, comment_thread: thread, organization: thread.organization)
+    expect(comment.comment_thread).to eq(thread)
+  end
+end
diff --git a/spec/models/comment_thread_spec.rb b/spec/models/comment_thread_spec.rb
new file mode 100644
index 00000000..3b06ef8d
--- /dev/null
+++ b/spec/models/comment_thread_spec.rb
@@ -0,0 +1,188 @@
+require "rails_helper"
+
+RSpec.describe CommentThread, type: :model do
+  let(:org) { create(:organization) }
+  let(:user) { create(:user, organization: org) }
+  let(:plan) { create(:plan, organization: org, created_by_user: user) }
+  let(:thread_record) { create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: user) }
+
+  it "is valid with valid attributes" do
+    expect(thread_record).to be_valid
+  end
+
+  it "validates status inclusion" do
+    thread_record.status = "invalid"
+    expect(thread_record).not_to be_valid
+    expect(thread_record.errors[:status]).to include("is not included in the list")
+  end
+
+  it "returns true for line_specific? when lines set" do
+    thread_record.start_line = 5
+    thread_record.end_line = 8
+    expect(thread_record).to be_line_specific
+  end
+
+  it "returns false for line_specific? for general thread" do
+    expect(thread_record).not_to be_line_specific
+  end
+
+  it "returns line range text for range" do
+    thread_record.start_line = 5
+    thread_record.end_line = 8
+    expect(thread_record.line_range_text).to eq("Lines 5–8")
+  end
+
+  it "returns line text for single line" do
+    thread_record.start_line = 5
+    thread_record.end_line = 5
+    expect(thread_record.line_range_text).to eq("Line 5")
+  end
+
+  it "returns nil line_range_text for general thread" do
+    expect(thread_record.line_range_text).to be_nil
+  end
+
+  it "resolve! sets status and user" do
+    thread_record.resolve!(user)
+    expect(thread_record.status).to eq("resolved")
+    expect(thread_record.resolved_by_user).to eq(user)
+  end
+
+  it "accept! sets status and user" do
+    thread_record.accept!(user)
+    expect(thread_record.status).to eq("accepted")
+    expect(thread_record.resolved_by_user).to eq(user)
+  end
+
+  it "dismiss! sets status and user" do
+    thread_record.dismiss!(user)
+    expect(thread_record.status).to eq("dismissed")
+    expect(thread_record.resolved_by_user).to eq(user)
+  end
+
+  it "open_threads scope returns only open threads" do
+    open_threads = plan.comment_threads.open_threads
+    expect(open_threads).to all(have_attributes(status: "open"))
+  end
+
+  it "returns true for anchored? when anchor_text set" do
+    thread_record.anchor_text = "some text"
+    expect(thread_record).to be_anchored
+  end
+
+  it "returns false for anchored? when anchor_text blank" do
+    expect(thread_record).not_to be_anchored
+  end
+
+  it "truncates long anchor text in anchor_preview" do
+    thread_record.anchor_text = "a" * 100
+    expect(thread_record.anchor_preview).to eq("a" * 80 + "…")
+  end
+
+  it "returns short anchor text as-is in anchor_preview" do
+    thread_record.anchor_text = "short text"
+    expect(thread_record.anchor_preview).to eq("short text")
+  end
+
+  describe ".active scope" do
+    it "returns open non-out-of-date threads" do
+      thread_record # ensure it exists
+      active = plan.comment_threads.active
+      expect(active).to all(have_attributes(status: "open", out_of_date: false))
+    end
+
+    it "excludes resolved threads" do
+      thread_record.resolve!(user)
+      active = plan.comment_threads.active
+      expect(active).not_to include(thread_record)
+    end
+
+    it "excludes out-of-date threads" do
+      thread_record.update_columns(out_of_date: true)
+      active = plan.comment_threads.active
+      expect(active).not_to include(thread_record)
+    end
+  end
+
+  describe ".archived scope" do
+    it "returns non-open or out-of-date threads" do
+      archived = plan.comment_threads.archived
+      expect(archived).to all(satisfy { |t| t.status != "open" || t.out_of_date? })
+    end
+
+    it "includes resolved threads" do
+      thread_record.resolve!(user)
+      archived = plan.comment_threads.archived
+      expect(archived).to include(thread_record)
+    end
+  end
+
+  describe ".mark_out_of_date_for_new_version!" do
+    it "keeps thread when anchor text still present" do
+      thread_record.update_columns(anchor_text: "world domination")
+
+      new_version = PlanVersion.create!(
+        plan: plan,
+        organization: org,
+        revision: plan.current_revision + 1,
+        content_markdown: "# Plan\n\nOur plan for world domination continues.",
+        actor_type: "human",
+        actor_id: user.id
+      )
+
+      plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
+      thread_record.reload
+      expect(thread_record).not_to be_out_of_date
+    end
+
+    it "marks thread when anchor text removed" do
+      thread_record.update_columns(anchor_text: "world domination")
+
+      new_version = PlanVersion.create!(
+        plan: plan,
+        organization: org,
+        revision: plan.current_revision + 1,
+        content_markdown: "# Plan\n\nCompletely new content here.",
+        actor_type: "human",
+        actor_id: user.id
+      )
+
+      plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
+      thread_record.reload
+      expect(thread_record).to be_out_of_date
+      expect(thread_record.out_of_date_since_version_id).to eq(new_version.id)
+    end
+
+    it "skips non-anchored threads" do
+      new_version = PlanVersion.create!(
+        plan: plan,
+        organization: org,
+        revision: plan.current_revision + 1,
+        content_markdown: "Completely different content.",
+        actor_type: "human",
+        actor_id: user.id
+      )
+
+      plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
+      thread_record.reload
+      expect(thread_record).not_to be_out_of_date
+    end
+
+    it "uses anchor_context when present" do
+      thread_record.update_columns(anchor_text: "plan", anchor_context: "Our plan for world domination")
+
+      new_version = PlanVersion.create!(
+        plan: plan,
+        organization: org,
+        revision: plan.current_revision + 1,
+        content_markdown: "# Plan\n\nWe have a plan but the context changed.",
+        actor_type: "human",
+        actor_id: user.id
+      )
+
+      plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
+      thread_record.reload
+      expect(thread_record).to be_out_of_date
+    end
+  end
+end
diff --git a/spec/models/edit_lease_spec.rb b/spec/models/edit_lease_spec.rb
new file mode 100644
index 00000000..eb3d3c5f
--- /dev/null
+++ b/spec/models/edit_lease_spec.rb
@@ -0,0 +1,147 @@
+require "rails_helper"
+
+RSpec.describe EditLease, type: :model do
+  let(:org) { create(:organization) }
+  let(:user) { create(:user, organization: org) }
+  let(:plan) { create(:plan, organization: org, created_by_user: user) }
+  let(:api_token) { create(:api_token, organization: org, user: user) }
+  let(:lease_token) { SecureRandom.hex(32) }
+
+  it "acquire creates new lease" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+    expect(lease).to be_persisted
+    expect(lease).to be_held
+    expect(lease.plan_id).to eq(plan.id)
+  end
+
+  it "acquire replaces expired lease" do
+    EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    lease = EditLease.find_by(plan_id: plan.id)
+    lease.update!(expires_at: 1.minute.ago)
+
+    other_token = create(:api_token, organization: org, user: user)
+    new_token = SecureRandom.hex(32)
+    new_lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: other_token.id,
+      lease_token: new_token
+    )
+    expect(new_lease).to be_persisted
+    expect(new_lease).to be_held
+  end
+
+  it "acquire raises conflict when held by another" do
+    EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    other_token = create(:api_token, organization: org, user: user)
+    expect {
+      EditLease.acquire!(
+        plan: plan,
+        holder_type: "local_agent",
+        holder_id: other_token.id,
+        lease_token: SecureRandom.hex(32)
+      )
+    }.to raise_error(EditLease::Conflict)
+  end
+
+  it "acquire with same token renews lease" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+    original_expires = lease.expires_at
+
+    travel 1.minute do
+      renewed = EditLease.acquire!(
+        plan: plan,
+        holder_type: "local_agent",
+        holder_id: api_token.id,
+        lease_token: lease_token
+      )
+      expect(renewed.expires_at).to be > original_expires
+    end
+  end
+
+  it "renew updates expiry" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    travel 1.minute do
+      lease.renew!(lease_token: lease_token)
+      expect(lease.expires_at).to be > Time.current
+    end
+  end
+
+  it "renew raises conflict with wrong token" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    expect {
+      lease.renew!(lease_token: "wrong-token")
+    }.to raise_error(EditLease::Conflict)
+  end
+
+  it "release destroys lease" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    lease.release!(lease_token: lease_token)
+    expect(EditLease.find_by(plan_id: plan.id)).to be_nil
+  end
+
+  it "release raises conflict with wrong token" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    expect {
+      lease.release!(lease_token: "wrong-token")
+    }.to raise_error(EditLease::Conflict)
+  end
+
+  it "held_by? checks token and expiry" do
+    lease = EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: api_token.id,
+      lease_token: lease_token
+    )
+
+    expect(lease.held_by?(lease_token: lease_token)).to be true
+    expect(lease.held_by?(lease_token: "wrong-token")).to be false
+  end
+end
diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb
new file mode 100644
index 00000000..bbedc829
--- /dev/null
+++ b/spec/models/organization_spec.rb
@@ -0,0 +1,53 @@
+require "rails_helper"
+
+RSpec.describe Organization, type: :model do
+  it "is valid with valid attributes" do
+    org = create(:organization)
+    expect(org).to be_valid
+  end
+
+  it "requires name" do
+    org = build(:organization, name: nil)
+    expect(org).not_to be_valid
+    expect(org.errors[:name]).to include("can't be blank")
+  end
+
+  it "requires slug" do
+    org = build(:organization, slug: nil)
+    expect(org).not_to be_valid
+    expect(org.errors[:slug]).to include("can't be blank")
+  end
+
+  it "validates slug uniqueness" do
+    existing = create(:organization)
+    org = build(:organization, slug: existing.slug)
+    expect(org).not_to be_valid
+    expect(org.errors[:slug]).to include("has already been taken")
+  end
+
+  it "rejects uppercase slugs" do
+    org = build(:organization, slug: "BadSlug")
+    expect(org).not_to be_valid
+    expect(org.errors[:slug]).to include("only allows lowercase letters, numbers, and hyphens")
+  end
+
+  it "returns true for allowed email domain" do
+    org = create(:organization, allowed_email_domains: ["acme.com"])
+    expect(org.email_domain_allowed?("user@acme.com")).to be true
+  end
+
+  it "returns false for disallowed email domain" do
+    org = create(:organization, allowed_email_domains: ["acme.com"])
+    expect(org.email_domain_allowed?("user@other.com")).to be false
+  end
+
+  it "checks email domain case-insensitively" do
+    org = create(:organization, allowed_email_domains: ["acme.com"])
+    expect(org.email_domain_allowed?("user@ACME.COM")).to be true
+  end
+
+  it "defaults allowed_email_domains to empty array" do
+    org = Organization.new
+    expect(org.allowed_email_domains).to eq([])
+  end
+end
diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb
new file mode 100644
index 00000000..22310c15
--- /dev/null
+++ b/spec/models/plan_spec.rb
@@ -0,0 +1,35 @@
+require "rails_helper"
+
+RSpec.describe Plan, type: :model do
+  it "is valid with valid attributes" do
+    plan = create(:plan)
+    expect(plan).to be_valid
+  end
+
+  it "requires title" do
+    plan = build(:plan, title: nil)
+    expect(plan).not_to be_valid
+    expect(plan.errors[:title]).to include("can't be blank")
+  end
+
+  it "validates status inclusion" do
+    plan = create(:plan)
+    plan.status = "invalid"
+    expect(plan).not_to be_valid
+  end
+
+  it "defaults status to brainstorm" do
+    plan = Plan.new
+    expect(plan.status).to eq("brainstorm")
+  end
+
+  it "returns current content from version" do
+    plan = create(:plan)
+    expect(plan.current_content).to include("Plan Content")
+  end
+
+  it "returns id for to_param" do
+    plan = create(:plan)
+    expect(plan.to_param).to eq(plan.id)
+  end
+end
diff --git a/spec/models/plan_version_spec.rb b/spec/models/plan_version_spec.rb
new file mode 100644
index 00000000..964d40c7
--- /dev/null
+++ b/spec/models/plan_version_spec.rb
@@ -0,0 +1,28 @@
+require "rails_helper"
+
+RSpec.describe PlanVersion, type: :model do
+  it "is valid with valid attributes" do
+    plan = create(:plan)
+    version = plan.current_plan_version
+    expect(version).to be_valid
+  end
+
+  it "computes sha256 automatically" do
+    version = build(:plan_version, content_markdown: "test content", content_sha256: nil)
+    expect(version).to be_valid
+    expect(version.content_sha256).to eq(Digest::SHA256.hexdigest("test content"))
+  end
+
+  it "validates revision uniqueness per plan" do
+    plan = create(:plan)
+    existing_version = plan.current_plan_version
+    version = build(:plan_version, plan: plan, organization: plan.organization, revision: existing_version.revision)
+    expect(version).not_to be_valid
+    expect(version.errors[:revision]).to include("has already been taken")
+  end
+
+  it "validates actor_type inclusion" do
+    version = build(:plan_version, actor_type: "robot")
+    expect(version).not_to be_valid
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
new file mode 100644
index 00000000..b7471be6
--- /dev/null
+++ b/spec/models/user_spec.rb
@@ -0,0 +1,63 @@
+require "rails_helper"
+
+RSpec.describe User, type: :model do
+  it "is valid with valid attributes" do
+    user = create(:user)
+    expect(user).to be_valid
+  end
+
+  it "requires email" do
+    user = build(:user, email: nil)
+    expect(user).not_to be_valid
+    expect(user.errors[:email]).to include("can't be blank")
+  end
+
+  it "requires name" do
+    user = build(:user, name: nil)
+    expect(user).not_to be_valid
+    expect(user.errors[:name]).to include("can't be blank")
+  end
+
+  it "validates email uniqueness within organization" do
+    existing = create(:user)
+    user = build(:user, organization: existing.organization, email: existing.email)
+    expect(user).not_to be_valid
+    expect(user.errors[:email]).to include("has already been taken")
+  end
+
+  it "allows same email in different orgs" do
+    org1 = create(:organization, allowed_email_domains: ["test.com"])
+    org2 = create(:organization, allowed_email_domains: ["test.com"])
+    create(:user, organization: org1, email: "same@test.com")
+    user = build(:user, organization: org2, email: "same@test.com")
+    expect(user).to be_valid
+  end
+
+  it "validates org_role inclusion" do
+    user = build(:user, org_role: "superadmin")
+    expect(user).not_to be_valid
+    expect(user.errors[:org_role]).to include("is not included in the list")
+  end
+
+  it "returns true for admin?" do
+    user = create(:user, :admin)
+    expect(user).to be_admin
+  end
+
+  it "returns false for admin? when member" do
+    user = create(:user, org_role: "member")
+    expect(user).not_to be_admin
+  end
+
+  it "validates email domain against org allowed domains" do
+    org = create(:organization, allowed_email_domains: ["acme.com"])
+    user = build(:user, organization: org, email: "user@other.com")
+    expect(user).not_to be_valid
+    expect(user.errors[:email]).to include("domain is not allowed for this organization")
+  end
+
+  it "extracts email domain" do
+    user = build(:user, email: "alice@example.com")
+    expect(user.email_domain).to eq("example.com")
+  end
+end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
new file mode 100644
index 00000000..adbc521b
--- /dev/null
+++ b/spec/rails_helper.rb
@@ -0,0 +1,24 @@
+require "spec_helper"
+ENV["RAILS_ENV"] ||= "test"
+require_relative "../config/environment"
+abort("The Rails environment is running in production mode!") if Rails.env.production?
+require "rspec/rails"
+
+begin
+  ActiveRecord::Migration.maintain_test_schema!
+rescue ActiveRecord::PendingMigrationError => e
+  abort e.to_s.strip
+end
+
+RSpec.configure do |config|
+  config.use_transactional_fixtures = true
+  config.infer_spec_type_from_file_location!
+  config.filter_rails_from_backtrace!
+
+  config.include FactoryBot::Syntax::Methods
+  config.include ActiveSupport::Testing::TimeHelpers
+
+  def sign_in_as(user)
+    post sign_in_path, params: { email: user.email }
+  end
+end
diff --git a/spec/requests/api/v1/comments_spec.rb b/spec/requests/api/v1/comments_spec.rb
new file mode 100644
index 00000000..b5340b60
--- /dev/null
+++ b/spec/requests/api/v1/comments_spec.rb
@@ -0,0 +1,68 @@
+require "rails_helper"
+
+RSpec.describe "Api::V1::Comments", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:alice_token) { create(:api_token, organization: org, user: alice, raw_token: "test-token-alice") }
+  let(:headers) { { "Authorization" => "Bearer test-token-alice" } }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice) }
+  let(:thread_record) { create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice) }
+
+  before do
+    alice_token # ensure token exists
+  end
+
+  it "create comment thread" do
+    expect {
+      post api_v1_plan_comments_path(plan),
+        params: {
+          body_markdown: "API comment here",
+          start_line: 1,
+          end_line: 3
+        },
+        headers: headers,
+        as: :json
+    }.to change(CommentThread, :count).by(1).and change(Comment, :count).by(1)
+    expect(response).to have_http_status(:created)
+    body = JSON.parse(response.body)
+    expect(body["thread_id"]).to be_present
+    expect(body["comment_id"]).to be_present
+  end
+
+  it "create general comment thread" do
+    expect {
+      post api_v1_plan_comments_path(plan),
+        params: { body_markdown: "General API feedback" },
+        headers: headers,
+        as: :json
+    }.to change(CommentThread, :count).by(1)
+    expect(response).to have_http_status(:created)
+  end
+
+  it "reply to thread" do
+    expect {
+      post reply_api_v1_plan_comment_path(plan, thread_record),
+        params: { body_markdown: "API reply" },
+        headers: headers,
+        as: :json
+    }.to change(Comment, :count).by(1)
+    expect(response).to have_http_status(:created)
+    body = JSON.parse(response.body)
+    expect(body["thread_id"]).to eq(thread_record.id)
+  end
+
+  it "reply to nonexistent thread" do
+    post reply_api_v1_plan_comment_path(plan, "nonexistent-id"),
+      params: { body_markdown: "Reply" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:not_found)
+  end
+
+  it "create comment requires auth" do
+    post api_v1_plan_comments_path(plan),
+      params: { body_markdown: "No auth" },
+      as: :json
+    expect(response).to have_http_status(:unauthorized)
+  end
+end
diff --git a/spec/requests/api/v1/leases_spec.rb b/spec/requests/api/v1/leases_spec.rb
new file mode 100644
index 00000000..22b8e108
--- /dev/null
+++ b/spec/requests/api/v1/leases_spec.rb
@@ -0,0 +1,90 @@
+require "rails_helper"
+
+RSpec.describe "Api::V1::Leases", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:bob) { create(:user, organization: org) }
+  let(:alice_token) { create(:api_token, organization: org, user: alice, raw_token: "test-token-alice") }
+  let(:bob_token) { create(:api_token, organization: org, user: bob, raw_token: "test-token-bob") }
+  let(:headers) { { "Authorization" => "Bearer test-token-alice" } }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice) }
+
+  before do
+    alice_token # ensure token exists
+  end
+
+  it "acquire lease" do
+    post api_v1_plan_lease_path(plan),
+      params: { lease_token: "my-token" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:created)
+    body = JSON.parse(response.body)
+    expect(body["lease_token"]).to eq("my-token")
+    expect(body["expires_at"]).to be_present
+  end
+
+  it "acquire lease generates token if not provided" do
+    post api_v1_plan_lease_path(plan), headers: headers, as: :json
+    expect(response).to have_http_status(:created)
+    body = JSON.parse(response.body)
+    expect(body["lease_token"]).to be_present
+  end
+
+  it "acquire lease conflicts when held by another" do
+    post api_v1_plan_lease_path(plan),
+      params: { lease_token: "first-token" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:created)
+
+    bob_token # ensure bob's token exists
+    bob_headers = { "Authorization" => "Bearer test-token-bob" }
+    post api_v1_plan_lease_path(plan),
+      params: { lease_token: "second-token" },
+      headers: bob_headers,
+      as: :json
+    expect(response).to have_http_status(:conflict)
+  end
+
+  it "renew lease" do
+    post api_v1_plan_lease_path(plan),
+      params: { lease_token: "my-token" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:created)
+
+    patch api_v1_plan_lease_path(plan),
+      params: { lease_token: "my-token" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:success)
+  end
+
+  it "renew lease with wrong token" do
+    post api_v1_plan_lease_path(plan),
+      params: { lease_token: "my-token" },
+      headers: headers,
+      as: :json
+
+    patch api_v1_plan_lease_path(plan),
+      params: { lease_token: "wrong-token" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:conflict)
+  end
+
+  it "release lease" do
+    post api_v1_plan_lease_path(plan),
+      params: { lease_token: "my-token" },
+      headers: headers,
+      as: :json
+
+    delete api_v1_plan_lease_path(plan),
+      params: { lease_token: "my-token" },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:no_content)
+    expect(EditLease.find_by(plan_id: plan.id)).to be_nil
+  end
+end
diff --git a/spec/requests/api/v1/operations_spec.rb b/spec/requests/api/v1/operations_spec.rb
new file mode 100644
index 00000000..f03251e6
--- /dev/null
+++ b/spec/requests/api/v1/operations_spec.rb
@@ -0,0 +1,87 @@
+require "rails_helper"
+
+RSpec.describe "Api::V1::Operations", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:alice_token) { create(:api_token, organization: org, user: alice, raw_token: "test-token-alice") }
+  let(:headers) { { "Authorization" => "Bearer test-token-alice" } }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice) }
+  let(:lease_token) { SecureRandom.hex(32) }
+
+  before do
+    alice_token # ensure token exists
+    EditLease.acquire!(
+      plan: plan,
+      holder_type: "local_agent",
+      holder_id: alice_token.id,
+      lease_token: lease_token
+    )
+  end
+
+  it "apply operations creates new version" do
+    expect {
+      post api_v1_plan_operations_path(plan),
+        params: {
+          lease_token: lease_token,
+          base_revision: plan.current_revision,
+          operations: [
+            { op: "replace_exact", old_text: "Some content here.", new_text: "Updated content.", count: 1 }
+          ]
+        },
+        headers: headers,
+        as: :json
+    }.to change(PlanVersion, :count).by(1)
+    expect(response).to have_http_status(:created)
+    body = JSON.parse(response.body)
+    expect(body["revision"]).to eq(plan.current_revision + 1)
+  end
+
+  it "apply operations fails without lease" do
+    EditLease.find_by(plan_id: plan.id)&.destroy
+
+    post api_v1_plan_operations_path(plan),
+      params: {
+        lease_token: "no-lease",
+        base_revision: plan.current_revision,
+        operations: [{ op: "replace_exact", old_text: "x", new_text: "y", count: 1 }]
+      },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:conflict)
+  end
+
+  it "apply operations fails on stale revision" do
+    post api_v1_plan_operations_path(plan),
+      params: {
+        lease_token: lease_token,
+        base_revision: 999,
+        operations: [{ op: "replace_exact", old_text: "x", new_text: "y", count: 1 }]
+      },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:conflict)
+  end
+
+  it "apply operations fails on invalid operation" do
+    post api_v1_plan_operations_path(plan),
+      params: {
+        lease_token: lease_token,
+        base_revision: plan.current_revision,
+        operations: [{ op: "replace_exact", old_text: "nonexistent text", new_text: "y", count: 1 }]
+      },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:unprocessable_entity)
+  end
+
+  it "apply operations requires lease_token" do
+    post api_v1_plan_operations_path(plan),
+      params: {
+        base_revision: plan.current_revision,
+        operations: [{ op: "replace_exact", old_text: "x", new_text: "y", count: 1 }]
+      },
+      headers: headers,
+      as: :json
+    expect(response).to have_http_status(:unprocessable_entity)
+  end
+end
diff --git a/spec/requests/api/v1/plans_spec.rb b/spec/requests/api/v1/plans_spec.rb
new file mode 100644
index 00000000..e5feb4a8
--- /dev/null
+++ b/spec/requests/api/v1/plans_spec.rb
@@ -0,0 +1,88 @@
+require "rails_helper"
+
+RSpec.describe "Api::V1::Plans", type: :request do
+  let(:org) { create(:organization) }
+  let(:other_org) { create(:organization, allowed_email_domains: ["other.com"]) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:carol) { create(:user, :admin, organization: other_org, email: "carol@other.com") }
+  let(:alice_token) { create(:api_token, organization: org, user: alice, raw_token: "test-token-alice") }
+  let(:carol_token) { create(:api_token, organization: other_org, user: carol, raw_token: "test-token-carol") }
+  let(:revoked_token) { create(:api_token, :revoked, organization: org, user: alice, raw_token: "test-token-revoked") }
+  let(:headers) { { "Authorization" => "Bearer test-token-alice" } }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice, title: "Acme Roadmap") }
+
+  before do
+    alice_token # ensure token exists
+  end
+
+  it "index returns plans" do
+    plan # trigger creation
+    get api_v1_plans_path, headers: headers
+    expect(response).to have_http_status(:success)
+    plans = JSON.parse(response.body)
+    expect(plans.any? { |p| p["title"] == "Acme Roadmap" }).to be true
+  end
+
+  it "index excludes other org plans" do
+    plan # trigger creation
+    carol_token # ensure token exists
+    get api_v1_plans_path, headers: { "Authorization" => "Bearer test-token-carol" }
+    expect(response).to have_http_status(:success)
+    plans = JSON.parse(response.body)
+    expect(plans.any? { |p| p["title"] == "Acme Roadmap" }).to be false
+  end
+
+  it "index requires auth" do
+    get api_v1_plans_path
+    expect(response).to have_http_status(:unauthorized)
+  end
+
+  it "index with revoked token" do
+    revoked_token # ensure token exists
+    get api_v1_plans_path, headers: { "Authorization" => "Bearer test-token-revoked" }
+    expect(response).to have_http_status(:unauthorized)
+  end
+
+  it "show returns plan" do
+    get api_v1_plan_path(plan), headers: headers
+    expect(response).to have_http_status(:success)
+    body = JSON.parse(response.body)
+    expect(body["title"]).to eq("Acme Roadmap")
+    expect(body["current_content"]).to be_present
+  end
+
+  it "show returns 404 for other org plan" do
+    carol_token # ensure token exists
+    get api_v1_plan_path(plan), headers: { "Authorization" => "Bearer test-token-carol" }
+    expect(response).to have_http_status(:not_found)
+  end
+
+  it "create creates new plan" do
+    expect {
+      post api_v1_plans_path, params: { title: "API Plan", content: "# API Plan\n\nCreated via API." }, headers: headers, as: :json
+    }.to change(Plan, :count).by(1)
+    expect(response).to have_http_status(:created)
+    body = JSON.parse(response.body)
+    expect(body["title"]).to eq("API Plan")
+    expect(body["current_revision"]).to eq(1)
+  end
+
+  it "create without title fails" do
+    post api_v1_plans_path, params: { content: "no title" }, headers: headers, as: :json
+    expect(response).to have_http_status(:unprocessable_entity)
+  end
+
+  it "versions returns version list" do
+    get versions_api_v1_plan_path(plan), headers: headers
+    expect(response).to have_http_status(:success)
+    versions = JSON.parse(response.body)
+    expect(versions.any? { |v| v["revision"] == 1 }).to be true
+  end
+
+  it "comments returns thread list" do
+    get comments_api_v1_plan_path(plan), headers: headers
+    expect(response).to have_http_status(:success)
+    threads = JSON.parse(response.body)
+    expect(threads).to be_a(Array)
+  end
+end
diff --git a/spec/requests/api_tokens_spec.rb b/spec/requests/api_tokens_spec.rb
new file mode 100644
index 00000000..eb250467
--- /dev/null
+++ b/spec/requests/api_tokens_spec.rb
@@ -0,0 +1,38 @@
+require "rails_helper"
+
+RSpec.describe "ApiTokens", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+
+  before { sign_in_as(alice) }
+
+  it "index shows tokens" do
+    create(:api_token, organization: org, user: alice)
+    get api_tokens_path
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include("data-table")
+  end
+
+  it "create token shows raw token" do
+    expect {
+      post api_tokens_path, params: { api_token: { name: "Test Token" } }
+    }.to change(ApiToken, :count).by(1)
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include("token-reveal")
+  end
+
+  it "revoke token" do
+    token = create(:api_token, organization: org, user: alice)
+    expect(token).not_to be_revoked
+    patch revoke_api_token_path(token)
+    expect(response).to redirect_to(api_tokens_path)
+    token.reload
+    expect(token).to be_revoked
+  end
+
+  it "requires authentication" do
+    delete sign_out_path
+    get api_tokens_path
+    expect(response).to redirect_to(sign_in_path)
+  end
+end
diff --git a/spec/requests/comment_threads_spec.rb b/spec/requests/comment_threads_spec.rb
new file mode 100644
index 00000000..7d54a4b0
--- /dev/null
+++ b/spec/requests/comment_threads_spec.rb
@@ -0,0 +1,78 @@
+require "rails_helper"
+
+RSpec.describe "CommentThreads", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:bob) { create(:user, organization: org) }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice) }
+
+  before { sign_in_as(alice) }
+
+  it "create comment thread with anchor text" do
+    expect {
+      post plan_comment_threads_path(plan), params: {
+        comment_thread: {
+          anchor_text: "world domination",
+          body_markdown: "This needs work."
+        }
+      }
+    }.to change(CommentThread, :count).by(1).and change(Comment, :count).by(1)
+    expect(response).to redirect_to(plan_path(plan))
+    thread = CommentThread.last
+    expect(thread.anchor_text).to eq("world domination")
+    expect(thread.status).to eq("open")
+    expect(thread.plan_version_id).to eq(plan.current_plan_version_id)
+  end
+
+  it "create general comment thread" do
+    expect {
+      post plan_comment_threads_path(plan), params: {
+        comment_thread: {
+          body_markdown: "General feedback."
+        }
+      }
+    }.to change(CommentThread, :count).by(1)
+    thread = CommentThread.last
+    expect(thread.anchor_text).to be_nil
+  end
+
+  it "resolve thread" do
+    thread = create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice)
+    patch resolve_plan_comment_thread_path(plan, thread)
+    expect(response).to redirect_to(plan_path(plan))
+    thread.reload
+    expect(thread.status).to eq("resolved")
+  end
+
+  it "accept thread as plan author" do
+    thread = create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice)
+    patch accept_plan_comment_thread_path(plan, thread)
+    thread.reload
+    expect(thread.status).to eq("accepted")
+  end
+
+  it "dismiss thread as plan author" do
+    thread = create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice)
+    patch dismiss_plan_comment_thread_path(plan, thread)
+    thread.reload
+    expect(thread.status).to eq("dismissed")
+  end
+
+  it "reopen resolved thread" do
+    thread = create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice)
+    thread.resolve!(alice)
+    patch reopen_plan_comment_thread_path(plan, thread)
+    thread.reload
+    expect(thread.status).to eq("open")
+    expect(thread.resolved_by_user_id).to be_nil
+  end
+
+  it "non-author cannot accept thread" do
+    sign_in_as(bob)
+    thread = create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: bob)
+    patch accept_plan_comment_thread_path(plan, thread)
+    expect(response).to have_http_status(:not_found)
+    thread.reload
+    expect(thread.status).to eq("open")
+  end
+end
diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb
new file mode 100644
index 00000000..9a952a5a
--- /dev/null
+++ b/spec/requests/comments_spec.rb
@@ -0,0 +1,22 @@
+require "rails_helper"
+
+RSpec.describe "Comments", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice) }
+  let(:thread_record) { create(:comment_thread, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice) }
+
+  before { sign_in_as(alice) }
+
+  it "create reply" do
+    expect {
+      post plan_comment_thread_comments_path(plan, thread_record), params: {
+        comment: { body_markdown: "I agree with this." }
+      }
+    }.to change(Comment, :count).by(1)
+    expect(response).to redirect_to(plan_path(plan))
+    comment = Comment.last
+    expect(comment.author_type).to eq("human")
+    expect(comment.author_id).to eq(alice.id)
+  end
+end
diff --git a/spec/requests/plans_spec.rb b/spec/requests/plans_spec.rb
new file mode 100644
index 00000000..537cbd34
--- /dev/null
+++ b/spec/requests/plans_spec.rb
@@ -0,0 +1,79 @@
+require "rails_helper"
+
+RSpec.describe "Plans", type: :request do
+  let(:org) { create(:organization) }
+  let(:alice) { create(:user, :admin, organization: org) }
+  let(:bob) { create(:user, organization: org) }
+  let(:plan) { create(:plan, :considering, organization: org, created_by_user: alice) }
+  let(:brainstorm_plan) { create(:plan, :brainstorm, organization: org, created_by_user: alice) }
+
+  before { sign_in_as(alice) }
+
+  it "index shows plans" do
+    plan # trigger creation
+    get plans_path
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include(plan.title)
+  end
+
+  it "index filters by status" do
+    plan # trigger creation
+    get plans_path(status: "considering")
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include(plan.title)
+  end
+
+  it "show plan renders successfully" do
+    get plan_path(plan)
+    expect(response).to have_http_status(:success)
+  end
+
+  it "show plan displays comments sidebar" do
+    get plan_path(plan)
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include("comment-threads-list")
+  end
+
+  it "show plan includes turbo stream subscription" do
+    get plan_path(plan)
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include("turbo-cable-stream-source")
+  end
+
+  it "edit plan" do
+    get edit_plan_path(plan)
+    expect(response).to have_http_status(:success)
+  end
+
+  it "update plan creates new version" do
+    plan # trigger creation
+    expect {
+      patch plan_path(plan), params: {
+        plan: { title: "Updated Title", content_markdown: "# Updated", change_summary: "Revised" }
+      }
+    }.to change(PlanVersion, :count).by(1)
+    plan.reload
+    expect(plan.title).to eq("Updated Title")
+    expect(plan.current_revision).to eq(2)
+    expect(response).to redirect_to(plan_path(plan))
+  end
+
+  it "cannot view brainstorm plan as non-author" do
+    sign_in_as(bob)
+    get plan_path(brainstorm_plan)
+    expect(response).to have_http_status(:not_found)
+  end
+
+  it "update marks existing threads as out of date" do
+    thread = create(:comment_thread, :with_anchor, plan: plan, organization: org, plan_version: plan.current_plan_version, created_by_user: alice, anchor_text: "original roadmap text")
+    expect(thread).not_to be_out_of_date
+
+    patch plan_path(plan), params: {
+      plan: { title: plan.title, content_markdown: "# Updated content", change_summary: "Updated" }
+    }
+
+    thread.reload
+    expect(thread).to be_out_of_date
+    expect(thread.out_of_date_since_version_id).not_to be_nil
+  end
+end
diff --git a/spec/requests/sessions_spec.rb b/spec/requests/sessions_spec.rb
new file mode 100644
index 00000000..0bacff73
--- /dev/null
+++ b/spec/requests/sessions_spec.rb
@@ -0,0 +1,47 @@
+require "rails_helper"
+
+RSpec.describe "Sessions", type: :request do
+  let(:org) { create(:organization, allowed_email_domains: ["acme.com"]) }
+  let!(:alice) { create(:user, :admin, organization: org, email: "alice@acme.com", name: "Alice") }
+
+  it "sign in page renders" do
+    get sign_in_path
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include("email")
+  end
+
+  it "sign in with valid email creates session" do
+    post sign_in_path, params: { email: "alice@acme.com" }
+    expect(response).to redirect_to(root_path)
+    follow_redirect!
+    expect(response).to have_http_status(:success)
+    expect(response.body).to include("Alice")
+  end
+
+  it "sign in with unknown domain shows error" do
+    post sign_in_path, params: { email: "user@unknown.com" }
+    expect(response).to have_http_status(:unprocessable_entity)
+    expect(response.body).to include("No organization found")
+  end
+
+  it "sign in creates new user if not exists" do
+    expect {
+      post sign_in_path, params: { email: "newuser@acme.com" }
+    }.to change(User, :count).by(1)
+    expect(response).to redirect_to(root_path)
+  end
+
+  it "sign out clears session" do
+    post sign_in_path, params: { email: "alice@acme.com" }
+    delete sign_out_path
+    expect(response).to redirect_to(sign_in_path)
+
+    get root_path
+    expect(response).to redirect_to(sign_in_path)
+  end
+
+  it "unauthenticated access redirects to sign in" do
+    get root_path
+    expect(response).to redirect_to(sign_in_path)
+  end
+end
diff --git a/spec/services/plans/apply_operations_spec.rb b/spec/services/plans/apply_operations_spec.rb
new file mode 100644
index 00000000..36bf2fc0
--- /dev/null
+++ b/spec/services/plans/apply_operations_spec.rb
@@ -0,0 +1,150 @@
+require "rails_helper"
+
+RSpec.describe Plans::ApplyOperations do
+  describe "replace_exact" do
+    it "replaces text" do
+      result = Plans::ApplyOperations.call(
+        content: "Hello world, hello universe.",
+        operations: [{ "op" => "replace_exact", "old_text" => "world", "new_text" => "planet", "count" => 1 }]
+      )
+      expect(result[:content]).to eq("Hello planet, hello universe.")
+      expect(result[:applied].length).to eq(1)
+    end
+
+    it "replaces all occurrences with count 2" do
+      result = Plans::ApplyOperations.call(
+        content: "foo bar foo baz",
+        operations: [{ "op" => "replace_exact", "old_text" => "foo", "new_text" => "qux", "count" => 2 }]
+      )
+      expect(result[:content]).to eq("qux bar qux baz")
+    end
+
+    it "fails when text not found" do
+      expect {
+        Plans::ApplyOperations.call(
+          content: "Hello world",
+          operations: [{ "op" => "replace_exact", "old_text" => "missing", "new_text" => "found", "count" => 1 }]
+        )
+      }.to raise_error(Plans::OperationError, /found 0 occurrences/)
+    end
+
+    it "fails when too many occurrences" do
+      expect {
+        Plans::ApplyOperations.call(
+          content: "foo foo foo",
+          operations: [{ "op" => "replace_exact", "old_text" => "foo", "new_text" => "bar", "count" => 1 }]
+        )
+      }.to raise_error(Plans::OperationError, /found 3 occurrences/)
+    end
+
+    it "requires old_text" do
+      expect {
+        Plans::ApplyOperations.call(
+          content: "Hello",
+          operations: [{ "op" => "replace_exact", "new_text" => "Bye" }]
+        )
+      }.to raise_error(Plans::OperationError, /requires 'old_text'/)
+    end
+  end
+
+  describe "insert_under_heading" do
+    it "inserts content under heading" do
+      content = "# Title\n\nIntro\n\n## Goals\n\nExisting goals."
+      result = Plans::ApplyOperations.call(
+        content: content,
+        operations: [{ "op" => "insert_under_heading", "heading" => "## Goals", "content" => "- New goal" }]
+      )
+      expect(result[:content]).to include("## Goals\n\n- New goal")
+      expect(result[:content]).to include("Existing goals.")
+    end
+
+    it "fails when heading not found" do
+      expect {
+        Plans::ApplyOperations.call(
+          content: "# Title\n\nContent",
+          operations: [{ "op" => "insert_under_heading", "heading" => "## Missing", "content" => "stuff" }]
+        )
+      }.to raise_error(Plans::OperationError, /no heading matching/)
+    end
+
+    it "fails when heading is ambiguous" do
+      content = "## Goals\n\nFirst\n\n## Goals\n\nSecond"
+      expect {
+        Plans::ApplyOperations.call(
+          content: content,
+          operations: [{ "op" => "insert_under_heading", "heading" => "## Goals", "content" => "stuff" }]
+        )
+      }.to raise_error(Plans::OperationError, /found 2 headings/)
+    end
+  end
+
+  describe "delete_paragraph_containing" do
+    it "removes paragraph" do
+      content = "First paragraph.\n\nThis is deprecated.\n\nThird paragraph."
+      result = Plans::ApplyOperations.call(
+        content: content,
+        operations: [{ "op" => "delete_paragraph_containing", "needle" => "deprecated" }]
+      )
+      expect(result[:content]).to eq("First paragraph.\n\nThird paragraph.")
+      expect(result[:content]).not_to include("deprecated")
+    end
+
+    it "fails when not found" do
+      expect {
+        Plans::ApplyOperations.call(
+          content: "Some content.",
+          operations: [{ "op" => "delete_paragraph_containing", "needle" => "missing" }]
+        )
+      }.to raise_error(Plans::OperationError, /no paragraph containing/)
+    end
+
+    it "fails when ambiguous" do
+      content = "First deprecated thing.\n\nSecond deprecated thing."
+      expect {
+        Plans::ApplyOperations.call(
+          content: content,
+          operations: [{ "op" => "delete_paragraph_containing", "needle" => "deprecated" }]
+        )
+      }.to raise_error(Plans::OperationError, /found 2 paragraphs/)
+    end
+  end
+
+  it "raises error for unknown operation" do
+    expect {
+      Plans::ApplyOperations.call(
+        content: "Hello",
+        operations: [{ "op" => "unknown_op" }]
+      )
+    }.to raise_error(Plans::OperationError, /unknown op/)
+  end
+
+  it "applies multiple operations sequentially" do
+    content = "# Plan\n\n## Phase 1\n\nDo stuff.\n\n## Phase 2\n\nOld approach."
+    result = Plans::ApplyOperations.call(
+      content: content,
+      operations: [
+        { "op" => "replace_exact", "old_text" => "Do stuff.", "new_text" => "Do important stuff.", "count" => 1 },
+        { "op" => "insert_under_heading", "heading" => "## Phase 2", "content" => "\n- New step" }
+      ]
+    )
+    expect(result[:content]).to include("Do important stuff.")
+    expect(result[:content]).to include("- New step")
+    expect(result[:applied].length).to eq(2)
+  end
+
+  it "works with string keys" do
+    result = Plans::ApplyOperations.call(
+      content: "Hello world",
+      operations: [{ "op" => "replace_exact", "old_text" => "world", "new_text" => "planet", "count" => 1 }]
+    )
+    expect(result[:content]).to eq("Hello planet")
+  end
+
+  it "works with symbol keys" do
+    result = Plans::ApplyOperations.call(
+      content: "Hello world",
+      operations: [{ op: "replace_exact", old_text: "world", new_text: "planet", count: 1 }]
+    )
+    expect(result[:content]).to eq("Hello planet")
+  end
+end
diff --git a/spec/services/plans/create_spec.rb b/spec/services/plans/create_spec.rb
new file mode 100644
index 00000000..ca7a77ea
--- /dev/null
+++ b/spec/services/plans/create_spec.rb
@@ -0,0 +1,27 @@
+require "rails_helper"
+
+RSpec.describe Plans::Create do
+  it "creates plan with initial version" do
+    user = create(:user)
+    plan = Plans::Create.call(
+      title: "New Plan",
+      content: "# New Plan\n\nSome content.",
+      user: user
+    )
+
+    expect(plan).to be_persisted
+    expect(plan.title).to eq("New Plan")
+    expect(plan.status).to eq("brainstorm")
+    expect(plan.created_by_user).to eq(user)
+    expect(plan.organization).to eq(user.organization)
+    expect(plan.current_revision).to eq(1)
+    expect(plan.plan_versions.count).to eq(1)
+
+    version = plan.current_plan_version
+    expect(version.content_markdown).to eq("# New Plan\n\nSome content.")
+    expect(version.revision).to eq(1)
+    expect(version.actor_type).to eq("human")
+    expect(version.actor_id).to eq(user.id)
+    expect(version.content_sha256).to be_present
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
new file mode 100644
index 00000000..327b58ea
--- /dev/null
+++ b/spec/spec_helper.rb
@@ -0,0 +1,94 @@
+# This file was generated by the `rails generate rspec:install` command. Conventionally, all
+# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`.
+# The generated `.rspec` file contains `--require spec_helper` which will cause
+# this file to always be loaded, without a need to explicitly require it in any
+# files.
+#
+# Given that it is always loaded, you are encouraged to keep this file as
+# light-weight as possible. Requiring heavyweight dependencies from this file
+# will add to the boot time of your test suite on EVERY test run, even for an
+# individual file that may not need all of that loaded. Instead, consider making
+# a separate helper file that requires the additional dependencies and performs
+# the additional setup, and require it from the spec files that actually need
+# it.
+#
+# See https://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
+RSpec.configure do |config|
+  # rspec-expectations config goes here. You can use an alternate
+  # assertion/expectation library such as wrong or the stdlib/minitest
+  # assertions if you prefer.
+  config.expect_with :rspec do |expectations|
+    # This option will default to `true` in RSpec 4. It makes the `description`
+    # and `failure_message` of custom matchers include text for helper methods
+    # defined using `chain`, e.g.:
+    #     be_bigger_than(2).and_smaller_than(4).description
+    #     # => "be bigger than 2 and smaller than 4"
+    # ...rather than:
+    #     # => "be bigger than 2"
+    expectations.include_chain_clauses_in_custom_matcher_descriptions = true
+  end
+
+  # rspec-mocks config goes here. You can use an alternate test double
+  # library (such as bogus or mocha) by changing the `mock_with` option here.
+  config.mock_with :rspec do |mocks|
+    # Prevents you from mocking or stubbing a method that does not exist on
+    # a real object. This is generally recommended, and will default to
+    # `true` in RSpec 4.
+    mocks.verify_partial_doubles = true
+  end
+
+  # This option will default to `:apply_to_host_groups` in RSpec 4 (and will
+  # have no way to turn it off -- the option exists only for backwards
+  # compatibility in RSpec 3). It causes shared context metadata to be
+  # inherited by the metadata hash of host groups and examples, rather than
+  # triggering implicit auto-inclusion in groups with matching metadata.
+  config.shared_context_metadata_behavior = :apply_to_host_groups
+
+# The settings below are suggested to provide a good initial experience
+# with RSpec, but feel free to customize to your heart's content.
+=begin
+  # This allows you to limit a spec run to individual examples or groups
+  # you care about by tagging them with `:focus` metadata. When nothing
+  # is tagged with `:focus`, all examples get run. RSpec also provides
+  # aliases for `it`, `describe`, and `context` that include `:focus`
+  # metadata: `fit`, `fdescribe` and `fcontext`, respectively.
+  config.filter_run_when_matching :focus
+
+  # Allows RSpec to persist some state between runs in order to support
+  # the `--only-failures` and `--next-failure` CLI options. We recommend
+  # you configure your source control system to ignore this file.
+  config.example_status_persistence_file_path = "spec/examples.txt"
+
+  # Limits the available syntax to the non-monkey patched syntax that is
+  # recommended. For more details, see:
+  # https://rspec.info/features/3-12/rspec-core/configuration/zero-monkey-patching-mode/
+  config.disable_monkey_patching!
+
+  # Many RSpec users commonly either run the entire suite or an individual
+  # file, and it's useful to allow more verbose output when running an
+  # individual spec file.
+  if config.files_to_run.one?
+    # Use the documentation formatter for detailed output,
+    # unless a formatter has already been configured
+    # (e.g. via a command-line flag).
+    config.default_formatter = "doc"
+  end
+
+  # Print the 10 slowest examples and example groups at the
+  # end of the spec run, to help surface which specs are running
+  # particularly slow.
+  config.profile_examples = 10
+
+  # Run specs in random order to surface order dependencies. If you find an
+  # order dependency and want to debug it, you can fix the order by providing
+  # the seed, which is printed after each run.
+  #     --seed 1234
+  config.order = :random
+
+  # Seed global randomization in this process using the `--seed` CLI option.
+  # Setting this allows you to use `--seed` to deterministically reproduce
+  # test failures related to randomization by passing the same `--seed` value
+  # as the one that triggered the failure.
+  Kernel.srand config.seed
+=end
+end
diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb
deleted file mode 100644
index cee29fd2..00000000
--- a/test/application_system_test_case.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-require "test_helper"
-
-class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
-  driven_by :selenium, using: :headless_chrome, screen_size: [ 1400, 1400 ]
-end
diff --git a/test/controllers/.keep b/test/controllers/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/controllers/api/v1/comments_controller_test.rb b/test/controllers/api/v1/comments_controller_test.rb
deleted file mode 100644
index 45a71f5b..00000000
--- a/test/controllers/api/v1/comments_controller_test.rb
+++ /dev/null
@@ -1,63 +0,0 @@
-require "test_helper"
-
-class Api::V1::CommentsControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    @headers = { "Authorization" => "Bearer test-token-alice" }
-    @plan = plans(:acme_roadmap)
-  end
-
-  test "create comment thread" do
-    assert_difference ["CommentThread.count", "Comment.count"], 1 do
-      post api_v1_plan_comments_path(@plan),
-        params: {
-          body_markdown: "API comment here",
-          start_line: 1,
-          end_line: 3
-        },
-        headers: @headers,
-        as: :json
-    end
-    assert_response :created
-    body = JSON.parse(response.body)
-    assert body["thread_id"].present?
-    assert body["comment_id"].present?
-  end
-
-  test "create general comment thread" do
-    assert_difference "CommentThread.count", 1 do
-      post api_v1_plan_comments_path(@plan),
-        params: { body_markdown: "General API feedback" },
-        headers: @headers,
-        as: :json
-    end
-    assert_response :created
-  end
-
-  test "reply to thread" do
-    thread = comment_threads(:roadmap_thread)
-    assert_difference "Comment.count", 1 do
-      post reply_api_v1_plan_comment_path(@plan, thread),
-        params: { body_markdown: "API reply" },
-        headers: @headers,
-        as: :json
-    end
-    assert_response :created
-    body = JSON.parse(response.body)
-    assert_equal thread.id, body["thread_id"]
-  end
-
-  test "reply to nonexistent thread" do
-    post reply_api_v1_plan_comment_path(@plan, "nonexistent-id"),
-      params: { body_markdown: "Reply" },
-      headers: @headers,
-      as: :json
-    assert_response :not_found
-  end
-
-  test "create comment requires auth" do
-    post api_v1_plan_comments_path(@plan),
-      params: { body_markdown: "No auth" },
-      as: :json
-    assert_response :unauthorized
-  end
-end
diff --git a/test/controllers/api/v1/leases_controller_test.rb b/test/controllers/api/v1/leases_controller_test.rb
deleted file mode 100644
index e279d960..00000000
--- a/test/controllers/api/v1/leases_controller_test.rb
+++ /dev/null
@@ -1,82 +0,0 @@
-require "test_helper"
-
-class Api::V1::LeasesControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    @headers = { "Authorization" => "Bearer test-token-alice" }
-    @plan = plans(:acme_roadmap)
-  end
-
-  test "acquire lease" do
-    post api_v1_plan_lease_path(@plan),
-      params: { lease_token: "my-token" },
-      headers: @headers,
-      as: :json
-    assert_response :created
-    body = JSON.parse(response.body)
-    assert_equal "my-token", body["lease_token"]
-    assert body["expires_at"].present?
-  end
-
-  test "acquire lease generates token if not provided" do
-    post api_v1_plan_lease_path(@plan), headers: @headers, as: :json
-    assert_response :created
-    body = JSON.parse(response.body)
-    assert body["lease_token"].present?
-  end
-
-  test "acquire lease conflicts when held by another" do
-    post api_v1_plan_lease_path(@plan),
-      params: { lease_token: "first-token" },
-      headers: @headers,
-      as: :json
-    assert_response :created
-
-    bob_headers = { "Authorization" => "Bearer test-token-bob" }
-    post api_v1_plan_lease_path(@plan),
-      params: { lease_token: "second-token" },
-      headers: bob_headers,
-      as: :json
-    assert_response :conflict
-  end
-
-  test "renew lease" do
-    post api_v1_plan_lease_path(@plan),
-      params: { lease_token: "my-token" },
-      headers: @headers,
-      as: :json
-    assert_response :created
-
-    patch api_v1_plan_lease_path(@plan),
-      params: { lease_token: "my-token" },
-      headers: @headers,
-      as: :json
-    assert_response :success
-  end
-
-  test "renew lease with wrong token" do
-    post api_v1_plan_lease_path(@plan),
-      params: { lease_token: "my-token" },
-      headers: @headers,
-      as: :json
-
-    patch api_v1_plan_lease_path(@plan),
-      params: { lease_token: "wrong-token" },
-      headers: @headers,
-      as: :json
-    assert_response :conflict
-  end
-
-  test "release lease" do
-    post api_v1_plan_lease_path(@plan),
-      params: { lease_token: "my-token" },
-      headers: @headers,
-      as: :json
-
-    delete api_v1_plan_lease_path(@plan),
-      params: { lease_token: "my-token" },
-      headers: @headers,
-      as: :json
-    assert_response :no_content
-    assert_nil EditLease.find_by(plan_id: @plan.id)
-  end
-end
diff --git a/test/controllers/api/v1/operations_controller_test.rb b/test/controllers/api/v1/operations_controller_test.rb
deleted file mode 100644
index 5fdb1db2..00000000
--- a/test/controllers/api/v1/operations_controller_test.rb
+++ /dev/null
@@ -1,82 +0,0 @@
-require "test_helper"
-
-class Api::V1::OperationsControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    @headers = { "Authorization" => "Bearer test-token-alice" }
-    @plan = plans(:acme_roadmap)
-    @lease_token = SecureRandom.hex(32)
-    EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-  end
-
-  test "apply operations creates new version" do
-    assert_difference "PlanVersion.count", 1 do
-      post api_v1_plan_operations_path(@plan),
-        params: {
-          lease_token: @lease_token,
-          base_revision: @plan.current_revision,
-          operations: [
-            { op: "replace_exact", old_text: "world domination", new_text: "success", count: 1 }
-          ]
-        },
-        headers: @headers,
-        as: :json
-    end
-    assert_response :created
-    body = JSON.parse(response.body)
-    assert_equal @plan.current_revision + 1, body["revision"]
-  end
-
-  test "apply operations fails without lease" do
-    EditLease.find_by(plan_id: @plan.id)&.destroy
-
-    post api_v1_plan_operations_path(@plan),
-      params: {
-        lease_token: "no-lease",
-        base_revision: @plan.current_revision,
-        operations: [{ op: "replace_exact", old_text: "x", new_text: "y", count: 1 }]
-      },
-      headers: @headers,
-      as: :json
-    assert_response :conflict
-  end
-
-  test "apply operations fails on stale revision" do
-    post api_v1_plan_operations_path(@plan),
-      params: {
-        lease_token: @lease_token,
-        base_revision: 999,
-        operations: [{ op: "replace_exact", old_text: "x", new_text: "y", count: 1 }]
-      },
-      headers: @headers,
-      as: :json
-    assert_response :conflict
-  end
-
-  test "apply operations fails on invalid operation" do
-    post api_v1_plan_operations_path(@plan),
-      params: {
-        lease_token: @lease_token,
-        base_revision: @plan.current_revision,
-        operations: [{ op: "replace_exact", old_text: "nonexistent text", new_text: "y", count: 1 }]
-      },
-      headers: @headers,
-      as: :json
-    assert_response :unprocessable_entity
-  end
-
-  test "apply operations requires lease_token" do
-    post api_v1_plan_operations_path(@plan),
-      params: {
-        base_revision: @plan.current_revision,
-        operations: [{ op: "replace_exact", old_text: "x", new_text: "y", count: 1 }]
-      },
-      headers: @headers,
-      as: :json
-    assert_response :unprocessable_entity
-  end
-end
diff --git a/test/controllers/api/v1/plans_controller_test.rb b/test/controllers/api/v1/plans_controller_test.rb
deleted file mode 100644
index d3c3c561..00000000
--- a/test/controllers/api/v1/plans_controller_test.rb
+++ /dev/null
@@ -1,73 +0,0 @@
-require "test_helper"
-
-class Api::V1::PlansControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    @headers = { "Authorization" => "Bearer test-token-alice" }
-  end
-
-  test "index returns plans" do
-    get api_v1_plans_path, headers: @headers
-    assert_response :success
-    plans = JSON.parse(response.body)
-    assert plans.any? { |p| p["title"] == "Acme Roadmap" }
-  end
-
-  test "index excludes other org plans" do
-    get api_v1_plans_path, headers: { "Authorization" => "Bearer test-token-carol" }
-    assert_response :success
-    plans = JSON.parse(response.body)
-    assert_not plans.any? { |p| p["title"] == "Acme Roadmap" }
-  end
-
-  test "index requires auth" do
-    get api_v1_plans_path
-    assert_response :unauthorized
-  end
-
-  test "index with revoked token" do
-    get api_v1_plans_path, headers: { "Authorization" => "Bearer test-token-revoked" }
-    assert_response :unauthorized
-  end
-
-  test "show returns plan" do
-    get api_v1_plan_path(plans(:acme_roadmap)), headers: @headers
-    assert_response :success
-    plan = JSON.parse(response.body)
-    assert_equal "Acme Roadmap", plan["title"]
-    assert plan["current_content"].present?
-  end
-
-  test "show returns 404 for other org plan" do
-    get api_v1_plan_path(plans(:acme_roadmap)), headers: { "Authorization" => "Bearer test-token-carol" }
-    assert_response :not_found
-  end
-
-  test "create creates new plan" do
-    assert_difference "Plan.count", 1 do
-      post api_v1_plans_path, params: { title: "API Plan", content: "# API Plan\n\nCreated via API." }, headers: @headers, as: :json
-    end
-    assert_response :created
-    plan = JSON.parse(response.body)
-    assert_equal "API Plan", plan["title"]
-    assert_equal 1, plan["current_revision"]
-  end
-
-  test "create without title fails" do
-    post api_v1_plans_path, params: { content: "no title" }, headers: @headers, as: :json
-    assert_response :unprocessable_entity
-  end
-
-  test "versions returns version list" do
-    get versions_api_v1_plan_path(plans(:acme_roadmap)), headers: @headers
-    assert_response :success
-    versions = JSON.parse(response.body)
-    assert versions.any? { |v| v["revision"] == 1 }
-  end
-
-  test "comments returns thread list" do
-    get comments_api_v1_plan_path(plans(:acme_roadmap)), headers: @headers
-    assert_response :success
-    threads = JSON.parse(response.body)
-    assert threads.is_a?(Array)
-  end
-end
diff --git a/test/controllers/api_tokens_controller_test.rb b/test/controllers/api_tokens_controller_test.rb
deleted file mode 100644
index ce7659ad..00000000
--- a/test/controllers/api_tokens_controller_test.rb
+++ /dev/null
@@ -1,37 +0,0 @@
-require "test_helper"
-
-class ApiTokensControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    sign_in_as users(:alice)
-  end
-
-  test "index shows tokens" do
-    get api_tokens_path
-    assert_response :success
-    assert_select "table.data-table"
-  end
-
-  test "create token shows raw token" do
-    assert_difference "ApiToken.count", 1 do
-      post api_tokens_path, params: { api_token: { name: "Test Token" } }
-    end
-    assert_response :success
-    assert_select ".token-reveal"
-    assert_select ".token-reveal__value code"
-  end
-
-  test "revoke token" do
-    token = api_tokens(:alice_token)
-    assert_not token.revoked?
-    patch revoke_api_token_path(token)
-    assert_redirected_to api_tokens_path
-    token.reload
-    assert token.revoked?
-  end
-
-  test "requires authentication" do
-    delete sign_out_path
-    get api_tokens_path
-    assert_redirected_to sign_in_path
-  end
-end
diff --git a/test/controllers/comment_threads_controller_test.rb b/test/controllers/comment_threads_controller_test.rb
deleted file mode 100644
index 564180e8..00000000
--- a/test/controllers/comment_threads_controller_test.rb
+++ /dev/null
@@ -1,76 +0,0 @@
-require "test_helper"
-
-class CommentThreadsControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    sign_in_as users(:alice)
-    @plan = plans(:acme_roadmap)
-  end
-
-  test "create comment thread with anchor text" do
-    assert_difference ["CommentThread.count", "Comment.count"], 1 do
-      post plan_comment_threads_path(@plan), params: {
-        comment_thread: {
-          anchor_text: "world domination",
-          body_markdown: "This needs work."
-        }
-      }
-    end
-    assert_redirected_to plan_path(@plan)
-    thread = CommentThread.last
-    assert_equal "world domination", thread.anchor_text
-    assert_equal "open", thread.status
-    assert_equal @plan.current_plan_version_id, thread.plan_version_id
-  end
-
-  test "create general comment thread" do
-    assert_difference "CommentThread.count", 1 do
-      post plan_comment_threads_path(@plan), params: {
-        comment_thread: {
-          body_markdown: "General feedback."
-        }
-      }
-    end
-    thread = CommentThread.last
-    assert_nil thread.anchor_text
-  end
-
-  test "resolve thread" do
-    thread = comment_threads(:roadmap_thread)
-    patch resolve_plan_comment_thread_path(@plan, thread)
-    assert_redirected_to plan_path(@plan)
-    thread.reload
-    assert_equal "resolved", thread.status
-  end
-
-  test "accept thread as plan author" do
-    thread = comment_threads(:roadmap_thread)
-    patch accept_plan_comment_thread_path(@plan, thread)
-    thread.reload
-    assert_equal "accepted", thread.status
-  end
-
-  test "dismiss thread as plan author" do
-    thread = comment_threads(:roadmap_thread)
-    patch dismiss_plan_comment_thread_path(@plan, thread)
-    thread.reload
-    assert_equal "dismissed", thread.status
-  end
-
-  test "reopen resolved thread" do
-    thread = comment_threads(:roadmap_thread)
-    thread.resolve!(users(:alice))
-    patch reopen_plan_comment_thread_path(@plan, thread)
-    thread.reload
-    assert_equal "open", thread.status
-    assert_nil thread.resolved_by_user_id
-  end
-
-  test "non-author cannot accept thread" do
-    sign_in_as users(:bob)
-    thread = comment_threads(:roadmap_thread)
-    patch accept_plan_comment_thread_path(@plan, thread)
-    assert_response :not_found
-    thread.reload
-    assert_equal "open", thread.status
-  end
-end
diff --git a/test/controllers/comments_controller_test.rb b/test/controllers/comments_controller_test.rb
deleted file mode 100644
index c82e3486..00000000
--- a/test/controllers/comments_controller_test.rb
+++ /dev/null
@@ -1,21 +0,0 @@
-require "test_helper"
-
-class CommentsControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    sign_in_as users(:alice)
-    @plan = plans(:acme_roadmap)
-    @thread = comment_threads(:roadmap_thread)
-  end
-
-  test "create reply" do
-    assert_difference "Comment.count", 1 do
-      post plan_comment_thread_comments_path(@plan, @thread), params: {
-        comment: { body_markdown: "I agree with this." }
-      }
-    end
-    assert_redirected_to plan_path(@plan)
-    comment = Comment.last
-    assert_equal "human", comment.author_type
-    assert_equal users(:alice).id, comment.author_id
-  end
-end
diff --git a/test/controllers/plans_controller_test.rb b/test/controllers/plans_controller_test.rb
deleted file mode 100644
index 6a16aea9..00000000
--- a/test/controllers/plans_controller_test.rb
+++ /dev/null
@@ -1,76 +0,0 @@
-require "test_helper"
-
-class PlansControllerTest < ActionDispatch::IntegrationTest
-  setup do
-    sign_in_as users(:alice)
-  end
-
-  test "index shows plans" do
-    get plans_path
-    assert_response :success
-    assert_select ".plans-list__title", "Acme Roadmap"
-  end
-
-  test "index filters by status" do
-    get plans_path(status: "considering")
-    assert_response :success
-    assert_select ".plans-list__title", "Acme Roadmap"
-  end
-
-  test "show plan renders markdown by default" do
-    get plan_path(plans(:acme_roadmap))
-    assert_response :success
-    assert_select "h1", "Acme Roadmap"
-    assert_select ".markdown-rendered"
-  end
-
-  test "show plan displays comments sidebar" do
-    get plan_path(plans(:acme_roadmap))
-    assert_response :success
-    assert_select ".comment-threads-list"
-  end
-
-  test "show plan includes turbo stream subscription" do
-    get plan_path(plans(:acme_roadmap))
-    assert_response :success
-    assert_select "turbo-cable-stream-source"
-  end
-
-  test "edit plan" do
-    get edit_plan_path(plans(:acme_roadmap))
-    assert_response :success
-  end
-
-  test "update plan creates new version" do
-    plan = plans(:acme_roadmap)
-    assert_difference "PlanVersion.count", 1 do
-      patch plan_path(plan), params: {
-        plan: { title: "Updated Title", content_markdown: "# Updated", change_summary: "Revised" }
-      }
-    end
-    plan.reload
-    assert_equal "Updated Title", plan.title
-    assert_equal 2, plan.current_revision
-    assert_redirected_to plan_path(plan)
-  end
-
-  test "cannot view brainstorm plan as non-author" do
-    sign_in_as users(:bob)
-    get plan_path(plans(:brainstorm_plan))
-    assert_response :not_found
-  end
-
-  test "update marks existing threads as out of date" do
-    plan = plans(:acme_roadmap)
-    thread = comment_threads(:roadmap_thread)
-    assert_not thread.out_of_date?
-
-    patch plan_path(plan), params: {
-      plan: { title: plan.title, content_markdown: "# Updated content", change_summary: "Updated" }
-    }
-
-    thread.reload
-    assert thread.out_of_date?
-    assert_not_nil thread.out_of_date_since_version_id
-  end
-end
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
deleted file mode 100644
index 0d02db44..00000000
--- a/test/controllers/sessions_controller_test.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-require "test_helper"
-
-class SessionsControllerTest < ActionDispatch::IntegrationTest
-  test "sign in page renders" do
-    get sign_in_path
-    assert_response :success
-    assert_select "input[name=email]"
-  end
-
-  test "sign in with valid email creates session" do
-    post sign_in_path, params: { email: "alice@acme.com" }
-    assert_redirected_to root_path
-    follow_redirect!
-    assert_response :success
-    assert_select ".site-nav__user", /Alice/
-  end
-
-  test "sign in with unknown domain shows error" do
-    post sign_in_path, params: { email: "user@unknown.com" }
-    assert_response :unprocessable_entity
-    assert_select ".flash--alert", /No organization found/
-  end
-
-  test "sign in creates new user if not exists" do
-    assert_difference "User.count", 1 do
-      post sign_in_path, params: { email: "newuser@acme.com" }
-    end
-    assert_redirected_to root_path
-  end
-
-  test "sign out clears session" do
-    post sign_in_path, params: { email: "alice@acme.com" }
-    delete sign_out_path
-    assert_redirected_to sign_in_path
-
-    get root_path
-    assert_redirected_to sign_in_path
-  end
-
-  test "unauthenticated access redirects to sign in" do
-    get root_path
-    assert_redirected_to sign_in_path
-  end
-end
diff --git a/test/fixtures/api_tokens.yml b/test/fixtures/api_tokens.yml
deleted file mode 100644
index 31526a09..00000000
--- a/test/fixtures/api_tokens.yml
+++ /dev/null
@@ -1,28 +0,0 @@
-alice_token:
-  id: "00000000-0000-0000-0000-000000000501"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  user_id: "00000000-0000-0000-0000-000000000011"
-  name: "Alice's Agent"
-  token_digest: <%= Digest::SHA256.hexdigest("test-token-alice") %>
-
-bob_token:
-  id: "00000000-0000-0000-0000-000000000502"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  user_id: "00000000-0000-0000-0000-000000000012"
-  name: "Bob's Agent"
-  token_digest: <%= Digest::SHA256.hexdigest("test-token-bob") %>
-
-carol_token:
-  id: "00000000-0000-0000-0000-000000000503"
-  organization_id: "00000000-0000-0000-0000-000000000002"
-  user_id: "00000000-0000-0000-0000-000000000013"
-  name: "Carol's Agent"
-  token_digest: <%= Digest::SHA256.hexdigest("test-token-carol") %>
-
-revoked_token:
-  id: "00000000-0000-0000-0000-000000000504"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  user_id: "00000000-0000-0000-0000-000000000011"
-  name: "Revoked Token"
-  token_digest: <%= Digest::SHA256.hexdigest("test-token-revoked") %>
-  revoked_at: <%= 1.day.ago.to_fs(:db) %>
diff --git a/test/fixtures/automated_plan_reviewers.yml b/test/fixtures/automated_plan_reviewers.yml
deleted file mode 100644
index 24a9eaae..00000000
--- a/test/fixtures/automated_plan_reviewers.yml
+++ /dev/null
@@ -1,61 +0,0 @@
-security_reviewer:
-  id: "00000000-0000-0000-0000-000000000081"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  key: "security-reviewer"
-  name: "Security Reviewer"
-  prompt_text: |
-    You are a security reviewer for technical plans and proposals.
-
-    Review the following plan for security concerns. Focus on:
-
-    - Authentication and authorization gaps
-    - Data exposure or leakage risks
-    - Input validation and injection vulnerabilities
-    - Secrets management issues
-    - Network and infrastructure security
-    - Compliance considerations
-
-    Be specific. Reference the relevant sections of the plan. Suggest concrete improvements where possible.
-
-    Respond in Markdown. Keep your review concise and actionable.
-  enabled: true
-  trigger_statuses: <%= ["considering"].to_json %>
-  ai_provider: "openai"
-  ai_model: "gpt-4o"
-
-scalability_reviewer:
-  id: "00000000-0000-0000-0000-000000000082"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  key: "scalability-reviewer"
-  name: "Scalability Reviewer"
-  prompt_text: |
-    You are a scalability reviewer for technical plans and proposals.
-
-    Review the following plan for scalability concerns. Focus on:
-
-    - Database query patterns and indexing strategies
-    - Caching opportunities and cache invalidation
-    - Background job volume and queue contention
-    - API rate limits and throughput bottlenecks
-    - Data growth projections and storage implications
-    - Horizontal vs vertical scaling considerations
-
-    Be specific. Reference the relevant sections of the plan. Suggest concrete improvements where possible.
-
-    Respond in Markdown. Keep your review concise and actionable.
-  enabled: true
-  trigger_statuses: <%= ["considering", "developing"].to_json %>
-  ai_provider: "openai"
-  ai_model: "gpt-4o"
-
-disabled_reviewer:
-  id: "00000000-0000-0000-0000-000000000083"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  key: "disabled-reviewer"
-  name: "Disabled Reviewer"
-  prompt_text: |
-    You are a test reviewer. This reviewer is disabled.
-  enabled: false
-  trigger_statuses: <%= [].to_json %>
-  ai_provider: "openai"
-  ai_model: "gpt-4o"
diff --git a/test/fixtures/comment_threads.yml b/test/fixtures/comment_threads.yml
deleted file mode 100644
index d43625b9..00000000
--- a/test/fixtures/comment_threads.yml
+++ /dev/null
@@ -1,18 +0,0 @@
-roadmap_thread:
-  id: "00000000-0000-0000-0000-000000000301"
-  plan_id: "00000000-0000-0000-0000-000000000101"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  plan_version_id: "00000000-0000-0000-0000-000000000201"
-  anchor_text: "original roadmap text"
-  status: "open"
-  out_of_date: false
-  created_by_user_id: "00000000-0000-0000-0000-000000000012"
-
-general_thread:
-  id: "00000000-0000-0000-0000-000000000302"
-  plan_id: "00000000-0000-0000-0000-000000000101"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  plan_version_id: "00000000-0000-0000-0000-000000000201"
-  status: "open"
-  out_of_date: false
-  created_by_user_id: "00000000-0000-0000-0000-000000000011"
diff --git a/test/fixtures/comments.yml b/test/fixtures/comments.yml
deleted file mode 100644
index 800f7b33..00000000
--- a/test/fixtures/comments.yml
+++ /dev/null
@@ -1,15 +0,0 @@
-thread_comment:
-  id: "00000000-0000-0000-0000-000000000401"
-  comment_thread_id: "00000000-0000-0000-0000-000000000301"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  author_type: "human"
-  author_id: "00000000-0000-0000-0000-000000000012"
-  body_markdown: "I think we should reconsider this section."
-
-general_comment:
-  id: "00000000-0000-0000-0000-000000000402"
-  comment_thread_id: "00000000-0000-0000-0000-000000000302"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  author_type: "human"
-  author_id: "00000000-0000-0000-0000-000000000011"
-  body_markdown: "Overall this looks good. Ship it!"
diff --git a/test/fixtures/edit_leases.yml b/test/fixtures/edit_leases.yml
deleted file mode 100644
index 9517f91c..00000000
--- a/test/fixtures/edit_leases.yml
+++ /dev/null
@@ -1 +0,0 @@
-# Edit leases are created/destroyed dynamically in tests
diff --git a/test/fixtures/files/.keep b/test/fixtures/files/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/fixtures/organizations.yml b/test/fixtures/organizations.yml
deleted file mode 100644
index a5cbdcf1..00000000
--- a/test/fixtures/organizations.yml
+++ /dev/null
@@ -1,11 +0,0 @@
-acme:
-  id: "00000000-0000-0000-0000-000000000001"
-  name: "Acme Corp"
-  slug: "acme"
-  allowed_email_domains: <%= ["acme.com"].to_json %>
-
-widgets:
-  id: "00000000-0000-0000-0000-000000000002"
-  name: "Widgets Inc"
-  slug: "widgets"
-  allowed_email_domains: <%= ["widgets.io", "widgets.com"].to_json %>
diff --git a/test/fixtures/plan_versions.yml b/test/fixtures/plan_versions.yml
deleted file mode 100644
index a7dea437..00000000
--- a/test/fixtures/plan_versions.yml
+++ /dev/null
@@ -1,21 +0,0 @@
-roadmap_v1:
-  id: "00000000-0000-0000-0000-000000000201"
-  plan_id: "00000000-0000-0000-0000-000000000101"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  revision: 1
-  content_markdown: "# Acme Roadmap\n\nOur plan for world domination."
-  content_sha256: <%= Digest::SHA256.hexdigest("# Acme Roadmap\n\nOur plan for world domination.") %>
-  actor_type: "human"
-  actor_id: "00000000-0000-0000-0000-000000000011"
-  created_at: <%= 1.day.ago.to_fs(:db) %>
-
-brainstorm_v1:
-  id: "00000000-0000-0000-0000-000000000202"
-  plan_id: "00000000-0000-0000-0000-000000000102"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  revision: 1
-  content_markdown: "# Secret Plan\n\nThis is private."
-  content_sha256: <%= Digest::SHA256.hexdigest("# Secret Plan\n\nThis is private.") %>
-  actor_type: "human"
-  actor_id: "00000000-0000-0000-0000-000000000011"
-  created_at: <%= 1.day.ago.to_fs(:db) %>
diff --git a/test/fixtures/plans.yml b/test/fixtures/plans.yml
deleted file mode 100644
index 93c4041c..00000000
--- a/test/fixtures/plans.yml
+++ /dev/null
@@ -1,21 +0,0 @@
-acme_roadmap:
-  id: "00000000-0000-0000-0000-000000000101"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  title: "Acme Roadmap"
-  status: "considering"
-  current_plan_version_id: "00000000-0000-0000-0000-000000000201"
-  current_revision: 1
-  tags: <%= [].to_json %>
-  metadata: <%= {}.to_json %>
-  created_by_user_id: "00000000-0000-0000-0000-000000000011"
-
-brainstorm_plan:
-  id: "00000000-0000-0000-0000-000000000102"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  title: "Secret Brainstorm"
-  status: "brainstorm"
-  current_plan_version_id: "00000000-0000-0000-0000-000000000202"
-  current_revision: 1
-  tags: <%= [].to_json %>
-  metadata: <%= {}.to_json %>
-  created_by_user_id: "00000000-0000-0000-0000-000000000011"
diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml
deleted file mode 100644
index cb57b3bb..00000000
--- a/test/fixtures/users.yml
+++ /dev/null
@@ -1,20 +0,0 @@
-alice:
-  id: "00000000-0000-0000-0000-000000000011"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  email: "alice@acme.com"
-  name: "Alice"
-  org_role: "admin"
-
-bob:
-  id: "00000000-0000-0000-0000-000000000012"
-  organization_id: "00000000-0000-0000-0000-000000000001"
-  email: "bob@acme.com"
-  name: "Bob"
-  org_role: "member"
-
-carol:
-  id: "00000000-0000-0000-0000-000000000013"
-  organization_id: "00000000-0000-0000-0000-000000000002"
-  email: "carol@widgets.io"
-  name: "Carol"
-  org_role: "admin"
diff --git a/test/helpers/.keep b/test/helpers/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/helpers/markdown_helper_test.rb b/test/helpers/markdown_helper_test.rb
deleted file mode 100644
index fe8907ec..00000000
--- a/test/helpers/markdown_helper_test.rb
+++ /dev/null
@@ -1,42 +0,0 @@
-require "test_helper"
-
-class MarkdownHelperTest < ActionView::TestCase
-  test "render_markdown converts markdown to HTML" do
-    html = render_markdown("# Hello\n\n**bold** text")
-    assert_includes html, "<h1>"
-    assert_includes html, "<strong>bold</strong>"
-    assert_includes html, "markdown-rendered"
-  end
-
-  test "render_markdown sanitizes dangerous HTML" do
-    html = render_markdown('<script>alert("xss")</script>')
-    assert_no_match(/<script>/, html)
-  end
-
-  test "render_markdown handles nil gracefully" do
-    html = render_markdown(nil)
-    assert_includes html, "markdown-rendered"
-  end
-
-  test "render_line_view creates numbered divs" do
-    html = render_line_view("line one\nline two\nline three")
-    assert_includes html, 'id="L1"'
-    assert_includes html, 'id="L2"'
-    assert_includes html, 'id="L3"'
-    assert_includes html, 'data-line="1"'
-    assert_includes html, 'data-line="3"'
-    assert_includes html, "line-view"
-  end
-
-  test "render_line_view handles empty lines" do
-    html = render_line_view("line one\n\nline three")
-    assert_includes html, 'id="L2"'
-    assert_includes html, "&nbsp;"
-  end
-
-  test "render_line_view escapes HTML in content" do
-    html = render_line_view('<script>alert("xss")</script>')
-    assert_no_match(/<script>/, html)
-    assert_includes html, "&lt;script&gt;"
-  end
-end
diff --git a/test/integration/.keep b/test/integration/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/mailers/.keep b/test/mailers/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/models/.keep b/test/models/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/models/api_token_test.rb b/test/models/api_token_test.rb
deleted file mode 100644
index dd0aa182..00000000
--- a/test/models/api_token_test.rb
+++ /dev/null
@@ -1,66 +0,0 @@
-require "test_helper"
-
-class ApiTokenTest < ActiveSupport::TestCase
-  test "valid token" do
-    token = api_tokens(:alice_token)
-    assert token.valid?
-  end
-
-  test "requires name" do
-    token = ApiToken.new(organization: organizations(:acme), user: users(:alice), token_digest: "abc123")
-    token.name = nil
-    assert_not token.valid?
-    assert_includes token.errors[:name], "can't be blank"
-  end
-
-  test "requires token_digest" do
-    token = ApiToken.new(organization: organizations(:acme), user: users(:alice), name: "Test")
-    token.token_digest = nil
-    assert_not token.valid?
-    assert_includes token.errors[:token_digest], "can't be blank"
-  end
-
-  test "authenticate with valid token" do
-    result = ApiToken.authenticate("test-token-alice")
-    assert_not_nil result
-    assert_equal api_tokens(:alice_token).id, result.id
-  end
-
-  test "authenticate with invalid token" do
-    result = ApiToken.authenticate("invalid-token")
-    assert_nil result
-  end
-
-  test "authenticate with revoked token returns nil" do
-    result = ApiToken.authenticate("test-token-revoked")
-    assert_nil result
-  end
-
-  test "authenticate with blank token returns nil" do
-    assert_nil ApiToken.authenticate("")
-    assert_nil ApiToken.authenticate(nil)
-  end
-
-  test "revoke sets revoked_at" do
-    token = api_tokens(:alice_token)
-    assert_not token.revoked?
-    token.revoke!
-    assert token.revoked?
-    assert_not_nil token.revoked_at
-  end
-
-  test "active? returns false when revoked" do
-    token = api_tokens(:revoked_token)
-    assert_not token.active?
-  end
-
-  test "active? returns true for valid token" do
-    token = api_tokens(:alice_token)
-    assert token.active?
-  end
-
-  test "generate_token returns hex string" do
-    raw = ApiToken.generate_token
-    assert_match /\A[0-9a-f]{64}\z/, raw
-  end
-end
diff --git a/test/models/automated_plan_reviewer_test.rb b/test/models/automated_plan_reviewer_test.rb
deleted file mode 100644
index 489e928c..00000000
--- a/test/models/automated_plan_reviewer_test.rb
+++ /dev/null
@@ -1,164 +0,0 @@
-require "test_helper"
-
-class AutomatedPlanReviewerTest < ActiveSupport::TestCase
-  test "valid reviewer" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    assert reviewer.valid?
-  end
-
-  test "requires key" do
-    reviewer = AutomatedPlanReviewer.new(name: "Test", prompt_text: "Review this plan.", ai_model: "gpt-4o")
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:key], "can't be blank"
-  end
-
-  test "requires name" do
-    reviewer = AutomatedPlanReviewer.new(key: "test", prompt_text: "Review this plan.", ai_model: "gpt-4o")
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:name], "can't be blank"
-  end
-
-  test "requires prompt_text" do
-    reviewer = AutomatedPlanReviewer.new(key: "test", name: "Test", ai_model: "gpt-4o")
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:prompt_text], "can't be blank"
-  end
-
-  test "requires ai_model" do
-    reviewer = AutomatedPlanReviewer.new(key: "test", name: "Test", prompt_text: "Review this plan.")
-    reviewer.ai_model = nil
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:ai_model], "can't be blank"
-  end
-
-  test "requires organization" do
-    reviewer = AutomatedPlanReviewer.new(
-      key: "test",
-      name: "Test",
-      prompt_text: "Review this plan.",
-      ai_model: "gpt-4o"
-    )
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:organization], "must exist"
-  end
-
-  test "key format only allows lowercase letters, numbers, and hyphens" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    reviewer.key = "Invalid Key!"
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:key], "only allows lowercase letters, numbers, and hyphens"
-  end
-
-  test "key uniqueness scoped to organization" do
-    existing = automated_plan_reviewers(:security_reviewer)
-    duplicate = AutomatedPlanReviewer.new(
-      organization: existing.organization,
-      key: existing.key,
-      name: "Duplicate",
-      prompt_text: "Review this plan.",
-      ai_model: "gpt-4o"
-    )
-    assert_not duplicate.valid?
-    assert_includes duplicate.errors[:key], "has already been taken"
-  end
-
-  test "same key allowed in different organizations" do
-    reviewer = AutomatedPlanReviewer.new(
-      organization: organizations(:widgets),
-      key: "security-reviewer",
-      name: "Security Reviewer",
-      prompt_text: "Review this plan.",
-      ai_model: "gpt-4o"
-    )
-    assert reviewer.valid?
-  end
-
-  test "prompt_text stores the prompt content" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    assert_includes reviewer.prompt_text, "security"
-  end
-
-  test "triggers_on_status? returns true for matching status" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    assert reviewer.triggers_on_status?("considering")
-  end
-
-  test "triggers_on_status? returns false for non-matching status" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    assert_not reviewer.triggers_on_status?("brainstorm")
-  end
-
-  test "enabled scope returns only enabled reviewers" do
-    enabled = AutomatedPlanReviewer.enabled
-    assert enabled.include?(automated_plan_reviewers(:security_reviewer))
-    assert_not enabled.include?(automated_plan_reviewers(:disabled_reviewer))
-  end
-
-  test "validates ai_provider inclusion" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    reviewer.ai_provider = "unknown-provider"
-    assert_not reviewer.valid?
-    assert_includes reviewer.errors[:ai_provider], "is not included in the list"
-  end
-
-  test "accepts valid ai_providers" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    AutomatedPlanReviewer::AI_PROVIDERS.each do |provider|
-      reviewer.ai_provider = provider
-      assert reviewer.valid?, "Expected #{provider} to be valid"
-    end
-  end
-
-  test "defaults ai_provider to openai" do
-    reviewer = AutomatedPlanReviewer.new
-    assert_equal "openai", reviewer.ai_provider
-  end
-
-  test "validates trigger_statuses against Plan::STATUSES" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    reviewer.trigger_statuses = [ "considering", "invalid-status" ]
-    assert_not reviewer.valid?
-    assert reviewer.errors[:trigger_statuses].any? { |e| e.include?("invalid-status") }
-  end
-
-  test "accepts valid trigger_statuses" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    reviewer.trigger_statuses = Plan::STATUSES.dup
-    assert reviewer.valid?
-  end
-
-  test "accepts empty trigger_statuses" do
-    reviewer = automated_plan_reviewers(:security_reviewer)
-    reviewer.trigger_statuses = []
-    assert reviewer.valid?
-  end
-
-  test "defaults trigger_statuses to empty array" do
-    reviewer = AutomatedPlanReviewer.new
-    assert_equal [], reviewer.trigger_statuses
-  end
-
-  test "defaults enabled to true" do
-    reviewer = AutomatedPlanReviewer.new
-    assert_equal true, reviewer.enabled
-  end
-
-  test "create_defaults_for creates default reviewers for an organization" do
-    org = Organization.create!(name: "Test Org", slug: "test-defaults-org")
-    reviewers = org.automated_plan_reviewers
-    assert_equal 3, reviewers.count
-    assert_equal %w[routing-reviewer scalability-reviewer security-reviewer], reviewers.pluck(:key).sort
-    reviewers.each do |r|
-      assert r.prompt_text.present?
-      assert r.ai_model.present?
-    end
-  end
-
-  test "create_defaults_for is idempotent" do
-    org = organizations(:acme)
-    AutomatedPlanReviewer.create_defaults_for(org)
-    initial_count = org.automated_plan_reviewers.count
-    AutomatedPlanReviewer.create_defaults_for(org)
-    assert_equal initial_count, org.automated_plan_reviewers.count
-  end
-end
diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb
deleted file mode 100644
index ec1b6328..00000000
--- a/test/models/comment_test.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-require "test_helper"
-
-class CommentTest < ActiveSupport::TestCase
-  test "valid comment" do
-    comment = comments(:thread_comment)
-    assert comment.valid?
-  end
-
-  test "validates body_markdown presence" do
-    comment = comments(:thread_comment)
-    comment.body_markdown = ""
-    assert_not comment.valid?
-    assert_includes comment.errors[:body_markdown], "can't be blank"
-  end
-
-  test "validates author_type inclusion" do
-    comment = comments(:thread_comment)
-    comment.author_type = "unknown"
-    assert_not comment.valid?
-    assert_includes comment.errors[:author_type], "is not included in the list"
-  end
-
-  test "belongs to comment_thread" do
-    comment = comments(:thread_comment)
-    assert_equal comment_threads(:roadmap_thread), comment.comment_thread
-  end
-end
diff --git a/test/models/comment_thread_test.rb b/test/models/comment_thread_test.rb
deleted file mode 100644
index 9871532e..00000000
--- a/test/models/comment_thread_test.rb
+++ /dev/null
@@ -1,207 +0,0 @@
-require "test_helper"
-
-class CommentThreadTest < ActiveSupport::TestCase
-  test "valid comment thread" do
-    thread = comment_threads(:roadmap_thread)
-    assert thread.valid?
-  end
-
-  test "validates status inclusion" do
-    thread = comment_threads(:roadmap_thread)
-    thread.status = "invalid"
-    assert_not thread.valid?
-    assert_includes thread.errors[:status], "is not included in the list"
-  end
-
-  test "line_specific? returns true when lines set" do
-    thread = comment_threads(:general_thread)
-    thread.start_line = 5
-    thread.end_line = 8
-    assert thread.line_specific?
-  end
-
-  test "line_specific? returns false for general thread" do
-    thread = comment_threads(:general_thread)
-    assert_not thread.line_specific?
-  end
-
-  test "line_range_text for range" do
-    thread = comment_threads(:general_thread)
-    thread.start_line = 5
-    thread.end_line = 8
-    assert_equal "Lines 5–8", thread.line_range_text
-  end
-
-  test "line_range_text for single line" do
-    thread = comment_threads(:general_thread)
-    thread.start_line = 5
-    thread.end_line = 5
-    assert_equal "Line 5", thread.line_range_text
-  end
-
-  test "line_range_text for general thread" do
-    thread = comment_threads(:general_thread)
-    assert_nil thread.line_range_text
-  end
-
-  test "resolve! sets status and user" do
-    thread = comment_threads(:roadmap_thread)
-    user = users(:alice)
-    thread.resolve!(user)
-    assert_equal "resolved", thread.status
-    assert_equal user, thread.resolved_by_user
-  end
-
-  test "accept! sets status and user" do
-    thread = comment_threads(:roadmap_thread)
-    user = users(:alice)
-    thread.accept!(user)
-    assert_equal "accepted", thread.status
-    assert_equal user, thread.resolved_by_user
-  end
-
-  test "dismiss! sets status and user" do
-    thread = comment_threads(:roadmap_thread)
-    user = users(:alice)
-    thread.dismiss!(user)
-    assert_equal "dismissed", thread.status
-    assert_equal user, thread.resolved_by_user
-  end
-
-  test "open_threads scope" do
-    plan = plans(:acme_roadmap)
-    open = plan.comment_threads.open_threads
-    assert open.all? { |t| t.status == "open" }
-  end
-
-  test "anchored? returns true when anchor_text set" do
-    thread = comment_threads(:roadmap_thread)
-    assert thread.anchored?
-  end
-
-  test "anchored? returns false when anchor_text blank" do
-    thread = comment_threads(:general_thread)
-    assert_not thread.anchored?
-  end
-
-  test "anchor_preview truncates long text" do
-    thread = comment_threads(:roadmap_thread)
-    thread.anchor_text = "a" * 100
-    assert_equal "a" * 80 + "…", thread.anchor_preview
-  end
-
-  test "anchor_preview returns short text as-is" do
-    thread = comment_threads(:roadmap_thread)
-    thread.anchor_text = "short text"
-    assert_equal "short text", thread.anchor_preview
-  end
-
-  test "active scope returns open non-out-of-date threads" do
-    plan = plans(:acme_roadmap)
-    active = plan.comment_threads.active
-    assert active.all? { |t| t.status == "open" && !t.out_of_date? }
-  end
-
-  test "active scope excludes resolved threads" do
-    thread = comment_threads(:roadmap_thread)
-    thread.resolve!(users(:alice))
-    active = plans(:acme_roadmap).comment_threads.active
-    assert_not active.include?(thread)
-  end
-
-  test "active scope excludes out-of-date threads" do
-    thread = comment_threads(:roadmap_thread)
-    thread.update_columns(out_of_date: true)
-    active = plans(:acme_roadmap).comment_threads.active
-    assert_not active.include?(thread)
-  end
-
-  test "archived scope returns non-open or out-of-date threads" do
-    plan = plans(:acme_roadmap)
-    archived = plan.comment_threads.archived
-    assert archived.all? { |t| t.status != "open" || t.out_of_date? }
-  end
-
-  test "archived scope includes resolved threads" do
-    thread = comment_threads(:roadmap_thread)
-    thread.resolve!(users(:alice))
-    archived = plans(:acme_roadmap).comment_threads.archived
-    assert archived.include?(thread)
-  end
-
-  test "mark_out_of_date_for_new_version! keeps thread when anchor text still present" do
-    plan = plans(:acme_roadmap)
-    thread = comment_threads(:roadmap_thread)
-    thread.update_columns(anchor_text: "world domination")
-
-    new_version = PlanVersion.create!(
-      plan: plan,
-      organization: plan.organization,
-      revision: 2,
-      content_markdown: "# Acme Roadmap\n\nOur plan for world domination continues.",
-      actor_type: "human",
-      actor_id: users(:alice).id
-    )
-
-    plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
-    thread.reload
-    assert_not thread.out_of_date?
-  end
-
-  test "mark_out_of_date_for_new_version! marks thread when anchor text removed" do
-    plan = plans(:acme_roadmap)
-    thread = comment_threads(:roadmap_thread)
-    thread.update_columns(anchor_text: "world domination")
-
-    new_version = PlanVersion.create!(
-      plan: plan,
-      organization: plan.organization,
-      revision: 2,
-      content_markdown: "# Acme Roadmap\n\nCompletely new content here.",
-      actor_type: "human",
-      actor_id: users(:alice).id
-    )
-
-    plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
-    thread.reload
-    assert thread.out_of_date?
-    assert_equal new_version.id, thread.out_of_date_since_version_id
-  end
-
-  test "mark_out_of_date_for_new_version! skips non-anchored threads" do
-    plan = plans(:acme_roadmap)
-    thread = comment_threads(:general_thread)
-
-    new_version = PlanVersion.create!(
-      plan: plan,
-      organization: plan.organization,
-      revision: 2,
-      content_markdown: "Completely different content.",
-      actor_type: "human",
-      actor_id: users(:alice).id
-    )
-
-    plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
-    thread.reload
-    assert_not thread.out_of_date?
-  end
-
-  test "mark_out_of_date_for_new_version! uses anchor_context when present" do
-    plan = plans(:acme_roadmap)
-    thread = comment_threads(:roadmap_thread)
-    thread.update_columns(anchor_text: "plan", anchor_context: "Our plan for world domination")
-
-    new_version = PlanVersion.create!(
-      plan: plan,
-      organization: plan.organization,
-      revision: 2,
-      content_markdown: "# Acme Roadmap\n\nWe have a plan but the context changed.",
-      actor_type: "human",
-      actor_id: users(:alice).id
-    )
-
-    plan.comment_threads.mark_out_of_date_for_new_version!(new_version)
-    thread.reload
-    assert thread.out_of_date?, "Thread should be out of date when anchor_context is no longer in content"
-  end
-end
diff --git a/test/models/edit_lease_test.rb b/test/models/edit_lease_test.rb
deleted file mode 100644
index b992cdda..00000000
--- a/test/models/edit_lease_test.rb
+++ /dev/null
@@ -1,145 +0,0 @@
-require "test_helper"
-
-class EditLeaseTest < ActiveSupport::TestCase
-  setup do
-    @plan = plans(:acme_roadmap)
-    @lease_token = SecureRandom.hex(32)
-  end
-
-  test "acquire creates new lease" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-    assert lease.persisted?
-    assert lease.held?
-    assert_equal @plan.id, lease.plan_id
-  end
-
-  test "acquire replaces expired lease" do
-    EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    lease = EditLease.find_by(plan_id: @plan.id)
-    lease.update!(expires_at: 1.minute.ago)
-
-    new_token = SecureRandom.hex(32)
-    new_lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:bob_token).id,
-      lease_token: new_token
-    )
-    assert new_lease.persisted?
-    assert new_lease.held?
-  end
-
-  test "acquire raises conflict when held by another" do
-    EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    other_token = SecureRandom.hex(32)
-    assert_raises(EditLease::Conflict) do
-      EditLease.acquire!(
-        plan: @plan,
-        holder_type: "local_agent",
-        holder_id: api_tokens(:bob_token).id,
-        lease_token: other_token
-      )
-    end
-  end
-
-  test "acquire with same token renews lease" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-    original_expires = lease.expires_at
-
-    travel 1.minute do
-      renewed = EditLease.acquire!(
-        plan: @plan,
-        holder_type: "local_agent",
-        holder_id: api_tokens(:alice_token).id,
-        lease_token: @lease_token
-      )
-      assert renewed.expires_at > original_expires
-    end
-  end
-
-  test "renew updates expiry" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    travel 1.minute do
-      lease.renew!(lease_token: @lease_token)
-      assert lease.expires_at > Time.current
-    end
-  end
-
-  test "renew raises conflict with wrong token" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    assert_raises(EditLease::Conflict) do
-      lease.renew!(lease_token: "wrong-token")
-    end
-  end
-
-  test "release destroys lease" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    lease.release!(lease_token: @lease_token)
-    assert_nil EditLease.find_by(plan_id: @plan.id)
-  end
-
-  test "release raises conflict with wrong token" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    assert_raises(EditLease::Conflict) do
-      lease.release!(lease_token: "wrong-token")
-    end
-  end
-
-  test "held_by? checks token and expiry" do
-    lease = EditLease.acquire!(
-      plan: @plan,
-      holder_type: "local_agent",
-      holder_id: api_tokens(:alice_token).id,
-      lease_token: @lease_token
-    )
-
-    assert lease.held_by?(lease_token: @lease_token)
-    assert_not lease.held_by?(lease_token: "wrong-token")
-  end
-end
diff --git a/test/models/organization_test.rb b/test/models/organization_test.rb
deleted file mode 100644
index 1734d266..00000000
--- a/test/models/organization_test.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require "test_helper"
-
-class OrganizationTest < ActiveSupport::TestCase
-  test "valid organization" do
-    org = organizations(:acme)
-    assert org.valid?
-  end
-
-  test "requires name" do
-    org = Organization.new(slug: "test", allowed_email_domains: ["test.com"])
-    assert_not org.valid?
-    assert_includes org.errors[:name], "can't be blank"
-  end
-
-  test "requires slug" do
-    org = Organization.new(name: "Test", allowed_email_domains: ["test.com"])
-    assert_not org.valid?
-    assert_includes org.errors[:slug], "can't be blank"
-  end
-
-  test "slug must be unique" do
-    org = Organization.new(name: "Duplicate", slug: "acme", allowed_email_domains: ["dup.com"])
-    assert_not org.valid?
-    assert_includes org.errors[:slug], "has already been taken"
-  end
-
-  test "slug format rejects uppercase" do
-    org = Organization.new(name: "Test", slug: "BadSlug", allowed_email_domains: ["test.com"])
-    assert_not org.valid?
-    assert_includes org.errors[:slug], "only allows lowercase letters, numbers, and hyphens"
-  end
-
-  test "email_domain_allowed? returns true for allowed domain" do
-    org = organizations(:acme)
-    assert org.email_domain_allowed?("user@acme.com")
-  end
-
-  test "email_domain_allowed? returns false for disallowed domain" do
-    org = organizations(:acme)
-    assert_not org.email_domain_allowed?("user@other.com")
-  end
-
-  test "email_domain_allowed? is case-insensitive" do
-    org = organizations(:acme)
-    assert org.email_domain_allowed?("user@ACME.COM")
-  end
-
-  test "defaults allowed_email_domains to empty array" do
-    org = Organization.new
-    assert_equal [], org.allowed_email_domains
-  end
-end
diff --git a/test/models/plan_test.rb b/test/models/plan_test.rb
deleted file mode 100644
index b1032e44..00000000
--- a/test/models/plan_test.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-require "test_helper"
-
-class PlanTest < ActiveSupport::TestCase
-  test "valid plan" do
-    plan = plans(:acme_roadmap)
-    assert plan.valid?
-  end
-
-  test "requires title" do
-    plan = Plan.new(organization: organizations(:acme), created_by_user: users(:alice))
-    assert_not plan.valid?
-    assert_includes plan.errors[:title], "can't be blank"
-  end
-
-  test "status must be valid" do
-    plan = plans(:acme_roadmap)
-    plan.status = "invalid"
-    assert_not plan.valid?
-  end
-
-  test "defaults status to brainstorm" do
-    plan = Plan.new
-    assert_equal "brainstorm", plan.status
-  end
-
-  test "current_content returns version content" do
-    plan = plans(:acme_roadmap)
-    assert_includes plan.current_content, "Acme Roadmap"
-  end
-
-  test "to_param returns id" do
-    plan = plans(:acme_roadmap)
-    assert_equal plan.id, plan.to_param
-  end
-end
diff --git a/test/models/plan_version_test.rb b/test/models/plan_version_test.rb
deleted file mode 100644
index 68751bf8..00000000
--- a/test/models/plan_version_test.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-require "test_helper"
-
-class PlanVersionTest < ActiveSupport::TestCase
-  test "valid version" do
-    version = plan_versions(:roadmap_v1)
-    assert version.valid?
-  end
-
-  test "computes sha256 automatically" do
-    version = PlanVersion.new(
-      plan: plans(:acme_roadmap),
-      organization: organizations(:acme),
-      revision: 99,
-      content_markdown: "test content",
-      actor_type: "human"
-    )
-    assert version.valid?
-    assert_equal Digest::SHA256.hexdigest("test content"), version.content_sha256
-  end
-
-  test "revision must be unique per plan" do
-    version = PlanVersion.new(
-      plan: plans(:acme_roadmap),
-      organization: organizations(:acme),
-      revision: 1,
-      content_markdown: "dupe",
-      actor_type: "human"
-    )
-    assert_not version.valid?
-    assert_includes version.errors[:revision], "has already been taken"
-  end
-
-  test "actor_type must be valid" do
-    version = plan_versions(:roadmap_v1)
-    version.actor_type = "robot"
-    assert_not version.valid?
-  end
-end
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
deleted file mode 100644
index 276de0d0..00000000
--- a/test/models/user_test.rb
+++ /dev/null
@@ -1,56 +0,0 @@
-require "test_helper"
-
-class UserTest < ActiveSupport::TestCase
-  test "valid user" do
-    user = users(:alice)
-    assert user.valid?
-  end
-
-  test "requires email" do
-    user = User.new(organization: organizations(:acme), name: "Test", org_role: "member")
-    assert_not user.valid?
-    assert_includes user.errors[:email], "can't be blank"
-  end
-
-  test "requires name" do
-    user = User.new(organization: organizations(:acme), email: "test@acme.com", org_role: "member")
-    assert_not user.valid?
-    assert_includes user.errors[:name], "can't be blank"
-  end
-
-  test "email must be unique within organization" do
-    user = User.new(organization: organizations(:acme), email: "alice@acme.com", name: "Dup", org_role: "member")
-    assert_not user.valid?
-    assert_includes user.errors[:email], "has already been taken"
-  end
-
-  test "same email allowed in different orgs" do
-    user = User.new(organization: organizations(:widgets), email: "alice@widgets.io", name: "Alice W", org_role: "member")
-    assert user.valid?
-  end
-
-  test "org_role must be member or admin" do
-    user = users(:alice)
-    user.org_role = "superadmin"
-    assert_not user.valid?
-    assert_includes user.errors[:org_role], "is not included in the list"
-  end
-
-  test "admin? returns true for admin role" do
-    assert users(:alice).admin?
-  end
-
-  test "admin? returns false for member role" do
-    assert_not users(:bob).admin?
-  end
-
-  test "email domain must be allowed" do
-    user = User.new(organization: organizations(:acme), email: "user@other.com", name: "Test", org_role: "member")
-    assert_not user.valid?
-    assert_includes user.errors[:email], "domain is not allowed for this organization"
-  end
-
-  test "email_domain extracts domain" do
-    assert_equal "acme.com", users(:alice).email_domain
-  end
-end
diff --git a/test/services/plans/apply_operations_test.rb b/test/services/plans/apply_operations_test.rb
deleted file mode 100644
index 019234a3..00000000
--- a/test/services/plans/apply_operations_test.rb
+++ /dev/null
@@ -1,152 +0,0 @@
-require "test_helper"
-
-class Plans::ApplyOperationsTest < ActiveSupport::TestCase
-  test "replace_exact replaces text" do
-    result = Plans::ApplyOperations.call(
-      content: "Hello world, hello universe.",
-      operations: [{ "op" => "replace_exact", "old_text" => "world", "new_text" => "planet", "count" => 1 }]
-    )
-    assert_equal "Hello planet, hello universe.", result[:content]
-    assert_equal 1, result[:applied].length
-  end
-
-  test "replace_exact with count 2 replaces all occurrences" do
-    result = Plans::ApplyOperations.call(
-      content: "foo bar foo baz",
-      operations: [{ "op" => "replace_exact", "old_text" => "foo", "new_text" => "qux", "count" => 2 }]
-    )
-    assert_equal "qux bar qux baz", result[:content]
-  end
-
-  test "replace_exact fails when text not found" do
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: "Hello world",
-        operations: [{ "op" => "replace_exact", "old_text" => "missing", "new_text" => "found", "count" => 1 }]
-      )
-    end
-    assert_match /found 0 occurrences/, error.message
-  end
-
-  test "replace_exact fails when too many occurrences" do
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: "foo foo foo",
-        operations: [{ "op" => "replace_exact", "old_text" => "foo", "new_text" => "bar", "count" => 1 }]
-      )
-    end
-    assert_match /found 3 occurrences/, error.message
-  end
-
-  test "replace_exact requires old_text" do
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: "Hello",
-        operations: [{ "op" => "replace_exact", "new_text" => "Bye" }]
-      )
-    end
-    assert_match /requires 'old_text'/, error.message
-  end
-
-  test "insert_under_heading inserts content" do
-    content = "# Title\n\nIntro\n\n## Goals\n\nExisting goals."
-    result = Plans::ApplyOperations.call(
-      content: content,
-      operations: [{ "op" => "insert_under_heading", "heading" => "## Goals", "content" => "- New goal" }]
-    )
-    assert_includes result[:content], "## Goals\n\n- New goal"
-    assert_includes result[:content], "Existing goals."
-  end
-
-  test "insert_under_heading fails when heading not found" do
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: "# Title\n\nContent",
-        operations: [{ "op" => "insert_under_heading", "heading" => "## Missing", "content" => "stuff" }]
-      )
-    end
-    assert_match /no heading matching/, error.message
-  end
-
-  test "insert_under_heading fails when heading is ambiguous" do
-    content = "## Goals\n\nFirst\n\n## Goals\n\nSecond"
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: content,
-        operations: [{ "op" => "insert_under_heading", "heading" => "## Goals", "content" => "stuff" }]
-      )
-    end
-    assert_match /found 2 headings/, error.message
-  end
-
-  test "delete_paragraph_containing removes paragraph" do
-    content = "First paragraph.\n\nThis is deprecated.\n\nThird paragraph."
-    result = Plans::ApplyOperations.call(
-      content: content,
-      operations: [{ "op" => "delete_paragraph_containing", "needle" => "deprecated" }]
-    )
-    assert_equal "First paragraph.\n\nThird paragraph.", result[:content]
-    assert_not_includes result[:content], "deprecated"
-  end
-
-  test "delete_paragraph_containing fails when not found" do
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: "Some content.",
-        operations: [{ "op" => "delete_paragraph_containing", "needle" => "missing" }]
-      )
-    end
-    assert_match /no paragraph containing/, error.message
-  end
-
-  test "delete_paragraph_containing fails when ambiguous" do
-    content = "First deprecated thing.\n\nSecond deprecated thing."
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: content,
-        operations: [{ "op" => "delete_paragraph_containing", "needle" => "deprecated" }]
-      )
-    end
-    assert_match /found 2 paragraphs/, error.message
-  end
-
-  test "unknown operation raises error" do
-    error = assert_raises(Plans::OperationError) do
-      Plans::ApplyOperations.call(
-        content: "Hello",
-        operations: [{ "op" => "unknown_op" }]
-      )
-    end
-    assert_match /unknown op/, error.message
-  end
-
-  test "multiple operations applied sequentially" do
-    content = "# Plan\n\n## Phase 1\n\nDo stuff.\n\n## Phase 2\n\nOld approach."
-    result = Plans::ApplyOperations.call(
-      content: content,
-      operations: [
-        { "op" => "replace_exact", "old_text" => "Do stuff.", "new_text" => "Do important stuff.", "count" => 1 },
-        { "op" => "insert_under_heading", "heading" => "## Phase 2", "content" => "\n- New step" }
-      ]
-    )
-    assert_includes result[:content], "Do important stuff."
-    assert_includes result[:content], "- New step"
-    assert_equal 2, result[:applied].length
-  end
-
-  test "operations with string keys work" do
-    result = Plans::ApplyOperations.call(
-      content: "Hello world",
-      operations: [{ "op" => "replace_exact", "old_text" => "world", "new_text" => "planet", "count" => 1 }]
-    )
-    assert_equal "Hello planet", result[:content]
-  end
-
-  test "operations with symbol keys work" do
-    result = Plans::ApplyOperations.call(
-      content: "Hello world",
-      operations: [{ op: "replace_exact", old_text: "world", new_text: "planet", count: 1 }]
-    )
-    assert_equal "Hello planet", result[:content]
-  end
-end
diff --git a/test/services/plans/create_test.rb b/test/services/plans/create_test.rb
deleted file mode 100644
index 72b16a59..00000000
--- a/test/services/plans/create_test.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-require "test_helper"
-
-class Plans::CreateTest < ActiveSupport::TestCase
-  test "creates plan with initial version" do
-    user = users(:alice)
-    plan = Plans::Create.call(
-      title: "New Plan",
-      content: "# New Plan\n\nSome content.",
-      user: user
-    )
-
-    assert plan.persisted?
-    assert_equal "New Plan", plan.title
-    assert_equal "brainstorm", plan.status
-    assert_equal user, plan.created_by_user
-    assert_equal user.organization, plan.organization
-    assert_equal 1, plan.current_revision
-    assert_equal 1, plan.plan_versions.count
-
-    version = plan.current_plan_version
-    assert_equal "# New Plan\n\nSome content.", version.content_markdown
-    assert_equal 1, version.revision
-    assert_equal "human", version.actor_type
-    assert_equal user.id, version.actor_id
-    assert version.content_sha256.present?
-  end
-end
diff --git a/test/system/.keep b/test/system/.keep
deleted file mode 100644
index e69de29b..00000000
diff --git a/test/test_helper.rb b/test/test_helper.rb
deleted file mode 100644
index f2fda62d..00000000
--- a/test/test_helper.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-ENV["RAILS_ENV"] ||= "test"
-require_relative "../config/environment"
-require "rails/test_help"
-
-module ActiveSupport
-  class TestCase
-    # Run tests in parallel with specified workers
-    parallelize(workers: :number_of_processors)
-
-    # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
-    fixtures :all
-
-    def sign_in_as(user)
-      post sign_in_path, params: { email: user.email }
-    end
-  end
-end

From 119ed28a6825c3b6e9c9810ec044b299594bd75b Mon Sep 17 00:00:00 2001
From: Hampton Lintorn-Catlin <hampton@squareup.com>
Date: Mon, 23 Feb 2026 15:30:04 -0600
Subject: [PATCH 2/5] Add automated review bots (Cloud Personas)

Implements Phase 7 from the plan:

- AiProviders::OpenAi service wrapping ruby-openai gem
- AiProviders::Anthropic stub for future implementation
- AutomatedReviewJob (SolidQueue) that calls AI and creates review comments
- Plans::TriggerAutomatedReviews service for auto-triggering on status change
- AutomatedReviewsController for manual 'Run Reviewer' from the UI
- Dropdown Stimulus controller + CSS for reviewer selection
- 20 new specs covering all components (all 195 specs pass)

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019c8c64-60ae-7133-b471-efb8f56a18a9
---
 app/assets/stylesheets/application.css        | 35 ++++++++
 .../automated_reviews_controller.rb           | 27 ++++++
 app/controllers/plans_controller.rb           |  1 +
 .../controllers/dropdown_controller.js        | 25 ++++++
 app/jobs/automated_review_job.rb              | 65 +++++++++++++++
 app/services/ai_providers/anthropic.rb        | 19 +++++
 app/services/ai_providers/open_ai.rb          | 42 ++++++++++
 .../plans/trigger_automated_reviews.rb        | 26 ++++++
 app/views/plans/_header.html.erb              | 15 ++++
 config/routes.rb                              |  1 +
 .../automated_reviews_controller_spec.rb      | 37 +++++++++
 spec/jobs/automated_review_job_spec.rb        | 83 +++++++++++++++++++
 spec/services/ai_providers/open_ai_spec.rb    | 58 +++++++++++++
 .../plans/trigger_automated_reviews_spec.rb   | 78 +++++++++++++++++
 14 files changed, 512 insertions(+)
 create mode 100644 app/controllers/automated_reviews_controller.rb
 create mode 100644 app/javascript/controllers/dropdown_controller.js
 create mode 100644 app/jobs/automated_review_job.rb
 create mode 100644 app/services/ai_providers/anthropic.rb
 create mode 100644 app/services/ai_providers/open_ai.rb
 create mode 100644 app/services/plans/trigger_automated_reviews.rb
 create mode 100644 spec/controllers/automated_reviews_controller_spec.rb
 create mode 100644 spec/jobs/automated_review_job_spec.rb
 create mode 100644 spec/services/ai_providers/open_ai_spec.rb
 create mode 100644 spec/services/plans/trigger_automated_reviews_spec.rb

diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css
index 03090a5b..6a2fd144 100644
--- a/app/assets/stylesheets/application.css
+++ b/app/assets/stylesheets/application.css
@@ -914,6 +914,41 @@ img, svg {
   min-width: 200px;
 }
 
+/* Dropdown */
+.dropdown {
+  position: relative;
+  display: inline-block;
+}
+
+.dropdown__menu {
+  position: absolute;
+  right: 0;
+  top: 100%;
+  margin-top: var(--space-xs);
+  background: var(--color-bg);
+  border: 1px solid var(--color-border);
+  border-radius: var(--radius);
+  box-shadow: 0 4px 12px rgba(0, 0, 0, 0.1);
+  min-width: 200px;
+  z-index: 100;
+}
+
+.dropdown__item {
+  display: block;
+  width: 100%;
+  padding: var(--space-sm) var(--space-md);
+  text-align: left;
+  background: none;
+  border: none;
+  cursor: pointer;
+  font-size: var(--font-sm);
+  color: var(--color-text);
+}
+
+.dropdown__item:hover {
+  background: var(--color-bg-muted);
+}
+
 /* Page header */
 .page-header__subtitle {
   color: var(--color-text-muted);
diff --git a/app/controllers/automated_reviews_controller.rb b/app/controllers/automated_reviews_controller.rb
new file mode 100644
index 00000000..af12f1d9
--- /dev/null
+++ b/app/controllers/automated_reviews_controller.rb
@@ -0,0 +1,27 @@
+class AutomatedReviewsController < ApplicationController
+  before_action :scope_to_organization
+  before_action :set_plan
+  before_action :set_reviewer, only: [:create]
+
+  def create
+    authorize!(@plan, :update?)
+
+    AutomatedReviewJob.perform_later(
+      plan_id: @plan.id,
+      reviewer_id: @reviewer.id,
+      triggered_by: current_user
+    )
+
+    redirect_to plan_path(@plan), notice: "#{@reviewer.name} review queued."
+  end
+
+  private
+
+  def set_plan
+    @plan = @organization.plans.find(params[:plan_id])
+  end
+
+  def set_reviewer
+    @reviewer = @organization.automated_plan_reviewers.enabled.find(params[:reviewer_id])
+  end
+end
diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb
index 32be818d..5d3b95ce 100644
--- a/app/controllers/plans_controller.rb
+++ b/app/controllers/plans_controller.rb
@@ -48,6 +48,7 @@ def update_status
     new_status = params[:status]
     if Plan::STATUSES.include?(new_status) && @plan.update(status: new_status)
       broadcast_plan_update(@plan)
+      Plans::TriggerAutomatedReviews.call(plan: @plan, new_status: new_status, triggered_by: current_user)
       redirect_to plan_path(@plan), notice: "Status updated to #{new_status}."
     else
       redirect_to plan_path(@plan), alert: "Invalid status."
diff --git a/app/javascript/controllers/dropdown_controller.js b/app/javascript/controllers/dropdown_controller.js
new file mode 100644
index 00000000..5e532474
--- /dev/null
+++ b/app/javascript/controllers/dropdown_controller.js
@@ -0,0 +1,25 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  static targets = ["menu"]
+
+  toggle() {
+    const menu = this.menuTarget
+    menu.style.display = menu.style.display === "none" ? "" : "none"
+  }
+
+  close(event) {
+    if (!this.element.contains(event.target)) {
+      this.menuTarget.style.display = "none"
+    }
+  }
+
+  connect() {
+    this._closeHandler = this.close.bind(this)
+    document.addEventListener("click", this._closeHandler)
+  }
+
+  disconnect() {
+    document.removeEventListener("click", this._closeHandler)
+  }
+}
diff --git a/app/jobs/automated_review_job.rb b/app/jobs/automated_review_job.rb
new file mode 100644
index 00000000..875cd148
--- /dev/null
+++ b/app/jobs/automated_review_job.rb
@@ -0,0 +1,65 @@
+class AutomatedReviewJob < ApplicationJob
+  queue_as :default
+
+  discard_on AiProviders::OpenAi::Error
+  discard_on AiProviders::Anthropic::Error
+
+  def perform(plan_id:, reviewer_id:, triggered_by: nil)
+    plan = Plan.find(plan_id)
+    reviewer = AutomatedPlanReviewer.find(reviewer_id)
+    version = plan.current_plan_version
+
+    return unless version
+    return unless reviewer.enabled?
+
+    response = call_ai_provider(reviewer, version.content_markdown)
+
+    create_review_comment(plan, version, reviewer, response, triggered_by)
+  end
+
+  private
+
+  def call_ai_provider(reviewer, content)
+    provider_class = resolve_provider(reviewer.ai_provider)
+    provider_class.call(
+      system_prompt: reviewer.prompt_text,
+      user_content: content,
+      model: reviewer.ai_model
+    )
+  end
+
+  def resolve_provider(provider_name)
+    case provider_name
+    when "openai" then AiProviders::OpenAi
+    when "anthropic" then AiProviders::Anthropic
+    else raise ArgumentError, "Unknown AI provider: #{provider_name}"
+    end
+  end
+
+  def create_review_comment(plan, version, reviewer, response, triggered_by)
+    thread = plan.comment_threads.create!(
+      organization: plan.organization,
+      plan_version: version,
+      created_by_user: triggered_by || plan.created_by_user,
+      status: "open"
+    )
+
+    thread.comments.create!(
+      organization: plan.organization,
+      author_type: AutomatedPlanReviewer::ACTOR_TYPE,
+      author_id: reviewer.id,
+      body_markdown: response
+    )
+
+    broadcast_new_thread(plan, thread)
+  end
+
+  def broadcast_new_thread(plan, thread)
+    Turbo::StreamsChannel.broadcast_prepend_to(
+      plan,
+      target: "comment-threads",
+      partial: "comment_threads/thread",
+      locals: { thread: thread, plan: plan }
+    )
+  end
+end
diff --git a/app/services/ai_providers/anthropic.rb b/app/services/ai_providers/anthropic.rb
new file mode 100644
index 00000000..572334fb
--- /dev/null
+++ b/app/services/ai_providers/anthropic.rb
@@ -0,0 +1,19 @@
+module AiProviders
+  class Anthropic
+    def self.call(system_prompt:, user_content:, model: "claude-sonnet-4-20250514")
+      new(system_prompt:, user_content:, model:).call
+    end
+
+    def initialize(system_prompt:, user_content:, model:)
+      @system_prompt = system_prompt
+      @user_content = user_content
+      @model = model
+    end
+
+    def call
+      raise Error, "Anthropic provider not yet implemented. Use OpenAI for now."
+    end
+
+    class Error < StandardError; end
+  end
+end
diff --git a/app/services/ai_providers/open_ai.rb b/app/services/ai_providers/open_ai.rb
new file mode 100644
index 00000000..5f90c4b0
--- /dev/null
+++ b/app/services/ai_providers/open_ai.rb
@@ -0,0 +1,42 @@
+module AiProviders
+  class OpenAi
+    def self.call(system_prompt:, user_content:, model: "gpt-4o")
+      new(system_prompt:, user_content:, model:).call
+    end
+
+    def initialize(system_prompt:, user_content:, model:)
+      @system_prompt = system_prompt
+      @user_content = user_content
+      @model = model
+    end
+
+    def call
+      client = OpenAI::Client.new(access_token: api_key)
+
+      response = client.chat(
+        parameters: {
+          model: @model,
+          messages: [
+            { role: "system", content: @system_prompt },
+            { role: "user", content: @user_content }
+          ]
+        }
+      )
+
+      content = response.dig("choices", 0, "message", "content")
+      raise Error, "No response content from OpenAI" if content.blank?
+
+      content
+    end
+
+    private
+
+    def api_key
+      key = Rails.application.credentials.dig(:openai, :api_key) || ENV["OPENAI_API_KEY"]
+      raise Error, "OpenAI API key not configured" if key.blank?
+      key
+    end
+
+    class Error < StandardError; end
+  end
+end
diff --git a/app/services/plans/trigger_automated_reviews.rb b/app/services/plans/trigger_automated_reviews.rb
new file mode 100644
index 00000000..9f89ad6c
--- /dev/null
+++ b/app/services/plans/trigger_automated_reviews.rb
@@ -0,0 +1,26 @@
+module Plans
+  class TriggerAutomatedReviews
+    def self.call(plan:, new_status:, triggered_by:)
+      new(plan:, new_status:, triggered_by:).call
+    end
+
+    def initialize(plan:, new_status:, triggered_by:)
+      @plan = plan
+      @new_status = new_status
+      @triggered_by = triggered_by
+    end
+
+    def call
+      reviewers = @plan.organization.automated_plan_reviewers.enabled
+      reviewers.each do |reviewer|
+        next unless reviewer.triggers_on_status?(@new_status)
+
+        AutomatedReviewJob.perform_later(
+          plan_id: @plan.id,
+          reviewer_id: reviewer.id,
+          triggered_by: @triggered_by
+        )
+      end
+    end
+  end
+end
diff --git a/app/views/plans/_header.html.erb b/app/views/plans/_header.html.erb
index 2ae5950a..c51c086b 100644
--- a/app/views/plans/_header.html.erb
+++ b/app/views/plans/_header.html.erb
@@ -6,6 +6,21 @@
     </div>
   </div>
   <div class="page-header__actions">
+    <% if plan.organization.automated_plan_reviewers.enabled.any? %>
+      <div class="dropdown" data-controller="dropdown">
+        <button class="btn btn--secondary" data-action="dropdown#toggle">
+          🤖 Run Reviewer ▾
+        </button>
+        <div class="dropdown__menu" data-dropdown-target="menu" style="display: none;">
+          <% plan.organization.automated_plan_reviewers.enabled.order(:name).each do |reviewer| %>
+            <%= button_to reviewer.name,
+                  plan_automated_reviews_path(plan, reviewer_id: reviewer.id),
+                  method: :post,
+                  class: "dropdown__item" %>
+          <% end %>
+        </div>
+      </div>
+    <% end %>
     <%= link_to "History (v#{plan.current_revision})", plan_versions_path(plan), class: "btn btn--secondary" %>
   </div>
 </div>
diff --git a/config/routes.rb b/config/routes.rb
index fce87c35..8ebe689b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -17,6 +17,7 @@
   resources :plans, only: [:index, :show, :edit, :update] do
     patch :update_status, on: :member
     resources :versions, controller: "plan_versions", only: [:index, :show]
+    resources :automated_reviews, only: [:create]
     resources :comment_threads, only: [:create] do
       member do
         patch :resolve
diff --git a/spec/controllers/automated_reviews_controller_spec.rb b/spec/controllers/automated_reviews_controller_spec.rb
new file mode 100644
index 00000000..86a9a64a
--- /dev/null
+++ b/spec/controllers/automated_reviews_controller_spec.rb
@@ -0,0 +1,37 @@
+require "rails_helper"
+
+RSpec.describe AutomatedReviewsController, type: :controller do
+  let(:org) { create(:organization) }
+  let(:user) { create(:user, organization: org) }
+  let(:plan) { create(:plan, organization: org, created_by_user: user) }
+  let!(:reviewer) { create(:automated_plan_reviewer, organization: org, enabled: true) }
+
+  before do
+    session[:user_id] = user.id
+  end
+
+  describe "POST #create" do
+    it "enqueues an AutomatedReviewJob" do
+      expect {
+        post :create, params: { plan_id: plan.id, reviewer_id: reviewer.id }
+      }.to have_enqueued_job(AutomatedReviewJob).with(
+        plan_id: plan.id,
+        reviewer_id: reviewer.id,
+        triggered_by: user
+      )
+    end
+
+    it "redirects to the plan with a notice" do
+      post :create, params: { plan_id: plan.id, reviewer_id: reviewer.id }
+      expect(response).to redirect_to(plan_path(plan))
+      expect(flash[:notice]).to include(reviewer.name)
+    end
+
+    it "returns 404 for disabled reviewers" do
+      reviewer.update!(enabled: false)
+      expect {
+        post :create, params: { plan_id: plan.id, reviewer_id: reviewer.id }
+      }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+end
diff --git a/spec/jobs/automated_review_job_spec.rb b/spec/jobs/automated_review_job_spec.rb
new file mode 100644
index 00000000..b194a0c5
--- /dev/null
+++ b/spec/jobs/automated_review_job_spec.rb
@@ -0,0 +1,83 @@
+require "rails_helper"
+
+RSpec.describe AutomatedReviewJob, type: :job do
+  let(:plan) { create(:plan, :considering) }
+  let(:reviewer) { create(:automated_plan_reviewer, organization: plan.organization) }
+  let(:user) { plan.created_by_user }
+
+  before do
+    allow(AiProviders::OpenAi).to receive(:call).and_return("## Security Review\n\nLooks good overall.")
+  end
+
+  describe "#perform" do
+    it "creates a comment thread with the AI response" do
+      expect {
+        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      }.to change(CommentThread, :count).by(1)
+        .and change(Comment, :count).by(1)
+    end
+
+    it "creates the comment with cloud_persona author type" do
+      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+
+      comment = Comment.last
+      expect(comment.author_type).to eq("cloud_persona")
+      expect(comment.author_id).to eq(reviewer.id)
+      expect(comment.body_markdown).to eq("## Security Review\n\nLooks good overall.")
+    end
+
+    it "creates a general (non-anchored) comment thread" do
+      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+
+      thread = CommentThread.last
+      expect(thread.anchor_text).to be_nil
+      expect(thread.plan_version).to eq(plan.current_plan_version)
+      expect(thread.status).to eq("open")
+    end
+
+    it "sets the triggered_by user as the thread creator" do
+      other_user = create(:user, organization: plan.organization)
+      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: other_user)
+
+      expect(CommentThread.last.created_by_user).to eq(other_user)
+    end
+
+    it "falls back to plan author when triggered_by is nil" do
+      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id)
+
+      expect(CommentThread.last.created_by_user).to eq(plan.created_by_user)
+    end
+
+    it "calls the AI provider with the reviewer prompt and plan content" do
+      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+
+      expect(AiProviders::OpenAi).to have_received(:call).with(
+        system_prompt: reviewer.prompt_text,
+        user_content: plan.current_plan_version.content_markdown,
+        model: reviewer.ai_model
+      )
+    end
+
+    it "does nothing if the reviewer is disabled" do
+      reviewer.update!(enabled: false)
+
+      expect {
+        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      }.not_to change(CommentThread, :count)
+    end
+
+    it "does nothing if the plan has no current version" do
+      plan.update_columns(current_plan_version_id: nil)
+
+      expect {
+        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      }.not_to change(CommentThread, :count)
+    end
+
+    it "enqueues on the default queue" do
+      expect {
+        described_class.perform_later(plan_id: plan.id, reviewer_id: reviewer.id)
+      }.to have_enqueued_job(described_class).on_queue("default")
+    end
+  end
+end
diff --git a/spec/services/ai_providers/open_ai_spec.rb b/spec/services/ai_providers/open_ai_spec.rb
new file mode 100644
index 00000000..a8a5a10b
--- /dev/null
+++ b/spec/services/ai_providers/open_ai_spec.rb
@@ -0,0 +1,58 @@
+require "rails_helper"
+
+RSpec.describe AiProviders::OpenAi do
+  let(:model) { "gpt-4o" }
+  let(:system_prompt) { "You are a reviewer." }
+  let(:user_content) { "# My Plan\n\nSome content." }
+
+  before do
+    allow(ENV).to receive(:[]).and_call_original
+    allow(ENV).to receive(:[]).with("OPENAI_API_KEY").and_return("test-key")
+  end
+
+  describe ".call" do
+    it "returns the AI response content" do
+      mock_client = instance_double(OpenAI::Client)
+      allow(OpenAI::Client).to receive(:new).and_return(mock_client)
+      allow(mock_client).to receive(:chat).and_return({
+        "choices" => [{ "message" => { "content" => "Review feedback here." } }]
+      })
+
+      result = described_class.call(
+        system_prompt: system_prompt,
+        user_content: user_content,
+        model: model
+      )
+
+      expect(result).to eq("Review feedback here.")
+      expect(mock_client).to have_received(:chat).with(
+        parameters: {
+          model: model,
+          messages: [
+            { role: "system", content: system_prompt },
+            { role: "user", content: user_content }
+          ]
+        }
+      )
+    end
+
+    it "raises an error when response has no content" do
+      mock_client = instance_double(OpenAI::Client)
+      allow(OpenAI::Client).to receive(:new).and_return(mock_client)
+      allow(mock_client).to receive(:chat).and_return({ "choices" => [] })
+
+      expect {
+        described_class.call(system_prompt: system_prompt, user_content: user_content, model: model)
+      }.to raise_error(AiProviders::OpenAi::Error, "No response content from OpenAI")
+    end
+
+    it "raises an error when API key is not configured" do
+      allow(ENV).to receive(:[]).with("OPENAI_API_KEY").and_return(nil)
+      allow(Rails.application.credentials).to receive(:dig).with(:openai, :api_key).and_return(nil)
+
+      expect {
+        described_class.call(system_prompt: system_prompt, user_content: user_content, model: model)
+      }.to raise_error(AiProviders::OpenAi::Error, "OpenAI API key not configured")
+    end
+  end
+end
diff --git a/spec/services/plans/trigger_automated_reviews_spec.rb b/spec/services/plans/trigger_automated_reviews_spec.rb
new file mode 100644
index 00000000..c725f936
--- /dev/null
+++ b/spec/services/plans/trigger_automated_reviews_spec.rb
@@ -0,0 +1,78 @@
+require "rails_helper"
+
+RSpec.describe Plans::TriggerAutomatedReviews do
+  let(:plan) { create(:plan, :considering) }
+  let(:user) { plan.created_by_user }
+
+  before do
+    # Clear default reviewers created by Organization after_create callback
+    plan.organization.automated_plan_reviewers.destroy_all
+  end
+
+  describe ".call" do
+    it "enqueues jobs for reviewers that trigger on the given status" do
+      reviewer = create(:automated_plan_reviewer,
+        organization: plan.organization,
+        trigger_statuses: ["considering"],
+        enabled: true
+      )
+
+      expect {
+        described_class.call(plan: plan, new_status: "considering", triggered_by: user)
+      }.to have_enqueued_job(AutomatedReviewJob).with(
+        plan_id: plan.id,
+        reviewer_id: reviewer.id,
+        triggered_by: user
+      )
+    end
+
+    it "does not enqueue jobs for reviewers that do not trigger on the status" do
+      create(:automated_plan_reviewer,
+        organization: plan.organization,
+        trigger_statuses: ["developing"],
+        enabled: true
+      )
+
+      expect {
+        described_class.call(plan: plan, new_status: "considering", triggered_by: user)
+      }.not_to have_enqueued_job(AutomatedReviewJob)
+    end
+
+    it "does not enqueue jobs for disabled reviewers" do
+      create(:automated_plan_reviewer,
+        organization: plan.organization,
+        trigger_statuses: ["considering"],
+        enabled: false
+      )
+
+      expect {
+        described_class.call(plan: plan, new_status: "considering", triggered_by: user)
+      }.not_to have_enqueued_job(AutomatedReviewJob)
+    end
+
+    it "enqueues multiple jobs when multiple reviewers match" do
+      create(:automated_plan_reviewer,
+        organization: plan.organization,
+        trigger_statuses: ["considering"],
+        enabled: true,
+        key: "reviewer-a"
+      )
+      create(:automated_plan_reviewer,
+        organization: plan.organization,
+        trigger_statuses: ["considering"],
+        enabled: true,
+        key: "reviewer-b"
+      )
+
+      expect {
+        described_class.call(plan: plan, new_status: "considering", triggered_by: user)
+      }.to have_enqueued_job(AutomatedReviewJob).exactly(2).times
+    end
+
+    it "does nothing when no reviewers exist" do
+      expect {
+        described_class.call(plan: plan, new_status: "considering", triggered_by: user)
+      }.not_to have_enqueued_job(AutomatedReviewJob)
+    end
+  end
+end

From 8703ec2ddc307e1f8ac4487b6174d9bdbea01c6f Mon Sep 17 00:00:00 2001
From: Hampton Lintorn-Catlin <hampton@squareup.com>
Date: Mon, 23 Feb 2026 16:27:12 -0600
Subject: [PATCH 3/5] Split AI review into individual anchored comment threads

Instead of one big comment, the AI reviewer now returns structured JSON
with individual feedback items. Each item becomes its own CommentThread,
anchored to the specific plan text it references.

- ReviewPromptFormatter: wraps reviewer prompt with JSON output instructions
- ReviewResponseParser: parses JSON response, validates anchor_text against
  plan content (demotes non-matching anchors to blockquotes), gracefully
  falls back to single unanchored comment for non-JSON responses
- Updated AutomatedReviewJob to create one thread per feedback item
- 10 new specs for parser + formatter, updated job specs (205 total, 0 failures)

Amp-Thread-ID: https://ampcode.com/threads/T-019c8c64-60ae-7133-b471-efb8f56a18a9
Co-authored-by: Amp <amp@ampcode.com>
---
 app/jobs/automated_review_job.rb              | 39 ++++++----
 app/services/plans/review_prompt_formatter.rb | 23 ++++++
 app/services/plans/review_response_parser.rb  | 59 ++++++++++++++
 spec/jobs/automated_review_job_spec.rb        | 63 ++++++++++-----
 .../plans/review_prompt_formatter_spec.rb     | 21 +++++
 .../plans/review_response_parser_spec.rb      | 76 +++++++++++++++++++
 6 files changed, 246 insertions(+), 35 deletions(-)
 create mode 100644 app/services/plans/review_prompt_formatter.rb
 create mode 100644 app/services/plans/review_response_parser.rb
 create mode 100644 spec/services/plans/review_prompt_formatter_spec.rb
 create mode 100644 spec/services/plans/review_response_parser_spec.rb

diff --git a/app/jobs/automated_review_job.rb b/app/jobs/automated_review_job.rb
index 875cd148..0740aea5 100644
--- a/app/jobs/automated_review_job.rb
+++ b/app/jobs/automated_review_job.rb
@@ -13,16 +13,18 @@ def perform(plan_id:, reviewer_id:, triggered_by: nil)
     return unless reviewer.enabled?
 
     response = call_ai_provider(reviewer, version.content_markdown)
+    feedback_items = Plans::ReviewResponseParser.call(response, plan_content: version.content_markdown)
 
-    create_review_comment(plan, version, reviewer, response, triggered_by)
+    create_review_comments(plan, version, reviewer, feedback_items, triggered_by)
   end
 
   private
 
   def call_ai_provider(reviewer, content)
+    system_prompt = Plans::ReviewPromptFormatter.call(reviewer_prompt: reviewer.prompt_text)
     provider_class = resolve_provider(reviewer.ai_provider)
     provider_class.call(
-      system_prompt: reviewer.prompt_text,
+      system_prompt: system_prompt,
       user_content: content,
       model: reviewer.ai_model
     )
@@ -36,22 +38,27 @@ def resolve_provider(provider_name)
     end
   end
 
-  def create_review_comment(plan, version, reviewer, response, triggered_by)
-    thread = plan.comment_threads.create!(
-      organization: plan.organization,
-      plan_version: version,
-      created_by_user: triggered_by || plan.created_by_user,
-      status: "open"
-    )
+  def create_review_comments(plan, version, reviewer, feedback_items, triggered_by)
+    created_by = triggered_by || plan.created_by_user
 
-    thread.comments.create!(
-      organization: plan.organization,
-      author_type: AutomatedPlanReviewer::ACTOR_TYPE,
-      author_id: reviewer.id,
-      body_markdown: response
-    )
+    feedback_items.each do |item|
+      thread = plan.comment_threads.create!(
+        organization: plan.organization,
+        plan_version: version,
+        created_by_user: created_by,
+        anchor_text: item[:anchor_text],
+        status: "open"
+      )
 
-    broadcast_new_thread(plan, thread)
+      thread.comments.create!(
+        organization: plan.organization,
+        author_type: AutomatedPlanReviewer::ACTOR_TYPE,
+        author_id: reviewer.id,
+        body_markdown: item[:comment]
+      )
+
+      broadcast_new_thread(plan, thread)
+    end
   end
 
   def broadcast_new_thread(plan, thread)
diff --git a/app/services/plans/review_prompt_formatter.rb b/app/services/plans/review_prompt_formatter.rb
new file mode 100644
index 00000000..e98e1341
--- /dev/null
+++ b/app/services/plans/review_prompt_formatter.rb
@@ -0,0 +1,23 @@
+module Plans
+  class ReviewPromptFormatter
+    RESPONSE_FORMAT_INSTRUCTIONS = <<~INSTRUCTIONS.freeze
+      You MUST respond with a JSON array of feedback items. Each item is an object with two keys:
+      - "anchor_text": An exact substring copied verbatim from the plan document that this feedback applies to. Keep it short (a phrase or single sentence). Must match the plan text exactly. Use null for general feedback not tied to specific text.
+      - "comment": Your feedback in Markdown. Be concise and actionable.
+
+      Example response:
+      ```json
+      [
+        {"anchor_text": "API tokens scoped to a user", "comment": "Consider adding token expiration by default. Long-lived tokens without expiry are a common security risk."},
+        {"anchor_text": null, "comment": "Overall the plan looks solid. One general concern: there's no mention of audit logging for administrative actions."}
+      ]
+      ```
+
+      Return ONLY the JSON array. No other text before or after it.
+    INSTRUCTIONS
+
+    def self.call(reviewer_prompt:)
+      "#{reviewer_prompt}\n\n#{RESPONSE_FORMAT_INSTRUCTIONS}"
+    end
+  end
+end
diff --git a/app/services/plans/review_response_parser.rb b/app/services/plans/review_response_parser.rb
new file mode 100644
index 00000000..808fbd06
--- /dev/null
+++ b/app/services/plans/review_response_parser.rb
@@ -0,0 +1,59 @@
+module Plans
+  class ReviewResponseParser
+    def self.call(response_text, plan_content:)
+      new(response_text, plan_content:).call
+    end
+
+    def initialize(response_text, plan_content:)
+      @response_text = response_text
+      @plan_content = plan_content
+    end
+
+    def call
+      items = parse_json
+      items.map { |item| normalize_item(item) }
+    end
+
+    private
+
+    def parse_json
+      json_text = extract_json_from_response
+      parsed = JSON.parse(json_text)
+
+      unless parsed.is_a?(Array)
+        return fallback_single_comment
+      end
+
+      parsed
+    rescue JSON::ParserError
+      fallback_single_comment
+    end
+
+    def extract_json_from_response
+      # Strip markdown code fences if present
+      text = @response_text.strip
+      if text.start_with?("```")
+        text = text.sub(/\A```(?:json)?\s*\n?/, "").sub(/\n?```\s*\z/, "")
+      end
+      text
+    end
+
+    def normalize_item(item)
+      anchor = item["anchor_text"].presence
+      comment = item["comment"].to_s.strip
+
+      # Verify anchor_text actually exists in the plan content
+      if anchor && !@plan_content.include?(anchor)
+        # Anchor doesn't match — demote to unanchored with the quote in the comment
+        comment = "> #{anchor}\n\n#{comment}"
+        anchor = nil
+      end
+
+      { anchor_text: anchor, comment: comment }
+    end
+
+    def fallback_single_comment
+      [{ "anchor_text" => nil, "comment" => @response_text }]
+    end
+  end
+end
diff --git a/spec/jobs/automated_review_job_spec.rb b/spec/jobs/automated_review_job_spec.rb
index b194a0c5..e77e79f8 100644
--- a/spec/jobs/automated_review_job_spec.rb
+++ b/spec/jobs/automated_review_job_spec.rb
@@ -4,60 +4,85 @@
   let(:plan) { create(:plan, :considering) }
   let(:reviewer) { create(:automated_plan_reviewer, organization: plan.organization) }
   let(:user) { plan.created_by_user }
+  let(:plan_content) { plan.current_plan_version.content_markdown }
+
+  let(:structured_response) do
+    '[
+      {"anchor_text": "Plan Content", "comment": "The title should be more specific."},
+      {"anchor_text": "Some content here", "comment": "Expand on this section."},
+      {"anchor_text": null, "comment": "Overall looks good."}
+    ]'
+  end
 
   before do
-    allow(AiProviders::OpenAi).to receive(:call).and_return("## Security Review\n\nLooks good overall.")
+    allow(AiProviders::OpenAi).to receive(:call).and_return(structured_response)
   end
 
   describe "#perform" do
-    it "creates a comment thread with the AI response" do
+    it "creates one comment thread per feedback item" do
       expect {
         described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
-      }.to change(CommentThread, :count).by(1)
-        .and change(Comment, :count).by(1)
+      }.to change(CommentThread, :count).by(3)
+        .and change(Comment, :count).by(3)
     end
 
-    it "creates the comment with cloud_persona author type" do
+    it "anchors threads to matching plan text" do
       described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
 
-      comment = Comment.last
-      expect(comment.author_type).to eq("cloud_persona")
-      expect(comment.author_id).to eq(reviewer.id)
-      expect(comment.body_markdown).to eq("## Security Review\n\nLooks good overall.")
+      threads = CommentThread.order(:created_at).last(3)
+      expect(threads[0].anchor_text).to eq("Plan Content")
+      expect(threads[1].anchor_text).to eq("Some content here")
+      expect(threads[2].anchor_text).to be_nil
     end
 
-    it "creates a general (non-anchored) comment thread" do
+    it "creates comments with cloud_persona author type" do
       described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
 
-      thread = CommentThread.last
-      expect(thread.anchor_text).to be_nil
-      expect(thread.plan_version).to eq(plan.current_plan_version)
-      expect(thread.status).to eq("open")
+      Comment.last(3).each do |comment|
+        expect(comment.author_type).to eq("cloud_persona")
+        expect(comment.author_id).to eq(reviewer.id)
+      end
     end
 
     it "sets the triggered_by user as the thread creator" do
       other_user = create(:user, organization: plan.organization)
       described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: other_user)
 
-      expect(CommentThread.last.created_by_user).to eq(other_user)
+      CommentThread.last(3).each do |thread|
+        expect(thread.created_by_user).to eq(other_user)
+      end
     end
 
     it "falls back to plan author when triggered_by is nil" do
       described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id)
 
-      expect(CommentThread.last.created_by_user).to eq(plan.created_by_user)
+      CommentThread.last(3).each do |thread|
+        expect(thread.created_by_user).to eq(plan.created_by_user)
+      end
     end
 
-    it "calls the AI provider with the reviewer prompt and plan content" do
+    it "passes the formatted prompt to the AI provider" do
       described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
 
       expect(AiProviders::OpenAi).to have_received(:call).with(
-        system_prompt: reviewer.prompt_text,
-        user_content: plan.current_plan_version.content_markdown,
+        system_prompt: Plans::ReviewPromptFormatter.call(reviewer_prompt: reviewer.prompt_text),
+        user_content: plan_content,
         model: reviewer.ai_model
       )
     end
 
+    it "falls back to a single unanchored comment for non-JSON AI responses" do
+      allow(AiProviders::OpenAi).to receive(:call).and_return("Plain text review with no JSON.")
+
+      expect {
+        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      }.to change(CommentThread, :count).by(1)
+
+      thread = CommentThread.last
+      expect(thread.anchor_text).to be_nil
+      expect(thread.comments.first.body_markdown).to eq("Plain text review with no JSON.")
+    end
+
     it "does nothing if the reviewer is disabled" do
       reviewer.update!(enabled: false)
 
diff --git a/spec/services/plans/review_prompt_formatter_spec.rb b/spec/services/plans/review_prompt_formatter_spec.rb
new file mode 100644
index 00000000..7afe5163
--- /dev/null
+++ b/spec/services/plans/review_prompt_formatter_spec.rb
@@ -0,0 +1,21 @@
+require "rails_helper"
+
+RSpec.describe Plans::ReviewPromptFormatter do
+  describe ".call" do
+    it "appends JSON response format instructions to the reviewer prompt" do
+      result = described_class.call(reviewer_prompt: "Review for security issues.")
+
+      expect(result).to start_with("Review for security issues.")
+      expect(result).to include("JSON array")
+      expect(result).to include("anchor_text")
+      expect(result).to include("comment")
+    end
+
+    it "preserves the original reviewer prompt" do
+      original = "You are a scalability reviewer.\n\nFocus on database queries."
+      result = described_class.call(reviewer_prompt: original)
+
+      expect(result).to include(original)
+    end
+  end
+end
diff --git a/spec/services/plans/review_response_parser_spec.rb b/spec/services/plans/review_response_parser_spec.rb
new file mode 100644
index 00000000..f0dae7c7
--- /dev/null
+++ b/spec/services/plans/review_response_parser_spec.rb
@@ -0,0 +1,76 @@
+require "rails_helper"
+
+RSpec.describe Plans::ReviewResponseParser do
+  let(:plan_content) { "# My Plan\n\nWe use API tokens scoped to a user.\n\nNo rate limiting yet." }
+
+  describe ".call" do
+    it "parses a valid JSON array response into feedback items" do
+      response = '[
+        {"anchor_text": "API tokens scoped to a user", "comment": "Add token expiration."},
+        {"anchor_text": "No rate limiting yet", "comment": "Add rate limiting before launch."}
+      ]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items).to eq([
+        { anchor_text: "API tokens scoped to a user", comment: "Add token expiration." },
+        { anchor_text: "No rate limiting yet", comment: "Add rate limiting before launch." }
+      ])
+    end
+
+    it "handles JSON wrapped in markdown code fences" do
+      response = "```json\n[{\"anchor_text\": null, \"comment\": \"Looks good.\"}]\n```"
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items).to eq([{ anchor_text: nil, comment: "Looks good." }])
+    end
+
+    it "handles null anchor_text for general feedback" do
+      response = '[{"anchor_text": null, "comment": "Overall solid plan."}]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items.first[:anchor_text]).to be_nil
+      expect(items.first[:comment]).to eq("Overall solid plan.")
+    end
+
+    it "demotes anchor_text that does not match plan content to unanchored" do
+      response = '[{"anchor_text": "text that does not exist in the plan", "comment": "Some feedback."}]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items.first[:anchor_text]).to be_nil
+      expect(items.first[:comment]).to include("> text that does not exist in the plan")
+      expect(items.first[:comment]).to include("Some feedback.")
+    end
+
+    it "falls back to a single unanchored comment for non-JSON responses" do
+      response = "Here is my review in plain text.\n\nSome concerns about security."
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items.length).to eq(1)
+      expect(items.first[:anchor_text]).to be_nil
+      expect(items.first[:comment]).to eq(response)
+    end
+
+    it "falls back to a single comment when JSON is not an array" do
+      response = '{"anchor_text": "something", "comment": "not an array"}'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items.length).to eq(1)
+      expect(items.first[:anchor_text]).to be_nil
+      expect(items.first[:comment]).to eq(response)
+    end
+
+    it "handles empty anchor_text string as nil" do
+      response = '[{"anchor_text": "", "comment": "General note."}]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items.first[:anchor_text]).to be_nil
+    end
+  end
+end

From 0263f1d8b333e47d42fe16cee0e32991813b95aa Mon Sep 17 00:00:00 2001
From: Hampton Lintorn-Catlin <hampton@squareup.com>
Date: Mon, 23 Feb 2026 17:04:39 -0600
Subject: [PATCH 4/5] Address PR review: pin version at enqueue time, guard
 no-op status changes

- Pass plan_version_id to AutomatedReviewJob at enqueue time so the job
  reviews the version that triggered the review, not whatever is current
  when the worker picks it up
- Guard TriggerAutomatedReviews behind saved_change_to_status? so
  setting the same status twice doesn't enqueue duplicate review jobs
- Added spec for pinned version behavior

Amp-Thread-ID: https://ampcode.com/threads/T-019c8c64-60ae-7133-b471-efb8f56a18a9
Co-authored-by: Amp <amp@ampcode.com>
---
 .../automated_reviews_controller.rb           |  1 +
 app/controllers/plans_controller.rb           |  4 +-
 app/jobs/automated_review_job.rb              |  5 +--
 .../plans/trigger_automated_reviews.rb        |  4 ++
 .../automated_reviews_controller_spec.rb      |  1 +
 spec/jobs/automated_review_job_spec.rb        | 43 +++++++++++--------
 .../plans/trigger_automated_reviews_spec.rb   |  1 +
 7 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/app/controllers/automated_reviews_controller.rb b/app/controllers/automated_reviews_controller.rb
index af12f1d9..495d31ae 100644
--- a/app/controllers/automated_reviews_controller.rb
+++ b/app/controllers/automated_reviews_controller.rb
@@ -9,6 +9,7 @@ def create
     AutomatedReviewJob.perform_later(
       plan_id: @plan.id,
       reviewer_id: @reviewer.id,
+      plan_version_id: @plan.current_plan_version_id,
       triggered_by: current_user
     )
 
diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb
index 5d3b95ce..ca560b6b 100644
--- a/app/controllers/plans_controller.rb
+++ b/app/controllers/plans_controller.rb
@@ -48,7 +48,9 @@ def update_status
     new_status = params[:status]
     if Plan::STATUSES.include?(new_status) && @plan.update(status: new_status)
       broadcast_plan_update(@plan)
-      Plans::TriggerAutomatedReviews.call(plan: @plan, new_status: new_status, triggered_by: current_user)
+      if @plan.saved_change_to_status?
+        Plans::TriggerAutomatedReviews.call(plan: @plan, new_status: new_status, triggered_by: current_user)
+      end
       redirect_to plan_path(@plan), notice: "Status updated to #{new_status}."
     else
       redirect_to plan_path(@plan), alert: "Invalid status."
diff --git a/app/jobs/automated_review_job.rb b/app/jobs/automated_review_job.rb
index 0740aea5..c03fdc4f 100644
--- a/app/jobs/automated_review_job.rb
+++ b/app/jobs/automated_review_job.rb
@@ -4,12 +4,11 @@ class AutomatedReviewJob < ApplicationJob
   discard_on AiProviders::OpenAi::Error
   discard_on AiProviders::Anthropic::Error
 
-  def perform(plan_id:, reviewer_id:, triggered_by: nil)
+  def perform(plan_id:, reviewer_id:, plan_version_id:, triggered_by: nil)
     plan = Plan.find(plan_id)
     reviewer = AutomatedPlanReviewer.find(reviewer_id)
-    version = plan.current_plan_version
+    version = PlanVersion.find(plan_version_id)
 
-    return unless version
     return unless reviewer.enabled?
 
     response = call_ai_provider(reviewer, version.content_markdown)
diff --git a/app/services/plans/trigger_automated_reviews.rb b/app/services/plans/trigger_automated_reviews.rb
index 9f89ad6c..03e209c9 100644
--- a/app/services/plans/trigger_automated_reviews.rb
+++ b/app/services/plans/trigger_automated_reviews.rb
@@ -11,6 +11,9 @@ def initialize(plan:, new_status:, triggered_by:)
     end
 
     def call
+      version_id = @plan.current_plan_version_id
+      return unless version_id
+
       reviewers = @plan.organization.automated_plan_reviewers.enabled
       reviewers.each do |reviewer|
         next unless reviewer.triggers_on_status?(@new_status)
@@ -18,6 +21,7 @@ def call
         AutomatedReviewJob.perform_later(
           plan_id: @plan.id,
           reviewer_id: reviewer.id,
+          plan_version_id: version_id,
           triggered_by: @triggered_by
         )
       end
diff --git a/spec/controllers/automated_reviews_controller_spec.rb b/spec/controllers/automated_reviews_controller_spec.rb
index 86a9a64a..654ff4d9 100644
--- a/spec/controllers/automated_reviews_controller_spec.rb
+++ b/spec/controllers/automated_reviews_controller_spec.rb
@@ -17,6 +17,7 @@
       }.to have_enqueued_job(AutomatedReviewJob).with(
         plan_id: plan.id,
         reviewer_id: reviewer.id,
+        plan_version_id: plan.current_plan_version_id,
         triggered_by: user
       )
     end
diff --git a/spec/jobs/automated_review_job_spec.rb b/spec/jobs/automated_review_job_spec.rb
index e77e79f8..3d0b62a7 100644
--- a/spec/jobs/automated_review_job_spec.rb
+++ b/spec/jobs/automated_review_job_spec.rb
@@ -4,7 +4,8 @@
   let(:plan) { create(:plan, :considering) }
   let(:reviewer) { create(:automated_plan_reviewer, organization: plan.organization) }
   let(:user) { plan.created_by_user }
-  let(:plan_content) { plan.current_plan_version.content_markdown }
+  let(:version) { plan.current_plan_version }
+  let(:plan_content) { version.content_markdown }
 
   let(:structured_response) do
     '[
@@ -14,6 +15,8 @@
     ]'
   end
 
+  let(:perform_args) { { plan_id: plan.id, reviewer_id: reviewer.id, plan_version_id: version.id, triggered_by: user } }
+
   before do
     allow(AiProviders::OpenAi).to receive(:call).and_return(structured_response)
   end
@@ -21,13 +24,13 @@
   describe "#perform" do
     it "creates one comment thread per feedback item" do
       expect {
-        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+        described_class.perform_now(**perform_args)
       }.to change(CommentThread, :count).by(3)
         .and change(Comment, :count).by(3)
     end
 
     it "anchors threads to matching plan text" do
-      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      described_class.perform_now(**perform_args)
 
       threads = CommentThread.order(:created_at).last(3)
       expect(threads[0].anchor_text).to eq("Plan Content")
@@ -35,8 +38,20 @@
       expect(threads[2].anchor_text).to be_nil
     end
 
+    it "attaches threads to the pinned version, not the current one" do
+      old_version = version
+      new_version = create(:plan_version, plan: plan, organization: plan.organization, revision: 2)
+      plan.update!(current_plan_version: new_version, current_revision: 2)
+
+      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, plan_version_id: old_version.id, triggered_by: user)
+
+      CommentThread.last(3).each do |thread|
+        expect(thread.plan_version).to eq(old_version)
+      end
+    end
+
     it "creates comments with cloud_persona author type" do
-      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      described_class.perform_now(**perform_args)
 
       Comment.last(3).each do |comment|
         expect(comment.author_type).to eq("cloud_persona")
@@ -46,7 +61,7 @@
 
     it "sets the triggered_by user as the thread creator" do
       other_user = create(:user, organization: plan.organization)
-      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: other_user)
+      described_class.perform_now(**perform_args.merge(triggered_by: other_user))
 
       CommentThread.last(3).each do |thread|
         expect(thread.created_by_user).to eq(other_user)
@@ -54,7 +69,7 @@
     end
 
     it "falls back to plan author when triggered_by is nil" do
-      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id)
+      described_class.perform_now(**perform_args.except(:triggered_by))
 
       CommentThread.last(3).each do |thread|
         expect(thread.created_by_user).to eq(plan.created_by_user)
@@ -62,7 +77,7 @@
     end
 
     it "passes the formatted prompt to the AI provider" do
-      described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+      described_class.perform_now(**perform_args)
 
       expect(AiProviders::OpenAi).to have_received(:call).with(
         system_prompt: Plans::ReviewPromptFormatter.call(reviewer_prompt: reviewer.prompt_text),
@@ -75,7 +90,7 @@
       allow(AiProviders::OpenAi).to receive(:call).and_return("Plain text review with no JSON.")
 
       expect {
-        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+        described_class.perform_now(**perform_args)
       }.to change(CommentThread, :count).by(1)
 
       thread = CommentThread.last
@@ -87,21 +102,13 @@
       reviewer.update!(enabled: false)
 
       expect {
-        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
-      }.not_to change(CommentThread, :count)
-    end
-
-    it "does nothing if the plan has no current version" do
-      plan.update_columns(current_plan_version_id: nil)
-
-      expect {
-        described_class.perform_now(plan_id: plan.id, reviewer_id: reviewer.id, triggered_by: user)
+        described_class.perform_now(**perform_args)
       }.not_to change(CommentThread, :count)
     end
 
     it "enqueues on the default queue" do
       expect {
-        described_class.perform_later(plan_id: plan.id, reviewer_id: reviewer.id)
+        described_class.perform_later(**perform_args)
       }.to have_enqueued_job(described_class).on_queue("default")
     end
   end
diff --git a/spec/services/plans/trigger_automated_reviews_spec.rb b/spec/services/plans/trigger_automated_reviews_spec.rb
index c725f936..601d6256 100644
--- a/spec/services/plans/trigger_automated_reviews_spec.rb
+++ b/spec/services/plans/trigger_automated_reviews_spec.rb
@@ -22,6 +22,7 @@
       }.to have_enqueued_job(AutomatedReviewJob).with(
         plan_id: plan.id,
         reviewer_id: reviewer.id,
+        plan_version_id: plan.current_plan_version_id,
         triggered_by: user
       )
     end

From 09bde025b97ae507c9a79ff7fce8728c017d26cb Mon Sep 17 00:00:00 2001
From: Hampton Lintorn-Catlin <hampton@squareup.com>
Date: Tue, 24 Feb 2026 11:01:25 -0600
Subject: [PATCH 5/5] Guard ReviewResponseParser against non-hash items and
 blank comments

- Return nil from normalize_item for non-Hash array elements (e.g. plain strings)
- Return nil for items with blank/null comments to prevent orphaned threads
- Use filter_map to drop nil results
- Add specs for all three edge cases

Amp-Thread-ID: https://ampcode.com/threads/T-019c9093-c658-747b-b2a7-255d089449ed
Co-authored-by: Amp <amp@ampcode.com>
---
 app/services/plans/review_response_parser.rb  |  6 ++++-
 .../plans/review_response_parser_spec.rb      | 24 +++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/app/services/plans/review_response_parser.rb b/app/services/plans/review_response_parser.rb
index 808fbd06..3a7e5d11 100644
--- a/app/services/plans/review_response_parser.rb
+++ b/app/services/plans/review_response_parser.rb
@@ -11,7 +11,7 @@ def initialize(response_text, plan_content:)
 
     def call
       items = parse_json
-      items.map { |item| normalize_item(item) }
+      items.filter_map { |item| normalize_item(item) }
     end
 
     private
@@ -39,9 +39,13 @@ def extract_json_from_response
     end
 
     def normalize_item(item)
+      return nil unless item.is_a?(Hash)
+
       anchor = item["anchor_text"].presence
       comment = item["comment"].to_s.strip
 
+      return nil if comment.blank?
+
       # Verify anchor_text actually exists in the plan content
       if anchor && !@plan_content.include?(anchor)
         # Anchor doesn't match — demote to unanchored with the quote in the comment
diff --git a/spec/services/plans/review_response_parser_spec.rb b/spec/services/plans/review_response_parser_spec.rb
index f0dae7c7..2e75de12 100644
--- a/spec/services/plans/review_response_parser_spec.rb
+++ b/spec/services/plans/review_response_parser_spec.rb
@@ -72,5 +72,29 @@
 
       expect(items.first[:anchor_text]).to be_nil
     end
+
+    it "skips non-hash items in the array" do
+      response = '["Looks good", {"anchor_text": null, "comment": "Real feedback."}]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items).to eq([{ anchor_text: nil, comment: "Real feedback." }])
+    end
+
+    it "skips items with blank comments" do
+      response = '[{"anchor_text": null, "comment": ""}, {"anchor_text": null, "comment": "Valid."}]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items).to eq([{ anchor_text: nil, comment: "Valid." }])
+    end
+
+    it "skips items with null comments" do
+      response = '[{"anchor_text": null, "comment": null}]'
+
+      items = described_class.call(response, plan_content: plan_content)
+
+      expect(items).to be_empty
+    end
   end
 end