Skip to content

feat: add buildkite annotations and status checks for remaining built-in tasks (lint, delivery, format, gazelle)#1016

Draft
gregmagolan wants to merge 8 commits intomainfrom
multi_task_status_checks
Draft

feat: add buildkite annotations and status checks for remaining built-in tasks (lint, delivery, format, gazelle)#1016
gregmagolan wants to merge 8 commits intomainfrom
multi_task_status_checks

Conversation

@gregmagolan
Copy link
Copy Markdown
Member

@gregmagolan gregmagolan commented Apr 23, 2026 •
edited
Loading

Summary

Extends GithubStatusChecks and BuildkiteAnnotations to dispatch on task_update.kind, so the lint, delivery, format, and gazelle tasks each render their own check-run summary instead of falling through to the bazel-build template. Each result kind ships an init_results + render_check_output pair loaded from a registry — new task kinds plug in by adding one entry.

Adds GitHub check-run annotations for lint findings (Marvin v1 parity), gated by a new LintTrait knob.

What's new

Per-task renderers

  • lib/lint_results.axl — by-severity + by-tool tables, trim cascade for >65KB bodies
  • lib/delivery_results.axl — per-target ok/warn/fail/skip rollup
  • lib/format_results.axl — files-need-formatting list + repro command
  • lib/gazelle_results.axl — out-of-date BUILD files + repro command

Lint check-run annotations

  • LintTrait.findings_destination = "comments" | "annotations" | "both" (default "comments"). Suggested fixes always go to comments — the annotations API has no patch primitive.
  • LintTrait.only_annotate_changed_regions = True (Marvin v1 default). Annotations filter to lines in changed_files so they appear in the Files Changed tab; auto-skipped when changed_files is empty (push to main, manual run).
  • Findings split by SARIF _fixes presence: fix-bearing → review comments with <details> suggestion blocks; non-fix → check-run annotations.
  • Multi-batch send: GitHub caps at 50 annotations per request, so _send_extra_annotation_batches PATCHes additional batches with title+summary preserved.

Buildkite parity

  • feature/buildkite_annotations.axl mirrors the dispatch table and maps each kind to a severity → annotation style (success/info/warning/error).

Public renames in bazel_results.axl

  • Drops the leading _ from helpers the new libraries reuse: build_summary_data, format_run_date, format_int_comma, merged_meta, compute_invocation_state, vcs_info, STATUS_ICONS, RENDER_META_STANDARD_KEYS, etc. No behaviour change.

Test plan

  • aspect dev test-template-snapshots — bazel_results, 12 scenarios
  • aspect dev test-lint-template-snapshots — 8 render scenarios + 7 annotation pipeline scenarios (severity map, cap=50, multi-batch chunking, changed-region filter, message/title contract, read-only invariant)
  • aspect dev test-delivery-template-snapshots — 10 scenarios
  • aspect dev test-format-template-snapshots — 12 scenarios
  • aspect dev test-gazelle-template-snapshots — 11 scenarios
  • aspect dev test-bk-annotation-snapshots — 16 scenarios across all kinds + severity styles
  • Live exercise: ASPECT_CLI_TESTING=github_status_checks aspect lint //... against a PR with mixed fix-bearing and non-fix findings to verify check-run annotations render in the Files Changed tab.

@gregmagolan gregmagolan force-pushed the multi_task_status_checks branch 15 times, most recently from 61fb57f to aa0c95f Compare April 29, 2026 11:32
@gregmagolan gregmagolan force-pushed the multi_task_status_checks branch from aa0c95f to 3905989 Compare May 5, 2026 06:29
@aspect-workflows
Copy link
Copy Markdown

aspect-workflows Bot commented May 5, 2026 •
edited
Loading

Aspect CI Status · 2026-05-08T03:33:02Z

Task Job Result
✅ passed build build-task 162 targets built
✅ passed build build-task 162 targets built
✅ passed format buildifier-task done
✅ passed format buildifier-task done
✅ passed delivery delivery-task done
✅ passed format format-task done
✅ passed format format-task done
🟡 running gazelle gazelle-from-source-task in progress
✅ passed gazelle gazelle-task done
✅ passed gazelle gazelle-task done
✅ passed lint lint-task done
✅ passed lint lint-task done
✅ passed test test-task all 25 tests cached
✅ passed test test-task all 25 tests cached

@gregmagolan gregmagolan force-pushed the multi_task_status_checks branch from 3905989 to 40aac17 Compare May 5, 2026 14:16
@gregmagolan gregmagolan changed the title multi task status checks feat: per-task status checks with lint annotations May 5, 2026
@gregmagolan gregmagolan force-pushed the multi_task_status_checks branch 4 times, most recently from 57f716d to 1d77782 Compare May 7, 2026 03:40
@gregmagolan gregmagolan force-pushed the multi_task_status_checks branch from 1d77782 to 7bc08fd Compare May 8, 2026 00:47
@gregmagolan gregmagolan changed the title feat: per-task status checks with lint annotations feat: add buildkite annotations and status checks for remaining built-in tasks (lint, delivery, format, gazelle) May 8, 2026
@gregmagolan
feat: review-pass polish for multi-task status checks
2c335ae 
Pulls the remaining build-task surfaces into lint / format / gazelle /
delivery and tightens the header.

- Drop redundant `ctx.features[GithubStatusChecks].enabled = True` and
  `ctx.features[BuildkiteAnnotations].enabled = True` from the dogfood
  config — both default to True and gate on the relevant CI env at
  runtime, so opt-in here was a no-op.
- Add Aspect CLI version (`💎 aspect-cli vX.Y.Z`) as the last item on
  row 1 of every check-run / annotation summary so the rendering
  surfaces a reproducibility marker without expanding the runner block.
- Share Targets · Build Metrics · Invocation · Aspect Workflows Runner
  · Workspace Status · Build Metadata · Explicit command line ·
  Options parsed across all task kinds. The bazel `DETAILS_TEMPLATE`
  is now `{aborted-prelude} + SHARED_DETAILS_BODY_TEMPLATE`; lint /
  format / gazelle / delivery append the shared body to their own
  task-specific top section, merge `build_details_data(...)` into
  their details data, and start their `init_results()` from
  `bazel_results.init_results()` so the full bazel state is keyed.
  Each task's BES loop now drives `process_event` instead of the
  light `capture_metadata_event` (removed; no remaining callers).
  Lint's local `aborted` field renamed to `lint_no_diagnostics_failure`
  to avoid colliding with bazel's BUILD_ABORTED key in the merged dict.
- Hoist the lint task to the repo root: shellcheck `http_archive`s move
  to root MODULE.bazel, the nested examples/lint MODULE.bazel /
  MODULE.bazel.lock / .bazelversion / TESTING.md / .aspect/ are
  removed, examples/lint is dropped from .bazelignore, the BUILD file
  switches from `@@+http_archive+...` canonical labels to apparent
  `@shellcheck_*` names, and linters.bzl uses `//examples/lint:...`
  absolute labels. CI yamls (BK / GHA / GitLab / CircleCI) drop their
  `cd examples/lint` / `working-directory: examples/lint` and target
  `--aspect=//examples/lint:linters.bzl%shellcheck -- //examples/lint/...
  -//examples/lint/exclude/...` from the repo root.

All six template snapshot suites pass.
@gregmagolan gregmagolan force-pushed the multi_task_status_checks branch from 25632f6 to 2c335ae Compare May 8, 2026 01:51
gregmagolan added 2 commits May 7, 2026 18:56
@gregmagolan
ci: add dedicated buildifier task in parallel with format
7b11df6 
Models the legacy Rosetta `buildifier` task using `aspect format` with a
Starlark-only `format_multirun` target. See
docs/cli/migration/buildifier.mdx for the recipe this follows.

- tools/format/BUILD: declare a `format-starlark` `format_multirun` (only
  `starlark = "@buildifier_prebuilt//:buildifier"`) alongside the existing
  multi-language `format` target. Also wires `starlark` into the existing
  `format` target so local `bazel run //tools/format` covers Starlark in
  one shot.
- .buildkite, .github, .gitlab, .circleci: add a `buildifier-task` step
  that runs `aspect format --formatter-target=//tools/format:format-starlark`
  with its own task-key, and update the existing `format-task` to ignore
  Starlark patterns (`**/*.bzl`, `**/BUILD`, `**/BUILD.bazel`,
  `**/MODULE.bazel`, `**/WORKSPACE`, `**/WORKSPACE.bazel`) so the two
  steps don't do duplicate work.

Each CI surface gets debug + non-debug variants matching the existing
`format-task` cadence.
@gregmagolan
fix: surface Aspect Workflows link as soon as the build is spawned
64e3664 
The runtime mints `sink_invocation_id` inside `Build::spawn` (build.rs)
before returning, so it is available the moment `ctx.bazel.build(...)`
returns. Tasks were assigning it to `results["sink_invocation_id"]`
*after* `build.wait()` though, so the link only appeared on
`task_complete` — leaving the in-progress BK annotation / status check
link-less for the entire build window.

Move the assignment to immediately after build spawn in lint, format,
gazelle, delivery (phase 1), and the shared bazel_runner used by build /
test, and emit one running task_update at that point so the BK annotation
re-renders with the link populated. The initial `task_started`
annotation is still link-less for the JVM-startup window (~1-2 sec)
since it fires before the build is spawned, but the link "pops in" as
soon as the build starts rather than when it finishes.

Drop `BuildkiteAnnotations.min_update_interval_ms` default from 2000ms
to 500ms — buildkite-agent annotate has no documented rate limit and
the previous value was over-conservative; 500ms keeps non-final updates
feeling live without flooding very-chatty tasks. Terminal task_updates
always pass through unconditionally.
gregmagolan added 4 commits May 7, 2026 19:52
@gregmagolan
fix(delivery): pass targets as task_started subject
74d5031 
`_state["_subject"]` flows into `render_ctx["targets"]`, which
`_assemble_explicit_command` uses to append target tokens after the
bazel flags. Delivery was firing `task_started` with `""`, so the
"Explicit command line" detail block rendered as `bazel build <flags>`
with no targets after the trailing `\\\n`. Use the same space-joined
form lint/build/test pass.
@gregmagolan
fix: stream metadata + preserve Aspect Workflows link across build_st…
989071b 
…arted

Two related bugs were keeping the running BK annotation / GH status
check empty until terminal task_complete:

- `process_event`'s `build_started` reset wiped the entire results dict,
  including `sink_invocation_id` — the field the task sets right after
  `ctx.bazel.build(...)` returns. The next BES event is `build_started`,
  which arrives milliseconds after the task's emit, so the Aspect
  Workflows link disappeared from every annotation for the rest of the
  build. Preserve `sink_invocation_id` across the reset; it is task-
  managed and not part of the BES-derived state being recycled.
- `process_event` returned `False` for `build_started`, `build_metadata`,
  `workspace_status`, `options_parsed`, and `configuration` on the
  premise that "metadata is context, not a status change". But the
  invocation header (row 1 user/host, row 2 repo/PR/branch/commit,
  row 3 cpu/mode) and the Invocation / Workspace Status / Build
  Metadata / Explicit command line / Options parsed detail blocks are
  populated entirely from these events. Until the first
  `target_completed` (which can be 5+ seconds in), the running emit had
  no metadata. Return `True` for these so live emits pick up the
  metadata burst as soon as it arrives.

Also wire the `if process_event: emit running task_update` pattern
(which `bazel_runner` has had for build/test) into the BES loops in
lint, format, gazelle, and delivery (phase 1). Without it, those tasks
relied on a single emit at build-spawn (no metadata yet) and the
terminal task_complete; the BK annotation / status check showed
`Running...` with no detail until completion.

The BK / GHSC throttles handle the higher emit rate.
@gregmagolan
fix(gazelle): subscribe to BES stream before emitting running task_up…
9fea294 
…date

The broadcaster behind `build.build_events()` only delivers events to
subscribers that joined BEFORE the event was sent
(see crates/axl-runtime/src/engine/bazel/stream/broadcaster.rs). With my
recent change adding a running task_update emit between
`ctx.bazel.build(...)` returning and the BES for-loop, the emit handler
(BK annotate spawn, status-check render) takes a few dozen ms — and on
a warm bazel daemon (second `aspect gazelle` invocation in a CI step,
fully cached) BES events stream within milliseconds. The new
subscriber registered AFTER `build_started` / `target_completed` /
`named_set_of_files` had already been broadcast, missing the
target_completed for the gazelle target. `runnable.determine_entrypoint`
then returned None and the task failed:

    Failed to determine the gazelle target's entrypoint.

The first (debug) invocation in the same step worked because the
daemon was cold — JVM startup + analysis phase gave the late
subscriber time to register.

Fix: hoist the `build.build_events()` call to BEFORE the running emit.
Subscribing immediately after `ctx.bazel.build(...)` returns
guarantees the iterator's channel buffers events from t=0, regardless
of how slow the emit handler is. Apply the same pattern to format,
lint, delivery, and the build/test runner — every site that calls
`build.build_events()` after a potentially-slow lifecycle hook.
@gregmagolan
fix(runtime): make BES sink failures non-fatal + surface real panic m…
8465b78 
…essage

Two fixes for the gazelle-from-source-task error seen on GHA but not BK:

    error: one of the sinks failed: Any { .. }
       --> .../aspect/gazelle.axl:289:12  (`build.wait()`)

The "Any { .. }" was a Box<dyn Any> from a sink-thread panic, formatted
via `{:#?}` which doesn't downcast — so we couldn't see what actually
failed. The user said the same task passes on BK; the most likely
explanation is a transient gRPC issue on the GHA runner's network path
to the BES backend that the BK runner doesn't share.

1. `stream_sink.rs`: replace the two `.expect("failed to {join,wait}")`
   calls in `GrpcEventStreamSink::spawn` with a match that catches both
   the inner SinkError (gRPC connect, send-channel disconnected, lifecycle
   publish failure) and the outer tokio JoinError. Log them as warnings
   to stderr (and to axl.log under ASPECT_DEBUG=1) and return cleanly.
   BES forwarding is a side-channel — Bazel's exit code already tells the
   user whether their build succeeded; a flaky gRPC connection to the
   results backend shouldn't take the whole task down.

2. `build.rs:Build::wait`: if a sink thread *does* panic at the Rust
   level (now only possible from a logic error inside the closure, not
   from a sink-error path), downcast the panic payload to surface the
   actual message. Previously `{:#?}` rendered any panic as "Any { .. }".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gregmagolan