Skip to content

fix(pi): standardize on name when installing/updating - #86

Merged
Abhijeet Prasad (AbhiPrasad) merged 1 commit into
mainfrom
abhi-pi-cleanup-update-install
Sep 16, 2026
Merged

Abhijeet Prasad (AbhiPrasad) merged 1 commit into
mainfrom
abhi-pi-cleanup-update-install

Conversation

@AbhiPrasad

Copy link
Copy Markdown
Member

bt trace update pi was broken, this was because we weren't using the name here properly.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-16T20:55:59.278942Z fbb1e54 New commits
ℹ️ About Codex in GitHub

Your 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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b866976fc4

ℹ️ 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".

Comment thread bt-daemon/src/setup.rs
Comment on lines +138 to +141
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

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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbb1e54fe6

ℹ️ 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".

Comment thread bt-daemon/src/setup.rs
Comment on lines +677 to 678
let plugin = pi_plugin_spec();
runner.run("pi", &["uninstall", &plugin])

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 👍 / 👎.

@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) merged commit c128225 into main Sep 16, 2026
20 checks passed
@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) deleted the abhi-pi-cleanup-update-install branch September 16, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants