fix: honest command descriptions for discovered vs bundled skills#594
Conversation
…ills Discovered (user/project) skills registered as commands were labelled "(Systematic - Skill) ...", falsely implying Systematic authored them. They are the user's own skills — drop the prefix entirely and use the skill's own description. Bundled Systematic skills-as-commands lose the "- Skill" qualifier too, so their prefix reads "(Systematic) " — matching bundled plain commands. isSystematicCommandConfig now recognizes only "(Systematic) ". The retired "(Systematic - Skill) " label needs no migration path: OpenCode applies config-hook mutations in memory per load and never persists them, so a previously-injected entry can never reappear as existing config. Discovered commands are consequently unmarked — add-if-absent, not refreshed or cleaned across reloads, which is fine since every launch rebuilds config from disk and simply omits deleted skills.
fro-bot
left a comment
fro-bot
left a comment
There was a problem hiding this comment.
Verified the diff directly (checked out pr-594, ran typecheck/lint/full test suite) against the stated intent: discovered (user/project) skills should show their own honest description with no Systematic attribution, while bundled skills/commands share one (Systematic) marker.
Verdict: PASS
Blocking issues
None
Non-blocking concerns
loadDiscoveredSkillAsCommandinlinesskill.description || \${skill.name} skill`instead of reusingformatSkillDescription's fallback logic — minor duplication of the "empty description →skill`" fallback across two files. Acceptable since the two functions now have genuinely different behavior (prefix vs. no prefix), but worth a shared helper if a third consumer appears.- The "add-if-absent, never refreshed/removed" behavior for discovered-skill commands (now unmarked) is a real semantic change from the prior marker-based cleanup, but the reasoning in the code comment and PR description is sound: OpenCode never persists config-hook mutations back to disk, so there's no stale-entry risk to clean up. Confirmed via the updated test (
discovered-skill command persists after its skill is deleted from disk) that this is intentional and tested, not an oversight. isSystematicCommandConfigdropping the retired(Systematic - Skill)string means any command literally carrying that legacy string in a user's own on-disk config (extremely unlikely, but theoretically possible if a user copied an old emitted value into their config) would no longer be recognized/refreshed as Systematic-owned and would need manual removal. Low likelihood, no migration needed per the PR's own analysis, and matches the "user's own command wins" principle already in place.
Missing tests
None — the diff includes a direct test locking honest attribution (no (Systematic substring) for discovered skills, updates all previously-hardcoded (Systematic - Skill) expectations to (Systematic), and flips the "orphaned command removed" test to "persists" with a comment explaining why that's correct.
Risk assessment: LOW
Small, self-contained change (two source files, both existing exports/behavior, no new public API surface). No security implications — the change only affects a description string that is either the user's own skill frontmatter (already trusted, sourced from disk they control) or a static bundled prefix constant; no template/execution path changes. Verified: bun run typecheck clean, Biome clean, zero remaining (Systematic - Skill) references repo-wide, and the full suite (1123 tests) passes. Blast radius is limited to command descriptions shown in OpenCode's command palette — cosmetic/trust-labeling only, not functional behavior of skill execution.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | marcusrbrown/systematic |
| Run ID | 28859810984 |
| Cache | hit |
| Session | ses_0c3d9b79bffeogeWdZ6nsNtUDn |
2 participants
Discovered (user/project) skills registered as slash commands were labelled
(Systematic - Skill) ...in their description — falsely implying Systematic authored them. They are the user's own skills. This drops the prefix from discovered-skill commands entirely (they now show their own description) and shortens the bundled-skill prefix to(Systematic), matching bundled plain commands.Changes
loadDiscoveredSkillAsCommanduses the skill's own frontmatter description (fallback<name> skill), no Systematic prefix.(Systematic):SKILL_DESCRIPTION_PREFIXdrops the- Skillqualifier.isSystematicCommandConfigrecognizes only(Systematic). The retired(Systematic - Skill)label needs no migration branch: OpenCode applies config-hook mutations in memory per load and never persists them (verified against v1.17.6 source — the hook result is discarded, config is rebuilt from disk every launch), so a previously-injected entry can never reappear as existing config.Consequence
Discovered-skill commands are now unmarked, so the merge treats them as add-if-absent and no longer refreshes or removes them across reloads. This is fine: every launch rebuilds config from disk and simply omits deleted skills, and the prior-art plugins this feature is modelled on don't do marker-based cleanup either. A user's own same-named command still wins.
Verification
(Systematic), discovered → raw description, the removal test flipped to assert persistence, a new test locking honest attribution).bun run typecheck, content-integrity, and lint all clean. Zero(Systematic - Skill)remain anywhere in the repo.