Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .bot/prompts/review/system.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
Repo-specific review guidance for `databricks-sql-python` (the Databricks SQL
connector for Python). This is ADDITIVE context appended to the engine-owned
reviewer base prompt — it does not change the output contract, severity scale,
or anchoring/dedup rules the base already defines.

You are reviewing the Databricks SQL connector for Python. Work through each
review axis against the changed code — a clean-looking diff still warrants
checking every one; don't stop at the first pass or finalize with "looks good"
until you've actually considered these:

- **Correctness & logic:** off-by-one, inverted/incorrect conditionals, wrong
parameter passing, broken control flow, state left inconsistent, resource
leaks, results silently dropped.
- **Error handling:** swallowed or over-broad exceptions, silent failures,
fallbacks that hide errors, missing propagation, unchecked return values.
- **Tests & coverage:** behavior changed without a test; assertions removed or
weakened; tests that can't actually fail; missing edge-case coverage for the
new/changed behavior.
- **Edge cases & inputs:** null / empty / boundary values, ordering and
concurrency, encoding, large inputs, partial failure.
- **Contracts & API:** signature or behavior changes that break callers;
comments / docstrings that no longer match the code; documented invariants
violated. This is a widely-consumed connector — public-API stability matters.
- **Security:** injection, credential handling, path traversal, unsafe
deserialization.
- **Repo conventions:** PEP 8 with a **100-char** line limit (per
`CONTRIBUTING.md`, not 79), type hints, and the patterns in `CONTRIBUTING.md`
/ `README.md`.

Landmarks for this repo:
- Conventions live in `CONTRIBUTING.md` (coding style: PEP 8 with a 100-char
line limit; DCO sign-off requirement) and `README.md`. When a finding is
convention-anchored, cite the exact rule line.
- The connector package is under `src/databricks/`; tests are pytest-based under
`tests/unit` (fast, mocked) and `tests/e2e` (integration against a warehouse).
New or changed behavior under `src/` should carry corresponding `tests/unit`
coverage.
78 changes: 78 additions & 0 deletions .github/actions/setup-jfrog/setup-jfrog/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name: Setup JFrog OIDC
description: Obtain a JFrog access token via GitHub OIDC and configure package managers

inputs:
configure-pip:
description: Configure pip to use JFrog PyPI proxy
default: "true"
configure-cargo:
description: Configure Cargo to use JFrog crates proxy
default: "false"
configure-npm:
description: Configure npm to use JFrog npm proxy
default: "false"

runs:
using: composite
steps:
- name: Get JFrog OIDC token
shell: bash
run: |
set -euo pipefail
ID_TOKEN=$(curl -sLS \
-H "User-Agent: actions/oidc-client" \
-H "Authorization: Bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \
"${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=jfrog-github" | jq .value | tr -d '"')
echo "::add-mask::${ID_TOKEN}"
ACCESS_TOKEN=$(curl -sLS -XPOST -H "Content-Type: application/json" \
"https://databricks.jfrog.io/access/api/v1/oidc/token" \
-d "{\"grant_type\": \"urn:ietf:params:oauth:grant-type:token-exchange\", \"subject_token_type\":\"urn:ietf:params:oauth:token-type:id_token\", \"subject_token\": \"${ID_TOKEN}\", \"provider_name\": \"github-actions\"}" | jq .access_token | tr -d '"')
echo "::add-mask::${ACCESS_TOKEN}"
if [ -z "$ACCESS_TOKEN" ] || [ "$ACCESS_TOKEN" = "null" ]; then
echo "FAIL: Could not extract JFrog access token"
exit 1
fi
echo "JFROG_ACCESS_TOKEN=${ACCESS_TOKEN}" >> "$GITHUB_ENV"
echo "JFrog OIDC token obtained successfully"

- name: Configure pip
if: inputs.configure-pip == 'true'
shell: bash
run: |
set -euo pipefail
echo "PIP_INDEX_URL=https://gha-service-account:${JFROG_ACCESS_TOKEN}@databricks.jfrog.io/artifactory/api/pypi/db-pypi/simple" >> "$GITHUB_ENV"
echo "pip configured to use JFrog registry"

- name: Configure Cargo
if: inputs.configure-cargo == 'true'
shell: bash
run: |
set -euo pipefail
mkdir -p ~/.cargo
cat > ~/.cargo/config.toml << EOF
[source.crates-io]
replace-with = "jfrog"
[source.jfrog]
registry = "sparse+https://databricks.jfrog.io/artifactory/api/cargo/db-cargo-remote/index/"
[registries.jfrog]
index = "sparse+https://databricks.jfrog.io/artifactory/api/cargo/db-cargo-remote/index/"
credential-provider = ["cargo:token"]
EOF
cat > ~/.cargo/credentials.toml << EOF
[registries.jfrog]
token = "Bearer ${JFROG_ACCESS_TOKEN}"
EOF
echo "CARGO_REGISTRIES_JFROG_TOKEN=Bearer ${JFROG_ACCESS_TOKEN}" >> "$GITHUB_ENV"
echo "Cargo configured to use JFrog registry"

- name: Configure npm
if: inputs.configure-npm == 'true'
shell: bash
run: |
set -euo pipefail
cat > ~/.npmrc << EOF
registry=https://databricks.jfrog.io/artifactory/api/npm/db-npm/
//databricks.jfrog.io/artifactory/api/npm/db-npm/:_authToken=${JFROG_ACCESS_TOKEN}
always-auth=true
EOF
echo "npm configured to use JFrog registry"
38 changes: 38 additions & 0 deletions .github/workflows/reviewer-bot-followup.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Reviewer Bot — follow-up on review-comment replies.
#
# Thin caller of the databricks-bot-engine reusable workflow. The engine owns
# the security + correctness floor (fork==false, PR open), token mint, engine
# install, and the agent invocation; loop-prevention (skip own reply / reconcile
# / non-reply comments) lives in the engine's followup.py post-checkout. This
# file is just `on:` + `uses:` + `secrets:`.
#
# Enablement policy: NO `if:` gate — engage every non-fork open PR, matching this
# repo's prior behavior. (To restrict later, add a caller-job `if:`, e.g. a
# `review-bot` label gate; it ANDs with the engine's security floor.)
#
# Engine pin: the SHA below is used in BOTH the `uses:` ref AND `engine-ref:`.
# Bump both in lockstep with reviewer-bot.yml; never use @main.
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 # JFrog OIDC exchange for the engine/SDK/CLI install

jobs:
followup:
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 }}
47 changes: 47 additions & 0 deletions .github/workflows/reviewer-bot.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Reviewer Bot — initial PR review.
#
# Thin caller of the databricks-bot-engine reusable workflow. The engine owns
# the whole job (security gate, App-token mint, Python setup, engine install,
# MODEL_ENDPOINT construction, workflow_dispatch input validation, and the
# PR-head content-root dispatch security); this file is just `on:` + `uses:` +
# `secrets:`. See docs/superpowers/specs/2026-07-14-onboard-bot-engine-design.md
# and the databricks-bot-engine README ("Adopting the bots in your repo").
#
# Engine pin: the SHA below is used in BOTH the `uses:` ref AND `engine-ref:`
# so the workflow and the installed engine match. Bump both in lockstep; never
# use @main (force-pushable, and the job carries secrets).
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 what would be posted instead of posting'
required: false
default: 'true'
type: string

permissions:
contents: read
pull-requests: write
id-token: write # JFrog OIDC exchange for the engine/SDK/CLI install

jobs:
review:
uses: databricks/databricks-bot-engine/.github/workflows/reviewer-bot.reusable.yml@d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d
with:
engine-ref: d780b2da60bb1ac68bb5cd1acb7cabf495b3ff2d
pr-number: ${{ inputs.pr_number }} # empty on pull_request events
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 }}
186 changes: 186 additions & 0 deletions docs/superpowers/specs/2026-07-14-onboard-bot-engine-design.md
Original file line number Diff line number Diff line change
@@ -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.
Loading