Skip to content

Keep refreshing opencode's other spelling - #627

Merged
jeremy merged 1 commit into
mainfrom
fix/opencode-legacy-skill-refresh
Aug 5, 2026
Merged

Keep refreshing opencode's other spelling#627
jeremy merged 1 commit into
mainfrom
fix/opencode-legacy-skill-refresh

Conversation

@jeremy

@jeremyjeremy commented Aug 5, 2026

Copy link
Copy Markdown
Member

#624's central claim was wrong, and I shipped it. This corrects the record and the regression it introduced.

What #624 said

opencode reads skills/, plural — it has never read either path we wrote.

What opencode actually does

From the shipped 1.18.4 binary's own docs table — not the docs site:

| Global skills | `~/.config/opencode/skill(s)/<name>/SKILL.md` |
| Project skills | `.opencode/skill(s)/<name>/SKILL.md` |
| Project agents | `.opencode/agent/<name>.md` or `.opencode/agents/<name>.md` |

skill(s) — the same optional plural opencode uses for agent(s) and command(s). Both spellings work. The singular paths were fine.

I asserted that negative from a summarizing fetch of https://opencode.ai/docs/skills/, which rendered the parenthetical as plain skills. A negative claim deserved a primary source, and the binary was available the whole time.

The regression

skillLocations is both the wizard's install targets and the refresh set. #624 moved it wholly to plural, so anyone who picked OpenCode in basecamp skillbefore#624 now has a file that:

  • opencode still loads — it works
  • the refresh loop no longer visits — it never updates again

Working and silently frozen at the version that installed it. That is worse than broken, because nothing reports it, and it is the exact failure mode #624 claimed to be fixing.

The fix

Separate the two roles the list was playing:

  • skillLocations — wizard install targets. Unchanged, still plural.
  • legacySkillLocations — paths an agent still reads that we no longer suggest. Refreshed, never offered.

refreshAllInstalledSkills walks both. Refresh updates in place and does not create the new path, so nobody ends up with two copies drifting apart — asserted in the test.

Verification

TestRefreshAllInstalledSkills_LegacyOpenCodePath fails against the pre-fix loop:

-- API coverage: See API-COVERAGE.md in the CLI repo
+old
a pre-#624 opencode install must keep getting refreshed

and passes after it. bin/ci green (EXIT=0).

What from #624 still holds

  • ~/.agents/skills/<name>/SKILL.md is auto-loaded by opencode — confirmed in the same table — so the skill has always worked there regardless of the wizard. That claim was right.
  • Plural install targets are valid, so the new default is fine; this is additive.
  • Pinning path literals in a test rather than deriving them was right, and that test's comment is corrected here too.

#617 gets a correction as well, since I told the requester the wizard was broken for them.


Summary by cubic

Restores refresh for OpenCode skills installed at the singular skill path so pre-#624 installs keep updating. Keeps plural install targets and updates legacy installs in place without creating duplicates.

  • Bug Fixes
    • Added legacySkillLocations for OpenCode skill paths (global and project).
    • refreshAllInstalledSkills now walks both install and legacy paths; no new files are created.
    • Confirmed OpenCode accepts skill(s); added regression test to ensure legacy paths keep refreshing.

Written for commit 6cc509f. Summary will update on new commits.

Review in cubic

#624 claimed opencode never read `~/.config/opencode/skill/basecamp/SKILL.md`.
That was wrong. opencode takes an optional plural throughout — its own
shipped docs table (verified against the 1.18.4 binary, not the docs site)
reads:
| Global skills | `~/.config/opencode/skill(s)/<name>/SKILL.md` |
| Project skills | `.opencode/skill(s)/<name>/SKILL.md` |
the same `agent(s)` / `command(s)` form used for its other directories.
So the singular paths worked, and #624 did not repair a broken install — it
moved the install target from one working path to another. What it did break
is upgrades: anyone who picked OpenCode in `basecamp skill` before #624 has a
file opencode still loads, but which the refresh loop no longer visits,
because that loop iterates the same list. Their skill keeps working and
silently stops updating, which is worse than failing, since nothing reports
it.
The two roles that list plays are now separated. skillLocations stays the
wizard's install targets. legacySkillLocations holds paths an agent still
reads but that we no longer suggest, and refreshAllInstalledSkills walks
both. Refresh updates in place and never creates the new path, so nobody
ends up with two copies.
The regression test fails against the pre-fix loop and passes after it.
I got this wrong in #624 by asserting a negative from a summarized fetch of
the docs page, which rendered `skill(s)` as `skills`. The binary is the
source of truth here.
CopilotAI balanced review requested due to automatic review settings August 5, 2026 19:51
@jeremyjeremy mentioned this pull request Aug 5, 2026
@github-actionsgithub-actionsBot added commands CLI command implementations tests Tests (unit and e2e) labels Aug 5, 2026
CopilotAI reviewed Aug 5, 2026

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jeremy
jeremy merged commit ca07647 into mainAug 5, 2026
24 of 25 checks passed
@jeremy
jeremy deleted the fix/opencode-legacy-skill-refresh branch August 5, 2026 19:54
@jeremy

Copy link
Copy Markdown
MemberAuthor

Correcting the scope claim in this PR, since it is broader than what actually shipped.

The description says the fix covers "anyone who picked OpenCode in the wizard" (and the comment at internal/commands/skill.go:44-46 says the same). Only the global legacy path is actually refreshed.refreshAllInstalledSkills skips any location that is not ~-prefixed or absolute:

// Skip project-relative paths — no reliable project root in PostRunE.if!strings.HasPrefix(loc.Path, "~") &&!filepath.IsAbs(loc.Path) {
continue
}

So of the two rows this PR added, ~/.config/opencode/skill/basecamp/SKILL.md is refreshed and .opencode/skill/basecamp/SKILL.md is inert. Someone whose OpenCode skill lives at the legacy project path is still silently frozen — and, because legacy paths are "refreshed, never suggested," they are not an install target either, so nothing writes that file at all.

That guard is pre-existing and not a regression here. It applies equally to the non-legacy project rows already in skillLocations.claude/skills/basecamp/SKILL.md and .opencode/skills/basecamp/SKILL.md — which are also never auto-refreshed, for the stated reason that PostRunE has no reliable project root. Those two do at least get written by an explicit basecamp skill install.

Whether project-local installs should ever be auto-refreshed is a separate decision — it needs a project-root resolution strategy and a test of its own — so I am not widening this fix. Recording the correction only, so the claim doesn't get read as coverage it doesn't have.

jeremy added a commit that referenced this pull request Aug 22, 2026
…or-roundtrip
* origin/main: (96 commits)
ci: bump the github-actions group with 6 updates (#639)
Reject three more doomed invocations before draining stdin (#645)
Stdin `-` support everywhere sensible; usage error for stray `-` elsewhere (#641)
Add hey-cli Windows signing secrets to the release env manifest (#642)
deps: bump the go-dependencies group with 5 updates (#638)
Update nix flake and plugin version for v0.9.1
ci: bump the github-actions group with 4 updates (#633)
Add basecamp files replace: publish a new version of an uploaded file (#634)
Add basecamp files versions — HELD, blocked on the SDK (#622)
Update nix flake and plugin version for v0.9.0
Make the Codex probe's timeout actually bound doctor (#629)
Make the lockstep check catch stale agreement and .yaml workflows (#628)
Keep refreshing opencode's other spelling (#627)
Lint the release the same way we lint everything else (#625)
Install the skill where opencode actually looks (#624)
Take the communiques out of the source tree (#623)
Correct the API coverage claim: 183/184, not 100% (#621)
Stop echoing back step fields the caller never changed (#620)
Drive the circuit breaker's clock from tests, not sleep() (#619)
Tell agents the truth about card column moves (#618)
...
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commandsCLI command implementationstestsTests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jeremy