mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
Fix workflow slug field to use kebab-case instead of snake_case (#556)
Multi-word workflow slugs typed into the New/Edit Automation form were
being silently converted to snake_case (e.g. `patch-cves` →
`patch_cves`), causing scheduled automations to resolve against a
non-existent directory and **silently never fire**.
## What changed
- `automation-form.tsx`: `onChange` for the Workflow slug field now
calls `kebabify()` instead of the removed `snakeify()`. The "create from
run" fallback prefill is updated the same way. Help text and placeholder
are updated to reflect dash-separated slugs.
- `snakeify()` is removed entirely (was only used in these two spots).
- `kebabify()` is unexported (it was `export function`; it's now only
used within the same file).
- `automations-new.test.tsx`: updates the pre-populate assertion from
`"fix_ci"` → `"fix-ci"`, adds a regression test that dashes are
preserved and `"Patch CVEs"` → `"patch-cves"`, and adds a unit test for
the `automationFormValuesFromRun` kebab fallback.
## Why kebab-case is correct
Workflow slugs are derived from on-disk directory names
(`.fabro/workflows/patch-cves/`), which are dash-separated by
convention. The backend validator already accepts dashes; `AutomationId`
actually forbids underscores. The snake_case behavior was a UI-only
outlier present since the form's first draft with no documented
rationale.
No backend changes are needed. Existing automations with a stored
snake_cased `workflow` selector will need a manual `PUT` to correct the
value — that is an operational fix, out of scope here.
### Fabro Details
<details>
<summary>Ran 8 stages in 31m 57s for $6.85</summary>
| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| toolchain | 1s | – | 0 |
| preflight_compile | 2m 20s | – | 0 |
| preflight_lint | 2m 33s | – | 0 |
| implement | 4m 28s | $2.46 | 0 |
| simplify_fable | 8m 39s | $3.31 | 0 |
| simplify_gpt | 2m 25s | $1.07 | 0 |
| verify | 11m 2s | – | 0 |
| **Total** | **31m 57s** | **$6.85** | **0** |
</details>
<details>
<summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14
edges)</summary>
```dot
digraph ImplementPlan {
graph [
goal="Implement and simplify",
model_stylesheet="
* { model: claude-opus-4-8; }
"
]
rankdir=LR
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0]
preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0]
preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0]
fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3]
implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"]
simplify_fable [label="Simplify (Fable)", prompt="@prompts/simplify.md", model="claude-fable-5", reasoning_effort="xhigh"]
simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"]
verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"]
fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3]
start -> toolchain
toolchain -> preflight_compile [condition="outcome=succeeded"]
toolchain -> exit
preflight_compile -> preflight_lint [condition="outcome=succeeded"]
preflight_compile -> exit
preflight_lint -> implement [condition="outcome=succeeded"]
preflight_lint -> fix_lints
fix_lints -> preflight_lint
implement -> simplify_fable -> simplify_gpt -> verify
verify -> exit [condition="outcome=succeeded"]
verify -> fixup
fixup -> verify
}
```
</details>
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro <noreply@fabro.sh>
This commit is contained in:
fabro-sh-fabro[bot]
GitHub
parent
8c7d5dc7d0
commit
09d1a6036e
2 changed files with 48 additions and 17 deletions
|
|
@ -86,7 +86,7 @@ export function automationFormValuesFromRun(
|
|||
name,
|
||||
repository,
|
||||
ref: cloneBranch ?? EMPTY_AUTOMATION_FORM.ref,
|
||||
workflow: run.workflow.slug?.trim() || snakeify(workflowName),
|
||||
workflow: run.workflow.slug?.trim() || kebabify(workflowName),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -116,7 +116,7 @@ export function isFormValid(values: AutomationFormValues): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
export function kebabify(value: string): string {
|
||||
function kebabify(value: string): string {
|
||||
return value
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9-]+/g, "-")
|
||||
|
|
@ -124,14 +124,6 @@ export function kebabify(value: string): string {
|
|||
.replace(/^-|-$/g, "");
|
||||
}
|
||||
|
||||
export function snakeify(value: string): string {
|
||||
return value
|
||||
.toLowerCase()
|
||||
.replace(/[^a-z0-9_]+/g, "_")
|
||||
.replace(/_+/g, "_")
|
||||
.replace(/^_|_$/g, "");
|
||||
}
|
||||
|
||||
function firstPresentString(...values: Array<string | null | undefined>): string {
|
||||
for (const value of values) {
|
||||
const trimmed = value?.trim();
|
||||
|
|
@ -300,15 +292,15 @@ export function AutomationFormFields({
|
|||
</Row>
|
||||
<Row
|
||||
title={<Label required>Workflow slug</Label>}
|
||||
help="Snake-case identifier used in the workflow file name (e.g. fix_build.fabro)."
|
||||
help="Dash-separated identifier matching the workflow directory name (e.g. patch-cves)."
|
||||
>
|
||||
<input
|
||||
type="text"
|
||||
name="workflow_slug"
|
||||
aria-label="Workflow slug"
|
||||
value={values.workflow}
|
||||
onChange={(e) => patch({ workflow: snakeify(e.target.value) })}
|
||||
placeholder="fix_build"
|
||||
onChange={(e) => patch({ workflow: kebabify(e.target.value) })}
|
||||
placeholder="patch-cves"
|
||||
autoComplete="off"
|
||||
spellCheck={false}
|
||||
className={`${INPUT_CLASS} font-mono`}
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ mock.module("swr", () => ({
|
|||
}));
|
||||
|
||||
const { default: AutomationsNew } = await import("./automations-new");
|
||||
const { automationFormValuesFromRun } = await import("../components/automation-form");
|
||||
mock.restore();
|
||||
|
||||
function makeRun(overrides: Record<string, unknown> = {}) {
|
||||
|
|
@ -109,7 +110,7 @@ function makeRun(overrides: Record<string, unknown> = {}) {
|
|||
goal: "Fix CI",
|
||||
title: "Fix failing tests",
|
||||
workflow: {
|
||||
slug: "fix_ci",
|
||||
slug: "fix-ci",
|
||||
name: "Fix CI",
|
||||
graph_name: "ci_graph",
|
||||
node_count: 0,
|
||||
|
|
@ -215,12 +216,26 @@ async function renderAutomationsNew(initialEntry: string) {
|
|||
return { renderer, router };
|
||||
}
|
||||
|
||||
function byLabel(renderer: TestRenderer.ReactTestRenderer, label: string) {
|
||||
return renderer.root.findByProps({ "aria-label": label });
|
||||
}
|
||||
|
||||
function fieldValue(renderer: TestRenderer.ReactTestRenderer, label: string) {
|
||||
return renderer.root.findByProps({ "aria-label": label }).props.value;
|
||||
return byLabel(renderer, label).props.value;
|
||||
}
|
||||
|
||||
function changeField(
|
||||
renderer: TestRenderer.ReactTestRenderer,
|
||||
label: string,
|
||||
value: string,
|
||||
) {
|
||||
act(() => {
|
||||
byLabel(renderer, label).props.onChange({ target: { value } });
|
||||
});
|
||||
}
|
||||
|
||||
function switchChecked(renderer: TestRenderer.ReactTestRenderer, label: string) {
|
||||
const props = renderer.root.findByProps({ "aria-label": label }).props;
|
||||
const props = byLabel(renderer, label).props;
|
||||
return props["aria-checked"] ?? props.checked;
|
||||
}
|
||||
|
||||
|
|
@ -265,6 +280,16 @@ describe("AutomationsNew", () => {
|
|||
expect(switchChecked(renderer, "Enable scheduled triggers")).toBe(false);
|
||||
});
|
||||
|
||||
test("workflow slug input normalizes to kebab-case and preserves dashes", async () => {
|
||||
const { renderer } = await renderAutomationsNew("/automations/new");
|
||||
|
||||
changeField(renderer, "Workflow slug", "patch-cves");
|
||||
expect(fieldValue(renderer, "Workflow slug")).toBe("patch-cves");
|
||||
|
||||
changeField(renderer, "Workflow slug", "Patch CVEs");
|
||||
expect(fieldValue(renderer, "Workflow slug")).toBe("patch-cves");
|
||||
});
|
||||
|
||||
test("/automations/new?from_run=run_1 pre-populates from run and settings data", async () => {
|
||||
currentRun = makeRun();
|
||||
currentRunSettings = makeRunSettings();
|
||||
|
|
@ -275,7 +300,7 @@ describe("AutomationsNew", () => {
|
|||
expect(fieldValue(renderer, "Automation slug")).toBe("fix-failing-tests");
|
||||
expect(fieldValue(renderer, "Repository")).toBe("qltysh/fabro");
|
||||
expect(fieldValue(renderer, "Default branch")).toBe("feature/from-run");
|
||||
expect(fieldValue(renderer, "Workflow slug")).toBe("fix_ci");
|
||||
expect(fieldValue(renderer, "Workflow slug")).toBe("fix-ci");
|
||||
expect(switchChecked(renderer, "Enable manual and API triggers")).toBe(true);
|
||||
expect(switchChecked(renderer, "Enable scheduled triggers")).toBe(false);
|
||||
expect(
|
||||
|
|
@ -285,6 +310,20 @@ describe("AutomationsNew", () => {
|
|||
expect(queryCalls).toContainEqual({ hook: "useRunSettings", id: "run_1" });
|
||||
});
|
||||
|
||||
test("automationFormValuesFromRun kebab-cases the workflow name fallback", () => {
|
||||
const run = makeRun({
|
||||
workflow: {
|
||||
slug: "",
|
||||
name: "Patch CVEs",
|
||||
graph_name: "PatchCves",
|
||||
node_count: 0,
|
||||
edge_count: 0,
|
||||
},
|
||||
});
|
||||
|
||||
expect(automationFormValuesFromRun(run as any).workflow).toBe("patch-cves");
|
||||
});
|
||||
|
||||
test("missing source run data renders an editable empty form with a non-blocking error", async () => {
|
||||
currentRun = null;
|
||||
currentRunError = new Error("not found");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue