Skip to content

Commit 32347d1

Browse files
feat(desktop): per-persona and per-agent env var overrides (#594)
1 parent 13dc0df commit 32347d1

25 files changed

Lines changed: 1702 additions & 28 deletions

desktop/playwright.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export default defineConfig({
3939
"**/integration.spec.ts",
4040
"**/profile.spec.ts",
4141
"**/tokens.spec.ts",
42+
"**/persona-env-vars.spec.ts",
4243
],
4344
use: {
4445
...devices["Desktop Chrome"],

desktop/scripts/check-file-sizes.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ const overrides = new Map([
4040
["src/features/channels/ui/ChannelScreen.tsx", 550], // profile panel state + mutual exclusion wiring + ProfilePanelProvider context + agent typing classification
4141
["src/features/notifications/hooks.ts", 535], // notification settings + feed notification lifecycle + profile batch resolution + truncated-pubkey guard + badge state
4242
["src/features/messages/hooks.ts", 500], // message query/mutation hooks + optimistic updates
43-
["src/features/messages/ui/MessageComposer.tsx", 710], // media upload handlers (paste, drop, dialog) + channelId reset effect + edit mode (pre-fill, save, cancel, escape) + autofocus on mount/channel switch
43+
["src/features/messages/ui/MessageComposer.tsx", 710], // media upload handlers (paste, drop, dialog) + channelId reset effect + edit mode (pre-fill, save, cancel, escape) + composer autofocus (#572)
4444
["src/features/settings/ui/SettingsView.tsx", 600],
4545
["src/features/sidebar/ui/AppSidebar.tsx", 860], // channels + forums creation forms + Pulse nav
4646
["src/shared/api/relayClientSession.ts", 930], // durable websocket session manager with reconnect/replay/recovery state + sendTypingIndicator + fetchChannelHistoryBefore + subscribeToChannelLive (huddle TTS) + subscribeToHuddleEvents (huddle indicator) + disconnect() for workspace switch teardown + fetchEvents/subscribeLive/publishEvent for NIP-RS read state + publishUserStatus/subscribeToUserStatusUpdates (NIP-38)
@@ -50,9 +50,9 @@ const overrides = new Map([
5050
["src-tauri/src/commands/agents.rs", 881], // remote agent lifecycle routing (local + provider branches) + scope enforcement + persona pack metadata wiring + mcp_toolsets field + NIP-OA auth_tag in deploy payload
5151
["src-tauri/src/commands/messages.rs", 510], // feed multi-query + NIP-50 search + forum thread resolution + thread ref + reactions via REQ
5252
["src-tauri/src/nostr_convert.rs", 1150], // 12 Nostr event→model converters (channels, profiles, members, notes, search, agents, relay members) + rank_user_search_results helper for NIP-50 user search + 33 unit tests
53-
["src-tauri/src/managed_agents/runtime.rs", 990], // ... + respond-to gate env (SPROUT_ACP_RESPOND_TO[_ALLOWLIST]) + per-mode env builder + tests
54-
["src-tauri/src/managed_agents/types.rs", 700], // ManagedAgentRecord/Summary + Create/Update request structs + RespondTo enum + validate_respond_to_allowlist + tests
55-
["src-tauri/src/managed_agents/backend.rs", 530], // provider IPC, validation, discovery, binary resolution + tests
53+
["src-tauri/src/managed_agents/runtime.rs", 1110], // ... + respond-to gate env (SPROUT_ACP_RESPOND_TO[_ALLOWLIST]) + per-mode env builder + tests + persona/agent env_vars spawn merge (helper + tests now in env_vars.rs)
54+
["src-tauri/src/managed_agents/types.rs", 715], // ManagedAgentRecord/Summary + Create/Update request structs + RespondTo enum + validate_respond_to_allowlist + tests + persona/agent env_vars field
55+
["src-tauri/src/managed_agents/backend.rs", 700], // provider IPC, validation, discovery, binary resolution + tests + redact_secrets_with for user env values + env_secrets_from_request + redact_env_values_in (shared with model discovery)
5656
["src/features/huddle/HuddleContext.tsx", 650], // huddle lifecycle context + joinHuddle + connectAndSetupMedia shared helper + activeSpeakers/isReconnecting state + PTT (reusable AudioContext) + TTS subscription + mic level analyser (10fps throttle) + agent pubkey refresh
5757
["src/features/agents/hooks.ts", 540], // agent query/mutation surface now includes built-in persona library activation + useUpdateManagedAgentMutation
5858
["src/features/agents/ui/AgentsView.tsx", 880], // remote agent lifecycle controls + persona/team management + persona import-update dialog wiring + built-in catalog/library state orchestration

desktop/src-tauri/src/commands/agent_models.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub async fn get_agent_models(
2525
app: AppHandle,
2626
state: State<'_, AppState>,
2727
) -> Result<AgentModelsResponse, String> {
28-
let (resolved_acp, agent_command, agent_args, persisted_model) = {
28+
let (resolved_acp, agent_command, agent_args, persisted_model, merged_env) = {
2929
let _store_guard = state
3030
.managed_agents_store_lock
3131
.lock()
@@ -53,9 +53,22 @@ pub async fn get_agent_models(
5353
.map(|p| p.display().to_string())
5454
.unwrap_or_else(|| record.agent_command.clone());
5555

56-
(resolved, resolved_agent, args, record.model.clone())
56+
// Same env layering as runtime spawn: persona env < agent env.
57+
// Model discovery needs the user's credentials. Fail closed on
58+
// persona-resolution errors so a corrupt personas.json doesn't
59+
// produce a model list as if the persona had no credentials.
60+
let persona_env =
61+
crate::managed_agents::resolve_persona_env(&app, record.persona_id.as_deref())?;
62+
let env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars);
63+
64+
(resolved, resolved_agent, args, record.model.clone(), env)
5765
}; // store lock released — subprocess runs without holding the lock
5866

67+
// Clone the env map for redaction below — `merged_env` is moved
68+
// into the spawn_blocking closure and we still need the values to
69+
// scrub any user-supplied secrets that the child surfaces in stderr.
70+
let env_for_redaction = merged_env.clone();
71+
5972
// Use spawn_blocking because the desktop Tauri crate doesn't enable
6073
// tokio's `process` feature. std::process::Command is synchronous
6174
// but fine for a short-lived subprocess (~2-5s).
@@ -74,8 +87,12 @@ pub async fn get_agent_models(
7487
.env(
7588
"GOOSE_MODE",
7689
std::env::var("GOOSE_MODE").unwrap_or_else(|_| "auto".into()),
77-
)
78-
.stdout(std::process::Stdio::piped())
90+
);
91+
// User env layering — written LAST so it overrides any Sprout-set env above.
92+
for (k, v) in &merged_env {
93+
cmd.env(k, v);
94+
}
95+
cmd.stdout(std::process::Stdio::piped())
7996
.stderr(std::process::Stdio::piped())
8097
.output()
8198
.map_err(|e| format!("failed to spawn sprout-acp models: {e}"))
@@ -86,8 +103,13 @@ pub async fn get_agent_models(
86103

87104
if !output.status.success() {
88105
let stderr = String::from_utf8_lossy(&output.stderr);
106+
// Scrub any user-supplied env values before surfacing stderr to
107+
// the frontend — persona/agent env_vars may carry API keys that
108+
// a failing child process echoed back.
109+
let stderr_redacted =
110+
crate::managed_agents::redact_env_values_in(stderr.as_ref(), &env_for_redaction);
89111
return Err(format!(
90-
"sprout-acp models failed (exit {}): {stderr}",
112+
"sprout-acp models failed (exit {}): {stderr_redacted}",
91113
output.status.code().unwrap_or(-1)
92114
));
93115
}
@@ -171,6 +193,10 @@ pub async fn update_managed_agent(
171193
if let Some(mcp_command) = input.mcp_command {
172194
record.mcp_command = mcp_command;
173195
}
196+
if let Some(env_vars) = input.env_vars {
197+
crate::managed_agents::validate_user_env_keys(&env_vars)?;
198+
record.env_vars = env_vars;
199+
}
174200

175201
// Inbound author gate: merge patch onto current values, then validate
176202
// the merged state. This lets a single update switch to Allowlist AND

desktop/src-tauri/src/commands/agents.rs

Lines changed: 73 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,23 @@ fn workspace_owner_hex(state: &AppState) -> Result<String, String> {
2828
}
2929

3030
/// Build the standard agent JSON payload for provider deploy calls.
31-
fn build_deploy_payload(record: &ManagedAgentRecord) -> serde_json::Value {
32-
serde_json::json!({
31+
///
32+
/// Fails closed if the agent points at a `persona_id` we can't load — persona
33+
/// env_vars typically hold API credentials, and silently deploying with an
34+
/// empty map would surface as an opaque 401 from the provider.
35+
fn build_deploy_payload(
36+
app: &AppHandle,
37+
record: &ManagedAgentRecord,
38+
) -> Result<serde_json::Value, String> {
39+
// Merge persona env_vars + agent env_vars for provider deploy. Same
40+
// precedence as local spawn: persona first, agent overrides last. Without
41+
// this, provider-backed agents wouldn't receive credentials saved on the
42+
// persona or the agent itself.
43+
let persona_env =
44+
crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?;
45+
let merged_env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars);
46+
47+
Ok(serde_json::json!({
3348
"name": &record.name,
3449
"relay_url": &record.relay_url,
3550
"private_key_nsec": &record.private_key_nsec,
@@ -46,7 +61,35 @@ fn build_deploy_payload(record: &ManagedAgentRecord) -> serde_json::Value {
4661
// to the harness default (`owner-only`) — no protocol break.
4762
"respond_to": record.respond_to,
4863
"respond_to_allowlist": &record.respond_to_allowlist,
49-
})
64+
// Merged persona + agent env vars. Providers that don't read this
65+
// field will simply ignore it — no protocol break.
66+
"env_vars": merged_env,
67+
}))
68+
}
69+
70+
/// Persist a deploy-preparation error (currently: persona env resolution
71+
/// failure inside `build_deploy_payload`) into the agent's `last_error`
72+
/// so a refresh shows the cause. Mirrors what `deploy_to_provider` does
73+
/// on its own failures — without this, an agent created with an invalid
74+
/// persona_id would appear as `not_deployed` with no recorded reason.
75+
fn persist_create_deploy_error(
76+
app: &AppHandle,
77+
state: &AppState,
78+
pubkey: &str,
79+
error: &str,
80+
) -> Result<(), String> {
81+
let _store_guard = state
82+
.managed_agents_store_lock
83+
.lock()
84+
.map_err(|e| e.to_string())?;
85+
let mut records = load_managed_agents(app)?;
86+
let rec = records
87+
.iter_mut()
88+
.find(|r| r.pubkey == pubkey)
89+
.ok_or_else(|| format!("agent {pubkey} not found"))?;
90+
rec.last_error = Some(error.to_string());
91+
rec.updated_at = now_iso();
92+
save_managed_agents(app, &records)
5093
}
5194

5295
/// Deploy an agent to a provider backend. Resolves the binary, calls deploy via
@@ -162,6 +205,7 @@ pub async fn create_managed_agent(
162205
return Err("parallelism must be between 1 and 32".to_string());
163206
}
164207
}
208+
crate::managed_agents::validate_user_env_keys(&input.env_vars)?;
165209

166210
// Validate & normalize the respond-to allowlist BEFORE any side effects.
167211
// The harness has its own validator (sprout-acp/src/config.rs) but we want
@@ -374,6 +418,7 @@ pub async fn create_managed_agent(
374418
// NOT the display_name — ACP's resolve_persona_by_name() matches slugs.
375419
persona_pack_path: pack_metadata.as_ref().map(|(path, _)| path.clone()),
376420
persona_name_in_pack: pack_metadata.as_ref().map(|(_, name)| name.clone()),
421+
env_vars: input.env_vars.clone(),
377422
created_at: now_iso(),
378423
updated_at: now_iso(),
379424
last_started_at: None,
@@ -442,11 +487,31 @@ pub async fn create_managed_agent(
442487
.iter()
443488
.find(|r| r.pubkey == pubkey)
444489
.ok_or_else(|| "agent disappeared".to_string())?;
445-
build_deploy_payload(rec)
490+
build_deploy_payload(&app, rec)
446491
};
447-
match deploy_to_provider(&app, &state, &pubkey, id, config, agent_json, None).await {
448-
Ok(()) => spawn_error,
449-
Err(e) => Some(e),
492+
// The agent was already persisted in Phase 3 — converting a
493+
// persona-resolution failure into `spawn_error` (rather than
494+
// unwinding) keeps the record on disk and surfaces the cause
495+
// in the agent's last_error / UI status. We persist the same
496+
// error string into `last_error` so a refresh after restart
497+
// still shows *why* deploy never happened, matching what
498+
// `deploy_to_provider` does on its own failures.
499+
match agent_json {
500+
Err(e) => {
501+
if let Err(persist_err) = persist_create_deploy_error(&app, &state, &pubkey, &e)
502+
{
503+
eprintln!(
504+
"sprout-desktop: failed to persist deploy-prep error for {pubkey}: {persist_err}"
505+
);
506+
}
507+
Some(e)
508+
}
509+
Ok(json) => {
510+
match deploy_to_provider(&app, &state, &pubkey, id, config, json, None).await {
511+
Ok(()) => spawn_error,
512+
Err(e) => Some(e),
513+
}
514+
}
450515
}
451516
} else {
452517
spawn_error
@@ -521,7 +586,7 @@ pub async fn start_managed_agent(
521586
return build_managed_agent_summary(&app, record, &runtimes);
522587
}
523588

524-
let payload = build_deploy_payload(record);
589+
let payload = build_deploy_payload(&app, record)?;
525590
(
526591
record.backend.clone(),
527592
record.provider_binary_path.clone(),

desktop/src-tauri/src/commands/personas.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ pub fn create_persona(
6666
.map(|s| s.trim().to_string())
6767
.filter(|s| !s.is_empty())
6868
.collect();
69+
crate::managed_agents::validate_user_env_keys(&input.env_vars)?;
6970
let persona = PersonaRecord {
7071
id: Uuid::new_v4().to_string(),
7172
display_name,
@@ -78,6 +79,7 @@ pub fn create_persona(
7879
is_active: true,
7980
source_pack: None,
8081
source_pack_persona_slug: None,
82+
env_vars: input.env_vars,
8183
created_at: now.clone(),
8284
updated_at: now,
8385
};
@@ -122,6 +124,13 @@ pub fn update_persona(
122124
.map(|s| s.trim().to_string())
123125
.filter(|s| !s.is_empty())
124126
.collect();
127+
if let Some(env_vars) = input.env_vars {
128+
// Caller explicitly sent env_vars — replace entirely (empty = clear).
129+
crate::managed_agents::validate_user_env_keys(&env_vars)?;
130+
persona.env_vars = env_vars;
131+
}
132+
// Absent env_vars means "don't touch" — preserve existing creds when
133+
// the caller only meant to edit a different field.
125134
persona.updated_at = now_iso();
126135

127136
save_personas(&app, &personas)?;
@@ -318,6 +327,12 @@ pub async fn export_persona_to_json(
318327
state: State<'_, AppState>,
319328
) -> Result<bool, String> {
320329
// Load persona data under lock, then drop lock before dialog.
330+
//
331+
// NOTE: `env_vars` are deliberately NOT included in the exported card.
332+
// Persona cards are designed to be shareable artifacts (uploaded,
333+
// forked, distributed), and bundling API keys / credentials in them
334+
// would be a significant footgun. Users who import a card and need
335+
// credentials must supply them post-import via the persona dialog.
321336
let (display_name, system_prompt, avatar_url, provider, model, name_pool) = {
322337
let _store_guard = state
323338
.managed_agents_store_lock

0 commit comments

Comments
 (0)