mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-10 22:43:37 +00:00
Remove unused run-manifest display and provenance metadata
Drop ManifestTarget.identifier (the raw token the user typed) and ManifestGoal.path (the original goal-file path) from the OpenAPI manifest schema, the Rust manifest builder, the regenerated Rust and TypeScript client types, and every canonical test fixture. Neither field had a production reader: the server selects the workflow by target.path and consumes only the resolved goal type and text. Target path, goal type/text, manifest versioning, and submitted-byte persistence are unchanged. Old request bodies that still carry the removed properties remain accepted through unknown-field tolerance, pinned by a dedicated public-route regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
e7fc25f62b
commit
d728ad5508
11 changed files with 102 additions and 55 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -2818,6 +2818,7 @@ dependencies = [
|
|||
"fabro-types",
|
||||
"fabro-workflow",
|
||||
"git2",
|
||||
"serde_json",
|
||||
"temp-env",
|
||||
"tempfile",
|
||||
"toml 0.8.23",
|
||||
|
|
|
|||
|
|
@ -9197,7 +9197,7 @@ components:
|
|||
type: string
|
||||
|
||||
ManifestGoal:
|
||||
description: Resolved goal with provenance.
|
||||
description: Resolved goal kind and content.
|
||||
type: object
|
||||
required:
|
||||
- type
|
||||
|
|
@ -9212,9 +9212,6 @@ components:
|
|||
text:
|
||||
type: string
|
||||
description: Resolved goal content.
|
||||
path:
|
||||
type: ["string", "null"]
|
||||
description: Original goal file path when the goal came from a file.
|
||||
|
||||
ManifestArgs:
|
||||
description: Sparse command-local args that affect run settings.
|
||||
|
|
@ -9248,13 +9245,8 @@ components:
|
|||
ManifestTarget:
|
||||
type: object
|
||||
required:
|
||||
- identifier
|
||||
- path
|
||||
properties:
|
||||
identifier:
|
||||
type: string
|
||||
description: What the user typed.
|
||||
example: smoke
|
||||
path:
|
||||
type: string
|
||||
description: Resolved path that keys into the workflows map.
|
||||
|
|
|
|||
|
|
@ -955,7 +955,6 @@ async fn create_seeded_run(
|
|||
"version": 1,
|
||||
"cwd": context.temp_dir.display().to_string(),
|
||||
"target": {
|
||||
"identifier": target_path,
|
||||
"path": target_path,
|
||||
},
|
||||
"args": args,
|
||||
|
|
|
|||
|
|
@ -1400,8 +1400,7 @@ mod tests {
|
|||
parent_id: None,
|
||||
title: None,
|
||||
target: types::ManifestTarget {
|
||||
identifier: "workflow.fabro".to_string(),
|
||||
path: "workflow.fabro".to_string(),
|
||||
path: "workflow.fabro".to_string(),
|
||||
},
|
||||
version: 1,
|
||||
workflows: HashMap::from([("workflow.fabro".to_string(), types::ManifestWorkflow {
|
||||
|
|
@ -2229,7 +2228,6 @@ id = "local"
|
|||
#[test]
|
||||
fn prepare_manifest_keeps_missing_metadata_names_absent() {
|
||||
let mut manifest = minimal_manifest();
|
||||
manifest.target.identifier = "release-flow".to_string();
|
||||
manifest.workflows.get_mut("workflow.fabro").unwrap().source = r"
|
||||
digraph GraphName {
|
||||
start [shape=Mdiamond]
|
||||
|
|
|
|||
|
|
@ -401,7 +401,6 @@ mod tests {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
|
|
|
|||
|
|
@ -3190,7 +3190,6 @@ fn manifest_json(target_path: &str, dot_source: &str) -> serde_json::Value {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": target_path,
|
||||
"path": target_path,
|
||||
},
|
||||
"workflows": {
|
||||
|
|
@ -3562,6 +3561,60 @@ async fn post_runs_ignores_removed_run_id_input() {
|
|||
assert_ne!(allocated_run_id, submitted_run_id);
|
||||
}
|
||||
|
||||
/// Old clients may still send the removed `target.identifier` and
|
||||
/// `goal.path` manifest properties. The server must keep accepting such
|
||||
/// bodies as unknown fields: the workflow is selected by `target.path`
|
||||
/// and the goal comes from the resolved `goal.text`.
|
||||
#[tokio::test]
|
||||
async fn post_runs_accepts_legacy_manifest_metadata_properties() {
|
||||
let state = test_app_state();
|
||||
let app = crate::test_support::build_test_router(Arc::clone(&state));
|
||||
let picked_dot = r"digraph PickedFlow {
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
start -> exit
|
||||
}";
|
||||
let decoy_dot = r"digraph DecoyFlow {
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
start -> exit
|
||||
}";
|
||||
let manifest = serde_json::json!({
|
||||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
// Legacy display metadata: deliberately names the decoy entry
|
||||
// to prove selection never reads it.
|
||||
"identifier": "decoy.fabro",
|
||||
"path": "picked.fabro",
|
||||
},
|
||||
"goal": {
|
||||
"type": "file",
|
||||
"text": "Goal text from the legacy body",
|
||||
"path": "/tmp/original/goal.md",
|
||||
},
|
||||
"workflows": {
|
||||
"picked.fabro": { "source": picked_dot, "files": {} },
|
||||
"decoy.fabro": { "source": decoy_dot, "files": {} },
|
||||
},
|
||||
});
|
||||
|
||||
let created = post_run_manifest(&app, manifest).await;
|
||||
let run_id = created["id"]
|
||||
.as_str()
|
||||
.expect("create response should contain an id")
|
||||
.parse::<RunId>()
|
||||
.expect("create response id should be valid");
|
||||
|
||||
let run_store = state.stores.runs.open_run_reader(&run_id).await.unwrap();
|
||||
let run_state = run_store.state().await.unwrap();
|
||||
assert_eq!(run_state.spec.graph.name, "PickedFlow");
|
||||
assert_eq!(
|
||||
run_state.spec.graph.goal(),
|
||||
"Goal text from the legacy body"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn post_runs_create_regression_keeps_api_behavior_without_automation_metadata() {
|
||||
let state = TestAppStateBuilder::new()
|
||||
|
|
@ -4225,7 +4278,6 @@ async fn validate_endpoint_returns_template_source_coordinates() {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
|
|
@ -11169,7 +11221,6 @@ async fn create_run_keeps_missing_project_and_workflow_names_absent() {
|
|||
"version": 1,
|
||||
"cwd": "/tmp/project",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"configs": [
|
||||
|
|
@ -13366,7 +13417,6 @@ async fn get_graph_returns_svg() {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
|
|
@ -13426,7 +13476,6 @@ async fn get_graph_source_returns_dot() {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
|
|
@ -13480,7 +13529,6 @@ async fn render_graph_from_manifest_returns_svg() {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
|
|
@ -13538,7 +13586,6 @@ async fn render_graph_from_manifest_accepts_fabro_dotted_attributes() {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
|
|
|
|||
|
|
@ -250,7 +250,6 @@ pub(crate) fn minimal_manifest_json(dot_source: &str) -> serde_json::Value {
|
|||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro"
|
||||
},
|
||||
"workflows": {
|
||||
|
|
|
|||
|
|
@ -25,5 +25,6 @@ git2.workspace = true
|
|||
toml.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
serde_json.workspace = true
|
||||
tempfile = "3"
|
||||
temp-env = "0.3"
|
||||
|
|
|
|||
|
|
@ -252,10 +252,7 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
|
|||
goal,
|
||||
parent_id: None,
|
||||
title: None,
|
||||
target: types::ManifestTarget {
|
||||
identifier: input.workflow.display().to_string(),
|
||||
path: target_key,
|
||||
},
|
||||
target: types::ManifestTarget { path: target_key },
|
||||
version: 1,
|
||||
workflows: context.workflows,
|
||||
},
|
||||
|
|
@ -709,7 +706,6 @@ fn resolve_manifest_goal(
|
|||
)
|
||||
.ok_or_else(|| anyhow!("unsupported manifest goal reference: {reference}"))?;
|
||||
return Ok(Some(types::ManifestGoal {
|
||||
path: Some(reference.to_string()),
|
||||
text: std::fs::read_to_string(&goal_path)
|
||||
.with_context(|| format!("Failed to read {}", goal_path.display()))?,
|
||||
type_: types::ManifestGoalType::Graph,
|
||||
|
|
@ -717,24 +713,21 @@ fn resolve_manifest_goal(
|
|||
}
|
||||
|
||||
Ok(Some(types::ManifestGoal {
|
||||
path: None,
|
||||
text: goal.to_string(),
|
||||
type_: types::ManifestGoalType::Graph,
|
||||
}))
|
||||
}
|
||||
|
||||
/// Translate a [`ResolvedRunGoal`] into the wire-level `ManifestGoal`
|
||||
/// shape. Inline goals get `type = Value`; file-sourced goals keep their
|
||||
/// absolute path as the `path` field and use `type = File`.
|
||||
/// shape. Inline goals get `type = Value`; file-sourced goals carry their
|
||||
/// already-resolved contents with `type = File`.
|
||||
fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal {
|
||||
match resolved.source {
|
||||
ResolvedGoalSource::Inline => types::ManifestGoal {
|
||||
path: None,
|
||||
text: resolved.text,
|
||||
type_: types::ManifestGoalType::Value,
|
||||
},
|
||||
ResolvedGoalSource::File { path } => types::ManifestGoal {
|
||||
path: Some(path.to_string_lossy().into_owned()),
|
||||
ResolvedGoalSource::File { .. } => types::ManifestGoal {
|
||||
text: resolved.text,
|
||||
type_: types::ManifestGoalType::File,
|
||||
},
|
||||
|
|
@ -1118,8 +1111,8 @@ mod tests {
|
|||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
built.manifest.target.path,
|
||||
".fabro/workflows/demo/workflow.fabro"
|
||||
serde_json::to_value(&built.manifest.target).unwrap(),
|
||||
serde_json::json!({ "path": ".fabro/workflows/demo/workflow.fabro" })
|
||||
);
|
||||
assert_eq!(built.manifest.workflows.len(), 2);
|
||||
let root = &built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"];
|
||||
|
|
@ -1139,7 +1132,10 @@ mod tests {
|
|||
root.files
|
||||
.contains_key(".fabro/workflows/demo/prompts/lint.md")
|
||||
);
|
||||
assert_eq!(built.manifest.goal.unwrap().text, "ship it");
|
||||
assert_eq!(
|
||||
serde_json::to_value(built.manifest.goal.as_ref().unwrap()).unwrap(),
|
||||
serde_json::json!({ "type": "graph", "text": "ship it" })
|
||||
);
|
||||
assert!(
|
||||
built
|
||||
.manifest
|
||||
|
|
@ -1573,11 +1569,10 @@ file = "prompts/goal.md"
|
|||
.unwrap();
|
||||
|
||||
let goal = built.manifest.goal.expect("manifest goal should be set");
|
||||
assert_eq!(goal.text, "ship from project root");
|
||||
assert_eq!(goal.type_, types::ManifestGoalType::File);
|
||||
let resolved = goal.path.expect("file goal must carry a path");
|
||||
let expected = project.join(".fabro").join("prompts").join("goal.md");
|
||||
assert_eq!(PathBuf::from(resolved), expected);
|
||||
assert_eq!(
|
||||
serde_json::to_value(&goal).unwrap(),
|
||||
serde_json::json!({ "type": "file", "text": "ship from project root" })
|
||||
);
|
||||
}
|
||||
|
||||
/// A relative `[run.goal] file = "..."` declared in `workflow.toml`
|
||||
|
|
@ -1623,11 +1618,35 @@ file = "prompts/goal.md"
|
|||
.unwrap();
|
||||
|
||||
let goal = built.manifest.goal.expect("manifest goal should be set");
|
||||
assert_eq!(goal.text, "ship from workflow dir");
|
||||
assert_eq!(goal.type_, types::ManifestGoalType::File);
|
||||
let resolved = goal.path.expect("file goal must carry a path");
|
||||
let expected = workflow_dir.join("prompts").join("goal.md");
|
||||
assert_eq!(PathBuf::from(resolved), expected);
|
||||
assert_eq!(
|
||||
serde_json::to_value(&goal).unwrap(),
|
||||
serde_json::json!({ "type": "file", "text": "ship from workflow dir" })
|
||||
);
|
||||
}
|
||||
|
||||
/// The wire-level goal carries only the resolved kind and content:
|
||||
/// inline and file-sourced goals serialize to exactly `type` + `text`.
|
||||
#[test]
|
||||
fn resolved_goals_serialize_type_and_text_only() {
|
||||
let inline = resolved_goal_to_manifest(ResolvedRunGoal {
|
||||
text: "inline goal".to_string(),
|
||||
source: ResolvedGoalSource::Inline,
|
||||
});
|
||||
assert_eq!(
|
||||
serde_json::to_value(&inline).unwrap(),
|
||||
serde_json::json!({ "type": "value", "text": "inline goal" })
|
||||
);
|
||||
|
||||
let file = resolved_goal_to_manifest(ResolvedRunGoal {
|
||||
text: "goal from file".to_string(),
|
||||
source: ResolvedGoalSource::File {
|
||||
path: PathBuf::from("/tmp/project/goal.md"),
|
||||
},
|
||||
});
|
||||
assert_eq!(
|
||||
serde_json::to_value(&file).unwrap(),
|
||||
serde_json::json!({ "type": "file", "text": "goal from file" })
|
||||
);
|
||||
}
|
||||
|
||||
/// When `[run] working_dir` points to a nested git repo, the manifest's
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@
|
|||
|
||||
|
||||
/**
|
||||
* Resolved goal with provenance.
|
||||
* Resolved goal kind and content.
|
||||
*/
|
||||
export interface ManifestGoal {
|
||||
'type': ManifestGoalTypeEnum;
|
||||
|
|
@ -23,10 +23,6 @@ export interface ManifestGoal {
|
|||
* Resolved goal content.
|
||||
*/
|
||||
'text': string;
|
||||
/**
|
||||
* Original goal file path when the goal came from a file.
|
||||
*/
|
||||
'path'?: string | null;
|
||||
}
|
||||
|
||||
export const ManifestGoalTypeEnum = {
|
||||
|
|
|
|||
|
|
@ -15,10 +15,6 @@
|
|||
|
||||
|
||||
export interface ManifestTarget {
|
||||
/**
|
||||
* What the user typed.
|
||||
*/
|
||||
'identifier': string;
|
||||
/**
|
||||
* Resolved path that keys into the workflows map.
|
||||
*/
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue