kevin/deepseek-harness
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(tools): shapeDispatchLog off the public registry surface

Browse Source 
Responding to review on #661: a public method on the generic ToolRegistry
service whose only caller is the run_code bridge was ad-hoc surface
widening. The bridge now receives it as a registry-private capability
closure in RunCodeBridgeOptions (the requireRuntime idiom, alongside the
cap), the method is private, and it leaves the generated service
catalog/API surfaces.

The pattern is now named as a code smell where reviewers look: the
packages/AGENTS.md capability-interface rule gains the inverse-smell
clause (ceiling 660→675 — the list is at capacity and the clause needs
one sentence), and dsh-code-review's capability-fit check tells reviewers
to flag single-consumer public service methods and require the closure
form.
This commit is contained in:
Tianyi Cui
2026-07-26 22:52:15 +08:00
parent a4644413e5
commit 443e2bc509
7 changed files with 33 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.agents/skills/dsh-code-review/SKILL.md
+1 -1
View File
@@ -30,7 +30,7 @@ description: Use when reviewing a pull request in the deepseek-harness repo —
- **Intent and seam contracts:** trace both sides of every changed interface. Confirm the implementation matches the PR and any Agent Note, including errors, cancellation, ownership, and disposal.
- **Lifecycle and concurrency:** for async setup, callbacks, processes, or teardown, apply [defensive-patterns.md](../../../docs/defensive-patterns.md). Check races before publication, cancellation during awaits, independent error reporting, callback containment, ownership before reentry, complete detach cleanup, and quiescent disposal.
- **Capability and consumer fit:** trace every current consumer, then flag consumer-specific behavior leaking into the interface under [the package contract](../../../packages/AGENTS.md).
- **Capability and consumer fit:** trace every current consumer, then flag consumer-specific behavior leaking into the interface under [the package contract](../../../packages/AGENTS.md). Flag the inverse too: a new public method on a generic service (registry, session, agent) whose only caller is one internal consumer is an ad-hoc surface widening — require a private capability closure handed to that consumer at construction instead.
- **Scope, ownership, and necessity:** map each abstraction, state machine, option, defensive copy, and compatibility path to its current contract, production consumer, and owning plugin or service. Challenge unrelated features and speculative generality, then test the PR's coherence against [the root contract](../../../AGENTS.md#conventions).
- **Configuration and public choices:** ask what current-consumer evidence or prior art supports each default, public operation set, format, or imported external concept. Require an explicit choice or deferral when that evidence is absent.
- **Model perspective:** inspect the exact prompts, tool schemas, results, and diagnostics the model receives across affected modes. Flag concepts outside the model's task, then verify stable text verbatim and dynamic behavior through snapshots or end-to-end coverage.
docs/cordis-catalog/services.md
+1 -11
View File
@@ -1830,16 +1830,6 @@ schemas(scope?: ScopeKey): ToolSchema[]
*/
executionMode(exec: ToolExecutionInput): ToolExecutionMode
/**
* Run the `tools/code-dispatch-log` waterfall over one settled sub-dispatch
* and return the content the bridge should log on `tool/code-dispatch`.
* Contained: a throwing listener falls back to the unshaped content — log
* shaping must never fail the dispatch or lose the settle event.
* @param dispatch - the sub-dispatch identity and its default logged content.
* @returns the (possibly reshaped) content for the durable event.
*/
async shapeDispatchLog(dispatch: CodeDispatchLog): Promise<ContentBlock[]>
/**
* Execute through pre-policy, guards, around-dispatch, post-policy,
* definition-owned content finalization, and final notification. Tool and
@@ -1857,7 +1847,7 @@ async shapeDispatchLog(dispatch: CodeDispatchLog): Promise<ContentBlock[]>
async execute(exec: ToolExecutionInput): Promise<ToolExecutionResult>
```
Types: [CodeDispatchLog](../core-data-structures/tools.md) · [ContentBlock](../core-data-structures/core.md) · [ScopeKey](../core-data-structures/scope.md) · [ToolDefinition](../core-data-structures/tools.md) · [ToolExecutionInput](../core-data-structures/tools.md) · [ToolExecutionMode](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) · [ToolGuard](../core-data-structures/tools.md) · [ToolRestriction](../core-data-structures/tools.md) · [ToolSchema](../core-data-structures/tools.md)
Types: [ScopeKey](../core-data-structures/scope.md) · [ToolDefinition](../core-data-structures/tools.md) · [ToolExecutionInput](../core-data-structures/tools.md) · [ToolExecutionMode](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) · [ToolGuard](../core-data-structures/tools.md) · [ToolRestriction](../core-data-structures/tools.md) · [ToolSchema](../core-data-structures/tools.md)
Source: [`packages/core/tools/src/index.ts:688`](../../packages/core/tools/src/index.ts)
packages/AGENTS.md
+1 -1
View File
@@ -7,7 +7,7 @@ These package-specific rules supplement the repo-wide [conventions](../AGENTS.md
- **Product-visible plugins require a non-unit REAL-composition test.** Hand-built `ctx.plugin(...)` suites are insufficient. Boot test-only `cordis.yml` through the Loader and app/process; mock only external/nondeterministic boundaries and assert model-visible, durable, or user-visible output. Keep opt-ins out of shipped defaults. [Policy](../docs/testing.md).
- **Initiator-owned private chains derive, then capture.** Under `ctx.agents.withInitiator()`, recover the Agent at each orchestration entry, derive `agent.session`, and let operation-local helpers close over it. Keep `Agent` and `Session` explicit at lifecycle, session-log, service, authority, worker/process, persistence, and wire interfaces; do not widen a leaf helper from `Session` to `Context` merely to hide a parameter ([rationale](../.agents/notes/implemented/architecture/2026-07-15-agent-initiator-scope.md)).
- **Represent one asynchronous operation with one lifecycle controller or transaction.** Separate readiness, cancellation, disposal, reservation, or sentinel state requires an independent owner or settlement boundary; otherwise fold it while preserving rollback, callback containment, and quiescence.
- **Shape capability interfaces around all current consumers.** Keep tool-schema, Loader, UI, transport, and backend-specific behavior in the consumer or adapter; do not let one consumer dictate the interface ([capability-seam rationale](../.agents/notes/implemented/architecture/2026-06-13-capability-seams.md)).
- **Shape capability interfaces around all current consumers.** Keep tool-schema, Loader, UI, transport, and backend-specific behavior in the consumer or adapter; do not let one consumer dictate the interface ([capability-seam rationale](../.agents/notes/implemented/architecture/2026-06-13-capability-seams.md)). Inverse smell: a public service method with one internal caller — pass a private capability closure instead (`RunCodeBridgeOptions`).
- **Require a current owner and need.** Tie each abstraction, state machine, option, defensive copy, and compatibility path to a current contract or production consumer, and keep behavior in its owning plugin or service.
- **Require evidence for public choices.** Configurability does not justify an unsupported default, public operation set, format, or imported external concept. Use current-consumer evidence or relevant prior art; otherwise require an explicit value or defer the choice.
- **Write model-facing contracts from the model's perspective.** Prompts, tool schemas, results, and diagnostics contain only task-relevant concepts, not UI, transport, or implementation vocabulary. Pin stable model-visible text verbatim and dynamic behavior through snapshots or end-to-end coverage.
packages/cordis/tool-cordis/src/api-catalog.ts
-8
View File
@@ -864,10 +864,6 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [
signature: 'executionMode(exec: ToolExecutionInput): ToolExecutionMode',
jsDoc: '/**\n * Classify a pending call through the caller\'s visible tool definition. Only\n * an exact `true` is parallel; unknown, hidden, undeclared, invalid, or\n * throwing classifiers are exclusive.\n * @param exec - call name, parsed arguments, and optional agent scope.\n * @returns the fail-closed scheduling mode.\n */',
},
{
signature: 'async shapeDispatchLog(dispatch: CodeDispatchLog): Promise<ContentBlock[]>',
jsDoc: '/**\n * Run the `tools/code-dispatch-log` waterfall over one settled sub-dispatch\n * and return the content the bridge should log on `tool/code-dispatch`.\n * Contained: a throwing listener falls back to the unshaped content — log\n * shaping must never fail the dispatch or lose the settle event.\n * @param dispatch - the sub-dispatch identity and its default logged content.\n * @returns the (possibly reshaped) content for the durable event.\n */',
},
{
signature: 'async execute(exec: ToolExecutionInput): Promise<ToolExecutionResult>',
jsDoc: '/**\n * Execute through pre-policy, guards, around-dispatch, post-policy,\n * definition-owned content finalization, and final notification. Tool and\n * listener failures resolve as materialized error results; an invisible tool\n * reports `UNKNOWN_TOOL`. The returned outcome is the same lossless, frozen\n * snapshot final observers receive. Cancellation\n * arriving after entry and before final result materialization skips a\n * not-yet-started body with `ABORTED_BEFORE_DISPATCH` or replaces a\n * successful started outcome with `ABORTED`; already-started work is still\n * drained and may retain a tool-owned structured error.\n * @param exec - the typed same-process call input. The registry assigns its\n * correlation token before policy begins.\n * @returns the materialized final result.\n */',
@@ -1439,10 +1435,6 @@ export const TYPE_API: readonly TypeApiEntry[] = [
name: 'CodeBindingNamespace',
declaration: 'export interface CodeBindingNamespace {\n global: string;\n functions: Record<string, CodeBindingFunction>;\n errorClass?: CodeBindingErrorClass;\n}',
},
{
name: 'CodeDispatchLog',
declaration: 'export interface CodeDispatchLog {\n readonly exec: ToolExecution;\n readonly agent?: Agent;\n readonly subCallId: CallId;\n readonly name: string;\n readonly isError: boolean;\n readonly content: ContentBlock[];\n}',
},
{
name: 'CodeJsonValue',
declaration: 'export type CodeJsonValue = null | boolean | number | string | CodeJsonValue[] | {\n [key: string]: CodeJsonValue;\n};',
packages/core/tools/src/code-mode.ts
+19 -7
View File
@@ -13,7 +13,7 @@ import { snapshotJsonValue } from '@deepseek-ai/dsh-session'
import type { JsonValue } from '@deepseek-ai/dsh-session'
import { defineTool } from './schema.ts'
import { TOOL_REGISTRY_SCHEDULER } from './index.ts'
import type { ToolDefinition, ToolExecutionResult, ToolRegistry, ToolRunContext } from './index.ts'
import type { CodeDispatchLog, ToolDefinition, ToolExecutionResult, ToolRegistry, ToolRunContext } from './index.ts'
declare module '@deepseek-ai/dsh-session' {
interface SessionEventMap {
@@ -186,6 +186,20 @@ function renderValue(value: JsonValue): string {
/** Canonical value returned by the outer Code Mode transport. */
type RunCodeOutput = { logs: string[]; result?: JsonValue }
/**
* Registry-private capabilities the bridge receives at construction — the
* `requireRuntime` idiom: operations only the owning registry can mint stay
* off its public service surface and flow here as closures instead.
*/
export interface RunCodeBridgeOptions {
/** Resolves `ctx.codeRuntime` or throws the loud misconfiguration error (shared with the registry's assembly-time checks). */
requireRuntime: () => CodeRuntime
/** The run's overlap cap for parallel-classified sub-calls (the registry passes its validated `maxParallelSubCalls`). */
maxParallel: number
/** Runs the contained `tools/code-dispatch-log` waterfall over one settled sub-dispatch (the registry's private invoker). */
shapeDispatchLog: (dispatch: CodeDispatchLog) => Promise<ContentBlock[]>
}
/**
* Build the `run_code` {@link ToolDefinition}: required `code` and
* `description` parameters, executed through the dispatch bridge described
@@ -194,13 +208,11 @@ type RunCodeOutput = { logs: string[]; result?: JsonValue }
* outside the filterable global/scoped capability layers.
* @param registry - the owning registry (sub-calls go through its `execute`,
* bindings cover its registered tools).
* @param requireRuntime - resolves `ctx.codeRuntime` or throws the loud
* misconfiguration error (shared with the registry's assembly-time checks).
* @param maxParallel - the run's overlap cap for parallel-classified
* sub-calls (the registry passes its validated `maxParallelSubCalls`).
* @param options - the registry-private capabilities described above.
* @returns the registry-ready definition.
*/
export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => CodeRuntime, maxParallel: number): ToolDefinition {
export function createRunCodeTool(registry: ToolRegistry, options: RunCodeBridgeOptions): ToolDefinition {
const { requireRuntime, maxParallel, shapeDispatchLog } = options
return defineTool({
name: RUN_CODE_NAME,
description:
@@ -407,7 +419,7 @@ export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () =>
// The durable copy may be reshaped (e.g. spilled to a preview +
// locator) by the log-shaping waterfall; the program's value
// and the model contract are untouched.
const logged = await registry.shapeDispatchLog({
const logged = await shapeDispatchLog({
exec, agent, subCallId, name, isError: result.isError,
// The registry deep-froze this projection at result
// finalization; append snapshots the final copy again, so
packages/core/tools/src/index.ts
+10 -5
View File
@@ -727,7 +727,11 @@ export class ToolRegistry extends Service {
// the filterable global/scoped capability layers.
this.codeTransport = this.mode === 'native'
? undefined
: createRunCodeTool(this, () => this.requireCodeRuntime(), resolveMaxParallelSubCalls(config.maxParallelSubCalls))
: createRunCodeTool(this, {
requireRuntime: () => this.requireCodeRuntime(),
maxParallel: resolveMaxParallelSubCalls(config.maxParallelSubCalls),
shapeDispatchLog: dispatch => this.shapeDispatchLog(dispatch),
})
ctx.systemPrompt.tools(context => this.wireSchemas(context.scope))
if (this.mode !== 'native') {
ctx.systemPrompt.section({
@@ -982,11 +986,12 @@ export class ToolRegistry extends Service {
* Run the `tools/code-dispatch-log` waterfall over one settled sub-dispatch
* and return the content the bridge should log on `tool/code-dispatch`.
* Contained: a throwing listener falls back to the unshaped content — log
* shaping must never fail the dispatch or lose the settle event.
* @param dispatch - the sub-dispatch identity and its default logged content.
* @returns the (possibly reshaped) content for the durable event.
* shaping must never fail the dispatch or lose the settle event. Private:
* the ONE consumer is the `run_code` bridge this registry constructs, which
* receives it as a capability parameter (the `requireRuntime` idiom) — the
* waterfall, not this invoker, is the public extension seam.
*/
async shapeDispatchLog(dispatch: CodeDispatchLog): Promise<ContentBlock[]> {
private async shapeDispatchLog(dispatch: CodeDispatchLog): Promise<ContentBlock[]> {
try {
return await this.ctx.waterfall(
scopeTarget(this, dispatch.agent), 'tools/code-dispatch-log', dispatch,
scripts/doc-budgets.manifest.json
+1 -1
View File
@@ -6,6 +6,6 @@
"docs/defensive-patterns.md": 550,
"docs/testing.md": 1100,
"examples/AGENTS.md": 310,
"packages/AGENTS.md": 660,
"packages/AGENTS.md": 675,
"packages/README.md": 835
}