fix(skills): use current SDK from upstream dotfiles - #186
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Makefile">
<violation number="1" location="Makefile:92">
P2: `skills-install` now runs `bun install --frozen-lockfile` unconditionally on every invocation, before the all-installed idempotency check that used to let the recipe exit 0 without touching bun. This makes `make sync` and offline/idempotent re-syncs depend on a live bun registry and rewrites node_modules every run; when the registry is unreachable the whole sync now hard-fails even though all skills are already present. Gate the install on the CLI already existing so the no-op path stays bun-free: `[ ! -x "$(SKILLS_CLI)" ] && ! (cd "$(SKILLS_PROJECT_DIR)" && bun install ... )` else error.</violation>
<violation number="2" location="Makefile:140">
P3: The `bun install --frozen-lockfile --minimum-release-age 0 --no-progress` command is duplicated verbatim in both `skills-install` and `skills-update`. A shared `SKILLS_*` variable (alongside the other SKILLS_ vars) would keep them in sync and make future flag changes a one-line edit.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if ! (cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null); then \ | ||
| echo "Error: failed to install the skills SDK from bun.lock"; \ | ||
| exit 1; \ | ||
| fi; \ |
There was a problem hiding this comment.
P2: skills-install now runs bun install --frozen-lockfile unconditionally on every invocation, before the all-installed idempotency check that used to let the recipe exit 0 without touching bun. This makes make sync and offline/idempotent re-syncs depend on a live bun registry and rewrites node_modules every run; when the registry is unreachable the whole sync now hard-fails even though all skills are already present. Gate the install on the CLI already existing so the no-op path stays bun-free: [ ! -x "$(SKILLS_CLI)" ] && ! (cd "$(SKILLS_PROJECT_DIR)" && bun install ... ) else error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 92:
<comment>`skills-install` now runs `bun install --frozen-lockfile` unconditionally on every invocation, before the all-installed idempotency check that used to let the recipe exit 0 without touching bun. This makes `make sync` and offline/idempotent re-syncs depend on a live bun registry and rewrites node_modules every run; when the registry is unreachable the whole sync now hard-fails even though all skills are already present. Gate the install on the CLI already existing so the no-op path stays bun-free: `[ ! -x "$(SKILLS_CLI)" ] && ! (cd "$(SKILLS_PROJECT_DIR)" && bun install ... )` else error.</comment>
<file context>
@@ -87,6 +89,10 @@ skills-install: ## Install external skills from skills-lock.json (skips already
@lock="$(SKILLS_LOCK_FILE)"; \
skills_dir="$(SKILLS_EXTERNAL_SOURCE_DIR)"; \
force="$${DOTAGENTS_FORCE_SKILLS_INSTALL:-0}"; \
+ if ! (cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null); then \
+ echo "Error: failed to install the skills SDK from bun.lock"; \
+ exit 1; \
</file context>
| if ! (cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null); then \ | |
| echo "Error: failed to install the skills SDK from bun.lock"; \ | |
| exit 1; \ | |
| fi; \ | |
| if [ ! -x "$(SKILLS_CLI)" ]; then \ | |
| if ! (cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null); then \ | |
| echo "Error: failed to install the skills SDK from bun.lock"; \ | |
| exit 1; \ | |
| fi; \ | |
| fi; \ | |
| .PHONY: skills-update | ||
| skills-update: ## Update installed external skills to latest and refresh the lock. | ||
| @bun x skills update --global --yes </dev/null | ||
| @cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null |
There was a problem hiding this comment.
P3: The bun install --frozen-lockfile --minimum-release-age 0 --no-progress command is duplicated verbatim in both skills-install and skills-update. A shared SKILLS_* variable (alongside the other SKILLS_ vars) would keep them in sync and make future flag changes a one-line edit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 140:
<comment>The `bun install --frozen-lockfile --minimum-release-age 0 --no-progress` command is duplicated verbatim in both `skills-install` and `skills-update`. A shared `SKILLS_*` variable (alongside the other SKILLS_ vars) would keep them in sync and make future flag changes a one-line edit.</comment>
<file context>
@@ -131,15 +137,16 @@ skills-refresh: ## Force a reinstall of all external skills from skills-lock.jso
.PHONY: skills-update
skills-update: ## Update installed external skills to latest and refresh the lock.
- @bun x skills update --global --yes </dev/null
+ @cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null
+ @$(SKILLS_CLI) update --global --yes </dev/null
@$(MAKE) skills-lock
</file context>
1 participant
What changed
skillsSDK (^1.5.22, resolved inbun.lock) instead of the stale CLI workaround.skillsexecutable cannot be selected.make skills-installworks from both~/dotfilesand~/dotfiles/dotagents.Verification
./node_modules/.bin/skills --version->1.5.22bun install --frozen-lockfile --minimum-release-age 0 --no-progressmake skills-installfrom~/dotfilesmake -C dotagents skills-installfrom~/dotfilesmake dotagents-syncfrom~/dotfilesSummary by cubic
Switch to the project-local
skillsSDK (^1.5.22, resolved inbun.lock) and CLI to avoid global conflicts and make installs/updates consistent from both~/dotfilesanddotagents/.Bug Fixes
bun installfrom the project directory and usesnode_modules/.bin/skillsinstead of a global binary.skills-installandskills-updatenow work the same from upstream~/dotfilesand nesteddotagents/.Dependencies
skillsfrom^1.5.20to^1.5.22.bun.lockwith new transitive packages (e.g.,tar).Written for commit fa405a0. Summary will update on new commits.