Add typed config endpoints with dual schema generation#9197
Conversation
wpfleger96
force-pushed
the
wpfleger/structured-settings
branch
from
ea40228 to
027bc19
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5531f34f25
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| #[serde(rename = "CLAUDE_THINKING_BUDGET")] | ||
| pub claude_thinking_budget: Option<i32>, |
There was a problem hiding this comment.
GooseConfigSchema exposes CLAUDE_THINKING_BUDGET but not ANTHROPIC_THINKING_BUDGET, even though Anthropic request construction still reads ANTHROPIC_THINKING_BUDGET first (thinking_budget_tokens in providers/formats/anthropic.rs). For users who already set the Anthropic key, GET /config/typed cannot surface the effective value and PATCH /config/typed cannot modify it, so UI edits to thinking budget can appear to succeed while runtime behavior remains unchanged.
Useful? React with 👍 / 👎.
|
|
||
| impl GooseConfigUpdate { | ||
| pub fn apply_to_config(&self, config: &Config) -> Result<(), ConfigError> { | ||
| self.config.apply_to_config(config)?; |
There was a problem hiding this comment.
GooseConfigUpdate::apply_to_config persists non-secret fields before attempting secret writes. If set_secret_values fails (for example, keyring access errors that do not fall back), the PATCH returns an error after regular config keys were already committed, leaving a partially applied update. That failure mode makes retries unsafe and can desynchronize client state from what the request outcome implies.
Useful? React with 👍 / 👎.
Single GooseConfigSchema struct derives both utoipa::ToSchema (for OpenAPI → TypeScript type safety) and schemars::JsonSchema (for standalone config.schema.json consumed by downstream tools). Adds GET/PATCH /config/typed endpoints alongside existing per-key endpoints. GooseConfigUpdate struct includes secret fields routed through the keyring. Existing UI and endpoints are unchanged. Signed-off-by: Will Pfleger <wpfleger@block.xyz>
Flatten GooseConfigUpdate to embed GooseConfigSchema via #[serde(flatten)], eliminating ~500 lines of duplication and reducing edit sites from 6 to 3 per new config key. Fix GOOSE_DISABLE_KEYRING type (String → bool), add 7 missing provider secret keys, make ALL_KEYS test bidirectional, add config roundtrip test, and log warnings on serialization failures. Signed-off-by: Will Pfleger <wpfleger@block.xyz>
Missed this file in the previous commit — CI uses @hey-api/openapi-ts 0.93.1 (no lockfile, resolves ^0.93.0 to latest) while local has 0.93.0. The CreateClientConfig return type now includes | Promise<...>. Signed-off-by: Will Pfleger <wpfleger@block.xyz>
Round-2 crossfire review (3 Claude agents + Codex + Gemini) surfaced a missing config key, a CLI display bug, incomplete test coverage, and the root cause of the CI codegen mismatch. This commit addresses all actionable findings and eliminates the manual TS codegen patching. Key changes: - Add GOOSE_DISABLE_TOOL_CALL_SUMMARY to GooseConfigSchema - Add NANOGPT_API_KEY and LITELLM_CUSTOM_HEADERS to GooseConfigUpdate - Fix configure.rs keyring status display for boolean GOOSE_DISABLE_KEYRING - Add exhaustive from_config/apply_to_config coverage test - Add CI check-config-schema job + Justfile targets - Fix generate-config-schema error message (underscores → hyphens) - Pin @hey-api/openapi-ts to 0.93.1, matching the lockfile - Update PATCH endpoint docs (sparse semantics, nested replacement, env caution) Signed-off-by: Will Pfleger <wpfleger@block.xyz>
The HTTP layer for GET/PATCH /config/typed had zero test coverage — Rust unit tests validate schema structs but not request deserialization, sparse patch semantics, or secret routing over the wire. These 15 tests hit a real goosed server and cover type diversity (string, bool, int, float, enum, array), sparse patch invariants (omit preserves, null preserves, empty body no-ops), nested object whole-value replacement, and secret field non-leakage in responses. Signed-off-by: Will Pfleger <wpfleger@block.xyz>
PR #8977 introduced structured per-provider config (`providers:` block + `active_provider` key). Wire these into GooseConfigSchema so they flow through the typed API endpoints and both JSON schemas. GOOSE_PROVIDER/GOOSE_MODEL reads now use the resolution-chain accessors (env → active_provider → legacy flat key), and writes route through set_active_provider to keep the providers block canonical. Signed-off-by: Will Pfleger <wpfleger@block.xyz>
wpfleger96
force-pushed
the
wpfleger/structured-settings
branch
from
5531f34 to
f055a64
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f055a64418
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .or_else(|| config.get_goose_model().ok()) | ||
| .unwrap_or_default(); | ||
| crate::config::set_active_provider(config, provider, &model)?; |
There was a problem hiding this comment.
Avoid falling back to config.get_goose_model() when GOOSE_PROVIDER is patched without GOOSE_MODEL: that getter returns the currently active provider’s model, so switching providers can overwrite the target provider’s saved model (for example, switching from OpenAI to Anthropic rewrites Anthropic’s configured model to OpenAI’s model). This mutates provider state during a simple provider switch and can break subsequent runs that rely on per-provider model persistence.
Useful? React with 👍 / 👎.
| } else if let Some(ref model) = self.goose_model { | ||
| config.set_goose_model(model)?; |
There was a problem hiding this comment.
Handle GOOSE_MODEL updates when no active provider exists, because config.set_goose_model(model) is a no-op in that state. A PATCH containing only GOOSE_MODEL can return success without writing anything, which makes typed config updates silently fail on fresh or partially configured installs.
Useful? React with 👍 / 👎.
1 participant
This PR adds
GooseConfigSchemaandGooseConfigUpdatetyped structs that derive bothutoipa::ToSchemaandschemars::JsonSchema, enabling full-stack type safety through the OpenAPI→TypeScript pipeline while also publishing a standaloneconfig.schema.jsonfor downstream consumers.Goose had no typed config struct — the runtime config is a dynamic
HashMap<String, Value>. Discussed in #8630 with @DOsinga, who suggested aligning with thestructured-settingsapproach so config types flow through the OpenAPI spec into generated TypeScript, giving the UI real type safety. This combines that with a standalone JSON Schema for external tooling.GooseConfigSchema(124 fields includingactive_providerandproviders, no secrets) withfrom_config()andapply_to_config()adapters bridging the typed struct to the runtime config storeGooseConfigUpdateusing#[serde(flatten)]to embedGooseConfigSchemaplus 15 provider secret fields routed to the system keyring — no field duplication between structsGET /config/typedandPATCH /config/typedendpoints registered in the OpenAPI spec; existing per-key endpoints are unchangedgenerate-config-schemabinary (with--checkmode) producingcrates/goose/config.schema.json, withjust check-config-schemawired into CI alongside the OpenAPI and ACP checksprovidersblock andactive_providerkey:ProviderEntryderivesJsonSchema+ToSchema,GOOSE_PROVIDER/GOOSE_MODELreads use the resolution-chain accessors and writes route throughset_active_providerto keep the structuredproviders:block canonicalALL_KEYS↔ struct test, config roundtrip test (covering String, bool, i32, u64, f64, Vec, and enum types), exhaustivefrom_config/apply_to_configstring-key coverage test, and serialization failure loggingtyped_config.test.ts) hitting a real goosed server — covers type diversity (string, bool, int, float, enum, array), sparse PATCH semantics (omit/null preserve, empty body no-ops), nested object whole-value replacement, and secret field non-leakage in responsesconfigure.rskeyring status display for booleanGOOSE_DISABLE_KEYRINGvalues@hey-api/openapi-tsto0.93.1to match the lockfile and eliminate manual codegen patchingRelates to #8630