Skip to content

fix(skill): point npx source at nottelabs/notte-skills#40

Open
giordano-lucas wants to merge 1 commit into
mainfrom
t3code/3395d787
Open

fix(skill): point npx source at nottelabs/notte-skills#40
giordano-lucas wants to merge 1 commit into
mainfrom
t3code/3395d787

Conversation

@giordano-lucas
Copy link
Copy Markdown
Member

Summary

  • notte skill add was running npx skills add nottelabs/notte-cli, which clones this repo and searches for SKILL.md files. After the skills/ dir was replaced by the notte-skills submodule (3eca2b3), git clone no longer pulls in skill content — the tool reported No skills found. Pointed it at nottelabs/notte-skills instead (where SKILL.md actually lives).
  • Added -f / --upgrade on notte skill add that delegates to npx skills update notte-browser to refresh an installed skill.
  • Added a skill-add CI job that builds the CLI, runs notte skill add and notte skill add --upgrade in temp dirs, and asserts .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.
  • Also updated the matching npx skills add ... command in the README.

Test plan

  • go build ./... clean
  • go test -short ./... passes (including new TestSkillAddUpgradeFlag and TestSkillSourcePointsToSkillsRepo regression guard)
  • Local end-to-end: notte skill add in a fresh temp dir installs .agents/skills/notte-browser/SKILL.md
  • Local end-to-end: notte skill add --upgrade refreshes the installed skill (✓ Updated notte-browser)
  • CI skill-add job passes on this PR

🤖 Generated with Claude Code

`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>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR fixes a broken notte skill add command by pointing npx skills add at nottelabs/notte-skills (where SKILL.md actually lives) instead of nottelabs/notte-cli (where it was an empty submodule). It also adds a --upgrade/-f flag, a shared runNpx helper, and a new skill-add CI job that performs an end-to-end install assertion to prevent silent regressions.

  • Bug fix: skillSource constant and README both updated from nottelabs/notte-clinottelabs/notte-skills.
  • New feature: --upgrade/-f flag on notte skill add delegates to npx skills update notte-browser to refresh an installed skill.
  • Testing: Unit regression guards (TestSkillSourcePointsToSkillsRepo, TestSkillAddUpgradeFlag) and a new CI skill-add job that runs the real notte skill add end-to-end and asserts SKILL.md is written.

Confidence Score: 5/5

Safe to merge — all changes are targeted at the broken npx source URL and are well-covered by new unit and CI end-to-end tests.

The fix is a one-constant change with no side effects on other code paths; the refactored runNpx helper is a straightforward extraction; the new --upgrade flag only activates on explicit opt-in.

No files require special attention.

Important Files Changed

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.

Fix All in Claude Code

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

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