From de7bb61ef5d443e9c4ce9370bdccb750b9412f15 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 29 Jul 2026 10:23:00 -0400 Subject: [PATCH] Remove nonfunctional run agent permissions setting --- ...-08-settings-toml-redesign-requirements.md | 9 +++---- ...-04-09-settings-toml-redesign-handoff-3.md | 2 +- docs/public/api-reference/fabro-api.yaml | 12 +++------- docs/public/execution/run-configuration.mdx | 2 +- docs/public/reference/user-configuration.mdx | 4 +--- ...2026-05-22-run-agent-fabro-tools-opt-in.md | 1 - lib/apps/fabro-cli/src/commands/exec.rs | 2 +- .../tests/workflow_settings_round_trip.rs | 2 ++ lib/foundation/fabro-config/src/layers/cli.rs | 5 ++-- .../fabro-config/src/layers/combine.rs | 7 +++--- lib/foundation/fabro-config/src/layers/run.rs | 12 ++-------- .../fabro-config/src/resolve/run.rs | 1 - .../fabro-config/src/tests/resolve_cli.rs | 5 ++-- .../fabro-config/src/tests/resolve_run.rs | 17 +++++++++++++ .../src/commands/docs_options_reference.rs | 5 ++-- .../fabro-types/src/settings/cli.rs | 10 +++++++- .../fabro-types/src/settings/run.rs | 11 --------- .../src/.openapi-generator/FILES | 1 - .../src/models/agent-permissions.ts | 24 ------------------- .../fabro-api-client/src/models/index.ts | 1 - .../src/models/run-agent-settings.ts | 5 +--- 21 files changed, 53 insertions(+), 85 deletions(-) delete mode 100644 lib/packages/fabro-api-client/src/models/agent-permissions.ts diff --git a/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md b/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md index 6dd9f4c7f..c34cb4ad0 100644 --- a/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md +++ b/docs/brainstorms/2026-04-08-settings-toml-redesign-requirements.md @@ -58,8 +58,8 @@ The new design must optimize for: - R21. `metadata` replaces Fabro-owned `labels` and exists on `project`, `workflow`, and `run` as flat string-to-string maps. - R22. `run.inputs` replaces `vars`. `run.inputs` must accept TOML scalar values. `metadata` remains string-to-string. `run.inputs` intentionally replaces the full inherited map rather than merging by key. - R23. `[run.model]` is the default model selection surface for LLM-backed workflow stages. `[run.agent]` is only for agent-specific settings. -- R24. `[run.agent]` owns agent-only knobs such as `permissions` and `mcps`. `[run.sandbox]` owns the sandbox selection and execution-environment surface, including `provider`, shared sandbox knobs, `env`, and provider-specific nested tables. -- R25. `run.agent.permissions` must remain a simple enum string, not an object. +- R24. `[run.agent]` owns agent-only knobs such as `fabro_tools` and `mcps`. `[run.sandbox]` owns the sandbox selection and execution-environment surface, including `provider`, shared sandbox knobs, `env`, and provider-specific nested tables. +- R25. Workspace tool permissions belong to `[cli.exec.agent]`. - R26. `[run.git]` and `[run.scm]` must remain separate concepts. `git` is local Git behavior such as commit author; `scm` is remote host/provider behavior. - R27. `[run.pull_request]` remains the provider-neutral run surface for PR behavior. - R28. `[run.prepare]` is the run preparation surface and replaces the old `setup` naming. @@ -275,7 +275,7 @@ Config-executed actions are part of Fabro's trusted configuration model, not the - `script` and `command` in prepare steps, hooks, and launching MCP transports are executable configuration, not passive metadata. - These actions execute under the trust boundary of the consuming process. -- They are not mediated by `run.agent.permissions` or `cli.exec.agent.permissions`. +- They are not mediated by `cli.exec.agent.permissions`. - Users should treat `fabro.toml` and `workflow.toml` as executable project configuration, not as untrusted data blobs. ## Migration and Failure Behavior @@ -386,9 +386,6 @@ provider = "anthropic" name = "sonnet" fallbacks = ["openai", "gpt-5.4", "gemini/gemini-flash"] -[run.agent] -permissions = "read-write" - [run.notifications.ops] enabled = true provider = "slack" diff --git a/docs/plans/2026-04-09-settings-toml-redesign-handoff-3.md b/docs/plans/2026-04-09-settings-toml-redesign-handoff-3.md index f277a9d55..e22e3fdaa 100644 --- a/docs/plans/2026-04-09-settings-toml-redesign-handoff-3.md +++ b/docs/plans/2026-04-09-settings-toml-redesign-handoff-3.md @@ -242,7 +242,7 @@ pass. The file-name collisions to resolve are `project.rs`, `run.rs`, | Crate | Legacy types it still imports | Suggested destination | |---|---|---| -| `fabro-agent` | `OutputFormat`, `PermissionLevel` from `settings::user` | Promote into `fabro-agent` itself — they're CLI/exec concerns. Or point at `settings::v2::cli::OutputFormat` / `v2::run::AgentPermissions` if shapes match. | +| `fabro-agent` | `OutputFormat`, `PermissionLevel` from `settings::user` | Promote into `fabro-agent` itself — they're CLI/exec concerns. Or point at `settings::v2::cli::OutputFormat` / `settings::v2::cli::AgentPermissions` if shapes match. | | `fabro-checkpoint` | `GitAuthorSettings` from `settings::server` | Promote into `fabro-checkpoint` or read directly from `v2::run::GitAuthorLayer` at the call site. | | `fabro-hooks` | `HookDefinition`, `HookEvent`, `HookSettings`, `HookType`, `TlsMode` | Promote all of them into `fabro-hooks`. They are runtime behavior types (has `resolved_hook_type()` / `runs_in_sandbox()` methods), not parse-tree types, so they belong in the consumer crate. | | `fabro-mcp` | `McpServerEntry`, `McpServerSettings`, `McpTransport`, `default_startup_timeout_secs`, `default_tool_timeout_secs` | Promote into `fabro-mcp`. Convert from v2 `run.agent.mcps.*` or `cli.exec.agent.mcps.*` at the call site. | diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index bd6a3fdb4..33b92f7c7 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -14627,21 +14627,15 @@ components: RunAgentSettings: type: object - required: [permissions, mcps] + required: [fabro_tools, mcps] properties: - permissions: - oneOf: - - $ref: "#/components/schemas/AgentPermissions" - - type: "null" + fabro_tools: + type: boolean mcps: type: object additionalProperties: $ref: "#/components/schemas/McpServerSettings" - AgentPermissions: - type: string - enum: [read-only, read-write, full] - McpServerSettings: type: object required: [name, transport, startup_timeout_secs, tool_timeout_secs] diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index eb81c82a9..7065bb4a9 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -466,7 +466,7 @@ fabro_tools = true One workflow-agent exception is intentional: `fabro_run_create` always creates [child runs](/execution/child-runs) parented to the current run. If an agent supplies `parent_id`, it must match the current run ID. -This setting is separate from normal agent `permissions` and from MCP server configuration. `permissions` controls workspace tool access, while `[run.agent.mcps]` configures external MCP servers available to the agent. +This setting is independent of `[run.agent.mcps]`, which configures external MCP servers available to the agent. ### `[run.agent.mcps]` diff --git a/docs/public/reference/user-configuration.mdx b/docs/public/reference/user-configuration.mdx index e60feb204..fef950d8c 100644 --- a/docs/public/reference/user-configuration.mdx +++ b/docs/public/reference/user-configuration.mdx @@ -434,19 +434,17 @@ enabled = true ## `[run.agent]` -`[run.agent]` — agent knobs only (Fabro tools, permissions, MCPs) +`[run.agent]` — agent knobs only (Fabro tools and MCPs) ```toml title="settings.toml" [run.agent] fabro_tools = true -permissions = "read-write" ``` | Key | Type / values | Default | Description | |---|---|---|---| | `fabro_tools` | boolean | false | Allow workflow agents to use Fabro run-management tools. | | `mcps` | table | None | Agent-scoped MCP server entries, keyed by name. | -| `permissions` | "read-only" \| "read-write" \| "full" | "read-write" | Default tool permission level for workflow agents. | ## `[run.agent.mcps.]` diff --git a/docs/superpowers/plans/2026-05-22-run-agent-fabro-tools-opt-in.md b/docs/superpowers/plans/2026-05-22-run-agent-fabro-tools-opt-in.md index 8730936d9..cdf3f7982 100644 --- a/docs/superpowers/plans/2026-05-22-run-agent-fabro-tools-opt-in.md +++ b/docs/superpowers/plans/2026-05-22-run-agent-fabro-tools-opt-in.md @@ -135,6 +135,5 @@ cargo +nightly-2026-04-14 clippy -p fabro-types -p fabro-config -p fabro-server - `fabro_tools` is a per-run opt-in setting only; this plan does not add a separate server-wide allow/deny policy. - Defaulting to `false` intentionally changes existing behavior: runs that need Fabro run tools must set `[run.agent] fabro_tools = true`. -- `run.agent.permissions` remains about ordinary agent tool permissions and does not imply Fabro API access. - `[run.agent.mcps]` remains independent; MCP tools are not enabled or disabled by `fabro_tools`. - `fabro mcp start` and standalone MCP exposure of Fabro tools are out of scope. diff --git a/lib/apps/fabro-cli/src/commands/exec.rs b/lib/apps/fabro-cli/src/commands/exec.rs index f72df2c22..bdab8caf0 100644 --- a/lib/apps/fabro-cli/src/commands/exec.rs +++ b/lib/apps/fabro-cli/src/commands/exec.rs @@ -302,7 +302,7 @@ fn run_mcp_servers_for_exec( pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResult<()> { use fabro_agent::cli::PermissionLevel as AgentPermissionLevel; - use fabro_types::settings::run::AgentPermissions; + use fabro_types::settings::cli::AgentPermissions; let cli = &ctx.user_settings().cli; #[cfg(feature = "sleep_inhibitor")] diff --git a/lib/foundation/fabro-api/tests/workflow_settings_round_trip.rs b/lib/foundation/fabro-api/tests/workflow_settings_round_trip.rs index f999aa8aa..c253f217b 100644 --- a/lib/foundation/fabro-api/tests/workflow_settings_round_trip.rs +++ b/lib/foundation/fabro-api/tests/workflow_settings_round_trip.rs @@ -61,6 +61,8 @@ approval = "auto" assert_eq!(json["run"]["goal"]["type"], "inline"); assert_eq!(json["run"]["goal"]["value"], "Ship it"); assert_eq!(json["run"]["execution"]["approval"], "auto"); + assert_eq!(json["run"]["agent"]["fabro_tools"], false); + assert!(json["run"]["agent"].get("permissions").is_none()); assert_eq!( json["run"]["environment"]["image"]["docker"], "buildpack-deps:noble" diff --git a/lib/foundation/fabro-config/src/layers/cli.rs b/lib/foundation/fabro-config/src/layers/cli.rs index b044d2d46..6e401c1bc 100644 --- a/lib/foundation/fabro-config/src/layers/cli.rs +++ b/lib/foundation/fabro-config/src/layers/cli.rs @@ -1,7 +1,8 @@ //! Sparse `[cli]` settings layer definitions. -use fabro_types::settings::cli::{CliAuthStrategy, OutputFormat, OutputVerbosity}; -use fabro_types::settings::run::AgentPermissions; +use fabro_types::settings::cli::{ + AgentPermissions, CliAuthStrategy, OutputFormat, OutputVerbosity, +}; use serde::{Deserialize, Serialize}; use super::maps::StickyMap; diff --git a/lib/foundation/fabro-config/src/layers/combine.rs b/lib/foundation/fabro-config/src/layers/combine.rs index abfe83ae1..ec8d624a0 100644 --- a/lib/foundation/fabro-config/src/layers/combine.rs +++ b/lib/foundation/fabro-config/src/layers/combine.rs @@ -1,10 +1,11 @@ use std::collections::{BTreeMap, HashMap}; use fabro_model::{AgentProfileKind, BillingPolicy, CodecKind, ProviderAuthConfig}; -use fabro_types::settings::cli::{CliAuthStrategy, OutputFormat, OutputVerbosity}; +use fabro_types::settings::cli::{ + AgentPermissions, CliAuthStrategy, OutputFormat, OutputVerbosity, +}; use fabro_types::settings::run::{ - AgentPermissions, ApprovalMode, EnvironmentNetworkMode, EnvironmentProvider, MergeStrategy, - RunMode, + ApprovalMode, EnvironmentNetworkMode, EnvironmentProvider, MergeStrategy, RunMode, }; use fabro_types::settings::server::{ GithubIntegrationStrategy, LogDestination, ObjectStoreProvider, ServerAuthMethod, diff --git a/lib/foundation/fabro-config/src/layers/run.rs b/lib/foundation/fabro-config/src/layers/run.rs index 853ba7a0c..8ac7135b0 100644 --- a/lib/foundation/fabro-config/src/layers/run.rs +++ b/lib/foundation/fabro-config/src/layers/run.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use fabro_types::settings::run::{ - AgentPermissions, ApprovalMode, HookEvent, McpHttpProtocol, MergeStrategy, RunMode, + ApprovalMode, HookEvent, McpHttpProtocol, MergeStrategy, RunMode, }; use fabro_types::settings::{Duration, InterpString, ModelRef}; use serde::de::{self, Deserializer}; @@ -390,7 +390,7 @@ pub struct InterviewProviderLayer { pub channel: Option, } -/// `[run.agent]` — agent knobs only (Fabro tools, permissions, MCPs). +/// `[run.agent]` — agent knobs only (Fabro tools and MCPs). #[derive( Debug, Clone, @@ -408,14 +408,6 @@ pub struct RunAgentLayer { #[option(default = "false", value_type = "boolean")] pub fabro_tools: Option, - /// Default tool permission level for workflow agents. - #[serde(default, skip_serializing_if = "Option::is_none")] - #[option( - default = "\"read-write\"", - value_type = "\"read-only\" | \"read-write\" | \"full\"" - )] - pub permissions: Option, - /// Agent-scoped MCP server entries, keyed by name. #[serde(default, skip_serializing_if = "StickyMap::is_empty")] #[option(value_type = "table")] diff --git a/lib/foundation/fabro-config/src/resolve/run.rs b/lib/foundation/fabro-config/src/resolve/run.rs index 1d20db7a6..158129ebb 100644 --- a/lib/foundation/fabro-config/src/resolve/run.rs +++ b/lib/foundation/fabro-config/src/resolve/run.rs @@ -318,7 +318,6 @@ fn resolve_agent( RunAgentSettings { fabro_tools: agent.fabro_tools.unwrap_or(false), - permissions: agent.permissions, mcps: resolve_mcp_entries(&agent.mcps, mcp_server_catalog, errors), } } diff --git a/lib/foundation/fabro-config/src/tests/resolve_cli.rs b/lib/foundation/fabro-config/src/tests/resolve_cli.rs index 58cfe8cb6..838a0c5c1 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_cli.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_cli.rs @@ -3,8 +3,9 @@ reason = "sync test fixture setup; not on a Tokio path" )] -use fabro_types::settings::cli::{CliTargetSettings, OutputFormat, OutputVerbosity}; -use fabro_types::settings::run::AgentPermissions; +use fabro_types::settings::cli::{ + AgentPermissions, CliTargetSettings, OutputFormat, OutputVerbosity, +}; use temp_env::with_var; use crate::{SettingsLayer, UserSettingsBuilder}; diff --git a/lib/foundation/fabro-config/src/tests/resolve_run.rs b/lib/foundation/fabro-config/src/tests/resolve_run.rs index dff845e04..9958e3d60 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_run.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_run.rs @@ -898,6 +898,23 @@ mod run_agent_fabro_tools { assert!(!settings.agent.fabro_tools); } + #[test] + fn rejects_removed_permissions_setting() { + let err = r#" +_version = 1 + +[run.agent] +permissions = "read-only" +"# + .parse::() + .expect_err("removed run.agent.permissions must be unknown"); + let message = err.to_string(); + assert!( + message.contains("permissions") || message.contains("unknown field"), + "expected unknown-field error mentioning permissions, got: {message}" + ); + } + #[test] fn resolves_true_from_run_agent_table() { let settings = super::workflow_settings_from_toml( diff --git a/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs b/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs index 2422b47bd..2c1f85476 100644 --- a/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs +++ b/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs @@ -129,9 +129,8 @@ enabled = true", ), Section::of::( "[run.agent]", - r#"[run.agent] -fabro_tools = true -permissions = "read-write""#, + r"[run.agent] +fabro_tools = true", ), ] } diff --git a/lib/foundation/fabro-types/src/settings/cli.rs b/lib/foundation/fabro-types/src/settings/cli.rs index 38a5dae63..7c85d8714 100644 --- a/lib/foundation/fabro-types/src/settings/cli.rs +++ b/lib/foundation/fabro-types/src/settings/cli.rs @@ -9,7 +9,7 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use super::run::{AgentPermissions, McpServerSettings}; +use super::run::McpServerSettings; /// A structurally resolved `[cli]` view for consumers. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] @@ -54,6 +54,14 @@ pub struct CliExecAgentSettings { pub mcps: Option>, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum AgentPermissions { + ReadOnly, + ReadWrite, + Full, +} + #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct CliOutputSettings { pub format: OutputFormat, diff --git a/lib/foundation/fabro-types/src/settings/run.rs b/lib/foundation/fabro-types/src/settings/run.rs index c5f62b480..9011f0210 100644 --- a/lib/foundation/fabro-types/src/settings/run.rs +++ b/lib/foundation/fabro-types/src/settings/run.rs @@ -1257,7 +1257,6 @@ pub struct InterviewProviderSettings { pub struct RunAgentSettings { #[serde(default)] pub fabro_tools: bool, - pub permissions: Option, pub mcps: HashMap, } @@ -1351,7 +1350,6 @@ mod run_agent_settings_tests { #[test] fn deserializes_missing_fabro_tools_as_false() { let settings: RunAgentSettings = serde_json::from_value(serde_json::json!({ - "permissions": null, "mcps": {} })) .expect("legacy run agent settings should deserialize"); @@ -1367,7 +1365,6 @@ mod run_agent_settings_tests { fn deserializes_old_format_bare_mcps_as_resolved_json() { let settings: RunAgentSettings = serde_json::from_value(serde_json::json!({ "fabro_tools": true, - "permissions": null, "mcps": { "filesystem": { "name": "filesystem", @@ -2282,14 +2279,6 @@ pub enum ApprovalMode { Auto, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub enum AgentPermissions { - ReadOnly, - ReadWrite, - Full, -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, strum::Display)] #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index d0ea6bd28..388c35617 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -31,7 +31,6 @@ models/activated-skill.ts models/agent-control-state.ts models/agent-mcp-tool-summary.ts models/agent-message-props.ts -models/agent-permissions.ts models/agent-session-activated-props.ts models/agent-skill-activation-source.ts models/agent-skill-summary.ts diff --git a/lib/packages/fabro-api-client/src/models/agent-permissions.ts b/lib/packages/fabro-api-client/src/models/agent-permissions.ts deleted file mode 100644 index 3880585a3..000000000 --- a/lib/packages/fabro-api-client/src/models/agent-permissions.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* tslint:disable */ -/* eslint-disable */ -/** - * Fabro Run API - * HTTP API for managing Fabro workflow run executions. - * - * The version of the OpenAPI document: 0.1.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - - - -export const AgentPermissions = { - READ_ONLY: 'read-only', - READ_WRITE: 'read-write', - FULL: 'full' -} as const; - -export type AgentPermissions = typeof AgentPermissions[keyof typeof AgentPermissions]; diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index ac6fc24a4..5a7909430 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -2,7 +2,6 @@ export * from './activated-skill'; export * from './agent-control-state'; export * from './agent-mcp-tool-summary'; export * from './agent-message-props'; -export * from './agent-permissions'; export * from './agent-session-activated-props'; export * from './agent-skill-activation-source'; export * from './agent-skill-summary'; diff --git a/lib/packages/fabro-api-client/src/models/run-agent-settings.ts b/lib/packages/fabro-api-client/src/models/run-agent-settings.ts index 69b9672bc..044c251c8 100644 --- a/lib/packages/fabro-api-client/src/models/run-agent-settings.ts +++ b/lib/packages/fabro-api-client/src/models/run-agent-settings.ts @@ -13,14 +13,11 @@ */ -// May contain unused imports in some cases -// @ts-ignore -import type { AgentPermissions } from './agent-permissions'; // May contain unused imports in some cases // @ts-ignore import type { McpServerSettings } from './mcp-server-settings'; export interface RunAgentSettings { - 'permissions': AgentPermissions | null; + 'fabro_tools': boolean; 'mcps': { [key: string]: McpServerSettings; }; }