fix(workflow): ignore deprecated project directory

Project workflows now resolve from the discovered .fabro directory instead of honoring project.directory. Keep the legacy field parse-only while removing it from resolved settings and API/client shapes.
This commit is contained in:
Bryan Helmkamp 2026-05-09 10:50:39 -04:00
parent dae5ea4c80
commit 5b0b1efdc2
No known key found for this signature in database
21 changed files with 105 additions and 214 deletions

View file

@ -44,7 +44,6 @@ function sampleSettings({
project: {
name: null,
description: null,
directory: ".",
metadata: {},
},
workflow: {

View file

@ -7894,14 +7894,12 @@ components:
ProjectNamespace:
type: object
required: [name, description, directory, metadata]
required: [name, description, metadata]
properties:
name:
type: ["string", "null"]
description:
type: ["string", "null"]
directory:
type: string
metadata:
$ref: "#/components/schemas/StringMap"

View file

@ -1,33 +1,31 @@
# Fixed Project Workflow Directory Plan
**Summary**
Remove the `[project].directory` feature entirely so project workflows are always discovered, created, listed, and resolved from `<repo_root>/.fabro/workflows/*`. Because this is greenfield, configs that still contain `[project] directory = ...` should fail schema validation instead of being ignored or migrated. User-level workflows under `~/.fabro/workflows` remain unchanged as an additional fallback/list section.
Remove the `[project].directory` feature as an effective setting so project workflows are always discovered, created, listed, and resolved from `<repo_root>/.fabro/workflows/*`. Configs that still contain `[project] directory = ...` should continue parsing, but the field is deprecated and ignored. User-level workflows under `~/.fabro/workflows` remain unchanged as an additional fallback/list section.
**Key Changes**
- Remove `directory` from the project config schema and resolved settings type:
- Delete `ProjectLayer::directory` and `ProjectNamespace::directory`.
- Remove `directory` from the resolved settings/API shape while tolerating old config files:
- Keep `ProjectLayer::directory` as a deprecated parse-only field so existing `project.toml` files with the field do not fail schema validation.
- Delete `ProjectNamespace::directory`.
- Remove `[project] directory = "."` from built-in defaults.
- Update project resolver/tests so `[project]` only carries `name`, `description`, and `metadata`.
- Update project resolver, `fabro-types` fixtures, OpenAPI schema, and generated TypeScript client so resolved `[project]` settings only carry `name`, `description`, and `metadata`.
- Make project Fabro root fixed:
- Change `resolve_fabro_root` to validate `.fabro/project.toml`, then return `config_path.parent()` directly.
- Update callers to handle `Result<PathBuf>` and use `<discovered .fabro>/workflows`.
- Delete now-unused path normalization code for joining `project.directory`.
- Update public API shape:
- Remove `directory` from `ProjectNamespace` in `docs/public/api-reference/fabro-api.yaml`.
- Update `workflow_settings_round_trip` expectations.
- Regenerate `lib/packages/fabro-api-client` so TypeScript `ProjectNamespace` no longer has `directory`.
- Delete `resolve_fabro_root`; it only exists to apply `project.directory`.
- Delete `load_project_config` and project-root path normalization code if they have no remaining callers after `resolve_fabro_root` is removed.
- At workflow discovery/create/list call sites, use the discovered config path's parent directory as the Fabro root and append `workflows`.
- Do not add a new project-config validation step for workflow directory discovery; settings validation remains the responsibility of settings-loading paths.
- Update docs and hints:
- Remove references to `project.directory` and the old `fabro.root -> project.directory` rename hint.
- Remove public references to `project.directory` and replace the old `fabro.root -> project.directory` rename hint with guidance that project workflows now live under `.fabro/workflows`.
- Add/keep explicit docs that project workflows live at `<repo_root>/.fabro/workflows/<name>/`.
**Test Plan**
- Config tests:
- Empty settings still resolve project metadata defaults.
- `[project] directory = "..."` is rejected as an unknown field.
- `resolve_fabro_root` always returns the `.fabro` directory containing `project.toml`.
- `[project] directory = "..."` still parses but is ignored in resolved settings.
- Project workflow root calculation uses the `.fabro` directory containing `project.toml`, regardless of any deprecated `project.directory` value.
- CLI integration tests:
- Replace custom-root workflow create/list tests with fixed-root assertions.
- Confirm `fabro workflow create <name>` writes `.fabro/workflows/<name>/workflow.{fabro,toml}`.
- Confirm `fabro workflow create <name>` writes both `.fabro/workflows/<name>/workflow.fabro` and `.fabro/workflows/<name>/workflow.toml`.
- Confirm named workflow resolution reads `.fabro/workflows/<name>/workflow.toml`.
- API/client checks:
- `cargo build -p fabro-api`
@ -36,6 +34,6 @@ Remove the `[project].directory` feature entirely so project workflows are alway
- `cd apps/fabro-web && bun run typecheck`
**Assumptions**
- No backwards compatibility or migration period: old configs with `[project].directory` should fail.
- Backwards compatibility is parse-only: old configs with `[project].directory` should not fail, but the value has no effect and is not exposed in resolved settings or the API.
- This change only fixes the project workflow directory. User workflows remain available unless a separate decision removes them later.
- GitHub retrieval itself is out of scope here; this refactor makes the repo path deterministic for that future work.

View file

@ -32,7 +32,10 @@ approval = "auto"
.expect("settings should resolve");
let json = serde_json::to_value(&settings).expect("workflow settings should serialize");
assert_eq!(json["project"]["directory"], "workspace");
assert!(
json["project"].get("directory").is_none(),
"resolved project settings should not expose deprecated directory"
);
assert_eq!(json["workflow"]["graph"], "ship.fabro");
assert_eq!(json["run"]["goal"]["type"], "inline");
assert_eq!(json["run"]["goal"]["value"], "Ship it");

View file

@ -6,7 +6,7 @@
use std::path::Path;
use anyhow::{Context, Result, bail};
use fabro_config::project::{discover_project_config, resolve_fabro_root};
use fabro_config::project::discover_project_config;
use crate::args::WorkflowCreateArgs;
use crate::command_context::CommandContext;
@ -23,8 +23,10 @@ pub(super) fn create_command(args: &WorkflowCreateArgs, base_ctx: &CommandContex
);
};
let fabro_root = resolve_fabro_root(&config_path);
let created = write_workflow_scaffold(args, &fabro_root)?;
let fabro_root = config_path
.parent()
.expect("project config should have a parent directory");
let created = write_workflow_scaffold(args, fabro_root)?;
if base_ctx.json_output() {
let created: Vec<_> = created.iter().map(|path| relative_path(path)).collect();

View file

@ -3,7 +3,6 @@ use cli_table::format::{Border, Separator};
use cli_table::{Cell, CellStruct, Color, Style, Table};
use fabro_config::project::{
WorkflowInfo, WorkflowSource, discover_project_config, list_workflows_detailed,
resolve_fabro_root,
};
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
@ -26,7 +25,9 @@ pub(super) fn list_command(_args: &WorkflowListArgs, base_ctx: &CommandContext)
);
};
let fabro_root = resolve_fabro_root(&config_path);
let fabro_root = config_path
.parent()
.expect("project config should have a parent directory");
let project_wf_dir = fabro_root.join("workflows");
let user_wf_dir = Some(fabro_util::Home::from_env().workflows_dir());

View file

@ -915,7 +915,6 @@ fn attach_json_errors_without_prompting_for_human_input() {
"settings": {
"project": {
"description": null,
"directory": ".",
"metadata": {},
"name": null
},

View file

@ -39,7 +39,7 @@ fn list() {
"_version = 1\n\n[project]\ndirectory = \"..\"\n",
)
.write_temp(
"workflows/my_test_wf/workflow.toml",
".fabro/workflows/my_test_wf/workflow.toml",
"_version = 1\n\n[run]\ngoal = \"A test workflow\"\n",
);
@ -55,7 +55,7 @@ fn list() {
User Workflows (~/.fabro/workflows)
(none)
Project Workflows (workflows)
Project Workflows (.fabro/workflows)
NAME DESCRIPTION
my_test_wf A test workflow
");

View file

@ -160,7 +160,7 @@ fn workflow_create_errors_without_project_config() {
}
#[test]
fn workflow_create_json_uses_resolved_custom_root_paths() {
fn workflow_create_json_ignores_deprecated_project_directory() {
let context = test_context!();
let project_dir = context.temp_dir.join("project");
context.write_temp(
@ -188,20 +188,24 @@ fn workflow_create_json_uses_resolved_custom_root_paths() {
{
"name": "hello-world",
"created": [
"custom/fabro-data/workflows/hello-world/workflow.fabro",
"custom/fabro-data/workflows/hello-world/workflow.toml"
".fabro/workflows/hello-world/workflow.fabro",
".fabro/workflows/hello-world/workflow.toml"
]
}
"#);
assert!(
project_dir
.join("custom/fabro-data/workflows/hello-world/workflow.fabro")
.join(".fabro/workflows/hello-world/workflow.fabro")
.exists()
);
assert!(
project_dir
.join("custom/fabro-data/workflows/hello-world/workflow.toml")
.join(".fabro/workflows/hello-world/workflow.toml")
.exists()
);
assert!(
!project_dir.join("custom/fabro-data/workflows").exists(),
"deprecated project.directory should not redirect workflow creation"
);
}

View file

@ -1,7 +1,7 @@
use std::fmt;
use std::path::Path;
use fabro_types::settings::{ProjectNamespace, RunNamespace, WorkflowNamespace};
use fabro_types::settings::{RunNamespace, WorkflowNamespace};
use fabro_types::{ServerSettings, UserSettings, WorkflowSettings};
use fabro_util::error::SharedError;
@ -419,15 +419,6 @@ impl WorkflowSettingsBuilder {
)
}
pub(crate) fn project_from_layer(
layer: &SettingsLayer,
) -> std::result::Result<ProjectNamespace, ResolveErrors> {
let layer = layer.clone().combine(DEFAULTS_LAYER.clone());
let mut errors = Vec::new();
let project = resolve_project(&layer.project.clone().unwrap_or_default(), &mut errors);
finish_dense_result(project, errors)
}
pub(crate) fn workflow_from_layer(
layer: &SettingsLayer,
) -> std::result::Result<WorkflowNamespace, ResolveErrors> {

View file

@ -2,9 +2,6 @@
# Dynamic and presence-gated defaults remain in Rust.
_version = 1
[project]
directory = "."
[workflow]
graph = "workflow.fabro"

View file

@ -12,8 +12,8 @@ pub struct ProjectLayer {
pub name: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub description: Option<String>,
/// The Fabro-managed project directory inside the repo. Defaults to
/// `.` after layering when unspecified.
/// Deprecated parse-only field. Project workflows always live under the
/// `.fabro` directory that contains `project.toml`.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub directory: Option<String>,
#[serde(default, skip_serializing_if = "ReplaceMap::is_empty")]

View file

@ -113,7 +113,7 @@ fn rename_hint(key: &str) -> Option<String> {
"artifact_storage" => "rename to `[server.artifacts]`",
"storage_dir" | "data_dir" => "rename to `[server.storage] root`",
"max_concurrent_runs" => "rename to `[server.scheduler]` field",
"fabro" => "rename to `[project]`; `fabro.root` becomes `project.directory`",
"fabro" => "rename to `[project]`; project workflows now live under `.fabro/workflows`",
"git" => "split into `[run.git]` (local git behavior) and `[server.integrations.github]`",
"github" => {
"split into `[server.integrations.github]` (App identity/auth) and \

View file

@ -10,13 +10,12 @@
)]
use std::fmt::Write;
use std::path::{Component, Path, PathBuf};
use std::path::{Path, PathBuf};
use fabro_types::settings::{InterpString, RunNamespace};
use serde::Serialize;
use crate::load::load_settings_path;
use crate::{Error, Result, SettingsLayer, WorkflowSettingsBuilder, run};
use crate::{Error, Result, WorkflowSettingsBuilder, run};
const CONFIG_FILENAME: &str = ".fabro/project.toml";
#[derive(Clone, Debug)]
@ -27,19 +26,6 @@ pub struct WorkflowPathResolution {
pub workflow_slug: Option<String>,
}
/// Load a project config from a file path.
///
/// Goes through [`load_settings_path`] so that relative `run.goal.file`
/// paths are anchored at the directory of `path` at load time.
fn load_project_config(path: &Path) -> Result<SettingsLayer> {
let config = load_settings_path(path)?;
let root = WorkflowSettingsBuilder::project_from_layer(&config)
.map_err(|errors| Error::resolve("Failed to resolve project settings", errors.into()))?
.directory;
tracing::debug!(path = %path.display(), root = %root, "Loaded project config");
Ok(config)
}
/// Walk ancestor directories from `start` looking for `.fabro/project.toml`.
/// Returns the config file path, or `None` if not found.
pub fn discover_project_config(start: &Path) -> Result<Option<PathBuf>> {
@ -146,7 +132,9 @@ fn resolve_workflow_arg_impl(
let name = arg.to_string_lossy();
match discover_project_config(start_dir) {
Ok(Some(config_path)) => {
let fabro_root = resolve_fabro_root(&config_path);
let fabro_root = config_path
.parent()
.expect("project config should have a parent directory");
let project_candidate = fabro_root
.join("workflows")
.join(&*name)
@ -334,40 +322,6 @@ pub fn resolve_workflow(arg: &Path) -> Result<PathBuf> {
Ok(resolution.dot_path)
}
fn normalize_joined_path(base_dir: &Path, reference: &Path) -> PathBuf {
if reference.is_absolute() {
return reference.to_path_buf();
}
let mut normalized = PathBuf::new();
for component in base_dir.join(reference).components() {
match component {
Component::CurDir => {}
Component::Normal(part) => normalized.push(part),
Component::ParentDir => {
normalized.pop();
}
Component::RootDir => normalized.push(Path::new("/")),
Component::Prefix(prefix) => normalized.push(prefix.as_os_str()),
}
}
normalized
}
/// Resolve the fabro root directory from a config file path and its config.
/// The returned path is the config file's parent directory joined with the
/// `project.directory` value (default: `.`).
pub fn resolve_fabro_root(config_path: &Path) -> PathBuf {
let project_dir = config_path
.parent()
.expect("config_path should have a parent directory");
let config = load_project_config(config_path).expect("project config should load");
let root = WorkflowSettingsBuilder::project_from_layer(&config)
.expect("project settings should resolve")
.directory;
normalize_joined_path(project_dir, Path::new(&root))
}
#[cfg(test)]
mod tests {
use std::fs;
@ -386,19 +340,31 @@ mod tests {
#[test]
fn parse_with_project_directory() {
assert_eq!(
WorkflowSettingsBuilder::from_toml(
r#"
r#"
_version = 1
[project]
directory = "custom/"
"#
.parse::<crate::SettingsLayer>()
.unwrap()
.project
.and_then(|project| project.directory),
Some("custom/".to_string())
);
let project = WorkflowSettingsBuilder::from_toml(
r#"
_version = 1
[project]
directory = "custom/"
"#,
)
.unwrap()
.project
.directory,
"custom/"
);
)
.unwrap()
.project;
let json = serde_json::to_value(&project).expect("project settings should serialize");
assert!(json.get("directory").is_none());
}
#[test]
@ -425,17 +391,6 @@ directory = "custom/"
);
}
#[test]
fn load_from_disk() {
let tmp = TempDir::new().unwrap();
let config_dir = tmp.path().join(".fabro");
fs::create_dir_all(&config_dir).unwrap();
let path = config_dir.join("project.toml");
fs::write(&path, "_version = 1\n").unwrap();
let config = load_project_config(&path).unwrap();
assert_eq!(config.version, Some(1));
}
#[test]
fn discover_walks_ancestors() {
let tmp = TempDir::new().unwrap();
@ -450,50 +405,14 @@ directory = "custom/"
}
#[test]
fn load_project_config_rewrites_relative_goal_file_path() {
use crate::RunGoalLayer;
let tmp = TempDir::new().unwrap();
let config_dir = tmp.path().join(".fabro");
fs::create_dir_all(&config_dir).unwrap();
let path = config_dir.join("project.toml");
fs::write(
&path,
r#"_version = 1
[run.goal]
file = "prompts/goal.md"
"#,
)
.unwrap();
let config = load_project_config(&path).unwrap();
let Some(RunGoalLayer::File { file }) =
config.run.as_ref().and_then(|run| run.goal.as_ref())
else {
panic!("expected file variant");
};
let expected = config_dir.join("prompts").join("goal.md");
assert_eq!(file.as_source(), expected.to_string_lossy());
}
#[test]
fn default_directory_resolves_to_config_parent() {
let tmp = TempDir::new().unwrap();
let config_dir = tmp.path().join(".fabro");
fs::create_dir_all(&config_dir).unwrap();
let config_path = config_dir.join("project.toml");
fs::write(&config_path, "_version = 1\n").unwrap();
assert_eq!(resolve_fabro_root(&config_path), config_dir);
}
#[test]
fn custom_relative_directory_resolves_from_config_parent() {
fn deprecated_project_directory_does_not_change_fabro_root() {
let tmp = TempDir::new().unwrap();
let config_dir = tmp.path().join(".fabro");
fs::create_dir_all(&config_dir).unwrap();
let config_path = config_dir.join("project.toml");
let workflow_dir = config_dir.join("workflows/demo");
fs::create_dir_all(&workflow_dir).unwrap();
fs::write(workflow_dir.join("workflow.toml"), "_version = 1\n").unwrap();
fs::write(
&config_path,
r#"_version = 1
@ -504,37 +423,9 @@ directory = "../custom"
)
.unwrap();
assert_eq!(resolve_fabro_root(&config_path), tmp.path().join("custom"));
}
#[test]
fn relative_goal_file_resolves_from_config_dir() {
use crate::RunGoalLayer;
let tmp = TempDir::new().unwrap();
let config_dir = tmp.path().join(".fabro");
fs::create_dir_all(&config_dir).unwrap();
let config_path = config_dir.join("project.toml");
fs::write(
&config_path,
r#"_version = 1
[run.goal]
file = "prompts/goal.md"
"#,
)
.unwrap();
let config = load_project_config(&config_path).unwrap();
let Some(RunGoalLayer::File { file }) =
config.run.as_ref().and_then(|run| run.goal.as_ref())
else {
panic!("expected file variant");
};
assert_eq!(
file.as_source(),
config_dir.join("prompts").join("goal.md").to_string_lossy()
resolve_workflow_arg_impl(Path::new("demo"), tmp.path(), None).unwrap(),
config_dir.join("workflows/demo/workflow.toml")
);
}

View file

@ -7,10 +7,6 @@ pub fn resolve_project(layer: &ProjectLayer, _errors: &mut Vec<ResolveError>) ->
ProjectNamespace {
name: layer.name.clone(),
description: layer.description.clone(),
directory: layer
.directory
.clone()
.expect("defaults.toml should provide project.directory"),
metadata: layer.metadata.clone().into_inner(),
}
}

View file

@ -18,12 +18,13 @@ fn embedded_defaults() -> SettingsLayer {
fn embedded_defaults_parse_successfully() {
let defaults = embedded_defaults();
assert_eq!(
assert!(
defaults
.project
.as_ref()
.and_then(|project| project.directory.as_deref()),
Some(".")
.and_then(|project| project.directory.as_deref())
.is_none(),
"built-in defaults should not materialize deprecated project.directory"
);
assert_eq!(
defaults
@ -38,12 +39,13 @@ fn embedded_defaults_parse_successfully() {
fn apply_builtin_defaults_materializes_expected_layer() {
let layer = SettingsLayer::default().combine(embedded_defaults());
assert_eq!(
assert!(
layer
.project
.as_ref()
.and_then(|project| project.directory.as_deref()),
Some(".")
.and_then(|project| project.directory.as_deref())
.is_none(),
"built-in defaults should not materialize deprecated project.directory"
);
assert_eq!(
layer

View file

@ -8,14 +8,18 @@ fn resolves_project_defaults_from_empty_settings() {
.expect("empty settings should resolve")
.project;
assert_eq!(project.directory, ".");
let json = serde_json::to_value(&project).expect("project settings should serialize");
assert!(
json.get("directory").is_none(),
"resolved project settings should not expose deprecated directory"
);
assert!(project.name.is_none());
assert!(project.description.is_none());
assert!(project.metadata.is_empty());
}
#[test]
fn resolves_project_directory_and_metadata() {
fn resolves_project_metadata_and_ignores_deprecated_directory() {
let project = WorkflowSettingsBuilder::from_toml(
r#"
_version = 1
@ -34,7 +38,11 @@ team = "platform"
assert_eq!(project.name.as_deref(), Some("Acme"));
assert_eq!(project.description.as_deref(), Some("Automation"));
assert_eq!(project.directory, ".fabro");
let json = serde_json::to_value(&project).expect("project settings should serialize");
assert!(
json.get("directory").is_none(),
"resolved project settings should not expose deprecated directory"
);
assert_eq!(
project.metadata.get("team").map(String::as_str),
Some("platform")

View file

@ -92,7 +92,12 @@ name = "gpt-5"
WorkflowSettingsBuilder::from_toml(source).expect("workflow settings should resolve");
let server = ServerSettingsBuilder::from_toml(source).expect("server settings should resolve");
assert_eq!(workflow_settings.project.directory, ".fabro");
let project_json = serde_json::to_value(&workflow_settings.project)
.expect("project settings should serialize");
assert!(
project_json.get("directory").is_none(),
"resolved project settings should not expose deprecated directory"
);
assert_eq!(workflow_settings.workflow.graph, "graphs/workflow.dot");
assert_eq!(server.server.storage.root.as_source(), "/srv/fabro");
assert_eq!(
@ -121,7 +126,12 @@ fn workflow_settings_resolve_defaults_and_expose_fields() {
let resolved = fabro_config::WorkflowSettingsBuilder::from_layer(&settings)
.expect("defaults should resolve");
assert_eq!(resolved.project.directory, ".");
let project_json =
serde_json::to_value(&resolved.project).expect("project settings should serialize");
assert!(
project_json.get("directory").is_none(),
"resolved project settings should not expose deprecated directory"
);
assert_eq!(resolved.workflow.graph, "workflow.fabro");
assert_eq!(resolved.run.execution.mode, RunMode::Normal);
}

View file

@ -1629,10 +1629,7 @@ mod runs {
pub(super) fn settings() -> serde_json::Value {
let settings = WorkflowSettings {
project: ProjectNamespace {
directory: "/workspace/api-server".into(),
..ProjectNamespace::default()
},
project: ProjectNamespace::default(),
workflow: WorkflowNamespace {
graph: "workflow.fabro".into(),
..WorkflowNamespace::default()

View file

@ -1,7 +1,4 @@
//! Project domain: first-class project object.
//!
//! `[project]` replaces the old flat `[fabro]` shape. `directory` means the
//! Fabro-managed project directory inside the repo, defaulting to `.`.
use std::collections::HashMap;
@ -12,6 +9,5 @@ use serde::{Deserialize, Serialize};
pub struct ProjectNamespace {
pub name: Option<String>,
pub description: Option<String>,
pub directory: String,
pub metadata: HashMap<String, String>,
}

View file

@ -17,7 +17,6 @@
export interface ProjectNamespace {
'name': string | null;
'description': string | null;
'directory': string;
'metadata': { [key: string]: string; };
}