feat: add script creation standards and cross-platform guidance#46
feat: add script creation standards and cross-platform guidance#46
Conversation
Add script-standards reference docs to both builders enforcing Python-first, PEP 723 metadata, uv run invocation, and cross-platform portability. Update build processes to load standards when scripts are involved and require explicit user approval for external dependencies. Add explanation doc covering why deterministic scripts improve skill quality.
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR introduces comprehensive standards and documentation for deterministic script usage in skills. It adds a new "Scripts in Skills" explanation page, establishes cross-platform script creation standards for both agent-builder and workflow-builder skills, updates build processes to enforce dependency approval and reference these standards, and shifts the default scripting approach from Bash-first to Python-first with restricted shell access. Changes
Possibly related PRs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
🤖 Augment PR SummarySummary: This PR strengthens guidance around using deterministic scripts inside skills, with an emphasis on cross-platform portability and predictable execution. Changes:
Technical Notes: The standards formalize PEP 723 inline metadata + 🤖 Was this summary useful? React with 👍 or 👎 |
| - `git`, `gh` — version control and GitHub CLI | ||
| - `uv run` — Python script execution with automatic dependency handling | ||
| - `npm`, `npx`, `pnpm` — Node.js ecosystem | ||
| - `mkdir -p` — directory creation |
There was a problem hiding this comment.
In skills/bmad-agent-builder/references/script-standards.md line 13, listing mkdir -p as “works reliably across all environments” isn’t correct for Windows-native shells (PowerShell/cmd don’t support -p). This could cause generated skills to fail if they follow the “safe bash commands” list on Windows.
Severity: medium
Other Locations
skills/bmad-workflow-builder/references/script-standards.md:13docs/explanation/scripts-in-skills.md:57
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| How a built skill's SKILL.md should reference its scripts: | ||
|
|
||
| - **Scripts with external dependencies:** `uv run scripts/analyze.py {args}` | ||
| - **Stdlib-only scripts:** `python3 scripts/scan.py {args}` (also fine to use `uv run` for consistency) |
There was a problem hiding this comment.
In skills/bmad-agent-builder/references/script-standards.md line 65, the guidance uses python3 ... for stdlib-only scripts, but python3 isn’t a reliable command on Windows-native installs (often python or py). Since the doc claims native Windows support, these examples may break when followed verbatim.
Severity: medium
Other Locations
skills/bmad-workflow-builder/references/script-standards.md:65docs/explanation/scripts-in-skills.md:63docs/explanation/scripts-in-skills.md:110skills/bmad-workflow-builder/references/script-standards.md:47skills/bmad-agent-builder/references/script-standards.md:47skills/bmad-workflow-builder/references/script-standards.md:77
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Summary
script-standards.mdreference to both agent builder and workflow builder, enforcing Python-first scripts, PEP 723 inline metadata,uv runinvocation, and cross-platform portability (macOS/Linux/Windows)scripts-in-skills.mdexplanation doc covering why deterministic scripts improve skill quality, cost, and reliabilitygit,gh,uv run, etc.)Test plan
Summary by CodeRabbit