diff --git a/apps/fabro-web/app/components/automation-form.tsx b/apps/fabro-web/app/components/automation-form.tsx index b7970e744..77bc90903 100644 --- a/apps/fabro-web/app/components/automation-form.tsx +++ b/apps/fabro-web/app/components/automation-form.tsx @@ -188,13 +188,16 @@ export function targetFromFormValues(values: AutomationFormValues): GitRunTarget }; } +function isWorkflowSourceRefValid(kind: AutomationGitWorkflowSourceKind, ref: string): boolean { + const reference = ref.trim(); + return kind === "commit" ? GIT_SHA_RE.test(reference) : reference !== ""; +} + function isWorkflowSourceValid(values: AutomationFormValues): boolean { if (!values.usesSeparateWorkflowSource) return true; - const reference = values.workflowSourceRef.trim(); return ( values.workflowSourceRepository.trim() !== "" && - reference !== "" && - (values.workflowSourceKind !== "commit" || GIT_SHA_RE.test(reference)) + isWorkflowSourceRefValid(values.workflowSourceKind, values.workflowSourceRef) ); } @@ -276,34 +279,38 @@ function describeCron(expression: string): string { return "Computed when saved"; } -function workflowSourceRefLabel(kind: AutomationGitWorkflowSourceKind): string { - switch (kind) { - case "branch": return "Branch"; - case "tag": return "Tag"; - case "commit": return "Exact commit"; - } +interface WorkflowSourceKindCopy { + label: string; + placeholder: string; + help: string; } -function workflowSourceRefPlaceholder(kind: AutomationGitWorkflowSourceKind): string { - switch (kind) { - case "branch": return "main"; - case "tag": return "v1.2.3"; - case "commit": return "0123456789abcdef0123456789abcdef01234567"; - } -} +const WORKFLOW_SOURCE_KINDS: Record = { + branch: { + label: "Branch", + placeholder: "main", + help: "Bare branch name resolved again whenever the automation fires.", + }, + tag: { + label: "Tag", + placeholder: "v1.2.3", + help: "Bare tag name resolved again whenever the automation fires.", + }, + commit: { + label: "Exact commit", + placeholder: "0123456789abcdef0123456789abcdef01234567", + help: "Exactly 40 hexadecimal characters; the same workflow bytes are used every time.", + }, +}; function workflowSourceRefHelp( kind: AutomationGitWorkflowSourceKind, valid: boolean, ): ReactNode { - if (kind === "commit") { - return valid - ? "Exactly 40 hexadecimal characters; the same workflow bytes are used every time." - : Enter exactly 40 hexadecimal characters.; + if (kind === "commit" && !valid) { + return Enter exactly 40 hexadecimal characters.; } - return kind === "branch" - ? "Bare branch name resolved again whenever the automation fires." - : "Bare tag name resolved again whenever the automation fires."; + return WORKFLOW_SOURCE_KINDS[kind].help; } interface AutomationFormFieldsProps { @@ -325,9 +332,11 @@ export function AutomationFormFields({ }: AutomationFormFieldsProps) { const slugTouchedRef = useRef(values.id.length > 0); const shaValid = isOptionalShaValid(values.targetSha); - const workflowSourceRefValid = values.workflowSourceKind !== "commit" - ? values.workflowSourceRef.trim() !== "" - : GIT_SHA_RE.test(values.workflowSourceRef.trim()); + const workflowSourceRefValid = isWorkflowSourceRefValid( + values.workflowSourceKind, + values.workflowSourceRef, + ); + const workflowSourceKind = WORKFLOW_SOURCE_KINDS[values.workflowSourceKind]; const compatibleEnvironments = environments .filter(isCloneBasedEnvironment) .sort((left, right) => left.id.localeCompare(right.id)); @@ -585,13 +594,13 @@ export function AutomationFormFields({ })} className={`${INPUT_CLASS} font-mono`} > - - - + {Object.entries(WORKFLOW_SOURCE_KINDS).map(([kind, copy]) => ( + + ))} {workflowSourceRefLabel(values.workflowSourceKind)}} + title={} help={workflowSourceRefHelp(values.workflowSourceKind, workflowSourceRefValid)} > patch({ workflowSourceRef: e.target.value })} - placeholder={workflowSourceRefPlaceholder(values.workflowSourceKind)} + placeholder={workflowSourceKind.placeholder} autoComplete="off" spellCheck={false} className={`${INPUT_CLASS} font-mono`} diff --git a/apps/fabro-web/app/lib/automation.ts b/apps/fabro-web/app/lib/automation.ts index 74a7b4014..f90f75a22 100644 --- a/apps/fabro-web/app/lib/automation.ts +++ b/apps/fabro-web/app/lib/automation.ts @@ -36,3 +36,10 @@ export function hasEnabledApiTrigger(automation: Automation): boolean { export function workflowSourceSummary(source: AutomationGitWorkflowSource): string { return `${source.repo} · ${source.kind} ${source.ref}`; } + +export const RUN_TARGET_CHECKOUT_LABEL = "run target checkout"; + +/** Where an automation's workflow files come from, for display. */ +export function workflowSourceLabel(source: AutomationGitWorkflowSource | undefined): string { + return source ? workflowSourceSummary(source) : RUN_TARGET_CHECKOUT_LABEL; +} diff --git a/apps/fabro-web/app/routes/automation-detail.tsx b/apps/fabro-web/app/routes/automation-detail.tsx index f7bb47818..289b2f632 100644 --- a/apps/fabro-web/app/routes/automation-detail.tsx +++ b/apps/fabro-web/app/routes/automation-detail.tsx @@ -24,7 +24,7 @@ import { findApiTrigger, findScheduleTrigger, gitTarget, - workflowSourceSummary, + workflowSourceLabel, } from "../lib/automation"; import { useAutomation, useAutomationRuns } from "../lib/queries"; import { queryKeys } from "../lib/query-keys"; @@ -101,7 +101,6 @@ function AutomationHeader({ automation }: { automation: Automation }) { const scheduleTrigger = findScheduleTrigger(automation); const apiTrigger = findApiTrigger(automation); const target = gitTarget(automation.target); - const workflowSource = automation.workflow_source; const canRun = apiTrigger?.enabled === true && automation.environment_id !== null; async function onRun() { @@ -158,9 +157,7 @@ function AutomationHeader({ automation }: { automation: Automation }) { ) : null} - Workflow · {automation.workflow} · {workflowSource - ? workflowSourceSummary(workflowSource) - : "run target checkout"} + Workflow · {automation.workflow} · {workflowSourceLabel(automation.workflow_source)} {automation.environment_id ?? ( diff --git a/apps/fabro-web/app/routes/automations.tsx b/apps/fabro-web/app/routes/automations.tsx index 11a9c7d6b..afd426e9d 100644 --- a/apps/fabro-web/app/routes/automations.tsx +++ b/apps/fabro-web/app/routes/automations.tsx @@ -19,6 +19,7 @@ import type { Automation, AutomationListResponse } from "@qltysh/fabro-api-clien import { Link, useNavigate } from "react-router"; import { ApiError, apiData, automationsApi } from "../lib/api-client"; import { + RUN_TARGET_CHECKOUT_LABEL, UNSUPPORTED_TARGET_LABEL, findScheduleTrigger, gitTarget, @@ -160,7 +161,7 @@ function AutomationCard({

- Workflow source · {automation.workflowSource ?? "run target checkout"} + Workflow source · {automation.workflowSource ?? RUN_TARGET_CHECKOUT_LABEL}

diff --git a/lib/apps/fabro-server/src/automation_materializer.rs b/lib/apps/fabro-server/src/automation_materializer.rs index d16ccf879..dba8ff0ac 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -13,7 +13,7 @@ use tokio::{fs, task}; use crate::git_checkout::{ GitAuthConfig, GitCheckoutError, GitCheckoutSelector, GitRepoCache, WorktreePrepareInput, - resolve_git_auth_config, + github_clone_url, resolve_git_auth_config, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -60,23 +60,15 @@ pub(crate) enum RunMaterializeError { #[source] source: AutomationValidationError, }, - #[error("failed to resolve automation target credentials")] - TargetCredentials { + #[error("failed to resolve automation {role} credentials")] + Credentials { + role: CheckoutRole, #[source] source: anyhow::Error, }, - #[error("failed to prepare automation target checkout")] - TargetCheckout { - #[source] - source: GitCheckoutError, - }, - #[error("failed to resolve automation workflow-source credentials")] - WorkflowSourceCredentials { - #[source] - source: anyhow::Error, - }, - #[error("failed to prepare automation workflow-source checkout")] - WorkflowSourceCheckout { + #[error("failed to prepare automation {role} checkout")] + Checkout { + role: CheckoutRole, #[source] source: GitCheckoutError, }, @@ -113,6 +105,15 @@ pub(crate) enum RunMaterializeError { }, } +/// Which repository a checkout serves; only the error message differs. +#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display)] +pub(crate) enum CheckoutRole { + #[strum(serialize = "target")] + Target, + #[strum(serialize = "workflow-source")] + WorkflowSource, +} + #[async_trait] pub(crate) trait AutomationRunMaterializer: Send + Sync { async fn materialize( @@ -123,34 +124,43 @@ pub(crate) trait AutomationRunMaterializer: Send + Sync { #[derive(Clone)] pub(crate) struct ProductionAutomationRunMaterializer { - credential_resolver: Arc, - repo_cache: Arc, - version_store: WorkflowVersionStore, - #[cfg(test)] - clone_urls: Arc>, + remote_resolver: Arc, + repo_cache: Arc, + version_store: WorkflowVersionStore, +} + +/// Where to fetch a repository from and how to authenticate. +#[derive(Clone)] +struct GitRemote { + clone_url: String, + auth: Option, } #[async_trait] -trait AutomationGitCredentialResolver: Send + Sync { - async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result>; +trait AutomationGitRemoteResolver: Send + Sync { + async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result; } -struct ServerGitHubCredentialResolver { +struct ServerGitHubRemoteResolver { credentials: Option, api_base_url: String, http_client: Option, } #[async_trait] -impl AutomationGitCredentialResolver for ServerGitHubCredentialResolver { - async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result> { - resolve_git_auth_config( +impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver { + async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result { + let auth = resolve_git_auth_config( self.credentials.as_ref(), repo, &self.api_base_url, self.http_client.clone(), ) - .await + .await?; + Ok(GitRemote { + clone_url: github_clone_url(repo), + auth, + }) } } @@ -163,50 +173,53 @@ impl ProductionAutomationRunMaterializer { version_store: WorkflowVersionStore, ) -> Self { Self { - credential_resolver: Arc::new(ServerGitHubCredentialResolver { + remote_resolver: Arc::new(ServerGitHubRemoteResolver { credentials: github_credentials, api_base_url: github_api_base_url, http_client, }), repo_cache, version_store, - #[cfg(test)] - clone_urls: Arc::new(std::collections::HashMap::new()), } } + #[cfg(test)] + fn with_remote_resolver(mut self, resolver: Arc) -> Self { + self.remote_resolver = resolver; + self + } + + async fn resolve_remote( + &self, + role: CheckoutRole, + repo: &GitHubRepositorySlug, + ) -> Result { + self.remote_resolver + .resolve(repo) + .await + .map_err(|source| RunMaterializeError::Credentials { role, source }) + } + async fn prepare_checkout( &self, + role: CheckoutRole, repo: &GitHubRepositorySlug, + remote: &GitRemote, selector: GitCheckoutSelector<'_>, - auth: Option<&GitAuthConfig>, worktree_dir: &Path, - ) -> Result { - let input = WorktreePrepareInput { - repo, - selector, - auth, - worktree_dir, - }; - #[cfg(test)] - if let Some(clone_url) = self.clone_urls.get(repo) { - return self - .repo_cache - .prepare_worktree_with_clone_url(input, clone_url) - .await; - } - self.repo_cache.prepare_worktree(input).await - } - - #[cfg(test)] - fn with_test_git( - mut self, - credential_resolver: Arc, - clone_urls: std::collections::HashMap, - ) -> Self { - self.credential_resolver = credential_resolver; - self.clone_urls = Arc::new(clone_urls); - self + ) -> Result { + self.repo_cache + .prepare_worktree( + WorktreePrepareInput { + repo, + selector, + auth: remote.auth.as_ref(), + worktree_dir, + }, + &remote.clone_url, + ) + .await + .map_err(|source| RunMaterializeError::Checkout { role, source }) } } @@ -234,23 +247,27 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { .map(AutomationGitWorkflowSource::validate) .transpose() .map_err(|source| RunMaterializeError::InvalidWorkflowSource { source })?; - let source_repo: Option = workflow_source + // A workflow source naming the target's exact coordinate shares its + // checkout; anything else needs a second worktree. + let separate_source = workflow_source .as_ref() .map(|source| { source .repo - .parse() + .parse::() + .map(|repo| (repo, source)) .map_err(|source| RunMaterializeError::InvalidWorkflowSource { source: AutomationValidationError::InvalidWorkflowSourceRepository { source, }, }) }) - .transpose()?; - let reuse_target_checkout = workflow_source.as_ref().is_none_or(|source| { - source_repo.as_ref() == Some(&target_repo) - && GitCheckoutSelector::from(source) == GitCheckoutSelector::from(&exact_target) - }); + .transpose()? + .filter(|(repo, source)| { + *repo != target_repo + || GitCheckoutSelector::from(*source) + != GitCheckoutSelector::from(&exact_target) + }); fs::create_dir_all(&input.temp_root) .await @@ -270,45 +287,38 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { source, })?; let target_checkout_dir = temp_dir.path().join("target"); - let target_auth = self - .credential_resolver - .resolve(&target_repo) - .await - .map_err(|source| RunMaterializeError::TargetCredentials { source })?; - + let target_remote = self + .resolve_remote(CheckoutRole::Target, &target_repo) + .await?; let checked_out_sha = self .prepare_checkout( + CheckoutRole::Target, &target_repo, + &target_remote, GitCheckoutSelector::from(&exact_target), - target_auth.as_ref(), &target_checkout_dir, ) - .await - .map_err(|source| RunMaterializeError::TargetCheckout { source })?; - let workflow_checkout_dir = if reuse_target_checkout { - target_checkout_dir - } else { - let source = workflow_source - .as_ref() - .expect("non-reused workflow checkout requires an explicit source"); - let repo = source_repo - .as_ref() - .expect("a validated workflow source has a repository"); - let source_auth = self - .credential_resolver - .resolve(repo) - .await - .map_err(|source| RunMaterializeError::WorkflowSourceCredentials { source })?; - let source_checkout_dir = temp_dir.path().join("workflow-source"); - self.prepare_checkout( - repo, - GitCheckoutSelector::from(source), - source_auth.as_ref(), - &source_checkout_dir, - ) - .await - .map_err(|source| RunMaterializeError::WorkflowSourceCheckout { source })?; - source_checkout_dir + .await?; + let workflow_checkout_dir = match separate_source { + None => target_checkout_dir, + Some((repo, source)) => { + let remote = if repo == target_repo { + target_remote + } else { + self.resolve_remote(CheckoutRole::WorkflowSource, &repo) + .await? + }; + let source_checkout_dir = temp_dir.path().join("workflow-source"); + self.prepare_checkout( + CheckoutRole::WorkflowSource, + &repo, + &remote, + GitCheckoutSelector::from(source), + &source_checkout_dir, + ) + .await?; + source_checkout_dir + } }; exact_target.sha = Some(checked_out_sha); @@ -365,9 +375,23 @@ struct TestAutomationRunMaterializerState { #[derive(Clone)] enum TestMaterializeFailure { InvalidTarget(TargetValidationError), + /// Unit because `AutomationValidationError` is not `Clone`; any variant + /// exercises the same handler path. InvalidWorkflowSource, } +#[cfg(any(test, feature = "test-support"))] +impl From for RunMaterializeError { + fn from(failure: TestMaterializeFailure) -> Self { + match failure { + TestMaterializeFailure::InvalidTarget(source) => Self::InvalidTarget { source }, + TestMaterializeFailure::InvalidWorkflowSource => Self::InvalidWorkflowSource { + source: AutomationValidationError::InvalidWorkflowSourceBranch, + }, + } + } +} + #[cfg(any(test, feature = "test-support"))] #[derive(Clone)] struct TestMaterializedWorkflow { @@ -478,16 +502,7 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer { guard.captured_inputs.push(input); guard.response.clone() }; - let materialized = *response.map_err(|failure| match failure { - TestMaterializeFailure::InvalidTarget(source) => { - RunMaterializeError::InvalidTarget { source } - } - TestMaterializeFailure::InvalidWorkflowSource => { - RunMaterializeError::InvalidWorkflowSource { - source: AutomationValidationError::InvalidWorkflowSourceBranch, - } - } - })?; + let materialized = *response.map_err(RunMaterializeError::from)?; let store = self .version_store .as_ref() @@ -562,23 +577,37 @@ mod tests { } } + /// Serves local bare fixtures as clone URLs while recording which + /// repositories had credentials resolved. + struct FixtureRemoteResolver { + credentials: Arc, + clone_urls: HashMap, + } + #[async_trait] - impl AutomationGitCredentialResolver for RecordingCredentialResolver { - async fn resolve( - &self, - repo: &GitHubRepositorySlug, - ) -> anyhow::Result> { - self.repositories + impl AutomationGitRemoteResolver for FixtureRemoteResolver { + async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result { + let recorder = &self.credentials; + recorder + .repositories .lock() .expect("credential recorder lock poisoned") .push(repo.clone()); - if self.fail_for.as_ref() == Some(repo) { + if recorder.fail_for.as_ref() == Some(repo) { anyhow::bail!("test repository access denied") } - Ok(Some(GitAuthConfig::new( - Some("x-access-token".to_string()), - Some(FAKE_TOKEN.to_string()), - ))) + let clone_url = self + .clone_urls + .get(repo) + .unwrap_or_else(|| panic!("no fixture clone URL for {repo}")) + .clone(); + Ok(GitRemote { + clone_url, + auth: Some(GitAuthConfig::new( + Some("x-access-token".to_string()), + Some(FAKE_TOKEN.to_string()), + )), + }) } } @@ -761,7 +790,10 @@ mod tests { Arc::new(GitRepoCache::new(root.join("cache"))), store, ) - .with_test_git(resolver, clone_urls) + .with_remote_resolver(Arc::new(FixtureRemoteResolver { + credentials: resolver, + clone_urls, + })) } #[tokio::test] @@ -786,13 +818,7 @@ mod tests { let closure = fabro_manifest::collect_workflow_versions(Path::new("root"), &checkout).unwrap(); - let database = fabro_store::test_support::test_database( - Arc::new(InMemory::new()), - "", - Duration::from_millis(1), - None, - ); - let store = WorkflowVersionStore::new(database.blobs()); + let store = test_version_store(); for _ in 0..2 { for (expected, version) in closure.versions() { @@ -938,7 +964,7 @@ mod tests { } #[tokio::test] - async fn same_repository_with_a_different_selector_uses_a_second_worktree() { + async fn same_repository_with_a_different_selector_reuses_credentials_for_a_second_worktree() { let temp = TempDir::new().unwrap(); let fixture = seed_repository(temp.path(), "shared", "shared workflow"); let repo = repository("fabro-sh/shared"); @@ -963,10 +989,7 @@ mod tests { .await .unwrap(); - assert_eq!(resolver.repositories(), vec![ - "fabro-sh/shared", - "fabro-sh/shared" - ]); + assert_eq!(resolver.repositories(), vec!["fabro-sh/shared"]); } #[tokio::test] @@ -1068,7 +1091,8 @@ mod tests { .materialize(missing_target) .await .unwrap_err(); - assert!(matches!(error, RunMaterializeError::TargetCheckout { + assert!(matches!(error, RunMaterializeError::Checkout { + role: CheckoutRole::Target, source: GitCheckoutError::FetchBranch { .. }, })); assert!(!format!("{error:?}").contains(FAKE_TOKEN)); @@ -1087,10 +1111,10 @@ mod tests { )) .await .unwrap_err(); - assert!(matches!( - error, - RunMaterializeError::TargetCredentials { .. } - )); + assert!(matches!(error, RunMaterializeError::Credentials { + role: CheckoutRole::Target, + .. + })); assert!(!format!("{error:?}").contains(FAKE_TOKEN)); let source_resolver = Arc::new(RecordingCredentialResolver::fails_for(source_repo)); @@ -1111,10 +1135,10 @@ mod tests { )) .await .unwrap_err(); - assert!(matches!( - error, - RunMaterializeError::WorkflowSourceCredentials { .. } - )); + assert!(matches!(error, RunMaterializeError::Credentials { + role: CheckoutRole::WorkflowSource, + .. + })); assert!(!format!("{error:?}").contains(FAKE_TOKEN)); let error = production_materializer( @@ -1143,12 +1167,10 @@ mod tests { )) .await .unwrap_err(); - assert!(matches!( - error, - RunMaterializeError::WorkflowSourceCheckout { - source: GitCheckoutError::FetchBranch { .. }, - } - )); + assert!(matches!(error, RunMaterializeError::Checkout { + role: CheckoutRole::WorkflowSource, + source: GitCheckoutError::FetchBranch { .. }, + })); assert!(!format!("{error:?}").contains(FAKE_TOKEN)); } diff --git a/lib/apps/fabro-server/src/git_checkout.rs b/lib/apps/fabro-server/src/git_checkout.rs index fcb8d1435..822cf1b7c 100644 --- a/lib/apps/fabro-server/src/git_checkout.rs +++ b/lib/apps/fabro-server/src/git_checkout.rs @@ -105,7 +105,8 @@ impl GitRepoCache { .join(format!("{}.git", repo.repo())) } - /// Prepare a worktree containing the requested ref of `repo` at + /// Prepare a worktree containing the requested ref of `repo`, fetched from + /// `clone_url`, at /// `worktree_dir`. Returns the resolved commit SHA. /// /// First call for a repo: a `--bare --depth 1` clone is created at @@ -118,14 +119,6 @@ impl GitRepoCache { pub(crate) async fn prepare_worktree( &self, args: WorktreePrepareInput<'_>, - ) -> Result { - let clone_url = github_clone_url(args.repo); - self.prepare_worktree_with_clone_url(args, &clone_url).await - } - - pub(crate) async fn prepare_worktree_with_clone_url( - &self, - args: WorktreePrepareInput<'_>, clone_url: &str, ) -> Result { let _guard = self @@ -280,7 +273,7 @@ async fn bare_clone_may_be_corrupt(bare_dir: &Path) -> bool { } } -fn github_clone_url(repo: &GitHubRepositorySlug) -> String { +pub(crate) fn github_clone_url(repo: &GitHubRepositorySlug) -> String { let mut url = repo.https_url(); url.push_str(".git"); url @@ -884,7 +877,7 @@ mod tests { let worktree_a = temp.path().join("wt-a"); let sha_a = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&target), @@ -906,7 +899,7 @@ mod tests { let worktree_b = temp.path().join("wt-b"); let sha_b = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&target), @@ -937,7 +930,7 @@ mod tests { let worktree_a = temp.path().join("wt-a"); cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&target), @@ -955,7 +948,7 @@ mod tests { let worktree_b = temp.path().join("wt-b"); let sha = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&target), @@ -993,7 +986,7 @@ mod tests { ("commit", git_target("main", None, Some(&expected_sha))), ] { let sha = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&target), @@ -1021,7 +1014,7 @@ mod tests { let unavailable_commit = git_target("main", None, Some(unavailable_sha)); let tag_error = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&missing_tag), @@ -1038,7 +1031,7 @@ mod tests { )); let commit_error = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, selector: GitCheckoutSelector::from(&unavailable_commit), diff --git a/lib/apps/fabro-server/src/server/automation_scheduler.rs b/lib/apps/fabro-server/src/server/automation_scheduler.rs index e448281da..23a7c19eb 100644 --- a/lib/apps/fabro-server/src/server/automation_scheduler.rs +++ b/lib/apps/fabro-server/src/server/automation_scheduler.rs @@ -263,8 +263,8 @@ async fn fire_scheduled_automation_run( .materialize_automation_run(AutomationRunMaterializeInput { automation_id: automation_id.clone(), target, - workflow_source: automation.workflow_source.clone(), - workflow: automation.workflow.clone(), + workflow_source: automation.workflow_source, + workflow: automation.workflow, run_id, temp_root: state.automation_temp_root(), }) @@ -447,6 +447,16 @@ mod tests { id: &str, name: &str, triggers: Vec, + ) -> Automation { + create_automation_with_source(state, id, name, None, triggers).await + } + + async fn create_automation_with_source( + state: &AppState, + id: &str, + name: &str, + workflow_source: Option, + triggers: Vec, ) -> Automation { state .automation_store() @@ -456,29 +466,7 @@ mod tests { description: None, environment_id: Some("default".to_string()), target: target(), - workflow_source: None, - workflow: "workflow.fabro".to_string(), - triggers, - }) - .await - .expect("test automation should be created") - } - - async fn create_automation_with_source( - state: &AppState, - id: &str, - workflow_source: AutomationGitWorkflowSource, - triggers: Vec, - ) -> Automation { - state - .automation_store() - .create(AutomationDraft { - id: AutomationId::new(id).expect("test automation id should be valid"), - name: id.to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow_source: Some(workflow_source), + workflow_source, workflow: "workflow.fabro".to_string(), triggers, }) @@ -741,7 +729,8 @@ mod tests { create_automation_with_source( state.as_ref(), "scheduled-source", - workflow_source.clone(), + "scheduled-source", + Some(workflow_source.clone()), vec![schedule_trigger("schedule", "* * * * *", true)], ) .await; @@ -865,11 +854,12 @@ mod tests { create_automation_with_source( state.as_ref(), "failing-source", - AutomationGitWorkflowSource { + "failing-source", + Some(AutomationGitWorkflowSource { repo: "fabro-sh/workflows".to_string(), kind: AutomationGitWorkflowSourceKind::Branch, reference: "main".to_string(), - }, + }), vec![schedule_trigger("schedule", "* * * * *", true)], ) .await; diff --git a/lib/components/fabro-automation/src/model.rs b/lib/components/fabro-automation/src/model.rs index c69a14cd8..960b36e6c 100644 --- a/lib/components/fabro-automation/src/model.rs +++ b/lib/components/fabro-automation/src/model.rs @@ -171,13 +171,6 @@ pub enum AutomationGitWorkflowSourceKind { Commit, } -impl AutomationGitWorkflowSourceKind { - #[must_use] - pub fn as_str(self) -> &'static str { - self.into() - } -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct AutomationGitWorkflowSource { @@ -403,7 +396,7 @@ fn normalize_replace( .filter(|environment_id| !environment_id.is_empty()); value.workflow_source = value .workflow_source - .map(normalize_workflow_source) + .map(AutomationGitWorkflowSource::validate) .transpose()?; validate_fields(&value, require_environment)?; @@ -442,12 +435,6 @@ fn normalize_replace( Ok(value) } -fn normalize_workflow_source( - source: AutomationGitWorkflowSource, -) -> Result { - source.validate() -} - fn validate_target(target: RunTarget) -> Result { if !matches!(&target, RunTarget::Git(_)) { return Err(AutomationValidationError::UnsupportedTarget { diff --git a/lib/components/fabro-automation/src/store.rs b/lib/components/fabro-automation/src/store.rs index 4f5517663..5ec5fd62f 100644 --- a/lib/components/fabro-automation/src/store.rs +++ b/lib/components/fabro-automation/src/store.rs @@ -162,7 +162,7 @@ impl AutomationStore { .bind(target.sha.as_deref()) .bind(&automation.workflow) .bind(workflow_source.map(|source| source.repo.as_str())) - .bind(workflow_source.map(|source| source.kind.as_str())) + .bind(workflow_source.map(|source| <&'static str>::from(source.kind))) .bind(workflow_source.map(|source| source.reference.as_str())) .bind(id.as_str()) .bind(expected.as_str()) @@ -373,7 +373,7 @@ pub(crate) async fn insert_automation_ignoring_conflict( .bind(target.sha.as_deref()) .bind(&automation.workflow) .bind(workflow_source.map(|source| source.repo.as_str())) - .bind(workflow_source.map(|source| source.kind.as_str())) + .bind(workflow_source.map(|source| <&'static str>::from(source.kind))) .bind(workflow_source.map(|source| source.reference.as_str())) .execute(&mut **transaction) .await?; diff --git a/lib/foundation/fabro-db/src/lib.rs b/lib/foundation/fabro-db/src/lib.rs index 6931ac68a..4f3687437 100644 --- a/lib/foundation/fabro-db/src/lib.rs +++ b/lib/foundation/fabro-db/src/lib.rs @@ -28,11 +28,6 @@ pub const RUNS_MIGRATION_SQL: &str = include_str!("../migrations/2026071104_runs /// the production schema without a filesystem path into this crate. pub const RUN_EVENTS_MIGRATION_SQL: &str = include_str!("../migrations/2026082701_run_events.sql"); -/// The automation workflow-source migration, exposed so storage fixtures can -/// install the production optional-coordinate columns and constraints. -pub const AUTOMATION_WORKFLOW_SOURCES_MIGRATION_SQL: &str = - include_str!("../migrations/2026082802_automation_workflow_sources.sql"); - #[derive(Clone)] pub struct Database { pool: DbPool,