[iscekic]

Evidence

Evidence: demonstrated — a database-backed suite, apps/web/src/lib/integrations/oauth/platforms/slack-callback.db.test.ts, runs the callback handler against local Postgres with the Chat SDK adapter stubbed. It shows that a completed install redirects with the expected result, that the integration row recorded for the workspace keeps its values and ownership, that the Chat SDK installation is set from that row, that an exchange token that is not part of the recorded installation is revoked, and that no extra row is created. A second case shows a repeat install by the recorded owner updates its own row and leaves the installation untouched. A route-level suite covers the other branches.

Summary

A Slack OAuth install now settles a workspace's Chat SDK installation from the integration recorded for that workspace.

The callback resolves the integration recorded for the workspace and makes the Chat SDK installation match it:

• The recorded installation (bot token, bot user, team name) is rebuilt and written with
  slackAdapter.setInstallation, so the workspace always resolves to the owner it is recorded
  for.
• When the workspace has no recorded integration, the installation is removed with
  slackAdapter.deleteInstallation.
• An exchange token that is not the recorded installation's own token is revoked, and a
  revocation that does not complete is captured to Sentry.
• Workspace identity links stay as they are.

The redirect for a workspace the store reports as already connected still reports
error=workspace_already_connected, and a settle that fails falls back to removing the
installation before that redirect.

The logic lives in restoreRecordedSlackInstallation in
apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts, inside the conflict branch of
the callback. It composes with the organization-authority path that upsertSlackInstallation
already gates, and the callback still passes allowWorkspaceRebind only for organization
owners.

Verification

• apps/web: NODE_ENV=test pnpm exec jest --runInBand src/lib/integrations/oauth/platforms/slack-callback.db.test.ts
  → 2 passed against a migrated local Postgres.
• apps/web: NODE_ENV=test pnpm exec jest --runInBand src/lib/integrations/oauth/platforms/slack-callback.test.ts
  → 12 passed. Covers the redirect and each settle branch (recorded integration, no recorded
  integration, recorded row without a token, write failure, removal failure, token reuse), plus
  the organization authority cases on main.
• apps/web: NODE_ENV=test pnpm exec jest --runInBand src/lib/integrations/slack-service.test.ts
  → 26 passed.
• apps/web: oxlint 0 warnings, 0 errors on the changed files; oxfmt no differences;
  tsgo --noEmit --checkers 1 clean.
• Manual/E2E: the changed surface is a server-side OAuth callback with no UI, and a Slack app
  install cannot be completed from this environment, so the database-backed callback suite is
  the end-to-end evidence. No screenshots apply.

Visual Changes

N/A

Production prerequisites

None.

Reviewer Notes

• getInstallationByTeamId is already exported by slack-service; no service changes.
• The rebuilt installation carries botToken, botUserId, and teamName. The adapter never
  reads enterpriseId/isEnterpriseInstall back from stored state, and the storage key is the
  same teamId, so the rebuild is equivalent for token resolution.
• Revocation is skipped when the exchange token equals the recorded token, so re-running OAuth
  on an existing install never revokes the live token.
• Existing Slack rows store the token in metadata.access_token; if a row has none, the
  installation is removed instead of restored.
• The branch is merged with main; the two test files that both added
  slack-callback.test.ts are combined into one suite.

Proxy answers

The owner's proxy gave these answers in the owner's place.

1. Question: Settle shape when the store reports the workspace as already connected?
   Answer: Restore the existing owner's installation
   Confidence: 0.60 · Precedent: Speed and least work: 'do the least amount of work possible, with no drop in quality'.
2. Question: Exchange token that the install does not use?
   Answer: Revoke only when it differs from the binding
   Confidence: 0.75 · Precedent: none
3. Question: Settle failure?
   Answer: Fall back to delete, then redirect
   Confidence: 0.55 · Precedent: Make on-call alerts informative and actionable.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only change since the previous review is a new database-backed suite, slack-callback.db.test.ts; it drives the real callback, upsertSlackInstallation, getInstallationByTeamId, and revokeSlackToken against Postgres with only the Chat SDK adapter, user auth, credential store, and model allow-list stubbed, and its assertions match the reviewed behavior — the recorded owner's row is unchanged, the recorded installation is rewritten, the incoming token is revoked, no acting-user row is created, and a same-owner reinstall leaves the installation and revocation untouched. The finally-based revocation and its failure reporting from the prior review are unchanged at HEAD.

Assumptions: [INFERENCE] The new file matches the standard apps/web jest project (testMatch: **/src/**/*.test.ts) and therefore requires the same Postgres/migration setup as the existing .db.test.ts suites; the PR reports it passing against a migrated local Postgres. [INFERENCE] The suite relies on the fixed team id T_DB_SHARED being unique under the global (platform, platform_installation_id) Slack index; a crashed prior run could leave a row, but the random per-run user ids and cascade deletes make that unlikely.

Files Reviewed (3 files)

• apps/web/src/lib/integrations/oauth/platforms/slack-callback.db.test.ts (new; full read)
• apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts (targeted re-verification of the reviewed settle path)
• apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts (targeted re-verification)

Previous Review Summaries (2 snapshots, latest commit 03c7470)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 03c7470)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change moves Slack conflict-rollback token revocation into a finally so it runs even when installation removal rejects, and reports a revocation that returns false; the new tests cover the no-token row, a rejecting removal, and the restore path.

Assumptions: [INFERENCE] Slack may invalidate the previously issued bot token when the same app is installed again into the same workspace. The restore path still writes the recorded metadata.access_token back, so a real reinstall should confirm that the recorded owner's token remains valid; if Slack rotates it, restoring the stored token would leave the recorded owner with a dead credential. [INFERENCE] A removal failure in the no-recorded-integration branch is captured only by the caller's generic install-conflict-rollback Sentry tag, not a removal-specific tag. Pre-existing and out of scope here: a non-conflict failure from upsertSlackInstallation still leaves the incoming Chat SDK installation in place.

Files Reviewed (2 files)

• apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts
• apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts

Previous review (commit ae22586)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The conflict rollback restores the recorded owner's Chat SDK installation, but its cleanup paths can skip the token revocation when installation removal fails, and the restore path may write a token Slack has already invalidated.

Assumptions: [INFERENCE] @chat-adapter/slack is not installed in this checkout, so SlackInstallation required fields and setInstallation/deleteInstallation semantics were only verified from call sites (apps/web/src/lib/bot/platforms/slack.tsx:283). [INFERENCE] Slack may invalidate the previous bot token when the same app is re-installed into the same workspace; if so, restoring metadata.access_token and revoking the incoming token (lines 85-87) can leave the recorded owner with no valid token, and the PR does not state which Slack behavior it relies on — please confirm with a real install. Pre-existing: any non-conflict failure from upsertSlackInstallation still leaves the freshly issued installation in Chat SDK state, because the generic catch does not roll back.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts	82	A failed deleteInstallation (line 82 or 79) rejects before revokeSlackToken, leaving the conflicting installation and its live token.

SUGGESTION

File	Line	Issue
apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts	86	revokeSlackToken returns false on failure and the result is ignored, so an unrevoked token is silent.
apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts	41	The null (no recorded token) branch deletes the installation but has no test.

Files Reviewed (2 files)

• apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts - 3 issues
• apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts - 0 issues

Fix these issues in Kilo Cloud

---

_{Reviewed by deepseek-flash-latest · Input: 86.2K · Output: 12.6K · Cached: 990.5K}

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

bot: Summary notes on head 03c7470:

• Token revocation now runs regardless of whether the installation write succeeds, and a revocation that does not complete is reported with the team id.
• The route suite covers the restore of the recorded installation, a recorded row without a stored credential, an installation removal that rejects, and a same-owner reinstall that performs no extra state writes. Locally on this head: slack-callback.test.ts + slack-service.test.ts → 2 suites, 27 tests passed.
• The stored-token assumption is recorded in Reviewer Notes: the restore writes back the token recorded for the team, and the incoming token is only revoked when it differs from that one.
• The non-conflict failure path is unchanged and out of scope for this work item.

[iscekic]

bot: Closing as superseded by #8234. That PR checks workspace ownership under a lock before writing the Slack installation, and restores the saved installation on conflict.