Skip to content

Commit 6cefc02

Browse files
eric-wang-1990eric-wang-1990
committed
feat(bots): reviewer bot via pinned databricks-bot-engine (PAT-free)
Reviewer + reviewer-followup as own-job workflows that install the engine via the local bot-prelude composite (PAT-free App-token auth) and run reviewer_bot.run_review / .followup. Triggers on every non-fork PR; fork-isolation gate in each job `if:`. .bot/prompts/review/system.md is repo-specific ADDITIVE guidance appended to the engine's base reviewer prompt. Verified live — posts a review with inline findings on this PR. Signed-off-by: Eric Wang <e.wang@databricks.com> Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
1 parent e6404d9 commit 6cefc02

3 files changed

Lines changed: 258 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: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Reviewer Bot — follow-up on review-comment replies.
2+
#
3+
# Own-job workflow (see reviewer-bot.yml: an external repo can't `uses:` the
4+
# internal engine's actions/workflows; the engine is pip-installed via the local
5+
# bot-prelude -> install-bot-engine composites). This file owns checkout, the
6+
# base-commit fetch, Setup Python, and the followup run; bot-prelude mints tokens
7+
# + Node + installs the engine, PAT-free.
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+
- name: Checkout PR head (read-only; agent only POSTS via GH_TOKEN)
42+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
43+
with:
44+
ref: ${{ github.event.pull_request.head.sha }}
45+
fetch-depth: 0
46+
persist-credentials: false
47+
48+
# followup.py runs `git rev-list <base>..<head>` for finding-anchor SHA
49+
# verification. fetch-depth:0 usually brings the base along, but not
50+
# guaranteed. Best-effort fetch when missing; NEVER fatal — a missing base
51+
# makes the rev-list helper fail-closed to an empty allowlist (SHA
52+
# verification degrades, reconciliation still runs). The persist-
53+
# credentials:false checkout means this fetch has no auth, so it may fail
54+
# on a private repo; swallow it and warn.
55+
- name: Ensure PR base commit is present (for rev-list base..head)
56+
env:
57+
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
58+
run: |
59+
if git cat-file -e "${PR_BASE_SHA}^{commit}" 2>/dev/null; then
60+
exit 0
61+
fi
62+
if ! git fetch --no-tags --depth=1 origin "$PR_BASE_SHA"; then
63+
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
64+
fi
65+
66+
# Reviewer reads code, doesn't run the connector — Python interpreter only.
67+
- name: Setup Python
68+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
69+
with:
70+
python-version: '3.11'
71+
72+
# Shared prelude: mint the review-bot token (posts replies / resolves
73+
# threads) + the engine-scoped token, set up Node, install the engine.
74+
- name: Bot prelude (tokens + Node + engine install)
75+
id: prelude
76+
uses: ./.github/actions/bot-prelude
77+
with:
78+
app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
79+
private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
80+
# Engine pin lives in ONE place: bot-prelude's `engine-ref` default.
81+
# Bump it there to move all four bots in lockstep; override here only
82+
# to run this workflow against a different engine commit.
83+
84+
- name: Run reviewer follow-up
85+
env:
86+
GH_TOKEN: ${{ steps.prelude.outputs.token }}
87+
GITHUB_REPOSITORY: ${{ github.repository }}
88+
PR_NUMBER: ${{ github.event.pull_request.number }}
89+
TRIGGER_COMMENT_ID: ${{ github.event.comment.id }}
90+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
91+
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
92+
MODEL_ENDPOINT: https://${{ secrets.DATABRICKS_HOST }}/serving-endpoints/databricks-claude-opus-4-8/invocations
93+
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
94+
DRY_RUN: 'false'
95+
RUNNER_TEMP: ${{ runner.temp }}
96+
run: python -m databricks_bot_engine.reviewer_bot.followup

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

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# Reviewer Bot — initial PR review.
2+
#
3+
# Own-job workflow. We do NOT use the engine's cross-repo actions/reusable
4+
# workflows (`uses: databricks/databricks-bot-engine/...`): an external repo
5+
# can't resolve the internal engine's Actions artifacts ("not found"). Instead
6+
# the engine is pip-installed via the LOCAL ./.github/actions/bot-prelude ->
7+
# install-bot-engine composites (local `./` refs always resolve). This file owns
8+
# checkout, Setup Python, and the run; bot-prelude mints tokens + Node + installs
9+
# the engine.
10+
#
11+
# Engine pin: the engine commit installed is bot-prelude's `engine-ref` default
12+
# (single source of truth for all four bots); bump it there, never @main.
13+
#
14+
# Auth: PAT-free — bot-prelude mints a short-lived, engine-scoped App token for
15+
# the install (no stored BOT_ENGINE_PAT).
16+
name: Reviewer Bot
17+
18+
on:
19+
pull_request:
20+
types: [opened, synchronize, reopened, ready_for_review]
21+
workflow_dispatch:
22+
inputs:
23+
pr_number:
24+
description: 'PR number to review'
25+
required: true
26+
type: string
27+
dry_run:
28+
description: 'Print what would be posted instead of posting'
29+
required: false
30+
default: 'true'
31+
type: string
32+
33+
permissions:
34+
contents: read
35+
pull-requests: write
36+
id-token: write # JFrog OIDC exchange for the engine/SDK/CLI install
37+
38+
jobs:
39+
review:
40+
# SECURITY FLOOR (was the reusable's job `if:`): manual dispatch, OR a
41+
# non-draft, non-fork PR. fork == false keeps the App + model tokens off
42+
# untrusted code.
43+
if: >-
44+
github.event_name == 'workflow_dispatch'
45+
|| (github.event.pull_request.draft == false
46+
&& github.event.pull_request.head.repo.fork == false)
47+
# Collapse redundant reviews: this triggers on every `synchronize` (push to a
48+
# PR), so rapid pushes would otherwise spawn overlapping runs against the same
49+
# PR. cancel-in-progress: true — a review of a superseded head is wasted work
50+
# (unlike the followups, which use cancel-in-progress: false to avoid dropping
51+
# a reconcile mid-thread). Matches the sibling workflows' concurrency pattern.
52+
concurrency:
53+
group: reviewer-bot-pr-${{ github.event.pull_request.number || inputs.pr_number }}
54+
cancel-in-progress: true
55+
environment: azure-prod # DATABRICKS_HOST / DATABRICKS_TOKEN live here
56+
runs-on:
57+
group: databricks-protected-runner-group
58+
labels: [linux-ubuntu-latest]
59+
timeout-minutes: 30
60+
steps:
61+
# Checkout FIRST. Default ref = the PR merge ref on pull_request, the
62+
# default branch on dispatch. persist-credentials:false — the reviewer
63+
# reads this tree via read_paths/grep, so no token may sit in .git/config.
64+
- name: Checkout
65+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
66+
with:
67+
fetch-depth: 0
68+
persist-credentials: false
69+
70+
# The reviewer reads code, it doesn't run the connector — so it needs only
71+
# a Python interpreter (to run the engine), NOT the connector's deps.
72+
- name: Setup Python
73+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
74+
with:
75+
python-version: '3.11'
76+
77+
# Shared prelude: mint the review-bot token (posts findings) + the
78+
# engine-scoped token, set up Node, install the pinned engine (PAT-free).
79+
- name: Bot prelude (tokens + Node + engine install)
80+
id: prelude
81+
uses: ./.github/actions/bot-prelude
82+
with:
83+
app-id: ${{ secrets.REVIEW_BOT_APP_ID }}
84+
private-key: ${{ secrets.REVIEW_BOT_APP_PRIVATE_KEY }}
85+
# Engine pin lives in ONE place: bot-prelude's `engine-ref` default.
86+
# Bump it there to move all four bots in lockstep; override here only
87+
# to run this workflow against a different engine commit.
88+
89+
- name: Resolve trigger inputs
90+
id: inputs
91+
# SECURITY: dispatcher-supplied values pass via env, never interpolated
92+
# into the script body (an inline expression would inject at assignment,
93+
# before the digits-only check). Env values expand without re-parsing.
94+
env:
95+
GH_TOKEN: ${{ steps.prelude.outputs.token }}
96+
EVENT_NAME: ${{ github.event_name }}
97+
INPUT_PR: ${{ inputs.pr_number }}
98+
INPUT_DRY_RUN: ${{ inputs.dry_run }}
99+
EVENT_PR: ${{ github.event.pull_request.number }}
100+
REPO: ${{ github.repository }}
101+
run: |
102+
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
103+
RAW_PR="$INPUT_PR"; DRY_RUN="$INPUT_DRY_RUN"
104+
else
105+
RAW_PR="$EVENT_PR"; DRY_RUN="false"
106+
fi
107+
[[ "$RAW_PR" =~ ^[0-9]+$ ]] || { echo "::error::Invalid pr_number '$RAW_PR'"; exit 1; }
108+
case "$DRY_RUN" in true|false) ;; *) DRY_RUN="true" ;; esac # fail safe
109+
HEAD_SHA=$(gh pr view "$RAW_PR" --repo "$REPO" --json headRefOid -q .headRefOid)
110+
{ echo "pr_number=$RAW_PR"; echo "head_sha=$HEAD_SHA"; echo "dry_run=$DRY_RUN"; } >> "$GITHUB_OUTPUT"
111+
112+
- name: Run reviewer
113+
env:
114+
GH_TOKEN: ${{ steps.prelude.outputs.token }}
115+
GITHUB_REPOSITORY: ${{ github.repository }}
116+
PR_NUMBER: ${{ steps.inputs.outputs.pr_number }}
117+
HEAD_SHA: ${{ steps.inputs.outputs.head_sha }}
118+
DRY_RUN: ${{ steps.inputs.outputs.dry_run }}
119+
MODEL_ENDPOINT: https://${{ secrets.DATABRICKS_HOST }}/serving-endpoints/databricks-claude-opus-4-8/invocations
120+
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
121+
RUNNER_TEMP: ${{ runner.temp }}
122+
# The reviewer's repo-specific ADDITIVE guidance, read from the TRUSTED
123+
# checkout (run_review enforces containment). Engine owns the base prompt.
124+
REVIEW_SYSTEM_PROMPT: .bot/prompts/review/system.md
125+
run: python -m databricks_bot_engine.reviewer_bot.run_review

0 commit comments

Comments
 (0)