diff --git a/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.i18n.yaml index 8ec7ff129e..1c04f72d28 100644 --- a/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-30-web-config-plane.md -2026-07-30-web-config-plane.md: 11554077d1848dcdf59b896dd9c29a39fd2f55d4 -2026-07-30-web-config-plane.zh.md: 527c2de8155a56789358b801f9c374e16c81931b +2026-07-30-web-config-plane.md: 483058c8a24ad8c414278eba84f4226e973f2923 +2026-07-30-web-config-plane.zh.md: 9247aac52401cd570d37f4aad47d9da6bb194d19 diff --git a/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.md b/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.md index 11554077d1..483058c8a2 100644 --- a/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.md +++ b/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.md @@ -16,7 +16,7 @@ PR1 made LLM adapter configuration restart-free at the seam, but the only writer **`describe()` grows layers and structural secret redaction.** `SettingsDescriptor` carries `base`/`user` beside the effective value, so the form marks "overridden" by presence in the user layer, not value inequality (an override *equal* to the base is still an override). `describe({ redactSecrets: true })` — mandatory at every wire face — strips `role('secret')` subtrees from all three layers via a pure structural walk of the schema (object/dict/array containers; a secret-role subtree is one opaque leaf) and enumerates the stripped slots as `{path, set}`, so a page can render write-only inputs without ever receiving a value. -**The Host identifies and opens the local settings document.** The settings seam exposes optional `documentPath` provider metadata and a `prepareDocument()` operation; `settings-local` returns its fully resolved custom or `$DSH_HOME/settings.yaml` filename and exclusively creates an absent empty document with owner-only permissions, while non-file providers retain the base `undefined`. The loopback-only `settings.describe` response carries only the boolean `hasDocument` capability beside the redacted namespace views. `ui-settings-general` registers a `settings.action` entry only on loopback pages, shows it only after the metadata confirms that a provider-owned local document can be prepared, and invokes pathless `settings.openDocument`; the Host resolves the provider path again before a text-document handoff (`open -t` on macOS so an arbitrary YAML file association cannot redirect the gesture, `xdg-open` on Linux, and `Invoke-Item` on Windows). Generic workspace paths retain the existing default-application handoff. The browser neither derives `$DSH_HOME` nor receives a filesystem target; remote pages make no privileged settings read for this action. +**The Host identifies and opens the local settings document.** The settings seam exposes optional `documentPath` provider metadata and a `prepareDocument()` operation; `settings-local` returns its fully resolved custom or `$DSH_HOME/settings.yaml` filename and exclusively creates an absent empty document with owner-only permissions, while non-file providers retain the base `undefined`. The loopback-only `settings.describe` response carries only the boolean `hasDocument` capability beside the redacted namespace views. `ui-settings-general` registers a `settings.action` entry only on loopback pages, shows it only after the metadata confirms that a provider-owned local document can be prepared, and invokes pathless `settings.openDocument`; the Host resolves the provider path again before a text-document handoff (`open -t` on macOS so an arbitrary YAML file association cannot redirect the gesture, `xdg-open` on desktop Linux, `Invoke-Item` on Windows, and `wslpath -w` followed by that Windows handoff on WSL). Generic workspace paths retain the default intent, including its browser preference for browser-renderable documents. The browser neither derives `$DSH_HOME` nor receives a filesystem target; remote pages make no privileged settings read for this action. **The llm seam declares configurability and announces topology.** `registerConfigurableProviders()` is an all-or-nothing, fiber-scoped directory of `{provider, displayName, settingsNs, settingsPath}` — the addressing a config page needs to open the right settings subtree for a route that may not exist yet; `listConfigurableProviders()` merges with live routes in the wire handler so undeclared live routes still report active. The zero-payload `'llm/adapters-updated'` event fires from all four registration/unregistration commit points with contained listener dispatch (INVARIANT rethrow), following the settings/commands precedent. `llm-deepseek`'s route renamed to `deepseek-official` because the pi-ai catalog legitimately owns `deepseek` as an aggregator entry; pre-release stance, no alias. diff --git a/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.zh.md b/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.zh.md index 527c2de815..9247aac524 100644 --- a/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-30-web-config-plane.zh.md @@ -16,7 +16,7 @@ PR1 让 LLM（大语言模型）适配器配置在 seam 层面免重启，但唯 **`describe()` 增加分层与结构化 secret 脱敏。**`SettingsDescriptor` 在生效值之外携带 `base`/`user`，表单据此按「字段是否出现在用户层」来标记「已覆盖」，而非按值是否不等（与 base *相等*的覆盖仍然是覆盖）。`describe({ redactSecrets: true })`——在每个 wire 面都强制启用——经由对 schema 的纯结构遍历（object/dict/array 容器；secret 角色子树整体是一个不透明叶节点）从全部三层剥除 `role('secret')` 子树，并把剥除的槽位枚举为 `{path, set}`，页面因此不必收到任何值就能渲染只写输入框。 -**Host 识别并打开本地设置文档。** settings seam 暴露可选的 `documentPath` 提供方元数据和 `prepareDocument()` 操作；`settings-local` 返回已完全解析的自定义文件名或 `$DSH_HOME/settings.yaml` 文件名，并在文档缺失时以仅属主可访问的权限独占创建空文档，非文件提供方则保留基类的 `undefined`。仅限回环访问的 `settings.describe` 响应会在脱敏 namespace 视图旁只携带布尔型 `hasDocument` 能力。`ui-settings-general` 只在回环页面注册一条 `settings.action` 条目，只有元数据确认可准备好一份由提供方持有的本地文档后才显示，并调用无路径参数的 `settings.openDocument`；Host 会在文本文档交接前再次解析提供方路径（macOS 上使用 `open -t`，使任意 YAML 文件关联无法重定向这次操作；Linux 上使用 `xdg-open`；Windows 上使用 `Invoke-Item`）。通用 Workspace 路径仍保留现有的默认应用交接。浏览器既不推导 `$DSH_HOME`，也不会收到文件系统目标；远程页面不会为这项操作发起特权 settings 读取。 +**Host 识别并打开本地设置文档。** settings seam 暴露可选的 `documentPath` 提供方元数据和 `prepareDocument()` 操作；`settings-local` 返回已完全解析的自定义文件名或 `$DSH_HOME/settings.yaml` 文件名，并在文档缺失时以仅属主可访问的权限独占创建空文档，非文件提供方则保留基类的 `undefined`。仅限回环访问的 `settings.describe` 响应会在脱敏 namespace 视图旁只携带布尔型 `hasDocument` 能力。`ui-settings-general` 只在回环页面注册一条 `settings.action` 条目，只有元数据确认可准备好一份由提供方持有的本地文档后才显示，并调用无路径参数的 `settings.openDocument`；Host 会在文本文档交接前再次解析提供方路径（macOS 上使用 `open -t`，使任意 YAML 文件关联无法重定向这次操作；桌面 Linux 上使用 `xdg-open`；Windows 上使用 `Invoke-Item`；WSL 上先执行 `wslpath -w`，再使用同一 Windows 交接）。通用 Workspace 路径仍保留默认意图，包括针对浏览器可渲染文档的浏览器偏好。浏览器既不推导 `$DSH_HOME`，也不会收到文件系统目标；远程页面不会为这项操作发起特权 settings 读取。 **llm seam 声明可配置性并公布拓扑。**`registerConfigurableProviders()` 是一个全有或全无、以 fiber 为作用域的目录，条目为 `{provider, displayName, settingsNs, settingsPath}`——这正是配置页要为一条可能尚不存在的路由打开正确设置子树时所需要的寻址；`listConfigurableProviders()` 在 wire 处理器里与存活路由合并，未声明的存活路由因此仍报告为激活。零负载的 `'llm/adapters-updated'` 事件从全部四个注册／注销提交点触发，listener 派发带异常隔离（INVARIANT 重抛），沿用 settings/commands 的先例。`llm-deepseek` 的路由重命名为 `deepseek-official`，因为 pi-ai catalog 名正言顺地拥有 `deepseek` 这个聚合器条目；依预发布立场，不设别名。 diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml new file mode 100644 index 0000000000..123058ac7e --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md +2026-08-06-api-key-format-validation.md: d1f6d31d362b76392514704be780f553b45d36ad +2026-08-06-api-key-format-validation.zh.md: 75b3fa247bdf449964a874e909e6e3bc9e0694fa diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md new file mode 100644 index 0000000000..d1f6d31d36 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md @@ -0,0 +1,107 @@ +# Agent Note: Validate API key format before it reaches an HTTP header + +Status: implemented + +English | [中文](2026-08-06-api-key-format-validation.zh.md) + +## Problem + +An API key holding characters no HTTP header value can carry was accepted by every configuration surface and failed only when a request was built, far from the field that caused it. + +Pasting a key containing an emoji, CJK text, or a full-width punctuation mark into the web Models page reported a successful save. The first turn then failed with `Cannot convert argument to a ByteString because the character at index 7 has a value of 55357 which is greater than 255` — the index and code point are UTF-16 internals with no action attached, and they disclose the code point of one character of the key. `llm-deepseek` produced this because `fetch` builds the `Bearer` header inside the `try` in [adapter.ts](../../../../packages/llm/llm-deepseek/src/adapter.ts), whose `catch` labels every failure `TRANSPORT`; that label is in `DEFAULT_RETRYABLE_CODES`, so a permanent, deterministic fault was also retried three times. + +`llm-pi-ai` was worse on the same input. Its discovery probe builds the same header with a bare `fetch` in [discovery.ts](../../../../packages/llm/llm-pi-ai/src/discovery.ts) and wrapped every failure as `could not reach `, so a local key fault was reported as an unreachable network. The probe is reachable from the unsaved draft: `ProviderEditor` puts the typed `keyDraft` into its probe request, so the model-listing button sent an illegal key before anything was stored. + +Whitespace passed every check. `ProviderEditor` tested `keyDraft.length` and `resolveAdapterOptions` tested `config.apiKey.length`, so a key of three spaces stored and then authenticated as `Bearer` plus blanks. `llm-pi-ai` rejected an empty literal `apiKey` in `resolveProfiles`, but applied no check whatsoever to a credential- or environment-sourced key — the path the Models page writes, and therefore the path users actually take. + +Sources: deepseek-harness#1594 and #1595; dsh-external#247, #249, #266, and #210. + +## Decision + +One rule defines a legal key: **after trimming, non-empty, and every character within `[\x21-\x7E]`** — printable ASCII, space excluded. + +This single predicate covers every input the sources list: empty, leading and trailing whitespace, interior whitespace, C0 control characters, emoji, CJK text, and full-width punctuation. It is also exactly the constraint that produced the ByteString failure, so the two issues close on one definition rather than on two coincidentally related fixes. + +A second, narrower rule catches a pasted environment line: input matching `^[A-Z][A-Z0-9_]*=[^=]` or wrapped in matching quotes is refused. Restricting the prefix to upper-case keeps real keys clear of it — `sk-` forms break the identifier match at the hyphen — and requiring a non-`=` character after the separator keeps base64 padding clear of it too. It reports the same format failure as an illegal character rather than its own message: the reader's next move is identical either way, so a separate line would name a cause without changing what to do. + +### Invariants belong at every layer; heuristics belong where the human is + +The charset rule is an invariant. A non-ASCII character *cannot* travel in a header value for any provider, so enforcing it in the browser, in each resolver, and on every credential read is consistent by construction rather than by agreement. + +The shape rule is a guess about how people paste, so it runs **only in the browser**. `llm-pi-ai` fronts OpenAI, Anthropic, and arbitrary hand-declared gateways whose key formats this repository does not own; a gateway issuing a key shaped like `TENANT1=abc` would, if the rule ran in the resolver, be locked out with no escape — the settings page would refuse it and a hand-written `.env` would be rejected on read. Confining the heuristic to the surface where the paste happens keeps the environment as the way through. + +### Absence is a configuration state, not a missing key + +"No API key" means three different things here, and only one of them is an error. The rule applies to a value that was *provided*; deciding whether one was provided at all stays with each caller. + +**Omitted.** A profile naming neither `apiKey` nor `apiKeyEnv` is authenticated by something other than a harness-held key. `routeAuth` in [provider.ts](../../../../packages/llm/llm-pi-ai/src/provider.ts) keeps the installed catalog provider's own auth precisely so provider-native ambient discovery survives, and `openai-codex` — shipped in that catalog — authenticates through OAuth and refuses an explicit key outright. `namesCredential` carries this distinction. In `llm-deepseek`, an absent `apiKey` likewise falls through to `apiKeyEnv`. Omission is never validated. + +**A blank field in the web UI.** The key input opens empty even for a provider whose key is already stored — the `keyStored` copy reads "Configured — enter a new value to replace" — so blank means *keep what is stored*. `ProviderEditor` skips `credentials.set` entirely when the draft is empty, and that stays a no-op: a blank field never blocks submit, or editing a base URL would demand re-entering the key. + +**Provided, but empty or whitespace-only.** What this means depends on what absence selects for that surface, and the two adapters differ for a reason. In `llm-pi-ai` it is an error, because absence there switches authentication mode — to the installed provider's ambient discovery or OAuth — so a blank key leaves genuine ambiguity about which was meant; its wording names the legitimate alternative rather than just refusing (*has an empty apiKey; omit it to use ambient authentication*). In `llm-deepseek` absence merely selects a different *source* for the same key, `apiKeyEnv`, so a blank literal resolves through that fallback exactly as an omitted one does. In the browser it is always a failure, on both cards: the field is where a person just typed, and silently discarding what they typed is never the right answer. + +`normalizeApiKey` therefore takes `string`, never `string | undefined`. + +### Where the rule lives + +`normalizeApiKey` is a module of the `dsh-llm` seam, beside [attribution.ts](../../../../packages/llm/llm/src/attribution.ts), which already owns shared header concerns. Both adapters depend on the seam and both need the rule, so it has two current consumers rather than a speculative one. It returns the trimmed value or a reason (`empty`, `illegalCharacters`). + +Both adapters also need the identical "refuse a stored credential" diagnosis, differing only by package prefix. `LlmError` is declared in the seam's `index.ts`, so `assertUsableApiKey(raw, pkg, ref)` lives there beside it and neither adapter carries a local copy. The predicate module stays dependency-free: importing `LlmError` into `api-key.ts` would cycle with `index.ts`'s re-export of it. + +The client cannot import any of this: client packages reference only client packages, so `packages/client/ui-models` mirrors the predicate in its own `apiKey.ts` and owns the localized messages, exactly as `validateDeepSeekModels` mirrors the host's `catalogModel` schema. Each side names the other in a comment. + +### What each surface does + +| Surface | Behavior | +|---|---| +| `dsh-llm` | Owns `normalizeApiKey`, `assertUsableApiKey`, and `INVALID_CREDENTIAL_CODE`, which is deliberately outside `DEFAULT_RETRYABLE_CODES`. | +| `llm-deepseek` `resolveAdapterOptions` | Refuses a literal `apiKey` no header can carry, beside the other beyond-schema bounds; uses the trimmed value. An absent or blank one falls through to `apiKeyEnv`. | +| `llm-deepseek` `resolveApiKey` | Normalizes what the credentials seam or environment returns, rejecting with `INVALID_CREDENTIAL` naming the Models page and never echoing the key. | +| `llm-pi-ai` `resolveProfiles` | Applies the shared rule, keeping its "omit it to use ambient authentication" wording, and writes the trimmed value into the resolved profile. | +| `llm-pi-ai` `resolveApiKey` | Normalizes the credential and environment paths. A profile naming no credential still returns `undefined`, so ambient and OAuth routes are unaffected. | +| `llm-pi-ai` `discoverModels` | Normalizes before building the header, so an illegal key is a credential fault rather than an unreachable endpoint. A probe carrying no key stays unauthenticated. | +| `ui-models` | Mirrors the charset rule, adds the shape heuristic, trims `keyDraft` before probe and `credentials.set`, and fixes the `stringAt` emptiness test. A blank field remains a no-op that submits; a field holding only whitespace is a field-level failure. Submit **and the endpoint interrogation** are both gated, so a refused key never spends a round trip to be told what the field already says, and the failure renders on the field, matching the existing `modelFailure` pattern. | + +`ProviderEditor` serves both the DeepSeek and pi-ai layouts, so one client change covers both providers. `CustomProviderCard` carries the same judgement for a hand-declared route. + +`credentials-local` is deliberately untouched. It stores credentials generally, and printable-ASCII is a constraint of HTTP headers rather than of credential storage; its existing refusal of values no dotenv style can represent stands as it was. + +## Alternatives considered + +**A `.pattern()` on the `apiKey` schema field.** Vendored schemastery supports it, and the pattern would serialize to the browser with the rest of the namespace schema — one rule, delivered rather than mirrored. It lost because a pattern cannot trim first: `cordis.yml` would then reject a padded key while `.env` tolerated one, and the resolver would disagree with the schema about the same string. Validating in `resolveAdapterOptions` keeps every surface trim-then-validate, and that function is already where this package re-judges bounds the schema cannot express. + +**A validation module shared by client and host.** Rejected by the source-plane layout: client packages reference only client packages plus `vendor/cordis` and `support/invariants`, and widening that to reach a host package would collide the two `Context` merges the split exists to keep apart. Mirroring a one-line predicate with a test on each side is the established shape here. + +**A per-adapter thrower in each of `llm-deepseek` and `llm-pi-ai`.** The first plan gave each adapter its own, differing only by the package prefix in the message, with a duplication-gate exemption to excuse the pair. Rejected before implementation: `LlmError` is declared in the seam, so the seam can own the diagnosis outright, and an exemption there would have hidden exactly the duplication it was covering for. + +**Sniffing the `TypeError` in the adapter's `catch`.** This would classify the ByteString failure after the fact, leaving the header construction itself unguarded. It depends on the wording of a Node error message, so it degrades silently across runtime versions, and it cannot help `llm-pi-ai`, whose request header is built inside the pi-ai SDK. Refusing the key before handing it over works for both adapters and for the discovery probe. + +**Enforcing in `credentials-local.set`.** It would catch every writer at once, including a hand-edited file. It lost because that provider stores credentials of every kind, and a rule derived from HTTP header encoding does not belong to it. + +**Running the shape heuristic in the resolvers too.** Symmetric, and it would stop a pasted environment line written directly into `.env`. Rejected for the lockout described above: a false positive in a resolver leaves the user no working path, while a false positive in the browser leaves the environment open. + +**Probing the provider at save time to prove the key works.** It would close the complaint the sources actually open with — a save that reports success and fails at the first turn. Rejected as out of scope and, on the code as it stood, unbuildable: `discoverModels` short-circuits to the installed catalog before any network call for exactly the providers pi-ai ships catalogs for, so it verified nothing about the key, and the DeepSeek card has no probe at all. A verifier's value is distinguishing "key rejected" from "cannot reach", which is the distinction this change makes reliable; building it first would have produced a verifier unable to tell its own outcomes apart. Comparable products also do not verify on save, so a blocking network call there would be an unexpected behavior rather than a missing one. + +## Consequences + +A malformed key is refused at the field that holds it, and a malformed stored key fails as `INVALID_CREDENTIAL` with a message naming where to fix it and no fragment of the key. Because that code sits outside `DEFAULT_RETRYABLE_CODES`, a deterministic credential fault is no longer retried three times as a transport blip. `llm-pi-ai` discovery reports an illegal probe key as a credential fault instead of an unreachable endpoint. + +The shape heuristic can refuse a real key. The first draft matched any upper-case identifier followed by `=`, which review showed was broader than intended: an all-upper-case base64 key ending in padding (`ABCD==`) matched an assignment it does not resemble. Requiring a non-`=` character after the separator excludes padding, since base64 only ever pads at the end. What remains — an upper-case name, one `=`, then a value — is a shape no known provider issues, and the rule runs only in the browser, so a user who still hits it can set the credential through the environment. The residual cost is a confusing refusal for a key nobody has yet reported. + +Restricting to printable ASCII is stricter than the transport requires: a header value may carry `\x80`–`\xFF`. Admitting latin-1 would let `é` through to return an opaque 401 instead of a local, explained refusal, so the stricter rule is deliberate. A provider that issues latin-1 keys would need this rule widened. + +The charset predicate exists twice, once per source plane. The layout forbids sharing it; each side carries its own test and names its twin. + +Keys already stored by an earlier build are read through `resolveApiKey`, so an illegal stored value fails at resolution rather than at request time. The diagnosis improves, but the failure moves earlier for anyone currently holding one. + +The costliest way to get this wrong would have been to treat absence as invalidity: a rule applied to `undefined` breaks every route authenticating through ambient discovery or OAuth, and a blank field that blocked submit makes editing any other setting demand re-entering the key. Both are pinned by tests rather than left to care. + +## Testing + +`packages/llm/llm/tests/api-key.spec.ts` drives `normalizeApiKey` and `assertUsableApiKey` over the whole input table — empty, whitespace-only, padded, interior-space, C0 control, emoji, CJK, full-width, latin-1, and the printable-ASCII boundary — and pins that a refusal carries `INVALID_CREDENTIAL` and no part of the key. + +`packages/llm/llm-deepseek/tests/` covers the literal-config path in `adapter.spec.ts` and the stored-credential path end to end in `dynamic-config.spec.ts`, through the real credentials seam rather than a stub. `packages/llm/llm-pi-ai/tests/` covers `resolveProfiles` — including that the trimmed value reaches the resolved profile, which the `...rest` spread would otherwise discard — and the discovery probe, including that a probe with no key sends no `authorization` header. + +`packages/client/ui-models/tests/` pins `apiKeyFailure` over the same table plus the paste-shape cases, and drives both cards: a blank field submits without writing a credential, a whitespace-only field fails on the field, an illegal or wrapped key blocks submit and the interrogation alike, a padded key is trimmed before `credentials.set` and before an interrogation, and a hand-declared route can be created with no key at all. + +The user-visible terminal state is pinned where it is actually assembled: `examples/headless-agent/tests/headless.snapshot.ts` runs the one-shot app against a stored key no header can carry, over the same keyless composition its missing-credential sibling uses, and records that the turn ends on `INVALID_CREDENTIAL` with an actionable message carrying neither the key nor the word `ByteString`. A package test could not have shown that, and the web e2e covers only the browser half. diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md new file mode 100644 index 0000000000..75b3fa247b --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md @@ -0,0 +1,107 @@ +# Agent Note: 在 API Key 进入 HTTP header 之前校验其格式 + +Status: implemented + +[English](2026-08-06-api-key-format-validation.md) | 中文 + +## Problem + +一个含有 HTTP header value 无法承载的字符的 API Key，曾被每一层配置界面接受，直到构造请求时才失败——离引发它的那个字段已经很远。 + +把含 emoji、中文或全角标点的 Key 粘进 Web 模型设置页，保存会报成功。第一轮对话随即失败于 `Cannot convert argument to a ByteString because the character at index 7 has a value of 55357 which is greater than 255`——其中的下标与码点是 UTF-16 内部细节，不附带任何可执行动作，却泄露了 Key 中某一个字符的码点。`llm-deepseek` 之所以产出这句，是因为 `fetch` 在 [adapter.ts](../../../../packages/llm/llm-deepseek/src/adapter.ts) 的 `try` 内部构造 `Bearer` header，而那个 `catch` 把一切失败都标为 `TRANSPORT`；该标签又在 `DEFAULT_RETRYABLE_CODES` 之中，于是一个永久且确定的故障还会被重试三次。 + +同样的输入在 `llm-pi-ai` 上更糟。它的探测路径在 [discovery.ts](../../../../packages/llm/llm-pi-ai/src/discovery.ts) 里用裸 `fetch` 构造同一个 header，并把一切失败包装成 `could not reach `，于是一个本地的 Key 故障被报成网络不可达。这条探测在保存之前就够得着：`ProviderEditor` 把用户输入的 `keyDraft` 直接放进探测请求，所以「获取模型列表」按钮会在任何东西落盘之前就把非法 Key 发出去。 + +空白字符能通过每一道检查。`ProviderEditor` 判的是 `keyDraft.length`，`resolveAdapterOptions` 判的是 `config.apiKey.length`，于是三个空格构成的 Key 会被存下，随后以 `Bearer` 加若干空格去认证。`llm-pi-ai` 在 `resolveProfiles` 中拒绝空的字面量 `apiKey`，却对来自凭据或环境的 Key 完全不做检查——而那正是模型设置页写入的路径，也就是用户真正走的路径。 + +来源：deepseek-harness#1594 与 #1595；dsh-external#247、#249、#266、#210。 + +## Decision + +一条规则定义什么是合法 Key：**trim 之后非空，且每个字符都落在 `[\x21-\x7E]`**——可打印 ASCII，不含空格。 + +这一个断言覆盖了来源列出的全部输入：空值、首尾空白、中间空白、C0 控制字符、emoji、中文、全角标点。它同时正是造成 ByteString 失败的那条约束，所以两个 issue 收敛于同一个定义，而不是两个恰好相关的修复。 + +第二条更窄的规则用于识别整行粘贴的环境变量：匹配 `^[A-Z][A-Z0-9_]*=[^=]` 或首尾成对引号的输入会被拒绝。把前缀限定为全大写可以让真实 Key 与之绝缘——`sk-` 这类形态会在连字符处中断标识符匹配——而要求分隔符之后必须是非 `=` 字符，则让 base64 的 padding 也与之绝缘。它报出的是与非法字符相同的那条格式失败，而不是自己的一句：读到它的人下一步动作完全一样，因此单列一句只会点出一个原因，却不改变该怎么做。 + +### 不变量属于每一层，启发式属于人所在的那一层 + +字符集规则是不变量。非 ASCII 字符对任何 provider 都**不可能**在 header value 中传输，因此在浏览器、在各个 resolver、在每一次凭据读取上执行它，是结构上的一致而非约定上的一致。 + +形状规则是对人如何粘贴的猜测，因此**只在浏览器中运行**。`llm-pi-ai` 前面挂着 OpenAI、Anthropic 以及任意手工声明的网关，本仓库并不掌握它们的 Key 格式；若这条规则运行在 resolver 中，一个签发形如 `TENANT1=abc` 的网关会让用户被彻底锁死、无路可走——设置页拒绝它，手写的 `.env` 在读取时同样被拒。把启发式限制在粘贴动作发生的那一层，环境变量便始终是那条出路。 + +### 「没有 Key」是一种配置状态，不是缺失 + +在这里，「没有 API Key」意味着三件完全不同的事，其中只有一件是错误。规则作用于**已提供**的值；至于究竟有没有提供，由各个调用方自行判断。 + +**未指定。** 既不写 `apiKey` 也不写 `apiKeyEnv` 的 profile，是由 harness 所持有的 Key 之外的东西来鉴权的。[provider.ts](../../../../packages/llm/llm-pi-ai/src/provider.ts) 中的 `routeAuth` 保留内置 catalog provider 自身的鉴权，正是为了让 provider 原生的 ambient 发现得以存活；而该 catalog 附带的 `openai-codex` 通过 OAuth 鉴权，并会直接拒绝一个显式的 Key。`namesCredential` 承载着这一区分。在 `llm-deepseek` 中，缺省的 `apiKey` 同样会回落到 `apiKeyEnv`。未指定的情形永不参与校验。 + +**Web UI 中留空的输入框。** 即便某个 provider 的 Key 已经存好，该输入框也是空着打开的——`keyStored` 的文案写的是「已配置——输入新值以替换」——所以留空意味着*保持已存储的值*。`ProviderEditor` 在草稿为空时完全跳过 `credentials.set`，这一点保持不变：留空绝不拦截提交，否则改一个 base URL 都得重新输一遍 Key。 + +**已提供，但为空或纯空白。** 它意味着什么，取决于「缺失」在该界面上选中了什么，而两个适配器的差异是有依据的。在 `llm-pi-ai` 中它是错误，因为那里的缺失切换的是**鉴权方式**——转向内置 provider 的 ambient 发现或 OAuth——因此一个空 Key 究竟想选哪一种是真有歧义；它的措辞指明了合法替代路径而非单纯拒绝（*has an empty apiKey; omit it to use ambient authentication*）。在 `llm-deepseek` 中，缺失只是为同一把 Key 选择了另一个**来源** `apiKeyEnv`，因此空白字面量会像缺省一样经该回落解析。在浏览器中它始终是失败，两张卡片皆然：字段是人刚刚敲过字的地方，静默丢弃他敲进去的内容永远不是正确答案。 + +因此 `normalizeApiKey` 接受 `string`，而绝非 `string | undefined`。 + +### 规则住在哪里 + +`normalizeApiKey` 是 `dsh-llm` seam 的一个模块，与已经承担共享 header 事务的 [attribution.ts](../../../../packages/llm/llm/src/attribution.ts) 并列。两个适配器都依赖该 seam 且都需要这条规则，因此它拥有两个当前消费者而非一个预设消费者。它返回 trim 后的值，或一个原因（`empty`、`illegalCharacters`）。 + +两个适配器同样都需要那句完全相同的「拒绝一个已存储凭据」的诊断，差别仅在包名前缀。`LlmError` 声明在 seam 的 `index.ts` 中，因此 `assertUsableApiKey(raw, pkg, ref)` 就住在它旁边，两个适配器都不再各留一份。断言模块本身保持零依赖：把 `LlmError` 引入 `api-key.ts` 会与 `index.ts` 对它的再导出成环。 + +客户端无法引入其中任何一个：client 包只 reference client 包，因此 `packages/client/ui-models` 在自己的 `apiKey.ts` 中镜像这个断言并持有本地化文案，正如 `validateDeepSeekModels` 镜像 host 侧的 `catalogModel` schema。两侧在注释中互相指名。 + +### 各个界面各做什么 + +| 界面 | 行为 | +|---|---| +| `dsh-llm` | 拥有 `normalizeApiKey`、`assertUsableApiKey` 与 `INVALID_CREDENTIAL_CODE`，后者刻意不进 `DEFAULT_RETRYABLE_CODES`。 | +| `llm-deepseek` `resolveAdapterOptions` | 拒绝标头无法承载的字面量 `apiKey`，与其他超出 schema 的边界检查并排；使用 trim 后的值。缺省或空白的 `apiKey` 回落到 `apiKeyEnv`。 | +| `llm-deepseek` `resolveApiKey` | 归一化凭据 seam 或环境返回的值，以 `INVALID_CREDENTIAL` 拒绝，消息指明模型设置页，绝不回显 Key。 | +| `llm-pi-ai` `resolveProfiles` | 施加这条共享规则，保留其「omit it to use ambient authentication」的措辞，并把 trim 后的值写进解析后的 profile。 | +| `llm-pi-ai` `resolveApiKey` | 归一化凭据与环境路径。不指定任何凭据的 profile 仍返回 `undefined`，ambient 与 OAuth 路由不受影响。 | +| `llm-pi-ai` `discoverModels` | 在构造 header 之前归一化，使非法 Key 成为凭据故障而非端点不可达。不带 Key 的探测保持未鉴权。 | +| `ui-models` | 镜像字符集规则，加入形状启发式，在探测与 `credentials.set` 之前 trim `keyDraft`，并修正 `stringAt` 的空值判断。留空的输入框仍是可以提交的空操作；只含空白的输入框则是字段级失败。提交**与端点探测**同时受拦截，因此被拒绝的密钥不会白花一次往返去换取字段上已经写明的答案；失败呈现在字段上，与既有的 `modelFailure` 模式一致。 | + +`ProviderEditor` 同时服务 DeepSeek 与 pi-ai 两种布局，因此一处客户端改动覆盖两个 provider。`CustomProviderCard` 为手工声明的路由承载同一套判定。 + +`credentials-local` 刻意不动。它存储各类凭据，而可打印 ASCII 是 HTTP header 的约束而非凭据存储的约束；它既有的、拒绝任何 dotenv 样式都无法表示的值的行为保持原样。 + +## Alternatives considered + +**在 `apiKey` schema 字段上加 `.pattern()`。** vendor 中的 schemastery 支持它，且该 pattern 会随命名空间 schema 一同序列化到浏览器——一条规则，投递而非镜像。它落败于 pattern 无法先行 trim：那样 `cordis.yml` 会拒绝带首尾空白的 Key 而 `.env` 却容忍，resolver 与 schema 会对同一个字符串给出分歧。在 `resolveAdapterOptions` 中校验可以让每一层都是 trim-then-validate，而该函数本就是本包重新裁定 schema 无法表达的边界之处。 + +**由 client 与 host 共享一个校验模块。** 被 source plane 布局否决：client 包只 reference client 包外加 `vendor/cordis` 与 `support/invariants`，把它放宽到够得着 host 包会撞上这一分割本就要隔开的两份 `Context` 合并。在两侧各镜像一行断言并各配一份测试，是此处的既定形态。 + +**在 `llm-deepseek` 与 `llm-pi-ai` 中各留一个抛错 helper。** 最初的计划正是各留一份，差别仅在消息中的包名前缀，并配一个重复检测豁免来放行这一对。在实现之前即被否决：`LlmError` 声明在 seam 中，因此 seam 完全可以自己拥有这句诊断，而那里的一个豁免恰恰会掩盖它本要遮掩的重复。 + +**在适配器的 `catch` 中嗅探 `TypeError`。** 这只是事后归类 ByteString 失败，header 构造本身仍无防护。它依赖 Node 错误消息的措辞，因而会随运行时版本静默失效；它也帮不到 `llm-pi-ai`——后者的请求 header 构造在 pi-ai SDK 内部。在交出 Key 之前就拒绝，则对两个适配器与探测路径同时有效。 + +**在 `credentials-local.set` 中执行。** 它能一次性拦住所有写入方，包括手工编辑的文件。它落败于该 provider 存储各种类型的凭据，而一条源自 HTTP header 编码的规则并不属于它。 + +**让形状启发式也在 resolver 中运行。** 更对称，且能拦住直接写进 `.env` 的整行环境变量。因上文所述的锁死风险而否决：resolver 中的一次误判会让用户无路可走，浏览器中的一次误判则仍留有环境变量这条路。 + +**在保存时探测 provider 以证明 Key 可用。** 它能关掉来源真正开篇抱怨的那件事——保存报成功、第一轮才失败。因超出范围而否决，且在当时的代码上无法建成：对 pi-ai 恰好自带 catalog 的那些 provider，`discoverModels` 会在任何网络调用之前短路到内置 catalog，因而对 Key 什么都验证不了；而 DeepSeek 卡片根本没有探测。验证器的价值在于分清「Key 被拒」与「无法连通」，而这正是本次改动让其变得可靠的区分；先建验证器只会得到一个分不清自身结果的验证器。同类产品也不在保存时验证，因此保存时的阻断式网络调用会是一个意外行为，而非一处缺失。 + +## Consequences + +格式错误的 Key 在持有它的那个字段上就被拒绝；格式错误的已存储 Key 以 `INVALID_CREDENTIAL` 失败，消息指明修复位置且不含 Key 的任何片段。由于该 code 位于 `DEFAULT_RETRYABLE_CODES` 之外，一个确定性的凭据故障不再被当作瞬时传输抖动重试三次。`llm-pi-ai` 的探测把非法 Key 报为凭据故障，而非端点不可达。 + +形状启发式可能拒绝一个真实的 Key。最初的写法匹配任意「全大写标识符接 `=`」，评审指出其覆盖面比预期更宽：一个以 padding 结尾的全大写 base64 Key（`ABCD==`）会命中它并不像的赋值形态。要求分隔符之后必须是非 `=` 字符即可排除 padding——base64 的 padding 只出现在末尾。剩下的形态（大写名称、一个 `=`、然后是值）是已知 provider 不会签发的，且该规则只在浏览器中运行，因此仍撞上它的用户可通过环境变量设置该凭据。残留代价是对一个尚无人报告过的 Key 给出一次令人困惑的拒绝。 + +限定为可打印 ASCII 比传输本身的要求更严：header value 是可以承载 `\x80`–`\xFF` 的。放行 latin-1 会让 `é` 通过并换回一个语焉不详的 401，而不是一次本地的、有解释的拒绝，因此从严是刻意的。若某个 provider 签发 latin-1 的 Key，这条规则需要放宽。 + +字符集断言存在两份，每个 source plane 一份。布局禁止共享它；两侧各自带测试并在注释中指名其孪生体。 + +早先版本已存下的 Key 会经 `resolveApiKey` 读取，因此一个非法的既存值将从解析时开始失败，而非到请求时才失败。诊断变好了，但对当前正持有这类值的人而言，失败点提前了。 + +把这件事做错的最大代价，会是把「未指定」当成「非法」：一条施加到 `undefined` 上的规则会打断每一条依赖 ambient 发现或 OAuth 鉴权的路由，而一个会拦截提交的空输入框，则会让改动任何其他设置都必须重新输入 Key。这两点都由测试钉住，而不是仅仰赖谨慎。 + +## Testing + +`packages/llm/llm/tests/api-key.spec.ts` 以整张输入表驱动 `normalizeApiKey` 与 `assertUsableApiKey`——空值、纯空白、带首尾空白、含中间空格、C0 控制字符、emoji、中文、全角、latin-1，以及可打印 ASCII 的边界字符——并钉住一次拒绝携带 `INVALID_CREDENTIAL` 且不含 Key 的任何部分。 + +`packages/llm/llm-deepseek/tests/` 在 `adapter.spec.ts` 中覆盖字面量配置路径，在 `dynamic-config.spec.ts` 中经真实凭据 seam（而非 stub）端到端覆盖已存储凭据路径。`packages/llm/llm-pi-ai/tests/` 覆盖 `resolveProfiles`——包括 trim 后的值确实到达解析后的 profile，否则会被 `...rest` 展开丢弃——以及探测路径，包括不带 Key 的探测不会发出 `authorization` 标头。 + +`packages/client/ui-models/tests/` 以同一张表加上形状用例钉住 `apiKeyFailure`，并驱动两张卡片：留空的输入框可提交且不写入凭据、只含空白的输入框在字段上失败、非法或被包裹的 Key 同时拦截提交与探测、带首尾空白的 Key 在 `credentials.set` 与探测之前被 trim，以及手工声明的路由可以完全不带 Key 创建。 + +用户可见的终态则钉在它真正被组装的位置：`examples/headless-agent/tests/headless.snapshot.ts` 让 one-shot 应用在一个 HTTP 标头无法承载的已存密钥下运行，复用其 missing-credential 兄弟场景的同一套无密钥 composition，并记录该轮以 `INVALID_CREDENTIAL` 结束、消息可操作且既不含密钥也不含 `ByteString` 字样。包级测试无法证明这一点，而 web e2e 只覆盖了浏览器那一半。 diff --git a/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.i18n.yaml b/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.i18n.yaml index 8754e37cc7..27786e85a2 100644 --- a/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.md -2026-07-23-session-telemetry-otel-revival.md: a58598d8a956d47cb0cf6aa3e659f38314bc4b17 -2026-07-23-session-telemetry-otel-revival.zh.md: 0deb0b81ce262db4fa4ae26076cd59756d628d20 +2026-07-23-session-telemetry-otel-revival.md: f83128e8bf62e0718e59912c16c4e449855aaa1a +2026-07-23-session-telemetry-otel-revival.zh.md: bef3bd9cbc22f363b92880864651dcaa782fc1de diff --git a/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.md b/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.md index a58598d8a9..f83128e8bf 100644 --- a/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.md +++ b/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.md @@ -12,9 +12,9 @@ Every deployment that wants harness sessions in an observability stack must hand `packages/telemetry/` revives the two reviewed packages under the SDK stance — the harness provides the capability, the deployment configures where records go and owns what leaves in them: -- **`@deepseek-ai/dsh-session-telemetry`** — the seam. `TelemetryBackend` (`emit`/`flush?`/`shutdown`), the service-registered `Telemetry` form, and `TelemetryCoordinator` owning capture: adoption with cursor read-back, the per-append firehose (project → `structuredClone` → redact → `emit`, zero I/O), the fixed first-chunk-per-(turn, step) projection, the `agent/error` relay, and dispose-time `shutdown` records. +- **`@deepseek-ai/dsh-session-telemetry`** — the seam. `TelemetryBackend` (`emit`/`flush?`/`shutdown`), the service-registered `Telemetry` form, and `TelemetryCoordinator` owning capture: live adoption with cursor read-back and the per-append firehose (project → `structuredClone` → redact → `emit`, zero I/O), buffer-free on-demand replay from the canonical log, the fixed first-chunk-per-(turn, step) projection, the live `agent/error` relay, and live dispose-time `shutdown` records. - **The `telemetry/record` waterfall** — the delta over the branch version and the seam's redaction extension point. Every record passes it before reaching any backend; the seam ships NO rules of its own — the innermost `next()` is a pass-through, deployments mount their rules as listeners (stacking by transforming `next()`'s return value), and a throwing rule withholds the record fail-closed. Redaction applies to the exported copy only; the canonical log is never rewritten. -- **`@deepseek-ai/dsh-session-telemetry-otel`** — the reference backend: OTel JS SDK log pipeline (`LoggerProvider` → `BatchLogRecordProcessor` → OTLP/HTTP exporter), configured verbatim through `exporter`/`processor` passthroughs. `exporter.url` is required and validated at load; unmounted or unconfigured, nothing leaves the process. +- **`@deepseek-ai/dsh-session-telemetry-otel`** — the reference backend: OTel JS SDK log pipeline (`LoggerProvider` → `BatchLogRecordProcessor` → OTLP/HTTP exporter), configured verbatim through `exporter`/`processor` passthroughs. Its default `FULL` mode requires `exporter.url`; the later [feedback-gated telemetry decision](2026-08-05-feedback-gated-session-telemetry.md) adds `FEEDBACK_ONLY` and `DISABLED` delivery modes without moving the redaction or backend boundary, while [buffer-free feedback replay](../simplification/2026-08-06-buffer-free-feedback-telemetry.md) avoids a second in-memory copy of the session prefix. The boundary axiom holds: the harness's aspect ends at `emit()`. Batching, retry, queueing, and loss policy are the reporting SDK's, configured through passthroughs — delivery is best-effort (at-most-once across a crash), which the READMEs state plainly. @@ -34,4 +34,4 @@ The boundary axiom holds: the harness's aspect ends at `emit()`. Batching, retry ## Consequences -A deployment adds one `cordis.yml` entry with an OTLP endpoint and gets its session stream in any OTel-compatible stack; removing the entry is the opt-out, with no residual state. A rule-free deployment exports records exactly as captured — including any credentials embedded in file contents or command output — so a deployment crossing a trust boundary must mount `telemetry/record` listeners, and both READMEs state this plainly. Where rules are mounted, exported bodies can differ from canonical log bytes, so receivers must not treat telemetry as a byte-exact replica; the log remains the source of truth. Crash durability is explicitly out of scope until the outbox decision above is revisited. +A deployment adds one `cordis.yml` entry with an OTLP endpoint and gets its session stream in any OTel-compatible stack. `FULL` preserves that behavior by default, `FEEDBACK_ONLY` replays a canonical-log prefix when feedback is recorded, and `DISABLED` constructs no reporting pipeline; removing the entry remains a silent opt-out, while the disabled mode keeps the local feedback warning. A rule-free deployment exports records exactly as captured — including any credentials embedded in file contents or command output — so a deployment crossing a trust boundary must mount `telemetry/record` listeners, and both READMEs state this plainly. Where rules are mounted, exported bodies can differ from canonical log bytes, so receivers must not treat telemetry as a byte-exact replica; the log remains the source of truth. Crash durability is explicitly out of scope until the outbox decision above is revisited. diff --git a/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.zh.md b/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.zh.md index 0deb0b81ce..bef3bd9cbc 100644 --- a/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.zh.md +++ b/.agents/notes/implemented/feature/2026-07-23-session-telemetry-otel-revival.zh.md @@ -12,9 +12,9 @@ Status: implemented `packages/telemetry/` 以 SDK 立场复活这两个经过评审的包——harness 提供能力，部署方配置上报去向并对导出内容负责： -- **`@deepseek-ai/dsh-session-telemetry`** —— seam 本体。`TelemetryBackend`（`emit`/`flush?`/`shutdown`）、服务注册形态的 `Telemetry`、以及拥有捕获侧的 `TelemetryCoordinator`：带游标回读的接管、逐 append 的 firehose（投影 → `structuredClone` → 脱敏 → `emit`，零 I/O）、固定的每个（轮次、步骤）组合的首分片投影、`agent/error` 转发、以及 dispose（资源释放）时的 `shutdown` 记录。 +- **`@deepseek-ai/dsh-session-telemetry`** —— seam 本体。`TelemetryBackend`（`emit`/`flush?`/`shutdown`）、服务注册形态的 `Telemetry`、以及拥有捕获侧的 `TelemetryCoordinator`：带游标回读的实时收养与逐 append 的 firehose（投影 → `structuredClone` → 脱敏 → `emit`，零 I/O）、从权威日志进行的无缓冲按需回放、固定的每个（轮次、步骤）组合首分片投影、实时 `agent/error` 转发，以及实时 dispose（资源释放）时的 `shutdown` 记录。 - **`telemetry/record` waterfall（瀑布式事件）** —— 相对分支版本的增量，也是该 seam 的脱敏扩展点。每条记录抵达任何后端前必经此处；seam 自身不带任何规则——最内层 `next()` 原样透传，部署方以监听器挂载自己的规则（通过变换 `next()` 的返回值堆叠），抛异常的规则将该记录 fail-closed 扣下。脱敏只作用于导出副本；canonical log 永不改写。 -- **`@deepseek-ai/dsh-session-telemetry-otel`** —— 参考后端：OTel JS SDK 日志流水线（`LoggerProvider` → `BatchLogRecordProcessor` → OTLP/HTTP exporter），经 `exporter`/`processor` passthrough 原样配置。`exporter.url` 必填且加载时校验；未挂载或未配置时，任何数据都不会离开进程。 +- **`@deepseek-ai/dsh-session-telemetry-otel`** —— 参考后端：OTel JS SDK 日志流水线（`LoggerProvider` → `BatchLogRecordProcessor` → OTLP/HTTP exporter），经 `exporter`/`processor` passthrough 原样配置。其默认 `FULL` 模式要求 `exporter.url`；后续的[反馈门控遥测决策](2026-08-05-feedback-gated-session-telemetry.md)增加了 `FEEDBACK_ONLY` 与 `DISABLED` 投递模式，但未移动脱敏或后端边界，而[无缓冲反馈回放](../simplification/2026-08-06-buffer-free-feedback-telemetry.md)避免在内存中创建会话前缀的第二份副本。 边界公理保持不变：harness 的职责止于 `emit()`。批处理、重试、排队与丢失策略属于 reporting SDK，经 passthrough 配置——投递是尽力而为（崩溃时至多一次），两份 README 对此如实陈述。 @@ -34,4 +34,4 @@ Status: implemented ## 后果 -部署方在 `cordis.yml` 加一个带 OTLP endpoint 的 Cordis 配置项即可把会话流接入任何 OTel 兼容体系；删除该配置项即退出，无残留状态。未挂载规则的部署导出的记录与捕获时完全一致——包括文件内容与命令输出中内嵌的任何凭据——因此跨信任边界的部署必须挂载 `telemetry/record` 监听器，两个 README 对此如实陈述。挂载规则后，导出的 body 可能与 canonical log 字节不同，接收端不得把遥测当作字节精确副本；日志仍是真源。崩溃持久性在上述 outbox 决定重新审议前明确不在范围内。 +部署方在 `cordis.yml` 加一个带 OTLP endpoint 的 Cordis 配置项即可把会话流接入任何 OTel 兼容体系。`FULL` 默认保留该行为，`FEEDBACK_ONLY` 在记录反馈时回放权威日志前缀，`DISABLED` 则不构造上报流水线；删除该配置项仍是静默退出方式，而禁用模式会保留本地反馈警告。未挂载规则的部署导出的记录与捕获时完全一致，包括文件内容与命令输出中内嵌的任何凭据。因此，跨信任边界的部署必须挂载 `telemetry/record` 监听器，两个 README 对此如实陈述。挂载规则后，导出的 body 可能与 canonical log 字节不同，接收端不得把遥测当作字节精确副本；日志仍是真源。崩溃持久性在上述 outbox 决定重新审议前明确不在范围内。 diff --git a/.agents/notes/implemented/feature/2026-07-28-feedback-command.i18n.yaml b/.agents/notes/implemented/feature/2026-07-28-feedback-command.i18n.yaml new file mode 100644 index 0000000000..b1dceca3b2 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-28-feedback-command.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-28-feedback-command.md +2026-07-28-feedback-command.md: 770c72954d66c9e26e03c47f99c261626e6ed09b +2026-07-28-feedback-command.zh.md: c8bd57ddb5e94bfb826ec40b771547ca9625c8e3 diff --git a/.agents/notes/implemented/feature/2026-07-28-feedback-command.md b/.agents/notes/implemented/feature/2026-07-28-feedback-command.md new file mode 100644 index 0000000000..770c72954d --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-28-feedback-command.md @@ -0,0 +1,63 @@ +# Agent Note: `/feedback` command + +Status: implemented + +English | [中文](2026-07-28-feedback-command.zh.md) + +## Problem + +A user who notices something wrong mid-session has nowhere to put that observation. Telling the model wastes a turn, changes the conversation the user was having, and buries the remark in derived history where no later reader can find it. Writing it outside the session loses the context that makes it meaningful — which session, at which point, against which work. + +The capture surface has to be usable at the moment of annoyance, which rules out anything requiring the user to leave the interactive client, and it must not perturb the run in progress: no model tokens, no turn of work, no change to the request the user is waiting on. + +## Decision + +`@deepseek-ai/dsh-command-feedback` in `packages/feedback/command-feedback/` registers one global `feedback` command over `ctx.commands`. `/feedback ` acknowledges; bare or whitespace-only input returns a direct usage error. The handler is synchronous, injects only `commands`, and has no configuration. + +The package declares the log-only `feedback/record { text }` session event and exports `recordFeedback(session, text)` as its command-independent producer. The producer discards surrounding whitespace, rejects an empty result, and appends exactly one event. `/feedback` delegates to it, so another UI, hook, or host integration can record the same domain fact without constructing a slash command. + +`dsh-commands` still writes its `command/run` / `command/done` lifecycle pair around `/feedback`, but this command sets `recordInput: false`. Its `command/run` therefore carries the command identity and source without `args`; the feedback text exists only in `feedback/record`, while `command/done` carries the acknowledgement outcome. All three records are log-only and non-surface. Their appends start persistence's ordinary eager drain; nothing forces a flush, so acknowledgement reports that the feedback is in the log rather than already on disk. + +Capture remains inert for the running agent and model. The optional OTel telemetry package later adds one infrastructure consumer: it observes `feedback/record` as a release trigger in `FEEDBACK_ONLY` mode and as the local-only warning trigger in `DISABLED` mode, without changing the feedback event or command path. See [Feedback-gated session telemetry](2026-08-05-feedback-gated-session-telemetry.md). + +### Why feedback owns an event + +Feedback is a domain fact, while `/feedback` is one trigger. Keeping the only payload in `feedback/record` lets later triggers use the same event and lets consumers select feedback without depending on command names or parsing command lifecycle records. Omitting `command/run.args` for this definition avoids two authoritative-looking copies of one human remark. + +### Why the model never sees it + +Feedback is about the session, not input to it. Injecting it as a user message would change the next model request, contradicting the requirement that recording not perturb the run, and would make the remark part of the conversation it comments on. `command/run` and `command/done` are absent from `SurfaceEventType`, so they cannot acquire a `surfaceOp` or enter derived history even by mistake. + +### Verbatim text + +Surrounding whitespace is discarded, but nothing else is parsed. `/feedback /plan felt slow` records `/plan felt slow`; the leading `/plan` is content, not a nested command. Control-word grammar of the kind `/goal` uses would make the corresponding literal feedback impossible to express, which is the opposite of what a capture surface is for. + +### A new group + +`packages/feedback/` is a new group because no existing one owns this. `goal/` is objective state, `session-title/` is titles, `core/` is the product spine. The group holds one producer package; cross-cutting consumers stay in their owning groups rather than forcing this one to grow. + +## Alternatives considered + +**Use `command/run` as the feedback record.** Rejected because feedback would then be coupled to one trigger and consumers would have to identify a domain fact by command name. A non-command producer could not create the same record without pretending to execute a command. + +**Store the text in both `feedback/record` and `command/run.args`.** Rejected because one act would have two payload copies with no useful distinction. `recordInput: false` preserves the generic lifecycle while leaving the domain event authoritative. + +**Inject feedback as a user message via `agent.inject()`.** Needs no new event type and reuses the path `/goal` mutations take. Rejected: it makes the feedback model-visible, so it enters the next request, changes the run being commented on, and consumes tokens — contradicting all three parts of the no-perturbation requirement. + +**Make `/feedback` a true no-op that records nothing.** The most literal reading of "does not do anything". Rejected because it makes the command pointless: the stated requirement was that the remark reach the session log. + +**Register the command inside an existing package** such as `packages/ui/commands`. Avoids a new group and its README pair. Rejected: `ctx.commands` is the registry, not a home for arbitrary command implementations, and the requester asked for a standalone package. + +**Parse structure out of the text** (category prefixes, severity markers). Rejected as speculative: no consumer needs that structure, and any control-word grammar makes the corresponding literal feedback unrecordable. Verbatim text is the widest surface a future consumer can narrow; a parsed one cannot be widened after the fact. + +**Add a model-facing tool instead of a slash command.** Rejected: feedback is a direct human observation. Routing it through the model spends a turn, lets the model paraphrase the user's words, and makes the record contingent on the model choosing to call the tool. + +## Consequences + +The shipped `dsh` base mounts the command unconditionally — no configuration, no dependency on the goal stack. The Web client exposes it through its command adapter. Headless mode, ACP, and JSON-RPC do not provide a command adapter, so `/feedback` is unavailable there. + +The package owns one independent append-only event with no cross-event or mutable-data relation for an invariant companion to check. The event follows the session log's existing replay, fork, persistence, and crash-tail behavior. + +Deferred: no product or model consumer; no structured fields; no amend or withdraw, since the log is append-only and this package adds no tombstone; and no explicit durability barrier, so an entry recorded immediately before a crash can be lost with any other unflushed tail. The optional telemetry consumer treats the event only as an export-policy trigger. + +No keyless transcript snapshot accompanies this change, at the requester's explicit direction. Package tests, a real Loader composition test over a `cordis.yml`, and the shipped Web composition test cover registration, capture, model exclusion, and product assembly. diff --git a/.agents/notes/implemented/feature/2026-07-28-feedback-command.zh.md b/.agents/notes/implemented/feature/2026-07-28-feedback-command.zh.md new file mode 100644 index 0000000000..c8bd57ddb5 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-28-feedback-command.zh.md @@ -0,0 +1,63 @@ +# Agent Note: `/feedback` 命令 + +Status: implemented + +[English](2026-07-28-feedback-command.md) | 中文 + +## 问题 + +用户在会话中途发现问题时，没有地方记下这个观察。告诉模型会浪费一个轮次、改变用户原本进行的对话，并把这条评论埋进派生历史，使后续读者无法找到它。写到会话之外则会丢失让它有意义的上下文：属于哪个会话、处于哪个时点、针对哪项工作。 + +采集接口必须能在用户产生不满的那一刻使用，因此任何需要用户离开交互式客户端的方案都不可行；它还不能扰动正在进行的运行：不消耗模型 token、不产生工作轮次、不改变用户正在等待的请求。 + +## 决策 + +位于 `packages/feedback/command-feedback/` 的 `@deepseek-ai/dsh-command-feedback` 通过 `ctx.commands` 注册一个全局 `feedback` 命令。`/feedback ` 给出确认；空输入或仅含空白的输入返回直接用法错误。处理器是同步的，只注入 `commands`，且没有任何配置。 + +本包（package）声明仅写入日志的 `feedback/record { text }` 会话事件，并导出 `recordFeedback(session, text)`，作为不依赖命令的生产方。该生产方丢弃前后空白，拒绝空结果，并且恰好追加一个事件。`/feedback` 委托给它，因此其他 UI、钩子或 host 集成无需构造斜杠命令也能记录同一个领域事实。 + +`dsh-commands` 仍会围绕 `/feedback` 写入 `command/run` / `command/done` 生命周期配对，但该命令设置了 `recordInput: false`。因此，它的 `command/run` 携带命令标识与来源，但不携带 `args`；反馈文本只存在于 `feedback/record` 中，而 `command/done` 携带确认结果。三个记录都仅写入日志且非 surface。它们的追加会启动持久化的常规即时排空；没有任何环节强制 flush，因此确认文本报告的是反馈已进入日志，而非已经落盘。 + +采集对正在运行的 agent 与模型仍不产生后续动作。可选的 OTel 遥测包后续增加了一个基础设施消费方：它在 `FEEDBACK_ONLY` 模式下将 `feedback/record` 作为释放触发器，在 `DISABLED` 模式下将其作为本地警告触发器，且不改变反馈事件或命令路径。见[反馈门控的会话遥测](2026-08-05-feedback-gated-session-telemetry.md)。 + +### 为何反馈拥有自己的事件 + +反馈是领域事实，而 `/feedback` 是一种触发方式。只把载荷保存在 `feedback/record` 中，既让后续触发方式可以使用同一个事件，也让消费方无需依赖命令名或解析命令生命周期记录即可筛选反馈。在该定义中省略 `command/run.args`，可避免同一条人类评价出现两个看起来都具有权威性的副本。 + +### 为何模型永不看到它 + +反馈是关于会话的，而不是会话的输入。将其作为 user 消息注入会改变下一次模型请求，与「记录不得扰动运行」的要求相冲突，也会让该评论成为它所评论的那段对话的一部分。`command/run` 与 `command/done` 不属于 `SurfaceEventType`，因此即便出错也无法获得 `surfaceOp` 或进入派生历史。 + +### 原样文本 + +前后空白会被丢弃，但除此之外不做解析。`/feedback /plan felt slow` 记录 `/plan felt slow`；开头的 `/plan` 是内容，而非嵌套命令。若采用 `/goal` 那样的控制词语法，对应的字面反馈将无法表达，这与采集接口的目的正好相反。 + +### 一个新的分组 + +`packages/feedback/` 是新分组，因为现有分组都不拥有此职责：`goal/` 负责目标状态，`session-title/` 负责标题，`core/` 是产品主干。该分组只包含一个生产方包；跨领域的消费方留在各自所属的分组，而不是迫使这个包不断膨胀。 + +## 考虑过的替代方案 + +**使用 `command/run` 作为反馈记录。** 已否决，因为这会将反馈与一种触发方式耦合，消费方还必须通过命令名识别领域事实。非命令生产方若不伪装成执行命令，就无法创建相同记录。 + +**同时在 `feedback/record` 与 `command/run.args` 中存储文本。** 已否决，因为同一行为会产生两个没有实质区别的载荷副本。`recordInput: false` 保留通用生命周期，同时让领域事件保持权威性。 + +**通过 `agent.inject()` 将反馈作为 user 消息注入。** 无需新增事件类型，并复用 `/goal` 变更所走的路径。已否决：它会让反馈对模型可见，从而进入下一次请求、改变正被评论的那次运行并消耗 token——与「不得扰动」要求的三个方面全部冲突。 + +**让 `/feedback` 成为真正的空操作，什么都不记录。** 这是对「什么都不做」最字面的理解。已否决：这会使命令失去意义——明确的要求是让这条评论进入会话日志。 + +**在现有包中注册该命令**，例如 `packages/ui/commands`。可省去新分组及其双语 README。已否决：`ctx.commands` 是注册表，而不是任意命令实现的归属地；且请求者明确要求独立的包。 + +**从文本中解析结构**（类别前缀、严重程度标记）。已否决，属于投机设计：没有消费方需要该结构，而任何控制词语法都会让对应的字面反馈无法记录。原样文本是未来消费方可以收窄的最宽接口；而已被解析的接口无法事后放宽。 + +**改为提供面向模型的工具。** 已否决：反馈是人类的直接观察。经由模型会消耗一个轮次、让模型改写用户的原话，并使记录取决于模型是否选择调用该工具。 + +## 后果 + +随附的 `dsh` 基础组合无条件挂载该命令：没有配置，也不依赖 goal 栈。Web 客户端通过命令适配器暴露该命令。无头模式、ACP 和 JSON-RPC 不提供命令适配器，因此 `/feedback` 在那里不可用。 + +本包拥有一个独立的仅追加事件，不存在跨事件关系或可变数据关系可供不变式伴生插件检查。该事件遵循会话日志现有的回放、fork、持久化和崩溃尾部行为。 + +延期事项：没有产品或模型消费方；没有结构化字段；不支持修改或撤回，因为日志仅追加且本包不新增 tombstone；且没有显式持久化屏障，因此紧临崩溃前记录的条目可能与其他未 flush 的尾部一同丢失。可选的遥测消费方只将该事件作为导出策略触发器。 + +本次变更按请求者的明确指示不附带无密钥 transcript（文本记录）快照。包测试、基于真实 `cordis.yml` 的 Loader 组合测试，以及随附的 Web 组合测试覆盖注册、采集、模型排除和产品组装。 diff --git a/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.i18n.yaml b/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.i18n.yaml index 9b8d037c43..5f2f7bc9ae 100644 --- a/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.md -2026-07-28-tool-call-file-open-in-os.md: a2c9b52507d32c2d851f811f0ecdd878a60b1e1c -2026-07-28-tool-call-file-open-in-os.zh.md: 725db61869383711042d85cc1508b00eb1b196b6 +2026-07-28-tool-call-file-open-in-os.md: 499d3caa11e1f2c17d96e43b7846df72a31c0eb1 +2026-07-28-tool-call-file-open-in-os.zh.md: a658a7b55d2575ad1a5d8baccb80f0a8444d6db9 diff --git a/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.md b/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.md index a2c9b52507..499d3caa11 100644 --- a/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.md +++ b/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.md @@ -10,14 +10,15 @@ Chat tool rows treated the whole summary line as a click target that opened the ## Decision -File-tool path summaries (`read` / `write` / `edit` args carrying `path` or `file_path`) render as hover-underline links with a pointer cursor. Clicking the path calls `host.openPath` through `WorkspacesService.openPath`, resolving relative paths against the session cwd. File-link rows disable args expand (leading icon is inert); whole-row click, row hover fill, and the click-to-open-details gesture are removed from tool rows (including bash and todo registrations). The details panel and its inject surface remain for programmatic selection; rows no longer drive them. +File-tool path summaries (`read` / `write` / `edit` args carrying `path` or `file_path`) render as links underlined at rest with a pointer cursor. Clicking the path calls `host.openPath` through `WorkspacesService.openPath`, resolving relative paths against the session cwd. File-link rows disable args expand (leading icon is inert); whole-row click, row hover fill, and the click-to-open-details gesture are removed from tool rows (including bash and todo registrations). The details panel and its inject surface remain for programmatic selection; rows no longer drive them. -`host.openPath` is a privileged unary RPC accepted only from loopback, same-origin browser requests (same carrier guard as `host.pickDirectory`). Platform adapters open without a shell: `open` on macOS, PowerShell `Invoke-Item` on Windows, `xdg-open` on Linux. The opener is injectable for tests. URL-only read args (`web_fetch`) are not file links. +`host.openPath` is a privileged unary RPC accepted only from loopback, same-origin browser requests (same carrier guard as `host.pickDirectory`). Platform adapters open without a shell: `open` on macOS, PowerShell `Invoke-Item` on Windows, and `xdg-open` on desktop Linux; browser-renderable documents prefer the named default browser on macOS and desktop Linux. WSL is a separate host shape despite Node reporting `linux`: the adapter recognizes its environment or Microsoft kernel release, translates the Linux path with `wslpath -w`, and passes the resulting Windows/UNC path to the same PowerShell handoff. The opener's platform facts and command runner are injectable for tests. URL-only read args (`web_fetch`) are not file links. ## Alternatives considered - Keep row-click details and add a separate file affordance — rejected; the product ask replaces the row gesture with the file link. - Open files inside an in-app preview — rejected; the ask is the OS default application. +- Treat WSL as desktop Linux — rejected; a WSL process reports `linux`, but a Linux desktop association is optional while its ordinary operator desktop and browser live on Windows. - Reuse `host.pickDirectory`'s timeout exemption — unnecessary; path open hand-off completes quickly under the normal unary deadline. ## Consequences @@ -26,5 +27,5 @@ Clicking a file path in a tool row opens that path on the host. Non-file tool ro ## Risks -- Linux hosts without `xdg-open` fail the RPC; the chat row stays silent while the host returns an internal error. +- Desktop Linux hosts without `xdg-open`, and WSL hosts without working Windows interop (`wslpath` plus `powershell.exe`), fail the RPC; the chat row stays silent while the host returns an internal error. - Relative paths without a session cwd are forwarded verbatim and may fail on the host. diff --git a/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.zh.md b/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.zh.md index 725db61869..a658a7b55d 100644 --- a/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.zh.md +++ b/.agents/notes/implemented/feature/2026-07-28-tool-call-file-open-in-os.zh.md @@ -10,14 +10,15 @@ Status: implemented ## 决策 -文件工具的路径摘要（`read`／`write`／`edit` 参数中的 `path` 或 `file_path`）渲染为悬停下划线链接并使用 pointer 光标。点击路径会经 `WorkspacesService.openPath` 调用 `host.openPath`，相对路径以会话 cwd 为基准解析。带文件链接的行关闭参数展开（左侧图标不可点）；工具行（含 bash 与 todo 注册）去掉整行点击、整行悬停底色，以及点击打开 details 的手势。details 面板及其 inject 面仍保留供程序化选择；工具行不再驱动它们。 +文件工具的路径摘要（`read`／`write`／`edit` 参数中的 `path` 或 `file_path`）渲染为静止状态下即带下划线的链接，并使用 pointer 光标。点击路径会经 `WorkspacesService.openPath` 调用 `host.openPath`，相对路径以会话 cwd 为基准解析。带文件链接的行关闭参数展开（左侧图标不可点）；工具行（含 bash 与 todo 注册）去掉整行点击、整行悬停底色，以及点击打开 details 的手势。details 面板及其 inject 面仍保留供程序化选择；工具行不再驱动它们。 -`host.openPath` 是特权一元 RPC，仅接受来自回环地址且同源的浏览器请求（与 `host.pickDirectory` 相同的载体守卫）。平台适配器不经 shell 打开：macOS 为 `open`，Windows 为 PowerShell `Invoke-Item`，Linux 为 `xdg-open`。打开器可在测试中注入。仅含 URL 的 read 参数（`web_fetch`）不是文件链接。 +`host.openPath` 是特权一元 RPC，仅接受来自回环地址且同源的浏览器请求（与 `host.pickDirectory` 相同的载体守卫）。平台适配器不经 shell 打开：macOS 为 `open`，Windows 为 PowerShell `Invoke-Item`，桌面 Linux 为 `xdg-open`；浏览器可渲染的文档会在 macOS 与桌面 Linux 上优先使用能够确定的默认浏览器。尽管 Node 将 WSL 报告为 `linux`，WSL 仍是一种独立的宿主形态：适配器根据其环境或 Microsoft 内核 release 识别它，用 `wslpath -w` 转换 Linux 路径，并将所得 Windows/UNC 路径交给同一 PowerShell 交接。打开器的平台信息和命令运行器可在测试中注入。仅含 URL 的 read 参数（`web_fetch`）不是文件链接。 ## 考虑过的替代方案 - 保留整行点击打开 details，另加文件入口 — 否决；产品要求用文件链接替换整行手势。 - 在应用内预览文件 — 否决；要求是操作系统默认应用。 +- 将 WSL 当作桌面 Linux — 否决；WSL 进程报告 `linux`，但 Linux 桌面文件关联并非必有，而其常规用户桌面和浏览器位于 Windows 上。 - 复用 `host.pickDirectory` 的超时豁免 — 不必要；打开路径的交接在常规一元截止时间内即可完成。 ## 后果 @@ -26,5 +27,5 @@ Status: implemented ## 风险 -- 没有 `xdg-open` 的 Linux 宿主会使 RPC 失败；聊天行保持静默，宿主返回内部错误。 +- 没有 `xdg-open` 的桌面 Linux 宿主，以及 Windows 互操作（`wslpath` 加 `powershell.exe`）不可用的 WSL 宿主，会使 RPC 失败；聊天行保持静默，宿主返回内部错误。 - 没有会话 cwd 时相对路径会原样转发，可能在宿主侧失败。 diff --git a/.agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.i18n.yaml b/.agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.i18n.yaml new file mode 100644 index 0000000000..c1cc57b233 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.md +2026-07-31-web-workspace-file-links.md: fe71a2d8caa4dc9cc732a250d7e388d1a3b44182 +2026-07-31-web-workspace-file-links.zh.md: 0f8c87822087169aad9ac4283f706fd8a8748be9 diff --git a/.agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.md b/.agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.md new file mode 100644 index 0000000000..fe71a2d8ca --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-31-web-workspace-file-links.md @@ -0,0 +1,35 @@ +# Agent Note: opening a produced file from the web UI + +Status: implemented + +English | [中文](2026-07-31-web-workspace-file-links.zh.md) + +> Scope: the produced-files row a finished turn ends with, the file-path link that reads as one, and the Host opener preferring the default browser for documents a browser renders. Not in scope, by decision: serving workspace files over HTTP, and previews for a client that is not on the Host machine. + +## Problem + +A web session that produced a file had no way to look at it. The agent wrote `deepseek-homepage.html`, said so, and the user's only recourse was to copy an absolute path like `/private/tmp/dsh-client-hotplug.ygPvsm/workspaces/plugin-hotplug/deepseek-homepage.html` into a terminal. + +Two distinct defects sat behind that. The transcript never said what a turn had produced: `ToolCallView.locations` — the follow-along vocabulary the file tools already populate — had no consumer in the client, so a reader's only account of the output was whatever the closing message happened to spell. And the affordance that did exist was invisible: `ToolRow` already renders a mutation or read row's path as a real button wired to `host.openPath`, but styled exactly like the surrounding prose and underlined only on hover, so nobody found it. The reported "I can't open what it made" was a discoverability failure sitting on top of a working capability. + +## Decision + +**A finished turn ends with the files it produced.** The row is its own plugin, `@deepseek-ai/dsh-client-ui-deliverables`, registered into the `conversation.chat.turnTail` hole the chat view renders between a closing message's body and its IconActions — ui-conversation owns the hole and the owner currency (nodes, closing seq, `openFile`), the plugin owns every policy. `producedForClosing` reads the paths off the mutation tools' own follow-along `locations` — a diff card, or a generic card whose `kind` is `edit` (the shape `str_replace_editor`'s insert presents) — so a turn's output is listed whether or not the closing message named it, and a new mutation tool joins by declaring what it does rather than by being added to a list. Reads, deletes, and failed calls contribute nothing; a path appears once per turn in first-seen order; accumulation resets on the turn boundary, so a turn that mutates and then ends without content text cannot spill into the next turn's row. One cordis.yml line composes the surface in or out; the unregistered hole renders nothing. + +**The path link reads as a link.** Underlined at rest, not only on hover. This is the smaller half of the diff and the larger half of the fix. + +**Opening stays the Host's job, and prefers the default browser.** `host.openPath` hands the path to the operating system, which yields a `file://` document in a real browser: full page capabilities, and no reachability into `/api`, because a `file://` document is not same-origin with it. Measured on the reported artifact: `localStorage` works, the theme toggle flips, the tabs switch, and `fetch` to the API fails. For documents a browser renders — `.html`, `.htm`, `.xhtml`, `.svg` — the opener resolves the default *browser* rather than the type's default application when the platform can name one, because a developer who binds `.html` to an editor would otherwise click a produced page and get source code. macOS reads the LaunchServices `https` handler and desktop Linux reads `$BROWSER`; either falls back to the default application when no browser can be named. Windows uses its registered association, and WSL first translates the path before using that same Windows handoff. + +**Serving workspace files over HTTP is out of scope, and so are non-local clients.** An earlier revision served files from the harness itself — first same-origin with `/api`, then behind `CSP: sandbox`, then from a second listener whose own port gave served documents their own origin. Each step answered a real problem, and the whole line was retired once the product scope was settled: previews for a browser that is not on the Host machine are not supported. With that decided, the Host opener answers the supported case completely and the HTTP machinery answered only the unsupported one. + +## Alternatives considered + +- **Serving `/f//` from the harness** — built and working, including confinement by dual `realpath`, the browser-trust fence, streamed reads, and a separate listener whose port gave served documents their own origin. It is the only design that shows a preview to a client on another machine, which is exactly the case ruled out of scope. Retired for that reason, not because it failed; its cost was a second socket with its own lifecycle, a port published into the page, and a URL-shape contract shared across two packages. +- **Same-origin HTTP serving without isolation** — measurably unsafe, and recorded so nobody retries it: a document served beside `/api` drove `settings.describe` to a `200` with full data and `session.list` to 35 KB of every session's transcript, from a page that need not be agent-authored at all (a read row makes every file in a cloned repository openable). +- **`Content-Security-Policy: sandbox` over that same-origin serving** — closes the hole by taking the document's origin away, which measurably breaks the pages this feature exists to show: the reported artifact throws `SecurityError` on load, and because an uncaught exception aborts the rest of its `