Skip to content

feat: report which skill failed to install and why - #291

Open
beeman wants to merge 1 commit into
mainfrom
beeman/report-skill-install-failures
Open

beeman wants to merge 1 commit into
mainfrom
beeman/report-skill-install-failures

Conversation

@beeman

@beeman beeman commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

A failed skill install previously only surfaced as Installed 1/2 skills, with the actual error hidden behind --verbose. This came up in solana-mobile-cli, where the react-kit-shadcn template's first skill (solana-foundation/solana-dev-skill) fails to install because its SKILL.md frontmatter has an unquoted description: containing : , which is invalid YAML — and there was no way to tell from the output.

The install-skills task now collects per-skill failures and always logs a warning per failed skill with the skill source, the first line of the skills CLI's stderr (e.g. the YAML parse error), and a pointer to the full error log written by execAndWait:

▲  Failed to install skill https://github.com/solana-foundation/solana-dev-skill: ⚠ Skipped …/SKILL.md — YAML parse error: Nested mappings are not allowed in compact mappings at line 2, column 14: (full log: …/skill-test-e2e.error.log)
◇  Installed 1/2 skills

Verified end-to-end with a built dist against gh:solana-mobile/templates/mobile/react-kit-shadcn. Tests updated: the failure test now asserts the always-on warning, and a new test covers reason extraction from CreateAppError.

@changeset-bot

changeset-bot Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 50ae922

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/create-solana-dapp@291

commit: 50ae922

@greptile-apps

greptile-apps Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge with no outstanding correctness, security, or repository-rule issues.

Summary

The PR improves best-effort skill installation diagnostics without changing its failure semantics.

  • Collects each failed skill installation and emits an always-visible warning identifying the source.
  • Extracts the first meaningful error line from CreateAppError and points users to the complete durable error log.
  • Adds coverage for ordinary errors, structured command errors, partial success, and complete failure.

Reviews (2) · Last reviewed commit: "feat: report which skill failed to insta..."

@beeman

beeman commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

This should make it easier to spot issues like this solana-foundation/solana-dev-skill#74

Previously a failed skill install only surfaced as "Installed 1/2 skills" with the actual error hidden behind --verbose. Each failure now logs a warning with the skill source and the first line of the skills CLI's stderr (e.g. a SKILL.md YAML parse error), plus a pointer to the full error log.
@beeman
beeman force-pushed the beeman/report-skill-install-failures branch from 43abe59 to 50ae922 Compare September 19, 2026 19:20
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.

1 participant