mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 00:02:13 +00:00
fix(cli): make a sync push into a fork converge on schedules and inline names (#10951)
* fix(cli): make a sync push into a fork converge on schedules and inline names Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcofduXAs9FT948Aj78V8m * fix(cli): gate the fork schedule lookup, tolerate fork-conflict, keep rendered names unique Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcofduXAs9FT948Aj78V8m * test(cli): use the OS path separator in the push convergence fixtures Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcofduXAs9FT948Aj78V8m * fix(cli): report a set-aside fork schedule flag and keep checkout inline names inside the flow folder Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcofduXAs9FT948Aj78V8m * fix(cli): enable a fork-only schedule on create, and treat a fork with no parent as owning its schedules Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JcofduXAs9FT948Aj78V8m --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
9b64a89cd4
commit
0f5a1db2ab
@@ -106,7 +106,8 @@ export async function pushSchedule(
|
||||
path: string,
|
||||
schedule: Schedule | ScheduleFile | undefined,
|
||||
localSchedule: ScheduleFile,
|
||||
permissionedAsContext?: PermissionedAsContext
|
||||
permissionedAsContext?: PermissionedAsContext,
|
||||
enabledOwnedByParent?: boolean
|
||||
): Promise<void> {
|
||||
path = removeType(path, "schedule").replaceAll(SEP, "/");
|
||||
log.debug(`Processing local schedule ${path}`);
|
||||
@@ -123,6 +124,21 @@ export async function pushSchedule(
|
||||
// Strip CLI-only boolean marker before sending to API
|
||||
delete (localSchedule as any).has_permissioned_as;
|
||||
|
||||
// In a fork, the file's `enabled` is the parent's for a path the parent
|
||||
// also has (see sync push's `parentOwnedScheduleEnabled`): the fork's own
|
||||
// flag stays as it is.
|
||||
if (enabledOwnedByParent && schedule) {
|
||||
if (
|
||||
localSchedule.enabled !== undefined &&
|
||||
localSchedule.enabled !== schedule.enabled
|
||||
) {
|
||||
log.warnAlways(
|
||||
`Schedule ${path} stays ${schedule.enabled ? "enabled" : "disabled"}: the file says ${localSchedule.enabled ? "enabled" : "disabled"}, but in a fork that flag is the parent workspace's`
|
||||
);
|
||||
}
|
||||
delete localSchedule.enabled;
|
||||
}
|
||||
|
||||
const preserveFields: { permissioned_as?: string; preserve_permissioned_as?: boolean } = {};
|
||||
if (permissionedAsContext?.userIsAdminOrDeployer) {
|
||||
if (schedule) {
|
||||
@@ -153,13 +169,9 @@ export async function pushSchedule(
|
||||
...preserveFields,
|
||||
},
|
||||
});
|
||||
// Tarball export from a fork strips `enabled` from schedule YAMLs so
|
||||
// the fork→parent git-sync round-trip can't flip the parent's state.
|
||||
// Skip the secondary setScheduleEnabled call when the local YAML
|
||||
// doesn't carry `enabled` — sending `{ enabled: undefined }` would
|
||||
// serialize to `{}` and the backend (`SetEnabled.enabled` is required)
|
||||
// would reject the request. Preserving the target's existing flag is
|
||||
// exactly the round-trip-safe behavior.
|
||||
// No `enabled` in the file (absent from the YAML, or set aside above)
|
||||
// leaves the remote flag alone: `SetEnabled.enabled` is required, so
|
||||
// `{ enabled: undefined }` would be rejected rather than ignored.
|
||||
if (
|
||||
localSchedule.enabled !== undefined &&
|
||||
localSchedule.enabled !== schedule.enabled
|
||||
@@ -167,13 +179,12 @@ export async function pushSchedule(
|
||||
log.info(colors.bold.yellow(
|
||||
`Schedule ${path} is ${localSchedule.enabled ? "enabled" : "disabled"} locally but not on remote, updating remote`
|
||||
));
|
||||
await wmill.setScheduleEnabled({
|
||||
workspace: workspace,
|
||||
await setEnabledUnlessParentOwned(
|
||||
workspace,
|
||||
path,
|
||||
requestBody: {
|
||||
enabled: localSchedule.enabled,
|
||||
},
|
||||
});
|
||||
localSchedule.enabled,
|
||||
schedule.enabled
|
||||
);
|
||||
}
|
||||
} catch (e) {
|
||||
console.error((e as any).body);
|
||||
@@ -194,6 +205,44 @@ export async function pushSchedule(
|
||||
console.error((e as any).body);
|
||||
throw e;
|
||||
}
|
||||
// A create in a fork lands disabled whatever the request says. A fork-only
|
||||
// path the file wants enabled is enabled here, so one push converges; a
|
||||
// parent-owned one stays disabled.
|
||||
if (enabledOwnedByParent !== undefined && localSchedule.enabled === true) {
|
||||
if (enabledOwnedByParent) {
|
||||
log.warnAlways(
|
||||
`Schedule ${path} created disabled: the file says enabled, but in a fork that flag is the parent workspace's`
|
||||
);
|
||||
} else {
|
||||
await setEnabledUnlessParentOwned(workspace, path, true, false);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The parent listing behind `enabledOwnedByParent` sees only what the pusher
|
||||
// may read; the backend's `fork-conflict` refusal is the last word, so a path
|
||||
// it says the parent has keeps the fork's flag rather than failing the push.
|
||||
async function setEnabledUnlessParentOwned(
|
||||
workspace: string,
|
||||
path: string,
|
||||
enabled: boolean,
|
||||
remoteEnabled: boolean
|
||||
): Promise<void> {
|
||||
try {
|
||||
await wmill.setScheduleEnabled({
|
||||
workspace,
|
||||
path,
|
||||
requestBody: { enabled },
|
||||
});
|
||||
} catch (e) {
|
||||
const conflict = parseForkConflict(e);
|
||||
if (!conflict) {
|
||||
throw e;
|
||||
}
|
||||
log.warnAlways(
|
||||
`Schedule ${path} left ${remoteEnabled ? "enabled" : "disabled"}: the parent workspace '${conflict.parentWorkspaceId}' has the same schedule, so its flag is the parent's to set`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+185
-51
@@ -25,7 +25,7 @@ import {
|
||||
} from "yaml";
|
||||
import JSZip from "jszip";
|
||||
import { minimatch } from "minimatch";
|
||||
import { yamlParseContent } from "../../utils/yaml.ts";
|
||||
import { yamlParseContent, yamlParseFile } from "../../utils/yaml.ts";
|
||||
import * as wmill from "../../../gen/services.gen.ts";
|
||||
|
||||
import {
|
||||
@@ -1138,7 +1138,7 @@ export function rawAppPathWithinFolder(
|
||||
return resolved;
|
||||
}
|
||||
|
||||
function ZipFSElement(
|
||||
export function ZipFSElement(
|
||||
zip: JSZip,
|
||||
useYaml: boolean,
|
||||
defaultTs: "bun" | "deno",
|
||||
@@ -1146,6 +1146,14 @@ function ZipFSElement(
|
||||
resourceTypeToIsFileset: Record<string, boolean>,
|
||||
ignoreCodebaseChanges: boolean,
|
||||
stripOnBehalfOf: boolean,
|
||||
// Names a flow's rendered inline-script files after the checkout's own
|
||||
// `!inline` references (module id -> file). The export carries script
|
||||
// source, never a reference, so without a checkout to defer to every file
|
||||
// is named from the step summary, and a file the checkout names otherwise
|
||||
// reads as a delete + add on every push while the resolved flows are equal.
|
||||
localFlowInlineMapping?: (
|
||||
flowDir: string,
|
||||
) => Promise<Record<string, string>>,
|
||||
): DynFSElement {
|
||||
// Pre-scan: find zip base paths of scripts that have modules.
|
||||
// These scripts use the folder layout: {basePath}__mod/script.{ext}
|
||||
@@ -1249,59 +1257,71 @@ function ZipFSElement(
|
||||
log.error(`Failed to parse flow.yaml at path: ${p}`);
|
||||
throw error;
|
||||
}
|
||||
let inlineScripts;
|
||||
let inlineScripts: InlineScript[];
|
||||
try {
|
||||
const assigner = newPathAssigner(defaultTs, {
|
||||
skipInlineScriptSuffix: getNonDottedPaths(),
|
||||
});
|
||||
// Preserve original !inline filenames from the flow to avoid phantom renames
|
||||
const inlineMapping = extractCurrentMapping(
|
||||
flow.value.modules as any,
|
||||
{},
|
||||
flow.value.failure_module,
|
||||
flow.value.preprocessor_module,
|
||||
);
|
||||
inlineScripts = extractInlineScriptsForFlows(
|
||||
flow.value.modules as any,
|
||||
inlineMapping,
|
||||
SEP,
|
||||
defaultTs,
|
||||
assigner,
|
||||
{
|
||||
// Extraction rewrites the modules' content into `!inline` refs,
|
||||
// so each attempt works on its own copy of the flow.
|
||||
const render = (
|
||||
source: OpenFlow,
|
||||
inlineMapping: Record<string, string>,
|
||||
): [OpenFlow, InlineScript[]] => {
|
||||
const f: OpenFlow = structuredClone(source);
|
||||
const assigner = newPathAssigner(defaultTs, {
|
||||
skipInlineScriptSuffix: getNonDottedPaths(),
|
||||
});
|
||||
const options = {
|
||||
skipInlineScriptSuffix: getNonDottedPaths(),
|
||||
failOnInlineDirective: true,
|
||||
},
|
||||
);
|
||||
if (flow.value.failure_module) {
|
||||
inlineScripts.push(
|
||||
...extractInlineScriptsForFlows(
|
||||
[flow.value.failure_module],
|
||||
inlineMapping,
|
||||
SEP,
|
||||
defaultTs,
|
||||
assigner,
|
||||
{
|
||||
skipInlineScriptSuffix: getNonDottedPaths(),
|
||||
failOnInlineDirective: true,
|
||||
},
|
||||
),
|
||||
);
|
||||
}
|
||||
if (flow.value.preprocessor_module) {
|
||||
inlineScripts.push(
|
||||
...extractInlineScriptsForFlows(
|
||||
[flow.value.preprocessor_module],
|
||||
inlineMapping,
|
||||
SEP,
|
||||
defaultTs,
|
||||
assigner,
|
||||
{
|
||||
skipInlineScriptSuffix: getNonDottedPaths(),
|
||||
failOnInlineDirective: true,
|
||||
},
|
||||
),
|
||||
};
|
||||
const scripts = extractInlineScriptsForFlows(
|
||||
f.value.modules as any,
|
||||
inlineMapping,
|
||||
SEP,
|
||||
defaultTs,
|
||||
assigner,
|
||||
options,
|
||||
);
|
||||
if (f.value.failure_module) {
|
||||
scripts.push(
|
||||
...extractInlineScriptsForFlows(
|
||||
[f.value.failure_module],
|
||||
inlineMapping,
|
||||
SEP,
|
||||
defaultTs,
|
||||
assigner,
|
||||
options,
|
||||
),
|
||||
);
|
||||
}
|
||||
if (f.value.preprocessor_module) {
|
||||
scripts.push(
|
||||
...extractInlineScriptsForFlows(
|
||||
[f.value.preprocessor_module],
|
||||
inlineMapping,
|
||||
SEP,
|
||||
defaultTs,
|
||||
assigner,
|
||||
options,
|
||||
),
|
||||
);
|
||||
}
|
||||
return [f, scripts];
|
||||
};
|
||||
const inlineMapping = localFlowInlineMapping
|
||||
? await localFlowInlineMapping(finalPath)
|
||||
: {};
|
||||
let rendered = render(flow, inlineMapping);
|
||||
// The assigner keeps the names it hands out unique, not the
|
||||
// checkout's: one of those equal to another step's
|
||||
// summary-derived name would leave two files at one path, so
|
||||
// such a flow renders the export's way.
|
||||
if (
|
||||
new Set(rendered[1].map((s) => s.path)).size !==
|
||||
rendered[1].length
|
||||
) {
|
||||
rendered = render(flow, {});
|
||||
}
|
||||
[flow, inlineScripts] = rendered;
|
||||
} catch (error) {
|
||||
log.error(
|
||||
`Failed to extract inline scripts for flow at path: ${p}`,
|
||||
@@ -2581,7 +2601,7 @@ export function preservePendingScriptLocks(
|
||||
}
|
||||
}
|
||||
|
||||
async function compareDynFSElement(
|
||||
export async function compareDynFSElement(
|
||||
els1: DynFSElement,
|
||||
els2: DynFSElement | undefined,
|
||||
ignore: (path: string, isDirectory: boolean) => boolean,
|
||||
@@ -2594,6 +2614,9 @@ async function compareDynFSElement(
|
||||
branchOverride?: string,
|
||||
isEls1Remote?: boolean,
|
||||
caseInsensitiveFs?: boolean,
|
||||
// Which schedule files carry an `enabled` that is not the target's to set
|
||||
// (see push's `parentOwnedScheduleEnabled`): those compare without it.
|
||||
parentOwnsScheduleEnabled?: (scheduleFilePath: string) => boolean,
|
||||
): Promise<{ changes: Change[]; localMap: Record<string, string> }> {
|
||||
let [m1, m2] = els2
|
||||
? await Promise.all([
|
||||
@@ -2785,12 +2808,28 @@ async function compareDynFSElement(
|
||||
);
|
||||
throw error;
|
||||
}
|
||||
if (
|
||||
parentOwnsScheduleEnabled &&
|
||||
getTypeStrFromPath(k) === "schedule" &&
|
||||
parentOwnsScheduleEnabled(k)
|
||||
) {
|
||||
delete parsedV?.enabled;
|
||||
delete parsedM2?.enabled;
|
||||
}
|
||||
if (deepEqual(parsedV, parsedM2)) {
|
||||
continue;
|
||||
}
|
||||
} else if (k.endsWith(".yaml")) {
|
||||
const before = parseYaml(k, m2[k]);
|
||||
const after = parseYaml(k, v);
|
||||
if (
|
||||
parentOwnsScheduleEnabled &&
|
||||
getTypeStrFromPath(k) === "schedule" &&
|
||||
parentOwnsScheduleEnabled(k)
|
||||
) {
|
||||
delete before?.enabled;
|
||||
delete after?.enabled;
|
||||
}
|
||||
if (deepEqual(before, after)) {
|
||||
continue;
|
||||
}
|
||||
@@ -4543,6 +4582,86 @@ async function checkServerLockJobs(
|
||||
}
|
||||
}
|
||||
|
||||
// The checkout's `!inline` references of one flow (module id -> file), as
|
||||
// `ZipFSElement`'s `localFlowInlineMapping` names the remote render. Empty
|
||||
// when the flow has no local flow.yaml.
|
||||
export async function checkoutInlineNames(
|
||||
flowYamlPath: string,
|
||||
): Promise<Record<string, string>> {
|
||||
let flow: any;
|
||||
try {
|
||||
flow = await yamlParseFile(flowYamlPath);
|
||||
} catch {
|
||||
return {};
|
||||
}
|
||||
const mapping = extractCurrentMapping(
|
||||
flow?.value?.modules,
|
||||
{},
|
||||
flow?.value?.failure_module,
|
||||
flow?.value?.preprocessor_module,
|
||||
);
|
||||
// A reference that leaves the flow folder would render the remote step
|
||||
// onto another item's path; such a step keeps its summary-derived name.
|
||||
for (const [id, ref] of Object.entries(mapping)) {
|
||||
if (path.isAbsolute(ref) || ref.split(/[\\/]/).includes("..")) {
|
||||
delete mapping[id];
|
||||
}
|
||||
}
|
||||
return mapping;
|
||||
}
|
||||
|
||||
// For a path the parent also has, a fork's export writes the parent's
|
||||
// `enabled` and the backend refuses to enable the fork's copy: the file's flag
|
||||
// is the parent's. A parent that cannot be listed (a fork-scoped job token)
|
||||
// may own every path. Undefined when the target is not a fork.
|
||||
async function parentOwnedScheduleEnabled(
|
||||
workspaceId: string,
|
||||
): Promise<((scheduleFilePath: string) => boolean) | undefined> {
|
||||
let parentWorkspaceId: string | null | undefined;
|
||||
let known = false;
|
||||
try {
|
||||
const { workspaces } = await wmill.listUserWorkspaces();
|
||||
const entry = workspaces?.find((w) => w.id === workspaceId);
|
||||
known = entry !== undefined;
|
||||
parentWorkspaceId = entry?.parent_workspace_id;
|
||||
} catch {
|
||||
// A fork-scoped token cannot list workspaces.
|
||||
}
|
||||
// No parent on record (a fork whose parent was deleted keeps its
|
||||
// `wm-fork-` id): nothing defers to a parent any more.
|
||||
if (known && !parentWorkspaceId) {
|
||||
return undefined;
|
||||
}
|
||||
// Without the listing only the `wm-fork-` prefix says fork: a dev
|
||||
// workspace (custom id) reached with a fork-scoped token counts as none.
|
||||
if (!isForkWorkspace(workspaceId, parentWorkspaceId)) {
|
||||
return undefined;
|
||||
}
|
||||
let parentPaths: Set<string> | undefined;
|
||||
if (parentWorkspaceId) {
|
||||
try {
|
||||
parentPaths = new Set();
|
||||
const perPage = 100;
|
||||
for (let page = 1; ; page++) {
|
||||
const batch = await wmill.listSchedules({
|
||||
workspace: parentWorkspaceId,
|
||||
page,
|
||||
perPage,
|
||||
});
|
||||
batch.forEach((s) => parentPaths!.add(s.path));
|
||||
if (batch.length < perPage) break;
|
||||
}
|
||||
} catch {
|
||||
parentPaths = undefined;
|
||||
}
|
||||
}
|
||||
return (scheduleFilePath) =>
|
||||
parentPaths === undefined ||
|
||||
parentPaths.has(
|
||||
removeType(scheduleFilePath, "schedule").replaceAll(SEP, "/"),
|
||||
);
|
||||
}
|
||||
|
||||
export async function push(
|
||||
opts: GlobalOptions &
|
||||
SyncOptions & {
|
||||
@@ -4633,6 +4752,9 @@ export async function push(
|
||||
|
||||
// Merge CLI flags with resolved settings (CLI flags take precedence only for explicit overrides)
|
||||
opts = mergeCliWithEffectiveOptions(originalCliOpts, effectiveOpts);
|
||||
const parentOwnsScheduleEnabled = opts.includeSchedules
|
||||
? await parentOwnedScheduleEnabled(workspace.workspaceId)
|
||||
: undefined;
|
||||
|
||||
if (opts.lint) {
|
||||
log.info("Running lint validation before push...");
|
||||
@@ -4696,6 +4818,10 @@ export async function push(
|
||||
// ignore
|
||||
}
|
||||
|
||||
// See ZipFSElement's `localFlowInlineMapping`.
|
||||
const localFlowInlineMapping = (flowDir: string) =>
|
||||
checkoutInlineNames(path.join(process.cwd(), flowDir, "flow.yaml"));
|
||||
|
||||
const remote = ZipFSElement(
|
||||
(await downloadZip(
|
||||
workspace,
|
||||
@@ -4721,6 +4847,7 @@ export async function push(
|
||||
resourceTypeToIsFileset,
|
||||
false,
|
||||
parseSyncBehavior(opts.syncBehavior) >= 1,
|
||||
localFlowInlineMapping,
|
||||
);
|
||||
|
||||
const local = await FSFSElement(
|
||||
@@ -4741,6 +4868,7 @@ export async function push(
|
||||
wsNameForFiles,
|
||||
false, // els1 (local) is not the remote source
|
||||
await isCaseInsensitiveFilesystem(process.cwd()),
|
||||
parentOwnsScheduleEnabled,
|
||||
);
|
||||
|
||||
// Detect resources/variables that the local config flags as ws_specific
|
||||
@@ -5807,6 +5935,9 @@ export async function push(
|
||||
originalLocalPath: originalWorkspaceSpecificPath,
|
||||
permissionedAsContext,
|
||||
wsSpecific: isWsSpecific ? true : undefined,
|
||||
enabledOwnedByParent: parentOwnsScheduleEnabled?.(
|
||||
change.path,
|
||||
),
|
||||
keyPushOpts: {
|
||||
noninteractive:
|
||||
(opts.yes ?? false) || !process.stdin.isTTY,
|
||||
@@ -5942,6 +6073,9 @@ export async function push(
|
||||
originalLocalPath: localFilePath,
|
||||
permissionedAsContext,
|
||||
wsSpecific: isAddedWsSpecific ? true : undefined,
|
||||
enabledOwnedByParent: parentOwnsScheduleEnabled?.(
|
||||
change.path,
|
||||
),
|
||||
keyPushOpts: {
|
||||
noninteractive:
|
||||
(opts.yes ?? false) || !process.stdin.isTTY,
|
||||
|
||||
@@ -41,6 +41,12 @@ export function warnStderr(msg: unknown) {
|
||||
console.error(`\x1b[33m${String(msg)}\x1b[39m`);
|
||||
}
|
||||
|
||||
// A notice that must reach a log even in silent (`--json-output`) mode:
|
||||
// stderr keeps stdout parseable, and a job that runs the CLI records both.
|
||||
export function warnAlways(msg: unknown) {
|
||||
console.error(`\x1b[33m${String(msg)}\x1b[39m`);
|
||||
}
|
||||
|
||||
export function error(msg: unknown) {
|
||||
console.error(`\x1b[31m${String(msg)}\x1b[39m`);
|
||||
}
|
||||
|
||||
+4
-1
@@ -189,6 +189,8 @@ export interface PushObjOptions {
|
||||
keyPushOpts?: PushWorkspaceKeyOptions;
|
||||
/** TypeScript runtime a bare `.ts` denotes, for raw-app runnables */
|
||||
defaultTs?: "bun" | "deno";
|
||||
/** schedule push into a fork: the file's `enabled` is the parent's */
|
||||
enabledOwnedByParent?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -217,6 +219,7 @@ export async function pushObj(
|
||||
wsSpecific,
|
||||
keyPushOpts,
|
||||
defaultTs,
|
||||
enabledOwnedByParent,
|
||||
} = opts;
|
||||
const typeEnding = getTypeStrFromPath(p);
|
||||
|
||||
@@ -250,7 +253,7 @@ export async function pushObj(
|
||||
} else if (typeEnding === "resource-type") {
|
||||
await pushResourceType(workspace, p, befObj, newObj);
|
||||
} else if (typeEnding === "schedule") {
|
||||
await pushSchedule(workspace, p, befObj, newObj, permissionedAsContext);
|
||||
await pushSchedule(workspace, p, befObj, newObj, permissionedAsContext, enabledOwnedByParent);
|
||||
} else if (typeEnding === "http_trigger") {
|
||||
await pushTrigger("http", workspace, p, befObj, newObj, permissionedAsContext);
|
||||
} else if (typeEnding === "websocket_trigger") {
|
||||
|
||||
@@ -0,0 +1,298 @@
|
||||
import { afterAll, beforeAll, expect, test } from "bun:test";
|
||||
import JSZip from "jszip";
|
||||
import { mkdtempSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join, sep } from "node:path";
|
||||
import {
|
||||
checkoutInlineNames,
|
||||
compareDynFSElement,
|
||||
ZipFSElement,
|
||||
} from "../src/commands/sync/sync.ts";
|
||||
|
||||
// The differ also reads the working tree (shared lockfiles, dependency files);
|
||||
// an empty one keeps that out of the picture.
|
||||
const originalCwd = process.cwd();
|
||||
beforeAll(() => {
|
||||
process.chdir(mkdtempSync(join(tmpdir(), "wmill-push-diff-")));
|
||||
});
|
||||
afterAll(() => {
|
||||
process.chdir(originalCwd);
|
||||
});
|
||||
|
||||
// A push is only useful when a second run of it finds nothing left to do.
|
||||
// These pin the two shapes that used to be listed on every run of a push into
|
||||
// a fork while the push itself either applied nothing or aborted.
|
||||
|
||||
type Mock = {
|
||||
isDirectory: boolean;
|
||||
path: string;
|
||||
getContentText(): Promise<string>;
|
||||
getChildren(): AsyncIterable<Mock>;
|
||||
};
|
||||
|
||||
// Both sides of the differ use the OS separator; fixtures are written with
|
||||
// "/" and rows are read back the same way.
|
||||
const osPath = (p: string) => p.split("/").join(sep);
|
||||
const slashPath = (p: string) => p.split(sep).join("/");
|
||||
|
||||
function local(files: Record<string, string>): Mock {
|
||||
return {
|
||||
isDirectory: true,
|
||||
path: "",
|
||||
async getContentText() {
|
||||
return "";
|
||||
},
|
||||
async *getChildren() {
|
||||
for (const [path, content] of Object.entries(files)) {
|
||||
yield {
|
||||
isDirectory: false,
|
||||
path: osPath(path),
|
||||
async getContentText() {
|
||||
return content;
|
||||
},
|
||||
async *getChildren() {},
|
||||
};
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const noIgnore = () => false;
|
||||
|
||||
async function diff(
|
||||
localEl: Mock,
|
||||
remoteEl: Mock,
|
||||
skips: Record<string, unknown>,
|
||||
parentOwnsScheduleEnabled?: (scheduleFilePath: string) => boolean,
|
||||
) {
|
||||
const { changes } = await compareDynFSElement(
|
||||
localEl as any,
|
||||
remoteEl as any,
|
||||
noIgnore,
|
||||
false,
|
||||
skips as any,
|
||||
true,
|
||||
[],
|
||||
false,
|
||||
undefined,
|
||||
undefined,
|
||||
false,
|
||||
false,
|
||||
parentOwnsScheduleEnabled,
|
||||
);
|
||||
return changes.map((c) => `${c.name} ${slashPath(c.path)}`);
|
||||
}
|
||||
|
||||
const SCHEDULE = (enabled: string) =>
|
||||
`summary: nightly\nargs: {}\nenabled: ${enabled}\nis_flow: true\nschedule: 0 0 0 * * *\nscript_path: f/mail/flow\ntimezone: UTC\n`;
|
||||
|
||||
test("push into a fork: a schedule the parent also has compares without `enabled`", async () => {
|
||||
const remote = local({ "f/mail/nightly.schedule.yaml": SCHEDULE("true") });
|
||||
const skips = { includeSchedules: true };
|
||||
// The parent has `f/mail/nightly`; any other schedule is the fork's own.
|
||||
const parentHas = (filePath: string) =>
|
||||
slashPath(filePath) === "f/mail/nightly.schedule.yaml";
|
||||
|
||||
// Not a fork: `enabled` is compared like any other field.
|
||||
expect(
|
||||
await diff(
|
||||
local({ "f/mail/nightly.schedule.yaml": SCHEDULE("false") }),
|
||||
remote,
|
||||
skips,
|
||||
),
|
||||
).toEqual(["edited f/mail/nightly.schedule.yaml"]);
|
||||
expect(
|
||||
await diff(
|
||||
local({ "f/mail/nightly.schedule.yaml": SCHEDULE("false") }),
|
||||
remote,
|
||||
skips,
|
||||
parentHas,
|
||||
),
|
||||
).toEqual([]);
|
||||
// The key being absent is the same case as it differing.
|
||||
expect(
|
||||
await diff(
|
||||
local({
|
||||
"f/mail/nightly.schedule.yaml": SCHEDULE("false").replace(
|
||||
"enabled: false\n",
|
||||
"",
|
||||
),
|
||||
}),
|
||||
remote,
|
||||
skips,
|
||||
parentHas,
|
||||
),
|
||||
).toEqual([]);
|
||||
// Only `enabled` is set aside.
|
||||
expect(
|
||||
await diff(
|
||||
local({
|
||||
"f/mail/nightly.schedule.yaml": SCHEDULE("false").replace(
|
||||
"0 0 0 * * *",
|
||||
"0 0 1 * * *",
|
||||
),
|
||||
}),
|
||||
remote,
|
||||
skips,
|
||||
parentHas,
|
||||
),
|
||||
).toEqual(["edited f/mail/nightly.schedule.yaml"]);
|
||||
// A schedule only the fork has keeps toggling from the file.
|
||||
expect(
|
||||
await diff(
|
||||
local({ "f/mail/fork_only.schedule.yaml": SCHEDULE("true") }),
|
||||
local({ "f/mail/fork_only.schedule.yaml": SCHEDULE("false") }),
|
||||
skips,
|
||||
parentHas,
|
||||
),
|
||||
).toEqual(["edited f/mail/fork_only.schedule.yaml"]);
|
||||
});
|
||||
|
||||
const SUMMARY = "process one mail end-to-end (spam check, classify)";
|
||||
|
||||
function remoteFlow(content: string) {
|
||||
const zip = new JSZip();
|
||||
zip.file(
|
||||
"f/mail/flow_v2.flow.json",
|
||||
JSON.stringify({
|
||||
summary: "Flow V2",
|
||||
description: "",
|
||||
value: {
|
||||
modules: [
|
||||
{
|
||||
id: "a",
|
||||
summary: SUMMARY,
|
||||
value: {
|
||||
type: "rawscript",
|
||||
content,
|
||||
input_transforms: {},
|
||||
language: "python3",
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
schema: { type: "object", properties: {} },
|
||||
}),
|
||||
// The backend's archive carries no directory entries.
|
||||
{ createFolders: false },
|
||||
);
|
||||
return zip;
|
||||
}
|
||||
|
||||
function localFlow(content: string) {
|
||||
return local({
|
||||
"f/mail/flow_v2.flow/flow.yaml": `summary: Flow V2\ndescription: ''\nvalue:\n modules:\n - id: a\n summary: ${SUMMARY}\n value:\n type: rawscript\n content: '!inline process_mail.inline_script.py'\n input_transforms: {}\n language: python3\nschema:\n type: object\n properties: {}\n`,
|
||||
"f/mail/flow_v2.flow/process_mail.inline_script.py": content,
|
||||
});
|
||||
}
|
||||
|
||||
// The checkout's `!inline` references, as `push` reads them from its flow.yaml.
|
||||
const checkoutNames = async (flowDir: string) =>
|
||||
slashPath(flowDir) === "f/mail/flow_v2.flow"
|
||||
? { a: "process_mail.inline_script.py" }
|
||||
: {};
|
||||
|
||||
test("push: an inline script the checkout names differently from the step summary is not a rename", async () => {
|
||||
const skips = { includeSchedules: false };
|
||||
const render = (content: string, withCheckout: boolean) =>
|
||||
ZipFSElement(
|
||||
remoteFlow(content),
|
||||
true,
|
||||
"bun",
|
||||
{},
|
||||
{},
|
||||
false,
|
||||
true,
|
||||
withCheckout ? checkoutNames : undefined,
|
||||
) as any;
|
||||
|
||||
// Same content, file named by hand: three rows before, none after.
|
||||
expect(
|
||||
await diff(
|
||||
localFlow("def main():\n return 1\n"),
|
||||
render("def main():\n return 1\n", false),
|
||||
skips,
|
||||
),
|
||||
).toEqual([
|
||||
"deleted f/mail/flow_v2.flow/process_one_mail_end-to-end_(spam_check,_classify).inline_script.py",
|
||||
"edited f/mail/flow_v2.flow/flow.yaml",
|
||||
"added f/mail/flow_v2.flow/process_mail.inline_script.py",
|
||||
]);
|
||||
expect(
|
||||
await diff(
|
||||
localFlow("def main():\n return 1\n"),
|
||||
render("def main():\n return 1\n", true),
|
||||
skips,
|
||||
),
|
||||
).toEqual([]);
|
||||
|
||||
// A real edit is still one.
|
||||
expect(
|
||||
await diff(
|
||||
localFlow("def main():\n return 2\n"),
|
||||
render("def main():\n return 1\n", true),
|
||||
skips,
|
||||
),
|
||||
).toEqual(["edited f/mail/flow_v2.flow/process_mail.inline_script.py"]);
|
||||
});
|
||||
|
||||
test("push: a checkout name that collides with another step's summary-derived name keeps two files", async () => {
|
||||
const zip = new JSZip();
|
||||
zip.file(
|
||||
"f/mail/flow_v2.flow.json",
|
||||
JSON.stringify({
|
||||
summary: "Flow V2",
|
||||
description: "",
|
||||
value: {
|
||||
modules: [
|
||||
{
|
||||
id: "a",
|
||||
summary: SUMMARY,
|
||||
value: {
|
||||
type: "rawscript",
|
||||
content: "a",
|
||||
input_transforms: {},
|
||||
language: "python3",
|
||||
},
|
||||
},
|
||||
{
|
||||
id: "b",
|
||||
summary: "process_mail",
|
||||
value: {
|
||||
type: "rawscript",
|
||||
content: "b",
|
||||
input_transforms: {},
|
||||
language: "python3",
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
schema: { type: "object", properties: {} },
|
||||
}),
|
||||
{ createFolders: false },
|
||||
);
|
||||
// Nothing local: every rendered file is a "deleted" row, one per path.
|
||||
const rows = await diff(
|
||||
local({}),
|
||||
ZipFSElement(zip, true, "bun", {}, {}, false, true, checkoutNames) as any,
|
||||
{ includeSchedules: false },
|
||||
);
|
||||
expect(rows.filter((r) => r.endsWith(".py")).sort()).toEqual([
|
||||
"deleted f/mail/flow_v2.flow/process_mail.inline_script.py",
|
||||
"deleted f/mail/flow_v2.flow/process_one_mail_end-to-end_(spam_check,_classify).inline_script.py",
|
||||
]);
|
||||
});
|
||||
|
||||
test("push: checkout inline names stay inside the flow folder", async () => {
|
||||
const flowYaml = join(process.cwd(), "flow.yaml");
|
||||
writeFileSync(
|
||||
flowYaml,
|
||||
`summary: x\nvalue:\n modules:\n - id: a\n value:\n type: rawscript\n content: '!inline a.inline_script.py'\n language: python3\n - id: b\n value:\n type: rawscript\n content: '!inline ../shared/b.py'\n language: python3\n - id: c\n value:\n type: rawscript\n content: '!inline /tmp/c.py'\n language: python3\n`,
|
||||
);
|
||||
expect(await checkoutInlineNames(flowYaml)).toEqual({
|
||||
a: "a.inline_script.py",
|
||||
});
|
||||
expect(
|
||||
await checkoutInlineNames(join(process.cwd(), "missing.yaml")),
|
||||
).toEqual({});
|
||||
});
|
||||
Reference in New Issue
Block a user