Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/plans/goose-schedules-in-berd.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ The current Berd pin already includes typed ACP schedule list, update, pause/res
| Layer | Scope | Acceptance |
|---|---|---|
| 1. `feat(automations): show local Goose schedules in OSS` | Give local schedules an OSS capability independent of the KGoose-gated company Automations capability; mount a read-only local schedule list in the shared hub; keep company-only details, history, builder, and requests gated. | With company automations disabled, list local jobs and status, and prove that company list/detail requests are not made. In the Block build, both sources remain visible in the same hub. |
| 2. `feat(automations): configure local Goose schedules` | Add guarded cron editing to the embedded schedule list, with supported-expression validation and save/cancel states. | Invalid input is rejected before reaching ACP, and valid updates use the typed update method against the live scheduler. The current Goose update handler mutates the active job before validating the new cron, so the UI must not pass unsupported expressions. |
| 2. `feat(automations): configure local Goose schedules` | Add cron editing to the embedded schedule list with explicit save/cancel, conservative validation, and error states that account for the pinned backend's update ordering. | Invalid or unsupported input is rejected before ACP; valid edits use the typed update method. Failed updates clearly warn that the current Goose backend may have removed the prior timer. |
| 3. `feat(automations): manage local Goose schedules` | Add pause/resume, stop active runs, confirmed removal, and bounded per-schedule run history on demand. Share the local ACP adapter between UI and `berdctl` if/when CLI controls are restacked. | Mutations use typed ACP on the same live Goose process; enforce running-job restrictions; removal preserves the recipe; history is fetched only when requested and remains bounded. Review deletion behavior against the exact Goose version before adding any multi-step pause/delete/rollback sequence. |
| Future upstream dependency | Add durable project/workspace context to scheduled jobs and pass it into every run's session creation. Then expose project filtering/association in Berd. | Schedule association survives restart and each execution uses the selected project's stable ID and workspace roots. |

Each proposed layer should be reviewed as a working prefix. The current first implementation covers only Layer 1; the later controls and upstream project-scoped execution work are planned separately because they carry distinct behavioral risks.
Each proposed layer should be reviewed as a working prefix. Layer 1 exposes read-only schedules. Layer 2 allows guarded cron updates but cannot guarantee the previous timer survives an unexpected backend rejection; failed updates warn users to verify the schedule and cannot be retried unchanged because Goose's same-expression shortcut may falsely report recovery. Pause/resume, stop, removal, run history, and upstream project-scoped execution remain separate follow-on work.

## Project association

Expand Down
27 changes: 25 additions & 2 deletions src/features/automations/api/localSchedules.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { getClient } from "@/shared/api/acpConnection";
import { listLocalSchedules } from "./localSchedules";
import { listLocalSchedules, updateLocalScheduleCron } from "./localSchedules";

const listSchedules = vi.hoisted(() => vi.fn());
const updateSchedule = vi.hoisted(() => vi.fn());

vi.mock("@/shared/api/acpConnection", () => ({
getClient: vi.fn(async () => ({
goose: { GooseUnstableSchedulesList: listSchedules },
goose: {
GooseUnstableSchedulesList: listSchedules,
GooseUnstableSchedulesUpdate: updateSchedule,
},
})),
}));

describe("local Goose schedules API", () => {
beforeEach(() => {
vi.clearAllMocks();
updateSchedule.mockResolvedValue({
job: {
id: "weekday-report",
source: "/tmp/scheduled_recipes/weekday-report.yaml",
cron: "0 10 * * 1-5",
currentlyRunning: false,
paused: true,
},
});
listSchedules.mockResolvedValue({
jobs: [
{
Expand All @@ -39,4 +52,14 @@ describe("local Goose schedules API", () => {
expect(getClient).toHaveBeenCalledOnce();
expect(listSchedules).toHaveBeenCalledWith({});
});

it("updates a schedule through the connected Goose instance", async () => {
await expect(
updateLocalScheduleCron("weekday-report", "0 10 * * 1-5"),
).resolves.toMatchObject({ cron: "0 10 * * 1-5", paused: true });
expect(updateSchedule).toHaveBeenCalledWith({
scheduleId: "weekday-report",
cron: "0 10 * * 1-5",
});
});
});
12 changes: 12 additions & 0 deletions src/features/automations/api/localSchedules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,15 @@ export async function listLocalSchedules(): Promise<ScheduledJobDto[]> {
const response = await client.goose.GooseUnstableSchedulesList({});
return response.jobs;
}

export async function updateLocalScheduleCron(
scheduleId: string,
cron: string,
): Promise<ScheduledJobDto> {
const client = await getClient();
const response = await client.goose.GooseUnstableSchedulesUpdate({
scheduleId,
cron,
});
return response.job;
}
43 changes: 43 additions & 0 deletions src/features/automations/lib/localScheduleCron.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { describe, expect, it } from "vitest";
import { validateLocalScheduleCron } from "./localScheduleCron";

describe("validateLocalScheduleCron", () => {
it.each([
"0 9 * * 1-5",
"*/15 * * * *",
"0,30 */5 8-17 * * 0,7",
"*/10 * * * *",
"0 0 0 28 * *",
"0 0 0 */1 * *",
"*/10 * * * * *",
"0 0 9 * * MON",
" 0 9 * * 1-5 ",
])("accepts supported cron expression %s", (cron) => {
expect(validateLocalScheduleCron(cron)).toBeNull();
});

it.each([
"",
"@daily",
"0 0 9 * * * 2028",
"60 * * * *",
"0 24 * * *",
"0 0 0 32 * *",
"0 0 0 1 FEB *",
"0 0 0 * 13 *",
"0 0 0 * * 8",
"0 0 0 * * 5-2",
"0 0 9 1 */1 MON",
"0 0 0 * * */0",
"0 0 0 * * 1,,2",
"0 0 0 * * 1/999",
"0 0 0 L * *",
"*,15 9 * * *",
"*/5,15 9 * * *",
"0 0 0 1 * MON",
"0 0 0 * * 1#2",
"0 9 * * 1-5\uFEFF",
])("rejects expressions outside the safe subset: %s", (cron) => {
expect(validateLocalScheduleCron(cron)).not.toBeNull();
});
});
150 changes: 150 additions & 0 deletions src/features/automations/lib/localScheduleCron.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
const FIELD_BOUNDS = [
[0, 59],
[0, 59],
[0, 23],
[1, 31],
[1, 12],
[0, 7],
] as const;

const MONTH_NAMES: Record<string, number> = {
JAN: 1,
FEB: 2,
MAR: 3,
APR: 4,
MAY: 5,
JUN: 6,
JUL: 7,
AUG: 8,
SEP: 9,
OCT: 10,
NOV: 11,
DEC: 12,
};

const WEEKDAY_NAMES: Record<string, number> = {
SUN: 0,
MON: 1,
TUE: 2,
WED: 3,
THU: 4,
FRI: 5,
SAT: 6,
};

const MONTH_ALIASES = new RegExp(
`\\b(${Object.keys(MONTH_NAMES).join("|")})\\b`,
"g",
);
const WEEKDAY_ALIASES = new RegExp(
`\\b(${Object.keys(WEEKDAY_NAMES).join("|")})\\b`,
"g",
);

function resolveValue(value: string, fieldIndex: number): number | null {
const upperValue = value.toUpperCase();
const resolved =
fieldIndex === 4
? MONTH_NAMES[upperValue]
: fieldIndex === 5
? WEEKDAY_NAMES[upperValue]
: undefined;
if (resolved !== undefined) return resolved;
if (!/^\d+$/.test(value)) return null;
const parsed = Number(value);
return Number.isSafeInteger(parsed) ? parsed : null;
}

function isFullWildcard(field: string): boolean {
// Goose only marks a literal `*` as wildcard for day-of-month/weekday
// matching; `*/1` is parsed as all values but still combined with AND.
return field === "*";
}

function isValidCronField(field: string, fieldIndex: number): boolean {
const [minimum, maximum] = FIELD_BOUNDS[fieldIndex];
if (field.length === 0) return false;
const items = field.split(",");

for (const item of items) {
if (!item) return false;

const steppedParts = item.split("/");
if (steppedParts.length > 2) return false;
const [base, stepText] = steppedParts;
if (stepText !== undefined) {
const step = resolveValue(stepText, fieldIndex);
if (step === null || step < 1 || step > maximum - minimum + 1) {
return false;
}
}

if (base === "*") {
if (items.length !== 1) return false;
continue;
}

const rangeParts = base.split("-");
if (rangeParts.length > 2) return false;
const start = resolveValue(rangeParts[0], fieldIndex);
const end =
rangeParts.length === 2 ? resolveValue(rangeParts[1], fieldIndex) : start;
if (
start === null ||
end === null ||
start < minimum ||
end > maximum ||
start > end
) {
return false;
}
}

return true;
}

/**
* Checks Berd's deliberately conservative editable cron subset before calling
* the pinned Goose scheduler, whose update path removes the old timer before
* parsing the replacement. Existing expressions outside this subset remain
* visible and can still be saved unchanged. The selected ruleset also excludes
* expressions whose recurrence semantics are ambiguous or infrequent.
*/
export function validateLocalScheduleCron(cron: string): string | null {
if ([...cron].some((character) => character.charCodeAt(0) > 127)) {
return "unsupported";
}
const trimmedCron = cron.trim();
if (!trimmedCron) return "unsupported";
const fields = trimmedCron.split(/[ \t\r\n\v\f]+/);
if (fields.length !== 5 && fields.length !== 6) {
return "fieldCount";
}

const normalized = fields.join(" ").toUpperCase();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 P1 · Validate aliases in their fields (blocking)

The validator replaces every month and weekday name across the entire expression before it validates fields. That turns an unsupported expression such as 0 9 * * JAN into a valid numeric weekday expression and 0 MON * * * into a valid numeric hour, while the editor sends the original raw expression to Goose. This defeats the field-aware lookup below and the conservative boundary whose purpose is to keep backend-rejectable input away from an update path that can remove the old timer before parsing the replacement.

User effect: A typo that uses a valid cron name in the wrong field can appear safe and be submitted. Goose may reject it after removing the existing timer, leaving the schedule unable to run, or interpret it with a cadence the user did not intend.

Recommended fix: Validate names within each field instead of globally canonicalizing the expression: allow month names only in the month field, weekday names only in the weekday field, and require numeric step values. Reject every name in seconds, minute, hour, day-of-month, or the wrong named field before calling ACP.

Test: Add rejection cases for 0 9 * * JAN, 0 MON * * *, JAN 9 * * *, 0 0 9 * * DEC, and named step values, while retaining acceptance coverage for weekday names and ranges in the weekday field.

const canonical = normalized
.replace(MONTH_ALIASES, (name) => String(MONTH_NAMES[name]))
.replace(WEEKDAY_ALIASES, (name) => String(WEEKDAY_NAMES[name]));
const canonicalFields = canonical.split(" ");
const fieldsWithSeconds =
canonicalFields.length === 5 ? ["0", ...canonicalFields] : canonicalFields;

if (
!fieldsWithSeconds.every((field, index) => isValidCronField(field, index))
) {
return "unsupported";
}

// Keep edits to frequent, unambiguous recurrences: no month restriction,
// and no simultaneous day-of-month + weekday restriction (Goose combines
// those with AND).
if (
!isFullWildcard(fieldsWithSeconds[4]) ||
(!isFullWildcard(fieldsWithSeconds[3]) &&
!isFullWildcard(fieldsWithSeconds[5]))
) {
return "safeWindow";
}

return null;
}
83 changes: 83 additions & 0 deletions src/features/automations/lib/localScheduleSpanish.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { describe, expect, it } from "vitest";
import esAutomations from "@/shared/i18n/locales/es/automations.json";

describe("Spanish local schedule copy", () => {
it.each([
[
"loadErrorDescription",
"El programador local no está disponible o Goose no está conectado.",
],
[
"refreshError",
"No se pudieron actualizar las programaciones: {{error}}. Se muestra la última lista cargada.",
],
["emptyTitle", "Aún no hay programaciones locales"],
[
"emptyBody",
"Las programaciones que crees en Goose aparecerán aquí. Usa la CLI de Goose u otro cliente de Goose para programar y configurar recetas.",
],
["running", "En ejecución"],
["lastRun", "Última ejecución: {{time}}"],
["recoverCron", "Recuperar la programación de {{id}}"],
["recoverAction", "Recuperar programación"],
["uncertainTitle", "Es necesario verificar el temporizador"],
["editCron", "Editar la expresión cron de {{id}}"],
["cronLabel", "Expresión cron de {{id}}"],
["reconcilingSchedule", "Comprobando el estado actual de la programación…"],
["updateError", "No se pudo actualizar la programación de Goose."],
[
"updateFailureCaution",
"Es posible que el programador haya quitado el temporizador anterior si la actualización falló inesperadamente. Verifica la programación en Goose antes de depender de ella.",
],
[
"refreshAfterUpdateError",
"La actualización de la lista también falló, así que se desconoce el estado actual de la programación.",
],
[
"failedUpdateRetry",
"Esta misma expresión ya falló. No la reintentes: es posible que Goose haya quitado el temporizador anterior. Verifica la programación o prueba otra expresión válida.",
],
[
"changedElsewhere",
"Esta programación cambió mientras la editabas. Cancela y vuelve a abrirla para revisar el valor más reciente.",
],
] as const)("preserves accented copy for %s", (key, expected) => {
expect(esAutomations.localSchedules[key]).toBe(expected);
});

it("preserves accented cron guidance", () => {
for (const word of [
"día-del-mes",
"día-de-la-semana",
"numéricos",
"días",
"más",
"comodín",
"podría",
"actualización",
"programación",
"después",
]) {
expect(esAutomations.localSchedules.cronHelp).toContain(word);
}
});

it.each([
["fieldCount", "Ingresa una expresión cron de cinco o seis campos."],
[
"unsupported",
"La expresión no es válida o usa una sintaxis que este editor no puede actualizar de forma segura. La programación existente no cambió.",
],
[
"safeWindow",
"Usa un comodín para el mes y un comodín en día-del-mes o día-de-la-semana. Goose exige que ambos campos de día coincidan si ninguno es comodín y los combina con AND.",
],
] as const)("preserves accented validation for %s", (key, expected) => {
expect(esAutomations.localSchedules.validation[key]).toBe(expected);
});

it("retains the active and unknown last-run labels", () => {
expect(esAutomations.localSchedules.active).toBe("Activa");
expect(esAutomations.localSchedules.unknownLastRun).toBe("Desconocida");
});
});
Loading
Loading