Skip to content

Commit 8c9aecd

Browse files
Pyinerclaude
andcommitted
Merge #TASK-933: custom agent model merge semantics
absent=preserve, explicit empty=reset to provider default. Gateway payload Option<String> + store resolver, CLI omit-when-absent + --clear-model, mobile always-present model/effort/tier. Reviewed + independently verified (gateway 10, CLI 28, mobile 57 tests; live e2e preserves reviewer model). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 parents 9d051fa + b1ab4d3 commit 8c9aecd

18 files changed

Lines changed: 482 additions & 94 deletions

File tree

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
# Custom Agent Model Merge Contract
2+
3+
## Problem
4+
5+
`PUT /api/custom-agents/{agent_id}` and `POST /api/custom-agents` currently
6+
treat omitted model fields as empty strings because the request payload uses
7+
`String` fields with `#[serde(default)]`. That collapses two different client
8+
intents:
9+
10+
- field absent: keep the stored value when updating an existing agent
11+
- field present with `""`: replace the stored value with provider default
12+
13+
The same endpoint already has merge semantics for adjacent optional fields such
14+
as provider environment, default workspace, and avatar data. Model selection,
15+
reasoning effort, and service tier should use the same contract.
16+
17+
## Contract
18+
19+
For custom-agent upsert requests:
20+
21+
| Request state | Existing agent update | New agent create |
22+
| --- | --- | --- |
23+
| `model` absent | keep stored `model` | store `""` |
24+
| `model: "x"` | store `"x"` after trimming | store `"x"` after trimming |
25+
| `model: ""` | store `""` (provider default) | store `""` |
26+
27+
`model_reasoning_effort` and `model_service_tier` follow the same rules. The
28+
camelCase aliases `modelReasoningEffort` and `modelServiceTier` remain accepted.
29+
30+
Stored `CustomAgentProfile` fields stay as `String`; the tri-state behavior is
31+
only a request-layer concern.
32+
33+
## Design
34+
35+
### Gateway
36+
37+
- Change `CustomAgentUpsertPayload.model`, `model_reasoning_effort`, and
38+
`model_service_tier` from `String` to `Option<String>`.
39+
- Change `UpsertCustomAgentRequest` the same way.
40+
- In `CustomAgentStore::upsert_agent`, resolve each field with one helper:
41+
- `Some(value)` trims and stores the value, including `""`.
42+
- `None` keeps the existing profile value.
43+
- `None` on create stores `""`.
44+
- Keep built-in-agent modification rejection before persisting changes.
45+
- Keep provider environment, auth, workspace, avatar, native config, and prompt
46+
behavior unchanged.
47+
48+
### CLI
49+
50+
- Stop serializing `model`, `model_reasoning_effort`, and
51+
`model_service_tier` when the caller omitted the matching option. This lets
52+
the gateway preserve existing values on update.
53+
- Keep create behavior unchanged: omitted keys create empty stored values, which
54+
means provider defaults.
55+
- Add `--clear-model` to `garyx agent update` and `garyx agent upsert`; it sends
56+
`model: ""` explicitly and conflicts with `--model`.
57+
- Keep existing explicit-empty support for reasoning effort and service tier,
58+
because passing `--model-reasoning-effort ""` or `--model-service-tier ""`
59+
still sends an explicit replacement value.
60+
- Update help text so omission on update/upsert is described as preserve, while
61+
omission on create remains provider default.
62+
63+
### Mobile
64+
65+
`GaryxCustomAgentRequest` already has optional request fields, so its shape does
66+
not change. The mobile create/edit flows must send model and reasoning effort
67+
values whenever the user is saving those controls:
68+
69+
- create: send the trimmed `model` and `model_reasoning_effort` strings even
70+
when they are empty
71+
- update: send the next model, next reasoning effort, and preserved service tier
72+
strings even when they are empty
73+
74+
This keeps "reset to Provider default" working under the new absent-as-preserve
75+
contract.
76+
77+
Desktop already sends model values from required controls, so no desktop change
78+
is planned.
79+
80+
## Tradeoffs
81+
82+
- Using `Option<String>` at the request boundary is more explicit than adding a
83+
separate clear flag to the API. JSON already has a native absent-vs-present
84+
distinction, and existing optional fields use that pattern.
85+
- Keeping stored fields as `String` avoids a storage migration and preserves
86+
the existing provider-default representation.
87+
- The CLI grows only the missing `--clear-model` affordance. Empty string
88+
values remain supported for the other two fields to avoid a broader CLI flag
89+
expansion.
90+
91+
## Rollout/compatibility
92+
93+
The new gateway treats absent model fields as preserve. Older iOS builds send
94+
`nil` for empty model controls, so those builds temporarily lose the ability to
95+
reset an agent model or reasoning effort to Provider default after the gateway
96+
change ships. They can still preserve existing settings.
97+
98+
Ship the gateway and iOS app changes together so mobile regains explicit
99+
empty-string clears. Desktop sends concrete model values from required controls,
100+
and the CLI change adds an explicit clear path, so desktop and CLI are not
101+
affected by this compatibility window.
102+
103+
## Validation
104+
105+
- Gateway store tests:
106+
- update without model fields preserves existing model settings
107+
- update with `Some("")` clears model settings
108+
- update with `Some("x")` replaces model settings
109+
- create without model fields stores empty strings
110+
- Gateway build coverage: `cargo build`
111+
- Focused gateway tests: `cargo test -p garyx-gateway custom_agents`
112+
- CLI tests:
113+
- update without model options omits the three keys
114+
- update with `--clear-model` sends `model: ""`
115+
- CLI end-to-end check against a real persisted agent: dump current agent,
116+
update without `--model`, and confirm the stored model is preserved.
117+
- Mobile core test: `GaryxCustomAgentRequest(model: "")` encodes a present
118+
`"model": ""` key.
119+
- iOS app target build: run `xcodebuild` for the app target because SwiftPM
120+
tests do not cover app-target wiring.

garyx-gateway/src/agent_identity.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,9 @@ mod tests {
236236
agent_id: "reviewer".to_owned(),
237237
display_name: "Reviewer".to_owned(),
238238
provider_type: ProviderType::CodexAppServer,
239-
model: "gpt-5".to_owned(),
240-
model_reasoning_effort: "high".to_owned(),
241-
model_service_tier: "priority".to_owned(),
239+
model: Some("gpt-5".to_owned()),
240+
model_reasoning_effort: Some("high".to_owned()),
241+
model_service_tier: Some("priority".to_owned()),
242242
provider_env: None,
243243
auth_source: None,
244244
base_url: None,

garyx-gateway/src/agent_team_provider/tests.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ async fn test_dispatcher() -> GatewaySubAgentDispatcher {
2121
agent_id: "planner".to_owned(),
2222
display_name: "Planner".to_owned(),
2323
provider_type: ProviderType::ClaudeCode,
24-
model: String::new(),
25-
model_reasoning_effort: String::new(),
26-
model_service_tier: String::new(),
24+
model: Some(String::new()),
25+
model_reasoning_effort: Some(String::new()),
26+
model_service_tier: Some(String::new()),
2727
provider_env: None,
2828
auth_source: None,
2929
base_url: None,
@@ -41,9 +41,9 @@ async fn test_dispatcher() -> GatewaySubAgentDispatcher {
4141
agent_id: "reviewer".to_owned(),
4242
display_name: "Reviewer".to_owned(),
4343
provider_type: ProviderType::ClaudeCode,
44-
model: String::new(),
45-
model_reasoning_effort: String::new(),
46-
model_service_tier: String::new(),
44+
model: Some(String::new()),
45+
model_reasoning_effort: Some(String::new()),
46+
model_service_tier: Some(String::new()),
4747
provider_env: None,
4848
auth_source: None,
4949
base_url: None,

garyx-gateway/src/api.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,11 @@ pub struct CustomAgentUpsertPayload {
304304
pub display_name: String,
305305
pub provider_type: garyx_models::ProviderType,
306306
#[serde(default)]
307-
pub model: String,
307+
pub model: Option<String>,
308308
#[serde(default, alias = "modelReasoningEffort")]
309-
pub model_reasoning_effort: String,
309+
pub model_reasoning_effort: Option<String>,
310310
#[serde(default, alias = "modelServiceTier")]
311-
pub model_service_tier: String,
311+
pub model_service_tier: Option<String>,
312312
#[serde(default, alias = "env", alias = "providerEnv")]
313313
pub provider_env: Option<HashMap<String, String>>,
314314
#[serde(default, alias = "authSource")]

garyx-gateway/src/application/chat/prepare/tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ async fn prepare_chat_request_resolves_provider_and_system_prompt_from_thread_ag
166166
agent_id: "spec-review".to_owned(),
167167
display_name: "Spec Review".to_owned(),
168168
provider_type: ProviderType::CodexAppServer,
169-
model: "gpt-5-codex".to_owned(),
170-
model_reasoning_effort: "xhigh".to_owned(),
171-
model_service_tier: String::new(),
169+
model: Some("gpt-5-codex".to_owned()),
170+
model_reasoning_effort: Some("xhigh".to_owned()),
171+
model_service_tier: Some(String::new()),
172172
provider_env: None,
173173
auth_source: None,
174174
base_url: None,

garyx-gateway/src/custom_agents.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ pub struct UpsertCustomAgentRequest {
1616
pub display_name: String,
1717
pub provider_type: ProviderType,
1818
#[serde(default)]
19-
pub model: String,
19+
pub model: Option<String>,
2020
#[serde(default, alias = "modelReasoningEffort")]
21-
pub model_reasoning_effort: String,
21+
pub model_reasoning_effort: Option<String>,
2222
#[serde(default, alias = "modelServiceTier")]
23-
pub model_service_tier: String,
23+
pub model_service_tier: Option<String>,
2424
#[serde(default, alias = "env", alias = "providerEnv")]
2525
pub provider_env: Option<HashMap<String, String>>,
2626
#[serde(default, alias = "authSource")]
@@ -134,9 +134,13 @@ impl CustomAgentStore {
134134
) -> Result<CustomAgentProfile, String> {
135135
let agent_id = request.agent_id.trim();
136136
let display_name = request.display_name.trim();
137-
let model = request.model.trim();
138-
let model_reasoning_effort = request.model_reasoning_effort.trim();
139-
let model_service_tier = request.model_service_tier.trim();
137+
let requested_model = request.model.map(|value| value.trim().to_owned());
138+
let requested_model_reasoning_effort = request
139+
.model_reasoning_effort
140+
.map(|value| value.trim().to_owned());
141+
let requested_model_service_tier = request
142+
.model_service_tier
143+
.map(|value| value.trim().to_owned());
140144
let provider_env = request.provider_env.map(|values| {
141145
values
142146
.into_iter()
@@ -195,6 +199,15 @@ impl CustomAgentStore {
195199
.and_then(|existing| existing.avatar_data_url.clone()),
196200
};
197201
let existing = inner.get(agent_id);
202+
let model = requested_model
203+
.or_else(|| existing.map(|profile| profile.model.clone()))
204+
.unwrap_or_default();
205+
let model_reasoning_effort = requested_model_reasoning_effort
206+
.or_else(|| existing.map(|profile| profile.model_reasoning_effort.clone()))
207+
.unwrap_or_default();
208+
let model_service_tier = requested_model_service_tier
209+
.or_else(|| existing.map(|profile| profile.model_service_tier.clone()))
210+
.unwrap_or_default();
198211
let provider_env = provider_env
199212
.or_else(|| existing.map(|profile| profile.provider_env.clone()))
200213
.unwrap_or_default();
@@ -219,9 +232,9 @@ impl CustomAgentStore {
219232
agent_id: agent_id.to_owned(),
220233
display_name: display_name.to_owned(),
221234
provider_type: request.provider_type,
222-
model: model.to_owned(),
223-
model_reasoning_effort: model_reasoning_effort.to_owned(),
224-
model_service_tier: model_service_tier.to_owned(),
235+
model,
236+
model_reasoning_effort,
237+
model_service_tier,
225238
provider_env,
226239
auth_source,
227240
base_url,

0 commit comments

Comments
 (0)