From d136ac9f6ff7b5ffafa6ca8a7d44437b89230b5a Mon Sep 17 00:00:00 2001 From: winewill Date: Mon, 19 Oct 2026 17:54:46 +0800 Subject: [PATCH] fix(diagnostics): retain observed plugin versions for recovery --- docs/event-producer-consumer.md | 8 ++++---- docs/event-producer-consumer.zh.md | 8 ++++---- packages/boot/plugin-manager/README.i18n.yaml | 4 ++-- packages/boot/plugin-manager/README.md | 2 +- packages/boot/plugin-manager/README.zh.md | 2 +- .../boot/plugin-manager/src/diagnostic.ts | 3 ++- packages/boot/plugin-manager/src/index.ts | 17 +++++++++++++++++ packages/boot/plugin-manager/src/types.ts | 3 ++- .../plugin-manager/tests/diagnostic.spec.ts | 6 ++++++ .../boot/plugin-manager/tests/manager.spec.ts | 19 +++++++++++++++++-- .../extensions/tool-cordis/src/api-catalog.ts | 2 +- 11 files changed, 57 insertions(+), 17 deletions(-) diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index 88f9b1c92b..004d187875 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -56,10 +56,10 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `llm/adapters-updated` | `emit` | [`packages/llm/llm/src/types.ts:23`](../packages/llm/llm/src/types.ts) | [`llm`](../packages/llm/llm) (`events.dispatch`) | [`acp`](../packages/acp/acp), `remotes` | | `llm/stream` | `waterfall` | [`packages/llm/llm/src/index.ts:75`](../packages/llm/llm/src/index.ts) | [`llm`](../packages/llm/llm) (`waterfall`) | [`llm-replay`](../packages/test-support/llm-replay), [`session-checkpoint-policy`](../packages/session/session-checkpoint-policy), [`session-title`](../packages/session/session-title) | | `permission-presets/catalog-changed` | `emit` | [`packages/interaction/permission-presets/src/types.ts:48`](../packages/interaction/permission-presets/src/types.ts) | [`permission-presets`](../packages/interaction/permission-presets) (`events.dispatch`) | `remotes` | -| `plugin-manager/changed` | `emit` | [`packages/boot/plugin-manager/src/types.ts:301`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | -| `plugin-manager/install-log` | `emit` | [`packages/boot/plugin-manager/src/types.ts:307`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | -| `plugin-manager/install-state` | `emit` | [`packages/boot/plugin-manager/src/types.ts:314`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | -| `plugin-manager/operation` | `emit` | [`packages/boot/plugin-manager/src/types.ts:293`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | - | +| `plugin-manager/changed` | `emit` | [`packages/boot/plugin-manager/src/types.ts:302`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | +| `plugin-manager/install-log` | `emit` | [`packages/boot/plugin-manager/src/types.ts:308`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | +| `plugin-manager/install-state` | `emit` | [`packages/boot/plugin-manager/src/types.ts:315`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | +| `plugin-manager/operation` | `emit` | [`packages/boot/plugin-manager/src/types.ts:294`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | - | | `plugin-market/changed` | `emit` | [`packages/boot/plugin-market/src/types.ts:136`](../packages/boot/plugin-market/src/types.ts) | [`plugin-market`](../packages/boot/plugin-market) (`emit`) | `remotes` | | `schedule/changed` | `emit` | [`packages/schedule/schedule/src/types.ts:478`](../packages/schedule/schedule/src/types.ts) | [`schedule`](../packages/schedule/schedule) (`emit`) | `remotes` | | `session-telemetry/record` | `waterfall` | [`packages/session/session-telemetry/src/index.ts:44`](../packages/session/session-telemetry/src/index.ts) | [`session-telemetry`](../packages/session/session-telemetry) (`waterfall`) | - | diff --git a/docs/event-producer-consumer.zh.md b/docs/event-producer-consumer.zh.md index d01444f66b..f5b33be132 100644 --- a/docs/event-producer-consumer.zh.md +++ b/docs/event-producer-consumer.zh.md @@ -58,10 +58,10 @@ | `llm/adapters-updated` | `emit` | [`packages/llm/llm/src/types.ts:23`](../packages/llm/llm/src/types.ts) | [`llm`](../packages/llm/llm) (`events.dispatch`) | [`acp`](../packages/acp/acp), `remotes` | | `llm/stream` | `waterfall` | [`packages/llm/llm/src/index.ts:75`](../packages/llm/llm/src/index.ts) | [`llm`](../packages/llm/llm) (`waterfall`) | [`llm-replay`](../packages/test-support/llm-replay), [`session-checkpoint-policy`](../packages/session/session-checkpoint-policy), [`session-title`](../packages/session/session-title) | | `permission-presets/catalog-changed` | `emit` | [`packages/interaction/permission-presets/src/types.ts:48`](../packages/interaction/permission-presets/src/types.ts) | [`permission-presets`](../packages/interaction/permission-presets) (`events.dispatch`) | `remotes` | -| `plugin-manager/changed` | `emit` | [`packages/boot/plugin-manager/src/types.ts:301`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | -| `plugin-manager/install-log` | `emit` | [`packages/boot/plugin-manager/src/types.ts:307`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | -| `plugin-manager/install-state` | `emit` | [`packages/boot/plugin-manager/src/types.ts:314`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | -| `plugin-manager/operation` | `emit` | [`packages/boot/plugin-manager/src/types.ts:293`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | - | +| `plugin-manager/changed` | `emit` | [`packages/boot/plugin-manager/src/types.ts:302`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | +| `plugin-manager/install-log` | `emit` | [`packages/boot/plugin-manager/src/types.ts:308`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | +| `plugin-manager/install-state` | `emit` | [`packages/boot/plugin-manager/src/types.ts:315`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | `remotes` | +| `plugin-manager/operation` | `emit` | [`packages/boot/plugin-manager/src/types.ts:294`](../packages/boot/plugin-manager/src/types.ts) | [`plugin-manager`](../packages/boot/plugin-manager) (`emit`) | - | | `plugin-market/changed` | `emit` | [`packages/boot/plugin-market/src/types.ts:136`](../packages/boot/plugin-market/src/types.ts) | [`plugin-market`](../packages/boot/plugin-market) (`emit`) | `remotes` | | `schedule/changed` | `emit` | [`packages/schedule/schedule/src/types.ts:478`](../packages/schedule/schedule/src/types.ts) | [`schedule`](../packages/schedule/schedule) (`emit`) | `remotes` | | `session-telemetry/record` | `waterfall` | [`packages/session/session-telemetry/src/index.ts:44`](../packages/session/session-telemetry/src/index.ts) | [`session-telemetry`](../packages/session/session-telemetry) (`waterfall`) | - | diff --git a/packages/boot/plugin-manager/README.i18n.yaml b/packages/boot/plugin-manager/README.i18n.yaml index 5bb38b4627..c41c21af21 100644 --- a/packages/boot/plugin-manager/README.i18n.yaml +++ b/packages/boot/plugin-manager/README.i18n.yaml @@ -24,8 +24,8 @@ en: d5643a06f300c30a zh: 9348b6a7676a2849 /deepseek-ai-dsh-plugin-manager/understand-the-implementation: - en: 531d50d67168f82b - zh: 295fbc453029f0c4 + en: 86c5eddc73fb60ca + zh: 3b9819ab0e336611 /deepseek-ai-dsh-plugin-manager/further-exploration: en: c6538243d8702a0d zh: 04a82d923353c75f diff --git a/packages/boot/plugin-manager/README.md b/packages/boot/plugin-manager/README.md index 661f926bf1..eac1bb33e9 100644 --- a/packages/boot/plugin-manager/README.md +++ b/packages/boot/plugin-manager/README.md @@ -103,7 +103,7 @@ A service run whose captured output stays silent for `idleTimeoutMs` is terminat Results contain the last attempted stage, target, saved-state change, application status and error codes. Web dictionaries render management text; pnpm and Loader diagnostics remain unmodified. Unrelated pre-existing inactive entries return warnings; new or changed failures and inactive explicit enablement targets fail the operation. A failed or cancelled package run, or failed bundle validation, restores the manifest and lockfile snapshotted before pnpm ran ([rationale](../../../.agents/notes/implemented/architecture/2026-09-15-guided-plugin-installation.md)); later failures retain saved changes as described in [Failure behavior](#failure-behavior). Installations are tracked by request id until their call settles, so a cancellation names one run and joins its settlement without taking the profile lock. The CLI inherits authentication variables and terminal descriptors; service operations use a scrubbed environment and captured output. -The `plugin-manager/operation` event correlates each service invocation with its start and observed result, or an unknown outcome when an exception escapes result construction. It excludes raw installation specs and error text. Diagnostic consumers own persistence and delivery limits; synchronous subscriber failures do not change management results. Direct CLI package operations do not emit this service event. +The `plugin-manager/operation` event correlates each service invocation with its start and observed result, or an unknown outcome when an exception escapes result construction. It retains the package version observed before a toggle or removal, and the installed version when installation resolves it; missing metadata leaves the version unknown without blocking recovery. It excludes raw installation specs and error text. Diagnostic consumers own persistence and delivery limits; synchronous subscriber failures do not change management results. Direct CLI package operations do not emit this service event. diff --git a/packages/boot/plugin-manager/README.zh.md b/packages/boot/plugin-manager/README.zh.md index ffc37ac4cf..f563448bab 100644 --- a/packages/boot/plugin-manager/README.zh.md +++ b/packages/boot/plugin-manager/README.zh.md @@ -103,7 +103,7 @@ service 运行若在 `idleTimeoutMs` 内没有任何捕获输出即被终止， 结果包含最后尝试的阶段、目标、磁盘变化、应用状态和错误码。Web 词典呈现管理文案；pnpm 与 Loader 的诊断保持原样。无关的已有故障作为警告返回；新出现、配置变化后的故障，以及显式启用目标未激活，都会使操作失败。包管理运行失败或被取消，或组合包校验失败时，会恢复 pnpm 运行前快照的 manifest 与 lockfile（[理由](../../../.agents/notes/implemented/architecture/2026-09-15-guided-plugin-installation.zh.md)）；后续失败按[失败行为](#failure-behavior)所述保留已保存的改动。安装按 request id 跟踪到调用结束，因此取消只针对一次运行，并且不取 profile 锁就能等待它结束。CLI 继承认证环境和终端描述符；service 使用清理后的环境并捕获输出。管理器直接读取文件和 Loader 状态，不维护第二份目标状态注册表，因此不发布单独的运行时不变式伴生入口。 -`plugin-manager/operation` 事件关联每次服务调用的开始和已观察到的结果；若异常使结果无法构造，则记录结果未知。事件不包含原始安装 spec 和错误文本。诊断消费者负责持久化和传输限额；同步订阅者失败不会改变管理结果。直接执行的 CLI 包操作不发送此服务事件。 +`plugin-manager/operation` 事件关联每次服务调用的开始和已观察到的结果；若异常使结果无法构造，则记录结果未知。事件保留启停或移除前观察到的包版本，以及安装完成后解析出的版本；元数据缺失时版本未知，不阻止恢复操作。事件不包含原始安装 spec 和错误文本。诊断消费者负责持久化和传输限额；同步订阅者失败不会改变管理结果。直接执行的 CLI 包操作不发送此服务事件。 diff --git a/packages/boot/plugin-manager/src/diagnostic.ts b/packages/boot/plugin-manager/src/diagnostic.ts index a5133fb045..02eeda6111 100644 --- a/packages/boot/plugin-manager/src/diagnostic.ts +++ b/packages/boot/plugin-manager/src/diagnostic.ts @@ -20,13 +20,14 @@ const fields = { target: z.string().max(256).nullable(), stage: z.enum(['install', 'enable', 'remove']), enabled: z.boolean().optional(), + version: z.string().max(128).optional(), } const operation: z.ZodType = z.discriminatedUnion('phase', [ z.object({ ...fields, phase: z.literal('started') }), z.object({ ...fields, phase: z.literal('unknown') }), z.object({ ...fields, phase: z.literal('settled'), application: z.enum(['applied', 'restart-required', 'overridden', 'failed', 'cancelled']), - changed: z.boolean(), version: z.string().max(128).optional(), + changed: z.boolean(), errorCode: z.enum(['management-required', 'unaddressable', 'unknown-plugin', 'invalid-spec', 'already-installed', 'ambiguous-install', 'not-bundle', 'not-removable', 'stop-profile', 'bundle-in-use', 'stale-approval', 'incompatible-version', 'operation-error']).optional(), diff --git a/packages/boot/plugin-manager/src/index.ts b/packages/boot/plugin-manager/src/index.ts index 4524396791..1496c7979d 100644 --- a/packages/boot/plugin-manager/src/index.ts +++ b/packages/boot/plugin-manager/src/index.ts @@ -902,8 +902,10 @@ export class PluginManager extends TypertRemoteService { reason: PluginChange['reason'], ): Promise { const id = randomUUID() as PluginOperationId + const version = this.operationVersion(reason, request.target) const facts = { id, reason, stage: request.stage, target: reason === 'install' ? null : request.target, + ...version === undefined ? {} : { version }, ...request.enabled === undefined ? {} : { enabled: request.enabled } } this.reportOperation({ ...facts, time: new Date().toISOString(), phase: 'started' }) let settled: ChangeResult @@ -952,6 +954,21 @@ export class PluginManager extends TypertRemoteService { } } + /** Read the pre-operation package version without preventing recovery from damaged metadata. */ + private operationVersion(reason: PluginChange['reason'], target: string): string | undefined { + if (reason === 'install') return undefined + try { + if (reason !== 'plugin') return bundleManifest(target, this.profile.dir, this.profile.installAnchor)?.version + const entry = [...this.ctx.loader.entries()].find(row => row.id === target) + const source = entry?.parent.tree.ctx.baseUrl + if (entry === undefined || source === undefined) return undefined + return this.ownerContext.get('pluginPackages')?.packageOf(entry.options.name, source)?.version + } catch (_error) { + // Disabling or removing a broken package must not depend on diagnostic metadata being readable. + return undefined + } + } + private diskState(): string { return ['package.json', 'cordis.patch.yml', 'pnpm-workspace.yaml', PROFILE_COMPATIBILITY_FILENAME].map((file) => { try { return readFileSync(join(this.profile.dir, file), 'utf8') } diff --git a/packages/boot/plugin-manager/src/types.ts b/packages/boot/plugin-manager/src/types.ts index 3c5807cf42..49e4164fe4 100644 --- a/packages/boot/plugin-manager/src/types.ts +++ b/packages/boot/plugin-manager/src/types.ts @@ -269,13 +269,14 @@ export type PluginOperation = { readonly target: string | null readonly stage: ChangeResult['stage'] readonly enabled?: boolean | undefined + /** Observed package version; absence means unavailable, never the latest registry version. */ + readonly version?: string | undefined } & ( | { readonly phase: 'started' } | { readonly phase: 'settled' readonly application: ChangeResult['application'] readonly changed: boolean - readonly version?: string | undefined readonly errorCode?: ManagementError['code'] | undefined readonly restartReason?: RestartReason | undefined } diff --git a/packages/boot/plugin-manager/tests/diagnostic.spec.ts b/packages/boot/plugin-manager/tests/diagnostic.spec.ts index 2134f81196..e8ef0f3832 100644 --- a/packages/boot/plugin-manager/tests/diagnostic.spec.ts +++ b/packages/boot/plugin-manager/tests/diagnostic.spec.ts @@ -21,3 +21,9 @@ it('rejects unsupported phases, oversized identifiers and invalid result fields' expect(parsePluginOperationRecord({ ...input, transportDropped: -1 })).toBeUndefined() expect(parsePluginOperationRecord({ ...input, runId: '' })).toBeUndefined() }) + +it('retains an observed version before an operation settles', () => { + const started = { ...input, operation: { ...input.operation, phase: 'started', version: '1.2.3' } } + expect(parsePluginOperationRecord(started)?.operation).toMatchObject({ phase: 'started', version: '1.2.3' }) + expect(parsePluginOperationRecord({ ...started, operation: { ...started.operation, version: 'x'.repeat(129) } })).toBeUndefined() +}) diff --git a/packages/boot/plugin-manager/tests/manager.spec.ts b/packages/boot/plugin-manager/tests/manager.spec.ts index ff90480f75..ace9643238 100644 --- a/packages/boot/plugin-manager/tests/manager.spec.ts +++ b/packages/boot/plugin-manager/tests/manager.spec.ts @@ -466,12 +466,27 @@ it('correlates diagnostic operation starts with persisted toggle results', async await manager.setBundleEnabled('extra', false) expect(readProfileManifest('test', dir).dsh?.profile?.bundles).not.toContain('extra') expect(facts).toHaveLength(2) - expect(facts[0]).toMatchObject({ phase: 'started', reason: 'bundle', target: 'extra', enabled: false }) - expect(facts[1]).toMatchObject({ id: facts[0]!.id, phase: 'settled', changed: true, application: 'restart-required' }) + expect(facts[0]).toMatchObject({ phase: 'started', reason: 'bundle', target: 'extra', enabled: false, version: '1.0.0' }) + expect(facts[1]).toMatchObject({ id: facts[0]!.id, phase: 'settled', changed: true, application: 'restart-required', version: '1.0.0' }) await manager.setBundleEnabled('extra', true) expect(facts[2]!.id).not.toBe(facts[0]!.id) }) +it('retains an observed version for a refused removal and leaves damaged metadata unknown', async () => { + const { manager, ctx, dir } = await fixture('startup') + const facts: PluginOperation[] = [] + ctx.on('plugin-manager/operation', (event) => { facts.push(event) }) + expect(await manager.removeBundle('extra')).toMatchObject({ application: 'failed', error: { code: 'stop-profile' } }) + expect(facts).toEqual([ + expect.objectContaining({ phase: 'started', reason: 'remove', version: '1.0.0' }), + expect.objectContaining({ phase: 'settled', reason: 'remove', version: '1.0.0', application: 'failed' }), + ]) + writeFileSync(join(dir, 'node_modules', 'extra', 'package.json'), '{') + expect(await manager.setBundleEnabled('extra', false)).toMatchObject({ changed: true, application: 'restart-required' }) + expect(facts[2]).not.toHaveProperty('version') + expect(facts[3]).not.toHaveProperty('version') +}) + it('excludes raw install specs and error diagnostics from operation facts', async () => { const { manager, ctx } = await fixture('startup') const facts: PluginOperation[] = [] diff --git a/packages/extensions/tool-cordis/src/api-catalog.ts b/packages/extensions/tool-cordis/src/api-catalog.ts index 1421fff7e7..5116b89eee 100644 --- a/packages/extensions/tool-cordis/src/api-catalog.ts +++ b/packages/extensions/tool-cordis/src/api-catalog.ts @@ -6439,7 +6439,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'PluginOperation', - declaration: 'export type PluginOperation = {\n readonly id: PluginOperationId;\n readonly time: string;\n readonly reason: PluginChange[\'reason\'];\n readonly target: string | null;\n readonly stage: ChangeResult[\'stage\'];\n readonly enabled?: boolean | undefined;\n} & ({\n readonly phase: \'started\';\n} | {\n readonly phase: \'settled\';\n readonly application: ChangeResult[\'application\'];\n readonly changed: boolean;\n readonly version?: string | undefined;\n readonly errorCode?: ManagementError[\'code\'] | undefined;\n readonly restartReason?: RestartReason | undefined;\n} | {\n readonly phase: \'unknown\';\n});', + declaration: 'export type PluginOperation = {\n readonly id: PluginOperationId;\n readonly time: string;\n readonly reason: PluginChange[\'reason\'];\n readonly target: string | null;\n readonly stage: ChangeResult[\'stage\'];\n readonly enabled?: boolean | undefined;\n readonly version?: string | undefined;\n} & ({\n readonly phase: \'started\';\n} | {\n readonly phase: \'settled\';\n readonly application: ChangeResult[\'application\'];\n readonly changed: boolean;\n readonly errorCode?: ManagementError[\'code\'] | undefined;\n readonly restartReason?: RestartReason | undefined;\n} | {\n readonly phase: \'unknown\';\n});', }, { name: 'PluginOperationId',