feat(bazel): announce version/command per spawn; unify setup phase + collapse lifecycle to one hook#1167
Merged
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0703f4c070
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
0703f4c to
f4a46a9
Compare
✨ Aspect Workflows Tasks📅 Sun May 31 05:41:07 UTC 2026
|
695cdde to
a07af55
Compare
92548d1 to
a53d925
Compare
553b14c to
1a5ceff
Compare
03180a5 to
60be37c
Compare
…collapse lifecycle to one hook Announce the detected Bazel version and exact command line before each spawn. Rename --announce-rc to --announce-bazel-rc and add --announce-bazel-version / --announce-bazel-command, all "auto"|"true"| "false" defaulting to auto (on under CI, quiet locally). The command and the rc disclosure reuse the existing stream::redaction (env values, header values with the name kept, URL creds) and the rc paths are shortened to ./ under the workspace root, ~/ under $HOME, else absolute. The three flags are declared once via a shared `announce_bazel_args` helper that every Bazel-spawning task splats into its args. Collapse setup_phase + preflight_phase into one always-on setup_phase that every task opens with, passing its result `kind` + `data`: runner-job-history, the Setup phase mark (the task's first task_update — inits the status surfaces AND renders their first body from kind+data), .bazelrc parse + flag resolution (gated on BazelTrait), then the health_check hooks. A health_check hook reporting an unhealthy environment fails the task from inside setup_phase: it concludes the surface with a "Health check failed" body, then fail()s with the hook's detailed reason — fixing a latent bug where an unhealthy Bazel server signaled the agent but let the task proceed. Collapse HealthCheckTrait to a single `health_check` hook (was pre_health_check + post_health_check, the latter unregistered). Collapse TaskLifecycleTrait to a single task_update hook. task_complete had no subscribers; task_started only did one-time surface init. Both are removed: a handler's first task_update initializes, and final=True concludes it. The task subject rides on TaskUpdate.subject; setup_phase sets it on the first update and a later update may refine it (delivery emits its resolved targets once a query resolves), so handlers latch the last non-empty value and refresh the surface title in place. The render-ctx subject field is named `subject` end-to-end; the per-task summary_title helpers share one truncate_for_title; an empty subject renders empty (no placeholder). Drop the GET /rate_limit setup probe: the first status-check API response seeds the same App-bucket quota state via record_from_headers, the first call is never throttle-gated, and the cold-start floor makes zero-vs-one observation identical — so the probe only added a startup round-trip. Fix the timing-breakdown alignment to pad by display width (emoji = 2 cells) and use a single-astral setup emoji (🔧) that renders two cells like the other phase emoji. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
60be37c to
d663f85
Compare
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
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.
Improves pre-task / bazel-spawn diagnostics and unifies the pre-task lifecycle.
1. Announce the Bazel version and command on each spawn
Before each
bazel build/test/runspawn, optionally print the detected Bazel version and the exact command line. The existing--announce-rcis renamed to--announce-bazel-rcso the three form one family, all"auto" | "true" | "false"defaulting toauto(shown on CI, quiet locally):--announce-bazel-rc.bazelrcflags (was--announce-rc)auto--announce-bazel-versionINFO: Bazel <version>auto--announce-bazel-commandINFO: Spawning: <command>autoauto→ on when a CI host is detected, off locally;true/falseforce it.stream::redaction): env-var values, request headers (header name kept, value hidden), and URL credentials become<REDACTED>.--announce-bazel-rcdisplays each rc source path relative to the most meaningful anchor:./under the workspace root,~/under$HOME, else absolute.Bazel development version (version-conditional flags assume latest).announce_bazel_argshelper that each Bazel-spawning task splats into its args.2. Unify the pre-task lifecycle into one
setupphasesetup_phaseandpreflight_phaseare collapsed into a singlesetup_phasethat every task opens with, locally and on CI, passing its resultkind+data: append runner-job-history, open the🔧 Setupphase mark (the task's firsttask_update— it inits the status surfaces and renders their first body fromkind/data), parse the.bazelrc+ resolve flags (when given aBazelTrait), then run thehealth_checkhooks. Bazel work is gated on theBazelTraitargument so a non-Bazel task can use the same phase.HealthCheckTraitis also collapsed to a singlehealth_checkhook (waspre_health_check+post_health_check). A hook reporting an unhealthy environment now fails the task from insidesetup_phase: it concludes the status surface (terminal "Health check failed") then aborts with the hook's detailed reason. Previously an unhealthy Bazel server signaled the runner but let the task proceed — a latent bug this fixes.3. Collapse the lifecycle to a single
task_updatehookTaskLifecycleTraithad three hooks —task_started,task_update,task_complete.task_completehad no subscribers (dead), andtask_startedonly ever did one-time surface init. Both are removed, leaving one hook:task_updateis its cue to initialize (authenticate, create the GitHub check run / first Buildkite annotation). The first update is the🔧 Setupphase mark fromsetup_phase, which also renders the first surface body.final = True; handlers conclude their surface there.TaskUpdate(newsubjectfield, namedsubjectend-to-end).setup_phasesets it on the first update; a later update may refine it (e.g.deliveryemits its resolved targets once a query resolves). Handlers latch the last non-empty value, so the surface title updates in place. An empty subject renders empty (no//...placeholder); the per-tasksummary_titlehelpers share onetruncate_for_titleso a long target list is capped identically.This removes the ordering constraint between the (now gone)
task_startedand the firsttask_update. It also drops the startupGET /rate_limitprobe: the first status-check API response already seeds the same App-bucket quota state, the first call is never throttle-gated, and the cold-start floor makes a zero- vs one-observation buffer identical — so the probe only cost a startup round-trip.4. Timing-breakdown alignment
Pad the phase label column by display width (emoji = 2 cells) instead of codepoint count, and use a single-astral setup emoji (
🔧) that renders two cells like the other phase emoji.Changes are visible to end-users: yes
Suggested release notes
--announce-rcto--announce-bazel-rc; added--announce-bazel-versionand--announce-bazel-command. All three default toauto(shown on CI, quiet locally). The command and rc disclosures redact secrets and shorten rc paths.Test plan
engine::bazelbuild (19) + redaction (26),bazelrc(83, incl. path-anchor edge cases),eval::multi_phase(2),task_info.aspect tests axl; template-snapshot dev tests (bazel-results, lint, format, gazelle, delivery — incl. a long-subject title-capping assertion, bk-annotation — incl. an empty-subject scenario);test-bazel-flags(incl.announce_bazel_args),test-health-check-ready(incl.run_health_checksfirst-error-wins),test-rate-limit,test-github-detect.aspect <task>drivessetup_phase→ firsttask_update→ terminal;🔧 Setuprow aligns with🔨 Build;--announce-bazel-version=trueprints the version line; an injected unhealthyhealth_checkhook makes setup_phase render a "Health check failed" terminal and fail the task with the detailed reason.