From ed9b185814f42981f2ba23a9a040d63bc9a587ab Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Thu, 29 Oct 2026 12:24:15 -0400
Subject: [PATCH 01/10] Restore removed reviewers when re-added

---
 .../coplan/review_tray_controller.js          | 10 +-
 spec/system/plan_reviews_spec.rb              | 96 +++++++++++++++++++
 2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index e818fb8b..10ee02f7 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -267,7 +267,15 @@ export default class extends Controller {
     event.preventDefault()
     if (!this.reviewerIdTarget.value) { this.assignmentErrorTarget.textContent = "Choose a person from the results first."; return }
     await this.withBusy(event.target, this.assignmentErrorTarget, async () => {
-      await this.write(this.urlValue, "POST", { review_request: { reviewer_id: this.reviewerIdTarget.value, requirement: event.target.querySelector('[type="checkbox"]').checked ? "required" : "optional" } })
+      const requirement = event.target.querySelector('[type="checkbox"]').checked ? "required" : "optional"
+      const request = await this.write(this.urlValue, "POST", { review_request: { reviewer_id: this.reviewerIdTarget.value, requirement } })
+      // Create retries preserve withdrawn tasks. Re-adding someone here is
+      // an explicit new request; reopen it through the existing update API.
+      // If that result is lost, the next create returns the pending task
+      // and does not send another request or notification.
+      if (request.status === "withdrawn") {
+        await this.write(`${this.urlValue}/${encodeURIComponent(request.id)}`, "PATCH", { review_request: { request_again: true, requirement } })
+      }
       this.searchTarget.value = ""; this.reviewerIdTarget.value = ""; this.resultsTarget.replaceChildren()
       this.closeAssignment()
       event.target.querySelector('[type="checkbox"]').checked = true
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 47fa5804..4feeaf46 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -66,6 +66,102 @@ def lose_next_review_result
     end
   end
 
+  [ "light", "dark" ].each do |theme|
+    it "re-adds a removed reviewer through the assignment pane in #{theme} theme", :review_readdition do
+      approval = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
+      sign_in_browser(author)
+      page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)
+      click_button "More actions"
+      within("#plan-menu") { click_button "Manage reviewers" }
+      within(".review-management[open]") do
+        click_button "Actions for Plan Reviewer", enable_aria_label: true
+        click_button "Remove request"
+        expect(page).to have_no_css("[data-review-assignments] .review-person")
+        click_button "Add reviewer", exact: true
+      end
+      expect(task.reload.status).to eq("withdrawn")
+      fill_in "Find a reviewer", with: reviewer.name
+      find(".review-search-result", text: reviewer.name).click
+      uncheck "Required"
+      within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+      within(".review-management[open] [data-review-assignments]") do
+        expect(page).to have_css(".review-person", text: "Plan Reviewer")
+        expect(page).to have_css(".review-person__requirement", text: "Optional")
+        expect(page).to have_css(".review-state", text: "Pending")
+      end
+      expect(task.reload).to have_attributes(status: "pending", requirement: "optional")
+      expect(plan.review_requests.where(reviewer: reviewer).count).to eq(1)
+      expect(task.review_responses.pluck(:id)).to eq([ approval.id ])
+      expect(task.notifications.unread.where(user: reviewer, reason: "review_requested").count).to eq(1)
+      page.save_screenshot(Rails.root.join("tmp/review-preview/readded-reviewer-#{theme}.png"))
+      sign_in_browser(reviewer)
+      expect(page).to have_css(".review-tray", text: "Your review is requested")
+    end
+  end
+
+  [ "lost", "rejected" ].each do |failure|
+    it "keeps re-adding a reviewer retryable after a #{failure} update result", :review_readdition do
+      CoPlan::Reviews::Withdraw.call(request: task, actor: author)
+      sign_in_browser(author)
+      click_button "More actions"
+      within("#plan-menu") { click_button "Manage reviewers" }
+      within(".review-management[open]") { click_button "Add reviewer", exact: true }
+      fill_in "Find a reviewer", with: reviewer.name
+      find(".review-search-result", text: reviewer.name).click
+      page.execute_script(<<~JS, failure)
+        const originalFetch = window.fetch.bind(window)
+        let interrupt = true
+        const failure = arguments[0]
+        window.fetch = async (url, options) => {
+          if (interrupt && options?.method === 'PATCH' && String(url).includes('/review_requests/')) {
+            interrupt = false
+            if (failure === 'rejected') return new Response(JSON.stringify({error: 'Please retry this request'}), {status: 422, headers: {'Content-Type': 'application/json'}})
+            await originalFetch(url, options)
+            throw new TypeError('Lost the result after the server reopened the task')
+          }
+          return originalFetch(url, options)
+        }
+      JS
+      within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+      expect(page).to have_css(".review-assignment [role='alert']", text: failure == "lost" ? "Couldn't reach CoPlan" : "Please retry this request")
+      expect(page).to have_field("Find a reviewer", with: reviewer.name)
+      expect(task.reload.status).to eq(failure == "lost" ? "pending" : "withdrawn")
+      within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+      within(".review-management[open] [data-review-assignments]") do
+        expect(page).to have_css(".review-person", text: reviewer.name)
+        expect(page).to have_css(".review-state", text: "Pending")
+      end
+      expect(task.reload.status).to eq("pending")
+      expect(plan.review_requests.where(reviewer: reviewer).count).to eq(1)
+      expect(plan.plan_events.where(event_type: "review_requested", after_value: task.id).count).to eq(2)
+      expect(task.notifications.unread.where(user: reviewer, reason: "review_requested").count).to eq(1)
+    end
+  end
+
+  it "leaves an already active review unchanged when a selected re-add is submitted", :review_readdition do
+    CoPlan::Reviews::Withdraw.call(request: task, actor: author)
+    sign_in_browser(author)
+    click_button "More actions"
+    within("#plan-menu") { click_button "Manage reviewers" }
+    within(".review-management[open]") { click_button "Add reviewer", exact: true }
+    fill_in "Find a reviewer", with: reviewer.name
+    find(".review-search-result", text: reviewer.name).click
+    # Another tab restores and completes this request after the person was
+    # selected. Submitting the stale add form must not reset that decision.
+    restored = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, requirement: "optional")
+    approval = CoPlan::Reviews::Respond.call(request: restored, actor: reviewer, revision: 1, decision: "approved")
+    counts = [ plan.plan_events.where(event_type: "review_requested").count, CoPlan::Notification.count ]
+    within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+    within(".review-management[open] [data-review-assignments]") do
+      expect(page).to have_css(".review-person", text: reviewer.name)
+      expect(page).to have_css(".review-state", text: "Approved")
+      expect(page).to have_css(".review-person__requirement", text: "Optional")
+    end
+    expect(task.reload).to have_attributes(status: "approved", requirement: "optional")
+    expect(task.review_responses.pluck(:id)).to eq([ approval.id ])
+    expect([ plan.plan_events.where(event_type: "review_requested").count, CoPlan::Notification.count ]).to eq(counts)
+  end
+
   it "offers equal review choices and preserves cancelled feedback without moving the reader" do
     within(".review-tray__actions") do
       expect(page).to have_button("Request changes")

From 729a7c6e031889d151b8d21c5f8d0d1200fd1416 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Thu, 29 Oct 2026 12:42:19 -0400
Subject: [PATCH 02/10] Start fresh after removing a reviewer

---
 .../api/v1/review_requests_controller.rb      |   4 +-
 .../coplan/review_requests_controller.rb      |  13 +-
 .../coplan/review_tray_controller.js          |   2 +-
 engine/app/models/coplan/plan_event.rb        |   2 +-
 engine/app/services/coplan/reviews/changed.rb |  10 +-
 engine/app/services/coplan/reviews/remove.rb  |  34 ++++++
 engine/app/services/coplan/reviews/request.rb |  13 +-
 .../agent_instructions/guides/api.text.erb    |   2 +
 .../guides/comments.text.erb                  |   1 +
 .../agent_instructions/guides/live.text.erb   |   1 +
 .../views/coplan/plans/_event_item.html.erb   |   2 +-
 .../app/views/coplan/reviews/_roster.html.erb |   2 +-
 spec/requests/api/v1/review_requests_spec.rb  |  31 +++++
 spec/requests/review_requests_spec.rb         |  40 +++++++
 spec/services/coplan/reviews/remove_spec.rb   | 112 ++++++++++++++++++
 spec/system/plan_reviews_spec.rb              |  71 ++++++++---
 16 files changed, 307 insertions(+), 33 deletions(-)
 create mode 100644 engine/app/services/coplan/reviews/remove.rb
 create mode 100644 spec/services/coplan/reviews/remove_spec.rb

diff --git a/engine/app/controllers/coplan/api/v1/review_requests_controller.rb b/engine/app/controllers/coplan/api/v1/review_requests_controller.rb
index c9b9d4da..35b75c4f 100644
--- a/engine/app/controllers/coplan/api/v1/review_requests_controller.rb
+++ b/engine/app/controllers/coplan/api/v1/review_requests_controller.rb
@@ -35,9 +35,9 @@ def create
         def update
           if ActiveModel::Type::Boolean.new.cast(params[:request_again])
             Reviews::Request.call(plan: @plan, reviewer: @review_request.reviewer, actor: current_user,
-              requirement: params[:requirement].presence || @review_request.requirement, api_token: @api_token)
+              requirement: params[:requirement].presence || @review_request.requirement, api_token: @api_token, request_id: @review_request.id)
           else
-            @plan.with_lock { @review_request.update!(requirement: params.require(:requirement)) }
+            @plan.with_lock { @review_request.reload.update!(requirement: params.require(:requirement)) }
             Reviews::Changed.call(request: @review_request, event_type: "review.requested", actor_token_id: api_token_id,
               payload: { "action" => "requirement_changed", "requirement" => @review_request.requirement })
           end
diff --git a/engine/app/controllers/coplan/review_requests_controller.rb b/engine/app/controllers/coplan/review_requests_controller.rb
index f62be26d..52733b65 100644
--- a/engine/app/controllers/coplan/review_requests_controller.rb
+++ b/engine/app/controllers/coplan/review_requests_controller.rb
@@ -1,7 +1,7 @@
 module CoPlan
   class ReviewRequestsController < ApplicationController
     before_action :set_plan
-    before_action :set_review_request, only: [ :update, :destroy, :respond, :compare ]
+    before_action :set_review_request, only: [ :update, :respond, :compare ]
     rescue_from Reviews::Respond::Conflict, with: :render_conflict
     rescue_from ActiveRecord::RecordInvalid, with: :render_invalid
 
@@ -29,9 +29,9 @@ def update
       attributes = params.expect(review_request: [ :requirement, :request_again ])
       if ActiveModel::Type::Boolean.new.cast(attributes[:request_again])
         Reviews::Request.call(plan: @plan, reviewer: @review_request.reviewer, actor: current_user,
-          requirement: attributes[:requirement].presence || @review_request.requirement)
+          requirement: attributes[:requirement].presence || @review_request.requirement, request_id: @review_request.id)
       else
-        @plan.with_lock { @review_request.update!(requirement: attributes.fetch(:requirement)) }
+        @plan.with_lock { @review_request.reload.update!(requirement: attributes.fetch(:requirement)) }
         Reviews::Changed.call(request: @review_request, event_type: "review.requested",
           payload: { "action" => "requirement_changed", "requirement" => @review_request.requirement })
       end
@@ -40,7 +40,12 @@ def update
 
     def destroy
       authorize!(@plan, :update?)
-      Reviews::Withdraw.call(request: @review_request, actor: current_user)
+      if ActiveModel::Type::Boolean.new.cast(params.dig(:review_request, :remove))
+        Reviews::Remove.call(plan: @plan, request_id: params[:id])
+      else
+        set_review_request
+        Reviews::Withdraw.call(request: @review_request, actor: current_user)
+      end
       head :no_content
     end
 
diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index 10ee02f7..b4c0d9fe 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -362,7 +362,7 @@ export default class extends Controller {
 
   requestAgain(event) { this.manage(event, "PATCH", { review_request: { request_again: true } }) }
   changeRequirement(event) { this.manage(event, "PATCH", { review_request: { requirement: event.currentTarget.dataset.requirement } }) }
-  withdraw(event) { this.manage(event, "DELETE") }
+  withdraw(event) { this.manage(event, "DELETE", { review_request: { remove: true } }) }
 
   async manage(event, method, body) {
     if (this.managing) return
diff --git a/engine/app/models/coplan/plan_event.rb b/engine/app/models/coplan/plan_event.rb
index da2e3e1e..f728a09f 100644
--- a/engine/app/models/coplan/plan_event.rb
+++ b/engine/app/models/coplan/plan_event.rb
@@ -9,7 +9,7 @@ module CoPlan
   # metadata-only changes).
   #
   # Records are append-only — never updated, never destroyed except through
-  # the parent plan's cascade.
+  # the parent plan's cascade or explicit removal of a reviewer's history.
   class PlanEvent < ApplicationRecord
     # Mirrors PlanVersion::ACTOR_TYPES so history items from either source can
     # be rendered uniformly.
diff --git a/engine/app/services/coplan/reviews/changed.rb b/engine/app/services/coplan/reviews/changed.rb
index 14d76fe7..089903dd 100644
--- a/engine/app/services/coplan/reviews/changed.rb
+++ b/engine/app/services/coplan/reviews/changed.rb
@@ -11,8 +11,14 @@ def initialize(request:, event_type:, actor_token_id:, payload:)
 
       def call
         request, event_type, actor_token_id, payload = @request, @event_type, @actor_token_id, @payload
-        AgentEvents::Publish.call(plan: request.plan, event_type: event_type, actor_token_id: actor_token_id,
-          payload: { "review_request_id" => request.id, "reviewer_id" => request.reviewer_id }.merge(payload))
+        request.plan.with_lock do
+          # A response can commit just before removal, then reach this fan-out
+          # afterward. Do not put an erased decision back in an agent's inbox.
+          if payload["action"] == "removed" || request.plan.review_requests.exists?(request.id)
+            AgentEvents::Publish.call(plan: request.plan, event_type: event_type, actor_token_id: actor_token_id,
+              payload: { "review_request_id" => request.id, "reviewer_id" => request.reviewer_id }.merge(payload))
+          end
+        end
         Notifications::BroadcastBadges.call(user_ids: [ request.reviewer_id, request.plan.created_by_user_id ])
         # No viewer-relative HTML or CSRF tokens on a shared stream. Each
         # open tab fetches its own controls, preserving its current draft.
diff --git a/engine/app/services/coplan/reviews/remove.rb b/engine/app/services/coplan/reviews/remove.rb
new file mode 100644
index 00000000..83d6bef3
--- /dev/null
+++ b/engine/app/services/coplan/reviews/remove.rb
@@ -0,0 +1,34 @@
+module CoPlan
+  module Reviews
+    # Removing a reviewer ends the request's identity. A later assignment gets
+    # a new ID, so delayed responses to the old request cannot complete it.
+    # API withdrawal is separate and continues to preserve review history.
+    class Remove
+      def self.call(plan:, request_id:)
+        removed_events = []
+        request = plan.with_lock do
+          request = plan.review_requests.find_by(id: request_id)
+          next unless request
+
+          events = plan.plan_events.where(event_type: %w[review_requested review_withdrawn], after_value: request.id)
+          removed_events = events.to_a
+          events.delete_all
+          event_ids = AgentEvent.where(plan: plan, event_type: %w[review.requested review.responded]).pluck(:id, :payload).filter_map do |id, payload|
+            id if payload.to_h["review_request_id"] == request.id
+          end
+          AgentEvent.where(id: event_ids).delete_all
+          request.destroy!
+          request
+        end
+        if request
+          removed_events.each { |event| Broadcaster.remove_to(plan, target: ActionView::RecordIdentifier.dom_id(event)) }
+          count = plan.plan_versions.count + plan.plan_events.count
+          Broadcaster.replace_to(plan, target: "history-count",
+            html: ApplicationController.helpers.content_tag(:span, ApplicationController.helpers.pluralize(count, "entry"), class: "section-count", id: "history-count"))
+          Changed.call(request: request, event_type: "review.requested", payload: { "action" => "removed", "removed" => true })
+        end
+        request
+      end
+    end
+  end
+end
diff --git a/engine/app/services/coplan/reviews/request.rb b/engine/app/services/coplan/reviews/request.rb
index 5e646a28..9e5b971d 100644
--- a/engine/app/services/coplan/reviews/request.rb
+++ b/engine/app/services/coplan/reviews/request.rb
@@ -1,19 +1,24 @@
 module CoPlan
   module Reviews
     class Request
-      def self.call(plan:, reviewer:, actor:, requirement: "required", api_token: nil, create_only: false)
-        new(plan:, reviewer:, actor:, requirement:, api_token:, create_only:).call
+      def self.call(plan:, reviewer:, actor:, requirement: "required", api_token: nil, create_only: false, request_id: nil)
+        new(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:).call
       end
 
-      def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:)
+      def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:)
         @plan, @reviewer, @actor, @requirement, @api_token = plan, reviewer, actor, requirement, api_token
         @create_only = create_only
+        @request_id = request_id
       end
 
       def call
         changed = false
         request = @plan.with_lock do
-          request = @plan.review_requests.find_or_initialize_by(reviewer: @reviewer)
+          request = if @request_id
+            @plan.review_requests.where(reviewer: @reviewer).find(@request_id)
+          else
+            @plan.review_requests.find_or_initialize_by(reviewer: @reviewer)
+          end
           # Retrying a create cannot change a decision, reopen a withdrawal,
           # or send another nudge. Only an explicit re-request starts a cycle.
           if @create_only && request.persisted?
diff --git a/engine/app/views/coplan/agent_instructions/guides/api.text.erb b/engine/app/views/coplan/agent_instructions/guides/api.text.erb
index d8087c87..24d86ef6 100644
--- a/engine/app/views/coplan/agent_instructions/guides/api.text.erb
+++ b/engine/app/views/coplan/agent_instructions/guides/api.text.erb
@@ -82,6 +82,8 @@ return 429. Invalid names return 422 with `invalid_request`.
 
 Creating a review request for an existing reviewer returns the current task with status 201. It leaves the requirement, decision, history, and notifications unchanged. Use `PATCH` to change the requirement or `request_again: true` to request another look.
 
+Removing a reviewer in the web management pane deletes their request and review decisions. Re-adding them creates a fresh pending request with a new ID. The removed request ID returns 404, including for old response retries. API withdrawal remains separate and preserves history.
+
 Include `idempotency_key` (a fresh UUID) for each intended review response. Reuse that UUID and identical fields when retrying a lost response; its returned `id` is the UUID. Use a new UUID for a new decision, including another review of the same version. A keyed retry returns the original recorded result even after the plan changes or the task is withdrawn. It preserves any newly pending review task. Without a key, an identical response is treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. A key reused for different fields returns 422.
 
 A review response returns 409 with `code: review_version_changed` for a stale revision. Re-read before responding. A new response to a withdrawn request returns 409 with `code: review_request_withdrawn`; stop responding unless the author requests another look.
diff --git a/engine/app/views/coplan/agent_instructions/guides/comments.text.erb b/engine/app/views/coplan/agent_instructions/guides/comments.text.erb
index 8ae64f43..1a255732 100644
--- a/engine/app/views/coplan/agent_instructions/guides/comments.text.erb
+++ b/engine/app/views/coplan/agent_instructions/guides/comments.text.erb
@@ -87,4 +87,5 @@ Discussion is the default. An approval means “satisfied with this exact versio
 - The plan author's credential may `GET /api/v1/plans/:plan_id/review_requests/suggestions` for suggested reviewers. Each entry in `suggestions` has `reviewer` (`id`, `name`, `username`), a `reason`, and a suggested `requirement`. A deployment may base them on the files and repositories the plan links to, so link the files a plan will change before asking. The list leaves out the author and people already reviewing, and is empty when the deployment suggests nobody. Suggestions are not requests. When your principal wants the plan reviewed, `POST` each chosen `reviewer.id` with its `requirement`.
 - `PATCH /api/v1/plans/:plan_id/review_requests/:id` accepts `requirement` without reopening the task or starting another review cycle. Use `{ "request_again": true }` when feedback is ready for another look. That preserves earlier responses and notifies the reviewer again. Live events distinguish these actions; see the live guide.
 - `DELETE` the same path withdraws the request and records the withdrawing actor and time in review activity. Earlier request cycles and decisions remain. It clears the reviewer's request nudge, not the author's unread responses. Archiving a plan does not withdraw pending reviews.
+- Removing a reviewer in the web management pane is different from API withdrawal: it deletes their request and past review decisions. Adding them again creates a new pending request with a new ID. Requests to the removed ID return 404; stop using that ID and fetch the current requests. Never retry an old response against a replacement request.
 - Only the requested reviewer's credential may `POST /api/v1/plans/:plan_id/review_requests/:id/respond` with `{ "revision": 4, "decision": "approved", "note": "optional public note", "idempotency_key": "RESPONSE_UUID" }` or `{ "revision": 4, "decision": "changes_requested", "note": "what needs adjustment", "idempotency_key": "RESPONSE_UUID" }`. Change requests create ordinary public comment threads. Agent responses are explicitly attributed to the token's agent acting for its human owner. A stale revision returns 409 with `code: review_version_changed`; re-read before responding. A new response to a withdrawn request returns 409 with `code: review_request_withdrawn`; stop responding unless the author requests another look. Replace `RESPONSE_UUID` with a fresh UUID for each intended response. Retry with the same UUID and identical fields if the HTTP response is lost. Use a new UUID for another review, even for the same version and decision. A keyed retry returns the original result even after a plan edit or withdrawal. It does not complete a newly re-requested task. Without a key, identical fields are treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. Reusing a key with different fields returns 422. Never manufacture a human sign-off.
diff --git a/engine/app/views/coplan/agent_instructions/guides/live.text.erb b/engine/app/views/coplan/agent_instructions/guides/live.text.erb
index c682ed2c..7761d69c 100644
--- a/engine/app/views/coplan/agent_instructions/guides/live.text.erb
+++ b/engine/app/views/coplan/agent_instructions/guides/live.text.erb
@@ -105,6 +105,7 @@ For `review.requested`, inspect `payload.action`:
 | `requested` | A new request or another look. `requirement` is `required` or `optional`. |
 | `requirement_changed` | Only `requirement` changed. This does not reopen the task or start another review cycle. Do not treat it as a request for another look. |
 | `withdrawn` | The request was withdrawn. Stop working on that request; its earlier decisions remain. The payload also has `withdrawn: true`. |
+| `removed` | The web management pane deleted this request and its review decisions. Stop using its ID. Reassignment creates a new pending request. The payload also has `removed: true`. |
 
 The attach and bridge tools show this review data and point to the review guide. Review events do not authorize content edits or execution.
 
diff --git a/engine/app/views/coplan/plans/_event_item.html.erb b/engine/app/views/coplan/plans/_event_item.html.erb
index a4f75cfe..847e4a73 100644
--- a/engine/app/views/coplan/plans/_event_item.html.erb
+++ b/engine/app/views/coplan/plans/_event_item.html.erb
@@ -2,7 +2,7 @@
     _version_item.html.erb. Non-interactive (no turbo-frame target) since
     there's no separate diff to display — the event payload itself is the
     diff. %>
-<div class="history-split__item history-split__item--event">
+<div class="history-split__item history-split__item--event" id="<%= dom_id(event) %>">
   <div class="history-split__event-chip"><%= event.field || event.event_type %></div>
   <div class="history-split__meta">
     <% if event.actor_user %>
diff --git a/engine/app/views/coplan/reviews/_roster.html.erb b/engine/app/views/coplan/reviews/_roster.html.erb
index 2092a34b..898fbca8 100644
--- a/engine/app/views/coplan/reviews/_roster.html.erb
+++ b/engine/app/views/coplan/reviews/_roster.html.erb
@@ -21,7 +21,7 @@
                   <button type="button" class="menu__item" data-action="coplan--review-tray#changeRequirement coplan--menu#close" data-url="<%= plan_review_request_path(plan, request) %>" data-requirement="<%= requirement == 'required' ? 'optional' : 'required' %>">Make <%= requirement == 'required' ? 'optional' : 'required' %></button>
                   <% if approval && approval.plan_version.revision != plan.current_revision %><button type="button" class="menu__item" data-action="coplan--review-tray#compare coplan--menu#close" data-url="<%= compare_plan_review_request_path(plan, request, response_id: approval.id) %>">Changes since approval</button><% end %>
                   <div class="menu__separator" role="separator"></div>
-                  <button type="button" class="menu__item" data-action="coplan--review-tray#withdraw coplan--menu#close" data-url="<%= plan_review_request_path(plan, request) %>">Remove request</button>
+                  <button type="button" class="menu__item" data-action="coplan--review-tray#withdraw coplan--menu#close" data-url="<%= plan_review_request_path(plan, request) %>">Remove reviewer</button>
                 </div>
               </span>
             <% elsif approval && approval.plan_version.revision != plan.current_revision %>
diff --git a/spec/requests/api/v1/review_requests_spec.rb b/spec/requests/api/v1/review_requests_spec.rb
index 3e0a02aa..595bb8b1 100644
--- a/spec/requests/api/v1/review_requests_spec.rb
+++ b/spec/requests/api/v1/review_requests_spec.rb
@@ -14,6 +14,37 @@ def headers(token)
 
   before { author_token; reviewer_token }
 
+  [ true, false ].each do |keyed|
+    it "cannot replay a removed #{keyed ? 'keyed' : 'legacy'} review into a fresh request", :review_removal do
+      payload = { revision: 1, decision: "changes_requested", note: "Earlier feedback" }
+      payload[:idempotency_key] = SecureRandom.uuid if keyed
+      post respond_api_v1_plan_review_request_path(plan, task), params: payload, headers: headers("reviews-reviewer"), as: :json
+      expect(response).to have_http_status(:created)
+      old_response_id = response.parsed_body.fetch("id")
+      sign_in_as(author)
+      delete plan_review_request_path(plan, task), params: { review_request: { remove: true } }, as: :json
+      expect(response).to have_http_status(:no_content)
+      post api_v1_plan_review_requests_path(plan), params: { reviewer_id: reviewer.id }, headers: headers("reviews-owner"), as: :json
+      expect(response).to have_http_status(:created)
+      fresh = plan.review_requests.find(response.parsed_body.fetch("id"))
+      expect(fresh.id).not_to eq(task.id)
+      expect(response.parsed_body.fetch("responses")).to be_empty
+
+      post respond_api_v1_plan_review_request_path(plan, task), params: payload, headers: headers("reviews-reviewer"), as: :json
+      expect(response).to have_http_status(:not_found)
+      patch api_v1_plan_review_request_path(plan, task), params: { request_again: true }, headers: headers("reviews-owner"), as: :json
+      expect(response).to have_http_status(:not_found)
+      delete api_v1_plan_review_request_path(plan, task), headers: headers("reviews-owner")
+      expect(response).to have_http_status(:not_found)
+      expect(CoPlan::ReviewResponse.exists?(old_response_id)).to be(false)
+      expect(fresh.reload.status).to eq("pending")
+      get api_v1_plan_review_requests_path(plan), headers: headers("reviews-owner")
+      requests = response.parsed_body.fetch("review_requests")
+      expect(requests.map { |request| request.fetch("id") }).to eq([ fresh.id ])
+      expect(requests.first.fetch("responses")).to be_empty
+    end
+  end
+
   [ true, false ].each do |keyed|
     it "replays a #{keyed ? 'keyed' : 'legacy unkeyed'} response through a renewed session token without changing its attribution", :review_retries do
       first_token, first_secret = reviewer_token.mint_session_token!(agent_name: "Originalbot")
diff --git a/spec/requests/review_requests_spec.rb b/spec/requests/review_requests_spec.rb
index 4a4c9baa..62c88cab 100644
--- a/spec/requests/review_requests_spec.rb
+++ b/spec/requests/review_requests_spec.rb
@@ -6,6 +6,46 @@
   let(:plan) { create(:plan, :published, created_by_user: author) }
   let(:task) { create(:review_request, plan: plan, reviewer: reviewer) }
 
+  [ true, false ].each do |keyed|
+    it "rejects old #{keyed ? 'keyed' : 'legacy'} responses after removing and re-adding a reviewer", :review_removal do
+      payload = { revision: 1, decision: "approved" }
+      payload[:idempotency_key] = SecureRandom.uuid if keyed
+      previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, **payload)
+      sign_in_as(author)
+      delete plan_review_request_path(plan, task), params: { review_request: { remove: true } }, as: :json
+      expect(response).to have_http_status(:no_content)
+      expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
+      post plan_review_requests_path(plan), params: { review_request: { reviewer_id: reviewer.id } }, as: :json
+      expect(response).to have_http_status(:created)
+      fresh = plan.review_requests.find(response.parsed_body.fetch("id"))
+      expect(fresh.id).not_to eq(task.id)
+
+      sign_in_as(reviewer)
+      post respond_plan_review_request_path(plan, task), params: { review_response: payload }, as: :json
+      expect(response).to have_http_status(:not_found)
+      sign_in_as(author)
+      patch plan_review_request_path(plan, task), params: { review_request: { request_again: true } }, as: :json
+      expect(response).to have_http_status(:not_found)
+      delete plan_review_request_path(plan, task), params: { review_request: { remove: true } }, as: :json
+      expect(response).to have_http_status(:no_content)
+      expect(fresh.reload.status).to eq("pending")
+      expect(fresh.review_responses).to be_empty
+      get plan_review_requests_path(plan)
+      expect(Nokogiri::HTML(response.body).css(".review-log__event").map { |node| node.text.squish }).to eq([
+        "#{author.name} requested a review from #{reviewer.name}"
+      ])
+    end
+  end
+
+  it "allows only the plan author to erase a reviewer's decisions", :review_removal do
+    previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
+    sign_in_as(reviewer)
+    delete plan_review_request_path(plan, task), params: { review_request: { remove: true } }, as: :json
+    expect(response).to have_http_status(:not_found)
+    expect(previous.reload.decision).to eq("approved")
+    expect(task.reload.status).to eq("approved")
+  end
+
   it "marks an unread note-less review decision as attention until its owner reads it", :review_attention do
     CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: plan.current_revision, decision: "approved")
     notice = author.notifications.find_by!(review_request: task, reason: "review_response")
diff --git a/spec/services/coplan/reviews/remove_spec.rb b/spec/services/coplan/reviews/remove_spec.rb
new file mode 100644
index 00000000..ced77aee
--- /dev/null
+++ b/spec/services/coplan/reviews/remove_spec.rb
@@ -0,0 +1,112 @@
+require "rails_helper"
+
+RSpec.describe CoPlan::Reviews::Remove do
+  let(:plan) { create(:plan, :published) }
+  let(:reviewer) { create(:coplan_user) }
+  let(:task) { CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user) }
+
+  %w[approved changes_requested].each do |decision|
+    it "erases a removed reviewer's #{decision} decisions and starts a new pending request" do
+      watcher = create(:api_token, user: plan.created_by_user)
+      CoPlan::AgentSession.create!(plan: plan, api_token: watcher, state: "watching", agent_name: "Authorbot")
+      previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1,
+        decision: decision, note: decision == "changes_requested" ? "Earlier feedback" : nil)
+      other = CoPlan::Reviews::Request.call(plan: plan, reviewer: create(:coplan_user), actor: plan.created_by_user)
+      other_response = CoPlan::Reviews::Respond.call(request: other, actor: other.reviewer, revision: 1, decision: "approved")
+
+      described_class.call(plan: plan, request_id: task.id)
+
+      expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
+      expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
+      expect(plan.notifications.where(review_request_id: task.id)).to be_empty
+      expect(plan.plan_events.where(after_value: task.id)).to be_empty
+      expect(watcher.agent_events.where(event_type: "review.responded").map { |event| event.payload["review_request_id"] }).to eq([ other.id ])
+      expect(watcher.agent_events.last.payload).to include("action" => "removed", "review_request_id" => task.id)
+      expect(other_response.reload.decision).to eq("approved")
+
+      fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user, create_only: true)
+      expect(fresh.id).not_to eq(task.id)
+      expect(fresh.status).to eq("pending")
+      expect(fresh.review_responses).to be_empty
+      expect(fresh.latest_approval).to be_nil
+    end
+  end
+
+  it "does not remove or reset the replacement when an old removal is retried" do
+    old_id = task.id
+    described_class.call(plan: plan, request_id: old_id)
+    fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user, create_only: true)
+    approval = CoPlan::Reviews::Respond.call(request: fresh, actor: reviewer, revision: 1, decision: "approved")
+    expect { described_class.call(plan: plan, request_id: old_id) }.not_to change(CoPlan::Notification, :count)
+    expect(fresh.reload.status).to eq("approved")
+    expect(fresh.review_responses).to eq([ approval ])
+  end
+
+  it "rejects an old re-request already in flight when the request is removed" do
+    old_id = task.id
+    described_class.call(plan: plan, request_id: old_id)
+    expect {
+      CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user, request_id: old_id)
+    }.to raise_error(ActiveRecord::RecordNotFound)
+    expect(plan.review_requests).to be_empty
+  end
+
+  it "does not erase a request from another plan" do
+    task
+    expect(described_class.call(plan: create(:plan, :published), request_id: task.id)).to be_nil
+    expect(task.reload.status).to eq("pending")
+  end
+
+  it "does not republish an erased response when its post-commit fan-out runs late" do
+    watcher = create(:api_token, user: plan.created_by_user)
+    CoPlan::AgentSession.create!(plan: plan, api_token: watcher, state: "watching", agent_name: "Authorbot")
+    stale_request = task
+    described_class.call(plan: plan, request_id: task.id)
+    CoPlan::Reviews::Changed.call(request: stale_request, event_type: "review.responded", payload: { "decision" => "approved" })
+    expect(watcher.agent_events.where(event_type: "review.responded")).to be_empty
+  end
+
+  describe "requests already waiting for the plan lock" do
+    self.use_transactional_tests = false
+    after { truncate_tables(*(ActiveRecord::Base.connection.tables - %w[schema_migrations ar_internal_metadata])) }
+
+    %i[respond request_again].each do |action|
+      it "rejects an old #{action} after removal and reassignment commit" do
+        old_id = task.id
+        reviewer_id = reviewer.id
+        ready = Queue.new
+        worker = nil
+        fresh = nil
+        plan.with_lock do
+          worker = Thread.new do
+            ActiveRecord::Base.connection_pool.with_connection do
+              stale = CoPlan::ReviewRequest.find(old_id)
+              ready << true
+              begin
+                if action == :respond
+                  CoPlan::Reviews::Respond.call(request: stale, actor: stale.reviewer, revision: 1, decision: "approved")
+                else
+                  CoPlan::Reviews::Request.call(plan: stale.plan, reviewer: stale.reviewer,
+                    actor: stale.requested_by_user, request_id: old_id)
+                end
+              rescue ActiveRecord::RecordNotFound => error
+                error
+              end
+            end
+          end
+          Timeout.timeout(10) { ready.pop }
+          described_class.call(plan: plan, request_id: old_id)
+          fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: CoPlan::User.find(reviewer_id),
+            actor: plan.created_by_user, create_only: true)
+        end
+        expect(worker.join(10)).to be_present
+        expect(worker.value).to be_a(ActiveRecord::RecordNotFound)
+        expect(fresh.reload.status).to eq("pending")
+        expect(fresh.review_responses).to be_empty
+        expect(plan.review_requests.pluck(:id)).to eq([ fresh.id ])
+      ensure
+        worker&.kill if worker&.alive?
+      end
+    end
+  end
+end
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 4feeaf46..8c296630 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -40,15 +40,9 @@ def lose_next_review_result
 
   before { sign_in_browser(reviewer, open_sections: !RSpec.current_example.metadata[:collapsed_support]) }
 
-  it "keeps a withdrawn request between its two request cycles", :review_withdrawals do
+  it "keeps an API-withdrawn request between its two request cycles", :review_withdrawals do
     sign_in_browser(author)
-    click_button "More actions"
-    within("#plan-menu") { click_button "Manage reviewers" }
-    within(".review-management[open]") do
-      click_button "Actions for Plan Reviewer", enable_aria_label: true
-      click_button "Remove request"
-      expect(page).to have_no_css("[data-review-assignments] .review-person")
-    end
+    CoPlan::Reviews::Withdraw.call(request: task, actor: author)
     expect(task.reload.status).to eq("withdrawn")
     CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author)
     visit plan_page_path(plan)
@@ -66,20 +60,22 @@ def lose_next_review_result
     end
   end
 
-  [ "light", "dark" ].each do |theme|
-    it "re-adds a removed reviewer through the assignment pane in #{theme} theme", :review_readdition do
-      approval = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
+  [ "light", "dark" ].product(%w[approved changes_requested]).each do |theme, decision|
+    it "starts fresh after removing a #{decision} reviewer in #{theme} theme", :review_readdition do
+      previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: decision,
+        note: decision == "changes_requested" ? "Earlier review feedback" : nil)
       sign_in_browser(author)
       page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)
       click_button "More actions"
       within("#plan-menu") { click_button "Manage reviewers" }
       within(".review-management[open]") do
         click_button "Actions for Plan Reviewer", enable_aria_label: true
-        click_button "Remove request"
+        click_button "Remove reviewer"
         expect(page).to have_no_css("[data-review-assignments] .review-person")
         click_button "Add reviewer", exact: true
       end
-      expect(task.reload.status).to eq("withdrawn")
+      expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
+      expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
       fill_in "Find a reviewer", with: reviewer.name
       find(".review-search-result", text: reviewer.name).click
       uncheck "Required"
@@ -89,17 +85,58 @@ def lose_next_review_result
         expect(page).to have_css(".review-person__requirement", text: "Optional")
         expect(page).to have_css(".review-state", text: "Pending")
       end
-      expect(task.reload).to have_attributes(status: "pending", requirement: "optional")
+      fresh = plan.review_requests.find_by!(reviewer: reviewer)
+      expect(fresh.id).not_to eq(task.id)
+      expect(fresh).to have_attributes(status: "pending", requirement: "optional")
       expect(plan.review_requests.where(reviewer: reviewer).count).to eq(1)
-      expect(task.review_responses.pluck(:id)).to eq([ approval.id ])
-      expect(task.notifications.unread.where(user: reviewer, reason: "review_requested").count).to eq(1)
+      expect(fresh.review_responses).to be_empty
+      expect(fresh.notifications.unread.where(user: reviewer, reason: "review_requested").count).to eq(1)
       page.save_screenshot(Rails.root.join("tmp/review-preview/readded-reviewer-#{theme}.png"))
+      within(".review-management[open]") { click_button "Close reviews", enable_aria_label: true }
+      within("#footnote-approvals") { click_button "View activity", exact: false }
+      within(".review-activity-dialog[open]") do
+        expect(all(".review-log__event").map { |entry| entry.text.squish }).to eq([ "Plan Author requested a review from Plan Reviewer" ])
+        expect(page).to have_no_text("Earlier review feedback")
+      end
+      page.save_screenshot(Rails.root.join("tmp/review-preview/fresh-review-history-#{theme}.png"))
       sign_in_browser(reviewer)
       expect(page).to have_css(".review-tray", text: "Your review is requested")
     end
   end
 
   [ "lost", "rejected" ].each do |failure|
+    it "keeps removal retryable after a #{failure} result", :review_readdition do
+      sign_in_browser(author)
+      click_button "More actions"
+      within("#plan-menu") { click_button "Manage reviewers" }
+      page.execute_script(<<~JS, failure)
+        const originalFetch = window.fetch.bind(window)
+        const failure = arguments[0]
+        // Also lose the socket invalidation so the stale row can be retried.
+        window.Turbo.StreamActions['coplan-review-changed'] = function () {}
+        let interrupt = true
+        window.fetch = async (url, options) => {
+          if (interrupt && options?.method === 'DELETE' && String(url).includes('/review_requests/')) {
+            interrupt = false
+            if (failure === 'rejected') return new Response(JSON.stringify({error: 'Please retry removal'}), {status: 422, headers: {'Content-Type': 'application/json'}})
+            await originalFetch(url, options)
+            throw new TypeError('Lost the successful removal result')
+          }
+          return originalFetch(url, options)
+        }
+      JS
+      within(".review-management[open]") do
+        click_button "Actions for Plan Reviewer", enable_aria_label: true
+        click_button "Remove reviewer"
+        expect(page).to have_css("[role='alert']", text: failure == "lost" ? "Couldn't reach CoPlan" : "Please retry removal")
+        expect(CoPlan::ReviewRequest.exists?(task.id)).to eq(failure == "rejected")
+        click_button "Actions for Plan Reviewer", enable_aria_label: true
+        click_button "Remove reviewer"
+        expect(page).to have_no_css("[data-review-assignments] .review-person")
+      end
+      expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
+    end
+
     it "keeps re-adding a reviewer retryable after a #{failure} update result", :review_readdition do
       CoPlan::Reviews::Withdraw.call(request: task, actor: author)
       sign_in_browser(author)
@@ -188,7 +225,7 @@ def lose_next_review_result
     expect(page).to have_field("Public review comment", with: "Can we clarify this boundary?", enable_aria_label: true)
   end
 
-  [ [ "Request another review", false ], [ "Remove request", true ] ].each do |action, failed|
+  [ [ "Request another review", false ], [ "Remove reviewer", true ] ].each do |action, failed|
     it "keeps reviewer-update progress visible for #{action}", :review_interactions do
       sign_in_browser(author)
       click_button "More actions"

From d46388f0f73514d1f038568dbce0054c4524cb36 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Thu, 29 Oct 2026 12:53:28 -0400
Subject: [PATCH 03/10] Limit resets to explicit reviewer reassignment

---
 .../coplan/review_requests_controller.rb      |  5 +-
 .../coplan/review_tray_controller.js          | 10 +--
 engine/app/models/coplan/plan_event.rb        |  2 +-
 engine/app/services/coplan/reviews/changed.rb | 10 +--
 engine/app/services/coplan/reviews/remove.rb  | 25 +-----
 engine/app/services/coplan/reviews/request.rb | 16 ++--
 .../agent_instructions/guides/api.text.erb    |  2 -
 .../views/coplan/plans/_event_item.html.erb   |  2 +-
 spec/requests/review_requests_spec.rb         | 31 -------
 spec/services/coplan/reviews/remove_spec.rb   | 34 ++------
 spec/system/plan_reviews_spec.rb              | 87 ++++---------------
 11 files changed, 47 insertions(+), 177 deletions(-)

diff --git a/engine/app/controllers/coplan/review_requests_controller.rb b/engine/app/controllers/coplan/review_requests_controller.rb
index 52733b65..4417b0f4 100644
--- a/engine/app/controllers/coplan/review_requests_controller.rb
+++ b/engine/app/controllers/coplan/review_requests_controller.rb
@@ -17,10 +17,11 @@ def suggestions
 
     def create
       authorize!(@plan, :update?)
-      attributes = params.expect(review_request: [ :reviewer_id, :requirement ])
+      attributes = params.expect(review_request: [ :reviewer_id, :requirement, :replace_withdrawn ])
       reviewer = User.find(attributes[:reviewer_id])
       request = Reviews::Request.call(plan: @plan, reviewer: reviewer, actor: current_user,
-        requirement: attributes[:requirement].presence || "required", create_only: true)
+        requirement: attributes[:requirement].presence || "required", create_only: true,
+        replace_withdrawn: ActiveModel::Type::Boolean.new.cast(attributes[:replace_withdrawn]))
       render json: Reviews::Serialize.call(request), status: :created
     end
 
diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index b4c0d9fe..4adabcdf 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -267,15 +267,7 @@ export default class extends Controller {
     event.preventDefault()
     if (!this.reviewerIdTarget.value) { this.assignmentErrorTarget.textContent = "Choose a person from the results first."; return }
     await this.withBusy(event.target, this.assignmentErrorTarget, async () => {
-      const requirement = event.target.querySelector('[type="checkbox"]').checked ? "required" : "optional"
-      const request = await this.write(this.urlValue, "POST", { review_request: { reviewer_id: this.reviewerIdTarget.value, requirement } })
-      // Create retries preserve withdrawn tasks. Re-adding someone here is
-      // an explicit new request; reopen it through the existing update API.
-      // If that result is lost, the next create returns the pending task
-      // and does not send another request or notification.
-      if (request.status === "withdrawn") {
-        await this.write(`${this.urlValue}/${encodeURIComponent(request.id)}`, "PATCH", { review_request: { request_again: true, requirement } })
-      }
+      await this.write(this.urlValue, "POST", { review_request: { reviewer_id: this.reviewerIdTarget.value, requirement: event.target.querySelector('[type="checkbox"]').checked ? "required" : "optional", replace_withdrawn: true } })
       this.searchTarget.value = ""; this.reviewerIdTarget.value = ""; this.resultsTarget.replaceChildren()
       this.closeAssignment()
       event.target.querySelector('[type="checkbox"]').checked = true
diff --git a/engine/app/models/coplan/plan_event.rb b/engine/app/models/coplan/plan_event.rb
index f728a09f..da2e3e1e 100644
--- a/engine/app/models/coplan/plan_event.rb
+++ b/engine/app/models/coplan/plan_event.rb
@@ -9,7 +9,7 @@ module CoPlan
   # metadata-only changes).
   #
   # Records are append-only — never updated, never destroyed except through
-  # the parent plan's cascade or explicit removal of a reviewer's history.
+  # the parent plan's cascade.
   class PlanEvent < ApplicationRecord
     # Mirrors PlanVersion::ACTOR_TYPES so history items from either source can
     # be rendered uniformly.
diff --git a/engine/app/services/coplan/reviews/changed.rb b/engine/app/services/coplan/reviews/changed.rb
index 089903dd..14d76fe7 100644
--- a/engine/app/services/coplan/reviews/changed.rb
+++ b/engine/app/services/coplan/reviews/changed.rb
@@ -11,14 +11,8 @@ def initialize(request:, event_type:, actor_token_id:, payload:)
 
       def call
         request, event_type, actor_token_id, payload = @request, @event_type, @actor_token_id, @payload
-        request.plan.with_lock do
-          # A response can commit just before removal, then reach this fan-out
-          # afterward. Do not put an erased decision back in an agent's inbox.
-          if payload["action"] == "removed" || request.plan.review_requests.exists?(request.id)
-            AgentEvents::Publish.call(plan: request.plan, event_type: event_type, actor_token_id: actor_token_id,
-              payload: { "review_request_id" => request.id, "reviewer_id" => request.reviewer_id }.merge(payload))
-          end
-        end
+        AgentEvents::Publish.call(plan: request.plan, event_type: event_type, actor_token_id: actor_token_id,
+          payload: { "review_request_id" => request.id, "reviewer_id" => request.reviewer_id }.merge(payload))
         Notifications::BroadcastBadges.call(user_ids: [ request.reviewer_id, request.plan.created_by_user_id ])
         # No viewer-relative HTML or CSRF tokens on a shared stream. Each
         # open tab fetches its own controls, preserving its current draft.
diff --git a/engine/app/services/coplan/reviews/remove.rb b/engine/app/services/coplan/reviews/remove.rb
index 83d6bef3..0fc88e3f 100644
--- a/engine/app/services/coplan/reviews/remove.rb
+++ b/engine/app/services/coplan/reviews/remove.rb
@@ -1,32 +1,15 @@
 module CoPlan
   module Reviews
-    # Removing a reviewer ends the request's identity. A later assignment gets
-    # a new ID, so delayed responses to the old request cannot complete it.
-    # API withdrawal is separate and continues to preserve review history.
+    # A later assignment gets a fresh ID, so old response URLs stay invalid.
     class Remove
       def self.call(plan:, request_id:)
-        removed_events = []
         request = plan.with_lock do
           request = plan.review_requests.find_by(id: request_id)
-          next unless request
-
-          events = plan.plan_events.where(event_type: %w[review_requested review_withdrawn], after_value: request.id)
-          removed_events = events.to_a
-          events.delete_all
-          event_ids = AgentEvent.where(plan: plan, event_type: %w[review.requested review.responded]).pluck(:id, :payload).filter_map do |id, payload|
-            id if payload.to_h["review_request_id"] == request.id
-          end
-          AgentEvent.where(id: event_ids).delete_all
-          request.destroy!
+          request&.destroy!
           request
         end
-        if request
-          removed_events.each { |event| Broadcaster.remove_to(plan, target: ActionView::RecordIdentifier.dom_id(event)) }
-          count = plan.plan_versions.count + plan.plan_events.count
-          Broadcaster.replace_to(plan, target: "history-count",
-            html: ApplicationController.helpers.content_tag(:span, ApplicationController.helpers.pluralize(count, "entry"), class: "section-count", id: "history-count"))
-          Changed.call(request: request, event_type: "review.requested", payload: { "action" => "removed", "removed" => true })
-        end
+        Changed.call(request: request, event_type: "review.requested",
+          payload: { "action" => "removed", "removed" => true }) if request
         request
       end
     end
diff --git a/engine/app/services/coplan/reviews/request.rb b/engine/app/services/coplan/reviews/request.rb
index 9e5b971d..f777fbd3 100644
--- a/engine/app/services/coplan/reviews/request.rb
+++ b/engine/app/services/coplan/reviews/request.rb
@@ -1,14 +1,15 @@
 module CoPlan
   module Reviews
     class Request
-      def self.call(plan:, reviewer:, actor:, requirement: "required", api_token: nil, create_only: false, request_id: nil)
-        new(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:).call
+      def self.call(plan:, reviewer:, actor:, requirement: "required", api_token: nil, create_only: false, request_id: nil, replace_withdrawn: false)
+        new(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:, replace_withdrawn:).call
       end
 
-      def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:)
+      def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:, replace_withdrawn:)
         @plan, @reviewer, @actor, @requirement, @api_token = plan, reviewer, actor, requirement, api_token
         @create_only = create_only
         @request_id = request_id
+        @replace_withdrawn = replace_withdrawn
       end
 
       def call
@@ -19,14 +20,17 @@ def call
           else
             @plan.review_requests.find_or_initialize_by(reviewer: @reviewer)
           end
-          # Retrying a create cannot change a decision, reopen a withdrawal,
-          # or send another nudge. Only an explicit re-request starts a cycle.
+          # API create retries preserve every existing task. The web add form
+          # deliberately replaces withdrawn tasks left by earlier removals.
           if @create_only && request.persisted?
             unless ReviewRequest::REQUIREMENTS.include?(@requirement)
               request.errors.add(:requirement, :inclusion)
               raise ActiveRecord::RecordInvalid, request
             end
-            next request
+            next request unless @replace_withdrawn && request.status == "withdrawn"
+
+            request.destroy!
+            request = @plan.review_requests.new(reviewer: @reviewer)
           end
 
           # Preserve the original request for tasks created before request
diff --git a/engine/app/views/coplan/agent_instructions/guides/api.text.erb b/engine/app/views/coplan/agent_instructions/guides/api.text.erb
index 24d86ef6..d8087c87 100644
--- a/engine/app/views/coplan/agent_instructions/guides/api.text.erb
+++ b/engine/app/views/coplan/agent_instructions/guides/api.text.erb
@@ -82,8 +82,6 @@ return 429. Invalid names return 422 with `invalid_request`.
 
 Creating a review request for an existing reviewer returns the current task with status 201. It leaves the requirement, decision, history, and notifications unchanged. Use `PATCH` to change the requirement or `request_again: true` to request another look.
 
-Removing a reviewer in the web management pane deletes their request and review decisions. Re-adding them creates a fresh pending request with a new ID. The removed request ID returns 404, including for old response retries. API withdrawal remains separate and preserves history.
-
 Include `idempotency_key` (a fresh UUID) for each intended review response. Reuse that UUID and identical fields when retrying a lost response; its returned `id` is the UUID. Use a new UUID for a new decision, including another review of the same version. A keyed retry returns the original recorded result even after the plan changes or the task is withdrawn. It preserves any newly pending review task. Without a key, an identical response is treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. A key reused for different fields returns 422.
 
 A review response returns 409 with `code: review_version_changed` for a stale revision. Re-read before responding. A new response to a withdrawn request returns 409 with `code: review_request_withdrawn`; stop responding unless the author requests another look.
diff --git a/engine/app/views/coplan/plans/_event_item.html.erb b/engine/app/views/coplan/plans/_event_item.html.erb
index 847e4a73..a4f75cfe 100644
--- a/engine/app/views/coplan/plans/_event_item.html.erb
+++ b/engine/app/views/coplan/plans/_event_item.html.erb
@@ -2,7 +2,7 @@
     _version_item.html.erb. Non-interactive (no turbo-frame target) since
     there's no separate diff to display — the event payload itself is the
     diff. %>
-<div class="history-split__item history-split__item--event" id="<%= dom_id(event) %>">
+<div class="history-split__item history-split__item--event">
   <div class="history-split__event-chip"><%= event.field || event.event_type %></div>
   <div class="history-split__meta">
     <% if event.actor_user %>
diff --git a/spec/requests/review_requests_spec.rb b/spec/requests/review_requests_spec.rb
index 62c88cab..6ac79639 100644
--- a/spec/requests/review_requests_spec.rb
+++ b/spec/requests/review_requests_spec.rb
@@ -6,37 +6,6 @@
   let(:plan) { create(:plan, :published, created_by_user: author) }
   let(:task) { create(:review_request, plan: plan, reviewer: reviewer) }
 
-  [ true, false ].each do |keyed|
-    it "rejects old #{keyed ? 'keyed' : 'legacy'} responses after removing and re-adding a reviewer", :review_removal do
-      payload = { revision: 1, decision: "approved" }
-      payload[:idempotency_key] = SecureRandom.uuid if keyed
-      previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, **payload)
-      sign_in_as(author)
-      delete plan_review_request_path(plan, task), params: { review_request: { remove: true } }, as: :json
-      expect(response).to have_http_status(:no_content)
-      expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
-      post plan_review_requests_path(plan), params: { review_request: { reviewer_id: reviewer.id } }, as: :json
-      expect(response).to have_http_status(:created)
-      fresh = plan.review_requests.find(response.parsed_body.fetch("id"))
-      expect(fresh.id).not_to eq(task.id)
-
-      sign_in_as(reviewer)
-      post respond_plan_review_request_path(plan, task), params: { review_response: payload }, as: :json
-      expect(response).to have_http_status(:not_found)
-      sign_in_as(author)
-      patch plan_review_request_path(plan, task), params: { review_request: { request_again: true } }, as: :json
-      expect(response).to have_http_status(:not_found)
-      delete plan_review_request_path(plan, task), params: { review_request: { remove: true } }, as: :json
-      expect(response).to have_http_status(:no_content)
-      expect(fresh.reload.status).to eq("pending")
-      expect(fresh.review_responses).to be_empty
-      get plan_review_requests_path(plan)
-      expect(Nokogiri::HTML(response.body).css(".review-log__event").map { |node| node.text.squish }).to eq([
-        "#{author.name} requested a review from #{reviewer.name}"
-      ])
-    end
-  end
-
   it "allows only the plan author to erase a reviewer's decisions", :review_removal do
     previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
     sign_in_as(reviewer)
diff --git a/spec/services/coplan/reviews/remove_spec.rb b/spec/services/coplan/reviews/remove_spec.rb
index ced77aee..7c4678e7 100644
--- a/spec/services/coplan/reviews/remove_spec.rb
+++ b/spec/services/coplan/reviews/remove_spec.rb
@@ -7,8 +7,6 @@
 
   %w[approved changes_requested].each do |decision|
     it "erases a removed reviewer's #{decision} decisions and starts a new pending request" do
-      watcher = create(:api_token, user: plan.created_by_user)
-      CoPlan::AgentSession.create!(plan: plan, api_token: watcher, state: "watching", agent_name: "Authorbot")
       previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1,
         decision: decision, note: decision == "changes_requested" ? "Earlier feedback" : nil)
       other = CoPlan::Reviews::Request.call(plan: plan, reviewer: create(:coplan_user), actor: plan.created_by_user)
@@ -19,9 +17,6 @@
       expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
       expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
       expect(plan.notifications.where(review_request_id: task.id)).to be_empty
-      expect(plan.plan_events.where(after_value: task.id)).to be_empty
-      expect(watcher.agent_events.where(event_type: "review.responded").map { |event| event.payload["review_request_id"] }).to eq([ other.id ])
-      expect(watcher.agent_events.last.payload).to include("action" => "removed", "review_request_id" => task.id)
       expect(other_response.reload.decision).to eq("approved")
 
       fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user, create_only: true)
@@ -42,28 +37,15 @@
     expect(fresh.review_responses).to eq([ approval ])
   end
 
-  it "rejects an old re-request already in flight when the request is removed" do
-    old_id = task.id
-    described_class.call(plan: plan, request_id: old_id)
+  it "keeps the old review if a replacement request is invalid" do
+    previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
+    CoPlan::Reviews::Withdraw.call(request: task, actor: plan.created_by_user)
     expect {
-      CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user, request_id: old_id)
-    }.to raise_error(ActiveRecord::RecordNotFound)
-    expect(plan.review_requests).to be_empty
-  end
-
-  it "does not erase a request from another plan" do
-    task
-    expect(described_class.call(plan: create(:plan, :published), request_id: task.id)).to be_nil
-    expect(task.reload.status).to eq("pending")
-  end
-
-  it "does not republish an erased response when its post-commit fan-out runs late" do
-    watcher = create(:api_token, user: plan.created_by_user)
-    CoPlan::AgentSession.create!(plan: plan, api_token: watcher, state: "watching", agent_name: "Authorbot")
-    stale_request = task
-    described_class.call(plan: plan, request_id: task.id)
-    CoPlan::Reviews::Changed.call(request: stale_request, event_type: "review.responded", payload: { "decision" => "approved" })
-    expect(watcher.agent_events.where(event_type: "review.responded")).to be_empty
+      CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user,
+        requirement: "invalid", create_only: true, replace_withdrawn: true)
+    }.to raise_error(ActiveRecord::RecordInvalid)
+    expect(previous.reload.decision).to eq("approved")
+    expect(task.reload.status).to eq("withdrawn")
   end
 
   describe "requests already waiting for the plan lock" do
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 8c296630..20d5f0a8 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -60,7 +60,7 @@ def lose_next_review_result
     end
   end
 
-  [ "light", "dark" ].product(%w[approved changes_requested]).each do |theme, decision|
+  [ [ "light", "approved" ], [ "dark", "changes_requested" ] ].each do |theme, decision|
     it "starts fresh after removing a #{decision} reviewer in #{theme} theme", :review_readdition do
       previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: decision,
         note: decision == "changes_requested" ? "Earlier review feedback" : nil)
@@ -104,75 +104,22 @@ def lose_next_review_result
     end
   end
 
-  [ "lost", "rejected" ].each do |failure|
-    it "keeps removal retryable after a #{failure} result", :review_readdition do
-      sign_in_browser(author)
-      click_button "More actions"
-      within("#plan-menu") { click_button "Manage reviewers" }
-      page.execute_script(<<~JS, failure)
-        const originalFetch = window.fetch.bind(window)
-        const failure = arguments[0]
-        // Also lose the socket invalidation so the stale row can be retried.
-        window.Turbo.StreamActions['coplan-review-changed'] = function () {}
-        let interrupt = true
-        window.fetch = async (url, options) => {
-          if (interrupt && options?.method === 'DELETE' && String(url).includes('/review_requests/')) {
-            interrupt = false
-            if (failure === 'rejected') return new Response(JSON.stringify({error: 'Please retry removal'}), {status: 422, headers: {'Content-Type': 'application/json'}})
-            await originalFetch(url, options)
-            throw new TypeError('Lost the successful removal result')
-          }
-          return originalFetch(url, options)
-        }
-      JS
-      within(".review-management[open]") do
-        click_button "Actions for Plan Reviewer", enable_aria_label: true
-        click_button "Remove reviewer"
-        expect(page).to have_css("[role='alert']", text: failure == "lost" ? "Couldn't reach CoPlan" : "Please retry removal")
-        expect(CoPlan::ReviewRequest.exists?(task.id)).to eq(failure == "rejected")
-        click_button "Actions for Plan Reviewer", enable_aria_label: true
-        click_button "Remove reviewer"
-        expect(page).to have_no_css("[data-review-assignments] .review-person")
-      end
-      expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
-    end
-
-    it "keeps re-adding a reviewer retryable after a #{failure} update result", :review_readdition do
-      CoPlan::Reviews::Withdraw.call(request: task, actor: author)
-      sign_in_browser(author)
-      click_button "More actions"
-      within("#plan-menu") { click_button "Manage reviewers" }
-      within(".review-management[open]") { click_button "Add reviewer", exact: true }
-      fill_in "Find a reviewer", with: reviewer.name
-      find(".review-search-result", text: reviewer.name).click
-      page.execute_script(<<~JS, failure)
-        const originalFetch = window.fetch.bind(window)
-        let interrupt = true
-        const failure = arguments[0]
-        window.fetch = async (url, options) => {
-          if (interrupt && options?.method === 'PATCH' && String(url).includes('/review_requests/')) {
-            interrupt = false
-            if (failure === 'rejected') return new Response(JSON.stringify({error: 'Please retry this request'}), {status: 422, headers: {'Content-Type': 'application/json'}})
-            await originalFetch(url, options)
-            throw new TypeError('Lost the result after the server reopened the task')
-          }
-          return originalFetch(url, options)
-        }
-      JS
-      within(".review-assignment-form") { click_button "Add reviewer", exact: true }
-      expect(page).to have_css(".review-assignment [role='alert']", text: failure == "lost" ? "Couldn't reach CoPlan" : "Please retry this request")
-      expect(page).to have_field("Find a reviewer", with: reviewer.name)
-      expect(task.reload.status).to eq(failure == "lost" ? "pending" : "withdrawn")
-      within(".review-assignment-form") { click_button "Add reviewer", exact: true }
-      within(".review-management[open] [data-review-assignments]") do
-        expect(page).to have_css(".review-person", text: reviewer.name)
-        expect(page).to have_css(".review-state", text: "Pending")
-      end
-      expect(task.reload.status).to eq("pending")
-      expect(plan.review_requests.where(reviewer: reviewer).count).to eq(1)
-      expect(plan.plan_events.where(event_type: "review_requested", after_value: task.id).count).to eq(2)
-      expect(task.notifications.unread.where(user: reviewer, reason: "review_requested").count).to eq(1)
-    end
+  it "re-adds a reviewer removed before permanent removal was supported", :review_readdition do
+    previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
+    CoPlan::Reviews::Withdraw.call(request: task, actor: author)
+    sign_in_browser(author)
+    click_button "More actions"
+    within("#plan-menu") { click_button "Manage reviewers" }
+    within(".review-management[open]") { click_button "Add reviewer", exact: true }
+    fill_in "Find a reviewer", with: reviewer.name
+    find(".review-search-result", text: reviewer.name).click
+    within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+    expect(page).to have_css(".review-person", text: reviewer.name)
+    expect(page).to have_css(".review-state", text: "Pending")
+    fresh = plan.review_requests.find_by!(reviewer: reviewer)
+    expect(fresh.id).not_to eq(task.id)
+    expect(fresh.review_responses).to be_empty
+    expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
   end
 
   it "leaves an already active review unchanged when a selected re-add is submitted", :review_readdition do

From f46f097117b6e364499c6ed55aa9ae4beacd35e8 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Thu, 29 Oct 2026 15:33:27 -0400
Subject: [PATCH 04/10] Exercise reviewer removal with real request history

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 spec/system/plan_reviews_spec.rb | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 20d5f0a8..dcfbbb05 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -62,7 +62,9 @@ def lose_next_review_result
 
   [ [ "light", "approved" ], [ "dark", "changes_requested" ] ].each do |theme, decision|
     it "starts fresh after removing a #{decision} reviewer in #{theme} theme", :review_readdition do
-      previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: decision,
+      task.destroy!
+      requested = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author)
+      previous = CoPlan::Reviews::Respond.call(request: requested, actor: reviewer, revision: 1, decision: decision,
         note: decision == "changes_requested" ? "Earlier review feedback" : nil)
       sign_in_browser(author)
       page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)
@@ -74,7 +76,7 @@ def lose_next_review_result
         expect(page).to have_no_css("[data-review-assignments] .review-person")
         click_button "Add reviewer", exact: true
       end
-      expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
+      expect(CoPlan::ReviewRequest.exists?(requested.id)).to be(false)
       expect(CoPlan::ReviewResponse.exists?(previous.id)).to be(false)
       fill_in "Find a reviewer", with: reviewer.name
       find(".review-search-result", text: reviewer.name).click
@@ -86,7 +88,7 @@ def lose_next_review_result
         expect(page).to have_css(".review-state", text: "Pending")
       end
       fresh = plan.review_requests.find_by!(reviewer: reviewer)
-      expect(fresh.id).not_to eq(task.id)
+      expect(fresh.id).not_to eq(requested.id)
       expect(fresh).to have_attributes(status: "pending", requirement: "optional")
       expect(plan.review_requests.where(reviewer: reviewer).count).to eq(1)
       expect(fresh.review_responses).to be_empty
@@ -95,8 +97,9 @@ def lose_next_review_result
       within(".review-management[open]") { click_button "Close reviews", enable_aria_label: true }
       within("#footnote-approvals") { click_button "View activity", exact: false }
       within(".review-activity-dialog[open]") do
-        expect(all(".review-log__event").map { |entry| entry.text.squish }).to eq([ "Plan Author requested a review from Plan Reviewer" ])
+        expect(all(".review-log__event").map { |entry| entry.text.squish }).to eq([ "Plan Author requested a review from Plan Reviewer" ] * 2)
         expect(page).to have_no_text("Earlier review feedback")
+        expect(plan.plan_events.where(event_type: "review_requested").pluck(:after_value)).to contain_exactly(requested.id, fresh.id)
       end
       page.save_screenshot(Rails.root.join("tmp/review-preview/fresh-review-history-#{theme}.png"))
       sign_in_browser(reviewer)

From b235f4fe811107dbe5f728ca5940fd105d84ddec Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Thu, 29 Oct 2026 15:44:09 -0400
Subject: [PATCH 05/10] Preserve legacy request audit and removal events

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 engine/app/services/coplan/reviews/remove.rb  |  7 +++++-
 engine/app/services/coplan/reviews/request.rb |  7 ++++++
 spec/services/coplan/reviews/remove_spec.rb   | 24 +++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/engine/app/services/coplan/reviews/remove.rb b/engine/app/services/coplan/reviews/remove.rb
index 0fc88e3f..e269863b 100644
--- a/engine/app/services/coplan/reviews/remove.rb
+++ b/engine/app/services/coplan/reviews/remove.rb
@@ -5,7 +5,12 @@ class Remove
       def self.call(plan:, request_id:)
         request = plan.with_lock do
           request = plan.review_requests.find_by(id: request_id)
-          request&.destroy!
+          if request
+            unless plan.plan_events.where(event_type: "review_requested", after_value: request.id).exists?
+              RecordRequest.call(request: request)
+            end
+            request.destroy!
+          end
           request
         end
         Changed.call(request: request, event_type: "review.requested",
diff --git a/engine/app/services/coplan/reviews/request.rb b/engine/app/services/coplan/reviews/request.rb
index f777fbd3..5f028d60 100644
--- a/engine/app/services/coplan/reviews/request.rb
+++ b/engine/app/services/coplan/reviews/request.rb
@@ -14,6 +14,7 @@ def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:,
 
       def call
         changed = false
+        removed = nil
         request = @plan.with_lock do
           request = if @request_id
             @plan.review_requests.where(reviewer: @reviewer).find(@request_id)
@@ -29,7 +30,11 @@ def call
             end
             next request unless @replace_withdrawn && request.status == "withdrawn"
 
+            unless @plan.plan_events.where(event_type: "review_requested", after_value: request.id).exists?
+              RecordRequest.call(request: request)
+            end
             request.destroy!
+            removed = request
             request = @plan.review_requests.new(reviewer: @reviewer)
           end
 
@@ -48,6 +53,8 @@ def call
           changed = true
           request
         end
+        Changed.call(request: removed, event_type: "review.requested", actor_token_id: @api_token&.id,
+          payload: { "action" => "removed", "removed" => true }) if removed
         Changed.call(request: request, event_type: "review.requested", actor_token_id: @api_token&.id,
           payload: { "action" => "requested", "requirement" => request.requirement }) if changed
         request
diff --git a/spec/services/coplan/reviews/remove_spec.rb b/spec/services/coplan/reviews/remove_spec.rb
index 7c4678e7..201b9d9d 100644
--- a/spec/services/coplan/reviews/remove_spec.rb
+++ b/spec/services/coplan/reviews/remove_spec.rb
@@ -37,6 +37,30 @@
     expect(fresh.review_responses).to eq([ approval ])
   end
 
+  it "preserves a legacy request's audit before deleting the task" do
+    legacy = create(:review_request, plan: plan, reviewer: reviewer)
+    expect(plan.plan_events.where(event_type: "review_requested")).to be_empty
+    described_class.call(plan: plan, request_id: legacy.id)
+    event = plan.plan_events.find_by!(event_type: "review_requested", after_value: legacy.id)
+    expect(event).to have_attributes(actor_id: legacy.requested_by_user_id, created_at: legacy.requested_at)
+    expect(event.metadata).to include("reviewer_name" => reviewer.name, "revision" => 1)
+    expect(CoPlan::ReviewRequest.exists?(legacy.id)).to be(false)
+  end
+
+  it "announces the old identity's removal when replacing a withdrawn legacy request" do
+    legacy = create(:review_request, plan: plan, reviewer: reviewer, status: "withdrawn")
+    watcher = create(:api_token, user: reviewer)
+    CoPlan::AgentSession.create!(plan: plan, api_token: watcher, state: "watching", agent_name: "Watcher")
+    fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: plan.created_by_user,
+      create_only: true, replace_withdrawn: true)
+    payloads = watcher.agent_events.where(event_type: "review.requested").order(:created_at).map(&:payload)
+    expect(payloads.size).to eq(2)
+    expect(payloads.first).to include("action" => "removed", "removed" => true, "review_request_id" => legacy.id)
+    expect(payloads.last).to include("action" => "requested", "review_request_id" => fresh.id)
+    expect(plan.plan_events.where(event_type: "review_requested").pluck(:after_value)).to contain_exactly(legacy.id, fresh.id)
+    expect(fresh.status).to eq("pending")
+  end
+
   it "keeps the old review if a replacement request is invalid" do
     previous = CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved")
     CoPlan::Reviews::Withdraw.call(request: task, actor: plan.created_by_user)

From 417a01d144167c4329232abf30a572c9204376e3 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Mon, 2 Nov 2026 15:54:04 -0500
Subject: [PATCH 06/10] Keep review drafts current after reassignment

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 .github/workflows/ci.yml                      |   2 +-
 .../api/v1/review_requests_controller.rb      |  10 +-
 .../coplan/review_requests_controller.rb      |  10 +-
 .../coplan/review_tray_controller.js          |   6 ++
 engine/app/services/coplan/reviews/changed.rb |   6 ++
 engine/app/services/coplan/reviews/request.rb |  21 ++--
 spec/services/coplan/reviews/remove_spec.rb   |   3 +-
 spec/services/coplan/reviews_spec.rb          |  12 +++
 spec/system/plan_reviews_spec.rb              | 102 ++++++++++++++++++
 9 files changed, 157 insertions(+), 15 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 94fce7e9..a1bcf54a 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -19,7 +19,7 @@ jobs:
   # test jobs rather than gating them: a bracket-spacing nit should not hide
   # whether the suite passes.
   lint:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
 
     steps:
       - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
diff --git a/engine/app/controllers/coplan/api/v1/review_requests_controller.rb b/engine/app/controllers/coplan/api/v1/review_requests_controller.rb
index 35b75c4f..45ff79e4 100644
--- a/engine/app/controllers/coplan/api/v1/review_requests_controller.rb
+++ b/engine/app/controllers/coplan/api/v1/review_requests_controller.rb
@@ -34,10 +34,14 @@ def create
 
         def update
           if ActiveModel::Type::Boolean.new.cast(params[:request_again])
-            Reviews::Request.call(plan: @plan, reviewer: @review_request.reviewer, actor: current_user,
-              requirement: params[:requirement].presence || @review_request.requirement, api_token: @api_token, request_id: @review_request.id)
+            Reviews::Request.again(request: @review_request, actor: current_user,
+              requirement: params[:requirement].presence, api_token: @api_token)
           else
-            @plan.with_lock { @review_request.reload.update!(requirement: params.require(:requirement)) }
+            @plan.with_lock do
+              # The request may have been removed while waiting for the plan lock.
+              @review_request.reload
+              @review_request.update!(requirement: params.require(:requirement))
+            end
             Reviews::Changed.call(request: @review_request, event_type: "review.requested", actor_token_id: api_token_id,
               payload: { "action" => "requirement_changed", "requirement" => @review_request.requirement })
           end
diff --git a/engine/app/controllers/coplan/review_requests_controller.rb b/engine/app/controllers/coplan/review_requests_controller.rb
index 4417b0f4..ec2bdc51 100644
--- a/engine/app/controllers/coplan/review_requests_controller.rb
+++ b/engine/app/controllers/coplan/review_requests_controller.rb
@@ -29,10 +29,14 @@ def update
       authorize!(@plan, :update?)
       attributes = params.expect(review_request: [ :requirement, :request_again ])
       if ActiveModel::Type::Boolean.new.cast(attributes[:request_again])
-        Reviews::Request.call(plan: @plan, reviewer: @review_request.reviewer, actor: current_user,
-          requirement: attributes[:requirement].presence || @review_request.requirement, request_id: @review_request.id)
+        Reviews::Request.again(request: @review_request, actor: current_user,
+          requirement: attributes[:requirement].presence)
       else
-        @plan.with_lock { @review_request.reload.update!(requirement: attributes.fetch(:requirement)) }
+        @plan.with_lock do
+          # The request may have been removed while waiting for the plan lock.
+          @review_request.reload
+          @review_request.update!(requirement: attributes.fetch(:requirement))
+        end
         Reviews::Changed.call(request: @review_request, event_type: "review.requested",
           payload: { "action" => "requirement_changed", "requirement" => @review_request.requirement })
       end
diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index 4adabcdf..4cfd9ead 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -234,6 +234,12 @@ export default class extends Controller {
         }
       }
       if (tray && existingTray) existingTray.dataset.completed = tray.dataset.completed
+      if (tray && existingTray) {
+        // Keep the draft and dictation state, but submit new feedback to the
+        // current request. An already-issued fetch retains its original URL.
+        const form = tray.querySelector('[data-coplan--review-tray-target="commentForm"]')
+        if (form && this.hasCommentFormTarget) this.commentFormTarget.action = form.action
+      }
       if (tray && !this.element.querySelector(".review-tray")) this.element.append(tray)
       const currentTray = this.element.querySelector(".review-tray")
       if (tray && currentTray) {
diff --git a/engine/app/services/coplan/reviews/changed.rb b/engine/app/services/coplan/reviews/changed.rb
index 14d76fe7..950d9b92 100644
--- a/engine/app/services/coplan/reviews/changed.rb
+++ b/engine/app/services/coplan/reviews/changed.rb
@@ -14,6 +14,12 @@ def call
         AgentEvents::Publish.call(plan: request.plan, event_type: event_type, actor_token_id: actor_token_id,
           payload: { "review_request_id" => request.id, "reviewer_id" => request.reviewer_id }.merge(payload))
         Notifications::BroadcastBadges.call(user_ids: [ request.reviewer_id, request.plan.created_by_user_id ])
+        if payload["removed"]
+          # Public notes survive removal; their deleted review decisions must
+          # disappear from the discussion labels in open tabs as well.
+          Broadcaster.replace_to(request.plan, target: "plan-general-comment-list", partial: "coplan/plans/general_comment_list",
+            locals: { threads: request.plan.comment_threads.with_kept_comments.includes(:comments).order(:created_at) })
+        end
         # No viewer-relative HTML or CSRF tokens on a shared stream. Each
         # open tab fetches its own controls, preserving its current draft.
         Broadcaster.custom_action_to(request.plan, action: "coplan-review-changed", target: "plan-review-controls", html: "")
diff --git a/engine/app/services/coplan/reviews/request.rb b/engine/app/services/coplan/reviews/request.rb
index 5f028d60..2a7d8f21 100644
--- a/engine/app/services/coplan/reviews/request.rb
+++ b/engine/app/services/coplan/reviews/request.rb
@@ -1,14 +1,19 @@
 module CoPlan
   module Reviews
     class Request
-      def self.call(plan:, reviewer:, actor:, requirement: "required", api_token: nil, create_only: false, request_id: nil, replace_withdrawn: false)
-        new(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:, replace_withdrawn:).call
+      def self.call(plan:, reviewer:, actor:, requirement: "required", api_token: nil, create_only: false, replace_withdrawn: false)
+        new(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, replace_withdrawn:).call
       end
 
-      def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, request_id:, replace_withdrawn:)
+      def self.again(request:, actor:, requirement: nil, api_token: nil)
+        new(plan: request.plan, reviewer: request.reviewer, request:, actor:, requirement:, api_token:,
+          create_only: false, replace_withdrawn: false).call
+      end
+
+      def initialize(plan:, reviewer:, actor:, requirement:, api_token:, create_only:, replace_withdrawn:, request: nil)
         @plan, @reviewer, @actor, @requirement, @api_token = plan, reviewer, actor, requirement, api_token
         @create_only = create_only
-        @request_id = request_id
+        @request = request
         @replace_withdrawn = replace_withdrawn
       end
 
@@ -16,8 +21,12 @@ def call
         changed = false
         removed = nil
         request = @plan.with_lock do
-          request = if @request_id
-            @plan.review_requests.where(reviewer: @reviewer).find(@request_id)
+          request = if @request
+            # The plan lock does not reload its children. Reject a request
+            # removed while this operation was waiting for the lock.
+            @request.reload
+            @requirement = @request.requirement if @requirement.nil?
+            @request
           else
             @plan.review_requests.find_or_initialize_by(reviewer: @reviewer)
           end
diff --git a/spec/services/coplan/reviews/remove_spec.rb b/spec/services/coplan/reviews/remove_spec.rb
index 201b9d9d..7340a19c 100644
--- a/spec/services/coplan/reviews/remove_spec.rb
+++ b/spec/services/coplan/reviews/remove_spec.rb
@@ -92,8 +92,7 @@
                 if action == :respond
                   CoPlan::Reviews::Respond.call(request: stale, actor: stale.reviewer, revision: 1, decision: "approved")
                 else
-                  CoPlan::Reviews::Request.call(plan: stale.plan, reviewer: stale.reviewer,
-                    actor: stale.requested_by_user, request_id: old_id)
+                  CoPlan::Reviews::Request.again(request: stale, actor: stale.requested_by_user)
                 end
               rescue ActiveRecord::RecordNotFound => error
                 error
diff --git a/spec/services/coplan/reviews_spec.rb b/spec/services/coplan/reviews_spec.rb
index ba55bceb..ee19d1fe 100644
--- a/spec/services/coplan/reviews_spec.rb
+++ b/spec/services/coplan/reviews_spec.rb
@@ -23,6 +23,18 @@ def respond(decision: "approved", note: nil, **attributes)
     expect(plan.plan_collaborators).to be_empty
   end
 
+  it "re-requests the existing task using its current requirement unless overridden" do
+    stale = request
+    CoPlan::ReviewRequest.find(stale.id).update!(requirement: "optional")
+
+    repeated = CoPlan::Reviews::Request.again(request: stale, actor: plan.created_by_user)
+    expect(repeated).to have_attributes(id: stale.id, requirement: "optional", status: "pending")
+    expect(plan.plan_events.where(event_type: "review_requested").last.metadata).to include("requirement" => "optional")
+
+    overridden = CoPlan::Reviews::Request.again(request: stale, actor: plan.created_by_user, requirement: "required")
+    expect(overridden).to have_attributes(id: stale.id, requirement: "required", status: "pending")
+  end
+
   it "preserves the original approval after edits and another review request" do
     response = respond(note: "Looks right")
     expect(response.comment_thread.comments.first.body_markdown).to eq("Looks right")
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index dcfbbb05..741664fb 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -38,6 +38,21 @@ def lose_next_review_result
     JS
   end
 
+  # Hold automatic invalidations so both separate and coalesced delivery can
+  # be exercised deterministically without replacing the open draft.
+  def pause_review_refreshes
+    page.execute_script(<<~JS)
+      const root = document.getElementById("plan-review-controls");
+      const controller = window.Stimulus.getControllerForElementAndIdentifier(root, "coplan--review-tray");
+      window.refreshReviewControls = controller.refresh.bind(controller);
+      controller.refresh = async () => {};
+    JS
+  end
+
+  def refresh_review_controls
+    page.driver.browser.execute_async_script("window.refreshReviewControls().then(arguments[0])")
+  end
+
   before { sign_in_browser(reviewer, open_sections: !RSpec.current_example.metadata[:collapsed_support]) }
 
   it "keeps an API-withdrawn request between its two request cycles", :review_withdrawals do
@@ -149,6 +164,93 @@ def lose_next_review_result
     expect([ plan.plan_events.where(event_type: "review_requested").count, CoPlan::Notification.count ]).to eq(counts)
   end
 
+  [ [ "light", false ], [ "dark", true ] ].each do |theme, separate_refresh|
+    it "submits a preserved draft to the replacement after #{separate_refresh ? 'separate' : 'coalesced'} refreshes", :review_readdition do
+      page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)
+      within(".review-tray") { click_button "Request changes", exact: true }
+      fill_in "Public review comment", with: "Please clarify the retry limits.", enable_aria_label: true
+      page.execute_script('window.reviewDraft = document.getElementById("review-discussion-body")')
+      pause_review_refreshes
+      CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+      if separate_refresh
+        refresh_review_controls
+        expect(page).to have_css('.review-tray__discussion [type="submit"][disabled]')
+        expect(page).to have_content("Your unsent feedback is still here.")
+      end
+      fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true)
+      refresh_review_controls
+      expect(page).to have_css("[data-review-decision-actions] button[data-url*='#{fresh.id}']", visible: :all)
+      expect(find('[data-coplan--review-tray-target="commentForm"]')["action"]).to end_with(respond_plan_review_request_path(plan, fresh))
+      expect(page).to have_field("Public review comment", with: "Please clarify the retry limits.", enable_aria_label: true)
+      expect(page.evaluate_script('window.reviewDraft === document.getElementById("review-discussion-body")')).to be(true)
+      page.save_screenshot(Rails.root.join("tmp/review-preview/preserved-reassigned-draft-#{theme}.png"))
+      within(".review-tray__discussion") { click_button "Request changes" }
+      expect(page).to have_content("Changes requested. Your feedback is in the plan’s comments.")
+      expect(fresh.reload.status).to eq("changes_requested")
+      expect(fresh.review_responses.last!.note).to eq("Please clarify the retry limits.")
+      expect(CoPlan::ReviewRequest.exists?(task.id)).to be(false)
+    end
+  end
+
+  it "keeps an in-flight submission bound to the removed request", :review_readdition do
+    within(".review-tray") { click_button "Request changes", exact: true }
+    fill_in "Public review comment", with: "Feedback submitted before removal", enable_aria_label: true
+    pause_review_refreshes
+    page.execute_script(<<~JS)
+      const originalFetch = window.fetch.bind(window);
+      window.reviewWrites = [];
+      window.fetch = async (...args) => {
+        if (args[1]?.method === "POST" && String(args[0]).endsWith("/respond")) {
+          window.reviewWrites.push({ url: String(args[0]), key: JSON.parse(args[1].body).review_response.idempotency_key });
+          if (window.reviewWrites.length === 1) {
+            document.documentElement.dataset.reviewWriteHeld = "true";
+            await new Promise(resolve => { window.releaseReviewWrite = resolve; });
+          }
+        }
+        return originalFetch(...args);
+      };
+    JS
+    within(".review-tray__discussion") { click_button "Request changes" }
+    expect(page).to have_css('html[data-review-write-held="true"]')
+    CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+    fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true)
+    refresh_review_controls
+    page.execute_script("window.releaseReviewWrite()")
+    expect(page).to have_content("Couldn't save your response. Your draft is still here.")
+    expect(fresh.reload.status).to eq("pending")
+    expect(fresh.review_responses).to be_empty
+    expect(page).to have_field("Public review comment", with: "Feedback submitted before removal", enable_aria_label: true)
+    within(".review-tray__discussion") { click_button "Request changes" }
+    expect(page).to have_content("Changes requested. Your feedback is in the plan’s comments.")
+    writes = page.evaluate_script("window.reviewWrites")
+    expect(writes.map { |write| URI(write.fetch("url")).path }).to eq([
+      respond_plan_review_request_path(plan, task), respond_plan_review_request_path(plan, fresh)
+    ])
+    expect(writes.map { |write| write.fetch("key") }.uniq.size).to eq(2)
+    expect(fresh.review_responses.count).to eq(1)
+  end
+
+  [ [ "light", false ], [ "dark", true ] ].each do |theme, legacy|
+    it "updates a surviving note's label after #{legacy ? 'legacy replacement' : 'removal'} without navigation", :review_readdition do
+      CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved", note: "Keep the audit trail.")
+      CoPlan::Reviews::Withdraw.call(request: task, actor: author) if legacy
+      sign_in_browser(author)
+      page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)
+      expect(page).to have_css("turbo-cable-stream-source[connected]", visible: :all)
+      expect(page).to have_css(".document-comment__type", text: "Review · Approved · v1")
+      if legacy
+        CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true, replace_withdrawn: true)
+      else
+        CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+      end
+      expect(page).to have_css(".document-comment__type", text: "Plan comment")
+      expect(page).to have_no_css(".document-comment__type", text: "Review · Approved · v1")
+      expect(page).to have_content("Keep the audit trail.")
+      page.execute_script('document.getElementById("plan-general-comments").scrollIntoView({block: "center"})')
+      page.save_screenshot(Rails.root.join("tmp/review-preview/removed-review-note-#{theme}.png"))
+    end
+  end
+
   it "offers equal review choices and preserves cancelled feedback without moving the reader" do
     within(".review-tray__actions") do
       expect(page).to have_button("Request changes")

From 510bb7db32acbf3295fbe2055de78c2a9bf787f9 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Mon, 2 Nov 2026 16:10:40 -0500
Subject: [PATCH 07/10] Invalidate approvals when review requests change

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 .../coplan/review_tray_controller.js          | 22 ++++-
 spec/system/plan_reviews_spec.rb              | 89 +++++++++++++++++++
 2 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index 4cfd9ead..87f12d21 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -132,6 +132,8 @@ export default class extends Controller {
   confirmApproval(event) {
     this.responseUrl = event.currentTarget.dataset.url
     this.approvalRevision = this.viewedRevision()
+    this.approvalInvalidated = false
+    this.approveButtonTarget.disabled = false
     this.approvalErrorTarget.textContent = ""
     this.approveButtonTarget.textContent = `Approve v${this.approvalRevision}`
     this.approvalTitleTarget.textContent = `Approve v${this.approvalRevision}?`
@@ -142,6 +144,12 @@ export default class extends Controller {
     this.approvalTarget.close()
   }
 
+  invalidateApproval() {
+    this.approvalInvalidated = true
+    this.approveButtonTarget.disabled = true
+    this.approvalErrorTarget.textContent = "This review request changed. Close this dialog and reopen approval for the current request. Your note is preserved."
+  }
+
   responseIntent(url, revision, decision, note) {
     this.responseIntents ||= {}
     const fields = JSON.stringify([url, String(revision), note.trim()])
@@ -156,11 +164,16 @@ export default class extends Controller {
 
   async approve(event) {
     event.preventDefault()
+    if (this.approvalInvalidated) { this.invalidateApproval(); return }
     await this.withBusy(event.target, this.approvalErrorTarget, async () => {
-      const intent = this.responseIntent(this.responseUrl, this.approvalRevision, "approved", this.approvalNoteTarget.value)
-      const result = await this.write(this.responseUrl, "POST", { review_response: { revision: this.approvalRevision, decision: "approved", note: this.approvalNoteTarget.value, idempotency_key: intent.key } })
+      const url = this.responseUrl
+      const intent = this.responseIntent(url, this.approvalRevision, "approved", this.approvalNoteTarget.value)
+      const result = await this.write(url, "POST", { review_response: { revision: this.approvalRevision, decision: "approved", note: this.approvalNoteTarget.value, idempotency_key: intent.key } })
       this.confirmIntent("approved", intent)
       const synced = !result.comment_thread_id || await this.refreshThreads(result.comment_thread_id)
+      // Removal can also arrive after the server saved the old approval but
+      // before its result and comments arrive. Keep the dialog and its note.
+      if (this.approvalInvalidated || this.responseUrl !== url) return
       this.approvalTarget.close()
       this.approvalNoteTarget.value = ""
       if (this.hasNoticeTarget) this.noticeTarget.textContent = `Your approval of v${this.approvalRevision} is recorded.` + (synced ? "" : " Refresh the page to see your comment.")
@@ -168,6 +181,9 @@ export default class extends Controller {
       if (tray) delete tray.dataset.reconsidering
       await this.refresh()
     }, "Recording…")
+    // A refresh can invalidate this dialog while the original POST is in
+    // flight. Keep it invalid after the busy-state cleanup restores buttons.
+    if (this.approvalInvalidated && this.approvalTarget.open) this.invalidateApproval()
   }
 
   async post(event) {
@@ -220,6 +236,8 @@ export default class extends Controller {
       if (sequence !== this.refreshSequence || !this.element.isConnected) return
       const tray = incoming.querySelector(".review-tray")
       const existingTray = this.element.querySelector(".review-tray")
+      const responsePath = tray?.querySelector('[data-coplan--review-tray-target="commentForm"]')?.getAttribute("action")
+      if (this.approvalTarget.open && responsePath !== this.responseUrl) this.invalidateApproval()
       const wasWithdrawn = this.reviewWithdrawn
       this.reviewWithdrawn = !tray
       if (!tray && existingTray) {
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index a562ff62..5ddb8f1a 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -230,6 +230,95 @@ def refresh_review_controls
     expect(fresh.review_responses.count).to eq(1)
   end
 
+  [ [ "light", false ], [ "dark", true ] ].each do |theme, separate_refresh|
+    it "invalidates an open approval after #{separate_refresh ? 'separate' : 'coalesced'} reassignment refreshes", :approval_reassignment do
+      page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)
+      within(".review-tray") { click_button "Approve", exact: true }
+      fill_in "Public note", with: "Keep the approval audit trail."
+      pause_review_refreshes
+      CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+      refresh_review_controls if separate_refresh
+      fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true)
+      refresh_review_controls
+      within(".review-confirmation[open]") do
+        expect(page).to have_content("This review request changed.")
+        expect(page).to have_button("Approve v1", disabled: true)
+        expect(page).to have_field("Public note", with: "Keep the approval audit trail.")
+        page.save_screenshot(Rails.root.join("tmp/review-preview/invalidated-approval-#{theme}.png"))
+        click_button "Close approval", enable_aria_label: true
+      end
+      expect(fresh.reload.status).to eq("pending")
+      expect(fresh.review_responses).to be_empty
+      within(".review-tray") { click_button "Approve", exact: true }
+      expect(page).to have_field("Public note", with: "Keep the approval audit trail.")
+      click_button "Approve v1"
+      expect(page).to have_no_css(".review-confirmation[open]")
+      expect(fresh.reload.status).to eq("approved")
+      expect(fresh.review_responses.last!.note).to eq("Keep the approval audit trail.")
+    end
+  end
+
+  it "keeps an approval dialog usable when only its requirement changes", :approval_reassignment do
+    within(".review-tray") { click_button "Approve", exact: true }
+    fill_in "Public note", with: "Same review request"
+    pause_review_refreshes
+    task.update!(requirement: "optional")
+    refresh_review_controls
+    expect(page).to have_button("Approve v1", disabled: false)
+    expect(page).to have_no_content("This review request changed.")
+    click_button "Approve v1"
+    expect(page).to have_no_css(".review-confirmation[open]")
+    expect(task.reload.status).to eq("approved")
+  end
+
+  [ false, true ].each do |saved|
+    it "invalidates an approval removed #{saved ? 'after' : 'before'} its in-flight submission is saved", :approval_reassignment do
+      within(".review-tray") { click_button "Approve", exact: true }
+      fill_in "Public note", with: "An approval sent before removal"
+      pause_review_refreshes
+      page.execute_script(<<~JS, saved)
+        const holdAfterSave = arguments[0];
+        const originalFetch = window.fetch.bind(window);
+        window.reviewWrites = [];
+        window.fetch = async (...args) => {
+          if (args[1]?.method === "POST" && String(args[0]).endsWith("/respond")) {
+            window.reviewWrites.push({ url: String(args[0]), key: JSON.parse(args[1].body).review_response.idempotency_key });
+            if (window.reviewWrites.length === 1) {
+              const result = holdAfterSave ? await originalFetch(...args) : null;
+              document.documentElement.dataset.reviewWriteHeld = "true";
+              await new Promise(resolve => { window.releaseReviewWrite = resolve; });
+              if (result) return result;
+            }
+          }
+          return originalFetch(...args);
+        };
+      JS
+      click_button "Approve v1"
+      expect(page).to have_css('html[data-review-write-held="true"]')
+      CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+      fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true)
+      refresh_review_controls
+      page.execute_script("window.releaseReviewWrite()")
+      expect(page).to have_no_css('.review-confirmation form[aria-busy="true"]')
+      expect(page).to have_content("This review request changed.")
+      expect(page).to have_button("Approve v1", disabled: true)
+      expect(page).to have_field("Public note", with: "An approval sent before removal")
+      expect(fresh.reload.status).to eq("pending")
+      expect(fresh.review_responses).to be_empty
+      click_button "Close approval", enable_aria_label: true
+      within(".review-tray") { click_button "Approve", exact: true }
+      click_button "Approve v1"
+      expect(page).to have_no_css(".review-confirmation[open]")
+      writes = page.evaluate_script("window.reviewWrites")
+      expect(writes.map { |write| URI(write.fetch("url")).path }).to eq([
+        respond_plan_review_request_path(plan, task), respond_plan_review_request_path(plan, fresh)
+      ])
+      expect(writes.map { |write| write.fetch("key") }.uniq.size).to eq(2)
+      expect(fresh.review_responses.count).to eq(1)
+      expect(fresh.reload.status).to eq("approved")
+    end
+  end
+
   [ [ "light", false ], [ "dark", true ] ].each do |theme, legacy|
     it "updates a surviving note's label after #{legacy ? 'legacy replacement' : 'removal'} without navigation", :review_readdition do
       CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved", note: "Keep the audit trail.")

From efed4fe93f0408a2f5d973d8b341b0df03c83d04 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Mon, 2 Nov 2026 16:23:31 -0500
Subject: [PATCH 08/10] Ignore results from removed review requests

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 .../coplan/review_tray_controller.js          |  43 +++++--
 engine/app/services/coplan/reviews/changed.rb |   8 +-
 spec/system/plan_reviews_spec.rb              | 115 ++++++++++++++++++
 3 files changed, 154 insertions(+), 12 deletions(-)

diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index 87f12d21..c617134c 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -11,7 +11,9 @@ export default class extends Controller {
     // Shared streams carry an invalidation signal, never another viewer's
     // forms, tokens or personal task status.
     window.Turbo.StreamActions["coplan-review-changed"] ||= function () {
-      document.getElementById(this.getAttribute("target"))?.dispatchEvent(new CustomEvent("coplan:reviews-changed"))
+      document.getElementById(this.getAttribute("target"))?.dispatchEvent(new CustomEvent("coplan:reviews-changed", {
+        detail: { removedRequestId: this.getAttribute("data-removed-review-request-id") }
+      }))
     }
     const tray = this.element.querySelector(".review-tray")
     if (tray) tray.showPopover()
@@ -162,6 +164,12 @@ export default class extends Controller {
     if (this.responseIntents[decision] === intent) delete this.responseIntents[decision]
   }
 
+  responseIsCurrent(url) {
+    const parsed = new URL(url, document.baseURI)
+    return !this.reviewWithdrawn && this.hasCommentFormTarget && this.commentFormTarget.action === parsed.href &&
+      !this.removedRequestIds?.has(parsed.pathname.split("/").at(-2))
+  }
+
   async approve(event) {
     event.preventDefault()
     if (this.approvalInvalidated) { this.invalidateApproval(); return }
@@ -170,10 +178,12 @@ export default class extends Controller {
       const intent = this.responseIntent(url, this.approvalRevision, "approved", this.approvalNoteTarget.value)
       const result = await this.write(url, "POST", { review_response: { revision: this.approvalRevision, decision: "approved", note: this.approvalNoteTarget.value, idempotency_key: intent.key } })
       this.confirmIntent("approved", intent)
-      const synced = !result.comment_thread_id || await this.refreshThreads(result.comment_thread_id)
+      const current = () => !this.approvalInvalidated && this.responseUrl === url && this.responseIsCurrent(url)
+      if (!current()) return
+      const synced = !result.comment_thread_id || await this.refreshThreads(result.comment_thread_id, current)
       // Removal can also arrive after the server saved the old approval but
       // before its result and comments arrive. Keep the dialog and its note.
-      if (this.approvalInvalidated || this.responseUrl !== url) return
+      if (!current()) return
       this.approvalTarget.close()
       this.approvalNoteTarget.value = ""
       if (this.hasNoticeTarget) this.noticeTarget.textContent = `Your approval of v${this.approvalRevision} is recorded.` + (synced ? "" : " Refresh the page to see your comment.")
@@ -193,11 +203,15 @@ export default class extends Controller {
     if (form.closest('[data-dictation-busy="true"]') || form.querySelector('[data-dictation-busy="true"]')) return
     const error = form.querySelector('[role="alert"]')
     await this.withBusy(form, error, async () => {
-      const intent = this.responseIntent(form.action, this.changesRevision, "changes_requested", input.value)
-      const result = await this.write(form.action, "POST", { review_response: { revision: this.changesRevision, decision: "changes_requested", note: input.value, idempotency_key: intent.key } })
+      const url = form.action
+      const current = () => form.isConnected && this.responseIsCurrent(url)
+      const intent = this.responseIntent(url, this.changesRevision, "changes_requested", input.value)
+      const result = await this.write(url, "POST", { review_response: { revision: this.changesRevision, decision: "changes_requested", note: input.value, idempotency_key: intent.key } })
       this.confirmIntent("changes_requested", intent)
+      if (!current()) return
       // Feedback lives in the ordinary discussion collection.
-      const synced = await this.refreshThreads(result.comment_thread_id)
+      const synced = await this.refreshThreads(result.comment_thread_id, current)
+      if (!current()) return
       this.noticeTarget.textContent = "Changes requested. Your feedback is in the plan’s comments." + (synced ? "" : " Refresh the page to see your comment.")
       input.value = ""
       this.collapse()
@@ -205,13 +219,17 @@ export default class extends Controller {
     }, "Posting…")
   }
 
-  async refreshThreads(threadId) {
+  async refreshThreads(threadId, current = () => true) {
+    // Another reviewer's removal can also make this collection snapshot stale.
+    const removalSequence = this.removalSequence
     const editor = document.querySelector('[data-coplan--inline-editor-comments-url-value]')
     if (!editor) return false
     try {
       const response = await fetch(editor.getAttribute("data-coplan--inline-editor-comments-url-value"), { credentials: "same-origin", headers: { Accept: "text/html" } })
       if (response.ok) {
-        const incoming = new DOMParser().parseFromString(await response.text(), "text/html")
+        const html = await response.text()
+        if (!current() || removalSequence !== this.removalSequence) return false
+        const incoming = new DOMParser().parseFromString(html, "text/html")
         const id = `comment_thread_${threadId}`
         const thread = incoming.getElementById(id)
         if (thread && !document.getElementById(id)) document.getElementById("plan-threads")?.append(thread)
@@ -227,7 +245,14 @@ export default class extends Controller {
     return false
   }
 
-  async refresh() {
+  async refresh(event) {
+    const removedId = event?.detail?.removedRequestId
+    if (removedId) {
+      this.removalSequence = (this.removalSequence || 0) + 1
+      this.removedRequestIds ||= new Set()
+      this.removedRequestIds.add(removedId)
+      if (this.approvalTarget.open && this.responseUrl?.endsWith(`/${removedId}/respond`)) this.invalidateApproval()
+    }
     const sequence = ++this.refreshSequence
     try {
       const response = await fetch(this.urlValue, { credentials: "same-origin", headers: { Accept: "text/html" } })
diff --git a/engine/app/services/coplan/reviews/changed.rb b/engine/app/services/coplan/reviews/changed.rb
index 950d9b92..f3119dda 100644
--- a/engine/app/services/coplan/reviews/changed.rb
+++ b/engine/app/services/coplan/reviews/changed.rb
@@ -14,15 +14,17 @@ def call
         AgentEvents::Publish.call(plan: request.plan, event_type: event_type, actor_token_id: actor_token_id,
           payload: { "review_request_id" => request.id, "reviewer_id" => request.reviewer_id }.merge(payload))
         Notifications::BroadcastBadges.call(user_ids: [ request.reviewer_id, request.plan.created_by_user_id ])
+        # No viewer-relative HTML or CSRF tokens on a shared stream. Announce
+        # removals before replacing comments so late HTTP results cannot put
+        # deleted decision labels back while the controls fetch is pending.
+        attrs = payload["removed"] ? { "data-removed-review-request-id" => request.id } : {}
+        Broadcaster.custom_action_to(request.plan, action: "coplan-review-changed", target: "plan-review-controls", html: "", attrs: attrs)
         if payload["removed"]
           # Public notes survive removal; their deleted review decisions must
           # disappear from the discussion labels in open tabs as well.
           Broadcaster.replace_to(request.plan, target: "plan-general-comment-list", partial: "coplan/plans/general_comment_list",
             locals: { threads: request.plan.comment_threads.with_kept_comments.includes(:comments).order(:created_at) })
         end
-        # No viewer-relative HTML or CSRF tokens on a shared stream. Each
-        # open tab fetches its own controls, preserving its current draft.
-        Broadcaster.custom_action_to(request.plan, action: "coplan-review-changed", target: "plan-review-controls", html: "")
       end
     end
   end
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 5ddb8f1a..76b12d26 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -53,6 +53,36 @@ def refresh_review_controls
     page.driver.browser.execute_async_script("window.refreshReviewControls().then(arguments[0])")
   end
 
+  def hold_review_delivery(stage, controls: false)
+    page.execute_script(<<~JS, stage, controls)
+      const stage = arguments[0], blockControls = arguments[1];
+      const originalFetch = window.fetch.bind(window);
+      const controlsUrl = document.getElementById("plan-review-controls").getAttribute("data-coplan--review-tray-url-value");
+      const commentsUrl = document.querySelector('[data-coplan--inline-editor-comments-url-value]').getAttribute("data-coplan--inline-editor-comments-url-value");
+      let held = false;
+      const controls = [];
+      window.blockReviewControls = blockControls;
+      window.releaseReviewControls = () => {
+        window.blockReviewControls = false;
+        controls.splice(0).forEach(resolve => resolve());
+      };
+      window.fetch = async (...args) => {
+        if (window.blockReviewControls && String(args[0]) === controlsUrl && !args[1]?.method) {
+          await new Promise(resolve => controls.push(resolve));
+        }
+        const response = await originalFetch(...args);
+        const selected = stage === "result" ? args[1]?.method === "POST" && String(args[0]).endsWith("/respond") : String(args[0]) === commentsUrl;
+        if (!held && selected) {
+          held = true;
+          window.heldReviewDelivery = await response.clone().text();
+          document.documentElement.dataset.reviewDeliveryHeld = "true";
+          await new Promise(resolve => { window.releaseReviewDelivery = resolve; });
+        }
+        return response;
+      };
+    JS
+  end
+
   before { sign_in_browser(reviewer, open_sections: !RSpec.current_example.metadata[:collapsed_support]) }
 
   it "keeps an API-withdrawn request between its two request cycles", :review_withdrawals do
@@ -319,6 +349,91 @@ def refresh_review_controls
     end
   end
 
+  it "preserves feedback when a saved result arrives after reassignment", :review_delivery do
+    expect(page).to have_css("turbo-cable-stream-source[connected]", visible: :all)
+    hold_review_delivery("result", controls: true)
+    within(".review-tray") { click_button "Request changes", exact: true }
+    fill_in "Public review comment", with: "Keep this feedback for the new request", enable_aria_label: true
+    within(".review-tray__discussion") { click_button "Request changes" }
+    expect(page).to have_css('html[data-review-delivery-held="true"]')
+    expect(task.reload.status).to eq("changes_requested")
+    CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+    fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true)
+    expect(page).to have_css(".document-comment__type", text: "Plan comment")
+    page.execute_script("window.releaseReviewDelivery()")
+    expect(page).to have_no_css('.review-tray__discussion form[aria-busy="true"]')
+    expect(page).to have_field("Public review comment", with: "Keep this feedback for the new request", enable_aria_label: true)
+    expect(page).to have_css(".review-tray__discussion", visible: true)
+    expect(page).to have_no_content("Changes requested. Your feedback is in the plan’s comments.")
+    expect(fresh.reload.status).to eq("pending")
+    page.execute_script("window.releaseReviewControls()")
+    expect(page).to have_css("[data-review-decision-actions] button[data-url*='#{fresh.id}']", visible: :all)
+    within(".review-tray__discussion") { click_button "Request changes" }
+    expect(page).to have_content("Changes requested. Your feedback is in the plan’s comments.")
+    expect(fresh.reload.status).to eq("changes_requested")
+  end
+
+  %w[approved changes_requested].each do |decision|
+    it "discards stale #{decision} comment HTML while replacement controls are delayed", :review_delivery do
+      expect(page).to have_css("turbo-cable-stream-source[connected]", visible: :all)
+      hold_review_delivery("comments", controls: true)
+      if decision == "approved"
+        within(".review-tray") { click_button "Approve", exact: true }
+        fill_in "Public note", with: "Keep the public note"
+        click_button "Approve v1"
+      else
+        within(".review-tray") { click_button "Request changes", exact: true }
+        fill_in "Public review comment", with: "Keep the public note", enable_aria_label: true
+        within(".review-tray__discussion") { click_button "Request changes" }
+      end
+      expect(page).to have_css('html[data-review-delivery-held="true"]')
+      expect(page.evaluate_script("window.heldReviewDelivery")).to include("Review · #{decision.humanize} · v1")
+      CoPlan::Reviews::Remove.call(plan: plan, request_id: task.id)
+      fresh = CoPlan::Reviews::Request.call(plan: plan, reviewer: reviewer, actor: author, create_only: true)
+      expect(page).to have_css(".document-comment__type", text: "Plan comment")
+      page.execute_script("window.releaseReviewDelivery()")
+      expect(page).to have_no_css('#plan-review-controls form[aria-busy="true"]', visible: :all)
+      expect(page).to have_css(".document-comment__type", text: "Plan comment")
+      expect(page).to have_no_css(".document-comment__type", text: "Review ·")
+      if decision == "approved"
+        expect(page).to have_css(".review-confirmation[open]", text: "This review request changed.")
+        expect(page).to have_field("Public note", with: "Keep the public note")
+        expect(page).to have_button("Approve v1", disabled: true)
+        click_button "Close approval", enable_aria_label: true
+      else
+        expect(page).to have_field("Public review comment", with: "Keep the public note", enable_aria_label: true)
+        expect(page).to have_css(".review-tray__discussion", visible: true)
+      end
+      expect(fresh.reload.status).to eq("pending")
+      expect(fresh.review_responses).to be_empty
+      page.execute_script("window.releaseReviewControls()")
+      expect(page).to have_css("[data-review-decision-actions] button[data-url*='#{fresh.id}']", visible: :all)
+    end
+  end
+
+  it "does not restore another reviewer's deleted label from a delayed comments response", :review_delivery do
+    other = create(:coplan_user)
+    other_task = CoPlan::Reviews::Request.call(plan: plan, reviewer: other, actor: author)
+    CoPlan::Reviews::Respond.call(request: other_task, actor: other, revision: 1, decision: "approved", note: "Other reviewer approval")
+    expect(page).to have_css(".document-comment", text: "Other reviewer approval")
+    hold_review_delivery("comments", controls: true)
+    within(".review-tray") { click_button "Approve", exact: true }
+    fill_in "Public note", with: "My current approval"
+    click_button "Approve v1"
+    expect(page).to have_css('html[data-review-delivery-held="true"]')
+    CoPlan::Reviews::Remove.call(plan: plan, request_id: other_task.id)
+    within(find(".document-comment", text: "Other reviewer approval")) do
+      expect(page).to have_css(".document-comment__type", text: "Plan comment")
+    end
+    page.execute_script("window.releaseReviewDelivery(); window.releaseReviewControls()")
+    expect(page).to have_no_css(".review-confirmation[open]")
+    within(find(".document-comment", text: "Other reviewer approval")) do
+      expect(page).to have_css(".document-comment__type", text: "Plan comment")
+    end
+    expect(task.reload.status).to eq("approved")
+    expect(task.review_responses.last!.note).to eq("My current approval")
+  end
+
   [ [ "light", false ], [ "dark", true ] ].each do |theme, legacy|
     it "updates a surviving note's label after #{legacy ? 'legacy replacement' : 'removal'} without navigation", :review_readdition do
       CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved", note: "Keep the audit trail.")

From 910ac4f3c7e574816dd154f0ce8a0ba127b4d3e3 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Mon, 2 Nov 2026 16:37:33 -0500
Subject: [PATCH 09/10] Refresh discussions after reviewer management

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 .../coplan/review_tray_controller.js          |  5 ++-
 spec/system/plan_reviews_spec.rb              | 32 +++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index c617134c..b57210d6 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -322,6 +322,7 @@ export default class extends Controller {
       event.target.querySelector('[type="checkbox"]').checked = true
       this.addReviewerButtonTarget.focus({ preventScroll: true })
       await this.refresh()
+      await this.refreshThreads(null)
     }, "Requesting…")
     this.assignButtonTarget.disabled = !this.reviewerIdTarget.value
   }
@@ -421,7 +422,9 @@ export default class extends Controller {
     try {
       await this.withBusy(button, this.managementErrorTarget, async () => {
         await this.write(button.dataset.url, method, body)
-        await this.refresh()
+        const removedId = method === "DELETE" && body?.review_request?.remove ? new URL(button.dataset.url, document.baseURI).pathname.split("/").at(-1) : null
+        await this.refresh(removedId ? { detail: { removedRequestId: removedId } } : undefined)
+        if (removedId) await this.refreshThreads(null)
       }, "Updating…")
     } finally {
       this.managing = false
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 76b12d26..6b34840b 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -194,6 +194,38 @@ def hold_review_delivery(stage, controls: false)
     expect([ plan.plan_events.where(event_type: "review_requested").count, CoPlan::Notification.count ]).to eq(counts)
   end
 
+  [ false, true ].each do |legacy|
+    it "updates discussion labels after #{legacy ? 'legacy reassignment' : 'removal'} without ActionCable", :review_offline do
+      CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved", note: "Retain this public discussion")
+      CoPlan::Reviews::Withdraw.call(request: task, actor: author) if legacy
+      sign_in_browser(author)
+      expect(page).to have_css(".document-comment__type", text: "Review · Approved · v1")
+      page.execute_script('document.querySelectorAll("turbo-cable-stream-source").forEach(source => source.remove())')
+      expect(page).to have_no_css("turbo-cable-stream-source", visible: :all)
+      click_button "More actions"
+      within("#plan-menu") { click_button "Manage reviewers" }
+      within(".review-management[open]") do
+        if legacy
+          click_button "Add reviewer", exact: true
+        else
+          click_button "Actions for Plan Reviewer", enable_aria_label: true
+          click_button "Remove reviewer"
+          expect(page).to have_no_css("[data-review-assignments] .review-person")
+        end
+      end
+      if legacy
+        fill_in "Find a reviewer", with: reviewer.name
+        find(".review-search-result", text: reviewer.name).click
+        within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+        expect(page).to have_css(".review-state", text: "Pending")
+      end
+      expect(page).to have_css(".document-comment__type", text: "Plan comment")
+      expect(page).to have_no_css(".document-comment__type", text: "Review · Approved")
+      within(".review-management[open]") { click_button "Close reviews", enable_aria_label: true }
+      expect(page).to have_content("Retain this public discussion")
+    end
+  end
+
   [ [ "light", false ], [ "dark", true ] ].each do |theme, separate_refresh|
     it "submits a preserved draft to the replacement after #{separate_refresh ? 'separate' : 'coalesced'} refreshes", :review_readdition do
       page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)

From 9ca85272e42e6e6e4f62ea7ea6176a021aa738c8 Mon Sep 17 00:00:00 2001
From: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
Date: Mon, 2 Nov 2026 16:48:37 -0500
Subject: [PATCH 10/10] Ignore review refreshes after navigation

Signed-off-by: Sean Prashad <13009507+seanprashad@users.noreply.github.com>
---
 .../coplan/review_tray_controller.js          |  3 +-
 spec/system/plan_reviews_spec.rb              | 46 +++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js
index b57210d6..a59b0d26 100644
--- a/engine/app/javascript/controllers/coplan/review_tray_controller.js
+++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js
@@ -220,6 +220,7 @@ export default class extends Controller {
   }
 
   async refreshThreads(threadId, current = () => true) {
+    if (!this.element.isConnected) return false
     // Another reviewer's removal can also make this collection snapshot stale.
     const removalSequence = this.removalSequence
     const editor = document.querySelector('[data-coplan--inline-editor-comments-url-value]')
@@ -228,7 +229,7 @@ export default class extends Controller {
       const response = await fetch(editor.getAttribute("data-coplan--inline-editor-comments-url-value"), { credentials: "same-origin", headers: { Accept: "text/html" } })
       if (response.ok) {
         const html = await response.text()
-        if (!current() || removalSequence !== this.removalSequence) return false
+        if (!this.element.isConnected || !current() || removalSequence !== this.removalSequence) return false
         const incoming = new DOMParser().parseFromString(html, "text/html")
         const id = `comment_thread_${threadId}`
         const thread = incoming.getElementById(id)
diff --git a/spec/system/plan_reviews_spec.rb b/spec/system/plan_reviews_spec.rb
index 6b34840b..f91bf6d4 100644
--- a/spec/system/plan_reviews_spec.rb
+++ b/spec/system/plan_reviews_spec.rb
@@ -226,6 +226,52 @@ def hold_review_delivery(stage, controls: false)
     end
   end
 
+  [ false, true ].each do |legacy|
+    it "does not apply a delayed #{legacy ? 'reassignment' : 'removal'} discussion refresh to another plan", :review_navigation do
+      CoPlan::Reviews::Respond.call(request: task, actor: reviewer, revision: 1, decision: "approved", note: "Plan A review discussion")
+      CoPlan::Reviews::Withdraw.call(request: task, actor: author) if legacy
+      destination = create(:plan, :published, created_by_user: author, title: "Destination plan")
+      thread = create(:comment_thread, plan: destination, created_by_user: author)
+      create(:comment, comment_thread: thread, author_id: author.id, body_markdown: "Plan B discussion stays here")
+      sign_in_browser(author)
+      hold_review_delivery("comments")
+      page.execute_script(<<~JS)
+        const root = document.getElementById("plan-review-controls");
+        const controller = window.Stimulus.getControllerForElementAndIdentifier(root, "coplan--review-tray");
+        const refresh = controller.refreshThreads.bind(controller);
+        controller.refreshThreads = async (...args) => {
+          const result = await refresh(...args);
+          document.documentElement.dataset.oldReviewFinished = "true";
+          return result;
+        };
+      JS
+      click_button "More actions"
+      within("#plan-menu") { click_button "Manage reviewers" }
+      within(".review-management[open]") do
+        if legacy
+          click_button "Add reviewer", exact: true
+        else
+          click_button "Actions for Plan Reviewer", enable_aria_label: true
+          click_button "Remove reviewer"
+        end
+      end
+      if legacy
+        fill_in "Find a reviewer", with: reviewer.name
+        find(".review-search-result", text: reviewer.name).click
+        within(".review-assignment-form") { click_button "Add reviewer", exact: true }
+      end
+      expect(page).to have_css('html[data-review-delivery-held="true"]')
+      expect(page.evaluate_script("window.heldReviewDelivery")).to include("Plan A review discussion")
+      page.execute_script("window.Turbo.visit(arguments[0])", plan_page_path(destination))
+      expect(page).to have_current_path(plan_page_path(destination))
+      expect(page).to have_css("#plan-general-comment-list", text: "Plan B discussion stays here", visible: :all)
+      page.execute_script("window.releaseReviewDelivery()")
+      expect(page).to have_css('html[data-old-review-finished="true"]')
+      expect(page).to have_css("#plan-general-comment-list", text: "Plan B discussion stays here", visible: :all)
+      expect(page).to have_no_css("#plan-general-comment-list", text: "Plan A review discussion", visible: :all)
+    end
+  end
+
   [ [ "light", false ], [ "dark", true ] ].each do |theme, separate_refresh|
     it "submits a preserved draft to the replacement after #{separate_refresh ? 'separate' : 'coalesced'} refreshes", :review_readdition do
       page.execute_script("document.documentElement.dataset.theme = arguments[0]", theme)