Skip to content

Commit d64fe4a

Browse files
dblockCopilot
andcommitted
Handle revoked installations in status checks
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5f688b3 commit d64fe4a

4 files changed

Lines changed: 70 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
### 2.2.2 (Next)
44

5+
* [#185](https://github.com/slack-ruby/slack-ruby-bot-server/pull/185): Skip inactive installations in status checks and deactivate revoked credentials through service callbacks, returning an explicit failed ping instead of failing the whole status response - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent).
56
* Your contribution here.
67

78
### 2.2.1 (2026/10/24)

‎UPGRADING.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
Upgrading Slack-Ruby-Bot-Server
22
===============================
33

4+
### Upgrading to >= 2.3.0
5+
6+
#### Status Checks and Inactive Installations
7+
8+
`GET /api/status` now pings only active installations. When Slack returns `account_inactive` or `invalid_auth`, the installation is deactivated through `Service#deactivate!`, and the endpoint returns HTTP `200` with `ping: { "ok": false, "error": "account_inactive" }` (or `"invalid_auth"`). The active-team count reflects that deactivation. Monitoring should inspect the failed `ping` as well as the HTTP status.
9+
10+
When no active installations remain, no Slack ping is attempted. Other Slack errors retain their existing error response and do not deactivate installations.
11+
412
### Upgrading to >= 2.2.0
513

614
#### Cursor Helper Iterator Changes

‎lib/slack-ruby-bot-server/api/presenters/status_presenter.rb‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,27 @@ module StatusPresenter
1010
"#{base_url(opts)}/status"
1111
end
1212

13+
property :ping
1314
property :teams_count
1415
property :active_teams_count
15-
property :ping
1616

1717
def ping
1818
if SlackRubyBotServer::Config.mongoid?
19-
team = Team.asc(:_id).first
19+
team = Team.active.asc(:_id).first
2020
elsif SlackRubyBotServer::Config.activerecord?
21-
team = Team.last
21+
team = Team.active.last
2222
else
2323
raise 'Unsupported database driver.'
2424
end
2525
return unless team
2626

2727
team.ping!
28+
rescue Slack::Web::Api::Errors::SlackError => e
29+
raise unless %w[account_inactive invalid_auth].include?(e.message)
30+
31+
Middleware.logger.warn "Status ping failed for #{team}: #{e.message}; deactivating installation."
32+
Service.instance.deactivate!(team)
33+
{ ok: false, error: e.message }
2834
end
2935

3036
def teams_count

‎spec/api/endpoints/status_endpoint_spec.rb‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,65 @@
1313
expect(status.teams_count).to eq 0
1414
end
1515

16+
it 'selects an active installation even when inactive records surround it' do
17+
Fabricate(:team, active: false)
18+
active = Fabricate(:team, active: true)
19+
Fabricate(:team, active: false)
20+
allow_any_instance_of(Team).to receive(:ping!) do |record|
21+
expect(record.id).to eq active.id
22+
{ ok: 1 }
23+
end
24+
expect(client.status.ping['ok']).to eq 1
25+
end
26+
1627
context 'with a team' do
17-
let!(:team) { Fabricate(:team, active: false) }
28+
let!(:team) { Fabricate(:team, active: true) }
1829
it 'returns a status with ping' do
1930
status = client.status
2031
expect(status.teams_count).to eq 1
2132
ping = status.ping
2233
expect(ping['ok']).to eq 1
2334
end
35+
36+
%w[account_inactive invalid_auth].each do |error|
37+
context "when Slack returns #{error}" do
38+
before do
39+
allow_any_instance_of(Team).to receive(:ping!).and_raise(Slack::Web::Api::Errors::SlackError, error)
40+
end
41+
42+
it 'deactivates the installation and returns an explicit failed ping' do
43+
expect(SlackRubyBotServer::Service.instance).to receive(:deactivate!).with(instance_of(Team)).and_call_original
44+
get '/api/status'
45+
status = JSON.parse(last_response.body)
46+
expect(last_response.status).to eq 200
47+
expect(status['ping']).to eq('ok' => false, 'error' => error)
48+
expect(status['active_teams_count']).to eq 0
49+
expect(team.reload.active?).to be false
50+
end
51+
end
52+
end
53+
54+
it 'does not hide unrelated Slack failures or deactivate the installation' do
55+
allow_any_instance_of(Team).to receive(:ping!).and_raise(Slack::Web::Api::Errors::SlackError, 'ratelimited')
56+
get '/api/status'
57+
expect(last_response.status).to eq 400
58+
expect(JSON.parse(last_response.body)['message']).to eq 'ratelimited'
59+
expect(team.reload.active?).to be true
60+
end
61+
end
62+
63+
context 'with only inactive teams' do
64+
before { Fabricate(:team, active: false) }
65+
66+
it 'does not ping revoked credentials' do
67+
expect_any_instance_of(Team).not_to receive(:ping!)
68+
get '/api/status'
69+
status = JSON.parse(last_response.body)
70+
expect(last_response.status).to eq 200
71+
expect(status['ping']).to be_nil
72+
expect(status['teams_count']).to eq 1
73+
expect(status['active_teams_count']).to eq 0
74+
end
2475
end
2576
end
2677
end

0 commit comments

Comments
 (0)