diff --git a/CHANGELOG.md b/CHANGELOG.md index bc1243f..515372c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * [#183](https://github.com/slack-ruby/slack-ruby-bot-server/pull/183): Add a `config.server_pages` block to configure page middleware options centrally without changing the installed library's defaults - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent). * [#184](https://github.com/slack-ruby/slack-ruby-bot-server/pull/184): Add a configurable API application and separately mountable team read/registration endpoints, preserving the default API while supporting custom routes and registration-only applications - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent). +* [#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). * Your contribution here. ### 2.2.1 (2026/10/24) diff --git a/UPGRADING.md b/UPGRADING.md index 7ea7101..2f1e6e6 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,14 @@ Upgrading Slack-Ruby-Bot-Server =============================== +### Upgrading to >= 2.3.0 + +#### Status Checks and Inactive Installations + +`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. + +When no active installations remain, no Slack ping is attempted. Other Slack errors retain their existing error response and do not deactivate installations. + ### Upgrading to >= 2.2.0 #### Cursor Helper Iterator Changes diff --git a/lib/slack-ruby-bot-server/api/presenters/status_presenter.rb b/lib/slack-ruby-bot-server/api/presenters/status_presenter.rb index a9d7db6..edff81a 100644 --- a/lib/slack-ruby-bot-server/api/presenters/status_presenter.rb +++ b/lib/slack-ruby-bot-server/api/presenters/status_presenter.rb @@ -10,21 +10,27 @@ module StatusPresenter "#{base_url(opts)}/status" end + property :ping property :teams_count property :active_teams_count - property :ping def ping if SlackRubyBotServer::Config.mongoid? - team = Team.asc(:_id).first + team = Team.active.asc(:_id).first elsif SlackRubyBotServer::Config.activerecord? - team = Team.last + team = Team.active.last else raise 'Unsupported database driver.' end return unless team team.ping! + rescue Slack::Web::Api::Errors::SlackError => e + raise unless %w[account_inactive invalid_auth].include?(e.message) + + Middleware.logger.warn "Status ping failed for #{team}: #{e.message}; deactivating installation." + Service.instance.deactivate!(team) + { ok: false, error: e.message } end def teams_count diff --git a/spec/api/endpoints/status_endpoint_spec.rb b/spec/api/endpoints/status_endpoint_spec.rb index cb79b63..feec3d4 100644 --- a/spec/api/endpoints/status_endpoint_spec.rb +++ b/spec/api/endpoints/status_endpoint_spec.rb @@ -13,14 +13,65 @@ expect(status.teams_count).to eq 0 end + it 'selects an active installation even when inactive records surround it' do + Fabricate(:team, active: false) + active = Fabricate(:team, active: true) + Fabricate(:team, active: false) + allow_any_instance_of(Team).to receive(:ping!) do |record| + expect(record.id).to eq active.id + { ok: 1 } + end + expect(client.status.ping['ok']).to eq 1 + end + context 'with a team' do - let!(:team) { Fabricate(:team, active: false) } + let!(:team) { Fabricate(:team, active: true) } it 'returns a status with ping' do status = client.status expect(status.teams_count).to eq 1 ping = status.ping expect(ping['ok']).to eq 1 end + + %w[account_inactive invalid_auth].each do |error| + context "when Slack returns #{error}" do + before do + allow_any_instance_of(Team).to receive(:ping!).and_raise(Slack::Web::Api::Errors::SlackError, error) + end + + it 'deactivates the installation and returns an explicit failed ping' do + expect(SlackRubyBotServer::Service.instance).to receive(:deactivate!).with(instance_of(Team)).and_call_original + get '/api/status' + status = JSON.parse(last_response.body) + expect(last_response.status).to eq 200 + expect(status['ping']).to eq('ok' => false, 'error' => error) + expect(status['active_teams_count']).to eq 0 + expect(team.reload.active?).to be false + end + end + end + + it 'does not hide unrelated Slack failures or deactivate the installation' do + allow_any_instance_of(Team).to receive(:ping!).and_raise(Slack::Web::Api::Errors::SlackError, 'ratelimited') + get '/api/status' + expect(last_response.status).to eq 400 + expect(JSON.parse(last_response.body)['message']).to eq 'ratelimited' + expect(team.reload.active?).to be true + end + end + + context 'with only inactive teams' do + before { Fabricate(:team, active: false) } + + it 'does not ping revoked credentials' do + expect_any_instance_of(Team).not_to receive(:ping!) + get '/api/status' + status = JSON.parse(last_response.body) + expect(last_response.status).to eq 200 + expect(status['ping']).to be_nil + expect(status['teams_count']).to eq 1 + expect(status['active_teams_count']).to eq 0 + end end end end