scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Simplify automation run-target plumbing

Browse source 
- Materializer derives the manifest GitContext from RunTarget::validate()
  instead of hand-building it and re-parsing the repository slug
- Drop parse_github_repository_slug and InvalidRepositorySlug, now unused
- Store reuses Automation::git_target() instead of a private duplicate
- Legacy TOML import returns the target directly rather than a tuple
- Automation target migration updates columns with a single UPDATE ... FROM
- Web: share gitTarget(), targetFromFormValues(), and one SHA validator
  across the automation form, list, detail, new, and edit views

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-08-26 18:17:22 -04:00
parent a65c4ff779
commit 50fed849f3
14 changed files with 129 additions and 175 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
apps/fabro-web/app/components/automation-form.tsx
View file

@ -8,7 +8,12 @@ import type {
WorkflowSettings,
} from "@qltysh/fabro-api-client";
import { findApiTrigger, findScheduleTrigger } from "../lib/automation";
import {
findApiTrigger,
findScheduleTrigger,
gitTarget,
type GitRunTarget,
} from "../lib/automation";
import { Panel, Row } from "./settings-panel";
import { INPUT_CLASS } from "./ui";
import { sandboxRuntime } from "../lib/run-sandbox-lifecycle";
@ -51,7 +56,7 @@ const CRON_PRESETS: ReadonlyArray<{ label: string; value: string }> = [
export function automationToFormValues(automation: Automation): AutomationFormValues {
const apiTrigger = findApiTrigger(automation);
const scheduleTrigger = findScheduleTrigger(automation);
const target = automation.target.kind === "git" ? automation.target : null;
const target = gitTarget(automation.target);
return {
id: automation.id,
name: automation.name,
@ -84,9 +89,7 @@ export function automationFormValuesFromRun(
run.workflow.graph_name,
name,
);
const canonicalTarget = runState?.spec.target?.kind === "git"
? runState.spec.target
: null;
const canonicalTarget = gitTarget(runState?.spec.target);
const repository = canonicalTarget?.repo
?? githubRepositoryFromSettings(settings)
?? githubRepositoryName(run.repository?.name)
@ -129,11 +132,30 @@ export function isFormValid(values: AutomationFormValues): boolean {
values.name.trim() !== "" &&
values.repository.trim() !== "" &&
values.branch.trim() !== "" &&
(values.sha.trim() === "" || /^[0-9a-fA-F]{40}$/.test(values.sha.trim())) &&
isOptionalShaValid(values.sha) &&
values.workflow.trim() !== ""
);
}
const GIT_SHA_RE = /^[0-9a-fA-F]{40}$/;
/** An empty SHA means "no pin"; anything else must be a full 40-hex commit id. */
function isOptionalShaValid(sha: string): boolean {
const trimmed = sha.trim();
return trimmed === "" || GIT_SHA_RE.test(trimmed);
}
/** Canonical Git target sent in create/replace requests. */
export function targetFromFormValues(values: AutomationFormValues): GitRunTarget {
return {
kind: "git",
repo: values.repository.trim(),
branch: values.branch.trim(),
tag: values.tag.trim() || undefined,
sha: values.sha.trim().toLowerCase() || undefined,
};
}
function kebabify(value: string): string {
return value
.toLowerCase()
@ -212,8 +234,7 @@ export function AutomationFormFields({
lockIdAndTarget = false,
}: AutomationFormFieldsProps) {
const slugTouchedRef = useRef(values.id.length > 0);
const sha = values.sha.trim();
const shaValid = sha === "" || /^[0-9a-fA-F]{40}$/.test(sha);
const shaValid = isOptionalShaValid(values.sha);
function patch(partial: Partial<AutomationFormValues>) {
onChange({ ...values, ...partial });

11
apps/fabro-web/app/lib/automation.ts
View file

@ -1,4 +1,13 @@
import type { Automation, AutomationTrigger } from "@qltysh/fabro-api-client";
import type { Automation, AutomationTrigger, RunTarget } from "@qltysh/fabro-api-client";
export type GitRunTarget = Extract<RunTarget, { kind: "git" }>;
/** Label shown in place of a repository when an automation's target is not Git-backed. */
export const UNSUPPORTED_TARGET_LABEL = "Unsupported target";
export function gitTarget(target: RunTarget | null | undefined): GitRunTarget | null {
return target?.kind === "git" ? target : null;
}
type TriggerOfType<K extends AutomationTrigger["type"]> = Extract<
AutomationTrigger,

11
apps/fabro-web/app/routes/automation-detail.tsx
View file

@ -18,7 +18,12 @@ import type {
import { toRunWithStatus } from "../data/runs";
import { ApiError, apiData, automationsApi } from "../lib/api-client";
import { findApiTrigger, findScheduleTrigger } from "../lib/automation";
import {
UNSUPPORTED_TARGET_LABEL,
findApiTrigger,
findScheduleTrigger,
gitTarget,
} from "../lib/automation";
import { useAutomation, useAutomationRuns } from "../lib/queries";
import { queryKeys } from "../lib/query-keys";
import { useDataUpdatedAt } from "../hooks/use-data-updated-at";
@ -93,7 +98,7 @@ function AutomationHeader({ automation }: { automation: Automation }) {
const scheduleTrigger = findScheduleTrigger(automation);
const apiTrigger = findApiTrigger(automation);
const target = automation.target.kind === "git" ? automation.target : null;
const target = gitTarget(automation.target);
const canRun = apiTrigger?.enabled === true;
async function onRun() {
@ -140,7 +145,7 @@ function AutomationHeader({ automation }: { automation: Automation }) {
</div>
<div className="mt-2 flex flex-wrap items-center gap-x-5 gap-y-2 text-sm">
<Chip icon={FolderIcon}>
{target?.repo ?? "Unsupported target"}
{target?.repo ?? UNSUPPORTED_TARGET_LABEL}
{target ? (
<span className="text-fg-muted/70">
{" · "}{target.branch}

11
apps/fabro-web/app/routes/automations-edit.tsx
View file

@ -11,6 +11,7 @@ import {
AutomationFormFields,
automationToFormValues,
isFormValid,
targetFromFormValues,
triggersFromFormValues,
type AutomationFormValues,
} from "../components/automation-form";
@ -85,14 +86,8 @@ function EditAutomationForm({ automation }: { automation: Automation }) {
automationsApi.replaceAutomation(automation.id, automation.revision, {
name: trimmedName,
description: values.description.trim() || null,
target: {
kind: "git",
repo: values.repository.trim(),
branch: values.branch.trim(),
tag: values.tag.trim() || undefined,
sha: values.sha.trim().toLowerCase() || undefined,
},
workflow: values.workflow.trim(),
target: targetFromFormValues(values),
workflow: values.workflow.trim(),
triggers: triggersFromFormValues(values),
}),
);

11
apps/fabro-web/app/routes/automations-new.tsx
View file

@ -11,6 +11,7 @@ import {
EMPTY_AUTOMATION_FORM,
automationFormValuesFromRun,
isFormValid,
targetFromFormValues,
triggersFromFormValues,
type AutomationFormValues,
} from "../components/automation-form";
@ -111,14 +112,8 @@ function AutomationCreateForm({
id: values.id.trim(),
name: trimmedName,
description: values.description.trim() || null,
target: {
kind: "git",
repo: values.repository.trim(),
branch: values.branch.trim(),
tag: values.tag.trim() || undefined,
sha: values.sha.trim().toLowerCase() || undefined,
},
workflow: values.workflow.trim(),
target: targetFromFormValues(values),
workflow: values.workflow.trim(),
triggers: triggersFromFormValues(values),
}),
);

11
apps/fabro-web/app/routes/automations.tsx
View file

@ -18,7 +18,12 @@ import { FilterButton } from "../components/runs-list/filter-button";
import type { Automation, AutomationListResponse } from "@qltysh/fabro-api-client";
import { Link, useNavigate } from "react-router";
import { ApiError, apiData, automationsApi } from "../lib/api-client";
import { findScheduleTrigger, hasEnabledApiTrigger } from "../lib/automation";
import {
UNSUPPORTED_TARGET_LABEL,
findScheduleTrigger,
gitTarget,
hasEnabledApiTrigger,
} from "../lib/automation";
import { useAutomations } from "../lib/queries";
import { queryKeys } from "../lib/query-keys";
import { ConfirmDialog, PRIMARY_BUTTON_CLASS } from "../components/ui";
@ -82,13 +87,13 @@ const MENU_ITEM_DANGER_CLASS =
function mapAutomations(result: AutomationListResponse | undefined): AutomationRow[] {
const automations = result?.data ?? [];
return automations.map((a) => {
const target = a.target.kind === "git" ? a.target : null;
const target = gitTarget(a.target);
return {
id: a.id,
revision: a.revision,
name: a.name,
workflow: a.workflow,
repository: target?.repo ?? "Unsupported target",
repository: target?.repo ?? UNSUPPORTED_TARGET_LABEL,
schedule: findScheduleTrigger(a)?.expression,
apiEnabled: hasEnabledApiTrigger(a),
icon: slugIconMap[a.workflow] ?? CodeBracketIcon,

113
lib/apps/fabro-server/src/automation_materializer.rs
View file

@ -3,10 +3,10 @@ use std::sync::Arc;
use async_trait::async_trait;
use fabro_api::types::RunManifest;
use fabro_automation::{AutomationId, AutomationValidationError};
use fabro_automation::AutomationId;
use fabro_config::{EnvironmentLayer, MergeMap};
use fabro_manifest::ManifestBuildInput;
use fabro_types::{DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, RunId};
use fabro_types::{GitHubRepositorySlug, GitRunTarget, RunId, RunTarget, TargetValidationError};
use tokio::{fs, task};
use crate::git_checkout::{
@ -32,11 +32,10 @@ pub(crate) struct AutomationRunMaterialized {
#[derive(thiserror::Error, Debug)]
pub(crate) enum RunMaterializeError {
#[error("invalid repository target {value:?}")]
#[error("invalid automation Git target")]
InvalidTarget {
value: String,
#[source]
source: AutomationValidationError,
source: TargetValidationError,
},
#[error("failed to prepare automation checkout")]
Checkout {
@ -117,7 +116,11 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
&self,
input: AutomationRunMaterializeInput,
) -> Result<AutomationRunMaterialized, RunMaterializeError> {
let repo = parse_target_repository(&input.target.repo)?;
let repo = GitHubRepositorySlug::try_new(&input.target.repo).ok_or(
RunMaterializeError::InvalidTarget {
source: TargetValidationError::Repository,
},
)?;
fs::create_dir_all(&input.temp_root)
.await
.map_err(|source| RunMaterializeError::TempDirectory {
@ -162,10 +165,7 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
workflow: input.workflow,
user_settings_path: input.user_settings_path,
checkout_dir,
git_context: ManifestGitContextInput {
repo,
target: exact_target,
},
target: exact_target,
environment_defaults: self.environment_defaults.clone(),
};
task::spawn_blocking(move || build_manifest_from_checkout(manifest_input))
@ -174,30 +174,15 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
}
}
fn parse_target_repository(value: &str) -> Result<GitHubRepositorySlug, RunMaterializeError> {
fabro_automation::parse_github_repository_slug(value).map_err(|source| {
RunMaterializeError::InvalidTarget {
value: value.to_string(),
source,
}
})
}
#[derive(Debug)]
pub(crate) struct ManifestFromCheckoutInput {
workflow: String,
user_settings_path: PathBuf,
checkout_dir: PathBuf,
git_context: ManifestGitContextInput,
target: GitRunTarget,
environment_defaults: MergeMap<EnvironmentLayer>,
}
#[derive(Debug)]
pub(crate) struct ManifestGitContextInput {
repo: GitHubRepositorySlug,
target: GitRunTarget,
}
fn build_manifest_from_checkout(
args: ManifestFromCheckoutInput,
) -> Result<AutomationRunMaterialized, RunMaterializeError> {
@ -205,9 +190,17 @@ fn build_manifest_from_checkout(
workflow,
user_settings_path,
checkout_dir,
git_context,
target,
environment_defaults,
} = args;
// Re-validating the exact target (now carrying the checked-out SHA) yields
// the same `GitContext` projection the run-intent path uses.
let validated = RunTarget::Git(target)
.validate()
.map_err(|source| RunMaterializeError::InvalidTarget { source })?;
let RunTarget::Git(target) = validated.target else {
unreachable!("validating a Git target yields a Git target");
};
let built = fabro_manifest::build_run_manifest(ManifestBuildInput {
workflow: workflow.into(),
cwd: checkout_dir,
@ -218,18 +211,13 @@ fn build_manifest_from_checkout(
.map_err(manifest_build_error)?;
let mut manifest = built.manifest;
manifest.git = Some(GitContext {
origin_url: git_context.repo.https_url(),
branch: git_context.target.branch.clone(),
sha: git_context.target.sha.clone(),
dirty: DirtyStatus::Clean,
});
manifest.git = validated.git;
let submitted_manifest_bytes = serde_json::to_vec(&manifest)
.map_err(|source| RunMaterializeError::SerializeManifest { source })?;
Ok(AutomationRunMaterialized {
manifest,
submitted_manifest_bytes,
target: git_context.target,
target,
})
}
@ -254,14 +242,7 @@ pub struct TestAutomationRunMaterializer {
#[cfg(any(test, feature = "test-support"))]
struct TestAutomationRunMaterializerState {
captured_inputs: Vec<AutomationRunMaterializeInput>,
response: TestMaterializeResponse,
}
#[cfg(any(test, feature = "test-support"))]
#[derive(Clone)]
enum TestMaterializeResponse {
Success(Box<AutomationRunMaterialized>),
InvalidTarget(String),
response: Result<Box<AutomationRunMaterialized>, TargetValidationError>,
}
#[cfg(any(test, feature = "test-support"))]
@ -271,20 +252,18 @@ impl TestAutomationRunMaterializer {
submitted_manifest_bytes: Vec<u8>,
target: GitRunTarget,
) -> Self {
Self::new(TestMaterializeResponse::Success(Box::new(
AutomationRunMaterialized {
manifest,
submitted_manifest_bytes,
target,
},
)))
Self::new(Ok(Box::new(AutomationRunMaterialized {
manifest,
submitted_manifest_bytes,
target,
})))
}
pub fn fail_invalid_target(message: impl Into<String>) -> Self {
Self::new(TestMaterializeResponse::InvalidTarget(message.into()))
pub fn fail_invalid_target() -> Self {
Self::new(Err(TargetValidationError::Repository))
}
fn new(response: TestMaterializeResponse) -> Self {
fn new(response: Result<Box<AutomationRunMaterialized>, TargetValidationError>) -> Self {
Self {
inner: std::sync::Arc::new(std::sync::Mutex::new(TestAutomationRunMaterializerState {
captured_inputs: Vec::new(),
@ -318,17 +297,11 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer {
.lock()
.expect("test automation materializer lock poisoned");
guard.captured_inputs.push(input);
match guard.response.clone() {
TestMaterializeResponse::Success(materialized) => Ok(*materialized),
TestMaterializeResponse::InvalidTarget(value) => {
Err(RunMaterializeError::InvalidTarget {
source: AutomationValidationError::InvalidRepositorySlug {
value: value.clone(),
},
value,
})
}
}
guard
.response
.clone()
.map(|materialized| *materialized)
.map_err(|source| RunMaterializeError::InvalidTarget { source })
}
}
@ -373,21 +346,17 @@ mod tests {
.unwrap();
let user_settings_path = temp.path().join("settings.toml");
fs::write(&user_settings_path, "_version = 1\n").unwrap();
let repo = parse_target_repository("workspace-org/app").unwrap();
let sha = "0123456789abcdef0123456789abcdef01234567".to_string();
let materialized = build_manifest_from_checkout(ManifestFromCheckoutInput {
workflow: "demo".to_string(),
user_settings_path: user_settings_path.clone(),
checkout_dir: checkout.clone(),
git_context: ManifestGitContextInput {
repo,
target: GitRunTarget {
repo: "workspace-org/app".to_string(),
branch: "release".to_string(),
tag: Some("v1".to_string()),
sha: Some(sha.clone()),
},
target: GitRunTarget {
repo: "workspace-org/app".to_string(),
branch: "release".to_string(),
tag: Some("v1".to_string()),
sha: Some(sha.clone()),
},
environment_defaults: test_environment_defaults(),
})

2
lib/apps/fabro-server/src/server/automation_scheduler.rs
View file

@ -714,7 +714,7 @@ mod tests {
#[tokio::test]
async fn failing_materializer_waits_until_next_cron_occurrence() {
let materializer = TestAutomationRunMaterializer::fail_invalid_target("boom");
let materializer = TestAutomationRunMaterializer::fail_invalid_target();
let state = test_state_with_materializer(materializer.clone());
create_automation(state.as_ref(), "nightly", "Nightly", vec![
schedule_trigger("schedule", "* * * * *", true),

22
lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs
View file

@ -96,7 +96,12 @@ fn parse_legacy_automation(
.map_err(|source| AutomationStoreError::invalid_utf8(path, source))?;
let legacy: LegacyPersistedAutomation =
toml::from_str(content).map_err(|source| AutomationStoreError::parse(path, source))?;
let (target, workflow) = legacy_target(legacy.target, path)?;
let LegacyAutomationTarget {
repository,
selector,
workflow,
} = legacy.target;
let target = legacy_target(repository, &selector, path)?;
Automation::from_stored(id.clone(), revision, AutomationReplace {
name: legacy.name,
description: legacy.description,
@ -108,10 +113,10 @@ fn parse_legacy_automation(
}
fn legacy_target(
legacy: LegacyAutomationTarget,
repository: String,
selector: &str,
path: &Path,
) -> Result<(RunTarget, String), AutomationStoreError> {
let selector = legacy.selector.as_str();
) -> Result<RunTarget, AutomationStoreError> {
let (branch, tag, sha) = if let Some(sha) = repository::normalize_git_commit_sha(selector) {
("main".to_string(), None, Some(sha))
} else if let Some(tag) = selector
@ -129,19 +134,18 @@ fn legacy_target(
} else {
(selector.to_string(), None, None)
};
let target = RunTarget::Git(GitRunTarget {
repo: legacy.repository,
RunTarget::Git(GitRunTarget {
repo: repository,
branch,
tag,
sha,
})
.validate()
.map(|validated| validated.target)
.map_err(|source| AutomationStoreError::LegacyTarget {
path: path.to_path_buf(),
source,
})?
.target;
Ok((target, legacy.workflow))
})
}
async fn legacy_automation_paths(

2
lib/components/fabro-automation/src/error.rs
View file

@ -15,8 +15,6 @@ pub enum AutomationValidationError {
InvalidAutomationTriggerId { value: String },
#[error("automation name must not be empty")]
EmptyName,
#[error("repository slug {value:?} must be a GitHub owner/repo slug")]
InvalidRepositorySlug { value: String },
#[error("automation target kind {kind:?} is not supported; only Git targets are accepted")]
UnsupportedTarget { kind: String },
#[error("automation Git target is invalid")]

2
lib/components/fabro-automation/src/lib.rs
View file

@ -10,6 +10,6 @@ pub use id::{AutomationId, AutomationRevision, AutomationRevisionParseError, Aut
pub use migrations::{ImportReport, import_legacy_directory_once};
pub use model::{
ApiTrigger, Automation, AutomationDraft, AutomationReplace, AutomationTrigger, ScheduleTrigger,
parse_github_repository_slug, parse_schedule_expression,
parse_schedule_expression,
};
pub use store::AutomationStore;

36
lib/components/fabro-automation/src/model.rs
View file

@ -4,7 +4,7 @@ use std::sync::LazyLock;
use croner::Cron;
use croner::errors::CronError;
use croner::parser::{CronParser, Seconds, Year};
use fabro_types::{GitHubRepositorySlug, GitRunTarget, RunTarget};
use fabro_types::{GitRunTarget, RunTarget};
use serde::{Deserialize, Serialize};
use crate::{
@ -330,16 +330,6 @@ fn normalize_replace(
Ok(value)
}
pub fn parse_github_repository_slug(
value: &str,
) -> Result<GitHubRepositorySlug, AutomationValidationError> {
GitHubRepositorySlug::try_new(value).ok_or_else(|| {
AutomationValidationError::InvalidRepositorySlug {
value: value.to_string(),
}
})
}
fn validate_target(target: RunTarget) -> Result<RunTarget, AutomationValidationError> {
if !matches!(&target, RunTarget::Git(_)) {
return Err(AutomationValidationError::UnsupportedTarget {
@ -532,30 +522,6 @@ enabled = true
assert_eq!(trigger_ids, vec!["nightly"]);
}
#[test]
fn repository_slug_parser_returns_the_shared_type() {
let slug: fabro_types::GitHubRepositorySlug =
crate::parse_github_repository_slug("owner/.github").unwrap();
assert_eq!(slug.owner(), "owner");
assert_eq!(slug.repo(), ".github");
}
#[test]
fn invalid_repository_slug_preserves_the_automation_error() {
let error = crate::parse_github_repository_slug("not/github/slug").unwrap_err();
assert!(matches!(
&error,
AutomationValidationError::InvalidRepositorySlug { value }
if value == "not/github/slug"
));
assert_eq!(
error.to_string(),
"repository slug \"not/github/slug\" must be a GitHub owner/repo slug"
);
}
#[test]
fn invalid_git_target_preserves_the_shared_validation_error() {
let error = super::validate_target(RunTarget::Git(GitRunTarget {

15
lib/components/fabro-automation/src/store.rs
View file

@ -95,7 +95,7 @@ impl AutomationStore {
draft: AutomationReplace,
) -> Result<Automation, AutomationStoreError> {
let (automation, _) = Automation::from_replace(id.clone(), draft)?;
let target = git_target(&automation.target);
let target = stored_git_target(&automation);
let mut transaction = self.pool.begin().await?;
let result = sqlx::query(
r"
@ -283,7 +283,7 @@ pub(crate) async fn insert_automation_ignoring_conflict(
transaction: &mut Transaction<'_, Sqlite>,
automation: &Automation,
) -> Result<bool, AutomationStoreError> {
let target = git_target(&automation.target);
let target = stored_git_target(automation);
let result = sqlx::query(
r"
INSERT INTO automations (
@ -320,13 +320,10 @@ pub(crate) async fn insert_automation_ignoring_conflict(
Ok(true)
}
fn git_target(target: &RunTarget) -> &GitRunTarget {
match target {
RunTarget::Git(target) => target,
RunTarget::None {} | RunTarget::Folder { .. } => {
unreachable!("stored automations have already passed Git-only validation")
}
}
fn stored_git_target(automation: &Automation) -> &GitRunTarget {
automation
.git_target()
.expect("stored automations have already passed Git-only validation")
}
async fn insert_schedule_triggers(

20
lib/foundation/fabro-db/migrations/2026082601_automation_run_targets.sql
View file

@ -94,20 +94,10 @@ ALTER TABLE automations ADD COLUMN target_sha TEXT
UPDATE automations
SET
target_branch = (
SELECT branch
FROM automation_target_migration_candidates
WHERE automation_target_migration_candidates.id = automations.id
),
target_tag = (
SELECT tag
FROM automation_target_migration_candidates
WHERE automation_target_migration_candidates.id = automations.id
),
target_sha = (
SELECT sha
FROM automation_target_migration_candidates
WHERE automation_target_migration_candidates.id = automations.id
);
target_branch = candidates.branch,
target_tag = candidates.tag,
target_sha = candidates.sha
FROM automation_target_migration_candidates AS candidates
WHERE candidates.id = automations.id;
DROP TABLE automation_target_migration_candidates;