Delete and revoke member credentials when an install is removed - #30
Merged
Merged
Conversation
Two reported faults, both on the app side. Uninstall left every member token behind. A lifecycle 404 forgot the workspace row, which is only routing; the sealed access and refresh tokens in `connections` were never deleted, the sync never reconciled member connections against what Initiative still held, and nothing ever asked GitHub to end the authorizations. The public documentation promises each credential is deleted and the app told to stop using them. Removal now deletes the credentials and revokes the grant at GitHub — the grant rather than the token, since a refresh token sits beside every access token and would mint a replacement. Subscriptions go with them. A turned-off app and a cleared workspace connection deliberately do not trigger any of it: the documentation promises an off switch keeps the setup. The sweep is keyed on the credentials themselves rather than on the workspace table, so an app disabled and then removed cannot be missed. Per-member convergence rides on the config read, which already lists who is still connected and was being discarded. Webhook handling was synchronous, unretried and blind to rate limits. Deliveries now become rows in an `outbox` and the request answers; delivery retries with exponential backoff and gives up after about half an hour. Install changes resync after the response rather than inside it. GitHub's three rate-limit signals are read and told apart from a plain permission 403, and the installation's repository listing is cached for fifteen minutes and dropped on the webhook that says it changed, instead of being paged on every sync. The outbox arrives as an appended schema step, so a database that has already run the initial schema takes it on the next boot rather than being dropped.
The route answers with what it wrote down rather than what it sent, so the field it reports is queued. The lease test still asserted the old published, which is the one thing CI caught.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two reported faults, both on the app side.
Uninstall left every member token behind
A lifecycle 404 forgot the workspace row, which is only routing. The sealed access and refresh tokens in
connectionswere never deleted, the sync never reconciled member connections against what Initiative still held, and nothing ever asked GitHub to end the authorizations — while the public docs promise each credential is deleted and the app told to stop using them.Removal now deletes the credentials and revokes the grant at GitHub. The grant rather than the token: a refresh token sits beside every access token and would otherwise mint a replacement. Subscriptions go with them — a delivery secret for a guild the app is no longer in is a credential that outlived its install.
Deliberately not triggered by a turned-off app or a cleared workspace connection: the docs promise an off switch keeps the setup, so neither touches a credential. The sweep is keyed on the credentials themselves rather than the workspace table, so an app disabled then removed cannot be missed. Per-member convergence rides on the config read, which already lists who is still connected and was being discarded.
Webhook handling was synchronous, unretried and rate-limit blind
Deliveries now become rows in an
outboxand the request answers immediately; delivery retries with exponential backoff and gives up after ~30 minutes. Install changes resync after the response rather than inside it. GitHub's three rate-limit signals are read and told apart from a plain permission 403. The installation's repository listing is cached 15 minutes and dropped on the webhook that says it changed, instead of being paged every sync.This composes with the existing
runDeliveryOnceledger: that still wraps the work, and the work is now just an enqueue.Schema
The outbox arrives as an appended step (
0003-outbox), so a database that already ran the initial schema takes it on next boot rather than being dropped.Verification
Rebased onto current
main(kit v0.20.0, vitest 5). Locally: typecheck, build, manifest/catalog/env-contract/github-app all pass, and 175 tests across 13 files pass. The 12 Postgres-backed test files could not be run locally — no database available on that machine — so CI is the real gate for those.