From 5af791c812120b3522c9120d701c796c95cfe53d Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Mon, 31 Aug 2026 17:13:20 -0400 Subject: [PATCH] Align remote workflow selectors with run targets --- .../app/components/automation-form.test.tsx | 51 +++--- .../app/components/automation-form.tsx | 172 ++++++++---------- apps/fabro-web/app/lib/automation.ts | 4 +- .../app/routes/automations-new.test.tsx | 26 +-- docs/public/api-reference/fabro-api.yaml | 60 +++--- docs/public/execution/automations.mdx | 13 +- .../src/automation_materializer.rs | 112 +++++------- lib/apps/fabro-server/src/git_checkout.rs | 37 ++-- .../src/server/automation_scheduler.rs | 29 +-- .../fabro-server/tests/it/api/automations.rs | 20 +- lib/components/fabro-automation/src/error.rs | 22 +-- lib/components/fabro-automation/src/lib.rs | 4 +- lib/components/fabro-automation/src/model.rs | 147 ++++++--------- lib/components/fabro-automation/src/store.rs | 63 +++---- .../fabro-automation/tests/store.rs | 71 ++++---- lib/foundation/fabro-api/build.rs | 7 +- lib/foundation/fabro-api/src/lib.rs | 51 +++--- .../fabro-api/tests/automation_round_trip.rs | 41 +++-- .../fabro-api/tests/run_summary_round_trip.rs | 19 +- ...2026082803_automation_workflow_sources.sql | 44 +++-- lib/foundation/fabro-db/tests/sqlite.rs | 40 ++-- lib/foundation/fabro-types/src/lib.rs | 5 +- lib/foundation/fabro-types/src/repository.rs | 20 -- lib/foundation/fabro-types/src/run_summary.rs | 24 ++- .../fabro-types/tests/run_event_serde.rs | 13 +- .../fabro-types/tests/run_spec_serde.rs | 11 +- .../src/.openapi-generator/FILES | 1 - .../automation-git-workflow-source-kind.ts | 27 --- .../models/automation-git-workflow-source.ts | 18 +- .../fabro-api-client/src/models/index.ts | 1 - ...resolved-automation-git-workflow-source.ts | 18 +- 31 files changed, 553 insertions(+), 618 deletions(-) delete mode 100644 lib/packages/fabro-api-client/src/models/automation-git-workflow-source-kind.ts diff --git a/apps/fabro-web/app/components/automation-form.test.tsx b/apps/fabro-web/app/components/automation-form.test.tsx index a74443320..d2e89406d 100644 --- a/apps/fabro-web/app/components/automation-form.test.tsx +++ b/apps/fabro-web/app/components/automation-form.test.tsx @@ -21,39 +21,38 @@ describe("automation workflow source form values", () => { }, sandbox: null, } as any); - expect(values.usesSeparateWorkflowSource).toBe(false); + expect(values.usesRemoteWorkflow).toBe(false); expect(workflowSourceFromFormValues(values)).toBeUndefined(); }); - test("branch, tag, and commit sources serialize unambiguously", () => { + test("branch, tag, and SHA selectors serialize with target precedence", () => { const base = { ...EMPTY_AUTOMATION_FORM, - usesSeparateWorkflowSource: true, + usesRemoteWorkflow: true, workflowSourceRepository: " fabro-sh/workflows ", + workflowSourceBranch: " main ", }; + expect(workflowSourceFromFormValues(base)).toEqual({ + repo: "fabro-sh/workflows", branch: "main", + }); expect(workflowSourceFromFormValues({ ...base, - workflowSourceKind: "branch", - workflowSourceRef: " main ", - })).toEqual({ repo: "fabro-sh/workflows", kind: "branch", ref: "main" }); + workflowSourceTag: " v1.2.3 ", + })).toEqual({ repo: "fabro-sh/workflows", branch: "main", tag: "v1.2.3" }); expect(workflowSourceFromFormValues({ ...base, - workflowSourceKind: "tag", - workflowSourceRef: " v1.2.3 ", - })).toEqual({ repo: "fabro-sh/workflows", kind: "tag", ref: "v1.2.3" }); - expect(workflowSourceFromFormValues({ - ...base, - workflowSourceKind: "commit", - workflowSourceRef: "ABCDEF0123456789ABCDEF0123456789ABCDEF01", + workflowSourceTag: " v1.2.3 ", + workflowSourceSha: "ABCDEF0123456789ABCDEF0123456789ABCDEF01", })).toEqual({ repo: "fabro-sh/workflows", - kind: "commit", - ref: "abcdef0123456789abcdef0123456789abcdef01", + branch: "main", + tag: "v1.2.3", + sha: "abcdef0123456789abcdef0123456789abcdef01", }); }); - test("separate source fields are required and commits need 40 hex characters", () => { + test("remote workflow fields are required and SHAs need 40 hex characters", () => { const validBase = { ...EMPTY_AUTOMATION_FORM, id: "nightly", @@ -62,15 +61,16 @@ describe("automation workflow source form values", () => { targetRepository: "fabro-sh/app", targetBranch: "main", workflow: "release", - usesSeparateWorkflowSource: true, + usesRemoteWorkflow: true, workflowSourceRepository: "fabro-sh/workflows", - workflowSourceKind: "commit" as const, - workflowSourceRef: "0123456789abcdef0123456789abcdef01234567", + workflowSourceBranch: "main", + workflowSourceSha: "0123456789abcdef0123456789abcdef01234567", }; expect(isFormValid(validBase)).toBe(true); expect(isFormValid({ ...validBase, workflowSourceRepository: "" })).toBe(false); - expect(isFormValid({ ...validBase, workflowSourceRef: "main" })).toBe(false); + expect(isFormValid({ ...validBase, workflowSourceBranch: "" })).toBe(false); + expect(isFormValid({ ...validBase, workflowSourceSha: "short" })).toBe(false); }); test("editing preserves an explicit source even when it equals the target", () => { @@ -81,17 +81,18 @@ describe("automation workflow source form values", () => { description: null, target: { kind: "git", repo: "fabro-sh/fabro", branch: "main" }, workflow: "release", - workflow_source: { repo: "fabro-sh/fabro", kind: "branch", ref: "main" }, + workflow_source: { repo: "fabro-sh/fabro", branch: "main" }, triggers: [], }); - expect(values.usesSeparateWorkflowSource).toBe(true); + expect(values.usesRemoteWorkflow).toBe(true); expect(values.workflowSourceRepository).toBe("fabro-sh/fabro"); - expect(values.workflowSourceKind).toBe("branch"); - expect(values.workflowSourceRef).toBe("main"); + expect(values.workflowSourceBranch).toBe("main"); + expect(values.workflowSourceTag).toBe(""); + expect(values.workflowSourceSha).toBe(""); expect(workflowSourceFromFormValues({ ...values, - usesSeparateWorkflowSource: false, + usesRemoteWorkflow: false, })).toBeUndefined(); }); }); diff --git a/apps/fabro-web/app/components/automation-form.tsx b/apps/fabro-web/app/components/automation-form.tsx index de8db2629..bd9d3a8ce 100644 --- a/apps/fabro-web/app/components/automation-form.tsx +++ b/apps/fabro-web/app/components/automation-form.tsx @@ -4,7 +4,6 @@ import { Switch } from "@headlessui/react"; import type { Automation, AutomationGitWorkflowSource, - AutomationGitWorkflowSourceKind, AutomationTrigger, Environment, Run, @@ -33,10 +32,11 @@ export interface AutomationFormValues { targetTag: string; targetSha: string; workflow: string; - usesSeparateWorkflowSource: boolean; + usesRemoteWorkflow: boolean; workflowSourceRepository: string; - workflowSourceKind: AutomationGitWorkflowSourceKind; - workflowSourceRef: string; + workflowSourceBranch: string; + workflowSourceTag: string; + workflowSourceSha: string; manualEnabled: boolean; scheduleEnabled: boolean; cron: string; @@ -52,10 +52,11 @@ export const EMPTY_AUTOMATION_FORM: AutomationFormValues = { targetTag: "", targetSha: "", workflow: "", - usesSeparateWorkflowSource: false, + usesRemoteWorkflow: false, workflowSourceRepository: "", - workflowSourceKind: "branch", - workflowSourceRef: "", + workflowSourceBranch: "main", + workflowSourceTag: "", + workflowSourceSha: "", manualEnabled: true, scheduleEnabled: false, cron: "0 9 * * 1-5", @@ -83,10 +84,11 @@ export function automationToFormValues(automation: Automation): AutomationFormVa targetTag: target?.tag ?? "", targetSha: target?.sha ?? "", workflow: automation.workflow, - usesSeparateWorkflowSource: workflowSource != null, + usesRemoteWorkflow: workflowSource != null, workflowSourceRepository: workflowSource?.repo ?? "", - workflowSourceKind: workflowSource?.kind ?? "branch", - workflowSourceRef: workflowSource?.ref ?? "", + workflowSourceBranch: workflowSource?.branch ?? "main", + workflowSourceTag: workflowSource?.tag ?? "", + workflowSourceSha: workflowSource?.sha ?? "", manualEnabled: apiTrigger?.enabled ?? false, scheduleEnabled: scheduleTrigger?.enabled ?? false, cron: scheduleTrigger?.expression ?? "0 9 * * 1-5", @@ -188,28 +190,24 @@ 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; + if (!values.usesRemoteWorkflow) return true; return ( values.workflowSourceRepository.trim() !== "" && - isWorkflowSourceRefValid(values.workflowSourceKind, values.workflowSourceRef) + values.workflowSourceBranch.trim() !== "" && + isOptionalShaValid(values.workflowSourceSha) ); } export function workflowSourceFromFormValues( values: AutomationFormValues, ): AutomationGitWorkflowSource | undefined { - if (!values.usesSeparateWorkflowSource) return undefined; - const reference = values.workflowSourceRef.trim(); + if (!values.usesRemoteWorkflow) return undefined; return { - repo: values.workflowSourceRepository.trim(), - kind: values.workflowSourceKind, - ref: values.workflowSourceKind === "commit" ? reference.toLowerCase() : reference, + repo: values.workflowSourceRepository.trim(), + branch: values.workflowSourceBranch.trim(), + tag: values.workflowSourceTag.trim() || undefined, + sha: values.workflowSourceSha.trim().toLowerCase() || undefined, }; } @@ -279,40 +277,6 @@ function describeCron(expression: string): string { return "Computed when saved"; } -interface WorkflowSourceKindCopy { - label: string; - placeholder: string; - help: string; -} - -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" && !valid) { - return Enter exactly 40 hexadecimal characters.; - } - return WORKFLOW_SOURCE_KINDS[kind].help; -} - interface AutomationFormFieldsProps { values: AutomationFormValues; onChange: (values: AutomationFormValues) => void; @@ -332,11 +296,7 @@ export function AutomationFormFields({ }: AutomationFormFieldsProps) { const slugTouchedRef = useRef(values.id.length > 0); const shaValid = isOptionalShaValid(values.targetSha); - const workflowSourceRefValid = isWorkflowSourceRefValid( - values.workflowSourceKind, - values.workflowSourceRef, - ); - const workflowSourceKind = WORKFLOW_SOURCE_KINDS[values.workflowSourceKind]; + const workflowSourceShaValid = isOptionalShaValid(values.workflowSourceSha); const compatibleEnvironments = environments .filter(isCloneBasedEnvironment) .sort((left, right) => left.id.localeCompare(right.id)); @@ -536,8 +496,8 @@ export function AutomationFormFields({ Workflow slug} help={ - values.usesSeparateWorkflowSource - ? "Dash-separated identifier resolved in the workflow source checkout." + values.usesRemoteWorkflow + ? "Dash-separated identifier resolved in the remote workflow checkout." : "Dash-separated identifier resolved in the run target checkout." } > @@ -554,25 +514,25 @@ export function AutomationFormFields({ /> patch({ usesSeparateWorkflowSource })} - label="Use a separate workflow source" + checked={values.usesRemoteWorkflow} + onChange={(usesRemoteWorkflow) => patch({ usesRemoteWorkflow })} + label="Use a remote workflow" /> - {values.usesSeparateWorkflowSource ? ( + {values.usesRemoteWorkflow ? ( <> Source repository} + title={} help="GitHub owner/repo containing the workflow files." > patch({ workflowSourceRepository: e.target.value })} placeholder="acme/automation-workflows" @@ -582,35 +542,53 @@ export function AutomationFormFields({ /> Source kind} - help="Choose how Fabro interprets the source ref on every firing." - > - - - {workflowSourceKind.label}} - help={workflowSourceRefHelp(values.workflowSourceKind, workflowSourceRefValid)} + title={} + help="Fallback revision and audit context. An exact SHA does not need to be reachable from this branch." > patch({ workflowSourceRef: e.target.value })} - placeholder={workflowSourceKind.placeholder} + name="workflow_source_branch" + aria-label="Remote workflow branch" + value={values.workflowSourceBranch} + onChange={(e) => patch({ workflowSourceBranch: e.target.value })} + placeholder="main" + autoComplete="off" + spellCheck={false} + className={`${INPUT_CLASS} font-mono`} + /> + + Tag} + help="Bare tag name resolved when the automation fires. Used only when exact SHA is empty." + > + patch({ workflowSourceTag: e.target.value })} + placeholder="v1.2.3" + autoComplete="off" + spellCheck={false} + className={`${INPUT_CLASS} font-mono`} + /> + + Exact SHA} + help={ + workflowSourceShaValid + ? "A 40-character commit SHA takes precedence over tag and branch. It is fetched directly and need not be reachable from the named branch." + : Enter exactly 40 hexadecimal characters. + } + > + patch({ workflowSourceSha: e.target.value })} + placeholder="0123456789abcdef0123456789abcdef01234567" 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 f90f75a22..c3ee46a14 100644 --- a/apps/fabro-web/app/lib/automation.ts +++ b/apps/fabro-web/app/lib/automation.ts @@ -34,7 +34,9 @@ export function hasEnabledApiTrigger(automation: Automation): boolean { } export function workflowSourceSummary(source: AutomationGitWorkflowSource): string { - return `${source.repo} · ${source.kind} ${source.ref}`; + if (source.sha) return `${source.repo} · commit ${source.sha}`; + if (source.tag) return `${source.repo} · tag ${source.tag}`; + return `${source.repo} · branch ${source.branch}`; } export const RUN_TARGET_CHECKOUT_LABEL = "run target checkout"; diff --git a/apps/fabro-web/app/routes/automations-new.test.tsx b/apps/fabro-web/app/routes/automations-new.test.tsx index 73608521c..6e54e4e59 100644 --- a/apps/fabro-web/app/routes/automations-new.test.tsx +++ b/apps/fabro-web/app/routes/automations-new.test.tsx @@ -310,8 +310,8 @@ describe("AutomationsNew", () => { expect(fieldValue(renderer, "Automation environment")).toBe(""); expect(switchChecked(renderer, "Enable manual and API triggers")).toBe(true); expect(switchChecked(renderer, "Enable scheduled triggers")).toBe(false); - expect(switchChecked(renderer, "Use a separate workflow source")).toBe(false); - expect(renderer.root.findAllByProps({ "aria-label": "Workflow source repository" })).toHaveLength(0); + expect(switchChecked(renderer, "Use a remote workflow")).toBe(false); + expect(renderer.root.findAllByProps({ "aria-label": "Remote workflow repository" })).toHaveLength(0); }); test("environment selector offers Docker and Daytona but not local", async () => { @@ -382,7 +382,7 @@ describe("AutomationsNew", () => { expect(fieldValue(renderer, "Automation environment")).toBe("default"); expect(switchChecked(renderer, "Enable manual and API triggers")).toBe(true); expect(switchChecked(renderer, "Enable scheduled triggers")).toBe(false); - expect(switchChecked(renderer, "Use a separate workflow source")).toBe(false); + expect(switchChecked(renderer, "Use a remote workflow")).toBe(false); expect( renderer.root.findAllByProps({ "aria-label": "Cron expression" }), ).toHaveLength(0); @@ -451,13 +451,16 @@ describe("AutomationsNew", () => { changeField(renderer, "Automation environment", "daytona-smoke"); changeField(renderer, "Workflow slug", "release"); act(() => { - byLabel(renderer, "Use a separate workflow source").props.onChange(true); - }); - changeField(renderer, "Workflow source repository", " fabro-sh/workflows "); - changeField(renderer, "Workflow source ref", "ABCDEF0123456789ABCDEF0123456789ABCDEF01"); - act(() => { - byLabel(renderer, "Workflow source kind").props.onChange({ target: { value: "commit" } }); + byLabel(renderer, "Use a remote workflow").props.onChange(true); }); + changeField(renderer, "Remote workflow repository", " fabro-sh/workflows "); + changeField(renderer, "Remote workflow branch", " release "); + changeField(renderer, "Remote workflow tag", " v2.0.0 "); + changeField( + renderer, + "Remote workflow exact commit SHA", + "ABCDEF0123456789ABCDEF0123456789ABCDEF01", + ); await act(async () => { await renderer.root.findByType("form").props.onSubmit({ preventDefault() {} }); @@ -467,8 +470,9 @@ describe("AutomationsNew", () => { expect(createAutomationMock.mock.calls[0]?.[0]).toMatchObject({ workflow_source: { repo: "fabro-sh/workflows", - kind: "commit", - ref: "abcdef0123456789abcdef0123456789abcdef01", + branch: "release", + tag: "v2.0.0", + sha: "abcdef0123456789abcdef0123456789abcdef01", }, }); }); diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 61474c200..d09d64f47 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -6728,22 +6728,18 @@ components: # ── Automations ────────────────────────────────────────────────────── - AutomationGitWorkflowSourceKind: - description: How an automation interprets the workflow source `ref`. - type: string - enum: [branch, tag, commit] - AutomationGitWorkflowSource: description: >- Explicit GitHub coordinate from which an automation acquires workflow - bytes. The kind makes `ref` unambiguous; this source is independent of - the run target and does not provide a working branch for the run. + bytes. The branch is the fallback selector and audit context. An + optional tag overrides the branch, and an optional exact SHA overrides + both without requiring branch ancestry. This source is independent of + the run target and does not provide its working branch. type: object additionalProperties: false required: - repo - - kind - - ref + - branch properties: repo: type: string @@ -6752,41 +6748,59 @@ components: pattern: "^[A-Za-z0-9][A-Za-z0-9-]*/[A-Za-z0-9._-]+$" description: GitHub repository slug in `owner/name` form. example: acme/workflows - kind: - $ref: "#/components/schemas/AutomationGitWorkflowSourceKind" - ref: + branch: type: string minLength: 1 maxLength: 255 pattern: "^[A-Za-z0-9/._-]+$" description: >- - Bare branch or tag name, or an exact 40-character commit SHA, as - selected by `kind`. Prefixes such as `refs/heads/` and `refs/tags/` - are not accepted. + Required bare branch name used when neither tag nor SHA is present. + It is retained as context when an override is present and is not + an ancestry constraint. example: main + tag: + type: string + minLength: 1 + maxLength: 255 + pattern: "^[A-Za-z0-9/._-]+$" + description: >- + Optional bare tag name. Without `sha`, this tag is resolved whenever + the automation fires. Prefixes such as `refs/tags/` are rejected. + example: v1.2.3 + sha: + type: string + pattern: "^[0-9A-Fa-f]{40}$" + description: >- + Optional exact commit, authoritative over tag and branch. The + server lowercase-normalizes it and fetches it directly; it need + not be reachable from the named branch. ResolvedAutomationGitWorkflowSource: description: >- Workflow source coordinate and exact commit captured when an - automation run was created. The requested ref remains available for - audit context while `resolved_sha` identifies the immutable source + automation run was created. The requested selectors remain available + for audit context while `resolved_sha` identifies the immutable source revision that supplied the workflow bytes. type: object additionalProperties: false required: - repo - - kind - - ref + - branch - resolved_sha properties: repo: type: string description: GitHub repository slug in `owner/name` form. - kind: - $ref: "#/components/schemas/AutomationGitWorkflowSourceKind" - ref: + branch: type: string - description: Branch, tag, or commit requested by the automation. + description: Required branch fallback and audit context. + tag: + type: string + description: Optional tag requested by the automation. + sha: + type: string + pattern: "^[0-9a-f]{40}$" + description: Optional exact commit requested by the automation. resolved_sha: type: string pattern: "^[0-9a-f]{40}$" diff --git a/docs/public/execution/automations.mdx b/docs/public/execution/automations.mdx index 468e3e0cf..a508c9fa6 100644 --- a/docs/public/execution/automations.mdx +++ b/docs/public/execution/automations.mdx @@ -45,13 +45,13 @@ An extensionless workflow such as `"release"` resolves directly to `.fabro/workf Automation admission does not read `.fabro/project.toml`. Put settings needed by the run in the workflow configuration or the selected server environment. Fabro packages the workflow and its runnable dependencies into immutable workflow versions before creating the run. -### Using a separate workflow source +### Using a remote workflow Omit `workflow_source` to resolve the `workflow` selector in the run-target checkout, as in the request above. This is the compatibility default for existing definitions. -To keep reusable workflow files in another repository, provide an explicit source with one unambiguous ref kind: +To keep reusable workflow files in another repository, enable a remote workflow and provide the same branch-plus-overrides coordinate used by Git run targets: -```json title="Create automation with a separate workflow source" +```json title="Create automation with a remote workflow" { "id": "nightly-release", "name": "Nightly release", @@ -64,8 +64,7 @@ To keep reusable workflow files in another repository, provide an explicit sourc "workflow": "release", "workflow_source": { "repo": "acme/automation-workflows", - "kind": "branch", - "ref": "main" + "branch": "main" }, "triggers": [ { "type": "api", "id": "manual", "enabled": true } @@ -73,7 +72,7 @@ To keep reusable workflow files in another repository, provide an explicit sourc } ``` -`kind` may be `branch`, `tag`, or `commit`. Branch and tag refs are bare names and are resolved again on every firing. A commit ref is exactly 40 hexadecimal characters and always selects that commit. The server uses its configured GitHub credentials independently for the target and workflow-source repositories, with read-only repository access for workflow materialization; automation requests never carry credentials. +`branch` is required and is used when neither override is present. An optional bare `tag` takes precedence over the branch, and an optional 40-character `sha` takes precedence over both. The branch is retained as fallback and audit context; Fabro fetches an exact SHA directly and does not require it to be reachable from the named branch. Branches and tags are resolved again on every firing. The server uses its configured GitHub credentials independently for the target and workflow-source repositories, with read-only repository access for workflow materialization; automation requests never carry credentials. Fabro resolves the run target to an exact commit and checks out the selected workflow source before it creates a run. It packages the workflow and its dependencies into immutable, content-addressed workflow versions, then admits the run with the root workflow-version ID and the independently exact run target. The run's automation metadata records both the requested workflow-source coordinate and its resolved commit, so the mutable source can be audited after its branch or tag moves. If an explicit source names the same repository and effective selector as the target, Fabro reuses the checkout without removing the explicit saved source. @@ -148,7 +147,7 @@ The server fires each enabled schedule trigger at its next occurrence and create The `/automations` area lists automations with create, edit, delete, and Run actions. The create and edit forms require a Docker or Daytona environment. Migrated automations without an environment are shown as incomplete and cannot run until edited. Saves are revision-checked, so concurrent edits fail loudly instead of silently overwriting each other. The detail page shows the automation's configuration, its most recent schedule error, and its run history with status, time, and repo filters. -To bootstrap an automation from work you have already run, open a run's actions menu and choose **Create automation from run** — the new-automation form is pre-filled from that run's target repository and workflow. Its workflow source defaults to the target checkout because normal run summaries do not retain the automation's mutable source coordinate. You can select a separate source before saving. Runs that were created by an automation show **View automation** instead. +To bootstrap an automation from work you have already run, open a run's actions menu and choose **Create automation from run** — the new-automation form is pre-filled from that run's target repository and workflow. Its workflow source defaults to the target checkout because normal run summaries do not retain the automation's mutable source coordinate. You can enable a remote workflow before saving. Runs that were created by an automation show **View automation** instead. ## API diff --git a/lib/apps/fabro-server/src/automation_materializer.rs b/lib/apps/fabro-server/src/automation_materializer.rs index 9981a8f37..fad2958d5 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -2,7 +2,9 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use async_trait::async_trait; -use fabro_automation::{AutomationGitWorkflowSource, AutomationId, AutomationValidationError}; +use fabro_automation::{ + AutomationGitWorkflowSource, AutomationId, AutomationValidationError, validate_workflow_source, +}; use fabro_manifest::WorkflowVersionCollectError; use fabro_types::{ GitHubRepositorySlug, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunId, RunIntent, @@ -245,7 +247,7 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { })?; let workflow_source = input .workflow_source - .map(AutomationGitWorkflowSource::validate) + .map(validate_workflow_source) .transpose() .map_err(|source| RunMaterializeError::InvalidWorkflowSource { source })?; // A workflow source naming the target's exact coordinate shares its @@ -257,9 +259,9 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { .repo .parse::() .map(|repo| (repo, source)) - .map_err(|source| RunMaterializeError::InvalidWorkflowSource { - source: AutomationValidationError::InvalidWorkflowSourceRepository { - source, + .map_err(|_| RunMaterializeError::InvalidWorkflowSource { + source: AutomationValidationError::InvalidWorkflowSource { + source: TargetValidationError::Repository, }, }) }) @@ -324,12 +326,10 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { }; exact_target.sha = Some(checked_out_sha); let resolved_workflow_source = workflow_source.map(|source| { - Box::new(ResolvedAutomationGitWorkflowSource { - repo: source.repo, - kind: source.kind, - reference: source.reference, - resolved_sha: workflow_checkout_sha, - }) + Box::new(ResolvedAutomationGitWorkflowSource::from_requested( + source, + workflow_checkout_sha, + )) }); let workflow = PathBuf::from(input.workflow); @@ -397,7 +397,9 @@ impl From for RunMaterializeError { match failure { TestMaterializeFailure::InvalidTarget(source) => Self::InvalidTarget { source }, TestMaterializeFailure::InvalidWorkflowSource => Self::InvalidWorkflowSource { - source: AutomationValidationError::InvalidWorkflowSourceBranch, + source: AutomationValidationError::InvalidWorkflowSource { + source: TargetValidationError::Branch, + }, }, } } @@ -506,12 +508,10 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer { input: AutomationRunMaterializeInput, ) -> Result { let workflow_source = input.workflow_source.as_ref().map(|source| { - Box::new(ResolvedAutomationGitWorkflowSource { - repo: source.repo.clone(), - kind: source.kind, - reference: source.reference.clone(), - resolved_sha: "ffffffffffffffffffffffffffffffffffffffff".to_string(), - }) + Box::new(ResolvedAutomationGitWorkflowSource::from_requested( + source.clone(), + "ffffffffffffffffffffffffffffffffffffffff".to_string(), + )) }); let response = { let mut guard = self @@ -559,7 +559,6 @@ mod tests { use std::sync::Mutex; use std::time::Duration; - use fabro_automation::AutomationGitWorkflowSourceKind; use object_store::memory::InMemory; use tempfile::TempDir; @@ -762,13 +761,15 @@ mod tests { fn source( repo: &str, - kind: AutomationGitWorkflowSourceKind, - reference: &str, + branch: &str, + tag: Option<&str>, + sha: Option<&str>, ) -> AutomationGitWorkflowSource { AutomationGitWorkflowSource { - repo: repo.to_string(), - kind, - reference: reference.to_string(), + repo: repo.to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), } } @@ -924,11 +925,7 @@ mod tests { let materialized = materializer .materialize(input( "fabro-sh/target", - Some(source( - "fabro-sh/workflows", - AutomationGitWorkflowSourceKind::Branch, - "main", - )), + Some(source("fabro-sh/workflows", "main", None, None)), &temp.path().join("runs"), )) .await @@ -942,8 +939,9 @@ mod tests { materialized.workflow_source, Some(Box::new(ResolvedAutomationGitWorkflowSource { repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Branch, - reference: "main".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, resolved_sha: source_fixture.initial_sha.clone(), })) ); @@ -980,11 +978,7 @@ mod tests { let materialized = materializer .materialize(input( "Fabro-Sh/Shared", - Some(source( - "fabro-sh/shared", - AutomationGitWorkflowSourceKind::Branch, - "main", - )), + Some(source("fabro-sh/shared", "main", None, None)), &temp.path().join("runs"), )) .await @@ -994,8 +988,9 @@ mod tests { materialized.workflow_source, Some(Box::new(ResolvedAutomationGitWorkflowSource { repo: "fabro-sh/shared".to_string(), - kind: AutomationGitWorkflowSourceKind::Branch, - reference: "main".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, resolved_sha: fixture.initial_sha, })) ); @@ -1020,8 +1015,9 @@ mod tests { "fabro-sh/shared", Some(source( "fabro-sh/shared", - AutomationGitWorkflowSourceKind::Tag, - "annotated-v1", + "main", + Some("annotated-v1"), + None, )), &temp.path().join("runs"), )) @@ -1032,7 +1028,7 @@ mod tests { } #[tokio::test] - async fn source_ref_modes_pin_commits_while_branches_advance() { + async fn source_selectors_pin_commits_while_branches_advance() { let temp = TempDir::new().unwrap(); let target_fixture = seed_repository(temp.path(), "target", "target workflow"); let source_fixture = seed_repository(temp.path(), "source", "source v1"); @@ -1054,27 +1050,26 @@ mod tests { ]), ); let runs = temp.path().join("runs"); - let materialize = |kind, reference: &str| { + let materialize = |branch: &str, tag: Option<&str>, sha: Option<&str>| { materializer.materialize(input( "fabro-sh/target", - Some(source("fabro-sh/source", kind, reference)), + Some(source("fabro-sh/source", branch, tag, sha)), &runs, )) }; - let branch_v1 = materialize(AutomationGitWorkflowSourceKind::Branch, "main") + let branch_v1 = materialize("main", None, None) .await .unwrap() .workflow_version_id; for tag in ["annotated-v1", "lightweight-v1"] { - let tagged = materialize(AutomationGitWorkflowSourceKind::Tag, tag) - .await - .unwrap(); + let tagged = materialize("main", Some(tag), None).await.unwrap(); assert_eq!(tagged.workflow_version_id, branch_v1, "{tag}"); } let committed_v1 = materialize( - AutomationGitWorkflowSourceKind::Commit, - &source_fixture.initial_sha, + "branch-that-does-not-exist", + Some("missing-tag"), + Some(&source_fixture.initial_sha), ) .await .unwrap() @@ -1082,14 +1077,15 @@ mod tests { assert_eq!(committed_v1, branch_v1); advance_repository(&source_fixture, "source v2"); - let branch_v2 = materialize(AutomationGitWorkflowSourceKind::Branch, "main") + let branch_v2 = materialize("main", None, None) .await .unwrap() .workflow_version_id; assert_ne!(branch_v2, branch_v1); let committed_after_advance = materialize( - AutomationGitWorkflowSourceKind::Commit, - &source_fixture.initial_sha, + "branch-that-does-not-exist", + None, + Some(&source_fixture.initial_sha), ) .await .unwrap() @@ -1165,11 +1161,7 @@ mod tests { ) .materialize(input( "fabro-sh/target", - Some(source( - "fabro-sh/source", - AutomationGitWorkflowSourceKind::Branch, - "main", - )), + Some(source("fabro-sh/source", "main", None, None)), &temp.path().join("source-failure"), )) .await @@ -1197,11 +1189,7 @@ mod tests { ) .materialize(input( "fabro-sh/target", - Some(source( - "fabro-sh/source", - AutomationGitWorkflowSourceKind::Branch, - "missing", - )), + Some(source("fabro-sh/source", "missing", None, None)), &temp.path().join("checkout-failure"), )) .await diff --git a/lib/apps/fabro-server/src/git_checkout.rs b/lib/apps/fabro-server/src/git_checkout.rs index be90f1ca4..5241ef973 100644 --- a/lib/apps/fabro-server/src/git_checkout.rs +++ b/lib/apps/fabro-server/src/git_checkout.rs @@ -4,7 +4,6 @@ use std::time::Duration; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; -use fabro_automation::{AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind}; use fabro_store::KeyedMutex; use fabro_types::{GitHubRepositorySlug, GitRunTarget}; use tokio::process::Command; @@ -220,16 +219,6 @@ impl<'a> From<&'a GitRunTarget> for GitCheckoutSelector<'a> { } } -impl<'a> From<&'a AutomationGitWorkflowSource> for GitCheckoutSelector<'a> { - fn from(source: &'a AutomationGitWorkflowSource) -> Self { - match source.kind { - AutomationGitWorkflowSourceKind::Branch => Self::Branch(&source.reference), - AutomationGitWorkflowSourceKind::Tag => Self::Tag(&source.reference), - AutomationGitWorkflowSourceKind::Commit => Self::Commit(&source.reference), - } - } -} - impl GitCheckoutSelector<'_> { fn selector(&self) -> Cow<'_, str> { match self { @@ -564,7 +553,6 @@ mod tests { use std::fs; use std::path::Path; - use fabro_automation::{AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind}; use tempfile::TempDir; use super::*; @@ -582,19 +570,17 @@ mod tests { } } - fn workflow_source( - kind: AutomationGitWorkflowSourceKind, - reference: &str, - ) -> AutomationGitWorkflowSource { - AutomationGitWorkflowSource { - repo: "fabro-sh/workflows".to_string(), - kind, - reference: reference.to_string(), + fn workflow_source(branch: &str, tag: Option<&str>, sha: Option<&str>) -> GitRunTarget { + GitRunTarget { + repo: "fabro-sh/workflows".to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), } } #[test] - fn checkout_selectors_preserve_target_precedence_and_source_kind() { + fn checkout_selectors_use_sha_then_tag_then_branch_precedence() { let target = git_target( "main", Some("v1"), @@ -607,17 +593,18 @@ mod tests { for (source, expected) in [ ( - workflow_source(AutomationGitWorkflowSourceKind::Branch, "main"), + workflow_source("main", None, None), GitCheckoutSelector::Branch("main"), ), ( - workflow_source(AutomationGitWorkflowSourceKind::Tag, "v1"), + workflow_source("main", Some("v1"), None), GitCheckoutSelector::Tag("v1"), ), ( workflow_source( - AutomationGitWorkflowSourceKind::Commit, - "abcdef0123456789abcdef0123456789abcdef01", + "unrelated-context", + Some("v1"), + Some("abcdef0123456789abcdef0123456789abcdef01"), ), GitCheckoutSelector::Commit("abcdef0123456789abcdef0123456789abcdef01"), ), diff --git a/lib/apps/fabro-server/src/server/automation_scheduler.rs b/lib/apps/fabro-server/src/server/automation_scheduler.rs index 72a149745..3bceb22d3 100644 --- a/lib/apps/fabro-server/src/server/automation_scheduler.rs +++ b/lib/apps/fabro-server/src/server/automation_scheduler.rs @@ -391,8 +391,7 @@ fn run_due_schedules_once<'a>( #[cfg(test)] mod tests { use fabro_automation::{ - AutomationDraft, AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind, - AutomationTrigger, ScheduleTrigger, + AutomationDraft, AutomationGitWorkflowSource, AutomationTrigger, ScheduleTrigger, }; use fabro_static::EnvVars; use fabro_store::ListRunsQuery; @@ -723,9 +722,10 @@ mod tests { let materializer = succeeding_materializer(); let state = test_state_with_materializer(materializer.clone()); let workflow_source = AutomationGitWorkflowSource { - repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Commit, - reference: "0123456789abcdef0123456789abcdef01234567".to_string(), + repo: "fabro-sh/workflows".to_string(), + branch: "context-only".to_string(), + tag: Some("v1".to_string()), + sha: Some("0123456789abcdef0123456789abcdef01234567".to_string()), }; create_automation_with_source( state.as_ref(), @@ -750,12 +750,12 @@ mod tests { .automation .as_ref() .and_then(|automation| automation.workflow_source.clone()), - Some(Box::new(ResolvedAutomationGitWorkflowSource { - repo: workflow_source.repo, - kind: workflow_source.kind, - reference: workflow_source.reference, - resolved_sha: "ffffffffffffffffffffffffffffffffffffffff".to_string(), - })) + Some(Box::new( + ResolvedAutomationGitWorkflowSource::from_requested( + workflow_source, + "ffffffffffffffffffffffffffffffffffffffff".to_string(), + ) + )) ); } @@ -870,9 +870,10 @@ mod tests { "failing-source", "failing-source", Some(AutomationGitWorkflowSource { - repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Branch, - reference: "main".to_string(), + repo: "fabro-sh/workflows".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, }), vec![schedule_trigger("schedule", "* * * * *", true)], ) diff --git a/lib/apps/fabro-server/tests/it/api/automations.rs b/lib/apps/fabro-server/tests/it/api/automations.rs index c4d61746c..d5dfa1e4d 100644 --- a/lib/apps/fabro-server/tests/it/api/automations.rs +++ b/lib/apps/fabro-server/tests/it/api/automations.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use axum::body::Body; use axum::http::{Method, Request, StatusCode, header}; -use fabro_automation::{AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind}; +use fabro_automation::AutomationGitWorkflowSource; use fabro_config::Storage; use fabro_server::server::build_router; use fabro_server::test_support::{ @@ -1047,8 +1047,8 @@ async fn api_triggered_run_passes_saved_workflow_source_to_materialization() { let mut body = automation_body("nightly", "Nightly"); body["workflow_source"] = json!({ "repo": "fabro-sh/workflows", - "kind": "tag", - "ref": "release-v1" + "branch": "main", + "tag": "release-v1" }); create_automation_with_body(&app, &body).await; @@ -1059,17 +1059,18 @@ async fn api_triggered_run_passes_saved_workflow_source_to_materialization() { assert_eq!( captured[0], Some(AutomationGitWorkflowSource { - repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Tag, - reference: "release-v1".to_string(), + repo: "fabro-sh/workflows".to_string(), + branch: "main".to_string(), + tag: Some("release-v1".to_string()), + sha: None, }) ); assert_eq!( created["automation"]["workflow_source"], json!({ "repo": "fabro-sh/workflows", - "kind": "tag", - "ref": "release-v1", + "branch": "main", + "tag": "release-v1", "resolved_sha": "ffffffffffffffffffffffffffffffffffffffff" }) ); @@ -1101,8 +1102,7 @@ async fn api_workflow_source_failure_does_not_create_or_start_a_run() { let mut body = automation_body("nightly", "Nightly"); body["workflow_source"] = json!({ "repo": "fabro-sh/workflows", - "kind": "branch", - "ref": "main" + "branch": "main" }); create_automation_with_body(&app, &body).await; diff --git a/lib/components/fabro-automation/src/error.rs b/lib/components/fabro-automation/src/error.rs index 711d2844a..44246bde7 100644 --- a/lib/components/fabro-automation/src/error.rs +++ b/lib/components/fabro-automation/src/error.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use croner::errors::CronError; -use fabro_types::{GitHubRepositorySlugError, TargetValidationError}; +use fabro_types::TargetValidationError; use toml::de::Error as TomlDeError; use toml::ser::Error as TomlSerError; @@ -24,17 +24,11 @@ pub enum AutomationValidationError { #[source] source: TargetValidationError, }, - #[error("automation workflow source repository must be a valid GitHub owner/name slug")] - InvalidWorkflowSourceRepository { + #[error("automation workflow source is invalid")] + InvalidWorkflowSource { #[source] - source: GitHubRepositorySlugError, + source: TargetValidationError, }, - #[error("automation workflow source branch must be a non-empty bare branch name")] - InvalidWorkflowSourceBranch, - #[error("automation workflow source tag must be a non-empty bare tag name")] - InvalidWorkflowSourceTag, - #[error("automation workflow source commit must be exactly 40 ASCII hexadecimal characters")] - InvalidWorkflowSourceCommit, #[error("workflow selector {value:?} is not safe")] InvalidWorkflowSelector { value: String }, #[error("duplicate automation trigger id {id:?}")] @@ -90,13 +84,6 @@ pub enum AutomationStoreError { StoredTriggerShape { id: AutomationId }, #[error("stored automation {id} has a partial workflow source coordinate")] StoredWorkflowSourceShape { id: AutomationId }, - #[error("stored automation {id} has unknown workflow source kind {kind:?}")] - StoredWorkflowSourceKind { - id: AutomationId, - kind: String, - #[source] - source: strum::ParseError, - }, #[error("stored automation {id} has an invalid revision")] InvalidRevision { id: AutomationId, @@ -184,7 +171,6 @@ impl AutomationStoreError { Self::StoredId { .. } => "stored_id", Self::StoredTriggerShape { .. } => "stored_trigger_shape", Self::StoredWorkflowSourceShape { .. } => "stored_workflow_source_shape", - Self::StoredWorkflowSourceKind { .. } => "stored_workflow_source_kind", Self::InvalidRevision { .. } => "invalid_revision", Self::Db { .. } => "db", Self::InvalidFilename { .. } => "invalid_filename", diff --git a/lib/components/fabro-automation/src/lib.rs b/lib/components/fabro-automation/src/lib.rs index f4bd12ea2..aeca76c9d 100644 --- a/lib/components/fabro-automation/src/lib.rs +++ b/lib/components/fabro-automation/src/lib.rs @@ -5,7 +5,7 @@ mod model; mod store; pub use error::{AutomationStoreError, AutomationValidationError}; -pub use fabro_types::{AutomationGitWorkflowSourceKind, GitHubRepositorySlug}; +pub use fabro_types::GitHubRepositorySlug; pub use id::{AutomationId, AutomationRevision, AutomationRevisionParseError, AutomationTriggerId}; pub use migrations::{ EnvironmentSelectorBackfillReport, ImportReport, backfill_environment_selectors, @@ -13,6 +13,6 @@ pub use migrations::{ }; pub use model::{ ApiTrigger, Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationReplace, - AutomationTrigger, ScheduleTrigger, parse_schedule_expression, + AutomationTrigger, ScheduleTrigger, parse_schedule_expression, validate_workflow_source, }; pub use store::AutomationStore; diff --git a/lib/components/fabro-automation/src/model.rs b/lib/components/fabro-automation/src/model.rs index 2f7212189..62ea4e4a2 100644 --- a/lib/components/fabro-automation/src/model.rs +++ b/lib/components/fabro-automation/src/model.rs @@ -4,10 +4,7 @@ use std::sync::LazyLock; use croner::Cron; use croner::errors::CronError; use croner::parser::{CronParser, Seconds, Year}; -use fabro_types::{ - AutomationGitWorkflowSourceKind, GitHubRepositorySlug, GitRunTarget, RunTarget, - is_valid_git_branch_name, is_valid_git_tag_name, normalize_git_commit_sha, -}; +use fabro_types::{GitRunTarget, RunTarget}; use serde::{Deserialize, Serialize}; use crate::{ @@ -151,42 +148,27 @@ impl Automation { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct AutomationGitWorkflowSource { - pub repo: String, - pub kind: AutomationGitWorkflowSourceKind, - #[serde(rename = "ref")] - pub reference: String, -} +/// A Git coordinate from which an automation loads workflow files. +/// +/// This deliberately reuses the run target's branch/tag/SHA model. The branch +/// is the fallback selector and audit context; it does not constrain an exact +/// SHA to be reachable from that branch. +pub type AutomationGitWorkflowSource = GitRunTarget; -impl AutomationGitWorkflowSource { - /// Validate and canonicalize this saved GitHub workflow coordinate without - /// resolving remote repository state. - pub fn validate(mut self) -> Result { - self.repo - .parse::() - .map_err( - |source| AutomationValidationError::InvalidWorkflowSourceRepository { source }, - )?; - match self.kind { - AutomationGitWorkflowSourceKind::Branch => { - if !is_valid_git_branch_name(&self.reference) { - return Err(AutomationValidationError::InvalidWorkflowSourceBranch); - } +/// Validate and canonicalize a saved workflow source without resolving remote +/// repository state. +pub fn validate_workflow_source( + source: AutomationGitWorkflowSource, +) -> Result { + RunTarget::Git(source) + .validate() + .map(|validated| match validated.target { + RunTarget::Git(source) => source, + RunTarget::None {} | RunTarget::Folder { .. } => { + unreachable!("a validated Git workflow source remains Git-backed") } - AutomationGitWorkflowSourceKind::Tag => { - if !is_valid_git_tag_name(&self.reference) { - return Err(AutomationValidationError::InvalidWorkflowSourceTag); - } - } - AutomationGitWorkflowSourceKind::Commit => { - self.reference = normalize_git_commit_sha(&self.reference) - .ok_or(AutomationValidationError::InvalidWorkflowSourceCommit)?; - } - } - Ok(self) - } + }) + .map_err(|source| AutomationValidationError::InvalidWorkflowSource { source }) } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -376,7 +358,7 @@ fn normalize_replace( .filter(|environment_id| !environment_id.is_empty()); value.workflow_source = value .workflow_source - .map(AutomationGitWorkflowSource::validate) + .map(validate_workflow_source) .transpose()?; validate_fields(&value, require_environment)?; @@ -491,9 +473,9 @@ mod tests { use fabro_types::{GitRunTarget, RunTarget, TargetValidationError}; use crate::{ - ApiTrigger, Automation, AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind, - AutomationId, AutomationReplace, AutomationStoreError, AutomationTrigger, - AutomationTriggerId, AutomationValidationError, ScheduleTrigger, + ApiTrigger, Automation, AutomationGitWorkflowSource, AutomationId, AutomationReplace, + AutomationStoreError, AutomationTrigger, AutomationTriggerId, AutomationValidationError, + ScheduleTrigger, }; fn target() -> RunTarget { @@ -525,13 +507,15 @@ mod tests { } fn workflow_source( - kind: AutomationGitWorkflowSourceKind, - reference: &str, + branch: &str, + tag: Option<&str>, + sha: Option<&str>, ) -> AutomationGitWorkflowSource { AutomationGitWorkflowSource { - repo: "fabro-sh/workflows".to_string(), - kind, - reference: reference.to_string(), + repo: "fabro-sh/workflows".to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), } } @@ -593,29 +577,27 @@ mod tests { } #[test] - fn workflow_sources_round_trip_and_commits_are_canonicalized() { - for (kind, reference, expected) in [ - (AutomationGitWorkflowSourceKind::Branch, "main", "main"), + fn workflow_sources_round_trip_and_shas_are_canonicalized() { + for (source, expected_sha) in [ + (workflow_source("main", None, None), None), + (workflow_source("main", Some("release/v1"), None), None), ( - AutomationGitWorkflowSourceKind::Tag, - "release/v1", - "release/v1", - ), - ( - AutomationGitWorkflowSourceKind::Commit, - "ABCDEF0123456789ABCDEF0123456789ABCDEF01", - "abcdef0123456789abcdef0123456789abcdef01", + workflow_source( + "context-only", + Some("release/v1"), + Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01"), + ), + Some("abcdef0123456789abcdef0123456789abcdef01"), ), ] { let (automation, bytes) = Automation::from_replace( AutomationId::new("nightly").unwrap(), - replace_with_source(Some(workflow_source(kind, reference))), + replace_with_source(Some(source)), ) .unwrap(); let source = automation.workflow_source.as_ref().unwrap(); - assert_eq!(source.kind, kind); - assert_eq!(source.reference, expected); + assert_eq!(source.sha.as_deref(), expected_sha); assert!( String::from_utf8(bytes.clone()) .unwrap() @@ -635,7 +617,7 @@ mod tests { replace_with_source(None), ) .unwrap(); - let mut explicit_source = workflow_source(AutomationGitWorkflowSourceKind::Branch, "main"); + let mut explicit_source = workflow_source("main", None, None); explicit_source.repo = "FABRO-SH/FABRO".to_string(); let (explicit, _) = Automation::from_replace( AutomationId::new("nightly").unwrap(), @@ -651,25 +633,25 @@ mod tests { fn workflow_source_validation_reports_the_invalid_coordinate_part() { let cases = [ ( - workflow_source(AutomationGitWorkflowSourceKind::Branch, "main"), - "repo", + workflow_source("main", None, None), + TargetValidationError::Repository, ), ( - workflow_source(AutomationGitWorkflowSourceKind::Branch, "refs/heads/main"), - "branch", + workflow_source("refs/heads/main", None, None), + TargetValidationError::Branch, ), ( - workflow_source(AutomationGitWorkflowSourceKind::Tag, "tags/v1"), - "tag", + workflow_source("main", Some("tags/v1"), None), + TargetValidationError::Tag, ), ( - workflow_source(AutomationGitWorkflowSourceKind::Commit, "short"), - "commit", + workflow_source("main", None, Some("short")), + TargetValidationError::Sha, ), ]; - for (mut source, expected_kind) in cases { - if expected_kind == "repo" { + for (mut source, expected) in cases { + if expected == TargetValidationError::Repository { source.repo = "not/a/github/slug".to_string(); } let error = Automation::from_replace( @@ -680,22 +662,11 @@ mod tests { let AutomationStoreError::Validation { source } = error else { panic!("expected validation error"); }; - assert!(match expected_kind { - "repo" => matches!( - source, - AutomationValidationError::InvalidWorkflowSourceRepository { .. } - ), - "branch" => matches!( - source, - AutomationValidationError::InvalidWorkflowSourceBranch - ), - "tag" => matches!(source, AutomationValidationError::InvalidWorkflowSourceTag), - "commit" => matches!( - source, - AutomationValidationError::InvalidWorkflowSourceCommit - ), - _ => false, - }); + assert!(matches!( + source, + AutomationValidationError::InvalidWorkflowSource { source } + if source == expected + )); } } diff --git a/lib/components/fabro-automation/src/store.rs b/lib/components/fabro-automation/src/store.rs index 5ec5fd62f..07bb8dcdf 100644 --- a/lib/components/fabro-automation/src/store.rs +++ b/lib/components/fabro-automation/src/store.rs @@ -6,9 +6,9 @@ use sqlx::sqlite::SqliteRow; use sqlx::{Row as _, Sqlite, Transaction}; use crate::{ - ApiTrigger, Automation, AutomationDraft, AutomationGitWorkflowSource, - AutomationGitWorkflowSourceKind, AutomationId, AutomationReplace, AutomationRevision, - AutomationStoreError, AutomationTrigger, AutomationTriggerId, ScheduleTrigger, + ApiTrigger, Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationId, + AutomationReplace, AutomationRevision, AutomationStoreError, AutomationTrigger, + AutomationTriggerId, ScheduleTrigger, }; /// Shared projection for loading automations with their schedule triggers. @@ -30,8 +30,9 @@ macro_rules! select_automations_sql { a.target_sha, a.target_workflow, a.workflow_source_repository, - a.workflow_source_kind, - a.workflow_source_ref, + a.workflow_source_branch, + a.workflow_source_tag, + a.workflow_source_sha, t.id AS trigger_id, t.enabled AS trigger_enabled, t.expression AS trigger_expression @@ -146,8 +147,9 @@ impl AutomationStore { target_sha = ?, target_workflow = ?, workflow_source_repository = ?, - workflow_source_kind = ?, - workflow_source_ref = ? + workflow_source_branch = ?, + workflow_source_tag = ?, + workflow_source_sha = ? WHERE id = ? AND revision = ? ", ) @@ -162,8 +164,9 @@ impl AutomationStore { .bind(target.sha.as_deref()) .bind(&automation.workflow) .bind(workflow_source.map(|source| source.repo.as_str())) - .bind(workflow_source.map(|source| <&'static str>::from(source.kind))) - .bind(workflow_source.map(|source| source.reference.as_str())) + .bind(workflow_source.map(|source| source.branch.as_str())) + .bind(workflow_source.and_then(|source| source.tag.as_deref())) + .bind(workflow_source.and_then(|source| source.sha.as_deref())) .bind(id.as_str()) .bind(expected.as_str()) .execute(&mut *transaction) @@ -355,9 +358,10 @@ pub(crate) async fn insert_automation_ignoring_conflict( target_sha, target_workflow, workflow_source_repository, - workflow_source_kind, - workflow_source_ref - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + workflow_source_branch, + workflow_source_tag, + workflow_source_sha + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO NOTHING ", ) @@ -373,8 +377,9 @@ 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| <&'static str>::from(source.kind))) - .bind(workflow_source.map(|source| source.reference.as_str())) + .bind(workflow_source.map(|source| source.branch.as_str())) + .bind(workflow_source.and_then(|source| source.tag.as_deref())) + .bind(workflow_source.and_then(|source| source.sha.as_deref())) .execute(&mut **transaction) .await?; if result.rows_affected() == 0 { @@ -389,25 +394,17 @@ fn stored_workflow_source( id: &AutomationId, ) -> Result, AutomationStoreError> { let repository = row.try_get::, _>("workflow_source_repository")?; - let kind = row.try_get::, _>("workflow_source_kind")?; - let reference = row.try_get::, _>("workflow_source_ref")?; - match (repository, kind, reference) { - (None, None, None) => Ok(None), - (Some(repo), Some(kind), Some(reference)) => { - let parsed_kind = - AutomationGitWorkflowSourceKind::from_str(&kind).map_err(|source| { - AutomationStoreError::StoredWorkflowSourceKind { - id: id.clone(), - kind, - source, - } - })?; - Ok(Some(AutomationGitWorkflowSource { - repo, - kind: parsed_kind, - reference, - })) - } + let branch = row.try_get::, _>("workflow_source_branch")?; + let tag = row.try_get::, _>("workflow_source_tag")?; + let sha = row.try_get::, _>("workflow_source_sha")?; + match (repository, branch) { + (None, None) if tag.is_none() && sha.is_none() => Ok(None), + (Some(repo), Some(branch)) => Ok(Some(AutomationGitWorkflowSource { + repo, + branch, + tag, + sha, + })), _ => Err(AutomationStoreError::StoredWorkflowSourceShape { id: id.clone() }), } } diff --git a/lib/components/fabro-automation/tests/store.rs b/lib/components/fabro-automation/tests/store.rs index afce7da7a..993494cf0 100644 --- a/lib/components/fabro-automation/tests/store.rs +++ b/lib/components/fabro-automation/tests/store.rs @@ -6,9 +6,9 @@ use std::path::Path; use fabro_automation::{ - ApiTrigger, AutomationDraft, AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind, - AutomationId, AutomationReplace, AutomationRevision, AutomationStore, AutomationStoreError, - AutomationTrigger, AutomationTriggerId, ScheduleTrigger, + ApiTrigger, AutomationDraft, AutomationGitWorkflowSource, AutomationId, AutomationReplace, + AutomationRevision, AutomationStore, AutomationStoreError, AutomationTrigger, + AutomationTriggerId, ScheduleTrigger, }; use fabro_db::Database; use fabro_types::{GitRunTarget, RunTarget}; @@ -43,13 +43,15 @@ fn schedule(id: &str, expression: &str, enabled: bool) -> AutomationTrigger { } fn workflow_source( - kind: AutomationGitWorkflowSourceKind, - reference: &str, + branch: &str, + tag: Option<&str>, + sha: Option<&str>, ) -> AutomationGitWorkflowSource { AutomationGitWorkflowSource { - repo: "fabro-sh/workflows".to_string(), - kind, - reference: reference.to_string(), + repo: "fabro-sh/workflows".to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), } } @@ -284,16 +286,17 @@ async fn insert_environment(pool: &fabro_db::DbPool, id: &str, provider: &str) { } #[tokio::test] -async fn crud_round_trips_each_workflow_source_kind_and_clears_to_omission() { +async fn crud_round_trips_workflow_source_selectors_and_clears_to_omission() { let (_dir, database) = test_database().await; let store = AutomationStore::new(database.clone_pool()); for (index, source) in [ - workflow_source(AutomationGitWorkflowSourceKind::Branch, "main"), - workflow_source(AutomationGitWorkflowSourceKind::Tag, "release/v1"), + workflow_source("main", None, None), + workflow_source("main", Some("release/v1"), None), workflow_source( - AutomationGitWorkflowSourceKind::Commit, - "ABCDEF0123456789ABCDEF0123456789ABCDEF01", + "context-only", + Some("release/v1"), + Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01"), ), ] .into_iter() @@ -303,14 +306,12 @@ async fn crud_round_trips_each_workflow_source_kind_and_clears_to_omission() { let mut value = draft(&id, true); value.workflow_source = Some(source); let created = store.create(value).await.unwrap(); - let expected_reference = if index == 2 { - "abcdef0123456789abcdef0123456789abcdef01" - } else { - created.workflow_source.as_ref().unwrap().reference.as_str() - }; assert_eq!( - created.workflow_source.as_ref().unwrap().reference, - expected_reference + created + .workflow_source + .as_ref() + .and_then(|source| source.sha.as_deref()), + (index == 2).then_some("abcdef0123456789abcdef0123456789abcdef01") ); assert_eq!(store.get(&created.id).await.unwrap(), Some(created.clone())); @@ -322,7 +323,8 @@ async fn crud_round_trips_each_workflow_source_kind_and_clears_to_omission() { .unwrap(); assert_eq!(replaced.workflow_source, None); let columns = sqlx::query( - "SELECT workflow_source_repository, workflow_source_kind, workflow_source_ref \ + "SELECT workflow_source_repository, workflow_source_branch, workflow_source_tag, \ + workflow_source_sha \ FROM automations WHERE id = ?", ) .bind(created.id.as_str()) @@ -334,11 +336,15 @@ async fn crud_round_trips_each_workflow_source_kind_and_clears_to_omission() { None ); assert_eq!( - columns.get::, _>("workflow_source_kind"), + columns.get::, _>("workflow_source_branch"), None ); assert_eq!( - columns.get::, _>("workflow_source_ref"), + columns.get::, _>("workflow_source_tag"), + None + ); + assert_eq!( + columns.get::, _>("workflow_source_sha"), None ); } @@ -351,7 +357,7 @@ async fn corrupt_workflow_source_rows_are_rejected_as_stored_shape_errors() { let (_dir, database) = test_database().await; let store = AutomationStore::new(database.clone_pool()); let partial = store.create(draft("partial", true)).await.unwrap(); - let unknown = store.create(draft("unknown", true)).await.unwrap(); + let orphan = store.create(draft("orphan", true)).await.unwrap(); let mut connection = database.pool().acquire().await.unwrap(); sqlx::query("DROP TRIGGER automation_workflow_source_all_or_none_update") @@ -369,14 +375,11 @@ async fn corrupt_workflow_source_rows_are_rejected_as_stored_shape_errors() { .execute(&mut *connection) .await .unwrap(); - sqlx::query( - "UPDATE automations SET workflow_source_repository = 'fabro-sh/workflows', \ - workflow_source_kind = 'unknown', workflow_source_ref = 'main' WHERE id = ?", - ) - .bind(unknown.id.as_str()) - .execute(&mut *connection) - .await - .unwrap(); + sqlx::query("UPDATE automations SET workflow_source_tag = 'v1' WHERE id = ?") + .bind(orphan.id.as_str()) + .execute(&mut *connection) + .await + .unwrap(); drop(connection); assert!(matches!( @@ -384,8 +387,8 @@ async fn corrupt_workflow_source_rows_are_rejected_as_stored_shape_errors() { AutomationStoreError::StoredWorkflowSourceShape { .. } )); assert!(matches!( - store.get(&unknown.id).await.unwrap_err(), - AutomationStoreError::StoredWorkflowSourceKind { .. } + store.get(&orphan.id).await.unwrap_err(), + AutomationStoreError::StoredWorkflowSourceShape { .. } )); } diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index fb65cd0bf..b5b090aa2 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -690,12 +690,7 @@ fn main() { ("Automation", "fabro_automation::Automation", &[]), ( "AutomationGitWorkflowSource", - "fabro_automation::AutomationGitWorkflowSource", - &[], - ), - ( - "AutomationGitWorkflowSourceKind", - "fabro_types::AutomationGitWorkflowSourceKind", + "fabro_types::GitRunTarget", &[], ), ("AutomationRef", "fabro_types::AutomationRef", &[]), diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index 951886ddd..9a012aebd 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -15,7 +15,7 @@ mod generated { } pub mod types { pub use fabro_automation::{ - Automation, AutomationDraft as CreateAutomationRequest, AutomationGitWorkflowSource, + Automation, AutomationDraft as CreateAutomationRequest, AutomationReplace as ReplaceAutomationRequest, AutomationTrigger, }; pub use fabro_environment::Environment; @@ -45,30 +45,31 @@ pub mod types { pub use fabro_types::{ ActivatedSkill, AgentControlState, AgentMcpToolSummary, AgentSkillActivationSource, AgentSkillSummary, AgentToolCategory, AgentToolSource, AgentToolSummary, - AgentToolsAvailableProps, AskFabro, AuthMethod, AutomationGitWorkflowSourceKind, - AutomationRef, BilledTokenCounts, BlobHash, CommandTermination, Conclusion, ContentPart, - CreateVariableRequest, DiffStats, DiffSummary, DirtyStatus, EventEnvelope, ExecOutputTail, - FailureCategory, FailureDetail, FailureSignature, GitContext, GitRunTarget, IdpIdentity, - IntegrationConnectionKind, IntegrationConnectionState, IntegrationConnectionStatus, - IntegrationProvider, IntegrationStatus, InterviewOption, InterviewQuestionRecord, - LlmOutputKind, McpServerDraft as CreateMcpServerRequest, McpServerProjection, - McpServerReplace as ReplaceMcpServerRequest, McpServerStatus, McpServerView as McpServer, - McpTransportView, Message, PairId, PairMessageId, PairMessageRecord, PairMessageRequest, - PairRecord, PairStartRequest, PairStatus, PairTarget, PairTranscriptEntry, - PairTranscriptResponse, ParallelBranchId, ParallelBranchResult, PendingInterviewRecord, - PermissionLevel, Principal, PullRequest, PullRequestCreation, PullRequestCreationId, - PullRequestCreationStatus, PullRequestDetails, PullRequestDetailsStatus, - PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse, - QuestionType, ReasoningOutput, RepositoryRef, ReviewTarget, ReviewTargetKind, Role, Run, - RunApproval, RunApprovalState, RunClientProvenance, RunEvent, RunEventDetailContentKind, - RunEventDetailResponse, RunFailure, RunIntent, RunIntentArgs, RunPairStatusResponse, - RunProjection, RunProvenance, RunRunnableSource, RunSandbox, RunSandboxFailure, - RunSandboxInstance, RunSandboxKind, RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, - RunSize, RunTarget, SandboxDetails, SandboxInfo, SandboxListMeta, SandboxListResponse, - SandboxNetwork, SandboxNetworkPolicy, SandboxNetworkPolicyMode, SandboxProviderKind, - SandboxProviderLookupError, SandboxResources, SandboxService, SandboxServiceListResponse, - SandboxState, SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, SessionDetail, - SessionId, SessionMessage, SessionRecord, SessionStatus, SessionSummary, SessionTurn, + AgentToolsAvailableProps, AskFabro, AuthMethod, AutomationRef, BilledTokenCounts, BlobHash, + CommandTermination, Conclusion, ContentPart, CreateVariableRequest, DiffStats, DiffSummary, + DirtyStatus, EventEnvelope, ExecOutputTail, FailureCategory, FailureDetail, + FailureSignature, GitContext, GitRunTarget, GitRunTarget as AutomationGitWorkflowSource, + IdpIdentity, IntegrationConnectionKind, IntegrationConnectionState, + IntegrationConnectionStatus, IntegrationProvider, IntegrationStatus, InterviewOption, + InterviewQuestionRecord, LlmOutputKind, McpServerDraft as CreateMcpServerRequest, + McpServerProjection, McpServerReplace as ReplaceMcpServerRequest, McpServerStatus, + McpServerView as McpServer, McpTransportView, Message, PairId, PairMessageId, + PairMessageRecord, PairMessageRequest, PairRecord, PairStartRequest, PairStatus, + PairTarget, PairTranscriptEntry, PairTranscriptResponse, ParallelBranchId, + ParallelBranchResult, PendingInterviewRecord, PermissionLevel, Principal, PullRequest, + PullRequestCreation, PullRequestCreationId, PullRequestCreationStatus, PullRequestDetails, + PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, PullRequestLink, + PullRequestMeta, PullRequestResponse, QuestionType, ReasoningOutput, RepositoryRef, + ReviewTarget, ReviewTargetKind, Role, Run, RunApproval, RunApprovalState, + RunClientProvenance, RunEvent, RunEventDetailContentKind, RunEventDetailResponse, + RunFailure, RunIntent, RunIntentArgs, RunPairStatusResponse, RunProjection, RunProvenance, + RunRunnableSource, RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, + RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, RunSize, RunTarget, SandboxDetails, + SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy, + SandboxNetworkPolicyMode, SandboxProviderKind, SandboxProviderLookupError, + SandboxResources, SandboxService, SandboxServiceListResponse, SandboxState, + SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, SessionDetail, SessionId, + SessionMessage, SessionRecord, SessionStatus, SessionSummary, SessionTurn, SkillsProjection, StageCompletion, StageContextWindow, StageContextWindowBreakdownItem, StageContextWindowCategory, StageContextWindowCountMethod, StageContextWindowProjection, StageContextWindowStaleness, StageContextWindowUnavailableReason, diff --git a/lib/foundation/fabro-api/tests/automation_round_trip.rs b/lib/foundation/fabro-api/tests/automation_round_trip.rs index be7c68fbd..6ba104929 100644 --- a/lib/foundation/fabro-api/tests/automation_round_trip.rs +++ b/lib/foundation/fabro-api/tests/automation_round_trip.rs @@ -1,13 +1,12 @@ use fabro_api::types::{ Automation as ApiAutomation, AutomationGitWorkflowSource as ApiAutomationGitWorkflowSource, - AutomationGitWorkflowSourceKind as ApiAutomationGitWorkflowSourceKind, AutomationTrigger as ApiAutomationTrigger, CreateAutomationRequest as ApiCreateAutomationRequest, ReplaceAutomationRequest as ApiReplaceAutomationRequest, }; use fabro_automation::{ - Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationGitWorkflowSourceKind, - AutomationReplace, AutomationTrigger, + Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationReplace, AutomationTrigger, + validate_workflow_source, }; use serde_json::json; @@ -18,7 +17,6 @@ use serde_json::json; const _: fn(ApiAutomation) -> Automation = |value| value; const _: fn(ApiAutomationTrigger) -> AutomationTrigger = |value| value; const _: fn(ApiAutomationGitWorkflowSource) -> AutomationGitWorkflowSource = |value| value; -const _: fn(ApiAutomationGitWorkflowSourceKind) -> AutomationGitWorkflowSourceKind = |value| value; const _: fn(ApiCreateAutomationRequest) -> AutomationDraft = |value| value; const _: fn(ApiReplaceAutomationRequest) -> AutomationReplace = |value| value; @@ -113,10 +111,19 @@ fn replace_automation_request_round_trips_public_json_shape() { #[test] fn automation_workflow_sources_round_trip_each_public_json_shape() { - for (kind, reference) in [ - ("branch", "main"), - ("tag", "release/v1"), - ("commit", "abcdef0123456789abcdef0123456789abcdef01"), + for source in [ + json!({"repo": "fabro-sh/workflows", "branch": "main"}), + json!({ + "repo": "fabro-sh/workflows", + "branch": "main", + "tag": "release/v1" + }), + json!({ + "repo": "fabro-sh/workflows", + "branch": "context-only", + "tag": "release/v1", + "sha": "abcdef0123456789abcdef0123456789abcdef01" + }), ] { let value = json!({ "id": "nightly-deps", @@ -128,11 +135,7 @@ fn automation_workflow_sources_round_trip_each_public_json_shape() { "branch": "main" }, "workflow": "dependency-update", - "workflow_source": { - "repo": "fabro-sh/workflows", - "kind": kind, - "ref": reference - }, + "workflow_source": source, "triggers": [] }); @@ -144,18 +147,18 @@ fn automation_workflow_sources_round_trip_each_public_json_shape() { #[test] fn automation_workflow_source_rejects_unknown_or_incomplete_coordinates() { for source in [ - json!({"repo": "fabro-sh/workflows", "kind": "unknown", "ref": "main"}), - json!({"repo": "fabro-sh/workflows", "kind": "branch"}), - json!({"repo": "fabro-sh/workflows", "kind": "branch", "ref": "main", "extra": true}), + json!({"repo": "fabro-sh/workflows"}), + json!({"branch": "main"}), + json!({"repo": "fabro-sh/workflows", "branch": "main", "extra": true}), ] { assert!(serde_json::from_value::(source).is_err()); } let invalid_commit: ApiAutomationGitWorkflowSource = serde_json::from_value(json!({ "repo": "fabro-sh/workflows", - "kind": "commit", - "ref": "short" + "branch": "main", + "sha": "short" })) .unwrap(); - assert!(invalid_commit.validate().is_err()); + assert!(validate_workflow_source(invalid_commit).is_err()); } diff --git a/lib/foundation/fabro-api/tests/run_summary_round_trip.rs b/lib/foundation/fabro-api/tests/run_summary_round_trip.rs index 7fbc62793..76280e5a4 100644 --- a/lib/foundation/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_summary_round_trip.rs @@ -9,11 +9,10 @@ use fabro_api::types::{ }; use fabro_types::status::{RunStatus, SuccessReason}; use fabro_types::{ - AskFabro, AskFabroUnavailableReason, AutomationGitWorkflowSourceKind, AutomationRef, - DiffSummary, PullRequestLink, RepositoryProvider, RepositoryRef, - ResolvedAutomationGitWorkflowSource, Run, RunApproval, RunApprovalState, RunBillingSummary, - RunId, RunLifecycle, RunLinks, RunOrigin, RunRunnableSource, RunSize, RunTimestamps, RunTiming, - WorkflowRef, fixtures, test_support, + AskFabro, AskFabroUnavailableReason, AutomationRef, DiffSummary, PullRequestLink, + RepositoryProvider, RepositoryRef, ResolvedAutomationGitWorkflowSource, Run, RunApproval, + RunApprovalState, RunBillingSummary, RunId, RunLifecycle, RunLinks, RunOrigin, + RunRunnableSource, RunSize, RunTimestamps, RunTiming, WorkflowRef, fixtures, test_support, }; use serde_json::json; @@ -85,8 +84,9 @@ fn run_summary_json_matches_openapi_shape() { trigger_id: Some("schedule_1".to_string()), workflow_source: Some(Box::new(ResolvedAutomationGitWorkflowSource { repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Commit, - reference: "0123456789abcdef0123456789abcdef01234567".to_string(), + branch: "context-only".to_string(), + tag: Some("v1".to_string()), + sha: Some("0123456789abcdef0123456789abcdef01234567".to_string()), resolved_sha: "0123456789abcdef0123456789abcdef01234567".to_string(), })), }), @@ -164,8 +164,9 @@ fn run_summary_json_matches_openapi_shape() { "trigger_id": "schedule_1", "workflow_source": { "repo": "fabro-sh/workflows", - "kind": "commit", - "ref": "0123456789abcdef0123456789abcdef01234567", + "branch": "context-only", + "tag": "v1", + "sha": "0123456789abcdef0123456789abcdef01234567", "resolved_sha": "0123456789abcdef0123456789abcdef01234567" } }, diff --git a/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql b/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql index c9c736bba..2786455a8 100644 --- a/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql +++ b/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql @@ -4,38 +4,54 @@ ALTER TABLE automations ADD COLUMN workflow_source_repository TEXT OR length(workflow_source_repository) BETWEEN 3 AND 140 ); -ALTER TABLE automations ADD COLUMN workflow_source_kind TEXT +ALTER TABLE automations ADD COLUMN workflow_source_branch TEXT CHECK ( - workflow_source_kind IS NULL - OR workflow_source_kind IN ('branch', 'tag', 'commit') + workflow_source_branch IS NULL + OR length(workflow_source_branch) BETWEEN 1 AND 255 ); -ALTER TABLE automations ADD COLUMN workflow_source_ref TEXT +ALTER TABLE automations ADD COLUMN workflow_source_tag TEXT CHECK ( - workflow_source_ref IS NULL - OR length(workflow_source_ref) BETWEEN 1 AND 255 + workflow_source_tag IS NULL + OR length(workflow_source_tag) BETWEEN 1 AND 255 + ); + +ALTER TABLE automations ADD COLUMN workflow_source_sha TEXT + CHECK ( + workflow_source_sha IS NULL + OR ( + length(workflow_source_sha) = 40 + AND workflow_source_sha NOT GLOB '*[^0-9a-f]*' + ) ); CREATE TRIGGER automation_workflow_source_all_or_none_insert BEFORE INSERT ON automations WHEN (NEW.workflow_source_repository IS NULL) - + (NEW.workflow_source_kind IS NULL) - + (NEW.workflow_source_ref IS NULL) NOT IN (0, 3) + + (NEW.workflow_source_branch IS NULL) NOT IN (0, 2) + OR ( + NEW.workflow_source_repository IS NULL + AND (NEW.workflow_source_tag IS NOT NULL OR NEW.workflow_source_sha IS NOT NULL) + ) BEGIN - SELECT RAISE(ABORT, 'automation workflow source must be entirely null or entirely present'); + SELECT RAISE(ABORT, 'automation workflow source requires repository and branch together'); END; CREATE TRIGGER automation_workflow_source_all_or_none_update BEFORE UPDATE OF workflow_source_repository, - workflow_source_kind, - workflow_source_ref + workflow_source_branch, + workflow_source_tag, + workflow_source_sha ON automations WHEN (NEW.workflow_source_repository IS NULL) - + (NEW.workflow_source_kind IS NULL) - + (NEW.workflow_source_ref IS NULL) NOT IN (0, 3) + + (NEW.workflow_source_branch IS NULL) NOT IN (0, 2) + OR ( + NEW.workflow_source_repository IS NULL + AND (NEW.workflow_source_tag IS NOT NULL OR NEW.workflow_source_sha IS NOT NULL) + ) BEGIN - SELECT RAISE(ABORT, 'automation workflow source must be entirely null or entirely present'); + SELECT RAISE(ABORT, 'automation workflow source requires repository and branch together'); END; diff --git a/lib/foundation/fabro-db/tests/sqlite.rs b/lib/foundation/fabro-db/tests/sqlite.rs index 7694f3494..33f0a697d 100644 --- a/lib/foundation/fabro-db/tests/sqlite.rs +++ b/lib/foundation/fabro-db/tests/sqlite.rs @@ -388,18 +388,26 @@ async fn automations_schema_enforces_aggregate_constraints() -> anyhow::Result<( .await .is_err() ); - for (repository, kind, reference) in [ - (Some("fabro-sh/workflows"), None, None), - (None, Some("branch"), Some("main")), - (Some("fabro-sh/workflows"), Some("unknown"), Some("main")), + for (repository, branch, tag, sha) in [ + (Some("fabro-sh/workflows"), None, None, None), + (None, Some("main"), None, None), + (None, None, Some("v1"), None), + ( + Some("fabro-sh/workflows"), + Some("main"), + None, + Some("short"), + ), ] { let result = sqlx::query( "UPDATE automations SET workflow_source_repository = ?, \ - workflow_source_kind = ?, workflow_source_ref = ? WHERE id = 'valid'", + workflow_source_branch = ?, workflow_source_tag = ?, workflow_source_sha = ? \ + WHERE id = 'valid'", ) .bind(repository) - .bind(kind) - .bind(reference) + .bind(branch) + .bind(tag) + .bind(sha) .execute(database.pool()) .await; assert!( @@ -410,7 +418,8 @@ async fn automations_schema_enforces_aggregate_constraints() -> anyhow::Result<( sqlx::query( "UPDATE automations SET workflow_source_repository = 'fabro-sh/workflows', \ - workflow_source_kind = 'branch', workflow_source_ref = 'main' WHERE id = 'valid'", + workflow_source_branch = 'main', workflow_source_tag = 'v1', \ + workflow_source_sha = '0123456789abcdef0123456789abcdef01234567' WHERE id = 'valid'", ) .execute(database.pool()) .await?; @@ -478,7 +487,8 @@ async fn automation_workflow_sources_migrate_without_rewriting_existing_rows() - let row = sqlx::query( "SELECT id, revision, target_repository, target_branch, target_tag, target_sha, \ - target_workflow, workflow_source_repository, workflow_source_kind, workflow_source_ref \ + target_workflow, workflow_source_repository, workflow_source_branch, \ + workflow_source_tag, workflow_source_sha \ FROM automations WHERE id = 'preserved'", ) .fetch_one(database.pool()) @@ -494,8 +504,9 @@ async fn automation_workflow_sources_migrate_without_rewriting_existing_rows() - row.get::, _>("workflow_source_repository"), None ); - assert_eq!(row.get::, _>("workflow_source_kind"), None); - assert_eq!(row.get::, _>("workflow_source_ref"), None); + assert_eq!(row.get::, _>("workflow_source_branch"), None); + assert_eq!(row.get::, _>("workflow_source_tag"), None); + assert_eq!(row.get::, _>("workflow_source_sha"), None); let trigger_count: i64 = sqlx::query_scalar( "SELECT COUNT(*) FROM automation_triggers WHERE automation_id = 'preserved'", ) @@ -523,10 +534,13 @@ async fn rewind_automation_workflow_source_migration( sqlx::query("DROP TRIGGER automation_workflow_source_all_or_none_insert") .execute(database.pool()) .await?; - sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_ref") + sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_sha") .execute(database.pool()) .await?; - sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_kind") + sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_tag") + .execute(database.pool()) + .await?; + sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_branch") .execute(database.pool()) .await?; sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_repository") diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index 7fa1caefd..2c619e11d 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -114,9 +114,8 @@ pub use pull_request::{ }; pub use reasoning::ReasoningOutput; pub use repository::{ - AutomationGitWorkflowSourceKind, GitHubRepositorySlug, GitHubRepositorySlugError, - RepositoryProvider, RepositoryRef, is_valid_git_branch_name, is_valid_git_tag_name, - normalize_git_commit_sha, + GitHubRepositorySlug, GitHubRepositorySlugError, RepositoryProvider, RepositoryRef, + is_valid_git_branch_name, is_valid_git_tag_name, normalize_git_commit_sha, }; pub use run::{ DirtyStatus, ForkSourceRef, GitContext, RunClientProvenance, RunProvenance, diff --git a/lib/foundation/fabro-types/src/repository.rs b/lib/foundation/fabro-types/src/repository.rs index ba428f3fc..b0e69f3e5 100644 --- a/lib/foundation/fabro-types/src/repository.rs +++ b/lib/foundation/fabro-types/src/repository.rs @@ -5,26 +5,6 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; -#[derive( - Debug, - Clone, - Copy, - PartialEq, - Eq, - Serialize, - Deserialize, - strum::Display, - strum::EnumString, - strum::IntoStaticStr, -)] -#[serde(rename_all = "snake_case")] -#[strum(serialize_all = "snake_case")] -pub enum AutomationGitWorkflowSourceKind { - Branch, - Tag, - Commit, -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RepositoryRef { pub name: String, diff --git a/lib/foundation/fabro-types/src/run_summary.rs b/lib/foundation/fabro-types/src/run_summary.rs index a9e8620e6..001b56e3e 100644 --- a/lib/foundation/fabro-types/src/run_summary.rs +++ b/lib/foundation/fabro-types/src/run_summary.rs @@ -3,9 +3,8 @@ use std::collections::HashMap; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -use crate::repository::AutomationGitWorkflowSourceKind; use crate::{ - DiffSummary, InterviewQuestionRecord, Principal, PullRequestLink, RepositoryRef, + DiffSummary, GitRunTarget, InterviewQuestionRecord, Principal, PullRequestLink, RepositoryRef, RunControlAction, RunId, RunSandbox, RunStatus, RunTiming, }; @@ -120,12 +119,27 @@ impl WorkflowRef { #[serde(deny_unknown_fields)] pub struct ResolvedAutomationGitWorkflowSource { pub repo: String, - pub kind: AutomationGitWorkflowSourceKind, - #[serde(rename = "ref")] - pub reference: String, + pub branch: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tag: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sha: Option, pub resolved_sha: String, } +impl ResolvedAutomationGitWorkflowSource { + #[must_use] + pub fn from_requested(source: GitRunTarget, resolved_sha: String) -> Self { + Self { + repo: source.repo, + branch: source.branch, + tag: source.tag, + sha: source.sha, + resolved_sha, + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct AutomationRef { pub id: String, diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index 1ad85164a..07dabaa3a 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -8,8 +8,8 @@ use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; use fabro_types::{ - AutomationGitWorkflowSourceKind, AutomationRef, EventBody, GitRunTarget, - ResolvedAutomationGitWorkflowSource, RunTarget, TurnId, WorkflowSettings, fixtures, + AutomationRef, EventBody, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunTarget, TurnId, + WorkflowSettings, fixtures, }; fn templated_settings() -> WorkflowSettings { @@ -41,8 +41,9 @@ fn run_created_props_round_trip_templated_settings() { trigger_id: Some("schedule_1".to_string()), workflow_source: Some(Box::new(ResolvedAutomationGitWorkflowSource { repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Tag, - reference: "v1".to_string(), + branch: "main".to_string(), + tag: Some("v1".to_string()), + sha: None, resolved_sha: "0123456789abcdef0123456789abcdef01234567".to_string(), })), }), @@ -85,8 +86,8 @@ fn run_created_props_round_trip_templated_settings() { assert_eq!(json["parent_id"], fixtures::RUN_2.to_string()); assert_eq!(json["automation"]["id"], "nightly"); assert_eq!(json["automation"]["trigger_id"], "schedule_1"); - assert_eq!(json["automation"]["workflow_source"]["kind"], "tag"); - assert_eq!(json["automation"]["workflow_source"]["ref"], "v1"); + assert_eq!(json["automation"]["workflow_source"]["branch"], "main"); + assert_eq!(json["automation"]["workflow_source"]["tag"], "v1"); assert_eq!( json["automation"]["workflow_source"]["resolved_sha"], "0123456789abcdef0123456789abcdef01234567" diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index 409a426ca..fd2088770 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -6,8 +6,8 @@ use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; use fabro_types::{ - AutomationGitWorkflowSourceKind, AutomationRef, GitRunTarget, - ResolvedAutomationGitWorkflowSource, RunTarget, WorkflowSettings, fixtures, + AutomationRef, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunTarget, WorkflowSettings, + fixtures, }; fn templated_settings() -> WorkflowSettings { @@ -37,8 +37,9 @@ fn run_spec_round_trips_templated_settings() { trigger_id: Some("schedule_1".to_string()), workflow_source: Some(Box::new(ResolvedAutomationGitWorkflowSource { repo: "fabro-sh/workflows".to_string(), - kind: AutomationGitWorkflowSourceKind::Branch, - reference: "main".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, resolved_sha: "0123456789abcdef0123456789abcdef01234567".to_string(), })), }), @@ -75,7 +76,7 @@ fn run_spec_round_trips_templated_settings() { assert_eq!(json["fork_source_ref"]["checkpoint_sha"], "def456"); assert_eq!(json["automation"]["id"], "nightly"); assert_eq!(json["automation"]["trigger_id"], "schedule_1"); - assert_eq!(json["automation"]["workflow_source"]["ref"], "main"); + assert_eq!(json["automation"]["workflow_source"]["branch"], "main"); assert_eq!( json["automation"]["workflow_source"]["resolved_sha"], "0123456789abcdef0123456789abcdef01234567" diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index f8582b270..105e4c5fe 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -60,7 +60,6 @@ models/auth-session-user.ts models/auth-session.ts models/auth-sessions-response.ts models/automation-api-trigger.ts -models/automation-git-workflow-source-kind.ts models/automation-git-workflow-source.ts models/automation-list-meta.ts models/automation-list-response.ts diff --git a/lib/packages/fabro-api-client/src/models/automation-git-workflow-source-kind.ts b/lib/packages/fabro-api-client/src/models/automation-git-workflow-source-kind.ts deleted file mode 100644 index 5be4f2aac..000000000 --- a/lib/packages/fabro-api-client/src/models/automation-git-workflow-source-kind.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.2.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - - -/** - * How an automation interprets the workflow source `ref`. - */ - -export const AutomationGitWorkflowSourceKind = { - BRANCH: 'branch', - TAG: 'tag', - COMMIT: 'commit' -} as const; - -export type AutomationGitWorkflowSourceKind = typeof AutomationGitWorkflowSourceKind[keyof typeof AutomationGitWorkflowSourceKind]; diff --git a/lib/packages/fabro-api-client/src/models/automation-git-workflow-source.ts b/lib/packages/fabro-api-client/src/models/automation-git-workflow-source.ts index f7e0a7688..d680f2688 100644 --- a/lib/packages/fabro-api-client/src/models/automation-git-workflow-source.ts +++ b/lib/packages/fabro-api-client/src/models/automation-git-workflow-source.ts @@ -13,21 +13,25 @@ */ -// May contain unused imports in some cases -// @ts-ignore -import type { AutomationGitWorkflowSourceKind } from './automation-git-workflow-source-kind'; /** - * Explicit GitHub coordinate from which an automation acquires workflow bytes. The kind makes `ref` unambiguous; this source is independent of the run target and does not provide a working branch for the run. + * Explicit GitHub coordinate from which an automation acquires workflow bytes. The branch is the fallback selector and audit context. An optional tag overrides the branch, and an optional exact SHA overrides both without requiring branch ancestry. This source is independent of the run target and does not provide its working branch. */ export interface AutomationGitWorkflowSource { /** * GitHub repository slug in `owner/name` form. */ 'repo': string; - 'kind': AutomationGitWorkflowSourceKind; /** - * Bare branch or tag name, or an exact 40-character commit SHA, as selected by `kind`. Prefixes such as `refs/heads/` and `refs/tags/` are not accepted. + * Required bare branch name used when neither tag nor SHA is present. It is retained as context when an override is present and is not an ancestry constraint. */ - 'ref': string; + 'branch': string; + /** + * Optional bare tag name. Without `sha`, this tag is resolved whenever the automation fires. Prefixes such as `refs/tags/` are rejected. + */ + 'tag'?: string; + /** + * Optional exact commit, authoritative over tag and branch. The server lowercase-normalizes it and fetches it directly; it need not be reachable from the named branch. + */ + 'sha'?: string; } diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 8bba11042..a22871b82 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -32,7 +32,6 @@ export * from './auth-sessions-response'; export * from './automation'; export * from './automation-api-trigger'; export * from './automation-git-workflow-source'; -export * from './automation-git-workflow-source-kind'; export * from './automation-list-meta'; export * from './automation-list-response'; export * from './automation-ref'; diff --git a/lib/packages/fabro-api-client/src/models/resolved-automation-git-workflow-source.ts b/lib/packages/fabro-api-client/src/models/resolved-automation-git-workflow-source.ts index 7a2767249..668c3ee20 100644 --- a/lib/packages/fabro-api-client/src/models/resolved-automation-git-workflow-source.ts +++ b/lib/packages/fabro-api-client/src/models/resolved-automation-git-workflow-source.ts @@ -13,23 +13,27 @@ */ -// May contain unused imports in some cases -// @ts-ignore -import type { AutomationGitWorkflowSourceKind } from './automation-git-workflow-source-kind'; /** - * Workflow source coordinate and exact commit captured when an automation run was created. The requested ref remains available for audit context while `resolved_sha` identifies the immutable source revision that supplied the workflow bytes. + * Workflow source coordinate and exact commit captured when an automation run was created. The requested selectors remain available for audit context while `resolved_sha` identifies the immutable source revision that supplied the workflow bytes. */ export interface ResolvedAutomationGitWorkflowSource { /** * GitHub repository slug in `owner/name` form. */ 'repo': string; - 'kind': AutomationGitWorkflowSourceKind; /** - * Branch, tag, or commit requested by the automation. + * Required branch fallback and audit context. */ - 'ref': string; + 'branch': string; + /** + * Optional tag requested by the automation. + */ + 'tag'?: string; + /** + * Optional exact commit requested by the automation. + */ + 'sha'?: string; /** * Exact lowercase Git commit that supplied the workflow bytes. */