feat: add skill CLI command and .opencode/tools/ auto-discovery#342
Merged
anandgupta42 merged 33 commits intomainfrom Mar 22, 2026
Merged
feat: add skill CLI command and .opencode/tools/ auto-discovery#342anandgupta42 merged 33 commits intomainfrom
skill CLI command and .opencode/tools/ auto-discovery#342anandgupta42 merged 33 commits intomainfrom
Conversation
…341) Add a top-level `altimate-code skill` command with `list`, `create`, and `test` subcommands, plus auto-discovery of user CLI tools on PATH. - `skill list` — shows skills with paired CLI tools, source, description - `skill create <name>` — scaffolds SKILL.md + CLI tool stub (bash/python/node) - `skill test <name>` — validates frontmatter, checks tool on PATH, runs `--help` - Auto-prepend `.opencode/tools/` (project) and `~/.config/altimate-code/tools/` (global) to PATH in `BashTool` and PTY sessions - Worktree-aware: `skill create` uses git root, PATH includes worktree tools - Input validation: regex + length limits, 5s timeout on tool `--help` - 14 unit tests (42 assertions) covering tool detection, templates, adversarial inputs - Updated docs: skills.md, tools/custom.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a top-level Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as altimate-code CLI
participant FS as Filesystem (.opencode/...)
participant Global as Global config (~/.config/...)
participant PTY
participant Tool as Executable
User->>CLI: run `altimate-code skill create <name>`
CLI->>FS: create `.opencode/skills/<name>/SKILL.md`
CLI->>FS: create `.opencode/tools/<name>` (stub) and set +x
CLI-->>User: success
User->>CLI: run `altimate-code skill test <name>`
CLI->>FS: read SKILL.md, detect tool refs
CLI->>FS: check project/worktree `.opencode/tools/<tool>`
CLI->>Global: check global tools dir `/.../config/tools/<tool>`
CLI->>PTY: spawn `<tool> --help` with PATH prefixed (ALTIMATE_BIN_DIR, project, worktree, global)
PTY->>Tool: executes
Tool-->>PTY: exit code / stdout/stderr
PTY-->>CLI: result
CLI-->>User: pass/warn/fail report
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/opencode/src/cli/cmd/skill.ts (1)
355-362: Avoidprocess.exit()inside the command handler.
packages/opencode/src/index.tswrapsawait cli.parse()in afinallyblock to flush telemetry, andbootstrap()also relies on returning from the callback so it can dispose the instance. Callingprocess.exit()here bypasses both paths on invalid names and existing skills. Prefer throwing a command error, or setprocess.exitCodeandreturn.Also applies to: 374-377
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/skill.ts` around lines 355 - 362, Replace direct process.exit() calls in the name validation blocks with non-exiting control flow so the CLI wrapper can run its finally/cleanup logic: when the regex check (the block referencing variable name) or the length check (and the similar checks around 374-377) fail, write the error to stderr as you do, then either set process.exitCode = 1 and return from the command handler, or throw a command Error (so the caller can handle/flush telemetry). Update both validation sites (the failing regex check and the length check, plus the analogous block at 374-377) to avoid calling process.exit() and instead return or throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/configure/skills.md`:
- Around line 174-179: Update the docs so both the "Skill Paths" and the earlier
"Discovery Paths" sections list the same project and global locations: use
`.opencode/skills/` and `.altimate-code/skills/` (project, highest priority) and
`~/.altimate-code/skills/` (global). Also ensure any examples or CLI references
(e.g., `skill create`) mention the same project path names and pluralization
(`skills`) so the page no longer has conflicting authoritative lists.
In `@packages/opencode/src/cli/cmd/skill.ts`:
- Around line 489-520: The current tool-check block (using Instance.worktree,
toolEnv, Bun.spawn, proc, timeout, exitCode) only calls warn() for spawn
failures, timeouts, and non-zero exits so hasErrors remains false and overall
result stays PASS; change the handlers so failures set the shared error state
(e.g., set hasErrors = true) or call the existing failure-reporting function
(fail(...) if present) instead of warn(), and in the catch block mark the run as
failed as well; apply the same change to the equivalent handling at lines around
529–533 so any non-zero exit, timeout (exitCode === null || 137 || 143), or
spawn/catch error causes the command to record an error and ultimately produce a
non-zero overall status.
- Around line 87-115: isToolOnPath() builds its PATH from process.env directly,
but runtime execution first merges Plugin.trigger("shell.env", …) and prepends
.opencode tool dirs (see packages/opencode/src/tool/bash.ts and
packages/opencode/src/pty/index.ts), so tools exposed via shell.env are falsely
reported missing; fix by reusing the shared env/path builder used by runtime
(extract or import the function that merges Plugin.trigger("shell.env", …) and
constructs the effective PATH—e.g., getEffectiveShellEnv or buildShellPath—and
replace the process.env-based PATH logic in isToolOnPath() so it reads the same
PATH dirs (including ALTIMATE_BIN_DIR and .opencode tool directories) before
checking fs.access).
In `@packages/opencode/src/tool/bash.ts`:
- Around line 186-193: The project-scoped auto-discovered tool path is currently
derived from cwd (projectToolsDir = path.join(cwd, ".opencode", "tools")), which
allows params.workdir to point outside the project and shadow toolchains; change
the project-scoped entry to use Instance.directory (e.g.,
path.join(Instance.directory, ".opencode", "tools")) and only use
Instance.worktree as the optional fallback (keeping the existing guard that
skips duplicate entries). Apply the same change to the mirrored block in
packages/opencode/src/pty/index.ts so both use Instance.directory for the
project anchor and Instance.worktree only as a secondary git-root fallback.
In `@packages/opencode/test/cli/skill.test.ts`:
- Around line 124-133: Replace manual temp dir management in the test suite
(tmpDir, beforeAll, afterAll) with the repository fixture tmpdir helper: import
tmpdir from fixture/fixture.ts, then in the suite use "await using tmp =
tmpdir()" and read the directory via "tmp.path" (ensure you realpath-resolve the
path where used). Remove fs.mkdtemp/fs.rm hooks and update any references from
tmpDir to tmp.path so cleanup is automatic; apply the same change in the other
suite mentioned around lines 279-294.
- Around line 13-53: The test contains a drifted copy of the parser: update
tests to import and exercise the real detectToolReferences implementation
(instead of the local copy) by exporting detectToolReferences and SHELL_BUILTINS
from the production module and changing the test to call that exported function;
ensure the production SHELL_BUILTINS includes the extra system-utility filters
(so reference the unique symbol SHELL_BUILTINS) and update the regexes in
detectToolReferences to accept Windows line endings (use /\r?\n/ for fences) and
to filter out backtick "Tools used:" entries (the Tools used: parsing is already
present—use its match and apply SHELL_BUILTINS filtering) so the test validates
the actual implementation rather than a diverged inline copy.
- Around line 135-141: Update the test to exercise the real production env
builder instead of manually mutating PATH: import the package entrypoint that
registers commands (packages/opencode/src/index.ts) and call its exported
production environment builder (e.g., createProductionEnv / buildProductionEnv)
to produce the env used by the CLI, then spawn the actual CLI entrypoint
(src/cli/cmd/skill.ts) with that env and assert that env.PATH contains
".opencode/tools" and that the bash tool from packages/opencode/src/tool/bash.ts
(and PATH-prepending behavior in packages/opencode/src/pty/index.ts) is visible
and that SkillCommand is registered/available (check the command registry or the
exported SkillCommand) instead of stubbing PATH in the test.
---
Nitpick comments:
In `@packages/opencode/src/cli/cmd/skill.ts`:
- Around line 355-362: Replace direct process.exit() calls in the name
validation blocks with non-exiting control flow so the CLI wrapper can run its
finally/cleanup logic: when the regex check (the block referencing variable
name) or the length check (and the similar checks around 374-377) fail, write
the error to stderr as you do, then either set process.exitCode = 1 and return
from the command handler, or throw a command Error (so the caller can
handle/flush telemetry). Update both validation sites (the failing regex check
and the length check, plus the analogous block at 374-377) to avoid calling
process.exit() and instead return or throw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8d4a7faa-443a-4fff-bc3c-194f10eb9278
📒 Files selected for processing (7)
docs/docs/configure/skills.mddocs/docs/configure/tools/custom.mdpackages/opencode/src/cli/cmd/skill.tspackages/opencode/src/index.tspackages/opencode/src/pty/index.tspackages/opencode/src/tool/bash.tspackages/opencode/test/cli/skill.test.ts
Comment on lines
+87
to
+115
| async function isToolOnPath(toolName: string, cwd: string): Promise<boolean> { | ||
| // Check .opencode/tools/ in both cwd and worktree (they may differ in monorepos) | ||
| const dirsToCheck = new Set([ | ||
| path.join(cwd, ".opencode", "tools"), | ||
| path.join(Instance.worktree !== "/" ? Instance.worktree : cwd, ".opencode", "tools"), | ||
| path.join(Global.Path.config, "tools"), | ||
| ]) | ||
|
|
||
| for (const dir of dirsToCheck) { | ||
| try { | ||
| await fs.access(path.join(dir, toolName), fs.constants.X_OK) | ||
| return true | ||
| } catch {} | ||
| } | ||
|
|
||
| // Check system PATH | ||
| const sep = process.platform === "win32" ? ";" : ":" | ||
| const binDir = process.env.ALTIMATE_BIN_DIR | ||
| const pathDirs = (process.env.PATH ?? "").split(sep).filter(Boolean) | ||
| if (binDir) pathDirs.unshift(binDir) | ||
|
|
||
| for (const dir of pathDirs) { | ||
| try { | ||
| await fs.access(path.join(dir, toolName), fs.constants.X_OK) | ||
| return true | ||
| } catch {} | ||
| } | ||
|
|
||
| return false |
There was a problem hiding this comment.
skill list/test are not using the same PATH resolution as runtime execution.
isToolOnPath() and the --help spawn build PATH from process.env, but both packages/opencode/src/tool/bash.ts and packages/opencode/src/pty/index.ts first merge Plugin.trigger("shell.env", …) before prepending tool directories. Any tool exposed through a shell.env hook will be reported as missing here even though the agent can run it. Please reuse the same env/path builder across these code paths.
Also applies to: 491-508
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/skill.ts` around lines 87 - 115, isToolOnPath()
builds its PATH from process.env directly, but runtime execution first merges
Plugin.trigger("shell.env", …) and prepends .opencode tool dirs (see
packages/opencode/src/tool/bash.ts and packages/opencode/src/pty/index.ts), so
tools exposed via shell.env are falsely reported missing; fix by reusing the
shared env/path builder used by runtime (extract or import the function that
merges Plugin.trigger("shell.env", …) and constructs the effective PATH—e.g.,
getEffectiveShellEnv or buildShellPath—and replace the process.env-based PATH
logic in isToolOnPath() so it reads the same PATH dirs (including
ALTIMATE_BIN_DIR and .opencode tool directories) before checking fs.access).
packages/opencode/src/tool/bash.ts
Outdated
Comment on lines
+186
to
+193
| const projectToolsDir = path.join(cwd, ".opencode", "tools") | ||
| if (!pathEntries.has(projectToolsDir)) { | ||
| prependDirs.push(projectToolsDir) | ||
| } | ||
| if (Instance.worktree !== "/") { | ||
| const worktreeToolsDir = path.join(Instance.worktree, ".opencode", "tools") | ||
| if (worktreeToolsDir !== projectToolsDir && !pathEntries.has(worktreeToolsDir)) { | ||
| prependDirs.push(worktreeToolsDir) |
There was a problem hiding this comment.
Don't derive auto-discovered tool paths from the command workdir.
params.workdir can point outside the project after external_directory approval. Prepending path.join(cwd, ".opencode", "tools") lets an arbitrary external .opencode/tools directory shadow the project/global toolchain for the rest of the command. Please anchor the project-scoped entry to Instance.directory and keep Instance.worktree as the optional git-root fallback; the mirrored block in packages/opencode/src/pty/index.ts needs the same constraint.
Suggested direction
- const projectToolsDir = path.join(cwd, ".opencode", "tools")
+ const projectToolsDir = path.join(Instance.directory, ".opencode", "tools")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const projectToolsDir = path.join(cwd, ".opencode", "tools") | |
| if (!pathEntries.has(projectToolsDir)) { | |
| prependDirs.push(projectToolsDir) | |
| } | |
| if (Instance.worktree !== "/") { | |
| const worktreeToolsDir = path.join(Instance.worktree, ".opencode", "tools") | |
| if (worktreeToolsDir !== projectToolsDir && !pathEntries.has(worktreeToolsDir)) { | |
| prependDirs.push(worktreeToolsDir) | |
| const projectToolsDir = path.join(Instance.directory, ".opencode", "tools") | |
| if (!pathEntries.has(projectToolsDir)) { | |
| prependDirs.push(projectToolsDir) | |
| } | |
| if (Instance.worktree !== "/") { | |
| const worktreeToolsDir = path.join(Instance.worktree, ".opencode", "tools") | |
| if (worktreeToolsDir !== projectToolsDir && !pathEntries.has(worktreeToolsDir)) { | |
| prependDirs.push(worktreeToolsDir) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/tool/bash.ts` around lines 186 - 193, The
project-scoped auto-discovered tool path is currently derived from cwd
(projectToolsDir = path.join(cwd, ".opencode", "tools")), which allows
params.workdir to point outside the project and shadow toolchains; change the
project-scoped entry to use Instance.directory (e.g.,
path.join(Instance.directory, ".opencode", "tools")) and only use
Instance.worktree as the optional fallback (keeping the existing guard that
skips duplicate entries). Apply the same change to the mirrored block in
packages/opencode/src/pty/index.ts so both use Instance.directory for the
project anchor and Instance.worktree only as a secondary git-root fallback.
Comment on lines
+13
to
+53
| /** Shell builtins to filter — mirrors SHELL_BUILTINS in skill.ts */ | ||
| const SHELL_BUILTINS = new Set([ | ||
| "echo", "cd", "export", "set", "if", "then", "else", "fi", "for", "do", "done", | ||
| "case", "esac", "printf", "source", "alias", "read", "local", "return", "exit", | ||
| "break", "continue", "shift", "trap", "type", "command", "builtin", "eval", "exec", | ||
| "test", "true", "false", | ||
| "cat", "grep", "awk", "sed", "rm", "cp", "mv", "mkdir", "ls", "chmod", "which", | ||
| "curl", "wget", "pwd", "touch", "head", "tail", "sort", "uniq", "wc", "tee", | ||
| "xargs", "find", "tar", "gzip", "unzip", "git", "npm", "yarn", "bun", "pip", | ||
| "python", "python3", "node", "bash", "sh", "zsh", "docker", "make", | ||
| "glob", "write", "edit", | ||
| ]) | ||
|
|
||
| /** Detect CLI tool references inside a skill's content. */ | ||
| function detectToolReferences(content: string): string[] { | ||
| const tools = new Set<string>() | ||
|
|
||
| const toolsUsedMatch = content.match(/Tools used:\s*(.+)/i) | ||
| if (toolsUsedMatch) { | ||
| const refs = toolsUsedMatch[1].matchAll(/`([a-z][\w-]*)`/gi) | ||
| for (const m of refs) tools.add(m[1]) | ||
| } | ||
|
|
||
| const bashBlocks = content.matchAll(/```(?:bash|sh)\n([\s\S]*?)```/g) | ||
| for (const block of bashBlocks) { | ||
| const lines = block[1].split("\n") | ||
| for (const line of lines) { | ||
| const trimmed = line.trim() | ||
| if (!trimmed || trimmed.startsWith("#")) continue | ||
| const cmdMatch = trimmed.match(/^(?:\$\s+)?([a-z][\w.-]*(?:-[\w]+)*)/i) | ||
| if (cmdMatch) { | ||
| const cmd = cmdMatch[1] | ||
| if (!SHELL_BUILTINS.has(cmd)) { | ||
| tools.add(cmd) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return Array.from(tools) | ||
| } |
There was a problem hiding this comment.
This copied parser has already drifted from skill.ts.
The test copy omits the extra system-utility filters in SHELL_BUILTINS, it adds Tools used: references without filtering them, and it only matches \n fences instead of \r?\n. That means this suite can pass while packages/opencode/src/cli/cmd/skill.ts still mis-detects builtins or Windows-formatted skills. Please export the helper into a pure module and test the production implementation directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/test/cli/skill.test.ts` around lines 13 - 53, The test
contains a drifted copy of the parser: update tests to import and exercise the
real detectToolReferences implementation (instead of the local copy) by
exporting detectToolReferences and SHELL_BUILTINS from the production module and
changing the test to call that exported function; ensure the production
SHELL_BUILTINS includes the extra system-utility filters (so reference the
unique symbol SHELL_BUILTINS) and update the regexes in detectToolReferences to
accept Windows line endings (use /\r?\n/ for fences) and to filter out backtick
"Tools used:" entries (the Tools used: parsing is already present—use its match
and apply SHELL_BUILTINS filtering) so the test validates the actual
implementation rather than a diverged inline copy.
Comment on lines
+135
to
+141
| test("creates skill and bash tool", async () => { | ||
| const result = Bun.spawnSync(["bun", "run", "src/cli/cmd/skill.ts", "--help"], { | ||
| cwd: path.join(import.meta.dir, "../../"), | ||
| }) | ||
| // Just verify the module parses without errors | ||
| // Full CLI integration requires bootstrap which needs a git repo | ||
| }) |
There was a problem hiding this comment.
These tests don't exercise the new CLI/PATH wiring.
The "creates skill and bash tool" case only verifies that src/cli/cmd/skill.ts --help parses, and the PATH case manually injects .opencode/tools into env.PATH. Both can pass even if packages/opencode/src/index.ts stops registering SkillCommand or if the new PATH-prepending logic in packages/opencode/src/tool/bash.ts / packages/opencode/src/pty/index.ts regresses. Please drive the real entrypoint and assert behavior through the production env builder instead of restating the expected environment in the test.
Also applies to: 303-310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/test/cli/skill.test.ts` around lines 135 - 141, Update the
test to exercise the real production env builder instead of manually mutating
PATH: import the package entrypoint that registers commands
(packages/opencode/src/index.ts) and call its exported production environment
builder (e.g., createProductionEnv / buildProductionEnv) to produce the env used
by the CLI, then spawn the actual CLI entrypoint (src/cli/cmd/skill.ts) with
that env and assert that env.PATH contains ".opencode/tools" and that the bash
tool from packages/opencode/src/tool/bash.ts (and PATH-prepending behavior in
packages/opencode/src/pty/index.ts) is visible and that SkillCommand is
registered/available (check the command registry or the exported SkillCommand)
instead of stubbing PATH in the test.
- Extract `detectToolReferences`, `SHELL_BUILTINS`, `skillSource`, `isToolOnPath` into `skill-helpers.ts` so tests import production code instead of duplicating it - Anchor PATH tool dirs to `Instance.directory` (not `cwd`) to prevent external_directory workdirs from shadowing project tools - Change `skill test` to FAIL (not warn) on broken paired tools: timeouts, non-zero exits, and spawn failures now set `hasErrors` - Add `.opencode/skills/` to the Discovery Paths doc section for consistency with `skill create` and the Skill Paths section - Use `tmpdir()` fixture from `test/fixture/fixture.ts` in tests instead of manual `fs.mkdtemp` + `afterAll` cleanup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/src/cli/cmd/skill-helpers.ts (1)
78-106: Share one tool-resolution order with the execution path builder.
isToolOnPath()is manually reconstructing the search order thatpackages/opencode/src/tool/bash.tsandpackages/opencode/src/pty/index.tsuse, but it already differs (cwd/.opencode/toolshere vsInstance.directory/.opencode/toolsthere). That makesskill list/skill testprone to disagreeing with the environment that actually executes commands. Extract the directory-resolution logic into a shared helper so availability checks and execution stay aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/skill-helpers.ts` around lines 78 - 106, isToolOnPath currently duplicates directory-resolution logic and conflicts with the execution path used by packages/opencode/src/tool/bash.ts and packages/opencode/src/pty/index.ts (note differing use of cwd vs Instance.directory/worktree); extract the directory-resolution into a single shared helper (e.g., resolveToolSearchDirs or getToolSearchPaths) that returns the ordered array/set of directories (including Instance.worktree/Instance.directory handling, cwd, Global.Path.config, ALTITUDE_BIN_DIR logic and PATH split), replace the loop in isToolOnPath to use that helper, and update bash.ts and pty/index.ts to call the same helper so availability checks and command execution use an identical search order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/configure/skills.md`:
- Around line 130-133: The fenced code block containing the two paths
(".opencode/tools/ # Project-level tools (auto-discovered" and
"~/.config/altimate-code/tools/ # Global tools (shared across projects)") needs
an explicit language to satisfy markdownlint; change the opening fence from ```
to ```text (or another appropriate language) so the block is annotated (e.g.,
use ```text) and save the change in the docs/docs/configure/skills.md file where
that fenced block appears.
In `@packages/opencode/src/tool/bash.ts`:
- Around line 169-207: The current logic builds prependDirs and inserts them at
the front of mergedEnv.PATH (via mergedEnv.PATH = `${prefix}${sep}${basePath}`),
which lets repo/global .opencode/tools shadow system binaries; change this to
append these tool dirs instead (i.e., merge as `${basePath}${sep}${suffix}` or
add to the end when basePath is empty) so system PATH entries retain priority.
Additionally, keep the existing high-priority behavior for true custom tool
resolution: detect when the invoked command was identified as a discovered
custom tool (use the code path that resolves tool discovery or introduce a small
predicate around where you set mergedEnv — e.g., check a flag like
isDiscoveredTool or match the executable name against known tool filenames) and
only prepend prependDirs in that case; otherwise append global/project/binDir
entries. Update the block that sets mergedEnv.PATH (and references binDir,
projectToolsDir, worktreeToolsDir, globalToolsDir, prependDirs, basePath, sep,
mergedEnv) accordingly.
---
Nitpick comments:
In `@packages/opencode/src/cli/cmd/skill-helpers.ts`:
- Around line 78-106: isToolOnPath currently duplicates directory-resolution
logic and conflicts with the execution path used by
packages/opencode/src/tool/bash.ts and packages/opencode/src/pty/index.ts (note
differing use of cwd vs Instance.directory/worktree); extract the
directory-resolution into a single shared helper (e.g., resolveToolSearchDirs or
getToolSearchPaths) that returns the ordered array/set of directories (including
Instance.worktree/Instance.directory handling, cwd, Global.Path.config,
ALTITUDE_BIN_DIR logic and PATH split), replace the loop in isToolOnPath to use
that helper, and update bash.ts and pty/index.ts to call the same helper so
availability checks and command execution use an identical search order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cfa7ec04-0759-451b-a1d7-ba74cc841e27
📒 Files selected for processing (6)
docs/docs/configure/skills.mdpackages/opencode/src/cli/cmd/skill-helpers.tspackages/opencode/src/cli/cmd/skill.tspackages/opencode/src/pty/index.tspackages/opencode/src/tool/bash.tspackages/opencode/test/cli/skill.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/opencode/src/pty/index.ts
- packages/opencode/src/cli/cmd/skill.ts
Comment on lines
+130
to
+133
| ``` | ||
| .opencode/tools/ # Project-level tools (auto-discovered) | ||
| ~/.config/altimate-code/tools/ # Global tools (shared across projects) | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced block.
markdownlint is already flagging this fence. Mark it as text (or the appropriate language) so the docs stay lint-clean.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/docs/configure/skills.md` around lines 130 - 133, The fenced code block
containing the two paths (".opencode/tools/ # Project-level tools
(auto-discovered" and "~/.config/altimate-code/tools/ # Global tools (shared
across projects)") needs an explicit language to satisfy markdownlint; change
the opening fence from ``` to ```text (or another appropriate language) so the
block is annotated (e.g., use ```text) and save the change in the
docs/docs/configure/skills.md file where that fenced block appears.
Comment on lines
+169
to
+207
| // altimate_change start — prepend bundled tools dir (ALTIMATE_BIN_DIR) and user tools dirs to PATH | ||
| const mergedEnv: Record<string, string | undefined> = { ...process.env, ...shellEnv.env } | ||
| const sep = process.platform === "win32" ? ";" : ":" | ||
| const basePath = mergedEnv.PATH ?? mergedEnv.Path ?? "" | ||
| const pathEntries = new Set(basePath.split(sep).filter(Boolean)) | ||
|
|
||
| // Collect directories to prepend (highest priority first) | ||
| const prependDirs: string[] = [] | ||
|
|
||
| // 1. Bundled tools (altimate-dbt, etc.) — highest priority | ||
| const binDir = process.env.ALTIMATE_BIN_DIR | ||
| if (binDir) { | ||
| const sep = process.platform === "win32" ? ";" : ":" | ||
| const basePath = mergedEnv.PATH ?? mergedEnv.Path ?? "" | ||
| const pathEntries = basePath.split(sep).filter(Boolean) | ||
| if (!pathEntries.some((entry) => entry === binDir)) { | ||
| mergedEnv.PATH = basePath ? `${binDir}${sep}${basePath}` : binDir | ||
| if (binDir && !pathEntries.has(binDir)) { | ||
| prependDirs.push(binDir) | ||
| } | ||
|
|
||
| // 2. Project-level user tools (.opencode/tools/) — user extensions | ||
| // Anchored to Instance.directory (not cwd) so external_directory workdirs | ||
| // can't shadow project tools. Also check worktree root for monorepos. | ||
| const projectToolsDir = path.join(Instance.directory, ".opencode", "tools") | ||
| if (!pathEntries.has(projectToolsDir)) { | ||
| prependDirs.push(projectToolsDir) | ||
| } | ||
| if (Instance.worktree !== "/" && Instance.worktree !== Instance.directory) { | ||
| const worktreeToolsDir = path.join(Instance.worktree, ".opencode", "tools") | ||
| if (!pathEntries.has(worktreeToolsDir)) { | ||
| prependDirs.push(worktreeToolsDir) | ||
| } | ||
| } | ||
|
|
||
| // 3. Global user tools (~/.config/altimate-code/tools/) — shared across projects | ||
| const globalToolsDir = path.join(Global.Path.config, "tools") | ||
| if (!pathEntries.has(globalToolsDir)) { | ||
| prependDirs.push(globalToolsDir) | ||
| } | ||
|
|
||
| if (prependDirs.length > 0) { | ||
| const prefix = prependDirs.join(sep) | ||
| mergedEnv.PATH = basePath ? `${prefix}${sep}${basePath}` : prefix | ||
| } |
There was a problem hiding this comment.
Don't let repo-local tools shadow unrelated commands.
Prepending .opencode/tools and the global tools dir ahead of the existing PATH means a workspace can override git, node, python, etc. after ctx.ask approved the literal command text. That weakens the permission boundary because git status can resolve to ./.opencode/tools/git instead of the expected system binary. Prefer explicit resolution for paired tools, or place these directories after the existing PATH unless the invoked command is a discovered custom tool.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/tool/bash.ts` around lines 169 - 207, The current logic
builds prependDirs and inserts them at the front of mergedEnv.PATH (via
mergedEnv.PATH = `${prefix}${sep}${basePath}`), which lets repo/global
.opencode/tools shadow system binaries; change this to append these tool dirs
instead (i.e., merge as `${basePath}${sep}${suffix}` or add to the end when
basePath is empty) so system PATH entries retain priority. Additionally, keep
the existing high-priority behavior for true custom tool resolution: detect when
the invoked command was identified as a discovered custom tool (use the code
path that resolves tool discovery or introduce a small predicate around where
you set mergedEnv — e.g., check a flag like isDiscoveredTool or match the
executable name against known tool filenames) and only prepend prependDirs in
that case; otherwise append global/project/binDir entries. Update the block that
sets mergedEnv.PATH (and references binDir, projectToolsDir, worktreeToolsDir,
globalToolsDir, prependDirs, basePath, sep, mergedEnv) accordingly.
Enhance the `/skills` dialog in the TUI to show: - Source category (Project / Global / Built-in) instead of flat "Skills" - Paired CLI tools in the footer (e.g., "Tools: altimate-dbt") - Reuses `detectToolReferences` and `skillSource` from `skill-helpers.ts` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx`:
- Line 33: The description normalization currently leaves whitespace-only
descriptions as an empty string; update the expression used when building the
dialog (the description property in dialog-skill.tsx where
skill.description?.replace(/\s+/g, " ").trim() is used) to coerce an empty
trimmed string to undefined (e.g., compute the trimmed value and set description
to trimmed === "" ? undefined : trimmed or use trimmed || undefined) so the UI
will omit blank description rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e0be38dd-3fdd-48a3-a38c-955e15019dfb
📒 Files selected for processing (1)
packages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx
| const toolStr = tools.length > 0 ? tools.join(", ") : undefined | ||
| return { | ||
| title: skill.name.padEnd(maxWidth), | ||
| description: skill.description?.replace(/\s+/g, " ").trim(), |
There was a problem hiding this comment.
Normalize empty descriptions to undefined to avoid blank UI rows.
After trim(), whitespace-only descriptions become "". Prefer coercing that to undefined so the dialog can omit the description cleanly.
Suggested patch
- description: skill.description?.replace(/\s+/g, " ").trim(),
+ description: skill.description?.replace(/\s+/g, " ").trim() || undefined,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| description: skill.description?.replace(/\s+/g, " ").trim(), | |
| description: skill.description?.replace(/\s+/g, " ").trim() || undefined, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx` at line 33, The
description normalization currently leaves whitespace-only descriptions as an
empty string; update the expression used when building the dialog (the
description property in dialog-skill.tsx where
skill.description?.replace(/\s+/g, " ").trim() is used) to coerce an empty
trimmed string to undefined (e.g., compute the trimmed value and set description
to trimmed === "" ? undefined : trimmed or use trimmed || undefined) so the UI
will omit blank description rows.
CLI `skill list`: - Sort skills alphabetically - Remove SOURCE column (noisy, not useful) - Truncate descriptions on word boundaries instead of mid-word TUI `/skills` dialog: - Group skills by domain (dbt, SQL, Schema, FinOps, etc.) instead of source - Truncate descriptions to 80 chars for readability - Show paired tools compactly in footer (max 2) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `/skills` dialog now supports keybind actions on the selected skill: - `ctrl+e` — open the SKILL.md in `$EDITOR` (skips built-in skills) - `ctrl+t` — run `skill test` and show PASS/FAIL result as a toast This makes the TUI a complete skill management surface (browse, use, edit, test) without dropping to the CLI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/opencode/src/cli/cmd/skill.ts (1)
394-406:⚠️ Potential issue | 🟠 MajorReuse the runtime shell env/PATH builder for the
--helpprobe.This inline
toolEnvassembly does not match the one BashTool/PTY use: it starts from rawprocess.env, skips the merged shell env, and flips theproject/.opencode/toolsvsworktree/.opencode/toolsprecedence.skill testcan therefore fail or hit a different executable than the agent runtime would. Please route this through the shared env/PATH helper instead of rebuilding it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/skill.ts` around lines 394 - 406, The inline construction of toolEnv (using Instance.worktree, projectDir, Global.Path.config and raw process.env) diverges from the runtime env/PATH used by BashTool/PTY and can flip tool precedence; replace this manual assembly with the shared runtime env/PATH helper used by BashTool/PTY (i.e., call the common function the BashTool/PTY uses to build the merged shell env and PATH) instead of building toolEnv here for the `skill test` probe so the probe uses the exact same merged environment and path order as the agent runtime.
🧹 Nitpick comments (1)
packages/opencode/src/cli/cmd/skill.ts (1)
277-286: Prefer exclusive creates over theaccess()preflight.Both scaffold branches do an existence check and then a separate write, which leaves a race between “already exists” and the actual create. A parallel invocation can still clobber
SKILL.mdor the tool stub after the preflight check. Switching both writes to an exclusive create path and handlingEEXISTwould make the “do not overwrite” behavior reliable.Also applies to: 294-313
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/skill.ts` around lines 277 - 286, Replace the preflight fs.access + plain fs.writeFile pattern with an exclusive-create write and handle EEXIST: remove the fs.access block that checks skillFile, then create the directory (fs.mkdir) as before and write the SKILL.md using an exclusive flag (e.g., fs.writeFile/ fs.open with 'wx') and catch errors where err.code === 'EEXIST' to emit the same "Skill already exists" message and exit; apply the same change to the other scaffold branch (the tool stub creation in the 294-313 area) so both skillFile and any tool stub are created with exclusive semantics and reliably fail on concurrent creates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/skill.ts`:
- Around line 351-366: The code currently uses Skill.get(name) which only
returns already-validated skills and thus hides malformed frontmatter; instead,
during the bootstrap callback resolve the SKILL.md file for the requested skill
name by searching the normal roots (e.g. .opencode/skills/<name>/SKILL.md and
other repo search paths used elsewhere) and read that file directly, then parse
its frontmatter with the skill module's raw-parse function (e.g. an exported
Skill.parse or parseFrontmatter function from
packages/opencode/src/skill/skill.ts) and validate name/description yourself;
replace the Skill.get(name) usage in this block with: locate SKILL.md,
fs.readFile it, call the skill parser to extract frontmatter, then use fail/pass
to report missing file or incomplete frontmatter (keeping bootstrap, fail, pass
logic intact).
---
Duplicate comments:
In `@packages/opencode/src/cli/cmd/skill.ts`:
- Around line 394-406: The inline construction of toolEnv (using
Instance.worktree, projectDir, Global.Path.config and raw process.env) diverges
from the runtime env/PATH used by BashTool/PTY and can flip tool precedence;
replace this manual assembly with the shared runtime env/PATH helper used by
BashTool/PTY (i.e., call the common function the BashTool/PTY uses to build the
merged shell env and PATH) instead of building toolEnv here for the `skill test`
probe so the probe uses the exact same merged environment and path order as the
agent runtime.
---
Nitpick comments:
In `@packages/opencode/src/cli/cmd/skill.ts`:
- Around line 277-286: Replace the preflight fs.access + plain fs.writeFile
pattern with an exclusive-create write and handle EEXIST: remove the fs.access
block that checks skillFile, then create the directory (fs.mkdir) as before and
write the SKILL.md using an exclusive flag (e.g., fs.writeFile/ fs.open with
'wx') and catch errors where err.code === 'EEXIST' to emit the same "Skill
already exists" message and exit; apply the same change to the other scaffold
branch (the tool stub creation in the 294-313 area) so both skillFile and any
tool stub are created with exclusive semantics and reliably fail on concurrent
creates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e8f74115-b13b-4827-a5e6-1454fa84c827
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/skill.tspackages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx
Comment on lines
+351
to
+366
| // 1. Check SKILL.md exists | ||
| await bootstrap(cwd, async () => { | ||
| const skill = await Skill.get(name) | ||
| if (!skill) { | ||
| fail(`Skill "${name}" not found. Check .opencode/skills/${name}/SKILL.md exists.`) | ||
| process.exitCode = 1 | ||
| return | ||
| } | ||
| pass(`SKILL.md found at ${skill.location}`) | ||
|
|
||
| // 2. Check frontmatter | ||
| if (skill.name && skill.description) { | ||
| pass(`Frontmatter valid (name: "${skill.name}", description present)`) | ||
| } else { | ||
| fail(`Frontmatter incomplete — name and description are required`) | ||
| } |
There was a problem hiding this comment.
Don’t use Skill.get() as the validator for skill test.
Skill.get(name) only sees skills that already parsed and passed the required name/description checks in packages/opencode/src/skill/skill.ts. A malformed SKILL.md or a mistyped name: therefore shows up here as “not found”, so this command cannot surface the frontmatter error it is supposed to validate. Please resolve the requested skill file from the normal search roots and parse that file directly here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/skill.ts` around lines 351 - 366, The code
currently uses Skill.get(name) which only returns already-validated skills and
thus hides malformed frontmatter; instead, during the bootstrap callback resolve
the SKILL.md file for the requested skill name by searching the normal roots
(e.g. .opencode/skills/<name>/SKILL.md and other repo search paths used
elsewhere) and read that file directly, then parse its frontmatter with the
skill module's raw-parse function (e.g. an exported Skill.parse or
parseFrontmatter function from packages/opencode/src/skill/skill.ts) and
validate name/description yourself; replace the Skill.get(name) usage in this
block with: locate SKILL.md, fs.readFile it, call the skill parser to extract
frontmatter, then use fail/pass to report missing file or incomplete frontmatter
(keeping bootstrap, fail, pass logic intact).
New subcommands for the skill CLI: `skill install <source>`: - Install skills from GitHub repos: `altimate-code skill install anthropics/skills` - Install from URL: `altimate-code skill install https://github.com/owner/repo.git` - Install from local path: `altimate-code skill install ./my-skills` - `--global` / `-g` flag for user-wide installation - Duplicate protection (skips already-installed skills) - Copies full skill directories (SKILL.md + references/ + scripts/) - Cleans up temp clones after install `skill show <name>`: - Display full skill content (name, description, location, tools, content) - Quick inspection without opening an editor Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add all CLI skill operations to the TUI for feature parity: `/skills` dialog keybinds: - `ctrl+o` — view skill content (maps to `skill show`) - `ctrl+e` — edit SKILL.md in $EDITOR - `ctrl+t` — test skill and show PASS/FAIL toast (maps to `skill test`) Slash commands (command palette): - `/skill-create <name>` — scaffolds a new skill (maps to `skill create`) - `/skill-install <source>` — install from GitHub/URL (maps to `skill install`) CLI ↔ TUI mapping: skill list → /skills dialog skill show → ctrl+o in dialog skill create → /skill-create skill test → ctrl+t in dialog skill install → /skill-install Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix name regex: allow single-char first segment (e.g., `a-tool`, `x-ray`) by changing `[a-z][a-z0-9]+` to `[a-z][a-z0-9]*` with separate length check - Validate empty/whitespace source in `skill install` (was treating `""` as cwd) - Add test cases for valid hyphenated names and adversarial install inputs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tall (#341) Remove non-functional `/skill-create` and `/skill-install` slash commands that only pre-filled input text. Replace with real keybind actions in the `/skills` dialog: - `ctrl+n` (create) — opens `DialogPrompt` for skill name, then runs `altimate-code skill create` and shows result as toast - `ctrl+i` (install) — opens `DialogPrompt` for source (owner/repo, URL, or path), then runs `altimate-code skill install` and shows result Full CLI ↔ TUI parity: skill list → /skills dialog skill show → ctrl+o in dialog skill create → ctrl+n in dialog (input prompt) skill test → ctrl+t in dialog skill install → ctrl+i in dialog (input prompt) skill edit → ctrl+e in dialog Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/opencode/test/cli/skill.test.ts (1)
329-343:⚠️ Potential issue | 🟠 MajorExercise production PATH auto-discovery flow, not a reconstructed env.
At Line 338, the test manually prepends
toolsDirtoPATH. This can pass even if the real runtime PATH-prepending logic regresses. Please assert discoverability through the same production env/path assembly used by BashTool/PTY.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/cli/skill.test.ts` around lines 329 - 343, The test "tool is discoverable when .opencode/tools/ is on PATH" currently manually prepends toolsDir to PATH (using env = { ...process.env, PATH: `${toolsDir}${sep}${process.env.PATH}` }), which bypasses the production PATH assembly; instead, call the same production helper used by BashTool/PTY to construct the environment (e.g. the BashTool.prepareEnv or PTY.buildEnv/prepareEnv API) so the test inserts toolsDir via that routine and then spawn the process with Bun.spawnSync(["my-test-tool"], { env, cwd: tmp.path }); update references to toolsDir and Bun.spawnSync accordingly and remove the manual PATH manipulation.packages/opencode/src/cli/cmd/skill.ts (1)
351-366:⚠️ Potential issue | 🟠 Major
skill testshould parse the targetSKILL.mddirectly for validation.At Line 353,
Skill.get(name)only sees already-loaded/valid skills, so malformed frontmatter is reported as “not found” instead of a validation failure. Resolve and parse the requestedSKILL.mddirectly in this command.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/skill.ts`:
- Around line 541-549: The cleanup currently checks
skillDir.startsWith(Global.Path.cache) which can mistakenly delete a
user-provided local directory; add an explicit boolean (e.g., skillDirIsTemp or
createdTempClone) initialized false and set it true at the exact point where you
create/clone a temporary directory (where temp clone logic or mkdtemp occurs,
and any call that assigns skillDir from a created temp). Then change all cleanup
conditions that currently rely on skillDir.startsWith(Global.Path.cache)
(references to skillDir cleanup checks in the code) to only delete when the new
boolean is true (and you may optionally keep an additional safe check like
pathIsInside(skillDir, Global.Path.cache)), ensuring only temp-created clone
dirs are removed.
---
Duplicate comments:
In `@packages/opencode/test/cli/skill.test.ts`:
- Around line 329-343: The test "tool is discoverable when .opencode/tools/ is
on PATH" currently manually prepends toolsDir to PATH (using env = {
...process.env, PATH: `${toolsDir}${sep}${process.env.PATH}` }), which bypasses
the production PATH assembly; instead, call the same production helper used by
BashTool/PTY to construct the environment (e.g. the BashTool.prepareEnv or
PTY.buildEnv/prepareEnv API) so the test inserts toolsDir via that routine and
then spawn the process with Bun.spawnSync(["my-test-tool"], { env, cwd: tmp.path
}); update references to toolsDir and Bun.spawnSync accordingly and remove the
manual PATH manipulation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f1204f65-1558-457c-a3ee-1e6fa7216814
📒 Files selected for processing (3)
packages/opencode/src/cli/cmd/skill.tspackages/opencode/src/cli/cmd/tui/component/dialog-skill.tsxpackages/opencode/test/cli/skill.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx
Comment on lines
+541
to
+549
| const resolved = path.isAbsolute(source) ? source : path.resolve(source) | ||
| try { | ||
| await fs.access(resolved) | ||
| } catch { | ||
| process.stderr.write(`Error: Path not found: ${resolved}` + EOL) | ||
| process.exit(1) | ||
| } | ||
| skillDir = resolved | ||
| } |
There was a problem hiding this comment.
Unsafe cleanup condition can delete local source directories.
The cleanup check uses skillDir.startsWith(Global.Path.cache). If a user installs from a local path under that cache prefix, this will recursively delete their source directory. Track whether the directory was created as a temp clone instead.
💡 Safer cleanup pattern
- let skillDir: string
+ let skillDir: string
+ let clonedTmpDir: string | undefined
if (source.startsWith("http://") || source.startsWith("https://")) {
// URL: clone the repo
process.stdout.write(`Fetching from ${source}...` + EOL)
const tmpDir = path.join(Global.Path.cache, "skill-install-" + Date.now())
@@
- skillDir = tmpDir
+ skillDir = tmpDir
+ clonedTmpDir = tmpDir
} else if (source.match(/^[a-zA-Z0-9_-]+\/[a-zA-Z0-9._-]+$/)) {
@@
- skillDir = tmpDir
+ skillDir = tmpDir
+ clonedTmpDir = tmpDir
} else {
@@
skillDir = resolved
}
@@
- if (skillDir.startsWith(Global.Path.cache)) {
- await fs.rm(skillDir, { recursive: true, force: true })
+ if (clonedTmpDir) {
+ await fs.rm(clonedTmpDir, { recursive: true, force: true })
}
process.exit(1)
}
@@
- if (skillDir.startsWith(Global.Path.cache)) {
- await fs.rm(skillDir, { recursive: true, force: true })
+ if (clonedTmpDir) {
+ await fs.rm(clonedTmpDir, { recursive: true, force: true })
}Also applies to: 563-565, 602-604
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/skill.ts` around lines 541 - 549, The cleanup
currently checks skillDir.startsWith(Global.Path.cache) which can mistakenly
delete a user-provided local directory; add an explicit boolean (e.g.,
skillDirIsTemp or createdTempClone) initialized false and set it true at the
exact point where you create/clone a temporary directory (where temp clone logic
or mkdtemp occurs, and any call that assigns skillDir from a created temp). Then
change all cleanup conditions that currently rely on
skillDir.startsWith(Global.Path.cache) (references to skillDir cleanup checks in
the code) to only delete when the new boolean is true (and you may optionally
keep an additional safe check like pathIsInside(skillDir, Global.Path.cache)),
ensuring only temp-created clone dirs are removed.
…cess spawns (#341) TUI keybind actions (create, test, install) were spawning `"altimate-code"` which resolves to the system-installed binary, not the currently running one. This causes failures when running a local build or a different version than what's installed globally. Fix: use `process.argv[0]` which is the path to the current running binary in all three spawn sites. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add explicit `env: { ...process.env }` to all Bun.spawn calls to ensure
NODE_PATH and other env vars are forwarded to subprocess
- Improve error messages: show both stderr and stdout on failure, truncate
to 200 chars, include error class name for catch blocks
- Consistent error format across create, install, and test actions
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
) The TUI dialog was spawning `process.argv[0] skill install ...` which failed because `process.argv[0]` in the compiled Bun binary doesn't resolve correctly for self-invocation (error: Script not found "skill"). Fix: inline all skill operations directly in the TUI component: - `createSkillDirect()` — creates SKILL.md + tool via `fs` operations - `installSkillDirect()` — clones repo via `git clone`, copies SKILL.md files via `fs`, no subprocess self-invocation needed - `testSkillDirect()` — spawns the tool's `--help` directly (not via the altimate-code binary) This eliminates the subprocess spawning problem entirely. The TUI now uses the same `Instance`, `Global`, and `fs` APIs as the CLI commands. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`Bun.spawnSync` blocks the TUI event loop during `git clone`, causing the install toast to show briefly then disappear with nothing installed. Fix: switch to async `Bun.spawn` with `await proc.exited` so the TUI stays responsive while cloning. Read stderr via `new Response().text()` for error messages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…pear (#341) The TUI skill dialog showed stale data after installing or creating skills because `Skill.state` is a singleton cache (via `Instance.state`) that never re-scans. Fix: - Add `State.invalidate(key, init)` to clear a single cached state entry without disposing all state for the directory - Add `Skill.invalidate()` that clears the skill cache, so the next call to `Skill.all()` re-scans all skill directories - Call `Skill.invalidate()` after successful create/install in the TUI dialog, so reopening `/skills` shows the newly installed skills Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ate (#341) The TUI runs in the main thread but the server (which serves the skills API) runs in a worker thread. Calling `Skill.invalidate()` from the TUI component had no effect because it cleared the cache in the wrong JS context. Fix: - Add `?reload=true` query param support to `GET /skill` endpoint that calls `Skill.invalidate()` in the server/worker context - TUI calls `GET /skill?reload=true` via `sdk.fetch()` after successful install/create to invalidate the worker's cache - Next `/skills` dialog open fetches fresh data from server Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problems: - "Installing..." toast disappeared immediately (short default duration) - No verification that skills actually loaded after install - No guidance on what to do next Fix: - Set 30s duration on "Installing..." toast so it persists during clone - After install/create, call `GET /skill?reload=true` and verify each installed skill appears in the returned list - Show detailed confirmation toast (8s duration) with: - Count and bullet list of installed skill names - Guidance: "Open /skills to browse, or type /<skill-name> to use" - Show explicit error if install succeeded but skills didn't load - Same pattern for create: confirms name and location Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Root cause of silent failures: `onConfirm` async callbacks had no
try/catch, so any thrown error was swallowed and no result toast shown.
Fixes:
- Wrap all install/create logic in try/catch with error toast
- Strip trailing dots from input (textarea was appending `.`)
- Strip `.git` suffix from URLs (users paste from browser)
- Trim whitespace and validate before proceeding
- "Installing..." toast now shows 60s duration with helpful text
("This may take a moment while the repo is cloned")
- Empty input shows immediate error instead of proceeding
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…efore completion (#341) The "Installing..." toast used a 60s timeout which could expire on slow connections. Changed to 600s (10 min) — effectively infinite since the result toast (`toast.show`) always replaces it on completion, clearing the old timeout automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of a static "Installing..." message, the toast now updates in real-time as each step completes: Installing from dagster-io/skills Cloning dagster-io/skills... → Cloned. Scanning for skills... → Found 2 skill(s). Installing... → Installed 1/2: dignified-python → Installed 2/2: dagster-expert → Verifying skills loaded... Each step calls `toast.show()` which replaces the previous message, keeping the toast visible throughout. The user always knows what's happening and how far along the operation is. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…#341) `Instance` and `Global` only exist in the worker thread context. The TUI dialog runs in the main thread, so accessing them throws "No context found for instance". Fix: pass `sdk.directory` (available from the SDK context in the main thread) to all skill operations instead of calling `Instance.worktree` or `Global.Path.cache`. Use `os.homedir()` for cache directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`sdk.directory` is the cwd where the TUI was launched (e.g. `packages/opencode/`), not the git worktree root. Skills installed via TUI went to the wrong `.opencode/skills/` directory. Fix: add `gitRoot()` helper that runs `git rev-parse --show-toplevel` to resolve the actual project root, matching what Instance.worktree does in the worker thread. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New CLI command: altimate-code skill remove <name> TUI keybind: ctrl+d in /skills dialog Safety: - Cannot remove skills tracked by git (part of the repo) - Cannot remove built-in skills (builtin: prefix) - Removes both skill directory and paired CLI tool Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. GitHub web URLs like `https://github.com/owner/repo/tree/main/skills/foo` now correctly extract `owner/repo` and clone the full repo. Previously tried to clone the `/tree/main` path which isn't a repo. 2. Reduced TUI dialog keybinds from 7 to 4 (edit, create, install, remove) so the footer isn't cramped. View and test are available via CLI (`skill show`, `skill test`). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `flexWrap="wrap"` to the DialogSelect keybind footer so it flows to two rows when there are many keybinds, instead of overflowing off-screen. Restore all 6 keybinds in /skills dialog: show ctrl+o | edit ctrl+e | test ctrl+t create ctrl+n | install ctrl+i | remove ctrl+d Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of 6 individual keybinds cramped in the footer, the /skills dialog now shows a single clean footer: actions ctrl+a Pressing ctrl+a on any skill opens a second-level action picker: ┌ Actions: dbt-develop ──────────────────────┐ │ Show details view info, tools, location │ │ Edit in $EDITOR open SKILL.md │ │ Test paired tool run --help on CLI tool │ │ Create new skill scaffold skill + tool │ │ Install from GitHub │ │ Remove skill delete skill + tool │ └────────────────────────────────────────────────┘ - Edit and Remove are hidden for git-tracked/builtin skills - Enter on the main list still inserts /<skill> (most common action) - Revert flexWrap on DialogSelect (no longer needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Actions (show, test, remove) were calling `dialog.clear()` which closed everything. Now they call `reopenSkillList()` which replaces the action picker with a fresh skill list dialog, so the user stays in the /skills flow. - Show: toast shows, skill list reopens behind it - Test: toast shows result, skill list reopens - Remove: skill removed, skill list reopens (with updated list) - Edit: still closes dialog (external editor takes over) - Create/Install: already used dialog.replace() (unchanged) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Opening $EDITOR with `stdio: "inherit"` conflicts with the TUI rendering, corrupts the display, and leaves the user stranded. Fix: use `open` (macOS) / `xdg-open` (Linux) to open the SKILL.md in the system default editor as a separate window. The TUI stays intact, shows a toast with the file path, and returns to the skill list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ctrl+a action picker now only shows per-skill actions: Show details, Edit, Test, Remove Global actions moved to main skill list footer keybinds: actions ctrl+a | new ctrl+n | install ctrl+i This avoids confusion where "Create new skill" and "Install from GitHub" appeared under "Actions: dbt-develop" even though they have nothing to do with that skill. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pressing Esc on the action picker was closing the dialog entirely. Now uses the `onClose` callback of `dialog.replace()` to reopen the skill list when the action picker is dismissed. Uses `setTimeout(0)` to defer the reopen so the dialog stack pop completes first. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
skills.md: - Add `skill show`, `skill install`, `skill remove` to CLI section - Add TUI keybind reference table (ctrl+a actions, ctrl+n, ctrl+i) - Document GitHub URL support (web URLs, shorthand, --global) tools/custom.md: - Add "Installing Community Skills" section with install/remove examples - Document TUI install/remove flow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Security: - Use `fs.lstat` instead of `fs.stat` during skill install to skip symlinks (prevents file disclosure from malicious repos) - Pass `dereference: false` to `fs.cp` for directory copies Bugs: - Create cache directory (`~/.cache/altimate-code`) before git clone so installs work on fresh systems - TUI `createSkillDirect` now checks if tool already exists before writing (matches CLI behavior, prevents clobbering user tools) - Add global tools dir to TUI test PATH (fixes false negatives for globally installed tools) UX: - Allow editing git-tracked skills (only block Remove, not Edit) - Split `isBuiltinOrTracked` into `isBuiltin` + `isRemovable` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New telemetry event types: - `skill_created` — tracks name, language, source (cli/tui) - `skill_installed` — tracks install_source, skill_count, skill_names - `skill_removed` — tracks skill_name, source All events follow existing patterns: - Wrapped in try/catch (telemetry never breaks operations) - Use Telemetry.getContext().sessionId (empty for CLI-only) - Include timestamp and source discriminator CLI commands instrumented: create, install, remove. TUI operations not instrumented (Telemetry runs in worker thread, TUI in main thread — would need a server endpoint to bridge). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nd (#341) `altimate-code skill install owner/repo.git` produced `https://github.com/owner/repo.git.git`. Now strips `.git` suffix from the source string before processing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a top-level
altimate-code skillCLI command withlist,create, andtestsubcommands, plus auto-discovery of user CLI tools in.opencode/tools/on the agent's PATH. This is the foundation for a skill + CLI tools extension ecosystem.New CLI commands:
altimate-code skill list— shows skills with paired CLI tools, source, descriptionaltimate-code skill create <name>— scaffoldsSKILL.md+ CLI tool stub (bash/python/node)altimate-code skill test <name>— validates frontmatter, checks paired tool on PATH, runs--helpPATH auto-discovery:
.opencode/tools/(project-level) auto-prepended to PATH in BashTool and PTY sessions~/.config/altimate-code/tools/(global) also auto-prependedSafety:
^[a-z][a-z0-9]+(-[a-z0-9]+)*$, max 64 chars (blocks path traversal, injection)--helpexecution (prevents hangs)Instance.worktree !== "/"guard prevents/.opencode/tools/on PATH outside git repos--skill-only/-sflag creates skills without CLI tool referencesType of change
Issue for this PR
Closes #341
How did you verify your code works?
skill create(bash/python/node/skill-only),skill list,skill test,skill testwith hanging tool (timeout), non-executable tool, from subdirectory, from non-git directory, concurrent createsChecklist
Summary by CodeRabbit
New Features
skillCLI with subcommands to list, create (scaffold), test, show, and install skills; CLI now augments PATH to discover project/worktree/global/bundled tools.UI
Documentation
Tests