Repository navigation
fix(core): skip dependency trees during skill discovery - #55234
kugesh-Rajasekaran wants to merge 2 commits into
Conversation
Skip dependency trees and reuse canonical-root scans per refresh. Replay aliases in source order to retain precedence and source paths.
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the port. It works as described.
I ran OpenCode from source on the base branch and on this branch with a project that has skills under .opencode/skills and .claude/skills, plus copies nested in node_modules, dist and .git. Then I listed skills through the client. On the base, the nested copies show up as skills (x, y, dist, pkg). With this PR only the real skills are listed. test/config/skill.test.ts (15/15) and test/glob.test.ts (2/2) pass, and core and util type-check cleanly. The new dependency-tree and aliased-root tests fail on the base, as expected.
Public API: unchanged. ignore is a new optional internal option on Glob.Options in packages/util.
One small request, inline: the update-during-loading test passes on the base too and isn't needed for this fix.
| ), | ||
| ) | ||
|
|
||
| it.live("keeps external skill updates that arrive during loading", () => |
There was a problem hiding this comment.
This test also passes on the base branch, so it doesn't cover anything this PR changes. The watch-before-read order was already the same before. It also depends on a race injected into readFileStringSafe and a 2-second timeout, which makes it more fragile than the other tests. Please drop it so the change stays focused.
Issue for this PR
Closes #51080.
V2 port of #51278, following the maintainer's invitation.
Type of change
What does this PR do?
On
573fd29, both V2 scanners recurse without exclusions (packages/core/src/config/plugin/compatibility.ts:47,packages/core/src/config/plugin/skill.ts:121). Skill-ID deduplication happens after traversal, so aliased roots still get scanned again.Pass glob ignores through the shared helper and prune
node_modules,.git,dist, and.cachein both scanners. Reuse canonical-root scans within each refresh, replaying results in source order to preserve precedence, paths, and IDs. Legitimate nested skills and existing watcher behavior remain supported.Synthetic V2 regressions reproduce the problem on clean
dev; an A/B/alias-of-A fixture drops from three scans to two per refresh without changing the winning skill. No V1 startup timings are reused.How did you verify your code works?
macOS, Bun 1.4.2:
bun run check:36 successful, 36 total;1371 warnings and 0 errors(same warnings on clean base). Ran in a colon-free worktree with byte-identical changes because the workspace path prevents Bun from resolving Turbo.bun run test test/config test/skill.test.ts test/skill-discovery.test.ts test/session-skill.test.ts test/tool-skill.test.ts test/skill/instructions.test.ts test/plugin/skill.test.ts:273 pass, 0 fail.bun run test:97 pass, 0 fail.bun run buildfrom core and util: exit 0. Changed-file Prettier andgit diff --check: pass.Six new regressions fail before the port. A separate update-during-loading regression caught and verified the external-symlink watch-before-read ordering.
The full core run timed out after seven minutes. Its seven observed shell failures also reproduce on clean
573fd29with macOS Bash 3.2 (25 pass, 7 failin the targeted baseline run). Windows and end-to-end startup timings were not verified locally.Screenshots / recordings
Not a UI change.
Checklist