From 67419be9fab04b2bbdadcd5b13890ac2fcd6cc8e Mon Sep 17 00:00:00 2001 From: Qiuner Date: Wed, 19 Aug 2026 18:29:59 +0800 Subject: [PATCH] refactor(desktop): own recoverable plugin install lifecycle --- dsh-community-market/src/install/service.ts | 27 ++++---- .../tests/market-install.spec.ts | 10 ++- dsh-plugin-desktop/src/pnpm.ts | 62 +++++++++++++------ dsh-plugin-desktop/tests/pnpm.spec.ts | 40 ++++++++---- 4 files changed, 94 insertions(+), 45 deletions(-) diff --git a/dsh-community-market/src/install/service.ts b/dsh-community-market/src/install/service.ts index 998ce3a4a6..f6d2fb23a4 100644 --- a/dsh-community-market/src/install/service.ts +++ b/dsh-community-market/src/install/service.ts @@ -54,16 +54,16 @@ export interface MarketDesktopPnpm { invokingDir: string, signal?: AbortSignal, ): MarketDesktopPnpmHandle - runPluginInstall( - args: readonly string[], - invokingDir: string, - recovery: { + installPlugin(request: { + readonly pnpmOptions?: readonly string[] + readonly invokingDir: string + readonly recovery: { readonly packageName: string readonly packageVersion: string readonly receiptId: string - }, - signal?: AbortSignal, - ): Promise + } + readonly signal?: AbortSignal + }): Promise recoveredInstallReceiptIds(): Promise acknowledgeRecoveredInstall(receiptId: string): Promise rollbackPluginInstall(receiptId: string): Promise @@ -850,7 +850,7 @@ export class MarketInstallService { } try { await this.runPlugin( - this.installArgs(candidate.packageName, candidate.version), + this.installOptions(candidate.packageName), profile, operationSignal, true, @@ -1162,7 +1162,12 @@ export class MarketInstallService { try { handle = installRecovery === undefined ? this.pnpm.runPlugin(args, profile.dir, combinedSignal) - : await this.pnpm.runPluginInstall(args, profile.dir, installRecovery, combinedSignal) + : await this.pnpm.installPlugin({ + pnpmOptions: args, + invokingDir: profile.dir, + recovery: installRecovery, + signal: combinedSignal, + }) } catch { throw new MarketInstallError('operation-failed', 'The desktop package manager could not start.') } handle.stdout.resume() @@ -1182,14 +1187,12 @@ export class MarketInstallService { } } - private installArgs(packageName: string, version: string): readonly string[] { + private installOptions(packageName: string): readonly string[] { const scope = packageName.startsWith('@') ? packageName.split('/', 1)[0] : undefined return [ - 'add', '--save-exact', `--registry=${NPM_REGISTRY}`, ...(scope === undefined ? [] : [`--${scope}:registry=${NPM_REGISTRY}`]), - `${packageName}@${version}`, ] } diff --git a/dsh-community-market/tests/market-install.spec.ts b/dsh-community-market/tests/market-install.spec.ts index d68a5ada8a..051a01284c 100644 --- a/dsh-community-market/tests/market-install.spec.ts +++ b/dsh-community-market/tests/market-install.spec.ts @@ -242,9 +242,13 @@ function recoverableRunner( let pendingPackageName: string | undefined return { ...implementation, - async runPluginInstall(args, dir, recovery, signal) { - pendingPackageName = recovery.packageName - return implementation.runPlugin(args, dir, signal) + async installPlugin(request) { + pendingPackageName = request.recovery.packageName + return implementation.runPlugin([ + 'add', + ...(request.pnpmOptions ?? []), + `${request.recovery.packageName}@${request.recovery.packageVersion}`, + ], request.invokingDir, request.signal) }, async recoveredInstallReceiptIds() { return [] }, async acknowledgeRecoveredInstall() {}, diff --git a/dsh-plugin-desktop/src/pnpm.ts b/dsh-plugin-desktop/src/pnpm.ts index 98ec07036e..a01835eb16 100644 --- a/dsh-plugin-desktop/src/pnpm.ts +++ b/dsh-plugin-desktop/src/pnpm.ts @@ -11,7 +11,6 @@ import type { } from '@deepseek-ai/dsh-subprocess' import { DesktopInstallRecoveryStore, - type BeginDesktopInstallRecoveryInput, } from './install-recovery.ts' import { assertDesktopProfileName } from './profile-manager.ts' @@ -67,6 +66,34 @@ export interface DesktopPnpmHandle { cancel(): void } +/** Receipt identity tied to one recoverable plugin installation. */ +export interface DesktopPluginInstallRecovery { + readonly packageName: string + readonly packageVersion: string + /** Host-generated before installation so every crash window can reconcile the receipt. */ + readonly receiptId: string +} + +/** Complete request for one Desktop-owned, recoverable plugin installation. */ +export interface DesktopPluginInstallRequest { + /** pnpm flags after the enforced `add` command and before the exact generated target. */ + readonly pnpmOptions?: readonly string[] + /** Absolute caller directory used to anchor relative package specifications. */ + readonly invokingDir: string + readonly recovery: DesktopPluginInstallRecovery + readonly signal?: AbortSignal +} + +/** Public package-operation interface for one immutable Desktop profile generation. */ +export interface DesktopPnpm { + run(args: readonly string[], signal?: AbortSignal): DesktopPnpmHandle + runPlugin(args: readonly string[], invokingDir: string, signal?: AbortSignal): DesktopPnpmHandle + installPlugin(request: DesktopPluginInstallRequest): Promise + recoveredInstallReceiptIds(): Promise + acknowledgeRecoveredInstall(receiptId: string): Promise + rollbackPluginInstall(receiptId: string): Promise +} + declare module '@deepseek-ai/cordis' { interface Context { /** Launcher-private inputs from which the Host provider constructs the service. */ @@ -130,8 +157,8 @@ function validateBootstrap(bootstrap: DesktopPnpmBootstrap): void { } } -/** Host service providing one managed pnpm operation at a time. */ -export class DesktopPnpm extends Service { +/** Cordis adapter implementing the public Desktop package-operation interface. */ +class DesktopPnpmService extends Service implements DesktopPnpm { private active: ActiveOperation | undefined private installPreparationActive = false private closed = false @@ -220,26 +247,21 @@ export class DesktopPnpm extends Service { * Snapshot the active profile before running one `dsh plugin add` operation. * The returned handle seals the post-install image before `done` resolves. */ - async runPluginInstall( - args: readonly string[], - invokingDir: string, - recovery: BeginDesktopInstallRecoveryInput, - signal?: AbortSignal, - ): Promise { - const resolvedArgs = validatedArgs(args) - if (resolvedArgs[0] !== 'add') { - throw new Error(`${BIN_NAME}: recoverable plugin install requires the add command`) + async installPlugin(request: DesktopPluginInstallRequest): Promise { + const resolvedOptions = request.pnpmOptions === undefined ? [] : [...request.pnpmOptions] + if (resolvedOptions.some(argument => argument.includes('\0'))) { + throw new Error(`${BIN_NAME}: desktop pnpm arguments must not contain NUL`) } - assertAbsolutePath('plugin invoking directory', invokingDir) + assertAbsolutePath('plugin invoking directory', request.invokingDir) if (this.closed) throw new Error(`${BIN_NAME}: desktop pnpm generation is closed`) if (this.active !== undefined || this.installPreparationActive) { throw new Error(`${BIN_NAME}: another desktop pnpm operation is already running`) } - signal?.throwIfAborted() + request.signal?.throwIfAborted() this.installPreparationActive = true let transaction: Awaited> | undefined try { - transaction = await this.installRecovery.begin(recovery) + transaction = await this.installRecovery.begin(request.recovery) const handle = this.start({ argv: [ this.bootstrap.appExecutable, @@ -248,12 +270,14 @@ export class DesktopPnpm extends Service { 'plugin', '--profile', this.bootstrap.activeProfileName, - ...resolvedArgs, + 'add', + ...resolvedOptions, + `${request.recovery.packageName}@${request.recovery.packageVersion}`, ], - cwd: invokingDir, + cwd: request.invokingDir, recoveryTransactionId: transaction.transactionId, allowInstallPreparation: true, - ...(signal === undefined ? {} : { signal }), + ...(request.signal === undefined ? {} : { signal: request.signal }), }) this.installPreparationActive = false return handle @@ -404,5 +428,5 @@ export const inject = ['desktopPnpmBootstrap', 'subprocess'] * @param ctx - Host context carrying launcher bootstrap values and subprocess ownership. */ export function apply(ctx: Context): void { - new DesktopPnpm(ctx, ctx.desktopPnpmBootstrap) + new DesktopPnpmService(ctx, ctx.desktopPnpmBootstrap) } diff --git a/dsh-plugin-desktop/tests/pnpm.spec.ts b/dsh-plugin-desktop/tests/pnpm.spec.ts index 131cc48ce4..1ce9234866 100644 --- a/dsh-plugin-desktop/tests/pnpm.spec.ts +++ b/dsh-plugin-desktop/tests/pnpm.spec.ts @@ -201,15 +201,15 @@ describe('desktop pnpm Host service', () => { writeFileSync(manifestPath, JSON.stringify({ dependencies: {} })) const harness = await createHarness([child], selectedBootstrap) - const pending = harness.service.runPluginInstall( - ['add', '--save-exact', 'example-plugin@1.0.0'], - '/workspace', - { + const pending = harness.service.installPlugin({ + pnpmOptions: ['--save-exact'], + invokingDir: '/workspace', + recovery: { packageName: 'example-plugin', packageVersion: '1.0.0', receiptId: 'receipt:test-install-0001', }, - ) + }) expect(() => harness.service.runPlugin(['remove', 'other-plugin'], '/workspace')).toThrow( 'another desktop pnpm operation is already running', ) @@ -218,7 +218,17 @@ describe('desktop pnpm Host service', () => { finish(child) await expect(operation.done).resolves.toEqual({ exitCode: 0, signal: null }) - expect(harness.spawn.mock.calls[0]?.[0].argv).toContain('example-plugin@1.0.0') + expect(harness.spawn.mock.calls[0]?.[0].argv).toEqual([ + selectedBootstrap.appExecutable, + '--expose-internals', + selectedBootstrap.dshBootstrapPath, + 'plugin', + '--profile', + selectedBootstrap.activeProfileName, + 'add', + '--save-exact', + 'example-plugin@1.0.0', + ]) expect(JSON.parse(readFileSync(selectedBootstrap.installRecoveryStatePath, 'utf8'))).toMatchObject({ packageName: 'example-plugin', packageVersion: '1.0.0', @@ -241,15 +251,14 @@ describe('desktop pnpm Host service', () => { mkdirSync(selectedBootstrap.activeProfileDir, { recursive: true }) writeFileSync(manifestPath, originalManifest) const harness = await createHarness([child], selectedBootstrap) - const operation = await harness.service.runPluginInstall( - ['add', 'broken-plugin@1.0.0'], - '/workspace', - { + const operation = await harness.service.installPlugin({ + invokingDir: '/workspace', + recovery: { packageName: 'broken-plugin', packageVersion: '1.0.0', receiptId: 'receipt:test-install-failure-0001', }, - ) + }) writeFileSync(manifestPath, JSON.stringify({ dependencies: { 'broken-plugin': '1.0.0' } })) finish(child, { exitCode: 1, signal: null }) @@ -276,6 +285,15 @@ describe('desktop pnpm Host service', () => { expect(() => harness.service.runPlugin(['add', 'plugin'], '/workspace')).toThrow( 'plugin add must use the recoverable install boundary', ) + await expect(harness.service.installPlugin({ + pnpmOptions: ['--registry=https://registry.example\0.invalid'], + invokingDir: '/workspace', + recovery: { + packageName: 'plugin', + packageVersion: '1.0.0', + receiptId: 'receipt:test-empty-install', + }, + })).rejects.toThrow('arguments must not contain NUL') expect(harness.spawn).not.toHaveBeenCalled() await harness.dispose() })