fix(setup): report BUILD_USER alongside the commit and CI metadata - #1487
joshcbarnes wants to merge 1 commit into
Conversation
`aspect setup workspace-data` replaces Bazel's own default workspace status script, which is the sole source of BUILD_USER. Pointing --workspace_status_command at it therefore blanked out the "User" row in the Web UI's Bazel details block, which reads BUILD_USER from the workspace_status event with no fallback. Collect it in `get_build_metadata`, so it reaches both consumers — the workspace status lines and the `--build_metadata` flags — from the one dict they share. It stays distinct from USER, which continues to prefer the commit author or CI actor: BUILD_USER is where the build physically ran. Resolution is USER -> LOGNAME -> USERNAME, from the environment only. This runs at the start of every Bazel invocation and Bazel fails the build outright when the status command exits non-zero, so a process spawn to cover the last edge case is the wrong trade; where nothing names the account the key is dropped by the existing empty-value filter. BUILD_HOST is lost the same way and is not addressed here — there is no reliable environment variable for the hostname, so it needs the spawn this deliberately avoids. Adds tests/workspace_data.rs, the first coverage for this task. The AXL entry point takes a `std` handle and has no fake, so running the real binary is the only thing that exercises the path. The helper pins ASPECT_CREDENTIALS_FILE because naming a CI host turns on the Deployment feature's `auto` endpoint wiring, which reaches the real keyring and fails under Bazel's sandbox. 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📅 Mon Sep 21 17:51:02 UTC 2026 Task Results
Reproduce❌ delivery (delivery-uncacheable · delivery-gha-debug · delivery-gha)Install ⏱ Last updated Mon Sep 21 17:59:32 UTC 2026 · 📊 GitHub API quota 2,281/7,700 (30% used, resets in 4m) |
|
Closing this — the premise is wrong, and the change would have been a regression. I claimed Tested on Bazel 9.0.1, with a status script emitting only All five built-ins survive. There was never a gap to fill — the Web UI's "User" row was being populated by Bazel all along. Worse, emitting the key doesn't add a duplicate, it overrides the built-in, and Bazel routes it to That breaks the invariant So the change traded Bazel's authoritative OS-user value for one guessed from For anyone who lands here later: Bazel always supplies |
1 participant
Problem
aspect setup workspace-datareplaces Bazel's own default workspace status script, and that default is the sole source ofBUILD_USER,BUILD_HOSTandBUILD_TIMESTAMP. So pointing--workspace_status_commandat this task blanked out the User row in the Web UI's Bazel details block, which readsBUILD_USERstraight off theworkspace_statusevent with no fallback (bazel_results.axl:3484).Change
Collect
BUILD_USERinget_build_metadata, so it reaches both consumers — the workspace status lines and the--build_metadataflags — from the one dict they share.It stays distinct from
USER, which continues to prefer the commit author or CI actor. That distinction is the point, and it's the onebazel_results.axl:3481already documents:Resolution is
USER→LOGNAME→USERNAME(Windows), from the environment only. No process spawn: this runs at the start of every Bazel invocation, and Bazel fails the build outright when the status command exits non-zero, so shelling out toid/whoamito cover the last edge case would buy a marginal gain for a new way to take down every build in the workspace. Where nothing names the account, the key is dropped by the existing empty-value filter rather than emitted bare.Not addressed
BUILD_HOSTis lost the same way, so the Host name row next to User is still blank. It needs a different mechanism — there's no reliable env var for the hostname (HOSTNAMEis bash-specific,HOSTzsh) — so it would require exactly the spawn this avoids. Worth deciding separately.Tests
Adds
crates/aspect-cli/tests/workspace_data.rsand itsrust_testtarget — the first coverage for this task. Five cases: each fallback, theUSER/BUILD_USERdistinction, and the drop-the-key case.The AXL entry point takes a
stdhandle and has no fake, so.aspect/axl.axlcan only reach the file's pure helpers; running the real binary is the only thing that exercises this path.Per CLAUDE.md, confirmed the tests fail with the change reverted — 4 of 5 fail, the fifth being the negative case that correctly passes either way. Green under both
cargo testandbazel test, along withtests axl(991 cases), the template snapshots, and the rest of theaspect-cliRust suite.One wrinkle worth flagging for reviewers: the test helper pins
ASPECT_CREDENTIALS_FILEto a temp path. Naming a CI host in the environment turns on CI detection, and with it the Deployment feature'sautoendpoint wiring, which callsctx.aspect.auth.list()and reaches the real keyring — a hard failure under Bazel's sandbox (A default keychain could not be found). Same isolationauth_status.rsdocuments, and pre-existing behavior rather than anything this change introduces.🤖 Generated with Claude Code