fix(bes): stream a feature-injected BES backend from every task - #1455
Open
joshcbarnes wants to merge 1 commit into
Open
joshcbarnes wants to merge 1 commit into
joshcbarnes wants to merge 1 commit into
Conversation
`--remote=bes` reached `build` and `test` and silently streamed nowhere for `format`, `gazelle`, `lint`, `run`, `warming`, and `delivery`. The `Deployment` feature puts its backend on `BazelTrait.bes_backends`, but `collect_bes_sinks` took those URIs as an `extra_backends` parameter rather than reading the trait it is already handed. Only `bazel/invocation.axl` — the spawn path behind `build`/`test` — passed them. The six tasks that build their own sinks call it with no `extra_backends`, so the default `[]` dropped the feature's backend on the floor. No error, no disclosure: those builds simply reported to nothing. The cache half never had the problem, which is what made it confusing: `--remote=cache` lands in `BazelTrait.base_flags`, read generically by the spawn path, so it reaches every task. Only the BES half needed each caller to cooperate. This completes what #1374 set out to do — it introduced the seam and said `--remote` "now reach[es] every bazel-driving task rather than just build/test" — and what #1326 wired a day later in `invocation.axl` alone. `collect_bes_sinks` and `dropped_bes_backends` now read `bazel_trait.bes_backends` themselves, so a feature-injected backend reaches a task by construction rather than by each call site remembering. `extra_backends` stays for a caller with a backend of its own and is unioned with the trait's, so a caller that still passes them (as `invocation.axl` did) gets one sink, not two; that now-redundant threading is dropped. Endpoint precedence is unchanged: a backend Bazel uploads to itself via `--bes_backend` is still dropped from the CLI's sinks and still named in the "Not streaming …" disclosure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Aspect Workflows Tasks📅 Tue Sep 15 23:15:15 UTC 2026 Task Results
Reproduce❌ delivery (delivery-uncacheable · delivery-gha-debug · delivery-gha · delivery-uncacheable-warn)Install ⏱ Last updated Tue Sep 15 23:21:09 UTC 2026 · 📊 GitHub API quota 259/7,700 (3% used, resets in 55m) |
jbedard
approved these changes
Sep 16, 2026
jbedard
left a comment
jbedard
left a comment
Member
There was a problem hiding this comment.
silently streamed nowhere for format, gazelle, lint, run, warming, and delivery.
Can those be tested somehow?
4 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
--remote=besreachedbuildandtest, and silently streamed nowhere forformat,gazelle,lint,run,warming, anddelivery.The bug
The
Deploymentfeature puts its backend onBazelTrait.bes_backends, butcollect_bes_sinkstook those URIs as anextra_backendsparameter rather than reading the trait it is already handed. Onlybazel/invocation.axl— the spawn path behindbuild/test— passed them:The six tasks that build their own sinks call it with no
extra_backends, so the default[]dropped the feature's backend on the floor. No error and no disclosure — those builds simply reported to nothing.The cache half never had the problem, which is what made it confusing to diagnose:
--remote=cachelands inBazelTrait.base_flags, which the spawn path reads generically, so it reaches every task. Only the BES half needed each caller to cooperate.Observed on a GitHub-hosted CI job:
formatgot--remote_cache=grpcs://cache.aspect.buildand the credential helper, but no BES stream — so the invocation never appeared in the Web UI, whilebuildandtestfrom the same run did.History
This completes what #1374 set out to do. It introduced the seam and stated the goal:
It added
BazelTrait.bes_backendswithout a consumer. #1326, a day later, wired it up — ininvocation.axlonly, touchingbuild.axl,test.axl,trait.axlandbazel.axl. The six tasks with their own sink construction were never revisited.The fix
collect_bes_sinksanddropped_bes_backendsreadbazel_trait.bes_backendsthemselves, so a feature-injected backend reaches a task by construction rather than by each call site remembering. That's deliberately the opposite of threading the parameter through six more call sites: a parameter that must be passed identically everywhere is what produced the bug.extra_backendsstays for a caller with a backend of its own, unioned with the trait's and deduped, so a caller that still passes them gets one sink rather than two. The now-redundant threading ininvocation.axlis dropped.Endpoint precedence is unchanged: a backend Bazel uploads to itself via
--bes_backendis still dropped from the CLI's sinks and still named in the "Not streaming …" disclosure.Test plan
_test_feature_injected_backend_reaches_every_taskinbuild_events_test.axl: a backend on the trait is streamed with noextra_backendsfrom the caller; it is still dropped when Bazel uploads there itself and still named bydropped_bes_backends; a caller passing the same backend is deduped._fake_traitgrows abes_backendsfield.fail: streamed with no extra_backends from the caller— rather than passing either way.--remote=cache,bes:aspect formatstreams togrpcs://bes.aspect.build0 times before the fix and 1 time after.aspect dev test-bes-sinks(19),test-bazel-invocation(11 sections),test-deployment-flags(19),test-bazel-flagsall pass;aspect formatclean.Notes for reviewers
build_events.axlchange is two hunks inbes_results_url_flag, a different function, so these merge cleanly. The two are complementary: feat(bazelrc): config groups, deployment sections, and a flag catalog with opt-outs #1442's--config=aspect-cloudroutes BES through Bazel's own--bes_backendand bypasses the CLI-sink path, whileaspect <task> --remotestill goes through it and hits this bug.--remote=bes(or--remotebare):format,gazelle,lint,run,warming, anddeliverynow produce Web UI invocations where they previously produced none. In this repo's CI that takes a pass from 2 hosted invocations to roughly 14.Changes are visible to end-users: yes
aspect <task> --remotenow streams build events from every Bazel-driving task.format,gazelle,lint,run,warming, anddeliverypreviously wired the remote cache but silently streamed no build events, so their invocations never reached the Aspect Web UI.🤖 Generated with Claude Code