From 0e88804303d1c08ed3cf143d4fb5de4633f80b97 Mon Sep 17 00:00:00 2001 From: Hardworking Honey Date: Thu, 6 Aug 2026 09:29:24 -0500 Subject: [PATCH] Include thinking effort in catalog personas Co-authored-by: Hardworking Honey Co-authored-by: Atish Patel Signed-off-by: Atish Patel --- .../src-tauri/src/managed_agents/env_vars.rs | 17 ++++++++- .../src/managed_agents/persona_events.rs | 31 +++++++++++++--- .../managed_agents/persona_events/tests.rs | 37 ++++++++++++++++++- docs/nips/NIP-AP.md | 18 ++++++--- 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 01e9521614..eb522f8d87 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -230,16 +230,29 @@ pub(crate) fn thinking_effort_from_record( .and_then(crate::managed_agents::known_acp_runtime_exact) .or_else(|| crate::managed_agents::known_acp_runtime(&record.agent_command))?; + thinking_effort_from_runtime_env(Some(runtime.id), &record.env_vars) +} + +/// Extract a portable thinking value for a declared runtime from its +/// runtime-specific persisted representation. Catalog sharing uses this +/// runtime-id-only form because publishing a persona never has an agent command +/// fallback and must not infer behavior from arbitrary local environment data. +pub(crate) fn thinking_effort_from_runtime_env( + runtime_id: Option<&str>, + env_vars: &BTreeMap, +) -> Option { + let runtime = runtime_id.and_then(crate::managed_agents::known_acp_runtime_exact)?; + if let (Some(env_key), Some(json_key)) = ( runtime.thinking_config_json_env_var, runtime.thinking_config_json_key, ) { - return json_env_string(&record.env_vars, env_key, json_key); + return json_env_string(env_vars, env_key, json_key); } runtime .thinking_env_var - .and_then(|key| record.env_vars.get(key)) + .and_then(|key| env_vars.get(key)) .cloned() } diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index de396f45c0..b7581c0658 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -10,16 +10,17 @@ use nostr::{EventBuilder, Kind, Tag}; use serde::{Deserialize, Serialize}; use super::{AgentDefinition, ManagedAgentRecord}; -use crate::app_state::AppState; +use crate::{ + app_state::AppState, + managed_agents::env_vars::{portable_config_for_import, thinking_effort_from_runtime_env}, +}; /// The JSON body stored in a persona event's content field. /// /// Field order MUST match the NIP-AP reference vectors (`docs/nips/NIP-AP.md` /// content body: `display_name, system_prompt, avatar_url, runtime, model, -/// provider, name_pool`). serde emits fields in declaration order, so this -/// order pins the exact content bytes and therefore the NIP-01 event id — a -/// reorder here breaks cross-implementation interop. Guarded by -/// `content_matches_nip_ap_vector`. +/// provider, name_pool`). New optional fields are appended so legacy content +/// retains its exact bytes and therefore its NIP-01 event id. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct PersonaEventContent { pub display_name: String, @@ -48,6 +49,11 @@ pub struct PersonaEventContent { pub respond_to_allowlist: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub parallelism: Option, + /// A non-secret, harness-neutral effort tier. This is deliberately a + /// scalar—not a general env map—so shared catalog events never expose + /// credentials or arbitrary local configuration. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub thinking_effort: Option, } /// Derive the d-tag (persona slug) from a `AgentDefinition`. @@ -178,6 +184,11 @@ pub fn persona_from_event(event: &nostr::Event) -> Result Result PersonaEventContent { model: record.model.clone(), provider: record.provider.clone(), name_pool: record.name_pool.clone(), + // Never publish `env_vars`: it may hold API keys. The only env-backed + // configuration that crosses the catalog boundary is this explicitly + // projected scalar, restored to its runtime-specific env target on + // import. + thinking_effort: thinking_effort_from_runtime_env( + record.runtime.as_deref(), + &record.env_vars, + ), // NIP-AP behavioral defaults: live since the create-path unification // (B5) — carried on AgentDefinition in wire shape and copied verbatim. // Quad-absent records serialize identically to the reserved era, so diff --git a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs index 0580b12ce2..f033232c47 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -239,9 +239,9 @@ fn d_tag_already_valid_slug_is_unchanged() { // through untouched (no spurious coordinate change on existing data). let mut record = sample_persona(); record.source_team_persona_slug = None; - record.id = "11111111-2222-3333-4444-555555555555".to_string(); + record.id = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee".to_string(); let d = persona_d_tag(&record); - assert_eq!(d, "11111111-2222-3333-4444-555555555555"); + assert_eq!(d, "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); assert!(passes_relay_slug_grammar(&d)); } @@ -303,6 +303,36 @@ fn round_trip_serialization() { assert!(restored.is_active); } +#[test] +fn catalog_round_trip_preserves_effort_without_publishing_env_vars() { + let mut record = sample_persona(); + record.runtime = Some("codex".to_string()); + record.env_vars = BTreeMap::from([ + ( + "CODEX_CONFIG".to_string(), + r#"{"model_reasoning_effort":"xhigh","model_providers":{"private":{"wire_api":"secret"}}}"#.to_string(), + ), + ("OPENAI_API_KEY".to_string(), "secret".to_string()), + ]); + + let event = build_persona_event(&record) + .unwrap() + .sign_with_keys(&nostr::Keys::generate()) + .unwrap(); + + assert!(event.content.contains(r#""thinking_effort":"xhigh""#)); + assert!(!event.content.contains("CODEX_CONFIG")); + assert!(!event.content.contains("OPENAI_API_KEY")); + assert!(!event.content.contains("secret")); + + let restored = persona_from_event(&event).unwrap(); + assert_eq!( + restored.env_vars.get("CODEX_CONFIG").map(String::as_str), + Some(r#"{"model_reasoning_effort":"xhigh"}"#) + ); + assert_eq!(restored.env_vars.len(), 1); +} + /// NIP-AP reference vector (Event 1, `docs/nips/NIP-AP.md:195-207`): the /// serialized content bytes MUST match the spec exactly, byte-for-byte. /// serde emits fields in declaration order, so this pins the content @@ -325,6 +355,7 @@ fn content_matches_nip_ap_vector() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + thinking_effort: None, }; assert_eq!( serde_json::to_string(&content).unwrap(), @@ -576,6 +607,7 @@ fn persona_content_hash_is_deterministic() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + thinking_effort: None, }; let hash1 = persona_content_hash(&content); let hash2 = persona_content_hash(&content); @@ -596,6 +628,7 @@ fn persona_content_hash_changes_on_edit() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + thinking_effort: None, }; let mut content2 = content1.clone(); content2.system_prompt = Some("Goodbye".to_string()); diff --git a/docs/nips/NIP-AP.md b/docs/nips/NIP-AP.md index ad23d92282..3d14d180c2 100644 --- a/docs/nips/NIP-AP.md +++ b/docs/nips/NIP-AP.md @@ -70,7 +70,8 @@ The `content` field is a **plaintext** (unencrypted) JSON object: "name_pool": ["", ...], "respond_to": "", "respond_to_allowlist": ["<64-hex pubkey>", ...], - "parallelism": "" + "parallelism": "", + "thinking_effort": "" } ``` @@ -93,14 +94,16 @@ The `content` field is a **plaintext** (unencrypted) JSON object: | `respond_to` | string \| null | `null` | **Reserved.** Default respond-to policy for instances spawned from this definition: `"anyone"`, `"owner-only"`, or `"allowlist"`. `null` defers to the client default. | | `respond_to_allowlist` | string[] | `[]` | **Reserved.** Allowlisted author pubkeys (64-char lowercase hex) when `respond_to` is `"allowlist"`. Ignored otherwise. | | `parallelism` | integer \| null | `null` | **Reserved.** Default max concurrent turns for spawned instances. `null` defers to the client default. | +| `thinking_effort` | string \| null | `null` | Non-secret effort tier for the declared runtime. Readers apply it only through their known runtime's dedicated effort setting; it is not a generic environment-variable mechanism. | The behavioral fields (`respond_to`, `respond_to_allowlist`, `parallelism`) are definition-level *defaults*: a spawned instance copies them -at creation and may be reconfigured independently afterwards. They were -previously carried only on the kind:30177 projection (see -"Slimming: kind:30177" below). +at creation and may be reconfigured independently afterwards. `thinking_effort` +is likewise a definition-level default, but readers must map it only through a +known runtime's explicit effort configuration target. They were previously +carried only on the kind:30177 projection (see "Slimming: kind:30177" below). -**Status: reserved.** In the current implementation these behavioral fields are +**Status: reserved.** In the current implementation the behavioral fields are *parsed but not yet applied*: readers tolerate and preserve them at the wire layer, but the local definition store does not yet carry them and writers do not emit them. The instance-copy-at-creation behavior activates in a @@ -108,6 +111,11 @@ subsequent release (the create-path unification). Until then a definition carrying these fields round-trips through the wire type but the values do not survive a local edit-and-republish cycle. +`thinking_effort` is implemented independently of those reserved behavioral +fields. Writers emit it only as a normalized non-secret scalar, and readers +map it only for a recognized runtime with an explicit effort configuration +target. + Unknown fields MUST be ignored by readers (forward compatibility). ### Prohibited: secrets in content