mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
refactor(system-prompt): rename TOOL_ORDER_REST from '...' to '<unlisted-tools>'
A three-dot rest entry reads as elision in a cordis.yml; the spelled-out sentinel says what lands there. The literal now appears once in code (the constant) and once in the value-pinning test; every other reference — the forwarding tests included — imports TOOL_ORDER_REST, which adds the dsh-system-prompt devDependency to the two app packages. Review follow-up on #196.
This commit is contained in:
imccyu
@@ -560,8 +560,8 @@ export interface Config {
|
||||
* 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 —
|
||||
* {@link TOOL_ORDER_REST} (`'<unlisted-tools>'`) entry in lexicographic name order. A
|
||||
* configured list must contain the rest entry 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
|
||||
|
||||
@@ -10,12 +10,12 @@ The order of the tool list a model call carries — `request/header.tools` on th
|
||||
|
||||
The system-prompt assembly owns the canonical model-facing tool order, exactly where it already owns section order:
|
||||
|
||||
- **One config key on `dsh-system-prompt`.** `toolOrder?: string[]` names tools in the exact order to send. A listed tool takes its listed position; a listed name with no registered tool is ignored (a deployment may list optional tools it does not always load); tools absent from the list are inserted at the `'...'` rest entry (`TOOL_ORDER_REST`), in lexicographic name order among themselves. The list must contain `'...'` exactly once and no duplicate names — violations throw from the service constructor, failing the fiber at load, never mid-conversation. When `toolOrder` is unset, the canonical order is plain lexicographic name order (code-unit comparison, locale-independent) — determinism requires no configuration.
|
||||
- **One config key on `dsh-system-prompt`.** `toolOrder?: string[]` names tools in the exact order to send. A listed tool takes its listed position; a listed name with no registered tool is ignored (a deployment may list optional tools it does not always load); tools absent from the list are inserted at the `'<unlisted-tools>'` rest entry (`TOOL_ORDER_REST`), in lexicographic name order among themselves. The list must contain the rest entry exactly once and no duplicate names — violations throw from the service constructor, failing the fiber at load, never mid-conversation. When `toolOrder` is unset, the canonical order is plain lexicographic name order (code-unit comparison, locale-independent) — determinism requires no configuration.
|
||||
- **Applied where the list is born: `assemble()`, before the `system-prompt/assemble` waterfall.** The assembly canonicalizes the tools it collects from providers the same way it sorts sections by their `order` field — on the initial assembly, killing the registration-order entropy at its source. Everything downstream inherits the order untouched: the waterfall, the loop's `EpochHeader`, the `request/header` event, the deep-frozen request, and the dev invariant's cross-check all see one deterministic list, with no new service surface and no loop change.
|
||||
|
||||
Scope is deliberately narrow: this fixes the REGISTRATION-ORDER race, not plugin behavior. A `system-prompt/assemble` listener may still add, remove, or rearrange tools — same as it may edit sections after their sort — and owns the determinism of what it emits; the waterfall contract already demands deterministic listeners (the reconstructability invariant would catch a listener that diverges between build and replay).
|
||||
|
||||
Config plumbing follows the `persona` precedent, and `toolOrder` sits beside it: the app configs (`dsh-stdio-agent`, `dsh-acp-agent`) accept the key and forward it through `dsh-agent-core` (whose schema is the intersection of the owners' schemas) to the `SystemPrompt` child. One schemastery footnote is load-bearing: a schemastery array defaults to `[]`, but an omitted `toolOrder` must stay ABSENT (= lexicographic) rather than become an explicitly-configured empty list (invalid — it lacks `'...'`), so every schema on the chain forces the default to `undefined`.
|
||||
Config plumbing follows the `persona` precedent, and `toolOrder` sits beside it: the app configs (`dsh-stdio-agent`, `dsh-acp-agent`) accept the key and forward it through `dsh-agent-core` (whose schema is the intersection of the owners' schemas) to the `SystemPrompt` child. One schemastery footnote is load-bearing: a schemastery array defaults to `[]`, but an omitted `toolOrder` must stay ABSENT (= lexicographic) rather than become an explicitly-configured empty list (invalid — it lacks the rest entry), so every schema on the chain forces the default to `undefined`.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
@@ -25,7 +25,7 @@ Config plumbing follows the `persona` precedent, and `toolOrder` sits beside it:
|
||||
- **Sorting in `ToolRegistry.schemas()` (the registry layer)** — equally deterministic, but the registry is a membership store consumed by more than the assembly; ordering is a prompt-composition concern, and the assembly already owns the composition policy for sections.
|
||||
- **A `LlmService` config + `orderTools()` method the loop calls before logging the header** — works, but adds a public service method and a loop edit solely to apply a policy at a distance; every future request composer must remember the call. Canonicalizing where the list is born makes an unordered list unrepresentable, with zero new surface.
|
||||
- **Normalizing inside `llm.stream()`** — runs after the header event is logged (the flake survives) and rebuilds the deep-frozen envelope, silently disarming the reconstruction invariant.
|
||||
- **An exhaustive list (no `'...'` rest entry)** — every newly loaded tool plugin would break boot; the mandatory rest entry keeps unlisted tools deterministic and their position explicit.
|
||||
- **An exhaustive list (no rest entry)** — every newly loaded tool plugin would break boot; the mandatory rest entry keeps unlisted tools deterministic and their position explicit.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import Loader from '@cordisjs/plugin-loader'
|
||||
import { TOOL_ORDER_REST } from '@deepseek-ai/dsh-system-prompt'
|
||||
import * as agentCore from '../src/index.ts'
|
||||
import { AgentId } from '@deepseek-ai/dsh-agent'
|
||||
|
||||
@@ -68,7 +69,7 @@ describe('dsh-agent-core bundle', () => {
|
||||
})
|
||||
|
||||
it('forwards toolOrder to the system-prompt assembly', async () => {
|
||||
const ctx = await mount({ toolOrder: ['zulu', '...'] })
|
||||
const ctx = await mount({ toolOrder: ['zulu', TOOL_ORDER_REST] })
|
||||
// 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']) {
|
||||
|
||||
@@ -7,7 +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). |
|
||||
| `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, names with no registered tool are ignored, 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. A list without exactly one rest entry, 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`)
|
||||
|
||||
|
||||
@@ -117,11 +117,11 @@ const GROUP_AT = /^\{\{([^{}]*)\}\}/
|
||||
* Deliberately not a valid model-facing tool name, so it can never collide
|
||||
* with a real tool.
|
||||
*/
|
||||
export const TOOL_ORDER_REST = '...'
|
||||
export const TOOL_ORDER_REST = '<unlisted-tools>'
|
||||
|
||||
/**
|
||||
* Validate a configured tool-order list at service construction: `'...'`
|
||||
* ({@link TOOL_ORDER_REST}) exactly once, no duplicate names. Returns the list
|
||||
* Validate a configured tool-order list at service construction: the
|
||||
* {@link TOOL_ORDER_REST} rest entry 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.
|
||||
*/
|
||||
@@ -141,7 +141,7 @@ function validateToolOrder(toolOrder: string[] | undefined): string[] | undefine
|
||||
/**
|
||||
* 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
|
||||
* listed position and every unlisted tool lands at the {@link TOOL_ORDER_REST} rest entry in
|
||||
* lexicographic name order. Never drops a tool, and both sorts are stable, so
|
||||
* tools sharing a name keep their collection order.
|
||||
*/
|
||||
@@ -176,8 +176,8 @@ export interface Config {
|
||||
* 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 —
|
||||
* {@link TOOL_ORDER_REST} (`'<unlisted-tools>'`) entry in lexicographic name order. A
|
||||
* configured list must contain the rest entry 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
|
||||
@@ -264,8 +264,8 @@ export class SystemPrompt extends Service {
|
||||
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
|
||||
// explicitly-configured empty list (which is invalid — it lacks the
|
||||
// rest 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[]),
|
||||
|
||||
@@ -18,8 +18,8 @@ function names(assembly: PromptAssembly): string[] {
|
||||
}
|
||||
|
||||
describe('SystemPrompt tool order', () => {
|
||||
it('exports the rest entry as "..."', () => {
|
||||
expect(TOOL_ORDER_REST).toBe('...')
|
||||
it('exports the rest entry as "<unlisted-tools>"', () => {
|
||||
expect(TOOL_ORDER_REST).toBe('<unlisted-tools>')
|
||||
})
|
||||
|
||||
it('assembles tools in lexicographic name order when no toolOrder is configured', async () => {
|
||||
@@ -40,7 +40,7 @@ describe('SystemPrompt tool order', () => {
|
||||
expect(names(await backward.systemPrompt.assemble())).toEqual(['alpha', 'zulu'])
|
||||
})
|
||||
|
||||
it('applies a configured toolOrder: listed positions, rest at "..." lexicographically, absent names ignored', async () => {
|
||||
it('applies a configured toolOrder: listed positions, rest at the rest entry 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'])
|
||||
@@ -73,8 +73,8 @@ describe('SystemPrompt tool order', () => {
|
||||
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')
|
||||
])('rejects %s at load (the rest entry is required)', async (_case, toolOrder) => {
|
||||
await expect(new Context().plugin(SystemPrompt, { toolOrder })).rejects.toThrow(`must contain the "${TOOL_ORDER_REST}" rest entry`)
|
||||
})
|
||||
|
||||
it.each([
|
||||
|
||||
@@ -24,7 +24,7 @@ Because the package wires no logger entry, an ACP leaf has **nothing to get wron
|
||||
|---|---|---|
|
||||
| `model` | (required) | the per-session agent template the bridge creates agents from |
|
||||
| `persona` | — | the deployment persona template (may reference `{{model}}`/`{{cwd}}`), routed to `dsh-system-prompt` |
|
||||
| `toolOrder` | — | explicit model-facing tool order (a name list with one `'...'` rest entry; absent — lexicographic), routed to `dsh-system-prompt` |
|
||||
| `toolOrder` | — | explicit model-facing tool order (a name list with one `'<unlisted-tools>'` rest entry; absent — lexicographic), routed to `dsh-system-prompt` |
|
||||
| `persistenceRoot` | `./.sessions` | the JSONL backend's root directory |
|
||||
|
||||
The leaf supplies the swappable backends: an LLM adapter (`llm-deepseek` for the real model, `llm-replay` for keyless snapshot replay) and a bash executor (`bash-local`).
|
||||
|
||||
@@ -45,6 +45,7 @@
|
||||
"@deepseek-ai/dsh-app-boot": "workspace:^",
|
||||
"@deepseek-ai/dsh-acp": "workspace:^",
|
||||
"@deepseek-ai/dsh-agent-core": "workspace:^",
|
||||
"@deepseek-ai/dsh-system-prompt": "workspace:^",
|
||||
"@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^",
|
||||
"cordis": "^4.0.0-rc.6",
|
||||
"schemastery": "^3.17.0"
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import Loader from '@cordisjs/plugin-loader'
|
||||
import { TOOL_ORDER_REST } from '@deepseek-ai/dsh-system-prompt'
|
||||
import * as acpAgent from '../src/index.ts'
|
||||
|
||||
/**
|
||||
@@ -55,7 +56,7 @@ describe('dsh-acp-agent composition', () => {
|
||||
it('forwards toolOrder through agent-core to the system-prompt assembly', async () => {
|
||||
const ctx = await mount({
|
||||
model: 'mock',
|
||||
toolOrder: ['zulu', '...'],
|
||||
toolOrder: ['zulu', TOOL_ORDER_REST],
|
||||
persistenceRoot: '/tmp/dsh-acp-agent-test-tool-order',
|
||||
})
|
||||
// The bundle's own bash tools pend on the absent `ctx.bash` executor in
|
||||
|
||||
@@ -25,7 +25,7 @@ The leaf `cordis.yml` supplies only the **swappable backends** — an LLM adapte
|
||||
|---|---|---|
|
||||
| `model` | (required) | the pre-created `main` agent's model |
|
||||
| `persona` | — | the deployment persona template (may reference `{{model}}`), routed to `dsh-system-prompt` |
|
||||
| `toolOrder` | — | explicit model-facing tool order (a name list with one `'...'` rest entry; absent — lexicographic), routed to `dsh-system-prompt` |
|
||||
| `toolOrder` | — | explicit model-facing tool order (a name list with one `'<unlisted-tools>'` rest entry; absent — lexicographic), routed to `dsh-system-prompt` |
|
||||
| `persistenceRoot` | `./.sessions` | the JSONL backend's root directory |
|
||||
| `welcome` | `ready.` | the stdin-chat banner |
|
||||
| `resumeSessionId` | — | resume a persisted session id instead of starting fresh (sourced from an env var in the leaf) |
|
||||
|
||||
@@ -50,6 +50,7 @@
|
||||
"@deepseek-ai/dsh-agent": "workspace:^",
|
||||
"@deepseek-ai/dsh-llm": "workspace:^",
|
||||
"@deepseek-ai/dsh-agent-core": "workspace:^",
|
||||
"@deepseek-ai/dsh-system-prompt": "workspace:^",
|
||||
"@deepseek-ai/dsh-session": "workspace:^",
|
||||
"@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^",
|
||||
"cordis": "^4.0.0-rc.6",
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import Loader from '@cordisjs/plugin-loader'
|
||||
import { AgentId } from '@deepseek-ai/dsh-agent'
|
||||
import { TOOL_ORDER_REST } from '@deepseek-ai/dsh-system-prompt'
|
||||
import * as stdioAgent from '../src/index.ts'
|
||||
|
||||
/**
|
||||
@@ -77,7 +78,7 @@ describe('dsh-stdio-agent app', () => {
|
||||
it('forwards toolOrder through agent-core to the system-prompt assembly', async () => {
|
||||
const ctx = await mount({
|
||||
model: 'mock',
|
||||
toolOrder: ['zulu', '...'],
|
||||
toolOrder: ['zulu', TOOL_ORDER_REST],
|
||||
persistenceRoot: '/tmp/dsh-stdio-agent-spec-tool-order',
|
||||
})
|
||||
// The bundle's own bash tools pend on the absent `ctx.bash` executor in
|
||||
|
||||
21
pnpm-lock.yaml
generated
21
pnpm-lock.yaml
generated
@@ -899,6 +899,9 @@ importers:
|
||||
'@deepseek-ai/dsh-session-persistence-jsonl':
|
||||
specifier: workspace:^
|
||||
version: link:../../session-persistence/session-persistence-jsonl
|
||||
'@deepseek-ai/dsh-system-prompt':
|
||||
specifier: workspace:^
|
||||
version: link:../../core/system-prompt
|
||||
cordis:
|
||||
specifier: ^4.0.0-rc.6
|
||||
version: 4.0.0-rc.6(@cordisjs/plugin-include@vendor+include)(@cordisjs/plugin-loader@vendor+loader)
|
||||
@@ -947,6 +950,9 @@ importers:
|
||||
'@deepseek-ai/dsh-session-persistence-jsonl':
|
||||
specifier: workspace:^
|
||||
version: link:../../session-persistence/session-persistence-jsonl
|
||||
'@deepseek-ai/dsh-system-prompt':
|
||||
specifier: workspace:^
|
||||
version: link:../../core/system-prompt
|
||||
cordis:
|
||||
specifier: ^4.0.0-rc.6
|
||||
version: 4.0.0-rc.6(@cordisjs/plugin-include@vendor+include)(@cordisjs/plugin-loader@vendor+loader)
|
||||
@@ -2360,6 +2366,9 @@ packages:
|
||||
'@types/tough-cookie@4.0.5':
|
||||
resolution: {integrity: sha512-/Ad8+nIOV7Rl++6f1BdKxFSMgmoqEoYbHRpPcx3JEfv8VRsQe9Z4mCXeJBzxs7mbHY/XOZZuXlRNfhpVPbs6ZA==}
|
||||
|
||||
'@types/trusted-types@2.0.7':
|
||||
resolution: {integrity: sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==}
|
||||
|
||||
'@types/unist@3.0.3':
|
||||
resolution: {integrity: sha512-ko/gIFJRv177XgZsZcBwnqJN5x/Gien8qNOn0D5bQU/zAzVf9Zt3BlcUiLqhV9y4ARk0GbT3tnUiPNgnTXzc/Q==}
|
||||
|
||||
@@ -5090,6 +5099,9 @@ snapshots:
|
||||
|
||||
'@types/tough-cookie@4.0.5': {}
|
||||
|
||||
'@types/trusted-types@2.0.7':
|
||||
optional: true
|
||||
|
||||
'@types/unist@3.0.3': {}
|
||||
|
||||
'@typescript-eslint/eslint-plugin@8.61.0(@typescript-eslint/parser@8.61.0(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)':
|
||||
@@ -5183,7 +5195,10 @@ snapshots:
|
||||
'@typescript-eslint/types': 8.61.0
|
||||
eslint-visitor-keys: 5.0.1
|
||||
|
||||
'@upsetjs/venn.js@2.0.0': {}
|
||||
'@upsetjs/venn.js@2.0.0':
|
||||
optionalDependencies:
|
||||
d3-selection: 3.0.0
|
||||
d3-transition: 3.0.1(d3-selection@3.0.0)
|
||||
|
||||
'@vitest/coverage-v8@4.1.8(vitest@4.1.8)':
|
||||
dependencies:
|
||||
@@ -5573,7 +5588,9 @@ snapshots:
|
||||
|
||||
diff@9.0.0: {}
|
||||
|
||||
dompurify@3.4.11: {}
|
||||
dompurify@3.4.11:
|
||||
optionalDependencies:
|
||||
'@types/trusted-types': 2.0.7
|
||||
|
||||
dts-resolver@3.0.0(oxc-resolver@11.20.0):
|
||||
optionalDependencies:
|
||||
|
||||
Reference in New Issue
Block a user