docs: design for onboarding to pinned databricks-bot-engine

Co-authored-by: Isaac

Author: eric-wang-1990

docs/superpowers/specs/2026-07-14-onboard-bot-engine-design.md

@@ -0,0 +1,186 @@
+# Onboard databricks-sql-python to the pinned databricks-bot-engine
+
+**Date:** 2026-07-14
+**Status:** Approved (pending spec review)
+**Repo:** `databricks/databricks-sql-python`
+
+## Problem
+
+The repo currently runs the PR **reviewer bot** from *vendored* code under
+`scripts/reviewer_bot/` + `scripts/shared/`, invoked as
+`python -m scripts.reviewer_bot.*`. The upstream `databricks/databricks-bot-engine`
+is the documented source of truth: consumers `pip install` it pinned to an
+immutable `engine-ref` and call its **reusable workflows**, rather than copying
+its code. The vendored copy has already drifted from the engine and will not pick
+up engine fixes without hand-resync.
+
+This work converts the repo to the documented consumer model, matching the
+existing `databricks/databricks-driver-test` consumer.
+
+## Goal
+
+- Reviewer bot (initial review + follow-up) runs from the pinned engine via its
+  reusable workflows, installed in REF mode.
+- All vendored bot code removed; engine is the single source of truth.
+- Trigger policy unchanged: every non-fork PR (no label gate).
+- Repo-specific review guidance preserved as engine *additive* prompt.
+
+Non-goals: adopting the **engineer bot** (bug-fix / coverage). Only the reviewer
+is in scope, matching what the repo runs today.
+
+## Engine pin
+
+- **Engine SHA:** `d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d` (current
+  `databricks-bot-engine` `main` HEAD).
+- The SHA is used in BOTH the `uses:` ref AND the `engine-ref:` input of each
+  caller, so the workflow definition and the installed engine match. Never
+  `@main` (force-pushable; the job carries secrets).
+
+## Design
+
+### 1. Workflows — thin callers of the engine reusables
+
+**`.github/workflows/reviewer-bot.yml`** (replaces the current hand-rolled job):
+```yaml
+name: Reviewer Bot
+on:
+  pull_request:
+    types: [opened, synchronize, reopened, ready_for_review]
+  workflow_dispatch:
+    inputs:
+      pr_number: { description: 'PR number to review', required: true, type: string }
+      dry_run:   { description: 'Print instead of posting', required: false, default: 'true', type: string }
+permissions:
+  contents: read
+  pull-requests: write
+  id-token: write
+jobs:
+  review:
+    uses: databricks/databricks-bot-engine/.github/workflows/reviewer-bot.reusable.yml@d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d
+    with:
+      engine-ref: d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d
+      pr-number: ${{ inputs.pr_number }}
+      dry-run: ${{ inputs.dry_run }}
+    secrets:
+      review-bot-app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
+      review-bot-app-private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
+      databricks-token: ${{ secrets.DATABRICKS_TOKEN }}
+      databricks-host: ${{ secrets.DATABRICKS_HOST }}
+      engine-pat: ${{ secrets.BOT_ENGINE_PAT }}
+```
+
+**`.github/workflows/reviewer-bot-followup.yml`** (replaces the current job):
+```yaml
+name: Reviewer Bot — Follow-up
+on:
+  pull_request_review_comment: { types: [created] }
+  pull_request: { types: [synchronize] }
+permissions:
+  contents: read
+  pull-requests: write
+  id-token: write
+jobs:
+  followup:
+    # No enablement `if:` — keep today's "every non-fork PR" policy. The engine
+    # reusable's job `if:` still enforces the security floor (fork==false, PR
+    # open) and loop-prevention lives in the engine's followup.py.
+    uses: databricks/databricks-bot-engine/.github/workflows/reviewer-bot-followup.reusable.yml@d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d
+    with:
+      engine-ref: d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d
+    secrets:
+      review-bot-app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
+      review-bot-app-private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
+      databricks-token: ${{ secrets.DATABRICKS_TOKEN }}
+      databricks-host: ${{ secrets.DATABRICKS_HOST }}
+      engine-pat: ${{ secrets.BOT_ENGINE_PAT }}
+```
+
+The engine reusables own: the fork/open-PR security gate (job `if:`), App-token
+mint, Python setup, JFrog-OIDC engine install, `MODEL_ENDPOINT` construction
+(`https://$DATABRICKS_HOST/serving-endpoints/databricks-claude-opus-4-8/invocations`),
+`workflow_dispatch` input validation, and (for `reviewer-bot`) the PR-head
+content-root dispatch security. So the ~250 lines of hand-rolled gate / checkout /
+pre-filter logic in the current workflows are removed.
+
+### 2. `.bot/prompts/review/system.md` — repo-specific additive prompt
+
+Verified against the engine source at the pinned SHA
+(`reviewer_bot/run_review.py::_review_system_prompt`): the reviewer's **base**
+system prompt is **engine-owned** (`SYSTEM_PROMPT` — it owns the output contract,
+severity scale, anchoring, and dedup rules). The consumer file is read from the
+default path `.bot/prompts/review/system.md` and **appended** as *additive* repo
+guidance via `append_repo_guidance`. It is **optional** at the default path
+(missing ⇒ engine base alone — no abort). It is read from the TRUSTED checkout
+(REPO_ROOT), never the PR-head tree (fork prompt-injection guard).
+
+> Note: the engine README describes this file as "the reviewer's *entire* system
+> prompt (absent ⇒ runtime abort)." The **code** at this SHA disagrees — it is
+> additive and optional. We follow the code.
+
+We port only the *repo-specific* parts of the current vendored `SYSTEM_PROMPT`
+and drop the engine-owned parts (severity scheme, anchor rules, output structure,
+dedup, `finalize_review` contract), which the base already owns. Kept:
+- "senior code reviewer for databricks-sql-python (the Databricks SQL connector
+  for Python)" framing + the review axes tuned for this repo.
+- Landmarks: `CONTRIBUTING.md` (PEP 8, 100-char limit, DCO sign-off), `README.md`,
+  connector package under `src/databricks/`, tests under `tests/unit` (fast,
+  mocked) + `tests/e2e` (warehouse integration).
+
+No `config.yaml`, engineer prompts, or `context-repos.yaml` — none are read by the
+reviewer path.
+
+### 3. Deletions (vendored removal)
+
+Delete:
+- `scripts/reviewer_bot/` (runtime + tests)
+- `scripts/shared/` (runtime + tests)
+- `scripts/__init__.py` (only present to package the vendored bot; `dependency_manager.py` runs as a path script)
+- `scripts/requirements-sdk.txt`
+- `.github/actions/setup-claude-sdk/`
+- `.github/workflows/sdk-smoke.yml` (the reviewer-bot *foundation* smoke test; the only consumer of `setup-claude-sdk`)
+- `.github/workflows/reviewer-bot-unit-tests.yml` (its tests import the deleted `scripts.reviewer_bot` / `scripts.shared`)
+
+**Keep** (used by unrelated CI — verified via grep):
+- `.github/actions/setup-jfrog/` — used by `kernel-e2e.yml` and `setup-poetry`.
+- `scripts/dependency_manager.py` — used by `code-quality-checks.yml`.
+
+### 4. Secrets (provisioned by the maintainer, documented in the PR)
+
+| Secret | Status | Purpose |
+|---|---|---|
+| `REVIEW_BOT_APP_ID` | exists | mint bot App token |
+| `REVIEW_BOT_APP_PRIVATE_KEY` | exists | " |
+| `DATABRICKS_TOKEN` | exists | model auth |
+| `DATABRICKS_HOST` | **NEW** — replaces `MODEL_ENDPOINT` | engine builds `MODEL_ENDPOINT` from it |
+| `BOT_ENGINE_PAT` | **NEW** | read access to the private engine repo (REF-mode install) |
+
+The old `MODEL_ENDPOINT` secret is no longer consumed after this change.
+
+## Testing / verification
+
+- Static: `actionlint` on the new workflow YAML if available, else a YAML parse
+  check; confirm no dangling references to deleted paths remain (grep).
+- Runtime: the reusable `uses:` against the **private** engine repo cannot fully
+  resolve in CI until `BOT_ENGINE_PAT` + `DATABRICKS_HOST` are added as secrets.
+  End-to-end verification (a real PR review) happens after secrets are
+  provisioned. This limitation is stated plainly in the PR description — the PR
+  is not claimed "verified working end-to-end."
+
+## Rollout
+
+1. New branch off `main`; commit workflows + `.bot/` + deletions.
+2. Open PR to `databricks/databricks-sql-python` with the secrets checklist.
+3. Maintainer adds `DATABRICKS_HOST` + `BOT_ENGINE_PAT`.
+4. Merge; first non-fork PR exercises the reviewer end-to-end.
+5. Future engine updates: bump the SHA in both `uses:` and `engine-ref:` in
+   lockstep across both workflows.
+
+## Risks
+
+- **Private-engine install:** REF mode fails without `BOT_ENGINE_PAT`; the engine
+  action fails fast with a clear error. Documented.
+- **README vs. code drift** on the review prompt semantics: resolved by following
+  the code (additive/optional), recorded above.
+- **Reusable `uses: ./…` cross-repo resolution:** the engine's reusables use local
+  `./.github/actions/...` refs that resolve against the engine repo at the called
+  ref (verified in the engine's own dogfood). No consumer action needed.