Repository navigation
feat: use BRAINTRUST_ prefix for tracing env vars - #165
Abhijeet Prasad (AbhiPrasad) wants to merge 2 commits into
Conversation
Rename the user-facing bt-daemon env vars from BT_* to BRAINTRUST_*, matching bt (braintrustdata/bt#416) and the Braintrust SDKs. Fixes SDK-245. - bt-daemon/src/env.rs reads BRAINTRUST_<NAME> and silently falls back to the deprecated BT_<NAME>. The canonical name wins when both are set. The old names can be dropped in a future major release. - bt trace run sets both socket/data-dir names on its children so plugins released before the rename still find the isolated daemon. - The JS daemon client, Cursor hook launchers, and the OpenCode bt tool read the new names first and fall back to the old ones. - BT_TRACE_MANAGED_RUN_ID / BT_TRACE_INVOCATION_SETTINGS stay unchanged as internal protocol between bt trace run and its child processes. - A guard test fails on any new BT_* name in bt-daemon/src. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f4021740e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| if let Some(runtime) = &isolated_runtime { | ||
| command | ||
| .env(paths::SOCKET_ENV, &runtime.socket) | ||
| .env(paths::DATA_DIR_ENV, runtime.temp_dir.path()); | ||
| env::set_with_alias(&mut command, paths::SOCKET_ENV, &runtime.socket); | ||
| env::set_with_alias(&mut command, paths::DATA_DIR_ENV, runtime.temp_dir.path()); |
There was a problem hiding this comment.
When a non-default BRAINTRUST_DAEMON_SOCKET is supplied, isolate_daemon is false, so this block never mirrors the resolved socket into BT_DAEMON_SOCKET. An already-installed Pi or OpenCode plugin from before the rename reads only the legacy variable and therefore connects to the default or stale socket, while the managed runner flushes the explicitly configured socket, silently losing the run's trace. Mirror an inherited explicit socket into both names on the child just as this block does for an isolated runtime.
AGENTS.md reference: AGENTS.md:L53-L58
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, fixed in 7432fa0. bt trace run now passes every resolved BRAINTRUST_* value to the child under both names (env::mirror_aliases), so an explicit BRAINTRUST_DAEMON_SOCKET reaches pre-rename plugins as BT_DAEMON_SOCKET too, and a stale BT_DAEMON_SOCKET is overwritten. The isolated-daemon override still applies on top. New test standalone_run_mirrors_an_explicit_socket_to_the_deprecated_name covers both the missing and stale cases; it failed before the fix (child saw an empty BT_DAEMON_SOCKET).
With an explicit BRAINTRUST_DAEMON_SOCKET, bt trace run does not create an isolated daemon, so the child only inherited the new name. Pi and OpenCode plugins released before the rename read BT_DAEMON_SOCKET and would connect to the default (or a stale) socket instead. Pass every resolved value to the child under both names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
1 participant
Summary
Fixes SDK-245. This is the coding-agent counterpart to braintrustdata/bt#416.
bt traceenv vars come frombt-daemon, and they all used theBT_*prefix. Everything else (btafter #416, and all the Braintrust SDKs) usesBRAINTRUST_*. This PR moves the user-facing tracing env vars toBRAINTRUST_*.BT_DAEMON_SOCKET,BT_DAEMON_DATA_DIR,BT_DAEMON_CONFIG,BT_ANTIGRAVITY_CONFIG_DIR,BT_CURSOR_PLUGIN_DIR,BT_CURSOR_CONFIG_DIR,BT_TRACE_PI_PLUGIN_SPEC,BT_TRACE_OPENCODE_PLUGIN_SPEC. For example,BT_DAEMON_SOCKETbecomesBRAINTRUST_DAEMON_SOCKET.bt-daemon/src/env.rsreads theBRAINTRUST_*name and falls back to theBT_*name. If both are set,BRAINTRUST_*wins. To drop the old names in a future major release, deleteDEPRECATED_BT_ENV_VARS.bt trace runpasses every resolved value to the child process under both the new and old names. That covers its own isolated daemon, an explicitBRAINTRUST_DAEMON_SOCKET, and a stale old value. The pi/opencode plugins already installed from npm only readBT_DAEMON_SOCKET, and this keeps them talking to the right daemon. The JS daemon client now readsBRAINTRUST_DAEMON_SOCKETfirst and falls back toBT_DAEMON_SOCKET, so new plugins also work with an olderbt.BT_BIN) and the OpenCodebttool (BT_EXECUTABLE) now both readBRAINTRUST_BT_BIN, and fall back to their old names. This is the one change that isn't prefix-only. Both old names meant "path tobt", so they now share one new name.bt trace runsetsBT_TRACE_MANAGED_RUN_ID,BT_TRACE_INVOCATION_SETTINGS, and_BT_TRACE_MANAGED_RUNfor its child process tree, and separately released plugins read them. They keep their names, the same exception bt#416 made for its runner protocol vars.env_vars_use_braintrust_prefixscansbt-daemon/src. It fails on anyBT_*name that is neither a deprecated alias nor internal protocol. It also fails if a deprecated alias maps to a name nothing reads.AGENTS.mdnow states the prefix rule.Not in this PR:
BT_AGENT_*,BT_CONSOLE_PROBE_*,BT_DAEMON_BINin test wrappers) keep their names.btneeds a bt-daemon SHA bump after this merges. Its tests setBT_DAEMON_CONFIGand will keep passing through the alias; they can switch to the new name in the same bump.Testing
cargo fmt --checkandcargo clippy --all-features --all-targets -D warningsare clean.cargo test --all-features: 491 passed. I checked that the guard test fails when a newBT_const is added, and when the deprecated list has a bogus entry.dispatch::tests::terminal_grok_hydration_waits_for_a_stable_transcript_boundaryfailed in 2 early full runs. It then passed in 6 more full runs on this branch and 6 onmain. This PR doesn't touch that file.standalone_run_mirrors_an_explicit_socket_to_the_deprecated_namerunsbt-daemon run piwith an explicitBRAINTRUST_DAEMON_SOCKET, once withBT_DAEMON_SOCKETmissing and once stale. It checks that the child sees the explicit socket under both names. It failed before the fix (the child saw an emptyBT_DAEMON_SOCKET).standalone_hook_honors_deprecated_bt_env_namesruns the realbt-daemonbinary. It checks that the old name alone works with empty stderr, and that the new name wins when both are set.node --test, Node 24): 10/10 pass. The new socket-name tests fail onmain(8 pass, 2 fail).make testpasses.trace.shlauncher with recording shims. It picksBRAINTRUST_BT_BIN, falls back toBT_BIN, and prefers the new name when both are set.Before and after with the real binary, running
bt-daemon hook --source codexagainst a disabled-tracing config:mainBRAINTRUST_DAEMON_CONFIG=ok.json~/.codex/braintrust.jsonok.jsonBT_DAEMON_CONFIG=ok.jsonok.jsonok.jsonBT_DAEMON_CONFIG=ok.json BRAINTRUST_DAEMON_CONFIG=missing.jsonok.jsonmissing.json(BRAINTRUST_*wins)Running
bt-daemon run piwith a fakePI_BINthat prints its env:maingives the child onlyBT_DAEMON_SOCKET/BT_DAEMON_DATA_DIR. This branch gives the child bothBRAINTRUST_DAEMON_*andBT_DAEMON_*with matching values. A deprecatedBT_TRACE_PI_PLUGIN_SPECis still honored.🤖 Generated with Claude Code
Co-authored by StarfolkAI (@starfolkai)[bot]