From 3d77d371c662569cbd8e23937dd7401f617a1e3b Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Mon, 3 Aug 2026 19:27:33 -0400 Subject: [PATCH 1/2] Remove the recorded pre-run push outcome, keep the push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manifest builder's best-effort pre-run push converted every result into a PreRunPushOutcome that was serialized into GitContext, expanded into five OpenAPI union arms, and generated into API clients — but no production path ever read it; every field read was a test. Delete the concept while preserving the behavior: - Drop the PreRunPushOutcome enum and GitContext.push_outcome from fabro-types; GitContext keeps origin_url, branch, optional sha, and dirty, which remain real execution inputs and provenance. - Rename the manifest outcome builder to push_manifest_branch_best_effort, a side-effect-only helper with the same decision rules: skip without an origin, skip on configured-repository mismatch, skip when the branch is already synced, otherwise push noninteractively and discard the result without failing manifest creation or logging raw Git stderr. - Prove the push through repository state instead of the deleted enum: a branch ahead of a local bare origin is pushed during manifest build, a mismatched configured repository is not, and a failing remote helper still cannot fail manifest creation. - Remove push_outcome from GitContext in OpenAPI, delete the five-arm union schemas, and drop the fabro-api type replacement and re-export. - Keep one regression proving historical run.created events with a nested push_outcome still deserialize through ordinary unknown-field tolerance and reserialize to the reduced shape. No migration or event rewrite. Old JSON carrying the removed field stays readable. Newly generated clients omit a field older servers required, so new-client-to-old-server compatibility is intentionally not promised for this pre-1.0 contract. Co-Authored-By: Claude Fable 5 --- docs/internal/events.md | 24 ++-- docs/public/api-reference/fabro-api.yaml | 91 ------------- lib/apps/fabro-cli/tests/it/cmd/support.rs | 10 -- .../src/automation_materializer.rs | 14 +- lib/apps/fabro-server/src/run_manifest.rs | 9 +- lib/apps/fabro-server/src/server/tests.rs | 12 +- lib/components/fabro-dump/src/lib.rs | 9 +- lib/components/fabro-manifest/src/lib.rs | 121 +++++++++++------ lib/components/fabro-store/src/slate/mod.rs | 9 +- .../tests/serializable_projection.rs | 9 +- .../fabro-workflow/src/operations/create.rs | 18 ++- .../fabro-workflow/src/operations/fork.rs | 9 +- .../fabro-workflow/src/operations/retry.rs | 13 +- .../src/pipeline/execute/tests.rs | 9 +- .../fabro-workflow/src/pipeline/initialize.rs | 9 +- .../fabro-workflow/src/pipeline/persist.rs | 9 +- .../src/pipeline/pull_request.rs | 18 ++- .../fabro-workflow/src/run_lookup.rs | 9 +- .../fabro-workflow/src/run_metadata.rs | 22 ++- lib/foundation/fabro-api/build.rs | 1 - lib/foundation/fabro-api/src/lib.rs | 17 ++- .../fabro-api/tests/git_context_round_trip.rs | 50 ++++--- .../tests/pre_run_push_outcome_round_trip.rs | 128 ------------------ lib/foundation/fabro-types/src/lib.rs | 2 +- lib/foundation/fabro-types/src/run.rs | 29 +--- .../fabro-types/tests/run_event_serde.rs | 48 ++++++- .../fabro-types/tests/run_spec_methods.rs | 14 +- .../fabro-types/tests/run_spec_serde.rs | 16 +-- 28 files changed, 256 insertions(+), 473 deletions(-) delete mode 100644 lib/foundation/fabro-api/tests/pre_run_push_outcome_round_trip.rs diff --git a/docs/internal/events.md b/docs/internal/events.md index 837f558544..8afce1bbb3 100644 --- a/docs/internal/events.md +++ b/docs/internal/events.md @@ -45,16 +45,11 @@ Emitted when the run record is created. "properties": { "workflow_slug": "my-workflow", "source_directory": "/home/user/src/my-project", - "repo_origin_url": "https://github.com/acme/my-project.git", - "base_branch": "main", - "pre_run_git": { - "display_base_sha": "abc123", - "local_dirty": "clean", - "push_outcome": { - "type": "succeeded", - "remote": "origin", - "branch": "main" - } + "git": { + "origin_url": "https://github.com/acme/my-project", + "branch": "main", + "sha": "abc123", + "dirty": "clean" }, "fork_source_ref": null, "in_place": false, @@ -80,18 +75,17 @@ Emitted when the run record is created. | `workflow_source` | string? | Workflow source text | | `labels` | object | Run labels | | `source_directory` | string? | Submitter-side source directory | -| `repo_origin_url` | string? | Normalized repository origin URL used by clone-based sandboxes | -| `base_branch` | string? | Submitter-side base branch | | `workflow_slug` | string? | Workflow slug | | `provenance` | object | Actor and request provenance | | `manifest_blob` | string? | Blob id for the submitted manifest | -| `pre_run_git` | object? | Submitter-side pre-run git context and push outcome | +| `git` | object? | Git provenance observed before the run: normalized `origin_url`, `branch`, optional `sha`, and `dirty` status | | `fork_source_ref` | object? | Source run/checkpoint reference when this run was forked | | `in_place` | boolean | Whether the run was created with `--in-place` (no git checkpoints) | Readers remain tolerant of the legacy `workflow_config`, `run_dir`, and -`db_prefix` properties when replaying historical events, but newly emitted -`run.created` events omit them. +`db_prefix` properties, and of a legacy `push_outcome` object nested inside +`git`, when replaying historical events; newly emitted `run.created` events +omit them. ### `run.started` diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index d21a439a07..4426790d98 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -9089,7 +9089,6 @@ components: - origin_url - branch - dirty - - push_outcome properties: origin_url: type: string @@ -9105,96 +9104,6 @@ components: example: abc123def dirty: $ref: "#/components/schemas/DirtyStatus" - push_outcome: - $ref: "#/components/schemas/PreRunPushOutcome" - - PreRunPushOutcome: - description: Outcome of the CLI's best-effort pre-run push. - oneOf: - - $ref: "#/components/schemas/PreRunPushOutcomeNotAttempted" - - $ref: "#/components/schemas/PreRunPushOutcomeSucceeded" - - $ref: "#/components/schemas/PreRunPushOutcomeFailed" - - $ref: "#/components/schemas/PreRunPushOutcomeSkippedNoRemote" - - $ref: "#/components/schemas/PreRunPushOutcomeSkippedRemoteMismatch" - discriminator: - propertyName: type - mapping: - not_attempted: "#/components/schemas/PreRunPushOutcomeNotAttempted" - succeeded: "#/components/schemas/PreRunPushOutcomeSucceeded" - failed: "#/components/schemas/PreRunPushOutcomeFailed" - skipped_no_remote: "#/components/schemas/PreRunPushOutcomeSkippedNoRemote" - skipped_remote_mismatch: "#/components/schemas/PreRunPushOutcomeSkippedRemoteMismatch" - - PreRunPushOutcomeNotAttempted: - type: object - required: - - type - properties: - type: - type: string - enum: - - not_attempted - - PreRunPushOutcomeSucceeded: - type: object - required: - - type - - remote - - branch - properties: - type: - type: string - enum: - - succeeded - remote: - type: string - branch: - type: string - - PreRunPushOutcomeFailed: - type: object - required: - - type - - remote - - branch - - message - properties: - type: - type: string - enum: - - failed - remote: - type: string - branch: - type: string - message: - type: string - - PreRunPushOutcomeSkippedNoRemote: - type: object - required: - - type - properties: - type: - type: string - enum: - - skipped_no_remote - - PreRunPushOutcomeSkippedRemoteMismatch: - type: object - required: - - type - - remote - - repo_origin_url - properties: - type: - type: string - enum: - - skipped_remote_mismatch - remote: - type: string - repo_origin_url: - type: string ManifestGoal: description: Resolved goal kind and content. diff --git a/lib/apps/fabro-cli/tests/it/cmd/support.rs b/lib/apps/fabro-cli/tests/it/cmd/support.rs index 6470375a37..c5cd167884 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/support.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/support.rs @@ -845,11 +845,6 @@ async fn seed_git_backed_changed_run(context: &TestContext) -> SeededGitRunSetup "branch": "main", "sha": base_sha, "dirty": "clean", - "push_outcome": { - "type": "succeeded", - "remote": "origin", - "branch": "main", - }, })), ) .await; @@ -889,11 +884,6 @@ async fn seed_git_backed_noop_run(context: &TestContext) -> RunSetup { "branch": "main", "sha": base_sha, "dirty": "clean", - "push_outcome": { - "type": "succeeded", - "remote": "origin", - "branch": "main", - }, })), ) .await; diff --git a/lib/apps/fabro-server/src/automation_materializer.rs b/lib/apps/fabro-server/src/automation_materializer.rs index ffd64b5646..b57d53b22a 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -7,7 +7,7 @@ use fabro_api::types::RunManifest; use fabro_automation::{AutomationId, AutomationTarget}; use fabro_config::{EnvironmentLayer, MergeMap}; use fabro_manifest::ManifestBuildInput; -use fabro_types::{DirtyStatus, GitContext, GitHubRepositorySlug, PreRunPushOutcome, RunId}; +use fabro_types::{DirtyStatus, GitContext, GitHubRepositorySlug, RunId}; use fabro_util::error::collect_chain; use tokio::{fs, task}; @@ -199,11 +199,10 @@ fn build_manifest_from_checkout( let mut manifest = built.manifest; manifest.git = Some(GitContext { - origin_url: github_metadata_url(&git_context.repo), - branch: git_context.ref_selector, - sha: Some(git_context.checked_out_sha), - dirty: DirtyStatus::Clean, - push_outcome: PreRunPushOutcome::NotAttempted, + origin_url: github_metadata_url(&git_context.repo), + branch: git_context.ref_selector, + sha: Some(git_context.checked_out_sha), + dirty: DirtyStatus::Clean, }); let submitted_manifest_bytes = serde_json::to_vec(&manifest) .context("failed to serialize materialized run manifest") @@ -299,7 +298,7 @@ mod tests { use std::collections::HashMap; use std::fs; - use fabro_types::{DirtyStatus, PreRunPushOutcome}; + use fabro_types::DirtyStatus; use tempfile::TempDir; use super::*; @@ -367,7 +366,6 @@ mod tests { assert_eq!(git.branch, "release"); assert_eq!(git.sha.as_deref(), Some(sha.as_str())); assert_eq!(git.dirty, DirtyStatus::Clean); - assert_eq!(git.push_outcome, PreRunPushOutcome::NotAttempted); let submitted_manifest: serde_json::Value = serde_json::from_slice(&materialized.submitted_manifest_bytes) .expect("submitted bytes should be a manifest"); diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 01b0e200e5..0ee804a3ad 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1695,11 +1695,10 @@ digraph Demo {{ fn git_context(origin_url: &str, branch: &str) -> types::GitContext { types::GitContext { - origin_url: origin_url.to_string(), - branch: branch.to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: origin_url.to_string(), + branch: branch.to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, } } diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 7c8b5814b9..24c355641e 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -3784,8 +3784,7 @@ layer = "project" "origin_url": "https://github.com/acme/payments.git", "branch": "feature/compiler", "sha": "0123456789abcdef", - "dirty": "clean", - "push_outcome": { "type": "not_attempted" } + "dirty": "clean" }); let manifest: RunManifest = serde_json::from_value(manifest_json).unwrap(); let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); @@ -7054,11 +7053,10 @@ async fn create_completed_run_ready_for_pull_request( ); let git = match (repo_origin_url, base_branch) { (Some(origin), Some(branch)) => Some(fabro_types::GitContext { - origin_url: origin.to_string(), - branch: branch.to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: origin.to_string(), + branch: branch.to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), _ => None, }; diff --git a/lib/components/fabro-dump/src/lib.rs b/lib/components/fabro-dump/src/lib.rs index a47cd283fb..1b3a10a4c9 100644 --- a/lib/components/fabro-dump/src/lib.rs +++ b/lib/components/fabro-dump/src/lib.rs @@ -492,11 +492,10 @@ mod tests { automation: None, source_directory: Some("/tmp/project".to_string()), git: Some(fabro_types::GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::from([("team".to_string(), "platform".to_string())]), provenance: test_support::test_run_provenance(), diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 3bfb92c5c8..eb1cc0dad6 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -24,7 +24,7 @@ use fabro_template::{ }; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode}; -use fabro_types::{DirtyStatus, GitContext, ManifestPath, PreRunPushOutcome, WorkflowSettings}; +use fabro_types::{DirtyStatus, GitContext, ManifestPath, WorkflowSettings}; use fabro_workflow::git::{ GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status, }; @@ -754,7 +754,7 @@ fn build_git_context( .filter(|url| !url.is_empty()) }) .unwrap_or_default(); - let push_outcome = build_manifest_push_outcome( + push_manifest_branch_best_effort( repo_path, &branch, origin_url.as_deref(), @@ -765,7 +765,6 @@ fn build_git_context( branch, sha, dirty, - push_outcome, }) } @@ -798,14 +797,18 @@ fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option, String Some((origin_url, branch)) } -fn build_manifest_push_outcome( +/// Best-effort push of the local branch so clone-based execution can see +/// local commits. A failed push must not fail manifest creation, and the +/// discarded push error may contain raw Git stderr, so it is deliberately +/// neither returned nor logged here. +fn push_manifest_branch_best_effort( repo_path: &Path, branch: &str, origin_url: Option<&str>, configured_repo_origin_url: Option<&str>, -) -> PreRunPushOutcome { +) { let Some(origin_url) = origin_url else { - return PreRunPushOutcome::SkippedNoRemote; + return; }; if let Some(repo_origin_url) = configured_repo_origin_url @@ -814,28 +817,15 @@ fn build_manifest_push_outcome( { let remote = fabro_github::normalize_repo_origin_url(origin_url); if remote != repo_origin_url { - return PreRunPushOutcome::SkippedRemoteMismatch { - remote, - repo_origin_url, - }; + return; } } if !branch_needs_push(repo_path, "origin", branch) { - return PreRunPushOutcome::NotAttempted; + return; } - match push_branch_noninteractive(repo_path, "origin", branch) { - Ok(()) => PreRunPushOutcome::Succeeded { - remote: "origin".to_string(), - branch: branch.to_string(), - }, - Err(err) => PreRunPushOutcome::Failed { - remote: "origin".to_string(), - branch: branch.to_string(), - message: err.to_string(), - }, - } + let _ = push_branch_noninteractive(repo_path, "origin", branch); } fn normalize_absolute_path(base_dir: &Path, reference: &str) -> Option { @@ -1709,19 +1699,58 @@ working_dir = "repos/target" .expect("manifest git info should be detected"); assert_eq!(git.branch, "target-branch"); assert_eq!(git.origin_url, "https://github.com/example/target"); - assert_eq!(git.push_outcome, PreRunPushOutcome::NotAttempted); } + /// A local branch ahead of its origin is pushed as a side effect of + /// building the manifest, so clone-based execution sees local commits. #[test] - fn build_manifest_git_skips_push_when_configured_repository_differs_from_origin() { + fn build_manifest_pushes_local_commits_to_bare_origin() { let temp = tempfile::tempdir().unwrap(); - let workspace = temp.path(); + let workspace = temp.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + let bare_origin = init_bare_origin(temp.path()); - init_git_repo( - workspace, - "feature", - "https://github.com/user/forked-target.git", + init_git_repo(&workspace, "feature", bare_origin.to_str().unwrap()); + + let workflow_dir = workspace.join(".fabro/workflows/demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + std::fs::write(workspace.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), + cwd: workspace.clone(), + environment_defaults: test_environment_defaults(), + ..Default::default() + }) + .unwrap(); + + assert!(built.manifest.git.is_some()); + let local_head = head_sha(&workspace).expect("workspace HEAD should resolve"); + assert_eq!( + bare_remote_branch_sha(&bare_origin, "feature").as_deref(), + Some(local_head.trim()), + "the local branch should be pushed to the bare origin during manifest build", ); + } + + #[test] + fn build_manifest_git_skips_push_when_configured_repository_differs_from_origin() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + let bare_origin = init_bare_origin(temp.path()); + + init_git_repo(&workspace, "feature", bare_origin.to_str().unwrap()); let workflow_dir = workspace.join(".fabro/workflows/demo"); std::fs::create_dir_all(&workflow_dir).unwrap(); @@ -1749,7 +1778,7 @@ repository = "target" let built = build_run_manifest(ManifestBuildInput { workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"), - cwd: workspace.to_path_buf(), + cwd: workspace.clone(), environment_defaults: test_environment_defaults(), ..Default::default() }) @@ -1760,10 +1789,11 @@ repository = "target" .git .expect("manifest git info should be detected"); assert_eq!(git.origin_url, "https://github.com/example/target"); - assert_eq!(git.push_outcome, PreRunPushOutcome::SkippedRemoteMismatch { - remote: "https://github.com/user/forked-target".to_string(), - repo_origin_url: "https://github.com/example/target".to_string(), - }); + assert_eq!( + bare_remote_branch_sha(&bare_origin, "feature"), + None, + "a mismatched configured repository must not be pushed to", + ); } #[cfg(unix)] @@ -1820,13 +1850,9 @@ exit 1 environment_defaults: test_environment_defaults(), ..Default::default() }) - .unwrap(); + .expect("a failed push must not fail manifest creation"); - let git = built - .manifest - .git - .expect("manifest git info should be detected"); - assert!(matches!(git.push_outcome, PreRunPushOutcome::Failed { .. })); + assert!(built.manifest.git.is_some()); }); }); @@ -1859,6 +1885,21 @@ exit 1 run_git(path, &["update-ref", &remote_ref, "HEAD"]); } + fn init_bare_origin(parent: &Path) -> PathBuf { + let bare = parent.join("origin.git"); + std::fs::create_dir_all(&bare).unwrap(); + run_git(&bare, &["init", "--bare", "--quiet"]); + bare + } + + fn bare_remote_branch_sha(bare_path: &Path, branch: &str) -> Option { + let repo = git2::Repository::open_bare(bare_path).expect("bare origin should open"); + repo.find_reference(&format!("refs/heads/{branch}")) + .ok() + .and_then(|reference| reference.target()) + .map(|oid| oid.to_string()) + } + fn run_git(path: &Path, args: &[&str]) { use std::process::Command; let output = Command::new("git") diff --git a/lib/components/fabro-store/src/slate/mod.rs b/lib/components/fabro-store/src/slate/mod.rs index 038b6527ae..2fb0d87f36 100644 --- a/lib/components/fabro-store/src/slate/mod.rs +++ b/lib/components/fabro-store/src/slate/mod.rs @@ -596,11 +596,10 @@ mod tests { manifest_blob: None, definition_blob: None, git: Some(fabro_types::GitContext { - origin_url: "https://github.com/fabro-sh/fabro".to_string(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/fabro-sh/fabro".to_string(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), fork_source_ref: None, } diff --git a/lib/components/fabro-store/tests/serializable_projection.rs b/lib/components/fabro-store/tests/serializable_projection.rs index 28bcf047dd..ef0ed067b4 100644 --- a/lib/components/fabro-store/tests/serializable_projection.rs +++ b/lib/components/fabro-store/tests/serializable_projection.rs @@ -26,11 +26,10 @@ fn sample_run_spec() -> RunSpec { manifest_blob: None, definition_blob: None, git: Some(fabro_types::GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), fork_source_ref: None, } diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index bf10a905c6..59f424e2dc 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -1883,11 +1883,10 @@ reasoning = false title: None, automation: None, git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), fork_source_ref: None, parent_id: None, @@ -2244,11 +2243,10 @@ reasoning = false title: None, automation: None, git: Some(fabro_types::GitContext { - origin_url: "https://github.com/acme/widgets".to_string(), - branch: String::new(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/acme/widgets".to_string(), + branch: String::new(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), fork_source_ref: None, parent_id: None, diff --git a/lib/components/fabro-workflow/src/operations/fork.rs b/lib/components/fabro-workflow/src/operations/fork.rs index a82fc5c65a..7e5e65516f 100644 --- a/lib/components/fabro-workflow/src/operations/fork.rs +++ b/lib/components/fabro-workflow/src/operations/fork.rs @@ -382,11 +382,10 @@ mod tests { provenance: test_support::test_run_provenance(), manifest_blob: None, git: Some(fabro_types::GitContext { - origin_url: "https://github.com/example/repo.git".to_string(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/example/repo.git".to_string(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), fork_source_ref: None, retried_from: None, diff --git a/lib/components/fabro-workflow/src/operations/retry.rs b/lib/components/fabro-workflow/src/operations/retry.rs index d9249ddc8d..27a9df68aa 100644 --- a/lib/components/fabro-workflow/src/operations/retry.rs +++ b/lib/components/fabro-workflow/src/operations/retry.rs @@ -118,8 +118,8 @@ mod tests { use fabro_store::{Database, RunProjectionReducer}; use fabro_types::{ AuthMethod, DirtyStatus, FailureReason, ForkSourceRef, GitContext, Graph, IdpIdentity, - PreRunPushOutcome, Principal, PullRequestLink, RunBlobId, RunRunnableSource, - RunServerProvenance, RunTiming, WorkflowSettings, fixtures, + Principal, PullRequestLink, RunBlobId, RunRunnableSource, RunServerProvenance, RunTiming, + WorkflowSettings, fixtures, }; use object_store::memory::InMemory; @@ -154,11 +154,10 @@ mod tests { fn git_context() -> GitContext { GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: Some("abc123".to_string()), - dirty: DirtyStatus::Clean, - push_outcome: PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: Some("abc123".to_string()), + dirty: DirtyStatus::Clean, } } diff --git a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs index b0bc2659f2..118767be8c 100644 --- a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs @@ -164,11 +164,10 @@ fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunI .to_string(), ), git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::new(), provenance: test_support::test_run_provenance(), diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index e4d526a129..5a7dd60699 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -861,11 +861,10 @@ mod tests { automation: None, source_directory: Some(std::env::current_dir().unwrap().display().to_string()), git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::new(), provenance: test_support::test_run_provenance(), diff --git a/lib/components/fabro-workflow/src/pipeline/persist.rs b/lib/components/fabro-workflow/src/pipeline/persist.rs index 58b647cab0..cc12c3cba8 100644 --- a/lib/components/fabro-workflow/src/pipeline/persist.rs +++ b/lib/components/fabro-workflow/src/pipeline/persist.rs @@ -138,11 +138,10 @@ mod tests { automation: None, source_directory: Some("/tmp/project".to_string()), git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::from([ ("env".to_string(), "test".to_string()), diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index d0b742de02..99dc4cab5c 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -1030,11 +1030,10 @@ mod tests { automation: None, source_directory: Some("/tmp/project".to_string()), git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::new(), provenance: test_support::test_run_provenance(), @@ -1098,11 +1097,10 @@ mod tests { automation: None, source_directory: Some("/tmp/project".to_string()), git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::new(), provenance: test_support::test_run_provenance(), diff --git a/lib/components/fabro-workflow/src/run_lookup.rs b/lib/components/fabro-workflow/src/run_lookup.rs index 7a43f2c379..99e9825fee 100644 --- a/lib/components/fabro-workflow/src/run_lookup.rs +++ b/lib/components/fabro-workflow/src/run_lookup.rs @@ -478,11 +478,10 @@ mod tests { automation: None, source_directory: Some("/tmp/project".to_string()), git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - push_outcome: fabro_types::PreRunPushOutcome::NotAttempted, + origin_url: String::new(), + branch: "main".to_string(), + sha: None, + dirty: fabro_types::DirtyStatus::Clean, }), labels: HashMap::new(), provenance: test_support::test_run_provenance(), diff --git a/lib/components/fabro-workflow/src/run_metadata.rs b/lib/components/fabro-workflow/src/run_metadata.rs index dd36fd373a..74be989dfc 100644 --- a/lib/components/fabro-workflow/src/run_metadata.rs +++ b/lib/components/fabro-workflow/src/run_metadata.rs @@ -537,9 +537,7 @@ mod tests { use std::sync::Arc; use fabro_store::RunProjection; - use fabro_types::{ - DirtyStatus, GitContext, PreRunPushOutcome, RunSpec, WorkflowSettings, test_support, - }; + use fabro_types::{DirtyStatus, GitContext, RunSpec, WorkflowSettings, test_support}; use git2::{ErrorClass, ErrorCode}; use super::*; @@ -634,11 +632,10 @@ mod tests { automation: None, source_directory: Some("/Users/client/project".to_string()), git: Some(GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: None, - dirty: DirtyStatus::Clean, - push_outcome: PreRunPushOutcome::NotAttempted, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: None, + dirty: DirtyStatus::Clean, }), labels: HashMap::new(), provenance: test_support::test_run_provenance(), @@ -670,11 +667,10 @@ mod tests { }, )), pre_run_git: Some(GitContext { - origin_url: origin_url.to_string(), - branch: "main".to_string(), - sha: None, - dirty: DirtyStatus::Clean, - push_outcome: PreRunPushOutcome::NotAttempted, + origin_url: origin_url.to_string(), + branch: "main".to_string(), + sha: None, + dirty: DirtyStatus::Clean, }), fork_source_ref: None, base_branch: None, diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index 8d3a682846..aa98adbbdb 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -576,7 +576,6 @@ fn main() { ), ("SecretType", "fabro_types::SecretType", &[]), ("DiffStats", "fabro_types::DiffStats", &[]), - ("PreRunPushOutcome", "fabro_types::PreRunPushOutcome", &[]), ("DirtyStatus", "fabro_types::DirtyStatus", &[]), ("GitContext", "fabro_types::GitContext", &[]), ( diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index 73a9e39beb..d9609ab10f 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -53,15 +53,14 @@ pub mod types { McpTransportView, Message, PairId, PairMessageId, PairMessageRecord, PairMessageRequest, PairRecord, PairStartRequest, PairStatus, PairTarget, PairTranscriptEntry, PairTranscriptResponse, ParallelBranchId, ParallelBranchResult, PendingInterviewRecord, - PermissionLevel, PreRunPushOutcome, Principal, PullRequest, PullRequestDetails, - PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, PullRequestLink, - PullRequestMeta, PullRequestResponse, QuestionType, ReasoningOutput, RepositoryRef, - ReviewTarget, ReviewTargetKind, Role, Run, RunApproval, RunApprovalState, - RunClientProvenance, RunEvent, RunEventDetailContentKind, RunEventDetailResponse, - RunFailure, RunPairStatusResponse, RunProjection, RunProvenance, RunRunnableSource, - RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, RunSandboxPlan, - RunSandboxRuntime, RunServerProvenance, RunSize, SandboxDetails, SandboxInfo, - SandboxListMeta, SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy, + PermissionLevel, Principal, PullRequest, PullRequestDetails, PullRequestDetailsStatus, + PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse, + QuestionType, ReasoningOutput, RepositoryRef, ReviewTarget, ReviewTargetKind, Role, Run, + RunApproval, RunApprovalState, RunClientProvenance, RunEvent, RunEventDetailContentKind, + RunEventDetailResponse, RunFailure, RunPairStatusResponse, RunProjection, RunProvenance, + RunRunnableSource, RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, + RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, RunSize, SandboxDetails, + SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy, SandboxNetworkPolicyMode, SandboxProviderKind, SandboxProviderLookupError, SandboxResources, SandboxService, SandboxServiceListResponse, SandboxState, SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, SessionDetail, SessionId, diff --git a/lib/foundation/fabro-api/tests/git_context_round_trip.rs b/lib/foundation/fabro-api/tests/git_context_round_trip.rs index e055ba91d4..e07b720abb 100644 --- a/lib/foundation/fabro-api/tests/git_context_round_trip.rs +++ b/lib/foundation/fabro-api/tests/git_context_round_trip.rs @@ -1,17 +1,13 @@ use std::any::{TypeId, type_name}; -use fabro_api::types::{ - DirtyStatus as ApiDirtyStatus, GitContext as ApiGitContext, - PreRunPushOutcome as ApiPreRunPushOutcome, -}; -use fabro_types::{DirtyStatus, GitContext, PreRunPushOutcome}; +use fabro_api::types::{DirtyStatus as ApiDirtyStatus, GitContext as ApiGitContext}; +use fabro_types::{DirtyStatus, GitContext}; use serde_json::json; #[test] fn git_context_reuses_canonical_types() { assert_same_type::(); assert_same_type::(); - assert_same_type::(); } #[test] @@ -33,14 +29,10 @@ fn dirty_status_serializes_with_snake_case_strings() { #[test] fn git_context_with_known_sha_round_trips() { let ctx = GitContext { - origin_url: "https://github.com/acme/widgets".to_string(), - branch: "main".to_string(), - sha: Some("abc123".to_string()), - dirty: DirtyStatus::Clean, - push_outcome: PreRunPushOutcome::Succeeded { - remote: "origin".to_string(), - branch: "main".to_string(), - }, + origin_url: "https://github.com/acme/widgets".to_string(), + branch: "main".to_string(), + sha: Some("abc123".to_string()), + dirty: DirtyStatus::Clean, }; let json = serde_json::to_value(&ctx).unwrap(); assert_eq!( @@ -50,11 +42,6 @@ fn git_context_with_known_sha_round_trips() { "branch": "main", "sha": "abc123", "dirty": "clean", - "push_outcome": { - "type": "succeeded", - "remote": "origin", - "branch": "main", - }, }) ); let round_trip: GitContext = serde_json::from_value(json).unwrap(); @@ -64,16 +51,14 @@ fn git_context_with_known_sha_round_trips() { #[test] fn git_context_omits_absent_sha_on_serialize() { let ctx = GitContext { - origin_url: "https://github.com/acme/widgets".to_string(), - branch: "feature/foo".to_string(), - sha: None, - dirty: DirtyStatus::Unknown, - push_outcome: PreRunPushOutcome::SkippedNoRemote, + origin_url: "https://github.com/acme/widgets".to_string(), + branch: "feature/foo".to_string(), + sha: None, + dirty: DirtyStatus::Unknown, }; let json = serde_json::to_value(&ctx).unwrap(); assert!(json.get("sha").is_none()); assert_eq!(json["dirty"], "unknown"); - assert_eq!(json["push_outcome"]["type"], "skipped_no_remote"); } #[test] @@ -82,12 +67,23 @@ fn git_context_deserializes_when_sha_is_absent() { "origin_url": "https://github.com/acme/widgets", "branch": "main", "dirty": "dirty", - "push_outcome": { "type": "not_attempted" }, })) .unwrap(); assert_eq!(ctx.sha, None); assert_eq!(ctx.dirty, DirtyStatus::Dirty); - assert_eq!(ctx.push_outcome, PreRunPushOutcome::NotAttempted); +} + +#[test] +fn git_context_tolerates_legacy_push_outcome_field() { + let ctx: GitContext = serde_json::from_value(json!({ + "origin_url": "https://github.com/acme/widgets", + "branch": "main", + "dirty": "clean", + "push_outcome": { "type": "not_attempted" }, + })) + .unwrap(); + assert_eq!(ctx.origin_url, "https://github.com/acme/widgets"); + assert_eq!(ctx.dirty, DirtyStatus::Clean); } fn assert_same_type() { diff --git a/lib/foundation/fabro-api/tests/pre_run_push_outcome_round_trip.rs b/lib/foundation/fabro-api/tests/pre_run_push_outcome_round_trip.rs deleted file mode 100644 index 92e2d45f6f..0000000000 --- a/lib/foundation/fabro-api/tests/pre_run_push_outcome_round_trip.rs +++ /dev/null @@ -1,128 +0,0 @@ -use std::any::{TypeId, type_name}; - -use fabro_api::types::PreRunPushOutcome as ApiPreRunPushOutcome; -use fabro_types::PreRunPushOutcome; -use serde_json::json; - -#[test] -fn pre_run_push_outcome_reuses_canonical_type() { - assert_same_type::(); -} - -#[test] -fn singletons_serialize_with_only_a_type_field() { - assert_eq!( - serde_json::to_value(PreRunPushOutcome::NotAttempted).unwrap(), - json!({ "type": "not_attempted" }) - ); - assert_eq!( - serde_json::to_value(PreRunPushOutcome::SkippedNoRemote).unwrap(), - json!({ "type": "skipped_no_remote" }) - ); -} - -#[test] -fn succeeded_carries_remote_and_branch() { - let outcome = PreRunPushOutcome::Succeeded { - remote: "origin".to_string(), - branch: "feature/foo".to_string(), - }; - assert_eq!( - serde_json::to_value(&outcome).unwrap(), - json!({ - "type": "succeeded", - "remote": "origin", - "branch": "feature/foo", - }) - ); -} - -#[test] -fn failed_carries_message_alongside_remote_and_branch() { - let outcome = PreRunPushOutcome::Failed { - remote: "origin".to_string(), - branch: "feature/foo".to_string(), - message: "permission denied".to_string(), - }; - assert_eq!( - serde_json::to_value(&outcome).unwrap(), - json!({ - "type": "failed", - "remote": "origin", - "branch": "feature/foo", - "message": "permission denied", - }) - ); -} - -#[test] -fn skipped_remote_mismatch_carries_remote_and_repo_origin_url() { - let outcome = PreRunPushOutcome::SkippedRemoteMismatch { - remote: "git@github.com:user/fork.git".to_string(), - repo_origin_url: "https://github.com/acme/canonical.git".to_string(), - }; - assert_eq!( - serde_json::to_value(&outcome).unwrap(), - json!({ - "type": "skipped_remote_mismatch", - "remote": "git@github.com:user/fork.git", - "repo_origin_url": "https://github.com/acme/canonical.git", - }) - ); -} - -#[test] -fn deserializes_each_variant_from_discriminator_payloads() { - let not_attempted: PreRunPushOutcome = - serde_json::from_value(json!({ "type": "not_attempted" })).unwrap(); - assert_eq!(not_attempted, PreRunPushOutcome::NotAttempted); - - let succeeded: PreRunPushOutcome = serde_json::from_value(json!({ - "type": "succeeded", - "remote": "origin", - "branch": "main", - })) - .unwrap(); - assert_eq!(succeeded, PreRunPushOutcome::Succeeded { - remote: "origin".to_string(), - branch: "main".to_string(), - }); - - let failed: PreRunPushOutcome = serde_json::from_value(json!({ - "type": "failed", - "remote": "origin", - "branch": "main", - "message": "denied", - })) - .unwrap(); - assert_eq!(failed, PreRunPushOutcome::Failed { - remote: "origin".to_string(), - branch: "main".to_string(), - message: "denied".to_string(), - }); - - let skipped_no_remote: PreRunPushOutcome = - serde_json::from_value(json!({ "type": "skipped_no_remote" })).unwrap(); - assert_eq!(skipped_no_remote, PreRunPushOutcome::SkippedNoRemote); - - let skipped_mismatch: PreRunPushOutcome = serde_json::from_value(json!({ - "type": "skipped_remote_mismatch", - "remote": "git@github.com:user/fork.git", - "repo_origin_url": "https://github.com/acme/canonical.git", - })) - .unwrap(); - assert_eq!(skipped_mismatch, PreRunPushOutcome::SkippedRemoteMismatch { - remote: "git@github.com:user/fork.git".to_string(), - repo_origin_url: "https://github.com/acme/canonical.git".to_string(), - }); -} - -fn assert_same_type() { - assert_eq!( - TypeId::of::(), - TypeId::of::(), - "{} should be the same type as {}", - type_name::(), - type_name::() - ); -} diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index c77cc2cd53..aad83b919a 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -107,7 +107,7 @@ pub use pull_request::{ pub use reasoning::ReasoningOutput; pub use repository::{GitHubRepositorySlug, RepositoryProvider, RepositoryRef}; pub use run::{ - DirtyStatus, ForkSourceRef, GitContext, PreRunPushOutcome, RunClientProvenance, RunProvenance, + DirtyStatus, ForkSourceRef, GitContext, RunClientProvenance, RunProvenance, RunServerProvenance, RunSpec, }; pub use run_blob_id::RunBlobId; diff --git a/lib/foundation/fabro-types/src/run.rs b/lib/foundation/fabro-types/src/run.rs index 2f1671fe63..cf0fbe1ffd 100644 --- a/lib/foundation/fabro-types/src/run.rs +++ b/lib/foundation/fabro-types/src/run.rs @@ -41,34 +41,13 @@ pub enum DirtyStatus { Unknown, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum PreRunPushOutcome { - NotAttempted, - Succeeded { - remote: String, - branch: String, - }, - Failed { - remote: String, - branch: String, - message: String, - }, - SkippedNoRemote, - SkippedRemoteMismatch { - remote: String, - repo_origin_url: String, - }, -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct GitContext { - pub origin_url: String, - pub branch: String, + pub origin_url: String, + pub branch: String, #[serde(default, skip_serializing_if = "Option::is_none")] - pub sha: Option, - pub dirty: DirtyStatus, - pub push_outcome: PreRunPushOutcome, + pub sha: Option, + pub dirty: DirtyStatus, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index 3fb16d1474..633abab043 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use fabro_types::graph::Graph; -use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext, PreRunPushOutcome}; +use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext}; use fabro_types::run_event::run::{RunCreatedProps, RunParentLinkedProps, RunParentUnlinkedProps}; use fabro_types::run_event::{RunSessionTurnFailedCode, RunSessionTurnFailedProps}; use fabro_types::settings::InterpString; @@ -33,11 +33,10 @@ fn run_created_props_round_trip_templated_settings() { provenance: test_run_provenance(), manifest_blob: None, git: Some(GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: None, - dirty: DirtyStatus::Unknown, - push_outcome: PreRunPushOutcome::SkippedNoRemote, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: None, + dirty: DirtyStatus::Unknown, }), fork_source_ref: Some(ForkSourceRef { source_run_id: fixtures::RUN_2, @@ -58,7 +57,7 @@ fn run_created_props_round_trip_templated_settings() { ); assert_eq!(json["git"]["branch"], "main"); assert_eq!(json["git"]["dirty"], "unknown"); - assert_eq!(json["git"]["push_outcome"]["type"], "skipped_no_remote"); + assert!(json["git"].get("push_outcome").is_none()); assert_eq!( json["web_url"], "http://localhost:3000/runs/01JNQVR7M0EJ5GKAT2SC4ERS1Z" @@ -138,6 +137,41 @@ fn run_created_props_defaults_additive_fields_for_legacy_events() { assert_eq!(props.automation, None); } +#[test] +fn run_created_props_tolerates_legacy_git_push_outcome() { + let json = serde_json::json!({ + "title": null, + "settings": WorkflowSettings::default(), + "graph": Graph::new("ship"), + "labels": {}, + "provenance": test_run_provenance(), + "git": { + "origin_url": "https://github.com/fabro-sh/fabro.git", + "branch": "main", + "sha": "abc123", + "dirty": "clean", + "push_outcome": { + "type": "failed", + "remote": "origin", + "branch": "main", + "message": "remote rejected" + } + } + }); + + let props: RunCreatedProps = + serde_json::from_value(json).expect("legacy event with push_outcome should deserialize"); + let git = props.git.as_ref().expect("git context should be present"); + assert_eq!(git.origin_url, "https://github.com/fabro-sh/fabro.git"); + assert_eq!(git.branch, "main"); + assert_eq!(git.sha.as_deref(), Some("abc123")); + assert_eq!(git.dirty, DirtyStatus::Clean); + + let reserialized = serde_json::to_value(&props).expect("props should reserialize"); + assert!(reserialized["git"].get("push_outcome").is_none()); + assert_eq!(reserialized["git"]["origin_url"], git.origin_url); +} + #[test] fn run_parent_events_round_trip_parent_ids() { let linked = EventBody::RunParentLinked(RunParentLinkedProps { diff --git a/lib/foundation/fabro-types/tests/run_spec_methods.rs b/lib/foundation/fabro-types/tests/run_spec_methods.rs index 6380733229..f6f76fecf6 100644 --- a/lib/foundation/fabro-types/tests/run_spec_methods.rs +++ b/lib/foundation/fabro-types/tests/run_spec_methods.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use fabro_types::graph::Graph; -use fabro_types::run::{DirtyStatus, GitContext, PreRunPushOutcome, RunSpec}; +use fabro_types::run::{DirtyStatus, GitContext, RunSpec}; use fabro_types::settings::{ProjectNamespace, WorkflowNamespace}; use fabro_types::test_support::test_run_provenance; use fabro_types::{WorkflowSettings, fixtures}; @@ -32,14 +32,10 @@ fn sample_run_spec() -> RunSpec { manifest_blob: None, definition_blob: None, git: Some(GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: Some("abc123".to_string()), - dirty: DirtyStatus::Dirty, - push_outcome: PreRunPushOutcome::SkippedRemoteMismatch { - remote: "https://github.com/user/fork.git".to_string(), - repo_origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - }, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: Some("abc123".to_string()), + dirty: DirtyStatus::Dirty, }), fork_source_ref: None, } diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index 1dac9e4ce3..97529bc936 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use fabro_types::graph::Graph; -use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext, PreRunPushOutcome, RunSpec}; +use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext, RunSpec}; use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::test_run_provenance; @@ -32,14 +32,10 @@ fn run_spec_round_trips_templated_settings() { manifest_blob: None, definition_blob: None, git: Some(GitContext { - origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), - branch: "main".to_string(), - sha: Some("abc123".to_string()), - dirty: DirtyStatus::Clean, - push_outcome: PreRunPushOutcome::Succeeded { - remote: "origin".to_string(), - branch: "main".to_string(), - }, + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), + branch: "main".to_string(), + sha: Some("abc123".to_string()), + dirty: DirtyStatus::Clean, }), fork_source_ref: Some(ForkSourceRef { source_run_id: fixtures::RUN_2, @@ -58,7 +54,7 @@ fn run_spec_round_trips_templated_settings() { assert_eq!(json["git"]["branch"], "main"); assert_eq!(json["git"]["sha"], "abc123"); assert_eq!(json["git"]["dirty"], "clean"); - assert_eq!(json["git"]["push_outcome"]["type"], "succeeded"); + assert!(json["git"].get("push_outcome").is_none()); assert_eq!(json["fork_source_ref"]["checkpoint_sha"], "def456"); assert_eq!(json["automation"]["id"], "nightly"); assert_eq!(json["automation"]["trigger_id"], "schedule_1"); From 5305dca6c26774e82f1cdbb6919f8302f977607c Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Mon, 3 Aug 2026 19:27:33 -0400 Subject: [PATCH 2/2] Regenerate TypeScript client without push outcome models Regenerates the Axios client from the reduced OpenAPI spec and removes the six stale pre-run-push-outcome model files the generator leaves behind, along with their barrel and generator-manifest entries. Co-Authored-By: Claude Fable 5 --- .../src/.openapi-generator/FILES | 6 ---- .../src/models/git-context.ts | 4 --- .../fabro-api-client/src/models/index.ts | 6 ---- .../src/models/pre-run-push-outcome-failed.ts | 28 --------------- .../pre-run-push-outcome-not-attempted.ts | 25 ------------- .../pre-run-push-outcome-skipped-no-remote.ts | 25 ------------- ...un-push-outcome-skipped-remote-mismatch.ts | 27 -------------- .../models/pre-run-push-outcome-succeeded.ts | 27 -------------- .../src/models/pre-run-push-outcome.ts | 36 ------------------- 9 files changed, 184 deletions(-) delete mode 100644 lib/packages/fabro-api-client/src/models/pre-run-push-outcome-failed.ts delete mode 100644 lib/packages/fabro-api-client/src/models/pre-run-push-outcome-not-attempted.ts delete mode 100644 lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-no-remote.ts delete mode 100644 lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-remote-mismatch.ts delete mode 100644 lib/packages/fabro-api-client/src/models/pre-run-push-outcome-succeeded.ts delete mode 100644 lib/packages/fabro-api-client/src/models/pre-run-push-outcome.ts diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index f5d8fa8dae..0f76936c6a 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -271,12 +271,6 @@ models/parallel-branch-result.ts models/pending-interview-record.ts models/pending-reason.ts models/permission-level.ts -models/pre-run-push-outcome-failed.ts -models/pre-run-push-outcome-not-attempted.ts -models/pre-run-push-outcome-skipped-no-remote.ts -models/pre-run-push-outcome-skipped-remote-mismatch.ts -models/pre-run-push-outcome-succeeded.ts -models/pre-run-push-outcome.ts models/preflight-check-detail.ts models/preflight-check-report.ts models/preflight-check-result.ts diff --git a/lib/packages/fabro-api-client/src/models/git-context.ts b/lib/packages/fabro-api-client/src/models/git-context.ts index 09cdfb7a1b..20c2056437 100644 --- a/lib/packages/fabro-api-client/src/models/git-context.ts +++ b/lib/packages/fabro-api-client/src/models/git-context.ts @@ -16,9 +16,6 @@ // May contain unused imports in some cases // @ts-ignore import type { DirtyStatus } from './dirty-status'; -// May contain unused imports in some cases -// @ts-ignore -import type { PreRunPushOutcome } from './pre-run-push-outcome'; /** * Observable git state captured before the run starts. @@ -37,5 +34,4 @@ export interface GitContext { */ 'sha'?: string | null; 'dirty': DirtyStatus; - 'push_outcome': PreRunPushOutcome; } diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index d87d466cbf..94d28532a7 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -241,12 +241,6 @@ export * from './parallel-branch-result'; export * from './pending-interview-record'; export * from './pending-reason'; export * from './permission-level'; -export * from './pre-run-push-outcome'; -export * from './pre-run-push-outcome-failed'; -export * from './pre-run-push-outcome-not-attempted'; -export * from './pre-run-push-outcome-skipped-no-remote'; -export * from './pre-run-push-outcome-skipped-remote-mismatch'; -export * from './pre-run-push-outcome-succeeded'; export * from './preflight-check-detail'; export * from './preflight-check-report'; export * from './preflight-check-result'; diff --git a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-failed.ts b/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-failed.ts deleted file mode 100644 index 1441845bb3..0000000000 --- a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-failed.ts +++ /dev/null @@ -1,28 +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 interface PreRunPushOutcomeFailed { - 'type': PreRunPushOutcomeFailedTypeEnum; - 'remote': string; - 'branch': string; - 'message': string; -} - -export const PreRunPushOutcomeFailedTypeEnum = { - FAILED: 'failed' -} as const; - -export type PreRunPushOutcomeFailedTypeEnum = typeof PreRunPushOutcomeFailedTypeEnum[keyof typeof PreRunPushOutcomeFailedTypeEnum]; diff --git a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-not-attempted.ts b/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-not-attempted.ts deleted file mode 100644 index df7973f64a..0000000000 --- a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-not-attempted.ts +++ /dev/null @@ -1,25 +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 interface PreRunPushOutcomeNotAttempted { - 'type': PreRunPushOutcomeNotAttemptedTypeEnum; -} - -export const PreRunPushOutcomeNotAttemptedTypeEnum = { - NOT_ATTEMPTED: 'not_attempted' -} as const; - -export type PreRunPushOutcomeNotAttemptedTypeEnum = typeof PreRunPushOutcomeNotAttemptedTypeEnum[keyof typeof PreRunPushOutcomeNotAttemptedTypeEnum]; diff --git a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-no-remote.ts b/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-no-remote.ts deleted file mode 100644 index 5381873881..0000000000 --- a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-no-remote.ts +++ /dev/null @@ -1,25 +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 interface PreRunPushOutcomeSkippedNoRemote { - 'type': PreRunPushOutcomeSkippedNoRemoteTypeEnum; -} - -export const PreRunPushOutcomeSkippedNoRemoteTypeEnum = { - SKIPPED_NO_REMOTE: 'skipped_no_remote' -} as const; - -export type PreRunPushOutcomeSkippedNoRemoteTypeEnum = typeof PreRunPushOutcomeSkippedNoRemoteTypeEnum[keyof typeof PreRunPushOutcomeSkippedNoRemoteTypeEnum]; diff --git a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-remote-mismatch.ts b/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-remote-mismatch.ts deleted file mode 100644 index 6f12886b5d..0000000000 --- a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-skipped-remote-mismatch.ts +++ /dev/null @@ -1,27 +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 interface PreRunPushOutcomeSkippedRemoteMismatch { - 'type': PreRunPushOutcomeSkippedRemoteMismatchTypeEnum; - 'remote': string; - 'repo_origin_url': string; -} - -export const PreRunPushOutcomeSkippedRemoteMismatchTypeEnum = { - SKIPPED_REMOTE_MISMATCH: 'skipped_remote_mismatch' -} as const; - -export type PreRunPushOutcomeSkippedRemoteMismatchTypeEnum = typeof PreRunPushOutcomeSkippedRemoteMismatchTypeEnum[keyof typeof PreRunPushOutcomeSkippedRemoteMismatchTypeEnum]; diff --git a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-succeeded.ts b/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-succeeded.ts deleted file mode 100644 index be736e6f9a..0000000000 --- a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome-succeeded.ts +++ /dev/null @@ -1,27 +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 interface PreRunPushOutcomeSucceeded { - 'type': PreRunPushOutcomeSucceededTypeEnum; - 'remote': string; - 'branch': string; -} - -export const PreRunPushOutcomeSucceededTypeEnum = { - SUCCEEDED: 'succeeded' -} as const; - -export type PreRunPushOutcomeSucceededTypeEnum = typeof PreRunPushOutcomeSucceededTypeEnum[keyof typeof PreRunPushOutcomeSucceededTypeEnum]; diff --git a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome.ts b/lib/packages/fabro-api-client/src/models/pre-run-push-outcome.ts deleted file mode 100644 index 6d6f89c395..0000000000 --- a/lib/packages/fabro-api-client/src/models/pre-run-push-outcome.ts +++ /dev/null @@ -1,36 +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. - */ - - -// May contain unused imports in some cases -// @ts-ignore -import type { PreRunPushOutcomeFailed } from './pre-run-push-outcome-failed'; -// May contain unused imports in some cases -// @ts-ignore -import type { PreRunPushOutcomeNotAttempted } from './pre-run-push-outcome-not-attempted'; -// May contain unused imports in some cases -// @ts-ignore -import type { PreRunPushOutcomeSkippedNoRemote } from './pre-run-push-outcome-skipped-no-remote'; -// May contain unused imports in some cases -// @ts-ignore -import type { PreRunPushOutcomeSkippedRemoteMismatch } from './pre-run-push-outcome-skipped-remote-mismatch'; -// May contain unused imports in some cases -// @ts-ignore -import type { PreRunPushOutcomeSucceeded } from './pre-run-push-outcome-succeeded'; - -/** - * @type PreRunPushOutcome - * Outcome of the CLI\'s best-effort pre-run push. - */ -export type PreRunPushOutcome = { type: 'failed' } & PreRunPushOutcomeFailed | { type: 'not_attempted' } & PreRunPushOutcomeNotAttempted | { type: 'skipped_no_remote' } & PreRunPushOutcomeSkippedNoRemote | { type: 'skipped_remote_mismatch' } & PreRunPushOutcomeSkippedRemoteMismatch | { type: 'succeeded' } & PreRunPushOutcomeSucceeded;