Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions config/claude/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,9 @@
source = ./statusline-git.sh;
executable = true;
};

home.file.".claude/install-skills.sh" = {
source = ./install-skills.sh;
executable = true;
};
}
18 changes: 18 additions & 0 deletions config/claude/install-skills.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash

Copilot AI Jan 15, 2026

Copy link

Choose a reason for hiding this comment

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

This script lacks test coverage. The repository has comprehensive bash script testing using shellspec (see spec/ directory), and all other scripts in config/claude/ have corresponding spec files. Create spec/install_skills_spec.sh following the pattern used in spec/notify_spec.sh, spec/security_spec.sh, etc. Additionally, update spec/coverage_spec.sh to include 'config/claude/install-skills.sh' in the covered_scripts list (around line 96).

Copilot uses AI. Check for mistakes.
# Install agent skills from external repositories
# Usage: ./install-skills.sh
# Docs: https://agentskills.io

set -euo pipefail

echo "Installing agent skills..."

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.

medium

The script executes npx without first checking if it's available. If npx is not installed or not in the PATH, the script will fail with a generic 'command not found' error. It's better practice to add an explicit check for dependencies at the start of a script to provide a more user-friendly error message.

Suggested change
echo "Installing agent skills..."
if ! command -v npx &> /dev/null; then
echo "Error: npx could not be found. Please install Node.js." >&2
exit 1
fi
echo "Installing agent skills..."


# Install Vercel agent-skills (React best practices + Web design guidelines)
# https://github.com/vercel-labs/agent-skills
npx add-skill vercel-labs/agent-skills \

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.

security-high high

For security and reproducibility, it's highly recommended to pin the version of the npm package being executed with npx. Using npx add-skill without a version specifier will always fetch the latest version. This could unexpectedly introduce breaking changes or even security vulnerabilities if the package is compromised (a supply chain attack). Pinning to a specific version ensures that the script's behavior is consistent and secure over time. Please find the current stable version of add-skill and specify it.

Suggested change
npx add-skill vercel-labs/agent-skills \
npx add-skill@<version> vercel-labs/agent-skills \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

No version or commit hash specified for vercel-labs/agent-skills. This means the script will always pull the latest version, which could introduce breaking changes or inconsistencies across machines. For reproducibility, consider:

  1. Pinning to a specific commit: vercel-labs/agent-skills@abc1234
  2. Pinning to a release tag: vercel-labs/agent-skills@v1.0.0
  3. Or document that skills should be manually reviewed/updated

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#579
File: config/claude/install-skills.sh#L12
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
No version or commit hash specified for `vercel-labs/agent-skills`. This means the script will always pull the latest version, which could introduce breaking changes or inconsistencies across machines. For reproducibility, consider:
1. Pinning to a specific commit: `vercel-labs/agent-skills@abc1234`
2. Pinning to a release tag: `vercel-labs/agent-skills@v1.0.0`
3. Or document that skills should be manually reviewed/updated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

No version pinning for the add-skill package. Each time this script runs, it could pull a different version of add-skill, potentially with breaking changes. Consider pinning to a specific version: npx add-skill@1.2.3 (replace with current stable version).

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#579
File: config/claude/install-skills.sh#L12
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
No version pinning for the `add-skill` package. Each time this script runs, it could pull a different version of `add-skill`, potentially with breaking changes. Consider pinning to a specific version: `npx add-skill@1.2.3` (replace with current stable version).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

Using npx without a Nix-managed path creates a dependency on the system's Node.js installation. In this Nix-based repository, other scripts use path substitution (e.g., ${pkgs.nodejs}/bin/npx) to ensure reproducibility. Consider either:

  1. Adding this script to config/claude/default.nix with pkgs.replaceVars to inject the Nix store path
  2. Converting this to a home-manager activation script like npm-globals does

See home-manager/modules/npm-globals/install-npm-globals.sh for a similar pattern.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#579
File: config/claude/install-skills.sh#L12
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Using `npx` without a Nix-managed path creates a dependency on the system's Node.js installation. In this Nix-based repository, other scripts use path substitution (e.g., `${pkgs.nodejs}/bin/npx`) to ensure reproducibility. Consider either:
1. Adding this script to `config/claude/default.nix` with `pkgs.replaceVars` to inject the Nix store path
2. Converting this to a home-manager activation script like `npm-globals` does

See `home-manager/modules/npm-globals/install-npm-globals.sh` for a similar pattern.

--agent claude-code \
--skill vercel-react-best-practices \
--skill web-design-guidelines \
--yes
Comment on lines +12 to +16

Copilot AI Jan 15, 2026

Copy link

Choose a reason for hiding this comment

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

The script should verify that Node.js/npx is available before attempting to use it. Other scripts in this repository check for required commands using 'command -v'. Add a check like 'if ! command -v npx >/dev/null 2>&1; then echo "Error: npx is required but not installed"; exit 1; fi' before the npx command.

Copilot uses AI. Check for mistakes.

echo "Done! Skills installed to ~/.claude/skills/"

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.

medium

The tilde ~ character is not expanded to the user's home directory when it is inside double quotes. This means the output will literally be Done! Skills installed to ~/.claude/skills/. While users will likely understand this, using the $HOME environment variable is more explicit and guarantees correct expansion by the shell, making the output message more accurate.

Suggested change
echo "Done! Skills installed to ~/.claude/skills/"
echo "Done! Skills installed to $HOME/.claude/skills/"

7 changes: 6 additions & 1 deletion spec/coverage_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ It 'has spec file for config/claude/statusline-git.sh'
The path "spec/statusline_git_spec.sh" should be exist
End

It 'has spec file for config/claude/install-skills.sh'
The path "spec/install_skills_spec.sh" should be exist
End

It 'has spec file for home-manager/programs/neovim/run_tests.sh'
The path "spec/neovim_tests_spec.sh" should be exist
End
Expand Down Expand Up @@ -93,7 +97,8 @@ Describe 'no shell scripts are missing from coverage list'
It 'covers all non-spec shell scripts in the repository'
# List of all shell scripts that should have tests
# Update this list when adding new shell scripts
covered_scripts="config/claude/notify.sh
covered_scripts="config/claude/install-skills.sh
config/claude/notify.sh
config/claude/pushover.sh
config/claude/security.sh
config/claude/statusline-git.sh
Expand Down
70 changes: 70 additions & 0 deletions spec/install_skills_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#!/usr/bin/env bash
# shellcheck disable=SC2329,SC2016

Describe 'install-skills.sh'
SCRIPT="$PWD/config/claude/install-skills.sh"

Describe 'script structure'
It 'has valid bash syntax'
When run bash -n "$SCRIPT"
The status should be success
End

It 'uses strict mode'
When run grep -q 'set -euo pipefail' "$SCRIPT"
The status should be success
End

It 'has executable permission'
When run test -x "$SCRIPT"
The status should be success
End
End

Describe 'npx add-skill invocation'
setup() {
mock_bin_setup npx
}
cleanup() {
mock_bin_cleanup
}
Before 'setup'
After 'cleanup'

It 'calls npx add-skill with correct arguments'
When run bash "$SCRIPT"
The status should be success
The output should include 'Installing agent skills'
End

It 'passes vercel-labs/agent-skills as source'
When run bash -c 'bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'vercel-labs/agent-skills'
End

It 'specifies claude-code as target agent'
When run bash -c 'bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include '--agent claude-code'
End

It 'includes vercel-react-best-practices skill'
When run bash -c 'bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'vercel-react-best-practices'
End

It 'includes web-design-guidelines skill'
When run bash -c 'bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include 'web-design-guidelines'
End

It 'uses --yes flag for non-interactive mode'
When run bash -c 'bash '"$SCRIPT"'; cat "$MOCK_LOG"'
The status should be success
The output should include '--yes'
End
End
End
Loading