Skip to content

fix(client): restore remote settings with lifecycle-aware service injection - #28

Merged
flymysql merged 2 commits into
flymysql:mainfrom
YiHui-Liu:fix/settings-service-lifecycle
Sep 9, 2026
Merged

flymysql merged 2 commits into
flymysql:mainfrom
YiHui-Liu:fix/settings-service-lifecycle

Conversation

@YiHui-Liu

Copy link
Copy Markdown
Contributor

Summary

Fix the missing Settings → Remote Workspace page on DSH 0.1.2-rc.1 and profiles using replacement workspace/sidebar components.

  • Declare the client plugin's required slots and locale services so apply() waits for them instead of permanently returning before slots are available.
  • Replace legacy dsh-client-runtime / native dsh-client-ui-workspace metadata with the current renderer and locale packages; keep npm lockfile peer metadata consistent.
  • Track sessions and betterSidebar as optional, lifecycle-scoped integrations; clear stale sessions when the provider unloads.
  • Wait for the two directory-flow slots independently so one missing slot cannot prevent the other from working.
  • Use order: 40 for the settings navigation entry and remove the unused workspaces lookup.
  • Add eight portable full-bundle VM regression tests, including delayed registration, provider removal/re-addition, and disposal.

Validation

  • node --test test/client-lifecycle.test.js test/i18n.test.js: 12 passed.
  • node check.mjs, node --check lib/client.js, git diff --check: passed.
  • Existing DSH 0.1.2-rc.1 GUI: verified the settings entry, machine form and port-forward UI in Chromium, including a page reload, with no page errors. No SSH credentials submitted or remote operations performed.
  • npm ci --legacy-peer-deps --no-audit --no-fund completed. Linked the locally installed DSH 0.1.2-rc.1 host peer packages for integration tests; npm test: 87 passed, 0 failed.
  • Real Cordis lifecycle checks passed: delayed service activation, settings without optional integrations, and disposal.
  • Installed this checkout through a persistent link: dependency in the existing Web profile; host import and refreshed settings page verified.

Related to #26.

YiHui-Liu and others added 2 commits September 8, 2026 09:49
…pm test works standalone

Since 0.8.10 (upload.test.js) and 0.8.12 (session-routing.test.js) import
lib/index.js at module scope, a clean `npm ci --legacy-peer-deps` in CI
fails with ERR_MODULE_NOT_FOUND: the host half statically imports
@deepseek-ai/dsh-tools (peerDependency), whose module graph also loads
@deepseek-ai/cordis, dsh-scope, dsh-llm, dsh-session and dsh-timeout.
npm never installs peer-only packages under --legacy-peer-deps, so CI
could not import the plugin's own host code. Local runs were masking
this via profile-linked node_modules.

Declare that import-time closure as devDependencies (never shipped:
files: lib + cordis.patch.yml). npm ci now installs it for the test
environment; production still receives the real peers from the dsh
host at load time. Fixes CI on main (red since 0.8.10) and for every PR.
@flymysql

flymysql commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Reviewed & CI-fixed. The client-lifecycle change itself is correct and well-tested — here's the state:

Code review — approves

Verified against the live dsh 0.1.2-rc.1 client contract (real rc.1 packages + the slot system's declared registration options):

  • exports.inject = ['slots', 'locale'] matches reality: settings.section and the two directoryFlow holes are declared by rc.1 ui-settings/ui-workspace packages, and both services are hard requirements.
  • settings.section is a list-kind slot sorted by order (not priority — the old priority: 40 occupant showed up with no order and sorted as 0). The order: 40 change is a real fix; it lands the entry between dsh-remote-debug (50) and the shipped pages (0/10/15/20).
  • Splitting the two directoryFlow slots.inject calls means one missing seat can no longer suppress the other; the -100 priority preserves the low-priority fill intent.
  • ctx.inject on sessions/betterSidebar keeps them optional and disposes them correctly; removing the WORKSPACES dead lookup and the if (slots === undefined) return early-return removes the "apply returns before slots arrive → page never registers" failure mode.
  • The peer manifest swap (dsh-client-ui-renderer/dsh-client-locale replacing the removed dsh-client-runtime/dsh-client-ui-workspace native packages) matches what rc.1 actually ships.

CI was red — but not because of this PR

The 6 failures (session-routing.test.js ×5, upload.test.js) were ERR_MODULE_NOT_FOUND: Cannot find package '@deepseek-ai/dsh-tools'. Root cause is pre-existing on main since 0.8.10 (bd61cc4b, 2026-08-31): upload.test.js was the first test to import { apply } from '../lib/index.js' at module scope, which statically imports @deepseek-ai/dsh-tools — only a peerDependency, so npm ci --legacy-peer-deps never installs it. session-routing.test.js (0.8.12) hit the same wall. Local runs masked this via profile-linked node_modules.

CI fix (pushed to this branch, 2743520)

dsh-tools's module graph statically loads @deepseek-ai/cordis, dsh-scope, dsh-llm, dsh-session, dsh-timeout. Declared that import-time closure as devDependenciesfiles: ["lib","cordis.patch.yml"] means it never ships; production still gets the real peers from the dsh host. Verified in a clean LF checkout: npm ci --legacy-peer-deps && node --check lib/* && node check.mjs && npm test87/87 pass. CI on this PR is now green, and the fix also unblocks main CI (red since 0.8.10) for future PRs.

Thanks @YiHui-Liu for the fix and for the thorough VM-based lifecycle tests.

@flymysql
flymysql merged commit 411e16b into flymysql:main Sep 9, 2026
1 check passed
flymysql pushed a commit that referenced this pull request Sep 9, 2026
…1 (PR #28)

- client.js: exports.inject=['slots','locale'] hard deps (remove the
  `if (slots === undefined) return` early-out that skipped registration)
- settings.section registers with order:40 (rc.1 list slots sort by
  `order`; the old priority:40 was a dead field sorting as 0)
- conversation.hero + sidebar directoryFlow seats inject independently
- sessions/betterSidebar via ctx.inject optional lifecycle; drop the
  dead WORKSPACES lookup
- manifest: dsh.client.inject + peers swap deprecated
  dsh-client-runtime/dsh-client-ui-workspace for rc.1
  dsh-client-ui-renderer/dsh-client-locale @0.1.2-rc.1
- test/client-lifecycle.test.js: 8 VM full-bundle lifecycle tests
- CI: declare the host peer import-time closure (dsh-tools/cordis/
  dsh-scope/dsh-llm/dsh-session/dsh-timeout) as devDependencies so
  `npm ci --legacy-peer-deps && npm test` works standalone (main CI
  was red since 0.8.10)
- lockfile regenerated; version bump 0.8.14
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.

2 participants