Make start-reviewing repos idempotent - #445
Merged
TheGreatAxios merged 4 commits intoAug 29, 2026
Merged
TheGreatAxios merged 4 commits into
TheGreatAxios merged 4 commits into
Conversation
startReviewingRepos loops repos minting a grant and a webhook trigger per repo, unconditionally. A failure partway through the loop leaves earlier repos live but unrecorded, and a "try again" retry mints duplicates for them. Adds a fake-ports test that fails a repo mid-loop, retries the same selection, and expects each repo to end up with exactly one grant and one trigger. Fixes CL-7134.
Adds a hasWebhookTrigger port, checked per repo before minting anything for it, backed by a read against the existing WebhookTriggerStore.list. A retry after a mid-loop failure now only mints for the repos the failed attempt never reached, instead of minting a second grant and trigger for repos already set up. selectedRepos is still persisted once at the end rather than incrementally: idempotency now comes from the per-repo trigger check, not from what's recorded in settings, so a retry is safe regardless of whether the prior attempt's persistSelectedRepos call ran. Fixes CL-7134.
The prior fix skipped a repo whose webhook trigger already exists, but grant minting and trigger creation are two separate steps, and the grant table carries no unique constraint. A failure between minting the grant and creating the trigger still re-mints a second grant for that repo on retry, since only the trigger's existence was checked. Adds a fake-ports test that fails createWebhookTrigger (after mintRepoGrant already succeeded) on repo 2 of 3, retries, and asserts the grant is minted exactly once per repo. Also adds a unit test for a webhookTriggerName helper, so the trigger-name string used to create a trigger and the one used to look it up can't independently drift. Fixes CL-7134.
Gates mintRepoGrant on a new hasRepoGrant port, checked independently of hasWebhookTrigger, so a retry after a failure between the two steps mints only whichever one the failed attempt didn't finish rather than re-minting a grant that already exists. apps/hub's hasRepoGrant binds to a read against the same grant table mintRepoGrant inserts into. Also pulls the "<repo> pull-request-opened" trigger-name convention into one exported webhookTriggerName helper, used by both the create and the lookup in apps/hub, so they read the same string by construction instead of by two hand-kept copies of it. Documents, rather than changes, that a trigger is never disabled or deleted on GitHub disconnect: re-adding a repo after a reconnect finds its old trigger still live and is skipped, not re-created. Fixes CL-7134.
TheGreatAxios
force-pushed
the
cl-7134-start-reviewing-repos-is-not-idempotent-a-mid-loop-failure
branch
from
August 29, 2026 04:46
e497225 to
2f3169e
Compare
8 of 9 tasks
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.
Fixes CL-7134 — https://linear.app/abklabs/issue/CL-7134
Problem
startReviewingRepos(packages/workflow-catalog/src/connect-github-setup.ts) loops selected repos callingports.mintRepoGrant(repo)thenports.createWebhookTrigger(repo)unconditionally, and persistsselectedReposonce after the loop. A failure on repo N leaves repos 1..N-1 with a live grant and trigger that nothing recorded. The route's generic "Try again in a moment." retries the same selection, and neitherWebhookTriggerStore.create(plain INSERT, no dedupe key) nor the grant table (no unique constraint) rejects the duplicate — the retry mints a second grant and trigger per repo.Change
startReviewingReposis idempotent per repo on both steps:hasRepoGrant(repo)gatesmintRepoGrant,hasWebhookTrigger(repo)gatescreateWebhookTrigger, checked independently so a failure between the two steps re-mints nothing on retry.webhookTriggerName(repo)is the one helper both trigger creation and the lookup use, so the identity string cannot drift.ConnectGithubRoutesDepsand bound inapps/hub/src/index.ts: the grant check reads the grant store scoped to(tenant, resource "repo:<name>", action "read")— exactly whatmintRepoGrantinserts — and the trigger check readsWebhookTriggerStore.list(tenantId)filtered on the code-review definition id andwebhookTriggerName(repo).selectedReposstays persisted once at the end: idempotency comes from the per-repo checks, not from what settings recorded, so a retry is safe whether or not the failed attempt reachedpersistSelectedRepos.Tests
connect-github-setup.test.ts: (1) failmintRepoGranton repo 2 of 3, retry → exactly one grant and one trigger per repo; (2) failcreateWebhookTriggerafter the grant succeeded on repo 2, retry → the grant is not re-minted; (3)webhookTriggerNameround-trips between create and lookup. Fakes are state-backed so the retry sees what the first attempt wrote. Confirmed red without the checks, green with them.