fix: add tools reorder to system prompt

This commit is contained in:
imccyu
2026-07-07 11:03:11 +08:00
parent dc5656fab3
commit 42e7a2f691
18 changed files with 459 additions and 43 deletions

View File

@@ -35,11 +35,11 @@ This is the [interface/implementation/consumer seam](../../../docs/rfc/implement
```ts
import type { Config } from '@deepseek-ai/dsh-agent-core'
// { agents?, persona? } — the schema is z.intersect([AgentLoop.Config, SystemPrompt.Config]),
// { agents?, persona?, toolOrder? } — the schema is z.intersect([AgentLoop.Config, SystemPrompt.Config]),
// so validation and defaulting can never drift from the owners'.
```
The bundle FORWARDS each field to the child that owns it: `agents` to `agent-loop` (default `[]`), so each app supplies its own pre-created agents — a stdio app pre-creates a `main`; the ACP app pre-creates none (it creates agents on demand at `session/new`) — and `persona` to `dsh-system-prompt` (default `''`), the deployment's persona section. Forwarding is exactly why the owners can live in the shared spine even though the apps disagree on what to configure.
The bundle FORWARDS each field to the child that owns it: `agents` to `agent-loop` (default `[]`), so each app supplies its own pre-created agents — a stdio app pre-creates a `main`; the ACP app pre-creates none (it creates agents on demand at `session/new`) — `persona` to `dsh-system-prompt` (default `''`), the deployment's persona section — and `toolOrder` to `dsh-system-prompt` (absent — lexicographic), the explicit model-facing tool order. Forwarding is exactly why the owners can live in the shared spine even though the apps disagree on what to configure.
## Why a code bundle, not a shared YAML include

View File

@@ -59,17 +59,20 @@ export const name = 'agent-core'
/**
* Bundle config: each field forwarded verbatim to the child that owns it —
* `agents` to the agent loop (an app that pre-creates no agents, like the ACP
* bridge, simply omits it), `persona` to the system-prompt plugin (the
* deployment's persona section). Both are optional INPUT here because each
* owner's schema supplies the default (`[]` / `''`); the schema is the
* INTERSECTION of the owners' own schemas, so validation and defaulting can
* never drift from them.
* bridge, simply omits it), `persona` and `toolOrder` to the system-prompt
* plugin (the deployment's persona section and the explicit model-facing tool
* order). Every field is optional INPUT here because each owner's schema
* supplies the default (`[]` / `''` / absent — lexicographic); the schema is
* the INTERSECTION of the owners' own schemas, so validation and defaulting
* can never drift from them.
*/
export interface Config {
/** The agent-loop `agents` list (see dsh-agent-loop's `Config`). */
agents?: AgentLoopConfig['agents']
/** The deployment persona (see dsh-system-prompt's `Config`). */
persona?: SystemPromptConfig['persona']
/** The explicit model-facing tool order (see dsh-system-prompt's `Config`). */
toolOrder?: SystemPromptConfig['toolOrder']
}
/** Intersect the owners' schemas so validation + defaulting stay identical. */
@@ -78,11 +81,11 @@ export const Config = z.intersect([AgentLoop.Config, SystemPrompt.Config]) as un
/**
* Load the spine. Each `ctx.plugin(...)` mounts one child of the bundle fiber;
* `agent-loop` receives the forwarded `agents` list and `system-prompt` the
* forwarded `persona`. Load order is irrelevant (cordis pends each fiber on
* its `inject` until the services it needs exist), but the listing mirrors the
* dependency layering for readability: the LLM vocabulary and core registries
* first, then the dev tripwire and the bash tool consumer, then the loop that
* drives them.
* forwarded `persona` and `toolOrder`. Load order is irrelevant (cordis pends
* each fiber on its `inject` until the services it needs exist), but the
* listing mirrors the dependency layering for readability: the LLM vocabulary
* and core registries first, then the dev tripwire and the bash tool consumer,
* then the loop that drives them.
*/
export function apply(ctx: Context, config: Config): void {
ctx.plugin(Timer)
@@ -91,8 +94,13 @@ export function apply(ctx: Context, config: Config): void {
// The forwarded fields are validated + defaulted by this bundle's intersected
// schema before apply runs, so the ?? fallbacks only narrow the
// optional-input TYPES — they mirror the owners' schema defaults, never
// introduce different ones.
ctx.plugin(SystemPrompt, { persona: config.persona ?? '' })
// introduce different ones. toolOrder has no owner-supplied default value —
// ABSENT means "lexicographic order" — so it is forwarded conditionally
// rather than via ??.
ctx.plugin(SystemPrompt, {
persona: config.persona ?? '',
...config.toolOrder !== undefined ? { toolOrder: config.toolOrder } : {},
})
ctx.plugin(ToolRegistry)
ctx.plugin(AgentRegistry)
ctx.plugin(invariants)

View File

@@ -67,6 +67,23 @@ describe('dsh-agent-core bundle', () => {
await ctx.fiber.dispose()
})
it('forwards toolOrder to the system-prompt assembly', async () => {
const ctx = await mount({ toolOrder: ['zulu', '...'] })
// The bundle's own bash tools pend on the absent `ctx.bash` executor in
// this providerless mount, so register two plain tools to order.
for (const name of ['alpha', 'zulu']) {
ctx.get('tools')!.register({
name,
description: name,
parameters: {},
execute: async () => [],
})
}
const assembly = await ctx.get('systemPrompt')!.assemble()
expect(assembly.tools.map(tool => tool.name)).toEqual(['zulu', 'alpha'])
await ctx.fiber.dispose()
})
it('re-exports the loop config schema as its own', () => {
expect(agentCore.Config).toBeDefined()
expect(agentCore.name).toBe('agent-core')

View File

@@ -0,0 +1,94 @@
/**
* Loop-level tool-order determinism: the request/header event — and therefore
* the frozen request the adapter receives — carries the assembly's canonical
* tool order (system-prompt's `toolOrder` config, or lexicographic name
* order), regardless of the order tool plugins happened to register in.
* Registration order is a plugin-load artifact (concurrent dynamic imports
* race), so nothing downstream of the registry may depend on it.
*/
import { describe, expect, it } from 'vitest'
import { Context } from 'cordis'
import LlmService from '@deepseek-ai/dsh-llm'
import SessionStore, { foldRequestHeader } from '@deepseek-ai/dsh-session'
import SystemPrompt, { TOOL_ORDER_REST } from '@deepseek-ai/dsh-system-prompt'
import type { Config as SystemPromptConfig } from '@deepseek-ai/dsh-system-prompt'
import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools'
import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent'
import AgentLoop, { ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop'
import { MockAdapter, textResponse } from './mock-adapter.ts'
async function harness(adapter: MockAdapter, toolOrder?: SystemPromptConfig['toolOrder']) {
const ctx = new Context()
await ctx.plugin(LlmService)
await ctx.plugin(SessionStore)
await ctx.plugin(SystemPrompt, { persona: 'stable base', ...toolOrder !== undefined ? { toolOrder } : {} })
await ctx.plugin(ToolRegistry)
await ctx.plugin(AgentRegistry)
await ctx.plugin(AgentLoop, { agents: [] })
ctx.llm.registerAdapter(['mock'], adapter)
return ctx
}
function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise<void> {
return new Promise((resolve) => {
const dispose = ctx.on('agent/status', (subject, status) => {
if (subject === agent && status === 'idle') {
dispose()
resolve()
}
})
})
}
function registerNamed(ctx: Context, name: string) {
ctx.tools.register(defineTool({
name,
description: `the ${name} tool`,
parameters: {},
async execute() {
return [{ type: 'text', text: name }]
},
}))
}
/** Run one text-only turn and return the harness context + agent. */
async function runTurn(registrationOrder: string[], toolOrder?: SystemPromptConfig['toolOrder']) {
const adapter = new MockAdapter([textResponse('done')])
const ctx = await harness(adapter, toolOrder)
for (const name of registrationOrder) registerNamed(ctx, name)
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
agent.send([{ type: 'text', text: 'go' }])
await waitForIdle(ctx, agent)
return { ctx, agent, adapter }
}
describe('loop-level canonical tool order', () => {
it('logs the request/header with tools in canonical order, not registration order', async () => {
const { agent, adapter } = await runTurn(['zulu', 'alpha', 'mike'])
const header = foldRequestHeader(agent.session.events)
expect(header?.tools?.map(tool => tool.name)).toEqual(['alpha', 'mike', 'zulu'])
// The dispatched request is built FROM the logged header (whose tools the
// assembly already canonicalized) and reaches the adapter deep-frozen —
// the marker the reconstruction invariant keys on.
expect(adapter.requests[0]?.tools?.map(tool => tool.name)).toEqual(['alpha', 'mike', 'zulu'])
expect(Object.isFrozen(adapter.requests[0])).toBe(true)
expect(adapter.requests[0]?.sessionId).toBe(agent.session.id)
})
it('produces the same header order for any registration order', async () => {
const first = await runTurn(['alpha', 'mike', 'zulu'])
const second = await runTurn(['zulu', 'mike', 'alpha'])
const names = (run: typeof first) => foldRequestHeader(run.agent.session.events)?.tools?.map(tool => tool.name)
expect(names(first)).toEqual(['alpha', 'mike', 'zulu'])
expect(names(second)).toEqual(names(first))
})
it('honors a configured toolOrder in the logged header and the dispatched request', async () => {
const { agent, adapter } = await runTurn(['alpha', 'zulu', 'mike'], ['zulu', TOOL_ORDER_REST])
const header = foldRequestHeader(agent.session.events)
expect(header?.tools?.map(tool => tool.name)).toEqual(['zulu', 'alpha', 'mike'])
expect(adapter.requests[0]?.tools?.map(tool => tool.name)).toEqual(['zulu', 'alpha', 'mike'])
expect(Object.isFrozen(adapter.requests[0])).toBe(true)
})
})

View File

@@ -7,6 +7,7 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool-
| Key | Default | Meaning |
|---|---|---|
| `persona` | `''` | The deployment persona: the ONE deployment-authored prompt fragment, rendered as the order-0 `deployment:persona` section and shared by every agent in the context (subagents included). A template — complete `{{…}}` groups are interpreted strictly against the registered variables (the shipped loop registers `{{model}}`/`{{cwd}}`), with no escape syntax for literal braces yet. Empty ⇒ the section is dropped at render. |
| `toolOrder` | — | Explicit model-facing tool order, as a list of `ToolSchema.name`s with one `'...'` rest entry (`TOOL_ORDER_REST`): listed tools take their listed position, names with no registered tool are ignored, unlisted tools land at `'...'` in lexicographic name order. Absent ⇒ plain lexicographic name order. Applied to the collected tools BEFORE the `system-prompt/assemble` waterfall — like the sections' `order` sort, it canonicalizes what the registry contributed (registration order is a plugin-load artifact), and a waterfall listener that mutates the list owns the determinism of what it emits. A list without exactly one `'...'`, or with duplicates, throws at load. Why a central list and not per-plugin weights: [Explicit model-facing tool order](../../../docs/rfc/implemented/feature/2026-07-06-explicit-tool-order.md). |
## Service: `SystemPrompt` (ctx key: `systemPrompt`)

View File

@@ -88,7 +88,8 @@ export interface AssembledSection {
*
* Tool schemas are part of the assembly by design: "what the model is told it
* can do" is one coherent thing managed here, even though adapters transmit
* `tools` as a separate wire field rather than prompt text.
* `tools` as a separate wire field rather than prompt text. They arrive in
* the canonical model-facing order (see {@link Config.toolOrder}).
*
* `variables` carries every registered prompt variable resolved against this
* assembly's context — key present means registered, `undefined` value means
@@ -110,6 +111,53 @@ const VARIABLE_NAME = /^[a-z][a-z0-9_]*$/
/** A complete `{{...}}` reference group at the scan position (validated after). */
const GROUP_AT = /^\{\{([^{}]*)\}\}/
/**
* The rest entry for {@link Config.toolOrder}: the position where registered
* tools not named in the list are inserted (in lexicographic name order).
* Deliberately not a valid model-facing tool name, so it can never collide
* with a real tool.
*/
export const TOOL_ORDER_REST = '...'
/**
* Validate a configured tool-order list at service construction: `'...'`
* ({@link TOOL_ORDER_REST}) exactly once, no duplicate names. Returns the list
* (or undefined when unconfigured); throws otherwise, failing the service at
* load — a bad order config must never reach an assembly.
*/
function validateToolOrder(toolOrder: string[] | undefined): string[] | undefined {
if (toolOrder === undefined) return undefined
const seen = new Set<string>()
for (const name of toolOrder) {
if (seen.has(name)) throw new Error(`toolOrder lists "${name}" more than once`)
seen.add(name)
}
if (!seen.has(TOOL_ORDER_REST)) {
throw new Error(`toolOrder must contain the "${TOOL_ORDER_REST}" rest entry (where unlisted tools are inserted)`)
}
return toolOrder
}
/**
* Order collected tool schemas by the validated policy: with no configured
* list, plain lexicographic name order; with one, listed names take their
* listed position and every unlisted tool lands at the `'...'` entry in
* lexicographic name order. Never drops a tool, and both sorts are stable, so
* tools sharing a name keep their collection order.
*/
function orderTools(tools: ToolSchema[], toolOrder: string[] | undefined): ToolSchema[] {
if (toolOrder === undefined) return tools.sort(compareToolNames)
const listed = new Set(toolOrder)
const rest = tools.filter(tool => !listed.has(tool.name)).sort(compareToolNames)
return toolOrder.flatMap(name =>
name === TOOL_ORDER_REST ? rest : tools.filter(tool => tool.name === name))
}
/** Lexicographic (code-unit) name comparison — locale-independent, so the order is identical on every machine. */
function compareToolNames(a: ToolSchema, b: ToolSchema): number {
return a.name < b.name ? -1 : a.name > b.name ? 1 : 0
}
export interface Config {
/**
* The deployment's persona — the ONE deployment-authored fragment of the
@@ -124,6 +172,22 @@ export interface Config {
* deployment opens with the harness identity alone.
*/
persona?: string
/**
* Explicit model-facing tool order, as a list of `ToolSchema.name`s: listed
* tools take their listed position, names with no registered tool are
* ignored, and tools absent from the list are inserted at the
* {@link TOOL_ORDER_REST} (`'...'`) entry in lexicographic name order. A
* configured list must contain `'...'` exactly once and no duplicate names —
* anything else throws at load; a bad order config must never reach a
* model request. When omitted, tools are ordered lexicographically by name.
* Applied to the tools {@link SystemPrompt.assemble} collects, BEFORE the
* `system-prompt/assemble` waterfall — like the sections' `order` sort, it
* canonicalizes what the registry contributed (registration order is a
* plugin-load artifact); a waterfall listener that mutates the tool list
* owns the determinism of what it emits. Rationale (and why not per-plugin
* weights): docs/rfc/implemented/feature/2026-07-06-explicit-tool-order.md.
*/
toolOrder?: string[]
}
/**
@@ -198,14 +262,23 @@ function interpolate(section: AssembledSection, variables: Record<string, string
export class SystemPrompt extends Service {
static Config: z<Config> = z.object({
persona: z.string().default(''),
// A schemastery array defaults to [] when omitted, but an omitted
// toolOrder must stay absent ("lexicographic order"), not become an
// explicitly-configured empty list (which is invalid — it lacks the '...'
// entry). Forcing the default to undefined keeps the key out of the
// validated config; the cast is needed because .default() expects the
// array type.
toolOrder: z.array(z.string()).default(undefined as unknown as string[]),
})
private sections: PromptSection[] = []
private toolProviders: (() => ToolSchema[])[] = []
private variableProviders = new Map<string, (context: AssembleContext) => string | undefined>()
private readonly toolOrder: string[] | undefined
constructor(ctx: Context, public config: Config) {
super(ctx, 'systemPrompt')
this.toolOrder = validateToolOrder(config.toolOrder)
// The harness-owned openers. They live HERE (not on the loop plugin) so a
// deployment that swaps in a different loop keeps them: the identity is a
// harness fact stated ahead of everything, and the persona is the
@@ -318,14 +391,19 @@ export class SystemPrompt extends Service {
/**
* Assemble the current prompt for one caller: section texts are resolved
* against `context` and sorted by order, tools collected from all
* providers, and every registered variable resolved against `context` into
* `assembly.variables`. Tool schemas are deep-cloned because adapters and
* request waterfalls may mutate schema objects. Runs through the
* `system-prompt/assemble` waterfall, giving listeners the opportunity to
* mutate or replace the assembly before it reaches the model. Await the
* result before reading the assembly values — waterfall listeners may be
* async. Interpolation happens later, in {@link renderPrompt}.
* against `context` and sorted by order, tools collected from all providers
* and put in the canonical model-facing order ({@link Config.toolOrder}, or
* lexicographic name order when unconfigured — provider registration order
* is a plugin-load artifact and never reaches the assembly), and every
* registered variable resolved against `context` into `assembly.variables`.
* Tool schemas are deep-cloned because adapters and request waterfalls may
* mutate schema objects. Runs through the `system-prompt/assemble`
* waterfall, giving listeners the opportunity to mutate or replace the
* assembly before it reaches the model — like the sections' `order` sort,
* tool canonicalization happens on the initial assembly, and a listener
* owns the determinism of whatever it emits. Await the result before
* reading the assembly values — waterfall listeners may be async.
* Interpolation happens later, in {@link renderPrompt}.
* @param context - what this assembly is for (defaults to an empty context;
* see {@link AssembleContext}).
* @returns the assembly after the waterfall has run.
@@ -343,8 +421,10 @@ export class SystemPrompt extends Service {
text: typeof section.text === 'function' ? section.text(context) : section.text,
}))
.sort((a, b) => a.order - b.order),
tools: this.toolProviders.flatMap(provider =>
provider().map(tool => ({ ...tool, parameters: structuredClone(tool.parameters) }))),
tools: orderTools(
this.toolProviders.flatMap(provider =>
provider().map(tool => ({ ...tool, parameters: structuredClone(tool.parameters) }))),
this.toolOrder),
variables,
}
return this.ctx.waterfall(this, 'system-prompt/assemble', assembly, context, () => Promise.resolve(assembly))

View File

@@ -0,0 +1,90 @@
import { describe, expect, it } from 'vitest'
import { Context } from 'cordis'
import SystemPrompt, { PromptAssembly, TOOL_ORDER_REST } from '@deepseek-ai/dsh-system-prompt'
import type { ToolSchema } from '@deepseek-ai/dsh-llm'
function tool(name: string, description = name): ToolSchema {
return { name, description, parameters: { type: 'object', properties: {} } }
}
async function mount(config: { persona?: string; toolOrder?: string[] } = {}): Promise<Context> {
const ctx = new Context()
await ctx.plugin(SystemPrompt, config)
return ctx
}
function names(assembly: PromptAssembly): string[] {
return assembly.tools.map(t => t.name)
}
describe('SystemPrompt tool order', () => {
it('exports the rest entry as "..."', () => {
expect(TOOL_ORDER_REST).toBe('...')
})
it('assembles tools in lexicographic name order when no toolOrder is configured', async () => {
const ctx = await mount()
ctx.systemPrompt.tools(() => [tool('charlie'), tool('alpha')])
ctx.systemPrompt.tools(() => [tool('bravo')])
expect(names(await ctx.systemPrompt.assemble())).toEqual(['alpha', 'bravo', 'charlie'])
})
it('assembles the same order regardless of provider registration order', async () => {
const forward = await mount()
forward.systemPrompt.tools(() => [tool('alpha')])
forward.systemPrompt.tools(() => [tool('zulu')])
const backward = await mount()
backward.systemPrompt.tools(() => [tool('zulu')])
backward.systemPrompt.tools(() => [tool('alpha')])
expect(names(await forward.systemPrompt.assemble())).toEqual(['alpha', 'zulu'])
expect(names(await backward.systemPrompt.assemble())).toEqual(['alpha', 'zulu'])
})
it('applies a configured toolOrder: listed positions, rest at "..." lexicographically, absent names ignored', async () => {
const ctx = await mount({ toolOrder: ['todo_write', 'ghost', TOOL_ORDER_REST, 'bash'] })
ctx.systemPrompt.tools(() => [tool('bash'), tool('echo_b'), tool('todo_write'), tool('echo_a')])
expect(names(await ctx.systemPrompt.assemble())).toEqual(['todo_write', 'echo_a', 'echo_b', 'bash'])
})
it('keeps collection order between tools that share a name (stable sort)', async () => {
const ctx = await mount()
ctx.systemPrompt.tools(() => [tool('dup', 'first'), tool('anchor'), tool('dup', 'second')])
const assembly = await ctx.systemPrompt.assemble()
expect(assembly.tools.map(t => t.description)).toEqual(['anchor', 'first', 'second'])
})
it('canonicalizes BEFORE the assemble waterfall: listeners see the ordered list and own their own edits', async () => {
const ctx = await mount()
ctx.systemPrompt.tools(() => [tool('zulu'), tool('alpha')])
let seen: string[] | undefined
ctx.on('system-prompt/assemble', function (assembly, _context, next) {
seen = assembly.tools.map(t => t.name)
// A listener-appended tool is NOT re-sorted — same contract as sections:
// canonicalization applies to what the registry contributed, and a
// listener owns the determinism of what it emits.
assembly.tools.push(tool('aardvark'))
return next()
})
const assembly = await ctx.systemPrompt.assemble()
expect(seen).toEqual(['alpha', 'zulu'])
expect(names(assembly)).toEqual(['alpha', 'zulu', 'aardvark'])
})
it.each([
['an empty list', []],
['a list without the rest entry', ['bash', 'todo_write']],
])('rejects %s at load (the "..." rest entry is required)', async (_case, toolOrder) => {
await expect(new Context().plugin(SystemPrompt, { toolOrder })).rejects.toThrow('must contain the "..." rest entry')
})
it.each([
['a duplicate tool name', ['bash', 'bash', TOOL_ORDER_REST]],
['a duplicate rest entry', [TOOL_ORDER_REST, 'bash', TOOL_ORDER_REST]],
])('rejects %s at load', async (_case, toolOrder) => {
await expect(new Context().plugin(SystemPrompt, { toolOrder })).rejects.toThrow('more than once')
})
it('throws from direct construction too', () => {
expect(() => new SystemPrompt(new Context(), { toolOrder: ['bash'] })).toThrow('rest entry')
})
})