From 9eaaeaeb9602b46dc4d00c542ed16704435cc005 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 13:46:13 +0800 Subject: [PATCH] fix(credentials,authorization,llm-pi-ai): harden the auth seams per review Review findings on #2509, all confirmed: - Every writer of .credentials.yaml now waits out the record-mutation lock (DOCUMENT_LOCK_WAIT_MS): refs and records share one file and one lock, so a reference write or record delete contending with an OAuth refresh must not fail at the 2s file-work default. - api-key records are admitted before they are rendered: an empty key, a non-POSIX env name, or an empty env value is refused at the write instead of persisting a document the next boot rejects wholesale. - llm-pi-ai no longer lets the credential-key grammar reject legal route ids: reads answer "nothing stored" via isCredentialKeySegment (new dsh-credentials export), deletes have nothing to remove, and only a write refuses, as LlmError UNSTORABLE_PROVIDER_ID; flow registration skips a future catalog id outside the grammar instead of failing the mount. - authorization/settled fans out with contained listener failures on the credentials seam's terms (INVARIANT still rethrows), so a broken watcher can never turn a finished attempt into a failure. - notify() is fire-and-forget at the seam: a surface that cannot render a notice loses the notice, never the attempt. - A declined prompt is an outcome: interactions reject with the new AuthorizationDeclinedError and the attempt settles cancelled instead of failed. - NOT_COMMITTED now confirms a commit observed during the attempt (credentials/record-updated for the flow's key), so a re-auth cannot pass a stale record off as fresh; a flow that deletes its record is refused on the same code. READMEs, the subsystem/event/config catalogs, and the Agent Note follow the shipped behavior; memory.ts carries the dedup TODO. --- ...-records-and-authorization-flows.i18n.yaml | 4 +- ...dential-records-and-authorization-flows.md | 6 +- ...tial-records-and-authorization-flows.zh.md | 6 +- docs/config-catalog.i18n.yaml | 4 +- docs/config-catalog.md | 2 +- docs/config-catalog.zh.md | 2 +- docs/event-producer-consumer.i18n.yaml | 4 +- docs/event-producer-consumer.md | 4 +- docs/event-producer-consumer.zh.md | 4 +- docs/subsystems/credentials.i18n.yaml | 4 +- docs/subsystems/credentials.md | 12 +- docs/subsystems/credentials.zh.md | 12 +- .../authorization/README.i18n.yaml | 4 +- packages/credentials/authorization/README.md | 10 +- .../credentials/authorization/README.zh.md | 10 +- .../credentials/authorization/src/index.ts | 148 +++++++++++++--- .../authorization/tests/authorization.spec.ts | 159 ++++++++++++++++++ .../credentials/authorization/tests/memory.ts | 3 + .../credentials-local/src/index.ts | 47 ++++-- .../credentials-local/tests/records.spec.ts | 18 ++ .../tests/review-fixes.spec.ts | 33 +++- .../credentials/credentials/README.i18n.yaml | 4 +- packages/credentials/credentials/README.md | 2 +- packages/credentials/credentials/README.zh.md | 2 +- packages/credentials/credentials/src/index.ts | 13 ++ .../credentials/tests/credentials.spec.ts | 15 +- .../extensions/tool-cordis/src/api-catalog.ts | 4 +- packages/llm/llm-pi-ai/README.i18n.yaml | 4 +- packages/llm/llm-pi-ai/README.md | 2 +- packages/llm/llm-pi-ai/README.zh.md | 2 +- packages/llm/llm-pi-ai/src/auth.ts | 19 ++- packages/llm/llm-pi-ai/src/login.ts | 11 ++ packages/llm/llm-pi-ai/tests/auth.spec.ts | 14 ++ 33 files changed, 500 insertions(+), 88 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.i18n.yaml index 269ef4ce46..999a35f13c 100644 --- a/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.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-08-13-credential-records-and-authorization-flows.md -2026-08-13-credential-records-and-authorization-flows.md: b962393f3efdf883b918ae47aa96d37846c12dd6 -2026-08-13-credential-records-and-authorization-flows.zh.md: e730b00c7be427e243abfab5206eac13079b11cc +2026-08-13-credential-records-and-authorization-flows.md: 19de5a5212e12a678a2162275f26130cf54f4f46 +2026-08-13-credential-records-and-authorization-flows.zh.md: ee79248a8c6ae1d652bb280bb9863c01655715c7 diff --git a/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.md b/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.md index b962393f3e..19de5a5212 100644 --- a/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.md +++ b/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.md @@ -26,7 +26,7 @@ Records do not layer. There is no environment an authorization grant could be re Two choices carry the weight: -- **The flow owns the write.** `run()` resolving means the record is already committed through `ctx.credentials`; the seam confirms it and refuses a flow that resolved without one. This is what lets `Models.login()` — which persists through the store adapter as part of logging in — stay the single writer, instead of the credential being copied back out and written a second time. +- **The flow owns the write.** `run()` resolving means the record is already committed through `ctx.credentials`; the seam confirms a commit it observed during the attempt — presence alone would let a re-authorization pass a stale record off as fresh — and refuses a flow that resolved without one. This is what lets `Models.login()` — which persists through the store adapter as part of logging in — stay the single writer, instead of the credential being copied back out and written a second time. - **The interaction travels with the request, not a registry.** Whoever starts an authorization is the one who can talk to the human about it, so prompts reach exactly the page that asked, a headless caller supplies an interaction that declines, and there is no ambient provider to be absent or ambiguous between two open tabs. **`llm-pi-ai` holds all three translations.** `credentialStoreFrom` maps pi-ai's `CredentialStore` onto records; `authContextFrom` answers pi-ai's ambient questions from the credential seam, then the launch environment, with file existence checked against the host process's filesystem; `registerPiAiFlows` restates pi-ai's `AuthEvent`/`AuthPrompt` in the neutral vocabulary and runs `Models.login()`. Every collection is built with the first two, which is what keeps a signed-in provider signed in across the collection rebuild a configuration change causes. With a posture that works, the directory stops withholding OAuth-only routes and `openai-codex` is offered again. @@ -35,7 +35,9 @@ The credential plane stays optional, as it already was for reference resolution. ### Two mechanisms the seams needed underneath -`withFileLock` takes a per-call wait limit. pi-ai runs an OAuth refresh *inside* `credentials.modify()`, so the record write path holds the lock across a network round trip; the 2s default was chosen for a render-and-rename and would fail every other writer of the document. The retry cadence stays fixed — that is a protocol constant — while how long to wait becomes a property of the operation holding the lock. +`withFileLock` takes a per-call wait limit. pi-ai runs an OAuth refresh *inside* `credentials.modify()`, so the record write path holds the lock across a network round trip; the 2s default was chosen for a render-and-rename and would fail every other writer of the document. The retry cadence stays fixed — that is a protocol constant — while the wait is sized by the longest holder a contender can meet: refs and records share one file and one lock, so every writer of the document (`DOCUMENT_LOCK_WAIT_MS`, reference writes and record deletes included) waits an OAuth refresh out, not only the mutation that runs one. + +The seam's edges get the same discipline as its write path. A prompt decline is an outcome, not a breakage — an interaction rejects with `AuthorizationDeclinedError` and the attempt settles `cancelled` — while a notice a surface cannot render is logged and lost rather than failing the flow, and `authorization/settled` fans out with contained listener failures on the credentials seam's terms. On the store side, an api-key record is admitted before it is rendered (what `parseRecord` refuses at the next boot is refused at the write), and `llm-pi-ai` asks `isCredentialKeySegment` before addressing a record, so an arbitrary hand-declared route key reads as "nothing stored" instead of throwing mid-resolution. Withdrawal settles an attempt whether or not its flow reacts to the signal. A flow is supposed to stop when its signal fires, but one that does not would hold its key for the life of the process, and a wedged key is indistinguishable from a busy one from outside. The orphaned run is left to finish on its own. diff --git a/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.zh.md b/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.zh.md index e730b00c7b..ee79248a8c 100644 --- a/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.zh.md +++ b/.agents/notes/implemented/architecture/2026-08-13-credential-records-and-authorization-flows.zh.md @@ -26,7 +26,7 @@ harness 的凭据平面只能表达一种机密：藏在某个环境变量名之 两个选择承担了主要分量： -- **写入由 flow 拥有。** `run()` 返回即表示记录已通过 `ctx.credentials` 提交；seam 随后核实，并拒绝返回时没留下记录的 flow。正是这一点让 `Models.login()`——它把持久化当作登录的一部分，经由 store 适配器完成——保持为唯一写入方，而不是把凭据复制出来再写第二遍。 +- **写入由 flow 拥有。** `run()` 返回即表示记录已通过 `ctx.credentials` 提交；seam 核实的是本次尝试期间观察到的提交——只看记录存在与否，会让重新授权把陈旧记录冒充成新鲜的——并拒绝返回时没提交记录的 flow。正是这一点让 `Models.login()`——它把持久化当作登录的一部分，经由 store 适配器完成——保持为唯一写入方，而不是把凭据复制出来再写第二遍。 - **交互随请求传入，而非注册表。** 发起授权的一方才是能与人对话的一方，因此提示恰好抵达发问的那个页面，无头调用方传入一个直接拒绝的交互实现，也不存在"环境提供方缺席"或"该归两个已打开标签页中哪一个"的问题。 **三处翻译全都留在 `llm-pi-ai`。** `credentialStoreFrom` 把 pi-ai 的 `CredentialStore` 映射到记录；`authContextFrom` 先查凭据 seam 再查启动环境来回答 pi-ai 的环境提问，文件存在性则按宿主进程的文件系统判断；`registerPiAiFlows` 把 pi-ai 的 `AuthEvent`/`AuthPrompt` 重述为中立词汇并运行 `Models.login()`。每个集合都用前两者构造，正是这一点让已登录的提供方在配置变更导致集合重建之后仍然处于登录状态。有了行得通的姿态之后，目录不再扣留仅 OAuth 的路由，`openai-codex` 重新被提供。 @@ -35,7 +35,9 @@ harness 的凭据平面只能表达一种机密：藏在某个环境变量名之 ### seam 底下需要的两处机制 -`withFileLock` 接受按调用声明的等待上限。pi-ai 在 `credentials.modify()` **内部**执行 OAuth 刷新，因此记录写入路径要跨越一次网络往返持锁；2 秒的默认值是按"渲染并 rename"的量级选的，会让该文档的每一个其他写入方失败。重试节奏保持固定——那是协议常量——而"等多久"成为持锁操作自身的属性。 +`withFileLock` 接受按调用声明的等待上限。pi-ai 在 `credentials.modify()` **内部**执行 OAuth 刷新，因此记录写入路径要跨越一次网络往返持锁；2 秒的默认值是按"渲染并 rename"的量级选的，会让该文档的每一个其他写入方失败。重试节奏保持固定——那是协议常量——而等待时长按争用方可能遇到的最长持锁方来定：refs 与 records 共享同一份文件、同一把锁，因此该文档的每一个写入方（`DOCUMENT_LOCK_WAIT_MS`，含引用写入与记录删除）都要等得起一次 OAuth 刷新，而不只是执行刷新的那个 mutation。 + +seam 的边缘与写入路径同一纪律。prompt 被拒是结果而非故障——交互实现以 `AuthorizationDeclinedError` 拒绝，尝试以 `cancelled` 结算；渲染不了 notice 的界面只丢那条 notice、绝不拖垮 flow；`authorization/settled` 按 credentials seam 的条款以遏制方式分发监听器故障。存储侧，api-key 记录在渲染前先行准入（`parseRecord` 下次启动会拒绝的，写入时就拒绝），`llm-pi-ai` 在寻址记录前先问 `isCredentialKeySegment`，任意手写路由键读作「没有存储任何东西」，而不是在解析途中抛错。 撤销会结算一次尝试，无论其 flow 是否响应信号。flow 本应在信号触发时停止，但不停止的那个会把键占到进程结束，而被卡住的键从外部看与忙碌中的键无法区分。被遗弃的执行体听任其自行结束。 diff --git a/docs/config-catalog.i18n.yaml b/docs/config-catalog.i18n.yaml index 6ddf964471..e25c7d8c31 100644 --- a/docs/config-catalog.i18n.yaml +++ b/docs/config-catalog.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 docs/config-catalog.md -config-catalog.md: b93b91f12703ea52225e1202d6cdc376a0b96331 -config-catalog.zh.md: 2562cae37ad4ebce7646b80d7c54cc59bcfd0abf +config-catalog.md: 36f3e96e69207076b1a1bdae58032f7b3d0c1b8f +config-catalog.zh.md: 8e66b1b2c9c4e54be33fd9c2ccff5b8a3494aab6 diff --git a/docs/config-catalog.md b/docs/config-catalog.md index b93b91f127..36f3e96e69 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -567,7 +567,7 @@ export interface Config { } ``` -Source: [`packages/credentials/credentials-local/src/index.ts:63`](../packages/credentials/credentials-local/src/index.ts) +Source: [`packages/credentials/credentials-local/src/index.ts:64`](../packages/credentials/credentials-local/src/index.ts) diff --git a/docs/config-catalog.zh.md b/docs/config-catalog.zh.md index 2562cae37a..8e66b1b2c9 100644 --- a/docs/config-catalog.zh.md +++ b/docs/config-catalog.zh.md @@ -569,7 +569,7 @@ export interface Config { } ``` -来源：[`packages/credentials/credentials-local/src/index.ts:63`](../packages/credentials/credentials-local/src/index.ts) +来源：[`packages/credentials/credentials-local/src/index.ts:64`](../packages/credentials/credentials-local/src/index.ts) diff --git a/docs/event-producer-consumer.i18n.yaml b/docs/event-producer-consumer.i18n.yaml index b7339dbd00..1ac2e8f56a 100644 --- a/docs/event-producer-consumer.i18n.yaml +++ b/docs/event-producer-consumer.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 docs/event-producer-consumer.md -event-producer-consumer.md: f5ead70b691b43bf9de4507d6753e7066c2d2013 -event-producer-consumer.zh.md: 7aa0f03492561e77c94ab36b5da1ee4b4346d861 +event-producer-consumer.md: 49c7a128fcf2539d342ad263bb856c40f2a33f0c +event-producer-consumer.zh.md: dec8b443aa73ae919ceee7cd282846bea4e8ce18 diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index f5ead70b69..49c7a128fc 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -22,7 +22,7 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `agent/status` | `emit` | [`packages/core/agent/src/runtime-types.ts:178`](../packages/core/agent/src/runtime-types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`agent`](../packages/core/agent), `agent-team`, `apiproxy`, [`compaction-basic`](../packages/compaction/compaction-basic), [`goal-round-driver`](../packages/goal/goal-round-driver), [`schedule`](../packages/schedule/schedule), `server` | | `agent/turn-stopping` | `serial` | [`packages/core/agent/src/runtime-types.ts:278`](../packages/core/agent/src/runtime-types.ts) | [`agent-loop`](../packages/core/agent-loop) (`serial`) | [`hooks-claude-code`](../packages/hooks/hooks-claude-code), [`hooks-codex`](../packages/hooks/hooks-codex) | | `approval/request` | `waterfall` | [`packages/interaction/user-approval/src/index.ts:30`](../packages/interaction/user-approval/src/index.ts) | [`user-approval`](../packages/interaction/user-approval) (`waterfall`) | [`acp`](../packages/acp/acp), `apiproxy` | -| `authorization/settled` | `emit` | [`packages/credentials/authorization/src/index.ts:57`](../packages/credentials/authorization/src/index.ts) | [`authorization`](../packages/credentials/authorization) (`emit`) | [`authorization`](../packages/credentials/authorization) | +| `authorization/settled` | `emit` | [`packages/credentials/authorization/src/index.ts:57`](../packages/credentials/authorization/src/index.ts) | [`authorization`](../packages/credentials/authorization) (`events.dispatch`) | [`authorization`](../packages/credentials/authorization) | | `commands/change` | `emit` | [`packages/interaction/commands/src/types.ts:72`](../packages/interaction/commands/src/types.ts) | [`commands`](../packages/interaction/commands) (`events.dispatch`) | `apiproxy` | | `cordis/dynamic-package` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:379`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | | `cordis/dynamic-retract` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:385`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | @@ -30,7 +30,7 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `cordis/inspect-query-resolved` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:397`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | | `cordis/request-run` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:367`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | | `cordis/request-run-resolved` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:373`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | -| `credentials/record-updated` | `emit` | [`packages/credentials/credentials/src/types.ts:87`](../packages/credentials/credentials/src/types.ts) | [`credentials`](../packages/credentials/credentials) (`events.dispatch`) | - | +| `credentials/record-updated` | `emit` | [`packages/credentials/credentials/src/types.ts:87`](../packages/credentials/credentials/src/types.ts) | [`credentials`](../packages/credentials/credentials) (`events.dispatch`) | [`authorization`](../packages/credentials/authorization) | | `credentials/updated` | `emit` | [`packages/credentials/credentials/src/types.ts:75`](../packages/credentials/credentials/src/types.ts) | [`credentials`](../packages/credentials/credentials) (`events.dispatch`) | `apiproxy`, [`credentials`](../packages/credentials/credentials) | | `domain/changed` | `emit` | [`packages/storage/storage-domain/src/events.ts:46`](../packages/storage/storage-domain/src/events.ts) | [`storage-domain`](../packages/storage/storage-domain) (`emit`) | `apiproxy`, [`storage-domain`](../packages/storage/storage-domain), [`workspace`](../packages/workspace/workspace) | | `fs/edit-intent` | `waterfall` | [`packages/fs/fs/src/index.ts:66`](../packages/fs/fs/src/index.ts) | [`tool-fs`](../packages/fs/tool-fs) (`waterfall`), [`tool-str-replace-editor`](../packages/fs/tool-str-replace-editor) (`waterfall`) | [`fs-observation-policy`](../packages/fs/fs-observation-policy) | diff --git a/docs/event-producer-consumer.zh.md b/docs/event-producer-consumer.zh.md index 7aa0f03492..dec8b443aa 100644 --- a/docs/event-producer-consumer.zh.md +++ b/docs/event-producer-consumer.zh.md @@ -24,7 +24,7 @@ | `agent/status` | `emit` | [`packages/core/agent/src/runtime-types.ts:178`](../packages/core/agent/src/runtime-types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`agent`](../packages/core/agent), `agent-team`, `apiproxy`, [`compaction-basic`](../packages/compaction/compaction-basic), [`goal-round-driver`](../packages/goal/goal-round-driver), [`schedule`](../packages/schedule/schedule), `server` | | `agent/turn-stopping` | `serial` | [`packages/core/agent/src/runtime-types.ts:278`](../packages/core/agent/src/runtime-types.ts) | [`agent-loop`](../packages/core/agent-loop) (`serial`) | [`hooks-claude-code`](../packages/hooks/hooks-claude-code), [`hooks-codex`](../packages/hooks/hooks-codex) | | `approval/request` | `waterfall` | [`packages/interaction/user-approval/src/index.ts:30`](../packages/interaction/user-approval/src/index.ts) | [`user-approval`](../packages/interaction/user-approval) (`waterfall`) | [`acp`](../packages/acp/acp), `apiproxy` | -| `authorization/settled` | `emit` | [`packages/credentials/authorization/src/index.ts:57`](../packages/credentials/authorization/src/index.ts) | [`authorization`](../packages/credentials/authorization) (`emit`) | [`authorization`](../packages/credentials/authorization) | +| `authorization/settled` | `emit` | [`packages/credentials/authorization/src/index.ts:57`](../packages/credentials/authorization/src/index.ts) | [`authorization`](../packages/credentials/authorization) (`events.dispatch`) | [`authorization`](../packages/credentials/authorization) | | `commands/change` | `emit` | [`packages/interaction/commands/src/types.ts:72`](../packages/interaction/commands/src/types.ts) | [`commands`](../packages/interaction/commands) (`events.dispatch`) | `apiproxy` | | `cordis/dynamic-package` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:379`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | | `cordis/dynamic-retract` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:385`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | @@ -32,7 +32,7 @@ | `cordis/inspect-query-resolved` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:397`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | | `cordis/request-run` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:367`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | | `cordis/request-run-resolved` | `emit` | [`packages/extensions/cordis-host-runner/src/types.ts:373`](../packages/extensions/cordis-host-runner/src/types.ts) | [`cordis-host-runner`](../packages/extensions/cordis-host-runner) (`emit`) | `apiproxy` | -| `credentials/record-updated` | `emit` | [`packages/credentials/credentials/src/types.ts:87`](../packages/credentials/credentials/src/types.ts) | [`credentials`](../packages/credentials/credentials) (`events.dispatch`) | - | +| `credentials/record-updated` | `emit` | [`packages/credentials/credentials/src/types.ts:87`](../packages/credentials/credentials/src/types.ts) | [`credentials`](../packages/credentials/credentials) (`events.dispatch`) | [`authorization`](../packages/credentials/authorization) | | `credentials/updated` | `emit` | [`packages/credentials/credentials/src/types.ts:75`](../packages/credentials/credentials/src/types.ts) | [`credentials`](../packages/credentials/credentials) (`events.dispatch`) | `apiproxy`, [`credentials`](../packages/credentials/credentials) | | `domain/changed` | `emit` | [`packages/storage/storage-domain/src/events.ts:46`](../packages/storage/storage-domain/src/events.ts) | [`storage-domain`](../packages/storage/storage-domain) (`emit`) | `apiproxy`, [`storage-domain`](../packages/storage/storage-domain), [`workspace`](../packages/workspace/workspace) | | `fs/edit-intent` | `waterfall` | [`packages/fs/fs/src/index.ts:66`](../packages/fs/fs/src/index.ts) | [`tool-fs`](../packages/fs/tool-fs) (`waterfall`), [`tool-str-replace-editor`](../packages/fs/tool-str-replace-editor) (`waterfall`) | [`fs-observation-policy`](../packages/fs/fs-observation-policy) | diff --git a/docs/subsystems/credentials.i18n.yaml b/docs/subsystems/credentials.i18n.yaml index e2526345a5..8f9e2f9474 100644 --- a/docs/subsystems/credentials.i18n.yaml +++ b/docs/subsystems/credentials.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 docs/subsystems/credentials.md -credentials.md: dd0f950b50fa4bcb9094f7b85227262b42e3b385 -credentials.zh.md: 3e906449db31d4889d8c87f27c765cccfc9ff539 +credentials.md: 8d75a254109dcce5d0b54cd4fd293a8ca4e335c5 +credentials.zh.md: 46edda84a063c539748da344d7efa66c7af666e7 diff --git a/docs/subsystems/credentials.md b/docs/subsystems/credentials.md index dd0f950b50..8d75a25410 100644 --- a/docs/subsystems/credentials.md +++ b/docs/subsystems/credentials.md @@ -104,17 +104,19 @@ cancel(key: CredentialKey): void * and the second would answer questions the first was asked. * * @param request - the key, the method, the surface, and the cancel signal. - * @returns `authorized` once the flow's record is committed and observed, - * or `cancelled` when the human or the caller withdrew. + * @returns `authorized` once the flow's record is committed during this + * attempt and observed, or `cancelled` when the human declined or the + * caller withdrew. * @throws {AuthorizationError} code `NO_FLOW` when nothing claims the key, * `UNKNOWN_METHOD` when the named method is not one the flow offers, * `ALREADY_IN_FLIGHT` when an attempt is already running for the key, or - * `NOT_COMMITTED` when the flow resolved without leaving a record behind. + * `NOT_COMMITTED` when the flow resolved without committing a record + * during the attempt. */ async begin(request: AuthorizationRequest): Promise ``` -Source: [`packages/credentials/authorization/src/index.ts:163`](../../packages/credentials/authorization/src/index.ts) +Source: [`packages/credentials/authorization/src/index.ts:182`](../../packages/credentials/authorization/src/index.ts) @@ -208,7 +210,7 @@ abstract modifyRecord( key: CredentialKey, mutate: (current: CredentialRecord | abstract deleteRecord(key: CredentialKey): Promise ``` -Source: [`packages/credentials/credentials/src/index.ts:164`](../../packages/credentials/credentials/src/index.ts) +Source: [`packages/credentials/credentials/src/index.ts:177`](../../packages/credentials/credentials/src/index.ts) diff --git a/docs/subsystems/credentials.zh.md b/docs/subsystems/credentials.zh.md index 3e906449db..46edda84a0 100644 --- a/docs/subsystems/credentials.zh.md +++ b/docs/subsystems/credentials.zh.md @@ -104,17 +104,19 @@ cancel(key: CredentialKey): void * and the second would answer questions the first was asked. * * @param request - the key, the method, the surface, and the cancel signal. - * @returns `authorized` once the flow's record is committed and observed, - * or `cancelled` when the human or the caller withdrew. + * @returns `authorized` once the flow's record is committed during this + * attempt and observed, or `cancelled` when the human declined or the + * caller withdrew. * @throws {AuthorizationError} code `NO_FLOW` when nothing claims the key, * `UNKNOWN_METHOD` when the named method is not one the flow offers, * `ALREADY_IN_FLIGHT` when an attempt is already running for the key, or - * `NOT_COMMITTED` when the flow resolved without leaving a record behind. + * `NOT_COMMITTED` when the flow resolved without committing a record + * during the attempt. */ async begin(request: AuthorizationRequest): Promise ``` -Source: [`packages/credentials/authorization/src/index.ts:163`](../../packages/credentials/authorization/src/index.ts) +Source: [`packages/credentials/authorization/src/index.ts:182`](../../packages/credentials/authorization/src/index.ts) @@ -208,7 +210,7 @@ abstract modifyRecord( key: CredentialKey, mutate: (current: CredentialRecord | abstract deleteRecord(key: CredentialKey): Promise ``` -Source: [`packages/credentials/credentials/src/index.ts:164`](../../packages/credentials/credentials/src/index.ts) +Source: [`packages/credentials/credentials/src/index.ts:177`](../../packages/credentials/credentials/src/index.ts) diff --git a/packages/credentials/authorization/README.i18n.yaml b/packages/credentials/authorization/README.i18n.yaml index cdec37bfab..21d3600aa1 100644 --- a/packages/credentials/authorization/README.i18n.yaml +++ b/packages/credentials/authorization/README.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 packages/credentials/authorization/README.md -README.md: f73d2231456d0873cef7ea047fe537629a7552e0 -README.zh.md: ecceaa4f90a971deebe89e8dd6e51157aa0072a0 +README.md: 373a070c1a5cbb9a60e7ae14734a752515e0a0e4 +README.zh.md: 8dcdc081a87979ec712f18189ff7bbe937328236 diff --git a/packages/credentials/authorization/README.md b/packages/credentials/authorization/README.md index f73d223145..373a070c1a 100644 --- a/packages/credentials/authorization/README.md +++ b/packages/credentials/authorization/README.md @@ -6,7 +6,7 @@ Authorization Service Definition (`ctx.authorization`). Some credentials cannot **A flow is a plugin's knowledge of how to get its own credential.** It is registered under the [`CredentialKey`](../credentials/README.md#two-key-spaces-two-questions) it writes, so a flow says which record it produces and, through that key's scope, which plugin answers for the format inside it. A second authorization protocol arrives as another flow, not as another seam. -**The flow owns the write.** `run()` resolving means the record is already committed through `ctx.credentials`; the seam confirms it and refuses a flow that resolved without one. Committing inside the flow is what lets a library that persists through its own store adapter stay the single writer instead of being copied back out and written twice. +**The flow owns the write.** `run()` resolving means the record is already committed through `ctx.credentials`; the seam confirms a commit it observed during the attempt — presence alone would let a re-authorization pass a stale record off as fresh — and refuses a flow that resolved without one. Committing inside the flow is what lets a library that persists through its own store adapter stay the single writer instead of being copied back out and written twice. **The interaction travels with the request, not a registry.** Whoever starts an authorization is the one who can talk to the human about it, so prompts reach exactly the surface that asked and a headless caller supplies an interaction that declines. There is no ambient provider to be absent, and no question about which of two open pages a prompt belongs to. @@ -14,7 +14,7 @@ Authorization Service Definition (`ctx.authorization`). Some credentials cannot ```ts import type { Context } from '@deepseek-ai/cordis' -import type { AuthorizationSession } from '@deepseek-ai/dsh-authorization' +import { AuthorizationDeclinedError, type AuthorizationSession } from '@deepseek-ai/dsh-authorization' import { credentialKey } from '@deepseek-ai/dsh-credentials' declare const ctx: Context @@ -39,7 +39,7 @@ ctx.authorization.list() // [{ key, label, methods, inFlight ctx.authorization.describe(key) // the same entry, or undefined await ctx.authorization.begin({ // { status: 'authorized' | 'cancelled' } key, - interaction: { notify: () => {}, prompt: () => Promise.reject(new Error('headless')) }, + interaction: { notify: () => {}, prompt: () => Promise.reject(new AuthorizationDeclinedError()) }, }) ctx.authorization.cancel(key) // withdraw whatever is running for the key dispose() @@ -51,7 +51,9 @@ One attempt per key at a time. A second caller is refused with `ALREADY_IN_FLIGH An attempt whose caller has already withdrawn never claims the key and never starts the flow — relying on each flow to check its signal before the first await would let one that does not hang holding the key. Validation still runs first, so a caller naming a key or method that does not exist hears about it whether or not it also gave up. -`authorization/settled (key, settlement)` fires after the key is released, for every terminal outcome. `settlement` adds `failed` to the two statuses `begin()` can return: a failure reaches its own caller as a thrown error, so the event stream is the only place a watcher that did not start the attempt can tell a refusal from a breakage. +A human's "no" is an outcome, not a breakage. An interaction that declines rejects its prompt with `AuthorizationDeclinedError`, and an attempt that fails after a declined prompt settles as `cancelled`, exactly as a withdrawn signal does; any other prompt rejection stays a flow failure that reaches the caller. A notice is fire-and-forget on the same principle, held at the seam: a surface that cannot render one loses the notice, never the attempt. + +`authorization/settled (key, settlement)` fires after the key is released, for every terminal outcome. `settlement` adds `failed` to the two statuses `begin()` can return: a failure reaches its own caller as a thrown error, so the event stream is the only place a watcher that did not start the attempt can tell a refusal from a breakage. Listener failures are contained: every listener runs, a throw or rejection is logged without changing the finished attempt's outcome, and only an `INVARIANT`-coded failure rethrows after the rest ran. ## The interaction vocabulary diff --git a/packages/credentials/authorization/README.zh.md b/packages/credentials/authorization/README.zh.md index ecceaa4f90..8dcdc081a8 100644 --- a/packages/credentials/authorization/README.zh.md +++ b/packages/credentials/authorization/README.zh.md @@ -6,7 +6,7 @@ **flow 是某个插件"如何取得自己那份凭据"的知识。** 它以自己写入的 [`CredentialKey`](../credentials/README.md#two-key-spaces-two-questions) 注册，因此 flow 声明了自己产出哪条记录，并通过该键的 scope 声明由哪个插件为记录内部的格式负责。第二种授权协议以另一个 flow 的形式到来，而不是另一个 seam。 -**写入由 flow 拥有。** `run()` 返回即表示记录已经通过 `ctx.credentials` 提交；seam 随后核实，并拒绝那些返回时没留下记录的 flow。让提交发生在 flow 内部，才能使一个通过自有 store 适配器持久化的库保持为唯一写入方，而不是把凭据复制出来再写第二遍。 +**写入由 flow 拥有。** `run()` 返回即表示记录已经通过 `ctx.credentials` 提交；seam 核实的是它在本次尝试期间观察到的提交——只看记录存在与否，会让重新授权把陈旧记录冒充成新鲜的——并拒绝那些返回时没提交记录的 flow。让提交发生在 flow 内部，才能使一个通过自有 store 适配器持久化的库保持为唯一写入方，而不是把凭据复制出来再写第二遍。 **交互随请求传入，而非注册表。** 发起授权的一方才是能与人对话的一方，因此提示恰好抵达发问的那个界面，无头调用方则传入一个直接拒绝的交互实现。这样既不存在"环境提供方缺席"的问题，也不会出现某个提示该归两个已打开页面中哪一个的疑问。 @@ -14,7 +14,7 @@ ```ts import type { Context } from '@deepseek-ai/cordis' -import type { AuthorizationSession } from '@deepseek-ai/dsh-authorization' +import { AuthorizationDeclinedError, type AuthorizationSession } from '@deepseek-ai/dsh-authorization' import { credentialKey } from '@deepseek-ai/dsh-credentials' declare const ctx: Context @@ -39,7 +39,7 @@ ctx.authorization.list() // [{ key, label, methods, inFlight ctx.authorization.describe(key) // the same entry, or undefined await ctx.authorization.begin({ // { status: 'authorized' | 'cancelled' } key, - interaction: { notify: () => {}, prompt: () => Promise.reject(new Error('headless')) }, + interaction: { notify: () => {}, prompt: () => Promise.reject(new AuthorizationDeclinedError()) }, }) ctx.authorization.cancel(key) // withdraw whatever is running for the key dispose() @@ -51,7 +51,9 @@ dispose() 调用方在发起前就已撤销的尝试，既不占用该键也不启动 flow——若指望每个 flow 都在首个 await 之前检查自己的 signal，那么没有检查的那个就会占着键一直挂起。校验仍然先执行，因此调用方给出的键或方法不存在时，无论它是否已经放弃都会收到报错。 -`authorization/settled (key, settlement)` 在键释放之后触发，覆盖每一种终态。`settlement` 在 `begin()` 能返回的两种状态之外增加了 `failed`：失败以抛出的错误抵达其调用方，因此事件流是未发起该尝试的旁观者唯一能区分"被拒绝"与"出故障"的地方。 +人的"不"是一种结果，不是故障。选择拒绝的交互实现让 prompt 以 `AuthorizationDeclinedError` 拒绝，在提示被拒之后才失败的尝试以 `cancelled` 结算，与 signal 撤销完全一致；其余任何 prompt 拒绝仍是抵达调用方的 flow 故障。notice 依同一原则即发即忘，并由 seam 兜底：渲染不了 notice 的界面丢掉的是那条 notice，而不是整次尝试。 + +`authorization/settled (key, settlement)` 在键释放之后触发，覆盖每一种终态。`settlement` 在 `begin()` 能返回的两种状态之外增加了 `failed`：失败以抛出的错误抵达其调用方，因此事件流是未发起该尝试的旁观者唯一能区分"被拒绝"与"出故障"的地方。监听器故障被就地遏制：每个监听器都会执行，抛错或拒绝只记录日志、不改变已结束尝试的结果，仅 `INVARIANT` 编码的故障在其余监听器执行完后重抛。 ## 交互词汇 diff --git a/packages/credentials/authorization/src/index.ts b/packages/credentials/authorization/src/index.ts index 9cff052feb..502f2f2828 100644 --- a/packages/credentials/authorization/src/index.ts +++ b/packages/credentials/authorization/src/index.ts @@ -66,6 +66,23 @@ export class AuthorizationError extends HarnessError { } } +/** + * The rejection an {@link AuthorizationInteraction.prompt} uses to say the + * human declined — dismissed the question, chose not to answer — rather than + * that the surface broke. An attempt whose flow fails after a prompt was + * declined settles as `cancelled`, the same outcome as a withdrawn signal, + * because the human saying no is a refusal, not a breakage. Only a human's + * "no" may reject with this class: a prompt withdrawn by its own `signal` (a + * flow retiring the losing question of a race) must reject with something + * else, or a later genuine failure would be misread as a decline. + */ +export class AuthorizationDeclinedError extends AuthorizationError { + constructor(message = 'the authorization prompt was declined') { + super(message, 'DECLINED') + this.name = 'AuthorizationDeclinedError' + } +} + /** * What a running flow is given to talk to the human. Every member is scoped to * one attempt: the flow neither knows nor chooses which surface is listening. @@ -93,10 +110,11 @@ export interface AuthorizationSession { /** * A plugin's knowledge of how to obtain one credential. The flow owns the * write: `run()` resolving means the record for `key` is committed through - * `ctx.credentials`, which the seam then confirms before reporting success. - * Committing inside the flow is what lets a library that persists through its - * own store adapter (pi-ai's `Models.login()`) stay the single writer instead - * of being copied back out and written twice. + * `ctx.credentials` during that run, which the seam confirms — a commit + * observed within the attempt, still present after it — before reporting + * success. Committing inside the flow is what lets a library that persists + * through its own store adapter (pi-ai's `Models.login()`) stay the single + * writer instead of being copied back out and written twice. */ export interface AuthorizationFlow { /** The credential record this flow writes. Its scope names the owning plugin. */ @@ -134,7 +152,8 @@ export interface AuthorizationInteraction { * Put a question to the human and wait. * @param prompt - what to ask, and how it should be presented. * @returns the typed text, or the chosen option's id. - * @throws when the human declines or the prompt is withdrawn. + * @throws {AuthorizationDeclinedError} when the human declines; any other + * rejection reads as the surface failing, not as an answer. */ prompt(prompt: AuthorizationPrompt): Promise } @@ -244,12 +263,14 @@ export class AuthorizationService extends Service { * and the second would answer questions the first was asked. * * @param request - the key, the method, the surface, and the cancel signal. - * @returns `authorized` once the flow's record is committed and observed, - * or `cancelled` when the human or the caller withdrew. + * @returns `authorized` once the flow's record is committed during this + * attempt and observed, or `cancelled` when the human declined or the + * caller withdrew. * @throws {AuthorizationError} code `NO_FLOW` when nothing claims the key, * `UNKNOWN_METHOD` when the named method is not one the flow offers, * `ALREADY_IN_FLIGHT` when an attempt is already running for the key, or - * `NOT_COMMITTED` when the flow resolved without leaving a record behind. + * `NOT_COMMITTED` when the flow resolved without committing a record + * during the attempt. */ async begin(request: AuthorizationRequest): Promise { const { key } = request @@ -286,10 +307,52 @@ export class AuthorizationService extends Service { this.running.delete(key) // After the slot is released, so a listener that reacts by starting the // next attempt is not refused by the one that just finished. - this.ctx.emit('authorization/settled', key, settlement) + this.settle(key, settlement) } } + /* jscpd:ignore-start -- deliberate symmetry with the credentials seam's + commit fan-out (`CredentialProvider`): the contained-dispatch shape is the + reviewed listener-lifecycle contract, and extracting it would couple the + two seams' event semantics. */ + /** + * Fan `authorization/settled` out with contained listener failures: every + * listener runs, and a sync throw or async rejection is logged without + * changing the finished attempt's own outcome — except `INVARIANT`-coded + * failures, which rethrow after every listener ran. The attempt is already + * over and its key released when this fires, so a broken watcher (that + * second browser tab) can never turn the caller's settled result into a + * failure of its own. + */ + private settle(key: CredentialKey, settlement: AuthorizationSettlement): void { + let invariantFailure: unknown + const args = ['authorization/settled', key, settlement] + for (const listener of this.ctx.events.dispatch('emit', args) as Array<(...listenerArgs: unknown[]) => unknown>) { + try { + const returned = listener(key, settlement) + if (returned != null && typeof (returned as PromiseLike).then === 'function') { + void Promise.resolve(returned as PromiseLike).then(undefined, (error: unknown) => { + this.warnSettledListenerFailure(key, error) + }) + } + } catch (error) { + if ((error as { code?: unknown } | null)?.code === 'INVARIANT') { + invariantFailure ??= error + continue + } + this.warnSettledListenerFailure(key, error) + } + } + if (invariantFailure !== undefined) throw invariantFailure as Error + } + /* jscpd:ignore-end */ + + /** Contained-listener diagnostic shared by the sync and async failure paths. */ + private warnSettledListenerFailure(key: CredentialKey, error: unknown): void { + this.ctx.logger.warn('authorization: an authorization/settled listener for "%s" failed', key) + this.ctx.logger.warn(error) + } + /** Run the flow, then hold it to its half of the commit contract. */ private async attempt( flow: AuthorizationFlow, @@ -308,30 +371,63 @@ export class AuthorizationService extends Service { // withdrawn, so this signal cannot already be aborted here. signal.addEventListener('abort', () => { resolve('withdrawn') }, { once: true }) }) - const running = flow.run({ - method, - signal, - notify: (notice) => { interaction.notify(notice) }, - prompt: prompt => interaction.prompt(prompt), + // What the seam itself witnessed during the run, held as properties + // because closure writes do not narrow locals across awaits: the prompt + // wrapper sees a decline first-hand (a flow that rewraps the rejection on + // its way out cannot hide it), and confirming the commit means confirming + // it happened *now* — on a re-auth the record already exists, so presence + // alone would let a flow that wrote nothing report the stale credential + // as freshly authorized. + const observed = { declined: false, committed: false } + const unwatch = this.ctx.on('credentials/record-updated', (key: CredentialKey) => { + if (key === flow.key) observed.committed = true }) try { - if (await Promise.race([running.then(() => 'ran' as const), withdrawn]) === 'withdrawn') { - // Nothing awaits the orphan any more, so its eventual failure has to be - // marked handled or it would take down the process. - void running.catch(() => { this.ctx.logger.debug('authorization: withdrawn flow failed after the fact') }) - return { status: 'cancelled' } + const running = flow.run({ + method, + signal, + notify: (notice) => { + try { + interaction.notify(notice) + } catch (error) { + // Fire-and-forget is held at the seam: a surface that cannot + // render a notice (a page whose connection just closed) loses the + // notice, never the attempt. + this.ctx.logger.warn('authorization: the interaction surface failed to render a notice') + this.ctx.logger.warn(error) + } + }, + prompt: prompt => interaction.prompt(prompt).catch((error: unknown) => { + if (error instanceof AuthorizationDeclinedError) observed.declined = true + throw error + }), + }) + try { + if (await Promise.race([running.then(() => 'ran' as const), withdrawn]) === 'withdrawn') { + // Nothing awaits the orphan any more, so its eventual failure has to be + // marked handled or it would take down the process. + void running.catch(() => { this.ctx.logger.debug('authorization: withdrawn flow failed after the fact') }) + return { status: 'cancelled' } + } + } catch (error) { + // A withdrawn attempt and a declined prompt are outcomes, not + // failures: the human said no, or closed the page. Anything else is + // the flow failing and belongs to the caller, cause chain intact. + if (signal.aborted || observed.declined) return { status: 'cancelled' } + throw error } - } catch (error) { - // A withdrawn attempt is an outcome, not a failure: the human said no, or - // closed the page. Anything else is the flow failing and belongs to the - // caller, cause chain intact. - if (signal.aborted) return { status: 'cancelled' } - throw error + } finally { + unwatch() + } + if (!observed.committed) { + throw new AuthorizationError( + `authorization flow for "${flow.key}" resolved without committing a credential record in this attempt`, + 'NOT_COMMITTED') } const stored = await this.ctx.credentials.describeRecord(flow.key) if (!stored.configured) { throw new AuthorizationError( - `authorization flow for "${flow.key}" resolved without committing a credential record`, + `authorization flow for "${flow.key}" deleted its credential record instead of committing one`, 'NOT_COMMITTED') } return { status: 'authorized' } diff --git a/packages/credentials/authorization/tests/authorization.spec.ts b/packages/credentials/authorization/tests/authorization.spec.ts index 286c99898b..99efb9476f 100644 --- a/packages/credentials/authorization/tests/authorization.spec.ts +++ b/packages/credentials/authorization/tests/authorization.spec.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { Context } from '@deepseek-ai/cordis' import { credentialKey } from '@deepseek-ai/dsh-credentials' import AuthorizationService, { + AuthorizationDeclinedError, type AuthorizationFlow, type AuthorizationInteraction, type AuthorizationSession, @@ -304,3 +305,161 @@ describe('AuthorizationService.begin', () => { .rejects.toThrow(/resolved without committing a credential record/) }) }) + +describe('commit confirmation', () => { + it('refuses a re-auth that left only the record of an earlier attempt', async () => { + const ctx = await harness() + await ctx.credentials.modifyRecord(KEY, () => + Promise.resolve({ kind: 'grant', payload: { token: 'stale' } })) + ctx.authorization.registerFlow({ + key: KEY, + label: 'Forgetful', + methods: [{ id: 'oauth', label: 'Sign in' }], + // A commit for another key is not this flow's commit either. + async run() { + await ctx.credentials.modifyRecord(OTHER, () => + Promise.resolve({ kind: 'grant', payload: { token: 'other' } })) + }, + }) + + await expect(ctx.authorization.begin({ key: KEY, interaction: surface() })) + .rejects.toThrow(/without committing a credential record in this attempt/) + // Refused, not cleaned up: the stale record still belongs to its owner. + expect(await ctx.credentials.readRecord(KEY)).toEqual({ kind: 'grant', payload: { token: 'stale' } }) + }) + + it('refuses a flow that deleted its record instead of committing one', async () => { + const ctx = await harness() + await ctx.credentials.modifyRecord(KEY, () => + Promise.resolve({ kind: 'grant', payload: { token: 'stale' } })) + ctx.authorization.registerFlow({ + key: KEY, + label: 'Destructive', + methods: [{ id: 'oauth', label: 'Sign in' }], + run: () => ctx.credentials.deleteRecord(KEY), + }) + + await expect(ctx.authorization.begin({ key: KEY, interaction: surface() })) + .rejects.toThrow(/deleted its credential record/) + }) +}) + +describe('declined prompts', () => { + it('reports an attempt whose prompt the human declined as cancelled, not failed', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx, KEY, async (session) => { + await session.prompt({ kind: 'text', message: 'Paste the code' }) + })) + const settled = vi.fn() + ctx.on('authorization/settled', settled) + const declining: AuthorizationInteraction = { + notify: () => undefined, + prompt: () => Promise.reject(new AuthorizationDeclinedError()), + } + + await expect(ctx.authorization.begin({ key: KEY, interaction: declining })) + .resolves.toEqual({ status: 'cancelled' }) + + expect(settled).toHaveBeenCalledWith(KEY, 'cancelled') + }) + + it('reads a decline through a flow that rewraps the rejection on its way out', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx, KEY, session => + session.prompt({ kind: 'text', message: 'Paste the code' }).then( + () => undefined, + () => { + throw new Error('sign-in aborted') + }))) + const declining: AuthorizationInteraction = { + notify: () => undefined, + prompt: () => Promise.reject(new AuthorizationDeclinedError()), + } + + await expect(ctx.authorization.begin({ key: KEY, interaction: declining })) + .resolves.toEqual({ status: 'cancelled' }) + }) + + it('keeps a prompt failure that is not a decline a flow failure', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx, KEY, async (session) => { + await session.prompt({ kind: 'text', message: 'Paste the code' }) + })) + const settled = vi.fn() + ctx.on('authorization/settled', settled) + const broken: AuthorizationInteraction = { + notify: () => undefined, + prompt: () => Promise.reject(new Error('the transport dropped')), + } + + await expect(ctx.authorization.begin({ key: KEY, interaction: broken })) + .rejects.toThrow('the transport dropped') + + expect(settled).toHaveBeenCalledWith(KEY, 'failed') + }) +}) + +describe('notice containment', () => { + it('loses the notice, never the attempt, when the surface cannot render it', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx, KEY, (session) => { + session.notify({ message: 'Continue in your browser' }) + return Promise.resolve() + })) + const broken: AuthorizationInteraction = { + notify: () => { + throw new Error('page connection closed') + }, + prompt: () => Promise.resolve('unused'), + } + + await expect(ctx.authorization.begin({ key: KEY, interaction: broken })) + .resolves.toEqual({ status: 'authorized' }) + }) +}) + +describe('the settled fan-out', () => { + it('keeps a throwing listener from changing a finished attempt, and later listeners still run', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx)) + ctx.on('authorization/settled', () => { + throw new Error('watcher boom') + }) + const second = vi.fn() + ctx.on('authorization/settled', second) + + await expect(ctx.authorization.begin({ key: KEY, interaction: surface() })) + .resolves.toEqual({ status: 'authorized' }) + + expect(second).toHaveBeenCalledWith(KEY, 'authorized') + }) + + it('contains an async listener rejection', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx)) + // An unknown-returning function keeps the typed surface legal while the + // runtime value is still the rejected promise the containment must handle. + const boom = (): unknown => Promise.reject(new Error('async watcher boom')) + ctx.on('authorization/settled', boom) + + await expect(ctx.authorization.begin({ key: KEY, interaction: surface() })) + .resolves.toEqual({ status: 'authorized' }) + await new Promise(resolve => setTimeout(resolve, 10)) + }) + + it('rethrows an invariant-coded listener failure after the remaining listeners', async () => { + const ctx = await harness() + ctx.authorization.registerFlow(committingFlow(ctx)) + ctx.on('authorization/settled', () => { + throw Object.assign(new Error('forged relation'), { code: 'INVARIANT' }) + }) + const second = vi.fn() + ctx.on('authorization/settled', second) + + await expect(ctx.authorization.begin({ key: KEY, interaction: surface() })) + .rejects.toThrow(/forged relation/) + // Harness-fatal by design — but the record itself committed first. + expect(second).toHaveBeenCalledWith(KEY, 'authorized') + expect(await ctx.credentials.readRecord(KEY)).toEqual({ kind: 'grant', payload: { token: 'granted' } }) + }) +}) diff --git a/packages/credentials/authorization/tests/memory.ts b/packages/credentials/authorization/tests/memory.ts index 94ebe091d2..1d9990af99 100644 --- a/packages/credentials/authorization/tests/memory.ts +++ b/packages/credentials/authorization/tests/memory.ts @@ -14,6 +14,9 @@ import type { * half is exercised — the seam's whole interest in this service is whether a * flow left a record behind — so the reference half answers "nothing stored". */ +// TODO: near-duplicate of the record half of +// packages/credentials/credentials/tests/memory.ts; fold both into a shared +// test-support double when a third suite needs one. export class MemoryCredentials extends CredentialProvider { private readonly records = new Map() diff --git a/packages/credentials/credentials-local/src/index.ts b/packages/credentials/credentials-local/src/index.ts index e394d8ade0..20a8313344 100644 --- a/packages/credentials/credentials-local/src/index.ts +++ b/packages/credentials/credentials-local/src/index.ts @@ -46,6 +46,7 @@ import { canonicalizeWatchPath, resolveDshHome } from '@deepseek-ai/dsh-home-pat import { launchEnvironmentOf } from '@deepseek-ai/dsh-launch-environment' import { CredentialProvider, credentialRef, parseCredentialKey } from '@deepseek-ai/dsh-credentials' import type { + ApiKeyRecord, CredentialInfo, CredentialKey, CredentialRecord, @@ -100,12 +101,15 @@ const GROUP_OTHER_BITS = 0o077 * mutation runs its caller's decision while holding the lock, and for the * operation this half exists to serve — an owner refreshing an expired token — * that decision includes a network round trip. The file-work default would - * fail every other writer of this document for its duration. Like the retry - * cadence in `dsh-atomic-write`, this is a robustness bound of the write - * protocol rather than a deployment choice: it is sized by what a provider - * request costs, which no deployment varies. + * fail every other writer of this document for its duration. A contender's + * wait is sized by the longest holder it can meet, and refs and records share + * one file and one lock, so every writer of this document — reference writes + * and record deletes included — waits this long, not only the mutation that + * holds it. Like the retry cadence in `dsh-atomic-write`, this is a + * robustness bound of the write protocol rather than a deployment choice: it + * is sized by what a provider request costs, which no deployment varies. */ -const RECORD_LOCK_WAIT_MS = 30_000 +const DOCUMENT_LOCK_WAIT_MS = 30_000 /** * Reject a credentials document other OS users can read, before its contents @@ -253,6 +257,26 @@ function parseRecords(section: unknown, filename: string): Map { if (section === undefined || section === null) return {} @@ -626,10 +650,11 @@ export class LocalCredentialProvider extends CredentialProvider { const current = this.records.get(key) const next = await mutate(current) if (next === undefined) return current - // Admitted before it is rendered: the promise that a payload comes - // back exactly as written is only keepable for values this document - // can spell, and a value rejected here has not been stored. + // Admitted before it is rendered: what the read path would refuse is + // refused here first, so a caller can never persist a document the + // next boot rejects, and a value refused here has not been stored. if (next.kind === 'grant') assertJsonValue(`record "${key}" payload`, next.payload, new Set()) + else assertStorableApiKey(key, next) const nextText = renderRecord(this.text, key, next) // 0600: a document holding secrets is never world-readable. await writeFileAtomic(this.spec.filename, nextText, { mode: 0o600, dirMode: 0o700 }) @@ -638,7 +663,7 @@ export class LocalCredentialProvider extends CredentialProvider { // After the commit, on the same terms as a reference write. this.notifyRecordUpdated(key) return next - }, { waitMs: RECORD_LOCK_WAIT_MS }) + }, { waitMs: DOCUMENT_LOCK_WAIT_MS }) }) } @@ -657,7 +682,7 @@ export class LocalCredentialProvider extends CredentialProvider { this.text = nextText this.records.delete(key) this.notifyRecordUpdated(key) - }) + }, { waitMs: DOCUMENT_LOCK_WAIT_MS }) }) } @@ -718,7 +743,7 @@ export class LocalCredentialProvider extends CredentialProvider { // After the commit: a broken observer must never make the durable // write look failed (an INVARIANT failure still rethrows). this.notifyUpdated(ref) - }) + }, { waitMs: DOCUMENT_LOCK_WAIT_MS }) }) } diff --git a/packages/credentials/credentials-local/tests/records.spec.ts b/packages/credentials/credentials-local/tests/records.spec.ts index dfae94a093..08aec98ad6 100644 --- a/packages/credentials/credentials-local/tests/records.spec.ts +++ b/packages/credentials/credentials-local/tests/records.spec.ts @@ -316,6 +316,24 @@ describe('record mutation', () => { await expect(readFile(path, 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) }) + it('refuses an api-key record this document could not read back', async () => { + const dir = await tempDir() + const path = join(dir, '.credentials.yaml') + const ctx = await boot({ path, watch: false }) + + // The same admission rule as the read path: an api-key record parseRecord + // would reject at the next boot is refused before it is rendered, so the + // current process can never report a success the next one refuses to load. + await expect(put(ctx, CODEX, { kind: 'api-key', key: '' })) + .rejects.toThrow(/empty key/) + await expect(put(ctx, CODEX, { kind: 'api-key', env: { 'not a name': 'value' } })) + .rejects.toThrow(/must match/) + await expect(put(ctx, CODEX, { kind: 'api-key', env: { AWS_REGION: '' } })) + .rejects.toThrow(/non-empty string/) + expect(await ctx.credentials.readRecord(CODEX)).toBeUndefined() + await expect(readFile(path, 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + it('refuses record writes once disposed', async () => { const dir = await tempDir() const ctx = new Context() diff --git a/packages/credentials/credentials-local/tests/review-fixes.spec.ts b/packages/credentials/credentials-local/tests/review-fixes.spec.ts index 495fe22b33..60aab4c6e1 100644 --- a/packages/credentials/credentials-local/tests/review-fixes.spec.ts +++ b/packages/credentials/credentials-local/tests/review-fixes.spec.ts @@ -7,7 +7,7 @@ import { Context } from '@deepseek-ai/cordis' import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { credentialRef } from '@deepseek-ai/dsh-credentials' +import { credentialKey, credentialRef } from '@deepseek-ai/dsh-credentials' import { LocalCredentialProvider } from '../src/index.ts' /** Credential documents are seeded owner-only, exactly as the provider creates them. */ @@ -80,6 +80,37 @@ describe('read-modify-write', () => { await ctx.credentials.set(ALPHA, 'one') if (process.platform !== 'win32') expect((await stat(home)).mode & 0o777).toBe(0o700) }) + + it('holds every writer of the document to the record-mutation lock wait', async () => { + const dir = await tempDir() + const path = join(dir, '.credentials.yaml') + const holder = await boot({ path, watch: false }) + const contender = await boot({ path, watch: false }) + const doomed = credentialKey('llm-pi-ai', 'doomed') + const slowKey = credentialKey('llm-pi-ai', 'slow') + await holder.credentials.modifyRecord(doomed, () => Promise.resolve({ kind: 'api-key', key: 'x' })) + const entered = Promise.withResolvers() + // The mutation holds the cross-process writer lock across a stand-in for + // an OAuth refresh round trip — longer than withFileLock's 2s default. + const slow = holder.credentials.modifyRecord(slowKey, async () => { + entered.resolve(undefined) + await new Promise(resolve => setTimeout(resolve, 2_400)) + return { kind: 'api-key', key: 'slow' } + }) + await entered.promise + // The other two writer paths — a reference write and a record delete — + // share that file and that lock, so they must wait the refresh out rather + // than fail at the file-work default. + await Promise.all([ + contender.credentials.set(ALPHA, 'waited'), + contender.credentials.deleteRecord(doomed), + ]) + await slow + const reread = await boot({ path, watch: false }) + expect(await reread.credentials.resolve(ALPHA)).toEqual({ value: 'waited', source: 'file' }) + expect(await reread.credentials.readRecord(doomed)).toBeUndefined() + expect(await reread.credentials.readRecord(slowKey)).toEqual({ kind: 'api-key', key: 'slow' }) + }) }) describe('contained update fan-out', () => { diff --git a/packages/credentials/credentials/README.i18n.yaml b/packages/credentials/credentials/README.i18n.yaml index 39ca7ff15c..8b474edf71 100644 --- a/packages/credentials/credentials/README.i18n.yaml +++ b/packages/credentials/credentials/README.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 packages/credentials/credentials/README.md -README.md: 0a8a10d48e9166ced2fd99613207060c06892a38 -README.zh.md: 9e621ccfa4f58b1cddfebe3bf0c9fe598d8dcf42 +README.md: d1b48aa724801a482517679790bba172fa60d013 +README.zh.md: a936b97353d7a66b7f0f7f9a760f4542bb2a9b30 diff --git a/packages/credentials/credentials/README.md b/packages/credentials/credentials/README.md index 0a8a10d48e..d1b48aa724 100644 --- a/packages/credentials/credentials/README.md +++ b/packages/credentials/credentials/README.md @@ -16,7 +16,7 @@ A `CredentialRef` answers *what is behind this environment-variable name*, layer A `CredentialKey` answers *what credential does this plugin hold for this id*. Nothing can layer here — an authorization grant has no environment to be read from — so presence of the record is the whole fact, and the empty-value rule does not apply: an `api-key` record carrying neither a key nor environment values states that its owner confirmed ambient authentication, which is configured. -The key is `/`, where `scope` is the **owning plugin's registered name**. The scope is the owner rather than the domain because a `grant` payload is written in its owner's format: two plugins serving the same provider name would otherwise read each other's payload, and a record left behind by an uninstalled plugin could not be told apart from a live one. The `/` also keeps the two grammars disjoint, so the key spaces can never collide. +The key is `/`, where `scope` is the **owning plugin's registered name**. The scope is the owner rather than the domain because a `grant` payload is written in its owner's format: two plugins serving the same provider name would otherwise read each other's payload, and a record left behind by an uninstalled plugin could not be told apart from a live one. The `/` also keeps the two grammars disjoint, so the key spaces can never collide. A consumer whose id arrives from somewhere else — a settings dict key, a library's own provider id — asks `isCredentialKeySegment` before building a key, because an id outside the grammar can never have stored a record and should read as "nothing stored" rather than throw on the address. ## Surface diff --git a/packages/credentials/credentials/README.zh.md b/packages/credentials/credentials/README.zh.md index 9e621ccfa4..a936b97353 100644 --- a/packages/credentials/credentials/README.zh.md +++ b/packages/credentials/credentials/README.zh.md @@ -16,7 +16,7 @@ `CredentialKey` 回答的是*某个插件为某个 id 持有什么凭据*。这里没有任何东西可以分层——授权 grant 没有可供读取的环境——因此记录的存在与否就是全部事实，空值规则在此不适用：一条既无 key 也无环境值的 `api-key` 记录，陈述的是其拥有者确认该路由靠 ambient 认证，这属于已配置。 -键的形式是 `/`，其中 `scope` 是**拥有该记录的插件的注册名**。scope 取拥有者而非领域，是因为 `grant` 的 payload 以其拥有者的格式写就：否则服务同一个提供方名称的两个插件会互相读到对方的 payload，而已卸载插件留下的记录也无法与仍在使用的区分开。`/` 同时让两种文法互斥，两个键空间因此不可能相撞。 +键的形式是 `/`，其中 `scope` 是**拥有该记录的插件的注册名**。scope 取拥有者而非领域，是因为 `grant` 的 payload 以其拥有者的格式写就：否则服务同一个提供方名称的两个插件会互相读到对方的 payload，而已卸载插件留下的记录也无法与仍在使用的区分开。`/` 同时让两种文法互斥，两个键空间因此不可能相撞。id 来自别处的消费方——settings dict 键、某个库自己的 provider id——应先问 `isCredentialKeySegment` 再构造键：文法之外的 id 不可能存过记录，应读作「没有存储任何东西」，而不是在寻址上抛错。 ## 接口 diff --git a/packages/credentials/credentials/src/index.ts b/packages/credentials/credentials/src/index.ts index 3934f51754..087672f2fd 100644 --- a/packages/credentials/credentials/src/index.ts +++ b/packages/credentials/credentials/src/index.ts @@ -43,6 +43,19 @@ export function isCredentialRefName(value: string): boolean { return REF_PATTERN.test(value) } +/** + * Whether a raw string could be a {@link credentialKey} segment at all. + * Consumers whose addressing units come from somewhere else — a settings dict + * key, a library's own provider id — ask this before building a key, because a + * unit outside the grammar can never have stored a record and should read as + * "nothing stored" rather than as a thrown error. + * @param value - candidate segment. + * @returns true when {@link credentialKey} would accept it as either segment. + */ +export function isCredentialKeySegment(value: string): boolean { + return KEY_SEGMENT_PATTERN.test(value) +} + /** * Brand a scope and an id as a {@link CredentialKey}. * @param scope - the owning plugin's registered name, such as `llm-pi-ai`. diff --git a/packages/credentials/credentials/tests/credentials.spec.ts b/packages/credentials/credentials/tests/credentials.spec.ts index a4676a2ea8..2941494f48 100644 --- a/packages/credentials/credentials/tests/credentials.spec.ts +++ b/packages/credentials/credentials/tests/credentials.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { Context } from '@deepseek-ai/cordis' -import { credentialRef } from '../src/index.ts' +import { credentialRef, isCredentialKeySegment } from '../src/index.ts' import type { CredentialRef } from '../src/index.ts' import { MemoryCredentials } from './memory.ts' @@ -26,6 +26,19 @@ describe('credentialRef', () => { }) }) +describe('isCredentialKeySegment', () => { + it('answers whether credentialKey would accept the segment', () => { + for (const valid of ['llm-pi-ai', 'openai-codex', 'a', 'z9']) { + expect(isCredentialKeySegment(valid)).toBe(true) + } + // The shapes an arbitrary settings dict key can take that a record id + // cannot: a consumer asks here instead of learning it from a throw. + for (const invalid of ['', 'My_Proxy', 'z.ai', 'UPPER', '9leading', 'a/b']) { + expect(isCredentialKeySegment(invalid)).toBe(false) + } + }) +}) + describe('the credentials seam through the memory provider', () => { it('mounts as ctx.credentials and resolves a seeded reference with its source', async () => { const ctx = await boot({ DEEPSEEK_API_KEY: 'sk-seeded' }) diff --git a/packages/extensions/tool-cordis/src/api-catalog.ts b/packages/extensions/tool-cordis/src/api-catalog.ts index 78b9368986..e520a9217e 100644 --- a/packages/extensions/tool-cordis/src/api-catalog.ts +++ b/packages/extensions/tool-cordis/src/api-catalog.ts @@ -490,8 +490,8 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ signature: 'async begin(request: AuthorizationRequest): Promise', description: 'Run one attempt to authorize a key, and report how it ended.\n\nOne attempt per key at a time. A second caller is refused rather than joined: the two would be prompting different humans through the same flow, and the second would answer questions the first was asked.', parameters: [{ name: 'request', description: 'the key, the method, the surface, and the cancel signal.' }], - returns: '`authorized` once the flow\'s record is committed and observed, or `cancelled` when the human or the caller withdrew.', - throws: ['{AuthorizationError} code `NO_FLOW` when nothing claims the key, `UNKNOWN_METHOD` when the named method is not one the flow offers, `ALREADY_IN_FLIGHT` when an attempt is already running for the key, or `NOT_COMMITTED` when the flow resolved without leaving a record behind.'], + returns: '`authorized` once the flow\'s record is committed during this attempt and observed, or `cancelled` when the human declined or the caller withdrew.', + throws: ['{AuthorizationError} code `NO_FLOW` when nothing claims the key, `UNKNOWN_METHOD` when the named method is not one the flow offers, `ALREADY_IN_FLIGHT` when an attempt is already running for the key, or `NOT_COMMITTED` when the flow resolved without committing a record during the attempt.'], }, ], }, diff --git a/packages/llm/llm-pi-ai/README.i18n.yaml b/packages/llm/llm-pi-ai/README.i18n.yaml index 19db6eb0fb..611cc52db3 100644 --- a/packages/llm/llm-pi-ai/README.i18n.yaml +++ b/packages/llm/llm-pi-ai/README.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 packages/llm/llm-pi-ai/README.md -README.md: 45b4815cb35e8709a6dc6cf4c50b778d2c2113f9 -README.zh.md: a1e952b70d9361e945b37ed146035fa274185003 +README.md: 29b12996208c44328a4bc1fdd237018564f1014d +README.zh.md: 618e85bb54eec8739bbece1099404801e48522ec diff --git a/packages/llm/llm-pi-ai/README.md b/packages/llm/llm-pi-ai/README.md index 45b4815cb3..29b1299620 100644 --- a/packages/llm/llm-pi-ai/README.md +++ b/packages/llm/llm-pi-ai/README.md @@ -140,7 +140,7 @@ Most listings disclose an id and nothing else; `context_window`/`context_length` Each resolution produces one **immutable** snapshot — the profiles plus a `createModels()` collection holding the `Provider` each route built — and every operation captures a whole snapshot before its first `await`. A configuration change builds a *new* collection rather than mutating the one in use: `Models.streamSimple()` resolves its provider lazily, when the stream is first consumed, which is after the credential await, so a mutated collection would let a request that started under one configuration finish under another or fail on a provider that no longer exists. This is what makes the seam's per-step call freeze (`llm.prepareCall()`) hold end to end — switching models mid-reply takes effect on the next step, never inside the one in flight. Requests reach their provider through `Models.streamSimple()`. A catalog route that keeps its catalog protocol **reuses** the installed provider with its model list replaced, because that provider owns API implementations this package cannot reconstruct — Bedrock loads its Smithy module through a separate entry point — so rebuilding it from parts would silently narrow which providers work. Every other route is built by `createProvider()` over the protocol table behind `supportedProtocols()`, whose entries are the same factories pi-ai's own provider factories use. -Credentials never enter that collection. The harness resolves a route's key through its own seam before the request reaches pi-ai and passes it as the request's `apiKey` option, which pi-ai treats as the highest-priority auth override; `Models` therefore holds no credential store, and the harness keeps its fail-loud reference semantics. A route naming no credential resolves as configured-but-keyless and leaves the requirement to the protocol, which is where it actually lives. +A route's `apiKeyEnv` key still resolves through the harness seam before the request reaches pi-ai and rides as the request's `apiKey` option, which pi-ai treats as the highest-priority auth override — the fail-loud reference semantics stay the harness's. Beneath that override the collection carries this plugin's credential store and ambient auth context: a stored sign-in (an OAuth grant, or a key typed into pi-ai's own login prompt) authenticates its route through them and refreshes itself under the store's cross-process lock. The store addresses records as `llm-pi-ai/`, and a hand-declared route key outside that record grammar (uppercase, dots, underscores) reads as holding nothing stored rather than as an addressing error; such a route cannot be signed into — a record write for it refuses with `LlmError('UNSTORABLE_PROVIDER_ID')` — and authenticates through `apiKeyEnv` or ambient provider settings. A route naming no credential at all resolves as configured-but-keyless and leaves the requirement to the protocol, which is where it actually lives. The selected model descriptor supplies the protocol implementation. This includes native API differences such as OpenAI models whose descriptor uses the Responses API rather than Chat Completions; the harness adapter does not hardcode endpoint selection by model name. diff --git a/packages/llm/llm-pi-ai/README.zh.md b/packages/llm/llm-pi-ai/README.zh.md index a1e952b70d..618e85bb54 100644 --- a/packages/llm/llm-pi-ai/README.zh.md +++ b/packages/llm/llm-pi-ai/README.zh.md @@ -141,7 +141,7 @@ pi-ai 依据提供方 id 与 baseURL 决定每个请求的形状：系统提示 每次解析产出一份**不可变**快照——profiles 加上一个持有各路由所建 `Provider` 的 `createModels()` 集合——每个操作都在自己第一个 `await` 之前整体捕获一份快照。配置变化会构造**新**集合，而不是改动正在被使用的那个：`Models.streamSimple()` 是惰性的，它在流首次被消费时才解析 provider，而那已在 credential await 之后，因此改动共享集合会让一个在旧配置下开始的请求在新配置下结束，或者撞上一个已不存在的 provider。这正是 seam 的每步调用冻结（`llm.prepareCall()`）能贯通到底的原因——回复途中切换模型会在下一步生效，绝不会影响在途的那一步。请求经 `Models.streamSimple()` 抵达提供方。保持 catalog 协议不变的 catalog 路由会**复用**已安装提供方，只替换其模型列表，因为该提供方持有本包无法重建的 API 实现——Bedrock 经由独立入口加载其 Smithy 模块——从零件重建会静默收窄可用提供方的范围。其余路由都由 `createProvider()` 基于 `supportedProtocols()` 背后的协议表构造，表中条目正是 pi-ai 自己的提供方工厂所用的同一批 factory。 -凭据绝不进入该集合。harness 在请求抵达 pi-ai 之前经自身 seam 解析路由密钥，并作为请求的 `apiKey` 选项传入，而 pi-ai 将其视为优先级最高的 auth 覆盖；因此 `Models` 不持有任何凭据存储，harness 也保住了自己明确失败的引用语义。没有点名任何凭据的路由会解析为「已配置但无密钥」，把该要求留给协议——那才是它真正所在的位置。 +路由的 `apiKeyEnv` 密钥仍在请求抵达 pi-ai 之前经 harness 自身 seam 解析，并作为请求的 `apiKey` 选项传入——pi-ai 将其视为优先级最高的 auth 覆盖，harness 因此保住自己明确失败的引用语义。在该覆盖之下，集合携带本插件的凭据存储与 ambient auth context：已存储的登录（OAuth grant，或在 pi-ai 自己的登录提示里键入的密钥）经由它们为路由完成认证，并在存储的跨进程锁下自行刷新。存储以 `llm-pi-ai/` 为记录地址；手写路由键若落在记录文法之外（大写、点、下划线），读取时视为「没有存储任何东西」而不是寻址错误——这样的路由无法登录（对它的记录写入会以 `LlmError('UNSTORABLE_PROVIDER_ID')` 拒绝），只能经 `apiKeyEnv` 或提供方 ambient 设置认证。完全没有点名任何凭据的路由会解析为「已配置但无密钥」，把该要求留给协议——那才是它真正所在的位置。 所选模型 descriptor 提供协议实现。这包括原生 API 差异，例如 descriptor 使用 Responses API 而非 Chat Completions 的 OpenAI 模型；harness 适配器不会按模型名称硬编码端点选择。 diff --git a/packages/llm/llm-pi-ai/src/auth.ts b/packages/llm/llm-pi-ai/src/auth.ts index 8b718cdd20..f729a77f85 100644 --- a/packages/llm/llm-pi-ai/src/auth.ts +++ b/packages/llm/llm-pi-ai/src/auth.ts @@ -14,7 +14,7 @@ import { resolve as resolvePath } from 'node:path' import type { AuthContext, Credential, CredentialInfo, CredentialStore } from '@earendil-works/pi-ai' import type { Context } from '@deepseek-ai/cordis' import { - credentialKey, credentialKeyId, credentialKeyScope, credentialRef, isCredentialRefName, + credentialKey, credentialKeyId, credentialKeyScope, credentialRef, isCredentialKeySegment, isCredentialRefName, } from '@deepseek-ai/dsh-credentials' import type { CredentialKey, CredentialProvider, CredentialRecord } from '@deepseek-ai/dsh-credentials' import { launchEnvironmentOf } from '@deepseek-ai/dsh-launch-environment' @@ -104,6 +104,13 @@ function writableStore(ctx: Context): CredentialProvider { * to cover a network round trip rather than a file rename — which is why the * record write path takes a wait limit of its own rather than the short one a * local write would need. + * + * pi-ai asks this store about every provider in the collection, hand-declared + * routes included, and a route key is an arbitrary settings dict key while a + * record id is not. An id outside the record grammar can never have stored a + * record, so reads answer "nothing stored" and a delete has nothing to remove; + * only `modify` refuses it, because a write that cannot land must not report + * that it did. * @param ctx - the plugin context carrying the optional `ctx.credentials`. * @returns the store to hand `createModels()`. */ @@ -112,6 +119,7 @@ export function credentialStoreFrom(ctx: Context): CredentialStore { async read(providerId) { const credentials = ctx.get('credentials') if (credentials === undefined) return undefined + if (!isCredentialKeySegment(providerId)) return undefined return toPiCredential(await credentials.readRecord(recordKeyFor(providerId))) }, async list(): Promise { @@ -129,6 +137,14 @@ export function credentialStoreFrom(ctx: Context): CredentialStore { return mine }, async modify(providerId, mutate) { + if (!isCredentialKeySegment(providerId)) { + throw new LlmError( + `llm-pi-ai: provider id "${providerId}" cannot address a stored credential record (a record id is a` + + ' lowercase hyphenated identifier); authenticate this route through apiKeyEnv instead of a stored' + + ' credential', + 'UNSTORABLE_PROVIDER_ID', + ) + } const stored = await writableStore(ctx).modifyRecord(recordKeyFor(providerId), async (current) => { const next = await mutate(toPiCredential(current)) return next === undefined ? undefined : toRecord(next) @@ -139,6 +155,7 @@ export function credentialStoreFrom(ctx: Context): CredentialStore { // store contract is promise-returning, and a synchronous throw would // escape the `ModelsError` wrapper every other storage failure gets. async delete(providerId) { + if (!isCredentialKeySegment(providerId)) return await writableStore(ctx).deleteRecord(recordKeyFor(providerId)) }, } diff --git a/packages/llm/llm-pi-ai/src/login.ts b/packages/llm/llm-pi-ai/src/login.ts index c71d68e3b5..578489afb4 100644 --- a/packages/llm/llm-pi-ai/src/login.ts +++ b/packages/llm/llm-pi-ai/src/login.ts @@ -11,6 +11,7 @@ import { createModels } from '@earendil-works/pi-ai' import type { AuthEvent, AuthPrompt, AuthType, Provider } from '@earendil-works/pi-ai' import type { Context } from '@deepseek-ai/cordis' import type { AuthorizationMethod, AuthorizationPrompt, AuthorizationSession } from '@deepseek-ai/dsh-authorization' +import { isCredentialKeySegment } from '@deepseek-ai/dsh-credentials' import { catalogProvider, catalogProviderIds } from './catalog.ts' import { recordKeyFor } from './auth.ts' import type { PiAiAuthInjection } from './adapter.ts' @@ -124,6 +125,16 @@ export function registerPiAiFlows(ctx: Context, auth: PiAiAuthInjection): void { installed provider ships a login, so nothing is skipped today; the guard is what keeps that from becoming a crash if either stops being true. */ if (provider === undefined || first === undefined) continue + /* v8 ignore next 7 -- every installed catalog id today is a lowercase + hyphenated identifier; the guard keeps a future upstream id outside the + record grammar (dotted or uppercase, as vendor ids elsewhere already + are) from throwing in `recordKeyFor` and failing the whole mount. */ + if (!isCredentialKeySegment(providerId)) { + ctx.logger.warn( + 'llm-pi-ai: catalog provider "%s" cannot address a credential record; its sign-in is not offered', + providerId) + continue + } ctx.authorization.registerFlow({ key: recordKeyFor(providerId), label: provider.name, diff --git a/packages/llm/llm-pi-ai/tests/auth.spec.ts b/packages/llm/llm-pi-ai/tests/auth.spec.ts index 55a20fbb66..fa6c635c86 100644 --- a/packages/llm/llm-pi-ai/tests/auth.spec.ts +++ b/packages/llm/llm-pi-ai/tests/auth.spec.ts @@ -116,6 +116,20 @@ describe('pi-ai credential store over harness records', () => { .rejects.toThrow(/mounts no credentials service/) await expect(store.delete('openai-codex')).rejects.toThrow(/mounts no credentials service/) }) + + it('treats a provider id outside the record grammar as holding nothing', async () => { + const store = credentialStoreFrom(await stored()) + + // A hand-declared route key is an arbitrary settings dict key, and pi-ai + // reads it during auth resolution: the answer is "not signed in", never a + // thrown address error… + await expect(store.read('My_Proxy')).resolves.toBeUndefined() + // …nothing can ever be stored under it, so a logout has nothing to remove… + await expect(store.delete('My_Proxy')).resolves.toBeUndefined() + // …while a write that cannot land must refuse rather than report success. + await expect(store.modify('My_Proxy', () => Promise.resolve({ type: 'api_key', key: 'k' }))) + .rejects.toThrow(/cannot address a stored credential record/) + }) }) describe('pi-ai ambient auth context', () => {