fix: pi.dev command reference transforms and template args passing#950
Conversation
Fix two pi.dev integration bugs: - Use colon-based filenames (opsx:explore.md) so CLI commands render as /opsx:explore instead of /opsx-explore - Inject $@ into template body so user arguments are passed through Adds getLegacyFilePaths to ToolCommandAdapter for migration-safe cleanup of old hyphenated files during init/update.
|
Task completed. I'll start by examining the PR changes to provide a thorough review. Powered by 1Code |
📝 WalkthroughWalkthroughThe PR extends Pi adapter command generation to transform Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/core/init.ts (1)
788-806: Extract legacy alias cleanup into a shared helper.This method now duplicates the same legacy-path unlink logic that already exists in
UpdateCommand, so the next naming migration can easily makeinitandupdatediverge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/init.ts` around lines 788 - 806, The legacy-path unlink logic in removeLegacyCommandAliases duplicates code in UpdateCommand; extract this into a shared helper (e.g., removeLegacyCommandFiles or cleanupLegacyCommandAliases) that accepts (projectPath: string, adapter: ToolCommandAdapter, commandId: string) and uses getCommandFilePaths, path.isAbsolute, path.join, fs.existsSync and fs.promises.unlink to remove legacy files, then replace the body of removeLegacyCommandAliases and the corresponding logic in UpdateCommand to call the new helper to avoid duplication.src/core/update.ts (1)
219-226: AddidtoGeneratedCommandpayload to eliminate array-index pairing.The current code at lines 219–226 (and 710–717) derives
commandIdfromcommandContents[index]?.idwhile writing files fromgeneratedCommands[index]. AlthoughgenerateCommands()currently preserves 1:1 input order via.map(), the reliance on parallel-array indexing is fragile. If the implementation ever filters or transforms the command list, this cleanup could target the wrong legacy file or skip cleanup entirely.Instead of relying on positional coupling, include the command
idin theGeneratedCommandtype payload. This makes the relationship explicit and eliminates the implicit invariant.Also applies to: 710–717
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/update.ts` around lines 219 - 226, generatedCommands is being paired with commandContents by index (generatedCommands[index] and commandContents[index]?.id), which is fragile; update the GeneratedCommand payload to include id and propagate it from generateCommands so each GeneratedCommand contains its own id, then change the write loop to read commandId directly from cmd.id instead of commandContents[index]?.id and call removeLegacyCommandAliases(resolvedProjectPath, adapter, cmd.id); update any other places (e.g., the loop around lines 710–717) that use positional pairing to use the new cmd.id and adjust types/interfaces and generateCommands implementation accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/supported-tools.md`:
- Line 44: Update the overview paragraph that currently states command files use
"opsx-*" to include the Pi naming exception: change the sentence in the
docs/supported-tools.md overview to mention that while most tools use the opsx-*
pattern, Pi command files use the opsx:<id>.md form (give the example
`.pi/prompts/opsx:<id>.md`) and clarify how the `<id>` maps to Pi prompts;
update any related wording that asserts the pattern is universal so it reflects
this exception.
In `@src/core/command-generation/adapters/pi.ts`:
- Around line 47-49: getFilePath currently builds filenames using
`opsx:${commandId}.md` which contains a colon invalid on Windows; update the
`getFilePath` implementation (the getFilePath function that calls `path.join`)
to use a Windows-safe separator (e.g., `opsx-${commandId}.md`) or implement a
platform-aware fallback (replace/escape invalid characters) so filenames are
valid on Windows and match the other adapters' naming convention.
---
Nitpick comments:
In `@src/core/init.ts`:
- Around line 788-806: The legacy-path unlink logic in
removeLegacyCommandAliases duplicates code in UpdateCommand; extract this into a
shared helper (e.g., removeLegacyCommandFiles or cleanupLegacyCommandAliases)
that accepts (projectPath: string, adapter: ToolCommandAdapter, commandId:
string) and uses getCommandFilePaths, path.isAbsolute, path.join, fs.existsSync
and fs.promises.unlink to remove legacy files, then replace the body of
removeLegacyCommandAliases and the corresponding logic in UpdateCommand to call
the new helper to avoid duplication.
In `@src/core/update.ts`:
- Around line 219-226: generatedCommands is being paired with commandContents by
index (generatedCommands[index] and commandContents[index]?.id), which is
fragile; update the GeneratedCommand payload to include id and propagate it from
generateCommands so each GeneratedCommand contains its own id, then change the
write loop to read commandId directly from cmd.id instead of
commandContents[index]?.id and call
removeLegacyCommandAliases(resolvedProjectPath, adapter, cmd.id); update any
other places (e.g., the loop around lines 710–717) that use positional pairing
to use the new cmd.id and adjust types/interfaces and generateCommands
implementation accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 84bcac05-b30c-4c8e-954a-03c7710a5129
📒 Files selected for processing (9)
docs/supported-tools.mdsrc/core/command-generation/adapters/pi.tssrc/core/command-generation/index.tssrc/core/command-generation/types.tssrc/core/init.tssrc/core/profile-sync-drift.tssrc/core/update.tstest/core/command-generation/adapters.test.tstest/core/update.test.ts
| | Kiro (`kiro`) | `.kiro/skills/openspec-*/SKILL.md` | `.kiro/prompts/opsx-<id>.prompt.md` | | ||
| | OpenCode (`opencode`) | `.opencode/skills/openspec-*/SKILL.md` | `.opencode/commands/opsx-<id>.md` | | ||
| | Pi (`pi`) | `.pi/skills/openspec-*/SKILL.md` | `.pi/prompts/opsx-<id>.md` | | ||
| | Pi (`pi`) | `.pi/skills/openspec-*/SKILL.md` | `.pi/prompts/opsx:<id>.md` | |
There was a problem hiding this comment.
Document the new naming exception in the overview.
This row is correct, but the overview on Line 10 still says command files use opsx-*. With Pi now using opsx:<id>.md, that statement is no longer universally true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/supported-tools.md` at line 44, Update the overview paragraph that
currently states command files use "opsx-*" to include the Pi naming exception:
change the sentence in the docs/supported-tools.md overview to mention that
while most tools use the opsx-* pattern, Pi command files use the opsx:<id>.md
form (give the example `.pi/prompts/opsx:<id>.md`) and clarify how the `<id>`
maps to Pi prompts; update any related wording that asserts the pattern is
universal so it reflects this exception.
- Transform /opsx: references to /opsx- in Pi command bodies and skills, matching the hyphenated filename convention (same approach as OpenCode) - Inject $@ into template body so user arguments are passed through Pi uses the filename (minus .md) as the slash command name, so opsx-propose.md becomes /opsx-propose. This keeps filenames cross-platform safe while ensuring command references in the body match the actual command names.
alfred-openspec
left a comment
There was a problem hiding this comment.
Looks good. This keeps Pi on Windows-safe hyphenated filenames, fixes the command references to match what Pi actually exposes, and passes args through cleanly.
Summary
/opsx:references to/opsx-(matching hyphenated filenames), using the sametransformToHyphenCommandsapproach as OpenCode$@interpolation via**Provided arguments**: $@after**Input**:headings, so user arguments (e.g.,/opsx-propose add-dark-mode) are passed through to the agentopsx-<id>.md(cross-platform safe — colons are invalid on Windows NTFS, and pi.dev supports Windows)Closes #912
Test plan
/opsx:to/opsx-in body$@into body's Input sectionopenspec initwith pi selected, verify/opsx:refs in body are transformed to/opsx-$@appears in generated prompt files🤖 Generated with Claude Code