Skip to content

fix: self-heal //external query on bzlmod-only workspaces#420

Merged
tinder-maxwellelliott merged 1 commit into
masterfrom
claude/serve-bzlmod-migration-error-b37dc5
Jul 9, 2026
Merged

fix: self-heal //external query on bzlmod-only workspaces#420
tinder-maxwellelliott merged 1 commit into
masterfrom
claude/serve-bzlmod-migration-error-b37dc5

Conversation

@tinder-maxwellelliott

Copy link
Copy Markdown
Collaborator

Problem

generate-hashes and the serve warmup could hard-fail on a Bzlmod-only workspace with:

ERROR: no such package 'external': //external package is not available since the WORKSPACE file is deprecated, please migrate to Bzlmod.
[Warn] warmup for revision 'main' failed; ... Bazel query failed, exit code 7, allowed exit codes: 0

BazelClient.queryAllTargets decides whether to query //external:all-targets from BazelModService.isBzlmodEnabled, whose only signal is "bazel mod graph exited 0". In restricted/cloud deploy environments that probe can false-negative (it can't resolve the module graph / lockfile), so //external:all-targets gets included and Bazel rejects it because the package is unavailable in bzlmod-only mode (e.g. Bazel 8/9 with --enable_workspace=false).

For serve this was worse than the warmup warning implies — a cold on-demand request hits the same crash, so the service was effectively broken for such workspaces.

Fix

React to ground truth instead of the probe:

  • BazelQueryService.runQuery now tees + captures Bazel's stderr (switching stderr from Redirect.PRINT to Redirect.Consume) and, when a query fails specifically because //external is unavailable (no such package 'external' / //external package is not available), throws a typed ExternalPackageUnavailableException instead of an opaque "exit code 7".
  • BazelClient catches it, drops //external:all-targets, and retries once. It latches a @Volatile externalPackageUnavailable flag so later revisions served by a long-running serve process skip //external up front rather than failing-then-retrying every time.

This cannot misfire for genuine WORKSPACE mode: there the query succeeds, so no retry happens and //external targets are still hashed as before. It also fixes generate-hashes, which shares the same query path.

Testing

  • BazelClientTest — added 3 cases: non-cquery retry-without-external, cquery external-repo drop, and the subsequent-call latch. All pass; existing cases unchanged.
  • BazelModServiceTest, HashServiceTest, ImpactedTargetsServiceTest, ServeCommandTest — pass.
  • E2ETest — the 4 local failures (testUseCqueryWith*, testBzlmodTransitiveDepsCquery, testFineGrainedHashExternalRepo) are pre-existing and environmental; confirmed identical on baseline with this change stashed (Unable to run coursier: Unable to locate a Java Runtime — the local Bazel sandbox has no JDK for rules_jvm_external).
  • ktfmt applied.

Notes for reviewers

  • Separate, lower-severity residual not addressed here: the same probe false-negative also nulls getModuleGraphJson(), disabling MODULE.bazel-change detection in that environment. Could be a follow-up (e.g. strengthening isBzlmodEnabled), but it's independent of this crash fix.
  • The stderr switch to Redirect.Consume still forwards Bazel's stderr live (line-buffered), so query progress/errors remain visible.

🤖 Generated with Claude Code

`generate-hashes` and the `serve` warmup could hard-fail with:

    ERROR: no such package 'external': //external package is not available
    since the WORKSPACE file is deprecated, please migrate to Bzlmod.
    (Bazel query failed, exit code 7)

`BazelClient.queryAllTargets` decides whether to query `//external:all-targets`
from `BazelModService.isBzlmodEnabled`, which is only "`bazel mod graph` exited
0". In restricted/cloud deploys that probe can false-negative (it can't resolve
the module graph), so `//external:all-targets` is included and Bazel rejects it
because the package is unavailable in bzlmod-only mode. For `serve` this broke
both warmup and cold on-demand requests for such workspaces.

Fix reacts to ground truth instead of the probe:

- `BazelQueryService.runQuery` tees + captures stderr (switching `stderr` from
  `Redirect.PRINT` to `Redirect.Consume`) and, when a query fails because
  `//external` is unavailable, throws a typed `ExternalPackageUnavailableException`
  instead of an opaque "exit code 7".
- `BazelClient` catches it, drops `//external:all-targets`, and retries once. It
  latches a `@Volatile externalPackageUnavailable` flag so later revisions served
  by a long-running `serve` skip `//external` up front instead of failing and
  retrying every time.

This cannot misfire for genuine WORKSPACE mode: there the query succeeds, so no
retry happens and `//external` targets are still hashed.

Adds BazelClientTest cases for the non-cquery retry, the cquery external-repo
drop, and the subsequent-call latch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tinder-maxwellelliott tinder-maxwellelliott marked this pull request as ready for review July 9, 2026 19:36
@tinder-maxwellelliott tinder-maxwellelliott merged commit c9d0b88 into master Jul 9, 2026
28 of 29 checks passed
@tinder-maxwellelliott tinder-maxwellelliott deleted the claude/serve-bzlmod-migration-error-b37dc5 branch July 9, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant