From 3d925efe9402e0ec93b91b04520f1808ff7d35b4 Mon Sep 17 00:00:00 2001 From: Duncan Date: Tue, 1 Sep 2026 11:58:10 -0400 Subject: [PATCH] =?UTF-8?q?chore(desktop):=20reclaim=20ratchet=20headroom?= =?UTF-8?q?=20=E2=80=94=20extract=20duplicate=20types,=20compact=20test=20?= =?UTF-8?q?fixtures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five files exceeded the file-size gate after the permission-policy additions and main merge. Reclaim mechanism per file: - types.ts (1028→923): Remove duplicate inline definitions of AgentPersona, CatalogSourceCoordinate, PersonaBehaviorInput, CreatePersonaInput, UpdatePersonaInput, SwitchManagedAgentModelStatus, and ControlResultFrame that were re-inlined by the merge. The re-export block already handles them; canonical definitions live in personaTypes.ts and permissionPolicy.ts. Add permissionPolicy field to AgentPersona and PersonaBehaviorInput in personaTypes.ts (canonical location) instead. - agent_models_tests.rs (1015→990): Remove 24-line dead AgentDefinition struct literal immediately shadowed by serde_json::from_str re-assignment of the same variable. - persona_events/tests.rs (1044→998): Replace two 20-27 line AgentDefinition struct literals in permission-policy tests with sample_persona() helper calls (field values verified to match assertions). - discovery/tests.rs (1822→1819): Remove duplicate doc comment line and redundant doc on record_with (2 lines; already-over-cap file may not grow). - readiness.rs (1742→1741): Trim one doc comment line (already-over-cap file may not grow). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src/commands/agent_models_tests.rs | 24 ---- .../src/managed_agents/discovery/tests.rs | 2 - .../managed_agents/persona_events/tests.rs | 61 ++-------- .../src-tauri/src/managed_agents/readiness.rs | 9 +- desktop/src/shared/api/personaTypes.ts | 4 + desktop/src/shared/api/types.ts | 104 ------------------ 6 files changed, 16 insertions(+), 188 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index add5bd0ba2..009ae289fb 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -428,30 +428,6 @@ fn model_discovery_ignores_stale_record_for_linked_agent() { ) .expect("sample managed agent record"); - let persona = crate::managed_agents::AgentDefinition { - permission_policy: None, - id: "persona-1".to_string(), - display_name: "Persona".to_string(), - avatar_url: None, - system_prompt: "You are a persona.".to_string(), - runtime: Some("goose".to_string()), - model: Some("persona-model".to_string()), - provider: Some("anthropic".to_string()), - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - team_catalog_source: None, - env_vars: BTreeMap::new(), - respond_to: None, - respond_to_allowlist: Vec::new(), - parallelism: None, - created_at: "".to_string(), - updated_at: "".to_string(), - }; let persona: crate::managed_agents::AgentDefinition = serde_json::from_str( r#"{ "id": "persona-1", diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 24588aad29..4c3f664938 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -205,8 +205,6 @@ fn effective_agent_command_explicit_override_wins() { ); } -/// Minimal record for `record_agent_command` tests. -/// Minimal record for `record_agent_command` tests; only resolution inputs vary. fn record_with( runtime: Option<&str>, persona_id: Option<&str>, 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 f0a2638952..0844a27e2d 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -378,30 +378,10 @@ fn content_matches_nip_ap_vector() { // An event built from this content carries the byte-exact vector as its // signed content, so a second implementer following the spec computes // the same NIP-01 id. - let record = AgentDefinition { - permission_policy: None, - description: None, - id: "test-agent".to_string(), - display_name: "Test Agent".to_string(), - avatar_url: Some("https://example.com/avatar.png".to_string()), - system_prompt: "You are a test assistant.".to_string(), - runtime: Some("goose".to_string()), - model: Some("claude-opus-4".to_string()), - provider: Some("anthropic".to_string()), - name_pool: vec!["Alpha".to_string(), "Beta".to_string()], - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - team_catalog_source: None, - env_vars: BTreeMap::new(), - respond_to: None, - respond_to_allowlist: Vec::new(), - parallelism: None, - created_at: "2025-01-01T00:00:00Z".to_string(), - updated_at: "2025-01-01T00:00:00Z".to_string(), + let record = { + let mut p = sample_persona(); + p.id = "test-agent".to_string(); + p }; let event = build_persona_event(&record) .unwrap() @@ -557,37 +537,12 @@ fn quad_absent_definition_hash_stable_across_activation() { } /// The definition permission policy is a local authority grant, never -/// published (Q4 local-only). `persona_content_hash` — the drift-badge basis -/// and republish trigger — is computed over `PersonaEventContent`, which has -/// no policy field. So flipping the definition's policy must not move the -/// hash: no deployed instance shows a spurious drift badge and no republish -/// wave fires when an owner edits the default. This pins that at the hash. +/// published. `persona_content_hash` is computed over `PersonaEventContent` +/// which has no policy field — flipping the definition's policy must not +/// move the hash (no spurious drift badge or republish wave). #[test] fn definition_permission_policy_does_not_affect_content_hash() { - let base = AgentDefinition { - permission_policy: None, - id: "policy-hash".to_string(), - display_name: "Test".to_string(), - avatar_url: None, - system_prompt: "Hello".to_string(), - runtime: Some("goose".to_string()), - model: Some("gpt-oss".to_string()), - provider: None, - name_pool: vec!["nib".to_string()], - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - team_catalog_source: None, - env_vars: BTreeMap::new(), - respond_to: Some("anyone".to_string()), - respond_to_allowlist: Vec::new(), - parallelism: Some(2), - created_at: "2026-01-01T00:00:00Z".to_string(), - updated_at: "2026-01-01T00:00:00Z".to_string(), - }; + let base = sample_persona(); let mut with_policy = base.clone(); with_policy.permission_policy = Some(crate::managed_agents::permission_policy::PermissionPolicy::Allow); diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 5ae14f13f4..85e26f47fa 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -112,8 +112,8 @@ pub(crate) struct EffectiveHarnessDescriptor { /// Returns `Err("DANGLING_HARNESS_ID:")` when the record (or its linked /// persona) references a runtime id that no longer exists in the registry — /// the same typed error produced by `try_record_agent_command`. Callers that -/// cannot continue with a dangling id (e.g. `spawn_agent_child`) propagate the -/// error; callers that degrade gracefully may use `.unwrap_or_else(|_| …)`. +/// cannot continue with a dangling id propagate the error; callers that degrade +/// gracefully may use `.unwrap_or_else(|_| …)`. /// /// Does NOT require an `AppHandle` so it is fully unit-testable. /// @@ -394,9 +394,8 @@ impl AgentReadiness { /// credential store — NOT `OPENAI_API_KEY`). /// * **unknown / custom command**: always `Ready` (no requirements known). /// -/// Databricks note: `DATABRICKS_TOKEN` is `.unwrap_or_default()` in -/// `buzz-agent/src/config.rs:143` — an escape hatch for static tokens, but -/// the normal path is OAuth PKCE, so we do NOT mark the token as required. +/// Databricks note: `DATABRICKS_TOKEN` is `.unwrap_or_default()` (escape hatch +/// for static tokens); normal path is OAuth PKCE, so token is not required. pub(crate) fn agent_readiness(effective: &EffectiveAgentEnv) -> AgentReadiness { let runtime = known_acp_runtime(&effective.effective_command); let missing = collect_missing_requirements(effective, runtime); diff --git a/desktop/src/shared/api/personaTypes.ts b/desktop/src/shared/api/personaTypes.ts index f18e9fe96b..1dc10584c7 100644 --- a/desktop/src/shared/api/personaTypes.ts +++ b/desktop/src/shared/api/personaTypes.ts @@ -39,6 +39,8 @@ export type AgentPersona = { respondTo: RespondToMode | null; respondToAllowlist: string[]; parallelism: number | null; + /** Definition-level default permission policy — tier 2 of the resolver (instance → this → global → built-in `ask`). Null = defer. Local-only. */ + permissionPolicy: import("./permissionPolicy").PermissionPolicy | null; createdAt: string; updatedAt: string; }; @@ -61,6 +63,8 @@ export type PersonaBehaviorInput = { respondTo?: RespondToMode; respondToAllowlist?: string[]; parallelism?: number; + /** Definition-level default permission policy. Within a present behavior group it replaces the stored value as a unit: omitted clears the default. Never published. */ + permissionPolicy?: import("./permissionPolicy").PermissionPolicy; }; export type CreatePersonaInput = { diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 7a02f9eb76..a0f6ba0592 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -462,24 +462,6 @@ export type { SwitchManagedAgentModelStatus, ControlResultFrame, } from "./permissionPolicy"; -/** Outcome of a live `switch_model` control frame; `failure` lands late. */ -export type SwitchManagedAgentModelStatus = - | "sent" - | "turn_ending" - | "ambiguous_target" - | "switched" - | "unsupported_model" - | "no_active_turn" - | "failure"; -export type ControlResultFrame = { - type: "cancel_turn" | "switch_model"; - status: string; - modelId?: string; - /** Opaque per-pick id echoed from the request; correlates late frames. */ - requestId?: string; - /** Buzz channel UUID from the observer envelope; disambiguates channels. */ - channelId?: string | null; -}; export type GitBashPrerequisite = { available: boolean; @@ -719,92 +701,6 @@ export type UpdateManagedAgentInput = { /** Absent = don't touch. `null` = clear to inherit. Remote: read-only. */ permissionPolicy?: PermissionPolicy | null; }; -export type AgentPersona = { - id: string; - displayName: string; - avatarUrl: string | null; - systemPrompt: string; - /** Preferred ACP runtime ID (e.g. "goose", "claude"). */ - runtime: string | null; - /** Opaque, harness-specific model identifier string. Buzz stores and passes through without interpretation. */ - model: string | null; - /** LLM inference provider (e.g. "databricks", "anthropic"). Injected as the runtime's provider env var at spawn time. */ - provider: string | null; - namePool: string[]; - isBuiltIn: boolean; - isActive: boolean; - /** Whether this persona is discoverable in the active community catalog. */ - shared: boolean; - /** Team ID if this persona was imported from a team directory. Team personas are non-editable. */ - sourceTeam?: string | null; - /** - * Set only on a local copy of another owner's shared catalog entry. A copy - * carries a fresh local `id`, so this coordinate is the only thing that can - * answer "is this catalog entry already added" without minting a duplicate. - */ - catalogSource?: CatalogSourceCoordinate | null; - /** Agent environment variables, layered after desktop parent and persona values. */ - envVars: Record; - /** NIP-AP behavioral defaults (wire shape). Null/empty = unset. */ - respondTo: RespondToMode | null; - respondToAllowlist: string[]; - parallelism: number | null; - /** Definition-level default permission policy — tier 2 of the resolver (instance → this → global → built-in `ask`). Null = defer. Local-only. */ - permissionPolicy: PermissionPolicy | null; - createdAt: string; - updatedAt: string; -}; - -/** - * A catalog publication's coordinate: the owner who published it and the - * `d`-tag identifying the persona within that owner's catalog. - */ -export type CatalogSourceCoordinate = { - ownerPubkey: string; - personaId: string; -}; - -/** - * NIP-AP behavioral group for a definition: absent preserves the stored group - * for legacy callers; present replaces it as a unit. Mirrors `PersonaBehaviorRequest`. - */ -export type PersonaBehaviorInput = { - respondTo?: RespondToMode; - respondToAllowlist?: string[]; - parallelism?: number; - /** Definition-level default permission policy. Within a present behavior group it replaces the stored value as a unit: omitted clears the default. Never published. */ - permissionPolicy?: PermissionPolicy; -}; - -export type CreatePersonaInput = { - displayName: string; - avatarUrl?: string; - systemPrompt: string; - runtime?: string; - model?: string; - provider?: string; - namePool?: string[]; - envVars?: Record; - behavior?: PersonaBehaviorInput; - /** - * Set when this persona is a copy of another owner's shared catalog entry, - * so the catalog can tell an already-added foreign entry from a new one. - */ - catalogSource?: CatalogSourceCoordinate; -}; - -export type UpdatePersonaInput = { - id: string; - displayName: string; - avatarUrl?: string; - systemPrompt: string; - runtime?: string; - model?: string; - provider?: string; - namePool?: string[]; - envVars?: Record; - behavior?: PersonaBehaviorInput; -}; // Persona (agent definition) types live in a sibling module to keep this // file inside the repo-wide size ratchet; re-exported so import paths // (`@/shared/api/types`) are unchanged.