Skip to content

ci: Fix offline Maven setup for Engineer Bot follow-ups #1753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vuanhphung merged 9 commits into from
Oct 29, 2026
Merged

ci: Fix offline Maven setup for Engineer Bot follow-ups #1753

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6ac6e07
ci: fix offline Maven setup for engineer follow-ups
vuanhphung Oct 29, 2026
c9e799d
ci: decouple offline Maven checks from PR test results
vuanhphung Oct 29, 2026
dfa73fa
ci: keep follow-ups running when Maven cache checks fail
vuanhphung Oct 29, 2026
950e120
ci: warm JUnit launcher before switching Maven offline
vuanhphung Oct 29, 2026
40b0507
ci: clarify Maven warnings and avoid recursive artifact fetches
vuanhphung Oct 29, 2026
4c6ca30
ci: budget setup and agent time for engineer follow-ups
vuanhphung Oct 29, 2026
52f1080
Merge branch 'main' into fix/engineer-bot-followup-offline-maven
vuanhphung Oct 29, 2026
c45ea1e
ci: simplify offline Maven follow-up setup
vuanhphung Oct 29, 2026
38b43ce
ci: reuse author workflow Surefire fallback
vuanhphung Oct 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 49 additions & 8 deletions .github/workflows/engineer-bot-followup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
# Like the author, this sets up Java 21 + Maven (via setup-maven → JFrog mirror)
# so the agent can run the mocked unit suite (`mvn -pl jdbc-core test`) to verify
# a fixup. Unlike the author it wires NO live-warehouse env — the follow-up prompt
# runs only unit tests and forbids the live e2e suite. Same spotless-churn revert
# + credential-scrub as the author (see comments there).
# runs only unit tests and forbids the live e2e suite. The smoke test runs after
# Maven credentials are scrubbed; Spotless churn is reverted before the agent runs.
#
# The gate below is the canonical label-gated form from the engine dogfood. It
# ANDs with the engine-side floor; do not weaken it.
Expand Down Expand Up @@ -76,7 +76,8 @@ jobs:
runs-on:
group: databricks-protected-runner-group
labels: linux-ubuntu-latest
timeout-minutes: 30
# Leave setup headroom before the agent's separate 30-minute limit.
timeout-minutes: 45
concurrency:
group: engineer-bot-followup-pr-${{ github.event.pull_request.number }}
cancel-in-progress: false
Expand Down Expand Up @@ -106,11 +107,37 @@ jobs:
- name: Build driver (skip tests; warms the Maven repo + compiles jdbc-core)
run: mvn -pl jdbc-core -am -B clean install -DskipTests -Ddependency-check.skip=true

# mvn install ran spotless:apply (compile-phase bound); revert any churn to
# pre-existing files so the followup's fixup commit carries only the agent's
# edits. The agent re-formats its own edits via `mvn spotless:apply`.
- name: Revert spotless churn (keep only the agent's edits)
run: git checkout -- .
# Resolve test tooling with credentials; run PR tests only after scrubbing.
- name: Warm test dependencies for the offline agent
timeout-minutes: 5
continue-on-error: true
run: |
Comment thread
vuanhphung marked this conversation as resolved.
Resolved
set -euo pipefail
SUREFIRE_VERSION=$(mvn -pl jdbc-core -B help:evaluate \

@peco-review-bot peco-review-bot Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low — The step runs under set -euo pipefail. If help:evaluate fails (for example, the help plugin can't be resolved or a transient mirror error occurs), set -e aborts the step at this assignment. Both dependency:get calls are then skipped, including the JUnit launcher warm-up, which doesn't depend on the Surefire version. Because of continue-on-error, you only get a generic step-failure annotation instead of your ::warning:: lines. The cache then reaches the offline agent without the provider or launcher, and the agent hits the same "surefire-junit-platform (absent)" error this PR is meant to fix.

The author workflow avoids this with a fallback version. Suggest the same here: SUREFIRE_VERSION=$(mvn ... -q -DforceStdout 2>/dev/null | tail -n1) || true followed by [ -n "$SUREFIRE_VERSION" ] || SUREFIRE_VERSION=3.1.2. Likewise, when JUNIT_PLATFORM_VERSION comes back empty, emit an explicit warning rather than running dependency:get with a malformed junit-platform-launcher: coordinate.

-Dexpression=maven-surefire-plugin.version -q -DforceStdout) || SUREFIRE_VERSION=""
# Use the same pinned fallback as engineer-bot.yml.
SUREFIRE_VERSION=${SUREFIRE_VERSION:-3.1.2}
# Surefire aligns its launcher with the project's resolved Platform engine.
JUNIT_PLATFORM_VERSION=$(mvn -pl jdbc-core -B dependency:list \
-DincludeGroupIds=org.junit.platform -DincludeArtifactIds=junit-platform-engine \
| sed -n 's/.*org\.junit\.platform:junit-platform-engine:jar:\([^: ]*\):.*/\1/p') || JUNIT_PLATFORM_VERSION=""
mvn -N -B dependency:get \
-Dartifact="org.apache.maven.surefire:surefire-junit-platform:${SUREFIRE_VERSION}" \
|| echo "::warning::Surefire provider warm-up failed; offline Maven tests may fail."
if [[ -n "$JUNIT_PLATFORM_VERSION" ]]; then
mvn -N -B dependency:get \
-Dartifact="org.junit.platform:junit-platform-launcher:${JUNIT_PLATFORM_VERSION}" \
|| echo "::warning::JUnit Platform launcher warm-up failed; offline Maven tests may fail."
else
echo "::warning::JUnit Platform version lookup failed; launcher warm-up skipped."
fi

# Empty offline settings use central, not the JFrog mirror's repository ID.
- name: Normalize _remote.repositories for offline resolution
run: |
set -euo pipefail
find ~/.m2/repository -name '_remote.repositories' \
-exec sed -i 's/jfrog-central/central/g' {} +

# --- shared bot setup ----------------------------------------------------
- name: Bot prelude (tokens + Node + engine install)
Expand All @@ -130,11 +157,25 @@ jobs:
printf '<settings />\n' > ~/.m2/settings.xml
echo "MAVEN_ARGS=-o" >> "$GITHUB_ENV"

- name: Check unit tests work offline

@peco-review-bot peco-review-bot Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low — The offline smoke test is the only check that the warmed ~/.m2 actually works offline, and its result is thrown away. It has both continue-on-error: true and || echo "::warning::...", so it can never fail the step. Because it runs after the credential scrub, it also can't download anything that's still missing, unlike the author workflow, which runs the same test while credentials are live.

So if the warm-up misses an artifact Surefire loads lazily (your fixture already saw warnings about missing provider transitives), the job still starts the agent. The agent then spends up to the 30-minute step limit hitting the same offline cache-miss failure as #1742, and the only sign of trouble is a yellow annotation in the run log.

Two options: give the step an id and skip or short-circuit the follow-up step when its outcome is not success, or at least note in a comment that this is purely diagnostic. Keeping warm-up failures non-blocking is reasonable. A failed offline smoke test, though, is a reliable sign that the agent's mvn test will fail too.

timeout-minutes: 2
continue-on-error: true

@peco-review-bot peco-review-bot Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — The step is called "Warm test runtime offline", but it runs after the scrub with MAVEN_ARGS=-o, so it can't fetch anything. It can only report whether the cache is already complete. The sibling author workflow runs this same test while credentials are live, because Surefire resolves some artifacts lazily at execution time. This PR handles the two lazy artifacts it already knows about (the provider and the launcher). Any other execution-time artifact still can't be fetched here; it only shows up as a warning, and the agent then hits the same offline dead-end this PR is fixing.

If the reason for running PR tests only after the scrub is to keep PR test code away from the JFrog token, note that the earlier Build driver step already runs PR-controlled build code (plugins, spotless) with that token live. Either:

  • run the smoke test once before the scrub to actually warm the cache, as engineer-bot.yml does, or
  • rename this step (e.g. "Verify test runtime offline") and state in the comment that it only detects missing artifacts and doesn't warm anything.

run: |

@peco-review-bot peco-review-bot Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low — The offline smoke test is capped at timeout-minutes: 1, but it runs a full mvn test lifecycle on a cold JVM: spotless:apply over the whole module (bound to compile per the old comment), incremental compile and test-compile, and a forked Surefire JVM. That can easily take close to a minute. When it overruns, Actions kills the step. You get the generic timeout annotation, not the custom ::warning:: explaining the cache miss, and the Surefire/launcher runtime classpath warm-up this step is meant to prove never gets validated. Because of continue-on-error, the job continues silently either way. The budget has room for more: the comment says 26 min of setup, and the steps add up to exactly 26. Consider giving this step 3–4 minutes, taken from the 6-minute build or warm-up caps, so the smoke test actually gives a signal.

mvn -o -pl jdbc-core -B test \
-Dtest="DatabricksParameterMetaDataTest#testInitialization" \
-Ddependency-check.skip=true \
|| echo "::warning::The offline smoke test failed; see the Maven output for the cause."

# The build and smoke test apply Spotless; restore files before agent edits.
- name: Revert spotless churn (keep only the agent's edits)
run: git checkout -- .

# Run the follow-up. NOTE: no live-warehouse connection env
# (DATABRICKS_HOST / _HTTP_PATH / _TEST_CONFIG_FILE) is wired here — the
# followup prompt runs only the mocked unit suite and forbids live e2e, so
# the agent never needs them.
- name: Run engineer follow-up (catch-up over all unaddressed threads)
timeout-minutes: 30
env:
GH_TOKEN: ${{ steps.prelude.outputs.token }}
GITHUB_REPOSITORY: ${{ github.repository }}
Expand Down
Loading