feat: add skill CLI command and .opencode/tools/ auto-discovery#342
Open
anandgupta42 wants to merge 2 commits intomainfrom
Open
feat: add skill CLI command and .opencode/tools/ auto-discovery#342anandgupta42 wants to merge 2 commits intomainfrom
skill CLI command and .opencode/tools/ auto-discovery#342anandgupta42 wants to merge 2 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}
📝 WalkthroughWalkthroughAdds a top-level Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as altimate-code CLI
participant FS as Filesystem (.opencode/...)
participant PTY
participant GlobalCfg as Global config (~/.config/...)
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->>GlobalCfg: check ~/.config/.../tools/<tool>
CLI->>PTY: spawn `<tool> --help` with PATH prefixed (ALTIMATE_BIN_DIR, project tools, worktree tools, global tools)
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 docstrings
🧪 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.
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
altimate-code skillCLI:list(optional JSON),create <name>(scaffold skill ± paired tool), andtest <name>(validate skill/tool).Documentation
Tests