fix: address web seam review findings

This commit is contained in:
Tianyi Cui
2026-07-01 17:08:53 +08:00
parent 29ce2df315
commit cf71c0b215
9 changed files with 64 additions and 14 deletions
+3 -1
View File
@@ -25,6 +25,8 @@ For a catalog of the **data structures** this architecture moves around — the
│ @deepseek-ai/dsh-bash-local (bash impl) │
│ @deepseek-ai/dsh-tool-bash (bash tool schemas) │
│ @deepseek-ai/dsh-web-search-exa (web search impl) │
│ @deepseek-ai/dsh-web-search-perplexity (web search impl) │
│ @deepseek-ai/dsh-web-search-deepseek (web search impl) │
│ @deepseek-ai/dsh-web-fetch-local (web fetch impl) │
│ @deepseek-ai/dsh-tool-web (web tool schemas) │
│ @deepseek-ai/dsh-subagent-* (subagent providers) │
@@ -78,7 +80,7 @@ Swappable capabilities are split into **three packages** so each part evolves in
The LLM seam has the same topology folded differently: `dsh-llm` carries the interface (`LlmAdapter`) AND the consumer surface (`ctx.llm.stream()`), with adapters as implementation packages — there the consumer is the loop itself, not a swappable schema surface. Use the full three-package split when the consumer is independently replaceable; keep interface + consumer together when they are one concern. Don't split preemptively: a capability with one conceivable implementation and one consumer stays one package until proven otherwise.
The web capability uses the same three-package split but folds two capabilities onto one seam: `dsh-web` owns the abstract `ctx.web` service, which is a provider REGISTRY (`registerSearchProvider`/`registerFetchProvider`, registration-order-independent selection, the `WebError` taxonomy) rather than a single backend. Providers register capabilities, not tools — `dsh-web-search-exa`, `dsh-web-search-perplexity`, and `dsh-web-fetch-local` each register into `ctx.web` the way an `LlmAdapter` registers into `ctx.llm`, so they are namespace plugins (`inject: ['web']`), not key-owning services. `dsh-tool-web` is the single consumer that owns the model-facing `web_search`/`web_fetch` schemas, prompt sections, and presentation; it reads only the aggregated `ctx.web.searchStatus()`/`fetchStatus()` and executes through `ctx.web.search()`/`fetch()`, so provider selection has one owner. Search and fetch are deliberately one seam (one thing to inject and configure, one selection policy, one abort/error vocabulary) despite sharing no request schema — see the [web capability seam RFC](rfc/implemented/architecture/2026-06-24-web-capability-seam.md).
The web capability uses the same three-package split but folds two capabilities onto one seam: `dsh-web` owns the abstract `ctx.web` service, which is a provider REGISTRY (`registerSearchProvider`/`registerFetchProvider`, registration-order-independent selection, the `WebError` taxonomy) rather than a single backend. Providers register capabilities, not tools — `dsh-web-search-exa`, `dsh-web-search-perplexity`, `dsh-web-search-deepseek`, and `dsh-web-fetch-local` each register into `ctx.web` the way an `LlmAdapter` registers into `ctx.llm`, so they are namespace plugins (`inject: ['web']`), not key-owning services. `dsh-tool-web` is the single consumer that owns the model-facing `web_search`/`web_fetch` schemas, prompt sections, and presentation; it reads only the aggregated `ctx.web.searchStatus()`/`fetchStatus()` and executes through `ctx.web.search()`/`fetch()`, so provider selection has one owner. Search and fetch are deliberately one seam (one thing to inject and configure, one selection policy, one abort/error vocabulary) despite sharing no request schema — see the [web capability seam RFC](rfc/implemented/architecture/2026-06-24-web-capability-seam.md).
> **"Capability" — two unrelated meanings.** (1) The *seam pattern* above ("one plugin provides a capability, another needs it") is realized by plain Cordis **services + `inject`**: a provider registers a service (`ctx.bash`, declared in `interface Context`); a consumer declares `inject: ['bash']` and its fiber stays pending until the service exists, tearing down via HMR if it later vanishes. No extra library is needed. (2) `@cordisjs/plugin-capability` is a different axis entirely — a **permission/capability-security** service (named permissions with inheritance/dependency, tested against a session via `ctx.capability.test`). It is a candidate for the deferred permissions/sandbox work (the `tools/execute` veto seam), NOT a mechanism for swapping implementations.
+2 -2
View File
@@ -1,6 +1,6 @@
# Web Access
The web access seam — a [capability seam](../rfc/implemented/architecture/2026-06-24-web-capability-seam.md) that spans **two capabilities** (search and fetch) on one `ctx.web` service, split across packages: interface ([dsh-web](../../packages/web/web), `ctx.web` + the provider registries), implementations ([dsh-web-search-exa](../../packages/web/web-search-exa), [dsh-web-search-perplexity](../../packages/web/web-search-perplexity), [dsh-web-fetch-local](../../packages/web/web-fetch-local)), and consumer ([dsh-tool-web](../../packages/web/tool-web), the `web_search`/`web_fetch` tool schemas). Web is **one optional capability**, not part of the agent-loop spine — so its vocabulary lives here, not in [core.md](core.md). A search-provider swap does not change how the model asks for a query, and a fetch-implementation swap does not change how the model asks for a URL.
The web access seam — a [capability seam](../rfc/implemented/architecture/2026-06-24-web-capability-seam.md) that spans **two capabilities** (search and fetch) on one `ctx.web` service, split across packages: interface ([dsh-web](../../packages/web/web), `ctx.web` + the provider registries), implementations ([dsh-web-search-exa](../../packages/web/web-search-exa), [dsh-web-search-perplexity](../../packages/web/web-search-perplexity), [dsh-web-search-deepseek](../../packages/web/web-search-deepseek), [dsh-web-fetch-local](../../packages/web/web-fetch-local)), and consumer ([dsh-tool-web](../../packages/web/tool-web), the `web_search`/`web_fetch` tool schemas). Web is **one optional capability**, not part of the agent-loop spine — so its vocabulary lives here, not in [core.md](core.md). A search-provider swap does not change how the model asks for a query, and a fetch-implementation swap does not change how the model asks for a URL.
Source: [`packages/web/web/src/types.ts`](../../packages/web/web/src/types.ts)
@@ -33,7 +33,7 @@ interface WebSearchResult {
}
```
`content` is optional provider-generated answer text (Exa returns none; Perplexity returns a generated answer). `sources[]` is the portable citation surface. A source always has a `url`; `title`/`snippet`/`publishedAt` are optional because not every provider returns them — Perplexity citations may be URL-only, and forcing adapters to invent the rest would make the seam lie. `dsh-tool-web` renders `title ?? hostname(url)`.
`content` is optional provider-generated answer text (Exa and DeepSeek return none; Perplexity returns a generated answer). `sources[]` is the portable citation surface. A source always has a `url`; `title`/`snippet`/`publishedAt` are optional because not every provider returns them — Perplexity citations may be URL-only, and forcing adapters to invent the rest would make the seam lie. `dsh-tool-web` renders `title ?? hostname(url)`.
```ts type-equiv
interface WebSearchSource {
@@ -17,7 +17,7 @@ There is also a provider-selection question. Existing `tool-bash` and `tool-fs`
Introduce web access as a first-class capability seam following [the capability-seam RFC](../../implemented/architecture/2026-06-13-capability-seams.md):
1. `@deepseek-ai/dsh-web` (`packages/web/web`) owns `ctx.web`, provider registration, provider selection, shared request/result vocabulary, and web-specific errors.
2. Provider packages implement concrete backends and register capabilities with `ctx.web`, for example `@deepseek-ai/dsh-web-search-exa`, `@deepseek-ai/dsh-web-search-perplexity`, and `@deepseek-ai/dsh-web-fetch-local`.
2. Provider packages implement concrete backends and register capabilities with `ctx.web`, for example `@deepseek-ai/dsh-web-search-exa`, `@deepseek-ai/dsh-web-search-perplexity`, `@deepseek-ai/dsh-web-search-deepseek`, and `@deepseek-ai/dsh-web-fetch-local`.
3. `@deepseek-ai/dsh-tool-web` (`packages/web/tool-web`) owns the model-facing `web_search` and `web_fetch` tool schemas, prompt sections, argument validation, result formatting, and tool-owned presentation over `ctx.web`.
Providers do not register tools. Providers register capabilities. `dsh-tool-web` is the only owner of model-facing names, descriptions, prompt guidance, JSON schemas, and presentation.
@@ -46,6 +46,8 @@ The dependency direction mirrors bash and filesystem:
consumer interface implementation
<--depends on-- @deepseek-ai/dsh-web-search-perplexity
implementation
<--depends on-- @deepseek-ai/dsh-web-search-deepseek
implementation
<--depends on-- @deepseek-ai/dsh-web-fetch-local
implementation
```
@@ -56,6 +58,7 @@ At runtime, provider packages register capabilities with `ctx.web`; `tool-web` r
flowchart LR
exa["@deepseek-ai/dsh-web-search-exa"] -->|registerSearchProvider| web["@deepseek-ai/dsh-web / ctx.web"]
perplexity["@deepseek-ai/dsh-web-search-perplexity"] -->|registerSearchProvider| web
deepseek["@deepseek-ai/dsh-web-search-deepseek"] -->|registerSearchProvider| web
fetchLocal["@deepseek-ai/dsh-web-fetch-local"] -->|registerFetchProvider| web
toolWeb["@deepseek-ai/dsh-tool-web"] -->|searchStatus/fetchStatus| web
toolWeb -->|ctx.tools.register| webSearch["tool: web_search"]
@@ -152,6 +155,9 @@ The "single provider auto-selects" rule is for tests, demos, and simple deployme
- id: web-search-perplexity
name: '@deepseek-ai/dsh-web-search-perplexity'
- id: web-search-deepseek
name: '@deepseek-ai/dsh-web-search-deepseek'
- id: web-fetch-local
name: '@deepseek-ai/dsh-web-fetch-local'
@@ -326,10 +332,11 @@ Land the work in seam order:
1. Add `packages/web/web` with `ctx.web`, provider registration, provider status, capability status, selection, request/result/error types, and contract tests.
2. Add `packages/web/web-search-exa` with parser/unit tests and a self-skipping real-provider smoke test.
3. Add `packages/web/web-search-perplexity` with parser/unit tests and a self-skipping real-provider smoke test.
4. Add `packages/web/web-fetch-local` with local HTTP behavior tests.
5. Add `packages/web/tool-web` with config-driven tool registration, prompt sections, model formatting, presentation, and tool-registry tests.
6. Wire product app/example configs only after package behavior is stable, because tool schemas and prompt sections affect agent behavior and snapshots.
7. Update `docs/architecture.md`, `packages/README.md`, package READMEs, generated Cordis catalogs if new events/services are added, and maintenance scripts.
4. Add `packages/web/web-search-deepseek` with parser/unit tests and a self-skipping real-provider smoke test.
5. Add `packages/web/web-fetch-local` with local HTTP behavior tests.
6. Add `packages/web/tool-web` with config-driven tool registration, prompt sections, model formatting, presentation, and tool-registry tests.
7. Wire product app/example configs only after package behavior is stable, because tool schemas and prompt sections affect agent behavior and snapshots.
8. Update `docs/architecture.md`, `packages/README.md`, package READMEs, generated Cordis catalogs if new events/services are added, and maintenance scripts.
## Alternatives considered
+2
View File
@@ -38,6 +38,7 @@ dsh-tool-bash ← dsh-bash, dsh-tools (bash tool schemas)
dsh-web ← dsh-llm (abstract web seam; search/fetch registries, WebError)
dsh-web-search-exa ← dsh-web (Exa WebSearchProvider)
dsh-web-search-perplexity ← dsh-web (Perplexity WebSearchProvider)
dsh-web-search-deepseek ← dsh-web (DeepSeek native-web-search WebSearchProvider)
dsh-web-fetch-local ← dsh-web (anonymous public HTTP(S) WebFetchProvider)
dsh-tool-web ← dsh-web, dsh-tools, dsh-system-prompt (web tool schemas)
dsh-llm-deepseek ← dsh-llm (DeepSeek adapter)
@@ -80,6 +81,7 @@ The rule: **extension** plugins depend on interfaces, never on the concrete loop
| `web/` | `web` | Abstract web seam (search/fetch provider registries + selection + vocabulary + `WebError`) | `ctx.web` |
| `web-search-exa/` | `web` | Exa-backed `WebSearchProvider` | (registers on `ctx.web`) |
| `web-search-perplexity/` | `web` | Perplexity-backed `WebSearchProvider` | (registers on `ctx.web`) |
| `web-search-deepseek/` | `web` | DeepSeek-backed `WebSearchProvider` using native `web_search` through the Anthropic-compatible API | (registers on `ctx.web`) |
| `web-fetch-local/` | `web` | Anonymous public HTTP(S) `WebFetchProvider` | (registers on `ctx.web`) |
| `tool-web/` | `web` | Model-facing `web_search`/`web_fetch` tool schemas | (registers on `ctx.tools`) |
| `llm-deepseek/` | `llm` | DeepSeek API adapter (hand-rolled fetch/SSE) | (registers on `ctx.llm`) |
+1
View File
@@ -7,6 +7,7 @@ The web access capability seam: an abstract web interface, search/fetch provider
| `web/` | Abstract web seam (search/fetch provider registries + selection + vocabulary + `WebError`) | `ctx.web` |
| `web-search-exa/` | Exa-backed `WebSearchProvider` | (registers on `ctx.web`) |
| `web-search-perplexity/` | Perplexity-backed `WebSearchProvider` | (registers on `ctx.web`) |
| `web-search-deepseek/` | DeepSeek-backed `WebSearchProvider` using native `web_search` through the Anthropic-compatible API | (registers on `ctx.web`) |
| `web-fetch-local/` | Anonymous public HTTP(S) `WebFetchProvider` | (registers on `ctx.web`) |
| `tool-web/` | Model-facing `web_search`/`web_fetch` tool schemas | (registers on `ctx.tools`) |
+2 -2
View File
@@ -20,8 +20,8 @@ It reuses `$DEEPSEEK_API_KEY` (no new secret) but **not** `$DEEPSEEK_BASE_URL`:
| `baseURL` | `https://api.deepseek.com/anthropic/v1` | Anthropic-compatible endpoint base; `/messages` is appended. Use a separate env var such as `$DEEPSEEK_SEARCH_BASE_URL` when overriding it; do not reuse `$DEEPSEEK_BASE_URL`, which belongs to the chat-completions LLM adapter. An unparseable value makes `status()` report `misconfigured`. |
| `model` | `deepseek-v4-flash` | Anthropic-format model name. |
| `apiVersion` | `2023-06-01` | `anthropic-version` header value. |
| `maxTokens` | `4096` | Upper bound on generated tokens for the Messages request. |
| `maxUses` | `5` | Maximum `web_search` server-tool uses per request. |
| `maxTokens` | `4096` | Positive-integer upper bound on generated tokens for the Messages request. |
| `maxUses` | `5` | Positive-integer maximum `web_search` server-tool uses per request. |
```yaml
- id: web-search-deepseek
@@ -64,18 +64,20 @@ export const Config: z<Config> = z.object({
baseURL: z.string(),
model: z.string(),
apiVersion: z.string(),
maxTokens: z.natural(),
maxUses: z.natural(),
maxTokens: z.number().step(1).min(1),
maxUses: z.number().step(1).min(1),
})
/** Register the DeepSeek search provider with `ctx.web`. */
export function apply(ctx: Context, config: Config): void {
const maxTokens = config.maxTokens ?? DEEPSEEK_DEFAULT_MAX_TOKENS
const maxUses = config.maxUses ?? DEEPSEEK_DEFAULT_MAX_USES
ctx.web.registerSearchProvider(new DeepSeekSearchProvider({
apiKey: config.apiKey ?? process.env.DEEPSEEK_API_KEY ?? '',
baseURL: config.baseURL ?? DEEPSEEK_DEFAULT_BASE_URL,
model: config.model ?? DEEPSEEK_DEFAULT_MODEL,
apiVersion: config.apiVersion ?? DEEPSEEK_DEFAULT_API_VERSION,
maxTokens: config.maxTokens ?? DEEPSEEK_DEFAULT_MAX_TOKENS,
maxUses: config.maxUses ?? DEEPSEEK_DEFAULT_MAX_USES,
maxTokens,
maxUses,
}))
}
@@ -147,6 +147,7 @@ export class DeepSeekSearchProvider implements WebSearchProvider {
status(): WebProviderStatus {
if (this.options.apiKey.length === 0) return { available: false, reason: 'missing-credential' }
if (!URL.canParse(this.options.baseURL)) return { available: false, reason: 'misconfigured' }
if (!isPositiveInteger(this.options.maxTokens) || !isPositiveInteger(this.options.maxUses)) return { available: false, reason: 'misconfigured' }
return { available: true }
}
@@ -215,3 +216,8 @@ export class DeepSeekSearchProvider implements WebSearchProvider {
function isAbortError(error: unknown): boolean {
return error instanceof DOMException && error.name === 'AbortError'
}
/** True for DeepSeek request limits that can be sent to the Messages API. */
function isPositiveInteger(value: number): boolean {
return Number.isInteger(value) && value > 0
}
@@ -152,6 +152,15 @@ describe('DeepSeekSearchProvider status', () => {
expect(new DeepSeekSearchProvider({ ...options, baseURL: 'not a url' }).status())
.toEqual({ available: false, reason: 'misconfigured' })
})
it('is misconfigured when request limits are not positive integers', () => {
expect(new DeepSeekSearchProvider({ ...options, maxTokens: 0 }).status())
.toEqual({ available: false, reason: 'misconfigured' })
expect(new DeepSeekSearchProvider({ ...options, maxUses: 0 }).status())
.toEqual({ available: false, reason: 'misconfigured' })
expect(new DeepSeekSearchProvider({ ...options, maxUses: 1.5 }).status())
.toEqual({ available: false, reason: 'misconfigured' })
})
})
describe('DeepSeekSearchProvider request mapping', () => {
@@ -263,6 +272,27 @@ describe('web-search-deepseek plugin registration', () => {
expect(ctx.web.searchStatus()).toEqual({ available: false, reason: 'configured-missing' })
})
it('rejects maxTokens: 0 at plugin construction', async () => {
const ctx = new Context()
await ctx.plugin(WebService, { searchProvider: DEEPSEEK_PROVIDER_ID })
await expect(ctx.plugin(deepseekPlugin, { apiKey: 'ds-key', maxTokens: 0 }))
.rejects.toThrow(/maxTokens expected number >= 1/)
})
it('rejects maxUses: 0 at plugin construction', async () => {
const ctx = new Context()
await ctx.plugin(WebService, { searchProvider: DEEPSEEK_PROVIDER_ID })
await expect(ctx.plugin(deepseekPlugin, { apiKey: 'ds-key', maxUses: 0 }))
.rejects.toThrow(/maxUses expected number >= 1/)
})
it('rejects a fractional maxUses at plugin construction', async () => {
const ctx = new Context()
await ctx.plugin(WebService, { searchProvider: DEEPSEEK_PROVIDER_ID })
await expect(ctx.plugin(deepseekPlugin, { apiKey: 'ds-key', maxUses: 1.5 }))
.rejects.toThrow(/maxUses expected number multiple of 1/)
})
it('has no default export (namespace plugin export shape)', () => {
expect('default' in deepseekPlugin).toBe(false)
})