Skip to content

fix: use correct Pi installation path - #69

Open
Bloomca wants to merge 1 commit into
mainfrom
seva/fix-pi-installation-path
Open

Bloomca wants to merge 1 commit into
mainfrom
seva/fix-pi-installation-path

Conversation

@Bloomca

@Bloomca Bloomca commented Sep 30, 2026 •

Copy link
Copy Markdown

Description

Pi used incorrect installation path -- it needs .pi/agent prefix for skills. It was fixed in todoist-cli in Doist/todoist-cli#414, so this PR is pretty much the same fix.

Testing

It is a bit annoying to test, but if you want, here are the steps:

rm -rf ~/.pi/skills/comms-cli ~/.pi/agent/skills/comms-cli
npm ci --ignore-scripts
npm run build
npm link --ignore-scripts
hash -r
realpath "$(command -v tdc)" # confirm that the path points to your folder
tdc skill install pi
pi
  • Comms-CLI skill in Pi should be recognized

To restore the global package, do this:

npm unlink -g @doist/comms-cli
npm install -g @doist/comms-cli
hash -r

@Bloomca Bloomca added the 👀 Show PR PR must be reviewed before or after merging label Sep 30, 2026
@doistbot
doistbot requested a review from nats12 September 30, 2026 23:25

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Clean, minimal fix that redirects Pi's global skill installs to ~/.pi/agent/skills while leaving local .pi/skills installs and all other agents unchanged — the new optional globalDirName in the installer config flows through getInstallPath, so install/update/uninstall paths stay consistent. No inline issues were flagged; the added tests cover both the new global and unchanged local paths, and no security or reuse concerns were introduced.

I also left one optional follow-up note in the details below.

Optional follow-up note (1)
  • P3 src/commands/skill/skill.test.ts:113: This local-path test duplicates the existing installer paths table case for pi, which already asserts the local path contains .pi, skills, comms-cli, SKILL.md, and cwd. Local path behavior didn't change in this PR, so drop this test (or fold only the new global .pi/agent assertion into the existing table) to avoid maintaining the pi path in two places.

Share Feedback • Review Logs

@Bloomca
Bloomca force-pushed the seva/fix-pi-installation-path branch from 52f2fea to a9eb4d5 Compare September 30, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants