Skip to content

Commit 84ab9b1

Browse files
ci(bots): add daily retrospective (rolling learning PR) + bump engine (#882)
* ci(bots): add daily retrospective (rolling learning PR) + bump engine Adds the daily learning-extraction flow (was not enabled on this repo) on the current engine d05dcb1, which maintains ONE rolling learning PR on a stable branch (ai/learning-pr). - engineer-bot-learning.yml: daily cron + workflow_dispatch; PAT-free via the existing bot-prelude; actions:read for Track B (author-run mining). - .bot/config.yaml: add retrospective: block (engine-owned batch prompt, no override) and author.knowledge_log to close the loop (author reads what the retrospective writes). - Seed .claude/knowledge/learning-log.md. - Bump bot-prelude engine-ref default 5368de7 → d05dcb1. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3653811351 at .github/workflows/engineer-bot-learning.yml:78 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654123624 at .github/workflows/engineer-bot-learning.yml:75 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: post Python Integration Tests check as the driver-test app (fix app-pinned gate) The required 'Python Integration Tests' check is pinned in the main ruleset to the driver-test app (driver-integration-test, integration_id 2731531). But the PR-side placeholder was posted via github.token, so it landed as github-actions — a DIFFERENT check context that does NOT satisfy the app-pinned gate. Result: the required check sat unfulfilled and PRs stayed BLOCKED even though a green 'Python Integration Tests' appeared in the list (e.g. #882). Mirror databricks-sql-go #426 / databricks-sql-nodejs #459: - Add skip-checks-reporter.yml, triggered by workflow_run on 'Trigger Integration Tests'. It runs in the base-repo context with secret access even for fork-triggered runs, mints the INTEGRATION_TEST_APP token, and posts 'Python Integration Tests'=success on every PR head — forks included — as the driver-test app, so the app-pinned gate is satisfied and every PR auto-enqueues. - Remove the inline skip-integration-tests-pr github.token stub (superseded; it posted under the wrong app and 403'd on forks entirely). - In merge-queue-python, mint the app token unconditionally and post the auto-pass (no-driver-change) and dispatch-failure checks with IT — the merge_group path isn't covered by the reporter, and a github.token check there likewise can't satisfy the pinned gate (queue would hang). The real result was already posted back by driver-test as the app; this aligns the no-op paths. The labeled-PR preview's inline checks stay on github.token (cosmetic; the reporter owns the pinned gate on pull_request events), matching go/nodejs. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654256027 at .github/workflows/skip-checks-reporter.yml:39 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: post PR-open skip check inline for internal PRs (fix waiting-for-status hang) Follow-up to the previous commit. Removing the inline PR-open poster in favour of the workflow_run reporter created a bootstrap gap: workflow_run workflows only run from the DEFAULT-branch copy, so on this PR (and any introducing PR) the reporter can't fire and NOTHING posts 'Python Integration Tests' on the PR head — the required check sits 'Expected — Waiting for status to be reported' forever. Fix: split the placeholder by PR origin, both posting as the driver-test app (the identity the ruleset pins the required check to): - Internal PRs: restore skip-integration-tests-pr in trigger-integration-tests.yml, now minting the INTEGRATION_TEST_APP token (not github.token) and self-guarded to head repo == base repo. Posts from the PR branch immediately — no label, no workflow_run dependency. This is what unblocks internal PRs like this one. - Fork PRs: skip-checks-reporter.yml (workflow_run) now self-guards to fork runs (head repo != base repo), since a fork's pull_request run can't mint secrets inline. The two guards are mutually exclusive — no double-post. Behaviour matches the intent: skipped placeholder on PR open, real run in the merge queue (and label preview). The merge-queue app-token posts from the prior commit are unchanged. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654339217 at .github/workflows/engineer-bot-learning.yml:34 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: clarify merge-queue mint-failure is fail-closed (review r3654295072) Soften the overstated 'the mint always succeeds here' comment and document the mint-failure case explicitly: it is uncloseable in-workflow (only the pinned app can post to the pinned context; a github.token fallback lands on a different context and neither satisfies nor fails the gate), but it is fail-CLOSED — the merge queue's check_response_timeout evicts an entry whose required check never reports, and the mint failure shows as a red run. No code change; comment only. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654417922 at .github/workflows/trigger-integration-tests.yml:366 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: refresh check state (re-post required checks on a clean head) No file changes. The prior head accumulated many check-suites across repeated force-push rebases, and GitHub's mergeability rollup stopped surfacing the required 'Kernel E2E' context even though a valid github-actions success check-run existed on the commit — leaving the PR unable to enter the merge queue ('Kernel E2E — Expected — Waiting for status to be reported'). Pushing a fresh non-force commit gives a clean check-suite set so skip-kernel-e2e-pr re-posts Kernel E2E into the current suite and the rollup picks it up. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> --------- Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent a94cfe2 commit 84ab9b1

6 files changed

Lines changed: 364 additions & 54 deletions

File tree

.bot/config.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ author:
8686
issue_url: ISSUE_URL
8787
context_files:
8888
- issue_body.txt # {{issue_body}}
89+
# Close the loop: the author phase READS this log (newest-first, capped) into its
90+
# system prompt as accumulated lessons, so retrospective learnings actually steer
91+
# future fixes. MUST match retrospective.log_path below (write half ↔ read half).
92+
knowledge_log: .claude/knowledge/learning-log.md
8993

9094
# Engine orchestration for the author phase. `bug-fix` runs the plan →
9195
# author_tests → fix pipeline (write a failing test → fix the code → re-run to
@@ -94,3 +98,20 @@ author:
9498
# testing specifics (commands, layout, fixtures) live in prompts/engineer/
9599
# system.md, not here.
96100
flow: bug-fix
101+
102+
# Daily-cron learning extraction (retrospective flow). Run by engineer-bot-learning.yml
103+
# via `python -m databricks_bot_engine.engineer_bot.retrospective`: over an adaptive
104+
# look-back window the engine ITSELF gathers merged PRs (diff + review comments) AND
105+
# recent engineer-bot author-run logs, and if the model finds durable learnings opens
106+
# ONE rolling PR appending a dated section to log_path. Human-gated — never commits the
107+
# canonical log directly. Omitting this block makes the retrospective a no-op.
108+
#
109+
# No `system_prompt` override: the engine ships the authoritative batch-aware base
110+
# (engineer_prompts.RETRO_SYSTEM_PROMPT); a `.bot/prompts/` override is optional.
111+
# No `context_files` — the daily-cron engine enumerates its own sources via the
112+
# GitHub API (context_files is an AUTHOR-phase key only).
113+
retrospective:
114+
log_path: .claude/knowledge/learning-log.md # MUST match author.knowledge_log above
115+
branch_prefix: ai/learning-pr- # rolling PR lands on the derived stable branch `ai/learning-pr`
116+
pr_label: engineer-bot-learning
117+
author_workflow: engineer-bot.yml # Track B: which workflow's author runs to mine

.claude/knowledge/learning-log.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Engineer-bot learning log
2+
3+
Durable, reusable engineering learnings distilled by the daily retrospective
4+
(`engineer-bot-learning.yml``databricks_bot_engine.engineer_bot.retrospective`).
5+
Each daily run appends one dated section of 0..N learnings to a single rolling PR
6+
until a human merges it. The engineer author phase reads this log (see
7+
`.bot/config.yaml` `author.knowledge_log`) so past lessons steer future fixes.
8+
9+
## Entries
10+
11+
--- *Add new entries above this line (oldest→newest); newest sections sort to the bottom.* ---

.github/actions/bot-prelude/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ inputs:
3131
# value to move every bot to a new engine commit; never @main.
3232
description: 'Engine commit SHA (full 40-char) to install.'
3333
required: false
34-
default: '5368de773858bef9b2a8bbf3099d08a4132a8487'
34+
default: 'd05dcb113332401b4aee8d6aa05c7107399ad44f'
3535
engine-repo:
3636
description: 'owner/name of the engine repo.'
3737
required: false
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# Engineer Bot — learning (retrospective) extraction — DAILY CRON.
2+
#
3+
# Over an adaptive look-back window the engine gathers merged PRs (diff + review
4+
# comments) AND recent engineer-bot author-run console logs ITSELF via the GitHub
5+
# API — no in-workflow context gathering, no per-PR trigger — and if the model
6+
# finds durable, reusable learnings, opens ONE ROLLING PR on a stable branch
7+
# (`ai/learning-pr`), appending a dated section per day until a human merges it.
8+
# Human-gated by design: it NEVER commits the canonical log directly.
9+
#
10+
# Own job (NOT `uses: databricks/databricks-bot-engine/...`): an external repo
11+
# can't resolve the internal engine's reusable workflows ("not found"). It shares
12+
# the SAME prelude the other bots use — ./.github/actions/bot-prelude (tokens +
13+
# Node + pinned engine install) — so the engine pin stays single-sourced in
14+
# bot-prelude's `engine-ref` default (no second SHA to drift).
15+
#
16+
# Opt-in is purely via the `retrospective:` block in .bot/config.yaml + this
17+
# workflow; absent that block the engine phase is a clean no-op.
18+
name: Engineer Bot — Learning
19+
20+
on:
21+
schedule:
22+
# 17:23 UTC daily — off-peak, off-:00 minute (GitHub delays/drops on-the-hour crons).
23+
- cron: "23 17 * * *"
24+
workflow_dispatch:
25+
inputs:
26+
since:
27+
description: 'ISO lower bound to shorten the window and recover a wedged flow. Empty = adaptive cursor.'
28+
type: string
29+
default: ''
30+
window-hours:
31+
# STRING, not number: a `type: number` workflow_dispatch input fails the
32+
# whole run at startup ("workflow file issue") when combined with the
33+
# `schedule` trigger. argparse coerces it to int downstream.
34+
description: 'Fallback look-back window (hours) used only when there is no prior successful run.'
35+
type: string
36+
default: '24'
37+
38+
permissions:
39+
contents: write # push the learning branch / open the learning PR
40+
pull-requests: write
41+
actions: read # Track B lists engineer-bot author runs + logs via the App token;
42+
# the engineer-bot App installation must ALSO carry actions:read
43+
# (a missing scope surfaces as a 403 that fails the whole run —
44+
# list_author_runs raises, no escape hatch).
45+
id-token: write # JFrog OIDC exchange for the engine/SDK/CLI install
46+
47+
concurrency:
48+
# One learning run at a time; a queued run waits rather than racing the rolling
49+
# PR's branch. Not keyed on a PR number (this is a cron, no PR event).
50+
group: engineer-bot-learning-cron
51+
cancel-in-progress: false
52+
53+
jobs:
54+
learning:
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: 20
60+
steps:
61+
# Checkout the default branch (the learning PR is cut from it) FIRST, so the
62+
# local `./` composites below resolve. persist-credentials:false — the
63+
# retrospective sets its own authenticated push remote (see the run step),
64+
# so no token is left in .git/config.
65+
- name: Checkout default branch (learning PR is cut from it)
66+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
67+
with:
68+
fetch-depth: 0
69+
persist-credentials: false
70+
71+
# No JFrog setup step here: install-bot-engine (via bot-prelude) does its own
72+
# keyless OIDC→JFrog mint and passes the credential through job-local files +
73+
# per-command --index-url flags. It deliberately never reads PIP_INDEX_URL /
74+
# JFROG_ACCESS_TOKEN from the environment, so a setup-jfrog step would be both
75+
# redundant AND a credential leak (setup-jfrog exports a token-bearing
76+
# PIP_INDEX_URL to $GITHUB_ENV, exposing it to every later step — including the
77+
# one that runs the model). Mirrors the read-only sibling reviewer-bot.yml.
78+
- name: Setup Python
79+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
80+
with:
81+
python-version: '3.11'
82+
83+
# Shared prelude: mint the engineer-bot token (opens the learning PR) + the
84+
# engine-scoped token, set up Node, install the pinned engine (PAT-free). The
85+
# engine pin comes from bot-prelude's `engine-ref` default — the SINGLE source
86+
# of truth for every bot; there is no second SHA in this file to drift.
87+
- name: Bot prelude (tokens + Node + engine install)
88+
id: prelude
89+
uses: ./.github/actions/bot-prelude
90+
with:
91+
app-id: ${{ secrets.ENGINEER_BOT_APP_ID }}
92+
private-key: ${{ secrets.ENGINEER_BOT_APP_PRIVATE_KEY }}
93+
94+
# NOTE: no git-identity step — the engine's retrospective configures the git
95+
# user AND DCO sign-off itself from .bot/config.yaml `bot_login_prefix`.
96+
# NOTE: no context-gather step — the daily-cron engine enumerates merged PRs
97+
# + author runs itself over the adaptive window.
98+
- name: Extract learnings + open rolling PR
99+
env:
100+
GH_TOKEN: ${{ steps.prelude.outputs.token }}
101+
GITHUB_REPOSITORY: ${{ github.repository }}
102+
# Only the `<workspace>/serving-endpoints/` prefix matters:
103+
# sdk_agent.translate_endpoint rewrites this to `.../serving-endpoints/anthropic`
104+
# and discards the model path segment. The effective model comes from
105+
# .bot/config.yaml `retrospective.model` (or the engine default).
106+
MODEL_ENDPOINT: https://${{ secrets.DATABRICKS_HOST }}/serving-endpoints/anthropic/invocations
107+
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
108+
RUNNER_TEMP: ${{ runner.temp }}
109+
SINCE: ${{ inputs.since }}
110+
WINDOW_HOURS: ${{ inputs.window-hours }}
111+
# The retrospective pushes the learning branch with a plain `git push
112+
# origin`, and the checkout ran persist-credentials:false — so set an
113+
# authenticated push remote from the minted App token first, mirroring
114+
# engineer-bot.yml's publish step. --since / --window-hours are passed only
115+
# when provided via workflow_dispatch (the schedule trigger leaves them
116+
# empty → the adaptive cursor drives the window).
117+
run: |
118+
git remote set-url origin "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git"
119+
args=(--repo-dir "$GITHUB_WORKSPACE")
120+
[ -n "$SINCE" ] && args+=(--since "$SINCE")
121+
[ -n "$WINDOW_HOURS" ] && args+=(--window-hours "$WINDOW_HOURS")
122+
python -m databricks_bot_engine.engineer_bot.retrospective "${args[@]}"
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
name: Report Integration Test Skip
2+
3+
# Posts the PR-open "skipped" placeholder for the required
4+
# `Python Integration Tests` check on FORK PRs, as the driver-test GitHub App.
5+
#
6+
# Division of labour:
7+
# - INTERNAL PRs: the placeholder is posted inline by trigger-integration-tests.yml's
8+
# `skip-integration-tests-pr` job (it can mint the app token directly). Posting it
9+
# there means it works from the PR branch immediately, without waiting for this
10+
# workflow_run file to reach the default branch.
11+
# - FORK PRs: a fork's `pull_request` run has a READ-ONLY GITHUB_TOKEN and no secrets,
12+
# so it cannot mint the app token or post any check on its own head. This
13+
# workflow_run workflow instead runs in THIS (base) repo's context with full secret
14+
# access even for fork-triggered runs, so it can post the app-attributed placeholder
15+
# on a fork PR's head. This job self-guards to fork runs to avoid double-posting on
16+
# internal PRs (which the inline job already covers).
17+
#
18+
# Why the app (not github.token): the ruleset pins the required
19+
# `Python Integration Tests` check to the driver-test app's integration id. Only a
20+
# check posted BY that app satisfies the gate — a github.token (github-actions) check
21+
# of the same name is a different context and does NOT.
22+
#
23+
# SECURITY: this workflow runs with secrets in a privileged context. It MUST NOT check
24+
# out or execute any PR/fork-controlled content. It only calls checks.create with a
25+
# static body; the sole fork-controlled input is `workflow_run.head_sha`, an opaque
26+
# commit SHA passed to the API. Do not add `actions/checkout` or a `run:` step that
27+
# executes repo content here.
28+
#
29+
# The real integration suite is unaffected: it runs as the required gate on the
30+
# `merge_group` commit (and as a label preview on internal PRs), dispatched by
31+
# trigger-integration-tests.yml. Mirrors databricks-sql-go / databricks-sql-nodejs.
32+
33+
on:
34+
workflow_run:
35+
workflows: ["Trigger Integration Tests"]
36+
types: [requested]
37+
38+
jobs:
39+
report-skip:
40+
# Fork PR-triggered runs only. Internal PRs are posted inline by
41+
# trigger-integration-tests.yml; the merge_group run posts the real required check.
42+
if: >-
43+
github.event.workflow_run.event == 'pull_request' &&
44+
github.event.workflow_run.head_repository.full_name != github.event.workflow_run.repository.full_name
45+
runs-on:
46+
group: databricks-protected-runner-group
47+
labels: linux-ubuntu-latest
48+
permissions:
49+
checks: write
50+
contents: read
51+
pull-requests: read
52+
steps:
53+
# This job is fork-only (see the job `if:`), so the placeholder is always
54+
# appropriate EXCEPT when the fork PR is already closed. A fork PR's label
55+
# preview cannot dispatch the real suite (no secret access), so nothing
56+
# else posts the required check for it — keep the placeholder even when
57+
# labeled. Skip only closed PRs. Resolve the PR by SHA because a fork's
58+
# workflow_run payload has an empty pull_requests array. Read-only lookup
59+
# (no checkout / no execution of PR content) — does not weaken the SECURITY
60+
# note above.
61+
- name: Decide whether to post the placeholder
62+
id: gate
63+
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0
64+
env:
65+
HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
66+
with:
67+
script: |
68+
let prs = context.payload.workflow_run.pull_requests || [];
69+
let number = prs.length ? prs[0].number : null;
70+
if (number === null) {
71+
// Fork PRs: workflow_run.pull_requests is empty. Resolve by SHA.
72+
const { data } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
73+
owner: context.repo.owner,
74+
repo: context.repo.repo,
75+
commit_sha: process.env.HEAD_SHA,
76+
});
77+
number = data.length ? data[0].number : null;
78+
}
79+
if (number === null) {
80+
// No PR resolvable (unexpected) — default to posting so the
81+
// required check isn't left unfulfilled.
82+
core.setOutput('post', 'true');
83+
return;
84+
}
85+
const { data: pr } = await github.rest.pulls.get({
86+
owner: context.repo.owner,
87+
repo: context.repo.repo,
88+
pull_number: number,
89+
});
90+
const isClosed = pr.state === 'closed';
91+
console.log(`PR #${number} closed=${isClosed} -> post=${!isClosed}`);
92+
core.setOutput('post', (!isClosed).toString());
93+
94+
- name: Generate GitHub App token (this repo)
95+
id: app-token
96+
if: steps.gate.outputs.post == 'true'
97+
uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0
98+
with:
99+
app-id: ${{ secrets.INTEGRATION_TEST_APP_ID }}
100+
private-key: ${{ secrets.INTEGRATION_TEST_PRIVATE_KEY }}
101+
owner: databricks
102+
repositories: databricks-sql-python
103+
104+
- name: Post skipped Python Integration Tests check
105+
if: steps.gate.outputs.post == 'true'
106+
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0
107+
env:
108+
HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
109+
with:
110+
github-token: ${{ steps.app-token.outputs.token }}
111+
script: |
112+
await github.rest.checks.create({
113+
owner: context.repo.owner,
114+
repo: context.repo.repo,
115+
name: 'Python Integration Tests',
116+
head_sha: process.env.HEAD_SHA,
117+
status: 'completed',
118+
conclusion: 'success',
119+
completed_at: new Date().toISOString(),
120+
output: {
121+
title: 'Skipped on PR — runs in merge queue',
122+
summary: 'Python Integration Tests are skipped on ordinary PR events and run as the required gate in the merge queue (dispatched to databricks-driver-test). Add the `integration-test` label to preview them on this PR. (Label previews cannot run on fork PRs, which lack secret access; fork PRs are exercised by the required merge-queue run.)',
123+
},
124+
});

0 commit comments

Comments
 (0)