Skip to content

fix(github): migrate repo identity on a GitHub repository rename webhook - #5918

Merged
JSONbored merged 2 commits into
mainfrom
fix/repo-rename-webhook-handling
Jul 14, 2026
Merged

fix(github): migrate repo identity on a GitHub repository rename webhook#5918
JSONbored merged 2 commits into
mainfrom
fix/repo-rename-webhook-handling

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Nothing in the codebase handled the GitHub App's repository webhook with action: "renamed" — a repo rename (e.g. gittensory → loopover) left every repo-identity-keyed row pointing at the old full_name until an unrelated write happened to touch it, and rows keyed only by the old name (repositories, repository_settings) never updated at all without a webhook-driven path.
  • Adds maybeHandleRepositoryRenamedWebhookEvent, wired into processGitHubWebhook right before the existing upsertRepositoryFromGitHub(payload.repository) call so that upsert UPDATEs the now-renamed anchor row instead of inserting a fresh, disconnected duplicate.
  • renameRepositoryIdentity (src/db/repo-identity-rename.ts) migrates the structural identity columns for repositories, repository_settings, pull_requests, issues, and audit_events's target_key — explicit per-table code (matching this codebase's house convention in repositories.ts) rather than a generic Drizzle helper, with a fold-on-collision pattern for the unique (repo_full_name, number) constraints.
  • Also detects the one thing a rename can't migrate on its own: a self-host operator's container-private per-repo config folder is derived from the current repo name and is read-only from the app's perspective, so a folder that existed under the old name but not the new one means the operator's gate/autonomy/review policy silently reverted to defaults. maybeWarnOnMissingLocalConfigAfterRename surfaces that as an error-level audit event + log line instead of a routine info line, using the new hasLocalManifest() (focus-manifest-loader.ts).

Scope

Deliberately narrow to structural repo-identity columns. Follow-up PRs (tracked separately) extend the same pattern to:

  • review/audit tables (gate_outcomes, advisories, check_summaries, pull_request_files, pull_request_reviews, etc.)
  • caches/analytics tables (ai_review_cache, signal_snapshots, product_usage_events, etc.)
  • REES/enrichment tables (review_targets, contributor_gate_history, review_audit, repo_chunks)

Each stays independently reviewable rather than one large cross-cutting migration.

Test plan

Nothing in the codebase handled the GitHub App's `repository` webhook
with `action: "renamed"` -- a repo rename (e.g. gittensory -> loopover)
left every repo-identity-keyed row pointing at the old full_name until
the next unrelated write happened to touch it, and any row keyed only
by the old name (repositories, repository_settings) never updated at
all without a webhook-driven path.

Adds maybeHandleRepositoryRenamedWebhookEvent, wired into
processGitHubWebhook right before the existing
upsertRepositoryFromGitHub(payload.repository) call so that upsert
UPDATEs the now-renamed anchor row instead of inserting a fresh,
disconnected duplicate. renameRepositoryIdentity (src/db/repo-identity-rename.ts)
migrates the structural identity columns for repositories,
repository_settings, pull_requests, issues, and audit_events'
target_key -- explicit per-table code (matching this codebase's house
convention in repositories.ts) rather than a generic Drizzle helper,
with a fold-on-collision pattern for the unique (repo_full_name, number)
constraints.

Also detects the one thing a rename can't migrate on its own: a
self-host operator's container-private per-repo config folder is
derived from the CURRENT repo name and is read-only from the app's
perspective, so a folder that existed under the old name but not the
new one means the operator's gate/autonomy/review policy silently
reverted to defaults. maybeWarnOnMissingLocalConfigAfterRename surfaces
that as an error-level audit event + log line instead of a routine info
line, using the new hasLocalManifest() (focus-manifest-loader.ts).

Scope is deliberately narrow to structural repo-identity columns --
review/audit tables (gate_outcomes, advisories, pull_request_reviews,
etc.), caches/analytics tables, and REES/enrichment tables are tracked
as separate follow-up PRs so each stays independently reviewable.
@JSONbored JSONbored self-assigned this Jul 14, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 LoopOver review result - fixes required

Review updated: 2026-07-14 23:02:22 UTC

7 files · 1 AI reviewer · 2 blockers · readiness 88/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

Review summary
Adds a `repository`/`renamed` webhook handler that migrates repo-identity columns (repositories, repository_settings, pull_requests, issues, audit_events.target_key) from the old full_name to the new one before the existing upsert runs, plus a self-host-only audit warning when a container-private config folder didn't move with the rename. The migration is idempotent (each step only touches rows still under the old name, so redelivery is safe) and collision-safe (a stray row already created under the new name is folded away in favor of the pre-existing row). Test coverage is thorough and exercises the real webhook dispatch path (`processJob`), not a fabricated payload shape, including the no-op cases (same name, missing `from`, wrong action).

Nits — 5 non-blocking
  • src/db/repo-identity-rename.ts: the five table updates run as separate sequential statements with no `db.batch()`/transaction wrapper, so a mid-migration crash leaves the identity partially renamed until a webhook redelivery completes it — worth a comment or a batch() call given D1's lack of implicit transactions across awaits.
  • src/queue/processors.ts maybeWarnOnMissingLocalConfigAfterRename: records outcome: "denied" for what is a configuration-drift warning, not a denial — consider a dedicated outcome value so this doesn't get misread as a gate decision when scanning audit_events.
  • src/queue/processors.ts maybeHandleRepositoryRenamedWebhookEvent: oldFullName is built from `payload.repository.owner.login` (the NEW owner), so a simultaneous org rename + repo rename would compute a non-matching oldFullName and silently no-op the migration — worth a one-line comment noting this known limitation since it isn't covered by a test.
  • Consider wrapping the five updates in repo-identity-rename.ts in `db.batch()` (D1 supports batched statements) to make the migration atomic rather than relying purely on idempotent redelivery.
  • src/queue/processors.ts: give the self-host config-drift audit event a distinct outcome (e.g. "warning") instead of "denied" so it's not conflated with actual gate denials when querying audit_events.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.

CI checks failing

  • validate
  • validate-tests (5)
  • validate-tests (3)
  • validate-tests (2)
  • validate-code
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 43 registered-repo PR(s), 35 merged, 385 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 43 PR(s), 385 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: significant — This closes a real, previously-unhandled data-integrity gap (renamed repos forking history into disconnected rows) with an idempotent, collision-safe migration and end-to-end test coverage against the actual webhook dispatch path.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 43 PR(s), 385 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 14, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit 1271956 into main Jul 14, 2026
14 checks passed
@JSONbored
JSONbored deleted the fix/repo-rename-webhook-handling branch July 14, 2026 23:26
JSONbored added a commit to nickmopen/gittensory that referenced this pull request Jul 14, 2026
…bles

Follow-up to the repo-rename webhook handler (JSONbored#5918): renameRepositoryIdentity
only covered the anchor tables (repositories, repository_settings,
pull_requests, issues, audit_events). Extends it to every review/audit
table keyed on repo_full_name so a GitHub repository rename migrates
their history too, instead of orphaning it under the old name:

- gate_outcomes, active_review_tracking, pull_request_detail_sync_state,
  recent_merged_pull_requests: same fold-then-rename shape as
  pull_requests/issues -- each has a unique (repo_full_name, pull_number|
  number) index and an id that embeds the repo name.
- pull_request_files: unique (repo_full_name, pull_number, path), a
  3-column key -- folds per (pull_number, path) pair rather than a
  single-column IN, since row counts here are small and this avoids a
  raw composite-tuple SQL construct.
- check_summaries: unique (repo_full_name, head_sha, name) -- same
  per-pair fold, but head_sha is nullable so the collision lookup
  branches on isNull vs eq per row (SQL NULL never equals NULL via `=`).
- pull_request_reviews: no separate unique index, just the PK id (which
  embeds the repo name) -- folds on the id the rename would produce
  rather than a business-key tuple.
- advisories: id is a random UUID (never repo-derived) with no unique
  constraint on repo columns, so this is a plain rename of repo_full_name
  plus the repo-embedded target_key, same LIKE+replace shape as
  audit_events.target_key.

30 new tests covering the rename, the collision-fold, an
unrelated-row-untouched case, and (for check_summaries) the NULL
head_sha fold specifically.
JSONbored added a commit that referenced this pull request Jul 15, 2026
#5953)

Follow-up to #5918/#5950: extends renameRepositoryIdentity to 14 more
repo_full_name-keyed tables:

- burden_forecasts, repo_queue_trend_snapshots, repo_sync_state:
  repo_full_name itself IS the primary key (single row per repo) --
  same fold-then-rename shape as the anchor tables.
- repo_sync_segments (unique on segment), contributor_repo_stats
  (unique on login), repo_labels (unique on name): each has an id that
  embeds the repo name and a unique index pairing repo_full_name with
  one other column -- folds on that column.
- collision_edges: id embeds the repo name but is built in
  packages/loopover-engine and passed through verbatim, with no unique
  index -- folds on the id collision the rename would produce, same
  shape as pull_request_reviews.
- notification_deliveries, github_agent_command_answers,
  repo_snapshots, repo_github_totals_snapshots,
  github_rate_limit_observations, product_usage_events,
  signal_snapshots: id is always a random UUID with no unique
  constraint tied to repo_full_name (several nullable) -- plain
  renames. notification_deliveries.deeplink and
  github_agent_command_answers.response_url are also rewritten, same
  as the anchor tables' own html_url treatment (their own canonical
  GitHub link, not incidental content).

Deliberately excludes the request-scoped AI/LLM result caches
(ai_review_cache, ai_slop_cache, linked_issue_satisfaction_cache,
grounding_file_content_cache) and the RAG chunk cache (repo_chunks) --
every one is a rebuildable cache, not identity data: a miss after a
rename just re-runs one LLM call or one re-index pass at the new name,
which is graceful and self-healing, unlike an orphaned PR/issue/audit
row a maintainer would otherwise need a GitHub API backfill to
recover. Documented inline so the omission reads as deliberate, not
forgotten.

24 new tests: a rename test for all 14 tables, plus a collision-fold
regression test for the 7 that have a real fold path (three
PK-is-repo_full_name tables, three single-column-unique-index tables,
and the id-collision fold for collision_edges).
JSONbored added a commit that referenced this pull request Jul 15, 2026
Follow-up to #5918/#5950/#5953: extends renameRepositoryIdentity to
review_audit, contributor_gate_history, and submitter_stats -- raw-SQL-
only REES/parity tables, never added to the Drizzle schema (see each
migration's own header comment), so these three blocks use
env.DB.prepare() directly instead of the query builder, matching how
every real writer (parity-wire.ts, outcomes-wire.ts, contributor-
calibration.ts, submitter-reputation.ts) already accesses them.

- review_audit, contributor_gate_history: PK `id` alone, no separate
  unique index. `project` IS the repo full name at every live writer;
  `target_id` (`${project}#${pullNumber}`) and `id` both embed it.
  Fold on the id collision the rename would produce -- same
  PK-collision-only shape as pull_request_reviews/collision_edges.
- submitter_stats: no id column -- PRIMARY KEY (project, submitter)
  directly. Fold on the other half of the composite key, `submitter`.

Deliberately excludes two other raw-SQL REES tables:
- review_targets: has NO live writer anywhere in this codebase
  (explicitly confirmed orphaned by src/review/public-stats.ts's own
  comment). Its `project` column is an agent/install-level slug that
  must NEVER be renamed (not a per-repo value), and its `repo` column's
  real on-disk semantics for that historical, one-time-bulk-copied data
  can't be independently verified from current code alone -- mutating
  identity columns on data no writer would ever repair carries real
  corruption risk for uncertain benefit.
- repo_chunks: a rebuildable RAG chunk/embedding cache (same reasoning
  as the AI/LLM caches #5953 excluded), whose project/repo columns hold
  the bare owner and bare repo name SEPARATELY (not a single owner/repo
  string) and whose id is a lowercased, truncated-to-64-chars hash of
  the two -- a safe in-place string rename isn't mechanically available
  without risking a silently-corrupted truncated id.

7 new tests: a rename test for all 3 tables, a collision-fold
regression test for each, plus an unrelated-row-untouched check for
submitter_stats.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant