reviewer-bot: workflow_dispatch reads default-branch code for explore tools (set REVIEW_CONTENT_ROOT)
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 84/100
Research direction
Start in reviewer-bot.yml and inspect the workflow_dispatch path alongside review_bot/run_review.py's _content_root() behavior. Verify how HEAD_SHA is obtained and how the separate checkout and REVIEW_CONTENT_ROOT should be wired without changing the trusted primary checkout. Done means manual dispatch explores the PR head while pull_request behavior remains unchanged.
Written by the indexing model from the issue text.
Description
Follow-up from PR #862 review (thread on reviewer-bot.yml).
Problem
On the workflow_dispatch (manual) trigger, reviewer-bot.yml does a single actions/checkout with no ref:, which resolves to the default branch — not the PR being reviewed. The PR head is resolved separately (HEAD_SHA = gh pr view ... --json headRefOid).
Result:
- The review diff and comment anchors are correct — the engine fetches them from the GitHub compare API keyed on
HEAD_SHA(repos/{repo}/compare/...), independent of the checkout. - But the agent's explore tools (
read_paths/grep) read from the engine's_content_root(), which falls back to the primary checkout (= default branch on dispatch) whenREVIEW_CONTENT_ROOTis unset. So on manual dispatch, the agent explores default-branch code, not the PR head.
The automatic pull_request trigger (the normal case) is unaffected — its checkout already is the PR merge ref.
Why the engine expects this
Per reviewer_bot/run_review.py _content_root(), workflow_dispatch is meant to check the PR head into a separate directory and export its path as REVIEW_CONTENT_ROOT, while engine code keeps running from the trusted default-branch checkout. This is a deliberate security boundary: dispatch is exempt from the fork guard, so the PR's own code must not run/read from the primary checkout dir.
Fix
On the dispatch path, add a second checkout of HEAD_SHA into a separate dir and export REVIEW_CONTENT_ROOT pointing at it (mirroring the engine's documented pattern).
Severity
Low — only manual dispatch is affected, and only the explore-tool reads (diff/anchors are always correct). Deferred from #862 as a follow-up.
- Dominant language
- Python
- Stars
- 236
- Forks
- 162
- Avg merge
- 13h 13m
- Merged PRs (30d)
- 23
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from databricks/databricks-sql-python
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
databricks/databricks-sql-python#1034 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
databricks/databricks-sql-python#1012 ·
Maintainers usually reply within 1 day
-
Token federation: `_exchange_token` raises KeyError('access_token') on error responses, discarding the real failure reasonPossibly taken @TangoEnSkai claimed this 81 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
databricks/databricks-sql-python#904 ·
Maintainers usually reply within 1 day
-
engineer-bot
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
databricks/databricks-sql-python#860 · 3 comments ·
Maintainers usually reply within 1 day
-
Cursor.close() leaks server-side handle for async commands that were never fetchedPossibly taken @nanookclaw claimed this 176 days ago. Openbug engineer-bot
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
databricks/databricks-sql-python#791 · 5 comments ·
Maintainers usually reply within 1 day
All issues in databricks/databricks-sql-python
Similar issues
-
fuzz fuzz-unconfirmed
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 1 day
-
Switch to ruffOpen
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 67/100
Maintainers usually reply within 1 day
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Qiskit/qiskit-aer#2533 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 64/100
etesync/etesync-dav#376 ·