diff --git a/Cargo.lock b/Cargo.lock index 2968e767b..1961af9b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2380,6 +2380,7 @@ dependencies = [ "fabro-types", "hex", "serde", + "serde_json", "sha2 0.10.9", "sqlx", "tempfile", diff --git a/apps/fabro-web/app/components/automation-form.test.tsx b/apps/fabro-web/app/components/automation-form.test.tsx new file mode 100644 index 000000000..d2e89406d --- /dev/null +++ b/apps/fabro-web/app/components/automation-form.test.tsx @@ -0,0 +1,98 @@ +import { describe, expect, test } from "bun:test"; + +import { + EMPTY_AUTOMATION_FORM, + automationFormValuesFromRun, + automationToFormValues, + isFormValid, + workflowSourceFromFormValues, +} from "./automation-form"; + +describe("automation workflow source form values", () => { + test("the default and create-from-run forms inherit the target checkout", () => { + expect(workflowSourceFromFormValues(EMPTY_AUTOMATION_FORM)).toBeUndefined(); + + const values = automationFormValuesFromRun({ + title: "Release", + workflow: { name: "Release", graph_name: "release", slug: "release" }, + repository: { + name: "fabro-sh/fabro", + origin_url: "https://github.com/fabro-sh/fabro.git", + }, + sandbox: null, + } as any); + expect(values.usesRemoteWorkflow).toBe(false); + expect(workflowSourceFromFormValues(values)).toBeUndefined(); + }); + + test("branch, tag, and SHA selectors serialize with target precedence", () => { + const base = { + ...EMPTY_AUTOMATION_FORM, + usesRemoteWorkflow: true, + workflowSourceRepository: " fabro-sh/workflows ", + workflowSourceBranch: " main ", + }; + + expect(workflowSourceFromFormValues(base)).toEqual({ + repo: "fabro-sh/workflows", branch: "main", + }); + expect(workflowSourceFromFormValues({ + ...base, + workflowSourceTag: " v1.2.3 ", + })).toEqual({ repo: "fabro-sh/workflows", branch: "main", tag: "v1.2.3" }); + expect(workflowSourceFromFormValues({ + ...base, + workflowSourceTag: " v1.2.3 ", + workflowSourceSha: "ABCDEF0123456789ABCDEF0123456789ABCDEF01", + })).toEqual({ + repo: "fabro-sh/workflows", + branch: "main", + tag: "v1.2.3", + sha: "abcdef0123456789abcdef0123456789abcdef01", + }); + }); + + test("remote workflow fields are required and SHAs need 40 hex characters", () => { + const validBase = { + ...EMPTY_AUTOMATION_FORM, + id: "nightly", + name: "Nightly", + environmentId: "daytona-smoke", + targetRepository: "fabro-sh/app", + targetBranch: "main", + workflow: "release", + usesRemoteWorkflow: true, + workflowSourceRepository: "fabro-sh/workflows", + workflowSourceBranch: "main", + workflowSourceSha: "0123456789abcdef0123456789abcdef01234567", + }; + + expect(isFormValid(validBase)).toBe(true); + expect(isFormValid({ ...validBase, workflowSourceRepository: "" })).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", () => { + const values = automationToFormValues({ + id: "nightly", + revision: "revision", + name: "Nightly", + description: null, + target: { kind: "git", repo: "fabro-sh/fabro", branch: "main" }, + workflow: "release", + workflow_source: { repo: "fabro-sh/fabro", branch: "main" }, + triggers: [], + }); + + expect(values.usesRemoteWorkflow).toBe(true); + expect(values.workflowSourceRepository).toBe("fabro-sh/fabro"); + expect(values.workflowSourceBranch).toBe("main"); + expect(values.workflowSourceTag).toBe(""); + expect(values.workflowSourceSha).toBe(""); + expect(workflowSourceFromFormValues({ + ...values, + usesRemoteWorkflow: false, + })).toBeUndefined(); + }); +}); diff --git a/apps/fabro-web/app/components/automation-form.tsx b/apps/fabro-web/app/components/automation-form.tsx index 2a744d394..bd9d3a8ce 100644 --- a/apps/fabro-web/app/components/automation-form.tsx +++ b/apps/fabro-web/app/components/automation-form.tsx @@ -3,6 +3,7 @@ import { Link } from "react-router"; import { Switch } from "@headlessui/react"; import type { Automation, + AutomationGitWorkflowSource, AutomationTrigger, Environment, Run, @@ -26,29 +27,39 @@ export interface AutomationFormValues { name: string; description: string; environmentId: string; - repository: string; - branch: string; - tag: string; - sha: string; + targetRepository: string; + targetBranch: string; + targetTag: string; + targetSha: string; workflow: string; + usesRemoteWorkflow: boolean; + workflowSourceRepository: string; + workflowSourceBranch: string; + workflowSourceTag: string; + workflowSourceSha: string; manualEnabled: boolean; scheduleEnabled: boolean; cron: string; } export const EMPTY_AUTOMATION_FORM: AutomationFormValues = { - id: "", - name: "", - description: "", + id: "", + name: "", + description: "", environmentId: "", - repository: "", - branch: "main", - tag: "", - sha: "", - workflow: "", - manualEnabled: true, - scheduleEnabled: false, - cron: "0 9 * * 1-5", + targetRepository: "", + targetBranch: "main", + targetTag: "", + targetSha: "", + workflow: "", + usesRemoteWorkflow: false, + workflowSourceRepository: "", + workflowSourceBranch: "main", + workflowSourceTag: "", + workflowSourceSha: "", + manualEnabled: true, + scheduleEnabled: false, + cron: "0 9 * * 1-5", }; const CRON_PRESETS: ReadonlyArray<{ label: string; value: string }> = [ @@ -62,19 +73,25 @@ export function automationToFormValues(automation: Automation): AutomationFormVa const apiTrigger = findApiTrigger(automation); const scheduleTrigger = findScheduleTrigger(automation); const target = gitTarget(automation.target); + const workflowSource = automation.workflow_source; return { - id: automation.id, - name: automation.name, - description: automation.description ?? "", + id: automation.id, + name: automation.name, + description: automation.description ?? "", environmentId: automation.environment_id ?? "", - repository: target?.repo ?? "", - branch: target?.branch ?? EMPTY_AUTOMATION_FORM.branch, - tag: target?.tag ?? "", - sha: target?.sha ?? "", - workflow: automation.workflow, - manualEnabled: apiTrigger?.enabled ?? false, - scheduleEnabled: scheduleTrigger?.enabled ?? false, - cron: scheduleTrigger?.expression ?? "0 9 * * 1-5", + targetRepository: target?.repo ?? "", + targetBranch: target?.branch ?? EMPTY_AUTOMATION_FORM.targetBranch, + targetTag: target?.tag ?? "", + targetSha: target?.sha ?? "", + workflow: automation.workflow, + usesRemoteWorkflow: workflowSource != null, + workflowSourceRepository: workflowSource?.repo ?? "", + 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", }; } @@ -97,7 +114,7 @@ export function automationFormValuesFromRun( name, ); const canonicalTarget = gitTarget(runState?.spec.target); - const repository = canonicalTarget?.repo + const targetRepository = canonicalTarget?.repo ?? githubRepositoryFromSettings(settings) ?? githubRepositoryName(run.repository?.name) ?? githubRepositoryFromOriginUrl(run.repository?.origin_url) @@ -112,16 +129,16 @@ export function automationFormValuesFromRun( : ""; return { ...EMPTY_AUTOMATION_FORM, - id: kebabify(name), + id: kebabify(name), name, environmentId, - repository, - branch: canonicalTarget?.branch + targetRepository, + targetBranch: canonicalTarget?.branch ?? cloneBranch - ?? EMPTY_AUTOMATION_FORM.branch, - tag: canonicalTarget?.tag ?? "", - sha: canonicalTarget?.sha ?? "", - workflow: run.workflow.slug?.trim() || kebabify(workflowName), + ?? EMPTY_AUTOMATION_FORM.targetBranch, + targetTag: canonicalTarget?.tag ?? "", + targetSha: canonicalTarget?.sha ?? "", + workflow: run.workflow.slug?.trim() || kebabify(workflowName), }; } @@ -146,10 +163,11 @@ export function isFormValid(values: AutomationFormValues): boolean { values.id.trim() !== "" && values.name.trim() !== "" && values.environmentId.trim() !== "" && - values.repository.trim() !== "" && - values.branch.trim() !== "" && - isOptionalShaValid(values.sha) && - values.workflow.trim() !== "" + values.targetRepository.trim() !== "" && + values.targetBranch.trim() !== "" && + isOptionalShaValid(values.targetSha) && + values.workflow.trim() !== "" && + isWorkflowSourceValid(values) ); } @@ -165,10 +183,31 @@ function isOptionalShaValid(sha: string): boolean { 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, + repo: values.targetRepository.trim(), + branch: values.targetBranch.trim(), + tag: values.targetTag.trim() || undefined, + sha: values.targetSha.trim().toLowerCase() || undefined, + }; +} + +function isWorkflowSourceValid(values: AutomationFormValues): boolean { + if (!values.usesRemoteWorkflow) return true; + return ( + values.workflowSourceRepository.trim() !== "" && + values.workflowSourceBranch.trim() !== "" && + isOptionalShaValid(values.workflowSourceSha) + ); +} + +export function workflowSourceFromFormValues( + values: AutomationFormValues, +): AutomationGitWorkflowSource | undefined { + if (!values.usesRemoteWorkflow) return undefined; + return { + repo: values.workflowSourceRepository.trim(), + branch: values.workflowSourceBranch.trim(), + tag: values.workflowSourceTag.trim() || undefined, + sha: values.workflowSourceSha.trim().toLowerCase() || undefined, }; } @@ -256,7 +295,8 @@ export function AutomationFormFields({ environmentsError = false, }: AutomationFormFieldsProps) { const slugTouchedRef = useRef(values.id.length > 0); - const shaValid = isOptionalShaValid(values.sha); + const shaValid = isOptionalShaValid(values.targetSha); + const workflowSourceShaValid = isOptionalShaValid(values.workflowSourceSha); const compatibleEnvironments = environments .filter(isCloneBasedEnvironment) .sort((left, right) => left.id.localeCompare(right.id)); @@ -380,14 +420,17 @@ export function AutomationFormFields({ - - Repository} help="GitHub repository in owner/repo form."> + + Repository} + help="GitHub repository whose workspace the run changes, in owner/repo form." + > patch({ repository: e.target.value })} + name="target_repository" + aria-label="Run target repository" + value={values.targetRepository} + onChange={(e) => patch({ targetRepository: e.target.value })} placeholder="acme/orders-api" autoComplete="off" spellCheck={false} @@ -400,10 +443,10 @@ export function AutomationFormFields({ > patch({ branch: e.target.value })} + value={values.targetBranch} + onChange={(e) => patch({ targetBranch: e.target.value })} placeholder="main" autoComplete="off" spellCheck={false} @@ -416,10 +459,10 @@ export function AutomationFormFields({ > patch({ tag: e.target.value })} + value={values.targetTag} + onChange={(e) => patch({ targetTag: e.target.value })} placeholder="v1.2.3" autoComplete="off" spellCheck={false} @@ -436,20 +479,27 @@ export function AutomationFormFields({ > patch({ sha: e.target.value })} + value={values.targetSha} + onChange={(e) => patch({ targetSha: e.target.value })} placeholder="0123456789abcdef0123456789abcdef01234567" autoComplete="off" spellCheck={false} className={`${INPUT_CLASS} font-mono`} /> + + + Workflow slug} - help="Dash-separated identifier matching the workflow directory name (e.g. patch-cves)." + help={ + values.usesRemoteWorkflow + ? "Dash-separated identifier resolved in the remote workflow checkout." + : "Dash-separated identifier resolved in the run target checkout." + } > + + patch({ usesRemoteWorkflow })} + label="Use a remote workflow" + /> + + {values.usesRemoteWorkflow ? ( + <> + Workflow repository} + help="GitHub owner/repo containing the workflow files." + > + patch({ workflowSourceRepository: e.target.value })} + placeholder="acme/automation-workflows" + autoComplete="off" + spellCheck={false} + className={`${INPUT_CLASS} font-mono`} + /> + + Branch} + help="Fallback revision and audit context. An exact SHA does not need to be reachable from this branch." + > + 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`} + /> + + + ) : null} diff --git a/apps/fabro-web/app/lib/automation.ts b/apps/fabro-web/app/lib/automation.ts index a14ceecb1..c3ee46a14 100644 --- a/apps/fabro-web/app/lib/automation.ts +++ b/apps/fabro-web/app/lib/automation.ts @@ -1,4 +1,9 @@ -import type { Automation, AutomationTrigger, RunTarget } from "@qltysh/fabro-api-client"; +import type { + Automation, + AutomationGitWorkflowSource, + AutomationTrigger, + RunTarget, +} from "@qltysh/fabro-api-client"; export type GitRunTarget = Extract; @@ -27,3 +32,16 @@ export function findScheduleTrigger( export function hasEnabledApiTrigger(automation: Automation): boolean { return findApiTrigger(automation)?.enabled === true; } + +export function workflowSourceSummary(source: AutomationGitWorkflowSource): string { + 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"; + +/** Where an automation's workflow files come from, for display. */ +export function workflowSourceLabel(source: AutomationGitWorkflowSource | undefined): string { + return source ? workflowSourceSummary(source) : RUN_TARGET_CHECKOUT_LABEL; +} diff --git a/apps/fabro-web/app/routes/automation-detail.tsx b/apps/fabro-web/app/routes/automation-detail.tsx index c8cdcf12e..289b2f632 100644 --- a/apps/fabro-web/app/routes/automation-detail.tsx +++ b/apps/fabro-web/app/routes/automation-detail.tsx @@ -24,6 +24,7 @@ import { findApiTrigger, findScheduleTrigger, gitTarget, + workflowSourceLabel, } from "../lib/automation"; import { useAutomation, useAutomationRuns } from "../lib/queries"; import { queryKeys } from "../lib/query-keys"; @@ -146,7 +147,7 @@ function AutomationHeader({ automation }: { automation: Automation }) {
- {target?.repo ?? UNSUPPORTED_TARGET_LABEL} + Run target · {target?.repo ?? UNSUPPORTED_TARGET_LABEL} {target ? ( {" · "}{target.branch} @@ -155,7 +156,9 @@ function AutomationHeader({ automation }: { automation: Automation }) { ) : null} - {automation.workflow} + + Workflow · {automation.workflow} · {workflowSourceLabel(automation.workflow_source)} + {automation.environment_id ?? ( Environment required diff --git a/apps/fabro-web/app/routes/automations-edit.tsx b/apps/fabro-web/app/routes/automations-edit.tsx index 939c60c6f..e182b8275 100644 --- a/apps/fabro-web/app/routes/automations-edit.tsx +++ b/apps/fabro-web/app/routes/automations-edit.tsx @@ -13,6 +13,7 @@ import { isFormValid, targetFromFormValues, triggersFromFormValues, + workflowSourceFromFormValues, type AutomationFormValues, } from "../components/automation-form"; import { Panel, PanelSkeleton } from "../components/settings-panel"; @@ -109,6 +110,7 @@ function EditAutomationForm({ environment_id: values.environmentId.trim(), target: targetFromFormValues(values), workflow: values.workflow.trim(), + workflow_source: workflowSourceFromFormValues(values), triggers: triggersFromFormValues(values), }), ); diff --git a/apps/fabro-web/app/routes/automations-new.test.tsx b/apps/fabro-web/app/routes/automations-new.test.tsx index 4ca2ec5f9..6e54e4e59 100644 --- a/apps/fabro-web/app/routes/automations-new.test.tsx +++ b/apps/fabro-web/app/routes/automations-new.test.tsx @@ -302,7 +302,7 @@ describe("AutomationsNew", () => { expect(fieldValue(renderer, "Automation name")).toBe(""); expect(fieldValue(renderer, "Automation slug")).toBe(""); - expect(fieldValue(renderer, "Repository")).toBe(""); + expect(fieldValue(renderer, "Run target repository")).toBe(""); expect(fieldValue(renderer, "Working branch")).toBe("main"); expect(fieldValue(renderer, "Tag")).toBe(""); expect(fieldValue(renderer, "Exact commit SHA")).toBe(""); @@ -310,6 +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 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 () => { @@ -340,7 +342,7 @@ describe("AutomationsNew", () => { test("creation sends the selected environment id", async () => { const { renderer } = await renderAutomationsNew("/automations/new"); changeField(renderer, "Automation name", "Nightly"); - changeField(renderer, "Repository", "fabro-sh/fabro"); + changeField(renderer, "Run target repository", "fabro-sh/fabro"); changeField(renderer, "Workflow slug", "hello"); changeField(renderer, "Automation environment", "daytona-smoke"); @@ -372,7 +374,7 @@ describe("AutomationsNew", () => { expect(fieldValue(renderer, "Automation name")).toBe("Fix failing tests"); expect(fieldValue(renderer, "Automation slug")).toBe("fix-failing-tests"); - expect(fieldValue(renderer, "Repository")).toBe("qltysh/fabro"); + expect(fieldValue(renderer, "Run target repository")).toBe("qltysh/fabro"); expect(fieldValue(renderer, "Working branch")).toBe("feature/from-run"); expect(fieldValue(renderer, "Tag")).toBe(""); expect(fieldValue(renderer, "Exact commit SHA")).toBe(""); @@ -380,6 +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 remote workflow")).toBe(false); expect( renderer.root.findAllByProps({ "aria-label": "Cron expression" }), ).toHaveLength(0); @@ -405,7 +408,7 @@ describe("AutomationsNew", () => { const { renderer } = await renderAutomationsNew("/automations/new?from_run=run_1"); - expect(fieldValue(renderer, "Repository")).toBe("canonical/repo"); + expect(fieldValue(renderer, "Run target repository")).toBe("canonical/repo"); expect(fieldValue(renderer, "Working branch")).toBe("release"); expect(fieldValue(renderer, "Tag")).toBe("v2.0.0"); expect(fieldValue(renderer, "Exact commit SHA")).toBe( @@ -436,8 +439,41 @@ describe("AutomationsNew", () => { expect(textFromNode(renderer.toJSON())).toContain("could not be loaded"); expect(textFromNode(renderer.toJSON())).toContain("fill it out manually"); expect(fieldValue(renderer, "Automation name")).toBe(""); - expect(fieldValue(renderer, "Repository")).toBe(""); + expect(fieldValue(renderer, "Run target repository")).toBe(""); expect(fieldValue(renderer, "Working branch")).toBe("main"); expect(fieldValue(renderer, "Workflow slug")).toBe(""); }); + + test("submits a canonical explicit workflow source", async () => { + const { renderer } = await renderAutomationsNew("/automations/new"); + changeField(renderer, "Automation name", "Nightly"); + changeField(renderer, "Run target repository", "fabro-sh/app"); + changeField(renderer, "Automation environment", "daytona-smoke"); + changeField(renderer, "Workflow slug", "release"); + act(() => { + 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() {} }); + }); + + expect(createAutomationMock).toHaveBeenCalledTimes(1); + expect(createAutomationMock.mock.calls[0]?.[0]).toMatchObject({ + workflow_source: { + repo: "fabro-sh/workflows", + branch: "release", + tag: "v2.0.0", + sha: "abcdef0123456789abcdef0123456789abcdef01", + }, + }); + }); }); diff --git a/apps/fabro-web/app/routes/automations-new.tsx b/apps/fabro-web/app/routes/automations-new.tsx index 82a5c35b3..ecc1b465c 100644 --- a/apps/fabro-web/app/routes/automations-new.tsx +++ b/apps/fabro-web/app/routes/automations-new.tsx @@ -14,6 +14,7 @@ import { isFormValid, targetFromFormValues, triggersFromFormValues, + workflowSourceFromFormValues, type AutomationFormValues, } from "../components/automation-form"; import { @@ -137,6 +138,7 @@ function AutomationCreateForm({ environment_id: values.environmentId.trim(), target: targetFromFormValues(values), workflow: values.workflow.trim(), + workflow_source: workflowSourceFromFormValues(values), triggers: triggersFromFormValues(values), }), ); diff --git a/apps/fabro-web/app/routes/automations.tsx b/apps/fabro-web/app/routes/automations.tsx index a60ab90c9..afd426e9d 100644 --- a/apps/fabro-web/app/routes/automations.tsx +++ b/apps/fabro-web/app/routes/automations.tsx @@ -19,10 +19,12 @@ import type { Automation, AutomationListResponse } from "@qltysh/fabro-api-clien import { Link, useNavigate } from "react-router"; import { ApiError, apiData, automationsApi } from "../lib/api-client"; import { + RUN_TARGET_CHECKOUT_LABEL, UNSUPPORTED_TARGET_LABEL, findScheduleTrigger, gitTarget, hasEnabledApiTrigger, + workflowSourceSummary, } from "../lib/automation"; import { useAutomations } from "../lib/queries"; import { queryKeys } from "../lib/query-keys"; @@ -55,6 +57,7 @@ interface AutomationRow { workflow: string; repository: string; environmentId: string | null; + workflowSource?: string; schedule?: string; apiEnabled: boolean; icon: ComponentType<{ className?: string }>; @@ -96,6 +99,9 @@ function mapAutomations(result: AutomationListResponse | undefined): AutomationR workflow: a.workflow, repository: target?.repo ?? UNSUPPORTED_TARGET_LABEL, environmentId: a.environment_id, + workflowSource: a.workflow_source + ? workflowSourceSummary(a.workflow_source) + : undefined, schedule: findScheduleTrigger(a)?.expression, apiEnabled: hasEnabledApiTrigger(a), icon: slugIconMap[a.workflow] ?? CodeBracketIcon, @@ -149,11 +155,14 @@ function AutomationCard({ )}

- {automation.repository} + Run target · {automation.repository} {" · "}{automation.environmentId ?? "environment required"}

+

+ Workflow source · {automation.workflowSource ?? RUN_TARGET_CHECKOUT_LABEL} +

@@ -291,7 +300,8 @@ export default function Automations() { (triggerFilter === "manual" && a.schedule == null)) && (a.name.toLowerCase().includes(lowerQuery) || a.workflow.toLowerCase().includes(lowerQuery) || - a.repository.toLowerCase().includes(lowerQuery)), + a.repository.toLowerCase().includes(lowerQuery) || + a.workflowSource?.toLowerCase().includes(lowerQuery)), ); async function confirmDelete() { diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 766f02909..5c294ed86 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -6728,6 +6728,84 @@ components: # ── Automations ────────────────────────────────────────────────────── + AutomationGitWorkflowSource: + description: >- + 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. + type: object + additionalProperties: false + required: + - repo + - branch + properties: + repo: + type: string + minLength: 3 + maxLength: 140 + pattern: "^[A-Za-z0-9][A-Za-z0-9-]*/[A-Za-z0-9._-]+$" + description: GitHub repository slug in `owner/name` form. + example: acme/workflows + branch: + type: string + minLength: 1 + maxLength: 255 + pattern: "^[A-Za-z0-9/._-]+$" + description: >- + 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 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 + - branch + - resolved_sha + properties: + repo: + type: string + description: GitHub repository slug in `owner/name` form. + branch: + type: string + 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}$" + description: Exact lowercase Git commit that supplied the workflow bytes. + Automation: description: Public automation definition. type: object @@ -6772,8 +6850,13 @@ components: $ref: "#/components/schemas/RunTarget" workflow: type: string - description: Workflow slug or path resolved in the selected repository checkout. + description: >- + Workflow slug or path resolved in the run-target checkout when + `workflow_source` is omitted, or in the explicit workflow-source + checkout when present. example: dependency-update + workflow_source: + $ref: "#/components/schemas/AutomationGitWorkflowSource" triggers: type: array items: @@ -6867,8 +6950,13 @@ components: $ref: "#/components/schemas/RunTarget" workflow: type: string - description: Workflow slug or path resolved in the selected repository checkout. + description: >- + Workflow slug or path resolved in the run-target checkout when + `workflow_source` is omitted, or in the explicit workflow-source + checkout when present. example: dependency-update + workflow_source: + $ref: "#/components/schemas/AutomationGitWorkflowSource" triggers: type: array items: @@ -6899,8 +6987,13 @@ components: $ref: "#/components/schemas/RunTarget" workflow: type: string - description: Workflow slug or path resolved in the selected repository checkout. + description: >- + Workflow slug or path resolved in the run-target checkout when + `workflow_source` is omitted, or in the explicit workflow-source + checkout when present. example: dependency-update + workflow_source: + $ref: "#/components/schemas/AutomationGitWorkflowSource" triggers: type: array items: @@ -12090,6 +12183,11 @@ components: type: ["string", "null"] trigger_id: type: ["string", "null"] + workflow_source: + description: Resolved workflow source for automation runs that declare one. + oneOf: + - $ref: "#/components/schemas/ResolvedAutomationGitWorkflowSource" + - type: "null" RunOrigin: type: object diff --git a/docs/public/execution/automations.mdx b/docs/public/execution/automations.mdx index be046a35c..a508c9fa6 100644 --- a/docs/public/execution/automations.mdx +++ b/docs/public/execution/automations.mdx @@ -3,7 +3,7 @@ title: "Automations" description: "Named, repeatable run configurations with API and schedule triggers" --- -An **automation** is a saved run configuration — a Git repository, working branch, optional tag or exact commit, workflow, and server-managed environment — plus the triggers that may start it. When a trigger fires, Fabro packages the selected workflow as an immutable workflow version and admits it through the same `RunIntent` pipeline as `POST /api/v1/runs`. Automation runs therefore get the same lifecycle, events, and observability as manually created runs. Each run records the automation and trigger that created it. +An **automation** is a saved run configuration — a Git run target, a workflow, a server-managed environment, and the triggers that may start it. By default, Fabro loads the workflow from the run-target checkout. An automation can instead name an independent GitHub repository and branch, tag, or exact commit for its workflow files. When a trigger fires, Fabro packages the selected workflow as an immutable workflow version and admits it through the same `RunIntent` pipeline as `POST /api/v1/runs`. Automation runs therefore get the same lifecycle, events, and observability as manually created runs. Each run records the automation and trigger that created it. ## Defining automations @@ -45,6 +45,39 @@ 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 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, enable a remote workflow and provide the same branch-plus-overrides coordinate used by Git run targets: + +```json title="Create automation with a remote workflow" +{ + "id": "nightly-release", + "name": "Nightly release", + "environment_id": "default", + "target": { + "kind": "git", + "repo": "acme/orders-api", + "branch": "main" + }, + "workflow": "release", + "workflow_source": { + "repo": "acme/automation-workflows", + "branch": "main" + }, + "triggers": [ + { "type": "api", "id": "manual", "enabled": true } + ] +} +``` + +`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. + +If target or source authentication, checkout, workflow discovery, packaging, or workflow-version storage fails, Fabro creates no run and sends no start request. + ### Upgrading legacy targets When upgrading from file-backed automation storage, startup imports every valid `automations/*.toml` file next to the active `settings.toml`. Existing SQLite definitions win on ID conflicts. After a successful import, Fabro renames the directory to a timestamped backup such as `automations.imported-20260711T180000000000Z.bak`. Invalid TOML or an invalid target leaves the original directory untouched for operator repair. @@ -86,7 +119,7 @@ The same conversion runs transactionally for automations already in SQLite. An u Automations created before environment selection was introduced are backfilled conservatively. Fabro selects a compatible environment named `default` when one exists, or the sole Docker or Daytona environment when there is exactly one. With no compatible environment or multiple ambiguous choices, the automation remains incomplete until an operator selects one in the web UI. An incomplete automation cannot run. -When a trigger fires, Fabro prepares the repository at the selected branch, tag, or exact commit, packages the workflow, and creates and starts the run. The created run records the exact checked-out commit in its canonical target, so later inspection and automation creation preserve the revision that actually ran. Repositories are cached server-side as bare clones, so repeat fires fetch only what changed. +When a trigger fires, Fabro resolves the run target and selected workflow checkout, packages the workflow, and creates and starts the run. The created run records the exact checked-out target commit in its canonical target, so later inspection and automation creation preserve the target revision that actually ran. Repositories are cached server-side as bare clones, so repeat fires fetch only what changed. ## Triggers @@ -114,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 repository and workflow. 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 4b53797e7..80d8cb917 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -1,33 +1,36 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; use async_trait::async_trait; -use fabro_automation::AutomationId; +use fabro_automation::{AutomationGitWorkflowSource, AutomationId}; use fabro_manifest::WorkflowVersionCollectError; use fabro_types::{ - GitHubRepositorySlug, GitRunTarget, RunId, RunIntent, RunIntentArgs, RunTarget, - TargetValidationError, WorkflowVersionId, + GitCoordinateValidationError, GitHubRepositorySlug, GitRunTarget, + ResolvedAutomationGitWorkflowSource, RunId, RunIntent, RunIntentArgs, RunTarget, + WorkflowVersionId, }; use fabro_workflow_version::{WorkflowVersionStore, WorkflowVersionStoreError}; use tokio::{fs, task}; use crate::git_checkout::{ - GitCheckoutError, GitRepoCache, WorktreePrepareInput, resolve_git_auth_config, + self, GitAuthConfig, GitCheckoutError, GitCheckoutSelector, GitRepoCache, WorktreePrepareInput, }; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct AutomationRunMaterializeInput { - pub automation_id: AutomationId, - pub target: GitRunTarget, - pub workflow: String, - pub run_id: RunId, - pub temp_root: PathBuf, + pub automation_id: AutomationId, + pub target: GitRunTarget, + pub workflow_source: Option, + pub workflow: String, + pub run_id: RunId, + pub temp_root: PathBuf, } #[derive(Debug, Clone)] pub(crate) struct AutomationRunMaterialized { pub workflow_version_id: WorkflowVersionId, pub target: GitRunTarget, + pub workflow_source: Option>, } impl AutomationRunMaterialized { @@ -51,11 +54,23 @@ pub(crate) enum RunMaterializeError { #[error("invalid automation Git target")] InvalidTarget { #[source] - source: TargetValidationError, + source: GitCoordinateValidationError, }, - #[error("failed to prepare automation checkout")] + #[error("invalid automation workflow source")] + InvalidWorkflowSource { + #[source] + source: GitCoordinateValidationError, + }, + #[error("failed to resolve automation {role} credentials")] + Credentials { + role: CheckoutRole, + #[source] + source: anyhow::Error, + }, + #[error("failed to prepare automation {role} checkout")] Checkout { - #[from] + role: CheckoutRole, + #[source] source: GitCheckoutError, }, #[error("failed to prepare automation temporary directory {path}")] @@ -84,13 +99,22 @@ pub(crate) enum RunMaterializeError { #[source] source: WorkflowVersionStoreError, }, - #[error("failed to load GitHub credentials")] - Credentials { + #[error("failed to load server GitHub credentials")] + LoadCredentials { #[source] source: anyhow::Error, }, } +/// Which repository a checkout serves; only the error message differs. +#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display)] +pub(crate) enum CheckoutRole { + #[strum(serialize = "target")] + Target, + #[strum(serialize = "workflow-source")] + WorkflowSource, +} + #[async_trait] pub(crate) trait AutomationRunMaterializer: Send + Sync { async fn materialize( @@ -101,11 +125,44 @@ pub(crate) trait AutomationRunMaterializer: Send + Sync { #[derive(Clone)] pub(crate) struct ProductionAutomationRunMaterializer { - github_credentials: Option, - github_api_base_url: String, - http_client: Option, - repo_cache: Arc, - version_store: WorkflowVersionStore, + remote_resolver: Arc, + repo_cache: Arc, + version_store: WorkflowVersionStore, +} + +/// Where to fetch a repository from and how to authenticate. +#[derive(Clone)] +struct GitRemote { + clone_url: String, + auth: Option, +} + +#[async_trait] +trait AutomationGitRemoteResolver: Send + Sync { + async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result; +} + +struct ServerGitHubRemoteResolver { + credentials: Option, + api_base_url: String, + http_client: Option, +} + +#[async_trait] +impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver { + async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result { + let auth = git_checkout::resolve_git_read_auth_config( + self.credentials.as_ref(), + repo, + &self.api_base_url, + self.http_client.clone(), + ) + .await?; + Ok(GitRemote { + clone_url: git_checkout::github_clone_url(repo), + auth, + }) + } } impl ProductionAutomationRunMaterializer { @@ -117,13 +174,54 @@ impl ProductionAutomationRunMaterializer { version_store: WorkflowVersionStore, ) -> Self { Self { - github_credentials, - github_api_base_url, - http_client, + remote_resolver: Arc::new(ServerGitHubRemoteResolver { + credentials: github_credentials, + api_base_url: github_api_base_url, + http_client, + }), repo_cache, version_store, } } + + #[cfg(test)] + fn with_remote_resolver(mut self, resolver: Arc) -> Self { + self.remote_resolver = resolver; + self + } + + async fn resolve_remote( + &self, + role: CheckoutRole, + repo: &GitHubRepositorySlug, + ) -> Result { + self.remote_resolver + .resolve(repo) + .await + .map_err(|source| RunMaterializeError::Credentials { role, source }) + } + + async fn prepare_checkout( + &self, + role: CheckoutRole, + repo: &GitHubRepositorySlug, + remote: &GitRemote, + selector: GitCheckoutSelector<'_>, + worktree_dir: &Path, + ) -> Result { + self.repo_cache + .prepare_worktree( + WorktreePrepareInput { + repo, + selector, + auth: remote.auth.as_ref(), + worktree_dir, + }, + &remote.clone_url, + ) + .await + .map_err(|source| RunMaterializeError::Checkout { role, source }) + } } #[async_trait] @@ -132,11 +230,25 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { &self, input: AutomationRunMaterializeInput, ) -> Result { - let repo = GitHubRepositorySlug::try_new(&input.target.repo).ok_or( - RunMaterializeError::InvalidTarget { - source: TargetValidationError::Repository, - }, - )?; + let validated_target = input + .target + .validate() + .map_err(|source| RunMaterializeError::InvalidTarget { source })?; + let target_repo = validated_target.repository().clone(); + let mut exact_target = validated_target.into_target(); + let workflow_source = input + .workflow_source + .map(GitRunTarget::validate) + .transpose() + .map_err(|source| RunMaterializeError::InvalidWorkflowSource { source })?; + // A workflow source naming the target's exact coordinate shares its + // checkout; anything else needs a second worktree. + let separate_source = workflow_source.as_ref().filter(|source| { + source.repository() != &target_repo + || GitCheckoutSelector::from(source.target()) + != GitCheckoutSelector::from(&exact_target) + }); + fs::create_dir_all(&input.temp_root) .await .map_err(|source| RunMaterializeError::TempDirectory { @@ -154,32 +266,53 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { path: input.temp_root.clone(), source, })?; - let checkout_dir = temp_dir.path().join("repo"); - let auth = resolve_git_auth_config( - self.github_credentials.as_ref(), - &repo, - &self.github_api_base_url, - self.http_client.clone(), - ) - .await - .map_err(|source| RunMaterializeError::Credentials { source })?; - - let checked_out_sha = self - .repo_cache - .prepare_worktree(WorktreePrepareInput { - repo: &repo, - target: &input.target, - auth: auth.as_ref(), - worktree_dir: &checkout_dir, - }) + let target_checkout_dir = temp_dir.path().join("target"); + let target_remote = self + .resolve_remote(CheckoutRole::Target, &target_repo) .await?; - - let mut exact_target = input.target; + let checked_out_sha = self + .prepare_checkout( + CheckoutRole::Target, + &target_repo, + &target_remote, + GitCheckoutSelector::from(&exact_target), + &target_checkout_dir, + ) + .await?; + let (workflow_checkout_dir, workflow_checkout_sha) = match separate_source { + None => (target_checkout_dir, checked_out_sha.clone()), + Some(source) => { + let repo = source.repository(); + let remote = if repo == &target_repo { + target_remote + } else { + self.resolve_remote(CheckoutRole::WorkflowSource, repo) + .await? + }; + let source_checkout_dir = temp_dir.path().join("workflow-source"); + let source_sha = self + .prepare_checkout( + CheckoutRole::WorkflowSource, + repo, + &remote, + GitCheckoutSelector::from(source.target()), + &source_checkout_dir, + ) + .await?; + (source_checkout_dir, source_sha) + } + }; exact_target.sha = Some(checked_out_sha); + let resolved_workflow_source = workflow_source.map(|source| { + Box::new(ResolvedAutomationGitWorkflowSource::from_requested( + source.into_target(), + workflow_checkout_sha, + )) + }); let workflow = PathBuf::from(input.workflow); let closure = task::spawn_blocking(move || { - fabro_manifest::collect_workflow_versions(&workflow, &checkout_dir) + fabro_manifest::collect_workflow_versions(&workflow, &workflow_checkout_dir) .map_err(package_error) }) .await @@ -201,6 +334,7 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { Ok(AutomationRunMaterialized { workflow_version_id: closure.root_id(), target: exact_target, + workflow_source: resolved_workflow_source, }) } } @@ -223,7 +357,26 @@ pub struct TestAutomationRunMaterializer { #[cfg(any(test, feature = "test-support"))] struct TestAutomationRunMaterializerState { captured_inputs: Vec, - response: Result, TargetValidationError>, + response: Result, TestMaterializeFailure>, +} + +#[cfg(any(test, feature = "test-support"))] +#[derive(Clone)] +enum TestMaterializeFailure { + InvalidTarget(GitCoordinateValidationError), + InvalidWorkflowSource(GitCoordinateValidationError), +} + +#[cfg(any(test, feature = "test-support"))] +impl From for RunMaterializeError { + fn from(failure: TestMaterializeFailure) -> Self { + match failure { + TestMaterializeFailure::InvalidTarget(source) => Self::InvalidTarget { source }, + TestMaterializeFailure::InvalidWorkflowSource(source) => { + Self::InvalidWorkflowSource { source } + } + } + } } #[cfg(any(test, feature = "test-support"))] @@ -253,10 +406,18 @@ impl TestAutomationRunMaterializer { } pub fn fail_invalid_target() -> Self { - Self::new(Err(TargetValidationError::Repository)) + Self::new(Err(TestMaterializeFailure::InvalidTarget( + GitCoordinateValidationError::Repository, + ))) } - fn new(response: Result, TargetValidationError>) -> Self { + pub fn fail_invalid_workflow_source() -> Self { + Self::new(Err(TestMaterializeFailure::InvalidWorkflowSource( + GitCoordinateValidationError::Branch, + ))) + } + + fn new(response: Result, TestMaterializeFailure>) -> Self { Self { inner: std::sync::Arc::new(std::sync::Mutex::new( TestAutomationRunMaterializerState { @@ -276,6 +437,16 @@ impl TestAutomationRunMaterializer { .clone() } + pub fn captured_workflow_sources(&self) -> Vec> { + self.inner + .lock() + .expect("test automation materializer lock poisoned") + .captured_inputs + .iter() + .map(|input| input.workflow_source.clone()) + .collect() + } + pub(crate) fn into_materializer( mut self, version_store: WorkflowVersionStore, @@ -312,6 +483,12 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer { &self, input: AutomationRunMaterializeInput, ) -> Result { + let workflow_source = input.workflow_source.as_ref().map(|source| { + Box::new(ResolvedAutomationGitWorkflowSource::from_requested( + source.clone(), + "ffffffffffffffffffffffffffffffffffffffff".to_string(), + )) + }); let response = { let mut guard = self .inner @@ -320,8 +497,7 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer { guard.captured_inputs.push(input); guard.response.clone() }; - let materialized = - *response.map_err(|source| RunMaterializeError::InvalidTarget { source })?; + let materialized = *response.map_err(RunMaterializeError::from)?; let store = self .version_store .as_ref() @@ -341,6 +517,7 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer { Ok(AutomationRunMaterialized { workflow_version_id, target: materialized.target, + workflow_source, }) } } @@ -352,8 +529,10 @@ mod tests { reason = "Materializer unit tests write small temporary workflow fixtures synchronously." )] + use std::collections::HashMap; use std::fs; use std::path::Path; + use std::sync::Mutex; use std::time::Duration; use object_store::memory::InMemory; @@ -361,6 +540,289 @@ mod tests { use super::*; + const FAKE_TOKEN: &str = "ghu_automation_materializer_secret"; + + struct RecordingCredentialResolver { + repositories: Mutex>, + fail_for: Option, + } + + impl RecordingCredentialResolver { + fn succeeds() -> Self { + Self { + repositories: Mutex::new(Vec::new()), + fail_for: None, + } + } + + fn fails_for(repo: GitHubRepositorySlug) -> Self { + Self { + repositories: Mutex::new(Vec::new()), + fail_for: Some(repo), + } + } + + fn repositories(&self) -> Vec { + self.repositories + .lock() + .expect("credential recorder lock poisoned") + .iter() + .map(ToString::to_string) + .collect() + } + } + + /// Serves local bare fixtures as clone URLs while recording which + /// repositories had credentials resolved. + struct FixtureRemoteResolver { + credentials: Arc, + clone_urls: HashMap, + } + + #[async_trait] + impl AutomationGitRemoteResolver for FixtureRemoteResolver { + async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result { + let recorder = &self.credentials; + recorder + .repositories + .lock() + .expect("credential recorder lock poisoned") + .push(repo.clone()); + if recorder.fail_for.as_ref() == Some(repo) { + anyhow::bail!("test repository access denied") + } + let clone_url = self + .clone_urls + .get(repo) + .unwrap_or_else(|| panic!("no fixture clone URL for {repo}")) + .clone(); + Ok(GitRemote { + clone_url, + auth: Some(GitAuthConfig::from_parts("x-access-token", FAKE_TOKEN)), + }) + } + } + + struct GitFixture { + bare: PathBuf, + work: PathBuf, + initial_sha: String, + } + + fn run_git(args: &[&str]) { + let status = std::process::Command::new("git") + .args(args) + .status() + .expect("git command should start"); + assert!(status.success(), "git command failed: {args:?}"); + } + + fn git_output(args: &[&str]) -> String { + let output = std::process::Command::new("git") + .args(args) + .output() + .expect("git command should start"); + assert!(output.status.success(), "git command failed: {args:?}"); + String::from_utf8(output.stdout) + .expect("git output should be UTF-8") + .trim() + .to_string() + } + + fn write_workflow(work: &Path, marker: &str) { + let workflow_dir = work.join(".fabro/workflows/demo"); + fs::create_dir_all(&workflow_dir).unwrap(); + fs::write(work.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + fs::write( + workflow_dir.join("workflow.fabro"), + format!( + "digraph Demo {{ graph [goal=\"{marker}\"] start [shape=Mdiamond] exit [shape=Msquare] start -> exit }}\n" + ), + ) + .unwrap(); + } + + fn seed_repository(root: &Path, name: &str, marker: &str) -> GitFixture { + let bare = root.join(format!("{name}.git")); + let work = root.join(format!("{name}-work")); + run_git(&[ + "init", + "--bare", + "--initial-branch=main", + bare.to_str().unwrap(), + ]); + run_git(&["init", "--initial-branch=main", work.to_str().unwrap()]); + for (key, value) in [ + ("user.email", "test@fabro.sh"), + ("user.name", "Fabro Test"), + ("commit.gpgsign", "false"), + ] { + run_git(&["-C", work.to_str().unwrap(), "config", key, value]); + } + write_workflow(&work, marker); + run_git(&["-C", work.to_str().unwrap(), "add", "."]); + run_git(&[ + "-C", + work.to_str().unwrap(), + "commit", + "-m", + "initial workflow", + ]); + run_git(&[ + "-C", + work.to_str().unwrap(), + "tag", + "-a", + "annotated-v1", + "-m", + "annotated v1", + ]); + run_git(&["-C", work.to_str().unwrap(), "tag", "lightweight-v1"]); + run_git(&[ + "-C", + work.to_str().unwrap(), + "push", + "--tags", + bare.to_str().unwrap(), + "main", + ]); + let initial_sha = git_output(&["-C", work.to_str().unwrap(), "rev-parse", "HEAD"]); + GitFixture { + bare, + work, + initial_sha, + } + } + + fn advance_repository(fixture: &GitFixture, marker: &str) -> String { + write_workflow(&fixture.work, marker); + run_git(&["-C", fixture.work.to_str().unwrap(), "add", "."]); + run_git(&[ + "-C", + fixture.work.to_str().unwrap(), + "commit", + "-m", + "advance workflow", + ]); + run_git(&[ + "-C", + fixture.work.to_str().unwrap(), + "push", + fixture.bare.to_str().unwrap(), + "main", + ]); + git_output(&["-C", fixture.work.to_str().unwrap(), "rev-parse", "HEAD"]) + } + + fn repository(value: &str) -> GitHubRepositorySlug { + GitHubRepositorySlug::try_new(value).expect("test repository should parse") + } + + fn target(repo: &str) -> GitRunTarget { + GitRunTarget { + repo: repo.to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + } + } + + fn source( + repo: &str, + branch: &str, + tag: Option<&str>, + sha: Option<&str>, + ) -> AutomationGitWorkflowSource { + AutomationGitWorkflowSource { + repo: repo.to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), + } + } + + fn input( + target_repo: &str, + workflow_source: Option, + temp_root: &Path, + ) -> AutomationRunMaterializeInput { + AutomationRunMaterializeInput { + automation_id: AutomationId::new("nightly").unwrap(), + target: target(target_repo), + workflow_source, + workflow: "demo".to_string(), + run_id: RunId::new(), + temp_root: temp_root.to_path_buf(), + } + } + + fn test_version_store() -> WorkflowVersionStore { + let database = fabro_store::test_support::test_database( + Arc::new(InMemory::new()), + "", + Duration::from_millis(1), + None, + ); + WorkflowVersionStore::new(database.blobs()) + } + + fn production_materializer( + root: &Path, + store: WorkflowVersionStore, + resolver: Arc, + clone_urls: HashMap, + ) -> ProductionAutomationRunMaterializer { + ProductionAutomationRunMaterializer::new( + None, + "https://api.github.com".to_string(), + None, + Arc::new(GitRepoCache::new(root.join("cache"))), + store, + ) + .with_remote_resolver(Arc::new(FixtureRemoteResolver { + credentials: resolver, + clone_urls, + })) + } + + #[tokio::test] + async fn coordinate_validation_errors_have_role_specific_nonduplicated_chains() { + let temp = TempDir::new().unwrap(); + let materializer = production_materializer( + temp.path(), + test_version_store(), + Arc::new(RecordingCredentialResolver::succeeds()), + HashMap::new(), + ); + + let mut invalid_target = + input("fabro-sh/target", None, &temp.path().join("invalid-target")); + invalid_target.target.branch = "refs/heads/main".to_string(); + let error = materializer.materialize(invalid_target).await.unwrap_err(); + assert_eq!(fabro_util::error::collect_chain(&error), [ + "invalid automation Git target", + "branch must be a non-empty branch name, not a ref or commit selector", + ]); + + let error = materializer + .materialize(input( + "fabro-sh/target", + Some(source("fabro-sh/workflows", "refs/heads/main", None, None)), + &temp.path().join("invalid-source"), + )) + .await + .unwrap_err(); + assert_eq!(fabro_util::error::collect_chain(&error), [ + "invalid automation workflow source", + "branch must be a non-empty branch name, not a ref or commit selector", + ]); + } + #[tokio::test] async fn collected_closure_stores_dependency_first_and_idempotently() { let temp = TempDir::new().unwrap(); @@ -383,13 +845,7 @@ mod tests { let closure = fabro_manifest::collect_workflow_versions(Path::new("root"), &checkout).unwrap(); - let database = fabro_store::test_support::test_database( - Arc::new(InMemory::new()), - "", - Duration::from_millis(1), - None, - ); - let store = WorkflowVersionStore::new(database.blobs()); + let store = test_version_store(); for _ in 0..2 { for (expected, version) in closure.versions() { @@ -404,4 +860,435 @@ mod tests { assert_eq!(loaded.root_id(), closure.root_id()); assert_eq!(loaded.versions().count(), 2); } + + #[tokio::test] + async fn omitted_source_packages_target_checkout_and_returns_exact_target() { + let temp = TempDir::new().unwrap(); + let target_fixture = seed_repository(temp.path(), "target", "target workflow"); + let target_repo = repository("fabro-sh/target"); + let store = test_version_store(); + let resolver = Arc::new(RecordingCredentialResolver::succeeds()); + let materializer = production_materializer( + temp.path(), + store.clone(), + Arc::clone(&resolver), + HashMap::from([( + target_repo.clone(), + target_fixture.bare.to_string_lossy().into_owned(), + )]), + ); + + let materialized = materializer + .materialize(input("fabro-sh/target", None, &temp.path().join("runs"))) + .await + .unwrap(); + + assert_eq!( + materialized.target.sha.as_deref(), + Some(target_fixture.initial_sha.as_str()) + ); + assert_eq!(materialized.workflow_source, None); + let version = store + .get(&materialized.workflow_version_id) + .await + .unwrap() + .unwrap(); + assert!( + version + .version() + .files() + .values() + .any(|contents| contents.contains("target workflow")) + ); + assert_eq!(resolver.repositories(), vec!["fabro-sh/target"]); + } + + #[tokio::test] + async fn independent_source_packages_source_and_resolves_both_repositories() { + let temp = TempDir::new().unwrap(); + let target_fixture = seed_repository(temp.path(), "target", "target workflow"); + let source_fixture = seed_repository(temp.path(), "source", "source workflow"); + let target_repo = repository("fabro-sh/target"); + let source_repo = repository("fabro-sh/workflows"); + let store = test_version_store(); + let resolver = Arc::new(RecordingCredentialResolver::succeeds()); + let materializer = production_materializer( + temp.path(), + store.clone(), + Arc::clone(&resolver), + HashMap::from([ + ( + target_repo, + target_fixture.bare.to_string_lossy().into_owned(), + ), + ( + source_repo, + source_fixture.bare.to_string_lossy().into_owned(), + ), + ]), + ); + + let materialized = materializer + .materialize(input( + "fabro-sh/target", + Some(source("fabro-sh/workflows", "main", None, None)), + &temp.path().join("runs"), + )) + .await + .unwrap(); + + assert_eq!( + materialized.target.sha.as_deref(), + Some(target_fixture.initial_sha.as_str()) + ); + assert_eq!( + materialized.workflow_source, + Some(Box::new(ResolvedAutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + resolved_sha: source_fixture.initial_sha.clone(), + })) + ); + let version = store + .get(&materialized.workflow_version_id) + .await + .unwrap() + .unwrap(); + let canonical = version.version().canonical_bytes().unwrap(); + let canonical = String::from_utf8(canonical).unwrap(); + assert!(canonical.contains("source workflow")); + assert!(!canonical.contains("target workflow")); + assert!(!canonical.contains(&temp.path().display().to_string())); + assert_eq!(resolver.repositories(), vec![ + "fabro-sh/target", + "fabro-sh/workflows" + ]); + } + + #[tokio::test] + async fn identical_explicit_coordinate_reuses_checkout_and_credentials() { + let temp = TempDir::new().unwrap(); + let fixture = seed_repository(temp.path(), "shared", "shared workflow"); + let repo = repository("fabro-sh/shared"); + let store = test_version_store(); + let resolver = Arc::new(RecordingCredentialResolver::succeeds()); + let materializer = production_materializer( + temp.path(), + store, + Arc::clone(&resolver), + HashMap::from([(repo, fixture.bare.to_string_lossy().into_owned())]), + ); + + let materialized = materializer + .materialize(input( + "Fabro-Sh/Shared", + Some(source("fabro-sh/shared", "main", None, None)), + &temp.path().join("runs"), + )) + .await + .unwrap(); + + assert_eq!( + materialized.workflow_source, + Some(Box::new(ResolvedAutomationGitWorkflowSource { + repo: "fabro-sh/shared".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + resolved_sha: fixture.initial_sha, + })) + ); + assert_eq!(resolver.repositories(), vec!["Fabro-Sh/Shared"]); + } + + #[tokio::test] + async fn same_repository_with_a_different_selector_reuses_credentials_for_a_second_worktree() { + let temp = TempDir::new().unwrap(); + let fixture = seed_repository(temp.path(), "shared", "shared workflow"); + let repo = repository("fabro-sh/shared"); + let resolver = Arc::new(RecordingCredentialResolver::succeeds()); + let materializer = production_materializer( + temp.path(), + test_version_store(), + Arc::clone(&resolver), + HashMap::from([(repo, fixture.bare.to_string_lossy().into_owned())]), + ); + + materializer + .materialize(input( + "fabro-sh/shared", + Some(source( + "fabro-sh/shared", + "main", + Some("annotated-v1"), + None, + )), + &temp.path().join("runs"), + )) + .await + .unwrap(); + + assert_eq!(resolver.repositories(), vec!["fabro-sh/shared"]); + } + + #[tokio::test] + 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"); + let store = test_version_store(); + let resolver = Arc::new(RecordingCredentialResolver::succeeds()); + let materializer = production_materializer( + temp.path(), + store, + resolver, + HashMap::from([ + ( + repository("fabro-sh/target"), + target_fixture.bare.to_string_lossy().into_owned(), + ), + ( + repository("fabro-sh/source"), + source_fixture.bare.to_string_lossy().into_owned(), + ), + ]), + ); + let runs = temp.path().join("runs"); + let materialize = |branch: &str, tag: Option<&str>, sha: Option<&str>| { + materializer.materialize(input( + "fabro-sh/target", + Some(source("fabro-sh/source", branch, tag, sha)), + &runs, + )) + }; + + let branch_v1 = materialize("main", None, None) + .await + .unwrap() + .workflow_version_id; + for tag in ["annotated-v1", "lightweight-v1"] { + let tagged = materialize("main", Some(tag), None).await.unwrap(); + assert_eq!(tagged.workflow_version_id, branch_v1, "{tag}"); + } + let committed_v1 = materialize( + "branch-that-does-not-exist", + Some("missing-tag"), + Some(&source_fixture.initial_sha), + ) + .await + .unwrap() + .workflow_version_id; + assert_eq!(committed_v1, branch_v1); + + advance_repository(&source_fixture, "source v2"); + let branch_v2 = materialize("main", None, None) + .await + .unwrap() + .workflow_version_id; + assert_ne!(branch_v2, branch_v1); + let committed_after_advance = materialize( + "branch-that-does-not-exist", + None, + Some(&source_fixture.initial_sha), + ) + .await + .unwrap() + .workflow_version_id; + assert_eq!(committed_after_advance, committed_v1); + } + + #[tokio::test] + async fn target_and_source_failures_keep_distinct_error_chains_without_tokens() { + let temp = TempDir::new().unwrap(); + let target_fixture = seed_repository(temp.path(), "target", "target workflow"); + let source_fixture = seed_repository(temp.path(), "source", "source workflow"); + let target_repo = repository("fabro-sh/target"); + let source_repo = repository("fabro-sh/source"); + let clone_urls = HashMap::from([ + ( + target_repo.clone(), + target_fixture.bare.to_string_lossy().into_owned(), + ), + ( + source_repo.clone(), + source_fixture.bare.to_string_lossy().into_owned(), + ), + ]); + + let mut missing_target = input( + "fabro-sh/target", + None, + &temp.path().join("target-checkout-failure"), + ); + missing_target.target.branch = "missing".to_string(); + let error = production_materializer( + temp.path(), + test_version_store(), + Arc::new(RecordingCredentialResolver::succeeds()), + clone_urls.clone(), + ) + .materialize(missing_target) + .await + .unwrap_err(); + assert!(matches!(error, RunMaterializeError::Checkout { + role: CheckoutRole::Target, + source: GitCheckoutError::FetchBranch { .. }, + })); + assert!(!format!("{error:?}").contains(FAKE_TOKEN)); + + let target_resolver = Arc::new(RecordingCredentialResolver::fails_for(target_repo.clone())); + let error = production_materializer( + temp.path(), + test_version_store(), + target_resolver, + clone_urls.clone(), + ) + .materialize(input( + "fabro-sh/target", + None, + &temp.path().join("target-failure"), + )) + .await + .unwrap_err(); + assert!(matches!(error, RunMaterializeError::Credentials { + role: CheckoutRole::Target, + .. + })); + assert!(!format!("{error:?}").contains(FAKE_TOKEN)); + + let source_resolver = Arc::new(RecordingCredentialResolver::fails_for(source_repo)); + let error = production_materializer( + temp.path(), + test_version_store(), + source_resolver, + clone_urls, + ) + .materialize(input( + "fabro-sh/target", + Some(source("fabro-sh/source", "main", None, None)), + &temp.path().join("source-failure"), + )) + .await + .unwrap_err(); + assert!(matches!(error, RunMaterializeError::Credentials { + role: CheckoutRole::WorkflowSource, + .. + })); + assert!(!format!("{error:?}").contains(FAKE_TOKEN)); + + let error = production_materializer( + temp.path(), + test_version_store(), + Arc::new(RecordingCredentialResolver::succeeds()), + HashMap::from([ + ( + target_repo, + target_fixture.bare.to_string_lossy().into_owned(), + ), + ( + repository("fabro-sh/source"), + source_fixture.bare.to_string_lossy().into_owned(), + ), + ]), + ) + .materialize(input( + "fabro-sh/target", + Some(source("fabro-sh/source", "missing", None, None)), + &temp.path().join("checkout-failure"), + )) + .await + .unwrap_err(); + assert!(matches!(error, RunMaterializeError::Checkout { + role: CheckoutRole::WorkflowSource, + source: GitCheckoutError::FetchBranch { .. }, + })); + assert!(!format!("{error:?}").contains(FAKE_TOKEN)); + } + + #[tokio::test] + async fn workflow_discovery_and_version_storage_failures_remain_distinct() { + let temp = TempDir::new().unwrap(); + let fixture = seed_repository(temp.path(), "target", "target workflow"); + let repo = repository("fabro-sh/target"); + let clone_urls = HashMap::from([(repo, fixture.bare.to_string_lossy().into_owned())]); + let mut missing = input( + "fabro-sh/target", + None, + &temp.path().join("missing-workflow"), + ); + missing.workflow = ".fabro/workflows/absent/workflow.fabro".to_string(); + let error = production_materializer( + temp.path(), + test_version_store(), + Arc::new(RecordingCredentialResolver::succeeds()), + clone_urls.clone(), + ) + .materialize(missing) + .await + .unwrap_err(); + assert!( + matches!(error, RunMaterializeError::WorkflowNotFound { .. }), + "unexpected missing-workflow error: {error:?}" + ); + + let pool = sqlx::sqlite::SqlitePoolOptions::new() + .connect_lazy("sqlite::memory:") + .unwrap(); + pool.close().await; + let failing_store = WorkflowVersionStore::new(Arc::new(fabro_store::BlobStore::new(pool))); + let error = production_materializer( + temp.path(), + failing_store, + Arc::new(RecordingCredentialResolver::succeeds()), + clone_urls, + ) + .materialize(input( + "fabro-sh/target", + None, + &temp.path().join("store-failure"), + )) + .await + .unwrap_err(); + assert!(matches!(error, RunMaterializeError::VersionStore { .. })); + + fs::write( + fixture.work.join(".fabro/workflows/demo/workflow.fabro"), + "this is not a graph\n", + ) + .unwrap(); + run_git(&["-C", fixture.work.to_str().unwrap(), "add", "."]); + run_git(&[ + "-C", + fixture.work.to_str().unwrap(), + "commit", + "-m", + "break workflow", + ]); + run_git(&[ + "-C", + fixture.work.to_str().unwrap(), + "push", + fixture.bare.to_str().unwrap(), + "main", + ]); + let error = production_materializer( + temp.path(), + test_version_store(), + Arc::new(RecordingCredentialResolver::succeeds()), + HashMap::from([( + repository("fabro-sh/target"), + fixture.bare.to_string_lossy().into_owned(), + )]), + ) + .materialize(input( + "fabro-sh/target", + None, + &temp.path().join("package-failure"), + )) + .await + .unwrap_err(); + assert!(matches!(error, RunMaterializeError::Package { .. })); + } } diff --git a/lib/apps/fabro-server/src/git_checkout.rs b/lib/apps/fabro-server/src/git_checkout.rs index 4f33ce463..9a2e8c6f0 100644 --- a/lib/apps/fabro-server/src/git_checkout.rs +++ b/lib/apps/fabro-server/src/git_checkout.rs @@ -104,7 +104,8 @@ impl GitRepoCache { .join(format!("{}.git", repo.repo())) } - /// Prepare a worktree containing the requested ref of `repo` at + /// Prepare a worktree containing the requested ref of `repo`, fetched from + /// `clone_url`, at /// `worktree_dir`. Returns the resolved commit SHA. /// /// First call for a repo: a `--bare --depth 1` clone is created at @@ -117,14 +118,6 @@ impl GitRepoCache { pub(crate) async fn prepare_worktree( &self, args: WorktreePrepareInput<'_>, - ) -> Result { - let clone_url = github_clone_url(args.repo); - self.prepare_worktree_with_clone_url(args, &clone_url).await - } - - async fn prepare_worktree_with_clone_url( - &self, - args: WorktreePrepareInput<'_>, clone_url: &str, ) -> Result { let _guard = self @@ -179,7 +172,7 @@ impl GitRepoCache { .map_err(|source| GitCheckoutError::Clone { source })?; } - let fetch_target = GitFetchTarget::from(args.target); + let fetch_target = args.selector; run_git_plan(build_bare_fetch_plan( bare_dir, clone_url, @@ -202,18 +195,19 @@ impl GitRepoCache { pub(crate) struct WorktreePrepareInput<'a> { pub repo: &'a GitHubRepositorySlug, - pub target: &'a GitRunTarget, + pub selector: GitCheckoutSelector<'a>, pub auth: Option<&'a GitAuthConfig>, pub worktree_dir: &'a Path, } -enum GitFetchTarget<'a> { +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum GitCheckoutSelector<'a> { Branch(&'a str), Tag(&'a str), Commit(&'a str), } -impl<'a> From<&'a GitRunTarget> for GitFetchTarget<'a> { +impl<'a> From<&'a GitRunTarget> for GitCheckoutSelector<'a> { fn from(target: &'a GitRunTarget) -> Self { if let Some(sha) = target.sha.as_deref() { Self::Commit(sha) @@ -225,7 +219,7 @@ impl<'a> From<&'a GitRunTarget> for GitFetchTarget<'a> { } } -impl GitFetchTarget<'_> { +impl GitCheckoutSelector<'_> { fn selector(&self) -> Cow<'_, str> { match self { Self::Branch(selector) | Self::Commit(selector) => Cow::Borrowed(selector), @@ -268,7 +262,7 @@ async fn bare_clone_may_be_corrupt(bare_dir: &Path) -> bool { } } -fn github_clone_url(repo: &GitHubRepositorySlug) -> String { +pub(crate) fn github_clone_url(repo: &GitHubRepositorySlug) -> String { let mut url = repo.https_url(); url.push_str(".git"); url @@ -276,40 +270,36 @@ fn github_clone_url(repo: &GitHubRepositorySlug) -> String { #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct GitAuthConfig { - extraheader: Option, + extraheader: String, sensitive_values: Vec, } impl GitAuthConfig { - fn new(username: Option, password: Option) -> Self { - let Some(password) = password.filter(|value| !value.is_empty()) else { - return Self { - extraheader: None, - sensitive_values: Vec::new(), - }; - }; - let username = username - .filter(|value| !value.is_empty()) - .unwrap_or_else(|| "x-access-token".to_string()); + pub(crate) fn new(credentials: &fabro_github::GitCloneCredentials) -> Self { + Self::from_parts(credentials.username(), credentials.password()) + } + + pub(crate) fn from_parts(username: &str, password: &str) -> Self { let encoded_credentials = BASE64_STANDARD.encode(format!("{username}:{password}")); let extraheader = basic_auth_header_from_encoded(&encoded_credentials); Self { - sensitive_values: vec![password, encoded_credentials, extraheader.clone()], - extraheader: Some(extraheader), + sensitive_values: vec![ + password.to_string(), + encoded_credentials, + extraheader.clone(), + ], + extraheader, } } fn git_env(&self, clone_url: &str) -> Vec<(String, String)> { - let Some(extraheader) = self.extraheader.as_ref() else { - return Vec::new(); - }; vec![ ("GIT_CONFIG_COUNT".to_string(), "1".to_string()), ( "GIT_CONFIG_KEY_0".to_string(), format!("http.{clone_url}.extraheader"), ), - ("GIT_CONFIG_VALUE_0".to_string(), extraheader.clone()), + ("GIT_CONFIG_VALUE_0".to_string(), self.extraheader.clone()), ] } @@ -318,7 +308,7 @@ impl GitAuthConfig { } } -pub(crate) async fn resolve_git_auth_config( +pub(crate) async fn resolve_git_read_auth_config( credentials: Option<&fabro_github::GitHubCredentials>, repo: &GitHubRepositorySlug, github_api_base_url: &str, @@ -333,9 +323,10 @@ pub(crate) async fn resolve_git_auth_config( } None => fabro_github::GitHubContext::new(credentials, github_api_base_url), }; - let (username, password) = - fabro_github::resolve_clone_credentials(&context, repo.owner(), repo.repo()).await?; - Ok(Some(GitAuthConfig::new(username, password))) + let credentials = + fabro_github::resolve_read_only_clone_credentials(&context, repo.owner(), repo.repo()) + .await?; + Ok(Some(GitAuthConfig::new(&credentials))) } #[cfg(test)] @@ -575,6 +566,69 @@ mod tests { } } + 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_use_sha_then_tag_then_branch_precedence() { + let target = git_target( + "main", + Some("v1"), + Some("abcdef0123456789abcdef0123456789abcdef01"), + ); + assert_eq!( + GitCheckoutSelector::from(&target), + GitCheckoutSelector::Commit("abcdef0123456789abcdef0123456789abcdef01") + ); + + for (source, expected) in [ + ( + workflow_source("main", None, None), + GitCheckoutSelector::Branch("main"), + ), + ( + workflow_source("main", Some("v1"), None), + GitCheckoutSelector::Tag("v1"), + ), + ( + workflow_source( + "unrelated-context", + Some("v1"), + Some("abcdef0123456789abcdef0123456789abcdef01"), + ), + GitCheckoutSelector::Commit("abcdef0123456789abcdef0123456789abcdef01"), + ), + ] { + assert_eq!(GitCheckoutSelector::from(&source), expected); + } + } + + #[test] + fn checkout_reuse_identity_folds_only_repository_case() { + assert_eq!( + repository_slug("Fabro-Sh/Workflows"), + repository_slug("fabro-sh/workflows") + ); + assert_ne!( + GitCheckoutSelector::Branch("Main"), + GitCheckoutSelector::Branch("main") + ); + assert_ne!( + GitCheckoutSelector::Branch("v1"), + GitCheckoutSelector::Tag("v1") + ); + assert_eq!( + GitCheckoutSelector::Commit("abcdef0123456789abcdef0123456789abcdef01"), + GitCheckoutSelector::Commit("abcdef0123456789abcdef0123456789abcdef01") + ); + } + #[test] fn target_repository_urls_are_github_metadata_urls_without_credentials() { let repo = repository_slug("fabro-sh/fabro"); @@ -690,10 +744,7 @@ mod tests { fn credential_config_env_keeps_clone_url_uncredentialed() { let repo = repository_slug("fabro-sh/fabro"); let clone_url = github_clone_url(&repo); - let auth = GitAuthConfig::new( - Some("x-access-token".to_string()), - Some("ghu_secret".to_string()), - ); + let auth = GitAuthConfig::from_parts("x-access-token", "ghu_secret"); let plan = build_bare_clone_plan(&clone_url, Path::new("/tmp/fabro-checkout"), Some(&auth)); assert!( @@ -807,10 +858,10 @@ mod tests { let worktree_a = temp.path().join("wt-a"); let sha_a = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &target, + selector: GitCheckoutSelector::from(&target), auth: None, worktree_dir: &worktree_a, }, @@ -829,10 +880,10 @@ mod tests { let worktree_b = temp.path().join("wt-b"); let sha_b = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &target, + selector: GitCheckoutSelector::from(&target), auth: None, worktree_dir: &worktree_b, }, @@ -860,10 +911,10 @@ mod tests { let worktree_a = temp.path().join("wt-a"); cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &target, + selector: GitCheckoutSelector::from(&target), auth: None, worktree_dir: &worktree_a, }, @@ -878,10 +929,10 @@ mod tests { let worktree_b = temp.path().join("wt-b"); let sha = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &target, + selector: GitCheckoutSelector::from(&target), auth: None, worktree_dir: &worktree_b, }, @@ -916,10 +967,10 @@ mod tests { ("commit", git_target("main", None, Some(&expected_sha))), ] { let sha = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &target, + selector: GitCheckoutSelector::from(&target), auth: None, worktree_dir: &temp.path().join(name), }, @@ -944,10 +995,10 @@ mod tests { let unavailable_commit = git_target("main", None, Some(unavailable_sha)); let tag_error = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &missing_tag, + selector: GitCheckoutSelector::from(&missing_tag), auth: None, worktree_dir: &temp.path().join("missing-tag"), }, @@ -961,10 +1012,10 @@ mod tests { )); let commit_error = cache - .prepare_worktree_with_clone_url( + .prepare_worktree( WorktreePrepareInput { repo: &repo, - target: &unavailable_commit, + selector: GitCheckoutSelector::from(&unavailable_commit), auth: None, worktree_dir: &temp.path().join("missing-commit"), }, diff --git a/lib/apps/fabro-server/src/run_compiler.rs b/lib/apps/fabro-server/src/run_compiler.rs index 7285f4ccd..732d68bbc 100644 --- a/lib/apps/fabro-server/src/run_compiler.rs +++ b/lib/apps/fabro-server/src/run_compiler.rs @@ -1025,9 +1025,10 @@ include = ["reports/{{ vars.path }}/*.json"] let run_id = RunId::new(); let parent_id = RunId::new(); let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule".to_string()), + workflow_source: None, }; let submitted = b"submitted manifest".to_vec(); let workflow_version_id = fabro_types::test_support::test_workflow_version_id(); diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index fcd6d0b0e..ab06cf48e 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -1205,7 +1205,7 @@ impl AppState { let credentials = self .github_credentials(&settings.server.integrations.github) .await - .map_err(|source| RunMaterializeError::Credentials { source })?; + .map_err(|source| RunMaterializeError::LoadCredentials { source })?; ProductionAutomationRunMaterializer::new( credentials, self.github_api_base_url.clone(), diff --git a/lib/apps/fabro-server/src/server/automation_scheduler.rs b/lib/apps/fabro-server/src/server/automation_scheduler.rs index 43d30d93d..3bceb22d3 100644 --- a/lib/apps/fabro-server/src/server/automation_scheduler.rs +++ b/lib/apps/fabro-server/src/server/automation_scheduler.rs @@ -263,7 +263,8 @@ async fn fire_scheduled_automation_run( .materialize_automation_run(AutomationRunMaterializeInput { automation_id: automation_id.clone(), target, - workflow: automation.workflow.clone(), + workflow_source: automation.workflow_source, + workflow: automation.workflow, run_id, temp_root: state.automation_temp_root(), }) @@ -286,9 +287,10 @@ async fn fire_scheduled_automation_run( system_kind: SystemActorKind::Engine, }; let automation_ref = AutomationRef { - id: automation_id.to_string(), - name: Some(automation.name.clone()), - trigger_id: Some(trigger_id.to_string()), + id: automation_id.to_string(), + name: Some(automation.name.clone()), + trigger_id: Some(trigger_id.to_string()), + workflow_source: materialized.workflow_source.clone(), }; // RunIntent admission produces a large future; box it to keep our // stack frame small (matches handler/automations.rs). @@ -388,10 +390,12 @@ fn run_due_schedules_once<'a>( #[cfg(test)] mod tests { - use fabro_automation::{AutomationDraft, AutomationTrigger, ScheduleTrigger}; + use fabro_automation::{ + AutomationDraft, AutomationGitWorkflowSource, AutomationTrigger, ScheduleTrigger, + }; use fabro_static::EnvVars; use fabro_store::ListRunsQuery; - use fabro_types::{GitRunTarget, RunStatus, RunTarget}; + use fabro_types::{GitRunTarget, ResolvedAutomationGitWorkflowSource, RunStatus, RunTarget}; use super::*; use crate::test_support::{TestAppStateBuilder, TestAutomationRunMaterializer}; @@ -432,6 +436,7 @@ mod tests { environment_id: Some("default".to_string()), last_error: None, target: target(), + workflow_source: None, workflow: "workflow.fabro".to_string(), triggers, } @@ -442,6 +447,16 @@ mod tests { id: &str, name: &str, triggers: Vec, + ) -> Automation { + create_automation_with_source(state, id, name, None, triggers).await + } + + async fn create_automation_with_source( + state: &AppState, + id: &str, + name: &str, + workflow_source: Option, + triggers: Vec, ) -> Automation { state .automation_store() @@ -451,6 +466,7 @@ mod tests { description: None, environment_id: Some("default".to_string()), target: target(), + workflow_source, workflow: "workflow.fabro".to_string(), triggers, }) @@ -701,6 +717,48 @@ mod tests { assert_eq!(cached_runs(state.as_ref()).await.len(), 1); } + #[tokio::test] + async fn scheduled_run_passes_saved_workflow_source_to_materialization() { + let materializer = succeeding_materializer(); + let state = test_state_with_materializer(materializer.clone()); + let workflow_source = AutomationGitWorkflowSource { + 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(), + "scheduled-source", + "scheduled-source", + Some(workflow_source.clone()), + vec![schedule_trigger("schedule", "* * * * *", true)], + ) + .await; + let mut planner = AutomationSchedulePlanner::default(); + + run_due_schedules_once(Arc::clone(&state), &mut planner, prime_time()).await; + run_due_schedules_once(Arc::clone(&state), &mut planner, first_due_time()).await; + + let captured = materializer.captured_inputs(); + assert_eq!(captured.len(), 1); + assert_eq!(captured[0].workflow_source, Some(workflow_source.clone())); + let runs = cached_runs(state.as_ref()).await; + assert_eq!(runs.len(), 1); + assert_eq!( + runs[0] + .automation + .as_ref() + .and_then(|automation| automation.workflow_source.clone()), + Some(Box::new( + ResolvedAutomationGitWorkflowSource::from_requested( + workflow_source, + "ffffffffffffffffffffffffffffffffffffffff".to_string(), + ) + )) + ); + } + #[tokio::test] async fn disabled_schedule_trigger_does_not_create_run() { let materializer = succeeding_materializer(); @@ -802,4 +860,30 @@ mod tests { assert!(cached_runs(state.as_ref()).await.is_empty()); assert_eq!(materializer.captured_inputs().len(), 2); } + + #[tokio::test] + async fn workflow_source_failure_creates_and_starts_no_scheduled_run() { + let materializer = TestAutomationRunMaterializer::fail_invalid_workflow_source(); + let state = test_state_with_materializer(materializer.clone()); + create_automation_with_source( + state.as_ref(), + "failing-source", + "failing-source", + Some(AutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + }), + vec![schedule_trigger("schedule", "* * * * *", true)], + ) + .await; + let mut planner = AutomationSchedulePlanner::default(); + + run_due_schedules_once(Arc::clone(&state), &mut planner, prime_time()).await; + run_due_schedules_once(Arc::clone(&state), &mut planner, first_due_time()).await; + + assert!(cached_runs(state.as_ref()).await.is_empty()); + assert_eq!(materializer.captured_inputs().len(), 1); + } } diff --git a/lib/apps/fabro-server/src/server/handler/automations.rs b/lib/apps/fabro-server/src/server/handler/automations.rs index 88eb51ad3..ac99cd822 100644 --- a/lib/apps/fabro-server/src/server/handler/automations.rs +++ b/lib/apps/fabro-server/src/server/handler/automations.rs @@ -140,6 +140,7 @@ async fn create_automation_run( .materialize_automation_run(AutomationRunMaterializeInput { automation_id: automation.id.clone(), target, + workflow_source: automation.workflow_source.clone(), workflow: automation.workflow.clone(), run_id, temp_root: state.automation_temp_root(), @@ -153,9 +154,10 @@ async fn create_automation_run( } }; let automation_ref = AutomationRef { - id: automation.id.to_string(), - name: Some(automation.name.clone()), - trigger_id: Some(api_trigger_id), + id: automation.id.to_string(), + name: Some(automation.name.clone()), + trigger_id: Some(api_trigger_id), + workflow_source: materialized.workflow_source.clone(), }; let response = Box::pin(runs::create_run_from_intent( diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 075e48c25..3ab74ee64 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -4586,9 +4586,10 @@ async fn create_run_from_manifest_helper_persists_automation_metadata_and_exact_ let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); let run_id = RunId::new(); let automation = fabro_types::AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule".to_string()), + workflow_source: None, }; let target = RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), @@ -4645,9 +4646,10 @@ async fn create_run_from_intent_helper_persists_automation_version_and_exact_tar let workflow_version_id = store_workflow_version(&state, MINIMAL_DOT, None).await; let run_id = RunId::new(); let automation = fabro_types::AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule".to_string()), + workflow_source: None, }; let target = RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), @@ -5083,6 +5085,7 @@ async fn fake_automation_materializer_injection_captures_input_and_returns_versi .materialize_automation_run(AutomationRunMaterializeInput { automation_id: AutomationId::new("nightly").unwrap(), target: target.clone(), + workflow_source: None, workflow: "demo".to_string(), run_id, temp_root: temp_root.clone(), diff --git a/lib/apps/fabro-server/tests/it/api/automations.rs b/lib/apps/fabro-server/tests/it/api/automations.rs index 9d09f0fec..d5dfa1e4d 100644 --- a/lib/apps/fabro-server/tests/it/api/automations.rs +++ b/lib/apps/fabro-server/tests/it/api/automations.rs @@ -2,6 +2,7 @@ use std::path::{Path, PathBuf}; use axum::body::Body; use axum::http::{Method, Request, StatusCode, header}; +use fabro_automation::AutomationGitWorkflowSource; use fabro_config::Storage; use fabro_server::server::build_router; use fabro_server::test_support::{ @@ -1034,6 +1035,47 @@ async fn incomplete_legacy_automation_fails_before_materialization() { ); } +#[tokio::test] +async fn api_triggered_run_passes_saved_workflow_source_to_materialization() { + let materializer = TestAutomationRunMaterializer::succeed(GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "main".to_string(), + tag: None, + sha: Some("0123456789abcdef0123456789abcdef01234567".to_string()), + }); + let (app, _temp_dir, _automation_dir) = automation_app_with_materializer(materializer.clone()); + let mut body = automation_body("nightly", "Nightly"); + body["workflow_source"] = json!({ + "repo": "fabro-sh/workflows", + "branch": "main", + "tag": "release-v1" + }); + create_automation_with_body(&app, &body).await; + + let created = create_automation_run(&app, "nightly", StatusCode::CREATED).await; + + let captured = materializer.captured_workflow_sources(); + assert_eq!(captured.len(), 1); + assert_eq!( + captured[0], + Some(AutomationGitWorkflowSource { + 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", + "branch": "main", + "tag": "release-v1", + "resolved_sha": "ffffffffffffffffffffffffffffffffffffffff" + }) + ); +} + #[tokio::test] async fn api_triggered_automation_with_missing_version_does_not_create_or_start_a_run() { let materializer = TestAutomationRunMaterializer::return_unstored_version(GitRunTarget { @@ -1053,6 +1095,29 @@ async fn api_triggered_automation_with_missing_version_does_not_create_or_start_ assert_eq!(runs["data"], json!([])); } +#[tokio::test] +async fn api_workflow_source_failure_does_not_create_or_start_a_run() { + let materializer = TestAutomationRunMaterializer::fail_invalid_workflow_source(); + let (app, _temp_dir, _automation_dir) = automation_app_with_materializer(materializer); + let mut body = automation_body("nightly", "Nightly"); + body["workflow_source"] = json!({ + "repo": "fabro-sh/workflows", + "branch": "main" + }); + create_automation_with_body(&app, &body).await; + + let error = create_automation_run(&app, "nightly", StatusCode::UNPROCESSABLE_ENTITY).await; + + assert!( + error["errors"][0]["detail"] + .as_str() + .is_some_and(|detail| detail.contains("workflow source")) + ); + let runs = list_automation_runs(&app, "/automations/nightly/runs").await; + assert_eq!(runs["meta"]["total"], 0); + assert_eq!(runs["data"], json!([])); +} + #[tokio::test] async fn automation_run_listing_includes_only_runs_for_that_automation() { let (app, _temp_dir, _automation_dir) = automation_app_with_fake_materializer(); diff --git a/lib/components/fabro-automation/Cargo.toml b/lib/components/fabro-automation/Cargo.toml index 798af66b2..6677b28c6 100644 --- a/lib/components/fabro-automation/Cargo.toml +++ b/lib/components/fabro-automation/Cargo.toml @@ -28,5 +28,6 @@ tracing.workspace = true [dev-dependencies] anyhow.workspace = true +serde_json.workspace = true tempfile = "3" tokio = { workspace = true, features = ["macros", "test-util"] } diff --git a/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs b/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs index cf7ba86d1..224506fd1 100644 --- a/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs +++ b/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs @@ -108,6 +108,7 @@ fn parse_legacy_automation( environment_id: None, target, workflow, + workflow_source: None, triggers: legacy.triggers, }) .map_err(|source| AutomationStoreError::StoredValidation { id, source }) diff --git a/lib/components/fabro-automation/migrations/2026082801_environment_selectors.rs b/lib/components/fabro-automation/migrations/2026082801_environment_selectors.rs index 47301780e..166119cb4 100644 --- a/lib/components/fabro-automation/migrations/2026082801_environment_selectors.rs +++ b/lib/components/fabro-automation/migrations/2026082801_environment_selectors.rs @@ -62,12 +62,13 @@ pub async fn backfill_environment_selectors( for automation in &incomplete { store .replace(&automation.id, &automation.revision, AutomationReplace { - name: automation.name.clone(), - description: automation.description.clone(), - environment_id: Some(environment_id.clone()), - target: automation.target.clone(), - workflow: automation.workflow.clone(), - triggers: automation.triggers.clone(), + name: automation.name.clone(), + description: automation.description.clone(), + environment_id: Some(environment_id.clone()), + target: automation.target.clone(), + workflow: automation.workflow.clone(), + workflow_source: automation.workflow_source.clone(), + triggers: automation.triggers.clone(), }) .await?; } diff --git a/lib/components/fabro-automation/src/error.rs b/lib/components/fabro-automation/src/error.rs index b7f319baa..55104055a 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::TargetValidationError; +use fabro_types::{GitCoordinateValidationError, TargetValidationError}; use toml::de::Error as TomlDeError; use toml::ser::Error as TomlSerError; @@ -24,6 +24,11 @@ pub enum AutomationValidationError { #[source] source: TargetValidationError, }, + #[error("automation workflow source is invalid")] + InvalidWorkflowSource { + #[source] + source: GitCoordinateValidationError, + }, #[error("workflow selector {value:?} is not safe")] InvalidWorkflowSelector { value: String }, #[error("duplicate automation trigger id {id:?}")] @@ -77,6 +82,8 @@ pub enum AutomationStoreError { }, #[error("stored automation {id} has an invalid trigger row")] StoredTriggerShape { id: AutomationId }, + #[error("stored automation {id} has a partial workflow source coordinate")] + StoredWorkflowSourceShape { id: AutomationId }, #[error("stored automation {id} has an invalid revision")] InvalidRevision { id: AutomationId, @@ -163,6 +170,7 @@ impl AutomationStoreError { Self::StoredValidation { .. } => "stored_validation", Self::StoredId { .. } => "stored_id", Self::StoredTriggerShape { .. } => "stored_trigger_shape", + Self::StoredWorkflowSourceShape { .. } => "stored_workflow_source_shape", 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 7eb2c3446..aeca76c9d 100644 --- a/lib/components/fabro-automation/src/lib.rs +++ b/lib/components/fabro-automation/src/lib.rs @@ -12,7 +12,7 @@ pub use migrations::{ import_legacy_directory_once, }; pub use model::{ - ApiTrigger, Automation, AutomationDraft, AutomationReplace, AutomationTrigger, ScheduleTrigger, - parse_schedule_expression, + ApiTrigger, Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationReplace, + 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 ed0312661..6df1e8e0c 100644 --- a/lib/components/fabro-automation/src/model.rs +++ b/lib/components/fabro-automation/src/model.rs @@ -34,19 +34,21 @@ pub fn parse_schedule_expression(expression: &str) -> Result { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct Automation { - pub id: AutomationId, - pub revision: AutomationRevision, - pub name: String, - pub description: Option, + pub id: AutomationId, + pub revision: AutomationRevision, + pub name: String, + pub description: Option, /// Server-managed environment selected when the automation fires. Legacy /// rows may be incomplete until an operator selects one. - pub environment_id: Option, + pub environment_id: Option, /// Most recent scheduler failure. Runtime status is not part of the /// optimistic-concurrency revision. - pub last_error: Option, - pub target: RunTarget, - pub workflow: String, - pub triggers: Vec, + pub last_error: Option, + pub target: RunTarget, + pub workflow: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workflow_source: Option, + pub triggers: Vec, } impl Automation { @@ -140,11 +142,35 @@ impl Automation { last_error: None, target: replace.target, workflow: replace.workflow, + workflow_source: replace.workflow_source, triggers: replace.triggers, } } } +/// 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; + +/// Validate and canonicalize a saved workflow source without resolving remote +/// repository state. +/// +/// # Errors +/// +/// Returns an error when the repository slug, branch, tag, or exact commit +/// does not use the canonical Git-coordinate grammar. +pub fn validate_workflow_source( + source: AutomationGitWorkflowSource, +) -> Result { + source + .validate() + .map(fabro_types::ValidatedGitRunTarget::into_target) + .map_err(|source| AutomationValidationError::InvalidWorkflowSource { source }) +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)] pub enum AutomationTrigger { @@ -200,26 +226,29 @@ pub struct ScheduleTrigger { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct AutomationDraft { - pub id: AutomationId, - pub name: String, + pub id: AutomationId, + pub name: String, #[serde(default, skip_serializing_if = "Option::is_none")] - pub description: Option, + pub description: Option, #[serde(default)] - pub environment_id: Option, - pub target: RunTarget, - pub workflow: String, - pub triggers: Vec, + pub environment_id: Option, + pub target: RunTarget, + pub workflow: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workflow_source: Option, + pub triggers: Vec, } impl From for (AutomationId, AutomationReplace) { fn from(value: AutomationDraft) -> Self { (value.id, AutomationReplace { - name: value.name, - description: value.description, - environment_id: value.environment_id, - target: value.target, - workflow: value.workflow, - triggers: value.triggers, + name: value.name, + description: value.description, + environment_id: value.environment_id, + target: value.target, + workflow: value.workflow, + workflow_source: value.workflow_source, + triggers: value.triggers, }) } } @@ -227,39 +256,44 @@ impl From for (AutomationId, AutomationReplace) { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct AutomationReplace { - pub name: String, + pub name: String, #[serde(default, skip_serializing_if = "Option::is_none")] - pub description: Option, + pub description: Option, #[serde(default)] - pub environment_id: Option, - pub target: RunTarget, - pub workflow: String, - pub triggers: Vec, + pub environment_id: Option, + pub target: RunTarget, + pub workflow: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workflow_source: Option, + pub triggers: Vec, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct PersistedAutomation { - name: String, + name: String, #[serde(default, skip_serializing_if = "Option::is_none")] - description: Option, + description: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - environment_id: Option, - target: RunTarget, - workflow: String, + environment_id: Option, + target: RunTarget, + workflow: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + workflow_source: Option, #[serde(default)] - triggers: Vec, + triggers: Vec, } impl From for PersistedAutomation { fn from(value: AutomationReplace) -> Self { Self { - name: value.name, - description: value.description, - environment_id: value.environment_id, - target: value.target, - workflow: value.workflow, - triggers: value.triggers, + name: value.name, + description: value.description, + environment_id: value.environment_id, + target: value.target, + workflow: value.workflow, + workflow_source: value.workflow_source, + triggers: value.triggers, } } } @@ -267,12 +301,13 @@ impl From for PersistedAutomation { impl From for AutomationReplace { fn from(value: PersistedAutomation) -> Self { Self { - name: value.name, - description: value.description, - environment_id: value.environment_id, - target: value.target, - workflow: value.workflow, - triggers: value.triggers, + name: value.name, + description: value.description, + environment_id: value.environment_id, + target: value.target, + workflow: value.workflow, + workflow_source: value.workflow_source, + triggers: value.triggers, } } } @@ -321,6 +356,10 @@ fn normalize_replace( .environment_id .map(|environment_id| environment_id.trim().to_string()) .filter(|environment_id| !environment_id.is_empty()); + value.workflow_source = value + .workflow_source + .map(validate_workflow_source) + .transpose()?; validate_fields(&value, require_environment)?; let api_enabled = value @@ -431,11 +470,14 @@ fn validate_triggers(triggers: &[AutomationTrigger]) -> Result<(), AutomationVal #[cfg(test)] mod tests { - use fabro_types::{GitRunTarget, RunTarget, TargetValidationError}; + use fabro_types::{ + GitCoordinateValidationError, GitRunTarget, RunTarget, TargetValidationError, + }; use crate::{ - ApiTrigger, Automation, AutomationId, AutomationReplace, AutomationTrigger, - AutomationTriggerId, AutomationValidationError, ScheduleTrigger, + ApiTrigger, Automation, AutomationGitWorkflowSource, AutomationId, AutomationReplace, + AutomationStoreError, AutomationTrigger, AutomationTriggerId, AutomationValidationError, + ScheduleTrigger, }; fn target() -> RunTarget { @@ -466,6 +508,170 @@ mod tests { }) } + fn workflow_source( + branch: &str, + tag: Option<&str>, + sha: Option<&str>, + ) -> AutomationGitWorkflowSource { + AutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), + } + } + + fn replace_with_source( + workflow_source: Option, + ) -> AutomationReplace { + AutomationReplace { + name: "Nightly".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source, + triggers: vec![api_trigger("manual")], + } + } + + #[test] + fn omitted_workflow_source_preserves_canonical_bytes_and_revision() { + let expected = concat!( + "name = \"Nightly\"\n", + "environment_id = \"default\"\n", + "workflow = \"release\"\n", + "\n", + "[target]\n", + "kind = \"git\"\n", + "repo = \"fabro-sh/fabro\"\n", + "branch = \"main\"\n", + "\n", + "[[triggers]]\n", + "type = \"api\"\n", + "id = \"manual\"\n", + "enabled = true\n", + ); + + let (automation, bytes) = Automation::from_replace( + AutomationId::new("nightly").unwrap(), + replace_with_source(None), + ) + .unwrap(); + + assert_eq!(automation.workflow_source, None); + assert_eq!(bytes, expected.as_bytes()); + assert_eq!( + automation.revision.as_str(), + "bc26bacc9ed091f4f171c8fdaf45cd319549a50b4291a5c93205028094abf385" + ); + + let decoded = + Automation::from_toml_bytes(AutomationId::new("nightly").unwrap(), expected.as_bytes()) + .unwrap(); + assert_eq!(decoded.workflow_source, None); + assert_eq!( + serde_json::to_value(decoded) + .unwrap() + .get("workflow_source"), + None + ); + } + + #[test] + 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), + ( + 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(source)), + ) + .unwrap(); + + let source = automation.workflow_source.as_ref().unwrap(); + assert_eq!(source.sha.as_deref(), expected_sha); + assert!( + String::from_utf8(bytes.clone()) + .unwrap() + .contains("[workflow_source]") + ); + + let decoded = + Automation::from_toml_bytes(AutomationId::new("nightly").unwrap(), &bytes).unwrap(); + assert_eq!(decoded.workflow_source, automation.workflow_source); + } + } + + #[test] + fn explicit_workflow_source_changes_revision_and_is_never_collapsed() { + let (omitted, _) = Automation::from_replace( + AutomationId::new("nightly").unwrap(), + replace_with_source(None), + ) + .unwrap(); + 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(), + replace_with_source(Some(explicit_source.clone())), + ) + .unwrap(); + + assert_ne!(explicit.revision, omitted.revision); + assert_eq!(explicit.workflow_source, Some(explicit_source)); + } + + #[test] + fn workflow_source_validation_reports_the_invalid_coordinate_part() { + let cases = [ + ( + workflow_source("main", None, None), + GitCoordinateValidationError::Repository, + ), + ( + workflow_source("refs/heads/main", None, None), + GitCoordinateValidationError::Branch, + ), + ( + workflow_source("main", Some("tags/v1"), None), + GitCoordinateValidationError::Tag, + ), + ( + workflow_source("main", None, Some("short")), + GitCoordinateValidationError::Sha, + ), + ]; + + for (mut source, expected) in cases { + if expected == GitCoordinateValidationError::Repository { + source.repo = "not/a/github/slug".to_string(); + } + let error = Automation::from_replace( + AutomationId::new("nightly").unwrap(), + replace_with_source(Some(source)), + ) + .unwrap_err(); + let AutomationStoreError::Validation { source } = error else { + panic!("expected validation error"); + }; + assert!(matches!( + source, + AutomationValidationError::InvalidWorkflowSource { source } + if source == expected + )); + } + } + #[test] fn persisted_toml_applies_defaults_and_canonicalizes_without_id_or_revision() { let bytes = br#" @@ -530,12 +736,13 @@ enabled = true fn enabled_schedule_triggers_returns_only_enabled_schedule_triggers() { let (automation, _) = Automation::from_replace(AutomationId::new("nightly").unwrap(), AutomationReplace { - name: "Nightly".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: ".fabro/workflows/test/workflow.toml".to_string(), - triggers: vec![ + name: "Nightly".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: ".fabro/workflows/test/workflow.toml".to_string(), + workflow_source: None, + triggers: vec![ api_trigger("manual"), schedule_trigger_with_enabled("nightly", "0 0 * * *", true), schedule_trigger_with_enabled("disabled", "0 1 * * *", false), @@ -581,81 +788,89 @@ enabled = true fn validation_rejects_invalid_inputs() { let cases = [ AutomationReplace { - name: " ".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![api_trigger("manual")], + name: " ".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![api_trigger("manual")], }, AutomationReplace { - name: "Bad repo".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: RunTarget::Git(GitRunTarget { + name: "Bad repo".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: RunTarget::Git(GitRunTarget { repo: "not/github/slug".to_string(), branch: "main".to_string(), tag: None, sha: None, }), - workflow: "release".to_string(), - triggers: vec![api_trigger("manual")], + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![api_trigger("manual")], }, AutomationReplace { - name: "Bad ref".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: RunTarget::Git(GitRunTarget { + name: "Bad ref".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "main;rm".to_string(), tag: None, sha: None, }), - workflow: "release".to_string(), - triggers: vec![api_trigger("manual")], + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![api_trigger("manual")], }, AutomationReplace { - name: "Bad workflow".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "../release".to_string(), - triggers: vec![api_trigger("manual")], + name: "Bad workflow".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "../release".to_string(), + workflow_source: None, + triggers: vec![api_trigger("manual")], }, AutomationReplace { - name: "Duplicate trigger".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![ + name: "Duplicate trigger".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![ api_trigger("manual"), schedule_trigger("manual", "0 0 * * *"), ], }, AutomationReplace { - name: "Two API triggers".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![api_trigger("one"), api_trigger("two")], + name: "Two API triggers".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![api_trigger("one"), api_trigger("two")], }, AutomationReplace { - name: "Six field cron".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![schedule_trigger("nightly", "0 0 0 * * *")], + name: "Six field cron".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![schedule_trigger("nightly", "0 0 0 * * *")], }, AutomationReplace { - name: "Bad cron".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![schedule_trigger("nightly", "99 0 * * *")], + name: "Bad cron".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![schedule_trigger("nightly", "99 0 * * *")], }, ]; diff --git a/lib/components/fabro-automation/src/store.rs b/lib/components/fabro-automation/src/store.rs index 38c676e5e..07bb8dcdf 100644 --- a/lib/components/fabro-automation/src/store.rs +++ b/lib/components/fabro-automation/src/store.rs @@ -6,8 +6,9 @@ use sqlx::sqlite::SqliteRow; use sqlx::{Row as _, Sqlite, Transaction}; use crate::{ - ApiTrigger, Automation, AutomationDraft, 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. @@ -28,6 +29,10 @@ macro_rules! select_automations_sql { a.target_tag, a.target_sha, a.target_workflow, + a.workflow_source_repository, + 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 @@ -125,6 +130,7 @@ impl AutomationStore { ) -> Result { let (automation, _) = Automation::from_replace(id.clone(), draft)?; let target = stored_git_target(&automation); + let workflow_source = automation.workflow_source.as_ref(); let mut transaction = self.pool.begin().await?; let result = sqlx::query( r" @@ -139,7 +145,11 @@ impl AutomationStore { target_branch = ?, target_tag = ?, target_sha = ?, - target_workflow = ? + target_workflow = ?, + workflow_source_repository = ?, + workflow_source_branch = ?, + workflow_source_tag = ?, + workflow_source_sha = ? WHERE id = ? AND revision = ? ", ) @@ -153,6 +163,10 @@ impl AutomationStore { .bind(target.tag.as_deref()) .bind(target.sha.as_deref()) .bind(&automation.workflow) + .bind(workflow_source.map(|source| source.repo.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) @@ -199,6 +213,7 @@ struct StoredAutomation { api_enabled: bool, target: RunTarget, workflow: String, + workflow_source: Option, schedule_triggers: Vec, } @@ -216,6 +231,7 @@ impl StoredAutomation { id: id.clone(), source, })?; + let workflow_source = stored_workflow_source(row, &id)?; Ok(Self { id, revision, @@ -231,6 +247,7 @@ impl StoredAutomation { sha: row.try_get("target_sha")?, }), workflow: row.try_get("target_workflow")?, + workflow_source, schedule_triggers: Vec::new(), }) } @@ -279,6 +296,7 @@ impl StoredAutomation { environment_id: self.environment_id, target: self.target, workflow: self.workflow, + workflow_source: self.workflow_source, triggers, }) .map_err(|source| AutomationStoreError::StoredValidation { id, source })?; @@ -324,6 +342,7 @@ pub(crate) async fn insert_automation_ignoring_conflict( automation: &Automation, ) -> Result { let target = stored_git_target(automation); + let workflow_source = automation.workflow_source.as_ref(); let result = sqlx::query( r" INSERT INTO automations ( @@ -337,8 +356,12 @@ pub(crate) async fn insert_automation_ignoring_conflict( target_branch, target_tag, target_sha, - target_workflow - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + target_workflow, + workflow_source_repository, + workflow_source_branch, + workflow_source_tag, + workflow_source_sha + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO NOTHING ", ) @@ -353,6 +376,10 @@ pub(crate) async fn insert_automation_ignoring_conflict( .bind(target.tag.as_deref()) .bind(target.sha.as_deref()) .bind(&automation.workflow) + .bind(workflow_source.map(|source| source.repo.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 { @@ -362,6 +389,26 @@ pub(crate) async fn insert_automation_ignoring_conflict( Ok(true) } +fn stored_workflow_source( + row: &SqliteRow, + id: &AutomationId, +) -> Result, AutomationStoreError> { + let repository = row.try_get::, _>("workflow_source_repository")?; + 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() }), + } +} + fn stored_git_target(automation: &Automation) -> &GitRunTarget { automation .git_target() diff --git a/lib/components/fabro-automation/tests/store.rs b/lib/components/fabro-automation/tests/store.rs index 0c025d6e9..993494cf0 100644 --- a/lib/components/fabro-automation/tests/store.rs +++ b/lib/components/fabro-automation/tests/store.rs @@ -6,11 +6,13 @@ use std::path::Path; use fabro_automation::{ - ApiTrigger, AutomationDraft, 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}; +use sqlx::Row as _; use tokio::fs; async fn test_database() -> (tempfile::TempDir, Database) { @@ -40,15 +42,29 @@ fn schedule(id: &str, expression: &str, enabled: bool) -> AutomationTrigger { }) } +fn workflow_source( + branch: &str, + tag: Option<&str>, + sha: Option<&str>, +) -> AutomationGitWorkflowSource { + AutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: branch.to_string(), + tag: tag.map(str::to_string), + sha: sha.map(str::to_string), + } +} + fn draft(id: &str, api_enabled: bool) -> AutomationDraft { AutomationDraft { - id: AutomationId::new(id).unwrap(), - name: "Nightly".to_string(), - description: Some("Runs every night".to_string()), - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![ + id: AutomationId::new(id).unwrap(), + name: "Nightly".to_string(), + description: Some("Runs every night".to_string()), + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![ schedule("z-last", "0 2 * * *", false), AutomationTrigger::Api(ApiTrigger { id: AutomationTriggerId::new("custom-api-id").unwrap(), @@ -61,12 +77,13 @@ fn draft(id: &str, api_enabled: bool) -> AutomationDraft { fn replacement(name: &str, expression: &str) -> AutomationReplace { AutomationReplace { - name: name.to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![ + name: name.to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![ schedule("nightly", expression, true), AutomationTrigger::Api(ApiTrigger { id: AutomationTriggerId::new("api").unwrap(), @@ -268,6 +285,113 @@ async fn insert_environment(pool: &fabro_db::DbPool, id: &str, provider: &str) { .unwrap(); } +#[tokio::test] +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("main", None, None), + workflow_source("main", Some("release/v1"), None), + workflow_source( + "context-only", + Some("release/v1"), + Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01"), + ), + ] + .into_iter() + .enumerate() + { + let id = format!("source-{index}"); + let mut value = draft(&id, true); + value.workflow_source = Some(source); + let created = store.create(value).await.unwrap(); + assert_eq!( + 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())); + + let mut cleared = replacement("Cleared", "30 4 * * *"); + cleared.workflow_source = None; + let replaced = store + .replace(&created.id, &created.revision, cleared) + .await + .unwrap(); + assert_eq!(replaced.workflow_source, None); + let columns = sqlx::query( + "SELECT workflow_source_repository, workflow_source_branch, workflow_source_tag, \ + workflow_source_sha \ + FROM automations WHERE id = ?", + ) + .bind(created.id.as_str()) + .fetch_one(database.pool()) + .await + .unwrap(); + assert_eq!( + columns.get::, _>("workflow_source_repository"), + None + ); + assert_eq!( + columns.get::, _>("workflow_source_branch"), + None + ); + assert_eq!( + columns.get::, _>("workflow_source_tag"), + None + ); + assert_eq!( + columns.get::, _>("workflow_source_sha"), + None + ); + } + + assert_eq!(store.list().await.unwrap().len(), 3); +} + +#[tokio::test] +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 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") + .execute(&mut *connection) + .await + .unwrap(); + sqlx::query("PRAGMA ignore_check_constraints = ON") + .execute(&mut *connection) + .await + .unwrap(); + sqlx::query( + "UPDATE automations SET workflow_source_repository = 'fabro-sh/workflows' WHERE id = ?", + ) + .bind(partial.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!( + store.get(&partial.id).await.unwrap_err(), + AutomationStoreError::StoredWorkflowSourceShape { .. } + )); + assert!(matches!( + store.get(&orphan.id).await.unwrap_err(), + AutomationStoreError::StoredWorkflowSourceShape { .. } + )); +} + #[tokio::test] async fn disabled_api_trigger_normalizes_to_absent() { let (_dir, database) = test_database().await; @@ -375,12 +499,13 @@ async fn failed_schedule_insert_rolls_back_parent_replace() { .await .unwrap(); let replacement = AutomationReplace { - name: "Should roll back".to_string(), - description: None, - environment_id: Some("default".to_string()), - target: target(), - workflow: "release".to_string(), - triggers: vec![schedule("blocked", "0 7 * * *", true)], + name: "Should roll back".to_string(), + description: None, + environment_id: Some("default".to_string()), + target: target(), + workflow: "release".to_string(), + workflow_source: None, + triggers: vec![schedule("blocked", "0 7 * * *", true)], }; let err = store @@ -449,6 +574,7 @@ async fn legacy_import_is_transactional_and_sql_wins() { assert_eq!(imported.name, "Imported"); assert_eq!(imported.revision, expected_revision); assert_eq!(imported.workflow, "release"); + assert_eq!(imported.workflow_source, None); assert!(matches!( imported.target, RunTarget::Git(GitRunTarget { diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 94592a59a..76ed229eb 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -9,6 +9,8 @@ use fabro_types::settings::run::MergeStrategy; use serde::Deserialize; use tokio::process::Command; +use crate::token_source::SecretString; + pub mod access; pub mod token_source; @@ -1266,42 +1268,119 @@ pub async fn update_app_webhook_config( Ok(()) } -/// Resolve git clone credentials for a GitHub repository. +/// Required credentials for an authenticated GitHub HTTPS clone. /// -/// Returns `(username, password)` for authenticated cloning and pushing. -/// Always generates a token regardless of repo visibility, since the token -/// is needed for pushing from the sandbox. +/// The password is redacted from `Debug` output and should only be exposed at +/// the point where it is passed to Git. +#[derive(Clone, Debug)] +pub struct GitCloneCredentials { + username: String, + password: SecretString, +} + +impl GitCloneCredentials { + fn from_token(token: String) -> anyhow::Result { + if token.is_empty() { + bail!("GitHub clone credential token is empty"); + } + Ok(Self { + username: "x-access-token".to_string(), + password: SecretString::new(token), + }) + } + + /// The username passed to Git's HTTPS basic authentication. + #[must_use] + pub fn username(&self) -> &str { + &self.username + } + + /// The secret passed to Git's HTTPS basic authentication. + /// + /// Callers must not log or persist the returned value. + #[must_use] + pub fn password(&self) -> &str { + self.password.expose() + } +} + +/// Resolve Git clone credentials for a GitHub repository. +/// +/// Always generates credentials regardless of repository visibility because +/// the token is needed for pushing from the sandbox. +/// +/// # Errors +/// +/// Returns an error when an installation token is expired, a GitHub App token +/// cannot be minted, the HTTP client cannot be created, or the resolved token +/// is empty. pub async fn resolve_clone_credentials( ctx: &GitHubContext<'_>, owner: &str, repo: &str, -) -> anyhow::Result<(Option, Option)> { +) -> anyhow::Result { let token = match ctx.creds { GitHubCredentials::Pat(token) => token.clone(), GitHubCredentials::Installation(token) => token.valid_token()?.to_string(), GitHubCredentials::App(_) => { let client = ctx.http_client()?; - mint_git_contents_write_token(&client, ctx, owner, repo).await? + mint_git_token( + &client, + ctx, + owner, + repo, + serde_json::json!({ "contents": "write" }), + ) + .await? } }; - Ok((Some("x-access-token".to_string()), Some(token))) + GitCloneCredentials::from_token(token) } -/// Mint an installation token scoped to repository contents writes. -async fn mint_git_contents_write_token( +/// Resolve credentials for fetching repository contents without granting a +/// GitHub App token permission to push. +/// +/// Static PATs and pre-minted installation tokens retain their configured +/// permissions. App credentials mint a repository-scoped token with +/// `contents: read`. +/// +/// # Errors +/// +/// Returns an error when an installation token is expired, a GitHub App token +/// cannot be minted, the HTTP client cannot be created, or the resolved token +/// is empty. +pub async fn resolve_read_only_clone_credentials( + ctx: &GitHubContext<'_>, + owner: &str, + repo: &str, +) -> anyhow::Result { + let token = match ctx.creds { + GitHubCredentials::Pat(token) => token.clone(), + GitHubCredentials::Installation(token) => token.valid_token()?.to_string(), + GitHubCredentials::App(_) => { + let client = ctx.http_client()?; + mint_git_token( + &client, + ctx, + owner, + repo, + serde_json::json!({ "contents": "read" }), + ) + .await? + } + }; + GitCloneCredentials::from_token(token) +} + +async fn mint_git_token( client: &impl HttpClient, ctx: &GitHubContext<'_>, owner: &str, repo: &str, + permissions: serde_json::Value, ) -> anyhow::Result { ctx.creds - .resolve_bearer_token( - client, - owner, - repo, - ctx.base_url, - serde_json::json!({ "contents": "write" }), - ) + .resolve_bearer_token(client, owner, repo, ctx.base_url, permissions) .await } @@ -1330,11 +1409,8 @@ pub async fn resolve_authenticated_url( url: &str, ) -> anyhow::Result { let (owner, repo) = parse_github_owner_repo(url)?; - let (_username, password) = resolve_clone_credentials(ctx, &owner, &repo).await?; - match password { - Some(token) => embed_token_in_url(url, &token), - None => DisplaySafeUrl::parse(url).context("Failed to parse GitHub HTTPS URL"), - } + let credentials = resolve_clone_credentials(ctx, &owner, &repo).await?; + embed_token_in_url(url, credentials.password()) } /// Fetch detailed information about a pull request. @@ -2538,13 +2614,22 @@ mod tests { .await .unwrap(); - assert_eq!( - credentials, - ( - Some("x-access-token".to_string()), - Some("ghu_test".to_string()) - ) - ); + assert_eq!(credentials.username(), "x-access-token"); + assert_eq!(credentials.password(), "ghu_test"); + assert!(!format!("{credentials:?}").contains("ghu_test")); + } + + #[tokio::test] + async fn resolve_read_only_clone_credentials_returns_static_token_unchanged() { + let creds = GitHubCredentials::Pat("ghu_test".to_string()); + + let credentials = + resolve_read_only_clone_credentials(&GitHubContext::new(&creds, ""), "owner", "repo") + .await + .unwrap(); + + assert_eq!(credentials.username(), "x-access-token"); + assert_eq!(credentials.password(), "ghu_test"); } #[tokio::test] @@ -2569,9 +2654,50 @@ mod tests { slug: None, }); let context = GitHubContext::new(&credentials, ""); - let token = mint_git_contents_write_token(&mock, &context, "owner", "repo") - .await - .unwrap(); + let token = mint_git_token( + &mock, + &context, + "owner", + "repo", + serde_json::json!({ "contents": "write" }), + ) + .await + .unwrap(); + + assert_eq!(token, "ghs_xxx"); + } + + #[tokio::test] + async fn read_only_clone_token_requests_only_contents_read() { + let mock = MockHttpClient::new() + .on( + HttpMethod::Get, + "/repos/owner/repo/installation", + 200, + r#"{"id": 123}"#, + ) + .on( + HttpMethod::Post, + "/app/installations/123/access_tokens", + 201, + r#"{"token": "ghs_xxx", "expires_at": "2099-01-01T00:00:00Z"}"#, + ) + .with_req_body(r#"{"permissions":{"contents":"read"},"repositories":["repo"]}"#); + let credentials = GitHubCredentials::App(GitHubAppCredentials { + app_id: "test".to_string(), + private_key_pem: test_rsa_key().to_string(), + slug: None, + }); + let context = GitHubContext::new(&credentials, ""); + let token = mint_git_token( + &mock, + &context, + "owner", + "repo", + serde_json::json!({ "contents": "read" }), + ) + .await + .unwrap(); assert_eq!(token, "ghs_xxx"); } diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs index 079c01fc2..37068764b 100644 --- a/lib/components/fabro-store/src/run_state.rs +++ b/lib/components/fabro-store/src/run_state.rs @@ -2473,9 +2473,10 @@ mod tests { #[test] fn run_created_projects_automation_into_spec_and_summary() { let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: None, }; let event = test_raw_event( 1, diff --git a/lib/components/fabro-store/src/run_summary_store.rs b/lib/components/fabro-store/src/run_summary_store.rs index 4750f1e21..d180213d3 100644 --- a/lib/components/fabro-store/src/run_summary_store.rs +++ b/lib/components/fabro-store/src/run_summary_store.rs @@ -2162,15 +2162,17 @@ mod tests { let mut first = projection(first_id, "bravo", created_at); first.spec.automation = Some(AutomationRef { - id: "nightly".to_string(), - name: None, - trigger_id: None, + id: "nightly".to_string(), + name: None, + trigger_id: None, + workflow_source: None, }); let mut second = projection(second_id, "alpha", created_at); second.spec.automation = Some(AutomationRef { - id: "nightly".to_string(), - name: None, - trigger_id: None, + id: "nightly".to_string(), + name: None, + trigger_id: None, + workflow_source: None, }); let mut archived = projection(archived_id, "charlie", created_at); archived.archived_at = Some(created_at); @@ -2219,9 +2221,10 @@ mod tests { let run_id = run_id(created_at.timestamp_millis().cast_unsigned(), 1); let mut projection = projection(run_id, "billed", created_at); projection.spec.automation = Some(AutomationRef { - id: "nightly".to_string(), - name: None, - trigger_id: None, + id: "nightly".to_string(), + name: None, + trigger_id: None, + workflow_source: None, }); projection.status = RunStatus::Succeeded { reason: SuccessReason::Completed, diff --git a/lib/components/fabro-workflow/src/event/convert.rs b/lib/components/fabro-workflow/src/event/convert.rs index 34db0184b..f89d53bf6 100644 --- a/lib/components/fabro-workflow/src/event/convert.rs +++ b/lib/components/fabro-workflow/src/event/convert.rs @@ -2850,9 +2850,10 @@ mod tests { subject: user_principal("alice"), }; let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: None, }; let workflow_version_id = test_support::test_workflow_version_id(); diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 8cfcdafec..bcb3087d5 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -1644,9 +1644,10 @@ reasoning = false let dir = tempfile::tempdir().unwrap(); let storage_root = dir.path().join("storage"); let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: None, }; let request = CreateRunInput { workflow: WorkflowInput::DotSource { @@ -1790,9 +1791,10 @@ reasoning = false let compiled_source = MINIMAL_DOT.replace("Build feature", "Compiled goal"); std::fs::write(&dot_path, &compiled_source).unwrap(); let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: None, }; let request = CreateRunInput { workflow: WorkflowInput::Path(dot_path.clone()), @@ -2493,9 +2495,10 @@ reasoning = false None, )); let automation = fabro_types::AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: None, }; let created = create( store.as_ref(), diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index 1c00421ce..f5831371a 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -1198,7 +1198,7 @@ async fn daytona_clone_public_repo_gets_credentials() { // Directly test resolve_clone_credentials against a repo in an org where the // app is installed - let (username, password) = fabro_github::resolve_clone_credentials( + let credentials = fabro_github::resolve_clone_credentials( &fabro_github::GitHubContext::new(&creds, &fabro_github::github_api_base_url()), "fabro-sh", "fabro", @@ -1207,12 +1207,12 @@ async fn daytona_clone_public_repo_gets_credentials() { .unwrap(); assert_eq!( - username.as_deref(), - Some("x-access-token"), + credentials.username(), + "x-access-token", "installed org repo should get credentials for pushing" ); assert!( - password.is_some(), + !credentials.password().is_empty(), "installed org repo should get a token for pushing" ); } diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index ef3d79c07..b5b090aa2 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -688,6 +688,11 @@ fn main() { ("SandboxTimestamps", "fabro_types::SandboxTimestamps", &[]), ("AskFabro", "fabro_types::AskFabro", &[]), ("Automation", "fabro_automation::Automation", &[]), + ( + "AutomationGitWorkflowSource", + "fabro_types::GitRunTarget", + &[], + ), ("AutomationRef", "fabro_types::AutomationRef", &[]), ( "AutomationTrigger", diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index af8454e7e..9a012aebd 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -48,27 +48,28 @@ pub mod types { AgentToolsAvailableProps, AskFabro, AuthMethod, 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, + 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 8555c535c..4d46df63f 100644 --- a/lib/foundation/fabro-api/tests/automation_round_trip.rs +++ b/lib/foundation/fabro-api/tests/automation_round_trip.rs @@ -1,9 +1,12 @@ use fabro_api::types::{ - Automation as ApiAutomation, AutomationTrigger as ApiAutomationTrigger, + Automation as ApiAutomation, AutomationGitWorkflowSource as ApiAutomationGitWorkflowSource, + AutomationTrigger as ApiAutomationTrigger, CreateAutomationRequest as ApiCreateAutomationRequest, ReplaceAutomationRequest as ApiReplaceAutomationRequest, }; -use fabro_automation::{Automation, AutomationDraft, AutomationReplace, AutomationTrigger}; +use fabro_automation::{ + Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationReplace, AutomationTrigger, +}; use serde_json::json; // Compile-time witnesses that the generated API types resolve to the same @@ -12,6 +15,7 @@ use serde_json::json; // checking and the build fails. const _: fn(ApiAutomation) -> Automation = |value| value; const _: fn(ApiAutomationTrigger) -> AutomationTrigger = |value| value; +const _: fn(ApiAutomationGitWorkflowSource) -> AutomationGitWorkflowSource = |value| value; const _: fn(ApiCreateAutomationRequest) -> AutomationDraft = |value| value; const _: fn(ApiReplaceAutomationRequest) -> AutomationReplace = |value| value; @@ -103,3 +107,57 @@ fn replace_automation_request_round_trips_public_json_shape() { let api: ApiReplaceAutomationRequest = serde_json::from_value(value.clone()).unwrap(); assert_eq!(serde_json::to_value(api).unwrap(), value); } + +#[test] +fn automation_workflow_sources_round_trip_each_public_json_shape() { + 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", + "name": "Nightly dependency update", + "environment_id": "daytona-smoke", + "target": { + "kind": "git", + "repo": "fabro-sh/app", + "branch": "main" + }, + "workflow": "dependency-update", + "workflow_source": source, + "triggers": [] + }); + + let api: ApiCreateAutomationRequest = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(serde_json::to_value(api).unwrap(), value); + } +} + +#[test] +fn automation_workflow_source_rejects_unknown_or_incomplete_coordinates() { + for source in [ + 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", + "branch": "main", + "sha": "short" + })) + .unwrap(); + assert!(fabro_automation::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 031cc6a5e..76280e5a4 100644 --- a/lib/foundation/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_summary_round_trip.rs @@ -10,9 +10,9 @@ use fabro_api::types::{ use fabro_types::status::{RunStatus, SuccessReason}; use fabro_types::{ AskFabro, AskFabroUnavailableReason, AutomationRef, DiffSummary, PullRequestLink, - RepositoryProvider, RepositoryRef, Run, RunApproval, RunApprovalState, RunBillingSummary, - RunId, RunLifecycle, RunLinks, RunOrigin, RunRunnableSource, RunSize, RunTimestamps, RunTiming, - WorkflowRef, fixtures, test_support, + RepositoryProvider, RepositoryRef, ResolvedAutomationGitWorkflowSource, Run, RunApproval, + RunApprovalState, RunBillingSummary, RunId, RunLifecycle, RunLinks, RunOrigin, + RunRunnableSource, RunSize, RunTimestamps, RunTiming, WorkflowRef, fixtures, test_support, }; use serde_json::json; @@ -79,9 +79,16 @@ fn run_summary_json_matches_openapi_shape() { edge_count: 9, }, automation: Some(AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: Some(Box::new(ResolvedAutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: "context-only".to_string(), + tag: Some("v1".to_string()), + sha: Some("0123456789abcdef0123456789abcdef01234567".to_string()), + resolved_sha: "0123456789abcdef0123456789abcdef01234567".to_string(), + })), }), repository: Some(RepositoryRef { name: "fabro".to_string(), @@ -154,7 +161,14 @@ fn run_summary_json_matches_openapi_shape() { "automation": { "id": "nightly", "name": "Nightly", - "trigger_id": "schedule_1" + "trigger_id": "schedule_1", + "workflow_source": { + "repo": "fabro-sh/workflows", + "branch": "context-only", + "tag": "v1", + "sha": "0123456789abcdef0123456789abcdef01234567", + "resolved_sha": "0123456789abcdef0123456789abcdef01234567" + } }, "repository": { "name": "fabro", diff --git a/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql b/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql new file mode 100644 index 000000000..2786455a8 --- /dev/null +++ b/lib/foundation/fabro-db/migrations/2026082803_automation_workflow_sources.sql @@ -0,0 +1,57 @@ +ALTER TABLE automations ADD COLUMN workflow_source_repository TEXT + CHECK ( + workflow_source_repository IS NULL + OR length(workflow_source_repository) BETWEEN 3 AND 140 + ); + +ALTER TABLE automations ADD COLUMN workflow_source_branch TEXT + CHECK ( + workflow_source_branch IS NULL + OR length(workflow_source_branch) BETWEEN 1 AND 255 + ); + +ALTER TABLE automations ADD COLUMN workflow_source_tag TEXT + CHECK ( + 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_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 requires repository and branch together'); +END; + +CREATE TRIGGER automation_workflow_source_all_or_none_update +BEFORE UPDATE OF + workflow_source_repository, + workflow_source_branch, + workflow_source_tag, + workflow_source_sha +ON automations +WHEN + (NEW.workflow_source_repository IS NULL) + + (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 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 ec197ee84..33f0a697d 100644 --- a/lib/foundation/fabro-db/tests/sqlite.rs +++ b/lib/foundation/fabro-db/tests/sqlite.rs @@ -388,6 +388,41 @@ async fn automations_schema_enforces_aggregate_constraints() -> anyhow::Result<( .await .is_err() ); + 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_branch = ?, workflow_source_tag = ?, workflow_source_sha = ? \ + WHERE id = 'valid'", + ) + .bind(repository) + .bind(branch) + .bind(tag) + .bind(sha) + .execute(database.pool()) + .await; + assert!( + result.is_err(), + "invalid workflow source row should be rejected" + ); + } + + sqlx::query( + "UPDATE automations SET workflow_source_repository = 'fabro-sh/workflows', \ + workflow_source_branch = 'main', workflow_source_tag = 'v1', \ + workflow_source_sha = '0123456789abcdef0123456789abcdef01234567' WHERE id = 'valid'", + ) + .execute(database.pool()) + .await?; assert!( sqlx::query( "INSERT INTO automation_triggers (automation_id, id, enabled, expression) \ @@ -431,6 +466,92 @@ async fn automations_schema_enforces_aggregate_constraints() -> anyhow::Result<( Ok(()) } +#[tokio::test] +async fn automation_workflow_sources_migrate_without_rewriting_existing_rows() -> anyhow::Result<()> +{ + let dir = tempfile::tempdir()?; + let db_path = dir.path().join("fabro.sqlite3"); + let database = fabro_db::Database::connect(&db_path).await?; + database.migrate().await?; + rewind_automation_workflow_source_migration(&database).await?; + + insert_minimal_automation(database.pool(), "preserved", 1).await?; + sqlx::query( + "INSERT INTO automation_triggers (automation_id, id, enabled, expression) \ + VALUES ('preserved', 'nightly', 1, '0 3 * * *')", + ) + .execute(database.pool()) + .await?; + + database.migrate().await?; + + let row = sqlx::query( + "SELECT id, revision, target_repository, target_branch, target_tag, target_sha, \ + target_workflow, workflow_source_repository, workflow_source_branch, \ + workflow_source_tag, workflow_source_sha \ + FROM automations WHERE id = 'preserved'", + ) + .fetch_one(database.pool()) + .await?; + assert_eq!(row.get::("id"), "preserved"); + assert_eq!(row.get::("revision"), "a".repeat(64)); + assert_eq!(row.get::("target_repository"), "fabro-sh/fabro"); + assert_eq!(row.get::("target_branch"), "main"); + assert_eq!(row.get::, _>("target_tag"), None); + assert_eq!(row.get::, _>("target_sha"), None); + assert_eq!(row.get::("target_workflow"), "release"); + assert_eq!( + row.get::, _>("workflow_source_repository"), + 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'", + ) + .fetch_one(database.pool()) + .await?; + assert_eq!(trigger_count, 1); + assert!(fabro_db::pre_migration_snapshot_path(&db_path).exists()); + + database.migrate().await?; + assert_eq!( + sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM automations WHERE id = 'preserved'") + .fetch_one(database.pool()) + .await?, + 1 + ); + Ok(()) +} + +async fn rewind_automation_workflow_source_migration( + database: &fabro_db::Database, +) -> anyhow::Result<()> { + sqlx::query("DROP TRIGGER automation_workflow_source_all_or_none_update") + .execute(database.pool()) + .await?; + sqlx::query("DROP TRIGGER automation_workflow_source_all_or_none_insert") + .execute(database.pool()) + .await?; + sqlx::query("ALTER TABLE automations DROP COLUMN workflow_source_sha") + .execute(database.pool()) + .await?; + 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") + .execute(database.pool()) + .await?; + sqlx::query("DELETE FROM _sqlx_migrations WHERE version = 2026082803") + .execute(database.pool()) + .await?; + Ok(()) +} + async fn insert_minimal_automation( pool: &fabro_db::DbPool, id: &str, diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index 9848a3560..e412be73b 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -114,7 +114,8 @@ pub use pull_request::{ }; pub use reasoning::ReasoningOutput; pub use repository::{ - GitHubRepositorySlug, RepositoryProvider, RepositoryRef, 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, @@ -131,7 +132,8 @@ pub use run_event::{ pub use run_failure::RunFailure; pub use run_id::{RunId, fixtures}; pub use run_intent::{ - GitRunTarget, RunIntent, RunIntentArgs, RunTarget, TargetValidationError, ValidatedRunTarget, + GitCoordinateValidationError, GitRunTarget, RunIntent, RunIntentArgs, RunTarget, + TargetValidationError, ValidatedGitRunTarget, ValidatedRunTarget, }; pub use run_projection::{ ActivatedSkill, AgentControlState, CheckpointRecord, McpServerProjection, McpServerStatus, @@ -146,9 +148,9 @@ pub use run_sandbox::{ RunSandboxRuntime, }; pub use run_summary::{ - AskFabro, AskFabroUnavailableReason, AutomationRef, Run, RunApproval, RunApprovalState, - RunBillingSummary, RunError, RunLifecycle, RunLinks, RunModel, RunOrigin, RunOriginKind, - RunSize, RunTimestamps, WorkflowRef, + AskFabro, AskFabroUnavailableReason, AutomationRef, ResolvedAutomationGitWorkflowSource, Run, + RunApproval, RunApprovalState, RunBillingSummary, RunError, RunLifecycle, RunLinks, RunModel, + RunOrigin, RunOriginKind, RunSize, RunTimestamps, WorkflowRef, }; pub use run_title::{ MAX_RUN_TITLE_CHARS, RunTitleError, infer_run_title, normalize_explicit_run_title, diff --git a/lib/foundation/fabro-types/src/repository.rs b/lib/foundation/fabro-types/src/repository.rs index 7eb431b92..b0e69f3e5 100644 --- a/lib/foundation/fabro-types/src/repository.rs +++ b/lib/foundation/fabro-types/src/repository.rs @@ -212,6 +212,33 @@ pub fn is_valid_github_ref_selector(value: &str) -> bool { .all(|part| !part.is_empty() && !part.starts_with('.') && !has_lock_suffix(part)) } +/// Reports whether `value` is a canonical bare Git branch name. +/// +/// Bare names exclude symbolic selectors, fully qualified refs, tag-prefixed +/// selectors, commit SHAs, and the `heads/` prefix accepted only after Git has +/// already entered the refs namespace. +#[must_use] +pub fn is_valid_git_branch_name(value: &str) -> bool { + is_valid_bare_git_ref_name(value) && !value.starts_with("heads/") +} + +/// Reports whether `value` is a canonical bare Git tag name. +/// +/// The input is a tag name rather than a selector, so `tags/`, `refs/`, +/// symbolic `HEAD`, and exact commit SHAs are rejected. +#[must_use] +pub fn is_valid_git_tag_name(value: &str) -> bool { + is_valid_bare_git_ref_name(value) +} + +fn is_valid_bare_git_ref_name(value: &str) -> bool { + value != "HEAD" + && !value.starts_with("tags/") + && !value.starts_with("refs/") + && normalize_git_commit_sha(value).is_none() + && is_valid_github_ref_selector(value) +} + /// Validates and canonicalizes an exact Git commit SHA. /// /// The grammar accepts exactly 40 untrimmed ASCII hexadecimal bytes. It does diff --git a/lib/foundation/fabro-types/src/run_intent.rs b/lib/foundation/fabro-types/src/run_intent.rs index 3aaa545e3..ba4c472f7 100644 --- a/lib/foundation/fabro-types/src/run_intent.rs +++ b/lib/foundation/fabro-types/src/run_intent.rs @@ -72,18 +72,54 @@ pub struct GitRunTarget { pub sha: Option, } -/// A bare branch or tag name: not `HEAD`, not a `refs/` or `tags/` selector, -/// not a commit SHA, and otherwise a valid GitHub ref selector. -/// -/// The selector grammar is checked on the bare name so its leading-character -/// rules apply to the name itself, not to a prefixed selector that would mask -/// them. -fn is_bare_ref_name(name: &str) -> bool { - name != "HEAD" - && !name.starts_with("tags/") - && !name.starts_with("refs/") - && repository::normalize_git_commit_sha(name).is_none() - && repository::is_valid_github_ref_selector(name) +impl GitRunTarget { + /// Validates and canonicalizes this Git coordinate without resolving remote + /// repository state. + /// + /// # Errors + /// + /// Returns an error when the repository slug, branch, tag, or exact commit + /// does not use the canonical grammar accepted for Git-backed runs. + pub fn validate(self) -> Result { + let Self { + repo, + branch, + tag, + sha, + } = self; + let repository = + GitHubRepositorySlug::try_new(&repo).ok_or(GitCoordinateValidationError::Repository)?; + if !repository::is_valid_git_branch_name(&branch) { + return Err(GitCoordinateValidationError::Branch); + } + if tag + .as_deref() + .is_some_and(|tag| !repository::is_valid_git_tag_name(tag)) + { + return Err(GitCoordinateValidationError::Tag); + } + let sha = sha + .map(|sha| { + repository::normalize_git_commit_sha(&sha).ok_or(GitCoordinateValidationError::Sha) + }) + .transpose()?; + let git = GitContext { + origin_url: repository.https_url(), + branch: branch.clone(), + sha: sha.clone(), + dirty: DirtyStatus::Clean, + }; + Ok(ValidatedGitRunTarget { + target: Self { + repo, + branch, + tag, + sha, + }, + repository, + git, + }) + } } impl RunTarget { @@ -98,41 +134,18 @@ impl RunTarget { /// Git targets include their derived operational Git projection. Targets /// without a repository return no projection. Folder paths require /// filesystem validation and canonicalization during provider admission. + /// + /// # Errors + /// + /// Returns an error when a Git target's repository slug, branch, tag, or + /// exact commit does not use the canonical grammar accepted for runs. pub fn validate(self) -> Result { match self { - Self::Git(GitRunTarget { - repo, - branch, - tag, - sha, - }) => { - let slug = GitHubRepositorySlug::try_new(&repo) - .ok_or(TargetValidationError::Repository)?; - if !is_bare_ref_name(&branch) || branch.starts_with("heads/") { - return Err(TargetValidationError::Branch); - } - if tag.as_deref().is_some_and(|tag| !is_bare_ref_name(tag)) { - return Err(TargetValidationError::Tag); - } - let sha = sha - .map(|sha| { - repository::normalize_git_commit_sha(&sha).ok_or(TargetValidationError::Sha) - }) - .transpose()?; - let git = GitContext { - origin_url: slug.https_url(), - branch: branch.clone(), - sha: sha.clone(), - dirty: DirtyStatus::Clean, - }; + Self::Git(target) => { + let validated = target.validate().map_err(TargetValidationError::from)?; Ok(ValidatedRunTarget { - target: Self::Git(GitRunTarget { - repo, - branch, - tag, - sha, - }), - git: Some(git), + target: Self::Git(validated.target), + git: Some(validated.git), }) } Self::None {} => Ok(ValidatedRunTarget { @@ -147,6 +160,34 @@ impl RunTarget { } } +/// A [`GitRunTarget`] whose local grammar has been validated and canonicalized. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ValidatedGitRunTarget { + target: GitRunTarget, + repository: GitHubRepositorySlug, + git: GitContext, +} + +impl ValidatedGitRunTarget { + /// The canonical Git target coordinate. + #[must_use] + pub fn target(&self) -> &GitRunTarget { + &self.target + } + + /// The parsed GitHub repository named by the target. + #[must_use] + pub fn repository(&self) -> &GitHubRepositorySlug { + &self.repository + } + + /// Consume the validation proof and return the canonical Git target. + #[must_use] + pub fn into_target(self) -> GitRunTarget { + self.target + } +} + /// A [`RunTarget`] whose grammar has been validated, together with its /// optional operational Git projection. #[derive(Debug, Clone, PartialEq, Eq)] @@ -155,6 +196,19 @@ pub struct ValidatedRunTarget { pub git: Option, } +/// A Git coordinate that failed local grammar validation. +#[derive(Debug, thiserror::Error, Clone, Copy, PartialEq, Eq)] +pub enum GitCoordinateValidationError { + #[error("repository must be a valid GitHub owner/name slug")] + Repository, + #[error("branch must be a non-empty branch name, not a ref or commit selector")] + Branch, + #[error("tag must be a non-empty bare tag name, not a ref or commit selector")] + Tag, + #[error("SHA must be exactly 40 ASCII hexadecimal characters")] + Sha, +} + /// A [`RunTarget`] that failed grammar validation. #[derive(Debug, thiserror::Error, Clone, Copy, PartialEq, Eq)] pub enum TargetValidationError { @@ -167,3 +221,14 @@ pub enum TargetValidationError { #[error("target SHA must be exactly 40 ASCII hexadecimal characters")] Sha, } + +impl From for TargetValidationError { + fn from(error: GitCoordinateValidationError) -> Self { + match error { + GitCoordinateValidationError::Repository => Self::Repository, + GitCoordinateValidationError::Branch => Self::Branch, + GitCoordinateValidationError::Tag => Self::Tag, + GitCoordinateValidationError::Sha => Self::Sha, + } + } +} diff --git a/lib/foundation/fabro-types/src/run_summary.rs b/lib/foundation/fabro-types/src/run_summary.rs index 0966df360..001b56e3e 100644 --- a/lib/foundation/fabro-types/src/run_summary.rs +++ b/lib/foundation/fabro-types/src/run_summary.rs @@ -4,7 +4,7 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use crate::{ - DiffSummary, InterviewQuestionRecord, Principal, PullRequestLink, RepositoryRef, + DiffSummary, GitRunTarget, InterviewQuestionRecord, Principal, PullRequestLink, RepositoryRef, RunControlAction, RunId, RunSandbox, RunStatus, RunTiming, }; @@ -113,13 +113,44 @@ impl WorkflowRef { } } +/// Requested workflow-source coordinate and the immutable commit selected for +/// one automation run. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ResolvedAutomationGitWorkflowSource { + pub repo: 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, + pub id: String, #[serde(default)] - pub name: Option, + pub name: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub trigger_id: Option, + pub trigger_id: Option, + /// Boxed because this metadata is uncommon and run specs cross many async + /// server boundaries where their inline size matters. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workflow_source: Option>, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index 0e11baca7..07dabaa3a 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -8,7 +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::{ - AutomationRef, EventBody, GitRunTarget, RunTarget, TurnId, WorkflowSettings, fixtures, + AutomationRef, EventBody, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunTarget, TurnId, + WorkflowSettings, fixtures, }; fn templated_settings() -> WorkflowSettings { @@ -35,9 +36,16 @@ fn run_created_props_round_trip_templated_settings() { sha: None, })), automation: Some(AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: Some(Box::new(ResolvedAutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: "main".to_string(), + tag: Some("v1".to_string()), + sha: None, + resolved_sha: "0123456789abcdef0123456789abcdef01234567".to_string(), + })), }), provenance: test_run_provenance(), manifest_blob: None, @@ -78,6 +86,12 @@ 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"]["branch"], "main"); + assert_eq!(json["automation"]["workflow_source"]["tag"], "v1"); + assert_eq!( + json["automation"]["workflow_source"]["resolved_sha"], + "0123456789abcdef0123456789abcdef01234567" + ); assert_eq!( json["workflow_version_id"], test_workflow_version_id().to_string() diff --git a/lib/foundation/fabro-types/tests/run_intent.rs b/lib/foundation/fabro-types/tests/run_intent.rs index 5d99b612e..a8c4b3b5e 100644 --- a/lib/foundation/fabro-types/tests/run_intent.rs +++ b/lib/foundation/fabro-types/tests/run_intent.rs @@ -217,6 +217,25 @@ fn target_validation_normalizes_sha_without_network_resolution() { ); } +#[test] +fn git_target_validation_carries_the_parsed_repository_proof() { + let validated = GitRunTarget { + repo: "Fabro-Sh/Fabro".to_string(), + branch: "feature/run-intent".to_string(), + tag: None, + sha: Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01".to_string()), + } + .validate() + .unwrap(); + + assert_eq!(validated.repository().owner(), "Fabro-Sh"); + assert_eq!(validated.repository().repo(), "Fabro"); + assert_eq!( + validated.target().sha.as_deref(), + Some("abcdef0123456789abcdef0123456789abcdef01") + ); +} + #[test] fn run_intent_none_target_validates_without_a_git_projection() { let validated = RunTarget::None {}.validate().unwrap(); diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index f052acd9a..fd2088770 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -5,7 +5,10 @@ use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext, RunSpec}; use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; -use fabro_types::{AutomationRef, GitRunTarget, RunTarget, WorkflowSettings, fixtures}; +use fabro_types::{ + AutomationRef, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunTarget, WorkflowSettings, + fixtures, +}; fn templated_settings() -> WorkflowSettings { let mut settings = WorkflowSettings::default(); @@ -29,9 +32,16 @@ fn run_spec_round_trips_templated_settings() { sha: Some("abc123".to_string()), })), automation: Some(AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + workflow_source: Some(Box::new(ResolvedAutomationGitWorkflowSource { + repo: "fabro-sh/workflows".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + resolved_sha: "0123456789abcdef0123456789abcdef01234567".to_string(), + })), }), source_directory: Some("/Users/client/project".to_string()), labels: HashMap::from([("team".to_string(), "platform".to_string())]), @@ -66,6 +76,11 @@ 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"]["branch"], "main"); + assert_eq!( + json["automation"]["workflow_source"]["resolved_sha"], + "0123456789abcdef0123456789abcdef01234567" + ); assert_eq!( json["workflow_version_id"], test_workflow_version_id().to_string() diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 242f517aa..105e4c5fe 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -60,6 +60,7 @@ models/auth-session-user.ts models/auth-session.ts models/auth-sessions-response.ts models/automation-api-trigger.ts +models/automation-git-workflow-source.ts models/automation-list-meta.ts models/automation-list-response.ts models/automation-ref.ts @@ -335,6 +336,7 @@ models/replace-mcp-server-request.ts models/repo-check-response-permissions.ts models/repo-check-response.ts models/repository-ref.ts +models/resolved-automation-git-workflow-source.ts models/review-target-kind.ts models/review-target.ts models/rewind-request.ts 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 new file mode 100644 index 000000000..d680f2688 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/automation-git-workflow-source.ts @@ -0,0 +1,37 @@ +/* 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. + */ + + + +/** + * 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; + /** + * 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. + */ + '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/automation-ref.ts b/lib/packages/fabro-api-client/src/models/automation-ref.ts index a22d99199..e73345dc6 100644 --- a/lib/packages/fabro-api-client/src/models/automation-ref.ts +++ b/lib/packages/fabro-api-client/src/models/automation-ref.ts @@ -13,9 +13,13 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { ResolvedAutomationGitWorkflowSource } from './resolved-automation-git-workflow-source'; export interface AutomationRef { 'id': string; 'name': string | null; 'trigger_id'?: string | null; + 'workflow_source'?: ResolvedAutomationGitWorkflowSource | null; } diff --git a/lib/packages/fabro-api-client/src/models/automation.ts b/lib/packages/fabro-api-client/src/models/automation.ts index e4d80ac23..8f78ed37e 100644 --- a/lib/packages/fabro-api-client/src/models/automation.ts +++ b/lib/packages/fabro-api-client/src/models/automation.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { AutomationGitWorkflowSource } from './automation-git-workflow-source'; // May contain unused imports in some cases // @ts-ignore import type { AutomationTrigger } from './automation-trigger'; @@ -41,8 +44,9 @@ export interface Automation { 'last_error': string | null; 'target': RunTarget; /** - * Workflow slug or path resolved in the selected repository checkout. + * Workflow slug or path resolved in the run-target checkout when `workflow_source` is omitted, or in the explicit workflow-source checkout when present. */ 'workflow': string; + 'workflow_source'?: AutomationGitWorkflowSource; 'triggers': Array; } diff --git a/lib/packages/fabro-api-client/src/models/create-automation-request.ts b/lib/packages/fabro-api-client/src/models/create-automation-request.ts index 0c3e23509..d875be5f1 100644 --- a/lib/packages/fabro-api-client/src/models/create-automation-request.ts +++ b/lib/packages/fabro-api-client/src/models/create-automation-request.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { AutomationGitWorkflowSource } from './automation-git-workflow-source'; // May contain unused imports in some cases // @ts-ignore import type { AutomationTrigger } from './automation-trigger'; @@ -33,8 +36,9 @@ export interface CreateAutomationRequest { 'environment_id': string; 'target': RunTarget; /** - * Workflow slug or path resolved in the selected repository checkout. + * Workflow slug or path resolved in the run-target checkout when `workflow_source` is omitted, or in the explicit workflow-source checkout when present. */ 'workflow': string; + 'workflow_source'?: AutomationGitWorkflowSource; 'triggers': Array; } diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 72c519693..a22871b82 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -31,6 +31,7 @@ export * from './auth-session-user'; export * from './auth-sessions-response'; export * from './automation'; export * from './automation-api-trigger'; +export * from './automation-git-workflow-source'; export * from './automation-list-meta'; export * from './automation-list-response'; export * from './automation-ref'; @@ -304,6 +305,7 @@ export * from './replace-mcp-server-request'; export * from './repo-check-response'; export * from './repo-check-response-permissions'; export * from './repository-ref'; +export * from './resolved-automation-git-workflow-source'; export * from './review-target'; export * from './review-target-kind'; export * from './rewind-request'; diff --git a/lib/packages/fabro-api-client/src/models/replace-automation-request.ts b/lib/packages/fabro-api-client/src/models/replace-automation-request.ts index 2049caff8..efd33f03c 100644 --- a/lib/packages/fabro-api-client/src/models/replace-automation-request.ts +++ b/lib/packages/fabro-api-client/src/models/replace-automation-request.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { AutomationGitWorkflowSource } from './automation-git-workflow-source'; // May contain unused imports in some cases // @ts-ignore import type { AutomationTrigger } from './automation-trigger'; @@ -32,8 +35,9 @@ export interface ReplaceAutomationRequest { 'environment_id': string; 'target': RunTarget; /** - * Workflow slug or path resolved in the selected repository checkout. + * Workflow slug or path resolved in the run-target checkout when `workflow_source` is omitted, or in the explicit workflow-source checkout when present. */ 'workflow': string; + 'workflow_source'?: AutomationGitWorkflowSource; 'triggers': Array; } 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 new file mode 100644 index 000000000..668c3ee20 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/resolved-automation-git-workflow-source.ts @@ -0,0 +1,41 @@ +/* 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. + */ + + + +/** + * 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; + /** + * Required branch fallback and audit context. + */ + '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. + */ + 'resolved_sha': string; +}