Просмотр исходного кода

Classify disabled expression errors in startup audits

turtle1999 1 неделя назад
Родитель
Сommit
25d5efcd86

+ 2 - 2
.agents/notes/implemented/architecture/2026-09-09-consumer-owned-startup-strictness.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-09-09-consumer-owned-startup-strictness.md
-2026-09-09-consumer-owned-startup-strictness.md: 1f3007d6bfc134c8fc741b87f001261f3f0ab468
-2026-09-09-consumer-owned-startup-strictness.zh.md: c1b52dc6e3fd9688878eb57c4815f983ed4b11b1
+2026-09-09-consumer-owned-startup-strictness.md: b9742b91cb66be8f3aa7d052caf16a12c5c35072
+2026-09-09-consumer-owned-startup-strictness.zh.md: dc9d58e8322e47d6f0559e7a5240b46584f0ad14

+ 2 - 0
.agents/notes/implemented/architecture/2026-09-09-consumer-owned-startup-strictness.md

@@ -14,6 +14,8 @@ DSH owns startup strictness outside vendored Cordis. App-boot audits the settled
 
 The required ids are `agent-loop`, `webserver`, `modules`, `connection`, `headless-runner`, `acp`, and `sdk-jsonrpc-server`. They represent shared Agent execution, application endpoints, and Web bootstrap/transport. Web needs its client module registry and authenticated connection even when the HTTP server can listen without them. Providers already required through injection need no separate entry: their absence leaves a listed consumer pending or failed.
 
+The audit treats a throwing `disabled` expression as an entry failure, not a disabled entry, because evaluation never established whether to skip it. The same optional/required policy applies to that failure.
+
 The audit runs only during initial application boot. Later config HMR remains best effort and keeps the failed candidate visible for repair.
 
 ## Alternatives considered

+ 2 - 0
.agents/notes/implemented/architecture/2026-09-09-consumer-owned-startup-strictness.zh.md

@@ -14,6 +14,8 @@ DSH 在 vendored Cordis 之外持有启动严格语义。App-boot 用一份全
 
 Required id 为 `agent-loop`、`webserver`、`modules`、`connection`、`headless-runner`、`acp` 和 `sdk-jsonrpc-server`。它们分别代表共享 Agent 执行、应用 endpoint,以及 Web 启动与传输。即使 HTTP server 不依赖它们也能监听,Web 仍需要客户端模块注册表和经过认证的连接。通过注入已成为必需项的 provider 不需要单列:它们缺失时,已列出的消费方会保持 pending 或失败。
 
+审计将 `disabled` 表达式抛出的异常视为 entry 失败,而不是 entry 已禁用,因为求值未能确定是否跳过它。该失败遵循相同的 optional/required 策略。
+
 该审计只在应用首次启动时运行。之后的 config HMR 仍采用 best effort,并保留 failed candidate 供后续修复。
 
 ## 考虑过的替代方案

+ 20 - 4
apps/cli/tests/profiles/web/tests/web-best-effort-startup.expected.e2e.ts

@@ -78,6 +78,9 @@ function createFixture(): Fixture {
     `      name: ${pathToFileURL(join(root, 'async-failure.mjs')).href}`,
     '    - id: web-probe-pending',
     `      name: ${pathToFileURL(join(root, 'pending.mjs')).href}`,
+    '    - id: web-probe-disabled-failure',
+    `      name: ${pathToFileURL(join(root, 'good.mjs')).href}`,
+    '      disabled: !!js "JSON.parse(\'invalid\')"',
     '',
   ].join('\n'))
   return { root, home, patch, events, stop }
@@ -98,7 +101,7 @@ async function waitForStartup(
   let settled = false
   const finish = (): void => {
     if (settled || url === undefined) return
-    if (!stderrText.includes('dsh: warning: 5 entries did not activate')) return
+    if (!stderrText.includes('dsh: warning: 6 entries did not activate')) return
     if (!stderrText.includes('web async apply failure')) return
     if (!stderrText.includes('webProbeMissingService')) return
     settled = true
@@ -170,6 +173,8 @@ describe.skipIf(!builtArtifactsExist)('dsh Web profile best-effort startup', ()
       expect(startup.stderr).toContain('web sync apply failure')
       expect(startup.stderr).toContain('web async apply failure')
       expect(startup.stderr).toContain('pending (waiting for service: webProbeMissingService)')
+      expect(startup.stderr).toContain('web-probe-disabled-failure')
+      expect(startup.stderr).toContain('disabled expression failed: SyntaxError')
     } finally {
       writeFileSync(fixture.stop, 'stop')
       result = await child
@@ -193,9 +198,20 @@ describe.skipIf(!builtArtifactsExist)('dsh Web profile best-effort startup', ()
     `)
   })
 
-  it.each(['modules', 'connection'])('fails the full Web profile when required %s cannot activate', async (id) => {
+  it.each([
+    ['modules', 'missing dependency'],
+    ['connection', 'missing dependency'],
+    ['modules', 'disabled expression'],
+    ['connection', 'disabled expression'],
+  ])('fails the full Web profile on required %s %s failure', async (id, failure) => {
     const fixture = createFixture()
-    writeFileSync(fixture.patch, `${readFileSync(fixture.patch, 'utf8')}- id: ${id}\n  inject: [webProbeMissingRequiredService]\n`)
+    const patch = failure === 'disabled expression'
+      ? 'disabled: !!js "JSON.parse(\'invalid\')"'
+      : 'inject: [webProbeMissingRequiredService]'
+    const diagnostic = failure === 'disabled expression'
+      ? 'disabled expression failed: SyntaxError'
+      : 'pending (waiting for service: webProbeMissingRequiredService)'
+    writeFileSync(fixture.patch, `${readFileSync(fixture.patch, 'utf8')}- id: ${id}\n  ${patch}\n`)
     try {
       const result = await execa(process.execPath, [
         dshBin,
@@ -223,7 +239,7 @@ describe.skipIf(!builtArtifactsExist)('dsh Web profile best-effort startup', ()
       expect(result.exitCode).toBe(1)
       expect(result.stdout).not.toContain('dsh web: http://')
       expect(result.stderr).toContain('required startup failure')
-      expect(result.stderr).toContain(`${id} (@deepseek-ai/dsh-client-${id}): pending (waiting for service: webProbeMissingRequiredService)`)
+      expect(result.stderr).toContain(`${id} (@deepseek-ai/dsh-client-${id}): ${diagnostic}`)
       expect(readFileSync(fixture.events, 'utf8')).toBe('good apply\ngood dispose\n')
     } finally {
       rmSync(fixture.root, { recursive: true, force: true })

+ 2 - 2
packages/boot/app-boot/README.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write packages/boot/app-boot/README.md
-README.md: ee97801e8ca3eedb4cd1fac85d0ed4d669ed8783
-README.zh.md: 2bf1b8e691d1ace0d5bb97fd55641abf727dc589
+README.md: 90998cedf68be91e9cb033c4dc8e54615c49a4b9
+README.zh.md: 74472de6d91fa5bb365044a928205f137fffd87b

+ 1 - 0
packages/boot/app-boot/README.md

@@ -70,6 +70,7 @@ After the Loader settles, app-boot classifies each enabled entry by stable id. O
 |---|---|---|
 | The root YAML cannot be read or parsed, or is not an entry list | Bootstrap Include fails | Reject and dispose; no partial application is accepted |
 | A plugin module cannot be imported | Entry has no fiber | Warn if optional; reject and dispose if required |
+| An entry's `disabled: !!js` expression throws | Entry cannot determine its disabled state; report the evaluation error | Warn if optional; reject and dispose if required |
 | Config expression evaluation or the plugin's config schema fails during activation | Fiber is `FAILED` with the validation error | Warn if optional; reject and dispose if required |
 | Synchronous `apply()` throws | Fiber is `FAILED` with the thrown error | Warn if optional; reject and dispose if required |
 | Asynchronous `apply()` throws | Fiber is `FAILED` with the thrown error | Warn if optional; reject and dispose if required |

+ 1 - 0
packages/boot/app-boot/README.zh.md

@@ -70,6 +70,7 @@ Loader 结算后,app-boot 按稳定 id 对每个已启用 entry 分类。Optio
 |---|---|---|
 | 根 YAML 无法读取或解析,或不是 entry list | Bootstrap Include 失败 | 拒绝并拆卸;不接受部分应用 |
 | Plugin module 无法 import | Entry 没有 fiber | Optional 时警告;required 时拒绝并拆卸 |
+| Entry 的 `disabled: !!js` 表达式抛出异常 | Entry 无法确定禁用状态;报告求值错误 | Optional 时警告;required 时拒绝并拆卸 |
 | Config expression 求值或 plugin config schema 在 activation 时失败 | Fiber 为 `FAILED`,保留校验错误 | Optional 时警告;required 时拒绝并拆卸 |
 | 同步 `apply()` throw | Fiber 为 `FAILED`,保留抛出的错误 | Optional 时警告;required 时拒绝并拆卸 |
 | 异步 `apply()` throw | Fiber 为 `FAILED`,保留抛出的错误 | Optional 时警告;required 时拒绝并拆卸 |

+ 13 - 7
packages/boot/app-boot/src/index.ts

@@ -738,17 +738,22 @@ interface InactiveEntry {
 }
 
 /**
- * Collect enabled Loader entries that did not become active. Failed fibers are
- * awaited to recover their recorded rejection reason and coalesce duplicate
- * Loader notifications through the next process rejection checkpoint.
+ * Collect Loader activation failures and disabled-expression errors. Failed
+ * fibers are awaited to recover their recorded rejection reason and coalesce
+ * duplicate Loader notifications through the next process rejection checkpoint.
  */
 async function inactiveEntries(ctx: Context): Promise<InactiveEntry[]> {
   const failures: InactiveEntry[] = []
   const rejectionReasons: unknown[] = []
   for (const entry of ctx.loader.entries()) {
-    const fiber = entry.fiber
-    if (entry.disabled) continue
     const subject = `${entry.options.id} (${entry.options.name})`
+    try {
+      if (entry.disabled) continue
+    } catch (error) {
+      failures.push({ entry, diagnostic: `${subject}: disabled expression failed: ${formatActivationError(error)}` })
+      continue
+    }
+    const fiber = entry.fiber
     if (fiber === undefined) {
       failures.push({ entry, diagnostic: `${subject}: failed to import` })
       continue
@@ -795,12 +800,13 @@ function activationDiagnostic(
  * Inactive entries from the global required list reject startup. Other
  * inactive entries produce one warning and leave successful siblings running.
  * Required ids absent from the tree, and disabled required entries, are ignored.
+ * A throwing disabled expression is an entry failure, not a disabled entry.
  * The bootstrap Include must activate so unreadable or invalid root config is fatal.
  * @param ctx - the settled context whose Loader entries to audit.
  * @param binName - the diagnostic prefix on optional-entry warnings.
  * @param warn - sink for optional-entry warnings.
- * @returns after all optional failures are warned when required startup entries are active.
- * @throws when the bootstrap Include or an enabled entry in {@link REQUIRED_STARTUP_ENTRY_IDS} is inactive.
+ * @returns after optional warnings if required startup checks pass.
+ * @throws when the bootstrap Include or a required entry is inactive or its disabled expression throws.
  */
 export async function auditStartupEntries(
   ctx: Context,

+ 28 - 2
packages/boot/app-boot/tests/app-boot.spec.ts

@@ -580,6 +580,26 @@ describe('auditStartupEntries', () => {
     ].join('\n'))
   })
 
+  it.each([
+    { id: 'tool-todo', required: false },
+    { id: 'webserver', required: true },
+  ])('reports a throwing disabled expression on $id (required: $required)', async ({ id, required }) => {
+    const error = new Error('disabled evaluation failed')
+    const warn = vi.fn()
+    const result = auditStartupEntries(ctxWith([{
+      options: { id, name: './plugin.mjs' },
+      get disabled(): boolean { throw error },
+    }]), NAME, warn)
+    const detail = `${id} (./plugin.mjs): disabled expression failed: ${error.stack!}`
+    if (required) {
+      await expect(result).rejects.toThrow(`required startup failure: 1 entry did not activate\n${detail}`)
+      expect(warn).not.toHaveBeenCalled()
+    } else {
+      await expect(result).resolves.toBeUndefined()
+      expect(warn).toHaveBeenCalledExactlyOnceWith(`${NAME}: warning: 1 entry did not activate\n${detail}\n`)
+    }
+  })
+
   it('preserves nested activation causes and aggregate member failures', async () => {
     const warn = vi.fn()
     const original = new Error('tool discovery failed')
@@ -867,7 +887,7 @@ describe('boot', () => {
     expect(ctx.get('loader')).toBeUndefined()
   })
 
-  it('keeps successful entries and warns about optional import, config, sync apply, async apply, and dependency failures', async () => {
+  it('keeps successful entries and warns about optional import, config, disabled, sync apply, async apply, and dependency failures', async () => {
     const dir = tmp()
     const configPath = join(dir, 'cordis.yml')
     const config = [
@@ -879,6 +899,9 @@ describe('boot', () => {
       '  name: ./noop.mjs',
       '  config:',
       '    value: !!js "JSON.parse(\'invalid\')"',
+      '- id: disabled-failure',
+      '  name: ./noop.mjs',
+      '  disabled: !!js "JSON.parse(\'invalid\')"',
       '- id: sync-failure',
       '  name: ./sync-failure.mjs',
       '- id: async-failure',
@@ -902,13 +925,15 @@ describe('boot', () => {
       const entries = [...ctx.loader.entries()]
       expect(entries.find(entry => entry.options.id === 'good')?.fiber?.state).toBe(2)
       expect(entries.find(entry => entry.options.id === 'import-failure')?.fiber).toBeUndefined()
+      expect(entries.find(entry => entry.options.id === 'disabled-failure')?.fiber).toBeUndefined()
       for (const id of ['invalid-config', 'sync-failure', 'async-failure']) {
         expect(entries.find(entry => entry.options.id === id)?.fiber?.state).toBe(3)
       }
       expect(entries.find(entry => entry.options.id === 'waiting')?.fiber?.state).toBe(0)
       const warning = write.mock.calls.map(call => String(call[0])).join('')
-      expect(warning).toContain(`${NAME}: warning: 5 entries did not activate`)
+      expect(warning).toContain(`${NAME}: warning: 6 entries did not activate`)
       expect(warning).toContain('import-failure (./missing.mjs): failed to import')
+      expect(warning).toContain('disabled-failure (./noop.mjs): disabled expression failed: SyntaxError')
       expect(warning).toContain('SyntaxError: Unexpected token')
       expect(warning).toContain('sync apply failure')
       expect(warning).toContain('async apply failure')
@@ -940,6 +965,7 @@ describe('boot', () => {
     ['import', undefined, '', 'failed to import'],
     ['config schema', 'export const Config = { "~standard": { version: 1, vendor: "app-boot-test", validate() { return { issues: [{ message: "schema failure" }] } } } }\nexport function apply() {}\n', '', 'schema failure'],
     ['config expression', 'export function apply() {}\n', '  config: { value: !!js "JSON.parse(\'invalid\')" }\n', 'SyntaxError'],
+    ['disabled expression', 'export function apply() {}\n', '  disabled: !!js "JSON.parse(\'invalid\')"\n', 'required startup failure: 1 entry did not activate\nwebserver (./required.mjs): disabled expression failed: SyntaxError'],
     ['sync apply', 'export function apply() { throw new Error("sync failure") }\n', '', 'sync failure'],
     ['async apply', 'export async function apply() { await Promise.resolve(); throw new Error("async failure") }\n', '', 'async failure'],
     ['missing dependency', 'export const inject = ["missingRequiredService"]\nexport function apply() {}\n', '', 'missingRequiredService'],

+ 14 - 4
packages/boot/app-boot/tests/user-patches.spec.ts

@@ -453,20 +453,30 @@ describe('boot with user patches', () => {
       expect(failures[0]).toBeInstanceOf(Error)
       expect(entryConfig(ctx, id)).toMatchObject({ fail: true })
 
+      writeFileSync(filename, `- id: ${id}\n  disabled: !!js "JSON.parse('invalid')"\n`)
+      watcher.emit('change', filename)
+      await eventually(() => failures.length === 2, 'disabled expression failure was not reported')
+      expect(failures[1]?.message).toContain(`${id} (./noop.mjs): disabled expression failed: SyntaxError`)
+      expect([...ctx.loader.entries()].find(entry => entry.options.id === id)?.options.disabled)
+        .toEqual({ __jsExpr: "JSON.parse('invalid')" })
+
       writeFileSync(filename, 'invalid: [unclosed\n')
       watcher.emit('change', filename)
-      await eventually(() => failures.length === 2, 'parse failure was not reported')
-      expect(failures[1]).toBeInstanceOf(Error)
-      expect(entryConfig(ctx, id)).toMatchObject({ fail: true })
+      await eventually(() => failures.length === 3, 'parse failure was not reported')
+      expect(failures[2]).toBeInstanceOf(Error)
+      expect([...ctx.loader.entries()].find(entry => entry.options.id === id)?.options.disabled)
+        .toEqual({ __jsExpr: "JSON.parse('invalid')" })
 
       writeFileSync(filename, `- id: ${id}\n  config:\n    value: recovered\n`)
       watcher.emit('change', filename)
       await eventually(() => (entryConfig(ctx, id) as { value?: string }).value === 'recovered', 'valid recovery was not applied')
+      await ctx.loader.await()
+      expect([...ctx.loader.entries()].find(entry => entry.options.id === id)?.fiber?.state).toBe(2)
 
       unlinkSync(filename)
       watcher.emit('unlink', filename)
       await eventually(() => (entryConfig(ctx, id) as { value?: string }).value === 'generated', 'user patch removal did not restore the app-owned patch')
-      expect(failures).toHaveLength(2)
+      expect(failures).toHaveLength(3)
 
       // Default compose: the user layer IS the whole patch list, so a
       // fresh generation replaces the app-owned layer instead of stacking on it.