Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bt-daemon/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ fn managed_run_args(
RunSource::Pi => {
let extension = match std::env::var_os("BT_TRACE_PI_PLUGIN_SPEC") {
Some(extension) => extension,
None => OsString::from(crate::setup::pi_plugin_spec()?),
None => OsString::from(crate::setup::pi_plugin_spec()),
};
Ok(vec![OsString::from("-e"), extension])
}
Expand Down Expand Up @@ -2032,7 +2032,7 @@ mod tests {
managed_run_args(RunSource::Pi, &test_run_hook_command()).unwrap(),
vec![
OsString::from("-e"),
OsString::from(crate::setup::pi_plugin_spec().unwrap()),
OsString::from(crate::setup::pi_plugin_spec()),
]
);
}
Expand Down
26 changes: 11 additions & 15 deletions bt-daemon/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const OPENCODE_PACKAGE: &str = "@braintrust/trace-opencode";
const PI_PACKAGE: &str = "@braintrust/pi-extension";
const OPENCODE_PACKAGE_MANIFEST: &str =
include_str!("../../src/plugins/opencode/content/package.json");
const PI_PACKAGE_MANIFEST: &str = include_str!("../../src/plugins/pi/content/package.json");
const ANTIGRAVITY_PLUGIN: &str = "braintrust-antigravity-tracing";
const LEGACY_CLAUDE_TRACING_ENV_KEYS: [&str; 2] = ["BRAINTRUST_CC_PROJECT", "BRAINTRUST_CC_DEBUG"];
#[cfg(unix)]
Expand Down Expand Up @@ -53,11 +52,8 @@ fn opencode_plugin_spec() -> anyhow::Result<String> {
npm_major_spec(OPENCODE_PACKAGE, OPENCODE_PACKAGE_MANIFEST)
}

pub(crate) fn pi_plugin_spec() -> anyhow::Result<String> {
Ok(format!(
"npm:{}",
npm_major_spec(PI_PACKAGE, PI_PACKAGE_MANIFEST)?
))
pub(crate) fn pi_plugin_spec() -> String {
format!("npm:{PI_PACKAGE}")
}

fn version_is_older(installed: &str, expected: &str) -> bool {
Expand Down Expand Up @@ -139,10 +135,10 @@ fn pi_update_required() -> bool {
return false;
}
let installed = String::from_utf8_lossy(&output.stdout);
let expected = pi_plugin_spec();
installed.lines().any(|line| {
let plugin = line.trim();
plugin.starts_with("npm:@braintrust/pi-extension")
&& Some(plugin) != pi_plugin_spec().ok().as_deref()
plugin.starts_with(&expected) && plugin != expected
Comment on lines +138 to +141

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Compare the installed Pi version instead of its source

For every installation created by the new setup_pi, pi list reports the same unversioned source npm:@braintrust/pi-extension, so this predicate remains false even after a newer extension is published and the locally resolved package is stale. Consequently update_warning("pi") can no longer prompt these users to run bt trace update pi; inspect Pi's installed/resolved version rather than comparing the configured source string.

Useful? React with 👍 / 👎.

})
}

Expand Down Expand Up @@ -673,18 +669,18 @@ fn disable_opencode() -> anyhow::Result<()> {
}

fn setup_pi(runner: &mut impl CommandRunner) -> anyhow::Result<()> {
let plugin = pi_plugin_spec()?;
let plugin = pi_plugin_spec();
runner.run("pi", &["install", &plugin])
}

fn disable_pi(runner: &mut impl CommandRunner) -> anyhow::Result<()> {
let plugin = pi_plugin_spec()?;
let plugin = pi_plugin_spec();
runner.run("pi", &["uninstall", &plugin])
Comment on lines +677 to 678

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve uninstall support for existing versioned Pi installs

Users who enabled Pi with any prior release have the configured source npm:@braintrust/pi-extension@^<major>, because that is what the previous setup_pi installed. Passing the newly unversioned source to pi uninstall no longer identifies that saved package source, so bt trace disable pi fails and leaves the extension installed for the existing user base. Reconcile the legacy versioned source during migration or uninstall the source reported by pi list rather than assuming the new source.

Useful? React with 👍 / 👎.

}

fn update_pi(runner: &mut impl CommandRunner) -> anyhow::Result<()> {
let package = format!("npm:{PI_PACKAGE}");
runner.run("pi", &["update", &package])
let plugin = pi_plugin_spec();
runner.run("pi", &["update", &plugin])
}

fn antigravity_home(config_dir: &Path) -> anyhow::Result<&Path> {
Expand Down Expand Up @@ -1333,12 +1329,12 @@ mod tests {
}

#[test]
fn pi_installs_the_published_extension_range() {
fn pi_installs_the_latest_published_extension() {
let mut runner = FakeRunner::new([]);

setup_pi(&mut runner).unwrap();

assert!(runner.called(&format!("pi install {}", pi_plugin_spec().unwrap())));
assert!(runner.called("pi install npm:@braintrust/pi-extension"));
}

#[test]
Expand Down Expand Up @@ -1672,7 +1668,7 @@ mod tests {

let mut pi = FakeRunner::new([]);
disable_pi(&mut pi).unwrap();
assert!(pi.called(&format!("pi uninstall {}", pi_plugin_spec().unwrap())));
assert!(pi.called("pi uninstall npm:@braintrust/pi-extension"));
}

#[test]
Expand Down
Loading