Skip to content

Commit 3ae7a2f

Browse files
cursoragenttonythethompson
cursoragent
and
tonythethompson
committed
fix(webhook): reconcile legacy orgs without github_account_id
Address Codex review on PR #76: org rows created before github_account_id was backfilled can still be reconciled on rename. - Add getOrganizationByInstallationId lookup for legacy rows - Resolve orgs by account id, then installation id, then previous slug - Backfill githubAccountId when reconciling stale slugs - Handle organization.renamed changes.login.from for legacy fallback - Add integration tests for legacy org rename paths Co-authored-by: Anthony Thompson <tonythethompson@hotmail.com>
1 parent 44c266e commit 3ae7a2f

3 files changed

Lines changed: 172 additions & 17 deletions

File tree

‎app/api/webhook/github/route.ts‎

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { getGitHubClient, upsertMockPullRequest } from "@/lib/github"
1212
import {
1313
getOrganizationBySlug,
1414
getOrganizationByGithubAccountId,
15+
getOrganizationByInstallationId,
1516
updateOrganizationSlug,
1617
isBypassAccountForOrg,
1718
getSignatureStatusByGithubId,
@@ -124,6 +125,11 @@ type OrganizationPayload = {
124125
id?: number
125126
login?: string
126127
}
128+
changes?: {
129+
login?: {
130+
from?: string
131+
}
132+
}
127133
}
128134

129135
type PingPayload = {
@@ -908,13 +914,15 @@ async function handleInstallation(payload: InstallationPayload) {
908914
return NextResponse.json({ error: "Missing installation account login" }, { status: 400 })
909915
}
910916

911-
// Reconcile a renamed account before doing any slug-keyed lookup below --
912-
// the account id is immutable, the login/slug is not.
913-
const existingByAccountId = accountId
914-
? await getOrganizationByGithubAccountId(String(accountId))
915-
: undefined
916-
if (existingByAccountId && existingByAccountId.githubOrgSlug !== orgSlug) {
917-
await updateOrganizationSlug(existingByAccountId.id, orgSlug)
917+
const existing = await resolveOrganizationForReconciliation({
918+
accountId,
919+
installationId,
920+
currentSlug: orgSlug,
921+
})
922+
if (existing && existing.githubOrgSlug !== orgSlug) {
923+
await updateOrganizationSlug(existing.id, orgSlug, {
924+
githubAccountId: accountId ?? existing.githubAccountId,
925+
})
918926
}
919927

920928
if (payload.action === "created" || payload.action === "unsuspend") {
@@ -932,7 +940,6 @@ async function handleInstallation(payload: InstallationPayload) {
932940
return NextResponse.json({ error: "Missing installation sender info" }, { status: 400 })
933941
}
934942

935-
const existing = existingByAccountId ?? (await getOrganizationBySlug(orgSlug))
936943
if (existing) {
937944
await setOrganizationActive(orgSlug, true)
938945
const updated = await updateOrganizationInstallationId(orgSlug, installationId ?? null, {
@@ -989,11 +996,15 @@ async function handleInstallationRepositories(payload: InstallationPayload) {
989996
return NextResponse.json({ error: "Missing installation account login" }, { status: 400 })
990997
}
991998

992-
if (accountId) {
993-
const existing = await getOrganizationByGithubAccountId(String(accountId))
994-
if (existing && existing.githubOrgSlug !== orgSlug) {
995-
await updateOrganizationSlug(existing.id, orgSlug)
996-
}
999+
const existing = await resolveOrganizationForReconciliation({
1000+
accountId,
1001+
installationId,
1002+
currentSlug: orgSlug,
1003+
})
1004+
if (existing && existing.githubOrgSlug !== orgSlug) {
1005+
await updateOrganizationSlug(existing.id, orgSlug, {
1006+
githubAccountId: accountId ?? existing.githubAccountId,
1007+
})
9971008
}
9981009

9991010
await updateOrganizationInstallationId(orgSlug, installationId ?? null, {
@@ -1014,22 +1025,31 @@ async function handleOrganization(payload: OrganizationPayload) {
10141025

10151026
const accountId = payload.organization?.id
10161027
const newSlug = payload.organization?.login
1028+
const previousSlug = payload.changes?.login?.from
10171029
if (!accountId || !newSlug) {
10181030
return NextResponse.json({ error: "Missing organization id/login" }, { status: 400 })
10191031
}
10201032

1021-
const existing = await getOrganizationByGithubAccountId(String(accountId))
1033+
const existing = await resolveOrganizationForReconciliation({
1034+
accountId,
1035+
previousSlug,
1036+
})
10221037
if (!existing) {
10231038
return NextResponse.json({
10241039
message: `No org record for account id ${accountId}, ignoring rename`,
10251040
})
10261041
}
10271042

10281043
if (existing.githubOrgSlug === newSlug) {
1044+
if (!existing.githubAccountId) {
1045+
await updateOrganizationSlug(existing.id, newSlug, { githubAccountId: accountId })
1046+
}
10291047
return NextResponse.json({ message: "Org slug already up to date" })
10301048
}
10311049

1032-
const updated = await updateOrganizationSlug(existing.id, newSlug)
1050+
const updated = await updateOrganizationSlug(existing.id, newSlug, {
1051+
githubAccountId: accountId,
1052+
})
10331053
return NextResponse.json({
10341054
message: `Renamed org slug ${existing.githubOrgSlug} -> ${newSlug}`,
10351055
org: updated,
@@ -1064,6 +1084,35 @@ function normalizeGitHubAccountType(type?: "Organization" | "User"): GitHubAccou
10641084
return type === "User" ? "user" : "organization"
10651085
}
10661086

1087+
async function resolveOrganizationForReconciliation(params: {
1088+
accountId?: number | null
1089+
installationId?: number | null
1090+
previousSlug?: string | null
1091+
currentSlug?: string | null
1092+
}) {
1093+
if (params.accountId) {
1094+
const byAccountId = await getOrganizationByGithubAccountId(String(params.accountId))
1095+
if (byAccountId) return byAccountId
1096+
}
1097+
1098+
if (params.installationId) {
1099+
const byInstallationId = await getOrganizationByInstallationId(params.installationId)
1100+
if (byInstallationId) return byInstallationId
1101+
}
1102+
1103+
if (params.previousSlug) {
1104+
const byPreviousSlug = await getOrganizationBySlug(params.previousSlug)
1105+
if (byPreviousSlug) return byPreviousSlug
1106+
}
1107+
1108+
if (params.currentSlug) {
1109+
const byCurrentSlug = await getOrganizationBySlug(params.currentSlug)
1110+
if (byCurrentSlug) return byCurrentSlug
1111+
}
1112+
1113+
return undefined
1114+
}
1115+
10671116
function isPersonalAccountOwner(
10681117
org: {
10691118
githubOrgSlug: string

‎lib/db/queries.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,36 @@ export async function getOrganizationByGithubAccountId(accountId: string) {
269269
return rows[0] ?? undefined
270270
}
271271

272-
export async function updateOrganizationSlug(orgId: string, newSlug: string) {
272+
export async function getOrganizationByInstallationId(installationId: number) {
273273
const db = await ensureDbReady()
274+
const rows = await db
275+
.select()
276+
.from(organizations)
277+
.where(eq(organizations.installationId, installationId))
278+
.limit(1)
279+
return rows[0] ?? undefined
280+
}
281+
282+
export async function updateOrganizationSlug(
283+
orgId: string,
284+
newSlug: string,
285+
options?: { githubAccountId?: string | number | null }
286+
) {
287+
const db = await ensureDbReady()
288+
const updateData: {
289+
githubOrgSlug: string
290+
githubAccountId?: string | null
291+
} = { githubOrgSlug: newSlug }
292+
if (options && "githubAccountId" in options) {
293+
updateData.githubAccountId =
294+
options.githubAccountId === undefined || options.githubAccountId === null
295+
? null
296+
: String(options.githubAccountId)
297+
}
298+
274299
const rows = await db
275300
.update(organizations)
276-
.set({ githubOrgSlug: newSlug })
301+
.set(updateData)
277302
.where(eq(organizations.id, orgId))
278303
.returning()
279304
return rows[0] ?? undefined

‎tests/integration/api-suite.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1850,6 +1850,7 @@ test("Webhook: organization renamed reconciles slug in place", async (baseUrl) =
18501850
const { res, data } = await sendWebhook(baseUrl, "organization", {
18511851
action: "renamed",
18521852
organization: { id: 2001, login: "fiveonefour-renamed" },
1853+
changes: { login: { from: "fiveonefour" } },
18531854
})
18541855
assertEqual(res.status, 200, "status")
18551856
assert(data.message.includes("fiveonefour -> fiveonefour-renamed"), "rename message")
@@ -1863,6 +1864,27 @@ test("Webhook: organization renamed reconciles slug in place", async (baseUrl) =
18631864
assertEqual(after.signers.length, before.signers.length, "signatures preserved across rename")
18641865
})
18651866

1867+
test("Webhook: organization renamed reconciles legacy org without account id via previous slug", async (baseUrl) => {
1868+
await resetDb(baseUrl)
1869+
await sql`UPDATE organizations SET github_account_id = NULL WHERE github_org_slug = ${"fiveonefour"}`
1870+
1871+
const before = await fetch(`${baseUrl}/api/orgs/fiveonefour`).then((r) => r.json())
1872+
assertEqual(before.org.githubAccountId, null, "legacy org has no account id")
1873+
1874+
const { res, data } = await sendWebhook(baseUrl, "organization", {
1875+
action: "renamed",
1876+
organization: { id: 2001, login: "fiveonefour-legacy-renamed" },
1877+
changes: { login: { from: "fiveonefour" } },
1878+
})
1879+
assertEqual(res.status, 200, "status")
1880+
assert(data.message.includes("fiveonefour -> fiveonefour-legacy-renamed"), "rename message")
1881+
1882+
const after = await fetch(`${baseUrl}/api/orgs/fiveonefour-legacy-renamed`).then((r) => r.json())
1883+
assertEqual(after.org.id, before.org.id, "same underlying org row, not a duplicate")
1884+
assertEqual(after.org.githubAccountId, "2001", "account id backfilled during rename")
1885+
assertEqual(after.signers.length, before.signers.length, "signatures preserved across rename")
1886+
})
1887+
18661888
test("Webhook: organization renamed for unknown account id is a no-op", async (baseUrl) => {
18671889
await resetDb(baseUrl)
18681890

@@ -1905,6 +1927,36 @@ test("Webhook: installation created after external rename reconciles slug instea
19051927
assertEqual(oldSlugRes.status, 404, "old slug no longer resolves")
19061928
})
19071929

1930+
test("Webhook: installation created reconciles legacy org without account id via installation id", async (baseUrl) => {
1931+
await resetDb(baseUrl)
1932+
await sql`UPDATE organizations SET github_account_id = NULL WHERE github_org_slug = ${"fiveonefour"}`
1933+
1934+
const before = await fetch(`${baseUrl}/api/orgs/fiveonefour`).then((r) => r.json())
1935+
assertEqual(before.org.githubAccountId, null, "legacy org has no account id")
1936+
1937+
const { res, data } = await sendWebhook(baseUrl, "installation", {
1938+
action: "created",
1939+
installation: {
1940+
id: 10001,
1941+
account: { login: "fiveonefour-legacy-install", id: 2001, type: "Organization" },
1942+
},
1943+
sender: { id: 1, login: "orgadmin" },
1944+
})
1945+
assertEqual(res.status, 200, "status")
1946+
assert(data.message.includes("fiveonefour-legacy-install"), "message references new slug")
1947+
1948+
const after = await fetch(`${baseUrl}/api/orgs/fiveonefour-legacy-install`).then((r) => r.json())
1949+
assertEqual(after.org.id, before.org.id, "same underlying org row, not a duplicate")
1950+
assertEqual(after.org.githubAccountId, "2001", "account id backfilled during reconcile")
1951+
1952+
const orgsRes = await fetch(`${baseUrl}/api/orgs`)
1953+
const orgsData = await orgsRes.json()
1954+
const matching = orgsData.orgs.filter((org: { githubOrgSlug: string }) =>
1955+
org.githubOrgSlug.startsWith("fiveonefour")
1956+
)
1957+
assertEqual(matching.length, 1, "no duplicate org row created for the renamed account")
1958+
})
1959+
19081960
test("Webhook: installation_repositories after external rename reconciles slug", async (baseUrl) => {
19091961
await resetDb(baseUrl)
19101962

@@ -1929,6 +1981,35 @@ test("Webhook: installation_repositories after external rename reconciles slug",
19291981
assertEqual(matching.length, 1, "no duplicate org row created")
19301982
})
19311983

1984+
test("Webhook: installation_repositories reconciles legacy org without account id via installation id", async (baseUrl) => {
1985+
await resetDb(baseUrl)
1986+
await sql`UPDATE organizations SET github_account_id = NULL WHERE github_org_slug = ${"fiveonefour"}`
1987+
1988+
const before = await fetch(`${baseUrl}/api/orgs/fiveonefour`).then((r) => r.json())
1989+
assertEqual(before.org.githubAccountId, null, "legacy org has no account id")
1990+
1991+
const { res, data } = await sendWebhook(baseUrl, "installation_repositories", {
1992+
action: "added",
1993+
installation: {
1994+
id: 10001,
1995+
account: { login: "fiveonefour-legacy-repos", id: 2001, type: "Organization" },
1996+
},
1997+
})
1998+
assertEqual(res.status, 200, "status")
1999+
assert(data.message.includes("fiveonefour-legacy-repos"), "message references new slug")
2000+
2001+
const after = await fetch(`${baseUrl}/api/orgs/fiveonefour-legacy-repos`).then((r) => r.json())
2002+
assertEqual(after.org.id, before.org.id, "same underlying org row, not a duplicate")
2003+
assertEqual(after.org.githubAccountId, "2001", "account id backfilled during reconcile")
2004+
2005+
const orgsRes = await fetch(`${baseUrl}/api/orgs`)
2006+
const orgsData = await orgsRes.json()
2007+
const matching = orgsData.orgs.filter((org: { githubOrgSlug: string }) =>
2008+
org.githubOrgSlug.startsWith("fiveonefour")
2009+
)
2010+
assertEqual(matching.length, 1, "no duplicate org row created")
2011+
})
2012+
19322013
test("Webhook: ping event is acknowledged", async (baseUrl) => {
19332014
await resetDb(baseUrl)
19342015
const { res, data } = await sendWebhook(baseUrl, "ping", {

0 commit comments

Comments
 (0)