Store user_id as comment author_id for local_agent comments (COPLAN-33)

db/migrate/20260609185102_backfill_local_agent_comment_author_ids.co_plan.rb

@@ -0,0 +1,23 @@
+# This migration comes from co_plan (originally 20260609000000)
+class BackfillLocalAgentCommentAuthorIds < ActiveRecord::Migration[8.1]
+  # For local_agent comments whose author_id still points at a
+  # coplan_api_tokens.id, rewrite it to that token's user_id. This
+  # pairs with the model change in engine/app/models/coplan/comment.rb:
+  # Comment#author now resolves via a direct find_by(id:) for both
+  # human and local_agent comments instead of joining coplan_api_tokens.
+  #
+  # Idempotent: once author_id holds a user UUID it won't match any
+  # coplan_api_tokens.id, so re-runs skip those rows.
+  def up
+    CoPlan::Comment.where(author_type: "local_agent").find_each do |comment|
+      token = CoPlan::ApiToken.find_by(id: comment.author_id)
+      next unless token
+
+      comment.update_columns(author_id: token.user_id) # rubocop:disable Rails/SkipsModelValidations
+    end
+  end
+
+  def down
+    # no-op (not reversible)
+  end
+end

engine/app/controllers/coplan/api/v1/comments_controller.rb

@@ -19,7 +19,7 @@ def create
 
           comment = thread.comments.create!(
             author_type: api_author_type,
-            author_id: api_actor_id,
+            author_id: current_user&.id,
             body_markdown: params[:body_markdown],
             agent_name: params[:agent_name]
           )
@@ -94,7 +94,7 @@ def reply
 
           comment = thread.comments.create!(
             author_type: api_author_type,
-            author_id: api_actor_id,
+            author_id: current_user&.id,
             body_markdown: params[:body_markdown],
             agent_name: params[:agent_name]
           )

engine/app/models/coplan/comment.rb

@@ -22,13 +22,14 @@ def agent?
 
     # Resolves the comment author to a CoPlan::User instance, or nil for
     # author types that don't map to a user (cloud_persona, system).
+    # For both human and local_agent comments, author_id is the user's id
+    # (local_agent rows store the user behind the API token, not the token
+    # id), so a single find_by resolves both — agent_name distinguishes
+    # which agent posted on the user's behalf.
     def author
-      case author_type
-      when "human"
-        CoPlan::User.find_by(id: author_id)
-      when "local_agent"
-        CoPlan::User.joins(:api_tokens).where(coplan_api_tokens: { id: author_id }).first
-      end
+      return unless author_type.in?(%w[human local_agent])
+
+      CoPlan::User.find_by(id: author_id)
     end
 
     private

engine/db/migrate/20260609000000_backfill_local_agent_comment_author_ids.rb

@@ -0,0 +1,22 @@
+class BackfillLocalAgentCommentAuthorIds < ActiveRecord::Migration[8.1]
+  # For local_agent comments whose author_id still points at a
+  # coplan_api_tokens.id, rewrite it to that token's user_id. This
+  # pairs with the model change in engine/app/models/coplan/comment.rb:
+  # Comment#author now resolves via a direct find_by(id:) for both
+  # human and local_agent comments instead of joining coplan_api_tokens.
+  #
+  # Idempotent: once author_id holds a user UUID it won't match any
+  # coplan_api_tokens.id, so re-runs skip those rows.
+  def up
+    CoPlan::Comment.where(author_type: "local_agent").find_each do |comment|
+      token = CoPlan::ApiToken.find_by(id: comment.author_id)
+      next unless token
+
+      comment.update_columns(author_id: token.user_id) # rubocop:disable Rails/SkipsModelValidations
+    end
+  end
+
+  def down
+    # no-op (not reversible)
+  end
+end

spec/helpers/coplan/comments_helper_spec.rb

@@ -0,0 +1,19 @@
+require "rails_helper"
+
+RSpec.describe CoPlan::CommentsHelper, type: :helper do
+  describe "#comment_author_name" do
+    it "renders 'Agent (via User)' for a local_agent comment" do
+      user = create(:coplan_user, name: "Alice")
+      comment = create(:comment, author_type: "local_agent", agent_name: "Amp", author_id: user.id)
+
+      expect(helper.comment_author_name(comment)).to eq("Amp (via Alice)")
+    end
+
+    it "renders just the user name for a human comment" do
+      user = create(:coplan_user, name: "Bob")
+      comment = create(:comment, author_type: "human", author_id: user.id)
+
+      expect(helper.comment_author_name(comment)).to eq("Bob")
+    end
+  end
+end

spec/migrations/backfill_local_agent_comment_author_ids_spec.rb

@@ -0,0 +1,48 @@
+require "rails_helper"
+require CoPlan::Engine.root.join("db/migrate/20260609000000_backfill_local_agent_comment_author_ids.rb")
+
+RSpec.describe BackfillLocalAgentCommentAuthorIds do
+  subject(:migration) { described_class.new }
+
+  before { migration.verbose = false }
+
+  it "rewrites local_agent author_id from a token id to the token's user_id" do
+    user = create(:coplan_user)
+    token = create(:api_token, user: user)
+    comment = create(:comment, author_type: "local_agent", agent_name: "Amp", author_id: token.id)
+
+    migration.up
+
+    expect(comment.reload.author_id).to eq(user.id)
+    expect(comment.author).to eq(user)
+  end
+
+  it "leaves human comments untouched" do
+    user = create(:coplan_user)
+    comment = create(:comment, author_type: "human", author_id: user.id)
+
+    migration.up
+
+    expect(comment.reload.author_id).to eq(user.id)
+  end
+
+  it "leaves local_agent comments whose author_id is already a user_id untouched" do
+    user = create(:coplan_user)
+    comment = create(:comment, author_type: "local_agent", agent_name: "Amp", author_id: user.id)
+
+    migration.up
+
+    expect(comment.reload.author_id).to eq(user.id)
+  end
+
+  it "is idempotent across repeated runs" do
+    user = create(:coplan_user)
+    token = create(:api_token, user: user)
+    comment = create(:comment, author_type: "local_agent", agent_name: "Amp", author_id: token.id)
+
+    migration.up
+    migration.up
+
+    expect(comment.reload.author_id).to eq(user.id)
+  end
+end

spec/models/comment_spec.rb

@@ -24,6 +24,25 @@
     expect(comment.comment_thread).to eq(thread)
   end
 
+  describe "#author" do
+    it "resolves a human comment to its user via direct lookup" do
+      user = create(:coplan_user)
+      comment = create(:comment, author_type: "human", author_id: user.id)
+      expect(comment.author).to eq(user)
+    end
+
+    it "resolves a local_agent comment to its user via direct lookup (no join)" do
+      user = create(:coplan_user)
+      comment = create(:comment, author_type: "local_agent", agent_name: "Amp", author_id: user.id)
+      expect(comment.author).to eq(user)
+    end
+
+    it "returns nil for author types that don't map to a user" do
+      comment = create(:comment, author_type: "system", author_id: SecureRandom.uuid)
+      expect(comment.author).to be_nil
+    end
+  end
+
   describe "Slack notification callback" do
     let(:thread_record) { create(:comment_thread) }
 

spec/requests/api/v1/comments_spec.rb

@@ -27,6 +27,12 @@
     body = JSON.parse(response.body)
     expect(body["thread_id"]).to be_present
     expect(body["comment_id"]).to be_present
+
+    comment = CoPlan::Comment.find(body["comment_id"])
+    expect(comment.author_type).to eq("local_agent")
+    expect(comment.author_id).to eq(alice.id)
+    expect(comment.author_id).not_to eq(alice_token.id)
+    expect(comment.author).to eq(alice)
   end
 
   it "create general comment thread" do
@@ -49,6 +55,10 @@
     expect(response).to have_http_status(:created)
     body = JSON.parse(response.body)
     expect(body["thread_id"]).to eq(thread_record.id)
+
+    comment = CoPlan::Comment.find(body["comment_id"])
+    expect(comment.author_id).to eq(alice.id)
+    expect(comment.author_id).not_to eq(alice_token.id)
   end
 
   it "reply to nonexistent thread" do