Skip to content

Refresh skills at all installed locations on CLI upgrade#356

Open
jeremy wants to merge 4 commits intomainfrom
skill-auto-refresh
Open

Refresh skills at all installed locations on CLI upgrade#356
jeremy wants to merge 4 commits intomainfrom
skill-auto-refresh

Conversation

@jeremy
Copy link
Member

@jeremy jeremy commented Mar 23, 2026

Summary

  • RefreshSkillsIfVersionChanged() now updates the skill file at every known install location (Claude Code, OpenCode, Codex) — not just the baseline ~/.agents/ directory
  • Broken Claude symlinks at ~/.claude/skills/basecamp are detected and repaired automatically
  • Project-relative paths are skipped since there is no reliable project root in the post-run hook

Previously, only ~/.agents/skills/basecamp/SKILL.md was refreshed on upgrade. Users who installed the skill to ~/.claude/skills/basecamp/ (as a copy, not symlink), ~/.config/opencode/skill/basecamp/, or ~/.codex/skills/basecamp/ would not get updates until they manually re-ran basecamp skill install.

Test plan

  • TestRefreshAllInstalledSkills_MultipleLocations — installs to 3 locations, all get refreshed
  • TestRefreshAllInstalledSkills_SkipsAbsentLocations — doesn't create files at uninstalled locations
  • TestRefreshAllInstalledSkills_SkipsProjectRelativePaths — doesn't touch project-relative skills
  • TestRepairClaudeSkillLink_BrokenSymlink — broken symlink gets repaired
  • TestRepairClaudeSkillLink_HealthySymlink — healthy symlink is left alone

Summary by cubic

Refreshes the Basecamp skill on CLI upgrade across all installed locations (Claude Code, OpenCode, Codex), not just ~/.agents. Repairs broken Claude symlinks, skips project-relative installs, and handles missing vs. unreadable paths correctly.

  • New Features
    • RefreshSkillsIfVersionChanged() updates SKILL.md at all detected global install paths; skips absent and project-relative paths, and treats permission/IO errors as failures.
    • Repairs a broken ~/.claude/skills/basecamp symlink when Claude is present; leaves healthy links and directory copies untouched.
    • Stamps the installed version in the baseline dir only when all refreshes succeed.
    • Updates the sentinel only when no refresh is needed or the refresh succeeds, so transient failures retry.

Written for commit 5e7dbd9. Summary will update on new commits.

RefreshSkillsIfVersionChanged now updates the skill file at every
known install location (Claude Code, OpenCode, Codex) — not just
the baseline ~/.agents/ directory. Broken Claude symlinks are
repaired automatically. Project-relative paths are skipped since
there is no reliable project root in the post-run hook.
Copilot AI review requested due to automatic review settings March 23, 2026 20:52
@github-actions github-actions bot added commands CLI command implementations tests Tests (unit and e2e) labels Mar 23, 2026
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 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="internal/commands/skill.go">

<violation number="1" location="internal/commands/skill.go:358">
P1: Version sentinel can be advanced after only a partial multi-location refresh, which suppresses retry for failed installed locations.

(Based on your team's feedback about only updating the sentinel after a successful refresh so retries still happen on failure.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2c5b258a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the CLI upgrade hook so embedded skill content is refreshed across all previously-installed global locations (Agents baseline, Claude, OpenCode, Codex), and adds automatic repair for broken Claude skill symlinks.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

  • Replace baseline-only refresh logic with refreshAllInstalledSkills() to overwrite SKILL.md at each known installed global location.
  • Add repairClaudeSkillLink() and invoke it during version-change refresh when Claude is detected.
  • Add unit tests covering multi-location refresh behavior, absent locations, project-relative skipping, and Claude symlink repair.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/commands/skill.go Refresh installed skill files across known locations on version change; repair broken Claude symlink.
internal/commands/skill_test.go Add tests validating multi-location refresh and Claude symlink repair behavior.
Comments suppressed due to low confidence (1)

internal/commands/skill.go:349

  • refreshAllInstalledSkills() returns true when any location was successfully written, but RefreshSkillsIfVersionChanged() uses that boolean to decide whether refreshing “succeeded” and it’s safe to update the version sentinel. This can incorrectly advance the sentinel on partial failure (e.g. baseline SKILL.md write fails due to permissions, but another location succeeds), preventing retries and leaving baseline stale. Consider having refreshAllInstalledSkills report per-location failures (or an allSucceeded flag), and gate the sentinel update on baseline refresh success (and/or all attempted locations succeeding) rather than “any file updated”.
	refreshed := refreshAllInstalledSkills()

	// Repair Claude symlink if broken (e.g. baseline dir was recreated)
	if harness.DetectClaude() {
		repairClaudeSkillLink()
	}

	// Update sentinel only when no refresh was needed or it succeeded.
	// On transient failure, leave the sentinel stale so the next run retries.
	needsRefresh := baselineSkillInstalled()
	if !needsRefresh || refreshed {
		_ = os.MkdirAll(filepath.Dir(sentinelPath), 0o755)             //nolint:gosec // G301: config dir
		_ = os.WriteFile(sentinelPath, []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added the enhancement New feature or request label Mar 23, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2287872c2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

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="internal/commands/skill_test.go">

<violation number="1" location="internal/commands/skill_test.go:553">
P2: Do not ignore `os.Getwd()` errors when changing global working directory in a test; failed CWD capture can leave the process in the wrong directory.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copilot AI review requested due to automatic review settings March 23, 2026 23:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

internal/commands/skill.go:349

  • The sentinel update logic only checks baselineSkillInstalled(). If the user has the skill installed only in a non-baseline location (e.g. OpenCode/Codex) and refreshAllInstalledSkills() fails (or partially fails), needsRefresh will be false and the code will still write the version sentinel, preventing future retries despite the refresh failing. Consider basing the sentinel decision on whether any known install location was present and refreshed successfully (e.g. have refreshAllInstalledSkills return attempted/success, or treat updated==0 && failed==0 as “no refresh needed”, but keep the sentinel stale when failed>0).
	// Update sentinel only when no refresh was needed or it succeeded.
	// On transient failure, leave the sentinel stale so the next run retries.
	needsRefresh := baselineSkillInstalled()
	if !needsRefresh || refreshed {
		_ = os.MkdirAll(filepath.Dir(sentinelPath), 0o755)             //nolint:gosec // G301: config dir
		_ = os.WriteFile(sentinelPath, []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added bug Something isn't working and removed enhancement New feature or request labels Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working commands CLI command implementations tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants