feat: [AI-7392] humanize tool-call titles at the source#980
feat: [AI-7392] humanize tool-call titles at the source#980ralphstodomingo wants to merge 4 commits into
Conversation
Rewrite each tool's state.title in the execute() wrapper so any client (chat webview, TUI, ...) can render a readable label straight from state.title — e.g. "Reading customers model" for a dbt model read, "Searching **/*.sql" for a glob. File-acting tools get a gerund verb plus a dbt-aware target (model/seed/macro, degrading to the filename off-dbt); every other tool keeps the rich title it already emits.
📝 WalkthroughWalkthroughAdds tool-call title humanization for file-oriented tools, classifies tool sources for registry, MCP, and skill tools, wires both into execution and prompt flows, and updates tests for the new labeling and source metadata behavior. ChangesTool metadata formatting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ToolExecute as tool.ts execute()
participant DescribeToolCall
Caller->>ToolExecute: invoke tool (id, args)
ToolExecute->>ToolExecute: run tool logic, produce result.title
ToolExecute->>DescribeToolCall: describeToolCall(id, args, result.title)
DescribeToolCall->>DescribeToolCall: build verb + friendly target (dbt-aware)
DescribeToolCall-->>ToolExecute: humanized title or undefined
ToolExecute->>ToolExecute: result.title = humanized ?? original title
ToolExecute-->>Caller: return result
sequenceDiagram
participant Prompt as session/prompt.ts
participant RegistryToolSource
participant SkillToolSource
participant McpToolSource
participant HumanizeMcpTitle
Prompt->>RegistryToolSource: registryToolSource(item.id)
RegistryToolSource-->>Prompt: builtin or altimate
Prompt->>SkillToolSource: skillToolSource(metadata.skillOrigin)
SkillToolSource-->>Prompt: builtin or altimate
Prompt->>McpToolSource: mcpToolSource(key)
McpToolSource-->>Prompt: altimate or mcp
Prompt->>HumanizeMcpTitle: humanizeMcpTitle(key)
HumanizeMcpTitle-->>Prompt: readable MCP title
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Paired chat-webview change (renders the humanized |
The execute() wrapper now humanizes file-tool titles at the source, so the write tool's title is "Writing <file>" rather than the raw relative path. Update the assertion accordingly.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/opencode/test/altimate/tool-label.test.ts (1)
31-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding a case for
liston a nested dbt subdirectory.The existing case only covers a top-level directory (
{ path: "models" }). A case like{ path: "models/staging" }would have caught the mislabeling issue flagged intool-label.ts(friendlyTargetapplying the dbt "model" suffix to a directory name).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/test/altimate/tool-label.test.ts` around lines 31 - 35, Add a test in tool-label.test.ts for describeToolCall("list", ...) using a nested dbt subdirectory target like models/staging, since the current list case only covers the top-level models path and misses the friendlyTarget mislabeling in tool-label.ts. Update the test set so it asserts the nested directory is labeled correctly (without the dbt “model” suffix), keeping the existing glob/grep/list coverage intact.packages/opencode/src/altimate/tool-label.ts (1)
41-71: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
liston a nested dbt subdirectory produces a misleading "model" label.
friendlyTargetstrips extensions and appends the dbt "kind" noun whenever any ancestor segment matchesDBT_DIR_KIND, regardless of whether the target is a file or a directory. This works fine forread/write/edit(always files) but forlistthe target is always a directory, so listing a nested dbt folder is mislabeled as if it were a single model:
describeToolCall("list", { path: "models" }, ...)→"Listing models"(correct, top-level dir has no ancestor match)describeToolCall("list", { path: "models/staging" }, ...)→"Listing staging model"(wrong — this is a directory of many models, not one model)Consider skipping the dbt kind-suffix rewrite for
list, or only applying it when the base segment has a recognized dbt file extension (sql/yaml/csv).💡 Possible fix
function friendlyTarget(rawPath: string): string { const segments = rawPath.replace(/\\/g, "/").replace(/^\.\//, "").split("/").filter(Boolean) const base = segments[segments.length - 1] ?? rawPath for (const segment of segments.slice(0, -1)) { const kind = DBT_DIR_KIND[segment.toLowerCase()] - if (kind) { + if (kind && /\.(sql|ya?ml|csv)$/i.test(base)) { const name = base.replace(/\.(sql|ya?ml|csv)$/i, "") return `${name} ${kind}` } } return base }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/tool-label.ts` around lines 41 - 71, The `list` tool is incorrectly reusing `friendlyTarget`, which adds a dbt kind suffix for any path with a dbt ancestor and makes directory listings like nested folders look like a single model. Update `fileTarget` so the `list` branch does not apply the file-oriented `friendlyTarget` rewrite, or make `friendlyTarget` only append the dbt kind when the target is a file with a recognized dbt extension. Keep the existing behavior for `read`/`write`/`edit`/`multiedit`, and verify `describeToolCall("list", ...)` produces directory names without misleading model labels.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/opencode/src/altimate/tool-label.ts`:
- Around line 41-71: The `list` tool is incorrectly reusing `friendlyTarget`,
which adds a dbt kind suffix for any path with a dbt ancestor and makes
directory listings like nested folders look like a single model. Update
`fileTarget` so the `list` branch does not apply the file-oriented
`friendlyTarget` rewrite, or make `friendlyTarget` only append the dbt kind when
the target is a file with a recognized dbt extension. Keep the existing behavior
for `read`/`write`/`edit`/`multiedit`, and verify `describeToolCall("list",
...)` produces directory names without misleading model labels.
In `@packages/opencode/test/altimate/tool-label.test.ts`:
- Around line 31-35: Add a test in tool-label.test.ts for
describeToolCall("list", ...) using a nested dbt subdirectory target like
models/staging, since the current list case only covers the top-level models
path and misses the friendlyTarget mislabeling in tool-label.ts. Update the test
set so it asserts the nested directory is labeled correctly (without the dbt
“model” suffix), keeping the existing glob/grep/list coverage intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d3e6e321-9dc3-4f33-9aca-1c5cc2a20592
📒 Files selected for processing (4)
packages/opencode/src/altimate/tool-label.tspackages/opencode/src/tool/tool.tspackages/opencode/test/altimate/tool-label.test.tspackages/opencode/test/tool/write.test.ts
There was a problem hiding this comment.
Pull request overview
This PR centralizes human-friendly tool-call titles in the tool execution layer so all clients can render state.title directly, with dbt-aware labels for file-acting tools.
Changes:
- Adds
describeToolCall()to generate readable, dbt-aware titles for file/path-oriented tools (read/write/edit/multiedit/glob/grep/list). - Rewrites
result.titlefor every tool call inside theTool.execute()wrapper to apply this labeling consistently. - Adds/updates Bun tests to validate title humanization behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/opencode/src/tool/tool.ts | Applies centralized title humanization in the tool execute wrapper. |
| packages/opencode/src/altimate/tool-label.ts | Introduces describeToolCall() and dbt-aware target formatting logic. |
| packages/opencode/test/altimate/tool-label.test.ts | Adds unit tests for tool-call title labeling behavior. |
| packages/opencode/test/tool/write.test.ts | Updates expectations to match the new humanized title format for write. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function friendlyTarget(rawPath: string): string { | ||
| const segments = rawPath.replace(/\\/g, "/").replace(/^\.\//, "").split("/").filter(Boolean) | ||
| const base = segments[segments.length - 1] ?? rawPath | ||
| for (const segment of segments.slice(0, -1)) { | ||
| const kind = DBT_DIR_KIND[segment.toLowerCase()] | ||
| if (kind) { | ||
| const name = base.replace(/\.(sql|ya?ml|csv)$/i, "") | ||
| return `${name} ${kind}` | ||
| } | ||
| } | ||
| return base | ||
| } |
| export function describeToolCall(tool: string, input: unknown, rawTitle?: string): string | undefined { | ||
| const fallback = asString(rawTitle) | ||
| const verb = FILE_TOOL_VERBS[tool] | ||
| if (verb && input && typeof input === "object") { | ||
| const target = fileTarget(tool, input as Record<string, unknown>) | ||
| if (target) return `${verb} ${target}` | ||
| } | ||
| // Non-file / rich-title tools: keep the title the tool already emitted. | ||
| return fallback | ||
| } |
| // The execute() wrapper humanizes file-tool titles at the source | ||
| // (see src/altimate/tool-label.ts) — a non-dbt path degrades to the | ||
| // filename, so the title is a readable "Writing <file>" label. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/tool-label.ts">
<violation number="1" location="packages/opencode/src/altimate/tool-label.ts:50">
P3: Non-dbt files under common directories like `src/models` can be shown as dbt objects because `friendlyTarget()` applies the dbt noun to any path segment named `models`, `tests`, or `macros`, regardless of the target file type. That makes labels such as `Reading User.tsx model` possible outside dbt projects; consider only applying the dbt noun when the target looks like a dbt resource (for example known dbt file extensions) and otherwise falling back to the basename.</violation>
<violation number="2" location="packages/opencode/src/altimate/tool-label.ts:85">
P2: When `list` targets the worktree root (no path argument or empty relative path), `fileTarget()` returns `undefined` and `asString(rawTitle)` also returns `undefined` (since `path.relative(worktree, worktree)` is `""`). The `??` fallback in `tool.ts` then yields the original empty-string title, producing a blank UI label. Consider producing a fallback like `"Listing ."` when a file tool has a verb but neither a usable target nor a non-empty raw title.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const verb = FILE_TOOL_VERBS[tool] | ||
| if (verb && input && typeof input === "object") { | ||
| const target = fileTarget(tool, input as Record<string, unknown>) | ||
| if (target) return `${verb} ${target}` |
There was a problem hiding this comment.
P2: When list targets the worktree root (no path argument or empty relative path), fileTarget() returns undefined and asString(rawTitle) also returns undefined (since path.relative(worktree, worktree) is ""). The ?? fallback in tool.ts then yields the original empty-string title, producing a blank UI label. Consider producing a fallback like "Listing ." when a file tool has a verb but neither a usable target nor a non-empty raw title.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/tool-label.ts, line 85:
<comment>When `list` targets the worktree root (no path argument or empty relative path), `fileTarget()` returns `undefined` and `asString(rawTitle)` also returns `undefined` (since `path.relative(worktree, worktree)` is `""`). The `??` fallback in `tool.ts` then yields the original empty-string title, producing a blank UI label. Consider producing a fallback like `"Listing ."` when a file tool has a verb but neither a usable target nor a non-empty raw title.</comment>
<file context>
@@ -0,0 +1,89 @@
+ const verb = FILE_TOOL_VERBS[tool]
+ if (verb && input && typeof input === "object") {
+ const target = fileTarget(tool, input as Record<string, unknown>)
+ if (target) return `${verb} ${target}`
+ }
+ // Non-file / rich-title tools: keep the title the tool already emitted.
</file context>
| const segments = rawPath.replace(/\\/g, "/").replace(/^\.\//, "").split("/").filter(Boolean) | ||
| const base = segments[segments.length - 1] ?? rawPath | ||
| for (const segment of segments.slice(0, -1)) { | ||
| const kind = DBT_DIR_KIND[segment.toLowerCase()] |
There was a problem hiding this comment.
P3: Non-dbt files under common directories like src/models can be shown as dbt objects because friendlyTarget() applies the dbt noun to any path segment named models, tests, or macros, regardless of the target file type. That makes labels such as Reading User.tsx model possible outside dbt projects; consider only applying the dbt noun when the target looks like a dbt resource (for example known dbt file extensions) and otherwise falling back to the basename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/tool-label.ts, line 50:
<comment>Non-dbt files under common directories like `src/models` can be shown as dbt objects because `friendlyTarget()` applies the dbt noun to any path segment named `models`, `tests`, or `macros`, regardless of the target file type. That makes labels such as `Reading User.tsx model` possible outside dbt projects; consider only applying the dbt noun when the target looks like a dbt resource (for example known dbt file extensions) and otherwise falling back to the basename.</comment>
<file context>
@@ -0,0 +1,89 @@
+ const segments = rawPath.replace(/\\/g, "/").replace(/^\.\//, "").split("/").filter(Boolean)
+ const base = segments[segments.length - 1] ?? rawPath
+ for (const segment of segments.slice(0, -1)) {
+ const kind = DBT_DIR_KIND[segment.toLowerCase()]
+ if (kind) {
+ const name = base.replace(/\.(sql|ya?ml|csv)$/i, "")
</file context>
dev-punia-altimate
left a comment
dev-punia-altimate
left a comment
There was a problem hiding this comment.
🤖 Code Review — OpenCodeReview (Gemini) — 3 finding(s)
- 2 anchored to a line (posted inline when the comment stream is on)
- 1 without a line anchor
All findings (full text)
1. packages/opencode/src/altimate/tool-label.ts (L49-L50)
[🟠 MEDIUM] Iterating left-to-right (from the root down) can incorrectly match an outer directory that coincidentally shares a name with a dbt folder, rather than the intended specific directory (e.g., an absolute path like /Users/user/models/my_project/macros/utils.sql would match models instead of macros, returning utils model instead of utils macro).
Consider iterating right-to-left (from the file upwards) to match the most specific parent directory.
Suggested change:
for (let i = segments.length - 2; i >= 0; i--) {
const segment = segments[i]
const kind = DBT_DIR_KIND[segment.toLowerCase()]
2. packages/opencode/src/altimate/tool-label.ts (L51-L54)
[🟠 MEDIUM] The regex misses .py (for Python models) and .md (for dbt documentation files), which are both common in dbt projects. Without these, a Python model would render as model.py model rather than model model. Consider adding them to the regex.
Suggested change:
if (kind) {
const name = base.replace(/\.(sql|ya?ml|csv|py|md)$/i, "")
return `${name} ${kind}`
}
3. packages/opencode/src/altimate/tool-label.ts
[🔵 LOW] FILE_TOOL_VERBS and DBT_DIR_KIND are plain objects. Looking up a string like constructor or toString could hit inherited Object.prototype properties, leading to unexpected string values in the label.
Consider using Object.assign(Object.create(null), {...}) to safely initialize these dictionaries without prototypes.
Suggested change:
const FILE_TOOL_VERBS: Record<string, string> = Object.assign(Object.create(null), {
read: "Reading",
write: "Writing",
edit: "Editing",
multiedit: "Editing",
glob: "Searching",
grep: "Searching",
list: "Listing",
})
/** dbt directory → singular noun used in the label. */
const DBT_DIR_KIND: Record<string, string> = Object.assign(Object.create(null), {
models: "model",
seeds: "seed",
macros: "macro",
snapshots: "snapshot",
tests: "test",
analyses: "analysis",
analysis: "analysis",
})
| for (const segment of segments.slice(0, -1)) { | ||
| const kind = DBT_DIR_KIND[segment.toLowerCase()] |
There was a problem hiding this comment.
[🟠 MEDIUM] Iterating left-to-right (from the root down) can incorrectly match an outer directory that coincidentally shares a name with a dbt folder, rather than the intended specific directory (e.g., an absolute path like /Users/user/models/my_project/macros/utils.sql would match models instead of macros, returning utils model instead of utils macro).
Consider iterating right-to-left (from the file upwards) to match the most specific parent directory.
Suggested change:
| for (const segment of segments.slice(0, -1)) { | |
| const kind = DBT_DIR_KIND[segment.toLowerCase()] | |
| for (let i = segments.length - 2; i >= 0; i--) { | |
| const segment = segments[i] | |
| const kind = DBT_DIR_KIND[segment.toLowerCase()] |
| if (kind) { | ||
| const name = base.replace(/\.(sql|ya?ml|csv)$/i, "") | ||
| return `${name} ${kind}` | ||
| } |
There was a problem hiding this comment.
[🟠 MEDIUM] The regex misses .py (for Python models) and .md (for dbt documentation files), which are both common in dbt projects. Without these, a Python model would render as model.py model rather than model model. Consider adding them to the regex.
Suggested change:
| if (kind) { | |
| const name = base.replace(/\.(sql|ya?ml|csv)$/i, "") | |
| return `${name} ${kind}` | |
| } | |
| if (kind) { | |
| const name = base.replace(/\.(sql|ya?ml|csv|py|md)$/i, "") | |
| return `${name} ${kind}` | |
| } |
🤖 Code Review — OpenCodeReview (Gemini) — 3 finding(s)
All findings (full text)1.
|
Classify every tool call's origin server-side and stamp it on the tool part's state.metadata.source (builtin | altimate | mcp), so clients render the source badge without re-deriving it from tool-name prefixes: - tool-source.ts: native-set inversion (any non-native registry tool is Altimate, so new Altimate tools classify with no maintenance); Datamates MCP folded into "altimate"; humanizeMcpTitle for readable MCP labels. - resolveTools: stamp metadata.source on registry tools; on MCP tools set both the source and a humanized title (they previously had title: "").
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| * the registry is Altimate-provided, so new Altimate tools classify correctly | ||
| * with no per-tool maintenance here. | ||
| */ | ||
| const NATIVE_TOOL_IDS = new Set<string>([ |
There was a problem hiding this comment.
[SUGGESTION]: The native batch tool would be misclassified as altimate
batch is a native opencode tool (Tool.define("batch", ...) in src/tool/batch.ts, registered in ToolRegistry.all() behind experimental.batch_tool), but it is absent from NATIVE_TOOL_IDS. As a result registryToolSource("batch") returns "altimate", contradicting the comment above the set that claims every non-listed registry tool is Altimate-provided. Adding "batch" to this set keeps its source badge correct.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/altimate/tool-source.ts">
<violation number="1" location="packages/opencode/src/altimate/tool-source.ts:58">
P2: MCP source badging can misclassify some third-party tools as Altimate because `mcpToolSource` matches `datamate` against the start of the entire key instead of the parsed `<client>` segment. Using a client-segment check (with `_` boundary) would avoid false Altimate badges for similarly named external MCP clients.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /** Classify an MCP tool by its `<client>_<tool>` key: Altimate (Datamates) vs third-party. */ | ||
| export function mcpToolSource(key: string): ToolSource { | ||
| const lower = key.toLowerCase() | ||
| return ALTIMATE_MCP_PREFIXES.some((p) => lower.startsWith(p)) ? "altimate" : "mcp" |
There was a problem hiding this comment.
P2: MCP source badging can misclassify some third-party tools as Altimate because mcpToolSource matches datamate against the start of the entire key instead of the parsed <client> segment. Using a client-segment check (with _ boundary) would avoid false Altimate badges for similarly named external MCP clients.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/tool-source.ts, line 58:
<comment>MCP source badging can misclassify some third-party tools as Altimate because `mcpToolSource` matches `datamate` against the start of the entire key instead of the parsed `<client>` segment. Using a client-segment check (with `_` boundary) would avoid false Altimate badges for similarly named external MCP clients.</comment>
<file context>
@@ -0,0 +1,72 @@
+/** Classify an MCP tool by its `<client>_<tool>` key: Altimate (Datamates) vs third-party. */
+export function mcpToolSource(key: string): ToolSource {
+ const lower = key.toLowerCase()
+ return ALTIMATE_MCP_PREFIXES.some((p) => lower.startsWith(p)) ? "altimate" : "mcp"
+}
+
</file context>
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (7 files)
Fix these issues in Kilo Cloud Reviewed by glm-5.2 · Input: 57.8K · Output: 16.5K · Cached: 545.6K |
Skill loads always classified as `builtin` (terminal badge) because `skill` is a native tool id. Classify skill loads per-call from the loaded skill's origin instead: Altimate-shipped skills (bundled under `~/.altimate/builtin` or binary-embedded `builtin:` skills) carry the `altimate` source badge; user global/project skills stay neutral so the badge never over-claims a personal or third-party skill. - `tool-source.ts`: add `skillToolSource(origin)` badge policy - `skill.ts`: export `classifySkillSource`, stamp `skillOrigin` on the tool metadata, and match `builtin:`/`.altimate/builtin` locations (normalizing Windows separators) - `prompt.ts`: stamp the skill source from the reported origin - tests: `skillToolSource` + `classifySkillSource` units, plus real `SkillTool.execute` integration covering project (neutral) and builtin (Altimate) origins
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opencode/test/tool/skill-source.test.ts (1)
46-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
tmpdir()'sinitoption for fixture setup instead of ad hocfs.mkdir/fs.writeFilecalls.Both tests create skill directories/files manually after
tmpdir()returns. As per path instructions,test/**/*.test.tsfiles should use theinitoption withtmpdir()for custom setup logic and return extra data accessible viatmp.extrarather than mutating the directory imperatively afterward.Based on path instructions ("Use the
initoption withtmpdir()for custom setup logic and return extra data accessible viatmp.extra").Also applies to: 72-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/test/tool/skill-source.test.ts` around lines 46 - 53, The test setup in skill-source.test.ts is creating the skill fixture imperatively after tmpdir() returns, which should be moved into tmpdir()’s init option instead. Update the relevant test fixture creation around the e2e-probe skill so init creates the .claude/skills/e2e-probe/SKILL.md file and returns any needed paths/data via tmp.extra, and apply the same pattern to the other skill fixture block in this file.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/opencode/src/tool/skill.ts`:
- Around line 20-30: The classifySkillSource helper is over-classifying any path
containing node_modules as builtin, which can mislabel third-party skills as
Altimate-shipped. Tighten the check in classifySkillSource so only
Altimate-owned npm packages are treated as builtin, using the existing builtin:/
and .altimate/builtin logic plus a scoped package-path match (for example, the
Altimate namespace used by the tests). Keep the global and project fallbacks
unchanged, and make sure skillToolSource continues to rely on the corrected
classification.
---
Nitpick comments:
In `@packages/opencode/test/tool/skill-source.test.ts`:
- Around line 46-53: The test setup in skill-source.test.ts is creating the
skill fixture imperatively after tmpdir() returns, which should be moved into
tmpdir()’s init option instead. Update the relevant test fixture creation around
the e2e-probe skill so init creates the .claude/skills/e2e-probe/SKILL.md file
and returns any needed paths/data via tmp.extra, and apply the same pattern to
the other skill fixture block in this file.
🪄 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: 906ca607-8a29-48ab-9b45-e70ea802c35c
📒 Files selected for processing (5)
packages/opencode/src/altimate/tool-source.tspackages/opencode/src/session/prompt.tspackages/opencode/src/tool/skill.tspackages/opencode/test/altimate/tool-source.test.tspackages/opencode/test/tool/skill-source.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/opencode/src/session/prompt.ts
- packages/opencode/src/altimate/tool-source.ts
| // altimate_change start — classifySkillSource helper for skill telemetry + source badge | ||
| export function classifySkillSource(location: string): "builtin" | "global" | "project" { | ||
| // Normalize separators so `.altimate/builtin` / homedir prefix match on Windows too. | ||
| const normalized = location.replace(/\\/g, "/") | ||
| // Embedded skills load with a `builtin:<name>/SKILL.md` location and Altimate's | ||
| // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped. | ||
| if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin")) | ||
| return "builtin" | ||
| if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global" | ||
| return "project" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unscoped node_modules substring check over-classifies third-party skills as Altimate-shipped.
normalized.includes("node_modules") matches any path containing node_modules, not specifically Altimate's own packages. A user who installs an unrelated third-party skill package (e.g., via npm) that happens to live under any node_modules directory will be misclassified as "builtin" and subsequently earn the Altimate badge via skillToolSource, misrepresenting the skill's provenance to the user. The companion test (/app/node_modules/@altimateai/pkg/skills/x/SKILL.md) implies the intent is scoped to Altimate's own npm scope, but the implementation doesn't enforce that scope.
🐛 Proposed fix to scope the node_modules check
- if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin"))
+ if (
+ normalized.startsWith("builtin:") ||
+ normalized.includes("node_modules/@altimateai/") ||
+ normalized.includes(".altimate/builtin")
+ )
return "builtin"📝 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.
| // altimate_change start — classifySkillSource helper for skill telemetry + source badge | |
| export function classifySkillSource(location: string): "builtin" | "global" | "project" { | |
| // Normalize separators so `.altimate/builtin` / homedir prefix match on Windows too. | |
| const normalized = location.replace(/\\/g, "/") | |
| // Embedded skills load with a `builtin:<name>/SKILL.md` location and Altimate's | |
| // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped. | |
| if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin")) | |
| return "builtin" | |
| if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global" | |
| return "project" | |
| } | |
| // altimate_change start — classifySkillSource helper for skill telemetry + source badge | |
| export function classifySkillSource(location: string): "builtin" | "global" | "project" { | |
| // Normalize separators so `.altimate/builtin` / homedir prefix match on Windows too. | |
| const normalized = location.replace(/\\/g, "/") | |
| // Embedded skills load with a `builtin:<name>/SKILL.md` location and Altimate's | |
| // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped. | |
| if ( | |
| normalized.startsWith("builtin:") || | |
| normalized.includes("node_modules/@altimateai/") || | |
| normalized.includes(".altimate/builtin") | |
| ) | |
| return "builtin" | |
| if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global" | |
| return "project" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/opencode/src/tool/skill.ts` around lines 20 - 30, The
classifySkillSource helper is over-classifying any path containing node_modules
as builtin, which can mislabel third-party skills as Altimate-shipped. Tighten
the check in classifySkillSource so only Altimate-owned npm packages are treated
as builtin, using the existing builtin:/ and .altimate/builtin logic plus a
scoped package-path match (for example, the Altimate namespace used by the
tests). Keep the global and project fallbacks unchanged, and make sure
skillToolSource continues to rely on the corrected classification.
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/tool/skill.ts">
<violation number="1" location="packages/opencode/src/tool/skill.ts:26">
P2: The `normalized.includes("node_modules")` check over-classifies third-party npm packages as Altimate-shipped skills. Any skill whose path traverses a `node_modules` directory — even one installed from a completely unrelated npm package — would be classified as `"builtin"` and subsequently earn the Altimate badge. Scoping this to `node_modules/@altimateai/` would match the intent shown by the test case while preventing misrepresentation of third-party skill provenance.</violation>
<violation number="2" location="packages/opencode/src/tool/skill.ts:28">
P3: The new home-directory classification uses `startsWith(...)` without a directory boundary check, so project paths that only share a home prefix can be reported as `global`. This can skew `skill_source` telemetry and make origin classification less reliable; checking `home` equality or `home + '/'` would avoid collisions.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const normalized = location.replace(/\\/g, "/") | ||
| // Embedded skills load with a `builtin:<name>/SKILL.md` location and Altimate's | ||
| // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped. | ||
| if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin")) |
There was a problem hiding this comment.
P2: The normalized.includes("node_modules") check over-classifies third-party npm packages as Altimate-shipped skills. Any skill whose path traverses a node_modules directory — even one installed from a completely unrelated npm package — would be classified as "builtin" and subsequently earn the Altimate badge. Scoping this to node_modules/@altimateai/ would match the intent shown by the test case while preventing misrepresentation of third-party skill provenance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tool/skill.ts, line 26:
<comment>The `normalized.includes("node_modules")` check over-classifies third-party npm packages as Altimate-shipped skills. Any skill whose path traverses a `node_modules` directory — even one installed from a completely unrelated npm package — would be classified as `"builtin"` and subsequently earn the Altimate badge. Scoping this to `node_modules/@altimateai/` would match the intent shown by the test case while preventing misrepresentation of third-party skill provenance.</comment>
<file context>
@@ -17,10 +17,15 @@ import os from "os"
+ const normalized = location.replace(/\\/g, "/")
+ // Embedded skills load with a `builtin:<name>/SKILL.md` location and Altimate's
+ // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped.
+ if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin"))
+ return "builtin"
+ if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global"
</file context>
| if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin")) | |
| if (normalized.startsWith("builtin:") || normalized.includes("node_modules/@altimateai/") || normalized.includes(".altimate/builtin")) |
| // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped. | ||
| if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin")) | ||
| return "builtin" | ||
| if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global" |
There was a problem hiding this comment.
P3: The new home-directory classification uses startsWith(...) without a directory boundary check, so project paths that only share a home prefix can be reported as global. This can skew skill_source telemetry and make origin classification less reliable; checking home equality or home + '/' would avoid collisions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tool/skill.ts, line 28:
<comment>The new home-directory classification uses `startsWith(...)` without a directory boundary check, so project paths that only share a home prefix can be reported as `global`. This can skew `skill_source` telemetry and make origin classification less reliable; checking `home` equality or `home + '/'` would avoid collisions.</comment>
<file context>
@@ -17,10 +17,15 @@ import os from "os"
+ // bundled skills land under `~/.altimate/builtin/` — both are Altimate-shipped.
+ if (normalized.startsWith("builtin:") || normalized.includes("node_modules") || normalized.includes(".altimate/builtin"))
+ return "builtin"
+ if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global"
return "project"
}
</file context>
| if (normalized.startsWith(os.homedir().replace(/\\/g, "/"))) return "global" | |
| if ( | |
| normalized === os.homedir().replace(/\\/g, "/").replace(/\/+$/, "") || | |
| normalized.startsWith(`${os.homedir().replace(/\\/g, "/").replace(/\/+$/, "")}/`) | |
| ) | |
| return "global" |
PINEAPPLE
Summary
Makes the Altimate Code server the source of truth for every tool call's display label and its origin, so any client (chat webview, TUI, …) renders them verbatim — no client-side labeling or classification.
state.title) — file tools get dbt-aware labels ("Reading customers model", "Searching**/*.sql", degrading to the filename off-dbt); MCP tools get a Title-Cased label from their<client>_<tool>key (previouslytitle: ""); every other tool keeps its own rich title.state.metadata.source=builtin | altimate | mcp) — clients badge a tool without prefix-guessing. Datamates-over-MCP folds intoaltimate.skilltool loads skills of varying origin, so it's classified per-call from the loaded skill's origin: Altimate-shipped skills (bundled under~/.altimate/builtin, or embeddedbuiltin:skills) →altimate; userglobal/projectskills → neutralbuiltin. Keeps the badge from over-claiming a user's own skill.Files:
src/altimate/tool-label.ts(describeToolCall),src/altimate/tool-source.ts(registryToolSource/mcpToolSource/skillToolSource/humanizeMcpTitle),src/tool/skill.ts(classifySkillSource+skillOriginon metadata),src/session/prompt.ts(stampsmetadata.source+ title in theresolveToolsloops).Paired renderer (rendering only, no classification):
AltimateAI/vscode-altimate-mcp-server#388.Test Plan
Unit (
bun test):tool-label(7);tool-sourceincl.skillToolSource+classifySkillSourceacross all branches (Windows separators,builtin:/.altimate/builtin/ homedir→global / project); updatedwritetitle test.Integration: real
SkillTool.executeagainst filesystem-discovered skills, asserting the emitted metadata drives the source for both a project skill (→builtin) and a bundled~/.altimate/builtinskill (→altimate).Serve-API E2E (the source of truth for a server change) — under
altimate serve, the session API returns the stamped values:query-optimize→skillOrigin: builtin,source: altimateskillOrigin: global,source: builtinsql_analyze/datamate_*→altimate;read/glob→builtinBadge rendering of these values is demonstrated on the paired webview PR (Upgrade notification silently skipped — users never see update indicator #388).
Checklist