ci: align adbc bots — reviewer auto-runs, engineer-bot --system-prompt + .github access - #605
Conversation
…r consumers) adbc was the only consumer whose reviewer required a `review-bot` label to run; databricks-sql-python/go/nodejs/kernel all auto-review every non-draft, non-fork PR. Align adbc by dropping the `contains(labels, 'review-bot')` clause from both reviewer-bot.yml and reviewer-bot-followup.yml, so the reviewer runs on any non-draft, non-fork PR like the rest of the fleet. The fork/draft security floor is unchanged (secrets still never reach untrusted code). Also drop the now-unneeded `labeled` pull_request trigger from reviewer-bot.yml (it existed only to react to the label being added). Co-authored-by: Isaac
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Medium
Looks good overall — the core change (dropping the review-bot label gate + labeled trigger to match the other consumer repos) is sound, and the security floor (fork == false, plus draft == false on the initial review) is preserved. One medium concern: the change orphans a still-live --add-label review-bot step and a now-stale comment in the untouched sibling engineer-bot.yaml, and the PR description's claim that engineer-bot doesn't add that label is incorrect.
eric-wang-1990
added
the
engineer-bot
…ed arg) The engine's followup phase now REQUIRES --system-prompt (engine #89 removed the .bot/ prompt-name fallback); adbc's engineer-bot-followup invoked it without the flag, so every run failed with "engineer-bot: error: the following arguments are required: --system-prompt". Point at the engineer-followup system prompt that already exists at .bot/prompts/engineer-followup/system.md, mirroring how databricks-jdbc wires it. Co-authored-by: Isaac
eric-wang-1990
changed the title
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean CI fixes to two draft bot workflows. Verified the --system-prompt path (.bot/prompts/engineer-followup/system.md) exists and matches .bot/config.yaml, and that dropping the review-bot label gate preserves the fork == false security floor and the auto-review-on-opened path for engineer-bot fix PRs. One low note about a comment overstating gate parity in the follow-up workflow.
…beling Two follow-ons to this PR's reviewer-auto-run change: 1. Remove `.github` from .bot/config.yaml denied_subpaths (maintainer decision): the engineer-bot may need to fix its own CI/workflow wiring (e.g. this PR's missing --system-prompt), which the deny-list previously blocked (the bot reported "denied in this workspace"). `.bot` stays denied; the human PR-review gate is the backstop for any .github diffs (they carry secrets/tokens). 2. Remove the now-dead "Label the fix PR for review" step in engineer-bot.yaml. It added `review-bot` so the reviewer would pick up fix PRs -- but the reviewer now auto-runs on every non-draft/non-fork PR (this PR's reviewer-bot.yml change), so the label-add is vestigial and its comment stale. publish still applies `engineer-bot` for the follow-up opt-in. Co-authored-by: Isaac
There was a problem hiding this comment.
Verdict: 1 Medium
Looks good overall — the --system-prompt fix points at a real file (.bot/prompts/engineer-followup/system.md exists), and the reviewer auto-run / label-removal changes across the four workflow files are internally consistent. One medium concern: the diff also drops .github from the engineer-bot's sandbox denylist (a defense-in-depth weakening) which the PR description doesn't mention.
eric-wang-1990
changed the title
….yml) Review nit: the comment claimed the follow-up gate "matches reviewer-bot.yml", but it intentionally omits `draft == false` (the follow-up must keep responding to existing threads even after a PR is drafted). Reword to state the label gate is dropped like reviewer-bot.yml, but the draft clause is deliberately absent so the follow-up runs on any open PR (draft or not). No behavior change. Co-authored-by: Isaac
1 participant
Summary
Four related fixes aligning adbc's bot workflows with the other consumer repos and the engine's current contract.
1. Reviewer bot auto-reviews (drop the
review-botlabel gate)adbc was the only consumer whose reviewer required an opt-in
review-botlabel; databricks-sql-python/go/nodejs/kernel auto-review every non-draft, non-fork PR. Dropped thecontains(labels, 'review-bot')clause from the jobif:in bothreviewer-bot.ymlandreviewer-bot-followup.yml(and removed the now-unneededlabeledtrigger). Fork/draft security floor unchanged.2. engineer-bot follow-up: pass the required
--system-promptThe engine's followup phase now requires
--system-prompt(engine #89 removed the.bot/prompt-name fallback). adbc invoked... --phase followup --bot .botwithout it, so every run failed:Added
--system-prompt .bot/prompts/engineer-followup/system.md(the prompt file already exists), mirroring databricks-jdbc's onboarding.3. Allow the engineer-bot to edit
.github/Removed
.githubfrom.bot/config.yamldenied_subpaths(maintainer decision) so the bug-fix bot can fix its own CI/workflow wiring (e.g. #2 above) instead of reporting "denied in this workspace" and blocking..botstays denied; the human PR-review gate is the backstop — reviewers must scrutinize any.githubdiffs since workflows carry secrets/tokens.4. Remove the vestigial review-bot labeling step
engineer-bot.yamlhad a "Label the fix PR for review" step that addedreview-botso the reviewer would pick up fix PRs. With #1 (reviewer auto-runs), that step is dead and its comment stale — removed.publishstill appliesengineer-botfor the follow-up opt-in.Notes
main—pull_request/comment-triggered bot jobs run from the base-branch copy of the workflow, so the fixes are dormant on this PR itself (expected).engineer-bot.yamllabel step vestigial — which it couldn't fix because.github/was denied. This PR both enables that access and does the cleanup.Verification
.bot/config.yamlparse; reviewer/followup jobs intact..githubno longer indenied_subpaths; no--add-label review-botstep remains; engineer-followup invocation includes--system-promptpointing at the existing prompt file.This pull request and its description were written by Isaac.