fix(skill): point npx source at nottelabs/notte-skills#40
Conversation
|
| Filename | Overview |
|---|---|
| internal/cmd/skills.go | Core fix: points npx skills add at nottelabs/notte-skills instead of nottelabs/notte-cli; extracts shared runNpx helper; adds --upgrade/-f flag that delegates to npx skills update notte-browser. Two minor documentation inconsistencies noted. |
| internal/cmd/skills_test.go | Adds TestSkillAddUpgradeFlag (flag registration guard) and TestSkillSourcePointsToSkillsRepo (regression guard for source URL/skill name); removes stale inline comments. |
| .github/workflows/ci.yml | New skill-add job: builds CLI, runs notte skill add and notte skill add --upgrade in isolated temp dirs, and asserts SKILL.md is written — catches future source-URL drift. |
| README.md | Updates the npx skills add example in the README from nottelabs/notte-cli to nottelabs/notte-skills, keeping docs in sync with the code change. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
internal/cmd/skills.go:75
The `action` label passed to `runNpx` is always `"skill installation"` regardless of whether `--upgrade` was used. If the upgrade step fails, the error message will read "skill installation failed with exit code …" which would mislead a user who ran `notte skill add --upgrade`.
```suggestion
action := "skill installation"
if skillAddUpgrade {
action = "skill upgrade"
}
return runNpx(cmd, action, npxArgs)
```
### Issue 2 of 2
internal/cmd/skills.go:52
The `Long` help text for `skillRemoveCmd` shows `npx skills remove --skill notte-browser` but the actual invocation also appends `-y`. Users reading `--help` output will see an incomplete command if they try to reproduce the behaviour manually.
```suggestion
This command runs: npx skills remove --skill notte-browser -y`,
```
Reviews (3): Last reviewed commit: "fix(skill): point npx source at nottelab..." | Re-trigger Greptile
`notte skill add` ran `npx skills add nottelabs/notte-cli`, which clones this repo and searches for SKILL.md files. After the skills/ directory was replaced by the notte-skills submodule (3eca2b3), `git clone` no longer pulls in skill content, so the tool reported "No skills found". Point the npx source at nottelabs/notte-skills (where SKILL.md actually lives) and refactor the npx invocation so add/remove/upgrade share one helper. Also add a `-f` / `--upgrade` flag that delegates to `npx skills update notte-browser` for refreshing an installed skill. Add a skill-add CI job that builds the CLI, runs `notte skill add` and `notte skill add --upgrade` in temp dirs, and asserts that .agents/skills/notte-browser/SKILL.md is written — so a future drift in the source URL or skill name fails CI loudly instead of silently saying "No skills found". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The skill-add CI job failed: `npx skills add` found the skill but then
hit its interactive agent picker ("Which agents do you want to install
to?"). With no TTY the picker aborts and installs nothing, so the job's
SKILL.md assertion failed. Locally this was masked because npx
auto-detects the agent when run inside Claude Code.
Forward the CLI's global --yes flag to npx as `-y`, which skips the
picker and installs to all detected assistants. This matches how the
rest of the cmd package gates non-interactive behavior (confirm.go).
Run the CI job with `notte --yes skill add`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
28242a2 to
872b7a0
Compare
Summary
notte skill addwas runningnpx skills add nottelabs/notte-cli, which clones this repo and searches forSKILL.mdfiles. After theskills/dir was replaced by thenotte-skillssubmodule (3eca2b3),git cloneno longer pulls in skill content — the tool reportedNo skills found. Pointed it atnottelabs/notte-skillsinstead (whereSKILL.mdactually lives).-f/--upgradeonnotte skill addthat delegates tonpx skills update notte-browserto refresh an installed skill.skill-addCI job that builds the CLI, runsnotte skill addandnotte skill add --upgradein temp dirs, and asserts.agents/skills/notte-browser/SKILL.mdis written — so a future drift in the source URL or skill name fails CI loudly instead of silently sayingNo skills found.npx skills add ...command in the README.Test plan
go build ./...cleango test -short ./...passes (including newTestSkillAddUpgradeFlagandTestSkillSourcePointsToSkillsReporegression guard)notte skill addin a fresh temp dir installs.agents/skills/notte-browser/SKILL.mdnotte skill add --upgraderefreshes the installed skill (✓ Updated notte-browser)skill-addjob passes on this PR🤖 Generated with Claude Code