fix: align path discovery with upstream opencode#2842
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
There was a problem hiding this comment.
4 issues found across 13 files
Confidence score: 3/5
- There is a concrete, high-confidence regression risk in
src/features/claude-code-command-loader/loader.ts: directory processing order is not reversed, so generic ancestor/global commands can overwrite child/profile-specific commands and produce incorrect command resolution. - The other findings are lower risk but still worth addressing:
src/features/opencode-skill-loader/loader.tshas duplicated discovery/loading logic (maintainability risk), andsrc/shared/project-discovery-dirs.tsusesexistsSyncin a way that can accept files instead of directories (edge-case path selection bug). - Given the two severity 8/10 issues with 10/10 confidence, this is more than a minor cleanup PR and carries user-facing behavior risk until precedence ordering is fixed.
- Pay close attention to
src/features/claude-code-command-loader/loader.ts,src/shared/project-discovery-dirs.ts- command precedence and directory validation can lead to incorrect loading behavior.
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="src/features/opencode-skill-loader/loader.ts">
<violation number="1" location="src/features/opencode-skill-loader/loader.ts:24">
P2: Refactor `loadProjectSkills` to call `discoverProjectClaudeSkills` to avoid duplicating the discovery and loading logic.</violation>
</file>
<file name="src/shared/project-discovery-dirs.ts">
<violation number="1" location="src/shared/project-discovery-dirs.ts:15">
P2: `existsSync` returns true for files as well as directories, which means this function could incorrectly return a file path if a file happens to be named (for example) `.opencode/skills`. Replace it with a directory-specific check.</violation>
</file>
<file name="src/features/claude-code-command-loader/loader.ts">
<violation number="1" location="src/features/claude-code-command-loader/loader.ts:131">
P1: Reverse the directory array before mapping so that specific profile commands properly overwrite general global commands.</violation>
<violation number="2" location="src/features/claude-code-command-loader/loader.ts:139">
P1: Reverse the directory array before mapping so that child project commands correctly overwrite generic ancestor commands.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return commandsToRecord(commands) | ||
| const opencodeProjectDirs = findProjectOpencodeCommandDirs(directory ?? process.cwd()) | ||
| const allCommands = await Promise.all( | ||
| opencodeProjectDirs.map((commandsDir) => |
There was a problem hiding this comment.
P1: Reverse the directory array before mapping so that child project commands correctly overwrite generic ancestor commands.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-code-command-loader/loader.ts, line 139:
<comment>Reverse the directory array before mapping so that child project commands correctly overwrite generic ancestor commands.</comment>
<file context>
@@ -121,16 +126,21 @@ export async function loadProjectCommands(directory?: string): Promise<Record<st
- return commandsToRecord(commands)
+ const opencodeProjectDirs = findProjectOpencodeCommandDirs(directory ?? process.cwd())
+ const allCommands = await Promise.all(
+ opencodeProjectDirs.map((commandsDir) =>
+ loadCommandsFromDir(commandsDir, "opencode-project"),
+ ),
</file context>
| opencodeProjectDirs.map((commandsDir) => | |
| opencodeProjectDirs.reverse().map((commandsDir) => |
| return commandsToRecord(commands) | ||
| const opencodeCommandDirs = getOpenCodeCommandDirs({ binary: "opencode" }) | ||
| const allCommands = await Promise.all( | ||
| opencodeCommandDirs.map((commandsDir) => loadCommandsFromDir(commandsDir, "opencode")), |
There was a problem hiding this comment.
P1: Reverse the directory array before mapping so that specific profile commands properly overwrite general global commands.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-code-command-loader/loader.ts, line 131:
<comment>Reverse the directory array before mapping so that specific profile commands properly overwrite general global commands.</comment>
<file context>
@@ -121,16 +126,21 @@ export async function loadProjectCommands(directory?: string): Promise<Record<st
- return commandsToRecord(commands)
+ const opencodeCommandDirs = getOpenCodeCommandDirs({ binary: "opencode" })
+ const allCommands = await Promise.all(
+ opencodeCommandDirs.map((commandsDir) => loadCommandsFromDir(commandsDir, "opencode")),
+ )
+ return commandsToRecord(allCommands.flat())
</file context>
| opencodeCommandDirs.map((commandsDir) => loadCommandsFromDir(commandsDir, "opencode")), | |
| opencodeCommandDirs.reverse().map((commandsDir) => loadCommandsFromDir(commandsDir, "opencode")), |
| const projectSkillDirs = findProjectClaudeSkillDirs(directory ?? process.cwd()) | ||
| const allSkills = await Promise.all( | ||
| projectSkillDirs.map((skillsDir) => loadSkillsFromDir({ skillsDir, scope: "project" })), | ||
| ) | ||
| return skillsToCommandDefinitionRecord(deduplicateSkillsByName(allSkills.flat())) |
There was a problem hiding this comment.
P2: Refactor loadProjectSkills to call discoverProjectClaudeSkills to avoid duplicating the discovery and loading logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/opencode-skill-loader/loader.ts, line 24:
<comment>Refactor `loadProjectSkills` to call `discoverProjectClaudeSkills` to avoid duplicating the discovery and loading logic.</comment>
<file context>
@@ -16,9 +21,11 @@ export async function loadUserSkills(): Promise<Record<string, CommandDefinition
- const projectSkillsDir = join(directory ?? process.cwd(), ".claude", "skills")
- const skills = await loadSkillsFromDir({ skillsDir: projectSkillsDir, scope: "project" })
- return skillsToCommandDefinitionRecord(skills)
+ const projectSkillDirs = findProjectClaudeSkillDirs(directory ?? process.cwd())
+ const allSkills = await Promise.all(
+ projectSkillDirs.map((skillsDir) => loadSkillsFromDir({ skillsDir, scope: "project" })),
</file context>
| const projectSkillDirs = findProjectClaudeSkillDirs(directory ?? process.cwd()) | |
| const allSkills = await Promise.all( | |
| projectSkillDirs.map((skillsDir) => loadSkillsFromDir({ skillsDir, scope: "project" })), | |
| ) | |
| return skillsToCommandDefinitionRecord(deduplicateSkillsByName(allSkills.flat())) | |
| const skills = await discoverProjectClaudeSkills(directory) | |
| return skillsToCommandDefinitionRecord(skills) |
| while (true) { | ||
| for (const targetPath of targetPaths) { | ||
| const candidateDirectory = join(currentDirectory, ...targetPath) | ||
| if (!existsSync(candidateDirectory) || seen.has(candidateDirectory)) { |
There was a problem hiding this comment.
P2: existsSync returns true for files as well as directories, which means this function could incorrectly return a file path if a file happens to be named (for example) .opencode/skills. Replace it with a directory-specific check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/project-discovery-dirs.ts, line 15:
<comment>`existsSync` returns true for files as well as directories, which means this function could incorrectly return a file path if a file happens to be named (for example) `.opencode/skills`. Replace it with a directory-specific check.</comment>
<file context>
@@ -0,0 +1,52 @@
+ while (true) {
+ for (const targetPath of targetPaths) {
+ const candidateDirectory = join(currentDirectory, ...targetPath)
+ if (!existsSync(candidateDirectory) || seen.has(candidateDirectory)) {
+ continue
+ }
</file context>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <[email protected]>
There was a problem hiding this comment.
1 issue found across 4 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="src/features/claude-code-command-loader/loader.ts">
<violation number="1" location="src/features/claude-code-command-loader/loader.ts:125">
P1: Custom agent: **Opencode Compatibility**
The deduplication logic reverses upstream OpenCode's configuration merging precedence by keeping the first encountered command instead of the last.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| function commandsToRecord(commands: LoadedCommand[]): Record<string, CommandDefinition> { | ||
| const result: Record<string, CommandDefinition> = {} | ||
| for (const cmd of commands) { | ||
| for (const cmd of deduplicateLoadedCommandsByName(commands)) { |
There was a problem hiding this comment.
P1: Custom agent: Opencode Compatibility
The deduplication logic reverses upstream OpenCode's configuration merging precedence by keeping the first encountered command instead of the last.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-code-command-loader/loader.ts, line 125:
<comment>The deduplication logic reverses upstream OpenCode's configuration merging precedence by keeping the first encountered command instead of the last.</comment>
<file context>
@@ -104,9 +104,25 @@ $ARGUMENTS
function commandsToRecord(commands: LoadedCommand[]): Record<string, CommandDefinition> {
const result: Record<string, CommandDefinition> = {}
- for (const cmd of commands) {
+ for (const cmd of deduplicateLoadedCommandsByName(commands)) {
const { name: _name, argumentHint: _argumentHint, ...openCodeCompatible } = cmd.definition
result[cmd.name] = openCodeCompatible as CommandDefinition
</file context>
Summary
.opencode/skilland.opencode/commandsaliases where our compatibility loaders previously missed them~/in plugin-config skill sources so config-backed skill discovery matches upstream behaviorTesting
bun test src/shared/project-discovery-dirs.test.ts src/shared/opencode-command-dirs.test.ts src/features/opencode-skill-loader/config-source-discovery.test.ts src/features/opencode-skill-loader/loader.test.ts src/features/claude-code-command-loader/loader.test.ts src/tools/slashcommand/command-discovery.test.tsbun run typecheckbun run buildbun --evalexercising ancestor.agents/skills,.opencode/skill,.opencode/commands, global plural commands, and~/skill-source expansion