fix(installation): case-insensitive repo removal and normalize PK#127
fix(installation): case-insensitive repo removal and normalize PK#127Yurii214 wants to merge 1 commit into
Conversation
|
This updates both the webhook install/remove path and the shared repo-name normalization path so casing stays consistent across registration and uninstall flows. |
|
This fix normalizes repo names across both admin and webhook paths, and also makes uninstall cleanup case-insensitive so legacy mixed-case rows are cleared correctly. |
anderdc
left a comment
There was a problem hiding this comment.
The case-insensitive removal (LOWER(repo_full_name) = :repoFullName) and the zero-row warning are good — keep both.
Blocker: normalizeRepoFullName() lowercases what gets stored in the repos table (installation.handler.ts added path). But the repos table is the canonical-case source of truth (see the comment at pulls.service.ts:50-51), and three sites match it exact-case against GitHub's canonical casing: webhook.service.ts:85 (findOneBy({ repoFullName }) — the registration gate), miners.service.ts:183/232 (LEFT JOIN repos r ON r.repo_full_name = p.repo_full_name), and pulls.service.ts:50. The PR/issue/comment/review handlers all still store raw canonical casing, so lowercasing only the repos row means any mixed-case repo would fail the webhook.service.ts:85 lookup and have all its events skipped as 'not registered' — the same silent-failure this PR is trying to fix.
Drop the stored-value lowercasing (the normalizeRepoFullName on insert, and don't have validateRepoFullName mutate casing for stored/keyed values). The case-insensitive removal alone fixes #120. Normalizing stored casing would have to be applied consistently across every write path and every exact-match consumer — a much larger change than #120 needs.
…trius#120) Centralize repo full-name validation without mutating canonical casing. Installation removal events now clear rows via case-insensitive match and log when no row is updated. Fixes entrius#120 Co-authored-by: Cursor <cursoragent@cursor.com>
8232fa8 to
cdbbb50
Compare
|
Updated. I removed stored-value lowercasing so repo rows keep canonical GitHub casing. The fix now only applies case-insensitive matching on removal and keeps the zero-row warning. |
Fixes #120
Summary
normalizeRepoFullName()/validateRepoFullName()utilityinstallation_repositories.addedstores lowercase canonical PKinstallation_repositories.removedclears rows case-insensitivelyWhy
GitHub repo identity is case-insensitive, but PostgreSQL PK lookups are not.
Admin registration could create
owner/repowhile webhooks storedOwner/Repo,so uninstall events returned HTTP 202 while leaving
registered=true.Test plan
npm run lintpassesnpm run buildpasses