aaif-goose / aaif-goose/goose

Design: per-request nonce header for declarative providers

オープン
#11,308 コメント 7 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Rust
スター
54.2k
フォーク
6.2k
平均マージ
3日 4時間
マージ済み PR(30日)
240

説明

Records the design behind #11302, per @michaelneale's request. One open question for maintainers at the bottom: where config-field validation should live given several construction paths bypass from_json.

## Problem

A proxy or sidecar sitting in front of an OpenAI-compatible endpoint often needs a **per-request unique value** — idempotency keys, replay-window dedup, request correlation. Declarative (custom) providers have no way to supply one.

`DeclarativeProviderConfig.headers` is resolved once at provider init, so it can carry a constant but never a fresh value. Meanwhile every built-in provider can reach `ApiClient::with_request_builder` and install a per-request decorator; `from_declarative_config` is the only construction path that cannot. So this is a gap rather than a new capability.

## Design

One optional config field, `nonce_header: Option`, on `DeclarativeProviderConfig`.

- **The header name is supplied by config, never hardcoded.** No consumer's vocabulary enters goose.
- **Absent means no-op** — no decorator installed, no code path taken.
- Present installs a decorator inserting a fresh UUIDv4 (122 bits CSPRNG) under that name.
- The name is parsed **once at config load**, so a typo fails immediately with a message naming thw key rather than turning every subsequent request into an opaque header-parse error.

A general dynamic-header or templating mechanism was deliberately not attempted. The nonce is the concrete need; the decorator seam leaves generalization open if one ever turns up.

## Things the review surfaced, and how they resolved

These are the parts worth keeping, since they're where the design actually got decided.

**Decorators compose now.** `ApiClient::request_builder` was a single `Option` that
`with_request_builder` replaced. The registry path installs a second decorator —
`openai_def::from_custom_config` adds the session-id builder right after `from_declarative_config`
returns — which silently discarded the nonce, so CLI and Desktop would never have sent it. The slot is a `Vec` now and the setter appends; decorators run in installation order. Every existing caller is unaffected since each installs one.

Worth recording why the tests didn't catch it: they called `from_declarative_config` directly and asserted on the wire, so they exercised the builder but not the registry path that wraps it. A test through `openai_def::from_custom_config` asserting both headers arrive is the right regression pin, added in #11302.

**Reserved names are rejected.** `agent-session-id` is removed and rewritten by
`session_id_request_builder`, and `ApiClient::send_request` applies authentication *after* every decorator — so `authorization` and `proxy-authorization` would be overwritten too. All three now fail at config load with an error explaining why, rather than silently carrying nothing. A name colliding with a provider's own configured `ApiKey` auth header has the same problem but isn't statically knowable there; that one is documented on the field.

**Scope is the OpenAI declarative builder only.** `from_json` dispatches to the anthropic and ollama builders too, and they don't install the decorator. An earlier revision rejected the field for those engines in `from_json` — but the registry doesn't go through `from_json`: `load_custom_providers`deserializes and the registry calls engine-specific `from_custom_config` directly, and an openai-engine config named `huggingface` routes to `HuggingFaceProvider::from_custom_config`, which builds its own `ApiClient`. So that guard was dead code on the path that matters and has been removed in favour of documenting the limitation on the field itself.

**That last one is the open design question**, and it's the reason this issue is worth having:
**where should validation of provider-config fields live, given several construction paths bypass `from_json`?** Options I can see are a shared validation step in the registry dispatch in `load_custom_providers`, installing the decorator in each specialised constructor, or keeping it builder-scoped and documented. Happy to implement whichever you prefer — I've guessed wrong twice, so I'd rather have the maintainers' answer than a third guess.

## Semver note

Adding a field to `DeclarativeProviderConfig` breaks struct-literal construction for out-of-tree callers — deserialization is fine thanks to `#[serde(default)]`. Seven in-repo sites needed updating, three of them in `goose/src/config/declarative_providers.rs` and two of those non-test. If `#[non_exhaustive]` or a builder is preferred, glad to follow.

Separately, `ApiClient::with_request_builder` changed from replace-on-set to append. Same signature, different behaviour for any out-of-tree caller that installed twice expecting replacement — unlikely to exist, but semver-relevant if `goose-providers` is published.

## Consumer

Mesh-LLM's local sidecar, so the *client* rather than the intermediary contributes the per-request nonce — Mesh-LLM/mesh-llm#1233. The companion Mesh-LLM PR is also mine and sets the key in the `mesh.json` that `mesh-llm goose` generates.

Implementation: #11302.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

The issue describes adding a `nonce_header` field to `DeclarativeProviderConfig`. Start by examining the `goose/src/config/declarative_providers.rs` file to understand the config structure. Look at `ApiClient::with_request_builder` and the request builder decorator pattern. Review the implementation in PR #11302 for context. The work involves updating the config, adding validation for reserved header names, and ensuring the decorator composes correctly in the registry path. Testing should include the `openai_def::from_custom_config` path to verify headers are sent.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
rust
領域
backend-api-design
issue の種類
機能追加
難易度
4/5
見積もり時間
3〜5日
活発さ
活発
明瞭さ
明確に書かれている
初心者へのやさしさ
40/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。