Skip to content

fix(dsh): honor configured skill installation paths - #827

Open
specetator wants to merge 1 commit into
zilliztech:mainfrom
specetator:dot/fix-dsh-skill-install-paths-20261029
Open

specetator wants to merge 1 commit into
zilliztech:mainfrom
specetator:dot/fix-dsh-skill-install-paths-20261029

Conversation

@specetator

Copy link
Copy Markdown

Summary

  • Respect the first configured plugins.dsh.memory_to_skill.paths destination when installing a skill from the DSH review dock.
  • Consume the existing memsearch config list --json-output interface, instead of parsing config get's Python list representation as JSON.
  • Resolve relative destinations against the session project directory and preserve absolute/home-relative destinations, without changing Python configuration semantics or the project-config trust boundary.

Problem and production evidence

The DSH platform reference explicitly says configured relative destinations resolve against the project directory. The real skills install implementation follows the same rule after expanding ~.

The production /memsearch-dsh/skill-review install handler calls resolveSkillInstallTarget, then starts the actual CLI with skills install NAME --path TARGET and the session's project cwd. For ordinary configured paths, config get returns Python repr (for example ['./configured-skills']), which the old resolver cannot parse as JSON. A real CLI + registered production route-handler test confirmed that the candidate was installed in ~/.agents/skills instead of the configured directory.

A path containing a single quote can make Python repr valid JSON. Relative paths of this kind already worked and are preserved. Absolute paths of this kind exposed the second bug: join(first, first) duplicated the absolute path. The regression fixture reflects these different real CLI formats rather than assuming all configured paths failed.

Validation

  • Focused regression against original production code: 5 passed / 5 failed; same 10 tests with this fix: 10 passed.
  • Full DSH Node suite: 88 passed / 1 Windows-only skipped.
  • uv run python -m pytest tests/test_skills.py tests/test_config.py -q: 71 passed, under an isolated HOME.
  • Real CLI + production registered install handler: six cases (relative, absolute, ~/, quotes/spaces, first of multiple paths, empty/default), verifying actual installed files and their contents. In all six, untrusted project config could not override the global plugin path.
  • Independent AI technical review separately exercised actual CLI installation and original-code behavior; a fixture inconsistency it found was corrected and re-reviewed.
  • Node syntax checks, ruff check src/ tests/, ruff format --check src/ tests/, and git diff --check passed.

Limitations and AI disclosure

This change and its tests were prepared with AI assistance and independently reviewed by another AI agent; this is not human review. Validation was on Linux only. The route integration uses fixture HTTP request/response and DSH context objects but real production route registration, configuration CLI, installation CLI, and filesystem operations. It is not a live browser or complete DSH-host lifecycle test. No real model/provider calls were made, and the full Python suite/all-extras and Windows runtime were not run.

An initial associated Python test run hit the environment's read-only default home directory (70 passed / 1 environment failure); rerunning with a dedicated writable test HOME passed all 71 without changing production code or permissions.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant