Skip to content

FE-1533: Add a petrinaut-shipping skill to the repo skills - #9392

Open
kube wants to merge 4 commits into
claude/fe-1532-petrinaut-launch-jsonfrom
claude/fe-1533-petrinaut-shipping-skill
Open

FE-1533: Add a petrinaut-shipping skill to the repo skills#9392
kube wants to merge 4 commits into
claude/fe-1532-petrinaut-launch-jsonfrom
claude/fe-1533-petrinaut-shipping-skill

Conversation

@kube

@kube kube commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

🌟 What is the purpose of this PR?

Ships the Petrinaut shipping procedure as a repo skill so every agent session is offered it, following the skills layout from #9323. Stacked on #9391; the standing conventions the skill applies land in #9390.

🔗 Related links

🔍 What does this change?

  • Adds the petrinaut-shipping skill under .agents/skills/: the per-package gate commands, the changeset step, docs-sync pointers, pre-PR hygiene checks, and the draft-to-ready CI loop. Standing conventions stay in the package AGENTS.md and are referenced, not restated.
  • Adds the .claude/skills/petrinaut-shipping symlink and regenerates skill-rules.json with yarn agents:skill-management generate-skill-rules.

Review fixes:

  • Keywords are down to petrinaut. changeset fuzzy-matched unrelated prompts through the hook's Levenshtein pass, so any monorepo changeset prompt pulled in this Petrinaut-only skill.
  • Dropped the files: block: the activation hook reads promptTriggers and never fileTriggers, so those globs did nothing.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

🛡 What tests cover this?

  • yarn agents:skill-management validate passes with the new skill.
  • yarn workspace @local/claude-hooks test:unit passes against the regenerated rules.

❓ How to test this?

  1. yarn agents:skill-management validate
  2. echo '{"session_id":"t","transcript_path":"/dev/null","cwd":".","permission_mode":"default","prompt":"fix a petrinaut lint failure and open a PR"}' | yarn workspace @local/claude-hooks run:skill
  3. Confirm petrinaut-shipping appears under the recommended skills.

🤖 Generated with Claude Code

@kube kube self-assigned this Aug 27, 2026
@kube
kube requested a review from lunelson August 27, 2026 22:35
@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hash Ready Ready Preview Sep 1, 2026 8:38am UTC
petrinaut Ready Ready Preview Sep 1, 2026 8:38am UTC
petrinaut-docs Ready Ready Preview Sep 1, 2026 8:38am UTC
1 Skipped Deployment
Project Deployment Actions Updated
hashdotdesign-tokens Ignored Ignored Preview Sep 1, 2026 8:38am UTC

Request Review

@cursor

cursor Bot commented Aug 27, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation and agent-hook configuration only; no runtime product or publishable library changes.

Overview
Adds a petrinaut-shipping agent skill so sessions that touch Petrinaut work get a suggested end-to-end ship checklist: per-package turbo/format/pytest gates, changeset and docs-sync pointers (conventions stay in package AGENTS.md), pre-PR hygiene, and draft-to-ready CI guidance including a gh pr checks JSON polling loop.

Wires it into the existing skills layout with .agents/skills/petrinaut-shipping/SKILL.md, a .claude/skills/petrinaut-shipping symlink, and a regenerated skill-rules.json entry (high-priority suggest, keyword petrinaut, intent patterns for implement/fix/PR/CI prompts; no file globs).

Reviewed by Cursor Bugbot for commit 9990f1a. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread .agents/skills/petrinaut-shipping/SKILL.md Outdated

@lunelson lunelson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool. The one thing that's odd here to me, although I know it's a pre-existing convention in this repo, is this non-standard frontmatter on the SKILL.md:

metadata:
  triggers:
    type: domain
    enforcement: suggest
    ...

The Agent Skills spec defines metadata as a string-to-string map, so a nested triggers object is non-conforming. A few concerns:

  • It creates two routing systems: the description-based model routing and HASH’s keyword/regex hook.
  • Several fields are still decorative anyway: the Claude hook reads prompt triggers and priority, but not fileTriggers, enforcement, blockMessage, or skipConditions.
  • The broad keywords are already causing trouble: Cursor correctly noted that changeset fuzzy-matches unrelated prompts.
  • Putting Claude-specific machinery in canonical .agents/skills somewhat undermines the supposed cross-agent portability.
  • The generated JSON makes the same configuration appear in three places: description, trigger metadata, and generated rules.

Personally I would remove metadata.triggers and rely on a precise description. If Claude-only activation is genuinely required, we should maybe work out a way to keep that configuration in a Claude-specific sidecar; or else find a way to encode it as a namespaced string-valued metadata field so the skill remains spec-compliant.

@kube

kube commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Sadly I cannot remove metadata.triggers, as it's required by our custom tooling. We need to check with Bilal and Tim for this.

For the record, yarn agents:skill-management validate (which CI runs as lint:skill) rejects a skill without it: metadata: Invalid input: expected object, received undefined. Frontmatter.metadata is required in libs/@local/repo-chores/node/scripts/skill-management/schemas.ts, and FrontmatterTriggers is a strictObject, so dropping the block is a schema change across all 14 skills rather than something this PR can do.

Your other points hold and I have acted on what is in scope here: the keywords are down to petrinaut (the changeset one was fuzzy-matching unrelated prompts, as Bugbot spotted), and the files: block is gone since the hook never reads fileTriggers.

Comment thread .agents/skills/petrinaut-shipping/SKILL.md Outdated
@indietyp

Copy link
Copy Markdown
Member

Note that this is a recent change, as evident by: https://github.com/agentskills/agentskills/pull/479/changes, we added these skills w/ metadata when it was first released, where it supported arbitrary key-value pairings, indeed these were all run through the validator during creation.

Comment thread .agents/skills/petrinaut-shipping/SKILL.md
lunelson
lunelson previously approved these changes Sep 1, 2026

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9990f1a. Configure here.

gh pr checks NNNN
```

An empty result means the checks do not exist yet and the loop keeps waiting. A PR that never settles has not started CI at all.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CI wait loop exits too early

High Severity

The draft-PR wait loop treats a pending count of 0 as “CI finished”, but gh pr checks --json prints [] when no checks have been scheduled yet. jq then prints 0, so the loop breaks immediately after open. The comment that an absent result keeps waiting does not apply: that case yields 0, not an empty string.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9990f1a. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/infra Relates to version control, CI, CD or IaC (area)

Development

Successfully merging this pull request may close these issues.

3 participants