mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): persist applied permission policy for remote deploys
ManagedAgentSummary recomputed the displayed permission policy from the mutable agent record plus global config, so flipping the global default after a remote deploy made the UI report a policy the worker was not actually running. Stamp the byte-identical policy sent to the provider onto the record at the deploy choke point, expose it on the summary as applied vs desired, and surface drift in the policy field UI with a redeploy prompt. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -117,6 +117,7 @@ fn agent_record() -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
agent_command_override: None,
|
||||
persona_source_version: None,
|
||||
provider: None,
|
||||
|
||||
@@ -408,13 +408,8 @@ pub(super) async fn start_local_agent_with_preflight(
|
||||
if record.backend != BackendKind::Local {
|
||||
return Err(format!("agent {pubkey} is no longer a local agent"));
|
||||
}
|
||||
// Re-snapshot the persona onto the record at every spawn so the agent always
|
||||
// starts with the current persona config (system_prompt, model, provider,
|
||||
// runtime). This clears the "out of date" drift badge without requiring a
|
||||
// delete+recreate. See `apply_persona_snapshot` for the precedence and
|
||||
// env-override self-heal rules.
|
||||
// Load personas once: used for snapshot application below and summary build
|
||||
// at the end — avoids a second disk read for the same file in the same call.
|
||||
// Re-snapshot the persona at every spawn (current persona config wins; clears
|
||||
// drift badge). Load once — also used for summary build at the end.
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
if let Some(persona_id) = record.persona_id.clone() {
|
||||
match personas.iter().find(|p| p.id == persona_id) {
|
||||
@@ -480,12 +475,15 @@ async fn deploy_to_provider(
|
||||
.map_or_else(|| resolve_provider_binary(provider_id), Ok)?;
|
||||
|
||||
let config_clone = config.clone();
|
||||
let applied_policy: Option<crate::managed_agents::permission_policy::PermissionPolicy> =
|
||||
agent_json["launch"]["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"]
|
||||
.as_str()
|
||||
.and_then(|s| serde_json::from_value(serde_json::Value::String(s.to_string())).ok());
|
||||
let deploy_result =
|
||||
tokio::task::spawn_blocking(move || provider_deploy(&bin_path, &agent_json, &config_clone))
|
||||
.await
|
||||
.map_err(|e| format!("spawn_blocking failed: {e}"))?;
|
||||
|
||||
// Persist result under lock.
|
||||
let _store_guard = state
|
||||
.managed_agents_store_lock
|
||||
.lock()
|
||||
@@ -502,6 +500,7 @@ async fn deploy_to_provider(
|
||||
rec.last_started_at = Some(now_iso());
|
||||
rec.updated_at = now_iso();
|
||||
rec.last_error = None;
|
||||
rec.applied_permission_policy = applied_policy;
|
||||
}
|
||||
Err(ref e) => {
|
||||
rec.last_error = Some(e.clone());
|
||||
@@ -913,6 +912,7 @@ pub async fn create_managed_agent(
|
||||
relay_mesh.clone()
|
||||
},
|
||||
permission_policy: None, // inherits global default or built-in `ask`
|
||||
applied_permission_policy: None, // populated on first successful remote deploy
|
||||
};
|
||||
|
||||
records.push(record);
|
||||
|
||||
@@ -59,6 +59,7 @@ fn bare_agent_record(
|
||||
catalog_source: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
auto_restart_on_config_change: false,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: vec![],
|
||||
|
||||
@@ -67,6 +67,7 @@ fn make_agent(
|
||||
catalog_source: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
auto_restart_on_config_change: false,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: vec![],
|
||||
|
||||
@@ -216,6 +216,7 @@ fn local_agent() -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -65,6 +65,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -655,6 +655,7 @@ pub async fn confirm_agent_snapshot_import(
|
||||
runtime: snapshot.definition.runtime.clone(),
|
||||
name_pool: snapshot.definition.name_pool.clone(),
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
};
|
||||
|
||||
records.push(record.clone());
|
||||
|
||||
@@ -74,6 +74,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -59,6 +59,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -610,6 +610,7 @@ pub async fn confirm_team_snapshot_import(
|
||||
definition_parallelism: minted_parallelism,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
runtime: member.definition.runtime.clone(),
|
||||
name_pool: member.definition.name_pool.clone(),
|
||||
};
|
||||
|
||||
@@ -230,6 +230,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
runtime: None,
|
||||
name_pool: vec![],
|
||||
};
|
||||
|
||||
@@ -217,6 +217,7 @@ mod tests {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -417,6 +417,7 @@ mod tests {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
agent_command_override: None,
|
||||
persona_source_version: None,
|
||||
provider: None,
|
||||
|
||||
@@ -73,6 +73,7 @@ fn minimal_record() -> ManagedAgentRecord {
|
||||
definition_parallelism: Some(4),
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -116,6 +116,7 @@ fn test_record() -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
agent_command_override: None,
|
||||
persona_source_version: None,
|
||||
provider: None,
|
||||
|
||||
@@ -283,13 +283,13 @@ fn record_with(
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn record_agent_command_own_runtime_wins_over_persona() {
|
||||
// A record with its own materialized runtime never consults the
|
||||
// persona list — the unified-model resolution.
|
||||
// A record with its own materialized runtime wins over the persona list.
|
||||
let personas = vec![persona_with_runtime("p1", Some("goose"))];
|
||||
let record = record_with(Some("claude"), Some("p1"), None);
|
||||
assert_eq!(record_agent_command(&record, &personas), "claude-agent-acp");
|
||||
|
||||
@@ -89,6 +89,7 @@ fn record(
|
||||
catalog_source: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
auto_restart_on_config_change: false,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: vec![],
|
||||
|
||||
@@ -350,6 +350,7 @@ fn bare_record() -> ManagedAgentRecord {
|
||||
catalog_source: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
auto_restart_on_config_change: false,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: vec![],
|
||||
|
||||
@@ -503,6 +503,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -118,6 +118,7 @@ mod tests {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -179,4 +179,38 @@ mod tests {
|
||||
assert_eq!(policy, PermissionPolicy::Reject);
|
||||
assert_eq!(source, PermissionPolicySource::Agent);
|
||||
}
|
||||
|
||||
/// Wes's regression: deploy under Allow, then flip global default to Reject.
|
||||
/// The summary's *desired* policy changes (global Reject wins) but the
|
||||
/// *applied* policy on the record must stay Allow — the worker is still
|
||||
/// running the policy it was launched with. The UI detects drift by
|
||||
/// comparing these two values and prompts a redeploy.
|
||||
#[test]
|
||||
fn test_applied_policy_survives_global_flip_deploy_allow_global_flips_to_reject() {
|
||||
let mut record = empty_record();
|
||||
// Simulate: agent was deployed with no per-agent override, global=Allow
|
||||
// at deploy time → applied_permission_policy stamped as Allow.
|
||||
record.permission_policy = None;
|
||||
record.applied_permission_policy = Some(PermissionPolicy::Allow);
|
||||
|
||||
// Global is now flipped to Reject (post-deploy mutation).
|
||||
let global_after_flip = GlobalAgentConfig {
|
||||
permission_policy: Some(PermissionPolicy::Reject),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
// Desired policy reflects the new global.
|
||||
let (desired, source) = resolve_effective_permission_policy(&record, &global_after_flip);
|
||||
assert_eq!(desired, PermissionPolicy::Reject);
|
||||
assert_eq!(source, PermissionPolicySource::GlobalDefault);
|
||||
|
||||
// Applied policy is unchanged — still what the worker was launched with.
|
||||
assert_eq!(
|
||||
record.applied_permission_policy,
|
||||
Some(PermissionPolicy::Allow)
|
||||
);
|
||||
|
||||
// Drift is detectable: applied ≠ desired.
|
||||
assert_ne!(record.applied_permission_policy, Some(desired));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,6 +59,7 @@ pub(super) fn sample_record() -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1465,9 +1465,8 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn resolve_effective_agent_env_user_env_wins_over_structured_fields() {
|
||||
// A record whose env_vars explicitly set provider/model must win over
|
||||
// any baked defaults. In OSS test builds the baked map is empty, so
|
||||
// this test validates the user-env layer is present in the output.
|
||||
// env_vars must win over baked defaults; in OSS builds the baked map is empty,
|
||||
// so this verifies the user-env layer is present.
|
||||
let mut env_vars = BTreeMap::new();
|
||||
env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string());
|
||||
env_vars.insert(
|
||||
@@ -1530,6 +1529,7 @@ mod tests {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
};
|
||||
|
||||
let runtime = known_acp_runtime_exact("buzz-agent");
|
||||
|
||||
@@ -344,6 +344,7 @@ pub fn build_managed_agent_summary(
|
||||
respond_to_allowlist: record.respond_to_allowlist.clone(),
|
||||
permission_policy: effective_permission_policy_summary,
|
||||
permission_policy_source: effective_permission_policy_source,
|
||||
applied_permission_policy: record.applied_permission_policy,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -90,5 +90,6 @@ pub(super) fn fixture(
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,6 +71,7 @@ fn record() -> ManagedAgentRecord {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -310,6 +310,7 @@ mod tests {
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -214,6 +214,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord {
|
||||
catalog_source: None,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: vec![],
|
||||
definition_parallelism: None,
|
||||
|
||||
@@ -151,6 +151,7 @@ impl AgentDefinition {
|
||||
definition_parallelism: self.parallelism,
|
||||
relay_mesh: None,
|
||||
permission_policy: None,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -352,6 +353,8 @@ pub struct ManagedAgentRecord {
|
||||
pub respond_to_allowlist: Vec<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub permission_policy: Option<super::permission_policy::PermissionPolicy>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub applied_permission_policy: Option<super::permission_policy::PermissionPolicy>,
|
||||
/// Optional display name distinct from the unique `name` handle. Absorbed
|
||||
/// from `AgentDefinition.display_name` (unified agent model, Phase 1A).
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -534,16 +537,11 @@ pub struct ManagedAgentSummary {
|
||||
/// persona is gone, so there is nothing newer to drift toward).
|
||||
pub persona_out_of_date: bool,
|
||||
/// `true` when the agent was created from a persona that no longer exists.
|
||||
/// Distinct from out-of-date: there is no current persona to respawn into.
|
||||
/// An orphaned agent also cannot be (re)started — `spawn_agent_child`
|
||||
/// refuses it (see `effective_config::resolve_effective_config`'s
|
||||
/// `OrphanedInstance` arm via `require_resolved`) — so the UI
|
||||
/// should surface that it's stuck, not merely stale.
|
||||
/// `true` when the agent's linked persona no longer exists; no current
|
||||
/// persona to respawn into and the agent cannot be (re)started.
|
||||
pub persona_orphaned: bool,
|
||||
/// `true` when the running process's spawn config no longer matches
|
||||
/// what a spawn would use today. Derived from `restart_diff` — lit
|
||||
/// exactly when there is something to show. Always `false` for stopped,
|
||||
/// orphaned, or `runtime_pid`-adopted agents.
|
||||
/// `true` when the running process's spawn config no longer matches what
|
||||
/// a spawn would use today. Always `false` for stopped/orphaned agents.
|
||||
pub needs_restart: bool,
|
||||
/// Fields that drifted since launch, redacted for display.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
@@ -568,6 +566,8 @@ pub struct ManagedAgentSummary {
|
||||
pub respond_to_allowlist: Vec<String>,
|
||||
pub permission_policy: super::permission_policy::PermissionPolicy,
|
||||
pub permission_policy_source: super::permission_policy::PermissionPolicySource,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub applied_permission_policy: Option<super::permission_policy::PermissionPolicy>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
|
||||
@@ -747,6 +747,7 @@ fn summary_fixture(
|
||||
permission_policy: crate::managed_agents::permission_policy::PermissionPolicy::Ask,
|
||||
permission_policy_source:
|
||||
crate::managed_agents::permission_policy::PermissionPolicySource::BuiltIn,
|
||||
applied_permission_policy: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -787,3 +788,31 @@ fn summary_with_drift_serializes_restart_diff_entries() {
|
||||
}]))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn applied_permission_policy_drift_serializes_correctly() {
|
||||
// When applied_permission_policy differs from permission_policy, both values
|
||||
// must reach the wire so the frontend can detect drift and prompt a redeploy.
|
||||
let mut summary = summary_fixture(Vec::new());
|
||||
summary.permission_policy = crate::managed_agents::permission_policy::PermissionPolicy::Reject;
|
||||
summary.applied_permission_policy =
|
||||
Some(crate::managed_agents::permission_policy::PermissionPolicy::Allow);
|
||||
|
||||
let wire = serde_json::to_value(&summary).expect("summary serializes");
|
||||
assert_eq!(wire["permission_policy"], serde_json::json!("reject"));
|
||||
assert_eq!(
|
||||
wire["applied_permission_policy"],
|
||||
serde_json::json!("allow")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn applied_permission_policy_none_omitted_from_wire() {
|
||||
// For local agents and never-deployed remote agents, applied_permission_policy
|
||||
// is None — it must be omitted from the wire (skip_serializing_if = "Option::is_none").
|
||||
let wire = serde_json::to_value(summary_fixture(Vec::new())).expect("summary serializes");
|
||||
assert!(
|
||||
wire.get("applied_permission_policy").is_none(),
|
||||
"absent applied_permission_policy must be omitted, got: {wire}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -23,7 +23,11 @@ export type AgentPermissionPolicyFieldHandle = {
|
||||
type Props = {
|
||||
agent: Pick<
|
||||
ManagedAgent,
|
||||
"backend" | "backendAgentId" | "permissionPolicy" | "permissionPolicySource"
|
||||
| "backend"
|
||||
| "backendAgentId"
|
||||
| "permissionPolicy"
|
||||
| "permissionPolicySource"
|
||||
| "appliedPermissionPolicy"
|
||||
>;
|
||||
disabled: boolean;
|
||||
};
|
||||
@@ -47,6 +51,11 @@ export const AgentPermissionPolicyField = React.forwardRef<
|
||||
const sourceLabel =
|
||||
SOURCE_LABEL[agent.permissionPolicySource] ?? agent.permissionPolicySource;
|
||||
|
||||
const hasDrift =
|
||||
isRemoteDeployed &&
|
||||
agent.appliedPermissionPolicy !== null &&
|
||||
agent.appliedPermissionPolicy !== agent.permissionPolicy;
|
||||
|
||||
return (
|
||||
<div className="space-y-1.5">
|
||||
<div className="flex items-center gap-1.5">
|
||||
@@ -61,9 +70,23 @@ export const AgentPermissionPolicyField = React.forwardRef<
|
||||
</span>
|
||||
</div>
|
||||
{isRemoteDeployed ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Read-only while deployed. To change, shut down and redeploy the agent.
|
||||
</p>
|
||||
<>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Read-only while deployed. To change, shut down and redeploy the
|
||||
agent.
|
||||
</p>
|
||||
{hasDrift && (
|
||||
<p className="text-xs text-amber-600 dark:text-amber-400">
|
||||
Applied policy:{" "}
|
||||
<span className="font-medium">
|
||||
{agent.appliedPermissionPolicy}
|
||||
</span>{" "}
|
||||
· Desired:{" "}
|
||||
<span className="font-medium">{agent.permissionPolicy}</span> —
|
||||
redeploy required to apply.
|
||||
</p>
|
||||
)}
|
||||
</>
|
||||
) : (
|
||||
<select
|
||||
className="w-full rounded-md border border-input bg-background px-3 py-1.5 text-sm shadow-sm focus:outline-none focus:ring-1 focus:ring-ring disabled:opacity-50"
|
||||
|
||||
@@ -53,6 +53,8 @@ export type RawManagedAgent = {
|
||||
// Pre-feature fixtures may omit these; defaults applied in fromRawManagedAgent.
|
||||
permission_policy?: PermissionPolicy;
|
||||
permission_policy_source?: PermissionPolicySource;
|
||||
/** Policy actually applied at the last remote deploy. `null` / absent for local or never-deployed agents. */
|
||||
applied_permission_policy?: PermissionPolicy | null;
|
||||
};
|
||||
|
||||
export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent {
|
||||
@@ -100,5 +102,6 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent {
|
||||
respondToAllowlist: agent.respond_to_allowlist ?? [],
|
||||
permissionPolicy: agent.permission_policy ?? "ask",
|
||||
permissionPolicySource: agent.permission_policy_source ?? "built_in",
|
||||
appliedPermissionPolicy: agent.applied_permission_policy ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -392,6 +392,8 @@ export type ManagedAgent = {
|
||||
permissionPolicy: PermissionPolicy;
|
||||
/** Where `permissionPolicy` came from: agent, global_default, or built_in. */
|
||||
permissionPolicySource: PermissionPolicySource;
|
||||
/** Policy active on the remote worker; non-null only after a successful deploy. Differs from `permissionPolicy` when drift exists. */
|
||||
appliedPermissionPolicy: PermissionPolicy | null;
|
||||
};
|
||||
|
||||
/** Inbound author gate mode. Mirrors buzz-acp's --respond-to CLI flag. */
|
||||
|
||||
Reference in New Issue
Block a user