feat: add assembly-time validation rejects placeholder

This commit is contained in:
imccyu
2026-07-07 22:03:12 +08:00
parent b149a040d0
commit 161275e287
5 files changed
+41 -16

No files matched your search

+8 -5
View File
@@ -582,10 +582,13 @@ export interface Config {
* exactly once, no duplicate names, and no name without a registered tool —
* a misconfigured order blocks work instead of silently reaching a model
* request: shape violations throw at load, and an unregistered name rejects
* every assembly (failing the turn before any model request — the earliest
* moment the registered tool set exists to check against, since tool
* plugins register after this service constructs). When omitted, tools are
* ordered lexicographically by name. Applied to the tools
* every assembly. `TOOL_ORDER_REST` is reserved for the list marker and may
* not be a collected tool name; such a provider output also rejects the
* assembly. The single assembly-time validation rejects either failure
* before any model request — the earliest moment the registered tool set
* exists to check against, since tool plugins register after this service
* constructs. 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
@@ -597,7 +600,7 @@ export interface Config {
}
```
Source: [`packages/core/system-prompt/src/index.ts:175`](../packages/core/system-prompt/src/index.ts)
Source: [`packages/core/system-prompt/src/index.ts:179`](../packages/core/system-prompt/src/index.ts)
## `@deepseek-ai/dsh-tool-fs`
@@ -13,6 +13,7 @@ The system-prompt assembly owns the canonical model-facing tool order, exactly w
- A listed tool that is registered takes its listed position.
- A listed name with no registered tool is a configuration error. Shape errors (rest entry missing or duplicate names) fail from the service constructor; an unregistered name rejects every `assemble()` — the earliest moment the registered tool set exists to check against (tool plugins register after the service constructs), and the only universal one (registrations can change at any time; cordis has no "all plugins loaded" event). Under the shipped loop the first turn fails before any model request — see the consequences below for the exact blast radius.
- A registered tool absent from the list is inserted at the `'<unlisted-tools>'` rest entry (`TOOL_ORDER_REST`), in lexicographic name order among the other unlisted tools.
- No collected tool may use `TOOL_ORDER_REST` as its `ToolSchema.name`; the assembly rejects that reserved name before ordering.
- The list must contain the rest entry exactly once and no duplicate names.
- When `toolOrder` is unset, the canonical order is plain lexicographic name order (code-unit comparison, locale-independent), so determinism requires no configuration.
@@ -41,7 +42,8 @@ Config plumbing follows the `persona` precedent, and `toolOrder` sits beside it:
- A pure tool reordering between steps is representable only as a `request/header` `'fallback'` snapshot (the name-keyed `ToolsDelta` cannot express it); with a stable canonical order such reorders no longer occur in practice, so the fallback path stays a safety valve.
- The `toolOrder` key rides the app → `agent-core``SystemPrompt` forwarding chain, so deployments set it next to `persona` in the app config; `dsh-llm` and the agent loop are untouched.
- A misspelled or unloaded tool name in `toolOrder` fails the turn at prompt assembly, not the boot: the loop assembles inside the turn (after `turn/start`, before `step/start`), so the rejection reaches the turn's outer catch — the turn closes balanced with an `error` reason carrying the message, `agent/error` mirrors it, no step opens, no `request/header` is logged, no request reaches the adapter, and the agent returns to idle. Every turn fails identically until the config is fixed; the process itself stays up (matching the repo rule that explicit config references must not be silently ignored — the enforcement point is the assembly because no earlier universal moment exists).
- A tool provider that returns the reserved rest-entry name has the same prompt-assembly failure shape as an unknown listed name. This keeps the sentinel from becoming an ambiguous real tool and preserves the "never drops a tool" ordering contract.
## Testing
Unit tests on `dsh-system-prompt` pin the ordering semantics (lexicographic default, listed/rest placement, unknown-name rejection at assembly, stable handling of shared names, provider-order independence), the pre-waterfall contract (listeners observe the canonical list; a listener-appended tool is not re-sorted), and each invalid-list rejection at load. Loop-level tests assert the `request/header` fold carries the canonical order for scrambled registration orders (identical across permutations), that a configured `toolOrder` reaches both the logged header and the dispatched deep-frozen request, that the frozen loop-built envelope survives to the adapter, and that an unregistered `toolOrder` name fails the turn with a balanced `error` `turn/end`, an `agent/error`, no step, no logged header, and no dispatched request. Forwarding is asserted at every level that exposes the key (`dsh-agent-core`, `dsh-stdio-agent`, `dsh-acp-agent`). The snapshot tier replays all scenarios while only the pinned `text-turn` header carries the full canonical tool list; non-pinning fixtures continue to compare through `{{tools}}`.
Unit tests on `dsh-system-prompt` pin the ordering semantics (lexicographic default, listed/rest placement, unknown-name rejection at assembly, reserved tool-name rejection, stable handling of shared names, provider-order independence), the pre-waterfall contract (listeners observe the canonical list; a listener-appended tool is not re-sorted), and each invalid-list rejection at load. Loop-level tests assert the `request/header` fold carries the canonical order for scrambled registration orders (identical across permutations), that a configured `toolOrder` reaches both the logged header and the dispatched deep-frozen request, that the frozen loop-built envelope survives to the adapter, and that an unregistered `toolOrder` name fails the turn with a balanced `error` `turn/end`, an `agent/error`, no step, no logged header, and no dispatched request. Forwarding is asserted at every level that exposes the key (`dsh-agent-core`, `dsh-stdio-agent`, `dsh-acp-agent`). The snapshot tier replays all scenarios while only the pinned `text-turn` header carries the full canonical tool list; non-pinning fixtures continue to compare through `{{tools}}`.
+3 -3
View File
@@ -7,16 +7,16 @@ 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 `'<unlisted-tools>'` rest entry (`TOOL_ORDER_REST`): listed tools take their listed position, unlisted tools land at the rest entry 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. Misconfiguration fails loud: a list without exactly one rest entry, or with duplicates, throws at load; a listed name with no registered tool rejects every `assemble()` — under the shipped loop the turn fails before any model request. 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). |
| `toolOrder` | — | Explicit model-facing tool order, as a list of `ToolSchema.name`s with one `'<unlisted-tools>'` rest entry (`TOOL_ORDER_REST`): listed tools take their listed position, unlisted tools land at the rest entry 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. Misconfiguration fails loud: a list without exactly one rest entry, or with duplicates, throws at load; a listed name with no registered tool rejects every `assemble()`; a tool provider returning the reserved rest-entry name also rejects. Under the shipped loop the turn fails before any model request. 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`)
### Public API
- `ctx.systemPrompt.section(section: PromptSection): () => void` Contribute a section. Duplicate names throw. Disposed with the calling fiber.
- `ctx.systemPrompt.tools(provider: () => ToolSchema[]): () => void` Contribute tool schemas (evaluated at each assembly). Disposed with the calling fiber.
- `ctx.systemPrompt.tools(provider: () => ToolSchema[]): () => void` Contribute tool schemas (evaluated at each assembly). A provider must not return a schema named `TOOL_ORDER_REST`; that name is reserved for `toolOrder`'s rest entry. Disposed with the calling fiber.
- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => void` Contribute a prompt variable, referenced from section text as `{{name}}`. Duplicate or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber.
- `ctx.systemPrompt.assemble(context?: AssembleContext): Promise<PromptAssembly>` Assemble the prompt for one caller. Runs through the `system-prompt/assemble` waterfall. Rejects when a configured `toolOrder` names a tool no provider contributed.
- `ctx.systemPrompt.assemble(context?: AssembleContext): Promise<PromptAssembly>` Assemble the prompt for one caller. Runs through the `system-prompt/assemble` waterfall. Rejects when a configured `toolOrder` names a tool no provider contributed, or when a provider returns the reserved rest-entry name.
### Events
+17 -7
View File
@@ -114,8 +114,8 @@ 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.
* Reserved: collected tool schemas using this name are rejected before
* ordering, so the marker can never collide with a real model-facing tool.
*/
export const TOOL_ORDER_REST = '<unlisted-tools>'
@@ -154,6 +154,10 @@ function validateToolOrder(toolOrder: string[] | undefined): string[] | undefine
* name keep their collection order.
*/
function orderTools(tools: ToolSchema[], toolOrder: string[] | undefined): ToolSchema[] {
const reserved = tools.find(tool => tool.name === TOOL_ORDER_REST)
if (reserved !== undefined) {
throw new Error(`tool provider returned reserved tool name "${TOOL_ORDER_REST}" (reserved for toolOrder's rest entry)`)
}
if (toolOrder === undefined) return tools.sort(compareToolNames)
const registered = new Set(tools.map(tool => tool.name))
const unknown = toolOrder.filter(name => name !== TOOL_ORDER_REST && !registered.has(name))
@@ -194,10 +198,13 @@ export interface Config {
* exactly once, no duplicate names, and no name without a registered tool —
* a misconfigured order blocks work instead of silently reaching a model
* request: shape violations throw at load, and an unregistered name rejects
* every assembly (failing the turn before any model request — the earliest
* moment the registered tool set exists to check against, since tool
* plugins register after this service constructs). When omitted, tools are
* ordered lexicographically by name. Applied to the tools
* every assembly. `TOOL_ORDER_REST` is reserved for the list marker and may
* not be a collected tool name; such a provider output also rejects the
* assembly. The single assembly-time validation rejects either failure
* before any model request — the earliest moment the registered tool set
* exists to check against, since tool plugins register after this service
* constructs. 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
@@ -356,7 +363,10 @@ export class SystemPrompt extends Service {
/**
* Contribute a tool-schema provider that is evaluated at each assembly
* call (so it can reflect the live registry state). The provider is
* removed when the calling fiber is disposed. Emits `system-prompt/change`.
* removed when the calling fiber is disposed. A provider must not return a
* schema named {@link TOOL_ORDER_REST}; that name is reserved for
* {@link Config.toolOrder}'s rest entry and rejects the assembly. Emits
* `system-prompt/change`.
* @param provider - evaluated at every {@link assemble} for fresh schemas.
* @returns the disposer that removes the provider.
*/
@@ -61,6 +61,16 @@ describe('SystemPrompt tool order', () => {
'toolOrder lists unregistered tool "ghost"; registered tools: (none)')
})
it.each([
['without an explicit toolOrder', undefined],
['with only the rest entry configured', [TOOL_ORDER_REST]],
])('rejects a provider tool named like the reserved rest entry %s', async (_case, toolOrder) => {
const ctx = await mount(toolOrder === undefined ? {} : { toolOrder })
ctx.systemPrompt.tools(() => [tool(TOOL_ORDER_REST)])
await expect(ctx.systemPrompt.assemble()).rejects.toThrow(
`tool provider returned reserved tool name "${TOOL_ORDER_REST}"`)
})
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')])