Rework FEP-7888 implementation (#36064)

app/controllers/activitypub/contexts_controller.rb

@@ -26,7 +26,8 @@ class ActivityPub::ContextsController < ActivityPub::BaseController
   end
 
   def set_conversation
-    @conversation = Conversation.local.find(params[:id])
+    account_id, status_id = params[:id].split('-')
+    @conversation = Conversation.local.find_by(parent_account_id: account_id, parent_status_id: status_id)
   end
 
   def set_items

app/lib/activitypub/tag_manager.rb

@@ -41,7 +41,7 @@ class ActivityPub::TagManager
     when :person
       target.instance_actor? ? instance_actor_url : account_url(target)
     when :conversation
-      context_url(target)
+      context_url(target) unless target.parent_account_id.nil? || target.parent_status_id.nil?
     when :note, :comment, :activity
       return activity_account_status_url(target.account, target) if target.reblog?
 

app/models/conversation.rb

@@ -24,6 +24,10 @@ class Conversation < ApplicationRecord
 
   before_validation :set_parent_account, on: :create
 
+  def to_param
+    "#{parent_account_id}-#{parent_status_id}" unless parent_account_id.nil? || parent_status_id.nil?
+  end
+
   def local?
     uri.nil?
   end

app/presenters/activitypub/context_presenter.rb

@@ -7,7 +7,7 @@ class ActivityPub::ContextPresenter < ActiveModelSerializers::Model
     def from_conversation(conversation)
       new.tap do |presenter|
         presenter.id = ActivityPub::TagManager.instance.uri_for(conversation)
-        presenter.attributed_to = ActivityPub::TagManager.instance.uri_for(conversation.parent_account)
+        presenter.attributed_to = ActivityPub::TagManager.instance.uri_for(conversation.parent_account) if conversation.parent_account.present?
       end
     end
   end

app/serializers/activitypub/note_serializer.rb

@@ -2,6 +2,7 @@
 
 class ActivityPub::NoteSerializer < ActivityPub::Serializer
   include FormattingHelper
+  include JsonLdHelper
 
   context_extensions :atom_uri, :conversation, :sensitive, :voters_count, :quotes, :interaction_policies
 
@@ -159,14 +160,16 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
     if object.conversation.uri?
       object.conversation.uri
     else
-      OStatus::TagManager.instance.unique_tag(object.conversation.created_at, object.conversation.id, 'Conversation')
+      # This means `parent_status_id` and `parent_account_id` must *not* get backfilled
+      ActivityPub::TagManager.instance.uri_for(object.conversation) || OStatus::TagManager.instance.unique_tag(object.conversation.created_at, object.conversation.id, 'Conversation')
     end
   end
 
   def context
     return if object.conversation.nil?
 
-    ActivityPub::TagManager.instance.uri_for(object.conversation)
+    uri = ActivityPub::TagManager.instance.uri_for(object.conversation)
+    uri unless unsupported_uri_scheme?(uri)
   end
 
   def local?

config/routes.rb

@@ -120,7 +120,7 @@ Rails.application.routes.draw do
   end
 
   resource :inbox, only: [:create], module: :activitypub
-  resources :contexts, only: [:show], module: :activitypub do
+  resources :contexts, only: [:show], module: :activitypub, constraints: { id: /[0-9]+-[0-9]+/ } do
     member do
       get :items
     end

db/migrate/20250909100506_add_index_on_parent_status_id_to_conversations.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddIndexOnParentStatusIdToConversations < ActiveRecord::Migration[8.0]
+  disable_ddl_transaction!
+
+  def change
+    add_index :conversations, :parent_status_id, algorithm: :concurrently, unique: true, where: 'parent_status_id IS NOT NULL'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2025_09_02_221600) do
+ActiveRecord::Schema[8.0].define(version: 2025_09_09_100506) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
 
@@ -361,6 +361,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_02_221600) do
     t.datetime "updated_at", precision: nil, null: false
     t.bigint "parent_status_id"
     t.bigint "parent_account_id"
+    t.index ["parent_status_id"], name: "index_conversations_on_parent_status_id", unique: true, where: "(parent_status_id IS NOT NULL)"
     t.index ["uri"], name: "index_conversations_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)"
   end
 

spec/requests/activitypub/contexts_spec.rb

@@ -3,14 +3,37 @@
 require 'rails_helper'
 
 RSpec.describe 'ActivityPub Contexts' do
-  let(:conversation) { Fabricate(:conversation) }
+  let(:conversation) { Fabricate(:status).owned_conversation }
 
   describe 'GET #show' do
-    subject { get context_path(id: conversation.id), headers: nil }
+    subject { get context_path(conversation), headers: nil }
 
     let!(:status) { Fabricate(:status, conversation: conversation) }
     let!(:unrelated_status) { Fabricate(:status) }
 
+    it 'returns http success and correct media type and correct items' do
+      subject
+
+      expect(response)
+        .to have_http_status(200)
+        .and have_cacheable_headers
+
+      expect(response.media_type)
+        .to eq 'application/activity+json'
+
+      expect(response.parsed_body[:type])
+        .to eq 'Collection'
+
+      expect(response.parsed_body[:first][:items])
+        .to be_an(Array)
+        .and have_attributes(size: 2)
+        .and include(ActivityPub::TagManager.instance.uri_for(status))
+        .and not_include(ActivityPub::TagManager.instance.uri_for(unrelated_status))
+    end
+
+    context 'when the initial account is deleted' do
+      before { conversation.parent_account.delete }
+
       it 'returns http success and correct media type and correct items' do
         subject
 
@@ -30,13 +53,12 @@ RSpec.describe 'ActivityPub Contexts' do
           .and include(ActivityPub::TagManager.instance.uri_for(status))
           .and not_include(ActivityPub::TagManager.instance.uri_for(unrelated_status))
       end
+    end
 
     context 'with pagination' do
       context 'with few statuses' do
         before do
-          3.times do
-            Fabricate(:status, conversation: conversation)
-          end
+          Fabricate.times(3, :status, conversation: conversation)
         end
 
         it 'does not include a next page link' do
@@ -48,7 +70,7 @@ RSpec.describe 'ActivityPub Contexts' do
 
       context 'with many statuses' do
         before do
-          (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do
+          ActivityPub::ContextsController::DESCENDANTS_LIMIT.times do
             Fabricate(:status, conversation: conversation)
           end
         end
@@ -63,13 +85,11 @@ RSpec.describe 'ActivityPub Contexts' do
   end
 
   describe 'GET #items' do
-    subject { get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil }
+    subject { get items_context_path(conversation, page: 0, min_id: nil), headers: nil }
 
     context 'with few statuses' do
       before do
-        3.times do
-          Fabricate(:status, conversation: conversation)
-        end
+        Fabricate.times(2, :status, conversation: conversation)
       end
 
       it 'returns http success and correct media type and correct items' do
@@ -94,9 +114,8 @@ RSpec.describe 'ActivityPub Contexts' do
 
     context 'with many statuses' do
       before do
-        (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do
-          Fabricate(:status, conversation: conversation)
-        end
+        stub_const 'ActivityPub::ContextsController::DESCENDANTS_LIMIT', 2
+        Fabricate.times(ActivityPub::ContextsController::DESCENDANTS_LIMIT, :status, conversation: conversation)
       end
 
       it 'includes a next page link' do
@@ -108,13 +127,12 @@ RSpec.describe 'ActivityPub Contexts' do
 
     context 'with page requested' do
       before do
-        (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do |_i|
-          Fabricate(:status, conversation: conversation)
-        end
+        stub_const 'ActivityPub::ContextsController::DESCENDANTS_LIMIT', 2
+        Fabricate.times(ActivityPub::ContextsController::DESCENDANTS_LIMIT, :status, conversation: conversation)
       end
 
       it 'returns the correct items' do
-        get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil
+        get items_context_path(conversation, page: 0, min_id: nil), headers: nil
         next_page = response.parsed_body['first']['next']
         get next_page, headers: nil