Skip to content

fix(acp): map pi, grok, and kimi to the skill dirs their engines scan - #218

Merged
wibus-wee merged 1 commit into
LodyAI:mainfrom
Astro-Han:fix/150-agents-skills-dirs
Sep 1, 2026
Merged

fix(acp): map pi, grok, and kimi to the skill dirs their engines scan#218
wibus-wee merged 1 commit into
LodyAI:mainfrom
Astro-Han:fix/150-agents-skills-dirs

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #150

Problem / pressure

ACP_SKILL_DIRS_BY_AGENT_TYPE is a hand-maintained mirror of facts owned by each engine, and it had drifted from what the engines actually scan:

  • pi (packages/coding-agent/docs/skills.md, "Locations") scans ~/.pi/agent/skills and ~/.agents/skills; the table listed only the first.
  • grok had no entry at all. MANAGED_BUILTIN_RUNTIMES maps grok-build to agentType: 'grok', but no grok key existed. Its skill roots come from CompatConfig::skill_config_dirs() in xai-grok-tools/src/types/compat.rs, which returns [".grok", ".agents", ".claude", ".cursor"] under the all-on VendorCompat::default() (pinned by that file's own skill_config_dirs_all_on_matches_legacy_constant test), at every tier.
  • kimi uses .kimi-code/skills + .agents/skills per project and ~/.kimi-code/skills + ~/.agents/skills globally (packages/agent-core-v2/src/features/skill/catalog/skillRoots.ts). The table had .kimi/skills and an empty global list. .kimi/skills is not in the engine's project tier at all, so it is stale rather than merely incomplete. (The documented first-launch migration copies the user dir ~/.kimi/skills; the project dir was never migrated, it simply stopped being scanned.)
  • deepseek had no entry either, and it is a BUILTIN_AGENTS member. dsh-skill-filesystem — mounted by the standard and code presets (packages/acp-extension-dsh/presets/*/agent.cordis.yml) with no roots configured — discovers .dsh/skills and .agents/skills per project and ~/.dsh/skills and ~/.agents/skills per user (skill-filesystem config defaults: dshHome = $DSH_HOME or ~/.dsh, agentsHome = $DSH_AGENTS_HOME or ~/.agents).

The grok symptom was worse than a missing row. getAllowedSkillMentionDirs returned an empty but non-null Set for an agent type absent from the table, and selectSkillMentionCandidates treats non-null as "apply this whitelist" — so every scanned $ skill was filtered out. A lookup miss meant "hide everything". The Skills panel, reading the same table via annotateAndSortGroups, treats a miss as merely unannotated and hides nothing, so the two surfaces disagreed about the same miss. Custom custom-<uuid> agents hit this too.

Nothing tied the table to the authoritative agent lists, so grok and deepseek could go missing without any check noticing.

Summary

  • Register grok and deepseek, and align pi / pi-acp / kimi / kimi-code / kimi-code-cli with their engines' dirs.
  • deepseek therefore gets the same treatment as the other three: it is a fourth agent that reads ~/.agents/skills while Lody could not see it. (The skills: enabled: false in profile.ts disables the demo spine's own bundled wiring — alongside toolBash, goals, and persona — because the host composition mounts the real skill registry and the presets mount the discovery and loader plugins.)
  • Return null (no whitelist) from getAllowedSkillMentionDirs for an unregistered agent type, via a new isRegisteredSkillAgentType. A registered entry that maps to nothing keeps its explicit empty whitelist — "reads no skills" and "we have no mapping" stay distinct.
  • Add a test asserting every BUILTIN_AGENTS type is registered, so a new builtin cannot repeat this.
  • Bump KNOWN_SKILL_DIRS_VERSION to 10 so cached scans re-fetch.

This revives the null change from #76 (closed unmerged under status:needs-pr-body, not on technical grounds); #150 names that same mechanism.

Before / after

Before After
$ menu showed no skills for grok: an unregistered type produced an empty whitelist that filtered every candidate. An unregistered type applies no whitelist; grok and deepseek are also registered outright.
~/.agents/skills was invisible to pi, grok, and kimi although all three load it. All three map to ~/.agents/skills.
kimi mapped to .kimi/skills and no global dir. kimi maps to .kimi-code/skills, ~/.kimi-code/skills, and ~/.agents/skills.
deepseek, a builtin, was absent, so its $ menu was empty. deepseek maps to .dsh/skills, .agents/skills, ~/.dsh/skills, ~/.agents/skills.
A builtin could be missing from the table undetected. A test fails unless every builtin is registered.

Test plan

  • pnpm --filter @lody/shared exec vitest run tests/acp-skills.test.ts — 22 passed.
  • pnpm --filter @lody/components exec vitest run tests/mention-skill-source.test.ts — 22 passed.
  • pnpm format, then pnpm typecheck, pnpm lint, pnpm lint:i18n, pnpm check:code-collab-imports, pnpm check:platform-boundaries, pnpm check:public-boundary — all clean.
  • packages/components has 19 test files / 72 tests failing on this branch, all pre-existing: the same 19/72 fail on an unmodified tree at the same base commit (use-chat-landing-defaults, vscode-diff-theme-registration and similar jsdom teardown failures, none skill-related).
  • Regression check: reverting only mention-skill-source.tsx to the base revision makes returns no whitelist for an unregistered agent type instead of an empty one fail while the other 21 cases pass, so that test fails on the old behavior through the production owner rather than a fixture.
  • Scan-set effect, diffed on the built aggregates: project gains .grok/skills, .kimi-code/skills, .cursor/skills, .dsh/skills and loses .kimi/skills; global gains ~/.grok/skills, ~/.kimi-code/skills, ~/.dsh/skills. Only .cursor/skills is new to the shared scan set — the Claude dirs were already in it via the claude entry.
  • Replaced one stale assertion that pinned kimi-code-cli to no global dirs. It now uses promptscript, which keeps the same shape under test (project dirs, zero global dirs) rather than duplicating the adjacent cline case.
  • Not done: no desktop run against live sessions. Dir facts come from each engine's source or docs.

Context handoff

Instructions for reviewing agents

  • Review focus: the entries in packages/shared/src/acp/skills.ts, and the null-vs-empty whitelist change in getAllowedSkillMentionDirs, which reaches every unregistered agent, not only grok.
  • Decisions to challenge: (1) grok's entry includes the Claude and Cursor compat roots. They are on by default in the engine but user-disablable, and the table cannot express "configurable"; including them also adds .cursor/skills to the shared project scan set. (2) deepseek's dirs hold for the standard and code presets; minimal mounts no skill plugin and cordis adds a packaged root, and this per-agent table cannot vary by preset. (3) [Bug] Skills panel and $ mention miss ~/.agents/skills for pi, grok, and kimi agents (stale per-agent skill-dir whitelist) #150 also proposed making .agents/skills an unconditional baseline for every agent. Not done: it contradicts the invariant in mentions/AGENTS.md that the dir is "a provider-specific alias, not a universal fallback".
  • Plausible failures / evidence gaps: Kimi's user dirs follow KIMI_CODE_HOME, so the hardcoded ~/.kimi-code misses a relocated data root.

Authoring context

  • User goal / directives: After the maintainer invited a PR on [Bug] Skills panel and $ mention miss ~/.agents/skills for pi, grok, and kimi agents (stale per-agent skill-dir whitelist) #150, the repo user asked whether a cleaner fix existed than patching three table rows, then asked to open the PR and resolve objections in review.
  • Constraints / non-goals: No behavior change for agents already correctly mapped; no new dir claim without a source in that engine's own source or docs.
  • Risk-bearing decisions: Widening the $ menu for unregistered agents (more candidates offered, never fewer); dropping .kimi/skills, which removes it from the shared project scan set so a project that still has one stops being surfaced, matching what the engine reads.
  • Destructive or irreversible behavior: None. No migration, no writes to user data. The version bump only discards a cache that is rebuilt on demand.
  • Deliberately not done or tested: The table stores each engine's dirs once per agent-type alias, so ~94 keys hold ~69 distinct dir tuples. Splitting engine facts from the alias mapping would remove that duplication but touches every row, so it is left out of a fix PR and can be raised separately. Kimi's extra_skill_dirs and built-in tiers are not modeled.
  • Unknowns / confidence: High confidence in the pi, grok, kimi, and deepseek mappings — grok's and kimi's come from engine source, deepseek's from its plugin config defaults plus Lody's preset composition, pi's from its documented locations. The null contract is already stated at the call site (null = show all). Lower confidence on whether the grok compat roots belong in the table given they are configurable.

@Astro-Han
Astro-Han force-pushed the fix/150-agents-skills-dirs branch 3 times, most recently from a7c5b48 to eea7f0e Compare August 31, 2026 10:24
pi loads ~/.agents/skills alongside ~/.pi/agent/skills, kimi's entry
pointed at .kimi dirs its engine no longer scans, and grok and deepseek
had no entry at all despite both being builtin agent types.

A missing entry was worse than a missing dir: getAllowedSkillMentionDirs
turned it into an empty but non-null whitelist, which filtered every $
candidate out. Return null there instead, keeping a registered empty
mapping distinct from an unregistered agent type, and add a test so a
builtin cannot go unregistered unnoticed.

Closes LodyAI#150

Model: claude-opus-5
@Astro-Han
Astro-Han force-pushed the fix/150-agents-skills-dirs branch from eea7f0e to ad7b7d4 Compare August 31, 2026 10:30

@wibus-wee wibus-wee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for your contribution!

@wibus-wee
wibus-wee merged commit 27b5623 into LodyAI:main Sep 1, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Skills panel and $ mention miss ~/.agents/skills for pi, grok, and kimi agents (stale per-agent skill-dir whitelist)

2 participants