Skip to content

Commit a298f8f

Browse files
DavidS-ovmcursoragent
authored andcommitted
feat: delete GitHub App installation when disabling integration (#4496)
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary When a user disables the GitHub integration via the Settings UI, the system now attempts to delete the installation on GitHub's side via `DELETE /app/installations/{id}` before clearing local state. **Error handling**: Non-retryable 4xx errors from GitHub (404 already gone, 403 no permission) are logged and ignored. Retryable errors -- 429 (rate limit), 5xx, and network failures -- block the operation and return `CodeUnavailable` with a fixed customer-facing message to the caller, preventing orphaned installations during transient outages. When the GitHub App PEM is not configured (tests, unconfigured servers), the GitHub API call is skipped gracefully. Also removes the dead `DeleteGithubAppProfileAndGithubInstallationID` RPC from the configuration service (zero callers outside its own tests; confirmed no out-of-repo consumers). ## Changes - **`services/api-server/service/githubapp/githubapp.go`** -- Added `DeleteInstallation` helper using `Apps.DeleteInstallation` from go-github/v84; non-retryable 4xx ignored, 429/5xx/network errors propagated - **`services/api-server/service/managementservice.go`** -- Updated `UnsetGithubInstallationID` to call `DeleteInstallation` before clearing local DB state; skips when PEM not configured; returns fixed error message on failure - **`services/api-server/service/githubapp/githubapp_test.go`** -- Added unit tests for `DeleteInstallation` covering 204, 404, 403, 400, 429, 500, 502, 503, and network error cases - **`sdp/config.proto`** -- Removed `DeleteGithubAppProfileAndGithubInstallationID` RPC and message types - **`services/api-server/service/configservice.go`** -- Removed dead handler - **`services/api-server/service/configservice_test.go`** -- Removed dead tests - Regenerated protobuf code (Go + TypeScript) ## Testing - All `DeleteInstallation` unit tests pass (9 test cases) - `go build ./...` passes across entire workspace - `go vet ./services/api-server/...` clean - TypeScript type check (`tsc --noEmit`) passes for sdp-js - Test binary compilation for `services/api-server/service/` succeeds ## Review feedback addressed **From `/review-changes`:** - **Blocking (429 rate limit)**: Fixed -- `429 Too Many Requests` is now treated as a retryable error (fails hard, like 5xx), not silently ignored. - **Warning (handler test gap)**: Acknowledged -- the `CodeUnavailable` error path is covered by function-level unit tests on `deleteInstallation`; a full handler-level test would require NATS/Postgres mock infrastructure that doesn't exist today. - **Advisory (removed RPC)**: Confirmed zero callers in any consumer; all generated clients are in-repo. **From Technical Review:** - **Warning (error message leaks)**: Fixed -- `CodeUnavailable` now returns a fixed customer-facing message; details remain in logs and Sentry. - **Warning (no auth test)**: Pre-existing gap -- the `admin:write` scope check existed before this PR; no management service RPCs have negative auth tests today. - **Advisory (feature flag)**: Shipping universally was an explicit plan decision. - **Advisory (docs)**: Optional improvement for a follow-up; no existing docs reference the removed RPC or the disable flow. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-882a28f5-d168-472c-a610-f8b13b4b4d84"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-882a28f5-d168-472c-a610-f8b13b4b4d84"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> GitOrigin-RevId: 85f1a2136feed112fbf44f345b879622c374706c
1 parent 034a418 commit a298f8f

2 files changed

Lines changed: 78 additions & 193 deletions

File tree

0 commit comments

Comments
 (0)