Remove nonfunctional run agent permissions setting

This commit is contained in:
Bryan Helmkamp 2026-07-29 10:23:00 -04:00
parent d8434e7672
commit de7bb61ef5
No known key found for this signature in database
21 changed files with 53 additions and 85 deletions

View file

@ -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"

View file

@ -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. |

View file

@ -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]

View file

@ -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]`

View file

@ -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.<name>]`

View file

@ -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.

View file

@ -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")]

View file

@ -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"

View file

@ -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;

View file

@ -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,

View file

@ -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<InterpString>,
}
/// `[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<bool>,
/// 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<AgentPermissions>,
/// Agent-scoped MCP server entries, keyed by name.
#[serde(default, skip_serializing_if = "StickyMap::is_empty")]
#[option(value_type = "table")]

View file

@ -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),
}
}

View file

@ -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};

View file

@ -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::<SettingsLayer>()
.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(

View file

@ -129,9 +129,8 @@ enabled = true",
),
Section::of::<fabro_config::RunAgentLayer>(
"[run.agent]",
r#"[run.agent]
fabro_tools = true
permissions = "read-write""#,
r"[run.agent]
fabro_tools = true",
),
]
}

View file

@ -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<HashMap<String, McpServerSettings>>,
}
#[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,

View file

@ -1257,7 +1257,6 @@ pub struct InterviewProviderSettings {
pub struct RunAgentSettings {
#[serde(default)]
pub fabro_tools: bool,
pub permissions: Option<AgentPermissions>,
pub mcps: HashMap<String, ResolvedMcpEntry>,
}
@ -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")]

View file

@ -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

View file

@ -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];

View file

@ -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';

View file

@ -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; };
}