fix(agent-bots): destroy permissibles on AgentBot deletion and skip orphans in index (#14273)

\`GET /platform/api/v1/agent_bots\` returns 500 when any \`AgentBot\`
that was previously registered with a Platform App has since been
deleted. The bug was introduced by a missing \`dependent: :destroy\` on
the \`AgentBot\` model — deleting a bot left orphaned rows in
\`platform_app_permissibles\`, which the index action later iterated
over and crashed rendering with a \`NoMethodError\` on \`nil\`.

Closes #13407

## Root cause

The index action loads all \`platform_app_permissibles\` for the
platform app and passes each \`resource.permissible\` (the associated
\`AgentBot\`) to a Jbuilder partial. When the \`AgentBot\` no longer
exists, \`resource.permissible\` returns \`nil\` and the partial crashes
calling \`.id\`, \`.name\`, etc. on it.

Every other \`AgentBot\` association (\`agent_bot_inboxes\`,
\`messages\`, \`assigned_conversations\`) had a \`dependent:\` option —
\`platform_app_permissibles\` was the only one missing it. There was
also an N+1 query: the index fired a separate SQL query per permissible
to load each bot.

## What changed

**1. Model — prevent orphans at deletion time**
\`\`\`ruby
has_many :platform_app_permissibles, as: :permissible, dependent:
:destroy
\`\`\`

**2. Controller — eager-load to eliminate N+1**
\`\`\`ruby
@resources = @platform_app.platform_app_permissibles
               .where(permissible_type: 'AgentBot')
               .includes(:permissible)
\`\`\`

**3. Jbuilder — defensive nil guard for pre-existing orphans**
\`\`\`ruby
bot = resource.permissible
next if bot.nil?
json.partial! '...', resource: bot
\`\`\`

## Trade-offs considered

| Option | Decision |
|---|---|
| Rescue \`NoMethodError\` in jbuilder | Hides the failure rather than
fixing it. Rejected. |
| Only add the nil guard, skip the model fix | Leaves the data integrity
gap open — future deletions continue creating orphans. Rejected. |
| Both layers (chosen) | Model fix prevents new orphans; nil guard is
defence-in-depth for any orphans that survived before deployment. |
| \`dependent: :nullify\` | Doesn't apply — a nullified permissible
would still cause the same nil dereference. Rejected. |

## How to reproduce

1. Create an AgentBot via the Platform API
2. Delete the AgentBot via any path (admin UI, API, or direct model
call)
3. Call \`GET /platform/api/v1/agent_bots\` with a Platform App token
4. Observe 500

After this fix, the endpoint returns 200 with an empty array.

Co-authored-by: Ramalau Debeila <rdebeila@datacentrix.co.za>
This commit is contained in:
ramalau 2026-04-27 15:47:32 +02:00 committed by GitHub
parent 16b8693e1b
commit 035d2858f5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 27 additions and 2 deletions

View File

@ -3,7 +3,7 @@ class Platform::Api::V1::AgentBotsController < PlatformController
before_action :validate_platform_app_permissible, except: [:index, :create]
def index
@resources = @platform_app.platform_app_permissibles.where(permissible_type: 'AgentBot').all
@resources = @platform_app.platform_app_permissibles.where(permissible_type: 'AgentBot').includes(:permissible)
end
def show; end

View File

@ -32,6 +32,7 @@ class AgentBot < ApplicationRecord
has_many :agent_bot_inboxes, dependent: :destroy_async
has_many :inboxes, through: :agent_bot_inboxes
has_many :messages, as: :sender, dependent: :nullify
has_many :platform_app_permissibles, as: :permissible, dependent: :destroy
has_many :assigned_conversations, class_name: 'Conversation',
foreign_key: :assignee_agent_bot_id,
dependent: :nullify,

View File

@ -1,3 +1,6 @@
json.array! @resources do |resource|
json.partial! 'platform/api/v1/models/agent_bot', formats: [:json], resource: resource.permissible
bot = resource.permissible
next if bot.nil?
json.partial! 'platform/api/v1/models/agent_bot', formats: [:json], resource: bot
end

View File

@ -39,6 +39,19 @@ RSpec.describe 'Platform Agent Bot API', type: :request do
expect(data.length).to eq(1)
expect(data.first['outgoing_url']).to eq(agent_bot.outgoing_url)
end
it 'returns 200 and skips orphaned permissibles when an agent bot has been deleted' do
create(:platform_app_permissible, platform_app: platform_app, permissible: agent_bot)
# Use delete (not destroy!) to bypass dependent: :destroy callbacks so the
# permissible row survives — exactly the orphan scenario described in the issue.
agent_bot.delete
get '/platform/api/v1/agent_bots',
headers: { api_access_token: platform_app.access_token.token }, as: :json
expect(response).to have_http_status(:success)
expect(response.parsed_body).to be_empty
end
end
end

View File

@ -6,6 +6,7 @@ RSpec.describe AgentBot do
describe 'associations' do
it { is_expected.to have_many(:agent_bot_inboxes) }
it { is_expected.to have_many(:inboxes) }
it { is_expected.to have_many(:platform_app_permissibles) }
end
describe 'concerns' do
@ -38,6 +39,13 @@ RSpec.describe AgentBot do
expect(message.reload.sender).to be_nil
end
it 'destroys associated platform_app_permissibles' do
platform_app = create(:platform_app)
create(:platform_app_permissible, platform_app: platform_app, permissible: agent_bot)
expect { agent_bot.destroy! }.to change(PlatformAppPermissible, :count).by(-1)
end
end
describe '#system_bot?' do