mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Reuse PermissionLevel and fix stale run spec snapshots
AgentPermissions duplicated fabro_types::PermissionLevel: same variants, same kebab-case wire form, same crate. PermissionLevel is strictly richer (Hash, strum, clap::ValueEnum) and is already the with_replacement target for the OpenAPI PermissionLevel schema, whose values are identical to the AgentPermissions schema this branch deletes. Delete AgentPermissions and type the [cli.exec.agent] permissions setting as PermissionLevel. This drops the adapter match in `fabro exec` and the `as AgentPermissionLevel` alias that existed only to tell the two names apart. The TOML wire form is unchanged. Removing run.agent.permissions also changed the serialized run spec, but two fabro-cli inline snapshots still carried "permissions": null. They failed on this branch and passed on main. Accept the updated snapshots. Also tighten the removed-setting test to assert the exact unknown-field message, rename its module to run_agent now that it covers more than fabro_tools, and drop three doc references to the removed setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
de7bb61ef5
commit
ef9606e6ec
11 changed files with 20 additions and 39 deletions
|
|
@ -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` / `settings::v2::cli::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` / `fabro_types::PermissionLevel` 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. |
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ MCP ([Model Context Protocol](https://modelcontextprotocol.io/)) lets you connec
|
|||
|
||||
Fabro can also run as an MCP server. MCP clients can use Fabro's run-management tools to create, inspect, control, wait for, and read events from workflow runs through the authenticated `fabro` CLI.
|
||||
|
||||
Workflow agents can opt in to that same run-management tool catalog with `[run.agent] fabro_tools = true`. This is not the same as configuring external MCP servers for the agent, and it does not change the agent's normal workspace permissions. When a workflow agent calls `fabro_run_create`, created runs are always [child runs](/execution/child-runs) of the current run; an explicit `parent_id` must match the current run ID.
|
||||
Workflow agents can opt in to that same run-management tool catalog with `[run.agent] fabro_tools = true`. This is not the same as configuring external MCP servers for the agent. When a workflow agent calls `fabro_run_create`, created runs are always [child runs](/execution/child-runs) of the current run; an explicit `parent_id` must match the current run ID.
|
||||
|
||||
## Fabro as an MCP server
|
||||
|
||||
|
|
|
|||
|
|
@ -98,7 +98,7 @@ cargo nextest run -p fabro-cli --test it runner
|
|||
|
||||
- [ ] Add `fabro_tools = true` to the `[run.agent]` generated sample.
|
||||
- [ ] Document that the setting defaults to `false`.
|
||||
- [ ] Document that the setting controls built-in Fabro run-management tools and is separate from ordinary agent `permissions` and `[run.agent.mcps]`.
|
||||
- [ ] Document that the setting controls built-in Fabro run-management tools and is separate from `[run.agent.mcps]`.
|
||||
- [ ] Run:
|
||||
|
||||
```bash
|
||||
|
|
|
|||
|
|
@ -301,19 +301,12 @@ 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::cli::AgentPermissions;
|
||||
|
||||
let cli = &ctx.user_settings().cli;
|
||||
#[cfg(feature = "sleep_inhibitor")]
|
||||
let _sleep_guard = sleep_inhibitor::guard(cli.exec.prevent_idle_sleep);
|
||||
let provider_str = cli.exec.model.provider.as_deref();
|
||||
let model_str = cli.exec.model.name.as_deref();
|
||||
let permissions = cli.exec.agent.permissions.map(|p| match p {
|
||||
AgentPermissions::ReadOnly => AgentPermissionLevel::ReadOnly,
|
||||
AgentPermissions::ReadWrite => AgentPermissionLevel::ReadWrite,
|
||||
AgentPermissions::Full => AgentPermissionLevel::Full,
|
||||
});
|
||||
let permissions = cli.exec.agent.permissions;
|
||||
let output_format = Some(match cli.output.format {
|
||||
SettingsOutputFormat::Text => OutputFormat::Text,
|
||||
SettingsOutputFormat::Json => OutputFormat::Json,
|
||||
|
|
|
|||
|
|
@ -927,8 +927,7 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
"run": {
|
||||
"agent": {
|
||||
"fabro_tools": false,
|
||||
"mcps": {},
|
||||
"permissions": null
|
||||
"mcps": {}
|
||||
},
|
||||
"artifacts": {
|
||||
"include": []
|
||||
|
|
|
|||
|
|
@ -193,7 +193,6 @@ fn inspect_resolves_selector_via_server_endpoint() {
|
|||
},
|
||||
"agent": {
|
||||
"fabro_tools": false,
|
||||
"permissions": null,
|
||||
"mcps": {}
|
||||
},
|
||||
"hooks": [],
|
||||
|
|
|
|||
|
|
@ -1,8 +1,7 @@
|
|||
//! Sparse `[cli]` settings layer definitions.
|
||||
|
||||
use fabro_types::settings::cli::{
|
||||
AgentPermissions, CliAuthStrategy, OutputFormat, OutputVerbosity,
|
||||
};
|
||||
use fabro_types::PermissionLevel;
|
||||
use fabro_types::settings::cli::{CliAuthStrategy, OutputFormat, OutputVerbosity};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::maps::StickyMap;
|
||||
|
|
@ -112,7 +111,7 @@ pub struct CliExecAgentLayer {
|
|||
default = "\"read-write\"",
|
||||
value_type = "\"read-only\" | \"read-write\" | \"full\""
|
||||
)]
|
||||
pub permissions: Option<AgentPermissions>,
|
||||
pub permissions: Option<PermissionLevel>,
|
||||
/// Agent-scoped MCP entries for `fabro exec`.
|
||||
#[serde(default, skip_serializing_if = "StickyMap::is_empty")]
|
||||
#[option(value_type = "table")]
|
||||
|
|
|
|||
|
|
@ -1,9 +1,8 @@
|
|||
use std::collections::{BTreeMap, HashMap};
|
||||
|
||||
use fabro_model::{AgentProfileKind, BillingPolicy, CodecKind, ProviderAuthConfig};
|
||||
use fabro_types::settings::cli::{
|
||||
AgentPermissions, CliAuthStrategy, OutputFormat, OutputVerbosity,
|
||||
};
|
||||
use fabro_types::PermissionLevel;
|
||||
use fabro_types::settings::cli::{CliAuthStrategy, OutputFormat, OutputVerbosity};
|
||||
use fabro_types::settings::run::{
|
||||
ApprovalMode, EnvironmentNetworkMode, EnvironmentProvider, MergeStrategy, RunMode,
|
||||
};
|
||||
|
|
@ -74,7 +73,7 @@ impl_combine_or_option!(
|
|||
CliAuthStrategy,
|
||||
OutputFormat,
|
||||
OutputVerbosity,
|
||||
AgentPermissions,
|
||||
PermissionLevel,
|
||||
ApprovalMode,
|
||||
HookAgentMarker,
|
||||
HookTlsMode,
|
||||
|
|
@ -309,7 +308,7 @@ mod tests {
|
|||
assert_option_leaf(CliAuthStrategy::None, CliAuthStrategy::Jwt);
|
||||
assert_option_leaf(OutputFormat::Json, OutputFormat::Text);
|
||||
assert_option_leaf(OutputVerbosity::Quiet, OutputVerbosity::Verbose);
|
||||
assert_option_leaf(AgentPermissions::ReadOnly, AgentPermissions::Full);
|
||||
assert_option_leaf(PermissionLevel::ReadOnly, PermissionLevel::Full);
|
||||
assert_option_leaf(ApprovalMode::Auto, ApprovalMode::Prompt);
|
||||
assert_option_leaf(HookAgentMarker::Enabled, HookAgentMarker::Enabled);
|
||||
assert_option_leaf(HookTlsMode::NoVerify, HookTlsMode::Verify);
|
||||
|
|
|
|||
|
|
@ -3,9 +3,8 @@
|
|||
reason = "sync test fixture setup; not on a Tokio path"
|
||||
)]
|
||||
|
||||
use fabro_types::settings::cli::{
|
||||
AgentPermissions, CliTargetSettings, OutputFormat, OutputVerbosity,
|
||||
};
|
||||
use fabro_types::PermissionLevel;
|
||||
use fabro_types::settings::cli::{CliTargetSettings, OutputFormat, OutputVerbosity};
|
||||
use temp_env::with_var;
|
||||
|
||||
use crate::{SettingsLayer, UserSettingsBuilder};
|
||||
|
|
@ -126,7 +125,7 @@ level = "debug"
|
|||
assert!(cli.exec.prevent_idle_sleep);
|
||||
assert_eq!(cli.exec.model.provider.as_deref(), Some("openai"));
|
||||
assert_eq!(cli.exec.model.name.as_deref(), Some("gpt-5"));
|
||||
assert_eq!(cli.exec.agent.permissions, Some(AgentPermissions::ReadOnly));
|
||||
assert_eq!(cli.exec.agent.permissions, Some(PermissionLevel::ReadOnly));
|
||||
assert_eq!(cli.exec.agent.mcps.as_ref().unwrap()["fs"].name, "fs");
|
||||
assert_eq!(cli.output.format, OutputFormat::Json);
|
||||
assert_eq!(cli.output.verbosity, OutputVerbosity::Verbose);
|
||||
|
|
|
|||
|
|
@ -879,7 +879,7 @@ issues = "{{ env.GH_PERM_LEVEL }}"
|
|||
}
|
||||
}
|
||||
|
||||
mod run_agent_fabro_tools {
|
||||
mod run_agent {
|
||||
use crate::SettingsLayer;
|
||||
use crate::layers::Combine;
|
||||
|
||||
|
|
@ -910,8 +910,8 @@ permissions = "read-only"
|
|||
.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}"
|
||||
message.contains("unknown field `permissions`"),
|
||||
"expected unknown-field error for run.agent.permissions, got: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ use std::collections::HashMap;
|
|||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::run::McpServerSettings;
|
||||
use crate::session::PermissionLevel;
|
||||
|
||||
/// A structurally resolved `[cli]` view for consumers.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
|
|
@ -49,19 +50,11 @@ pub struct CliExecModelSettings {
|
|||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct CliExecAgentSettings {
|
||||
pub permissions: Option<AgentPermissions>,
|
||||
pub permissions: Option<PermissionLevel>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue