morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4419] [CLOSED] fix(setup): install the DeepSeek Harness skill in the client's own root #6229

New issue

Closed

opened 2026-10-31 01:12:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:51 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4419
Author: @cainiao1992
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/dsh-native-skill-root

📝 Commits (1)

490ec4c fix(setup): install the DeepSeek Harness skill in the client's own root

📊 Changes

8 files changed (+69 additions, -70 deletions)

View changed files

📝 docs/client-configuration.md (+9 -9)
📝 docs/client-verification.md (+3 -3)
📝 scripts/verify-deepseek-client.mjs (+2 -4)
📝 src/application/SupportedClients.ts (+8 -14)
📝 tests/acceptance/setup/dshClientSetup.test.ts (+8 -10)
📝 tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts (+3 -3)
📝 tests/boundary/filesystem/dshClientConfiguration.test.ts (+36 -26)
📝 tests/fixtures/clientEnvironment.ts (+0 -1)

📄 Description

Summary

Related to #4402 (design discussion; not marked Fixes so the maintainer decision stays visible).

rea setup --client dsh installed the bundled skill into the shared ~/.agents/skills root. That root is where REA points kimi, pi, qwen_code, codex, omp and grok_build, so a Harness-only installation also changed six other clients' skill catalogs. Harness ranks its own user root <dshHome>/skills (rank 400 user-dsh) above the shared root (rank 500 user-agents), and the shared root's own contract calls it "configuration shared with other agent tools". The skill now lands in the client's own root.

Problem and expected behavior

Trigger: rea setup --client dsh on a machine that does not use the shared root.

Before: ~/.agents/skills/reverse-engineer-anything is created, and every client that discovers that root gains the skill. After: $DSH_HOME/skills/reverse-engineer-anything (default ~/.dsh/skills) is created, and ~/.agents/skills is untouched.

The violated invariant is ownership of a shared directory: REA wrote a client-specific installation into cross-client storage that the requesting client does not own, even though the client offers a higher-priority user root of its own.

Change and scope

src/application/SupportedClients.ts: the dsh destination resolves to join(dshHomeDirectory(context), "skills"), reusing the same Harness home resolution the Cordis patch path already uses.
REA no longer reads DSH_AGENTS_HOME. After this change the variable describes only the shared agents root, which the shared skill scope still maps to ~/.agents/skills; --skill-client shared and --skill-client dsh now name two distinct directories.
The previous two-root path helper had a single remaining caller, so it is folded into dshHomeDirectory instead of keeping an unused fallback parameter.
Setup plan action id for this destination becomes install_skill:dsh (was install_skill, the shared-root spelling). This matches install_skill:claude_code and the other client-specific destinations; install actions are validated by target, not by id.
Not included: no migration of existing shared-root copies (setup already documents that it leaves skill copies in other locations untouched), no new CLI flag or selection scope, no change to MCP registration, Harness home resolution, other clients' roots, or provider behavior.

Contract and boundary impact

Semantic owner and earliest changed stage: the dsh skill destination in src/application/SupportedClients.ts; destination resolution happens before setup planning and before any filesystem inspection.
Contribution scope fit and linked maintainer decision: an ordinary correction inside the existing client-configuration integration; no new provider, capability, target execution, injection, mutation, or protection recovery. The scope question is the open decision in #4402.
CLI and MCP/tool-catalog contract: no new commands, flags, tools, or prompts. The only user-visible change is the destination and the install_skill:dsh action id in setup JSON output.
Provider, bridge, target-format, or platform compatibility: none. Destination resolution continues to mirror Harness home-paths semantics for DSH_HOME (blank treated as unset, ~/~//~\ expansion, relative values against cwd, nonblank values keep their spaces).
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: REA writes only under $DSH_HOME/skills; no target process, network, or target file is involved. uninstall removes the REA-managed skill files and the registration as before; it leaves $DSH_HOME/cordis.patch.yml emptied to [] and keeps cordis.patch.yml.rea.backup, both unchanged behavior.
Generated metadata: no client was added and no skill content changed, so docs/public/product-catalog.json and skills/ are unaffected; docs:check validates them.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: the dsh boundary path matrix now expects $DSH_HOME/skills; a new case sets DSH_AGENTS_HOME and asserts the dsh destination ignores it; the two dsh acceptance journeys install, doctor, repeat, and uninstall through the new destination; tests/fixtures/clientEnvironment.ts drops the retired variable; the Claude Desktop MSIX skill fixture moves from DSH_AGENTS_HOME to DSH_HOME.
Base reproduction: on the base revision the updated boundary case fails with Received: {client: "shared", directory: "/home/a/.agents/skills"} against Expected: {client: "dsh", directory: "/home/a/.dsh/skills"} (10 failures), and the dsh journey fails on install_skill versus install_skill:dsh. Both were watched before the source change.

User-visible CLI/MCP output: setup --client dsh --dry-run --json now plans

[
  { "id": "configure_client:dsh", "target": "/Users/<user>/.dsh/cordis.patch.yml", "operation": "create" },
  { "id": "install_skill:dsh", "target": "/Users/<user>/.dsh/skills/reverse-engineer-anything", "operation": "install" }
]

Remaining proof gaps: Windows is unverified; live DeepSeek API compatibility is unverified; the native lane still reports passed: false on a pre-existing macOS-only /private/var versus /var mismatch in its local_path assertion, reproduced identically on the unmodified base revision. The complete check:pr gate has not been run.

Observed, derived, and inferred claims stay distinguishable above: the Harness ranking, the variable semantics, and the shared-root reachability are derived from the client's source; the install results and command outputs are observed; a maintainer's preference between this default and an opt-in selection is inferred and left to #4402. The artifact/provenance items below are not applicable rather than unverified: this change moves a configuration destination and touches no evidence or artifact semantics.

Validation performed

npm run test:focused -- tests/boundary/filesystem/dshClientConfiguration.test.ts tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts tests/boundary/filesystem/kimiClientConfiguration.test.ts tests/boundary/filesystem/setupSkill.test.ts tests/boundary/filesystem/generatedSkill.test.ts tests/boundary/filesystem/clientLifecycle.test.ts tests/acceptance/setup/dshClientSetup.test.ts tests/acceptance/setup/selectedSkillSetup.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts — 10 files, 258 tests passed.
npm run test:focused -- tests/process-global/updateMaintenance.test.ts tests/boundary/cli/clientProfiles.test.ts tests/boundary/cli/cliSetup.test.ts tests/acceptance/setup/qwenProfileSetup.test.ts tests/acceptance/setup/piClientSetup.test.ts — 5 files, 64 tests passed (an added skill destination changes the update-maintenance surface, so it is covered explicitly).
npm run check:fast — typecheck and lint passed.
npm run docs:check — generated metadata and doc facts passed.
npm run test:boundary — 7039 passed, 327 skipped, 8 failed in 5 files on macOS 15 arm64. The failures visible in the output come from this machine's environment: a third-party IDE Node shim intercepts subprocess deletion (ERANGE, ENAMETOOLONG) and a path-length-limit assertion depends on the host. I did not enumerate all eight by name, and I did not compare them against a base-revision run of the same command.
npm run verify:deepseek-client — reached skillLoaded: true through the new root with a complete 207-tool catalog and the dry-run, setup, idempotent-setup and doctor lifecycle; the lane ends passed: false on the macOS /private/var local_path assertion. Stashing this change and rerunning the lane on the base revision produces the same failure, so it is unrelated.
Real installation on macOS 15 arm64, Harness 0.2.0-rc.2, REA 6.5.0 from a checkout: setup --client dsh --yes wrote $DSH_HOME/cordis.patch.yml, installed the skill to ~/.dsh/skills/reverse-engineer-anything, left ~/.agents/skills untouched, and reported ready with the registration and skill aligned. The generated launcher answered a stdio handshake with the full tool catalog, dsh --profile web --dump-config composed the home patch, and the client loaded the skill and served REA tools after a restart. uninstall then removed the registration and the skill files.

Compatibility, safety, and release

Breaking changes or migration steps: not marked breaking; no command or schema changes. Behavior change: a Harness-only installation no longer writes the shared agents root. An existing installation in ~/.agents/skills is not moved (setup leaves skill copies in other locations untouched, and uninstall still removes that copy through the shared root), so a machine that installed before this change keeps a copy there until it is uninstalled. After upgrading, doctor reports the skill as missing at the new destination until setup runs again. The plan action id for this destination changes from install_skill to install_skill:dsh.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider or target workflow changed.
Package or release metadata impact: none; no generated catalog, package metadata, or skill content changed.
Security, privacy, process, or containment review: writes stay inside the client's own user root, no new network, credential, or authority access; the change removes a write into cross-client storage.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4419 **Author:** [@cainiao1992](https://github.com/cainiao1992) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/dsh-native-skill-root` --- ### 📝 Commits (1) - [`490ec4c`](https://github.com/morluto/rea/commit/490ec4c7dd9b8d4c84ed451c1a362f6a76c2b7cf) fix(setup): install the DeepSeek Harness skill in the client's own root ### 📊 Changes **8 files changed** (+69 additions, -70 deletions) <details> <summary>View changed files</summary> 📝 `docs/client-configuration.md` (+9 -9) 📝 `docs/client-verification.md` (+3 -3) 📝 `scripts/verify-deepseek-client.mjs` (+2 -4) 📝 `src/application/SupportedClients.ts` (+8 -14) 📝 `tests/acceptance/setup/dshClientSetup.test.ts` (+8 -10) 📝 `tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts` (+3 -3) 📝 `tests/boundary/filesystem/dshClientConfiguration.test.ts` (+36 -26) 📝 `tests/fixtures/clientEnvironment.ts` (+0 -1) </details> ### 📄 Description ## Summary Related to #4402 (design discussion; not marked `Fixes` so the maintainer decision stays visible). `rea setup --client dsh` installed the bundled skill into the shared `~/.agents/skills` root. That root is where REA points kimi, pi, qwen_code, codex, omp and grok_build, so a Harness-only installation also changed six other clients' skill catalogs. Harness ranks its own user root `<dshHome>/skills` (rank 400 `user-dsh`) above the shared root (rank 500 `user-agents`), and the shared root's own contract calls it "configuration shared with other agent tools". The skill now lands in the client's own root. ## Problem and expected behavior Trigger: `rea setup --client dsh` on a machine that does not use the shared root. Before: `~/.agents/skills/reverse-engineer-anything` is created, and every client that discovers that root gains the skill. After: `$DSH_HOME/skills/reverse-engineer-anything` (default `~/.dsh/skills`) is created, and `~/.agents/skills` is untouched. The violated invariant is ownership of a shared directory: REA wrote a client-specific installation into cross-client storage that the requesting client does not own, even though the client offers a higher-priority user root of its own. ## Change and scope - `src/application/SupportedClients.ts`: the `dsh` destination resolves to `join(dshHomeDirectory(context), "skills")`, reusing the same Harness home resolution the Cordis patch path already uses. - REA no longer reads `DSH_AGENTS_HOME`. After this change the variable describes only the shared agents root, which the `shared` skill scope still maps to `~/.agents/skills`; `--skill-client shared` and `--skill-client dsh` now name two distinct directories. - The previous two-root path helper had a single remaining caller, so it is folded into `dshHomeDirectory` instead of keeping an unused `fallback` parameter. - Setup plan action id for this destination becomes `install_skill:dsh` (was `install_skill`, the shared-root spelling). This matches `install_skill:claude_code` and the other client-specific destinations; install actions are validated by target, not by id. - Not included: no migration of existing shared-root copies (setup already documents that it leaves skill copies in other locations untouched), no new CLI flag or selection scope, no change to MCP registration, Harness home resolution, other clients' roots, or provider behavior. ## Contract and boundary impact - Semantic owner and earliest changed stage: the dsh skill destination in `src/application/SupportedClients.ts`; destination resolution happens before setup planning and before any filesystem inspection. - Contribution scope fit and linked maintainer decision: an ordinary correction inside the existing client-configuration integration; no new provider, capability, target execution, injection, mutation, or protection recovery. The scope question is the open decision in #4402. - CLI and MCP/tool-catalog contract: no new commands, flags, tools, or prompts. The only user-visible change is the destination and the `install_skill:dsh` action id in `setup` JSON output. - Provider, bridge, target-format, or platform compatibility: none. Destination resolution continues to mirror Harness `home-paths` semantics for `DSH_HOME` (blank treated as unset, `~`/`~/`/`~\` expansion, relative values against cwd, nonblank values keep their spaces). - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: REA writes only under `$DSH_HOME/skills`; no target process, network, or target file is involved. `uninstall` removes the REA-managed skill files and the registration as before; it leaves `$DSH_HOME/cordis.patch.yml` emptied to `[]` and keeps `cordis.patch.yml.rea.backup`, both unchanged behavior. - Generated metadata: no client was added and no skill content changed, so `docs/public/product-catalog.json` and `skills/` are unaffected; `docs:check` validates them. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: the dsh boundary path matrix now expects `$DSH_HOME/skills`; a new case sets `DSH_AGENTS_HOME` and asserts the dsh destination ignores it; the two dsh acceptance journeys install, doctor, repeat, and uninstall through the new destination; `tests/fixtures/clientEnvironment.ts` drops the retired variable; the Claude Desktop MSIX skill fixture moves from `DSH_AGENTS_HOME` to `DSH_HOME`. - Base reproduction: on the base revision the updated boundary case fails with `Received: {client: "shared", directory: "/home/a/.agents/skills"}` against `Expected: {client: "dsh", directory: "/home/a/.dsh/skills"}` (10 failures), and the dsh journey fails on `install_skill` versus `install_skill:dsh`. Both were watched before the source change. - User-visible CLI/MCP output: `setup --client dsh --dry-run --json` now plans ```json [ { "id": "configure_client:dsh", "target": "/Users/<user>/.dsh/cordis.patch.yml", "operation": "create" }, { "id": "install_skill:dsh", "target": "/Users/<user>/.dsh/skills/reverse-engineer-anything", "operation": "install" } ] ``` - Remaining proof gaps: Windows is unverified; live DeepSeek API compatibility is unverified; the native lane still reports `passed: false` on a pre-existing macOS-only `/private/var` versus `/var` mismatch in its `local_path` assertion, reproduced identically on the unmodified base revision. The complete `check:pr` gate has not been run. Observed, derived, and inferred claims stay distinguishable above: the Harness ranking, the variable semantics, and the shared-root reachability are derived from the client's source; the install results and command outputs are observed; a maintainer's preference between this default and an opt-in selection is inferred and left to #4402. The artifact/provenance items below are not applicable rather than unverified: this change moves a configuration destination and touches no evidence or artifact semantics. ## Validation performed - `npm run test:focused -- tests/boundary/filesystem/dshClientConfiguration.test.ts tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts tests/boundary/filesystem/kimiClientConfiguration.test.ts tests/boundary/filesystem/setupSkill.test.ts tests/boundary/filesystem/generatedSkill.test.ts tests/boundary/filesystem/clientLifecycle.test.ts tests/acceptance/setup/dshClientSetup.test.ts tests/acceptance/setup/selectedSkillSetup.test.ts tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts` — 10 files, 258 tests passed. - `npm run test:focused -- tests/process-global/updateMaintenance.test.ts tests/boundary/cli/clientProfiles.test.ts tests/boundary/cli/cliSetup.test.ts tests/acceptance/setup/qwenProfileSetup.test.ts tests/acceptance/setup/piClientSetup.test.ts` — 5 files, 64 tests passed (an added skill destination changes the update-maintenance surface, so it is covered explicitly). - `npm run check:fast` — typecheck and lint passed. - `npm run docs:check` — generated metadata and doc facts passed. - `npm run test:boundary` — 7039 passed, 327 skipped, 8 failed in 5 files on macOS 15 arm64. The failures visible in the output come from this machine's environment: a third-party IDE Node shim intercepts subprocess deletion (`ERANGE`, `ENAMETOOLONG`) and a path-length-limit assertion depends on the host. I did not enumerate all eight by name, and I did not compare them against a base-revision run of the same command. - `npm run verify:deepseek-client` — reached `skillLoaded: true` through the new root with a complete 207-tool catalog and the dry-run, setup, idempotent-setup and doctor lifecycle; the lane ends `passed: false` on the macOS `/private/var` `local_path` assertion. Stashing this change and rerunning the lane on the base revision produces the same failure, so it is unrelated. - Real installation on macOS 15 arm64, Harness `0.2.0-rc.2`, REA `6.5.0` from a checkout: `setup --client dsh --yes` wrote `$DSH_HOME/cordis.patch.yml`, installed the skill to `~/.dsh/skills/reverse-engineer-anything`, left `~/.agents/skills` untouched, and reported `ready` with the registration and skill `aligned`. The generated launcher answered a stdio handshake with the full tool catalog, `dsh --profile web --dump-config` composed the home patch, and the client loaded the skill and served REA tools after a restart. `uninstall` then removed the registration and the skill files. ## Compatibility, safety, and release - Breaking changes or migration steps: not marked breaking; no command or schema changes. Behavior change: a Harness-only installation no longer writes the shared agents root. An existing installation in `~/.agents/skills` is not moved (setup leaves skill copies in other locations untouched, and uninstall still removes that copy through the shared root), so a machine that installed before this change keeps a copy there until it is uninstalled. After upgrading, doctor reports the skill as missing at the new destination until setup runs again. The plan action id for this destination changes from `install_skill` to `install_skill:dsh`. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider or target workflow changed. - Package or release metadata impact: none; no generated catalog, package metadata, or skill content changed. - Security, privacy, process, or containment review: writes stay inside the client's own user root, no new network, credential, or authority access; the change removes a write into cross-client storage. ## Author readiness  Human owner: - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:51 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:11 +00:00

[PR #6229] [MERGED] fix(pcap): report workspace allocation failures by their system cause #7212

bsakaguchi changed title from ~~[PR #4419] [CLOSED] fix(setup): install the DeepSeek Harness skill in the client's own root~~ to [PR #4419] fix(setup): install the DeepSeek Harness skill in the client's own root

2026-11-03 13:32:19 +00:00

bsakaguchi changed title from ~~[PR #4419] fix(setup): install the DeepSeek Harness skill in the client's own root~~ to [PR #4419] [CLOSED] fix(setup): install the DeepSeek Harness skill in the client's own root

2026-11-03 14:34:03 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6229

No description provided.

Rows
Columns