Wire GitHub numeric IDs through sync + push for rename robustness

Capture GitHub's stable repo / owner / installation IDs on first
successful sync, write them to bindings + content rows, and have the
push event processor prefer ID-keyed binding lookup with a name-based
fallback for un-synced rows. Internal matching now survives a GitHub
repo or org rename without operator intervention; the public API
remains name-keyed and exposes the captured IDs in the binding GET
response for debugging.

Key decisions:
- ``InstallationAuth`` grew a required ``installation_id: int`` field
  rather than a separate ``get_installation_id`` round-trip from the
  syncer. The app client already resolves the installation id inside
  ``get_installation_auth`` and was discarding it; threading it onto
  the auth record is a no-cost change for production callers and lets
  the syncer write ``github_installation_id`` to the binding without
  doubling the GitHub round-trip count. The dataclass field is
  required (no default) so production code cannot accidentally
  construct an auth record without it; the test fixture's
  ``installation_auth`` helper sources it from the mock's seeded
  ``(owner, repo) -> id`` map (or ``default_installation_id``) so
  existing direct-construction tests keep their ergonomic shape.
- The tree fetcher gained a ``_fetch_repo`` step that calls
  ``GET /repos/{owner}/{repo}`` rather than overloading the existing
  ``commits/{ref}`` call, because the commits response does not
  include the repository's numeric ID and adding the field to its
  schema is GitHub's prerogative. Repo + owner IDs land on
  ``FetchedTree`` as required ``int`` fields rather than
  ``int | None`` because the syncer is the only consumer in this
  slice and it always wants both — leaving them ``Optional`` would
  have leaked ``None`` into the store-layer guard for no benefit.
  The ``raise_for_status`` chain catches a missing repo deterministi-
  cally, so a real GitHub anomaly (e.g. installation lost mid-sync)
  surfaces as a recorded sync failure rather than a silent no-id
  capture.
- ``DashboardGitHubTemplateBindingStore`` exposes two new methods
  rather than re-purposing ``list_by_repo_ref``:
  ``list_by_repo_id_and_ref`` for the ID-keyed primary lookup
  (backed by ``idx_dashboard_github_template_bindings_repo_id_ref``
  from #240) and ``list_unsynced_by_repo_ref`` whose
  ``github_repo_id IS NULL`` filter encodes the rename-safety
  invariant directly. Without that filter, a binding whose name
  coincidentally matches a *different* repo now sitting at the old
  string would re-sync against the wrong upstream; pushing the
  filter into the store query (vs. dedup-after-the-fact) keeps the
  invariant load-bearing in code rather than only in tests.
  ``list_by_repo_ref`` stays in the API surface even though the
  push processor no longer calls it, because the binding store is
  a public storage primitive and external callers (admin tooling,
  future debug consoles) may legitimately want the unfiltered
  name-only view.
- ``PushEventProcessor._lookup_bindings`` unions the two store calls
  with a ``seen: set[int]`` dedup guard. The two queries are
  disjoint by construction (the unsynced filter excludes IDs the
  primary lookup might find), so the dedup is defense-in-depth — but
  the ``test_process_dedupes_id_and_name_matches`` test pins the
  guard so a future loosening of either query (e.g. dropping the
  ``IS NULL`` filter "for symmetry") cannot regress to double-
  enqueueing the same ``dashboard_sync`` job. ``_coerce_int`` rejects
  ``bool`` explicitly because ``isinstance(True, int)`` is ``True``
  in Python and a malformed payload that sent ``"id": true`` would
  otherwise leak ``1`` through as a real repo id.
- The store-layer "only assign if non-None" guard on
  ``update_sync_state`` and ``upsert`` (already pinned by tests in
  #240) is what protects populated IDs from being overwritten on a
  failed re-sync. The syncer now always passes populated IDs on the
  success path, so the guard's job is to fence the *failure* path
  (where ``_record_failure`` calls ``update_sync_state`` without ID
  kwargs). The new ``test_sync_does_not_overwrite_populated_ids_
  on_resync`` test pins the contract end-to-end so a future syncer
  refactor that started passing ``None`` explicitly cannot zero a
  binding's IDs.
- ``GitHubMock.seed_tree`` now auto-seeds the
  ``GET /repos/{owner}/{repo}`` endpoint with default ``(repo_id=1,
  owner_id=1)`` so existing tree-fetcher tests that do not care
  about ID capture keep working with a single ``seed_tree(...)``
  call. Tests that round-trip captured IDs into the database pass
  ``repo_id=`` / ``owner_id=`` to ``seed_tree`` and the auto-call
  uses those instead. ``seed_repo`` is exposed separately for tests
  that bypass ``seed_tree`` (the syncer's GitHub-error fixture
  needs the repo endpoint mocked but the ref endpoint to return
  404, so it calls ``seed_repo`` then mocks ``commits/{ref}``
  directly).

Next-iteration notes:
- Slice #242 ("rename + installation webhooks") will consume the
  ``github_owner_id`` / ``github_repo_id`` columns this slice now
  populates. When a ``repository.renamed`` event arrives, the
  handler can match the binding by stable ID (the rewrite target)
  and update ``github_owner`` / ``github_repo`` strings without
  re-syncing content. This slice intentionally does not register
  any rename-event handlers — name-keyed pushes still match
  un-synced bindings and ID-keyed pushes still match synced ones,
  so the system is functional without rename handlers; they're a
  staleness-fix, not a correctness requirement.
- The blocker merge of ``tickets/DM-54689-github-webhook`` (PR
  #264) into this branch is a no-op once #264 lands on main first.
  GitHub auto-merge will flatten the merge commit. If #264 is
  rebased instead of merged cleanly, this branch needs a manual
  re-merge.
- ``InstallationAuth.installation_id`` is now required, but the
  GitHub App secrets remain optional on ``Factory`` /
  ``WorkerFactoryBuilder``. The optionality is a feature-flag for
  the dashboard-template-from-github feature; once the feature
  graduates from optional, the three GitHub App fields can be
  tightened to non-Optional and ``startup`` raised earlier — the
  ``InstallationAuth`` change has no bearing on that decision.
- The ``DashboardTemplateBinding`` client model now exposes the
  three numeric ID fields as informational. If a future client
  wants to assert on rename-robustness from the outside, it can
  observe ``github_repo_id`` flipping from ``null`` to populated
  after the first successful sync — no new endpoint needed.

Closes #241

Author: jonathansick

client/src/docverse/client/models/dashboard_template.py

@@ -151,6 +151,34 @@ class DashboardTemplateBinding(BaseModel):
         ),
     )
 
+    github_owner_id: int | None = Field(
+        default=None,
+        description=(
+            "GitHub's stable numeric ID for the owner. Captured on first "
+            "successful sync; ``None`` for un-synced bindings. Informational "
+            "only — the public API remains keyed on ``github_owner``."
+        ),
+    )
+
+    github_repo_id: int | None = Field(
+        default=None,
+        description=(
+            "GitHub's stable numeric ID for the repository. Captured on "
+            "first successful sync; ``None`` for un-synced bindings. "
+            "Informational only — the public API remains keyed on "
+            "``github_repo``."
+        ),
+    )
+
+    github_installation_id: int | None = Field(
+        default=None,
+        description=(
+            "GitHub App installation ID for the repository. Captured on "
+            "first successful sync; ``None`` for un-synced bindings or when "
+            "the GitHub App is not installed. Informational only."
+        ),
+    )
+
     date_created: datetime = Field(
         description="Timestamp when the binding was created."
     )

src/docverse/handlers/orgs/models.py

@@ -535,6 +535,9 @@ def from_domain(
             last_sync_status=domain.last_sync_status,
             last_sync_error=domain.last_sync_error,
             last_sync_queue_job_url=last_sync_queue_job_url,
+            github_owner_id=domain.github_owner_id,
+            github_repo_id=domain.github_repo_id,
+            github_installation_id=domain.github_installation_id,
             date_created=domain.date_created,
             date_updated=domain.date_updated,
         )

src/docverse/services/dashboard_templates/push_processor.py

@@ -9,6 +9,9 @@
 import structlog
 
 from docverse.client.models.dashboard_template import normalize_github_ref
+from docverse.domain.dashboard_github_template import (
+    DashboardGitHubTemplateBinding,
+)
 from docverse.domain.queue import QueueJob
 from docverse.storage.dashboard_templates.github import (
     DashboardGitHubTemplateBindingStore,
@@ -95,10 +98,10 @@ async def process(self, payload: Mapping[str, Any]) -> list[QueueJob]:
             return []
 
         normalized_ref = normalize_github_ref(ref)
-        bindings = await self._binding_store.list_by_repo_ref(
-            github_owner=owner,
-            github_repo=repo_name,
-            github_ref=normalized_ref,
+        repo_id = _coerce_int(repo.get("id"))
+
+        bindings = await self._lookup_bindings(
+            owner=owner, repo=repo_name, ref=normalized_ref, repo_id=repo_id
         )
         if not bindings:
             self._logger.info(
@@ -107,6 +110,7 @@ async def process(self, payload: Mapping[str, Any]) -> list[QueueJob]:
                 github_repo=repo_name,
                 github_ref_raw=ref,
                 github_ref=normalized_ref,
+                github_repo_id=repo_id,
             )
             return []
 
@@ -143,6 +147,51 @@ async def process(self, payload: Mapping[str, Any]) -> list[QueueJob]:
         )
         return jobs
 
+    async def _lookup_bindings(
+        self,
+        *,
+        owner: str,
+        repo: str,
+        ref: str,
+        repo_id: int | None,
+    ) -> list[DashboardGitHubTemplateBinding]:
+        """Find bindings for a push, preferring ``github_repo_id``.
+
+        Two queries, unioned and deduplicated by binding id:
+
+        - ``list_by_repo_id_and_ref`` — when the payload carries a
+          ``repository.id``, this is the rename-robust primary lookup.
+          Bindings that completed at least one successful sync have
+          their ``github_repo_id`` populated and match here even when
+          the GitHub display name has since changed.
+        - ``list_unsynced_by_repo_ref`` — fallback for bindings that
+          have never synced (and therefore lack a stable ID). The
+          ``github_repo_id IS NULL`` filter inside the store keeps
+          synced bindings whose name happens to coincide with this
+          push out of the result, so a different repo now sitting at
+          the old name does not accidentally trigger a sync.
+
+        Dedup defends against any future loosening of either query.
+        """
+        seen: set[int] = set()
+        bindings: list[DashboardGitHubTemplateBinding] = []
+        if repo_id is not None:
+            for binding in await self._binding_store.list_by_repo_id_and_ref(
+                github_repo_id=repo_id, github_ref=ref
+            ):
+                if binding.id in seen:
+                    continue
+                seen.add(binding.id)
+                bindings.append(binding)
+        for binding in await self._binding_store.list_unsynced_by_repo_ref(
+            github_owner=owner, github_repo=repo, github_ref=ref
+        ):
+            if binding.id in seen:
+                continue
+            seen.add(binding.id)
+            bindings.append(binding)
+        return bindings
+
     async def _resolve_changed_paths(
         self, payload: Mapping[str, Any], *, owner: str, repo: str
     ) -> list[str]:
@@ -180,6 +229,21 @@ def _split_full_name_tuple(full_name: object) -> tuple[str, str] | None:
     return None
 
 
+def _coerce_int(value: object) -> int | None:
+    """Return ``value`` as ``int`` when it is a non-bool int, else ``None``.
+
+    GitHub webhooks send numeric IDs as JSON ints, but defensive
+    payload parsing prefers a strict guard: the ``not isinstance(bool)``
+    clause keeps the ``isinstance(True, int)`` quirk from leaking a
+    truth value through as the repo id.
+    """
+    if isinstance(value, bool):
+        return None
+    if isinstance(value, int):
+        return value
+    return None
+
+
 def _normalize_root(root_path: str) -> str:
     return root_path.strip("/")
 

src/docverse/services/dashboard_templates/sync.py

@@ -219,13 +219,18 @@ async def sync(self, binding_id: int) -> DashboardSyncResult:
             etag=etag,
             template_toml=template_toml,
             files=other_files,
+            github_owner_id=fetched.owner_id,
+            github_repo_id=fetched.repo_id,
         )
 
         updated = await self._binding_store.update_sync_state(
             binding_id=binding.id,
             last_sync_status="succeeded",
             last_sync_error=None,
             github_template_id=upsert.template.id,
+            github_owner_id=fetched.owner_id,
+            github_repo_id=fetched.repo_id,
+            github_installation_id=auth.installation_id,
         )
         if updated is None:
             msg = (

src/docverse/storage/dashboard_templates/github/binding_store.py

@@ -221,6 +221,62 @@ async def list_by_repo_ref(
             for binding_row, public_id, commit_sha in result
         ]
 
+    async def list_by_repo_id_and_ref(
+        self,
+        *,
+        github_repo_id: int,
+        github_ref: str,
+    ) -> list[DashboardGitHubTemplateBinding]:
+        """List every binding pinned to a stable GitHub repo ID + ref.
+
+        Backed by the
+        ``idx_dashboard_github_template_bindings_repo_id_ref`` composite
+        index. Used by the push event processor as the primary
+        rename-robust lookup: bindings keep matching their upstream
+        repository even after a GitHub rename or transfer because the
+        numeric ID is invariant.
+        """
+        result = await self._session.execute(
+            select(SqlDashboardGitHubTemplateBinding).where(
+                SqlDashboardGitHubTemplateBinding.github_repo_id
+                == github_repo_id,
+                SqlDashboardGitHubTemplateBinding.github_ref == github_ref,
+            )
+        )
+        return [
+            DashboardGitHubTemplateBinding.model_validate(row)
+            for row in result.scalars().all()
+        ]
+
+    async def list_unsynced_by_repo_ref(
+        self,
+        *,
+        github_owner: str,
+        github_repo: str,
+        github_ref: str,
+    ) -> list[DashboardGitHubTemplateBinding]:
+        """List ``(owner, repo, ref)`` matches that have no captured repo ID.
+
+        The push event processor unions this with
+        :meth:`list_by_repo_id_and_ref` to cover bindings that have
+        never completed a successful sync (and therefore lack a
+        ``github_repo_id``). Already-synced bindings whose name
+        coincidentally matches a different repo are excluded — the
+        ID lookup is the authoritative match for those.
+        """
+        result = await self._session.execute(
+            select(SqlDashboardGitHubTemplateBinding).where(
+                SqlDashboardGitHubTemplateBinding.github_owner == github_owner,
+                SqlDashboardGitHubTemplateBinding.github_repo == github_repo,
+                SqlDashboardGitHubTemplateBinding.github_ref == github_ref,
+                SqlDashboardGitHubTemplateBinding.github_repo_id.is_(None),
+            )
+        )
+        return [
+            DashboardGitHubTemplateBinding.model_validate(row)
+            for row in result.scalars().all()
+        ]
+
     async def list_project_overrides_for_org(
         self, org_id: int
     ) -> list[DashboardGitHubTemplateBinding]:

src/docverse/storage/github/app_client.py

@@ -29,9 +29,16 @@ class InstallationAuth:
     ``base_url`` and attach ``Authorization: Bearer {token}`` on each
     request so the process-wide ``httpx.AsyncClient`` defaults stay
     untouched.
+
+    ``installation_id`` is GitHub's stable identifier for the
+    repository's app installation. The syncer captures it onto the
+    binding so future push events can reference the installation
+    directly without an extra ``/repos/{owner}/{repo}/installation``
+    round-trip.
     """
 
     token: str
+    installation_id: int
     base_url: str = GITHUB_API_BASE_URL
 
 
@@ -147,4 +154,4 @@ async def get_installation_auth(
         """
         installation_id = await self.get_installation_id(owner, repo)
         token = await self.exchange_installation_token(installation_id)
-        return InstallationAuth(token=token)
+        return InstallationAuth(token=token, installation_id=installation_id)

src/docverse/storage/github/tree_fetcher.py

@@ -55,6 +55,12 @@ class FetchedTree:
     ``commit_sha`` is the resolved commit the ref pointed at when the
     tree was read, independent of whether ``ref`` was a branch, tag, or
     raw SHA.
+
+    ``repo_id`` and ``owner_id`` are GitHub's stable numeric IDs for
+    the repository and its owner. The syncer writes them to the
+    binding + content rows so the push event processor can match
+    incoming events by ID rather than by display name, which keeps
+    matching robust across GitHub repo / org renames.
     """
 
     owner: str
@@ -65,6 +71,8 @@ class FetchedTree:
     tree_sha: str
     etag: str | None
     files: tuple[FetchedTreeFile, ...]
+    repo_id: int
+    owner_id: int
 
 
 def _normalize_root(root_path: str) -> str:
@@ -101,10 +109,13 @@ async def fetch(
     ) -> FetchedTree:
         """Fetch the tree under ``root_path`` at ``ref`` for ``owner/repo``.
 
-        Steps: resolve the ref to a commit (giving us ``commit_sha`` +
-        the top-level tree SHA), list the recursive tree, keep only
-        blobs inside ``root_path``, then fetch each blob's bytes.
+        Steps: fetch the repository metadata (for the stable
+        ``repo_id`` / ``owner_id``), resolve the ref to a commit
+        (giving us ``commit_sha`` + the top-level tree SHA), list the
+        recursive tree, keep only blobs inside ``root_path``, then
+        fetch each blob's bytes.
         """
+        repo_id, owner_id = await self._fetch_repo(owner, repo)
         commit_sha, tree_sha = await self._resolve_ref(owner, repo, ref)
         tree_entries, etag = await self._list_tree(owner, repo, tree_sha)
 
@@ -153,6 +164,8 @@ async def _fetch_into(
             tree_sha=tree_sha,
             etag=etag,
             files=tuple(cast("list[FetchedTreeFile]", files)),
+            repo_id=repo_id,
+            owner_id=owner_id,
         )
 
     def _auth_headers(self, accept: str) -> dict[str, str]:
@@ -161,6 +174,22 @@ def _auth_headers(self, accept: str) -> dict[str, str]:
             "Authorization": f"Bearer {self._auth.token}",
         }
 
+    async def _fetch_repo(self, owner: str, repo: str) -> tuple[int, int]:
+        """Fetch repository metadata, returning ``(repo_id, owner_id)``.
+
+        Reads ``GET /repos/{owner}/{repo}`` to capture GitHub's stable
+        numeric IDs for the repository and its owner. The syncer
+        writes these to the binding + content rows so push events
+        match by ID even after a GitHub repo or org rename.
+        """
+        response = await self._http_client.get(
+            f"{self._auth.base_url}/repos/{owner}/{repo}",
+            headers=self._auth_headers("application/vnd.github+json"),
+        )
+        response.raise_for_status()
+        data = response.json()
+        return int(data["id"]), int(data["owner"]["id"])
+
     async def _resolve_ref(
         self, owner: str, repo: str, ref: str
     ) -> tuple[str, str]:

tests/handlers/dashboard_template_test.py

@@ -204,6 +204,61 @@ async def test_org_get_returns_existing_binding(client: AsyncClient) -> None:
     assert body["github_owner"] == "lsst-sqre"
     assert body["github_ref"] == "main"
     assert body["last_sync_status"] == "pending"
+    # Pre-sync the GitHub numeric IDs are unset; the response surfaces
+    # them as ``null`` so operators can see the binding has not yet
+    # captured them.
+    assert body["github_owner_id"] is None
+    assert body["github_repo_id"] is None
+    assert body["github_installation_id"] is None
+
+
+@pytest.mark.asyncio
+async def test_org_get_surfaces_captured_github_numeric_ids(
+    client: AsyncClient,
+) -> None:
+    """A populated binding's GitHub numeric IDs are visible to operators.
+
+    The public API stays name-keyed; the IDs are informational only,
+    surfaced for debugging the rename-robust internal lookup path.
+    """
+    await _setup_org(client)
+    await client.put(
+        f"/docverse/orgs/{_ORG}/dashboard-template",
+        json=_VALID_BODY,
+        headers={"X-Auth-Request-User": _ADMIN},
+    )
+
+    async for session in db_session_dependency():
+        async with session.begin():
+            org_store = OrganizationStore(
+                session=session, logger=structlog.get_logger("test")
+            )
+            org = await org_store.get_by_slug(_ORG)
+            assert org is not None
+            binding_store = DashboardGitHubTemplateBindingStore(
+                session=session, logger=structlog.get_logger("test")
+            )
+            existing = await binding_store.get_org_default(org.id)
+            assert existing is not None
+            await binding_store.update_sync_state(
+                binding_id=existing.id,
+                last_sync_status="succeeded",
+                github_owner_id=111,
+                github_repo_id=222,
+                github_installation_id=333,
+            )
+            await session.commit()
+        break
+
+    response = await client.get(
+        f"/docverse/orgs/{_ORG}/dashboard-template",
+        headers={"X-Auth-Request-User": _ADMIN},
+    )
+    assert response.status_code == 200
+    body = response.json()
+    assert body["github_owner_id"] == 111
+    assert body["github_repo_id"] == 222
+    assert body["github_installation_id"] == 333
 
 
 @pytest.mark.asyncio