Skip to content

Commit 197129d

Browse files
eric-wang-1990eric-wang-1990
committed
feat(bots): reviewer bot via pinned databricks-bot-engine (PAT-free)
Onboard the PR reviewer to the upstream engine (pinned SHA b6205fb), replacing the previously-vendored scripts/reviewer_bot. Own-job workflows: mint the review-bot App token, install the engine via the local composite (engine-scoped App token, no PAT), run reviewer_bot.run_review / .followup. Trigger: every non-fork PR. .bot/prompts/review/system.md is repo-specific ADDITIVE guidance appended to the engine-owned base prompt. Verified live — posted a review as peco-review-bot on this PR. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
1 parent 6a0cd78 commit 197129d

3 files changed

Lines changed: 294 additions & 0 deletions

File tree

‎.bot/prompts/review/system.md‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
Repo-specific review guidance for `databricks-sql-python` (the Databricks SQL
2+
connector for Python). This is ADDITIVE context appended to the engine-owned
3+
reviewer base prompt — it does not change the output contract, severity scale,
4+
or anchoring/dedup rules the base already defines.
5+
6+
You are reviewing the Databricks SQL connector for Python. Work through each
7+
review axis against the changed code — a clean-looking diff still warrants
8+
checking every one; don't stop at the first pass or finalize with "looks good"
9+
until you've actually considered these:
10+
11+
- **Correctness & logic:** off-by-one, inverted/incorrect conditionals, wrong
12+
parameter passing, broken control flow, state left inconsistent, resource
13+
leaks, results silently dropped.
14+
- **Error handling:** swallowed or over-broad exceptions, silent failures,
15+
fallbacks that hide errors, missing propagation, unchecked return values.
16+
- **Tests & coverage:** behavior changed without a test; assertions removed or
17+
weakened; tests that can't actually fail; missing edge-case coverage for the
18+
new/changed behavior.
19+
- **Edge cases & inputs:** null / empty / boundary values, ordering and
20+
concurrency, encoding, large inputs, partial failure.
21+
- **Contracts & API:** signature or behavior changes that break callers;
22+
comments / docstrings that no longer match the code; documented invariants
23+
violated. This is a widely-consumed connector — public-API stability matters.
24+
- **Security:** injection, credential handling, path traversal, unsafe
25+
deserialization.
26+
- **Repo conventions:** PEP 8 with a **100-char** line limit (per
27+
`CONTRIBUTING.md`, not 79), type hints, and the patterns in `CONTRIBUTING.md`
28+
/ `README.md`.
29+
30+
Landmarks for this repo:
31+
- Conventions live in `CONTRIBUTING.md` (coding style: PEP 8 with a 100-char
32+
line limit; DCO sign-off requirement) and `README.md`. When a finding is
33+
convention-anchored, cite the exact rule line.
34+
- The connector package is under `src/databricks/`; tests are pytest-based under
35+
`tests/unit` (fast, mocked) and `tests/e2e` (integration against a warehouse).
36+
New or changed behavior under `src/` should carry corresponding `tests/unit`
37+
coverage.

‎.github/workflows/reviewer-bot-followup.yml‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
# Reviewer Bot — follow-up on review-comment replies.
2+
#
3+
# Own-job composite caller (see reviewer-bot.yml for why we don't use the engine
4+
# reusable WORKFLOW: cross-repo reusable-workflow calls to the private engine
5+
# fail "workflow was not found"; cross-repo composite ACTIONS resolve in-org).
6+
# This file owns the job and replicates the reusable followup's steps with
7+
# cross-repo action refs, PAT-free (engine-scoped App token).
8+
#
9+
# Enablement: no label gate — engage every non-fork open PR (this repo's policy).
10+
# The security floor (fork==false, PR open) is the job `if:` below; loop-
11+
# prevention lives in the engine's followup.py post-checkout.
12+
name: Reviewer Bot — Follow-up
13+
14+
on:
15+
pull_request_review_comment:
16+
types: [created]
17+
pull_request:
18+
types: [synchronize]
19+
20+
permissions:
21+
contents: read
22+
pull-requests: write
23+
id-token: write # JFrog OIDC exchange for the engine/SDK/CLI install
24+
25+
jobs:
26+
followup:
27+
# SECURITY FLOOR: never run the agent on a fork PR (keeps App + model tokens
28+
# off untrusted code); only act on OPEN PRs.
29+
if: >-
30+
github.event.pull_request.head.repo.fork == false
31+
&& github.event.pull_request.state == 'open'
32+
environment: azure-prod # DATABRICKS_HOST / DATABRICKS_TOKEN live here
33+
runs-on:
34+
group: databricks-protected-runner-group
35+
labels: [linux-ubuntu-latest]
36+
timeout-minutes: 20
37+
concurrency:
38+
group: reviewer-bot-followup-pr-${{ github.event.pull_request.number }}
39+
cancel-in-progress: false
40+
steps:
41+
# Review-bot token (posts inline replies + resolves threads).
42+
- name: Mint review-bot App token
43+
id: app-token
44+
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
45+
with:
46+
app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
47+
private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
48+
49+
- name: Checkout PR head (read-only; agent only POSTS via GH_TOKEN)
50+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
51+
with:
52+
ref: ${{ github.event.pull_request.head.sha }}
53+
fetch-depth: 0
54+
persist-credentials: false
55+
56+
# followup.py runs `git rev-list <base>..<head>` for finding-anchor SHA
57+
# verification. fetch-depth:0 usually brings the base along, but not
58+
# guaranteed. Best-effort fetch when missing; NEVER fatal — a missing base
59+
# makes the rev-list helper fail-closed to an empty allowlist (SHA
60+
# verification degrades, reconciliation still runs). The persist-
61+
# credentials:false checkout means this fetch has no auth, so it may fail
62+
# on a private repo; swallow it and warn.
63+
- name: Ensure PR base commit is present (for rev-list base..head)
64+
env:
65+
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
66+
run: |
67+
if git cat-file -e "${PR_BASE_SHA}^{commit}" 2>/dev/null; then
68+
exit 0
69+
fi
70+
if ! git fetch --no-tags --depth=1 origin "$PR_BASE_SHA"; then
71+
echo "::warning::Could not fetch PR base commit ${PR_BASE_SHA}. Continuing: followup.py fail-closes to an empty SHA allowlist, so SHA verification degrades gracefully but reconciliation still runs." >&2
72+
fi
73+
74+
- name: Setup Python
75+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
76+
with:
77+
python-version: '3.11'
78+
79+
- name: Setup Node (for the Claude Code CLI the SDK spawns)
80+
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
81+
with:
82+
node-version: '20'
83+
84+
# PAT-free engine install: engine-scoped token (contents:read) from the
85+
# review-bot App creds — SEPARATE from app-token above (which is scoped to
86+
# THIS repo for PR posting).
87+
- name: Mint engine-scoped install token
88+
id: engine-token
89+
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
90+
with:
91+
app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
92+
private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
93+
owner: databricks
94+
repositories: databricks-bot-engine
95+
permission-contents: read
96+
97+
# LOCAL composite (cross-repo `uses:` of the engine won't resolve here).
98+
- name: Install engine + Claude SDK/CLI
99+
uses: ./.github/actions/install-bot-engine
100+
with:
101+
engine-ref: b6205fb502566d617a456ccf3c37f3e4e3072ccd
102+
engine-token: ${{ steps.engine-token.outputs.token }}
103+
104+
- name: Run reviewer follow-up
105+
env:
106+
GH_TOKEN: ${{ steps.app-token.outputs.token }}
107+
GITHUB_REPOSITORY: ${{ github.repository }}
108+
PR_NUMBER: ${{ github.event.pull_request.number }}
109+
TRIGGER_COMMENT_ID: ${{ github.event.comment.id }}
110+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
111+
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
112+
MODEL_ENDPOINT: https://${{ secrets.DATABRICKS_HOST }}/serving-endpoints/databricks-claude-opus-4-8/invocations
113+
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
114+
DRY_RUN: 'false'
115+
RUNNER_TEMP: ${{ runner.temp }}
116+
run: python -m databricks_bot_engine.reviewer_bot.followup

‎.github/workflows/reviewer-bot.yml‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
# Reviewer Bot — initial PR review.
2+
#
3+
# Own-job composite caller of the databricks-bot-engine actions. We deliberately
4+
# do NOT use the engine's reusable WORKFLOW (jobs.<id>.uses:
5+
# .../reviewer-bot.reusable.yml@<sha>): a cross-repo reusable-workflow call to
6+
# the private engine fails to resolve ("workflow was not found"). Cross-repo
7+
# composite ACTIONS (steps: - uses: .../.github/actions/...@<sha>) DO resolve
8+
# in-org — this is the pattern databricks-driver-test's engineer bot uses in
9+
# production, and the one the engineer-bot workflows here use too. So this file
10+
# owns the job and replicates the reusable's steps (fork gate, token mint,
11+
# install, run) with cross-repo action refs.
12+
#
13+
# Engine pin: the SHA below pins BOTH the action refs AND engine-ref, so the
14+
# workflow and installed engine match. Bump in lockstep; never @main.
15+
#
16+
# Auth: PAT-free. We mint a short-lived, engine-scoped App installation token
17+
# from the review-bot App creds and pass it as engine-pat to install-bot-engine
18+
# (no stored BOT_ENGINE_PAT).
19+
name: Reviewer Bot
20+
21+
on:
22+
pull_request:
23+
types: [opened, synchronize, reopened, ready_for_review]
24+
workflow_dispatch:
25+
inputs:
26+
pr_number:
27+
description: 'PR number to review'
28+
required: true
29+
type: string
30+
dry_run:
31+
description: 'Print what would be posted instead of posting'
32+
required: false
33+
default: 'true'
34+
type: string
35+
36+
permissions:
37+
contents: read
38+
pull-requests: write
39+
id-token: write # JFrog OIDC exchange for the engine/SDK/CLI install
40+
41+
jobs:
42+
review:
43+
# SECURITY FLOOR (was the reusable's job `if:`): manual dispatch, OR a
44+
# non-draft, non-fork PR. fork == false keeps the App + model tokens off
45+
# untrusted code.
46+
if: >-
47+
github.event_name == 'workflow_dispatch'
48+
|| (github.event.pull_request.draft == false
49+
&& github.event.pull_request.head.repo.fork == false)
50+
environment: azure-prod # DATABRICKS_HOST / DATABRICKS_TOKEN live here
51+
runs-on:
52+
group: databricks-protected-runner-group
53+
labels: [linux-ubuntu-latest]
54+
timeout-minutes: 30
55+
steps:
56+
# Checkout FIRST. Default ref = the PR merge ref on pull_request, the
57+
# default branch on dispatch. persist-credentials:false — the reviewer
58+
# reads this tree via read_paths/grep, so no token may sit in .git/config.
59+
- name: Checkout
60+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
61+
with:
62+
fetch-depth: 0
63+
persist-credentials: false
64+
65+
# Mint the review-bot token (posts findings).
66+
- name: Mint review-bot App token
67+
id: setup
68+
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
69+
with:
70+
app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
71+
private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
72+
73+
- name: Setup Python
74+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
75+
with:
76+
python-version: '3.11'
77+
78+
- name: Setup Node (for the Claude Code CLI the SDK spawns)
79+
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
80+
with:
81+
node-version: '20'
82+
83+
# PAT-free engine install: mint a per-run token scoped to ONLY the engine
84+
# repo (contents:read) from the review-bot App creds. Requires the
85+
# review-bot App installed on databricks-bot-engine with contents:read.
86+
- name: Mint engine-scoped install token
87+
id: engine-token
88+
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
89+
with:
90+
app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
91+
private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
92+
owner: databricks
93+
repositories: databricks-bot-engine
94+
permission-contents: read
95+
96+
# LOCAL composite (not a cross-repo `uses:` of the engine — that fails to
97+
# resolve from this external repo). It pip-installs the pinned engine via
98+
# the engine-scoped token.
99+
- name: Install engine + Claude SDK/CLI
100+
uses: ./.github/actions/install-bot-engine
101+
with:
102+
engine-ref: b6205fb502566d617a456ccf3c37f3e4e3072ccd
103+
engine-token: ${{ steps.engine-token.outputs.token }}
104+
105+
- name: Resolve trigger inputs
106+
id: inputs
107+
# SECURITY: dispatcher-supplied values pass via env, never interpolated
108+
# into the script body (an inline expression would inject at assignment,
109+
# before the digits-only check). Env values expand without re-parsing.
110+
env:
111+
GH_TOKEN: ${{ steps.setup.outputs.token }}
112+
EVENT_NAME: ${{ github.event_name }}
113+
INPUT_PR: ${{ inputs.pr_number }}
114+
INPUT_DRY_RUN: ${{ inputs.dry_run }}
115+
EVENT_PR: ${{ github.event.pull_request.number }}
116+
REPO: ${{ github.repository }}
117+
run: |
118+
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
119+
RAW_PR="$INPUT_PR"; DRY_RUN="$INPUT_DRY_RUN"
120+
else
121+
RAW_PR="$EVENT_PR"; DRY_RUN="false"
122+
fi
123+
[[ "$RAW_PR" =~ ^[0-9]+$ ]] || { echo "::error::Invalid pr_number '$RAW_PR'"; exit 1; }
124+
case "$DRY_RUN" in true|false) ;; *) DRY_RUN="true" ;; esac # fail safe
125+
HEAD_SHA=$(gh pr view "$RAW_PR" --repo "$REPO" --json headRefOid -q .headRefOid)
126+
{ echo "pr_number=$RAW_PR"; echo "head_sha=$HEAD_SHA"; echo "dry_run=$DRY_RUN"; } >> "$GITHUB_OUTPUT"
127+
128+
- name: Run reviewer
129+
env:
130+
GH_TOKEN: ${{ steps.setup.outputs.token }}
131+
GITHUB_REPOSITORY: ${{ github.repository }}
132+
PR_NUMBER: ${{ steps.inputs.outputs.pr_number }}
133+
HEAD_SHA: ${{ steps.inputs.outputs.head_sha }}
134+
DRY_RUN: ${{ steps.inputs.outputs.dry_run }}
135+
MODEL_ENDPOINT: https://${{ secrets.DATABRICKS_HOST }}/serving-endpoints/databricks-claude-opus-4-8/invocations
136+
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
137+
RUNNER_TEMP: ${{ runner.temp }}
138+
# The reviewer's repo-specific ADDITIVE guidance, read from the TRUSTED
139+
# checkout (run_review enforces containment). Engine owns the base prompt.
140+
REVIEW_SYSTEM_PROMPT: .bot/prompts/review/system.md
141+
run: python -m databricks_bot_engine.reviewer_bot.run_review

0 commit comments

Comments
 (0)