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

fix(subagent): declare the dsh-scope dependency; make the re-assert REPLACE conflicting entries

ds-review-bot round-2 findings: (1) dsh-subagent's runtime import of
@deepseek-ai/dsh-scope was undeclared in its manifest and tsconfig
references (the root paths map masked it; the emitted package would import
an undeclared dependency) — wired as peer+dev with the project reference,
module graph regenerated. (2) The structured re-assert only ensured
PRESENCE, so a downstream listener injecting a same-named entry with the
wrong schema kept it model-visible while validateStructuredValue enforced
the real one; it now REPLACES any same-named tool/section with the run's
own. Pinned by a wrong-schema-injection test asserting exactly one entry
carrying the run's schema.
Tianyi Cui 2 месяцев назад
Родитель
Сommit
e5093244fb

+ 2 - 1
docs/module-graph.md

@@ -145,6 +145,7 @@ flowchart TD
   pkg_tool_fs --> pkg_tools
   pkg_subagent --> pkg_agent
   pkg_subagent --> pkg_llm
+  pkg_subagent --> pkg_scope
   pkg_subagent --> pkg_tools
   pkg_tool_web --> pkg_llm
   pkg_tool_web --> pkg_system_prompt
@@ -251,7 +252,7 @@ flowchart TD
 | [`agent-loop`](../packages/core/agent-loop) | `core` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope), [`session`](../packages/core/session), [`session-persistence`](../packages/session-persistence/session-persistence), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) |
 | [`tool-bash`](../packages/bash/tool-bash) | `bash` | [`agent`](../packages/core/agent), [`bash`](../packages/bash/bash), [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) |
 | [`tool-fs`](../packages/fs/tool-fs) | `fs` | [`fs`](../packages/fs/fs), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) |
-| [`subagent`](../packages/subagent/subagent) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`tools`](../packages/core/tools) |
+| [`subagent`](../packages/subagent/subagent) | `subagent` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope), [`tools`](../packages/core/tools) |
 | [`tool-web`](../packages/web/tool-web) | `web` | [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`web`](../packages/web/web) |
 | [`tool-todo`](../packages/todo/tool-todo) | `todo` | [`agent`](../packages/core/agent), [`session`](../packages/core/session), [`tools`](../packages/core/tools) |
 | [`hooks-codex`](../packages/hooks/hooks-codex) | `hooks` | [`agent`](../packages/core/agent), [`hook-protocol`](../packages/hooks/hook-protocol), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`tools`](../packages/core/tools) |

+ 12 - 6
packages/subagent/subagent-inprocess/src/structured.ts

@@ -135,12 +135,18 @@ export function attachStructuredRuntime(childCtx: Context, schema: StructuredOut
     this: unknown, _assembly: PromptAssembly, _context: AssembleContext, next: () => Promise<PromptAssembly>,
   ): Promise<PromptAssembly> {
     const final = await next()
-    if (!final.tools.some(tool => tool.name === STRUCTURED_OUTPUT_TOOL)) {
-      final.tools = [...final.tools, { ...schemaEntry, parameters: structuredClone(schemaEntry.parameters) }]
-    }
-    if (!final.sections.some(section => section.name === `tool:${STRUCTURED_OUTPUT_TOOL}`)) {
-      final.sections = [...final.sections, { name: `tool:${STRUCTURED_OUTPUT_TOOL}`, order: 190, text: STRUCTURED_OUTPUT_INSTRUCTION }]
-    }
+    // REPLACE, not merely ensure-present: a downstream listener may have
+    // mutated or injected a same-named entry with the WRONG schema/text, and
+    // the model-visible demand must be exactly this run's own — the same
+    // schema validateStructuredValue enforces.
+    final.tools = [
+      ...final.tools.filter(tool => tool.name !== STRUCTURED_OUTPUT_TOOL),
+      { ...schemaEntry, parameters: structuredClone(schemaEntry.parameters) },
+    ]
+    final.sections = [
+      ...final.sections.filter(section => section.name !== `tool:${STRUCTURED_OUTPUT_TOOL}`),
+      { name: `tool:${STRUCTURED_OUTPUT_TOOL}`, order: 190, text: STRUCTURED_OUTPUT_INSTRUCTION },
+    ]
     return final
   }, { prepend: true })
 

+ 26 - 0
packages/subagent/subagent-inprocess/tests/structured.spec.ts

@@ -412,6 +412,32 @@ describe('in-process structured output', () => {
       await runB.dispose()
     })
 
+    it('the re-assert REPLACES a conflicting injected schema, not merely ensures presence', async () => {
+      const { ctx, parent, adapter } = await setup([
+        toolCallResponse('c1', STRUCTURED_OUTPUT_TOOL, { answer: 5 }),
+      ])
+      // A global listener that INJECTS a wrong-schema structured_output entry:
+      // the child's re-assert must replace it with the run's own schema.
+      ctx.on('system-prompt/assemble', async (_assembly, _context, next) => {
+        const replaced = await next()
+        return {
+          sections: replaced.sections,
+          tools: [
+            ...replaced.tools.filter(tool => tool.name !== STRUCTURED_OUTPUT_TOOL),
+            { name: STRUCTURED_OUTPUT_TOOL, description: 'wrong', parameters: { type: 'object', properties: { bogus: { type: 'string' } } } },
+          ],
+          variables: { ...replaced.variables },
+        }
+      })
+      const run = ctx.subagents.start('spawn', structuredRequest(parent))
+      const result = await run.result
+      expect(result.structured).toEqual({ answer: 5 })
+      const entries = adapter.requests[0]!.tools!.filter(tool => tool.name === STRUCTURED_OUTPUT_TOOL)
+      expect(entries).toHaveLength(1)
+      expect(entries[0]!.parameters).toEqual(SCHEMA)
+      await run.dispose()
+    })
+
     it('the re-assert wins against a downstream listener that REPLACES the assembly object', async () => {
       const { ctx, parent, adapter } = await setup([
         toolCallResponse('c1', STRUCTURED_OUTPUT_TOOL, { answer: 5 }),

+ 2 - 0
packages/subagent/subagent/package.json

@@ -24,12 +24,14 @@
   "peerDependencies": {
     "@deepseek-ai/dsh-agent": "^0.0.1",
     "@deepseek-ai/dsh-llm": "^0.0.1",
+    "@deepseek-ai/dsh-scope": "^0.0.1",
     "@deepseek-ai/dsh-tools": "^0.0.1",
     "cordis": "^4.0.0-rc.6"
   },
   "devDependencies": {
     "@deepseek-ai/dsh-agent": "workspace:^",
     "@deepseek-ai/dsh-llm": "workspace:^",
+    "@deepseek-ai/dsh-scope": "workspace:^",
     "@deepseek-ai/dsh-tools": "workspace:^",
     "cordis": "^4.0.0-rc.6"
   }

+ 3 - 0
packages/subagent/subagent/tsconfig.json

@@ -22,6 +22,9 @@
     },
     {
       "path": "../../core/tools"
+    },
+    {
+      "path": "../../core/scope"
     }
   ]
 }

+ 3 - 0
pnpm-lock.yaml

@@ -587,6 +587,9 @@ importers:
       '@deepseek-ai/dsh-llm':
         specifier: workspace:^
         version: link:../../llm/llm
+      '@deepseek-ai/dsh-scope':
+        specifier: workspace:^
+        version: link:../../core/scope
       '@deepseek-ai/dsh-tools':
         specifier: workspace:^
         version: link:../../core/tools