Skip to content

fix: address review comments on skill refactor#16

Merged
pjcdawkins merged 2 commits intorefactor-skillsfrom
fix-review-comments-2
Apr 21, 2026
Merged

fix: address review comments on skill refactor#16
pjcdawkins merged 2 commits intorefactor-skillsfrom
fix-review-comments-2

Conversation

@pjcdawkins
Copy link
Copy Markdown
Collaborator

@pjcdawkins pjcdawkins commented Apr 20, 2026

Stacked on #14 (refactor-skills). Addresses review comments on the skill refactor.

Summary

  • SKILL.md frontmatter. allowed-tools now uses comma separation so the read-only patterns parse as separate entries instead of one big invalid spec. Pattern shapes kept narrow and read-only.
  • Deploy strategy default. upsun push / upsun deploy default to stopstart, not rolling. Flipped the description; rolling is now described as opt-in for zero-downtime deploys.
  • `env:deploy:type` syntax. Was shown as a property set via `set env:deploy:type manual`; actual syntax is `upsun env:deploy:type manual` (it's a subcommand).
  • Resources guidance. Clarified that `upsun resources:set --size` takes numeric CPU values (e.g. `myapp:0.25`); container profiles (`HIGH_CPU` etc.) are still correct but live in `.upsun/config.yaml` as `container_profile:`, not on the CLI.
  • MCP tool names. Qualified `list-project` / `list-environment` with the `mcp__upsun__` prefix so agents invoke them correctly.
  • Install paths. Updated to `github.com/upsun/cli` (CLI repo moved from `platformsh/cli`); tap is `upsun/tap`, Scoop bucket is `upsun/homebrew-tap`. Mentioned native Alpine/Debian/RPM packages. Applied the same updates to `README.md` and the CI workflow.
  • README cleanup. Replaced stale `using-upsun` references with `upsun`; collapsed the eleven-file reference list to the single `config.md` that ships with the new skill; switched `auth:browser-login` to `upsun login`.
  • CI workflow. Renamed the skill install step; dropped the redundant `$HOME/.platformsh/bin` PATH export (the installer uses `$HOME/.local/bin`, which is already on PATH for the ubuntu-latest runner).

- SKILL.md: switch allowed-tools frontmatter to comma separation so
  read-only patterns are parsed as separate entries.
- SKILL.md: correct deploy strategy default (stopstart, not rolling);
  rolling is opt-in for zero-downtime deploys.
- SKILL.md: fix env:deploy:type syntax — it's a command, not a property
  set via `set`.
- SKILL.md: clarify resources:set uses numeric CPU sizes; container
  profile (HIGH_CPU/BALANCED/HIGH_MEMORY/HIGHER_MEMORY) is set in
  .upsun/config.yaml, not via CLI.
- SKILL.md: update install commands to github.com/upsun/cli (CLI repo
  moved from platformsh/cli) and note Alpine/Debian/RPM packages.
- SKILL.md: qualify MCP tool names with mcp__upsun__ prefix.
- README.md: replace stale `using-upsun` references with `upsun`;
  simplify skill documentation section now that there is one reference
  file instead of eleven. Update install commands to upsun/cli.
- run-evals.yml: rename step to "Install upsun skill"; update CLI
  installer URL; drop redundant PATH export ($HOME/.local/bin is on
  PATH by default on ubuntu-latest runners).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pjcdawkins pjcdawkins mentioned this pull request Apr 20, 2026
Comment thread plugins/upsun/skills/upsun/SKILL.md Outdated
Co-authored-by: Ganeshdip Dumbare <ganeshdip.dumbare@gmail.com>
@pjcdawkins pjcdawkins merged commit 197e86f into refactor-skills Apr 21, 2026
1 check passed
@pjcdawkins pjcdawkins deleted the fix-review-comments-2 branch April 21, 2026 07:49
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